Lifecycle hardening for IRC client behavior

[louzt]

Summary

This PR has grown beyond the original disconnect-only hardening and now prepares ObsidianIRC's transport stack for a future native backend without changing the current default behavior.

In practical terms, this PR:

• hardens intentional disconnect vs unexpected close handling in IRCClient
• makes the frontend socket lifecycle more idempotent in src/lib/socket.ts
• makes the Tauri Rust bridge shutdown/removal path idempotent in src-tauri/src/socket.rs
• extracts an explicit transport seam so future native transport work can plug in below the current IRC logic
• updates architecture docs so they match the real web/native transport split

Why

ObsidianIRC already has a split transport model:

• browser-compatible paths use wss://
• desktop/native paths use irc:// / ircs:// through the Tauri socket bridge

The previous code still mixed transport lifecycle concerns across layers, which made a few things harder than they should be:

• distinguishing intentional disconnects from connection loss
• keeping close/shutdown behavior idempotent
• testing transport edges in isolation
• preparing a future native transport backend without rewriting IRCClient

This PR fixes those lifecycle edges first, then opens a seam for the next step.

What Changed

1. IRCClient lifecycle hardening

In src/lib/irc/IRCClient.ts:

• intentional disconnects are tracked explicitly
• manual disconnects no longer trigger unwanted reconnection attempts
• sockets already in CLOSING are still treated as intentional disconnects
• disconnect behavior is safer during connection initialization
• transport target resolution now comes from the socket layer instead of being assembled inline in IRCClient

2. Frontend socket seam + lifecycle hardening

In src/lib/socket.ts:

• TCPSocket close handling is now idempotent
• closing during CONNECTING no longer results in a late onopen
• duplicate close handling is suppressed when backend close signals arrive late
• routing/factory logic is now explicit through:
  • resolveSocketProtocol()
  • resolveSocketTarget()
  • SocketFactory
  • setSocketFactory() / resetSocketFactory()

This keeps the current routing intact while making future transport injection cleaner:

• wss:// -> WebSocketWrapper
• irc:// / ircs:// -> TCPSocket

3. Tauri Rust bridge hardening

In src-tauri/src/socket.rs:

• connection removal is centralized through take_connection()
• late read/shutdown paths do not re-handle already-removed connections
• disconnect() is now idempotent from the bridge's point of view
• added a focused Rust test for idempotent connection removal

4. Architecture documentation

In ARCHITECTURE.md:

• corrected the old "websockets only" framing
• documented the real split between web websocket transport and native Tauri TCP/TLS transport
• clarified where future transport work belongs

Transport Layout

flowchart LR
  A[IRCClient] --> B[resolveSocketTarget in socket.ts]
  B --> C[createSocket / SocketFactory]
  C --> D[WebSocketWrapper]
  C --> E[TCPSocket]
  E --> F[Tauri invoke bridge]
  F --> G[src-tauri/src/socket.rs]
  D --> H[(IRC/WebSocket endpoint)]
  G --> I[(IRC TCP/TLS endpoint)]
  J[Future native transport backend PR] -. plugs in behind seam .-> C

Loading

Tests / Validation

TypeScript / frontend transport slice

• npm run test -- tests/lib/ircClient.test.ts tests/lib/socket.test.ts
• npm run build

Rust bridge slice

• cd src-tauri && cargo test

Focused coverage added in this PR

• intentional disconnect regression coverage in tests/lib/ircClient.test.ts
• transport seam and lifecycle coverage in tests/lib/socket.test.ts
• Rust idempotency coverage in src-tauri/src/socket.rs

Scope Boundary

This PR does not introduce a new native transport backend yet.

Instead, it does the prep work needed so the next branch/PR can implement one more safely:

• lifecycle semantics are more stable
• transport routing is more explicit
• frontend and Rust bridge responsibilities are clearer
• tests exist around the edges most likely to regress

Follow-Up

The natural next branch after this one is a focused native transport backend experiment that plugs in behind the seam added here, rather than changing IRCClient protocol logic again.

That follow-up should be able to build directly on:

• resolveSocketTarget()
• SocketFactory
• the TCPSocket/bridge lifecycle hardening
• the idempotent Rust-side connection removal path

[coderabbitai]

📝 Walkthrough

Walkthrough

The changes add per-server intentional-disconnect tracking to IRCClient. When a server disconnects intentionally, the client clears the flag on reconnection and prevents reconnection logic after intentional socket closure. The disconnect method is updated to conditionally send QUIT and mark servers as intentional before closing. Tests verify disconnect behavior during connection establishment and after active connection.

Changes

