fix(client-connection): resolve webServer from Connection, not the RPC caller (PRs appear disabled — fix branch attached)

[SergioMazariego]

Summary

A third-party plugin that registers an RPC endpoint through the public, carrier-neutral connection.rpc.handle() API silently fails to mount its route in a Web composition, because the Host implementation resolves webServer from the calling plugin's own context instead of from Connection's own context. I found the root cause, fixed it, and added regression coverage. Pull requests appear to be disabled on this repository, so I'm posting the fix here instead, with the full diff on a public branch for a maintainer to cherry-pick or pull directly.

Branch (fork): https://github.com/SergioMazariego/deepseek-harness/tree/fix/client-connection-external-rpc-web
Commit: 118e6c9be8bd4408fc965d7826864d0fb2dc0a0b
Compare view: master...SergioMazariego:deepseek-harness:fix/client-connection-external-rpc-web

Regression

DSH 0.1.5-rc.1
Windows

Settings -> Subscriptions

Failed to load proxy settings:
transport failure for /subscriptions-auth/proxyGet: HTTP 405

The endpoint belongs to a third-party plugin that authenticates providers via subscriptions/OAuth instead of API keys. It registers its RPC endpoints through Connection's public API (connection.rpc.handle(...)), declaring only inject: ['connection'] — the documented contract for that API, which is meant to be carrier-neutral.

Root cause

HostConnectionService.register() (the implementation behind connection.rpc.handle(), in packages/client/connection/src/rpc-host.ts) mounted the channel's route by calling owner.webServer.register(route) directly on owner, the calling plugin's own context (get rpc() captures owner = this.ctx, rebound per caller — see the class doc: "whose channel registrations belong to the caller fiber").

Cordis resolves a service reference by walking the accessing fiber's own ancestor chain; a fiber that never crosses a webServer-providing ancestor gets cannot get property "webServer" without inject. A plugin that only declares inject: ['connection'] — exactly what the public API asks for — has no such path to webServer whenever webServer is provided by its own separate plugin fiber rather than directly on the root context (the shape of a normal Web composition, and the shape the existing test suite happened not to exercise: its mounted() helper called ctx.provide('webServer', …) directly on the literal root, which masks the bug).

So the very first call to connection.rpc.handle() from such a plugin throws synchronously inside the registration effect. That failure is local to the registering plugin's own fiber, so the rest of the app keeps running — but the plugin's channel is never mounted. A later request to that channel's path finds no route, falls through to the webServer's fallback seat, and — under the shipped Web composition, where the fallback is frontend-static — receives HTTP 405, because frontend-static's fallback answers any non-GET/HEAD request with 405 outside its own named routes (packages/host/frontend-static/src/index.ts).

This is not a new dependency requirement working as intended: connection.rpc.intercept('/api', …) and connection.fetch.register(...) — the other two RPC/Fetch registration APIs on the same service — never touch webServer at all; they only touch Connection's own internal maps and piggyback on the one /api route Connection's own apply() already mounts through its existing optional ctx.inject(['webServer'], …). rpc.handle() was the one path that bypassed that abstraction and reached for the caller's own webServer access instead.

Fix

