fix(sessions): close stale-response race in _loadOlderMessages

nesquena · claude · nesquena · commit d974388d1f35 · 2026-04-27T16:09:05.000-07:00
The cancellation guard at the top of _loadOlderMessages checks
_loadingSessionId !== null &amp;&amp; _loadingSessionId !== sid. That catches
the case where a NEW session load is in progress when the older-messages
fetch returns. It misses the case where:

  1. user is on session A, _loadOlderMessages in flight
  2. user switches to session B; loadSession(B) completes; sets
     _loadingSessionId = null
  3. older-messages response for A returns
     -&gt; _loadingSessionId === null
     -&gt; guard evaluates to (null !== null) &amp;&amp; ... -&gt; false -&gt; NO bail
     -&gt; S.messages = [...olderMsgs, ...S.messages] prepends A's old
        messages onto B's S.messages

Tighten the guard by also comparing the captured sid against the
currently-active S.session.session_id. Together they cover both windows:
mid-switch (new sid loading) and post-switch (no load in progress).

Added a regression test (test_load_older_compares_against_active_session_id)
that asserts the new check is present and runs before any S.messages
mutation. Existing 5 cancellation tests still pass.

Co-Authored-By: Claude Opus 4.7 (1M context) &lt;noreply@anthropic.com&gt;
diff --git a/static/sessions.js b/static/sessions.js
@@ -370,7 +370,15 @@ async function _loadOlderMessages() {
   _loadingOlder = true;
   try {
     const data = await api(`/api/session?session_id=${encodeURIComponent(sid)}&messages=1&resolve_model=0&msg_before=${_oldestIdx}&msg_limit=${_INITIAL_MSG_LIMIT}`);
-    if (!data || !data.session || (_loadingSessionId !== null && _loadingSessionId !== sid)) return;
+    // Cancellation guards:
+    //  - response shape sane
+    //  - the active session is still the one we issued the request for.
+    //    Compare against S.session.session_id, NOT _loadingSessionId — the
+    //    latter is null between session loads, leaving a window where a
+    //    stale response could prepend onto the new session's S.messages.
+    if (!data || !data.session) return;
+    if (!S.session || S.session.session_id !== sid) return;
+    if (_loadingSessionId !== null && _loadingSessionId !== sid) return;
     const olderMsgs = (data.session.messages || []).filter(m => m && m.role);
     if (!olderMsgs.length) { _messagesTruncated = false; return; }
     // Prepend older messages
diff --git a/tests/test_parallel_session_switch.py b/tests/test_parallel_session_switch.py
@@ -532,3 +532,28 @@ def test_oldest_idx_reset_prevents_wrong_cursor(self):
             "_loadOlderMessages should bail out if _oldestIdx <= 0, "
             "which is the reset value after session switch."
         )
+
+    def test_load_older_compares_against_active_session_id(self):
+        """_loadOlderMessages must verify S.session.session_id === sid after await.
+
+        _loadingSessionId alone is insufficient: it is null between session
+        loads, so a stale older-messages response that lands AFTER a
+        completed session switch would otherwise pass the guard and prepend
+        onto the new session's S.messages. The S.session.session_id check
+        closes that window.
+        """
+        fn_start = SESSIONS_JS.find("async function _loadOlderMessages")
+        fn_end = SESSIONS_JS.find("\n}", fn_start) + 2
+        fn_body = SESSIONS_JS[fn_start:fn_end]
+
+        assert "S.session.session_id !== sid" in fn_body, (
+            "_loadOlderMessages must compare S.session.session_id against "
+            "the captured sid after await — _loadingSessionId is null "
+            "between sessions and would let a stale response through."
+        )
+        # The S.session check must appear BEFORE the S.messages mutation.
+        active_check_idx = fn_body.find("S.session.session_id !== sid")
+        mutation_idx = fn_body.find("S.messages = [...olderMsgs")
+        assert active_check_idx >= 0 and mutation_idx >= 0 and active_check_idx < mutation_idx, (
+            "Active-session guard must run before S.messages mutation."
+        )
