diff --git a/api/models.py b/api/models.py index 7d2cccdc..c0ee5b4f 100644 --- a/api/models.py +++ b/api/models.py @@ -1618,10 +1618,24 @@ def get_cli_sessions() -> list: return cli_sessions +def _json_loads_if_string(value): + if not isinstance(value, str): + return value + text = value.strip() + if not text: + return None + try: + return json.loads(text) + except Exception: + return value + + def get_cli_session_messages(sid) -> list: """Read messages for a single CLI/external-agent session. - Returns a list of {role, content, timestamp} dicts. - Returns empty list on any error. + + Preserve tool-call/result and reasoning metadata from the agent state.db so + CLI-origin transcripts render with the same tool cards as WebUI-native + sessions. Returns empty list on any error. """ import os if str(sid or '').startswith(f'{CLAUDE_CODE_SOURCE}_'): @@ -1644,19 +1658,47 @@ def get_cli_session_messages(sid) -> list: with closing(sqlite3.connect(str(db_path))) as conn: conn.row_factory = sqlite3.Row cur = conn.cursor() - cur.execute(""" - SELECT role, content, timestamp + cur.execute("PRAGMA table_info(messages)") + available = {str(row['name']) for row in cur.fetchall()} + required = {'role', 'content', 'timestamp'} + if not required.issubset(available): + return [] + optional = [ + 'tool_call_id', + 'tool_calls', + 'tool_name', + 'reasoning', + 'reasoning_details', + 'codex_reasoning_items', + 'reasoning_content', + 'codex_message_items', + ] + selected = ['role', 'content', 'timestamp'] + [c for c in optional if c in available] + cur.execute(f""" + SELECT {', '.join(selected)} FROM messages WHERE session_id = ? ORDER BY timestamp ASC """, (sid,)) msgs = [] for row in cur.fetchall(): - msgs.append({ + msg = { 'role': row['role'], 'content': row['content'], 'timestamp': row['timestamp'], - }) + } + for col in optional: + if col not in row.keys(): + continue + value = row[col] + if value in (None, ''): + continue + if col in {'tool_calls', 'reasoning_details', 'codex_reasoning_items', 'codex_message_items'}: + value = _json_loads_if_string(value) + msg[col] = value + if msg.get('role') == 'tool' and msg.get('tool_name') and not msg.get('name'): + msg['name'] = msg['tool_name'] + msgs.append(msg) except Exception: return [] return msgs diff --git a/api/routes.py b/api/routes.py index 0b8838cf..e644b193 100644 --- a/api/routes.py +++ b/api/routes.py @@ -7861,6 +7861,49 @@ def _normalize_message_for_import_refresh(message: object) -> object: return normalized +def _message_has_cli_tool_metadata(message: object) -> bool: + if not isinstance(message, dict): + return False + if message.get("role") == "assistant" and message.get("tool_calls"): + return True + if message.get("role") == "tool" and (message.get("tool_call_id") or message.get("tool_name") or message.get("name")): + return True + return False + + +def _strip_cli_tool_metadata_for_refresh(message: object) -> object: + if not isinstance(message, dict): + return _normalize_message_for_import_refresh(message) + normalized = _normalize_message_for_import_refresh(message) + if not isinstance(normalized, dict): + return normalized + for key in ("tool_calls", "tool_call_id", "tool_name", "name"): + normalized.pop(key, None) + return normalized + + +def _is_cli_tool_metadata_enrichment(existing_messages: list, fresh_messages: list) -> bool: + """Return True when fresh messages only add CLI tool metadata. + + Older imports from get_cli_session_messages() persisted assistant/tool rows + without tool_calls, tool_call_id, or tool_name. After #1772 the refreshed + transcript can have the same length but richer metadata, so re-imports must + rebuild the stored sidecar even without a new row. + """ + if not isinstance(existing_messages, list) or not isinstance(fresh_messages, list): + return False + if len(existing_messages) != len(fresh_messages): + return False + if any(_message_has_cli_tool_metadata(m) for m in existing_messages): + return False + if not any(_message_has_cli_tool_metadata(m) for m in fresh_messages): + return False + for idx, existing_message in enumerate(existing_messages): + if _strip_cli_tool_metadata_for_refresh(existing_message) != _strip_cli_tool_metadata_for_refresh(fresh_messages[idx]): + return False + return True + + def _is_messages_refresh_prefix_match(existing_messages: list, fresh_messages: list) -> bool: """Return True when existing_messages is a prefix of fresh_messages by value. @@ -7905,6 +7948,11 @@ def _handle_session_import_cli(handler, body): if _is_messages_refresh_prefix_match(existing.messages, fresh_msgs): existing.messages = fresh_msgs changed = True + elif fresh_msgs and _is_cli_tool_metadata_enrichment(existing.messages, fresh_msgs): + # Same row count, richer payload: rebuild sidecars imported before + # CLI tool metadata was preserved (#1772). + existing.messages = fresh_msgs + changed = True if cli_meta: updates = { "is_cli_session": True, diff --git a/docs/pr-media/1772/cli-tool-metadata-api-evidence.json b/docs/pr-media/1772/cli-tool-metadata-api-evidence.json new file mode 100644 index 00000000..80cea476 --- /dev/null +++ b/docs/pr-media/1772/cli-tool-metadata-api-evidence.json @@ -0,0 +1,25 @@ +{ + "issue": 1772, + "check": "api.models.get_cli_session_messages preserves CLI tool metadata for WebUI rendering", + "session_id": "cli_issue_1772_demo", + "message_count": 2, + "assistant_tool_calls": [ + { + "id": "call_1772_demo", + "type": "function", + "function": { + "name": "terminal", + "arguments": "{\"command\": \"printf ok\"}" + } + } + ], + "tool_result": { + "role": "tool", + "tool_call_id": "call_1772_demo", + "tool_name": "terminal", + "name": "terminal", + "content": { + "output": "ok" + } + } +} diff --git a/tests/test_cli_session_tool_metadata.py b/tests/test_cli_session_tool_metadata.py new file mode 100644 index 00000000..ceb8a367 --- /dev/null +++ b/tests/test_cli_session_tool_metadata.py @@ -0,0 +1,152 @@ +"""Regression coverage for CLI session tool-call metadata import (#1772).""" + +from __future__ import annotations + +import json +import sqlite3 + +import api.models as models + + +def _patch_active_home(monkeypatch, home): + import api.profiles as profiles + + monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: home) + monkeypatch.setattr(profiles, "get_active_profile_name", lambda: None) + + +def _create_state_db_with_tool_turn(path, session_id="cli_tool_session_001"): + conn = sqlite3.connect(str(path)) + conn.execute( + """ + CREATE TABLE messages ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + session_id TEXT NOT NULL, + role TEXT NOT NULL, + content TEXT, + tool_call_id TEXT, + tool_calls TEXT, + tool_name TEXT, + timestamp REAL NOT NULL, + token_count INTEGER, + finish_reason TEXT, + reasoning TEXT, + reasoning_details TEXT, + codex_reasoning_items TEXT, + reasoning_content TEXT, + codex_message_items TEXT + ) + """ + ) + tool_calls = [ + { + "id": "call_123", + "type": "function", + "function": { + "name": "terminal", + "arguments": json.dumps({"command": "printf ok"}), + }, + } + ] + conn.execute( + """ + INSERT INTO messages ( + session_id, role, content, tool_calls, timestamp, reasoning, reasoning_content + ) VALUES (?, 'assistant', '', ?, 1.0, 'Need a shell check', 'Need a shell check') + """, + (session_id, json.dumps(tool_calls)), + ) + conn.execute( + """ + INSERT INTO messages ( + session_id, role, content, tool_call_id, tool_name, timestamp + ) VALUES (?, 'tool', ?, 'call_123', 'terminal', 2.0) + """, + (session_id, json.dumps({"output": "ok"})), + ) + conn.commit() + conn.close() + return tool_calls + + +def test_get_cli_session_messages_preserves_tool_call_metadata(tmp_path, monkeypatch): + hermes_home = tmp_path / "hermes" + hermes_home.mkdir() + _patch_active_home(monkeypatch, hermes_home) + expected_tool_calls = _create_state_db_with_tool_turn(hermes_home / "state.db") + + messages = models.get_cli_session_messages("cli_tool_session_001") + + assert messages[0]["role"] == "assistant" + assert messages[0]["content"] == "" + assert messages[0]["tool_calls"] == expected_tool_calls + assert messages[0]["reasoning"] == "Need a shell check" + assert messages[0]["reasoning_content"] == "Need a shell check" + assert messages[1]["role"] == "tool" + assert messages[1]["tool_call_id"] == "call_123" + assert messages[1]["tool_name"] == "terminal" + assert messages[1]["name"] == "terminal" + assert json.loads(messages[1]["content"])["output"] == "ok" + + +def test_existing_cli_import_refreshes_same_length_tool_metadata(monkeypatch): + """Previously imported CLI sessions with stripped metadata must be rebuilt. + + The broken importer saved the same assistant/tool rows without tool_calls, + tool_call_id, or tool_name. A later import after the loader fix has the same + message count, so the refresh path must still replace the stripped messages. + """ + import api.routes as routes + + session_id = "existing_cli_tool_session_001" + stripped = [ + {"role": "assistant", "content": "", "timestamp": 1.0}, + {"role": "tool", "content": json.dumps({"output": "ok"}), "timestamp": 2.0}, + ] + enriched = [ + { + "role": "assistant", + "content": "", + "timestamp": 1.0, + "tool_calls": [{"id": "call_123", "function": {"name": "terminal", "arguments": "{}"}}], + }, + { + "role": "tool", + "content": json.dumps({"output": "ok"}), + "timestamp": 2.0, + "tool_call_id": "call_123", + "tool_name": "terminal", + "name": "terminal", + }, + ] + + class FakeSession: + def __init__(self): + self.messages = list(stripped) + self.source_tag = "cli" + self.raw_source = "cli" + self.session_source = "cli" + self.source_label = "CLI" + self.parent_session_id = None + self.is_cli_session = True + + def compact(self): + return {"session_id": session_id, "title": "Imported CLI"} + + def save(self, touch_updated_at=False): + save_calls.append(touch_updated_at) + + save_calls = [] + existing = FakeSession() + monkeypatch.setattr(routes.Session, "load", classmethod(lambda _cls, sid: existing if sid == session_id else None)) + monkeypatch.setattr(routes, "require", lambda body, *keys: None) + monkeypatch.setattr(routes, "j", lambda _handler, payload, status=200, extra_headers=None: payload) + monkeypatch.setattr(routes, "get_cli_session_messages", lambda sid: enriched if sid == session_id else []) + monkeypatch.setattr(routes, "get_cli_sessions", lambda: [{"session_id": session_id, "source_tag": "cli", "raw_source": "cli", "session_source": "cli", "source_label": "CLI"}]) + + response = routes._handle_session_import_cli(object(), {"session_id": session_id}) + + assert response["imported"] is False + assert existing.messages == enriched + assert response["session"]["messages"] == enriched + assert save_calls == [False]