Merge PR #7301 from laitekin/fix/7299-hidden-session-poll

docs/rfcs/session-sse-contract-v1.md

@@ -59,6 +59,41 @@ proposes. It is routed in `api/routes.py` and implemented by
 is a global invalidation signal, not a per-session lifecycle stream. The proposed
 `GET /api/sessions/{session_id}/events` is per-session and path-distinct.
 
+### Hidden-tab observation and recovery (implemented client behavior)
+
+The browser closes its persistent per-session SSE while hidden and uses
+`_startHiddenActiveStreamPoll()` in `static/messages.js` to poll
+`GET /api/session/status?session_id=...` immediately and then every six seconds,
+subject to browser timer throttling. An active stream can be attached through
+the existing replay path; successful attachment stops the poll.
+
+HTTP `404` is ambiguous: older profile-visibility guards and the legacy
+unknown-profile path can return it for a live session. Current master returns
+`409 session_profile_mismatch` for a known foreign profile; that response
+remains retryable when another tab changes the browser-wide profile cookie.
+A single `404` therefore keeps polling and retains the hidden-resume owner.
+After three consecutive `404` responses, the poll pauses to bound repeated
+missing-session requests, but retains that owner so returning to the visible
+tab can reopen SSE. Any other response or network error resets this budget;
+a newly started poll also starts with a fresh budget.
+
+HTTP `410` is terminal: it stops the interval and clears the matching
+hidden-resume session ID. Returning to the visible tab then does not reopen
+SSE through that owner. Responses and queued ticks belong to one poll timer,
+not merely a session ID: they cannot stop or attach a replacement poll, even
+when the replacement observes the same session. The budget counts responses
+received by the current poll. Cleanup affects browser observation state only;
+it does not delete a session or cancel an agent run.
+
+Successful idle responses with no `active_stream_id`, network failures, and
+other non-success HTTP responses (including `401`, `403`, `429`, and `5xx`)
+remain retryable. Visibility return normally restores per-session SSE when a
+resume owner still exists, including after repeated `404` responses. Only a
+terminal `410` removes that automatic recovery path. A profile restored before
+the three-miss limit can recover through the next hidden poll; after the limit,
+recovery waits for visibility return or explicit session loading. Behavior coverage lives in
+`tests/test_hidden_tab_server_initiated_turn.py`.
+
 ### Heartbeat
 
 `_SSE_HEARTBEAT_INTERVAL_SECONDS = 5` (defined in `api/routes.py`) is the current

static/messages.js

