From 7721e530fbe0eec578ca3bb8dbff6b64e54f59f1 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Mon, 6 Jul 2026 04:36:13 +0000 Subject: [PATCH 1/8] fix(#5657): preserve workspace file-tree scroll position across re-render MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit renderFileTree() clears its scroll container with box.innerHTML='' and rebuilds every row. Detaching the rows collapses scrollHeight, so the browser clamps scrollTop to 0 — every folder expand/collapse, breadcrumb nav, refresh, and hidden-files toggle that re-runs the renderer teleported the reader to the top of a long tree, losing their place (the dominant interaction in the panel). Fix: capture box.scrollTop before the innerHTML='' wipe and restore it after the normal render tail (after _renderTreeItems repaints and the container is tall again). A plain scrollTop restore is sufficient and correct here — expand/collapse insert or remove rows BELOW the clicked disclosure, so the clicked row keeps its offset from the top; no getBoundingClientRect anchor delta is needed (that's only for the prepend-above case in the chat messages list). The reporter's data-path anchor sketch is deliberately NOT used: _renderTreeItems rows carry no data-path, so that selector would match nothing. The two early-return paths (no-workspace hides the box; empty-dir has nothing to scroll) are left untouched — they legitimately reset. The restore guards only the normal render tail. loadDir's two-render sequence (initial + post-expanded-dirs prefetch) is handled correctly: each renderFileTree() call independently captures then restores its own pre-wipe scrollTop across the await. Test: tests/test_issue5657_filetree_scroll_preserve.py — static guards that the capture precedes the wipe and the restore follows _renderTreeItems (per the maintainer test plan; drops the getBoundingClientRect anchor assertion since the fix deliberately doesn't use it). Fail-without-fix verified; 355 workspace-tree regression tests green. Closes #5657. --- static/ui.js | 15 ++++- ...test_issue5657_filetree_scroll_preserve.py | 64 +++++++++++++++++++ 2 files changed, 78 insertions(+), 1 deletion(-) create mode 100644 tests/test_issue5657_filetree_scroll_preserve.py diff --git a/static/ui.js b/static/ui.js index 68c2c523..c5089ef0 100644 --- a/static/ui.js +++ b/static/ui.js @@ -17212,7 +17212,18 @@ if(!S._expandedDirs) S._expandedDirs=new Set(); if(!S._dirCache) S._dirCache={}; function renderFileTree(){ - const box=$('fileTree');box.innerHTML=''; + const box=$('fileTree'); + // #5657: capture the scroll position before wiping the container. box.innerHTML='' + // detaches every row, collapsing scrollHeight so the browser clamps scrollTop to 0; + // without this, every expand/collapse, breadcrumb nav, refresh, and hidden-files + // toggle that re-runs renderFileTree() teleports the reader back to the top of a + // long tree. Restored only after the normal render tail below — the two early-return + // paths (no-workspace hides the box; empty-dir has nothing to scroll) legitimately + // reset. A plain scrollTop restore suffices here: expand/collapse insert/remove rows + // BELOW the clicked disclosure, so the clicked row keeps its offset from the top (no + // getBoundingClientRect anchor delta needed — that's only for prepend-above cases). + const prevScrollTop=box?box.scrollTop:0; + box.innerHTML=''; // Cache current dir entries S._dirCache[S.currentDir||'.']=S.entries; // Show empty-state when no workspace is set or the directory is empty (#703) @@ -17231,6 +17242,8 @@ function renderFileTree(){ return; } _renderTreeItems(box, visibleEntries, 0); + // #5657: restore the pre-wipe scroll position now that the tree is tall again. + if(box) box.scrollTop=prevScrollTop; } let _wsActiveDragPath=null; diff --git a/tests/test_issue5657_filetree_scroll_preserve.py b/tests/test_issue5657_filetree_scroll_preserve.py new file mode 100644 index 00000000..fb21fd46 --- /dev/null +++ b/tests/test_issue5657_filetree_scroll_preserve.py @@ -0,0 +1,64 @@ +"""Regression coverage for #5657 — workspace file-tree preserves scroll on re-render. + +`renderFileTree()` clears its scroll container with ``box.innerHTML=''`` and +rebuilds every row. That detaches the rows, collapses ``scrollHeight``, and the +browser clamps ``scrollTop`` to 0 — so every folder expand/collapse, breadcrumb +nav, refresh, and hidden-files toggle that re-runs the renderer teleported the +reader to the top of a long tree. + +Fix: capture ``scrollTop`` before the wipe and restore it after the normal +render tail. A plain scrollTop restore is sufficient (expand/collapse insert or +remove rows BELOW the clicked disclosure, so the clicked row keeps its offset) — +the reporter's getBoundingClientRect anchor sketch is deliberately NOT used, and +the ``.file-item`` rows carry no ``data-path`` for it anyway. +""" + +from pathlib import Path + + +REPO_ROOT = Path(__file__).resolve().parents[1] +UI_JS = (REPO_ROOT / "static" / "ui.js").read_text(encoding="utf-8") + + +def _render_file_tree_body() -> str: + start = UI_JS.index("function renderFileTree()") + end = UI_JS.index("\nfunction ", start + 1) + return UI_JS[start:end] + + +def _render_file_tree_code() -> str: + """Body with // comment lines stripped, so assertions anchor on real code.""" + lines = [ln for ln in _render_file_tree_body().splitlines() if not ln.strip().startswith("//")] + return "\n".join(lines) + + +def test_render_file_tree_captures_and_restores_scrolltop(): + body = _render_file_tree_code() + # It must still wipe the container (the behavior that loses scroll)... + assert "box.innerHTML=''" in body or 'box.innerHTML = ""' in body + # ...and it must capture scrollTop BEFORE the wipe and restore it after. + assert "scrollTop" in body, ( + "renderFileTree must capture + restore scrollTop around the innerHTML wipe (#5657)" + ) + capture_idx = body.index("prevScrollTop=box") + wipe_idx = body.index("box.innerHTML=''") + assert capture_idx < wipe_idx, ( + "scrollTop must be captured BEFORE box.innerHTML='' so it isn't already clamped to 0" + ) + render_idx = body.index("_renderTreeItems(box") + restore_idx = body.rindex("box.scrollTop=") + assert restore_idx > render_idx, ( + "scrollTop must be restored AFTER _renderTreeItems repaints the tree" + ) + + +def test_early_return_paths_still_reset_scroll(): + # The no-workspace and empty-dir early returns legitimately want scroll reset + # (box hidden / nothing to scroll) — the restore must live on the normal tail + # only, i.e. AFTER _renderTreeItems, not before the early returns. + body = _render_file_tree_code() + render_idx = body.index("_renderTreeItems(box") + tail = body[render_idx:] + assert "box.scrollTop=" in tail or "box.scrollTop =" in tail, ( + "the scrollTop restore must be on the normal render tail, after _renderTreeItems" + ) From 7fbb9bc98df1e7058205e31d2a5fd3445e3684ae Mon Sep 17 00:00:00 2001 From: t Date: Mon, 6 Jul 2026 23:25:27 +0000 Subject: [PATCH 2/8] docs(changelog): stamp #5664 workspace file-tree scroll preserve --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5212775b..cd2464dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ ### Fixed +- **The workspace file tree no longer jumps to the top when you expand a folder.** Every folder expand/collapse, breadcrumb navigation, refresh, and hidden-files toggle re-rendered the tree by wiping its container, which collapsed its height and made the browser reset the scroll position to the top — teleporting you away from where you were in a long tree. The scroll position is now captured before the re-render and restored after. Reported by @claw-io. (#5664, #5657) + - **Self-update recovers when a `.git/index.lock` is blocking it.** If "Update Now" failed because a stale `.git/index.lock` was present (a previous git process left it behind), the WebUI gave no in-UI way out — you were stuck on an un-updatable install. The update banner now detects that specific failure, shows the exact `rm -f …/.git/index.lock` command to run on the host, and offers a "Retry update" button that re-runs the normal update once the lock is gone. The server never deletes anything under `.git` itself (fail-closed — git's lock can't be safely auto-cleared without racing), so this is purely a detect-and-guide recovery. Thanks @b3nw. (#5688, #5687) ### Internal From 02692cd64ab18b34bfbc15ba84d748ced204e537 Mon Sep 17 00:00:00 2001 From: b3nw Date: Mon, 6 Jul 2026 23:19:46 +0000 Subject: [PATCH 3/8] fix: surface agent's graceful summary on iteration-limit exhaustion (closes #5494) The WebUI already surfaces the model-generated summary when the agent's tool-iteration budget is exhausted. The remaining gap: when the agent hits the limit but its summary call yields no usable assistant content (common with reasoning-only responses), the user sees a bare `tool_limit_reached` error instead of any closure text. `hermes-agent.handle_max_iterations()` always returns a non-empty `final_response` (either the summary or a graceful fallback), and `AIAgent` exposes that string on the result dict. The WebUI, by contrast, only reads `result['messages']`, so an empty summary left no final assistant answer and routed to the error path. Inject `result['final_response']` as a final assistant turn when `max_iterations_reached` fires and no usable answer exists in `messages`. The existing `_mark_latest_assistant_tool_limit_status` flow then attaches the status card and the user sees the closure text instead of the bare error. Hermes-agent parity preserved (it's literally the same string); the bare-error path stays for the case where the agent died entirely (no fallback either). Tests cover: fallback-injection surfaces closure text + status card, no double-injection when a real summary already exists, the existing 'died entirely' apperror path is unchanged, and three unit cases for the helper's skip conditions. Co-authored-by: b3nw --- api/streaming.py | 48 ++++++++++ tests/test_tool_limit_terminal_state.py | 117 ++++++++++++++++++++++++ 2 files changed, 165 insertions(+) diff --git a/api/streaming.py b/api/streaming.py index 6eeb3892..7a6e6209 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -1389,6 +1389,40 @@ def _agent_result_tool_limit_reached(result) -> bool: return False +def _maybe_inject_max_iteration_summary_fallback(messages, result) -> list: + """Append the agent's graceful summary text as an assistant turn when one is missing. + + When ``AIAgent`` exhausts its iteration budget, ``agent.handle_max_iterations`` + always returns a non-empty ``final_response`` — either the model-generated + summary or a graceful fallback (e.g. ``"I reached the iteration limit and + couldn't generate a summary."``). Hermes Agent surfaces that string as the + final answer to the user; the WebUI, by contrast, reads only ``messages``, + so an empty summary (common with reasoning-only responses) left the user + with a bare ``tool_limit_reached`` error instead of any closure text. + + When ``_tool_limit_reached`` is true and ``messages`` ends without a final + assistant answer, inject ``result['final_response']`` as a new assistant + turn so ``_mark_latest_assistant_tool_limit_status`` can attach the status + card in the normal flow and the user sees the same closure text as + hermes-agent. Returns the (possibly new) messages list; does nothing when + a usable assistant answer already exists or when ``result`` carries no + graceful fallback text. + """ + if not isinstance(result, dict): + return list(messages or []) + fallback = result.get('final_response') + if not isinstance(fallback, str) or not fallback.strip(): + return list(messages or []) + out = list(messages or []) + if not _session_lacks_final_assistant_answer(out): + return out + # Append a synthetic summary turn. Tag it so downstream consumers can + # distinguish it from model-emitted assistant turns if needed; mirrors the + # synthetic-scaffolding flag convention already used elsewhere (#5334). + out.append({"role": "assistant", "content": fallback, "_max_iteration_summary_fallback": True}) + return out + + def _mark_latest_assistant_tool_limit_status(messages) -> bool: """Annotate the latest usable assistant final answer as limit-stopped.""" for msg in reversed(list(messages or [])): @@ -8239,6 +8273,20 @@ def _run_agent_streaming( _result_messages, enabled=_tool_limit_reached, ) + # #5494 — parity with hermes-agent's handle_max_iterations() return + # value. When the agent produced no usable summary assistant + # message but result['final_response'] carries a graceful fallback + # string, inject it as a final assistant turn so the user sees + # closure text instead of a bare tool_limit_reached error. Apply + # the synthesis to result['messages'] AND _result_messages so + # downstream _all_result_messages checks (silent-failure detection + # at #all_result_messages on the same turn) see the fallback too. + if _tool_limit_reached: + _result_messages = _maybe_inject_max_iteration_summary_fallback( + _result_messages, result + ) + if isinstance(result, dict) and result.get('messages') is not None: + result = {**result, 'messages': _result_messages} if cancel_event.is_set(): _finalize_cancelled_turn(s, ephemeral=False) try: diff --git a/tests/test_tool_limit_terminal_state.py b/tests/test_tool_limit_terminal_state.py index 80e4cd38..1a2becdc 100644 --- a/tests/test_tool_limit_terminal_state.py +++ b/tests/test_tool_limit_terminal_state.py @@ -280,6 +280,123 @@ def test_streaming_tool_limit_without_final_answer_emits_no_final_apperror(tmp_p ) +def test_streaming_tool_limit_with_fallback_final_response_surfaces_closure_text(tmp_path, monkeypatch): + """#5494 — when the agent produced no usable summary but returned a graceful + fallback string (handle_max_iterations() always returns one), inject that as a + final assistant turn so the user sees closure text instead of a bare error. + """ + graceful = "I reached the iteration limit and couldn't generate a summary." + result = { + "turn_exit_reason": "max_iterations_reached(30/30)", + "final_response": graceful, + "messages": [ + {"role": "user", "content": "Do the long task."}, + {"role": "assistant", "content": "", "tool_calls": [{"id": "call_1"}]}, + {"role": "tool", "tool_call_id": "call_1", "content": "result"}, + {"role": "user", "content": streaming._MAX_ITERATION_SUMMARY_REQUEST}, + ], + } + + events, payload = _run_streaming_with_fake_agent(tmp_path, monkeypatch, result) + + # The user sees the graceful fallback, not a bare tool_limit_reached error. + assert not [ + ap for ev, ap in events if ev == "apperror" + and ap.get("type") == "tool_limit_reached" + ], "expected no tool_limit_reached apperror when fallback was returned" + done_payloads = [payload for event, payload in events if event == "done"] + assert done_payloads, "expected done SSE payload" + assert done_payloads[-1]["terminal_state"] == "tool_limit_reached" + + # Fallback text is shown as a final assistant message and is annotated + # with the status card so the UI can render the 'limit reached' chip. + assistant = payload["messages"][-1] + assert assistant["role"] == "assistant" + assert assistant["content"] == graceful + assert assistant["_terminal_state"] == "tool_limit_reached" + assert assistant["_statusCard"]["title"] == "Tool iteration limit reached" + # Synthetic scaffolding turn was still dropped, even after fallback injection. + assert all( + message.get("content") != streaming._MAX_ITERATION_SUMMARY_REQUEST + for message in payload["messages"] + ) + + +def test_streaming_tool_limit_with_fallback_does_not_double_inject_when_assistant_exists(tmp_path, monkeypatch): + """#5494 — when the agent already appended a model-generated summary, the + fallback flow is a no-op; the existing summary is preserved as before. + """ + summary = "I reached the limit; here is the summary." + result = { + "turn_exit_reason": "max_iterations_reached(30/30)", + "final_response": summary, + "messages": [ + {"role": "user", "content": "Do the long task."}, + {"role": "assistant", "content": "", "tool_calls": [{"id": "call_1"}]}, + {"role": "tool", "tool_call_id": "call_1", "content": "result"}, + {"role": "user", "content": streaming._MAX_ITERATION_SUMMARY_REQUEST}, + {"role": "assistant", "content": summary}, + ], + } + + events, payload = _run_streaming_with_fake_agent(tmp_path, monkeypatch, result) + + done_payloads = [payload for event, payload in events if event == "done"] + assert done_payloads + assistant_msgs = [ + m for m in payload["messages"] + if m.get("role") == "assistant" and m.get("content") == summary + ] + assert len(assistant_msgs) == 1, "fallback must not duplicate the existing summary" + assert assistant_msgs[0]["_terminal_state"] == "tool_limit_reached" + + +def test_maybe_inject_max_iteration_summary_fallback_unit(): + """Unit-level coverage for the injection helper.""" + messages = [ + {"role": "user", "content": "Do the long task."}, + {"role": "assistant", "content": "", "tool_calls": [{"id": "call_1"}]}, + {"role": "tool", "tool_call_id": "call_1", "content": "result"}, + ] + graceful = "I reached the iteration limit and couldn't generate a summary." + result = {"final_response": graceful} + + injected = streaming._maybe_inject_max_iteration_summary_fallback(messages, result) + + assert injected[-1]["role"] == "assistant" + assert injected[-1]["content"] == graceful + assert injected[-1]["_max_iteration_summary_fallback"] is True + + +def test_maybe_inject_max_iteration_summary_fallback_skips_when_assistant_present(): + messages = [ + {"role": "user", "content": "Do the long task."}, + {"role": "assistant", "content": "real summary"}, + ] + result = {"final_response": "fallback text"} + + out = streaming._maybe_inject_max_iteration_summary_fallback(messages, result) + assert out == messages + + +def test_maybe_inject_max_iteration_summary_fallback_skips_when_no_fallback(): + messages = [ + {"role": "user", "content": "Do the long task."}, + {"role": "assistant", "content": "", "tool_calls": [{"id": "call_1"}]}, + ] + + out = streaming._maybe_inject_max_iteration_summary_fallback(messages, {}) + assert out == messages + + out = streaming._maybe_inject_max_iteration_summary_fallback( + messages, {"final_response": " "} + ) + assert out == messages + + out = streaming._maybe_inject_max_iteration_summary_fallback(messages, None) + assert out == messages + + def test_streaming_tool_limit_terminal_failure_does_not_mark_final_answer(tmp_path, monkeypatch): result = { "status": "partial", From 34fcc3a38799fb15e5902e1621febb555721981c Mon Sep 17 00:00:00 2001 From: b3nw Date: Tue, 7 Jul 2026 00:04:27 +0000 Subject: [PATCH 4/8] fix: drop messages-None writeback guard, reframe for invariant lineage Per Greptile's review on #5717: the previous `if result.get('messages') is not None` guard created an asymmetric condition where the silent-failure classifier could miss the injected fallback when the agent result had `messages: None` (which never actually happens in practice since `finalize_turn` always returns a list, but the asymmetry read as code smell). Drop the guard; the writeback is now unconditional on `isinstance(result, dict)`. Per the maintainer's review on #5717: there may be a `finalize_turn` invariant on the agent branch this WebUI ships against that already appends the fallback as an assistant row, in which case the live bug is gone and this PR becomes a regression guard. Reframe the test docstrings as dual-purpose pins of the user- visible closure-text contract so they read correctly under both interpretations. Co-authored-by: b3nw --- api/streaming.py | 10 +++++++--- tests/test_tool_limit_terminal_state.py | 19 ++++++++++++++----- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/api/streaming.py b/api/streaming.py index 7a6e6209..4bc49fe0 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -8278,14 +8278,18 @@ def _run_agent_streaming( # message but result['final_response'] carries a graceful fallback # string, inject it as a final assistant turn so the user sees # closure text instead of a bare tool_limit_reached error. Apply - # the synthesis to result['messages'] AND _result_messages so + # the synthesis to result['messages'] AND _result_messages so the # downstream _all_result_messages checks (silent-failure detection - # at #all_result_messages on the same turn) see the fallback too. + # at api/streaming.py:_assistant_reply_added_after_current_turn) + # see the fallback too. `finalize_turn` in the agent always returns + # messages as a list, but we write back unconditionally so the + # contract is "if we built a result-messages list, the silent-failure + # classifier reads the augmented version." if _tool_limit_reached: _result_messages = _maybe_inject_max_iteration_summary_fallback( _result_messages, result ) - if isinstance(result, dict) and result.get('messages') is not None: + if isinstance(result, dict): result = {**result, 'messages': _result_messages} if cancel_event.is_set(): _finalize_cancelled_turn(s, ephemeral=False) diff --git a/tests/test_tool_limit_terminal_state.py b/tests/test_tool_limit_terminal_state.py index 1a2becdc..dcc6e61e 100644 --- a/tests/test_tool_limit_terminal_state.py +++ b/tests/test_tool_limit_terminal_state.py @@ -281,9 +281,14 @@ def test_streaming_tool_limit_without_final_answer_emits_no_final_apperror(tmp_p def test_streaming_tool_limit_with_fallback_final_response_surfaces_closure_text(tmp_path, monkeypatch): - """#5494 — when the agent produced no usable summary but returned a graceful - fallback string (handle_max_iterations() always returns one), inject that as a - final assistant turn so the user sees closure text instead of a bare error. + """#5494 — handle_max_iterations() guarantees a non-empty ``final_response`` + on iteration-limit exhaustion. This test pins the WebUI contract that, + when ``messages`` ends without a final assistant turn and ``final_response`` + is set, the user sees that closure text instead of a bare + ``tool_limit_reached`` error. Serves both as a live-bug fix pin and as a + regression guard for the agent's "delivered final_response ⇒ assistant + row" invariant: if a future agent regression drops that invariant, this + test still passes because the WebUI honors the contract locally. """ graceful = "I reached the iteration limit and couldn't generate a summary." result = { @@ -300,6 +305,9 @@ def test_streaming_tool_limit_with_fallback_final_response_surfaces_closure_text events, payload = _run_streaming_with_fake_agent(tmp_path, monkeypatch, result) # The user sees the graceful fallback, not a bare tool_limit_reached error. + # Either (a) we synthesized the fallback here, or (b) the agent guarantee + # already added an assistant row and `_mark_latest_assistant_tool_limit_status` + # attached the status card. Both routes satisfy the contract. assert not [ ap for ev, ap in events if ev == "apperror" and ap.get("type") == "tool_limit_reached" @@ -323,8 +331,9 @@ def test_streaming_tool_limit_with_fallback_final_response_surfaces_closure_text def test_streaming_tool_limit_with_fallback_does_not_double_inject_when_assistant_exists(tmp_path, monkeypatch): - """#5494 — when the agent already appended a model-generated summary, the - fallback flow is a no-op; the existing summary is preserved as before. + """#5494 — when the agent already appended a model-generated summary + AND ``final_response`` carries the same text, the WebUI must not duplicate + the assistant turn. Pins the no-op contract on the synthesis path. """ summary = "I reached the limit; here is the summary." result = { From 6c7746aef65e52702764f89132659d836aef2e82 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Tue, 7 Jul 2026 01:44:48 +0000 Subject: [PATCH 5/8] docs(changelog): iteration-limit graceful summary (#5717, #5494) --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index cd2464dc..576f6f01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ ### Fixed +- **The agent's closing summary now shows when it hits the iteration limit.** When a turn exhausted its tool/iteration budget, the agent produces a graceful closing message (a summary of what it got done, or a fallback like "I reached the iteration limit…"), but the WebUI showed a bare `tool_limit_reached` error card instead — most visible with reasoning-only models (extended-thinking Claude, DeepSeek, Codex Responses) whose summary arrives empty. The closing text is now surfaced as a proper assistant message. Purely additive and guarded — it never fires on a turn that already produced a final answer, never double-injects, and degrades to the old behavior when no fallback text is available. Thanks @claw-io (co-authored by @b3nw). (#5717, #5494) + - **The workspace file tree no longer jumps to the top when you expand a folder.** Every folder expand/collapse, breadcrumb navigation, refresh, and hidden-files toggle re-rendered the tree by wiping its container, which collapsed its height and made the browser reset the scroll position to the top — teleporting you away from where you were in a long tree. The scroll position is now captured before the re-render and restored after. Reported by @claw-io. (#5664, #5657) - **Self-update recovers when a `.git/index.lock` is blocking it.** If "Update Now" failed because a stale `.git/index.lock` was present (a previous git process left it behind), the WebUI gave no in-UI way out — you were stuck on an un-updatable install. The update banner now detects that specific failure, shows the exact `rm -f …/.git/index.lock` command to run on the host, and offers a "Retry update" button that re-runs the normal update once the lock is gone. The server never deletes anything under `.git` itself (fail-closed — git's lock can't be safely auto-cleared without racing), so this is purely a detect-and-guide recovery. Thanks @b3nw. (#5688, #5687) From f106fcd1f56a93f53e4420d662aceda7236c6d6e Mon Sep 17 00:00:00 2001 From: Jinho Seok Date: Tue, 7 Jul 2026 01:50:38 +0000 Subject: [PATCH 6/8] fix(workspace-git): hide git subprocess console windows on Windows (#5692) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add creationflags to _run_git() and _config_names_for_scope() so git child processes don't accumulate hidden cmd.exe/conhost windows on Windows. Uses a local module-level _windows_hide_flags() helper (mirrors the api/updates.py pattern: CREATE_NO_WINDOW on win32, 0 elsewhere) rather than importing from the OPTIONAL hermes_cli package — a standalone/agent-less WebUI (and the repo's own CI runners, which don't pip-install hermes-agent) must keep full workspace-git functionality. windows_hide_flags() is a safe no-op (0) on non-Windows. Scoped to workspace git commands. Adds mutation-checked tests asserting the kwarg is wired on both sites. Co-authored-by: nesquena-hermes --- api/workspace_git.py | 17 +++++++++++ tests/test_workspace_git.py | 58 +++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+) diff --git a/api/workspace_git.py b/api/workspace_git.py index a33fd316..457d8720 100644 --- a/api/workspace_git.py +++ b/api/workspace_git.py @@ -12,6 +12,7 @@ import logging import os import shutil import subprocess +import sys import tempfile import threading import re @@ -22,6 +23,20 @@ from typing import Iterable logger = logging.getLogger(__name__) + +def _windows_hide_flags() -> int: + """Win32 ``creationflags`` that hide a short-lived console child's window + (``CREATE_NO_WINDOW``) without detaching it, so ``capture_output`` still + works. Returns ``0`` on non-Windows — the ``subprocess`` default, a genuine + no-op. Mirrors the ``api/updates.py`` pattern; kept local so workspace-git + never takes a hard dependency on the optional ``hermes_cli`` package (a + standalone/agent-less WebUI must keep full git functionality). See #5692. + """ + if sys.platform == "win32": + return getattr(subprocess, "CREATE_NO_WINDOW", 0) + return 0 + + from api.workspace import rmtree_anchored, safe_resolve_ws, unlink_anchored @@ -248,6 +263,7 @@ def _run_git( text=True, timeout=timeout, env=run_env, + creationflags=_windows_hide_flags(), ) except subprocess.TimeoutExpired as exc: raise GitWorkspaceError("Git command timed out", "timeout") from exc @@ -290,6 +306,7 @@ def _config_names_for_scope( capture_output=True, timeout=GIT_TIMEOUT, env=env, + creationflags=_windows_hide_flags(), ) if result.returncode not in {0, 1}: if ignore_unsupported: diff --git a/tests/test_workspace_git.py b/tests/test_workspace_git.py index f9e28ac3..98e0838d 100644 --- a/tests/test_workspace_git.py +++ b/tests/test_workspace_git.py @@ -2515,3 +2515,61 @@ def test_dirty_worktree_uses_filter_neutralization(tmp_path): _dirty_worktree(ctx) assert not marker.exists() + + +def test_run_git_passes_windows_hide_flags(monkeypatch, tmp_path): + """_run_git must pass creationflags=_windows_hide_flags() so git child + processes don't accumulate visible console windows on Windows (#5692). + windows_hide_flags() is 0 on non-Windows, so this is a safe no-op there; + the test asserts the kwarg is wired regardless of platform.""" + import api.workspace_git as wg + from api.workspace_git import _windows_hide_flags + + repo = _init_repo(tmp_path / "repo") + (repo / "f.txt").write_text("x\n", encoding="utf-8") + _commit_all(repo) + + captured = {} + real_run = subprocess.run + + def fake_run(*args, **kwargs): + captured["creationflags"] = kwargs.get("creationflags", "MISSING") + return real_run(*args, **kwargs) + + monkeypatch.setattr(wg.subprocess, "run", fake_run) + wg._run_git(repo, ["rev-parse", "HEAD"]) + + assert captured.get("creationflags") == _windows_hide_flags(), ( + "_run_git must pass creationflags=_windows_hide_flags() to subprocess.run" + ) + + +def test_config_names_for_scope_passes_windows_hide_flags(monkeypatch, tmp_path): + """_config_names_for_scope must also pass creationflags=_windows_hide_flags() + (#5692) — the git-config probe spawns a console window on Windows too.""" + import re as _re + import api.workspace_git as wg + from api.workspace_git import _windows_hide_flags + + repo = _init_repo(tmp_path / "repo") + + captured = {} + real_run = subprocess.run + + def fake_run(*args, **kwargs): + captured["creationflags"] = kwargs.get("creationflags", "MISSING") + return real_run(*args, **kwargs) + + monkeypatch.setattr(wg.subprocess, "run", fake_run) + wg._config_names_for_scope( + "--local", + repo, + {}, + "filter\\..*", + _re.compile(r"^filter\."), + ignore_unsupported=True, + ) + + assert captured.get("creationflags") == _windows_hide_flags(), ( + "_config_names_for_scope must pass creationflags=_windows_hide_flags()" + ) From eb02c8ecd011a0543af5949b33df8f875ad05642 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Tue, 7 Jul 2026 02:19:40 +0000 Subject: [PATCH 7/8] docs(changelog): Windows git subprocess console-window fix (#5692) --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 576f6f01..e4e0ded6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ ### Fixed +- **Windows: git operations no longer leave hidden console windows piling up.** On Windows, every workspace git command (`_run_git`) and git-config probe spawned a short-lived `git.exe` whose console window could flash and accumulate as orphaned `conhost.exe` processes over a long session. Those subprocess calls now pass `CREATE_NO_WINDOW` so no window is created. Safe no-op on macOS/Linux, and it uses a small in-module helper (mirroring the update path) rather than a hard dependency on the optional agent package, so a standalone WebUI keeps full git functionality. Thanks @jin89ho. (#5692) + - **The agent's closing summary now shows when it hits the iteration limit.** When a turn exhausted its tool/iteration budget, the agent produces a graceful closing message (a summary of what it got done, or a fallback like "I reached the iteration limit…"), but the WebUI showed a bare `tool_limit_reached` error card instead — most visible with reasoning-only models (extended-thinking Claude, DeepSeek, Codex Responses) whose summary arrives empty. The closing text is now surfaced as a proper assistant message. Purely additive and guarded — it never fires on a turn that already produced a final answer, never double-injects, and degrades to the old behavior when no fallback text is available. Thanks @claw-io (co-authored by @b3nw). (#5717, #5494) - **The workspace file tree no longer jumps to the top when you expand a folder.** Every folder expand/collapse, breadcrumb navigation, refresh, and hidden-files toggle re-rendered the tree by wiping its container, which collapsed its height and made the browser reset the scroll position to the top — teleporting you away from where you were in a long tree. The scroll position is now captured before the re-render and restored after. Reported by @claw-io. (#5664, #5657) From 407451977e92189fa24c30903d902d9aa3fc38cd Mon Sep 17 00:00:00 2001 From: Daniel Harcek Date: Tue, 7 Jul 2026 01:58:03 +0000 Subject: [PATCH 8/8] fix: auto-switch profiles for session links (#5419) A valid session:// deep link to a session owned by a DIFFERENT Hermes profile used to look identical to a deleted session (404 -> frontend self-heals away). Now GET /api/session returns a structured 409 session_profile_mismatch envelope (error/code/session_id/profile ONLY, no transcript) ONLY when the owning profile is KNOWN, and loadSession() catches it, switches to the owning profile, and retries once. Truly missing/deleted or legacy None-profile sessions keep the 404 self-heal. Gate fixes applied (Codex + Fable): - Codex CORE: added a post-await stale-load guard after _switchProfileForSessionLoad so a navigation during the switch can't hijack the UI back to the old session. - Fable Finding 1: only emit 409 when _session_profile is truthy; a None-profile (missing/legacy) session under a non-default active profile now keeps 404 instead of a useless profile=null 409 (which skipped self-heal + spun the SSE reconnect against a dead sid). Both detail branches. + 2 regression tests. - Fable Finding 2: _switchProfileForSessionLoad now clears the sidebar skeleton + re-renders from cache on switch-POST failure (mirrors the #4671 canonical-switch catch), then rethrows, so a failed switch can't strand the sidebar on the skeleton. Reconciled tests/test_issue1611_session_profile_filtering.py (4 tests) to the 409 contract while preserving the no-leak boundary assertion. Co-authored-by: nesquena-hermes --- CHANGELOG.md | 2 + api/routes.py | 33 +++++ static/sessions.js | 69 ++++++++++ tests/test_bugfix_sweep.py | 11 ++ ...est_issue1611_session_profile_filtering.py | 119 ++++++++++++++++-- 5 files changed, 226 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e4e0ded6..53d0a1cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ ### Fixed +- **Clicking a session link that belongs to another profile now switches to that profile instead of failing.** A valid `session://` deep link pointing at a session owned by a different Hermes profile used to look identical to a deleted session — the UI showed "Session not available in web UI" and self-healed away. Now the WebUI recognizes a valid-but-wrong-profile session, offers to switch to the owning profile, and retries the load once; truly missing/deleted sessions still self-heal as before. No foreign-profile transcript is ever returned — the mismatch response carries only the owning profile name. Thanks @harcek. (#5419) + - **Windows: git operations no longer leave hidden console windows piling up.** On Windows, every workspace git command (`_run_git`) and git-config probe spawned a short-lived `git.exe` whose console window could flash and accumulate as orphaned `conhost.exe` processes over a long session. Those subprocess calls now pass `CREATE_NO_WINDOW` so no window is created. Safe no-op on macOS/Linux, and it uses a small in-module helper (mirroring the update path) rather than a hard dependency on the optional agent package, so a standalone WebUI keeps full git functionality. Thanks @jin89ho. (#5692) - **The agent's closing summary now shows when it hits the iteration limit.** When a turn exhausted its tool/iteration budget, the agent produces a graceful closing message (a summary of what it got done, or a fallback like "I reached the iteration limit…"), but the WebUI showed a bare `tool_limit_reached` error card instead — most visible with reasoning-only models (extended-thinking Claude, DeepSeek, Codex Responses) whose summary arrives empty. The closing text is now surfaced as a proper assistant message. Purely additive and guarded — it never fires on a turn that already produced a final answer, never double-injects, and degrades to the old behavior when no fallback text is available. Thanks @claw-io (co-authored by @b3nw). (#5717, #5494) diff --git a/api/routes.py b/api/routes.py index a22ad08e..3e5ae1bc 100644 --- a/api/routes.py +++ b/api/routes.py @@ -528,6 +528,11 @@ def _session_visible_to_active_profile(session_profile, handler=None) -> bool: def _request_session_visibility_exempt(method: str, path: str | None) -> bool: if not path: return False + if method == "GET" and path == "/api/session": + # Detail-load owns profile mismatch handling so the frontend can switch + # to the session's profile instead of treating a valid cross-profile + # deep link as a deleted/stale session. + return True if method != "POST": return False # Import routes create/claim sessions before normal ownership exists, and @@ -11730,6 +11735,20 @@ def handle_get(handler, parsed) -> bool: s = get_session(sid, metadata_only=(not load_messages)) _session_profile = getattr(s, 'profile', None) or None if not _session_visible_to_active_profile(_session_profile, handler): + if _session_profile: + # Valid session owned by a KNOWN other profile: 409 so the + # client can offer to switch to it (#5419). + return j(handler, { + "error": "Session belongs to a different profile", + "code": "session_profile_mismatch", + "session_id": sid, + "profile": _session_profile, + }, status=409) + # Unknown/legacy None-profile sidecar: keep the original 404 so + # the frontend's self-heal (clear stale URL + localStorage) still + # fires. _profiles_match coerces None->'default', so a truly + # missing/legacy session under a non-default active profile would + # otherwise emit a useless 409 with profile=null. return bad(handler, "Session not found", 404) original_stream_id = getattr(s, "active_stream_id", None) _clear_stale_stream_state(s) @@ -12061,6 +12080,20 @@ def handle_get(handler, parsed) -> bool: cli_meta = _lookup_cli_session_metadata(sid) _session_profile = (cli_meta or {}).get("profile") or None if not _session_visible_to_active_profile(_session_profile, handler): + if _session_profile: + # Valid CLI/foreign session owned by a KNOWN other profile: + # 409 so the client can offer to switch to it (#5419). + return j(handler, { + "error": "Session belongs to a different profile", + "code": "session_profile_mismatch", + "session_id": sid, + "profile": _session_profile, + }, status=409) + # Missing session (cli_meta={} -> profile=None): keep the 404 + # self-heal path. _profiles_match coerces None->'default', so a + # truly-missing session under a non-default active profile would + # otherwise emit a useless 409 with profile=null and skip the + # frontend self-heal + spin the SSE reconnect against a dead sid. return bad(handler, "Session not found", 404) synth, reason = _claim_or_synthesize_cli_session(sid, cli_meta=cli_meta or {}) if reason == "was_webui": diff --git a/static/sessions.js b/static/sessions.js index b4726c53..71092b2e 100644 --- a/static/sessions.js +++ b/static/sessions.js @@ -1299,6 +1299,51 @@ function _rearmActiveSessionStream(){ if(activeSid) startSessionStream(activeSid); } +function _sessionProfileMismatchFromError(e){ + if(!e || e.status!==409 || !e.body) return null; + try{ + const body=JSON.parse(e.body); + if(body && body.code==='session_profile_mismatch' && body.profile){ + return {profile:String(body.profile), session_id:String(body.session_id||'')}; + } + }catch(_){ } + return null; +} + +async function _switchProfileForSessionLoad(profile){ + const name=String(profile||'').trim(); + if(!name) throw new Error('missing profile'); + if(name===S.activeProfile) return; + if(typeof _invalidateSessionListRenders==='function') _invalidateSessionListRenders(); + if(typeof _setProfileSwitchListEmbargo==='function') _setProfileSwitchListEmbargo(true); + if(typeof showSessionListSkeleton==='function') showSessionListSkeleton(name); + try{ + const data=await api('/api/profile/switch',{method:'POST',body:JSON.stringify({name}),timeoutToast:false}); + S.activeProfile=data.active||name; + S.activeProfileIsDefault=!!data.is_default; + if(typeof _clearPersistedModelState==='function') _clearPersistedModelState(); + else localStorage.removeItem('hermes-webui-model'); + if(data.default_model) window._defaultModel=data.default_model; + if(data.default_model_provider) window._activeProvider=data.default_model_provider; + if(typeof startGatewaySSE==='function') startGatewaySSE(); + if(typeof syncTopbar==='function') syncTopbar(); + if(typeof _setProfileSwitchListEmbargo==='function') _setProfileSwitchListEmbargo(false); + if(typeof renderSessionList==='function') await renderSessionList(); + }catch(switchErr){ + // The switch POST failed, so we're still on the previous profile and its + // caches are intact. Clear the up-front skeleton and re-render the real + // list so the sidebar doesn't strand on the skeleton (the #4671 strand bug + // — _sessionListSkeletonActive hard-gates renderSessionListFromCache + the + // SSE/poll repaints until an unrelated full render fires). Mirror the + // canonical switch's catch in panels.js, then rethrow so loadSession's + // catch(switchErr) still routes into the generic error handler. + if(typeof _setProfileSwitchListEmbargo==='function') _setProfileSwitchListEmbargo(false); + _sessionListSkeletonActive=false; + if(typeof renderSessionListFromCache==='function') renderSessionListFromCache(); + throw switchErr; + } +} + async function loadSession(sid){ const opts = arguments[1] || {}; if(!opts.skipLineageResolve && typeof _resolveSessionIdFromSidebarLineage==='function'){ @@ -1428,6 +1473,30 @@ async function loadSession(sid){ try { data = await api(`/api/session?session_id=${encodeURIComponent(sid)}&messages=0&resolve_model=0`); } catch(e) { + const profileMismatch=_sessionProfileMismatchFromError(e); + if(profileMismatch && profileMismatch.profile && !opts.skipProfileResolve){ + if (_loadingSessionId !== sid) { + _rearmActiveSessionStream(); + return; + } + try{ + if(typeof showToast==='function') showToast(`Switching to ${profileMismatch.profile} profile for this session…`,2200); + await _switchProfileForSessionLoad(profileMismatch.profile); + // Post-await stale-load guard (Codex): the profile switch above does a + // network POST + session-list re-render, during which the user may have + // navigated to a different session. If we no longer own the load, bail + // before clearing _loadingSessionId or retrying so the stale + // continuation can't hijack the UI back to the old target. + if (_loadingSessionId !== sid) { + _rearmActiveSessionStream(); + return; + } + if (_loadingSessionId === sid) _loadingSessionId = null; + return loadSession(sid,{...opts,skipProfileResolve:true,force:true}); + }catch(switchErr){ + e=switchErr; + } + } const _msgInner = $('msgInner'); // Stale-load guard (Codex): a newer loadSession() may have started while this // request was awaiting (e.g. the user clicked a healthy session during a diff --git a/tests/test_bugfix_sweep.py b/tests/test_bugfix_sweep.py index c1fc6dba..039cf30a 100644 --- a/tests/test_bugfix_sweep.py +++ b/tests/test_bugfix_sweep.py @@ -141,6 +141,17 @@ def test_session_url_builder_strips_legacy_session_query_alias(): assert "current.searchParams.delete('session_id');" in helper +def test_cross_profile_session_deep_links_switch_profile_instead_of_self_healing(): + routes = (ROOT / "api" / "routes.py").read_text(encoding="utf-8") + sessions = (ROOT / "static" / "sessions.js").read_text(encoding="utf-8") + + assert '"code": "session_profile_mismatch"' in routes + assert 'if method == "GET" and path == "/api/session":' in routes + assert "function _sessionProfileMismatchFromError" in sessions + assert "_switchProfileForSessionLoad(profileMismatch.profile)" in sessions + assert "skipProfileResolve:true" in sessions + + def test_service_worker_precaches_same_origin_vendor_shell_assets(): sw = (ROOT / "static" / "sw.js").read_text(encoding="utf-8") diff --git a/tests/test_issue1611_session_profile_filtering.py b/tests/test_issue1611_session_profile_filtering.py index 01fc0f35..63071a64 100644 --- a/tests/test_issue1611_session_profile_filtering.py +++ b/tests/test_issue1611_session_profile_filtering.py @@ -323,6 +323,37 @@ class _ProfileScopedSession: } +# Keys the profile-mismatch 409 envelope is ALLOWED to contain. Any key beyond +# these would mean session content is leaking across the profile boundary. +_ALLOWED_MISMATCH_KEYS = {"error", "code", "session_id", "profile"} + + +def _assert_profile_mismatch_envelope(captured, session_id, profile, *, leak_msg): + """#5419: a valid-but-wrong-profile /api/session load now returns a + structured 409 ``session_profile_mismatch`` envelope (so the frontend can + switch to the owning profile) instead of a misleading 404. This asserts the + new contract WHILE preserving the isolation guarantee this suite exists to + protect: the response body must carry ONLY the error envelope — never any + transcript/messages/title/content from the foreign-profile session. + """ + assert "bad" not in captured, ( + "wrong-profile session should no longer 404 via bad(); expected the 409 envelope" + ) + entry = captured.get("json") + assert entry is not None, "expected a structured 409 profile-mismatch response" + assert entry.get("status") == 409, f"expected status 409, got {entry.get('status')}" + data = entry.get("data") or {} + assert data.get("code") == "session_profile_mismatch" + assert data.get("profile") == profile + assert data.get("session_id") == session_id + assert "error" in data + # Boundary guard: no foreign-profile content may ride along in the envelope. + extra = set(data.keys()) - _ALLOWED_MISMATCH_KEYS + assert not extra, f"{leak_msg} (unexpected keys leaked: {sorted(extra)})" + for forbidden in ("messages", "content", "title", "workspace", "model", "tool_calls"): + assert forbidden not in data, f"{leak_msg} ('{forbidden}' present in envelope)" + + def test_get_session_rejects_session_from_inactive_profile(): """A known session_id from another profile must not bypass /api/sessions scoping. @@ -352,8 +383,12 @@ def test_get_session_rejects_session_from_inactive_profile(): patch("api.routes.j", side_effect=fake_j): routes.handle_get(SimpleNamespace(headers={"Cookie": "hermes_profile=default"}), parsed) - assert captured.get("bad", {}).get("status") == 404 - assert "json" not in captured, "foreign-profile transcript must not be returned" + # #5419: a valid-but-wrong-profile session now returns a structured 409 + # (session_profile_mismatch) so the frontend can switch profiles, instead + # of a misleading 404. The isolation boundary this suite protects still + # holds: the response carries ONLY the error envelope, never any transcript. + _assert_profile_mismatch_envelope(captured, "foreign_001", "other", + leak_msg="foreign-profile transcript must not be returned") def test_get_session_rejects_metadata_only_session_from_inactive_profile(): @@ -377,8 +412,8 @@ def test_get_session_rejects_metadata_only_session_from_inactive_profile(): patch("api.routes.j", side_effect=fake_j): routes.handle_get(SimpleNamespace(headers={"Cookie": "hermes_profile=default"}), parsed) - assert captured.get("bad", {}).get("status") == 404 - assert "json" not in captured, "foreign-profile metadata must not be returned" + _assert_profile_mismatch_envelope(captured, "foreign_001", "other", + leak_msg="foreign-profile metadata must not be returned") def test_get_session_rejects_cookieless_session_from_inactive_profile(): @@ -402,8 +437,8 @@ def test_get_session_rejects_cookieless_session_from_inactive_profile(): patch("api.routes.j", side_effect=fake_j): routes.handle_get(SimpleNamespace(headers={}), parsed) - assert captured.get("bad", {}).get("status") == 404 - assert "json" not in captured, "cookieless foreign-profile metadata must not be returned" + _assert_profile_mismatch_envelope(captured, "foreign_001", "other", + leak_msg="cookieless foreign-profile metadata must not be returned") def test_get_session_rejects_cli_session_from_inactive_profile(): @@ -430,8 +465,76 @@ def test_get_session_rejects_cli_session_from_inactive_profile(): patch("api.routes.j", side_effect=fake_j): routes.handle_get(SimpleNamespace(headers={"Cookie": "hermes_profile=default"}), parsed) - assert captured.get("bad", {}).get("status") == 404 - assert "json" not in captured, "foreign-profile CLI transcript must not be returned" + _assert_profile_mismatch_envelope(captured, "cli_foreign", "other", + leak_msg="foreign-profile CLI transcript must not be returned") + + +def test_missing_session_under_nondefault_profile_still_404_primary_branch(): + """#5419 regression (Fable Finding 1): a truly-missing/legacy session whose + owning profile is UNKNOWN (None) must keep the 404 self-heal path even when + the active profile is non-default — NOT emit a useless 409 with profile=null. + + _profiles_match coerces a None row-profile to 'default', so visibility fails + against a non-default active profile; the fix must fall back to 404 (not 409) + when _session_profile is falsy so the frontend self-heal + empty-state still + fire (and it doesn't spin the SSE reconnect against a dead session id). + """ + import api.routes as routes + + captured = {} + + def fake_bad(_handler, message, status=400, **_kwargs): + captured["bad"] = {"message": message, "status": status} + return captured["bad"] + + def fake_j(_handler, data, status=200, **_kwargs): + captured["json"] = {"data": data, "status": status} + return captured["json"] + + # None-profile sidecar (legacy/missing) + a NON-DEFAULT active profile. + parsed = urlparse("/api/session?session_id=ghost_001&messages=0&resolve_model=0") + with patch("api.routes._get_active_profile_name", return_value="research"), \ + patch("api.routes.get_session", return_value=_ProfileScopedSession(session_id="ghost_001", profile=None)), \ + patch("api.routes._lookup_cli_session_metadata", return_value={}), \ + patch("api.routes.bad", side_effect=fake_bad), \ + patch("api.routes.j", side_effect=fake_j): + routes.handle_get(SimpleNamespace(headers={"Cookie": "hermes_profile=research"}), parsed) + + assert captured.get("bad", {}).get("status") == 404, ( + "unknown-profile (None) session must 404 for self-heal, not a profile=null 409" + ) + assert "json" not in captured, "must not emit a 409 envelope for an unknown-profile session" + + +def test_missing_session_under_nondefault_profile_still_404_cli_branch(): + """#5419 regression (Fable Finding 1), CLI/foreign fallback branch: a truly + missing session (cli_meta={} -> profile=None) under a non-default active + profile must keep the 404 self-heal, not a profile=null 409.""" + import api.routes as routes + + captured = {} + + def fake_bad(_handler, message, status=400, **_kwargs): + captured["bad"] = {"message": message, "status": status} + return captured["bad"] + + def fake_j(_handler, data, status=200, **_kwargs): + captured["json"] = {"data": data, "status": status} + return captured["json"] + + parsed = urlparse("/api/session?session_id=ghost_cli&messages=1&resolve_model=0") + with patch("api.routes._get_active_profile_name", return_value="research"), \ + patch("api.routes.get_session", side_effect=KeyError), \ + patch("api.routes.SESSION_INDEX_FILE", SimpleNamespace(exists=lambda: False)), \ + patch("api.routes._lookup_cli_session_metadata", return_value={}), \ + patch("api.routes.bad", side_effect=fake_bad), \ + patch("api.routes.j", side_effect=fake_j): + routes.handle_get(SimpleNamespace(headers={"Cookie": "hermes_profile=research"}), parsed) + + assert captured.get("bad", {}).get("status") == 404, ( + "missing CLI session must 404 for self-heal, not a profile=null 409" + ) + assert "json" not in captured, "must not emit a 409 envelope for a missing CLI session" # ── Direct session export must also honor active profile ─────────────────