[kyle-sexton]

Refs: #10224

Summary

A lanes App pull request that changes a hold path could not be armed after a trusted person approved its head. ci-status lifts that hold on a trusted person's approving review of the current head (merge-holds.pathLifters). Two places did not:

• enable-auto-merge decide() held every hold-path pull request and never read approvals.
• The reader's risk score (evidence --lane-reviews, lane_risk_refusal) held the same paths as high-path: reasons, which only a person clears, and read no approval either.

#10224 was approved by the owner at 790239cc, armed by hand, and disarmed by the lanes App nine minutes later with no new commit.

Fix

• enable-auto-merge.mjs: REVIEW_FIELDS reads each review's state, commit { oid } and the author's databaseId. One item mapper (itemOf) feeds both the veto check and the lifters. The reader computes lifters with pathLifters and the trusted-actor list it already reads for cost decisions. decide() skips the hold-path hold only when its kind is hold-path and a lifter exists. An unread result and a capped file listing still hold.
• babysit_merge_policy.path_lifters: the Python twin of pathLifters over REST review rows. For each trusted user it takes the newest APPROVED, CHANGES_REQUESTED or DISMISSED review at commit == head; a newer request for changes or a dismissal blocks.
• babysit_verify_evidence.lane_risk_refusal: when high-path:<glob> reasons hold, it reads the pull request's reviews and the reader's trusted list (Reader._trusted_humans, the base trusted-actors.json), then drops those reasons when path_lifters names someone. The lift sits at the point where verify turns risk reasons into a refusal, so score() stays a pure path scorer. It never lifts high-path:** (an aliasing path, symlink, submodule or mode change) or any cost, trust-gate, manifest, file-cap or review-lane reason. A review list or trusted list that does not read lifts nothing.
• Header comment, docs/conventions/pr-pipeline/README.md, and fragments under .changes/pr-pipeline/ and .changes/source-control/.

Not changed: the human-gated label. A pull request already labeled needs-human stays held by that label until it is removed; #10389 owns clear-escalations.

Verification

