fix(streaming+ui): preserve user message on cancel + persist activity-panel expand state (#1298)

From PR #1338. Already independently APPROVED by nesquena before being absorbed into v0.50.246.

CHANGELOG entries from this PR were dropped during squash (the v0.50.245 section is already
shipped); they will be re-added under [v0.50.246] in the release commit.

Co-authored-by: nesquena-hermes <nesquena-hermes@users.noreply.github.com>

api/streaming.py

@@ -2586,6 +2586,55 @@ def cancel_stream(stream_id: str) -> bool:
         with _get_session_agent_lock(_cancel_session_id):
             try:
                 _cs = get_session(_cancel_session_id)
+                # ── Preserve the user's typed message before clearing pending state (#1298) ──
+                # The agent's internal messages list (where the user message was appended at
+                # the start of run_conversation()) may not have been merged back into
+                # _cs.messages yet — cancel_stream() races with the streaming thread's final
+                # _merge_display_messages_after_agent_result() call. Without this guard, the
+                # user's message is lost: pending_user_message gets cleared below, and
+                # _cs.messages still only contains messages from prior turns. The reporter
+                # of #1298 sees their typed text vanish from chat after clicking Stop.
+                #
+                # Recovery rule: if pending_user_message is set AND the latest message in
+                # _cs.messages isn't already a matching user turn, synthesize one. The
+                # match check guards against double-append when the streaming thread DID
+                # reach its merge step before cancel_stream() got the session lock.
+                #
+                # Wrapped in its own try/except so an unexpected _cs.messages shape (e.g.
+                # in unit tests using Mock sessions) cannot escape and skip the rest of
+                # the cleanup.
+                try:
+                    _pending_user = getattr(_cs, 'pending_user_message', None)
+                    _pending_atts_raw = getattr(_cs, 'pending_attachments', None)
+                    _pending_atts = list(_pending_atts_raw) if isinstance(_pending_atts_raw, (list, tuple)) else []
+                    _msgs_for_recovery = _cs.messages if isinstance(_cs.messages, list) else None
+                    if _pending_user and _msgs_for_recovery is not None:
+                        _last_user = None
+                        for _m in reversed(_msgs_for_recovery):
+                            if isinstance(_m, dict) and _m.get('role') == 'user':
+                                _last_user = _m
+                                break
+                        _already_persisted = False
+                        if _last_user is not None:
+                            _last_content = _last_user.get('content')
+                            if isinstance(_last_content, str):
+                                # Tolerate the workspace prefix the streaming thread prepends.
+                                if _pending_user in _last_content or _last_content in _pending_user:
+                                    _already_persisted = True
+                        if not _already_persisted:
+                            _user_turn: dict = {
+                                'role': 'user',
+                                'content': _pending_user,
+                                'timestamp': int(time.time()),
+                            }
+                            if _pending_atts:
+                                _user_turn['attachments'] = _pending_atts
+                            _msgs_for_recovery.append(_user_turn)
+                except Exception:
+                    logger.debug(
+                        "Failed to recover pending user message on cancel for %s",
+                        _cancel_session_id,
+                    )
                 _cs.active_stream_id = None
                 _cs.pending_user_message = None
                 _cs.pending_attachments = []

static/ui.js

@@ -2644,6 +2644,30 @@ function _thinkingActivityNode(text){
   row.innerHTML=_thinkingCardHtml(text);
   return row;
 }
+// ── Activity-group user expand intent (#1298) ──────────────────────────────
+// When the user manually expands the live "Activity" dropdown during streaming,
+// preserve that intent across the destroy/recreate cycle that fires on every
+// thinking/tool event. Without this, ensureActivityGroup() re-creates the group
+// with the default collapsed state and finalizeThinkingCard() force-collapses
+// it whenever the assistant transitions from thinking → tool → thinking, so
+// the panel snaps shut every few seconds while the user is trying to read it.
+//
+// The tracker is a singleton boolean: there is at most one live activity group
+// at a time (selector .tool-call-group[data-live-tool-call-group="1"]). It is
+// set to true when the user clicks the summary to expand, false when they
+// click to collapse, and cleared back to undefined when the live group is
+// finalized into a settled assistant turn (the live attribute is removed in
+// _convertLiveActivityGroupToSettled / when liveAssistantTurn loses its id).
+let _liveActivityUserExpanded;
+function _onLiveActivityToggle(group){
+  if(!group) return;
+  // Only track explicit user clicks on the live group, not programmatic toggles.
+  if(group.getAttribute('data-live-tool-call-group')!=='1') return;
+  _liveActivityUserExpanded = !group.classList.contains('tool-call-group-collapsed');
+}
+function _clearLiveActivityUserIntent(){
+  _liveActivityUserExpanded = undefined;
+}
 function ensureActivityGroup(inner, opts){
   opts=opts||{};
   if(!inner) return null;
@@ -2652,12 +2676,16 @@ function ensureActivityGroup(inner, opts){
   let group=inner.querySelector(selector);
   if(!group){
     group=document.createElement('div');
-    const collapsed=opts.collapsed!==false;
+    let collapsed=opts.collapsed!==false;
+    // Restore the user's explicit expand intent when recreating the live
+    // activity group within the same turn (#1298).
+    if(live && _liveActivityUserExpanded === true) collapsed=false;
+    else if(live && _liveActivityUserExpanded === false) collapsed=true;
     group.className='tool-call-group agent-activity-group'+(collapsed?' tool-call-group-collapsed':'');
     group.setAttribute('data-tool-call-group','1');
     group.setAttribute('data-agent-activity-group','1');
     if(live) group.setAttribute('data-live-tool-call-group','1');
-    group.innerHTML=`<button type="button" class="tool-call-group-summary" aria-expanded="${collapsed?'false':'true'}" onclick="const g=this.closest('.tool-call-group');const c=g.classList.toggle('tool-call-group-collapsed');this.setAttribute('aria-expanded',String(!c));"><span class="tool-call-group-chevron">${li('chevron-right',12)}</span><span class="tool-call-group-label">Activity</span><span class="tool-call-group-list">tools / thinking</span><span class="tool-call-group-count">0</span></button><div class="tool-call-group-body"></div>`;
+    group.innerHTML=`<button type="button" class="tool-call-group-summary" aria-expanded="${collapsed?'false':'true'}" onclick="const g=this.closest('.tool-call-group');const c=g.classList.toggle('tool-call-group-collapsed');this.setAttribute('aria-expanded',String(!c));if(typeof _onLiveActivityToggle==='function')_onLiveActivityToggle(g);"><span class="tool-call-group-chevron">${li('chevron-right',12)}</span><span class="tool-call-group-label">Activity</span><span class="tool-call-group-list">tools / thinking</span><span class="tool-call-group-count">0</span></button><div class="tool-call-group-body"></div>`;
     const anchor=opts.anchor||null;
     if(anchor&&anchor.parentElement===inner) anchor.insertAdjacentElement('afterend', group);
     else inner.appendChild(group);
@@ -3576,6 +3604,9 @@ function appendLiveToolCard(tc){
 function clearLiveToolCards(){
   const inner=_assistantTurnBlocks($('liveAssistantTurn'));
   if(inner) inner.querySelectorAll('.tool-call-group[data-live-tool-call-group],.tool-card-row[data-live-tid]').forEach(el=>el.remove());
+  // Reset the per-turn user expand intent so the next turn starts at the
+  // default collapsed state (#1298).
+  if(typeof _clearLiveActivityUserIntent==='function') _clearLiveActivityUserIntent();
   // Legacy #liveToolCards container cleanup — kept for safety in case any
   // leftover cards were inserted there before this refactor took effect.
   const container=$('liveToolCards');
@@ -4262,9 +4293,15 @@ function finalizeThinkingCard(){
   const turn=$('liveAssistantTurn');
   const group=turn&&turn.querySelector('.tool-call-group[data-live-tool-call-group="1"]');
   if(group){
-    group.classList.add('tool-call-group-collapsed');
-    const summary=group.querySelector('.tool-call-group-summary');
-    if(summary) summary.setAttribute('aria-expanded','false');
+    // Respect the user's explicit expand intent (#1298) — only force-collapse
+    // when the user has not manually expanded this turn's activity group, or
+    // has manually collapsed it. Otherwise the panel snaps shut whenever new
+    // activity arrives, even mid-read.
+    if(_liveActivityUserExpanded !== true){
+      group.classList.add('tool-call-group-collapsed');
+      const summary=group.querySelector('.tool-call-group-summary');
+      if(summary) summary.setAttribute('aria-expanded','false');
+    }
     const active=group.querySelector('.agent-activity-thinking[data-thinking-active="1"]');
     if(active) active.removeAttribute('data-thinking-active');
     _syncToolCallGroupSummary(group);

tests/test_issue1298_cancel_and_activity.py

@@ -0,0 +1,326 @@
+"""Regression tests for #1298 — Activity panel UI state and Stop/Cancel data loss.
+
+Two distinct bugs reported in YanTianlong-01's bug report on v0.50.240:
+
+  1. The expanded Activity list collapses automatically when new activity arrives.
+  2. The latest user message disappears after clicking Stop/Cancel during streaming.
+
+Bug 2 is server-side data loss (the message is gone from session JSON, not just
+the in-memory client copy) caused by cancel_stream() clearing pending_user_message
+without first persisting it to s.messages. This test suite locks down both fixes.
+"""
+import pathlib
+import queue
+import re
+import threading
+from unittest.mock import Mock
+
+import pytest
+
+import api.config as config
+import api.models as models
+import api.streaming as streaming
+from api.models import Session
+from api.streaming import cancel_stream
+
+REPO_ROOT = pathlib.Path(__file__).parent.parent.resolve()
+
+
+# ── Fixtures ────────────────────────────────────────────────────────────────
+
+@pytest.fixture(autouse=True)
+def _isolate_session_dir(tmp_path, monkeypatch):
+    """Redirect SESSION_DIR / SESSION_INDEX_FILE to an isolated temp dir."""
+    session_dir = tmp_path / "sessions"
+    session_dir.mkdir()
+    index_file = session_dir / "_index.json"
+    monkeypatch.setattr(models, "SESSION_DIR", session_dir)
+    monkeypatch.setattr(models, "SESSION_INDEX_FILE", index_file)
+    models.SESSIONS.clear()
+    yield
+    models.SESSIONS.clear()
+
+
+@pytest.fixture(autouse=True)
+def _isolate_stream_state():
+    config.STREAMS.clear()
+    config.CANCEL_FLAGS.clear()
+    config.AGENT_INSTANCES.clear()
+    config.STREAM_PARTIAL_TEXT.clear()
+    yield
+    config.STREAMS.clear()
+    config.CANCEL_FLAGS.clear()
+    config.AGENT_INSTANCES.clear()
+    config.STREAM_PARTIAL_TEXT.clear()
+
+
+@pytest.fixture(autouse=True)
+def _isolate_agent_locks():
+    config.SESSION_AGENT_LOCKS.clear()
+    yield
+    config.SESSION_AGENT_LOCKS.clear()
+
+
+def _make_pending_session(session_id="cancel_sid_1298",
+                          pending_msg="Help me debug this issue",
+                          messages=None,
+                          attachments=None):
+    """Build a session in mid-stream state: pending_user_message set, messages may be empty."""
+    s = Session(
+        session_id=session_id,
+        title="Test Session",
+        messages=messages or [],
+    )
+    s.pending_user_message = pending_msg
+    s.pending_attachments = list(attachments or [])
+    s.pending_started_at = None
+    s.active_stream_id = "stream_1298"
+    s.save()
+    models.SESSIONS[session_id] = s
+    return s
+
+
+def _setup_cancel_stream_state(session_id, stream_id="stream_1298"):
+    """Wire up STREAMS/CANCEL_FLAGS/AGENT_INSTANCES so cancel_stream() can run."""
+    config.STREAMS[stream_id] = queue.Queue()
+    config.CANCEL_FLAGS[stream_id] = threading.Event()
+    mock_agent = Mock()
+    mock_agent.session_id = session_id
+    mock_agent.interrupt = Mock()
+    config.AGENT_INSTANCES[stream_id] = mock_agent
+    return stream_id, mock_agent
+
+
+# ── Server-side: cancel preserves pending_user_message in s.messages ────────
+
+class TestIssue1298CancelPreservesUserMessage:
+    """Issue 2: Latest user message disappears after Stop/Cancel during streaming.
+
+    Root cause: cancel_stream() at api/streaming.py:2575+ clears
+    s.pending_user_message before the streaming thread's
+    _merge_display_messages_after_agent_result() has a chance to merge the
+    user turn into s.messages. The session is saved with neither
+    pending_user_message nor a corresponding s.messages entry, so the user's
+    typed text is lost permanently.
+
+    Fix: synthesize a user turn from pending_user_message into s.messages when
+    the most recent message isn't already that turn.
+    """
+
+    def test_cancel_synthesizes_user_message_when_messages_empty(self):
+        """When the agent thread is killed before it can append the user turn,
+        cancel_stream() must persist pending_user_message into s.messages so
+        the typed text survives a session reload."""
+        s = _make_pending_session(
+            session_id="cancel_sid_empty",
+            pending_msg="What's the weather forecast?",
+            messages=[],
+        )
+        stream_id, _agent = _setup_cancel_stream_state(s.session_id)
+
+        result = cancel_stream(stream_id)
+        assert result is True
+
+        # Reload from disk to confirm save happened
+        s2 = models.SESSIONS[s.session_id]
+        roles = [m.get("role") for m in s2.messages if isinstance(m, dict)]
+        contents = [m.get("content") for m in s2.messages if isinstance(m, dict)]
+
+        assert "user" in roles, (
+            "Expected user turn synthesized into s.messages — "
+            f"got roles={roles}"
+        )
+        assert "What's the weather forecast?" in contents, (
+            "Expected pending_user_message text preserved verbatim in s.messages — "
+            f"got contents={contents}"
+        )
+        assert s2.pending_user_message is None, (
+            "pending_user_message must be cleared after cancel"
+        )
+        assert s2.active_stream_id is None
+
+    def test_cancel_does_not_double_append_when_streaming_thread_already_merged(self):
+        """If the streaming thread won the race and already merged the user turn
+        into s.messages before cancel_stream() got the lock, cancel must not
+        append a duplicate."""
+        prior_user = {"role": "user", "content": "Run a tool for me"}
+        s = _make_pending_session(
+            session_id="cancel_sid_already_merged",
+            pending_msg="Run a tool for me",
+            messages=[prior_user],
+        )
+        stream_id, _agent = _setup_cancel_stream_state(s.session_id)
+
+        cancel_stream(stream_id)
+
+        s2 = models.SESSIONS[s.session_id]
+        user_messages = [m for m in s2.messages
+                         if isinstance(m, dict) and m.get("role") == "user"]
+        # Exactly one user turn — no duplicate
+        matching = [m for m in user_messages
+                    if "Run a tool for me" in str(m.get("content") or "")]
+        assert len(matching) == 1, (
+            "Expected exactly one user turn matching pending_user_message — "
+            f"got {len(matching)} ({user_messages})"
+        )
+
+    def test_cancel_synthesized_user_message_carries_attachments(self):
+        """A cancelled turn that had attachments uploaded should keep them on
+        the recovered user message."""
+        s = _make_pending_session(
+            session_id="cancel_sid_attachments",
+            pending_msg="Look at this screenshot",
+            messages=[],
+            attachments=["bug_screenshot.png", "stack_trace.txt"],
+        )
+        stream_id, _agent = _setup_cancel_stream_state(s.session_id)
+
+        cancel_stream(stream_id)
+
+        s2 = models.SESSIONS[s.session_id]
+        user_msgs = [m for m in s2.messages
+                     if isinstance(m, dict) and m.get("role") == "user"]
+        assert user_msgs, "User turn must be persisted on cancel"
+        recovered = user_msgs[0]
+        assert recovered.get("attachments") == [
+            "bug_screenshot.png", "stack_trace.txt"
+        ], (
+            "Attachment list must be preserved on the synthesized user turn — "
+            f"got {recovered.get('attachments')}"
+        )
+
+    def test_cancel_no_pending_user_message_does_nothing_extra(self):
+        """When there is no pending_user_message (e.g. cancel after the agent
+        has already returned), cancel_stream() must not synthesize a phantom
+        user turn."""
+        s = Session(
+            session_id="cancel_sid_no_pending",
+            title="Test",
+            messages=[{"role": "user", "content": "earlier turn"}],
+        )
+        s.active_stream_id = "stream_1298"
+        s.pending_user_message = None
+        s.save()
+        models.SESSIONS[s.session_id] = s
+        stream_id, _agent = _setup_cancel_stream_state(s.session_id)
+
+        cancel_stream(stream_id)
+
+        s2 = models.SESSIONS[s.session_id]
+        user_messages = [m for m in s2.messages
+                         if isinstance(m, dict) and m.get("role") == "user"]
+        # Still exactly one — the original earlier turn
+        assert len(user_messages) == 1
+        assert user_messages[0].get("content") == "earlier turn"
+
+
+# ── Client-side: ui.js source-level guards for activity-group state ─────────
+
+class TestIssue1298ActivityGroupExpandPersistence:
+    """Issue 1: Expanded Activity list collapses automatically when new
+    activity arrives.
+
+    Root cause:
+      - ensureActivityGroup() (static/ui.js) creates the live activity group
+        with `tool-call-group-collapsed` whenever it's missing
+      - finalizeThinkingCard() force-adds `tool-call-group-collapsed` on every
+        tool boundary, regardless of user intent
+      - The user's manually-set expand state lives only on a DOM class list,
+        so any destroy/recreate cycle (which fires on every thinking → tool →
+        thinking transition) wipes it.
+
+    Fix: track the user's last explicit toggle in a per-turn singleton, and
+    skip the force-collapse when the user has explicitly expanded.
+    """
+
+    def test_ui_js_tracks_user_expand_intent_for_live_activity_group(self):
+        src = (REPO_ROOT / "static" / "ui.js").read_text()
+        assert "_liveActivityUserExpanded" in src, (
+            "ui.js must declare a per-turn tracker for the user's expand intent "
+            "on the live activity group (#1298)"
+        )
+        assert "_onLiveActivityToggle" in src, (
+            "ui.js must expose a helper that records the user's manual toggle "
+            "of the live activity group"
+        )
+
+    def test_ensure_activity_group_restores_expand_intent(self):
+        """ensureActivityGroup() must consult _liveActivityUserExpanded when
+        creating a fresh live group so the user's prior expand survives the
+        destroy/recreate cycle."""
+        src = (REPO_ROOT / "static" / "ui.js").read_text()
+        # Find the ensureActivityGroup function body
+        m = re.search(
+            r"function ensureActivityGroup\(inner, opts\)\{(.*?)\n\}",
+            src, re.DOTALL,
+        )
+        assert m, "ensureActivityGroup() must exist in ui.js"
+        body = m.group(1)
+        assert "_liveActivityUserExpanded" in body, (
+            "ensureActivityGroup() body must reference the user-expand tracker "
+            "to restore intent on re-create (#1298)"
+        )
+        assert "live" in body and "_liveActivityUserExpanded === true" in body, (
+            "ensureActivityGroup() must override the default `collapsed` flag "
+            "when the user previously expanded the live group"
+        )
+
+    def test_finalize_thinking_card_respects_user_expand(self):
+        """finalizeThinkingCard() must NOT force-collapse the live activity
+        group when the user has explicitly expanded it (#1298)."""
+        src = (REPO_ROOT / "static" / "ui.js").read_text()
+        m = re.search(
+            r"function finalizeThinkingCard\(\)\{(.*?)\n\}",
+            src, re.DOTALL,
+        )
+        assert m, "finalizeThinkingCard() must exist in ui.js"
+        body = m.group(1)
+        assert "_liveActivityUserExpanded" in body, (
+            "finalizeThinkingCard() must respect the user's expand intent — "
+            "without this guard, the panel snaps shut on every tool boundary"
+        )
+        # Hard fail if force-collapse is unconditional
+        assert "_liveActivityUserExpanded !== true" in body or \
+               "_liveActivityUserExpanded!==true" in body.replace(" ", ""), (
+            "finalizeThinkingCard() must skip the force-collapse path when "
+            "_liveActivityUserExpanded === true"
+        )
+
+    def test_inline_onclick_records_user_intent(self):
+        """The summary button's inline onclick must call _onLiveActivityToggle
+        so user clicks update the tracker (#1298)."""
+        src = (REPO_ROOT / "static" / "ui.js").read_text()
+        # The summary button is built inline inside ensureActivityGroup.
+        assert "_onLiveActivityToggle" in src, (
+            "_onLiveActivityToggle helper must be defined"
+        )
+        # The inline onclick string must include the call so user toggles
+        # are captured into _liveActivityUserExpanded.
+        m = re.search(r'class="tool-call-group-summary"[^`]*`', src)
+        assert m, "live activity summary button template must be present"
+        # The onclick fragment is in the same template literal that builds
+        # the button — pull a wider window
+        m2 = re.search(
+            r"group\.innerHTML=`<button[^`]*?_onLiveActivityToggle[^`]*?`",
+            src, re.DOTALL,
+        )
+        assert m2, (
+            "ensureActivityGroup() inline onclick must invoke "
+            "_onLiveActivityToggle(g) so user clicks update the tracker"
+        )
+
+    def test_clear_live_tool_cards_resets_expand_intent(self):
+        """clearLiveToolCards() — invoked between turns — must reset the
+        per-turn user-expand tracker so the next turn starts collapsed by
+        default (#1298)."""
+        src = (REPO_ROOT / "static" / "ui.js").read_text()
+        m = re.search(
+            r"function clearLiveToolCards\(\)\{(.*?)\n\}",
+            src, re.DOTALL,
+        )
+        assert m, "clearLiveToolCards() must exist"
+        body = m.group(1)
+        assert "_clearLiveActivityUserIntent" in body, (
+            "clearLiveToolCards() must reset _liveActivityUserExpanded between "
+            "turns so prior expand intent doesn't bleed into the next turn"
+        )