[Bug] dsh-client-connection@0.1.5-rc.1 fix(client-connection): register() resolves webServer without inject, breaking every rpc.handle() consumer #6227
Replies: 4 comments 2 replies
|
Independently reproduced, and the proposed fix is verified working end-to-end. Environment: macOS 15, Node v25.9.0, Confirmed both things you described:
Two data points for the report1. The blast radius is larger than a single plugin. You noted the out-of-tree plugin appeared to be the only 2. Scope is exactly Verified fixYour suggested direction works. I used a variant that preserves the original effect-ownership semantics: // apply() — reuse the webCtx already created here
ctx.inject(["webServer"], (webCtx) => {
connection.webCtx = webCtx;
// ...existing /api route registration unchanged
});
// register() — resolve webServer through the webServer-injected context,
// but keep owner.effect() so the route still dies with its caller
return owner.effect(
() => (this.webCtx ?? owner).webServer.register(route),
`client-connection: ${channel} rpc channel`
);One detail worth keeping if you take the Results after patching only
|
| Check | Result |
|---|---|
| plugin tree load | succeeds, no error |
| plugin entries | 141 active / 28 disabled / 0 with a load error |
POST /api/pluginManager/list |
{"ok":true}, full entry list returned |
| RPC channel reachability | /api/pluginManager/list answers 200 with a structured response; unknown endpoints 404 |
Reproduced on a real profile with all four rpc.handle() consumers installed and enabled — not a unit-test-only confirmation. I verified at the API level rather than through the rendered UI, so treat the "Web UI boots" half as implied by the above rather than separately measured.
|
Thanks @bfg10knewtype — happy to. One wrinkle worth flagging first.
So I pushed the commit to my fork and am pasting it below — please take it however is most useful:
diff --git a/packages/client/connection/src/index.ts b/packages/client/connection/src/index.ts
index 98a4354..f466b69 100644
--- a/packages/client/connection/src/index.ts
+++ b/packages/client/connection/src/index.ts
@@ -118,6 +118,9 @@ export async function apply(ctx: Context, config?: ConnectionConfig): Promise<vo
)
ctx.inject(['webServer'], (webCtx) => {
assertImageBodyCapacity(webCtx, maxRequestBodyBytes)
+ // `rpc.handle` mounts caller-owned channels on the HTTP server; hand it the
+ // context that actually injected `webServer` (see attachWebContext).
+ connection.attachWebContext(webCtx)
webCtx.on('webserver/index-inject', (table) => {
table.push({ kind: 'global', name: '__DSH_CONNECTION_RECOVERY__', value: recovery })
})
diff --git a/packages/client/connection/src/rpc-host.ts b/packages/client/connection/src/rpc-host.ts
index d74734e..bbe91f3 100644
--- a/packages/client/connection/src/rpc-host.ts
+++ b/packages/client/connection/src/rpc-host.ts
@@ -60,6 +60,11 @@ declare module '@deepseek-ai/cordis' {
export class HostConnectionService extends Service implements HostConnectionHandle {
private readonly interceptors = new Map<string, ConnectionRpcInterceptor>()
private readonly fetchRoutes = new Map<string, RegisteredFetchRoute>()
+ /**
+ * The context that injected `webServer`, adopted via {@link attachWebContext}.
+ * `undefined` on deployments without an HTTP server (headless/tui).
+ */
+ private webCtx: Context | undefined
/**
* Provide the Host half over the active HTTP server.
@@ -75,6 +80,23 @@ export class HostConnectionService extends Service implements HostConnectionHand
super(ctx, 'connection')
}
+ /**
+ * Adopt the context that injected `webServer`, so channel routes can be
+ * mounted through it.
+ *
+ * `rpc.handle` receives the *caller's* Context as its owner, but a caller
+ * cannot inject `webServer` itself: the service lives on a sibling row, so
+ * property access walks only the caller's own ancestor chain and fails with
+ * `cannot get property "webServer" without inject` — which aborts the whole
+ * plugin-tree load, not just the offending plugin. The context that mounted
+ * this service is the one place that both injects `webServer` and stays alive
+ * for the process lifetime, so it is the correct resolver.
+ * @param webCtx - the `webServer`-injected context.
+ */
+ attachWebContext(webCtx: Context): void {
+ this.webCtx = webCtx
+ }
+
/** Generic channel registry scoped to the Context reading this service. */
get rpc(): HostConnectionRpc {
const owner = this.ctx
@@ -175,8 +197,14 @@ export class HostConnectionService extends Service implements HostConnectionHand
await bridge(req, res, fetchHandler)
},
}
+ // Resolve through the webServer-injected context, but keep `owner.effect`
+ // as the OUTER wrapper: that is what ties the route's lifetime to the
+ // calling plugin. `webCtx.effect(...)` would tie it to Connection instead,
+ // so a caller that unloads would leave its channel mounted. The `?? owner`
+ // fallback preserves today's behaviour — and its loud failure — on
+ // deployments without an HTTP server (headless/tui).
return owner.effect(
- () => owner.webServer.register(route),
+ () => (this.webCtx ?? owner).webServer.register(route),
`client-connection: ${channel} rpc channel`,
)
}If there is a repository where you'd like this as an actual PR — a downstream fork, a patch collection, anything — point me at it and I'll open it there. And yes please to the Windows 11 / Node v24 run against a real profile. This is a whole-tree load failure with load-order masking, so an independent composition is exactly the evidence it deserves. |
|
Independent repro of the same root cause, plus a same-package contrast that makes it obvious: Write-up: #6337 |
|
Adding a cross-link and one data point: reproduced this exact issue via For what it's worth, I also tried applying @wsxwj123's verified |
Uh oh!
There was an error while loading. Please reload this page.
Environment
0.1.5-rc.1(npm@deepseek-ai/dsh@0.1.5-rc.1, also reproduced from a monorepo checkout of the same tag)ctx.connection.rpc.handle(...)Summary
HostConnectionService.register()resolves its route throughowner.webServer, but the connection plugin's own staticinjectno longer declareswebServer. Any plugin that registers a channel throughctx.connection.rpc.handle()therefore fails the whole plugin-tree load with:The failure takes down the entire Web UI — not just the offending plugin.
Root cause
In
@deepseek-ai/dsh-client-connection@0.1.5-rc.1,lib/index.js:ownerhere isthis.ctx— the connection plugin's own context, not the calling plugin's. Line 618 resolvesowner.webServer, which requires the connection fiber to declarewebServerin its inject map.But the plugin's static declaration dropped it during this release:
Versus the previous release:
The plugin now acquires
webServerthrough a dynamic inject inapply()instead (line 758), which does not extend the plugin fiber's owninjectmap:So the plugin's own
/apiroute is fine, but the publicregister()API — which third-party plugins call throughrpc.handle()— is not.The cordis guard that throws is
reflect.ts/lib/index.js:680-694:Because the failing fiber is the connection plugin's, adding
webServerto the calling plugin's inject has no effect — which makes this bug especially confusing to diagnose from plugin side.Why it is not caught by in-tree tests
No in-tree plugin uses
connection.rpc.handle(). In-tree consumers either register exact Fetch routes (connection.fetch.register, which never touchesowner.webServer) or callctx.webServer.register()directly from a plugin that does declare the injection. This out-of-tree plugin appears to be the onlyrpc.handle()consumer, so the regression slipped through.Suggested fix
Make
register()resolvewebServerthe same wayapply()already does, e.g.:…or restore
'webServer'in the plugin's staticinjectif the connection plugin is meant to require the web server (note this would also change headless/SDK load behaviour, which is presumably why the dynamic inject was introduced).Workaround for plugin authors (works on 0.1.2 and 0.1.5)
Do not let the connection plugin resolve
webServerfor itself. Obtain a webServer-injected child context and pass it as the owner:Verified working against both
0.1.2-rc.1and0.1.5-rc.1.Reproduction
0.1.5-rc.1.ctx.connection.rpc.handle('/channel', handler, { authority: 'loopback' })inapply().dsh web→ plugin tree fails to load, Web UI never starts.Related finding (same release): an undocumented breaking change
While adapting the same plugin to
0.1.5-rc.1we hit a second breakage that does not appear in the release notes' breaking-change list (which documents only "移除ctx.agent,调用方需显式传递 Agent").Session.events— the public array property — was replaced by thesnapshotEvents()method. Reading the old property now yieldsundefined, with two very different symptoms:The second form is the dangerous one: nothing throws, the feature just stops producing output. In our case an auto-summarizer quietly stopped writing entries into the memory store; it was found only because a different consumer crashed on the same property.
Suggested documentation fix: list
session.events → snapshotEvents()(witheventAt()/ownEvents()as the indexed-read replacements) among the0.1.5breaking changes.Cross-version read that works on both releases:
All reactions