From 07daee427ae8d4642b7cb5d956f12994dd9c4429 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Thu, 17 Sep 2026 16:19:26 -0500 Subject: [PATCH] Revert "Migrate to shared claude-code-review.yml reusable workflow" This reverts commit 6b2257e4f3325d8d5bd75caed50be1541f1d77eb. --- .github/workflows/claude-code-review.yml | 185 +++++++++++++++++++---- 1 file changed, 156 insertions(+), 29 deletions(-) diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index da0029f..60d8f98 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -1,43 +1,170 @@ name: Claude Code Review -# Thin caller. All logic lives in Postgres-Extensions/ai; read that file for -# the SECURITY rationale behind pull_request_target + the trusted-author gate. -# Everything below is the minimum GitHub requires to live in THIS repo: -# - the trigger (a called workflow cannot declare its own) -# - run-level concurrency (only a workflow-level `concurrency:` can cancel -# the whole caller run outright; `jobs..concurrency` on the caller job -# itself can't, and the per-label cancellation logic below needs exactly -# that) -# - the GITHUB_TOKEN ceiling (a called workflow can only narrow it, never widen) -# Do not add logic here. If this repo needs different behavior, change ai/ so -# every repo gets it. +# Runs on PRs INTO this repo. We use pull_request_target (not pull_request) so +# that PRs from a fork can access CLAUDE_CODE_OAUTH_TOKEN — GitHub withholds +# secrets from `pull_request` runs triggered by forks, which is why the plain +# `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 authored by jnasbyupgrade only +# — github.event.pull_request.user.login is the PR's original author and +# can't be spoofed by PR content, so this check holds regardless of whether +# the PR head lives in this repo or an external fork. The workflow file +# always comes from the base branch (master), so a PR cannot modify the +# reviewer that runs on it. This workflow never checks out the PR's own ref +# into the workspace (see the checkout step below) -- claude-code-action +# fetches and reads the PR's content itself, safely, and never builds or +# executes it. on: pull_request_target: - # `labeled` lets adding the claude-debug label start a run on its own, with - # no push needed. Scoped in ai/'s job `if:` so only that label proceeds. - types: [opened, synchronize, reopened, ready_for_review, labeled] + types: [opened, synchronize, reopened, ready_for_review] concurrency: - # A non-debug `labeled` event gets its own per-label group so it can never - # cancel an in-progress real review: cancellation resolves when a run is - # admitted, before any `if:` is evaluated, so an `if:` can only no-op itself, - # not un-cancel what it displaced. labeled+claude-debug deliberately keeps the - # plain group -- it is meant to supersede a running review. - # 'claude-debug' is spelled out because `inputs` is not readable here; it must - # match ai/'s debug_label default. - group: claude-review-${{ github.event.pull_request.number }}${{ (github.event.action == 'labeled' && github.event.label.name != 'claude-debug') && format('-{0}', github.event.label.name) || '' }} + group: claude-review-${{ github.event.pull_request.number }} cancel-in-progress: true jobs: claude-review: - uses: Postgres-Extensions/ai/.github/workflows/claude-code-review.yml@main + # jnasbyupgrade's own PRs only, and skip drafts (don't spend API/CI on + # unfinished PRs). + # + # !!! SECURITY-CRITICAL -- DO NOT REMOVE OR WEAKEN THE user.login CHECK + # BELOW !!! It is the ONLY thing standing between an arbitrary external + # actor's PR and this job's write-capable GITHUB_TOKEN and + # CLAUDE_CODE_OAUTH_TOKEN. Drop or loosen this check and any PR can + # trigger a job that runs with this repo's secrets. NOTE: this used to + # check head.repo.owner.login (the owner of the fork the PR head lives + # in), but that only distinguishes forks -- for an upstream-branch-headed + # PR (base and head both in this repo, e.g. from `gh stack` or a plain + # `gh pr create` without a fork) it's always this repo's own org, + # regardless of who actually opened the PR, so it silently skipped review + # on every such PR. github.event.pull_request.user.login is the PR's + # actual author and can't be spoofed by PR content either, and it + # correctly covers both fork-headed and upstream-branch-headed PRs. To + # trust an additional author, EXTEND this condition explicitly (e.g. + # `|| ... == 'other-trusted-account'`) -- never replace it with something + # broader (a wildcard, etc.). + if: >- + github.event.pull_request.draft == false && + github.event.pull_request.user.login == 'jnasbyupgrade' + runs-on: ubuntu-latest + timeout-minutes: 60 permissions: contents: read pull-requests: write # post the review comments checks: read # read sibling check-runs for the cost gate - # actions: write is the only scope that permits an Actions cache write - # (no narrower one exists). Don't "tighten" this to read. - actions: write - secrets: inherit - with: - trusted_authors: jnasbyupgrade + actions: write # lets a step save its Actions cache -- there is no + # narrower cache-write-only scope; without this the + # job still succeeds but silently fails to cache, + # logging "Cache reservation failed: cache write + # denied: token has no writable scopes" every run + steps: + # 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 + # reviewing a PR that is already known-broken. Uniform across all repos: + # it discovers sibling checks dynamically (no per-repo workflow names). + # - decision=run : all sibling checks completed with a good conclusion, + # OR no sibling checks exist after a short grace window + # (nothing to gate on), OR the poll timed out is treated + # as skip (see below). + # - decision=skip : at least one sibling check failed/cancelled/etc, or + # we timed out waiting for still-pending checks. + # We exclude this workflow's own check-run (job name `claude-review`) so the + # gate never waits on or fails because of itself. + - name: Wait for CI; skip the paid review if any check failed + id: gate + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + SHA: ${{ github.event.pull_request.head.sha }} + run: | + decision=skip + for i in $(seq 1 72); do # ~24 min max + json=$(gh api "repos/$REPO/commits/$SHA/check-runs" --paginate \ + --jq '[.check_runs[] | select(.name != "claude-review")]' 2>/dev/null) || json='' + [ -z "$json" ] && { sleep 20; continue; } + total=$(jq 'length' <<<"$json") + if [ "$total" -eq 0 ]; then + [ "$i" -ge 9 ] && { decision=run; break; } # ~3 min grace: nothing to gate on + sleep 20; continue + fi + pending=$(jq '[.[]|select(.status!="completed")]|length' <<<"$json") + if [ "$pending" -eq 0 ]; then + bad=$(jq '[.[]|select((.conclusion//"")|test("^(failure|cancelled|timed_out|action_required|stale)$"))]|length' <<<"$json") + [ "$bad" -eq 0 ] && decision=run || decision=skip + break + fi + sleep 20 + done + echo "decision=$decision" >> "$GITHUB_OUTPUT" + echo "gate decision: $decision" + + - name: Check out base branch + if: steps.gate.outputs.decision == 'run' + # Deliberately NO ref:/repository: override -- this checks out this + # repo's own base branch (master), not the PR's fork/ref. Checking + # out an untrusted PR ref into the workspace root before this action + # is exactly the anti-pattern anthropics/claude-code-action's own + # docs/security.md warns against; its "preferred" pattern is a plain + # checkout of the base ref, nothing more. claude-code-action fetches + # and reviews the PR's actual content itself, from ITS OWN internal + # logic (see its src/github/operations/branch.ts): for a fork PR it + # fetches origin's refs/pull//head -- a ref GitHub maintains on + # THIS repo for any PR, fork or not, so it never needs direct access + # to the fork's own remote at all. That's why this step must leave + # `origin` pointing at this repo (the default) rather than being + # redirected to the fork: an earlier version of this step did that, + # which broke the action's own internal fetch ("couldn't find remote + # ref pull//head") since that ref doesn't exist on the fork. + # Intentionally tracks the major-version tag (not a pinned SHA) so + # upstream fixes are picked up automatically. + uses: actions/checkout@v7 + with: + # This job's permissions include pull-requests: write, a real + # write-capable credential -- nothing here legitimately runs `git + # push` (review comments post via the API/claude-code-action, not + # git), so there's no reason to leave that credential sitting in + # .git/config for the rest of the job to misuse if anything later + # goes wrong. + persist-credentials: false + + - name: Run Claude Code Review + if: steps.gate.outputs.decision == 'run' + uses: anthropics/claude-code-action@v1 + with: + claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} + # 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. (Pattern + # modeled on Postgres-Extensions/cat_tools PR #69.) + track_progress: true + # A bare `prompt:` (no `@claude` mention) runs the action in "agent + # mode", which decides which MCP servers to start by scanning an + # --allowedTools flag inside claude_args -- it does NOT consult the + # invoked plugin's own allowed-tools frontmatter. Without this, the + # github_inline_comment MCP server never starts, so the tool the + # code-review plugin needs for real per-line inline comments doesn't + # exist in this session at all -- not blocked, absent. The plugin + # silently falls back to one consolidated PR comment instead, with + # no error/warning. (Found in Postgres-Extensions/cat_tools PR #62.) + claude_args: '--allowedTools mcp__github_inline_comment__create_inline_comment' + # 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' + plugins: 'code-review@claude-code-plugins' + # --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'