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 <nesquena-hermes@users.noreply.github.com>
This commit is contained in:
1 parent
243c0f698d
commit
407451977e
5 files changed
+226
-8
No files matched your search
@@ -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)
|
||||
|
||||
@@ -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":
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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 ─────────────────
|
||||
|
||||
Reference in new issue
Block a user