diff --git a/CHANGELOG.md b/CHANGELOG.md index 97944c7e..0743df6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ - **No more false "No response from provider" after a completed answer that used tools.** When an assistant turn ended with a real final answer delivered as structured content following tool calls (common with reasoning models and multi-tool turns), the completion detector could miss the final text and render a spurious "No response from provider" card under the finished reply. Final-answer detection now understands structured content parts (text after tool-use boundaries), while genuine empty completions still correctly show the no-response notice, and a real terminal provider error still surfaces as an error (never masked as a final answer). Thanks @jtstothard. (#5990, #5686) +- **Custom/proxy providers keep the full vendor namespace when you deliberately pick a non-default model (follow-up to the earlier #5979 fix).** The initial fix preserved `x-ai/grok-4.5` when it was your configured default or already in a warm model catalog, but a model you *selected in the picker* that wasn't your default (e.g. `x-ai/grok-composer-2.5-fast`) was still truncated to the bare `grok-composer-2.5-fast` when the models catalog was cold at send time — the normal state right after a restart, a settings save, or a cache expiry. The proxy then rejected the bare id with `HTTP 400`. For a `custom` / `custom:` proxy the resolver now remembers that you deliberately selected the model this session and preserves the full `vendor/model` id on a cold catalog, while a stale cross-provider leftover you didn't pick still has its redundant prefix stripped so it keeps routing. The send path also warms model-catalog provenance from the durable on-disk cache first (network-free, non-blocking, never triggers a live probe). Thanks @b3nw for the detailed follow-up and the CLI-vs-WebUI comparison that localized it. (#5979) + - **Cron unread ("Tasks") dots are now scoped to the active profile — no more ghost dots after switching.** In multi-profile setups, the session-sidebar unread dot and the Tasks badge could keep showing a cron completion from a *different* profile after you switched, and the count could resurrect from a stale poll. Cron completion markers now carry their source + owning profile, are cleared for the profile you switch away from (including legacy/untagged markers), and a previous-profile poll response can no longer restore a stale unread. Non-cron completion unread and the current profile's own cron unread are preserved; default ↔ renamed-root aliases are treated as the same profile so a root cron marker is neither wrongly cleared nor wrongly kept. Thanks @jbbottoms. (#5975, #5960) - **The Docker "manual update" banner instruction is now localized.** On Docker/no-git deployments the update banner shows the manual update command; that guidance string was hardcoded in English. It now goes through the i18n system, so it renders in the active language (English fallback where a translation isn't present yet). Thanks @jbbottoms. (#5973, #5959) diff --git a/api/config.py b/api/config.py index 8e6ac076..7bba9db8 100644 --- a/api/config.py +++ b/api/config.py @@ -2560,7 +2560,7 @@ def _get_provider_cfg(provider_id) -> dict: return provider_cfg if isinstance(provider_cfg, dict) else {} -def resolve_model_provider(model_id: str) -> tuple: +def resolve_model_provider(model_id: str, *, explicitly_picked: bool = False) -> tuple: """Resolve model name, provider, and base_url for AIAgent. Model IDs from the dropdown can be in several formats: @@ -2580,6 +2580,16 @@ def resolve_model_provider(model_id: str) -> tuple: provider. Returns (model, provider, base_url) where provider and base_url may be None. + + ``explicitly_picked``: True when the caller knows the user DELIBERATELY + selected ``model_id`` this session (persisted from an ``explicit_model_pick`` + UI action), as opposed to it being a stale session leftover. Used ONLY for + the custom-proxy COLD-catalog decision (#5979): with no provenance available, + a deliberately-picked ``vendor/model`` is preserved verbatim (the user chose + it, the proxy routes on it), while an UNMARKED id (a stale cross-provider + leftover, e.g. #433's ``openai/gpt-5.4`` on a bare-only relay) still gets the + legacy redundant-prefix strip so it keeps routing when cold. Warm provenance + (endpoint-advertised ids) always takes precedence over this flag. """ config_provider = None config_base_url = None @@ -2898,13 +2908,30 @@ def resolve_model_provider(model_id: str) -> tuple: return model_id, config_provider, config_base_url # (3) Provenance genuinely unavailable (cold/unbuilt or # fingerprint-mismatched catalog AND not config-declared). - # Fall back to the LEGACY family heuristic so this narrow edge - # is never WORSE than the pre-fix behaviour: a first-party - # redundant prefix still strips (#433 relay works cold), while - # an intrinsic/unknown prefix (bedrock/opus-4-6 #3872, - # zai-org/GLM-5.1 #548) is preserved. The #5979 active-user - # path can't reach here — a selected id is either config- - # declared or in the catalog the dropdown was built from. + # Distinguish a DELIBERATE selection from a stale leftover: + # + # * explicitly_picked → PRESERVE verbatim. The user chose this + # exact ``vendor/model`` in the UI this session; the proxy + # routes on it. A wrong strip destroys a namespace the proxy + # needs (recurs every turn, unrepairable short of declaring + # every model in config) — this is b3nw's #5979 case: a + # non-default pick on a custom: proxy, cold catalog. + # * NOT explicitly_picked → legacy redundant-prefix strip. An + # unmarked id here is a stale cross-provider leftover (the + # user switched providers and the old session model lingers, + # e.g. #433's ``openai/gpt-5.4`` on a relay that only serves + # bare ``gpt-5.4``); stripping keeps it routing while cold. + # + # Warm provenance (case 2, endpoint-advertised ids) always + # wins over this flag; the send path also warms provenance + # network-free from the disk cache first + # (warm_models_catalog_provenance_if_cold), so this branch is + # reached only in the narrow no-disk-cache window. The flag + # removes the data-driven flaw where a model graduating into + # the static first-party catalog silently flipped routing + # (exactly how #5979 regressed). + if explicitly_picked: + return model_id, config_provider, config_base_url if prefix in _PROVIDER_MODELS and _is_first_party_model(prefix, bare): return bare, config_provider, config_base_url return model_id, config_provider, config_base_url @@ -8162,6 +8189,82 @@ def _models_cache_file_age_seconds(cache_path: Path, now: float) -> float | None return None +def warm_models_catalog_provenance_if_cold() -> None: + """Best-effort, NON-BLOCKING, disk-only publish of catalog provenance. + + The send path (``api/streaming.py``) resolves the wire model via + ``resolve_model_provider`` without ever building the models catalog, and the + #1855 chat/start fast path deliberately skips the catalog when a session + already carries a persisted model+provider — so "cold at send" is the + designed behaviour after any process restart, memory-TTL expiry, or cache + invalidation, not a rare race. In that state the custom-proxy provenance + signal (``_endpoint_advertised_model_ids``) is ``None`` and resolution falls + to the cold-preserve default; this helper restores the endpoint-advertised + signal from the durable disk cache so the #433 bare-only-strip stays exact. + + Deliberately does NOT call ``get_available_models(prefer_cache=True)``: even + in prefer-cache mode that acquires ``_available_models_cache_lock`` and can + block up to ~60s waiting on an in-flight rebuild (unbounded in synchronous + rebuild mode) — unacceptable on the send hot path. Instead this: + * tries the cache lock NON-BLOCKING and returns immediately if it's busy + (a concurrent rebuild will publish provenance itself); + * reads ONLY the on-disk cache (no network, no live probe, no rebuild); + * publishes the snapshot + source fingerprint via the same globals the + real publish sites use, then ``_sync_models_cache_provenance()``. + Publishing the fingerprint from the CURRENT runtime is correct: the disk + cache is validated by schema/version/source-fingerprint on load + (``_is_loadable_disk_cache``), so a load success means it belongs to this + profile. Callers must not hold ``_cfg_lock`` (this reads config for the + fingerprint); the send worker satisfies that. + + Profile isolation: the fast no-op is taken ONLY when the resident provenance + fingerprint matches the CURRENT profile's runtime fingerprint. The catalog + globals are process-wide, so a concurrently-active profile B could have left + its own (or a stale) provenance resident; an unconditional non-``None`` + early return would let B's catalog block profile A from loading A's own valid + disk cache (A would then resolve against B's advertised ids). Comparing the + published fingerprint to the current one before short-circuiting closes that + hole — a mismatch falls through to load THIS profile's disk snapshot. + """ + global _available_models_cache, _available_models_cache_ts + global _available_models_cache_source_fingerprint + + def _provenance_is_current() -> bool: + prov = _models_cache_provenance + if prov is None: + return False + try: + return prov[1] == _models_cache_source_fingerprint() + except Exception: + return False + + if _provenance_is_current(): + return # already warm for THIS profile — one global read, no work + got = _available_models_cache_lock.acquire(blocking=False) + if not got: + return # a concurrent build/publish holds the lock; it will publish + try: + if _provenance_is_current(): + return # published for this profile while we waited for the lock + try: + disk_groups = _load_models_cache_from_disk() + except Exception: + disk_groups = None + if disk_groups is None: + return # no durable cache for this profile → stay cold, preserve verbatim + _available_models_cache = disk_groups + _available_models_cache_ts = time.monotonic() + try: + _available_models_cache_source_fingerprint = _models_cache_source_fingerprint() + except Exception: + _available_models_cache_source_fingerprint = None + _sync_models_cache_provenance() + except Exception: + logger.debug("models catalog provenance warm failed", exc_info=True) + finally: + _available_models_cache_lock.release() + + def get_available_models_for_session_visit() -> dict: """Return /api/models with a short session-visit freshness horizon. diff --git a/api/models.py b/api/models.py index cd78fa13..8ecbbc0c 100644 --- a/api/models.py +++ b/api/models.py @@ -1081,6 +1081,21 @@ def _parse_nonnegative_int(value): return parsed if parsed >= 0 else None +def model_explicit_pick_signature(model, model_provider) -> str: + """Stable signature of a (model, provider) selection for #5979 explicit-pick + provenance. The persisted ``Session.model_explicit_pick_signature`` is set to + this when the user deliberately picks a model; the streaming resolver only + treats a selection as deliberate when the CURRENT routing context produces + the same signature. Any model/provider change (chat-start, session-update, + normalization, provider repair) yields a different signature and thus + invalidates the stale pick — so a #433 first-party leftover is never wrongly + preserved. Uses \\x1f (unit separator) so it can't collide with model ids. + """ + _m = str(model or "").strip() + _p = str(model_provider or "").strip().lower() + return f"{_m}\x1f{_p}" + + class Session: def __init__(self, session_id: str=None, title: str='Untitled', workspace=str(DEFAULT_WORKSPACE), model=DEFAULT_MODEL, @@ -1136,6 +1151,16 @@ class Session: self.workspace = str(Path(workspace).expanduser().resolve()) self.model = model self.model_provider = str(model_provider).strip().lower() if model_provider else None + # #5979: signature of the model the user DELIBERATELY picked this session + # (``"\x1f"``), or None. Used by the streaming resolver + # to preserve a custom-proxy vendor namespace on a COLD catalog ONLY when + # the current routing context still matches what was picked. Storing a + # SIGNATURE (not a bare bool) means any later model/provider change — via + # /api/chat/start, /api/session/update, normalization, or provider repair + # — automatically invalidates the pick (the signatures no longer match), + # so a stale first-party leftover (#433) is never wrongly preserved. + # Restored from persisted metadata on load (arrives via **kwargs). + self.model_explicit_pick_signature = kwargs.get('model_explicit_pick_signature') or None self.messages = messages or [] self.tool_calls = tool_calls or [] self.created_at = created_at or time.time() @@ -1255,7 +1280,7 @@ class Session: # without parsing the full messages array (which may be 400KB+). # Fields are listed in the order they should appear in the JSON file. METADATA_FIELDS = [ - 'session_id', 'title', 'workspace', 'model', 'model_provider', 'created_at', 'updated_at', + 'session_id', 'title', 'workspace', 'model', 'model_provider', 'model_explicit_pick_signature', 'created_at', 'updated_at', 'pinned', 'archived', 'project_id', 'profile', 'input_tokens', 'output_tokens', 'estimated_cost', 'cache_read_tokens', 'cache_write_tokens', diff --git a/api/routes.py b/api/routes.py index 999b8caa..9d4685c1 100644 --- a/api/routes.py +++ b/api/routes.py @@ -21680,6 +21680,20 @@ def _handle_chat_start(handler, body, diag=None): profile_config=_pp_cfg, explicit_model_pick=explicit_model_pick, ) + # #5979: record a SIGNATURE of the deliberately-picked model+provider so + # the streaming resolver can preserve a custom-proxy vendor namespace on a + # cold catalog — but ONLY while the routing context still matches. On a + # fresh explicit pick, stamp the signature of the resolved model+provider; + # otherwise leave any prior signature in place (it self-invalidates when + # the model/provider changes, since the streaming side recomputes and + # compares). This survives same-model follow-up sends (the onchange marker + # is one-shot) yet can't outlive a real switch. + try: + if explicit_model_pick: + from api.models import model_explicit_pick_signature as _mk_sig + s.model_explicit_pick_signature = _mk_sig(model, model_provider) + except Exception: + pass catalog_profile_provider = _pp_provider if catalog_profile_provider is None and isinstance(_pp_cfg, dict): profile_model_config = _pp_cfg.get("model") or {} diff --git a/api/streaming.py b/api/streaming.py index 4bc663e1..cdc0e5d8 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -37,6 +37,7 @@ from api.config import ( resolve_model_provider, resolve_custom_provider_connection, model_with_provider_context, + warm_models_catalog_provenance_if_cold, load_settings, parse_reasoning_effort, coerce_reasoning_effort_for_model, @@ -7890,9 +7891,46 @@ def _run_agent_streaming( # Initialize SessionDB so session_search works in WebUI sessions _state_db_path = (Path(_profile_home) / "state.db") if _profile_home else None _session_db = _build_session_db_for_stream(_state_db_path) - resolved_model, resolved_provider, resolved_base_url = resolve_model_provider( - model_with_provider_context(model, provider_context) - ) + # #5979: publish catalog provenance from the durable disk cache when + # memory is cold, so the custom-proxy resolver below sees the + # endpoint-advertised model ids (non-blocking, disk-only, never + # live-rebuilds). Both the warm and the resolve read profile-keyed + # config (cache path + source fingerprint via get_active_profile_name), + # but this streaming worker is a separate thread that does NOT inherit + # the HTTP handler's request-profile TLS — without binding it, a cold + # send from a NAMED profile would resolve against the DEFAULT profile's + # config and route to the wrong provider/base_url. Bind the captured + # owning-session profile across warm + resolve so both see the right + # profile (no-op for the default/root profile). + from api import profiles as profiles_api + # #5979: treat this send as a deliberate pick ONLY when the persisted + # explicit-pick signature matches the CURRENT model+provider routing + # context. Storing/comparing a signature (not a bare bool) means a + # later model/provider change via /api/chat/start, /api/session/update, + # normalization, or provider repair automatically invalidates a stale + # pick — so a #433 first-party leftover is never wrongly preserved on + # a cold catalog. Only affects the cold custom-proxy branch; warm + # endpoint-advertised provenance always wins over this flag. + from api.models import model_explicit_pick_signature as _mk_sig + _picked_sig = getattr(s, "model_explicit_pick_signature", None) + # Compare against the session's persisted model+provider — the exact + # fields /api/chat/start stamped the signature from (it persists the + # resolved model+provider onto the session before dispatch). Falls + # back to the worker's model/provider_context if the session fields + # are unset. A mismatch (any later model/provider change) yields a + # different signature → treated as NOT a deliberate pick. + _sig_model = getattr(s, "model", None) or model + _sig_provider = getattr(s, "model_provider", None) or provider_context + _current_sig = _mk_sig(_sig_model, _sig_provider) + _explicitly_picked = bool(_picked_sig) and _picked_sig == _current_sig + with profiles_api.profile_scope_for_detached_worker( + _resolved_profile_name, "model resolution", logger_override=logger + ): + warm_models_catalog_provenance_if_cold() + resolved_model, resolved_provider, resolved_base_url = resolve_model_provider( + model_with_provider_context(model, provider_context), + explicitly_picked=_explicitly_picked, + ) configured_base_url = resolved_base_url # Resolve API key via Hermes runtime provider (matches gateway behaviour). diff --git a/tests/test_issue1897_profile_switch_agent_cache.py b/tests/test_issue1897_profile_switch_agent_cache.py index 97190f66..7c6782f7 100644 --- a/tests/test_issue1897_profile_switch_agent_cache.py +++ b/tests/test_issue1897_profile_switch_agent_cache.py @@ -156,7 +156,7 @@ def test_same_session_profile_switch_rebuilds_agent_under_new_soul_home(tmp_path monkeypatch.setattr( streaming, "resolve_model_provider", - lambda _model: ("test-model", "test-provider", None), + lambda _model, **_kw: ("test-model", "test-provider", None), ) monkeypatch.setattr(streaming, "_maybe_schedule_title_refresh", lambda *args, **kwargs: None) monkeypatch.setattr(profiles, "get_hermes_home_for_profile", home_for_profile) diff --git a/tests/test_issue2965_streaming_sessiondb_profile_home.py b/tests/test_issue2965_streaming_sessiondb_profile_home.py index 7988792c..520eca3e 100644 --- a/tests/test_issue2965_streaming_sessiondb_profile_home.py +++ b/tests/test_issue2965_streaming_sessiondb_profile_home.py @@ -129,7 +129,7 @@ def test_streaming_sessiondb_uses_session_profile_state_db(tmp_path, monkeypatch monkeypatch.setattr( streaming, "resolve_model_provider", - lambda _model: ("test-model", "test-provider", None), + lambda _model, **_kw: ("test-model", "test-provider", None), ) monkeypatch.setattr(streaming, "_maybe_schedule_title_refresh", lambda *args, **kwargs: None) monkeypatch.setattr(profiles, "get_hermes_home_for_profile", lambda _profile: profile_home) diff --git a/tests/test_model_resolver.py b/tests/test_model_resolver.py index 5c67d1e8..165e4d3c 100644 --- a/tests/test_model_resolver.py +++ b/tests/test_model_resolver.py @@ -7,7 +7,8 @@ import pytest import api.config as config -def _resolve_with_config(model_id, provider=None, base_url=None, default=None, custom_providers=None): +def _resolve_with_config(model_id, provider=None, base_url=None, default=None, custom_providers=None, + explicitly_picked=False): """Helper: temporarily set config.cfg model/custom provider sections, call resolve, restore.""" old_cfg = dict(config.cfg) model_cfg = {} @@ -21,14 +22,14 @@ def _resolve_with_config(model_id, provider=None, base_url=None, default=None, c if custom_providers is not None: config.cfg['custom_providers'] = custom_providers try: - return config.resolve_model_provider(model_id) + return config.resolve_model_provider(model_id, explicitly_picked=explicitly_picked) finally: config.cfg.clear() config.cfg.update(old_cfg) def _resolve_with_catalog(model_id, advertised_ids, *, provider=None, base_url=None, - provider_id='custom', default=None): + provider_id='custom', default=None, explicitly_picked=False): """Resolve with a seeded models-catalog snapshot (#5979 provenance). ``advertised_ids`` is the list of model ids the endpoint's own group @@ -57,7 +58,8 @@ def _resolve_with_catalog(model_id, advertised_ids, *, provider=None, base_url=N config._advertised_model_ids_memo = None # force recompute against the seeded snapshot config._sync_models_cache_provenance() # publish the atomic (snapshot, fingerprint) pair try: - return _resolve_with_config(model_id, provider=provider, base_url=base_url, default=default) + return _resolve_with_config(model_id, provider=provider, base_url=base_url, default=default, + explicitly_picked=explicitly_picked) finally: config._available_models_cache = old_cache config._advertised_model_ids_memo = old_memo @@ -300,28 +302,36 @@ def test_named_custom_slug_preserves_advertised_full_id_5979(): assert model == 'x-ai/grok-4.5', f"full id must be preserved for custom:slug, got {model!r}" -def test_custom_remote_cold_catalog_falls_back_to_legacy_heuristic_5979(): - """#5979 tri-state: with a COLD/unbuilt catalog AND no config declaration - (no provenance at all), resolution falls back to the LEGACY family heuristic - so this narrow edge is never worse than the pre-fix behaviour. +def test_custom_remote_cold_catalog_explicit_pick_preserves_5979(): + """#5979 (reopened): with a COLD catalog and no config declaration, an + EXPLICITLY-PICKED custom-proxy id is preserved verbatim (the user chose it; + the proxy routes on it), while an UNMARKED id gets the legacy strip. - A first-party redundant prefix still strips (the #433 relay keeps working - cold), while an intrinsic/unknown prefix is preserved. The #5979 active-user - path never reaches this branch — a selected id is either config-declared - (see the config-declared test) or in the catalog the dropdown was built from. + This is the persisted-explicit-pick resolution (Codex's mechanism, Nathan's + call): the cold decision is no longer an unconditional preserve — it is + gated on whether the user deliberately selected the model this session. """ - # first-party redundant prefix → strip (matches pre-fix / #433 cold path) - model_a, _, _ = _resolve_with_catalog( + # explicitly picked → PRESERVE even though grok graduated into first-party x-ai + picked, _, _ = _resolve_with_catalog( 'openai/gpt-5.4', advertised_ids=None, provider='custom', base_url='https://router.example.com/v1', + explicitly_picked=True, ) - assert model_a == 'gpt-5.4', f"cold first-party prefix must strip (legacy), got {model_a!r}" - # intrinsic/unknown prefix → preserve - model_b, _, _ = _resolve_with_catalog( + assert picked == 'openai/gpt-5.4', f"explicit pick must preserve cold, got {picked!r}" + # NOT picked (stale leftover) → legacy strip keeps #433 relay routing cold + stale, _, _ = _resolve_with_catalog( + 'openai/gpt-5.4', advertised_ids=None, + provider='custom', base_url='https://router.example.com/v1', + explicitly_picked=False, + ) + assert stale == 'gpt-5.4', f"unmarked stale id must strip cold (legacy #433), got {stale!r}" + # intrinsic/unknown prefix → preserve regardless (no first-party family match) + unknown, _, _ = _resolve_with_catalog( 'zai-org/GLM-5.1', advertised_ids=None, provider='custom', base_url='https://api.deepinfra.com/v1/openai', + explicitly_picked=False, ) - assert model_b == 'zai-org/GLM-5.1', f"cold unknown prefix must preserve, got {model_b!r}" + assert unknown == 'zai-org/GLM-5.1', f"cold unknown prefix must preserve, got {unknown!r}" def test_custom_remote_config_declared_full_id_preserved_cold_5979(): @@ -405,36 +415,40 @@ def test_custom_remote_foreign_profile_catalog_ignored_5979(): """Profile-isolation fail-safe: when the catalog snapshot's source fingerprint does NOT match the current runtime (a concurrently-active foreign profile published it), that snapshot is NOT trusted for provenance — - resolution falls back to the legacy family heuristic instead of stripping - against another profile's catalog. + the id is preserved verbatim instead of stripped against another profile's + catalog. - Proof id: ``zai-org/GLM-5.1``. The foreign snapshot advertises a bare - ``GLM-5.1`` (which, if trusted, would strip the prefix), but ``zai-org`` is - NOT a first-party provider, so the legacy fallback preserves the full id. - A result of ``zai-org/GLM-5.1`` therefore proves the foreign catalog was - ignored (a trusted-catalog strip would have returned ``GLM-5.1``). + Proof id: ``openai/gpt-5.4`` with the FOREIGN catalog advertising ONLY the + bare ``gpt-5.4``, resolved as an EXPLICIT pick. If the foreign catalog were + (wrongly) trusted, the bare-only-advertised rule (warm provenance, which + wins over the pick flag) would strip → ``gpt-5.4``. Because the fingerprint + mismatches, the snapshot is ignored and resolution takes the cold branch; + with the explicit-pick flag set that preserves → ``openai/gpt-5.4``. The two + outcomes genuinely diverge, so ``openai/gpt-5.4`` proves the foreign catalog + was ignored. """ old_cache = config._available_models_cache old_memo = config._advertised_model_ids_memo old_fp = config._available_models_cache_source_fingerprint old_prov = config._models_cache_provenance config._available_models_cache = { - 'groups': [{'provider_id': 'custom', 'models': [{'id': 'GLM-5.1', 'label': 'GLM-5.1'}]}] + 'groups': [{'provider_id': 'custom', 'models': [{'id': 'gpt-5.4', 'label': 'gpt-5.4'}]}] } config._available_models_cache_source_fingerprint = {'config_yaml': {'path': '/some/other/profile'}} config._advertised_model_ids_memo = None config._sync_models_cache_provenance() try: model, _, _ = _resolve_with_config( - 'zai-org/GLM-5.1', provider='custom', base_url='https://relay.example/v1', + 'openai/gpt-5.4', provider='custom', base_url='https://relay.example/v1', + explicitly_picked=True, ) finally: config._available_models_cache = old_cache config._advertised_model_ids_memo = old_memo config._available_models_cache_source_fingerprint = old_fp config._models_cache_provenance = old_prov - assert model == 'zai-org/GLM-5.1', ( - f"foreign-profile catalog must be ignored (legacy fallback preserves), got {model!r}" + assert model == 'openai/gpt-5.4', ( + f"foreign-profile catalog must be ignored (cold explicit-pick preserve), got {model!r}" ) @@ -484,6 +498,242 @@ def test_resolver_provenance_read_does_not_block_on_cache_lock_5979(): config.invalidate_models_cache() +def test_b3nw_cold_nondeclared_custom_slug_preserves_full_id_5979(): + """#5979 (reopened, b3nw's exact case): a NON-declared model actively + selected on a ``custom:`` proxy is preserved on a COLD catalog. + + b3nw's config: ``provider: custom:llm-proxy``, ``model.default: x-ai/grok-4.5``, + NO ``models[]`` allowlist. He picks ``x-ai/grok-composer-2.5-fast`` in the UI + (not his default). On a cold send the old code stripped it to + ``grok-composer-2.5-fast`` via the legacy family heuristic (the model had + graduated into the x-ai first-party catalog), and his proxy 400'd. The flip + to cold-preserve fixes it. grok-composer-2.5-fast is stubbed into + ``_PROVIDER_MODELS['x-ai']`` (as test_custom_remote_preserves_advertised_full_id_5979 + does) so the first-party trigger is reproduced deterministically regardless + of the agent catalog version. + """ + mid = 'x-ai/grok-composer-2.5-fast' + bare = 'grok-composer-2.5-fast' + xai = list(config._PROVIDER_MODELS.get('x-ai') or []) + had = any(isinstance(m, dict) and m.get('id') == bare for m in xai) + old_xai = config._PROVIDER_MODELS.get('x-ai') + old_cfg = dict(config.cfg) + old_prov = config._models_cache_provenance + old_cache = config._available_models_cache + if not had: + config._PROVIDER_MODELS['x-ai'] = xai + [{'id': bare, 'label': 'Grok Composer 2.5 Fast'}] + config.cfg.clear() + config.cfg.update({ + 'model': {'default': 'x-ai/grok-4.5', 'provider': 'custom:llm-proxy', + 'base_url': 'https://proxy.example/v1'}, + 'custom_providers': [{'name': 'llm-proxy', 'base_url': 'https://proxy.example/v1', + 'key_env': 'LLM_PROXY_API_KEY'}], + }) + config._available_models_cache = None # COLD + config._advertised_model_ids_memo = None + config._sync_models_cache_provenance() + try: + assert config._is_first_party_model('x-ai', bare), "precondition: stub failed" + model, provider, _ = config.resolve_model_provider(mid, explicitly_picked=True) + finally: + if not had: + if old_xai is None: + config._PROVIDER_MODELS.pop('x-ai', None) + else: + config._PROVIDER_MODELS['x-ai'] = old_xai + config.cfg.clear() + config.cfg.update(old_cfg) + config._models_cache_provenance = old_prov + config._available_models_cache = old_cache + assert model == mid, f"b3nw's non-declared cold custom-proxy pick must preserve, got {model!r}" + assert provider == 'custom:llm-proxy' + + +def test_custom_slug_cold_stale_not_picked_still_strips_5979(): + """Companion to b3nw's case: the SAME cold custom: shape, but the id is + NOT explicitly picked (a stale first-party leftover), still gets the legacy + strip — so #433-style stale sessions keep routing when cold and only + deliberate picks are preserved verbatim. + """ + mid = 'x-ai/grok-composer-2.5-fast' + bare = 'grok-composer-2.5-fast' + xai = list(config._PROVIDER_MODELS.get('x-ai') or []) + had = any(isinstance(m, dict) and m.get('id') == bare for m in xai) + old_xai = config._PROVIDER_MODELS.get('x-ai') + old_cfg = dict(config.cfg) + old_prov = config._models_cache_provenance + old_cache = config._available_models_cache + if not had: + config._PROVIDER_MODELS['x-ai'] = xai + [{'id': bare, 'label': 'Grok Composer 2.5 Fast'}] + config.cfg.clear() + config.cfg.update({ + 'model': {'default': 'x-ai/grok-4.5', 'provider': 'custom:llm-proxy', + 'base_url': 'https://proxy.example/v1'}, + 'custom_providers': [{'name': 'llm-proxy', 'base_url': 'https://proxy.example/v1', + 'key_env': 'LLM_PROXY_API_KEY'}], + }) + config._available_models_cache = None # COLD + config._advertised_model_ids_memo = None + config._sync_models_cache_provenance() + try: + model, _, _ = config.resolve_model_provider(mid, explicitly_picked=False) + finally: + if not had: + if old_xai is None: + config._PROVIDER_MODELS.pop('x-ai', None) + else: + config._PROVIDER_MODELS['x-ai'] = old_xai + config.cfg.clear() + config.cfg.update(old_cfg) + config._models_cache_provenance = old_prov + config._available_models_cache = old_cache + assert model == bare, f"unmarked stale cold custom:slug id must strip (legacy), got {model!r}" + + +def test_warm_models_catalog_provenance_if_cold_publishes_from_disk_5979(): + """The send-path warm helper publishes provenance from a valid disk cache + when memory is cold — restoring the endpoint-advertised signal so #433 + strips and #5979 preserves — WITHOUT a live rebuild. + """ + old_cfg = dict(config.cfg) + config.cfg.clear() + config.cfg.update({ + 'model': {'default': 'x-ai/grok-4.5', 'provider': 'custom:llm-proxy', + 'base_url': 'https://proxy.example/v1'}, + 'custom_providers': [{'name': 'llm-proxy', 'base_url': 'https://proxy.example/v1', + 'key_env': 'LLM_PROXY_API_KEY', + 'models': ['x-ai/grok-4.5', 'x-ai/grok-composer-2.5-fast']}], + }) + try: + # Build + persist to disk, then simulate a cold memory cache (restart). + config.invalidate_models_cache() + config.get_available_models() # publishes to memory + disk + assert config._models_cache_provenance is not None + # Simulate cold memory but valid disk cache (do NOT delete disk). + config._available_models_cache = None + config._advertised_model_ids_memo = None + config._sync_models_cache_provenance() + assert config._models_cache_provenance is None, "precondition: memory cold" + # The warm helper should republish provenance from disk (network-free). + config.warm_models_catalog_provenance_if_cold() + assert config._models_cache_provenance is not None, ( + "warm helper must publish provenance from the disk cache" + ) + adv = config._endpoint_advertised_model_ids('custom:llm-proxy') + assert adv and 'x-ai/grok-composer-2.5-fast' in adv, ( + f"warmed provenance must carry the endpoint-advertised ids, got {adv!r}" + ) + finally: + config.cfg.clear() + config.cfg.update(old_cfg) + config.invalidate_models_cache() + + +def test_warm_models_catalog_provenance_noop_when_already_warm_5979(): + """The warm helper is a cheap no-op when provenance for the CURRENT profile + is already published — it must not reload or mutate a warm catalog. + """ + # Use the CURRENT runtime fingerprint so the profile-match fast path is hit. + current_fp = config._models_cache_source_fingerprint() + sentinel = ({'groups': []}, current_fp) + old_prov = config._models_cache_provenance + config._models_cache_provenance = sentinel + try: + config.warm_models_catalog_provenance_if_cold() + assert config._models_cache_provenance is sentinel, ( + "warm helper must not touch already-warm current-profile provenance" + ) + finally: + config._models_cache_provenance = old_prov + + +def test_warm_models_catalog_provenance_ignores_foreign_profile_resident_5979(monkeypatch): + """Profile isolation (Codex finding): a FOREIGN profile's resident provenance + must NOT satisfy the fast no-op. The helper falls through, loads THIS + profile's own disk snapshot, and republishes provenance stamped with the + current runtime fingerprint — so a concurrently-active profile's catalog can + never block this profile from warming its own. + """ + foreign = ({'groups': [{'provider_id': 'custom', 'models': [{'id': 'gpt-5.4'}]}]}, + {'config_yaml': {'path': '/some/other/profile'}}) + my_disk = {'groups': [{'provider_id': 'custom:llm-proxy', + 'models': [{'id': 'x-ai/grok-composer-2.5-fast'}]}]} + old_prov = config._models_cache_provenance + old_cache = config._available_models_cache + old_fp = config._available_models_cache_source_fingerprint + old_memo = config._advertised_model_ids_memo + config._models_cache_provenance = foreign + monkeypatch.setattr(config, '_load_models_cache_from_disk', lambda: my_disk) + try: + config.warm_models_catalog_provenance_if_cold() + prov = config._models_cache_provenance + assert prov is not None and prov[0] is my_disk, ( + "warm must replace foreign provenance with THIS profile's disk snapshot" + ) + assert prov[1] == config._models_cache_source_fingerprint(), ( + "republished provenance must carry the current profile's fingerprint" + ) + finally: + config._models_cache_provenance = old_prov + config._available_models_cache = old_cache + config._available_models_cache_source_fingerprint = old_fp + config._advertised_model_ids_memo = old_memo + + +def test_warm_models_catalog_provenance_never_live_rebuilds_5979(monkeypatch): + """The warm helper must NEVER call get_available_models (which can acquire + the cache lock and block on / trigger a rebuild). It reads the disk cache + directly, so a live provider probe is structurally impossible on the send + path. + """ + old_prov = config._models_cache_provenance + config._models_cache_provenance = None + called = {'get_available_models': False} + def _boom(*a, **k): + called['get_available_models'] = True + raise AssertionError("warm must not call get_available_models") + monkeypatch.setattr(config, 'get_available_models', _boom) + try: + config.warm_models_catalog_provenance_if_cold() # must not raise / call the above + finally: + config._models_cache_provenance = old_prov + assert called['get_available_models'] is False, ( + "warm helper must read disk directly, never via get_available_models" + ) + + +def test_warm_models_catalog_provenance_nonblocking_when_lock_held_5979(): + """The warm helper must NOT block on the models-cache lock. It tries the + lock non-blocking and returns immediately when a concurrent build/publish + holds it, so a send can never hang up to 60s behind a rebuild. + """ + import threading + import time as _time + old_prov = config._models_cache_provenance + config._models_cache_provenance = None + got = config._available_models_cache_lock.acquire(blocking=False) + assert got, "precondition: could not take cache lock" + try: + result = {} + def _worker(): + t0 = _time.time() + config.warm_models_catalog_provenance_if_cold() + result['elapsed'] = _time.time() - t0 + th = threading.Thread(target=_worker) + th.start() + th.join(timeout=5) + blocked = th.is_alive() + finally: + config._available_models_cache_lock.release() + config._models_cache_provenance = old_prov + if blocked: + th.join(timeout=5) + assert not blocked, "warm helper blocked on a held cache lock (must be non-blocking)" + assert result.get('elapsed', 99) < 2.0, ( + f"warm helper should return promptly when lock held, took {result.get('elapsed')!r}s" + ) + + def test_custom_remote_preserves_unknown_prefix_548(): """#548: an unknown vendor prefix (zai-org/GLM-5.1) is always preserved. @@ -1207,3 +1457,86 @@ def test_providers_scan_active_own_providers_entry_owns_over_other_slug_5511(): f"another slug's overlapping entry; got {provider!r}" ) assert base_url == 'https://my.example/v1' + + +# ── #5979: explicit-pick signature lifecycle (Codex-flagged session-update hole) ── + +def test_explicit_pick_signature_matches_same_context_5979(): + """The signature matches when model+provider are unchanged (deliberate pick + is honored across same-model follow-up sends).""" + import api.models as models + sig = models.model_explicit_pick_signature + picked = sig('x-ai/grok-composer-2.5-fast', 'custom:llm-proxy') + assert picked == sig('x-ai/grok-composer-2.5-fast', 'custom:llm-proxy') + + +def test_explicit_pick_signature_invalidated_on_model_change_5979(): + """A model change (e.g. via /api/session/update) yields a DIFFERENT signature, + so a stale explicit-pick can't wrongly preserve a #433 leftover on cold.""" + import api.models as models + sig = models.model_explicit_pick_signature + picked = sig('x-ai/grok-composer-2.5-fast', 'custom:llm-proxy') + # switched to a stale first-party id on the same proxy → signature differs + assert picked != sig('openai/gpt-5.4', 'custom:llm-proxy') + # provider switch also differs + assert picked != sig('x-ai/grok-composer-2.5-fast', 'openai') + + +def test_explicit_pick_signature_persists_round_trip_5979(): + """Session.model_explicit_pick_signature survives save/reload (b3nw's cold + restart scenario), and defaults to None when absent.""" + import api.models as models + s = models.Session(session_id='sig5979', model='x-ai/grok-composer-2.5-fast', + model_provider='custom:llm-proxy') + s.model_explicit_pick_signature = models.model_explicit_pick_signature( + 'x-ai/grok-composer-2.5-fast', 'custom:llm-proxy') + data = {k: getattr(s, k, None) for k in + ['session_id', 'title', 'workspace', 'model', 'model_provider', + 'model_explicit_pick_signature']} + s2 = models.Session(**data) + assert s2.model_explicit_pick_signature == s.model_explicit_pick_signature + # absent → None (unmarked) + s3 = models.Session(session_id='nopick', model='y') + assert s3.model_explicit_pick_signature is None + + +def test_streaming_explicitly_picked_computation_5979(): + """End-to-end lifecycle (Fable fast-follow): reproduce the exact + signature-comparison the streaming worker does at api/streaming.py, proving + the persisted signature drives explicitly_picked correctly across the + deliberate-pick, stale-after-update, and never-picked cases. + + Mirrors streaming.py: + _picked_sig = s.model_explicit_pick_signature + _current_sig = mk_sig(s.model or model, s.model_provider or provider_context) + _explicitly_picked = bool(_picked_sig) and _picked_sig == _current_sig + """ + import api.models as models + mk = models.model_explicit_pick_signature + + def _explicitly_picked(session): + picked = getattr(session, 'model_explicit_pick_signature', None) + cur = mk(getattr(session, 'model', None), getattr(session, 'model_provider', None)) + return bool(picked) and picked == cur + + # 1) deliberate pick, unchanged context → honored + s = models.Session(session_id='e2e1', model='x-ai/grok-composer-2.5-fast', + model_provider='custom:llm-proxy') + s.model_explicit_pick_signature = mk('x-ai/grok-composer-2.5-fast', 'custom:llm-proxy') + assert _explicitly_picked(s) is True + + # 2) session/update switches the model WITHOUT restamping → stale pick ignored + s.model = 'openai/gpt-5.4' # e.g. user switched; signature now stale + assert _explicitly_picked(s) is False + + # 3) provider switch without restamp → stale pick ignored + s2 = models.Session(session_id='e2e2', model='x-ai/grok-composer-2.5-fast', + model_provider='custom:llm-proxy') + s2.model_explicit_pick_signature = mk('x-ai/grok-composer-2.5-fast', 'custom:llm-proxy') + s2.model_provider = 'openai' + assert _explicitly_picked(s2) is False + + # 4) never picked → unmarked + s3 = models.Session(session_id='e2e3', model='x-ai/grok-composer-2.5-fast', + model_provider='custom:llm-proxy') + assert _explicitly_picked(s3) is False