CI: fix claude-code-review checkout failure + enable track_progress - #15
Conversation
The review step's `prompt:` input puts claude-code-action into automation mode, which by default posts nothing to the PR until the whole run finishes. Combined with the cost gate that waits for sibling CI, a review can look silently stuck for the better part of an hour with no visible progress. track_progress: true posts a live-updating tracking comment with a checklist instead. Pattern modeled on Postgres-Extensions/cat_tools PR #69. Note: because this workflow runs on pull_request_target, GitHub always executes the workflow file from the base branch (master), never a PR's own version -- so this PR's own claude-review check will still run the old workflow without track_progress. The new behavior can only be verified on a subsequent PR, after this one merges. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
|
Note on CI: the `claude-review` check is failing here, but it's a pre-existing issue unrelated to this change — it fails identically on other recent PRs (#10, #14) at the "Check out PR head" step, before the step this PR modifies ever runs: ``` This is All substantive checks (the PostgreSQL 9.3–17 matrix and |
actions/checkout v4.4.0 (backported to all major-version tags) added a new default-on refusal for checking out a fork PR's head under pull_request_target, since it can't see that this job is already gated to the trusted jnasbyupgrade fork only (see the if: condition and SECURITY comment above) and never builds or executes the fetched code. Root-caused via the actual failed run logs on PRs Postgres-Extensions#10/Postgres-Extensions#14/Postgres-Extensions#15, which all failed at this checkout step with: Refusing to check out fork pull request code from a 'pull_request_target' workflow. ... set 'allow-unsafe-pr-checkout: true' on the actions/checkout step. Without this, track_progress: true (added in the prior commit) never had a chance to matter -- the workflow was failing before the review step ever ran.
jnasbyupgrade
changed the title
… check The head.repo.owner.login check is the entire security boundary that makes allow-unsafe-pr-checkout: true safe on the checkout step below. Make that explicit and unmissable, not just implied by a comment on the checkout step referencing it.
f3b7d8e
into
Postgres-Extensions:master
jnasbyupgrade
changed the title
…eckout fix is on master)
…eckout fix is on master)
#28) 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.
1 participant
Two independent fixes to the Claude review workflow:
run) —
actions/checkout's newallow-unsafe-pr-checkoutguard (addedin v4.4.0, backported to all major tags) blocks checking out fork-PR
code under
pull_request_targetunless explicitly opted into. Addedallow-unsafe-pr-checkout: truewith a comment explaining why it's safehere (job already gated to the trusted fork, checked-out code is only
ever read, never built or executed).
track_progress: trueon the review step, so it posts a live-updatingchecklist comment instead of staying silent until the whole run
finishes (pattern from
cat_toolsPR #69).Confirmed via actual failed-run logs that the checkout error is the only
thing failing — nothing else lurking behind it.
Note: this workflow runs on
pull_request_target, which alwaysexecutes from
master— so this PR's ownclaude-reviewcheck can'tverify either fix. Both need a follow-up PR after merge to confirm.