From 7f2b5c1d5b13cc905536418acaa242696f396dd2 Mon Sep 17 00:00:00 2001 From: Anuj Hydrabadi Date: Tue, 15 Sep 2026 17:51:40 +0530 Subject: [PATCH] fix(module-ci): report raptor's real outcome per output type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PR comment restated the flag rather than the result, so every row read "created if absent" — including the rows that were skipped because the type already exists. That is precisely the question the comment exists to answer: which definitions does this pull request create, and which already exist and will therefore change when it merges? A per-file constant cannot answer it. Found on a live run against a real modules repository: four rows, one genuinely created and three untouched, all four labelled identically. The step now captures raptor's own output and takes the outcome from it, adds the resolved @namespace/name, and carries the compatibility findings into a "Reported changes" column. A row that says "skipped (already exists)" with findings is the one a reviewer has to look at — those changes land on merge. Findings have "|" escaped so a finding cannot split the table cell. Co-Authored-By: Claude Opus 5 --- module-ci-action/README.md | 2 +- module-ci-action/action.yml | 28 +++++++++++++++++++++------- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/module-ci-action/README.md b/module-ci-action/README.md index 8b02796..d71ce3f 100644 --- a/module-ci-action/README.md +++ b/module-ci-action/README.md @@ -67,7 +67,7 @@ So the two modes are not the same operation at different strengths: | Mode | What happens to an output type | |------|-------------------------------| -| **preview** (pull request) | `raptor create output-type -f --if-absent`. A type the Control Plane has never seen is **created** — it has no consumers, so the write cannot break anything, and it is the only way a PR that adds a module *and* its new type can validate its own module. A type that already exists is **only reported**; the PR comment lists what would change. | +| **preview** (pull request) | `raptor create output-type -f --if-absent`. A type the Control Plane has never seen is **created** — it has no consumers, so the write cannot break anything, and it is the only way a PR that adds a module *and* its new type can validate its own module. A type that already exists is **only reported**. The PR comment gives raptor's own per-file outcome (`created` vs `skipped (already exists)`) plus any compatibility findings, so a reviewer can see which definitions this PR creates and which already exist and will therefore change on merge. | | **publish** (push) | The full apply. raptor still refuses a change that removes or retypes a field while some module produces or consumes the type, and names those modules. Run `raptor create output-type -f --allow-breaking` yourself when you really mean it. | | **cleanup** (PR closed) | Nothing. See below. | diff --git a/module-ci-action/action.yml b/module-ci-action/action.yml index 644fec1..3769d19 100644 --- a/module-ci-action/action.yml +++ b/module-ci-action/action.yml @@ -365,29 +365,43 @@ runs: { echo "### Facets output types" echo "" - echo "| Definition | Result |" - echo "|---|---|" + echo "| Definition | Type | Result | Reported changes |" + echo "|---|---|---|---|" } > "$SECTION" # shellcheck disable=SC2086,SC2154 # $output_files is an intentional space-separated list injected via $GITHUB_ENV for file in $output_files; do echo "::group::Output type ${file}" + # Capture raptor's own outcome rather than restating the flag. The whole + # point of the comment is to answer "which of these did this PR create, + # and which already exist and will therefore CHANGE on merge?" — a + # per-file constant cannot answer it. # shellcheck disable=SC2086 # $MODE_FLAG is a deliberate empty-or-one-flag word - if raptor create output-type -f "$file" $MODE_FLAG; then - RESULT="applied" - if [ "$MODE" = "preview" ]; then RESULT="created if absent"; fi + if OUTPUT="$(raptor create output-type -f "$file" $MODE_FLAG 2>&1)"; then + printf '%s\n' "$OUTPUT" + RESULT="$(printf '%s\n' "$OUTPUT" | sed -n "s/^✓ Output type '.*' \(.*\)\$/\1/p" | tail -n1)" + [ -n "$RESULT" ] || RESULT="applied" else + printf '%s\n' "$OUTPUT" echo "::error::Output type apply failed for ${file}" echo "${file} (output-type)" >> "$FAILURES" RESULT="failed" fi - echo "| \`${file}\` | ${RESULT} |" >> "$SECTION" + + # The reference raptor resolved, and the compatibility findings it + # printed. A row that says "skipped (already exists)" with findings is + # the one a reviewer has to look at: those changes land on merge. + REF="$(printf '%s\n' "$OUTPUT" | sed -n "s/^✓ Output type '\(@[^']*\)'.*\$/\1/p" | tail -n1)" + # Join on "; " and escape "|", which would otherwise split the table cell. + NOTES="$(printf '%s\n' "$OUTPUT" | sed -n 's/^ ⚠️ //p' | sed 's/|/\\|/g' | paste -sd ';' - | sed 's/;/; /g')" + [ -n "$NOTES" ] || NOTES="—" + echo "| \`${file}\` | \`${REF:-?}\` | ${RESULT} | ${NOTES} |" >> "$SECTION" echo "::endgroup::" done { echo "" - echo "_On a pull request an output type is only created when the control plane does not have it. A change to an existing type is reported here and applied on merge, because an output type has no version and no preview stage._" + echo "_On a pull request an output type is only created when the control plane does not have it. A row that already exists is left untouched — any change listed against it lands when this PR merges, because an output type has no version and no preview stage._" } >> "$SECTION" - name: Preview modules (pull request)