Batch release exp-v0.52.235 — cancelling-run reattach (#7096) + GPT-5.6 max reasoning (#7083) + test order-independence (#7101) (#7102)
* fix(tests): make mtime_invalidation and glm_5_3 tests order-independent (#7100) * fix: expose max reasoning for GPT-5.6 models * fix(recovery): do not reattach cancelling runs ACTIVE_RUNS tracks worker lifecycle, which is deliberately broader than "a turn a browser may attach to". cancel_stream() keeps the row as phase="cancelling" while the worker unwinds so a successor turn cannot start on top of it, but the client has already reached a terminal state for that stream: its run journal ends in a terminal event. The recovery lookups treated every same-session ACTIVE_RUNS row as attachable. An idle session holding a cancelling row therefore received a recovered server_turn_started on every /api/session/stream subscription: the client attached, replayed the terminal event, tore the renderer down, resubscribed, and the server replayed the same frame again. The result is an endless attach/replay loop that rebuilds the transcript repeatedly. Separate the two meanings instead of narrowing one call site: - api/config.py gains active_run_is_attachable() and active_run_cancel_is_stale() as the shared predicates. - active_stream_id_for_session() (browser recovery) returns attachable rows only; _session_has_active_turn() (busy check) keeps counting a fresh cancellation so a successor cannot overlap the unwinding worker. - _live_active_stream_id() applies the same rule to the hidden-tab status poller, on both the STREAMS and ACTIVE_RUNS paths. - routes._cancelled_run_is_stale() now delegates to the shared predicate rather than keeping a parallel copy of the staleness rule. - A cancelling row past a bounded unwind window with no live STREAMS channel is reclaimed from ACTIVE_RUNS and its stream owner released, so a wedged worker cannot suppress background wakeups forever. Age alone does not reclaim a row that still owns a live channel. Staleness anchors on cancelled_at, falling back to started_at, so a long-running turn cancelled moments ago is never treated as an orphan. tests/test_cancelling_run_not_attachable.py covers both directions of each rule. The six behavioral tests fail on the unpatched tree and pass with the fix; two targeted mutations (forcing the attachability predicate true, and disabling the staleness reaper) each turn the suite red. * docs(rfc): document cancellation attach/admission split Records the runtime contract introduced by the recovery fix in the WebUI run-state consistency RFC, so the distinction is discoverable instead of living only in code comments. - Adds ACTIVE_RUNS to the State Layers table as the worker-lifecycle registry, explicitly not the set of runs a browser may attach to. - Adds invariant 9: lifecycle-busy is not client-attachable. Cancellation splits the two meanings, recovery paths must exclude cancelling rows, and admission checks must keep counting them. - Documents the bounded cancellation-unwind window: reclamation needs both age and the absence of a live STREAMS channel, and staleness is anchored on the cancellation timestamp. - Extends the review checklist with the admission-vs-attachment question and the evidence required when changing a reclamation window. * docs(changelog): note #7096 cancelling-run reattach, #7083 GPT-5.6 max reasoning, #7101 test order-independence --------- Co-authored-by: webtecnica <webtecnica@gmail.com> Co-authored-by: Abdulrahman Elkenany <boudy.elkenany123@gmail.com> Co-authored-by: allenliang2022 <allenliang2022@users.noreply.github.com> Co-authored-by: n <a@n>
This commit is contained in:
10 files changed
+508
-103
No files matched your search
@@ -5,6 +5,12 @@
|
||||
|
||||
### Fixed
|
||||
|
||||
- **Cancelling runs are no longer reattached on recovery.** A background run in the middle of cancellation could be resurrected by a reconnect/recovery reattach, producing a dying run that could double-deliver or churn. Recovery paths now exclude runs whose phase is `cancelling` (they stay busy but are not reattached), while genuinely-live runs still reattach; stale-cancelling reclamation is bounded (age ≥ 180s and absent from active streams) and idempotent. Thanks @allenliang2022. (#7096)
|
||||
|
||||
- **GPT-5.6 models expose their full reasoning range.** The four GPT-5.6 model IDs support the `max` reasoning-effort level, but the capability filter capped them below that. The ceiling is now lifted for exactly those IDs on the OpenAI-family lanes (older GPT-5 and o-series ceilings unchanged), and `max` is offered only when the authoritative configured/live catalog includes it. Thanks @boudywho. (#7083)
|
||||
|
||||
- **Test suite: two model-catalog tests are now order-independent.** `test_mtime_invalidation` depended on sibling-test cache state and `test_glm_5_3_in_models_payload_for_zai_provider` depended on the installed agent-core catalog version, so both failed spuriously under isolation/sharding. They now set up their own preconditions. Thanks @webtecnica. (#7101)
|
||||
|
||||
- **Binary files in the workspace no longer truncate when served on Windows.** The shared anchored file-open helper omitted the platform's `O_BINARY` flag, so on Windows a binary workspace file (video, image, archive) was read in text mode and truncated at its first `0x1A` (Ctrl-Z) byte — a 5.2 MB video could arrive as a few hundred bytes, breaking Range requests and playback. Non-directory anchored opens now set `O_BINARY` where the platform provides it (a no-op on Linux/macOS). Thanks @allenliang2022. (#7077)
|
||||
|
||||
- **Unstyled native form controls now use the UI font.** Bare `button`, `input`, `select`, and `textarea` elements fell back to the browser default font instead of the app's UI font, and the markdown table filter lost the UI font after its `font: inherit` shorthand. Both now route through `--font-ui`; mono, conversation, and skin-scoped controls are unchanged. Thanks @starship-s. (#6871)
|
||||
|
||||
+79
-28
@@ -271,6 +271,77 @@ def subscribe_to_session_channel(
|
||||
return ch, q
|
||||
|
||||
|
||||
# Bounded window a cancelling worker may stay lifecycle-busy before an entry
|
||||
# with no live SSE channel is treated as an orphan. Matches the unwind ceiling
|
||||
# used by the chat-start successor guard in ``api.routes``.
|
||||
_ACTIVE_RUN_CANCEL_UNWIND_SECONDS = 180.0
|
||||
|
||||
|
||||
def _active_run_ids_for_session(
|
||||
session_id: str,
|
||||
*,
|
||||
attachable_only: bool,
|
||||
) -> list[str]:
|
||||
"""Return this session's run stream ids under the requested semantics.
|
||||
|
||||
``attachable_only`` selects browser-recovery semantics and drops every
|
||||
``phase="cancelling"`` row: cancellation is already terminal for the client
|
||||
even while the worker unwinds. Busy checks pass ``False`` so a freshly
|
||||
cancelled run still blocks a successor for its bounded unwind window.
|
||||
|
||||
A cancelling row past that window with no live ``STREAMS`` channel is an
|
||||
orphan: it is dropped from ``ACTIVE_RUNS`` and its stream-owner entry is
|
||||
released, so a wedged worker cannot suppress background wakeups forever.
|
||||
"""
|
||||
from api import config as _cfg
|
||||
|
||||
sid = str(session_id or "").strip()
|
||||
if not sid:
|
||||
return []
|
||||
try:
|
||||
with _cfg.STREAMS_LOCK:
|
||||
live_stream_ids = set((_cfg.STREAMS or {}).keys())
|
||||
now = time.time()
|
||||
matches: list[str] = []
|
||||
stale_keys: list[str] = []
|
||||
with _cfg.ACTIVE_RUNS_LOCK:
|
||||
for run_key, meta in list((_cfg.ACTIVE_RUNS or {}).items()):
|
||||
if not isinstance(meta, dict) or meta.get("session_id") != sid:
|
||||
continue
|
||||
stream_id = str(meta.get("stream_id") or run_key or "").strip()
|
||||
if not stream_id:
|
||||
continue
|
||||
cancelling = not _cfg.active_run_is_attachable(meta)
|
||||
stale_cancel = (
|
||||
cancelling
|
||||
and _cfg.active_run_cancel_is_stale(
|
||||
meta,
|
||||
grace_seconds=_ACTIVE_RUN_CANCEL_UNWIND_SECONDS,
|
||||
now=now,
|
||||
)
|
||||
and run_key not in live_stream_ids
|
||||
and stream_id not in live_stream_ids
|
||||
)
|
||||
if stale_cancel:
|
||||
stale_keys.append(run_key)
|
||||
continue
|
||||
if attachable_only and cancelling:
|
||||
continue
|
||||
matches.append(stream_id)
|
||||
for run_key in stale_keys:
|
||||
(_cfg.ACTIVE_RUNS or {}).pop(run_key, None)
|
||||
for run_key in stale_keys:
|
||||
_cfg.unregister_stream_owner(run_key)
|
||||
return matches
|
||||
except Exception:
|
||||
logger.debug(
|
||||
"ACTIVE_RUNS lookup failed for %s",
|
||||
sid,
|
||||
exc_info=True,
|
||||
)
|
||||
return []
|
||||
|
||||
|
||||
def active_stream_id_for_session(session_id: str) -> Optional[str]:
|
||||
"""Return the stream_id of the live run for *session_id*, or None.
|
||||
|
||||
@@ -288,25 +359,14 @@ def active_stream_id_for_session(session_id: str) -> Optional[str]:
|
||||
|
||||
Keys on ACTIVE_RUNS (worker-lifecycle registry) — the same source
|
||||
``_session_has_active_turn`` / ``_emit_to_session_streams`` already trust
|
||||
to map a stream back to its owning session. Returns the first matching
|
||||
stream_id (a session has at most one live run; cancel/reconnect can
|
||||
briefly hold two — either is a valid attach target, the frontend dedupes
|
||||
by stream_id).
|
||||
to map a stream back to its owning session — but returns only rows that
|
||||
are still ATTACHABLE. A ``phase="cancelling"`` row stays lifecycle-busy
|
||||
while its worker unwinds, yet the client already reached a terminal state
|
||||
for that run, so replaying ``server_turn_started`` for it makes the tab
|
||||
attach, receive the terminal event, resubscribe, and loop forever.
|
||||
"""
|
||||
from api import config as _cfg
|
||||
|
||||
try:
|
||||
with _cfg.ACTIVE_RUNS_LOCK:
|
||||
for _stream_id, meta in (_cfg.ACTIVE_RUNS or {}).items():
|
||||
if isinstance(meta, dict) and meta.get("session_id") == session_id:
|
||||
return str(_stream_id)
|
||||
except Exception:
|
||||
logger.debug(
|
||||
"active_stream_id_for_session lookup failed for %s",
|
||||
session_id,
|
||||
exc_info=True,
|
||||
)
|
||||
return None
|
||||
matches = _active_run_ids_for_session(session_id, attachable_only=True)
|
||||
return matches[0] if matches else None
|
||||
|
||||
|
||||
def persisted_message_count_for_session(session_id: str) -> Optional[int]:
|
||||
@@ -1518,16 +1578,7 @@ def _session_has_active_turn(session_id: str) -> bool:
|
||||
``_start_chat_stream_for_session``'s own active-stream guard — i.e. the
|
||||
same lock /api/chat/start uses is the authoritative race backstop.
|
||||
"""
|
||||
from api import config as _cfg
|
||||
|
||||
try:
|
||||
with _cfg.ACTIVE_RUNS_LOCK:
|
||||
for _stream_id, meta in (_cfg.ACTIVE_RUNS or {}).items():
|
||||
if isinstance(meta, dict) and meta.get("session_id") == session_id:
|
||||
return True
|
||||
except Exception:
|
||||
logger.debug("ACTIVE_RUNS active-turn check failed", exc_info=True)
|
||||
return False
|
||||
return bool(_active_run_ids_for_session(session_id, attachable_only=False))
|
||||
|
||||
|
||||
def _start_server_side_wakeup_turn(
|
||||
|
||||
+92
-23
@@ -3515,6 +3515,24 @@ def _zai_glm_thinking_toggle_supported(model_id: str, provider_id: str) -> bool
|
||||
return cls in {"effort", "thinking"}
|
||||
|
||||
|
||||
_OPENAI_FAMILY_REASONING_PROVIDERS = frozenset({
|
||||
"openai-codex", "openai", "openai-api",
|
||||
"azure-foundry", "azure-openai", "azure",
|
||||
})
|
||||
|
||||
_GPT_5_6_REASONING_MODELS = frozenset({
|
||||
"gpt-5.6",
|
||||
"gpt-5.6-sol",
|
||||
"gpt-5.6-terra",
|
||||
"gpt-5.6-luna",
|
||||
})
|
||||
|
||||
|
||||
def _is_gpt_5_6_reasoning_model(bare_model: str) -> bool:
|
||||
"""Return whether an OpenAI-family model uses GPT-5.6's max ladder."""
|
||||
return str(bare_model or "").strip().lower() in _GPT_5_6_REASONING_MODELS
|
||||
|
||||
|
||||
def _filter_reasoning_efforts_for_provider(
|
||||
efforts: list[str],
|
||||
model_id: str,
|
||||
@@ -3529,15 +3547,15 @@ def _filter_reasoning_efforts_for_provider(
|
||||
normalized = list(dict.fromkeys(normalized))
|
||||
provider = _resolve_provider_alias(str(provider_id or "").strip().lower())
|
||||
bare = _strip_provider_hint_for_reasoning(model_id).lower().rsplit("/", 1)[-1]
|
||||
# OpenAI-family lanes (Codex, direct OpenAI, Azure Foundry) cap GPT-5 at xhigh
|
||||
# and o-series at high — 'max' is a WebUI-only level none of them accept.
|
||||
if provider in {"openai-codex", "openai", "openai-api", "azure-foundry", "azure-openai", "azure"}:
|
||||
# OpenAI-family lanes cap pre-GPT-5.6 GPT-5 models at xhigh and o-series at
|
||||
# high. GPT-5.6's alias and Sol/Terra/Luna variants natively accept max.
|
||||
if provider in _OPENAI_FAMILY_REASONING_PROVIDERS:
|
||||
if bare.startswith(("o1", "o3", "o4")):
|
||||
return [eff for eff in normalized if eff in {"low", "medium", "high"}]
|
||||
if bare.startswith("gpt-5"):
|
||||
if bare.startswith("gpt-5") and not _is_gpt_5_6_reasoning_model(bare):
|
||||
return [eff for eff in normalized if eff != "max"]
|
||||
# 'max' is a WebUI-level ceiling; providers whose native ladder tops out lower
|
||||
# must NOT advertise it, otherwise a stored/CLI 'max' degrades WORSE than the
|
||||
# Providers whose native ladder tops out below 'max' must NOT advertise it,
|
||||
# otherwise a stored/CLI 'max' degrades WORSE than the
|
||||
# prior max->xhigh coercion (Gemini's adapter treats unknown 'max' as medium;
|
||||
# pre-adaptive Anthropic manual-thinking lacks a 'max' budget and falls to 8k).
|
||||
# Dropping 'max' here lets the existing downgrade ladder land on xhigh/high.
|
||||
@@ -3884,11 +3902,11 @@ def resolve_model_reasoning_efforts(
|
||||
"""Return supported reasoning-effort levels for *model_id*, or [] if none.
|
||||
|
||||
Always passes the sourced list through _filter_reasoning_efforts_for_provider
|
||||
so the hard provider ceilings (openai-codex/openai/azure GPT-5 cap at xhigh,
|
||||
Gemini + pre-adaptive/cloud-hosted Claude cap at xhigh) are applied uniformly
|
||||
— the UI dropdown (which gates options on this list) and coercion therefore
|
||||
agree: 'max' is offered ONLY for models whose native ladder genuinely includes
|
||||
it, and is stripped everywhere it would be rejected/mishandled.
|
||||
so the hard provider ceilings (OpenAI-family GPT-5 before 5.6 at xhigh and
|
||||
o-series at high; Gemini + pre-adaptive/cloud-hosted Claude at xhigh) are
|
||||
applied uniformly. The UI dropdown and coercion therefore agree: ``max`` is
|
||||
retained for GPT-5.6 and other models whose native ladder includes it, and
|
||||
stripped where it would be rejected or mishandled.
|
||||
"""
|
||||
raw = _resolve_model_reasoning_efforts_impl(model_id, provider_id, base_url)
|
||||
if not raw:
|
||||
@@ -4081,16 +4099,17 @@ def coerce_reasoning_effort_for_model(
|
||||
# Hard provider ceilings must win regardless of what the sourced capability
|
||||
# list says. resolve_model_reasoning_efforts() draws from hermes_cli /
|
||||
# models.dev / heuristics, and those can (a) return [] for an unrecognized
|
||||
# model or (b) wrongly advertise a WebUI-only level like 'max' for a provider
|
||||
# model or (b) wrongly advertise 'max' for a provider
|
||||
# whose native ladder tops out lower. _filter_reasoning_efforts_for_provider
|
||||
# encodes the known ceilings (openai-codex gpt-5, Gemini, pre-adaptive
|
||||
# Anthropic all cap below 'max'); if it actively EXCLUDES the requested level,
|
||||
# honor that ceiling and degrade down the ladder even when the sourced list is
|
||||
# empty or (mistakenly) includes the level. This keeps a stored/CLI 'max' from
|
||||
# reaching an adapter that would silently downgrade it worse than xhigh/high
|
||||
# (Gemini→medium, legacy Claude manual-thinking→8k). For providers with NO
|
||||
# ceiling rule the filter returns the full list unchanged, so genuinely
|
||||
# unknown models still preserve the configured effort (#3505 behavior).
|
||||
# encodes the known ceilings (OpenAI-family GPT-5 before 5.6, Gemini, and
|
||||
# pre-adaptive Anthropic all cap below 'max'); if it actively EXCLUDES the
|
||||
# requested level, honor that ceiling and degrade down the ladder even when
|
||||
# the sourced list is empty or (mistakenly) includes the level. This keeps a
|
||||
# stored/CLI 'max' from reaching an adapter that would silently downgrade it
|
||||
# worse than xhigh/high (Gemini→medium, legacy Claude manual-thinking→8k).
|
||||
# GPT-5.6 is intentionally not capped. For providers with NO ceiling rule the
|
||||
# filter returns the full list unchanged, so genuinely unknown models still
|
||||
# preserve the configured effort (#3505 behavior).
|
||||
ceiling = _filter_reasoning_efforts_for_provider(
|
||||
list(VALID_REASONING_EFFORTS), str(model_id or ""), str(provider_id or "")
|
||||
)
|
||||
@@ -4108,15 +4127,15 @@ def coerce_reasoning_effort_for_model(
|
||||
# both for models KNOWN not to support reasoning AND for models we simply
|
||||
# don't recognize (custom providers, aggregator-rewritten ids, brand-new
|
||||
# releases). Coercion exists to avoid sending a level a KNOWN-incompatible
|
||||
# model rejects (e.g. openai-codex gpt-5 'max', o1/o3/o4 above 'high') -
|
||||
# model rejects (e.g. pre-5.6 GPT-5 'max', o1/o3/o4 above 'high') -
|
||||
# those paths return a NON-empty clamped set, so the degrade ladder below
|
||||
# still applies. When the set is empty we can't tell "unsupported" from
|
||||
# "unknown", so preserve the user's configured effort verbatim where it is
|
||||
# still valid. (#3505 review)
|
||||
#
|
||||
# EXCEPTION for 'max' (the #3505 default-deny refinement, maintainer call
|
||||
# 2026-07-11): 'max' is a WebUI-only level ABOVE the universally-safe ceiling
|
||||
# 'xhigh'. A genuinely unknown/custom provider will 400 on it. So when the
|
||||
# 2026-07-11): 'max' is ABOVE the universally-safe ceiling 'xhigh'. A
|
||||
# genuinely unknown/custom provider will 400 on it. So when the
|
||||
# capability list is empty AND the provider is not one we recognize as
|
||||
# reasoning-capable, degrade 'max' -> 'xhigh' rather than send an unsupported
|
||||
# supra-ceiling level. But do NOT degrade for a RECOGNIZED reasoning provider
|
||||
@@ -9146,6 +9165,56 @@ LAST_RUN_FINISHED_AT: float | None = None
|
||||
SERVER_START_TIME = time.time()
|
||||
|
||||
|
||||
def active_run_is_attachable(run_entry) -> bool:
|
||||
"""Return whether a run row still represents renderable live work.
|
||||
|
||||
``ACTIVE_RUNS`` tracks WORKER LIFECYCLE, which is deliberately broader than
|
||||
"a turn a browser may attach to": ``cancel_stream()`` leaves the row in
|
||||
``phase="cancelling"`` while the worker unwinds so a successor cannot start
|
||||
on top of it. That row is already terminal from the client's perspective —
|
||||
its run journal ends in a terminal event — so recovery paths that hand a
|
||||
stream id to the renderer must exclude it. Otherwise every fresh
|
||||
``/api/session/stream`` subscription replays ``server_turn_started`` for a
|
||||
cancelled run, the client attaches, consumes the terminal event, tears the
|
||||
renderer down and resubscribes, and the loop repeats indefinitely.
|
||||
|
||||
Non-dict entries stay attachable so callers that store an opaque marker are
|
||||
unaffected; production registrations are dicts carrying ``phase``.
|
||||
"""
|
||||
return not (
|
||||
isinstance(run_entry, dict)
|
||||
and str(run_entry.get("phase") or "").strip() == "cancelling"
|
||||
)
|
||||
|
||||
|
||||
def active_run_cancel_is_stale(
|
||||
run_entry,
|
||||
*,
|
||||
grace_seconds: float,
|
||||
now: float | None = None,
|
||||
) -> bool:
|
||||
"""Return whether a cancelling worker outlived its bounded unwind window.
|
||||
|
||||
The age anchor is ``cancelled_at`` rather than the original ``started_at``
|
||||
so a long-running turn that was just cancelled is never mistaken for an
|
||||
orphan; ``started_at`` remains the fallback for rows created before the
|
||||
cancellation timestamp existed. Callers own the grace window because the
|
||||
tolerated unwind differs per surface.
|
||||
"""
|
||||
if not isinstance(run_entry, dict):
|
||||
return False
|
||||
if str(run_entry.get("phase") or "").strip() != "cancelling":
|
||||
return False
|
||||
anchor = run_entry.get("cancelled_at") or run_entry.get("started_at")
|
||||
if not anchor:
|
||||
return False
|
||||
try:
|
||||
age = (time.time() if now is None else float(now)) - float(anchor)
|
||||
return age >= float(grace_seconds)
|
||||
except (TypeError, ValueError):
|
||||
return False
|
||||
|
||||
|
||||
def register_active_run(stream_id: str, **metadata) -> None:
|
||||
"""Mark a WebUI agent worker as alive until its outer finally exits."""
|
||||
if not stream_id:
|
||||
|
||||
+7
-8
@@ -2934,16 +2934,15 @@ def _cancelled_run_is_stale(run_entry) -> bool:
|
||||
phase="cancelling". ``started_at`` is accepted as a fallback anchor so runs
|
||||
cancelled before the stamp was introduced are still reclaimed eventually.
|
||||
"""
|
||||
if not isinstance(run_entry, dict) or run_entry.get("phase") != "cancelling":
|
||||
return False
|
||||
anchor = run_entry.get("cancelled_at") or run_entry.get("started_at")
|
||||
if not anchor:
|
||||
return False
|
||||
try:
|
||||
age = time.time() - float(anchor)
|
||||
except (TypeError, ValueError):
|
||||
from api import config as _live_config
|
||||
|
||||
return _live_config.active_run_cancel_is_stale(
|
||||
run_entry,
|
||||
grace_seconds=_STALE_CANCELLED_RUN_GRACE_SECONDS,
|
||||
)
|
||||
except Exception:
|
||||
return False
|
||||
return age >= _STALE_CANCELLED_RUN_GRACE_SECONDS
|
||||
|
||||
|
||||
def _clear_stale_stream_state(session) -> bool:
|
||||
|
||||
+11
-4
@@ -30,19 +30,26 @@ def _live_active_stream_id(session) -> str | None:
|
||||
the hidden-tab poller) would make a client attach its renderer to a stream
|
||||
that never emits — a permanent fake "thinking" state. Liveness test mirrors
|
||||
routes._clear_stale_stream_state: live iff present in STREAMS (open SSE
|
||||
channel) or ACTIVE_RUNS (worker bookkeeping).
|
||||
channel) or ACTIVE_RUNS (worker bookkeeping) — except that a
|
||||
``phase="cancelling"`` run is excluded on both paths, because the worker may
|
||||
still be unwinding while the client already reached a terminal state for
|
||||
that stream.
|
||||
"""
|
||||
stream_id = getattr(session, 'active_stream_id', None)
|
||||
if not stream_id:
|
||||
return None
|
||||
try:
|
||||
from api import config as _cfg
|
||||
with _cfg.ACTIVE_RUNS_LOCK:
|
||||
_active_run_present = stream_id in (_cfg.ACTIVE_RUNS or {})
|
||||
_active_run_entry = (_cfg.ACTIVE_RUNS or {}).get(stream_id)
|
||||
if _active_run_present and not _cfg.active_run_is_attachable(_active_run_entry):
|
||||
return None
|
||||
with _cfg.STREAMS_LOCK:
|
||||
if stream_id in _cfg.STREAMS:
|
||||
return stream_id
|
||||
with _cfg.ACTIVE_RUNS_LOCK:
|
||||
if stream_id in (_cfg.ACTIVE_RUNS or {}):
|
||||
return stream_id
|
||||
if _active_run_present:
|
||||
return stream_id
|
||||
except Exception:
|
||||
# On any introspection failure, fail SAFE (report no live stream) rather
|
||||
# than surfacing a possibly-stale id.
|
||||
|
||||
@@ -70,6 +70,7 @@ and 5; it does not mark every run-state boundary implemented.
|
||||
| Model context / `context_messages` | Supplies conversation state to the agent | Must include the current visible user turn unless deliberately excluded with a user-visible reason | Let the agent resume from context that contradicts what the user can see |
|
||||
| Pending turn metadata | Bridges submitted-but-not-yet-finalized user input | Must identify the user turn and stream that own active work | Become a permanent duplicate transcript row after recovery |
|
||||
| Live stream / SSE | Delivers active runtime events to the browser | Must remain an observation path, not the only durable truth for already-emitted events | Lose the visible scene on refresh, reconnect, or session switch |
|
||||
| Worker lifecycle registry (`ACTIVE_RUNS`) | Tracks whether a worker still occupies the session, so a successor turn cannot start on top of it | Broader than "attachable UI work": a cancelled worker stays registered while it unwinds | Be read directly as the set of runs a browser may attach to |
|
||||
| Run journal / replay | Rebuilds emitted runtime events after reconnect or restart | Must be cursor-safe and idempotent | Duplicate assistant text, thinking text, tool cards, or compression cards |
|
||||
| Compression summary / handoff | Gives the agent recovery context after automatic compression | Must remain agent-facing recovery material unless explicitly rendered as history | Pollute the active turn or become implicit current user intent |
|
||||
| Live UI scene/cache | Preserves expanded rows, in-progress cards, local scroll, and transient grouping | May optimize presentation but must be rebuildable or degradable from transcript/replay | Become the only place where chronological ordering exists |
|
||||
@@ -121,6 +122,26 @@ and 5; it does not mark every run-state boundary implemented.
|
||||
8. **Every mutation names its layer.** A PR touching streaming, recovery,
|
||||
context reconstruction, compression, replay, or sidebar metadata should state
|
||||
which layer it changes and what regression proves the invariant still holds.
|
||||
9. **Lifecycle-busy is not client-attachable.** `ACTIVE_RUNS` answers "may a new
|
||||
turn start?", not "may a browser attach a renderer?". Cancellation splits the
|
||||
two: `cancel_stream()` keeps the row as `phase="cancelling"` so a successor
|
||||
cannot overlap the unwinding worker, but the client has already reached a
|
||||
terminal state for that stream because its run journal ends in a terminal
|
||||
event. Recovery paths that hand a stream id to a renderer — session SSE
|
||||
recovery and hidden-tab status polling — must therefore exclude cancelling
|
||||
rows, while busy/admission checks must keep counting them. Reading the
|
||||
registry with a single meaning resurrects a cancelled run on every fresh
|
||||
subscription: the client attaches, consumes the terminal event, tears the
|
||||
renderer down, resubscribes, and the loop repeats indefinitely.
|
||||
|
||||
Because a cancelling row can otherwise persist forever, cancellation unwind is
|
||||
bounded: a cancelling row older than that window **and** owning no live
|
||||
`STREAMS` channel is reclaimed from `ACTIVE_RUNS` along with its stream-owner
|
||||
entry, so a wedged worker cannot suppress background wakeups permanently.
|
||||
Reclamation requires both conditions — age alone must not evict a row that
|
||||
still owns a live channel. Staleness is measured from the cancellation
|
||||
timestamp (falling back to run start), so a long-running turn cancelled
|
||||
moments ago is never mistaken for an orphan.
|
||||
|
||||
## Review Checklist
|
||||
|
||||
@@ -138,6 +159,11 @@ context reconstruction, or session metadata:
|
||||
interim assistant text, tool cards, compression cards, and terminal states?
|
||||
- Can this change move a session in the sidebar without meaningful user or
|
||||
assistant activity?
|
||||
- Does this change read `ACTIVE_RUNS` for admission ("may a turn start?") or for
|
||||
attachment ("may a browser render this?"), and does it use the matching
|
||||
predicate for that question?
|
||||
- If it introduces or changes a reclamation window, what proves an in-flight
|
||||
cancellation is not evicted early, and that a wedged one is eventually freed?
|
||||
- Can automatic compression or recovery text become visible active-turn content?
|
||||
- What test or manual evidence proves the invariant?
|
||||
|
||||
|
||||
@@ -0,0 +1,178 @@
|
||||
"""A cancelling run is lifecycle-busy but NOT attachable live UI work.
|
||||
|
||||
``ACTIVE_RUNS`` tracks worker lifecycle. ``cancel_stream()`` deliberately keeps
|
||||
the row as ``phase="cancelling"`` while the worker unwinds so a successor turn
|
||||
cannot start on top of it. The client, however, has already reached a terminal
|
||||
state for that stream: its run journal ends in a terminal event.
|
||||
|
||||
Before this fix the recovery lookups treated every same-session ``ACTIVE_RUNS``
|
||||
row as attachable, so an idle session whose sidecar had already cleared
|
||||
``active_stream_id`` still received a recovered ``server_turn_started`` for the
|
||||
cancelled stream on EVERY ``/api/session/stream`` subscription. The client
|
||||
attached, replayed the terminal event, tore the renderer down, resubscribed, and
|
||||
the server replayed the same frame again — an endless attach/replay loop.
|
||||
|
||||
These tests pin both directions of the resulting contract:
|
||||
|
||||
* browser recovery excludes cancelling rows, while busy checks still keep a
|
||||
FRESH cancellation busy (a successor must not overlap the unwinding worker);
|
||||
* a cancelling row past the bounded unwind window with no live ``STREAMS``
|
||||
channel is reclaimed, so a wedged worker cannot suppress wakeups forever;
|
||||
* a cancelling row that still owns a live ``STREAMS`` channel is NOT reclaimed
|
||||
on age alone.
|
||||
"""
|
||||
|
||||
import sys
|
||||
import time
|
||||
from pathlib import Path
|
||||
from types import SimpleNamespace
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parents[1]
|
||||
sys.path.insert(0, str(REPO_ROOT))
|
||||
|
||||
from api import background_process as bp # noqa: E402
|
||||
from api import config as cfg # noqa: E402
|
||||
from api.session_ops import _live_active_stream_id # noqa: E402
|
||||
|
||||
|
||||
def _clear(stream_id: str) -> None:
|
||||
cfg.unregister_active_run(stream_id)
|
||||
with cfg.STREAMS_LOCK:
|
||||
cfg.STREAMS.pop(stream_id, None)
|
||||
|
||||
|
||||
def test_running_run_is_still_attachable_and_busy():
|
||||
"""Control: an ordinary running turn keeps both semantics unchanged."""
|
||||
sid = "sess-running-control"
|
||||
stream_id = "stream-running-control"
|
||||
cfg.register_active_run(stream_id, session_id=sid, phase="running")
|
||||
try:
|
||||
assert bp.active_stream_id_for_session(sid) == stream_id
|
||||
assert bp._session_has_active_turn(sid) is True
|
||||
assert _live_active_stream_id(
|
||||
SimpleNamespace(session_id=sid, active_stream_id=stream_id)
|
||||
) == stream_id
|
||||
finally:
|
||||
_clear(stream_id)
|
||||
|
||||
|
||||
def test_fresh_cancelling_run_is_not_attachable_but_stays_busy():
|
||||
"""The loop-producing case: recovery must not replay a cancelled run.
|
||||
|
||||
The same row must still count as busy so a successor cannot start while the
|
||||
cancelled worker is unwinding.
|
||||
"""
|
||||
sid = "sess-cancelling-fresh"
|
||||
stream_id = "stream-cancelling-fresh"
|
||||
cfg.register_active_run(
|
||||
stream_id,
|
||||
session_id=sid,
|
||||
phase="cancelling",
|
||||
cancelled_at=time.time() - 5.0,
|
||||
)
|
||||
try:
|
||||
assert bp.active_stream_id_for_session(sid) is None
|
||||
assert bp._session_has_active_turn(sid) is True
|
||||
assert stream_id in cfg.ACTIVE_RUNS
|
||||
finally:
|
||||
_clear(stream_id)
|
||||
|
||||
|
||||
def test_hidden_tab_status_does_not_resurrect_a_cancelling_run():
|
||||
"""/api/session/status must not hand a cancelling stream to the poller."""
|
||||
sid = "sess-cancelling-status"
|
||||
stream_id = "stream-cancelling-status"
|
||||
cfg.register_active_run(stream_id, session_id=sid, phase="cancelling")
|
||||
try:
|
||||
assert _live_active_stream_id(
|
||||
SimpleNamespace(session_id=sid, active_stream_id=stream_id)
|
||||
) is None
|
||||
finally:
|
||||
_clear(stream_id)
|
||||
|
||||
|
||||
def test_hidden_tab_status_ignores_cancelling_run_with_live_stream_channel():
|
||||
"""A cancelling row is terminal for the client even if STREAMS still holds it."""
|
||||
sid = "sess-cancelling-status-live-channel"
|
||||
stream_id = "stream-cancelling-status-live-channel"
|
||||
cfg.register_active_run(stream_id, session_id=sid, phase="cancelling")
|
||||
with cfg.STREAMS_LOCK:
|
||||
cfg.STREAMS[stream_id] = object()
|
||||
try:
|
||||
assert _live_active_stream_id(
|
||||
SimpleNamespace(session_id=sid, active_stream_id=stream_id)
|
||||
) is None
|
||||
finally:
|
||||
_clear(stream_id)
|
||||
|
||||
|
||||
def test_stale_cancelling_orphan_stops_blocking_and_is_reclaimed():
|
||||
"""Past the unwind window with no live channel, the row is an orphan."""
|
||||
sid = "sess-cancelling-stale"
|
||||
stream_id = "stream-cancelling-stale"
|
||||
cfg.register_active_run(
|
||||
stream_id,
|
||||
session_id=sid,
|
||||
phase="cancelling",
|
||||
cancelled_at=time.time() - 240.0,
|
||||
)
|
||||
cfg.register_stream_owner(stream_id, sid)
|
||||
try:
|
||||
assert bp._session_has_active_turn(sid) is False
|
||||
assert stream_id not in cfg.ACTIVE_RUNS
|
||||
assert cfg.stream_owner_session_id(stream_id) is None
|
||||
finally:
|
||||
_clear(stream_id)
|
||||
|
||||
|
||||
def test_stale_cancelling_run_with_live_channel_is_not_reclaimed():
|
||||
"""Reverse control: age alone must not reap a row that still owns a channel."""
|
||||
sid = "sess-cancelling-stale-live-channel"
|
||||
stream_id = "stream-cancelling-stale-live-channel"
|
||||
cfg.register_active_run(
|
||||
stream_id,
|
||||
session_id=sid,
|
||||
phase="cancelling",
|
||||
cancelled_at=time.time() - 240.0,
|
||||
)
|
||||
with cfg.STREAMS_LOCK:
|
||||
cfg.STREAMS[stream_id] = object()
|
||||
try:
|
||||
assert bp._session_has_active_turn(sid) is True
|
||||
assert stream_id in cfg.ACTIVE_RUNS
|
||||
finally:
|
||||
_clear(stream_id)
|
||||
|
||||
|
||||
def test_cancel_staleness_anchors_on_cancel_time_not_run_start():
|
||||
"""A long-running turn cancelled just now must not read as stale."""
|
||||
entry = {
|
||||
"session_id": "sess-anchor",
|
||||
"phase": "cancelling",
|
||||
"started_at": time.time() - 4000.0,
|
||||
"cancelled_at": time.time() - 5.0,
|
||||
}
|
||||
assert cfg.active_run_cancel_is_stale(entry, grace_seconds=180.0) is False
|
||||
|
||||
legacy = {
|
||||
"session_id": "sess-anchor-legacy",
|
||||
"phase": "cancelling",
|
||||
"started_at": time.time() - 4000.0,
|
||||
}
|
||||
assert cfg.active_run_cancel_is_stale(legacy, grace_seconds=180.0) is True
|
||||
|
||||
running = {
|
||||
"session_id": "sess-anchor-running",
|
||||
"phase": "running",
|
||||
"started_at": time.time() - 4000.0,
|
||||
}
|
||||
assert cfg.active_run_cancel_is_stale(running, grace_seconds=180.0) is False
|
||||
|
||||
|
||||
def test_attachability_predicate_edges():
|
||||
"""Non-dict/opaque entries stay attachable; only cancelling is excluded."""
|
||||
assert cfg.active_run_is_attachable({"phase": "running"}) is True
|
||||
assert cfg.active_run_is_attachable({"phase": "cancelling"}) is False
|
||||
assert cfg.active_run_is_attachable({"phase": " cancelling "}) is False
|
||||
assert cfg.active_run_is_attachable({}) is True
|
||||
assert cfg.active_run_is_attachable(object()) is True
|
||||
@@ -197,6 +197,20 @@ def test_glm_5_3_in_models_payload_for_zai_provider(tmp_path, monkeypatch):
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
# Pin the agent-core catalog: get_available_models() sources the zai
|
||||
# model list from the INSTALLED hermes-cli core (via
|
||||
# _read_live_provider_model_ids -> provider_model_ids), not from the
|
||||
# repo's static _PROVIDER_MODELS. On a box whose installed core predates
|
||||
# glm-5.3 this test previously failed spuriously ("glm-5.3 missing from
|
||||
# zai group models"). Stub the core catalog to a deterministic empty
|
||||
# list so the repo's own _PROVIDER_MODELS fallback is exercised — this
|
||||
# tests WebUI catalog propagation, not the installed core version.
|
||||
try:
|
||||
import hermes_cli.models as hm
|
||||
monkeypatch.setattr(hm, "provider_model_ids", lambda _pid: [])
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
result = c.get_available_models()
|
||||
c.reload_config()
|
||||
|
||||
|
||||
@@ -3,6 +3,23 @@
|
||||
from api import config as cfg
|
||||
|
||||
|
||||
GPT_5_6_MODELS = (
|
||||
"gpt-5.6",
|
||||
"gpt-5.6-sol",
|
||||
"gpt-5.6-terra",
|
||||
"gpt-5.6-luna",
|
||||
)
|
||||
|
||||
OPENAI_FAMILY_PROVIDERS = (
|
||||
"openai",
|
||||
"openai-api",
|
||||
"openai-codex",
|
||||
"azure",
|
||||
"azure-openai",
|
||||
"azure-foundry",
|
||||
)
|
||||
|
||||
|
||||
def test_cursor_acp_models_do_not_support_reasoning_effort_levels():
|
||||
assert cfg.resolve_model_reasoning_efforts(
|
||||
"cursor/composer-2.5",
|
||||
@@ -40,6 +57,21 @@ def test_openai_codex_max_effort_is_clamped_before_streaming():
|
||||
) == "xhigh"
|
||||
|
||||
|
||||
def test_openai_family_gpt56_models_expose_and_preserve_max():
|
||||
for provider in OPENAI_FAMILY_PROVIDERS:
|
||||
for model in GPT_5_6_MODELS:
|
||||
efforts = cfg.resolve_model_reasoning_efforts(
|
||||
f"@{provider}:{model}",
|
||||
provider_id=provider,
|
||||
)
|
||||
assert "max" in efforts, f"{model} on {provider} must expose max"
|
||||
assert cfg.coerce_reasoning_effort_for_model(
|
||||
"max",
|
||||
f"@{provider}:{model}",
|
||||
provider_id=provider,
|
||||
) == "max", f"{model} on {provider} must preserve max"
|
||||
|
||||
|
||||
def test_unsupported_xhigh_degrades_to_high_not_disabled():
|
||||
# o1/o3/o4 on openai-codex cap at low/medium/high. A configured xhigh (or
|
||||
# max) must clamp DOWN to the highest supported level (high), not silently
|
||||
@@ -79,8 +111,8 @@ def test_coerce_preserves_effort_for_unrecognized_model():
|
||||
"some-unknown-model-xyz",
|
||||
provider_id="some-custom-provider",
|
||||
) == "high"
|
||||
# #3505 default-deny refinement (maintainer 2026-07-11): 'max' is a supra-
|
||||
# ceiling WebUI-only level, so on an UNRECOGNIZED provider it degrades to
|
||||
# #3505 default-deny refinement (maintainer 2026-07-11): 'max' is above the
|
||||
# universally safe ceiling, so on an UNRECOGNIZED provider it degrades to
|
||||
# xhigh (a truly-unknown provider would 400 on max). All OTHER levels still
|
||||
# preserve verbatim below.
|
||||
assert cfg.coerce_reasoning_effort_for_model(
|
||||
@@ -434,16 +466,18 @@ def test_max_effort_preserved_for_adaptive_anthropic_and_deepseek():
|
||||
) == "max"
|
||||
|
||||
|
||||
def test_max_degrades_across_all_openai_family_lanes():
|
||||
# 'max' is WebUI-only; direct OpenAI, openai-api, and Azure Foundry GPT-5 all
|
||||
# cap at xhigh (o-series at high), not just openai-codex. (#4627 re-gate)
|
||||
for prov in ("openai", "openai-api", "azure-foundry", "openai-codex"):
|
||||
assert cfg.coerce_reasoning_effort_for_model(
|
||||
"max", model_id="gpt-5.1", provider_id=prov
|
||||
) == "xhigh", f"gpt-5 on {prov} must degrade max->xhigh"
|
||||
assert cfg.coerce_reasoning_effort_for_model(
|
||||
"max", model_id="o3", provider_id=prov
|
||||
) == "high", f"o-series on {prov} must degrade max->high"
|
||||
def test_max_degrades_for_pre_gpt56_and_o_series_across_openai_family_lanes():
|
||||
# GPT-5 models before 5.6 cap at xhigh, while o1/o3/o4 cap at high, across
|
||||
# direct OpenAI, ChatGPT/Codex, and Azure provider aliases.
|
||||
for provider in OPENAI_FAMILY_PROVIDERS:
|
||||
for model in ("gpt-5", "gpt-5.1", "gpt-5.5"):
|
||||
assert cfg.coerce_reasoning_effort_for_model(
|
||||
"max", model_id=model, provider_id=provider
|
||||
) == "xhigh", f"{model} on {provider} must degrade max->xhigh"
|
||||
for model in ("o1", "o3-mini", "o4-mini"):
|
||||
assert cfg.coerce_reasoning_effort_for_model(
|
||||
"max", model_id=model, provider_id=provider
|
||||
) == "high", f"{model} on {provider} must degrade max->high"
|
||||
|
||||
|
||||
def test_max_degrades_for_azure_bedrock_hosted_legacy_claude():
|
||||
@@ -461,7 +495,7 @@ def test_max_degrades_for_azure_bedrock_hosted_legacy_claude():
|
||||
|
||||
def test_max_degrades_on_unknown_provider_but_other_levels_preserved():
|
||||
# #3505 default-deny refinement (maintainer call 2026-07-11): 'max' is above
|
||||
# the universal ceiling, so an unknown/custom provider (empty capability list)
|
||||
# the universally safe ceiling, so an unknown/custom provider (empty capability list)
|
||||
# must degrade 'max'->'xhigh' rather than send an unsupported level — while all
|
||||
# other levels keep the conservative preserve-verbatim behavior.
|
||||
assert cfg.coerce_reasoning_effort_for_model(
|
||||
@@ -560,4 +594,3 @@ def test_qwen_prefixed_alias_reasoning_detection():
|
||||
f"{model} must remain reasoning-capable (DeepSeek-R1 hybrid, "
|
||||
f"Qwen 2.x must not shadow the DeepSeek detector)"
|
||||
)
|
||||
|
||||
+48
-26
@@ -111,39 +111,61 @@ def test_ttl_expiry():
|
||||
def test_mtime_invalidation():
|
||||
"""Populate the cache, then change _cfg_mtime to simulate a config file
|
||||
change on disk. The next call should invalidate the cache and re-scan.
|
||||
|
||||
This test is fully self-contained: it invalidates both memory and disk
|
||||
caches first, then populates the cache via a priming call, then verifies
|
||||
that an mtime mismatch triggers a cache rebuild. It does not depend on
|
||||
any sibling test's execution order.
|
||||
"""
|
||||
_reset_cache()
|
||||
# Fully invalidate (memory + disk) so we start from a known clean state.
|
||||
config.invalidate_models_cache()
|
||||
|
||||
# Force the synchronous (unbounded) rebuild path so the priming call is
|
||||
# guaranteed to have published the cache when it returns. With the
|
||||
# default bounded budget (_LIVE_REBUILD_BUDGET_SECONDS=4.0), a cold
|
||||
# rebuild on a slow/loaded box can exceed the budget and return a
|
||||
# static fallback WITHOUT populating _available_models_cache — which
|
||||
# made this test fail when run in isolation (it previously only passed
|
||||
# because a sibling test had already warmed the cache in-process).
|
||||
original_budget = config._LIVE_REBUILD_BUDGET_SECONDS
|
||||
config._LIVE_REBUILD_BUDGET_SECONDS = 0.0
|
||||
real_mtime = 0.0
|
||||
|
||||
# Ensure _cfg_mtime matches file so first call doesn't re-scan due to mtime
|
||||
try:
|
||||
real_mtime = config.Path(config._get_config_path()).stat().st_mtime
|
||||
except OSError:
|
||||
real_mtime = 0.0
|
||||
config._cfg_mtime = real_mtime
|
||||
# Ensure _cfg_mtime matches file so first call doesn't re-scan due to mtime
|
||||
try:
|
||||
real_mtime = config.Path(config._get_config_path()).stat().st_mtime
|
||||
except OSError:
|
||||
real_mtime = 0.0
|
||||
config._cfg_mtime = real_mtime
|
||||
|
||||
# First call populates cache
|
||||
result1 = config.get_available_models()
|
||||
assert config._available_models_cache is not None
|
||||
# Priming call: populate the cache (both memory and disk)
|
||||
result1 = config.get_available_models()
|
||||
assert config._available_models_cache is not None, (
|
||||
"Priming call should populate the in-memory cache"
|
||||
)
|
||||
assert config._available_models_cache_ts > 0.0, (
|
||||
"Priming call should set a valid cache timestamp"
|
||||
)
|
||||
|
||||
# Simulate config.yaml changed on disk by setting _cfg_mtime to 0
|
||||
# (which won't match the actual file mtime)
|
||||
config._cfg_mtime = 0.0
|
||||
# Simulate config.yaml changed on disk by setting _cfg_mtime to 0
|
||||
# (which won't match the actual file mtime)
|
||||
config._cfg_mtime = 0.0
|
||||
|
||||
# The next call should detect mtime mismatch, reload, and invalidate cache
|
||||
old_cache = config._available_models_cache
|
||||
old_ts = config._available_models_cache_ts
|
||||
# The next call should detect mtime mismatch, reload, and invalidate cache
|
||||
result2 = config.get_available_models()
|
||||
|
||||
result2 = config.get_available_models()
|
||||
|
||||
# Cache must have been refreshed — timestamp advanced since we reset it
|
||||
# to 0.0 on invalidation.
|
||||
assert config._available_models_cache_ts > 0.0, (
|
||||
"Cache timestamp should be updated after invalidation + rebuild"
|
||||
)
|
||||
|
||||
# Restore
|
||||
config._cfg_mtime = real_mtime
|
||||
_reset_cache()
|
||||
# Cache must have been refreshed — timestamp advanced since we reset it
|
||||
# to 0.0 on invalidation.
|
||||
assert config._available_models_cache_ts > 0.0, (
|
||||
"Cache timestamp should be updated after invalidation + rebuild"
|
||||
)
|
||||
# Both calls must return a valid /api/models payload.
|
||||
assert "groups" in result1 and "groups" in result2
|
||||
finally:
|
||||
config._LIVE_REBUILD_BUDGET_SECONDS = original_budget
|
||||
config._cfg_mtime = real_mtime
|
||||
config.invalidate_models_cache()
|
||||
|
||||
|
||||
# ── 4. test_deepcopy_isolation ────────────────────────────────────────────
|
||||
|
||||
Reference in new issue
Block a user