Summary
Under workflow_dispatch, the review-responder agent operates on the default branch (main), not the PR's head branch. It generates patches against main's file contents, which the post-agent job then pushes to the PR branch. If the PR branch has diverged from main for the file being edited, the edit is wrong (based on stale content) and may land cleanly on the PR but produce incorrect code — or fail to apply.
How it was found
While auditing checkout: config for security issue #92 (C5), I traced how the compiled lock file handles PR checkout. In .github/workflows/review-responder.lock.yml around the agent job:
- name: Checkout PR branch
if: |
github.event.pull_request || github.event.issue.pull_request
uses: actions/github-script@...
script: |
const { main } = require('${{ runner.temp }}/gh-aw/actions/checkout_pr_branch.cjs');
await main();
The if: guard evaluates against github.event fields populated only when the triggering event is pull_request or issue_comment. Our responder is dispatched via workflow_dispatch from pipeline-orchestrator.yml (gh workflow run review-responder.lock.yml -f pr_number="$PR"), so:
github.event.pull_request → undefined
github.event.issue.pull_request → undefined
- The condition is false → step is skipped
Consequence: the initial actions/checkout (with no ref: override) pulls the default branch, and nothing later switches to the PR's head branch. The agent's working tree is main throughout.
Why it often looks like it works
Most review comments target files that haven't diverged between the PR branch and main (especially for young PRs or small diffs). Our aw-labeled PRs also tend to be short-lived and frequently rebased. So patches generated against main often apply cleanly to the PR branch and happen to contain the right content. This is coincidence, not correctness.
Failure modes
- Stale edits: responder "fixes" the file based on main's content; the PR branch has a different version of that file; the push lands a commit that partially reverts the PR's changes or misses context.
- File not on main: responder tries to edit a file introduced by the PR. On main it doesn't exist → the agent may hallucinate content or skip the edit with an error that gets swallowed.
- Hunk conflicts if the post-agent
push_to_pull_request_branch handler tries to apply the patch on a ref where the surrounding lines no longer match.
Suggested fix (for discussion)
Option A — pass PR ref into the checkout explicitly:
# review-responder.md
checkout:
ref: refs/pull/${{ inputs.pr_number }}/head
fetch-depth: 1
Option B — add a manual git fetch + git checkout step driven by inputs.pr_number before the agent runs (pre-agent step).
Option C — upstream gh-aw: broaden the Checkout PR branch step's if: to also handle workflow_dispatch with a pr_number input convention.
Option A is the smallest local change.
Related
Severity
Medium. Currently a latent correctness bug, not a security issue. Most responder runs appear to produce reasonable output, but the mechanism is wrong and will fail silently on longer-lived PRs or files new to the PR.
Summary
Under
workflow_dispatch, thereview-responderagent operates on the default branch (main), not the PR's head branch. It generates patches against main's file contents, which the post-agent job then pushes to the PR branch. If the PR branch has diverged from main for the file being edited, the edit is wrong (based on stale content) and may land cleanly on the PR but produce incorrect code — or fail to apply.How it was found
While auditing
checkout:config for security issue #92 (C5), I traced how the compiled lock file handles PR checkout. In.github/workflows/review-responder.lock.ymlaround the agent job:The
if:guard evaluates againstgithub.eventfields populated only when the triggering event ispull_requestorissue_comment. Our responder is dispatched viaworkflow_dispatchfrompipeline-orchestrator.yml(gh workflow run review-responder.lock.yml -f pr_number="$PR"), so:github.event.pull_request→ undefinedgithub.event.issue.pull_request→ undefinedConsequence: the initial
actions/checkout(with noref:override) pulls the default branch, and nothing later switches to the PR's head branch. The agent's working tree ismainthroughout.Why it often looks like it works
Most review comments target files that haven't diverged between the PR branch and main (especially for young PRs or small diffs). Our
aw-labeled PRs also tend to be short-lived and frequently rebased. So patches generated against main often apply cleanly to the PR branch and happen to contain the right content. This is coincidence, not correctness.Failure modes
push_to_pull_request_branchhandler tries to apply the patch on a ref where the surrounding lines no longer match.Suggested fix (for discussion)
Option A — pass PR ref into the checkout explicitly:
Option B — add a manual
git fetch+git checkoutstep driven byinputs.pr_numberbefore the agent runs (pre-agent step).Option C — upstream gh-aw: broaden the
Checkout PR branchstep'sif:to also handleworkflow_dispatchwith apr_numberinput convention.Option A is the smallest local change.
Related
pipeline-orchestrator.ymldispatches viaworkflow_dispatch(notpull_request).github/workflows/review-responder.lock.ymlSeverity
Medium. Currently a latent correctness bug, not a security issue. Most responder runs appear to produce reasonable output, but the mechanism is wrong and will fail silently on longer-lived PRs or files new to the PR.