From 8c4c5e7eabde13a028f4222497534fa57b3b28d3 Mon Sep 17 00:00:00 2001 From: Tapan Chugh Date: Tue, 4 Aug 2026 15:07:59 -0700 Subject: [PATCH] fix(openclaw): release the gateway binding when the status callback throws startGatewayAccountEffect registers the account in activeClients before connecting, but the connect path had no cleanup on failure. The host's setStatus is arbitrary caller code, and reportConnected invokes it inside Effect.sync, so a throw surfaces as a defect rather than a failure -- which connectGatewayCore's catchAll does not observe. The binding stayed registered and the core was never disconnected. Release the binding on any error escaping the connect path, and make disconnectAndRemove drop the entry even when disconnect itself fails, matching disconnectCoreOnAbort which already used Effect.ensuring. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01P76aaa1STr3WPZ3nDascta --- docs/modules/openclaw-channel/src.mdx | 8 +-- packages/openclaw-channel/src/MODULE.md | 8 +-- .../src/openclaw-entry.delivery.test.ts | 55 +++++++++++++++++++ .../openclaw-channel/src/openclaw-entry.ts | 13 ++++- 4 files changed, 74 insertions(+), 10 deletions(-) diff --git a/docs/modules/openclaw-channel/src.mdx b/docs/modules/openclaw-channel/src.mdx index 59259f4b6..78593b5fe 100644 --- a/docs/modules/openclaw-channel/src.mdx +++ b/docs/modules/openclaw-channel/src.mdx @@ -15,7 +15,7 @@ runtime entries from `index.*` at the extension root only, so the built ## Public surface -### [`createMoltzapChannelPlugin`](https://github.com/chughtapan/moltzap/blob/main/packages/openclaw-channel/src/openclaw-entry.ts#L1226) +### [`createMoltzapChannelPlugin`](https://github.com/chughtapan/moltzap/blob/main/packages/openclaw-channel/src/openclaw-entry.ts#L1235) _Function_ @@ -63,7 +63,7 @@ to `agent:<name>`. Other colon-prefixed shapes are rejected. **Returns:** The created moltzap channel plugin. -### [`default`](https://github.com/chughtapan/moltzap/blob/main/packages/openclaw-channel/src/openclaw-entry.ts#L1256) +### [`default`](https://github.com/chughtapan/moltzap/blob/main/packages/openclaw-channel/src/openclaw-entry.ts#L1265) _Variable_ @@ -71,7 +71,7 @@ _Variable_ const plugin = ``` -### [`moltzapChannelPlugin`](https://github.com/chughtapan/moltzap/blob/main/packages/openclaw-channel/src/openclaw-entry.ts#L1253) +### [`moltzapChannelPlugin`](https://github.com/chughtapan/moltzap/blob/main/packages/openclaw-channel/src/openclaw-entry.ts#L1262) _Variable_ @@ -84,7 +84,7 @@ Shared singleton so a single registration reuses the same `activeClients` closure across `startAccount` and `sendText`. Tests import this directly to assert against that shared state. -### [`MoltzapChannelPlugin`](https://github.com/chughtapan/moltzap/blob/main/packages/openclaw-channel/src/openclaw-entry.ts#L1244) +### [`MoltzapChannelPlugin`](https://github.com/chughtapan/moltzap/blob/main/packages/openclaw-channel/src/openclaw-entry.ts#L1253) _TypeAlias_ diff --git a/packages/openclaw-channel/src/MODULE.md b/packages/openclaw-channel/src/MODULE.md index d9917de24..dc2597289 100644 --- a/packages/openclaw-channel/src/MODULE.md +++ b/packages/openclaw-channel/src/MODULE.md @@ -10,7 +10,7 @@ runtime entries from `index.*` at the extension root only, so the built ## Public surface -### [`createMoltzapChannelPlugin`](./openclaw-entry.ts#L1226) +### [`createMoltzapChannelPlugin`](./openclaw-entry.ts#L1235) _Function_ @@ -58,7 +58,7 @@ to `agent:<name>`. Other colon-prefixed shapes are rejected. **Returns:** The created moltzap channel plugin. -### [`default`](./openclaw-entry.ts#L1256) +### [`default`](./openclaw-entry.ts#L1265) _Variable_ @@ -66,7 +66,7 @@ _Variable_ const plugin = ``` -### [`moltzapChannelPlugin`](./openclaw-entry.ts#L1253) +### [`moltzapChannelPlugin`](./openclaw-entry.ts#L1262) _Variable_ @@ -79,7 +79,7 @@ Shared singleton so a single registration reuses the same `activeClients` closure across `startAccount` and `sendText`. Tests import this directly to assert against that shared state. -### [`MoltzapChannelPlugin`](./openclaw-entry.ts#L1244) +### [`MoltzapChannelPlugin`](./openclaw-entry.ts#L1253) _TypeAlias_ diff --git a/packages/openclaw-channel/src/openclaw-entry.delivery.test.ts b/packages/openclaw-channel/src/openclaw-entry.delivery.test.ts index 581259dd8..e5c6cc949 100644 --- a/packages/openclaw-channel/src/openclaw-entry.delivery.test.ts +++ b/packages/openclaw-channel/src/openclaw-entry.delivery.test.ts @@ -52,6 +52,9 @@ const OUTBOUND_TEXT = "Hello from outbound"; const AGENT_TEXT = "Hello nova"; const BEFORE_STOP_TEXT = "before stop"; const AFTER_STOP_TEXT = "after stop"; +const AFTER_STATUS_FAILURE_TEXT = "after status failure"; +const STATUS_FAILURE_ACCOUNT_ID = "status-failure-test"; +const STATUS_CALLBACK_MESSAGE = "status callback failed"; const LOOKUP_FAILED_MESSAGE = "lookup failed"; const SERVER_REJECTED_MESSAGE = "Server rejected"; const INTERNAL_SERVER_ERROR_MESSAGE = "Internal server error"; @@ -160,6 +163,10 @@ describe("Flow 6: Outbound delivery - deliver callback + sendText", () => { it("deliver reports transient RPC send failures", sendFailureIsReported); it("a later delivery retries after a send failure", retriesAfterSendFailure); it("stopAccount removes client from active pool", stopRemovesClient); + it( + "releases the gateway when the status callback throws", + statusFailureReleasesGateway, + ); it( "property: resolveTarget normalizes generated agent names", plainAgentNamesResolve, @@ -614,6 +621,54 @@ function stopRemovesClient() { }); } +// The host's setStatus is arbitrary caller code. Effect.sync surfaces a throw +// from it as a defect, which the connect path's catchAll does not observe, so +// this asserts on the binding rather than on the raised error. +function statusFailureReleasesGateway() { + return Effect.gen(function* () { + const fixture = createFakeChannelService({ ownAgentId: SELF_AGENT_ID }); + const setStatus = vi.fn(() => { + throw new Error(STATUS_CALLBACK_MESSAGE); + }); + const plugin = createMoltzapChannelPlugin({ + createService: () => fixture.service, + }); + const abortController = new AbortController(); + abortControllers.push(abortController); + + yield* Effect.ignore( + Effect.tryPromise({ + try: () => + plugin.gateway.startAccount({ + cfg: makeCfg(), + accountId: STATUS_FAILURE_ACCOUNT_ID, + account: makeAccount(), + abortSignal: abortController.signal, + log: testLogger(), + setStatus, + }), + catch: (cause) => cause, + }), + ); + + expect(setStatus).toHaveBeenCalledTimes(1); + expect(fixture.state.closeCalls.count).toBe(1); + + const result = yield* Effect.tryPromise({ + try: () => + plugin.outbound.sendText({ + cfg: makeCfg(), + to: STOP_TARGET, + text: AFTER_STATUS_FAILURE_TEXT, + accountId: STATUS_FAILURE_ACCOUNT_ID, + }), + catch: (cause) => + new DeliveryTestError({ message: "sendText failed", cause }), + }); + expectFailureMessage(result, /not connected/i); + }); +} + function plainAgentNamesResolve() { return Effect.sync(() => { fc.assert( diff --git a/packages/openclaw-channel/src/openclaw-entry.ts b/packages/openclaw-channel/src/openclaw-entry.ts index 80246d8d3..b95e7806d 100644 --- a/packages/openclaw-channel/src/openclaw-entry.ts +++ b/packages/openclaw-channel/src/openclaw-entry.ts @@ -769,7 +769,14 @@ function startGatewayAccountEffect( }, { once: true }, ); - yield* connectGatewayCore(core, service, ctx, setStatus); + // The binding is registered before connecting, so anything that escapes the + // connect path — including a host setStatus callback that throws, which + // Effect.sync surfaces as a defect rather than a failure — must release it. + yield* connectGatewayCore(core, service, ctx, setStatus).pipe( + Effect.onError(() => + Effect.ignore(disconnectAndRemove(core, activeClients, accountId)), + ), + ); }); } @@ -799,9 +806,11 @@ function disconnectAndRemove( activeClients: Map, accountId: string, ) { + // A disconnect that fails must still drop the binding; leaving it registered + // would strand the account with no route to recovery. return core .disconnect() - .pipe(Effect.tap(() => Effect.sync(() => activeClients.delete(accountId)))); + .pipe(Effect.ensuring(Effect.sync(() => activeClients.delete(accountId)))); } interface RegisterInboundHandlerParams {