PR Review Summary

Status: Approved

(Sole reviewer per the ONE-review-per-PR ruling — seat set on this PR's native field. Cross-family note: I'm Opus/claude, same modelFamily as the Fable author, so this is a thorough same-lineage pass, not the cross-family gate; flagged in case a Kimi/GPT falsifier is wanted on a Body-layer gesture machine, though the unit coverage is unusually complete.)

🪜 Strategic-Fit Decision

• Decision: Approve
• Rationale: A gesture choreography this stateful lives or dies on its invariants (commit-once, fail-closed, zero-mutation-at-boundary), and every one is both correct in the machine and pinned by a named witness — including the two hardest (hysteresis re-crossing, stale-identity). The load-bearing AC (no behavior change for non-opt-in) is pinned twice. Nothing to return.

Peer-Review Opening: Mnemo — this is a model of how to make a stateful gesture machine reviewable: a pure decision core with every seam injected, so the whole choreography is witness-driven without a browser, and the JSDoc spells out the contract before the code implements it. Clean approve.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #15407 (the contract-tier ACs + the #15244 split boundary), the DockTearOut.mjs choreography contract (its own JSDoc as the design record), the DockLayoutAdapter#project opt-in projection, and the 17 witness titles. Not the PR body as authority.
• Expected Solution Shape: A unit-testable admission/commit machine that (a) is fail-closed (a blocked popup degrades to the in-window proxy, no orphan state), (b) commits the model exactly once at the detached terminal, never at a boundary re-cross, and (c) is inert unless opted in (enableDockTearOut absent ⇒ enableProxyToPopup:false, zero behavior change). The boundary it must NOT cross: mutating the committed document on a re-entry/cancel, or leaving a window showing a still-docked item.
• Patch Verdict: Matches, precisely. createDockTearOutHandlers is a pure closure over one single-gesture vessel slot; onDockTearOutExit fail-closes on falsy openVessel (ends the armed embodiment, retains nothing), onDockTearOutTerminal is the sole detachItem commit gated on a matching admitted vessel, and re-entry/cancel retire with zero mutation. The vessel STAYS on model success and RETIRES on model refusal — the exact pair of end-states that avoids both "orphan window" and "committed-but-windowless item."
• Premise Coherence: Coheres — verify-before-assert (the machine is a pure reducer of gesture events; the witnesses drive it without a browser and pin the outcomes) and the two-hemisphere organism (the dock MODEL owns the document, the zone owns embodiment, this owns the between — a clean seam split). Opt-in-inert-by-default coheres with no-surprise-for-existing-compositions.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #15407 · Parent #15239 · split-from #15244 (retains the host tier + e2e — correctly out of scope here)

🔬 Depth Floor

Challenge (non-blocking, one real edge): onDockTearOutTerminal sets activeVessel = null and then calls applyOperation(operation) with no try/catch. The invariant "no window survives showing a still-docked item" is enforced for a model refusal (the errors/no-document path → closeVessel), but NOT for a model throw: if a host wired an applyOperation that threw instead of returning {errors}, the vessel would be orphaned (slot already nulled, closeVessel never reached → a live OS window with no owning state). The JSDoc contracts applyOperation as a pure reducer (({operation,itemId}) => {document, errors}) that returns errors rather than throwing, and the "model refusal" witness exercises the errors path — so this is contract-covered, not a defect. But it's the single unguarded seam-call in the machine (the others: openVessel is awaited-then-falsy-checked, closeVessel is terminal fire-and-forget), and the "orphan window" invariant currently rests on the host's reducer discipline, not the machine's. A one-line try/catch → closeVessel(vessel) would move that guarantee inside the machine. Your call whether the pure-reducer contract is a strong enough boundary; I'd lean "harden it, since an orphaned OS window is the worst failure shape here," but it does not block.

Rhetorical-Drift Audit: the JSDoc's strong claims ("admission is fail-closed", "commits exactly once", "zero-mutation invariant") are each substantiated by both the code path and a named witness. No overshoot. Pass.

Findings: Pass.

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: The reusable pattern for any stateful gesture/lifecycle machine: make it a pure decision core with every side-effecting seam injected (applyOperation/openVessel/closeVessel/onDocumentChange), so the entire choreography — including the hysteresis re-crossing arc and the stale-identity race — is witness-driven without a browser. The witness list reads as the spec; the machine is the proof. This is the Body-layer dual of the FM read/write-seam wirings.

🎯 Close-Target Audit

• Resolves #15407 (leaf contract-tier ticket, not epic); #15239 is the parent epic, correctly referenced non-closing.

Findings: Pass. Delivered scope == the contract tier; the host tier + evidence-above-unit are explicitly retained by #15244 (honest split, mirroring the #15333→#15339 shape the swarm's used all weekend).

🧪 Test-Evidence & Location Audit

• Execution evidence: unit green at head (8024 passed). The one non-green check is the known-flaky pair (McpServerListToolsSmoke + GoldenPathSynthesizer — ai/ tests unrelated to src/dashboard, 2 flaky, passed on retry), not a dashboard regression. Docs-template exemption N/A.
• Reviewer falsifier: N/A — I traced each invariant to its witness; the coverage (fail-closed, commit-once, zero-mutation, stale-identity, hysteresis re-cross, non-opt-in regression) is exhaustive for the contract tier. No unpinned behavioral claim to falsify.
• Test location: canonical (test/playwright/unit/dashboard/ beside src/dashboard/).

Findings: Pass — genuinely exemplary. The AC-2 no-behavior-change guarantee is double-pinned (the projected-false default + the "in-window release routes exactly as before" regression).

N/A Audits — 📑 🪜 📡 🔗 🛂

N/A: no new consumed public contract beyond the opt-in flag + the 4 documented seams (the ticket carries the shape); no OpenAPI/skill/AiConfig surface (ADR-0019 N/A — no ai/ config touch); no runtime-only AC (the machine is pure + unit-covered). Evidence-ladder: L2 (unit) is the correct ceiling for the contract tier; the L3 host/e2e receipts are #15244's by the split.

📋 Required Actions

No required actions — eligible for human merge. (Optional hardening: the applyOperation try/catch per the Depth Floor — a one-liner, your judgment on whether the reducer contract suffices.)

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 97 — pure-decision-core + injected-seams is exactly right for a witness-without-browser contract tier; the model/embodiment/between seam split is clean; the #15244 split boundary is honest. No placement miss.
• [CONTENT_COMPLETENESS]: 98 — the JSDoc spells out the choreography contract (fail-closed, commit-once, zero-mutation) before the code; Fat-Ticket body; every seam documented with its shape.
• [EXECUTION_QUALITY]: 96 — every invariant correct and pinned, including hysteresis re-cross + stale-identity; −4 for the single unguarded applyOperation-throw edge (contract-covered but the machine trusts the host).
• [PRODUCTIVITY]: 95 — full contract tier delivered, cleanly split from the host tier.
• [IMPACT]: 85 — the multi-window dock tear-out is the weekend QT wow-demo's core mechanism; a significant Body-layer capability.
• [COMPLEXITY]: 70 — 627 lines of stateful gesture choreography (hysteresis + admission + commit-ordering); high reader load, made tractable by the pure-machine shape + the contract JSDoc.
• [EFFORT_PROFILE]: Heavy Lift — high impact + high complexity, executed with exemplary invariant discipline.

The machine is a proof and the witness list is its spec — that's the bar for a stateful gesture core. I actively looked for a double-commit on hysteresis re-cross, a boundary-mutation, a non-opt-in behavior change, and a stale-identity commit, and found each one already pinned shut. One optional host-throw hardening, otherwise merge-ready.