From dfd37e085e76d1073932feba07d17c110b47f226 Mon Sep 17 00:00:00 2001
From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com>
Date: Fri, 23 Oct 2026 00:34:47 -0400
Subject: [PATCH] fix(pr-pipeline): keep needs-human a review lane holds in
 clear-escalations

clear-escalations treats bot comments as silence and reads markers only
on lanes App comments, so a review lane's hold that sat under the App's
own needs-human label was cleared with the escalation. The sweep now
keeps the label while a review-lane hold comment (a line opening
`<!-- claude-lane-hold lane=`, by claude[bot] or by github-actions[bot]
ending in the `claude-lane:` footer) was written or edited since the
label was last removed. A person unlabeling lifts the hold, so an older
hold no longer blocks a later escalation.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 .../scripts/pr-merge/clear-escalations.mjs    | 36 +++++++++++
 .../pr-merge/clear-escalations.test.mjs       | 64 +++++++++++++++++++
 2 files changed, 100 insertions(+)

diff --git a/.github/scripts/pr-merge/clear-escalations.mjs b/.github/scripts/pr-merge/clear-escalations.mjs
index f1b04e9758..b36a852b18 100755
--- a/.github/scripts/pr-merge/clear-escalations.mjs
+++ b/.github/scripts/pr-merge/clear-escalations.mjs
@@ -8,6 +8,12 @@
 //   App bot as its actor, and the App's first escalation comment after it
 //   came within PAIRED_MS (report-escalation.sh writes the label, then the
 //   comment).
+// - No review lane holds it: no comment carrying a REVIEW_HOLD marker line, by
+//   claude[bot] or by github-actions[bot] ending in the lane's `claude-lane:`
+//   footer, was written or edited since the label was last removed (ever,
+//   when it never was). A lane adding a label the PR already carries writes
+//   no `labeled` event, so its hold can sit under the App's label, and a
+//   person removing the label is what lifts it.
 // - Every escalation comment since that label event is the App's: its author
 //   is the bot login and a Bot account, its first line is the pr-unblock
 //   escalation marker, its second the resume-key line, and its `- Reason:`
