diff --git a/cli.py b/cli.py index caf31036c87a..af020b9692bd 100644 --- a/cli.py +++ b/cli.py @@ -8079,10 +8079,13 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin): print("(._.) No messages to retry.") return None - # Walk backwards to find the last user message + # Walk backwards to the last *real* user message. Timeline bookkeeping + # rows (display_kind set) are role=user but are not user turns — match + # CLI resume counting and list_recent_user_messages. last_user_idx = None for i in range(len(self.conversation_history) - 1, -1, -1): - if self.conversation_history[i].get("role") == "user": + msg = self.conversation_history[i] + if msg.get("role") == "user" and not msg.get("display_kind"): last_user_idx = i break @@ -8127,10 +8130,13 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin): if n < 1: n = 1 - # Walk backwards collecting the indices of the last N user messages. + # Walk backwards collecting the indices of the last N *real* user + # messages (exclude display_kind timeline rows — same predicate as + # list_recent_user_messages and resume turn counting). user_indices = [] for i in range(len(self.conversation_history) - 1, -1, -1): - if self.conversation_history[i].get("role") == "user": + msg = self.conversation_history[i] + if msg.get("role") == "user" and not msg.get("display_kind"): user_indices.append(i) if len(user_indices) >= n: break diff --git a/gateway/slash_commands.py b/gateway/slash_commands.py index cf1d44929cbc..e0f58bf7b23b 100644 --- a/gateway/slash_commands.py +++ b/gateway/slash_commands.py @@ -2603,12 +2603,17 @@ class GatewaySlashCommandsMixin: session_entry = await self.async_session_store.get_or_create_session(source) history = await self.async_session_store.load_transcript(session_entry.session_id) - # Find the last user message + # Find the last *real* user message. Timeline bookkeeping rows carry + # role=user + display_kind (model_switch / async_delegation_complete / + # auto_continue / hidden); clients never count them as user turns. + # Without this filter /retry rewrote the transcript around a marker + # and re-sent opaque bookkeeping text (same class as the TUI ordinal). last_user_msg = None last_user_idx = None for i in range(len(history) - 1, -1, -1): - if history[i].get("role") == "user": - last_user_msg = history[i].get("content", "") + msg = history[i] + if msg.get("role") == "user" and not msg.get("display_kind"): + last_user_msg = msg.get("content", "") last_user_idx = i break diff --git a/hermes_state.py b/hermes_state.py index 8de6faffbbcd..5b12134cb2e6 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -8049,16 +8049,27 @@ class SessionDB: Each entry is a dict with keys ``id``, ``timestamp``, ``preview``. ``preview`` is the first 80 characters of the message content (with line breaks collapsed to spaces). Used by the /rewind - slash command picker. + slash command picker, CLI/TUI/gateway ``/undo [N]``, and any other + caller that needs real user-turn targets. + + Bookkeeping timeline rows (``display_kind`` set — e.g. model_switch, + async_delegation_complete, auto_continue, hidden) are excluded. They + are durable ``role='user'`` rows for the API transcript, but no client + counts them as user turns (desktop demotes them to system / drops them; + the CLI already uses ``not m.get("display_kind")``). Including them here + made ``/undo`` soft-delete from a marker instead of the last real turn — + same class of index skew as the prompt.submit ordinal bug. By default only active messages are returned. """ active_clause = "" if include_inactive else " AND active = 1" + # Match CLI/desktop: only real user turns, not timeline bookkeeping. + display_clause = " AND (display_kind IS NULL OR display_kind = '')" with self._lock: cursor = self._conn.execute( "SELECT id, timestamp, content FROM messages " "WHERE session_id = ? AND role = 'user'" - f"{active_clause} " + f"{active_clause}{display_clause} " "ORDER BY id DESC LIMIT ?", (session_id, int(limit)), ) diff --git a/tests/tui_gateway/test_protocol.py b/tests/tui_gateway/test_protocol.py index 7e55821301f0..2e64cc2605e2 100644 --- a/tests/tui_gateway/test_protocol.py +++ b/tests/tui_gateway/test_protocol.py @@ -1908,6 +1908,45 @@ def test_command_dispatch_retry_finds_last_user_message(server): assert server._sessions[sid]["history_version"] == 1 +def test_command_dispatch_retry_skips_display_kind_timeline_rows(server): + """/retry must resend the last real user turn, not a display_kind marker.""" + sid = "test-session-retry-timeline" + history = [ + {"role": "user", "content": "first question"}, + {"role": "assistant", "content": "first answer"}, + {"role": "user", "content": "second question"}, + {"role": "assistant", "content": "second answer"}, + { + "role": "user", + "content": "background agent work finished", + "display_kind": "async_delegation_complete", + }, + ] + server._sessions[sid] = { + "session_key": sid, + "agent": None, + "history": history, + "history_lock": threading.Lock(), + "history_version": 0, + } + + resp = server.handle_request({ + "id": "r4b", + "method": "command.dispatch", + "params": {"name": "retry", "session_id": sid}, + }) + + assert "error" not in resp + result = resp["result"] + assert result["type"] == "send" + assert result["message"] == "second question" + # Truncated through the real last user turn (and the trailing marker). + 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/tests/tui_gateway/test_undo_command.py b/tests/tui_gateway/test_undo_command.py index cd0f7e5c4e0b..e95e9ef9a2fd 100644 --- a/tests/tui_gateway/test_undo_command.py +++ b/tests/tui_gateway/test_undo_command.py @@ -176,6 +176,56 @@ def test_undo_refuses_when_session_busy(server, session_with_history): assert "busy" in resp["error"]["message"].lower() +def test_undo_skips_display_kind_timeline_rows(server, db): + """/undo must target real user turns, not durable timeline bookkeeping. + + async_delegation_complete / model_switch / auto_continue rows are stored + as role=user with display_kind set. Clients never count them as user turns. + Without the list_recent_user_messages filter, /undo 1 soft-deleted only + the trailing marker and left the last real exchange intact. + """ + sid = "sid-undo-timeline" + session_key = "tui-undo-timeline" + db.create_session(session_key, source="tui") + db.append_message(session_key, "user", "question 1") + db.append_message(session_key, "assistant", "answer 1") + db.append_message(session_key, "user", "question 2") + db.append_message(session_key, "assistant", "answer 2") + db.append_message( + session_key, + "user", + "background agent work finished", + display_kind="async_delegation_complete", + ) + history = db.get_messages_as_conversation(session_key) + agent = MagicMock() + agent._memory_manager = MagicMock() + agent._last_flushed_db_idx = len(history) + s = { + "session_key": session_key, + "history": list(history), + "history_lock": threading.Lock(), + "history_version": 0, + "running": False, + "agent": agent, + "attached_images": [], + "cols": 120, + } + server._sessions[sid] = s + server._db = db + + resp = _call(server, "command.dispatch", session_id=sid, name="undo", arg="") + result = resp["result"] + assert result["type"] == "prefill" + # Must prefill the last *real* user turn, not the marker text. + assert result["message"] == "question 2" + # Active history: only q1/a1 remain (q2+a2+marker soft-deleted). + assert len(s["history"]) == 2 + assert [m.get("content") for m in s["history"]] == ["question 1", "answer 1"] + active = db.get_messages(session_key, include_inactive=False) + assert [r["content"] for r in active] == ["question 1", "answer 1"] + + def test_undo_errors_when_no_active_session(server): resp = _call(server, "command.dispatch", session_id="no-such-sid", name="undo", arg="") assert "error" in resp diff --git a/tui_gateway/server.py b/tui_gateway/server.py index b555504dfe28..9f7d31f895b1 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -10122,13 +10122,20 @@ def _(rid, params: dict) -> dict: removed = 0 with session["history_lock"]: history = session.get("history", []) - while history and history[-1].get("role") in {"assistant", "tool"}: - history.pop() - removed += 1 - if history and history[-1].get("role") == "user": - history.pop() - removed += 1 - if removed: + # 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. + 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"): + last_user_idx = i + break + if last_user_idx is not None: + removed = len(history) - last_user_idx + del history[last_user_idx:] session["history_version"] = int(session.get("history_version", 0)) + 1 return _ok(rid, {"removed": removed}) @@ -15719,10 +15726,16 @@ def _(rid, params: dict) -> dict: history = session.get("history", []) if not history: return _err(rid, 4018, "no previous user message to retry") - # Walk backwards to find the last user message + # Walk backwards to the last *real* user turn. Timeline bookkeeping + # rows (display_kind set) are durable role=user but no client counts + # 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. last_user_idx = None for i in range(len(history) - 1, -1, -1): - if history[i].get("role") == "user": + msg = history[i] + if msg.get("role") == "user" and not msg.get("display_kind"): last_user_idx = i break if last_user_idx is None: