From 681aa3573bd2c0ed8eecee2e07c5a630fafa2595 Mon Sep 17 00:00:00 2001 From: joaomarcos Date: Wed, 29 Jul 2026 16:43:30 -0300 Subject: [PATCH 1/2] fix(history): persist personality marker durably and stop hiding merged real prompts The personality-switch marker was appended as an in-memory-only role=user message identified purely by a "[System:" text prefix, unlike the model-switch marker which is durably persisted via SessionDB.append_message. When alternation-repair merged the marker with the next real user turn (both role=user), the merged blob still started with "[System:" and the display projection dropped the whole row, silently swallowing the real prompt. Persist the marker durably and identify it via display_kind="personality_switch" plus display_metadata={"marker_text": ...}, mirroring the model-switch marker's pattern. Both survive alternation-repair's merge (which only rewrites `content`), so the hide-projection can now strip just the marker's own span from a merged row instead of dropping the whole thing. Closes #74315 --- tests/test_tui_gateway_server.py | 88 ++++++++++++++++++++++++ tui_gateway/server.py | 111 +++++++++++++++++++++++++++++-- 2 files changed, 195 insertions(+), 4 deletions(-) diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index 7456f116bcd07..b05ef498d8963 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -2165,6 +2165,94 @@ def test_history_to_messages_hides_gateway_system_markers(): ] +def test_personality_marker_survives_alternation_repair_merge(): + # Reproduces #74315: a personality switch appends a role=user marker to + # the live history. If the very next turn is also role=user (no + # assistant reply landed in between — e.g. the switch happened between + # turns, or a crash/resume left the pair adjacent), alternation-repair + # merges the two consecutive user rows into one, concatenating the real + # prompt onto the marker's content. With the old bare "[System:" text + # marker, the hide-projection then dropped the WHOLE merged row — + # silently swallowing the real prompt. The fix persists the marker with + # structured display_kind/display_metadata (mirrors the model-switch + # marker) so repair's merge (which only rewrites `content`) leaves that + # metadata intact, and the projection can strip just the marker's own + # span. + from agent.agent_runtime_helpers import repair_message_sequence + + marker = ( + "[System: The user has changed the assistant's personality. " + "From this point forward, adopt the following persona and respond " + "accordingly: You are a pirate.]" + ) + history = [ + {"role": "user", "content": "hello there"}, + {"role": "assistant", "content": "hi!"}, + { + "role": "user", + "content": marker, + "display_kind": "personality_switch", + "display_metadata": {"marker_text": marker}, + }, + {"role": "user", "content": "what's the weather like today?"}, + ] + + repairs = repair_message_sequence(None, history) + assert repairs == 1 + # The merge concatenated the real prompt onto the marker row in place — + # confirms the reproduction matches the bug's actual mechanism. + assert len(history) == 3 + merged = history[-1] + assert merged["role"] == "user" + assert merged["content"] == marker + "\n\n" + "what's the weather like today?" + # display_kind/display_metadata are untouched by the merge (only + # `content` is rewritten) — the structural precondition the fix relies on. + assert merged["display_kind"] == "personality_switch" + assert merged["display_metadata"] == {"marker_text": marker} + + projected = server._history_to_messages(history) + + # The real prompt must still be visible after projection — this is the + # exact user-facing symptom from #74315 (the prompt "disappears"). + texts = [m["text"] for m in projected] + assert "what's the weather like today?" in texts + # The marker's own text must never leak into a client transcript as + # literal "[System: …]" user-bubble text, before or after the merge. + assert not any(t.lstrip().startswith("[System:") for t in texts) + assert projected == [ + {"role": "user", "text": "hello there"}, + {"role": "assistant", "text": "hi!"}, + {"role": "user", "text": "what's the weather like today?"}, + ] + + +def test_history_to_messages_hides_bare_personality_marker_without_merge(): + # When nothing merges onto it (an assistant reply lands before the next + # user turn, the normal case), the personality marker is still hidden + # entirely — same contract as the model-switch marker. + marker = ( + "[System: The user has cleared the personality overlay. " + "From this point forward, respond in your normal default style.]" + ) + history = [ + {"role": "user", "content": "hi"}, + {"role": "assistant", "content": "hello"}, + { + "role": "user", + "content": marker, + "display_kind": "personality_switch", + "display_metadata": {"marker_text": marker}, + }, + {"role": "assistant", "content": "back to normal"}, + ] + + assert server._history_to_messages(history) == [ + {"role": "user", "text": "hi"}, + {"role": "assistant", "text": "hello"}, + {"role": "assistant", "text": "back to normal"}, + ] + + def test_history_to_messages_drops_display_hidden_scaffolding(): # A mid-stream steer persists an interrupted-turn checkpoint. When nothing # reached the screen the row carries only model-facing scaffolding and is diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 7d39f0ac12397..ebea640821b86 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -5477,6 +5477,16 @@ def _apply_personality_to_session( knows to pivot its style from this point forward (without this, LLMs tend to continue the tone established by earlier messages in the transcript). + The marker is persisted durably (mirrors ``_append_model_switch_marker``) + and carries structured ``display_kind``/``display_metadata`` instead of + relying on the "[System:" text-prefix convention alone. Alternation-repair + may later merge the very next real user turn onto this row's content + (consecutive ``user`` rows get concatenated) — ``display_kind`` and + ``display_metadata`` survive that merge untouched since only ``content`` + is rewritten, so the hide-projection can still recognize the marker + unambiguously and strip only its own span instead of dropping the whole + merged row (and the real prompt riding on its tail) (#74315). + Returns (history_reset, info) — history_reset is always False since we preserve the conversation. """ @@ -5500,9 +5510,42 @@ def _apply_personality_to_session( "[System: The user has cleared the personality overlay. " "From this point forward, respond in your normal default style.]" ) + entry = { + "role": "user", + "content": marker, + "display_kind": "personality_switch", + "display_metadata": {"marker_text": marker}, + } with session["history_lock"]: - session["history"].append({"role": "user", "content": marker}) + session["history"].append(entry) session["history_version"] = int(session.get("history_version", 0)) + 1 + + session_key = str(session.get("session_key") or "").strip() + if session_key: + try: + db = getattr(agent, "_session_db", None) + if db is not None: + db.append_message( + session_id=session_key, + role="user", + content=marker, + display_kind="personality_switch", + display_metadata={"marker_text": marker}, + ) + else: + _ensure_session_db_row(session) + with _session_db(session) as scoped_db: + if scoped_db is not None: + scoped_db.append_message( + session_id=session_key, + role="user", + content=marker, + display_kind="personality_switch", + display_metadata={"marker_text": marker}, + ) + except Exception: + logger.debug("failed to persist personality marker", exc_info=True) + info = _session_info(agent) _emit("session.info", sid, info) return False, info @@ -6475,6 +6518,45 @@ def _is_display_hidden_marker(role: str | None, text: str) -> bool: return role == "user" and text.lstrip().startswith("[System:") +# display_kind values that mark a row as gateway bookkeeping (a synthetic +# pivot notice, not something the user typed) rather than ordinary chat +# content. See _bookkeeping_marker_span. +_BOOKKEEPING_MARKER_KINDS = frozenset({"model_switch", "personality_switch"}) + + +def _bookkeeping_marker_span(m: dict, content_text: str) -> tuple[str, bool] | None: + """Split a bookkeeping-marker row into its marker span and any real content. + + Alternation-repair merges consecutive ``user`` rows by concatenating + their content (``marker_text + "\\n\\n" + real_text``); ``display_kind`` + and ``display_metadata`` are left untouched by that merge since only + ``content`` is rewritten. When a row's own marker text was recorded at + write time (``display_metadata["marker_text"]``), that lets this + projection tell a bare marker apart from a marker a real user turn got + merged onto — the old text-prefix check (:func:`_is_display_hidden_marker`) + could only hide the whole row, silently swallowing the merged-in real + prompt (#74315). + + Returns ``(visible_text, is_pure_marker)`` when this row is a recognized + bookkeeping marker with a recorded span, else ``None`` (caller should fall + back to :func:`_is_display_hidden_marker`'s legacy text-prefix handling). + """ + if m.get("display_kind") not in _BOOKKEEPING_MARKER_KINDS: + return None + meta = m.get("display_metadata") + marker_text = meta.get("marker_text") if isinstance(meta, dict) else None + if not isinstance(marker_text, str) or not marker_text: + return None + if content_text == marker_text: + return "", True + prefix = marker_text + "\n\n" + if content_text.startswith(prefix): + return content_text[len(prefix):], False + # Content diverged from the recorded span (edited/truncated some other + # way) — don't guess, fall back to the legacy handling. + return None + + def _skill_scaffold_projection(content_text: str) -> str: """Return the invocation a slash-skill-expanded turn came from, else "". @@ -6560,7 +6642,21 @@ def _history_to_messages(history: list[dict]) -> list[dict]: if m.get("display_kind") == "hidden": continue content_text = _coerce_message_text(m.get("content")) - if _is_display_hidden_marker(role, content_text): + # A bookkeeping marker (model-switch/personality-switch) that + # alternation-repair merged a real user turn onto still carries its + # own display_kind/display_metadata (the merge only rewrites + # content) — strip just the marker's span instead of falling + # through to the text-prefix check, which would hide the whole row + # and silently swallow the real prompt riding on its tail (#74315). + suppress_display_kind = False + marker_span = _bookkeeping_marker_span(m, content_text) + if marker_span is not None: + visible_text, is_pure_marker = marker_span + if is_pure_marker: + continue + content_text = visible_text + suppress_display_kind = True + elif _is_display_hidden_marker(role, content_text): continue if role == "assistant" and m.get("tool_calls"): for tc in m["tool_calls"]: @@ -6617,10 +6713,17 @@ def _history_to_messages(history: list[dict]) -> list[dict]: # Forward display-only timeline metadata so the TUI can render # model switches and delegation completions as events instead of # opaque user messages, and hide compaction handoffs entirely. - display_kind = m.get("display_kind") or _legacy_display_kind(role, content_text) + # A stripped marker span (suppress_display_kind) is real user + # content now, not the marker — it must render as an ordinary + # message, not carry the marker's display_kind/display_metadata. + display_kind = ( + None + if suppress_display_kind + else m.get("display_kind") or _legacy_display_kind(role, content_text) + ) if display_kind: msg["display_kind"] = display_kind - if m.get("display_metadata"): + if m.get("display_metadata") and not suppress_display_kind: msg["display_metadata"] = m["display_metadata"] messages.append(msg) From 1eaf0e41c06e6198961e54b21b5a8abcd1d907f9 Mon Sep 17 00:00:00 2001 From: joaomarcos Date: Thu, 30 Jul 2026 23:20:45 -0300 Subject: [PATCH 2/2] fix(history): preserve merged-marker rows as real turns for undo/retry/rewind teknium's review on #74350: repair_message_sequence's consecutive-user merge only rewrites content, so a real prompt merged onto a personality/model-switch marker tail still carries the marker's display_kind. undo, retry, and rewind ordinal selection all treat role=user with no display_kind as the definition of a real turn, so they silently skipped this visible prompt. Pass 2 of repair_message_sequence now stamps merged_real_turn=True on a bookkeeping-marker row when a distinct real turn gets merged onto it. The four role=user and not display_kind predicates in tui_gateway/server.py (session.undo, prompt.submit rewind ordinal, retry, rollback.restore) now also accept merged_real_turn rows as real. Added regression coverage for undo, retry, and rewind after the merge. --- agent/agent_runtime_helpers.py | 12 +++ tests/test_tui_gateway_server.py | 136 +++++++++++++++++++++++++++++ tests/tui_gateway/test_protocol.py | 61 +++++++++++++ tui_gateway/server.py | 36 +++++--- 4 files changed, 234 insertions(+), 11 deletions(-) diff --git a/agent/agent_runtime_helpers.py b/agent/agent_runtime_helpers.py index 5505d70756237..f93a78b10b55f 100644 --- a/agent/agent_runtime_helpers.py +++ b/agent/agent_runtime_helpers.py @@ -696,6 +696,18 @@ def _is_verification_candidate(m: Dict) -> bool: # content alone — collapsing image/audio blocks risks # mangling the attachment structure. if isinstance(prev_content, str) and isinstance(new_content, str): + if prev.get("display_kind") and not msg.get("display_kind") and new_content: + # A real user turn got merged onto a bookkeeping-marker + # row (personality/model switch). display_kind and + # display_metadata are left untouched below so the + # display projection can still recognize the marker and + # strip just its own span (_bookkeeping_marker_span) — + # but /undo, /retry, and rewind ordinal selection treat + # "role=user with no display_kind" as the definition of + # a real turn, and would otherwise skip this row + # entirely, silently dropping the visible prompt riding + # on the marker's tail (#74350). + prev["merged_real_turn"] = True prev["content"] = ( (prev_content + "\n\n" + new_content) if prev_content and new_content diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index b05ef498d8963..37b2afb13a1f0 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -2209,6 +2209,12 @@ def test_personality_marker_survives_alternation_repair_merge(): # `content` is rewritten) — the structural precondition the fix relies on. assert merged["display_kind"] == "personality_switch" assert merged["display_metadata"] == {"marker_text": marker} + # merged_real_turn distinguishes this row (a real prompt riding on the + # marker's tail) from a bare marker for /undo, /retry, and rewind + # ordinal selection, which otherwise treat any role=user row carrying + # display_kind as pure bookkeeping and would skip this visible prompt + # entirely (#74350 review feedback). + assert merged["merged_real_turn"] is True projected = server._history_to_messages(history) @@ -8522,6 +8528,46 @@ def test_session_undo_allowed_when_idle(): server._sessions.pop("sid", None) +def test_session_undo_does_not_skip_prompt_merged_onto_marker(): + """/undo must truncate through a real prompt alternation-repair merged + onto a bookkeeping marker's tail, not treat the merged row as pure + bookkeeping and remove an earlier real exchange instead (#74350). + """ + marker = ( + "[System: The user has changed the assistant's personality. " + "From this point forward, adopt the following persona and respond " + "accordingly: You are a pirate.]" + ) + server._sessions["sid"] = _session( + running=False, + history=[ + {"role": "user", "content": "hi"}, + {"role": "assistant", "content": "hello"}, + { + "role": "user", + "content": marker + "\n\n" + "what's the weather like today?", + "display_kind": "personality_switch", + "display_metadata": {"marker_text": marker}, + "merged_real_turn": True, + }, + ], + ) + try: + resp = server.handle_request( + {"id": "1", "method": "session.undo", "params": {"session_id": "sid"}} + ) + assert resp.get("result"), f"got error: {resp.get('error')}" + # Only the merged row is removed — undo must not reach back past it + # into the "hi"/"hello" exchange. + assert resp["result"]["removed"] == 1 + assert server._sessions["sid"]["history"] == [ + {"role": "user", "content": "hi"}, + {"role": "assistant", "content": "hello"}, + ] + finally: + server._sessions.pop("sid", None) + + def test_session_compress_rejects_while_running(monkeypatch): server._sessions["sid"] = _session(running=True) try: @@ -8883,6 +8929,96 @@ def replace_messages(self, session_id, messages): server._sessions.pop("sid", None) +def test_prompt_submit_truncate_ordinal_counts_merged_real_turn(monkeypatch): + """truncate_before_user_ordinal must count a row where a real prompt was + alternation-repair-merged onto a bookkeeping marker's tail as a real + user turn (merged_real_turn=True), not skip it as pure bookkeeping and + miscount every ordinal after it (#74350). + """ + + seen = {} + + class _Agent: + def run_conversation(self, prompt, conversation_history=None, stream_callback=None, **_kwargs): + seen["prompt"] = prompt + seen["history"] = conversation_history + return { + "final_response": "reply", + "messages": [ + *(conversation_history or []), + {"role": "user", "content": prompt}, + {"role": "assistant", "content": "reply"}, + ], + } + + class _ImmediateThread: + def __init__(self, target=None, daemon=None): + self._target = target + + def start(self): + self._target() + + marker = ( + "[System: The user has changed the assistant's personality. " + "From this point forward, adopt the following persona and respond " + "accordingly: You are a pirate.]" + ) + original_history = [ + {"role": "user", "content": "first"}, + {"role": "assistant", "content": "first reply"}, + { + "role": "user", + "content": marker + "\n\n" + "second", + "display_kind": "personality_switch", + "display_metadata": {"marker_text": marker}, + "merged_real_turn": True, + }, + {"role": "assistant", "content": "second reply"}, + ] + server._sessions["sid"] = _session(agent=_Agent(), history=original_history) + + class _StubDb: + def __init__(self): + self.replaced = [] + + def replace_messages(self, session_id, messages): + self.replaced.append((session_id, list(messages))) + + stub_db = _StubDb() + + try: + monkeypatch.setattr(server.threading, "Thread", _ImmediateThread) + monkeypatch.setattr(server, "_get_usage", lambda _a: {}) + monkeypatch.setattr(server, "render_message", lambda _t, _c: "") + monkeypatch.setattr(server, "_emit", lambda *a: None) + monkeypatch.setattr(server, "_get_db", lambda: stub_db) + + # ordinal=1 means "truncate before the 2nd-from-last real user turn" + # which is the merged marker row. If merged_real_turn were ignored, + # user_indices would be [0] only (just "first") and ordinal=1 would + # be rejected as out of range instead of resolving to index 2. + resp = server.handle_request( + { + "id": "1", + "method": "prompt.submit", + "params": { + "session_id": "sid", + "text": "edited second", + "truncate_before_user_ordinal": 1, + }, + } + ) + assert resp.get("result"), f"got error: {resp.get('error')}" + assert seen["history"] == original_history[:2], ( + f"Expected truncation to first 2 messages, got {seen['history']}" + ) + assert stub_db.replaced == [("session-key", original_history[:2])], ( + f"Expected DB replace with first 2 messages, got {stub_db.replaced}" + ) + finally: + server._sessions.pop("sid", None) + + # --------------------------------------------------------------------------- # session.interrupt must only cancel pending prompts owned by the calling # session — it must not blast-resolve clarify/sudo/secret prompts on diff --git a/tests/tui_gateway/test_protocol.py b/tests/tui_gateway/test_protocol.py index 2e64cc2605e22..1f742fa5e4b27 100644 --- a/tests/tui_gateway/test_protocol.py +++ b/tests/tui_gateway/test_protocol.py @@ -1947,6 +1947,67 @@ def test_command_dispatch_retry_skips_display_kind_timeline_rows(server): ] +def test_command_dispatch_retry_resends_prompt_merged_onto_marker(server): + """/retry must resend a real prompt alternation-repair merged onto a + bookkeeping marker's tail, not skip past it as pure bookkeeping (#74350). + + Reproduces the exact shape repair_message_sequence produces: a + personality-switch marker followed immediately by a real user turn with + no assistant reply in between gets merged into one row whose `content` + is `marker + "\\n\\n" + real_text` but whose `display_kind` survives the + merge untouched — except now `merged_real_turn` is also set so /retry + (and /undo, and rewind) still recognize it as a real turn. + """ + sid = "test-session-retry-merged-marker" + marker = ( + "[System: The user has changed the assistant's personality. " + "From this point forward, adopt the following persona and respond " + "accordingly: You are a pirate.]" + ) + history = [ + {"role": "user", "content": "first question"}, + {"role": "assistant", "content": "first answer"}, + { + "role": "user", + "content": marker + "\n\n" + "what's the weather like today?", + "display_kind": "personality_switch", + "display_metadata": {"marker_text": marker}, + "merged_real_turn": True, + }, + ] + server._sessions[sid] = { + "session_key": sid, + "agent": None, + "history": history, + "history_lock": threading.Lock(), + "history_version": 0, + } + # Captured before dispatch: /retry truncates `history` in place, so + # reading history[-1] afterward would see "first answer" instead. + expected_message = history[-1]["content"] + + resp = server.handle_request({ + "id": "r4c", + "method": "command.dispatch", + "params": {"name": "retry", "session_id": sid}, + }) + + assert "error" not in resp + result = resp["result"] + assert result["type"] == "send" + # The resent text is the merged row's full content (marker + real + # prompt) since /retry resends raw `content`, not the display + # projection — the point under test is that this row was selected as + # the retry target at all, instead of /retry falling through to the + # error path or an earlier real turn. + assert result["message"] == expected_message + # Truncated through the merged row itself (and nothing earlier). + assert [m["content"] for m in server._sessions[sid]["history"]] == [ + "first question", + "first answer", + ] + + def test_command_dispatch_retry_empty_history(server): """command.dispatch /retry with empty history returns error.""" sid = "test-session" diff --git a/tui_gateway/server.py b/tui_gateway/server.py index ebea640821b86..cdcb16afd9512 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -10446,15 +10446,19 @@ def _(rid, params: dict) -> dict: removed = 0 with session["history_lock"]: history = session.get("history", []) - # Truncate from the last *real* user turn (no display_kind). Popping - # only trailing assistant/tool then one user left timeline markers - # (async_delegation_complete, model_switch, …) as the undo target — - # so session.undo removed bookkeeping instead of the last exchange. - # Match list_recent_user_messages / CLI turn counting. + # Truncate from the last *real* user turn (no display_kind, or a + # merged row where alternation-repair concatenated a real prompt + # onto a bookkeeping marker's tail — see merged_real_turn, #74350). + # Popping only trailing assistant/tool then one user left timeline + # markers (async_delegation_complete, model_switch, …) as the undo + # target — so session.undo removed bookkeeping instead of the last + # exchange. Match list_recent_user_messages / CLI turn counting. last_user_idx = None for i in range(len(history) - 1, -1, -1): msg = history[i] - if msg.get("role") == "user" and not msg.get("display_kind"): + if msg.get("role") == "user" and ( + not msg.get("display_kind") or msg.get("merged_real_turn") + ): last_user_idx = i break if last_user_idx is not None: @@ -11330,7 +11334,9 @@ def _(rid, params: dict) -> dict: history = session.get("history", []) user_indices = [ i for i, m in enumerate(history) - if m.get("role") == "user" and not m.get("display_kind") + if m.get("role") == "user" and ( + not m.get("display_kind") or m.get("merged_real_turn") + ) ] # Reject out-of-range ordinals on BOTH ends. A negative value would # otherwise sail past the upper-bound check and hit Python's negative @@ -16184,11 +16190,16 @@ def _(rid, params: dict) -> dict: # them as user turns — same predicate as CLI resume/count and the # prompt.submit ordinal fix. Without this, /retry re-sends opaque # markers (model_switch / async_delegation_complete / auto_continue) - # and truncates only the marker instead of the failed exchange. + # and truncates only the marker instead of the failed exchange. A + # merged row (real prompt concatenated onto a marker's tail by + # alternation-repair) still counts as real via merged_real_turn + # (#74350). last_user_idx = None for i in range(len(history) - 1, -1, -1): msg = history[i] - if msg.get("role") == "user" and not msg.get("display_kind"): + if msg.get("role") == "user" and ( + not msg.get("display_kind") or msg.get("merged_real_turn") + ): last_user_idx = i break if last_user_idx is None: @@ -18948,12 +18959,15 @@ def go(mgr, cwd): removed = 0 with session["history_lock"]: history = session.get("history", []) - # Truncate from the last *real* user turn (no display_kind). + # Truncate from the last *real* user turn (no display_kind, + # or a merged row via merged_real_turn, #74350). # Same predicate as list_recent_user_messages / /undo / /retry. last_user_idx = None for i in range(len(history) - 1, -1, -1): msg = history[i] - if msg.get("role") == "user" and not msg.get("display_kind"): + if msg.get("role") == "user" and ( + not msg.get("display_kind") or msg.get("merged_real_turn") + ): last_user_idx = i break if last_user_idx is not None: