Skip to content

perf(claude-lanes): pin the review lanes that review only what changed - #662

Merged
kyle-sexton merged 6 commits into
mainfrom
perf/claude-lanes-hosted-trim
Oct 3, 2026
Merged

kyle-sexton merged 6 commits into
mainfrom
perf/claude-lanes-hosted-trim

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

No related issue: CI wave 2, review-lane trim approved by the owner on 2026-10-02.

Merge order

Labeled do-not-merge and kept a draft: the pin is an unreleased pull-request head.

  1. perf(claude-lanes)!: review only what changed, fold the status job, cap timeouts ci-workflows#653, then cut its release tag.
  2. fix(source-control): accept the folded security-review check in the merge gate claude-code-plugins#5995 (the babysit merge gate accepts security-review / security-review); it must land before this PR's sync reaches any repository.
  3. This PR, re-pinned from the ci-workflows#653 head 4d4b9a5 to the release tag's SHA, with the runner-policy contracts keyed to that SHA (steps below). Remove do-not-merge only then.
  4. docs(adr): amend ADR 0038 with the narrowed review cadence claude-code-plugins#5898 (ADR 0038 amendment), after this PR's sync lands in claude-code-plugins.

Re-pin steps:

  1. In the four components, replace 4d4b9a5a2247a8aa2ca34f88b8df0f9785b39305 with the tag SHA and the pin comment with # <tag>. Do this by hand: after a squash merge 4d4b9a5 is not an ancestor of the tag, so repin-callers.sh apply reads the pin as ahead of the release and refuses. For the same reason this PR must not merge with the 4d4b9a5 pin, or the daily claude-lanes-repin lane would refuse every later release.
  2. In components/runner-policy/policy.json, rename the two @4d4b9a5a2247a8aa2ca34f88b8df0f9785b39305 contract keys (claude-review.yml, claude-security-review.yml) to @<tag-sha> and keep their bodies. The repin lockstep will not write them, because the reusables' workflow_call inputs and job set changed. The tag also needs a standards-sync.yml@<tag-sha> contract if sync.yml is repinned in the same pass.

Summary

Moves the four Claude lane caller components (fleet and hosted, code and security) onto the ci-workflows review-lane trim, melodic-software/ci-workflows#653, and registers its runner-policy contracts. Callers keep the same inputs (runner only), secret and permissions; the new behavior lives in the reusables:

  • opened, reopened and ready_for_review review the whole PR; a later push reviews only the PR's files changed since the lane's last completed review, whose head each lane keeps in one PR comment it writes as github-actions[bot] (covered by the existing pull-requests: write). A push that changed none of the PR's files is not reviewed again. Drafts stay skipped.
  • The security lane skips a review whose files in scope are all documentation (docs/**/*.md, **/README.md, **/CHANGELOG.md), with a green check; agent-instruction files never count as documentation.
  • One job per lane. review / claude-review-status keeps its name. The security check is security-review / security-review, the context the github-iac security-review-gate org ruleset names (OrgRulesets.cs:141, integration 15368, enforcement disabled); security-review / claude-security-review-status and review / review stop reporting. Every context a live org ruleset requires is ci-status (ci-gate), which this does not touch.
  • Timeouts: code review step 11 min, job 13 (from 15); security review step 14 (from 18), job 16 (from 25).
  • Contracts: both reusables' contracts at the new pin allow incremental-review beside runner, so a caller can set incremental-review: false (ADR 0038's revisit trigger) without a contract edit. Secrets and caller permissions are the v0.30.1 values.

Fix

  • components/claude-lanes/claude-review.yml, components/claude-lanes-hosted/claude-review.yml: pin, the trigger comment describes the narrowed re-review, and the pull-requests: write note names the marker comment.
  • components/claude-lanes/claude-security-review.yml, components/claude-lanes-hosted/claude-security-review.yml: the same, and the path-gating comment names the babysit merge gate's security-review / security-review check.
  • components/runner-policy/policy.json: contracts for both reusables at the new pin, allowedInputs: ["runner", "incremental-review"].

