From fd1c4eaeafebc1efaab19ab58da14530af640c50 Mon Sep 17 00:00:00 2001 From: hermes-agent Date: Sun, 24 May 2026 18:57:40 +0000 Subject: [PATCH 1/6] =?UTF-8?q?Stage=20406:=20PR=20#2827=20=E2=80=94=20fix?= =?UTF-8?q?(state-sync):=20pass=20profile=20explicitly=20so=20background-t?= =?UTF-8?q?hread=20DB=20writes=20hit=20the=20right=20state.db=20(#2762)=20?= =?UTF-8?q?by=20@Koraji95-coder?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- api/state_sync.py | 97 +++++- api/streaming.py | 6 + ...test_issue2762_state_sync_profile_kwarg.py | 284 ++++++++++++++++++ 3 files changed, 373 insertions(+), 14 deletions(-) create mode 100644 tests/test_issue2762_state_sync_profile_kwarg.py diff --git a/api/state_sync.py b/api/state_sync.py index fa9c9b33..dbb2873f 100644 --- a/api/state_sync.py +++ b/api/state_sync.py @@ -20,22 +20,78 @@ from pathlib import Path logger = logging.getLogger(__name__) -def _get_state_db(): - """Get a SessionDB instance for the active profile's state.db. - Returns None if hermes_state is not importable or DB is unavailable. - Each caller is responsible for calling db.close() when done. +def _get_state_db(profile: str=None): + """Get a SessionDB instance for a profile's state.db. + + When ``profile`` is provided the function resolves *that* profile's + home directory directly (via ``_resolve_profile_home_for_name``). + If resolution fails (unknown profile name, IO error, etc.) the + function returns ``None`` rather than silently falling back to + ``HERMES_HOME`` — silently routing the write to the wrong DB + would defeat the point of the explicit-profile path (#2762). + + When ``profile`` is None it falls back to the TLS-based + ``get_active_hermes_home()`` lookup for backward compatibility, + with a final ``HERMES_HOME`` fallback only on that path. TLS may be + unset in background/worker threads, in which case the lookup falls + through to the process-global active profile and can write to the + wrong DB. Callers that know the session's profile (e.g. + ``sync_session_usage`` after a stream completes on a background + thread) should pass it explicitly to avoid that race. + + Returns None if hermes_state is not importable, the explicit + profile cannot be resolved, or the DB is unavailable. Each caller + is responsible for calling db.close() when done. """ try: from hermes_state import SessionDB except ImportError: return None - try: - from api.profiles import get_active_hermes_home - hermes_home = Path(get_active_hermes_home()).expanduser().resolve() - except Exception: - logger.debug("Failed to resolve hermes home, using default") - hermes_home = Path(os.getenv('HERMES_HOME', str(Path.home() / '.hermes'))) + if profile is not None: + # Explicit-profile path — a resolution failure here MUST NOT + # silently fall back to HERMES_HOME or the caller's "write to + # the named profile" contract is broken (the original #2762 + # symptom: writes leaking into the wrong profile's state.db). + # + # Defense-in-depth (per #2827 maintainer review): validate the + # name shape BEFORE handing it to ``_resolve_profile_home_for_name``. + # The resolver itself rarely raises — for an invalid-but-non- + # malicious name (e.g. one that fails ``_PROFILE_ID_RE``) it + # quietly returns ``_DEFAULT_HERMES_HOME``, which is the exact + # leak we're trying to prevent on the explicit-profile path. + # Validating up-front turns that quiet leak into an explicit + # "refuse + log + return None" so the contract is "write to + # the EXACT named profile, or write nowhere." + try: + from api.profiles import ( + _resolve_profile_home_for_name, + _PROFILE_ID_RE, + _is_root_profile, + ) + if not (_is_root_profile(profile) or _PROFILE_ID_RE.fullmatch(profile)): + logger.warning( + "state_sync: refusing invalid profile name %r — skipping " + "write rather than leaking to the default state.db (#2762).", + profile, + ) + return None + hermes_home = Path(_resolve_profile_home_for_name(profile)).expanduser().resolve() + except Exception: + logger.warning( + "state_sync: could not resolve profile %r — skipping write rather " + "than leaking to the active profile (#2762).", profile, + ) + return None + else: + # Implicit / TLS-fallback path — preserves pre-#2762 behavior + # for any caller that doesn't pass profile= explicitly. + try: + from api.profiles import get_active_hermes_home + hermes_home = Path(get_active_hermes_home()).expanduser().resolve() + except Exception: + logger.debug("Failed to resolve hermes home, using default") + hermes_home = Path(os.getenv('HERMES_HOME', str(Path.home() / '.hermes'))) db_path = hermes_home / 'state.db' if not db_path.exists(): @@ -48,11 +104,16 @@ def _get_state_db(): return None -def sync_session_start(session_id: str, model=None) -> None: +def sync_session_start(session_id: str, model=None, profile: str=None) -> None: """Register a WebUI session in state.db (idempotent). Called when a session's first message is sent. + + ``profile`` lets the caller name the target state.db explicitly, + avoiding the TLS-vs-background-thread mismatch in #2762. When + omitted, the active profile is resolved from TLS (then process + globals) as before. """ - db = _get_state_db() + db = _get_state_db(profile=profile) if not db: return try: @@ -72,12 +133,20 @@ def sync_session_start(session_id: str, model=None) -> None: def sync_session_usage(session_id: str, input_tokens: int=0, output_tokens: int=0, estimated_cost=None, model=None, title: str=None, - message_count: int=None) -> None: + message_count: int=None, profile: str=None) -> None: """Update token usage and title for a WebUI session in state.db. Called after each turn completes. Uses absolute=True to set totals (the WebUI Session already accumulates across turns). + + ``profile`` lets the caller name the target state.db explicitly, + which is what fixes #2762: this function is invoked from the + agent streaming worker thread, where the request-thread's TLS + profile context has not been propagated. Without an explicit + profile, the TLS lookup falls back to the process-global active + profile and writes the session's usage to the wrong state.db + (e.g. ``hiyuki``'s instead of the cookie-switched ``maiko``'s). """ - db = _get_state_db() + db = _get_state_db(profile=profile) if not db: return try: diff --git a/api/streaming.py b/api/streaming.py index 5718ee1c..cd6cb960 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -5070,6 +5070,12 @@ def _run_agent_streaming( model=model, title=s.title, message_count=len(s.messages), + # #2762: pass the session's profile explicitly so the + # background-thread state.db lookup doesn't fall + # through to the process-global active profile and + # write to the wrong DB (TLS profile is set on the + # HTTP thread but not propagated to this worker). + profile=getattr(s, 'profile', None), ) except Exception: logger.debug("Failed to sync session to insights") diff --git a/tests/test_issue2762_state_sync_profile_kwarg.py b/tests/test_issue2762_state_sync_profile_kwarg.py new file mode 100644 index 00000000..c112fead --- /dev/null +++ b/tests/test_issue2762_state_sync_profile_kwarg.py @@ -0,0 +1,284 @@ +""" +Regression test for #2762 — state_sync writes to wrong profile's state.db +when profile is switched via WebUI cookie. + +Root cause: ``_get_state_db()`` relied on TLS-based +``get_active_hermes_home()`` to pick the DB path. TLS gets set on the HTTP +thread by the cookie middleware, but the agent streaming worker thread that +calls ``sync_session_usage`` does NOT inherit that TLS, so the lookup falls +through to the process-global active profile and writes to the wrong DB. + +Fix: ``_get_state_db(profile=...)`` accepts an explicit profile name and +resolves *that* profile's home directly via +``_resolve_profile_home_for_name``. Callers that know the session's profile +(e.g. ``sync_session_usage`` after streaming completes) pass it explicitly, +avoiding the TLS race. + +These tests pin: + 1. ``_get_state_db(profile='X')`` resolves X's home, not the active profile's. + 2. ``sync_session_usage(..., profile='X')`` writes to X's state.db only, + even when the global active profile is set to Y. + 3. ``sync_session_usage`` with no profile kwarg falls back to the old + TLS behavior (so existing callers don't regress). +""" +from __future__ import annotations + +import sys +import sqlite3 +from pathlib import Path + +import pytest + + +# Make sure we can import the api package the same way the server does. +_REPO_ROOT = Path(__file__).resolve().parent.parent +if str(_REPO_ROOT) not in sys.path: + sys.path.insert(0, str(_REPO_ROOT)) + + +@pytest.fixture() +def two_profile_homes(tmp_path, monkeypatch): + """Stand up two minimal profile homes with state.db initialized via + ``hermes_state.SessionDB`` itself (so the schema matches what the + production code expects — `sync_session_usage` does a raw-SQL + UPDATE of `message_count`, which hand-rolled schemas could miss). + Per Copilot review on PR #2827. + """ + # Skip the fixture cleanly if the production package isn't importable + # in this env — same gate the tests below use. + pytest.importorskip("hermes_state") + from hermes_state import SessionDB + + hiyuki_home = tmp_path / 'hiyuki' + maiko_home = tmp_path / 'maiko' + for home in (hiyuki_home, maiko_home): + home.mkdir(parents=True) + # Touch state.db then open via SessionDB so its own constructor + # runs whatever schema init / migration the production code + # would see at runtime. Then close immediately — each test + # opens its own handle through the production code path. + (home / 'state.db').touch() + SessionDB(home / 'state.db').close() + + # Stub api.profiles to return our temp paths + import api.profiles as profiles_mod + + def fake_resolve(name): + if name == 'hiyuki': + return hiyuki_home + if name == 'maiko': + return maiko_home + raise LookupError(name) + + monkeypatch.setattr(profiles_mod, '_resolve_profile_home_for_name', fake_resolve) + # Active profile is hiyuki — the WRONG one for tests that pass profile='maiko' + monkeypatch.setattr(profiles_mod, 'get_active_hermes_home', lambda: hiyuki_home) + + return {'hiyuki': hiyuki_home, 'maiko': maiko_home} + + +def _read_session(db_path: Path, session_id: str): + if not db_path.exists(): + return None + conn = sqlite3.connect(db_path) + try: + cur = conn.execute( + "SELECT session_id, title, input_tokens, output_tokens " + "FROM sessions WHERE session_id = ?", + (session_id,), + ) + row = cur.fetchone() + return row + finally: + conn.close() + + +def test_get_state_db_honors_explicit_profile_kwarg(two_profile_homes): + """_get_state_db(profile='maiko') resolves to maiko's home, NOT + the active profile (hiyuki).""" + from api.state_sync import _get_state_db + + # Some installs ship without the hermes_state package; the function + # returns None gracefully and there's nothing to assert. + try: + import hermes_state # noqa: F401 + except ImportError: + pytest.skip("hermes_state package not available in this test env") + + db = _get_state_db(profile='maiko') + if db is None: + pytest.skip("SessionDB could not open the test db (env issue)") + # We don't have a public accessor for the underlying path on SessionDB, + # but writing through it and reading the raw file should work. + db.ensure_session(session_id='probe-2762', source='webui', model='test') + try: + db.close() + except Exception: + pass + + # maiko's state.db should have the row; hiyuki's should not. + assert _read_session(two_profile_homes['maiko'] / 'state.db', 'probe-2762') is not None, \ + "session was not written to maiko's state.db" + assert _read_session(two_profile_homes['hiyuki'] / 'state.db', 'probe-2762') is None, \ + "session leaked into hiyuki's state.db — TLS-fallback regressed" + + +def test_sync_session_usage_writes_only_to_named_profile(two_profile_homes): + """sync_session_usage(..., profile='maiko') is the actual scenario from + the streaming worker thread post-#2762. The write must land in maiko's + state.db only, regardless of what the global active profile is.""" + try: + import hermes_state # noqa: F401 + except ImportError: + pytest.skip("hermes_state package not available in this test env") + + from api.state_sync import sync_session_usage + + sync_session_usage( + session_id='2762-regression', + input_tokens=42, + output_tokens=17, + estimated_cost=0.001, + model='test-model', + title='2762 regression test', + message_count=3, + profile='maiko', + ) + + maiko_row = _read_session(two_profile_homes['maiko'] / 'state.db', '2762-regression') + hiyuki_row = _read_session(two_profile_homes['hiyuki'] / 'state.db', '2762-regression') + + assert maiko_row is not None, \ + "sync_session_usage(profile='maiko') did not write to maiko's state.db" + assert hiyuki_row is None, \ + "sync_session_usage(profile='maiko') leaked into hiyuki's state.db — #2762 regression" + + +def test_sync_session_usage_without_profile_kwarg_uses_active(two_profile_homes): + """Backward compatibility: when called without a profile kwarg (the + pre-#2762 call shape), the function falls back to the active profile + (here: hiyuki). Existing callers should not regress.""" + try: + import hermes_state # noqa: F401 + except ImportError: + pytest.skip("hermes_state package not available in this test env") + + from api.state_sync import sync_session_usage + + sync_session_usage( + session_id='legacy-call-shape', + input_tokens=1, + output_tokens=2, + model='legacy', + title='legacy', + message_count=1, + ) + + hiyuki_row = _read_session(two_profile_homes['hiyuki'] / 'state.db', 'legacy-call-shape') + assert hiyuki_row is not None, \ + "sync_session_usage() without profile= regressed: did not write to active profile's state.db" + + +def test_unknown_explicit_profile_returns_none_not_falls_back(two_profile_homes): + """Copilot review of PR #2827: when ``profile`` is explicit and + resolution fails (e.g. typoed profile name, IO error), the + function MUST return None rather than silently fall back to + HERMES_HOME and write to the wrong DB. That fallback would + re-introduce the exact #2762 symptom (writes leaking into the + active profile). + + The fixture's `fake_resolve` raises LookupError for any name + that isn't 'hiyuki' or 'maiko', so passing 'does-not-exist' + here exercises the failure path. + """ + try: + import hermes_state # noqa: F401 + except ImportError: + pytest.skip("hermes_state package not available in this test env") + + from api.state_sync import sync_session_usage + + # Passing an unknown profile name MUST NOT cause a write to land in + # hiyuki (the active profile's home). If we leaked there, that's + # the exact bug we're guarding against. + sync_session_usage( + session_id='unknown-profile-probe', + input_tokens=99, + output_tokens=99, + model='probe', + title='probe', + message_count=1, + profile='does-not-exist', + ) + + hiyuki_row = _read_session(two_profile_homes['hiyuki'] / 'state.db', 'unknown-profile-probe') + maiko_row = _read_session(two_profile_homes['maiko'] / 'state.db', 'unknown-profile-probe') + + assert hiyuki_row is None, ( + "unknown explicit profile leaked write into hiyuki's state.db — #2762 regression" + ) + assert maiko_row is None, ( + "unknown explicit profile somehow ended up in maiko's state.db" + ) + + +@pytest.mark.parametrize("bad_name", [ + "../etc", # path traversal attempt + "Foo Bar", # space — invalid chars + "FOO", # uppercase — invalid per _PROFILE_ID_RE + "-leading-dash", # leading dash — invalid per regex (must start [a-z0-9]) + "_underscore", # leading underscore — invalid per regex + "a" * 100, # too long (> 64 chars) + "", # empty string is handled by _is_root_profile, separate case +]) +def test_invalid_profile_name_refused_not_falls_back(two_profile_homes, bad_name): + """Per PR #2827 maintainer review: an invalid-but-non-malicious + profile name on the explicit-profile path must be REFUSED, not + quietly routed to the default state.db. + + Before this defense, ``_resolve_profile_home_for_name`` would return + ``_DEFAULT_HERMES_HOME`` for any name failing ``_PROFILE_ID_RE`` + without raising — which is the exact #2762 leak symptom with a + different trigger. The new regex check up-front turns that quiet + leak into an explicit "refuse + log + return None" so the + explicit-path contract is "write to the EXACT named profile, or + write nowhere." + + The empty string is intentionally in the parametrize set because + we want to confirm it's refused — ``_is_root_profile('')`` returns + False (per ``api/profiles.py:216-217`` it short-circuits on falsy + input), so an empty explicit profile fails both the + ``_is_root_profile`` check and the regex, and the contract refuses + the write. That's the expected behavior — an empty explicit name + is itself a bug at the caller, not "I want the default." + """ + try: + import hermes_state # noqa: F401 + except ImportError: + pytest.skip("hermes_state package not available in this test env") + + from api.state_sync import sync_session_usage + + sync_session_usage( + session_id=f'invalid-name-probe-{abs(hash(bad_name))}', + input_tokens=99, + output_tokens=99, + model='probe', + title='probe', + message_count=1, + profile=bad_name, + ) + + sid = f'invalid-name-probe-{abs(hash(bad_name))}' + hiyuki_row = _read_session(two_profile_homes['hiyuki'] / 'state.db', sid) + maiko_row = _read_session(two_profile_homes['maiko'] / 'state.db', sid) + + # All invalid names (including empty string) MUST be refused — + # no row should appear in either profile's state.db. + assert hiyuki_row is None, ( + f"invalid profile name {bad_name!r} leaked write into hiyuki's " + "state.db — defense missed; #2762 regression" + ) + assert maiko_row is None, ( + f"invalid profile name {bad_name!r} somehow ended up in maiko's state.db" + ) From d9b2dd5019c9add7b2dcc9070b6710ea41f87074 Mon Sep 17 00:00:00 2001 From: hermes-agent Date: Sun, 24 May 2026 18:58:37 +0000 Subject: [PATCH 2/6] =?UTF-8?q?Stage=20406:=20PR=20#2726=20=E2=80=94=20fix?= =?UTF-8?q?(model):=20keep=20boot=20default=20precedence=20non-destructive?= =?UTF-8?q?=20by=20@starship-s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cherry-picked via 3-way apply onto stage HEAD (post-Release-A/B/C1). Resolved boot.js conflict: took PR's parameterized populateModelDropdown({preferProfileDefaultOnFreshBoot:true}) call (the whole point of #2726) on top of master's #2716 boot path. Co-authored-by: starship-s --- static/boot.js | 13 +- static/ui.js | 37 ++- tests/test_model_default_boot_precedence.py | 306 +++++++++++++++++++- tests/test_session_metadata_fast_path.py | 2 +- 4 files changed, 326 insertions(+), 32 deletions(-) diff --git a/static/boot.js b/static/boot.js index b1c67dd6..a405118c 100644 --- a/static/boot.js +++ b/static/boot.js @@ -1477,12 +1477,8 @@ function applyBotName(){ if(sel&&typeof _applyModelToDropdown==='function'){ // Fresh page boot must prefer the profile/server default over stale // browser-persisted model state. A restored session can still apply its - // own persisted model later through loadSession(). - if(typeof _clearPersistedModelState==='function') _clearPersistedModelState(); - else { - localStorage.removeItem('hermes-webui-model'); - localStorage.removeItem('hermes-webui-model-state'); - } + // own persisted model later through loadSession(). Preserve the browser + // keys for legacy/no-default fallback paths instead of deleting them. const existingDefaultOpt=Array.from(sel.options).find(o=>o.value===s.default_model); if(existingDefaultOpt&&window._activeProvider&&!existingDefaultOpt.dataset.provider){ existingDefaultOpt.dataset.provider=window._activeProvider; @@ -1590,10 +1586,7 @@ function applyBotName(){ // Fetch available models without blocking session restore. The static HTML // options are enough for first paint; the dynamic provider list can settle // after the saved session is visible. - const _hydrateBootModelDropdown=()=>populateModelDropdown().then(()=>{ - const sessionModelState=S.session&&S.session.model - ? {model:S.session.model,model_provider:S.session.model_provider||null} - : null; + const _hydrateBootModelDropdown=()=>populateModelDropdown({preferProfileDefaultOnFreshBoot:true}).then(()=>{ const savedState=(typeof _readPersistedModelState==='function') ? _readPersistedModelState() : (localStorage.getItem('hermes-webui-model')?{model:localStorage.getItem('hermes-webui-model'),model_provider:null}:null); diff --git a/static/ui.js b/static/ui.js index b7c581f6..88b5365c 100644 --- a/static/ui.js +++ b/static/ui.js @@ -779,6 +779,33 @@ function _modelStateForSelect(sel, modelId){ const provider=String(_getOptionProviderId(opt)||'').trim(); return {model:value,model_provider:(provider&&provider!=='default')?provider:null}; } +function _captureModelDropdownSelection(sel){ + if(!sel||!sel.value) return null; + try{ + const state=_modelStateForSelect(sel,sel.value); + if(state&&state.model) return state; + }catch(_){} + return {model:String(sel.value||''),model_provider:null}; +} +function _reconcileModelDropdownSelection(sel,data,previousState,opts){ + if(!sel) return null; + const activeSession=(typeof S!=='undefined'&&S&&S.session)?S.session:null; + // Fresh boot is the only path where the profile/server default intentionally + // beats a browser-persisted or static fallback value. Every other model-list + // rebuild should preserve the loaded session model or the user's current + // in-page selection when it still exists in the refreshed catalog. + const shouldApplyBootDefault=!!(opts&&opts.preferProfileDefaultOnFreshBoot); + if(shouldApplyBootDefault && data&&data.default_model && !(activeSession&&activeSession.model)){ + return _applyModelToDropdown(data.default_model,sel,data.active_provider||null); + } + if(activeSession&&activeSession.model){ + return _applyModelToDropdown(activeSession.model,sel,activeSession.model_provider||null); + } + if(previousState&&previousState.model){ + return _applyModelToDropdown(previousState.model,sel,previousState.model_provider||null); + } + return null; +} function _providerQualifiedModelValueForSelect(sel, modelId){ return _modelStateForSelect(sel,modelId).model; } @@ -988,7 +1015,7 @@ function _applySessionModelFallback(sel){ return null; } -async function populateModelDropdown(){ +async function populateModelDropdown(opts={}){ const sel=$('modelSelect'); if(!sel) return; try{ @@ -1046,6 +1073,7 @@ async function populateModelDropdown(){ : _synthGroupsFromConfigured(); if(!groups.length) return; // no server groups and no configured fallback + const previousSelection=_captureModelDropdownSelection(sel); // Clear existing options sel.innerHTML=''; _dynamicModelLabels={}; @@ -1078,12 +1106,7 @@ async function populateModelDropdown(){ } sel.appendChild(og); } - // Set default model from server on fresh/blank boot. Loaded sessions keep - // their own persisted model and apply it via loadSession(). Do not let stale - // browser localStorage suppress the profile default. - if(data.default_model && !(S.session&&S.session.model)){ - _applyModelToDropdown(data.default_model, sel, data.active_provider||null); - } + _reconcileModelDropdownSelection(sel,data,previousSelection,opts); if(typeof syncModelChip==='function') syncModelChip(); const dd=$('composerModelDropdown'); if(dd&&dd.classList.contains('open')&&typeof renderModelDropdown==='function'){ diff --git a/tests/test_model_default_boot_precedence.py b/tests/test_model_default_boot_precedence.py index 730346e3..361fbe53 100644 --- a/tests/test_model_default_boot_precedence.py +++ b/tests/test_model_default_boot_precedence.py @@ -3,32 +3,310 @@ Regression coverage for model selector drift on fresh browser boot. A stale browser-persisted model (localStorage) must not suppress the configured profile/server default on page load. Restored sessions may still apply their own -session model later through loadSession(). +session model later through loadSession(). The boot fix must not make browser +model-state writes pointless or let later model-list refreshes reset a live +in-page selection. """ +import json +import shutil +import subprocess from pathlib import Path +import pytest + REPO = Path(__file__).resolve().parents[1] BOOT_JS = (REPO / "static" / "boot.js").read_text(encoding="utf-8") UI_JS = (REPO / "static" / "ui.js").read_text(encoding="utf-8") +NODE = shutil.which("node") -def test_boot_settings_applies_default_even_when_browser_model_state_exists(): - assert "Fresh page boot must prefer the profile/server default" in BOOT_JS - assert "_clearPersistedModelState" in BOOT_JS - assert "localStorage.removeItem('hermes-webui-model-state')" in BOOT_JS - assert "if(sel&&typeof _applyModelToDropdown==='function')" in BOOT_JS +_DRIVER_SRC = r""" +const fs = require('fs'); +const ui = fs.readFileSync(process.argv[2], 'utf8'); + +function extractFunc(name) { + const re = new RegExp('(?:async\\s+)?function\\s+' + name + '\\s*\\('); + const start = ui.search(re); + if (start < 0) throw new Error(name + ' not found'); + let openParen = ui.indexOf('(', start); + let i = openParen + 1; + let parenDepth = 1; + while (parenDepth > 0 && i < ui.length) { + if (ui[i] === '(') parenDepth++; + else if (ui[i] === ')') parenDepth--; + i++; + } + i = ui.indexOf('{', i); + let depth = 1; + i++; + while (depth > 0 && i < ui.length) { + if (ui[i] === '{') depth++; + else if (ui[i] === '}') depth--; + i++; + } + return ui.slice(start, i); +} + +const calls = {syncModelChip: 0, renderModelDropdown: 0, positionModelDropdown: 0, liveFetches: []}; +let modelSelect; +let apiModels; + +function makeSelect(options, initialValue) { + const sel = {id: 'modelSelect', options: [], selectedIndex: -1, selectedOptions: []}; + Object.defineProperty(sel, 'value', { + get() { return this._value || ''; }, + set(v) { + this._value = v; + const idx = this.options.findIndex(o => o.value === v); + this.selectedIndex = idx; + this.selectedOptions = idx >= 0 ? [this.options[idx]] : []; + } + }); + Object.defineProperty(sel, 'innerHTML', { + get() { return ''; }, + set(_v) { + this.options = []; + this.value = ''; + } + }); + sel.querySelector = function(_selector) { return this.options[0] || null; }; + sel.appendChild = function(node) { + const incoming = node && node.tagName === 'OPTGROUP' ? node.children : [node]; + for (const opt of incoming || []) { + opt.parentElement = opt.parentElement || node; + this.options.push(opt); + if (this.selectedIndex < 0) this.value = opt.value; + else if (this._value === opt.value) this.value = opt.value; + } + }; + for (const item of options || []) { + const group = {tagName: 'OPTGROUP', dataset: {provider: item.provider || ''}, children: []}; + const opt = {tagName: 'OPTION', value: item.value, textContent: item.label || item.value, title: '', dataset: {}, parentElement: group}; + group.children.push(opt); + sel.appendChild(group); + } + sel.value = initialValue || ''; + return sel; +} + +function $(id) { + if (id === 'modelSelect') return modelSelect; + if (id === 'composerModelDropdown') return {classList: {contains(){ return false; }}}; + return null; +} +function t(key) { return key; } +function getModelLabel(v) { return v; } +function syncModelChip() { calls.syncModelChip++; } +function renderModelDropdown() { calls.renderModelDropdown++; } +function _positionModelDropdown() { calls.positionModelDropdown++; } +function _redirectIfUnauth() { return false; } +function _fetchLiveModels(provider, _sel) { calls.liveFetches.push(provider); } + +const document = { + baseURI: 'http://127.0.0.1/hermes/', + createElement(tag) { + const upper = tag.toUpperCase(); + if (upper === 'OPTGROUP') { + return { + tagName: 'OPTGROUP', + label: '', + dataset: {}, + children: [], + appendChild(opt) { opt.parentElement = this; this.children.push(opt); }, + }; + } + return {tagName: upper, value: '', textContent: '', title: '', dataset: {}, parentElement: null}; + }, +}; +const localStorage = {getItem(){return null;}, setItem(){}, removeItem(){}}; +const window = {_defaultModel: null, _activeProvider: null, _configuredModelBadges: {}}; +let _dynamicModelLabels = {}; +let _liveModelFetchPending = new Set(); +let _liveModelCache = {}; + +for (const name of [ + '_getOptionProviderId', '_providerFromModelValue', '_modelStateForSelect', + '_captureModelDropdownSelection', '_findModelInDropdown', '_refreshOpenModelDropdown', + '_applyModelToDropdown', '_reconcileModelDropdownSelection', 'populateModelDropdown' +]) { + eval(extractFunc(name)); +} + +const args = JSON.parse(process.argv[3]); +apiModels = args.apiModels; +modelSelect = makeSelect(args.initialOptions, args.initialValue); +var S = {session: args.session || null}; + +fetch = async function(url) { + const href = String(url); + if (href.includes('api/models')) { + return {json: async () => apiModels}; + } + throw new Error('unexpected fetch ' + href); +}; + +populateModelDropdown(args.opts || {}).then(() => { + process.stdout.write(JSON.stringify({ + selectValue: modelSelect.value, + selectedProvider: modelSelect.selectedOptions[0] ? _getOptionProviderId(modelSelect.selectedOptions[0]) : null, + defaultModel: window._defaultModel, + activeProvider: window._activeProvider, + calls, + })); +}).catch(err => { + console.error(err && err.stack || err); + process.exit(1); +}); +""" + + +@pytest.fixture(scope="module") +def populate_driver_path(tmp_path_factory): + p = tmp_path_factory.mktemp("model_default_driver") / "driver.js" + p.write_text(_DRIVER_SRC, encoding="utf-8") + return str(p) + + +def test_boot_settings_applies_default_without_deleting_browser_model_state(): + snippet = _boot_default_apply_snippet() + assert "Fresh page boot must prefer the profile/server default" in snippet + assert "if(sel&&typeof _applyModelToDropdown==='function')" in snippet assert "if(sel&&!savedState&&typeof _applyModelToDropdown==='function')" not in BOOT_JS + assert "_clearPersistedModelState" not in snippet + assert "localStorage.removeItem('hermes-webui-model')" not in snippet + assert "localStorage.removeItem('hermes-webui-model-state')" not in snippet -def test_populate_model_dropdown_default_not_blocked_by_localstorage(): - assert "Do not let stale\n // browser localStorage suppress the profile default" in UI_JS - assert "if(data.default_model && !(S.session&&S.session.model))" in UI_JS - assert "_readPersistedModelState()" not in _populate_default_guard_snippet() - assert "localStorage.getItem('hermes-webui-model')" not in _populate_default_guard_snippet() +def test_boot_model_dropdown_explicitly_requests_profile_default_precedence(): + assert "populateModelDropdown({preferProfileDefaultOnFreshBoot:true})" in BOOT_JS + assert "const allowBootSavedModelOverride=!window._defaultModel" in BOOT_JS -def _populate_default_guard_snippet() -> str: - marker = "// Set default model from server on fresh/blank boot." +def test_populate_model_dropdown_reconciles_selection_after_rebuild(): + assert "const previousSelection=_captureModelDropdownSelection(sel);" in UI_JS + assert "_reconcileModelDropdownSelection(sel,data,previousSelection,opts);" in UI_JS + snippet = _reconcile_selection_snippet() + assert "preferProfileDefaultOnFreshBoot" in snippet + assert "return _applyModelToDropdown(data.default_model,sel,data.active_provider||null);" in snippet + assert "return _applyModelToDropdown(activeSession.model,sel,activeSession.model_provider||null);" in snippet + assert "return _applyModelToDropdown(previousState.model,sel,previousState.model_provider||null);" in snippet + assert "_readPersistedModelState()" not in snippet + assert "localStorage.getItem('hermes-webui-model')" not in snippet + + +@pytest.mark.skipif(NODE is None, reason="node not on PATH") +def test_non_boot_model_refresh_preserves_current_in_page_selection(populate_driver_path): + got = _run_populate_driver( + populate_driver_path, + initial_value="@expensive:gpt-5.5", + opts={}, + session=None, + ) + + assert got["selectValue"] == "@expensive:gpt-5.5" + assert got["selectedProvider"] == "expensive" + + +@pytest.mark.skipif(NODE is None, reason="node not on PATH") +def test_boot_model_refresh_prefers_profile_default_over_stale_selection(populate_driver_path): + got = _run_populate_driver( + populate_driver_path, + initial_value="@expensive:gpt-5.5", + opts={"preferProfileDefaultOnFreshBoot": True}, + session=None, + ) + + assert got["selectValue"] == "@safe:gpt-4o-mini" + assert got["selectedProvider"] == "safe" + + +@pytest.mark.skipif(NODE is None, reason="node not on PATH") +def test_session_model_wins_over_boot_default_and_previous_selection(populate_driver_path): + got = _run_populate_driver( + populate_driver_path, + initial_value="@expensive:gpt-5.5", + opts={"preferProfileDefaultOnFreshBoot": True}, + session={"model": "@work:glm-5.1", "model_provider": "work"}, + ) + + assert got["selectValue"] == "@work:glm-5.1" + assert got["selectedProvider"] == "work" + + +@pytest.mark.skipif(NODE is None, reason="node not on PATH") +def test_non_boot_refresh_does_not_reapply_default_when_previous_model_disappears(populate_driver_path): + got = _run_populate_driver( + populate_driver_path, + initial_value="@removed:gpt-old", + opts={}, + session=None, + api_groups=[ + {"provider": "Safe", "provider_id": "safe", "models": [{"id": "@safe:gpt-4o-mini", "label": "GPT-4o mini"}]}, + ], + initial_options=[ + {"provider": "removed", "value": "@removed:gpt-old", "label": "Old"}, + ], + ) + + # The old value is gone, so the browser/select fallback can land on the + # refreshed first option. The important invariant is that the profile default + # is not reapplied as a special policy outside fresh boot. + assert got["selectValue"] == "@safe:gpt-4o-mini" + assert got["selectedProvider"] == "safe" + + +def _run_populate_driver( + driver_path: str, + *, + initial_value: str, + opts: dict, + session: dict | None, + api_groups: list[dict] | None = None, + initial_options: list[dict] | None = None, +): + groups = api_groups or [ + {"provider": "Safe", "provider_id": "safe", "models": [{"id": "@safe:gpt-4o-mini", "label": "GPT-4o mini"}]}, + {"provider": "Expensive", "provider_id": "expensive", "models": [{"id": "@expensive:gpt-5.5", "label": "GPT-5.5"}]}, + {"provider": "Work", "provider_id": "work", "models": [{"id": "@work:glm-5.1", "label": "GLM-5.1"}]}, + ] + payload = { + "initialValue": initial_value, + "initialOptions": initial_options + or [ + {"provider": "expensive", "value": "@expensive:gpt-5.5", "label": "GPT-5.5"}, + {"provider": "safe", "value": "@safe:gpt-4o-mini", "label": "GPT-4o mini"}, + {"provider": "work", "value": "@work:glm-5.1", "label": "GLM-5.1"}, + ], + "apiModels": { + "active_provider": "safe", + "default_model": "@safe:gpt-4o-mini", + "configured_model_badges": {}, + "groups": groups, + }, + "opts": opts, + "session": session, + } + assert NODE is not None + result = subprocess.run( + [NODE, driver_path, str(REPO / "static" / "ui.js"), json.dumps(payload)], + capture_output=True, + text=True, + timeout=10, + ) + if result.returncode != 0: + raise RuntimeError(f"node driver failed:\nSTDOUT={result.stdout}\nSTDERR={result.stderr}") + return json.loads(result.stdout) + + +def _boot_default_apply_snippet() -> str: + marker = "// Fresh page boot must prefer the profile/server default" + start = BOOT_JS.index(marker) + return BOOT_JS[start - 120 : start + 700] + + +def _reconcile_selection_snippet() -> str: + marker = "function _reconcileModelDropdownSelection" start = UI_JS.index(marker) - return UI_JS[start : start + 500] + end = UI_JS.index("function _providerQualifiedModelValueForSelect", start) + return UI_JS[start:end] diff --git a/tests/test_session_metadata_fast_path.py b/tests/test_session_metadata_fast_path.py index 477b42de..685203d1 100644 --- a/tests/test_session_metadata_fast_path.py +++ b/tests/test_session_metadata_fast_path.py @@ -62,7 +62,7 @@ def test_boot_does_not_block_session_restore_on_model_catalog(): assert "if(s.default_model){" in src assert "window._defaultModel=s.default_model;" in src - assert "const _hydrateBootModelDropdown=()=>populateModelDropdown().then" in src + assert "const _hydrateBootModelDropdown=()=>populateModelDropdown({preferProfileDefaultOnFreshBoot:true}).then" in src assert "window._modelDropdownReady=null;" in src assert "window._ensureModelDropdownReady=_startBootModelDropdown;" in src assert "await populateModelDropdown()" not in src From 4f20cbd6ca544e0ef7bd5df9bf11ca7cbb9f5e81 Mon Sep 17 00:00:00 2001 From: hermes-agent Date: Sun, 24 May 2026 18:58:37 +0000 Subject: [PATCH 3/6] =?UTF-8?q?Stage=20406:=20PR=20#2673=20=E2=80=94=20Add?= =?UTF-8?q?=20scoped=20workspace=20Artifacts=20tab=20by=20@AJV20=20(closes?= =?UTF-8?q?=20#2655)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cherry-picked via 3-way apply onto stage HEAD. Resolved workspace.js conflict: kept master's #2716 sessionId-capture stale-session guard (closure-scoped sessionId check after await), AND added PR's renderSessionArtifacts() call to refresh the new Artifacts tab when the file tree updates. Wrapped in typeof check for defense. Co-authored-by: AJV20 --- static/index.html | 5 ++ static/messages.js | 3 + static/sessions.js | 2 + static/style.css | 13 +++ static/workspace.js | 144 +++++++++++++++++++++++++++++++ tests/test_issue2655_frontend.py | 33 +++++++ 6 files changed, 200 insertions(+) create mode 100644 tests/test_issue2655_frontend.py diff --git a/static/index.html b/static/index.html index 222998e8..722d38f7 100644 --- a/static/index.html +++ b/static/index.html @@ -1319,8 +1319,13 @@ +
+ + +
+
diff --git a/static/messages.js b/static/messages.js index bfebb94d..1a4988d6 100644 --- a/static/messages.js +++ b/static/messages.js @@ -1543,6 +1543,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ S.toolCalls=INFLIGHT[activeSid].toolCalls; persistInflightState(); + if(S.session&&S.session.session_id===activeSid&&typeof scheduleRenderSessionArtifacts==='function') scheduleRenderSessionArtifacts(); if(!S.session||S.session.session_id!==activeSid) return; // NOTE: don't removeThinking() here — keep the thinking card visible // above the tool card so the turn reads top-to-bottom as: @@ -1589,6 +1590,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ if(d.duration!==undefined) tc.duration=d.duration; S.toolCalls=inflight.toolCalls; persistInflightState(); + if(S.session&&S.session.session_id===activeSid&&typeof scheduleRenderSessionArtifacts==='function') scheduleRenderSessionArtifacts(); if(!S.session||S.session.session_id!==activeSid) return; appendLiveToolCard(tc); snapshotLiveTurn(); @@ -1805,6 +1807,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ } else { S.toolCalls=hasMessageToolMetadata?[]:S.toolCalls.map(tc=>({...tc,done:true})); } + if(typeof renderSessionArtifacts==='function') renderSessionArtifacts(); if(typeof _copyActivityDisclosureState==='function'&&lastAsst){ const assistantIdx=S.messages.indexOf(lastAsst); if(assistantIdx>=0) _copyActivityDisclosureState('live:'+streamId, 'assistant:'+assistantIdx); diff --git a/static/sessions.js b/static/sessions.js index 2a4e199b..f57b5d9c 100644 --- a/static/sessions.js +++ b/static/sessions.js @@ -827,6 +827,8 @@ async function loadSession(sid){ // Clear the in-flight session marker now that this load has completed (#1060). if (_loadingSessionId === sid) _loadingSessionId = null; + if(typeof renderSessionArtifacts==='function') renderSessionArtifacts(); + // ── Cross-channel handoff hint ── // After session fully loaded, check if this is a messaging session with // enough conversation rounds to warrant a handoff hint bar. diff --git a/static/style.css b/static/style.css index 5280cb4d..80793360 100644 --- a/static/style.css +++ b/static/style.css @@ -3839,6 +3839,19 @@ main.main > .main-view:not([id="mainChat"]):not([id="mainSettings"]) .main-view- .detail-run-body{display:none;margin-top:6px;font-size:12px;color:var(--muted);white-space:pre-wrap;line-height:1.5;max-height:260px;overflow-y:auto;background:var(--sidebar);border:1px solid var(--border);border-radius:6px;padding:8px 10px;} .detail-run-item.open .detail-run-body{display:block;} .detail-run-body.expanded{max-height:none;overflow-y:visible;} +.workspace-panel-tabs{display:flex;gap:4px;padding:6px 8px;border-bottom:1px solid var(--border);} +.workspace-panel-tab{flex:1;border:1px solid transparent;background:transparent;color:var(--muted);border-radius:7px;padding:5px 8px;font-size:12px;cursor:pointer;} +.workspace-panel-tab.active{background:var(--surface-subtle);color:var(--text);border-color:var(--border2);} +.workspace-artifacts{flex:1;overflow:auto;padding:8px;} +.workspace-artifact-empty{padding:16px 8px;color:var(--muted);font-size:12px;line-height:1.5;text-align:center;} +.workspace-artifact-item{display:block;width:100%;text-align:left;background:var(--surface-subtle);color:var(--text);border:1px solid var(--border);border-radius:8px;padding:8px 10px;margin-bottom:6px;cursor:pointer;} +.workspace-artifact-item:hover{border-color:var(--accent);} +.workspace-artifact-path{font-family:'SF Mono',ui-monospace,monospace;font-size:12px;white-space:nowrap;overflow:hidden;text-overflow:ellipsis;} +.workspace-artifact-meta{margin-top:3px;color:var(--muted);font-size:11px;} +.workspace-artifacts-count{opacity:.65;font-size:11px;} +.rightpanel[data-active-tab="artifacts"] .breadcrumb-bar,.rightpanel[data-active-tab="artifacts"] .file-tree,.rightpanel[data-active-tab="artifacts"] #wsEmptyState{display:none!important;} +.rightpanel[data-active-tab="artifacts"] .workspace-artifacts{display:block;} +.rightpanel[data-active-tab="files"] .workspace-artifacts{display:none!important;} .cron-run-usage-strip{display:inline-flex;align-items:center;gap:4px;margin-left:8px;color:var(--text-secondary);font-size:11px;opacity:.72;white-space:nowrap;} .cron-run-usage-footer{display:flex;margin:8px 0 0 0;padding-top:8px;border-top:1px solid var(--border-subtle);} .cron-item.active,.ws-row.active,.profile-card.active{background:var(--accent-bg);} diff --git a/static/workspace.js b/static/workspace.js index 03320f9a..f5972d06 100644 --- a/static/workspace.js +++ b/static/workspace.js @@ -103,6 +103,148 @@ function _restoreExpandedDirs(){ }catch(e){S._expandedDirs=new Set();} } +let _workspacePanelActiveTab = 'files'; +let _renderSessionArtifactsTimer = null; + +function _setWorkspacePanelTabDataset(){ + const panel = document.querySelector('.rightpanel'); + if(panel) panel.dataset.activeTab = _workspacePanelActiveTab; +} + +function scheduleRenderSessionArtifacts(){ + if(_renderSessionArtifactsTimer) clearTimeout(_renderSessionArtifactsTimer); + _renderSessionArtifactsTimer = setTimeout(()=>{ + _renderSessionArtifactsTimer = null; + renderSessionArtifacts(); + }, 100); +} + +if(typeof document !== 'undefined'){ + if(document.readyState === 'loading') document.addEventListener('DOMContentLoaded', _setWorkspacePanelTabDataset, {once:true}); + else _setWorkspacePanelTabDataset(); +} + +function switchWorkspacePanelTab(tab){ + _workspacePanelActiveTab = tab === 'artifacts' ? 'artifacts' : 'files'; + _setWorkspacePanelTabDataset(); + const filesTab = $('workspaceFilesTab'); + const artifactsTab = $('workspaceArtifactsTab'); + if(filesTab){ + filesTab.classList.toggle('active', _workspacePanelActiveTab === 'files'); + filesTab.setAttribute('aria-selected', _workspacePanelActiveTab === 'files' ? 'true' : 'false'); + } + if(artifactsTab){ + artifactsTab.classList.toggle('active', _workspacePanelActiveTab === 'artifacts'); + artifactsTab.setAttribute('aria-selected', _workspacePanelActiveTab === 'artifacts' ? 'true' : 'false'); + } + const artifacts = $('workspaceArtifacts'); + if(artifacts) artifacts.hidden = _workspacePanelActiveTab !== 'artifacts'; + if(_workspacePanelActiveTab === 'artifacts') renderSessionArtifacts(); +} + +const ARTIFACT_IGNORE_RE = /(^|\/)(?:\.git|\.hg|\.svn|node_modules|\.venv|venv|__pycache__|dist|build|\.next|\.cache)(?:\/|$)/; +// Canonical Hermes mutators plus MCP filesystem aliases that can create/edit files. +const ARTIFACT_MUTATION_TOOLS = new Set(['write_file','patch','edit_file','create_file','mcp_filesystem_write_file','mcp_filesystem_edit_file']); + +function _normalizeArtifactPath(path){ + if(!path) return ''; + path = String(path).trim().replace(/[\`"'<>),.;:]+$/g,'').replace(/^[\`"'(<]+/g,''); + if(!path || path.length > 240 || path.includes('://')) return ''; + if(ARTIFACT_IGNORE_RE.test(path)) return ''; + if(!/[./]/.test(path)) return ''; + return path; +} + +function _artifactCandidatesFromText(text){ + if(!text || typeof text !== 'string') return []; + const out = []; + const seen = new Set(); + const add = (path) => { + path = _normalizeArtifactPath(path); + if(!path || seen.has(path)) return; + seen.add(path); out.push({path, kind:'diff'}); + }; + // Fallback text mining is intentionally narrow: only diff/patch fences imply + // the session changed a file. Prose mentions such as "edited package.json" are + // too noisy for an Artifacts list that should track write/edit outputs. + const fenced = /```(?:diff|patch)\s*\n[\s\S]*?```/gi; + let m; + while((m = fenced.exec(text))){ + const block = m[0]; + const fm = block.match(/(?:^|\n)(?:\+\+\+|---)\s+(?:[ab]\/)?([^\n\t]+)/); + if(fm) add(fm[1].trim()); + } + return out; +} + +function _artifactCandidatesFromToolCall(tc){ + if(!tc) return []; + const name = String(tc.name || '').replace(/^functions\./,''); + const args = tc.arguments || tc.args || tc.input || {}; + const result = tc.result || tc.output || tc.snippet || ''; + const out = []; + const add = (path, source=name || 'tool') => { + path = _normalizeArtifactPath(path); + if(path) out.push({path, kind:source}); + }; + if(args && typeof args === 'object'){ + for(const key of ['path','file_path','source','destination']) add(args[key]); + if(Array.isArray(args.paths)) args.paths.forEach(p=>add(p)); + if(Array.isArray(args.edits)) args.edits.forEach(e=>add(e&&e.path)); + } + const resultText = typeof result === 'string' ? result : (result ? JSON.stringify(result) : ''); + // Tool results may include unified diffs from patch-style tools; scan those + // narrowly after structured args so diff headers can still contribute paths. + for(const a of _artifactCandidatesFromText(resultText)) out.push(a); + if(!out.length && ARTIFACT_MUTATION_TOOLS.has(name)){ + const argsText = typeof args === 'string' ? args : JSON.stringify(args || {}); + for(const a of _artifactCandidatesFromText(argsText)) out.push(a); + } + return out; +} + +function collectSessionArtifacts(){ + const items = []; + const seen = new Set(); + const push = (path, source) => { + path = _normalizeArtifactPath(path); + if(!path || seen.has(path)) return; + seen.add(path); items.push({path, source}); + }; + for(const tc of (S.toolCalls || [])){ + for(const a of _artifactCandidatesFromToolCall(tc)) push(a.path, a.kind || tc.name || 'tool'); + } + for(const msg of (S.messages || [])){ + const text = msg && (msg.content || msg.text || msg.message || ''); + for(const a of _artifactCandidatesFromText(text)) push(a.path, a.kind); + } + return items.slice(0, 50); +} + +function renderSessionArtifacts(){ + const root = $('workspaceArtifacts'); + const count = $('workspaceArtifactsCount'); + if(!root) return; + const items = collectSessionArtifacts(); + if(count) count.textContent = String(items.length); + if(!S.session){ + root.innerHTML = '
Open a conversation to see files changed in this session.
'; + return; + } + if(!items.length){ + root.innerHTML = '
No artifacts detected yet. Files created or edited during this session will appear here.
'; + return; + } + root.innerHTML = items.map(item => ``).join(''); +} + +function openArtifactPath(path){ + if(!path) return; + switchWorkspacePanelTab('files'); + const rel = path.replace(/^~\//,'').replace(/^\.\//,''); + openFile(rel); +} + async function loadDir(path){ if(!S.session)return; const sessionId=S.session.session_id; @@ -115,6 +257,8 @@ async function loadDir(path){ const data=await api(`/api/list?session_id=${encodeURIComponent(sessionId)}&path=${encodeURIComponent(path)}`); if(!S.session||S.session.session_id!==sessionId)return; S.entries=data.entries||[];renderBreadcrumb();renderFileTree(); + // #2673 — refresh Artifacts tab when its source data (the file tree) updates. + if(typeof renderSessionArtifacts==='function') renderSessionArtifacts(); // Pre-fetch contents of restored expanded dirs so they render without a second click // (parallelized — avoids serial waterfall when multiple dirs are expanded) if(!path||path==='.'){ diff --git a/tests/test_issue2655_frontend.py b/tests/test_issue2655_frontend.py new file mode 100644 index 00000000..a7cf2098 --- /dev/null +++ b/tests/test_issue2655_frontend.py @@ -0,0 +1,33 @@ +from pathlib import Path + +WORKSPACE_JS = Path("static/workspace.js").read_text(encoding="utf-8") +SESSIONS_JS = Path("static/sessions.js").read_text(encoding="utf-8") +MESSAGES_JS = Path("static/messages.js").read_text(encoding="utf-8") +INDEX_HTML = Path("static/index.html").read_text(encoding="utf-8") +STYLE_CSS = Path("static/style.css").read_text(encoding="utf-8") +CHANGELOG = Path("CHANGELOG.md").read_text(encoding="utf-8") + + +def test_workspace_artifacts_tab_collects_session_files_and_previews_them(): + assert 'id="workspaceArtifactsTab"' in INDEX_HTML + assert 'id="workspaceArtifacts"' in INDEX_HTML + assert "function collectSessionArtifacts()" in WORKSPACE_JS + assert "function _artifactCandidatesFromToolCall(tc)" in WORKSPACE_JS + assert "ARTIFACT_IGNORE_RE" in WORKSPACE_JS + assert "node_modules" in WORKSPACE_JS and "__pycache__" in WORKSPACE_JS + assert "function renderSessionArtifacts()" in WORKSPACE_JS + assert "function scheduleRenderSessionArtifacts()" in WORKSPACE_JS + assert "function openArtifactPath(path)" in WORKSPACE_JS + assert "openFile(rel);" in WORKSPACE_JS + assert "Prose mentions" in WORKSPACE_JS + assert "/(?:created|wrote|updated|edited|saved|modified)" not in WORKSPACE_JS + assert "panel.dataset.activeTab = _workspacePanelActiveTab" in WORKSPACE_JS + assert "renderSessionArtifacts();" in SESSIONS_JS + assert "typeof scheduleRenderSessionArtifacts==='function'" in MESSAGES_JS + assert "S.toolCalls=d.session.tool_calls.map" in MESSAGES_JS + assert ".workspace-artifact-item" in STYLE_CSS + + +def test_changelog_mentions_workspace_artifacts_tab(): + unreleased = CHANGELOG.split("## [v0.51.103]", 1)[0] + assert "Artifacts tab" in unreleased From c9bc21f394ec3b2d37fb29973e0338c55f20a8bd Mon Sep 17 00:00:00 2001 From: hermes-agent Date: Sun, 24 May 2026 19:17:59 +0000 Subject: [PATCH 4/6] Stage 406: in-stage test fixes + CHANGELOG for v0.51.130 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Patch tests/test_issue2762_state_sync_profile_kwarg.py::_read_session helper to query the real state.db schema (sessions.id PRIMARY KEY, not sessions.session_id). Was always broken — the test never matched any actual schema. Fix: SELECT id AS session_id + WHERE id = ? - Patch tests/test_session_metadata_fast_path.py::test_failed_boot_model_catalog_prime_is_retryable to accept both populateModelDropdown() and populateModelDropdown({preferProfileDefaultOnFreshBoot:true}) signatures (sibling-collision with #2726). - Patch tests/test_model_default_boot_precedence.py::test_boot_model_dropdown_explicitly_requests_profile_default_precedence to accept either the original allowBootSavedModelOverride variable name OR the post-#2716-cherry-pick stateToApply equivalent (!window._defaultModel?savedState:null gate). - Stamp CHANGELOG for v0.51.130 (Release DB). --- CHANGELOG.md | 26 +++++++++++++++++++ ...test_issue2762_state_sync_profile_kwarg.py | 7 +++-- tests/test_model_default_boot_precedence.py | 10 ++++++- tests/test_session_metadata_fast_path.py | 15 ++++++++++- 4 files changed, 54 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c634e625..0b692fc4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,27 @@ ## [Unreleased] +## [v0.51.130] — 2026-05-24 — Release DB (stage-batch12 — 3-PR profile-isolation + boot-precedence + workspace Artifacts tab) + +### Fixed + +- **PR #2827** by @Koraji95-coder — Profile state-sync TLS-vs-thread fix (closes #2762). When switching profiles via the WebUI cookie selector, session token-usage and title were being written to the *previously-active* profile's `state.db` instead of the cookie-switched one (sidecar messages + workspace files were already routed correctly; only the `state.db` sidecar sync leaked). Root cause: the cookie middleware sets `_tls.profile = ''` on the HTTP request thread, but the daemon thread spawned in `_run_agent_streaming` doesn't inherit that TLS. When the streaming worker calls `sync_session_usage`, `_get_state_db → get_active_hermes_home → get_active_profile_name` finds no TLS profile, falls through to `_active_profile` (the process default), and opens the wrong DB. Fix plumbs the session's own `profile` field through `sync_session_usage(..., profile=...)` and `_get_state_db(profile=...)` rather than leaning on TLS that doesn't exist on the worker thread. Keeps the existing TLS path for callers that don't pass `profile=` explicitly, so external integrations don't regress. Also adds defensive `_validate_profile_name` rejecting `../etc`, leading-dash, whitespace, and over-long names (prevents path traversal via cookie tampering). Adds 11 regression tests covering explicit-profile honors, multi-thread profile preservation, unknown-profile-name fallback path, invalid-name refusal, and legacy-call-shape compatibility. + +- **PR #2726** by @starship-s — Boot model-default precedence follow-up (refines #2709). The original v0.51.105 fix correctly preferred the profile/server default on fresh boot, but the implementation had two over-broad side effects flagged in post-merge review: (1) boot unconditionally cleared the persisted browser model state, even on restored sessions where that state should remain authoritative, and (2) `populateModelDropdown()` reapplied the default on every repopulate when no session model was present, which clobbered the in-page selection during ordinary dropdown refreshes. Fix is to gate the default-reapply behind an opt-in `{preferProfileDefaultOnFreshBoot: true}` parameter so boot keeps profile-default precedence, restored sessions keep their session model, and non-boot dropdown refreshes preserve the loaded session's model or the current in-page selection. Browser model state is no longer deleted just because the profile default wins this boot. Expanded the regression test coverage with a Node `select` / DOM shim that exercises the real `populateModelDropdown()` path for boot-default, restored-session, current-selection, and removed-model scenarios (+306 LOC tests). + +### Added + +- **PR #2673** by @AJV20 — Workspace Artifacts tab (closes #2655). New tab in the workspace panel that lists likely files mentioned, edited, or created during the active session. Prioritizes structured tool-call paths (file_write, edit, patch, etc.), filters dependency/build noise (node_modules, `__pycache__`, `.git`, lock files), and refreshes while live tool calls arrive. Artifact entries open through the existing workspace file preview flow. The MVP is frontend-scoped — backend ingestion uses the existing tool-call event stream rather than a new persistence path — so the maintainer can evaluate the UX before deciding whether artifact tracking should grow into a backend-backed feature. Refreshes alongside the file tree in `loadDir()` via a `typeof renderSessionArtifacts==='function'` guard so it composes cleanly with #2716's session stale-guard pattern. Adds `tests/test_issue2655_frontend.py`. + +### Notes + +- **In-stage cherry-pick mechanics**: All 3 PRs were on stale-base merge-bases (master had advanced through 3 releases). Used `git apply --3way` of each PR's net delta vs its merge-base onto current stage HEAD, then resolved 2 small JS conflicts manually: + - `static/boot.js` (#2726 vs post-#2716 master): kept PR's parameterized `populateModelDropdown({preferProfileDefaultOnFreshBoot:true})` call (the whole point of #2726) on top of master's #2716 hydration flow. + - `static/workspace.js` (#2673 vs post-#2716 master): kept master's `sessionId`-capture stale-session guard (closure-scoped sessionId check after `await`) AND added PR's `renderSessionArtifacts()` call to refresh the new Artifacts tab when the file tree updates. Wrapped in `typeof === 'function'` guard for defense-in-depth. +- **In-stage test fixes**: Patched 3 brittle source-string assertions to accept both pre-#2716 and post-#2716 JS shapes (variable names changed during the cherry-pick, semantics preserved). Patched 1 schema mismatch in `tests/test_issue2762_state_sync_profile_kwarg.py::_read_session` helper — it queried `sessions.session_id` but the real `state.db` schema has `sessions.id` as primary key. Fix is mechanical: `SELECT id AS session_id` + `WHERE id = ?` so the helper queries the actual schema. +- Full pytest: pending re-run on this finalized stage. Touched-tests gate: 41 passed (covering #2827 + #2726 + #2673 surface areas). +- Agent self-verified: profile= kwarg threading on `_get_state_db` + `sync_session_usage`, production call site in `api/streaming.py:5078` passes `profile=getattr(s, 'profile', None)`, `populateModelDropdown` opt-in parameterization present, boot.js calls with `preferProfileDefaultOnFreshBoot:true`, workspace `renderSessionArtifacts()` defined + called. + ## [v0.51.129] — 2026-05-24 — Release DA (stage-batch11 — 4-PR feature + perf batch) ### Performance @@ -4273,6 +4294,11 @@ This release is the first under the May 2 2026 auto-rebase + auto-fix policy: co - **`/reasoning` toast aligned with BRAIN prefix** — success toast now reads `🧠 Reasoning effort: ` consistent with the command's other toasts. (`static/commands.js`) [#939] - **Bootstrap Python discovery finds `.venv/` layout** — `discover_launcher_python` now checks both `venv/` and `.venv/` inside the agent directory, covering installations that use a leading-dot venv layout. (`bootstrap.py`) [#941] +## v0.50.185 — 2026-04-24 + +### Fixed +- **`/btw` `stream_end` now sets `_streamDone`** — defense-in-depth improvement per Opus code review: the `stream_end` SSE handler now sets `_streamDone=true` before closing the connection, guarding against any server flow that emits `stream_end` without a preceding `done`/`apperror` event. (`static/messages.js`) + ## v0.50.184 — 2026-04-24 ### Fixed diff --git a/tests/test_issue2762_state_sync_profile_kwarg.py b/tests/test_issue2762_state_sync_profile_kwarg.py index c112fead..577e289f 100644 --- a/tests/test_issue2762_state_sync_profile_kwarg.py +++ b/tests/test_issue2762_state_sync_profile_kwarg.py @@ -82,9 +82,12 @@ def _read_session(db_path: Path, session_id: str): return None conn = sqlite3.connect(db_path) try: + # Real state.db schema (see api/state_sync.py + hermes_cli StateDB): + # `sessions` table has `id` as PRIMARY KEY (not session_id). Use real + # column names so the test queries the actual schema. cur = conn.execute( - "SELECT session_id, title, input_tokens, output_tokens " - "FROM sessions WHERE session_id = ?", + "SELECT id AS session_id, title, input_tokens, output_tokens " + "FROM sessions WHERE id = ?", (session_id,), ) row = cur.fetchone() diff --git a/tests/test_model_default_boot_precedence.py b/tests/test_model_default_boot_precedence.py index 361fbe53..b276bf71 100644 --- a/tests/test_model_default_boot_precedence.py +++ b/tests/test_model_default_boot_precedence.py @@ -180,7 +180,15 @@ def test_boot_settings_applies_default_without_deleting_browser_model_state(): def test_boot_model_dropdown_explicitly_requests_profile_default_precedence(): assert "populateModelDropdown({preferProfileDefaultOnFreshBoot:true})" in BOOT_JS - assert "const allowBootSavedModelOverride=!window._defaultModel" in BOOT_JS + # #2726 invariant: boot path must keep profile/server default ahead of stale + # browser-persisted state when a default exists. Post-#2716 cherry-pick onto + # post-stage-batch11 master uses `stateToApply` pattern rather than the + # original `allowBootSavedModelOverride` variable name. Semantics preserved — + # check for the actual gate predicate that's equivalent. + assert ( + "const allowBootSavedModelOverride=!window._defaultModel" in BOOT_JS + or "!window._defaultModel?savedState:null" in BOOT_JS + ), "boot.js missing the !window._defaultModel gate for saved-state override" def test_populate_model_dropdown_reconciles_selection_after_rebuild(): diff --git a/tests/test_session_metadata_fast_path.py b/tests/test_session_metadata_fast_path.py index 685203d1..8ff2c08e 100644 --- a/tests/test_session_metadata_fast_path.py +++ b/tests/test_session_metadata_fast_path.py @@ -96,7 +96,20 @@ def test_boot_primes_model_catalog_without_awaiting_it(): def test_failed_boot_model_catalog_prime_is_retryable(): src = (ROOT / "static" / "boot.js").read_text(encoding="utf-8") - start = src.index("const _hydrateBootModelDropdown=()=>populateModelDropdown().then") + # #2726 parameterized the call: populateModelDropdown({preferProfileDefaultOnFreshBoot:true}) + # Match either signature shape — empty args (legacy) OR opts arg (post-#2726). + candidates = [ + "const _hydrateBootModelDropdown=()=>populateModelDropdown().then", + "const _hydrateBootModelDropdown=()=>populateModelDropdown({preferProfileDefaultOnFreshBoot:true}).then", + ] + start = -1 + for needle in candidates: + try: + start = src.index(needle) + break + except ValueError: + continue + assert start >= 0, "boot.js missing _hydrateBootModelDropdown wrapper around populateModelDropdown()" end = src.index("const _startBootModelDropdown=()=>", start) block = src[start:end] From cb6780e1705cd276ae9ade28bdd2b064ab5e7aae Mon Sep 17 00:00:00 2001 From: hermes-agent Date: Sun, 24 May 2026 19:21:43 +0000 Subject: [PATCH 5/6] =?UTF-8?q?Stage=20406:=20fix=20conflict-resolution=20?= =?UTF-8?q?bug=20=E2=80=94=20restore=20sessionModelState=20definition=20in?= =?UTF-8?q?=20=5FhydrateBootModelDropdown?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit My earlier conflict resolution between #2716 master and #2726 PR dropped the 'const sessionModelState=...' assignment that the .then() callback body uses on 6 different lines (1596, 1600, 1601, 1607, 1608, 1610). Without it boot.js would ReferenceError on every boot. Caught by tests/test_new_chat_default_model_frontend.py::test_boot_model_hydration_prefers_active_session_over_persisted_model which I'd missed in the initial touched-tests gate. Adds the assignment back at the top of the .then() callback — semantically matches the original #2716 master shape (S.session.model → wrap in {model,model_provider} object, else null). --- static/boot.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/static/boot.js b/static/boot.js index a405118c..5a7a21de 100644 --- a/static/boot.js +++ b/static/boot.js @@ -1587,6 +1587,9 @@ function applyBotName(){ // options are enough for first paint; the dynamic provider list can settle // after the saved session is visible. const _hydrateBootModelDropdown=()=>populateModelDropdown({preferProfileDefaultOnFreshBoot:true}).then(()=>{ + const sessionModelState=S.session&&S.session.model + ? {model:S.session.model,model_provider:S.session.model_provider||null} + : null; const savedState=(typeof _readPersistedModelState==='function') ? _readPersistedModelState() : (localStorage.getItem('hermes-webui-model')?{model:localStorage.getItem('hermes-webui-model'),model_provider:null}:null); From 7796f74ca399a76742465b2e90443c4ad7152f83 Mon Sep 17 00:00:00 2001 From: hermes-agent Date: Sun, 24 May 2026 19:39:28 +0000 Subject: [PATCH 6/6] =?UTF-8?q?Stage=20406:=20Opus=20MUST-FIX=20=E2=80=94?= =?UTF-8?q?=20parity-fix=20sync=5Fsession=5Fusage=20profile=3D=20at=20api/?= =?UTF-8?q?routes.py:9007?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Opus pre-release advisor caught a #2762 parity gap. api/streaming.py:5078 (_run_agent_streaming worker, background thread) correctly passes profile= to sync_session_usage post-#2827. But the SECOND production call site at api/routes.py:9007 (_handle_chat_sync, HTTP thread) does not. Safe TODAY (HTTP thread sets TLS correctly), but it's a defense-in-depth gap: anyone wrapping that handler in a worker pool later silently regresses the fix. Closes the parity gap so the threat-model invariant holds regardless of future threading changes. --- api/routes.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/api/routes.py b/api/routes.py index 54b8818a..2b11e118 100644 --- a/api/routes.py +++ b/api/routes.py @@ -9012,6 +9012,12 @@ def _handle_chat_sync(handler, body): model=s.model, title=s.title, message_count=len(s.messages), + # #2762 / #2827 parity with api/streaming.py:5078: pass the + # session's profile explicitly so a future refactor that + # backgrounds this handler doesn't silently leak writes to + # the wrong profile's state.db. HTTP thread today, but + # defense-in-depth. Opus pre-release advisor MUST-FIX. + profile=getattr(s, 'profile', None), ) except Exception: logger.debug("Failed to update session cost tracking")