work-package review mode: 27 definition defects from PR #1877 review run

[m2ux]

Context

A work-package v3.36.0 run in review mode (PR review of a third party's pull request, verdict posted here) surfaced 27 definition defects. The deliverable itself was sound; almost every defect is in the reporting and bookkeeping layer around it, and most were caught by user review rather than by the definition.

Two facts frame the list:

• The run's highest-value finding was produced by a dispatch the workflow cannot reach. dispatch-prism is selected when problem_complexity == complex, but a depth-1 worker has no dispatch primitive (harness-compat::depth-1-only), so it ran only because the orchestrator noticed and dispatched it out-of-band. That pass overturned the code review's own verified conclusion and produced both Critical findings the verdict rests on.
• Review mode suppressed every user gate. Four assumption-interview gates are conditioned is_review_mode != true, so the merge conditions the posted review asks the author to meet are agent positions that no human ratified.

Full register, with per-item evidence and reasoning: deferred-items.md (internal). Retrospective: 14-COMPLETE.md in the same folder.

Required fixes

Six items are marked required — each shipped broken output that had to be repaired by hand.

ID	Fix
DI-31	manual-diff-review must cite change blocks by permanent blob URL at the reviewed sha, never by review-worktree path. All 15 citations in the change-block index pointed into the worktree that remove-worktree deletes at close-out, so the artifact self-destructed inside the same run that produced it.
DI-35	Link engineering artifacts by branch (/blob/<branch>/), not by commit sha; keep the reviewed sha for code citations only. Sha-pinning the engineering side is the wrong trade for a living record — the reports keep changing after the ref is captured, so the pinned tree is stale as soon as anything is added. Supersedes DI-32: re-pinning later only moves the failure, branch-linking removes it.
DI-32	(Subsumed by DI-35, kept for the evidence.) artifact_publish_ref is captured before complete writes the close-out, retrospective, session trace and follow-ups register, so every report link in a posted review lands a reader in a tree that predates those files — four dead links, two clicks from a review on someone else's PR. This run went stale twice, at 682c2b1 and again at 43e52c2, each needing a hand re-pin plus a further edit to the posted review.
DI-33	Token accounting must count every dispatch. activity_usage records one entry per activity transition, so resumes, out-of-band dispatches, abandoned sessions and the close-out activities are all invisible. This run reported 2,751,188 tokens against an actual 4,130,657 across 33 dispatches — a third of the run missing, including the prism pass that produced the Critical findings.
DI-34	Split ownership between session-trace and token-usage: the trace reports mechanical execution only, cost lives solely in the cost artifact. Both currently report the same per-activity figures from the same ledger, so a wrong ledger produces two wrong artifacts — this run misattributed one activity's tokens to another and omitted a third entirely.
DI-23	Remove the @ (activity-prefix) column from the planning README Progress table. Note sync-progress-status selects rows by artifact_prefix, which is that column, so the matching rule needs a new anchor first.

Review mode gates the wrong things off

ID	Fix
DI-8	Gate unsigned-commits-prompt and resign-unsigned-commits on is_review_mode != true, as refresh-pr-body and apply-cleanup already are. Both are author-side actions; the checkpoint blocked an activity to ask whether to force-push a rewrite of another organisation's branch.
DI-18	Make the token/cost artifact available in review mode — ungate render-token-usage or give review-mode close-out its own cost summary. Review runs are not cheap runs.
DI-19	Make the retrospective interview runnable or drop the requirement. The resource mandates one item at a time and forbids batching; the activity provides one non-blocking 30s auto-advance confirm, and a depth-1 worker has no other interaction primitive. Twenty items went behind a single confirm.
DI-21	Ungate create-complete-doc for review mode. It is conditioned is_review_mode != true, so review mode skips the artifact complete-wp calls "the single terminal artifact" — while conduct-retrospective stays ungated and still produces content whose only declared home is that file. The close-out and retrospective ended up in the planning README.
DI-20	Close the review-mode binding gaps. assumption_decisions is structurally unsatisfiable in review mode (its only producer is a gate conditioned off). Eight further inputs have no producer at all: identifying_context, initiative_name, issue_request, discovered_path, comprehension_dir, force_rebuild, target_content, output_path.

Posted-comment format

The review-mode skeleton specifies table structure and nothing else, which is the single root cause behind most of this group.

ID	Fix
DI-9	Add a Strategic Review section to the skeleton with a findings table matching the other five. Strategic findings are routinely deferred to the author but have no canonical home, so the renderer invented a section and rendered it as prose.
DI-10	Make Reference, Don't Restate enforceable at render time. The rule exists but nothing checks it, so this run reproduced ~1,900 words of the code review inline.
DI-11	Constrain the prose register: plain language, short sentences, no stacked qualifications, no dense chains of symbols and line numbers.
DI-13	Rename the Source column to @, render each cell as a hyperlinked ASCII >, and move it to position 2. Filename link text forces the column wide and wraps every other cell.
DI-14	Drop the Disposition column from findings tables — the Action Items tiers already express it. Prior Feedback Triage is the carve-out and keeps it, moved to the far right.
DI-15	Apply Reference, Don't Restate to caveats too: one line, claim plus a link to the report section holding its basis and any confirmation procedure.
DI-16	Use Finding as the item-column header in every findings table, not Finding / Gap / Item by section.
DI-17	Give What This Change Gets Right its own section, between Strategic Review and Action Items. Findings tables carry only non-passing items by rule, so what a change does well has no home.

Planning README and Progress table

ID	Fix
DI-22	Replace ◐ as the in-progress icon with 🟡. ◐ is a thin outline glyph beside full-colour emoji, so the one status meaning "being worked on now" is the hardest to see on a dark background.
DI-24	Every Progress row whose activity produced an artifact must hyperlink its Item cell to it. Activity rows currently link nothing even when the activity wrote or amended a file.
DI-25	Keep the Description field succinct and free of hyperlinks; links belong in the Item cell.
DI-26	Item labels carry the human name only, never the filename — Close-out, not Close-out (COMPLETE.md).
DI-28	Progress status must track the deliverable, not the producer. When a step is skipped but its content lands elsewhere, link the row there and mark it complete — never cancelled-with-a-link.

Artifact routing and link integrity

ID	Fix
DI-27	Route in-task follow-ups to follow-ups.md and have close-out link that register rather than restating open work in its own table. Nothing in complete checks the register before writing the close-out.
DI-29	Unlink any Progress row whose seeded artifact was never created. readme-seed pre-links every row to the file it would produce, so any skipped activity leaves a dead link.
DI-30	Add a link check over every planning artifact as a close-out gate: no missing relative target, no citation into the review worktree, and no resource id emitted as a relative link. Three distinct classes shipped dead in one run, 19 links total.

Suggested sequencing

DI-35 is the cheapest of the required set and closes the failure mode this run hit twice — do it first, and DI-32 needs nothing further afterwards. DI-31, DI-33 and DI-34 are independent and each fixes output that is currently wrong on every run. DI-9 through DI-17 are one change to the review-mode resource and are best done together. DI-23 needs the sync-progress-status matching change first. DI-21 and DI-18 are one-line condition removals.

Verifying a fix

Two checks caught most of this and are worth having as close-out gates rather than as manual passes (DI-30):

• resolve every relative link in every planning artifact at the ref the posted review points at, not just in the working tree — the working-tree check passes while the published tree is broken;
• reconcile the token ledger against the actual dispatch count. A run whose ledger says 11 dispatches and whose harness says 33 is understating cost by a third.