fix(gateway): add max reconnect limit with opt-in maxReconnectAttempts#77961
fix(gateway): add max reconnect limit with opt-in maxReconnectAttempts#77961stellamariesays wants to merge 5 commits into
Conversation
…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.
openclaw-barnacle
Bot
added
triage: needs-real-behavior-proof
|
Thanks for the context here. I swept through the related work, and this is now duplicate or superseded. Keep open: this is a useful linked fix candidate, but current main moved the active reconnect scheduler into the gateway-client package, while this branch is conflicting and still patches the stale facade-era implementation with no production caller opt-in. Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Canonical path: Close this stale PR. The latest review rated it F, the branch still lacks merge-ready proof, and there has been no human follow-up after the durable review. So I’m closing this here because the remaining work is already tracked in the canonical issue. Review detailsBest possible solution: Close this stale PR. The latest review rated it F, the branch still lacks merge-ready proof, and there has been no human follow-up after the durable review. Do we have a high-confidence way to reproduce the issue? Yes at source level: current package-client close handling reaches Is this the best way to solve the issue? No as submitted: the opt-in cap is a plausible direction, but this branch patches the stale facade-era implementation and does not wire any caller that should stop retrying. The maintainable fix belongs in the package client with maintainer-approved caller policy. Security review: Security review cleared: The diff changes gateway TypeScript and tests only; it adds no dependency, workflow, lockfile, package-script, credential, permission, or artifact-download surface. AGENTS.md: found and applied where relevant. What I checked:
Likely related people:
Codex review notes: model internal, reasoning high; reviewed against 010b61746379. |
|
Hi maintainers — the Real behavior proof check is failing because our evidence is vitest output, which the checker classifies as "mock only." This PR is a targeted fix to GatewayClient that adds an opt-in maxReconnectAttempts option (default: unlimited, no behavior change). The change is fully covered by unit tests and reviewed by ClawSweeper (both P2 and P3 feedback addressed). We don't have a live gateway instance on this ARM host to produce runtime logs. Could someone apply the proof: override label? Happy to provide additional evidence if needed. Thanks! |
|
This pull request has been automatically marked as stale due to inactivity. |
openclaw-barnacle
Bot
added
stale
clawsweeper
Bot
added
rating: 🧂 unranked krab
clawsweeper
Bot
added
the
merge-risk: 🚨 compatibility
clawsweeper
Bot
added
rating: 🧂 unranked krab
|
This pull request has been automatically marked as stale due to inactivity. |
|
@stellamariesays thanks for the PR. ClawSweeper is still waiting on real behavior proof before this can move forward. Useful proof can be a screenshot, short video, terminal output, copied live output, linked artifact, or redacted logs that show the changed behavior after the fix. Please redact private tokens, phone numbers, private endpoints, customer data, and anything else sensitive. Once proof is added to the PR body or a comment, ClawSweeper or a maintainer can re-check it. |
2 participants
Summary
Adds an opt-in reconnect attempt limit for GatewayClient to prevent infinite reconnect loops in short-lived clients (gateway CLI, MCP bridge, etc.).
Resolves #45469
Changes
maxReconnectAttempts?: numbertoGatewayClientOptionsscheduleReconnect()gives up after N attempts and firesonReconnectPausedwithdetailCode: nullreconnectAttemptscounter on successful hello-ok handshakeWhy opt-in
Node-host relies on GatewayClient retrying indefinitely during gateway outages. A global cap would zombie node-host processes. Making it opt-in means node-host is unchanged while CLI/bridge callers can opt in to bounded retries.
Test plan
Real behavior proof
Behavior or issue addressed: GatewayClient reconnects infinitely with no upper bound on attempts. After applying this patch, callers can opt in to a bounded reconnect limit via
maxReconnectAttemptswhile node-host retains unbounded retries.Real environment tested: ARM64 Linux (aarch64, Ubuntu), Node.js v22.22.2, running vitest against patched
src/gateway/client.tsfromfix/max-reconnect-limit-v2branch.Exact steps or command run after the patch: Ran targeted reconnect tests:
npx vitest run src/gateway/client.test.ts -t "reconnect" --reporter=verboseObserved result after fix: With
maxReconnectAttempts: 30set, the client firesonReconnectPaused({ code: -1, reason: "max reconnect attempts reached (30)", detailCode: null })after 30 failures and stops reconnecting. After a successful hello-ok handshake the counter resets, allowing another full cycle. Without the option set, reconnects continue unbounded — no behavior change.Not tested: Full end-to-end reconnect against a live gateway server (no local gateway instance available on this ARM host).