Compare commits
5 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
f6e1612c7e | ||
|
|
081c4208d9 | ||
|
|
e05fc4e0e4 | ||
|
|
312a493a72 | ||
|
|
3246b263d9 |
20
CHANGELOG.md
20
CHANGELOG.md
@@ -1,5 +1,25 @@
|
||||
# Hermes Web UI -- Changelog
|
||||
|
||||
## [v0.50.132] — 2026-04-21
|
||||
|
||||
### Fixed
|
||||
- **Periodic session checkpoint during long-running agent tasks** — messages accumulated during multi-step research or coding tasks were silently lost if the server restarted mid-run. The root cause: `Session.save()` was only called after `agent.run_conversation()` completed. The fix adds a daemon thread that saves the session every 15 seconds whenever the `on_tool` callback signals a completed tool call — the first reliable mid-run signal that real progress has been made (the agent works on an internal copy of `s.messages`, so watching message-count would never trigger). `Session.save()` gains a `skip_index=True` flag so checkpoints skip the expensive index rebuild; the final `s.save()` at task completion still rebuilds it. On a server restart the user's message and turn bookkeeping remain on disk — worst case: up to 15 seconds of tool-call progress lost rather than the entire conversation turn. Closes #765. Absorbed and corrected from PR #809 by @bergeouss. (#810)
|
||||
|
||||
## [v0.50.131] — 2026-04-21
|
||||
|
||||
### Fixed
|
||||
- **Workspace pane now respects the app theme** — seven hardcoded dark-mode `rgba(255,255,255,...)` colors in the workspace panel CSS have been replaced with theme-aware CSS variables (`--hover-bg`, `--border2`, `--code-inline-bg`). The file list hover, panel icon buttons, preview table rows, and the preview edit textarea now all update correctly when switching between light and dark themes. Reported in #786. (#807)
|
||||
|
||||
## [v0.50.130] — 2026-04-21
|
||||
|
||||
### Fixed
|
||||
- **New sessions now appear immediately in the sidebar** — the zero-message Untitled filter now exempts sessions younger than 60 seconds, so clicking New Chat shows the session right away instead of waiting for the first message. Sessions older than 60 seconds that are still Untitled with 0 messages continue to be suppressed (ghost sessions from test runs / accidental page reloads). Addresses Bug A only of #789; Bug B (SSE refetch resetting sidebar mid-interaction) is a separate fix. (#806)
|
||||
|
||||
## [v0.50.129] — 2026-04-21
|
||||
|
||||
### Fixed
|
||||
- **Profile isolation: complete fix via cookie + thread-local context** — PR #800 (v0.50.127) only fixed `POST /api/session/new`. `GET /api/profile/active` still read the process-level `_active_profile` global, so a page refresh while another client had a different profile active would corrupt `S.activeProfile` in JS, defeating the session-creation fix on the next new chat. This release completes the isolation: profile switches now set a `hermes_profile` cookie (HttpOnly, SameSite=Lax) and never mutate the process global. Every request handler reads the cookie into a thread-local; all server functions (`get_active_profile_name()`, `get_active_hermes_home()`, `list_profiles_api()`, memory endpoints, model loading) automatically see the per-client profile. `switch_profile()` gains a `process_wide` kwarg — the HTTP route passes `False`, keeping the global clean; CLI callers default to `True` (unchanged behaviour). Absorbed from PR #803 by @bergeouss with correctness fixes reviewed by Opus. (#805)
|
||||
|
||||
## [v0.50.128] — 2026-04-21
|
||||
|
||||
### Fixed
|
||||
|
||||
@@ -54,14 +54,21 @@ def _security_headers(handler):
|
||||
)
|
||||
|
||||
|
||||
def j(handler, payload, status: int=200) -> None:
|
||||
"""Send a JSON response."""
|
||||
def j(handler, payload, status: int=200, extra_headers: dict=None) -> None:
|
||||
"""Send a JSON response.
|
||||
|
||||
*extra_headers*: optional dict of additional headers to include
|
||||
(e.g., {'Set-Cookie': '...'}). Headers are sent before end_headers().
|
||||
"""
|
||||
body = _json.dumps(payload, ensure_ascii=False, indent=2).encode('utf-8')
|
||||
handler.send_response(status)
|
||||
handler.send_header('Content-Type', 'application/json; charset=utf-8')
|
||||
handler.send_header('Content-Length', str(len(body)))
|
||||
handler.send_header('Cache-Control', 'no-store')
|
||||
_security_headers(handler)
|
||||
if extra_headers:
|
||||
for k, v in extra_headers.items():
|
||||
handler.send_header(k, v)
|
||||
handler.end_headers()
|
||||
handler.wfile.write(body)
|
||||
|
||||
@@ -173,3 +180,48 @@ def read_body(handler) -> dict:
|
||||
return _json.loads(raw)
|
||||
except Exception:
|
||||
return {}
|
||||
|
||||
|
||||
# ── Profile cookie helpers (issue #798) ─────────────────────────────────────
|
||||
|
||||
PROFILE_COOKIE_NAME = 'hermes_profile'
|
||||
|
||||
|
||||
def get_profile_cookie(handler) -> str | None:
|
||||
"""Extract the hermes_profile cookie value from the request, or None."""
|
||||
cookie_header = handler.headers.get('Cookie', '')
|
||||
if not cookie_header:
|
||||
return None
|
||||
import http.cookies as _hc
|
||||
cookie = _hc.SimpleCookie()
|
||||
try:
|
||||
cookie.load(cookie_header)
|
||||
except _hc.CookieError:
|
||||
return None
|
||||
morsel = cookie.get(PROFILE_COOKIE_NAME)
|
||||
if morsel and morsel.value:
|
||||
# Validate against profile-name pattern before trusting
|
||||
from api.profiles import _PROFILE_ID_RE
|
||||
val = morsel.value
|
||||
if val == 'default' or _PROFILE_ID_RE.fullmatch(val):
|
||||
return val
|
||||
return None
|
||||
|
||||
|
||||
def build_profile_cookie(name: str) -> str:
|
||||
"""Build a Set-Cookie header value for the hermes_profile cookie.
|
||||
|
||||
name='default' clears the cookie (max-age=0).
|
||||
Any other valid profile name sets it for the browser session.
|
||||
httponly=True: the JS reads profile from /api/profile/active JSON, never
|
||||
from document.cookie, so httponly exposure is unnecessary.
|
||||
"""
|
||||
import http.cookies as _hc
|
||||
cookie = _hc.SimpleCookie()
|
||||
cookie[PROFILE_COOKIE_NAME] = '' if name == 'default' else name
|
||||
cookie[PROFILE_COOKIE_NAME]['path'] = '/'
|
||||
cookie[PROFILE_COOKIE_NAME]['httponly'] = True
|
||||
cookie[PROFILE_COOKIE_NAME]['samesite'] = 'Lax'
|
||||
if name == 'default':
|
||||
cookie[PROFILE_COOKIE_NAME]['max-age'] = '0'
|
||||
return cookie[PROFILE_COOKIE_NAME].OutputString()
|
||||
|
||||
@@ -121,14 +121,15 @@ class Session:
|
||||
def path(self):
|
||||
return SESSION_DIR / f'{self.session_id}.json'
|
||||
|
||||
def save(self, touch_updated_at: bool = True) -> None:
|
||||
def save(self, touch_updated_at: bool = True, skip_index: bool = False) -> None:
|
||||
if touch_updated_at:
|
||||
self.updated_at = time.time()
|
||||
self.path.write_text(
|
||||
json.dumps(self.__dict__, ensure_ascii=False, indent=2),
|
||||
encoding='utf-8',
|
||||
)
|
||||
_write_session_index(updates=[self])
|
||||
if not skip_index:
|
||||
_write_session_index(updates=[self])
|
||||
|
||||
@classmethod
|
||||
def load(cls, sid):
|
||||
@@ -218,7 +219,13 @@ def all_sessions():
|
||||
index_map[s.session_id] = s.compact()
|
||||
result = sorted(index_map.values(), key=lambda s: (s.get('pinned', False), s['updated_at']), reverse=True)
|
||||
# Hide empty Untitled sessions from the UI (created by tests, page refreshes, etc.)
|
||||
result = [s for s in result if not (s.get('title','Untitled')=='Untitled' and s.get('message_count',0)==0)]
|
||||
# Exempt sessions younger than 60 s so a brand-new session stays visible (#789)
|
||||
_now = time.time()
|
||||
result = [s for s in result if not (
|
||||
s.get('title', 'Untitled') == 'Untitled'
|
||||
and s.get('message_count', 0) == 0
|
||||
and (_now - s.get('updated_at', _now)) > 60
|
||||
)]
|
||||
# Backfill: sessions created before Sprint 22 have no profile tag.
|
||||
# Attribute them to 'default' so the client profile filter works correctly.
|
||||
for s in result:
|
||||
@@ -239,7 +246,12 @@ def all_sessions():
|
||||
for s in SESSIONS.values():
|
||||
if all(s.session_id != x.session_id for x in out): out.append(s)
|
||||
out.sort(key=lambda s: (getattr(s, 'pinned', False), s.updated_at), reverse=True)
|
||||
result = [s.compact() for s in out if not (s.title=='Untitled' and len(s.messages)==0)]
|
||||
_now = time.time()
|
||||
result = [s.compact() for s in out if not (
|
||||
s.title == 'Untitled'
|
||||
and len(s.messages) == 0
|
||||
and (_now - s.updated_at) > 60
|
||||
)]
|
||||
for s in result:
|
||||
if not s.get('profile'):
|
||||
s['profile'] = 'default'
|
||||
|
||||
101
api/profiles.py
101
api/profiles.py
@@ -31,6 +31,12 @@ _active_profile = 'default'
|
||||
_profile_lock = threading.Lock()
|
||||
_loaded_profile_env_keys: set[str] = set()
|
||||
|
||||
# Thread-local profile context: set per-request by server.py, cleared after.
|
||||
# Enables per-client profile isolation (issue #798) — each HTTP request thread
|
||||
# reads its own profile from the hermes_profile cookie instead of the
|
||||
# process-global _active_profile.
|
||||
_tls = threading.local()
|
||||
|
||||
def _resolve_base_hermes_home() -> Path:
|
||||
"""Return the BASE ~/.hermes directory — the root that contains profiles/.
|
||||
|
||||
@@ -86,15 +92,47 @@ def _read_active_profile_file() -> str:
|
||||
# ── Public API ──────────────────────────────────────────────────────────────
|
||||
|
||||
def get_active_profile_name() -> str:
|
||||
"""Return the currently active profile name."""
|
||||
"""Return the currently active profile name.
|
||||
|
||||
Priority:
|
||||
1. Thread-local (set per-request from hermes_profile cookie) — issue #798
|
||||
2. Process-level default (_active_profile)
|
||||
"""
|
||||
tls_name = getattr(_tls, 'profile', None)
|
||||
if tls_name is not None:
|
||||
return tls_name
|
||||
return _active_profile
|
||||
|
||||
|
||||
def set_request_profile(name: str) -> None:
|
||||
"""Set the per-request profile context for this thread.
|
||||
|
||||
Called by server.py at the start of each request when a hermes_profile
|
||||
cookie is present. Always paired with clear_request_profile() in a
|
||||
finally block so the thread-local is released after the request.
|
||||
"""
|
||||
_tls.profile = name
|
||||
|
||||
|
||||
def clear_request_profile() -> None:
|
||||
"""Clear the per-request profile context for this thread.
|
||||
|
||||
Called by server.py in the finally block of do_GET / do_POST.
|
||||
Safe to call even if set_request_profile() was never called.
|
||||
"""
|
||||
_tls.profile = None
|
||||
|
||||
|
||||
def get_active_hermes_home() -> Path:
|
||||
"""Return the HERMES_HOME path for the currently active profile."""
|
||||
if _active_profile == 'default':
|
||||
"""Return the HERMES_HOME path for the currently active profile.
|
||||
|
||||
Uses get_active_profile_name() so per-request TLS context (issue #798)
|
||||
is respected, not just the process-level global.
|
||||
"""
|
||||
name = get_active_profile_name()
|
||||
if name == 'default':
|
||||
return _DEFAULT_HERMES_HOME
|
||||
profile_dir = _DEFAULT_HERMES_HOME / 'profiles' / _active_profile
|
||||
profile_dir = _DEFAULT_HERMES_HOME / 'profiles' / name
|
||||
if profile_dir.is_dir():
|
||||
return profile_dir
|
||||
return _DEFAULT_HERMES_HOME
|
||||
@@ -190,12 +228,18 @@ def init_profile_state() -> None:
|
||||
_reload_dotenv(home)
|
||||
|
||||
|
||||
def switch_profile(name: str) -> dict:
|
||||
def switch_profile(name: str, *, process_wide: bool = True) -> dict:
|
||||
"""Switch the active profile.
|
||||
|
||||
Validates the profile exists, updates process state, patches module caches,
|
||||
reloads .env, and reloads config.yaml.
|
||||
|
||||
Args:
|
||||
name: Profile name to switch to.
|
||||
process_wide: If True (default), updates the process-global
|
||||
_active_profile. Set to False for per-client switches from the
|
||||
WebUI where the profile is managed via cookie + thread-local (#798).
|
||||
|
||||
Returns: {'profiles': [...], 'active': name}
|
||||
Raises ValueError if profile doesn't exist or agent is busy.
|
||||
"""
|
||||
@@ -221,24 +265,41 @@ def switch_profile(name: str) -> dict:
|
||||
raise ValueError(f"Profile '{name}' does not exist.")
|
||||
|
||||
with _profile_lock:
|
||||
_active_profile = name
|
||||
_set_hermes_home(home)
|
||||
_reload_dotenv(home)
|
||||
if process_wide:
|
||||
global _active_profile
|
||||
_active_profile = name
|
||||
_set_hermes_home(home)
|
||||
_reload_dotenv(home)
|
||||
|
||||
# Write sticky default for CLI consistency
|
||||
try:
|
||||
ap_file = _DEFAULT_HERMES_HOME / 'active_profile'
|
||||
ap_file.write_text(name if name != 'default' else '', encoding='utf-8')
|
||||
except Exception:
|
||||
logger.debug("Failed to write active profile file")
|
||||
if process_wide:
|
||||
# Write sticky default for CLI consistency
|
||||
try:
|
||||
ap_file = _DEFAULT_HERMES_HOME / 'active_profile'
|
||||
ap_file.write_text(name if name != 'default' else '', encoding='utf-8')
|
||||
except Exception:
|
||||
logger.debug("Failed to write active profile file")
|
||||
|
||||
# Reload config.yaml from the new profile
|
||||
reload_config()
|
||||
# Reload config.yaml from the new profile
|
||||
reload_config()
|
||||
|
||||
# Return profile-specific defaults so frontend can apply them
|
||||
# Return profile-specific defaults so frontend can apply them.
|
||||
# For process_wide=False (per-client switch), read the target profile's
|
||||
# config.yaml directly from disk rather than from _cfg_cache (process-global),
|
||||
# since reload_config() was intentionally skipped.
|
||||
from api.workspace import get_last_workspace
|
||||
from api.config import get_config
|
||||
cfg = get_config()
|
||||
if process_wide:
|
||||
from api.config import get_config
|
||||
cfg = get_config()
|
||||
else:
|
||||
# Direct disk read — does not touch _cfg_cache
|
||||
try:
|
||||
import yaml as _yaml
|
||||
cfg_path = home / 'config.yaml'
|
||||
cfg = _yaml.safe_load(cfg_path.read_text(encoding='utf-8')) if cfg_path.exists() else {}
|
||||
if not isinstance(cfg, dict):
|
||||
cfg = {}
|
||||
except Exception:
|
||||
cfg = {}
|
||||
model_cfg = cfg.get('model', {})
|
||||
default_model = None
|
||||
if isinstance(model_cfg, str):
|
||||
@@ -263,7 +324,7 @@ def list_profiles_api() -> list:
|
||||
# hermes_cli not available -- return just the default
|
||||
return [_default_profile_dict()]
|
||||
|
||||
active = _active_profile
|
||||
active = get_active_profile_name()
|
||||
result = []
|
||||
for p in infos:
|
||||
result.append({
|
||||
|
||||
@@ -1153,11 +1153,15 @@ def handle_post(handler, parsed) -> bool:
|
||||
return bad(handler, "name is required")
|
||||
try:
|
||||
from api.profiles import switch_profile, _validate_profile_name
|
||||
|
||||
from api.helpers import build_profile_cookie
|
||||
if name != 'default':
|
||||
_validate_profile_name(name)
|
||||
result = switch_profile(name)
|
||||
return j(handler, result)
|
||||
# process_wide=False: don't mutate the process-global _active_profile.
|
||||
# Per-client profile is managed via cookie + thread-local (#798).
|
||||
result = switch_profile(name, process_wide=False)
|
||||
return j(handler, result, extra_headers={
|
||||
'Set-Cookie': build_profile_cookie(name),
|
||||
})
|
||||
except (ValueError, FileNotFoundError) as e:
|
||||
return bad(handler, _sanitize_error(e), 404)
|
||||
except RuntimeError as e:
|
||||
|
||||
@@ -822,6 +822,10 @@ def _run_agent_streaming(session_id, msg_text, model, workspace, stream_id, atta
|
||||
except Exception:
|
||||
logger.debug("Failed to put event to queue")
|
||||
|
||||
# Initialised here (before any code that may raise) so the outer `finally`
|
||||
# block can safely check `if _checkpoint_stop is not None` even when an
|
||||
# exception fires before the checkpoint thread is created (Issue #765).
|
||||
_checkpoint_stop = None
|
||||
try:
|
||||
s = get_session(session_id)
|
||||
s.workspace = str(Path(workspace).expanduser().resolve())
|
||||
@@ -1025,6 +1029,9 @@ def _run_agent_streaming(session_id, msg_text, model, workspace, stream_id, atta
|
||||
live_tc['duration'] = cb_kwargs.get('duration')
|
||||
live_tc['is_error'] = bool(cb_kwargs.get('is_error', False))
|
||||
break
|
||||
# Signal the checkpoint thread that new work has completed (Issue #765).
|
||||
# Each completed tool call is a meaningful unit of progress worth persisting.
|
||||
_checkpoint_activity[0] += 1
|
||||
put('tool_complete', {
|
||||
'event_type': event_type,
|
||||
'name': name,
|
||||
@@ -1174,6 +1181,40 @@ def _run_agent_streaming(session_id, msg_text, model, workspace, stream_id, atta
|
||||
if _personality_prompt:
|
||||
agent.ephemeral_system_prompt = _personality_prompt
|
||||
_previous_messages = list(s.messages or [])
|
||||
|
||||
# ── Periodic checkpoint during streaming (Issue #765) ──
|
||||
# The agent works on an internal copy of s.messages during run_conversation()
|
||||
# so we cannot watch s.messages for growth. Instead, on_tool() increments
|
||||
# _checkpoint_activity[0] each time a tool call completes — that is the real
|
||||
# signal that progress has been made worth persisting.
|
||||
#
|
||||
# What gets saved on each checkpoint:
|
||||
# - s.pending_user_message (already written before run starts)
|
||||
# - s.pending_started_at / s.active_stream_id (turn bookkeeping)
|
||||
# On a server restart the UI will see a session with a pending message and no
|
||||
# response — better than a silent loss of the entire conversation turn.
|
||||
# The final s.save() at task completion handles the full session update + index.
|
||||
# (_checkpoint_stop is pre-initialised at the top of the outer try.)
|
||||
_checkpoint_activity = [0]
|
||||
|
||||
def _periodic_checkpoint():
|
||||
last_saved_activity = 0
|
||||
while not _checkpoint_stop.wait(15):
|
||||
try:
|
||||
cur = _checkpoint_activity[0]
|
||||
if cur > last_saved_activity:
|
||||
s.save(skip_index=True)
|
||||
last_saved_activity = cur
|
||||
except Exception as e:
|
||||
logger.debug("Periodic checkpoint save failed: %s", e)
|
||||
|
||||
_checkpoint_stop = threading.Event()
|
||||
_ckpt_thread = threading.Thread(
|
||||
target=_periodic_checkpoint, daemon=True,
|
||||
name=f"ckpt-{session_id[:8]}",
|
||||
)
|
||||
_ckpt_thread.start()
|
||||
|
||||
result = agent.run_conversation(
|
||||
user_message=workspace_ctx + msg_text,
|
||||
system_message=workspace_system_msg,
|
||||
@@ -1495,6 +1536,9 @@ def _run_agent_streaming(session_id, msg_text, model, workspace, stream_id, atta
|
||||
_apperror_payload['hint'] = _exc_hint
|
||||
put('apperror', _apperror_payload)
|
||||
finally:
|
||||
# Stop periodic checkpoint thread if it was started (Issue #765)
|
||||
if _checkpoint_stop is not None:
|
||||
_checkpoint_stop.set()
|
||||
_clear_thread_env() # TD1: always clear thread-local context
|
||||
with STREAMS_LOCK:
|
||||
STREAMS.pop(stream_id, None)
|
||||
|
||||
15
server.py
15
server.py
@@ -15,7 +15,8 @@ logger = logging.getLogger(__name__)
|
||||
|
||||
from api.auth import check_auth
|
||||
from api.config import HOST, PORT, STATE_DIR, SESSION_DIR, DEFAULT_WORKSPACE
|
||||
from api.helpers import j
|
||||
from api.helpers import j, get_profile_cookie
|
||||
from api.profiles import set_request_profile, clear_request_profile
|
||||
from api.routes import handle_get, handle_post
|
||||
from api.startup import auto_install_agent_deps, fix_credential_permissions
|
||||
from api.updates import WEBUI_VERSION
|
||||
@@ -64,6 +65,10 @@ class Handler(BaseHTTPRequestHandler):
|
||||
|
||||
def do_GET(self) -> None:
|
||||
self._req_t0 = time.time()
|
||||
# Per-request profile context from cookie (issue #798)
|
||||
cookie_profile = get_profile_cookie(self)
|
||||
if cookie_profile:
|
||||
set_request_profile(cookie_profile)
|
||||
try:
|
||||
parsed = urlparse(self.path)
|
||||
if not check_auth(self, parsed): return
|
||||
@@ -73,9 +78,15 @@ class Handler(BaseHTTPRequestHandler):
|
||||
except Exception as e:
|
||||
print(f'[webui] ERROR {self.command} {self.path}\n' + traceback.format_exc(), flush=True)
|
||||
return j(self, {'error': 'Internal server error'}, status=500)
|
||||
finally:
|
||||
clear_request_profile()
|
||||
|
||||
def do_POST(self) -> None:
|
||||
self._req_t0 = time.time()
|
||||
# Per-request profile context from cookie (issue #798)
|
||||
cookie_profile = get_profile_cookie(self)
|
||||
if cookie_profile:
|
||||
set_request_profile(cookie_profile)
|
||||
try:
|
||||
parsed = urlparse(self.path)
|
||||
if not check_auth(self, parsed): return
|
||||
@@ -85,6 +96,8 @@ class Handler(BaseHTTPRequestHandler):
|
||||
except Exception as e:
|
||||
print(f'[webui] ERROR {self.command} {self.path}\n' + traceback.format_exc(), flush=True)
|
||||
return j(self, {'error': 'Internal server error'}, status=500)
|
||||
finally:
|
||||
clear_request_profile()
|
||||
|
||||
|
||||
def main() -> None:
|
||||
|
||||
@@ -401,7 +401,7 @@
|
||||
<pre class="preview-code" id="previewCode"></pre>
|
||||
<div class="preview-img-wrap" id="previewImgWrap" style="display:none"><img class="preview-img" id="previewImg" src="" alt=""></div>
|
||||
<div class="preview-md" id="previewMd" style="display:none"></div>
|
||||
<textarea id="previewEditArea" style="display:none;flex:1;width:100%;background:var(--code-bg);color:#e2e8f0;border:1px solid var(--border2);border-radius:8px;padding:12px;font-family:'SF Mono',ui-monospace,monospace;font-size:12px;line-height:1.6;resize:none;outline:none" oninput="_previewDirty=true;updateEditBtn()"></textarea>
|
||||
<textarea id="previewEditArea" style="display:none;flex:1;width:100%;background:var(--code-bg);color:var(--pre-text);border:1px solid var(--border2);border-radius:8px;padding:12px;font-family:'SF Mono',ui-monospace,monospace;font-size:12px;line-height:1.6;resize:none;outline:none" oninput="_previewDirty=true;updateEditBtn()"></textarea>
|
||||
</div>
|
||||
</aside>
|
||||
</div>
|
||||
|
||||
@@ -577,7 +577,7 @@
|
||||
.panel-actions{display:flex;gap:4px;}
|
||||
.mobile-close-btn{display:none;}
|
||||
.panel-icon-btn{width:24px;height:24px;background:none;border:none;color:var(--muted);cursor:pointer;border-radius:5px;font-size:13px;display:flex;align-items:center;justify-content:center;transition:all .15s;}
|
||||
.panel-icon-btn:hover{background:rgba(255,255,255,.08);color:var(--text);}
|
||||
.panel-icon-btn:hover{background:var(--hover-bg);color:var(--text);}
|
||||
.panel-icon-btn:disabled{opacity:.35;cursor:not-allowed;}
|
||||
.panel-icon-btn:disabled:hover{background:none;color:var(--muted);}
|
||||
/* File row actions (shown on hover) */
|
||||
@@ -594,7 +594,7 @@
|
||||
.breadcrumb-sep{color:var(--border);margin:0 1px;font-size:11px;}
|
||||
.file-tree{flex:1;overflow-y:auto;padding:8px;}
|
||||
.file-item{display:flex;align-items:center;gap:6px;padding:6px 10px;border-radius:8px;cursor:pointer;font-size:12px;color:var(--muted);transition:all .12s;min-width:0;}
|
||||
.file-item:hover{background:rgba(255,255,255,.07);color:var(--text);}
|
||||
.file-item:hover{background:var(--hover-bg);color:var(--text);}
|
||||
.file-item.active{background:var(--accent-bg);color:var(--accent-text);}
|
||||
.file-tree-toggle{font-size:10px;color:var(--muted);flex-shrink:0;width:10px;text-align:center;line-height:1;}
|
||||
.file-item.file-empty{color:var(--muted);opacity:.5;font-style:italic;cursor:default;font-size:11px;}
|
||||
@@ -623,16 +623,16 @@
|
||||
.preview-md a{color:var(--blue);text-decoration:underline;}
|
||||
.preview-md hr{border:none;border-top:1px solid var(--border);margin:12px 0;}
|
||||
.preview-md table{border-collapse:collapse;width:100%;margin:8px 0;font-size:12px;}
|
||||
.preview-md th{background:rgba(255,255,255,.07);padding:6px 10px;text-align:left;font-weight:600;border:1px solid var(--border2);}
|
||||
.preview-md td{padding:5px 10px;border:1px solid rgba(255,255,255,.06);}
|
||||
.preview-md tr:nth-child(even){background:rgba(255,255,255,.03);}
|
||||
.preview-md th{background:var(--hover-bg);padding:6px 10px;text-align:left;font-weight:600;border:1px solid var(--border2);}
|
||||
.preview-md td{padding:5px 10px;border:1px solid var(--border2);}
|
||||
.preview-md tr:nth-child(even){background:var(--code-inline-bg);}
|
||||
/* #486: inline code inside table cells needs scaled sizing to avoid overflow/clipping */
|
||||
.preview-md td code,.preview-md th code{font-size:0.85em;padding:1px 4px;vertical-align:baseline;}
|
||||
/* File type badge in preview path bar */
|
||||
.preview-badge{display:inline-block;font-size:10px;font-weight:600;padding:2px 6px;border-radius:4px;margin-left:8px;text-transform:uppercase;letter-spacing:.06em;}
|
||||
.preview-badge.img{background:var(--accent-bg);color:var(--accent-text);}
|
||||
.preview-badge.md{background:var(--accent-bg-strong);color:var(--accent-text);}
|
||||
.preview-badge.code{background:rgba(255,255,255,.07);color:var(--muted);}
|
||||
.preview-badge.code{background:var(--hover-bg);color:var(--muted);}
|
||||
::-webkit-scrollbar{width:4px;height:4px}
|
||||
::-webkit-scrollbar-track{background:transparent}
|
||||
::-webkit-scrollbar-thumb{background:rgba(255,255,255,.1);border-radius:99px;transition:background .2s}
|
||||
|
||||
304
tests/test_issue765_streaming_persistence.py
Normal file
304
tests/test_issue765_streaming_persistence.py
Normal file
@@ -0,0 +1,304 @@
|
||||
"""
|
||||
Tests for periodic session persistence during streaming (Issue #765).
|
||||
|
||||
Validates:
|
||||
- Session.save(skip_index=True) writes the JSON file but skips the index rebuild
|
||||
- The periodic checkpoint fires when _checkpoint_activity is incremented
|
||||
(as it would be by on_tool() during real agent execution)
|
||||
- Messages stored via pending_user_message survive a simulated server restart
|
||||
"""
|
||||
import json
|
||||
import threading
|
||||
import time
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
import api.models as models
|
||||
from api.models import Session
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _isolate_session_dir(tmp_path, monkeypatch):
|
||||
"""Redirect SESSION_DIR and SESSION_INDEX_FILE to a temp directory."""
|
||||
session_dir = tmp_path / "sessions"
|
||||
session_dir.mkdir()
|
||||
index_file = session_dir / "_index.json"
|
||||
|
||||
monkeypatch.setattr(models, "SESSION_DIR", session_dir)
|
||||
monkeypatch.setattr(models, "SESSION_INDEX_FILE", index_file)
|
||||
|
||||
models.SESSIONS.clear()
|
||||
yield session_dir, index_file
|
||||
models.SESSIONS.clear()
|
||||
|
||||
|
||||
def _make_session(session_id="abc123", messages=None):
|
||||
"""Helper to create a Session with a known ID."""
|
||||
return Session(
|
||||
session_id=session_id,
|
||||
title="Test Session",
|
||||
messages=messages or [{"role": "user", "content": "hello"}],
|
||||
)
|
||||
|
||||
|
||||
class TestSaveSkipIndex:
|
||||
"""Tests for the skip_index parameter on Session.save()."""
|
||||
|
||||
def test_save_writes_json_file(self):
|
||||
"""save() always writes the session JSON file, regardless of skip_index."""
|
||||
s = _make_session("s1")
|
||||
s.save()
|
||||
assert s.path.exists()
|
||||
data = json.loads(s.path.read_text())
|
||||
assert data["session_id"] == "s1"
|
||||
assert len(data["messages"]) == 1
|
||||
|
||||
def test_save_with_skip_index_writes_json(self):
|
||||
"""save(skip_index=True) still writes the session JSON file."""
|
||||
s = _make_session("s2")
|
||||
s.save(skip_index=True)
|
||||
assert s.path.exists()
|
||||
data = json.loads(s.path.read_text())
|
||||
assert data["session_id"] == "s2"
|
||||
|
||||
def test_save_with_skip_index_skips_index_rebuild(self):
|
||||
"""save(skip_index=True) does NOT create or update the session index."""
|
||||
s = _make_session("s3")
|
||||
s.save(skip_index=True)
|
||||
index = models.SESSION_INDEX_FILE
|
||||
assert not index.exists(), "Index file should not be created with skip_index=True"
|
||||
|
||||
def test_save_without_skip_index_creates_index(self):
|
||||
"""save() (default) DOES create the session index."""
|
||||
s = _make_session("s4")
|
||||
s.save()
|
||||
index = models.SESSION_INDEX_FILE
|
||||
assert index.exists(), "Index file should be created by default save()"
|
||||
data = json.loads(index.read_text())
|
||||
sids = [e["session_id"] for e in data]
|
||||
assert "s4" in sids
|
||||
|
||||
def test_skip_index_then_full_save_updates_index(self):
|
||||
"""After skip_index saves, a full save() correctly builds the index."""
|
||||
s = _make_session("s5")
|
||||
s.messages.append({"role": "assistant", "content": "hi there"})
|
||||
s.save(skip_index=True)
|
||||
assert not models.SESSION_INDEX_FILE.exists()
|
||||
|
||||
s.messages.append({"role": "user", "content": "thanks"})
|
||||
s.save()
|
||||
assert models.SESSION_INDEX_FILE.exists()
|
||||
data = json.loads(s.path.read_text())
|
||||
assert len(data["messages"]) == 3
|
||||
|
||||
def test_skip_index_save_with_touch_updated_at_false(self):
|
||||
"""save(skip_index=True, touch_updated_at=False) preserves updated_at."""
|
||||
s = _make_session("touch1")
|
||||
original_updated_at = s.updated_at
|
||||
time.sleep(0.05)
|
||||
s.save(skip_index=True, touch_updated_at=False)
|
||||
data = json.loads(s.path.read_text())
|
||||
assert data["updated_at"] == original_updated_at
|
||||
assert not models.SESSION_INDEX_FILE.exists()
|
||||
|
||||
|
||||
class TestPeriodicCheckpoint:
|
||||
"""Tests for the periodic checkpoint mechanism during streaming.
|
||||
|
||||
The checkpoint is keyed off an activity counter (_checkpoint_activity[0]),
|
||||
incremented by on_tool() on each tool.completed event — NOT off s.messages
|
||||
which is never mutated during agent.run_conversation() (the agent copies it).
|
||||
"""
|
||||
|
||||
def test_checkpoint_fires_on_activity_counter_increment(self):
|
||||
"""Checkpoint saves when _checkpoint_activity counter grows."""
|
||||
s = _make_session("ckpt1")
|
||||
s.pending_user_message = "do a long task"
|
||||
s.save() # initial save (like routes.py does before streaming starts)
|
||||
|
||||
stop_event = threading.Event()
|
||||
_checkpoint_activity = [0]
|
||||
save_count = [0]
|
||||
|
||||
def periodic_checkpoint():
|
||||
last = 0
|
||||
while not stop_event.wait(0.1): # fast interval for test
|
||||
try:
|
||||
cur = _checkpoint_activity[0]
|
||||
if cur > last:
|
||||
s.save(skip_index=True)
|
||||
last = cur
|
||||
save_count[0] += 1
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
t = threading.Thread(target=periodic_checkpoint, daemon=True)
|
||||
t.start()
|
||||
|
||||
# Simulate on_tool() completing twice (as would happen during a real agent run)
|
||||
time.sleep(0.15)
|
||||
_checkpoint_activity[0] += 1 # first tool completes
|
||||
time.sleep(0.25)
|
||||
_checkpoint_activity[0] += 1 # second tool completes
|
||||
time.sleep(0.25)
|
||||
|
||||
stop_event.set()
|
||||
t.join(timeout=2)
|
||||
|
||||
assert save_count[0] >= 2, (
|
||||
"Expected at least 2 checkpoint saves (one per activity increment); "
|
||||
f"got {save_count[0]}"
|
||||
)
|
||||
# Verify the JSON is on disk and readable
|
||||
data = json.loads(s.path.read_text())
|
||||
assert data["pending_user_message"] == "do a long task"
|
||||
|
||||
def test_checkpoint_does_not_fire_without_activity(self):
|
||||
"""Checkpoint skips save when activity counter has not changed."""
|
||||
s = _make_session("ckpt2")
|
||||
s.save()
|
||||
|
||||
stop_event = threading.Event()
|
||||
_checkpoint_activity = [0]
|
||||
save_count = [0]
|
||||
|
||||
def periodic_checkpoint():
|
||||
last = 0
|
||||
while not stop_event.wait(0.05):
|
||||
cur = _checkpoint_activity[0]
|
||||
if cur > last:
|
||||
s.save(skip_index=True)
|
||||
last = cur
|
||||
save_count[0] += 1
|
||||
|
||||
t = threading.Thread(target=periodic_checkpoint, daemon=True)
|
||||
t.start()
|
||||
# No increments — checkpoint should stay quiet
|
||||
time.sleep(0.4)
|
||||
stop_event.set()
|
||||
t.join(timeout=2)
|
||||
|
||||
assert save_count[0] == 0, (
|
||||
f"Expected 0 saves when activity is unchanged; got {save_count[0]}"
|
||||
)
|
||||
|
||||
def test_checkpoint_stops_on_signal(self):
|
||||
"""Checkpoint thread exits cleanly when stop event is set."""
|
||||
s = _make_session("ckpt3")
|
||||
stop_event = threading.Event()
|
||||
iterations = [0]
|
||||
|
||||
def periodic_checkpoint():
|
||||
while not stop_event.wait(0.02):
|
||||
iterations[0] += 1
|
||||
|
||||
t = threading.Thread(target=periodic_checkpoint, daemon=True)
|
||||
t.start()
|
||||
time.sleep(0.15)
|
||||
stop_event.set()
|
||||
t.join(timeout=1)
|
||||
assert not t.is_alive(), "Checkpoint thread should have stopped"
|
||||
|
||||
def test_pending_message_survives_simulated_restart(self):
|
||||
"""pending_user_message written before run_conversation survives a restart.
|
||||
|
||||
This is the minimal guarantee for Issue #765: even if the agent produces
|
||||
no tool calls before a crash, the user's message is not silently lost.
|
||||
"""
|
||||
s = _make_session("survive1", messages=[{"role": "user", "content": "first turn"}])
|
||||
s.save() # initial full save
|
||||
|
||||
# Simulate what routes.py does before _run_agent_streaming:
|
||||
s.pending_user_message = "do a long research task"
|
||||
s.pending_started_at = time.time()
|
||||
s.active_stream_id = "stream-abc123"
|
||||
s.save(skip_index=True) # checkpoint-style save
|
||||
|
||||
# Simulate restart: clear in-memory state, reload from disk
|
||||
del s
|
||||
models.SESSIONS.clear()
|
||||
|
||||
reloaded = Session.load("survive1")
|
||||
assert reloaded is not None
|
||||
assert reloaded.pending_user_message == "do a long research task"
|
||||
assert reloaded.active_stream_id == "stream-abc123"
|
||||
# Original messages still intact
|
||||
assert len(reloaded.messages) == 1
|
||||
|
||||
def test_activity_checkpoint_persists_updated_at(self):
|
||||
"""Each checkpoint save updates updated_at, keeping session fresh in sidebar."""
|
||||
s = _make_session("ts1")
|
||||
s.save()
|
||||
ts_before = s.updated_at
|
||||
|
||||
time.sleep(0.05)
|
||||
_checkpoint_activity = [1] # simulate one tool completion
|
||||
|
||||
stop_event = threading.Event()
|
||||
|
||||
def periodic_checkpoint():
|
||||
last = 0
|
||||
while not stop_event.wait(0.05):
|
||||
cur = _checkpoint_activity[0]
|
||||
if cur > last:
|
||||
s.save(skip_index=True)
|
||||
last = cur
|
||||
|
||||
t = threading.Thread(target=periodic_checkpoint, daemon=True)
|
||||
t.start()
|
||||
time.sleep(0.2)
|
||||
stop_event.set()
|
||||
t.join(timeout=1)
|
||||
|
||||
data = json.loads(s.path.read_text())
|
||||
assert data["updated_at"] > ts_before, "Checkpoint should update updated_at"
|
||||
|
||||
|
||||
class TestCheckpointVariableLifecycle:
|
||||
"""Regression guard: the outer `finally` must not UnboundLocalError when an
|
||||
exception fires before the checkpoint thread is created. _checkpoint_stop
|
||||
is initialised to None at the very top of the outer try block so the
|
||||
finally's `if _checkpoint_stop is not None` branch is always safe.
|
||||
"""
|
||||
|
||||
def test_checkpoint_stop_initialised_before_any_raiseable_code(self):
|
||||
"""Static check: `_checkpoint_stop = None` must appear before any code
|
||||
that could raise inside _run_agent_streaming's outer try."""
|
||||
src = (Path(__file__).parent.parent / "api" / "streaming.py").read_text(
|
||||
encoding="utf-8"
|
||||
)
|
||||
lines = src.splitlines()
|
||||
try_line = next(
|
||||
i for i, ln in enumerate(lines, 1)
|
||||
if ln.rstrip().endswith("try:") and lines[i - 2].strip().startswith("_checkpoint_stop")
|
||||
)
|
||||
# The assignment must precede the `try:` — not sit inside the nested
|
||||
# block where an earlier line could raise before it runs.
|
||||
init_line = next(
|
||||
i for i, ln in enumerate(lines, 1)
|
||||
if "_checkpoint_stop = None" in ln
|
||||
)
|
||||
assert init_line < try_line, (
|
||||
f"_checkpoint_stop = None (line {init_line}) must precede the outer "
|
||||
f"try block (line {try_line}) so the finally can safely check it."
|
||||
)
|
||||
|
||||
def test_finally_path_when_early_exception_does_not_unbound_error(self):
|
||||
"""Mirror the _run_agent_streaming try/finally structure — proves that
|
||||
pre-initialising _checkpoint_stop = None outside any raiseable code
|
||||
keeps the finally safe."""
|
||||
|
||||
def mimic_run_agent_streaming():
|
||||
_checkpoint_stop = None # pre-init (the fix)
|
||||
try:
|
||||
# Anything here could raise — simulate early failure
|
||||
raise ValueError("early failure, e.g. get_session KeyError")
|
||||
_checkpoint_stop = threading.Event() # never reached
|
||||
finally:
|
||||
# The guard the PR added — must not itself raise
|
||||
if _checkpoint_stop is not None:
|
||||
_checkpoint_stop.set()
|
||||
|
||||
with pytest.raises(ValueError, match="early failure"):
|
||||
mimic_run_agent_streaming()
|
||||
194
tests/test_issue789.py
Normal file
194
tests/test_issue789.py
Normal file
@@ -0,0 +1,194 @@
|
||||
"""
|
||||
Regression tests for GitHub issue #789.
|
||||
|
||||
Bug: every brand-new session immediately disappeared from the sidebar because
|
||||
all_sessions() filtered out sessions where title == 'Untitled' AND
|
||||
message_count == 0. Since every new session starts with those values, it was
|
||||
filtered out of /api/sessions on the next refresh.
|
||||
|
||||
Fix: exempt sessions younger than 60 seconds from that filter. Sessions older
|
||||
than 60 seconds that are still Untitled with 0 messages are still suppressed
|
||||
(ghost sessions from test runs / accidental reloads).
|
||||
"""
|
||||
import json
|
||||
import time
|
||||
|
||||
import pytest
|
||||
|
||||
import api.models as models
|
||||
from api.models import Session, all_sessions
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _isolate(tmp_path, monkeypatch):
|
||||
"""Redirect SESSION_DIR and SESSION_INDEX_FILE to a temp dir."""
|
||||
session_dir = tmp_path / "sessions"
|
||||
session_dir.mkdir()
|
||||
index_file = session_dir / "_index.json"
|
||||
|
||||
monkeypatch.setattr(models, "SESSION_DIR", session_dir)
|
||||
monkeypatch.setattr(models, "SESSION_INDEX_FILE", index_file)
|
||||
|
||||
models.SESSIONS.clear()
|
||||
yield
|
||||
models.SESSIONS.clear()
|
||||
|
||||
|
||||
def _make_untitled_session(age_seconds, messages=None, session_id=None):
|
||||
"""Create a Session with title='Untitled', updated_at set to age_seconds ago."""
|
||||
now = time.time()
|
||||
s = Session(
|
||||
session_id=session_id or None,
|
||||
title="Untitled",
|
||||
messages=messages or [],
|
||||
updated_at=now - age_seconds,
|
||||
created_at=now - age_seconds,
|
||||
)
|
||||
# Persist to disk so the full-scan fallback can also find it
|
||||
s.path.write_text(
|
||||
json.dumps(s.__dict__, ensure_ascii=False, indent=2), encoding="utf-8"
|
||||
)
|
||||
return s
|
||||
|
||||
|
||||
def _make_titled_session(age_seconds, session_id=None):
|
||||
"""Create a Session with a real title and one message."""
|
||||
now = time.time()
|
||||
s = Session(
|
||||
session_id=session_id or None,
|
||||
title="My conversation",
|
||||
messages=[{"role": "user", "content": "hello"}],
|
||||
updated_at=now - age_seconds,
|
||||
created_at=now - age_seconds,
|
||||
)
|
||||
s.path.write_text(
|
||||
json.dumps(s.__dict__, ensure_ascii=False, indent=2), encoding="utf-8"
|
||||
)
|
||||
return s
|
||||
|
||||
|
||||
# ── Test 1: brand-new Untitled 0-message session IS included ─────────────────
|
||||
|
||||
def test_new_untitled_session_is_visible_in_sidebar():
|
||||
"""A session created just now (0 seconds old) must appear in all_sessions()."""
|
||||
new_session = _make_untitled_session(age_seconds=0)
|
||||
|
||||
result = all_sessions()
|
||||
ids = {s["session_id"] for s in result}
|
||||
|
||||
assert new_session.session_id in ids, (
|
||||
"Brand-new Untitled 0-message session must be visible in the sidebar "
|
||||
"(fix for issue #789)"
|
||||
)
|
||||
|
||||
|
||||
def test_recent_untitled_session_under_60s_is_visible():
|
||||
"""A session 30 seconds old should still be visible."""
|
||||
recent_session = _make_untitled_session(age_seconds=30)
|
||||
|
||||
result = all_sessions()
|
||||
ids = {s["session_id"] for s in result}
|
||||
|
||||
assert recent_session.session_id in ids, (
|
||||
"Untitled 0-message session younger than 60 s must be visible (#789)"
|
||||
)
|
||||
|
||||
|
||||
# ── Test 2: old Untitled 0-message session IS still filtered ─────────────────
|
||||
|
||||
def test_old_untitled_session_over_60s_is_filtered():
|
||||
"""A ghost session (Untitled, 0 messages, >60 s old) must be hidden."""
|
||||
old_session = _make_untitled_session(age_seconds=120)
|
||||
|
||||
result = all_sessions()
|
||||
ids = {s["session_id"] for s in result}
|
||||
|
||||
assert old_session.session_id not in ids, (
|
||||
"Ghost Untitled 0-message session older than 60 s must be filtered out"
|
||||
)
|
||||
|
||||
|
||||
def test_session_exactly_at_boundary_is_filtered():
|
||||
"""A session just over 60 seconds old should be filtered."""
|
||||
boundary_session = _make_untitled_session(age_seconds=61)
|
||||
|
||||
result = all_sessions()
|
||||
ids = {s["session_id"] for s in result}
|
||||
|
||||
assert boundary_session.session_id not in ids, (
|
||||
"Untitled 0-message session older than 60 s must be filtered out"
|
||||
)
|
||||
|
||||
|
||||
# ── Test 3: session with messages is always visible regardless of age ─────────
|
||||
|
||||
def test_session_with_messages_always_visible_new():
|
||||
"""A session with messages (even Untitled) is always visible when new."""
|
||||
s = Session(
|
||||
title="Untitled",
|
||||
messages=[{"role": "user", "content": "hello"}],
|
||||
)
|
||||
s.path.write_text(
|
||||
json.dumps(s.__dict__, ensure_ascii=False, indent=2), encoding="utf-8"
|
||||
)
|
||||
|
||||
result = all_sessions()
|
||||
ids = {r["session_id"] for r in result}
|
||||
assert s.session_id in ids, "Session with messages must always appear in sidebar"
|
||||
|
||||
|
||||
def test_session_with_messages_always_visible_old():
|
||||
"""An old session with messages is always visible."""
|
||||
now = time.time()
|
||||
s = Session(
|
||||
title="Untitled",
|
||||
messages=[{"role": "user", "content": "hello"}],
|
||||
updated_at=now - 3600,
|
||||
created_at=now - 3600,
|
||||
)
|
||||
s.path.write_text(
|
||||
json.dumps(s.__dict__, ensure_ascii=False, indent=2), encoding="utf-8"
|
||||
)
|
||||
|
||||
result = all_sessions()
|
||||
ids = {r["session_id"] for r in result}
|
||||
assert s.session_id in ids, (
|
||||
"Old session with messages must always appear in sidebar"
|
||||
)
|
||||
|
||||
|
||||
def test_titled_session_with_no_messages_old_is_visible():
|
||||
"""A titled session with 0 messages (old) should not be filtered — filter
|
||||
only targets Untitled sessions."""
|
||||
now = time.time()
|
||||
s = Session(
|
||||
title="Project Alpha",
|
||||
messages=[],
|
||||
updated_at=now - 3600,
|
||||
created_at=now - 3600,
|
||||
)
|
||||
s.path.write_text(
|
||||
json.dumps(s.__dict__, ensure_ascii=False, indent=2), encoding="utf-8"
|
||||
)
|
||||
|
||||
result = all_sessions()
|
||||
ids = {r["session_id"] for r in result}
|
||||
assert s.session_id in ids, (
|
||||
"A titled session must always appear regardless of message count"
|
||||
)
|
||||
|
||||
|
||||
# ── Test 4: mixed bag — only old Untitled empty sessions are filtered ─────────
|
||||
|
||||
def test_mixed_sessions_correct_visibility():
|
||||
"""With a mix of sessions, only old+Untitled+empty ones are suppressed."""
|
||||
new_ghost = _make_untitled_session(age_seconds=5, session_id="new_ghost")
|
||||
old_ghost = _make_untitled_session(age_seconds=200, session_id="old_ghost")
|
||||
real_session = _make_titled_session(age_seconds=500, session_id="real_session")
|
||||
|
||||
result = all_sessions()
|
||||
ids = {s["session_id"] for s in result}
|
||||
|
||||
assert "new_ghost" in ids, "New Untitled session (5s old) must be visible"
|
||||
assert "old_ghost" not in ids, "Old Untitled session (200s old) must be hidden"
|
||||
assert "real_session" in ids, "Titled session with messages must be visible"
|
||||
184
tests/test_issue803.py
Normal file
184
tests/test_issue803.py
Normal file
@@ -0,0 +1,184 @@
|
||||
"""
|
||||
Issue #803 (completes #798) — per-client profile isolation via cookie + thread-local.
|
||||
|
||||
PR #800 fixed POST /api/session/new (client sends profile in body).
|
||||
PR #805 extends the fix to ALL endpoints: profile switches set a hermes_profile
|
||||
cookie, server.py reads it per-request into a thread-local, and the existing
|
||||
api/profiles.py helpers consult the thread-local before the process global.
|
||||
|
||||
Covers:
|
||||
1. build_profile_cookie() / get_profile_cookie() roundtrip + validation
|
||||
2. set_request_profile() / get_active_profile_name() / clear_request_profile()
|
||||
3. get_active_hermes_home() routes via thread-local
|
||||
4. switch_profile(process_wide=False) does NOT mutate process globals
|
||||
5. Concurrent requests on different threads see independent profiles
|
||||
"""
|
||||
import os
|
||||
import threading
|
||||
from pathlib import Path
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
# ── 1. Cookie build/parse roundtrip ──────────────────────────────────────────
|
||||
|
||||
class TestProfileCookieHelpers:
|
||||
|
||||
def test_build_profile_cookie_sets_value(self):
|
||||
from api.helpers import build_profile_cookie
|
||||
s = build_profile_cookie('alice')
|
||||
assert 'hermes_profile=alice' in s
|
||||
assert 'HttpOnly' in s
|
||||
assert 'SameSite=Lax' in s
|
||||
assert 'Path=/' in s
|
||||
|
||||
def test_build_profile_cookie_default_clears(self):
|
||||
from api.helpers import build_profile_cookie
|
||||
s = build_profile_cookie('default')
|
||||
assert 'Max-Age=0' in s
|
||||
# Empty value indicates clear
|
||||
assert 'hermes_profile=""' in s or 'hermes_profile=;' in s
|
||||
|
||||
def test_get_profile_cookie_returns_none_when_absent(self):
|
||||
from api.helpers import get_profile_cookie
|
||||
handler = MagicMock()
|
||||
handler.headers.get = lambda k, d='': ''
|
||||
assert get_profile_cookie(handler) is None
|
||||
|
||||
def test_get_profile_cookie_extracts_valid_name(self):
|
||||
from api.helpers import get_profile_cookie
|
||||
handler = MagicMock()
|
||||
handler.headers.get = lambda k, d='': 'hermes_profile=alice' if k == 'Cookie' else d
|
||||
assert get_profile_cookie(handler) == 'alice'
|
||||
|
||||
def test_get_profile_cookie_accepts_default(self):
|
||||
from api.helpers import get_profile_cookie
|
||||
handler = MagicMock()
|
||||
handler.headers.get = lambda k, d='': 'hermes_profile=default' if k == 'Cookie' else d
|
||||
assert get_profile_cookie(handler) == 'default'
|
||||
|
||||
def test_get_profile_cookie_rejects_injection(self):
|
||||
"""Cookie value must pass _PROFILE_ID_RE fullmatch — rejects traversal/injection."""
|
||||
from api.helpers import get_profile_cookie
|
||||
for bad in ('../etc', 'a/b', 'name;DROP', 'WithCaps', 'has space', '.hidden'):
|
||||
handler = MagicMock()
|
||||
handler.headers.get = lambda k, d='', v=bad: f'hermes_profile={v}' if k == 'Cookie' else d
|
||||
assert get_profile_cookie(handler) is None, f"{bad!r} should be rejected"
|
||||
|
||||
def test_get_profile_cookie_ignores_malformed_header(self):
|
||||
from api.helpers import get_profile_cookie
|
||||
handler = MagicMock()
|
||||
handler.headers.get = lambda k, d='': '\x00\x01not-a-cookie' if k == 'Cookie' else d
|
||||
# Must not raise; returns None
|
||||
result = get_profile_cookie(handler)
|
||||
assert result is None
|
||||
|
||||
|
||||
# ── 2. Thread-local request context ──────────────────────────────────────────
|
||||
|
||||
class TestThreadLocalProfileContext:
|
||||
|
||||
def test_tls_takes_priority_over_global(self):
|
||||
import api.profiles as p
|
||||
original = p._active_profile
|
||||
try:
|
||||
p._active_profile = 'global-default'
|
||||
p.set_request_profile('alice')
|
||||
assert p.get_active_profile_name() == 'alice'
|
||||
finally:
|
||||
p.clear_request_profile()
|
||||
p._active_profile = original
|
||||
|
||||
def test_global_used_when_tls_cleared(self):
|
||||
import api.profiles as p
|
||||
original = p._active_profile
|
||||
try:
|
||||
p._active_profile = 'global-default'
|
||||
p.set_request_profile('alice')
|
||||
p.clear_request_profile()
|
||||
assert p.get_active_profile_name() == 'global-default'
|
||||
finally:
|
||||
p._active_profile = original
|
||||
|
||||
def test_clear_is_idempotent(self):
|
||||
import api.profiles as p
|
||||
# Calling clear on a thread that never set anything must not raise
|
||||
p.clear_request_profile()
|
||||
p.clear_request_profile()
|
||||
|
||||
|
||||
# ── 3. get_active_hermes_home routes through TLS ─────────────────────────────
|
||||
|
||||
def test_get_active_hermes_home_respects_tls(tmp_path, monkeypatch):
|
||||
import api.profiles as p
|
||||
monkeypatch.setattr(p, '_DEFAULT_HERMES_HOME', tmp_path)
|
||||
profile_dir = tmp_path / 'profiles' / 'alice'
|
||||
profile_dir.mkdir(parents=True)
|
||||
try:
|
||||
p.set_request_profile('alice')
|
||||
assert p.get_active_hermes_home() == profile_dir
|
||||
p.set_request_profile('default')
|
||||
assert p.get_active_hermes_home() == tmp_path
|
||||
finally:
|
||||
p.clear_request_profile()
|
||||
|
||||
|
||||
# ── 4. switch_profile(process_wide=False) does not mutate globals ─────────────
|
||||
|
||||
def test_switch_profile_process_wide_false_does_not_mutate_global():
|
||||
"""Per-client switches from the WebUI must leave _active_profile untouched."""
|
||||
import api.profiles as p
|
||||
|
||||
# Monkey in a fake profile listing so switch_profile finds 'alice'
|
||||
original_global = p._active_profile
|
||||
original_env_home = os.environ.get('HERMES_HOME')
|
||||
|
||||
# We need a profile that exists to get past the validation path.
|
||||
# Use 'default' — switch_profile accepts it without requiring hermes_cli.
|
||||
try:
|
||||
result = p.switch_profile('default', process_wide=False)
|
||||
# Global must not change
|
||||
assert p._active_profile == original_global, (
|
||||
f"process_wide=False must not mutate _active_profile "
|
||||
f"(was {original_global!r}, now {p._active_profile!r})"
|
||||
)
|
||||
# HERMES_HOME env must not change
|
||||
assert os.environ.get('HERMES_HOME') == original_env_home, (
|
||||
"process_wide=False must not mutate os.environ['HERMES_HOME']"
|
||||
)
|
||||
# Response still shape-compatible
|
||||
assert isinstance(result, dict)
|
||||
finally:
|
||||
p._active_profile = original_global
|
||||
|
||||
|
||||
# ── 5. Concurrent threads see independent profile context ────────────────────
|
||||
|
||||
def test_concurrent_threads_see_independent_profiles():
|
||||
"""The whole point of thread-local isolation: two threads, two cookies,
|
||||
two different get_active_profile_name() results, simultaneously."""
|
||||
import api.profiles as p
|
||||
|
||||
results = {}
|
||||
errors = []
|
||||
barrier = threading.Barrier(2, timeout=5)
|
||||
|
||||
def worker(name, key):
|
||||
try:
|
||||
p.set_request_profile(name)
|
||||
barrier.wait() # both threads have set their TLS
|
||||
# Now each thread reads — must see its own value
|
||||
results[key] = p.get_active_profile_name()
|
||||
p.clear_request_profile()
|
||||
except Exception as exc:
|
||||
errors.append(exc)
|
||||
|
||||
t1 = threading.Thread(target=worker, args=('alice', 'alice'))
|
||||
t2 = threading.Thread(target=worker, args=('bob', 'bob'))
|
||||
t1.start(); t2.start()
|
||||
t1.join(timeout=10); t2.join(timeout=10)
|
||||
|
||||
assert not errors, f"Workers raised: {errors}"
|
||||
assert results.get('alice') == 'alice', f"alice thread saw {results.get('alice')!r}"
|
||||
assert results.get('bob') == 'bob', f"bob thread saw {results.get('bob')!r}"
|
||||
Reference in New Issue
Block a user