fix(pr-pipeline): sweep lanes App heads whose review dispatch skipped and never retried

.changes/pr-pipeline/fix-unblock-unsent-head-sweep-5b7e2c91.md

@@ -0,0 +1,7 @@
+---
+bump: patch
+---
+
+### Fixed
+
+- The half-hourly address-feedback sweep (`pr-feedback-dispatch.yml`, `sweepUnsent`) now sends a lanes App pull request whose head has an unresolved review thread and no address-feedback round, 10 minutes after the last review or pr-merge run at that head finished, and no round for it was created since that review started. Before, a review completion that skipped on a cause that clears later, such as a pr-verify FAIL-cap read that did not fit in its time limit, left the head with no other trigger, and one advisory thread held the pull request until a person dispatched the round.

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

@@ -85,10 +85,13 @@
 // main whose head's newest reply check took one of those skips, fewer than
 // SWEEP_HEAD_CAP times at that head, is decided as above with the
 // newest completed review run at that head as the upstream (reviewRunAt),
-// which the record names. With no such head, and no stale stop, it sends the
-// first idle adopted owner PR whose reviewed head has no round yet, or whose
-// round's stop no thread holds any more (sweepIdleOwner), since that PR's own
-// review completion found it active.
+// which the record names. With no such head and no stale stop, it sends the
+// first lanes App head with an unresolved thread that no round was sent for
+// (sweepUnsent): its review completion skipped on a cause that may clear, such
+// as an unreadable FAIL cap. With none, it sends the first idle adopted owner
+// PR whose reviewed head has no round yet, or whose round's stop no thread
+// holds any more (sweepIdleOwner), since that PR's own review completion found
+// it active.
 //
 // Before those, the tick takes a fix request nothing else would start: no
 // review run follows a reply on a head, and an earlier round's check run holds
