From a8180bc51f9c2c900c6a570dc7df34193e793295 Mon Sep 17 00:00:00 2001 From: webtecnica Date: Tue, 22 Sep 2026 22:54:12 -0300 Subject: [PATCH 1/5] fix(context-length): guard provider ownership on global model.base_url backfill (#7535) A session on a built-in registry provider (e.g. opencode-go, endpoint in PROVIDER_REGISTRY, empty base_url by design) had its empty base_url slot backfilled with the global model.base_url owned by a *different* provider (custom:ark). The context-length resolver then probed the wrong endpoint's /models catalog and fell back to the generic 128K catalog entry for a 1M-context model, while the on-disk session and SSE stream carried 1,000,000. Only fill the global model.base_url slot when the session provider IS the configured model.provider owner, mirroring the ownership predicate used for model_cfg's API key in _context_length_config_api_key_for_provider. --- api/routes.py | 12 ++- ...e3256_context_length_default_only_guard.py | 73 +++++++++++++++++++ 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/api/routes.py b/api/routes.py index 5f5893ba..407abbac 100644 --- a/api/routes.py +++ b/api/routes.py @@ -7183,7 +7183,17 @@ def _context_length_lookup_inputs_for_model( if not effective_provider: effective_provider = _canonical_context_provider(model_cfg.get("provider")) if not effective_base_url: - effective_base_url = str(model_cfg.get("base_url") or "").strip() + # #7535: the global model.base_url may only fill an empty slot when + # the session provider IS the configured model.provider owner + # (mirror the ownership predicate used for model_cfg's API key in + # _context_length_config_api_key_for_provider). A built-in registry + # provider (empty base_url by design) must keep the slot empty so + # the registry endpoint resolves instead of another provider's URL. + _model_cfg_provider = _canonical_context_provider(model_cfg.get("provider")) + if not effective_provider or _providers_match_for_context( + _model_cfg_provider, effective_provider + ): + effective_base_url = str(model_cfg.get("base_url") or "").strip() custom_providers = cfg.get("custom_providers") if isinstance(cfg, dict) else None if not isinstance(custom_providers, list): diff --git a/tests/test_issue3256_context_length_default_only_guard.py b/tests/test_issue3256_context_length_default_only_guard.py index ea9ac9da..34f4b50f 100644 --- a/tests/test_issue3256_context_length_default_only_guard.py +++ b/tests/test_issue3256_context_length_default_only_guard.py @@ -631,3 +631,76 @@ def test_sse_done_path_broadened_to_any_model_window_mismatch(): assert "_model_matches_configured_default as _mmcd_cc" not in _STREAMING_SRC, ( "the old default-only save matcher (_mmcd_cc) must be removed (#4618)" ) + + +# --- #7535: provider-ownership guard on the global model.base_url backfill --- +# Same family as #3256: a global value scoped to its owner must not leak onto +# sessions owned by a different provider. + + +def test_builtin_provider_lookup_does_not_inherit_other_providers_base_url(): + """#7535: a session on a built-in registry provider (whose base_url is empty + by design because the endpoint lives in PROVIDER_REGISTRY, not config.yaml) + must NOT inherit the global ``model.base_url`` owned by a different (custom) + provider. The unguarded backfill handed the custom endpoint to the + context-length resolver, which probed the wrong ``/models`` catalog and fell + back to the generic 128K catalog entry for a 1M-context model, while the + on-disk session and SSE stream both carried 1,000,000.""" + from api.routes import _context_length_lookup_inputs_for_model + + ark_base = "https://ark.cn-beijing.volces.com/api/coding/v3" + cfg = { + "model": { + "provider": "custom:ark", + "base_url": ark_base, + "default": "ark-code-latest", + "context_length": 1000000, + }, + "custom_providers": [ + { + "name": "ark", + "base_url": ark_base, + "api_key": "ark-key", + "model": "ark-code-latest", + } + ], + } + + lookup = _context_length_lookup_inputs_for_model( + "@opencode-go:deepseek-v4.1-flash", + "opencode-go", + cfg=cfg, + ) + + assert lookup.provider == "opencode-go" + assert lookup.base_url == "", ( + "the global model.base_url belongs to custom:ark; the opencode-go " + "registry slot must stay empty so PROVIDER_REGISTRY's endpoint " + "resolves the context window (#7535)" + ) + + +def test_same_provider_session_keeps_global_base_url_backfill(): + """#7535 control (compat): when the session provider IS the configured + owner of ``model.base_url``, the backfill behavior is preserved.""" + from api.routes import _context_length_lookup_inputs_for_model + + ark_base = "https://ark.cn-beijing.volces.com/api/coding/v3" + cfg = { + "model": { + "provider": "custom:ark", + "base_url": ark_base, + "default": "ark-code-latest", + }, + } + + lookup = _context_length_lookup_inputs_for_model( + "ark-code-latest", + "custom:ark", + cfg=cfg, + ) + + assert lookup.provider == "custom:ark" + assert lookup.base_url == ark_base, ( + "same-owner sessions must keep the model.base_url backfill (#7535 compat)" + ) From 301ed54ab7f176038cc977bb9758a0210a1605e3 Mon Sep 17 00:00:00 2001 From: webtecnica Date: Wed, 23 Sep 2026 09:26:24 -0300 Subject: [PATCH 2/5] fix(context-length): restore base_url for two guarded configs (#7743) The #7535 ownership guard skipped the global model.base_url backfill for two shapes that master handled: - A `model` section with `base_url` but no `provider` declares no owner, so `_providers_match_for_context("", )` was false and the endpoint was dropped, silently falling back to 256,000 instead of querying the configured /v1/models (the profile-setup path writes this shape). - `opencode_go` and `opencode-go` are two spellings of the same built-in provider, but `_canonical_context_provider` does not fold underscores, so the owner session lost its URL. Allow the backfill when no owner is declared, and compare both sides through api.config._canonicalise_provider_id, which folds `_` to `-` while keeping distinct `custom:*` slugs distinct. The #7535 case (a custom:ark owner vs an opencode-go session) still resolves to an empty base_url. --- api/routes.py | 22 ++++- ...e3256_context_length_default_only_guard.py | 89 +++++++++++++++++++ 2 files changed, 109 insertions(+), 2 deletions(-) diff --git a/api/routes.py b/api/routes.py index 27e3c912..b6c611ed 100644 --- a/api/routes.py +++ b/api/routes.py @@ -7189,9 +7189,27 @@ def _context_length_lookup_inputs_for_model( # _context_length_config_api_key_for_provider). A built-in registry # provider (empty base_url by design) must keep the slot empty so # the registry endpoint resolves instead of another provider's URL. + # + # Two shapes cannot own the slot and therefore cannot conflict, so + # they keep master's backfill: a config that declares no provider + # at all (the profile-setup path writes model.base_url without one) + # and the two spellings of the same built-in id (opencode_go == + # opencode-go), folded through api.config._canonicalise_provider_id + # so distinct custom:* slugs stay distinct. _model_cfg_provider = _canonical_context_provider(model_cfg.get("provider")) - if not effective_provider or _providers_match_for_context( - _model_cfg_provider, effective_provider + _owner_provider = _model_cfg_provider + _session_provider = effective_provider + try: + from api.config import _canonicalise_provider_id as _canon_provider_id + + _owner_provider = _canon_provider_id(_owner_provider) or _owner_provider + _session_provider = _canon_provider_id(_session_provider) or _session_provider + except Exception: + pass + if ( + not effective_provider + or not _model_cfg_provider + or _providers_match_for_context(_owner_provider, _session_provider) ): effective_base_url = str(model_cfg.get("base_url") or "").strip() diff --git a/tests/test_issue3256_context_length_default_only_guard.py b/tests/test_issue3256_context_length_default_only_guard.py index 34f4b50f..b7bb3288 100644 --- a/tests/test_issue3256_context_length_default_only_guard.py +++ b/tests/test_issue3256_context_length_default_only_guard.py @@ -704,3 +704,92 @@ def test_same_provider_session_keeps_global_base_url_backfill(): assert lookup.base_url == ark_base, ( "same-owner sessions must keep the model.base_url backfill (#7535 compat)" ) + + +def test_base_url_only_config_without_provider_keeps_endpoint(): + """Regression (PR #7743 gate): a ``model`` section that declares + ``base_url`` but NO ``provider`` has no owner that could conflict, so any + session must still receive the URL — exactly as master did. The profile + setup path writes this shape (api/profiles.py), and dropping the URL made + the lookup skip the endpoint and silently fall back to 256,000.""" + from api.routes import _context_length_lookup_inputs_for_model + + only_base_url = "http://127.0.0.1:9/v1" + cfg = { + "model": { + "base_url": only_base_url, + "default": "some-model", + }, + } + + for session_provider in ("custom", "openrouter"): + lookup = _context_length_lookup_inputs_for_model( + "some-model", + session_provider, + cfg=cfg, + ) + + assert lookup.provider == session_provider + assert lookup.base_url == only_base_url, ( + f"a config with no declared provider owner cannot conflict with the " + f"{session_provider!r} session, so the base_url must survive the " + f"ownership guard" + ) + + +def test_underscore_provider_owner_matches_hyphen_session(): + """Regression (PR #7743 gate): ``opencode_go`` and ``opencode-go`` are two + spellings of the SAME built-in provider, so the owner session must still + receive ``model.base_url``. Comparing the ids through the repo's + canonicaliser folds the underscore without collapsing distinct + ``custom:*`` slugs.""" + from api.routes import _context_length_lookup_inputs_for_model + + owner_base = "http://127.0.0.1:9/v1" + cfg = { + "model": { + "provider": "opencode_go", + "base_url": owner_base, + "default": "some-model", + }, + } + + lookup = _context_length_lookup_inputs_for_model( + "some-model", + "opencode-go", + cfg=cfg, + ) + + assert lookup.provider == "opencode-go" + assert lookup.base_url == owner_base, ( + "opencode_go and opencode-go are the same built-in provider, so the " + "owner session must keep the base_url backfill" + ) + + +def test_underscore_provider_still_blocks_distinct_custom_owner(): + """Counterpart to test_underscore_provider_owner_matches_hyphen_session: + folding underscores must not open the #7535 guard. ``custom:my_box`` and a + session on the unrelated built-in ``opencode-go`` are still different + providers, so the custom owner's URL stays out of the registry slot.""" + from api.routes import _context_length_lookup_inputs_for_model + + cfg = { + "model": { + "provider": "custom:my_box", + "base_url": "http://127.0.0.1:9/v1", + "default": "some-model", + }, + } + + lookup = _context_length_lookup_inputs_for_model( + "some-model", + "opencode-go", + cfg=cfg, + ) + + assert lookup.provider == "opencode-go" + assert lookup.base_url == "", ( + "underscore folding must not let a distinct custom provider's " + "base_url leak onto another provider's session (#7535 guard)" + ) From 5466d09e91548e07b3eac910f99bd09d31fc9c70 Mon Sep 17 00:00:00 2001 From: happy5318 <5318happy@users.noreply.github.com> Date: Wed, 23 Sep 2026 21:23:32 +0800 Subject: [PATCH 3/5] fix(recovery): map vanished-session KeyError to the skip path in WebUI startup recovery (#7753) --- api/background_process.py | 17 ++- ...est_background_process_restart_recovery.py | 102 +++++++++++++++++- 2 files changed, 117 insertions(+), 2 deletions(-) diff --git a/api/background_process.py b/api/background_process.py index 6e6323af..eea15ebb 100644 --- a/api/background_process.py +++ b/api/background_process.py @@ -1883,7 +1883,22 @@ def recover_processes_for_webui(process_registry=None, get_session_fn=None) -> i try: proc_session = process_registry.get(process_id) session_key = str(getattr(proc_session, "session_key", "") or "") - if not session_key or get_session_fn(session_key, metadata_only=True) is None: + if not session_key: + continue + # The session resolver (``api.models._resolve_session_once``) + # raises ``KeyError(sid)`` for a missing session — that is its + # documented contract for the "owner gone" outcome, not a + # fault. Treat it identically to a ``None`` return: skip the + # process without logging, and let a neighbouring live + # process still rebind. The narrow ``except KeyError`` lives + # INSIDE the outer try so an unrelated ``KeyError`` from + # ``process_registry.get()`` (or any routing-index work) still + # surfaces in the existing warning path. + try: + resolved = get_session_fn(session_key, metadata_only=True) + except KeyError: + continue + if resolved is None: continue except Exception: logger.warning( diff --git a/tests/test_background_process_restart_recovery.py b/tests/test_background_process_restart_recovery.py index 0ab76e36..45ee82ff 100644 --- a/tests/test_background_process_restart_recovery.py +++ b/tests/test_background_process_restart_recovery.py @@ -1,5 +1,6 @@ """Regression coverage for notify_on_complete across a WebUI restart.""" +import logging import threading import time from types import SimpleNamespace @@ -158,4 +159,103 @@ def test_recovery_is_fail_soft_without_agent(monkeypatch): monkeypatch.setattr("builtins.__import__", fake_import) assert bp.recover_processes_for_webui() == 0 - assert bp._PROCESS_RECOVERY_DONE is False \ No newline at end of file + assert bp._PROCESS_RECOVERY_DONE is False + + +def test_recovery_skips_process_whose_session_is_gone(monkeypatch, caplog): + """A checkpointed process whose owner has been pruned (the resolver's + ``KeyError(sid)`` contract) must be silently skipped, while a neighbouring + live process in the same recovery sweep still rebinds. Regression for + issue #7753: previously every vanished owner logged a full WARNING + traceback, indistinguishable from a real fault. + """ + calls = {"registered": []} + + class FakeRegistry: + def recover_from_checkpoint(self): + return 2 # two checkpointed processes survive restart + + def list_sessions(self): + return [ + {"session_id": "proc_vanished", "detached": True}, + {"session_id": "proc_live", "detached": True}, + ] + + def get(self, process_id): + if process_id == "proc_vanished": + return _FakeProcessSession("vanished-session") + if process_id == "proc_live": + return _FakeProcessSession("live-session") + raise AssertionError(process_id) + + fake_registry = FakeRegistry() + monkeypatch.setattr(bp, "_PROCESS_CHECKPOINT_RECOVERED", False) + monkeypatch.setattr(bp, "_PROCESS_RECOVERY_DONE", False) + monkeypatch.setattr( + bp, + "register_process_session", + lambda key, sid: calls["registered"].append((key, sid)), + ) + + def get_session(sid, metadata_only=False): + # Mirrors api.models._resolve_session_once's contract: missing + # sessions raise KeyError, NOT None. + if sid == "vanished-session": + raise KeyError(sid) + return SimpleNamespace(id=sid) + + with caplog.at_level(logging.WARNING, logger="api.background_process"): + result = bp.recover_processes_for_webui(fake_registry, get_session) + + assert result == 2 + # The live process is rebound; the vanished one is silently skipped. + assert calls["registered"] == [("live-session", "live-session")] + # No WARNING emitted for the vanished-owner case. + warning_records = [ + r for r in caplog.records if r.levelno >= logging.WARNING + ] + assert warning_records == [], ( + "vanished-session KeyError must not produce a WARNING, got: " + f"{[r.getMessage() for r in warning_records]}" + ) + assert "Could not resolve recovered WebUI process" not in caplog.text + + +def test_recovery_still_warns_on_unexpected_resolve_failure(monkeypatch, caplog): + """Non-KeyError failures from the resolver (e.g. ``OSError`` on an + unreadable sidecar) must keep entering the existing WARNING path. This + pins that the fix for #7753 is narrow and does not over-broaden the + silent-skip contract. + """ + class FakeRegistry: + def recover_from_checkpoint(self): + return 1 + + def list_sessions(self): + return [{"session_id": "proc_unexpected", "detached": True}] + + def get(self, process_id): + assert process_id == "proc_unexpected" + return _FakeProcessSession("webui-session") + + fake_registry = FakeRegistry() + monkeypatch.setattr(bp, "_PROCESS_CHECKPOINT_RECOVERED", False) + monkeypatch.setattr(bp, "_PROCESS_RECOVERY_DONE", False) + + def get_session(sid, metadata_only=False): + raise OSError("sidecar unreadable") + + with caplog.at_level(logging.WARNING, logger="api.background_process"): + result = bp.recover_processes_for_webui(fake_registry, get_session) + + assert result == 1 + warning_records = [ + r for r in caplog.records if r.levelno >= logging.WARNING + ] + assert any( + "Could not resolve recovered WebUI process 'proc_unexpected'" in r.getMessage() + for r in warning_records + ), ( + "OSError from the resolver must still emit a WARNING, got: " + f"{[r.getMessage() for r in warning_records]}" + ) \ No newline at end of file From ef7d325a5484d2db32a66ce753c12791cc414eab Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Wed, 23 Sep 2026 14:34:41 +0000 Subject: [PATCH 4/5] docs(changelog): #7774 --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7bb2efc3..e0bfaa58 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,12 @@ ### Fixed +- **A session deleted during a restart no longer produces a spurious recovery warning.** When + WebUI startup recovery re-attached background processes, a session that had vanished between + enumeration and rebind raised a `KeyError` that was logged as a warning. It now follows the + existing skip path, confined to the session lookup, so the vanished owner is skipped, live owners + still rebind, and registry errors still warn. (#7753, #7774 by @happy5318) + - **Waiting on the Agent's session lease is shown as a warning instead of looking stuck.** When another Hermes process (gateway, CLI or cron) holds the session's turn lease, the Agent's "another Hermes process is using this session" notices now reach the chat as a warning status From 16bc5ab3edec0df63aea53b130838fede07b7340 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Wed, 23 Sep 2026 14:34:41 +0000 Subject: [PATCH 5/5] docs(changelog): #7743 --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7bb2efc3..fbc1014f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,13 @@ ### Fixed +- **Context-length lookup keeps the configured base URL for ownerless and underscore-named + providers.** The #7535 ownership guard also dropped the global `model.base_url` for a `model:` + section with a `base_url` but no `provider`, and for provider IDs written with an underscore + (`opencode_go` vs `opencode-go`), so those sessions could resolve the wrong context window. Both + now keep the URL, while a different declared owner still does not leak its endpoint. + (#7743 by @webtecnica) + - **Waiting on the Agent's session lease is shown as a warning instead of looking stuck.** When another Hermes process (gateway, CLI or cron) holds the session's turn lease, the Agent's "another Hermes process is using this session" notices now reach the chat as a warning status