Cohort / File(s)	Summary
Intentional Disconnect Tracking src/lib/irc/IRCClient.ts	Added intentionalDisconnects map to track per-server intentional disconnections. Modified connect and socket onopen to clear the flag, updated socket onclose handler to skip reconnection when marked intentional, revised disconnect to send QUIT conditionally and mark servers intentional, and updated removeServer to clean up the flag.
Disconnect Behavior Tests tests/lib/ircClient.test.ts	Added two test cases: one verifying no QUIT is sent when disconnecting during WebSocket.CONNECTING state, and another confirming intentional disconnection suppresses reconnection scheduling and emits proper state transitions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A disconnect most true and tried,
No secret threads we try to hide—
We mark the servers we depart,
And silence all the reconnect's start,
Intentional hops, clean and bright! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Lifecycle hardening for IRC client behavior' directly reflects the PR's main objective of hardening IRC client lifecycle handling, as confirmed by the PR description and the changes to intentional disconnection tracking and socket lifecycle management.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

src/lib/irc/IRCClient.ts (2)

657-663: Clear intentionalDisconnects inside the early-return branch.

When onclose takes the intentional path, the flag is never removed — it's only cleared later by a subsequent connect(serverId) or removeServer(serverId). If neither call ever happens for that serverId, the entry lingers in the set. Also, if the server later re-enters a normal connect flow through a different path, a stale flag could short-circuit a legitimate onclose. Cheap to fix:

♻️ Proposed fix

         socket.onclose = () => {
           if (this.intentionalDisconnects.has(server.id)) {
+            this.intentionalDisconnects.delete(server.id);
             this.stopWebSocketPing(server.id);
             this.sockets.delete(server.id);
             this.pendingConnections.delete(connectionKey);
             return;
           }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/irc/IRCClient.ts` around lines 657 - 663, The onclose handler's
early-return path uses the intentionalDisconnects set but never removes the
server id, causing stale flags; update the onclose branch that checks
this.intentionalDisconnects.has(server.id) to also
this.intentionalDisconnects.delete(server.id) before calling
this.stopWebSocketPing(server.id), this.sockets.delete(server.id), and
this.pendingConnections.delete(connectionKey) so the intentional flag is cleared
immediately and cannot short-circuit future connects; reference the onclose
handler, intentionalDisconnects, stopWebSocketPing, sockets.delete,
pendingConnections.delete, connectionKey, and server.id when locating the
change.

---

620-622: Redundant clear — line 496 already covers reconnection; new connections get a fresh UUID.

When serverId is provided (reconnection), the flag was already cleared at lines 495–497 before the pending-connection check. When serverId is not provided, server.id is a freshly minted UUID at line 582, so the set can't contain it. This line is effectively dead code. Safe to drop, or at minimum collapse both clears into a single location after the server object exists.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/irc/IRCClient.ts` around lines 620 - 622, The call to
this.intentionalDisconnects.delete(server.id) is redundant because when serverId
is provided the flag is already cleared earlier via
this.intentionalDisconnects.delete(serverId), and when serverId is absent
server.id is a new UUID that can't be present; remove the extra delete or
collapse both clears into one spot after the Server object exists (use serverId
if present else server.id) to ensure intentionalDisconnects is cleared exactly
once; update the block that sets this.nicks.set(server.id, nickname) accordingly
so the delete occurs immediately before or after setting the nick to keep logic
grouped.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/lib/irc/IRCClient.ts`:
- Around line 730-748: The disconnect() logic fails to mark intent when a socket
exists but is CLOSING, allowing onclose to treat it as unintentional; update
disconnect() so that as soon as a socket object is present you add serverId to
this.intentionalDisconnects unconditionally, while still only sending the QUIT
and calling socket.close() when socket.readyState is CONNECTING or OPEN; ensure
you keep the existing checks around socket.send(`QUIT ...`) and socket.close()
but move the intentionalDisconnects.add(serverId) to run whenever socket is
truthy, then continue to this.sockets.delete(serverId) as before so onclose will
not trigger reconnection.

---

Nitpick comments:
In `@src/lib/irc/IRCClient.ts`:
- Around line 657-663: The onclose handler's early-return path uses the
intentionalDisconnects set but never removes the server id, causing stale flags;
update the onclose branch that checks this.intentionalDisconnects.has(server.id)
to also this.intentionalDisconnects.delete(server.id) before calling
this.stopWebSocketPing(server.id), this.sockets.delete(server.id), and
this.pendingConnections.delete(connectionKey) so the intentional flag is cleared
immediately and cannot short-circuit future connects; reference the onclose
handler, intentionalDisconnects, stopWebSocketPing, sockets.delete,
pendingConnections.delete, connectionKey, and server.id when locating the
change.
- Around line 620-622: The call to this.intentionalDisconnects.delete(server.id)
is redundant because when serverId is provided the flag is already cleared
earlier via this.intentionalDisconnects.delete(serverId), and when serverId is
absent server.id is a new UUID that can't be present; remove the extra delete or
collapse both clears into one spot after the Server object exists (use serverId
if present else server.id) to ensure intentionalDisconnects is cleared exactly
once; update the block that sets this.nicks.set(server.id, nickname) accordingly
so the delete occurs immediately before or after setting the nick to keep logic
grouped.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 81ff2dfd-e4a1-45c9-92e2-4d79c629970a

📥 Commits

Reviewing files that changed from the base of the PR and between e7d7899 and 753542d.

📒 Files selected for processing (2)

• src/lib/irc/IRCClient.ts
• tests/lib/ircClient.test.ts