Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions agent/agent_runtime_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
224 changes: 224 additions & 0 deletions tests/test_tui_gateway_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -2234,6 +2234,100 @@ 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}
# 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)

# 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
Expand Down Expand Up @@ -8527,6 +8621,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:
Expand Down Expand Up @@ -8888,6 +9022,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
Expand Down
61 changes: 61 additions & 0 deletions tests/tui_gateway/test_protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,67 @@ def test_skills_manage_search_uses_tools_hub_sources(server):
search.assert_called_once_with("showroom", ["source"], source_filter="all", limit=20)


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",
]


# ── dispatch(): pool routing for long handlers (#12546) ──────────────


Expand Down
4 changes: 3 additions & 1 deletion tui_gateway/methods_prompt.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,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
Expand Down
16 changes: 10 additions & 6 deletions tui_gateway/methods_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -2296,15 +2296,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:
Expand Down
Loading
Loading