ci(security-review): fix origin/HEAD missing so /security-review's git diff works#1327
Open
tejaskash wants to merge 1 commit into
Open
ci(security-review): fix origin/HEAD missing so /security-review's git diff works#1327tejaskash wants to merge 1 commit into
tejaskash wants to merge 1 commit into
Conversation
…works The /security-review slash command runs `git diff origin/HEAD...` as its first action to enumerate the PR's changes. actions/checkout doesn't set up the remote's symbolic HEAD ref, so that command fails with "ambiguous argument 'origin/HEAD...': unknown revision". Claude then loops trying variants until --max-turns 30 trips and the action exits 1. Set origin/HEAD to the PR's base ref right after checkout so the skill's git invocations resolve correctly. Run #26186045056 on PR #1321 was the trigger - 30 turns spent on shell-error recovery, no findings posted.
Contributor
Package TarballHow to installgh release download pr-1327-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.14.1.tgz |
agentcore-cli-automation
approved these changes
May 20, 2026
agentcore-cli-automation
left a comment
There was a problem hiding this comment.
LGTM. The diagnosis matches the failure (actions/checkout doesn't materialize refs/remotes/origin/HEAD, and git diff origin/HEAD... is the first thing the bundled /security-review skill runs), and the fix is the right shape:
fetch-depth: 0already pulls everyrefs/heads/*intorefs/remotes/origin/*(seegetRefSpecForAllHistoryin actions/checkout v6), soorigin/$BASE_REFis guaranteed to exist locally beforegit remote set-headruns.- Using the PR's actual
base.ref(rather than--auto, which would query the remote default branch) is correctly more conservative — a PR targeting a release branch will diff against that release branch, which is what a security review actually wants. set -euo pipefailplus the trailinggit symbolic-ref refs/remotes/origin/HEADgives a clean fail-fast if the symbolic ref somehow didn't get set, instead of letting the failure surface 30 turns deep inside Claude.
No product code touched, so telemetry/mocking guidance doesn't apply. Ship it.
Contributor
|
Claude Security Review: no high-confidence findings. (run) |
Contributor
Coverage Report
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Run #26186045056 on PR #1321 failed with
Reached maximum number of turns (30)because the very first thing the bundled/security-reviewskill does isgit diff origin/HEAD..., and that resolves to:actions/checkoutclones the remote and setsorigin/<head-ref>but does not set the remote's symbolicHEADref. Soorigin/HEADis undefined, the skill's git command crashes, Claude tries 30 turns of recovery variants, then the action exits 1.This affects every PR run after #1310 merged (we switched from inlining the prompt + custom git context to letting the bundled skill drive its own git commands).
What changes
One step inserted right after
Checkout PR head:git remote set-head origin <base-ref>makesorigin/HEADresolve to the PR's base branch. The follow-upgit symbolic-refline is a sanity print that fails the step early if the head still isn't set.Test plan
safe-to-review); confirm the security review run no longer hitserror_max_turns, that the skill'sgit diff origin/HEAD...resolves correctly, and that inline findings (or a "no findings" summary) post as expected.