diff --git a/.changes/pr-pipeline/feat-hitl-admission-mark-stranded-check-02acbfe9.md b/.changes/pr-pipeline/feat-hitl-admission-mark-stranded-check-02acbfe9.md new file mode 100644 index 0000000000..5d292efb03 --- /dev/null +++ b/.changes/pr-pipeline/feat-hitl-admission-mark-stranded-check-02acbfe9.md @@ -0,0 +1,13 @@ +--- +bump: minor +--- + +### Added + +- A listed human adding `admission-deferred` to a decided issue with `status: ready` now has the hourly work-unblock sweep run intake's admission rules on its own work-class label and Size, Complexity and Uncertainty fields, each counting only when a listed human or the lanes App added or set it, admitting it when there is room and refusing it to `status: needs-decision` with a comment otherwise. +- The sweep now re-fires an `agent-ready` issue left with no run and no report for 3 hours, once per version of the lane workflow files, and then hands it to a person with `needs-human` and a comment. + +### Changed + +- Intake admission now reads the `- [ ]` acceptance criteria only from text a listed human wrote, in the body or a comment of their own, since pr-verify judges a lane pull request only against that text (owner decision Q9). The lanes App's refinement comment no longer satisfies the criteria check; a person who confirms it posts the criteria as their own comment. +- Intake admission now refuses an issue with more than 100 comments, since pr-verify reads only the first 100 and stops on such an issue. diff --git a/.github/scripts/intake-triage/admit.mjs b/.github/scripts/intake-triage/admit.mjs index a07941a432..be296b194d 100644 --- a/.github/scripts/intake-triage/admit.mjs +++ b/.github/scripts/intake-triage/admit.mjs @@ -13,10 +13,13 @@ // parent issue a listed human filed that awaits no decision; // - 3: the class, Size and Uncertainty apply-verdict wrote are in `classes`, // `sizes` and `uncertainty`; -// - 7: the body, or a comment the lanes App's account posted opening with -// REFINE_MARKER (refine.mjs, which writes a task list only for an issue -// it made exact) after the body's last edit, has a `task-list` line, the -// criteria the lane checks; +// - 7 (with owner decision Q9, #8999): a `task-list` line, the criteria the +// lane checks, in text pr-verify will read: the body or a comment written, +// and last edited if ever, by a listed human (resolve-pr.mjs linkedIssues). +// Criteria only the lanes App wrote (refine.mjs's refinement comment, an +// App-filed issue) do not count, since pr-verify never sees them, and an +// issue with more comments than the 100 pr-verify reads is refused, since +// pr-verify stops on it; // - 8: no `refuse-labels` label, every blocker a merged pull request or an // issue closed as completed by a listed human or a merge here (the close // work-unblock counts), and no open pull request in this repository is for @@ -41,7 +44,8 @@ // skips the issue while such a pull request is open and admits it once none // is. An issue that waits only on a blocker loses `admission-deferred`, and // work-unblock admits it when its blocker closes. A refused issue loses it -// too, so the sweep, which does not read these rules, cannot admit it. An +// too. The sweep runs these rules itself only on an issue a listed human +// marked (added admission-deferred to an issue with `status: ready`). An // issue already agent-ready, or whose agent-ready was ever removed, is left // alone. Issue text can only // refuse: it never satisfies a criterion the fields, labels or API decide. @@ -55,8 +59,8 @@ export const DEFERRED = "admission-deferred"; export const ADMIT_MARKER = ""; // Opens the lanes App's refinement comment (refine.mjs). export const REFINE_MARKER = ""; -const READY = "status: ready"; -const NEEDS_DECISION = "status: needs-decision"; +export const READY = "status: ready"; +export const NEEDS_DECISION = "status: needs-decision"; const LANES_APP_LOGIN = "melodic-automation-lanes[bot]"; const PARENT_HOLDS = new Set(["needs-human", "needs-owner-decision", NEEDS_DECISION]); const LISTS = ["types", "types-under-parent", "classes", "sizes", "uncertainty", "refuse-labels"]; @@ -253,7 +257,7 @@ export function admissionSettled(labels, events) { } // {verdict: admit | refuse | wait | skip, reasons}. Pure. -export function decide({ issue, verdict, rules, parent, blockers, trustedCloses, pulls = [], events, comments = [], bodyEditedAt = null, humans, appId, repository, webHosts }) { +export function decide({ issue, verdict, rules, parent, blockers, trustedCloses, pulls = [], events, spec, humans, appId, repository, webHosts }) { const labels = names(issue.labels); if (!labels.includes(READY)) { return { verdict: "skip", reasons: [`no ${READY}`] }; @@ -287,16 +291,18 @@ export function decide({ issue, verdict, rules, parent, blockers, trustedCloses, if (!rules.uncertainty.includes(verdict.dimensions?.uncertainty)) { refuse.push(`Uncertainty ${verdict.dimensions?.uncertainty} is not one of ${rules.uncertainty.join(", ")}`); } - // A refinement drafted before the body's last edit may not fit the body now. - const refinements = refinementsOf(comments, appId) - .filter((comment) => bodyEditedAt === null || Date.parse(comment.created_at) >= Date.parse(bodyEditedAt)) - .map((comment) => String(comment.body)); - const lines = [String(issue.body ?? ""), ...refinements].flatMap((text) => text.split("\n")); - if (!lines.some((line) => line.trimStart().startsWith(rules["task-list"]))) { - refuse.push(`it has no \`${rules["task-list"]}\` task list to check`); + // pr-verify judges a lane PR only against text a listed human wrote, so + // the criteria count only there, and it stops on an issue with more + // comments than it reads (resolve-pr.mjs linkedIssues). + if ((spec?.commentCount ?? 0) > (spec?.comments?.length ?? 0)) { + refuse.push(`it has ${spec.commentCount} comments, more than the ${SPEC_COMMENTS} pr-verify reads, so pr-verify cannot verify a pull request for it`); } - // A refinement breaks `://` and `www.` with a zero-width space so no link - // renders; the check reads the URL as written. + const lines = trustedSpecTexts(spec, humans).flatMap((text) => text.split("\n")); + if (!hasTrustedCriteria(spec, humans, rules["task-list"])) { + refuse.push(noTrustedSpec(rules["task-list"])); + } + // A copied refinement breaks `://` and `www.` with a zero-width space so no + // link renders; the check reads the URL as written. const unrunnable = unrunnableReason(unrunnableCriteria(lines.join("\n").replaceAll("\u200b", ""), rules["task-list"], webHosts)); if (unrunnable) { refuse.push(unrunnable); @@ -343,7 +349,7 @@ async function parentOf(github, base) { } // One App comment carrying ADMIT_MARKER, edited in place on a later run. -async function sayWhy(github, repository, base, comments, text) { +export async function sayWhy(github, repository, base, comments, text) { const body = `${ADMIT_MARKER}\n${text}`; const own = comments.find( (comment) => comment.user?.login === LANES_APP_LOGIN && String(comment.body ?? "").startsWith(ADMIT_MARKER), @@ -360,23 +366,50 @@ async function sayWhy(github, repository, base, comments, text) { } } -const refinementsOf = (comments, appId) => - comments.filter((comment) => appId !== undefined && comment?.user?.id === appId && String(comment.body ?? "").startsWith(REFINE_MARKER)); - -const BODY_EDIT_QUERY = `query($owner: String!, $name: String!, $number: Int!) { - repository(owner: $owner, name: $name) { issue(number: $number) { lastEditedAt } } -}`; +// The body, the comment count and the first 100 comments, each with its +// author, last editor and edit time: the first 100 comments are what pr-verify +// reads, and it stops on an issue with more. +const SPEC_COMMENTS = 100; +const SPEC_QUERY = `query($owner: String!, $name: String!, $number: Int!) { + repository(owner: $owner, name: $name) { issue(number: $number) { + body lastEditedAt author { ...actor } editor { ...actor } + comments(first: ${SPEC_COMMENTS}) { totalCount nodes { body lastEditedAt author { ...actor } editor { ...actor } } } + } } +} +fragment actor on Actor { __typename ... on User { databaseId } ... on Bot { databaseId } }`; -// When the issue's body was last edited, or null when never. An unexpected shape throws. -async function bodyEditedAtOf(github, repository, number) { +// {body, lastEditedAt, author, editor, commentCount, comments: [{body, +// lastEditedAt, author, editor}]} for the issue, as GraphQL answers them. An +// unexpected shape throws. +export async function specOf(github, repository, number) { const [owner, name] = repository.split("/"); - const issue = (await github("POST", "/graphql", { query: BODY_EDIT_QUERY, variables: { owner, name, number: Number(number) } }))?.data?.repository?.issue; - if (issue === null || typeof issue !== "object" || (issue.lastEditedAt !== null && typeof issue.lastEditedAt !== "string")) { - throw new Error("the issue's body edit time has an unexpected shape"); + const issue = (await github("POST", "/graphql", { query: SPEC_QUERY, variables: { owner, name, number: Number(number) } }))?.data?.repository?.issue; + const text = (item) => + item !== null && typeof item === "object" && typeof item.body === "string" && (item.lastEditedAt === null || typeof item.lastEditedAt === "string"); + if (!text(issue) || !Array.isArray(issue.comments?.nodes) || !issue.comments.nodes.every(text) || !Number.isSafeInteger(issue.comments.totalCount)) { + throw new Error("the issue's text, authors and edit times have an unexpected shape"); } - return issue.lastEditedAt; + const { body, lastEditedAt, author, editor } = issue; + return { body, lastEditedAt, author, editor, commentCount: issue.comments.totalCount, comments: issue.comments.nodes }; } +export const noTrustedSpec = (taskList) => + `no trusted-human acceptance criteria: a listed human must put the confirmed \`${taskList.trim()}\` criteria in the body or post them as their own comment (owner Q9, #8999)`; + +// The texts pr-verify will read as spec: the body and each comment written, +// and last edited if ever, by a User among `humans` +// (.github/scripts/pr-verify/resolve-pr.mjs linkedIssues). Bots and Apps, the +// lanes App among them, never count. Pure. +export function trustedSpecTexts(spec, humans) { + const trusted = (actor) => actor?.__typename === "User" && humans.has(actor.databaseId); + const written = (item) => trusted(item?.author) && (item.lastEditedAt == null || trusted(item.editor)); + return spec ? [spec, ...(spec.comments ?? [])].filter(written).map((item) => String(item.body ?? "")) : []; +} + +// Whether that text holds a `taskList` line: criterion 7. Pure. +export const hasTrustedCriteria = (spec, humans, taskList) => + trustedSpecTexts(spec, humans).some((text) => text.split("\n").some((line) => line.trimStart().startsWith(taskList))); + // For an issue apply-verdict held without admission (a flag, an escalation or // a hold), the same split note a refusal posts, when a done-when item is one // the lane cannot perform, so the split survives a verdict that never reaches @@ -398,34 +431,51 @@ export async function noteUnrunnable({ github, repository, issue, rules, webHost return { outcome: "noted", reasons: [reason] }; } -// Reads the issue fresh, decides, and writes. `status` answers check-wip-cap's -// status for the issue lane. Returns {outcome, reasons}. -export async function admit({ github, repository, number, verdict, rules, humans, appId, status, webHosts, log = console.log }) { +// Every fact decide() reads but the verdict, the rules and the caller's own +// (humans, appId, repository, webHosts), for `issue` as just read. +// `isTrusted(number)` answers whether a closed blocker's close counts; by +// default it reads the blocker fresh. +export async function admissionFacts({ github, repository, number, issue, rules, humans, isTrusted }) { const base = `/repos/${repository}/issues/${number}`; - const issue = await github("GET", base); const blockers = await paginate(github, `${base}/dependencies/blocked_by`); // Anyone may close their own issue as completed, so a blocking issue counts // only with a trusted close, read fresh from the issue. + const trusted = + isTrusted ?? (async (blocker) => isTrustedClose(github, repository, await github("GET", `/repos/${repository}/issues/${blocker}`), humans)); const trustedCloses = new Set(); for (const blocker of blockers) { if (!blocker.pull_request && blocker.state === "closed" && blocker.state_reason === "completed" && here(blocker.repository_url, repository)) { - if (await isTrustedClose(github, repository, await github("GET", `/repos/${repository}/issues/${blocker.number}`), humans)) { + if (await trusted(blocker.number)) { trustedCloses.add(blocker.number); } } } - const comments = await paginate(github, `${base}/comments`); - const decision = decide({ + // A throw here stops the caller before decide(), so nothing is written. + const spec = await specOf(github, repository, number); + return { issue, - comments, - bodyEditedAt: refinementsOf(comments, appId).length > 0 ? await bodyEditedAtOf(github, repository, number) : null, - verdict, - rules, + // Read for the admission comment's own earlier copy (sayWhy). + comments: await paginate(github, `${base}/comments`), + spec, parent: rules["types-under-parent"].includes(issue.type?.name) ? await parentOf(github, base) : null, blockers, trustedCloses, pulls: await openPullsFor(github, repository, number), events: await paginate(github, `${base}/events`), + }; +} + +// Reads the issue fresh, decides, and writes. `status` answers check-wip-cap's +// status for the issue lane. Returns {outcome, reasons}. +export async function admit({ github, repository, number, verdict, rules, humans, appId, status, webHosts, log = console.log }) { + const base = `/repos/${repository}/issues/${number}`; + const issue = await github("GET", base); + const facts = await admissionFacts({ github, repository, number, issue, rules, humans }); + const { comments } = facts; + const decision = decide({ + ...facts, + verdict, + rules, humans, appId, repository, @@ -442,8 +492,10 @@ export async function admit({ github, repository, number, verdict, rules, humans return { outcome: "skipped", reasons: decision.reasons }; } if (decision.verdict === "refuse") { - await github("DELETE", `${base}/labels/${encodeURIComponent(READY)}`); + // The hold before status: ready comes off: a failed write never leaves + // admission-deferred on an issue with neither, which the sweep admits. await github("POST", `${base}/labels`, { labels: [NEEDS_DECISION] }); + await github("DELETE", `${base}/labels/${encodeURIComponent(READY)}`); await undefer(); await sayWhy(github, repository, base, comments, refusalText(reasons)); log(`admit: #${number} refused (${reasons})`); diff --git a/.github/scripts/intake-triage/admit.test.mjs b/.github/scripts/intake-triage/admit.test.mjs index b6f8ea08a0..10a4a33b2b 100644 --- a/.github/scripts/intake-triage/admit.test.mjs +++ b/.github/scripts/intake-triage/admit.test.mjs @@ -35,6 +35,13 @@ const issueOf = (overrides = {}) => ({ labels: [{ name: READY }, { name: "work-class: mechanical" }], ...overrides, }); +// Authors and editors as GraphQL answers them, for the trusted-spec criterion. +const USER = (databaseId) => ({ __typename: "User", databaseId }); +const APP_ACTOR = { __typename: "Bot", databaseId: APP }; +// A body or comment as SPEC_QUERY answers it; by default it carries BODY's task list. +const written = (author, { body = BODY, editor = null, lastEditedAt = editor ? "2026-10-20T00:00:00Z" : null } = {}) => ({ body, author, editor, lastEditedAt }); +// The issue's text as a listed human wrote it, unedited. +const humanSpec = (issue, comments = []) => ({ ...written(USER(HUMAN), { body: issue.body }), comments }); const VERDICT = { workClass: "work-class: mechanical", dimensions: { size: "small", complexity: "low", uncertainty: "known" }, @@ -52,10 +59,69 @@ const facts = (overrides = {}) => ({ appId: APP, webHosts: HOSTS, repository: "o/r", + spec: humanSpec(overrides.issue ?? issueOf()), ...overrides, }); describe("decide", () => { + // Owner decision Q9 (#8999): pr-verify judges a lane PR only against text a + // listed human wrote and last edited (resolve-pr.mjs linkedIssues), so an + // issue with no such body or comment fails verify at every head. + describe("trusted-human acceptance criteria", () => { + const appIssue = issueOf({ user: { id: APP } }); + const NO_SPEC = + "no trusted-human acceptance criteria: a listed human must put the confirmed `- [ ]` criteria in the body or post them as their own comment (owner Q9, #8999)"; + const goAhead = written(USER(HUMAN), { body: "Go ahead." }); + + test("an App-authored issue with criteria only in its body is refused", () => { + for (const spec of [{ ...written(APP_ACTOR), comments: [] }, { ...written(APP_ACTOR), comments: [written(APP_ACTOR), written(USER(OUTSIDER))] }]) { + assert.deepEqual(decide(facts({ issue: appIssue, spec })), { verdict: "refuse", reasons: [NO_SPEC] }); + } + }); + + test("an App-drafted task list with a listed human's comment that carries none is refused", () => { + assert.deepEqual(decide(facts({ issue: appIssue, spec: { ...written(APP_ACTOR), comments: [goAhead] } })), { verdict: "refuse", reasons: [NO_SPEC] }); + }); + + test("the same issue with a listed human's comment carrying the task list is admitted", () => { + for (const comment of [written(USER(HUMAN)), written(USER(HUMAN), { editor: USER(HUMAN) })]) { + assert.deepEqual(decide(facts({ issue: appIssue, spec: { ...written(APP_ACTOR), comments: [goAhead, comment] } })), { verdict: "admit", reasons: [] }); + } + // A human's comment the App last edited is no longer theirs. + const edited = decide(facts({ issue: appIssue, spec: { ...written(APP_ACTOR), comments: [written(USER(HUMAN), { editor: APP_ACTOR })] } })); + assert.deepEqual(edited.reasons, [NO_SPEC]); + }); + + test("a human-authored body last edited by the App is refused unless a human comment carries the task list", () => { + const body = written(USER(HUMAN), { editor: APP_ACTOR }); + assert.deepEqual(decide(facts({ spec: { ...body, comments: [] } })), { verdict: "refuse", reasons: [NO_SPEC] }); + assert.deepEqual(decide(facts({ spec: { ...body, comments: [goAhead] } })), { verdict: "refuse", reasons: [NO_SPEC] }); + assert.deepEqual(decide(facts({ spec: { ...body, comments: [written(USER(HUMAN))] } })), { verdict: "admit", reasons: [] }); + // No spec read at all fails closed. + assert.deepEqual(decide(facts({ spec: undefined })).reasons, [NO_SPEC]); + }); + + test("admit() writes nothing when the text read answers an unexpected shape", async () => { + for (const spec of ["no-issue", "no-body"]) { + const { github, calls } = fakeGitHub({ spec }); + await assert.rejects( + admit({ github, repository: "o/r", number: 7, verdict: VERDICT, rules: RULES, humans, appId: APP, webHosts: HOSTS, status: async () => ({}), log: () => {} }), + /unexpected shape/, + ); + assert.deepEqual(calls.filter((call) => call.method !== "GET" && call.path !== "/graphql"), [], spec); + } + }); + + test("admit() reads the authors and refuses an App-only spec to status: needs-decision", async () => { + const { github, calls } = fakeGitHub({ issue: appIssue, spec: { ...written(APP_ACTOR), comments: [] } }); + const result = await admit({ github, repository: "o/r", number: 7, verdict: VERDICT, rules: RULES, humans, appId: APP, webHosts: HOSTS, status: async () => assert.fail("a refusal reads no status"), log: () => {} }); + assert.deepEqual(result, { outcome: "refused", reasons: [NO_SPEC] }); + const writes = calls.filter((call) => call.method !== "GET" && call.path !== "/graphql").map((call) => `${call.method} ${call.path}`); + assert.deepEqual(writes, ["POST /repos/o/r/issues/7/labels", "DELETE /repos/o/r/issues/7/labels/status%3A%20ready", "POST /repos/o/r/issues/7/comments"]); + assert.deepEqual(calls.find((call) => call.method === "POST" && call.path.endsWith("/labels")).body, { labels: [NEEDS_DECISION] }); + }); + }); + test("admits a clear Task that meets every criterion", () => { assert.deepEqual(decide(facts()), { verdict: "admit", reasons: [] }); }); @@ -110,7 +176,7 @@ describe("decide", () => { /Size large/, ], ["criterion 3: a structural class", { verdict: { ...VERDICT, workClass: "work-class: structural" } }, /work-class: structural/], - ["criterion 7: no task list to check", { issue: issueOf({ body: "Make the README better." }) }, /no `- \[ \] ` task list/], + ["criterion 7: no task list to check", { issue: issueOf({ body: "Make the README better." }) }, /no trusted-human acceptance criteria: .*confirmed `- \[ \]` criteria/], [ "criterion 8: a duplicate label", { @@ -239,45 +305,50 @@ describe("decide", () => { assert.equal(result.verdict, "refuse"); }); - describe("criterion 7 from the lanes App's refinement comment", () => { + // The App's refinement comment is model text pr-verify never reads, so its + // task list admits nothing; a listed human who confirms it posts it as their own. + describe("criterion 7 and the lanes App's refinement comment", () => { const thin = issueOf({ body: "Make the README skill count right." }); - const refinement = (user, body = `${REFINE_MARKER}\n**Refinement.**\n\n- [ ] Line 3 says nineteen.`) => ({ id: 41, body, user, created_at: "2026-10-20T00:00:00Z" }); - const app = { id: APP, login: "melodic-automation-lanes[bot]", type: "Bot" }; + const refinement = (body = `${REFINE_MARKER}\n**Refinement.**\n\n- [ ] Line 3 says nineteen.`) => written(APP_ACTOR, { body }); + const NO_SPEC = /^no trusted-human acceptance criteria: /; + + test("a thin issue whose only task list is the App's refinement comment is refused", () => { + const result = decide(facts({ issue: thin, spec: humanSpec(thin, [refinement()]) })); + assert.equal(result.verdict, "refuse"); + assert.match(result.reasons.join(), NO_SPEC); + }); - test("admits a thin issue whose task list is in the App's refinement comment", () => { - assert.deepEqual(decide(facts({ issue: thin, comments: [refinement(app)] })), { verdict: "admit", reasons: [] }); + test("a listed human's comment carrying the confirmed criteria admits the thin issue", () => { + const confirmed = written(USER(HUMAN), { body: "Confirmed:\n\n- [ ] Line 3 says nineteen." }); + assert.deepEqual(decide(facts({ issue: thin, spec: humanSpec(thin, [refinement(), confirmed]) })), { verdict: "admit", reasons: [] }); }); - test("a rendered refinement criterion that reads a page outside the lane's web hosts is still refused", () => { + test("a rendered refinement a listed human posts as their own is still checked for pages outside the lane's web hosts", () => { const output = { exact: true, outcome: "Match the spec.", criteria: ["The page at https://example.org/spec says nineteen."], dependencies: [], related: [], questions: [], sources: [], split: [] }; const body = render(validateOutput(output, new Set())); assert.ok(!body.includes("https://"), "the rendered comment breaks the URL"); - const result = decide(facts({ issue: thin, comments: [refinement(app, body)] })); + const result = decide(facts({ issue: thin, spec: humanSpec(thin, [written(USER(HUMAN), { body })]) })); assert.equal(result.verdict, "refuse"); assert.match(result.reasons.join(), /example\.org/); }); - const lookalikes = [ - ["github-actions[bot]", refinement({ id: 41898282, login: "github-actions[bot]", type: "Bot" })], - ["a listed human", refinement({ id: HUMAN, login: "maintainer", type: "User" })], - ["an outsider using the App's login text", refinement({ id: OUTSIDER, login: "melodic-automation-lanes[bot]", type: "User" })], - ["the App, with the marker mid-body", refinement(app, `See below.\n${REFINE_MARKER}\n- [ ] Line 3 says nineteen.`)], - ["the App, with only a quoted task list", refinement(app, `${REFINE_MARKER}\n> - [ ] Line 3 says nineteen.`)], - ["the App's admission comment", refinement(app, `${ADMIT_MARKER}\n- [ ] Line 3 says nineteen.`)], - ]; - test("a refinement older than the body's last edit does not count; one newer does", () => { - const before = decide(facts({ issue: thin, comments: [refinement(app)], bodyEditedAt: "2026-10-21T00:00:00Z" })); - assert.equal(before.verdict, "refuse"); - assert.match(before.reasons.join(), /task list/); - const after = decide(facts({ issue: thin, comments: [refinement(app)], bodyEditedAt: "2026-10-19T00:00:00Z" })); - assert.equal(after.verdict, "admit"); + test("criteria past the comments read do not count: more comments than read is a refusal", () => { + const spec = { ...humanSpec(thin, [written(USER(HUMAN))]), commentCount: 150 }; + const result = decide(facts({ issue: thin, spec })); + assert.equal(result.verdict, "refuse"); + assert.match(result.reasons.join(), /150 comments, more than the 100 pr-verify reads/); + assert.equal(decide(facts({ issue: thin, spec: { ...spec, commentCount: 1 } })).verdict, "admit"); }); - for (const [name, comment] of lookalikes) { - test(`a look-alike from ${name} does not count`, () => { - const result = decide(facts({ issue: thin, comments: [comment] })); + for (const [name, author] of [ + ["github-actions[bot]", { __typename: "Bot", databaseId: 41898282 }], + ["an outsider", USER(OUTSIDER)], + ["the App", APP_ACTOR], + ]) { + test(`a task list from ${name} does not count`, () => { + const result = decide(facts({ issue: thin, spec: humanSpec(thin, [written(author)]) })); assert.equal(result.verdict, "refuse"); - assert.match(result.reasons.join(), /task list/); + assert.match(result.reasons.join(), NO_SPEC); }); } }); @@ -285,17 +356,27 @@ describe("decide", () => { // A fake GitHub for admit(): the issue, its parent, blockers, timeline, // events and comments, recording every call. -function fakeGitHub({ issue = issueOf(), parent = null, blockers = [], closed = {}, timeline = [], closing = [], events = [], comments = [], bodyEditedAt = null } = {}) { +// `spec` is what SPEC_QUERY answers (by default the issue's body as a listed +// human wrote it); "no-issue" and "no-body" answer an unexpected shape. +// `closing` is what closedByPullRequestsReferences answers. +function fakeGitHub({ issue = issueOf(), parent = null, blockers = [], closed = {}, timeline = [], closing = [], events = [], comments = [], spec, failOn } = {}) { const calls = []; const github = async (method, path, body) => { calls.push({ method, path, body }); + if (failOn?.(method, path, body)) { + throw new Error(`${method} ${path} failed with status 500`); + } const base = "/repos/o/r/issues/7"; if (method === "POST" && path === "/graphql") { if (body.query.includes("closedByPullRequestsReferences")) { const refs = { nodes: closing, pageInfo: { hasNextPage: false, endCursor: null } }; return { data: { repository: { issue: { closedByPullRequestsReferences: refs } } } }; } - return { data: { repository: { issue: { lastEditedAt: bodyEditedAt } } } }; + if (spec === "no-issue") { + return { data: { repository: { issue: null } } }; + } + const answer = spec === "no-body" ? { ...humanSpec(issue), body: null } : (spec ?? humanSpec(issue)); + return { data: { repository: { issue: { ...answer, comments: { totalCount: answer.totalCount ?? answer.comments.length, nodes: answer.comments } } } } }; } if (method !== "GET") { return null; @@ -350,6 +431,7 @@ const go = (fake, { status = open } = {}) => status: async () => status, log: () => {}, }); +// The GraphQL reads are POSTs but write nothing. const writes = (calls) => calls.filter((call) => call.method !== "GET" && call.path !== "/graphql").map(({ method, path, body }) => `${method} ${path} ${JSON.stringify(body)}`); describe("admit", () => { @@ -398,11 +480,11 @@ describe("admit", () => { assert.equal(result.outcome, "refused"); const done = writes(fake.calls); assert.equal(done.length, 3); - assert.match(done[0], /^DELETE \/repos\/o\/r\/issues\/7\/labels\/status%3A%20ready/); - assert.equal(done[1], `POST /repos/o/r/issues/7/labels ${JSON.stringify({ labels: [NEEDS_DECISION] })}`); + assert.equal(done[0], `POST /repos/o/r/issues/7/labels ${JSON.stringify({ labels: [NEEDS_DECISION] })}`); + assert.match(done[1], /^DELETE \/repos\/o\/r\/issues\/7\/labels\/status%3A%20ready/); assert.match(done[2], /^POST \/repos\/o\/r\/issues\/7\/comments /); assert.ok(done[2].includes(ADMIT_MARKER.replaceAll('"', '\\"'))); - assert.match(done[2], /task list/); + assert.match(done[2], /no trusted-human acceptance criteria/); }); test("a Windows-host done-when item stays out of the lane: needs-decision and a recommended split, never agent-ready", async () => { @@ -411,11 +493,24 @@ describe("admit", () => { assert.equal(result.outcome, "refused"); const done = writes(fake.calls); assert.ok(!done.some((call) => call.includes("/labels ") && call.includes(AGENT_READY))); - assert.match(done[0], /^DELETE \/repos\/o\/r\/issues\/7\/labels\/status%3A%20ready/); - assert.equal(done[1], `POST /repos/o/r/issues/7/labels ${JSON.stringify({ labels: [NEEDS_DECISION] })}`); + assert.equal(done[0], `POST /repos/o/r/issues/7/labels ${JSON.stringify({ labels: [NEEDS_DECISION] })}`); + assert.match(done[1], /^DELETE \/repos\/o\/r\/issues\/7\/labels\/status%3A%20ready/); assert.match(done[2], /item 2 .*Windows 11 host.*recommended split: keep the other items here .* needs-human child issue/); }); + test("a refusal whose needs-decision write fails keeps status: ready, so admission-deferred never stands alone", async () => { + const failOn = (method, path, body) => method === "POST" && path.endsWith("/labels") && body?.labels?.includes(NEEDS_DECISION); + const fake = fakeGitHub({ issue: issueOf({ body: "Make the README better.", labels: [{ name: READY }, { name: DEFERRED }] }), failOn }); + await assert.rejects(() => go(fake), /failed with status 500/); + assert.ok(!writes(fake.calls).some((call) => call.startsWith("DELETE ")), writes(fake.calls).join("\n")); + }); + + test("an issue with more comments than the text read covers is refused, as pr-verify stops on it", async () => { + const result = await go(fakeGitHub({ spec: { ...humanSpec(issueOf()), totalCount: 101 } })); + assert.equal(result.outcome, "refused"); + assert.match(result.reasons.join(), /101 comments, more than the 100 pr-verify reads/); + }); + test("a refusal edits the App's earlier admission comment instead of adding one", async () => { const fake = fakeGitHub({ issue: issueOf({ body: "Make the README better." }), @@ -431,15 +526,13 @@ describe("admit", () => { assert.match(writes(fake.calls).at(-1), /^PATCH \/repos\/o\/r\/issues\/comments\/31 /); }); - test("a thin issue the App refined is admitted from the comment's task list; another author's copy is refused", async () => { + test("a thin issue is refused on the App's refinement alone and admitted on a listed human's confirmed criteria", async () => { const thin = issueOf({ body: "Make the README skill count right." }); - const comment = (user) => ({ id: 41, body: `${REFINE_MARKER}\n- [ ] Line 3 says nineteen.`, user }); - const refined = await go(fakeGitHub({ issue: thin, comments: [comment({ id: APP, login: "melodic-automation-lanes[bot]", type: "Bot" })] })); - assert.equal(refined.outcome, "admitted"); - const copied = await go(fakeGitHub({ issue: thin, comments: [comment({ id: OUTSIDER, login: "someone", type: "User" })] })); - assert.equal(copied.outcome, "refused"); - const rewritten = await go(fakeGitHub({ issue: thin, bodyEditedAt: "2026-10-21T00:00:00Z", comments: [{ ...comment({ id: APP, login: "melodic-automation-lanes[bot]", type: "Bot" }), created_at: "2026-10-20T00:00:00Z" }] })); - assert.equal(rewritten.outcome, "refused", "the body changed after the refinement"); + const refinement = written(APP_ACTOR, { body: `${REFINE_MARKER}\n- [ ] Line 3 says nineteen.` }); + const refined = await go(fakeGitHub({ issue: thin, spec: humanSpec(thin, [refinement]) })); + assert.equal(refined.outcome, "refused"); + const confirmed = await go(fakeGitHub({ issue: thin, spec: humanSpec(thin, [refinement, written(USER(HUMAN), { body: "- [ ] Line 3 says nineteen." })]) })); + assert.equal(confirmed.outcome, "admitted"); }); test("an open blocker posts the reason and keeps status: ready", async () => { diff --git a/.github/scripts/intake-triage/refine.mjs b/.github/scripts/intake-triage/refine.mjs index d2c6c8900e..ce2d1525c7 100755 --- a/.github/scripts/intake-triage/refine.mjs +++ b/.github/scripts/intake-triage/refine.mjs @@ -22,10 +22,16 @@ // `--apply` (the lanes App token, issues write) validates the model's // structured output and posts it as a separate comment opening with // REFINE_MARKER; only a split (below) edits the body. Exact: the comment lists the drafted -// criteria as a task list, which admit.mjs counts for criterion 7; when the -// lanes App set the issue's `status: needs-decision`, it takes that off and -// adds `needs-triage`, and the job's `redispatch` output starts one more -// triage, which admits the issue when the rules hold. Not exact: the comment +// criteria as a task list, which admit.mjs does not count for criterion 7 +// until a listed human posts them as their own (owner decision Q9: pr-verify +// reads only their text). Only when that text already holds a task list +// (admit.mjs hasTrustedCriteria) does it ask for one more triage, which runs +// admission again: it takes the lanes App's `status: needs-decision` off, or, +// for a `status: ready` issue admission has not taken, keeps the labels, adds +// `needs-triage`, and sets the job's `redispatch` output. Otherwise the labels +// stay and the comment asks a listed human to post the confirmed criteria as +// their own `**Decision recorded.**` comment, which re-enters intake; a +// re-triage then would only refuse again. Not exact: the comment // asks numbered questions, each with a `RECOMMENDED:` answer and its basis, // and the issue keeps or gets `status: needs-decision`, which a person's // `**Decision recorded.**` sends back through intake-reenter.yml. It never @@ -69,7 +75,7 @@ import { BORN_TRIAGED_MARKER } from "../../actions/check-born-triaged/check-born import { writeOutputs } from "../../actions/check-kill-switch/check-kill-switch.mjs"; import { createGitHub, GitHubError, loadTrustedList, paginate } from "../../actions/check-trusted-trigger/check-trusted-trigger.mjs"; import { selectTrustedIssueComments } from "../../actions/select-trusted-text/select-trusted-text.mjs"; -import { ADMIT_MARKER, loadRules, REFINE_MARKER, refusalText, taskItems, unrunnableCriteria, unrunnableReason } from "./admit.mjs"; +import { ADMIT_MARKER, hasTrustedCriteria, loadRules, REFINE_MARKER, refusalText, specOf, taskItems, unrunnableCriteria, unrunnableReason } from "./admit.mjs"; import { LANES_APP_LOGIN, MARKER } from "./apply-verdict.mjs"; export { REFINE_MARKER }; @@ -302,8 +308,9 @@ export function line(text) { return out; } -// `child` is the issue a split filed. -export function render(output, { child } = {}) { +// `child` is the issue a split filed; `retriage`, whether intake triages the +// issue again. +export function render(output, { child, retriage = false } = {}) { const parts = [REFINE_MARKER, "**Refinement.** A read-only run explored this repository, searched its issues and pull requests, and read the allowed docs to draft what follows. It is model text, to check, not instructions.", ""]; const section = (title, items) => { if (items.length > 0) { @@ -327,9 +334,11 @@ export function render(output, { child } = {}) { ); section("Sources read", output.sources.map((item) => `- ${line(item)}`)); parts.push( - output.exact - ? "Automated refinement by the lanes App. Intake triages the issue again with it." - : "Automated refinement by the lanes App. Answer with a comment whose first line is `**Decision recorded.**`.", + !output.exact + ? "Automated refinement by the lanes App. Answer with a comment whose first line is `**Decision recorded.**`." + : retriage + ? "Automated refinement by the lanes App. Intake triages the issue again with it." + : "Automated refinement by the lanes App. Admission counts only criteria a listed human wrote: to confirm these, post them as your own comment whose first line is `**Decision recorded.**`, which sends the issue back through intake.", ); return parts.join("\n"); } @@ -442,7 +451,10 @@ async function childOf(github, repository, base, appId, body) { // the child first, then sets the labels, then rewrites the body: a failure // before the body edit leaves the held items in place for the next triage to // hold and split again, onto the same child. -export async function applyRefinement({ github, repository, number, appId, rules, output, context, log = console.log }) { +// `humans` and `rules` decide whether an exact refinement asks for a +// re-triage (admit.mjs hasTrustedCriteria); a failed read of the spec throws +// before any label write. +export async function applyRefinement({ github, repository, number, appId, humans, rules, output, context, log = console.log }) { const base = `/repos/${repository}/issues/${number}`; const issue = await github("GET", base); const labels = labelNames(issue); @@ -492,11 +504,21 @@ export async function applyRefinement({ github, repository, number, appId, rules await github("POST", `${base}/labels`, { labels: [NEEDS_TRIAGE] }); await removeLabel(github, base, NEEDS_DECISION); await github("PATCH", base, { body: splitBody(issue.body, rules["task-list"], output.split, child) }); - await github("POST", `${base}/comments`, { body: render(output, { child }) }); + await github("POST", `${base}/comments`, { body: render(output, { child, retriage: true }) }); log(`refine: #${number} split; items ${output.split.join(", ")} moved to #${child}, ${NEEDS_DECISION} lifted and triage asked again`); return { outcome: "split", redispatch: true, child }; } - const post = () => github("POST", `${base}/comments`, { body: render(output) }); + const post = (retriage = false) => github("POST", `${base}/comments`, { body: render(output, { retriage }) }); + // Admission reads criteria only from a listed human's text, so a re-triage + // without them would refuse again: one read, before any label write. + const untrusted = async () => { + if (hasTrustedCriteria(await specOf(github, repository, number), humans, rules["task-list"])) { + return false; + } + await post(); + log(`refine: #${number} is exact; no listed human's text holds a task list yet, so the labels stay and no triage is asked`); + return true; + }; if (!output.exact) { if (labels.includes(READY)) { await removeLabel(github, base, READY); @@ -509,15 +531,18 @@ export async function applyRefinement({ github, repository, number, appId, rules return { outcome: "needs-decision", redispatch: false }; } // A `status: ready` issue admission has not taken (no needs-decision, no - // agent-ready) is triaged again so admission reads the new criteria. + // agent-ready) is triaged again when a listed human's text holds criteria. if (!labels.includes(NEEDS_DECISION)) { if (!labels.includes(READY) || labels.includes(AGENT_READY)) { await post(); log(`refine: #${number} is exact; no needs-decision to lift and nothing awaits admission`); return { outcome: "exact", redispatch: false }; } + if (await untrusted()) { + return { outcome: "exact", redispatch: false }; + } await github("POST", `${base}/labels`, { labels: [NEEDS_TRIAGE] }); - await post(); + await post(true); log(`refine: #${number} is exact; triage asked again so admission reads the criteria`); return { outcome: "exact", redispatch: true }; } @@ -528,10 +553,13 @@ export async function applyRefinement({ github, repository, number, appId, rules log(`refine: #${number} is exact; a person set its needs-decision, so it stays`); return { outcome: "exact", redispatch: false }; } + if (await untrusted()) { + return { outcome: "exact", redispatch: false }; + } // needs-triage first: a failure between the two leaves a label a sweep picks up. await github("POST", `${base}/labels`, { labels: [NEEDS_TRIAGE] }); await removeLabel(github, base, NEEDS_DECISION); - await post(); + await post(true); log(`refine: #${number} is exact; ${NEEDS_DECISION} lifted and triage asked again`); return { outcome: "exact", redispatch: true }; } @@ -573,7 +601,7 @@ if (import.meta.url === pathToFileURL(process.argv[1] ?? "").href) { const known = new Set((context.candidates ?? []).map((item) => item.number)); const output = validateOutput(JSON.parse(readFileSync(env.REFINEMENT_PATH, "utf8")).output, known, context.split ?? null); const github = createGitHub({ token: env.GH_TOKEN, apiUrl: env.GITHUB_API_URL }); - const result = await applyRefinement({ github, repository, number, appId, rules: output.split.length > 0 ? rules() : null, output, context }); + const result = await applyRefinement({ github, repository, number, appId, humans, rules: rules(), output, context }); writeOutputs({ redispatch: String(result.redispatch) }); } else { throw new Error("pass --select or --apply"); diff --git a/.github/scripts/intake-triage/refine.test.mjs b/.github/scripts/intake-triage/refine.test.mjs index dd1e958413..27e972309f 100644 --- a/.github/scripts/intake-triage/refine.test.mjs +++ b/.github/scripts/intake-triage/refine.test.mjs @@ -54,7 +54,7 @@ describe("searchTerms", () => { }); // A fake GitHub recording every call. -function fakeGitHub({ issue, comments = [], events = [], timeline = [], search = { items: [] } } = {}) { +function fakeGitHub({ issue, comments = [], events = [], timeline = [], search = { items: [] }, spec = specOf() } = {}) { const calls = []; const github = async (method, path, body) => { calls.push({ method, path, body }); @@ -62,6 +62,9 @@ function fakeGitHub({ issue, comments = [], events = [], timeline = [], search = if (method === "POST" && path === "/graphql" && body.query.includes("issueTypes")) { return { data: { repository: { issueTypes: { nodes: [{ id: "IT_bug", name: "Bug", isEnabled: true }, { id: "IT_task", name: "Task", isEnabled: true }] } } } }; } + if (method === "POST" && path === "/graphql" && body.query.includes("comments(first")) { + return { data: { repository: { issue: spec } } }; + } if (method === "POST" && path === "/graphql") { return { data: { nodes: (body.variables.ids ?? []).map((id) => ({ id, lastEditedAt: null })) } }; } @@ -90,6 +93,18 @@ function fakeGitHub({ issue, comments = [], events = [], timeline = [], search = }; return { github, calls }; } +// The issue's text as admit.mjs specOf reads it: the body a listed human +// wrote (`body`), and comments, each [authorId, body] (APP is a Bot). +const actor = (id) => (id === APP ? { __typename: "Bot", databaseId: APP } : { __typename: "User", databaseId: id }); +const specOf = (body = "The README is wrong.", comments = []) => ({ + body, + lastEditedAt: null, + author: actor(HUMAN), + editor: null, + comments: { totalCount: comments.length, nodes: comments.map(([id, text]) => ({ body: text, lastEditedAt: null, author: actor(id), editor: null })) }, +}); +const CONFIRMED = specOf("The README is wrong.", [[HUMAN, "**Decision recorded.**\n\n- [ ] The README count matches the plugin count"]]); +const APP_ONLY = specOf("The README is wrong.", [[APP, `${REFINE_MARKER}\n- [ ] The README count matches the plugin count`]]); const writes = (calls) => calls.filter((call) => call.method !== "GET" && call.path !== "/graphql"); const issueOf = (overrides = {}) => ({ @@ -250,11 +265,11 @@ describe("render", () => { describe("applyRefinement", () => { const labeledBy = (actor) => [{ event: "labeled", label: { name: NEEDS_DECISION }, actor: { id: actor } }]; const apply = (fake, output) => - applyRefinement({ github: fake.github, repository: "o/r", number: 7, appId: APP, rules, output: validateOutput(output, new Set([3])), context, log: () => {} }); + applyRefinement({ github: fake.github, repository: "o/r", number: 7, appId: APP, humans, rules, output: validateOutput(output, new Set([3])), context, log: () => {} }); const context = { issue: { title: issueOf().title, body: issueOf().body } }; test("an exact refinement posts the comment, lifts the App's needs-decision, and asks for re-triage", async () => { - const fake = fakeGitHub({ issue: issueOf(), comments: [laneComment()], events: labeledBy(APP) }); + const fake = fakeGitHub({ issue: issueOf(), comments: [laneComment()], events: labeledBy(APP), spec: CONFIRMED }); const result = await apply(fake, EXACT); assert.equal(result.redispatch, true); const done = writes(fake.calls).map(({ method, path, body }) => `${method} ${path} ${JSON.stringify(body)}`); @@ -272,7 +287,7 @@ describe("applyRefinement", () => { }); test("an exact refinement of a status: ready issue admission refused asks for re-triage, once", async () => { - const fake = fakeGitHub({ issue: issueOf({ labels: [{ name: "status: ready" }] }), comments: [laneComment()], events: [] }); + const fake = fakeGitHub({ issue: issueOf({ labels: [{ name: "status: ready" }] }), comments: [laneComment()], events: [], spec: CONFIRMED }); const result = await apply(fake, EXACT); assert.equal(result.redispatch, true); const done = writes(fake.calls).map(({ method, path, body }) => `${method} ${path} ${JSON.stringify(body)}`); @@ -311,7 +326,7 @@ describe("applyRefinement", () => { }); test("a failed label update posts no marker, so the next refinement can finish the labels", async () => { - const fake = fakeGitHub({ issue: issueOf(), comments: [laneComment()], events: labeledBy(APP) }); + const fake = fakeGitHub({ issue: issueOf(), comments: [laneComment()], events: labeledBy(APP), spec: CONFIRMED }); const github = async (method, path, body) => { if (method === "POST" && path.endsWith("/labels")) { fake.calls.push({ method, path, body }); @@ -323,7 +338,7 @@ describe("applyRefinement", () => { assert.ok(!writes(fake.calls).some(({ path }) => path.endsWith("/comments")), "no marker comment was posted"); assert.ok(!writes(fake.calls).some(({ method }) => method === "DELETE"), "the issue keeps needs-decision when adding needs-triage fails"); // The retry sees the labels the failed run left: the App's needs-decision, no marker. - const retry = fakeGitHub({ issue: issueOf(), comments: [laneComment()], events: labeledBy(APP) }); + const retry = fakeGitHub({ issue: issueOf(), comments: [laneComment()], events: labeledBy(APP), spec: CONFIRMED }); const result = await apply(retry, EXACT); assert.equal(result.redispatch, true); assert.ok( @@ -332,6 +347,39 @@ describe("applyRefinement", () => { ); }); + // Owner decision Q9: admission reads criteria only from a listed human's + // text, so a re-triage on the App's text alone would only refuse again. + for (const [branch, setup] of [ + ["the App's needs-decision", { issue: issueOf(), events: labeledBy(APP) }], + ["a status: ready issue awaiting admission", { issue: issueOf({ labels: [{ name: "status: ready" }] }), events: [] }], + ]) { + test(`an exact refinement of ${branch} whose only task list is the App's keeps the labels and asks for no re-triage`, async () => { + const fake = fakeGitHub({ ...setup, comments: [laneComment()], spec: APP_ONLY }); + const result = await apply(fake, EXACT); + assert.deepEqual(result, { outcome: "exact", redispatch: false }); + const done = writes(fake.calls); + assert.equal(done.length, 1, "the comment only: no needs-triage, needs-decision kept"); + assert.match(done[0].body.body, /post them as your own comment whose first line is `\*\*Decision recorded\.\*\*`/); + assert.doesNotMatch(done[0].body.body, /triages the issue again/); + }); + + test(`an exact refinement of ${branch} reads the spec before its first label write and re-triages on a listed human's task list`, async () => { + const fake = fakeGitHub({ ...setup, comments: [laneComment()], spec: specOf("The README is wrong.\n\n- [ ] The README count matches the plugin count") }); + const result = await apply(fake, EXACT); + assert.deepEqual(result, { outcome: "exact", redispatch: true }); + const specRead = fake.calls.findIndex(({ path, body }) => path === "/graphql" && body.query.includes("comments(first")); + const labelWrite = fake.calls.findIndex(({ method, path }) => method === "POST" && path.endsWith("/labels")); + assert.ok(specRead !== -1 && specRead < labelWrite, "the spec is read before needs-triage is added"); + assert.match(writes(fake.calls).at(-1).body.body, /triages the issue again/); + }); + + test(`an exact refinement of ${branch} whose spec read comes back malformed throws before any label write`, async () => { + const fake = fakeGitHub({ ...setup, comments: [laneComment()], spec: { body: 7 } }); + await assert.rejects(apply(fake, EXACT), /unexpected shape/); + assert.deepEqual(writes(fake.calls), []); + }); + } + const untouched = [ ["an issue whose body changed while the model ran", { issue: issueOf({ body: "Rewritten while the model ran." }), comments: [laneComment()], events: labeledBy(APP) }], ["an issue whose title changed while the model ran", { issue: issueOf({ title: "Renamed" }), comments: [laneComment()], events: labeledBy(APP) }], diff --git a/.github/scripts/work-unblock/label-unblocked.mjs b/.github/scripts/work-unblock/label-unblocked.mjs index 5507c6c980..58df776323 100755 --- a/.github/scripts/work-unblock/label-unblocked.mjs +++ b/.github/scripts/work-unblock/label-unblocked.mjs @@ -17,6 +17,26 @@ // defers those), stays deferred, and any other loses admission-deferred. Restarts, re-fires and relabels below // re-run admitted issues and are never paced. // +// A listed human adding admission-deferred to an issue with `status: ready` +// is the mark of a decided issue (owner decision Q75): the sweep reads its +// work-class label and its Size, Complexity and Uncertainty fields (a label +// or value counts only when a listed human or the App added or set it, as +// `status: ready` must be; no default stands in) and runs intake's admission +// rules on it (admit.mjs decide). It is admitted within the room only on an +// admit verdict. A refusal gets admit.mjs's comment, `status: needs-decision` +// for `status: ready`, and loses admission-deferred; a wait on an open pull +// request stays deferred, and one on a blocker loses admission-deferred, so +// its blocker's close admits it. One the rules leave alone loses +// admission-deferred with a comment saying why. +// +// An agent-ready issue left with nothing running is stranded: no hold, no open +// lane pull request, no outcome marker since its newest agent-ready, and no +// work-resolve-issue run for that label, or only one that ended with no +// report, over STRANDED_HOURS ago. It is re-fired once per version of the +// lane workflow files with the re-fire record below; one already re-fired on +// those files gets one comment saying why and needs-human instead. Both share +// limits.restarts-per-run. +// // On `schedule` it sweeps instead: every issue closed in the last // LOOKBACK_HOURS goes through the same rules, so an issue a bot closed (a // pull request merge), which starts no close run, is still picked up. A @@ -103,6 +123,8 @@ // GITHUB_API_URL // RESOLVED_CONFIG resolve-config's output (default // $RUNNER_TEMP/resolved-config.json), for the limits +// BASE_PATH the default branch checkout (default .base), for +// the admission rules and models.issue-dimensions import { readFileSync } from "node:fs"; import process from "node:process"; import { pathToFileURL } from "node:url"; @@ -112,7 +134,7 @@ import { writeOutputs } from "../../actions/check-kill-switch/check-kill-switch. import { admissionLine, checkWip, searchRateLimit } from "../../actions/check-wip-cap/check-wip-cap.mjs"; import { clampReset, isLaneBranch, parseOutcomeMarker, refsHoldMarker } from "../work-resolve-issue/report-outcome.mjs"; import { readLimits, readWip } from "../work-resolve-issue/limits.mjs"; -import { openPullsFor } from "../intake-triage/admit.mjs"; +import { admissionFacts, decide, loadRules, NEEDS_DECISION, openPullsFor, READY, refusalText, sayWhy } from "../intake-triage/admit.mjs"; const LABEL = "agent-ready"; const HUMAN_LABEL = "needs-human"; @@ -149,6 +171,12 @@ const RELABELED = /^/; const IN_FLIGHT = ["queued", "in_progress", "waiting", "pending", "requested"]; // The sweep's window: three hourly runs overlap, so a missed run loses nothing. export const LOOKBACK_HOURS = 3; +// How long an agent-ready issue may sit with no run working on it. +export const STRANDED_HOURS = 3; +const STRANDED = /^/; +// The REST version that documents issue-field-values. +const FIELD_API = { "X-GitHub-Api-Version": "2026-03-10" }; +const DIMENSIONS = ["size", "complexity", "uncertainty"]; // Whether the account that closed the issue (ACTOR_ID, by id) and the one // that started this run (TRIGGERING_ACTOR, by login; a re-runner may be @@ -204,6 +232,10 @@ function isRestart(events, index, markers, bot) { // human (by id): the labelers whose labels the lane acts on. const trustedLabeler = (event, { bot, humans }) => event?.actor?.login === bot || humans.has(event?.actor?.id); +// The newest `labeled` event for label `name` among an issue's events. +const newestAdd = (events, name) => + events.filter((event) => event.event === "labeled" && String(event.label?.name).toLowerCase() === name.toLowerCase()).at(-1); + // Why `number` is not promoted (`reason`, null when it is ready), whether a // hold label is the reason, and whether it carries admission-deferred. async function refusal(args) { @@ -212,7 +244,7 @@ async function refusal(args) { const lower = (current.labels ?? []).map((label) => String(typeof label === "string" ? label : label.name).toLowerCase()); const held = lower.find((label) => HOLDS.has(label)); const reason = current.state !== "open" ? "not open" : held ? `held by ${held}` : await readiness({ ...args, base, lower }); - return { reason, held: current.state === "open" && Boolean(held), deferred: lower.includes(DEFERRED_LABEL) }; + return { reason, held: current.state === "open" && Boolean(held), deferred: lower.includes(DEFERRED_LABEL), current }; } async function readiness({ github, repo, humans, closes, bot = DEFAULT_BOT, base, lower }) { @@ -333,7 +365,7 @@ async function admit({ github, repo, number, gate, deferred }) { // Revisits the open admission-deferred issues, oldest first. Returns how many // reads or writes failed. -async function admitDeferred({ github, repo, humans, bot, gate }) { +async function admitDeferred({ github, repo, humans, bot, gate, appId, admission }) { let waiting; try { waiting = (await paginate(github, `/repos/${repo}/issues?state=open&labels=${DEFERRED_LABEL}&sort=created&direction=asc`)).filter( @@ -351,14 +383,17 @@ async function admitDeferred({ github, repo, humans, bot, gate }) { // Anyone who can label could add admission-deferred, and re-admitting // it adds the App's agent-ready, which starts a credentialed run: only // the App's or a listed human's newest add of it counts. - const adds = (await paginate(github, `/repos/${repo}/issues/${number}/events`)).filter( - (event) => event.event === "labeled" && String(event.label?.name).toLowerCase() === DEFERRED_LABEL, - ); - if (!trustedLabeler(adds.at(-1), { bot, humans })) { + const events = await paginate(github, `/repos/${repo}/issues/${number}/events`); + const mark = newestAdd(events, DEFERRED_LABEL); + if (!trustedLabeler(mark, { bot, humans })) { console.log(`::warning title=label-unblocked::#${number}: ${DEFERRED_LABEL} was added by neither the lanes App nor a listed human, so it is left as is and not re-admitted`); continue; } - const { reason, held, deferred } = await refusal({ github, repo, number, humans, closes, bot }); + const { reason, held, deferred, current } = await refusal({ github, repo, number, humans, closes, bot }); + if (humans.has(mark.actor?.id) && labelNames(current).includes(READY) && !held && current.state === "open") { + await admitMarked({ github, repo, number, issue: current, events, humans, bot, appId, closes, gate, admission }); + continue; + } // Intake defers an issue an open pull request is for; it waits here // until no such pull request is open. const pulls = reason === null ? await openPullsFor(github, repo, number) : []; @@ -380,6 +415,86 @@ async function admitDeferred({ github, repo, humans, bot, gate }) { return failed; } +const labelNames = (issue) => (issue?.labels ?? []).map((label) => String(typeof label === "string" ? label : label?.name)); + +// The verdict intake's admission reads, from the issue itself: its one +// work-class label, which counts only when a listed human or the App added +// it, and 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 lanes' +// reader (resolve-config dimensions.mjs) reads an unset field as a default; +// admission never does. A read of an unexpected shape throws. +async function issueVerdict({ github, repo, number, issue, trusted, humans, appId, dimensionsConfig, fieldSetters }) { + const classes = labelNames(issue) + .filter((name) => name.startsWith("work-class:")) + .map((name) => (trusted(name) ? name : `${name} (added by neither a listed human nor the lanes App)`)); + const rows = await github("GET", `/repos/${repo}/issues/${number}/issue-field-values`, undefined, FIELD_API); + if (!Array.isArray(rows)) { + throw new Error("the issue's field values have an unexpected shape"); + } + const setters = await fieldSetters(github, repo, number); + const dimensions = {}; + for (const name of DIMENSIONS) { + const { "field-id": fieldId, options } = dimensionsConfig.fields[name]; + const row = rows.find((item) => item?.issue_field_id === fieldId); + const setter = setters[String(fieldId)]; + const option = String(row?.single_select_option?.id); + const counts = row !== undefined && (humans.has(setter) || (appId !== undefined && setter === appId)); + dimensions[name] = counts && Object.hasOwn(options, option) ? options[option] : "unset"; + } + const workClass = classes.length === 1 ? classes[0] : classes.length === 0 ? "no work-class label" : classes.join(" and "); + return { workClass, dimensions }; +} + +export const skipText = (reason) => `Left out of admission: ${reason}, so work-unblock removed \`${DEFERRED_LABEL}\`.`; + +// Runs intake's admission rules on an issue a listed human marked, and admits +// it within the gate's room only on an admit verdict. `status: ready` and the +// work-class label count only when the newest add of each, in `events`, is a +// listed human's or the App's, as the mark's own add must be. +async function admitMarked({ github, repo, number, issue, events, humans, bot, appId, closes, gate, admission }) { + const base = `/repos/${repo}/issues/${number}`; + const trusted = (name) => trustedLabeler(newestAdd(events, name), { bot, humans }); + const { rules, dimensionsConfig, webHosts, fieldSetters } = await admission(); + const verdict = await issueVerdict({ github, repo, number, issue, trusted, humans, appId, dimensionsConfig, fieldSetters }); + const facts = await admissionFacts({ + github, + repository: repo, + number, + issue, + rules, + humans, + isTrusted: (blocker) => trustedClose({ github, repo, number: blocker, humans, closes }), + }); + const decision = trusted(READY) + ? decide({ ...facts, verdict, rules, humans, appId, repository: repo, webHosts }) + : { verdict: "skip", reasons: [`${READY} was added by neither a listed human nor the lanes App`] }; + const reasons = decision.reasons.join("; "); + if (decision.verdict === "admit") { + console.log(`label-unblocked: #${number}, marked by a person, passes the admission rules`); + await admit({ github, repo, number, gate, deferred: true }); + } else if (decision.verdict === "refuse") { + // The comment first, then the needs-decision hold before status: ready + // comes off, and admission-deferred last: a failed write leaves the issue + // either still marked, for the next sweep, or held, which no sweep admits. + await sayWhy(github, repo, base, facts.comments, refusalText(reasons)); + await github("POST", `${base}/labels`, { labels: [NEEDS_DECISION] }); + await github("DELETE", `${base}/labels/${encodeURIComponent(READY)}`); + await github("DELETE", `${base}/labels/${DEFERRED_LABEL}`); + console.log(`label-unblocked: #${number}, marked by a person, refused: ${reasons}`); + } else if (decision.verdict === "wait") { + await sayWhy(github, repo, base, facts.comments, `Not admitted to the issue lane yet: ${reasons}.`); + if (decision.pulls.length === 0) { + await github("DELETE", `${base}/labels/${DEFERRED_LABEL}`); + } + console.log(`label-unblocked: #${number}, marked by a person, waits: ${reasons}`); + } else { + // A skip drops the person's mark, so the comment tells them why. + await sayWhy(github, repo, base, facts.comments, skipText(reasons)); + await github("DELETE", `${base}/labels/${DEFERRED_LABEL}`); + console.log(`label-unblocked: #${number} deferral dropped: ${reasons}`); + } +} + // Labels each issue `closed` blocked that is ready, each at most once per run, // within the gate's room. Returns how many reads or writes failed. async function labelUnblocked({ github, repo, closed, sweep, humans, bot, gate }) { @@ -434,8 +549,21 @@ export const escalationComment = (stops) => // The re-fire's record, posted before the label moves: one per issue per // version of the lane workflow files. -export const refireComment = (key) => - `\nThe work-resolve-issue run that \`${LABEL}\` started failed or was cancelled before it reported, or it stopped or the token broker refused it because its workflow files differ from main's. work-unblock removed and added \`${LABEL}\` back to start a fresh run on main's workflow files. It does this once per version of those files, so if this run fails the same way, a person looks at it.`; +const FAILED_CAUSE = `The work-resolve-issue run that \`${LABEL}\` started failed or was cancelled before it reported, or it stopped or the token broker refused it because its workflow files differ from main's.`; +export const refireComment = (key, cause = FAILED_CAUSE) => + `\n${cause} work-unblock removed and added \`${LABEL}\` back to start a fresh run on main's workflow files. It does this once per version of those files, so if this run fails the same way, a person looks at it.`; + +// Why a stranded issue has nothing running, as the comments say it. +const strandedCause = { + "no-run": `No work-resolve-issue run started in the ${STRANDED_HOURS} hours since \`${LABEL}\` was last added.`, + "no-report": `The work-resolve-issue run that \`${LABEL}\` started ended over ${STRANDED_HOURS} hours ago with no report and no pull request.`, + "failed-again": `The work-resolve-issue run that \`${LABEL}\` started failed or was cancelled before it reported, over ${STRANDED_HOURS} hours ago.`, +}; + +// The hand-off of a stranded issue already re-fired on these workflow files: +// one per label add. +export const strandedComment = (key, cause) => + `\n${strandedCause[cause]} work-unblock already re-fired it once on these lane workflow files, so it no longer does, and the issue would hold a place in the issue lane's queue with nothing running. It added \`${HUMAN_LABEL}\`. To try again, remove \`${HUMAN_LABEL}\`, then remove \`${LABEL}\` and add it back.`; // The relabel's record, posted before the label moves: one per recurrence // comment. @@ -718,22 +846,47 @@ async function refireRefusal({ github, actions, repo, number, lastAdd, addedAt, if (!actions) { return { reason: "no Actions token to read its lane run" }; } + // A record counts once the App added the label back at or after it; one + // whose label write failed is finished by the next sweep, not posted again. + const relabeled = (marker) => + events.some((event) => isLabel(event, "labeled") && event.actor?.login === bot && Date.parse(event.created_at) >= marker.at); + // An issue with nothing running since `since`: once STRANDED_HOURS have + // passed with no open lane PR, re-fired once per version of the lane + // workflow files, then handed to a person. + const stranded = async (since, why, cause) => { + if (now - since < STRANDED_HOURS * HOUR_MS) { + return { reason: why }; + } + const open = (await lanePulls({ github, repo, number, bot })).find((item) => item.state === "open"); + if (open) { + return { reason: `${why}; its lane PR #${open.number} is open` }; + } + const key = await workflowKey(); + const fired = markers.filter((marker) => marker.outcome === "refired" && marker.key === key); + if (fired.some(relabeled)) { + const explained = comments.some( + (comment) => comment.user?.login === bot && STRANDED.exec(String(comment.body ?? ""))?.[1] === key && Date.parse(comment.created_at) >= addedAt, + ); + return { reason: null, resets: since, stranded: cause, key, explained, why }; + } + return { reason: null, resets: since, refire: key, recorded: fired.length > 0, cause: strandedCause[cause] }; + }; const { mine: runs, untitled } = await laneRuns(actions, repo, number, addedAt); if (runs.length === 0) { - return { - reason: - untitled > 0 - ? `cannot tell whether a lane run started since ${LABEL} was added: ${untitled} work-resolve-issue run(s) then name no issue in their title` - : `no lane run since ${LABEL} was added`, - }; + return untitled > 0 + ? { + reason: `cannot tell whether a lane run started since ${LABEL} was added: ${untitled} work-resolve-issue run(s) then name no issue in their title`, + } + : stranded(addedAt, `no lane run since ${LABEL} was added`, "no-run"); } if (runs.some((run) => run.status !== "completed")) { return { reason: "its lane run is still going" }; } + // When the newest of its runs ended: every clock below starts there. + const ended = Math.max(...runs.map((run) => Date.parse(run.updated_at)).filter(Number.isFinite), addedAt); const jobsOf = jobLister(actions, repo); const stalled = await didNoWork(jobsOf, runs); if (stalled) { - const ended = Math.max(...runs.map((run) => Date.parse(run.updated_at)).filter(Number.isFinite), addedAt); const due = ended + limits["reset-clamp-min"] * HOUR_MS; if (now < due) { return { reason: `its lane run did no work; it is re-fired after ${new Date(due).toISOString()}` }; @@ -756,16 +909,12 @@ async function refireRefusal({ github, actions, repo, number, lastAdd, addedAt, return { reason: null, resets: due, refire: key, recorded }; } if (!runs.some((run) => FAILED.has(run.conclusion)) && !(await cancelledBeforeWork(jobsOf, runs)) && !(await stoppedStale(jobsOf, runs))) { - return { reason: "its lane run did not fail" }; + return stranded(ended, "its lane run did not fail", "no-report"); } const key = await workflowKey(); - // A record counts once the App added the label back at or after it; one - // whose label write failed is finished by the next sweep, not posted again. const recorded = markers.filter((marker) => marker.outcome === "refired" && marker.key === key); - const relabeled = (marker) => - events.some((event) => isLabel(event, "labeled") && event.actor?.login === bot && Date.parse(event.created_at) >= marker.at); if (recorded.some(relabeled)) { - return { reason: `already re-fired for workflow files ${key}` }; + return stranded(ended, `already re-fired for workflow files ${key}`, "failed-again"); } return { reason: null, resets: addedAt, refire: key, recorded: recorded.length > 0 }; } @@ -947,8 +1096,22 @@ async function restartLimited({ github, actions, repo, bot, humans, now, limits, for (const { number } of ready.slice(cap)) { console.log(`label-unblocked: #${number} restart deferred: ${cap} restarts this run`); } - for (const { number, refire, recurrence, recorded, handOff, pr, explained, release } of ready.slice(0, cap)) { + for (const { number, refire, cause, recurrence, recorded, handOff, pr, explained, release, stranded, key: strandedKey, why } of ready.slice(0, cap)) { const base = `/repos/${repo}/issues/${number}`; + if (stranded) { + // The comment first and the hold last, as the escalation does. + try { + if (!explained) { + await github("POST", `${base}/comments`, { body: strandedComment(strandedKey, stranded) }); + } + await github("POST", `${base}/labels`, { labels: [HUMAN_LABEL] }); + console.log(`label-unblocked: #${number} handed to a person: nothing running on it (${why})`); + } catch (error) { + failed += 1; + reportError(number, error); + } + continue; + } if (handOff) { // The comment first, then the labels; agent-ready comes off last, so a // failed write leaves the issue for the next sweep, which finds the @@ -972,7 +1135,7 @@ async function restartLimited({ github, actions, repo, bot, humans, now, limits, try { // The record first: a failed comment re-fires nothing, so the bound holds. if (refire && !recorded) { - await github("POST", `${base}/comments`, { body: refireComment(refire) }); + await github("POST", `${base}/comments`, { body: refireComment(refire, cause) }); } // The labels, not the record, bound a relabel, so a failed record (the // alert locks every fix issue) is named and the relabel goes ahead. @@ -1002,7 +1165,29 @@ async function restartLimited({ github, actions, repo, bot, humans, now, limits, return failed; } -export async function main({ env = process.env, github, actions, now = Date.now(), rate = searchRateLimit() } = {}) { +// What a marked issue's admission check reads from the default branch: the +// rules, models.issue-dimensions, the issue lane's web hosts and the field +// setter reader. resolve-config needs the packages its step installed in this +// job, so it loads only when a marked issue needs it. +const baseAdmission = (env) => { + let loaded; + return () => + (loaded ??= (async () => { + const config = await import(new URL("../../actions/resolve-config/resolve-config.mjs", import.meta.url).href); + const dimensionsConfig = config.issueDimensionsFromBase(env); + if (!dimensionsConfig?.fields) { + throw new Error("the config sets no models.issue-dimensions"); + } + return { + rules: loadRules(`${env.BASE_PATH || ".base"}/.github/actions/resolve-config/admission-rules.json`), + dimensionsConfig, + webHosts: config.issueLaneWebHostsFromBase(env), + fieldSetters: config.fieldSetters, + }; + })()); +}; + +export async function main({ env = process.env, github, actions, now = Date.now(), rate = searchRateLimit(), admission = baseAdmission(env) } = {}) { try { if (!REPO.test(env.GITHUB_REPOSITORY ?? "")) { throw new Error("GITHUB_REPOSITORY is not owner/repo"); @@ -1029,7 +1214,7 @@ export async function main({ env = process.env, github, actions, now = Date.now( } } const repo = env.GITHUB_REPOSITORY; - const humans = loadTrustedList(env.TRUSTED_ACTORS_PATH).humans; + const { humans, logins } = loadTrustedList(env.TRUSTED_ACTORS_PATH); github ??= createGitHub({ token: env.GH_TOKEN, apiUrl: env.GITHUB_API_URL || undefined, onResponse: rate.onResponse }); const closed = sweep ? await closedSince(github, repo, new Date(now - LOOKBACK_HOURS * 3_600_000)) @@ -1045,7 +1230,7 @@ export async function main({ env = process.env, github, actions, now = Date.now( let read; 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 }) : 0; + let failed = sweep ? await admitDeferred({ github, repo, humans, bot, gate, appId: logins.get(bot), admission }) : 0; failed += await labelUnblocked({ github, repo, closed, sweep, humans, bot, gate }); if (sweep) { actions ??= env.ACTIONS_TOKEN ? createGitHub({ token: env.ACTIONS_TOKEN, apiUrl: env.GITHUB_API_URL || undefined }) : null; diff --git a/.github/scripts/work-unblock/label-unblocked.test.mjs b/.github/scripts/work-unblock/label-unblocked.test.mjs index 4218c9a954..1eecf726e6 100644 --- a/.github/scripts/work-unblock/label-unblocked.test.mjs +++ b/.github/scripts/work-unblock/label-unblocked.test.mjs @@ -11,6 +11,7 @@ import { dirname, join } from "node:path"; import { test } from "node:test"; import { fileURLToPath } from "node:url"; +import { ADMIT_MARKER, loadRules, refusalText } from "../intake-triage/admit.mjs"; import { refsHoldMarker } from "../work-resolve-issue/report-outcome.mjs"; import { checkCloser, @@ -21,6 +22,8 @@ import { refireComment, refsClosedComment, refsMergedComment, + skipText, + strandedComment, } from "./label-unblocked.mjs"; const HERE = dirname(fileURLToPath(import.meta.url)); @@ -103,13 +106,15 @@ function world({ queuedIssues = [], deferred = [], searchIncomplete = false, + fieldValues = {}, + specs = {}, failOn, } = {}) { const calls = []; const base = `/repos/${REPO}/issues`; const github = async (method, path, body) => { calls.push(body === undefined ? `${method} ${path}` : `${method} ${path} ${JSON.stringify(body)}`); - if (failOn && path.startsWith(failOn)) { + if (failOn && (typeof failOn === "function" ? failOn(method, path, body) : path.startsWith(failOn))) { throw new Error(`${method} ${path} failed with status 500`); } const route = path.replace(/\?.*$/, ""); @@ -180,6 +185,9 @@ function world({ if ((match = route.match(/\/issues\/(\d+)\/dependencies\/blocked_by$/))) { return listed(blockedBy[match[1]] ?? []); } + if (method === "GET" && (match = route.match(/\/issues\/(\d+)\/issue-field-values$/))) { + return fieldValues[match[1]] ?? []; + } if ((match = route.match(/\/issues\/(\d+)\/comments$/))) { return listed(comments[match[1]] ?? []); } @@ -189,7 +197,7 @@ function world({ if (method === "POST" && (match = route.match(/\/issues\/(\d+)\/labels$/))) { return []; } - if (method === "DELETE" && route.match(/\/issues\/(\d+)\/labels\/(agent-ready|needs-human|admission-deferred)$/)) { + if (method === "DELETE" && route.match(/\/issues\/(\d+)\/labels\/(agent-ready|needs-human|admission-deferred|status%3A%20ready)$/)) { return []; } const lookup = (number) => @@ -206,6 +214,18 @@ function world({ if (method === "POST" && route === "/graphql") { const { owner, name, number } = body.variables; assert.equal(`${owner}/${name}`, REPO); + // admit.mjs's text read: by default a listed human wrote the body. + if (body.query.includes("comments(first: 100)")) { + const spec = specs[String(number)] ?? { + body: String(lookup(String(number))?.body ?? ""), + author: { __typename: "User", databaseId: OWNER.id }, + editor: null, + lastEditedAt: null, + comments: [], + }; + const comments = { totalCount: spec.totalCount ?? spec.comments?.length, nodes: spec.comments }; + return { data: { repository: { issue: spec === "bad" ? null : { ...spec, comments } } } }; + } const found = lookup(String(number)); const nodes = found?.state === "closed" ? [{ closer: found.graphCloser }] : []; return { data: { repository: { issue: { timelineItems: { nodes } } } } }; @@ -1129,6 +1149,7 @@ const laneRun = ({ number = 30, at = "2026-10-16T10:00:03Z", status = "completed created_at: at, status, conclusion, + updated_at: at, }); const refireNote = (key, at = "2026-10-16T09:00:00Z", user = LANES_BOT) => ({ id: 50, body: refireComment(key), user, created_at: at }); const refired = (n, key = KEY) => [ @@ -2299,3 +2320,314 @@ test("at the queued cap a run the WIP cap skipped is still re-fired: runs are ne assert.equal(await sweep({ github, actions }, PAUSING), 0); assert.deepEqual(writes(calls), refired(30)); }); + +// The HITL mark (owner decision Q75): a listed human adding admission-deferred +// to a decided issue with `status: ready` has the sweep run intake's admission +// rules (admit.mjs decide, on admission-rules.json) on it. The verdict is the +// issue's own work-class label and the Size, Complexity and Uncertainty a +// listed human or the App set; it is admitted within the room only on admit. +const DIMS = { + fields: { + size: { "field-id": 15462270, options: { 27053568: "small", 27053567: "medium", 27053566: "large" } }, + complexity: { "field-id": 48625720, options: { 85127651: "low", 85127652: "medium", 85127653: "high" } }, + uncertainty: { "field-id": 48625721, options: { 85127654: "known", 85127655: "knowable", 85127656: "unknown" } }, + }, +}; +const fieldRow = (field, option) => ({ issue_field_id: field, single_select_option: { id: option } }); +const SMALL_LOW_KNOWN = [fieldRow(15462270, 27053568), fieldRow(48625720, 85127651), fieldRow(48625721, 85127654)]; +const LARGE_LOW_KNOWN = [fieldRow(15462270, 27053566), fieldRow(48625720, 85127651), fieldRow(48625721, 85127654)]; +const RULES_PATH = join(HERE, "../../actions/resolve-config/admission-rules.json"); +const admissionFor = (setter = OWNER.id) => async () => ({ + rules: loadRules(RULES_PATH), + dimensionsConfig: DIMS, + webHosts: [], + fieldSetters: async () => ({ 15462270: setter, 48625720: setter, 48625721: setter }), +}); +const MARKED = ["admission-deferred", "status: ready", "work-class: mechanical"]; +const markedIssue = (labels) => ({ ...issue(25, { labels }), user: OWNER, type: { name: "Task" }, body: "Fix it.\n\n## Done when\n\n- [ ] Line 3 says nineteen." }); +// Who added each label: intake's App wrote the decision, a person marked it. +const labelAdd = (name, actor, at = "2026-10-16T07:00:00Z") => ({ event: "labeled", label: { name }, actor, created_at: at }); +const DECIDED = [labelAdd("status: ready", LANES_BOT), labelAdd("work-class: mechanical", LANES_BOT)]; +const markedWorld = ({ + labels = MARKED, + fields = SMALL_LOW_KNOWN, + by = OWNER, + decided = DECIDED, + queuedIssues = [], + specs = {}, + timeline = [], + closing = [], + comments = [note()], + extraEvents = [], + blockers = [issue(10, { state: "closed" })], + failOn, +} = {}) => + world({ + specs, + failOn, + closing: { 25: closing }, + deferred: [markedIssue(labels)], + issues: { 25: markedIssue(labels) }, + blockedBy: { 25: blockers }, + comments: { 25: comments }, + events: { 25: [...decided, ...extraEvents, deferredAdd(by)] }, + timeline: { 25: timeline }, + fieldValues: { 25: fields }, + queuedIssues, + }); +const fieldReads = (calls) => calls.filter((call) => call === `GET /repos/${REPO}/issues/25/issue-field-values`).length; + +test("a person's mark on an admissible decided issue runs the admission rules and admits it within the room", async (t) => { + const log = t.mock.method(console, "log", () => {}); + const { github, calls } = markedWorld(); + assert.equal(await sweep({ github, admission: admissionFor() }, PAUSING), 0); + assert.deepEqual(writes(calls), [labeled(25), undeferred(25)]); + assert.equal(fieldReads(calls), 1); + const lines = log.mock.calls.map((call) => String(call.arguments[0])); + assert.ok(lines.includes("label-unblocked: #25, marked by a person, passes the admission rules"), lines.join("\n")); +}); + +test("a person's mark on an inadmissible issue is refused: the admission comment, needs-decision, admission-deferred off", async (t) => { + t.mock.method(console, "log", () => {}); + const refusedWrites = (reasons) => [ + `POST /repos/${REPO}/issues/25/comments ${JSON.stringify({ body: `${ADMIT_MARKER}\n${refusalText(reasons.join("; "))}` })}`, + `POST /repos/${REPO}/issues/25/labels {"labels":["status: needs-decision"]}`, + `DELETE /repos/${REPO}/issues/25/labels/status%3A%20ready`, + undeferred(25), + ]; + const large = markedWorld({ fields: LARGE_LOW_KNOWN }); + assert.equal(await sweep({ github: large.github, admission: admissionFor() }, PAUSING), 0); + assert.deepEqual(writes(large.calls), refusedWrites(["Size large is not one of small, medium"])); + // A value no listed human or App set counts as unset, never as a default. + const stranger = markedWorld(); + assert.equal(await sweep({ github: stranger.github, admission: admissionFor(4242) }, PAUSING), 0); + assert.deepEqual( + writes(stranger.calls), + refusedWrites(["Size unset is not one of small, medium", "Uncertainty unset is not one of known, knowable"]), + ); + // Intake's own deferral (the App's add) keeps the room-only path: no rules read. + const intake = markedWorld({ fields: LARGE_LOW_KNOWN, by: LANES_BOT }); + assert.equal(await sweep({ github: intake.github, admission: admissionFor() }, PAUSING), 0); + assert.deepEqual(writes(intake.calls), [labeled(25), undeferred(25)]); + assert.equal(fieldReads(intake.calls), 0); +}); + +test("a person's mark whose criteria only the App wrote is refused; a text read of an unexpected shape writes nothing", async (t) => { + t.mock.method(console, "log", () => {}); + const app = { __typename: "Bot", databaseId: LANES_APP_ID }; + const appOnly = markedWorld({ + specs: { 25: { body: "- [ ] Line 3 says nineteen.", author: app, editor: null, lastEditedAt: null, comments: [{ body: "Go ahead.", author: { __typename: "User", databaseId: OWNER.id }, editor: null, lastEditedAt: null }] } }, + }); + assert.equal(await sweep({ github: appOnly.github, admission: admissionFor() }, PAUSING), 0); + assert.ok(writes(appOnly.calls).includes(`POST /repos/${REPO}/issues/25/labels {"labels":["status: needs-decision"]}`), writes(appOnly.calls).join("\n")); + assert.ok(writes(appOnly.calls).some((call) => call.includes("no trusted-human acceptance criteria"))); + const bad = markedWorld({ specs: { 25: "bad" } }); + assert.equal(await sweep({ github: bad.github, admission: admissionFor() }, PAUSING), 1); + assert.deepEqual(writes(bad.calls), []); +}); + +test("a person's mark waits, still deferred, while an open pull request closes or Refs it; one that only mentions it does not hold it", async (t) => { + const log = t.mock.method(console, "log", () => {}); + const waiting = `POST /repos/${REPO}/issues/25/comments ${JSON.stringify({ body: `${ADMIT_MARKER}\nNot admitted to the issue lane yet: open pull request #40 is for it.` })}`; + for (const facts of [{ closing: [{ number: 40, repository: { nameWithOwner: REPO } }] }, { timeline: [namingPull("open")] }]) { + log.mock.resetCalls(); + const open = markedWorld(facts); + assert.equal(await sweep({ github: open.github, admission: admissionFor() }, PAUSING), 0); + assert.deepEqual(writes(open.calls), [waiting], JSON.stringify(facts)); + const lines = log.mock.calls.map((call) => String(call.arguments[0])); + assert.ok(lines.includes("label-unblocked: #25, marked by a person, waits: open pull request #40 is for it"), lines.join("\n")); + } + const mention = markedWorld({ timeline: [namingPull("open", "Same shape as #25.")] }); + assert.equal(await sweep({ github: mention.github, admission: admissionFor() }, PAUSING), 0); + assert.deepEqual(writes(mention.calls), [labeled(25), undeferred(25)]); +}); + +test("a person's admissible mark with no room stays deferred", async (t) => { + const log = t.mock.method(console, "log", () => {}); + const { github, calls } = markedWorld({ queuedIssues: queuedN(24) }); + assert.equal(await sweep({ github, admission: admissionFor() }, PAUSING), 0); + assert.deepEqual(writes(calls), []); + const lines = log.mock.calls.map((call) => String(call.arguments[0])); + assert.ok(lines.includes("label-unblocked: #25, marked by a person, passes the admission rules"), lines.join("\n")); + assert.ok(lines.includes("label-unblocked: #25 admission deferred: queued work is at the queued cap"), lines.join("\n")); +}); + +const NEEDS_DECISION_WRITE = `POST /repos/${REPO}/issues/25/labels {"labels":["status: needs-decision"]}`; +// The labels a sweep's writes leave on the marked fixture, the failed write left out. +const labelsAfter = (labels, calls, failed) => + writes(calls) + .filter((call) => call !== failed) + .reduce((now, call) => { + const add = call.match(/^POST \/repos\/\S+\/issues\/25\/labels (.+)$/); + const removed = call.match(/^DELETE \/repos\/\S+\/issues\/25\/labels\/(.+)$/); + if (add) { + return [...now, ...JSON.parse(add[1]).labels.filter((name) => !now.includes(name))]; + } + return removed ? now.filter((name) => name !== decodeURIComponent(removed[1])) : now; + }, labels); + +test("a refusal whose needs-decision write fails keeps status: ready, so the next sweep admits nothing", async (t) => { + t.mock.method(console, "log", () => {}); + const failOn = (method, path, body) => method === "POST" && path.endsWith("/issues/25/labels") && body?.labels?.includes("status: needs-decision"); + const first = markedWorld({ fields: LARGE_LOW_KNOWN, failOn }); + assert.equal(await sweep({ github: first.github, admission: admissionFor() }, PAUSING), 1); + assert.ok(!writes(first.calls).includes(`DELETE /repos/${REPO}/issues/25/labels/status%3A%20ready`), writes(first.calls).join("\n")); + const left = labelsAfter(MARKED, first.calls, NEEDS_DECISION_WRITE); + const second = markedWorld({ labels: left, fields: LARGE_LOW_KNOWN }); + assert.equal(await sweep({ github: second.github, admission: admissionFor() }, PAUSING), 0); + assert.ok(!writes(second.calls).includes(labeled(25)), `${left}\n${writes(second.calls).join("\n")}`); +}); + +test("a person's mark whose field values or field setters read in an unexpected shape writes nothing", async (t) => { + t.mock.method(console, "log", () => {}); + const rows = markedWorld({ fields: { message: "Not Found" } }); + assert.equal(await sweep({ github: rows.github, admission: admissionFor() }, PAUSING), 1); + assert.deepEqual(writes(rows.calls), []); + const noSetters = async () => ({ ...(await admissionFor()()), fieldSetters: async () => { throw new Error("the issue's field events have an unexpected shape"); } }); + const setters = markedWorld(); + assert.equal(await sweep({ github: setters.github, admission: noSetters }, PAUSING), 1); + assert.deepEqual(writes(setters.calls), []); +}); + +test("a person's mark admits only when a listed human or the App added status: ready and the work-class label", async (t) => { + t.mock.method(console, "log", () => {}); + const outsider = { id: 999999, login: "drive-by", type: "User" }; + const human = markedWorld({ decided: [labelAdd("status: ready", OWNER), labelAdd("work-class: mechanical", OWNER)] }); + assert.equal(await sweep({ github: human.github, admission: admissionFor() }, PAUSING), 0); + assert.deepEqual(writes(human.calls), [labeled(25), undeferred(25)]); + // The newest add counts: an outsider's re-add after the App's does not. + for (const decided of [ + [labelAdd("status: ready", LANES_BOT), labelAdd("work-class: mechanical", outsider)], + [labelAdd("status: ready", LANES_BOT), labelAdd("work-class: mechanical", LANES_BOT), labelAdd("work-class: mechanical", outsider, "2026-10-16T07:30:00Z")], + [labelAdd("status: ready", outsider), labelAdd("work-class: mechanical", LANES_BOT)], + [labelAdd("work-class: mechanical", LANES_BOT)], + ]) { + const { github, calls } = markedWorld({ decided }); + assert.equal(await sweep({ github, admission: admissionFor() }, PAUSING), 0); + assert.ok(!writes(calls).includes(labeled(25)), `${JSON.stringify(decided)}\n${writes(calls).join("\n")}`); + assert.ok(writes(calls).includes(undeferred(25)), JSON.stringify(decided)); + } + const workClass = markedWorld({ decided: [labelAdd("status: ready", LANES_BOT), labelAdd("work-class: mechanical", outsider)] }); + await sweep({ github: workClass.github, admission: admissionFor() }, PAUSING); + assert.ok(writes(workClass.calls).some((call) => call.includes("added by neither a listed human nor the lanes App")), writes(workClass.calls).join("\n")); +}); + +test("a person's mark waiting on an open blocker loses admission-deferred with the admission comment", async (t) => { + const log = t.mock.method(console, "log", () => {}); + const { github, calls } = markedWorld({ blockers: [issue(10)] }); + assert.equal(await sweep({ github, admission: admissionFor() }, PAUSING), 0); + const why = "blocker #10 is not closed as completed by a listed human or a merge"; + assert.deepEqual(writes(calls), [ + `POST /repos/${REPO}/issues/25/comments ${JSON.stringify({ body: `${ADMIT_MARKER}\nNot admitted to the issue lane yet: ${why}.` })}`, + undeferred(25), + ]); + const lines = log.mock.calls.map((call) => String(call.arguments[0])); + assert.ok(lines.includes(`label-unblocked: #25, marked by a person, waits: ${why}`), lines.join("\n")); +}); + +test("a person's mark the admission rules skip loses admission-deferred with a comment saying why", async (t) => { + t.mock.method(console, "log", () => {}); + const removed = { event: "unlabeled", label: { name: "agent-ready" }, actor: OWNER, created_at: "2026-10-16T07:10:00Z" }; + const { github, calls } = markedWorld({ extraEvents: [removed] }); + assert.equal(await sweep({ github, admission: admissionFor() }, PAUSING), 0); + assert.deepEqual(writes(calls), [ + `POST /repos/${REPO}/issues/25/comments ${JSON.stringify({ body: `${ADMIT_MARKER}\n${skipText("agent-ready was removed before")}` })}`, + undeferred(25), + ]); +}); + +// A stranded agent-ready issue: no hold, no open lane PR, no marker since its +// newest agent-ready, and no lane run for that label, or one that ended with +// no report, over 3 hours ago. It is re-fired once per version of the lane +// workflow files with the same record as a failed run; then it goes to a person. +const NO_RUN_CAUSE = "No work-resolve-issue run started in the 3 hours since `agent-ready` was last added."; +const NO_REPORT_CAUSE = "The work-resolve-issue run that `agent-ready` started ended over 3 hours ago with no report and no pull request."; +const refiredFor = (n, cause) => [`POST /repos/${REPO}/issues/${n}/comments ${JSON.stringify({ body: refireComment(KEY, cause) })}`, ...restarted(n)]; + +test("a stranded agent-ready issue with no lane run is re-fired once with the re-fire record", async () => { + const { github, actions, calls } = failedRun({ runs: [], events: [added("2026-10-16T08:00:00Z", OWNER)] }); + assert.equal(await sweep({ github, actions }), 0); + assert.deepEqual(writes(calls), refiredFor(30, NO_RUN_CAUSE)); + assert.ok(refireComment(KEY, NO_RUN_CAUSE).startsWith(`\n${NO_RUN_CAUSE}`)); +}); + +test("a stranded issue already re-fired on these workflow files gets one comment and needs-human", async () => { + const events = [ + added("2026-10-16T06:00:00Z", OWNER), + { event: "unlabeled", label: { name: "agent-ready" }, actor: LANES_BOT, created_at: "2026-10-16T08:00:00Z" }, + added("2026-10-16T08:00:01Z"), + ]; + const first = failedRun({ runs: [], comments: [note(), refireNote(KEY, "2026-10-16T07:59:59Z")], events }); + assert.equal(await sweep({ github: first.github, actions: first.actions }), 0); + assert.deepEqual(writes(first.calls), [ + `POST /repos/${REPO}/issues/30/comments ${JSON.stringify({ body: strandedComment(KEY, "no-run") })}`, + `POST /repos/${REPO}/issues/30/labels {"labels":["needs-human"]}`, + ]); + assert.ok(strandedComment(KEY, "no-run").includes(NO_RUN_CAUSE)); + // A sweep after a failed label write adds only the label. + const posted = { id: 51, body: strandedComment(KEY, "no-run"), user: LANES_BOT, created_at: "2026-10-16T09:00:00Z" }; + const again = failedRun({ runs: [], comments: [note(), refireNote(KEY, "2026-10-16T07:59:59Z"), posted], events }); + assert.equal(await sweep({ github: again.github, actions: again.actions }), 0); + assert.deepEqual(writes(again.calls), [`POST /repos/${REPO}/issues/30/labels {"labels":["needs-human"]}`]); +}); + +test("an issue with a recent lane run or label is left alone; past 3 hours it counts as stranded", async () => { + // A green run with no report, 2 h 59 min 57 s old, and a label just under 3 h old: nothing. + for (const [runs, at] of [ + [[laneRun({ at: "2026-10-16T09:00:03Z", conclusion: "success" })], "2026-10-16T09:00:00Z"], + [[], "2026-10-16T09:00:01Z"], + ]) { + const { github, actions, calls } = failedRun({ runs, events: [added(at, OWNER)] }); + assert.equal(await sweep({ github, actions }), 0, at); + assert.deepEqual(writes(calls), [], at); + } + const stale = failedRun({ runs: [laneRun({ at: "2026-10-16T08:59:00Z", conclusion: "success" })], events: [added("2026-10-16T08:58:57Z", OWNER)] }); + assert.equal(await sweep({ github: stale.github, actions: stale.actions }), 0); + assert.deepEqual(writes(stale.calls), refiredFor(30, NO_REPORT_CAUSE)); + // An open lane PR keeps a stranded issue out of the re-fire. + const lanePr = { number: 7900, user: LANES_BOT, state: "open", created_at: "2026-10-16T05:00:00Z", head: { ref: "work-resolve-issue/issue-30-4000-1", repo: { full_name: REPO } }, base: { repo: { full_name: REPO } } }; + const withPr = world({ + openReady: [ready(30)], + comments: { 30: [note()] }, + events: { 30: [added("2026-10-16T08:00:00Z", OWNER)] }, + blobs: BLOBS, + timeline: { 30: [{ source: { issue: { number: 7900, pull_request: {}, user: LANES_BOT, repository_url: `${API}/repos/${REPO}` } } }] }, + pulls: { 7900: lanePr }, + }); + assert.equal(await sweep({ github: withPr.github, actions: withPr.github }), 0); + assert.deepEqual(writes(withPr.calls), []); +}); + +test("a stranded issue's no-report clock starts when its run ended, not when it started", async () => { + // A run created over 3 hours ago that ended an hour ago: left alone. + const recent = failedRun({ + runs: [{ ...laneRun({ at: "2026-10-16T08:00:03Z", conclusion: "success" }), updated_at: "2026-10-16T11:00:00Z" }], + events: [added("2026-10-16T08:00:00Z", OWNER)], + }); + assert.equal(await sweep({ github: recent.github, actions: recent.actions }), 0); + assert.deepEqual(writes(recent.calls), []); + const ended = failedRun({ + runs: [{ ...laneRun({ at: "2026-10-16T08:00:03Z", conclusion: "success" }), updated_at: "2026-10-16T08:59:00Z" }], + events: [added("2026-10-16T08:00:00Z", OWNER)], + }); + assert.equal(await sweep({ github: ended.github, actions: ended.actions }), 0); + assert.deepEqual(writes(ended.calls), refiredFor(30, NO_REPORT_CAUSE)); +}); + +test("a stranded issue whose re-fired run failed again over 3 hours ago gets one comment and needs-human", async () => { + const { github, actions, calls } = failedRun({ + comments: [note(), refireNote(KEY, "2026-10-16T07:59:59Z")], + events: [ + added("2026-10-16T07:00:00Z", OWNER), + { event: "unlabeled", label: { name: "agent-ready" }, actor: LANES_BOT, created_at: "2026-10-16T08:00:00Z" }, + added("2026-10-16T08:00:01Z"), + ], + runs: [{ ...laneRun({ at: "2026-10-16T08:00:04Z" }), updated_at: "2026-10-16T08:30:00Z" }], + }); + assert.equal(await sweep({ github, actions }), 0); + assert.deepEqual(writes(calls), [ + `POST /repos/${REPO}/issues/30/comments ${JSON.stringify({ body: strandedComment(KEY, "failed-again") })}`, + `POST /repos/${REPO}/issues/30/labels {"labels":["needs-human"]}`, + ]); + assert.ok(strandedComment(KEY, "failed-again").includes("failed or was cancelled before it reported")); +}); diff --git a/docs/conventions/pr-pipeline.yaml b/docs/conventions/pr-pipeline.yaml index 8b115d248a..de9f66234c 100644 --- a/docs/conventions/pr-pipeline.yaml +++ b/docs/conventions/pr-pipeline.yaml @@ -427,7 +427,18 @@ activities: # hold label, and whose agent-ready label was never removed. The label # starts resolve-issue. At the issue lane's queued cap (check-wip-cap) it # labels such an issue admission-deferred instead, and each hourly sweep - # admits the deferred ones oldest first within the room. Hourly it also hands an agent-ready issue whose + # admits the deferred ones oldest first within the room. A listed human + # adding admission-deferred to an issue with `status: ready` marks it + # decided (owner decision Q75): the sweep reads its work-class label, its + # Size, Complexity and Uncertainty issue fields (each counting only when a + # listed human or the lanes App set it) and the default branch's + # .github/actions/resolve-config/admission-rules.json, runs intake's + # admission rules, and admits it only on their admit verdict; a refusal + # moves it to `status: needs-decision` with a comment. Hourly it also + # re-fires an agent-ready issue left stranded (no run, or one that ended + # with no report, for three hours) once per version of the lane workflow + # files, then hands it to a person with needs-human and a comment; and it + # hands an agent-ready issue whose # lanes-App PR merged with `Refs:` or closed unmerged to triage # (needs-triage on, agent-ready off), lifting only the needs-human # report-outcome set for a merged Refs PR. A script, no model. @@ -461,8 +472,11 @@ activities: # After intake triage, for a thin issue (Flags: intent, or no task list in # the body): posts the read-only refine-issue run's output as one marked # comment. Exact: the drafted criteria as a task list (admission's - # criterion 7 counts it), the lanes App's `status: needs-decision` lifted - # and one more triage. Not exact: numbered questions with RECOMMENDED + # criterion 7 counts them only once a listed human posts them as their + # own, owner decision Q9); one more triage, lifting the lanes App's + # `status: needs-decision`, only when a listed human's text already holds + # a task list, else the labels stay and the comment asks a listed human + # to post the criteria as their own decision comment. Not exact: numbered questions with RECOMMENDED # answers at `status: needs-decision`. For an issue admission held at the # App's needs-decision for some, not all, done-when items a lane run # cannot perform, a split the model judges sound files one needs-human