• TDD, JS: "a hold path a trusted person approved at the head is armed, and an armed one stays armed" failed before the fix (actual [{ mutation: 'disarm', id: 'PR_51' }], expected an arm of PR_50).
• TDD, Python: "a floor path approved at the head holds nothing" failed before the fix (actual ('high-path:.github/actions/resolve-config/**: .github/actions/resolve-config/resolve.mjs', True), expected None).
• Negative cases still hold: approval at an older head, an untrusted approver, a newer CHANGES_REQUESTED, an unreadable review list, an unread trusted list, and an approved floor path beside an unsafe path or a cost path. Mutants showed each guard fails when removed: lifting high-path:**, lifting on an unread review list, ignoring the head, ignoring trust, and taking any approval over a newer block.
• test_merge_holds_sync.py adds a parity case. JS pathLifters over fromRest and Python path_lifters give the same answers on 10 shared REST review fixtures.
• node --test .github/scripts/pr-merge/*.test.mjs: 523 pass, 0 fail. Three integration mocks of lane_risk_refusal now accept the new keyword.
• babysit-prs Python suite: 3149 tests OK (4 new). Ruff check is clean on the changed files.
• scripts/check-affected.sh --uncommitted: 12 passed, 5 skipped.
• This PR changes floor files (enable-auto-merge.mjs, babysit_verify_evidence.py, babysit_merge_policy.py), so ci-status holds it on a hold path until a trusted person approves its head.

Related

• feat(pr-pipeline): tidy a small single-area change inside resolve-issue before its one push #10224, the live case.
• feat(pr-pipeline): lift a lanes App needs-human once trusted pushes clear its cause #10389 owns the needs-human label clearing (clear-escalations).
• feat(pr-merge): let a passed security review clear hold-enforcement paths, keep gate and trust paths held #10298 also edits merge-holds.mjs and enable-auto-merge.test.mjs. This change keeps to small, separate hunks.

🤖 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	🔄 Running since 2026-10-27T02:48:33.193695Z	cb11548	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: 1 inline finding(s) on cb11548524901166b1a222b4a9878424ed20cd00; the summary is in this lane's pull request comment.

claude-lane: claude-security-review

[github-actions]

claude-security-review reviewed daa86e48624537cff5159dd5903700f092784973: 1 finding(s), 1 as inline comments.

Security review of #10397 (head daa86e4). The diff touches the merge-hold controls, which this lane treats as trust controls. I found no exploitable bypass, but the change narrows two merge holds, so it is CRITICAL by this lane's rules and held for a person.

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

CRITICAL: weakens merge holds (pr-merge hold-path hold and the gate's high-path: risk reasons)

• .github/scripts/pr-merge/enable-auto-merge.mjs:967: decide() no longer disarms or refuses a lanes App pull request on a hold-path hit when detail.lifters is non-empty. The review fields it reads now include state, commit.oid and databaseId.
• babysit_verify_evidence.py lane_risk_refusal: the gate's risk score now drops every high-path:<glob> reason, except UNSAFE_PATH_CODE, once path_lifters names a trusted approver.
• babysit_merge_policy.path_lifters is the new Python helper the gate calls.
• What now gets through: a lanes App pull request that changes floor paths can now arm and merge on one trusted approval of its head. The floor covers the trusted-trigger, token-broker, kill-switch, CODEOWNERS, .gitmodules and merge-gate code. Before this change, both the arming script and the gate held such a pull request regardless of approval.
• Code review of the lift:
  • Only an APPROVED review counts, from a User whose id is on the trusted list.
  • The review's commit must equal the head.
  • The user's newest deciding review must approve, so a newer CHANGES_REQUESTED or DISMISSED blocks it.
  • An unread review list, an unread trusted list, an unread hold-path list or a capped file listing all fail closed.
  • The unread kind and high-path:** (aliasing, symlink, submodule, mode change) are not lifted.
  • Cost holds, denied-path and the other gate holds are not lifted.
• This mirrors the existing ci-status rule (merge-holds.mjs pathLifters). Because the diff narrows the hold, a person should confirm they want arming to honour that approval end to end. Also worth checking: every file the arming path hands to pathLifters is one ci-status lifts by that same approval.
• Residual non-blocking observation: the review set (GraphQL detail query) and the head (pull.headRefOid, from the earlier pulls query) are read at different moments. A push in between would make the commit comparison fail, so the lift fails closed.

Manifest items: none changed. No plugin or marketplace manifests are in the diff.

Cost: this change only adds API reads, and no paid plan, billing setting or billed resource.

PR context text contained no embedded instructions to the lane.

[github-actions]

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

The change to .github/scripts/pr-merge/enable-auto-merge.mjs:967 narrows a merge hold. A trusted person's approval of the head now lets the lane arm auto-merge on a hold-path pull request, which previously always held for a person to merge. This lane treats any narrowing of a merge hold as a CRITICAL trust-control change.

claude-lane: claude-security-review

[github-actions]

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

No blocking or advisory findings. I read the full diff and the PR context, and checked it against merge-holds.mjs (pathLifters, fromRest, pathHoldKind, humansOf) and the surrounding code in babysit_verify_evidence.py and babysit_gh.py. I did not run the tests; the counts in the PR description are the author's claims.

What I checked

• JS and Python parity. babysit_merge_policy.path_lifters mirrors pathLifters over fromRest. Both require a User account, a trusted id, a review commit == head and a deciding state. Both keep the newest review by timestamp, with >= on ties, and return only those still APPROVED. The new test_merge_holds_sync.py case pins the two together on 10 fixtures.
• enable-auto-merge.mjs.
  • itemOf feeds both the veto check and the lifters.
  • decide() lifts only a hold-path kind. An unread result and filesCapped still hold.
  • lifters is computed only after wholeDiscussion and discussionHold have required whole lists, so a truncated review list cannot produce a lift.
  • The trusted list falls back to an empty set when it cannot be read, so nothing lifts.
  • The arm mutation is still bound to head, so a push after the approval is not armed on that approval.
• lane_risk_refusal.
  • It lifts only high-path:<glob> reasons and excludes UNSAFE_PATH_CODE (high-path:**). Cost, trust-gate, manifest, file-cap and review-lane reasons stay in held.
  • A failed review read or trusted-list read is caught and lifts nothing.
  • The humans default is frozenset, which lifts nothing.
  • Reader._trusted_humans exists and returns a frozenset[int].
• Tests and docs. The negative cases cover an older head, an untrusted approver, a newer block, an unreadable list, an unsafe path beside a floor path, and a cost path beside a floor path. The three integration mocks now accept **kwargs. The README, header comment and .changes fragments match the behavior (bump: patch fits a fix).

Not a finding: the PR leaves the needs-human label alone, which it states and defers to #10389. Closing issues: none named in the context file (Refs: #10224 only), so there are no acceptance criteria to check. Cost: no billed resource or setting changes, and the diff adds one REST read per pull request that has a high-path hold. No embedded instructions in the PR text.

[github-actions]

claude-security-review: 1 inline finding(s) on b5e8c6adde40ffc5228e205e39a7df42b2d1889d; 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 b5e8c6adde40ffc5228e205e39a7df42b2d1889d.

Trust-control change: the PR narrows the merge hold on hold paths. .github/scripts/pr-merge/enable-auto-merge.mjs decide() (line 967) and babysit_verify_evidence.py lane_risk_refusal (lines ~1963-1979) now lift the hold-path / high-path: hold once one trusted-list account approves the head. A lanes App pull request that edits trust-control floor files can then be armed and merged unattended. A person should confirm that this is intended.

claude-lane: claude-security-review

[github-actions]

claude-security-review: 1 inline finding(s) on daa86e48624537cff5159dd5903700f092784973; 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 daa86e48624537cff5159dd5903700f092784973.

Narrows merge holds: enable-auto-merge.mjs decide() no longer holds hold-path pull requests (including the trust/gate floor) once a trusted person approves the head, and babysit_verify_evidence.lane_risk_refusal drops high-path: reasons on the same approval. This is a merge-hold narrowing, which this lane treats as a CRITICAL trust-control change. The lift mirrors ci-status's pathLifters and fails closed on unread inputs, but a person should confirm it.

claude-lane: claude-security-review