@@ -75,6 +81,11 @@ const PR_MERGE_MARKERS = Object.freeze([
   "<!-- babysit-merge-shared-copy-escalation -->",
 ]);
 const AT_HEAD = / at head `([^`]*)`/g;
+// A review lane's hold comment opens a line with this marker. The lanes post
+// as claude[bot], or as github-actions[bot] with the lane's footer as the last
+// non-blank line (address-feedback.mjs's lane comment rule).
+const REVIEW_HOLD = "<!-- claude-lane-hold lane=";
+const LANE_FOOTER = /^`?claude-lane: (?:claude-review|claude-security-review)`?$/;
 const HEAD_MOVED = "head-moved";
 const REPO = /^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/;
 const SHA = /^[0-9a-f]{40}$/;
@@ -94,6 +105,16 @@ const lines = (body) => String(body ?? "").split("\n").map((line) => line.replac
 const same = (a, b) => String(a ?? "").toLowerCase() === String(b).toLowerCase();
 const isApp = (user, bot) => user?.login === bot && user?.type === "Bot";
 
+// The REVIEW_HOLD marker line of a review lane's hold comment, or undefined.
+function laneHold(item) {
+  const body = lines(item.body).map((line) => line.trim());
+  const marker = body.find((line) => line.startsWith(REVIEW_HOLD));
+  const lane =
+    isApp(item.user, "claude[bot]") ||
+    (isApp(item.user, "github-actions[bot]") && LANE_FOOTER.test(body.findLast((line) => line !== "") ?? ""));
+  return item.event === "commented" && lane ? marker : undefined;
+}
+
 // Why each reason no longer holds at `pull`'s head, or null while it holds.
 const CHECKS = Object.freeze({
   "workflow-staged": async ({ github, repo, pull, mainSha }) => {
@@ -183,6 +204,21 @@ async function verdict({ github, repo, number, label, bot, mainSha }) {
     return { reason: `${label} was last added by ${labeled?.actor?.login ?? "no one on the timeline"}, not the lanes App` };
   }
   const since = Date.parse(labeled.created_at);
+  // A comment's writes are its creation and its last edit; a tie, or a time
+  // that does not parse, keeps the label.
+  const lifted = Math.max(
+    -Infinity,
+    ...timeline
+      .filter((item) => item.event === "unlabeled" && same(item.label?.name, label))
+      .map((item) => Date.parse(item.created_at))
+      .filter((time) => time <= since),
+  );
+  const held = timeline.find(
+    (item) => laneHold(item) !== undefined && !(Math.max(Date.parse(item.created_at), Date.parse(item.updated_at ?? item.created_at)) < lifted),
+  );
+  if (held !== undefined) {
+    return { reason: `a review lane holds it: ${laneHold(held).slice(0, 120)}` };
+  }
   // Every escalation since the label went on: a later one posts no new label
   // event, so each reason it recorded must have cleared.
   const appComments = timeline.filter((item) => item.event === "commented" && isApp(item.user, bot));
diff --git a/.github/scripts/pr-merge/clear-escalations.test.mjs b/.github/scripts/pr-merge/clear-escalations.test.mjs
index 7a4c1af9c5..dabace88da 100644
--- a/.github/scripts/pr-merge/clear-escalations.test.mjs
+++ b/.github/scripts/pr-merge/clear-escalations.test.mjs
@@ -361,3 +361,67 @@ test("a feedback-protected-path label comes off once the PR changes nothing unde
     assert.equal(w.writes().at(-1)?.method === "DELETE", clears, pullFiles[0].filename);
   }
 });
+
+// A review lane's hold: its marker comment, by claude[bot], or by
+// github-actions[bot] when the comment ends in the lane's footer (the
+// pr-review-cli shape). Adding a label the PR already carries writes no
+// `labeled` event, so the hold may sit under the App's own label.
+const HOLD = "<!-- claude-lane-hold lane=claude-security-review sha=" + HEAD + " criteria=trust-gate -->";
+const laneHold = (user, at, { footer = true, updatedAt = at } = {}) =>
+  commented(user, `${HOLD}\nChanges a trust gate.${footer ? "\n\n`claude-lane: claude-security-review`\n" : ""}`, at, updatedAt);
+const unlabeled = (actor, at) => ({ event: "unlabeled", actor, label: { name: "needs-human" }, created_at: at });
+
+test("a review-lane hold keeps the label a clearing pr-unblock escalation would take off", async () => {
+  for (const hold of [laneHold(ACTIONS_BOT, "2026-10-18T11:00:00Z"), laneHold(CLAUDE_BOT, "2026-10-18T11:00:00Z")]) {
+    const w = world({ timeline: [labeled(BOT, "2026-10-18T10:00:00Z"), escalated(), hold] });
+    const { logs } = await run(w);
+    assert.deepEqual(w.writes(), [], hold.user.login);
+    assert.match(logs.join("\n"), /#7 kept: a review lane holds it/, hold.user.login);
+  }
+});
+
+test("a review-lane hold written before the App's label, with no unlabel since, keeps it", async () => {
+  const w = world({ timeline: [laneHold(ACTIONS_BOT, "2026-10-18T09:00:00Z"), labeled(BOT, "2026-10-18T10:00:00Z"), escalated()] });
+  await run(w);
+  assert.deepEqual(w.writes(), []);
+});
+
+test("a review-lane hold keeps a pr-merge head-moved escalation too", async () => {
+  const w = world({ timeline: [...mergeEscalated(verifyBody(OLD)), laneHold(ACTIONS_BOT, "2026-10-18T11:00:00Z")] });
+  await run(w);
+  assert.deepEqual(w.writes(), []);
+});
+
+test("a review-lane hold a person approved by unlabeling no longer keeps a later escalation", async () => {
+  const w = world({
+    timeline: [
+      laneHold(ACTIONS_BOT, "2026-10-18T08:00:00Z"),
+      unlabeled(OWNER, "2026-10-18T09:00:00Z"),
+      labeled(BOT, "2026-10-18T10:00:00Z"),
+      escalated(),
+    ],
+  });
+  await run(w);
+  assert.equal(w.writes().at(-1)?.method, "DELETE");
+});
+
+test("a review-lane hold edited after the unlabel keeps it", async () => {
+  const w = world({
+    timeline: [
+      laneHold(ACTIONS_BOT, "2026-10-18T08:00:00Z", { updatedAt: "2026-10-18T09:30:00Z" }),
+      unlabeled(OWNER, "2026-10-18T09:00:00Z"),
+      labeled(BOT, "2026-10-18T10:00:00Z"),
+      escalated(),
+    ],
+  });
+  await run(w);
+  assert.deepEqual(w.writes(), []);
+});
+
+test("a hold marker from github-actions[bot] with no lane footer, or from another bot, is not a review-lane hold", async () => {
+  for (const hold of [laneHold(ACTIONS_BOT, "2026-10-18T11:00:00Z", { footer: false }), laneHold(CODEX_BOT, "2026-10-18T11:00:00Z")]) {
+    const w = world({ timeline: [labeled(BOT, "2026-10-18T10:00:00Z"), escalated(), hold] });
+    await run(w);
+    assert.equal(w.writes().at(-1)?.method, "DELETE", hold.user.login);
+  }
+});