HostConnectionService now resolves webServer itself, through its own soft dependency declared in its constructor (ctx.inject(['webServer'], …), on Connection's own context — the same context that already optionally mounts /api), instead of requiring the calling plugin's context to reach it:

• register() records the channel's WebRoute in an internal map (channelRoutes) and mounts it immediately if webServer is already active.
• The constructor's ctx.inject(['webServer'], …) callback mounts every already-recorded channel whenever webServer (re)activates, and withdraws every mounted channel when webServer stops (a restart, or an isolated realm unloading).
• The disposer returned to the caller (rpc.handle()'s return value) still belongs to the caller's own fiber — removing the channel's registration when that plugin unloads — preserving the class's existing "channel registrations belong to the caller fiber" contract. Only the route-mounting step moved off the caller's context.

This is the minimal change: no changes to registerInterceptor/registerFetchRoute (already carrier-neutral), no changes to the public HostConnectionRpc/HostConnectionFetch types, and client-connection's own top-level inject = ['credentials'] is untouched — webServer stays optional for the whole plugin, exactly as the existing doc comment describes ("Provides carrier-neutral RPC and Fetch registries. When webServer is present, the plugin also mounts the /api browser transport"), so Desktop, headless, and other webServer-less compositions are unaffected.

Tests

• Added packages/client/connection/tests/external-rpc-web.host.spec.ts:
  • Mounts webServer as its own nested plugin fiber (not directly on the root context, which is what let the existing suite miss this) alongside client-connection, then a sibling plugin declaring only inject: ['connection'] that calls connection.rpc.handle(...). Asserts the route mounts, a real request reaches the handler (not a 405), and disposing the external plugin withdraws the route.
  • A second case registers the channel before webServer exists anywhere in the tree, asserts nothing is mounted yet, then activates webServer and asserts the channel mounts; disposing webServer withdraws the route without disturbing the external plugin's own registration.
  • Both cases fail against the pre-fix code with cannot get property "webServer" without inject, and pass with the fix.
• Updated the one existing assertion whose expected error text changed (packages/client/connection/tests/node-half.host.spec.ts): the duplicate-channel error is now raised by HostConnectionService itself (already registered) rather than by (in that test) the fake webServer's own duplicate-route check.
• Ran the full packages/client/connection suite (165 tests) and packages/host/webserver + packages/host/frontend-static (5 tests): all green.
• tsc -b packages/client/connection/tsconfig.host.json and tsconfig.client.json: no errors.
• oxlint over the changed files: 0 warnings, 0 errors.
• Verified 100% statement/branch coverage on rpc-host.ts (this repo's per-file coverage gate) with the new test alone.

Manual verification

Before:
POST /subscriptions-auth/proxyGet -> HTTP 405

Temporary local workaround (evidence used to localize the regression,
not the fix landed here):
client-connection inject changed from:
['credentials']
to:
['webServer', 'credentials']

After restarting DSH:
subscription proxy settings loaded correctly and external RPC calls succeeded.

That workaround happened to work because it moved client-connection's own base fiber onto a Web composition where webServer was declared close enough in the tree for the caller's own context to resolve it in that particular installation's plugin layout — it does not hold in general (e.g. it cannot help a plugin mounted where no ancestor of its own fiber ever provides webServer), and it makes webServer mandatory for the whole client-connection plugin, which is not architecturally correct (see packages/client/connection/src/index.ts's doc comment). The fix on the branch above resolves webServer from Connection's own context unconditionally, so it works regardless of where a connection.rpc.handle() caller sits in the plugin tree, and keeps webServer optional for client-connection as a whole.

Note on delivery

I tried opening a pull request first (fix/client-connection-external-rpc-web on my fork, SergioMazariego/deepseek-harness, against master), but this repository has Pull Requests disabled at the repository-feature level (confirmed via the API: has_pull_requests: false — listing existing PRs also 404s), so posting here per the README's contribution guidance. Happy to adjust the change, split it, or answer questions — the branch above stays up to date if I need to push a revision.

[PerryLink]

Thanks for the thorough write-up — I verified the root cause against current master (c291e7961a515f6d7af9304e7fd1d257929aef26), and your diagnosis is exactly right.

Where it breaks on master:

• HostConnectionService is documented as "whose channel registrations belong to the caller fiber" — packages/client/connection/src/rpc-host.ts:59.
• The rpc getter binds owner = this.ctx per reader (rpc-host.ts:79-86), so rpc.handle(channel, handler) calls register(owner, channel, handler) with the caller's context.
• register() mounts the channel via owner.webServer.register(route) (rpc-host.ts:158-182, the call is at line 179). Cordis resolves webServer through the accessing fiber, and the constructor declares no injection (rpc-host.ts:70-76), so a plugin that follows the documented contract — inject: ['connection'] only — has no path to webServer and throws cannot get property "webServer" without inject. The route is never mounted, the request falls through to the Web fallback, and the POST gets the HTTP 405 you saw.

I also fetched and reviewed your fix commit 118e6c9be8bd4408fc965d7826864d0fb2dc0a0b from the fork branch. The shape is right and consistent with this codebase's conventions:

• The constructor resolves webServer through its own ctx.inject(['webServer'], …) soft dependency, decoupled from the caller's fiber.
• channelRoutes survives webServer restarts, mountedChannels tracks only live disposers, and the inject callback re-mounts on (re)activation and withdraws on stop — so channels registered before webServer activates still mount.
• rpc.handle() still returns a disposer owned by the caller's fiber, preserving plugin-unload semantics (what the class doc promises).
• The duplicate-channel throw matches the existing exact-Fetch-route behavior (rpc-host.ts:149-155), and the regression test mounts webServer as its own plugin fiber with a sibling plugin injecting only connection — the shape the old suite masked.

Caveat: I reviewed the diff against master's code paths but have not built or run the suite with it applied, so treat this as a structural review, not a green build.

One process note: upstream master shows regular PR merges (e.g. Merge pull request #3977), so if the fork can open a PR that is the normal route; if PR creation is blocked from your fork, this discussion plus the branch is the right fallback and the commit is directly cherry-pickable.

[SergioMazariego]

Thanks for taking the time to trace through this, @PerryLink — glad the root-cause analysis and the fix line up with what you're seeing in the code paths.

Since PRs are disabled on this repo, what's the preferred path to get something like this merged? Happy to:

• open a PR against a different branch/fork target if there's one used for external contributions, or
• have someone with write access cherry-pick the commit directly from SergioMazariego:fix/client-connection-external-rpc-web (118e6c9), or
• provide anything else you'd want to see (full test output, coverage report, etc.) before it lands.

Let me know what works best on your end.

[PerryLink]

Good question, and I can now give you the authoritative answer: this repo does not accept external PRs at the moment — CONTRIBUTING.md states it directly ("We are sorry that we cannot accept external pull requests at the moment", CONTRIBUTING.md:9), and the repo has Issues/PRs turned off in favor of Discussions (I checked: the issues feature is disabled and the pulls API returns 404 for this repository). To be clear about my role: I'm a community contributor like you, not a maintainer — I can verify and document, but I can't merge anything either.

So the realistic path for your fix:

1. The discussion is the channel. This thread already carries the root-cause verification plus your branch and commit (SergioMazariego:fix/client-connection-external-rpc-web @ 118e6c9). Maintainers monitor discussions (CONTRIBUTING.md says so) and can cherry-pick the commit directly — it sits cleanly on master's code paths as far as my structural review went.
2. Make the deliverable turnkey: attach the test output you offered (suite run, coverage) and a short "what / why / how-to-review" summary in a comment here. That's what a maintainer needs to cherry-pick with confidence.
3. Keep the fork branch rebased on master so the commit stays cherry-pickable; if the repo ever re-opens external PRs, that's the moment to open one.

Nothing further needed from me on the code side — the shape, duplicate-channel behavior, and the injected-runner test seam were all consistent with this codebase's conventions.

---