Gateway: bound websocket shutdown close by mbelinky · Pull Request #61565 · openclaw/openclaw

mbelinky · 2026-04-05T23:57:40Z

Summary

bound gateway websocket shutdown so restart/stop can continue even if wss.close() never resolves
terminate tracked websocket clients after a short grace window and continue after a final force window
add regressions for both lingering-client and zero-tracked-client hangs

Why

On jpclawhq, gateway restarts were hanging in shutdown until the 25s watchdog killed the process. Root cause was createGatewayCloseHandler waiting forever on wss.close() unless the server both tracked clients and cooperated with close callbacks.

Testing

bunx vitest run src/gateway/server-close.test.ts --pool forks --maxWorkers 1
pnpm build in the clean worktree hit an unrelated existing @anthropic-ai/sdk unresolved-import failure from src/agents/anthropic-transport-stream.ts
verified the same logic live on jpclawhq: restart now completes cleanly instead of timing out in websocket shutdown

AI

AI-assisted
fully tested on the touched shutdown path; broader maintainer gates still pending

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9040dd5715

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-04-06T00:55:55Z

+            setTimeout(() => {
+              shutdownLog.warn(
+                `websocket server close still pending after ${WEBSOCKET_CLOSE_FORCE_CONTINUE_MS}ms force window; continuing shutdown`,
+              );
+              resolve();


Clear force-window timer when websocket close finishes

Promise.race does not cancel losing branches, so the setTimeout used for the force window keeps running even when closePromise resolves first. In that common case (close completes shortly after client termination), this callback still fires and logs "websocket server close still pending..." even though shutdown already succeeded, which produces false operational warnings and can keep the process alive unnecessarily for another 250ms.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-04-06T00:55:55Z

+      const closedWithinGrace = await Promise.race([
+        closePromise.then(() => true),
+        new Promise<false>((resolve) => setTimeout(() => resolve(false), WEBSOCKET_CLOSE_GRACE_MS)),
+      ]);


Cancel grace timer once websocket close resolves

The grace-period setTimeout in the first Promise.race is never cleared when closePromise resolves quickly. That leaves a pending 1s timer on every normal shutdown path, which can delay process exit/restart despite websocket close already being complete. Using a cancellable timer (or clearing the timeout after the race settles) avoids this new shutdown latency.

Useful? React with 👍 / 👎.

greptile-apps · 2026-04-06T00:56:12Z

Greptile Summary

Bounds gateway WebSocket shutdown with a 1 s grace window followed by forced client termination and a 250 ms force-continue window, fixing an indefinite hang in createGatewayCloseHandler that caused restarts on jpclawhq to wait out the full 25 s watchdog. The two-stage logic is correct and the regression tests cover both the lingering-client and zero-tracked-client hang paths.

Confidence Score: 5/5

Safe to merge — the shutdown fix is correct and no blocking issues found.

All findings are P2 style. The core logic correctly races wss.close() against a grace timeout, terminates tracked clients on expiry, and force-continues after a second short window. Both regression tests exercise the right failure modes.

No files require special attention.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/gateway/server-close.test.ts
Line: 6-7

Comment:
**Timing constants duplicated from source**

`WEBSOCKET_CLOSE_GRACE_MS` and `WEBSOCKET_CLOSE_FORCE_CONTINUE_MS` are re-declared in the test rather than imported from `server-close.ts`. If either value is adjusted in the source, `vi.advanceTimersByTimeAsync(...)` in both tests will advance by the wrong amount — the lingering-client test would never see `terminate` called, and the zero-client test would advance past the actual grace window without triggering the force path, causing silent or confusing failures.

Consider exporting the constants from `server-close.ts` and importing them here to keep them automatically in sync.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "gateway: add changelog attribution" | Re-trigger Greptile}

greptile-apps · 2026-04-06T00:56:16Z

+const WEBSOCKET_CLOSE_GRACE_MS = 1_000;
+const WEBSOCKET_CLOSE_FORCE_CONTINUE_MS = 250;


Timing constants duplicated from source

WEBSOCKET_CLOSE_GRACE_MS and WEBSOCKET_CLOSE_FORCE_CONTINUE_MS are re-declared in the test rather than imported from server-close.ts. If either value is adjusted in the source, vi.advanceTimersByTimeAsync(...) in both tests will advance by the wrong amount — the lingering-client test would never see terminate called, and the zero-client test would advance past the actual grace window without triggering the force path, causing silent or confusing failures.

Consider exporting the constants from server-close.ts and importing them here to keep them automatically in sync.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/gateway/server-close.test.ts Line: 6-7 Comment: **Timing constants duplicated from source** `WEBSOCKET_CLOSE_GRACE_MS` and `WEBSOCKET_CLOSE_FORCE_CONTINUE_MS` are re-declared in the test rather than imported from `server-close.ts`. If either value is adjusted in the source, `vi.advanceTimersByTimeAsync(...)` in both tests will advance by the wrong amount — the lingering-client test would never see `terminate` called, and the zero-client test would advance past the actual grace window without triggering the force path, causing silent or confusing failures. Consider exporting the constants from `server-close.ts` and importing them here to keep them automatically in sync. How can I resolve this? If you propose a fix, please make it concise.

@mbelinky