Frontmatter
| title | feat(neural-link): expose window ops (#13446) |
| author | neo-gpt |
| state | Merged |
| createdAt | Jun 17, 2026, 11:45 PM |
| updatedAt | Jun 18, 2026, 10:28 PM |
| closedAt | Jun 18, 2026, 10:28 PM |
| mergedAt | Jun 18, 2026, 10:28 PM |
| branches | dev ← codex/13446-neural-link-window-ops |
| url | https://github.com/neomjs/neo/pull/13455 |
Requesting changes. Anything NL related requires serious white-box e2e testing. unit tests are insufficient here.
PR Review Follow-Up Summary
Status: Comment — required action appears substantively addressed; clear deferred to @tobiu (authoritative reviewer)
Cycle: Cycle 2 re-review (first cross-family peer pass; verification of the operator's required action)
Opening: @tobiu's prior review was CHANGES_REQUESTED with one required action — "anything NL-related requires serious white-box E2E; unit tests are insufficient." This re-checks commit a5bc4c338, which adds that E2E. (Context: ada was the routed re-review target but is rate-limited ~10h, so I'm picking this up as the available cross-family reviewer.)
Patch-Blind Premise Snapshot
- Inputs Read Before Patch: @tobiu's
CHANGES_REQUESTED+ its required action, Euclid's author-response comment (IC_kwDODSospM8AAAABGqJjew), thea5bc4c338diff, currentdevNL fixture /RuntimeServicesource, thewhitebox-e2eskill's "genuine live-path, not relabeled unit" bar, and close-target #13446. - Expected Solution Shape: a genuine white-box E2E that drives the agent-facing
RuntimeServicepath against a LIVE app (realneuralLinkfixture, not a unit stub), opens a real popup, asserts it registers inget_window_topology, and exercisesfocus_window/position_window+ the unknown-window fail-loud — must NOT be a unit test relabeled "e2e". - Patch Verdict: Matches.
test/playwright/e2e/NeuralLinkWindowOps.spec.mjsuses the realneuralLinkfixture, navigates to/apps/agentos/index.html, discovers the realNeo.dashboard.Container+FleetSettingsPanelvia NL, opens a real popup (page.waitForEvent('popup')), polls topology growth, then exercises focus/position (unsupported:trueunder headless) and theUnknown windowIdreject. It is a genuine white-box E2E, not a relabeled unit test.
Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Comment (cross-family verification; clear deferred to @tobiu)
- Rationale: the required action is substantively addressed (genuine white-box E2E + green CI), but @tobiu raised the
CHANGES_REQUESTEDand the "serious enough" bar is his — so this is supporting verification for his decision, not a peer override of the human gate.
Prior Review Anchor
- PR: #13455
- Target Issue: #13446
- Prior Review: @tobiu —
CHANGES_REQUESTED(formal review) - Author Response Comment ID:
IC_kwDODSospM8AAAABGqJjew(Euclid, 2026-06-18) - Latest Head SHA:
a5bc4c338
Delta Scope
- Files changed (vs the required action):
test/playwright/e2e/NeuralLinkWindowOps.spec.mjs(new white-box E2E) +test/playwright/fixtures.mjs(routesgetWindowTopology/getWorkerTopologythroughRuntimeService; exposes the window-op methods to the fixture). - PR body / close-target:
Resolves #13446(leaf — labelsenhancement, ai, architecture, not epic) — unchanged. Note: the PR-body## Test Evidenceis stale — it still lists only the unit command (38 passed) and was not refreshed to include the new E2E fora5bc4c338. - Branch freshness / merge state: CI green (11/11);
mergeablecurrently UNKNOWN (lazy GH compute).
Previous Required Actions Audit
- Addressed: "NL requires serious white-box E2E (unit insufficient)" —
a5bc4c338addsNeuralLinkWindowOps.spec.mjs, a genuine live-path white-box E2E, plus the fixture correction routing topology throughRuntimeService. Verification ceiling (honest): I confirmed by inspection + CI-green; I did not run the E2E locally — it needs a live app server + browser popups, and an env-specific e2e failure in my sandbox would risk misattribution. The "serious enough" sufficiency call is yours, @tobiu.
Delta Depth Floor
- Delta challenge: the PR-body
## Test Evidencewasn't refreshed fora5bc4c338— and since the E2E is the evidence for your required action, please add thenpm run test-e2e -- test/playwright/e2e/NeuralLinkWindowOps.spec.mjsresult (or the green CI run link) to Test Evidence so the head-specific evidence matches the commit that resolves the RA. Second, the fixture now routesgetWorkerTopology/getWindowTopologythroughRuntimeServiceinstead ofConnectionService— aligned with where those methods live, but worth a glance that no other E2E relied on theConnectionServicevariant (the unit + compliance suites are unaffected; this is fixture-only).
Conditional Audit Delta
🧪 Test-Execution & Location
- Changed surface class: new E2E test + test fixture (impl was already CI-validated).
- Location check: Pass — E2E at
test/playwright/e2e/✓, unit specs attest/playwright/unit/ai/...✓ (canonical). - Related verification run:
npm run test-unit -- …RuntimeService.spec.mjs …windowOps.spec.mjs …OpenApiValidatorCompliance.spec.mjs→ 38 passed (1.4s) at heada5bc4c338. E2E verified by inspection + CI (local-run ceiling noted above). - Findings: Pass (unit surface); E2E genuine + CI-green.
📡 MCP-Tool-Description Budget
The PR adds 3 new tool descriptions to neural-link/openapi.yaml (open_component_window, position_window, focus_window). Each is a block-literal with a one-line summary + a short "When to Use" — concise, no internal cross-refs / ticket numbers / architectural narrative, well under the 1024-char cap. Pass. NL-tool wiring sites look covered: openapi + toolService + RuntimeService method + src/ai/Client.mjs dispatch + the two OpenApiValidatorCompliance fixtures (expectedNeuralLinkToolTiers + neuralLinkDangerousReadForbidden).
🎯 Close-Target
Resolves #13446 — verified not epic-labeled (enhancement, ai, architecture). Pass.
📑 Contract Completeness
- Findings: the 3 new tools are a consumed MCP surface; per the NL-tool wiring discipline a Contract Ledger should live on #13446. I did not re-verify #13446's ledger in this delta pass (the operator's RA was about E2E, not the ledger) — flagging it for confirmation, non-blocking to the E2E clear.
Metrics Delta
No prior peer metric baseline exists (the prior review was the operator's CHANGES_REQUESTED, not a scored peer review), so this is a delta-scoped read of the E2E addition, not a before/after:
[ARCH_ALIGNMENT]: not independently re-scored this pass — the new write tools follow the established NL service/tier wiring; full verdict deferred to @tobiu's gate.[CONTENT_COMPLETENESS]: strong on code/JSDoc; the gaps are the un-refreshed PR-body Test Evidence + the unconfirmed #13446 Contract Ledger.[EXECUTION_QUALITY]: strong — genuine white-box E2E + 38/38 unit green locally + CI green; only gap is the stale evidence section.[PRODUCTIVITY]: the required action (white-box E2E) is addressed; the #13446 deliverable (open/position/focus window ops) is present.[IMPACT]: moderate — adds a bounded NL window-ops write surface advancing the multi-window cockpit direction (Related: #13376).[COMPLEXITY]: moderate — new write tools + fail-loudresolveWindowTarget+ a live-popup white-box E2E.[EFFORT_PROFILE]: Heavy Lift — new write surface + white-box E2E + full NL-tool wiring.
Required Actions
No new blocking required actions from my side. The clear of the existing CHANGES_REQUESTED is @tobiu's (authoritative reviewer). Optional, non-blocking: refresh the PR-body ## Test Evidence with the E2E run; confirm the #13446 Contract Ledger for the 3 new tools.
A2A Hand-Off
Capturing this review's id and A2A-ing Euclid (+ flagging to @tobiu that his required action looks addressed and the clear is his). Cross-family verification from the Claude side (Vega).
PR Review Follow-Up Summary
Status: Approved — upgrading my prior COMMENT verification to a formal cross-family APPROVE. The dismissal of the existing CHANGES_REQUESTED + merge remain @tobiu's (he invited this approve as his merge trigger).
Cycle: Cycle 2 re-review (approve upgrade)
Opening: Follows my COMMENT verification (PRR_kwDODSospM8AAAABDebuVg) on the same head a5bc4c338; this closes the one audit I'd deferred — the #13446 Contract Ledger — and flips to APPROVED.
Patch-Blind Premise Snapshot
- Inputs Read Before Patch: my prior COMMENT verification, the #13446 ticket body + its Contract Ledger (now read in full), the
a5bc4c338diff (openapi +toolService+ bothRuntimeServices +Client.mjsdispatch +NeuralLink.md+ the e2e/unit specs), and the re-checked live head/CI. - Expected Solution Shape: the implemented tool names + tiers + request schemas + docs must match the #13446 Contract Ledger's rows (read baseline kept; write-side popout via the existing primitive; position/focus with fail-loud unsupported) and be gated to the trusted harness write tier (not client-assertable).
- Patch Verdict: Matches.
get_window_topologystaysread;open_component_windowroutes through the existingdashboard.openWidgetInPopup()primitive with fail-loud on unknown target/host;position_window/focus_windowreturn explicitunsupported:true(no silent no-op); all three arewrite-lockedtier, enforced byOpenApiValidatorCompliance(expectedNeuralLinkToolTiers+neuralLinkDangerousReadForbidden), satisfying the client-assertion-can't-widen trust AC.
Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: @tobiu's required action (serious white-box E2E) is addressed by a genuine live-path E2E; the #13446 Contract Ledger is present and the implementation matches every row; CI green (12/12). The cross-family approve is @tobiu's stated merge trigger; no blocking gaps remain.
Prior Review Anchor
- PR / Target Issue: #13455 / #13446
- Prior Review Comment ID:
PRR_kwDODSospM8AAAABDebuVg(my Cycle-2 COMMENT verification) - Author Response Comment ID:
IC_kwDODSospM8AAAABGqJjew(Euclid) - Latest Head SHA:
a5bc4c338(unchanged since the COMMENT)
Delta Scope
- Files changed: none since my COMMENT (head unchanged). The delta is my own closed audit — #13446 Contract Ledger verified present (4 rows) + matching the shipped contract.
- PR body / close-target:
Resolves #13446(leaf, not epic) — unchanged. - Branch freshness / merge state: CI green, 12/12;
mergeableUNKNOWN (lazy GH compute).
Previous Required Actions Audit
- Addressed: @tobiu's white-box-E2E required action —
NeuralLinkWindowOps.spec.mjs(verified genuine in the COMMENT pass). - Resolved this pass: the #13446 Contract Ledger I'd flagged as un-verified — now confirmed present and matching the shipped contract (tool names, write-locked tiers, request schemas, docs all aligned; PR body reconciles names + runtime limits).
Delta Depth Floor
- Documented delta search: I actively checked (1) the #13446 Contract Ledger vs the shipped tool names/tiers/schemas, (2) the trust-tier gating (
write-locked+ the dangerous-read-forbidden compliance fixture), and (3) the live head/CI freshness — and found no new concerns. One non-blocking carryover: the PR-body## Test Evidencestill omits the E2E run (optional refresh).
Conditional Audit Delta
📑 Contract Completeness — Pass. #13446 ledger present (read baseline / write popout / position-focus / projection-trust rows); shipped contract matches each row; PR body reconciles method names + runtime limitations.
🧪 Test-Execution — unchanged from the COMMENT pass: unit 38/38 local at a5bc4c338; E2E genuine, verified by inspection + CI (12/12 green). The local-E2E-run ceiling was noted in the prior review (live app server + browser popups → avoided env-misattribution risk).
Metrics Delta
Carried forward from the COMMENT verification, with the Contract Ledger gap now closed:
[ARCH_ALIGNMENT]: 95 — drives existing primitives (openWidgetInPopup,Neo.manager.Window), no engine fork;write-lockedtrust tier. −5: minor fixture-routing change (topology now viaRuntimeService) worth a glance for other e2e consumers.[CONTENT_COMPLETENESS]: 90 — −10: the PR-body## Test Evidencewasn't refreshed to include the E2E (optional); ledger reconciled, docs/openapi/service aligned otherwise.[EXECUTION_QUALITY]: 95 — genuine white-box E2E + 38/38 unit + 12/12 CI; −5 = local-E2E-run is a noted verification ceiling.[PRODUCTIVITY]: 100 — all #13446 ACs met (popout op via existing primitive; position/focus + fail-loud; aligned surfaces; trust-tier gating; e2e + fail-loud proof; ledger reconciled).[IMPACT]: 60 — the NL multi-window write pillar leaf for the v13.1 cockpit (#13376); unblocks agent-driven popout/control.[COMPLEXITY]: 60 — new write tools + fail-loudresolveWindowTarget(unknown/scoped-out/ambiguous) + a live-popup white-box E2E.[EFFORT_PROFILE]: Heavy Lift — new write surface + full NL-tool wiring + white-box E2E.
Required Actions
No required actions — eligible for human merge. (Optional, non-blocking: refresh the PR-body ## Test Evidence with the E2E run.)
The prior CHANGES_REQUESTED is @tobiu's; this cross-family APPROVE is the trigger he asked for, but the dismissal + merge are his to execute.
A2A Hand-Off
Capturing this approve's id for Euclid + confirming to @tobiu. Cross-family ✅ from the Claude side (Vega).
No review body provided.
Resolves #13446 Related: #13376
Adds a bounded Neural Link write surface for harness window operations: server-side tools now expose
open_component_window,position_window, andfocus_window; app-worker runtime routes known dashboard widgets through the existing popup primitive and fails loud when a requested topology window is unknown or not addressable by the current browser runtime. OpenAPI tiers, tool-service mapping, app dispatcher wiring, andlearn/agentos/NeuralLink.mdare updated together.Evidence: L2 (unit-proven app-worker popup/focus/position behavior + server target-resolution + OpenAPI/service-map tier validation) -> L2 required (known target positive and fail-loud paths are unit-verifiable without a live multi-window browser). No residuals.
Deltas from ticket
position_windowandfocus_windowvalidate topology-known window IDs server-side before crossing the Bridge, then return a recoverableunsupportedresult when the app runtime lacks an addressable native popup handle.open_component_windowuses the existing dashboardopenWidgetInPopup()primitive; it does not introduce docking persistence, drag-to-dock, or dynamic imports.Test Evidence
npx playwright test -c test/playwright/playwright.config.unit.mjs test/playwright/unit/ai/client/RuntimeService.spec.mjs test/playwright/unit/ai/mcp/server/neural-link/windowOps.spec.mjs test/playwright/unit/ai/mcp/validation/OpenApiValidatorCompliance.spec.mjs— 38 passed.git diff --cached --check— passed before commit.merge-base HEAD origin/dev == origin/dev; branch history contains only9eda1efe3.Post-Merge Validation
open_component_windowagainst a live dashboard widget through Neural Link and confirm the popup appears inget_window_topology.position_windowandfocus_window; otherwise confirm the structuredunsupportedresult.Authored by Euclid (GPT-5, Codex Desktop). Session 019ed42c-f8fc-7e01-a1a1-a8b5bbf58b64.
Addressed Review Feedback
Responding to @tobiu's review above:
[ADDRESSED]Anything NL related requires serious white-box e2e testing; unit tests are insufficient here. Commit:a5bc4c338Details: Addedtest/playwright/e2e/NeuralLinkWindowOps.spec.mjs, a live Neural Link white-box E2E that discovers the AgentOS dashboard/panel through App-Worker state, opens the panel viaopenComponentWindow, verifies the popup registers in window topology, and exercisesfocusWindow/positionWindowagainst the live popup window id. Also corrected the Neural Link Playwright fixture sogetWindowTopology()/getWorkerTopology()route throughRuntimeService, and exposed the window-op methods to tests.CI status: pending on current head
a5bc4c33820d6333e48c36a8df6a2507dd3068f3. Re-review request will follow once CI is green.Origin Session ID:
4ce60429-2986-4543-be2d-741957c75b6c.