PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The premise is right and the chosen owner (DockZoneModel) is the right Body-side authority, but the regression is only one-way. That leaves the ticket's core stale-vocabulary failure mode reachable in the future, so this needs a same-PR fix rather than a follow-up.

Peer-Review Opening: This is the right seam to remove: the Neural Link service should read the dock operation vocabulary from the executor, not from a hand-maintained worker-side mirror. The remaining issue is the proof shape, not the ownership direction.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: Issue #14715, parent epic #13158, changed-file list, current dev versions of src/dashboard/DockZoneModel.mjs and src/ai/client/DockService.mjs, learn/agentos/HarnessDockZoneModel.md operations/drag-boundary sections, current unit specs, KB search for DockZoneModel/DockService, and Memory Core prior-art sweeps for #14715, #14625, and dock vocabulary SSOT (all clean misses).
• Expected Solution Shape: A correct fix should make DockZoneModel the sole executable vocabulary authority and make the Neural Link client consume that authority by reference. The boundary this must not hardcode is a second operation list in DockService; the test isolation must prove both directions of vocabulary/dispatch parity, not just today's values.
• Patch Verdict: Partially matches. DockService.operations = DockZoneModel.operations removes the worker mirror, and the frozen export is in the right owner. The regression at test/playwright/unit/dashboard/DockZoneModel.spec.mjs:999 only iterates exported names and checks that they dispatch; it does not fail if applyOperation() gains a new case that is missing from DockZoneModel.operations.
• Premise Coherence: Coheres with verify-before-assert and the two-hemisphere boundary: Body owns the semantic dock model, and the Neural Link surface reads it. The current proof undercuts friction→gold because it codifies only half of the drift seam the ticket was created to close.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #14715
• Related Graph Nodes: Related: #13158, #14587, #14652, #14714; concept tags: dockZone.v1, DockZoneModel.operations, DockService.operations, Neural Link dock tools.

🔬 Depth Floor

Challenge OR documented search (per guide §7.1):

• Challenge: The parity proof is asymmetric. At head, the exported array and applyOperation() cases currently match, but the spec only protects operations -> switch; it does not protect switch -> operations. If a future change adds case 'transferItem' to applyOperation() and forgets to add it to DockZoneModel.operations, the new spec still passes because it never enumerates switch cases. That is the same hidden executor capability / stale advertised vocabulary failure mode described in #14715.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: mostly matches the diff, but overstates the dispatch-parity guarantee.
• Anchor & Echo summaries: src/dashboard/DockZoneModel.mjs:71-76 says a new operation landing with an applyOperation case but missing the export makes the parity spec fail. The current spec does not make that fail.
• [RETROSPECTIVE] tag: N/A.
• Linked anchors: #14715 establishes bidirectional anti-drift as the ticket shape.

Findings: Rhetorical drift flagged with Required Action below.

🧠 Graph Ingestion Notes

• [KB_GAP]: N/A — the source-of-authority docs and issue body were sufficient.
• [TOOLING_GAP]: Local Playwright unit invocation for the two related files stayed silent and was interrupted; GitHub CI at c28ef17cbb1417896abc6ac7df41e65c6f1544ee is green and is the execution evidence cited here.
• [RETROSPECTIVE]: SSOT regressions need to prove both directions unless dispatch is mechanically derived from the same map. A one-way vocabulary test can look green while preserving the original drift class.

🎯 Close-Target Audit

For every issue named as close-target, verify it does NOT carry the epic label:

• Close-targets identified: #14715
• #14715 confirmed open leaf issue, not epic-labeled. #13158 is referenced only as related parent context.

Findings: Pass.

📑 Contract Completeness Audit

• Originating ticket defines the consumed surfaces and ACs for the local Body ↔ Neural Link vocabulary seam.
• Implemented PR diff matches the ticket contract exactly.

Findings: Contract drift flagged: #14715 requires that dispatch cannot diverge from the owned vocabulary. The current static export plus switch still permits switch-only additions without a failing regression.

🪜 Evidence Audit

Findings: N/A — close-target ACs are fully unit/static-contract coverable; no runtime-only surface is required.

📡 MCP-Tool-Description Budget Audit

Findings: N/A — this PR does not modify ai/mcp/server/*/openapi.yaml.

🔗 Cross-Skill Integration Audit

• No skill/startup workflow surface changed.
• Downstream runtime consumer checked: DockService.operations reads the executor export by reference.
• OpenAPI remains unchanged by ticket decision, but review language must not claim the one-way dispatch-parity spec guards prose vocabulary drift.

Findings: No cross-skill integration gap; the downstream contract gap is the bidirectional parity issue in Required Actions.

🧪 Test-Execution & Location Audit

• Branch checked out locally at exact head c28ef17cbb1417896abc6ac7df41e65c6f1544ee.
• Canonical Location: changed unit files remain under test/playwright/unit/dashboard/ and test/playwright/unit/ai/client/, matching the touched source surfaces.
• If a test file changed: Related GitHub CI unit passed at head; local focused Playwright run was interrupted after no output.
• If code changed: Static checks passed locally (git diff --check origin/dev...HEAD, node --check for both changed source files and both changed specs). Read-only source comparison confirmed current exported names equal current applyOperation() cases, but the regression does not enforce that in both directions.

Findings: CI green, static checks green, but test coverage has a blocking one-way parity gap.

📋 Required Actions

To proceed with merging, please address the following:

• Make the vocabulary/dispatch proof bidirectional, or collapse dispatch onto the same operation map/list. Acceptable shapes: derive applyOperation() dispatch from an operation-handler map whose keys are exported as DockZoneModel.operations, or add a regression that enumerates every applyOperation() case and fails when any case is absent from DockZoneModel.operations. Also tighten the JSDoc/PR framing so it no longer claims the current one-way parity spec catches switch-only additions or OpenAPI prose drift unless a real guard is added for that surface.

📊 Evaluation Metrics

Verdict weights: 30% premise / right thing, 30% architecture + placement, 30% diff correctness, 10% AC/audit sanity. These are importance-to-verdict weights, not effort budgets.

• [ARCH_ALIGNMENT]: 82 - Correct owner and correct Body/Neural Link boundary; deducted because the static list plus switch still creates two internal surfaces unless bidirectional parity or map-derived dispatch closes it.
• [CONTENT_COMPLETENESS]: 78 - PR body and JSDoc identify the seam clearly; deducted because the parity guarantee is overstated relative to what the tests enforce.
• [EXECUTION_QUALITY]: 68 - Current head is CI-green and static checks pass, but the new regression misses the switch-only-addition failure mode that the ticket is meant to prevent.
• [PRODUCTIVITY]: 70 - The worker-side mirror is retired, but #14715's “dispatch cannot diverge” AC is not fully satisfied yet.
• [IMPACT]: 55 - Moderate-high developer/agent reliability impact for Neural Link dock operations; not core Base/Neo risk, but it protects a public tool-facing vocabulary.
• [COMPLEXITY]: 28 - Small four-file refactor/test update with one subtle contract-proof dimension.
• [EFFORT_PROFILE]: Quick Win - High ROI and low code volume; the missing piece is a narrow proof correction.

Once the parity guard is bidirectional (or the switch is derived from a single handler map), this should be straightforward to clear.