@@ -8315,32 +8315,37 @@ function _startHiddenActiveStreamPoll(sid) {
   if (!sid) return;
   _stopHiddenActiveStreamPoll();
   _sessionStreamHiddenPollSid = sid;
+  let notFoundCount = 0;
+  let pollTimer = null;
+  const ownsPoll = () => _sessionStreamHiddenPollSid === sid &&
+    _sessionStreamHiddenPollTimer === pollTimer;
   const tick = () => {
+    // A queued tick/response must not mutate a replacement, even for the same sid.
+    if (!ownsPoll()) return;
     // Stop conditions: tab became visible (real SSE takes over), session
     // switched, or we're already rendering a stream.
     if (typeof document !== 'undefined' && !document.hidden) { _stopHiddenActiveStreamPoll(); return; }
-    if (_sessionStreamHiddenPollSid !== sid) { _stopHiddenActiveStreamPoll(); return; }
     if (S.activeStreamId) return; // already rendering; wait it out
     try {
       fetch(_apiUrl('api/session/status?session_id=' + encodeURIComponent(sid)), {credentials: 'same-origin'})
         .then(r => {
-          // #7299: 404 Not Found / 410 Gone are TERMINAL for this
-          // session-owned poll. The session has been deleted or no
-          // longer exists in the active state directory, so further
-          // polls are guaranteed to fail. Stop the poll immediately
-          // to avoid the infinite 404 loop on stale background tabs
-          // (one tab could fire ~10 such requests per minute; multiple
-          // tabs multiply the noise). Transient failures (5xx, rate
-          // limit, network error) keep polling — only the missing
-          // session itself is terminal.
-          if ((r.status === 404 || r.status === 410) && _sessionStreamHiddenPollSid === sid) {
+          if (!ownsPoll()) return null;
+          if (r && r.status === 404) {
+            // Profile visibility also returns 404. Bound repeated misses, but
+            // retain the resume owner so a profile flip back can self-heal.
+            if (++notFoundCount >= 3) _stopHiddenActiveStreamPoll();
+            return null;
+          }
+          notFoundCount = 0;
+          if (r && r.status === 410) {
+            if (_sessionStreamHiddenSid === sid) _sessionStreamHiddenSid = null;
             _stopHiddenActiveStreamPoll();
             return null;
           }
-          return r.ok ? r.json() : null;
+          return r && r.ok ? r.json() : null;
         })
         .then(d => {
-          if (!d || _sessionStreamHiddenPollSid !== sid) return;
+          if (!d || !ownsPoll()) return;
           const streamId = d.active_stream_id;
           if (streamId && S.activeStreamId !== String(streamId)) {
             // Server-initiated turn in flight while hidden → attach as replay.
@@ -8373,10 +8378,11 @@ function _startHiddenActiveStreamPoll(sid) {
             }
           }
         })
-        .catch(() => {});
-    } catch (_) {}
+        .catch(() => { notFoundCount = 0; });
+    } catch (_) { notFoundCount = 0; }
   };
-  _sessionStreamHiddenPollTimer = setInterval(tick, 6000);
+  pollTimer = setInterval(tick, 6000);
+  _sessionStreamHiddenPollTimer = pollTimer;
   // Fire one immediately so a turn already running when we go hidden is caught
   // without waiting a full interval.
   tick();

tests/test_hidden_tab_server_initiated_turn.py

@@ -21,8 +21,14 @@ tests pinning the contract:
   tab that transitions to hidden via the ``visibilitychange`` hook.
 """
 
+import json
+import shutil
+import subprocess
+import textwrap
 from pathlib import Path
 
+import pytest
+
 REPO_ROOT = Path(__file__).resolve().parent.parent
 MESSAGES_JS = (REPO_ROOT / "static" / "messages.js").read_text(encoding="utf-8")
 SESSION_OPS = (REPO_ROOT / "api" / "session_ops.py").read_text(encoding="utf-8")
@@ -89,9 +95,7 @@ def test_hidden_poll_hits_session_status_and_attaches_as_replay():
     """
     start = MESSAGES_JS.find("function _startHiddenActiveStreamPoll(sid)")
     assert start != -1
-    # #7299 added a 404/410 terminal-stop branch to the response handler.
-    # The window must cover the original fetch chain + the new branch.
-    body = MESSAGES_JS[start:start + 8000]
+    body = MESSAGES_JS[start:MESSAGES_JS.index("function _stopHiddenActiveStreamPoll()", start)]
     assert "api/session/status?session_id=" in body
     assert "d.active_stream_id" in body
     # attaches as replay (recovered=true) — turn is already mid-flight
@@ -174,9 +178,7 @@ def test_poll_stops_only_when_attach_succeeds():
     """
     start = MESSAGES_JS.find("function _startHiddenActiveStreamPoll(sid)")
     assert start != -1, "_startHiddenActiveStreamPoll signature not found"
-    # Window 2400: the multi-pane follow-up adds an explanatory comment block
-    # before the attach call, pushing it past a narrower slice.
-    body = MESSAGES_JS[start:start + 8000]
+    body = MESSAGES_JS[start:MESSAGES_JS.index("function _stopHiddenActiveStreamPoll()", start)]
     assert "const attached = _attachServerInitiatedStream(sid, streamId, true)" in body
     # Stop the poll only on a true attach (the false branch keeps polling within
     # the bounded-retry budget rather than stopping).
@@ -185,3 +187,320 @@ def test_poll_stops_only_when_attach_succeeds():
     # The bounded-retry give-up: a never-current pane stops after the budget.
     assert "_sessionStreamHiddenPollFalseCount" in body
     assert "_SESSION_STREAM_HIDDEN_POLL_MAX_FALSE" in body
+
+
+# ── Missing-session responses preserve recoverable profile ownership ──────
+
+NODE = shutil.which("node")
+
+
+@pytest.fixture(scope="module")
+def hidden_poll_results():
+    """Execute the real poll and visibility handler with deterministic responses."""
+    if NODE is None:
+        pytest.skip("node not available")
+    start = MESSAGES_JS.index("function _startHiddenActiveStreamPoll(sid)")
+    poll_end = MESSAGES_JS.index("function _chatStreamActiveForSession(sid)", start)
+    stream_end = MESSAGES_JS.index("function stopSessionStream()", poll_end)
+    functions = {
+        "poll": MESSAGES_JS[start:poll_end],
+        "session": MESSAGES_JS[poll_end:stream_end],
+    }
+
+    driver = textwrap.dedent(
+        r"""
+        const functions = JSON.parse(process.argv[1]);
+        let _sessionStreamHiddenPollTimer = null;
+        let _sessionStreamHiddenPollSid = null;
+        let _sessionStreamHiddenPollFalseStreamId = null;
+        let _sessionStreamHiddenPollFalseCount = 0;
+        let _sessionStreamHiddenSid = null;
+        let _sessionStreamSessionId = null;
+        let _sessionEventSource = null;
+        let _sessionStreamReconnectTimer = null;
+        const _SESSION_STREAM_HIDDEN_POLL_MAX_FALSE = 20;
+        let visibilityChange = null;
+        const document = {
+          hidden: true,
+          addEventListener: (type, listener) => {
+            if (type === 'visibilitychange') visibilityChange = listener;
+          },
+        };
+        const S = {
+          activeStreamId: null,
+          messages: [],
+          session: {session_id: 'session-a', message_count: 0},
+        };
+        const _apiUrl = value => value;
+        let attachCalls = 0;
+        const _attachServerInitiatedStream = () => { attachCalls += 1; return true; };
+        let intervalFn = null;
+        let intervalSeq = 0;
+        let fetchCalls = 0;
+        let eventSourceCalls = 0;
+
+        class EventSource {
+          constructor() {
+            eventSourceCalls += 1;
+            this.readyState = 1;
+          }
+          addEventListener() {}
+          close() { this.readyState = 2; }
+        }
+
+        globalThis.setInterval = fn => {
+          intervalFn = fn;
+          return ++intervalSeq;
+        };
+        globalThis.clearInterval = () => { intervalFn = null; };
+        eval(functions.poll);
+
+        function stopSessionStream() {
+          if (_sessionEventSource) _sessionEventSource.close();
+          _sessionEventSource = null;
+          _sessionStreamSessionId = null;
+          _stopHiddenActiveStreamPoll();
+        }
+
+        eval(functions.session);
+
+        const flush = () => new Promise(resolve => setImmediate(resolve));
+        const response = status => ({
+          ok: status >= 200 && status < 300,
+          status,
+          json: async () => ({active_stream_id: null}),
+        });
+
+        async function settle() {
+          await flush();
+          await flush();
+        }
+
+        function reset() {
+          stopSessionStream();
+          _sessionStreamHiddenSid = null;
+          document.hidden = true;
+          document._hermesSessionStreamVisibilityHook = false;
+          visibilityChange = null;
+          intervalFn = null;
+          fetchCalls = 0;
+          eventSourceCalls = 0;
+          attachCalls = 0;
+        }
+
+        async function runStatus(status, reject = false) {
+          reset();
+          _sessionStreamHiddenSid = 'session-a';
+          globalThis.fetch = () => {
+            fetchCalls += 1;
+            return reject ? Promise.reject(new Error('offline')) : Promise.resolve(response(status));
+          };
+          _startHiddenActiveStreamPoll('session-a');
+          await settle();
+          const nextTick = intervalFn;
+          if (nextTick) {
+            nextTick();
+            await settle();
+          }
+          return {
+            fetchCalls,
+            running: intervalFn !== null,
+            pollSid: _sessionStreamHiddenPollSid,
+            hiddenSid: _sessionStreamHiddenSid,
+          };
+        }
+
+        async function runStaleResponse(status, sameSession = false) {
+          reset();
+          let resolveA;
+          globalThis.fetch = url => {
+            fetchCalls += 1;
+            if (fetchCalls === 1) {
+              return new Promise(resolve => { resolveA = resolve; });
+            }
+            return Promise.resolve(response(200));
+          };
+          _sessionStreamHiddenSid = 'session-a';
+          _startHiddenActiveStreamPoll('session-a');
+          const oldTick = intervalFn;
+          const replacementSid = sameSession ? 'session-a' : 'session-b';
+          _sessionStreamHiddenSid = replacementSid;
+          _startHiddenActiveStreamPoll(replacementSid);
+          await settle();
+          resolveA(response(status));
+          await settle();
+          oldTick();
+          await settle();
+          return {
+            running: intervalFn !== null,
+            pollSid: _sessionStreamHiddenPollSid,
+            hiddenSid: _sessionStreamHiddenSid,
+          };
+        }
+
+        async function runVisibilityRecovery(status) {
+          reset();
+          globalThis.fetch = () => {
+            fetchCalls += 1;
+            return Promise.resolve(response(status));
+          };
+          startSessionStream('session-a');
+          await settle();
+          if (!visibilityChange) throw new Error('visibilitychange listener was not installed');
+          document.hidden = false;
+          visibilityChange();
+          await settle();
+          return {
+            eventSourceCalls,
+            running: intervalFn !== null,
+            pollSid: _sessionStreamHiddenPollSid,
+            hiddenSid: _sessionStreamHiddenSid,
+          };
+        }
+
+        async function runLateJson() {
+          reset();
+          let resolveBody;
+          globalThis.fetch = () => {
+            fetchCalls++;
+            if (fetchCalls === 1) return Promise.resolve({ok: true, status: 200,
+              json: () => new Promise(resolve => { resolveBody = resolve; })});
+            return Promise.resolve(response(200));
+          };
+          _sessionStreamHiddenSid = 'session-a';
+          _startHiddenActiveStreamPoll('session-a');
+          await settle();
+          _startHiddenActiveStreamPoll('session-a');
+          resolveBody({active_stream_id: 'old-stream'});
+          await settle();
+          return {attachCalls, running: intervalFn !== null};
+        }
+
+        async function runSequence(statuses) {
+          reset();
+          const states = [];
+          globalThis.fetch = () => {
+            const status = statuses[fetchCalls++];
+            if (status === -1) return Promise.reject(new Error('offline'));
+            if (status === 'active') return Promise.resolve({ok: true, status: 200,
+              json: async () => ({active_stream_id: 'live-a'})});
+            return Promise.resolve(response(status));
+          };
+          startSessionStream('session-a');
+          for (let i = 0; i < statuses.length; i++) {
+            if (i && intervalFn) intervalFn();
+            await settle();
+            states.push({running: intervalFn !== null, hiddenSid: _sessionStreamHiddenSid});
+          }
+          document.hidden = false;
+          visibilityChange();
+          await settle();
+          return {states, fetchCalls, eventSourceCalls, attachCalls};
+        }
+
+        (async () => {
+          const result = {
+            missing404: await runStatus(404),
+            missing410: await runStatus(410),
+            server500: await runStatus(500),
+            profile409: await runStatus(409),
+            auth401: await runStatus(401),
+            forbidden403: await runStatus(403),
+            rate429: await runStatus(429),
+            offline: await runStatus(0, true),
+            idle200: await runStatus(200),
+            stale404: await runStaleResponse(404),
+            stale410: await runStaleResponse(410),
+            sameSession410: await runStaleResponse(410, true),
+            visible404: await runVisibilityRecovery(404),
+            visible410: await runVisibilityRecovery(410),
+            repeated404: await runSequence([404, 404, 404, 404]),
+            recoveredProfile: await runSequence([404, 'active']),
+            recoveredKnownProfile: await runSequence([409, 'active']),
+            reset200: await runSequence([404, 404, 200, 404, 404, 404]),
+            reset500: await runSequence([404, 404, 500, 404, 404, 404]),
+            resetOffline: await runSequence([404, 404, -1, 404, 404, 404]),
+            reset409: await runSequence([404, 404, 409, 404, 404, 404]),
+            lateJson: await runLateJson(),
+          };
+          process.stdout.write(JSON.stringify(result));
+        })().catch(error => {
+          console.error(error);
+          process.exit(1);
+        });
+        """
+    )
+
+    proc = subprocess.run(
+        [NODE, "-e", driver, json.dumps(functions)],
+        capture_output=True,
+        text=True,
+        timeout=10,
+    )
+    assert proc.returncode == 0, proc.stderr
+    return json.loads(proc.stdout)
+
+
+def test_hidden_poll_transient_404_preserves_visibility_recovery(hidden_poll_results):
+    result = hidden_poll_results
+    assert result["visible404"]["eventSourceCalls"] == 1
+    assert result["missing404"] == {
+        "fetchCalls": 2, "running": True, "pollSid": "session-a", "hiddenSid": "session-a",
+    }
+    assert result["recoveredProfile"]["attachCalls"] == 1
+    assert result["recoveredKnownProfile"]["attachCalls"] == 1
+
+
+def test_hidden_poll_repeated_404_is_bounded_but_can_resume(hidden_poll_results):
+    result = hidden_poll_results["repeated404"]
+    assert [s["running"] for s in result["states"]] == [True, True, False, False]
+    assert all(s["hiddenSid"] == "session-a" for s in result["states"])
+    assert result["fetchCalls"] == 3
+    assert result["eventSourceCalls"] == 1
+
+
+@pytest.mark.parametrize("key", ["reset200", "reset500", "resetOffline", "reset409"])
+def test_hidden_poll_404_budget_requires_consecutive_responses(hidden_poll_results, key):
+    result = hidden_poll_results[key]
+    assert [s["running"] for s in result["states"]] == [True] * 5 + [False]
+    assert result["eventSourceCalls"] == 1
+
+
+def test_hidden_poll_410_stops_and_clears_resume_owner(hidden_poll_results):
+    result = hidden_poll_results
+    assert result["visible410"] == {
+            "eventSourceCalls": 0,
+            "running": False,
+            "pollSid": None,
+            "hiddenSid": None,
+    }
+    assert result["missing410"] == {
+            "fetchCalls": 1,
+            "running": False,
+            "pollSid": None,
+            "hiddenSid": None,
+    }
+
+
+def test_hidden_poll_transient_failures_and_idle_remain_retryable(hidden_poll_results):
+    result = hidden_poll_results
+    for key in ("server500", "offline", "idle200", "profile409", "auth401", "forbidden403", "rate429"):
+        assert result[key]["fetchCalls"] == 2
+        assert result[key]["running"] is True
+        assert result[key]["pollSid"] == "session-a"
+        assert result[key]["hiddenSid"] == "session-a"
+
+
+@pytest.mark.parametrize("key,sid", [
+    ("stale404", "session-b"), ("stale410", "session-b"), ("sameSession410", "session-a"),
+])
+def test_hidden_poll_stale_response_and_tick_cannot_stop_replacement(hidden_poll_results, key, sid):
+    assert hidden_poll_results[key] == {
+        "running": True,
+        "pollSid": sid,
+        "hiddenSid": sid,
+    }
+
+
+def test_hidden_poll_late_json_cannot_attach_into_replacement(hidden_poll_results):
+    assert hidden_poll_results["lateJson"] == {"attachCalls": 0, "running": True}