fix(gateway): add max reconnect limit to scheduleReconnect (#45469) by stellamariesays · Pull Request #77556 · openclaw/openclaw

stellamariesays · 2026-05-04T21:59:16Z

scheduleReconnect() retried indefinitely with exponential backoff capped at 30s but no maximum retry count. If the gateway is unreachable, the client loops forever accumulating listeners and zombie processes.

Changes:

Add MAX_RECONNECT_ATTEMPTS (30) as a static class constant
scheduleReconnect() now gives up after 30 failed attempts, fires onReconnectPaused with a descriptive reason, and sets closed = true
reconnectAttempts resets to 0 on successful hello handshake
reconnectAttempts resets to 0 on explicit stop()
Two new tests: max-reconnect-gives-up and counter-resets-on-success

Closes #45469

Summary

Describe the problem and fix in 2–5 bullets:

If this PR fixes a plugin beta-release blocker, title it fix(<plugin-id>): beta blocker - <summary> and link the matching Beta blocker: <plugin-name> - <summary> issue labeled beta-blocker. Contributors cannot label PRs, so the title is the PR-side signal for maintainers and automation.

Problem:
Why it matters:
What changed:
What did NOT change (scope boundary):

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Closes #
Related #
This PR fixes a bug or regression

Root Cause (if applicable)

For bug fixes or regressions, explain why this happened, not just what changed. Otherwise write N/A. If the cause is unclear, write Unknown.

Root cause:
Missing detection / guardrail:
Contributing context (if known):

Regression Test Plan (if applicable)

For bug fixes or regressions, name the smallest reliable test coverage that should catch this. Otherwise write N/A.

Coverage level that should have caught this:
- Unit test
- Seam / integration test
- End-to-end test
- Existing coverage already sufficient
Target test or file:
Scenario the test should lock in:
Why this is the smallest reliable guardrail:
Existing test that already covers this (if any):
If no new test is added, why not:

User-visible / Behavior Changes

List user-visible changes (including defaults/config).
If none, write None.

Diagram (if applicable)

For UI changes or non-trivial logic flows, include a small ASCII diagram reviewers can scan quickly. Otherwise write N/A.

Before:
[user action] -> [old state]

After:
[user action] -> [new state] -> [result]

Security Impact (required)

New permissions/capabilities? (Yes/No)
Secrets/tokens handling changed? (Yes/No)
New/changed network calls? (Yes/No)
Command/tool execution surface changed? (Yes/No)
Data access scope changed? (Yes/No)
If any Yes, explain risk + mitigation:

Repro + Verification

Environment

OS:
Runtime/container:
Model/provider:
Integration/channel (if any):
Relevant config (redacted):

Steps

Expected

Actual

Evidence

Attach at least one:

Failing test/log before + passing after
Trace/log snippets
Screenshot/recording
Perf numbers (if relevant)

Human Verification (required)

What you personally verified (not just CI), and how:

Verified scenarios:
Edge cases checked:
What you did not verify:

Review Conversations

I replied to or resolved every bot review conversation I addressed in this PR.
I left unresolved only the conversations that still need reviewer or maintainer judgment.

If a bot review conversation is addressed by this PR, resolve that conversation yourself. Do not leave bot review conversation cleanup for maintainers.

Compatibility / Migration

Backward compatible? (Yes/No)
Config/env changes? (Yes/No)
Migration needed? (Yes/No)
If yes, exact upgrade steps:

Risks and Mitigations

List only real risks for this PR. Add/remove entries as needed. If none, write None.

Risk:
- Mitigation:

clawsweeper · 2026-05-04T22:04:56Z

Codex review: needs real behavior proof before merge.

Summary
This PR adds an optional maxReconnectAttempts GatewayClient option, tracks and resets reconnect attempts, pauses reconnect after the opt-in cap, and adds two client tests.

Reproducibility: yes. Source inspection shows current main can keep scheduling ordinary reconnects indefinitely, and the latest PR diff only caps clients that explicitly pass maxReconnectAttempts; no production caller currently does so.

Real behavior proof
Needs real behavior proof before merge: The external PR body has no after-fix terminal output, logs, screenshots, recordings, or linked artifacts showing the changed reconnect behavior in a real setup.

Next step before merge
Contributor action and maintainer policy are both needed: the PR lacks real behavior proof and the production retry policy is not safely inferable for automation.

Security
Cleared: The diff changes in-process reconnect counters/options and tests only; it adds no dependencies, permissions, secret handling, downloads, or code-execution surface.

Review findings

[P2] Wire the reconnect limit to a real caller — src/gateway/client.ts:164

Review details

Best possible solution:

Define a caller-aware reconnect policy, wire it to the intended production surface with regression coverage, preserve node-host recovery unless maintainers choose otherwise, and document any new client option or terminal reconnect behavior.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection shows current main can keep scheduling ordinary reconnects indefinitely, and the latest PR diff only caps clients that explicitly pass maxReconnectAttempts; no production caller currently does so.

Is this the best way to solve the issue?

No. The opt-in cap avoids the prior node-host regression, but it does not solve the reported production behavior; the narrower maintainable path is an explicit caller-specific policy or threshold callback chosen by maintainers.

Full review comments:

[P2] Wire the reconnect limit to a real caller — src/gateway/client.ts:164
After the latest force-push, maxReconnectAttempts is only an optional GatewayClient option and the only non-test behavior changes when a caller supplies it. All existing production constructors, including node-host, omit it, so merging this would still leave the linked unbounded reconnect path unchanged while closing [Bug P2] scheduleReconnect() has no max retry limit — infinite reconnect loop #45469. Either set a caller-specific policy for the intended surface or keep this as a non-closing preparatory API change.
Confidence: 0.88

Overall correctness: patch is incorrect
Overall confidence: 0.88

Acceptance criteria:

pnpm test src/gateway/client.test.ts src/gateway/client.watchdog.test.ts
pnpm test src/tui/gateway-chat.test.ts src/tui/tui.test.ts
pnpm check:changed

What I checked:

Current main reconnects ordinary closes without a retry ceiling: The close handler still calls scheduleReconnect() for ordinary closes, and scheduleReconnect() only applies backoff capped at 30 seconds before calling start() again. (src/gateway/client.ts:391, b31774749cc0)
Latest PR diff makes the cap opt-in: The latest patch adds maxReconnectAttempts?: number and only stops reconnecting when that option is non-null. (src/gateway/client.ts:160, b3385ff8c15c)
Production call sites do not pass the new option: Search found current GatewayClient constructors in node-host, TUI, SDK, ACP, MCP, probes, tests, and extensions, but maxReconnectAttempts is only present in unrelated realtime transcription/SSE clients on current main and in the PR tests. (src/node-host/runner.ts:222, b31774749cc0)
Node-host recovery is intentionally unbounded today: runNodeHost comments that connect errors should keep retrying because GatewayClient handles retrying, and its reconnect-paused handler exits only for auth detail codes. (src/node-host/runner.ts:255, b31774749cc0)
Protocol docs list backoff but no retry-attempt maximum: The gateway protocol docs document initial reconnect backoff and max reconnect backoff as reference client constants, with no maximum reconnect-attempt limit. Public docs: docs/gateway/protocol.md. (docs/gateway/protocol.md:569, b31774749cc0)
Related issue discussion flagged policy ambiguity: A related issue comment notes node clients may intentionally retry forever for gateway recovery and suggests listener cleanup, warning/reset behavior, or an event after N failures instead of a blanket stop.

Likely related people:

steipete: Recent commits changed GatewayClient startup/retry handling, stale-token reconnect-loop handling, and node-host gateway restart recovery on the same reconnect surface. (role: recent maintainer and adjacent owner; confidence: high; commits: 4ee537a04ad0, 885806d5ca8d, 8d58ad4c15cd; files: src/gateway/client.ts, src/gateway/client.test.ts, src/node-host/runner.ts)
joshavant: Merged history includes shared auth resolution across node-host plus token fallback/reconnect behavior and docs adjacent to the retry-policy decision. (role: recent reconnect/auth maintainer; confidence: medium; commits: a76e81019333, 25252ab5ab15; files: src/gateway/client.ts, src/gateway/client.test.ts, src/node-host/runner.ts)
Takhoffman: Adjacent GatewayClient reliability work bounded unanswered client requests and touched the same client/watchdog area related to listener and timeout cleanup. (role: adjacent reliability owner; confidence: medium; commits: 5fc43ff0ecd8; files: src/gateway/client.ts, src/gateway/client.watchdog.test.ts)

Remaining risk / open question:

Merging as-is would close [Bug P2] scheduleReconnect() has no max retry limit — infinite reconnect loop #45469 without changing any existing production caller's retry behavior.
The PR has no after-fix evidence from a real gateway or node-host setup.
The intended reconnect policy still needs maintainer judgment because long-lived node-host recovery conflicts with a hard stop.

Codex review notes: model gpt-5.5, reasoning high; reviewed against b31774749cc0.

…45469) scheduleReconnect() retried indefinitely with exponential backoff capped at 30s but no maximum retry count. If the gateway is unreachable, the client loops forever accumulating listeners and zombie processes. Changes: - Add MAX_RECONNECT_ATTEMPTS (30) as a static class constant - scheduleReconnect() now gives up after 30 failed attempts, fires onReconnectPaused with a descriptive reason, and sets closed = true - reconnectAttempts resets to 0 on successful hello handshake - reconnectAttempts resets to 0 on explicit stop() - Two new tests: max-reconnect-gives-up and counter-resets-on-success Closes openclaw#45469

