Skip to content

ci(security-review): fix origin/HEAD missing so /security-review's git diff works#1327

Merged
tejaskash merged 1 commit into
mainfrom
fix-origin-head
May 21, 2026
Merged

ci(security-review): fix origin/HEAD missing so /security-review's git diff works#1327
tejaskash merged 1 commit into
mainfrom
fix-origin-head

Conversation

@tejaskash
Copy link
Copy Markdown
Contributor

Why

Run #26186045056 on PR #1321 failed with Reached maximum number of turns (30) because the very first thing the bundled /security-review skill does is git diff origin/HEAD..., and that resolves to:

fatal: ambiguous argument 'origin/HEAD...': unknown revision or path not in the working tree

actions/checkout clones the remote and sets origin/<head-ref> but does not set the remote's symbolic HEAD ref. So origin/HEAD is 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:

- name: Set origin/HEAD for /security-review skill
  env:
    BASE_REF: ${{ steps.pr.outputs.base_ref }}
  run: |
    set -euo pipefail
    git remote set-head origin "$BASE_REF"
    git symbolic-ref refs/remotes/origin/HEAD

git remote set-head origin <base-ref> makes origin/HEAD resolve to the PR's base branch. The follow-up git symbolic-ref line is a sanity print that fails the step early if the head still isn't set.

Test plan

  • Merge.
  • Open a small PR (or wait for the next community PR + safe-to-review); confirm the security review run no longer hits error_max_turns, that the skill's git diff origin/HEAD... resolves correctly, and that inline findings (or a "no findings" summary) post as expected.

…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.
@tejaskash tejaskash requested a review from a team May 20, 2026 20:00
@github-actions github-actions Bot added the size/xs PR size: XS label May 20, 2026
@github-actions github-actions Bot added the agentcore-harness-reviewing AgentCore Harness review in progress label May 20, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Package Tarball

aws-agentcore-0.14.1.tgz

How to install

gh 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

Copy link
Copy Markdown

@agentcore-cli-automation agentcore-cli-automation left a comment

Choose a reason for hiding this comment

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

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: 0 already pulls every refs/heads/* into refs/remotes/origin/* (see getRefSpecForAllHistory in actions/checkout v6), so origin/$BASE_REF is guaranteed to exist locally before git remote set-head runs.
  • 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 pipefail plus the trailing git symbolic-ref refs/remotes/origin/HEAD gives 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.

@github-actions github-actions Bot removed the agentcore-harness-reviewing AgentCore Harness review in progress label May 20, 2026
@agentcore-devx-automation
Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label May 20, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 43.73% 9316 / 21299
🔵 Statements 42.98% 9884 / 22996
🔵 Functions 40.33% 1608 / 3987
🔵 Branches 40.45% 6058 / 14976
Generated in workflow #3164 for commit e480956 by the Vitest Coverage Report Action

Copy link
Copy Markdown
Contributor

@Hweinstock Hweinstock left a comment

Choose a reason for hiding this comment

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

nice!

@tejaskash tejaskash merged commit 34886be into main May 21, 2026
30 checks passed
@tejaskash tejaskash deleted the fix-origin-head branch May 21, 2026 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/xs PR size: XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants