Repository navigation
feat(workflows): Claude lane feature set — config currency, cadence, retry, kill-switches, copy (PR-B, 2b-2i) - #280
Merged
Merged
Conversation
…osites (PR-A2) Both reusable review lanes replace their embedded freshness, outcome, and marker-comment steps with the composite trio merged in PR-A1, pinned at its merge SHA b5d54bf (self-reference pins resolve only post-merge; a local ./ ref resolves against the caller's checkout in a reusable workflow). The four generated-embed classifier files retire with the embeds, along with their ci.yml test step and their selector-conformance.yml path triggers + test step. The security lane keeps #266's fail-closed contract through the repoint: the outcome composite records a failure without failing, so a new inline "Fail closed on an in-scope non-run" step owns the required-check red under the same pull_request-only carve-out. Its expanded marker body rides the composite's body-copy input as blockquote-continuation lines (input description updated to sanction multiline use). Tests move with the behavior: the superseded-guard suite pins the freshness composite reference instead of the inline github-script; the fail-closed suite drops the executed-bash classifier cases (owned by the composite's classify.test.cjs corpus) and pins the new wiring — resolve-attempt inputs, composite outputs, and the fail-closed step's exact condition. Phase 1 closes out in PLAN.md with merged-main sanity evidence, and the 2a-addendum records this execution shape. Known cosmetic delta until 2g: the composite annotation says "not a code-quality signal" on both lanes; the 2g copy rewrite aligns the security wording. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The four classify-infra-failure files were staged for the repoint commit but a stash round-trip during the zizmor baseline comparison silently unstaged the deletions. Without this, the render tripwire test still ships and fails against workflows whose embeds are gone. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Bump claude-code-action to v1.0.183, default the lanes to claude-sonnet-5, allow the inline-comment MCP tool on both review lanes, widen exclude_comments_by_actor to both Dependabot spellings (upstream #1514) and lift it to an input on the security lane, extend skip-actors with the org's self-trigger ban, name the code-review lane in its default prompt and defer security scope to REVIEW.md's split, and correct the stale selected-repositories claim about the org secret (live visibility: all). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Daily interval; claude-code-action is excluded from the 7-day cooldown and from the github-actions group so its bumps arrive immediately as standalone PRs the lanes can adopt without unbatching. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…r-PR review cap (2c) The code-review lane now reviews on open/ready/reopen only: the canonical caller drops `synchronize` (dogfooded in the self-caller) and the job skips draft PRs. A new max-reviews-per-pr input (default 5) caps spend on long-lived PRs via a visible status comment that doubles as the counter and the human "was this reviewed" signal — chosen over the runs API (counts every advisory success, needs an uncallered `actions: read` grant) and a hidden marker comment (renders as an empty box and races). Missing or deleted comment = count 0, so deletion re-enables review; a capped run is a name-stable skip that never posts an infra-failure warning. The security lane keeps `synchronize` — its check certifies execution at the merge head. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
All three Claude lanes now run a single gated retry: attempt 1 carries continue-on-error plus a step-level timeout, a github-script gate decides whether a second attempt is safe and useful, a jittered backoff (retry-delay-seconds input, default 90s, plus 0-29s of random spread) separates the attempts, and a resolve step feeds the effective attempt's outcome and execution file to the outcome path. Terminal double-failure still flows through the marker/class path; the retry machinery never exits non-zero. The gate retries iff the execution file parses to ZERO assistant turns AND the failure is not auth-class. Zero turns is the artifact-safety condition: the action posts nothing review-shaped before its first assistant turn, so attempt 2 cannot duplicate inline comments. The auth exclusion (api_error_status 401/402/403 or the three auth-class error types, mirroring claude-lane-outcome/classify.cjs) skips the one class no retry can clear - the credential needs an operator. Mid-review contention failures are deliberately not retried; the trade-off is documented at each gate. The review and security lanes' gates also best-effort delete the attempt-1 orphan tracking comment (track_progress posts it before the first assistant turn); the e2e lane sets no track_progress and has no orphan step. Job budgets follow 2 x attempt timeout + delay + 5: review 11-minute attempts inside the 30-minute default, security 18 inside 45, e2e inputs.timeout-minutes per attempt inside the fixed 60 ceiling. The security lane's previous unconditional retry-on-failure (sleep 60) is replaced by this gate; the retry stays PR-scoped. Wiring tests assert the gate chain, the jittered backoff, the zero-turn filter and auth substrings, and the verbatim retry with-block; the review lane's guard tripwires move to 10 superseded-gates and 9 capped-gates. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adversarial-review fixes: the review-count gate now performs its comment lookup even when the cap is disabled, so the upsert step always has the prior count and comment id (a disabled or unset cap previously degraded the upsert to an insert-per-review and pinned the count at 1); paginate calls request per_page 100; the input description records the soft-cap window under concurrent different-head runs. Post-review steps across all three lanes swap always() for !cancelled(): cancellation is the concurrency group's retirement mechanism, so a cancelled run must not report an outcome, raise the security lane's fail-closed red, or mutate the newer run's PR comment state. Teardown steps (app stop, credential strip) keep always() — cleanup must survive cancellation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…erifier D1-D4) CLAUDE.md now records the org secret's live posture (visibility: all repositories, never touched from automation; sandbox-repo override for forced-failure tests) instead of instructing agents to keep a selected-repositories scope that no longer exists. The shared adoption snippet notes trigger types are per lane now that the code-review lane drops synchronize. Hand-maintained input enumerations in README lane entries are replaced with pointers to the workflow headers — the authoritative, drift-free list. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Consumer-contract documentation only; the shapes themselves ship in the sync-managed caller components. The code-review canonical caller gains a workflow-level per-PR concurrency group (run_id fallback, cancel-in-progress: true) plus a note that the managed component adds a separate job-level `queue: max` group `claude-review-<repo>` serializing reviews repo-wide. The security canonical caller documents cancel-in-progress: false as the deliberate value — a cancelled REQUIRED check is not a skip and does not read success (claude-code-plugins' live value on the one repo requiring the check) — and no queue group until overflow smoke-testing proves a cancelled overflow arrival cannot wedge the required check. Both headers state the values are per-lane and deliberate (devils-advocate F5), so callers do not normalize them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Each reusable's lane job composes two Actions-variable clauses into its job `if`: the master `CLAUDE_LANES_DISABLED` plus a per-lane switch (`CLAUDE_REVIEW_DISABLED`, `CLAUDE_SECURITY_REVIEW_DISABLED`, `CLAUDE_E2E_VERIFY_DISABLED`). An absent variable evaluates to '' — not 'true' — so lanes stay enabled by default, and a repo-level variable overrides an org-level one (platform-verified by the 0c probe), giving one repo an opt-out under an org-wide value. The e2e lane gains its first job-level `if` to carry them. On the security lane the clauses gate the `security-review` job only — never `changes` — so a kill-switched run is a name-stable SKIP a required-check ruleset reads as success; a new test pins that split. 2f-addendum (docs route): both review-lane headers now record that REVIEW.md's code-review-lane security exclusion keys on the security WORKFLOW FILE existing, not on the kill-switch state — disabling the security lane while the file exists leaves security findings reported by NO lane until the switch is lifted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…on noun (2g) The marker copy predates two cadence changes and is now wrong twice over: the code-review lane dropped `synchronize` (a push does NOT re-trigger it), and the automatic retry is gated (it is skipped when assistant turns were already spent or the failure is auth-class), so "one automatic retry already ran" overstates it. - claude-lane-marker-comment default body-copy becomes "Re-run the job to retry the review." — the one sentence true for every lane. Takes effect for the review lanes at the next composite re-pin (Phase 3g); until then their explicit values below carry the copy. - claude-review.yml passes an explicit body-copy, live now at the old pin: re-run to retry, a new push does not re-trigger this lane, and the retry-may-already-have-run caveat as a `> `-prefixed continuation line. - claude-security-review.yml's "one automatic retry already ran" line becomes "An automatic retry may already have run — it is skipped when a partial review could duplicate comments, or when the failure class needs an operator." Every other line is unchanged. - claude-lane-outcome's ::error annotation derives its disclaimer noun from the existing `lane` input (/security/i → "not a security signal", else "not a code-quality signal") instead of hardcoding "code-quality" for every lane — no new input, so callers need no wiring and the fix lands at the next re-pin. The `class=<token>` term stays bare and greppable. Clears the cosmetic delta recorded in the 2a-addendum. The e2e lane's reporting is untouched (marker/class adoption deferred, 2h). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Phase 1 evidence gains the sync-PR merge state (all five merged; automerge had never actually been armed) and the resolution of the REVIEW.md-exercise open question (lanes skip the sync bot by design). The 2a-addendum records the A2 merge SHA. A 2b-2i execution record captures the PR-B commit trail, the sanity-check amendments the plan text predates, the 2f-addendum docs-route decision, and the cancellation-semantics hardening. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ill-switch docs Verifier findings on the 2d-2g series. The retry gate now requires a successfully parsed execution file before retrying: a missing file proves nothing about assistant turns (a step timeout or hard kill can lose it after turns were spent), and retrying on absence of evidence risks the duplicate comments the gate exists to prevent. The orphan tracking-comment deletion matches claude[bot] — the App identity that actually authors tracking comments on live PRs — alongside github-actions[bot], keeping the run-URL conjunct as the safety scope; the security lane's paginate call gains the per_page the commit that claimed it missed. README's lane contract gains the kill-switch documentation, including the coverage-gap warning: a disabled security lane leaves no lane reporting security findings, because REVIEW.md's exclusion keys on the workflow file existing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 069a2c3eea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+13
to
+15
| # No `synchronize` — dogfoods the canonical cadence: review on | ||
| # open/ready/reopen; re-run the job for a fresh review after a push. | ||
| types: [opened, ready_for_review, reopened] |
There was a problem hiding this comment.
After a PR receives another push, re-running its earlier job does not provide the promised fresh review: GitHub preserves the original event payload and head SHA for a re-run, so the freshness guard in claude-review.yml recognizes that SHA as superseded and skips every review-producing step. With synchronize removed and no dispatch-based current-head path in this caller, updated PRs cannot receive another review except through an unrelated close/reopen or ready event; retain a current-head trigger or add an explicit fresh-run mechanism.
Useful? React with 👍 / 👎.
Comment on lines
+466
to
+468
| } catch { | ||
| // A missing or unparsable execution file records no assistant | ||
| // turn, so the artifact-safety condition holds. |
There was a problem hiding this comment.
When the first E2E attempt is killed after running the prompt's gh pr comment instruction but before its execution file is finalized, this catch treats the missing or malformed file as an empty transcript and launches a second attempt, producing a duplicate findings comment. The two review lanes in this same change correctly reject this case because a timeout or OOM can lose the file after assistant turns were already spent; apply that evidence-only guard here as well and retry only when a parsed transcript proves zero turns.
Useful? React with 👍 / 👎.
kyle-sexton
added a commit
that referenced
this pull request
Jul 27, 2026
…se-out (#282) Reusable-side prerequisite for the Phase 3 caller components (`docs/topics/claude-review-lanes/PLAN.md`, item 3a): the security lane gains a `paths-file` input so each consumer keeps its security-sensitive-surface list in its own tree at the conventional `.github/claude-security-paths`, instead of inlining it in the synced caller. - Precedence: non-empty inline `paths` wins (unchanged semantics, file never fetched); else `paths-file` is fetched from the PR's **base** repository at the base branch via the contents API inside the existing github-script step — no checkout added; both empty → every call relevant (byte-compatible with today). - Fail-open discipline throughout: absent file, fetch error, non-string response, or vacuous content → `relevant=true` with a warning. A matcher fault can never silently skip a required security review. - The `changes` job gains only `contents: read` (single-blob read; the job stays checkout-free). Base workflow's `security-review` job already required `contents: read`, so no existing caller breaks. - PLAN.md: Phase 2 marked [DONE] with close-out evidence (PR #280 / `cf666f67`, kill-switch dogfood, v0.9.0); Phase 3 [DOING]. Verification: fresh-context verifier ran a 13-case base/head differential (byte-identical semantics on all pre-existing paths) plus a 15-case adversarial fail-open matrix — verdict SHIP; its two findings (trust-boundary wording, vacuous-file regression pin) are folded in. 286 script tests green; actionlint clean; zizmor identical to base. ## Related No linked issue — part of the claude-review-lanes effort (Phase 3a prerequisite). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 29, 2026
kyle-sexton
added a commit
that referenced
this pull request
Jul 29, 2026
) ## Summary The retry gate asked the wrong question, and the answer made the retry unreachable for the only failure class it was built for. An Anthropic API error does not end the stream quietly. Claude Code emits a **synthetic assistant turn** carrying the error as ordinary text — `message.model` is the literal `<synthetic>`, alongside `error`, `isApiErrorMessage`, and `apiErrorStatus` — and only then the result entry. So a 429 that killed the run on its first API call still records a `type: "assistant"` entry, and the guard ```js const assistantTurns = entries.filter((entry) => entry?.type === "assistant").length; if (assistantTurns > 0) { ...; return; } // returns here const status = last?.api_error_status; // never reached ``` returned eight lines before the failure class was ever read. Not just the 429 path: for any payload carrying an assistant entry, the **entire** post-guard section was dead code — including the auth exclusion below it. Evidence, org-wide on 2026-07-29: 219 recovered result projections, every one `num_turns: 1`, `total_cost_usd: 0`, first-turn 429. The gate reached a decision 19 times (all `ci-workflows`, the only repo running the gate) and emitted the turn-count refusal all 19 times. The other two notices never fired anywhere. ### Why the predicate changed and not the order Testing `api_error_status` before the turn count was the obvious fix and it is unsound. `api_error_status` records the error that **terminated** the conversation and says nothing about what preceded it, so a 429 arriving after thirty real turns presents identically to one arriving before the first. Class first would retry a mid-review failure that had already posted inline comments — precisely the duplicate the guard exists to prevent. The predicate was what was wrong, so the predicate is what changed: synthetic error turns no longer count as work, and a single turn of real work still blocks the retry exactly as before. ### A hole closed in the same change Discounting synthetic turns opens one: the action has a write path that saves the execution file with **no result entry**, so an auth-class failure can be recorded only on the synthetic turn. Before this change `assistantTurns = 1` blocked that case incidentally; after it, `realTurns = 0` would have let an auth failure retry. The auth predicate now reads both sources. It stays a superset of `classify.cjs` on *inputs*, not on classes — `classify.cjs` is deliberately not widened, since that would change the fail-closed mapping. ### Field grounding, and which way drift breaks `error` is carried by the SDK's published message union. `isApiErrorMessage`, `apiErrorStatus`, and the `<synthetic>` sentinel are observed wire fields the published types do not declare. Depending on them is safe in one direction only, and that is the direction taken: if upstream renames any of them the turn counts as real, `realTurns` rises above zero, and the gate declines. **Drift degrades to not retrying, never to duplicate comments.** That asymmetry is what justifies depending on untyped fields at all, and it is recorded in the code, not only here. ### Per-lane findings | lane | ordering defect | second finding | |---|---|---| | `claude-security-review.yml` | yes | — | | `claude-review.yml` | yes | — | | `claude-e2e-verify.yml` | yes | **also** initialised `entries = []` with no evidence guard | `claude-e2e-verify` carried a second, independent defect: a missing or unparsable execution file fell through to a zero count and **retried**. A hard kill can lose the file after turns were already spent, and this lane posts a findings comment a second attempt would duplicate. It now refuses on an unprovable file like its siblings. **Scope note:** that e2e change is a behavior change beyond the ordering defect — that lane used to retry on a missing file and now does not. Called out so it is not discovered in the diff. ### The backoff constant Second commit. The retry's "measured motivation" cited run `30217744377` as failing in 25s and returning a verdict in 3m17s on a manual re-run. Both numbers are real; neither is a delay. From the run's own attempt timestamps: ``` attempt 1 security-review job 19:52:02Z -> 19:52:27Z 25s the failure attempt 2 security-review job 21:20:05Z -> 21:23:22Z 3m17s that attempt's DURATION ``` Recovery took **87m27s**, and even that is bounded below by when a human happened to press re-run. The run cannot calibrate an in-job delay of any length. Also corrected in `POSTURE`: *"a bounded in-job retry absorbs the sporadic-429 class"* asserted a property the code did not have, and it is load-bearing there — one of the two ways availability is bought back while a required check fails closed. **Decision: keep the mechanism, keep 90s, make both claims honest.** Retiring the in-job retry was considered and rejected on evidence grounds. It has never executed — zero times in 251 attempts — but the reason is *reachability*, not efficacy: the gate refused every attempt that reached it, and no consumer pin carries a retry step at all. Deleting an unexecuted mechanism would reason from absent evidence, the same error as the citation. 90s stays for the same reason — there is nothing to derive a different constant from, and inventing one repeats the defect. A **revisit trigger** is recorded in the idiom this file already uses for the merge-queue amendment: a lane failure whose retry step executes and whose second attempt succeeds calibrates the constant; if 429s recur and no such instance appears now that the gate is reachable, retirement becomes supported. Note the constant is **90s + 0–29s jitter** (90–119s effective), not 60s. ### Current fleet state (reported, not changed) No consumer pin has ever run this gate — it was introduced by #280 (`cf666f67`), and today it executes only via `ci-workflows`' own `*-self.yml` callers, which use `uses: ./.github/workflows/…`. | repo | `claude-review` pin | `claude-security-review` pin | |---|---|---| | `claude-code-plugins` | `e2951077` | **`66073e5`** | | `standards`, `medley`, `dotfiles`, `github-iac`, `provisioning` | `90f1c54` | n/a | `claude-code-plugins` bumped its security pin to `66073e5` **today at 14:14:42Z** (`326a8e6c6`), after the blackout. `66073e5` fails closed **and carries an ungated unconditional retry**. So the risk in a forward bump past `cf666f67` is not "arms a broken gate" — it is **replaces a working unconditional retry with an inert one**, while keeping fail-closed. Strictly worse availability than what is deployed. During the blackout that repo was still at `e2951077`, which has no retry step and fails **open** — so the 29 green-but-unreviewed security checks measured today are historical posture, not current. No consumer is re-pinned by this PR. That decision is the user's. ## Test plan New `.github/scripts/claude-lane-retry-gate.test.cjs` **executes** each lane's `script:` body against real wire payloads rather than pattern-matching the YAML — a regex cannot tell a reachable branch from a dead one, which is how this survived review. 16 cases per lane: first-call 429 retries; 429-after-real-work does not; 401/402/403 from the result entry and from a synthetic turn with no result entry; the `errors[]` auth variant; a synthetic turn with no readable status; an empty array; missing, unparsable, non-array, and unset execution files; and orphan tracking-comment deletion scoped to this run. Proof the suite is not vacuous — run against **unmodified `origin/main`** (`70e11d9`, workflows untouched, only the test file added): ``` ℹ tests 48 ℹ pass 11 ℹ fail 37 ✖ claude-security-review: a 429 on the first API call retries ✖ claude-security-review: an auth-class 401 on the result entry does not retry ✖ claude-e2e-verify: a missing execution file does not retry … 34 more ``` The auth cases fail at HEAD on the *reason*, not the decision — HEAD returns `retry=false` with the turn-count notice, demonstrating the auth branch is unreachable too. On this branch: ``` $ node --test .github/scripts/claude-lane-retry-gate.test.cjs ℹ tests 42 ℹ pass 42 ℹ fail 0 $ node --test .github/scripts/*.test.cjs ℹ tests 346 ℹ pass 346 ℹ fail 0 $ node --test .github/actions/claude-lane-outcome/*.test.cjs ℹ tests 10 ℹ pass 10 ℹ fail 0 ``` `actionlint 1.7.12` (the fleet version) exits 0 on all three lanes. **Disclosure:** it was run as `actionlint -shellcheck= -pyflakes=` — with the shellcheck integration enabled it did not terminate within 15 minutes on this Windows host, so it was killed rather than reported as passing. This change touches no `run:` block, only a `script:` body and comments, so the shell path is unaffected; CI's own actionlint job covers it. `claude-security-review-fail-closed.test.cjs` keeps its wiring assertions and now points at the executable suite for the decision itself instead of restating it. ## Related No linked issue — this defect was found while auditing the lanes' retry behavior, and #228/#238 (dead-credential surfacing) cover a different class. ## Do not merge before verification completes This repo carries `babysit_loop_merge: c3-autonomous`, and a PR auto-merged earlier today while its verifier was still running. The `do-not-merge` label is applied — per this effort's own finding, the label is the only thing that blocks; a heading in a PR body blocks nothing. A fresh-context verifier is running now; remove the label only once its verdict is recorded here. One expectation to set: this PR's own `*-self` lanes exercise the modified gate, and the org is currently rate-limited. Whatever they report is reported as-is — they get **one** attempt and will not be re-run to chase green. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
kyle-sexton
added a commit
that referenced
this pull request
Sep 24, 2026
…bled (#621) No related issue: operator request to drop the review-count status comment every fleet caller disables. ## Summary Every caller in the fleet sets `max-reviews-per-pr: 0`, yet the lane still looked up and upserted the "Claude has reviewed this PR N times" comment after every review. With the cap disabled the comment carries a count nothing reads. ## Fix - `Check the per-PR review count` returns before the comment lookup when the cap is `<= 0`. - `Update the review-count status comment` returns before any write when the cap is `<= 0`; its body now always states the cap, since it only runs with one. - A positive cap keeps the comment and gate unchanged. - Input description and README updated. The security lane posts no count comment; its `last-reviewed head` marker is functional (incremental relevance) and is untouched. ## Verification - New tests in `claude-review-outcome-wiring.test.cjs` execute both step scripts against a recording client: cap `0` makes no API call (no read, no create/update); cap `5` still lists comments and creates one. - `node --test .github/scripts/*.test.cjs`: 264 pass, 0 fail. `actionlint` clean. ## Related - Introduced by #280. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
kyle-sexton
added a commit
that referenced
this pull request
Sep 24, 2026
…ure visibility (#622) No related issue: operator-approved removal of lane bookkeeping. ## Summary Slims `claude-review.yml` (1,462 to 241 lines) and `claude-security-review.yml` (1,749 to 228 lines) to three purposes: running the review (plugin command, `pull_request` triggers, draft skip), security (fork skip, bot skip, privileged-trigger tripwire, least-privilege permissions, one named secret, credential strip, `display_report: false`, `exclude-comments-by-actor`), and failure visibility (`claude-lane-outcome` classification and an always-on status job per lane). ## Fix Removed from both lanes: - **Dispatch re-review:** `pr-number` input, "Resolve PR context", dispatch delivery snapshot/collect, and the `workflow_dispatch` triggers in both self callers. Steps read `github.event.pull_request.*` directly. - **Retry:** the retry gate, back-off, retry attempt, "Resolve the effective review attempt", and `retry-delay-seconds`. One attempt with `continue-on-error` and a step timeout; job timeouts sized for one attempt (15 and 25 minutes). - **Freshness:** the `claude-lane-freshness` step, every `superseded` gate, and the job-level per-head concurrency blocks. Callers own concurrency. - **Kill-switches:** `CLAUDE_LANES_DISABLED`, `CLAUDE_REVIEW_DISABLED`, `CLAUDE_SECURITY_REVIEW_DISABLED`. - **Marker comments:** both infra-status post/clear steps. - **Inputs:** `prompt` (the plugin command text is now inline in `prompt:`), `skip-actors` (replaced by `!endsWith(github.actor, '[bot]')`), and `status-check` (status jobs run on `always()`). Removed from the code-review lane: - **Review-count cap:** `max-reviews-per-pr`, the count check, and the count comment upsert. - **Standards mount:** `standards-ref`, the `STANDARDS_REVIEW_APP_*` secrets, the token/clear/checkout steps, and the `--add-dir` branch. - **Inputs:** `track-progress` (hardcoded true), `display-report` (hardcoded false), `timeout-minutes`, and `allowed-bots`. The #443 author-association clause is unreachable once every bot actor skips, so it is gone and `allowed_bots` is no longer passed. - **Outputs:** `review-ran`. Removed from the security lane: - **Path gating:** `paths`, `paths-file`, the `changes` job with its incremental last-reviewed-head listing (#259), and "Persist the last-reviewed head". The lane runs on every non-draft PR. - **Ruling:** "Rule on an in-scope non-run". - **Outputs:** `relevant`, `review-ran`, `review-failed`, `failure-class`. Shared composites: deleted `claude-lane-freshness` and `claude-lane-marker-comment`. `claude-lane-outcome` drops `dispatch-evidence.cjs`, the `event-name`/`delivery-evidence` inputs, and the `no-delivery` class. The lanes keep their `claude-lane-outcome@ac06265` (v0.27.0) pin. Tests and docs: deleted the tests of removed features; pruned compose-args, status-check, plugin-path, declared-outputs, outcome-wiring and outcome-step; renamed `claude-lane-bot-association.test.cjs` to `claude-lane-job-gates.test.cjs`, which now pins the bot, draft and fork skips and the tripwire for both lanes. Rewrote the README lane sections and deleted `security-review-absent-mitigation.md`. Beyond the brief: - The code-review lane now skips fork PRs like the security lane. With the status check always on, a secretless fork run would otherwise redden `claude-review-status` on every fork PR. - `cursor[bot]` pushes are no longer reviewed. It was in `allowed_bots` on both lanes and passed the #443 clause when the PR author was OWNER, MEMBER or COLLABORATOR; the blanket bot skip now skips it. ## Verification - `node --test .github/scripts/*.test.cjs`: 157 tests, 157 pass, 0 fail. - `node --test .github/actions/claude-lane-outcome/*.test.cjs`: 15 tests, 15 pass, 0 fail. - `actionlint`: clean. - `npx -y @biomejs/biome@2.5.11 check --config-path=fixtures/typescript/good/biome.json` on the 7 changed `.cjs` files: clean. - `npx markdownlint-cli2 README.md`: 0 issues. `typos` on the changed files: clean. `lychee --offline README.md`: 0 errors. - The pinned-revision check in `claude-review-outcome-wiring.test.cjs` ran against `ac06265` (not skipped) and passes: both consumed outputs, `review-failed` and `failure-class`, are declared at that pin. ## Related - #280 - #619 - #621 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 2 feature set of the claude-review-lanes plan (
docs/topics/claude-review-lanes/PLAN.md), delivered as PR-B on top of the merged composite extraction (PR-A1 #274) and repoint (PR-A2 #276).What's in here
claude-sonnet-5defaults on all three lanes; inline-comment MCP tool allowed on both review lanes;exclude_comments_by_actorwidened to both Dependabot spellings (upstream #1514) and lifted to an input on the security lane; skip-actors self-trigger ban; org-secret posture corrected (visibility: all); code-review default prompt names its lane and defers security scope to REVIEW.md's split.synchronize, dogfooded in the self-caller);max-reviews-per-pr(default 5) counted via a visible status comment that doubles as the human signal — fail-open, deletion resets. Security lane keepssynchronize(its check certifies execution at the merge head).queue: max; security:cancel-in-progress: false, no queue — a cancelled required check is not a skip).CLAUDE_LANES_DISABLED+ per-lane variables on all three lanes; name-stable skip; repo overrides org; README + headers record the security-lane coverage-gap window.always()for!cancelled()across all lanes — cancellation is the concurrency group's retirement mechanism.Verification
Two fresh-context verifier passes (2b/2i/2c and 2d-2g) — all checks PASS, overall SHIP; their findings (doc drift, count-gate hardening, tracking-comment authorship, retry evidence guard) are folded in as dedicated commits. 280 script tests + 10 composite tests green; actionlint clean; zizmor identical to baseline.
Related
synchronizedropped for the code-review lane; the security lane deliberately keeps it (QF1: a required execution check must report on the latest head; the paths gate makes non-relevant pushes skip in seconds).🤖 Generated with Claude Code