diff --git a/.github/scripts/work-resolve-issue/report-outcome.mjs b/.github/scripts/work-resolve-issue/report-outcome.mjs index f1d0c0823b2..0988aa22970 100755 --- a/.github/scripts/work-resolve-issue/report-outcome.mjs +++ b/.github/scripts/work-resolve-issue/report-outcome.mjs @@ -673,7 +673,28 @@ 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 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. 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 +// 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.`, @@ -1886,7 +1907,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(); }; @@ -2127,7 +2148,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 = {}, verifyClose = "" }) { const waited = plan.action === "wait" ? await openPull(github, repo, plan.pr, { bot, humans }) : ""; if (plan.action === "wait" && waited !== "open") { @@ -2197,10 +2218,19 @@ 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") { - 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}`; + // 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. + // 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 8431d71e9f7..d6ae9a566ae 100644 --- a/.github/scripts/work-resolve-issue/report-outcome.test.mjs +++ b/.github/scripts/work-resolve-issue/report-outcome.test.mjs @@ -4380,12 +4380,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, extraEnv = {}, extraFiles = {} }) => { +const stopRun = async ({ reason, summary = "", routes, trusted = false, issue = ISSUE[2], extraEnv = {}, extraFiles = {} }) => { let result; const files = { "work-outcome.json": JSON.stringify({ exit: "stopped", reason, summary }), ...(trusted && { "trusted.json": TRUSTED_LIST }), ...extraFiles }; 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, []], @@ -4715,6 +4715,51 @@ 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 +// 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); + 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(labelWrites(bare.calls), ["PATCH "]); + // A label already gone (404) counts as removed. + 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 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), FAILED_STRIP); +}); + +// A verified-done parent closes through the same route, so its queue labels +// come off first too, and a failed removal leaves it open. +test("main's verified-done close removes the queue labels before the close, and stays open when one fails", async () => { + const opts = { reason: VERIFIED, extraEnv: { VERIFY_CLOSE: "80,81" }, issue: withLabels("status: ready", "agent-ready") }; + const run = await stopRun({ ...opts, routes: [subs(doneSub(80), doneSub(81))] }); + assert.equal(run.code, 0); + 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), FAILED_STRIP); +}); + 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 16925129b3f..257f1c77443 100755 --- a/.github/scripts/work-unblock/label-unblocked.mjs +++ b/.github/scripts/work-unblock/label-unblocked.mjs @@ -10,6 +10,14 @@ // 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 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 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). +// // 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: @@ -281,7 +289,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 { 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, confirmedOnly, decide, deferralLapsed, loadRules, NEEDS_DECISION, openPullsFor, openSubIssues, READY, refusalText, removalStands, sayWhy, SIZE_SPLIT_DONE, SIZE_SPLIT_PENDING, specOf, stillBlocking, subIssueWait, UNTRUSTED, untrustedHold } from "../intake-triage/admit.mjs"; import { subIssueLeftOpen } from "../lib/done-evidence.mjs"; @@ -609,14 +617,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 }; })(), ); } @@ -861,6 +876,34 @@ const noMirror = async () => {}; // A label change as the status mirror takes it (write-status.mjs statusMirror). 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; 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, read }) { + const base = `/repos/${repo}/issues/${number}`; + try { + 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; + } + 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 @@ -1123,16 +1166,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; + if (sweep) { + 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) { @@ -1983,13 +2031,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; @@ -2050,8 +2093,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, mirror = noMirror }) { - const closes = new Map(); +async function closeSplitParents({ github, repo, closed, sweep, humans, bot, closes = new Map(), mirror = noMirror }) { let failed = 0; for (const number of closed) { try { @@ -2067,7 +2109,9 @@ async function closeSplitParents({ github, repo, closed, sweep, humans, bot, mir // Closes the open parent of issue `number` once its split is finished // (closeDoneParent). async function closeSplitParent({ github, repo, number, humans, closes, bot, sweep, mirror }) { - 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, mirror }); @@ -2132,8 +2176,11 @@ async function closeDoneParent({ github, repo, parent, humans, closes, bot, mirr 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.`, }); } + // 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`); + console.log(`label-unblocked: split parent #${parent.number} closed as completed; removed ${removed.length} queue label(s)`); } // Takes status: confirmed off `parent` (an open issue as read, with its @@ -2274,10 +2321,16 @@ 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, mirror: mirrorTo }); + failed += await closeSplitParents({ github, repo, closed, sweep, humans, bot, closes, mirror: mirrorTo }); + // 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 076c0fac9c9..a5c9ffba558 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] }); @@ -686,6 +756,24 @@ 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`]); + // 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), []); + // 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); @@ -2953,7 +3041,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 +3193,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 +3327,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) => { @@ -4469,6 +4570,36 @@ 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 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[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 [ diff --git a/.github/workflows/work-unblock.yml b/.github/workflows/work-unblock.yml index 002568e03b6..00f2cbc2640 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.