fix(irc): monitor stays disconnected after IRC socket closes#100799
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 6, 2026, 7:46 AM ET / 11:46 UTC. Summary PR surface: Source +74, Tests +245. Total +319 across 4 files. Reproducibility: yes. Source inspection shows current main has no post-ready close notification from the IRC client to the monitor, and the PR proof/test path exercises a ready close followed by reconnect and channel rejoin. Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the plugin-local IRC reconnect fix after required CI is clean because the proof now covers the requested real reconnect behavior. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection shows current main has no post-ready close notification from the IRC client to the monitor, and the PR proof/test path exercises a ready close followed by reconnect and channel rejoin. Is this the best way to solve the issue? Yes. The IRC client is the right layer to report transport disconnects, while the IRC monitor is the right layer to reconnect using account config and joined channels. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 90e1ef176820. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +74, Tests +245. Total +319 across 4 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (1 earlier review cycle)
|
8232574 to
117e2f3
Compare
117e2f3 to
b14404c
Compare
b14404c to
a3e376f
Compare
|
maintainer validation complete; this is ready to land.
behavior proof covers an established IRC connection being closed by the server, followed by reconnect, registration, and rejoin. explicit stop/abort cancels pending reconnects, registration timeout destroys the incomplete socket, and initial connection failure remains fail-closed. known proof gaps: none for this bounded channel lifecycle change. |
|
Merged via squash.
|
What Problem This Solves
Fixes an issue where IRC channel users would stop receiving bot responses after the IRC server closed an already-established monitor connection. The monitor logged the socket error/close but did not establish a new IRC session, so inbound IRC messages were missed until the OpenClaw process was restarted.
Why This Change Was Made
The IRC client now reports unexpected post-ready socket closes to the monitor, and the monitor schedules a reconnect that reuses the same account config and channel join list. The client also closes sockets when IRC registration times out, so repeated reconnect attempts do not leave accepted-but-not-ready sockets behind. This does not change IRC config, auth, message parsing, or outbound send contracts.
User Impact
IRC channel monitors recover from transient network/server disconnects without requiring a manual OpenClaw restart. After reconnecting, the bot logs in again and rejoins its configured channels so inbound messages can be handled normally.
Evidence
Near-real IRC runtime proof uses the real
monitorIrcProvideragainst a real loopback TCP IRC server. The server accepts a ready IRC session, observesJOIN #openclaw, closes that established socket, and then observes the monitor reconnect and rejoin the configured channel.Focused regression checks:
node scripts/run-vitest.mjs run --config test/vitest/vitest.extension-irc.config.ts extensions/irc/src/monitor.test.tsnode scripts/run-vitest.mjs run --config test/vitest/vitest.extension-irc.config.ts extensions/irc/src/client.test.ts -t "closes the socket when registration never becomes ready"After syncing with current
main, the previously failing unrelated CI areas were rechecked locally:pnpm tsgo:prodpnpm lint --threads=2node scripts/run-vitest.mjs run test/scripts/android-app-i18n.test.ts