fix(client): sync running state on iOS foreground return

[dimakis]

Summary

• Fixes stale stop button when iOS Safari backgrounds during agent processing and the session_end SSE event is silently dropped
• On foreground return (_foreground event), queries /api/sessions/:id/meta and sets running: false when isActive is false
• Independent of the reconnect path, so it works regardless of doReconnectPost issues being fixed in #fix/reconnect-on-stale-post

Root cause

The _foreground handler called fetchAndRestoreMessages() which restored messages but never validated running state. When SSE session_end was lost during iOS background (EventSource appeared alive, so no reconnect triggered), running stayed true indefinitely, showing the stop/interrupt/queue buttons instead of the send button.

Test plan

• New test: syncs running state from session meta on foreground when session_end was missed
• New test: does not change running state when session is still active on foreground
• All 345 client tests pass
• Manual: background iOS Safari during agent processing, return after agent finishes, verify send button appears

🤖 Generated with Claude Code

[dimakis]

Centaur Review

Found 1 issue(s) (1 warning).

packages/client/src/store.ts

Clean, well-tested fix for a real iOS backgrounding bug. One minor race condition: syncRunningState should verify the active session hasn't changed before applying the state update.

• 🟡 unsafe_assumptions (L230): Race condition: syncRunningState captures sessionId at call time but doesn't verify sessions.active still matches when the promise resolves. If the user switches to a new session (which resets then re-sets running: true) while the meta fetch for the old session is in flight, !meta.isActive for the old session will incorrectly clear running for the new session. Guard with if (store.getState().sessions.active !== sessionId) return; before applying the state change. [fixable]

[dimakis]

🟡 unsafe_assumptions: Race condition: syncRunningState captures sessionId at call time but doesn't verify sessions.active still matches when the promise resolves. If the user switches to a new session (which resets then re-sets running: true) while the meta fetch for the old session is in flight, !meta.isActive for the old session will incorrectly clear running for the new session. Guard with if (store.getState().sessions.active !== sessionId) return; before applying the state change. [fixable]

[dimakis]

Centaur Review

Found 3 issue(s).

packages/client/src/store.ts

Clean, well-tested fix for a real iOS issue. The session-switch race guard is correct. Only minor style and coverage suggestions — no bugs or regressions.

• 🔵 style (L219): The docstring is 4 lines — longer than the project's 'one short line max' comment convention. The session-switch guard and iOS context are already clear from the code and the PR description; this could be a single line or omitted entirely. [fixable]
• 🔵 unsafe_assumptions (L224): syncRunningState has no deduplication guard (unlike fetchAndRestoreMessages which uses recoveryInFlight). Rapid foreground events (e.g., quick app-switch back and forth) could fire multiple concurrent meta fetches. In practice the session-switch guard and idempotent SET_RUNNING(false) make this harmless, but it's an asymmetry worth noting. [fixable]

packages/client/__tests__/store.test.ts

Clean, well-tested fix for a real iOS issue. The session-switch race guard is correct. Only minor style and coverage suggestions — no bugs or regressions.

• 🔵 missing_tests: No test covers the case where getSessionMeta rejects (network error, 404). The implementation silently catches, which is correct, but a test asserting that running state is unchanged on fetch failure would document this contract and prevent regressions. [fixable]

[dimakis]

🔵 style: The docstring is 4 lines — longer than the project's 'one short line max' comment convention. The session-switch guard and iOS context are already clear from the code and the PR description; this could be a single line or omitted entirely. [fixable]

[dimakis]

🔵 unsafe_assumptions: syncRunningState has no deduplication guard (unlike fetchAndRestoreMessages which uses recoveryInFlight). Rapid foreground events (e.g., quick app-switch back and forth) could fire multiple concurrent meta fetches. In practice the session-switch guard and idempotent SET_RUNNING(false) make this harmless, but it's an asymmetry worth noting. [fixable]

[dimakis]

Centaur Review

Found 2 issue(s).

packages/client/src/store.ts

Clean, well-tested fix for a real iOS edge case. The session-switch guard, the one-directional correction (only clears running, never sets it), and the four test scenarios covering happy path, still-active, stale-session, and network-error all demonstrate solid defensive thinking. Two minor suggestions on consistency.

• 🔵 unsafe_assumptions (L222): The runningSyncInFlight guard silently drops subsequent foreground events while a meta fetch is in flight. If a rapid background→foreground→background→foreground cycle occurs and the first fetch takes long, the second foreground recovery won't sync running state. This is consistent with how recoveryInFlight works for message recovery, so it's a known tradeoff — but worth noting that the guard could be replaced with an AbortController pattern that cancels the stale request and starts a new one, rather than dropping it. Not a bug in practice since iOS foreground events are infrequent. [fixable]
• 🔵 style (L237): Empty catch(() => {}) swallows errors silently. The parallel fetchAndRestoreMessages logs a console.warn on failure. Consider adding a matching console.warn('[mitzo] running state sync failed', err) for consistency and debuggability, especially since this is a new network call that could fail for various reasons on iOS. [fixable]

[dimakis]

🔵 unsafe_assumptions: The runningSyncInFlight guard silently drops subsequent foreground events while a meta fetch is in flight. If a rapid background→foreground→background→foreground cycle occurs and the first fetch takes long, the second foreground recovery won't sync running state. This is consistent with how recoveryInFlight works for message recovery, so it's a known tradeoff — but worth noting that the guard could be replaced with an AbortController pattern that cancels the stale request and starts a new one, rather than dropping it. Not a bug in practice since iOS foreground events are infrequent. [fixable]

[dimakis]

🔵 style: Empty catch(() => {}) swallows errors silently. The parallel fetchAndRestoreMessages logs a console.warn on failure. Consider adding a matching console.warn('[mitzo] running state sync failed', err) for consistency and debuggability, especially since this is a new network call that could fail for various reasons on iOS. [fixable]

[dimakis]

Centaur Review

Found 2 issue(s).

packages/client/src/store.ts

Clean, well-tested fix with good race-condition guards. Two minor suggestions around atomicity and defensive boolean checks — no blocking issues.

• 🔵 unsafe_assumptions (L232): Minor TOCTOU: msgState.running is read from a snapshot on line 230, but setState on line 233 uses a fresh s.messages. If running became false between the check and the dispatch (e.g., a normal session_end arrived in the gap), the reducer redundantly sets running: false — harmless but technically unnecessary. Could collapse the guard and dispatch into a single setState updater that checks s.messages.running && !meta.isActive atomically. Not a bug, just a minor cleanliness improvement. [fixable]
• 🔵 style (L232): The condition !meta.isActive treats undefined as falsy. If the server ever returns a response without the isActive field (e.g., older server version), this would incorrectly clear running. Consider an explicit meta.isActive === false check for defensive clarity, consistent with the explicit boolean semantics of the SessionMetaResponse type. [fixable]

[dimakis]

🔵 unsafe_assumptions: Minor TOCTOU: msgState.running is read from a snapshot on line 230, but setState on line 233 uses a fresh s.messages. If running became false between the check and the dispatch (e.g., a normal session_end arrived in the gap), the reducer redundantly sets running: false — harmless but technically unnecessary. Could collapse the guard and dispatch into a single setState updater that checks s.messages.running && !meta.isActive atomically. Not a bug, just a minor cleanliness improvement. [fixable]

[dimakis]

🔵 style: The condition !meta.isActive treats undefined as falsy. If the server ever returns a response without the isActive field (e.g., older server version), this would incorrectly clear running. Consider an explicit meta.isActive === false check for defensive clarity, consistent with the explicit boolean semantics of the SessionMetaResponse type. [fixable]