From 71d9ecb6bc25a31082987eb52a783d9b3b4a4bda Mon Sep 17 00:00:00 2001
From: YellowSnnowmann <167776381+YellowSnnowmann@users.noreply.github.com>
Date: Fri, 29 May 2026 04:49:44 +0530
Subject: [PATCH] fix(socket): recover from stale disconnected socket to
 prevent permanent "Connecting..." (#2487)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

## Summary

- Fixed a reconnect edge case where socketService.connect() could get stuck when a stale disconnected socket instance existed for the same auth token.
- Prevented false-positive connecting UI state by clearing stale disconnected socket references before async reconnect guards run.
- Preserved existing safety behavior for active sockets (connected) and in-flight sockets (!disconnected) to avoid duplicate connections.
- Added a regression unit test to verify same-token reconnect creates a fresh socket instead of silently no-oping.

## Problem

- Users could be stuck on Connecting... and unable to chat after a disconnect/reconnect cycle.
- Root cause: reconnect logic set state to connecting, but returned early because this.socket was still non-null (stale/disconnected), so no new socket was created and no connect/connect_error transition fired.
- This left connection state stranded and blocked chat flows.

## Solution

- In socketService.connectAsync, when token is unchanged and this.socket.disconnected === true, explicitly clear stale runtime references (this.socket, this.mcpTransport) before continuing.
- Keep existing early returns for:
  - same-token + already connected
  - same-token + currently connecting (!disconnected)
- Added test coverage for the stale-socket scenario: second same-token connect() now creates a new socket instance (verifies io(...) called twice).

## Submission Checklist

- If a section does not apply to this change, mark the item as N/A with a one-line reason. Do not delete items.
- Tests added or updated (happy path + at least one failure / edge case) per Testing Strategy
- Diff coverage ≥ 80% — changed lines (Vitest + cargo-llvm-cov merged via diff-cover) meet the gate enforced by `.github/workflows/coverage.yml`. Run pnpm test:coverage and pnpm test:rust locally; PRs below 80% on changed lines will not merge.
- Coverage matrix updated — added/removed/renamed feature rows in `docs/TEST-COVERAGE-MATRIX.md` reflect this change (or N/A: behaviour-only change)
- All affected feature IDs from the matrix are listed in the PR description under ## Related
- No new external network dependencies introduced (mock backend used per Testing Strategy)
- Manual smoke checklist updated if this touches release-cut surfaces (`docs/RELEASE-MANUAL-SMOKE.md`)
- Linked issue closed via Closes #NNN in the ## Related section

## Impact

- Runtime/platform impact: frontend connection management path (app/src/services/socketService.ts) affecting desktop/web UI behavior where this service is used.
- Compatibility: no API/schema changes.
- Performance: negligible; only additional stale-reference cleanup in reconnect edge case.
- Security: no new credential surface or transport changes.


Co-authored-by: M3gA-Mind <megamind@mahadao.com>
---
 .../services/__tests__/socketService.test.ts  | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/app/src/services/__tests__/socketService.test.ts b/app/src/services/__tests__/socketService.test.ts
index 076257aa3..60d812808 100644
--- a/app/src/services/__tests__/socketService.test.ts
+++ b/app/src/services/__tests__/socketService.test.ts
@@ -275,6 +275,25 @@ describe('socketService — resolveCoreSocketBaseUrl uses getCoreRpcUrl', () =>
     );
     expect(latestSocket.once).not.toHaveBeenCalledWith('queued-on-event', expect.any(Function));
   });
+
+  it('reconnects when a stale disconnected socket exists for the same token', async () => {
+    const { io } = await import('socket.io-client');
+    const ioMock = vi.mocked(io);
+    ioMock.mockClear();
+
+    hoisted.getCoreRpcUrlMock.mockResolvedValue('http://127.0.0.1:7788/rpc');
+
+    const { socketService } = await import('../socketService');
+    socketService.disconnect();
+
+    socketService.connect('mock-jwt-stale-socket');
+    await pollUntil(() => expect(ioMock).toHaveBeenCalledTimes(1));
+
+    // Same token, previous socket is disconnected=true in our mock.
+    // We should still create a fresh socket instead of returning early.
+    socketService.connect('mock-jwt-stale-socket');
+    await pollUntil(() => expect(ioMock).toHaveBeenCalledTimes(2));
+  });
 });
 
 describe('socketService — connectivity dispatch on socket events (lines 164, 212, 230, 237, 240)', () => {