Skip to content

feat(core): composite rpc router over fetch + additive ws mount at /rpc-ws (#314, PR 1/3) - #319

Merged
omridevk merged 2 commits into
mainfrom
ws-transport-server-314
Aug 8, 2026
Merged

feat(core): composite rpc router over fetch + additive ws mount at /rpc-ws (#314, PR 1/3)#319
omridevk merged 2 commits into
mainfrom
ws-transport-server-314

Conversation

@omridevk

@omridevk omridevk commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Server-side PR 1 of the three-PR rollout for #314 (widget↔engine rpc over WebSocket, killing the browser's 6-connections-per-host starvation). Implements the FINAL binding spec on the issue. Zero client changes; fetch transport and all existing /rpc/** URLs are byte-identical.

What

  • Runtime composite router {...core, ext: {<slug>}} built after extensions mount; ONE fetch RPCHandler at /rpc replaces the old split (core middleware + per-extension middlewares) with an identical URL space.
  • Additive ws mount at /rpc-ws via @orpc/server/websocket .message()/.close() hooks wired through hono upgradeWebSocket — never handler.upgrade() (pre-open frame buffer race), no ws.raw, no casts.
  • Per-call context on BOTH mounts: shared rootInterceptors derivation (@conciv/extension/rpc-mount) reads per-call headers from the oRPC standard request, so CONCIV_SESSION_HEADER-gated procedures behave identically over fetch and ws.
  • @conciv/serve owns teardown: graceful close (1001) of live sockets, terminate() only after a deadline; explicit maxPayload; fetch typed to accept the server env arg.
  • Auth unchanged: the /t/<token> path prefix scopes the upgrade; no query-param token.

Tests (all real sockets, no mocks)

  • rpc round-trip over ws through the token-prefixed new Hono().mount('/t/<token>', app.fetch) (the previously unproven path); wrong token never upgrades
  • per-call session header reaches an approval-gated procedure over both transports; header-less ws call refused
  • extension procedure answers over ws at ext.<slug> and over the unchanged fetch URL
  • origin enforcement on the upgrade (existing corsMiddleware, no second path)
  • first rpc frame sent at CONNECTING is answered, never dropped
  • teardown: graceful 1001 vs deadline terminate (discrimination-proven), oversized frame → 1009, 20 rejected upgrades leave no waiter behind

Rollout

  • PR 2: test-infra migration (ws-piping proxy, ws fake-core, page.routeWebSocket, ext fixtures)
  • PR 3: client cut-over (browser factory → ws, partysocket wrapper, rebind, 6-tab gate red→green; Safari/WKWebView ws:// from https matrix is the entry gate)

🤖 Generated with Claude Code

…unt (#314)

Build `{...coreRouter, ext: {<slug>: router}}` after extensions mount and
serve that identical composite from both a fetch RPCHandler at `/rpc` and a
new WebSocket mount at `/rpc-ws`. The per-extension fetch middlewares go
away; `/rpc/<proc>` and `/rpc/ext/<slug>/<proc>` are byte-identical, so this
is additive on the server with zero client changes.

Per-call context is derived once, in a shared oRPC `rootInterceptors` seam
(`@conciv/extension/rpc-mount`), and used by both mounts plus the terminal
test fixture. `RpcContext` becomes `{origin, headers}` so procedures read
per-call headers from the standard request instead of a raw `Request` — over
ws the upgrade request would otherwise swallow `conciv-session-id`.

The ws route uses `upgradeWebSocket`'s `onMessage`/`onClose` hooks, never
`handler.upgrade()`, so frames buffered before open are not dropped. It sits
behind the existing global corsMiddleware; no second enforcement path.

`@conciv/serve` owns the WebSocketServer lifecycle: explicit `maxPayload`,
graceful close of live sockets with `terminate()` only after a deadline
(`events.once` + `AbortSignal.timeout`), and a `fetch` type that accepts the
server env argument so a wrapper cannot silently 500 every upgrade.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds composite oRPC routing over fetch and WebSocket to address browser HTTP connection starvation.

Changes:

  • Combines core and extension routers while preserving existing fetch URLs.
  • Adds /rpc-ws, shared per-call context, and integration coverage.
  • Adds graceful WebSocket shutdown and payload limits.

Reviewed changes

Copilot reviewed 24 out of 25 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
.changeset/ws-transport-server-mount.md Documents RPC transport changes.
AGENTS.md Updates gate concurrency guidance.
pnpm-lock.yaml Resolves dependency updates.
packages/core/package.json Adds WebSocket runtime/test dependencies.
packages/core/src/api/rpc/mount.ts Implements composite fetch/WS mounts.
packages/core/src/api/rpc/router.ts Uses transport-neutral RPC context.
packages/core/src/app.ts Mounts the composite router.
packages/core/src/chat/commands.ts Builds MCP URLs from context origin.
packages/core/src/lib/api-base.ts Removes superseded URL helper.
packages/core/src/start.ts Adopts environment-aware fetch typing.
packages/core/test/api/extension-router.it.test.ts Updates extension context coverage.
packages/core/test/api/rpc-ws.it.test.ts Tests WebSocket RPC behavior.
packages/extension/package.json Exports RPC mounting helpers.
packages/extension/src/rpc-mount.ts Adds shared RPC context interception.
packages/extension/tsdown.config.ts Builds the new RPC entry point.
packages/extensions/terminal/src/server.ts Migrates terminal RPC context.
packages/extensions/terminal/test/helpers.ts Updates terminal test mounting.
packages/protocol/package.json Exports RPC context types.
packages/protocol/src/rpc-types.ts Defines transport-neutral RPC context.
packages/protocol/tsdown.config.ts Builds RPC type exports.
packages/serve/package.json Enables serve-package tests.
packages/serve/src/serve.ts Adds WS limits and teardown.
packages/serve/test/teardown.it.test.ts Tests socket teardown behavior.
packages/serve/tsconfig.json Includes tests and Vitest configuration.
packages/serve/vitest.config.ts Configures Node-based Vitest tests.
Files not reviewed (1)
  • pnpm-lock.yaml: Generated file

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/serve/src/serve.ts Outdated
Comment on lines 41 to 44
return async () => {
await closeLiveSockets(wss, gracefulCloseMs)
if ('closeAllConnections' in server) server.closeAllConnections()
await new Promise<void>((resolve, reject) => server.close((error) => (error ? reject(error) : resolve())))
Comment thread packages/core/src/api/rpc/mount.ts Outdated
Comment on lines +34 to +36
export type MountedExtensionRouter = {slug: string; router: AnyRouter}

export type CompositeRpcRouter = ReturnType<typeof makeRpcRouter> & {ext: Record<string, AnyRouter>}
const served = opts.token ? new Hono().mount(prefix, app.fetch) : app
const {port, close} = await serveHono({fetch: served.fetch})
const base = `http://127.0.0.1:${port}${prefix}`
const shutdown = {done: null as Promise<void> | null}
…oles in the ws mount (#314)

Four findings from adversarial review of the composite rpc router:

Teardown admission race: closeLiveSockets snapshotted wss.clients and drained
before server.close(), so a socket upgraded after the snapshot was never closed
and closeAllConnections() does not cover upgraded sockets, leaving close()
hung forever. Stop admission first (server.close() initiates), then drain in a
loop bounded by the same deadline so late arrivals are caught, terminate any
remainder, then await the close callback.

Unhandled rejection on malformed frames: handler.message() rejects when oRPC
fails to decode a frame, and hono only catches sync throws, so one garbage text
frame became a process-level unhandled rejection. Catch it, log through the
existing error path, and close that peer with 1011.

Slug collision: uniqueness was asserted on extension NAMES, but "Foo Bar" and
"foo-bar" share slug "foo-bar" and Object.fromEntries is last-wins, silently
re-pointing an existing URL at a different extension (main's sequential
middlewares were first-wins). Throw at composite construction instead.

The shared handler-options seam is typed with StandardRPCHandlerOptions from
@orpc/server/standard rather than the fetch adapter's RPCHandlerOptions, which
Omit<>s plugins and nominally coupled the shared seam to one transport.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants