fix(pr-pipeline): honor Hold: lines and lane escalations on owner PRs in address-feedback

.changes/pr-pipeline/fix-feedback-dispatch-owner-pr-holds-4b8e2c61.md

@@ -0,0 +1,8 @@
+---
+bump: patch
+---
+
+### Fixed
+
+- On an adopted owner PR, the `address-feedback` select job skips `awaiting-human` when the body gained a `Hold:` line after the round was dispatched, so a hold the owner adds while the round is queued stops it before any model turn. Lanes App PRs, whose `Hold: a human merges this` holds only the merge, are unchanged.
+- On an adopted owner PR, a lane escalation's human-gated label lets an `address-feedback` round dispatch and run only as it lets fix-ci work the PR (`gate-source.mjs` `prHold`): it holds after an `address-feedback` escalation, or once a person commented or reviewed after the label went on, and then only a thread a trusted person with write access asked the lane to fix is worked. On a lanes App PR a lane escalation's label still lets the round work, as before.

.github/scripts/pr-unblock/address-feedback.mjs

@@ -25,7 +25,10 @@
 //           human-gated label it keeps only threads a trusted person with
 //           write access asked it to fix (isFixRequest), and skips
 //           awaiting-human when there are none, unless a lane escalation put
-//           the label on and no review lane holds the PR (gate-source.mjs).
+//           the label on and no review lane holds the PR (gate-source.mjs),
+//           and on an adopted owner PR only as prHold lets fix-ci work it.
+//           On an adopted owner PR a body line starting `Hold:` skips
+//           awaiting-human.
 //
 // Every comment body is copied into the thread file and never logged or
 // interpolated into a command. Only a trusted person's reply to the lane's
@@ -37,7 +40,7 @@ import { appendFileSync, lstatSync, readdirSync, readFileSync, writeFileSync } f
 import { join } from "node:path";
 import { pathToFileURL } from "node:url";
 
-import { commentItems, gateSource, laneHold, listLabelEvents, ROUND_MARKER } from "./gate-source.mjs";
+import { commentItems, gateSource, laneHold, listLabelEvents, prHold, ROUND_MARKER } from "./gate-source.mjs";
 import { retryFetch } from "./retry-fetch.mjs";
 import { isProtectedPath, namedInScope, prPathsOf } from "./thread-scope.mjs";
 import { retryRung, stopSkip } from "./escalation-routes.mjs";
@@ -53,6 +56,8 @@ export const FRESH_FIXER_ROUNDS = 1;
 // How the round marker and the neutral checks show the count.
 export const roundLine = (round, max) => `round ${round} of max ${max}`;
 export { ROUND_MARKER };
+// The activity select decides, as escalation-routes.json names it.
+const ACTIVITY = "address-feedback";
 // github-actions[bot], the author of the review lanes' reviewed-head markers.
 export const MARKER_AUTHOR_ID = 41898282;
 const REVIEWED_HEAD =
@@ -610,19 +615,37 @@ export async function select({
     api.listReviewComments(pr),
     api.listThreads(pr),
   ]);
+  // An owner PR whose body gained a `Hold:` line after dispatch is held, as
+  // feedback-dispatch.mjs ownerSkip holds it at dispatch.
+  const owner = ownerAuthored(pull, adoption);
+  if (owner && HOLD_LINE.test(String(pull.body ?? ""))) {
+    return { decision: "skip", skipReason: "awaiting-human", threads: [] };
+  }
   // The human-gated label holds the PR except for threads a trusted person
   // with write access asked the lane to fix, unless a lane escalation put it
-  // on and no review lane holds the PR (gate-source.mjs). The label stays on.
+  // on and no review lane holds the PR (gate-source.mjs). On an owner PR the
+  // lane escalation lets the round work only as it lets fix-ci work there
+  // (prHold): not after address-feedback's own escalation, nor once a person
+  // spoke after the label went on. The label stays on.
   const labeled = (pull.labels ?? []).some((label) => label.name === humanGatedLabel);
-  const escalated =
+  const timeline = async () => [...commentItems(issueComments), ...(await api.listLabelEvents(pr))];
+  const isLanesApp = (user) => user?.id === appId && user?.type === "Bot";
+  const gated =
     labeled &&
