From 407451977e92189fa24c30903d902d9aa3fc38cd Mon Sep 17 00:00:00 2001 From: Daniel Harcek Date: Tue, 7 Jul 2026 01:58:03 +0000 Subject: [PATCH] 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 ─────────────────