[kyle-sexton]

Refs: #9276

Summary

The pr-merge clear-escalations sweep can take needs-human off a pull request while a review lane is holding it. It now keeps the label whenever a review-lane hold comment was written or edited since the label was last removed. This is PR 2 of the #9276 plan: once the review lanes apply needs-human themselves, the escalation sweep must not lift that hold.

Fix

• Today the sweep treats bot comments as silence and reads markers only on lanes App comments. A review lane adding a label the PR already carries writes no labeled event, so its hold can sit under the App's own label, and a clearing pr-unblock or pr-merge escalation removed it.
• verdict() now checks, right after the labeled-actor check and before both the pr-unblock path and the pr-merge head-moved path, for a review-lane hold: a comment with a line opening the claude-lane-hold lane= HTML-comment marker, authored by claude[bot], or by github-actions[bot] when its last non-blank line is the claude-lane: <lane> footer. That is the same lane-comment rule as feat(pr-pipeline): run the security review on pr-review-cli with key-first funding #9307's address-feedback.mjs (LANE_FOOTER). It is matched by login plus type: Bot, because timeline items carry no author id field.
• The hold counts when its created or updated time is not before the label's newest unlabeled event (any time, when there is none). A person removing the label is the approval, so an older hold does not block a later escalation, but a hold the lane edits afterwards does. A tie or an unparsable time keeps the label.
• The script's header rule list states the new condition.

Verification

• node --test .github/scripts/pr-merge/clear-escalations.test.mjs: 34 pass, 0 fail. The 6 new tests were written first: 4 failed before the guard and the 2 control cases passed. They cover:
  • a github-actions[bot] (with footer) or claude[bot] hold surviving a pr-unblock escalation that would otherwise clear
  • a hold written before the App's label with no unlabel since
  • the pr-merge head-moved path also being kept
  • a hold older than a person's unlabel not blocking
  • a hold edited after the unlabel still keeping the label
  • a footerless github-actions[bot] marker or another bot's marker not counting
• The existing first test still shows a plain pr-unblock escalation clearing.
• npm test in .github/scripts/pr-merge: 163 pass, 0 fail.
• scripts/check-affected.sh --tests: 9 passed, 5 skipped.
• Left out: a live sweep against GitHub. The plan's sandbox canary for the clear-escalations guard (PR 5) covers it once ci-workflows emits the hold comment (PR 3).

Related

• feat(pr-pipeline): fail ci-status while a pull request carries needs-human #9312 (PR 1 of the plan): ci-status fails while needs-human is on.
• feat(pr-pipeline): run the security review on pr-review-cli with key-first funding #9307: source of the claude-lane: footer rule. Follow-up once it merges: share one lane-comment predicate instead of the copied LANE_FOOTER.

🤖 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-23T04:43:37.364711Z	b3ed385	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 b3ed385187255d6bd05800c13223c30155c35602: 0 finding(s), 0 as inline comments.

No security findings in this pull request. I read the diff through gh pr diff, but not the surrounding files.

The change makes clear-escalations keep needs-human when a review-lane hold comment exists. That moves the sweep in the safe direction, because it removes the label less often. I traced the paths an attacker could use:

• Spoofing the hold: a hold counts only when the author's login and type: Bot match claude[bot]. It also counts for github-actions[bot], but only when the last non-blank line is the lane footer. A human or an outside contributor cannot forge those identities. A workflow in this repository that posts as github-actions[bot] could write a hold, but that only keeps the label, which fails safe.
• Unparsable times: a comment time that does not parse gives NaN, and the comparison with NaN is false, so the hold still counts. A bad time can only keep the label.
• Unlabel handling: the code ignores an unlabeled event whose time is NaN, so it cannot lift a hold. A hold is lifted by an unlabeled event at or before the label's last labeled time, whoever caused it. Anyone who can remove the label can already do that, so it gives no new access. The PR describes this as a person's approval.
• Marker handling: the hold reason is cut to 120 characters in the log. I saw no injection or secret exposure from that.
• Ordering: the new check runs after the labeled-actor check and before the pr-unblock and pr-merge paths. It can only add an early "kept" exit and cannot reach a delete.
• Instruction-surface deletions: the diff removes no standing instruction or guardrail. It only adds a rule to the script header.
• Plugin manifests: the diff touches no manifests, so there are no manifest <field> lines.

The register file docs/conventions/instruction-exception-register/README.md was not consulted. It was not needed, since nothing was deleted or softened.

critical-findings: 0

[github-actions]

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

No findings.

I read the full diff (clear-escalations.mjs and its test file) and the PR context, then checked the new guard against the surrounding code and the test helpers.

• Hold window: lifted is the newest unlabeled event at or before the label's since. A hold counts unless both its creation and its last edit come before lifted. With no unlabel, lifted is -Infinity, so any hold counts. A tie or an unparsable time keeps the label, as the PR states.
• Lane identity: claude[bot] is matched by login plus type: Bot. github-actions[bot] also needs the claude-lane: <lane> footer as its last non-blank line. The footer regex is anchored, the lines() helper strips \r, and findLast is available in the supported Node versions.
• Placement: the check runs after the labeled-actor check and before both the pr-unblock and pr-merge head-moved paths. The new tests cover both paths.
• Test helpers: commented(user, body, at, updatedAt), ACTIONS_BOT, CLAUDE_BOT and CODEX_BOT exist in the test file with the shapes the new tests use (clear-escalations.test.mjs:17-29). I did not run the tests myself. The pass counts in the PR body are author-claimed and unverified by me.
• Closing issue: the PR uses Refs: #9276, so there is no closing issue and no acceptance criteria to check.
• Manifests and rules: the diff touches no plugin manifests. No .claude/rules/ rule I checked matches the changed files.

Minor observation, not a finding: any claude[bot] comment with a line that starts with the marker, for example inside a code fence in a review that discusses this feature, would count as a hold. The probability looks low, and a person unlabeling lifts it.