From dc9b9a0eeff076d556ff28e55aca80062e642d20 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 3 Nov 2026 11:10:17 -0500 Subject: [PATCH 1/6] fix(ci): strip queue labels when an issue closes A hand-closed issue kept agent-ready, status: ready and admission-deferred, so queue counts and listings still read it as waiting. work-unblock's close run now removes those labels from the closed issue itself, reading it fresh: only the labels present come off, agent-ready last, and a 404 counts as removed. An issue open again by the time the run reads it, or a post-merge fix issue (which keeps agent-ready for its reopen on a repeat failure), keeps them. The list is report-outcome's ADMISSION_LABELS, now exported, and the lane-PR hand-off to triage removes its labels through the same helper, so both paths share one source. Co-Authored-By: Claude Opus 5.5 --- .../work-resolve-issue/report-outcome.mjs | 4 +- .../scripts/work-unblock/label-unblocked.mjs | 66 ++++++++++++-- .../work-unblock/label-unblocked.test.mjs | 87 ++++++++++++++++++- .github/workflows/work-unblock.yml | 4 + 4 files changed, 150 insertions(+), 11 deletions(-) diff --git a/.github/scripts/work-resolve-issue/report-outcome.mjs b/.github/scripts/work-resolve-issue/report-outcome.mjs index b851467a334..a7a94bd46d5 100755 --- a/.github/scripts/work-resolve-issue/report-outcome.mjs +++ b/.github/scripts/work-resolve-issue/report-outcome.mjs @@ -664,7 +664,9 @@ const ROUTE_LABEL = { "host-run": HOST_RUN_LABEL, attended: ATTENDED_LABEL, park const KEPT = new Set(["retry", "wait"]); // The routes whose stops are counted against a cap: past it, retry-spent's. const COUNTED = new Set([...KEPT, "intake"]); -const ADMISSION_LABELS = ["agent-ready", "status: ready", "admission-deferred"]; +// The queue labels: admission's, which work-unblock also strips from a closed +// issue and from one its lane PR handed back to triage. +export const ADMISSION_LABELS = ["agent-ready", "status: ready", "admission-deferred"]; const ROUTE_REMOVES = { intake: ADMISSION_LABELS, park: ADMISSION_LABELS }; const ROUTE_TEXT = { "host-run": `It added \`${HOST_RUN_LABEL}\` and removed \`agent-ready\`: an agent run on a host builds it, and it waits on no decision from a person.`, diff --git a/.github/scripts/work-unblock/label-unblocked.mjs b/.github/scripts/work-unblock/label-unblocked.mjs index ec7042f7a5a..c9add2da640 100755 --- a/.github/scripts/work-unblock/label-unblocked.mjs +++ b/.github/scripts/work-unblock/label-unblocked.mjs @@ -10,6 +10,11 @@ // marker, and a listed human closing it runs the sweep below at once, not the // close path. No model. // +// The close path also strips the closed issue's own queue labels +// (agent-ready, `status: ready`, admission-deferred; stripClosed), only those +// present, unless it is open again or is a post-merge fix issue. A close the +// broker refuses (a bot's) runs no close path, so it keeps them. +// // Each new agent-ready is paced: at or over the issue lane's queued cap, or // with the lane paused over its WIP or held cap (check-wip-cap.mjs, on or // after wip.pause-from, or whenever a search answers incomplete_results: @@ -277,7 +282,7 @@ import { notBornTriaged } from "../../actions/check-born-triaged/check-born-tria import { createGitHub, isTrustedClose as closedByHumanOrMerge, loadTrustedList, paginate, readRerunner } from "../../actions/check-trusted-trigger/check-trusted-trigger.mjs"; import { writeOutputs } from "../../actions/check-kill-switch/check-kill-switch.mjs"; import { admissionLine, checkWip, searchRateLimit } from "../../actions/check-wip-cap/check-wip-cap.mjs"; -import { clampReset, isLaneBranch, LANE_HOLDS, LANE_WORKFLOWS, parseOutcomeMarker, REFIRED, refireComment as refireRecord, refsHoldMarker } from "../work-resolve-issue/report-outcome.mjs"; +import { ADMISSION_LABELS, clampReset, isLaneBranch, LANE_HOLDS, LANE_WORKFLOWS, parseOutcomeMarker, REFIRED, refireComment as refireRecord, refsHoldMarker } from "../work-resolve-issue/report-outcome.mjs"; import { readLimits, readWip } from "../work-resolve-issue/limits.mjs"; import { ADMIT_MARKER, admissionFacts, admissionParked, admissionSettled, ATTENDED, authorAdmitted, attendedHold, blockersOf, CONFIRMED, decide, deferralLapsed, loadRules, NEEDS_DECISION, openPullsFor, openSubIssues, READY, refusalText, removalStands, sayWhy, SIZE_SPLIT_PENDING, specOf, stillBlocking, subIssueWait, UNTRUSTED, untrustedHold } from "../intake-triage/admit.mjs"; import { clearedPastReport, retriageNote, doneReason, judgedSince, mergedRefsReader, splitLeftOpen, splitParentClosed } from "../intake-triage/admit.mjs"; @@ -856,6 +861,52 @@ const noMirror = async () => {}; // A label change as the status mirror takes it (write-status.mjs statusMirror). const change = (had = [], add = [], remove = []) => ({ had, add, remove }); +// The queue labels (report-outcome's ADMISSION_LABELS), agent-ready last: the +// sweep lists open agent-ready issues, so a failed write leaves the issue for it. +const QUEUE_LABELS = [...ADMISSION_LABELS.filter((name) => name !== LABEL), LABEL]; + +// Removes the queue labels `had` holds from the issue at `base`, and returns +// their names. A 404 is a label already gone, so it counts as removed. +async function stripQueueLabels(github, base, had) { + const present = new Set(had.map((name) => name.toLowerCase())); + const removed = QUEUE_LABELS.filter((name) => present.has(name)); + for (const name of removed) { + await github("DELETE", `${base}/labels/${encodeURIComponent(name)}`).catch((error) => { + if (error?.status !== 404) { + throw error; + } + }); + } + return removed; +} + +// A closed issue loses its queue labels, so no queue or count reads it as +// waiting. Read fresh: a run queued behind another may land after a reopen, +// and an open issue keeps them. A post-merge fix issue keeps agent-ready +// across its close, as post-merge-demotion-alert.yml reopens it with the label +// and the sweep relabels it (recurrenceRefusal). No Work status write: the +// field's states are an open issue's. +async function stripClosed({ github, repo, number }) { + const base = `/repos/${repo}/issues/${number}`; + try { + const issue = await github("GET", base); + if (issue?.state !== "closed") { + console.log(`label-unblocked: #${number} is open again; its queue labels stay`); + return 0; + } + if (issue.user?.login === ALERT_BOT && FIX_ISSUE.test(String(issue.body ?? ""))) { + console.log(`label-unblocked: #${number} is a post-merge fix issue; its queue labels stay`); + return 0; + } + const removed = await stripQueueLabels(github, base, labelNames(issue)); + console.log(`label-unblocked: #${number} closed; removed ${removed.length} queue label(s)${removed.length > 0 ? `: ${removed.join(", ")}` : ""}`); + return 0; + } catch (error) { + reportError(number, error); + return 1; + } +} + // The verdict intake's admission reads, from the issue itself: each of Size, // Complexity and Uncertainty as the config's key for the option a listed // human or the App set, else `unset`. The work class gates nothing (owner @@ -1974,13 +2025,8 @@ async function restartLimited({ github, actions, jobLog, repo, bot, humans, now, if (release) { await github("DELETE", `${base}/labels/${HUMAN_LABEL}`); } - await github("DELETE", `${base}/labels/${encodeURIComponent(READY)}`).catch((error) => { - if (error?.status !== 404) { - throw error; - } - }); - await github("DELETE", `${base}/labels/${LABEL}`); - await mirror(number, change(had, [], [LABEL, READY, ...(release ? [HUMAN_LABEL] : [])])); + const removed = await stripQueueLabels(github, base, had); + await mirror(number, change(had, [], [...removed, ...(release ? [HUMAN_LABEL] : [])])); console.log(`label-unblocked: #${number} handed to triage: lane PR #${pr} ${handOff === "merged" ? "merged with Refs:" : "closed unmerged"}`); } catch (error) { failed += 1; @@ -2249,6 +2295,10 @@ export async function main({ env = process.env, github, actions, jobLog, now = D // Before readmitReady, so a split parent whose last sub-issue closed is // closed, not decided, on the sweep that sees that close. failed += await closeSplitParents({ github, repo, closed, sweep, humans, bot }); + // The sweep marker carries no queue labels, so a kick skips this. + if (!sweep) { + failed += await stripClosed({ github, repo, number: Number(event.issue.number) }); + } if (sweep) { // One listing of the open issues, oldest updated first, for the parent // rescan and readmitReady. diff --git a/.github/scripts/work-unblock/label-unblocked.test.mjs b/.github/scripts/work-unblock/label-unblocked.test.mjs index 44371a7b539..deb62686e77 100644 --- a/.github/scripts/work-unblock/label-unblocked.test.mjs +++ b/.github/scripts/work-unblock/label-unblocked.test.mjs @@ -402,6 +402,76 @@ test("the last blocker closing leaves an umbrella with an open sub-issue unlabel assert.deepEqual(writes(drained.calls), [labeled(20)]); }); +// The close path strips the closed issue's own queue labels (agent-ready, +// status: ready, admission-deferred), read fresh from the API, so a +// hand-closed issue leaves the queue. Number 10 blocks nothing here. +const closedSelf = (labels, extra = {}) => world({ blocking: [], issues: { 10: { ...issue(10, { state: "closed", labels }), ...extra } } }); +const unlabeled = (name) => `DELETE /repos/${REPO}/issues/10/labels/${encodeURIComponent(name)}`; + +test("a closed issue loses the queue labels it carries, agent-ready last, and keeps its other labels", async () => { + const { github, calls } = closedSelf(["priority: high", "agent-ready", "status: ready", "work-class: scoped"]); + assert.equal(await main({ env: envFor(), github }), 0); + assert.deepEqual(writes(calls), [unlabeled("status: ready"), unlabeled("agent-ready")]); + const deferred = closedSelf(["admission-deferred", "status: ready"]); + assert.equal(await main({ env: envFor(), github: deferred.github }), 0); + assert.deepEqual(writes(deferred.calls), [unlabeled("status: ready"), unlabeled("admission-deferred")]); +}); + +test("a closed issue with no queue label gets no write", async () => { + const { github, calls } = closedSelf(["priority: high", "bug"]); + assert.equal(await main({ env: envFor(), github }), 0); + assert.deepEqual(writes(calls), []); + assert.ok(calls.includes(`GET /repos/${REPO}/issues/10`)); +}); + +test("a queue label already gone (404) counts as removed, and the rest still come off", async () => { + const { github: inner, calls } = closedSelf(["agent-ready", "status: ready"]); + const github = async (method, path, body) => { + if (method === "DELETE" && path.endsWith("/issues/10/labels/status%3A%20ready")) { + calls.push(`${method} ${path}`); + throw Object.assign(new Error("DELETE failed with status 404"), { status: 404 }); + } + return inner(method, path, body); + }; + assert.equal(await main({ env: envFor(), github }), 0); + assert.deepEqual(writes(calls), [unlabeled("status: ready"), unlabeled("agent-ready")]); +}); + +test("a failed queue-label removal fails the run and leaves agent-ready for the next close or person", async (t) => { + const { github: inner, calls } = closedSelf(["agent-ready", "status: ready"]); + const github = async (method, path, body) => { + if (method === "DELETE" && path.endsWith("/issues/10/labels/status%3A%20ready")) { + calls.push(`${method} ${path}`); + throw Object.assign(new Error("DELETE failed with status 502"), { status: 502 }); + } + return inner(method, path, body); + }; + const log = t.mock.method(console, "log", () => {}); + assert.equal(await main({ env: envFor(), github }), 1); + log.mock.restore(); + assert.deepEqual(writes(calls), [unlabeled("status: ready")]); +}); + +test("an issue open again when the run reads it keeps its queue labels", async () => { + const { github, calls } = world({ blocking: [], issues: { 10: issue(10, { labels: ["agent-ready", "status: ready"] }) } }); + assert.equal(await main({ env: envFor(), github }), 0); + assert.deepEqual(writes(calls), []); +}); + +test("a post-merge fix issue keeps agent-ready across its close, for its reopen on a repeat failure", async () => { + const alert = { login: "github-actions[bot]", type: "Bot" }; + const body = "\nFix main."; + const fix = closedSelf(["agent-ready", "status: ready"], { user: alert, body }); + assert.equal(await main({ env: envFor(), github: fix.github }), 0); + assert.deepEqual(writes(fix.calls), []); + // The same body from another author, or another body from the alert, is stripped. + for (const extra of [{ user: OWNER, body }, { user: alert, body: "Fix main." }]) { + const other = closedSelf(["agent-ready"], extra); + assert.equal(await main({ env: envFor(), github: other.github }), 0); + assert.deepEqual(writes(other.calls), [unlabeled("agent-ready")], JSON.stringify(extra)); + } +}); + test("an issue held by needs-owner-decision, status: needs-decision, status: needs-info, needs-human, untrusted-provenance or attended stays unlabeled", async () => { for (const hold of ["needs-owner-decision", "status: needs-decision", "status: needs-info", "needs-human", "work-class: untrusted-provenance", "attended"]) { const { github, calls } = oneDependent({ labels: [hold] }); @@ -2953,7 +3023,8 @@ const crossRef = (pull, repo = REPO) => ({ }, }, }); -function lanePr({ pulls = [lanePull()], comments = [note()], events = [added(LABELED_AT, OWNER)], labels = [], runs = [] } = {}) { +// The issue carries `status: ready`, as admission left it. +function lanePr({ pulls = [lanePull()], comments = [note()], events = [added(LABELED_AT, OWNER)], labels = ["status: ready"], runs = [] } = {}) { const fake = world({ openReady: [ready(30, labels)], comments: { 30: comments }, @@ -3104,7 +3175,7 @@ const humanLabeled = (at = "2026-10-16T10:20:05Z", actor = LANES_BOT, event = "l }); const refsHeld = (extra = {}) => lanePr({ - labels: ["needs-human"], + labels: ["needs-human", "status: ready"], comments: [note(), holdNote()], events: [added(LABELED_AT, OWNER), humanLabeled()], ...extra, @@ -3238,6 +3309,18 @@ test("a hand-off whose label write failed is finished by the next sweep with the assert.deepEqual(writes(next.calls), triaged(30, 7493).slice(1)); }); +// The hand-off and the close path share one queue-label list: each removes +// only the queue labels the issue carries, agent-ready last. +test("a hand-off removes only the queue labels the issue carries, admission-deferred included, agent-ready last", async () => { + const bare = lanePr({ pulls: [CLOSED_PR()], labels: [] }); + assert.equal(await sweep({ github: bare.github }), 0); + assert.deepEqual(writes(bare.calls), [triaged(30, 7493, refsClosedComment(7493))[0], `DELETE /repos/${REPO}/issues/30/labels/agent-ready`]); + const deferred = lanePr({ pulls: [CLOSED_PR()], labels: ["status: ready", "admission-deferred"] }); + assert.equal(await sweep({ github: deferred.github }), 0); + const [comment, status, agentReady] = triaged(30, 7493, refsClosedComment(7493)); + assert.deepEqual(writes(deferred.calls), [comment, status, `DELETE /repos/${REPO}/issues/30/labels/admission-deferred`, agentReady]); +}); + // Runs from before the run name carried ` #` cannot be matched to an // issue, so the sweep says it cannot tell instead of that no run started. test("a run in the label's window whose title names no issue makes the sweep say it cannot tell", async (t) => { diff --git a/.github/workflows/work-unblock.yml b/.github/workflows/work-unblock.yml index 30266e5046a..cf87d8d479a 100644 --- a/.github/workflows/work-unblock.yml +++ b/.github/workflows/work-unblock.yml @@ -13,6 +13,10 @@ name: work-unblock # .github/scripts/work-unblock/label-unblocked.mjs; the lane and its # effect (mutate-tracker) are in docs/conventions/pr-pipeline.yaml. # +# The same close run removes the closed issue's own queue labels (agent-ready, +# status: ready, admission-deferred), so a hand-closed issue leaves the queue; +# a post-merge fix issue keeps them for its reopen. +# # It runs no AI model and never checks out anything but the main branch. # The label is added with a lanes App token from the token broker: a label # added with the job's own GITHUB_TOKEN could never start a workflow. From fdcce382718ecaaab2535c4e9537290c2f006154 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 3 Nov 2026 11:16:42 -0500 Subject: [PATCH 2/6] fix(ci): strip queue labels where the lanes App closes an issue An App close starts no work-unblock close run, so the issue kept its queue labels. The two places the App closes an issue now strip them right after the close, with the token each already holds: report-outcome's close route (already-done, report-delivered) and work-unblock's split-parent close. stripQueueLabels and the list move to report-outcome.mjs, which label-unblocked already imports, so every path shares one helper; report-outcome's API errors now carry their HTTP status so a 404 counts as removed. Co-Authored-By: Claude Opus 5.5 --- .../work-resolve-issue/report-outcome.mjs | 30 ++++++++++++++++--- .../report-outcome.test.mjs | 21 +++++++++++++ .../scripts/work-unblock/label-unblocked.mjs | 30 +++++-------------- .../work-unblock/label-unblocked.test.mjs | 12 ++++++++ 4 files changed, 66 insertions(+), 27 deletions(-) diff --git a/.github/scripts/work-resolve-issue/report-outcome.mjs b/.github/scripts/work-resolve-issue/report-outcome.mjs index a7a94bd46d5..acd837edc28 100755 --- a/.github/scripts/work-resolve-issue/report-outcome.mjs +++ b/.github/scripts/work-resolve-issue/report-outcome.mjs @@ -664,9 +664,27 @@ const ROUTE_LABEL = { "host-run": HOST_RUN_LABEL, attended: ATTENDED_LABEL, park const KEPT = new Set(["retry", "wait"]); // The routes whose stops are counted against a cap: past it, retry-spent's. const COUNTED = new Set([...KEPT, "intake"]); -// The queue labels: admission's, which work-unblock also strips from a closed +// The queue labels: admission's, which stripQueueLabels removes from a closed // issue and from one its lane PR handed back to triage. export const ADMISSION_LABELS = ["agent-ready", "status: ready", "admission-deferred"]; +// Agent-ready last: work-unblock's sweep lists open agent-ready issues, so a +// failed write leaves the issue for it. +const QUEUE_LABELS = [...ADMISSION_LABELS.filter((name) => name !== "agent-ready"), "agent-ready"]; + +// Removes the queue labels `had` holds from the issue at `base`, and returns +// their names. A 404 is a label already gone, so it counts as removed. +export async function stripQueueLabels(github, base, had) { + const present = new Set(had.map((name) => String(typeof name === "string" ? name : name?.name).toLowerCase())); + const removed = QUEUE_LABELS.filter((name) => present.has(name)); + for (const name of removed) { + await github("DELETE", `${base}/labels/${encodeURIComponent(name)}`).catch((error) => { + if (error?.status !== 404) { + throw error; + } + }); + } + return removed; +} const ROUTE_REMOVES = { intake: ADMISSION_LABELS, park: ADMISSION_LABELS }; const ROUTE_TEXT = { "host-run": `It added \`${HOST_RUN_LABEL}\` and removed \`agent-ready\`: an agent run on a host builds it, and it waits on no decision from a person.`, @@ -1878,7 +1896,7 @@ function api({ fetch: rawFetch, apiUrl, token, sleep }) { ...(body === undefined ? {} : { body: JSON.stringify(body) }), }); if (!response.ok) { - throw new Error(`${method} ${path} answered ${response.status}`); + throw Object.assign(new Error(`${method} ${path} answered ${response.status}`), { status: response.status }); } return response.status === 204 ? null : response.json(); }; @@ -2182,8 +2200,12 @@ async function finishStop(github, repo, plan, { bot, humans, runId, limits, lane } await applyHandBack(github, repo, plan); if (plan.action === "close") { - await github("PATCH", `${repo}/issues/${plan.issueNumber}`, { state: "closed", state_reason: "completed" }, { safeToResend: true }); - console.log(`report-outcome: closed issue #${plan.issueNumber} (${plan.stop})`); + const base = `${repo}/issues/${plan.issueNumber}`; + const closed = await github("PATCH", base, { state: "closed", state_reason: "completed" }, { safeToResend: true }); + // The App's close starts no work-unblock close run, so the queue labels + // still on the issue come off here, as read from the close's answer. + const removed = await stripQueueLabels(github, base, closed?.labels ?? []); + console.log(`report-outcome: closed issue #${plan.issueNumber} (${plan.stop}); removed ${removed.length} queue label(s)`); } } diff --git a/.github/scripts/work-resolve-issue/report-outcome.test.mjs b/.github/scripts/work-resolve-issue/report-outcome.test.mjs index aa698aae6b6..f62bb35116d 100644 --- a/.github/scripts/work-resolve-issue/report-outcome.test.mjs +++ b/.github/scripts/work-resolve-issue/report-outcome.test.mjs @@ -4614,6 +4614,27 @@ test("main posts a delivered report and closes the issue", async () => { assert.deepEqual(run.calls.find((c) => c.method === "PATCH").body, { state: "closed", state_reason: "completed" }); }); +// The App's close starts no work-unblock close run, so the queue labels the +// close's answer still shows come off right after it, agent-ready last. +test("main's close removes the queue labels the closed issue still carries, after the close and agent-ready last", async () => { + const closedWith = (labels) => ["PATCH", /\/issues\/77$/, 200, { state: "closed", labels: labels.map((name) => ({ name })) }]; + const run = await stopRun({ reason: "report-delivered: the audit found three stale rows", routes: [closedWith(["status: ready", "admission-deferred", "agent-ready", "priority: low"])] }); + assert.equal(run.code, 0); + const after = run.calls.slice(run.calls.findIndex((c) => c.method === "PATCH") + 1).map((c) => `${c.method} ${c.path.replace(/^.*\/issues\/77/, "")}`); + assert.deepEqual(after, ["DELETE /labels/status%3A%20ready", "DELETE /labels/admission-deferred", "DELETE /labels/agent-ready"]); + // None left on the issue: nothing more is written after the close. + const bare = await stopRun({ reason: "report-delivered: the audit found three stale rows", routes: [closedWith(["priority: low"])] }); + assert.equal(bare.code, 0); + assert.deepEqual(bare.calls.slice(bare.calls.findIndex((c) => c.method === "PATCH") + 1), []); + // A label already gone (404) counts as removed. + const gone = await stopRun({ + reason: "report-delivered: the audit found three stale rows", + routes: [closedWith(["status: ready", "agent-ready"]), ["DELETE", /\/labels\/status%3A%20ready$/, 404, {}]], + }); + assert.equal(gone.code, 0); + assert.ok(gone.calls.some((c) => c.method === "DELETE" && c.path.endsWith("/77/labels/agent-ready"))); +}); + test("retries count the App's retry markers since a listed human last added agent-ready, not this run's", () => { const marker = (outcome, run, at) => ({ user: { login: "melodic-automation-lanes[bot]" }, body: ``, created_at: at }); const comments = [marker("retry", "1", "2026-10-17T01:00:00Z"), marker("retry", "2", "2026-10-17T03:00:00Z"), marker("turn-cap", "3", "2026-10-17T04:00:00Z"), marker("retry", "555", "2026-10-17T05:00:00Z")]; diff --git a/.github/scripts/work-unblock/label-unblocked.mjs b/.github/scripts/work-unblock/label-unblocked.mjs index b53dcc1140c..879543cbc61 100755 --- a/.github/scripts/work-unblock/label-unblocked.mjs +++ b/.github/scripts/work-unblock/label-unblocked.mjs @@ -12,8 +12,9 @@ // // The close path also strips the closed issue's own queue labels // (agent-ready, `status: ready`, admission-deferred; stripClosed), only those -// present, unless it is open again or is a post-merge fix issue. A close the -// broker refuses (a bot's) runs no close path, so it keeps them. +// present, unless it is open again or is a post-merge fix issue. A bot's close +// runs no close path: where the lanes App closes an issue itself (a split +// parent here, report-outcome's close route) it strips them after the close. // // Each new agent-ready is paced: at or over the issue lane's queued cap, or // with the lane paused over its WIP or held cap (check-wip-cap.mjs, on or @@ -283,7 +284,7 @@ import { notBornTriaged } from "../../actions/check-born-triaged/check-born-tria import { createGitHub, isTrustedClose as closedByHumanOrMerge, loadTrustedList, paginate, readRerunner } from "../../actions/check-trusted-trigger/check-trusted-trigger.mjs"; import { writeOutputs } from "../../actions/check-kill-switch/check-kill-switch.mjs"; import { admissionLine, checkWip, searchRateLimit } from "../../actions/check-wip-cap/check-wip-cap.mjs"; -import { ADMISSION_LABELS, clampReset, isLaneBranch, LANE_HOLDS, LANE_WORKFLOWS, parseOutcomeMarker, REFIRED, refireComment as refireRecord, refsHoldMarker } from "../work-resolve-issue/report-outcome.mjs"; +import { clampReset, isLaneBranch, LANE_HOLDS, LANE_WORKFLOWS, parseOutcomeMarker, REFIRED, refireComment as refireRecord, refsHoldMarker, stripQueueLabels } from "../work-resolve-issue/report-outcome.mjs"; import { readLimits, readWip, resolveStrandedBackoff, STRANDED_BACKOFF_MAX } from "../work-resolve-issue/limits.mjs"; import { ADMIT_MARKER, admissionFacts, admissionParked, admissionSettled, ATTENDED, authorAdmitted, attendedHold, blockersOf, CONFIRMED, decide, deferralLapsed, loadRules, NEEDS_DECISION, openPullsFor, openSubIssues, READY, refusalText, removalStands, sayWhy, SIZE_SPLIT_PENDING, specOf, stillBlocking, subIssueWait, UNTRUSTED, untrustedHold } from "../intake-triage/admit.mjs"; import { clearedPastReport, retriageNote, doneReason, judgedSince, mergedRefsReader, splitLeftOpen, splitParentClosed } from "../intake-triage/admit.mjs"; @@ -862,25 +863,6 @@ const noMirror = async () => {}; // A label change as the status mirror takes it (write-status.mjs statusMirror). const change = (had = [], add = [], remove = []) => ({ had, add, remove }); -// The queue labels (report-outcome's ADMISSION_LABELS), agent-ready last: the -// sweep lists open agent-ready issues, so a failed write leaves the issue for it. -const QUEUE_LABELS = [...ADMISSION_LABELS.filter((name) => name !== LABEL), LABEL]; - -// Removes the queue labels `had` holds from the issue at `base`, and returns -// their names. A 404 is a label already gone, so it counts as removed. -async function stripQueueLabels(github, base, had) { - const present = new Set(had.map((name) => name.toLowerCase())); - const removed = QUEUE_LABELS.filter((name) => present.has(name)); - for (const name of removed) { - await github("DELETE", `${base}/labels/${encodeURIComponent(name)}`).catch((error) => { - if (error?.status !== 404) { - throw error; - } - }); - } - return removed; -} - // A closed issue loses its queue labels, so no queue or count reads it as // waiting. Read fresh: a run queued behind another may land after a reopen, // and an open issue keeps them. A post-merge fix issue keeps agent-ready @@ -2173,7 +2155,9 @@ async function closeDoneParent({ github, repo, parent, humans, closes, bot }) { }); } await github("PATCH", base, { state: "closed", state_reason: "completed" }); - console.log(`label-unblocked: split parent #${parent.number} closed as completed`); + // The App's close starts no close run, so its queue labels come off here. + const removed = await stripQueueLabels(github, base, labelNames(parent)); + console.log(`label-unblocked: split parent #${parent.number} closed as completed; removed ${removed.length} queue label(s)`); } // The admission comment's text for a wait, before its reasons (sayWhy's callers). diff --git a/.github/scripts/work-unblock/label-unblocked.test.mjs b/.github/scripts/work-unblock/label-unblocked.test.mjs index 5114754d2ed..4871c064964 100644 --- a/.github/scripts/work-unblock/label-unblocked.test.mjs +++ b/.github/scripts/work-unblock/label-unblocked.test.mjs @@ -4552,6 +4552,18 @@ test("the last sub-issue closing by a listed human or a merge closes its split p assert.match(comment, /#31, #32/); }); +// The App's close starts no close run, so the split parent's queue labels +// come off right after it: only those present, agent-ready last. +test("closing a split parent removes the queue labels it carries, agent-ready last", async () => { + const { github, calls } = splitWorld({ parent: issue(30, { labels: ["agent-ready", "status: ready", "priority: high"] }) }); + assert.equal(await sweep({ github }), 0); + assert.deepEqual(writtenPaths(calls), [ + ...closeParentWrites, + `DELETE /repos/${REPO}/issues/30/labels/status%3A%20ready`, + `DELETE /repos/${REPO}/issues/30/labels/agent-ready`, + ]); +}); + test("a split parent stays open while a sub-issue is open, or closed by someone else, or with no split record from the App", async () => { const outsider = { id: 4242, login: "someone-else", type: "User" }; for (const setup of [ From c9e1b34e2f2a97d715d8019119224558b3d17787 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 3 Nov 2026 11:19:26 -0500 Subject: [PATCH 3/6] fix(ci): strip queue labels from merge-closed issues on the unblock pass GitHub closes an issue when a pull request the lanes App merged lands, and no close run follows a bot's close, so such an issue kept its queue labels. work-unblock's existing sweep, which runs after every work-resolve-issue run and hourly, already walks each issue a listed human or a merge closed in its window; it now strips their queue labels there too, through the shared helper, with the same exemptions. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/work-unblock/label-unblocked.mjs | 9 ++++++++- .../work-unblock/label-unblocked.test.mjs | 16 ++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/.github/scripts/work-unblock/label-unblocked.mjs b/.github/scripts/work-unblock/label-unblocked.mjs index 879543cbc61..cfc1676a9af 100755 --- a/.github/scripts/work-unblock/label-unblocked.mjs +++ b/.github/scripts/work-unblock/label-unblocked.mjs @@ -14,7 +14,9 @@ // (agent-ready, `status: ready`, admission-deferred; stripClosed), only those // present, unless it is open again or is a post-merge fix issue. A bot's close // runs no close path: where the lanes App closes an issue itself (a split -// parent here, report-outcome's close route) it strips them after the close. +// parent here, report-outcome's close route) it strips them after the close, +// and the sweep strips each issue in its window a listed human or a merge +// closed (a merge the App ran among them). // // Each new agent-ready is paced: at or over the issue lane's queued cap, or // with the lane paused over its WIP or held cap (check-wip-cap.mjs, on or @@ -1163,6 +1165,11 @@ async function labelUnblocked({ github, repo, closed, sweep, humans, bot, gate, console.log(`label-unblocked: #${source} was closed by neither a listed human nor a merge; skipped`); continue; } + // A merge the App ran closes an issue with no close run, so the sweep + // strips its queue labels; a listed human's close run already did. + if (sweep) { + failed += await stripClosed({ github, repo, number: source }); + } blocked = await paginate(github, `/repos/${repo}/issues/${source}/dependencies/blocking`); } catch (error) { failed += 1; diff --git a/.github/scripts/work-unblock/label-unblocked.test.mjs b/.github/scripts/work-unblock/label-unblocked.test.mjs index 4871c064964..60d23dccc05 100644 --- a/.github/scripts/work-unblock/label-unblocked.test.mjs +++ b/.github/scripts/work-unblock/label-unblocked.test.mjs @@ -756,6 +756,22 @@ function sweptDependent({ sibling = "closed" } = {}) { }); } +// A merge the App ran closes an issue with no close run, so the sweep strips +// the queue labels from each issue in its window a listed human or a merge closed. +test("the sweep strips the queue labels from a merge-closed issue, agent-ready last, and writes nothing for one without them", async () => { + const mergeClosed = (labels) => ({ ...closedAt(10, "2026-10-16T11:30:00Z"), labels: labels.map((name) => ({ name })) }); + const labeled = world({ closedList: [mergeClosed(["agent-ready", "status: ready", "priority: low"])], blocking: [] }); + assert.equal(await sweep({ github: labeled.github }), 0); + assert.deepEqual(writes(labeled.calls), [`DELETE /repos/${REPO}/issues/10/labels/status%3A%20ready`, `DELETE /repos/${REPO}/issues/10/labels/agent-ready`]); + const bare = world({ closedList: [mergeClosed(["priority: low"])], blocking: [] }); + assert.equal(await sweep({ github: bare.github }), 0); + assert.deepEqual(writes(bare.calls), []); + // A close by neither a listed human nor a merge is left alone. + const outsider = world({ closedList: [{ ...mergeClosed(["agent-ready"]), graphCloser: null }], blocking: [] }); + assert.equal(await sweep({ github: outsider.github }), 0); + assert.deepEqual(writes(outsider.calls), []); +}); + test("the sweep labels an issue whose last blocker a bot closed", async () => { const { github, calls } = sweptDependent(); assert.equal(await sweep({ github }), 0); From 76cc089c5d47b6ca89c4749a0ae9949570162bad Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 3 Nov 2026 11:23:10 -0500 Subject: [PATCH 4/6] perf(ci): reuse the close check's issue read when stripping labels On a sweep pass, each closed issue was read three times: by the close check, by the label strip, and by the split-parent pass. The close check (readClose) now caches the issue it read beside its verdict; the strip takes that read, and the split-parent pass shares the same cache, so a merge-closed issue is read once per run. The close-event path keeps its own fresh read, since that job can run after a reopen. Co-Authored-By: Claude Opus 5.5 --- .../scripts/work-unblock/label-unblocked.mjs | 51 +++++++++++-------- .../work-unblock/label-unblocked.test.mjs | 2 + 2 files changed, 33 insertions(+), 20 deletions(-) diff --git a/.github/scripts/work-unblock/label-unblocked.mjs b/.github/scripts/work-unblock/label-unblocked.mjs index cfc1676a9af..7480911bac3 100755 --- a/.github/scripts/work-unblock/label-unblocked.mjs +++ b/.github/scripts/work-unblock/label-unblocked.mjs @@ -613,14 +613,21 @@ async function closedSince(github, repo, since) { // One answer per issue per run: a swept close is also its dependents' // blocker, and a blocker can block several dependents. `where` is the // blocker's repository: outside `repo`, only a listed human's close counts. -function trustedClose({ github, repo, where = repo, number, humans, closes, bot = DEFAULT_BOT }) { +function trustedClose(args) { + return readClose(args).then(({ trusted }) => trusted); +} + +// trustedClose's one read, cached in `closes`: { issue, trusted }, the issue +// as read then, so a caller that needs it reads it no second time. +function readClose({ github, repo, where = repo, number, humans, closes, bot = DEFAULT_BOT }) { const key = `${where.toLowerCase()}#${number}`; if (!closes.has(key)) { closes.set( key, (async () => { - const closed = await github("GET", `/repos/${where}/issues/${number}`); - return where.toLowerCase() === repo.toLowerCase() ? isTrustedClose(github, repo, closed, humans, bot) : humans.has(closed?.closed_by?.id); + const issue = await github("GET", `/repos/${where}/issues/${number}`); + const trusted = await (where.toLowerCase() === repo.toLowerCase() ? isTrustedClose(github, repo, issue, humans, bot) : humans.has(issue?.closed_by?.id)); + return { issue, trusted }; })(), ); } @@ -867,14 +874,15 @@ const change = (had = [], add = [], remove = []) => ({ had, add, remove }); // A closed issue loses its queue labels, so no queue or count reads it as // waiting. Read fresh: a run queued behind another may land after a reopen, -// and an open issue keeps them. A post-merge fix issue keeps agent-ready +// and an open issue keeps them; the sweep hands in the issue its close check +// just read (`read`) instead. A post-merge fix issue keeps agent-ready // across its close, as post-merge-demotion-alert.yml reopens it with the label // and the sweep relabels it (recurrenceRefusal). No Work status write: the // field's states are an open issue's. -async function stripClosed({ github, repo, number }) { +async function stripClosed({ github, repo, number, read }) { const base = `/repos/${repo}/issues/${number}`; try { - const issue = await github("GET", base); + const issue = read ?? (await github("GET", base)); if (issue?.state !== "closed") { console.log(`label-unblocked: #${number} is open again; its queue labels stay`); return 0; @@ -1154,21 +1162,21 @@ async function readmitReady({ github, repo, listed: open, humans, bot, gate, app // Labels each issue `closed` blocked that is ready, each at most once per run, // within the gate's room, oldest opened first (owner Q95). Returns how many // reads or writes failed. -async function labelUnblocked({ github, repo, closed, sweep, humans, bot, gate, appId, admission, mirror = noMirror }) { +async function labelUnblocked({ github, repo, closed, sweep, humans, bot, gate, appId, admission, mirror = noMirror, closes = new Map() }) { const seen = new Map(); - const closes = new Map(); let failed = 0; for (const source of closed) { let blocked; try { - if (sweep && !(await trustedClose({ github, repo, number: source, humans, closes, bot }))) { - console.log(`label-unblocked: #${source} was closed by neither a listed human nor a merge; skipped`); - continue; - } - // A merge the App ran closes an issue with no close run, so the sweep - // strips its queue labels; a listed human's close run already did. if (sweep) { - failed += await stripClosed({ github, repo, number: source }); + const { issue, trusted } = await readClose({ github, repo, number: source, humans, closes, bot }); + if (!trusted) { + console.log(`label-unblocked: #${source} was closed by neither a listed human nor a merge; skipped`); + continue; + } + // A merge the App ran closes an issue with no close run, so the sweep + // strips its queue labels, from the issue the close check just read. + failed += await stripClosed({ github, repo, number: source, read: issue }); } blocked = await paginate(github, `/repos/${repo}/issues/${source}/dependencies/blocking`); } catch (error) { @@ -2081,8 +2089,7 @@ async function isTrustedClose(github, repo, issue, humans, bot) { // For each issue in `closed` (on the sweep, only one a listed human or a // merge closed), closes its split parent when it is done (closeSplitParent). // Returns how many reads or writes failed. -async function closeSplitParents({ github, repo, closed, sweep, humans, bot }) { - const closes = new Map(); +async function closeSplitParents({ github, repo, closed, sweep, humans, bot, closes = new Map() }) { let failed = 0; for (const number of closed) { try { @@ -2098,7 +2105,9 @@ async function closeSplitParents({ github, repo, closed, sweep, humans, bot }) { // Closes the open parent of issue `number` once its split is finished // (closeDoneParent). async function closeSplitParent({ github, repo, number, humans, closes, bot, sweep }) { - if (!(await github("GET", `/repos/${repo}/issues/${number}`))?.parent_issue_url || (sweep && !(await trustedClose({ github, repo, number, humans, closes, bot })))) { + // The sweep's close check already read the issue (readClose, cached in `closes`). + const { issue, trusted } = sweep ? await readClose({ github, repo, number, humans, closes, bot }) : { issue: await github("GET", `/repos/${repo}/issues/${number}`), trusted: true }; + if (!issue?.parent_issue_url || !trusted) { return; } await closeDoneParent({ github, repo, parent: await github("GET", `/repos/${repo}/issues/${number}/parent`), humans, closes, bot }); @@ -2287,10 +2296,12 @@ export async function main({ env = process.env, github, actions, jobLog, now = D const status = () => (read ??= checkWip(github, { repo, bot, now, limits, wip, remaining: rate.remaining })); const gate = admissionGate(status); let failed = sweep ? await admitDeferred({ github, repo, humans, bot, gate, appId: logins.get(bot), admission, mirror: mirrorTo }) : 0; - failed += await labelUnblocked({ github, repo, closed, sweep, humans, bot, gate, appId: logins.get(bot), admission, mirror: mirrorTo }); + // One close check per issue this run, shared by both passes below. + const closes = new Map(); + failed += await labelUnblocked({ github, repo, closed, sweep, humans, bot, gate, appId: logins.get(bot), admission, mirror: mirrorTo, closes }); // Before readmitReady, so a split parent whose last sub-issue closed is // closed, not decided, on the sweep that sees that close. - failed += await closeSplitParents({ github, repo, closed, sweep, humans, bot }); + failed += await closeSplitParents({ github, repo, closed, sweep, humans, bot, closes }); // The sweep marker carries no queue labels, so a kick skips this. if (!sweep) { failed += await stripClosed({ github, repo, number: Number(event.issue.number) }); diff --git a/.github/scripts/work-unblock/label-unblocked.test.mjs b/.github/scripts/work-unblock/label-unblocked.test.mjs index 60d23dccc05..2b4a2ad4bb0 100644 --- a/.github/scripts/work-unblock/label-unblocked.test.mjs +++ b/.github/scripts/work-unblock/label-unblocked.test.mjs @@ -763,6 +763,8 @@ test("the sweep strips the queue labels from a merge-closed issue, agent-ready l const labeled = world({ closedList: [mergeClosed(["agent-ready", "status: ready", "priority: low"])], blocking: [] }); assert.equal(await sweep({ github: labeled.github }), 0); assert.deepEqual(writes(labeled.calls), [`DELETE /repos/${REPO}/issues/10/labels/status%3A%20ready`, `DELETE /repos/${REPO}/issues/10/labels/agent-ready`]); + // The strip reuses the close check's read: one GET of the issue in all. + assert.equal(labeled.calls.filter((call) => call === `GET /repos/${REPO}/issues/10`).length, 1, labeled.calls.join("\n")); const bare = world({ closedList: [mergeClosed(["priority: low"])], blocking: [] }); assert.equal(await sweep({ github: bare.github }), 0); assert.deepEqual(writes(bare.calls), []); From 7f5543ba37b615165f7c05680fa5c7b3794399e0 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 3 Nov 2026 11:54:23 -0500 Subject: [PATCH 5/6] fix(ci): strip queue labels before the App closes an issue report-outcome's close route and closeDoneParent stripped the queue labels after their PATCH, so a failed label DELETE (not resent on a 5xx) left a closed issue with agent-ready that no sweep reads. Both now strip first; a failed removal fails the run with the issue still open and agent-ready on it, for work-unblock's sweep. Co-Authored-By: Claude Opus 5.5 --- .../work-resolve-issue/report-outcome.mjs | 15 ++++--- .../report-outcome.test.mjs | 40 ++++++++++++------- .../scripts/work-unblock/label-unblocked.mjs | 7 ++-- .../work-unblock/label-unblocked.test.mjs | 24 +++++++++-- 4 files changed, 59 insertions(+), 27 deletions(-) diff --git a/.github/scripts/work-resolve-issue/report-outcome.mjs b/.github/scripts/work-resolve-issue/report-outcome.mjs index acd837edc28..da3a200a630 100755 --- a/.github/scripts/work-resolve-issue/report-outcome.mjs +++ b/.github/scripts/work-resolve-issue/report-outcome.mjs @@ -668,7 +668,8 @@ const COUNTED = new Set([...KEPT, "intake"]); // issue and from one its lane PR handed back to triage. export const ADMISSION_LABELS = ["agent-ready", "status: ready", "admission-deferred"]; // Agent-ready last: work-unblock's sweep lists open agent-ready issues, so a -// failed write leaves the issue for it. +// failed write leaves the issue for it. A caller that closes the issue itself +// strips first, so the issue is still open when a write fails. const QUEUE_LABELS = [...ADMISSION_LABELS.filter((name) => name !== "agent-ready"), "agent-ready"]; // Removes the queue labels `had` holds from the issue at `base`, and returns @@ -2137,7 +2138,7 @@ const openPull = (github, repo, pr, { bot, humans }) => // carry over: intake triage re-judged it before admitting the issue again. A // retry or wait under the cap posts its comment once per run (a rerun of this // report finds its own marker) and keeps agent-ready. A close comments, -// removes agent-ready, then closes the issue as completed. +// removes the queue labels, then closes the issue as completed. async function finishStop(github, repo, plan, { bot, humans, runId, limits, lane = {} }) { const waited = plan.action === "wait" ? await openPull(github, repo, plan.pr, { bot, humans }) : ""; if (plan.action === "wait" && waited !== "open") { @@ -2198,13 +2199,15 @@ async function finishStop(github, repo, plan, { bot, humans, runId, limits, lane const base = `${repo}/issues/${plan.issueNumber}`; await sayWhy(github, repo.slice("/repos/".length), base, (await read()).comments, refusalText(plan.admission), bot); } - await applyHandBack(github, repo, plan); + // A close leaves its labels to stripQueueLabels below, which throws on a + // failed removal where applyHandBack only logs it. + await applyHandBack(github, repo, plan.action === "close" ? { ...plan, removeLabels: [] } : plan); if (plan.action === "close") { const base = `${repo}/issues/${plan.issueNumber}`; - const closed = await github("PATCH", base, { state: "closed", state_reason: "completed" }, { safeToResend: true }); // The App's close starts no work-unblock close run, so the queue labels - // still on the issue come off here, as read from the close's answer. - const removed = await stripQueueLabels(github, base, closed?.labels ?? []); + // come off here, before the close: a failed removal leaves the issue open. + const removed = await stripQueueLabels(github, base, (await github("GET", base))?.labels ?? []); + await github("PATCH", base, { state: "closed", state_reason: "completed" }, { safeToResend: true }); console.log(`report-outcome: closed issue #${plan.issueNumber} (${plan.stop}); removed ${removed.length} queue label(s)`); } } diff --git a/.github/scripts/work-resolve-issue/report-outcome.test.mjs b/.github/scripts/work-resolve-issue/report-outcome.test.mjs index f62bb35116d..d4dde58477b 100644 --- a/.github/scripts/work-resolve-issue/report-outcome.test.mjs +++ b/.github/scripts/work-resolve-issue/report-outcome.test.mjs @@ -4379,12 +4379,12 @@ test("route close: report-delivered retries on a resumed branch when the run com // main against a fake API for the routes whose effect needs a read first. // `trusted` adds the trusted-actor list, whose humans' pull requests a wait accepts. -const stopRun = async ({ reason, summary = "", routes, trusted = false }) => { +const stopRun = async ({ reason, summary = "", routes, trusted = false, issue = ISSUE[2] }) => { let result; const files = { "work-outcome.json": JSON.stringify({ exit: "stopped", reason, summary }), ...(trusted && { "trusted.json": TRUSTED_LIST }) }; await withAsyncDir(files, async (dir) => { const { fetch, calls } = fakeFetch([ - ["GET", /\/issues\/77$/, ...ISSUE.slice(1)], + ["GET", /\/issues\/77$/, 200, issue], ...routes, ["GET", /\/issues\/77\/comments/, 200, []], ["GET", /\/issues\/77\/events/, 200, []], @@ -4615,24 +4615,34 @@ test("main posts a delivered report and closes the issue", async () => { }); // The App's close starts no work-unblock close run, so the queue labels the -// close's answer still shows come off right after it, agent-ready last. -test("main's close removes the queue labels the closed issue still carries, after the close and agent-ready last", async () => { - const closedWith = (labels) => ["PATCH", /\/issues\/77$/, 200, { state: "closed", labels: labels.map((name) => ({ name })) }]; - const run = await stopRun({ reason: "report-delivered: the audit found three stale rows", routes: [closedWith(["status: ready", "admission-deferred", "agent-ready", "priority: low"])] }); +// issue carries come off before the close, agent-ready last. +const withLabels = (...names) => ({ ...ISSUE[2], labels: names.map((name) => ({ name })) }); +const labelWrites = (calls) => calls.filter((c) => c.method === "DELETE" || c.method === "PATCH").map((c) => `${c.method} ${c.path.replace(/^.*\/issues\/77/, "")}`); +test("main's close removes the queue labels the issue carries, before the close and agent-ready last", async () => { + const reason = "report-delivered: the audit found three stale rows"; + const run = await stopRun({ reason, routes: [], issue: withLabels("status: ready", "admission-deferred", "agent-ready", "priority: low") }); assert.equal(run.code, 0); - const after = run.calls.slice(run.calls.findIndex((c) => c.method === "PATCH") + 1).map((c) => `${c.method} ${c.path.replace(/^.*\/issues\/77/, "")}`); - assert.deepEqual(after, ["DELETE /labels/status%3A%20ready", "DELETE /labels/admission-deferred", "DELETE /labels/agent-ready"]); - // None left on the issue: nothing more is written after the close. - const bare = await stopRun({ reason: "report-delivered: the audit found three stale rows", routes: [closedWith(["priority: low"])] }); + assert.deepEqual(labelWrites(run.calls), ["DELETE /labels/status%3A%20ready", "DELETE /labels/admission-deferred", "DELETE /labels/agent-ready", "PATCH "]); + // None on the issue: the close is the only write. + const bare = await stopRun({ reason, routes: [], issue: withLabels("priority: low") }); assert.equal(bare.code, 0); - assert.deepEqual(bare.calls.slice(bare.calls.findIndex((c) => c.method === "PATCH") + 1), []); + assert.deepEqual(labelWrites(bare.calls), ["PATCH "]); // A label already gone (404) counts as removed. - const gone = await stopRun({ + const gone = await stopRun({ reason, routes: [["DELETE", /\/labels\/status%3A%20ready$/, 404, {}]], issue: withLabels("status: ready", "agent-ready") }); + assert.equal(gone.code, 0); + assert.deepEqual(labelWrites(gone.calls), ["DELETE /labels/status%3A%20ready", "DELETE /labels/agent-ready", "PATCH "]); +}); + +// A label DELETE is not resent, so a 5xx on one fails the run before the +// close: the issue stays open with agent-ready, for work-unblock's sweep. +test("main's close leaves the issue open when a queue-label removal fails", async () => { + const run = await stopRun({ reason: "report-delivered: the audit found three stale rows", - routes: [closedWith(["status: ready", "agent-ready"]), ["DELETE", /\/labels\/status%3A%20ready$/, 404, {}]], + routes: [["DELETE", /\/labels\/status%3A%20ready$/, 502, {}]], + issue: withLabels("status: ready", "agent-ready"), }); - assert.equal(gone.code, 0); - assert.ok(gone.calls.some((c) => c.method === "DELETE" && c.path.endsWith("/77/labels/agent-ready"))); + assert.notEqual(run.code, 0); + assert.deepEqual(labelWrites(run.calls), ["DELETE /labels/status%3A%20ready"]); }); test("retries count the App's retry markers since a listed human last added agent-ready, not this run's", () => { diff --git a/.github/scripts/work-unblock/label-unblocked.mjs b/.github/scripts/work-unblock/label-unblocked.mjs index 7480911bac3..bf77b93ce6e 100755 --- a/.github/scripts/work-unblock/label-unblocked.mjs +++ b/.github/scripts/work-unblock/label-unblocked.mjs @@ -14,7 +14,7 @@ // (agent-ready, `status: ready`, admission-deferred; stripClosed), only those // present, unless it is open again or is a post-merge fix issue. A bot's close // runs no close path: where the lanes App closes an issue itself (a split -// parent here, report-outcome's close route) it strips them after the close, +// parent here, report-outcome's close route) it strips them before the close, // and the sweep strips each issue in its window a listed human or a merge // closed (a merge the App ran among them). // @@ -2170,9 +2170,10 @@ async function closeDoneParent({ github, repo, parent, humans, closes, bot }) { body: `${SPLIT_CLOSED}\nEvery sub-issue of this split is closed as completed by a listed human or a merge (${children.map((child) => `#${child.number}`).join(", ")}), so the lanes App closes it as completed.`, }); } - await github("PATCH", base, { state: "closed", state_reason: "completed" }); - // The App's close starts no close run, so its queue labels come off here. + // The App's close starts no close run, so its queue labels come off here, + // before the close: a failed removal leaves the parent open. const removed = await stripQueueLabels(github, base, labelNames(parent)); + await github("PATCH", base, { state: "closed", state_reason: "completed" }); console.log(`label-unblocked: split parent #${parent.number} closed as completed; removed ${removed.length} queue label(s)`); } diff --git a/.github/scripts/work-unblock/label-unblocked.test.mjs b/.github/scripts/work-unblock/label-unblocked.test.mjs index 2b4a2ad4bb0..e6c736c0c70 100644 --- a/.github/scripts/work-unblock/label-unblocked.test.mjs +++ b/.github/scripts/work-unblock/label-unblocked.test.mjs @@ -4571,17 +4571,35 @@ test("the last sub-issue closing by a listed human or a merge closes its split p }); // The App's close starts no close run, so the split parent's queue labels -// come off right after it: only those present, agent-ready last. -test("closing a split parent removes the queue labels it carries, agent-ready last", async () => { +// come off before it: only those present, agent-ready last. +test("closing a split parent removes the queue labels it carries before the close, agent-ready last", async () => { const { github, calls } = splitWorld({ parent: issue(30, { labels: ["agent-ready", "status: ready", "priority: high"] }) }); assert.equal(await sweep({ github }), 0); assert.deepEqual(writtenPaths(calls), [ - ...closeParentWrites, + closeParentWrites[0], `DELETE /repos/${REPO}/issues/30/labels/status%3A%20ready`, `DELETE /repos/${REPO}/issues/30/labels/agent-ready`, + closeParentWrites[1], ]); }); +// A failed removal stops the close, so the parent stays open with agent-ready +// for the next sweep. +test("a failed queue-label removal leaves the split parent open", async (t) => { + const { github: inner, calls } = splitWorld({ parent: issue(30, { labels: ["agent-ready", "status: ready"] }) }); + const github = async (method, path, body) => { + if (method === "DELETE" && path.endsWith("/issues/30/labels/status%3A%20ready")) { + calls.push(`${method} ${path}`); + throw Object.assign(new Error("DELETE failed with status 502"), { status: 502 }); + } + return inner(method, path, body); + }; + const log = t.mock.method(console, "log", () => {}); + assert.equal(await sweep({ github }), 1); + log.mock.restore(); + assert.deepEqual(writtenPaths(calls), [closeParentWrites[0], `DELETE /repos/${REPO}/issues/30/labels/status%3A%20ready`]); +}); + test("a split parent stays open while a sub-issue is open, or closed by someone else, or with no split record from the App", async () => { const outsider = { id: 4242, login: "someone-else", type: "User" }; for (const setup of [ From 911b5906235da2f63fc1da601503378ad8fdf18d Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Tue, 3 Nov 2026 13:04:14 -0500 Subject: [PATCH 6/6] fix(ci): resend the close route's queue-label removals after a 5xx A label DELETE is idempotent and a 404 already counts as removed, so the close route sends its removals as safeToResend. One transient 5xx no longer fails a finished stop and sends the issue back for another lane run; a removal that keeps failing still fails the run before the close. Co-Authored-By: Claude Opus 5.5 --- .../scripts/work-resolve-issue/report-outcome.mjs | 5 ++++- .../work-resolve-issue/report-outcome.test.mjs | 12 +++++++----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/.github/scripts/work-resolve-issue/report-outcome.mjs b/.github/scripts/work-resolve-issue/report-outcome.mjs index 24ed562074c..0988aa22970 100755 --- a/.github/scripts/work-resolve-issue/report-outcome.mjs +++ b/.github/scripts/work-resolve-issue/report-outcome.mjs @@ -2225,7 +2225,10 @@ async function finishStop(github, repo, plan, { bot, humans, runId, limits, lane const base = `${repo}/issues/${plan.issueNumber}`; // The App's close starts no work-unblock close run, so the queue labels // come off here, before the close: a failed removal leaves the issue open. - const removed = await stripQueueLabels(github, base, (await github("GET", base))?.labels ?? []); + // A label DELETE is idempotent (a 404 counts as removed), so it is resent + // after a 5xx; only a removal that keeps failing leaves the issue open. + const resend = (method, path) => github(method, path, undefined, { safeToResend: true }); + const removed = await stripQueueLabels(resend, base, (await github("GET", base))?.labels ?? []); await github("PATCH", base, { state: "closed", state_reason: "completed" }, { safeToResend: true }); console.log(`report-outcome: closed issue #${plan.issueNumber} (${plan.stop}); removed ${removed.length} queue label(s)`); } diff --git a/.github/scripts/work-resolve-issue/report-outcome.test.mjs b/.github/scripts/work-resolve-issue/report-outcome.test.mjs index 0f0a4819885..d6ae9a566ae 100644 --- a/.github/scripts/work-resolve-issue/report-outcome.test.mjs +++ b/.github/scripts/work-resolve-issue/report-outcome.test.mjs @@ -4734,16 +4734,18 @@ test("main's close removes the queue labels the issue carries, before the close assert.deepEqual(labelWrites(gone.calls), ["DELETE /labels/status%3A%20ready", "DELETE /labels/agent-ready", "PATCH "]); }); -// A label DELETE is not resent, so a 5xx on one fails the run before the -// close: the issue stays open with agent-ready, for work-unblock's sweep. -test("main's close leaves the issue open when a queue-label removal fails", async () => { +// A label DELETE is idempotent, so a 5xx is resent (three tries in all); one +// that keeps failing fails the run before the close: the issue stays open with +// agent-ready, for work-unblock's sweep. +const FAILED_STRIP = Array(3).fill("DELETE /labels/status%3A%20ready"); +test("main's close resends a failed queue-label removal, then leaves the issue open", async () => { const run = await stopRun({ reason: "report-delivered: the audit found three stale rows", routes: [["DELETE", /\/labels\/status%3A%20ready$/, 502, {}]], issue: withLabels("status: ready", "agent-ready"), }); assert.notEqual(run.code, 0); - assert.deepEqual(labelWrites(run.calls), ["DELETE /labels/status%3A%20ready"]); + assert.deepEqual(labelWrites(run.calls), FAILED_STRIP); }); // A verified-done parent closes through the same route, so its queue labels @@ -4755,7 +4757,7 @@ test("main's verified-done close removes the queue labels before the close, and assert.deepEqual(labelWrites(run.calls), ["DELETE /labels/status%3A%20ready", "DELETE /labels/agent-ready", "PATCH "]); const failed = await stopRun({ ...opts, routes: [subs(doneSub(80), doneSub(81)), ["DELETE", /\/labels\/status%3A%20ready$/, 502, {}]] }); assert.notEqual(failed.code, 0); - assert.deepEqual(labelWrites(failed.calls), ["DELETE /labels/status%3A%20ready"]); + assert.deepEqual(labelWrites(failed.calls), FAILED_STRIP); }); test("retries count the App's retry markers since a listed human last added agent-ready, not this run's", () => {