Stage 308: PR #1761 — fix: scope terminal stream cleanup to owner session by @dso2ng

static/messages.js

@@ -357,6 +357,37 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
   function _isActiveSession(){
     return !!(S.session&&S.session.session_id===activeSid);
   }
+  function _clearActivePaneInflightIfOwner(){
+    if(_isActiveSession()) clearInflight();
+  }
+  function _approvalBelongsToOwner(){
+    return _approvalSessionId===activeSid||(!_approvalSessionId&&_isActiveSession());
+  }
+  function _clarifyBelongsToOwner(){
+    return _clarifySessionId===activeSid||(!_clarifySessionId&&_isActiveSession());
+  }
+  function _clearApprovalForOwner(){
+    if(!_approvalBelongsToOwner()) return;
+    stopApprovalPolling();
+    hideApprovalCard(true);
+  }
+  function _clearClarifyForOwner(reason){
+    if(!_clarifyBelongsToOwner()) return;
+    stopClarifyPolling();
+    hideClarifyCard(true, reason||'terminal');
+  }
+  function _clearOwnerInflightState(){
+    delete INFLIGHT[activeSid];
+    clearInflightState(activeSid);
+    _clearActivePaneInflightIfOwner();
+  }
+  function _setActivePaneIdleIfOwner(){
+    if(_isActiveSession()||!S.session||!INFLIGHT[S.session.session_id]){
+      setBusy(false);
+      setComposerStatus('');
+      if(typeof setStatus==='function') setStatus('');
+    }
+  }
   function persistInflightState(){
     const inflight=INFLIGHT[activeSid];
     if(!inflight||typeof saveInflightState!=='function') return;
@@ -852,15 +883,12 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
       if(!isSessionViewed && typeof _markSessionCompletionUnread==='function'){
         _markSessionCompletionUnread(completedSid, completedSession.message_count);
       }
-      delete INFLIGHT[activeSid];
-      clearInflight();clearInflightState(activeSid);
+      _clearOwnerInflightState();
       if(typeof _markSessionCompletedInList==='function'){
         _markSessionCompletedInList(completedSession, activeSid);
       }
-      stopApprovalPollingForSession(activeSid);
-      stopClarifyPollingForSession(activeSid);
-      if(!_approvalSessionId || _approvalSessionId===activeSid) hideApprovalCard(true);
-      if(!_clarifySessionId || _clarifySessionId===activeSid) hideClarifyCard(true, 'terminal');
+      _clearApprovalForOwner();
+      _clearClarifyForOwner('terminal');
       if(isActiveSession){
         S.activeStreamId=null;
       }
@@ -944,13 +972,9 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
         // TTS auto-read: speak the last assistant response if enabled (#499)
         if(typeof autoReadLastAssistant==='function') setTimeout(()=>autoReadLastAssistant(), 300);
       }
+      if(isActiveSession) _queueDrainSid=activeSid;
       renderSessionList();
-      if(isActiveSession||!S.session||!INFLIGHT[S.session.session_id]){
-        _queueDrainSid=activeSid;
-        setBusy(false);
-        setStatus('');
-        setComposerStatus('');
-      }
+      _setActivePaneIdleIfOwner();
       playNotificationSound();
       sendBrowserNotification('Response complete',assistantText?assistantText.slice(0,100):'Task finished');
     });
@@ -1031,9 +1055,9 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
       // Application-level error sent explicitly by the server (rate limit, crash, etc.)
       // This is distinct from the SSE network 'error' event below.
       source.close();
-      delete INFLIGHT[activeSid];clearInflight();clearInflightState(activeSid);stopApprovalPollingForSession(activeSid);stopClarifyPollingForSession(activeSid);
-      if(!_approvalSessionId||_approvalSessionId===activeSid) hideApprovalCard(true);
-      if(!_clarifySessionId||_clarifySessionId===activeSid) hideClarifyCard(true, 'terminal');
+      _clearOwnerInflightState();
+      _clearApprovalForOwner();
+      _clearClarifyForOwner('terminal');
       if(S.session&&S.session.session_id===activeSid){
         S.activeStreamId=null;
         clearLiveToolCards();if(!assistantText)removeThinking();
@@ -1057,7 +1081,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
         try{const d=JSON.parse(e.data);trackBackgroundError(activeSid,_errTitle,d.message||'Error');}
         catch(_){trackBackgroundError(activeSid,_errTitle,'Error');}
       }
