From 97a25a98fb456518c0dd3a9f3797da25e0b173fb Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Sat, 27 Jun 2026 19:13:36 +0000 Subject: [PATCH] fix(streaming): strip consumed OOB steer blocks from model-facing history (#4978) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a turn is steered, the agent embeds an [OUT-OF-BAND USER MESSAGE]...[/OUT-OF-BAND USER MESSAGE] control block, and that block gets persisted into the session's messages/context_messages (appended onto a tool-result entry — verified present in real prod session JSON, in both messages[] and context_messages[]). WebUI then replays context_messages into the provider-facing conversation history every turn, so the model kept re-seeing the internal control marker: wasted tokens + stale control data. Adds a non-mutating, recursive _strip_oob_blocks() (regex, DOTALL+IGNORECASE, non-greedy) wired into every model-facing chokepoint: _sanitize_messages_for_api, _api_safe_message_positions, _restore_reasoning_metadata projection, and the gateway runs-API conversation_history (context_messages + prefill). The compression path (/session/compress) inherits the strip via _sanitize_messages_for_api. The marker stays intact in the persisted transcript (display/reload/audit fidelity); it's removed only on the way to the provider. Idempotent, only strips a complete open+close pair, and does not mutate persisted session objects (the content key is reassigned to a freshly-built value). Maintainer hardening (Opus gate): relaxed the opening pattern from a mandatory `]\r?\n` to `]\s*?` so a single-line OOB block is also stripped (DOTALL .*? already spans newlines) — robustness against a format that omits the newline. Co-authored-by: Stacey2911 --- CHANGELOG.md | 2 + api/gateway_chat.py | 5 + api/streaming.py | 37 ++- tests/test_context_history_sanitization.py | 263 +++++++++++++++++++++ 4 files changed, 306 insertions(+), 1 deletion(-) create mode 100644 tests/test_context_history_sanitization.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 23c6bd95..2d2392ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ ### Fixed +- **Consumed mid-turn steer markers no longer leak into the model's replayed history.** When you steer a running turn, the agent embeds an `[OUT-OF-BAND USER MESSAGE]…[/OUT-OF-BAND USER MESSAGE]` control block in the turn, and that block was being persisted into the session's `messages`/`context_messages` (appended onto a tool-result entry). On every subsequent turn WebUI replayed that stored context into the provider-facing conversation history, so the model kept re-seeing the internal control marker — wasted tokens plus stale control data that can confuse the agent. WebUI now strips consumed OOB blocks at the model-facing boundary (the `_sanitize_messages_for_api` / runs-API sanitizers, plus the reasoning-projection and compression paths that route through them), so the markers are removed on the way to the provider while staying intact in the persisted transcript for display/reload fidelity. The strip is non-mutating, idempotent, and only removes a complete open+close marker pair. Thanks @Stacey2911. (#4978) + - **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) diff --git a/api/gateway_chat.py b/api/gateway_chat.py index f5d21da1..ebd202a7 100644 --- a/api/gateway_chat.py +++ b/api/gateway_chat.py @@ -299,6 +299,8 @@ def _run_gateway_runs_api_streaming( except Exception: logger.debug("Failed to build runs-API multimodal attachment payload", exc_info=True) message_content = str(msg_text or "") + from api.streaming import _strip_oob_blocks + instructions_parts = [] conversation_history = [] for entry in getattr(session, "context_messages", None) or []: @@ -309,6 +311,7 @@ def _run_gateway_runs_api_streaming( continue content = entry.get("content") if content is not None: + content = _strip_oob_blocks(content) conversation_history.append({"role": role, "content": content}) for entry in prefill_messages or []: if not isinstance(entry, dict): @@ -323,6 +326,8 @@ def _run_gateway_runs_api_streaming( continue if role not in {"user", "assistant"}: continue + if content is not None: + content = _strip_oob_blocks(content) conversation_history.append({"role": role, "content": content}) run_input = message_content if isinstance(run_input, list): diff --git a/api/streaming.py b/api/streaming.py index ed523f96..77eec9d4 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -1452,6 +1452,8 @@ from api.workspace import set_last_workspace # Fields that are safe to send to LLM provider APIs. # Everything else (attachments, timestamp, _ts, etc.) is display-only # metadata added by the webui and must be stripped before the API call. +# `reasoning_content` is provider-facing for reasoning-capable models. Display +# metadata such as `reasoning`, `thinking`, and `_reasoning` stays omitted here. _API_SAFE_MSG_KEYS = {'role', 'content', 'tool_calls', 'tool_call_id', 'name', 'refusal', 'reasoning_content'} _NATIVE_IMAGE_MAX_BYTES = 20 * 1024 * 1024 @@ -3560,6 +3562,32 @@ def _strip_native_image_parts_from_content(content): return clean_parts +_OOB_USER_MESSAGE_BLOCK_RE = re.compile( + r'\[OUT-OF-BAND\s+USER\s+MESSAGE(?:\s*(?:—|-)\s*.*?)?\]\s*?.*?\[/OUT-OF-BAND\s+USER\s+MESSAGE\]', + re.DOTALL | re.IGNORECASE, +) + + +def _strip_oob_blocks(content): + """Remove consumed [OUT-OF-BAND USER MESSAGE ...] blocks from content. + + These markers are internal control data that should never reach the model. + They can appear as plain strings or inside list-based content parts. + """ + if isinstance(content, str): + return _OOB_USER_MESSAGE_BLOCK_RE.sub('', content) + if isinstance(content, list): + return [_strip_oob_blocks(part) for part in content] + if isinstance(content, dict): + return { + key: _strip_oob_blocks(value) + if isinstance(value, (str, list, dict)) + else copy.deepcopy(value) + for key, value in content.items() + } + return content + + def _content_has_reasoning_only_parts(content) -> bool: if not isinstance(content, list) or not content: return False @@ -3667,6 +3695,8 @@ def _sanitize_messages_for_api(messages, *, cfg: dict = None): sanitized = {k: v for k, v in msg.items() if k in _API_SAFE_MSG_KEYS} if is_recovered: sanitized['_recovered'] = True # temporary marker — stripped before return + if 'content' in sanitized: + sanitized['content'] = _strip_oob_blocks(sanitized['content']) if strip_native_images and 'content' in sanitized: sanitized['content'] = _strip_native_image_parts_from_content(sanitized.get('content')) if sanitized.get('role'): @@ -3764,6 +3794,8 @@ def _api_safe_message_positions(messages): sanitized = {k: v for k, v in msg.items() if k in _API_SAFE_MSG_KEYS} if is_recovered: sanitized['_recovered'] = True # temporary marker — stripped before return + if 'content' in sanitized: + sanitized['content'] = _strip_oob_blocks(sanitized['content']) if sanitized.get('role'): out.append((idx, sanitized)) @@ -3898,7 +3930,10 @@ def _restore_reasoning_metadata(previous_messages, updated_messages): def _safe_projection(msg): if not isinstance(msg, dict): return None - return {k: v for k, v in msg.items() if k in _API_SAFE_MSG_KEYS and msg.get('role')} + projected = {k: v for k, v in msg.items() if k in _API_SAFE_MSG_KEYS and msg.get('role')} + if 'content' in projected: + projected['content'] = _strip_oob_blocks(projected['content']) + return projected safe_pos = 0 while safe_pos < len(prev_safe): diff --git a/tests/test_context_history_sanitization.py b/tests/test_context_history_sanitization.py new file mode 100644 index 00000000..750f76e7 --- /dev/null +++ b/tests/test_context_history_sanitization.py @@ -0,0 +1,263 @@ +"""Regression tests for model-facing context-history sanitization.""" +from __future__ import annotations + +import json +import threading +from types import SimpleNamespace +from unittest.mock import patch + +from api.streaming import _sanitize_messages_for_api, _strip_oob_blocks + + +OOB_BLOCK = ( + "[OUT-OF-BAND USER MESSAGE — consumed steer]\n" + "internal control text that must not reach the model\n" + "[/OUT-OF-BAND USER MESSAGE]" +) + + +def _contains_oob(value) -> bool: + return "OUT-OF-BAND USER MESSAGE" in json.dumps(value) + + +def test_strip_oob_blocks_string(): + content = f"visible before\n{OOB_BLOCK}\nvisible after" + + cleaned = _strip_oob_blocks(content) + + assert "visible before" in cleaned + assert "visible after" in cleaned + assert "internal control text" not in cleaned + assert "OUT-OF-BAND USER MESSAGE" not in cleaned + + +def test_strip_oob_blocks_list_content(): + content = [ + {"type": "text", "text": f"keep\n{OOB_BLOCK}\nkeep later"}, + {"type": "input_text", "content": f"prefix\n{OOB_BLOCK}\nsuffix"}, + {"type": "image_url", "image_url": {"url": "data:image/png;base64,abc"}}, + ] + + cleaned = _strip_oob_blocks(content) + + assert cleaned[0]["text"].startswith("keep") + assert "keep later" in cleaned[0]["text"] + assert "suffix" in cleaned[1]["content"] + assert cleaned[2] == content[2] + assert not _contains_oob(cleaned) + + +def test_sanitize_messages_for_api_no_oob_in_output(): + messages = [ + {"role": "user", "content": f"question\n{OOB_BLOCK}\nvisible question"}, + { + "role": "assistant", + "content": f"answer\n{OOB_BLOCK}\nvisible answer", + "reasoning": "display-only reasoning", + "thinking": "display-only thinking", + "_reasoning": "display-only private reasoning", + "reasoning_content": "provider-facing reasoning metadata", + }, + ] + + sanitized = _sanitize_messages_for_api(messages) + + assert not _contains_oob(sanitized) + assert sanitized[0]["content"].startswith("question") + assert "visible question" in sanitized[0]["content"] + assert "visible answer" in sanitized[1]["content"] + assert "reasoning" not in sanitized[1] + assert "thinking" not in sanitized[1] + assert "_reasoning" not in sanitized[1] + assert sanitized[1]["reasoning_content"] == "provider-facing reasoning metadata" + + +def test_sanitize_messages_for_api_preserves_tool_chains(): + messages = [ + {"role": "user", "content": f"please inspect\n{OOB_BLOCK}\nvisible request"}, + { + "role": "assistant", + "content": "", + "tool_calls": [ + { + "type": "function", + "id": "call-1", + "function": {"name": "terminal", "arguments": "{}"}, + } + ], + }, + {"role": "tool", "tool_call_id": "call-1", "content": f"tool output\n{OOB_BLOCK}\nresult"}, + {"role": "assistant", "content": "done"}, + ] + + sanitized = _sanitize_messages_for_api(messages) + + assert [msg["role"] for msg in sanitized] == ["user", "assistant", "tool", "assistant"] + assert sanitized[1]["tool_calls"][0]["id"] == "call-1" + assert sanitized[2]["tool_call_id"] == "call-1" + assert "result" in sanitized[2]["content"] + assert not _contains_oob(sanitized) + + +def test_gateway_conversation_history_no_oob(): + from api.config import STREAM_PARTIAL_TEXT, STREAM_REASONING_TEXT + from api.gateway_chat import _STREAM_RUN_IDS, _run_gateway_runs_api_streaming + + requests = [] + stream_id = "stream-oob-sanitization" + STREAM_PARTIAL_TEXT[stream_id] = "" + STREAM_REASONING_TEXT[stream_id] = "" + + class _JsonResponse: + def __init__(self, payload): + self._payload = json.dumps(payload).encode("utf-8") + + def read(self, _limit=None): + return self._payload + + def __enter__(self): + return self + + def __exit__(self, *args): + return None + + class _SseResponse: + def __iter__(self): + return iter([ + b'data: {"event":"run.completed","output":"done","usage":{"input_tokens":1,"output_tokens":1}}\n', + b'\n', + ]) + + def __enter__(self): + return self + + def __exit__(self, *args): + return None + + def fake_urlopen(req, *, timeout=None): + requests.append(req) + if req.full_url.endswith("/v1/runs"): + return _JsonResponse({"run_id": "run-oob"}) + return _SseResponse() + + session = SimpleNamespace(context_messages=[ + {"role": "system", "content": f"ignored system\n{OOB_BLOCK}"}, + {"role": "user", "content": f"history request\n{OOB_BLOCK}\nvisible request"}, + {"role": "assistant", "content": [{"type": "text", "text": f"history reply\n{OOB_BLOCK}\nvisible reply"}]}, + {"role": "tool", "tool_call_id": "call-ignored", "content": f"ignored\n{OOB_BLOCK}"}, + ]) + + try: + with patch("urllib.request.urlopen", side_effect=fake_urlopen): + final_text, usage = _run_gateway_runs_api_streaming( + session_id="sess-oob", + msg_text="current user", + model="test-model", + workspace="/tmp", + stream_id=stream_id, + base_url="http://gw:8642", + api_key="secret", + prefill_messages=[], + body_extras={}, + put_gateway_event=lambda *_args, **_kwargs: None, + cancel_event=threading.Event(), + session=session, + ) + finally: + STREAM_PARTIAL_TEXT.pop(stream_id, None) + STREAM_REASONING_TEXT.pop(stream_id, None) + _STREAM_RUN_IDS.pop(stream_id, None) + + run_body = json.loads(requests[0].data.decode("utf-8")) + assert final_text == "done" + assert usage["input_tokens"] == 1 + assert usage["output_tokens"] == 1 + assert not _contains_oob(run_body["conversation_history"]) + assert run_body["conversation_history"] == [ + {"role": "user", "content": "history request\n\nvisible request"}, + {"role": "assistant", "content": [{"type": "text", "text": "history reply\n\nvisible reply"}]}, + ] + + +# ── Regex variant coverage tests ───────────────────────────────────────────── + +OOB_HYPHEN = ( + "[OUT-OF-BAND USER MESSAGE - consumed steer]\n" + "internal control text that must not reach the model\n" + "[/OUT-OF-BAND USER MESSAGE]" +) + + +def test_strip_oob_blocks_hyphen_marker(): + """Hyphen variant of opening marker should be stripped.""" + content = f"visible before\n{OOB_HYPHEN}\nvisible after" + + cleaned = _strip_oob_blocks(content) + + assert "visible before" in cleaned + assert "visible after" in cleaned + assert "internal control text" not in cleaned + assert "OUT-OF-BAND USER MESSAGE" not in cleaned + + +OOB_CRLF = ( + "[OUT-OF-BAND USER MESSAGE — consumed steer]\r\n" + "internal control text that must not reach the model\r\n" + "[/OUT-OF-BAND USER MESSAGE]" +) + + +def test_strip_oob_blocks_crlf_line_endings(): + """Windows CRLF line endings should be handled.""" + content = f"visible before\r\n{OOB_CRLF}\r\nvisible after" + + cleaned = _strip_oob_blocks(content) + + assert "visible before" in cleaned + assert "visible after" in cleaned + assert "internal control text" not in cleaned + assert "OUT-OF-BAND USER MESSAGE" not in cleaned + + +OOB_NO_SUFFIX = ( + "[OUT-OF-BAND USER MESSAGE]\n" + "internal control text that must not reach the model\n" + "[/OUT-OF-BAND USER MESSAGE]" +) + + +def test_strip_oob_blocks_no_suffix_marker(): + """Opening marker without dash/suffix should be stripped.""" + content = f"visible before\n{OOB_NO_SUFFIX}\nvisible after" + + cleaned = _strip_oob_blocks(content) + + assert "visible before" in cleaned + assert "visible after" in cleaned + assert "internal control text" not in cleaned + assert "OUT-OF-BAND USER MESSAGE" not in cleaned + + +def test_strip_oob_blocks_multiple_in_one_string(): + """Multiple OOB blocks in a single string should all be stripped.""" + content = ( + f"before {OOB_BLOCK} middle {OOB_HYPHEN} end" + ) + + cleaned = _strip_oob_blocks(content) + + assert "before" in cleaned + assert "middle" in cleaned + assert "end" in cleaned + # Count occurrences — should be zero after stripping + assert cleaned.count("OUT-OF-BAND USER MESSAGE") == 0 + + +def test_strip_oob_blocks_incomplete_block_preserved(): + """Incomplete/unclosed OOB blocks should NOT be stripped.""" + content = "visible before\n[OUT-OF-BAND USER MESSAGE — incomplete" + + cleaned = _strip_oob_blocks(content) + + # Incomplete block should remain untouched + assert "[OUT-OF-BAND USER MESSAGE" in cleaned