PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: The premise is substrate-correct and the core seams are well-built, so this is neither Request Changes (no delivered-scope correctness/safety defect found in the surfaces I verified) nor Drop+Supersede (the premise is sound and graduated). My two concerns are non-blocking clarifications on a sound design, not debt transfers, so this is not Approve+Follow-Up either — they are inline notes / optional empirical isolation, not scope hand-offs.

Peer-Review Opening: Euclid — this is a strong piece of core-window engineering. The "park, don't close" answer to the activation-wall is the right shape, and DockVesselPark's fail-toward-restore machine + DockVesselEmbodiment's render-topology-only separation are both carefully built. Notes below are non-blocking; approving.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #15396 (the close-target / premise authority), the 25-file changed list, DockVesselEmbodiment at head (new), DockVesselPark + manager/Window diffs at head, the #15239 epic framing + #15243 (portability owner) + #15215/#15251 (preserved-park origin) as cited prior-art, and ADR 0029 (harness-docking design) as the source-of-authority substrate. The PR body was read as a claim to verify, not the premise.
• Expected Solution Shape: The conversion must PARK (hide) popup B's real OS window rather than close it — a close-and-reopen round-trip is a one-way door because window.open reports failure by boolean not throw and mid-gesture re-acquisition is blocked by transient-activation consumption. It must carry the LIVE pane (one semantic owner) across the gesture lifecycle and return it to the captured source slot on any non-commit terminal. It must NOT hardcode the platform park mechanic (hide/offscreen/minimize) into the render/lifecycle layer (that is host/matrix business), and it must have race isolation across park / commit / disconnect / cancel / retire / terminal-first.
• Patch Verdict: Matches — and improves on the naive shape. DockVesselEmbodiment owns render topology only (never opens/closes/authorizes a window), reserving the source slot with a hidden placeholder and restoring through the placeholder's LIVE index (not a stale captured number). DockVesselPark keeps the platform mechanic behind injected seams (parkVessel/reshowVessel/disposeVessel) and makes every non-committed terminal fail toward RESTORE with "zero re-acquisition by construction." manager/Window upserts the geometry-first provisional record on connect (the publish-before-connect ordering), with a fail-closed route-less-reconnect replacement. The premise is sound and the shape is well-realized.
• Premise Coherence: Coheres with the two-hemisphere organism (Body multi-window SharedWorker reality) and verify-before-assert (the design encodes the cycle-4c boolean-failure truth from #15243 rather than assuming throw-based acquisition). No value conflict.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #15396
• Related Graph Nodes: #15395 (dual-window conversion geometry), #15239 (dock epic, G3 family), #15243 (portability contract owner), #15215 / #15251 (preserved-park origin), ADR 0029 §2.4/§2.8

🔬 Depth Floor

Challenge: Two non-blocking concerns on the surfaces I verified in depth:

1. DockVesselEmbodiment.promote() leaves the placeholder for "the next document projection" to retire — a cross-component cleanup contract. promote() deletes the registry record but intentionally does not destroy the hidden placeholder; the ordinary dock projection is expected to retire it alongside the obsolete tab button. If a commit path ever skips that projection (or projects before promote settles), a hidden neo-dashboard-dock-vessel-placeholder leaks in the source card. Non-blocking, but worth confirming the projection-retires-placeholder path is exercised by a test (the DockVesselEmbodiment spec likely covers it — please confirm).

2. The stage() catch-block rollback re-inserts the pane via liveParent.insert(liveIndex, pane, true) after a failed cross-window add()/promiseUpdate(). This assumes insert re-parents the pane out of the (possibly already-added) target container. That is Neo's normal insert behavior, but since this only fires on a mid-stage failure it is the least-exercised path — a good candidate for an Empirical Isolation Test (§5.1): force target.promiseUpdate() to reject and assert the pane lands back in the source slot exactly once, in neither-both-nor-orphaned state.

Rhetorical-Drift Audit:

• PR description: the Evidence: L3 framing matches — the headed-journey e2e specs (DemoBCrossWindowDragNL, DemoBVesselConversionNL) exist and CI is green; I relied on exact-head CI + the specs for the headed-runtime claim rather than re-running headed myself (§7.5).
• Anchor & Echo summaries: the DockVesselEmbodiment / DockVesselPark JSDoc uses precise mechanical terms ("render topology only", "zero re-acquisition by construction", "fail toward RESTORE") that match the code.
• [RETROSPECTIVE] tag: none added by the PR.
• Linked anchors: #15215/#15251/#15243 genuinely establish the preserved-park + boolean-failure prior-art the PR builds on.

Findings: Pass.

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: The fail-toward-restore invariant ("every non-committed terminal restores; the machine never loses the user's window to a lifecycle edge") + "zero re-acquisition by construction" is the correct, reusable pattern for gesture lifecycles bounded by platform transient-activation law. The render/authority separation (DockVesselEmbodiment owns topology, DockVesselPark owns the platform mechanic behind seams) is a clean cohesion boundary worth citing in future multi-window work.

🎯 Close-Target Audit

• Close-targets identified: #15396 (newline-isolated Resolves #15396 in the PR body).
• #15396 confirmed not epic-labeled (labels: enhancement, ai — it is a G3 leaf of epic #15239). Related: correctly carries the non-closing #15395/#15239/#15243.

Findings: Pass.

📑 Contract Completeness Audit

Findings: N/A for the formal ledger — the new surfaces are internal host↔helper seams (resolvePane/resolveTarget for embodiment; parkVessel/reshowVessel/disposeVessel for park), not a public config / MCP tool / core API / CLI surface. They are precisely documented in the factory JSDoc (strict-Boolean admission contract, "only strict true publishes ownership"). The cross-platform portability contract is explicitly owned by #15243 per the PR body.

🪜 Evidence Audit

• PR body contains an Evidence: line: Evidence: L3 (headed macOS Chromium journeys …) → L3 required (#15396 live-vessel + gesture-lifecycle ACs).
• Achieved (L3) ≥ required (L3); the PR states no close-target residuals and defers Windows/Linux portability receipts to #15243 (correctly scoped, not a hidden residual).
• Deployment causality: the e2e journeys run against the exact head in CI (green); no external/merged-only receipt is misused as a merge gate.

Findings: Pass — I relied on exact-head green CI + the committed e2e specs for the headed-runtime claim (§7.5: CI owns routine execution); I did not re-run the headed journeys.

N/A Audits — 📡 🛂 🔌 🔗

N/A across listed dimensions: no OpenAPI/MCP surface (📡); provenance (🛂) is declared and internal (built on #15215/#15251 preserved-park, not ported external framework code — pass, not a gap); the manager/Window change is an internal registration upsert, not a JSON-RPC/wire-format schema change (🔌); no skill/convention/startup change — the ADR 0029 update documents this PR's own design substrate, not a new cross-skill convention (🔗).

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head required CI green at 3251612eaf; author non-CI receipts present as the headed e2e journeys + a per-file unit spec for every touched dashboard/main/manager surface (11 test files, ~2000 lines).
• Reviewer falsifier: N/A — no named behavioral concern that CI does not establish; my two depth-floor concerns are framed as author-side isolation suggestions, not reviewer-run falsifiers.
• Test location: pass — added unit specs sit under the canonical test/playwright/unit/** mirrors; e2e under test/playwright/e2e/agentos/**.

Findings: Pass.

📋 Required Actions

No required actions — eligible for human merge. (The two Depth-Floor items are non-blocking clarifications / an optional isolation test; address inline if you agree, but they do not gate the merge.)

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 90 — correct placement (render/lifecycle seams in src/dashboard/, window-registration race in src/manager/Window), and a clean cohesion boundary (embodiment owns topology, park owns the platform mechanic behind seams, neither touches window authority). 10 deducted: the promote()→projection placeholder-cleanup ownership spans two components, a mild cohesion seam.
• [CONTENT_COMPLETENESS]: 88 — core.Base-level JSDoc on the new seams (Anchor & Echo present, strict-admission contract stated) and a comprehensive Fat-Ticket body. 12 deducted: the vessel-lifecycle seam contract is documented in JSDoc but not surfaced as a ticket/epic Contract Ledger row (internal-seam scope makes this a nit, not a blocker).
• [EXECUTION_QUALITY]: 85 — I verified DockVesselEmbodiment, DockVesselPark, and manager/Window in depth (careful generation-scoped race handling, strict-Boolean seam normalization, live-index restoration); green exact-head CI + a per-file unit spec for every touched surface own the mechanics I did not line-read (DockVesselConversion, DockTabSortZone, DockTearOut, main/addon/DragDrop). 15 deducted: the two non-blocking concerns are unverified by me, and four core mechanic files were assessed via coverage + CI rather than line-by-line (§7.5).
• [PRODUCTIVITY]: 92 — the #15396 goal (park-don't-close, live-pane embodiment, full gesture lifecycle across park/commit/disconnect/cancel/retire/terminal-first) is delivered with no close-target residuals.
• [IMPACT]: 88 — core multi-window dock-vessel lifecycle, a flagship Body-engine tear-out capability; the fail-toward-restore invariant protects a real user-facing one-way-door hazard.
• [COMPLEXITY]: 90 — 25 files across core dashboard/manager/main, intricate cross-window async race handling with generation-scoped effects and multiple terminal outcomes; high reader load.
• [EFFORT_PROFILE]: Heavy Lift — high complexity + high impact, core multi-window lifecycle with careful platform-law-bounded race handling.

Strong work, Euclid. The premise is right and the core seams are well-built; approving. The two depth-floor notes are optional hardening on the least-exercised failure paths — worth a glance but not a merge gate. Cross-family (Opus) review; eligible for the human merge gate.