@@ -767,6 +770,11 @@ export async function sweepReleased({ api, repository, appSenderId, log, setting
     return stale;
   }
   log("sweep: no head with a failing address-feedback stop and nothing left to fix");
+  const unsent = await sweepPulls(context, (pull) => sweepUnsent({ ...context, pull }), pulls);
+  if (unsent !== null) {
+    return unsent;
+  }
+  log("sweep: no head with an unresolved thread that no round was sent for");
   const owners = listed.filter((pull) => pull.user?.id !== appSenderId);
   const idle = owners.length === 0 ? null : await sweepPulls(context, (pull) => sweepIdleOwner({ ...context, pull }), owners);
   if (idle === null) {
@@ -912,6 +920,57 @@ async function sweepStale({ api, pull, repository, appSenderId, log, settings, r
   return { number, workflowRunId: id, upstreamRunId: upstream.id };
 }
 
+// pr-feedback-dispatch.yml's dispatch job timeout-minutes.
+export const UNSENT_GRACE_MS = 10 * 60_000;
+
+// A head with an unresolved thread and no address-feedback check run, and no
+// run of the activity for the PR created since its newest review run started:
+// its review completion's dispatch skipped for a reason that may clear, such
+// as verify-cap.mjs's unreadable FAIL cap, and no review run follows to send
+// it again. A skip that stands costs the reads dispatchFeedback makes before
+// it, each tick.
+async function sweepUnsent(context) {
+  const { api, pull, repository, appSenderId, log, settings, rest } = context;
+  const head = pull.head.sha;
+  if ((await api.listCheckRuns({ sha: head, checkName: settings.checkName, appId: settings.appId })).length > 0) {
+    return null;
+  }
+  const unresolved = (await api.listReviewThreads(pull.number)).filter((thread) => thread.isResolved === false);
+  if (unresolved.length === 0) {
+    return null;
+  }
+  const upstream = await reviewRunAt({ api, head, repository, settings });
+  if (upstream === null) {
+    return null;
+  }
+  // Each review, pr-merge and comment-relay run's completion at the head
+  // starts a dispatch in the head's concurrency group, not the sweep's, so the
+  // head waits out that job's timeout after the last of them. An unreadable
+  // time does not count. A pr-verify FAIL's dispatch runs in a verify-<id>
+  // group at main's head, so the notes it posts as unresolved threads just
+  // before it stand in for it: the newest comment on one counts too.
+  const triggers = [];
+  for (const [path, event] of [...settings.reviewWorkflows.map((path) => [path, "pull_request"]), [MERGE_WORKFLOW, "pull_request_target"], [RELAY_WORKFLOW, RELAY_EVENT]]) {
+    triggers.push(...(await api.listCompletedRuns(path, head, event)).filter((run) => run.head_sha === head && run.event === event));
+  }
+  const commented = unresolved.flatMap((thread) => thread.comments?.nodes ?? []).map((comment) => Date.parse(comment.createdAt ?? "")).filter(Number.isFinite);
+  const last = Math.max(...[upstream, ...triggers].map((run) => Date.parse(run.updated_at ?? "")), ...commented);
+  if (!(last <= (rest.now ?? Date.now()) - UNSENT_GRACE_MS)) {
+    return null;
+  }
+  // A round that ended before posting its check is sent once.
+  const reviewStarted = Date.parse(upstream.run_started_at ?? upstream.created_at ?? "");
+  const sent = (await api.listWorkflowRuns(settings.workflow)).find(
+    (run) => run.display_title === runTitle(settings.activity, pull.number) && !(Date.parse(run.created_at ?? "") < reviewStarted),
+  );
+  if (sent) {
+    log(`skip: #${pull.number}: ${settings.workflow} run ${sent.id} for ${settings.activity} started after head ${head}'s review`);
+    return null;
+  }
+  const dispatched = await dispatchFeedback({ ...rest, api, repository, appSenderId, log, settings, upstream });
+  return dispatched === null ? null : { ...dispatched, upstreamRunId: upstream.id };
+}
+
 // Whether the head's newest reply check released it, and whether releasing
 // skips reached SWEEP_HEAD_CAP there (logged).
 async function releaseState({ api, pull, log, settings }) {
@@ -1304,8 +1363,8 @@ export function createGitHubApi({ token, repository, apiUrl, graphqlUrl, fetch:
       throw new Error(`commit ${sha} has more than 1000 statuses`);
     },
     // The newest completed pull_request runs of a workflow file at a head.
-    async listCompletedRuns(path, head) {
-      const query = new URLSearchParams({ head_sha: head, event: "pull_request", status: "completed", per_page: "10" });
+    async listCompletedRuns(path, head, event = "pull_request") {
+      const query = new URLSearchParams({ head_sha: head, event, status: "completed", per_page: "10" });
       return (await request(`${repo}/actions/workflows/${path.split("/").at(-1)}/runs?${query}`)).workflow_runs ?? [];
     },
     // A workflow file's newest 100 runs created since `since`: many ticks'

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

@@ -5,7 +5,7 @@ import { test } from "node:test";
 
 import { LIMIT_DEFAULTS } from "../work-resolve-issue/limits.mjs";
 import { emptyRecord, parseRecord, serializeRecord } from "./dispatch-record.mjs";
-import { createGitHubApi, dispatchFeedback, failureAnnotation, FAILED_RUN_WINDOW_HOURS, IDLE_OWNER_WINDOW_HOURS, main, NO_ADOPTION, ownerAdoption, reviewRunAt, SWEEP_HEAD_CAP, sweepReleased } from "./feedback-dispatch.mjs";
+import { createGitHubApi, dispatchFeedback, failureAnnotation, FAILED_RUN_WINDOW_HOURS, IDLE_OWNER_WINDOW_HOURS, main, NO_ADOPTION, ownerAdoption, reviewRunAt, SWEEP_HEAD_CAP, sweepReleased, UNSENT_GRACE_MS } from "./feedback-dispatch.mjs";
 import { onReviewedHead, threadAuthors } from "./address-feedback.mjs";
 import { verifyReader } from "./verify-cap.mjs";
 import { tempDir } from "../../../lib/test-tmp.mjs";
@@ -2515,9 +2515,13 @@ test("a review-lane hold at the head gates a github-actions[bot] lane thread (#1
   assert.ok(lines.at(-1).includes("is labeled needs-human (review-hold: "), lines.at(-1));
 });
 
-test("the sweep does not revisit a head the human gate skipped once the label is off", async () => {
-  // The skipped dispatch posted no reply check, so sweepOne finds no release.
-  const api = { ...fakeApi({ pulls: pull(), threads: [LANE_FINDING] }), listOpenPulls: async () => pull() };
+// #12578, #12604, #12605: each review completion skipped on an artifact listing
+// past the caller's limit, a cause that clears on a later read, and no review
+// run followed, so one advisory lane thread held each lanes App PR until a
+// person dispatched it.
+const DEADLINE = "verify:unreadable: the artifact listing does not fit in the caller's limit: repository artifacts: 13962 artifacts in 140 pages need 147 s";
+const REVIEWED = upstreamRun({ id: 13, run_started_at: "2026-10-17T12:00:00Z", updated_at: "2026-10-17T12:10:00Z" });
+const sweepUnsent = async (api, verifyCapped, now = Date.parse("2026-10-17T12:20:00Z")) => {
   const lines = [];
   const result = await sweepReleased({
     api,
@@ -2526,10 +2530,70 @@ test("the sweep does not revisit a head the human gate skipped once the label is
     appSenderId: APP_ID,
     trustedHumans: new Set([KYLE.databaseId]),
     resolveSlot: async () => resolvedLane(),
+    verifyCapped,
+    now,
     log: (line) => lines.push(line),
   });
-  assert.equal(result, null);
-  assert.deepEqual(api.calls.dispatch, []);
+  return { result, lines };
+};
+const unsentApi = (options = {}) => {
+  const api = sweepApi({ checksAt: {}, threads: [LANE_FINDING], reviewRuns: { [HEAD]: [REVIEWED] }, ...options });
+  api.listWorkflowRuns = async () => options.workflowRuns ?? [];
+  // Completed runs of the other workflows at the head, by path.
+  const listCompletedRuns = api.listCompletedRuns;
+  api.listCompletedRuns = async (path, head, event) => [...(await listCompletedRuns(path, head, event)), ...(options.others?.[path] ?? [])];
+  return api;
+};
+
+test("the sweep sends a lanes App head with an unresolved thread that no round was sent for, once its skip clears", async () => {
+  // The review completion's read did not fit; the head stays unrecorded.
+  const record = emptyRecord();
+  const skipped = await decide(fakeApi({ pulls: pull(), threads: [LANE_FINDING] }), resolvedLane(), { record, verifyCapped: () => DEADLINE });
+  assert.equal(skipped.result, null);
+  assert.equal(record.handled.size, 0);
+
+  // The tick's read fits, so the head is sent through its newest review run.
+  const api = unsentApi();
+  const { result } = await sweepUnsent(api, () => null);
+  assert.deepEqual(result, { number: 42, workflowRunId: "38779200001", upstreamRunId: 13 });
+  assert.deepEqual(api.calls.dispatch.map((call) => call.inputs["pr-number"]), ["42"]);
+
+  // A read that still does not fit sends nothing, and the next tick tries again.
+  const still = unsentApi();
+  assert.equal((await sweepUnsent(still, () => DEADLINE)).result, null);
+  assert.deepEqual(still.calls.dispatch, []);
+});
+
+test("the unsent-head sweep leaves a head with a round, no unresolved thread, a review inside the grace, or a run since its review", async () => {
+  for (const [label, options, now] of [
+    ["a round at the head", { checksAt: { [HEAD]: [answeredRound] } }],
+    ["every thread resolved", { threads: [thread([CLAUDE], true)] }],
+    ["no completed review run", { reviewRuns: {} }],
+    ["a review completed inside the grace", {}, Date.parse("2026-10-17T12:10:00Z") + UNSENT_GRACE_MS - 1],
+    ["a review with no completion time", { reviewRuns: { [HEAD]: [upstreamRun({ id: 13, run_started_at: "2026-10-17T12:00:00Z" })] } }],
+    // Created first, so not reviewRunAt's run, but finished inside the grace.
+    [
+      "the other review lane finished inside the grace",
+      { others: { ".github/workflows/pr-review-security-hosted.yml": [upstreamRun({ id: 12, path: ".github/workflows/pr-review-security-hosted.yml", created_at: "2026-10-17T11:59:00Z", updated_at: "2026-10-17T12:15:00Z" })] } },
+    ],
+    ["a pr-merge run finished inside the grace", { others: { ".github/workflows/pr-merge.yml": [{ ...MERGE_RUN, conclusion: "success", updated_at: "2026-10-17T12:15:00Z" }] } }],
+    [
+      "a comment relay finished inside the grace",
+      { others: { ".github/workflows/pr-review-comment-relay.yml": [upstreamRun({ id: 14, path: ".github/workflows/pr-review-comment-relay.yml", event: "pull_request_review_comment", conclusion: "success", updated_at: "2026-10-17T12:15:00Z" })] } },
+    ],
+    // A pr-verify FAIL's notes, posted just before its dispatch in a group of its own.
+    ["a pr-verify note posted inside the grace", { threads: [{ ...LANE_FINDING, comments: { ...LANE_FINDING.comments, nodes: LANE_FINDING.comments.nodes.map((node) => ({ ...node, createdAt: "2026-10-17T12:15:00Z" })) } }] }],
+    ["a run since its review", { workflowRuns: [{ id: 9, status: "completed", display_title: TITLE_42, created_at: "2026-10-17T12:05:00Z" }] }],
+    ["do-not-merge", { openPulls: pull({ labels: [{ name: "do-not-merge" }] }) }],
+  ]) {
+    const api = unsentApi(options);
+    const { result } = await sweepUnsent(api, () => null, now);
+    assert.equal(result, null, label);
+    assert.deepEqual(api.calls.dispatch, [], label);
+  }
+  // A run from before the review does not hold it.
+  const older = unsentApi({ workflowRuns: [{ id: 8, status: "completed", display_title: TITLE_42, created_at: "2026-10-17T11:00:00Z" }] });
+  assert.equal((await sweepUnsent(older, () => null)).result?.number, 42);
 });
 
 // A label coming off a PR starts pr-merge.yml on pull_request_target; its

.github/workflows/pr-feedback-dispatch.yml

@@ -52,7 +52,9 @@ name: pr-feedback-dispatch
 # such head, it dispatches one head whose pr-unblock/address-feedback status
 # still fails though no review thread is left unresolved, once per stop, so
 # the run finds nothing to fix and report-stop-status clears the status
-# (sweepStale). Taking
+# (sweepStale). With neither, it dispatches one head with an unresolved thread
+# and no address-feedback round, whose review completion skipped on a cause
+# that may clear, such as an unreadable pr-verify FAIL cap (sweepUnsent). Taking
 # the tick's completion rather than a schedule of this file's own keeps
 # every dispatch a workflow_run dispatcher's, the one kind the gate and the
 # broker trust with a review run as its upstream.