fix(#5307): recover subagent child transcript view-only (Codex hardening)
Reworked per the Codex gate finding: the first approach widened the client import predicate, which (a) conflicted with #3603's intentional _isExternalSession gate and (b) let import_cli persist the subagent child as a WRITABLE, CLI-classified session that then passed the poll-skip/active-refresh gates. Corrected to a server-side, view-only recovery: - api/routes.py: mark source='subagent' as NON-claimable in _is_claimable_cli_source (both cli_meta and state.db source denylists). A subagent child now resolves via the not_claimable branch -> read-only Session with its state.db transcript, and build_session takes is_cli_flag (False for subagent children) so the recovered session is NOT CLI-classified and can't widen the frontend _isExternalSession gates. - static/sessions.js: REVERTED to master (no client change needed; #3603 contract intact). - tests: assert reason=not_claimable, read_only=True, is_cli_session!=True, transcript present; #2782 deleted-webui 404 preserved; #3603 _isExternalSession contract preserved. 58 tests green (5307 + 3603 + claim-cli + core-data-loss). Fixes #5307
This commit is contained in:
1 parent
bed98c787c
commit
a823ab5ff8
3 files changed
+58
-60
No files matched your search
+23
-5
@@ -6644,7 +6644,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.
|
||||
@@ -6663,7 +6663,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, ""
|
||||
|
||||
@@ -6740,7 +6740,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",
|
||||
@@ -6751,7 +6751,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"),
|
||||
@@ -6853,7 +6859,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"
|
||||
|
||||
|
||||
|
||||
+5
-41
@@ -1807,39 +1807,6 @@ 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) {
|
||||
@@ -6944,7 +6911,7 @@ function renderSessionListFromCache(){
|
||||
row.title=t('session_lineage_segment_open');
|
||||
row.onclick=async(e)=>{
|
||||
e.stopPropagation();
|
||||
if(_sessionNeedsServerImportForLoad(seg)){
|
||||
if(_isExternalSession(seg)){
|
||||
try{await api('/api/session/import_cli',{method:'POST',body:JSON.stringify(_externalImportPayload(seg))});}
|
||||
catch(_e){ /* read-only fallback */ }
|
||||
}
|
||||
@@ -6961,7 +6928,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(_sessionNeedsServerImportForLoad(childSession)){
|
||||
if(_isExternalSession(childSession)){
|
||||
try{await api('/api/session/import_cli',{method:'POST',body:JSON.stringify(_externalImportPayload(childSession))});}
|
||||
catch(_e){ /* read-only fallback */ }
|
||||
}
|
||||
@@ -7588,12 +7555,9 @@ function renderSessionListFromCache(){
|
||||
_tapTimer=null;
|
||||
_lastTapTime=0;
|
||||
if(_renamingSid) return;
|
||||
// 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)){
|
||||
// For external sessions (CLI, Discord, Telegram, Slack), import into
|
||||
// WebUI store first so /api/chat/start finds a persisted session.
|
||||
if(_isExternalSession(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 */ }
|
||||
|
||||
@@ -168,16 +168,16 @@ def test_was_webui_gate_excludes_subagent_children():
|
||||
), "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."""
|
||||
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")
|
||||
assert "function _isSubagentChildSession(" in js
|
||||
assert "function _sessionNeedsServerImportForLoad(" in js
|
||||
assert js.count("_sessionNeedsServerImportForLoad(") >= 4, (
|
||||
"expected the helper definition + 3 load/tap call sites"
|
||||
# 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)"
|
||||
)
|
||||
|
||||
|
||||
@@ -186,12 +186,15 @@ def test_sessions_js_uses_wider_import_predicate():
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_subagent_child_indexed_as_webui_is_not_404_was_webui(
|
||||
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' — it must recover
|
||||
its state.db transcript (#5307)."""
|
||||
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,
|
||||
@@ -206,9 +209,22 @@ def test_subagent_child_indexed_as_webui_is_not_404_was_webui(
|
||||
)
|
||||
|
||||
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}"
|
||||
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(
|
||||
|
||||
Reference in new issue
Block a user