CI: consolidate claude-code-review.yml fixes (trust gate, checkout, track_progress, debug label, inline comments) - #27
Open
jnasbyupgrade wants to merge 9 commits into
Conversation
Combines three related CI-workflow improvements into one PR (touching only claude-code-review.yml and claude.yml): - Bump actions/checkout@v4 -> @v7 (current latest major) to clear the Node.js-20-deprecation warning it triggers on every run. Supersedes/ incorporates upstream PR Postgres-Extensions#13 and fork branch ci/bump-actions-versions, which made the same v4->v7 bump to the same two lines. anthropics/claude-code-action@v1 is still current (a floating v1 tag exists at v1.0.185) so it is left as-is. - Add track_progress: true to the claude-code-action step in claude-code-review.yml so it posts a live, updating checklist comment as it works instead of staying silent until the whole run finishes -- which, combined with this workflow's cost gate, could leave a PR dark for the better part of an hour. Disabled specifically for labeled-triggered runs (see below): the action's own track_progress validation only accepts opened/synchronize/reopened/ready_for_review for pull_request(_target) events and throws for any other action. - Add a claude-debug PR-label toggle so a maintainer can skip the cost gate and turn on full transcript output (show_full_output) by just labeling the PR, without editing/pushing the workflow file. The label is queried live via `gh pr view` inside the step rather than read from the event payload, since GitHub's "Re-run jobs" replays the original stored payload and would miss a label added afterward. `labeled` is added to the trigger types so applying the label alone starts a fresh run, scoped tightly in the job's `if:` so an unrelated label can't re-trigger this paid workflow. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
actions/checkout now refuses, by default, to check out a fork PR's head under pull_request_target (a "pwn request" guard) -- this job has always been safe to opt out of that guard (trusted-fork gate + read-only use), it just started failing when the guard shipped.
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.
The review step drives claude-code-action with a bare prompt: (no @claude mention), which runs it in "agent mode". That mode decides which MCP servers to start from an --allowedTools flag inside claude_args, not from the invoked plugin's own allowed-tools frontmatter. Without mcp__github_inline_comment__create_inline_comment listed there, that MCP server never starts, so the code-review plugin silently falls back to one consolidated PR comment instead of real per-line inline comments.
…d repo owner head.repo.owner.login only identifies the fork for fork-headed PRs; for an upstream-branch-headed PR (base and head both in this repo) it's always this repo's own org, never the actual author, so the gate silently skipped review on every such PR regardless of who opened it. user.login is GitHub's own authenticated record of who opened the PR and isn't attacker-spoofable, so this isn't a weaker check -- it's the more correct one, and covers both fork-headed and upstream-headed PRs.
…onsolidate-claude-code-review
…onsolidate-claude-code-review # Conflicts: # .github/workflows/claude-code-review.yml
…e-perms' into consolidate-claude-code-review # Conflicts: # .github/workflows/claude-code-review.yml
|
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 |
This was referenced Aug 7, 2026
Dropped along with the fork-checkout override, but it's independently worth keeping: this job never pushes anything, so there's no reason to leave a push-capable credential in .git/config for the rest of the job. Matches Postgres-Extensions/pg_count_nulls#53's version of this same fix.
jnasbyupgrade
added a commit
to Postgres-Extensions/test_factory
that referenced
this pull request
Aug 8, 2026
…ckout step Lost when the fork-checkout override (repository:/ref:) was removed from this step's with: block earlier -- deleting the whole block wholesale threw away persist-credentials: false along with it, silently reverting to actions/checkout's default of true. This job's permissions grant pull-requests: write, a real write-capable credential; nothing here legitimately runs git push, so there's no reason to leave it persisted in .git/config for the rest of the job to misuse if anything later goes wrong. Found via the same pattern in Postgres-Extensions/object_reference#27 (see ~/more-fixes.md), where the same wholesale with:-block deletion had the same effect; pg_count_nulls#53's version of this fix got it right by removing only repository:/ref: and keeping fetch-depth/persist-credentials.
jnasbyupgrade
added a commit
to Postgres-Extensions/test_factory
that referenced
this pull request
Aug 8, 2026
…ckout step Lost when the fork-checkout override (repository:/ref:) was removed from this step's with: block earlier -- deleting the whole block wholesale threw away persist-credentials: false along with it, silently reverting to actions/checkout's default of true. This job's permissions grant pull-requests: write, a real write-capable credential; nothing here legitimately runs git push, so there's no reason to leave it persisted in .git/config for the rest of the job to misuse if anything later goes wrong. Found via the same pattern in Postgres-Extensions/object_reference#27 (see ~/more-fixes.md), where the same wholesale with:-block deletion had the same effect; pg_count_nulls#53's version of this fix got it right by removing only repository:/ref: and keeping fetch-depth/persist-credentials.
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
1 participant
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.
Summary
Consolidates all the currently-open claude-code-review.yml fixes into one PR -- there's no real reason for them to be separate; that's an artifact of finding issues over time, not a design that needs three independent review points. Supersedes #15, #21, #23 (closing those, not merging separately).
user.login(PR author) instead ofhead.repo.owner.login, so upstream-branch-headed PRs (base and head both in this repo) aren't silently skipped regardless of who opened them.actions/checkout@v7, noref:/repository:override -- claude-code-action fetches and reads the PR's content itself; checking out an untrusted ref into the workspace is the anti-pattern its own docs warn against. Keptpersist-credentials: falseexplicitly (matchesPostgres-Extensions/pg_count_nulls#53's version of this same fix) -- this job never pushes anything, so there's no reason to leave a push-capable credential in.git/config.track_progress: true(disabled forlabeled-triggered runs, which the action's own validation rejects) so the review posts a live updating comment instead of staying silent until the whole run finishes.claude-debugPR label: live-queried (not the cached event payload), skips the cost-gate wait and turns on full transcript output.claude_args: '--allowedTools mcp__github_inline_comment__create_inline_comment'so the inline-comment MCP server actually starts in agent mode -- without it the plugin silently falls back to one consolidated comment instead of real per-line comments.Same
pull_request_targetself-verification limitation as each of the PRs this supersedes: this PR's ownclaude-reviewcheck runs the OLD workflow frommaster, so none of this is verifiable pre-merge. Verification happens on a subsequent PR after this merges.🤖 Generated with Claude Code