diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c3f8aa..9265d8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,17 @@ workspace subtree) and never enumerate blocked system roots. (`api/routes.py`, `api/workspace.py`, `static/panels.js`, `static/style.css`) (partial for #616) +## [v0.50.163] — 2026-04-23 + +### Fixed +- **Message ordering after task cancellation** — cancelling a stream while the + agent is responding no longer causes subsequent responses to appear above the + "Task cancelled." marker. The cancel handler now fetches the authoritative + message list from the server (same as the done event), and the server persists + the cancel message to the session so both paths stay in sync. Falls back to + the previous local-push behaviour if the API call fails. (`api/streaming.py`, + `static/messages.js`) (@mittyok, #882) + ## [v0.50.161] — 2026-04-23 ### Fixed diff --git a/api/streaming.py b/api/streaming.py index bd67f4d..97d0d89 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -1666,6 +1666,18 @@ def cancel_stream(stream_id: str) -> bool: _cs.pending_user_message = None _cs.pending_attachments = [] _cs.pending_started_at = None + # Add cancel message to session messages so client sees consistent state. + # _error=True flags this as a synthetic UI marker so + # _sanitize_messages_for_api() (line 591-593) strips it from the + # conversation_history passed to the agent on the NEXT user message — + # otherwise the model would see "Task cancelled." in its history as a + # prior assistant turn and could respond accordingly. + _cs.messages.append({ + 'role': 'assistant', + 'content': '*Task cancelled.*', + '_error': True, + 'timestamp': int(time.time()), + }) _cs.save() except Exception: logger.debug("Failed to clear session state on cancel for %s", _cancel_session_id) diff --git a/static/messages.js b/static/messages.js index ca7e9dd..d2d8d95 100644 --- a/static/messages.js +++ b/static/messages.js @@ -653,10 +653,26 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ if(S.session&&S.session.session_id===activeSid){ S.activeStreamId=null;const _cbc=$('btnCancel');if(_cbc)_cbc.style.display='none'; } - if(S.session&&S.session.session_id===activeSid){ - clearLiveToolCards();if(!assistantText)removeThinking(); - S.messages.push({role:'assistant',content:'*Task cancelled.*'});renderMessages(); - } + // Fetch latest session from server to get accurate message list (includes cancel status) + // This ensures messages stay in sync with server, fixing race condition where local + // "*Task cancelled.*" message gets lost when done event overwrites S.messages + (async()=>{ + try{ + const data=await api(`/api/session?session_id=${encodeURIComponent(activeSid)}`); + if(data&&data.session&&S.session&&S.session.session_id===activeSid){ + S.session=data.session; + S.messages=(data.session.messages||[]).filter(m=>m&&m.role); + clearLiveToolCards();if(!assistantText)removeThinking(); + renderMessages(); + } + }catch(_){ + // Fallback to local cancel message if API fails + if(S.session&&S.session.session_id===activeSid){ + clearLiveToolCards();if(!assistantText)removeThinking(); + S.messages.push({role:'assistant',content:'*Task cancelled.*'});renderMessages(); + } + } + })(); renderSessionList(); if(!S.session||!INFLIGHT[S.session.session_id]){setBusy(false);setComposerStatus('');} }); diff --git a/tests/test_sprint36.py b/tests/test_sprint36.py index 67cd7df..bf058ed 100644 --- a/tests/test_sprint36.py +++ b/tests/test_sprint36.py @@ -158,11 +158,13 @@ def test_sse_cancel_handler_still_present(): def test_sse_cancel_handler_calls_set_busy(): """The SSE cancel handler must still call setBusy(false).""" src = read("static/messages.js") - idx = src.find("addEventListener('cancel'") + idx = src.find("addEventListener('cancel'") if idx == -1: idx = src.find('addEventListener("cancel"') assert idx != -1 - block = src[idx:idx + 1200] + # 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)" ) @@ -180,3 +182,53 @@ def test_cancel_failed_i18n_key_exists_in_all_locales(): f"cancel_failed key only found {count} times in i18n.js — " f"expected at least {locale_count} (one per locale)" ) + + +# ── 8. Server-persisted cancel marker doesn't leak into agent history ──────── + +def test_cancel_marker_flagged_as_error_to_skip_in_api_history(): + """The server-side cancel marker appended in cancel_stream() must carry + _error: True so _sanitize_messages_for_api() strips it from the + conversation_history sent to the agent on the next user message. + + Without this flag, the LLM sees "*Task cancelled.*" as a prior assistant + turn and may reference it in subsequent responses ("As I mentioned, I was + cancelled...") — a behavioral regression introduced when this PR started + persisting the marker to the session. + """ + src = read("api/streaming.py") + idx = src.find("'content': '*Task cancelled.*'") + if idx == -1: + idx = src.find('"content": "*Task cancelled.*"') + assert idx != -1, "cancel marker content string not found in cancel_stream()" + + # Walk back to the start of the dict literal (opening brace) + brace_open = src.rfind("{", 0, idx) + brace_close = src.find("}", idx) + assert brace_open != -1 and brace_close != -1, "couldn't locate cancel marker dict" + + marker_dict = src[brace_open:brace_close + 1] + assert "_error" in marker_dict and "True" in marker_dict, ( + "cancel marker is missing _error: True — it will leak into the agent's " + "conversation_history via _sanitize_messages_for_api() on the next turn. " + "See line 591-593 of api/streaming.py for the error-marker filter." + ) + + +def test_sanitize_strips_error_flagged_assistant_messages(): + """_sanitize_messages_for_api() must drop messages with _error: True — + this is the invariant the cancel marker's _error flag relies on.""" + from api.streaming import _sanitize_messages_for_api + messages = [ + {"role": "user", "content": "hello"}, + {"role": "assistant", "content": "hi"}, + {"role": "assistant", "content": "*Task cancelled.*", "_error": True}, + {"role": "user", "content": "next"}, + ] + sanitized = _sanitize_messages_for_api(messages) + assert len(sanitized) == 3, ( + f"expected 3 messages (cancel marker stripped), got {len(sanitized)}: {sanitized}" + ) + assert all("Task cancelled" not in (m.get("content") or "") for m in sanitized), ( + "_sanitize_messages_for_api must filter cancel markers from API history" + )