Merge remote-tracking branch 'origin/master' into maint/pr-2865
# Conflicts: # CHANGELOG.md
This commit is contained in:
commit
6e2991f45a
15 files changed
+956
-44
No files matched your search
@@ -7,6 +7,27 @@
|
||||
|
||||
- New WebUI sessions no longer persist `display.personality` into per-session `Session.personality`; only explicit personality changes remain durable, preventing stale global display defaults from overriding profile-scoped session behavior. Closes #2845.
|
||||
|
||||
## [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 = '<cookie>'` 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
|
||||
@@ -4277,6 +4298,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: <level>` 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
|
||||
|
||||
@@ -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")
|
||||
|
||||
+83
-14
@@ -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:
|
||||
|
||||
@@ -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")
|
||||
|
||||
+3
-7
@@ -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,7 +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 _hydrateBootModelDropdown=()=>populateModelDropdown({preferProfileDefaultOnFreshBoot:true}).then(()=>{
|
||||
const sessionModelState=S.session&&S.session.model
|
||||
? {model:S.session.model,model_provider:S.session.model_provider||null}
|
||||
: null;
|
||||
|
||||
@@ -1319,8 +1319,13 @@
|
||||
<button class="panel-icon-btn close-preview has-tooltip has-tooltip--bottom" id="btnClearPreview" data-tooltip="Close preview"><svg width="14" height="14" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" aria-hidden="true"><line x1="18" y1="6" x2="6" y2="18"/><line x1="6" y1="6" x2="18" y2="18"/></svg></button>
|
||||
</div>
|
||||
</div>
|
||||
<div class="workspace-panel-tabs" role="tablist" aria-label="Workspace panel views">
|
||||
<button class="workspace-panel-tab active" id="workspaceFilesTab" type="button" onclick="switchWorkspacePanelTab('files')" role="tab" aria-selected="true">Files</button>
|
||||
<button class="workspace-panel-tab" id="workspaceArtifactsTab" type="button" onclick="switchWorkspacePanelTab('artifacts')" role="tab" aria-selected="false">Artifacts <span id="workspaceArtifactsCount" class="workspace-artifacts-count">0</span></button>
|
||||
</div>
|
||||
<div class="breadcrumb-bar" id="breadcrumbBar" style="display:none"></div>
|
||||
<div class="file-tree" id="fileTree"></div>
|
||||
<div class="workspace-artifacts" id="workspaceArtifacts" hidden></div>
|
||||
<div id="wsEmptyState" style="display:none;flex:1;align-items:center;justify-content:center;padding:24px 16px;text-align:center;color:var(--muted);font-size:12px;line-height:1.6"></div>
|
||||
<div class="preview-area" id="previewArea">
|
||||
<div class="preview-path" id="previewPath">
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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);}
|
||||
|
||||
+30
-7
@@ -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'){
|
||||
|
||||
@@ -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 = '<div class="workspace-artifact-empty">Open a conversation to see files changed in this session.</div>';
|
||||
return;
|
||||
}
|
||||
if(!items.length){
|
||||
root.innerHTML = '<div class="workspace-artifact-empty">No artifacts detected yet. Files created or edited during this session will appear here.</div>';
|
||||
return;
|
||||
}
|
||||
root.innerHTML = items.map(item => `<button type="button" class="workspace-artifact-item" data-artifact-path="${esc(item.path)}" onclick="openArtifactPath(this.dataset.artifactPath)"><div class="workspace-artifact-path">${esc(item.path)}</div><div class="workspace-artifact-meta">${esc(item.source || 'session')}</div></button>`).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==='.'){
|
||||
|
||||
@@ -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
|
||||
@@ -0,0 +1,287 @@
|
||||
"""
|
||||
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:
|
||||
# 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 id AS session_id, title, input_tokens, output_tokens "
|
||||
"FROM sessions WHERE 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"
|
||||
)
|
||||
@@ -3,32 +3,318 @@ 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
|
||||
# #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 _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]
|
||||
@@ -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
|
||||
@@ -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]
|
||||
|
||||
|
||||
Reference in new issue
Block a user