Repository navigation
Fire the Claude code-review routine when a pull request is ready - #1273
Conversation
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.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly identifies the main change: triggering the Claude code-review routine when a pull request is ready for review. It is concise and related to the workflow changes. |
| Docstring Coverage | |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Description Check | |
Check skipped - CodeRabbit’s high-level summary is enabled. |
Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2 files. (1 skipped: 1 unsupported.)
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
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.
cipherstash-agent
left a comment
cipherstash-agent
left a comment
There was a problem hiding this comment.
Automated review attestation
Source: Claude Code routine, /code-review medium followed by /cross-check-review-findings, on head ce01c85.
Result: no findings. Nothing critical or blocking.
Scope: .github/workflows/code-review.yml, its test, and the fork-guard allowlist entry.
Checked, and found sound:
- Token handling:
permissions: {}. The routine token reaches curl only on stdin (-H @-), never on the command line. - Injection: PR fields go through
envandjq --arg, with no${{ }}in the script. The trigger id is regex-validated before it goes into the URL. - Triggers and guards: no
synchronize, so there is one review per event. The job skips drafts, bots and forks. The allowlist entry inworkflow-dispatch-job-conditions.test.mjsmatches. - No retry: a timed-out request can't start a second session.
Not verified: I did not run the vitest suites, because dependencies aren't installed in this environment. CI should confirm them.
Non-blocking note: there is no concurrency group, so a quick reopen followed by ready-for-review could start two review sessions. Acceptable given the documented manual re-run model.
Generated by Claude Code
cipherstash-agent
left a comment
cipherstash-agent
left a comment
There was a problem hiding this comment.
Automated review attestation: /code-review medium followed by /cross-check-review-findings, run against head ce01c85. Source: Claude Code scheduled review routine.
Critical / blocking: none.
Findings (2 survived cross-check, both non-blocking):
- Medium: the job has no enable gate. It fails every PR until
CLAUDE_ROUTINE_TOKENandCLAUDE_ROUTINE_TRIGGER_IDare set (inline, line 31). - Low: no
concurrencygroup, so draft/ready toggles can start duplicate review sessions (inline, line 21).
Checked and found fine:
- Secrets reach the script only through
env, never expanded into it. - Token goes on stdin, not the command line.
- The
jq-built body is injection-safe. - The trigger id is validated before it goes into the URL.
- Fork and bot guards are in place.
- Job permissions are
{}.
Dropped as intentional or documented in the PR: no synchronize trigger, no retry, and duplicate review when claude-review.yml is also enabled.
I did not run the tests or the workflow.
Generated by Claude Code
cipherstash-bot
left a comment
cipherstash-bot
left a comment
There was a problem hiding this comment.
Recommendation: 🟢 merge as it is
Nothing in this review must change before merge. The three findings are test gaps, and a follow-up pull request can fix them.
This review does not cover two points that the pull request discussion already raises: a missing enable condition and a missing concurrency group. This review does not decide if those two points must be fixed before merge. Before merge, read the discussion to learn what each point means and to decide if it must be fixed.
The three findings are test gaps. In each case, the workflow behaves correctly today:
- The test checks the text of the job's
if:condition, but it does not evaluate it. - No test runs the step with a failed
curlrequest. - No trigger id test case checks the
^anchor of the regex.
I checked each finding directly:
- I ran
runsWhenon an editedif:condition. - I ran the step's script under bash with a
curlstub that exits 22. - I ran bash on the regex with each anchor removed.
I did not run the vitest suites, because the workspace dependencies are not installed.
How this review was made
| Agent | Model | Review type | Result |
|---|---|---|---|
| claude | claude-opus-5-5 | test-gap | 3 found, 3 posted |
| codex | gpt-5.6-terra | test-gap | 1 found, 1 posted |
Synthesis: claude-opus-5-5 merged the findings, removed duplicates and dropped findings it could not confirm in the code. 1 posted finding(s) were raised by two or more models.
Plain language: claude-opus-5-5 read every comment as a new reader would. No comment had a problem that stopped the reader acting, so none was rewritten. It also rewrote the review body.
Stack: not part of a stack.
Context loaded: the description, 0 linked issue(s) and 6 discussion entries.
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016TP99TH97eaf1rX12JSRae
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016TP99TH97eaf1rX12JSRae
5 participants
Requested by Toby
Before: the only automated review in this repo is
claude-review.yml, which runs claude-code-action in CI and is off unlessCLAUDE_REVIEW_ENABLEDis set.After: when a pull request is opened, reopened or marked ready for review, a new
Code reviewworkflow fires a Claude code-review routine with the PR's URL and number, and the routine's session reviews it. Drafts, bots' PRs (Dependabot, release-plz) and forks are skipped. There is nosynchronizetrigger, so mark a PR draft and then ready again to get a fresh review.How: one curl POST to the routine's
/fireendpoint. jq builds the body, the bearer token goes in on stdin rather than on the command line, and the trigger id is checked against^trig_[A-Za-z0-9]+$before it goes into the URL. It doesn't retry, because a request that timed out may already have started a session. The workflow is ported from tobyhede/zod'scode-review.yml, with the trigger id moved into a repository variable.Setup needed before this works:
CLAUDE_ROUTINE_TOKEN, the routine's bearer token.CLAUDE_ROUTINE_TRIGGER_ID, the routine'strig_…id.Until those exist, the job fails with an error saying which one is missing. If
claude-review.ymlis turned on as well, each PR will be reviewed twice.Tests:
scripts/__tests__/code-review-workflow.test.mjspins the triggers, the empty permissions and the secret and variable the workflow reads. It also runs the step's script against a stubcurland checks that the token is passed only on stdin, that quotes in PR fields stay inside the JSON body, and that a missing token or a malformed trigger id is refused beforecurlruns. The job'sif:now uses the canonical fork-guard clause, and the job is listed inworkflow-dispatch-job-conditions.test.mjs, which caught the old spelling.The workflow tests and
claude-review-workflow.test.mjspass locally (199 tests), and so do actionlint, zizmor and biome. I didn't run the wholetest:scriptssuite:bench-index-expressions.test.mjsneeds the workspace packages, which I didn't install.Closes #1292.
Summary by CodeRabbit
Generated by Claude Code