TypeScript strict null checks require string | null, not undefined.

Address ClawSweeper review feedback: - P2: Make reconnect cap opt-in via maxReconnectAttempts option. Undefined = unlimited, preserving node-host unbounded retry behavior. - P3: Fix reset test to send proper type:"res" with payload.type:"hello-ok" instead of type:"connect" so handleMessage actually processes it.

stellamariesays · 2026-05-05T17:15:54Z

Closing in favor of a new PR. Fixed review feedback: made reconnect cap opt-in via maxReconnectAttempts option.

openclaw-barnacle Bot added gateway Gateway runtime size: S triage: blank-template Candidate: PR template appears mostly untouched. labels May 4, 2026

stellamariesays force-pushed the fix/max-reconnect-limit branch from 26881f7 to e2c4778 Compare May 4, 2026 22:17

stellamariesays added 2 commits May 4, 2026 22:45

fix(gateway): use null instead of undefined for detailCode type

71c3132

TypeScript strict null checks require string | null, not undefined.

stellamariesays force-pushed the fix/max-reconnect-limit branch from 8913ad3 to 71c3132 Compare May 4, 2026 22:45

openclaw-barnacle Bot added the triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. label May 5, 2026

stellamariesays closed this May 5, 2026

This was referenced Jun 18, 2026

