mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-07-20 22:51:07 +00:00
Merge branch 'master' into docs/roadmap-full-refresh
This commit is contained in:
@@ -5,6 +5,14 @@
|
||||
|
||||
### 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)
|
||||
|
||||
- **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
|
||||
|
||||
@@ -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":
|
||||
|
||||
@@ -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,24 @@ 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 the
|
||||
# downstream _all_result_messages checks (silent-failure detection
|
||||
# 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):
|
||||
result = {**result, 'messages': _result_messages}
|
||||
if cancel_event.is_set():
|
||||
_finalize_cancelled_turn(s, ephemeral=False)
|
||||
try:
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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
|
||||
|
||||
+14
-1
@@ -17666,7 +17666,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)
|
||||
@@ -17685,6 +17696,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;
|
||||
|
||||
@@ -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")
|
||||
|
||||
|
||||
@@ -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 ─────────────────
|
||||
|
||||
@@ -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"
|
||||
)
|
||||
@@ -280,6 +280,132 @@ 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 — 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 = {
|
||||
"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.
|
||||
# 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"
|
||||
], "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
|
||||
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 = {
|
||||
"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",
|
||||
|
||||
@@ -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()"
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user