diff --git a/CHANGELOG.md b/CHANGELOG.md index 216789ba..35ec1c2d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,10 +5,20 @@ ### Fixed +- **Two WebUI sessions with the same auto-generated title no longer leave the second one blank.** Follow-up to the `state.db` title sync: hermes-agent enforces a uniqueness rule on session titles, so when two sessions independently generated a byte-identical auto-title, the second sync raised (and silently swallowed) a collision error and left that session untitled in `hermes sessions list`. The sync now catches the collision, derives a de-duplicated variant via the existing lineage helper (`"My Session"` → `"My Session #2"`), and retries — and because the retry goes through the same provenance guard, a title you set manually is still never overwritten. Thanks @webtecnica. (#6964) +- **Custom-provider models keep their exact identity across catalog refreshes.** When a session used a model served through a custom provider whose upstream id contains a namespace (e.g. `z-ai/glm-5.2` under a `tokenrouter` custom endpoint), a catalog refresh could mis-resolve the picker selection — matching a *different* model whose bare suffix collided, or (in an interim fix) fabricating a new bare-id option instead of re-selecting the real one. Model resolution now matches the exact routed id first, and only falls back to a provider-hinted bare-id repair (preserving the older #6195 behavior for legacy sessions that stored just the suffix) when there is exactly one unambiguous candidate — otherwise it declines to guess. The resolved value is always an existing dropdown option, so re-renders no longer inject duplicates or lose the selection. Thanks @josephcy95. (#6944) +- **WebUI no longer crashes a send when running against an older hermes-agent.** The streaming path passed `persist_user_timestamp=` to `agent.run_conversation()` unconditionally, so a WebUI paired with an older agent whose `run_conversation()` predates that keyword raised `TypeError: unexpected keyword argument` and the message failed. The three call sites now check the callable's signature first (via a small `_supports_kwarg` introspection helper) and only pass the timestamp when the agent accepts it — the same defensive pattern already used for `moa_config`. Modern agents are unaffected (the timestamp is still passed with the identical value); older agents degrade gracefully instead of erroring. Thanks @harsh2hell. (#6935) +- **`hermes sessions list` no longer shows blank titles for WebUI sessions.** Background title generation wrote the auto-title to the WebUI session sidecar but never to hermes-agent's `state.db`, so the CLI/TUI session list showed those WebUI sessions untitled. The title is now bridged into `state.db` after it's generated (and on adaptive refresh) via `set_auto_title_if_empty`, which respects title provenance — a name you set manually via CLI/Gateway/TUI (`user` source) is never overwritten by the auto-title, an already-auto-titled row stays put, and an existing session's `source` is preserved (the sync creates the row with INSERT-OR-IGNORE semantics). Thanks @liuguangyong93. (#6892) +- **No more phantom "Compressing context" barrier after switching sessions.** The compression UI state is per-session, but `loadSession()` did not clear it on a switch, so a compression card from a prior session could leak across the load and appear as a phantom "Compressing context" barrier on a fresh session that never triggered compression. Switching sessions now clears the compression UI (state, elapsed timer, composer session-lock, and barrier element) alongside the other per-session resets, before the transcript loads. A compression genuinely running on the session you switch *to* is unaffected — it re-surfaces from that session's own live event stream (and manual compression from its server job status). Thanks @happy5318. (#6938) + - **Layout test helper no longer flags scroll-reachable fields as off-viewport.** The shared `assert_layout_sane` degenerate check reported a false "interactive element is off-viewport" for an input that sits below the fold at rest but is reachable by scrolling an `overflow-y:auto` ancestor (e.g. a tall dialog on a short landscape viewport). It now exempts elements whose off-viewport edge is absorbed by a scrollable ancestor, while still flagging genuinely-unreachable elements (no scroll escape). The Kanban board settings modal layout test also drops its synthetic 480×320 short-landscape case (a viewport no real device uses, whose sub-fold geometry was environment-marginal and CI-flaky); real desktop/tablet/narrow viewports still assert the layout. (internal test infra) ### Changed +- **Slash-command autocomplete now searches skills by keyword.** Typing `/` and a term now matches plain skills on a case-insensitive keyword in their **name or description** (previously only a name prefix), so a skill is discoverable by what it does, not just its exact slug. Built-in, agent/plugin, and bundle commands keep prefix matching and always take precedence over a same-slug skill, and skill suggestions are held until the bundle-metadata request settles so a slow response can't briefly surface a shadowed skill. Thanks @MinhoJJang. (#6932) + +- **Extensions can observe turn lifecycle events via `handle.events.on(...)`.** A registered extension (see `window.hermesExt.register`) can now subscribe to session turn-lifecycle events — `turn:start`, `turn:complete`, `turn:error`, and `turn:cancel` — through a frozen `events` accessor on its handle, each delivering a frozen scalar event (`sessionId`, `streamId`, `status`, and start/end timestamps; no message content). Core owns the authoritative live-session stream and emits each event exactly once per real transition (a per-session/stream state machine suppresses duplicate starts and post-terminal events, including on reconnect/replay), with per-listener error isolation so a misbehaving extension cannot break streaming. Subscriptions return an idempotent unsubscribe. This is the B1 observer surface on top of the E0 identity handle — a stable page-local facade instead of extensions polling private DOM state. Thanks @franksong2702. (#6924) + - **Extensions can claim a cooperative page identity via `window.hermesExt.register(id)`.** An injected extension can now claim a stable browser-page identity together with its existing settings and storage accessors: `register(id)` returns a frozen `{id, settings, storage}` handle for an ID present in the effective-enabled, Core-sanitized manifest inventory captured at page boot, and returns `null` (fail-closed, no namespace side effects) for empty, malformed, unknown, or untrusted IDs. Re-registering the same ID in one page returns the same handle; boot-time trust is immutable across status refreshes (a refresh cannot elevate an ID absent at boot). This is cooperative attribution for extensions sharing the page — explicitly not a sandbox, capability, permission, isolation mechanism, or security boundary. Thanks @franksong2702. (#6795) - **Refine selected transcript text directly in the composer.** Selecting text inside a chat message now shows a "Refine" action alongside "Reply with selection" in the floating selection toolbar. Refine seeds the selected text into the composer as an editable draft (a quoted block plus a localized "Refine instruction:" line, caret ready) so you can shape a follow-up before sending — the ChatGPT/Notion-style quote-to-edit flow. The transfer is one-way and marker-free (no internal sentinel reaches the composer or the sent message), preserves any existing draft and attachments, collapses to 44×44 icon-only buttons on phones, and covers all 15 locales. Thanks @rodboev. (#6410) diff --git a/README.md b/README.md index e121ec52..ad139126 100644 --- a/README.md +++ b/README.md @@ -294,6 +294,7 @@ If an AI assistant is helping with install, reinstall, bootstrap, provider setup ### Slash commands - Type `/` in the composer for autocomplete dropdown +- Plain skills match case-insensitive keywords in their name or description; built-in, agent/plugin, and bundle commands keep prefix matching and take precedence over a same-slug skill - Built-in: `/help`, `/clear`, `/compress [focus topic]`, `/compact` (alias), `/model `, `/workspace `, `/new`, `/usage`, `/theme` - Arrow keys navigate, Tab/Enter select, Escape closes - Unrecognized commands pass through to the agent diff --git a/api/state_sync.py b/api/state_sync.py index 9f1b9da5..26225a56 100644 --- a/api/state_sync.py +++ b/api/state_sync.py @@ -195,3 +195,48 @@ def sync_session_usage(session_id: str, input_tokens: int=0, output_tokens: int= db.close() except Exception: logger.debug("Failed to close state.db") + + +def sync_session_title(session_id: str, title: str, profile: Optional[str] = None) -> None: + """Sync an auto-generated title to state.db (not gated by sync_to_insights). + + Background title generation writes the title to the WebUI sidecar JSON but + not to hermes-agent's state.db, so ``hermes sessions list`` shows blank + titles for WebUI sessions. This function bridges that gap and is called + from the background title update/refresh paths after a title is persisted. + + Uses ``set_auto_title_if_empty`` so it will only populate a NULL title and + never overwrite a manual rename made via CLI/Gateway/TUI. This means + title refreshes (where state.db already holds the initial auto-title) are + effectively no-ops at the state.db layer -- acceptable because the primary + goal is ensuring ``hermes sessions list`` is not blank. + + On a title collision (two sessions with the same auto-title), the title is + de-duplicated via ``get_next_title_in_lineage`` (e.g. "My Session" -> + "My Session #2") and retried, so the second session is never left blank. + """ + if not title: + return + db = _get_state_db(profile=profile) + if not db: + return + try: + # Ensure the session row exists (idempotent) so the UPDATE has a target. + db.ensure_session(session_id=session_id, source='webui') + try: + db.set_auto_title_if_empty(session_id, title) + except ValueError: + # state.db enforces uniqueness on sessions.title, so a byte-identical + # auto-title generated for two sessions raises ValueError here. Derive + # a de-duplicated variant (e.g. "My Session" -> "My Session #2") and + # retry instead of leaving the second row blank (#6964). + alt = db.get_next_title_in_lineage(title) + if alt and alt != title: + db.set_auto_title_if_empty(session_id, alt) + except Exception: + logger.debug("Failed to sync session title to state.db for %s", session_id) + finally: + try: + db.close() + except Exception: + logger.debug("Failed to close state.db") diff --git a/api/streaming.py b/api/streaming.py index ddc25de4..21ff9005 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -21,6 +21,7 @@ import threading import time import traceback import copy +import inspect from pathlib import Path from typing import Optional @@ -307,6 +308,19 @@ def _install_streaming_cronjob_profile_wrapper() -> None: _STREAMING_CRONJOB_WRAPPER_INSTALLED = True +def _supports_kwarg(func, kwarg_name: str) -> bool: + """Return True if callable `func` accepts `kwarg_name` (explicitly or via **kwargs).""" + try: + sig = inspect.signature(func) + for param in sig.parameters.values(): + if param.kind == param.VAR_KEYWORD or param.name == kwarg_name: + return True + return False + except Exception: + return True + + + _PERSISTENT_MEMORY_FILES = ( ("memory", ("memories", "MEMORY.md")), ("user", ("memories", "USER.md")), @@ -4466,6 +4480,12 @@ def _run_background_title_update(session_id: str, user_text: str, assistant_text else: _put_title_status(put_event, session_id, source, llm_status, effective_title, raw_preview) put_event('title', {'session_id': session_id, 'title': effective_title}) + # Sync the generated title to state.db so `hermes sessions list` shows it. + try: + from api.state_sync import sync_session_title + sync_session_title(session_id, effective_title, profile=getattr(s, 'profile', None) or 'default') + except Exception: + logger.debug("Failed to sync title to state.db after generation for %s", session_id) else: _put_title_status(put_event, session_id, 'skipped', source or 'unchanged', effective_title, raw_preview) finally: @@ -4538,6 +4558,12 @@ def _run_background_title_refresh(session_id: str, user_text: str, assistant_tex s.save(touch_updated_at=False) _put_title_status(put_event, session_id, 'refreshed', llm_status, effective_title, raw_preview) put_event('title', {'session_id': session_id, 'title': effective_title}) + # Sync the refreshed title to state.db so `hermes sessions list` stays current. + try: + from api.state_sync import sync_session_title + sync_session_title(session_id, effective_title, profile=getattr(s, 'profile', None) or 'default') + except Exception: + logger.debug("Failed to sync refreshed title to state.db for %s", session_id) logger.info("Adaptive title refresh: session=%s new_title=%r", session_id, effective_title) except Exception: logger.debug("Background title refresh failed for session %s", session_id, exc_info=True) @@ -9884,8 +9910,9 @@ def _run_agent_streaming( ), task_id=session_id, persist_user_message=msg_text, - persist_user_timestamp=getattr(s, 'pending_started_at', None), ) + if _supports_kwarg(agent.run_conversation, 'persist_user_timestamp'): + _run_conversation_kwargs['persist_user_timestamp'] = getattr(s, 'pending_started_at', None) # Only pass moa_config when a /moa override is actually active, so a # normal send never trips a TypeError on an older hermes-agent whose # run_conversation() predates the moa_config kwarg. @@ -10387,8 +10414,9 @@ def _run_agent_streaming( ), task_id=session_id, persist_user_message=msg_text, - persist_user_timestamp=getattr(s, 'pending_started_at', None), ) + if _supports_kwarg(agent.run_conversation, 'persist_user_timestamp'): + _heal_kwargs['persist_user_timestamp'] = getattr(s, 'pending_started_at', None) if moa_config is not None: _heal_kwargs["moa_config"] = moa_config _heal_result = agent.run_conversation(**_heal_kwargs) @@ -11627,8 +11655,9 @@ def _run_agent_streaming( ), task_id=session_id, persist_user_message=msg_text, - persist_user_timestamp=getattr(s, 'pending_started_at', None), ) + if _supports_kwarg(_heal_agent.run_conversation, 'persist_user_timestamp'): + _heal_kwargs2['persist_user_timestamp'] = getattr(s, 'pending_started_at', None) if moa_config is not None: _heal_kwargs2["moa_config"] = moa_config _heal_result = _heal_agent.run_conversation(**_heal_kwargs2) diff --git a/docs/EXTENSIONS.md b/docs/EXTENSIONS.md index c9374a55..7d7c8f0b 100644 --- a/docs/EXTENSIONS.md +++ b/docs/EXTENSIONS.md @@ -289,8 +289,8 @@ Core-sanitized manifest inventory that was present at the initial page boot. It returns `null` for empty, malformed, unknown, or untrusted IDs without creating settings or storage state. Re-registering the same ID in one page returns the same handle object; different IDs receive different handles. The handle exposes -only the canonical `id`, `settings`, and `storage` fields, backed by the same -factories used by the legacy accessors. +only the canonical `id`, `settings`, `storage`, and `events` fields. Settings and +storage are backed by the same factories used by the legacy accessors. Manifest enable/disable changes still take effect after a WebUI reload. A handle already created in the current page may remain cached across a status refresh; @@ -300,6 +300,54 @@ sandbox, isolation mechanism, capability or permission system, or security boundary. Extensions still execute with the full WebUI session authority described above. +### Turn lifecycle events + +A registered extension can react when a session turn observed by the current page +starts or reaches a terminal state: + +```js +const ext = window.hermesExt.register("desktop-companion"); +const stop = ext && ext.events.on("turn:complete", event => { + console.log(event.sessionId, event.streamId, event.status); +}); + +// Remove the listener when the extension no longer needs it. +if (stop) stop(); +``` + +The supported event types are `turn:start`, `turn:complete`, `turn:error`, and +`turn:cancel`. The frozen event object always contains `type`, `sessionId`, +`streamId`, and `timestamp` (Unix seconds). Start events also contain +`startedAt`; terminal events contain `endedAt` and may contain `status`. +`events.on(type, handler)` returns an idempotent unsubscribe function, or `null` +for an unsupported type or non-function handler. An exception in one extension +listener is logged and does not prevent other listeners or the chat UI from +continuing. + +The version-1 mapping is deliberately small: attaching a confirmed live session +stream emits `turn:start`; SSE `done` emits `turn:complete`; SSE `cancel` and +application errors classified as `cancelled` or `interrupted` emit +`turn:cancel`; other application errors and an unrecoverable live-stream +connection failure emit `turn:error`. Transport-only `stream_end` does not emit +a second terminal event. + +Core suppresses reconnect duplicates so a recently observed +`sessionId`/`streamId` pair produces at most one start event and one terminal +event. `sessionId` is the owner of the original stream; server-side compression +or session rotation at completion does not change that lifecycle identity. +Terminal callbacks run after Core has cleared the original stream ownership, +applied the current session's immediate terminal transcript projection, and +returned the owned pane to its idle state. They do not wait for unrelated +follow-up work such as notifications, queued turns, or optional recovery +refreshes. + +This is a page-local, live-stream notification surface. It does not replay +history, report turns that finish without this page observing their stream, or +provide token, tool, approval, or metrics events. It follows the same cooperative +trust model as `register(id)` and adds no sandbox or permission boundary. A +durable or global agent event stream is a separate Core contract. Ephemeral +`/btw` answer streams are not included in this version. + ## URL rules Injected asset URLs are deliberately restricted: diff --git a/static/commands.js b/static/commands.js index aa3f0486..8bef376d 100644 --- a/static/commands.js +++ b/static/commands.js @@ -187,6 +187,7 @@ function getMatchingCommands(prefix){ const matches=COMMANDS.filter(c=>c.name.startsWith(q)).map(c=>({...c,source:'builtin'})); const seen=new Set(matches.map(c=>c.name)); const reserved=_getReservedSlashCommandSlugs(); + const bundleSlugs=new Set(_bundleCommandCache.map(bundle=>bundle.name)); for(const [name, spec] of Object.entries(SLASH_SUBARG_SOURCES)){ if(!name.startsWith(q)||seen.has(name))continue; matches.push({ @@ -227,10 +228,16 @@ function getMatchingCommands(prefix){ seen.add(bundle.name); } } + // A same-slug bundle owns dispatch. Hold plain skills until the independent + // bundle metadata request settles so a slow bundle response cannot briefly + // expose a selectable, shadowed skill. + if(!_bundleCommandCacheReady)return matches; for(const skill of _skillCommandCache){ - if(!skill.name.startsWith(q)||seen.has(skill.name)||reserved.has(skill.name))continue; + const name=String(skill&&skill.name||'').toLowerCase(); + const description=String(skill&&skill.desc||'').toLowerCase(); + if((!name.includes(q)&&!description.includes(q))||seen.has(name)||reserved.has(name)||bundleSlugs.has(name))continue; matches.push(skill); - seen.add(skill.name); + seen.add(name); } return matches; } @@ -2086,8 +2093,9 @@ function refreshSlashCommandDropdown(){ }); } function ensureSkillCommandsLoadedForAutocomplete(){ - if(_skillCommandCacheReady||_skillCommandLoadPromise)return; - loadSkillCommands().then(()=>{refreshSlashCommandDropdown();}); + if(!_skillCommandCacheReady&&!_skillCommandLoadPromise){ + loadSkillCommands().then(()=>{refreshSlashCommandDropdown();}); + } if(!_bundleCommandCacheReady&&!_bundleCommandLoadPromise){ loadBundleCommands().then(()=>{refreshSlashCommandDropdown();}); } diff --git a/static/extension_settings.js b/static/extension_settings.js index e73c21c3..cd49f3d5 100644 --- a/static/extension_settings.js +++ b/static/extension_settings.js @@ -4,9 +4,13 @@ const SETTINGS_PREFIX='hermes.ext.settings.'; const STORAGE_PREFIX='hermes.ext.storage.'; const FIELD_TYPES=new Set(['boolean','string','number','integer','enum']); + const TURN_LIFECYCLE_TYPES=new Set(['turn:start','turn:complete','turn:error','turn:cancel']); + const TURN_LIFECYCLE_STATE_LIMIT=512; const schemas=new Map(); const trustedExtensions=new Map(); const registrations=new Map(); + const turnLifecycleListeners=new Map(); + const turnLifecycleStates=new Map(); let trustedSeeded=false; function extensionId(value){ @@ -318,6 +322,90 @@ return storageAccessor(clean,meta); } + function eventAccessor(clean){ + return Object.freeze({ + on(type,handler){ + if(!TURN_LIFECYCLE_TYPES.has(type)||typeof handler!=='function') return null; + let listenersByExtension=turnLifecycleListeners.get(type); + if(!listenersByExtension){ + listenersByExtension=new Map(); + turnLifecycleListeners.set(type,listenersByExtension); + } + let listeners=listenersByExtension.get(clean); + if(!listeners){ + listeners=new Set(); + listenersByExtension.set(clean,listeners); + } + listeners.add(handler); + let active=true; + return function unsubscribe(){ + if(!active) return false; + active=false; + listeners.delete(handler); + if(!listeners.size) listenersByExtension.delete(clean); + if(!listenersByExtension.size) turnLifecycleListeners.delete(type); + return true; + }; + }, + }); + } + + function turnLifecycleKey(sessionId,streamId){ + return `${sessionId}\u0000${streamId}`; + } + + function rememberTurnLifecycleState(key,state){ + if(turnLifecycleStates.has(key)) turnLifecycleStates.delete(key); + turnLifecycleStates.set(key,state); + while(turnLifecycleStates.size>TURN_LIFECYCLE_STATE_LIMIT){ + turnLifecycleStates.delete(turnLifecycleStates.keys().next().value); + } + } + + function dispatchTurnLifecycle(type,raw){ + if(!TURN_LIFECYCLE_TYPES.has(type)||!raw||typeof raw!=='object') return false; + const sessionId=typeof raw.sessionId==='string'?raw.sessionId.trim():''; + const streamId=typeof raw.streamId==='string'?raw.streamId.trim():''; + if(!sessionId||!streamId) return false; + + const key=turnLifecycleKey(sessionId,streamId); + const previous=turnLifecycleStates.get(key)||{started:false,terminal:false}; + const terminal=type!=='turn:start'; + if((type==='turn:start'&&(previous.started||previous.terminal))||(terminal&&previous.terminal)) return false; + rememberTurnLifecycleState(key,{ + started:previous.started||type==='turn:start', + terminal:previous.terminal||terminal, + }); + + const now=Date.now()/1000; + const event={ + type, + sessionId, + streamId, + timestamp:Number.isFinite(raw.timestamp)?raw.timestamp:now, + }; + if(type==='turn:start') event.startedAt=Number.isFinite(raw.startedAt)?raw.startedAt:event.timestamp; + else event.endedAt=Number.isFinite(raw.endedAt)?raw.endedAt:event.timestamp; + if(typeof raw.status==='string'&&raw.status.trim()) event.status=raw.status.trim(); + const frozenEvent=Object.freeze(event); + const listenersByExtension=turnLifecycleListeners.get(type); + if(!listenersByExtension) return true; + for(const [extensionId,listeners] of listenersByExtension){ + for(const listener of [...listeners]){ + try{ + listener(frozenEvent); + }catch(error){ + if(typeof console!=='undefined'&&typeof console.error==='function'){ + try{ + console.error(`[Hermes extensions] ${extensionId} ${type} listener failed:`,error); + }catch(_loggingError){ } + } + } + } + } + return true; + } + function registerExtension(id){ if(typeof id!=='string') return null; const clean=extensionId(id); @@ -330,6 +418,7 @@ id:clean, settings:settingsAccessor(clean,trusted,true), storage:storageAccessor(clean,trusted), + events:eventAccessor(clean), }); registrations.set(clean,handle); return handle; @@ -341,6 +430,7 @@ namespaceForExtension, settingsForExtension, storageForExtension, + _dispatchTurnLifecycle:dispatchTurnLifecycle, resetSettingsForExtension(id){return settingsForExtension(id).reset();}, clearStorageForExtension(id){return storageForExtension(id).clear();}, }; diff --git a/static/messages.js b/static/messages.js index 9b68157a..14b5f287 100644 --- a/static/messages.js +++ b/static/messages.js @@ -2066,9 +2066,26 @@ function closeOtherLiveStreams(activeSid){ } } +function _dispatchExtensionTurnLifecycle(type,sessionId,streamId,details={}){ + const runtime=typeof window!=='undefined'&&window.HermesExtensionSettings; + const dispatch=runtime&&runtime._dispatchTurnLifecycle; + if(typeof dispatch!=='function') return false; + try{ + return dispatch(type,{sessionId,streamId,...details}); + }catch(error){ + if(typeof console!=='undefined'&&typeof console.error==='function'){ + try{console.error('[Hermes extensions] lifecycle dispatch failed:',error);}catch(_loggingError){ } + } + return false; + } +} + function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ if(!activeSid||!streamId) return; const reconnecting=!!options.reconnecting; + const _extensionTurnStartedAt=(S.session&&S.session.session_id===activeSid&&Number.isFinite(S.session.pending_started_at)) + ?S.session.pending_started_at + :Date.now()/1000; // #4416: start (or, on reconnect for the SAME stream, keep) tracking whether // the tab was hidden during this stream so the done-notification fires for a // backgrounded tab. A reconnect with a different streamId re-seeds (the old @@ -6308,6 +6325,10 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ if(isActiveSession) _queueDrainSid=activeSid; renderSessionList(); _setActivePaneIdleIfOwner(); + _dispatchExtensionTurnLifecycle('turn:complete',activeSid,streamId,{ + status:d.status||'completed', + endedAt:Date.now()/1000, + }); playNotificationSound(); // #4416: notify if the tab was hidden at ANY point during this stream // (not just at done-receive time, which a throttled background-tab SSE @@ -6492,6 +6513,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ _clearClarifyForOwner('terminal'); let d={}; try{ d=JSON.parse(e.data||'{}')||{}; }catch(_){ d={}; } + const _extensionErrorType=(d.type==='cancelled'||d.type==='interrupted')?'turn:cancel':'turn:error'; const currentSid=S.session&&S.session.session_id; const eventSid=d.old_session_id||d.session_id||''; const continuationSid=(d.session&&d.session.session_id)||d.new_session_id||d.continuation_session_id||''; @@ -6589,6 +6611,10 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ } _setActivePaneIdleIfOwner(); renderSessionList(); // clear streaming indicator immediately on apperror + _dispatchExtensionTurnLifecycle(_extensionErrorType,activeSid,streamId,{ + status:d.status||d.type||(_extensionErrorType==='turn:cancel'?'cancelled':'error'), + endedAt:Date.now()/1000, + }); }); source.addEventListener('warning',e=>{ @@ -6791,6 +6817,8 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ // a second GET race where the visible cancelled work briefly collapses to only // the fallback "Task cancelled" marker (#4076). const _cancelSessionPayload=_cancelData&&typeof _cancelData.session==='object'?_cancelData.session:null; + renderSessionList(); + _setActivePaneIdleIfOwner(); (async()=>{ try{ if(_applyCancelSessionPayload(_cancelSessionPayload)) return; @@ -6816,10 +6844,13 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ if(_wasFollowingAtCancelFb && typeof scrollToBottom==='function') scrollToBottom(); _markSessionViewed(activeSid, S.messages.length); } + }finally{ + _dispatchExtensionTurnLifecycle('turn:cancel',activeSid,streamId,{ + status:_cancelData.status||_cancelData.type||'cancelled', + endedAt:Date.now()/1000, + }); } })(); - renderSessionList(); - _setActivePaneIdleIfOwner(); }); for(const _runJournalEventName of ['token','interim_assistant','reasoning','tool','tool_complete','todo_state','approval','clarify','state_saved','title','title_status','context_status','goal','goal_continue','done','stream_end','pending_steer_leftover','compressing','compressed','metering','apperror','warning','error','cancel']){ @@ -7047,6 +7078,10 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ } } _setActivePaneIdleIfOwner(); + _dispatchExtensionTurnLifecycle('turn:error',activeSid,streamId,{ + status:'connection_lost', + endedAt:Date.now()/1000, + }); } (async()=>{ @@ -7088,6 +7123,9 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ }catch(_){} } const replayParams=(reconnecting||replayOnly)?_runJournalReplayParams():''; + _dispatchExtensionTurnLifecycle('turn:start',activeSid,streamId,{ + startedAt:_extensionTurnStartedAt, + }); _wireSSE(new EventSource(new URL(`api/chat/stream?stream_id=${encodeURIComponent(streamId)}${replayParams}`,document.baseURI||location.href).href,{withCredentials:true})); })(); diff --git a/static/sessions.js b/static/sessions.js index 739bfc04..0c584aec 100644 --- a/static/sessions.js +++ b/static/sessions.js @@ -1734,6 +1734,13 @@ async function loadSession(sid){ _yoloEnabled=false;_updateYoloPill(); if(typeof stopClarifyPolling==='function') stopClarifyPolling(); if(typeof hideClarifyCard==='function') hideClarifyCard(forceReload, forceReload?'external-refresh':'dismissed'); + // #6572: clear stale compression state when switching sessions. + // The compression UI state is per-session and must not leak across loads. + // Without this, a compression card from a prior session can appear as a + // phantom "Compressing context" barrier on a fresh session that never + // triggered compression. + if(typeof clearCompressionUi==='function') clearCompressionUi(); + else window._compressionUi=null; // Show loading indicator immediately for responsiveness. // Cleared by renderMessages() once full session data arrives. // Persist the current composer draft before switching away so it can be diff --git a/static/ui.js b/static/ui.js index 566029a8..46954203 100644 --- a/static/ui.js +++ b/static/ui.js @@ -3195,7 +3195,34 @@ function _findModelInDropdown(modelId, sel, preferredProviderId){ } const preferred=String(preferredProviderId||explicitProvider||'').toLowerCase(); if(preferred){ - const providerMatch=options.find(o=>norm(o.value)===target && _getOptionProviderId(o).toLowerCase()===preferred); + if(preferred==='custom'||preferred.startsWith('custom:')){ + // A slash is part of a custom endpoint's upstream model ID, not a + // provider namespace. Match the exact routed ID (allowing only the + // WebUI's @provider: wrapper and dash/dot spelling compatibility). + const routeNorm=value=>{ + let routed=String(value||''); + const prefix=`@${preferred}:`; + if(routed.toLowerCase().startsWith(prefix)) routed=routed.slice(prefix.length); + return routed.toLowerCase().replace(/-/g,'.'); + }; + const providerOptions=options.filter(o=>_getOptionProviderId(o).toLowerCase()===preferred); + const providerMatch=providerOptions.find(o=>routeNorm(o.value)===routeNorm(rawModel)); + if(providerMatch) return providerMatch.value; + // Legacy sessions may store only the bare suffix of a routed custom + // option. Preserve #6195's provider-hinted repair, but only for an + // explicit @provider: row; an unwrapped slash ID belongs to the active + // endpoint and must not substitute for a distinct bare model. + if(!rawModel.includes('/')&&!rawModel.startsWith('@')){ + const prefix=`@${preferred}:`; + const suffixMatches=providerOptions.filter(o=> + String(o.value||'').toLowerCase().startsWith(prefix) + &&norm(o.value)===target + ); + if(suffixMatches.length===1) return suffixMatches[0].value; + } + return null; + } + const providerMatch=options.find(o=>norm(o.value)===target&&_getOptionProviderId(o).toLowerCase()===preferred); if(providerMatch) return providerMatch.value; } // 2. Normalized match — but ONLY when unambiguous. If the bare id diff --git a/tests/test_3230_preserve_manual_session_title.py b/tests/test_3230_preserve_manual_session_title.py index edddf161..833abd1b 100644 --- a/tests/test_3230_preserve_manual_session_title.py +++ b/tests/test_3230_preserve_manual_session_title.py @@ -173,6 +173,10 @@ def test_cleared_title_allows_initial_auto_generation(monkeypatch): "profile_env_for_background_worker", lambda *_args, **_kwargs: contextlib.nullcontext(), ) + monkeypatch.setattr( + "api.state_sync.sync_session_title", + lambda *_args, **_kwargs: None, + ) streaming._run_background_title_update( session.session_id, diff --git a/tests/test_cli_only_slash_commands.py b/tests/test_cli_only_slash_commands.py index 4f1d9fd4..9ac5c2d6 100644 --- a/tests/test_cli_only_slash_commands.py +++ b/tests/test_cli_only_slash_commands.py @@ -215,6 +215,18 @@ def _run_commands_js(script_body: str) -> dict: {{ name: 'plugin review', description: 'Plugin collisions should stay hidden from slash autocomplete' + }}, + {{ + name: 'hermes-upgrade', + description: 'Safely upgrade and verify a Hermes installation' + }}, + {{ + name: 'maintenance-guide', + description: 'Plan a safe upgrade and rollback path' + }}, + {{ + name: 'reinstall-helper', + description: 'Recover from a failed reinstall' }} ] }}; @@ -352,6 +364,102 @@ def test_cli_only_slugs_reserve_skill_autocomplete_namespace(): assert "use" in result["use_names"] +def test_skill_autocomplete_matches_keyword_in_name_or_description(): + result = _run_commands_js( + """ + await loadBundleCommands(true); + await loadSkillCommands(true); + const upgrade = await getSlashAutocompleteMatches('/upgrade'); + const reinstall = await getSlashAutocompleteMatches('/reinstall'); + return { + upgrade_names: upgrade.map(item => item.name), + upgrade_sources: upgrade.map(item => item.source), + reinstall_names: reinstall.map(item => item.name), + reinstall_sources: reinstall.map(item => item.source) + }; + """ + ) + + assert result["upgrade_names"] == ["hermes-upgrade", "maintenance-guide"] + assert result["upgrade_sources"] == ["skill", "skill"] + assert result["reinstall_names"] == ["reinstall-helper"] + assert result["reinstall_sources"] == ["skill"] + + +def test_skill_autocomplete_hides_keyword_match_shadowed_by_bundle(): + result = _run_commands_js( + """ + await loadAgentCommandMetadata(true); + await loadBundleCommands(true); + await loadSkillCommands(true); + const matches = await getSlashAutocompleteMatches('/non-colliding'); + return matches.map(item => ({ name: item.name, source: item.source })); + """ + ) + + assert result == [] + + +def test_skill_autocomplete_waits_for_bundle_metadata_before_showing_colliding_keyword_match(): + script = textwrap.dedent( + f""" + const vm = require('vm'); + let releaseBundles; + const bundlesReady = new Promise(resolve => {{ releaseBundles = resolve; }}); + const ctx = {{ + console, + localStorage: {{ getItem(){{return null;}}, setItem(){{}}, removeItem(){{}} }}, + t: (key) => key, + api: async (path) => {{ + if (path === '/api/commands') return {{ commands: [] }}; + if (path === '/api/commands/bundles') return bundlesReady; + if (path === '/api/skills') return {{ skills: [ + {{ + name: 'incident-review', + description: 'Handle a non-colliding keyword', + category: 'ops' + }} + ] }}; + throw new Error('unexpected api path: ' + path); + }} + }}; + vm.createContext(ctx); + vm.runInContext({json.dumps(COMMANDS_JS)}, ctx); + (async () => {{ + await vm.runInContext('loadSkillCommands(true)', ctx); + const bundleLoad = vm.runInContext('loadBundleCommands(true)', ctx); + const before = await vm.runInContext("getSlashAutocompleteMatches('/non-colliding')", ctx); + releaseBundles({{ bundles: [ + {{ + name: 'incident-review', + description: 'Incident review bundle', + skill_count: 3, + source: 'bundle' + }} + ] }}); + await bundleLoad; + const after = await vm.runInContext("getSlashAutocompleteMatches('/non-colliding')", ctx); + process.stdout.write(JSON.stringify({{ + before: before.map(item => ({{ name: item.name, source: item.source }})), + after: after.map(item => ({{ name: item.name, source: item.source }})) + }})); + }})().catch(err => {{ + console.error(err && err.stack || err); + process.exit(1); + }}); + """ + ) + with tempfile.NamedTemporaryFile("w", suffix=".js", encoding="utf-8", delete=False) as handle: + handle.write(script) + script_path = Path(handle.name) + try: + proc = subprocess.run(["node", str(script_path)], check=True, capture_output=True, text=True) + finally: + script_path.unlink(missing_ok=True) + + assert json.loads(proc.stdout) == {"before": [], "after": []} + + def test_bundle_collisions_stay_hidden_until_agent_metadata_is_ready(): script = textwrap.dedent( f""" diff --git a/tests/test_custom_provider_model_identity.py b/tests/test_custom_provider_model_identity.py new file mode 100644 index 00000000..e9187fe2 --- /dev/null +++ b/tests/test_custom_provider_model_identity.py @@ -0,0 +1,148 @@ +"""Regression coverage for exact custom-provider model identity during catalog refresh.""" + +import json +import shutil +import subprocess +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[1] +UI_JS = REPO_ROOT / "static" / "ui.js" +NODE = shutil.which("node") + +_DRIVER = r""" +const fs=require('fs'); +const src=fs.readFileSync(process.argv[2],'utf8'); +function extract(name){ + const start=src.indexOf('function '+name+'('); + if(start<0) throw new Error('missing '+name); + let i=src.indexOf('{',start), depth=0; + for(;ix!==child);child.parentElement=null;} + querySelectorAll(selector){ + if(selector==='optgroup') return this.children.filter(x=>x.tagName==='OPTGROUP'); + return []; + } + get options(){ + if(this.tagName!=='SELECT') return []; + return this.children.flatMap(x=>x.tagName==='OPTGROUP'?x.children:[x]).filter(x=>x.tagName==='OPTION'); + } + get value(){return this._value;} + set value(value){this._value=String(value||'');} + get selectedOptions(){const hit=this.options.find(x=>x.value===this._value);return hit?[hit]:[];} +} +const document={createElement:tag=>new Node(tag)}; +const window={_configuredModelBadges:{},_activeProvider:'custom:cpa'}; +const S={session:null}; +const _dynamicModelLabels={}; +const _liveModelFetchPending=new Set(); +const $=()=>null; +const getModelLabel=value=>value; +const syncModelChip=()=>{}; +for(const name of ['_getOptionProviderId','_providerFromModelValue','_modelPickerOptionIdentity','_deduplicateModelPickerOptions','_modelStateForSelect','_findModelInDropdown','_refreshOpenModelDropdown','_applyModelToDropdown','_ensureModelOptionInDropdown','_addLiveModelsToSelect']) eval(extract(name)); +function makeSelect(selected){ + const sel=new Node('select');sel.id='modelSelect'; + const group=new Node('optgroup');group.dataset.provider='custom:cpa';sel.appendChild(group); + for(const id of ['jb/gpt-5.6-sol']){const opt=new Node('option');opt.value=id;opt.dataset.provider='custom:cpa';group.appendChild(opt);} + sel.value=selected||'jb/gpt-5.6-sol';return sel; +} +const bare=makeSelect(); +const appliedBare=_ensureModelOptionInDropdown('gpt-5.6-sol',bare,'custom:cpa'); +const beforeLive=_modelStateForSelect(bare,bare.value); +_addLiveModelsToSelect('custom:cpa',[{id:'gpt-5.6-sol'},{id:'jb/gpt-5.6-sol'}],bare); +const afterLive=_modelStateForSelect(bare,bare.value); +const qualified=makeSelect('jb/gpt-5.6-sol'); +_addLiveModelsToSelect('custom:cpa',[{id:'gpt-5.6-sol'}],qualified); +console.log(JSON.stringify({ + appliedBare,beforeLive,afterLive, + bareOptions:bare.options.map(x=>x.value), + qualifiedAfter:_modelStateForSelect(qualified,qualified.value), +})); +""" + + +@pytest.mark.skipif(NODE is None, reason="node not in PATH") +def test_partial_then_live_catalog_preserves_exact_custom_provider_model(tmp_path): + driver = tmp_path / "driver.js" + driver.write_text(_DRIVER, encoding="utf-8") + result = subprocess.run( + [NODE, str(driver), str(UI_JS)], capture_output=True, text=True, timeout=30 + ) + assert result.returncode == 0, result.stderr + payload = json.loads(result.stdout) + + expected = {"model": "gpt-5.6-sol", "model_provider": "custom:cpa"} + assert payload["beforeLive"] == expected + assert payload["afterLive"] == expected + assert payload["appliedBare"] == "@custom:cpa:gpt-5.6-sol" + assert "@custom:cpa:gpt-5.6-sol" in payload["bareOptions"] + assert "@custom:cpa:jb/gpt-5.6-sol" in payload["bareOptions"] + assert payload["qualifiedAfter"] == { + "model": "jb/gpt-5.6-sol", + "model_provider": "custom:cpa", + } + + +# Ambiguity guard for the #6195 bare-id fallback (Codex round-2 SHOULD-FIX): +# when two options under the same custom provider normalize to the same target, +# the hinted-fallback must refuse to guess and return null rather than pick +# an arbitrary one. +_AMBIGUITY_DRIVER = r""" +const fs=require('fs'); +const src=fs.readFileSync(process.argv[2],'utf8'); +function extract(name){ + const start=src.indexOf('function '+name+'('); + if(start<0) throw new Error('missing '+name); + let i=src.indexOf('{',start), depth=0; + for(;i resolves to the existing namespaced option. +const single={options:[ + {value:'@custom:tok:z-ai/glm-5.2', providerId:'custom:tok'}, + {value:'openai/gpt-4', providerId:'openai'}, +]}; +// Two same-provider options normalizing to the same target -> ambiguous. +const ambiguous={options:[ + {value:'@custom:tok:z-ai/glm-5.2', providerId:'custom:tok'}, + {value:'@custom:tok:other/glm-5.2', providerId:'custom:tok'}, +]}; +console.log(JSON.stringify({ + single:_findModelInDropdown('glm-5.2', single, 'custom:tok'), + ambiguous:_findModelInDropdown('glm-5.2', ambiguous, 'custom:tok'), + exact:_findModelInDropdown('z-ai/glm-5.2', single, 'custom:tok'), +})); +""" + + +@pytest.mark.skipif(NODE is None, reason="node not in PATH") +def test_hinted_bare_id_fallback_refuses_ambiguous_match(tmp_path): + driver = tmp_path / "amb_driver.js" + driver.write_text(_AMBIGUITY_DRIVER, encoding="utf-8") + result = subprocess.run( + [NODE, str(driver), str(UI_JS)], capture_output=True, text=True, timeout=30 + ) + assert result.returncode == 0, result.stderr + payload = json.loads(result.stdout) + + # One unambiguous suffix match: the bare id resolves to the existing + # namespaced catalog option (#6195 repair preserved). + assert payload["single"] == "@custom:tok:z-ai/glm-5.2" + # Two same-normalized options under the same provider: refuse to guess. + assert payload["ambiguous"] is None + # Exact routed match is unaffected by the fallback path (#6944 fix intact). + assert payload["exact"] == "@custom:tok:z-ai/glm-5.2" diff --git a/tests/test_extension_settings_runtime.py b/tests/test_extension_settings_runtime.py index 4221e7f2..fac5fef0 100644 --- a/tests/test_extension_settings_runtime.py +++ b/tests/test_extension_settings_runtime.py @@ -214,7 +214,7 @@ def test_hermes_ext_register_runtime_identity_and_reload_lifecycle(): assert.ok(alpha); assert.strictEqual(alpha.id, 'alpha.ext'); assert.strictEqual(Object.isFrozen(alpha), true); - assert.deepStrictEqual(Object.keys(alpha).sort(), ['id', 'settings', 'storage']); + assert.deepStrictEqual(Object.keys(alpha).sort(), ['events', 'id', 'settings', 'storage']); assert.deepStrictEqual(alpha.settings.values, {{enabled: false, mode: 'alpha'}}); assert.strictEqual(alpha.settings.set('enabled', true).ok, true); assert.strictEqual(alpha.storage.set('note', 'alpha-note'), true); diff --git a/tests/test_extension_turn_lifecycle.py b/tests/test_extension_turn_lifecycle.py new file mode 100644 index 00000000..4a1f5b71 --- /dev/null +++ b/tests/test_extension_turn_lifecycle.py @@ -0,0 +1,495 @@ +"""Behavioral coverage for extension turn-lifecycle subscriptions.""" + +import json +from pathlib import Path +import re +import shutil +import subprocess +import textwrap + +import pytest + + +ROOT = Path(__file__).parent.parent +STATIC = ROOT / "static" +EXTENSION_SETTINGS_JS = ROOT / "static" / "extension_settings.js" +MESSAGES_JS = (ROOT / "static" / "messages.js").read_text(encoding="utf-8") +INDEX_HTML = (STATIC / "index.html").read_text(encoding="utf-8") +UI_JS = (STATIC / "ui.js").read_text(encoding="utf-8") +SUPPORT_SCRIPTS = [ + (STATIC / name).read_text(encoding="utf-8") + for name in ("i18n.js", "icons.js", "assistant_turn_anchors.js") +] +HARNESS_HTML = re.sub(r"]*>.*?", "", INDEX_HTML, flags=re.I | re.S) +HARNESS_HTML = re.sub(r"]*>", "", HARNESS_HTML, flags=re.I) + + +_MOCK_EVENT_SOURCE = r""" +class MockEventSource { + static CONNECTING = 0; + static OPEN = 1; + static CLOSED = 2; + static instances = []; + constructor(url) { + this.url = String(url); + this.readyState = MockEventSource.OPEN; + this.listeners = new Map(); + MockEventSource.instances.push(this); + } + addEventListener(name, listener) { + const listeners = this.listeners.get(name) || []; + listeners.push(listener); + this.listeners.set(name, listeners); + } + removeEventListener(name, listener) { + this.listeners.set(name, (this.listeners.get(name) || []).filter(fn => fn !== listener)); + } + async emit(name, payload) { + const event = {type: name, data: JSON.stringify(payload), lastEventId: ''}; + await Promise.all((this.listeners.get(name) || []).map(listener => listener(event))); + } + close() { this.readyState = MockEventSource.CLOSED; } +} +window.EventSource = MockEventSource; +window.MockEventSource = MockEventSource; +""" + + +_STABILIZE_UNRELATED_UI = r""" +appendLiveCompressionCard = () => false; +resetTurnWorkspaceMutations = () => {}; +_resetStreamScrollFollow = () => {}; +ensureLiveWorklogShell = () => {}; +_shouldUseLiveProseFade = () => false; +_shouldFollowMessagesOnDomReplace = () => false; +_isMessagePaneNearBottom = () => false; +_markSessionViewed = () => {}; +setBusy = value => { S.busy = Boolean(value); }; +setComposerStatus = () => {}; +setStatus = () => {}; +syncTopbar = () => {}; +renderMessages = () => {}; +renderSessionList = () => {}; +renderSessionArtifacts = () => {}; +loadDir = () => {}; +playNotificationSound = () => {}; +sendBrowserNotification = () => {}; +api = async () => ({}); +""" + + +_LIFECYCLE_SCENARIO = r""" +async ({kind}) => { + const activeSid = 'session-a'; + const streamId = `stream-${kind}`; + const lifecycle = []; + const extension = window.hermesExt.register('lifecycle-probe'); + if (!extension) throw new Error('lifecycle probe did not register'); + for (const type of ['turn:start', 'turn:complete', 'turn:error', 'turn:cancel']) { + extension.events.on(type, event => { + const lastMessage = Array.isArray(S.messages) ? S.messages.at(-1) : null; + lifecycle.push({ + event: {...event}, + activeStreamId: S.activeStreamId || null, + busy: Boolean(S.busy), + currentSid: S.session && S.session.session_id || null, + lastContent: lastMessage && lastMessage.content || null, + }); + }); + } + + S.session = {session_id: activeSid, messages: [{role: 'user', content: 'question'}]}; + S.messages = [{role: 'user', content: 'question'}]; + S.toolCalls = []; + S.activeStreamId = streamId; + S.busy = true; + attachLiveStream(activeSid, streamId, []); + const source = MockEventSource.instances.at(-1); + if (!source) throw new Error('attachLiveStream did not construct EventSource'); + + if (kind === 'done') { + await source.emit('done', { + status: 'completed', + stream_id: streamId, + session: { + session_id: activeSid, + messages: [{role: 'assistant', content: 'settled-done'}], + tool_calls: [], + }, + }); + } else if (kind === 'apperror') { + await source.emit('apperror', { + type: 'provider_error', + status: 'provider_error', + message: 'provider failed', + session_id: activeSid, + session: { + session_id: activeSid, + messages: [{role: 'assistant', content: 'settled-error'}], + }, + }); + } else if (kind === 'apperror-cancel') { + await source.emit('apperror', { + type: 'interrupted', + status: 'interrupted', + message: 'interrupted', + session_id: activeSid, + session: { + session_id: activeSid, + messages: [{role: 'assistant', content: 'settled-interrupted'}], + }, + }); + } else if (kind === 'cancel') { + await source.emit('cancel', { + type: 'cancelled', + status: 'cancelled', + session_id: activeSid, + session: { + session_id: activeSid, + messages: [{role: 'assistant', content: 'settled-cancel'}], + }, + }); + } else if (kind === 'connection-error') { + const nativeSetTimeout = window.setTimeout; + api = async url => String(url).includes('/api/chat/stream/status') + ? {active: false, replay_available: false} + : {}; + window.setTimeout = callback => { + queueMicrotask(callback); + return 1; + }; + try { + await source.emit('error', {}); + await new Promise((resolve, reject) => { + const deadline = Date.now() + 2000; + const poll = () => { + if (lifecycle.some(entry => entry.event.type === 'turn:error')) return resolve(); + if (Date.now() >= deadline) return reject(new Error('connection terminal event not observed')); + nativeSetTimeout(poll, 0); + }; + poll(); + }); + } finally { + window.setTimeout = nativeSetTimeout; + } + } else { + throw new Error(`unknown lifecycle scenario: ${kind}`); + } + return lifecycle; +} +""" + + +def _function_body(source: str, name: str) -> str: + start = source.index(f"function {name}") + brace = source.index("){", start) + 1 + depth = 0 + for index in range(brace, len(source)): + if source[index] == "{": + depth += 1 + elif source[index] == "}": + depth -= 1 + if depth == 0: + return source[start : index + 1] + raise AssertionError(f"function {name} body not found") + + +def _event_listener_body(source: str, event_name: str) -> str: + start = source.index(f"source.addEventListener('{event_name}'") + brace = source.index("{", start) + depth = 0 + for index in range(brace, len(source)): + if source[index] == "{": + depth += 1 + elif source[index] == "}": + depth -= 1 + if depth == 0: + return source[start : index + 1] + raise AssertionError(f"event listener {event_name!r} body not found") + + +def _run_node(script: str): + node = shutil.which("node") + if not node: + pytest.skip("node is required for extension lifecycle runtime tests") + result = subprocess.run( + [node, "-e", script], + cwd=ROOT, + text=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + timeout=20, + ) + assert result.returncode == 0, result.stderr + result.stdout + + +def test_registered_extension_receives_bounded_turn_lifecycle_events(): + script = textwrap.dedent( + f""" + const fs = require('fs'); + const assert = require('assert'); + const store = new Map(); + const loggedErrors = []; + global.window = {{ + __HERMES_EXTENSION_CONFIG__: {{ + extensions: [ + {{id: 'alpha.ext', storage_owned: false}}, + {{id: 'beta.ext', storage_owned: false}}, + ], + }}, + localStorage: {{ + getItem(key) {{ return store.has(key) ? store.get(key) : null; }}, + setItem(key, value) {{ store.set(key, String(value)); }}, + removeItem(key) {{ store.delete(key); }}, + }}, + }}; + global.console = {{ + error(...args) {{ loggedErrors.push(args.map(String).join(' ')); }}, + }}; + eval(fs.readFileSync({str(EXTENSION_SETTINGS_JS)!r}, 'utf8')); + + const alpha = window.hermesExt.register('alpha.ext'); + const beta = window.hermesExt.register('beta.ext'); + assert.deepStrictEqual(Object.keys(alpha).sort(), ['events', 'id', 'settings', 'storage']); + assert.strictEqual(Object.isFrozen(alpha.events), true); + + const alphaEvents = []; + const betaEvents = []; + const unsubscribeStart = alpha.events.on('turn:start', event => {{ + assert.strictEqual(Object.isFrozen(event), true); + alphaEvents.push(event); + }}); + alpha.events.on('turn:complete', () => {{ throw new Error('extension failure'); }}); + alpha.events.on('turn:complete', event => alphaEvents.push(event)); + alpha.events.on('turn:error', event => alphaEvents.push(event)); + alpha.events.on('turn:cancel', event => alphaEvents.push(event)); + beta.events.on('turn:complete', event => betaEvents.push(event)); + + assert.strictEqual(typeof unsubscribeStart, 'function'); + assert.strictEqual(alpha.events.on('token', () => {{}}), null); + assert.strictEqual(alpha.events.on('turn:start', null), null); + + const emit = window.HermesExtensionSettings._dispatchTurnLifecycle; + assert.strictEqual(typeof emit, 'function'); + assert.strictEqual(emit('turn:start', {{sessionId: '', streamId: 'stream-a'}}), false); + assert.strictEqual(emit('turn:start', {{sessionId: 'session-a', streamId: ''}}), false); + assert.strictEqual(emit('token', {{sessionId: 'session-a', streamId: 'stream-a'}}), false); + + assert.strictEqual(emit('turn:start', {{ + sessionId: 'session-a', streamId: 'stream-a', startedAt: 10, + }}), true); + assert.strictEqual(emit('turn:start', {{ + sessionId: 'session-a', streamId: 'stream-a', startedAt: 11, + }}), false); + assert.strictEqual(emit('turn:complete', {{ + sessionId: 'session-a', streamId: 'stream-a', status: 'completed', endedAt: 20, + }}), true); + assert.strictEqual(emit('turn:error', {{ + sessionId: 'session-a', streamId: 'stream-a', status: 'late-error', endedAt: 21, + }}), false); + assert.strictEqual(emit('turn:cancel', {{ + sessionId: 'session-a', streamId: 'stream-a', status: 'late-cancel', endedAt: 22, + }}), false); + + assert.strictEqual(emit('turn:start', {{sessionId: 'session-b', streamId: 'stream-b'}}), true); + assert.strictEqual(emit('turn:error', {{ + sessionId: 'session-b', streamId: 'stream-b', status: 'provider_error', + }}), true); + assert.strictEqual(emit('turn:start', {{sessionId: 'session-c', streamId: 'stream-c'}}), true); + assert.strictEqual(emit('turn:cancel', {{ + sessionId: 'session-c', streamId: 'stream-c', status: 'cancelled', + }}), true); + + unsubscribeStart(); + unsubscribeStart(); + assert.strictEqual(emit('turn:start', {{sessionId: 'session-d', streamId: 'stream-d'}}), true); + + assert.deepStrictEqual( + alphaEvents.map(event => [event.type, event.sessionId, event.streamId, event.status || null]), + [ + ['turn:start', 'session-a', 'stream-a', null], + ['turn:complete', 'session-a', 'stream-a', 'completed'], + ['turn:start', 'session-b', 'stream-b', null], + ['turn:error', 'session-b', 'stream-b', 'provider_error'], + ['turn:start', 'session-c', 'stream-c', null], + ['turn:cancel', 'session-c', 'stream-c', 'cancelled'], + ], + ); + assert.deepStrictEqual( + betaEvents.map(event => [event.type, event.sessionId, event.streamId]), + [['turn:complete', 'session-a', 'stream-a']], + ); + assert.strictEqual(loggedErrors.length, 1); + assert.match(loggedErrors[0], /alpha[.]ext.*turn:complete.*extension failure/); + """ + ) + _run_node(script) + + +def test_live_stream_bridge_forwards_normalized_lifecycle_details(): + bridge = _function_body(MESSAGES_JS, "_dispatchExtensionTurnLifecycle") + script = textwrap.dedent( + f""" + const assert = require('assert'); + const calls = []; + global.window = {{ + HermesExtensionSettings: {{ + _dispatchTurnLifecycle(type, details) {{ + calls.push([type, details]); + return 'delivered'; + }}, + }}, + }}; + eval({json.dumps(bridge)}); + + assert.strictEqual( + _dispatchExtensionTurnLifecycle( + 'turn:error', 'session-a', 'stream-a', {{status: 'connection_lost'}}, + ), + 'delivered', + ); + assert.deepStrictEqual(calls, [[ + 'turn:error', + {{sessionId: 'session-a', streamId: 'stream-a', status: 'connection_lost'}}, + ]]); + + delete window.HermesExtensionSettings; + assert.strictEqual( + _dispatchExtensionTurnLifecycle('turn:start', 'session-b', 'stream-b'), + false, + ); + + window.HermesExtensionSettings = {{ + _dispatchTurnLifecycle() {{ throw new Error('broken extension runtime'); }}, + }}; + global.console = {{error() {{}}}}; + assert.strictEqual( + _dispatchExtensionTurnLifecycle('turn:start', 'session-c', 'stream-c'), + false, + ); + """ + ) + _run_node(script) + + +@pytest.fixture(scope="module") +def lifecycle_browser(): + playwright_api = pytest.importorskip("playwright.sync_api") + with playwright_api.sync_playwright() as playwright: + browser = playwright.chromium.launch( + headless=True, + args=["--no-sandbox", "--disable-dev-shm-usage"], + ) + yield browser + browser.close() + + +def _run_lifecycle_scenario(browser, kind: str) -> list[dict]: + page = browser.new_page() + page.route( + "**/*", + lambda route: route.fulfill( + status=200, + content_type="text/html" + if route.request.url == "http://harness.test/" + else "text/plain", + body=HARNESS_HTML if route.request.url == "http://harness.test/" else "", + ), + ) + page.add_init_script(_MOCK_EVENT_SOURCE) + try: + page.goto("http://harness.test/", wait_until="domcontentloaded") + page.evaluate( + """ + window.__HERMES_EXTENSION_CONFIG__ = { + extensions: [{id: 'lifecycle-probe', storage_owned: false}], + }; + """ + ) + for script in SUPPORT_SCRIPTS: + page.add_script_tag(content=script) + page.add_script_tag(content=EXTENSION_SETTINGS_JS.read_text(encoding="utf-8")) + page.add_script_tag(content=UI_JS) + page.add_script_tag(content=MESSAGES_JS) + page.evaluate(_STABILIZE_UNRELATED_UI) + return page.evaluate(_LIFECYCLE_SCENARIO, {"kind": kind}) + finally: + page.close() + + +@pytest.mark.parametrize( + ("kind", "terminal_type", "terminal_status", "last_content"), + [ + ("done", "turn:complete", "completed", "settled-done"), + ("apperror", "turn:error", "provider_error", "settled-error"), + ("apperror-cancel", "turn:cancel", "interrupted", "settled-interrupted"), + ("cancel", "turn:cancel", "cancelled", "settled-cancel"), + ( + "connection-error", + "turn:error", + "connection_lost", + "**Connection interrupted:** The browser lost the live SSE connection before the response finished. If the worker completed, reopening this session should restore the settled transcript.", + ), + ], +) +def test_real_sse_terminal_callback_observes_settled_core_state( + lifecycle_browser, + kind, + terminal_type, + terminal_status, + last_content, +): + lifecycle = _run_lifecycle_scenario(lifecycle_browser, kind) + + assert [entry["event"]["type"] for entry in lifecycle] == [ + "turn:start", + terminal_type, + ] + start, terminal = lifecycle + assert start["activeStreamId"] == f"stream-{kind}" + assert start["busy"] is True + assert terminal["event"]["sessionId"] == "session-a" + assert terminal["event"]["streamId"] == f"stream-{kind}" + assert terminal["event"]["status"] == terminal_status + assert terminal["activeStreamId"] is None + assert terminal["busy"] is False + assert terminal["currentSid"] == "session-a" + assert terminal["lastContent"] == last_content + + +def test_live_stream_terminal_paths_use_original_stream_owner_identity(): + attach_start = MESSAGES_JS.index("function attachLiveStream(") + attach_body = MESSAGES_JS[attach_start : MESSAGES_JS.index("\nfunction transcript(", attach_start)] + done = _event_listener_body(MESSAGES_JS, "done") + application_error = _event_listener_body(MESSAGES_JS, "apperror") + cancel = _event_listener_body(MESSAGES_JS, "cancel") + connection_error = _function_body(MESSAGES_JS, "_handleStreamError") + + start_dispatch = attach_body.index("_dispatchExtensionTurnLifecycle('turn:start',activeSid,streamId") + dead_reconnect_return = attach_body.index("_scheduleAnchorRegistryCleanup(120000);") + event_source_attach = attach_body.index("_wireSSE(new EventSource", start_dispatch) + assert dead_reconnect_return < start_dispatch < event_source_attach + assert "_dispatchExtensionTurnLifecycle('turn:complete',activeSid,streamId" in done + assert "_dispatchExtensionTurnLifecycle(_extensionErrorType,activeSid,streamId" in application_error + assert "_dispatchExtensionTurnLifecycle('turn:cancel',activeSid,streamId" in cancel + assert "_dispatchExtensionTurnLifecycle('turn:error',activeSid,streamId" in connection_error + + assert done.index("_setActivePaneIdleIfOwner()") < done.index( + "_dispatchExtensionTurnLifecycle('turn:complete'" + ) + assert application_error.index("renderSessionList()") < application_error.index( + "_dispatchExtensionTurnLifecycle(_extensionErrorType" + ) + assert cancel.index("await api(") < cancel.index( + "_dispatchExtensionTurnLifecycle('turn:cancel'" + ) + assert cancel.index("_setActivePaneIdleIfOwner()") < cancel.index( + "_dispatchExtensionTurnLifecycle('turn:cancel'" + ) + assert connection_error.index("_setActivePaneIdleIfOwner()") < connection_error.index( + "_dispatchExtensionTurnLifecycle('turn:error'" + ) + + assert "_dispatchExtensionTurnLifecycle('turn:complete',completedSid" not in done + assert "_dispatchExtensionTurnLifecycle(_extensionErrorType,d.session_id" not in application_error + assert "_dispatchExtensionTurnLifecycle('turn:cancel',_cancelData.session_id" not in cancel diff --git a/tests/test_issue5720_reasoning_owner.py b/tests/test_issue5720_reasoning_owner.py index 6cc31e99..975a7e39 100644 --- a/tests/test_issue5720_reasoning_owner.py +++ b/tests/test_issue5720_reasoning_owner.py @@ -531,6 +531,7 @@ global.EventSource=FakeEventSource; const attachStart=messagesSrc.indexOf('function attachLiveStream('); const attachEnd=messagesSrc.indexOf('\nfunction transcript(){',attachStart); if(attachStart<0||attachEnd<0) throw new Error('attachLiveStream source boundary not found'); +eval(extractFunc(messagesSrc,'_dispatchExtensionTurnLifecycle')); eval(messagesSrc.slice(attachStart,attachEnd)); attachLiveStream('sid-1','stream-1'); const source=FakeEventSource.instances[0]; diff --git a/tests/test_issue6572_loadsession_compression_cleanup.py b/tests/test_issue6572_loadsession_compression_cleanup.py new file mode 100644 index 00000000..92d7fbf3 --- /dev/null +++ b/tests/test_issue6572_loadsession_compression_cleanup.py @@ -0,0 +1,97 @@ +"""Regression: ``loadSession`` must clear stale compression UI state on switch (#6572). + +Context +------- +The compression UI state (``window._compressionUi`` plus its elapsed timer, +composer session-lock, and the "Compressing context" barrier element) is +per-session. Before this fix, ``loadSession`` in ``static/sessions.js`` did not +clear it when switching sessions, so a compression card belonging to a prior +session leaked across the load and surfaced as a phantom "Compressing context" +barrier on a fresh session that never triggered compression. + +The fix clears the state near the top of ``loadSession`` — alongside the other +per-session resets (``_yoloEnabled=false``, ``stopClarifyPolling``, +``hideClarifyCard``) and before transcript loading — by calling the canonical +``clearCompressionUi()`` teardown (defined in ``static/ui.js``), with a +fail-soft ``window._compressionUi=null`` fallback if the function is not yet +defined: + + if(typeof clearCompressionUi==='function') clearCompressionUi(); + else window._compressionUi=null; + +This is safe for a compression genuinely running on the *target* session: +clearing only tears down browser-local UI state; an active automatic +compression re-surfaces from the target session's own SSE ``compressing`` +stream, and a manual compression is re-discovered from server job status when +its session loads. Releasing the composer session-lock does not signal or +cancel the backend compression. + +Per AGENTS.md the WEBUI suite intentionally avoids a node/jsdom dependency, so +this is a source-level grep + brace-balance assertion — the same convention the +rest of the JS regression suite uses (e.g. +``test_bg_task_complete_loadsession_stream_restart.py``). +""" +from __future__ import annotations + +import re +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent + + +def _read_sessions_js() -> str: + return (REPO_ROOT / "static" / "sessions.js").read_text() + + +def _load_session_body() -> str: + """Return the ``async function loadSession(`` source slice → next top-level + ``function`` / ``async function`` declaration.""" + js = _read_sessions_js() + start = js.index("async function loadSession(") + rest = js[start + 1 :] + m = re.search(r"\n(async function |function )", rest) + end = start + 1 + (m.start() if m else len(rest)) + return js[start:end] + + +def test_loadsession_clears_compression_ui_state(): + """loadSession must clear per-session compression UI state on switch.""" + body = _load_session_body() + # The canonical teardown must be invoked, with a fail-soft fallback so a + # partial script load cannot leave a dangling barrier/timer/lock. + assert "clearCompressionUi()" in body, ( + "loadSession must call clearCompressionUi() to clear stale per-session " + "compression state (#6572 phantom barrier)" + ) + assert re.search( + r"typeof\s+clearCompressionUi\s*===\s*['\"]function['\"]", body + ), "the clearCompressionUi call must be typeof-guarded" + assert re.search( + r"else\s+window\._compressionUi\s*=\s*null", body + ), "a fail-soft window._compressionUi=null fallback must guard the guarded call" + + +def test_loadsession_clears_compression_before_transcript_load(): + """The clear must precede transcript rendering so no phantom barrier shows. + + It must sit with the other early per-session resets (near hideClarifyCard), + not after renderMessages(), otherwise a stale barrier could paint for a + frame on the freshly loaded session. + """ + body = _load_session_body() + clear_idx = body.index("clearCompressionUi") + # Anchored to a sibling early reset that is known to run near the top. + assert "hideClarifyCard" in body, "expected hideClarifyCard reset in loadSession" + clarify_idx = body.index("hideClarifyCard") + # The compression clear should be co-located with the other early resets. + assert abs(clear_idx - clarify_idx) < 600, ( + "clearCompressionUi() should sit with the other early per-session resets " + "(near hideClarifyCard), before transcript loading" + ) + # And it must come before the success-tail render. + render_match = re.search(r"renderMessages\s*\(", body) + if render_match: + assert clear_idx < render_match.start(), ( + "clearCompressionUi() must run before renderMessages() so a stale " + "compression barrier never paints on the loaded session" + ) diff --git a/tests/test_issue6892_sync_title_coverage.py b/tests/test_issue6892_sync_title_coverage.py new file mode 100644 index 00000000..c3319e17 --- /dev/null +++ b/tests/test_issue6892_sync_title_coverage.py @@ -0,0 +1,95 @@ +"""Direct coverage for api.state_sync.sync_session_title (#6892). + +Codex round-1 SHOULD-FIX: the PR's only test change mocks sync_session_title to a +no-op, so the new state.db-writing path had no direct test proving it actually +persists a generated title AND preserves a manual rename. These tests exercise +the real function against a temporary hermes-agent SessionDB. +""" + +import pytest + + +def _make_db(tmp_path): + hermes_state = pytest.importorskip("hermes_state") + SessionDB = hermes_state.SessionDB + return SessionDB(db_path=tmp_path / "state.db") + + +@pytest.mark.requires_agent_modules +def test_sync_session_title_persists_generated_title(tmp_path, monkeypatch): + """A generated title is written to state.db so `hermes sessions list` isn't blank.""" + from api import state_sync + + db = _make_db(tmp_path) + try: + # Route sync_session_title at the real temp DB (a fresh handle each call, + # mirroring production, but pointed at our tmp state.db). + monkeypatch.setattr( + state_sync, + "_get_state_db", + lambda profile=None: _make_db(tmp_path), + ) + + state_sync.sync_session_title("sess-gen", "Generated Title", profile="default") + + assert db.get_session_title("sess-gen") == "Generated Title" + # Provenance is the automatic LLM source, not a user rename. + assert db.get_session_title_source("sess-gen") == db.TITLE_SOURCE_LLM + finally: + db.close() + + +@pytest.mark.requires_agent_modules +def test_sync_session_title_does_not_overwrite_manual_rename(tmp_path, monkeypatch): + """A user rename (source=user) is never clobbered by a later title sync.""" + from api import state_sync + + db = _make_db(tmp_path) + try: + # Seed a manual rename first — this records `user` provenance. + db.ensure_session(session_id="sess-manual", source="cli") + db.set_session_title("sess-manual", "My Manual Name") + assert db.get_session_title_source("sess-manual") == db.TITLE_SOURCE_USER + + monkeypatch.setattr( + state_sync, + "_get_state_db", + lambda profile=None: _make_db(tmp_path), + ) + + # A background generation now tries to sync an auto title — must be a no-op. + state_sync.sync_session_title("sess-manual", "Auto Generated", profile="default") + + assert db.get_session_title("sess-manual") == "My Manual Name" + assert db.get_session_title_source("sess-manual") == db.TITLE_SOURCE_USER + finally: + db.close() + + +@pytest.mark.requires_agent_modules +def test_sync_session_title_preserves_existing_source_on_ensure(tmp_path, monkeypatch): + """ensure_session(source='webui') must not clobber an existing cli/gateway row's source.""" + from api import state_sync + + db = _make_db(tmp_path) + try: + db.ensure_session(session_id="sess-cli", source="cli") + + monkeypatch.setattr( + state_sync, + "_get_state_db", + lambda profile=None: _make_db(tmp_path), + ) + + # Sync (which calls ensure_session(source='webui')) on a pre-existing cli row. + state_sync.sync_session_title("sess-cli", "Some Title", profile="default") + + # INSERT OR IGNORE semantics: the original source is preserved, the title lands. + assert db.get_session_title("sess-cli") == "Some Title" + with db._lock: + row = db._conn.execute( + "SELECT source FROM sessions WHERE id = ?", ("sess-cli",) + ).fetchone() + assert row["source"] == "cli" + finally: + db.close() diff --git a/tests/test_issue6935_persist_user_timestamp_kwarg.py b/tests/test_issue6935_persist_user_timestamp_kwarg.py new file mode 100644 index 00000000..2a6714dc --- /dev/null +++ b/tests/test_issue6935_persist_user_timestamp_kwarg.py @@ -0,0 +1,58 @@ +""" +Test for Issue #6935: Ensure agent.run_conversation() does not trip a TypeError +when persist_user_timestamp is passed to agent implementations that do not accept it. +""" +from api.streaming import _supports_kwarg + + +class LegacyAgentStub: + """Agent stub mimicking older hermes-agent whose run_conversation does NOT accept **kwargs or persist_user_timestamp.""" + def run_conversation(self, user_message, system_message=None, conversation_history=None, task_id=None, persist_user_message=None): + return {"final_response": "ok", "messages": []} + + +class ModernAgentWithKwargsStub: + """Agent stub whose run_conversation accepts **kwargs.""" + def run_conversation(self, **kwargs): + return {"final_response": "ok", "messages": []} + + +class ModernAgentWithExplicitParamStub: + """Agent stub whose run_conversation explicitly accepts persist_user_timestamp.""" + def run_conversation(self, user_message, persist_user_timestamp=None): + return {"final_response": "ok", "messages": []} + + +def test_supports_kwarg_detection(): + legacy_agent = LegacyAgentStub() + modern_kwargs_agent = ModernAgentWithKwargsStub() + modern_explicit_agent = ModernAgentWithExplicitParamStub() + + assert _supports_kwarg(legacy_agent.run_conversation, "persist_user_timestamp") is False + assert _supports_kwarg(modern_kwargs_agent.run_conversation, "persist_user_timestamp") is True + assert _supports_kwarg(modern_explicit_agent.run_conversation, "persist_user_timestamp") is True + + +def test_legacy_agent_run_conversation_kwargs_filtering(): + legacy_agent = LegacyAgentStub() + kwargs = { + "user_message": "Hello", + "system_message": "System prompt", + "conversation_history": [], + "task_id": "test-session-123", + "persist_user_message": "Hello", + "persist_user_timestamp": 1700000000.0, + } + + if not _supports_kwarg(legacy_agent.run_conversation, "persist_user_timestamp"): + kwargs.pop("persist_user_timestamp", None) + + # Calling run_conversation with filtered kwargs must not raise TypeError + result = legacy_agent.run_conversation(**kwargs) + assert result["final_response"] == "ok" + + +if __name__ == "__main__": + test_supports_kwarg_detection() + test_legacy_agent_run_conversation_kwargs_filtering() + print("All tests passed successfully!") diff --git a/tests/test_issue6964_title_dedup.py b/tests/test_issue6964_title_dedup.py new file mode 100644 index 00000000..63d190cf --- /dev/null +++ b/tests/test_issue6964_title_dedup.py @@ -0,0 +1,135 @@ +"""Regression tests for state.db title de-dup on collision (#6964). + +Follow-up to #6892 (sync auto-generated session titles to state.db). +When two WebUI sessions generate the SAME auto-title, hermes-agent's +state.db uniqueness rule makes the second ``set_auto_title_if_empty`` +raise ValueError, which ``sync_session_title`` used to swallow at debug +level -- leaving the second row blank in ``hermes sessions list``. +These tests prove the collision path now de-duplicates (suffix variant) +instead of leaving the row untitled, and that unique titles are unchanged. +""" + +import pytest + + +def _make_db(tmp_path): + hermes_state = pytest.importorskip("hermes_state") + SessionDB = hermes_state.SessionDB + return SessionDB(db_path=tmp_path / "state.db") + + +@pytest.mark.requires_agent_modules +def test_sync_session_title_dedups_second_session_on_collision(tmp_path, monkeypatch): + """Two sessions with the same auto-title: the 2nd gets a de-duplicated + variant instead of being left blank in state.db.""" + from api import state_sync + + db = _make_db(tmp_path) + try: + # Route sync_session_title at the real temp DB (a fresh handle each call, + # mirroring production, but pointed at our tmp state.db). + monkeypatch.setattr( + state_sync, + "_get_state_db", + lambda profile=None: _make_db(tmp_path), + ) + + state_sync.sync_session_title("sess-1", "Same Title", profile="default") + state_sync.sync_session_title("sess-2", "Same Title", profile="default") + + t1 = db.get_session_title("sess-1") + t2 = db.get_session_title("sess-2") + assert t1 == "Same Title" + # De-duplicated with a lineage suffix -- never left blank. + assert t2 == "Same Title #2" + assert t1 != t2 + finally: + db.close() + + +@pytest.mark.requires_agent_modules +def test_sync_session_title_dedup_handles_numbered_base(tmp_path, monkeypatch): + """A colliding title that already carries a '#N' suffix keeps incrementing + deterministically (e.g. 'Same Title #2' -> 'Same Title #3').""" + from api import state_sync + + db = _make_db(tmp_path) + try: + monkeypatch.setattr( + state_sync, + "_get_state_db", + lambda profile=None: _make_db(tmp_path), + ) + + state_sync.sync_session_title("sess-1", "Same Title #2", profile="default") + state_sync.sync_session_title("sess-2", "Same Title #2", profile="default") + + t1 = db.get_session_title("sess-1") + t2 = db.get_session_title("sess-2") + assert t1 == "Same Title #2" + assert t2 == "Same Title #3" + assert t1 != t2 + finally: + db.close() + + +@pytest.mark.requires_agent_modules +def test_sync_session_title_unique_titles_unchanged(tmp_path, monkeypatch): + """Non-colliding titles keep their exact value (no suffix added).""" + from api import state_sync + + db = _make_db(tmp_path) + try: + monkeypatch.setattr( + state_sync, + "_get_state_db", + lambda profile=None: _make_db(tmp_path), + ) + + state_sync.sync_session_title("sess-a", "Alpha Title", profile="default") + state_sync.sync_session_title("sess-b", "Beta Title", profile="default") + + assert db.get_session_title("sess-a") == "Alpha Title" + assert db.get_session_title("sess-b") == "Beta Title" + finally: + db.close() + + +@pytest.mark.requires_agent_modules +def test_sync_session_title_dedup_retry_never_clobbers_manual_rename(tmp_path, monkeypatch): + """The de-dup retry still respects title provenance: if the second session + was manually renamed (``user`` source) before the colliding auto-title sync + runs, the retry with the ``#2`` variant must NOT overwrite the user's name. + + Codex #6966 SHOULD-FIX: covers the manual-rename-during-retry interleaving + (the existing tests cover manual-rename-before-sync only indirectly).""" + from api import state_sync + + db = _make_db(tmp_path) + try: + monkeypatch.setattr( + state_sync, + "_get_state_db", + lambda profile=None: _make_db(tmp_path), + ) + + # sess-1 takes "Same Title" via the normal auto-title sync. + state_sync.sync_session_title("sess-1", "Same Title", profile="default") + + # sess-2 is manually renamed by the user (higher-ranked `user` source) + # BEFORE the colliding auto-title sync fires for it. + db.ensure_session(session_id="sess-2", source="webui") + db.set_session_title("sess-2", "My Own Name") + + # Now the colliding auto-title sync runs for sess-2. It collides on + # "Same Title", derives "Same Title #2", and retries -- but the retry + # goes through the same provenance rank guard, so the user's manual + # name must survive. + state_sync.sync_session_title("sess-2", "Same Title", profile="default") + + assert db.get_session_title("sess-1") == "Same Title" + # The manual (user-source) rename is preserved -- never clobbered by + # either the initial auto-title or the de-dup retry variant. + assert db.get_session_title("sess-2") == "My Own Name" + finally: + db.close() diff --git a/tests/test_sprint47.py b/tests/test_sprint47.py index c3c20a0f..d5060c67 100644 --- a/tests/test_sprint47.py +++ b/tests/test_sprint47.py @@ -63,7 +63,9 @@ def test_skill_mutations_invalidate_slash_skill_caches(): def test_builtin_commands_take_precedence_over_skill_slug_collisions(): assert "_getReservedSlashCommandSlugs" in COMMANDS_JS assert "if(_getReservedSlashCommandSlugs().has(slug)) return null;" in COMMANDS_JS - assert "if(!skill.name.startsWith(q)||seen.has(skill.name)||reserved.has(skill.name))continue;" in COMMANDS_JS + command_matching = _function_body(COMMANDS_JS, "getMatchingCommands") + assert "reserved.has(name)" in command_matching + assert "!name.includes(q)&&!description.includes(q)" in command_matching def test_bundle_entries_merge_before_plain_skill_entries():