diff --git a/api/helpers.py b/api/helpers.py index e9abdbc1b0c..f369639c986 100644 --- a/api/helpers.py +++ b/api/helpers.py @@ -367,9 +367,9 @@ def _redact_value(v, *, _enabled: bool | None = None): def redact_session_data(session_dict: dict) -> dict: - """Redact credentials from message content and tool_call data before API response. + """Redact credentials from message content, tool data, and session sidecars. - Applies to: messages[], tool_calls[], and title. + Applies to: messages[], tool_calls[], todo_state, and title. The underlying session file is not modified; redaction is response-layer only. Reads the ``api_redact_enabled`` setting ONCE for the entire response and @@ -387,6 +387,8 @@ def redact_session_data(session_dict: dict) -> dict: result['messages'] = _redact_value(result['messages'], _enabled=_enabled) if 'tool_calls' in result: result['tool_calls'] = _redact_value(result['tool_calls'], _enabled=_enabled) + if 'todo_state' in result: + result['todo_state'] = _redact_value(result['todo_state'], _enabled=_enabled) return result diff --git a/api/routes.py b/api/routes.py index 0d8a1d0be0b..07b26844f32 100644 --- a/api/routes.py +++ b/api/routes.py @@ -2854,6 +2854,7 @@ def _keep_latest_messaging_session_per_source( read_run_events, stale_interrupted_event, ) +from api.todo_state import attach_todo_state from api.providers import get_providers, get_provider_quota, get_provider_cost_history, set_provider_key, remove_provider_key from api.onboarding import ( apply_onboarding_setup, @@ -4714,6 +4715,14 @@ def handle_get(handler, parsed) -> bool: journal, active=bool(getattr(s, "active_stream_id", None)), ) + # Cold-load: derive the latest settled todo snapshot from the full + # merged transcript, not the truncated display window. This keeps + # the Todos panel correct after refresh even when the latest todo + # tool result is outside msg_limit, and treats an explicit empty + # todo list as the current state instead of falling through to an + # older non-empty write. + if load_messages and _all_msgs: + attach_todo_state(raw, _all_msgs) if _merged_last_message_at: raw["last_message_at"] = max( float(raw.get("last_message_at") or 0), @@ -4789,6 +4798,7 @@ def handle_get(handler, parsed) -> bool: "messages": msgs, "tool_calls": [], } + attach_todo_state(sess, msgs) sess = _merge_cli_sidebar_metadata(sess, cli_meta) return j(handler, {"session": redact_session_data(sess)}) return bad(handler, "Session not found", 404) diff --git a/api/todo_state.py b/api/todo_state.py new file mode 100644 index 00000000000..cc3a637480e --- /dev/null +++ b/api/todo_state.py @@ -0,0 +1,122 @@ +"""Derive settled todo snapshots for session GET responses. + +The browser's Todos panel currently reconstructs state by reverse-scanning +loaded tool messages. That works only when the latest todo tool result is inside +the returned message window. This helper gives ``/api/session`` a compact, +explicit ``todo_state`` sidecar derived from the full settled transcript. + +The detector intentionally mirrors ``run_agent.AIAgent._hydrate_todo_store``: +walk messages newest-first and use the first tool message whose JSON content has +a ``todos`` list. Empty ``todos`` is a valid snapshot so a cleared task list does +not fall through to an older non-empty write. +""" + +from __future__ import annotations + +import json +import logging +from typing import Any, Iterable, Optional, Sequence + +logger = logging.getLogger(__name__) + +VERSION = 1 +PAYLOAD_KEY = "todo_state" + + +def _normalize_snapshot(data: Any) -> Optional[dict]: + """Return the canonical todo snapshot shape, or ``None`` for non-todo data.""" + if not isinstance(data, dict): + return None + todos = data.get("todos") + if not isinstance(todos, list): + return None + summary = data.get("summary") + if not isinstance(summary, dict): + summary = {} + return { + "todos": todos, + "summary": summary, + "version": VERSION, + } + + +def parse_todo_tool_result(function_result: Any) -> Optional[dict]: + """Parse a todo tool result JSON string or pre-parsed dict into a snapshot.""" + data: Any = function_result + if isinstance(function_result, str): + try: + data = json.loads(function_result) + except (TypeError, ValueError): + return None + return _normalize_snapshot(data) + + +def _message_ts_float(ts_raw: Any) -> float: + """Coerce a message ``timestamp`` field to a positive float, or 0.0.""" + try: + return float(ts_raw) if ts_raw is not None else 0.0 + except (TypeError, ValueError): + return 0.0 + + +def _max_timestamp_through(messages: Sequence[Any], upto_idx: int) -> float: + """Return the largest valid timestamp at or before ``upto_idx``.""" + best = 0.0 + end = min(upto_idx, len(messages) - 1) + for i in range(end, -1, -1): + msg = messages[i] + if not isinstance(msg, dict): + continue + best = max(best, _message_ts_float(msg.get("timestamp"))) + return best + + +def derive_todo_state(messages: Optional[Iterable[dict]]) -> Optional[dict]: + """Derive the latest settled todo snapshot from conversation history. + + Returns ``None`` when the session has no todo writes. Malformed or + non-string tool contents are skipped so unrelated tool results never break + session loading. + """ + if not messages: + return None + if not isinstance(messages, (list, tuple)): + messages = list(messages) + + for idx in range(len(messages) - 1, -1, -1): + msg = messages[idx] + if not isinstance(msg, dict) or msg.get("role") != "tool": + continue + content = msg.get("content", "") + if not isinstance(content, str) or '"todos"' not in content: + continue + snapshot = parse_todo_tool_result(content) + if snapshot is None: + continue + + ts_val = _message_ts_float(msg.get("timestamp")) + if ts_val <= 0: + ts_val = _max_timestamp_through(messages, idx) + if ts_val > 0: + snapshot["ts"] = ts_val + return snapshot + return None + + +def attach_todo_state(payload: dict, messages: Optional[Iterable[dict]]) -> bool: + """Attach ``todo_state`` to a session payload when one can be derived. + + Mutates ``payload`` in place. Errors are swallowed deliberately: a malformed + historical tool message must not make ``/api/session`` fail. + """ + if not messages: + return False + try: + snapshot = derive_todo_state(messages) + if snapshot is None: + return False + payload[PAYLOAD_KEY] = snapshot + return True + except Exception: + logger.debug("todo_state attach failed", exc_info=True) + return False diff --git a/static/panels.js b/static/panels.js index a244470191a..8e6bfde2a47 100644 --- a/static/panels.js +++ b/static/panels.js @@ -2650,37 +2650,50 @@ async function loadKanbanTask(taskId){ function loadTodos() { const panel = $('todoPanel'); if (!panel) return; - const sourceMessages = (S.session && Array.isArray(S.session.messages) && S.session.messages.length) ? S.session.messages : S.messages; - // Parse the most recent todo state from message history - let todos = []; - for (let i = sourceMessages.length - 1; i >= 0; i--) { - const m = sourceMessages[i]; - if (m && m.role === 'tool') { - try { - const d = JSON.parse(typeof m.content === 'string' ? m.content : JSON.stringify(m.content)); - if (d && Array.isArray(d.todos) && d.todos.length) { - todos = d.todos; - break; - } - } catch(e) {} - } + + const sessionTodoState = S.session && S.session.todo_state; + let todos; + if (sessionTodoState && Array.isArray(sessionTodoState.todos)) { + todos = sessionTodoState.todos; + } else { + todos = _legacyTodosFromMessages(); } + if (!todos.length) { panel.innerHTML = `
${esc(t('todos_no_active'))}
`; return; } const statusIcon = {pending:li('square',14), in_progress:li('loader',14), completed:li('check',14), cancelled:li('x',14)}; const statusColor = {pending:'var(--muted)', in_progress:'var(--blue)', completed:'rgba(100,200,100,.8)', cancelled:'rgba(200,100,100,.5)'}; - panel.innerHTML = todos.map(t => ` + panel.innerHTML = todos.map(todo => `
- ${statusIcon[t.status]||li('square',14)} + ${statusIcon[todo.status]||li('square',14)}
-
${esc(t.content)}
-
${esc(t.id)} · ${esc(t.status)}
+
${esc(todo.content)}
+
${esc(todo.id)} · ${esc(todo.status)}
`).join(''); } +function _legacyTodosFromMessages() { + const sourceMessages = (S.session && Array.isArray(S.session.messages) && S.session.messages.length) ? S.session.messages : S.messages; + if (!Array.isArray(sourceMessages)) return []; + for (let i = sourceMessages.length - 1; i >= 0; i--) { + const m = sourceMessages[i]; + if (!m || m.role !== 'tool') continue; + let content = m.content; + if (typeof content !== 'string') { + try { content = JSON.stringify(content); } catch (_) { continue; } + } + if (!content || content.indexOf('"todos"') < 0) continue; + try { + const d = JSON.parse(content); + if (d && Array.isArray(d.todos) && d.todos.length) return d.todos; + } catch (_) {} + } + return []; +} + // ──────────────────────────────────────────────────────────────────────────── // Kanban: multi-board switcher + create/rename/archive modal // ──────────────────────────────────────────────────────────────────────────── diff --git a/static/sessions.js b/static/sessions.js index a3a5bb45802..354609fcbc5 100644 --- a/static/sessions.js +++ b/static/sessions.js @@ -1387,6 +1387,8 @@ async function _ensureMessagesLoaded(sid) { S.messages = msgs; if(S.session&&S.session.session_id===sid){ S.session.message_count=Number(data.session.message_count || msgs.length); + if(Object.prototype.hasOwnProperty.call(data.session,'todo_state')) S.session.todo_state=data.session.todo_state; + else delete S.session.todo_state; S.lastUsage={...(data.session.last_usage||S.lastUsage||{})}; _setSessionViewedCount(sid, Number(S.session.message_count || msgs.length)); } diff --git a/tests/test_security_redaction.py b/tests/test_security_redaction.py index eb8cc9053e4..86a22e50684 100644 --- a/tests/test_security_redaction.py +++ b/tests/test_security_redaction.py @@ -283,6 +283,26 @@ def test_redact_session_data_messages(): assert result["session_id"] == "abc123" assert result["messages"][1]["content"] == "sure" +def test_redact_session_data_todo_state_sidecar(): + """redact_session_data masks credentials in derived todo_state sidecars.""" + from api.helpers import redact_session_data + session = { + "session_id": "todo-redact", + "messages": [], + "tool_calls": [], + "todo_state": { + "todos": [ + {"id": "1", "content": f"rotate api key {_FAKE_SK_KEY}", "status": "pending"}, + ], + "summary": {"total": 1, "pending": 1}, + "version": 1, + }, + } + result = redact_session_data(session) + dump = json.dumps(result) + _assert_no_plaintext_credentials(dump, "todo_state redaction") + assert result["todo_state"]["todos"][0]["status"] == "pending" + def test_redact_session_data_multiple_cred_types(): """redact_session_data handles sk-, ghp_, hf_, and AKIA keys.""" diff --git a/tests/test_session_todo_state_route.py b/tests/test_session_todo_state_route.py new file mode 100644 index 00000000000..12e75a1e69a --- /dev/null +++ b/tests/test_session_todo_state_route.py @@ -0,0 +1,47 @@ +from __future__ import annotations + +import ast +from pathlib import Path + + +ROUTES_PY = Path(__file__).parent.parent / "api" / "routes.py" + + +def _attach_todo_state_calls() -> list[ast.Call]: + tree = ast.parse(ROUTES_PY.read_text(encoding="utf-8")) + calls: list[ast.Call] = [] + for node in ast.walk(tree): + if not isinstance(node, ast.Call): + continue + func = node.func + if isinstance(func, ast.Name) and func.id == "attach_todo_state": + calls.append(node) + return calls + + +def test_routes_imports_attach_todo_state(): + tree = ast.parse(ROUTES_PY.read_text(encoding="utf-8")) + + assert any( + isinstance(node, ast.ImportFrom) + and node.module == "api.todo_state" + and any(alias.name == "attach_todo_state" for alias in node.names) + for node in ast.walk(tree) + ) + + +def test_routes_attach_todo_state_from_webui_and_cli_session_paths(): + calls = _attach_todo_state_calls() + + assert len(calls) >= 2 + arg_names = [] + for call in calls[:2]: + assert len(call.args) == 2 + assert not call.keywords + assert isinstance(call.args[0], ast.Name) + assert isinstance(call.args[1], ast.Name) + assert call.args[0].id != call.args[1].id + arg_names.append((call.args[0].id, call.args[1].id)) + + assert ("raw", "_all_msgs") in arg_names + assert ("sess", "msgs") in arg_names diff --git a/tests/test_todo_panel_cold_load_static.py b/tests/test_todo_panel_cold_load_static.py new file mode 100644 index 00000000000..8754dd4bc42 --- /dev/null +++ b/tests/test_todo_panel_cold_load_static.py @@ -0,0 +1,35 @@ +from pathlib import Path + + +REPO_ROOT = Path(__file__).parent.parent + + +def test_ensure_messages_loaded_copies_session_todo_state_sidecar(): + src = (REPO_ROOT / "static" / "sessions.js").read_text(encoding="utf-8") + + assert "Object.prototype.hasOwnProperty.call(data.session,'todo_state')" in src + assert "S.session.todo_state=data.session.todo_state" in src + assert "else delete S.session.todo_state" in src + + +def test_load_todos_prefers_session_todo_state_before_legacy_scan(): + src = (REPO_ROOT / "static" / "panels.js").read_text(encoding="utf-8") + start = src.find("function loadTodos()") + end = src.find("function _legacyTodosFromMessages()") + + assert start != -1 + assert end != -1 + load_todos = src[start:end] + + assert "const sessionTodoState = S.session && S.session.todo_state;" in load_todos + assert "sessionTodoState && Array.isArray(sessionTodoState.todos)" in load_todos + assert "todos = sessionTodoState.todos;" in load_todos + assert "todos = _legacyTodosFromMessages();" in load_todos + assert load_todos.find("todos = sessionTodoState.todos;") < load_todos.find("todos = _legacyTodosFromMessages();") + + +def test_legacy_todos_fallback_still_uses_raw_session_messages(): + src = (REPO_ROOT / "static" / "panels.js").read_text(encoding="utf-8") + + assert "function _legacyTodosFromMessages()" in src + assert "const sourceMessages = (S.session && Array.isArray(S.session.messages) && S.session.messages.length) ? S.session.messages : S.messages;" in src diff --git a/tests/test_todo_state.py b/tests/test_todo_state.py new file mode 100644 index 00000000000..78c8e66c9d7 --- /dev/null +++ b/tests/test_todo_state.py @@ -0,0 +1,92 @@ +import json + +from api.todo_state import VERSION, attach_todo_state, derive_todo_state, parse_todo_tool_result + + +def _todo_payload(todos): + summary = { + "total": len(todos), + "pending": sum(1 for t in todos if t["status"] == "pending"), + "in_progress": sum(1 for t in todos if t["status"] == "in_progress"), + "completed": sum(1 for t in todos if t["status"] == "completed"), + "cancelled": sum(1 for t in todos if t["status"] == "cancelled"), + } + return json.dumps({"todos": todos, "summary": summary}, ensure_ascii=False) + + +def _todo_msg(todos, timestamp=None): + msg = {"role": "tool", "content": _todo_payload(todos)} + if timestamp is not None: + msg["timestamp"] = timestamp + return msg + + +def test_parse_todo_tool_result_accepts_json_string_and_dict(): + raw = _todo_payload([{"id": "1", "content": "review", "status": "pending"}]) + + from_string = parse_todo_tool_result(raw) + from_dict = parse_todo_tool_result(json.loads(raw)) + + assert from_string is not None + assert from_dict is not None + assert from_string == from_dict + assert from_string["version"] == VERSION + assert from_string["todos"][0]["content"] == "review" + assert from_string["summary"]["total"] == 1 + + +def test_parse_todo_tool_result_rejects_non_todo_shapes(): + for bad in (None, "", "not json", "{}", '{"todos":"not-list"}', [1, 2, 3]): + assert parse_todo_tool_result(bad) is None + + +def test_derive_todo_state_uses_latest_tool_write_even_when_empty(): + messages = [ + _todo_msg([{"id": "old", "content": "old task", "status": "pending"}], timestamp=10), + {"role": "assistant", "content": "done", "timestamp": 11}, + _todo_msg([], timestamp=12), + ] + + state = derive_todo_state(messages) + + assert state is not None + assert state["todos"] == [] + assert state["summary"] == {"total": 0, "pending": 0, "in_progress": 0, "completed": 0, "cancelled": 0} + assert state["ts"] == 12 + + +def test_derive_todo_state_skips_malformed_and_non_string_tool_content(): + messages = [ + _todo_msg([{"id": "good", "content": "keep", "status": "in_progress"}]), + {"role": "tool", "content": ["multimodal parts are not todo output"]}, + {"role": "tool", "content": '{"todos": broken'}, + ] + + state = derive_todo_state(messages) + + assert state is not None + assert state["todos"][0]["id"] == "good" + + +def test_derive_todo_state_recency_falls_back_to_prior_message_timestamp(): + messages = [ + _todo_msg([{"id": "old", "content": "old", "status": "pending"}], timestamp=10), + {"role": "assistant", "content": "checkpoint", "timestamp": 20}, + _todo_msg([{"id": "new", "content": "new", "status": "completed"}]), + ] + + state = derive_todo_state(messages) + + assert state is not None + assert state["todos"][0]["id"] == "new" + assert state["ts"] == 20 + + +def test_attach_todo_state_mutates_payload_and_swallows_missing_state(): + payload: dict = {"session_id": "s1"} + assert attach_todo_state(payload, [_todo_msg([{"id": "1", "content": "x", "status": "pending"}])]) is True + assert payload["todo_state"]["todos"][0]["id"] == "1" + + empty_payload: dict = {"session_id": "s2"} + assert attach_todo_state(empty_payload, [{"role": "assistant", "content": "none"}]) is False + assert "todo_state" not in empty_payload