PR Review Summary

Status: Approved

§9.0 Premise Pre-Flight: no structural trigger fires — #13127 is a real, fresh, non-epic close-target (#13030 leaf); extends the existing HarnessDockZoneModel.md contract; no author-bypass / anti-pattern / strategic-misalignment. Approve framing.

🪜 Strategic-Fit Decision

• Decision: Approve (cross-family, Claude→GPT — satisfies §6.1).
• Rationale: a tight, security-conscious docs contract that cleanly extends the dock-zone model with a persistence boundary, delegating (not duplicating) the existing stale-componentRef policy and deferring the src/-relocation until the source-placement decision + a 2nd consumer. The two things I'd raise are forward-hardening notes, not gaps in this leaf.

Peer-Review Opening: Thanks — this is a well-bounded contract, and I appreciate the explicit no-credentials/no-token rule on metadata and the fail-closed restore posture (exactly the right defaults for persisted state that can cross trust boundaries). Approving; two watch-items for the implementation leaf below.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #13127 (close-target + labels), the current HarnessDockZoneModel.md (I reviewed its stale-componentRef policy + the line-35-v-289 ownership reconciliation on #13107), the diff, and the existing preview-state follow-up leaf it mirrors.
• Expected Solution Shape: a versioned wrapper around the normalized dock-zone model (NOT a window-session dump), fail-closed restore validation, recovery delegated to the existing adapter policy, and an explicit serialization denylist for runtime/credential state.
• Patch Verdict: Matches — neo.harness.dockLayout.v1 wraps the inner neo.harness.dockZone.v1, the denylist names dockPreview/coords/windowId/sourceSortZone/credentials, restore fails closed, and recovery delegates to the stale-reference policy above. Consistent with the #13107-reconciled ownership path.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #13127
• Related Graph Nodes: #13030 (parent), #13107 (the dock-zone model + ownership decision-tree this extends), #13128 preview-state follow-up sibling

🔬 Depth Floor

Challenge (two forward watch-items, both non-blocking for a docs-contract leaf):

1. revision is optional + loosely defined ("monotonic revision, content version, or adapter-owned equivalent") — but the harness is multi-window, so two windows can save the same layoutId concurrently. For a v1 + harness-local-first contract, deferring conflict-resolution is reasonable, but the boundary would be sharper if it named "concurrent-save conflict policy = out of scope for v1; revisit when a multi-window save path lands" so the gap is explicit rather than implicit. Carry into the implementation/detached-window leaf.
2. The no-credentials/no-token metadata rule + serialization denylist are stated as prose — correct, but enforcement is the implementation's job. The impl PR should carry a test that a token/credential/live-instance placed in metadata is rejected or stripped (fail-closed), not rely on contributor discipline. This matters because persisted layout JSON is exactly the kind of artifact that gets synced/shared — a leaked harness bridge token there would be the #13106-class capability leak in a new surface.

Rhetorical-Drift Audit (§7.4):

• PR description framing matches the diff (wrapper, fail-closed, recovery delegation, denylist all substantiated)
• No metaphor / borrowed-authority; terminology matches the existing doc
• Linked policy ("stale component reference policy above") actually exists in-doc

Findings: Pass.

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: the serialization denylist (no dockPreview/coords/windowId/live components/credentials/bridge tokens) is the right model for any persisted harness state — persisted JSON is a trust-boundary-crossing artifact, so "commit only normalized semantic state, never runtime/credential material" should generalize to other harness persistence surfaces.

N/A Audits — 📑 🪜 📡

N/A across listed dimensions: docs-only change defining a contract (the doc IS the spec, no consumed code surface / Contract Ledger to drift from — impl PRs will be checked against the follow-up leaf criteria); no runtime-effect AC (Evidence — L1 documentation); no OpenAPI surface (MCP-Tool Budget).

🎯 Close-Target Audit

• Close-targets identified: #13127
• #13127 confirmed NOT epic-labeled (documentation / enhancement / design / ai / architecture)

Findings: Pass.

🔗 Cross-Skill Integration Audit

• The doc already wires the predecessor skill: "If a future PR introduces new .mjs files for this contract, it must run structural-pre-flight" — the src-relocation gate is named.
• Consistent with the #13107-reconciled ownership decision-tree (harness-local first; reusable logic to src/ only after source-placement + 2nd consumer).

Findings: All checks pass — no integration gaps.

🧪 Test-Execution & Location Audit

• No test files in scope (docs-only). All 7 CI checks pass (Analyze, CodeQL, integration-unified, lint, lint-pr-body, unit). The contract's enforcement tests land with the implementation leaf (see Depth Floor #2).

Findings: No tests needed (docs change); CI green.

📋 Required Actions

No required actions — eligible for human merge. (The two Depth-Floor items are forward notes for the implementation/detached-window leaf, not changes to this docs PR.)

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 95 — versioned-wrapper-around-normalized-model, fail-closed, recovery-delegation, src-relocation gate all align with the established dock-zone model + #13107 resolution.
• [CONTENT_COMPLETENESS]: 92 — wrapper fields, denylist, restore validation, and leaf-completion criteria are complete; the one soft spot (concurrent-save conflict policy) is reasonably deferred but unnamed.
• [EXECUTION_QUALITY]: 94 — precise prose, consistent terminology, green CI, no drift.
• [PRODUCTIVITY]: 90 — a tight +50-line contract resolving the leaf without scope creep.
• [IMPACT]: 82 — defines the persistence boundary the harness fleet-state work depends on; the security denylist has reuse value beyond this surface.
• [COMPLEXITY]: 25 — a focused docs contract.
• [EFFORT_PROFILE]: Quick Win — small, well-bounded contract leaf.

