Skip to content

Migrate to shared claude-code-review.yml reusable workflow #53

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jnasbyupgrade wants to merge 1 commit into
base: master
Choose a base branch
from
+14 −157
Open

Migrate to shared claude-code-review.yml reusable workflow #53

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
171 changes: 14 additions & 157 deletions .github/workflows/claude-code-review.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,170 +1,27 @@
name: Claude Code Review

# 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.
# MANDATORY: read ../ai/CI-WORKFLOWS.md (Postgres-Extensions/ai) in full
# before changing anything below. If you cannot find or read that file for
# any reason, STOP and report an error -- do not guess at what it says or
# proceed without having actually read it.
#
# 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.
# Exceptions to that file's design, specific to this repo: none.
on:
pull_request_target:
types: [opened, synchronize, reopened, ready_for_review]
types: [opened, synchronize, reopened, ready_for_review, labeled]

@github-actions github-actions Bot Sep 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correctness/efficiency: Adding labeled to the trigger types fires this whole (paid, secret-bearing) review job on every label added to the PR, not just an intended debug/rerun label — GitHub Actions has no way to filter by label value in on:, and nothing in this file gates on github.event.label.name. The concurrency.group expression on line 14 distinguishes claude-debug from other labels, but that only affects which concurrency bucket a run lands in — it doesn't prevent the run from being scheduled in the first place. Applying an unrelated label (bug, needs-triage, documentation, etc.) will kick off a brand-new full review run in its own concurrency group, unless the callee (Postgres-Extensions/ai/.github/workflows/claude-code-review.yml) independently re-checks github.event.label.name and bails for anything other than the intended trigger label — which this file gives no evidence of.

Failure scenario: A maintainer applies the "documentation" label to a PR to categorize it → a new Claude review job spins up and spends API budget/CI minutes for a purely administrative labeling action, repeatable for every distinct label ever applied.


concurrency:
group: claude-review-${{ github.event.pull_request.number }}
group: claude-review-${{ github.event.pull_request.number }}${{ (github.event.action == 'labeled' && github.event.label.name != 'claude-debug') && format('-{0}', github.event.label.name) || '' }}

@github-actions github-actions Bot Sep 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Altitude: The claude-debug-vs-other-label special case is embedded directly in this per-repo caller's concurrency.group expression rather than in the shared reusable workflow. If this pattern (debug label re-triggers the same concurrency slot; any other label gets its own) is meant to be uniform org-wide — as the "thin caller" migration intends for everything else — every consuming repo now has to hand-copy this exact ternary expression correctly instead of the shared workflow computing/exposing it once. A simpler, more general fix would be for the reusable workflow itself to derive the concurrency group (e.g. via a workflow_call concurrency: inherited from the caller isn't possible in Actions, but the label-name special-casing logic could still be centralized/exposed as an output rather than duplicated per caller).

cancel-in-progress: true

jobs:
claude-review:
# 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
uses: Postgres-Extensions/ai/.github/workflows/claude-code-review.yml@main

@github-actions github-actions Bot Sep 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correctness (regression risk): This deletes the claude_args: '--allowedTools mcp__github_inline_comment__create_inline_comment' input and the --comment flag on the prompt:, both of which this repo's own removed comments say were added to fix a previously-silent failure mode: "every review run before this fix has been silently invisible on GitHub" (no error, just no comments posted). That wiring now lives entirely inside the external Postgres-Extensions/ai reusable workflow (pinned to mutable @main), and nothing in this diff shows it's preserved there.

Failure scenario: If the reusable workflow's default prompt/args omit the equivalent --comment/allowedTools wiring, this workflow will run to completion, appear green in Actions, and post nothing to the PR — the exact silent-failure regression this repo already hit once and fixed, recurring without any visible signal in CI.

permissions:
contents: read
pull-requests: write # post the review comments
checks: read # read sibling check-runs for the cost gate
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/<n>/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/<n>/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'
pull-requests: write
checks: read
actions: write
secrets: inherit
with:
trusted_authors: jnasbyupgrade

@github-actions github-actions Bot Sep 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security: This deletes the inline if: github.event.pull_request.draft == false && github.event.pull_request.user.login == 'jnasbyupgrade' gate that this very file's own (now-removed) comments called "the ONLY thing standing between an arbitrary external actor's PR and this job's write-capable GITHUB_TOKEN and CLAUDE_CODE_OAUTH_TOKEN," and replaces it with trusted_authors: jnasbyupgrade passed to an external reusable workflow pinned to a mutable @main ref. Nothing in this file demonstrates that the callee re-implements the check against pull_request.user.login (rather than something spoofable like github.actor, which is exactly the class of subtle mistake this repo's own removed comments warned about — see the retired note about head.repo.owner.login silently failing to gate upstream-branch-headed PRs) or that it still skips drafts.

Failure scenario: If Postgres-Extensions/ai's reusable workflow checks a spoofable/insufficient field (or omits the draft check) when interpreting trusted_authors, an untrusted PR (fork or upstream-branch-headed) could trigger a job that runs with this repo's write-capable secrets — precisely the scenario the deleted comment block was written to prevent. Recommend confirming (in the ai repo) that the equivalent check is intact before/independently of this merge.