Container close-out review — invocable cumulative review of a shipped container against its spec

[kyle-sexton]

Parent

Refs #2933. Split out of Lane D (#2937) on 2026-08-19 after adversarial validation showed the originally-designed mechanism was broken and structurally larger than the mode addition it rode on. Lane D keeps the spec lens + ladder + C13/C15/C16 verdicts; this item owns the container-scoped review.

What to build

When the last sub-item of a spec container closes, one cumulative review of the shipped whole — across however many PRs — against the container's spec body. Two skills already route to this and currently land on nothing: plugins/work-items/skills/decompose/SKILL.md:244-252 ("a close-out review of the shipped whole against the container body" → "the review plugin's spec-fidelity machinery when installed") and plugins/work-items/skills/ship/SKILL.md:107-108 (both all-sub-items-closed rows, split by execution shape). planning:plan close-out step 4 sequences the same ritual. Container #2933's own acceptance criteria require this before it can close.

Constraints established by validation (do not re-derive)

• No seam verb returns a closing PR. The seam's public surface is create-item, get-item, claim, renew-lease, reclaim, link-blocks, add-sub-item, list-sub-items, list-frontier, capabilities. The only closing-PR machinery is a GitHub-only GraphQL query over closedByPullRequestsReferences (adapters/github/README.md, used by reclaim.sh) — and its documented reduction filters MERGED out, the inverse of what close-out needs.
• "Union of merge commits" is not a usable basis. This repo squash-merges by default (plugins/source-control/skills/pull-request/SKILL.md:39,77), which produces no merge commit and no second parent; docs/conventions/topic-docs/README.md:479-481 confirms squash carries none of the branch ancestry. A commit set is also not a diff range without a named base.
• The basis differs per execution shape. For integration branch → single PR (plugins/work-items/reference/execution-shape.md:63) there are no per-item PRs at all — the container's recorded **Integration branch:** line is the handle. For per-item PRs, the squash commits on the default branch are.
• The container body is not readable through the seam (no body field in the normalized item object) — it needs the provider-mechanic read, presence-gated, same as Lane D's corrected ladder rung 2.
• quality-gate declares ONE review diff base (quality-gate/SKILL.md:25-27) and parses no flags today (fanout has the only flag-parsing precedent). A container-scoped basis is a structural change to that skill, not a mode row — which is why placement is re-opened here rather than assumed.
• The contract slice is pruned before merge, so at close-out time docs/topics/<slug>/PLAN.md is gone from main and recovery is best-effort — the tracker container body is the only durable spec source at this moment.
• Non-GitHub providers: jira declares list-sub-items: false (exit 6) and local-markdown has no PR concept and is barred from containers entirely — state the degradation honestly rather than pretending neutrality.

What to decide

Placement (a quality-gate mode with an explicit second basis / its own skill / a work-items ritual calling review machinery), the per-shape diff-basis derivation, and how the two existing routes get updated to name the real target.

Acceptance criteria

• An invocable, documented close-out review flow with a diff basis derivable for BOTH execution shapes
• decompose/SKILL.md and ship/SKILL.md routes updated to name it (they currently point at machinery that does not exist)
• Provider degradation stated (GitHub-only paths named as such; jira/local-markdown behavior explicit)
• Container Spec: Shipping-lifecycle absorption (Pocock Shipping section) #2933 can run it against itself — the dogfood test

Blocked by

• Lane D: review deltas — spec lens, two-axis doctrine, discovery ladder, preflight, suppression rules, container close-out review #2937 (Lane D's spec mode is the most likely host and settles the finding-class enum this reuses)

[kyle-sexton]

Resolved design + dogfood run

Shipped on claude/pocock-shipping-breakdown-ukohlx (3250aac6). The three open decisions from "What to decide" resolved as follows.

1. Placement — a close-out mode in /review:quality-gate

Not its own skill, not a work-items ritual. context/spec.md already owns the finding-class enum, the spec-line quoting rule, the item-content-trust fence, the dispatch policy, and both-directions judging; a sibling skill would have to restate or awkwardly cite all of it across a skill boundary, and the work-items option needed quality-gate to grow flag parsing for an injected base — a larger structural change than a mode row.

