Require explicit delivery attribution before closeout

CHANGELOG.md

@@ -8,6 +8,7 @@
 
 ### Fixed
 
+- Require explicit delivery attribution before merged-PR closeout ([pm-github-uqjf](https://github.com/unbraind/pm-github/blob/main/.agents/pm/issues/pm-github-uqjf.toon))
 - The daily release regenerates the changelog with a different status filter than release:check, so an open changelog-unreleased item delivered since the last tag fails the release ([pm-github-l1ot](https://github.com/unbraind/pm-github/blob/main/.agents/pm/issues/pm-github-l1ot.toon))
 - Release timeout must report current workflow approval state ([pm-github-b145](https://github.com/unbraind/pm-github/blob/main/.agents/pm/issues/pm-github-b145.toon))
 - Preserve legacy GitHub ownership across copied provider examples ([pm-github-b143](https://github.com/unbraind/pm-github/blob/main/.agents/pm/issues/pm-github-b143.toon))

README.md

@@ -39,15 +39,17 @@ Only `https://github.com/owner/repo/blob/main/.agents/pm/<folder>/<id>.toon`
 links in PR bodies are recognized. Percent-encoded names and nested SDK tracker
 folders are supported; malformed encodings and traversal segments are ignored. Repository comparisons ignore case. Links to
 other repositories, unknown ids, closed/canceled items and `--skip` ids produce
-explicit skip rows. Bare ids and `pm:` trailers are not supported. If several
-selected PRs reference one item, the lowest PR number supplies its evidence and
-later references are skipped.
+explicit skip rows. Bare mentions and `pm:` trailers are not delivery evidence. Use a whole-line
+`Closes <item-id>` or `pm-github: <item-id>` declaration, a closing keyword before
+the canonical item link, or an item field `github_pr`, `pr_url` or
+`pull_request_url` containing the exact PR URL. Multiple merged PRs explicitly
+claiming one item are reported as `ambiguous_delivery`; none closes it.
 
 **A mention is not delivery.** PR bodies routinely link follow-ups, epics and
 related work for context, so a link alone never closes an item. An item is
 closed only when:
 
-- it is `in_progress` (claimed work), **or** its link is preceded on the same
+- its delivery is explicitly declared; a canonical link may be preceded on the same
   line by a GitHub-style closing keyword: `Closes`, `Fixes`, `Resolves` or
   `Delivers` (and their tense forms), for example `- Closes [pm-x](…/pm-x.toon)`;
 - it is not an `Epic`; epics close when their children are done;

closeout.ts

@@ -20,7 +20,7 @@ export interface CloseoutRow {
 /** Stable decisions used by automation and compact skip counts. */
 export type CloseoutCode = "already_terminal" | "other_repository" | "duplicate_selection" | "explicit_skip"
   | "unknown_item" | "epic" | "active_children" | "not_delivery" | "close_failed"
-  | "head_checks_passed" | "head_checks_failed" | "head_evidence_unavailable" | "required_checks_unverified" | "incomplete_merge";
+  | "ambiguous_delivery" | "head_checks_passed" | "head_checks_failed" | "head_evidence_unavailable" | "required_checks_unverified" | "incomplete_merge";
 
 /** Which head-check policy was evaluated for one selected pull request. */
 interface CheckGate {
@@ -260,6 +260,18 @@ const TERMINAL_STATUSES: readonly string[] = ["closed", "canceled"];
 /** GitHub-style closing keywords that declare delivery of the link they directly precede. */
 const CLOSING_KEYWORDS = new Set(["close", "closes", "closed", "fix", "fixes", "fixed", "resolve", "resolves", "resolved", "deliver", "delivers", "delivered"]);
 
+/**
+ * Preserve the original non-Unicode regex case rules for declaration lookup.
+ * @param id - Literal item ID or declaration token.
+ * @returns Its case-insensitive key without expanding characters or ASCII lookalikes.
+ */
+function declarationKey(id: string): string {
+  return id.replace(/[\s\S]/g, character => {
+    const upper = character.toUpperCase();
+    return upper.length !== 1 || (character.charCodeAt(0) >= 128 && upper.charCodeAt(0) < 128) ? character : upper;
+  });
+}
+
 /**
  * Whether a closing keyword directly precedes a link, given the line text before the link.
  *
@@ -392,6 +404,7 @@ export async function runCloseout(pmRoot: string, repo: string | undefined, opti
     if (typeof parent === "string" && !TERMINAL_STATUSES.includes(item.status)) activeChildren.set(parent, (activeChildren.get(parent) ?? 0) + 1);
   }
   const selected = new Set<string>();
+  const candidates = new Map<string, Set<number>>();
   const plan: CloseoutRow[] = [];
   const checks: CheckGate[] = [];
   const closures: { row: CloseoutRow; reason: string; at: string; resolution: string; expectedResult: string; actualResult: string }[] = [];
@@ -400,7 +413,31 @@ export async function runCloseout(pmRoot: string, repo: string | undefined, opti
     let gate: CheckGate = { pr: number, code: "incomplete_merge", rule: "unavailable", ok: false, passed: [], reason: "Pull request is not merged or merge evidence is incomplete." };
     if (mergeEvidence(pr)) gate = await checkCloseoutHead(api, repo, pr, declared);
     checks.push(gate);
-    for (const ref of extractCloseoutItems(pr.body ?? "")) {
+    const refs = extractCloseoutItems(pr.body ?? "");
+    // Scan declarations once per PR, retaining case-insensitive IDs and optional punctuation.
+    const declarations = new Set<string>();
+    for (const match of (pr.body ?? "").matchAll(/^(?:[-*][ \t]+)?([a-z]+|pm-github:)[ \t]+(\S+)[ \t]*$/gim)) {
+      const keyword = match[1]!.toLowerCase();
+      if (!CLOSING_KEYWORDS.has(keyword) && keyword !== "pm-github:") continue;
+      const id = declarationKey(match[2]!);
+      declarations.add(id);
+      if (/[.!]$/.test(id)) declarations.add(id.slice(0, -1));
+    }
+    for (const item of items.values()) {
+      // Only canonical delivery metadata or a whole-line trailer/closing declaration is authoritative.
+      const linked = [item.github_pr, item.pr_url, item.pull_request_url].includes(pr.html_url);
+      if (linked || declarations.has(declarationKey(item.id))) {
+        const ref = refs.find(ref => ref.item === item.id && ref.repo.toLowerCase() === repo.toLowerCase());
+        if (ref) ref.declared = true;
+        else refs.push({ item: item.id, repo, declared: true });
+      }
+    }
+    for (const ref of refs) {
+      if (ref.declared && pr.merged && ref.repo.toLowerCase() === repo.toLowerCase()) {
+        const choices = candidates.get(ref.item) ?? new Set<number>();
+        choices.add(number);
+        candidates.set(ref.item, choices);
+      }
       let reason: string;
       let code: CloseoutCode;
       if (ref.repo.toLowerCase() !== repo.toLowerCase()) { code = "other_repository"; reason = "Link points to another repository."; }
@@ -410,9 +447,9 @@ export async function runCloseout(pmRoot: string, repo: string | undefined, opti
       else if (!gate.ok) { code = gate.code; reason = gate.reason; }
       else if (items.get(ref.item)!.type === "Epic") { code = "epic"; reason = "Epics close when their children are done, not by a pull request."; }
       else if (activeChildren.has(ref.item)) { code = "active_children"; reason = `Item has ${activeChildren.get(ref.item)} non-terminal child item(s).`; }
-      else if (items.get(ref.item)!.status !== "in_progress" && !ref.declared) {
+      else if (!ref.declared) {
         code = "not_delivery";
-        reason = `Item is ${items.get(ref.item)!.status}, not in progress; a mention is not delivery. Prefix its link with Closes, Fixes, Resolves or Delivers to close it.`;
+        reason = "A mention is not delivery, regardless of item status. Declare Closes, Fixes, Resolves or Delivers, link the item to this PR, or use a pm-github trailer.";
       }
       else if (selected.has(ref.item)) { code = "duplicate_selection"; reason = "Item already selected by another pull request."; }
       else {
@@ -432,10 +469,20 @@ export async function runCloseout(pmRoot: string, repo: string | undefined, opti
       plan.push({ item: ref.item, pr: number, action: "skip", code, reason });
     }
   }
+  // Resolve attribution across the complete selection before the first SDK write.
+  for (const row of plan) {
+    const claimants = candidates.get(row.item);
+    if (claimants && claimants.size > 1 && claimants.has(row.pr) && !["already_terminal", "other_repository", "unknown_item", "explicit_skip"].includes(row.code)) {
+      row.action = "skip";
+      row.code = "ambiguous_delivery";
+      row.reason = "Multiple merged PRs explicitly claim delivery; select the intended PR before closing.";
+    }
+  }
   let closed = 0;
   let failed = 0;
   if (apply) {
     for (const entry of closures) {
+      if (entry.row.action !== "close") continue;
       try {
         await close(entry.row.item, entry.reason, { author: "pm-github", completedAt: entry.at,
           resolution: entry.resolution, expectedResult: entry.expectedResult, actualResult: entry.actualResult,

docs/explicit-closeout-attribution.md

@@ -0,0 +1,30 @@
+# Explicit closeout delivery attribution
+
+pm-ado issue [33](https://github.com/unbraind/pm-ado/issues/33) identified a durable provenance error: merged PR 16 reported the statement-enforcement gap, while PR 19 delivered enforcement. Both bodies link the same tracker item. The previous rule let any in-progress item close on a bare body link, then ascending PR order gave PR 16 first choice.
+
+Closeout now requires explicit evidence regardless of the item's status:
+
+- A closing keyword immediately before a canonical tracker link.
+- A whole-line closing declaration such as `Closes pm-ado-5w3m`, with spaces or tabs between the keyword and ID.
+- A whole-line `pm-github: pm-ado-5w3m` trailer, with spaces or tabs between the trailer and ID. A newline cannot join a declaration.
+- The item's `github_pr`, `pr_url` or `pull_request_url` set to the exact PR URL.
+
+The entire selected PR set is planned before any SDK close. If multiple merged PRs explicitly claim the same item, every claimant is reported as `ambiguous_delivery` and no candidate closes it. A PR that only mentions the item retains `not_delivery`. Existing required-check, child-item, epic, skip and terminal-item safeguards still apply. Titles and incidental mentions never establish delivery.
+
+The minimal fixtures retain the real titles, relevant body paragraphs and commit messages fetched with `gh pr view -R unbraind/pm-ado 16 --json title,body,commits` and the corresponding PR 19 command. Neither historical body explicitly declares delivery. Both are therefore refused until an explicit link or declaration is supplied; with the item's PR URL set to 19, closeout uses its October 8 merge chronology. Existing proposed pm-ado PR 32 tracker history is not rewritten here.
+
+`node --test test/closeout-attribution.test.ts` covers seven real-tracker cases, including multiline declarations, two claimants plus a mention-only PR, and a many-item/many-PR plan. Restoring only the old in-progress bypass in runCloseout makes both original real-shape provenance regressions fail: PR 16 becomes eligible again and the explicit PR 19 link cannot select the intended delivery. Review-round function-only proofs independently restore the old declaration regex or remove claimant membership while retaining the tests and other fix. Each corresponding regression fails with ERR_ASSERTION (exit 1). The six original cases and the compact single-delivery control remain covered unchanged.
+
+Before review round 1, both npm run release:check and bun run release:check exited 0 at code head b21ff5bdbdcfdab6c97b1204d73ff528b0f65745. Each ran 897/897 tests with zero failures, cancellations or skips. Both measured exact 100/100/100/100 over all 14 authored modules: 10471/10471 lines and statements, 274/274 functions and 3187/3187 branches. Dependency pins, source exemptions and thresholds are unchanged. The focused compact-output single-delivery case also passed. Final tracker/documentation receipts are regenerated and checked separately; final-head CI and fresh-clone strict health are reported in the PR.
+
+Review round 1 completed both npm run release:check and bun run release:check with exit 0 at code head c9a29d199ac9982e842e28cfdf6b75fb014572c3. Each passed 899/899 tests, zero failures/skips/cancellations and exact 100/100/100/100 over 14 authored modules. Both measured 10472/10472 lines and statements and 274/274 functions; branches were 3187/3187 (npm) and 3185/3185 (Bun invocation). All release-check stages passed with unchanged thresholds, source scope, helper/test budgets and dependency pins. The [review-round tracker receipt](../.agents/pm/history/pm-github-uqjf.jsonl) records completed results separately from earlier pending comments. [Code-head CI](https://github.com/unbraind/pm-github/actions/runs/39174854340) passed Node 22/26, each with 899 Node tests, 899 native Bun tests across 41 files and exact authored-source coverage, plus CodeQL and Semgrep. Receipt-head CI and fresh-clone health are recorded in the PR after that head is pushed.
+
+The first concurrent Bun attempt hit the existing 500 ms startup-watchdog probe tracked by [pm-github-sah1](../.agents/pm/issues/pm-github-sah1.toon): the child had not printed ready before its deadline. That already-failed attempt was stopped; cancellation diagnostics from stopping it are not additional regressions. The unchanged isolated watchdog suite passed 2/2. The complete Bun rerun, started after the other local gates finished, passed the original watchdog and all 899 tests. The startup-sensitive probe remains a separate open follow-up; this PR changes neither its helper nor its budgets.
+
+The follow-up merge incorporates main's PR 149 at 68e7b89 through merge commit c0aba84a9a57651a3e5cbeee0273916f8798153c. Only CHANGELOG.md conflicted: main's version was selected, then regenerated after tracker writes. PM files merged cleanly and no clone-local receipts were generated, so reconciliation was unnecessary. An independent fresh `--no-local` clone passed strict health with merge drivers required and tracker validation.
+
+The subsequent performance review removes per-item declaration regex construction and repeated body scans. Each PR body is scanned once into a declaration-ID set, preserving whole-line boundaries, spaces/tabs and optional punctuation. Lookup keys retain the original non-Unicode regex case rules, including rejection of a Unicode Kelvin-sign lookalike for ASCII k. The six existing attribution tests remain byte-for-byte unchanged; the complete focused suite passes 7/7. Its new real-SDK case uses 40 items and 12 PRs and verifies 24 distinct delivery choices, two ambiguous claimants, twelve mention-only rows and unchanged item statuses, including an item whose ID is imitated by the Kelvin sign. The same bulk-result case passes against the pre-refactor function, as expected for this behavior-preserving optimization; it makes no timing assertion. Reverting only the key function to ordinary lowercasing makes the bulk result assertion fail with ERR_ASSERTION (exit 1); restoring the key passes. Earlier correctness regressions and their failing function-only revert proofs remain applicable. Completed combined-head gate receipts are recorded separately in the tracker and PR, scoped to their exact code heads.
+
+The completed combined follow-up passes both `npm run release:check` and `bun run release:check` at code head 23495cf5469f0d0b6f5795b147a852dde4ca41fb. Each exits 0 with 910/910 tests and zero failures, skips or cancellations. Both measure exact 100/100/100/100 over all 15 authored modules: 10518/10518 lines and statements, 276/276 functions and 3216/3216 branches. Every release-check stage passes with unchanged thresholds, source scope, helper/test budgets and dependency pins. The corrected PM-linked run also passes both commands: attribution 7/7 and compact dry-run 1/1. Its project policy disables result storage, so the completed [tracker comments](../.agents/pm/history/pm-github-uqjf.jsonl) provide the durable receipt.
+
+A separate concurrent attempt at the earlier ed63b0e head reproduced the same startup-watchdog flake: empty output before the unchanged 500 ms deadline. That already-failed run was stopped; the unchanged isolated watchdog passed 2/2. Interrupted and superseded runs are not acceptance receipts. The complete corrected Bun gate ran after npm finished and passed the original watchdog plus all 910 tests. Final receipt-head CI and independent fresh-clone health are recorded in the PR after that head is prepared.