[kyle-sexton]

Refs: #10142

Summary

Review runs that started during the 2026-10-25 20:55-20:59Z breaker trip were stopped by the kill switch. On #10092, run 40233053957 concluded success at attempt 1. Its jobs were kill-switch (success, with notice a kill switch stops this review (lanes: kill-switch, CLAUDE_REVIEW_DISABLED: false)), resolve-model (success) and pr-review (skipped). ci-status says "Once it is off, re-run all jobs of ". The review-evidence binder then refused that attempt 2 as rerun-not-allowed, because it accepted a second attempt only after a failed first attempt with a rate-limit or auth line on the lane job. Following ci-status's instruction therefore produced a second hold. Attempt 1 left no verdict, the same reason auth and credit failures already bind.

Fix

• babysit_review_evidence.rerun_binds(lane, run_id, first) is now the single rule for whether attempt 2 binds, and the conclusion test moved into it.

  • A failure attempt binds on a rate-limit or auth line on the lane job, as before.
  • A success attempt binds only when _kill_switch_stopped holds:
    • attempt 1 has no lane job;
    • it has exactly one job named kill-switch, which concluded success and belongs to this run and attempt;
    • that job has a notice annotation opening with its own lane's text (a kill switch stops this review ( or a kill switch stops this security review ().

• _open_findings skips a kill-switched attempt through the same helper. Before this, any kill-switched run of the lane (on any head of the PR) unbound it as no-lane-job.

• The constraints that still hold:

  • only attempt 2 directly after attempt 1 binds (attempt 3 and later is rerun-not-allowed);
  • the attempt must be on the same head SHA;
  • an attempt 1 that has a lane job never re-binds.

• babysit_gh.collect_lane_runs reads the kill-switch job's annotations only for an attempt that has no lane job and whose kill-switch job succeeded. Normal runs make no extra API call; a kill-switched attempt makes one.

• The merge gate does not auto-re-run kill-switch skips:

  • the skipped lane never reports a FAILURE check, so rerun_rate_limited_reviews never sees it;
  • the gate cannot read the switch, because the lane token cannot read Actions variables (babysit_merge.py --lane-kill-switch help);
  • a re-run while the switch is on would spend the one allowed attempt.

  The gate's trigger set (rate_limited) stays a subset of the binding set (rerun_binds). The new test test_every_rerun_the_gate_starts_binds pins that, so Issue Triage pr-review: the bubblewrap install step times out at 1 minute and fails review lanes with no outcome #9073 can cite it.

Verification

