From b53bc1a2ea8620b144f28fa0d25af780c8bcdd81 Mon Sep 17 00:00:00 2001 From: AJV20 <24819659+AJV20@users.noreply.github.com> Date: Thu, 28 May 2026 20:15:54 -0400 Subject: [PATCH 1/3] fix: harden webui passive performance paths --- ARCHITECTURE.md | 1 + CHANGELOG.md | 8 ++ README.md | 12 ++- api/routes.py | 31 ++++--- api/updates.py | 82 ++++++++++++++---- static/messages.js | 16 +++- static/sessions.js | 45 +++++++++- static/ui.js | 21 ++++- tests/test_api_timeout.py | 3 + .../test_firefox_sidebar_scroll_stability.py | 18 ++-- ...t_issue856_background_completion_unread.py | 6 +- tests/test_live_activity_timeline.py | 18 +++- tests/test_performance_polling_hardening.py | 43 ++++++++++ ...test_stage326_composer_draft_validation.py | 18 +++- tests/test_update_banner_fixes.py | 84 +++++++++++++++++++ tests/test_updates.py | 4 +- 16 files changed, 358 insertions(+), 52 deletions(-) create mode 100644 tests/test_performance_polling_hardening.py diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 55884d71..98946a4e 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -125,6 +125,7 @@ Environment variables controlling behavior: HERMES_PREFILL_MESSAGES_FILE Optional JSON message list for browser-turn prefill context HERMES_WEBUI_PREFILL_MESSAGES_SCRIPT Optional command that prints JSON messages or plain-text user prefill context HERMES_WEBUI_PREFILL_MESSAGES_SCRIPT_TIMEOUT Optional script timeout in seconds (default 5, max 30) + HERMES_WEBUI_PREFILL_CONTEXT_MAX_CHARS Optional parsed prefill budget in characters (default 12000, 0 disables) HERMES_HOME Base directory for Hermes state (~/.hermes by default) Test isolation environment variables (set by conftest.py): diff --git a/CHANGELOG.md b/CHANGELOG.md index 3eb9b420..05418d43 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,14 @@ ## [Unreleased] +### Fixed + +- Coalesced duplicate in-flight sidebar/project refreshes and suppressed overlapping approval/clarify fallback polls, reducing repeated passive requests while preserving latest-refresh-wins sidebar state. +- Skipped duplicate composer-draft writes when the normalized autosave payload is unchanged, avoiding redundant full session JSON rewrites during debounced input/focus churn. +- Refined empty Activity waiting placeholders to distinguish stream creation, first-token wait, post-tool model wait, and running-tool wait states. +- Self-update restart safety now checks active agent runs as well as open SSE streams and waits for in-flight work before re-exec, avoiding update-triggered interruption when a run outlives its browser stream. +- Documented the WebUI prefill context budget in README and architecture notes so operators can keep new-browser-turn startup context compact. + ## [v0.51.163] — 2026-05-30 — Release EI (stage-batch45 — session duplicate/branch field propagation) ### Fixed diff --git a/README.md b/README.md index d5208b55..9ce29386 100644 --- a/README.md +++ b/README.md @@ -158,9 +158,15 @@ a `messages` list, or plain text; plain text is wrapped as one `user` prefill message so dynamic recall text becomes ordinary context instead of an extra system instruction. If the hook must provide system-level guidance, emit JSON messages with an explicit `role: "system"` entry instead. Script output is capped -at 256 KiB before parsing. The browser only receives a compact status event -(`source`, `label`, message count, and redacted errors), never the prefill -message bodies. +at 256 KiB before parsing. Parsed prefill context is then bounded by +`webui_prefill_context_max_chars` or `HERMES_WEBUI_PREFILL_CONTEXT_MAX_CHARS` +(default: 12,000 characters; set to `0` to disable). When a dynamic script +exceeds the budget and a compact static prefill file is configured, WebUI falls +back to that file. If no compact fallback is available, WebUI injects a short +retrieval instruction instead of sending the oversized note/body payload with +every new browser turn. The browser only receives a compact status event +(`source`, `label`, message count, compaction metadata, and redacted errors), +never the prefill message bodies. ### Optional Gateway-backed browser chat diff --git a/api/routes.py b/api/routes.py index a559f65f..71f29dad 100644 --- a/api/routes.py +++ b/api/routes.py @@ -5637,19 +5637,30 @@ def handle_post(handler, parsed) -> bool: s = get_session(sid) except KeyError: return bad(handler, "Session not found", 404) + unchanged = False with _get_session_agent_lock(sid): - draft = getattr(s, "composer_draft", {}) or {} + current_draft = dict(getattr(s, "composer_draft", {}) or {}) + next_draft = dict(current_draft) if text is not None: - draft["text"] = text + next_draft["text"] = text if files is not None: - draft["files"] = files - s.composer_draft = draft - # Draft persistence is not conversation activity. Touching updated_at - # here makes the active-session external-refresh poll force-reload the - # current chat every few seconds while the user is typing, and that - # delayed reload can restore an older draft over newer local input. - s.save(touch_updated_at=False, skip_index=True) - return j(handler, {"ok": True, "draft": s.composer_draft}) + next_draft["files"] = files + if next_draft == current_draft: + unchanged = True + saved_draft = current_draft + else: + s.composer_draft = next_draft + # Draft persistence is not conversation activity. Touching updated_at + # here makes the active-session external-refresh poll force-reload the + # current chat every few seconds while the user is typing, and that + # delayed reload can restore an older draft over newer local input. + s.save(touch_updated_at=False, skip_index=True) + saved_draft = s.composer_draft + payload = {"ok": True, "draft": saved_draft} + if unchanged: + payload["unchanged"] = True + j(handler, payload) + return True if parsed.path == "/api/session/update": try: diff --git a/api/updates.py b/api/updates.py index 93c6ce95..da207a32 100644 --- a/api/updates.py +++ b/api/updates.py @@ -60,32 +60,81 @@ def _sanitize_git_diagnostic(output: str, *, limit: int = _GIT_DIAGNOSTIC_MAX_CH return sanitized +def _restart_blocker_snapshot() -> dict: + """Return active chat work that should block a self-restart.""" + with STREAMS_LOCK: + stream_ids = [str(k) for k in STREAMS.keys()] + run_ids: list[str] = [] + try: + from api import config as _config + active_runs = getattr(_config, 'ACTIVE_RUNS', {}) + active_runs_lock = getattr(_config, 'ACTIVE_RUNS_LOCK', None) + if active_runs_lock is not None: + with active_runs_lock: + run_ids = [str(k) for k in active_runs.keys()] + else: + run_ids = [str(k) for k in active_runs.keys()] + except Exception: + run_ids = [] + return { + 'active_streams': len(stream_ids), + 'active_runs': len(run_ids), + 'blocking_stream_ids': stream_ids[:10], + 'blocking_run_ids': run_ids[:10], + 'restart_blocked': bool(stream_ids or run_ids), + } + + def _active_stream_count() -> int: """Return the current in-memory chat stream count. - Self-update schedules an in-process re-exec after git pull/reset. That is - restart-equivalent for live streams, even when systemd does not see a unit - restart. Refuse update/force-update while a stream exists so a browser - update click cannot recreate the pending-message loss class fixed in #1543. + Kept for compatibility with older tests/helpers; restart safety should use + ``_restart_blocker_snapshot()`` so detached worker runs also block updates. """ - with STREAMS_LOCK: - return len(STREAMS) + return int(_restart_blocker_snapshot().get('active_streams') or 0) -def _restart_blocked_response(target: str, active_streams: int) -> dict: - plural = "s" if active_streams != 1 else "" +def _restart_blocked_response(target: str, blocker_snapshot: dict | int) -> dict: + if isinstance(blocker_snapshot, int): + blocker_snapshot = { + 'active_streams': blocker_snapshot, + 'active_runs': 0, + 'blocking_stream_ids': [], + 'blocking_run_ids': [], + 'restart_blocked': bool(blocker_snapshot), + } + active_streams = int(blocker_snapshot.get('active_streams') or 0) + active_runs = int(blocker_snapshot.get('active_runs') or 0) + parts = [] + if active_streams: + parts.append(f"{active_streams} active chat stream{'s' if active_streams != 1 else ''}") + if active_runs: + parts.append(f"{active_runs} active agent run{'s' if active_runs != 1 else ''}") + detail = ' and '.join(parts) or 'active chat work' return { 'ok': False, 'message': ( - f'Cannot update {target} while {active_streams} active chat stream{plural} ' - 'is running. Wait for the response to finish, then retry the update.' + f'Cannot update {target} while {detail} is running. ' + 'Wait for the response to finish, then retry the update.' ), 'target': target, 'restart_blocked': True, 'active_streams': active_streams, + 'active_runs': active_runs, + 'blocking_stream_ids': blocker_snapshot.get('blocking_stream_ids') or [], + 'blocking_run_ids': blocker_snapshot.get('blocking_run_ids') or [], } +def _wait_until_restart_safe(poll_seconds: float = 2.0) -> dict: + """Wait for active work to finish before self-reexec.""" + snapshot = _restart_blocker_snapshot() + while snapshot.get('restart_blocked'): + time.sleep(max(0.1, poll_seconds)) + snapshot = _restart_blocker_snapshot() + return snapshot + + def _run_git(args, cwd, timeout=10): """Run a git command and return (useful output, ok). @@ -928,6 +977,7 @@ def _schedule_restart(delay: float = 2.0) -> None: # Threads die when execv replaces the process image, so the lock is # released atomically by the kernel. with _apply_lock: + _wait_until_restart_safe() try: os.execv(sys.executable, [sys.executable] + sys.argv) except Exception: @@ -950,9 +1000,9 @@ def apply_force_update(target: str) -> dict: response with ``conflict: True`` or ``diverged: True`` and the user has confirmed they want to discard local changes. """ - active_streams = _active_stream_count() - if active_streams: - return _restart_blocked_response(target, active_streams) + blocker_snapshot = _restart_blocker_snapshot() + if blocker_snapshot.get('restart_blocked'): + return _restart_blocked_response(target, blocker_snapshot) if not _apply_lock.acquire(blocking=False): return {'ok': False, 'message': 'Update already in progress'} @@ -1002,9 +1052,9 @@ def apply_force_update(target: str) -> dict: def apply_update(target): """Stash, pull --ff-only, pop for the given target repo.""" - active_streams = _active_stream_count() - if active_streams: - return _restart_blocked_response(target, active_streams) + blocker_snapshot = _restart_blocker_snapshot() + if blocker_snapshot.get('restart_blocked'): + return _restart_blocked_response(target, blocker_snapshot) if not _apply_lock.acquire(blocking=False): return {'ok': False, 'message': 'Update already in progress'} diff --git a/static/messages.js b/static/messages.js index 1c92a2ae..5af08e2f 100644 --- a/static/messages.js +++ b/static/messages.js @@ -564,6 +564,7 @@ async function send(){ } streamId=startData.stream_id; S.activeStreamId = streamId; + if(typeof appendThinking==='function') appendThinking('',{pending:true}); // setBusy(true) already ran with activeStreamId=null; refresh now that we // have a stream id so the primary button can switch to Stop (see getComposerPrimaryAction). if(typeof updateSendBtn==='function') updateSendBtn(); @@ -2507,6 +2508,7 @@ async function toggleYoloFromApproval() { // ── Approval polling ── let _approvalPollTimer = null; +let _approvalFallbackPollInFlight = false; let _approvalHideTimer = null; let _approvalVisibleSince = 0; let _approvalSignature = ''; @@ -2707,11 +2709,14 @@ function _startApprovalFallbackPoll(sid) { if (!S.busy || !S.session || S.session.session_id !== sid) { stopApprovalPolling(); _hideApprovalCardIfOwner(sid, true); return; } + if (_approvalFallbackPollInFlight) return; + _approvalFallbackPollInFlight = true; try { - const data = await api("/api/approval/pending?session_id=" + encodeURIComponent(sid)); + const data = await api("/api/approval/pending?session_id=" + encodeURIComponent(sid),{timeoutToast:false}); if (data.pending) { showApprovalForSession(sid, data.pending, data.pending_count||1); } else { _clearApprovalPendingForSession(sid); _hideApprovalCardIfOwner(sid); } } catch(e) { /* ignore poll errors */ } + finally { _approvalFallbackPollInFlight = false; } }, 1500); // matches the v0.50.247 polling cadence so degraded-mode users see the same responsiveness } @@ -2724,6 +2729,7 @@ function stopApprovalPolling() { if (_approvalPollTimer) { clearInterval(_approvalPollTimer); _approvalPollTimer = null; } if (_approvalEventSource) { try { _approvalEventSource.close(); } catch(_){} _approvalEventSource = null; } if (_approvalSSEHealthTimer) { clearInterval(_approvalSSEHealthTimer); _approvalSSEHealthTimer = null; } + _approvalFallbackPollInFlight = false; _approvalPollingSessionId = null; } @@ -3197,6 +3203,7 @@ async function respondClarify(response) { var _clarifyEventSource = null; var _clarifyFallbackTimer = null; var _clarifyHealthTimer = null; +let _clarifyFallbackPollInFlight = false; let _clarifyPollingSessionId = null; function startClarifyPolling(sid) { @@ -3265,8 +3272,10 @@ function _startClarifyFallbackPoll(sid) { if (!S.session || S.session.session_id !== sid) { stopClarifyPolling(); _hideClarifyCardIfOwner(sid, true, 'session'); return; } + if (_clarifyFallbackPollInFlight) return; + _clarifyFallbackPollInFlight = true; try { - const data = await api("/api/clarify/pending?session_id=" + encodeURIComponent(sid)); + const data = await api("/api/clarify/pending?session_id=" + encodeURIComponent(sid),{timeoutToast:false}); if (data.pending) { showClarifyForSession(sid, data.pending); } else { _clearClarifyPendingForSession(sid); _hideClarifyCardIfOwner(sid, false, 'expired'); } } catch(e) { @@ -3279,6 +3288,8 @@ function _startClarifyFallbackPoll(sid) { } stopClarifyPolling(); } + } finally { + _clarifyFallbackPollInFlight = false; } }, 3000); } @@ -3292,6 +3303,7 @@ function stopClarifyPolling() { if (_clarifyEventSource) { try { _clarifyEventSource.close(); } catch(_){} _clarifyEventSource = null; } if (_clarifyFallbackTimer) { clearInterval(_clarifyFallbackTimer); _clarifyFallbackTimer = null; } if (_clarifyHealthTimer) { clearInterval(_clarifyHealthTimer); _clarifyHealthTimer = null; } + _clarifyFallbackPollInFlight = false; _clarifyPollingSessionId = null; } diff --git a/static/sessions.js b/static/sessions.js index e14d1952..c76360db 100644 --- a/static/sessions.js +++ b/static/sessions.js @@ -2098,6 +2098,8 @@ window.addEventListener('resize',()=>{ // concurrently. Without this guard, a slower older response can overwrite _allSessions // with stale data, causing sessions to vanish from the sidebar. let _renderSessionListGen = 0; +let _renderSessionListInFlight = null; +let _renderSessionListQueuedRequest = null; let _sessionListRefreshAnimationPending = false; let _sessionListFirstRenderAnimated = false; let _sessionListEnterAllAnimationPending = false; @@ -2292,9 +2294,17 @@ function _applySessionListPayload(sessData, projData){ renderSessionListFromCache(); // no-ops if rename is in progress } -async function renderSessionList(opts={}){ +function _mergeRenderSessionListOptions(prev, next){ + const merged={...(prev||{}),...(next||{})}; + // Immediate refreshes must not be downgraded by a later passive polling tick. + if((prev&&prev.deferWhileInteracting===false)||(next&&next.deferWhileInteracting===false)){ + merged.deferWhileInteracting=false; + } + return merged; +} + +async function _runRenderSessionListRefresh(opts, _gen){ const deferWhileInteracting=Boolean(opts&&opts.deferWhileInteracting); - const _gen = ++_renderSessionListGen; if(!deferWhileInteracting) _pendingSessionListPayload=null; try{ if(!($('sessionSearch').value||'').trim()) _contentSearchResults = []; @@ -2314,6 +2324,37 @@ async function renderSessionList(opts={}){ }catch(e){console.warn('renderSessionList',e);} } +async function _drainRenderSessionListQueue(initialRequest){ + let request=initialRequest; + try{ + while(request){ + await _runRenderSessionListRefresh(request.opts, request.gen); + request=_renderSessionListQueuedRequest; + _renderSessionListQueuedRequest=null; + } + }finally{ + _renderSessionListInFlight=null; + if(_renderSessionListQueuedRequest){ + const next=_renderSessionListQueuedRequest; + _renderSessionListQueuedRequest=null; + _renderSessionListInFlight=_drainRenderSessionListQueue(next); + } + } +} + +async function renderSessionList(opts={}){ + const request={opts:opts||{},gen:++_renderSessionListGen}; + if(_renderSessionListInFlight){ + _renderSessionListQueuedRequest={ + opts:_mergeRenderSessionListOptions(_renderSessionListQueuedRequest&&_renderSessionListQueuedRequest.opts, request.opts), + gen:request.gen, + }; + return _renderSessionListInFlight; + } + _renderSessionListInFlight=_drainRenderSessionListQueue(request); + return _renderSessionListInFlight; +} + // ── Gateway session SSE (real-time sync for agent sessions) ── let _gatewaySSE = null; let _gatewayPollTimer = null; diff --git a/static/ui.js b/static/ui.js index 52be1f18..50ffce78 100644 --- a/static/ui.js +++ b/static/ui.js @@ -7996,10 +7996,23 @@ function appendThinking(text='', options){ const body=group&&group.querySelector('.tool-call-group-body'); if(!body) return; if(!cleanThinking||cleanThinking==='Thinking…'){ - const label=body.querySelector('.tool-card.tool-card-running')?'Waiting on tool result':'Waiting on model'; - const detail=body.querySelector('.tool-card-row') - ? 'The agent is running; tool results and response text will appear here.' - : 'No tool activity has been reported yet.'; + const hasRunningTool=!!body.querySelector('.tool-card.tool-card-running'); + const hasToolCard=!!body.querySelector('.tool-card-row'); + let label; + let detail; + if(!S.activeStreamId && options && options.pending){ + label='Starting agent'; + detail='Creating the stream and sending your message…'; + }else if(hasRunningTool){ + label='Waiting on tool result'; + detail='The tool is still running; the response will continue after it completes.'; + }else if(hasToolCard){ + label='Waiting on model'; + detail='Tool finished; waiting for the model to continue.'; + }else{ + label='Waiting for first model token'; + detail='Stream connected; no model output has arrived yet.'; + } _appendActivityEvent(group,{id:'thinking-placeholder',kind:'waiting',label,detail,status:'waiting',ts:_activityNowSeconds()}); const active=body.querySelector('.agent-activity-thinking[data-thinking-active="1"]'); if(active) active.removeAttribute('data-thinking-active'); diff --git a/tests/test_api_timeout.py b/tests/test_api_timeout.py index f5a51fcc..61788883 100644 --- a/tests/test_api_timeout.py +++ b/tests/test_api_timeout.py @@ -221,6 +221,7 @@ def test_passive_background_polls_suppress_timeout_toasts(): """Passive refreshes should be best-effort and not emit generic timeout toasts.""" workspace = _source(WORKSPACE_JS) sessions = _source(SESSIONS_JS) + messages = _source(ROOT / "static" / "messages.js") ui = _source(UI_JS) panels = _source(PANELS_JS) @@ -228,6 +229,8 @@ def test_passive_background_polls_suppress_timeout_toasts(): assert "api('/api/sessions' + allProfilesQS,{timeoutToast:false})" in sessions assert "api('/api/projects' + allProfilesQS,{timeoutToast:false})" in sessions assert "api(`/api/session?session_id=${encodeURIComponent(sid)}&messages=0&resolve_model=0`,{timeoutToast:false})" in sessions + assert 'api("/api/approval/pending?session_id=" + encodeURIComponent(sid),{timeoutToast:false})' in messages + assert 'api("/api/clarify/pending?session_id=" + encodeURIComponent(sid),{timeoutToast:false})' in messages assert "api('/api/dashboard/status',{timeoutToast:false})" in ui assert "api('/api/system/health',{timeoutToast:false})" in ui assert "api('/api/health/agent',{timeoutToast:false})" in ui diff --git a/tests/test_firefox_sidebar_scroll_stability.py b/tests/test_firefox_sidebar_scroll_stability.py index 5c9e00c6..1df3cc8c 100644 --- a/tests/test_firefox_sidebar_scroll_stability.py +++ b/tests/test_firefox_sidebar_scroll_stability.py @@ -25,15 +25,17 @@ def test_session_list_disables_browser_scroll_anchoring(): def test_polling_payloads_are_deferred_while_user_scrolls_sidebar(): render_block = _block("async function renderSessionList", "// ── Gateway session SSE") - apply_block = _block("function _applySessionListPayload", "async function renderSessionList") + refresh_block = _block("async function _runRenderSessionListRefresh", "async function _drainRenderSessionListQueue") + apply_block = _block("function _applySessionListPayload", "function _mergeRenderSessionListOptions") assert "function _isSessionListUserInteracting()" in SESSIONS_JS assert "async function renderSessionList(opts={})" in render_block - assert "const deferWhileInteracting=Boolean(opts&&opts.deferWhileInteracting);" in render_block - assert "if(deferWhileInteracting&&_isSessionListUserInteracting())" in render_block - assert "_pendingSessionListPayload={gen:_gen,sessData,projData};" in render_block - assert "_schedulePendingSessionListApply();" in render_block - assert "_applySessionListPayload(sessData,projData);" in render_block + assert "async function _runRenderSessionListRefresh(opts, _gen)" in refresh_block + assert "const deferWhileInteracting=Boolean(opts&&opts.deferWhileInteracting);" in refresh_block + assert "if(deferWhileInteracting&&_isSessionListUserInteracting())" in refresh_block + assert "_pendingSessionListPayload={gen:_gen,sessData,projData};" in refresh_block + assert "_schedulePendingSessionListApply();" in refresh_block + assert "_applySessionListPayload(sessData,projData);" in refresh_block assert "_markPollingCompletionUnreadTransitions(_allSessions);" in apply_block, ( "deferring sidebar refreshes must preserve background-completion unread semantics" ) @@ -41,9 +43,9 @@ def test_polling_payloads_are_deferred_while_user_scrolls_sidebar(): def test_deferred_payloads_keep_generation_stale_response_guard(): schedule_apply_block = _block("function _schedulePendingSessionListApply", "function _applySessionListPayload") - render_block = _block("async function renderSessionList", "// ── Gateway session SSE") + refresh_block = _block("async function _runRenderSessionListRefresh", "async function _drainRenderSessionListQueue") - assert "if(!deferWhileInteracting) _pendingSessionListPayload=null;" in render_block, ( + assert "if(!deferWhileInteracting) _pendingSessionListPayload=null;" in refresh_block, ( "explicit/user-initiated refreshes must clear older deferred background payloads" ) assert "payload.gen!==_renderSessionListGen" in schedule_apply_block, ( diff --git a/tests/test_issue856_background_completion_unread.py b/tests/test_issue856_background_completion_unread.py index d6c11fe8..2a62e935 100644 --- a/tests/test_issue856_background_completion_unread.py +++ b/tests/test_issue856_background_completion_unread.py @@ -121,7 +121,9 @@ def test_polling_transition_marks_completion_unread_without_sse_done(): ) render_idx = SESSIONS_JS.find("async function renderSessionList") assert render_idx != -1, "renderSessionList not found" - render_block = SESSIONS_JS[render_idx:SESSIONS_JS.find("// ── Gateway session SSE", render_idx)] + refresh_idx = SESSIONS_JS.find("async function _runRenderSessionListRefresh") + assert refresh_idx != -1, "_runRenderSessionListRefresh not found" + refresh_block = SESSIONS_JS[refresh_idx:SESSIONS_JS.find("async function _drainRenderSessionListQueue", refresh_idx)] apply_idx = SESSIONS_JS.find("function _applySessionListPayload(") assert apply_idx != -1, "_applySessionListPayload not found" @@ -136,7 +138,7 @@ def test_polling_transition_marks_completion_unread_without_sse_done(): ) assert "_markSessionCompletionUnread(sid, s.message_count);" in transition_block assert "_sessionStreamingById.set(sid, isStreaming);" in transition_block - assert "_applySessionListPayload(sessData,projData);" in render_block + assert "_applySessionListPayload(sessData,projData);" in refresh_block assert "_markPollingCompletionUnreadTransitions(_allSessions);" in apply_block diff --git a/tests/test_live_activity_timeline.py b/tests/test_live_activity_timeline.py index 21e04f2c..d1205ffa 100644 --- a/tests/test_live_activity_timeline.py +++ b/tests/test_live_activity_timeline.py @@ -10,6 +10,7 @@ import pathlib REPO = pathlib.Path(__file__).parent.parent UI_JS = (REPO / "static" / "ui.js").read_text(encoding="utf-8") +MESSAGES_JS = (REPO / "static" / "messages.js").read_text(encoding="utf-8") STYLE_CSS = (REPO / "static" / "style.css").read_text(encoding="utf-8") @@ -23,12 +24,27 @@ def test_live_activity_group_has_observable_baseline_events(): def test_empty_thinking_placeholder_becomes_status_row_not_raw_thinking_card(): assert "data-activity-event-id=\"thinking-placeholder\"" in UI_JS + assert "Starting agent" in UI_JS + assert "Creating the stream and sending your message…" in UI_JS + assert "Waiting for first model token" in UI_JS + assert "Stream connected; no model output has arrived yet." in UI_JS assert "Waiting on model" in UI_JS - assert "No tool activity has been reported yet." in UI_JS + assert "Tool finished; waiting for the model to continue." in UI_JS assert "Waiting on tool result" in UI_JS + assert "The tool is still running; the response will continue after it completes." in UI_JS assert "_thinkingActivityNode(thinkingText, false)" in UI_JS +def test_stream_start_refreshes_waiting_status_after_stream_id_arrives(): + active_idx = MESSAGES_JS.find("S.activeStreamId = streamId;") + assert active_idx != -1 + refresh_idx = MESSAGES_JS.find("appendThinking('',{pending:true})", active_idx) + attach_idx = MESSAGES_JS.find("attachLiveStream(activeSid, streamId, uploadedNames);", active_idx) + assert refresh_idx != -1 + assert attach_idx != -1 + assert refresh_idx < attach_idx + + def test_tool_events_update_activity_timeline_and_summary(): assert "Tool finished: ${toolName}" in UI_JS assert "Running tool: ${toolName}" in UI_JS diff --git a/tests/test_performance_polling_hardening.py b/tests/test_performance_polling_hardening.py new file mode 100644 index 00000000..a79666a1 --- /dev/null +++ b/tests/test_performance_polling_hardening.py @@ -0,0 +1,43 @@ +"""Static regressions for frontend passive polling hardening.""" + +from __future__ import annotations + +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +SESSIONS_JS = ROOT / "static" / "sessions.js" +MESSAGES_JS = ROOT / "static" / "messages.js" + + +def _source(path: Path) -> str: + return path.read_text(encoding="utf-8") + + +def test_session_list_refreshes_are_coalesced_while_in_flight(): + src = _source(SESSIONS_JS) + assert "let _renderSessionListInFlight = null" in src + assert "let _renderSessionListQueuedRequest = null" in src + assert "async function _runRenderSessionListRefresh" in src + assert "async function _drainRenderSessionListQueue" in src + assert "const request={opts:opts||{},gen:++_renderSessionListGen}" in src + assert "if(_renderSessionListInFlight)" in src + assert "_renderSessionListQueuedRequest={" in src + assert "opts:_mergeRenderSessionListOptions" in src + assert "if (_gen !== _renderSessionListGen) return" in src + assert "api('/api/sessions' + allProfilesQS,{timeoutToast:false})" in src + assert "api('/api/projects' + allProfilesQS,{timeoutToast:false})" in src + + +def test_approval_and_clarify_fallback_polls_do_not_overlap(): + src = _source(MESSAGES_JS) + assert "let _approvalFallbackPollInFlight = false" in src + assert "if (_approvalFallbackPollInFlight) return" in src + assert "_approvalFallbackPollInFlight = true" in src + assert "finally { _approvalFallbackPollInFlight = false; }" in src + assert "_approvalFallbackPollInFlight = false;\n _approvalPollingSessionId = null;" in src + + assert "let _clarifyFallbackPollInFlight = false" in src + assert "if (_clarifyFallbackPollInFlight) return" in src + assert "_clarifyFallbackPollInFlight = true" in src + assert "finally {\n _clarifyFallbackPollInFlight = false;\n }" in src + assert "_clarifyFallbackPollInFlight = false;\n _clarifyPollingSessionId = null;" in src diff --git a/tests/test_stage326_composer_draft_validation.py b/tests/test_stage326_composer_draft_validation.py index af9c0cac..04cbe959 100644 --- a/tests/test_stage326_composer_draft_validation.py +++ b/tests/test_stage326_composer_draft_validation.py @@ -81,7 +81,7 @@ def test_draft_validation_appears_before_persist(): src = Path(__file__).parents[1].joinpath("api", "routes.py").read_text(encoding="utf-8") # Anchor on the unique POST-validation comment marker. marker_idx = src.find("Stage-326 hardening (per Opus advisor)") - persist_idx = src.find("s.composer_draft = draft\n # Draft persistence is not conversation activity") + persist_idx = src.find("s.composer_draft = next_draft\n # Draft persistence is not conversation activity") assert marker_idx != -1 and persist_idx != -1, ( "could not locate validation marker or persist site" ) @@ -98,7 +98,21 @@ def test_draft_save_does_not_touch_session_updated_at(): update and force-reloads the current chat a few seconds later. """ src = Path(__file__).parents[1].joinpath("api", "routes.py").read_text(encoding="utf-8") - persist_idx = src.find("s.composer_draft = draft") + persist_idx = src.find("s.composer_draft = next_draft") assert persist_idx != -1, "could not locate composer draft persist site" save_idx = src.find("s.save(touch_updated_at=False, skip_index=True)", persist_idx) assert save_idx != -1, "composer draft save must preserve session updated_at and skip index churn" + + +def test_draft_save_skips_unchanged_payload_before_persist(): + """Duplicate debounced draft POSTs should not rewrite the full session JSON.""" + src = Path(__file__).parents[1].joinpath("api", "routes.py").read_text(encoding="utf-8") + draft_idx = src.find('current_draft = dict(getattr(s, "composer_draft", {}) or {})') + unchanged_idx = src.find("if next_draft == current_draft", draft_idx) + save_idx = src.find("s.save(touch_updated_at=False, skip_index=True)", draft_idx) + + assert draft_idx != -1, "draft route should snapshot current composer_draft" + assert unchanged_idx != -1, "draft route should no-op unchanged normalized payloads" + assert save_idx != -1, "draft route should still save changed drafts" + assert unchanged_idx < save_idx, "unchanged guard must run before full session save" + assert 'payload["unchanged"] = True' in src diff --git a/tests/test_update_banner_fixes.py b/tests/test_update_banner_fixes.py index c9473ba0..6855ad65 100644 --- a/tests/test_update_banner_fixes.py +++ b/tests/test_update_banner_fixes.py @@ -426,6 +426,90 @@ class TestApplyUpdateRestartSafety: assert result.get('restart_blocked') is True assert 'active chat stream' in result['message'] + def test_apply_update_refuses_when_active_run_without_stream(self, tmp_path, monkeypatch): + import api.updates as upd + from api.config import ACTIVE_RUNS, ACTIVE_RUNS_LOCK, STREAMS, STREAMS_LOCK + + (tmp_path / '.git').mkdir() + monkeypatch.setattr(upd, 'REPO_ROOT', tmp_path) + monkeypatch.setattr(upd, '_AGENT_DIR', tmp_path) + called = [] + monkeypatch.setattr(upd, '_run_git', lambda *a, **k: (called.append(a) or ('', True))) + monkeypatch.setattr(upd, '_schedule_restart', lambda delay=2.0: (_ for _ in ()).throw(AssertionError('must not restart'))) + + with STREAMS_LOCK: + old_streams = dict(STREAMS) + STREAMS.clear() + with ACTIVE_RUNS_LOCK: + old_runs = dict(ACTIVE_RUNS) + ACTIVE_RUNS.clear() + ACTIVE_RUNS['run_active'] = {'session_id': 's1', 'stream_id': 'missing-stream'} + try: + result = upd.apply_update('webui') + finally: + with ACTIVE_RUNS_LOCK: + ACTIVE_RUNS.clear() + ACTIVE_RUNS.update(old_runs) + with STREAMS_LOCK: + STREAMS.clear() + STREAMS.update(old_streams) + + assert result['ok'] is False + assert result.get('active_streams') == 0 + assert result.get('active_runs') == 1 + assert result.get('restart_blocked') is True + assert 'active agent run' in result['message'] + assert called == [] + + def test_force_update_refuses_when_active_run_without_stream(self, tmp_path, monkeypatch): + import api.updates as upd + from api.config import ACTIVE_RUNS, ACTIVE_RUNS_LOCK, STREAMS, STREAMS_LOCK + + (tmp_path / '.git').mkdir() + monkeypatch.setattr(upd, 'REPO_ROOT', tmp_path) + monkeypatch.setattr(upd, '_AGENT_DIR', tmp_path) + monkeypatch.setattr(upd, '_run_git', lambda *a, **k: (_ for _ in ()).throw(AssertionError('must not run git'))) + monkeypatch.setattr(upd, '_schedule_restart', lambda delay=2.0: (_ for _ in ()).throw(AssertionError('must not restart'))) + + with STREAMS_LOCK: + old_streams = dict(STREAMS) + STREAMS.clear() + with ACTIVE_RUNS_LOCK: + old_runs = dict(ACTIVE_RUNS) + ACTIVE_RUNS.clear() + ACTIVE_RUNS['run_active'] = {'session_id': 's1', 'stream_id': 'missing-stream'} + try: + result = upd.apply_force_update('agent') + finally: + with ACTIVE_RUNS_LOCK: + ACTIVE_RUNS.clear() + ACTIVE_RUNS.update(old_runs) + with STREAMS_LOCK: + STREAMS.clear() + STREAMS.update(old_streams) + + assert result['ok'] is False + assert result.get('active_streams') == 0 + assert result.get('active_runs') == 1 + assert result.get('restart_blocked') is True + assert 'active agent run' in result['message'] + + def test_wait_until_restart_safe_waits_for_active_run_to_clear(self, monkeypatch): + import api.updates as upd + + snapshots = [ + {'restart_blocked': True, 'active_streams': 0, 'active_runs': 1}, + {'restart_blocked': False, 'active_streams': 0, 'active_runs': 0}, + ] + sleeps = [] + monkeypatch.setattr(upd, '_restart_blocker_snapshot', lambda: snapshots.pop(0)) + monkeypatch.setattr(upd.time, 'sleep', lambda seconds: sleeps.append(seconds)) + + result = upd._wait_until_restart_safe(poll_seconds=0.25) + + assert result['restart_blocked'] is False + assert sleeps == [0.25] + class TestSuccessfulUpdateReturnsRestartScheduled: """#814 — successful apply_update must return restart_scheduled: True.""" diff --git a/tests/test_updates.py b/tests/test_updates.py index 71792c44..793fd61c 100644 --- a/tests/test_updates.py +++ b/tests/test_updates.py @@ -231,7 +231,7 @@ def test_apply_force_update_fetches_tags_with_force(tmp_path): with patch.object(updates, '_run_git', side_effect=fake_git), \ patch.object(updates, 'REPO_ROOT', tmp_path), \ - patch.object(updates, '_active_stream_count', return_value=0): + patch.object(updates, '_restart_blocker_snapshot', return_value={'restart_blocked': False, 'active_streams': 0, 'active_runs': 0}): updates.apply_force_update('webui') fetch_calls = [a for a in seen_args if a[:2] == ['fetch', 'origin']] @@ -256,7 +256,7 @@ def test_apply_update_fetches_tags_with_force(tmp_path): with patch.object(updates, '_run_git', side_effect=fake_git), \ patch.object(updates, 'REPO_ROOT', tmp_path), \ - patch.object(updates, '_active_stream_count', return_value=0): + patch.object(updates, '_restart_blocker_snapshot', return_value={'restart_blocked': False, 'active_streams': 0, 'active_runs': 0}): updates.apply_update('webui') fetch_calls = [a for a in seen_args if a[:2] == ['fetch', 'origin']] From 694fdd1096e6a4164f114245587b4b7d4364db12 Mon Sep 17 00:00:00 2001 From: nesquena-hermes <[email protected]> Date: Sat, 30 May 2026 01:53:38 +0000 Subject: [PATCH 2/3] =?UTF-8?q?review-3105:=20bound=20=5Fwait=5Funtil=5Fre?= =?UTF-8?q?start=5Fsafe=20with=20300s=20max-wait=20+=20execv=20fallback=20?= =?UTF-8?q?(Opus=20review=20=E2=80=94=20avoid=20self-update=20soft-jam=20o?= =?UTF-8?q?n=20long=20runs)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- api/updates.py | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/api/updates.py b/api/updates.py index da207a32..2db8269f 100644 --- a/api/updates.py +++ b/api/updates.py @@ -10,6 +10,7 @@ Skips repos that are not git checkouts (e.g. Docker baked images where """ import hashlib import json +import logging import os import re import subprocess @@ -23,6 +24,8 @@ from urllib.parse import urlparse from api.config import REPO_ROOT, STREAMS, STREAMS_LOCK +logger = logging.getLogger(__name__) + # Lazy -- may be None if agent not found try: from api.config import _AGENT_DIR @@ -126,10 +129,28 @@ def _restart_blocked_response(target: str, blocker_snapshot: dict | int) -> dict } -def _wait_until_restart_safe(poll_seconds: float = 2.0) -> dict: - """Wait for active work to finish before self-reexec.""" +def _wait_until_restart_safe(poll_seconds: float = 2.0, max_wait_seconds: float = 300.0) -> dict: + """Wait for active work to finish before self-reexec. + + Bounded by ``max_wait_seconds`` so a long-running (or stuck/orphaned) agent + run can't soft-jam the self-update indefinitely. If the deadline is reached + while work is still in flight, the snapshot is returned with + ``wait_timed_out=True`` so the caller can proceed with the re-exec anyway + (preserving the pre-#3105 "execv preempts in-flight work" fallback) rather + than holding ``_apply_lock`` for the run's full lifetime. + """ snapshot = _restart_blocker_snapshot() + deadline = time.monotonic() + max(0.0, max_wait_seconds) while snapshot.get('restart_blocked'): + if time.monotonic() >= deadline: + logger.warning( + "restart-safety wait exceeded %.0fs with work still in flight (%s); " + "proceeding with re-exec anyway", + max_wait_seconds, snapshot, + ) + snapshot = dict(snapshot) + snapshot['wait_timed_out'] = True + return snapshot time.sleep(max(0.1, poll_seconds)) snapshot = _restart_blocker_snapshot() return snapshot From a7848c884e835bc63f9c75609fc81dfd8bf22133 Mon Sep 17 00:00:00 2001 From: nesquena-hermes <[email protected]> Date: Sat, 30 May 2026 01:54:52 +0000 Subject: [PATCH 3/3] stage-batch46: CHANGELOG for v0.51.164 (Release EJ) --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 05418d43..72c3b370 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,12 +3,14 @@ ## [Unreleased] +## [v0.51.164] — 2026-05-30 — Release EJ (stage-batch46 — passive performance hardening: refresh coalescing + draft-save dedup + activity placeholders + bounded restart-safety wait) + ### Fixed - Coalesced duplicate in-flight sidebar/project refreshes and suppressed overlapping approval/clarify fallback polls, reducing repeated passive requests while preserving latest-refresh-wins sidebar state. - Skipped duplicate composer-draft writes when the normalized autosave payload is unchanged, avoiding redundant full session JSON rewrites during debounced input/focus churn. - Refined empty Activity waiting placeholders to distinguish stream creation, first-token wait, post-tool model wait, and running-tool wait states. -- Self-update restart safety now checks active agent runs as well as open SSE streams and waits for in-flight work before re-exec, avoiding update-triggered interruption when a run outlives its browser stream. +- Self-update restart safety now checks active agent runs as well as open SSE streams and waits for in-flight work before re-exec, avoiding update-triggered interruption when a run outlives its browser stream. The wait is bounded (300s) with a logged fallback to re-exec so a long-running or stuck agent run cannot soft-jam the self-update indefinitely. - Documented the WebUI prefill context budget in README and architecture notes so operators can keep new-browser-turn startup context compact. ## [v0.51.163] — 2026-05-30 — Release EI (stage-batch45 — session duplicate/branch field propagation)