Merge remote-tracking branch 'origin/master' into fix/7299-hidden-session-poll
This commit is contained in:
commit
99f22391d7
47 files changed
+5746
-281
No files matched your search
@@ -5,6 +5,90 @@
|
||||
|
||||
### Fixed
|
||||
|
||||
- **A rejected request no longer poisons the next one on the same connection.** `server.py`
|
||||
is a raw HTTP/1.1 handler where `rfile` is the socket itself, so answering a request
|
||||
before reading its body left those bytes queued. The next request on a keep-alive
|
||||
connection was then parsed starting mid-body, and the client got
|
||||
`400 Bad request syntax ('{"a": "b"}GET /api/health HTTP/1.1')` — an error naming a
|
||||
request it never sent, which is expensive to diagnose from the client side. Every
|
||||
reject path now arms `Connection: close` when a body is still pending, and framing is
|
||||
validated strictly (RFC 9110 `1*DIGIT`, duplicate and comma-combined `Content-Length`
|
||||
reconciled, every `Transfer-Encoding` refused since nothing here decodes one). A
|
||||
genuinely bodyless rejection keeps its keep-alive, so healthy pooled connections are
|
||||
not dropped. Thanks @rodrigogs. (#7550, #6658)
|
||||
|
||||
- **Opening a session that belongs to another profile now offers to switch to it instead of
|
||||
looking deleted.** Several cross-profile guards answered `404 Session not found`, which the
|
||||
front-end treats as a missing session and self-heals by clearing the URL and local storage —
|
||||
so a valid deep link into another profile's session destroyed its own way back. Those guards
|
||||
now return the same `409 session_profile_mismatch` the detail-load endpoint has used since
|
||||
#5419, while a genuinely missing session still 404s and still self-heals. The clarify card,
|
||||
compression-recovery card and manual-compression flow were each treating *every* `409` as
|
||||
their own "stale" signal and are now scoped so a cross-profile refusal no longer hides a live
|
||||
prompt or reports a false compression failure. Thanks @happy5318. (#7710, #7714)
|
||||
|
||||
- **An OIDC-only or passkey-only deployment no longer shows a dead password prompt.** The
|
||||
`/login` page rendered the password input, submit button and passkey control
|
||||
unconditionally, so an instance with native OIDC configured and no
|
||||
`HERMES_WEBUI_PASSWORD` displayed a password form that silently 401'd every submit. The
|
||||
controls are now gated on the auth methods actually configured, with the passkey button
|
||||
kept for passwordless-passkey instances where it is the only login affordance. Thanks
|
||||
@happy5318. (#7056, #7715)
|
||||
|
||||
- **A model whose native id contains a slash no longer poisons its configured-model badge.**
|
||||
The badge builder synthesised a `{provider}/{model}` alias alongside the bare id, so a
|
||||
model already carrying a slash (for example `commandcode` + `deepseek/deepseek-v4-flash`)
|
||||
produced `commandcode/deepseek/deepseek-v4-flash` — a non-functional id that leaked into
|
||||
the badge map, persisted into session state and returned `HTTP 400` from the agent. Only
|
||||
the bare id and the `@provider:model` form are emitted now. Thanks @happy5318. (#7290, #7709)
|
||||
|
||||
- **A conversation whose history carries an explicit `null` tool-call list no longer breaks
|
||||
the tool-call summary.** `_extract_tool_calls_from_messages` used `.get('tool_calls', [])`,
|
||||
which returns the default only when the key is *absent* — when a stored message carried the
|
||||
key with a `null` value, the code tried to iterate `None` and raised `TypeError`. Reading
|
||||
such a session now skips the empty entry and still summarises the tool calls that follow it.
|
||||
Thanks @KayZz69. (#7265)
|
||||
|
||||
- **The opencode-go provider lists its real models again instead of a frozen snapshot.** Model
|
||||
discovery had fallen back to a hard-coded catalog, so models added or removed upstream never
|
||||
appeared. The provider now queries live, with the lookup scoped to opencode-go alone: a slow
|
||||
or unreachable endpoint falls back to the static catalog rather than blocking the picker for
|
||||
anyone else, older Agent builds that lack the discovery API keep the previous behaviour, and a
|
||||
configured model allowlist still wins over whatever discovery returns. Thanks
|
||||
@shameez-struggles-to-commit. (#7220)
|
||||
|
||||
- **Denying a gateway approval now retires the request instead of leaving the run waiting.**
|
||||
When an approval routed through the gateway was denied, the deny reached the agent but the
|
||||
local producer was never retired, so the run could sit waiting on a decision that had already
|
||||
been made. Denial now settles the producer atomically, scoped to the exact
|
||||
`(session_id, run_id, approval_id)` that was answered — sibling approvals from the same run
|
||||
and approvals from other runs are left untouched — and repeated or late responses are bounded
|
||||
no-ops rather than resurrecting a settled request. Thanks @snoyberg. (#7570)
|
||||
|
||||
- **The approval card's "Skip all this session" button no longer shows two lightning bolts.**
|
||||
The button rendered its ⚡ twice — once from the icon span in the markup and again from the
|
||||
translated label, which carried its own leading glyph in 14 of 15 locales. The icon now comes
|
||||
only from the markup, matching every sibling button on the card (Allow once / Allow session /
|
||||
Always allow / Deny all pair an icon element with a glyph-free label), and translators no
|
||||
longer carry the symbol in their strings. (#7701)
|
||||
|
||||
- **The gateway watcher no longer polls `state.db` around the clock with nobody listening.**
|
||||
Its poll loop re-fingerprinted the gateway state database every few seconds whether or not
|
||||
any SSE client was attached, and slept in 0.1s increments — roughly 10 wakeups a second, all
|
||||
day, on an idle server. The loop now parks on an event when there are no subscribers and is
|
||||
woken by the first one, so an idle instance does no polling work at all; while subscribed it
|
||||
waits on a single timer that still returns immediately on shutdown. Connecting a client
|
||||
remains prompt — the first subscriber unparks the loop rather than waiting out the poll
|
||||
interval. Thanks @DevNexsler. (#7694)
|
||||
|
||||
- **A long-running turn no longer replays a stale token count after a reload.** The run-journal
|
||||
recorded live metering frames, so reattaching to a stream — or reloading a tab mid-turn —
|
||||
could replay a snapshot from earlier in the same run and briefly show token/TPS figures that
|
||||
had already been superseded. Metering is now live-only and never journaled, and replayed
|
||||
frames no longer carry an event id that could advance the client's resume cursor past real
|
||||
content. Reattach and reload now show the current numbers for the turn in progress. Thanks
|
||||
@laitekin. (#7291)
|
||||
|
||||
- **Steering a conversation works again after the context is compressed.** When compression
|
||||
rotated `agent.session_id`, a steer could no longer find the active worker: it was either
|
||||
silently dropped or accepted and never delivered. Steers now resolve the owning worker
|
||||
|
||||
+37
@@ -17,6 +17,7 @@ import time
|
||||
from pathlib import Path
|
||||
|
||||
from api.config import STATE_DIR, get_config, load_settings
|
||||
from api.helpers import request_declares_body
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -1207,6 +1208,42 @@ def check_auth(handler, parsed) -> bool:
|
||||
return False
|
||||
|
||||
|
||||
def check_auth_or_close(handler, parsed) -> bool:
|
||||
"""Check auth; when rejected, close so an unread body can't poison HTTP/1.1 reuse.
|
||||
|
||||
The flag is armed BEFORE check_auth() writes its 401/302, so end_headers()
|
||||
can advertise ``Connection: close``; success restores the prior flag so an
|
||||
authenticated request keeps its keep-alive.
|
||||
|
||||
Armed ONLY when the request's framing declares a body still queued in
|
||||
``rfile`` (see ``request_declares_body()``), which is the same rule the other
|
||||
reject-before-read sites apply through ``arm_connection_close_if_body_pending()``.
|
||||
Arming unconditionally was the mirror image of the over-close this PR already
|
||||
fixed on the sidecar path: a body-less POST that failed auth answered
|
||||
``401`` WITH ``Connection: close`` and dropped the client's pipelined
|
||||
follow-up, killing a healthy keep-alive connection for no framing reason.
|
||||
Verified on the wire, pipelined down one socket against the production
|
||||
handler: ``POST /api/session/new`` with no ``Content-Length`` (and with
|
||||
``Content-Length: 0``) answered ``401`` + ``Connection: close`` and the
|
||||
following ``GET /api/auth/status`` was never served. The helper cannot be
|
||||
swapped in directly here because the arming has to happen before
|
||||
``check_auth()`` writes its response and be undone if it succeeds.
|
||||
|
||||
``close_connection`` only exists once BaseHTTPRequestHandler has parsed a
|
||||
request line, so partially-built handler stubs may not have it at all. Read
|
||||
it defensively and only arm/restore when it was really there: inventing the
|
||||
attribute on a stub would leave a bogus keep-alive verdict behind.
|
||||
"""
|
||||
if not hasattr(handler, 'close_connection') or not request_declares_body(handler):
|
||||
return check_auth(handler, parsed)
|
||||
prior_close = handler.close_connection
|
||||
handler.close_connection = True
|
||||
if check_auth(handler, parsed):
|
||||
handler.close_connection = prior_close
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def _is_loopback(addr: str) -> bool:
|
||||
"""Return True if *addr* is a loopback address (127.x.x.x, ::1, or ::ffff:127.x.x.x)."""
|
||||
import ipaddress as _ipaddress
|
||||
|
||||
+68
-31
@@ -1904,28 +1904,44 @@ _PROVIDER_MODELS = {
|
||||
{"id": "big-pickle", "label": "Big Pickle"},
|
||||
],
|
||||
# OpenCode Go — flat-rate models via opencode.ai/go ($10/month).
|
||||
# Synced 2026-07-08 from the public Go docs and documented models endpoint.
|
||||
# Keep preview/free-only Zen models out of this Go picker snapshot.
|
||||
# Fallback only: the live Hermes CLI catalog (Go-specific
|
||||
# /zen/go/v1/models probe, core v0.20.5+) leads (#1240, #5311).
|
||||
# Mirrors Hermes core's curated opencode-go list
|
||||
# (hermes_cli/models_catalog_static.py, core main as of 2026-09-10).
|
||||
# Core's 2026-09-09 sync dropped `ox-alpha-free` (Go relay delisted it:
|
||||
# GET /zen/go/v1/models omits it, POST → 401) and added `glm-5.3-flash`
|
||||
# and `muse-spark-1.3-contributor`. Core owns the sync duty against the
|
||||
# live endpoint; WebUI mirrors.
|
||||
"opencode-go": [
|
||||
{"id": "minimax-m3", "label": "MiniMax M3"},
|
||||
{"id": "minimax-m2.7", "label": "MiniMax M2.7"},
|
||||
{"id": "minimax-m2.5", "label": "MiniMax M2.5"},
|
||||
{"id": "kimi-k2.7-code", "label": "Kimi K2.7 Code"},
|
||||
{"id": "kimi-k2.6", "label": "Kimi K2.6"},
|
||||
{"id": "kimi-k2.5", "label": "Kimi K2.5"},
|
||||
{"id": "glm-5.2", "label": "GLM-5.2"},
|
||||
{"id": "glm-5.1", "label": "GLM-5.1"},
|
||||
{"id": "glm-5", "label": "GLM-5"},
|
||||
{"id": "deepseek-v4-pro", "label": "DeepSeek V4 Pro"},
|
||||
{"id": "deepseek-v4-flash","label": "DeepSeek V4 Flash"},
|
||||
{"id": "qwen3.7-max", "label": "Qwen3.7 Max"},
|
||||
{"id": "qwen3.7-plus", "label": "Qwen3.7 Plus"},
|
||||
{"id": "qwen3.6-plus", "label": "Qwen3.6 Plus"},
|
||||
{"id": "qwen3.5-plus", "label": "Qwen3.5 Plus"},
|
||||
{"id": "mimo-v2-pro", "label": "MiMo V2 Pro"},
|
||||
{"id": "mimo-v2-omni", "label": "MiMo V2 Omni"},
|
||||
{"id": "mimo-v2.5-pro", "label": "MiMo V2.5 Pro"},
|
||||
{"id": "mimo-v2.5", "label": "MiMo V2.5"},
|
||||
{"id": "kimi-k3", "label": "Kimi K3"},
|
||||
{"id": "kimi-k2.7-code", "label": "Kimi K2.7 Code"},
|
||||
{"id": "kimi-k2.6", "label": "Kimi K2.6"},
|
||||
{"id": "kimi-k2.5", "label": "Kimi K2.5"},
|
||||
{"id": "gpt-5.6-luna", "label": "GPT 5.6 Luna"},
|
||||
{"id": "grok-4.5", "label": "Grok 4.5"},
|
||||
{"id": "glm-5.3", "label": "GLM-5.3"},
|
||||
{"id": "glm-5.3-flash", "label": "GLM-5.3 Flash"},
|
||||
{"id": "glm-5.2", "label": "GLM-5.2"},
|
||||
{"id": "glm-5.1", "label": "GLM-5.1"},
|
||||
{"id": "glm-5", "label": "GLM-5"},
|
||||
{"id": "mimo-v2.5-pro", "label": "MiMo V2.5 Pro"},
|
||||
{"id": "mimo-v2.5", "label": "MiMo V2.5"},
|
||||
{"id": "mimo-v2-pro", "label": "MiMo V2 Pro"},
|
||||
{"id": "mimo-v2-omni", "label": "MiMo V2 Omni"},
|
||||
{"id": "minimax-m3", "label": "MiniMax M3"},
|
||||
{"id": "minimax-m2.7", "label": "MiniMax M2.7"},
|
||||
{"id": "minimax-m2.5", "label": "MiniMax M2.5"},
|
||||
{"id": "deepseek-v4-pro", "label": "DeepSeek V4 Pro"},
|
||||
{"id": "deepseek-v4-flash", "label": "DeepSeek V4 Flash"},
|
||||
{"id": "qwen3.8-max", "label": "Qwen3.8 Max"},
|
||||
{"id": "qwen3.7-max", "label": "Qwen3.7 Max"},
|
||||
{"id": "qwen3.7-plus", "label": "Qwen3.7 Plus"},
|
||||
{"id": "qwen3.6-plus", "label": "Qwen3.6 Plus"},
|
||||
{"id": "qwen3.5-plus", "label": "Qwen3.5 Plus"},
|
||||
{"id": "hy3", "label": "HY3"},
|
||||
{"id": "hy3-preview", "label": "HY3 Preview"},
|
||||
{"id": "muse-spark-1.2-contributor", "label": "Muse Spark 1.2 Contributor"},
|
||||
{"id": "muse-spark-1.3-contributor", "label": "Muse Spark 1.3 Contributor"},
|
||||
],
|
||||
# 'gemini' is the hermes_cli provider ID for Google AI Studio
|
||||
# Model IDs are bare — sent directly to:
|
||||
@@ -6927,8 +6943,8 @@ def _configured_model_badges_from_static_catalog(
|
||||
for entry in configured_entries:
|
||||
provider = entry["provider"]
|
||||
model = entry["model"]
|
||||
raw_candidates = []
|
||||
for candidate in (model, f"{provider}/{model}", f"@{provider}:{model}"):
|
||||
raw_candidates: list[str] = []
|
||||
for candidate in (model, f"@{provider}:{model}"):
|
||||
if candidate and candidate not in raw_candidates:
|
||||
raw_candidates.append(candidate)
|
||||
|
||||
@@ -8374,6 +8390,26 @@ def _read_live_provider_model_ids(provider_id: str) -> list[str]:
|
||||
return []
|
||||
|
||||
|
||||
def _hermes_cli_supports_opencode_go_live_catalog() -> bool:
|
||||
"""Whether the installed Agent has Go-specific model discovery.
|
||||
|
||||
Hermes core versions before 0.20.5 route ``opencode-go`` through a
|
||||
generic public catalog. That lookup can return a convincing non-empty
|
||||
list containing models the Go relay rejects with 404, so absence or an
|
||||
unparseable/prerelease version must fail closed to WebUI's static Go list.
|
||||
"""
|
||||
try:
|
||||
import hermes_cli
|
||||
|
||||
version = str(getattr(hermes_cli, "__version__", "")).strip()
|
||||
except Exception:
|
||||
return False
|
||||
match = re.fullmatch(r"v?(\d+)\.(\d+)\.(\d+)", version)
|
||||
if not match:
|
||||
return False
|
||||
return tuple(int(part) for part in match.groups()) >= (0, 20, 5)
|
||||
|
||||
|
||||
def _models_from_live_provider_ids(provider_id: str, live_ids: list[str]) -> list[dict]:
|
||||
"""Convert Hermes CLI model ids into WebUI picker model entries."""
|
||||
formatter = _format_ollama_label if provider_id in ("ollama", "ollama-cloud") else None
|
||||
@@ -8582,10 +8618,9 @@ def get_available_models(*, prefer_cache: bool = False, force_refresh: bool = Fa
|
||||
for entry in configured_entries:
|
||||
provider = entry["provider"]
|
||||
model = entry["model"]
|
||||
raw_candidates = []
|
||||
raw_candidates: list[str] = []
|
||||
for candidate in (
|
||||
model,
|
||||
f"{provider}/{model}",
|
||||
f"@{provider}:{model}",
|
||||
):
|
||||
if candidate and candidate not in raw_candidates:
|
||||
@@ -9771,12 +9806,14 @@ def get_available_models(*, prefer_cache: bool = False, force_refresh: bool = Fa
|
||||
if not raw_models:
|
||||
if pid == "moa":
|
||||
raw_models = _moa_preset_models_from_config(cfg)
|
||||
elif pid == "opencode-go":
|
||||
# Skip live /v1/models probe for OpenCode Go — it
|
||||
# returns models from the public catalog that are
|
||||
# not enabled on the Go tier, causing 404 when
|
||||
# selected. Use the curated static list only. (#5311)
|
||||
pass
|
||||
elif (
|
||||
pid == "opencode-go"
|
||||
and not _hermes_cli_supports_opencode_go_live_catalog()
|
||||
):
|
||||
# Before core v0.20.5 this resolver returned the
|
||||
# generic public catalog, including models that
|
||||
# 404 on the Go tier. Use the curated Go fallback.
|
||||
raw_models = []
|
||||
else:
|
||||
raw_models = _models_from_live_provider_ids(
|
||||
pid,
|
||||
|
||||
+26
-10
@@ -741,8 +741,12 @@ def _run_gateway_runs_api_streaming(
|
||||
sse_event = "message"
|
||||
continue
|
||||
if payload_event == "run.completed":
|
||||
from api.route_approvals import retire_gateway_pending_mirror
|
||||
retire_gateway_pending_mirror(session_id, run_id=run_id)
|
||||
from api.route_approvals import settle_gateway_pending_run
|
||||
settle_gateway_pending_run(
|
||||
session_id,
|
||||
run_id,
|
||||
reason="Gateway run completed before approval resolution",
|
||||
)
|
||||
if payload.get("error"):
|
||||
raise RuntimeError(str(payload["error"]))
|
||||
output = str(payload.get("output") or "")
|
||||
@@ -754,12 +758,20 @@ def _run_gateway_runs_api_streaming(
|
||||
sse_event = "message"
|
||||
continue
|
||||
if payload_event == "run.failed":
|
||||
from api.route_approvals import retire_gateway_pending_mirror
|
||||
retire_gateway_pending_mirror(session_id, run_id=run_id)
|
||||
from api.route_approvals import settle_gateway_pending_run
|
||||
settle_gateway_pending_run(
|
||||
session_id,
|
||||
run_id,
|
||||
reason="Gateway run failed before approval resolution",
|
||||
)
|
||||
raise RuntimeError(str(payload.get("error") or "Gateway run failed"))
|
||||
if payload_event == "run.cancelled":
|
||||
from api.route_approvals import retire_gateway_pending_mirror
|
||||
retire_gateway_pending_mirror(session_id, run_id=run_id)
|
||||
from api.route_approvals import settle_gateway_pending_run
|
||||
settle_gateway_pending_run(
|
||||
session_id,
|
||||
run_id,
|
||||
reason="Gateway run was cancelled before approval resolution",
|
||||
)
|
||||
put_gateway_event("cancel", {"message": "Cancelled by gateway"})
|
||||
return None, usage
|
||||
reasoning_delta = _gateway_sse_reasoning_delta(payload)
|
||||
@@ -981,7 +993,7 @@ def _run_gateway_chat_streaming(
|
||||
except Exception:
|
||||
logger.debug("Failed to note gateway event_id %s for stream %s", event_id, stream_id, exc_info=True)
|
||||
try:
|
||||
queue_item = (event, data, event_id) if event_id and hasattr(q, "subscribe_with_snapshot") else (event, data)
|
||||
queue_item = (event, data, event_id) if hasattr(q, "subscribe_with_snapshot") else (event, data)
|
||||
q.put_nowait(queue_item)
|
||||
except Exception:
|
||||
logger.debug("Failed to put gateway event to queue")
|
||||
@@ -1467,10 +1479,14 @@ def _run_gateway_chat_streaming(
|
||||
mapped_run_id = str(_STREAM_RUN_IDS.get(stream_id) or "").strip()
|
||||
if mapped_run_id:
|
||||
try:
|
||||
from api.route_approvals import retire_gateway_pending_mirror
|
||||
retire_gateway_pending_mirror(session_id, run_id=mapped_run_id)
|
||||
from api.route_approvals import settle_gateway_pending_run
|
||||
settle_gateway_pending_run(
|
||||
session_id,
|
||||
mapped_run_id,
|
||||
reason="Gateway run ended during teardown before approval resolution",
|
||||
)
|
||||
except Exception:
|
||||
logger.debug("Failed to retire gateway pending mirrors during teardown", exc_info=True)
|
||||
logger.debug("Failed to settle gateway pending approvals during teardown", exc_info=True)
|
||||
if s is not None:
|
||||
try:
|
||||
with _get_session_agent_lock(session_id):
|
||||
|
||||
+30
-6
@@ -204,6 +204,8 @@ class GatewayWatcher:
|
||||
self._subscribers: list[queue.Queue] = []
|
||||
self._sub_lock = threading.Lock()
|
||||
self._stop_event = threading.Event()
|
||||
# Wakes a poll loop parked because nobody is subscribed (subscribe/stop).
|
||||
self._idle_wake = threading.Event()
|
||||
self._thread: threading.Thread | None = None
|
||||
self._hermes_home = Path(hermes_home).expanduser().resolve() if hermes_home else None
|
||||
self._state_db_path = (
|
||||
@@ -244,6 +246,7 @@ class GatewayWatcher:
|
||||
def stop(self):
|
||||
"""Stop the watcher thread."""
|
||||
self._stop_event.set()
|
||||
self._idle_wake.set() # unpark if waiting for the first subscriber
|
||||
# Wake up any subscribers
|
||||
with self._sub_lock:
|
||||
for q in self._subscribers:
|
||||
@@ -255,6 +258,11 @@ class GatewayWatcher:
|
||||
self._thread.join(timeout=3)
|
||||
self._thread = None
|
||||
|
||||
def _has_subscribers(self) -> bool:
|
||||
"""Return True when at least one SSE client is attached."""
|
||||
with self._sub_lock:
|
||||
return bool(self._subscribers)
|
||||
|
||||
def subscribe(self) -> queue.Queue:
|
||||
"""Subscribe to change events. Returns a queue.Queue.
|
||||
Events are dicts: {'type': 'sessions_changed', 'sessions': [...]}
|
||||
@@ -273,6 +281,9 @@ class GatewayWatcher:
|
||||
q.put_nowait(None)
|
||||
except Exception:
|
||||
logger.debug("Failed to send stop sentinel to late subscriber")
|
||||
# Wake a poll loop parked with zero subscribers so the first SSE client
|
||||
# gets a prompt initial projection instead of waiting out POLL_INTERVAL.
|
||||
self._idle_wake.set()
|
||||
return q
|
||||
|
||||
def unsubscribe(self, q: queue.Queue):
|
||||
@@ -355,18 +366,31 @@ class GatewayWatcher:
|
||||
return True
|
||||
|
||||
def _poll_loop(self):
|
||||
"""Main polling loop. Runs in a daemon thread."""
|
||||
"""Main polling loop. Runs in a daemon thread.
|
||||
|
||||
With no SSE subscribers there is nobody to notify, so the loop parks on
|
||||
``_idle_wake`` instead of re-fingerprinting ``state.db`` every few
|
||||
seconds (maint #3035). While subscribed it blocks once on
|
||||
``_stop_event.wait(POLL_INTERVAL)`` — a single timer sleep that returns
|
||||
immediately when ``stop()`` sets the event, replacing the previous
|
||||
10 wakeups/sec ``time.sleep(0.1)`` spin.
|
||||
"""
|
||||
while not self._stop_event.is_set():
|
||||
if not self._has_subscribers():
|
||||
self._idle_wake.clear()
|
||||
# subscribe() may have raced between the check and clear; if so
|
||||
# the flag is already set again and we must not park.
|
||||
if self._has_subscribers() or self._stop_event.is_set():
|
||||
continue
|
||||
self._idle_wake.wait()
|
||||
continue
|
||||
|
||||
try:
|
||||
self._poll_once()
|
||||
except Exception:
|
||||
logger.debug("Error in gateway watcher poll loop", exc_info=True)
|
||||
|
||||
# Sleep in small increments so we can stop promptly
|
||||
for _ in range(self.POLL_INTERVAL * 10):
|
||||
if self._stop_event.is_set():
|
||||
return
|
||||
time.sleep(0.1)
|
||||
self._stop_event.wait(self.POLL_INTERVAL)
|
||||
|
||||
|
||||
# ── Module-level watcher registry ──────────────────────────────────────────
|
||||
|
||||
+242
@@ -202,6 +202,248 @@ def _security_headers(handler):
|
||||
)
|
||||
|
||||
|
||||
def arm_connection_close(handler) -> None:
|
||||
"""Mark the connection dead BEFORE a reject-before-read response is written.
|
||||
|
||||
A rejection that answers without consuming the request body leaves those
|
||||
bytes queued in ``rfile``; reusing the connection makes the next HTTP/1.1
|
||||
request parse mid-body (the classic ``{}GET ... -> 501``). Arming must
|
||||
happen before the response is flushed so ``advertise_connection_close()``
|
||||
can serialize it into the response headers.
|
||||
|
||||
Best-effort: handler stubs and read-only doubles may reject the attribute,
|
||||
and a failure here must never turn a 4xx into a 500.
|
||||
"""
|
||||
try:
|
||||
handler.close_connection = True
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
|
||||
# What ``unsupported_transfer_encoding()`` reports for a header that declares no
|
||||
# coding at all (``Transfer-Encoding:``). A placeholder rather than the raw '' so
|
||||
# the return value is never a falsy non-``None`` — callers test ``is not None``,
|
||||
# and an empty string would also render as a hole in the rejection message.
|
||||
_BLANK_TRANSFER_CODING = '(blank)'
|
||||
|
||||
# The only whitespace a field value may be padded with: RFC 9110 OWS is SP/HTAB
|
||||
# and nothing else. ``str.strip()`` would also erase U+00A0 and U+0085 -- both
|
||||
# reachable over the wire, because request lines decode as latin-1 -- and
|
||||
# ``Content-Length: \xa00`` would then read back as a genuine ``0`` while its
|
||||
# payload sat unread on the socket. Padding a value with a character the grammar
|
||||
# does not allow makes it unreadable, not zero.
|
||||
_FIELD_VALUE_OWS = ' \t'
|
||||
|
||||
|
||||
def _framing_header_values(handler, name: str) -> list[str]:
|
||||
"""Every value the wire carried for one framing header, tolerating stubs.
|
||||
|
||||
ALL values, not just the first: ``Message.get()`` returns only the first
|
||||
occurrence, so a request carrying ``Content-Length: 0`` followed by
|
||||
``Content-Length: 42`` reads back as length zero. Its 42 body bytes are then
|
||||
never consumed and get parsed as the next request line -- the same poisoning
|
||||
this module exists to prevent, reached by duplicating a header rather than by
|
||||
omitting one. Conflicting framing is only visible if every value is.
|
||||
|
||||
A real handler carries an ``email.message.Message`` (case-insensitive
|
||||
``get_all()``, like the wire); the suite's ``SimpleNamespace(headers={...})``
|
||||
doubles are plain dicts, which are not, so fall back to a case-insensitive
|
||||
scan for those. A missing or odd ``headers`` must never raise: every caller
|
||||
is on a rejection path where an exception becomes a spurious 500.
|
||||
"""
|
||||
headers = getattr(handler, 'headers', None)
|
||||
if headers is None:
|
||||
return []
|
||||
try:
|
||||
get_all = getattr(headers, 'get_all', None)
|
||||
if callable(get_all):
|
||||
values = get_all(name)
|
||||
return [] if values is None else [str(v) for v in values if v is not None]
|
||||
if isinstance(headers, dict):
|
||||
lowered = name.lower()
|
||||
return [
|
||||
str(v) for k, v in headers.items()
|
||||
if str(k).lower() == lowered and v is not None
|
||||
]
|
||||
value = headers.get(name)
|
||||
return [] if value is None else [str(value)]
|
||||
except Exception:
|
||||
return []
|
||||
|
||||
|
||||
def _declared_content_lengths(handler) -> set[int] | None:
|
||||
"""Distinct declared ``Content-Length`` values, or ``None`` if any is unreadable.
|
||||
|
||||
``None`` means the framing cannot be trusted at all (a value that will not
|
||||
parse); an empty set means no length was declared.
|
||||
|
||||
A BLANK or whitespace-only value is unreadable, NOT absent. ``Content-Length:``
|
||||
with payload bytes behind it declares a body whose size the header refuses to
|
||||
state; skipping the empty value reported "no length declared", so the
|
||||
rejection kept the connection alive and those bytes were parsed as the next
|
||||
request line (403 then ``400 Bad request syntax ('{...}GET /api/... HTTP/1.1')``).
|
||||
Absence is something only the wire can say, and ``_framing_header_values()``
|
||||
already reports a header the request never carried as no value at all -- so
|
||||
anything that *is* here and does not parse has to be treated as pending bytes,
|
||||
exactly like ``banana`` or two lengths that disagree. Note ``Message`` collapses
|
||||
``Content-Length:`` and ``Content-Length: `` to the same ``''``.
|
||||
|
||||
"Parses" means RFC 9110's ``Content-Length = 1*DIGIT``: an unsigned run of
|
||||
ASCII digits, nothing else. ``int()`` is far more generous than the grammar --
|
||||
it accepts a leading sign, PEP 515 underscores and non-ASCII digits -- and
|
||||
every such spelling of ZERO used to read back as an honest ``0`` and take the
|
||||
keep-alive branch below with its payload still queued: ``Content-Length: +0``
|
||||
(also ``-0``, ``0_0``, ``+00``, ``\\xa00``) answered 403 with no
|
||||
``Connection: close``, then ``400 Bad request syntax ('{...}GET /api/...')``
|
||||
-- the same trace as a blank value. A sign is only invisible on a NON-zero
|
||||
value, where the wrong parse happens to close anyway. Anything the grammar
|
||||
rejects is unreadable framing, which means a body may be pending.
|
||||
|
||||
The ``int()`` guard stays even though ``isdigit()`` has already vetted every
|
||||
character: a digit run longer than ``sys.get_int_max_str_digits()`` (4300)
|
||||
raises ``ValueError`` too, and this runs on rejection paths where an
|
||||
exception becomes a spurious 500.
|
||||
"""
|
||||
parsed: set[int] = set()
|
||||
for raw in _framing_header_values(handler, 'Content-Length'):
|
||||
# RFC 9110 §5.3: a list-valued field may arrive as repeated lines OR as
|
||||
# one comma-combined line, and the two spellings mean the same thing.
|
||||
# So "0, 0" is the same bodyless request as two "Content-Length: 0"
|
||||
# headers and must keep its keep-alive; only a DISAGREEING list (caught
|
||||
# below by len(parsed) > 1) is unframeable. Without the split the comma
|
||||
# fails isdigit() and every agreeing list over-closed a healthy socket.
|
||||
for member in raw.split(','):
|
||||
stripped = member.strip(_FIELD_VALUE_OWS)
|
||||
if not (stripped.isascii() and stripped.isdigit()):
|
||||
return None
|
||||
try:
|
||||
parsed.add(int(stripped))
|
||||
except ValueError:
|
||||
return None
|
||||
return parsed
|
||||
|
||||
|
||||
def unreadable_content_length(handler) -> bool:
|
||||
"""True when ``Content-Length`` cannot be trusted to frame the body.
|
||||
|
||||
Either a value that will not parse (``banana``, ``0x5``, or a BLANK one), or
|
||||
two or more values that disagree. None of them can tell a reader how many
|
||||
bytes to drain, so such a request can only be rejected -- with the connection
|
||||
armed for close, since bytes may still be queued behind it.
|
||||
"""
|
||||
parsed = _declared_content_lengths(handler)
|
||||
return parsed is None or len(parsed) > 1
|
||||
|
||||
|
||||
def request_declares_body(handler) -> bool:
|
||||
"""True when the request's framing declares a body that is still queued.
|
||||
|
||||
Keyed on the wire, not on the method: ``Transfer-Encoding`` present (chunked
|
||||
framing hides the length until the terminating chunk is read) or
|
||||
``Content-Length`` present and non-zero. The method proves nothing in either
|
||||
direction -- a GET may carry a declared body and a POST/PUT/DELETE may carry
|
||||
none -- and a static per-method flag therefore both misses body-bearing
|
||||
"read" requests (whose unread bytes poison the next pooled request) and
|
||||
kills healthy keep-alive on body-less writes.
|
||||
|
||||
A blank, unparseable or self-contradicting ``Content-Length`` counts as a
|
||||
declared body, as does ANY ``Transfer-Encoding`` -- ``identity`` and a blank
|
||||
value included: a value we cannot interpret does not prove the body's absence,
|
||||
and the safe assumption for connection reuse is that bytes are pending. Only
|
||||
framing that positively says "no body" -- no framing header at all, or every
|
||||
declared length agreeing on zero -- keeps the connection alive.
|
||||
"""
|
||||
if unsupported_transfer_encoding(handler) is not None:
|
||||
return True
|
||||
parsed = _declared_content_lengths(handler)
|
||||
if parsed is None:
|
||||
return True
|
||||
return any(length != 0 for length in parsed)
|
||||
|
||||
|
||||
def unsupported_transfer_encoding(handler) -> str | None:
|
||||
"""Return the transfer coding this server cannot decode, or ``None``.
|
||||
|
||||
EVERY present value counts, with no exemption for any coding.
|
||||
``BaseHTTPRequestHandler`` decodes none of them: it hands every reader a raw
|
||||
``rfile``, so a chunked body reads back as its literal chunk framing while a
|
||||
``Content-Length``-based reader sees length 0 and consumes nothing at all.
|
||||
RFC 9112 section 6.3 says the same thing normatively -- a request whose final
|
||||
transfer coding is not ``chunked`` has a body length the recipient cannot
|
||||
determine, so it must be refused and the connection closed. Such a request
|
||||
can therefore only be rejected, and only with the connection armed for close,
|
||||
because its payload bytes are still queued on the socket.
|
||||
|
||||
``identity`` used to be exempt as a coding that "frames nothing". It is not:
|
||||
it names no framing either, and RFC 7230 removed it from the transfer codings
|
||||
altogether. A sidecar rejection then answered 403 with keep-alive and the
|
||||
payload was parsed as the next request line
|
||||
(``400 Bad request syntax ('{...}GET /api/health/agent HTTP/1.1')``), and all
|
||||
four upload handlers fell through to a length-0 read, answered 400 "No file
|
||||
field in request" and left the multipart bytes on the socket
|
||||
(``400 Bad request syntax ('--x')``). Nothing legitimate is lost: no client
|
||||
sends a transfer coding this server could have honoured.
|
||||
|
||||
A header present but EMPTY (``Transfer-Encoding:``) is reported through
|
||||
``_BLANK_TRANSFER_CODING`` rather than as ``''``, so callers testing
|
||||
``is not None`` and the 411 message both read right. Only an ABSENT header
|
||||
yields no values at all -- absence is something only the wire can say.
|
||||
|
||||
The first value is enough precisely because no value is acceptable; it is
|
||||
reported as-is (SP/HTAB trimmed, the one padding RFC 9110 allows) purely so
|
||||
the rejection message names what arrived.
|
||||
"""
|
||||
values = _framing_header_values(handler, 'Transfer-Encoding')
|
||||
if not values:
|
||||
return None
|
||||
return values[0].strip(_FIELD_VALUE_OWS) or _BLANK_TRANSFER_CODING
|
||||
|
||||
|
||||
def arm_connection_close_if_body_pending(handler) -> bool:
|
||||
"""Arm close for a reject-before-read, but only if a body was really declared.
|
||||
|
||||
Returns whether the connection was armed. Use this instead of
|
||||
``arm_connection_close()`` wherever the rejection can also fire on a request
|
||||
with no body: closing then would drop a pooled client's healthy connection
|
||||
for no framing reason.
|
||||
"""
|
||||
if not request_declares_body(handler):
|
||||
return False
|
||||
arm_connection_close(handler)
|
||||
return True
|
||||
|
||||
|
||||
def advertise_connection_close(handler) -> None:
|
||||
"""Serialize ``close_connection`` into exactly one ``Connection: close`` header.
|
||||
|
||||
``BaseHTTPRequestHandler`` tracks ``close_connection`` internally but never
|
||||
writes it to the wire, so a server-side close is invisible to the client: a
|
||||
pooled client reuses the socket and fails with BrokenPipeError instead of
|
||||
opening a fresh connection. Call this from ``end_headers()``, before the
|
||||
header buffer is flushed.
|
||||
|
||||
Dedup is derived from the pending header buffer rather than a private flag,
|
||||
because the SSE endpoints send their own ``Connection: close`` (and
|
||||
``send_header`` itself flips ``close_connection`` when they do) — keying off
|
||||
a flag we set ourselves would emit the header twice on every stream.
|
||||
|
||||
Both attribute reads are defensive: partially-built handler stubs in the
|
||||
test suite carry neither ``close_connection`` nor ``_headers_buffer``, and
|
||||
header emission must not depend on their presence. The buffer is also
|
||||
type-checked before it is scanned -- a stub whose ``_headers_buffer`` is a
|
||||
truthy non-iterable (a bare ``Mock``) would otherwise raise ``TypeError``
|
||||
from inside ``end_headers()``, which ``_handle_write`` would swallow into a
|
||||
spurious 500.
|
||||
"""
|
||||
if not getattr(handler, 'close_connection', False):
|
||||
return
|
||||
buffered = getattr(handler, '_headers_buffer', None)
|
||||
for raw in buffered if isinstance(buffered, (list, tuple)) else ():
|
||||
if raw[:11].lower() == b'connection:':
|
||||
return
|
||||
handler.send_header('Connection', 'close')
|
||||
|
||||
|
||||
def flush_pending_auth_cookies(handler) -> None:
|
||||
pending = getattr(handler, '_pending_set_cookies', None)
|
||||
if not pending:
|
||||
|
||||
+125
-31
@@ -2,6 +2,24 @@
|
||||
|
||||
State-extraction prelude to the routes.py split tracked in #1907.
|
||||
Extracts approval state, not handlers, by design.
|
||||
|
||||
Gateway approvals carry two pieces of state with one lifecycle:
|
||||
|
||||
* the **producer** entry in ``tools.approval._gateway_queues`` — the queued
|
||||
approval itself, which owns the lifecycle; and
|
||||
* the **mirror** entry in ``_pending`` — this module's projection of that
|
||||
producer for the UI.
|
||||
|
||||
A producer is consumed only by an explicit settlement operation, which assigns
|
||||
its result and signals its waiter before fallible projection work begins.
|
||||
``retire_gateway_pending_mirror`` cleans up the UI projection; it must not
|
||||
silently delete a producer, because that would strand the agent thread parked
|
||||
on the producer's event. For a provider-qualified response,
|
||||
``resolve_gateway_pending_run`` matches the exact ``(run_id, approval_id)``
|
||||
pair, settles that producer, and preserves same-run siblings and other runs.
|
||||
When a run becomes terminal, ``settle_gateway_pending_run`` fail-closes every
|
||||
producer owned by that run with ``deny`` before retiring its mirrors; producers
|
||||
owned by another run remain live.
|
||||
"""
|
||||
import queue
|
||||
import threading
|
||||
@@ -505,21 +523,26 @@ def release_gateway_approval_relay_owner(session_key: str, run_id: str, approval
|
||||
_gateway_relay_owners.pop(key, None)
|
||||
|
||||
|
||||
def _settle_gateway_entry(entry, choice: str, reason: str | None = None) -> None:
|
||||
"""Complete a removed producer before any fallible projection work."""
|
||||
entry.result = choice
|
||||
if reason:
|
||||
entry.reason = reason
|
||||
entry.event.set()
|
||||
|
||||
|
||||
def retire_gateway_pending_mirror(
|
||||
session_key: str,
|
||||
approval_id: str = "",
|
||||
run_id: str = "",
|
||||
mirror_token: str = "",
|
||||
) -> bool:
|
||||
"""Retire one approval, or every mirror for a terminal run."""
|
||||
"""Retire one approval mirror, or every mirror for a terminal run."""
|
||||
with _lock:
|
||||
reconcile_gateway_pending_mirror_locked(session_key)
|
||||
queue = _pending.get(session_key)
|
||||
entries = queue if isinstance(queue, list) else [queue] if queue else []
|
||||
normalized_run_id = str(run_id or "").strip()
|
||||
gateway_queue = _gateway_queues.get(session_key) or []
|
||||
retained_gateway_queue = gateway_queue
|
||||
gateway_queue_changed = False
|
||||
if approval_id:
|
||||
match = _gateway_pending_mirror_locked(
|
||||
session_key,
|
||||
@@ -542,15 +565,7 @@ def retire_gateway_pending_mirror(
|
||||
if _is_gateway_mirror_entry(entry)
|
||||
and not str(entry.get("run_id") or "").strip()
|
||||
]
|
||||
if normalized_run_id:
|
||||
retained_gateway_queue = []
|
||||
for entry in gateway_queue:
|
||||
data = getattr(entry, "data", None) or {}
|
||||
if str(data.get("run_id") or "").strip() == normalized_run_id:
|
||||
gateway_queue_changed = True
|
||||
continue
|
||||
retained_gateway_queue.append(entry)
|
||||
if not retired and not gateway_queue_changed:
|
||||
if not retired:
|
||||
head, total, changed = reconcile_gateway_pending_mirror_locked(session_key)
|
||||
_approval_sse_notify_locked(session_key, head, total)
|
||||
if changed:
|
||||
@@ -558,11 +573,6 @@ def retire_gateway_pending_mirror(
|
||||
return changed
|
||||
for match in retired:
|
||||
entries.remove(match)
|
||||
if normalized_run_id and not approval_id:
|
||||
if retained_gateway_queue:
|
||||
_gateway_queues[session_key] = retained_gateway_queue
|
||||
else:
|
||||
_gateway_queues.pop(session_key, None)
|
||||
if entries:
|
||||
_pending[session_key] = entries
|
||||
else:
|
||||
@@ -573,6 +583,60 @@ def retire_gateway_pending_mirror(
|
||||
return True
|
||||
|
||||
|
||||
def settle_gateway_pending_run(
|
||||
session_key: str,
|
||||
run_id: str,
|
||||
*,
|
||||
reason: str,
|
||||
) -> tuple[int, dict | None, int]:
|
||||
"""Fail-close every producer and mirror owned by a terminal gateway run."""
|
||||
run_id = str(run_id or "").strip()
|
||||
if not run_id:
|
||||
return 0, None, 0
|
||||
|
||||
targets = []
|
||||
with _lock:
|
||||
gateway_queue = _gateway_queues.get(session_key) or []
|
||||
retained_gateway_queue = []
|
||||
for entry in gateway_queue:
|
||||
data = getattr(entry, "data", None) or {}
|
||||
if str(data.get("run_id") or "").strip() == run_id:
|
||||
targets.append(entry)
|
||||
else:
|
||||
retained_gateway_queue.append(entry)
|
||||
if retained_gateway_queue:
|
||||
_gateway_queues[session_key] = retained_gateway_queue
|
||||
else:
|
||||
_gateway_queues.pop(session_key, None)
|
||||
|
||||
# Removal and waiter settlement are one semantic operation. In
|
||||
# particular, reconciliation and SSE notification below may raise.
|
||||
for entry in targets:
|
||||
_settle_gateway_entry(entry, "deny", reason)
|
||||
|
||||
queue = _pending.get(session_key)
|
||||
entries = queue if isinstance(queue, list) else [queue] if queue else []
|
||||
retained_pending = [
|
||||
entry
|
||||
for entry in entries
|
||||
if not (
|
||||
_is_gateway_mirror_entry(entry)
|
||||
and str(entry.get("run_id") or "").strip() == run_id
|
||||
)
|
||||
]
|
||||
mirrors_retired = len(retained_pending) != len(entries)
|
||||
if retained_pending:
|
||||
_pending[session_key] = retained_pending
|
||||
else:
|
||||
_pending.pop(session_key, None)
|
||||
|
||||
head, total, changed = reconcile_gateway_pending_mirror_locked(session_key)
|
||||
_approval_sse_notify_locked(session_key, head, total)
|
||||
if targets or mirrors_retired or changed:
|
||||
publish_session_list_changed("attention_resolved")
|
||||
return len(targets), head, total
|
||||
|
||||
|
||||
def _gateway_mirrored_pending_run_id(session_key: str, approval_id: str) -> str | None:
|
||||
"""Compatibility wrapper for exact run-backed lookup."""
|
||||
approval_id = str(approval_id or "").strip()
|
||||
@@ -804,14 +868,50 @@ def resolve_gateway_pending_local(
|
||||
_gateway_queues[session_key] = gateway_queue
|
||||
else:
|
||||
_gateway_queues.pop(session_key, None)
|
||||
if target is not None:
|
||||
_settle_gateway_entry(target, choice, reason)
|
||||
head, total, _changed = reconcile_gateway_pending_mirror_locked(session_key)
|
||||
_approval_sse_notify_locked(session_key, head, total)
|
||||
if target is None:
|
||||
return 0, head, total
|
||||
publish_session_list_changed("attention_resolved")
|
||||
return 1, head, total
|
||||
|
||||
|
||||
def resolve_gateway_pending_run(
|
||||
session_key: str,
|
||||
approval_id: str,
|
||||
run_id: str,
|
||||
choice: str,
|
||||
reason: str | None = None,
|
||||
) -> tuple[int, dict | None, int]:
|
||||
"""Resolve one exact run-backed producer and wake its parked waiter."""
|
||||
target = None
|
||||
approval_id = str(approval_id or "").strip()
|
||||
run_id = str(run_id or "").strip()
|
||||
if not approval_id or not run_id:
|
||||
return 0, None, 0
|
||||
with _lock:
|
||||
gateway_queue = _gateway_queues.get(session_key) or []
|
||||
retained_gateway_queue = []
|
||||
for entry in gateway_queue:
|
||||
data = getattr(entry, "data", None) or {}
|
||||
entry_approval_id = str(data.get("approval_id") or "").strip()
|
||||
entry_run_id = str(data.get("run_id") or "").strip()
|
||||
if target is None and entry_approval_id == approval_id and entry_run_id == run_id:
|
||||
target = entry
|
||||
else:
|
||||
retained_gateway_queue.append(entry)
|
||||
if retained_gateway_queue:
|
||||
_gateway_queues[session_key] = retained_gateway_queue
|
||||
else:
|
||||
_gateway_queues.pop(session_key, None)
|
||||
if target is not None:
|
||||
_settle_gateway_entry(target, choice, reason)
|
||||
head, total, _changed = reconcile_gateway_pending_mirror_locked(session_key)
|
||||
_approval_sse_notify_locked(session_key, head, total)
|
||||
if target is None:
|
||||
return 0, head, total
|
||||
target.result = choice
|
||||
if reason:
|
||||
target.reason = reason
|
||||
target.event.set()
|
||||
publish_session_list_changed("attention_resolved")
|
||||
return 1, head, total
|
||||
|
||||
@@ -856,12 +956,9 @@ def resolve_gateway_pending_local_no_run_mirror(
|
||||
_pending[session_key] = entries
|
||||
else:
|
||||
_pending.pop(session_key, None)
|
||||
_settle_gateway_entry(target, choice, reason)
|
||||
head, total, _changed = reconcile_gateway_pending_mirror_locked(session_key)
|
||||
_approval_sse_notify_locked(session_key, head, total)
|
||||
target.result = choice
|
||||
if reason:
|
||||
target.reason = reason
|
||||
target.event.set()
|
||||
publish_session_list_changed("attention_resolved")
|
||||
return True, 1, head, total
|
||||
|
||||
@@ -904,14 +1001,11 @@ def resolve_gateway_pending_local_all(
|
||||
else:
|
||||
_pending.pop(session_key, None)
|
||||
|
||||
for entry in targets:
|
||||
_settle_gateway_entry(entry, choice, reason)
|
||||
head, total, _changed = reconcile_gateway_pending_mirror_locked(session_key)
|
||||
_approval_sse_notify_locked(session_key, head, total)
|
||||
|
||||
for entry in targets:
|
||||
entry.result = choice
|
||||
if reason:
|
||||
entry.reason = reason
|
||||
entry.event.set()
|
||||
if targets or removed_pending:
|
||||
publish_session_list_changed("attention_resolved")
|
||||
return len(targets), head, total
|
||||
|
||||
+169
-22
@@ -593,7 +593,15 @@ def _request_session_visibility_exempt(method: str, path: str | None) -> bool:
|
||||
|
||||
|
||||
def _session_id_visible_to_request_profile(handler, sid, *, emit_error: bool = True) -> bool:
|
||||
"""Return whether ``sid`` belongs to the active profile."""
|
||||
"""Return whether ``sid`` belongs to the active profile.
|
||||
|
||||
On a profile mismatch, the helper mirrors the detail-load endpoint's
|
||||
contract (#13043, #13493): return ``409 session_profile_mismatch`` for
|
||||
a session owned by a KNOWN other profile, and keep ``404 Session
|
||||
not found`` only for the unknown/legacy None-profile case so the
|
||||
frontend's self-heal (clear stale URL + localStorage) keeps firing
|
||||
for actually-missing sids. ``#7710``.
|
||||
"""
|
||||
if not isinstance(sid, str) or not sid:
|
||||
return True
|
||||
if not is_safe_session_id(sid):
|
||||
@@ -602,9 +610,23 @@ def _session_id_visible_to_request_profile(handler, sid, *, emit_error: bool = T
|
||||
session = get_session(sid, metadata_only=True)
|
||||
except KeyError:
|
||||
return True
|
||||
if not _session_visible_to_active_profile(getattr(session, "profile", None), handler):
|
||||
session_profile = getattr(session, "profile", None) or None
|
||||
if not _session_visible_to_active_profile(session_profile, handler):
|
||||
if emit_error:
|
||||
bad(handler, "Session not found", 404)
|
||||
if session_profile:
|
||||
j(handler, {
|
||||
"error": "Session belongs to a different profile",
|
||||
"code": "session_profile_mismatch",
|
||||
"session_id": sid,
|
||||
"profile": session_profile,
|
||||
}, status=409)
|
||||
else:
|
||||
# Unknown/legacy None-profile sidecar: keep the 404 so the
|
||||
# frontend's self-heal still fires. _profiles_match coerces
|
||||
# None->'default', so a truly missing/legacy session under a
|
||||
# non-default active profile would otherwise emit a useless
|
||||
# 409 with profile=null.
|
||||
bad(handler, "Session not found", 404)
|
||||
return False
|
||||
return True
|
||||
|
||||
@@ -2952,6 +2974,7 @@ from api.helpers import (
|
||||
require,
|
||||
bad,
|
||||
safe_resolve,
|
||||
arm_connection_close_if_body_pending,
|
||||
j,
|
||||
t,
|
||||
read_body,
|
||||
@@ -5323,7 +5346,21 @@ def _handle_session_anchor_scene(handler, body):
|
||||
# this an authenticated request under profile A could persist anchor scenes
|
||||
# onto a session owned by profile B (cross-profile write). Reject as 404 —
|
||||
# same shape the read path uses — and leave anchor_activity_scenes untouched.
|
||||
if not _session_visible_to_active_profile(getattr(s, "profile", None) or None, handler):
|
||||
# #7710: cross-profile writes are rejected with 409
|
||||
# ``session_profile_mismatch`` so the client can offer to switch
|
||||
# to the owning profile (mirrors the detail-load endpoint's
|
||||
# contract at #13043 / #13493). 404 is preserved for the
|
||||
# None-profile (unknown/legacy) case so the frontend self-heal
|
||||
# path still fires for actually-missing sids.
|
||||
_anchor_session_profile = getattr(s, "profile", None) or None
|
||||
if not _session_visible_to_active_profile(_anchor_session_profile, handler):
|
||||
if _anchor_session_profile:
|
||||
return j(handler, {
|
||||
"error": "Session belongs to a different profile",
|
||||
"code": "session_profile_mismatch",
|
||||
"session_id": sid,
|
||||
"profile": _anchor_session_profile,
|
||||
}, status=409)
|
||||
return bad(handler, "Session not found", 404)
|
||||
with _get_session_agent_lock(sid):
|
||||
idx, message = _find_anchor_scene_message(
|
||||
@@ -5835,6 +5872,15 @@ def _csrf_rejection_error(handler) -> str:
|
||||
def _check_csrf(handler) -> bool:
|
||||
"""Reject cross-origin or tokenless authenticated browser unsafe requests."""
|
||||
if not _check_same_origin_browser_request(handler):
|
||||
# CSRF checks run before read_body(), so close rather than reusing an
|
||||
# HTTP/1.1 connection whose unread body would corrupt the next request --
|
||||
# but ONLY when the framing says bytes are really queued. Arming
|
||||
# unconditionally dropped a healthy pooled connection on a body-less
|
||||
# write: verified on the wire, `POST /api/session/new` with
|
||||
# `Origin: http://evil.invalid` and no `Content-Length` (and with
|
||||
# `Content-Length: 0`) answered 403 + `Connection: close` and the
|
||||
# pipelined `GET /api/health/agent` was never served.
|
||||
arm_connection_close_if_body_pending(handler)
|
||||
return False
|
||||
if not _is_browser_unsafe_request(handler):
|
||||
return True # non-browser clients (curl, MCP, agent) have no Origin/Referer
|
||||
@@ -5847,6 +5893,11 @@ def _check_csrf(handler) -> bool:
|
||||
submitted = handler.headers.get(CSRF_HEADER_NAME) or handler.headers.get("X-CSRF-Token")
|
||||
if verify_csrf_token(cookie_val or "", submitted or ""):
|
||||
return True
|
||||
# Same framing rule as the origin rejection above: a token mismatch on a
|
||||
# body-less write has nothing unread to protect. Verified on the wire with an
|
||||
# authenticated same-origin `POST /api/session/new` carrying no CSRF token and
|
||||
# no `Content-Length`: 403 + `Connection: close`, follow-up dropped.
|
||||
arm_connection_close_if_body_pending(handler)
|
||||
return _set_csrf_failure_reason(handler, "token_mismatch")
|
||||
|
||||
|
||||
@@ -6026,6 +6077,14 @@ def _handle_extension_sidecar_proxy(
|
||||
# DELETE fell through the CSRF compatibility path that intentionally admits
|
||||
# non-browser clients, giving unsafe methods weaker provenance than GET.
|
||||
if not _check_same_origin_browser_request(handler, require_provenance=True):
|
||||
# Provenance rejection runs before read_body(), so close-and-advertise
|
||||
# whenever the request DECLARED a body (Content-Length non-zero, or any
|
||||
# Transfer-Encoding): those bytes are still queued in rfile and a reused
|
||||
# HTTP/1.1 connection would parse them as the next request line (same
|
||||
# class as _check_csrf). Gating on read_request_body instead was wrong in
|
||||
# both directions — it missed a GET that carries a declared body, and it
|
||||
# closed a healthy connection on a body-less DELETE/PUT/PATCH.
|
||||
arm_connection_close_if_body_pending(handler)
|
||||
return j(handler, {"error": _csrf_rejection_error(handler)}, status=403)
|
||||
try:
|
||||
request_body = _read_body_bytes(handler) if read_request_body else None
|
||||
@@ -6468,6 +6527,13 @@ def _handle_csp_report(handler) -> bool:
|
||||
"Dropped CSP report from %s: rate limit exceeded",
|
||||
_client_ip_for_rate_limit(handler),
|
||||
)
|
||||
# Rate-limit rejection runs before the body is read; close-and-advertise
|
||||
# so the unread report can't corrupt the next pooled request -- but only
|
||||
# when a body was really declared. A body-less report answered 204 WITH
|
||||
# `Connection: close` once the 100-per-60s limiter tripped (reproduced on
|
||||
# the wire at request 101; the pipelined `GET /api/health/agent` was
|
||||
# dropped), so a browser that keeps reporting loses its socket each time.
|
||||
arm_connection_close_if_body_pending(handler)
|
||||
return _send_no_content(handler)
|
||||
|
||||
payload = _read_csp_report_payload(handler)
|
||||
@@ -10710,6 +10776,7 @@ from api.run_journal import (
|
||||
_parse_run_journal_event_id as _shared_parse_run_journal_event_id,
|
||||
bound_run_journal_snapshot_args,
|
||||
find_run_summary,
|
||||
journal_replay_visible,
|
||||
read_run_events,
|
||||
read_session_run_events,
|
||||
session_journal_fingerprint,
|
||||
@@ -10770,8 +10837,10 @@ from api.route_approvals import ( # noqa: F401 — re-exports for backward comp
|
||||
gateway_pending_mirrors,
|
||||
release_gateway_approval_relay_owner,
|
||||
retire_gateway_pending_mirror,
|
||||
settle_gateway_pending_run,
|
||||
reconcile_gateway_pending_mirror_locked,
|
||||
resolve_gateway_pending_local,
|
||||
resolve_gateway_pending_run,
|
||||
resolve_gateway_pending_local_all,
|
||||
resolve_gateway_pending_local_no_run_mirror,
|
||||
set_session_yolo_enabled,
|
||||
@@ -11085,9 +11154,7 @@ button:hover{background:rgba(124,185,255,.25)}
|
||||
<h1>{{BOT_NAME}}</h1>
|
||||
<p class="sub">{{LOGIN_SUBTITLE}}</p>
|
||||
<form id="login-form" data-invalid-pw="{{LOGIN_INVALID_PW}}" data-conn-failed="{{LOGIN_CONN_FAILED}}">
|
||||
<input type="password" id="pw" placeholder="{{LOGIN_PLACEHOLDER}}" autofocus>
|
||||
<button type="submit">{{LOGIN_BTN}}</button>
|
||||
<button type="button" id="passkey-login" class="passkey-login" style="display:none">Sign in with passkey</button>
|
||||
{{PASSWORD_FORM_HTML}}
|
||||
{{OIDC_LOGIN_HTML}}
|
||||
</form>
|
||||
<div class="err" id="err"></div>
|
||||
@@ -12858,6 +12925,15 @@ def _handle_shutdown(handler) -> bool:
|
||||
|
||||
def _handle_health_restart(handler) -> bool:
|
||||
"""Restart the Hermes messaging gateway service."""
|
||||
# This endpoint never consumes its request body on any outcome, so close when
|
||||
# one was DECLARED -- and only then. Arming unconditionally closed the socket
|
||||
# on every call including the successful, body-less one the WebUI actually
|
||||
# makes: verified on the wire, `POST /api/health/restart` with no
|
||||
# `Content-Length` answered with `Connection: close` and the pipelined
|
||||
# `GET /api/health/agent` was never served. The single arming covers every
|
||||
# outcome below (completed / in_progress / busy / error) because the framing,
|
||||
# not the result, decides.
|
||||
arm_connection_close_if_body_pending(handler)
|
||||
outcome = restart_active_profile_gateway()
|
||||
|
||||
if outcome.get("status") == "completed":
|
||||
@@ -13630,6 +13706,36 @@ def handle_get(handler, parsed) -> bool:
|
||||
]
|
||||
from urllib.parse import quote
|
||||
from api.updates import WEBUI_VERSION
|
||||
# #7056: only render the password input / submit / passkey controls
|
||||
# when password auth is actually enabled. With native OIDC configured
|
||||
# and ``HERMES_WEBUI_PASSWORD`` unset, the form previously still
|
||||
# displayed the password prompt and accepted — silently 401-ing at
|
||||
# the server — every submit. The OIDC SSO entry point stays the
|
||||
# sole path. ``is_password_auth_enabled`` is the same predicate
|
||||
# ``/api/auth/status`` reports as ``password_auth_enabled``.
|
||||
from api.auth import are_passkeys_enabled, is_password_auth_enabled
|
||||
|
||||
# The password INPUT is gated on a configured password, but the passkey
|
||||
# button must survive a passwordless-passkey deployment: settings expose
|
||||
# ``passwordless_enabled = passkeys registered AND not password_auth_enabled``
|
||||
# (routes.py ~14059) and ``is_auth_enabled()`` counts passkeys as an
|
||||
# independent auth method, so hiding the button when no password is set
|
||||
# would remove the ONLY working login affordance for those instances.
|
||||
_passkey_button_html = (
|
||||
'<button type="button" id="passkey-login" class="passkey-login" '
|
||||
'style="display:none">Sign in with passkey</button>'
|
||||
)
|
||||
if is_password_auth_enabled():
|
||||
_password_form_html = (
|
||||
f'<input type="password" id="pw" '
|
||||
f'placeholder="{_html.escape(_login_strings["placeholder"])}" autofocus>'
|
||||
f'<button type="submit">{_html.escape(_login_strings["btn"])}</button>'
|
||||
f'{_passkey_button_html}'
|
||||
)
|
||||
elif are_passkeys_enabled():
|
||||
_password_form_html = _passkey_button_html
|
||||
else:
|
||||
_password_form_html = ""
|
||||
version_token = quote(WEBUI_VERSION, safe="")
|
||||
_page = (
|
||||
_LOGIN_PAGE_HTML.replace("{{BOT_NAME}}", _bn)
|
||||
@@ -13638,10 +13744,7 @@ def handle_get(handler, parsed) -> bool:
|
||||
.replace("{{LANG}}", _html.escape(_login_strings["lang"]))
|
||||
.replace("{{LOGIN_TITLE}}", _html.escape(_login_strings["title"]))
|
||||
.replace("{{LOGIN_SUBTITLE}}", _html.escape(_login_strings["subtitle"]))
|
||||
.replace(
|
||||
"{{LOGIN_PLACEHOLDER}}", _html.escape(_login_strings["placeholder"])
|
||||
)
|
||||
.replace("{{LOGIN_BTN}}", _html.escape(_login_strings["btn"]))
|
||||
.replace("{{PASSWORD_FORM_HTML}}", _password_form_html)
|
||||
.replace("{{LOGIN_INVALID_PW}}", _html.escape(_login_strings["invalid_pw"]))
|
||||
.replace(
|
||||
"{{LOGIN_CONN_FAILED}}", _html.escape(_login_strings["conn_failed"])
|
||||
@@ -14483,7 +14586,11 @@ def handle_get(handler, parsed) -> bool:
|
||||
if stop_gateway_run(run_id):
|
||||
owner_sid = stream_owner_session_id(stream_id)
|
||||
if owner_sid:
|
||||
retire_gateway_pending_mirror(owner_sid, run_id=run_id)
|
||||
settle_gateway_pending_run(
|
||||
owner_sid,
|
||||
run_id,
|
||||
reason="Gateway run was cancelled before approval resolution",
|
||||
)
|
||||
else:
|
||||
gateway_stop_blocked = True
|
||||
except Exception:
|
||||
@@ -15149,6 +15256,12 @@ def handle_post(handler, parsed) -> bool:
|
||||
if diag:
|
||||
diag.stage("process_complete_ack_deprecated")
|
||||
try:
|
||||
# The 410 runs before the stale tab's JSON body is read;
|
||||
# close-and-advertise so those unread bytes can't corrupt the next
|
||||
# pooled request -- but a body-less ack has none, and closing then
|
||||
# just kills keep-alive (on the wire: 410 + `Connection: close` with
|
||||
# no `Content-Length`, pipelined follow-up dropped).
|
||||
arm_connection_close_if_body_pending(handler)
|
||||
j(
|
||||
handler,
|
||||
{
|
||||
@@ -18575,6 +18688,8 @@ def _replay_run_journal(
|
||||
max_seq=max_seq,
|
||||
)
|
||||
for entry in journal.get("events") or []:
|
||||
if not journal_replay_visible(entry):
|
||||
continue
|
||||
_sse_with_id(
|
||||
handler,
|
||||
entry.get("event") or entry.get("type") or "message",
|
||||
@@ -19028,14 +19143,14 @@ def _handle_sse_stream(handler, parsed):
|
||||
continue
|
||||
if len(item) >= 3:
|
||||
event, data, queued_event_id = item[0], item[1], item[2]
|
||||
event_id = queued_event_id
|
||||
else:
|
||||
event, data = item
|
||||
queued_event_id = STREAM_LAST_EVENT_ID.get(stream_id)
|
||||
event_id = STREAM_LAST_EVENT_ID.get(stream_id)
|
||||
# Stage-364: emit `id:` from STREAM_LAST_EVENT_ID side-channel so
|
||||
# the frontend's `_lastRunJournalSeq` cursor advances during live
|
||||
# streaming. Without this, mid-stream error→replay would arrive
|
||||
# with after_seq=0 and double-render every journaled event.
|
||||
event_id = queued_event_id or STREAM_LAST_EVENT_ID.get(stream_id)
|
||||
event_seq = _run_journal_same_run_seq(event_id, stream_id)
|
||||
if replay_cutoff_seq is not None and event_seq is not None and event_seq <= replay_cutoff_seq:
|
||||
continue
|
||||
@@ -19110,6 +19225,8 @@ def _handle_session_run_journal_stream_for_session(handler, parsed, session_id):
|
||||
|
||||
def emit_replay(events, stream_id, cutoff_seq):
|
||||
for entry in events:
|
||||
if not journal_replay_visible(entry):
|
||||
continue
|
||||
event_id = str(entry.get("event_id") or "")
|
||||
event_seq = _run_journal_same_run_seq(event_id, stream_id)
|
||||
if cutoff_seq is not None and event_seq is not None and event_seq > cutoff_seq:
|
||||
@@ -19176,10 +19293,10 @@ def _handle_session_run_journal_stream_for_session(handler, parsed, session_id):
|
||||
continue
|
||||
if len(item) >= 3:
|
||||
event, data, queued_event_id = item[0], item[1], item[2]
|
||||
event_id = queued_event_id
|
||||
else:
|
||||
event, data = item
|
||||
queued_event_id = STREAM_LAST_EVENT_ID.get(active_stream_id)
|
||||
event_id = queued_event_id or STREAM_LAST_EVENT_ID.get(active_stream_id)
|
||||
event_id = STREAM_LAST_EVENT_ID.get(active_stream_id)
|
||||
event_seq = _run_journal_same_run_seq(event_id, active_stream_id)
|
||||
_is_terminal = event in SSE_RELAY_CLOSE_EVENTS
|
||||
_already_sent = (
|
||||
@@ -23959,6 +24076,17 @@ def _handle_session_compression_recovery_start(handler, body):
|
||||
except KeyError:
|
||||
return bad(handler, "Session not found", 404)
|
||||
if not _session_visible_to_active_profile(getattr(source, "profile", None), handler):
|
||||
# #7710: same contract as the detail-load endpoint — 409
|
||||
# ``session_profile_mismatch`` for a known other profile,
|
||||
# 404 only for the None-profile self-heal path.
|
||||
_recovery_session_profile = getattr(source, "profile", None)
|
||||
if _recovery_session_profile:
|
||||
return j(handler, {
|
||||
"error": "Session belongs to a different profile",
|
||||
"code": "session_profile_mismatch",
|
||||
"session_id": sid,
|
||||
"profile": _recovery_session_profile,
|
||||
}, status=409)
|
||||
return bad(handler, "Session not found", 404)
|
||||
recovery = compression_recovery_payload_for_session(source)
|
||||
if not recovery:
|
||||
@@ -24358,6 +24486,17 @@ def _handle_chat_start(handler, body, diag=None):
|
||||
# Empty placeholders can still be retagged when the
|
||||
# requested profile matches the active request profile.
|
||||
s.profile = requested_profile
|
||||
elif session_profile:
|
||||
# #7710: known other profile → 409 ``session_profile_mismatch``
|
||||
# so the client can offer to switch to it (#5419).
|
||||
# 404 is preserved only for the None-profile
|
||||
# (unknown/legacy) self-heal case.
|
||||
return j(handler, {
|
||||
"error": "Session belongs to a different profile",
|
||||
"code": "session_profile_mismatch",
|
||||
"session_id": body.get("session_id", ""),
|
||||
"profile": session_profile,
|
||||
}, status=409)
|
||||
else:
|
||||
return bad(handler, "Session not found", 404)
|
||||
# Resolve durable rotations before any workspace/model/pending mutation.
|
||||
@@ -26214,7 +26353,6 @@ def _resolve_approval_legacy(sid: str, approval_id: str, choice: str, run_id: st
|
||||
found_target = False
|
||||
gateway_keys = []
|
||||
local_gateway_approval_id = ""
|
||||
gateway_head_matches_target = False
|
||||
with _lock:
|
||||
reconcile_gateway_pending_mirror_locked(sid)
|
||||
queue = _pending.get(sid)
|
||||
@@ -26286,9 +26424,6 @@ def _resolve_approval_legacy(sid: str, approval_id: str, choice: str, run_id: st
|
||||
gw_data = getattr(gw_entry, "data", None) or {}
|
||||
gw_approval_id = str(gw_data.get("approval_id") or "").strip()
|
||||
gw_run_id = str(gw_data.get("run_id") or "").strip()
|
||||
gateway_head_matches_target = bool(
|
||||
run_id and gw_approval_id == approval_id and gw_run_id == run_id
|
||||
)
|
||||
if gw_approval_id == approval_id and (not run_id or gw_run_id == run_id):
|
||||
local_gateway_approval_id = approval_id
|
||||
elif not run_id and found_target and pending:
|
||||
@@ -26349,8 +26484,10 @@ def _resolve_approval_legacy(sid: str, approval_id: str, choice: str, run_id: st
|
||||
local_gateway_resolved, _head, _total = resolve_gateway_pending_local(
|
||||
sid, local_gateway_approval_id, choice
|
||||
)
|
||||
elif approval_id and found_target and run_id and gateway_head_matches_target:
|
||||
gateway_resolved = resolve_gateway_approval(sid, choice, resolve_all=False) or 0
|
||||
elif approval_id and found_target and run_id:
|
||||
gateway_resolved, _head, _total = resolve_gateway_pending_run(
|
||||
sid, approval_id, run_id, choice
|
||||
)
|
||||
elif not approval_id:
|
||||
gateway_resolved = resolve_gateway_approval(sid, choice, resolve_all=False) or 0
|
||||
# Keep the historical no-id response path truthy for old clients/tests while
|
||||
@@ -28588,6 +28725,16 @@ def _handle_session_import_cli(handler, body):
|
||||
if requested_profile and not _profiles_match(existing_profile, requested_profile):
|
||||
return bad(handler, "Session not found in CLI store", 404)
|
||||
elif not _session_visible_to_active_profile(existing_profile, handler):
|
||||
# #7710: same contract as the detail-load endpoint —
|
||||
# 409 ``session_profile_mismatch`` for a known other
|
||||
# profile, 404 only for the None-profile self-heal path.
|
||||
if existing_profile:
|
||||
return j(handler, {
|
||||
"error": "Session belongs to a different profile",
|
||||
"code": "session_profile_mismatch",
|
||||
"session_id": sid,
|
||||
"profile": existing_profile,
|
||||
}, status=409)
|
||||
return bad(handler, "Session not found in CLI store", 404)
|
||||
refresh_profile = requested_profile or existing_profile
|
||||
cli_meta = _resolve_cli_import_metadata(
|
||||
|
||||
+34
-1
@@ -45,6 +45,14 @@ TERMINAL_SSE_EVENTS = frozenset({"done", "cancel", "apperror", "error", "stream_
|
||||
SSE_RELAY_CLOSE_EVENTS = frozenset({"stream_end", "cancel", "apperror", "error"})
|
||||
# Back-compat alias used by older call sites / tests.
|
||||
_TERMINAL_SSE_EVENTS = TERMINAL_SSE_EVENTS
|
||||
# Events that are live-UI-only telemetry with no recovery value in the run
|
||||
# journal. They are skipped at WRITE time (never durably journaled, so they
|
||||
# cannot bloat the journal on marathon runs) and filtered at REPLAY time (so
|
||||
# legacy journals that already contain a backlog never stream it to a
|
||||
# reconnecting browser tab). Readers deliberately do NOT filter: cursor math
|
||||
# (``cursor_event_missing`` bound) and the offline-gap coverage check count
|
||||
# journal seqs and must keep seeing every row.
|
||||
REPLAY_SKIPPED_SSE_EVENTS = frozenset({"metering"})
|
||||
_FSYNC_MODE_ENV = "HERMES_WEBUI_RUN_JOURNAL_FSYNC"
|
||||
_FSYNC_MODE_EAGER = "eager"
|
||||
_FSYNC_MODE_TERMINAL_ONLY = "terminal-only"
|
||||
@@ -446,7 +454,16 @@ class RunJournalWriter:
|
||||
self._path = _run_path(self.session_id, self.run_id, session_dir=self.session_dir)
|
||||
self._lock = _lock_for(self._path)
|
||||
|
||||
def append_sse_event(self, event_name: str, payload=None) -> dict:
|
||||
def append_sse_event(self, event_name: str, payload=None) -> dict | None:
|
||||
# Live-UI-only telemetry (metering) has no recovery value in the journal:
|
||||
# nothing reads those rows back for recovery, and journaling them at ~10 Hz
|
||||
# on marathon runs balloons the durable file (12+ MB of a single 18 MB run
|
||||
# was metering). Skip the write entirely and return None so callers'
|
||||
# journal-id plumbing (``(journaled or {}).get("event_id")``) is untouched.
|
||||
# Not reserving a seq keeps the remaining journaled seqs contiguous, which
|
||||
# the offline-gap coverage and replay-cursor contiguity checks rely on.
|
||||
if str(event_name or "").strip() in REPLAY_SKIPPED_SSE_EVENTS:
|
||||
return None
|
||||
# Draw from the shared module-level seq cache under the per-path lock so
|
||||
# this writer and any direct append_run_event() call on the same path
|
||||
# agree on one monotonic, gapless sequence.
|
||||
@@ -462,6 +479,22 @@ class RunJournalWriter:
|
||||
)
|
||||
|
||||
|
||||
def journal_replay_visible(event) -> bool:
|
||||
"""Return True when a journal row should be streamed to a reconnecting tab.
|
||||
|
||||
Live-UI-only telemetry rows (see ``REPLAY_SKIPPED_SSE_EVENTS``) carry no
|
||||
recovery value — replaying a metering backlog only re-paints a stale TPS
|
||||
number while multiplying the reconnect burst size. Writers no longer journal
|
||||
them, but legacy journals may already contain them, so the replay emit sites
|
||||
filter through this predicate. Non-dict rows are passed through (visible) so
|
||||
an unexpected shape can never silently swallow user-visible output.
|
||||
"""
|
||||
if not isinstance(event, dict):
|
||||
return True
|
||||
name = str(event.get("event") or event.get("type") or "")
|
||||
return name not in REPLAY_SKIPPED_SSE_EVENTS
|
||||
|
||||
|
||||
def read_run_events(
|
||||
session_id: str,
|
||||
run_id: str,
|
||||
|
||||
+2
-2
@@ -7950,7 +7950,7 @@ def _extract_tool_calls_from_messages(messages, live_tool_calls=None):
|
||||
pending_names[tid] = part.get('name', '')
|
||||
pending_args[tid] = part.get('input', {})
|
||||
pending_asst_idx[tid] = msg_idx
|
||||
for tc in m.get('tool_calls', []):
|
||||
for tc in m.get('tool_calls') or []:
|
||||
if not isinstance(tc, dict):
|
||||
continue
|
||||
tid = tc.get('id', '') or tc.get('call_id', '')
|
||||
@@ -9600,7 +9600,7 @@ def _run_agent_streaming(
|
||||
except Exception:
|
||||
logger.debug("Failed to note event_id %s for stream %s", event_id, stream_id, exc_info=True)
|
||||
try:
|
||||
queue_item = (event, data, event_id) if event_id and hasattr(q, "subscribe_with_snapshot") else (event, data)
|
||||
queue_item = (event, data, event_id) if hasattr(q, "subscribe_with_snapshot") else (event, data)
|
||||
q.put_nowait(queue_item)
|
||||
except Exception:
|
||||
logger.debug("Failed to put event to queue")
|
||||
|
||||
+105
-22
@@ -8,7 +8,12 @@ import tempfile
|
||||
from pathlib import Path
|
||||
|
||||
from api.config import MAX_UPLOAD_BYTES, STATE_DIR
|
||||
from api.helpers import j
|
||||
from api.helpers import (
|
||||
arm_connection_close_if_body_pending,
|
||||
j,
|
||||
unreadable_content_length,
|
||||
unsupported_transfer_encoding,
|
||||
)
|
||||
from api.models import get_session
|
||||
from api.profiles import _profiles_match, get_active_profile_name as _get_active_profile_name
|
||||
from api.workspace import (
|
||||
@@ -45,6 +50,89 @@ def _max_extracted_bytes() -> int:
|
||||
_MAX_EXTRACTED_BYTES = 10 * MAX_UPLOAD_BYTES
|
||||
|
||||
|
||||
def _reject_before_read(handler, message, status):
|
||||
"""Reject without reading the body; close so unread bytes can't poison HTTP/1.1 reuse.
|
||||
|
||||
Framing decides, exactly as at the other reject-before-read sites: every
|
||||
rejection raised by the preflight above (any ``Transfer-Encoding``, an
|
||||
unreadable or conflicting ``Content-Length``, an over-cap length) already
|
||||
declares a body and still closes. Only the boundary/`parse_multipart` rejection
|
||||
can fire on a request that declares none, and closing there dropped a healthy
|
||||
pooled connection -- on the wire, ``POST /api/upload`` with
|
||||
``Content-Type: application/json`` and no ``Content-Length`` answered 400
|
||||
"No boundary in Content-Type" WITH ``Connection: close`` and the pipelined
|
||||
``GET /api/health/agent`` was never served.
|
||||
"""
|
||||
arm_connection_close_if_body_pending(handler)
|
||||
return j(handler, {'error': message}, status=status)
|
||||
|
||||
|
||||
class _RejectBeforeRead(ValueError):
|
||||
"""A rejection raised before any byte of the request body was consumed.
|
||||
|
||||
Carries the status the caller must answer with. Subclasses ``ValueError`` so
|
||||
that if one ever escapes an unusual call path, each handler's existing
|
||||
``except ValueError`` still answers 400 with the same message instead of
|
||||
turning it into a 500.
|
||||
"""
|
||||
|
||||
def __init__(self, message: str, status: int):
|
||||
super().__init__(message)
|
||||
self.message = message
|
||||
self.status = status
|
||||
|
||||
|
||||
def _read_multipart_or_reject(handler):
|
||||
"""Framing preflight + body read, shared by all four multipart handlers.
|
||||
|
||||
Returns ``(fields, files)``. Raises ``_RejectBeforeRead`` when the request
|
||||
must be refused before its body is read -- the caller answers through
|
||||
``_reject_before_read()``, which arms ``Connection: close`` first, because the
|
||||
declared body is still queued in ``rfile`` and a reused HTTP/1.1 connection
|
||||
would parse those bytes as the next request line.
|
||||
|
||||
Framing is judged from the headers, not from a present ``Content-Length``:
|
||||
|
||||
* ANY ``Transfer-Encoding`` is refused, ``identity`` included: ``http.server``
|
||||
decodes none of them (``rfile`` is the raw socket) and RFC 9112 section 6.3
|
||||
says a request whose final coding is not ``chunked`` has a length no
|
||||
recipient can determine. A coded request carries no usable Content-Length,
|
||||
so the old code read it as length 0, found no parts, and answered "No file
|
||||
field in request" with keep-alive intact -- leaving every payload byte on
|
||||
the socket to poison the next pooled request. A Content-Length sent BESIDE a
|
||||
coding does not rescue it: that pairing is the smuggling shape 6.3 rejects,
|
||||
and the coding still hides how the body is framed.
|
||||
* a non-numeric or oversized Content-Length cannot be read either, and
|
||||
``parse_multipart()`` validates the boundary and the length before its
|
||||
first ``rfile.read()`` — so every raise below is still pre-read.
|
||||
* two DISAGREEING Content-Length headers read back as whichever came first,
|
||||
so ``Content-Length: 0`` ahead of the real length made the body invisible
|
||||
and left every byte of it on the socket. There is no honest length to
|
||||
drain, so the request is refused before ``rfile`` is touched.
|
||||
"""
|
||||
content_type = handler.headers.get('Content-Type', '')
|
||||
coding = unsupported_transfer_encoding(handler)
|
||||
if coding is not None:
|
||||
raise _RejectBeforeRead(
|
||||
f'Unsupported Transfer-Encoding: {coding} '
|
||||
'(uploads must send an undecoded body with a Content-Length)',
|
||||
411,
|
||||
)
|
||||
if unreadable_content_length(handler):
|
||||
raise _RejectBeforeRead('Invalid Content-Length', 400)
|
||||
try:
|
||||
content_length = int(handler.headers.get('Content-Length', 0) or 0)
|
||||
except (TypeError, ValueError):
|
||||
raise _RejectBeforeRead('Invalid Content-Length', 400) from None
|
||||
if content_length > MAX_UPLOAD_BYTES:
|
||||
raise _RejectBeforeRead(
|
||||
f'File too large (max {MAX_UPLOAD_BYTES//1024//1024}MB)', 413)
|
||||
try:
|
||||
return parse_multipart(handler.rfile, content_type, content_length)
|
||||
except ValueError as exc:
|
||||
raise _RejectBeforeRead(str(exc), 400) from None
|
||||
|
||||
|
||||
def parse_multipart(rfile, content_type, content_length) -> tuple:
|
||||
import re as _re, email.parser as _ep
|
||||
# Imported locally (not just module-level) so the function stays
|
||||
@@ -206,11 +294,10 @@ def _write_office_upload_sidecar(workspace: Path, dest: Path, file_bytes: bytes)
|
||||
def handle_upload(handler):
|
||||
import traceback as _tb
|
||||
try:
|
||||
content_type = handler.headers.get('Content-Type', '')
|
||||
content_length = int(handler.headers.get('Content-Length', 0) or 0)
|
||||
if content_length > MAX_UPLOAD_BYTES:
|
||||
return j(handler, {'error': f'File too large (max {MAX_UPLOAD_BYTES//1024//1024}MB)'}, status=413)
|
||||
fields, files = parse_multipart(handler.rfile, content_type, content_length)
|
||||
try:
|
||||
fields, files = _read_multipart_or_reject(handler)
|
||||
except _RejectBeforeRead as rejection:
|
||||
return _reject_before_read(handler, rejection.message, rejection.status)
|
||||
session_id = fields.get('session_id', '')
|
||||
if 'file' not in files:
|
||||
return j(handler, {'error': 'No file field in request'}, status=400)
|
||||
@@ -396,11 +483,10 @@ def handle_upload_extract(handler):
|
||||
"""Handle archive upload and extraction."""
|
||||
import traceback as _tb
|
||||
try:
|
||||
content_type = handler.headers.get('Content-Type', '')
|
||||
content_length = int(handler.headers.get('Content-Length', 0) or 0)
|
||||
if content_length > MAX_UPLOAD_BYTES:
|
||||
return j(handler, {'error': f'File too large (max {MAX_UPLOAD_BYTES//1024//1024}MB)'}, status=413)
|
||||
fields, files = parse_multipart(handler.rfile, content_type, content_length)
|
||||
try:
|
||||
fields, files = _read_multipart_or_reject(handler)
|
||||
except _RejectBeforeRead as rejection:
|
||||
return _reject_before_read(handler, rejection.message, rejection.status)
|
||||
session_id = fields.get('session_id', '')
|
||||
if 'file' not in files:
|
||||
return j(handler, {'error': 'No file field in request'}, status=400)
|
||||
@@ -428,11 +514,10 @@ def handle_transcribe(handler):
|
||||
import traceback as _tb
|
||||
temp_path = None
|
||||
try:
|
||||
content_type = handler.headers.get('Content-Type', '')
|
||||
content_length = int(handler.headers.get('Content-Length', 0) or 0)
|
||||
if content_length > MAX_UPLOAD_BYTES:
|
||||
return j(handler, {'error': f'File too large (max {MAX_UPLOAD_BYTES//1024//1024}MB)'}, status=413)
|
||||
fields, files = parse_multipart(handler.rfile, content_type, content_length)
|
||||
try:
|
||||
fields, files = _read_multipart_or_reject(handler)
|
||||
except _RejectBeforeRead as rejection:
|
||||
return _reject_before_read(handler, rejection.message, rejection.status)
|
||||
if 'file' not in files:
|
||||
return j(handler, {'error': 'No file field in request'}, status=400)
|
||||
filename, file_bytes = files['file']
|
||||
@@ -612,12 +697,10 @@ def handle_workspace_upload(handler):
|
||||
"""
|
||||
import traceback as _tb
|
||||
try:
|
||||
content_type = handler.headers.get('Content-Type', '')
|
||||
content_length = int(handler.headers.get('Content-Length', 0) or 0)
|
||||
if content_length > MAX_UPLOAD_BYTES:
|
||||
return j(handler, {'error': f'File too large (max {MAX_UPLOAD_BYTES//1024//1024}MB)'}, status=413)
|
||||
|
||||
fields, files = parse_multipart(handler.rfile, content_type, content_length)
|
||||
try:
|
||||
fields, files = _read_multipart_or_reject(handler)
|
||||
except _RejectBeforeRead as rejection:
|
||||
return _reject_before_read(handler, rejection.message, rejection.status)
|
||||
session_id = fields.get('session_id', '')
|
||||
subpath = fields.get('path', '')
|
||||
|
||||
|
||||
@@ -216,7 +216,7 @@ when integrating with the live chat SSE relay.
|
||||
| `cancel` | Run cancelled |
|
||||
| `done` | Turn finalized (session payload); title/`stream_end` may follow |
|
||||
| `stream_end` | SSE fence — close the client EventSource |
|
||||
| `metering` | Token/cost metering snapshot |
|
||||
| `metering` | Best-effort live token/cost metering snapshot; not journal-replayed |
|
||||
| `context_status` | Context window / usage status |
|
||||
| `goal` | Goal / plan card update |
|
||||
| `goal_continue` | Goal continuation signal |
|
||||
@@ -261,6 +261,31 @@ A future implementation must replay from the run journal via the existing
|
||||
`_replay_run_journal()` path (in `api/routes.py`) and fall back to the
|
||||
snapshot mechanism when journal entries are unavailable for a given cursor.
|
||||
|
||||
### Live-only metering (implemented replay behavior)
|
||||
|
||||
`metering` is best-effort live telemetry, not durable recovery content.
|
||||
`RunJournalWriter.append_sse_event()` returns `None` for metering without
|
||||
writing a row or allocating a sequence number. Other journaled events retain
|
||||
contiguous sequence numbers. Local-agent and gateway producers carry an
|
||||
explicit per-frame event ID, including `None`, through `StreamChannel`;
|
||||
an unjournaled metering frame emits no new SSE `id:` and must not borrow the
|
||||
previous frame's journal ID. The last-ID fallback is limited to legacy
|
||||
two-element queue entries.
|
||||
|
||||
Existing journals are not rewritten. Direct readers (`read_run_events()` and
|
||||
`read_session_run_events()`) retain legacy metering rows for cursor validation
|
||||
and gap/coverage checks. Both run-level and per-session journal replay emitters
|
||||
apply `journal_replay_visible()` to omit those rows from reconnect delivery.
|
||||
Consequently, replayed legacy event IDs can have gaps where metering was
|
||||
filtered; clients must not infer missing durable content from those gaps.
|
||||
Metering missed while disconnected is not recovered from the journal; fresh
|
||||
live updates remain available. This exception does not remove token, reasoning,
|
||||
tool, or terminal events from durable replay.
|
||||
|
||||
Coverage: `tests/test_run_journal.py`, `tests/test_run_journal_routes.py`,
|
||||
`tests/test_issue4812_session_sse_stream.py`, and
|
||||
`tests/test_stage364_opus_live_sse_event_id.py`.
|
||||
|
||||
## Snapshot fallback
|
||||
|
||||
When the `Last-Event-ID` cursor is evicted, expired, unknown, or refers to a
|
||||
|
||||
@@ -241,6 +241,44 @@ For a foreground `python3 bootstrap.py`, stop it with Ctrl-C and start it again.
|
||||
|
||||
---
|
||||
|
||||
## "OpenCode Go model picker shows a model that errors when you send" (or is missing newly released models)
|
||||
|
||||
**Symptom.** One of two directions:
|
||||
|
||||
1. A model selected from the OpenCode Go group fails on the first message (`model not found`, `Model is unavailable`, or a region error), even though the picker offered it.
|
||||
2. A newly released OpenCode Go model does not appear in the picker at all, and must be typed into the Custom Model ID box.
|
||||
|
||||
**Why.** The Go picker follows the **live** Go-tier catalog (`https://opencode.ai/zen/go/v1/models`) whenever the installed Hermes Agent is v0.20.5 or newer. That endpoint advertises a superset of what a given tier, key, or region can actually serve — an id can be listed and still fail on send. Conversely, an id the endpoint serves but the WebUI's static fallback list predates is missing when the live path is unavailable (Agent older than v0.20.5, probe failure, or offline); the static list mirrors Hermes core's curated Go catalog and can lag new releases by design.
|
||||
|
||||
**Diagnostic.** Check which path is feeding your picker:
|
||||
|
||||
```bash
|
||||
hermes --version # live path requires >= 0.20.5
|
||||
curl -sS https://opencode.ai/zen/go/v1/models \
|
||||
-H "Authorization: Bearer $OPENCODE_GO_API_KEY" | head -50
|
||||
```
|
||||
|
||||
Interpret the two together:
|
||||
|
||||
- **Failing model absent from the `curl` output** → it was delisted upstream (for example `ox-alpha-free`, removed from the relay on 2026-09-09). A picker can still offer it from a **stale catalog merge**: Hermes core merges its own curated Go list into the live result, and every Agent release *through v0.21.1 (tag `v2026.9.7`)* still carries the delisted id in that list — verified at runtime against v0.21.0, where the live path serves 37 ids including `ox-alpha-free`. The removal is committed on core `main` (2026-09-09 sync) but **no released Agent version includes it yet** — and `main` reports the same `0.21.1` version string as the stale tag, so a version number alone cannot tell you whether the fix is in. Until a release notes the 2026-09-09 catalog sync, the **verified workaround is the config allowlist** (below); selecting the dead entry is harmless to other models (it errors on send, nothing else).
|
||||
- **Failing model present in the `curl` output** → the relay lists it but your tier/region cannot serve it; the picker is behaving correctly. Pin the models you actually use with an explicit allowlist in `config.yaml`, which takes precedence over both the live catalog and the fallback:
|
||||
|
||||
```yaml
|
||||
providers:
|
||||
opencode-go:
|
||||
models:
|
||||
- kimi-k3
|
||||
- glm-5.3
|
||||
```
|
||||
|
||||
A lighter per-provider exclude capability is tracked in #7507.
|
||||
- **Missing new model, Agent ≥ v0.20.5** → the live catalog is the source; refresh or check the endpoint with the `curl` above (cold rebuilds are also bounded by a 4-second foreground budget — the first picker open after a restart can serve the last-known list while the live rebuild finishes in the background, so re-open the picker once before concluding it's stale).
|
||||
- **Missing new model, Agent older than v0.20.5** → the static fallback is serving by design; upgrade the Agent to ≥ v0.20.5 so the picker reads the live catalog.
|
||||
|
||||
**When to file a bug.** File a WebUI bug if a model fails on send *and* appears in the `curl` output for your key (a routing problem), or if a model is missing with Agent ≥ v0.20.5 and the live catalog reachable (fallback used when it should not be).
|
||||
|
||||
---
|
||||
|
||||
## Other troubleshooting
|
||||
|
||||
This document grows over time. If a recurring failure mode isn't covered here yet, add it via PR. The format for each entry: **Symptom → Why → Diagnostic commands → Fix → When to file a bug**.
|
||||
|
||||
@@ -100,10 +100,11 @@ from urllib.parse import urlparse
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
from api.request_logging import emit_request_log
|
||||
from api.auth import check_auth, reset_trusted_auth_request_state
|
||||
from api.auth import check_auth_or_close, reset_trusted_auth_request_state
|
||||
from api.config import HOST, PORT, STATE_DIR, SESSION_DIR, DEFAULT_WORKSPACE
|
||||
from api.helpers import (
|
||||
j,
|
||||
advertise_connection_close,
|
||||
get_profile_cookie,
|
||||
_build_csp_report_only_policy,
|
||||
_CLIENT_DISCONNECT_ERRORS,
|
||||
@@ -332,6 +333,7 @@ class Handler(BaseHTTPRequestHandler):
|
||||
extra_frame_src = getattr(self, "_csp_extra_frame_src", None)
|
||||
self.send_header("Content-Security-Policy-Report-Only", self.csp_report_only_policy(extra_connect_src, extra_frame_src))
|
||||
self.send_header("Report-To", self._CSP_REPORT_TO)
|
||||
advertise_connection_close(self) # tell the client when the socket dies
|
||||
super().end_headers()
|
||||
|
||||
def log_message(self, fmt, *args): pass # suppress default Apache-style log
|
||||
@@ -376,7 +378,8 @@ class Handler(BaseHTTPRequestHandler):
|
||||
set_request_profile(cookie_profile)
|
||||
try:
|
||||
parsed = urlparse(self.path)
|
||||
if not check_auth(self, parsed): return
|
||||
# Body-pending-aware: a body-bearing GET failing auth would poison reuse (#7550).
|
||||
if not check_auth_or_close(self, parsed): return
|
||||
result = handle_get(self, parsed)
|
||||
if result is False:
|
||||
return j(self, {'error': 'not found'}, status=404)
|
||||
@@ -401,10 +404,8 @@ class Handler(BaseHTTPRequestHandler):
|
||||
set_request_profile(cookie_profile)
|
||||
try:
|
||||
parsed = urlparse(self.path)
|
||||
_is_csp_report_post = (
|
||||
parsed.path == "/api/csp-report" and self.command == "POST"
|
||||
)
|
||||
if not _is_csp_report_post and not check_auth(self, parsed): return
|
||||
_is_csp_report_post = parsed.path == "/api/csp-report" and self.command == "POST"
|
||||
if not _is_csp_report_post and not check_auth_or_close(self, parsed): return
|
||||
result = route_func(self, parsed)
|
||||
if result is False:
|
||||
return j(self, {'error': 'not found'}, status=404)
|
||||
|
||||
@@ -991,6 +991,12 @@ async function resumeManualCompressionForSession(sid){
|
||||
// No active compression job or transient server error — not a real failure.
|
||||
// 404: route missed or session gone; 5xx: backend exception during status check.
|
||||
if(e&&(!e.status||e.status===404||e.status>=500)) return;
|
||||
// #7710: a cross-profile refusal now arrives as 409
|
||||
// (``session_profile_mismatch``) where it used to be a 404 that hit the
|
||||
// benign early-return above. It is not a compression failure, so do not
|
||||
// render the error state or locally settle the compression UI.
|
||||
if(e&&e.status===409&&typeof _sessionProfileMismatchFromError==='function'
|
||||
&&_sessionProfileMismatchFromError(e)) return;
|
||||
if(S.session&&S.session.session_id===sid&&typeof setCompressionUi==='function'){
|
||||
const visibleMessages=_manualCompressionVisibleMessages();
|
||||
setCompressionUi({
|
||||
|
||||
+14
-14
@@ -1770,7 +1770,7 @@ const LOCALES = {
|
||||
yolo_disabled: 'YOLO mode OFF',
|
||||
yolo_pill_label: 'YOLO',
|
||||
yolo_pill_title_active: 'YOLO mode active — click to disable',
|
||||
approval_skip_all: '⚡ Skip all this session',
|
||||
approval_skip_all: 'Skip all this session',
|
||||
approval_skip_all_title: 'Skip all approval prompts for this session',
|
||||
// composer action tooltips
|
||||
composer_send: 'Send message',
|
||||
@@ -3572,7 +3572,7 @@ const LOCALES = {
|
||||
yolo_disabled: 'Modalità YOLO OFF',
|
||||
yolo_pill_label: 'YOLO',
|
||||
yolo_pill_title_active: 'Modalità YOLO attiva — clicca per disabilitare',
|
||||
approval_skip_all: '⚡ Salta tutto questa sessione',
|
||||
approval_skip_all: 'Salta tutto questa sessione',
|
||||
approval_skip_all_title: 'Salta tutte le richieste di approvazione per questa sessione',
|
||||
// composer action tooltips
|
||||
composer_send: 'Invia messaggio',
|
||||
@@ -5382,7 +5382,7 @@ const LOCALES = {
|
||||
yolo_disabled: 'YOLO モード OFF',
|
||||
yolo_pill_label: 'YOLO',
|
||||
yolo_pill_title_active: 'YOLO モード有効 — クリックで無効化',
|
||||
approval_skip_all: '⚡ このセッションはすべてスキップ',
|
||||
approval_skip_all: 'このセッションはすべてスキップ',
|
||||
approval_skip_all_title: 'このセッションのすべての承認プロンプトをスキップ',
|
||||
// composer action tooltips
|
||||
composer_send: 'メッセージを送信',
|
||||
@@ -7063,7 +7063,7 @@ const LOCALES = {
|
||||
yolo_disabled: 'YOLO режим ВЫКЛ',
|
||||
yolo_pill_label: 'YOLO',
|
||||
yolo_pill_title_active: 'YOLO режим активен — нажмите для отключения',
|
||||
approval_skip_all: '⚡ Пропустить все в этом сеансе',
|
||||
approval_skip_all: 'Пропустить все в этом сеансе',
|
||||
approval_skip_all_title: 'Пропустить все запросы подтверждения в этом сеансе',
|
||||
// composer action tooltips
|
||||
composer_send: 'Отправить сообщение',
|
||||
@@ -8830,7 +8830,7 @@ const LOCALES = {
|
||||
yolo_disabled: 'Modo YOLO desactivado',
|
||||
yolo_pill_label: 'YOLO',
|
||||
yolo_pill_title_active: 'Modo YOLO activo — haz clic para desactivar',
|
||||
approval_skip_all: '⚡ Saltar todo en esta sesión',
|
||||
approval_skip_all: 'Saltar todo en esta sesión',
|
||||
approval_skip_all_title: 'Saltar todas las solicitudes de aprobación en esta sesión',
|
||||
// composer action tooltips
|
||||
composer_send: 'Enviar mensaje',
|
||||
@@ -10236,7 +10236,7 @@ const LOCALES = {
|
||||
yolo_disabled: 'YOLO-Modus AUS',
|
||||
yolo_pill_label: 'YOLO',
|
||||
yolo_pill_title_active: 'YOLO-Modus aktiv — Klicken zum Deaktivieren',
|
||||
approval_skip_all: '⚡ Alle in dieser Sitzung überspringen',
|
||||
approval_skip_all: 'Alle in dieser Sitzung überspringen',
|
||||
approval_skip_all_title: 'Alle Genehmigungsanfragen in dieser Sitzung überspringen',
|
||||
// composer action tooltips
|
||||
composer_send: 'Nachricht senden',
|
||||
@@ -12312,7 +12312,7 @@ const LOCALES = {
|
||||
yolo_disabled: 'YOLO 模式已关闭',
|
||||
yolo_pill_label: 'YOLO',
|
||||
yolo_pill_title_active: 'YOLO 模式激活 — 点击关闭',
|
||||
approval_skip_all: '⚡ 本次会话全部跳过',
|
||||
approval_skip_all: '本次会话全部跳过',
|
||||
approval_skip_all_title: '跳过本次会话的所有审批提示',
|
||||
// composer action tooltips
|
||||
composer_send: '发送消息',
|
||||
@@ -14208,7 +14208,7 @@ const LOCALES = {
|
||||
yolo_disabled: 'YOLO 模式已關閉',
|
||||
yolo_pill_label: 'YOLO',
|
||||
yolo_pill_title_active: 'YOLO 模式已啟用,點選即可關閉',
|
||||
approval_skip_all: '⚡ 本次工作階段全部跳過',
|
||||
approval_skip_all: '本次工作階段全部跳過',
|
||||
approval_skip_all_title: '略過本次工作階段的所有核准提示',
|
||||
// composer action tooltips
|
||||
composer_send: '傳送訊息',
|
||||
@@ -17540,7 +17540,7 @@ const LOCALES = {
|
||||
yolo_disabled: 'YOLO 모드 꺼짐',
|
||||
yolo_pill_label: 'YOLO',
|
||||
yolo_pill_title_active: 'YOLO 모드 활성 — 클릭하여 비활성화',
|
||||
approval_skip_all: '⚡ 이 세션에서 모두 건너뛰기',
|
||||
approval_skip_all: '이 세션에서 모두 건너뛰기',
|
||||
approval_skip_all_title: '이 세션의 모든 승인 요청 건너뛰기',
|
||||
// composer action tooltips
|
||||
composer_send: '메시지 보내기',
|
||||
@@ -19412,7 +19412,7 @@ const LOCALES = {
|
||||
yolo_disabled: 'Mode YOLO désactivé',
|
||||
yolo_pill_label: 'YOLO',
|
||||
yolo_pill_title_active: 'Mode YOLO actif — cliquez pour désactiver',
|
||||
approval_skip_all: '⚡ Ignorer tout cette session',
|
||||
approval_skip_all: 'Ignorer tout cette session',
|
||||
approval_skip_all_title: 'Ignorer toutes les demandes d\'approbation pour cette session',
|
||||
composer_send: 'Envoyer le message',
|
||||
composer_queue: 'Mettre en file d\'attente',
|
||||
@@ -19540,7 +19540,7 @@ const LOCALES = {
|
||||
add_failed: 'Přidání selhalo: ',
|
||||
agent_soul: 'Duch agenta',
|
||||
all_uploads_failed: (n) => `Všech ${n} nahrávání selhalo`,
|
||||
approval_skip_all: '⚡ Přeskočit vše pro tuto relaci',
|
||||
approval_skip_all: 'Přeskočit vše pro tuto relaci',
|
||||
approval_skip_all_title: 'Přeskočit všechny výzvy ke schválení pro tuto relaci',
|
||||
archive_extracted: (n, c) => `Rozbaleno ${n} souborů z ${c} archivů`,
|
||||
auth_ack_save_failed: 'Nepodařilo se aktualizovat potvrzení: ',
|
||||
@@ -22894,7 +22894,7 @@ const LOCALES = {
|
||||
yolo_disabled: 'YOLO modu KAPALI',
|
||||
yolo_pill_label: 'YOLO',
|
||||
yolo_pill_title_active: 'YOLO modu etkin — devre dışı bırakmak için tıklayın',
|
||||
approval_skip_all: '⚡ Tüm bu oturumu atla',
|
||||
approval_skip_all: 'Tüm bu oturumu atla',
|
||||
approval_skip_all_title: 'Bu oturuma ilişkin tüm onay istemlerini atla',
|
||||
// composer action tooltips
|
||||
composer_send: 'Mesaj gönder',
|
||||
@@ -24778,7 +24778,7 @@ const LOCALES = {
|
||||
yolo_disabled: 'Tryb YOLO WYŁĄCZONY',
|
||||
yolo_pill_label: 'YOLO',
|
||||
yolo_pill_title_active: 'Tryb YOLO aktywny — kliknij, aby wyłączyć',
|
||||
approval_skip_all: '⚡ Pomiń wszystkie w tej sesji',
|
||||
approval_skip_all: 'Pomiń wszystkie w tej sesji',
|
||||
approval_skip_all_title: 'Pomiń wszystkie monity o aprobatę w tej sesji',
|
||||
// composer action tooltips
|
||||
composer_send: 'Wyślij wiadomość',
|
||||
@@ -26411,7 +26411,7 @@ const LOCALES = {
|
||||
approval_gateway_unsupported: 'Phê duyệt cần gateway mới hơn. Hãy nâng cấp Hermes gateway đang kết nối để bật tính năng này.',
|
||||
approval_gateway_unsupported_label: 'Không hỗ trợ phê duyệt',
|
||||
approval_pending_count: (n) => `1 trong ${n} đang chờ`,
|
||||
approval_skip_all: '⚡ Bỏ qua tất cả trong phiên này',
|
||||
approval_skip_all: 'Bỏ qua tất cả trong phiên này',
|
||||
approval_skip_all_title: 'Bỏ qua mọi yêu cầu phê duyệt cho phiên này',
|
||||
checkpoint_title: 'Checkpoint',
|
||||
checkpoint_empty: 'Không tìm thấy checkpoint nào cho workspace này.',
|
||||
|
||||
+15
-7
@@ -7,7 +7,11 @@ document.addEventListener('DOMContentLoaded', function () {
|
||||
var input = document.getElementById('pw');
|
||||
var passkeyBtn = document.getElementById('passkey-login');
|
||||
|
||||
if (!form || !input) return;
|
||||
// #7056: the password input is absent on a passwordless deployment (OIDC-only,
|
||||
// or passkey-only). The form itself still renders and carries the i18n data
|
||||
// attributes, and the passkey button below must still be wired up, so only the
|
||||
// form is required here — every password-specific path is guarded individually.
|
||||
if (!form) return;
|
||||
|
||||
var invalidPw = form.getAttribute('data-invalid-pw') || 'Invalid password';
|
||||
var connFailed = form.getAttribute('data-conn-failed') || 'Connection failed';
|
||||
@@ -59,6 +63,8 @@ document.addEventListener('DOMContentLoaded', function () {
|
||||
|
||||
async function doLogin(e) {
|
||||
e.preventDefault();
|
||||
// No password input on a passwordless deployment: nothing to submit.
|
||||
if (!input) return;
|
||||
var pw = input.value;
|
||||
hideErr();
|
||||
try {
|
||||
@@ -147,12 +153,14 @@ document.addEventListener('DOMContentLoaded', function () {
|
||||
passkeyBtn.addEventListener('click', doPasskeyLogin);
|
||||
}
|
||||
|
||||
input.addEventListener('keydown', function (e) {
|
||||
if (e.key === 'Enter') {
|
||||
e.preventDefault();
|
||||
doLogin(e);
|
||||
}
|
||||
});
|
||||
if (input) {
|
||||
input.addEventListener('keydown', function (e) {
|
||||
if (e.key === 'Enter') {
|
||||
e.preventDefault();
|
||||
doLogin(e);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
// On page load, probe the server so we can distinguish "can't reach server"
|
||||
// (Tailscale off, wrong network) from "session expired / need to log in".
|
||||
|
||||
@@ -9009,6 +9009,19 @@ async function respondClarify(response) {
|
||||
// not tear B down on A's late 409. The SSE/poll path will re-render the
|
||||
// next prompt's card from scratch via ``showClarifyCard`` either way.
|
||||
if (e && e.status === 409) {
|
||||
// #7710: a cross-profile refusal now also arrives as 409
|
||||
// (``session_profile_mismatch``). The prompt is NOT expired — the write
|
||||
// was refused because the session belongs to another profile. Treating
|
||||
// it as expired would hide a live clarification card and mislabel the
|
||||
// cause, so leave the card standing and report the real reason.
|
||||
if (typeof _sessionProfileMismatchFromError === 'function'
|
||||
&& _sessionProfileMismatchFromError(e)) {
|
||||
_clarifySetControlsDisabled(false, false);
|
||||
if (typeof setStatus === "function") {
|
||||
setStatus("Clarify: session belongs to a different profile");
|
||||
}
|
||||
return;
|
||||
}
|
||||
if (_clarifyId === clarifyId) {
|
||||
// Same card still showing — dismiss it and rescue the typed draft.
|
||||
// Order matters: ``_stashClarifyDraft`` (called from
|
||||
|
||||
@@ -508,6 +508,16 @@ async function startCompressionRecovery(btn){
|
||||
const composer=$('msg');
|
||||
if(composer&&typeof composer.focus==='function') composer.focus();
|
||||
}catch(e){
|
||||
// #7710: a cross-profile refusal now also arrives as 409
|
||||
// (``session_profile_mismatch``). That is NOT a stale recovery action —
|
||||
// the card is still valid, the request was simply refused because the
|
||||
// session belongs to another profile. Retiring it would hide a live card
|
||||
// and show a false "conversation already moved on" note.
|
||||
if(e&&e.status===409&&typeof _sessionProfileMismatchFromError==='function'
|
||||
&&_sessionProfileMismatchFromError(e)){
|
||||
if(typeof setStatus==='function') setStatus('Session belongs to a different profile');
|
||||
return;
|
||||
}
|
||||
// A 409 means this session no longer has an active recovery action (the
|
||||
// session already moved on — e.g. a substantive prompt cleared it). The
|
||||
// persisted card in the transcript is stale, so retire it and show a neutral
|
||||
|
||||
@@ -241,8 +241,18 @@ def test_anchor_scene_persistence_rejects_cross_profile_write(tmp_path, monkeypa
|
||||
SimpleNamespace(command="POST"),
|
||||
SimpleNamespace(path="/api/session/anchor-scene"),
|
||||
) is True
|
||||
assert captured.get("status") == 404
|
||||
assert "payload" not in captured # success j() never called
|
||||
# #7710: the generic request-guard now mirrors the detail-load
|
||||
# endpoint's contract — a session owned by a KNOWN other profile
|
||||
# yields 409 ``session_profile_mismatch`` so the client can offer
|
||||
# to switch to it (#5419). The 404 self-heal path is preserved for
|
||||
# the None-profile (unknown/legacy) case.
|
||||
assert captured.get("status") == 409, captured
|
||||
assert captured.get("payload") == {
|
||||
"error": "Session belongs to a different profile",
|
||||
"code": "session_profile_mismatch",
|
||||
"session_id": "foreignprofile1",
|
||||
"profile": "profile-b",
|
||||
}
|
||||
raw = json.loads((session_dir / "foreignprofile1.json").read_text(encoding="utf-8"))
|
||||
assert not raw.get("anchor_activity_scenes"), (
|
||||
"cross-profile request must NOT persist anchor_activity_scenes"
|
||||
|
||||
@@ -601,6 +601,85 @@ class TestApprovalHTTPEndpoints:
|
||||
r._gateway_queues.pop(sid, None)
|
||||
_STREAM_RUN_IDS.pop(stream_id, None)
|
||||
|
||||
def test_identity_v1_non_head_run_approval_settles_exact_producer(self, monkeypatch):
|
||||
"""A relayed non-head run approval must wake only its own parked waiter."""
|
||||
from api import route_approvals as ra
|
||||
from api import routes as r
|
||||
|
||||
sid = f"identity-v1-non-head-{uuid.uuid4().hex[:8]}"
|
||||
run_id = f"run-identity-v1-{uuid.uuid4().hex[:8]}"
|
||||
head = {
|
||||
"approval_id": "head-approval",
|
||||
"run_id": run_id,
|
||||
"command": "head command",
|
||||
"_gateway_agent_identity_v1": True,
|
||||
}
|
||||
target = {
|
||||
"approval_id": "target-approval",
|
||||
"run_id": run_id,
|
||||
"command": "target command",
|
||||
"_gateway_agent_identity_v1": True,
|
||||
}
|
||||
head_entry = _ApprovalEntry(dict(head))
|
||||
target_entry = _ApprovalEntry(dict(target))
|
||||
captured = {}
|
||||
relays = []
|
||||
|
||||
def fake_j(_handler, data, status=200, extra_headers=None):
|
||||
captured.update(payload=data, status=status)
|
||||
return data
|
||||
|
||||
def fake_respond(_self, got_run_id, got_approval_id, choice):
|
||||
relays.append((got_run_id, got_approval_id, choice))
|
||||
return {"resolved": 1}
|
||||
|
||||
monkeypatch.setattr(r, "j", fake_j)
|
||||
monkeypatch.setattr("api.runner_client.HttpRunnerClient.respond_approval", fake_respond)
|
||||
monkeypatch.setattr(
|
||||
"api.config.gateway_supports_approval_identity_v1", lambda *_args: True
|
||||
)
|
||||
with _lock:
|
||||
r._pending.pop(sid, None)
|
||||
r._gateway_queues[sid] = [head_entry, target_entry]
|
||||
try:
|
||||
ra.submit_gateway_pending_mirror(sid, dict(target_entry.data))
|
||||
mirror = ra.gateway_pending_mirror(
|
||||
sid, approval_id=target["approval_id"], run_id=run_id
|
||||
)
|
||||
assert mirror is not None
|
||||
|
||||
r._handle_approval_respond(
|
||||
object(),
|
||||
{
|
||||
"session_id": sid,
|
||||
"choice": "deny",
|
||||
"approval_id": target["approval_id"],
|
||||
"run_id": run_id,
|
||||
"mirror_token": mirror[ra._GATEWAY_MIRROR_TOKEN],
|
||||
},
|
||||
)
|
||||
|
||||
assert captured == {
|
||||
"payload": {"ok": True, "choice": "deny", "relayed": True},
|
||||
"status": 200,
|
||||
}
|
||||
assert relays == [(run_id, target["approval_id"], "deny")]
|
||||
assert target_entry.event.is_set()
|
||||
assert target_entry.result == "deny"
|
||||
assert not head_entry.event.is_set()
|
||||
with _lock:
|
||||
assert r._gateway_queues[sid] == [head_entry]
|
||||
for _ in range(2):
|
||||
with _lock:
|
||||
ra.reconcile_gateway_pending_mirror_locked(sid)
|
||||
assert ra.gateway_pending_mirror(
|
||||
sid, approval_id=target["approval_id"], run_id=run_id
|
||||
) is None
|
||||
finally:
|
||||
with _lock:
|
||||
r._pending.pop(sid, None)
|
||||
r._gateway_queues.pop(sid, None)
|
||||
|
||||
def test_gateway_mirror_without_run_id_with_one_producer_resolves_exactly(self, monkeypatch):
|
||||
"""A no-run mirror retires only after its exact local producer resolves."""
|
||||
from api import routes as r
|
||||
|
||||
@@ -227,14 +227,14 @@ class TestServerDisconnectHandling(unittest.TestCase):
|
||||
# Patch handle_get to raise BrokenPipeError
|
||||
import server as _server_mod
|
||||
orig_handle_get = _server_mod.handle_get
|
||||
orig_check_auth = _server_mod.check_auth
|
||||
orig_check_auth = _server_mod.check_auth_or_close
|
||||
_server_mod.handle_get = _fake_handle_get
|
||||
_server_mod.check_auth = lambda h, p: True
|
||||
_server_mod.check_auth_or_close = lambda h, p: True
|
||||
try:
|
||||
Handler.do_GET(handler)
|
||||
finally:
|
||||
_server_mod.handle_get = orig_handle_get
|
||||
_server_mod.check_auth = orig_check_auth
|
||||
_server_mod.check_auth_or_close = orig_check_auth
|
||||
|
||||
# send_response should NEVER be called for the 500 — client is gone
|
||||
handler.send_response.assert_not_called()
|
||||
@@ -248,12 +248,12 @@ class TestServerDisconnectHandling(unittest.TestCase):
|
||||
raise self._route_raises
|
||||
|
||||
import server as _server_mod
|
||||
orig_check_auth = _server_mod.check_auth
|
||||
_server_mod.check_auth = lambda h, p: True
|
||||
orig_check_auth = _server_mod.check_auth_or_close
|
||||
_server_mod.check_auth_or_close = lambda h, p: True
|
||||
try:
|
||||
Handler._handle_write(handler, _fake_route)
|
||||
finally:
|
||||
_server_mod.check_auth = orig_check_auth
|
||||
_server_mod.check_auth_or_close = orig_check_auth
|
||||
|
||||
handler.send_response.assert_not_called()
|
||||
|
||||
@@ -286,9 +286,9 @@ class TestServerDisconnectHandling(unittest.TestCase):
|
||||
|
||||
import server as _server_mod
|
||||
orig_handle_get = _server_mod.handle_get
|
||||
orig_check_auth = _server_mod.check_auth
|
||||
orig_check_auth = _server_mod.check_auth_or_close
|
||||
_server_mod.handle_get = _fake_handle_get
|
||||
_server_mod.check_auth = lambda h, p: True
|
||||
_server_mod.check_auth_or_close = lambda h, p: True
|
||||
|
||||
closed_stdout = io.StringIO()
|
||||
closed_stdout.close()
|
||||
@@ -299,7 +299,7 @@ class TestServerDisconnectHandling(unittest.TestCase):
|
||||
finally:
|
||||
sys.stdout = original_stdout
|
||||
_server_mod.handle_get = orig_handle_get
|
||||
_server_mod.check_auth = orig_check_auth
|
||||
_server_mod.check_auth_or_close = orig_check_auth
|
||||
|
||||
handler.send_response.assert_called_once_with(500)
|
||||
|
||||
@@ -312,14 +312,14 @@ class TestServerDisconnectHandling(unittest.TestCase):
|
||||
|
||||
import server as _server_mod
|
||||
orig_handle_get = _server_mod.handle_get
|
||||
orig_check_auth = _server_mod.check_auth
|
||||
orig_check_auth = _server_mod.check_auth_or_close
|
||||
_server_mod.handle_get = _fake_handle_get
|
||||
_server_mod.check_auth = lambda h, p: True
|
||||
_server_mod.check_auth_or_close = lambda h, p: True
|
||||
try:
|
||||
Handler.do_GET(handler)
|
||||
finally:
|
||||
_server_mod.handle_get = orig_handle_get
|
||||
_server_mod.check_auth = orig_check_auth
|
||||
_server_mod.check_auth_or_close = orig_check_auth
|
||||
|
||||
# Should send 500 for real errors
|
||||
handler.send_response.assert_called_once_with(500)
|
||||
@@ -334,14 +334,14 @@ class TestServerDisconnectHandling(unittest.TestCase):
|
||||
|
||||
import server as _server_mod
|
||||
orig_handle_get = _server_mod.handle_get
|
||||
orig_check_auth = _server_mod.check_auth
|
||||
orig_check_auth = _server_mod.check_auth_or_close
|
||||
_server_mod.handle_get = _fake_handle_get
|
||||
_server_mod.check_auth = lambda h, p: True
|
||||
_server_mod.check_auth_or_close = lambda h, p: True
|
||||
try:
|
||||
Handler.do_GET(handler)
|
||||
finally:
|
||||
_server_mod.handle_get = orig_handle_get
|
||||
_server_mod.check_auth = orig_check_auth
|
||||
_server_mod.check_auth_or_close = orig_check_auth
|
||||
|
||||
handler.send_response.assert_not_called()
|
||||
|
||||
@@ -355,14 +355,14 @@ class TestServerDisconnectHandling(unittest.TestCase):
|
||||
|
||||
import server as _server_mod
|
||||
orig_handle_get = _server_mod.handle_get
|
||||
orig_check_auth = _server_mod.check_auth
|
||||
orig_check_auth = _server_mod.check_auth_or_close
|
||||
_server_mod.handle_get = _fake_handle_get
|
||||
_server_mod.check_auth = lambda h, p: True
|
||||
_server_mod.check_auth_or_close = lambda h, p: True
|
||||
try:
|
||||
Handler.do_GET(handler)
|
||||
finally:
|
||||
_server_mod.handle_get = orig_handle_get
|
||||
_server_mod.check_auth = orig_check_auth
|
||||
_server_mod.check_auth_or_close = orig_check_auth
|
||||
|
||||
handler.send_response.assert_not_called()
|
||||
|
||||
@@ -406,14 +406,14 @@ class TestServer500ResponseSafety(unittest.TestCase):
|
||||
|
||||
import server as _server_mod
|
||||
orig_handle_get = _server_mod.handle_get
|
||||
orig_check_auth = _server_mod.check_auth
|
||||
orig_check_auth = _server_mod.check_auth_or_close
|
||||
_server_mod.handle_get = _fake_handle_get
|
||||
_server_mod.check_auth = lambda h, p: True
|
||||
_server_mod.check_auth_or_close = lambda h, p: True
|
||||
try:
|
||||
Handler.do_GET(handler)
|
||||
finally:
|
||||
_server_mod.handle_get = orig_handle_get
|
||||
_server_mod.check_auth = orig_check_auth
|
||||
_server_mod.check_auth_or_close = orig_check_auth
|
||||
|
||||
# send_response WAS called (we tried to send 500), but write failed
|
||||
handler.send_response.assert_called_once_with(500)
|
||||
@@ -431,14 +431,14 @@ class TestServer500ResponseSafety(unittest.TestCase):
|
||||
|
||||
import server as _server_mod
|
||||
orig_handle_get = _server_mod.handle_get
|
||||
orig_check_auth = _server_mod.check_auth
|
||||
orig_check_auth = _server_mod.check_auth_or_close
|
||||
_server_mod.handle_get = _fake_handle_get
|
||||
_server_mod.check_auth = lambda h, p: True
|
||||
_server_mod.check_auth_or_close = lambda h, p: True
|
||||
try:
|
||||
Handler.do_GET(handler)
|
||||
finally:
|
||||
_server_mod.handle_get = orig_handle_get
|
||||
_server_mod.check_auth = orig_check_auth
|
||||
_server_mod.check_auth_or_close = orig_check_auth
|
||||
|
||||
# send_response WAS called (we tried to send 500), but write failed
|
||||
handler.send_response.assert_called_once_with(500)
|
||||
|
||||
@@ -0,0 +1,105 @@
|
||||
"""Comma-combined ``Content-Length`` must not over-close a healthy connection.
|
||||
|
||||
RFC 9110 §5.3 lets a list-valued field arrive either as repeated header lines or
|
||||
as a single comma-combined line, and the two spellings are equivalent. So
|
||||
``Content-Length: 0, 0`` describes the same bodyless request as two separate
|
||||
``Content-Length: 0`` headers, and rejecting it must keep the keep-alive.
|
||||
|
||||
``_declared_content_lengths()`` originally ran ``isdigit()`` across the whole raw
|
||||
value, so the comma failed the check, the field read as *unreadable*, and every
|
||||
helper-backed rejection armed ``Connection: close`` on a request with nothing
|
||||
queued behind it. That is the over-close half of this PR's contract — the mirror
|
||||
image of the bug it fixes, and the one that silently drops pooled connections.
|
||||
|
||||
The dangerous direction has to stay closed, so the disagreeing and malformed
|
||||
spellings are asserted here alongside the recoverable ones: ``0, 42`` genuinely
|
||||
cannot frame a body and must still be refused with the close armed.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
|
||||
from api.helpers import request_declares_body, unreadable_content_length
|
||||
|
||||
|
||||
class _Hdrs(dict):
|
||||
"""email.message-like: ``get()`` returns the first value, ``get_all()`` all."""
|
||||
|
||||
def get(self, k, default=None):
|
||||
for key, val in self.items():
|
||||
if key.lower() == k.lower():
|
||||
return val[0] if isinstance(val, list) else val
|
||||
return default
|
||||
|
||||
def get_all(self, k, default=None):
|
||||
out = []
|
||||
for key, val in self.items():
|
||||
if key.lower() == k.lower():
|
||||
out.extend(val if isinstance(val, list) else [val])
|
||||
return out or default
|
||||
|
||||
|
||||
class _H:
|
||||
def __init__(self, headers):
|
||||
self.headers = headers
|
||||
|
||||
|
||||
def _handler(cl):
|
||||
return _H(_Hdrs({"Content-Length": cl}))
|
||||
|
||||
|
||||
@pytest.mark.parametrize("value", ["0, 0", "00, 00", "0 , 0", "0,0", "0,\t0"])
|
||||
def test_agreeing_bodyless_comma_list_is_readable_and_bodyless(value):
|
||||
"""An agreeing all-zero list is framed, bodyless, and must keep keep-alive."""
|
||||
h = _handler(value)
|
||||
assert unreadable_content_length(h) is False, (
|
||||
f"Content-Length {value!r} read as unreadable; a bodyless rejection would "
|
||||
"then arm Connection: close and drop a healthy pooled connection"
|
||||
)
|
||||
assert bool(request_declares_body(h)) is False, (
|
||||
f"Content-Length {value!r} declares no body, so nothing is queued"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("value", ["42, 42", "42,42", "042, 42"])
|
||||
def test_agreeing_nonzero_comma_list_is_readable_with_a_body(value):
|
||||
"""An agreeing nonzero list frames a real body: readable, and a body IS pending."""
|
||||
h = _handler(value)
|
||||
assert unreadable_content_length(h) is False, value
|
||||
assert bool(request_declares_body(h)) is True, value
|
||||
|
||||
|
||||
@pytest.mark.parametrize("value", ["0, 42", "42, 0", "0, 0, 42"])
|
||||
def test_disagreeing_comma_list_stays_unreadable(value):
|
||||
"""Disagreeing members cannot frame a body — must stay refused, close armed."""
|
||||
h = _handler(value)
|
||||
assert unreadable_content_length(h) is True, (
|
||||
f"Content-Length {value!r} disagrees; there is no honest length to drain, "
|
||||
"so it must be refused with the connection armed for close"
|
||||
)
|
||||
assert bool(request_declares_body(h)) is True, value
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"value",
|
||||
["0, banana", "banana, 0", "0,", ",0", "0, ,0", "0, +0", "0, \xa00", "0, 0x5"],
|
||||
)
|
||||
def test_malformed_comma_members_stay_unreadable(value):
|
||||
"""One bad member poisons the whole field: fail closed, never treat as zero."""
|
||||
h = _handler(value)
|
||||
assert unreadable_content_length(h) is True, (
|
||||
f"Content-Length {value!r} has an unparseable member and must fail closed"
|
||||
)
|
||||
assert bool(request_declares_body(h)) is True, value
|
||||
|
||||
|
||||
def test_repeated_headers_and_comma_list_agree():
|
||||
"""The two RFC spellings of the same field must behave identically."""
|
||||
repeated = _H(_Hdrs({"Content-Length": ["0", "0"]}))
|
||||
combined = _handler("0, 0")
|
||||
assert unreadable_content_length(repeated) == unreadable_content_length(combined)
|
||||
assert bool(request_declares_body(repeated)) == bool(request_declares_body(combined))
|
||||
|
||||
repeated_bad = _H(_Hdrs({"Content-Length": ["0", "42"]}))
|
||||
combined_bad = _handler("0, 42")
|
||||
assert unreadable_content_length(repeated_bad) is True
|
||||
assert unreadable_content_length(combined_bad) is True
|
||||
@@ -10,6 +10,8 @@ import threading
|
||||
from pathlib import Path
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from api.gateway_chat import _gateway_runs_approval_event
|
||||
|
||||
REPO_ROOT = Path(__file__).parent.parent
|
||||
@@ -873,3 +875,346 @@ def test_legacy_approval_without_run_id_retires_locally():
|
||||
r._pending.pop(session_id, None)
|
||||
r._gateway_queues.pop(session_id, None)
|
||||
_STREAM_RUN_IDS.pop(stream_id, None)
|
||||
|
||||
|
||||
def test_route_deny_settles_exact_non_head_run_producer():
|
||||
"""The response route must settle the exact producer whose card was denied.
|
||||
|
||||
The route is the only caller that supplies a target's ``(run_id,
|
||||
approval_id)`` to the legacy resolver, so a regression in that wiring is
|
||||
invisible to helper-level coverage: the producer would be dropped from the
|
||||
queue without its waiter ever being woken, leaving the agent thread parked
|
||||
until the approval timeout.
|
||||
|
||||
Unlike the route-level test in ``test_approval_unblock.py`` (which needs the
|
||||
installed agent's ``tools.approval`` and is skipped without it), this one
|
||||
runs wherever the suite runs.
|
||||
"""
|
||||
from types import SimpleNamespace
|
||||
from api import route_approvals as ra
|
||||
from api import routes as r
|
||||
|
||||
sid = "sess-route-non-head-deny"
|
||||
run_id = "run-route-non-head-deny"
|
||||
head_id = "approval-route-head"
|
||||
target_id = "approval-route-target"
|
||||
sibling_id = "approval-route-sibling"
|
||||
other_run_id = "run-route-unrelated"
|
||||
head = {
|
||||
"approval_id": head_id,
|
||||
"run_id": run_id,
|
||||
"command": "echo head",
|
||||
"description": "Head approval on the run",
|
||||
"_gateway_agent_identity_v1": True,
|
||||
}
|
||||
sibling = {
|
||||
"approval_id": sibling_id,
|
||||
"run_id": run_id,
|
||||
"command": "echo sibling",
|
||||
"description": "Non-head sibling approval on the same run",
|
||||
"_gateway_agent_identity_v1": True,
|
||||
}
|
||||
target = {
|
||||
"approval_id": target_id,
|
||||
"run_id": run_id,
|
||||
"command": "echo target",
|
||||
"description": "Non-head approval on the same run",
|
||||
"_gateway_agent_identity_v1": True,
|
||||
}
|
||||
other_run = {
|
||||
"approval_id": "approval-route-other",
|
||||
"run_id": other_run_id,
|
||||
"command": "echo else",
|
||||
"description": "Approval on an unrelated run",
|
||||
}
|
||||
|
||||
def producer(payload):
|
||||
"""The agent's real producer entry when installed, an equivalent one otherwise.
|
||||
|
||||
The suite runs without hermes-agent in CI, where
|
||||
``tools.approval._ApprovalEntry`` does not exist; the stand-in carries the
|
||||
same ``data`` / ``event`` / ``result`` / ``reason`` contract the resolution
|
||||
path touches, so the assertions below mean the same thing in both shapes.
|
||||
"""
|
||||
try:
|
||||
from tools.approval import _ApprovalEntry
|
||||
except Exception:
|
||||
_ApprovalEntry = None
|
||||
if _ApprovalEntry is not None:
|
||||
return _ApprovalEntry(dict(payload))
|
||||
return SimpleNamespace(
|
||||
data=dict(payload), event=threading.Event(), result=None, reason=None
|
||||
)
|
||||
|
||||
head_entry = producer(head)
|
||||
sibling_entry = producer(sibling)
|
||||
target_entry = producer(target)
|
||||
other_run_entry = producer(other_run)
|
||||
relayed = []
|
||||
captured = {}
|
||||
|
||||
def fake_j(_handler, data, status=200, extra_headers=None):
|
||||
captured.update(payload=data, status=status)
|
||||
return data
|
||||
|
||||
def fake_respond(_self, got_run_id, got_approval_id, choice):
|
||||
relayed.append((got_run_id, got_approval_id, choice))
|
||||
return {"resolved": 1}
|
||||
|
||||
try:
|
||||
with ra._lock:
|
||||
ra._pending.pop(sid, None)
|
||||
ra._gateway_queues[sid] = [head_entry, sibling_entry, target_entry, other_run_entry]
|
||||
ra.submit_gateway_pending_mirror(sid, dict(target_entry.data))
|
||||
mirror = ra.gateway_pending_mirror(sid, approval_id=target_id, run_id=run_id)
|
||||
assert mirror is not None, "the non-head approval must be mirrored before it is answered"
|
||||
|
||||
with patch("api.routes.j", new=fake_j), \
|
||||
patch("api.runner_client.HttpRunnerClient.respond_approval", new=fake_respond), \
|
||||
patch("api.config.gateway_supports_approval_identity_v1", return_value=True):
|
||||
r._handle_approval_respond(
|
||||
object(),
|
||||
{
|
||||
"session_id": sid,
|
||||
"choice": "deny",
|
||||
"approval_id": target_id,
|
||||
"run_id": run_id,
|
||||
"mirror_token": mirror[ra._GATEWAY_MIRROR_TOKEN],
|
||||
},
|
||||
)
|
||||
|
||||
assert captured == {
|
||||
"payload": {"ok": True, "choice": "deny", "relayed": True},
|
||||
"status": 200,
|
||||
}
|
||||
assert relayed == [(run_id, target_id, "deny")]
|
||||
|
||||
assert target_entry.event.is_set(), "the denied approval's waiter must be woken"
|
||||
assert target_entry.result == "deny"
|
||||
with ra._lock:
|
||||
remaining = list(ra._gateway_queues.get(sid) or [])
|
||||
assert target_entry not in remaining, "the denied approval's producer must be consumed"
|
||||
assert head_entry in remaining, (
|
||||
"the run's own head approval must not be consumed by answering another approval"
|
||||
)
|
||||
assert not head_entry.event.is_set()
|
||||
assert sibling_entry in remaining, (
|
||||
"a sibling approval on the same run must not be consumed"
|
||||
)
|
||||
assert not sibling_entry.event.is_set()
|
||||
assert other_run_entry in remaining, (
|
||||
"an unrelated run's producer must not be consumed"
|
||||
)
|
||||
assert not other_run_entry.event.is_set()
|
||||
|
||||
# These are the observations made by subsequent HTTP polling.
|
||||
for _ in range(2):
|
||||
with ra._lock:
|
||||
ra.reconcile_gateway_pending_mirror_locked(sid)
|
||||
assert ra.gateway_pending_mirror(
|
||||
sid, approval_id=target_id, run_id=run_id
|
||||
) is None, "reconciliation must not resurrect the denied approval's mirror"
|
||||
finally:
|
||||
with ra._lock:
|
||||
ra._pending.pop(sid, None)
|
||||
ra._gateway_queues.pop(sid, None)
|
||||
|
||||
|
||||
def test_deny_settles_run_producer_so_reconciliation_cannot_resurrect_mirror():
|
||||
"""Deny must settle the producer as well as its WebUI mirror.
|
||||
|
||||
Selection is the behaviour under test, so the queue carries more than the
|
||||
one entry being resolved: a sibling approval for the same run, and a producer
|
||||
belonging to a different run. Resolving one approval must consume exactly
|
||||
that producer, wake its waiter, and leave both others queued.
|
||||
"""
|
||||
from types import SimpleNamespace
|
||||
from api import route_approvals as ra
|
||||
|
||||
sid = "sess-deny-no-resurrection"
|
||||
approval_id = "approval-deny-no-resurrection"
|
||||
run_id = "run-deny-no-resurrection"
|
||||
sibling_approval_id = "approval-deny-sibling"
|
||||
other_run_id = "run-deny-unrelated"
|
||||
approval = {
|
||||
"approval_id": approval_id,
|
||||
"run_id": run_id,
|
||||
"command": "echo pricing",
|
||||
"description": "Run pricing probe",
|
||||
}
|
||||
sibling_approval = {
|
||||
"approval_id": sibling_approval_id,
|
||||
"run_id": run_id,
|
||||
"command": "echo sibling",
|
||||
"description": "Sibling approval on the same run",
|
||||
}
|
||||
other_run_approval = {
|
||||
"approval_id": "approval-deny-other-run",
|
||||
"run_id": other_run_id,
|
||||
"command": "echo else",
|
||||
"description": "Approval on an unrelated run",
|
||||
}
|
||||
|
||||
def producer(payload):
|
||||
return SimpleNamespace(data=dict(payload), event=threading.Event(), result=None)
|
||||
|
||||
target = producer(approval)
|
||||
same_run_sibling = producer(sibling_approval)
|
||||
other_run = producer(other_run_approval)
|
||||
try:
|
||||
with ra._lock:
|
||||
ra._pending.pop(sid, None)
|
||||
ra._gateway_queues[sid] = [target, same_run_sibling, other_run]
|
||||
queued_before = list(ra._gateway_queues[sid])
|
||||
ra.submit_gateway_pending_mirror(sid, dict(approval))
|
||||
mirror = ra.gateway_pending_mirror(sid, approval_id=approval_id, run_id=run_id)
|
||||
assert mirror is not None
|
||||
|
||||
resolved, _head, _total = ra.resolve_gateway_pending_run(
|
||||
sid,
|
||||
approval_id=approval_id,
|
||||
run_id=run_id,
|
||||
choice="deny",
|
||||
)
|
||||
assert resolved == 1
|
||||
|
||||
assert queued_before == [target, same_run_sibling, other_run], (
|
||||
"the queue should have held all three producers before retirement"
|
||||
)
|
||||
with ra._lock:
|
||||
remaining = list(ra._gateway_queues.get(sid) or [])
|
||||
assert target not in remaining, "the denied approval's producer must be consumed"
|
||||
assert target.event.is_set(), "the denied approval's waiter must be woken"
|
||||
assert target.result == "deny"
|
||||
assert same_run_sibling in remaining, (
|
||||
"a sibling approval on the same run must not be consumed by retiring another approval"
|
||||
)
|
||||
assert other_run in remaining, (
|
||||
"an unrelated run's producer must not be consumed"
|
||||
)
|
||||
|
||||
# These are the same observations made by subsequent HTTP polling: the
|
||||
# denied approval must not come back, and the survivors must stay live.
|
||||
# `gateway_pending_mirror` acquires `_lock` itself, so it must be called
|
||||
# outside the locked section.
|
||||
for _ in range(2):
|
||||
with ra._lock:
|
||||
ra.reconcile_gateway_pending_mirror_locked(sid)
|
||||
assert ra.gateway_pending_mirror(
|
||||
sid, approval_id=approval_id, run_id=run_id
|
||||
) is None, "reconciliation must not resurrect the denied approval's mirror"
|
||||
with ra._lock:
|
||||
refreshed = list(ra._gateway_queues.get(sid) or [])
|
||||
assert target not in refreshed
|
||||
assert same_run_sibling in refreshed
|
||||
assert other_run in refreshed
|
||||
finally:
|
||||
with ra._lock:
|
||||
ra._pending.pop(sid, None)
|
||||
ra._gateway_queues.pop(sid, None)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"failure_point",
|
||||
["reconcile_gateway_pending_mirror_locked", "_approval_sse_notify_locked"],
|
||||
)
|
||||
def test_exact_run_resolution_settles_before_projection_failure(failure_point):
|
||||
"""Projection failures must not strand an already-consumed producer."""
|
||||
from types import SimpleNamespace
|
||||
from api import route_approvals as ra
|
||||
|
||||
sid = f"sess-exact-projection-failure-{failure_point}"
|
||||
run_id = "run-exact-projection-failure"
|
||||
target = SimpleNamespace(
|
||||
data={"run_id": run_id, "approval_id": "approval-target"},
|
||||
event=threading.Event(),
|
||||
result=None,
|
||||
reason=None,
|
||||
)
|
||||
sibling = SimpleNamespace(
|
||||
data={"run_id": run_id, "approval_id": "approval-sibling"},
|
||||
event=threading.Event(),
|
||||
result=None,
|
||||
reason=None,
|
||||
)
|
||||
try:
|
||||
with ra._lock:
|
||||
ra._pending.pop(sid, None)
|
||||
ra._gateway_queues[sid] = [target, sibling]
|
||||
|
||||
with patch.object(ra, failure_point, side_effect=RuntimeError("projection failed")):
|
||||
with pytest.raises(RuntimeError, match="projection failed"):
|
||||
ra.resolve_gateway_pending_run(
|
||||
sid,
|
||||
approval_id="approval-target",
|
||||
run_id=run_id,
|
||||
choice="deny",
|
||||
reason="user denied",
|
||||
)
|
||||
|
||||
assert target.result == "deny"
|
||||
assert target.reason == "user denied"
|
||||
assert target.event.is_set(), "the consumed producer must be woken before projection work"
|
||||
assert sibling.result is None
|
||||
assert sibling.reason is None
|
||||
assert not sibling.event.is_set()
|
||||
with ra._lock:
|
||||
assert ra._gateway_queues[sid] == [sibling]
|
||||
finally:
|
||||
with ra._lock:
|
||||
ra._pending.pop(sid, None)
|
||||
ra._gateway_queues.pop(sid, None)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"terminal_reason",
|
||||
[
|
||||
"Gateway run completed before approval resolution",
|
||||
"Gateway run failed before approval resolution",
|
||||
"Gateway run was cancelled before approval resolution",
|
||||
"Gateway run ended during teardown before approval resolution",
|
||||
],
|
||||
)
|
||||
def test_terminal_run_settlement_denies_all_run_producers_and_preserves_other_runs(
|
||||
terminal_reason,
|
||||
):
|
||||
"""Every terminal exit fails closed without consuming another run's producer."""
|
||||
from types import SimpleNamespace
|
||||
from api import route_approvals as ra
|
||||
|
||||
sid = f"sess-terminal-run-{terminal_reason.split()[2]}"
|
||||
run_id = "run-terminal"
|
||||
|
||||
def producer(entry_run_id, approval_id):
|
||||
return SimpleNamespace(
|
||||
data={"run_id": entry_run_id, "approval_id": approval_id},
|
||||
event=threading.Event(),
|
||||
result=None,
|
||||
reason=None,
|
||||
)
|
||||
|
||||
targets = [producer(run_id, "approval-a"), producer(run_id, "approval-b")]
|
||||
survivor = producer("run-other", "approval-other")
|
||||
try:
|
||||
with ra._lock:
|
||||
ra._pending.pop(sid, None)
|
||||
ra._gateway_queues[sid] = [targets[0], survivor, targets[1]]
|
||||
|
||||
settled, _head, _total = ra.settle_gateway_pending_run(
|
||||
sid, run_id, reason=terminal_reason
|
||||
)
|
||||
|
||||
assert settled == 2
|
||||
for target in targets:
|
||||
assert target.result == "deny"
|
||||
assert target.reason == terminal_reason
|
||||
assert target.event.is_set()
|
||||
assert survivor.result is None
|
||||
assert survivor.reason is None
|
||||
assert not survivor.event.is_set()
|
||||
with ra._lock:
|
||||
assert ra._gateway_queues[sid] == [survivor]
|
||||
finally:
|
||||
with ra._lock:
|
||||
ra._pending.pop(sid, None)
|
||||
ra._gateway_queues.pop(sid, None)
|
||||
@@ -333,6 +333,178 @@ def test_gateway_runs_api_submission():
|
||||
assert captured["body_extras"]["reasoning_effort"] == "high"
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("terminal_event", "terminal_reason"),
|
||||
[
|
||||
("run.completed", "Gateway run completed before approval resolution"),
|
||||
("run.failed", "Gateway run failed before approval resolution"),
|
||||
("run.cancelled", "Gateway run was cancelled before approval resolution"),
|
||||
],
|
||||
)
|
||||
def test_gateway_terminal_event_fail_closes_parked_run_producer(
|
||||
terminal_event,
|
||||
terminal_reason,
|
||||
):
|
||||
"""Each terminal gateway event denies and wakes the run's parked producers."""
|
||||
from api import gateway_chat
|
||||
from api import route_approvals as approvals
|
||||
|
||||
sid = f"sess-terminal-event-{terminal_event}"
|
||||
stream_id = f"stream-terminal-event-{terminal_event}"
|
||||
run_id = f"run-terminal-event-{terminal_event}"
|
||||
target = SimpleNamespace(
|
||||
data={"run_id": run_id, "approval_id": "approval-target"},
|
||||
event=threading.Event(),
|
||||
result=None,
|
||||
reason=None,
|
||||
)
|
||||
survivor = SimpleNamespace(
|
||||
data={"run_id": "run-other", "approval_id": "approval-other"},
|
||||
event=threading.Event(),
|
||||
result=None,
|
||||
reason=None,
|
||||
)
|
||||
|
||||
class JsonResponse:
|
||||
def __enter__(self):
|
||||
return self
|
||||
|
||||
def __exit__(self, *_args):
|
||||
return None
|
||||
|
||||
def read(self, _limit=None):
|
||||
return json.dumps({"run_id": run_id}).encode()
|
||||
|
||||
class SseResponse:
|
||||
def __enter__(self):
|
||||
return self
|
||||
|
||||
def __exit__(self, *_args):
|
||||
return None
|
||||
|
||||
def __iter__(self):
|
||||
payload = {"event": terminal_event}
|
||||
if terminal_event == "run.failed":
|
||||
payload["error"] = "terminal failure"
|
||||
return iter([f"data: {json.dumps(payload)}".encode(), b""])
|
||||
|
||||
def fake_urlopen(req, *, timeout=None):
|
||||
del timeout
|
||||
return JsonResponse() if req.full_url.endswith("/v1/runs") else SseResponse()
|
||||
|
||||
try:
|
||||
with approvals._lock:
|
||||
approvals._pending.pop(sid, None)
|
||||
approvals._gateway_queues[sid] = [target, survivor]
|
||||
with patch("urllib.request.urlopen", side_effect=fake_urlopen):
|
||||
if terminal_event == "run.failed":
|
||||
with pytest.raises(RuntimeError, match="terminal failure"):
|
||||
gateway_chat._run_gateway_runs_api_streaming(
|
||||
sid, "hi", "test", "/tmp", stream_id, "http://gw:8642", "", [], {},
|
||||
put_gateway_event=lambda *_args: None,
|
||||
cancel_event=threading.Event(),
|
||||
session=SimpleNamespace(context_messages=[]),
|
||||
)
|
||||
else:
|
||||
gateway_chat._run_gateway_runs_api_streaming(
|
||||
sid, "hi", "test", "/tmp", stream_id, "http://gw:8642", "", [], {},
|
||||
put_gateway_event=lambda *_args: None,
|
||||
cancel_event=threading.Event(),
|
||||
session=SimpleNamespace(context_messages=[]),
|
||||
)
|
||||
|
||||
assert target.result == "deny"
|
||||
assert target.reason == terminal_reason
|
||||
assert target.event.is_set()
|
||||
assert survivor.result is None
|
||||
assert not survivor.event.is_set()
|
||||
with approvals._lock:
|
||||
assert approvals._gateway_queues[sid] == [survivor]
|
||||
finally:
|
||||
gateway_chat._STREAM_RUN_IDS.pop(stream_id, None)
|
||||
with approvals._lock:
|
||||
approvals._pending.pop(sid, None)
|
||||
approvals._gateway_queues.pop(sid, None)
|
||||
|
||||
|
||||
def test_gateway_stream_teardown_fail_closes_parked_run_producer():
|
||||
"""A disconnect before a terminal event still settles the mapped run."""
|
||||
from api import gateway_chat
|
||||
from api import route_approvals as approvals
|
||||
from api.config import STREAMS, STREAMS_LOCK
|
||||
|
||||
sid = "sess-terminal-teardown"
|
||||
stream_id = "stream-terminal-teardown"
|
||||
run_id = "run-terminal-teardown"
|
||||
target = SimpleNamespace(
|
||||
data={"run_id": run_id, "approval_id": "approval-target"},
|
||||
event=threading.Event(),
|
||||
result=None,
|
||||
reason=None,
|
||||
)
|
||||
survivor = SimpleNamespace(
|
||||
data={"run_id": "run-other", "approval_id": "approval-other"},
|
||||
event=threading.Event(),
|
||||
result=None,
|
||||
reason=None,
|
||||
)
|
||||
events = []
|
||||
queue = MagicMock()
|
||||
queue.put_nowait = lambda item: events.append(item)
|
||||
with STREAMS_LOCK:
|
||||
STREAMS[stream_id] = queue
|
||||
|
||||
session = MagicMock()
|
||||
session.active_stream_id = stream_id
|
||||
session.workspace = "/tmp"
|
||||
session.model = "test"
|
||||
session.model_provider = None
|
||||
session.profile = None
|
||||
session.context_messages = []
|
||||
session.messages = []
|
||||
session.pending_user_message = None
|
||||
session.pending_attachments = None
|
||||
session.pending_started_at = None
|
||||
|
||||
def disconnect_after_run_id(*_args, **_kwargs):
|
||||
gateway_chat._publish_gateway_run_id(stream_id, run_id)
|
||||
raise RuntimeError("gateway disconnected")
|
||||
|
||||
try:
|
||||
with approvals._lock:
|
||||
approvals._pending.pop(sid, None)
|
||||
approvals._gateway_queues[sid] = [target, survivor]
|
||||
with patch.dict(
|
||||
"os.environ",
|
||||
{"HERMES_WEBUI_CHAT_BACKEND": "gateway", "HERMES_WEBUI_GATEWAY_USE_RUNS_API": "1"},
|
||||
), patch("api.gateway_chat.gateway_supports_approval", return_value=True), patch(
|
||||
"api.gateway_chat._run_gateway_runs_api_streaming",
|
||||
side_effect=disconnect_after_run_id,
|
||||
), patch("api.gateway_chat.get_session", return_value=session):
|
||||
gateway_chat._run_gateway_chat_streaming(
|
||||
session_id=sid,
|
||||
msg_text="hi",
|
||||
model="test-model",
|
||||
workspace="/tmp",
|
||||
stream_id=stream_id,
|
||||
)
|
||||
|
||||
assert target.result == "deny"
|
||||
assert target.reason == "Gateway run ended during teardown before approval resolution"
|
||||
assert target.event.is_set()
|
||||
assert survivor.result is None
|
||||
assert not survivor.event.is_set()
|
||||
with approvals._lock:
|
||||
assert approvals._gateway_queues[sid] == [survivor]
|
||||
finally:
|
||||
gateway_chat._STREAM_RUN_IDS.pop(stream_id, None)
|
||||
with STREAMS_LOCK:
|
||||
STREAMS.pop(stream_id, None)
|
||||
with approvals._lock:
|
||||
approvals._pending.pop(sid, None)
|
||||
approvals._gateway_queues.pop(sid, None)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 3. Approval event translation
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -1268,22 +1440,32 @@ def test_terminal_run_retirement_removes_all_same_run_mirrors():
|
||||
approvals._pending.pop(sid, None)
|
||||
|
||||
|
||||
def test_terminal_run_retirement_clears_same_run_gateway_queue_state():
|
||||
def test_terminal_run_settlement_clears_same_run_gateway_queue_state():
|
||||
import api.route_approvals as approvals
|
||||
|
||||
sid = "sess-terminal-run-queue"
|
||||
approvals._pending.pop(sid, None)
|
||||
approvals._gateway_queues.pop(sid, None)
|
||||
try:
|
||||
approvals._gateway_queues[sid] = [
|
||||
SimpleNamespace(data={"run_id": "run-a", "approval_id": "appr-a", "command": "a"}),
|
||||
SimpleNamespace(data={"command": "local-head"}),
|
||||
]
|
||||
target = SimpleNamespace(
|
||||
data={"run_id": "run-a", "approval_id": "appr-a", "command": "a"},
|
||||
event=threading.Event(),
|
||||
result=None,
|
||||
reason=None,
|
||||
)
|
||||
local_head = SimpleNamespace(data={"command": "local-head"})
|
||||
approvals._gateway_queues[sid] = [target, local_head]
|
||||
approvals.submit_gateway_pending_mirror(
|
||||
sid,
|
||||
{"run_id": "run-a", "approval_id": "appr-a", "command": "a"},
|
||||
)
|
||||
assert approvals.retire_gateway_pending_mirror(sid, run_id="run-a")
|
||||
settled, _head, _total = approvals.settle_gateway_pending_run(
|
||||
sid, "run-a", reason="terminal"
|
||||
)
|
||||
assert settled == 1
|
||||
assert target.result == "deny"
|
||||
assert target.reason == "terminal"
|
||||
assert target.event.is_set()
|
||||
assert approvals.gateway_pending_mirror(sid, run_id="run-a") is None
|
||||
assert not any(
|
||||
str((getattr(entry, "data", None) or {}).get("run_id") or "").strip() == "run-a"
|
||||
@@ -1294,18 +1476,30 @@ def test_terminal_run_retirement_clears_same_run_gateway_queue_state():
|
||||
approvals._gateway_queues.pop(sid, None)
|
||||
|
||||
|
||||
def test_terminal_run_retirement_clears_non_head_same_run_gateway_queue_state():
|
||||
def test_terminal_run_settlement_clears_non_head_same_run_gateway_queue_state():
|
||||
import api.route_approvals as approvals
|
||||
|
||||
sid = "sess-terminal-run-non-head-queue"
|
||||
approvals._pending.pop(sid, None)
|
||||
approvals._gateway_queues.pop(sid, None)
|
||||
try:
|
||||
target = SimpleNamespace(
|
||||
data={"run_id": "run-a", "approval_id": "appr-a", "command": "a"},
|
||||
event=threading.Event(),
|
||||
result=None,
|
||||
reason=None,
|
||||
)
|
||||
approvals._gateway_queues[sid] = [
|
||||
SimpleNamespace(data={"command": "local-head"}),
|
||||
SimpleNamespace(data={"run_id": "run-a", "approval_id": "appr-a", "command": "a"}),
|
||||
target,
|
||||
]
|
||||
assert approvals.retire_gateway_pending_mirror(sid, run_id="run-a")
|
||||
settled, _head, _total = approvals.settle_gateway_pending_run(
|
||||
sid, "run-a", reason="terminal"
|
||||
)
|
||||
assert settled == 1
|
||||
assert target.result == "deny"
|
||||
assert target.reason == "terminal"
|
||||
assert target.event.is_set()
|
||||
assert approvals.gateway_pending_mirror(sid, run_id="run-a") is None
|
||||
assert not any(
|
||||
str((getattr(entry, "data", None) or {}).get("run_id") or "").strip() == "run-a"
|
||||
@@ -1865,7 +2059,15 @@ def test_chat_cancel_waits_for_worker_published_run_id_before_settlement(
|
||||
STREAMS[stream_id] = SimpleNamespace(put_nowait=lambda *_args, **_kwargs: None)
|
||||
register_stream_owner(stream_id, sid)
|
||||
approvals._pending.pop(sid, None)
|
||||
approvals.submit_gateway_pending_mirror(sid, {"run_id": "run-stop/1", "command": "first"})
|
||||
approval_entry = SimpleNamespace(
|
||||
data={"run_id": "run-stop/1", "approval_id": "approval-stop"},
|
||||
event=threading.Event(),
|
||||
result=None,
|
||||
reason=None,
|
||||
)
|
||||
with approvals._lock:
|
||||
approvals._gateway_queues[sid] = [approval_entry]
|
||||
approvals.submit_gateway_pending_mirror(sid, approval_entry.data)
|
||||
session = SimpleNamespace(
|
||||
profile=None,
|
||||
workspace="/tmp",
|
||||
@@ -1945,8 +2147,15 @@ def test_chat_cancel_waits_for_worker_published_run_id_before_settlement(
|
||||
assert called["stop"] == "run-stop/1"
|
||||
assert called["cancel"] is expect_cancel
|
||||
if stop_result:
|
||||
assert approval_entry.result == "deny"
|
||||
assert approval_entry.reason == (
|
||||
"Gateway run was cancelled before approval resolution"
|
||||
)
|
||||
assert approval_entry.event.is_set()
|
||||
assert approvals.gateway_pending_mirror(sid, run_id="run-stop/1") is None
|
||||
else:
|
||||
assert approval_entry.result is None
|
||||
assert not approval_entry.event.is_set()
|
||||
assert stream_id in STREAMS
|
||||
assert stream_owner_session_id(stream_id) == sid
|
||||
assert session.active_stream_id == stream_id
|
||||
@@ -1957,7 +2166,9 @@ def test_chat_cancel_waits_for_worker_published_run_id_before_settlement(
|
||||
release_worker.set()
|
||||
request_thread.join(timeout=5)
|
||||
worker_thread.join(timeout=5)
|
||||
approvals._pending.pop(sid, None)
|
||||
with approvals._lock:
|
||||
approvals._pending.pop(sid, None)
|
||||
approvals._gateway_queues.pop(sid, None)
|
||||
STREAMS.pop(stream_id, None)
|
||||
ACTIVE_RUNS.pop(stream_id, None)
|
||||
unregister_stream_owner(stream_id)
|
||||
|
||||
@@ -0,0 +1,158 @@
|
||||
"""Regression: GatewayWatcher must not burn CPU when nobody is listening.
|
||||
|
||||
Maint #3035 / production evidence: with zero inbound traffic the daemon still
|
||||
re-fingerprinted every sessions row every 5s and woke 10×/s on ``time.sleep(0.1)``
|
||||
stop-checks, holding ~7% of a core forever.
|
||||
|
||||
The poll loop must:
|
||||
1. park (no fingerprint / no projection) while subscriber count is zero, and
|
||||
2. wait on ``threading.Event.wait`` for the poll interval — not a spin of
|
||||
short sleeps — so a quiet process costs essentially nothing.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import importlib
|
||||
import sqlite3
|
||||
import time
|
||||
from pathlib import Path
|
||||
|
||||
|
||||
def _make_db(tmp_path: Path):
|
||||
db = tmp_path / "state.db"
|
||||
conn = sqlite3.connect(str(db))
|
||||
conn.executescript(
|
||||
"""
|
||||
CREATE TABLE sessions (
|
||||
id TEXT PRIMARY KEY,
|
||||
source TEXT NOT NULL,
|
||||
session_source TEXT,
|
||||
model TEXT,
|
||||
started_at REAL NOT NULL,
|
||||
ended_at REAL,
|
||||
end_reason TEXT,
|
||||
parent_session_id TEXT,
|
||||
message_count INTEGER DEFAULT 0,
|
||||
title TEXT,
|
||||
archived INTEGER DEFAULT 0
|
||||
);
|
||||
CREATE TABLE messages (
|
||||
id INTEGER PRIMARY KEY AUTOINCREMENT,
|
||||
session_id TEXT NOT NULL,
|
||||
role TEXT NOT NULL,
|
||||
content TEXT,
|
||||
timestamp REAL NOT NULL
|
||||
);
|
||||
"""
|
||||
)
|
||||
conn.execute(
|
||||
"INSERT INTO sessions (id, source, model, started_at, message_count, title) "
|
||||
"VALUES ('tg1', 'telegram', 'm', ?, 1, 'Chat')",
|
||||
(time.time(),),
|
||||
)
|
||||
conn.execute(
|
||||
"INSERT INTO messages (session_id, role, content, timestamp) "
|
||||
"VALUES ('tg1', 'user', 'x', ?)",
|
||||
(time.time(),),
|
||||
)
|
||||
conn.commit()
|
||||
return db, conn
|
||||
|
||||
|
||||
def test_poll_loop_uses_blocking_event_wait_not_spin_sleep():
|
||||
"""Source contract: the idle sleep must be one Event.wait, not 50× sleep(0.1)."""
|
||||
src = (
|
||||
Path(__file__).resolve().parents[1] / "api" / "gateway_watcher.py"
|
||||
).read_text(encoding="utf-8")
|
||||
start = src.index("def _poll_loop(self)")
|
||||
end = src.index("\n# ── Module-level watcher registry", start)
|
||||
body = src[start:end]
|
||||
# Strip docstrings so a historical mention of the old spin cannot false-fail.
|
||||
code = body
|
||||
while '"""' in code:
|
||||
a = code.index('"""')
|
||||
b = code.index('"""', a + 3) + 3
|
||||
code = code[:a] + code[b:]
|
||||
assert "time.sleep(0.1)" not in code, (
|
||||
"0.1s stop-check spin burns syscalls forever; use Event.wait(POLL_INTERVAL)"
|
||||
)
|
||||
assert "self._stop_event.wait(" in code, (
|
||||
"poll interval must block on _stop_event.wait so stop() wakes immediately"
|
||||
)
|
||||
assert "_idle_wake" in code and "_has_subscribers" in code, (
|
||||
"zero-subscriber path must park on _idle_wake instead of fingerprinting"
|
||||
)
|
||||
|
||||
|
||||
def test_poll_loop_parks_without_subscribers(tmp_path, monkeypatch):
|
||||
"""Zero subscribers → zero fingerprint work; subscribe wakes and polls resume."""
|
||||
gw = importlib.import_module("api.gateway_watcher")
|
||||
db, _conn = _make_db(tmp_path)
|
||||
|
||||
polls: list[float] = []
|
||||
real_poll_once = gw.GatewayWatcher._poll_once
|
||||
|
||||
def tracing_poll_once(self, *args, **kwargs):
|
||||
polls.append(time.monotonic())
|
||||
return real_poll_once(self, *args, **kwargs)
|
||||
|
||||
monkeypatch.setattr(gw.GatewayWatcher, "_poll_once", tracing_poll_once)
|
||||
|
||||
watcher = gw.GatewayWatcher(state_db_path=db)
|
||||
# Keep the active interval short so the subscribed phase is fast to assert.
|
||||
watcher.POLL_INTERVAL = 0.05
|
||||
watcher.start()
|
||||
try:
|
||||
time.sleep(0.25)
|
||||
assert polls == [], (
|
||||
"with no SSE subscribers the watcher must not fingerprint/poll state.db; "
|
||||
f"got {len(polls)} poll(s)"
|
||||
)
|
||||
|
||||
q = watcher.subscribe()
|
||||
deadline = time.monotonic() + 2.0
|
||||
while not polls and time.monotonic() < deadline:
|
||||
time.sleep(0.01)
|
||||
assert polls, "first subscribe must wake the parked poll loop promptly"
|
||||
|
||||
# Let at least one more subscribed cycle land, then detach.
|
||||
before_unsub = len(polls)
|
||||
deadline = time.monotonic() + 2.0
|
||||
while len(polls) <= before_unsub and time.monotonic() < deadline:
|
||||
time.sleep(0.01)
|
||||
assert len(polls) > before_unsub, "subscribed watcher must keep polling"
|
||||
|
||||
watcher.unsubscribe(q)
|
||||
# Drain any in-flight poll that started before unsubscribe, then prove
|
||||
# the loop re-parks (no further growth).
|
||||
time.sleep(watcher.POLL_INTERVAL + 0.05)
|
||||
settled = len(polls)
|
||||
time.sleep(0.3)
|
||||
assert len(polls) == settled, (
|
||||
"after the last subscriber leaves, polling must park again; "
|
||||
f"grew from {settled} to {len(polls)}"
|
||||
)
|
||||
finally:
|
||||
watcher.stop()
|
||||
assert not watcher.is_alive() or (
|
||||
watcher._thread is None or not watcher._thread.is_alive()
|
||||
)
|
||||
|
||||
|
||||
def test_stop_unparks_idle_watcher_promptly(tmp_path):
|
||||
"""stop() must release a watcher parked with no subscribers within join timeout."""
|
||||
gw = importlib.import_module("api.gateway_watcher")
|
||||
db, _conn = _make_db(tmp_path)
|
||||
watcher = gw.GatewayWatcher(state_db_path=db)
|
||||
watcher.POLL_INTERVAL = 30 # would hang badly if stop relied on the interval alone
|
||||
watcher.start()
|
||||
try:
|
||||
# Confirm we are parked (thread alive, no work).
|
||||
assert watcher.is_alive()
|
||||
time.sleep(0.05)
|
||||
t0 = time.monotonic()
|
||||
watcher.stop()
|
||||
elapsed = time.monotonic() - t0
|
||||
assert elapsed < 2.0, f"stop() blocked {elapsed:.2f}s on an idle parked watcher"
|
||||
assert not watcher.is_alive()
|
||||
finally:
|
||||
watcher.stop()
|
||||
@@ -0,0 +1,158 @@
|
||||
"""A body-bearing GET that fails auth must not poison a keep-alive connection.
|
||||
|
||||
The sibling files cover the WRITE paths: a rejected POST/PUT/PATCH/DELETE arms the
|
||||
close through ``check_auth_or_close``. ``do_GET`` was the documented residual — it
|
||||
called plain ``check_auth``, so a GET that *declares a body* and then fails auth
|
||||
answered its 401/302 and left those bytes queued in ``rfile``. On a keep-alive
|
||||
socket the next request is then parsed starting mid-body, and the client gets an
|
||||
error about a request it never sent.
|
||||
|
||||
A GET with a body is unusual but entirely legal, and it is what a proxy, a
|
||||
``fetch`` with a body, or a scripted client can produce.
|
||||
|
||||
Both halves of the contract are asserted here, because the naive fix (close every
|
||||
auth-failed GET) would regress the body-less login-redirect flow:
|
||||
|
||||
* body-bearing + auth-failed -> connection MUST close (framing unresynchronisable)
|
||||
* body-less + auth-failed -> connection MUST stay alive (nothing queued)
|
||||
|
||||
``test_unfixed_handler_reproduces_the_poisoning`` is the negative control: the same
|
||||
socket exchange against a handler that answers without arming the close shows the
|
||||
leftover body surfacing as a bogus ``400 Bad request syntax``, proving the exchange
|
||||
really does exercise the bug rather than passing vacuously.
|
||||
"""
|
||||
|
||||
import socket
|
||||
import threading
|
||||
from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
|
||||
|
||||
BODY = b'{"q": "search"}'
|
||||
|
||||
|
||||
def _get_with_body(path=b"/api/sessions"):
|
||||
return (
|
||||
b"GET " + path + b" HTTP/1.1\r\n"
|
||||
b"Host: localhost\r\n"
|
||||
b"Content-Type: application/json\r\n"
|
||||
b"Content-Length: " + str(len(BODY)).encode() + b"\r\n"
|
||||
b"\r\n" + BODY
|
||||
)
|
||||
|
||||
|
||||
BODYLESS_GET = (
|
||||
b"GET /api/sessions HTTP/1.1\r\n"
|
||||
b"Host: localhost\r\n"
|
||||
b"\r\n"
|
||||
)
|
||||
FOLLOWING_GET = b"GET /api/health HTTP/1.1\r\nHost: localhost\r\n\r\n"
|
||||
|
||||
|
||||
def _declares_body(handler):
|
||||
"""The production rule: framing declares a body, so it is queued in rfile."""
|
||||
raw = handler.headers.get("Content-Length")
|
||||
if raw is not None:
|
||||
stripped = raw.strip(" \t")
|
||||
if not (stripped.isascii() and stripped.isdigit()):
|
||||
return True # unreadable framing -> fail closed
|
||||
try:
|
||||
if int(stripped) > 0:
|
||||
return True
|
||||
except ValueError:
|
||||
return True
|
||||
return bool((handler.headers.get("Transfer-Encoding") or "").strip())
|
||||
|
||||
|
||||
class _AuthRejectingHandler(BaseHTTPRequestHandler):
|
||||
"""Mirrors do_GET's shape: reject before the body is ever read."""
|
||||
|
||||
protocol_version = "HTTP/1.1"
|
||||
arm_close = True
|
||||
|
||||
def do_GET(self):
|
||||
if self.path == "/api/health":
|
||||
payload = b'{"status": "ok"}'
|
||||
self.send_response(200)
|
||||
self.send_header("Content-Type", "application/json")
|
||||
self.send_header("Content-Length", str(len(payload)))
|
||||
self.end_headers()
|
||||
self.wfile.write(payload)
|
||||
return
|
||||
# Auth failure BEFORE the body is read — the residual's exact shape.
|
||||
if self.arm_close and _declares_body(self):
|
||||
self.close_connection = True
|
||||
payload = b'{"error": "unauthorized"}'
|
||||
self.send_response(401)
|
||||
self.send_header("Content-Type", "application/json")
|
||||
self.send_header("Content-Length", str(len(payload)))
|
||||
if self.arm_close and _declares_body(self):
|
||||
self.send_header("Connection", "close")
|
||||
self.end_headers()
|
||||
self.wfile.write(payload)
|
||||
|
||||
def log_message(self, *a):
|
||||
pass
|
||||
|
||||
|
||||
class _UnfixedHandler(_AuthRejectingHandler):
|
||||
arm_close = False
|
||||
|
||||
|
||||
def _exchange(handler_cls, request_bytes):
|
||||
"""Pipeline `request_bytes` + a follow-up GET down ONE socket; return the reply."""
|
||||
srv = ThreadingHTTPServer(("127.0.0.1", 0), handler_cls)
|
||||
threading.Thread(target=srv.serve_forever, daemon=True).start()
|
||||
try:
|
||||
with socket.create_connection(srv.server_address, timeout=5) as s:
|
||||
s.sendall(request_bytes + FOLLOWING_GET)
|
||||
s.settimeout(5)
|
||||
chunks = []
|
||||
while True:
|
||||
try:
|
||||
b = s.recv(4096)
|
||||
except socket.timeout:
|
||||
break
|
||||
if not b:
|
||||
break
|
||||
chunks.append(b)
|
||||
return b"".join(chunks)
|
||||
finally:
|
||||
srv.shutdown()
|
||||
srv.server_close()
|
||||
|
||||
|
||||
def test_body_bearing_get_that_fails_auth_closes_the_connection():
|
||||
raw = _exchange(_AuthRejectingHandler, _get_with_body())
|
||||
assert b"401" in raw, raw[:200]
|
||||
assert b"Connection: close" in raw, f"401 did not advertise close: {raw[:200]}"
|
||||
# The follow-up GET must NEVER be answered on this socket.
|
||||
assert raw.count(b"HTTP/1.1") == 1, f"connection was reused: {raw[:300]}"
|
||||
assert b'"status": "ok"' not in raw, f"follow-up GET was served: {raw[:300]}"
|
||||
# And the leftover body must never surface as a bogus request line.
|
||||
assert b"Bad request syntax" not in raw, raw[:300]
|
||||
assert b"501" not in raw, raw[:300]
|
||||
|
||||
|
||||
def test_bodyless_get_that_fails_auth_keeps_keep_alive():
|
||||
"""The over-close half: nothing is queued, so the socket must stay usable."""
|
||||
raw = _exchange(_AuthRejectingHandler, BODYLESS_GET)
|
||||
assert b"401" in raw, raw[:200]
|
||||
assert b"Connection: close" not in raw, f"over-closed a bodyless reject: {raw[:200]}"
|
||||
# The pipelined follow-up IS served, proving keep-alive survived.
|
||||
assert b'"status": "ok"' in raw, f"keep-alive was dropped: {raw[:300]}"
|
||||
assert raw.count(b"HTTP/1.1") == 2, raw[:300]
|
||||
|
||||
|
||||
def test_unfixed_handler_reproduces_the_poisoning():
|
||||
"""Negative control: without the arming, the leftover body corrupts the stream."""
|
||||
raw = _exchange(_UnfixedHandler, _get_with_body())
|
||||
assert b"401" in raw, raw[:200]
|
||||
poisoned = (
|
||||
b"Bad request syntax" in raw
|
||||
or b"400" in raw
|
||||
or b"501" in raw
|
||||
or b'"status": "ok"' not in raw
|
||||
)
|
||||
assert poisoned, (
|
||||
"expected the unread body to poison the next request on this socket; "
|
||||
f"got {raw[:300]!r}"
|
||||
)
|
||||
@@ -144,7 +144,7 @@ def test_server_bypasses_auth_for_csp_report(monkeypatch):
|
||||
called["path"] = parsed.path
|
||||
return True
|
||||
|
||||
monkeypatch.setattr("server.check_auth", fail_auth)
|
||||
monkeypatch.setattr("server.check_auth_or_close", fail_auth)
|
||||
monkeypatch.setattr("server.clear_request_profile", lambda: None)
|
||||
monkeypatch.setattr("server.get_profile_cookie", lambda _handler: None)
|
||||
|
||||
|
||||
@@ -41,7 +41,17 @@ class _FakeHandler:
|
||||
|
||||
def _capture(monkeypatch):
|
||||
cap = {}
|
||||
monkeypatch.setattr(routes, "j", lambda h, o: (cap.__setitem__("ok", o), True)[1])
|
||||
# #7710: the real ``j`` signature is ``j(handler, payload, status=200,
|
||||
# extra_headers=None, *, pretty=True)``; the mock has to accept the
|
||||
# status keyword so the helper's 409 ``session_profile_mismatch``
|
||||
# payload can be captured here. Capture the STATUS too — swallowing it
|
||||
# into ``*_`` would let a wrong status (200/404) pass this test.
|
||||
def _fake_j(h, o, status=200, *_, **__):
|
||||
cap["ok"] = o
|
||||
cap["ok_status"] = status
|
||||
return True
|
||||
|
||||
monkeypatch.setattr(routes, "j", _fake_j)
|
||||
monkeypatch.setattr(
|
||||
routes,
|
||||
"bad",
|
||||
@@ -72,7 +82,10 @@ class _FakeSession:
|
||||
|
||||
def test_import_cli_existing_foreign_profile_unqualified_request_404(monkeypatch):
|
||||
"""Unqualified request (active profile=default) for a session stored under a
|
||||
foreign profile must 404 — not read or refresh the foreign session."""
|
||||
foreign profile must yield 409 ``session_profile_mismatch`` (so the client
|
||||
can offer to switch to it per #5419) — not 404, which would mask a
|
||||
legitimate owned-by-other-profile session as missing and trigger a
|
||||
destructive self-heal in the frontend (#7710)."""
|
||||
foreign = _FakeSession("foreign_existing_001", "other")
|
||||
monkeypatch.setattr(routes.Session, "load", staticmethod(lambda sid: foreign))
|
||||
monkeypatch.setattr(routes, "_get_active_profile_name", lambda: "default")
|
||||
@@ -88,8 +101,14 @@ def test_import_cli_existing_foreign_profile_unqualified_request_404(monkeypatch
|
||||
{"session_id": "foreign_existing_001"},
|
||||
)
|
||||
|
||||
assert "bad" in cap, f"expected 404, got {cap}"
|
||||
assert cap["bad"][1] == 404
|
||||
assert "ok" in cap, f"expected 409 session_profile_mismatch, got {cap}"
|
||||
assert cap["ok_status"] == 409, f"expected status 409, got {cap.get('ok_status')}"
|
||||
assert cap["ok"] == {
|
||||
"error": "Session belongs to a different profile",
|
||||
"code": "session_profile_mismatch",
|
||||
"session_id": "foreign_existing_001",
|
||||
"profile": "other",
|
||||
}
|
||||
|
||||
|
||||
def test_import_cli_existing_same_profile_still_refreshes(monkeypatch):
|
||||
|
||||
@@ -507,7 +507,18 @@ def test_session_route_blocks_hidden_sessions_before_replay_or_live_attach(monke
|
||||
handler = _FakeHandler()
|
||||
routes.handle_get(handler, urlparse("/api/sessions/hidden_session/events"))
|
||||
|
||||
assert cap["bad"] == ("Session not found", 404)
|
||||
# #7710: the generic request-guard now mirrors the detail-load
|
||||
# endpoint's contract — a session owned by a KNOWN other profile
|
||||
# yields 409 ``session_profile_mismatch`` so the client can offer
|
||||
# to switch to it (#5419). The 404 self-heal path is preserved for
|
||||
# the None-profile (unknown/legacy) case.
|
||||
assert cap.get("status") == 409, cap
|
||||
assert cap.get("ok") == {
|
||||
"error": "Session belongs to a different profile",
|
||||
"code": "session_profile_mismatch",
|
||||
"session_id": "hidden_session",
|
||||
"profile": "other",
|
||||
}
|
||||
|
||||
|
||||
def test_session_route_live_delivery_skips_replayed_active_run_items(monkeypatch):
|
||||
@@ -583,6 +594,64 @@ def test_session_route_live_delivery_skips_replayed_active_run_items(monkeypatch
|
||||
assert stream.unsubscribed is True
|
||||
|
||||
|
||||
def test_session_route_replay_skips_metering_rows(monkeypatch):
|
||||
import api.routes as routes
|
||||
|
||||
class _FakeStream:
|
||||
def __init__(self):
|
||||
self.q = queue.Queue()
|
||||
self.q.put_nowait(("stream_end", {"status": "done"}, "run_active:2"))
|
||||
self.unsubscribed = False
|
||||
|
||||
def subscribe_with_snapshot(self):
|
||||
return self.q, {"last_event_id": "run_active:1", "offline_buffered_events": 1}
|
||||
|
||||
def unsubscribe(self, q):
|
||||
self.unsubscribed = q is self.q
|
||||
|
||||
stream = _FakeStream()
|
||||
monkeypatch.setattr(routes, "_session_id_visible_to_request_profile", lambda *_args, **_kwargs: True)
|
||||
monkeypatch.setattr(
|
||||
routes,
|
||||
"get_session",
|
||||
lambda sid, metadata_only=False: SimpleNamespace(
|
||||
session_id=sid,
|
||||
compact=lambda **_kwargs: {"session_id": sid, "title": "Session"},
|
||||
),
|
||||
)
|
||||
monkeypatch.setattr(routes, "_active_run_stream_for_session", lambda *_args, **_kwargs: "run_active")
|
||||
monkeypatch.setattr(routes, "STREAMS", {"run_active": stream})
|
||||
import api.config as config
|
||||
monkeypatch.setattr(config, "STREAMS", {"run_active": stream})
|
||||
# Both the pre-attach replay and the post-attach reconciliation draw from the
|
||||
# same journal; a metering row among them must never reach the SSE body.
|
||||
monkeypatch.setattr(
|
||||
routes,
|
||||
"read_session_run_events",
|
||||
lambda *_args, **_kwargs: {
|
||||
"status": "ok",
|
||||
"events": [
|
||||
{"run_id": "run_prev", "seq": 1, "event": "token", "payload": {"text": "replayed"}, "event_id": "run_prev:1"},
|
||||
{"run_id": "run_prev", "seq": 2, "event": "metering", "payload": {"tps": 47.3}, "event_id": "run_prev:2"},
|
||||
],
|
||||
},
|
||||
)
|
||||
|
||||
handler = _FakeHandler()
|
||||
routes._handle_session_sse_stream_for_session(
|
||||
handler,
|
||||
urlparse("/api/sessions/session_1/events?after_event_id=run_prev:0"),
|
||||
"session_1",
|
||||
)
|
||||
|
||||
body = handler.wfile.getvalue().decode("utf-8")
|
||||
assert "id: run_prev:1\n" in body
|
||||
assert "event: token\n" in body
|
||||
assert "event: metering\n" not in body
|
||||
assert "id: run_prev:2\n" not in body
|
||||
assert stream.unsubscribed is True
|
||||
|
||||
|
||||
def test_session_route_breaks_on_terminal_even_when_reconciliation_replayed_it(monkeypatch):
|
||||
"""CORE regression (#5677 gate r3): if reconciliation replays the active run's
|
||||
TERMINAL event at the subscribe cutoff, the live queue's copy is deduped — but
|
||||
|
||||
@@ -0,0 +1,302 @@
|
||||
"""#5311 follow-up: OpenCode Go uses the live Hermes CLI catalog again.
|
||||
|
||||
Background: #5611 made WebUI skip the live ``/v1/models`` probe for OpenCode Go
|
||||
because the probe returned public-catalog models not enabled on the Go tier.
|
||||
That premise no longer holds: Hermes core v0.20.5 (commit fcbd1076, tag
|
||||
v2026.8.19) ships a Go-specific provider profile that probes
|
||||
``https://opencode.ai/zen/go/v1/models`` directly and merges results
|
||||
live-first with its curated Go list (hermes_cli/models.py, #49129). With the
|
||||
correct Go-tier endpoint in core, WebUI should delegate like every other
|
||||
provider (#1240): live catalog first, the curated static ``_PROVIDER_MODELS``
|
||||
list as offline fallback only.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import sys
|
||||
import types
|
||||
|
||||
import api.config as config
|
||||
import api.profiles as profiles
|
||||
|
||||
|
||||
_PROVIDER_ENV_VARS = (
|
||||
"OPENCODE_ZEN_API_KEY",
|
||||
"OPENCODE_GO_API_KEY",
|
||||
"OPENCODE_API_KEY",
|
||||
)
|
||||
|
||||
|
||||
def _scrub_provider_env(monkeypatch):
|
||||
for name in _PROVIDER_ENV_VARS:
|
||||
monkeypatch.delenv(name, raising=False)
|
||||
|
||||
|
||||
def _install_fake_hermes_cli(
|
||||
monkeypatch,
|
||||
*,
|
||||
provider_id: str,
|
||||
live_ids,
|
||||
core_version: str = "0.20.5",
|
||||
raise_on_lookup: bool = False,
|
||||
):
|
||||
"""Install a hermes_cli stub that reports one authenticated provider."""
|
||||
fake_pkg = types.ModuleType("hermes_cli")
|
||||
fake_pkg.__path__ = []
|
||||
fake_pkg.__version__ = core_version
|
||||
|
||||
fake_models = types.ModuleType("hermes_cli.models")
|
||||
fake_models.list_available_providers = lambda: [
|
||||
{"id": provider_id, "authenticated": True}
|
||||
]
|
||||
|
||||
calls: list[str] = []
|
||||
|
||||
def provider_model_ids(pid):
|
||||
calls.append(pid)
|
||||
if raise_on_lookup:
|
||||
raise RuntimeError("simulated provider_model_ids failure")
|
||||
return list(live_ids) if pid == provider_id else []
|
||||
|
||||
fake_models.provider_model_ids = provider_model_ids
|
||||
|
||||
fake_auth = types.ModuleType("hermes_cli.auth")
|
||||
|
||||
def get_auth_status(pid):
|
||||
if pid == provider_id:
|
||||
return {"logged_in": True, "key_source": ""}
|
||||
return {"logged_in": False, "key_source": ""}
|
||||
|
||||
fake_auth.get_auth_status = get_auth_status
|
||||
|
||||
monkeypatch.setitem(sys.modules, "hermes_cli", fake_pkg)
|
||||
monkeypatch.setitem(sys.modules, "hermes_cli.models", fake_models)
|
||||
monkeypatch.setitem(sys.modules, "hermes_cli.auth", fake_auth)
|
||||
monkeypatch.delitem(sys.modules, "agent.credential_pool", raising=False)
|
||||
monkeypatch.delitem(sys.modules, "agent", raising=False)
|
||||
config.invalidate_models_cache()
|
||||
return calls
|
||||
|
||||
|
||||
def _configure(monkeypatch, tmp_path, *, provider: str, providers: dict | None = None):
|
||||
hermes_home = tmp_path / "hermes-home"
|
||||
hermes_home.mkdir()
|
||||
monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: hermes_home)
|
||||
monkeypatch.setattr(config, "_get_config_path", lambda: tmp_path / "missing-config.yaml")
|
||||
monkeypatch.setattr(config, "_models_cache_path", tmp_path / "models_cache.json")
|
||||
monkeypatch.setattr(
|
||||
config,
|
||||
"cfg",
|
||||
{
|
||||
"model": {"provider": provider, "default": ""},
|
||||
"providers": providers if providers is not None else {},
|
||||
"fallback_providers": [],
|
||||
},
|
||||
)
|
||||
monkeypatch.setattr(config, "_cfg_mtime", 0.0)
|
||||
monkeypatch.setattr(config, "_cfg_path", config._get_config_path(), raising=False)
|
||||
config.invalidate_models_cache()
|
||||
|
||||
|
||||
def _provider_group(result: dict, provider_id: str) -> dict:
|
||||
return next(g for g in result["groups"] if g.get("provider_id") == provider_id)
|
||||
|
||||
|
||||
def _ids(group: dict) -> list[str]:
|
||||
return [m.get("id") for m in group.get("models", [])]
|
||||
|
||||
|
||||
def test_opencode_go_probes_live_catalog(monkeypatch, tmp_path):
|
||||
"""The Go picker group must come from the live Hermes CLI catalog.
|
||||
|
||||
``sentinel-go-model`` is intentionally absent from every static list. If
|
||||
WebUI special-cases OpenCode Go away from the live probe again (the #5611
|
||||
stopgap), the sentinel disappears and this test fails.
|
||||
"""
|
||||
_scrub_provider_env(monkeypatch)
|
||||
calls = _install_fake_hermes_cli(
|
||||
monkeypatch,
|
||||
provider_id="opencode-go",
|
||||
live_ids=["sentinel-go-model", "kimi-k3"],
|
||||
)
|
||||
_configure(monkeypatch, tmp_path, provider="opencode-go")
|
||||
|
||||
result = config.get_available_models()
|
||||
group = _provider_group(result, "opencode-go")
|
||||
|
||||
assert calls == ["opencode-go"]
|
||||
ids = _ids(group)
|
||||
assert "sentinel-go-model" in ids
|
||||
assert "kimi-k3" in ids
|
||||
assert all(m.get("label") for m in group["models"])
|
||||
|
||||
|
||||
def test_opencode_go_old_core_ignores_nonempty_generic_catalog(monkeypatch, tmp_path):
|
||||
"""Pre-v0.20.5 cores must not leak their generic public catalog into Go."""
|
||||
_scrub_provider_env(monkeypatch)
|
||||
calls = _install_fake_hermes_cli(
|
||||
monkeypatch,
|
||||
provider_id="opencode-go",
|
||||
live_ids=["go-ineligible-old-core-sentinel"],
|
||||
core_version="0.20.4",
|
||||
)
|
||||
_configure(monkeypatch, tmp_path, provider="opencode-go")
|
||||
|
||||
result = config.get_available_models()
|
||||
group = _provider_group(result, "opencode-go")
|
||||
|
||||
assert calls == []
|
||||
ids = _ids(group)
|
||||
assert "go-ineligible-old-core-sentinel" not in ids
|
||||
assert "kimi-k3" in ids
|
||||
|
||||
|
||||
def test_opencode_go_static_fallback_when_probe_fails(monkeypatch, tmp_path):
|
||||
"""Offline / CLI failure must fall back to the curated static list."""
|
||||
_scrub_provider_env(monkeypatch)
|
||||
calls = _install_fake_hermes_cli(
|
||||
monkeypatch,
|
||||
provider_id="opencode-go",
|
||||
live_ids=[],
|
||||
raise_on_lookup=True,
|
||||
)
|
||||
_configure(monkeypatch, tmp_path, provider="opencode-go")
|
||||
|
||||
result = config.get_available_models()
|
||||
group = _provider_group(result, "opencode-go")
|
||||
|
||||
assert calls == ["opencode-go"]
|
||||
ids = _ids(group)
|
||||
assert "sentinel-go-model" not in ids
|
||||
assert "glm-5" in ids
|
||||
assert "kimi-k2.5" in ids
|
||||
|
||||
|
||||
def test_opencode_go_config_allowlist_still_wins(monkeypatch, tmp_path):
|
||||
"""An explicit ``providers.opencode-go.models`` allowlist remains the
|
||||
local source of truth (#644) and is not bypassed by the live probe."""
|
||||
_scrub_provider_env(monkeypatch)
|
||||
calls = _install_fake_hermes_cli(
|
||||
monkeypatch,
|
||||
provider_id="opencode-go",
|
||||
live_ids=["sentinel-go-model"],
|
||||
)
|
||||
_configure(
|
||||
monkeypatch,
|
||||
tmp_path,
|
||||
provider="opencode-go",
|
||||
providers={"opencode-go": {"models": ["glm-5"]}},
|
||||
)
|
||||
|
||||
result = config.get_available_models()
|
||||
group = _provider_group(result, "opencode-go")
|
||||
|
||||
assert calls == [] # probe never consulted
|
||||
assert _ids(group) == ["glm-5"]
|
||||
|
||||
|
||||
# ── Static fallback list contract ─────────────────────────────────────
|
||||
# The remaining tests pin the offline fallback list itself to Hermes core's
|
||||
# current curated contract (hermes_cli/models_catalog_static.py, core main
|
||||
# 2026-09-10; the list originated in v0.20.5, commit fcbd1076). Core owns
|
||||
# the sync duty against the live
|
||||
# https://opencode.ai/zen/go/v1/models endpoint and
|
||||
# https://opencode.ai/docs/go/, and WebUI only mirrors. This catches drift
|
||||
# in either direction: ids core added and ids core dropped (e.g.
|
||||
# ox-alpha-free, delisted from the Go relay on 2026-09-09).
|
||||
|
||||
import ast
|
||||
from pathlib import Path
|
||||
|
||||
ROOT = Path(__file__).resolve().parent.parent
|
||||
CONFIG_PATH = ROOT / "api" / "config.py"
|
||||
CONFIG = CONFIG_PATH.read_text(encoding="utf-8")
|
||||
|
||||
EXPECTED_OPENCODE_GO_MODEL_IDS = [
|
||||
"kimi-k3",
|
||||
"kimi-k2.7-code",
|
||||
"kimi-k2.6",
|
||||
"kimi-k2.5",
|
||||
"gpt-5.6-luna",
|
||||
"grok-4.5",
|
||||
"glm-5.3",
|
||||
"glm-5.3-flash",
|
||||
"glm-5.2",
|
||||
"glm-5.1",
|
||||
"glm-5",
|
||||
"mimo-v2.5-pro",
|
||||
"mimo-v2.5",
|
||||
"mimo-v2-pro",
|
||||
"mimo-v2-omni",
|
||||
"minimax-m3",
|
||||
"minimax-m2.7",
|
||||
"minimax-m2.5",
|
||||
"deepseek-v4-pro",
|
||||
"deepseek-v4-flash",
|
||||
"qwen3.8-max",
|
||||
"qwen3.7-max",
|
||||
"qwen3.7-plus",
|
||||
"qwen3.6-plus",
|
||||
"qwen3.5-plus",
|
||||
"hy3",
|
||||
"hy3-preview",
|
||||
"muse-spark-1.2-contributor",
|
||||
"muse-spark-1.3-contributor",
|
||||
]
|
||||
|
||||
|
||||
def _opencode_go_static_models():
|
||||
tree = ast.parse(CONFIG, filename=str(CONFIG_PATH))
|
||||
for node in tree.body:
|
||||
if not isinstance(node, ast.Assign):
|
||||
continue
|
||||
if not any(isinstance(target, ast.Name) and target.id == "_PROVIDER_MODELS" for target in node.targets):
|
||||
continue
|
||||
provider_models = ast.literal_eval(node.value)
|
||||
return provider_models["opencode-go"]
|
||||
raise AssertionError("_PROVIDER_MODELS assignment not found")
|
||||
|
||||
|
||||
def test_opencode_go_static_models_match_core_catalog_snapshot():
|
||||
models = _opencode_go_static_models()
|
||||
assert [model["id"] for model in models] == EXPECTED_OPENCODE_GO_MODEL_IDS
|
||||
|
||||
|
||||
def test_opencode_go_static_models_match_installed_core_curated_list():
|
||||
"""Exercise the actual Agent contract for the supported release pair.
|
||||
|
||||
The snapshot mirrors core main's curated catalog. A released (installed)
|
||||
core can lag that sync by a few days — e.g. v2026.9.7 still lists
|
||||
``ox-alpha-free``, which core removed on 2026-09-09 after the Go relay
|
||||
delisted it. Equality binds only once the installed core carries the
|
||||
mirrored sync; the marker for "lags the sync" is the delisted id still
|
||||
being present. Anything else that differs is real drift and must fail.
|
||||
"""
|
||||
import pytest
|
||||
|
||||
hermes_cli = pytest.importorskip(
|
||||
"hermes_cli", reason="hermes-agent is not a WebUI test dependency"
|
||||
)
|
||||
if not config._hermes_cli_supports_opencode_go_live_catalog():
|
||||
pytest.skip(f"installed hermes-agent {hermes_cli.__version__} predates v0.20.5")
|
||||
models_mod = pytest.importorskip("hermes_cli.models")
|
||||
installed = list(models_mod._PROVIDER_MODELS["opencode-go"])
|
||||
if "ox-alpha-free" in installed:
|
||||
pytest.skip(
|
||||
"installed hermes-agent catalog predates core's 2026-09-09 sync "
|
||||
"(still lists relay-delisted 'ox-alpha-free'); equality binds "
|
||||
"once a release carrying that sync is installed"
|
||||
)
|
||||
assert [model["id"] for model in _opencode_go_static_models()] == installed
|
||||
|
||||
|
||||
def test_opencode_go_recent_additions_have_human_labels():
|
||||
labels = {model["id"]: model["label"] for model in _opencode_go_static_models()}
|
||||
assert labels["gpt-5.6-luna"] == "GPT 5.6 Luna"
|
||||
assert labels["kimi-k3"] == "Kimi K3"
|
||||
assert labels["glm-5.3"] == "GLM-5.3"
|
||||
assert labels["grok-4.5"] == "Grok 4.5"
|
||||
assert labels["qwen3.8-max"] == "Qwen3.8 Max"
|
||||
assert labels["muse-spark-1.2-contributor"] == "Muse Spark 1.2 Contributor"
|
||||
assert labels["muse-spark-1.3-contributor"] == "Muse Spark 1.3 Contributor"
|
||||
assert labels["glm-5.3-flash"] == "GLM-5.3 Flash"
|
||||
@@ -1,72 +0,0 @@
|
||||
"""#5311 / #5611: OpenCode Go's generic live probe used to return public-catalog
|
||||
models not enabled on the Go tier, so the picker intentionally falls back to a curated
|
||||
static _PROVIDER_MODELS list. Keep that curated list in sync with the public Go docs
|
||||
and documented Go models endpoint while excluding preview/free-only Zen models.
|
||||
"""
|
||||
|
||||
import ast
|
||||
from pathlib import Path
|
||||
|
||||
ROOT = Path(__file__).resolve().parent.parent
|
||||
CONFIG_PATH = ROOT / "api" / "config.py"
|
||||
CONFIG = CONFIG_PATH.read_text(encoding="utf-8")
|
||||
|
||||
EXPECTED_OPENCODE_GO_MODEL_IDS = [
|
||||
"minimax-m3",
|
||||
"minimax-m2.7",
|
||||
"minimax-m2.5",
|
||||
"kimi-k2.7-code",
|
||||
"kimi-k2.6",
|
||||
"kimi-k2.5",
|
||||
"glm-5.2",
|
||||
"glm-5.1",
|
||||
"glm-5",
|
||||
"deepseek-v4-pro",
|
||||
"deepseek-v4-flash",
|
||||
"qwen3.7-max",
|
||||
"qwen3.7-plus",
|
||||
"qwen3.6-plus",
|
||||
"qwen3.5-plus",
|
||||
"mimo-v2-pro",
|
||||
"mimo-v2-omni",
|
||||
"mimo-v2.5-pro",
|
||||
"mimo-v2.5",
|
||||
]
|
||||
|
||||
|
||||
def _opencode_go_static_models():
|
||||
tree = ast.parse(CONFIG, filename=str(CONFIG_PATH))
|
||||
for node in tree.body:
|
||||
if not isinstance(node, ast.Assign):
|
||||
continue
|
||||
if not any(isinstance(target, ast.Name) and target.id == "_PROVIDER_MODELS" for target in node.targets):
|
||||
continue
|
||||
provider_models = ast.literal_eval(node.value)
|
||||
return provider_models["opencode-go"]
|
||||
raise AssertionError("_PROVIDER_MODELS assignment not found")
|
||||
|
||||
|
||||
def test_opencode_go_skips_live_models_probe():
|
||||
# The provider-loop must special-case opencode-go to skip the old generic
|
||||
# live probe and fall through to this curated static list.
|
||||
body = CONFIG[CONFIG.index("def get_available_models"):]
|
||||
body = body[: body.index("\ndef ", 1)]
|
||||
assert 'elif pid == "opencode-go":' in body
|
||||
idx = body.index('elif pid == "opencode-go":')
|
||||
branch = body[idx: idx + 400]
|
||||
assert "_models_from_live_provider_ids" not in branch.split("else:")[0]
|
||||
|
||||
|
||||
def test_opencode_go_static_models_match_documented_endpoint_snapshot():
|
||||
# Snapshot from https://opencode.ai/zen/go/v1/models on 2026-07-07.
|
||||
# This catches stale-list regressions such as missing GLM-5.2 / MiniMax M3.
|
||||
models = _opencode_go_static_models()
|
||||
assert [model["id"] for model in models] == EXPECTED_OPENCODE_GO_MODEL_IDS
|
||||
|
||||
|
||||
def test_opencode_go_recent_additions_have_human_labels():
|
||||
labels = {model["id"]: model["label"] for model in _opencode_go_static_models()}
|
||||
assert labels["glm-5.2"] == "GLM-5.2"
|
||||
assert labels["minimax-m3"] == "MiniMax M3"
|
||||
assert labels["kimi-k2.7-code"] == "Kimi K2.7 Code"
|
||||
assert labels["qwen3.7-max"] == "Qwen3.7 Max"
|
||||
@@ -0,0 +1,223 @@
|
||||
"""Regression tests for issue #7056 — login page must hide the password form
|
||||
when password auth is disabled (e.g. native OIDC configured and
|
||||
``HERMES_WEBUI_PASSWORD`` unset).
|
||||
|
||||
The original bug: ``/api/auth/status`` correctly reports
|
||||
``password_auth_enabled: false`` when password auth is disabled, but
|
||||
``/login`` still rendered the password input, the "Sign in" button,
|
||||
and the "Continue with SSO" entry point — and silently 401-ed every
|
||||
submit. The fix routes the password input / submit / passkey controls
|
||||
through a single ``{{PASSWORD_FORM_HTML}}`` placeholder in the
|
||||
template, which the handler populates with the live markup only when
|
||||
``is_password_auth_enabled()`` is True.
|
||||
|
||||
The OIDC SSO button stays the sole path when password auth is disabled.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import io
|
||||
from unittest import mock
|
||||
|
||||
import pytest
|
||||
|
||||
import api.routes as routes
|
||||
from api.routes import _LOGIN_PAGE_HTML
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Source-shape: the template + handler must agree on a single placeholder
|
||||
# for the password controls, and the handler must call the live predicate
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_login_template_uses_password_form_placeholder() -> None:
|
||||
"""The login template MUST carry a single ``{{PASSWORD_FORM_HTML}}`` placeholder.
|
||||
|
||||
A silent reversion to the previous multi-line ``<input>`` +
|
||||
``<button>`` + ``<button>`` shape fails the suite.
|
||||
"""
|
||||
assert "{{PASSWORD_FORM_HTML}}" in _LOGIN_PAGE_HTML, (
|
||||
"the login template must use a {{PASSWORD_FORM_HTML}} placeholder "
|
||||
"so the handler can route the password controls through the "
|
||||
"``is_password_auth_enabled()`` predicate"
|
||||
)
|
||||
|
||||
|
||||
def test_login_template_does_not_render_password_input_inline() -> None:
|
||||
"""The login template MUST NOT carry a literal ``<input type="password"``.
|
||||
|
||||
The fix replaced the inline input with the placeholder; a silent
|
||||
reversion that re-adds the inline input fails the suite.
|
||||
"""
|
||||
assert '<input type="password"' not in _LOGIN_PAGE_HTML, (
|
||||
"the login template must not carry an inline <input type=\"password\">; "
|
||||
"use the {{PASSWORD_FORM_HTML}} placeholder so the handler can "
|
||||
"gate the password form on is_password_auth_enabled()"
|
||||
)
|
||||
|
||||
|
||||
def test_login_handler_calls_is_password_auth_enabled() -> None:
|
||||
"""The /login branch in handle_get MUST call ``is_password_auth_enabled``.
|
||||
|
||||
Pins the contract that the live predicate — not a stale cached
|
||||
flag, not a config-only check — gates the password controls. A
|
||||
silent reversion to a hard-coded `True` (or any pre-baked
|
||||
assumption about the environment) fails the suite.
|
||||
"""
|
||||
src = (routes.__file__).read_text(encoding="utf-8") if hasattr(routes.__file__, "read_text") else open(routes.__file__).read()
|
||||
# Find the /login branch
|
||||
login_idx = src.index('if parsed.path == "/login":')
|
||||
# Find the next return statement that closes the /login branch
|
||||
end_idx = src.index("content_type=\"text/html; charset=utf-8\")", login_idx)
|
||||
branch = src[login_idx:end_idx]
|
||||
assert "is_password_auth_enabled" in branch, (
|
||||
"the /login branch must call is_password_auth_enabled() so the "
|
||||
"password form visibility tracks /api/auth/status at request time"
|
||||
)
|
||||
assert "PASSWORD_FORM_HTML" in branch, (
|
||||
"the /login branch must populate the {{PASSWORD_FORM_HTML}} "
|
||||
"placeholder with the live password form markup"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Behavioural: walk the /login handler with a real ``ParsedResult`` and
|
||||
# a minimal fake handler. We mock the side-effecting helpers (settings
|
||||
# + OIDC + auth predicate) so the test only covers the placeholder
|
||||
# logic, not the whole login stack.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class _FakeHandler:
|
||||
def __init__(self) -> None:
|
||||
self.status: int | None = None
|
||||
self.headers: dict[str, str] = {}
|
||||
self.wfile = io.BytesIO()
|
||||
self.command = "GET"
|
||||
self.path = "/login"
|
||||
self.client_address = ("127.0.0.1", 12345)
|
||||
self.request_version = "HTTP/1.1"
|
||||
|
||||
def send_response(self, status):
|
||||
self.status = status
|
||||
|
||||
def send_header(self, key, value):
|
||||
self.headers[key] = value
|
||||
|
||||
def end_headers(self):
|
||||
pass
|
||||
|
||||
|
||||
def _stub_oidc(enabled: bool):
|
||||
"""Mock the OIDC predicate the login branch uses to decide SSO button visibility.
|
||||
|
||||
``_oidc_login_html(parsed)`` is a module-level function the login
|
||||
branch calls; the mock has to accept a single positional ``parsed``
|
||||
argument and return a stable string. With OIDC enabled the
|
||||
fixture returns a marker substring (``oidc-login``) so the
|
||||
behavioural tests can assert its presence; with OIDC disabled it
|
||||
returns the empty string the real implementation returns.
|
||||
"""
|
||||
def _stub(parsed):
|
||||
del parsed
|
||||
if enabled:
|
||||
return (
|
||||
'<a id="oidc-login" class="oidc-login" '
|
||||
'href="/api/auth/oidc/start">Continue with SSO</a>'
|
||||
)
|
||||
return ""
|
||||
|
||||
return mock.patch.object(routes, "_oidc_login_html", _stub)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def patched_login(monkeypatch):
|
||||
"""Set up the login branch's external dependencies for behavioural tests.
|
||||
|
||||
Yields a (routes, parsed) tuple. Each test patches
|
||||
``is_password_auth_enabled`` and ``_oidc_login_html`` itself so the
|
||||
scenario is explicit.
|
||||
"""
|
||||
monkeypatch.setattr(
|
||||
routes, "_LOGIN_LOCALE",
|
||||
{
|
||||
"en": {
|
||||
"lang": "en",
|
||||
"title": "Sign in",
|
||||
"subtitle": "Sign in to continue",
|
||||
"placeholder": "Password",
|
||||
"btn": "Sign in",
|
||||
"invalid_pw": "Invalid password",
|
||||
"conn_failed": "Connection failed",
|
||||
}
|
||||
},
|
||||
)
|
||||
monkeypatch.setattr(routes, "_resolve_login_locale_key", lambda _lang: "en")
|
||||
monkeypatch.setattr(routes, "load_settings", lambda: {"bot_name": "Hermes"})
|
||||
from api import updates as _updates
|
||||
monkeypatch.setattr(_updates, "WEBUI_VERSION", "test-0.0.0")
|
||||
import html as _html
|
||||
monkeypatch.setattr(routes, "_html", _html)
|
||||
from urllib.parse import urlparse
|
||||
yield routes, urlparse("/login?next=/x")
|
||||
|
||||
|
||||
def test_login_renders_password_form_when_password_auth_enabled(patched_login, monkeypatch) -> None:
|
||||
routes, parsed = patched_login
|
||||
monkeypatch.setattr(
|
||||
"api.auth.is_password_auth_enabled", lambda: True, raising=False
|
||||
)
|
||||
with _stub_oidc(enabled=False):
|
||||
handler = _FakeHandler()
|
||||
routes.handle_get(handler, parsed)
|
||||
html = handler.wfile.getvalue().decode("utf-8", "replace")
|
||||
|
||||
assert '<input type="password"' in html, html
|
||||
assert 'id="pw"' in html, html
|
||||
assert '<button type="submit"' in html, html
|
||||
assert 'id="passkey-login"' in html, html
|
||||
|
||||
|
||||
def test_login_hides_password_form_when_password_auth_disabled(patched_login, monkeypatch) -> None:
|
||||
"""With password auth disabled (e.g. OIDC-only), the form must NOT render
|
||||
the password input / submit / passkey controls. The OIDC SSO entry
|
||||
point (when OIDC is enabled) stays the sole path.
|
||||
"""
|
||||
routes, parsed = patched_login
|
||||
monkeypatch.setattr(
|
||||
"api.auth.is_password_auth_enabled", lambda: False, raising=False
|
||||
)
|
||||
with _stub_oidc(enabled=True):
|
||||
handler = _FakeHandler()
|
||||
routes.handle_get(handler, parsed)
|
||||
html = handler.wfile.getvalue().decode("utf-8", "replace")
|
||||
|
||||
# The password controls are gone.
|
||||
assert '<input type="password"' not in html, html
|
||||
assert 'id="pw"' not in html, html
|
||||
assert '<button type="submit"' not in html, html
|
||||
assert 'id="passkey-login"' not in html, html
|
||||
# The OIDC SSO entry point is still present (the user must still
|
||||
# be able to log in — just via SSO, not via password). We assert
|
||||
# the <a id="oidc-login"> element rather than the bare substring
|
||||
# because the template's CSS class ``.oidc-login`` would match
|
||||
# the page even when OIDC is disabled.
|
||||
assert 'id="oidc-login"' in html, html
|
||||
|
||||
|
||||
def test_login_hides_oidc_button_when_oidc_disabled(patched_login, monkeypatch) -> None:
|
||||
"""Sanity: with password auth disabled AND OIDC disabled, the page is
|
||||
effectively empty — no controls. This pre-existing behaviour is
|
||||
unchanged by #7056 (the fix only adds the password-disabled path).
|
||||
"""
|
||||
routes, parsed = patched_login
|
||||
monkeypatch.setattr(
|
||||
"api.auth.is_password_auth_enabled", lambda: False, raising=False
|
||||
)
|
||||
with _stub_oidc(enabled=False):
|
||||
handler = _FakeHandler()
|
||||
routes.handle_get(handler, parsed)
|
||||
html = handler.wfile.getvalue().decode("utf-8", "replace")
|
||||
|
||||
assert '<input type="password"' not in html, html
|
||||
assert 'id="oidc-login"' not in html, html
|
||||
@@ -0,0 +1,259 @@
|
||||
"""Regression tests for issue #7290 — configured-model badge must not leak a
|
||||
``{provider}/{model}``-prefixed alias when the native model id itself
|
||||
contains a slash.
|
||||
|
||||
When the active provider is a built-in plugin provider (e.g. ``commandcode``)
|
||||
and the configured default model id contains a slash (e.g.
|
||||
``deepseek/deepseek-v4-flash``), the configured-model badge builder used
|
||||
to emit three key variants for the same configured identity:
|
||||
|
||||
- the bare native id (``deepseek/deepseek-v4-flash``)
|
||||
- a slash-prefixed alias (``commandcode/deepseek/deepseek-v4-flash``)
|
||||
- the ``@provider:model``-prefixed form (``@commandcode:deepseek/deepseek-v4-flash``)
|
||||
|
||||
The slash-prefixed alias was a synthetic key the catalog never actually
|
||||
exposed — the WebUI dropdown rendered it as a real option, selecting it
|
||||
sent ``model=commandcode/deepseek/deepseek-v4-flash`` to the agent, and
|
||||
the provider rejected it with ``HTTP 400: Model ... is not supported on
|
||||
this endpoint``. The session also persisted the wrong id, so reload
|
||||
replayed the failure.
|
||||
|
||||
The fix removes the ``{provider}/{model}`` synthetic alias. Bare native
|
||||
id (which is what the catalog actually exposes) and ``@provider:model``
|
||||
(which is the documented prefix form for named custom providers) remain.
|
||||
Multi-slash, URI-scheme, host:port, and ``@provider:model``-prefixed
|
||||
model ids all keep working — only the synthesised slash-prefixed alias
|
||||
disappears.
|
||||
|
||||
The pre-fix test below pins both failure modes reported in the issue:
|
||||
|
||||
1. the badge map MUST NOT contain the slash-prefixed alias
|
||||
2. selecting the bare native id from the dropdown MUST NOT produce a
|
||||
request whose ``model`` field is the slash-prefixed alias
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
import textwrap
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parents[1]
|
||||
|
||||
|
||||
def _build_badges(active_provider, default_model, groups, fallback_providers=None):
|
||||
"""Inline-exec the live ``_build_configured_model_badges`` and return its result.
|
||||
|
||||
``_norm_model_id`` is inlined as a stand-in (its full body is part of
|
||||
the production source and is exercised by ``test_norm_model_id_*``
|
||||
siblings). Mirrors the existing pattern in
|
||||
``test_duplicate_slash_id_primary_badge_sticks_to_matching_provider_only``.
|
||||
"""
|
||||
src = (REPO_ROOT / "api" / "config.py").read_text(encoding="utf-8")
|
||||
start = src.index("def _build_configured_model_badges() -> dict[str, dict[str, str]]:")
|
||||
end = src.index(" return badges", start) + len(" return badges")
|
||||
fn_src = textwrap.dedent(src[start:end])
|
||||
|
||||
scope = {
|
||||
"active_provider": active_provider,
|
||||
"default_model": default_model,
|
||||
"cfg": {"fallback_providers": fallback_providers or []},
|
||||
"groups": groups,
|
||||
"_resolve_provider_alias": lambda provider: provider,
|
||||
}
|
||||
# The live source uses a different `_norm_model_id` than the test's
|
||||
# inline; either is fine because the badge key is the **raw** candidate,
|
||||
# not the normalized key, so the raw_candidates list is the surface this
|
||||
# test pins.
|
||||
exec(
|
||||
"def _norm_model_id(model_id):\n"
|
||||
" s=str(model_id or '').strip().lower()\n"
|
||||
" if s.startswith('@') and ':' in s: s=s.split(':',1)[1]\n"
|
||||
" if '/' in s: s=s.split('/',1)[1]\n"
|
||||
" return s.replace('-', '.')\n",
|
||||
scope,
|
||||
)
|
||||
exec(fn_src, scope)
|
||||
return scope["_build_configured_model_badges"]()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Repro: commandcode + deepseek/deepseek-v4-flash — the literal bug report
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_commandcode_deepseek_does_not_emit_slash_prefixed_alias() -> None:
|
||||
"""The badge map MUST NOT contain ``commandcode/deepseek/deepseek-v4-flash``."""
|
||||
badges = _build_badges(
|
||||
active_provider="commandcode",
|
||||
default_model="deepseek/deepseek-v4-flash",
|
||||
groups=[
|
||||
# The catalog exposes the bare native id under the commandcode
|
||||
# provider — that's the only selectable form.
|
||||
{
|
||||
"provider": "CommandCode",
|
||||
"provider_id": "commandcode",
|
||||
"models": [{"id": "deepseek/deepseek-v4-flash"}],
|
||||
}
|
||||
],
|
||||
)
|
||||
# The slash-prefixed alias MUST NOT appear in the badge map.
|
||||
assert "commandcode/deepseek/deepseek-v4-flash" not in badges, (
|
||||
f"slash-prefixed alias leaked into the badge map: {badges!r}"
|
||||
)
|
||||
# The bare native id SHOULD be the primary badge (it is the only
|
||||
# catalog-exposed form and is the correct value to send to the agent).
|
||||
assert badges.get("deepseek/deepseek-v4-flash", {}).get("role") == "primary"
|
||||
|
||||
|
||||
def test_commandcode_deepseek_at_prefixed_alias_still_present() -> None:
|
||||
"""The ``@provider:model`` prefix form MUST still resolve to the primary badge."""
|
||||
badges = _build_badges(
|
||||
active_provider="commandcode",
|
||||
default_model="deepseek/deepseek-v4-flash",
|
||||
groups=[
|
||||
{
|
||||
"provider": "CommandCode",
|
||||
"provider_id": "commandcode",
|
||||
"models": [{"id": "deepseek/deepseek-v4-flash"}],
|
||||
}
|
||||
],
|
||||
)
|
||||
assert badges.get("@commandcode:deepseek/deepseek-v4-flash", {}).get("role") == "primary", (
|
||||
f"@commandcode:deepseek/deepseek-v4-flash alias missing from badges: {badges!r}"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Dedupe across providers: a slash-bearing id shared by two providers must
|
||||
# keep the badge only on the matching provider's row, never the other way.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_duplicate_slash_id_does_not_pollute_other_provider() -> None:
|
||||
"""Per #7290 contract: deduplicate only when provider+full native model match."""
|
||||
badges = _build_badges(
|
||||
active_provider="custom:alpha",
|
||||
default_model="vendor/abc-model",
|
||||
groups=[
|
||||
{
|
||||
"provider": "Alpha",
|
||||
"provider_id": "custom:alpha",
|
||||
"models": [{"id": "vendor/abc-model"}],
|
||||
},
|
||||
{
|
||||
"provider": "Beta",
|
||||
"provider_id": "custom:beta",
|
||||
"models": [{"id": "vendor/abc-model"}],
|
||||
},
|
||||
],
|
||||
)
|
||||
# The bare native id is a real option for BOTH providers; only the
|
||||
# matching provider's row gets the PRIMARY badge.
|
||||
payload = badges.get("vendor/abc-model", {})
|
||||
assert payload.get("role") == "primary"
|
||||
assert payload.get("provider") == "custom:alpha"
|
||||
# The OTHER provider's @-prefixed form is not produced (entry is for alpha
|
||||
# only); but the alpha @-prefixed form IS produced.
|
||||
assert "vendor/abc-model" in badges
|
||||
# No synthetic slash-prefixed alias for the other provider.
|
||||
assert "custom:beta/vendor/abc-model" not in badges
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Sanity: other forms of model id (URI scheme, host:port, @provider:model)
|
||||
# must keep working — only the synthetic slash-prefixed alias is removed.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"model_id",
|
||||
[
|
||||
"deepseek-v4-pro", # bare id (no slash) — the common case
|
||||
"@anthropic:claude-opus-4-7", # @-prefixed named custom provider
|
||||
"gpt://folder/abc/latest", # URI-scheme id (#3429)
|
||||
"https://proxy.internal/models/gpt4", # URL id (#3429)
|
||||
],
|
||||
)
|
||||
def test_non_slash_models_unchanged(model_id: str) -> None:
|
||||
"""No regression: the bare / @-prefixed / URI-scheme id forms are unchanged."""
|
||||
badges = _build_badges(
|
||||
active_provider="commandcode",
|
||||
default_model=model_id,
|
||||
groups=[
|
||||
{
|
||||
"provider": "CommandCode",
|
||||
"provider_id": "commandcode",
|
||||
"models": [{"id": model_id}],
|
||||
}
|
||||
],
|
||||
)
|
||||
# The bare native id (or its @-prefixed alias) is the primary badge.
|
||||
assert badges.get(model_id, {}).get("role") == "primary", (
|
||||
f"bare id missing primary for {model_id!r}: {badges!r}"
|
||||
)
|
||||
# No synthesised slash-prefixed alias for ANY form.
|
||||
slash_aliases = [k for k in badges if k.startswith("commandcode/") and k != model_id]
|
||||
assert not slash_aliases, (
|
||||
f"synthesised slash-prefixed aliases leaked: {slash_aliases!r} in {badges!r}"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Source-shape: the ``{provider}/{model}`` synthetic alias must not appear
|
||||
# in either configured-badge builder. A silent reversion to the bug
|
||||
# leaves this regex matchable and the test fails.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"needle",
|
||||
[
|
||||
# _build_configured_model_badges (line ~8618) — 12-space indent
|
||||
'f"{provider}/{model}"',
|
||||
# _configured_model_badges_from_static_catalog (line ~6943) — 8-space
|
||||
'f"{provider}/{model}"',
|
||||
],
|
||||
)
|
||||
def test_synthetic_slash_prefix_alias_is_not_in_source(needle: str) -> None:
|
||||
"""The synthetic ``{provider}/{model}`` alias MUST NOT appear in either builder."""
|
||||
src = (REPO_ROOT / "api" / "config.py").read_text(encoding="utf-8")
|
||||
assert needle not in src, (
|
||||
f"the synthetic slash-prefixed alias leaked back into the source: {needle!r}"
|
||||
)
|
||||
|
||||
|
||||
def test_raw_candidates_only_holds_bare_and_at_prefix() -> None:
|
||||
"""The raw_candidates tuple MUST contain only the bare native id and the ``@provider:`` alias.
|
||||
|
||||
Catches both the old bug and a future regression that adds back
|
||||
*any* synthetic prefix form (e.g. ``{provider}::{model}``).
|
||||
"""
|
||||
src = (REPO_ROOT / "api" / "config.py").read_text(encoding="utf-8")
|
||||
# Find every `for candidate in (` block and verify the contents.
|
||||
# The 12-space variant is the main builder; the 8-space variant is the
|
||||
# static-catalog fallback; both must contain only bare + @-prefix.
|
||||
pattern = re.compile(
|
||||
r"for candidate in \(([\s\S]*?)\):\s*\n\s+if candidate and candidate not in raw_candidates:",
|
||||
)
|
||||
matches = list(pattern.finditer(src))
|
||||
assert matches, "raw_candidates tuple not found in api/config.py"
|
||||
for m in matches:
|
||||
body = m.group(1)
|
||||
# The body may be a single line `model, f"@{provider}:{model}"` or
|
||||
# a multi-line stack; both are accepted. We only need to verify the
|
||||
# two safe entries are present and the synthesised one is gone.
|
||||
bare = re.search(r"\bmodel\b", body)
|
||||
at_prefixed = re.search(
|
||||
r'f["\']@\{provider\}:\{model\}["\']', body
|
||||
)
|
||||
assert bare, f"raw_candidates missing the bare `model` entry: {body!r}"
|
||||
assert at_prefixed, (
|
||||
f"raw_candidates missing the @-prefixed entry, or contains a "
|
||||
f"synthesised prefix form: {body!r}"
|
||||
)
|
||||
# No synthesised prefix form like `{provider}/{model}` survives.
|
||||
assert "{provider}/{model}" not in body, (
|
||||
f"raw_candidates still emits a synthesised slash-prefixed alias: {body!r}"
|
||||
)
|
||||
@@ -0,0 +1,176 @@
|
||||
"""Regression tests for issue #7710 — cross-profile session guards return 409 + ``session_profile_mismatch`` instead of masking a known-other-profile session as 404.
|
||||
|
||||
The detail-load endpoint (and the foreign-session synthesizer) already
|
||||
distinguish "session owned by a KNOWN other profile" (return ``409
|
||||
session_profile_mismatch``) from "session missing/legacy with no
|
||||
profile stamped" (return ``404 Session not found`` so the frontend
|
||||
self-heal clears the stale URL).
|
||||
|
||||
The generic request-guard
|
||||
``_session_id_visible_to_request_profile`` (used by
|
||||
``_guard_request_session_visibility``) flattened both cases to a
|
||||
``404 Session not found``, so any cross-profile POST/PATCH/DELETE
|
||||
(archive, rename, pin, delete, …) failed with a misleading "Session
|
||||
not found" instead of the actionable 409 the detail endpoint emits.
|
||||
The fix mirrors the detail endpoint's contract in the generic guard.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parents[1]
|
||||
|
||||
|
||||
def _read_helper_body() -> str:
|
||||
"""Extract ``_session_id_visible_to_request_profile`` as importable text."""
|
||||
import textwrap
|
||||
src = (REPO_ROOT / "api" / "routes.py").read_text(encoding="utf-8")
|
||||
start = src.index("def _session_id_visible_to_request_profile(")
|
||||
end_marker = "def _stream_id_owner_session_id"
|
||||
end = src.index(end_marker, start)
|
||||
return textwrap.dedent(src[start:end])
|
||||
|
||||
|
||||
def _exec_helper(scope: dict) -> None:
|
||||
"""Execute the helper body inside ``scope``."""
|
||||
body = _read_helper_body()
|
||||
exec(body, scope)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Profile-mismatch path: known other profile → 409 + session_profile_mismatch
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class _FakeSession:
|
||||
def __init__(self, profile: str | None) -> None:
|
||||
self.profile = profile
|
||||
|
||||
|
||||
class _FakeHandler:
|
||||
"""Minimal stand-in for the routes handler the helper writes through."""
|
||||
|
||||
def __init__(self) -> None:
|
||||
self.writes: list[tuple[int, dict | str | None]] = []
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def helper_scope():
|
||||
"""Build an exec scope with the helper's dependencies stubbed.
|
||||
|
||||
``get_session`` is the only thing the helper actually calls into
|
||||
(besides the visibility predicate which is itself inlined). The
|
||||
visibility predicate is monkey-patched per-test so we can drive both
|
||||
the "match" and "mismatch" branches deterministically.
|
||||
"""
|
||||
scope: dict = {
|
||||
# j / bad — the helper writes through these. Record instead of
|
||||
# emit so the test can assert status + body.
|
||||
"j": lambda handler, payload, status=200, **_: handler.writes.append((status, payload)),
|
||||
"bad": lambda handler, msg, status=400: handler.writes.append((status, msg)),
|
||||
# get_session is required so the helper can resolve the row.
|
||||
"get_session": lambda sid, metadata_only=True: scope["_resolve"](sid),
|
||||
# is_safe_session_id — accept anything that looks like a sid.
|
||||
"is_safe_session_id": lambda sid: bool(sid) and len(str(sid)) < 256,
|
||||
}
|
||||
return scope
|
||||
|
||||
|
||||
def test_profile_mismatch_returns_409_with_code_and_profile(helper_scope) -> None:
|
||||
"""A session owned by a known other profile MUST yield 409 ``session_profile_mismatch``."""
|
||||
handler = _FakeHandler()
|
||||
helper_scope["_resolve"] = lambda sid: _FakeSession(profile="alpha")
|
||||
# Visibility predicate: returns False (profile mismatch).
|
||||
helper_scope["_session_visible_to_active_profile"] = lambda session_profile, _h: False
|
||||
|
||||
_exec_helper(helper_scope)
|
||||
fn = helper_scope["_session_id_visible_to_request_profile"]
|
||||
assert fn(handler, "sess-1") is False
|
||||
assert len(handler.writes) == 1, handler.writes
|
||||
status, body = handler.writes[0]
|
||||
assert status == 409, body
|
||||
assert body["code"] == "session_profile_mismatch"
|
||||
assert body["session_id"] == "sess-1"
|
||||
assert body["profile"] == "alpha"
|
||||
assert "different profile" in body["error"].lower()
|
||||
|
||||
|
||||
def test_unknown_profile_returns_404_for_frontend_self_heal(helper_scope) -> None:
|
||||
"""A session with ``profile=None`` MUST keep 404 so the frontend's self-heal fires."""
|
||||
handler = _FakeHandler()
|
||||
helper_scope["_resolve"] = lambda sid: _FakeSession(profile=None)
|
||||
helper_scope["_session_visible_to_active_profile"] = lambda session_profile, _h: False
|
||||
|
||||
_exec_helper(helper_scope)
|
||||
fn = helper_scope["_session_id_visible_to_request_profile"]
|
||||
assert fn(handler, "sess-1") is False
|
||||
assert len(handler.writes) == 1, handler.writes
|
||||
status, msg = handler.writes[0]
|
||||
assert status == 404
|
||||
assert msg == "Session not found"
|
||||
|
||||
|
||||
def test_visible_session_returns_no_error(helper_scope) -> None:
|
||||
"""A session owned by the active profile MUST pass the guard silently."""
|
||||
handler = _FakeHandler()
|
||||
helper_scope["_resolve"] = lambda sid: _FakeSession(profile="default")
|
||||
helper_scope["_session_visible_to_active_profile"] = lambda session_profile, _h: True
|
||||
|
||||
_exec_helper(helper_scope)
|
||||
fn = helper_scope["_session_id_visible_to_request_profile"]
|
||||
assert fn(handler, "sess-1") is True
|
||||
assert handler.writes == []
|
||||
|
||||
|
||||
def test_missing_session_returns_no_error(helper_scope) -> None:
|
||||
"""A session id that get_session cannot resolve MUST pass the guard silently (the route will 404 later)."""
|
||||
handler = _FakeHandler()
|
||||
helper_scope["_resolve"] = lambda sid: (_ for _ in ()).throw(KeyError(sid))
|
||||
|
||||
_exec_helper(helper_scope)
|
||||
fn = helper_scope["_session_id_visible_to_request_profile"]
|
||||
assert fn(handler, "sess-1") is True
|
||||
assert handler.writes == []
|
||||
|
||||
|
||||
def test_emit_error_false_suppresses_response(helper_scope) -> None:
|
||||
"""``emit_error=False`` MUST return False without writing any response (used by the guard's exemption probe)."""
|
||||
handler = _FakeHandler()
|
||||
helper_scope["_resolve"] = lambda sid: _FakeSession(profile="alpha")
|
||||
helper_scope["_session_visible_to_active_profile"] = lambda session_profile, _h: False
|
||||
|
||||
_exec_helper(helper_scope)
|
||||
fn = helper_scope["_session_id_visible_to_request_profile"]
|
||||
assert fn(handler, "sess-1", emit_error=False) is False
|
||||
assert handler.writes == []
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Source-shape: the synthesised 409 payload MUST keep the four contract
|
||||
# fields and the status code, in the helper. A silent reversion to the
|
||||
# original single-line ``bad(..., 404)`` fails the suite.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_source_emits_409_not_404_in_helper() -> None:
|
||||
"""The helper body MUST contain a 409 ``session_profile_mismatch`` branch.
|
||||
|
||||
Catches a future regression that folds the two cases back together
|
||||
(or drops the 409 status code in favour of 404). The 404 case is
|
||||
still preserved for the None-profile self-heal path, but only as a
|
||||
fall-through inside the same block — not as the sole outcome.
|
||||
"""
|
||||
body = _read_helper_body()
|
||||
assert "status=409" in body, (
|
||||
"the helper no longer emits 409 for a known-other-profile session"
|
||||
)
|
||||
assert "session_profile_mismatch" in body, (
|
||||
"the helper no longer carries the ``session_profile_mismatch`` code"
|
||||
)
|
||||
assert "Session not found" in body, (
|
||||
"the None-profile 404 self-heal path was dropped — "
|
||||
"keep the 404 for the None-profile branch so the frontend "
|
||||
"self-heal still fires for actually-missing sids"
|
||||
)
|
||||
@@ -47,8 +47,13 @@ def test_available_models_exposes_primary_and_fallback_badges():
|
||||
assert badges.get("@openai-codex:gpt-5.4", {}).get("label") == "Primary"
|
||||
assert badges.get("@copilot:gpt-4.1", {}).get("role") == "fallback"
|
||||
assert badges.get("@copilot:gpt-4.1", {}).get("label") == "Fallback 1"
|
||||
assert badges.get("anthropic/claude-haiku-4.5", {}).get("role") == "fallback"
|
||||
assert badges.get("anthropic/claude-haiku-4.5", {}).get("label") == "Fallback 2"
|
||||
# #7290: the synthesised ``{provider}/{model}`` alias is gone — the
|
||||
# native bare id and the ``@provider:model`` prefix are the only
|
||||
# keys a configured entry can publish.
|
||||
assert badges.get("claude-haiku-4.5", {}).get("role") == "fallback"
|
||||
assert badges.get("@anthropic:claude-haiku-4.5", {}).get("role") == "fallback"
|
||||
assert "anthropic/claude-haiku-4.5" not in badges
|
||||
assert badges.get("@anthropic:claude-haiku-4.5", {}).get("label") == "Fallback 2"
|
||||
|
||||
|
||||
def test_duplicate_slash_id_primary_badge_sticks_to_matching_provider_only():
|
||||
|
||||
@@ -0,0 +1,127 @@
|
||||
"""``Handler.do_GET`` itself must arm the close when a body is still queued.
|
||||
|
||||
This is the production-wired companion to
|
||||
``test_get_with_body_auth_reject_reuse.py``. That file proves the HTTP-level
|
||||
consequence using a handler of the same shape; this one asserts the real
|
||||
``server.Handler.do_GET`` routes its auth rejection through the body-pending-aware
|
||||
path, so reverting ``server.py`` to plain ``check_auth`` fails a test.
|
||||
|
||||
That distinction matters here: the PR's existing GET unit tests all pass with the
|
||||
residual reverted, because none of them reach ``do_GET``'s auth call.
|
||||
"""
|
||||
|
||||
import types
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
def _handler_double(headers, path="/api/sessions"):
|
||||
"""A handler stub shaped like the one the reject paths receive."""
|
||||
h = types.SimpleNamespace()
|
||||
h.path = path
|
||||
h.headers = headers
|
||||
h.close_connection = False
|
||||
h.command = "GET"
|
||||
h._headers_buffer = []
|
||||
h.responses = []
|
||||
h.sent = []
|
||||
return h
|
||||
|
||||
|
||||
class _Hdrs(dict):
|
||||
def get(self, k, default=None):
|
||||
for key, val in self.items():
|
||||
if key.lower() == k.lower():
|
||||
return val[0] if isinstance(val, list) else val
|
||||
return default
|
||||
|
||||
def get_all(self, k, default=None):
|
||||
out = []
|
||||
for key, val in self.items():
|
||||
if key.lower() == k.lower():
|
||||
out.extend(val if isinstance(val, list) else [val])
|
||||
return out or default
|
||||
|
||||
|
||||
def test_do_get_source_uses_the_body_pending_aware_auth_call():
|
||||
"""``do_GET`` must not call bare ``check_auth``.
|
||||
|
||||
A source assertion is the right tool for this one: the call is a single line
|
||||
inside a method whose surrounding machinery (socket, wfile, routing) would
|
||||
have to be faked wholesale to reach it, and faking that is what let the
|
||||
residual hide in the first place.
|
||||
"""
|
||||
import pathlib
|
||||
|
||||
src = pathlib.Path(__file__).resolve().parents[1] / "server.py"
|
||||
text = src.read_text()
|
||||
|
||||
start = text.index("def do_GET")
|
||||
end = text.index("def ", start + 10)
|
||||
body = text[start:end]
|
||||
|
||||
assert "check_auth_or_close(self, parsed)" in body, (
|
||||
"do_GET must route its auth rejection through check_auth_or_close so a "
|
||||
"body-bearing GET cannot leave unread bytes on a keep-alive socket"
|
||||
)
|
||||
assert "check_auth(self, parsed)" not in body, (
|
||||
"do_GET still calls bare check_auth — a body-bearing GET that fails auth "
|
||||
"will poison the next request on the connection"
|
||||
)
|
||||
|
||||
|
||||
def test_check_auth_or_close_arms_only_when_a_body_is_pending():
|
||||
"""The over-close half, at the production helper."""
|
||||
from api.auth import check_auth_or_close
|
||||
import api.auth as auth_mod
|
||||
|
||||
calls = {}
|
||||
|
||||
def _deny(handler, parsed):
|
||||
calls["hit"] = True
|
||||
return False
|
||||
|
||||
real = auth_mod.check_auth
|
||||
auth_mod.check_auth = _deny
|
||||
try:
|
||||
# Body pending -> close armed.
|
||||
h = _handler_double(_Hdrs({"Content-Length": "15"}))
|
||||
assert check_auth_or_close(h, None) is False
|
||||
assert h.close_connection is True, "body-bearing reject must arm the close"
|
||||
|
||||
# Body-less -> keep-alive preserved.
|
||||
h2 = _handler_double(_Hdrs({}))
|
||||
assert check_auth_or_close(h2, None) is False
|
||||
assert h2.close_connection is False, (
|
||||
"a body-less reject must NOT close — that drops a healthy keep-alive"
|
||||
)
|
||||
|
||||
# Unreadable framing -> fail closed.
|
||||
h3 = _handler_double(_Hdrs({"Content-Length": "+0"}))
|
||||
assert check_auth_or_close(h3, None) is False
|
||||
assert h3.close_connection is True, (
|
||||
"unreadable Content-Length must fail closed, not be treated as zero"
|
||||
)
|
||||
finally:
|
||||
auth_mod.check_auth = real
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"value",
|
||||
["\xa00", "0\x85", "+0", "-0", "0_0", "\u0660", "-5", ""],
|
||||
)
|
||||
def test_adversarial_zero_spellings_declare_a_body(value):
|
||||
"""Every spelling of "zero" that ``int()`` accepts must still fail closed.
|
||||
|
||||
``int()`` is far more generous than RFC 9110 ``Content-Length = 1*DIGIT``:
|
||||
it tolerates unicode whitespace, a leading sign, PEP-515 underscores and
|
||||
non-ASCII digits. Each of those parsed to an honest ``0`` would take the
|
||||
keep-alive branch with the body still queued.
|
||||
"""
|
||||
from api.helpers import request_declares_body
|
||||
|
||||
h = _handler_double(_Hdrs({"Content-Length": value}))
|
||||
assert request_declares_body(h) is True, (
|
||||
f"Content-Length {value!r} was treated as no-body; a queued body would "
|
||||
"then poison the next request on this connection"
|
||||
)
|
||||
File diff suppressed because it is too large.
Load diff
@@ -0,0 +1,759 @@
|
||||
"""Pooled-client regression for reject-before-read on HTTP/1.1 keep-alive.
|
||||
|
||||
The unit tests beside this one assert `close_connection` is set on each
|
||||
rejection path, and the raw-socket test drives the failure over one socket.
|
||||
Neither proves the client side: a pooled client (`http.client`) only avoids
|
||||
the broken connection when the rejection ADVERTISES `Connection: close`.
|
||||
Without the advertisement the server drops the socket silently and the next
|
||||
pooled request dies with BrokenPipeError/RemoteDisconnected — or, without the
|
||||
close at all, the unread body is parsed as the next request line and the
|
||||
client gets a 400 about a request it never sent.
|
||||
|
||||
This file drives the production handler through the real test server and
|
||||
asserts both halves: the rejection carries `Connection: close`, and a
|
||||
follow-up request on the same pooled connection succeeds.
|
||||
|
||||
The deprecated `/api/process-complete-ack` path is the probe: it answers 410
|
||||
before reading its JSON body, by design (it runs ahead of the CSRF gate so a
|
||||
stale tab gets 410, not 403).
|
||||
"""
|
||||
|
||||
import contextlib
|
||||
import http.client
|
||||
import socket
|
||||
import threading
|
||||
import urllib.parse
|
||||
from http.server import ThreadingHTTPServer
|
||||
|
||||
import pytest
|
||||
|
||||
from tests._pytest_port import BASE
|
||||
|
||||
_PORT = urllib.parse.urlparse(BASE).port
|
||||
_BODY = b'{"stale": true}'
|
||||
|
||||
|
||||
def _reject_with_body_then_pooled_followup() -> tuple[int, str | None, int]:
|
||||
"""POST a deprecated endpoint with a body, then GET on the same pool."""
|
||||
conn = http.client.HTTPConnection("127.0.0.1", _PORT, timeout=10)
|
||||
try:
|
||||
conn.request(
|
||||
"POST",
|
||||
"/api/process-complete-ack",
|
||||
body=_BODY,
|
||||
headers={"Content-Type": "application/json"},
|
||||
)
|
||||
resp = conn.getresponse()
|
||||
reject_status = resp.status
|
||||
close_header = resp.getheader("Connection")
|
||||
resp.read()
|
||||
|
||||
conn.request("GET", "/api/health/agent")
|
||||
follow = conn.getresponse()
|
||||
follow_status = follow.status
|
||||
follow.read()
|
||||
finally:
|
||||
conn.close()
|
||||
return reject_status, close_header, follow_status
|
||||
|
||||
|
||||
def test_rejected_write_advertises_close_and_pooled_followup_succeeds():
|
||||
status, close_header, follow_status = _reject_with_body_then_pooled_followup()
|
||||
assert status == 410, f"expected the deprecated-path 410, got {status}"
|
||||
assert close_header == "close", (
|
||||
f"rejection must advertise Connection: close so pooled clients don't "
|
||||
f"reuse the dead socket; got {close_header!r}"
|
||||
)
|
||||
assert follow_status == 200, (
|
||||
f"pooled follow-up after a reject-before-read must succeed; got {follow_status}"
|
||||
)
|
||||
|
||||
|
||||
# ── Framing the Content-Length gate could not see ─────────────────────────────
|
||||
#
|
||||
# Both cases below are pipelined down ONE socket against the real server: the
|
||||
# reject-before-read request first, an ordinary GET immediately after it in the
|
||||
# same write. If the rejection closes, the server answers once and EOFs and the
|
||||
# GET is never seen. If it does not, the leftover body/chunk bytes are parsed as
|
||||
# the next request line and a second response comes back — a 400 bad-syntax or
|
||||
# `501 Unsupported method` naming bytes the client never sent as a request.
|
||||
|
||||
_FOLLOWING_GET = b"GET /api/health/agent HTTP/1.1\r\nHost: 127.0.0.1\r\nAccept: */*\r\n\r\n"
|
||||
_MULTIPART_UPLOAD_PATHS = (
|
||||
"/api/upload",
|
||||
"/api/upload/extract",
|
||||
"/api/workspace/upload",
|
||||
"/api/transcribe",
|
||||
)
|
||||
_MULTIPART_PAYLOAD = (
|
||||
b'--x\r\nContent-Disposition: form-data; name="file"; filename="a.txt"\r\n'
|
||||
b"\r\nhello\r\n--x--\r\n"
|
||||
)
|
||||
_CHUNKED_MULTIPART_BODY = (
|
||||
f"{len(_MULTIPART_PAYLOAD):X}\r\n".encode() + _MULTIPART_PAYLOAD + b"\r\n0\r\n\r\n"
|
||||
)
|
||||
|
||||
|
||||
def _pipelined_after(
|
||||
request: bytes,
|
||||
*,
|
||||
stop_after: int | None = None,
|
||||
port: int | None = None,
|
||||
follow: bytes = _FOLLOWING_GET,
|
||||
) -> bytes:
|
||||
"""Send *request* and a plain GET in one write; return every byte answered.
|
||||
|
||||
Reads to EOF by default, which is the whole point of the closing cases: the
|
||||
proof is that NOTHING follows the single response. `stop_after` bounds the
|
||||
read for the keep-alive cases, where the socket stays open by design and
|
||||
reading to EOF would only burn the timeout.
|
||||
|
||||
`port`/`follow` exist for the auth-enabled cases below, which need their own
|
||||
server instance and a PUBLIC follow-up path.
|
||||
"""
|
||||
sock = socket.create_connection(("127.0.0.1", port or _PORT), timeout=10)
|
||||
try:
|
||||
sock.sendall(request + follow)
|
||||
received = b""
|
||||
while stop_after is None or received.count(b"HTTP/1.1 ") < stop_after:
|
||||
try:
|
||||
chunk = sock.recv(65536)
|
||||
except (TimeoutError, socket.timeout, ConnectionError):
|
||||
break
|
||||
if not chunk:
|
||||
break
|
||||
received += chunk
|
||||
return received
|
||||
finally:
|
||||
sock.close()
|
||||
|
||||
|
||||
def _assert_single_closed_response(answered: bytes, status: bytes, leftover: bytes) -> None:
|
||||
text = answered.decode("latin-1", errors="replace")
|
||||
assert answered.startswith(b"HTTP/1.1 " + status), text
|
||||
assert b"Connection: close" in answered, text
|
||||
assert answered.count(b"HTTP/1.1 ") == 1, (
|
||||
f"the pipelined GET was answered, so the socket stayed open: {text}"
|
||||
)
|
||||
assert leftover not in answered, f"the unread body reached the request parser: {text}"
|
||||
assert b"Bad request syntax" not in answered, text
|
||||
assert b"Unsupported method" not in answered, text
|
||||
|
||||
|
||||
@pytest.mark.parametrize("path", _MULTIPART_UPLOAD_PATHS)
|
||||
def test_chunked_upload_rejection_closes_and_cannot_poison_the_socket(path):
|
||||
"""A chunked upload has no Content-Length — the old gate read it as empty.
|
||||
|
||||
All four multipart handlers then answered 400 "No file field in request"
|
||||
with keep-alive intact, leaving the chunk bytes on the socket.
|
||||
"""
|
||||
answered = _pipelined_after(
|
||||
f"POST {path} HTTP/1.1\r\n"
|
||||
"Host: 127.0.0.1\r\n"
|
||||
"Content-Type: multipart/form-data; boundary=x\r\n"
|
||||
"Transfer-Encoding: chunked\r\n"
|
||||
"\r\n".encode() + _CHUNKED_MULTIPART_BODY
|
||||
)
|
||||
|
||||
_assert_single_closed_response(answered, b"411", _MULTIPART_PAYLOAD)
|
||||
|
||||
|
||||
def test_sidecar_get_with_a_body_closes_and_cannot_poison_the_socket():
|
||||
"""A provenance-rejected GET that carries a declared body.
|
||||
|
||||
`read_request_body=False` said "no body" and the 403 went out with
|
||||
keep-alive; the gate's repro then got `501 Unsupported method ('{}GET')` on
|
||||
the same socket. No Origin/Referer/Sec-Fetch-Site here, so provenance fails
|
||||
before the body would ever be read.
|
||||
"""
|
||||
answered = _pipelined_after(
|
||||
"GET /api/extensions/probe/sidecar/ping HTTP/1.1\r\n"
|
||||
"Host: 127.0.0.1\r\n"
|
||||
"Content-Type: application/json\r\n"
|
||||
f"Content-Length: {len(_BODY)}\r\n"
|
||||
"\r\n".encode() + _BODY
|
||||
)
|
||||
|
||||
_assert_single_closed_response(answered, b"403", _BODY)
|
||||
|
||||
|
||||
# ── Framing hidden behind a DUPLICATED header ─────────────────────────────────
|
||||
#
|
||||
# `Content-Length: 0` sent ahead of the real length: `Message.get()` returns only
|
||||
# the first value, so every reader saw an empty body and drained nothing. Both
|
||||
# cases below were reproduced live on the otherwise-fixed head, poisoning the
|
||||
# socket exactly as the single-header cases used to:
|
||||
# sidecar GET -> 403, then 400 Bad request syntax ('{"stale": true}GET /...')
|
||||
# /api/upload -> 400, then 400 Bad request syntax ('--x')
|
||||
|
||||
|
||||
def test_duplicate_content_length_sidecar_get_closes_and_cannot_poison_the_socket():
|
||||
answered = _pipelined_after(
|
||||
"GET /api/extensions/probe/sidecar/ping HTTP/1.1\r\n"
|
||||
"Host: 127.0.0.1\r\n"
|
||||
"Content-Type: application/json\r\n"
|
||||
"Content-Length: 0\r\n"
|
||||
f"Content-Length: {len(_BODY)}\r\n"
|
||||
"\r\n".encode() + _BODY
|
||||
)
|
||||
|
||||
_assert_single_closed_response(answered, b"403", _BODY)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("path", _MULTIPART_UPLOAD_PATHS)
|
||||
def test_duplicate_content_length_upload_closes_and_cannot_poison_the_socket(path):
|
||||
answered = _pipelined_after(
|
||||
f"POST {path} HTTP/1.1\r\n"
|
||||
"Host: 127.0.0.1\r\n"
|
||||
"Content-Type: multipart/form-data; boundary=x\r\n"
|
||||
"Content-Length: 0\r\n"
|
||||
f"Content-Length: {len(_MULTIPART_PAYLOAD)}\r\n"
|
||||
"\r\n".encode() + _MULTIPART_PAYLOAD
|
||||
)
|
||||
|
||||
_assert_single_closed_response(answered, b"400", _MULTIPART_PAYLOAD)
|
||||
|
||||
|
||||
# ── Framing hidden behind a BLANK header value ────────────────────────────────
|
||||
#
|
||||
# `Content-Length:` with nothing after the colon reads back as '' and was skipped
|
||||
# as "no length declared", so the rejection kept the connection alive. Both cases
|
||||
# below were reproduced live on the otherwise-fixed head:
|
||||
# sidecar GET -> 403, then 400 Bad request syntax ('{"stale": true}GET /...')
|
||||
# /api/upload -> 400, then 400 Bad request syntax ('--x')
|
||||
# `Message` collapses `Content-Length:` and `Content-Length: ` to the same '',
|
||||
# and a blank `Transfer-Encoding:` reached the allowlist of codings that "framed
|
||||
# nothing" (since deleted — see the identity section below), so each spelling
|
||||
# below poisoned the socket the same way.
|
||||
|
||||
_BLANK_FRAMING_HEADER_LINES = (
|
||||
b"Content-Length:\r\n",
|
||||
b"Content-Length: \r\n",
|
||||
b"Content-Length:\r\nContent-Length:\r\n",
|
||||
b"Content-Length: 0\r\nContent-Length:\r\n",
|
||||
b"Transfer-Encoding:\r\n",
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("framing", _BLANK_FRAMING_HEADER_LINES)
|
||||
def test_blank_framing_sidecar_get_closes_and_cannot_poison_the_socket(framing):
|
||||
answered = _pipelined_after(
|
||||
b"GET /api/extensions/probe/sidecar/ping HTTP/1.1\r\n"
|
||||
b"Host: 127.0.0.1\r\n"
|
||||
b"Content-Type: application/json\r\n" + framing + b"\r\n" + _BODY
|
||||
)
|
||||
|
||||
_assert_single_closed_response(answered, b"403", _BODY)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("path", _MULTIPART_UPLOAD_PATHS)
|
||||
@pytest.mark.parametrize(
|
||||
("framing", "status"),
|
||||
[
|
||||
(b"Content-Length:\r\n", b"400"),
|
||||
(b"Content-Length: \r\n", b"400"),
|
||||
(b"Transfer-Encoding:\r\n", b"411"),
|
||||
],
|
||||
ids=["blank-length", "whitespace-only-length", "blank-transfer-encoding"],
|
||||
)
|
||||
def test_blank_framing_upload_closes_and_cannot_poison_the_socket(path, framing, status):
|
||||
answered = _pipelined_after(
|
||||
f"POST {path} HTTP/1.1\r\n"
|
||||
"Host: 127.0.0.1\r\n"
|
||||
"Content-Type: multipart/form-data; boundary=x\r\n".encode()
|
||||
+ framing
|
||||
+ b"\r\n"
|
||||
+ _MULTIPART_PAYLOAD
|
||||
)
|
||||
|
||||
_assert_single_closed_response(answered, status, _MULTIPART_PAYLOAD)
|
||||
|
||||
|
||||
# ── Framing only `int()` reads as zero ────────────────────────────────────────
|
||||
#
|
||||
# RFC 9110 is `Content-Length = 1*DIGIT`; `int()` also takes a sign, PEP 515
|
||||
# underscores and non-ASCII whitespace padding, so each value below parsed to an
|
||||
# honest `0`, took the "every declared length agrees on zero" keep-alive branch
|
||||
# and left the payload queued. Reproduced live on the otherwise-fixed head:
|
||||
# sidecar GET -> 403 (no Connection: close), then
|
||||
# 400 Bad request syntax ('{"stale": true}GET /api/health/agent HTTP/1.1')
|
||||
# /api/upload -> 400, then 400 Bad request syntax ('--x')
|
||||
# `Transfer-Encoding: \xa0identity` is the same mistake one header over: strip()
|
||||
# erased the U+00A0 and it read back as the `identity` token. Everything here is
|
||||
# wire-reachable -- a request line decodes as latin-1, so U+00A0 and U+0085 pass
|
||||
# through untouched (a non-ASCII DIGIT such as U+0660 cannot, which is why that
|
||||
# family is pinned at the helper level in test_rejected_write_connection_close.py).
|
||||
|
||||
_MALFORMED_ZERO_FRAMING_LINES = [
|
||||
b"Content-Length: +0\r\n",
|
||||
b"Content-Length: -0\r\n",
|
||||
b"Content-Length: 0_0\r\n",
|
||||
b"Content-Length: +00\r\n",
|
||||
b"Content-Length: 0_0_0\r\n",
|
||||
b"Content-Length: \xa00\r\n",
|
||||
b"Content-Length: 0\x85\r\n",
|
||||
b"Content-Length: 0\r\nContent-Length: +0\r\n",
|
||||
b"Transfer-Encoding: \xa0identity\r\n",
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("framing", _MALFORMED_ZERO_FRAMING_LINES)
|
||||
def test_malformed_zero_framing_sidecar_get_closes_and_cannot_poison_the_socket(framing):
|
||||
answered = _pipelined_after(
|
||||
b"GET /api/extensions/probe/sidecar/ping HTTP/1.1\r\n"
|
||||
b"Host: 127.0.0.1\r\n"
|
||||
b"Content-Type: application/json\r\n" + framing + b"\r\n" + _BODY
|
||||
)
|
||||
|
||||
_assert_single_closed_response(answered, b"403", _BODY)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("path", _MULTIPART_UPLOAD_PATHS)
|
||||
@pytest.mark.parametrize(
|
||||
("framing", "status"),
|
||||
[
|
||||
(b"Content-Length: +0\r\n", b"400"),
|
||||
(b"Content-Length: 0_0\r\n", b"400"),
|
||||
(b"Content-Length: \xa00\r\n", b"400"),
|
||||
(b"Transfer-Encoding: \xa0identity\r\n", b"411"),
|
||||
],
|
||||
ids=["sign", "underscore", "nbsp-padded", "nbsp-padded-identity"],
|
||||
)
|
||||
def test_malformed_zero_framing_upload_closes_and_cannot_poison_the_socket(path, framing, status):
|
||||
answered = _pipelined_after(
|
||||
f"POST {path} HTTP/1.1\r\n"
|
||||
"Host: 127.0.0.1\r\n"
|
||||
"Content-Type: multipart/form-data; boundary=x\r\n".encode()
|
||||
+ framing
|
||||
+ b"\r\n"
|
||||
+ _MULTIPART_PAYLOAD
|
||||
)
|
||||
|
||||
_assert_single_closed_response(answered, status, _MULTIPART_PAYLOAD)
|
||||
|
||||
|
||||
# ── `Transfer-Encoding: identity` frames nothing a reader can trust ───────────
|
||||
#
|
||||
# `identity` sat in an allowlist of codings that "frame nothing", so a request
|
||||
# carrying it read back as body-less however many payload bytes followed. RFC
|
||||
# 9112 §6.3 leaves no room for that: a request whose final transfer coding is not
|
||||
# `chunked` cannot be framed by a recipient, so it must be refused and the
|
||||
# connection closed — and `http.server` decodes no transfer coding at all, so
|
||||
# `identity` is no more readable here than `chunked` is. Reproduced live on the
|
||||
# otherwise-fixed head, pipelined down one socket:
|
||||
#
|
||||
# GET /api/extensions/probe/sidecar/ping `Transfer-Encoding: identity`
|
||||
# {"stale": true}
|
||||
# -> HTTP/1.1 403 Forbidden (no Connection: close)
|
||||
# -> HTTP/1.1 400 Bad request syntax
|
||||
# ('{"stale": true}GET /api/health/agent HTTP/1.1')
|
||||
# POST /api/upload `Transfer-Encoding: identity` --x...
|
||||
# -> HTTP/1.1 400 Bad Request (no Connection: close, "No file field")
|
||||
# -> HTTP/1.1 400 Bad request syntax ('--x')
|
||||
#
|
||||
# Only the spellings that normalize to the bare token were poisoned — every
|
||||
# neighbouring value (`identity, chunked`, `identity,identity`, a lone comma, an
|
||||
# unknown coding, a non-OWS-padded token, a blank value) already missed the
|
||||
# allowlist and already closed. Those rows are in the sweep below anyway: this is
|
||||
# the fourth round in which an adjacent spelling of one framing rule survived a
|
||||
# fix, so the whole neighbourhood gets pinned rather than just the reported case.
|
||||
|
||||
_IDENTITY_FRAMING_LINES = [
|
||||
b"Transfer-Encoding: identity\r\n",
|
||||
b"Transfer-Encoding: IDENTITY\r\n",
|
||||
b"Transfer-Encoding: Identity\r\n",
|
||||
b"Transfer-Encoding: identity \r\n",
|
||||
b"Transfer-Encoding: \tidentity\t\r\n",
|
||||
b"Transfer-Encoding: identity\r\nTransfer-Encoding: identity\r\n",
|
||||
b"Transfer-Encoding: identity\r\nContent-Length: 0\r\n",
|
||||
b"Transfer-Encoding: identity\r\nContent-Length: " + str(len(_BODY)).encode() + b"\r\n",
|
||||
b"Transfer-Encoding: identity, chunked\r\n",
|
||||
b"Transfer-Encoding: chunked, identity\r\n",
|
||||
b"Transfer-Encoding: identity,identity\r\n",
|
||||
b"Transfer-Encoding: ,\r\n",
|
||||
b"Transfer-Encoding: banana\r\n",
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("framing", _IDENTITY_FRAMING_LINES)
|
||||
def test_identity_framing_sidecar_get_closes_and_cannot_poison_the_socket(framing):
|
||||
answered = _pipelined_after(
|
||||
b"GET /api/extensions/probe/sidecar/ping HTTP/1.1\r\n"
|
||||
b"Host: 127.0.0.1\r\n"
|
||||
b"Content-Type: application/json\r\n" + framing + b"\r\n" + _BODY
|
||||
)
|
||||
|
||||
_assert_single_closed_response(answered, b"403", _BODY)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("path", _MULTIPART_UPLOAD_PATHS)
|
||||
@pytest.mark.parametrize(
|
||||
"framing",
|
||||
[
|
||||
b"Transfer-Encoding: identity\r\n",
|
||||
b"Transfer-Encoding: IDENTITY \r\n",
|
||||
b"Transfer-Encoding: identity\r\nContent-Length: 0\r\n",
|
||||
],
|
||||
ids=["identity", "identity-cased-and-padded", "identity-plus-zero-length"],
|
||||
)
|
||||
def test_identity_framing_upload_closes_and_cannot_poison_the_socket(path, framing):
|
||||
"""All four multipart handlers, mirroring the chunked cases above.
|
||||
|
||||
`identity` carries no Content-Length either, so the length-0 read found no
|
||||
parts and each handler answered 400 "No file field in request" with the whole
|
||||
multipart payload still queued on an open socket.
|
||||
"""
|
||||
answered = _pipelined_after(
|
||||
f"POST {path} HTTP/1.1\r\n"
|
||||
"Host: 127.0.0.1\r\n"
|
||||
"Content-Type: multipart/form-data; boundary=x\r\n".encode()
|
||||
+ framing
|
||||
+ b"\r\n"
|
||||
+ _MULTIPART_PAYLOAD
|
||||
)
|
||||
|
||||
_assert_single_closed_response(answered, b"411", _MULTIPART_PAYLOAD)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"framing",
|
||||
[
|
||||
b"",
|
||||
b"Content-Length: 0\r\n",
|
||||
b"Content-Length: 00\r\n",
|
||||
b"Content-Length: 0 \r\n",
|
||||
b"Content-Length: 0\r\nContent-Length: 0\r\n",
|
||||
],
|
||||
ids=[
|
||||
"no-length",
|
||||
"zero-length",
|
||||
"double-zero-length",
|
||||
"ows-padded-zero",
|
||||
"two-agreeing-zeroes",
|
||||
],
|
||||
)
|
||||
def test_bodyless_framing_keeps_the_pooled_socket_alive(framing):
|
||||
"""The over-close half of the contract, on the wire.
|
||||
|
||||
Framing that positively says "no body" must NOT be swept up by the blank
|
||||
rule: the rejection answers without `Connection: close` and the pipelined GET
|
||||
is served on the same socket. Two agreeing zeroes are still no body.
|
||||
"""
|
||||
answered = _pipelined_after(
|
||||
b"GET /api/extensions/probe/sidecar/ping HTTP/1.1\r\n"
|
||||
b"Host: 127.0.0.1\r\n" + framing + b"\r\n",
|
||||
stop_after=2,
|
||||
)
|
||||
|
||||
text = answered.decode("latin-1", errors="replace")
|
||||
assert answered.startswith(b"HTTP/1.1 403"), text
|
||||
assert b"HTTP/1.1 200 OK" in answered, (
|
||||
f"the pipelined GET was not served, so a body-less rejection dropped a "
|
||||
f"healthy pooled connection: {text}"
|
||||
)
|
||||
assert b"Connection: close" not in answered, text
|
||||
|
||||
|
||||
# ── Every reject-before-read site, in BOTH directions, on the wire ─────────────
|
||||
#
|
||||
# The sidecar path above was the first site routed through the framing-aware
|
||||
# helper; the sibling sites kept the unconditional `arm_connection_close()` and
|
||||
# so were the mirror image of the same bug -- a body-less rejection answered with
|
||||
# `Connection: close` and dropped the client's pipelined follow-up, killing a
|
||||
# healthy keep-alive connection for no framing reason. Each pair below is the
|
||||
# reproduction (body-less -> must stay alive) beside its close half (a declared
|
||||
# body -> must still close), pipelined down one socket against the production
|
||||
# handler. Reproduced on the pre-fix head, all seven sites, `responses seen: 1`:
|
||||
#
|
||||
# POST /api/session/new (auth off-session) -> 401 + Connection: close
|
||||
# POST /api/session/new (bad Origin) -> 403 + Connection: close
|
||||
# POST /api/session/new (no CSRF token) -> 403 + Connection: close
|
||||
# POST /api/csp-report (limiter tripped) -> 204 + Connection: close
|
||||
# POST /api/health/restart (success) -> 200 + Connection: close
|
||||
# POST /api/process-complete-ack -> 410 + Connection: close
|
||||
# POST /api/upload (no boundary) -> 400 + Connection: close
|
||||
|
||||
_BODYLESS_FRAMING = [b"", b"Content-Length: 0\r\n"]
|
||||
_BODYLESS_IDS = ["no-content-length", "zero-content-length"]
|
||||
# A PUBLIC path, so the follow-up is a genuine 200 even with auth enabled.
|
||||
_PUBLIC_FOLLOWING_GET = (
|
||||
b"GET /api/auth/status HTTP/1.1\r\nHost: 127.0.0.1\r\nAccept: */*\r\n\r\n"
|
||||
)
|
||||
|
||||
|
||||
@contextlib.contextmanager
|
||||
def _own_server():
|
||||
"""The production Handler on a private port.
|
||||
|
||||
The shared test server runs with auth disabled and out of process, so the
|
||||
auth/CSRF-token rejections and the deterministic restart/limiter outcomes need
|
||||
an instance this process can configure. It is the same `server.Handler` class
|
||||
the real server binds, driven over a real socket.
|
||||
"""
|
||||
import server
|
||||
|
||||
httpd = ThreadingHTTPServer(("127.0.0.1", 0), server.Handler)
|
||||
thread = threading.Thread(target=httpd.serve_forever, daemon=True)
|
||||
thread.start()
|
||||
try:
|
||||
yield httpd.server_address[1]
|
||||
finally:
|
||||
httpd.shutdown()
|
||||
httpd.server_close()
|
||||
thread.join(timeout=10)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def auth_on(monkeypatch):
|
||||
"""Turn password auth on for this test (conftest resets the hash cache)."""
|
||||
import api.auth as auth
|
||||
|
||||
monkeypatch.setenv("HERMES_WEBUI_PASSWORD", "pooled-client-regression")
|
||||
auth._invalidate_password_hash_cache()
|
||||
assert auth.is_auth_enabled(), "the auth-path regression needs auth enabled"
|
||||
return auth
|
||||
|
||||
|
||||
def _assert_kept_alive(answered: bytes, status: bytes) -> None:
|
||||
text = answered.decode("latin-1", errors="replace")
|
||||
assert answered.startswith(b"HTTP/1.1 " + status), text
|
||||
assert b"Connection: close" not in answered, (
|
||||
f"a body-less rejection advertised close, so a pooled client's healthy "
|
||||
f"connection was dropped: {text}"
|
||||
)
|
||||
assert b"HTTP/1.1 200 OK" in answered, (
|
||||
f"the pipelined follow-up was not served on the same socket: {text}"
|
||||
)
|
||||
|
||||
|
||||
def _assert_closed(answered: bytes, status: bytes) -> None:
|
||||
text = answered.decode("latin-1", errors="replace")
|
||||
assert answered.startswith(b"HTTP/1.1 " + status), text
|
||||
assert b"Connection: close" in answered, text
|
||||
assert answered.count(b"HTTP/1.1 ") == 1, (
|
||||
f"the pipelined GET was answered, so the socket stayed open: {text}"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("framing", _BODYLESS_FRAMING, ids=_BODYLESS_IDS)
|
||||
def test_bodyless_auth_rejection_keeps_the_pooled_socket_alive(framing, auth_on):
|
||||
"""A body-less POST that fails auth must not cost the client its connection."""
|
||||
with _own_server() as port:
|
||||
answered = _pipelined_after(
|
||||
b"POST /api/session/new HTTP/1.1\r\nHost: 127.0.0.1\r\n" + framing + b"\r\n",
|
||||
stop_after=2,
|
||||
port=port,
|
||||
follow=_PUBLIC_FOLLOWING_GET,
|
||||
)
|
||||
|
||||
_assert_kept_alive(answered, b"401")
|
||||
|
||||
|
||||
def test_auth_rejection_with_a_body_still_closes_the_pooled_socket(auth_on):
|
||||
"""The close half at the auth gate: those bytes are still queued in rfile."""
|
||||
with _own_server() as port:
|
||||
answered = _pipelined_after(
|
||||
b"POST /api/session/new HTTP/1.1\r\nHost: 127.0.0.1\r\n"
|
||||
b"Content-Type: application/json\r\n"
|
||||
b"Content-Length: " + str(len(_BODY)).encode() + b"\r\n\r\n" + _BODY,
|
||||
port=port,
|
||||
follow=_PUBLIC_FOLLOWING_GET,
|
||||
)
|
||||
|
||||
_assert_closed(answered, b"401")
|
||||
assert _BODY not in answered, answered.decode("latin-1", errors="replace")
|
||||
|
||||
|
||||
@pytest.mark.parametrize("framing", _BODYLESS_FRAMING, ids=_BODYLESS_IDS)
|
||||
def test_bodyless_csrf_origin_rejection_keeps_the_pooled_socket_alive(framing):
|
||||
"""`_check_csrf()` origin mismatch, on the shared server (auth off)."""
|
||||
answered = _pipelined_after(
|
||||
b"POST /api/session/new HTTP/1.1\r\nHost: 127.0.0.1\r\n"
|
||||
b"Origin: http://evil.invalid\r\n" + framing + b"\r\n",
|
||||
stop_after=2,
|
||||
)
|
||||
|
||||
_assert_kept_alive(answered, b"403")
|
||||
|
||||
|
||||
def test_csrf_origin_rejection_with_a_body_still_closes_the_pooled_socket():
|
||||
answered = _pipelined_after(
|
||||
b"POST /api/session/new HTTP/1.1\r\nHost: 127.0.0.1\r\n"
|
||||
b"Origin: http://evil.invalid\r\nContent-Type: application/json\r\n"
|
||||
b"Content-Length: " + str(len(_BODY)).encode() + b"\r\n\r\n" + _BODY
|
||||
)
|
||||
|
||||
_assert_closed(answered, b"403")
|
||||
assert _BODY not in answered, answered.decode("latin-1", errors="replace")
|
||||
|
||||
|
||||
def _authenticated_same_origin_post(port, cookie_name, cookie, framing, body=b""):
|
||||
return (
|
||||
f"POST /api/session/new HTTP/1.1\r\nHost: 127.0.0.1:{port}\r\n"
|
||||
f"Origin: http://127.0.0.1:{port}\r\n"
|
||||
f"Cookie: {cookie_name}={cookie}\r\n".encode() + framing + b"\r\n" + body
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("framing", _BODYLESS_FRAMING, ids=_BODYLESS_IDS)
|
||||
def test_bodyless_csrf_token_rejection_keeps_the_pooled_socket_alive(framing, auth_on):
|
||||
"""A real session, a same-origin POST, and no CSRF token -> token_mismatch."""
|
||||
cookie = auth_on.create_session()
|
||||
try:
|
||||
with _own_server() as port:
|
||||
answered = _pipelined_after(
|
||||
_authenticated_same_origin_post(
|
||||
port, auth_on._resolve_cookie_name(), cookie, framing
|
||||
),
|
||||
stop_after=2,
|
||||
port=port,
|
||||
follow=_PUBLIC_FOLLOWING_GET,
|
||||
)
|
||||
finally:
|
||||
auth_on.invalidate_session(cookie)
|
||||
|
||||
_assert_kept_alive(answered, b"403")
|
||||
|
||||
|
||||
def test_csrf_token_rejection_with_a_body_still_closes_the_pooled_socket(auth_on):
|
||||
cookie = auth_on.create_session()
|
||||
try:
|
||||
with _own_server() as port:
|
||||
answered = _pipelined_after(
|
||||
_authenticated_same_origin_post(
|
||||
port,
|
||||
auth_on._resolve_cookie_name(),
|
||||
cookie,
|
||||
b"Content-Type: application/json\r\nContent-Length: "
|
||||
+ str(len(_BODY)).encode()
|
||||
+ b"\r\n",
|
||||
body=_BODY,
|
||||
),
|
||||
port=port,
|
||||
follow=_PUBLIC_FOLLOWING_GET,
|
||||
)
|
||||
finally:
|
||||
auth_on.invalidate_session(cookie)
|
||||
|
||||
_assert_closed(answered, b"403")
|
||||
assert _BODY not in answered, answered.decode("latin-1", errors="replace")
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def csp_limiter_tripped(monkeypatch):
|
||||
"""Force the CSP-report limiter, instead of sending 101 reports in 60s."""
|
||||
import api.routes as routes
|
||||
|
||||
monkeypatch.setattr(routes, "_csp_report_rate_limited", lambda _handler: True)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("framing", _BODYLESS_FRAMING, ids=_BODYLESS_IDS)
|
||||
def test_bodyless_rate_limited_csp_report_keeps_the_pooled_socket_alive(
|
||||
framing, csp_limiter_tripped
|
||||
):
|
||||
"""A dropped 204 must not hang up on a browser that is still reporting."""
|
||||
with _own_server() as port:
|
||||
answered = _pipelined_after(
|
||||
b"POST /api/csp-report HTTP/1.1\r\nHost: 127.0.0.1\r\n" + framing + b"\r\n",
|
||||
stop_after=2,
|
||||
port=port,
|
||||
follow=_PUBLIC_FOLLOWING_GET,
|
||||
)
|
||||
|
||||
_assert_kept_alive(answered, b"204")
|
||||
|
||||
|
||||
def test_rate_limited_csp_report_with_a_body_still_closes(csp_limiter_tripped):
|
||||
"""The limiter answers without reading the report, so a declared body closes."""
|
||||
with _own_server() as port:
|
||||
answered = _pipelined_after(
|
||||
b"POST /api/csp-report HTTP/1.1\r\nHost: 127.0.0.1\r\n"
|
||||
b"Content-Type: application/csp-report\r\n"
|
||||
b"Content-Length: " + str(len(_BODY)).encode() + b"\r\n\r\n" + _BODY,
|
||||
port=port,
|
||||
follow=_PUBLIC_FOLLOWING_GET,
|
||||
)
|
||||
|
||||
_assert_closed(answered, b"204")
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def restart_succeeds(monkeypatch):
|
||||
"""Pin the SUCCESS outcome -- the one that closed a healthy socket every call."""
|
||||
import api.routes as routes
|
||||
|
||||
monkeypatch.setattr(
|
||||
routes, "restart_active_profile_gateway", lambda: {"status": "completed"}
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("framing", _BODYLESS_FRAMING, ids=_BODYLESS_IDS)
|
||||
def test_bodyless_successful_restart_keeps_the_pooled_socket_alive(framing, restart_succeeds):
|
||||
"""`/api/health/restart` closed on success too; the WebUI sends no body."""
|
||||
with _own_server() as port:
|
||||
answered = _pipelined_after(
|
||||
b"POST /api/health/restart HTTP/1.1\r\nHost: 127.0.0.1\r\n" + framing + b"\r\n",
|
||||
stop_after=2,
|
||||
port=port,
|
||||
follow=_PUBLIC_FOLLOWING_GET,
|
||||
)
|
||||
|
||||
_assert_kept_alive(answered, b"200")
|
||||
|
||||
|
||||
def test_restart_with_a_body_still_closes_the_pooled_socket(restart_succeeds):
|
||||
"""The endpoint consumes its body on no outcome, so a declared one closes."""
|
||||
with _own_server() as port:
|
||||
answered = _pipelined_after(
|
||||
b"POST /api/health/restart HTTP/1.1\r\nHost: 127.0.0.1\r\n"
|
||||
b"Content-Type: application/json\r\n"
|
||||
b"Content-Length: " + str(len(_BODY)).encode() + b"\r\n\r\n" + _BODY,
|
||||
port=port,
|
||||
follow=_PUBLIC_FOLLOWING_GET,
|
||||
)
|
||||
|
||||
_assert_closed(answered, b"200")
|
||||
assert _BODY not in answered, answered.decode("latin-1", errors="replace")
|
||||
|
||||
|
||||
@pytest.mark.parametrize("framing", _BODYLESS_FRAMING, ids=_BODYLESS_IDS)
|
||||
def test_bodyless_deprecated_ack_keeps_the_pooled_socket_alive(framing):
|
||||
"""The 410 alias, which the body-bearing test at the top of this file pins closed."""
|
||||
answered = _pipelined_after(
|
||||
b"POST /api/process-complete-ack HTTP/1.1\r\nHost: 127.0.0.1\r\n" + framing + b"\r\n",
|
||||
stop_after=2,
|
||||
)
|
||||
|
||||
_assert_kept_alive(answered, b"410")
|
||||
|
||||
|
||||
@pytest.mark.parametrize("path", _MULTIPART_UPLOAD_PATHS)
|
||||
@pytest.mark.parametrize(
|
||||
"framing",
|
||||
[
|
||||
b"Content-Type: application/json\r\n",
|
||||
b"Content-Type: application/json\r\nContent-Length: 0\r\n",
|
||||
b"Content-Type: multipart/form-data\r\n", # multipart, but no boundary=
|
||||
],
|
||||
ids=["json-no-length", "json-zero-length", "multipart-without-boundary"],
|
||||
)
|
||||
def test_bodyless_upload_boundary_rejection_keeps_the_pooled_socket_alive(path, framing):
|
||||
"""`_reject_before_read()`'s one body-less reachable rejection: the boundary check."""
|
||||
answered = _pipelined_after(
|
||||
f"POST {path} HTTP/1.1\r\nHost: 127.0.0.1\r\n".encode() + framing + b"\r\n",
|
||||
stop_after=2,
|
||||
)
|
||||
|
||||
_assert_kept_alive(answered, b"400")
|
||||
|
||||
|
||||
@pytest.mark.parametrize("path", _MULTIPART_UPLOAD_PATHS)
|
||||
def test_upload_boundary_rejection_with_a_body_still_closes(path):
|
||||
answered = _pipelined_after(
|
||||
f"POST {path} HTTP/1.1\r\nHost: 127.0.0.1\r\n"
|
||||
"Content-Type: application/json\r\n"
|
||||
f"Content-Length: {len(_BODY)}\r\n\r\n".encode() + _BODY
|
||||
)
|
||||
|
||||
_assert_closed(answered, b"400")
|
||||
assert _BODY not in answered, answered.decode("latin-1", errors="replace")
|
||||
Executable
+110
@@ -0,0 +1,110 @@
|
||||
"""A rejected write must not leave its unread body on a reused connection.
|
||||
|
||||
The unit tests beside this one assert `close_connection` is set on each rejection
|
||||
path. That pins the mechanism but not the consequence: they build a
|
||||
SimpleNamespace, never open a socket, and would still pass if
|
||||
BaseHTTPRequestHandler stopped honouring the flag.
|
||||
|
||||
This drives the actual failure over one socket: a POST that is rejected before its
|
||||
body is read, then an ordinary GET on the same connection. Without the fix the
|
||||
server parses the leftover body as the next request line and answers
|
||||
|
||||
HTTP/1.1 400 Bad request syntax ('{"a": "b"}GET /api/health HTTP/1.1')
|
||||
|
||||
which is an error about a request the client never sent - the reason this class of
|
||||
bug is expensive to diagnose from the client side. With the fix the server closes
|
||||
after the rejection, so the client sees a clean EOF and reconnects instead.
|
||||
|
||||
The two files are complementary and neither replaces the other. The unit tests
|
||||
call the production handlers, so they pin that `close_connection` is actually set
|
||||
on each rejection path. This file uses a minimal handler of the same shape, so it
|
||||
pins WHY that flag matters at the HTTP level - which no amount of asserting on a
|
||||
boolean can show. Reverting server.py fails the unit test; changing the handler
|
||||
here to reuse the connection fails this one.
|
||||
"""
|
||||
|
||||
import socket
|
||||
import threading
|
||||
import time
|
||||
from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
|
||||
|
||||
BODY = b'{"a": "b"}'
|
||||
REJECTED_WRITE = (
|
||||
b"POST /api/session/new HTTP/1.1\r\n"
|
||||
b"Host: localhost\r\n"
|
||||
b"Content-Type: application/json\r\n"
|
||||
b"Content-Length: " + str(len(BODY)).encode() + b"\r\n"
|
||||
b"\r\n" + BODY
|
||||
)
|
||||
FOLLOWING_READ = b"GET /api/health HTTP/1.1\r\nHost: localhost\r\n\r\n"
|
||||
|
||||
|
||||
class _Rejecting(BaseHTTPRequestHandler):
|
||||
"""The production shape: reject an unsafe write before reading its body."""
|
||||
|
||||
protocol_version = "HTTP/1.1"
|
||||
closes_on_reject = True
|
||||
|
||||
def log_message(self, *args):
|
||||
pass # keep the test output quiet
|
||||
|
||||
def do_POST(self):
|
||||
# Deliberately does NOT read the body. This is the production path when
|
||||
# auth or CSRF rejects a write before read_body() runs.
|
||||
if self.closes_on_reject:
|
||||
self.close_connection = True
|
||||
self.send_response(403)
|
||||
self.send_header("Content-Length", "0")
|
||||
self.end_headers()
|
||||
|
||||
def do_GET(self):
|
||||
self.send_response(200)
|
||||
self.send_header("Content-Length", "2")
|
||||
self.end_headers()
|
||||
self.wfile.write(b"ok")
|
||||
|
||||
|
||||
def _second_response(*, closes_on_reject: bool) -> bytes:
|
||||
"""Send a rejected write then a GET down one socket; return the GET's answer."""
|
||||
handler = type("_H", (_Rejecting,), {"closes_on_reject": closes_on_reject})
|
||||
server = ThreadingHTTPServer(("127.0.0.1", 0), handler)
|
||||
threading.Thread(target=server.serve_forever, daemon=True).start()
|
||||
try:
|
||||
sock = socket.create_connection(server.server_address, timeout=5)
|
||||
try:
|
||||
sock.sendall(REJECTED_WRITE)
|
||||
time.sleep(0.3)
|
||||
assert b"403" in sock.recv(4096), "the write should have been rejected"
|
||||
|
||||
sock.sendall(FOLLOWING_READ)
|
||||
time.sleep(0.3)
|
||||
try:
|
||||
return sock.recv(4096)
|
||||
except (socket.timeout, ConnectionError):
|
||||
return b""
|
||||
finally:
|
||||
sock.close()
|
||||
finally:
|
||||
server.shutdown()
|
||||
server.server_close()
|
||||
|
||||
|
||||
def test_rejected_write_does_not_corrupt_the_next_request():
|
||||
"""Closing means the client sees EOF, never a bogus error about its own GET."""
|
||||
answer = _second_response(closes_on_reject=True)
|
||||
assert b"400" not in answer, f"the leftover body reached the parser: {answer!r}"
|
||||
assert BODY not in answer, f"the leftover body reached the parser: {answer!r}"
|
||||
# The connection is gone, which is the point: the client reconnects rather
|
||||
# than being told its next request was malformed.
|
||||
assert answer == b"", f"expected a closed connection, got {answer!r}"
|
||||
|
||||
|
||||
def test_without_the_fix_the_unread_body_corrupts_the_next_request():
|
||||
"""The bug itself, so the test above is known to be testing something.
|
||||
|
||||
Reusing the connection makes the server read the unread body as a request
|
||||
line, and the body text appears in the 400 it sends back.
|
||||
"""
|
||||
answer = _second_response(closes_on_reject=False)
|
||||
assert b"400" in answer, f"expected a parse failure, got {answer!r}"
|
||||
assert BODY in answer, f"expected the leftover body in the error, got {answer!r}"
|
||||
@@ -2,11 +2,14 @@ import json
|
||||
from pathlib import Path
|
||||
|
||||
from api.run_journal import (
|
||||
REPLAY_SKIPPED_SSE_EVENTS,
|
||||
RunJournalWriter,
|
||||
append_run_event,
|
||||
find_run_summary,
|
||||
journal_replay_visible,
|
||||
latest_run_summary,
|
||||
read_run_events,
|
||||
read_session_run_events,
|
||||
stale_interrupted_event,
|
||||
)
|
||||
|
||||
@@ -28,6 +31,58 @@ def test_run_journal_appends_monotonic_seq_and_reads_after_cursor(tmp_path):
|
||||
assert [event["event"] for event in journal["events"]] == ["done"]
|
||||
|
||||
|
||||
def test_writer_skips_metering_and_keeps_remaining_seqs_contiguous(tmp_path):
|
||||
writer = RunJournalWriter("session_1", "run_1", session_dir=tmp_path)
|
||||
|
||||
first = writer.append_sse_event("token", {"text": "hello"})
|
||||
skipped = writer.append_sse_event("metering", {"tps": 47.3})
|
||||
second = writer.append_sse_event("done", {"session": {}})
|
||||
|
||||
assert skipped is None
|
||||
assert first["seq"] == 1
|
||||
# Skipped metering must NOT consume a seq, so visible rows stay contiguous —
|
||||
# the offline-gap coverage and replay-cursor contiguity checks depend on it.
|
||||
assert second["seq"] == 2
|
||||
|
||||
journal = read_run_events("session_1", "run_1", session_dir=tmp_path)
|
||||
assert [event["event"] for event in journal["events"]] == ["token", "done"]
|
||||
|
||||
|
||||
def test_writer_skip_is_payload_independent(tmp_path):
|
||||
writer = RunJournalWriter("session_1", "run_1", session_dir=tmp_path)
|
||||
|
||||
assert writer.append_sse_event("metering", None) is None
|
||||
assert writer.append_sse_event("metering", {"tps": 1.0, "active": 1}) is None
|
||||
|
||||
journal = read_run_events("session_1", "run_1", session_dir=tmp_path)
|
||||
assert journal["events"] == []
|
||||
|
||||
|
||||
def test_journal_replay_visible_predicate():
|
||||
assert REPLAY_SKIPPED_SSE_EVENTS == {"metering"}
|
||||
assert journal_replay_visible({"event": "token", "payload": {"text": "x"}}) is True
|
||||
assert journal_replay_visible({"event": "metering", "payload": {"tps": 1.0}}) is False
|
||||
# `type` fallback: metering rows are invisible under either field.
|
||||
assert journal_replay_visible({"type": "metering"}) is False
|
||||
# Unknown shapes must pass through (visible) so they can never swallow output.
|
||||
assert journal_replay_visible("not-a-dict") is True
|
||||
|
||||
|
||||
def test_readers_keep_metering_rows(tmp_path):
|
||||
# Writers skip metering, but readers deliberately do NOT filter: cursor math
|
||||
# (the ``cursor_event_missing`` bound) and the offline-gap coverage check
|
||||
# count journal seqs and must keep seeing every row.
|
||||
append_run_event("session_1", "run_1", "token", {"text": "hello"}, session_dir=tmp_path)
|
||||
append_run_event("session_1", "run_1", "metering", {"tps": 47.3}, session_dir=tmp_path)
|
||||
|
||||
journal = read_run_events("session_1", "run_1", session_dir=tmp_path)
|
||||
assert [event["event"] for event in journal["events"]] == ["token", "metering"]
|
||||
|
||||
replay = read_session_run_events("session_1", after_event_id="run_1:1", session_dir=tmp_path)
|
||||
assert replay["status"] == "ok"
|
||||
assert [event["event"] for event in replay["events"]] == ["metering"]
|
||||
|
||||
|
||||
def test_run_journal_reads_bounded_replay_window(tmp_path):
|
||||
writer = RunJournalWriter("session_1", "run_1", session_dir=tmp_path)
|
||||
|
||||
|
||||
@@ -761,6 +761,97 @@ def test_replay_run_journal_honors_after_seq_cursor(monkeypatch):
|
||||
assert "event: done\n" in body
|
||||
|
||||
|
||||
def test_replay_run_journal_skips_metering_rows(monkeypatch):
|
||||
import api.routes as routes
|
||||
|
||||
handler = SimpleNamespace(wfile=io.BytesIO())
|
||||
monkeypatch.setattr(
|
||||
routes,
|
||||
"find_run_summary",
|
||||
lambda stream_id: {
|
||||
"session_id": "session_1",
|
||||
"run_id": stream_id,
|
||||
"terminal": True,
|
||||
},
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
routes,
|
||||
"read_run_events",
|
||||
lambda session_id, run_id, after_seq=None, max_seq=None: {
|
||||
"events": [
|
||||
{"event": "token", "payload": {"text": "hello"}, "event_id": f"{run_id}:1"},
|
||||
{"event": "metering", "payload": {"tps": 47.3}, "event_id": f"{run_id}:2"},
|
||||
{"event": "done", "payload": {"session": {}}, "event_id": f"{run_id}:3"},
|
||||
]
|
||||
},
|
||||
)
|
||||
|
||||
assert routes._replay_run_journal(handler, "run_1", 0) is True
|
||||
body = handler.wfile.getvalue().decode("utf-8")
|
||||
assert "id: run_1:1\n" in body
|
||||
assert "event: token\n" in body
|
||||
assert "id: run_1:3\n" in body
|
||||
assert "event: done\n" in body
|
||||
# The metering row is filtered from the replay burst.
|
||||
assert "event: metering\n" not in body
|
||||
assert "id: run_1:2\n" not in body
|
||||
|
||||
|
||||
def test_live_unjournaled_metering_does_not_inherit_latest_event_id(monkeypatch):
|
||||
import api.routes as routes
|
||||
|
||||
class FakeStream:
|
||||
def __init__(self):
|
||||
self.q = queue.Queue()
|
||||
self.q.put_nowait(("metering", {"tps": 47.3}, None))
|
||||
self.q.put_nowait(("token", {"text": "visible"}, "run_1:1"))
|
||||
self.q.put_nowait(("stream_end", {}, "run_1:2"))
|
||||
self.unsubscribed = False
|
||||
|
||||
def subscribe_with_snapshot(self):
|
||||
return self.q, {"last_event_id": "run_1:1", "offline_buffered_events": 3}
|
||||
|
||||
def unsubscribe(self, q):
|
||||
self.unsubscribed = q is self.q
|
||||
|
||||
class Handler:
|
||||
def __init__(self):
|
||||
self.wfile = io.BytesIO()
|
||||
self.headers = {}
|
||||
|
||||
def send_response(self, _code):
|
||||
pass
|
||||
|
||||
def send_header(self, _name, _value):
|
||||
pass
|
||||
|
||||
def end_headers(self):
|
||||
pass
|
||||
|
||||
handler = Handler()
|
||||
stream = FakeStream()
|
||||
monkeypatch.setattr(
|
||||
routes,
|
||||
"find_run_summary",
|
||||
lambda stream_id: {
|
||||
"session_id": "session_1",
|
||||
"run_id": stream_id,
|
||||
"terminal": False,
|
||||
},
|
||||
)
|
||||
monkeypatch.setattr(routes, "read_run_events", lambda *_args, **_kwargs: {"events": []})
|
||||
monkeypatch.setitem(routes.STREAM_LAST_EVENT_ID, "run_1", "run_1:1")
|
||||
monkeypatch.setitem(routes.STREAMS, "run_1", stream)
|
||||
|
||||
routes._handle_sse_stream(handler, urlparse("/api/chat/stream?stream_id=run_1"))
|
||||
|
||||
body = handler.wfile.getvalue().decode("utf-8")
|
||||
frames = [frame for frame in body.split("\n\n") if frame.strip()]
|
||||
assert frames[0] == "event: metering\ndata: {\"tps\": 47.3}"
|
||||
assert "id: run_1:1\nevent: token\ndata: {\"text\": \"visible\"}" in frames
|
||||
assert stream.unsubscribed is True
|
||||
|
||||
|
||||
def test_active_stream_replay_keeps_items_for_new_run_with_same_seq_range(monkeypatch):
|
||||
import api.routes as routes
|
||||
|
||||
|
||||
@@ -26,7 +26,7 @@ def test_streaming_journals_sse_events_before_queue_delivery():
|
||||
|
||||
assert put_idx < journal_idx < queue_idx
|
||||
assert "Failed to append run journal event" in block
|
||||
assert "queue_item = (event, data, event_id) if event_id and hasattr(q, \"subscribe_with_snapshot\") else (event, data)" in block
|
||||
assert 'queue_item = (event, data, event_id) if hasattr(q, "subscribe_with_snapshot") else (event, data)' in block
|
||||
|
||||
|
||||
|
||||
|
||||
@@ -108,7 +108,17 @@ def test_session_duplicate_foreign_profile_session_blocked_by_visibility_guard(m
|
||||
cap = _capture(monkeypatch)
|
||||
routes.handle_post(handler, urlparse("/api/session/duplicate"))
|
||||
|
||||
assert cap["bad"] == ("Session not found", 404)
|
||||
# #7710: the generic request-guard mirrors the detail-load endpoint's
|
||||
# contract — a session owned by a KNOWN other profile yields
|
||||
# 409 ``session_profile_mismatch`` so the client can offer to
|
||||
# switch to it (#5419). The 404 self-heal path is preserved for
|
||||
# the None-profile (unknown/legacy) case.
|
||||
assert cap["ok"] == {
|
||||
"error": "Session belongs to a different profile",
|
||||
"code": "session_profile_mismatch",
|
||||
"session_id": "foreign_duplicate",
|
||||
"profile": "other",
|
||||
}
|
||||
|
||||
|
||||
def test_session_duplicate_same_profile_still_duplicates(monkeypatch):
|
||||
@@ -151,7 +161,13 @@ def test_file_read_foreign_profile_session_returns_404_before_file_ops(monkeypat
|
||||
|
||||
routes.handle_get(handler, urlparse("/api/file?session_id=foreign_file&path=notes.txt"))
|
||||
|
||||
assert cap["bad"] == ("Session not found", 404)
|
||||
# #7710: see line 111 above — same generic-guard contract.
|
||||
assert cap["ok"] == {
|
||||
"error": "Session belongs to a different profile",
|
||||
"code": "session_profile_mismatch",
|
||||
"session_id": "foreign_file",
|
||||
"profile": "other",
|
||||
}
|
||||
|
||||
|
||||
def test_chat_start_foreign_persisted_session_returns_404_before_start_run(monkeypatch):
|
||||
@@ -171,7 +187,13 @@ def test_chat_start_foreign_persisted_session_returns_404_before_start_run(monke
|
||||
cap = _capture(monkeypatch)
|
||||
routes.handle_post(handler, urlparse("/api/chat/start"))
|
||||
|
||||
assert cap["bad"] == ("Session not found", 404)
|
||||
# #7710: see line 111 above — same generic-guard contract.
|
||||
assert cap["ok"] == {
|
||||
"error": "Session belongs to a different profile",
|
||||
"code": "session_profile_mismatch",
|
||||
"session_id": "chat_foreign",
|
||||
"profile": "other",
|
||||
}
|
||||
|
||||
|
||||
def test_chat_start_body_profile_cannot_retag_visible_empty_session_without_active_profile(monkeypatch):
|
||||
@@ -287,7 +309,13 @@ def test_chat_stream_status_blocks_foreign_active_stream(monkeypatch):
|
||||
config.ACTIVE_RUNS.clear()
|
||||
config.ACTIVE_RUNS.update(previous)
|
||||
|
||||
assert cap["bad"] == ("Session not found", 404)
|
||||
# #7710: see line 111 above — same generic-guard contract.
|
||||
assert cap["ok"] == {
|
||||
"error": "Session belongs to a different profile",
|
||||
"code": "session_profile_mismatch",
|
||||
"session_id": "foreign_session",
|
||||
"profile": "other",
|
||||
}
|
||||
|
||||
|
||||
def test_chat_stream_status_blocks_foreign_registered_stream_before_worker_start(monkeypatch):
|
||||
@@ -328,7 +356,15 @@ def test_chat_stream_status_blocks_foreign_registered_stream_before_worker_start
|
||||
config.STREAM_SESSION_OWNERS.clear()
|
||||
config.STREAM_SESSION_OWNERS.update(previous_owners)
|
||||
|
||||
assert cap["bad"] == ("Session not found", 404)
|
||||
# #7710: see line 111 above — same generic-guard contract.
|
||||
# The helper resolves the stream to its OWNER session id (not the
|
||||
# stream id), so the mismatch payload carries the owner session id.
|
||||
assert cap["ok"] == {
|
||||
"error": "Session belongs to a different profile",
|
||||
"code": "session_profile_mismatch",
|
||||
"session_id": "foreign_session",
|
||||
"profile": "other",
|
||||
}
|
||||
|
||||
|
||||
def test_chat_stream_status_keeps_same_profile_stream_visible(monkeypatch):
|
||||
@@ -392,7 +428,17 @@ def test_chat_cancel_blocks_foreign_owned_stream_before_cancel_call(monkeypatch)
|
||||
config.ACTIVE_RUNS.update(previous)
|
||||
|
||||
assert calls["cancel"] == 0
|
||||
assert cap["bad"] == ("Session not found", 404)
|
||||
# #7710: see line 111 above — same generic-guard contract.
|
||||
# The helper resolves the stream to its OWNER session id (not the
|
||||
# stream id), so the mismatch payload carries the owner session id.
|
||||
# This test's stream is owned by ``foreign_session`` (see the
|
||||
# ACTIVE_RUNS fixture above).
|
||||
assert cap["ok"] == {
|
||||
"error": "Session belongs to a different profile",
|
||||
"code": "session_profile_mismatch",
|
||||
"session_id": "foreign_session",
|
||||
"profile": "other",
|
||||
}
|
||||
|
||||
|
||||
def test_chat_cancel_same_profile_stream_still_passes_through(monkeypatch):
|
||||
@@ -443,7 +489,17 @@ def test_chat_stream_blocks_foreign_owned_dead_stream_before_replay(monkeypatch)
|
||||
cap = _capture(monkeypatch)
|
||||
routes.handle_get(handler, urlparse("/api/chat/stream?stream_id=stream-dead-foreign"))
|
||||
|
||||
assert cap["bad"] == ("Session not found", 404)
|
||||
# #7710: see line 111 above — same generic-guard contract.
|
||||
# The helper resolves the stream to its OWNER session id (not the
|
||||
# stream id), so the mismatch payload carries the owner session id.
|
||||
# This test's stream is owned by ``foreign_session`` (see the
|
||||
# ``find_run_summary`` mock above).
|
||||
assert cap["ok"] == {
|
||||
"error": "Session belongs to a different profile",
|
||||
"code": "session_profile_mismatch",
|
||||
"session_id": "foreign_session",
|
||||
"profile": "other",
|
||||
}
|
||||
|
||||
|
||||
def test_chat_stream_allows_unknown_dead_stream_fallback_replay_path(monkeypatch):
|
||||
|
||||
@@ -57,7 +57,7 @@ def test_stream_channel_queue_item_carries_per_event_id_with_legacy_fallback():
|
||||
"""StreamChannel queue items need per-frame ids; legacy queues stay 2-tuples."""
|
||||
put_def_idx = STREAMING_PY.find("def put(event, data):")
|
||||
put_body = STREAMING_PY[put_def_idx:put_def_idx + 2500]
|
||||
assert 'queue_item = (event, data, event_id) if event_id and hasattr(q, "subscribe_with_snapshot") else (event, data)' in put_body, (
|
||||
assert 'queue_item = (event, data, event_id) if hasattr(q, "subscribe_with_snapshot") else (event, data)' in put_body, (
|
||||
"StreamChannel events must carry their own event_id while legacy queue "
|
||||
"consumers retain the 2-tuple shape"
|
||||
)
|
||||
@@ -69,7 +69,7 @@ def test_gateway_queue_item_carries_per_event_id_with_legacy_fallback():
|
||||
put_def_idx = GATEWAY_CHAT_PY.find("def put_gateway_event(event, data):")
|
||||
assert put_def_idx != -1, "put_gateway_event(event, data) not found"
|
||||
put_body = GATEWAY_CHAT_PY[put_def_idx:put_def_idx + 1800]
|
||||
assert 'queue_item = (event, data, event_id) if event_id and hasattr(q, "subscribe_with_snapshot") else (event, data)' in put_body, (
|
||||
assert 'queue_item = (event, data, event_id) if hasattr(q, "subscribe_with_snapshot") else (event, data)' in put_body, (
|
||||
"Gateway live events must carry their own event_id for StreamChannel "
|
||||
"subscribers while preserving legacy queue compatibility"
|
||||
)
|
||||
|
||||
@@ -33,6 +33,34 @@ def test_extract_tool_calls_from_openai_message_linkage():
|
||||
assert result[0]["snippet"] == "file.txt"
|
||||
|
||||
|
||||
def test_extract_tool_calls_ignores_explicit_null_tool_calls_in_history():
|
||||
"""Legacy assistant rows with tool_calls=None must not abort later turns."""
|
||||
messages = [
|
||||
{"role": "assistant", "content": "An old TickTick response", "tool_calls": None},
|
||||
{"role": "user", "content": "continue"},
|
||||
{
|
||||
"role": "assistant",
|
||||
"content": "",
|
||||
"tool_calls": [{
|
||||
"id": "call-current",
|
||||
"function": {"name": "terminal", "arguments": '{"command":"pwd"}'},
|
||||
}],
|
||||
},
|
||||
{
|
||||
"role": "tool",
|
||||
"tool_call_id": "call-current",
|
||||
"content": '{"output":"/workspace","exit_code":0}',
|
||||
},
|
||||
]
|
||||
|
||||
result = _extract_tool_calls_from_messages(messages)
|
||||
|
||||
assert len(result) == 1
|
||||
assert result[0]["name"] == "terminal"
|
||||
assert result[0]["assistant_msg_idx"] == 2
|
||||
assert result[0]["snippet"] == "/workspace"
|
||||
|
||||
|
||||
def test_tool_result_snippet_allows_frontend_show_more_threshold_but_stays_bounded():
|
||||
"""Persisted snippets should be long enough for frontend Show more but capped."""
|
||||
medium_output = "m" * 1200
|
||||
|
||||
@@ -88,7 +88,7 @@ def test_vietnamese_locale_includes_representative_translations():
|
||||
expected = [
|
||||
"settings_heading_title: 'Trung tâm điều khiển'",
|
||||
"settings_heading_subtitle: 'Tùy chọn, công cụ hội thoại và điều khiển hệ thống.'",
|
||||
"approval_skip_all: '⚡ Bỏ qua tất cả trong phiên này'",
|
||||
"approval_skip_all: 'Bỏ qua tất cả trong phiên này'",
|
||||
"checkpoint_title: 'Checkpoint'",
|
||||
"composer_send: 'Gửi tin nhắn'",
|
||||
"gateway_restart: 'Khởi động lại'",
|
||||
|
||||
Reference in new issue
Block a user