fix(server): report idle sessions as not-running on reconnect

[dimakis]

Summary

• Root cause: handleReconnect used isActive() (session exists in registry) to determine running state, but this returns true for sessions whose query loop is alive but idle between turns. The frontend interprets running=true as "agent is busy" and queues messages behind a 5-second fallback timer instead of sending immediately — causing the persistent "two messages needed after reattach" bug.
• Fix: After the existing isActive/storeMeta.isActive checks, also check lastSpeaker from EventStore. lastSpeaker=assistant means the agent completed its turn (idle) → running=false. lastSpeaker=user means the agent hasn't answered yet → running=true.
• Adds 2 test cases covering idle (lastSpeaker=assistant) and mid-turn (lastSpeaker=user) scenarios. All 136 tests pass.

Context

This bug has been the target of 13+ PRs (#320, #348, #353, #360, #362, #367, #368, #371, #372, #381, #384, #386, #392) fixing symptoms at different layers (SSE race conditions, iOS WS stale readyState, idempotency, state machine infra). None addressed the semantic mismatch: "query loop alive" ≠ "agent actively processing." The 5-second PENDING_SEND_TIMEOUT_MS timer in store.ts:355 is literally a band-aid for this — its comment says "stale running state after reconnect."

Test plan

• All 136 ws-handler-v2 tests pass (134 existing + 2 new)
• Manual: close app mid-session, reopen, send message — should respond immediately without needing a second "nudge" message
• Verify agent mid-turn reconnect still shows running state correctly

🤖 Generated with Claude Code

[dimakis]

Centaur Review

Found 3 issue(s) (1 warning).

server/ws-handler-v2.ts

Core fix is correct and well-tested, but the same idle-session running-state logic is missing from handleSwitchSession, which sends the same running field to the same client queue mechanism — leaving the 5-second delay bug reachable via the switch_session path.

• 🟡 regressions: handleSwitchSession (line 429) computes running without the new lastSpeaker idle check. Both reconnected and session_switched drive SET_RUNNING in the client store, which controls whether sendMessage queues behind a 5-second fallback timer. An idle session (lastSpeaker=assistant) will be reported as running=false on reconnect but running=true on switch_session, causing the same queuing delay this PR fixes — just via a different code path. [fixable]

server/__tests__/ws-handler-v2.test.ts

Core fix is correct and well-tested, but the same idle-session running-state logic is missing from handleSwitchSession, which sends the same running field to the same client queue mechanism — leaving the 5-second delay bug reachable via the switch_session path.

• 🔵 missing_tests (L1148): The new idle-session tests don't verify that reattachChat is NOT called when running is downgraded to false by the lastSpeaker check. The zombie test (line 1117) explicitly asserts expect(reattachChat).not.toHaveBeenCalled(), but the new idle tests skip this. Since the code at line 284 gates reattach on running, and running is now false for idle sessions, it would be good to confirm the reattach is correctly skipped — especially since the session is still alive (unlike zombies where it's removed). [fixable]
• 🔵 missing_tests (L1148): No test covers the case where storeMeta exists, isActive is true, but lastSpeaker is null (brand-new session that hasn't had any turns yet). The implementation handles it correctly (null !== 'assistant' → running stays true), but an explicit test would document this edge case and guard against future regressions. [fixable]

[dimakis]

🔵 missing_tests: The new idle-session tests don't verify that reattachChat is NOT called when running is downgraded to false by the lastSpeaker check. The zombie test (line 1117) explicitly asserts expect(reattachChat).not.toHaveBeenCalled(), but the new idle tests skip this. Since the code at line 284 gates reattach on running, and running is now false for idle sessions, it would be good to confirm the reattach is correctly skipped — especially since the session is still alive (unlike zombies where it's removed). [fixable]

[dimakis]