[Bug P2] scheduleReconnect() has no max retry limit — infinite reconnect loop #45469

Open

fix(gateway): add max reconnect limit with opt-in maxReconnectAttempts #77961

Open

yetval mentioned this pull request Jul 8, 2026

realtime-transcription: reconnect give-up cap and exponential backoff never trigger on a flapping-after-ready websocket #102251

Closed

clawsweeper Bot mentioned this pull request Jul 8, 2026

fix(realtime-transcription): preserve reconnect backoff attempts #102285

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(gateway): add max reconnect limit to scheduleReconnect (#45469)#77556

fix(gateway): add max reconnect limit to scheduleReconnect (#45469)#77556
stellamariesays wants to merge 3 commits into
openclaw:mainfrom
stellamariesays:fix/max-reconnect-limit

stellamariesays commented May 4, 2026

Uh oh!

clawsweeper Bot commented May 4, 2026 •

edited

Loading

Uh oh!

stellamariesays commented May 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

stellamariesays commented May 4, 2026

Summary

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Root Cause (if applicable)

Regression Test Plan (if applicable)

User-visible / Behavior Changes

Diagram (if applicable)

Security Impact (required)

Repro + Verification

Environment

Steps

Expected

Actual

Evidence

Human Verification (required)

Review Conversations

Compatibility / Migration

Risks and Mitigations

Uh oh!

clawsweeper Bot commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stellamariesays commented May 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

clawsweeper Bot commented May 4, 2026 •

edited

Loading