From f57c0330ec3e9cb511604559af354e7950dbab3e Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Fri, 19 Jun 2026 23:02:58 +0000 Subject: [PATCH] fix(sessions): scope #4354 unstick to skip the active mid-send session (#2689 Opus gate) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gate-found (Opus SHIP-WITH-FIXES; Codex SAFE — split resolved by reading the contract: no dropped-turn, but a real transient start-race flicker). #4424 removed the _sendInProgress guards entirely; that re-opened the #2689 start-race SYMPTOM for the one session mid-send (a /api/sessions poll landing during the /api/chat/start round-trip would blank the just-sent bubble + queue a spurious force-reload). Re-add the guard SCOPED to sid===_sendInProgressSid only, in both _reconcileActiveSessionIdleStateFromList and _purgeStaleInflightEntries — a long-hung session has _sendInProgress===false so it still gets unstuck (#4354), only the in-flight start window is protected. Also fixes Opus's test-masking finding: test_inflight_send_start_race now strips JS comments before asserting + requires a real , so a comment can't green-light the #2689 guard, and adds coverage for the reconcile path. Co-authored-by: bergeouss --- static/sessions.js | 17 +++++++++++-- tests/test_inflight_send_start_race.py | 33 +++++++++++++++++++++++++- 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/static/sessions.js b/static/sessions.js index eca36ad4..8d6a2782 100644 --- a/static/sessions.js +++ b/static/sessions.js @@ -381,9 +381,16 @@ function _isServerIdleSessionRow(s) { function _reconcileActiveSessionIdleStateFromList(serverRows) { if (!S || !S.session || !S.session.session_id) return false; - // #4354: Removed _sendInProgress guard — server is_streaming=false is authoritative. if (!Array.isArray(serverRows)) return false; const sid=S.session.session_id; + // #4354: clear a stuck indicator when the server reports idle — server + // is_streaming/active_stream_id is authoritative. BUT skip the ONE session + // that is actively mid-send (#2689 start-race): during the /api/chat/start + // round-trip the server row is still idle while the client owns the optimistic + // turn, so reconciling it here would blank the just-sent bubble + queue a + // spurious force-reload. A long-hung session has _sendInProgress===false, so + // it still gets unstuck — only the in-flight start window is protected. + if (typeof _sendInProgress !== 'undefined' && _sendInProgress && sid === _sendInProgressSid) return false; const serverRow=serverRows.find(s=>s&&s.session_id===sid); if (!serverRow) return false; if (!_isServerIdleSessionRow(serverRow)) return false; @@ -435,7 +442,13 @@ function _purgeStaleInflightEntries() { } } for (const sid of Object.keys(INFLIGHT)) { - // #4354: Removed _sendInProgressSid skip — purge stale INFLIGHT even during hung sends. + // #4354: purge stale INFLIGHT even for a hung/idle session, BUT skip the one + // session actively mid-send (#2689 start-race) — during /api/chat/start the + // server row is briefly idle while the client owns the optimistic INFLIGHT + // entry; purging it here would drop the in-flight turn's local state. + if (typeof _sendInProgress !== 'undefined' && _sendInProgress && sid === _sendInProgressSid) { + continue; + } if (!sessionsById.has(sid)) { // Session is absent from _allSessions — it was deleted / archived / // filtered and can never stream again, so drop the entry. diff --git a/tests/test_inflight_send_start_race.py b/tests/test_inflight_send_start_race.py index 521e384a..927515d0 100644 --- a/tests/test_inflight_send_start_race.py +++ b/tests/test_inflight_send_start_race.py @@ -39,9 +39,19 @@ def test_send_preserves_optimistic_messages_across_chat_start_await(): ) +def _strip_js_comments(src: str) -> str: + """Remove // line comments and /* */ block comments so source-grep assertions + match real statements, not comment text (a comment must not satisfy a guard + regression check). Good enough for these structural checks — not a full JS parser.""" + import re + src = re.sub(r"/\*.*?\*/", "", src, flags=re.DOTALL) + src = re.sub(r"(?m)//.*$", "", src) + return src + + def test_stale_inflight_purge_preserves_current_send_before_stream_id_exists(): """Sidebar cleanup must not delete the active send before /api/chat/start responds.""" - body = _function_body(SESSIONS_JS, "_purgeStaleInflightEntries") + body = _strip_js_comments(_function_body(SESSIONS_JS, "_purgeStaleInflightEntries")) assert "_sendInProgress" in body and "_sendInProgressSid" in body, ( "_purgeStaleInflightEntries() should skip the current send while start is in progress" @@ -49,6 +59,27 @@ def test_stale_inflight_purge_preserves_current_send_before_stream_id_exists(): skip_idx = body.index("_sendInProgress") delete_idx = body.index("delete INFLIGHT[sid];") assert skip_idx < delete_idx, "the current-send skip must run before any purge deletion" + # The skip must be a real guarded `continue`, not just a token in a comment (#4354/#2689). + assert "continue;" in body[skip_idx:delete_idx], ( + "the current-send skip must be an actual `continue` before the purge deletion" + ) + + +def test_idle_reconcile_preserves_current_send_before_stream_id_exists(): + """The list-poll idle reconciler must not clear the session that is mid-send. + + #4354 removed the _sendInProgress guard here to unstick a hung indicator; + #2689's start-race protection must still cover the ONE session actively + mid-send (server row is briefly idle during /api/chat/start). Verified + against comment-stripped source so a comment can't satisfy the check. + """ + body = _strip_js_comments(_function_body(SESSIONS_JS, "_reconcileActiveSessionIdleStateFromList")) + assert "_sendInProgress" in body and "_sendInProgressSid" in body, ( + "_reconcileActiveSessionIdleStateFromList() must skip the active mid-send session" + ) + guard_idx = body.index("_sendInProgress") + clear_idx = body.index("S.busy=false") + assert guard_idx < clear_idx, "the mid-send skip must run before the idle clear" def test_send_clears_stale_busy_state_before_queue_branch():