-    gateSource({
-      timeline: [...commentItems(issueComments), ...(await api.listLabelEvents(pr))],
-      label: humanGatedLabel,
-      head: headSha,
-      isLanesApp: (user) => user?.id === appId && user?.type === "Bot",
-    }).source === "lane-escalation";
-  const gated = labeled && !escalated;
+    (owner
+      ? (
+          await prHold({
+            labels: pull.labels.map((label) => label.name),
+            holdLabels: [],
+            gateLabel: humanGatedLabel,
+            head: headSha,
+            activity: ACTIVITY,
+            appId,
+            readTimeline: timeline,
+            readReviews: async () => ({ comments: reviewComments, reviews }),
+          })
+        ).held
+      : gateSource({ timeline: await timeline(), label: humanGatedLabel, head: headSha, isLanesApp }).source !== "lane-escalation");
   const fixers = await reviewFixers({ reviewComments, trustedHumans, permission: api.getPermission });
   const heads = reviewedHeads(issueComments);
   const kept = keepThreads({
@@ -657,7 +680,7 @@ export async function select({
     // is the owner's (one they wrote, answered, or a lane answer the lanes App
     // cannot resolve there), and so no stop: it still holds the merge until it
     // is resolved. Any other person's PR still escalates.
-    if (untaken && ownerAuthored(pull, adoption)) {
+    if (untaken && owner) {
       return { decision: "skip", skipReason: "not-applicable", threads: kept };
     }
     return untaken
@@ -728,7 +751,7 @@ export async function select({
     maxRounds: bound,
     // True drops the human-gated label from the write runner's holds: every
     // kept thread carries a write-access fix request, or a lane escalation
-    // put the label on.
+    // that lets this activity work put the label on.
     fixRequested: labeled,
     threads: worked,
     file: { version: 1, repository, pr, head_sha: headSha, pr_paths: prPathsOf(files), threads: worked },
@@ -1109,6 +1132,12 @@ export const NO_ADOPTION = Object.freeze({ ownerIds: Object.freeze(new Set()), i
 export const ownerAuthored = (pull, adoption = NO_ADOPTION) =>
   adoption.idleHours !== undefined && pull.user?.type === "User" && adoption.ownerIds.has(pull.user?.id);
 
+// A body line starting "Hold:", as babysit's BODY_MERGE_HOLD_RE reads it
+// (babysit_merge.py): any case, after any non-newline whitespace, NBSP
+// included, with a bare CR as a line break too. On an owner PR it holds the
+// lane off: feedback-dispatch.mjs ownerSkip, select, fix-ci-dispatch.mjs.
+export const HOLD_LINE = /(?:^|(?<=[\r\n]))[^\S\r\n]*hold:/i;
+
 // The owners whose PRs a round serves: the base trusted-actors list's people,
 // under the base config's `adoption` section. An unreadable section adopts no
 // owner, and lanes App PRs proceed.

.github/scripts/pr-unblock/address-feedback.test.mjs

@@ -46,7 +46,7 @@ function fakeApi(overrides = {}) {
     posted,
     permissionReads,
     // The lanes App opened the PR unless the case names another opener.
-    getPull: async () => ({ number: 42, user: overrides.opener ? { id: overrides.opener, type: "User" } : { id: APP, type: "Bot" }, labels: (overrides.labels ?? []).map((name) => ({ name })) }),
+    getPull: async () => ({ number: 42, user: overrides.opener ? { id: overrides.opener, type: "User" } : { id: APP, type: "Bot" }, labels: (overrides.labels ?? []).map((name) => ({ name })), body: overrides.body ?? "" }),
     async getPermission(login) {
       permissionReads.push(login);
       return (overrides.permissions ?? { "kyle-sexton": "write" })[login] ?? "none";
@@ -1639,6 +1639,52 @@ test("on an adopted owner PR an untaken thread skips with no stop; elsewhere it
   assert.equal((await run(fakeApi({ ...threads, opener: HUMAN }))).escalationReason, "feedback-untaken-threads", "an owner PR with no adoption section");
 });
 
+// A `Hold:` body line an owner adds after dispatch holds the round as
+// feedback-dispatch.mjs ownerSkip holds it at dispatch; a lanes App PR's own
+// `Hold: a human merges this` holds only its merge.
+test("on an adopted owner PR a Hold: body line added after dispatch skips awaiting-human before any thread is kept", async () => {
+  const adoption = { ownerIds: new Set([HUMAN]), idleHours: 4 };
+  const adopted = (api) => select({ api, pr: 42, headSha: HEAD, repository: "o/r", appId: APP, reviewerId: REVIEWER, trustedHumans: new Set([HUMAN]), adoption });
+  const held = await adopted(fakeApi({ opener: HUMAN, body: "Closes #1\r\n  HOLD: still on this" }));
+  assert.deepEqual(held, { decision: "skip", skipReason: "awaiting-human", threads: [] });
+  assert.equal((await adopted(fakeApi({ opener: HUMAN, body: "Closes #1\n\nNo hold here." }))).decision, "proceed", "an owner PR with no Hold: line");
+  assert.equal((await adopted(fakeApi({ body: "Closes #1\n\nHold: a human merges this" }))).decision, "proceed", "a lanes App PR's Hold: line");
+});
+
+// On an adopted owner PR a lane escalation's needs-human lets the round work
+// only as it lets fix-ci work there (gate-source.mjs prHold): another lane's
+// escalation with no person speaking since the label does; address-feedback's
+// own escalation, or a person's review comment since it, holds the PR, and
+// then only a write-access fix request is kept. A lanes App PR reads the
+// escalation through gateSource alone, as before. Every author is typed as the
+// REST API returns it, so a bot is no person speaking.
+test("on an adopted owner PR a lane escalation's needs-human lets the round work only as fix-ci's prHold does", async () => {
+  const adoption = { ownerIds: new Set([HUMAN]), idleHours: 4 };
+  const adopted = (api) => select({ api, pr: 42, headSha: HEAD, repository: "o/r", appId: APP, reviewerId: REVIEWER, trustedHumans: new Set([HUMAN]), adoption });
+  const escalation = (reason) => ({ ...LANE_ESCALATION, body: `${LANE_ESCALATION.body}\n\n- Reason: \`${reason}\`` });
+  const bot = { id: REVIEWER, login: LOGINS[REVIEWER], type: "Bot" };
+  const gate = (reason, extra = []) => ({
+    labels: ["needs-human"],
+    issueComments: [{ ...reviewedHead, user: { id: ACTIONS_BOT, type: "Bot" } }, escalation(reason)],
+    labelEvents: APP_LABELED,
+    reviewComments: [comment(100, REVIEWER, { user: bot, created_at: "2026-10-18T09:00:00Z", updated_at: "2026-10-18T09:00:00Z" }), ...extra],
+    reviews: [{ id: 10, user: bot, commit_id: HEAD, submitted_at: "2026-10-18T09:00:00Z" }],
+  });
+  const other = await adopted(fakeApi({ ...gate("ci-fix-round-cap"), opener: HUMAN }));
+  assert.equal(other.decision, "proceed", "another lane's escalation, no person since");
+  assert.equal(other.fixRequested, true);
+  const own = await adopted(fakeApi({ ...gate("feedback-round-cap"), opener: HUMAN }));
+  assert.deepEqual([own.decision, own.skipReason], ["skip", "awaiting-human"], "address-feedback's own escalation");
+  const spoke = comment(101, HUMAN, { created_at: "2026-10-18T11:00:00Z", updated_at: "2026-10-18T11:00:00Z" });
+  const person = await adopted(fakeApi({ ...gate("ci-fix-round-cap", [spoke]), opener: HUMAN }));
+  assert.deepEqual([person.decision, person.skipReason], ["skip", "awaiting-human"], "a person's review comment since the label");
+  const asked = await adopted(fakeApi({ ...gate("feedback-round-cap"), ...escalated({ replies: [[102, HUMAN, REQUEST]] }), opener: HUMAN }));
+  assert.equal(asked.decision, "proceed", "a write-access fix request on a held owner PR");
+  assert.deepEqual(asked.file.threads.map((thread) => thread.root_comment_id), [100]);
+  const appPr = await adopted(fakeApi(gate("feedback-round-cap", [spoke])));
+  assert.equal(appPr.decision, "proceed", "a lanes App PR under its own escalation");
+});
+
 test("on an owner PR a thread a person opened is never kept", async () => {
   const result = await run(fakeApi({ opener: HUMAN, reviewComments: [comment(100, HUMAN)], threads: [thread("PRRT_1", [100])] }));
   assert.deepEqual(result.threads, []);

.github/scripts/pr-unblock/feedback-dispatch.mjs

@@ -29,7 +29,8 @@
 //   asked the lane to fix it (isFixRequest), or a trusted person answered the
 //   lane's escalation (address-feedback.mjs threadAuthors); on a PR carrying
 //   the human-gated label, the same when a lane escalation put the label on
-//   and no review lane holds the PR (gate-source.mjs gateSource), and
+//   and no review lane holds the PR (gate-source.mjs gateSource), on an owner
+//   PR only as it lets fix-ci work there (prHold),
 //   otherwise only a thread where a trusted person with write access asked
 //   the lane to fix it (isFixRequest);
 // - that head has no pr-unblock / address-feedback check run yet (one the
@@ -125,10 +126,11 @@ import { pathToFileURL } from "node:url";
 import { commitPulls } from "../../actions/check-trusted-trigger/check-trusted-trigger.mjs";
 import { countRequests } from "../../actions/log-api-budget/rate-guard.mjs";
 import { LOOKBACK_HOURS, LOST } from "../maintenance-reconcile-lost-runs/infra-failure.mjs";
-import { fixRequesters, humanGatedLabelFrom, NO_ADOPTION, onReviewedHead, ownerAdoption, ownerAuthored, reviewedHeads, reviewerIdsFrom, threadAuthors, trustedHumansFrom } from "./address-feedback.mjs";
+import { fixRequesters, HOLD_LINE, humanGatedLabelFrom, NO_ADOPTION, onReviewedHead, ownerAdoption, ownerAuthored, reviewedHeads, reviewerIdsFrom, threadAuthors, trustedHumansFrom } from "./address-feedback.mjs";
 
-export { NO_ADOPTION, ownerAdoption, ownerAuthored };
-import { commentItems, gateSource, listLabelEvents } from "./gate-source.mjs";
+// fix-ci-dispatch.mjs imports HOLD_LINE from here.
+export { HOLD_LINE, NO_ADOPTION, ownerAdoption, ownerAuthored };
+import { commentItems, gateSource, listLabelEvents, prHold, reviewReader } from "./gate-source.mjs";
 import { emptyRecord, isHandled, markHandled, readRecord, writeRecord } from "./dispatch-record.mjs";
 import { anyHandles, KILL_SWITCH_STOP, LANE_STOP, laneStop, limitResetsAt, permanentlyHandles } from "./limit-stop.mjs";
 import { budgetedFetch } from "./request-budget.mjs";
@@ -164,14 +166,10 @@ export const DEFAULTS = Object.freeze({
 
 const SHA = /^[0-9a-f]{40}$/;
 const RUN_ID = /^[1-9][0-9]{0,14}$/;
-// A body line starting "Hold:", as babysit's BODY_MERGE_HOLD_RE reads it
-// (babysit_merge.py): any case, after any non-newline whitespace, NBSP
-// included, with a bare CR as a line break too. fix-ci-dispatch.mjs imports
-// it from here, so this floored module loads no unfloored one.
-export const HOLD_LINE = /(?:^|(?<=[\r\n]))[^\S\r\n]*hold:/i;
 
 // Why an owner PR is not adopted for a round now, or "": as fix-ci adopts one
-// (fix-ci-dispatch.mjs skipReason), no body line starting `Hold:`, and both
+// (fix-ci-dispatch.mjs skipReason), no body line starting `Hold:` (the select
+// job checks it again, as the round starts), and both
 // the PR's last update and its head commit at least idle-hours old, so the
 // lane does not push onto a branch a session is still working on. An
 // unreadable date is not idle.
@@ -388,18 +386,29 @@ export async function dispatchFeedback({
     const threads = await api.listReviewThreads(number);
     // A human-gated PR dispatches only for a thread a trusted person with
     // write access asked the lane to fix, unless a lane escalation put the
-    // label on and no review lane holds the PR. The label stays on either way:
-    // clear-escalations.mjs owns taking it off.
+    // label on and no review lane holds the PR; on an owner PR only as that
+    // escalation lets fix-ci work it (gate-source.mjs prHold). The label
+    // stays on either way: clear-escalations.mjs owns taking it off.
     const labeled = (pull.labels ?? []).some((label) => label.name === humanGatedLabel);
-    const gate = labeled
-      ? gateSource({
-          timeline: [...commentItems(comments), ...(await api.listLabelEvents(number))],
-          label: humanGatedLabel,
-          head,
-          isLanesApp: (user) => user?.id === appSenderId && user?.type === "Bot",
-        })
-      : null;
-    const gated = labeled && gate.source !== "lane-escalation";
+    const timeline = async () => [...commentItems(comments), ...(await api.listLabelEvents(number))];
+    let gated = false;
+    let why = "";
+    if (labeled && owner) {
+      ({ held: gated, why } = await prHold({
+        labels: pull.labels.map((label) => label.name),
+        holdLabels: [],
+        gateLabel: humanGatedLabel,
+        head,
+        activity: settings.activity,
+        appId: appSenderId,
+        readTimeline: timeline,
+        readReviews: reviewReader(api, number),
+      }));
+    } else if (labeled) {
+      const gate = gateSource({ timeline: await timeline(), label: humanGatedLabel, head, isLanesApp: (user) => user?.id === appSenderId && user?.type === "Bot" });
+      gated = gate.source !== "lane-escalation";
+      why = `${gate.source}: ${gate.reason}`;
+    }
     const fixers = await fixRequesters({
       comments: threads.flatMap((thread) =>
         thread.comments.nodes.map((node) => ({ authorId: node.author?.databaseId, login: node.author?.login, body: node.body })),
@@ -414,7 +423,7 @@ export async function dispatchFeedback({
     // lane-stopped check handles the head, so the stall gets a stop once.
     const workable = hasWorkableThread(threads, { reviewerId: settings.reviewerId, reviewerIds, appId: appSenderId, trustedHumans, heads: reviewedHeads(comments), requesters, fixers });
     if (gated && !workable) {
-      return skip(`#${number} is labeled ${humanGatedLabel} (${gate.source}: ${gate.reason}) and has no thread a trusted person with write access asked the lane to fix`);
+      return skip(`#${number} is labeled ${humanGatedLabel} (${why}) and has no thread a trusted person with write access asked the lane to fix`);
     }
     if (!workable && !threads.some((thread) => thread.isResolved === false)) {
       return skip(`#${number} has no unresolved review thread`);
@@ -803,6 +812,19 @@ export function createGitHubApi({ token, repository, apiUrl, graphqlUrl, fetch:
     return response.status === 204 ? {} : response.json();
   }
 
+  // Every page of a REST listing, at most 50.
+  async function pages(url) {
+    const rows = [];
+    for (let page = 1; page <= 50; page += 1) {
+      const batch = await request(`${url}&page=${page}`);
+      rows.push(...batch);
+      if (batch.length < 100) {
+        return rows;
+      }
+    }
+    throw new Error(`${url} has more than 50 pages`);
+  }
+
   async function graphql(query, variables, signal) {
     const result = await request(graphqlUrl, "POST", { query, variables }, signal);
     if (result.errors?.length) {
@@ -844,6 +866,10 @@ export function createGitHubApi({ token, repository, apiUrl, graphqlUrl, fetch:
       throw new Error(`#${number} has more than 5000 comments`);
     },
     listLabelEvents: (number) => listLabelEvents({ graphql, repository, number }),
+    // prHold's review reads (gate-source.mjs reviewReader), on an owner PR
+    // carrying the human-gated label only.
+    listReviewComments: (number, since) => pages(`${repo}/pulls/${number}/comments?since=${encodeURIComponent(since)}&per_page=100`),
+    listReviews: (number) => pages(`${repo}/pulls/${number}/reviews?per_page=100`),
     async listReviewThreads(number) {
       const threads = [];
       let after = null;