Merge master (exp-v0.52.358) into stage/7651-inflight-cursor
# Conflicts: # CHANGELOG.md
This commit is contained in:
commit
b2b5ea282c
5 files changed
+317
-1
No files matched your search
@@ -33,6 +33,17 @@
|
||||
could seed the replay floor and the reattached stream skipped the current reply's early events.
|
||||
Replay now seeds only from the current live assistant row, and falls back to a full replay that
|
||||
rebuilds the assistant body when a cursor outlives its live state. (#7651 by @happy5318)
|
||||
- **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)
|
||||
- **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
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -7183,6 +7183,34 @@ 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:
|
||||
# #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.
|
||||
#
|
||||
# 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"))
|
||||
_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()
|
||||
|
||||
custom_providers = cfg.get("custom_providers") if isinstance(cfg, dict) else None
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
"""Regression coverage for notify_on_complete across a WebUI restart."""
|
||||
|
||||
import logging
|
||||
import threading
|
||||
import time
|
||||
from types import SimpleNamespace
|
||||
@@ -159,3 +160,102 @@ def test_recovery_is_fail_soft_without_agent(monkeypatch):
|
||||
|
||||
assert bp.recover_processes_for_webui() == 0
|
||||
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]}"
|
||||
)
|
||||
@@ -631,3 +631,165 @@ 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)"
|
||||
)
|
||||
|
||||
|
||||
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)"
|
||||
)
|
||||
Reference in new issue
Block a user