fix(client): recover SSE on browser visibilitychange#325
Conversation
iOS Safari kills EventSource connections when the tab is backgrounded without firing an error event, so the browser's native auto-reconnect never triggers. The Capacitor lifecycle hook already called ensureConnected() on resume, but browser clients had no equivalent recovery path — leaving health events (and thus the voice mic button) permanently dead after a single background cycle. Add a visibilitychange listener in the EventBus singleton that calls ensureConnected() when the page becomes visible again. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
96d4fc3 to
3cbdca1
Compare
dimakis
left a comment
There was a problem hiding this comment.
Centaur Review
Found 3 issue(s) (1 critical) (1 warning).
frontend/src/lib/event-bus-singleton.ts
The PR ships tests for a visibilitychange recovery feature whose implementation is missing from the diff — the singleton module has no listener, so both tests will fail.
- 🔴 bugs: The PR adds tests for a visibilitychange listener that calls
eventBus.ensureConnected(), but the listener itself was never added toevent-bus-singleton.ts. The module still only exports the EventBus and callsconnect()— there is nodocument.addEventListener('visibilitychange', ...). Both tests will fail because dispatching the event does nothing. The commit message says 'Add a visibilitychange listener in the EventBus singleton' but the implementation is missing from the diff.[fixable]
frontend/src/lib/__tests__/event-bus-singleton.test.ts
The PR ships tests for a visibilitychange recovery feature whose implementation is missing from the diff — the singleton module has no listener, so both tests will fail.
- 🔵 missing_tests: No test verifies the actual recovery behavior — that
ensureConnected()recreates a CLOSED EventSource. The tests only check thatensureConnectedis called, but a test confirming the EventSource is recreated when readyState is CLOSED (the real bug scenario from the commit message) would add meaningful coverage beyond the spy check.[fixable] - 🟡 unsafe_assumptions (L30): The test mutates
document.visibilityStateviaObject.definePropertywithout restoring it in anafterEach. Since both tests modify the same global, a test ordering dependency exists — the second test ('hidden') could inadvertently pass if it relies on state left by the first test. UseafterEachto restore the original descriptor.[fixable]
| const ensureConnectedSpy = vi.spyOn(eventBus, 'ensureConnected'); | ||
|
|
||
| // Simulate page becoming visible | ||
| Object.defineProperty(document, 'visibilityState', { |
There was a problem hiding this comment.
🟡 unsafe_assumptions: The test mutates document.visibilityState via Object.defineProperty without restoring it in an afterEach. Since both tests modify the same global, a test ordering dependency exists — the second test ('hidden') could inadvertently pass if it relies on state left by the first test. Use afterEach to restore the original descriptor. [fixable]
dimakis
left a comment
There was a problem hiding this comment.
Centaur Review
LGTM — no issues found.
iOS Safari kills EventSource connections when the tab is backgrounded without firing an error event, so the browser's native auto-reconnect never triggers. The Capacitor lifecycle hook already called ensureConnected() on resume, but browser clients had no equivalent recovery path — leaving health events (and thus the voice mic button) permanently dead after a single background cycle. Add a visibilitychange listener in the EventBus singleton that calls ensureConnected() when the page becomes visible again. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Summary
Fix
Add a
visibilitychangelistener inevent-bus-singleton.tsthat callsensureConnected()when the page becomes visible.Test plan
event-bus-singleton.test.ts— verifiesensureConnectedcalled on visible, not called on hidden🤖 Generated with Claude Code