From a823ab5ff84a9405df430c3dc7658d7d09efcf2f Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Wed, 1 Jul 2026 07:24:16 +0000 Subject: [PATCH] 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 --- api/routes.py | 28 +++++++++--- static/sessions.js | 46 +++----------------- tests/test_5307_subagent_child_transcript.py | 44 +++++++++++++------ 3 files changed, 58 insertions(+), 60 deletions(-) diff --git a/api/routes.py b/api/routes.py index 8d50a915..2da545c4 100644 --- a/api/routes.py +++ b/api/routes.py @@ -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" diff --git a/static/sessions.js b/static/sessions.js index 9e0a5695..80c63719 100644 --- a/static/sessions.js +++ b/static/sessions.js @@ -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 */ } diff --git a/tests/test_5307_subagent_child_transcript.py b/tests/test_5307_subagent_child_transcript.py index 097148fe..61f71e29 100644 --- a/tests/test_5307_subagent_child_transcript.py +++ b/tests/test_5307_subagent_child_transcript.py @@ -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(