diff --git a/api/routes.py b/api/routes.py index ed76827cb..8d50a9158 100644 --- a/api/routes.py +++ b/api/routes.py @@ -6563,6 +6563,45 @@ def _session_index_marks_was_webui(sid: str) -> bool: return False +def _state_db_session_source(sid: str) -> str: + """Return the lowercased ``sessions.source`` for ``sid`` from state.db. + + Cheap single-row lookup used to distinguish delegated ``subagent`` children + (which have a recoverable state.db transcript) from genuinely-deleted WebUI + sessions. Returns "" on any error / missing row so callers fall back to + their existing behaviour. + """ + if not sid or not is_safe_session_id(sid): + return "" + try: + from api.models import _active_state_db_path + db_path = _active_state_db_path() + if not db_path or not Path(db_path).exists(): + return "" + import sqlite3 as _sqlite + with closing(_sqlite.connect(str(db_path))) as _conn: + row = _conn.execute( + "SELECT source FROM sessions WHERE id = ?", (sid,) + ).fetchone() + except Exception: + return "" + if not row: + return "" + return str(row[0] or "").strip().lower() + + +def _is_subagent_child_session_id(sid: str) -> bool: + """Return True when ``sid`` is a delegated subagent child in state.db. + + Delegated ``delegate_task`` children are recorded in Hermes state.db with + ``source='subagent'`` and a ``parent_session_id``. They frequently have no + WebUI sidecar (they ran server-side), so opening one from the sidebar must + recover the transcript from state.db rather than 404 as a deleted WebUI + session (#5307). + """ + return _state_db_session_source(sid) == "subagent" + + def _is_claimable_cli_source(cli_meta: dict, state_db_source: str = "") -> tuple[bool, str]: """Decide whether a foreign-origin session is safe to claim writeable in WebUI. Returns ``(claimable, reason_if_not)``. @@ -6728,7 +6767,13 @@ def _claim_or_synthesize_cli_session(sid: str, cli_meta: dict = None): if not is_safe_session_id(sid): return None, "invalid_sid" - if _session_index_marks_was_webui(sid): + if _session_index_marks_was_webui(sid) and not _is_subagent_child_session_id(sid): + # A delegated subagent child (source='subagent' in state.db) can be + # registered in the WebUI index as a webui/fork/blank-source row (it + # shares the parent's lineage) yet have no WebUI sidecar of its own. + # Those must recover their transcript from state.db below rather than + # 404 as a genuinely-deleted WebUI session (#5307). Every other + # index-marked-WebUI id keeps the #2782 self-heal 404 contract. return None, "was_webui" if cli_meta is None: cli_meta = _lookup_cli_session_metadata(sid) or {} diff --git a/static/sessions.js b/static/sessions.js index 80c637193..9e0a56957 100644 --- a/static/sessions.js +++ b/static/sessions.js @@ -1807,6 +1807,39 @@ function _isExternalSession(session) { return !!(session.is_cli_session || _isMessagingSession(session)); } +/** + * Returns true when a session is a delegated subagent child (delegate_task + * spawns these with source='subagent' in Hermes state.db). These children + * usually have no WebUI sidecar of their own — their transcript lives only in + * state.db — so opening one must run the same server-side import/merge path + * that CLI/messaging sessions use, even though they are not _isExternalSession + * (their normalized session_source is 'other', not cli/messaging). Without + * this the child pane loads empty despite state.db carrying messages (#5307). + */ +function _isSubagentChildSession(session) { + if (!session) return false; + const src = ( + session.session_source + || session.raw_source + || session.source_tag + || session.source + || session.source_label + || '' + ).toString().toLowerCase(); + return src === 'subagent'; +} + +/** + * Sessions that need a server-side import/materialize before the WebUI can + * read a full transcript: external channels (CLI/messaging) plus delegated + * subagent children (#5307). Kept separate from _isExternalSession so the + * active-session refresh/force-reload gating that keys on _isExternalSession + * is not widened by this classification. + */ +function _sessionNeedsServerImportForLoad(session) { + return _isExternalSession(session) || _isSubagentChildSession(session); +} + function _externalImportPayload(session) { const payload = {session_id: session.session_id}; if (_showAllProfiles && session && typeof session.profile === 'string' && session.profile) { @@ -6911,7 +6944,7 @@ function renderSessionListFromCache(){ row.title=t('session_lineage_segment_open'); row.onclick=async(e)=>{ e.stopPropagation(); - if(_isExternalSession(seg)){ + if(_sessionNeedsServerImportForLoad(seg)){ try{await api('/api/session/import_cli',{method:'POST',body:JSON.stringify(_externalImportPayload(seg))});} catch(_e){ /* read-only fallback */ } } @@ -6928,7 +6961,7 @@ function renderSessionListFromCache(){ ['pointerdown','pointerup','click','touchstart','touchmove','touchend','touchcancel'].forEach(ev=>childList.addEventListener(ev,e=>e.stopPropagation())); const sortedChildren=[...s._child_sessions].sort((a,b)=>_sessionTimestampMs(b)-_sessionTimestampMs(a)); const openChildSession=async(childSession)=>{ - if(_isExternalSession(childSession)){ + if(_sessionNeedsServerImportForLoad(childSession)){ try{await api('/api/session/import_cli',{method:'POST',body:JSON.stringify(_externalImportPayload(childSession))});} catch(_e){ /* read-only fallback */ } } @@ -7555,9 +7588,12 @@ function renderSessionListFromCache(){ _tapTimer=null; _lastTapTime=0; if(_renamingSid) return; - // For external sessions (CLI, Discord, Telegram, Slack), import into - // WebUI store first so /api/chat/start finds a persisted session. - if(_isExternalSession(s)){ + // For external sessions (CLI, Discord, Telegram, Slack) and delegated + // subagent children, import into WebUI store first so /api/chat/start + // (and the transcript load) finds a persisted/merged session. Subagent + // children (source='subagent') live only in state.db and would + // otherwise open to an empty pane (#5307). + if(_sessionNeedsServerImportForLoad(s)){ try{ await api('/api/session/import_cli',{method:'POST',body:JSON.stringify(_externalImportPayload(s))}); }catch(e){ /* import failed -- fall through to read-only view */ } diff --git a/tests/test_5307_subagent_child_transcript.py b/tests/test_5307_subagent_child_transcript.py new file mode 100644 index 000000000..097148fec --- /dev/null +++ b/tests/test_5307_subagent_child_transcript.py @@ -0,0 +1,243 @@ +"""Regression tests for issue #5307 — delegated subagent child transcript load. + +A delegated ``delegate_task`` child is recorded in Hermes ``state.db`` with +``source='subagent'`` and usually has **no WebUI JSON sidecar** (it ran +server-side; its transcript lives only in state.db). But its ``session_id`` is +registered in the WebUI ``_index.json`` sharing the parent's lineage, often as +a ``webui``/``fork``/blank-source row. + +Before the fix, ``GET /api/session`` -> ``get_session()`` raised ``KeyError`` +-> ``_claim_or_synthesize_cli_session()`` saw ``_session_index_marks_was_webui`` +True and returned ``"was_webui"`` -> **404**, so the child pane opened empty even +though state.db held messages. + +The fix excludes delegated subagent children from the ``was_webui`` 404 gate +(``_is_subagent_child_session_id``) so they recover their state.db transcript, +while genuinely-deleted WebUI sessions keep the #2782 self-heal 404 contract. +""" +from __future__ import annotations + +import json +import re +import sqlite3 +from pathlib import Path + +import pytest + +ROOT = Path(__file__).resolve().parents[1] +ROUTES_PY = ROOT / "api" / "routes.py" +SESSIONS_JS = ROOT / "static" / "sessions.js" + + +# --------------------------------------------------------------------------- +# Local seed helpers (mirror test_chat_start_claim_cli_session, kept +# self-contained so this file has no cross-test import dependency) +# --------------------------------------------------------------------------- + + +def _make_state_db(path: Path, sid: str, *, message_count: int = 2, + title: str = "tui session", model: str = "MiniMax-M3", + source: str = "tui", cwd: str = "/root") -> None: + """Create a minimal state.db with one session and a few messages. + + Schema mirrors hermes_state.SessionDB closely enough for + get_state_db_session_messages to return rows. + """ + conn = sqlite3.connect(str(path)) + conn.executescript( + """ + CREATE TABLE IF NOT EXISTS schema_version (version INTEGER); + CREATE TABLE IF NOT EXISTS sessions ( + id TEXT PRIMARY KEY, + source TEXT, + user_id TEXT, + model TEXT, + model_config TEXT, + system_prompt TEXT, + parent_session_id TEXT, + started_at REAL, + ended_at REAL, + end_reason TEXT, + message_count INTEGER DEFAULT 0, + tool_call_count INTEGER DEFAULT 0, + input_tokens INTEGER DEFAULT 0, + output_tokens INTEGER DEFAULT 0, + cache_read_tokens INTEGER DEFAULT 0, + cache_write_tokens INTEGER DEFAULT 0, + reasoning_tokens INTEGER DEFAULT 0, + billing_provider TEXT, + billing_base_url TEXT, + billing_mode TEXT, + estimated_cost_usd REAL, + actual_cost_usd REAL, + cost_status TEXT, + cost_source TEXT, + pricing_version TEXT, + title TEXT, + api_call_count INTEGER DEFAULT 0, + handoff_state TEXT, + handoff_platform TEXT, + handoff_error TEXT, + cwd TEXT, + rewind_count INTEGER DEFAULT 0, + archived INTEGER DEFAULT 0 + ); + CREATE TABLE IF NOT EXISTS messages ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + session_id TEXT, + role TEXT, + content TEXT, + timestamp REAL, + tool_call_id TEXT, + tool_calls TEXT, + tool_call_count INTEGER DEFAULT 0 + ); + """ + ) + conn.execute( + "INSERT OR REPLACE INTO sessions (id, source, model, message_count, started_at, title, cwd) " + "VALUES (?, ?, ?, ?, 1781024055.0, ?, ?)", + (sid, source, model, message_count, title, cwd), + ) + for i in range(message_count): + conn.execute( + "INSERT INTO messages (session_id, role, content, timestamp) VALUES (?, ?, ?, ?)", + (sid, "user" if i % 2 == 0 else "assistant", + f"msg {i}", 1781024055.0 + i), + ) + conn.commit() + conn.close() + + +def _write_index(path: Path, entries: list[dict]) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps(entries), encoding="utf-8") + + +@pytest.fixture +def routes_module(): + return pytest.importorskip("api.routes") + + +@pytest.fixture +def isolated_state_db(tmp_path, monkeypatch): + db = tmp_path / "state.db" + state_dir = tmp_path / "webui-state" + sessions_dir = state_dir / "sessions" + sessions_dir.mkdir(parents=True) + index_path = sessions_dir / "_index.json" + index_path.write_text("[]", encoding="utf-8") + import api.routes as _routes + import api.models as _models + monkeypatch.setattr(_models, "_active_state_db_path", lambda: db) + monkeypatch.setattr(_routes, "SESSION_INDEX_FILE", index_path) + monkeypatch.setattr(_models, "SESSION_INDEX_FILE", index_path) + monkeypatch.setattr(_models, "SESSION_DIR", sessions_dir) + return {"db": db, "state_dir": state_dir, "sessions_dir": sessions_dir, + "index_path": index_path} + + +# --------------------------------------------------------------------------- +# Static checks: the fix is present in source +# --------------------------------------------------------------------------- + + +def test_subagent_child_helpers_defined_in_routes(): + src = ROUTES_PY.read_text(encoding="utf-8") + assert "def _is_subagent_child_session_id(" in src, ( + "routes.py must define _is_subagent_child_session_id to distinguish " + "delegated subagent children from deleted WebUI sessions (#5307)" + ) + assert "def _state_db_session_source(" in src, ( + "routes.py must define _state_db_session_source (cheap state.db source lookup)" + ) + + +def test_was_webui_gate_excludes_subagent_children(): + """The was_webui 404 gate must be guarded by + ``not _is_subagent_child_session_id(sid)`` so subagent children fall + through to state.db transcript recovery instead of 404ing.""" + src = ROUTES_PY.read_text(encoding="utf-8") + start = src.index("def _claim_or_synthesize_cli_session(") + m = re.search(r"\n(?:def |class )", src[start + 1:]) + block = src[start:(start + 1 + m.start()) if m else len(src)] + assert "_session_index_marks_was_webui(sid)" in block + assert re.search( + r"_session_index_marks_was_webui\(sid\)\s+and\s+not\s+_is_subagent_child_session_id\(sid\)", + block, + ), "was_webui and not subagent-child must gate the same 404 return (#5307)" + + +def test_sessions_js_uses_wider_import_predicate(): + """All transcript-load import_cli sites must gate on + _sessionNeedsServerImportForLoad so subagent children trigger the + server-side merge; the poll-skip / active-refresh gating stays keyed on the + narrower _isExternalSession.""" + js = SESSIONS_JS.read_text(encoding="utf-8") + assert "function _isSubagentChildSession(" in js + assert "function _sessionNeedsServerImportForLoad(" in js + assert js.count("_sessionNeedsServerImportForLoad(") >= 4, ( + "expected the helper definition + 3 load/tap call sites" + ) + + +# --------------------------------------------------------------------------- +# Functional: helper recovers subagent children, preserves #2782 404 +# --------------------------------------------------------------------------- + + +def test_subagent_child_indexed_as_webui_is_not_404_was_webui( + routes_module, isolated_state_db +): + """A subagent child (source='subagent' in state.db) registered in the WebUI + index as a webui-lineage row must NOT return 'was_webui' — it must recover + its state.db transcript (#5307).""" + _make_state_db( + isolated_state_db["db"], "subagent-child-1", + source="subagent", title="delegate child", message_count=2, + ) + _write_index( + isolated_state_db["index_path"], + [ + {"session_id": "subagent-child-1", "source_tag": "webui", + "raw_source": "webui", "session_source": "webui", + "parent_session_id": "parent-abc"}, + ], + ) + + sess, reason = routes_module._claim_or_synthesize_cli_session("subagent-child-1") + assert reason != "was_webui", ( + f"subagent child must not 404 as a deleted WebUI session; got reason={reason!r}" + ) + + +def test_deleted_webui_session_still_returns_was_webui( + routes_module, isolated_state_db +): + """#2782 self-heal contract preserved: a genuinely-deleted WebUI session + (webui-origin index row, NO state.db row) still returns 'was_webui'.""" + _make_state_db(isolated_state_db["db"], "some-other-sid", source="tui") + _write_index( + isolated_state_db["index_path"], + [ + {"session_id": "webui-orphan", "source_tag": "webui", + "raw_source": "webui", "session_source": "webui"}, + ], + ) + + sess, reason = routes_module._claim_or_synthesize_cli_session("webui-orphan") + assert sess is None + assert reason == "was_webui", ( + "a deleted WebUI session with no state.db row must keep the #2782 404" + ) + + +def test_state_db_source_helper_reads_subagent(routes_module, isolated_state_db): + """_state_db_session_source returns the lowercased source; _is_subagent_child + is True only for source='subagent', and False for a missing row.""" + _make_state_db( + isolated_state_db["db"], "sa-1", source="subagent", message_count=1, + ) + assert routes_module._state_db_session_source("sa-1") == "subagent" + assert routes_module._is_subagent_child_session_id("sa-1") is True + assert routes_module._is_subagent_child_session_id("does-not-exist") is False