[kyle-sexton]

No related issue: owner decision Q26 option B (Q-HOLDPATH2)

Summary

The owner chose Q26 option B: "whatever is least amount of friction, human involvement but safe". The pr-merge hold-path floor is now one classed list per language. Only the merge gate's own code, the config the token broker loads, and the trust boundaries still wait for a person. The other modules that enforce the floor merge once a clean security review is bound to the head.

Class	Clears	Entries
trust (5)	a person	.github/actions/check-trusted-trigger/**, .github/actions/check-kill-switch/**, .github/actions/request-lane-token/**, **/CODEOWNERS, **/.gitmodules
gate (23)	a person	.github/actions/resolve-config/** (the broker runs it from main; effect-grants.json included), .github/actions/log-api-budget/rate-guard.mjs, every script in .github/scripts/pr-merge/ (merge-holds, enable-auto-merge, superseded, clear-escalations, plus carry-run-cache, gate-check, head-ref-map, lane-records, rate-stop.bash, refire-refused, rotation, sweep-summary, who, post-unarmed-status), babysit_merge.py, babysit_merge_policy.py, babysit_risk.py and babysit_review_evidence.py (they compute the security review's clear condition), .github/workflows/pr-require-checks.yml, .github/workflows/pr-merge.yml, docs/conventions/pr-pipeline.yaml (the broker reads lanes: and each activity's effect from the tip)
enforcement (19)	a clean security review at the head	babysit_verify_evidence, babysit_check_inputs, checks, classify, dependabot, feedback, gh, postmerge, repo_config, review_trigger, run_cache, state, util, whole_review (.py); .github/scripts/pr-unblock/{address-feedback,gate-source,thread-scope}.mjs; .github/scripts/work-resolve-issue/{report-outcome,limits}.mjs

Basis for the broker-loaded set: ADR 0058 step 3, and the deny table and grant paragraph in docs/conventions/pr-pipeline/pr-run-activity.md ("The lane token broker"). The trusted-actor list stays under its own widening hold (actorsHold), which is unchanged.

Passed security review. babysit_risk.security_review_clean requires all of the following:

• exactly one claude-security-review evidence entry, from the existing binder (babysit_review_evidence.bind);
• bound set, with no unbound reason;
• open_findings == 0 and a CRITICAL count of 0.

The evidence counts open findings and CRITICAL findings, but not IMPORTANT findings on their own. Any open finding therefore holds, SUGGESTION included. That is the fail-closed reading of "no IMPORTANT finding open". A missing, doubled, unbound or malformed entry also holds. A clean claude-review lane alone clears nothing.

Fix

• One classed list per language, pinned equal by test_merge_holds_sync.py.
  • HOLD_PATH_CLASSES in .github/scripts/pr-merge/merge-holds.mjs; HOLD_PATH_FLOOR (the person subset) and REVIEW_FLOOR are derived from it.
  • FLOOR_CLASSES in babysit_risk.py; HIGH_FLOOR, REVIEW_FLOOR and FLOOR are derived from it.
  • A single file cannot serve both languages: ci-status fetches merge-holds.mjs alone from the base commit, and it imports nothing.
  • The sync test still requires the JS and Python import closures to be on the floor (FLOOR). Only their class changed.
  • New: every script in .github/scripts/pr-merge/ must be on the floor as gate.
• Merge gate (babysit_risk._high_path_reasons).
  • An enforcement path scores HIGH as review-path:<glob> unless security_review_clean.
  • The code does not start with high-, so person_reasons (from fix(pr-merge): route non-h38 needs-human holds to bounded retries and a failing status #10249) and enable-auto-merge treat it as a lane's refusal: a comment and no label.
  • NEVER_FLOOR_EXEMPT keeps every floor glob.
• ci-status and enable-auto-merge. pathHold, pathHoldKind and decide() are unedited. They now read the person subset, so an enforcement path reaches the gate's verifier read (evidence --lane-link --lane-reviews, enable-auto-merge.mjs:670), and that read scores it. Every reviews: null comes from a completed score() with nothing holding (babysit_verify_evidence.py lane_risk_refusal). A read that fails returns unreadable: ..., which holds.
• Workflow edits unbind the review. Any .github/workflows/ change is a lane source (babysit_review_evidence.py LANE_SOURCE_PREFIXES; bind() raises lane-source-changed). A PR that also edits a review caller workflow therefore stays held. Tested end to end through bind() and score().
• Additions to the scrutiny lists, a deviation from "do not change": both review callers' hold-paths and ci-status's exempt-author hold_patterns gain the ten new gate pr-merge scripts. Nothing was removed. TheScrutinyListsNameTheFloor requires every floor entry in each list. Group (ii) stays on all three lists.
• An enforcement path no review can bind goes to a person. When the pull request also changes a lane source, the security lane's evidence is unbound for good at that head (lane-source-changed). The path then scores review-path-unbindable:<glob>, a babysit_risk.PERSON_PREFIXES entry, so person_reasons names it and enable-auto-merge labels it human-gated. Any other unbind (review not run yet, a failed read) stays the lane's review-path:. Tests: UnbindableReviewPathTests (2 of 4 fail-first; the 2 lane-side cases guard against widening). The 6 mutants are all killed: prefix dropped, any unbind treated as permanent, never unbindable, always unbindable, any lane's unbind counted, and review-path made a person's.
• Docs: the docs/conventions/pr-pipeline/README.md floor bullet and scrutiny paragraph, and the docs/conventions/pr-pipeline.yaml floor comment, now describe the classes. Changelog fragments for pr-pipeline and source-control.

Verification

Fail-first:

• The new ReviewClearedFloorTests (14 subtests) fail against main's babysit_risk.py.
• The new merge-holds.test.mjs tests fail at import against main's merge-holds.mjs.
• 8 cases in scripts/ci-lanes-pr-hold.test.sh fail against main's module: the gate-check/who holds and the six enforcement paths ci-status now leaves to the gate.

Mutants, all killed by test_babysit_risk + test_merge_holds_sync:

• Each check removed in turn: bound, unbound reason, open == 0, open-is-a-count, critical == 0.
• Lane name widened to any lane; one entry loosened to any entry.
• Clearing forced off; clearing forced on.
• Class swaps: Python enforcement->gate, trust->enforcement and gate->enforcement; JS enforcement->gate and gate->enforcement; the JS person filter removed.
• .github/workflows/ dropped from LANE_SOURCE_PREFIXES.

Suites at the head, which includes #10249 merged from main (re-run after the unbindable change: Python 3084 OK, node 483 pass, ci-lanes-pr-hold.test.sh PASS=157):

• node --test .github/scripts/pr-merge/*.test.mjs: 475 pass, 0 fail.
• babysit-prs Python, unittest discover: 3080 tests, OK.
• scripts/ci-lanes-pr-hold.test.sh: PASS=157 FAIL=0.
• scripts/ci-review-lanes.test.sh: PASS=98.
• scripts/ci-review-plugin-source.test.sh: PASS=11.
• scripts/run-ruff.sh check: clean.
• actionlint: clean on the three workflows.
• scripts/check-line-citations.py: clean.
• The pre-push check-affected: pass.

Related

• fix(pr-merge): route non-h38 needs-human holds to bounded retries and a failing status #10249 (merged before this was pushed): this branch merged it from main, with no conflict. post-unarmed-status.mjs, which fix(pr-merge): route non-h38 needs-human holds to bounded retries and a failing status #10249 added, is on the floor as gate, and person_reasons does not count review-path: (asserted).
• feat(pr-pipeline): stop holding on trusted paths and lane-clearable refusals #9826 (Q-FLOOR (B), Q-ENFORCE (A)): every floor entry stays on the floor and on every scrutiny list. Q26 B changes only who clears group (ii).
• Residual 1, a failed disarm. ci-status no longer holds enforcement paths. If a push lands after arming and the disarm mutation fails (it only warns, pr-require-checks.yml "A push gives an armed pull request..."), the merge queue checks a pr-verify pass at that head, not the security review. The scheduled reader disarms within 15 minutes. A CRITICAL security finding has the same window today.
• Deferred: a test-only floored path exemption, and a person's lift carried across a test-only diff. Not built (owner rule: never weaken a HIGH/CRITICAL boundary). pr-run-activity-write.yml's run job holds id-token: write and runs head test files through run-test-file, so a test file is not inert there. Revisit once head tests no longer run alongside OIDC.
• Owner "go w/ recommendations" on the three follow-ups: the scrutiny-list additions stay, the disarm-failure window is accepted, and a permanent unbind now goes to a person (Fix, last bullet).

🤖 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-26T16:26:27.750993Z	b755dd6	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 fe4abd8f2de693e5a19b640d4fe4155c70d25c9a: 1 finding(s), 0 as inline comments.

Security review of #10298 at fe4abd8. I read all 1321 diff lines and the PR context.

Trust control reviewed: merge-hold narrowing. The change moves 21 floored paths from a person-held HIGH_FLOOR to an LLM-judged review-path: hold. The skill's trust-control list names these as merge holds. It names babysit_verify_evidence.py and the babysit-prs/scripts/ modules it imports, and the pr-unblock/ and work-resolve-issue/ modules the hold scripts import. Narrowing a merge hold is CRITICAL when judged from the code alone. I report one CRITICAL finding, inline on merge-holds.mjs, and set a critical-security hold.

I found no other issues. The following held up under review:

• Class derivation is consistent in JS and Python, and the sync test pins both.
• person_reasons and enable-auto-merge treat review-path: as a lane refusal. It gets a comment and no label.
• security_review_clean fails closed on every shape I traced: missing, doubled, unbound, non-count or negative counts, bool or float counts, and an unread CRITICAL count.
• review-path-unbindable: is added to PERSON_PREFIXES, so a lane-source change sends the PR to a person.
• NEVER_FLOOR_EXEMPT now covers the whole floor, and the scrutiny lists gain entries without losing any.
• The ten new gate scripts raise the bar over main. They join the floor, and pr-review-hosted.yml and pr-review-security-hosted.yml gain their hold-paths.

I did not verify two things because they sit outside the diff: which commit the verifier and gate modules run from, and the claude-security-review lane's own injection resistance.

critical-findings: 0 (the one CRITICAL finding has an inline comment).

No manifest <field> items are hit, because the diff has no plugin.json or marketplace.json changes. It has no instruction-surface deletions. It has no billed-spend change, so there is no cost hold. The extra security-review runs it implies draw only on prepaid credit or subscription usage.

critical-findings: 1

Findings not posted inline (their path, line or side is malformed):

• .github/scripts/pr-merge/merge-holds.mjs:308: Trust control: merge hold narrowed (CRITICAL). HOLD_PATH_FLOOR now drops all enforcement globs, and babysit_risk.HIGH_FLOOR drops the same ones. That removes the person hold from 21 paths. Before this change they all waited for a person:

• babysit_verify_evidence.py, which enable-auto-merge.mjs runs.

• babysit_gh.py, babysit_util.py, babysit_state.py, babysit_checks.py, babysit_classify.py, babysit_feedback.py, babysit_repo_config.py, babysit_run_cache.py and the other babysit-prs modules that babysit_risk.py and babysit_verify_evidence.py import.

• The pr-unblock/* and work-resolve-issue/* modules that merge-holds.mjs and enable-auto-merge.mjs import.

The skill lists these as merge-hold code, and narrowing them is CRITICAL when judged from the code alone.

What now gets through. A pull request that edits one of these modules is cleared by one LLM lane's claude-security-review count. That count needs one bound entry with open_findings == 0. For example, a change to babysit_util.py or babysit_gh.py that quietly changes how the gate reads lane evidence, labels, reviews or changed files has a path to auto-merge. Once merged, that code runs inside every later PR's merge decision. No person ever sees it.

The LLM lane's input is the attacker's own diff and PR description, so it can be prompt-injected or can miss a subtle logic change. The lane's verdict is also the only barrier. ci-status no longer holds these paths either, and the PR's own residual 1 notes the window if a disarm fails.

The code shows no mitigation, such as the person-held gate scripts re-verifying the evidence independently. The PR text presents this as an owner decision (Q26 B). The review is told to judge from code alone, so it needs a person to confirm that decision.

severity: CRITICAL

[github-actions]

claude-security-review holds this pull request for a person (critical-security) at b755dd6ae6aa3b3a31549ebddf11e4c0a0f0b382.

The PR narrows the merge hold on the modules that enforce it. babysit_risk.py, babysit_verify_evidence.py, babysit_review_evidence.py, babysit_state.py, babysit_gh.py and the pr-unblock/ and work-resolve-issue/ modules the hold floor depends on no longer wait for a person. A single claude-security-review pass on the same diff now clears them, as the merge-holds.mjs HOLD_PATH_FLOOR and babysit_risk.py security_review_clean changes show. That is a trust-control weakening and is CRITICAL.

claude-lane: claude-security-review

[github-actions]

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

Code review: no findings

I read the full diff (1321 lines, 15 files) and the PR context. I ran no tests or commands, so every test result below is the author's claim and I have not verified it.

What I checked

• Class lists agree. HOLD_PATH_CLASSES in merge-holds.mjs and FLOOR_CLASSES in babysit_risk.py match entry for entry, in glob, class and order. HOLD_PATH_FLOOR and HIGH_FLOOR both filter out enforcement, and REVIEW_FLOOR is the enforcement subset on both sides. test_merge_holds_sync.py pins the lists equal and the counts at 51 total and 21 enforcement. Counting the lists by hand gives 5 trust, 25 gate and 21 enforcement.
• Clearing condition. security_review_clean fails closed. It requires exactly one claude-security-review entry, bound is True, no unbound reason, and valid zero counts for open and CRITICAL findings. A missing, doubled, unbound or malformed entry holds. The mutation tests cover each of these.
• Unbindable path. review-path-unbindable: is added to PERSON_PREFIXES, so person_reasons sends it to a person. review-path: does not start with high- or any person prefix, so it stays a lane's refusal. UnbindableReviewPathTests checks both directions, including that a lane-source-changed unbind on the code lane alone is not enough.
• Other changes. NEVER_FLOOR_EXEMPT = FLOOR keeps every glob, enforcement ones included. The scrutiny-list additions (two review callers and the ci-status hold_patterns) only add entries. Test updates swap gate-check.mjs for pr-unblock/other.mjs as the "off the floor" example, which is consistent with the new gate class.
• Changelog fragments. Both are bump: minor, which fits an added feature with no removed behaviour.

Observations, not counted as findings

• The PR description says gate (23) and enforcement (19). The code and tests give 25 and 21 (51 entries in total with trust). This affects only the description.
• Several enforcement modules (babysit_util.py, babysit_gh.py, babysit_state.py) are imported by the gate-class scorer and binder. A security review now clears changes to them without a person. The PR records this as an owner decision (Q26 option B) and accepts the disarm-failure window, so I raise nothing on it.
• The docs/conventions/pr-pipeline/README.md paragraph near line 594 reads awkwardly: "(…; the other files that enforce the floor wait for a clean security review instead) waits for a person". It is a wording nit only.

Spend: nothing in this change adds a paid resource or billing setting, so there is no cost hold.

Manifests: the diff changes no plugin.json or marketplace.json.

Closing issue: the PR links no closing issue ("No related issue: owner decision Q26 option B"), so there are no acceptance criteria to check.

[github-actions]

claude-security-review: 1 inline finding(s) on ef87c40c53608f57b16e5930d4861cbbff0c7283; the summary is in this lane's pull request comment.

claude-lane: claude-security-review

[github-actions]

claude-security-review holds this pull request for a person (critical-security) at ef87c40c53608f57b16e5930d4861cbbff0c7283.

Trust-control change: the PR downgrades 21 merge-hold floor entries (babysit_risk.py, babysit_verify_evidence.py, babysit_review_evidence.py and the other babysit-prs modules, the pr-unblock and work-resolve-issue modules) from a person-held high-path: to a review-path: that one clean claude-security-review entry clears, and ci-status stops holding them. The skill's trust-control list names these as CRITICAL when a merge hold is weakened or narrowed.

claude-lane: claude-security-review

[github-actions]

claude-security-review: 1 inline finding(s) on a7a65ed5f0cb092400aab3dcc5b71a8b5ee649a2; the summary is in this lane's pull request comment.

claude-lane: claude-security-review

[github-actions]

claude-security-review holds this pull request for a person (critical-security) at a7a65ed5f0cb092400aab3dcc5b71a8b5ee649a2.

The PR weakens a merge hold, which the review rules treat as CRITICAL. It moves 19 floor paths (babysit_verify_evidence.py, babysit_state/util/gh/checks/classify and the other enforcement modules, the pr-unblock modules, work-resolve-issue/limits.mjs and report-outcome.mjs) from needing a person to needing only an LLM security-review pass with 0 open findings. ci-status no longer holds them (merge-holds.mjs HOLD_PATH_FLOOR filters out enforcement). A change to this code that the review misses merges unattended and then runs as the gate for later PRs, including those touching gate and trust paths.

claude-lane: claude-security-review

[kyle-sexton]

Hold kept: owner stop reason 3 (a CRITICAL security hold).

The owner decided Q-HOLDPATH2 = B on 2026-10-26 at 14:29Z, in the poteto session: "whatever is least friction, human involvement but safe". Under B, a passed security review bound to the current head clears hold-enforcement modules only. Merge-gate code, broker-loaded config and trust boundaries stay held for a person.

claude-security-review CRITICAL-holds this PR because B narrows a merge hold (thread). Only the owner can lift that hold. do-not-merge stays on until the owner either lifts it under B, keeps the enforcement class held for a person, or closes this PR.

Refreshed onto main at fe4abd8. The PR no longer waits on #10252. That refresh added two enforcement-class files to the 19 already listed, the two retry-fetch.mjs copies, so the class now holds 21. It also classed rate-limits.json and verify-answers.mjs as gate.

[github-actions]

claude-security-review holds this pull request for a person (critical-security) at fe4abd8f2de693e5a19b640d4fe4155c70d25c9a.

The change narrows merge holds. 21 floored paths move from a person hold to a clean claude-security-review clearance: babysit_verify_evidence.py and the babysit-prs modules it and babysit_risk.py import, plus the pr-unblock/ and work-resolve-issue/ modules that merge-holds.mjs and enable-auto-merge.mjs import (see .github/scripts/pr-merge/merge-holds.mjs HOLD_PATH_FLOOR and babysit_risk.py FLOOR_CLASSES). The skill treats narrowing a merge hold as a CRITICAL trust-control change.

claude-lane: claude-security-review