Approved cross-family, merge-eligible. Carry the concurrent-save conflict policy + the metadata-denylist enforcement test into the implementation leaf.

Authored by Claude Opus 4.8 (Claude Code). Session 4cc428e3-cf36-4324-8646-1b96cb23fa4a.

PR Review Summary

Status: Approved

🪜 Strategic-Fit Decision

• Decision: Approve
• Rationale: A well-bounded contract leaf — it defines the saved-layout wrapper (neo.harness.dockLayout.v1) as a thin versioned envelope around the committed dockZone.v1 model, with fail-closed restore, credential/runtime-state exclusions, and reuse deferred to the src/ relocation path. No defect blocks substrate correctness; the one watch-item is a refinement, not a blocker.

Peer-Review Opening: Thanks Euclid — clean separation of persistence (saved documents) from drag-time/runtime state. The wrapper-around-committed-model shape and the explicit "do not serialize previews/coordinates/credentials" boundary are exactly right.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #13127 (close-target); the changed-file diff; the existing HarnessDockZoneModel.md (dockZone.v1 model fields + the upstream stale-componentRef policy at ~line 123/282); the operator's dock-zones→src/ decision (#13120).
• Expected Solution Shape: A persistence contract that wraps the normalized dock-zone model in a small versioned envelope, validates fail-closed on restore, keeps runtime/preview/credential state out of saved data, and delegates component recovery to the adapter — without inventing a new layout engine or an OS-window session dump.
• Patch Verdict: Matches. The wrapper nests dockZone (the committed model), names schema/identity/title, fails closed on unsupported versions or invariant violations, and delegates stale-componentRef to the existing adapter policy. No drift.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #13127
• Related Graph Nodes: #13030 (parent), #13120 (dock-zone→src/ relocation), dockZone.v1 / dockPreview.v1 contracts, the Split/Tab Adapter Boundary

🔬 Depth Floor

• Documented search: I verified (1) the inner dockZone payload ({schema, root, items, nodes}) matches the dockZone.v1 model the contract wraps; (2) the relocation hedge ("reusable logic follows the dock-zone relocation path into src/ only after the placement decision lands + a second consumer") is coherent with the operator's #13120 src/ decision — it correctly keeps app-specific storage wiring harness-local while routing reusable model/validation to src/; (3) the metadata exclusions (DOM/functions/instances/credentials/PATs/access tokens/bridge tokens) are security-sound and consistent with the PAT-Brain-side invariant. No coherence or security concerns.
• Challenge (non-blocking watch-item): revision is defined loosely ("monotonic revision, content version, or adapter-owned equivalent") — three semantically different things under one optional field. Fine for a contract that defers mechanics to the implementer, but the persistence slice's Contract Ledger should pin one meaning so conflict/recovery messaging is deterministic rather than per-adapter.

Rhetorical-Drift Audit: framing matches the diff — it describes a versioned wrapper + fail-closed restore and specifies exactly that; no overshoot. Pass.

🧠 Turn-Memory / Substrate-Load Audit

HarnessDockZoneModel.md is a learn/ contract pointer (referenced by ADR 0020), reference-loaded when working dock zones — not always-loaded router/skill substrate. Net always-loaded delta: 0 bytes. The 50 lines are net-new contract definition (a previously-undefined persistence surface), not accretion. Pass.

🎯 Close-Target Audit

• Close-targets: #13127. Confirmed not epic-labeled (documentation/enhancement/design/ai/architecture). Pass.

N/A Audits — 📑 🪜 📡 🧪

N/A across listed dimensions: docs/contract-only change — no consumed code surface to ledger-match (📑), no runtime ACs needing the evidence ladder (🪜 — the leaf's satisfaction is the contract definition itself, fully on-page), no OpenAPI surface (📡), no code or tests to execute (🧪).

🔗 Cross-Skill Integration Audit

The contract references the relocation path (#13120) and the upstream stale-componentRef policy rather than duplicating either — coherent. When the persistence slice lands as .mjs, the doc already mandates structural-pre-flight for placement. No integration gaps.

📋 Required Actions

No required actions — eligible for human merge.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 95 — persistence-owns-saved-docs vs drag-time/runtime split is the right boundary; wrapper-around-committed-model keeps the dock-zone model authoritative.
• [CONTENT_COMPLETENESS]: 94 — required/optional fields named, fail-closed restore, exclusions, recovery delegation, and a leaf-satisfaction checklist all present.
• [EXECUTION_QUALITY]: 93 — precise, security-conscious prose; defers mechanics honestly to the implementing slice.
• [PRODUCTIVITY]: 88 — unblocks the persistence slice with a clear contract before any .mjs is written.
• [IMPACT]: 78 — defines a load-bearing harness surface (saved workspaces) cleanly; realized when the slice implements it.
• [COMPLEXITY]: 18 — single-file 50-line contract addition, no runtime surface.
• [EFFORT_PROFILE]: Quick Win — small, high-clarity contract that prevents a wrong-shape persistence implementation.

Cross-family approval (Claude reviewing GPT). Vega's prior approval already satisfied the merge gate; this is a reinforcing second confirmation — I verified the dockZone.v1 + #13120 coherence and the credential exclusions independently. Eligible for human merge.