From b35855308aff0c3ac791bbf3a4e6fa5a56d918a9 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Mon, 19 Oct 2026 11:14:36 +1100 Subject: [PATCH 1/4] Fire the code-review routine when a pull request is ready Ported from tobyhede/zod: on opened, reopened and ready_for_review (never a draft, a bot's or a fork's pull request), POST the pull request's URL and number to the Claude code-review routine. The trigger id is the repository variable CLAUDE_ROUTINE_TRIGGER_ID and the bearer token the secret CLAUDE_ROUTINE_TOKEN. --- .github/workflows/code-review.yml | 57 +++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 .github/workflows/code-review.yml diff --git a/.github/workflows/code-review.yml b/.github/workflows/code-review.yml new file mode 100644 index 000000000..042629427 --- /dev/null +++ b/.github/workflows/code-review.yml @@ -0,0 +1,57 @@ +# Asks Claude for a code review of a pull request: fires the code-review routine, whose session reviews the pull +# request this run names (its URL and number, in the turn appended to the session). It runs when a pull request is +# opened, reopened or marked ready for review, and never on a draft. Bots' pull requests (Dependabot's, release-plz's) +# and those from forks are skipped: forks are not given the repository's secrets. A pull request is reviewed once per +# event, not on every push (no `synchronize`): mark it draft and ready again for a fresh review. +# +# Ported from tobyhede/zod's code-review.yml. claude-review.yml is the other review, run in CI by claude-code-action +# behind CLAUDE_REVIEW_ENABLED; with both on, a pull request is reviewed twice. +# +# Secret: CLAUDE_ROUTINE_TOKEN, the routine's bearer token. +# Variable: CLAUDE_ROUTINE_TRIGGER_ID, the routine's trigger id (`trig_…`). +name: Code review + +on: + pull_request: + types: [opened, reopened, ready_for_review] + +permissions: {} + +jobs: + request: + if: >- + ${{ !github.event.pull_request.draft + && github.event.pull_request.user.type != 'Bot' + && github.event.pull_request.head.repo.full_name == github.repository }} + timeout-minutes: 5 + runs-on: ubuntu-latest + steps: + - name: Fire the code-review routine + env: + CLAUDE_ROUTINE_TOKEN: ${{ secrets.CLAUDE_ROUTINE_TOKEN }} + TRIGGER_ID: ${{ vars.CLAUDE_ROUTINE_TRIGGER_ID }} + PR_URL: ${{ github.event.pull_request.html_url }} + PR_NUMBER: ${{ github.event.pull_request.number }} + REPO: ${{ github.repository }} + run: | + if [ -z "$CLAUDE_ROUTINE_TOKEN" ]; then + echo "::error::CLAUDE_ROUTINE_TOKEN is empty: add it as a repository secret under Settings > Secrets and variables > Actions" + exit 1 + fi + # The id goes into the URL, so anything but a trigger id is refused. + if ! [[ "$TRIGGER_ID" =~ ^trig_[A-Za-z0-9]+$ ]]; then + echo "::error::CLAUDE_ROUTINE_TRIGGER_ID is not a trigger id (trig_…): set it as a repository variable under Settings > Secrets and variables > Actions > Variables" + exit 1 + fi + # jq writes the body, so nothing in the pull request's fields can break the JSON. + body=$(jq -n --arg url "$PR_URL" --arg number "$PR_NUMBER" --arg repo "$REPO" \ + '{text: "Review pull request #\($number) in \($repo): \($url)"}') + # No retry: a request that timed out may still have started a session, and a second would review twice. A + # failed run is re-run by hand. The token goes in on standard input, never on curl's command line. + curl --fail-with-body --silent --show-error --connect-timeout 10 --max-time 30 -X POST \ + "https://api.anthropic.com/v1/claude_code/routines/$TRIGGER_ID/fire" \ + -H @- \ + -H "anthropic-version: 2023-06-01" \ + -H "anthropic-beta: experimental-cc-routine-2026-04-01" \ + -H "Content-Type: application/json" \ + -d "$body" <<<"Authorization: Bearer $CLAUDE_ROUTINE_TOKEN" From ce01c85f112276ca23c3d7ed35a8633884da54b1 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Mon, 19 Oct 2026 11:34:01 +1100 Subject: [PATCH 2/4] Test the code-review routine workflow code-review-workflow.test.mjs pins the triggers (no synchronize), the empty permissions, the secret and variable it reads, and runs the step's script against a stub curl: the token only on stdin, the body built by jq, and a missing token or a malformed trigger id refused before curl. The fork guard now takes the canonical spelling and the job is listed in workflow-dispatch-job-conditions.test.mjs, which caught the old one. --- .github/workflows/code-review.yml | 6 +- .../__tests__/code-review-workflow.test.mjs | 174 ++++++++++++++++++ .../workflow-dispatch-job-conditions.test.mjs | 2 + 3 files changed, 180 insertions(+), 2 deletions(-) create mode 100644 scripts/__tests__/code-review-workflow.test.mjs diff --git a/.github/workflows/code-review.yml b/.github/workflows/code-review.yml index 042629427..a750da453 100644 --- a/.github/workflows/code-review.yml +++ b/.github/workflows/code-review.yml @@ -19,10 +19,12 @@ permissions: {} jobs: request: + # The fork guard as workflow-dispatch-job-conditions.test.mjs requires it spelled. if: >- - ${{ !github.event.pull_request.draft + (github.event_name != 'pull_request' + || github.event.pull_request.head.repo.full_name == github.repository) + && github.event.pull_request.draft != true && github.event.pull_request.user.type != 'Bot' - && github.event.pull_request.head.repo.full_name == github.repository }} timeout-minutes: 5 runs-on: ubuntu-latest steps: diff --git a/scripts/__tests__/code-review-workflow.test.mjs b/scripts/__tests__/code-review-workflow.test.mjs new file mode 100644 index 000000000..2de2b84ed --- /dev/null +++ b/scripts/__tests__/code-review-workflow.test.mjs @@ -0,0 +1,174 @@ +import { spawnSync } from 'node:child_process' +import { + chmodSync, + existsSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { readWorkflow } from './lib/workflows.mjs' + +const WORKFLOW = '.github/workflows/code-review.yml' +const gha = (expression) => `\${{ ${expression} }}` + +const workflow = readWorkflow(WORKFLOW) +const triggers = workflow.on ?? workflow[true] +const request = workflow.jobs.request +const fire = request.steps.find( + (step) => step.name === 'Fire the code-review routine', +) + +describe('Code-review routine trigger', () => { + it('contains only the request job, with a single step', () => { + expect(Object.keys(workflow.jobs)).toEqual(['request']) + expect(request.steps).toEqual([fire]) + }) + + it('fires once per lifecycle event, never on every push', () => { + // No `synchronize`: each push would start another review session. Mark the + // pull request draft and ready again for a fresh review. + expect(Object.keys(triggers)).toEqual(['pull_request']) + expect(triggers.pull_request.types).toEqual([ + 'opened', + 'reopened', + 'ready_for_review', + ]) + }) + + it('admits only non-draft, non-bot pull requests from this repository', () => { + // Forks are not given the routine token; bots (Dependabot, release-plz) + // are not reviewed. workflow-dispatch-job-conditions.test.mjs holds the + // fork guard's spelling and meaning. + const condition = String(request.if).replace(/\s+/g, ' ') + expect(condition).toContain('github.event.pull_request.draft != true') + expect(condition).toContain("github.event.pull_request.user.type != 'Bot'") + expect(condition).toContain( + 'github.event.pull_request.head.repo.full_name == github.repository', + ) + }) + + it('grants the job token nothing and stays bounded', () => { + // The request is authenticated by the routine token alone. + expect(workflow.permissions).toEqual({}) + expect(request.permissions).toBeUndefined() + expect(request['runs-on']).toBe('ubuntu-latest') + expect(request['timeout-minutes']).toBe(5) + }) + + it('reads the routine token from a secret and the trigger id from a variable', () => { + expect(fire.env).toEqual({ + CLAUDE_ROUTINE_TOKEN: gha('secrets.CLAUDE_ROUTINE_TOKEN'), + TRIGGER_ID: gha('vars.CLAUDE_ROUTINE_TRIGGER_ID'), + PR_URL: gha('github.event.pull_request.html_url'), + PR_NUMBER: gha('github.event.pull_request.number'), + REPO: gha('github.repository'), + }) + // Pull-request fields reach the script only through env, never expanded + // into it. + expect(fire.run).not.toMatch(/\$\{\{/) + }) +}) + +// The step's script, run as GitHub runs it (`bash -eo pipefail`), with `curl` +// replaced by a stub that records its arguments and standard input. +describe('Code-review routine trigger script', () => { + let dir + const baseEnv = () => ({ + PATH: `${dir}:${process.env.PATH}`, + CURL_LOG: join(dir, 'curl.log'), + CLAUDE_ROUTINE_TOKEN: 'sk-routine-token', + TRIGGER_ID: 'trig_01ABCdef', + PR_URL: 'https://github.com/cipherstash/stack/pull/7', + PR_NUMBER: '7', + REPO: 'cipherstash/stack', + }) + const run = (env) => + spawnSync( + 'bash', + ['--noprofile', '--norc', '-eo', 'pipefail', '-c', fire.run], + { + cwd: dir, + encoding: 'utf8', + env, + }, + ) + const curlCall = () => { + const [args, stdin] = readFileSync(join(dir, 'curl.log'), 'utf8').split( + '\0', + ) + return { args: JSON.parse(args), stdin } + } + + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'code-review-')) + const stub = join(dir, 'curl') + writeFileSync( + stub, + `#!${process.execPath} +const { readFileSync, writeFileSync } = require('node:fs') +writeFileSync(process.env.CURL_LOG, JSON.stringify(process.argv.slice(2)) + '\\0' + readFileSync(0, 'utf8')) +`, + ) + chmodSync(stub, 0o755) + }) + + afterEach(() => { + rmSync(dir, { recursive: true, force: true }) + }) + + it('posts the pull request to the trigger, the token only on standard input', () => { + const result = run(baseEnv()) + expect(result.status, result.stderr).toBe(0) + const { args, stdin } = curlCall() + + expect(args).toContain( + 'https://api.anthropic.com/v1/claude_code/routines/trig_01ABCdef/fire', + ) + expect(args.join(' ')).not.toContain('sk-routine-token') + expect(stdin.trim()).toBe('Authorization: Bearer sk-routine-token') + expect(args[args.indexOf('-H') + 1]).toBe('@-') + + const body = JSON.parse(args[args.indexOf('-d') + 1]) + expect(body).toEqual({ + text: 'Review pull request #7 in cipherstash/stack: https://github.com/cipherstash/stack/pull/7', + }) + }) + + it('does not retry, so a timed-out request never reviews twice', () => { + expect(fire.run).not.toMatch(/--retry/) + }) + + it('keeps quotes in pull-request fields inside the JSON body', () => { + const result = run({ ...baseEnv(), PR_URL: 'https://x/"},"text":"evil' }) + expect(result.status, result.stderr).toBe(0) + const { args } = curlCall() + const body = JSON.parse(args[args.indexOf('-d') + 1]) + expect(Object.keys(body)).toEqual(['text']) + expect(body.text).toContain('"},"text":"evil') + }) + + it('fails without calling curl when the token is missing', () => { + const result = run({ ...baseEnv(), CLAUDE_ROUTINE_TOKEN: '' }) + expect(result.status).toBe(1) + expect(result.stdout).toContain('::error::CLAUDE_ROUTINE_TOKEN is empty') + expect(existsSync(join(dir, 'curl.log'))).toBe(false) + }) + + it.each([ + ['', 'unset'], + ['trig_01../../other', 'a path'], + ['trig_01?x=1', 'a query'], + ['01ABCdef', 'no trig_ prefix'], + ])('refuses trigger id %j (%s) without calling curl', (id) => { + const result = run({ ...baseEnv(), TRIGGER_ID: id }) + expect(result.status).toBe(1) + expect(result.stdout).toContain( + '::error::CLAUDE_ROUTINE_TRIGGER_ID is not a trigger id', + ) + expect(existsSync(join(dir, 'curl.log'))).toBe(false) + }) +}) diff --git a/scripts/__tests__/workflow-dispatch-job-conditions.test.mjs b/scripts/__tests__/workflow-dispatch-job-conditions.test.mjs index 59172aa0a..3b02b86e7 100644 --- a/scripts/__tests__/workflow-dispatch-job-conditions.test.mjs +++ b/scripts/__tests__/workflow-dispatch-job-conditions.test.mjs @@ -118,6 +118,8 @@ const DISPATCH_SKIPPED_JOBS = [ const EXPECTED_FORK_GUARDED_JOBS = [ // Anthropic federation is available only to same-repository pull requests. '.github/workflows/claude-review.yml / review', + // The routine token is a repository secret, which forks are not given. + '.github/workflows/code-review.yml / request', '.github/workflows/integration-drizzle.yml / integration', '.github/workflows/integration-prisma-next.yml / integration', '.github/workflows/integration-protect-ffi.yml / integration', From c3fde53cc6b1fb5bf98dad1ec5e0ee5dddc12aaf Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 08:19:07 +0000 Subject: [PATCH 3/4] Run one code-review request per pull request at a time A reopen or a draft/ready toggle in quick succession fired one routine session per event. A concurrency group keyed on the pull request queues the second run instead, and a third replaces the queued one, so a burst fires at most twice. It does not cancel a running request, which may already have started a session. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_016TP99TH97eaf1rX12JSRae --- .github/workflows/code-review.yml | 6 ++++++ scripts/__tests__/code-review-workflow.test.mjs | 9 +++++++++ 2 files changed, 15 insertions(+) diff --git a/.github/workflows/code-review.yml b/.github/workflows/code-review.yml index a750da453..aeb9a7bf6 100644 --- a/.github/workflows/code-review.yml +++ b/.github/workflows/code-review.yml @@ -17,6 +17,12 @@ on: permissions: {} +# One run per pull request at a time. A second event waits rather than cancelling the first, whose request may already +# have started a session; a third replaces the waiting one, so a burst of toggles fires at most twice. +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number }} + cancel-in-progress: false + jobs: request: # The fork guard as workflow-dispatch-job-conditions.test.mjs requires it spelled. diff --git a/scripts/__tests__/code-review-workflow.test.mjs b/scripts/__tests__/code-review-workflow.test.mjs index 2de2b84ed..c3d8733d6 100644 --- a/scripts/__tests__/code-review-workflow.test.mjs +++ b/scripts/__tests__/code-review-workflow.test.mjs @@ -51,6 +51,15 @@ describe('Code-review routine trigger', () => { ) }) + it('runs one request per pull request at a time, never cancelling one', () => { + // Cancelling a run mid-request could leave a session started behind a + // cancelled check; a queued run is replaced by the next, bounding a burst. + expect(workflow.concurrency).toEqual({ + group: `${gha('github.workflow')}-${gha('github.event.pull_request.number')}`, + 'cancel-in-progress': false, + }) + }) + it('grants the job token nothing and stays bounded', () => { // The request is authenticated by the routine token alone. expect(workflow.permissions).toEqual({}) From 20843cf6d004462d591dc4b6d4c4c067aed9ad16 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 09:01:29 +0000 Subject: [PATCH 4/4] Keep a failed code-review request off the pull request check The review is advisory, so the step now has continue-on-error: an outage or a revoked token shows as a failed step in the run, not as a red check. The tests also cover three gaps a review found: the job's if: is evaluated with runsWhen rather than string-matched, a curl that exits 22 must fail the step, and a path before trig_ must be refused, which pins the regex's ^ anchor. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_016TP99TH97eaf1rX12JSRae --- .github/workflows/code-review.yml | 5 +- .../__tests__/code-review-workflow.test.mjs | 47 +++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/.github/workflows/code-review.yml b/.github/workflows/code-review.yml index aeb9a7bf6..34e7f2648 100644 --- a/.github/workflows/code-review.yml +++ b/.github/workflows/code-review.yml @@ -35,6 +35,9 @@ jobs: runs-on: ubuntu-latest steps: - name: Fire the code-review routine + # The review is advisory: a failed request (an outage, a revoked token) shows as a failed step in this run, not + # as a red check on the pull request. + continue-on-error: true env: CLAUDE_ROUTINE_TOKEN: ${{ secrets.CLAUDE_ROUTINE_TOKEN }} TRIGGER_ID: ${{ vars.CLAUDE_ROUTINE_TRIGGER_ID }} @@ -55,7 +58,7 @@ jobs: body=$(jq -n --arg url "$PR_URL" --arg number "$PR_NUMBER" --arg repo "$REPO" \ '{text: "Review pull request #\($number) in \($repo): \($url)"}') # No retry: a request that timed out may still have started a session, and a second would review twice. A - # failed run is re-run by hand. The token goes in on standard input, never on curl's command line. + # failed request is re-run by hand. The token goes in on standard input, never on curl's command line. curl --fail-with-body --silent --show-error --connect-timeout 10 --max-time 30 -X POST \ "https://api.anthropic.com/v1/claude_code/routines/$TRIGGER_ID/fire" \ -H @- \ diff --git a/scripts/__tests__/code-review-workflow.test.mjs b/scripts/__tests__/code-review-workflow.test.mjs index c3d8733d6..3b09436be 100644 --- a/scripts/__tests__/code-review-workflow.test.mjs +++ b/scripts/__tests__/code-review-workflow.test.mjs @@ -10,6 +10,7 @@ import { import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { runsWhen } from './lib/expressions.mjs' import { readWorkflow } from './lib/workflows.mjs' const WORKFLOW = '.github/workflows/code-review.yml' @@ -60,6 +61,42 @@ describe('Code-review routine trigger', () => { }) }) + it('skips draft, bot and fork pull requests and runs the rest', () => { + // Evaluated, not string-matched: a wrong operator between the clauses + // still contains every clause. + const REPO = 'cipherstash/stack' + const context = (pullRequest) => ({ + github: { + event_name: 'pull_request', + repository: REPO, + event: { + pull_request: { + head: { repo: { full_name: REPO } }, + draft: false, + user: { type: 'User' }, + ...pullRequest, + }, + }, + }, + }) + const condition = String(request.if) + expect(runsWhen(condition, context({}))).toBe(true) + expect(runsWhen(condition, context({ draft: true }))).toBe(false) + expect(runsWhen(condition, context({ user: { type: 'Bot' } }))).toBe(false) + expect( + runsWhen( + condition, + context({ head: { repo: { full_name: 'contributor/stack' } } }), + ), + ).toBe(false) + }) + + it('reports a failed request without failing the pull request check', () => { + // The review is advisory: an API error or outage shows in the run, not as + // a red check on the pull request. + expect(fire['continue-on-error']).toBe(true) + }) + it('grants the job token nothing and stays bounded', () => { // The request is authenticated by the routine token alone. expect(workflow.permissions).toEqual({}) @@ -120,6 +157,7 @@ describe('Code-review routine trigger script', () => { `#!${process.execPath} const { readFileSync, writeFileSync } = require('node:fs') writeFileSync(process.env.CURL_LOG, JSON.stringify(process.argv.slice(2)) + '\\0' + readFileSync(0, 'utf8')) +process.exit(Number(process.env.CURL_EXIT ?? 0)) `, ) chmodSync(stub, 0o755) @@ -151,6 +189,14 @@ writeFileSync(process.env.CURL_LOG, JSON.stringify(process.argv.slice(2)) + '\\0 expect(fire.run).not.toMatch(/--retry/) }) + it('fails the step when curl reports an HTTP error', () => { + // curl exits 22 when --fail-with-body receives a 4xx or 5xx answer: a + // revoked token or a deleted trigger must not read as a fired review. + const result = run({ ...baseEnv(), CURL_EXIT: '22' }) + expect(result.status).toBe(22) + expect(curlCall().args).toContain('--fail-with-body') + }) + it('keeps quotes in pull-request fields inside the JSON body', () => { const result = run({ ...baseEnv(), PR_URL: 'https://x/"},"text":"evil' }) expect(result.status, result.stderr).toBe(0) @@ -172,6 +218,7 @@ writeFileSync(process.env.CURL_LOG, JSON.stringify(process.argv.slice(2)) + '\\0 ['trig_01../../other', 'a path'], ['trig_01?x=1', 'a query'], ['01ABCdef', 'no trig_ prefix'], + ['../../other/trig_01ABC', 'a path before trig_'], ])('refuses trigger id %j (%s) without calling curl', (id) => { const result = run({ ...baseEnv(), TRIGGER_ID: id }) expect(result.status).toBe(1)