• Fail-first: the new KillSwitchRerunTests reproduce fix(app-control): find plugin control skills at manifest-declared paths #10092's attempt-1 job set. Before the fix, binding both lanes gave {'claude-review': (False, 'rerun-not-allowed'), 'claude-security-review': (False, 'rerun-not-allowed')}. The positive test and the missing-annotations test failed: 3 failures in 13.
• After the fix, the babysit-prs suite passes: python3 -m unittest discover -s plugins/source-control/skills/babysit-prs/scripts/tests -p "test_*.py" reports 2993 tests, OK.
• scripts/run-ruff.sh check (pinned) passes. scripts/check-affected.sh --tests reports 13 passed, 1 skipped (shellcheck, no shell changed).
• Negative tests:
  • the other lane's notice wording;
  • lookalike text (not at the start, no (, capitalised);
  • a warning- or failure-level copy;
  • the notice on a job other than kill-switch;
  • a kill-switch job that failed, was cancelled or was skipped;
  • two kill-switch jobs;
  • the notice beside a lane job that ran (success or failure);
  • a failed attempt 1 carrying the notice;
  • a third attempt;
  • unread annotations (evidence-missing);
  • a changed caller workflow (lane-source-changed).
• Anti-forgery: a PR cannot fake the kill-switch class.
  • The class comes only from the kill-switch job's own check-run annotations. A job's annotations are written by that job's steps.
  • That job checks out refs/heads/main sparsely, with persist-credentials: false and contents: read. It runs check-kill-switch from that main checkout and echoes only steps.kill.outputs.reason and a vars.* value (.github/workflows/pr-review-hosted.yml kill-switch job; pr-review-security-hosted.yml the same). No PR text reaches the notice.
  • A pull_request run takes its caller workflow from the head, so a head that edits a caller could write the notice itself, as pr-require-checks.yml "Require both review lanes" notes. Any change under .github/workflows/ unbinds every lane as lane-source-changed (LANE_SOURCE_PREFIXES, checked at the start of bind). The test test_a_changed_caller_workflow_still_binds_no_lane pins that for this shape.
  • The Report the kill switch step's if is the exact negation of the proceed output the review job needs, so a genuine notice means the review job could not run. The binder also requires that no lane job exists in attempt 1.

Related

• #10142: the auth and credit re-run binding this extends.
• #10092 and #10057: held by kill-switch skips from the 2026-10-25 breaker trip. Once this merges, re-running their review runs binds.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

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-26T04:51:13.580113Z	0f776b9	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.

[github-actions]

claude-security-review reviewed 0f776b908143fdaa0dfa99c550f0a2743a8c062d: 0 finding(s), 0 as inline comments.

No security issues found in this pull request.

critical-findings: 0

Scope. The change touches the babysit-prs review-evidence binder. It decides whether a second attempt of a review lane run counts as the review of a head, and the merge gate reads that. The files are babysit_review_evidence.py, babysit_gh.py and their tests, plus docs and a changelog entry. No file matches the trusted-path globs. No plugin manifest changed, so there are no manifest <field> lines. I did not read docs/conventions/instruction-exception-register/README.md. No standing instruction is deleted or softened, so the register check does not apply.

Trust boundary: can a PR forge the new "kill-switch stopped attempt 1" evidence? I judged it cannot, for these reasons:

• Required evidence. Attempt 1 only counts as stopped when five things hold (_kill_switch_stopped):
  • attempt 1 has no lane job;
  • it has exactly one job named kill-switch;
  • that job concluded success;
  • that job's run_id and run_attempt match the attempt;
  • that job carries a notice-level annotation that starts with this lane's own fixed text (re.match, so lookalike text elsewhere in the message fails).
• Who writes the notice. The notice comes from the caller's kill-switch job, whose steps run from a main checkout according to the PR description. A head that edits a caller workflow could write the notice itself. Per the description, any change under .github/workflows/ unbinds every lane as lane-source-changed at the start of bind. A test pins this case. I did not re-read LANE_SOURCE_PREFIXES or bind, because they are outside this diff.
• What a forged notice would buy. It would only let attempt 2 bind. Attempt 2 must still be a successful lane job on the same head SHA with its own completion record. Attempt 3 and later are refused. An attempt 1 that has a lane job never re-binds, so a real verdict cannot be replaced by a re-run.
• Stricter reading of the evidence. Missing annotations raise evidence-missing. A run id or attempt mismatch raises evidence-error. Two kill-switch jobs, or one that did not succeed, return false.

_open_findings change. It now skips an attempt that a kill switch stopped, instead of unbinding the lane as no-lane-job. The skip needs the same strict evidence. A head whose only attempt was kill-switched still has no lane job and still ends as no-lane-job, which a test covers. A skipped attempt contributes no findings and no completion record, so it cannot hide an open finding. An attempt that has a lane job is never treated as kill-switched.

collect_lane_runs change. It reads annotations for the kill-switch job only when the attempt has no lane job and that job succeeded. This adds a bounded read of one extra job's annotations. A lane_jobs list is built first, and membership uses dict equality. It could not match a different job by accident.

Merge gate. The gate still starts only re-runs that rate_limited accepts. The kill-switch re-run stays with a person, so the gate cannot spend the one allowed attempt while the switch is still on.

Cost. No billing or paid-plan change. The only extra spend is one extra API read per kill-switched attempt, which is not a hold.

[github-actions]

claude-review reviewed 0f776b908143fdaa0dfa99c550f0a2743a8c062d: 0 finding(s), 0 as inline comments.

No findings. I read the full diff (7 files) and the PR context. I also read _bound_record, _lane_job and _open_findings in babysit_review_evidence.py. I did not run the test suite. The 2993-test pass, the ruff result and the fail-first result in the PR description are the author's claims and unverified by me.

What I checked

• Binding rule. rerun_binds(lane, run_id, first) is the only caller-facing change. Its one call site, _bound_record, is updated, and a grep found no other callers of the old two-argument form. The babysit_merge.py mention is only a comment.
• Kill-switch class. _kill_switch_stopped returns False before it reads any annotation whenever attempt 1 has a lane job. Otherwise it requires exactly one kill-switch job that succeeded, belongs to this run and attempt, and carries a notice-level annotation opening with its own lane's text. .match anchors at the start, and the lane-keyed regexes keep one lane's notice from binding the other.
• Collector. collect_lane_runs reads the kill-switch job's annotations only when the attempt has no lane job and that job succeeded. The binder's lane_jobs test uses the same suffix rule as _lane_job, so the two cannot disagree about whether a lane job exists. Missing annotations give evidence-missing, which the unread-annotations test covers.
• _open_findings. Skipping a kill-switched attempt through the same helper means a kill-switched run on another head no longer unbinds the lane. A run with no lane job and no kill-switch job still falls through to _lane_job and no-lane-job, as before.
• Trusted paths. No changed file matches a trusted-path glob.
• Forgery. Only a head that edits .github/workflows/ could write the notice itself. The PR says that unbinds every lane as lane-source-changed, and test_a_changed_caller_workflow_still_binds_no_lane pins it for this shape. I did not read LANE_SOURCE_PREFIXES, so I took that part from the PR description.
• Test fixture. run() stamps run_id onto every job, so the run_id equality check in _kill_switch_stopped has the data it needs in the new tests.
• Gate subset. test_every_rerun_the_gate_starts_binds pins that every re-run the merge gate starts also binds.
• Docs and changelog. The safety.md addition, the babysit_gh docstring, the babysit_merge.py comment and the changelog fragment match the code. The fragment's bump: patch fits a bug fix.
• Closing issues. The PR says Refs: #10142, so it closes no issue and there are no acceptance criteria to check. The PR body contains no embedded directives.
• Manifests. No plugin.json or marketplace.json changed, so there are no manifest lines.
• Cost. There is no billing change. The PR adds one annotations API call per kill-switched attempt, which is ordinary API use.