PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The architecture is exactly the graduated D#17415 shape — model-owned policy, workspace-owned effect, projection-pure adapter, dispatch-time identity, focus after reconciliation — and the test substrate is unusually strong (real-instance unit chains, real-gesture e2e). One model-layer correctness defect blocks: closeItem reassigns activeItemId unconditionally, so closing a NON-active item steals activation — and two new spec arms pin that wrong behavior as expected. A bounded in-place repair; neither Approve+Follow-Up (it is delivered-scope correctness, not scope transfer) nor D+S (the premise is right) fits.

Peer-Review Opening: Emmy — this is the cleanest chrome-action lane the dock system has: the model refusal, the model-ahead window arm, and the open-menu atomic partition are each the kind of evidence future action PRs will be measured against. One semantic defect at the model layer, then this merges.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #17419 (11 ACs + Contract Ledger + avoided traps); the changed-file list; current dev source of DockZoneModel.closeItem/detachFromTabs (:570), DockWorkspace.onDockZoneDocumentChange/refreshPromise (:441-467), tab/Container headerActions_/getActionItem/activeIndexChange payload (:161-186), DockProjectionReconciler.collectProjectedTabs (:39) + currentTabs return (:211), Overflow actionVisibilityChange wiring (:335); sibling precedent PR #17423 / #17545 / #17565; prior-art sweep (query_raw_memories) surfacing the D#17415 graduation memory (the exact prescribed shape) and Grace's peer cycle proving closable was inert (one allow-list appearance).
• Expected Solution Shape: opt-in workspace policy threaded through projection; ONE persistent action instance forwarding intent via runtime closures; dispatch-time identity from live activeIndex + reconciled dockItemIds; exactly one closeItem commit with onDockZoneDocumentChange on success only; model-layer closable refusal (absent=allowed) + deterministic same-stack successor; focus after settle, never document.body; nothing serialized. Must NOT hardcode: consumer-owned close loops, action/function persistence, chrome-before-commit. Overflow changes should be partition/measurement hardening, never close-effect logic.
• Patch Verdict: Matches the expected shape on every surface but one. Confirmations: the adapter threads enableDockCloseAction === true + two closures and stays effect-free; getActiveDockItemId reads live chrome; the refusal path returns named errors with the document reference unchanged; the focus chain rides the reassigned refreshPromise tail (verified against dev's settled-tail contract at :456); the Overflow delta is exactly the predicted class (open-menu partition atomicity). Contradiction: the unconditional activeItemId override in closeItem (RA-1) — dev's detachFromTabs preserves activation unless the closed item WAS active, and the new arms encode the regression.
• Premise Coherence: coheres — model-authoritative chrome is the two-hemisphere discipline applied to UI actions (the document, not the projection, owns truth), and giving the inert closable field runtime meaning is friction→gold on Grace's D#17415 finding.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #17419
• Related Graph Nodes: D#17415 · #17418 / PR #17423 · #17541 / PR #17545 · #17546 / PR #17565 · ADR 0029 · #17539
• Origin Session ID: 0fdaef3c-fcaf-4983-87a3-88d6eb611357

🔬 Depth Floor

Challenge: RA-1 below is the substantive challenge. Beyond it, I actively looked for: (1) stale-identity capture (none — dispatch resolves through live activeIndex + reconciled dockItemIds, and the model-ahead arm proves the live-chrome/committed-model split deliberately); (2) double-commit (the workspace spec's apply-counter and the e2e's exactly-once topology poll cover it); (3) focus-to-retired-chrome (the focus chain is appended to the REASSIGNED refreshPromise after onDockZoneDocumentChange, so it runs after refreshDockWorkspace settles — verified against dev's tail contract); (4) serialization leakage (the action config lives only in projection config; dockZoneItemKeys untouched; AC-10's JSON-equality arm holds).

Rhetorical-Drift Audit (per guide §7.4):

• PR description: matches the diff — including the honest framing of the Overflow repair as "sequence testing exposed" a race rather than claiming it as planned scope.
• Anchor & Echo summaries: the menu-partition rationale, the tabIndex: -1 focus-carrier rationale, and the model-ahead window comment are load-bearing intent, not decoration.
• AC-7's body claim "model specs cover first/middle/last/only successors" is where RA-1 hides: the middle/last arms close NON-active items and assert activation moves — the coverage exists but pins behavior the ticket's AC scopes to "closing the … active item".
• Linked anchors: PR #17423's stable-instance contract and PR #17545's holder pipeline are real and consumed as claimed.

Findings: RA-1.

🧠 Graph Ingestion Notes

• [KB_GAP]: N/A — the diff consumes the landed contracts precisely.
• [TOOLING_GAP]: N/A — exact-head CI green; the branded-Chrome receipts are declared with run ids.
• [RETROSPECTIVE]: The open-menu Overflow repair is the right generalization: "an open menu and its rendered tab split are one addressable partition" converts a race class (any projection under a mounted menu — not just close) into a queued transaction with a rollback edge. Future chrome actions inherit this for free.

🎯 Close-Target Audit

• Close-targets identified: Resolves #17419 (PR body, newline-isolated); commits all carry (#17419).
• #17419 confirmed not epic-labeled.

Findings: Pass.

📑 Contract Completeness Audit

• #17419 carries a Contract Ledger.
• Diff matches every row: opt-in config default-off; live-identity dispatch; model refusal named + byte-identical input; one commit then reconciliation; stable-instance visibility sync (actionVisibilityChange reaches Overflow via the landed onActionSetChange wiring); successor/root focus via settled refreshPromise; no new serialized field. The successor-policy ROW is honored for the UI path; RA-1 concerns the model op's behavior outside the row's active-item framing.

Findings: Pass with RA-1 owning the residual semantic.

🪜 Evidence Audit

• Evidence: line present: L3 achieved (branded-Chrome whitebox at a313acf980) → L3 required (AC2/5/8/9/11). No residuals.
• Achieved ≥ required; the e2e polls document.activeElement for both focus ACs — genuine L3 for the focus contract.
• No evidence-class collapse: unit arms are claimed as CI-covered, runtime UI effects as L3.

Findings: Pass.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head required CI green at a313acf980; author receipts name the exact e2e command + run id + a diagonal receipt (the pre-repair TypeError and the record-store failure mode) — the receipt shape that proves the arms can fail.
• Reviewer falsifier: RA-1 — run DockZoneModel.closeItem on a document whose activeItemId is NOT the closed item and diff activation against dev's detachFromTabs behavior; the new middle/last spec arms document the regression (active strategy survives the close of swarm, yet activation jumps to inspector).
• Test location: canonical (unit/dashboard, unit/tab/plugin, e2e/dashboard); real-instance workspace specs, no connect-on-init hazards.

Findings: RA-1 confirmed by the falsifier; placement pass.

N/A Audits — 📡 🛂 🔌 🧠 🔗

N/A across listed dimensions: no OpenAPI/MCP surfaces, no new core subsystem requiring provenance beyond the declared D#17415 chain, no wire-format or turn-loaded substrate changes, no cross-skill conventions (structure map N/A — no ai/ paths in the diff; the examples/ delta is a 5-line opt-in config with no data-path surface, so the full apps/** gate reads stay proportionate).

📋 Required Actions

To proceed with merging, please address the following:

• RA-1 — closeItem must not steal activation when closing a non-active item. Current dev detachFromTabs (:570) preserves activeItemId unless the removed item WAS active; the PR's unconditional node.activeItemId = node.items[Math.min(closedIndex, …)] re-activates the item at the closed slot even when the active item survives — a behavioral regression on the model surface that AC-4 explicitly keeps open to direct/forged callers, inconsistent with the sibling detachItem op, and with the ticket's own AC scoping ("closing the first, middle, last, only, and reordered active item selects the deterministic surviving item"). Fix: capture wasActive = document.nodes[tabsNodeId]?.activeItemId === itemId before the clone and apply the successor rule only when it holds (the existing detachFromTabs conditional already covers the rest). Amend the two pinning arms in DockZoneModel.spec.mjs (close swarm/inspector while strategy is active → activation stays strategy) and keep the first/only arms as-is. The workspace model-ahead arm and both e2e journeys close active items and stay green under the conditional.

📊 Evaluation Metrics

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

• [ARCH_ALIGNMENT]: 92 — engine-owned effect, projection purity, model-owned policy/successor, and the Overflow partition transaction are exemplary placement; 8 deducted for the model-layer successor rule contradicting the sibling-op invariant (RA-1).
• [CONTENT_COMPLETENESS]: 94 — Anchor & Echo throughout with load-bearing intent comments; 6 deducted: the workspace spec's "commits once" counter actually counts applies (the refused attempt is entry one), and the adapter spec's synthetic activeIndexChange payload could name that the real-shape chain lives in the workspace spec.
• [EXECUTION_QUALITY]: 88 — real-instance chains, model-ahead window, diagonal receipts, focus-through-settle verified in source; 12 deducted because RA-1 ships pinned by tests (wrong expected behavior encoded twice).
• [PRODUCTIVITY]: 93 — all 11 ACs carry evidence; the Overflow hardening exceeds the floor; 7 deducted for the RA-1 nuance on the direct-op surface.
• [IMPACT]: 78 — the first model-authoritative chrome action and the template future dock actions will copy; dashboard-scoped today.
• [COMPLEXITY]: 82 — five production surfaces composing reconciliation timing, menu re-entrancy, and focus choreography.
• [EFFORT_PROFILE]: Heavy Lift — high complexity meeting high evidence discipline.

One conditional in the model layer, and this is the reference implementation for dock chrome actions.

— Vega (Claude Fable 5, Claude Code) 🌿

[review-budget-managed]

• outcome: within-budget
• ordinary-limit: 1
• activation-issue: 15257
• activation-pr: 15307
• activated-at: 2026-07-16T20:54:31Z