Skip to content

CI: fix claude-code-review by removing the fork-checkout step entirely - #28

Merged
jnasbyupgrade merged 1 commit into
masterfrom
fix-review-checkout-pattern
Aug 5, 2026
Merged

CI: fix claude-code-review by removing the fork-checkout step entirely#28
jnasbyupgrade merged 1 commit into
masterfrom
fix-review-checkout-pattern

Conversation

@jnasbyupgrade

Copy link
Copy Markdown
Contributor

PR #15's allow-unsafe-pr-checkout: true fix silenced the checkout-refusal
error but was solving the wrong problem, and traded it for a new one:
after merging, review runs on #10 and #16 failed with a different error,
fatal: couldn't find remote ref pull/10/head.

Root cause: anthropics/claude-code-action's own docs/security.md
explicitly names our checkout step's pattern (checking out the PR's own
untrusted ref, from the fork, into the workspace root) as the anti-pattern
to avoid, and its "preferred" fix is a plain checkout of the base ref with
no override. The action fetches and reads the PR's actual content itself
-- confirmed by reading its source (src/github/operations/branch.ts): for
a fork PR it runs git fetch origin ... pull/<n>/head, a ref GitHub
maintains on the BASE repo for any PR (fork or not), so it never needs
direct access to the fork's remote. Our step redirecting origin to the
fork broke that internal fetch, since refs/pull/<n>/head doesn't exist
there.

Fix: remove the repository:/ref:/allow-unsafe-pr-checkout overrides
entirely -- just uses: actions/checkout@v7 with no inputs, checking out
this repo's own base branch. Updated the surrounding comments (the
top-of-file SECURITY note and the job's trust-check warning) to match --
they previously described the now-removed manual fork-checkout.

The if: trust gate (head.repo.owner.login == 'jnasbyupgrade') is
unchanged and still load-bearing defense-in-depth, even though the
checkout itself is now safe by construction regardless of that check.

Confirmed via gh api search/code + reading the action's actual source,
not guessed. Same pull_request_target self-verification limitation as
before: this PR's own claude-review check runs the OLD workflow from
master and can't demonstrate the fix on itself -- needs a subsequent PR
after merge to confirm.

PR #15's allow-unsafe-pr-checkout fix silenced the checkout-refusal
error but was fixing the wrong problem. anthropics/claude-code-action's
own docs/security.md calls our checkout step's pattern (checking out
the untrusted PR ref into the workspace root) the exact anti-pattern to
avoid, and shows the fix: a plain checkout of the base ref, nothing
more. The action fetches and reads the PR's actual content itself,
safely, via GitHub's refs/pull/<n>/head ref on THIS repo -- which is
also why our redirecting `origin` to the fork broke the action's own
internal fetch ("couldn't find remote ref pull/10/head": that ref only
exists on the base repo, not the fork). Confirmed by reading the
action's actual source (src/github/operations/branch.ts).
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: b85e7b16-a624-41db-8222-1bd42bd3da29

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • ✅ Review completed - (🔄 Check again to review again)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@jnasbyupgrade

Copy link
Copy Markdown
Contributor Author

@claude pls review

@claude

claude Bot commented Aug 5, 2026

Copy link
Copy Markdown

Claude finished @jnasbyupgrade's task in 1m 24s —— View job


Reviewing PR #28

  • Read PR description and diff
  • Verify claim about pull/<n>/head ref and claude-code-action source
  • Review workflow YAML changes for correctness
  • Post final review

Review

This is a solid fix and I agree with the root-cause analysis.

Correctness of the fix

  • refs/pull/<n>/head is a ref GitHub maintains on the base repo for every PR, including fork PRs — so redirecting origin to the fork (the old repository:/ref: override) broke claude-code-action's own internal git fetch origin ... pull/<n>/head, exactly as described. Removing the override and doing a plain actions/checkout@v7 (checking out this repo's own base branch) fixes that, and also sidesteps the actions/checkout v4.4.0+ refusal that PR CI: fix claude-code-review checkout failure + enable track_progress #15's allow-unsafe-pr-checkout: true was working around — there's no fork ref being checked out anymore, so the flag is no longer needed at all.
  • Worth noting: .github/workflows/claude.yml (the @claude mention handler) already uses this same plain-checkout pattern with no fork override. This change brings claude-code-review.yml into line with it, rather than introducing a new pattern.
  • Dropping the explicit fetch-depth: 1 / persist-credentials: false is fine: fetch-depth: 1 is already actions/checkout's default, and persist-credentials: false was there specifically because the old step checked out untrusted fork code — now that the workspace only ever holds this repo's own trusted base branch, there's no reason to suppress credential persistence (and the action likely needs the persisted token for its own authenticated fetch of pull/<n>/head).
  • The job-level head.repo.owner.login == 'jnasbyupgrade' trust gate is untouched and still the load-bearing check, as the PR description notes — good that it wasn't touched in a PR that's also editing security commentary right next to it.

Caveats

  • I couldn't independently fetch anthropics/claude-code-action's source (src/github/operations/branch.ts, docs/security.md) to verify byte-for-byte, since this sandbox only allows file access within this repo's working directory — I'm relying on the PR description's account of having read it directly, which is internally consistent with known GitHub Actions behavior (pull ref semantics, the v4.4.0+ checkout restriction) and with the actual failure mode described (couldn't find remote ref pull/10/head).
  • As the author already flagged: this PR's own claude-review check is running the old workflow from master (pull_request_target always uses the base branch's workflow file), so it can't self-validate the fix. A follow-up PR after merge is the right way to confirm.

