Merge pull request #5063 from nesquena/stage/4978-oob-strip

Release: strip consumed OOB steer blocks from model-facing history (#4978)

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)

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):

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):

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