Skip to content

refactor(notebook): extract local RPC adapter - #665

Merged
ewen-poch merged 1 commit into
mainfrom
refactor/notebook-local-rpc-adapter
Aug 3, 2026
Merged

refactor(notebook): extract local RPC adapter#665
ewen-poch merged 1 commit into
mainfrom
refactor/notebook-local-rpc-adapter

Conversation

@ewen-poch

@ewen-poch ewen-poch commented Aug 3, 2026

Copy link
Copy Markdown
Member

Problem

NotebookLocalRpcServer owned both transport concerns and the concrete NotebookRuntimeService dispatch table. That coupled authentication, trusted context injection, and input-run lease lifetime to 16 Notebook capability methods, making the local RPC boundary harder to evolve safely.

Proposed change

  • Add a narrow NotebookLocalRpcCapability contract for exactly the 16 existing Notebook methods.
  • Move the method allowlist, common sessionId/workspaceCwd validation, method-to-capability translation, and four execution-method classification into local-rpc-notebook-adapter.ts.
  • Keep HTTP transport, tokens, aliases, trusted provenance/input injection, Specialist/Compute/Agents/Artifact/Skill routes, and input-run lease ownership in NotebookLocalRpcServer.
  • Import captured-completion identity from its defining module instead of the large runtime-service facade.
flowchart LR
  A["Local HTTP / MCP caller"] --> B["NotebookLocalRpcServer<br/>auth, alias, trusted context, lease lifetime"]
  B --> C["Notebook local RPC adapter<br/>16-method allowlist, validation, translation"]
  C --> D["NotebookLocalRpcCapability"]
  B --> E["Artifact / Skill / MCP / Compute / Agents routes"]
Loading

Scope and non-goals

  • No data-model, data-relationship, persistence, or user-interaction changes.
  • No public IPC, preload, Web RPC, CLI, Task, or MCP schema changes.
  • Preserve the current Electron/Web/CLI/Task capability asymmetry for Specialist, Permission, and Compute.
  • runtime:list-packages and runtime:list-package-counts remain Settings/runtime-selection APIs and do not enter local Notebook RPC.
  • Related to [Feature]: Provider-neutral multi-agent research orchestration #458 only as a forward-compatible ownership seam; this PR adds no orchestration state or public orchestration interface.

Production churn: 223 lines (local-rpc-server.ts 9 additions/74 deletions plus the 140-line adapter), below the 450-line re-review threshold.

Acceptance criteria and validation

All commands below ran after the final material edit.

  • 16-method mapping, validation, exact allowlist, request/result/error identity, and four lease-opening methods -> npm test -- --run src/main/notebook/local-rpc-notebook-adapter.test.ts -> 20 passed.
  • HTTP dispatch, trusted provenance/input injection, captured completion, and rejection-path lease cleanup/revocation -> npm test -- --run src/main/notebook/local-rpc-server.test.ts -> 22 passed.
  • Local RPC/MCP/Specialist/Compute compatibility -> targeted 9-file Notebook matrix -> 115 passed, 15 environment-gated skipped.
  • Latest-main package listing/runtime selection/Electron preload/Web RPC and updater contracts -> rebase-targeted 17-file matrix -> 266 passed, 15 environment-gated skipped.
  • Type safety -> npm run typecheck -> passed.
  • Repository lint -> npm run lint -> 0 errors; 18 pre-existing warnings in unchanged files.
  • Full regression on exact head 9f9f75b -> npm test -> 697 files / 10,188 tests passed; 15 files / 184 tests environment-gated skipped.
  • Independent Spec review -> 0 findings.
  • Independent Standards review -> 0 findings.

Uncovered risk: environment-gated host integration/certification cases remain skipped locally and are delegated to CI; no production behavior or platform-specific code changed.

Review focus

  • Confirm auth/alias/trusted-context/input-lease ordering remains in the server.
  • Confirm the adapter surface contains only the existing 16 Notebook methods.
  • Confirm package listing and Specialist/Permission/Compute cross-surface behavior remain unchanged.

@ewen-poch
ewen-poch force-pushed the refactor/notebook-local-rpc-adapter branch from 915b5f9 to 9f9f75b Compare August 3, 2026 08:06
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown

Codex Review

Verdict: mergeable

No actionable findings.

Summary: Static inspection found no concrete merge-blocking defect in the pull request changes. Branch and pull request title prechecks are valid.

@github-actions github-actions Bot added the ready-to-merge All completed AI reviewers found this pull request mergeable. label Aug 3, 2026
@ewen-poch
ewen-poch force-pushed the refactor/notebook-local-rpc-adapter branch from 9f9f75b to 3cffcfa Compare August 3, 2026 08:17
@github-actions github-actions Bot removed the ready-to-merge All completed AI reviewers found this pull request mergeable. label Aug 3, 2026
Give the local Notebook RPC surface an explicit capability contract while keeping transport authentication, trusted context injection, and input-run lease ownership in the server.
@ewen-poch
ewen-poch force-pushed the refactor/notebook-local-rpc-adapter branch from 3cffcfa to abab099 Compare August 3, 2026 08:20
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown

Codex Review

Verdict: mergeable

No actionable findings.

Summary: Static inspection found no concrete merge-blocking defects in the pull request changes. Branch and PR title prechecks are valid; no tests or project commands were run.

@github-actions github-actions Bot added the ready-to-merge All completed AI reviewers found this pull request mergeable. label Aug 3, 2026
@ewen-poch
ewen-poch merged commit a3c37ea into main Aug 3, 2026
23 checks passed
@ewen-poch
ewen-poch deleted the refactor/notebook-local-rpc-adapter branch August 3, 2026 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge All completed AI reviewers found this pull request mergeable.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant