fix(sse): guard gateway reopen on focus so transient blur can't thrash it (#4151)
The #4151 focus hook reopened both global sidebar streams on `focus` with a bare `startGatewaySSE()`. But `startGatewaySSE()` is NOT idempotent — it begins with an unconditional `stopGatewaySSE()` (sessions.js:3778), which closes the live gateway EventSource, cancels its poll fallback (`stopGatewayPollFallback`), and resets `_gatewayProbeInFlight` / `_gatewaySSEWarningShown`. So on a transient blur shorter than the 1s debounce — where the blur-close timer is cleared on the following focus and the stream was never torn down — the focus handler still dropped and reconnected the gateway on every window-switch, cancelling its fallback and resetting probe/warning state. That is the exact thrash the debounce exists to prevent, in the multi-window PWA scenario this PR targets. (`ensureSessionEventsSSE()` is already idempotent via `if(_sessionEventsSSE) return`, so the session-events stream was unaffected; only the gateway churned. greptile P1.) Guard the reopen on the gateway actually being closed: `if(!_gatewaySSE) startGatewaySSE()`, mirroring the session-events idempotency. A sustained blur (timer fires → both streams closed) still reopens the gateway on focus; a transient blur leaves the still-open gateway untouched. Verified with a blur/focus/timer harness (transient → no teardown/reconnect; sustained → close then reopen; CLI-disabled → stays closed) and a new regression guard `test_focus_reopen_does_not_thrash_the_gateway_stream`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
408dbeb4f0
commit
266d9cdbd3
2 changed files with 40 additions and 5 deletions
|
|
@ -3694,9 +3694,16 @@ function _installSidebarSseFocusHook(){
|
|||
});
|
||||
window.addEventListener('focus', () => {
|
||||
if(_sidebarSseBlurCloseTimer){ clearTimeout(_sidebarSseBlurCloseTimer); _sidebarSseBlurCloseTimer = 0; }
|
||||
// Reopen (both ensure-idempotent) and catch up on anything missed while blurred.
|
||||
// Reopen and catch up on anything missed while blurred. ensureSessionEventsSSE()
|
||||
// is idempotent (`if(_sessionEventsSSE) return`), but startGatewaySSE() is NOT — it
|
||||
// begins with an unconditional stopGatewaySSE(). So only reopen the gateway when it
|
||||
// was actually closed; otherwise a transient blur shorter than the debounce (where
|
||||
// the blur-close timer was cleared and the stream was never torn down) would
|
||||
// drop+reconnect the live gateway on every window switch, cancelling its poll
|
||||
// fallback and resetting probe/warning state — the exact thrash the debounce exists
|
||||
// to prevent, in the multi-window scenario this fix targets (#4151).
|
||||
ensureSessionEventsSSE();
|
||||
startGatewaySSE();
|
||||
if(!_gatewaySSE) startGatewaySSE();
|
||||
void refreshSessionList('focus');
|
||||
});
|
||||
}
|
||||
|
|
|
|||
|
|
@ -50,7 +50,7 @@ def test_focus_hook_closes_both_global_sidebar_streams():
|
|||
"""The blur path closes the session-events AND gateway streams."""
|
||||
assert "function _installSidebarSseFocusHook()" in SESSIONS_JS
|
||||
start = SESSIONS_JS.find("function _installSidebarSseFocusHook()")
|
||||
block = SESSIONS_JS[start:start + 1100]
|
||||
block = SESSIONS_JS[start:start + 1700]
|
||||
# Installed once.
|
||||
assert "_hermesSidebarSseFocusHook" in block
|
||||
# Blur listener tears down both global streams.
|
||||
|
|
@ -70,7 +70,7 @@ def test_blur_close_is_debounced_and_rechecks_focus_at_fire_time():
|
|||
when it fires, so focus returning during the debounce cancels the teardown.
|
||||
"""
|
||||
start = SESSIONS_JS.find("function _installSidebarSseFocusHook()")
|
||||
block = SESSIONS_JS[start:start + 1100]
|
||||
block = SESSIONS_JS[start:start + 1700]
|
||||
assert "_sidebarSseBlurCloseTimer" in block
|
||||
assert "setTimeout(" in block
|
||||
# Re-check guards the actual close so a returned focus is a no-op.
|
||||
|
|
@ -79,6 +79,34 @@ def test_blur_close_is_debounced_and_rechecks_focus_at_fire_time():
|
|||
assert "clearTimeout(_sidebarSseBlurCloseTimer)" in block
|
||||
|
||||
|
||||
def test_focus_reopen_does_not_thrash_the_gateway_stream():
|
||||
"""The focus handler must NOT unconditionally restart the gateway stream.
|
||||
|
||||
startGatewaySSE() begins with an unconditional stopGatewaySSE() (it is NOT
|
||||
idempotent, unlike ensureSessionEventsSSE()'s `if(_sessionEventsSSE) return`).
|
||||
On a transient blur shorter than the 1s debounce, the blur-close timer is
|
||||
cleared and the gateway stream is never torn down — so an unconditional
|
||||
startGatewaySSE() on the following focus would drop+reconnect the live gateway,
|
||||
cancel its poll fallback, and reset probe/warning state on every window switch
|
||||
(the exact thrash the debounce exists to prevent, in the multi-window scenario
|
||||
#4151 targets). The reopen must therefore be guarded on the gateway actually
|
||||
being closed. (greptile P1.)
|
||||
"""
|
||||
start = SESSIONS_JS.find("function _installSidebarSseFocusHook()")
|
||||
block = SESSIONS_JS[start:start + 1700]
|
||||
focus_idx = block.find("window.addEventListener('focus'")
|
||||
assert focus_idx != -1
|
||||
focus_body = block[focus_idx:]
|
||||
# The gateway reopen is guarded on the stream being closed (mirrors the
|
||||
# session-events idempotency), not called unconditionally.
|
||||
assert "if(!_gatewaySSE) startGatewaySSE()" in focus_body, (
|
||||
"focus handler must guard startGatewaySSE() on `!_gatewaySSE` so a "
|
||||
"transient blur+focus does not drop+reconnect a still-open gateway stream"
|
||||
)
|
||||
# And it must NOT call startGatewaySSE() bare (unguarded) on focus.
|
||||
assert "\n startGatewaySSE();" not in focus_body
|
||||
|
||||
|
||||
def test_session_events_open_guard_uses_backgrounded_predicate():
|
||||
"""ensureSessionEventsSSE installs the focus hook and gates open on the predicate."""
|
||||
start = SESSIONS_JS.find("function ensureSessionEventsSSE()")
|
||||
|
|
@ -121,7 +149,7 @@ def test_per_session_stream_NOT_closed_on_blur():
|
|||
)
|
||||
# (b) the sidebar focus hook only manages the two global streams.
|
||||
start = SESSIONS_JS.find("function _installSidebarSseFocusHook()")
|
||||
block = SESSIONS_JS[start:start + 1100]
|
||||
block = SESSIONS_JS[start:start + 1700]
|
||||
assert "stopSessionStream" not in block
|
||||
assert "startSessionStream" not in block
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue