fix(sessions): reload active transcript only on message-count change (#5061)

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 <allenliang2022@users.noreply.github.com>

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)

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.

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