Skip to content

fix(ci): show why the gate said revise, not just that it did - #374

Merged
unohee merged 2 commits into
mainfrom
fix/action-surface-the-verdict
Aug 1, 2026
Merged

fix(ci): show why the gate said revise, not just that it did#374
unohee merged 2 commits into
mainfrom
fix/action-surface-the-verdict

Conversation

@unohee

@unohee unohee commented Aug 1, 2026

Copy link
Copy Markdown
Owner

The first run of the gate with working tools returned revise. The job log contained the single word "revise".

--json suppresses the human report by design, and SARIF carries only findings that have a file and line — the analysis for that run uploaded 0 results. So the feedback and any unlocated issues, which is often the whole of a revise, appeared nowhere. An operator gets a verdict and no way to act on it, which is most of the gate's value missing.

Fix

The review document is rendered into both the step log and the GitHub job summary: decision, feedback, issues, and follow-ups with locations.

The exit-1 message now quotes the actual decision instead of asserting "rejected this change", so it cannot misreport if that mapping ever widens beyond reject.

Verification

Exercised against the local fixture harness with a revise document carrying feedback, an unlocated issue, and two follow-ups (one with a location, one without). All of it reaches the log and $GITHUB_STEP_SUMMARY; bash -n and YAML parse clean.

The first run of the gate that had working tools returned `revise` — and the job
log contained the single word "revise". `--json` suppresses the human report by
design, and SARIF carries only findings that have a file and line, so feedback
and unlocated issues — often the whole of a `revise` — appeared nowhere at all.
An operator got a verdict with no way to act on it, which is most of the value
of the gate missing.

The document is now rendered into the step log and the GitHub job summary:
decision, feedback, issues, and follow-ups with their locations.

The exit-1 message quotes the actual decision rather than asserting "rejected",
so it cannot misreport if that mapping ever widens.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2e12b10cf3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread action.yml Outdated
Comment thread action.yml
Both from review of the previous commit, and the first one is a hole that commit
opened.

Rendering the review document into the step log meant printing model-generated
text — written after reading an untrusted diff — straight to stdout. GitHub
turns any stdout line beginning with `::` into a workflow command, so injected
prose could forge annotations, or emit `::stop-commands::` and silence the rest
of the step including the verdict itself. The block is now wrapped in
stop-commands with a random token the writer cannot predict, which is GitHub's
documented fence. Verified against a review whose feedback contains both an
`::error::` and a `::stop-commands::`: they render as text, and the `::notice::`
after the closing token still executes, so the fence closes cleanly.

The summary file is Markdown and is not parsed for commands, so it takes the
text as-is.

`suggestions` was also being dropped. It is part of the ReviewJson contract and
the normal report renders it, so a failing `revise` could still withhold the
proposed fix — which is the whole thing this change exists to surface.
@unohee

unohee commented Aug 1, 2026

Copy link
Copy Markdown
Owner Author

Both findings addressed — the first was a hole this PR opened, which is a fair catch on a change whose entire subject is making a gate honest.

P2 · workflow commands in review prose. Rendering the document to the step log meant printing model-generated text, written after reading an untrusted diff, straight to stdout. Any stdout line starting with :: is a workflow command, so injected prose could forge annotations or emit ::stop-commands:: and silence the rest of the step — including the verdict. The block is now wrapped in stop-commands with a random token the writer cannot predict.

Verified against a review whose feedback contains both ::error::FORGED ANNOTATION and ::stop-commands::pwned:

::stop-commands::c215cc54-…
## OpenSwarm review: revise
Looks fine.
::error::FORGED ANNOTATION
::stop-commands::pwned
…
::c215cc54-…::
::notice::OpenSwarm review: revise      ← still executes

Both forgeries render as text, and the real ::notice:: after the closing token still works — so the fence closes cleanly rather than leaving commands disabled for the remainder of the step. The job summary is Markdown and is not command-parsed, so it takes the text as-is.

P2 · dropped suggestions. Correct, and it defeated the point: suggestions is part of the ReviewJson contract and the normal report renders it, so a failing revise could still withhold the proposed fix. Added as its own section before the follow-ups.

@unohee
unohee merged commit f19ec99 into main Aug 1, 2026
10 checks passed
@unohee
unohee deleted the fix/action-surface-the-verdict branch August 1, 2026 11:06
@unohee unohee mentioned this pull request Aug 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant