From 4f64443f63f60d0927c5231a2bd10e19cb342d94 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Sat, 27 Jun 2026 18:49:08 +0000 Subject: [PATCH] fix(sessions): reload active transcript only on message-count change (#5061) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The active-session external-refresh check (refreshActiveSessionIfExternallyUpdated) force-reloaded the whole transcript whenever last_message_at/updated_at advanced. The post-turn background skill/memory review bumps those timestamps without adding any chat messages, so the destructive loadSession(force) (clears S.messages, awaits a round-trip) made the open conversation visibly disappear and reappear with no new content. The refresh now reloads only when the message count actually changed, and treats a timestamp-only bump as metadata (advance the local last-seen marker + refresh the lightweight sidebar list, no transcript teardown). Maintainer fix on top of the contributor's PR (Codex gate): the count check fires in BOTH directions — remoteCount !== localCount, not just >. A LOWER remote count means another tab/client truncated/undid/retried/regenerated the transcript (/api/session/truncate, /retry, /undo all shrink s.messages and persist a lower message_count); the original >-only gate would have treated that as metadata-only and silently kept the stale longer transcript forever. Added a regression test for the count-decrease case. The pending-user-message count-inflation path is excluded by the existing busy/active-stream guards. Co-authored-by: allenliang2022 --- CHANGELOG.md | 2 + static/sessions.js | 25 ++++++++++- tests/test_webui_external_refresh_frontend.py | 41 ++++++++++++++++++- 3 files changed, 66 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 95475fdc..23c6bd95 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ ### Fixed +- **A background skill/memory review no longer makes the open conversation briefly vanish and reappear.** The active-session external-refresh check force-reloaded the whole transcript whenever the session's `last_message_at`/`updated_at` advanced — but the post-turn background skill/memory review bumps those timestamps without adding any chat messages, so the destructive reload (`loadSession(force)` clears the message list and awaits a round-trip) made the entire conversation visibly disappear and reappear a moment later with no new content. The refresh now reloads only when the message *count* actually changed, and treats a timestamp-only bump as metadata — advancing the local last-seen marker and refreshing the lightweight sidebar list without touching the transcript. Crucially, the count check fires in **both directions**: a *lower* remote count (another tab/client truncated, undid, retried, or regenerated the transcript) still force-reloads, so this tab can never silently keep a stale conversation. Thanks @allenliang2022. (#5061) + - **Cancelling a running turn now keeps the partial work it had already produced, with no flicker.** When you hit Stop mid-turn, the browser briefly collapsed the already-streamed reasoning, tool cards, and text down to a bare "Task cancelled" marker before a follow-up `GET /api/session` repopulated them — a visible race between the cancel event and the refetch. The terminal cancel event now carries a canonical, redacted session snapshot captured under the per-session agent lock right after the cancelled turn is persisted (including the recovered partial-assistant message), so the frontend renders the preserved partial reasoning/tool/text immediately and skips the GET round-trip entirely. The embedded snapshot is a fully detached deep copy (redaction rebuilds every container) so it can never alias or be staled by concurrent writes, it is only applied when its `session_id` matches the active view, and a missing/failed snapshot degrades cleanly to the existing GET fallback. Thanks @franksong2702. (#4647, fixes #4076, #1361) - **A WebUI turn whose stream lost its terminal event no longer loses already-produced text on refresh.** If a stream drops its `done`/`stream_end` while the agent keeps writing to `state.db`, the browser showed live output but a refresh reloaded the stale sidecar and the produced text appeared to vanish. WebUI now self-heals on read: when the durable `state.db` transcript is genuinely newer than the sidecar and the stream is truly gone (not in the live SSE/worker registries, and past a startup grace window), it reconciles the sidecar from `state.db` through the existing append-only merger (preserving compaction/truncation semantics) and clears the stale unfinished-run state. The reconcile + write run under the per-session lock against a freshly-reloaded session, so it can never race or clobber a concurrent worker writeback or a just-starting turn. Thanks @allenliang2022. (#5053) diff --git a/static/sessions.js b/static/sessions.js index b4754f1d..40025e35 100644 --- a/static/sessions.js +++ b/static/sessions.js @@ -4363,9 +4363,32 @@ async function refreshActiveSessionIfExternallyUpdated(reason){ if(S.busy || S.activeStreamId) return; const remoteCount = Number(data.session.message_count || 0); const remoteLast = Number(data.session.last_message_at || data.session.updated_at || 0); - if(remoteCount > localCount || remoteLast > localLast){ + // Force-reload the whole transcript whenever the visible conversation's + // message count CHANGED in either direction. A higher count means new + // messages; a LOWER count means another tab/client truncated, undid, + // retried, or regenerated the transcript (/api/session/truncate, /retry, + // /undo all shrink s.messages and write a lower message_count) — both must + // re-sync or this tab silently keeps a stale transcript. + // + // A bump in last_message_at WITHOUT a count change means a non-transcript + // write touched the session — most commonly the post-turn background + // skill/memory review, which rewrites memory/skills and advances updated_at + // but adds no chat messages. Reloading on that bump tears down and re-fetches + // the transcript: loadSession(force) clears S.messages and awaits a + // round-trip before re-rendering, so the whole conversation visibly + // disappears and "reappears a moment later" with no new content. Skip the + // destructive reload in that case and just refresh the lightweight sidebar + // list metadata, advancing the local last-seen marker so the same metadata + // bump doesn't re-trigger on every subsequent poll. + if(remoteCount !== localCount){ await loadSession(sid, {force:true, externalRefreshReason:reason||'poll'}); if(typeof renderSessionList==='function') void renderSessionList(); + }else if(remoteLast > localLast){ + if(S.session && S.session.session_id === sid){ + S.session.last_message_at = remoteLast; + if(data.session.updated_at) S.session.updated_at = data.session.updated_at; + } + if(typeof renderSessionList==='function') void renderSessionList(); } }catch(e){ // Ignore transient refresh failures; the next poll/focus event will retry. diff --git a/tests/test_webui_external_refresh_frontend.py b/tests/test_webui_external_refresh_frontend.py index b424907a..a346ab0e 100644 --- a/tests/test_webui_external_refresh_frontend.py +++ b/tests/test_webui_external_refresh_frontend.py @@ -19,12 +19,51 @@ def test_active_session_external_refresh_uses_metadata_then_force_reload(): assert "function ensureActiveSessionExternalRefreshPoll()" in SESSIONS_JS assert "async function refreshActiveSessionIfExternallyUpdated(reason)" in SESSIONS_JS assert "messages=0&resolve_model=0" in SESSIONS_JS - assert "remoteCount > localCount || remoteLast > localLast" in SESSIONS_JS + assert "if(remoteCount !== localCount)" in SESSIONS_JS + assert "else if(remoteLast > localLast)" in SESSIONS_JS assert "if(S.busy || S.activeStreamId) return;" in SESSIONS_JS assert "document.hidden" in SESSIONS_JS assert "externalRefreshReason:reason||'poll'" in SESSIONS_JS +def test_active_session_external_refresh_skips_destructive_reload_on_metadata_only_bump(): + """A timestamp-only active-session update should not blank the transcript. + + Background skill/memory review can update session timestamps without adding + chat messages. The old `remoteCount > localCount || remoteLast > localLast` + condition called `loadSession(..., {force:true})` for that metadata-only + bump; `loadSession(force)` clears S.messages before async message fetches, + so the whole transcript visibly disappeared and reappeared with no new + content. Only a message_count CHANGE should force-reload the transcript; + timestamp-only bumps update local metadata and refresh the lightweight + sidebar list. + """ + assert "remoteCount > localCount || remoteLast > localLast" not in SESSIONS_JS + assert "if(remoteCount !== localCount){" in SESSIONS_JS + assert "await loadSession(sid, {force:true, externalRefreshReason:reason||'poll'});" in SESSIONS_JS + assert "}else if(remoteLast > localLast){" in SESSIONS_JS + assert "S.session.last_message_at = remoteLast" in SESSIONS_JS + assert "if(data.session.updated_at) S.session.updated_at = data.session.updated_at;" in SESSIONS_JS + + +def test_active_session_external_refresh_force_reloads_on_count_decrease(): + """A LOWER remote message_count must still force-reload the transcript. + + Another tab/client can shrink the active transcript via /api/session/truncate, + /retry, /undo, or regenerate — all reduce message_count while advancing + updated_at. A `remoteCount > localCount` gate would treat that as a + metadata-only bump and silently keep the stale (longer) transcript forever. + The condition is `remoteCount !== localCount` precisely so a decrease also + re-syncs. + """ + # The force-reload branch must trigger on ANY count change, not just growth. + assert "if(remoteCount !== localCount){" in SESSIONS_JS + assert "if(remoteCount > localCount){" not in SESSIONS_JS + # The metadata-only branch must be gated on an unchanged count (the else of + # the count-change check), never reachable when the count differs. + assert "}else if(remoteLast > localLast){" in SESSIONS_JS + + def test_webui_source_never_counts_as_external_session(): assert "function _isWebUiSourceSession(session)" in SESSIONS_JS assert "if (!session || _isWebUiSourceSession(session)) return false;" in SESSIONS_JS