From a7b04bbc1e35065f610b324127f1246c29a6635d Mon Sep 17 00:00:00 2001 From: ai-ag2026 Date: Wed, 6 May 2026 22:47:58 +0200 Subject: [PATCH 1/7] fix: preserve pending user turn on stream errors --- api/streaming.py | 39 +++++++++++++++++++ tests/test_issue1361_cancel_data_loss.py | 49 ++++++++++++++++++++++++ 2 files changed, 88 insertions(+) diff --git a/api/streaming.py b/api/streaming.py index 8bed63dc..ee5c6418 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -1545,6 +1545,43 @@ def _sse(handler, event, data): handler.wfile.flush() +def _materialize_pending_user_turn_before_error(session) -> bool: + """Persist the pending user prompt before clearing runtime stream state. + + Error paths often clear ``pending_user_message`` before appending an assistant + error marker. In deferred session-save mode that pending field can be the + only durable copy of the user's current turn, so clearing it makes the user + bubble disappear on reload/reconcile. Return True when a recovered user turn + was appended. + """ + pending_text = str(getattr(session, 'pending_user_message', None) or '') + if not pending_text: + return False + normalized_pending = " ".join(pending_text.split()) + if normalized_pending: + for existing in reversed(list(getattr(session, 'messages', None) or [])[-8:]): + if not isinstance(existing, dict) or existing.get('role') != 'user': + continue + existing_text = " ".join(str(existing.get('content') or '').split()) + if existing_text == normalized_pending: + return False + recovered_ts = int(time.time()) + pending_started_at = getattr(session, 'pending_started_at', None) + if isinstance(pending_started_at, (int, float)) and pending_started_at > 0: + recovered_ts = int(pending_started_at) + recovered = { + 'role': 'user', + 'content': pending_text, + 'timestamp': recovered_ts, + '_recovered': True, + } + pending_attachments = getattr(session, 'pending_attachments', None) + if pending_attachments: + recovered['attachments'] = list(pending_attachments) + session.messages.append(recovered) + return True + + def _last_resort_sync_from_core(session, stream_id, agent_lock): """Final-exit guard: if the stream exits with pending_user_message still set, sync messages from the core transcript or add an error marker. @@ -2568,6 +2605,7 @@ def _run_agent_streaming( # Persist the error so it survives page reload. # _error=True ensures _sanitize_messages_for_api excludes it from # subsequent API calls so the LLM never sees its own error as prior context. + _materialize_pending_user_turn_before_error(s) s.active_stream_id = None s.pending_user_message = None s.pending_attachments = [] @@ -3029,6 +3067,7 @@ def _run_agent_streaming( # API calls so the LLM never sees its own error as prior context on the next turn. _lock_ctx = _agent_lock if _agent_lock is not None else contextlib.nullcontext() with _lock_ctx: + _materialize_pending_user_turn_before_error(s) s.active_stream_id = None s.pending_user_message = None s.pending_attachments = [] diff --git a/tests/test_issue1361_cancel_data_loss.py b/tests/test_issue1361_cancel_data_loss.py index cdf89dcd..7d74b661 100644 --- a/tests/test_issue1361_cancel_data_loss.py +++ b/tests/test_issue1361_cancel_data_loss.py @@ -315,3 +315,52 @@ class TestCancelWithReasoningOnlyNoText: f"Expected no partial msg when nothing was streamed. Got partials: {partial_msgs}" assert len(cancel_msgs) == 1, \ f"Expected exactly 1 cancel marker. Got: {cancel_msgs}" + +# ── §D: Error paths must not lose pending user turn ───────────────────────── + +def test_stream_error_materializes_pending_user_turn_before_clearing_runtime_state(): + """If a stream errors before normal merge, pending_user_message must become a + durable user message before the error marker is saved. Otherwise reload/server + reconcile makes the user's just-submitted prompt disappear. + """ + from api.streaming import _materialize_pending_user_turn_before_error + + sid = "test_pending_error_d1" + s = _make_session( + session_id=sid, + pending_msg="please restart the WebUI", + messages=[{"role": "assistant", "content": "previous answer"}], + ) + s.pending_started_at = 1778098700.0 + s.pending_attachments = [{"name": "screenshot.png"}] + + appended = _materialize_pending_user_turn_before_error(s) + + assert appended is True + assert s.messages[-1]["role"] == "user" + assert s.messages[-1]["content"] == "please restart the WebUI" + assert s.messages[-1]["timestamp"] == 1778098700 + assert s.messages[-1]["attachments"] == [{"name": "screenshot.png"}] + assert s.pending_user_message == "please restart the WebUI" + + +def test_stream_error_pending_materialization_does_not_duplicate_eager_checkpoint(): + """Eager session-save mode may already have checkpointed the current user turn; + the error materializer must not append the same user message again. + """ + from api.streaming import _materialize_pending_user_turn_before_error + + sid = "test_pending_error_d2" + s = _make_session( + session_id=sid, + pending_msg="please restart the WebUI", + messages=[ + {"role": "assistant", "content": "previous answer"}, + {"role": "user", "content": "please restart the WebUI"}, + ], + ) + + appended = _materialize_pending_user_turn_before_error(s) + + assert appended is False + assert [m.get("role") for m in s.messages].count("user") == 1 From 98a6f88ef7f8e038132cdca526407afa1958dcad Mon Sep 17 00:00:00 2001 From: Dennis Soong Date: Thu, 7 May 2026 05:56:17 +0800 Subject: [PATCH 2/7] fix: scope terminal stream cleanup to owner session --- static/messages.js | 99 +++++++++++-------- tests/test_1694_terminal_cleanup_ownership.py | 93 +++++++++++++++++ ...t_issue856_background_completion_unread.py | 8 +- tests/test_regressions.py | 9 +- ...st_session_runtime_ownership_invariants.py | 26 +++-- tests/test_sprint30.py | 6 +- tests/test_sprint36.py | 13 ++- 7 files changed, 198 insertions(+), 56 deletions(-) create mode 100644 tests/test_1694_terminal_cleanup_ownership.py diff --git a/static/messages.js b/static/messages.js index 44262a85..8756fb93 100644 --- a/static/messages.js +++ b/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){ + 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(); } diff --git a/tests/test_1694_terminal_cleanup_ownership.py b/tests/test_1694_terminal_cleanup_ownership.py new file mode 100644 index 00000000..57fea1d1 --- /dev/null +++ b/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 diff --git a/tests/test_issue856_background_completion_unread.py b/tests/test_issue856_background_completion_unread.py index 0262ee08..47a7da6c 100644 --- a/tests/test_issue856_background_completion_unread.py +++ b/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" ) diff --git a/tests/test_regressions.py b/tests/test_regressions.py index 444ec66e..cbd50faf 100644 --- a/tests/test_regressions.py +++ b/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): diff --git a/tests/test_session_runtime_ownership_invariants.py b/tests/test_session_runtime_ownership_invariants.py index ae84abe8..68e1a10c 100644 --- a/tests/test_session_runtime_ownership_invariants.py +++ b/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." diff --git a/tests/test_sprint30.py b/tests/test_sprint30.py index cec2bc8a..92e0e360 100644 --- a/tests/test_sprint30.py +++ b/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() diff --git a/tests/test_sprint36.py b/tests/test_sprint36.py index 7c26b83e..00d540b1 100644 --- a/tests/test_sprint36.py +++ b/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 ───────────────────────────────────────────────────── From d5a31a0f4d77a9317db0c2b8f93fb5996185ab15 Mon Sep 17 00:00:00 2001 From: Nick Date: Wed, 6 May 2026 11:49:26 -0700 Subject: [PATCH 3/7] fix: isolate profile cookie per webui instance --- api/helpers.py | 24 ++++++++++++++++-------- tests/test_issue803.py | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 8 deletions(-) diff --git a/api/helpers.py b/api/helpers.py index f6c8b584..fd0c1983 100644 --- a/api/helpers.py +++ b/api/helpers.py @@ -2,6 +2,7 @@ Hermes Web UI -- HTTP helper functions. """ import json as _json +import os import re as _re from pathlib import Path from api.config import IMAGE_EXTS, MD_EXTS @@ -252,8 +253,13 @@ def read_body(handler) -> dict: PROFILE_COOKIE_NAME = 'hermes_profile' +def get_profile_cookie_name() -> str: + """Return the cookie name used to persist the active WebUI profile.""" + return os.getenv('WEBUI_PROFILE_COOKIE_NAME', PROFILE_COOKIE_NAME) + + def get_profile_cookie(handler) -> str | None: - """Extract the hermes_profile cookie value from the request, or None.""" + """Extract the active-profile cookie value from the request, or None.""" cookie_header = handler.headers.get('Cookie', '') if not cookie_header: return None @@ -263,7 +269,8 @@ def get_profile_cookie(handler) -> str | None: cookie.load(cookie_header) except _hc.CookieError: return None - morsel = cookie.get(PROFILE_COOKIE_NAME) + cookie_name = get_profile_cookie_name() + morsel = cookie.get(cookie_name) if morsel and morsel.value: # Validate against profile-name pattern before trusting from api.profiles import _PROFILE_ID_RE @@ -274,7 +281,7 @@ def get_profile_cookie(handler) -> str | None: def build_profile_cookie(name: str) -> str: - """Build a Set-Cookie header value for the hermes_profile cookie. + """Build a Set-Cookie header value for the active-profile cookie. Always persist the selected profile in the cookie, including 'default'. Clearing the cookie causes the backend to fall back to process-global @@ -287,8 +294,9 @@ def build_profile_cookie(name: str) -> str: """ import http.cookies as _hc cookie = _hc.SimpleCookie() - cookie[PROFILE_COOKIE_NAME] = name - cookie[PROFILE_COOKIE_NAME]['path'] = '/' - cookie[PROFILE_COOKIE_NAME]['httponly'] = True - cookie[PROFILE_COOKIE_NAME]['samesite'] = 'Lax' - return cookie[PROFILE_COOKIE_NAME].OutputString() + cookie_name = get_profile_cookie_name() + cookie[cookie_name] = name + cookie[cookie_name]['path'] = '/' + cookie[cookie_name]['httponly'] = True + cookie[cookie_name]['samesite'] = 'Lax' + return cookie[cookie_name].OutputString() diff --git a/tests/test_issue803.py b/tests/test_issue803.py index f882e14d..e1a2698e 100644 --- a/tests/test_issue803.py +++ b/tests/test_issue803.py @@ -73,6 +73,38 @@ class TestProfileCookieHelpers: result = get_profile_cookie(handler) assert result is None + def test_profile_cookie_name_defaults_to_hermes_profile(self, monkeypatch): + from api.helpers import build_profile_cookie + + monkeypatch.delenv('WEBUI_PROFILE_COOKIE_NAME', raising=False) + + s = build_profile_cookie('alice') + assert 'hermes_profile=alice' in s + + def test_profile_cookie_name_can_be_isolated_per_webui_instance(self, monkeypatch): + from api.helpers import build_profile_cookie, get_profile_cookie + + monkeypatch.setenv('WEBUI_PROFILE_COOKIE_NAME', 'hermes_profile_social') + + s = build_profile_cookie('writer') + assert 'hermes_profile_social=writer' in s + assert 'hermes_profile=writer' not in s + + handler = MagicMock() + handler.headers.get = lambda k, d='': ( + 'hermes_profile=wrong; hermes_profile_social=writer' if k == 'Cookie' else d + ) + assert get_profile_cookie(handler) == 'writer' + + def test_configured_profile_cookie_ignores_default_cookie_name(self, monkeypatch): + from api.helpers import get_profile_cookie + + monkeypatch.setenv('WEBUI_PROFILE_COOKIE_NAME', 'hermes_profile_main') + + handler = MagicMock() + handler.headers.get = lambda k, d='': 'hermes_profile=social_profile' if k == 'Cookie' else d + assert get_profile_cookie(handler) is None + # ── 2. Thread-local request context ────────────────────────────────────────── From eab39f14dbf5711f913d7bd8e4fb6b3cca29863a Mon Sep 17 00:00:00 2001 From: skspade Date: Wed, 6 May 2026 14:28:03 -0400 Subject: [PATCH 4/7] fix: gateway status card shows 'not running' when no platforms connected Use agent_health.build_agent_health_payload() as the authoritative running signal instead of bool(identity_map). An empty identity_map means zero connected messaging platforms, not that the gateway is down. Falls back to identity_map heuristic when agent_health module is unavailable (e.g. WebUI-only deployments). --- api/routes.py | 25 ++- tests/test_gateway_status_agent_health.py | 223 ++++++++++++++++++++++ 2 files changed, 247 insertions(+), 1 deletion(-) create mode 100644 tests/test_gateway_status_agent_health.py diff --git a/api/routes.py b/api/routes.py index 1477e5b0..ff65e39e 100644 --- a/api/routes.py +++ b/api/routes.py @@ -3291,7 +3291,30 @@ def handle_get(handler, parsed) -> bool: import datetime identity_map = _load_gateway_session_identity_map() sessions_path = _gateway_session_metadata_path() - running = bool(identity_map) + + # Detect whether the gateway process is alive, independent of + # connected messaging platforms. An empty identity_map just + # means zero platforms connected, not that the gateway is down. + # + # agent_health.build_agent_health_payload() is the authoritative + # signal: it reads gateway.status runtime metadata and returns a + # tri-state `alive` field (True/False/None). This avoids the + # false-negative where the gateway is running but has zero active + # messaging sessions (empty identity_map). + running = False + try: + health = build_agent_health_payload() + running = health.get("alive") is True + except Exception: + # Fall back to identity_map heuristic when the agent_health + # module is not available (e.g. WebUI-only deployments). + import logging + logging.getLogger("routes").warning( + "agent_health.build_agent_health_payload() raised; " + "falling back to identity_map heuristic for gateway status" + ) + running = bool(identity_map) + platforms_set: set[str] = set() for meta in identity_map.values(): raw = meta.get("raw_source") or meta.get("platform") or "" diff --git a/tests/test_gateway_status_agent_health.py b/tests/test_gateway_status_agent_health.py new file mode 100644 index 00000000..d209ba27 --- /dev/null +++ b/tests/test_gateway_status_agent_health.py @@ -0,0 +1,223 @@ +"""Regression coverage: /api/gateway/status uses agent_health payload as +the authoritative 'running' signal (#d0568682 / parent review t_9098e3db). + +Before the fix, the handler called gateway.status.get_running_pid() directly +and fell back to bool(identity_map) when the module was unavailable. The fix +makes it consult agent_health.build_agent_health_payload() so the tri-state +`alive` field is the single source of truth for gateway process health. + +Tests use handle_get + monkeypatched build_agent_health_payload() and +_load_gateway_session_identity_map() to isolate the gateway status route +from real filesystem state. +""" + +from __future__ import annotations + +import json +from urllib.parse import urlparse + + +# ── FakeHandler (mirrors test_1560_password_env_var_no_op._FakeHandler) ──────── + +class _FakeHandler: + """Minimal BaseHTTPRequestHandler stand-in for routes.handle_get.""" + + def __init__(self): + self.status = None + self.sent_headers: list[tuple[str, str]] = [] + self.body = bytearray() + self.wfile = self + + def send_response(self, code): + self.status = code + + def send_header(self, key, value): + self.sent_headers.append((key, value)) + + def end_headers(self): + pass + + def write(self, data): + """Accumulate bytes written to wfile.""" + self.body.extend(data if isinstance(data, (bytes, bytearray)) else data.encode("utf-8")) + + def get_json(self): + """Parse the accumulated body as JSON.""" + return json.loads(self.body.decode("utf-8")) + + +# ── Helpers ────────────────────────────────────────────────────────────────── + +def _call_gateway_status(monkeypatch, agent_health_alive, identity_map=None): + """Invoke handle_get for /api/gateway/status and return the parsed JSON. + + monkeypatches build_agent_health_payload to return the given `alive` value + and _load_gateway_session_identity_map to return the given identity_map. + """ + from api import routes + + monkeypatch.setattr( + routes, + "build_agent_health_payload", + lambda: { + "alive": agent_health_alive, + "checked_at": "2026-05-06T12:00:00+00:00", + "details": {}, + }, + ) + + if identity_map is not None: + monkeypatch.setattr( + routes, + "_load_gateway_session_identity_map", + lambda: identity_map, + ) + + handler = _FakeHandler() + parsed = urlparse("http://example.com/api/gateway/status") + routes.handle_get(handler, parsed) + return handler.get_json() + + +# ── Acceptance criteria tests ───────────────────────────────────────────────── + +def test_gateway_status_running_true_when_agent_health_alive_and_no_sessions(monkeypatch): + """AC1: alive=true + empty identity_map → running=true, platforms=[]""" + result = _call_gateway_status(monkeypatch, agent_health_alive=True, identity_map={}) + assert result["running"] is True + assert result["platforms"] == [] + + +def test_gateway_status_running_false_when_agent_health_alive_false_and_no_sessions(monkeypatch): + """AC2: alive=false + empty identity_map → running=false, platforms=[]""" + result = _call_gateway_status(monkeypatch, agent_health_alive=False, identity_map={}) + assert result["running"] is False + assert result["platforms"] == [] + + +def test_gateway_status_running_false_when_agent_health_alive_none_and_no_sessions(monkeypatch): + """AC2 (extended): alive=None + empty identity_map → running=false, platforms=[]""" + result = _call_gateway_status(monkeypatch, agent_health_alive=None, identity_map={}) + assert result["running"] is False + assert result["platforms"] == [] + + +def test_gateway_status_running_true_and_platforms_when_agent_health_alive_and_sessions(monkeypatch): + """AC3: alive=true + sessions with platforms → running=true, platforms populated""" + identity_map = { + "sess_a": {"raw_source": "telegram", "platform": "telegram"}, + "sess_b": {"raw_source": "discord", "platform": "discord"}, + } + result = _call_gateway_status(monkeypatch, agent_health_alive=True, identity_map=identity_map) + assert result["running"] is True + assert len(result["platforms"]) == 2 + names = {p["name"] for p in result["platforms"]} + assert names == {"telegram", "discord"} + + +# ── Edge case tests ─────────────────────────────────────────────────────────── + +def test_gateway_status_handles_agent_health_unavailable_fallback_to_sessions(monkeypatch): + """Edge: agent_health raises → fall back to session-only detection (current behavior).""" + from api import routes + + monkeypatch.setattr( + routes, + "build_agent_health_payload", + lambda: (_ for _ in ()).throw(RuntimeError("gateway module busted")), + ) + monkeypatch.setattr( + routes, + "_load_gateway_session_identity_map", + lambda: {"sess_c": {"raw_source": "telegram", "platform": "telegram"}}, + ) + + handler = _FakeHandler() + parsed = urlparse("http://example.com/api/gateway/status") + routes.handle_get(handler, parsed) + result = handler.get_json() + # With sessions present, fallback should report running=true + assert result["running"] is True + + +def test_gateway_status_handles_corrupted_sessions_json(monkeypatch): + """Edge: sessions.json is corrupted → identity_map empty, rely on agent_health alone.""" + from api import routes + + monkeypatch.setattr( + routes, + "build_agent_health_payload", + lambda: {"alive": True, "checked_at": "2026-05-06T12:00:00+00:00", "details": {}}, + ) + # _load_gateway_session_identity_map already returns {} on JSON parse failure; + # we monkeypatch it to return {} to simulate corrupted file. + monkeypatch.setattr(routes, "_load_gateway_session_identity_map", lambda: {}) + + handler = _FakeHandler() + parsed = urlparse("http://example.com/api/gateway/status") + routes.handle_get(handler, parsed) + result = handler.get_json() + assert result["running"] is True + assert result["platforms"] == [] + assert result["session_count"] == 0 + + +def test_gateway_status_blank_platform_fields_empty_platforms_running_true(monkeypatch): + """Edge: sessions exist but all have blank/missing platform fields → platforms=[], running=true.""" + from api import routes + + monkeypatch.setattr( + routes, + "build_agent_health_payload", + lambda: {"alive": True, "checked_at": "2026-05-06T12:00:00+00:00", "details": {}}, + ) + monkeypatch.setattr( + routes, + "_load_gateway_session_identity_map", + lambda: { + "sess_d": {"raw_source": "", "platform": ""}, + "sess_e": {}, # no platform field at all + }, + ) + + handler = _FakeHandler() + parsed = urlparse("http://example.com/api/gateway/status") + routes.handle_get(handler, parsed) + result = handler.get_json() + assert result["running"] is True + assert result["platforms"] == [] + + +# ── Existing behavior preservation tests ────────────────────────────────────── + +def test_gateway_status_running_false_when_agent_health_down_even_with_sessions(monkeypatch): + """When agent_health says alive=false, running should be false regardless of sessions.""" + from api import routes + + monkeypatch.setattr( + routes, + "build_agent_health_payload", + lambda: {"alive": False, "checked_at": "2026-05-06T12:00:00+00:00", "details": {}}, + ) + monkeypatch.setattr( + routes, + "_load_gateway_session_identity_map", + lambda: {"sess_f": {"raw_source": "telegram", "platform": "telegram"}}, + ) + + handler = _FakeHandler() + parsed = urlparse("http://example.com/api/gateway/status") + routes.handle_get(handler, parsed) + result = handler.get_json() + # Running should be false even though sessions exist — agent_health is authoritative + assert result["running"] is False + # But platforms should still be extracted from sessions + assert len(result["platforms"]) == 1 + assert result["platforms"][0]["name"] == "telegram" + + +def test_gateway_status_missing_r_field_handled_by_frontend(monkeypatch): + """Edge: response still has running field; frontend handles missing field via catch block. + This test verifies the backend always includes the 'running' field in responses.""" + result = _call_gateway_status(monkeypatch, agent_health_alive=True, identity_map={}) + assert "running" in result \ No newline at end of file From 7193cee1523e19e09ca8999436b4fb56d6dde8b5 Mon Sep 17 00:00:00 2001 From: skspade Date: Wed, 6 May 2026 15:36:07 -0400 Subject: [PATCH 5/7] =?UTF-8?q?fix:=20tri-state=20gateway=20status=20?= =?UTF-8?q?=E2=80=94=20distinguish=20not-configured=20from=20not-running?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Backend: return `configured` field alongside `running`. When alive=None (no gateway metadata), configured=false with fallback to identity_map heuristic. - Frontend: amber "Gateway not configured" when configured=false, red "Gateway not running" only when configured but process is down, green "Running" when both true. - Replace dead try/except fallback with explicit tri-state check on health["alive"]. - Add regression test for last_active guard when alive=true and identity_map is empty. All 87 gateway-related tests pass. --- api/routes.py | 29 ++++++++------ static/panels.js | 4 ++ tests/test_gateway_status_agent_health.py | 46 +++++++++++++++++------ 3 files changed, 56 insertions(+), 23 deletions(-) diff --git a/api/routes.py b/api/routes.py index ff65e39e..9016636f 100644 --- a/api/routes.py +++ b/api/routes.py @@ -3301,19 +3301,23 @@ def handle_get(handler, parsed) -> bool: # tri-state `alive` field (True/False/None). This avoids the # false-negative where the gateway is running but has zero active # messaging sessions (empty identity_map). - running = False - try: - health = build_agent_health_payload() - running = health.get("alive") is True - except Exception: - # Fall back to identity_map heuristic when the agent_health - # module is not available (e.g. WebUI-only deployments). - import logging - logging.getLogger("routes").warning( - "agent_health.build_agent_health_payload() raised; " - "falling back to identity_map heuristic for gateway status" - ) + # + # `alive` tri-state semantics: + # True → gateway process is alive + # False → gateway metadata exists but process is down + # None → no gateway metadata/status available; this WebUI + # setup is probably not configured with a gateway + health = build_agent_health_payload() + alive = health.get("alive") + if alive is True: + running = True + configured = True + elif alive is False: + running = False + configured = True + else: # alive is None → gateway not configured / unavailable running = bool(identity_map) + configured = False platforms_set: set[str] = set() for meta in identity_map.values(): @@ -3341,6 +3345,7 @@ def handle_get(handler, parsed) -> bool: pass return j(handler, { "running": running, + "configured": configured, "platforms": platforms, "last_active": last_active, "session_count": len(identity_map), diff --git a/static/panels.js b/static/panels.js index 12531130..be20e724 100644 --- a/static/panels.js +++ b/static/panels.js @@ -5413,6 +5413,10 @@ function loadGatewayStatus(){ if(!card) return; api('/api/gateway/status').then(r=>{ if(!r) return; + if(!r.configured){ + card.innerHTML=`
Gateway not configured
`; + return; + } if(!r.running){ card.innerHTML=`
Gateway not running
`; return; diff --git a/tests/test_gateway_status_agent_health.py b/tests/test_gateway_status_agent_health.py index d209ba27..843f7cd5 100644 --- a/tests/test_gateway_status_agent_health.py +++ b/tests/test_gateway_status_agent_health.py @@ -82,34 +82,39 @@ def _call_gateway_status(monkeypatch, agent_health_alive, identity_map=None): # ── Acceptance criteria tests ───────────────────────────────────────────────── def test_gateway_status_running_true_when_agent_health_alive_and_no_sessions(monkeypatch): - """AC1: alive=true + empty identity_map → running=true, platforms=[]""" + """AC1: alive=true + empty identity_map → running=true, configured=true, platforms=[]""" result = _call_gateway_status(monkeypatch, agent_health_alive=True, identity_map={}) assert result["running"] is True + assert result["configured"] is True assert result["platforms"] == [] def test_gateway_status_running_false_when_agent_health_alive_false_and_no_sessions(monkeypatch): - """AC2: alive=false + empty identity_map → running=false, platforms=[]""" + """AC2: alive=false + empty identity_map → running=false, configured=true, platforms=[]""" result = _call_gateway_status(monkeypatch, agent_health_alive=False, identity_map={}) assert result["running"] is False + assert result["configured"] is True assert result["platforms"] == [] def test_gateway_status_running_false_when_agent_health_alive_none_and_no_sessions(monkeypatch): - """AC2 (extended): alive=None + empty identity_map → running=false, platforms=[]""" + """When alive=None (not configured): fall back to identity_map heuristic, + and set configured=false so frontend can show 'not configured' state.""" result = _call_gateway_status(monkeypatch, agent_health_alive=None, identity_map={}) assert result["running"] is False + assert result["configured"] is False assert result["platforms"] == [] def test_gateway_status_running_true_and_platforms_when_agent_health_alive_and_sessions(monkeypatch): - """AC3: alive=true + sessions with platforms → running=true, platforms populated""" + """AC3: alive=true + sessions with platforms → running=true, configured=true, platforms populated""" identity_map = { "sess_a": {"raw_source": "telegram", "platform": "telegram"}, "sess_b": {"raw_source": "discord", "platform": "discord"}, } result = _call_gateway_status(monkeypatch, agent_health_alive=True, identity_map=identity_map) assert result["running"] is True + assert result["configured"] is True assert len(result["platforms"]) == 2 names = {p["name"] for p in result["platforms"]} assert names == {"telegram", "discord"} @@ -117,14 +122,15 @@ def test_gateway_status_running_true_and_platforms_when_agent_health_alive_and_s # ── Edge case tests ─────────────────────────────────────────────────────────── -def test_gateway_status_handles_agent_health_unavailable_fallback_to_sessions(monkeypatch): - """Edge: agent_health raises → fall back to session-only detection (current behavior).""" +def test_gateway_status_alive_none_falls_back_to_identity_map_heuristic(monkeypatch): + """When alive=None (not configured) but sessions exist, running reflects identity_map. + configured=false tells the frontend to show 'not configured' state.""" from api import routes monkeypatch.setattr( routes, "build_agent_health_payload", - lambda: (_ for _ in ()).throw(RuntimeError("gateway module busted")), + lambda: {"alive": None, "checked_at": "2026-05-06T12:00:00+00:00", "details": {}}, ) monkeypatch.setattr( routes, @@ -136,8 +142,10 @@ def test_gateway_status_handles_agent_health_unavailable_fallback_to_sessions(mo parsed = urlparse("http://example.com/api/gateway/status") routes.handle_get(handler, parsed) result = handler.get_json() - # With sessions present, fallback should report running=true + # Fallback to identity_map: sessions exist → running=true assert result["running"] is True + # But configured=false because alive was None (no gateway metadata) + assert result["configured"] is False def test_gateway_status_handles_corrupted_sessions_json(monkeypatch): @@ -211,13 +219,29 @@ def test_gateway_status_running_false_when_agent_health_down_even_with_sessions( result = handler.get_json() # Running should be false even though sessions exist — agent_health is authoritative assert result["running"] is False + # But configured=true because alive=False means gateway metadata exists + assert result["configured"] is True # But platforms should still be extracted from sessions assert len(result["platforms"]) == 1 assert result["platforms"][0]["name"] == "telegram" def test_gateway_status_missing_r_field_handled_by_frontend(monkeypatch): - """Edge: response still has running field; frontend handles missing field via catch block. - This test verifies the backend always includes the 'running' field in responses.""" + """Edge: response always has 'running' and 'configured' fields. + Frontend handles missing field via catch block. This test verifies the backend + always includes both fields in responses.""" result = _call_gateway_status(monkeypatch, agent_health_alive=True, identity_map={}) - assert "running" in result \ No newline at end of file + assert "running" in result + assert "configured" in result + + +def test_gateway_status_last_active_empty_when_alive_and_no_sessions_path(monkeypatch): + """Bonus: alive=true + identity_map={} → last_active is empty string. + This guards the 'if running and sessions_path.exists()' guard from being + silently removed in a future refactor that might expose a stale timestamp.""" + result = _call_gateway_status(monkeypatch, agent_health_alive=True, identity_map={}) + assert result["running"] is True + assert result["configured"] is True + # In test context, sessions_path won't exist (no real filesystem), + # so last_active must be empty. + assert result["last_active"] == "" \ No newline at end of file From fc5423f4aa2ae8b87116573775c2d6f02f8db56b Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Wed, 6 May 2026 22:02:37 +0000 Subject: [PATCH 6/7] auto-fix: preserve _setActivePaneIdleIfOwner permissive-fallback disjunct from PR #1753 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #1753 (shipped v0.51.12) introduced the 3-way OR guard in done/error/cancel handlers: 'isActiveSession || !S.session || !INFLIGHT[S.session.session_id]'. The third disjunct ('no other inflight on the active pane') is the permissive fallback Opus stage-306 verified — it allows the active pane to idle when no other session is running, even when the completing stream is from a different session. PR #1761's centralizing helper _setActivePaneIdleIfOwner inadvertently dropped this disjunct, so a user viewing pane A (idle) while pane B completes in the background would not get pane A's composer state cleared. Restored: _setActivePaneIdleIfOwner now checks the same 3-way OR. Verified via: - node -c static/messages.js — clean - pytest tests/test_session_runtime_ownership_invariants.py tests/test_1694_terminal_cleanup_ownership.py — 9 passed Co-authored-by: dso2ng --- static/messages.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/static/messages.js b/static/messages.js index 8756fb93..03698087 100644 --- a/static/messages.js +++ b/static/messages.js @@ -382,7 +382,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ _clearActivePaneInflightIfOwner(); } function _setActivePaneIdleIfOwner(){ - if(_isActiveSession()||!S.session){ + if(_isActiveSession()||!S.session||!INFLIGHT[S.session.session_id]){ setBusy(false); setComposerStatus(''); if(typeof setStatus==='function') setStatus(''); From e8659d1a40b3c7a284b425a28e73f71421cb0727 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Wed, 6 May 2026 22:20:17 +0000 Subject: [PATCH 7/7] =?UTF-8?q?chore(release):=20stamp=20v0.51.14=20?= =?UTF-8?q?=E2=80=94=204-PR=20contributor=20batch=20(#1756,=20#1757,=20#17?= =?UTF-8?q?60,=20#1761)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Constituent PRs: - #1760 (@ai-ag2026) preserve pending user turn on stream errors. Closes #1361. - #1761 (@dso2ng) scope terminal stream cleanup to owner session. Refs #1694. AUTO-FIX applied: restored !INFLIGHT[S.session.session_id] disjunct in _setActivePaneIdleIfOwner (regression introduced by helper centralization). - #1756 (@ng-technology-llc) isolate profile cookie per webui instance. Closes #803. - #1757 (@skspade) tri-state gateway status (alive: True/False/None). Tests: 4642 → 4662 collected (+20). 4649 passed, 9 skipped (test-isolation prong-2 noise), 3 xpassed, 0 failed in 152s. Pre-release verification: - All 4 PRs CI-green or rebased clean (#1757 had stale base; CHANGELOG conflict auto-resolved by dropping the PR's redundant entry). - node -c clean on static/messages.js + static/panels.js. - 11/11 browser API endpoints PASS. - Pre-stamp re-fetch: all PR heads match local rebases. - Opus advisor: SHIP, all 5 verification questions clean, 0 MUST-FIX, 0 SHOULD-FIX. - Two NICE-TO-HAVE coverage gaps absorbed in-release: (1) test_sprint36.py asserts !INFLIGHT[...] disjunct in helper body (2) test_issue1361_cancel_data_loss.py adds structural-grep test to pin _materialize_pending_user_turn_before_error call sites at error branches. Closes #803, #1361, #1694. --- CHANGELOG.md | 25 ++++++++++++++ ROADMAP.md | 2 +- TESTING.md | 4 +-- tests/test_issue1361_cancel_data_loss.py | 43 ++++++++++++++++++++++++ tests/test_sprint36.py | 11 ++++++ 5 files changed, 82 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e5343ca2..2f4613fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,30 @@ # Hermes Web UI -- Changelog +## [v0.51.14] — 2026-05-06 — 4-PR contributor batch + +### Fixed + +- **PR #1760** by @ai-ag2026 — Preserve pending user turn on stream errors. Adds reconciliation in `api/streaming.py` so the user's pending turn is appended (with timestamp + attachments) BEFORE runtime state is cleared on `apperror`-no-response and outer-Exception paths. Reload + session reconcile now see the turn instead of losing it. Includes `_materialize_pending_user_turn_before_error()` helper with dedup against eager-checkpointed messages (8-message lookback, whitespace-normalized comparison). Closes #1361. +- **PR #1761** by @dso2ng — Scope terminal stream cleanup to owner session (refs #1694). Centralizes owner-only cleanup behind helpers (`_setActivePaneIdleIfOwner`, `_clearOwnerInflightState`, `_clearApprovalForOwner`, `_clearClarifyForOwner`) at SSE `done`/`error`/`cancel` event handlers in `static/messages.js`. Replaces inline 3-way OR guards introduced by PR #1753 (v0.51.12) with structured helper calls. The actual #1694 bug fix is in `_clearActivePaneInflightIfOwner`, which now gates `clearInflight()` on `_isActiveSession()` — previously unconditional, so a background completion would inadvertently clear the global `INFLIGHT_KEY` localStorage marker for the active pane. **Auto-fix applied**: PR's centralizing helper inadvertently dropped the `!INFLIGHT[S.session.session_id]` permissive-fallback disjunct from #1753; restored in `_setActivePaneIdleIfOwner` so the helper preserves the same 3-way OR contract Opus stage-306 verified. +- **PR #1756** by @ng-technology-llc — Isolate profile cookie per webui instance (closes #803). Adds `WEBUI_PROFILE_COOKIE_NAME` env var so multi-instance WebUI deployments can isolate the active-profile cookie per process. Default cookie name `hermes_profile` preserved when env var not set; backwards-compatible. `get_profile_cookie_name()` resolves per-request via `os.getenv()` so deployments can change the env var without restart (existing client cookies under the old name are treated as no cookie → user re-selects profile, no data loss). +- **PR #1757** by @skspade — Tri-state gateway status (closes earlier "gateway shows 'not running' when no platforms connected" reports). Replaces `bool(identity_map)` running signal with `agent_health.build_agent_health_payload()` as the authoritative source. Adds `alive: True/False/None` + `configured: bool` + `running: bool` fields. Frontend `static/panels.js` distinguishes three states: green "running" / amber "Gateway not configured" / red "not running". `build_agent_health_payload()` is robust to every failure (gateway import error, runtime status read exception, missing PID) — silently nulls and never raises. 247 LOC test coverage in `tests/test_gateway_status_agent_health.py`. + +### Tests + +4642 → **4662 collected** (+20 across 4 new test files plus regression coverage tightening). Includes 2 new structural-grep regression tests absorbed in-release per Opus advisor's NICE-TO-HAVE follow-ups: (1) `tests/test_sprint36.py` now asserts `_setActivePaneIdleIfOwner` body contains the `!INFLIGHT[...]` disjunct (catches the auto-fix repaired regression in #1761); (2) `tests/test_issue1361_cancel_data_loss.py` adds `test_materialize_helper_called_immediately_before_error_path_clears` to pin the helper call's call-site location in `api/streaming.py` error branches (catches future refactor that drops the call but keeps the clearing). + +### Pre-release verification + +- All 4 PRs CI-green individually (#1760, #1761) or rebased clean (#1756, #1757 — #1757 had stale base from before v0.51.10 stamps; CHANGELOG conflict auto-resolved by dropping the PR's redundant changelog entry, since we write the v0.51.14 entry at stamp time). +- Auto-fix on #1761 verified by 9-test pass before merge (5 invariants + 4 new ownership tests). +- `node -c` clean on both `static/messages.js` and `static/panels.js`. +- pytest: 4649 passed, 0 failed (single clean run, ~152s). +- `scripts/run-browser-tests.sh`: all 11 endpoints PASS on isolated port 8789. +- Pre-stamp re-fetch: all 4 PR heads still match local rebases — no late commits. +- Opus advisor: SHIP all 4, all 5 verification questions clean, 0 MUST-FIX, 0 SHOULD-FIX. Two NICE-TO-HAVE coverage gaps absorbed in-release as ~30 LOC of defensive structural-grep regression tests (covered above). + +Closes #803, #1361, #1694. + ## [v0.51.13] — 2026-05-06 — single-PR composer UX ### Added diff --git a/ROADMAP.md b/ROADMAP.md index 803c49ae..c0517281 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2,7 +2,7 @@ > Web companion to the Hermes Agent CLI. Same workflows, browser-native. > -> Last updated: v0.51.13 (May 6, 2026) — 4642 tests collected — single-PR composer UX (#1758) +> Last updated: v0.51.14 (May 6, 2026) — 4662 tests collected — 4-PR contributor batch (#1756, #1757, #1760, #1761) > Test source: `pytest tests/ --collect-only -q` > Per-version detail: see [CHANGELOG.md](./CHANGELOG.md) diff --git a/TESTING.md b/TESTING.md index be959df9..1fa48fe5 100644 --- a/TESTING.md +++ b/TESTING.md @@ -1835,8 +1835,8 @@ Bridged CLI sessions: --- -*Last updated: v0.51.13, May 6, 2026* -*Total automated tests collected: 4642* +*Last updated: v0.51.14, May 6, 2026* +*Total automated tests collected: 4662* *Regression gate: tests/test_regressions.py* *Run: pytest tests/ -v --timeout=60* *Source: /* diff --git a/tests/test_issue1361_cancel_data_loss.py b/tests/test_issue1361_cancel_data_loss.py index 7d74b661..3e19291a 100644 --- a/tests/test_issue1361_cancel_data_loss.py +++ b/tests/test_issue1361_cancel_data_loss.py @@ -364,3 +364,46 @@ def test_stream_error_pending_materialization_does_not_duplicate_eager_checkpoin assert appended is False assert [m.get("role") for m in s.messages].count("user") == 1 + + +# ── Structural guard: pin call sites of the materialize helper at error branches ── + +def test_materialize_helper_called_immediately_before_error_path_clears(): + """Pin call sites of _materialize_pending_user_turn_before_error. + + Catches a future refactor that drops the call from the apperror-no-response + or outer-Exception paths in api/streaming.py while leaving the + `pending_user_message = None` clearing in place — which is exactly the + user-turn-data-loss regression #1361 was filed for. + + Strategy: count how many `pending_user_message = None` clearings have the + helper call within the preceding 4 lines. Currently 2 (apperror at 2610, + outer-Exception at 3072). The success path (2716) and cancel path (3375) + legitimately don't need the helper. If a future refactor drops the helper + call from one of the error sites, this assertion fires. + """ + from pathlib import Path + src = Path(__file__).parent.parent.joinpath('api', 'streaming.py').read_text(encoding='utf-8') + lines = src.splitlines() + + helper_name = '_materialize_pending_user_turn_before_error(' + clear_sites = [(i + 1, line) for i, line in enumerate(lines) + if 'pending_user_message = None' in line] + assert len(clear_sites) >= 4, ( + f"Expected ≥4 sites that clear pending_user_message; found {len(clear_sites)}. " + f"If api/streaming.py was refactored, re-audit this test." + ) + + sites_with_helper = [] + for lineno, _ in clear_sites: + prev_block = '\n'.join(lines[max(0, lineno - 5):lineno - 1]) + if helper_name in prev_block: + sites_with_helper.append(lineno) + + # Concretely, PR #1760 wired up the helper at the apperror-no-response + # path and the outer-Exception path. Both must remain wired. + assert len(sites_with_helper) >= 2, ( + f"Expected ≥2 clear sites preceded by {helper_name} within 4 lines; " + f"found {sites_with_helper}. PR #1760 / #1361 regression — re-wire the " + f"helper at the error-branch clear sites in api/streaming.py." + ) diff --git a/tests/test_sprint36.py b/tests/test_sprint36.py index 00d540b1..a5c2cbfe 100644 --- a/tests/test_sprint36.py +++ b/tests/test_sprint36.py @@ -178,6 +178,17 @@ def test_sse_cancel_handler_calls_set_busy(): 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 + # The helper MUST preserve the v0.51.12 (#1753) 3-way OR guard so + # idling the active pane on a background completion is gated on the + # permissive-fallback disjunct ("no other inflight on the active pane") + # in addition to "is active" / "no session". Without this, a user + # viewing pane A (idle) while pane B completes in the background + # would not get pane A's composer state cleared. Catches the exact + # regression v0.51.14's auto-fix repaired in PR #1761. + assert "!INFLIGHT[S.session.session_id]" in helper, ( + "_setActivePaneIdleIfOwner must preserve the !INFLIGHT[...] " + "permissive-fallback disjunct from PR #1753 (v0.51.12)." + ) # ── 7. i18n key preserved ─────────────────────────────────────────────────────