-      if(!S.session||!INFLIGHT[S.session.session_id]){setBusy(false);setComposerStatus('');}
+      _setActivePaneIdleIfOwner();
       renderSessionList(); // clear streaming indicator immediately on apperror
     });
 
@@ -1109,9 +1133,9 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
       _smdEndParser();
       if(typeof finalizeThinkingCard==='function') finalizeThinkingCard();
       source.close();
-      delete INFLIGHT[activeSid];clearInflight();clearInflightState(activeSid);stopApprovalPollingForSession(activeSid);stopClarifyPollingForSession(activeSid);
-      if(!_approvalSessionId||_approvalSessionId===activeSid) hideApprovalCard(true);
-      if(!_clarifySessionId||_clarifySessionId===activeSid) hideClarifyCard(true, 'cancelled');
+      _clearOwnerInflightState();
+      _clearApprovalForOwner();
+      _clearClarifyForOwner('cancelled');
       if(S.session&&S.session.session_id===activeSid){
         S.activeStreamId=null;
       }
@@ -1138,7 +1162,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
         }
       })();
       renderSessionList();
-      if(!S.session||!INFLIGHT[S.session.session_id]){setBusy(false);setComposerStatus('');}
+      _setActivePaneIdleIfOwner();
     });
   }
 
@@ -1148,10 +1172,10 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
       const session=data&&data.session;
       if(!session) return false;
       if(session.active_stream_id||session.pending_user_message) return false;
-      delete INFLIGHT[activeSid];clearInflight();clearInflightState(activeSid);stopApprovalPollingForSession(activeSid);stopClarifyPollingForSession(activeSid);
+      _clearOwnerInflightState();
       _closeSource();
-      if(!_approvalSessionId||_approvalSessionId===activeSid) hideApprovalCard(true);
-      if(!_clarifySessionId||_clarifySessionId===activeSid) hideClarifyCard(true, 'terminal');
+      _clearApprovalForOwner();
+      _clearClarifyForOwner('terminal');
       const isSessionViewed=_isSessionActivelyViewed(activeSid);
       const completedSid=session.session_id||activeSid;
       if(!isSessionViewed && typeof _markSessionCompletionUnread==='function'){
@@ -1185,12 +1209,9 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
         if(isSessionViewed) _markSessionViewed(completedSid, session.message_count ?? S.messages.length);
         syncTopbar();renderMessages({preserveScroll:true});
       }
+      if(_isActiveSession()) _queueDrainSid=activeSid;
       renderSessionList();
-      if(isActiveSession||!S.session||!INFLIGHT[S.session.session_id]){
-        _queueDrainSid=activeSid;
-        setBusy(false);
-        setComposerStatus('');
-      }
+      _setActivePaneIdleIfOwner();
       return true;
     }catch(_){
       return false;
@@ -1204,10 +1225,10 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
     _streamFinalized=true;
     if(_pendingRafHandle!==null){cancelAnimationFrame(_pendingRafHandle);clearTimeout(_pendingRafHandle);_pendingRafHandle=null;_renderPending=false;}
     if(typeof finalizeThinkingCard==='function') finalizeThinkingCard();
-    delete INFLIGHT[activeSid];clearInflight();clearInflightState(activeSid);stopApprovalPollingForSession(activeSid);stopClarifyPollingForSession(activeSid);
+    _clearOwnerInflightState();
     _closeSource();
-    if(!_approvalSessionId||_approvalSessionId===activeSid) hideApprovalCard(true);
-    if(!_clarifySessionId||_clarifySessionId===activeSid) hideClarifyCard(true, 'terminal');
+    _clearApprovalForOwner();
+    _clearClarifyForOwner('terminal');
     if(S.session&&S.session.session_id===activeSid){
       S.activeStreamId=null;
       clearLiveToolCards();if(!assistantText)removeThinking();
@@ -1219,7 +1240,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
         trackBackgroundError(activeSid,_errTitle,'Connection lost');
       }
     }