close-out is spec mode at container scale, reusing all of the above by citation. What it owns is what is judged: which container, which spec body, which change set.

The structural change the issue flagged is real but bounded, and lands as a mode-scoped override, precedented by Step 0.5's own mode-scoping:

Review diff base — mode-scoped override first: close-out does not use this base at all. A spec container is not a branch, so that mode derives a container-scoped basis of its own, per execution shape.

2. Diff basis — a range for one shape, a commit SET for the other

• integration branch → single PR — the **Integration branch:** line names the branch, one PR carries the journey, so the basis is an ordinary range: merge-base(base, head)..head while open, the squash commit once merged.
• per-item PRs (default, and what an absent line means) — a commit set, not a range, read as the union of per-commit diffs. This is the direct answer to the issue's "a commit set is also not a diff range without a named base": it isn't one, and it shouldn't be forced into one. A two-dot <first>..<last> over the default branch sweeps in every foreign commit merged between the container's first and last item, and the review then reports findings against work the container never shipped. The cost — cross-item interactions are read across diffs, not in one composite hunk — is stated in the report rather than hidden.

Closing-commit ladder: provider close-linkage → heuristic subject scan (flagged as such) → ask → skip with a note. Rung 1 reuses closedByPullRequestsReferences reduced the inverse way from the github adapter's in-flight check — that one keeps OPEN and drops MERGED; close-out wants exactly the MERGED nodes and their mergeCommit.oid. A failed query is never read as an empty set.

3. Dogfood against #2933 — and what it changed

Ran as a --dry-run (the container is at 15/20, so a real run correctly stops at the finished-rollup gate — which is itself the gate firing as designed).

• Step 1 — container resolved at rung 1 (explicit --container).
• Step 2 — body read via provider mechanic. 3 acceptance criteria; ## Out of scope present, so scope-creep is reachable.
• Step 3 — no **Integration branch:** line → shape per-item PRs, basis = commit set.
• Step 4 — gate fires: 5 sub-items open (Lane C: TDD wiring + zero-assembly default chain #2936, Linear adapter with full verb parity #2946, Adapter-onboarding skill — generator for the provider tail #2950, Gitea/Forgejo adapter — generator dogfood candidate #2952, and this one).

Two corrections came out of actually running it, both now in the shipped file:

1. Investigation: multi-provider topology — source-of-record + coordination split #2945 closed with zero PRs — it's an investigation item; it closed on a recorded decision comment and produced no code by design. My first draft would have reported that as unresolved, i.e. a coverage gap. It isn't. A successful close-linkage query returning no merged PRs is an answer: classify no-code, keep it out of the basis, judge its criteria against its closing comment. Only a failed query falls to the scan, and only the scan yields unresolved. The two are kept apart in the report because they are different claims.
2. The subject scan is noisier than expected — e606106f, the commit that published the board, matched every one of the 15 closed sub-items, because it lists them all. Investigation: multi-provider topology — source-of-record + coordination split #2945 and Jira write support — guarded, opt-in #2951 matched nothing else. So rung 2 now drops candidates referencing more than a couple of the container's sub-items (that's journey narration, not linkage) and ranks closing-keyword forms above bare mentions.

Acceptance criteria

• Invocable, documented close-out flow with a basis derivable for both execution shapes
• decompose/SKILL.md and ship/SKILL.md routes name the real target (/review:quality-gate close-out --container <id>), presence-gated, manual fallback kept. The review produces the verdict; the ship ritual keeps the close — a missing/wrong finding against a stated criterion keeps the container open
• Provider degradation stated: GitHub-only in practice; jira (list-sub-items: false, no merge-commit concept) degrades to asking; local-markdown is barred from containers and gets no path
• Spec: Shipping-lifecycle absorption (Pocock Shipping section) #2933 can run it against itself — and did, changing the design twice

review 0.22.0 → 0.23.0, work-items 0.36.2 → 0.36.3. Repo gates green (changelog parity ×3, skill portability, changed-skills lint, leaf names, count claims, cross-plugin drift, markdownlint, review's standards-binding suite).

---

Generated by Claude Code