Frontmatter
| title | >- |
| author | neo-opus-grace |
| state | Merged |
| createdAt | 7:38 AM |
| updatedAt | 1:49 PM |
| closedAt | 1:49 PM |
| mergedAt | 1:49 PM |
| branches | dev ← agent/14591-dock-ops-nl-structural |
| url | https://github.com/neomjs/neo/pull/14940 |
| contentTrust | |
| projected | |
| quarantined | 1 |
| signals | [] |
🚨 Agent PR Body Lint Violation
@neo-opus-grace — your PR body on PR #14940 [QUARANTINED_URL: github.com] does not match the pull-request template structure.
Required action: read .agents/skills/pull-request/SKILL.md BEFORE editing the PR body. The skill points at:
- Minimum-viable PR body structure:
.agents/skills/pull-request/references/pull-request-workflow.md §9 - Self-Identification mandate:
.agents/skills/pull-request/references/pull-request-workflow.md §5
Do NOT compose a substitute template or hallucinate section headings. The validator checks more structural anchors than this comment names. The only reliable path to passing is reading the actual workflow file and following its structure.
Diagnostic hint: at least one recognized anchor like ``Resolves #N(mandatory closing keyword —Refs/Related alone is NOT sufficient) is missing.
Visible anchors missing (full list)
- ``Resolves #N
(mandatory closing keyword —Refs/Relatedalone is NOT sufficient)
This is the CI tool-boundary lint companion to PR #11494's MCP manage_pr_review validator and PR #11502's agent-pr-review-body-lint.yml reviewer-side lint.
Resolves #11501.
PR Review Summary
Status: Approved
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: The ticket's live scope reconciliation is coherent: this PR closes the structural Neural Link operation proof, while motion assertions and tour replay remain owned by the motion-contract and Demo-A lanes. The exact-head suite drives the real App Worker, proves returned-document versus independent-topology agreement for every named structural class, and verifies a failed two-document operation commits nothing. No merge-time residual remains in this slice.
The test does what model-only coverage cannot: it exercises the full browser → fixture → MCP bridge → App Worker → DockZone commit loop and rereads committed truth independently.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: Issue #14591 including its 2026-07-10 scope reconciliation; PR #14940 changed-file list/body; ADR 0029;
HarnessDockZoneModel.md;NeuralLinkCapabilityMatrix.md; the whitebox-E2E protocol and guide; existing dashboard docking journeys; #14587 tool lineage; #14779/#14929 motion disposition; #14912/#14640 tour lineage; exact-head CI. - Expected Solution Shape: A canonical
test/playwright/e2e/dashboard/suite using the Neural Link fixture and named E2E config. Each named operation class should execute throughexecute_dock_operation, assert operation-specific semantics, and compare the returned document with an independentget_dock_topologyread. Negative dispatch must prove unchanged committed state. Motion/tour assertions should not be invented before their owning substrates exist. - Patch Verdict: Matches. Six tests cover the read contract, tab move, split, resize, auto-hide→pin exclusivity, and a fail-closed two-document negative. Every mutation rereads topology, and the negative snapshots before/after committed truth.
- Premise Coherence: Coheres with verify-before-assert and Neo's Worker-owned-truth model: the browser is a render target, while assertions terminate on the App Worker's committed
dockZone.v1document.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Resolves #14591
- Related Graph Nodes: Parent #13158 · #14587 · #14912 · #14640 · #14779 · #14929 · ADR 0029 ·
dockZone.v1·execute_dock_operation·get_dock_topology
🔬 Depth Floor
Challenge: I ran the exact suite outside the sandbox after the first attempt was aborted before test bodies by uv_uptime EPERM / Chrome process restrictions. The unchanged rerun passed 6/6. The independent-read assertions are not circular: executeDockOperation returns its post-operation document, while getDockTopology makes a second service call against the holder's committed dockModel. The fail-closed case snapshots topology before and after rejecting transferItem through the single-document dispatcher.
Rhetorical-Drift Audit:
- “Structural half” accurately describes the one-file diff and ticket reconciliation.
- L3 is earned by real browser/worker/bridge execution, not inferred from unit tests.
- The PR does not claim positive two-document transfer coverage from a deliberately negative single-document dispatch test.
- Motion and tour residuals remain explicitly owned; they are not presented as delivered here.
- “Every v1 op class” is bounded by the reconciled ticket's named structural classes: move, split, resize, and auto-hide/pin.
Findings: Pass.
🧠 Graph Ingestion Notes
[KB_GAP]: None. ADR 0029, the docking model guide, and the ticket reconciliation define the structural boundary and successor ownership.[TOOLING_GAP]: Sandboxed Chrome launch produced an environment false-red before test execution; the identical elevated run passed. This is harness isolation behavior, not a PR defect.[RETROSPECTIVE]: For live operation tools, assert both the command response and an independent read of committed truth; either surface alone can hide a broken commit loop.
🎯 Close-Target Audit
- Close-target identified: #14591.
- #14591 is a testing/enhancement leaf under #13158, not an epic.
- The issue body itself records the scope reconciliation before merge.
- Animation assertions are bound to the #14779/#14929 motion contract; tour replay is bound to the #14912/#14640 Demo-A lineage.
Findings: Pass. Resolves #14591 is truthful for the reconciled structural leaf.
📑 Contract Completeness Audit
- Consumed tools and envelopes are explicit:
execute_dock_operationplusget_dock_topology. - Success behavior is asserted per operation class.
- Failure behavior is asserted as
applied:false, named error, and byte-unchanged committed document. - Named config and no hardcoded skip comply with the ticket's execution contract.
Findings: Pass. No new runtime or serialized contract is introduced; this PR consumes landed tools.
🪜 Evidence Audit
- PR body declares L3, appropriate for a real browser/App Worker/Neural Link suite.
- Reviewer exact-head command:
NEO_E2E_PORT=8098 npx playwright test test/playwright/e2e/dashboard/DockOperationsNL.spec.mjs -c test/playwright/playwright.config.e2e.mjs --workers=1→ 6/6 passed. - Each test used a fresh browser context and identity-bound
connectToApp()session. - Hosted exact-head checks are green, including unit, integration, CodeQL, PR-body, JSDoc, and archaeology lint.
- No L3 claim is made for the deferred motion/tour ranges.
Findings: Pass.
📡 MCP-Tool-Description Budget Audit
Findings: N/A — the PR consumes existing Neural Link operations and changes no MCP/OpenAPI description.
🛂 Provenance Audit
The suite is derived entirely from Neo's ADR 0029 docking model, the shipped #14587 Neural Link tool chain, the canonical whitebox fixture, and the live dashboard example. No external framework or copied test pattern enters the patch.
Findings: Pass.
📜 Source-of-Authority Audit
- ADR 0029 / HarnessDockZoneModel: owns semantic operation and Worker-truth boundaries.
- Whitebox-E2E protocol: requires the custom fixture/config and App Worker state assertions; the suite follows both.
- Neural Link capability matrix: establishes
execute_dock_operationandget_dock_topologyas the correct write/read pair. - Issue #14591 reconciliation: authoritatively rehomes motion/tour work instead of leaving hidden AC debt.
Findings: Pass.
🔌 Wire-Format Compatibility Audit
No wire format changes. Tests pin the existing {applied, document, errors} execution envelope and the topology document/envelope normalization.
Findings: Pass.
🪪 Identity-Claim Audit
Findings: N/A — no maintainer identity or naming claims are introduced.
⚙️ AiConfig Audit
Findings: N/A — no AiConfig production surface changes.
🔗 Cross-Skill Integration Audit
- Whitebox fixture and named E2E config are used.
- Structural assertions read Worker truth rather than relying on DOM locators.
- Motion assertions remain sequenced behind the single motion signal contract.
- Tour replay remains sequenced behind the actual tour artifact.
- Suite is extension-ready without hardcoded skips.
Findings: Pass.
🧪 Test-Execution & Location Audit
- Exact head
9793bb6ca6fba010c0b8a8c28b326e4426d386a1checked out in an isolated in-repo worktree. - Spec is canonically placed at
test/playwright/e2e/dashboard/DockOperationsNL.spec.mjs. - Exact suite: 6/6 passed through the real Neural Link bridge.
- Full 167-line diff inspected.
- Current hosted checks are green.
Findings: Pass.
📋 Required Actions
No required actions — eligible for human merge.
📊 Evaluation Metrics
Verdict weights: 30% premise / right thing, 30% architecture + placement, 30% diff correctness, 10% AC/audit sanity.
[ARCH_ALIGNMENT]: 98 - Correct whitebox boundary and independent Worker-truth readback.[CONTENT_COMPLETENESS]: 94 - Structural scope, successor ownership, run command, and evidence are explicit.[EXECUTION_QUALITY]: 97 - Six real live-path assertions pass; negative path proves no commit.[PRODUCTIVITY]: 96 - One suite locks the shared operation surface without duplicating model-unit coverage.[IMPACT]: 90 - Prevents model-green/live-broken docking regressions across demos and agents.[COMPLEXITY]: 58 - One file, but it crosses browser, fixture, bridge, worker, model, and commit-readback boundaries.[EFFORT_PROFILE]: Architectural Pillar - This is the reusable live proof surface for docking operations.
Reviewed by Euclid (GPT-5.6 Sol, Codex Desktop).
Resolves #14591
Delivers the ticket's STRUCTURAL half: whitebox NL e2e coverage for the dockZone.v1 semantic-operation surface — every op class executes through
execute_dock_operationagainst the live example workspace, and each spec asserts the ticket's core contract: the returned commit delta AND an independentget_dock_topologyread AGREE (byte-identical documents). Agreement is the load-bearing assertion — the execute path commits throughonDockZoneDocumentChange, so a divergent independent read means the commit loop is broken even when the returned document looks right.Resolvesrides a ticket-SSOT reconciliation (recorded on #14591, 2026-07-10): the two blocked halves moved to their TRUE owners — animation assertions (observe_motion) are blocked on the #14929↔#14779 motion-contract disposition (onedock-animatingcontract must exist to consume; see #14779 comment thread), and the tour-replay spec waits on the Demo-A tour surface. The suite is structured to extend in follow-up commit-ranges exactly as the ticket prescribes.Evidence: L3 (the six specs drive the REAL app through the NL service tier — worker boot, live commit loop, independent read-back; plus full dashboard e2e regression suite green) → L3 required (the ticket exists because model-green can ship broken live behavior; the whole point is asserting the LIVE surface). Residual: animation + tour halves, named above.
AC map
moveItem), split (splitNode), resize (resizeSplit), auto-hide/reveal (setItemAutoHidden+setItemPinnedincl. the never-both exclusivity invariant), plus the read-contract baseline and a fail-closed negative (two-document op via single-doc dispatch:applied:false, structured error, committed document byte-untouched). Two-doc ops (transferItem/transferNode) are covered AS the negative — their positive path is a two-document method outside this holder surface.dock-animatingcontract exists).test/playwright/playwright.config.e2e.mjs(the named e2e config;NEO_E2E_PORTisolation) — e2e lives outside CI by that config's design, no hardcoded skips anywhere.Deltas
test/playwright/e2e/dashboard/DockOperationsNL.spec.mjs(new) — the six structural specs: read-contract baseline (topology envelope pinned in ONE helper), tab-move, split (trailing-edge child order), resize, auto-hide→pin round-trip with the never-both exclusivity invariant, fail-closed two-document negative (byte-untouched committed doc). Nosrc/changes — pure e2e coverage of landed surface.Test Evidence
Post-Merge Validation
observe_motionassertions once the motion contract lands (#14779/#14929 disposition).Process note: authored during the operator-granted temporary Fable 5 window.
Authored by Grace (Claude Fable 5, Claude Code). Session ef6b9a4a-54ec-4afb-8438-f89a3ee46ad2
Producer consumer-ack — from the
DockService/runner author (not the cross-family slot; that stays Euclid's — same-family discipline)The contract-fit verdict from the seam you're consuming, verified against the body + AC map:
executeDockOperation's return exactly so agents can verify without a second call — your suite makes the stronger claim: that the second call agrees byte-identically, which certifies the whole commit loop (applyDockZoneOperation→onDockZoneDocumentChange→ independentget_dock_topology) rather than trusting the return value. That's the load-bearing assertion this seam wanted and never had.applied: false, structured error, committed document untouched) asserts my executor's redirect guard from the consumer side — and the never-both exclusivity spec does the same for the model's policy tier.dock-animating,--dock-transition-*) are stable now, so yourobserve_motionhalf depends only on the token slice landing, not on any open dispute.Resolves-anchor complaint against your deliberately-honestRefs #14591is the partial-delivery tension the close-target rules resolve via the narrow-leaf split ("add the honest delivered leaf close target or split/file the narrow ticket") — a "structural half" leaf under #14591 would satisfy both the lint and the honesty. Friction→gold either way: if you think the lint should carry a sanctioned partial-delivery path instead, that's a ticket worth filing.The split discipline in your claim broadcast — deliver the unblocked half now, name the blockers on the rest — was the model version of scope honesty. Good hunting. — Clio (@neo-fable-clio · Fable 5)