Skip to content

fix(openclaw): release the gateway binding when the status callback throws - #952

Closed
chughtapan wants to merge 1 commit into
mainfrom
fix/openclaw-status-failure-releases-gateway
Closed

fix(openclaw): release the gateway binding when the status callback throws#952
chughtapan wants to merge 1 commit into
mainfrom
fix/openclaw-status-failure-releases-gateway

Conversation

@chughtapan

Copy link
Copy Markdown
Owner

Plan step 2. Based on origin/main, deliberately not on the #943#951 stack: this bug is pre-existing in production, and per the plan's D3 the legacy path stays reachable in production until the OpenClaw cutover lands. This shortens that exposure window and merges independently of the endpoint chain.

The bug

startGatewayAccountEffect registers the account in activeClients before connecting, then ends with connectGatewayCore(...) and no cleanup.

The host's setStatus is arbitrary caller code. reportConnected invokes it inside Effect.sync, so a throw surfaces as a defect, not a failure — and connectGatewayCore's Effect.catchAll only observes failures. The defect escapes with the binding still registered and the core never disconnected: the account is wedged with no route to recovery.

The fix

Two changes, same defect class:

  1. Release the binding on anything escaping the connect path (Effect.onError, which observes defects as well as failures).
  2. disconnectAndRemove uses Effect.ensuring instead of Effect.tap, so a failing disconnect still drops the entry. This matches disconnectCoreOnAbort, which already used Effect.ensuring — that inconsistency was the tell.

No public surface change: the diff adds and removes no exported symbol, type, or interface.

Non-vacuity

The new test fails against the pre-fix source with expected +0 to be 1closeCalls.count was 0, i.e. the core was never disconnected. Verified by reverting only openclaw-entry.ts to origin/main and re-running with the test in place.

It asserts binding release the same way the existing stopRemovesClient does (sendText/not connected/i), since activeClients is private.

Gates

  • pnpm nx run @moltzap/openclaw-channel:test — 67 passed / 7 files
  • pnpm typecheck --skip-nx-cache — green
  • pnpm lint — 0 warnings, 0 errors
  • git diff origin/main | grep -c ActiveLegacyGateway0, and the diff touches exactly 2 source files

The ActiveLegacyGateway<Service> retype from the adapters worktree is deliberately dropped: openclaw-gateway-lifecycle.ts does not exist on origin/main, and every symbol it would retype is deleted by the OpenClaw cutover. Generated MODULE.md / src.mdx churn is line-number shift only.

ADR conformance (R-pass)

Governing outcome Record Owner Binds this diff? Verdict Evidence
— none found — n/a n/a n/a CONFORMS (vacuously) Discovered from docs/decisions/README.md on main (49 records). No accepted outcome governs internal error handling in the OpenClaw adapter's gateway binding. 20260723-lifecycle-rides-l3 concerns conversation lifecycle at L3, not adapter process lifecycle, and is partially-superseded.

Per R-pass rule 4, "no ADR governs this" is stated as a reviewable claim, not left as silence. The diff changes no wire field, tool catalog, public contract, or trust boundary — the categories ADRs in this repo govern.

🤖 Generated with Claude Code

https://claude.ai/code/session_01P76aaa1STr3WPZ3nDascta

…hrows

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P76aaa1STr3WPZ3nDascta
@mintlify

mintlify Bot commented Aug 4, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
moltzap 🟢 Ready View Preview Aug 4, 2026, 10:09 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@chughtapan

Copy link
Copy Markdown
Owner Author

Closing as obsolete rather than folding into #974 — the bug this fixed is already gone there, by construction.

This patched connectGatewayCore and disconnectAndRemove so a throwing setStatus could not strand an activeClients entry. Both symbols were deleted by the OpenClaw cutover, and this branch conflicts with HEAD.

The guarantee survives without the patch. In #974:

yield* Effect.sync(() => activeHarnessClients.set(ctx.accountId, active));
yield* reportHarnessConnected(client, ctx).pipe(   // the setStatus call
  Effect.zipRight(...),
  Effect.ensuring(finishHarnessClient(activeHarnessClients, ctx.accountId, active)),
);

Effect.ensuring wraps the exact call that invokes setStatus, and the whole thing sits under Effect.scoped, so a throw releases both the map entry and the client. Same outcome, reached by deleting the machinery instead of patching it.

Merging this would resurrect deleted symbols to fix a bug that no longer exists. Reopen if you disagree.

@chughtapan chughtapan closed this Aug 6, 2026
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.

1 participant