No changes requested — this looks correct and the comments accurately describe the new behavior.

@jnasbyupgrade
jnasbyupgrade merged commit 12188f9 into master Aug 5, 2026
17 checks passed
jnasbyupgrade added a commit to jnasbyupgrade/extension_tools that referenced this pull request Aug 5, 2026
jnasbyupgrade added a commit to jnasbyupgrade/extension_tools that referenced this pull request Aug 5, 2026
jnasbyupgrade added a commit to jnasbyupgrade/object_reference that referenced this pull request Aug 5, 2026
allow-unsafe-pr-checkout: true plus a repository:/ref: override checking
out the fork directly is the wrong fix -- it silences the checkout
refusal but breaks claude-code-action's own internal fetch of
refs/pull/<n>/head (which only exists on this repo, not the fork), per
Postgres-Extensions/extension_tools#28 hitting and fixing the identical
mistake. The action already fetches and reads the PR's actual content
itself; this step only needs to check out the base branch.
jnasbyupgrade added a commit to Postgres-Extensions/test_factory that referenced this pull request Aug 5, 2026
…ion fetch the PR itself

The previous commit's allow-unsafe-pr-checkout fix solved the checkout-refusal
error but traded it for a different, worse one: anthropics/claude-code-action
fetches and reads a PR's actual content itself (src/github/operations/
branch.ts: for a fork PR it fetches origin's refs/pull/<n>/head, a ref GitHub
maintains on the BASE repo for any PR, fork or not). Redirecting the checkout
step's `origin` to the fork (via repository:/ref:) breaks that internal
fetch, since refs/pull/<n>/head doesn't exist on the fork's own remote --
`fatal: couldn't find remote ref pull/<n>/head`.

Caught before it ever hit CI here by checking
Postgres-Extensions/extension_tools#28, which hit and fixed the exact same
mistake (their PR #15 was the same allow-unsafe-pr-checkout approach; #28
corrected it). anthropics/claude-code-action's own docs/security.md names
this checkout pattern (checking out the PR's own untrusted ref into the
workspace) as the anti-pattern to avoid in the first place; its preferred
pattern is a plain checkout of the base ref, nothing more.

Fix: remove the repository:/ref:/allow-unsafe-pr-checkout overrides entirely
-- just `uses: actions/checkout@v7` with no inputs, checking out this repo's
own base branch. The if: trust gate is unchanged and still load-bearing
defense-in-depth, even though the checkout itself is now safe by
construction regardless of that check.

Same self-verification limitation as before: this PR's own claude-review
check runs the OLD workflow from the base branch and can't demonstrate this
on itself.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
jnasbyupgrade added a commit to Postgres-Extensions/linter that referenced this pull request Aug 5, 2026
…oggle

Three fixes to claude-code-review.yml, folded into this PR alongside the
--comment fix since they all touch the review workflow:

- The existing checkout step redirected `origin` to the PR's fork via
  `repository:`/`ref:` + `allow-unsafe-pr-checkout: true`. That breaks
  anthropics/claude-code-action's own internal PR fetch (it runs
  `git fetch origin pull/<N>/head`, a ref that only exists on the base
  repo) with "couldn't find remote ref pull/<N>/head" -- the same bug
  root-caused and fixed in Postgres-Extensions/extension_tools#28. Fix:
  drop the override entirely and just check out the base branch; the
  action fetches the actual PR head itself.
- Add `track_progress: true` so a long review posts a live-updating
  tracking comment instead of staying silent until the whole run
  finishes (cat_tools PR #69).
- Add a `claude-debug` PR label toggle: skips the cost gate and turns on
  `show_full_output` for a fast, fully-verbose debug iteration instead of
  a 5-20+ minute wait per attempt (cat_tools PR #64).

Ported from cat_tools' current claude-code-review.yml, which already has
all three fixes live.
jnasbyupgrade added a commit to Postgres-Extensions/test_factory that referenced this pull request Aug 5, 2026
…ion fetch the PR itself

The previous commit's allow-unsafe-pr-checkout fix solved the checkout-refusal
error but traded it for a different, worse one: anthropics/claude-code-action
fetches and reads a PR's actual content itself (src/github/operations/
branch.ts: for a fork PR it fetches origin's refs/pull/<n>/head, a ref GitHub
maintains on the BASE repo for any PR, fork or not). Redirecting the checkout
step's `origin` to the fork (via repository:/ref:) breaks that internal
fetch, since refs/pull/<n>/head doesn't exist on the fork's own remote --
`fatal: couldn't find remote ref pull/<n>/head`.

Caught before it ever hit CI here by checking
Postgres-Extensions/extension_tools#28, which hit and fixed the exact same
mistake (their PR #15 was the same allow-unsafe-pr-checkout approach; #28
corrected it). anthropics/claude-code-action's own docs/security.md names
this checkout pattern (checking out the PR's own untrusted ref into the
workspace) as the anti-pattern to avoid in the first place; its preferred
pattern is a plain checkout of the base ref, nothing more.

Fix: remove the repository:/ref:/allow-unsafe-pr-checkout overrides entirely
-- just `uses: actions/checkout@v7` with no inputs, checking out this repo's
own base branch. The if: trust gate is unchanged and still load-bearing
defense-in-depth, even though the checkout itself is now safe by
construction regardless of that check.

Same self-verification limitation as before: this PR's own claude-review
check runs the OLD workflow from the base branch and can't demonstrate this
on itself.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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