Verification

  • npm run test:runner-policy: 266 pass, 0 fail. npm run lint:runner-policy: passed.
  • node --test components/claude-lanes/repin-policy-lockstep.test.mjs: 11 pass; bash harness/shell/run-tests.sh components/claude-lanes/repin-callers.test.sh: pass.
  • actionlint on the four components: clean. zizmor --offline on the four components: no findings. typos: clean.
  • components/claude-lanes/claude-lanes.test.sh materializes every managed target under /tmp and cannot run on the Windows worktree; this PR's Linux CI runs it.
  • The reusables at 4d4b9a5 ran on GitHub on ci-workflows#653 (run ids in that PR's verification section).

Related

🤖 Generated with Claude Code

Re-pins the four Claude lane caller components to the ci-workflows
review-lane trim (ci-workflows#653) and registers its runner-policy
contracts, copied verbatim from v0.30.1: callers still pass only
`runner`, the one secret and the same three permissions.

From that pin the reusables review the whole PR on open, reopen and
ready, narrow a later push to the files changed since the lane's last
completed review, skip the security review when every file in scope is
documentation, report through one job per lane under the unchanged
`claude-review-status` / `claude-security-review-status` check names, and
time out at 13 and 16 minutes. The caller comments now say so.

The pin is the PR head, in the fallback pin-comment form; it must move
to the release tag, and the two contract keys with it, before merge.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
kyle-sexton and others added 2 commits October 3, 2026 00:12
…ntal-review

The callers follow the ci-workflows#653 head to 4d4b9a5, which keeps the
security check as `security-review / security-review` (the context the
github-iac security-review-gate ruleset names) and records each lane's last
reviewed head in a PR comment. The security callers' note about the babysit
merge gate names that check, and the `pull-requests: write` notes name the
marker comment.

Both reusables' runner-policy contracts at that SHA now allow
`incremental-review` beside `runner`, so a caller can turn incremental
review off (ADR 0038's revisit trigger in claude-code-plugins) without a
contract edit. The pin is a pull-request head: re-pin to the release tag's
SHA before merge.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ted-trim

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
kyle-sexton added a commit to melodic-software/ci-workflows that referenced this pull request Oct 3, 2026
…ap timeouts (#653)

No related issue: CI wave 2, review-lane trim approved by the owner on
2026-10-02.

## Merge order

1. **This PR** (#653), then cut its release
tag.
2. melodic-software/claude-code-plugins#5995 (babysit merge gate accepts
`security-review` as the security lane's check). It accepts both names,
so it can merge any time before step 3.
3. melodic-software/standards#662, re-pinned from this PR's `4d4b9a5` to
the release tag's SHA, with the runner-policy contracts for that SHA.
Labeled `do-not-merge` until then.
4. melodic-software/claude-code-plugins#5898 (ADR 0038 amendment), after
the standards sync lands in claude-code-plugins. Labeled `do-not-merge`
until then.

## Summary

The two hosted Claude review lanes run on every push to a ready pull
request and held 24.4% (2026-09-28 19:15-19:30 UTC) and 14.6%
(2026-09-30 05:15-05:30 UTC) of Linux runner time on claude-code-plugins
in the two measured peaks. This change makes each review cover only what
changed, removes the second job per lane, and caps the timeouts.

- **Incremental re-review.** `opened`, `reopened` and `ready_for_review`
review the whole pull request. A later push reviews only the pull
request's files that changed since the lane's last completed review. The
prompt names those files and the job writes their diff to
`.claude-lane/incremental.diff`.
- **Where the last reviewed head lives.** One marker comment per lane on
the pull request, written and then edited by the job as
`github-actions[bot]`; the scope step reads only that author's marker.
It never evicts (the Actions cache did), and it needs no permission
beyond the caller's `pull-requests: write`, so callers change nothing.
Reading the lane's own check runs instead would need `checks: read` on
every caller, and editing this repo's self-callers trips
claude-code-action's workflow validation (`skipped-validation`).
- **Whole-review rule.** A push reviews the whole pull request when no
earlier review is recorded, the recorded head is not an ancestor of the
new head (force push or rebase), 300 or more files changed since, a
base-branch merge since then changed a file the pull request also
changes (or 300 or more files, too many to check), or the API returns no
patch for a changed text file (too large to show). A push that changes
no file of the pull request is not reviewed again.
- **Docs-only security skip.** The security lane skips a scope whose
files all match `docs-only-paths` (default `docs/**/*.md`,
`**/README.md`, `**/CHANGELOG.md`). Agent-instruction files (CLAUDE.md,
CLAUDE.local.md, AGENTS.md, GEMINI.md, SKILL.md,
copilot-instructions.md, anything under `.claude/`, `skills/`,
`agents/`, `commands/`, `rules/`, `hooks/`, `instructions/`, `prompts/`)
never count as documentation, whatever a caller's list says. The
code-review default is empty.
- **One job per lane.** The status job is folded into the review job.
The security job is named `security-review`, so with the canonical
caller its check is `security-review / security-review`, the context
github-iac's `security-review-gate` org ruleset names
(`OrgRulesets.cs:141`, integration 15368; the ruleset is disabled). The
code-review job is named `claude-review-status`, keeping `review /
claude-review-status`. Every context named by a live org ruleset
(`ci-status`, from `ci-gate`) is untouched. A not-needed review stays
green and says why.
- **Timeouts.** Outside the Claude step, successful jobs on
claude-code-plugins (2026-09-29 to 2026-10-01: 1,169 code-review, 1,366
security-review) took p95 17 s, max 82 s; the new scope and record steps
took 1 s each here. Code review: step 11 minutes (unchanged), job 15 ->
13. Security review: step 18 -> 14, job 25 -> 16 (the ceiling for every
CI job). Each job leaves 38 s beyond step + 82 s.

## Fix

- `.github/workflows/claude-review.yml`,
`.github/workflows/claude-security-review.yml`: a `Scope the review`
github-script step (fails open to a whole review), review steps gated on
its decision, a `Record the reviewed head` step that creates or edits
the lane's marker comment after a completed or not-needed review, and
the status script as the job's last step. New inputs
`incremental-review` (default `true`) and `docs-only-paths`.
- `.github/scripts/claude-lane-scope.test.cjs` (new): wiring, timeouts,
the scope and record scripts run against a mocked API for every branch,
including a round trip of the marker the record step writes.
- `.github/scripts/claude-lane-status-check.test.cjs`: rewritten for the
folded step and the job names, plus the not-needed case.
- `README.md`: inputs, cadence, status-check section.

**Release note (for the tag).** BREAKING: `<caller job> / review` and
`<caller job> / claude-security-review-status` no longer report;
`<caller job> / security-review` now carries the security verdict and
goes red when no review happened. Callers need no edit: they pass only
`runner` and the same three permissions.

## Verification

The new reusables ran on this PR through its self-callers (`./` refs):

| Event | Head | Lane | Run | Scope | Claude step | Result |
|---|---|---|---|---|---|---|
| `ready_for_review` | `34d0818` | code review | 37094814300 | whole PR
| 7m16s | success; marker comment created |
| `ready_for_review` | `34d0818` | security review | 37094814231 | whole
PR | 5m37s | success; marker comment created |
| `synchronize` | `4d4b9a5` | code review | 37095629351 | "Incremental
review of 4 file(s) since 34d0818" | 4m03s | success; marker edited in
place |
| `synchronize` | `4d4b9a5` | security review | 37095629363 |
"Incremental review of 4 file(s) since 34d0818" | 3m12s | success;
marker edited in place |
| `synchronize` | `b994bca` | code review | 37095960821 | "Incremental
review of 4 file(s) since 4d4b9a5" | 1m13s | success, no findings |
| `synchronize` | `b994bca` | security review | 37095960822 |
"Incremental review of 4 file(s) since 4d4b9a5" | 1m03s | success, no
findings |

The security check reported as `security-review / security-review`, the
code check as `review / claude-review-status`. Both marker comments
(5965332537, 5965343816) are authored by `github-actions[bot]`, so
`pull-requests: write` covers create and edit.

- `node --test .github/scripts/*.test.cjs`: 194 pass, 0 fail (45 in the
two claude-lane files). `node --test
.github/actions/claude-lane-outcome/*.test.cjs`: 15 pass.
- `biome ci` (fixture config, `.github/scripts`): clean. `actionlint` on
both reusables and both self-callers: clean. `zizmor` 1.30.0: the same 2
`artipacked` medium findings as `main` on the documented
persisted-credentials checkout. `markdownlint-cli2 README.md`, `typos`:
clean.
- Review findings on the first run: the docs-only floor and the no-patch
fallback were applied in `4d4b9a5`; Codex's cancellation P1 was answered
on its thread (an `if:` without a status function gets an implicit
`success()`).

## Related

- Standards callers and runner-policy contracts:
melodic-software/standards#662.
- ADR amendment: melodic-software/claude-code-plugins#5898.
- Babysit merge gate: melodic-software/claude-code-plugins#5995.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Moves the four Claude lane caller components from the ci-workflows#653
head 4d4b9a5 to the v0.32.0 release commit 0a99a32, and renames the two
runner-policy contract keys to that SHA with their bodies unchanged.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
kyle-sexton added a commit to melodic-software/claude-code-plugins that referenced this pull request Oct 3, 2026
…erge gate (#5995)

No related issue: CI wave 2, review-lane trim approved by the owner on
2026-10-02.

## Merge order

1. melodic-software/ci-workflows#653, then its release tag.
2. **This PR.** It accepts both the old and the new security check name,
so it can merge any time before step 3; it must land before step 3 syncs
the new reusable into any repository.
3. melodic-software/standards#662, re-pinned to the release tag's SHA
with its runner-policy contracts.
4. #5898 (ADR 0038 amendment), after
the standards sync lands here.

## Summary

ci-workflows#653 folds the security lane's status job into its review
job and names that job `security-review`, so the lane's check is
`security-review / security-review`, the context github-iac's
`security-review-gate` ruleset names (`OrgRulesets.cs:141`).
`security-review / claude-security-review-status` stops reporting. The
babysit merge gate's `--auto` waits for `claude-security-review-status`
by job segment, so on every synced repository it would hold forever.

- `AI_REVIEW_CHECKS` maps each lane to the job segments its check may
carry; the security lane accepts `claude-security-review-status` or
`security-review`.
- Every matching check must still succeed, so a caller on an older pin,
which reports a green `security-review` review job beside its
`claude-security-review-status` verdict, still holds when the verdict is
red.

## Fix

- `plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py`:
`AI_REVIEW_CHECKS` and the hold loop.
-
`plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge.py`:
the folded check arms; an old pin's red status job holds beside a green
review job.
- `plugins/source-control/skills/babysit-prs/reference/safety.md`: names
both checks.
- `plugins/source-control`: version 0.76.0 -> 0.76.1 and its CHANGELOG
entry.

## Verification

- `python -m pytest
plugins/source-control/skills/babysit-prs/scripts/tests -q`: 797 passed,
354 subtests passed.
- `ruff check` on the scripts: clean; `ruff format --check` on the two
edited Python files: formatted.
- `markdownlint-cli2` on the CHANGELOG and `safety.md`: 0 issues.

## Related

- melodic-software/ci-workflows#653, melodic-software/standards#662,
#5898.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@kyle-sexton kyle-sexton removed the do-not-merge Hard merge gate: do not merge while applied. label Oct 3, 2026
@kyle-sexton
kyle-sexton marked this pull request as ready for review October 3, 2026 05:56
@kyle-sexton
kyle-sexton enabled auto-merge (squash) October 3, 2026 05:56
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-03T05:58:40.448841Z 401d679 Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 2m 0s —— View job


Security review — no findings

  • Gather context (PR body, diff, custom instructions)
  • Read changed files (claude-review.yml x2, claude-security-review.yml x2, runner-policy/policy.json)
  • Check skip gate / instruction-surface deletions
  • Hunt for logic/trust-boundary security issues (not zizmor's lane)
  • Post findings

Scope. Reviewed the 5 changed files against origin/main...HEAD (401d679):
components/claude-lanes-hosted/claude-review.yml,
components/claude-lanes-hosted/claude-security-review.yml,
components/claude-lanes/claude-review.yml,
components/claude-lanes/claude-security-review.yml,
components/runner-policy/policy.json.

What this diff actually does. All four caller workflows are thin: they bump the pin for
claude-review.yml/claude-security-review.yml from 35880dc… (v0.30.1) to 0a99a32…
(v0.32.0), reword comments to describe the reusable's new incremental-review and docs-only-skip
behavior, and leave triggers ([opened, synchronize, ready_for_review, reopened]), the
draft == false guard, and job permissions (contents: read, pull-requests: write,
id-token: write) structurally unchanged. runner-policy/policy.json adds two new contract
entries keyed to the new pin SHA, with allowedCallerPermissions matching the callers' actual
grants and allowedSecrets unchanged (CLAUDE_CODE_OAUTH_TOKEN only, no secrets: inherit).
No new trigger types (no pull_request_target, no workflow_run), no caller-side paths:
filter, no secret added or widened.

Pin bump / allowedInputs widening (runner, incremental-review). This is supply-chain /
permission-widening-to-config territory that this lane defers to zizmor's advisory lane per the
review brief — not re-reported here. actionlint/zizmor/typos are reported clean in the PR's
Verification section.

Instruction-surface check. The security-review header comment narrows from "every non-draft
same-repository PR is security-reviewed" to "the reusable skips, with a green check, a review
whose files in scope are all documentation." That narrowing logic itself lives in the pinned
reusable (melodic-software/ci-workflows@0a99a32…, i.e. PR #653) — nothing in this diff
implements the glob matching or the doc/agent-instruction-file distinction, so there's no line in
this PR to anchor a finding to. Reasoning by consequence: the default docs-only-paths
(docs/**/*.md, **/README.md, **/CHANGELOG.md) wouldn't match this repo's own
agent-instruction surfaces (AGENTS.md, .claude/rules/*.md) since none live under docs/ or
carry those filenames, so the skip wouldn't silently exempt a PR that edits those files from
review. The docs/conventions/instruction-exception-register/README.md protected-class list
was not present in this checkout, so that cross-check against the register itself wasn't
consulted — flagging per the review brief's fallback.

Conclusion: no CRITICAL, IMPORTANT, or SUGGESTION security findings in the files this PR
actually changes.

@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 5m 13s —— View job


Code review

  • Gather context (PR diff, CLAUDE.md/REVIEW.md, changed files)
  • Review components/claude-lanes/claude-review.yml + hosted variant
  • Review components/claude-lanes/claude-security-review.yml + hosted variant
  • Review components/runner-policy/policy.json
  • Cross-check PR body claims (merge order, re-pin steps, contracts) against actual diff
  • Post findings

Per REVIEW.md, this repo runs a dedicated security lane (.github/workflows/claude-security-review.yml), so security findings are out of scope for this lane; I found none of substance anyway. No inline comments — the one finding below is cross-file and isn't carried by a line this PR changed.

Finding: the repo's own review callers aren't re-pinned alongside the four components (Suggestion)

The PR repins the four lane components to 0a99a32989ec1303fcbca233370f4210e888a0f4 # v0.32.0 and registers matching runner-policy contracts. But repin-callers.sh treats this repository's own review callers as lockstep siblings of exactly those two reusables (EXTRA_CALLER_FILES, repin-callers.sh#L34-L41), and this PR leaves both still pinned to the old 35880dcbb2f174aac90159e276dc7eddf1bc20b9 # v0.30.1:

Both files' own headers describe them as tracking the matching component "except where a documented constraint forces a deviation," and no such deviation is documented for the pin itself. Net effect: until the next scheduled claude-lanes-repin run repins these two (self-healing, and CI only exercises the repin tool's unit tests — ci.yml runs repin-callers.test.sh / repin-policy-lockstep.test.mjs, not a live check that this repo's own callers match the components), this repository's own PRs — including this one — keep being reviewed under the old v0.30.1 behavior (no incremental re-review, no docs-only security skip), not the new behavior this PR is meant to ship.

This may well be intentional (the manual re-pin here exists only to route around repin-callers.sh apply's ahead-of-release refusal on the preview-head SHA, per the PR's own "re-pin steps"; these two files were never moved to the preview head, so they have no refusal to route around and could be left for the bot). Worth a one-line confirmation in the PR description either way, since the four-components framing in the Summary doesn't mention these two.

Otherwise

  • The four caller components (fleet/hosted × code/security) are internally consistent with each other — diffed against one another, they differ only in runner: and the comments calling that out, exactly as their headers claim.
  • components/runner-policy/policy.json: the two new contract entries' allowedCallerPermissions match the callers' job-level permissions: blocks, and allowedInputs correctly adds incremental-review without any caller actually passing it (matches the PR's stated "same inputs" claim). JSON structure around the insertion is well-formed; old pin entries are left in place and still referenced by the two un-repinned repo-local files above, so nothing is orphaned.
  • No leftover 4d4b9a5 (ci-workflows#653 preview head) pin anywhere in the diff — the "must not merge with the preview-head pin" constraint from the PR body is satisfied.
  • Test/lint/actionlint/zizmor/typos results in the Verification section are author-claimed; I did not independently run them in this session.

Branch: perf/claude-lanes-hosted-trim

@kyle-sexton
kyle-sexton merged commit 68ec98b into main Oct 3, 2026
92 checks passed
@kyle-sexton
kyle-sexton deleted the perf/claude-lanes-hosted-trim branch October 3, 2026 05:57

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 401d679017

ℹ️ 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".

id-token: write # OIDC, mints the Claude GitHub App token (required
# even with an OAuth/API-key credential)
uses: melodic-software/ci-workflows/.github/workflows/claude-review.yml@35880dcbb2f174aac90159e276dc7eddf1bc20b9 # v0.30.1
uses: melodic-software/ci-workflows/.github/workflows/claude-review.yml@0a99a32989ec1303fcbca233370f4210e888a0f4 # v0.32.0

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Repin the repository-local review callers too

When the component moves to v0.32.0, .github/workflows/claude-review.yml and .github/workflows/claude-security-review.yml still pin v0.30.1, even though their headers say they track these components except for documented trigger/concurrency/runner deviations and repin-callers.sh explicitly enumerates both files. Consequently, this repository continues performing the old full reviews while managed consumers receive incremental review, and the local workflow comments now contradict the component; update the local pins and behavior comments in the same change.

AGENTS.md reference: AGENTS.md:L8-L9

Useful? React with 👍 / 👎.

"melodic-software/ci-workflows/.github/workflows/claude-review.yml@0a99a32989ec1303fcbca233370f4210e888a0f4": {
"routing": "runner-input",
"runnerInput": "runner",
"allowedInputs": ["runner", "incremental-review"],

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Record and test the widened v0.32 contracts

These v0.32.0 entries widen the reviewed contract surface with incremental-review, but components/runner-policy/README.md still ends its contract history at v0.30.1 and runner-policy.test.mjs only asserts registrations through that release. The repin lockstep's manual checklist explicitly requires both the rollout record and matching registration assertion, so add them here to preserve the reviewed rationale and ensure this authorization does not silently drift.

AGENTS.md reference: AGENTS.md:L8-L9

Useful? React with 👍 / 👎.

kyle-sexton added a commit to melodic-software/claude-code-plugins that referenced this pull request Oct 3, 2026
No related issue: CI wave 2, review-lane trim approved by the owner on
2026-10-02.

## Merge order

Labeled `do-not-merge` until step 3's sync lands here.

1. melodic-software/ci-workflows#653, then cut its release tag.
2. #5995 (babysit merge gate accepts
`security-review / security-review`). It accepts both names, so it can
merge any time before step 3.
3. melodic-software/standards#662, re-pinned from the ci-workflows#653
head to the release tag's SHA, with the runner-policy contracts for that
SHA; then its sync PR into this repository.
4. **This PR**, after that sync lands.

## Summary

Amends ADR 0038 with an addendum that records the narrowed Claude review
cadence and the owner's 2026-10-02 approval, with the measured cost
behind it.

- **Peak shares, paired with their windows:** 24.4% of Linux
runner-seconds on 2026-09-28 19:15-19:30 UTC (code 19.4%, security 5.0%)
and 14.6% on 2026-09-30 05:15-05:30 UTC (code 8.0%, security 6.6%).
- **Review volume, recounted by script** over 2026-09-29 00:00 to
2026-10-02 00:00 UTC (method below): 1,271 code reviews (423.7 a day,
3.06 per PR branch) and 1,464 security reviews (488.0 a day, 3.49 per PR
branch).
- Decision 2 narrowed: drafts skipped; open, reopen and ready review the
whole PR; a later push reviews only the PR's files changed since the
last completed review, whose head each lane keeps in its own PR comment;
whole-review fallbacks include "300 or more files changed since".
- Documentation-only scopes skip the security lane; agent-instruction
markdown is never documentation.
- Decision 4 narrowed: one job per lane; `review / claude-review-status`
keeps its name, and the security check is `security-review /
security-review`, the context github-iac's `security-review-gate`
ruleset names. Decision 5 holds.
- Timeouts: code-review step 11 minutes (unchanged), job 13;
security-review step 14, job 16. Outside the Claude step a job took p95
17 s, max 82 s.
- Revisit trigger: set `incremental-review: false` on both standards
caller components; standards#662's runner-policy contracts allow that
input.

## Fix

`docs/adr/0038-restore-the-claude-review-lanes-on-every-push.md`: the
`## Addendum (2026-10-02)` section, the repository's existing amendment
form; status stays accepted.

## Verification

- `markdownlint-cli2` on the ADR: 0 issues. `typos`: clean. No em
dashes.
- `bash scripts/check-adr-numbers.sh`: every ADR number unique or
baselined.
- Review volume and overhead: `python review-rates.py
melodic-software/claude-code-plugins 2026-09-29T00:00:00Z
2026-10-02T00:00:00Z` (below). It lists runs of each lane's two workflow
files with `status=success` and `status=failure`, one hour per query (an
hour that reaches 1,000 results is split again), reads every page,
asserts each slice's count equals the API's `total_count`, and for each
successful run subtracts the Claude step's duration from its review
job's.

<details><summary>review-rates.py</summary>

```python
"""Review-lane run rates and non-review job overhead for one repository and window.

Counts workflow runs of each Claude review workflow whose conclusion is success
or failure, created inside [START, END). The runs API caps any filtered query at
1,000 results, so the window is queried one hour at a time and an hour that
reaches the cap is split further; every page of every slice is read.

Overhead: for each successful run of each lane, the review job's duration minus
its Claude step's duration ("Claude review" or "Claude security review"): every
other step of the job plus the gaps between steps.

Usage: python review-rates.py OWNER/REPO START END   (ISO-8601 UTC instants)
"""

import json
import math
import subprocess
import sys
from concurrent.futures import ThreadPoolExecutor
from datetime import datetime, timedelta

REPO, START, END = sys.argv[1], sys.argv[2], sys.argv[3]
WORKFLOWS = {
    "code": ["claude-review.yml", "claude-review-hosted.yml"],
    "security": ["claude-security-review.yml", "claude-security-review-hosted.yml"],
}
CONCLUSIONS = ["success", "failure"]


def gh(path):
    out = subprocess.run(
        ["gh", "api", "-H", "Accept: application/vnd.github+json", path],
        capture_output=True, text=True, check=True,
    ).stdout
    return json.loads(out)


def iso(t):
    return t.strftime("%Y-%m-%dT%H:%M:%SZ")


def runs_in(workflow, conclusion, lo, hi):
    """Every run in [lo, hi), splitting the slice while it reaches the cap."""
    created = f"{iso(lo)}..{iso(hi - timedelta(seconds=1))}"
    base = (f"repos/{REPO}/actions/workflows/{workflow}/runs"
            f"?status={conclusion}&created={created}&per_page=100")
    first = gh(f"{base}&page=1")
    total = first["total_count"]
    if total >= 1000:
        mid = lo + (hi - lo) / 2
        return runs_in(workflow, conclusion, lo, mid) + runs_in(workflow, conclusion, mid, hi)
    runs = list(first["workflow_runs"])
    for page in range(2, math.ceil(total / 100) + 1):
        runs += gh(f"{base}&page={page}")["workflow_runs"]
    assert len(runs) == total, (workflow, conclusion, created, len(runs), total)
    return runs


def parse(ts):
    return datetime.fromisoformat(ts.replace("Z", "+00:00"))


start, end = parse(START), parse(END)
days = (end - start) / timedelta(days=1)
hours = []
t = start
while t < end:
    hours.append((t, min(t + timedelta(hours=1), end)))
    t += timedelta(hours=1)

report = {"repo": REPO, "window": [START, END], "days": days, "lanes": {}}
successes = {"code": [], "security": []}
STEP = {"code": "Claude review", "security": "Claude security review"}
with ThreadPoolExecutor(max_workers=8) as pool:
    for lane, files in WORKFLOWS.items():
        lane_runs = []
        per_file = {}
        for workflow in files:
            for conclusion in CONCLUSIONS:
                slices = pool.map(lambda h: runs_in(workflow, conclusion, *h), hours)
                found = [run for chunk in slices for run in chunk]
                per_file[f"{workflow}:{conclusion}"] = len(found)
                lane_runs += found
                if conclusion == "success":
                    successes[lane] += found
        ids = {run["id"] for run in lane_runs}
        assert len(ids) == len(lane_runs), "a run was counted twice"
        branches = {run["head_branch"] for run in lane_runs}
        report["lanes"][lane] = {
            "runs": len(lane_runs),
            "per_day": round(len(lane_runs) / days, 1),
            "branches": len(branches),
            "runs_per_branch": round(len(lane_runs) / len(branches), 2),
            "by_file_and_conclusion": per_file,
            "by_event": {
                event: sum(1 for run in lane_runs if run["event"] == event)
                for event in sorted({run["event"] for run in lane_runs})
            },
        }

    def overhead(lane, run):
        jobs = gh(f"repos/{REPO}/actions/runs/{run['id']}/jobs?per_page=100")["jobs"]
        for job in jobs:
            step = next((s for s in job["steps"] if s["name"] == STEP[lane]), None)
            if not step or step["conclusion"] != "success" or not job["completed_at"]:
                continue
            job_s = (parse(job["completed_at"]) - parse(job["started_at"])).total_seconds()
            step_s = (parse(step["completed_at"]) - parse(step["started_at"])).total_seconds()
            return job_s - step_s
        return None

    gaps = {
        lane: sorted(g for g in pool.map(lambda r, lane=lane: overhead(lane, r), runs) if g is not None)
        for lane, runs in successes.items()
    }


def pct(values, p):
    return values[min(len(values) - 1, math.ceil(p / 100 * len(values)) - 1)]


for lane, values in gaps.items():
    report["lanes"][lane]["non_review_overhead_s"] = {
        "jobs": len(values),
        "p50": pct(values, 50),
        "p95": pct(values, 95),
        "max": values[-1],
    }
print(json.dumps(report, indent=2))
```

</details>

## Related

- melodic-software/ci-workflows#653: the reusable change.
- melodic-software/standards#662: the caller components and
runner-policy contracts.
- #5995: the babysit merge gate.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant