From 4ad96035907c0e8dbcbb79b00dae76c5e7231582 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 31 Jul 2026 13:18:58 -0500 Subject: [PATCH 1/5] claude-code-review.yml: add missing --comment flag Same issue as pgxntool-test#61/pgxntool#84: the /code-review plugin only prints its findings to the job log by default; it needs --comment to post a PR comment. Every automated review here has run correctly but never posted anything visible on the PR. --- .github/workflows/claude-code-review.yml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index b530389..80bd5c1 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -113,4 +113,11 @@ jobs: # marketplace repo's default branch (upstream anthropics/claude-code). plugin_marketplaces: 'https://github.com/anthropics/claude-code.git' plugins: 'code-review@claude-code-plugins' - prompt: '/code-review:code-review ${{ github.repository }}/pull/${{ github.event.pull_request.number }}' + # --comment is required: without it, the code-review plugin only + # prints its findings to the job log and never posts anything to + # the PR (confirmed by capturing the hidden SDK transcript on a + # canary PR in pgxntool-test: the review correctly found an + # injected bug but ended with "No `--comment` argument was + # provided, so no GitHub comments were posted"). Every review run + # before this fix has been silently invisible on GitHub. + prompt: '/code-review:code-review ${{ github.repository }}/pull/${{ github.event.pull_request.number }} --comment' From 5c208d8b0d3f3ea16e960585b1b03d2f2c68624e Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Wed, 5 Aug 2026 16:59:17 -0500 Subject: [PATCH 2/5] Fix broken fork-checkout, add track_progress and claude-debug label toggle 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//head`, a ref that only exists on the base repo) with "couldn't find remote ref pull//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. --- .github/workflows/claude-code-review.yml | 95 ++++++++++++++++++------ 1 file changed, 71 insertions(+), 24 deletions(-) diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index 80bd5c1..a667288 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -9,14 +9,31 @@ name: Claude Code Review # write-capable token. The job is gated to PRs from the trusted `jnasbyupgrade` # fork only — an arbitrary external fork can never trigger this secret-bearing # job. The workflow file always comes from the base branch (main), so a PR -# cannot modify the reviewer that runs on it. We check out the PR head only for -# read context (persist-credentials: false) and never build or execute PR code. +# cannot modify the reviewer that runs on it. We never check out the fork's PR +# head ourselves here: anthropics/claude-code-action's own internal checkout +# logic (setupBranch() in src/github/operations/branch.ts) already fetches the +# PR branch via `git fetch origin pull//head`, which requires `origin` to be +# the BASE repo -- checking out the fork directly instead (as a prior version +# of this file did) points `origin` at the fork, which has no such ref, and +# breaks that fetch with "couldn't find remote ref pull//head". on: pull_request_target: - types: [opened, synchronize, reopened, ready_for_review] + # labeled: lets adding the claude-debug label (see the "Check for + # claude-debug label" step below) kick off a fresh run by itself, with no + # push/re-run needed. Scoped in the job's `if:` below to only actually + # proceed when the label added IS claude-debug -- otherwise every + # unrelated label added to a PR would trigger another paid review. + types: [opened, synchronize, reopened, ready_for_review, labeled] concurrency: - group: claude-review-${{ github.event.pull_request.number }} + # Concurrency cancellation resolves when a run is admitted, before the + # job's `if:` is evaluated -- a job's `if:` can only no-op itself, it can't + # un-cancel whatever the run already displaced. So only a labeled event + # whose label is NOT claude-debug gets its own per-label group here, + # keeping it from ever colliding with (and cancelling) the real review's + # group. labeled+claude-debug deliberately keeps the plain group, since + # it's meant to supersede an in-progress review. + group: claude-review-${{ github.event.pull_request.number }}${{ (github.event.action == 'labeled' && github.event.label.name != 'claude-debug') && format('-{0}', github.event.label.name) || '' }} cancel-in-progress: true jobs: @@ -24,16 +41,15 @@ jobs: # Trusted fork only, and skip drafts (don't spend API/CI on unfinished PRs). # To add more trusted owners, extend the head-owner check. # - # SECURITY-CRITICAL: this owner check is the ONLY thing that makes - # `allow-unsafe-pr-checkout: true` below acceptable. Without it, this - # pull_request_target job would check out and let Claude act on - # arbitrary fork code while holding base-repo secrets/token — a "pwn - # request". Do not remove or loosen this condition (e.g. drop the - # owner check, or allow non-owner forks) without re-evaluating the - # fork-checkout step's safety. + # SECURITY-CRITICAL: this owner check is what makes it safe to run this + # pull_request_target job -- which holds base-repo secrets/token -- on + # every fork PR unattended. Do not remove or loosen this condition (e.g. + # drop the owner check, or allow non-owner forks) without re-evaluating + # whether this job should keep running on arbitrary forks. if: >- github.event.pull_request.draft == false && - github.event.pull_request.head.repo.owner.login == 'jnasbyupgrade' + github.event.pull_request.head.repo.owner.login == 'jnasbyupgrade' && + (github.event.action != 'labeled' || github.event.label.name == 'claude-debug') runs-on: ubuntu-latest timeout-minutes: 60 permissions: @@ -41,6 +57,28 @@ jobs: pull-requests: write # post the review comments checks: read # read sibling check-runs for the cost gate steps: + # DEBUG MODE: add the "claude-debug" label to a PR to (a) skip the cost + # gate below entirely -- a debug session shouldn't wait 5-20+ min per + # iteration on sibling CI -- and (b) get show_full_output: true on the + # Run Claude Code Review step, dumping the full raw Claude Code JSON + # transcript (including tool results -- see that input's own WARNING + # below) to the job log. This is how you'd catch something like a + # silently-swallowed `--comment` flag (see that step's other comment). + # Queried live via `gh pr view`, not the static event payload, so + # adding the label and clicking "Re-run jobs" on an existing run picks + # it up without needing a new push. + - name: Check for claude-debug label + id: debug + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + PR: ${{ github.event.pull_request.number }} + run: | + enabled=$(gh pr view "$PR" --repo "$REPO" --json labels \ + --jq 'any(.labels[]; .name == "claude-debug")' 2>/dev/null) || enabled=false + echo "enabled=$enabled" >> "$GITHUB_OUTPUT" + echo "claude-debug label present: $enabled" + # COST GATE: the paid Claude review is the last thing to run. Wait for the # PR head's OTHER check-runs to finish and only proceed if they are clean. # If any sibling check failed we skip the review to avoid spending money @@ -56,6 +94,7 @@ jobs: # gate never waits on or fails because of itself. - name: Wait for CI; skip the paid review if any check failed id: gate + if: steps.debug.outputs.enabled != 'true' env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} REPO: ${{ github.repository }} @@ -82,33 +121,41 @@ jobs: echo "decision=$decision" >> "$GITHUB_OUTPUT" echo "gate decision: $decision" - - name: Check out PR head (read-only context) - if: steps.gate.outputs.decision == 'run' + - name: Check out base branch + if: steps.debug.outputs.enabled == 'true' || steps.gate.outputs.decision == 'run' # Intentionally tracks the major-version tag (not a pinned SHA) so # upstream fixes are picked up automatically. - uses: actions/checkout@v6 + # + # No `repository:`/`ref:` here on purpose — this checks out the base + # branch (main), never the fork's PR head. See the SECURITY note + # above; anthropics/claude-code-action fetches the actual PR head + # itself afterward via the base repo's `refs/pull//head` ref. + uses: actions/checkout@v7 with: - repository: ${{ github.event.pull_request.head.repo.full_name }} - ref: ${{ github.event.pull_request.head.sha }} fetch-depth: 1 persist-credentials: false - # Unsafe by default (actions/checkout refuses fork-PR checkouts - # under pull_request_target/workflow_run). Only OK here because - # the job is gated to the project owner's forks — see the `if:` - # on the `claude-review` job above; that check is what makes - # this safe. - allow-unsafe-pr-checkout: true - name: Run Claude Code Review - if: steps.gate.outputs.decision == 'run' + if: steps.debug.outputs.enabled == 'true' || steps.gate.outputs.decision == 'run' uses: anthropics/claude-code-action@v1 with: claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} + # See the "Check for claude-debug label" step above -- WARNING (from + # this input's own description): outputs ALL Claude messages + # including tool execution results, which may contain secrets, and + # these logs are publicly visible in GitHub Actions. + show_full_output: ${{ steps.debug.outputs.enabled == 'true' }} # Provide github_token so the action uses it directly for GitHub API # calls instead of the OIDC->GitHub-App-token exchange, which 401s under # pull_request_target. GITHUB_TOKEN is repo/workflow-scoped (independent # of the actor's role) and has pull-requests: write here. github_token: ${{ secrets.GITHUB_TOKEN }} + # A `prompt:` input puts the action in "automation mode", which by + # default posts nothing until the whole run finishes -- there's no + # visibility into a review that runs long. track_progress forces a + # tracking PR comment with a live checklist that updates as Claude + # works, so a slow run is visible instead of silent. + track_progress: true # NOTE: plugin_marketplaces can't be pinned — it tracks the # marketplace repo's default branch (upstream anthropics/claude-code). plugin_marketplaces: 'https://github.com/anthropics/claude-code.git' From c5c4f6c8b264eb4adf823ba1ea44707b01a560a6 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Wed, 5 Aug 2026 17:16:11 -0500 Subject: [PATCH 3/5] Add actions: write permission and inline-comment MCP allowlist Two more fixes from cat_tools' current claude-code-review.yml, per ~/security-notice.md's two newly-added addenda: - actions: write permission -- without it, claude-code-action's own setup step fails to save its Actions cache with a silent warning ("Cache reservation failed: cache write denied"), not a hard failure, so the job still passes while being slower/less-cached every run. There's no narrower "cache write only" scope GitHub offers. - claude_args --allowedTools for mcp__github_inline_comment__create_inline_comment -- agent mode (a bare prompt: with no @claude mention) only starts the inline-comment MCP server if this tool is listed in an --allowedTools flag inside claude_args; it does not consult the code-review plugin's own allowed-tools frontmatter. Without it every review silently falls back to one consolidated PR comment instead of real inline line comments, with no error to notice (cat_tools PR #62). Ported from cat_tools PR #47 and #62 respectively. --- .github/workflows/claude-code-review.yml | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index a667288..6b5354a 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -56,6 +56,13 @@ jobs: contents: read pull-requests: write # post the review comments checks: read # read sibling check-runs for the cost gate + # GitHub has no narrower "cache write" scope -- actions: write is the + # only permission that lets a step save an Actions cache entry (it also + # grants cancelling/deleting workflow runs and managing artifacts, which + # this job doesn't use). Without it, claude-code-review's own setup step + # can never write a cache, only ever miss. Granted here on top of the + # existing trusted-fork-owner gate below, not instead of it. + actions: write steps: # DEBUG MODE: add the "claude-debug" label to a PR to (a) skip the cost # gate below entirely -- a debug session shouldn't wait 5-20+ min per @@ -168,3 +175,14 @@ jobs: # provided, so no GitHub comments were posted"). Every review run # before this fix has been silently invisible on GitHub. prompt: '/code-review:code-review ${{ github.repository }}/pull/${{ github.event.pull_request.number }} --comment' + # A direct `prompt:` (no @claude mention) runs the action in "agent + # mode". In that mode, claude-code-action only installs the + # github_inline_comment MCP server if it sees + # mcp__github_inline_comment__create_inline_comment listed in an + # --allowedTools flag inside claude_args (src/modes/agent/parse-tools.ts) -- + # it does NOT look at the code-review plugin's own `allowed-tools` + # frontmatter to decide that. Without this, the MCP server never + # starts, the tool genuinely doesn't exist in the session, and the + # plugin silently falls back to one consolidated PR comment instead + # of real inline line comments. + claude_args: '--allowedTools mcp__github_inline_comment__create_inline_comment' From 08fe602fbd18703169d1e33dd1c33ee9ac9febd4 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 7 Aug 2026 13:59:22 -0500 Subject: [PATCH 4/5] Trust gate: check PR author, not head repo owner Folds in Postgres-Extensions/linter#6. head.repo.owner.login only distinguishes fork ownership for fork-headed PRs. For an upstream-branch-headed PR (base and head both in this repo -- what `gh stack` requires, or any `gh pr create` without forking), head.repo is always this repo itself, so head.repo.owner.login is always the org, never the actual author -- silently skipping review on PRs that were legitimately the trusted account's own work (confirmed elsewhere via the Checks API reporting claude-review as "skipped" on a trusted upstream-branch PR stack). Check github.event.pull_request.user.login instead: it can't be spoofed by a third party any more than head repo owner can, and it's the more direct question for this gate's actual purpose -- trusting the PERSON asking for a review, not the repository their branch happens to live in. Applied on top of this PR's already-restructured `if:` (the claude-debug label clause), rather than as PR #6's standalone diff against the original file, since both touch the same line and #6 was opened against the pre-PR-3 version of this workflow. --- .github/workflows/claude-code-review.yml | 46 +++++++++++++++--------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index 6b5354a..334d8c4 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -6,16 +6,18 @@ name: Claude Code Review # `pull_request` version never worked for fork PRs. # # SECURITY: pull_request_target runs in the BASE repo with secrets and a -# write-capable token. The job is gated to PRs from the trusted `jnasbyupgrade` -# fork only — an arbitrary external fork can never trigger this secret-bearing -# job. The workflow file always comes from the base branch (main), so a PR -# cannot modify the reviewer that runs on it. We never check out the fork's PR -# head ourselves here: anthropics/claude-code-action's own internal checkout -# logic (setupBranch() in src/github/operations/branch.ts) already fetches the -# PR branch via `git fetch origin pull//head`, which requires `origin` to be -# the BASE repo -- checking out the fork directly instead (as a prior version -# of this file did) points `origin` at the fork, which has no such ref, and -# breaks that fetch with "couldn't find remote ref pull//head". +# write-capable token. The job is gated to PRs opened by the trusted +# `jnasbyupgrade` account only (PR author, not head repo owner -- see the +# job's own `if:` comment for why) — an untrusted author can never trigger +# this secret-bearing job. The workflow file always comes from the base +# branch (main), so a PR cannot modify the reviewer that runs on it. We +# never check out the fork's PR head ourselves here: anthropics/claude-code-action's +# own internal checkout logic (setupBranch() in src/github/operations/branch.ts) +# already fetches the PR branch via `git fetch origin pull//head`, which +# requires `origin` to be the BASE repo -- checking out the fork directly +# instead (as a prior version of this file did) points `origin` at the fork, +# which has no such ref, and breaks that fetch with "couldn't find remote ref +# pull//head". on: pull_request_target: # labeled: lets adding the claude-debug label (see the "Check for @@ -38,17 +40,29 @@ concurrency: jobs: claude-review: - # Trusted fork only, and skip drafts (don't spend API/CI on unfinished PRs). - # To add more trusted owners, extend the head-owner check. + # Trusted author only, and skip drafts (don't spend API/CI on unfinished PRs). + # To add more trusted authors, extend the author check. # - # SECURITY-CRITICAL: this owner check is what makes it safe to run this + # SECURITY-CRITICAL: this author check is what makes it safe to run this # pull_request_target job -- which holds base-repo secrets/token -- on # every fork PR unattended. Do not remove or loosen this condition (e.g. - # drop the owner check, or allow non-owner forks) without re-evaluating + # drop the author check, or allow untrusted authors) without re-evaluating # whether this job should keep running on arbitrary forks. + # + # Checks PR AUTHOR (user.login), not head.repo.owner.login: the latter + # only distinguishes fork ownership for fork-headed PRs. For an + # upstream-branch-headed PR (base and head both in this repo -- what `gh + # stack` requires, or any `gh pr create` without forking), head.repo is + # always this repo itself, so head.repo.owner.login is always the org, + # never the actual author -- that silently skipped review on legitimately + # trusted PRs (confirmed via the Checks API reporting `claude-review` as + # "skipped" on a trusted upstream-branch PR stack). user.login can't be + # spoofed by a third party any more than head repo owner can, and it's + # the more direct question for this gate's actual purpose: trusting the + # PERSON asking for a review, not the repository their branch lives in. if: >- github.event.pull_request.draft == false && - github.event.pull_request.head.repo.owner.login == 'jnasbyupgrade' && + github.event.pull_request.user.login == 'jnasbyupgrade' && (github.event.action != 'labeled' || github.event.label.name == 'claude-debug') runs-on: ubuntu-latest timeout-minutes: 60 @@ -61,7 +75,7 @@ jobs: # grants cancelling/deleting workflow runs and managing artifacts, which # this job doesn't use). Without it, claude-code-review's own setup step # can never write a cache, only ever miss. Granted here on top of the - # existing trusted-fork-owner gate below, not instead of it. + # existing trusted-author gate below, not instead of it. actions: write steps: # DEBUG MODE: add the "claude-debug" label to a PR to (a) skip the cost From 3b808f8e0ce1d1bf02eed5740546560101bb511c Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Sun, 9 Aug 2026 14:36:02 -0500 Subject: [PATCH 5/5] Bump remaining Actions to v7; add all-checks-passed gate to ci.yml Folds in #4 and #7 (both closed in favor of this PR): - #4's actions/checkout@v6 -> v7 bump on claude.yml and ci.yml's `test` job. Its bump to claude-code-review.yml's checkout step is superseded by this PR's earlier commit, which already rewrote that step to use v7 as part of removing the broken fork-checkout override entirely. - #7's all-checks-passed gate job in ci.yml (needs: [test], if: always(), fails on failure/cancelled -- a stable required-status-check name that survives future job renames) and dropping paths-ignore: '**.md' from both triggers, since a doc-only push/PR would otherwise never run this workflow at all and a required all-checks-passed check would sit stuck Pending forever instead of passing. Its new job's own checkout step is bumped to v7 too, for consistency with the rest of this commit. Branch protection requiring all-checks-passed is still a manual follow-up after this merges -- that's a live repo setting, not something a merge can apply. See the PR description's reminder. --- .github/workflows/ci.yml | 48 ++++++++++++++++++++++++++++++++---- .github/workflows/claude.yml | 2 +- 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 50106c7..0a32119 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,19 +1,57 @@ name: CI +# Deliberately no `paths-ignore: '**.md'` here (a prior version had one on +# both triggers): with it, a doc-only push/PR never runs this workflow at +# all, so a required "all-checks-passed" status check for such a PR would +# never report and would sit stuck Pending in branch protection forever, +# instead of passing. `test` is a single cheap Perl job with no matrix to +# skip for cost reasons, so there's no tradeoff in just always running it. on: push: branches: - main - paths-ignore: - - '**.md' pull_request: - paths-ignore: - - '**.md' jobs: test: name: Perl tests runs-on: ubuntu-latest steps: - name: Check out the repo - uses: actions/checkout@v6 + uses: actions/checkout@v7 - name: Run test suite run: make test + + # A single stable check name for use as a required status check in branch + # protection rules, so a future job rename/addition doesn't require a + # matching branch-protection update. Passes if all other jobs passed or + # were skipped, fails if any failed or were cancelled. + all-checks-passed: + needs: [test] + if: always() + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - name: Verify all jobs are listed in needs + # Ensures this job won't silently ignore a newly-added job that was + # omitted from the needs list above. + run: | + DEFINED=$(python3 -c " + import yaml + with open('.github/workflows/ci.yml') as f: + w = yaml.safe_load(f) + print('\n'.join(sorted(j for j in w['jobs'] if j != 'all-checks-passed'))) + ") + NEEDED=$(echo '${{ toJson(needs) }}' | python3 -c " + import json, sys + print('\n'.join(sorted(json.load(sys.stdin)))) + ") + if [ "$DEFINED" != "$NEEDED" ]; then + echo "Some jobs are missing from all-checks-passed needs:" + diff <(echo "$DEFINED") <(echo "$NEEDED") + exit 1 + fi + - name: Check all jobs passed or were skipped + run: | + if [[ "${{ contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') }}" == "true" ]]; then + echo "One or more jobs failed or were cancelled" + exit 1 + fi diff --git a/.github/workflows/claude.yml b/.github/workflows/claude.yml index 9a08bbb..dc559d1 100644 --- a/.github/workflows/claude.yml +++ b/.github/workflows/claude.yml @@ -36,7 +36,7 @@ jobs: - name: Checkout repository # Intentionally tracks the major-version tag (not a pinned SHA) so # upstream fixes are picked up automatically. - uses: actions/checkout@v6 + uses: actions/checkout@v7 with: fetch-depth: 1 persist-credentials: false