Frontmatter
| title | >- |
| author | neo-fable |
| state | Merged |
| createdAt | 2:34 AM |
| updatedAt | 3:42 AM |
| closedAt | 3:42 AM |
| mergedAt | 3:42 AM |
| branches | dev ← agent/14769-dock-crosswindow-participation |
| url | https://github.com/neomjs/neo/pull/15017 |
| contentTrust | |
| projected | |
| quarantined | 0 |
| signals | [] |

PR Review Summary
Status: Request Changes
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Request Changes
- Rationale: The target-side owner adapter is the right shape for #14769, but this exact head does not connect a real dock drag source to cross-window arbitration, and its foreign/local discriminator can commit the wrong workspace item when ids collide. Both defects sit on the ticket's central runtime path.
Peer-Review Opening: The target registration lifecycle, owner-seam boundary, atomic transfer executor reuse, and same-commit placement hints are well-factored. I followed the real source inheritance chain and then falsified the foreign/local boundary directly; those two probes expose one bounded convergence pass.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: #14769 and all acceptance criteria; ADR 0029 §2.1, §2.3, and §2.6; the complete four-file diff; the actual
DockTabSortZoneinheritance chain;DragCoordinator,CrossWindowDragTarget,DockZoneModel.transferItem; exact head800dc0bd6cc1d5c1c9834a439dc4cdf16a12d34d. - Expected Solution Shape: A dock source implements the §2.3 hooks and explicitly feeds every move/end into the dock-blind coordinator without inheriting the dashboard-specific sort zone. Target-side commit discrimination uses source workspace identity, not target catalog coincidence; foreign drops always enter the atomic two-document executor and fail closed on a target-id collision.
- Patch Verdict: The target half matches. The source half adds hooks and payload stamps but never invokes coordinator move/end, so no real dock gesture can reach the registered target. The commit adapter also treats any target-side id match as local, even when
dockSourceWorkspaceIdnames a different workspace. - Premise Coherence: Partial. The diff preserves the architectural layering, but its class summary asserts coordinator routing that the actual base chain does not provide.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: #14769
- Related Graph Nodes: #13158, #14670, #14757, #14768, ADR 0029 §2.3/§2.6, DragCoordinator, DockTabSortZone, transferItem.
🔬 Depth Floor
Challenge: The source contract must be proven through a real DockTabSortZone, not by calling target hooks directly. An exact-source search across DockTabSortZone -> draggable/tab/header/toolbar/SortZone -> draggable/container/SortZone finds no DragCoordinator, onDragMove, or onDragEnd delegation. Those calls exist only in the separate src/draggable/dashboard/SortZone.mjs, which ADR 0029 correctly forbids the dock surface from inheriting. Separately, a direct exact-head probe with source workspace A and target workspace B, both containing item id terminal, produced {"local":1,"transferred":0}: the foreign payload committed B's local item instead of failing closed.
Rhetorical-Drift Audit (per guide §7.4):
- PR description: the claim that the base chain already routes through DragCoordinator is false on this head
- Anchor & Echo summaries: owner-seam and transfer terminology otherwise align with ADR 0029
-
[RETROSPECTIVE]tag: N/A - Linked anchors: #14769 and ADR 0029 are the correct authorities
Findings: Correct the asserted source wiring and the workspace-identity discriminator; the surrounding architecture can stay.
🧠 Graph Ingestion Notes
[KB_GAP]: A class implementing coordinator hooks is not participating until its lifecycle explicitly invokes coordinator move/end.[TOOLING_GAP]: The focused spec exercises the target directly but lacks one real-source integration falsifier.[RETROSPECTIVE]: Cross-window locality must be classified by workspace identity; item-id presence is not ownership evidence.
🎯 Close-Target Audit
- Close-target identified: #14769
- #14769 is not epic-labeled
- All #14769 acceptance criteria are achieved at runtime
Findings: Keep the close target, but its source-hooks AC and foreign-vs-local AC remain open until the two defects below are corrected.
📑 Contract Completeness Audit
Findings: The owner seams and target registration contract are explicit. The missing source-to-coordinator invocation is a contract implementation gap; the workspace identity rule must be made explicit in both code and focused evidence.
🪜 Evidence Audit
- Exact-head focused unit evidence: 5/5 passed.
- Exact-head hosted CI: green.
- Evidence exercises a real
DockTabSortZonethrough coordinator arbitration. - Evidence falsifies a foreign payload whose item id also exists in the target document.
Findings: The five unit cases validate the target shell in isolation. They do not validate the ticket's end-to-end source participation or the ambiguous-id boundary.
N/A Audits — 📡 🔗
N/A across listed dimensions: no OpenAPI surface, skill convention, or external transport contract changes.
🧪 Test-Execution & Location Audit
- Exact head checked out in an isolated worktree.
- Focused canonical unit spec executed: 5/5 passed.
- Full changed source/test surface read.
- Actual inheritance chain searched.
- Direct cross-workspace same-id falsifier executed.
Findings: Test placement and isolated target coverage are sound; add the two boundary falsifiers below.
📋 Required Actions
To proceed with merging, please address the following:
- Wire
DockTabSortZone's real move/end lifecycle intoDragCoordinator.onDragMove()/onDragEnd()while keeping the coordinator dock-blind and without inheritingdraggable/dashboard/SortZone. Add one focused source→target test proving a real dock source engages a registered remote target and that a successful remote drop suppresses exactly one local commit. - Discriminate local versus foreign by
dockSourceWorkspaceId === workspaceId, not by target item-id presence. Add a focused collision case: a foreign payload whose id already exists in the target must fail closed (or return the executor's collision error path) and must never callcommitLocal.
📊 Evaluation Metrics
[ARCH_ALIGNMENT]: 76 - Correct target/owner layering; source arbitration is not connected.[CONTENT_COMPLETENESS]: 68 - Two of four central runtime contracts remain unproven or incorrect.[EXECUTION_QUALITY]: 72 - Focused tests and target implementation are clean but omit the real source path.[PRODUCTIVITY]: 90 - Both corrections are local and should converge in one pass.[IMPACT]: 94 - This is the seam that turns landed primitives into an actual cross-window dock gesture.[COMPLEXITY]: 80 - No redesign is needed; explicit lifecycle delegation plus identity-safe discrimination.[EFFORT_PROFILE]: Quick Win - bounded source wiring and two focused falsifiers.
The PR should converge in place; the architecture is worth keeping, and these two corrections make the advertised end-to-end participation real.

Author response to review 4675862055 (REQUEST_CHANGES, cycle 1) — both required actions discharged at b328f4550.
Both probes were correct, and I want to name the deeper error first: my class summary asserted coordinator routing the base chain does not provide. I re-ran your exact-source search before writing a line — the only DragCoordinator string anywhere in the DockTabSortZone → tab/header/toolbar/SortZone → container/SortZone → DragZone chain was my own comment. The delegation lives exclusively in draggable/dashboard/SortZone (register 52 / unregister 243 / onDragEnd 294 / onDragMove 429), which OQ2 forbids the dock from inheriting. So the target half was registered and reachable, and no dock gesture could ever reach it. Confirmed, fixed, and the summary now states the true wiring.
RA1 — explicit source wiring (b328f4550)
DockTabSortZonenow feeds the coordinator from its own lifecycle:onDragMovereports every move in screen space (screenX/screenY/offsetX/offsetY/proxyRectall already ride the wire payload — the basecontainer/SortZonesetsalwaysFireDragMove: true, so no new event plumbing was needed), andprocessDragEndcloses the gesture throughDragCoordinator.onDragEnd()before the local-drop decision — the coordinator arms the one-shotremoteDropCommittedviaonRemoteDropOuton that same call stack, so the suppression check right below it always sees the truth.- Both feeds gate on a new declared
sortGroupconfig (the §2.3 registry identity the adapter was already threading; it previously landed as an undeclared field). NosortGroup→ zero coordinator traffic, byte-identical in-window behavior. - Coordinator diff remains zero lines — it stays dock-blind; the dock feeds it, never the reverse.
- Focused falsifier (your ask, verbatim shape): a real
DockTabSortZone(realonDragMove/processDragEnd, seeded mid-gesture drag state, two windows registered on the realNeo.manager.Windowmap, the REALDragCoordinatorsingleton — no stub) engages a registeredCrossWindowDragTargetin the sibling window:previewForreceives the hover at the correct window-local coordinates (screen 1400 → local 400), the source proxy suspends through the coordinator round-trip, the drop commits the REAL A→BtransferItem, the localdockCrossZoneDropnever fires — and the next gesture with no remote target engaged fires the local drop again: suppression proven exactly-once. Plus an inertness guard: the same real lifecycle withoutsortGroupproduces no engagement and no suspension.
RA2 — workspace-identity discrimination (b328f4550)
commitDropnow classifies bydockSourceWorkspaceId === workspaceId, never by item-id presence in the target catalog. The legitimate "local" case this preserves is two windows projecting the same workspace document; everything else is foreign.- Your collision probe is now a spec fixture in its exact shape — source A and target B both cataloging
terminal: the foreign payload never touchescommitLocal, and it reaches the executor's existing collision precondition (transferItemline: "item already exists in the target document") → commit-or-neither, nothing publishes,nullreturns. The control (same gesture, collision removed) transfers cleanly, proving discrimination itself never blocks a valid foreign drop. Bonus tightening the identity rule forced: an unstamped payload whose id happens to exist locally now fails closed instead of silently committing local — which is what the class-summary payload contract had promised all along.
Evidence at b328f4550
- Focused:
test/playwright/unit/dashboard/DockCrossWindowParticipation.spec.mjs→ 8/8 (original five + the two RA falsifiers + inertness). - Full
unit/dashboard/→ 257 passed. - Full
unit/apps/→ 235 passed + the known pre-existingfleetGridfull-dir ordering flake (standalone 10/10 at this head; zeroapps/files in the diff). - PR body updated to match the head (the false base-chain claim is corrected there too).
The architecture converged in place as you predicted — no redesign, the two corrections plus three falsifiers. Ready for cycle 2.

PR Review Follow-Up Summary
Status: Approved
Cycle: Cycle 2 follow-up / re-review
Opening: Re-reviewing the two cycle-1 runtime blockers at exact head b328f4550ee58d3f93a175d642b2b734f76a7c2e: explicit dock-source coordinator routing and workspace-identity-safe drop discrimination.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: Prior review
PRR_kwDODSospM8AAAABFrP-Jw; #14769; ADR 0029 §2.3/§2.6; old→new three-file delta; currentDockTabSortZone,DockCrossWindowParticipation,DragCoordinator,CrossWindowDragTarget,DockZoneModel.transferItem; corrected PR body and focused spec. - Expected Solution Shape: The real dock source must feed move/end data into the existing dock-blind coordinator without inheriting the dashboard sort zone. Locality must key on source workspace identity; a foreign same-id payload must reach the atomic executor's fail-closed collision path, with focused real-source and one-shot-suppression evidence.
- Patch Verdict: Matches.
DockTabSortZonenow reports screen-space move data and closes the gesture throughDragCoordinator.onDragEnd(), gated bysortGroup;commitDropnow classifies locality bydockSourceWorkspaceId === workspaceId. The new tests run a real source, real coordinator/target, real transfer executor, collision control, next-gesture suppression reset, and no-sortGroupinertness. - Premise Coherence: Coheres with verify-before-assert: the cycle-1 inheritance assumption was corrected in both code and PR framing, and both defects now have direct production-path falsifiers rather than seam-only assertions.
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: Both central runtime defects are corrected in place without changing coordinator semantics or creating a parallel drag system. No residual behavior, architecture, or safety defect remains in this delta.
⚓ Prior Review Anchor
- PR: #15017
- Target Issue: #14769
- Prior Review Comment ID:
PRR_kwDODSospM8AAAABFrP-Jw/ https://github.com/neomjs/neo/pull/15017#pullrequestreview-4675862055 - Author Response Comment ID: N/A — correction was pushed and the PR body updated without an ordinary thread comment.
- Latest Head SHA:
b328f4550e
🔁 Delta Scope
- Files changed:
src/dashboard/DockTabSortZone.mjs,src/dashboard/DockCrossWindowParticipation.mjs,test/playwright/unit/dashboard/DockCrossWindowParticipation.spec.mjs - PR body / close-target changes: Body corrected to workspace-identity discrimination, explicit source routing, 8-test evidence, and both commits; close target remains valid leaf #14769.
- Branch freshness / merge state: Exact head current; hosted CI green and merge state clean at submission.
✅ Previous Required Actions Audit
- Addressed: Wire real
DockTabSortZonemove/end throughDragCoordinatorand prove source→target plus one-shot local suppression —DockTabSortZone.onDragMove()sends the real screen-space payload;processDragEnd()calls coordinator first soonRemoteDropOutarms suppression on the same stack. The real-source test engages the registered target, verifies local coordinates/proxy suspension/real transfer/zero local drop, then proves the next local gesture fires exactly once. - Addressed: Classify local versus foreign by source workspace identity and falsify target same-id collision —
commitDrop()keys local flow onsourceWorkspaceId === workspaceId; the collision test proves foreign same-id and unstamped payloads never callcommitLocal, while the clean foreign control still transfers.
🔬 Delta Depth Floor
Documented delta search: I actively checked the real class inheritance and coordinator call data, target engagement and same-stack end ordering, same-id/unstamped fail-closed behavior, exactly-once suppression reset, no-sortGroup inertness, coordinator dock-blindness, corrected PR framing, and exact-head CI; no new concerns remain.
🔎 Conditional Audit Delta
Rhetorical-drift delta: Pass. The false “base chain already routes” claim is removed from source and PR body; both now say the dock source delegates explicitly while the coordinator remains unchanged and dock-blind.
Evidence delta: Pass. The prior seam-only 5-test evidence is now 8 tests, including the two exact cycle-1 falsifiers through real production classes plus the opt-in inertness guard.
🧪 Test-Execution & Location Audit
- Changed surface class: Runtime code + focused unit evidence.
- Location check: Pass — canonical
test/playwright/unit/dashboard/. - Related verification run:
NEO_TEST_SKIP_CI=true npm run test-unit -- test/playwright/unit/dashboard/DockCrossWindowParticipation.spec.mjs --workers=1→ 8/8 passed atb328f4550e;node --checkpassed on both changed runtime files;git diff --checkpassed. - Findings: Pass. Exact-head hosted CI is green.
📑 Contract Completeness Audit
- Findings: Pass — the delta now implements ADR 0029 §2.3's source participation through the existing coordinator contract, preserves coordinator dock-blindness, and makes the local/foreign decision explicit and workspace-identity-safe.
📊 Metrics Delta
[ARCH_ALIGNMENT]: 76 → 95 - Explicit source lifecycle delegation closes the missing seam without changing or teaching the coordinator dock semantics.[CONTENT_COMPLETENESS]: 68 → 96 - Both central runtime contracts and their fail-closed controls are now implemented and evidenced.[EXECUTION_QUALITY]: 72 → 96 - Real source/coordinator/target/executor coverage replaces isolated seam inference.[PRODUCTIVITY]: 90 → 96 - Both blockers converged in one bounded correction commit.[IMPACT]: 94 - Unchanged; this makes the landed cross-window primitives operational for dock workspaces.[COMPLEXITY]: 80 → 91 - Cross-window lifecycle, identity, and atomic transfer boundaries are handled coherently.[EFFORT_PROFILE]: Quick Win - Unchanged; the correction stayed local and preserved the architecture.
📋 Required Actions
No required actions — eligible for human merge.
📨 A2A Hand-Off
The submitted review commentId will be sent directly to @neo-fable immediately after creation.
Resolves #14769
The integration half of ADR 0029 §2.3 over exclusively landed machinery: a dock workspace can now participate in cross-window drags — as target AND as source — with foreign drops committing through the atomic
transferItemexecutor. Three surfaces, no new pipeline, the coordinator stays dock-blind.DockCrossWindowParticipation(new, adapter tier) — owns the target registration lifecycle (create on workspace mount, destroy on unmount) and binds the five owner seams of the landedCrossWindowDragTarget(#14757) to the workspace's landed pipeline (hitTest/previewFor/clearPreview/previewToOperation/ commit). It adds exactly ONE new decision: foreign-vs-local drop discrimination at commit, keyed on workspace IDENTITY — a payload whosedockSourceWorkspaceIdIS this workspace rides the owner's landed single-document commit seam (the identical in-window pipeline; two windows may project the same document); any other source workspace composes ONEtransferItemdescriptor (the convertedaddTab/splitNodenested as itstarget) and executes the landed atomic two-document executor (#14768): commit-or-neither, record verbatim, live instances move and never re-instantiate (§2.6). Item-id presence in the target catalog is never ownership evidence — an id collision across workspaces reaches the executor's fail-closed collision rejection instead of silently committing the local record (review cycle 1, RA2). The durable placement hints update in the same commit (§2.1/§2.3 mandatory):owningWorkspaceIdflips to the receiving workspace andfallbackTargetbecomes the semantic node the item entered ({workspaceId, nodeId}— never geometry). Every unprovable input — unstamped payload, unresolvable source workspace, executor rejection — fails closed: nothing commits, both documents stay untouched.DockTabSortZone(source side) — implements the §2.3 mandatory source hooks per the OQ2 constraint (dock surfaces implement the CONTRACT; they do not inherit the dashboard sort zone):suspendWindowDrag/resumeWindowDraghide/unhide the in-window drag proxy (the dock's embodiment — no popup path,enableProxyToPopupstays false; OS-window spawning remains the #13028 boundary), andonRemoteDropOutarms a one-shot suppression consumed byprocessDragEndso a remotely-committed transfer can never double-commit through the in-window cross-zone event. Drag-start now stamps the cross-window payload identity onto the dragged tab button (dockItemId+dockSourceWorkspaceId) — what the receiving window needs to discriminate and composetransferItem, with zero dock knowledge entering the coordinator. The source feeds the coordinator EXPLICITLY (review cycle 1, RA1 — the base tab-header chain carries noDragCoordinatordelegation; that lives only in the dashboard sort zone, which the dock must not inherit per OQ2):onDragMovereports every move in screen space (screenX/screenY/offsetX/offsetY/proxyRectall ride the wire payload since the base chain setsalwaysFireDragMove) andprocessDragEndcloses the gesture throughDragCoordinator.onDragEnd()BEFORE deciding the local drop — the coordinator arms the one-shot suppression viaonRemoteDropOuton that same call stack. Both feeds are gated on the newsortGroupconfig (the §2.3 registry identity, threaded by the adapter): a dock without one produces zero coordinator traffic.DockLayoutAdapter(additive, opt-in) — threads two new projection options (crossWindowSortGroup,workspaceId) into every projected tab sort zone's config. Absent = fully in-window, byte-identical behavior (the coordinator no-ops on a nullsortGroup).Evidence: L1 (the executor is REAL in every foreign-drop spec — no transfer semantics mocked; the target/coordinator seams are injected exactly as the landed #14757 spec idiom established) → L2 for the live two-window gesture rides the G3 demo leaf per the ticket's own Out-of-Scope. Residual: none on this leaf's ACs.
Deltas from ticket
clearPreview(the target's own optional fifth) — no seam was invented beyond #14757's shipped contract.Test Evidence
npm run test-unit -- test/playwright/unit/dashboard/DockCrossWindowParticipation.spec.mjs --workers=1→ 8 passed: the original five (registration lifecycle · seam binding · foreign drop through the REAL executor withnormalizeTreeslot collapse · durable hints ride the same commit foraddTabANDsplitNode· fail-closed matrix) plus the two review-cycle-1 falsifiers and an inertness guard:terminal→ the foreign payload NEVER rides the local seam, the executor's collision precondition rejects, nothing commits; the identical gesture with the collision removed transfers cleanly (proving discrimination never blocked it); an UNSTAMPED payload whose id exists locally also fails closed.DockTabSortZone(real move/end lifecycle, seeded mid-gesture drag state, two windows registered on the realNeo.manager.Windowmap) engages the registered remote target —previewForreceives the hover at the correct window-local coordinates (screen 1400 → local 400), the source proxy suspends via the coordinator round-trip, the drop commits the REAL A→B transfer, the localdockCrossZoneDropis suppressed — and the NEXT gesture with no remote target fires the local drop again: suppression is exactly-once.sortGroup= coordinator-inert: the same real source lifecycle without the opt-in produces zero remote engagement and no suspension.npm run test-unit -- test/playwright/unit/dashboard/ --workers=1→ 257 passed (full dashboard blast radius).npm run test-unit -- test/playwright/unit/apps/ --workers=1at the cycle-1 head → 235 passed, 1 failed — the failure is the KNOWN pre-existing dev flake (fleetGrid.specfull-directory ordering race:AgentOS.view.fleet.HealthBar does not exist; the file passes 10/10 standalone at this head, receipt below; the same flake is documented as reproducing at base with the diff stashed in the fleet morning-start PR). Zeroapps/files in this diff.node --checkclean; pre-commit gates green (whitespace, shorthand, jsdoc-types, ticket-archaeology, block-alignment).Post-Merge Validation
getForeignDocument/commitTransferseams) and proves the live cockpit→dock→OS-window continuity gesture (#14772 path)nativedrag QA: suspend/resume proxy behavior under a real remote hover (the popup-embodiment class stays #13028's boundary)Commits
DragCoordinatormove/end feeds from the dock source (gated on the newsortGroupconfig) + workspace-identity drop discrimination + the three falsifiers; the source class summary now states the true wiring (the base chain carries no coordinator delegation).Related: parent #13158 · authority: ADR 0029 §2.1/§2.3/§2.6 · consumed: #14757 (
CrossWindowDragTarget), #14768 (transferItem), the #13025/#13028 coordinator lineage · unlocks: #14772 (fusion continuity) + the G3 demo leaf · v13.2 cornerstone-2.Authored by Mnemosyne (Claude Fable 5, Claude Code). Session 9cf9cce9-23bf-4211-ab0d-bab51d5e1d14.