diff --git a/.github/workflows/conversation-lifecycle.yml b/.github/workflows/conversation-lifecycle.yml index e952d34f..24ede84b 100644 --- a/.github/workflows/conversation-lifecycle.yml +++ b/.github/workflows/conversation-lifecycle.yml @@ -12,7 +12,9 @@ on: - 'api/**.py' - 'server.py' - 'tests/browser_conversation_lifecycle.py' + - 'tests/sidebar_terminal_barrier_server.py' - 'tests/browser_historical_transcript_hydration.py' + - 'tests/browser_reconnect_scene_redraw.py' - 'requirements*.txt' - 'pyproject.toml' - '.github/workflows/conversation-lifecycle.yml' @@ -23,7 +25,9 @@ on: - 'api/**.py' - 'server.py' - 'tests/browser_conversation_lifecycle.py' + - 'tests/sidebar_terminal_barrier_server.py' - 'tests/browser_historical_transcript_hydration.py' + - 'tests/browser_reconnect_scene_redraw.py' - 'requirements*.txt' - 'pyproject.toml' - '.github/workflows/conversation-lifecycle.yml' @@ -32,7 +36,8 @@ jobs: live-to-final: runs-on: ubuntu-latest timeout-minutes: 10 - continue-on-error: true + # Keep existing proof rows informational; reconnect regressions fail CI. + continue-on-error: ${{ matrix.name != 'reconnect-scene-redraw' }} strategy: fail-fast: false matrix: @@ -45,6 +50,9 @@ jobs: scenario: terminal-error - name: historical-transcript-hydration script: tests/browser_historical_transcript_hydration.py + - name: reconnect-scene-redraw + script: tests/browser_reconnect_scene_redraw.py + browsers: chromium steps: - uses: actions/checkout@v4 @@ -69,6 +77,7 @@ jobs: LIFECYCLE_ARTIFACT_DIR: lifecycle-artifacts HISTORICAL_HYDRATION_ARTIFACT_DIR: lifecycle-artifacts LIFECYCLE_SCENARIO: ${{ matrix.scenario }} + BROWSERS: ${{ matrix.browsers }} run: python ${{ matrix.script }} - name: Run settlement frame proof @@ -79,6 +88,14 @@ jobs: LIFECYCLE_TEST_BITE: settle-worklog-frame-proof run: python tests/browser_conversation_lifecycle.py + - name: Recover a missing terminal frame + if: matrix.scenario == 'normal' + env: + LIFECYCLE_ARTIFACT_DIR: lifecycle-artifacts/missing-terminal + LIFECYCLE_SCENARIO: normal + LIFECYCLE_TEST_BITE: sidebar-idle-missing-terminal + run: python tests/browser_conversation_lifecycle.py + - name: Upload failure artifacts if: failure() uses: actions/upload-artifact@v4 diff --git a/CHANGELOG.md b/CHANGELOG.md index 8962c4ab..b224e768 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,10 @@ ### Added +- **Full-session resolve concurrency is configurable.** `HERMES_WEBUI_MAX_SESSION_RESOLVE` sets how + many full-transcript session resolves may run at once (default 2, a positive integer up to 64; + zero, negative, non-numeric or out-of-range values fall back to 2). It is process-wide, so a + profile's `.env` cannot override it. (#7421, #7656 by @happy5318) - **The sidebar's recent-session window is configurable.** `HERMES_WEBUI_VISIBLE_SESSION_LIMIT` sets how many recent sessions the sidebar lists (default 20). It also bounds how many delegated subagent children can nest at once, so raise it for wide fan-outs. Invalid or non-positive values fall back to @@ -24,6 +28,54 @@ ### Fixed +- **A first visit now uses the browser's language.** The server stores "no preference" as `null` + instead of defaulting to `"en"`, so a first-time visitor gets `navigator.language` while an + explicitly saved language (including English) still wins, and legacy `settings.json` files that + already hold `"en"` keep English. Reading the browser language is guarded, so an environment where + `navigator` throws falls back cleanly. (#7622, #7730 by @happy5318) +- **The live model list for a custom provider respects its `models:` allowlist, without treating + per-model metadata as one.** `/api/models/live` filters a custom provider's live catalog to its + configured `models:` when that is a list, or when it is a mapping with `discover_models: false`. + A mapping of per-model settings (the shape `hermes setup` writes) keeps the full live catalog, as + the Agent does. (#7165 by @happy5318) +- **`MEDIA:` links wrapped in inline code no longer 404.** Every `MEDIA:` capture site (renderer, + streaming parser, TTS stripper, session-media authorization and snapshot capture, seven in all) + swallowed the closing backtick of `` `MEDIA:/path` `` into the path, so the file lookup and the + session allowlist both missed. Backtick-wrapped refs are now normalized first, while bare paths that + genuinely contain a backtick keep their full name. (#7359, #7708 by @happy5318) +- **A turn's Worklog no longer vanishes when the sidebar reports idle before the final frame.** + `/api/sessions` could say a session was idle before the chat stream's terminal frame reached the + page, and three sidebar paths (idle reconciliation, the INFLIGHT purge and optimistic-row + retirement) then erased the pane's Worklog and stream id, so the late `done` was rejected as stale + and the settled scene was never saved. The cleanup now waits briefly for the pane's own open + stream, then checks `/api/chat/stream/status` once, and falls back to the existing + interrupted-stream recovery if that check fails. (#7749 by @franksong2702) +- **A session's run journal can no longer be written out of order.** The journal writer reserved a + sequence number under the per-path lock, released it, and then appended, so two concurrent writers + could land sequence N+1 on disk before N and the replay reader would stop at the gap + (`replay_noncontiguous`). Sequence allocation and the physical append now happen under the same + existing lock. (#7751 by @franksong2702) +- **A stopped chat can no longer publish or reuse its agent after Stop.** A worker that was + cancelled could still publish its cached Agent or invoke it after Stop landed, and its late + cleanup could close or evict the same Agent object a successor turn had just picked up. The initial + path and both credential self-heal paths now take one Stop admission, the cancellation event, live + stream and exact Agent are rechecked immediately before invocation (no registry lock is held across + provider or tool execution), and cache/lifecycle handles are retired only while the surviving owner + still matches. (#7748 by @franksong2702) +- **Reconnecting to a running session no longer redraws every tool card over and over.** After a + reconnect restored the live activity scene from the run journal, the client also replayed its older + cached in-flight tool list on top, so a turn with N tool cards redrew them N times. When the + journal-backed scene restores successfully the replay is now skipped (newer rows still arrive + through the reattached stream); the replay stays for legacy restores and for a failed or + unavailable scene. (#7436, @atchisonbrent) + +- **A settled assistant answer is no longer shown twice (#2051).** Two client-side paths could put a + second copy of a finished answer on screen after a turn settled: a stale live-turn node that was + still treated as live, and a settled rebuild branch that appended a turn instead of replacing it. + The same fix stops an interrupted turn's notice from appearing twice, and stops the composer's + model picker from listing one configured custom-provider model twice. The stored transcript was + always correct; only the rendering duplicated it. (#7374, @Thireus) + - **A hidden browser tab stops polling a session that was deleted.** When a tab is in the background, its stream poll kept asking for a session every few seconds after it was deleted or archived away, forever. A `404`/`410` for the polled session now stops that poll; `503` and network @@ -220,6 +272,7 @@ ### Changed +- **Two flaky tests are stable again.** Three `get_available_models()` cache-metadata tests raced the 4-second live-rebuild budget on a loaded CI host and could time out onto the stale-cache path. They now pin the budget to `0`, which is the documented setting for the legacy unbounded synchronous rebuild, so they still exercise the real rebuild (#7735). A source-text oracle that string-matched `delete_cli_session`'s source to prove it opens a writable connection is removed. The behavioral test in `test_issue1494_state_db_fd_leak.py` still guards that contract: it fails with `attempt to write a readonly database` if the delete path is ever switched to a read-only connection (#7726). Test-only; no runtime change. Thanks @webtecnica. (#7744, #7742) - **The chat composer grows natively instead of being resized by JavaScript on every keystroke.** `autoResize()` measured `scrollHeight` and wrote `style.height` on each input event — a forced synchronous reflow on the most-typed-in surface in the app. Browsers that support CSS `field-sizing: content` (Chromium today; also Firefox 152 and Safari 26.2) now own the geometry directly, gated on `CSS.supports()`, and the existing JavaScript path is untouched for every other engine. Measured behaviour is identical across both paths: 44px resting height, no jump when the first character is typed or the last deleted, growth to the 200px ceiling, then internal scrolling. Because `field-sizing` deliberately includes placeholder text in content sizing, `:placeholder-shown` pins fixed sizing while the composer is empty so a long placeholder can't inflate it. Thanks @starship-s. (#6760, #5514) ### Fixed diff --git a/TESTING.md b/TESTING.md index af111974..f96a4f88 100644 --- a/TESTING.md +++ b/TESTING.md @@ -154,12 +154,35 @@ HISTORICAL_HYDRATION_TEST_BITE=break-tool-link \ python tests/browser_historical_transcript_hydration.py ``` -The dedicated `Conversation lifecycle (informational)` workflow runs the current -proof rows (`normal`, `terminal-error`, and `historical-transcript-hydration`) and -stays non-blocking while the public -matrix expands to additional behavior rows. The maintainer's private QA harness -remains broader; later public slices will add session switching, reconnect/replay, -cancellation, compression, and recovery. +The dedicated `Conversation lifecycle (informational)` workflow keeps the existing +proof rows (`normal`, `terminal-error`, and `historical-transcript-hydration`) +non-blocking while the public matrix expands. The Chromium +`reconnect-scene-redraw` row does **not** allow failures: it exercises the real +`loadSession` reconnect path and fails the workflow on a regression. Required +merge checks remain a maintainer-controlled repository setting. +The maintainer's private QA harness remains broader; later public slices will +add cancellation, compression, and recovery coverage. + +### Active-session reconnect redraw gate + +`tests/browser_reconnect_scene_redraw.py` is an opt-in deterministic browser +gate for reconnecting to a running session with a large Anchor activity scene. +It uses an isolated temporary server/home and fixture SSE events—no agent, +provider credentials, or production state. By default it checks Chromium and +WebKit, desktop and 390px viewports, and all three activity display modes: + +```bash +pip install playwright +python -m playwright install chromium webkit +python tests/browser_reconnect_scene_redraw.py +``` + +`TOOL_COUNT`, `BROWSERS`, and `MODES` narrow the matrix. `MEASURE_BASELINE=1` +records an unoptimized baseline without enforcing the redraw ceiling; +`METRICS_FILE` saves JSON metrics and `SCREENSHOT_DIR` saves generated-fixture +screenshots. This gate tests page reconstruction, journal-cursor resume, and +subsequent fixture SSE updates. It does not test a real provider/network or PWA +service-worker cache behavior. ### Streaming reader intent diff --git a/api/config.py b/api/config.py index 5c38d375..27e10cf9 100644 --- a/api/config.py +++ b/api/config.py @@ -11483,7 +11483,13 @@ _SETTINGS_DEFAULTS = { "hidden_tabs": [], # sidebar tab panel names hidden by user (e.g. ["tasks","kanban"]); chat and settings are always visible "tab_order": [], # user-defined sidebar/rail tab order for reorderable tabs; chat/settings stay fixed "composer_control_order": [], # user-defined composer footer control order; invalid/duplicate keys are ignored - "language": "en", # UI locale code; must match a key in static/i18n.js LOCALES + # #7622 (round 3): language is intentionally absent from the defaults so + # `load_settings()` reports `None` for a fresh install. This lets the + # client distinguish "no preference" from an explicit saved choice, + # so a user who genuinely picked English (and has `language: "en"` + # persisted on disk) is no longer overridden by the browser hint on + # their first hydration. When a user picks a locale in the Settings + # modal the value is written here and the field is set explicitly. "bot_name": os.getenv( "HERMES_WEBUI_BOT_NAME", "Hermes" ), # display name for the assistant @@ -11723,6 +11729,16 @@ def load_settings() -> dict: settings["default_model_provider"] = str(model_cfg.get("provider")) except Exception: logger.debug("Failed to resolve default model provider for settings") + # #7622/#7730 (round 4): keep the tri-state signal explicit. `language` + # is intentionally absent from `_SETTINGS_DEFAULTS` (see above), so + # without this line a fresh install's returned dict would OMIT the key + # entirely and the API payload would carry no `language` field. Emit an + # explicit `None` (serialized as JSON `null`) so the client receives the + # three-way signal it trusts: `null` = no preference, "en" = explicitly + # saved English, any other code = explicitly chosen locale. Stored + # values (including a legacy English pick written before this change) + # win via the merge above and are never touched here. + settings.setdefault("language", None) return settings @@ -11730,6 +11746,15 @@ _SETTINGS_ALLOWED_KEYS = set(_SETTINGS_DEFAULTS.keys()) - { "password_hash", "default_model", "simplified_tool_calling", +} | { + # #7622 (round 3): `language` is intentionally absent from + # `_SETTINGS_DEFAULTS` so a fresh install returns `None` for + # `settings["language"]` and the client can distinguish "no + # preference" from an explicit saved choice. But the user + # MUST still be able to pick a locale in the Settings modal, + # so we add it back to the explicit allow-list here. The + # existing BCP-47 validation at save-time still applies. + "language", } _SETTINGS_ENUM_VALUES = { "send_key": {"enter", "ctrl+enter", "shift+enter"}, diff --git a/api/media_snapshots.py b/api/media_snapshots.py index 929c1431..e54c34d8 100644 --- a/api/media_snapshots.py +++ b/api/media_snapshots.py @@ -505,6 +505,13 @@ def annotate_media_snapshots( if allowed_predicate is None: allowed_predicate = media_capture_allowed media_re = _re.compile(r"MEDIA:([^\s\)\]]+)") + # #7680 re-gate (9/22): two-pass scan. First strip backtick + # wrappers (`` `MEDIA:path` `` → ``MEDIA:path``) so the bare + # class below does not consume the closing backtick as part of + # the path. Then the bare class (no backtick in the exclusion + # set) captures the full filename even when the path itself + # contains a backtick (e.g. ``report`final.png``). + backtick_re = _re.compile(r"`MEDIA:([^`\s]+)`") captured = 0 for msg in messages or []: if not isinstance(msg, dict) or msg.get("role") != "assistant": @@ -530,6 +537,7 @@ def annotate_media_snapshots( if not text_parts: continue text = "\n".join(text_parts) + text = backtick_re.sub(lambda m: f"MEDIA:{m.group(1)}", text) refs = media_re.findall(text) if not refs: continue diff --git a/api/models.py b/api/models.py index dd775c68..0322a97b 100644 --- a/api/models.py +++ b/api/models.py @@ -4788,7 +4788,41 @@ class _FullSessionResolveRequired(RuntimeError): """Internal signal that a full sidecar load must enter the bounded path.""" -_FULL_SESSION_RESOLVE_MAX_CONCURRENT = 2 +def _read_max_session_resolve_concurrent() -> int: + """#7421: env override for the heavy session-resolve cap. The + hardcoded literal 2 is too low for high-concurrency + deployments where several parallel active sessions all need + a full-transcript resolve; operators can set + ``HERMES_WEBUI_MAX_SESSION_RESOLVE`` to a positive int to + raise the cap without code changes. A bad or missing value + falls back to the previous default of 2, so an operator who + sets ``HERMES_WEBUI_MAX_SESSION_RESOLVE=0`` or a non-numeric + value does not regress to a blocked or unbounded state. + + The cap is a *safety bound* (per-resolve cost tracked in + #7310), not a user preference — a typo like ``=999999`` + must fall back to the safe default, never to the permissive + extreme. Every other bad input in this function falls back + to ``2``; out-of-range values do too. The previously + accepted upper bound (64) is reachable only via an explicit + in-range operator value; a profile's ``.env`` cannot reach + it at all (see ``_PROTECTED_ENV_KEYS``). + """ + raw = (os.getenv("HERMES_WEBUI_MAX_SESSION_RESOLVE") or "").strip() + if not raw: + return 2 + try: + n = int(raw) + except (TypeError, ValueError): + return 2 + if n < 1: + return 2 + if n > 64: + return 2 # #7656 round-3: typo-safe fallback, not clamp-up-to-64 + return n + + +_FULL_SESSION_RESOLVE_MAX_CONCURRENT = _read_max_session_resolve_concurrent() _FULL_SESSION_RESOLVE_SLOTS = threading.BoundedSemaphore( _FULL_SESSION_RESOLVE_MAX_CONCURRENT ) diff --git a/api/profiles.py b/api/profiles.py index a4b5d8f7..e56b32fd 100644 --- a/api/profiles.py +++ b/api/profiles.py @@ -202,6 +202,14 @@ _PROTECTED_ENV_KEYS = frozenset({ 'HERMES_WEBUI_ISOLATED_PROFILE', # Server-wide sidebar window; a profile .env must not widen the per-request query. 'HERMES_WEBUI_VISIBLE_SESSION_LIMIT', + # #7656 round-3: HERMES_WEBUI_MAX_SESSION_RESOLVE is a process-wide resource + # cap (controls the BoundedSemaphore that gates full-transcript resolves). + # A profile's .env is per-profile by design; allowing it to set a + # server-wide limit (and a first-load-wins asymmetry) would let a + # single profile drag the rest into a tighter or looser posture than + # the operator intended. Same shape as the isolated-profile key: only + # the operator/launcher env at startup can set it. + 'HERMES_WEBUI_MAX_SESSION_RESOLVE', }) @@ -939,6 +947,12 @@ _BLOCKED_RUNTIME_ENV_KEYS = { # profile's own env on any runtime/gateway-parity path. 'HERMES_WEBUI_ISOLATED_PROFILE', 'HERMES_WEBUI_VISIBLE_SESSION_LIMIT', + # #7656 round-3: process-wide resolve cap. Filter out a profile's + # .env override the same way we filter the isolated-profile key, so + # the cap stays at the operator/launcher value across profile + # switches rather than re-sizing to whatever the first-loaded + # profile env set. + 'HERMES_WEBUI_MAX_SESSION_RESOLVE', } diff --git a/api/routes.py b/api/routes.py index 407abbac..27e3c912 100644 --- a/api/routes.py +++ b/api/routes.py @@ -20641,6 +20641,16 @@ def _serve_inline_html_preview(handler, target: Path, cache_control: str, *, csp _MEDIA_TOKEN_RE = re.compile(r"MEDIA:([^\s\)\]]+)") +# #7680 re-gate (9/22): two-pass scan. +# 1. `` `MEDIA:path` `` (backtick-wrapped, inline-code form) → strip +# the wrapping backticks so the bare-token pass below sees a +# plain ``MEDIA:path`` and the closing backtick is not consumed +# as part of the path. +# 2. ``MEDIA:[^\s\)\]]+`` (bare, no backtick in the exclusion +# class) so a filename that legally contains a backtick +# (``report`final.png``) is captured in full instead of being +# truncated at the first backtick. +_BACKTICK_MEDIA_RE = re.compile(r"`MEDIA:([^`\s]+)`") def _message_content_text(content) -> str: @@ -20700,6 +20710,10 @@ def _session_media_token_allows_path(sid: str, target: Path, allowed_mimes: set[ ) if "MEDIA:" not in text: continue + # #7680 re-gate: strip backtick wrappers first so the bare + # class below captures the full path even when the filename + # itself contains a backtick. + text = _BACKTICK_MEDIA_RE.sub(lambda m: f"MEDIA:{m.group(1)}", text) for ref in _MEDIA_TOKEN_RE.findall(text): if "://" in ref: continue @@ -21892,6 +21906,99 @@ def _handle_live_models(handler, parsed): _env = str(_cp.get("key_env") or "").strip() return os.getenv(_env, "").strip() if _env else "" + def _custom_provider_models_discover_is_false(_cp): + """True when ``discover_models`` is an explicit ``false`` opt-out. + + Mirrors ``api.config._provider_discover_allowed`` / Hermes + Agent ``model_switch_providers._discover_flag``: ``discover_models`` + defaults to True, and the string forms ``"false"``/``"no"``/``"0"`` + (case-insensitive) mean False. Used to decide whether a + dict-shaped ``models`` mapping is a hand-pinned allowlist (only + when discovery is off) rather than per-model metadata. + """ + _discover = _cp.get("discover_models", True) if isinstance(_cp, dict) else True + if isinstance(_discover, str): + return _discover.strip().lower() in {"false", "no", "0"} + return not bool(_discover) + + def _custom_provider_allowlist_ids(_cp): + """Plural ``models`` list only — the explicit allowlist signal. + + The singular ``model`` field is sticky/default metadata, NOT an + allowlist: it must not gate live-catalog filtering, otherwise a + provider configured with only ``model: assistant`` (no ``models`` + list) would be collapsed to a single model. Only an explicit + ``models`` allowlist expresses "show exactly these models". + + An auto-discovered catalog is NOT an allowlist: when Hermes + persisted discovery results back into config (``models: {...}`` + plus ``models_discovered: true``), that mapping is a snapshot + of what the gateway exposed at discovery time. Gating on it + would permanently pin the live catalog to the first-discovery + set, silently dropping any model the user pulls in later + (LM Studio / Ollama). ``_provider_models_are_discovered_catalog()`` + is the shared predicate the ``/api/models`` path already uses; + when it says "discovered", return no allowlist and let the + live probe win. An explicit ``discover_models: false`` opt-out + re-pins the catalog (the predicate accounts for it), so a + hand-pinned discovered catalog still filters. + + A dict-shaped ``models`` mapping that is NOT marked discovered is + *per-model metadata* written by the Hermes Agent setup flow + (``hermes_cli/model_switch.py::_save_custom_provider`` and the + setup wizard) — e.g. ``{chat-a: {context_length: 128000}}``. It is + not a catalog narrow: treating its keys as an allowlist would + collapse the live picker to the single saved default (keyless + Ollama) while the CLI live-probe shows the full catalog. Only an + explicit ``discover_models: false`` opts into treating the dict + keys as a pinned allowlist. List/JSON-array-string/Python-literal + shapes remain plain allowlists. + + The plural value is decoded through ``_parse_config_string_list()`` + because ``hermes config set`` / JSON-mode editor saves persist + lists as quoted JSON-array strings (``'["chat-a","chat-b"]'``) or + Python literals (``"['chat-a']"``). Handling only ``dict``/``list`` + made a serialized allowlist fall through to ``[]``, which the + caller reads as "no allowlist configured" and floods the picker + with the full upstream catalog — the same class of bug as the + ``skills.disabled`` regression (#7120 / #7134). + """ + from api.config import _provider_models_are_discovered_catalog + + if _provider_models_are_discovered_catalog(_cp): + return [] + _ids = [] + _models = _cp.get("models") + # Serialized shapes only: ``hermes config set`` / JSON-mode + # editor saves persist lists as quoted JSON-array strings + # (``'["chat-a","chat-b"]'``) or Python literals + # (``"['chat-a']"``). Decode those through the shared helper so + # they are honored; native dict/list values are walked below so + # dict *entries* keep their id|model|name metadata (the decoder + # str()-ifies list members, which would mangle them). + if isinstance(_models, str): + _models = _parse_config_string_list(_models) + if isinstance(_models, dict): + # A dict-shaped ``models`` is per-model metadata written by + # the Hermes Agent setup flow, NOT a catalog narrow. Only a + # ``discover_models: false`` opt-out treats the dict keys as a + # pinned allowlist; otherwise return the empty allowlist so the + # live probe returns the full catalog. + if not _custom_provider_models_discover_is_false(_cp): + return [] + for _mid in _models: + if isinstance(_mid, str) and _mid.strip(): + _ids.append(_mid.strip()) + elif isinstance(_models, (list, tuple)): + for _item in _models: + if isinstance(_item, str) and _item.strip(): + _ids.append(_item.strip()) + elif isinstance(_item, dict): + _mid = _item.get("id") or _item.get("model") or _item.get("name") + if _mid and str(_mid).strip(): + _ids.append(str(_mid).strip()) + return _ids + # For 'custom' and 'custom:*' providers, provider_model_ids() # returns [] because they aren't real hermes_cli endpoints. # Fall back to the custom_providers entries from config.yaml so @@ -21900,11 +22007,13 @@ def _handle_live_models(handler, parsed): # Collect config-specified model IDs separately so they don't # prevent the live fetch below from running (#3718). _config_ids = [] + _allowlist_ids = [] if provider == "custom" or provider.startswith("custom:"): for _cp in _custom_provider_entries_for_request(): if custom_provider_entry is None: custom_provider_entry = _cp _config_ids.extend(_custom_provider_model_ids(_cp)) + _allowlist_ids.extend(_custom_provider_allowlist_ids(_cp)) # Always try live fetch for custom providers — config entries are a # fallback, not a replacement. The live endpoint should return ALL @@ -21975,15 +22084,39 @@ def _handle_live_models(handler, parsed): except Exception as _fetch_err: logger.debug("Live fetch from custom provider failed: %s", _fetch_err) - # If live fetch succeeded, merge with config entries (live takes - # priority). If live fetch failed, fall back to config-only list. + # If live fetch succeeded, filter the live catalog down to the + # config-declared models allowlist first, then append any + # allowlisted models the live endpoint didn't return. Custom + # providers (esp. New-API-style gateways) expose their ENTIRE + # catalog via /v1/models — including image/audio models that + # are not chat models. The picker must not surface models the + # user never declared in config.yaml. Only a NON-EMPTY explicit + # ``models`` allowlist gates the filter — a provider configured + # with just a singular ``model`` (no ``models`` list) keeps the + # unfiltered live catalog. When no allowlist is configured, + # return the live list as-is (preserves the pre-filter + # discovery behaviour). + # + # An empty allowlist (``models: []``, ``models: "[]"``, or a + # value that decodes to no usable ids) is deliberately treated + # as "not configured", NOT as "allow nothing". Gating on + # declared-ness instead would emit an EMPTY picker and make the + # provider unselectable — a harder failure than surfacing a few + # extra models, and unrecoverable from the UI because + # ``custom_providers`` is hand-edited in config.yaml (the WebUI + # has no write path for it). ``[]`` in practice means a + # leftover/placeholder key, not an intentional deny-all, and no + # deny-all use case exists: a provider the user wants hidden is + # removed from ``custom_providers`` outright. if ids: - _live_set = set(ids) - for _cid in _config_ids: - if _cid not in _live_set: - ids.append(_cid) + if _allowlist_ids: + _allowlist_set = set(_allowlist_ids) + ids = [m for m in ids if m in _allowlist_set] or [] + for _aid in _allowlist_ids: + if _aid not in ids: + ids.append(_aid) else: - ids = list(_config_ids) + ids = list(_allowlist_ids or _config_ids) # ── OpenAI-compat live fetch fallback ────────────────────────────────── # When provider_model_ids() is unavailable or returns [] for a provider diff --git a/api/run_journal.py b/api/run_journal.py index 087a08ba..48cd8ef7 100644 --- a/api/run_journal.py +++ b/api/run_journal.py @@ -451,8 +451,6 @@ class RunJournalWriter: self.session_id = _validate_id(session_id, "session_id") self.run_id = _validate_id(run_id, "run_id") self.session_dir = Path(session_dir) if session_dir is not None else None - self._path = _run_path(self.session_id, self.run_id, session_dir=self.session_dir) - self._lock = _lock_for(self._path) def append_sse_event(self, event_name: str, payload=None) -> dict | None: # Live-UI-only telemetry (metering) has no recovery value in the journal: @@ -464,18 +462,15 @@ class RunJournalWriter: # the offline-gap coverage and replay-cursor contiguity checks rely on. if str(event_name or "").strip() in REPLAY_SKIPPED_SSE_EVENTS: return None - # Draw from the shared module-level seq cache under the per-path lock so - # this writer and any direct append_run_event() call on the same path - # agree on one monotonic, gapless sequence. - with self._lock: - seq = _reserve_next_seq(self._path) + # Allocate the sequence inside the same per-path transaction that writes + # the row. Reserving here, then releasing the lock before append, lets a + # concurrent writer put a higher sequence on disk first. return append_run_event( self.session_id, self.run_id, event_name, payload or {}, session_dir=self.session_dir, - seq=seq, ) diff --git a/api/streaming.py b/api/streaming.py index b61490fb..4dfa5b63 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -9791,6 +9791,84 @@ def _run_agent_streaming( _streaming_skill_home_snapshot = None _restore_streaming_skill_home_modules = False _acquired_streaming_skill_home_patch_lock = False + def _register_agent_if_current(candidate, cache_signature=None): + nonlocal agent + # Every constructor, including both credential self-heal branches, must + # share the Stop publication edge. CANCEL_FLAGS may already be detached; + # the worker-retained event and stream membership remain authoritative. + with STREAMS_LOCK: + cancelled = cancel_event.is_set() or stream_id not in STREAMS + if not cancelled: + AGENT_INSTANCES[stream_id] = candidate + # Check and publication share one admission, not a check followed + # by an unlocked cache/lifecycle write that could undo Stop. + if cache_signature is not None: + from api.config import SESSION_AGENT_CACHE, SESSION_AGENT_CACHE_LOCK + with SESSION_AGENT_CACHE_LOCK: + SESSION_AGENT_CACHE[session_id] = (candidate, cache_signature) + SESSION_AGENT_CACHE.move_to_end(session_id) + if not ephemeral: + try: + # Pure in-memory bookkeeping; no Agent/provider call. + from api.session_lifecycle import register_agent + register_agent(session_id, candidate) + except Exception: + logger.debug("Lifecycle register_agent failed for session %s", session_id, exc_info=True) + if not cancelled: + return True + # A cache hit is a borrowed reusable object, not this worker's private + # candidate. Stop may already have let a successor reuse it. Rejected + # cache-hit admission must not interrupt that other turn. + if cache_signature is None and not ephemeral: + if agent is candidate: + agent = None + return False + # Newly constructed candidates were never published on this failed + # admission; they cannot have been borrowed by a successor from us. + # No Agent call, session persistence or SSE write under registry locks. + try: + candidate.interrupt("Cancelled before start") + except Exception: + logger.debug("Failed to interrupt cancelled candidate agent") + return False + + def _agent_can_invoke(candidate): + nonlocal agent + # Prompt preparation and LRU cleanup can yield after registration. Admit + # each invocation again at its point of use; never hold registry locks + # across run_conversation or interrupt. A later Stop interrupts the + # already-admitted invocation through the registered Agent as usual. + from api.config import ( + SESSION_AGENT_CACHE, SESSION_AGENT_CACHE_LOCK, + SESSION_WRITEBACK_OWNERS, SESSION_WRITEBACK_OWNERS_LOCK, + ) + with STREAMS_LOCK: + current = (not cancel_event.is_set() and stream_id in STREAMS + and AGENT_INSTANCES.get(stream_id) is candidate) + if not current: + if AGENT_INSTANCES.get(stream_id) is candidate: + AGENT_INSTANCES.pop(stream_id, None) + # Agent identity is not turn identity: a successor can reuse the + # same cached object. Use the existing cancellation-surviving + # ownership record, holding its lock through cache retirement. + # Missing ownership also fails closed (successor may have ended). + with SESSION_WRITEBACK_OWNERS_LOCK: + if SESSION_WRITEBACK_OWNERS.get(session_id) == stream_id: + with SESSION_AGENT_CACHE_LOCK: + entry = SESSION_AGENT_CACHE.get(session_id) + if entry and entry[0] is candidate: + SESSION_AGENT_CACHE.pop(session_id, None) + # Dirty memory segments keep their original owner. + from api.session_lifecycle import unregister_agent + unregister_agent(session_id) + # This invocation never started. Stop owns any required interrupt of the + # running Agent; a late duplicate interrupt could hit its new borrower. + # Drop our local borrowed handle too: final Steer drain must not reach + # the successor through the old worker's fallback `agent` reference. + if not current and agent is candidate: + agent = None + return current + # Initialised here (before any code that may raise) so the outer `finally` # block can safely check `if _checkpoint_stop is not None` even when an # exception fires before the checkpoint thread is created (Issue #765). @@ -11013,6 +11091,7 @@ def _run_agent_streaming( # ── Agent cache: reuse across messages in the same session ── # Mirrors gateway _agent_cache. Keeps _user_turn_count alive so # injectionFrequency: "first-turn" actually suppresses after turn 1. + _cache_new_agent = False if ephemeral: agent = _AIAgent(**_agent_kwargs) logger.debug('[webui] Created ephemeral agent for session %s', session_id) @@ -11055,14 +11134,6 @@ def _run_agent_streaming( session_id, _cached_agent_session_identity(_cached_agent), ) - if agent is not None: - # Reopened/cache-hit sessions must register the agent - # so later lifecycle commits can find it. - try: - from api.session_lifecycle import register_agent - register_agent(session_id, agent) - except Exception: - logger.debug("Lifecycle register_agent failed for cached session %s", session_id, exc_info=True) if _identity_mismatch_entry is not None: try: @@ -11132,82 +11203,65 @@ def _run_agent_streaming( agent._interrupt_message = None else: agent = _AIAgent(**_agent_kwargs) - # Register the new agent with the memory lifecycle so - # its commit_memory_session() can be found later. - try: - from api.session_lifecycle import register_agent - register_agent(session_id, agent) - except Exception: - logger.debug("Lifecycle register_agent failed for new session %s", session_id, exc_info=True) - _evicted_items = [] - # Snapshot the set of session_ids with a LIVE agent worker - # BEFORE taking SESSION_AGENT_CACHE_LOCK, so LRU eviction never - # closes an agent mid-run AND we never nest ACTIVE_RUNS_LOCK - # inside SESSION_AGENT_CACHE_LOCK (avoids any lock-ordering - # deadlock). A cancel/reconnect can drop STREAMS while the - # worker is still unwinding or blocked in a provider call, so - # ACTIVE_RUNS (worker lifecycle) is the authoritative liveness - # signal, not STREAMS. (#3536 review round 2) - _active_sids = set() - try: - from api.config import ACTIVE_RUNS, ACTIVE_RUNS_LOCK - with ACTIVE_RUNS_LOCK: - for _entry in (ACTIVE_RUNS or {}).values(): - _sid = (_entry or {}).get("session_id") - if _sid: - _active_sids.add(_sid) - except Exception: - _active_sids = set() - with SESSION_AGENT_CACHE_LOCK: - SESSION_AGENT_CACHE[session_id] = (agent, _agent_sig) - SESSION_AGENT_CACHE.move_to_end(session_id) # LRU: mark as recently used - from api.config import SESSION_AGENT_CACHE_MAX - # Evict the oldest INACTIVE entries first. Walk LRU order - # (front = oldest); skip any session with a live run. If - # every over-cap entry is active, leave the cache - # temporarily above cap rather than close a live worker's - # agent — a later insertion/finalization trims it once the - # run ends. - while len(SESSION_AGENT_CACHE) > SESSION_AGENT_CACHE_MAX: - _evictable_sid = None - for _sid in list(SESSION_AGENT_CACHE.keys()): - if _sid not in _active_sids: - _evictable_sid = _sid - break - if _evictable_sid is None: - break # all over-cap entries are active; defer - evicted_entry = SESSION_AGENT_CACHE.pop(_evictable_sid) - _evicted_items.append((_evictable_sid, evicted_entry)) - # Commit and close evicted agents outside the cache lock so - # concurrent cache users are not blocked by provider I/O. - for _evicted_sid, _evicted_entry in _evicted_items: - try: - _evicted_agent = _evicted_entry[0] if isinstance(_evicted_entry, tuple) else None - _close_evicted_agent_at_session_boundary(_evicted_sid, _evicted_agent) - except Exception: - logger.debug("Failed to close evicted agent for session %s", _evicted_sid, exc_info=True) - logger.debug('[webui] Evicted LRU agent from cache: %s', _evicted_sid) - logger.debug('[webui] Created new agent for session %s', session_id) + _cache_new_agent = True - # Stop may already have detached CANCEL_FLAGS while initialization - # was in flight. The worker-owned event and stream membership, not - # the removable registry entry, fence registration with cancellation. - with STREAMS_LOCK: - _cancelled_before_start = cancel_event.is_set() or stream_id not in STREAMS - if not _cancelled_before_start: - AGENT_INSTANCES[stream_id] = agent - if _cancelled_before_start: - # Interrupt, persistence and event writes must not hold the - # registry lock (the session lock can be held by another caller). - try: - agent.interrupt("Cancelled before start") - except Exception: - logger.debug("Failed to interrupt agent before start") + if not _register_agent_if_current(agent, _agent_sig if _cache_new_agent else None): with _agent_lock: _finalize_cancelled_turn(s, ephemeral=ephemeral, message='Task cancelled before start.', stream_id=stream_id) put('cancel', _cancel_event_payload('Cancelled by user')) return + # Cache publication is already fenced with Stop above. Keep LRU + # eviction and any memory-provider close outside the stream lock. + if _cache_new_agent: + _evicted_items = [] + # Snapshot the set of session_ids with a LIVE agent worker + # BEFORE taking SESSION_AGENT_CACHE_LOCK, so LRU eviction never + # closes an agent mid-run AND we never nest ACTIVE_RUNS_LOCK + # inside SESSION_AGENT_CACHE_LOCK (avoids any lock-ordering + # deadlock). A cancel/reconnect can drop STREAMS while the + # worker is still unwinding or blocked in a provider call, so + # ACTIVE_RUNS (worker lifecycle) is the authoritative liveness + # signal, not STREAMS. (#3536 review round 2) + _active_sids = set() + try: + from api.config import ACTIVE_RUNS, ACTIVE_RUNS_LOCK + with ACTIVE_RUNS_LOCK: + for _entry in (ACTIVE_RUNS or {}).values(): + _sid = (_entry or {}).get("session_id") + if _sid: + _active_sids.add(_sid) + except Exception: + _active_sids = set() + with SESSION_AGENT_CACHE_LOCK: + from api.config import SESSION_AGENT_CACHE_MAX + # Evict the oldest INACTIVE entries first. Walk LRU order + # (front = oldest); skip any session with a live run. If + # every over-cap entry is active, leave the cache + # temporarily above cap rather than close a live worker's + # agent — a later insertion/finalization trims it once the + # run ends. + while len(SESSION_AGENT_CACHE) > SESSION_AGENT_CACHE_MAX: + _evictable_sid = None + for _sid in list(SESSION_AGENT_CACHE.keys()): + if _sid not in _active_sids: + _evictable_sid = _sid + break + if _evictable_sid is None: + break # all over-cap entries are active; defer + evicted_entry = SESSION_AGENT_CACHE.pop(_evictable_sid) + _evicted_items.append((_evictable_sid, evicted_entry)) + # Commit and close evicted agents outside the cache lock so + # concurrent cache users are not blocked by provider I/O. + for _evicted_sid, _evicted_entry in _evicted_items: + try: + _evicted_agent = _evicted_entry[0] if isinstance(_evicted_entry, tuple) else None + _close_evicted_agent_at_session_boundary(_evicted_sid, _evicted_agent) + except Exception: + logger.debug("Failed to close evicted agent for session %s", _evicted_sid, exc_info=True) + logger.debug('[webui] Evicted LRU agent from cache: %s', _evicted_sid) + logger.debug('[webui] Created new agent for session %s', session_id) + # Prepend workspace context so the agent always knows which directory # to use for file operations, regardless of session age or AGENTS.md defaults. workspace_ctx = _workspace_context_prefix(str(s.workspace)) @@ -11467,6 +11521,11 @@ def _run_agent_streaming( ) _run_conversation_kwargs["user_message"] = user_message _result_partial_pre_call_context = list(_previous_context_messages) + if not _agent_can_invoke(agent): + with _agent_lock: + _finalize_cancelled_turn(s, ephemeral=ephemeral, message='Task cancelled before start.', stream_id=stream_id) + put('cancel', _cancel_event_payload('Cancelled by user')) + return result = agent.run_conversation(**_run_conversation_kwargs) _remember_pending_steer_result(result) _active_turn_identity = _resolve_active_turn_authority( @@ -11949,8 +12008,6 @@ def _run_agent_streaming( if 'credential_pool' in _agent_params: _agent_kwargs['credential_pool'] = _runtime_bundle['credential_pool'] agent = _AIAgent(**_agent_kwargs) - with STREAMS_LOCK: - AGENT_INSTANCES[stream_id] = agent _agent_sig = _compute_agent_cache_signature( resolved_model, resolved_api_key, @@ -11967,10 +12024,11 @@ def _run_agent_streaming( profile_home=_profile_home, safe_profile_runtime_env=_safe_profile_runtime_env, ) - from api.config import SESSION_AGENT_CACHE as _SAC, SESSION_AGENT_CACHE_LOCK as _SAC_L - with _SAC_L: - _SAC[session_id] = (agent, _agent_sig) - _SAC.move_to_end(session_id) + if not _register_agent_if_current(agent, _agent_sig): + # Returned-error settlement already owns _agent_lock. + _finalize_cancelled_turn(s, ephemeral=ephemeral, message='Task cancelled before start.', stream_id=stream_id) + put('cancel', _cancel_event_payload('Cancelled by user')) + return # Retry the conversation once with fresh credentials _self_healed = True _token_sent = False @@ -12003,6 +12061,11 @@ def _run_agent_streaming( _result_partial_pre_call_context = list( _heal_context_messages ) + if not _agent_can_invoke(agent): + # Returned-error settlement already owns the lock. + _finalize_cancelled_turn(s, ephemeral=ephemeral, message='Task cancelled before start.', stream_id=stream_id) + put('cancel', _cancel_event_payload('Cancelled by user')) + return _heal_result = agent.run_conversation(**_heal_kwargs) _remember_pending_steer_result(_heal_result) _active_turn_identity = _resolve_active_turn_authority( @@ -13301,8 +13364,6 @@ def _run_agent_streaming( if 'credential_pool' in _agent_params: _heal_kwargs['credential_pool'] = _runtime_bundle['credential_pool'] _heal_agent = _AIAgent(**_heal_kwargs) - with STREAMS_LOCK: - AGENT_INSTANCES[stream_id] = _heal_agent _agent_sig = _compute_agent_cache_signature( resolved_model, resolved_api_key, @@ -13319,10 +13380,11 @@ def _run_agent_streaming( profile_home=_profile_home, safe_profile_runtime_env=_safe_profile_runtime_env, ) - from api.config import SESSION_AGENT_CACHE as _SAC2, SESSION_AGENT_CACHE_LOCK as _SAC2_L - with _SAC2_L: - _SAC2[session_id] = (_heal_agent, _agent_sig) - _SAC2.move_to_end(session_id) + if not _register_agent_if_current(_heal_agent, _agent_sig): + with _agent_lock: + _finalize_cancelled_turn(s, ephemeral=ephemeral, message='Task cancelled before start.', stream_id=stream_id) + put('cancel', _cancel_event_payload('Cancelled by user')) + return # Retry the conversation _token_sent = False try: @@ -13354,6 +13416,11 @@ def _run_agent_streaming( _result_partial_pre_call_context = list( _heal_context_messages ) + if not _agent_can_invoke(_heal_agent): + with _agent_lock: + _finalize_cancelled_turn(s, ephemeral=ephemeral, message='Task cancelled before start.', stream_id=stream_id) + put('cancel', _cancel_event_payload('Cancelled by user')) + return _heal_result = _heal_agent.run_conversation(**_heal_kwargs2) _remember_pending_steer_result(_heal_result) _active_turn_identity = _resolve_active_turn_authority( diff --git a/docs/CONTRACTS.md b/docs/CONTRACTS.md index 2989b587..0b81b9bb 100644 --- a/docs/CONTRACTS.md +++ b/docs/CONTRACTS.md @@ -89,6 +89,15 @@ contributor guidance; it does not change runtime behavior or CI gates. `models_cache.json` refresh timestamps), the stat-identity fallbacks, and the schema/version stamps. Start here before changing model-catalog caching, the `/api/models` cache keys, or the Codex catalog dependency (#2443, #7540). +- [`docs/architecture/live-models-allowlist.md`](architecture/live-models-allowlist.md): + current contract for how `/api/models/live` filters a custom provider's + upstream catalog: the four signals in evaluation order (discovered catalog + defers to the live probe, explicit plural `models:` allowlist filters, + singular `model:` never gates, no allowlist shows the full catalog), the + serialized-list shapes `hermes config set` persists, the deliberate + empty-allowlist-is-not-configured rule, and the probe-failure fallback. + Start here before changing custom-provider model filtering or + discovery-vs-allowlist semantics (#7165, #7404). - [`docs/architecture/profile-home-resolve-cache.md`](architecture/profile-home-resolve-cache.md): current contract for the call-scoped memoization around `_resolve_profile_home_param()` in `api/workspace.py`: what it caches, the diff --git a/docs/architecture/live-models-allowlist.md b/docs/architecture/live-models-allowlist.md new file mode 100644 index 00000000..83c7363a --- /dev/null +++ b/docs/architecture/live-models-allowlist.md @@ -0,0 +1,123 @@ +# Live model picker allowlist contract (`/api/models/live`) + +This document records the runtime contract for how `/api/models/live` decides +which models a `custom` / `custom:*` provider contributes to the chat model +picker. It describes shipped behavior and changes no runtime behavior. It was +added because the #7165 review flagged the contract as undocumented: the +filtering rules live in `_handle_live_models` (`api/routes.py`) and had only +code comments backing them. + +Start here before changing how the live catalog of a custom provider is +filtered, how discovery snapshots are distinguished from allowlists, or what +the endpoint returns when the upstream probe fails. + +## Why the endpoint filters at all + +OpenAI-compatible gateways (New-API-style aggregators, LM Studio, Ollama +proxies) expose their **entire** catalog from `/v1/models`, including +image-generation, audio, TTS, and embedding models that are not chat models. +The background enrichment path must not surface models the user never opted +into — but it also must not hide models the gateway gained since config was +last edited. The contract below is how those two goals are reconciled. + +## The four signals, in evaluation order + +For a custom provider the handler consults these signals: + +1. **Discovered catalog → no allowlist (live probe wins).** + When the provider entry carries `models_discovered: true` and discovery is + allowed (`discover_models` is not an explicit opt-out), the `models:` + mapping is a *snapshot Hermes persisted from a past discovery*, not a + hand-curated list. It must not gate the live probe: a model the user pulls + into LM Studio / Ollama after discovery has to appear in the picker. + Detected by the shared predicate + `api.config._provider_models_are_discovered_catalog()` — the same + predicate the `/api/models` catalog path uses, so both endpoints agree. + With this signal, no allowlist is derived and the full live catalog is + returned. + +2. **Explicit plural `models:` allowlist → filter.** + A non-empty plural `models:` value on an entry that is *not* a discovered + catalog expresses "show exactly these models". The live catalog is + intersected with it, then any allowlisted model missing from the live + response is appended (so an allowlisted-but-offline model stays + selectable). This is the #7165 filter. Only **list** and **string** + (serialized) shapes are plain allowlists — see "Dict-shaped `models:`" + below. + +3. **Singular `model:` → never gates.** + The singular field is sticky/default metadata (the model to preselect), + not an allowlist. A provider configured only with `model: assistant` keeps + the full unfiltered live catalog; gating on it would collapse discovery to + a single model. + +4. **No allowlist → full live catalog.** + Preserves the pre-#7165 discovery behavior for setups that rely on it. + +### Serialized shapes + +`hermes config set` and the JSON-mode editor persist lists as quoted +JSON-array strings (`'["chat-a","chat-b"]'`) or Python literals +(`"['chat-a']"`). The plural value is decoded through the shared +`_parse_config_string_list()` before id extraction — but only when the value +is a `str`; native `dict` / `list` values are walked directly so dict *entries* +keep their `id|model|name` metadata (the decoder stringifies members). A +dict-only parser drops serialized allowlists to "no allowlist" and floods the +picker — the same bug class as `skills.disabled` (#7120 / #7134). + +### Dict-shaped `models:` + +A dict-shaped `models:` mapping (e.g. `{chat-a: {context_length: 128000}}`) +is *per-model metadata* written by the Hermes Agent setup flow — +`hermes_cli/model_switch.py::_save_custom_provider` and the setup wizard — +**not** a catalog narrow. Treating its keys as an allowlist would collapse the +live picker to the single saved default (keyless Ollama) while the CLI +live-probe shows the full catalog. So a dict is **never** a plain allowlist: + +- without an explicit `discover_models: false`, a dict-shaped `models:` + contributes **no allowlist** and the full live catalog is returned; +- with `discover_models: false` (bool `False` or the string forms + `"false"`/`"no"`/`"0"`, case-insensitive — mirroring + `model_switch_providers.py:557`), the dict **keys** are honored as a pinned + allowlist. + +This complements the discovered-catalog rule: `models_discovered: true` +without the opt-out is already ignored (signal 1). The dict guard covers the +Agent-setup shape that is *not* flagged discovered. + +If someone later "fixes" the dict shape back into an always-allowlist, the +regression test `test_dict_models_without_discover_false_does_not_gate` fails +and forces the trade-off to be re-argued. + +### Empty allowlist semantics + +`models: []`, `models: "[]"`, an empty mapping, or a blank scalar are +deliberately treated as **not configured**, never as "allow nothing". The +filter runs only on a non-empty allowlist because gating on declared-ness +would emit an empty picker and make the provider unselectable — a harder +failure than showing a few extra models, and unrecoverable from the UI +(`custom_providers` has no WebUI write path; it is hand-edited in +config.yaml). There is no deny-all use case: a provider the user wants hidden +is removed from `custom_providers` outright. + +### Probe failure / empty probe + +When the upstream probe fails or returns no models, the endpoint falls back +to `allowlist or config ids` — the non-empty plural allowlist first, so a +singular `model:` outside the allowlist cannot leak in precisely when the +probe is unavailable; with no allowlist, the configured ids (singular + +plural) are served. A discovered catalog therefore still serves its saved +models on a transient probe failure instead of emptying the picker. + +## Tests + +- `tests/test_issue3718_live_models_custom_probe.py` — real-handler + (`_handle_live_models`) coverage of every signal above: plural list / + JSON-array-string / Python-literal / list-of-dicts filtering, dict-shaped + `models:` (not gated) and dict + `discover_models: false` (pinned), + singular passthrough, malformed safety, discovered-catalog passthrough, + `discover_models: false` pinning, and both fallback branches. +- `tests/test_issue7404_models_discovered_not_allowlist.py` — the + `/api/models` side of the discovered-catalog contract. +- `tests/test_byok_model_dropdown.py` — picker rendering, profile-scoped + caching, and current-session model preservation around this endpoint. diff --git a/docs/pr-media/6469-sidebar-handoff/README.md b/docs/pr-media/6469-sidebar-handoff/README.md new file mode 100644 index 00000000..d458093d --- /dev/null +++ b/docs/pr-media/6469-sidebar-handoff/README.md @@ -0,0 +1,91 @@ +# Sidebar idle / terminal handoff evidence + +## Failure and scope + +The independent `/api/sessions` response can report idle before the chat SSE's +terminal frame reaches the page. Three sidebar paths were retiring chat state: +idle reconciliation, INFLIGHT purging, and optimistic-row retirement. The idle +path clears the Worklog and active stream ID; the subsequent `done` is rejected +as stale, so no settled Anchor scene is persisted. + +A real-HTTP barrier reproduces this on both published PR head +`75c8ff8d2a991f22389e3e4263ee03fb8d8f67a0` and frozen upstream +`b09fded7b8c98b549a49aaaece3721015337d307`. Applying only the `sessions.js` +product fix to the frozen upstream also passes the identical oracle: this is a +shared lifecycle defect, not evidence that artifact replay introduced it. The +historical hosted run lacks this instrumentation, so its exact interleaving +cannot be proved retrospectively; its empty-surface/no-persistence signature +matches the controlled reproduction. + +## Reproduction + +```sh +LIFECYCLE_SCENARIO=normal LIFECYCLE_TEST_BITE=settle-worklog-frame-proof \ + .venv/bin/python tests/browser_conversation_lifecycle.py +./scripts/test.sh tests/test_sidebar_terminal_handoff.py -q +``` + +The existing frame proof now uses a test-only server wrapper to hold the actual +`done` HTTP write. The real worker finishes; the real browser receives an idle +sidebar response through `renderSessionList()` while its native EventSource +remains open. The test verifies pane, INFLIGHT and registry ownership, then +releases `done` and retains all existing painted-frame, final-answer, semantic +activity and hard-reload assertions. The fixture always releases/cleans up and +has a bounded wait. Normal production server startup is unchanged. + +No renderer, terminal payload or persistence response is fabricated. Runtime +events come from the existing deterministic localhost Gateway fixture, not a +live model/provider. All state and files are disposable. These are browser +viewport checks, not physical-device acceptance. + +## Before / after + +Each screenshot is taken after the idle sidebar response, before the terminal +HTTP frame is released. The exact published source fails at both widths. The +patched source preserves three live activity rows and then passes completion +and hard reload. `results.json` records the predicate results without machine +paths or credentials. + +| Viewport | Before | After | +| --- | --- | --- | +| 1440 x 900 | [Empty surface](before-1440.png) | [Owned handoff retained](after-1440.png) | +| 390 x 844 | [Empty surface](before-390.png) | [Owned handoff retained](after-390.png) | + +The fix only defers these sidebar cleanups for the current pane's exact OPEN +chat transport. Missing, CONNECTING, CLOSED and mismatched sources still take +the existing stale recovery path. + +## Missing terminal frame: bounded recovery + +The OPEN-only guard at `85f1c2d34b62b0ade85f51ed2cf8c1cf56f5ab9e` could wait +forever when the terminal frame never arrived. The follow-up keeps the live +handoff for 1.5 seconds, then probes the exact stream through +`/api/chat/stream/status` with an eight-second timeout and no retries. +Repeated sidebar hints cannot restart the deadline or create concurrent +requests. `active:true` keeps the OPEN owner intact even when the persisted +session fields already look idle; only explicit runtime inactivity permits the +canonical session restore. A failed or malformed runtime probe uses the +existing interrupted-stream path rather than claiming successful settlement. +Normal terminal delivery, transport replacement/close, navigation, and a newer +optimistic turn invalidate the old request. Already-running stream-end recovery +remains the sole recovery owner. + +```sh +LIFECYCLE_SCENARIO=normal LIFECYCLE_TEST_BITE=sidebar-idle-missing-terminal \ + .venv/bin/python tests/browser_conversation_lifecycle.py +./scripts/test.sh tests/test_sidebar_idle_stream_recovery.py -q +``` + +The new browser row keeps the real terminal HTTP barrier closed until cleanup; +the client must recover without receiving `done` or `stream_end`. Published +`85f1c2d34b` fails the 11-second completion bound with `S.busy` still true. +The patched implementation restores the final answer and passes the unchanged +painted-frame, semantic activity, persistence and hard-reload checks before the +barrier is released. The cloud workflow runs both delayed- and missing-terminal +rows. No live provider or production state is used. + +A production Gateway regression also blocks post-turn goal evaluation after +the success transcript has already cleared `active_stream_id` and pending +fields. During that interval the worker and exact entry in `STREAMS` remain +live and no `done` event has been emitted, proving why persisted session flags +cannot authorize sidebar settlement on their own. diff --git a/docs/pr-media/6469-sidebar-handoff/after-1440.png b/docs/pr-media/6469-sidebar-handoff/after-1440.png new file mode 100644 index 00000000..e28d30be Binary files /dev/null and b/docs/pr-media/6469-sidebar-handoff/after-1440.png differ diff --git a/docs/pr-media/6469-sidebar-handoff/after-390.png b/docs/pr-media/6469-sidebar-handoff/after-390.png new file mode 100644 index 00000000..f876ed24 Binary files /dev/null and b/docs/pr-media/6469-sidebar-handoff/after-390.png differ diff --git a/docs/pr-media/6469-sidebar-handoff/before-1440.png b/docs/pr-media/6469-sidebar-handoff/before-1440.png new file mode 100644 index 00000000..87953ff1 Binary files /dev/null and b/docs/pr-media/6469-sidebar-handoff/before-1440.png differ diff --git a/docs/pr-media/6469-sidebar-handoff/before-390.png b/docs/pr-media/6469-sidebar-handoff/before-390.png new file mode 100644 index 00000000..20c745cb Binary files /dev/null and b/docs/pr-media/6469-sidebar-handoff/before-390.png differ diff --git a/docs/pr-media/6469-sidebar-handoff/results.json b/docs/pr-media/6469-sidebar-handoff/results.json new file mode 100644 index 00000000..c1564591 --- /dev/null +++ b/docs/pr-media/6469-sidebar-handoff/results.json @@ -0,0 +1,66 @@ +[ + { + "mode": "before", + "viewport": [ + 1440, + 900 + ], + "exit": 1, + "observation": { + "serverIdle": true, + "paneRetained": false, + "inflightRetained": false, + "sourceRetained": true, + "registryRetained": true, + "liveRows": 0 + } + }, + { + "mode": "before", + "viewport": [ + 390, + 844 + ], + "exit": 1, + "observation": { + "serverIdle": true, + "paneRetained": false, + "inflightRetained": false, + "sourceRetained": true, + "registryRetained": true, + "liveRows": 0 + } + }, + { + "mode": "after", + "viewport": [ + 1440, + 900 + ], + "exit": 0, + "observation": { + "serverIdle": true, + "paneRetained": true, + "inflightRetained": true, + "sourceRetained": true, + "registryRetained": true, + "liveRows": 3 + } + }, + { + "mode": "after", + "viewport": [ + 390, + 844 + ], + "exit": 0, + "observation": { + "serverIdle": true, + "paneRetained": true, + "inflightRetained": true, + "sourceRetained": true, + "registryRetained": true, + "liveRows": 3 + } + } +] diff --git a/docs/rfcs/live-to-final-assistant-replies.md b/docs/rfcs/live-to-final-assistant-replies.md index 6564390e..60e762c6 100644 --- a/docs/rfcs/live-to-final-assistant-replies.md +++ b/docs/rfcs/live-to-final-assistant-replies.md @@ -470,6 +470,19 @@ Expected behavior: - A session row's running indicator should reflect a real active run or a clearly restorable state, not stale persisted metadata alone. +- An idle sidebar response does not prove that the browser has consumed the + terminal chat frame. While the current pane still owns its exact OPEN chat + transport, list reconciliation and cache purging must preserve that handoff. + Missing, connecting, closed, or mismatched transports keep the normal stale + state recovery path; a cached stream ID or busy flag is insufficient. + OPEN is not proof that a terminal frame will arrive: an idle hint must also + schedule bounded recovery without erasing the live scene. Before reading a + settled session snapshot, that recovery must check the exact stream's runtime + status. A runtime-active stream retains ownership even when persisted + `active_stream_id` and pending fields are already clear; only explicit + runtime inactivity permits canonical snapshot settlement. Probe failure is a + degraded/interrupted state, not evidence of successful completion. A newer + turn, replacement source, or received terminal event invalidates the request. - Background completion, cancellation, or failure should be represented without stealing the visible pane from the user. - Session switching should not erase pending live context, in-flight snapshots, diff --git a/docs/rfcs/stable-assistant-turn-anchors.md b/docs/rfcs/stable-assistant-turn-anchors.md index 9c522519..39be9aa4 100644 --- a/docs/rfcs/stable-assistant-turn-anchors.md +++ b/docs/rfcs/stable-assistant-turn-anchors.md @@ -574,6 +574,38 @@ early renderers can avoid chasing `S.messages` during every paint. If settlement later rewrites the transcript message, the anchor must be refreshed from that message instead of allowing the two copies to drift silently. +### Settled ownership of a restored live turn + +A preserved or snapshotted live-turn node is not evidence of a running turn. The +settled assistant message is persisted a few milliseconds before the stream's +terminal event clears the active stream id, so a live node whose stream has +already ended can survive a transcript rebuild or a session switch and be +appended under the settled answer, putting the same answer on screen twice +(#6948 follow-up; upstream symptom report #2051). + +A live-turn node is therefore discarded only where the settled transcript is +proved to own it: the transcript ends with a settled assistant message, no +message still carries a live-projection marker, that settled message's +persisted stream identity equals the id of the stream that built the live node +— or, only where no scene identity was persisted, the markdown the stream +produced equals the persisted message source — and the node carries nothing the +settled rebuild could not have produced, meaning no unpersisted tool card, +reasoning row or transparent-stream row and no second live segment. + +Ownership is a state proof, never a comparison of two rendered DOM trees: the +live body comes from the streaming parser and the settled body from the +markdown renderer, and an answer that reads the same as the previous one must +not delete a genuinely live turn. An unrecognised node shape keeps the turn. +Only the two branches that ADD a turn are gated; a branch that replaces a node +cannot duplicate one. + +A settled activity scene projects one answer as well. A `process_prose` row +whose text duplicates the scene's final answer — the same text, or a prefix of +it within ten percent of its length — is dropped where both settled renderers +take their rows, so the answer is not rebuilt as a second assistant segment +above itself. Live rendering is unchanged: while the turn streams, the inline +live segment is hidden and the prose row is the visible answer. + ## Replay, Reload, And Reconstruction Replay/reload should reconstruct the same Assistant Turn Anchor from durable diff --git a/docs/rfcs/webui-run-state-consistency-contract.md b/docs/rfcs/webui-run-state-consistency-contract.md index d1362548..7df7f1a0 100644 --- a/docs/rfcs/webui-run-state-consistency-contract.md +++ b/docs/rfcs/webui-run-state-consistency-contract.md @@ -30,6 +30,50 @@ This RFC defines a consistency contract for those layers. It complements the larger run adapter direction in #1925 by documenting what must remain coherent while WebUI still has multiple overlapping state stores. +## Agent registration after cancellation + +Initial and credential self-heal Agent construction use the same registration +boundary. Under `STREAMS_LOCK`, both the worker-retained cancellation event and +live stream membership must permit registration. A removed `CANCEL_FLAGS` entry +is not permission to restart. If Stop won during initial or self-heal +construction, the candidate must not enter the reusable cache or call +`run_conversation`. Stream registration, reusable-cache publication and the +in-memory lifecycle handle share one atomic Stop admission, using lock order +`STREAMS_LOCK` then `SESSION_AGENT_CACHE_LOCK`. Merely moving a cache write +after a cancellation check does not protect that check-to-publication gap. + +After prompt preparation and immediately before each initial/self-heal invocation, +revalidate the retained cancel event, stream membership and exact registered +Agent. If Stop already won, retire a matching reusable entry only while the +existing `SESSION_WRITEBACK_OWNERS` record still equals this exact stream. +Hold that ownership lock through cache/lifecycle retirement. Object identity is +insufficient because successors can reuse the same Agent; absent ownership is +not permission either, since a completed successor clears its record. Never +clear a successor's cache or lifecycle handle, and do not issue another Agent +interrupt for an invocation that never started. Rejected cache-hit registration +likewise must not interrupt the borrowed Agent; only a never-published newly +constructed candidate may receive construction-cancellation cleanup. Drop the +old worker's local borrowed handle as well, so final pending-Steer drain cannot +reach a successor through an object-reference fallback. +Stop after invocation admission uses the existing Agent interrupt mechanism; +registry locks must not span provider or tool execution. LRU eviction/close stays +outside the stream lock and retains the existing active-worker policy. + +Interrupt and cancellation finalization occur outside the stream registry lock. +Session finalization still owns the session lock: the returned-error path already +holds it, while initial registration and the exception path acquire it. Do not +reacquire this non-reentrant lock from a branch that already owns it. + +## Run-journal sequence publication + +Within one WebUI process, auto-numbered appends to the same journal allocate +sequence numbers and write their rows under the same per-path lock. +`RunJournalWriter` delegates both operations to `append_run_event`; it must not +reserve a sequence and release the lock before the physical append. Otherwise +individually valid rows can reach disk out of order and the session replay +reader must reject them as noncontiguous. This does not change caller-supplied +sequence semantics, cross-process ownership, or failed-write recovery. + ## Inactive compression continuation recovery The Agent profile's SQLite compression lineage owns the canonical continuation, diff --git a/static/boot.js b/static/boot.js index d6476b60..06c3888d 100644 --- a/static/boot.js +++ b/static/boot.js @@ -3431,8 +3431,17 @@ window._mirrorSpeechSettingsFromServer=_mirrorSpeechSettingsFromServer; localStorage.setItem('hermes-font-size',fontSize); _applyFontSize(fontSize); if(typeof setLocale==='function'){ + // #7622 (round 3): the settings payload's `s.language` is + // absent (None) for a fresh install, so an explicit non-empty + // value is the user's genuine saved choice. The browser + // navigator hint is now read via the guarded + // `_detectBrowserLanguageHint()` helper (round-3 finding 2) so + // a throwing `navigator` accessor in some embedded webviews + // can no longer abort this branch and reset loaded preferences. + // The fallback ternary preserves the pre-#7622 boot behaviour + // when neither helper is in scope (defence in depth). const _lang=typeof resolvePreferredLocale==='function' - ? resolvePreferredLocale(s.language, localStorage.getItem('hermes-lang')) + ? resolvePreferredLocale(s.language, localStorage.getItem('hermes-lang'), _detectBrowserLanguageHint()) : (s.language || localStorage.getItem('hermes-lang') || 'en'); setLocale(_lang); if(typeof applyLocaleToDOM==='function')applyLocaleToDOM(); diff --git a/static/i18n.js b/static/i18n.js index 0192fd5c..269ffe43 100644 --- a/static/i18n.js +++ b/static/i18n.js @@ -26983,15 +26983,68 @@ function resolveLocale(lang) { /** * Resolve locale with precedence: - * 1) primary (typically server setting) - * 2) fallback (typically localStorage) - * 3) English - * @param {string} primary - * @param {string} fallback + * 1) primary (typically server setting) + * 2) fallback (typically localStorage) + * 3) fallback2 (typically browser navigator hint — first-visit only) + * 4) 'en' (final fallback) + * + * Resolve a language preference to a known LOCALES key, applying a strict + * `primary -> fallback -> fallback2 -> 'en'` precedence chain. + * + * #7622 (round 3): the server no longer injects `"language": "en"` as a + * schema default — `load_settings()` reports `None` (the key is absent + * from the response) until the user explicitly picks a locale in the + * Settings modal. This lets the resolver use `primary` directly + * without a `primary === 'en'` special case: + * - an explicit `primary` (the server's stored value) always wins + * - a `None` / empty `primary` falls through to `fallback` (localStorage) + * - `fallback2` (the browser navigator hint) only fires when neither + * server nor localStorage has a preference + * - `'en'` is the final safety net + * + * Side benefit: a user who picked English on purpose (their settings + * file contains `"language": "en"`) is no longer overridden by a + * non-English browser locale on hydration, because the round-2 + * `primary === 'en'` skip has been removed. + * + * @param {string|null|undefined} primary server-stored language + * @param {string|null|undefined} fallback localStorage `hermes-lang` + * @param {string|null|undefined} fallback2 browser navigator hint * @returns {string} */ -function resolvePreferredLocale(primary, fallback) { - return resolveLocale(primary) || resolveLocale(fallback) || 'en'; +function resolvePreferredLocale(primary, fallback, fallback2) { + return resolveLocale(primary) + || resolveLocale(fallback) + || resolveLocale(fallback2) + || 'en'; +} + +/** + * Detect the browser's preferred language for first-visit locale + * selection. Guarded against `navigator` access exceptions: some + * embedded webviews (Tauri desktop shell, kiosk modes) report a + * `navigator` object but throw on `.languages` / `.language` reads. + * Callers (loadLocale, boot.js, panels.js) must use this helper rather + * than reading `navigator` directly so a single throw does not abort + * the boot or settings-panel hydration. + * + * @returns {string|null} the first entry of `navigator.languages`, or + * `navigator.language`, or `null` when neither is reachable. + */ +function _detectBrowserLanguageHint() { + try { + const nav = (typeof navigator !== 'undefined') ? navigator : null; + if (!nav) return null; + if (Array.isArray(nav.languages) && nav.languages.length) { + return nav.languages[0] || null; + } + if (typeof nav.language === 'string' && nav.language) { + return nav.language; + } + return null; + } catch (_) { + return null; + } } /** @@ -27029,11 +27082,26 @@ function setLocale(lang) { /** * Load locale from localStorage (called once at boot, before DOMContentLoaded). * Server-persisted preference is applied later in loadSettingsPanel(). + * + * #7622: on the FIRST visit (no stored preference), fall back to the + * browser's preferred language (`navigator.languages[0]` / + * `navigator.language`) so non-English speakers don't have to dig into + * Settings after every fresh install. This is strictly a first-visit + * hint — the moment the user picks a language in Settings (or the + * server-side preference is applied via `loadSettingsPanel`), the + * stored value takes over and the browser hint is never consulted + * again for that browser profile. */ function loadLocale() { let stored = null; try { stored = localStorage.getItem('hermes-lang'); } catch (_) {} - setLocale(resolvePreferredLocale(null, stored)); + // Only consult the browser hint when no stored preference exists. + // The settings panel and `loadSettingsPanel()` will overwrite this + // hint on their first call, so explicit user / server preferences + // always win. #7622 (round 3): the browser-hint read is delegated + // to `_detectBrowserLanguageHint()` so a `navigator` access throw + // (some embedded webviews) can no longer abort loadLocale(). + setLocale(resolvePreferredLocale(stored, _detectBrowserLanguageHint())); } /** diff --git a/static/messages.js b/static/messages.js index dac891d6..81e021ec 100644 --- a/static/messages.js +++ b/static/messages.js @@ -2089,6 +2089,7 @@ function closeLiveStream(sessionId, streamId, source){ if(!live) return; if(streamId&&live.streamId!==streamId) return; if(source&&live.source!==source) return; + if(typeof live.cancelIdleRecovery==='function') live.cancelIdleRecovery(); // Snapshot the current live-turn DOM BEFORE tearing the stream down. The // per-event snapshot (snapshotLiveTurn) only fires on content/tool_complete // SSE events, so switching away during a quiet window (mid tool-exec, silent @@ -4845,7 +4846,12 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ function _smdMediaTailFlushEntry(entry){ const chunk=_smdMediaTailEntryChunk(entry); if(!chunk) return; - const m=/^MEDIA:([^\s\)\]]+)$/.exec(String(chunk)); + // #7680 re-gate (9/22): strip backtick wrappers so the bare-token + // match below sees a plain ``MEDIA:path`` and the bare class + // (no backtick in the exclusion set) captures the full filename + // even when the path itself contains a backtick. + const normalized = String(chunk).replace(/`MEDIA:([^`\s]+)`/g, 'MEDIA:$1'); + const m=/^MEDIA:([^\s\)\]]+)$/.exec(normalized); const emitted=!!(m && entry && entry.parent && _smdAppendMediaNode(entry.parent, m[1])); if(!emitted && entry) _smdMediaWriteText(entry.parent, entry.data, entry.baseAddText, entry.writeText, chunk); } @@ -4893,23 +4899,29 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ // Prose runs go through the owning text writer. MEDIA tokens go through // the single-token DOMParser helper only after a delimiter or // reliable filename suffix proves the ref is complete. + // #7680 re-gate (9/22): strip backtick wrappers first so the bare + // class (no backtick in the exclusion set) captures the full + // filename even when the path itself contains a backtick. + // The pre-pass replaces `` `MEDIA:path` `` with ``MEDIA:path`` + // so the wrapped form is consumed before the bare scan. + const normalized = combined.replace(/`MEDIA:([^`\s]+)`/g, 'MEDIA:$1'); const re=/MEDIA:([^\s\)\]]+)/g; let last=0, m; let unmatchedTail=null; - while((m=re.exec(combined))){ + while((m=re.exec(normalized))){ const matchEnd = m.index + m[0].length; if(m.index>last){ - const slice = combined.slice(last, m.index); + const slice = normalized.slice(last, m.index); writeCurrent(slice); } - if(matchEnd===combined.length && !_smdMediaRefHasReliableBoundary(m[1])){ - const candidate = combined.slice(m.index); + if(matchEnd===normalized.length && !_smdMediaRefHasReliableBoundary(m[1])){ + const candidate = normalized.slice(m.index); if(candidate.length < _MEDIA_TAIL_MAX){ unmatchedTail = candidate; } else { writeCurrent(candidate); } - last = combined.length; + last = normalized.length; break; } if(!_smdAppendMediaNode(parent, m[1])) writeCurrent(m[0]); @@ -4917,7 +4929,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ } // Tail buffer — hold trailing bytes that look like an unterminated // MEDIA prefix; flush any prose before the partial MEDIA suffix. - const rest = combined.slice(last); + const rest = normalized.slice(last); if(rest){ const tailMatch = /MEDIA:[^\s\)\]]*$/.exec(rest); const prefixTail = tailMatch ? '' : _smdMediaPrefixTail(rest); @@ -5745,12 +5757,82 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ return true; } + function _bindSidebarIdleRecovery(live){ + let pending=null; + live.cancelIdleRecovery=()=>{ + if(pending&&pending.timer) clearTimeout(pending.timer); + pending=null; + }; + live.recoverFromSidebarIdle=()=>{ + if(pending||_streamFinalized||_terminalStateReached||_pendingStreamEndRecovery) return; + const request={inflight:INFLIGHT[activeSid],timer:null}; + const isCurrent=()=>pending===request&&!_streamFinalized&&!_terminalStateReached&& + LIVE_STREAMS[activeSid]===live&&live.source.readyState===1&& + S.session&&S.session.session_id===activeSid&&S.activeStreamId===streamId&& + INFLIGHT[activeSid]===request.inflight&& + !(typeof _sendInProgress!=='undefined'&&_sendInProgress&&activeSid===_sendInProgressSid); + pending=request; + // Give the independently delivered terminal frame a short handoff window, + // then use canonical session recovery even if the transport stays OPEN. + // One ticket spans both timer and request; list refreshes cannot extend it. + request.timer=setTimeout(async()=>{ + request.timer=null; + if(!isCurrent()){ + if(pending===request) live.cancelIdleRecovery(); + return; + } + try{ + let runtimeStatus=null; + try{ + runtimeStatus=await api( + `/api/chat/stream/status?stream_id=${encodeURIComponent(streamId)}`, + {timeoutMs:8000,retries:0,timeoutToast:false} + ); + }catch(_){ + // Runtime ownership is authoritative here. If the probe itself + // fails, do not infer completion from already-cleared session + // fields; surface the existing interrupted-stream path instead. + if(isCurrent()) _handleStreamError(live.source); + return; + } + if(!isCurrent()) return; + if(!runtimeStatus||typeof runtimeStatus.active!=='boolean'){ + _handleStreamError(live.source); + return; + } + // Gateway success writeback clears persisted active/pending fields + // before post-turn goal evaluation and terminal event emission. + // STREAMS-backed status therefore owns this decision: an exact active + // runtime must keep its OPEN browser handoff even if the sidebar row + // already looks idle. + if(runtimeStatus.active) return; + const status=await _restoreSettledSession(live.source,{ + status:true, + isCurrent, + requestOptions:{timeoutMs:8000,retries:0,timeoutToast:false}, + preserveVisibleOnShorterTerminalSnapshot:true, + }); + // A stale sidebar response is not permission to terminate a worker + // which the authoritative session snapshot still reports as active. + if(isCurrent()&&status!=='restored'&&status!=='active') _handleStreamError(live.source); + }finally{ + if(pending===request) live.cancelIdleRecovery(); + } + },1500); + }; + for(const event of ['done','cancel','apperror','stream_end','error']){ + live.source.addEventListener(event,live.cancelIdleRecovery); + } + } + function _wireSSE(source){ const existingLive=LIVE_STREAMS[activeSid]; if(existingLive&&existingLive.source&&existingLive.source!==source){ + if(typeof existingLive.cancelIdleRecovery==='function') existingLive.cancelIdleRecovery(); try{if(existingLive.source.readyState!==2)existingLive.source.close();}catch(_){ } } LIVE_STREAMS[activeSid]={streamId,source}; + _bindSidebarIdleRecovery(LIVE_STREAMS[activeSid]); // Note on #631 Bug B: the original PR description stated the server // "replays buffered token events" on reconnect, and proposed resetting @@ -7024,13 +7106,16 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ async function _restoreSettledSession(source, options=null){ const returnStatus=!!(options&&options.status); + const isCurrent=options&&typeof options.isCurrent==='function'?options.isCurrent:null; + if(isCurrent&&!isCurrent()) return returnStatus?'stale':false; const preserveVisibleOnShorterTerminalSnapshot=!!(options&&options.preserveVisibleOnShorterTerminalSnapshot); if(_isActiveSession() && S.activeStreamId!==streamId){ _closeSource(source); return returnStatus?'stale':false; } try{ - const data=await api(`/api/session?session_id=${encodeURIComponent(activeSid)}`); + const data=await api(`/api/session?session_id=${encodeURIComponent(activeSid)}`,options&&options.requestOptions||{}); + if(isCurrent&&!isCurrent()) return returnStatus?'stale':false; // Opus #2852 race-fix: if a late `done` event ran the finalize path while // we were awaiting the network roundtrip, bail out — done already settled. if(_streamFinalized) return returnStatus?'restored':true; diff --git a/static/panels.js b/static/panels.js index dd5090c3..9215cc77 100644 --- a/static/panels.js +++ b/static/panels.js @@ -9300,8 +9300,17 @@ async function loadSettingsPanel(){ _setHiddenTabs(hiddenTabs); _applyTabVisibility(hiddenTabs); _renderTabVisibilityChips(); + // #7622 (round 3): the settings payload's `settings.language` is + // absent (None) for a fresh install, so an explicit non-empty + // value is the user's genuine saved choice. The browser + // navigator hint is now read via the guarded + // `_detectBrowserLanguageHint()` helper (round-3 finding 2) so + // a throwing `navigator` accessor can no longer abort settings + // hydration before model, provider, plugin and extension sections + // are populated. The fallback ternary preserves the pre-#7622 + // settings-modal behaviour when neither helper is in scope. const resolvedLanguage=(typeof resolvePreferredLocale==='function') - ? resolvePreferredLocale(settings.language, localStorage.getItem('hermes-lang')) + ? resolvePreferredLocale(settings.language, localStorage.getItem('hermes-lang'), _detectBrowserLanguageHint()) : (settings.language || localStorage.getItem('hermes-lang') || 'en'); // Keep settings modal and current page strings in sync with the resolved locale. if(typeof setLocale==='function'){ diff --git a/static/sessions.js b/static/sessions.js index c6a302a4..f748f45a 100644 --- a/static/sessions.js +++ b/static/sessions.js @@ -852,6 +852,14 @@ function _reconcileActiveSessionIdleStateFromList(serverRows) { const serverRow=serverRows.find(s=>s&&s.session_id===sid); if (!serverRow) return false; if (!_isServerIdleSessionRow(serverRow)) return false; + // Sidebar idle metadata can beat the terminal frame on the independent chat + // SSE. Let its exact OPEN transport finish the Anchor handoff; orphaned or + // disconnected streams still use the existing idle recovery below. + if (_hasOwnedOpenLiveStream(sid)) { + const live=LIVE_STREAMS[sid]; + if(typeof live.recoverFromSidebarIdle==='function') live.recoverFromSidebarIdle(); + return false; + } let changed=false; if (S.busy) { S.busy=false; changed=true; } if (S.activeStreamId) { S.activeStreamId=null; changed=true; } @@ -940,6 +948,8 @@ function _purgeStaleInflightEntries() { if (typeof _sendInProgress !== 'undefined' && _sendInProgress && sid === _sendInProgressSid) { continue; } + // The sidebar render must not purge what the idle reconciler preserved. + if (_hasOwnedOpenLiveStream(sid)) continue; if (!sessionsById.has(sid)) { const knownSource = sourceById ? sourceById.get(sid) : null; if (currentSidebarSource && (!knownSource || knownSource !== currentSidebarSource)) { @@ -961,6 +971,14 @@ function _purgeStaleInflightEntries() { } } +function _hasOwnedOpenLiveStream(sid) { + if (typeof S === 'undefined' || !S || !S.session || S.session.session_id !== sid || !S.activeStreamId) return false; + const live = typeof LIVE_STREAMS === 'object' && LIVE_STREAMS ? LIVE_STREAMS[sid] : null; + // readyState 1 is EventSource.OPEN. A cached stream ID or busy flag alone + // is not ownership and must never disable recovery for a stuck indicator. + return Boolean(live && live.streamId === S.activeStreamId && live.source && live.source.readyState === 1); +} + function _rememberSessionListSource(s, sid = null, allowScopeFallback = true) { const resolvedSid = sid || (s && s.session_id); if (!resolvedSid) return; @@ -2169,6 +2187,12 @@ async function loadSession(sid){ S.activeStreamId=activeStreamId; const liveToolReplayId=(tc)=>String(tc&&(tc.tid||tc.id||tc.tool_call_id||tc.tool_use_id||tc.call_id||'')||'').trim(); const replayPersistedLiveToolCards=(opts)=>{ + // The journal-backed Anchor scene is authoritative through its resume + // cursor; newer rows arrive through the reattached SSE stream. Replaying + // the older INFLIGHT tool cache after a successful scene restore would + // redraw all N rows N times. Keep the #3707 replay only for legacy HTML + // restoration or a failed/unavailable scene render. + if(restoredAnchorScene) return; const liveToolCalls=Array.isArray(S.toolCalls) ? S.toolCalls : (Array.isArray(INFLIGHT[sid]&&INFLIGHT[sid].toolCalls)?INFLIGHT[sid].toolCalls:[]); @@ -5272,7 +5296,8 @@ function _shouldKeepLocalOnlyOptimisticSessionRow(local){ function _dropStaleOptimisticSessionRow(sid){ if(!sid) return; if(typeof _rememberSessionListSource==='function') _rememberSessionListSource(null, sid, false); - if(INFLIGHT&&INFLIGHT[sid]){ + // Retiring sidebar optimism must not retire the independent chat owner. + if(INFLIGHT&&INFLIGHT[sid]&&!_hasOwnedOpenLiveStream(sid)){ delete INFLIGHT[sid]; if(typeof clearInflightState==='function') clearInflightState(sid); } diff --git a/static/ui.js b/static/ui.js index 675bd66d..201eb7fd 100644 --- a/static/ui.js +++ b/static/ui.js @@ -3949,6 +3949,27 @@ function _isEquivalentConfiguredModelEntry(modelId,badge,entries){ // providers (@custom:name:model) without collapsing matching model IDs from // different providers. const rawId=String(modelId||''); + // `/` is another routing spelling of ``, so its badge + // key must not become a second picker row. configured_model_badges holds every + // spelling of a configured model and renderModelDropdown() synthesises a row + // for each key this predicate does not recognise. The provider-qualified + // spelling was missed because _normalizeConfiguredModelKey() strips only one + // leading slash segment (#3360 keeps `vendor_a/x` and `vendor_b/y/x` distinct), + // so `acme/example-model` and `custom/acme/example-model` normalise to + // different keys and the picker lists one model twice. + // Match it the way the `@provider:` rule below does: the badge declares a + // provider, the key starts with that provider's `/` prefix, and an + // existing row from the same provider normalises equal to the remainder. Two + // different models never satisfy the last clause, so this can only drop a + // duplicate of a row the catalog already produced. + const slashPrefix=provider?`${provider}/`:''; + if(slashPrefix&&rawId.toLowerCase().startsWith(slashPrefix)){ + const slashRoutedId=rawId.slice(slashPrefix.length); + if(slashRoutedId&&(entries||[]).some(entry=> + String(entry.providerId||'').toLowerCase()===provider + &&_normalizeConfiguredModelKey(entry.value)===_normalizeConfiguredModelKey(slashRoutedId) + )) return true; + } const prefix=provider?`@${provider}:`:''; if(!prefix||!rawId.toLowerCase().startsWith(prefix)) return false; const routedId=rawId.slice(prefix.length); @@ -7702,6 +7723,16 @@ function renderMd(raw){ // generated images) and replace them with inline or download links. // Stashed so the path/URL is never processed as markdown. const media_stash=[]; + // #7680 re-gate (9/22): two-pass scan. + // 1. `` `MEDIA:path` `` (backtick-wrapped, inline-code form) → strip + // the wrapping backticks so the bare-token pass below sees a + // plain ``MEDIA:path`` and the closing backtick is not consumed + // as part of the path. + // 2. ``MEDIA:[^\s\)\]]+`` (bare, no backtick in the exclusion + // class) so a filename that legally contains a backtick + // (``report`final.png``) is captured in full instead of being + // truncated at the first backtick. + s=s.replace(/`MEDIA:([^`\s]+)`/g,'MEDIA:$1'); s=s.replace(/MEDIA:([^\s\)\]]+)/g,(_,raw_ref)=>{ media_stash.push(raw_ref); return '\x00D'+(media_stash.length-1)+'\x00'; @@ -9149,6 +9180,12 @@ function _stripForTTS(text){ // Strip links, keep text text=text.replace(/\[([^\]]+)\]\([^)]+\)/g,'$1'); // Replace MEDIA: paths with a simple label + // #7680 re-gate (9/22): TTS does not need the backtick-aware + // terminator. Inline code was already stripped at the top of this + // function (`:9137`), so a bare ``MEDIA:[^\s]+`` is correct and + // round-trips the original filename including a backtick in the + // path — the only thing the TTS ever does with the captured + // substring is throw it away. text=text.replace(/MEDIA:[^\s]+/g,'a file'); // Strip emoji and emoticons text=text.replace(/[\u{1F600}-\u{1F64F}\u{1F300}-\u{1F5FF}\u{1F680}-\u{1F6FF}\u{1F1E0}-\u{1F1FF}\u{2600}-\u{26FF}\u{2700}-\u{27BF}\u{FE00}-\u{FE0F}\u{1F900}-\u{1F9FF}\u{1FA00}-\u{1FA6F}\u{1FA70}-\u{1FAFF}\u{200D}]/gu,''); @@ -9928,6 +9965,99 @@ function _liveAssistantSegmentTextLength(seg){ return String(body.textContent||'').trim().length; } +// ── #6948 follow-up: settled-transcript ownership of a live-turn node ────── +// (duplicate assistant answer; upstream symptom report #2051) +// +// Both live-turn insertion paths — the renderMessages() re-attach and +// restoreLiveTurnHtmlForSession() — have one branch that ADDS a turn the +// settled rebuild did not produce. The assistant row is persisted a few ms +// BEFORE the stream's terminal event clears S.activeStreamId, so a dead live +// node left behind by an INFLIGHT entry that outlived its stream is re-attached +// on top of the settled transcript: the same answer twice, self-sustaining +// across later renders (the node keeps id=liveAssistantTurn and is re-preserved +// by the #6948 guard), healed only by a reload. +// +// Dropping a live node must be an OWNERSHIP proof, never "the rendered text +// looks the same": +// * the live body is built by the streaming smd parser and the settled body +// by renderMd + post-processing, so their textContent differs for anything +// past single-paragraph plain text (lists, fenced code with its language +// label, tables, `_underscore_` emphasis, the injected copy button) — a +// text comparison would miss most real answers; and +// * an identical answer earlier in the history (a repeated "Done.", a +// greeting, a same-prompt resend) would delete a GENUINELY LIVE turn: while +// the current turn is unpersisted, the last settled assistant message IS the +// previous turn's answer. +// So ownership is proved from state: the transcript must END with a settled +// assistant message that THIS stream produced, and the node must carry nothing +// the settled rebuild could not have produced. + +// Stream identity persisted on a settled assistant message. The server stamps +// _anchor_stream_id from the anchor-scene sidecar record (api/routes.py); the +// client stamps _anchor_stream_id and scene.identity.stream_id at stream end +// (_attachProjectedAnchorSceneToLastAssistant, static/messages.js). +function _settledAssistantStreamId(message){ + if(!message) return ''; + const scene=message._anchor_activity_scene||null; + const identity=(scene&&scene.identity)||null; + return String(message._anchor_stream_id||(scene&&scene.stream_id)||(identity&&identity.stream_id)||''); +} +// The live-projection markers the #6948 preserve guard already treats as proof +// of a live owner. A message the client still considers live is never evidence +// that the turn has settled. +function _messageHasLiveAssistantProjection(m){ + return !!(m&&m.role==='assistant'&&(m._live||m._activityBurstId!==undefined||m._liveSegmentSeq!==undefined)); +} +// True when the live turn holds something the settled rebuild cannot have +// produced from S.messages — an unpersisted tool card, reasoning/thinking row, +// transparent-stream row, or a second live segment. Such a node is never +// dropped: #3714's whole premise is that the live DOM can be AHEAD of +// S.messages, and a tail-segment match must not discard the tool card or the +// earlier segment above it. An unknown node shape fails closed (keep the turn). +function _liveTurnCarriesUnsettledContent(turn){ + if(!turn||typeof turn.querySelectorAll!=='function') return true; + if(turn.querySelectorAll( + '.tool-card-row,.wl-reason,.agent-activity-thinking,.thinking-card-row,.transparent-event-row' + ).length) return true; + return turn.querySelectorAll('[data-live-assistant="1"]').length>1; +} +function _settledTranscriptOwnsLiveTurn(sid, turn){ + if(!turn) return false; + const msgs=(typeof S!=='undefined'&&S&&Array.isArray(S.messages))?S.messages:null; + if(!msgs||!msgs.length) return false; + // The transcript must END with a settled assistant answer. A trailing user + // turn means the live turn IS the current turn — nothing of it is persisted + // yet, so it can never be a leftover (this is the case a text comparison gets + // wrong when the previous answer happens to read the same), and a live + // projection anywhere means the rebuild still owns a live turn of its own. + const last=msgs[msgs.length-1]; + if(!last||last.role!=='assistant') return false; + if(msgs.some(_messageHasLiveAssistantProjection)) return false; + const inflight=(typeof INFLIGHT!=='undefined'&&INFLIGHT)?INFLIGHT[sid]:null; + // Owner of the live DOM: INFLIGHT carries the id of the stream that built it + // (attachLiveStream), and S.activeStreamId is that same id until the terminal + // event clears it. + const liveStreamId=String((inflight&&inflight.streamId)||(typeof S!=='undefined'&&S&&S.activeStreamId)||''); + const settledStreamId=_settledAssistantStreamId(last); + if(settledStreamId){ + // Identity recorded on both sides decides — renderer- and text-independent, + // so code blocks, lists and tables are handled like plain prose. + if(!liveStreamId||settledStreamId!==liveStreamId) return false; + }else{ + // No persisted identity on the tail (a turn whose scene was not worklog + // worthy). Compare SOURCE with SOURCE — the markdown this stream produced + // (INFLIGHT.lastAssistantText) against the persisted message content — + // never two independently rendered DOM trees. Regeneration cannot reach + // this branch: startRegeneration() truncates S.messages at the user turn, + // so the tail is a user message while a regenerated answer streams. + const streamed=String((inflight&&inflight.lastAssistantText)||'').replace(/\s+/g,' ').trim(); + if(!streamed) return false; + const settled=String((typeof msgContent==='function'?msgContent(last):last.content)||'').replace(/\s+/g,' ').trim(); + if(!settled||streamed!==settled) return false; + } + return !_liveTurnCarriesUnsettledContent(turn); +} + function _mergeRestoredLiveAssistantSegment(restored, existing){ if(!restored||!existing) return; const existingLive=existing.querySelector('[data-live-assistant="1"]'); @@ -9964,6 +10094,18 @@ function restoreLiveTurnHtmlForSession(sid){ const existing=$('liveAssistantTurn'); _mergeRestoredLiveAssistantSegment(restored, existing); if(existing) existing.replaceWith(restored); + // #6948 follow-up (duplicate assistant answer; #2051): only this branch ADDS a + // turn — replacing an existing live turn cannot duplicate. When the settled + // transcript already owns the stream this snapshot belongs to, appending it + // pins a SECOND copy of the same answer under the settled one. Release the + // stale snapshot (no other consumer can use it — it would be re-refused on + // every later restore) and report "nothing restored", so loadSession takes the + // same path it already takes for an INFLIGHT entry that never carried one. + else if(typeof _settledTranscriptOwnsLiveTurn==='function' + &&_settledTranscriptOwnsLiveTurn(sid, restored)){ + inflight.liveTurnHtml=null; + return false; + } else inner.appendChild(restored); // Transparent Stream: liveTurnHtml is restored via template.innerHTML, which // drops the property-bound onclick/onkeydown handlers wired by @@ -11469,6 +11611,15 @@ function _assistantMessageBelongsInWorklog(m, rawIdx, toolCallAssistantIdxs, vis const isTurnFinalAssistant=!!(opts&&opts.isTurnFinalAssistant); const visibleText=String(visibleContent!==undefined?visibleContent:msgContent(m)||'').trim(); const hasVisibleText=!!visibleText&&!_isAssistantEmptyPlaceholderContent(m, visibleText); + // The caller only consults this predicate once the turn has settled (it gates + // on `!S.busy`), so an `_live` marker seen here is a leftover of the + // live-snapshot projection, not an ongoing stream. Folding on it hid the turn's + // final answer inline and echoed it into the Worklog, leaving the turn with no + // visible content until the #3875 blank-turn fail-safe revealed both copies. A + // turn-final assistant message with visible text is the answer, so keep it out + // of the fold; every other `_live` message folds as before. Kept as a separate + // guard so the live rule below keeps its position. + if(m._live&&hasVisibleText&&isTurnFinalAssistant) return false; if(m._live) return true; if(hasVisibleText&&m._anchor_activity_scene) return false; if(hasVisibleText&&isTurnFinalAssistant) return false; @@ -13151,6 +13302,17 @@ function _anchorSceneRowsForRendering(scene, opts){ const rows=Array.isArray(scene&&scene.activity_rows)?scene.activity_rows:[]; const settled=!!(opts&&opts.settled); const live=!settled; + // #6948 follow-up (duplicate assistant answer; #2051): on a SETTLED turn the + // assistant segment owns the final answer, so a process_prose row carrying + // that same text must not also be rebuilt into the activity scene. The + // transparent-stream renderer suppresses it per row + // (_anchorSceneTransparentNodeForRow), but the compact-worklog row builder + // (_anchorSceneNodeForRow) has no such guard and renders the row as a second + // .assistant-segment above the settled one. Drop it here — the single place + // BOTH settled renderers (and the #5839 deferred-rows path) get their rows. + // LIVE rendering is untouched: while streaming, the inline live segment is + // hidden and the prose row IS the visible answer. + const settledFinalAnswer=settled?String((scene&&scene.final_answer)||'').trim():''; const out=[]; const byKey=new Map(); const liveProseTextKeys=new Map(); @@ -13173,6 +13335,9 @@ function _anchorSceneRowsForRendering(scene, opts){ if(_anchorSceneIsSettledSuccessfulCompression(row,settled)) continue; const text=String(row.text||'').trim(); if((row.role==='prose'||row.role==='thinking')&&!text) continue; + if(settledFinalAnswer&&row.role==='prose' + &&typeof _anchorSceneProseDuplicatesFinalAnswer==='function' + &&_anchorSceneProseDuplicatesFinalAnswer(text,settledFinalAnswer)) continue; // #6948 follow-up const key=keyFor(row); if(byKey.has(key)){ const index=byKey.get(key); @@ -13447,6 +13612,26 @@ function _anchorSceneProseMatchesFinalAnswer(proseText, finalAnswer){ const shorter=Math.min(a.length,b.length), longer=Math.max(a.length,b.length); return shorter>=80 && (shorter/longer)>=0.9; } +// #6948 follow-up (duplicate assistant answer; #2051): the same near-equality +// test as _anchorSceneProseMatchesFinalAnswer, minus its absolute `shorter>=80` +// floor. That floor makes the matcher a no-op for every SHORT final answer +// (greetings, "Done.", one-liners), so a prose row that is the answer minus its +// last streamed token — e.g. "Hey! What can I help you with today" vs +// "...today?" — was rebuilt as a SECOND assistant-segment beside the settled +// one. The >=0.9 length ratio is what actually keeps a distinct short +// intermediate sentence from being swallowed by a long final answer (Codex +// #4568), so it alone is kept. Deliberately a separate helper: the per-row +// render-time matcher above, and the tests pinning its exact predicate, stay +// untouched. +function _anchorSceneProseDuplicatesFinalAnswer(proseText, finalAnswer){ + if(_anchorSceneProseMatchesFinalAnswer(proseText,finalAnswer)) return true; + const norm=(s)=>String(s||'').replace(/\s+/g,' ').trim(); + const a=norm(proseText), b=norm(finalAnswer); + if(!a||!b) return false; + if(!(a.startsWith(b)||b.startsWith(a))) return false; + const shorter=Math.min(a.length,b.length), longer=Math.max(a.length,b.length); + return (shorter/longer)>=0.9; +} function _anchorSceneWorklogGroup(blocks, opts){ if(!blocks) return null; const live=!!(opts&&opts.live); @@ -16922,6 +17107,23 @@ function _processWakeupCardHtml(info, rawText, extras){ return `
${li('chevron-right',12)}${li('terminal',13)}${esc(t('process_wakeup_label'))}${cmdHtml}${chip}${extras.timeHtml||''}
${extras.filesHtml||''}${patternRow}${cmdRow}
${outHtml}
${extras.footHtml||''}
`; } +// #2051: parse into a