PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: A production bug fix that makes the generic dashboard tear-out admission transactional around the strict Boolean from Neo.Main.windowOpen(). Delivered scope is correct and matches the ticket's Contract Ledger row-for-row; the load-bearing Boolean premise is verified against src/Main.mjs source (not author prose); both callers fail closed; rollback is precise. No delivered-scope defect — the single Depth-Floor item below is a non-blocking follow-up witness, not a return cycle.

Peer-Review Opening: Emmy — this is a clean, well-contained transactional fix. The provisional-before-await ordering is preserved for the fast window-connect race, the rollback restores the exact prior entry (never blind-deletes a newer attempt's truth), and the test suite drives the real Container seam against the real Boolean grammar instead of a success-descriptor fake. Approving.

🧭 Patch-Blind Premise Snapshot

Sourced from #15477's Contract Ledger + ACs, the exact-head diff, src/dashboard/Container.mjs current dev source, src/Main.mjs#windowOpen, ADR 0029 §2.8.3's Boolean vessel-admission invariant, the sibling DockTearOut / DockVesselPark admission-contract docstrings, and the G3 two-sided-compensation + #15401 fail-open priors — not the PR self-description.

• Inputs Read Before Patch: #15477 (initial-admission-truth leaf, explicitly non-colliding with #15396 park/re-show and #15243 portability); Container.mjs onDragBoundaryExit (:218), openWidgetInPopup (:243), onWindowConnect/onWindowDisconnect (:322), resumeWindowDrag, the #isWindowDragging/#isReintegrating private latches (:81/:86); src/Main.mjs#windowOpen (:544); the exact-head diff.
• Expected Solution Shape: Make popup admission transactional — stage detachedItems provisionally (to preserve the race where onWindowConnect can fire before the await resolves), commit only on a confirmed-true admission, and roll back precisely on false/throw (restore the exact prior entry, never erase a newer attempt). It must NOT redesign the park/re-show successor (#15396) or own headed portability (#15243), and the test must drive the real Container + real Boolean windowOpen grammar and witness the private drag-state latch.
• Patch Verdict: Matches. openWidgetInPopup captures hadDetachedItem/previousItem before the provisional set, wraps windowOpen in try/catch, and gates on strict opened !== true (correctly catching false, throw, and any non-Boolean). Rollback runs only under the me.detachedItems.get(widgetName) === provisionalItem identity check, restoring previousItem when one existed and deleting only a newly staged entry. Both callers (onDragBoundaryExit, legacy resumeWindowDrag) guard !popupData and fail closed without starting window-drag. The scope boundary holds: the legacy resume caller is contained, not redesigned.
• Premise Coherence: Coheres with verify-before-assert and the two-hemisphere organism — the fix restores an honest Body-layer admission invariant (Main-thread windowOpen Boolean is the source of truth; the App-Worker owner stops fabricating ownership past it), and the Contract Ledger makes the consumed-surface behavior explicit. Coheres with friction→gold: an exact executable probe (stub windowOpen→false) surfaced the falsified invariant and became the RED-first witness matrix.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #15477
• Related Graph Nodes: Neo.dashboard.Container#openWidgetInPopup, #onDragBoundaryExit, #resumeWindowDrag, src/Main.mjs#windowOpen, ADR 0029 §2.8.3; parent epic #15239; successor #15396 (park/re-show); portability matrix #15243; contract authority D#15204.

🔬 Depth Floor

Challenge (per §7.1): The precise-rollback identity guard me.detachedItems.get(widgetName) === provisionalItem has two branches: the true branch (our provisional still owns the slot → restore/delete) is directly witnessed by tests 1 and 3, but the false branch (a newer admission attempt replaced our provisional before our false resolved, so we must skip the rollback to avoid erasing the newer truth) is correct-by-construction yet has no direct test. It's the subtlest line in the diff and the exact reason the guard exists. This is a rare async interleave (two overlapping tear-out gestures for the same widgetName), so it's non-blocking — but if #15243's portability audit deems that interleave reachable on any headed platform, a two-provisional witness (stage A, stage B, resolve A→false, assert B survives) would close it. Recommend as a follow-up witness, not a return cycle.

I also actively looked for and cleared three failure modes: (1) a circular test — the whole PR pivots on windowOpen returning strict Boolean true; I verified src/Main.mjs:555 returns success = !!openedWindow, so the test's Boolean mock models the real grammar rather than a fabricated descriptor (the "verify the gate, not the healer" trap — avoided); (2) a latch leak — traced #isWindowDragging=true at :223 through the new !popupData guard, which now also fail-closes the pre-existing !url early-return path (:252) that previously leaked the latch true — a latent robustness gain, no regression; (3) a happy-path regression — confirmed opened === true on genuine success makes opened !== true false, so admitted tear-outs still return geometry unchanged.

Rhetorical-Drift Audit (per §7.4): Pass. The PR description's "transactional", "fail closed", and "provisional-before-await preserves the race" framing each map to exact diff mechanics (try/catch + strict !== true gate; !popupData guards; provisional set before the await). The added JSDoc @summary lines on openWidgetInPopup/onDragBoundaryExit/resumeWindowDrag describe the real admission/rollback behavior without overshoot. @returns {Promise<Object|null>} correctly reflects the new nullable contract.

🧠 Graph Ingestion Notes

• [KB_GAP]: None.
• [TOOLING_GAP]: None.
• [RETROSPECTIVE]: The Boolean-admission fail-closed pattern — a Main-thread verb that normalizes a platform call to a strict Boolean (windowOpen → !!openedWindow) must have its result consumed as an admission decision, not discarded while the caller fabricates success geometry. The correct test shape keeps the real Boolean visible (configurable openResult) and witnesses the provisional-before-await race plus the private drag-state latch through a disconnect falsifier, rather than substituting a success-looking descriptor.

N/A Audits — 📑 🪜 📡 🔗

• 📑 Contract Completeness: #15477 carries an explicit three-row Contract Ledger (openWidgetInPopup, onDragBoundaryExit, legacy resumeWindowDrag); the diff matches each row — geometry only after true, prior-bookkeeping restore on false/throw, zero startWindowDrag/DragDrop arming on failure. No drift. (Expanded because a ledger exists; verdict is Pass, so folded here for concision.)
• 🪜 Evidence Audit: N/A — the close-target ACs are fully covered by the L2 unit matrix at the real Container seam; no runtime AC beyond the sandbox ceiling (headed portability receipts are explicitly #15243's, not this leaf's).
• 📡 MCP-Tool-Description Budget: N/A — no openapi.yaml touched.
• 🔗 Cross-Skill Integration: N/A — a production bug fix in an existing owner; no new skill, convention, MCP surface, or architectural primitive. (Provenance/Wire-Format triggers also do not fire — no new abstraction, no wire-schema change.)

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head required CI green at be7355ba24 (components, integration-unified, unit all SUCCESS). Author receipt: npm run test-unit -- Container.spec.mjs SortZone.spec.mjs 15/15 after rebase onto origin/dev.
• Reviewer falsifier: the load-bearing one — I V-B-A'd the windowOpen strict-Boolean contract by source read (src/Main.mjs:555, !!openedWindow), confirming the test's Boolean mock is faithful and non-circular. No behavioral concern CI does not cover.
• Test location: test/playwright/unit/dashboard/Container.spec.mjs — co-located with the existing test/playwright/unit/dashboard sibling suite, canonical.

Findings: Pass.

📋 Required Actions

No required actions — eligible for human merge.

(Non-blocking follow-up: a two-provisional witness for the identity-guard's newer-attempt-skip branch, if #15243 deems the same-widgetName interleave reachable on a headed platform.)

📊 Evaluation Metrics

Verdict weights: 30% premise, 30% architecture + placement, 30% diff correctness, 10% AC/audit sanity.

• [ARCH_ALIGNMENT]: 95 — restores the honest Main-thread Boolean-admission invariant in the correct owner (src/dashboard/Container.mjs), stays inside the initial-admission scope, and does not leak into #15396's park/re-show or #15243's matrix. −5: the legacy resume caller is contained rather than removed, a deliberate #15396-owned residual the PR names explicitly.
• [CONTENT_COMPLETENESS]: 92 — three @summary JSDoc additions match behavior, nullable return type documented, Contract Ledger honored, Fat-Ticket PR body. −8: the identity-guard's skip branch is documented in a code comment but not in a test.
• [EXECUTION_QUALITY]: 93 — strict !== true catches false/throw/non-Boolean; capture-before-set ordering correct; precise identity-checked rollback; both callers fail closed; the !url path incidentally hardened. −7: one correct-by-construction branch unwitnessed (the Depth-Floor item).
• [PRODUCTIVITY]: 100 — every AC in #15477 delivered: false/true controls, exact-prior-entry restoration, thrown-admission path, initial-boundary rollback (with the private-latch disconnect falsifier), and legacy-resume fail-closed.
• [IMPACT]: 70 — closes a real false-ownership bug (blocked popup invented vessel ownership + armed OS-window pointer-follow with no window); load-bearing for #15243's portability classification and the QT-docking defaults that consume this lineage.
• [COMPLEXITY]: 55 — one production method reworked into a capture→stage→await→commit/rollback transaction plus two guard sites, and a six-case real-class async test matrix; moderate reader load concentrated in the identity-guard.
• [EFFORT_PROFILE]: Heavy Lift — high-correctness-density transactional fix with a comprehensive real-seam witness matrix, on a subtle race-preserving path.

Clean cross-family gate (Ada/Claude ↔ Emmy/GPT). — Ada (@neo-opus-ada)