-    if(!S.session||!INFLIGHT[S.session.session_id]){setBusy(false);setComposerStatus('');}
+    _setActivePaneIdleIfOwner();
   }
 
   (async()=>{
@@ -1229,19 +1250,15 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
       try{
         const st=await api(`/api/chat/stream/status?stream_id=${encodeURIComponent(streamId)}`);
         if(!st.active){
-          delete INFLIGHT[activeSid];
-          clearInflight();
-          clearInflightState(activeSid);
-          stopApprovalPollingForSession(activeSid);
-          stopClarifyPollingForSession(activeSid);
-          if(!_approvalSessionId||_approvalSessionId===activeSid) hideApprovalCard(true);
-          if(!_clarifySessionId||_clarifySessionId===activeSid) hideClarifyCard(true, 'terminal');
+          _clearOwnerInflightState();
+          _clearApprovalForOwner();
+          _clearClarifyForOwner('terminal');
           if(S.session&&S.session.session_id===activeSid){
             S.activeStreamId=null;
             clearLiveToolCards();
             removeThinking();
-            _queueDrainSid=activeSid;setBusy(false);
-            setComposerStatus('');
+            if(_isActiveSession()) _queueDrainSid=activeSid;
+            _setActivePaneIdleIfOwner();
             renderMessages({preserveScroll:true});
             renderSessionList();
           }

tests/test_1694_terminal_cleanup_ownership.py

@@ -0,0 +1,93 @@
+"""Regression tests for #1694 terminal stream cleanup ownership.
+
+Terminal SSE events for one session must not mutate another currently viewed
+active pane. The owning session's persisted/runtime stream marker can be cleared,
+but global pane state such as ``clearInflight()``, approval/clarify polling, and
+``setBusy(false)`` must be gated to the session that owns the active pane/card.
+"""
+from pathlib import Path
+
+REPO_ROOT = Path(__file__).parent.parent
+MESSAGES_JS = (REPO_ROOT / "static" / "messages.js").read_text(encoding="utf-8")
+
+
+def _body_from_brace(src: str, brace: int, label: str) -> str:
+    assert brace >= 0, f"body opening brace not found for: {label}"
+    depth = 1
+    i = brace + 1
+    while i < len(src) and depth:
+        ch = src[i]
+        if ch == "{":
+            depth += 1
+        elif ch == "}":
+            depth -= 1
+        i += 1
+    assert depth == 0, f"body did not close for: {label}"
+    return src[brace + 1 : i - 1]
+
+
+def _brace_body_after(src: str, marker: str) -> str:
+    start = src.find(marker)
+    assert start >= 0, f"marker not found: {marker}"
+    brace = src.find("{", start)
+    return _body_from_brace(src, brace, marker)
+
+
+def _event_body(event_name: str) -> str:
+    return _brace_body_after(MESSAGES_JS, f"source.addEventListener('{event_name}'")
+
+
+def _function_body(name: str) -> str:
+    marker = f"function {name}("
+    start = MESSAGES_JS.find(marker)
+    assert start >= 0, f"function not found: {name}"
+    signature_end = MESSAGES_JS.find("){", start)
+    assert signature_end >= 0, f"function body not found: {name}"
+    return _body_from_brace(MESSAGES_JS, signature_end + 1, name)
+
+
+def test_terminal_handlers_use_session_owned_cleanup_helpers():
+    """Patch #1694 should centralize terminal cleanup behind owner-aware helpers."""
+    attach_body = _function_body("attachLiveStream")
+    assert "function _clearOwnerInflightState()" in attach_body
+    owner_helper = _function_body("_clearOwnerInflightState")
+    assert "delete INFLIGHT[activeSid]" in owner_helper
+    assert "clearInflightState(activeSid)" in owner_helper
+    assert "_clearActivePaneInflightIfOwner();" in owner_helper
+    assert "function _clearActivePaneInflightIfOwner()" in attach_body
+    assert "function _clearApprovalForOwner()" in attach_body
+    assert "function _clearClarifyForOwner(" in attach_body
+    assert "function _setActivePaneIdleIfOwner(" in attach_body
+
+
+def test_done_event_does_not_clear_active_pane_for_background_session():
+    """A background done event may clear its owner marker, not the active pane."""
+    body = _event_body("done")
+    assert "_clearOwnerInflightState();" in body
+    assert "clearInflight();clearInflightState(activeSid)" not in body
+    assert "delete INFLIGHT[activeSid];\n      clearInflight();" not in body
+    assert "renderSessionList();setBusy(false)" not in body
+    assert "_setActivePaneIdleIfOwner" in body
+
+
+def test_error_and_cancel_events_do_not_blanket_stop_active_pane_polling():
+    """Background app errors/cancels must not stop another pane's prompt polling."""
+    for event_name in ("apperror", "cancel"):
+        body = _event_body(event_name)
+        assert "_clearOwnerInflightState();" in body, event_name
+        assert "_clearApprovalForOwner" in body, event_name
+        assert "_clearClarifyForOwner" in body, event_name
+        assert "stopApprovalPolling();stopClarifyPolling();" not in body, event_name
+        assert "clearInflight();clearInflightState(activeSid)" not in body, event_name
+
+
+def test_reconnect_settled_and_error_paths_keep_cleanup_session_scoped():
+    """Reconnect terminal cleanup paths should follow the same owner model."""
+    restore_body = _function_body("_restoreSettledSession")
+    error_body = _function_body("_handleStreamError")
+    combined = restore_body + "\n" + error_body
+    assert combined.count("_clearOwnerInflightState();") >= 2
+    assert "delete INFLIGHT[activeSid];clearInflight();clearInflightState(activeSid)" not in combined
+    assert "stopApprovalPolling();stopClarifyPolling();" not in combined
+    assert "renderSessionList();setBusy(false)" not in combined
+    assert "_setActivePaneIdleIfOwner" in combined

tests/test_issue856_background_completion_unread.py

@@ -74,17 +74,19 @@ def test_done_event_updates_sidebar_cache_immediately_after_completion_marker():
     done_block = _done_block()
 
     marker_idx = done_block.find("_markSessionCompletionUnread(completedSid")
-    delete_idx = done_block.find("delete INFLIGHT[activeSid];")
+    cleanup_idx = done_block.find("_clearOwnerInflightState();")
+    if cleanup_idx == -1:
+        cleanup_idx = done_block.find("delete INFLIGHT[activeSid];")
     cache_idx = done_block.find("_markSessionCompletedInList(completedSession, activeSid);")
     refresh_idx = done_block.find("renderSessionList();", cache_idx)
     sound_idx = done_block.find("playNotificationSound();", cache_idx)
 
     assert "function _markSessionCompletedInList(" in SESSIONS_JS
     assert marker_idx != -1, "done handler must write the completion-unread marker first"
-    assert delete_idx != -1, "done handler must clear local INFLIGHT before rendering idle state"
+    assert cleanup_idx != -1, "done handler must clear local INFLIGHT before rendering idle state"
     assert cache_idx != -1, "done handler must update the sidebar cache immediately"
     assert refresh_idx != -1 and sound_idx != -1
-    assert marker_idx < delete_idx < cache_idx < refresh_idx < sound_idx, (
+    assert marker_idx < cleanup_idx < cache_idx < refresh_idx < sound_idx, (
         "the sidebar should flip from spinner to dot from the done payload before "
         "waiting for /api/sessions or playing the completion cue"
     )

tests/test_regressions.py

@@ -248,8 +248,13 @@ def test_done_handler_guards_setbusy_with_inflight_check(cleanup_test_sessions):
     disable B's Send button.
     """
     src = (REPO_ROOT / "static/messages.js").read_text()
-    # The fix wraps setBusy(false) in a guard
-    assert "INFLIGHT[S.session.session_id]" in src,         "messages.js must guard setBusy(false) with INFLIGHT check for current session"
+    # The fix wraps setBusy(false) in an active-pane ownership guard. Newer
+    # implementations may centralize the guard in a helper rather than repeat the
+    # raw INFLIGHT expression at every terminal event site.
+    assert (
+        "INFLIGHT[S.session.session_id]" in src
+        or "function _setActivePaneIdleIfOwner" in src
+    ), "messages.js must guard setBusy(false) for the current session"
 
 
 def test_refresh_handler_does_not_drop_tool_messages_needed_by_todos(cleanup_test_sessions):

tests/test_session_runtime_ownership_invariants.py

@@ -63,9 +63,13 @@ class TestSessionOwnedRuntimeInvariants:
             "A background session's done event must not unconditionally call setBusy(false); "
             "that can idle an unrelated active pane that is still running."
         )
-        assert "if(isActiveSession||!S.session||!INFLIGHT[S.session.session_id])" in done.replace(" ", ""), (
-            "The done handler should only idle composer state when the completed stream "
-            "belongs to the active pane, or when no other active-pane inflight runtime exists."
+        normalized = done.replace(" ", "")
+        assert (
+            "if(isActiveSession||!S.session||!INFLIGHT[S.session.session_id])" in normalized
+            or "_setActivePaneIdleIfOwner();" in done
+        ), (
+            "The done handler should only idle composer state through an active-pane guard, "
+            "not from background completions owned by another session."
         )
 
     def test_server_session_finalize_does_not_idle_unrelated_active_pane(self):
@@ -75,7 +79,11 @@ class TestSessionOwnedRuntimeInvariants:
             "The fallback server-finalize path must not idle the active pane for a "
             "background session completion."
         )
-        assert "if(isActiveSession||!S.session||!INFLIGHT[S.session.session_id])" in finalize.replace(" ", ""), (
+        normalized = finalize.replace(" ", "")
+        assert (
+            "if(isActiveSession||!S.session||!INFLIGHT[S.session.session_id])" in normalized
+            or "_setActivePaneIdleIfOwner();" in finalize
+        ), (
             "The fallback server-finalize path should use the same active-pane guard as the live done event."
         )
 
@@ -96,8 +104,14 @@ class TestSessionOwnedRuntimeInvariants:
         )
 
         done = _event_handler(messages, "done")
-        assert "stopApprovalPollingForSession(activeSid)" in done
-        assert "stopClarifyPollingForSession(activeSid)" in done
+        assert (
+            "stopApprovalPollingForSession(activeSid)" in done
+            or "_clearApprovalForOwner();" in done
+        )
+        assert (
+            "stopClarifyPollingForSession(activeSid)" in done
+            or "_clearClarifyForOwner('terminal');" in done
+        )
         assert "stopApprovalPolling();\n      stopClarifyPolling();" not in done, (
             "The done handler must not blindly stop whatever approval/clarify poller "
             "the active pane currently owns."

tests/test_sprint30.py

@@ -602,8 +602,10 @@ class TestClarifyCardTimerLogic:
                       src, re.DOTALL)
         assert m, 'cancel event handler not found'
         body = m.group(0)
-        assert "hideClarifyCard(true, 'cancelled')" in body, \
-            'explicit stream cancel must not use the timeout/terminal draft preservation path'
+        assert (
+            "hideClarifyCard(true, 'cancelled')" in body
+            or "_clearClarifyForOwner('cancelled')" in body
+        ), 'explicit stream cancel must not use the timeout/terminal draft preservation path'
 
     def test_clarify_urgent_countdown_has_non_color_cue(self):
         css = self._get_css().read_text()

tests/test_sprint36.py

@@ -166,9 +166,18 @@ def test_sse_cancel_handler_calls_set_busy():
     # Find the closing of this handler block (next top-level addEventListener)
     next_handler = src.find("source.addEventListener(", idx + 50)
     block = src[idx:next_handler] if next_handler != -1 else src[idx:idx + 3000]
-    assert "setBusy(false)" in block, (
-        "SSE cancel handler no longer calls setBusy(false)"
+    assert (
+        "setBusy(false)" in block
+        or "_setActivePaneIdleIfOwner()" in block
+    ), (
+        "SSE cancel handler no longer idles the owning active pane"
     )
+    if "_setActivePaneIdleIfOwner()" in block:
+        helper_idx = src.find("function _setActivePaneIdleIfOwner")
+        assert helper_idx != -1
+        next_function = src.find("\n  function ", helper_idx + 1)
+        helper = src[helper_idx:next_function if next_function != -1 else helper_idx + 800]
+        assert "setBusy(false)" in helper
 
 
 # ── 7. i18n key preserved ─────────────────────────────────────────────────────