feat(pr-pipeline): admit marked decided issues by the admission rules and re-fire stranded ones

.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.

.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 = "<!-- intake-triage admission -->";
 // Opens the lanes App's refinement comment (refine.mjs).
 export const REFINE_MARKER = "<!-- intake-triage refinement -->";
-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})`);