Merge #5307 (subagent child transcript view-only recovery) into stage

This commit is contained in:
nesquena-hermes committed 2026-07-01 09:13:08 +00:00
commit 794837d635
5 files changed
+633 -24

No files matched your search

+5 -1
View File
@@ -176,7 +176,11 @@ def is_cli_session_row(row: dict) -> bool:
source_label = _safe_lower(row.get("source_label"))
if "webui" in {source, source_tag, raw_source, source_name, source_label}:
return False
non_cli_sources = MESSAGING_SOURCES | {"cron", "webhook", "tool", "api", "api_server"}
# 'subagent' is a delegated delegate_task child: view-only, owned by the
# runner, never a writable WebUI/CLI session (#5307). Classify it non-CLI so
# sidebar rows and every is_cli_session_row() consumer keep it out of the
# CLI/writable treatment.
non_cli_sources = MESSAGING_SOURCES | {"cron", "webhook", "tool", "api", "api_server", "subagent"}
if {source, source_tag, raw_source, source_name, source_label} & non_cli_sources:
return False
if source == "messaging":
+241 -11
View File
@@ -2330,6 +2330,32 @@ def _build_session_list_cache_payload(
diag_stage("filter_archived_sessions")
diag_stage("visible_lineage_metadata")
_enrich_sidebar_lineage_metadata(scoped)
# Delegated subagent children (#5307) are view-only, owned by the delegate
# runner. Coerce their sidebar rows to read_only=True + is_cli_session=False
# so the UI never offers delete / edit / truncate / pin affordances on them
# (defense-in-depth is also enforced server-side on the mutation routes).
def _coerce_subagent_rows(_rows):
for _r in _rows:
if not isinstance(_r, dict):
continue
_src = (
str(_r.get("source_tag") or _r.get("raw_source")
or _r.get("session_source") or _r.get("source") or "").strip().lower()
)
_is_sa = _src == "subagent"
# A stale index row can say webui/fork while state.db records the
# row as source='subagent' (the child shares the parent's lineage).
# For rows not already read-only, confirm via the state.db source so
# a delegated child can't surface as a writable/CLI sidebar row.
if not _is_sa and not _r.get("read_only"):
_sid = str(_r.get("session_id") or "").strip()
if _sid and _is_subagent_child_session_id(_sid):
_is_sa = True
if _is_sa:
_r["read_only"] = True
_r["is_cli_session"] = False
_coerce_subagent_rows(scoped)
_coerce_subagent_rows(sidebar_reference_sessions)
return {
"sessions": [
dict(s) if isinstance(s, dict) else {}
@@ -4712,6 +4738,16 @@ def _get_or_materialize_session(sid: str, *, refresh_cli_messages: bool = False)
# below (and the heuristic record-check would mis-trip on mock sessions).
if getattr(s, "read_only", False):
raise PermissionError("read-only imported session")
# A previously-persisted subagent sidecar (#5307) is view-only and
# owned by the delegate runner — even if it was stored with
# read_only=False (e.g. materialized before this fix), it must not be
# mutated / used as a writable chat session. This mirrors the
# missing-sidecar subagent guard below on the happy path.
if (
(getattr(s, "source_tag", "") or getattr(s, "raw_source", "") or "").strip().lower() == "subagent"
or _is_subagent_child_session_id(sid)
):
raise PermissionError("read-only subagent child session")
if refresh_cli_messages and getattr(s, "is_cli_session", False):
latest_messages = get_cli_session_messages(
sid,
@@ -4733,6 +4769,20 @@ def _get_or_materialize_session(sid: str, *, refresh_cli_messages: bool = False)
# Fallback: try to materialize from CLI/agent session metadata
cli_meta = _lookup_cli_session_metadata(sid)
# Delegated subagent children (#5307) are view-only: their transcript lives
# in state.db and ownership belongs to the delegate runner, not WebUI. They
# must never be materialized as a writable sidecar here — this is the shared
# chokepoint reached by POST /api/chat/start (_get_or_materialize_session),
# so gating it closes the write path that bypasses the GET/import_cli guards.
# Checked via state.db source (independent of cli_meta, which is often empty
# for a server-side subagent child).
_mat_source_tag = (
(cli_meta or {}).get("source_tag") or (cli_meta or {}).get("raw_source") or ""
).strip().lower()
if _mat_source_tag == "subagent" or _is_subagent_child_session_id(sid):
raise PermissionError("read-only subagent child session")
if not cli_meta:
raise KeyError(sid)
@@ -6563,6 +6613,69 @@ 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 _session_is_subagent_view_only(sid: str) -> bool:
"""Return True when ``sid`` is a delegated subagent child by ANY signal —
state.db source OR a persisted WebUI sidecar tagged subagent.
Delegated children are view-only and owned by the delegate runner. Direct
transcript/metadata mutation routes (delete / truncate / clear / pin /
rename / move) must refuse them so a stray WebUI action can't delete or
fork the child's state.db transcript (#5307). This is the shared
defense-in-depth guard for routes that bypass
``_get_or_materialize_session()``.
"""
if _is_subagent_child_session_id(sid):
return True
try:
s = get_session(sid)
except Exception:
return False
src = (
str(getattr(s, "source_tag", "") or getattr(s, "raw_source", "")
or getattr(s, "session_source", "") or "").strip().lower()
)
return src == "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)``.
@@ -6605,7 +6718,7 @@ def _is_claimable_cli_source(cli_meta: dict, state_db_source: str = "") -> tuple
# in a future log / user-visible diagnostic.
cli_meta_source_tag = (cm.get("source_tag") or cm.get("raw_source") or "").strip().lower()
if cli_meta_source_tag in {"claude_code", "cron", "external_agent",
"gateway", "messaging", "unknown"}:
"gateway", "messaging", "subagent", "unknown"}:
# gateway/unknown are the platformless gateway fallbacks
# (gateway/run.py, gateway/slash_commands.py) — they own the
# conversation in the gateway, not in WebUI.
@@ -6624,7 +6737,7 @@ def _is_claimable_cli_source(cli_meta: dict, state_db_source: str = "") -> tuple
if not cli_meta_source_tag and state_db_source:
state_db_source_tag = state_db_source.strip().lower()
if state_db_source_tag in {"claude_code", "cron", "messaging",
"external_agent", "gateway", "unknown"}:
"external_agent", "gateway", "subagent", "unknown"}:
return False, f"state_db_source={state_db_source_tag}"
return True, ""
@@ -6701,7 +6814,7 @@ def _claim_or_synthesize_cli_session(sid: str, cli_meta: dict = None):
workspace = "/"
return workspace
def build_session(sid, cli_meta, msgs, read_only_flag):
def build_session(sid, cli_meta, msgs, read_only_flag, is_cli_flag=True):
return Session(
session_id=sid,
title=(cli_meta or {}).get("title") or "CLI Session",
@@ -6712,7 +6825,13 @@ def _claim_or_synthesize_cli_session(sid: str, cli_meta: dict = None):
created_at=(cli_meta or {}).get("created_at") or 0,
updated_at=(cli_meta or {}).get("updated_at") or 0,
profile=(cli_meta or {}).get("profile"),
is_cli_session=True,
# ``is_cli_flag`` is True for genuine CLI/TUI/Desktop sessions so the
# sidebar renders the source badge and the client's external-session
# gating applies. It is False for delegated subagent children (#5307):
# they are recovered read-only and must NOT be CLI-classified, or they
# would pass the frontend ``_isExternalSession`` poll-skip /
# active-refresh gates that #3603 keeps narrow.
is_cli_session=is_cli_flag,
source_tag=(cli_meta or {}).get("source_tag"),
raw_source=(cli_meta or {}).get("raw_source"),
session_source=(cli_meta or {}).get("session_source"),
@@ -6728,7 +6847,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 {}
@@ -6808,7 +6933,19 @@ def _claim_or_synthesize_cli_session(sid: str, cli_meta: dict = None):
# readonly=True so the GET stub keeps rendering the original
# read-only badge, and return 'not_claimable' so the POST path
# 403s instead of bare-404ing.
return build_session(sid, cli_meta, msgs, read_only_flag=True), "not_claimable"
#
# Delegated subagent children (#5307) additionally must NOT be
# CLI-classified: they are recovered read-only for viewing, but
# is_cli_session=True would let them pass the frontend
# _isExternalSession poll-skip / active-refresh gates that #3603
# keeps narrow. Every other non-claimable foreign source keeps the
# CLI classification so its source badge renders.
_sa_child = _is_subagent_child_session_id(sid)
return (
build_session(sid, cli_meta, msgs, read_only_flag=True,
is_cli_flag=not _sa_child),
"not_claimable",
)
return build_session(sid, cli_meta, msgs, read_only_flag=False), "materialized"
@@ -11203,6 +11340,15 @@ def handle_get(handler, parsed) -> bool:
raw["model"] = effective_model
if effective_provider:
raw["model_provider"] = effective_provider
# A subagent child (#5307) is view-only regardless of what a stale
# sidecar stored: coerce the serialized flags so the browser never
# treats an existing subagent sidecar as writable / CLI-classified.
if (
(str(raw.get("source_tag") or raw.get("raw_source") or raw.get("session_source") or "").strip().lower() == "subagent")
or _is_subagent_child_session_id(sid)
):
raw["is_cli_session"] = False
raw["read_only"] = True
redact = redact_session_data(raw)
_t5 = _time.monotonic()
resp = j(handler, {"session": redact})
@@ -11257,7 +11403,13 @@ def handle_get(handler, parsed) -> bool:
"archived": bool(getattr(synth, "archived", False)),
"project_id": getattr(synth, "project_id", None),
"profile": synth.profile,
"is_cli_session": True,
# Read is_cli_session from the synthesized Session, not a
# hardcoded True: delegated subagent children (#5307) are
# recovered read-only with is_cli_session=False so they don't
# pass the frontend _isExternalSession poll-skip / active-refresh
# gates (#3603). Every other synthesized foreign session keeps
# is_cli_session=True so its source badge renders.
"is_cli_session": bool(getattr(synth, "is_cli_session", False)),
"source_tag": synth.source_tag,
"raw_source": synth.raw_source,
"session_source": synth.session_source,
@@ -12329,6 +12481,8 @@ def handle_post(handler, parsed) -> bool:
sid = body.get("session_id")
if not sid:
return bad(handler, "session_id is required")
if _session_is_subagent_view_only(sid):
return bad(handler, "Subagent sessions are view-only and cannot be duplicated from WebUI", 400)
session = Session.load(sid)
if not session:
@@ -12592,6 +12746,8 @@ def handle_post(handler, parsed) -> bool:
if "name" not in body:
return bad(handler, "Missing required field: name")
sid = body["session_id"]
if _session_is_subagent_view_only(sid):
return bad(handler, "Subagent sessions are view-only and cannot be modified from WebUI", 400)
name = body["name"].strip()
try:
s = get_session(sid)
@@ -12641,6 +12797,8 @@ def handle_post(handler, parsed) -> bool:
except ValueError as e:
return bad(handler, str(e))
sid = body["session_id"]
if _session_is_subagent_view_only(sid):
return bad(handler, "Subagent sessions are view-only and cannot be modified from WebUI", 400)
toolsets = body.get("toolsets")
try:
toolsets = _validate_session_toolsets_shape(toolsets)
@@ -12676,6 +12834,8 @@ def handle_post(handler, parsed) -> bool:
except ValueError as e:
return bad(handler, str(e))
sid = body["session_id"]
if _session_is_subagent_view_only(sid):
return bad(handler, "Subagent sessions are view-only and cannot store a draft from WebUI", 400)
text = body.get("text")
files = body.get("files")
# Stage-326 hardening (per Opus advisor): size + type validation on
@@ -12804,6 +12964,11 @@ def handle_post(handler, parsed) -> bool:
cli_meta_for_delete = _lookup_cli_session_metadata(sid)
if cli_meta_for_delete.get("read_only"):
return bad(handler, "Read-only imported sessions cannot be deleted from WebUI", 400)
# A delegated subagent child (#5307) is view-only and owned by the
# delegate runner. Deleting it here would call delete_cli_session() and
# erase the child's state.db transcript — refuse it.
if _session_is_subagent_view_only(sid):
return bad(handler, "Subagent sessions are view-only and cannot be deleted from WebUI", 400)
is_messaging_session = _is_messaging_session_id(sid)
worktree_retained = _worktree_retained_payload_for_session_id(sid)
try:
@@ -12882,6 +13047,8 @@ def handle_post(handler, parsed) -> bool:
require(body, "session_id")
except ValueError as e:
return bad(handler, str(e))
if _session_is_subagent_view_only(body["session_id"]):
return bad(handler, "Subagent sessions are view-only and cannot be modified from WebUI", 400)
try:
s = get_session(body["session_id"])
except KeyError:
@@ -12909,6 +13076,8 @@ def handle_post(handler, parsed) -> bool:
require(body, "session_id")
except ValueError as e:
return bad(handler, str(e))
if _session_is_subagent_view_only(body["session_id"]):
return bad(handler, "Subagent sessions are view-only and cannot be modified from WebUI", 400)
if body.get("keep_count") is None:
return bad(handler, "Missing required field(s): keep_count")
try:
@@ -12959,6 +13128,8 @@ def handle_post(handler, parsed) -> bool:
# (Opus pre-release follow-up.)
if not isinstance(body["session_id"], str):
return bad(handler, "session_id must be a string")
if _session_is_subagent_view_only(body["session_id"]):
return bad(handler, "Subagent sessions are view-only and cannot be branched from WebUI", 400)
try:
source = get_session(body["session_id"])
except KeyError:
@@ -13077,6 +13248,8 @@ def handle_post(handler, parsed) -> bool:
require(body, "session_id")
except ValueError as e:
return bad(handler, str(e))
if _session_is_subagent_view_only(body["session_id"]):
return bad(handler, "Subagent sessions are view-only and cannot be modified from WebUI", 400)
try:
from api.session_ops import retry_last
result = retry_last(body["session_id"])
@@ -13091,6 +13264,8 @@ def handle_post(handler, parsed) -> bool:
require(body, "session_id")
except ValueError as e:
return bad(handler, str(e))
if _session_is_subagent_view_only(body["session_id"]):
return bad(handler, "Subagent sessions are view-only and cannot be modified from WebUI", 400)
try:
from api.session_ops import undo_last
result = undo_last(body["session_id"])
@@ -13684,6 +13859,8 @@ def handle_post(handler, parsed) -> bool:
require(body, "session_id")
except ValueError as e:
return bad(handler, str(e))
if _session_is_subagent_view_only(body["session_id"]):
return bad(handler, "Subagent sessions are view-only and cannot be modified from WebUI", 400)
try:
s = get_session(body["session_id"])
s = _ensure_full_session_before_mutation(body["session_id"], s)
@@ -13755,6 +13932,8 @@ def handle_post(handler, parsed) -> bool:
except ValueError as e:
return bad(handler, str(e))
sid = body["session_id"]
if _session_is_subagent_view_only(sid):
return bad(handler, "Subagent sessions are view-only and cannot be archived from WebUI", 400)
try:
s = get_session(sid)
# #1558: save() refuses metadata-only session stubs because their
@@ -13773,6 +13952,13 @@ def handle_post(handler, parsed) -> bool:
return bad(handler, "Session not found", 404)
if cli_meta.get("read_only"):
return bad(handler, "Read-only imported sessions cannot be archived from WebUI", 400)
# Delegated subagent children (#5307) are view-only and owned by the
# delegate runner — never materialize one into a writable WebUI
# sidecar via the archive fallback (the 3rd of the shared
# import_cli_session write paths).
_arch_source_tag = (cli_meta.get("source_tag") or cli_meta.get("raw_source") or "").strip().lower()
if _arch_source_tag == "subagent" or _is_subagent_child_session_id(sid):
return bad(handler, "Subagent sessions cannot be archived from WebUI", 400)
if _is_messaging_session_record(cli_meta):
s = Session(
session_id=sid,
@@ -17861,6 +18047,8 @@ def _handle_btw(handler, body):
require(body, "question")
except ValueError as e:
return bad(handler, str(e))
if _session_is_subagent_view_only(str(body.get("session_id") or "")):
return bad(handler, "Subagent sessions are view-only and cannot be used for /btw from WebUI", 400)
try:
s = get_session(body["session_id"])
except KeyError:
@@ -18683,6 +18871,8 @@ def _handle_goal_command(handler, body):
require(body, "session_id")
except ValueError as e:
return bad(handler, str(e))
if _session_is_subagent_view_only(str(body.get("session_id") or "")):
return bad(handler, "Subagent sessions are view-only and cannot run /goal from WebUI", 400)
try:
s = get_session(body["session_id"])
except KeyError:
@@ -19040,6 +19230,8 @@ def _normalize_chat_attachments(raw_attachments):
def _handle_chat_sync(handler, body):
"""Fallback synchronous chat endpoint (POST /api/chat). Not used by frontend."""
if _session_is_subagent_view_only(str(body.get("session_id") or "")):
return bad(handler, "Subagent sessions are view-only and cannot be written from WebUI", 400)
s = get_session(body["session_id"])
msg = str(body.get("message", "")).strip()
if not msg:
@@ -20968,6 +21160,8 @@ def _handle_session_compress(handler, body):
sid = str(body.get("session_id") or "").strip()
if not sid:
return bad(handler, "session_id is required")
if _session_is_subagent_view_only(sid):
return bad(handler, "Subagent sessions are view-only and cannot be compressed from WebUI", 400)
# Cap focus_topic to 500 chars — matches the defensive input-size pattern
# used elsewhere (session title :80, first-exchange snippets :500) and
@@ -21474,6 +21668,8 @@ def _handle_handoff_summary(handler, body):
from api.models import get_cli_session_messages, count_conversation_rounds, CONVERSATION_ROUND_THRESHOLD
if _session_is_subagent_view_only(sid):
return bad(handler, "Subagent sessions are view-only and cannot be summarized from WebUI", 400)
rounds = count_conversation_rounds(sid, since=since)
if rounds < CONVERSATION_ROUND_THRESHOLD:
return bad(handler, "Not enough conversation rounds to generate a summary.", 400)
@@ -22184,18 +22380,35 @@ def _handle_session_import_cli(handler, body):
existing.messages = fresh_msgs
changed = True
if cli_meta:
# A subagent child must never be flipped to CLI-classified /
# writable on an existing-session refresh either (#5307).
_existing_is_sa = (
(existing.source_tag or existing.raw_source or "").strip().lower() == "subagent"
or (cli_meta.get("source_tag") or cli_meta.get("raw_source") or "").strip().lower() == "subagent"
or _is_subagent_child_session_id(sid)
)
updates = {
"is_cli_session": True,
"is_cli_session": (False if _existing_is_sa else True),
"source_tag": existing.source_tag or cli_meta.get("source_tag"),
"raw_source": existing.raw_source or cli_meta.get("raw_source") or cli_meta.get("source_tag"),
"session_source": existing.session_source or cli_meta.get("session_source"),
"source_label": existing.source_label or cli_meta.get("source_label"),
"parent_session_id": existing.parent_session_id or cli_meta.get("parent_session_id"),
}
# A subagent child is view-only: also coerce read_only=True on the
# persisted sidecar so a stale writable (pre-fix) sidecar can't be
# used to start a WebUI turn (#5307).
if _existing_is_sa:
updates["read_only"] = True
for attr, value in updates.items():
if getattr(existing, attr, None) != value:
setattr(existing, attr, value)
changed = True
else:
_existing_is_sa = (
(existing.source_tag or existing.raw_source or "").strip().lower() == "subagent"
or _is_subagent_child_session_id(sid)
)
if changed:
existing.save(touch_updated_at=False)
publish_session_list_changed(
@@ -22208,7 +22421,7 @@ def _handle_session_import_cli(handler, body):
"session": existing.compact()
| {
"messages": existing.messages,
"is_cli_session": True,
"is_cli_session": (False if _existing_is_sa else True),
# Greptile #4911 follow-up: read read_only from
# the persisted Session, NOT from cli_meta. This
# refresh path is for an already-WebUI-owned
@@ -22250,6 +22463,20 @@ def _handle_session_import_cli(handler, body):
cli_platform = cli_meta.get("platform") if cli_meta else None
cli_parent_session_id = cli_meta.get("parent_session_id") if cli_meta else None
cli_read_only = bool((cli_meta or {}).get("read_only"))
# Delegated subagent children (#5307) are recovered VIEW-ONLY: they must
# never be materialized as a writable WebUI sidecar via this endpoint, or a
# subsequent chat-start/composer write would take ownership of a session
# that belongs to the delegate runner. Treat them like an explicitly
# read-only source (return the read-only stub payload, do not import), and
# keep them out of the _isExternalSession frontend gates (is_cli_session=False).
_sa_child = _is_subagent_child_session_id(sid)
# Also treat a resolved-metadata subagent source as view-only: with
# all_profiles=true, cli_meta is resolved from the requested (possibly
# non-active) profile, so the active-profile state.db check (_sa_child)
# can miss it (#5307 cross-profile edge).
_cli_sa = (cli_source_tag or cli_raw_source or "").strip().lower() == "subagent"
_sa_child = _sa_child or _cli_sa
_read_only_view = cli_read_only or _sa_child
# Use the CLI session title if available (e.g., cron job name), otherwise derive from messages
title = cli_title or title_from(msgs, "CLI Session")
@@ -22259,7 +22486,7 @@ def _handle_session_import_cli(handler, body):
if is_cron_session(sid, cli_source_tag):
cron_project_id = ensure_cron_project()
if cli_read_only:
if _read_only_view:
session_payload = {
"session_id": sid,
"title": title,
@@ -22273,7 +22500,10 @@ def _handle_session_import_cli(handler, body):
"archived": False,
"project_id": None,
"profile": profile,
"is_cli_session": True,
# Subagent children (#5307) are recovered view-only and must NOT be
# CLI-classified (keeps them out of the frontend _isExternalSession
# gates); other explicitly-read-only sources keep is_cli_session=True.
"is_cli_session": (False if _sa_child else True),
"source_tag": cli_source_tag,
"raw_source": cli_raw_source or cli_source_tag,
"session_source": cli_session_source,
@@ -0,0 +1,375 @@
"""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_open_handlers_keep_isexternalsession_contract():
"""The fix is server-side + view-only, so sessions.js must be UNCHANGED —
the #3603 contract that the open/tap/child handlers gate on
_isExternalSession is preserved (subagent children are recovered read-only
by the server, not by widening the client import trigger)."""
js = SESSIONS_JS.read_text(encoding="utf-8")
# We did NOT add a widened import predicate — recovery is server-side.
assert "_sessionNeedsServerImportForLoad" not in js, (
"the #5307 fix is server-side (read-only recovery); the client import "
"predicate must NOT be widened (preserves #3603's _isExternalSession contract)"
)
# ---------------------------------------------------------------------------
# Functional: helper recovers subagent children, preserves #2782 404
# ---------------------------------------------------------------------------
def test_subagent_child_indexed_as_webui_recovers_readonly_not_404(
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' (404). It must
recover its state.db transcript VIEW-ONLY: reason='not_claimable', a Session
is returned, read_only=True, and it is NOT CLI-classified/writable (#5307 +
Codex hardening — a delegated child must not become a writable imported
session)."""
_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 == "not_claimable", (
f"subagent child must recover view-only (not_claimable), not 404 or "
f"become writable; got reason={reason!r}"
)
assert sess is not None, "the read-only transcript session must be returned"
# View-only: must be read_only and must NOT be a writable CLI-classified session.
assert getattr(sess, "read_only", False) is True, (
"recovered subagent child must be read_only (not writable)"
)
assert getattr(sess, "is_cli_session", False) is not True, (
"recovered subagent child must NOT be CLI-classified (would widen "
"poll-skip/active-refresh gating)"
)
# And it carries the state.db transcript.
msgs = getattr(sess, "messages", None)
assert msgs and len(msgs) >= 2, "the state.db transcript must be recovered"
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
def test_import_cli_endpoint_does_not_materialize_subagent_child():
"""Codex hardening: POST /api/session/import_cli must NOT create a writable
sidecar for a subagent child — the handler gates source='subagent' into the
read-only view payload (is_cli_session=False, imported=False) before ever
calling import_cli_session(). Pinned as a static-source contract check."""
src = ROUTES_PY.read_text(encoding="utf-8")
start = src.index("def _handle_session_import_cli(")
m = re.search(r"\n(?:def |class )", src[start + 1:])
block = src[start:(start + 1 + m.start()) if m else len(src)]
assert "_is_subagent_child_session_id(sid)" in block, (
"import_cli handler must detect subagent children"
)
assert "_read_only_view" in block, (
"import_cli must route subagent children through the read-only view "
"payload (no writable import_cli_session materialization)"
)
# the read-only view branch must return before import_cli_session()
ro_idx = block.index("_read_only_view")
import_idx = block.index("import_cli_session(")
assert ro_idx < import_idx, (
"the read-only-view gate must precede import_cli_session() so subagent "
"children never materialize a writable sidecar"
)
def test_materialize_helper_refuses_subagent_child(routes_module, isolated_state_db):
"""The shared _get_or_materialize_session chokepoint (reached by POST
/api/chat/start) must raise PermissionError for a subagent child, so no
entry point can turn a delegated child into a writable sidecar (#5307
Codex round 3 — the chat-start write path)."""
_make_state_db(
isolated_state_db["db"], "sa-mat-1", source="subagent", message_count=2,
)
with pytest.raises(PermissionError):
routes_module._get_or_materialize_session("sa-mat-1")
def test_materialize_helper_still_allows_tui(routes_module, isolated_state_db):
"""Guard scope check: a genuine TUI/CLI session is still materializable
(the subagent gate must not regress normal CLI/TUI/Desktop claiming)."""
_make_state_db(
isolated_state_db["db"], "tui-mat-1", source="tui", message_count=2,
)
# Should NOT raise PermissionError for a claimable tui source.
try:
routes_module._get_or_materialize_session("tui-mat-1")
except PermissionError:
pytest.fail("a genuine TUI session must remain materializable")
except Exception:
# Other errors (e.g. workspace/save plumbing under the minimal fixture)
# are out of scope; the contract here is: NOT a PermissionError.
pass
def test_materialize_helper_refuses_persisted_writable_subagent_sidecar(
routes_module, isolated_state_db, monkeypatch
):
"""A subagent sidecar previously persisted with read_only=False (e.g.
materialized before this fix) must still be refused by
_get_or_materialize_session on the happy path — chat-start cannot use it as
a writable session (#5307 Codex round 6)."""
import api.models as _models
class _FakeSession:
session_id = "persisted-sa"
read_only = False
source_tag = "subagent"
raw_source = "subagent"
is_cli_session = False
messages = [{"role": "user", "content": "hi"}]
monkeypatch.setattr(routes_module, "get_session", lambda _sid: _FakeSession())
monkeypatch.setattr(
routes_module, "_ensure_full_session_before_mutation",
lambda _sid, s: s, raising=False,
)
with pytest.raises(PermissionError):
routes_module._get_or_materialize_session("persisted-sa")
def test_subagent_view_only_guard_helper(routes_module, isolated_state_db):
"""_session_is_subagent_view_only is True for a state.db subagent source
(used by the delete/truncate/clear/pin mutation-route guards, #5307)."""
_make_state_db(
isolated_state_db["db"], "sa-guard-1", source="subagent", message_count=1,
)
assert routes_module._session_is_subagent_view_only("sa-guard-1") is True
_make_state_db(
isolated_state_db["db"], "tui-guard-1", source="tui", message_count=1,
)
# a non-subagent id with no matching sidecar is not flagged
assert routes_module._session_is_subagent_view_only("nope-nope") is False
def test_mutation_routes_guard_subagent_source_in_source():
"""Static contract: the direct session-mutation routes (delete / clear /
truncate / pin) refuse subagent children via _session_is_subagent_view_only
before mutating, and the /api/sessions list coerces subagent rows to
read_only=True / is_cli_session=False (#5307 Codex round 8 — prevents
delete/swipe from erasing the child's state.db transcript)."""
src = ROUTES_PY.read_text(encoding="utf-8")
# each mutation route body must call the guard
for route in ("/api/session/delete", "/api/session/clear",
"/api/session/truncate", "/api/session/pin"):
idx = src.index(f'parsed.path == "{route}"')
nxt = src.index('parsed.path == "/api/session', idx + 10)
block = src[idx:nxt]
assert "_session_is_subagent_view_only(" in block, (
f"{route} must guard against subagent children before mutating"
)
# list coercion present
assert "_coerce_subagent_rows" in src, (
"the /api/sessions list must coerce subagent rows to read_only/non-CLI"
)
+2 -2
View File
@@ -344,7 +344,7 @@ def test_server_delete_prunes_session_index(cleanup_test_sessions):
text.find('if parsed.path == "/api/session/delete":'),
)
if delete_idx >= 0:
delete_block = text[delete_idx:delete_idx+1800]
delete_block = text[delete_idx:delete_idx+2400]
assert "prune_session_from_index(sid)" in delete_block, \
f"{label} session/delete must prune SESSION_INDEX_FILE"
return
@@ -359,7 +359,7 @@ def test_server_delete_removes_session_bak_snapshot(cleanup_test_sessions):
routes_src.find('if parsed.path == "/api/session/delete":'),
)
assert delete_idx >= 0, "session/delete handler not found in api/routes.py"
delete_block = routes_src[delete_idx:delete_idx+1800]
delete_block = routes_src[delete_idx:delete_idx+2400]
assert "with_suffix('.json.bak').unlink" in delete_block or 'with_suffix(".json.bak").unlink' in delete_block, \
"session/delete must unlink <sid>.json.bak to avoid later orphan-backup recovery"
+10 -10
View File
@@ -98,7 +98,7 @@ def test_duplicate_creates_independent_session():
# Extract the duplicate endpoint code (next few lines)
lines = content[duplicate_start:].split('\n')
endpoint_code = '\n'.join(lines[:80])
endpoint_code = '\n'.join(lines[:100])
# Verify that parent_session_id is NOT passed to Session constructor
assert 'parent_session_id' not in endpoint_code, \
@@ -127,7 +127,7 @@ def test_duplicate_session_copies_title_logic():
assert duplicate_start != -1, "Duplicate endpoint not found"
lines = content[duplicate_start:].split('\n')
endpoint_code = '\n'.join(lines[:80])
endpoint_code = '\n'.join(lines[:100])
# Verify title includes (copy). Accept the original `session.title + " (copy)"`
# form OR the May 2 2026 SF-3 hardened form `(session.title or "Untitled") + " (copy)"`
@@ -150,7 +150,7 @@ def test_duplicate_session_copies_messages_logic():
assert duplicate_start != -1, "Duplicate endpoint not found"
lines = content[duplicate_start:].split('\n')
endpoint_code = '\n'.join(lines[:80])
endpoint_code = '\n'.join(lines[:100])
# Verify messages are copied from original session. Accept either the
# plain assignment (insufficient — see test_duplicate_runtime_messages_independence)
@@ -171,7 +171,7 @@ def test_duplicate_session_copies_model_logic():
assert duplicate_start != -1, "Duplicate endpoint not found"
lines = content[duplicate_start:].split('\n')
endpoint_code = '\n'.join(lines[:80])
endpoint_code = '\n'.join(lines[:100])
# Verify model is copied
assert 'model=session.model' in endpoint_code, \
@@ -189,7 +189,7 @@ def test_duplicate_session_copies_workspace_logic():
assert duplicate_start != -1, "Duplicate endpoint not found"
lines = content[duplicate_start:].split('\n')
endpoint_code = '\n'.join(lines[:80])
endpoint_code = '\n'.join(lines[:100])
# Verify workspace is copied
assert 'workspace=session.workspace' in endpoint_code, \
@@ -207,7 +207,7 @@ def test_duplicate_session_copies_all_session_properties():
assert duplicate_start != -1, "Duplicate endpoint not found"
lines = content[duplicate_start:].split('\n')
endpoint_code = '\n'.join(lines[:80])
endpoint_code = '\n'.join(lines[:100])
# Extract the copied_session = Session( lines
session_construction_start = endpoint_code.find('copied_session = Session(')
@@ -257,7 +257,7 @@ def test_duplicate_uses_deepcopy_for_messages():
duplicate_start = content.find('if parsed.path == "/api/session/duplicate":')
assert duplicate_start != -1, "Duplicate endpoint not found"
lines = content[duplicate_start:].split('\n')
endpoint_code = '\n'.join(lines[:80])
endpoint_code = '\n'.join(lines[:100])
assert 'copy.deepcopy(session.messages)' in endpoint_code, \
"duplicate must use copy.deepcopy(session.messages) — plain assignment shares list refs"
assert 'copy.deepcopy(session.tool_calls)' in endpoint_code, \
@@ -277,7 +277,7 @@ def test_duplicate_explicitly_persists_to_disk():
duplicate_start = content.find('if parsed.path == "/api/session/duplicate":')
assert duplicate_start != -1, "Duplicate endpoint not found"
lines = content[duplicate_start:].split('\n')
endpoint_code = '\n'.join(lines[:80])
endpoint_code = '\n'.join(lines[:100])
assert 'copied_session.save()' in endpoint_code, \
"duplicate must call .save() explicitly — without it the copy vanishes on refresh"
@@ -294,7 +294,7 @@ def test_duplicate_resets_pinned_and_archived():
duplicate_start = content.find('if parsed.path == "/api/session/duplicate":')
assert duplicate_start != -1, "Duplicate endpoint not found"
lines = content[duplicate_start:].split('\n')
endpoint_code = '\n'.join(lines[:80])
endpoint_code = '\n'.join(lines[:100])
# Both must be hard-coded to False, NOT inherited from `session.pinned`/`session.archived`
assert 'pinned=False' in endpoint_code, \
"duplicate must reset pinned=False — duplicating shouldn't propagate pin state"
@@ -318,7 +318,7 @@ def test_duplicate_returns_404_when_session_not_found():
duplicate_start = content.find('if parsed.path == "/api/session/duplicate":')
assert duplicate_start != -1, "Duplicate endpoint not found"
lines = content[duplicate_start:].split('\n')
endpoint_code = '\n'.join(lines[:80])
endpoint_code = '\n'.join(lines[:100])
assert 'bad(handler, "Session not found", status=404)' in endpoint_code, \
"missing session must return status=404, not the default 400"