🔵 missing_tests: No test covers the case where storeMeta exists, isActive is true, but lastSpeaker is null (brand-new session that hasn't had any turns yet). The implementation handles it correctly (null !== 'assistant' → running stays true), but an explicit test would document this edge case and guard against future regressions. [fixable]

[dimakis]

Centaur Review

Found 5 issue(s) (2 critical) (2 warning).

server/ws-handler-v2.ts

Critical gap: the running field is computed but never added to the reconnected summary — all three new tests will fail, and the existing 'no running field' test contradicts the PR's intent. The server-side reattach skip works correctly but the client parser ignores the summary payload, so the stated goal (avoiding the 5s fallback timer) requires client-side changes too.

• 🔴 bugs (L321): The running field is never added to the reconnected summary. The summaries array (line 233) is typed Array<{ sessionId: string; replayed: number }> and pushed to at line 321-324 with only sessionId and replayed. The new code at lines 270-281 modifies the local running variable, but this value is discarded — it's never included in the summary sent to the client. All three new tests assert summary.sessions[0].running which will be undefined, causing test failures. The summary push needs running added: summaries.push({ sessionId: ..., replayed: ..., running }). [fixable]
• 🟡 unsafe_assumptions (L270): The comment says the goal is to make 'the client send messages immediately instead of queuing them behind a 5-second fallback timer.' However, the client's protocol parser (packages/client/src/protocol-parser.ts:129-134) explicitly ignores the reconnected payload — running state is derived from replayed session_state_changed events. Even after adding running to the summary, the client-side parser and store would need changes to consume it. Without that, idle sessions that replayed a session_state_changed with state: 'running' but no subsequent state: 'idle' transition will still appear running to the client. [fixable]
• 🔵 style (L233): The summaries type annotation Array<{ sessionId: string; replayed: number }> should be updated to include running: boolean once the field is added. Consider extracting an interface (e.g., ReconnectSessionSummary) since it's now part of the client-facing contract. [fixable]

server/__tests__/ws-handler-v2.test.ts

Critical gap: the running field is computed but never added to the reconnected summary — all three new tests will fail, and the existing 'no running field' test contradicts the PR's intent. The server-side reattach skip works correctly but the client parser ignores the summary payload, so the stated goal (avoiding the 5s fallback timer) requires client-side changes too.

• 🔴 regressions (L1064): Existing test 'reconnected summary does not include running field' (line 1064-1087) explicitly asserts expect(summary.sessions[0]).not.toHaveProperty('running'). If running is added to the summary (as the new tests require), this existing test will break. It needs to be updated or removed to reflect the new contract. [fixable]
• 🟡 missing_tests: The server-side behavioral change — skipping the reattach at line 270 for idle sessions — is not directly tested. The three new tests only check the summary payload. A test should verify that reattachChat is NOT called when lastSpeaker === 'assistant' (idle session) and IS called when lastSpeaker === 'user' (mid-turn session), since skipping reattach is the primary server-side effect of this change. [fixable]

[dimakis]

Centaur Review

Found 5 issue(s) (2 critical) (1 warning).

server/ws-handler-v2.ts

Implementation is incomplete: running is never added to the reconnected summary (all new tests will fail), and mutating the running variable in-place prevents reattach and suspend-resume for idle sessions — use a separate variable for the reported state.

• 🔴 bugs (L321): The running value is computed and mutated but never included in the summaries.push() at line 321 — it only pushes { sessionId, replayed }. The summaries type (line 233) is Array<{ sessionId: string; replayed: number }> with no running field. All three new tests expect summary.sessions[N].running to be true or false, but the property is never set, so those assertions will fail (undefined is neither true nor false). The implementation is incomplete — summaries.push and its type need to include running. [fixable]
• 🔴 regressions (L270): Setting running = false for idle sessions (lastSpeaker=assistant) at line ~275 prevents the reattach block at line 270 (if (found && running && !isAttached)) and the suspend-resume block at line 303 (if (found && running && isSuspended)) from executing. An idle but detached session that reconnects will NOT be reattached to the new WebSocket transport, leaving the user unable to send messages to it. The fix should use a separate variable (e.g. reportedRunning) for the summary, or move the lastSpeaker check to after the reattach/resume logic but before summaries.push(). [fixable]

server/__tests__/ws-handler-v2.test.ts

Implementation is incomplete: running is never added to the reconnected summary (all new tests will fail), and mutating the running variable in-place prevents reattach and suspend-resume for idle sessions — use a separate variable for the reported state.

• 🟡 regressions (L1064): The existing test at line 1064 ('reconnected summary does not include running field') explicitly asserts not.toHaveProperty('running'). Once the implementation is fixed to include running in the summary, this existing test will break and needs to be updated to expect running: true (since it sets isActive to true and getSession returns null, so the lastSpeaker check won't fire). [fixable]
• 🔵 missing_tests: No test covers the case where eventStore.getSession() returns null for a running session (e.g. session exists in registry but not yet in event store). The ?. optional chaining handles it safely, but a test would document the expected behavior: running should stay true when getSession returns null. [fixable]
• 🔵 missing_tests: No test verifies that an idle session (lastSpeaker=assistant) is still reattached on reconnect despite being reported as not-running. Once the reattach regression is fixed, a test should confirm that reattach/rekey still happens for idle detached sessions. [fixable]

[dimakis]

Centaur Review

Found 5 issue(s) (2 critical) (2 warning).

server/ws-handler-v2.ts

The running field is never added to the reconnected summary sent to clients — all three new tests will fail. Additionally, the client-side parser ignores this field, so even with a fix, client-side changes are needed to achieve the stated goal.

• 🔴 bugs (L267): The running field is never added to the reconnected summary message. The summaries array (line 233) is typed Array<{ sessionId: string; replayed: number }> and the summaries.push() at line 321 only pushes sessionId and replayed. The new code modifies a local running variable but never propagates it to the client-facing message. All three new tests will fail because summary.sessions[0].running is undefined, not true/false. The summaries.push() must include running for this fix to work. [fixable]
• 🟡 regressions (L278): Setting running = false for idle sessions prevents the reattach block (line 270: if (found && running && ...)) from executing on reconnect. The session stays detached and on the 10-minute TTL abort timer. The reattach only happens lazily on the next handleSendV2 call. If the user reconnects but doesn't send a message quickly, the session could be aborted. Consider whether idle sessions should still be reattached on reconnect even if reported as not-running. [fixable]
• 🟡 bugs (L267): Even if running were added to the summary, the client-side protocol parser (packages/client/src/protocol-parser.ts:129-131) explicitly ignores it: 'Running state derived from replayed session_state_changed events, not from the reconnected message payload.' A client-side test also confirms this ('reconnected does not dispatch SET_RUNNING'). The stated goal (client sends messages immediately instead of queuing behind a 5-second timer) won't be achieved without client-side changes to consume the running field. [fixable]

server/__tests__/ws-handler-v2.test.ts

The running field is never added to the reconnected summary sent to clients — all three new tests will fail. Additionally, the client-side parser ignores this field, so even with a fix, client-side changes are needed to achieve the stated goal.

• 🔴 regressions (L1114): The existing test at line 1064 ('reconnected summary does not include running field') explicitly asserts expect(summary.sessions[0]).not.toHaveProperty('running'). The three new tests directly contradict this by asserting running IS present with specific boolean values. One of these test groups must be updated — either the existing test is removed (if running is being added) or the new tests are wrong. [fixable]
• 🔵 missing_tests (L1114): No test verifies the behavioral side-effect: that idle sessions (lastSpeaker=assistant) skip the reattach logic on reconnect. The current tests only check the summary payload. A test should verify that reattachChat is NOT called for idle sessions even when the session is detached. [fixable]

[dimakis]

🔴 bugs: The running field is never added to the reconnected summary message. The summaries array (line 233) is typed Array<{ sessionId: string; replayed: number }> and the summaries.push() at line 321 only pushes sessionId and replayed. The new code modifies a local running variable but never propagates it to the client-facing message. All three new tests will fail because summary.sessions[0].running is undefined, not true/false. The summaries.push() must include running for this fix to work. [fixable]

[dimakis]

🟡 regressions: Setting running = false for idle sessions prevents the reattach block (line 270: if (found && running && ...)) from executing on reconnect. The session stays detached and on the 10-minute TTL abort timer. The reattach only happens lazily on the next handleSendV2 call. If the user reconnects but doesn't send a message quickly, the session could be aborted. Consider whether idle sessions should still be reattached on reconnect even if reported as not-running. [fixable]

[dimakis]

🟡 bugs: Even if running were added to the summary, the client-side protocol parser (packages/client/src/protocol-parser.ts:129-131) explicitly ignores it: 'Running state derived from replayed session_state_changed events, not from the reconnected message payload.' A client-side test also confirms this ('reconnected does not dispatch SET_RUNNING'). The stated goal (client sends messages immediately instead of queuing behind a 5-second timer) won't be achieved without client-side changes to consume the running field. [fixable]

[dimakis]

🔴 regressions: The existing test at line 1064 ('reconnected summary does not include running field') explicitly asserts expect(summary.sessions[0]).not.toHaveProperty('running'). The three new tests directly contradict this by asserting running IS present with specific boolean values. One of these test groups must be updated — either the existing test is removed (if running is being added) or the new tests are wrong. [fixable]

[dimakis]

🔵 missing_tests: No test verifies the behavioral side-effect: that idle sessions (lastSpeaker=assistant) skip the reattach logic on reconnect. The current tests only check the summary payload. A test should verify that reattachChat is NOT called for idle sessions even when the session is detached. [fixable]