A title request against a slow local model (a reasoning model on modest
hardware) hit `auxiliary.title_generation.timeout`, was retried on the same
provider twice more with backoff, and only then fell back — roughly four
configured windows, exactly the 3x30s the reporter read off the Ollama access
log — while the auto-title thread outlived the deadline the user thought they
had set. The failure was logged at INFO as a "connection error", the same
label as an unreachable endpoint, so the only WARNING the operator saw came
from the fallback provider complaining about a model it never had, which reads
as "your base_url was ignored".
Title generation joins compression and vision in the existing
critical-path rung that skips the same-provider retry after a full-budget
timeout, and the fallback ladder now classifies a timeout before the
connection-error rung and logs it at WARNING with the endpoint, the budget
and the config key to raise. The route carries its effective timeout so the
message can say how long it waited. Docs list the title default and the
one-window rule.
Slim redo of the retry/fallback half of #100013 on the existing rung; its
10s default and concurrency change are not carried.
Fixes#89445
Part of #66251
Co-authored-by: AJ Fasano <ajfasano@gmail.com>
The api.x.ai host row mapped every auto-routed api.x.ai client to
xai-oauth, including an aux client the auto route had tagged with the
API-key `xai` backend (XAI_API_KEY). A 401/403 on that key then ran the
xai-oauth refresher: with a stale grant on disk the refresh rotation was
spent and the retry silently switched the call onto xai-oauth.
`_auth_refresh_provider_for_route` now takes the client's effective
provider (already recorded on auto clients) and skips the host row when
that provider is the row's API-key sibling; the ladder passes it through.
The auto/oauth path that #84845 fixed is unchanged.
An auxiliary call_llm() with no provider override (plugin hooks, title/compression on a
gateway whose main provider is xai-oauth) reaches the recovery ladder as resolved_provider
"auto". _auth_refresh_provider_for_route infers the concrete backend from the client host,
but _AUTH_REFRESH_PROVIDER_BY_HOST had no api.x.ai row, so the refresh rung was skipped
and the pool rung marked the single device_code grant exhausted and walked to fallbacks
that were never configured. The main agent loop refreshed the same token a second later.
Adding the host row lets the existing rung refresh, evict the "auto"-labelled cached client
and retry once on xai-oauth before any fallback. Live against a stand-in replaying xAI's
documented 403 body: before 403 -> pool exhausted -> re-raised; after 403 -> refresh POST
-> retry with the new bearer -> 200.
Fixes#84845
Co-authored-by: andyylin <51783311+andyylin@users.noreply.github.com>
The comment said the lock is an RLock because save_config internally
calls read_raw_config. After the previous fix that is no longer true:
save_config takes its raw mapping from require_readable_config_before_write
and never re-enters the lock through read_raw_config.
The reentrancy that still exists is external: hermes_cli/plugins.py holds
_CONFIG_LOCK across its read-modify-write and then calls save_config(),
which acquires the lock again. Name that, and leave the RLock as is.
The transient case that was RED on main: an intact config.yaml holding
real schema sections pinned to their scalar defaults, with the cached
read_raw_config() patched to return {}, must still have every one of those
sections after save_config(load_config()). Pins that the explicit-path
evidence comes from the fail-closed read, not the cached one (#113301).
Sections are chosen only where the first default value is a scalar, and
the floor on how many exist is modest (>= 10) so an unrelated
DEFAULT_CONFIG reorder or reshaping cannot fail this test.
A key the user set explicitly to its schema default (skills.write_approval:
true) must survive save_config(load_config()) on an intact file: the raw
read supplies the preserve set for the strip pass, so this is #113301's
32 -> 32 row. Test from #113323, kept verbatim.
save_config() called require_readable_config_before_write() -- which parses
config.yaml fresh, uncached, and returns the mapping -- then threw that
mapping away and re-read the file through the cached read_raw_config().
That second read can return {} while the file is intact:
_read_raw_config_impl returns {} from its two swallowed-error paths (the
`except (FileNotFoundError, OSError)` around stat and the `except Exception`
around open/parse), so a transient stat/open error on an intact file yields
{} for this call. A {} here leaves _strip_default_values with no
explicit-path evidence, so every user section whose value matches a schema
default is dropped: the 32 -> 7 section collapse in #113301.
Use the mapping the fail-closed read already returned as _raw_for_paths.
One read, one authority, and a truly empty or missing file still behaves
as a first install (only explicit keys written, no raise).
Reported via #113321 (@kvnloo, first submitter) and #113323 (@nolanchic);
both proposed a different mechanism (skip strip-defaults / raise) -- see
PR. Skipping the strip pass writes every schema default into config.yaml;
raising when the cached read is empty would break edit_config's save over
an empty file and a fresh install. The per-save sys._getframe caller log
from #113323 is out of scope (#96571) and not taken here.
Fixes#113301
Credit: #113321 (@kvnloo), #113323 (@nolanchic)
`_try_nous()` resolves Nous runtime credentials on every auto-route discovery
walk before it checks stored auth, so users who never logged into Nous Portal
got a new "Auxiliary Nous client unavailable: ... not logged into Nous Portal.
Run `hermes model`" WARNING on every pass, on top of the ladder's existing
"no Nous authentication found" summary. record_nous_credential_failure now
logs at WARNING only when a credential exists that failed (a coded AuthError
such as invalid_grant / a quarantine marker, or persisted Nous auth state)
and at DEBUG for the uncoded never-logged-in case. The detail is still
remembered either way so the goal judge keeps naming the cause (#42177).
Also sentence-terminate the AuthError message before format_auth_error
appends the remediation ("Invalid refresh token. Run `hermes model`..."
instead of "Invalid refresh token Run ..."), and assert the contract (code +
remediation present) instead of the literal string in the test.
With auxiliary.goal_judge.provider pinned to nous and a dead refresh token
(invalid_grant), the status line read "judge error: RuntimeError": the ladder's
_resolve_nous_runtime_api swallowed the resolver's AuthError at DEBUG and the
generic "no API key" RuntimeError was reduced to its type name by judge_goal.
Users were sent to context-length / model debugging instead of `hermes model`.
Now the failure is remembered in agent/auxiliary_unavailable.py (one WARNING per
distinct message, preferring the persisted quarantine marker because the pool
rung wipes the dead tokens before the auth-store resolver runs), the ladder
raises AuxiliaryClientUnavailable carrying that detail for provider=nous, and
judge_goal maps it to
"goal_judge auxiliary client unavailable: Nous Portal runtime credentials
unavailable: Invalid refresh token. Run `hermes model` to re-authenticate.
(code: invalid_grant)" while still failing open with the same 5-tuple contract.
Slim redo of #42179 for the current call_llm-based judge_goal.
Fixes#42177
Co-authored-by: Harry Riddle <ntconguit@gmail.com>
_print_unknown_key_notice always appended "Custom top-level keys are supported and bridged to
the environment". Before the unseeded-key change no nested path reached this notice; now
`stt.provider` or `agent.max_turnz` did and were told they are env-bridged, which they are not.
Nested paths get only the --force hint.
In _validate_config_key the fuzzy same-level sibling (cutoff 0.6) was tried before the
structural "path minus its wrong prefix is a known key" check. While every unknown sub-key was
refused that order did not matter; now that only the wrong-prefix case is refused, a path whose
stray middle segment fuzzy-matched a sibling was WRITTEN with a misleading did-you-mean:
`agent.gateway.strict` -> (False, 'agent.gateway_timeout') instead of being refused as
`gateway.strict`. A structural match is proof, a fuzzy match is a guess; check it first.
Drop the display.tool_progress row (it pinned a did-you-mean the comment itself called misleading; seeding the key would fail it for an unrelated reason) and the skills.creation_nudge_interval row (same invariant as stt.provider, the key named in the issue).
The display.tool_progress row passed suggestion=None, but
_validate_config_key("display.tool_progress") returns
(False, "display.tool_progress_command") because only the _command sibling
is seeded in DEFAULT_CONFIG. The `if suggestion:` guard let the row pass
without ever checking the notice, so the misleading "Did you mean" this
real runtime key (cli.py:2568) receives was neither asserted nor visible.
Pin the actual suggestion for that row and assert the notice text exactly
for every row: present with the expected sibling, or absent. Fix the
comment too — only stt.provider comes from da942e4483's list; the rows are
unseeded runtime-read keys at agent/agent_init.py:1324, cli.py:2568 and
tools/transcription_tools.py:241.
Seeding display.tool_progress in config_defaults.py (hermes_cli/AGENTS.md:
"every reader a registry entry") is the proper follow-up that removes the
misleading suggestion; it is out of scope for this salvage.
The CLI reference, the configuration guide and the set_config_value
docstring still said that any unknown path under a known section is
refused with a did-you-mean. After b47f40e699 only a path whose suffix is
itself a known key (gateway.discord.foo -> discord.foo) is refused; every
other unknown path under a known section — a same-section typo or an
unseeded runtime-read key — is written with a did-you-mean notice, because
DEFAULT_CONFIG is an incomplete schema and cannot tell the two apart.
Reword all three sites to state the refusal that actually exists and the
write-with-notice fallback, so users are not told a typo will be blocked.
Fold the trade-off into the salvaged parametrized test instead of adding a
new test function: `agent.max_turnz` (suggestion `agent.max_turns`) is now
written with the post-write notice rather than refused, because the schema
walk cannot tell a same-section typo from a deliberately unseeded
runtime-read key such as `stt.provider`. Assert the notice and the
suggestion so reviewers see the behaviour change; refresh the class
docstring that still described the pre-#114107 refusal rule.
The flush payload's ``ts`` is ``int(time.time())`` while ``sessions.started_at`` is a REAL, so
``started_at > not_after`` rejected a row minted later in the same second as the flush — the
common "message arrives, session minted, SIGTERM" shape lost recoverability (file preserved, not
misrouted). Compare ``floor(started_at)`` against the whole-second ``ts`` so same-second rows
are adopted and only rows from a later second are refused.
Spotted by the round-2 gate on the salvage stack.
`_db_for_key` deliberately returns None for a named-profile key whose home cannot be
resolved (fail closed, #66887/#102157), but `resolve_session_id_for_key` returned
`(session_id, None)` on a routing-map hit, and `_recover_one_payload` treated that None
as "fall back to session_db" — the owned ROOT state.db at the production call site. A
profile message with an unopenable home was appended to the root store: the very
split-identity write the fail-closed return exists to prevent.
- resolver: an unresolvable store answers None before the peek and the row fallback
(None already means "preserve the flush file").
- recovery: the resolver's db is authoritative for resolver-resolved payloads; the owned
default store only serves payloads that already carry a session_id (and cap-drop spool).
- `started_at` is `REAL NOT NULL` in the schema, so the `float()` fence stays as is.
resolve_session_id_for_key calls the peer finder directly, while the
sibling _query_recoverable_row also applies _recovered_row_matches_source_scope
and _recovered_row_allowed_for_active_profile. Read both fences against the
resolver's call shape; both are redundant there, so this adds a WHY comment
at the call site rather than a fence call.
- Profile fence: it returns True immediately when recovered.session_key ==
requested key. The resolver passes no chat_id/chat_type, so
find_latest_gateway_session_for_peer returns after the _PEER_BY_KEY_SQL
branch (`s.session_key = ?`) or None; the tuple fallback is unreachable.
Every hit therefore carries the requested key and the fence is a no-op.
- Slack workspace fence: it compares origin_json.scope_id with
source.scope_id. The resolver has no SessionSource (only the key), and a
scoped Slack key embeds the scope_id as its own slot
(`<ns>:slack:<chat_type>:<scope_id>:...`, build_session_key). An exact
key match therefore already pins the workspace, regardless of whether
the row lives in a profile store or a non-multiplexed root store — the
store choice (_db_for_key) partitions by profile, not by workspace, and
the key does the workspace work in either store. The sibling needs the
fence only because it also looks up the UNscoped legacy key, whose row
can belong to any workspace; the resolver never does that lookup.
_find_gateway_session_row and resolve_session_id_for_key each carried the
same guarded call: getattr the finder off a possibly-None db, check
callable, try/except -> debug log -> None. Extract it as the mixin-private
static `_peer_row(db, *, source, session_key, raise_on_lookup_error=False,
**peer)` and call it from both sites.
Behaviour is unchanged at both call sites: the tuple site passes
user_id/chat_id/chat_type/thread_id through **peer with the same
allow_peer_fallback gating and keeps raise_on_lookup_error; the exact-key
site passes only source + session_key, so the finder's tuple fallback
still never runs there. The only observable difference is the resolver's
failure debug line, which now reads "Gateway session DB recovery failed"
(shared) instead of "Session key->id resolution failed"; no test asserts
on either.
The platform-slot parse (parts[2]) in resolve_session_id_for_key is left
in place: the only key-parsing helper on the mixin,
_profile_from_session_key, returns the profile namespace (parts[1]) and
nothing returns the platform slot, so there is nothing to fold onto.
Keep test_recover_payload_without_session_id_uses_resolver_and_deletes_file
(positive path) and add one negative fence test on the resolver: a row
started after the flush ts is rejected, an older one is adopted with its
owning db. Drop the WhatsApp alias tests (feature removed), the duplicate
positive test and the redundant no-resolver/None-resolver file-preserved
tests, which were exercising the same branch three ways.
Follow-up to the #75536 + #106112 picks; changes vs #106112:
- Routing map first: peek_session_id (sessions.json) is authoritative for
the session a message was routed to at shutdown. The durable-row lookup
(find_latest_gateway_session_for_peer under the exact key) is only the
fallback for a pruned map.
- Fence added per gysyl's #106112 review: a row whose started_at is later
than the flush payload's ts (passed as not_after by _recover_one_payload)
cannot be the message's origin and is never adopted; None is returned so
the flush file is preserved instead of appending into a newer session.
- WhatsApp phone<->LID alias expansion dropped: main's session_recovery
does no alias expansion anywhere else, and re-implementing it here made
the resolver diverge from the recovery path it sits beside. Exact-key
match only.
- The private _db_for_key reach stays inside SessionRecoveryMixin so the
(session_id, db) pair lands the append in the owning profile partition;
shutdown_flush never touches store internals.
Pins that an ordinary append failure on one spool file only skips that
file: the remaining files are still recovered, the good file is unlinked,
and the failing one is preserved for the next startup retry.
Fails before the handler reorder — the RuntimeError propagates out of
recover_pending_to_db and the second message is never recovered.
`recover_pending_to_db` walks the shutdown flush spool and appends each
pending message back into state.db. Its per-file `try` listed
`except BaseException:` above `except Exception as exc:`. Exception is a
BaseException subclass, so the ordinary-error handler was unreachable and
every ordinary failure took the interrupt path: close the owned DB and
re-raise.
The effect is that a single unrecoverable spool file aborts the entire
recovery pass. Because the file is only unlinked after a successful
append, it survives and re-poisons the next boot, and the pass is walked
in `sorted(glob("*.json"))` order over uuid4-named files, so a different
subset of the user's pending messages is stranded each time. The caller in
`gateway/run.py` wraps the call in `except Exception: pass`, so nothing is
logged — the messages simply never come back.
Reordering the two handlers restores the documented per-file behaviour
("Leave the file for next startup retry") while keeping the interrupt
contract exactly as written: a KeyboardInterrupt, SystemExit or
CancelledError still closes an owned SessionDB and propagates.
recover_pending_to_db skipped every real flush file: _serialise_value
captures only the text field from adapter MessageEvent objects (they
have no session_id attribute), so recovery always hit the 'no
session_id' skip branch — messages queued during a gateway drain were
written to disk and then silently never re-ingested, despite the
user-facing 'queued for the next turn' promise. The existing tests
masked it by hand-writing session_id into payloads real events never
produce. Add an optional session_resolver parameter and wire it to
SessionStore.peek_session_id at the startup call site; files remain
preserved (with the warning) when a key genuinely can't resolve.
dispatchServerRequest hand-rolled request.fail(JSON_RPC_METHOD_NOT_FOUND,
'Hermes Desktop has no server-request registry yet'), but the channel already
owns that reply: JsonRpcRequestChannel.deliverRequest answers -32601 and fires
onUnhandledRequest when a ServerRequestHandler returns false, and
GatewayBootOptions.handleServerRequest already documents "false = no handler
(the channel answers -32601)".
Make dispatchServerRequest return false when the registry has no
onServerRequest and true after forwarding; dispatchPrimaryServerRequest and
both gateway.onRequest registrations (store/gateway.ts secondary sockets,
use-gateway-boot.ts primary) now propagate that value to the channel. Keep
the desktop-specific wording by wiring onUnhandledRequest on HermesGateway
beside the onRequestHandlerError sink (console.warn), which needs the same
GatewayClientOptions passthrough onRequestHandlerError got. Drop the now
unused JSON_RPC_METHOD_NOT_FOUND import from the store and export
JSON_RPC_INTERNAL_ERROR from the shared barrel beside it.
Test: the store test asserts the false return with no fail() call when the
registry is missing, and true + forwarded profile when it is present.
Follow-ups (same class, outside this stack): use-gateway-boot.ts ~L889-891
still hand-rolls -32601 when the registry is present but has no handler;
ui-tui has its own copy.
Mutation check: reverting store/gateway.ts to the merge-base left the desktop
suite green — nothing exercised dispatchServerRequest. Cover both arms through
dispatchPrimaryServerRequest: a registry configured without onServerRequest
answers request.fail(-32601, /registry/) exactly once; with onServerRequest
the request is forwarded carrying the profile and fail is never called.
The deliverRequest catch branch wrote to a module-level console.error sink
(the only direct console.* in apps/shared/src) and fired onUnhandledRequest,
whose contract is "nobody handled it, already answered -32601" — so the TUI
logged a -32603 crash as "unhandled server request".
Add onRequestHandlerError(error, request) to JsonRpcRequestChannelOptions
beside onHeartbeatFailure, call it from the catch after answering -32603,
and drop the console sink. Wire both owners: HermesGateway (desktop, via a
GatewayClientOptions passthrough) logs to console.error like its dial-failure
sink; ui-tui gatewayClient pushes a [protocol] log line. Collapse the two
normalisation arms into the existing `error instanceof Error ? … : new
Error(String(error))` idiom and restore the early `return true` instead of
the handled flag + break — nothing runs after the loop but the -32601
fallthrough.
Test: the crash case now asserts onRequestHandlerError fires once for the
-32603 request and onUnhandledRequest only for the -32601 one.
Follow-up to the salvage of #112791.
- json-rpc-gateway.ts: revert the try/catch around `channel.handleFrame`
to main. deliverRequest is the single chokepoint that dispatches into
feature handlers and now answers -32603 itself; a second catch in the
socket listener is defense-in-depth that would also hide bugs in event
and response handling that should surface as uncaught errors.
- store/gateway.ts: the primary and secondary dispatch sites carried the
same copy-pasted "no registry -> fail -32601" block. Fold both into one
file-local `dispatchServerRequest(request, profile, connectionId)`.
The secondary keeps main's tagging (its own connectionId, no fallback
to the active connection), which the contributor's version changed.
A clarify request renders as an eternal spinner when anything in the
renderer's handler chain throws: deliverRequest had no error handling,
the WS message listener let the exception escape as an uncaught error,
and both dispatch sites silently no-oped via optional chaining when the
registry was absent. In every case the backend (clarify_tool blocks up
to 3600s) never receives any frame — no result, no error — and waits
out its whole deadline.
- json-rpc-channel: wrap each handler invocation; a crash now answers
-32603 ("server request handler crashed: <method>"), fires the
onUnhandledRequest hook, logs the stack, and stops. The unhandled
path still answers -32601.
- json-rpc-gateway: guard the socket message listener so no frame can
escape as an uncaught error.
- store/gateway (primary + secondary wiring): missing registry now
fails the request immediately with -32601 instead of dropping it.
Backend already handles {"error"} response frames
(tui_gateway/server_requests.resolve_response), so fail-fast answers
settle the tool at once; no backend change needed.
Regression test drives boom→-32603, unknown→-32601, and a working
request after the crash.
The lib test sits beside markdown-preprocess.directives.test.ts as
markdown-preprocess.reasoning.test.ts, and the frame-fidelity test joins
its markdown-text.*.test.tsx siblings as markdown-text.reasoning.test.tsx,
so a glance at the directory shows what each file exercises. Also trim the
REASONING_TAGS comment: how the two tag lists converged is git history, not
something a reader of the constant needs.
OPEN_REASONING_BLOCK_RE needs the full `<tag>`, so a frame ending in `<thin`
at a block boundary painted `<thin` as prose for one frame and then erased it
— the paint/un-paint class of #62774, one frame long. The fidelity test carved
an escape hatch around exactly those frames.
Add a third pass that drops a trailing partial open tag at a block boundary,
restricted to prefixes of the known reasoning tag names (built from
REASONING_TAGS) so `<div` at a line start still renders. This is the desktop
counterpart of agent/think_scrubber.py `_hold_partial`/`_max_partial_suffix`,
cited rather than ported. The fidelity test's escape hatch is deleted so the
monotonic-prefix invariant holds on every frame; the unit test gains the
`<thin` positive and `<div` negative cases.
stripReasoningBlocks runs on the accumulated message text every streaming
flush (markdown-text.tsx). Its replacer sliced the whole text twice per
closed block to test whether a space is needed at the seam — O(n) copies plus
a forward scan per block per flush. Read the single char on each side of the
match instead.
The seam test also had a second defect: with two adjacent closed blocks the
second block's `before` ended in the first block's `>`, so a second space was
emitted (`no Hermes`). Matching a run of adjacent blocks as one match makes
the two-edge check see the real prose on both sides. Covered by an extra
expect in the existing seam test.
WHAT: one REASONING_TAGS alternation feeds both regexes and now also covers
`thought` and `reasoning_scratchpad` (agent/think_scrubber.py THINK_TAG_NAMES);
the desktop-only `scratchpad`/`analysis` stay. Behaviour change beyond the
contributor's: closed or unterminated `<thought>`/`<reasoning_scratchpad>`
blocks are now stripped on the desktop too.
Doc comments trimmed to the WHY (block-boundary rule, why an unterminated block
is held back). Tests cut to the ≤2-invariant shape: two unit cases (seam,
unterminated-vs-mention) and one DOM frame-by-frame case; the accents/plain-prose
DOM cases were protection for unrelated code and are dropped.
preprocessMarkdown runs on the accumulated text on every streaming flush, so
the plain REASONING_BLOCK_RE replace damaged the visible answer in two ways a
settled message never shows:
- a model that inlines its chain of thought in the answer channel streams the
open tag long before the close one, and the regex needs both, so the reasoning
rendered as chat prose until the close tag arrived — and when it did the whole
span vanished in one frame, taking text the reader had already read with it
(#62774: the mid-reply "truncation" on long answers).
- the regex ate the whitespace on its seam, so a block sitting between two words
fused them: `no` + `Hermes` rendered as `noHermes`.
An unterminated block now strips at a block boundary — the same rule
agent/think_scrubber.py draws, so a real reasoning preamble (always its own
block) disappears while prose that merely mentions `<thinking>` mid-sentence
survives — and a removal between two prose fragments leaves one space instead of
deleting the seam.
Tests: markdown-reasoning-stream.test.ts (module level: seam, unterminated
blocks, per-delta leak + visibility monotonicity) and
streaming-text-fidelity.test.tsx (real surface: accents/emoji and plain prose
streamed in 1- and 3-character deltas, plus the chain of thought never painting
a frame).
The no-keepalive proof branch ran on ANY timeout wake. With idle_timeout_seconds=60 the wait is min(180, 60); an overlapping RPC hides the recycle deadline so _recycle_if_due() is False and the session was marked proven after 60 s, clearing the #62212 rapid-drop budget early. Fix a proof_at deadline before the loop and gate the branch on it; the unproven timeout is the remaining distance to proof_at. Tests: the stdio test now shows the first (idle-limit) wake does not prove; the interval test covers an explicit keepalive_interval on stdio.
WHAT: drop the idle-vs-lifetime parametrize on
`test_stdio_recycle_wakes_after_active_rpc`, keeping the idle case, and drop
its `_recycled_reason == reason` assertion.
WHY: both parameters exercised the same new invariant (releasing the RPC lock
wakes the lifecycle loop so the now-visible deadline recycles the server); the
reason assertion pinned pre-existing `_stdio_recycle_reason` behaviour that is
not this stack's concern. The stack now adds exactly two test functions.
WHAT: `_wait_for_lifecycle_event` hoists `is_http = self._is_http()` (it only
reads the immutable `"url" in self._config`) and reads `keepalive_interval`
from the config once instead of `in` + `.get()`. The one-shot RPC-idle waiter
is now spawned on `not is_http and self._rpc_lock.locked()` alone, dropping
the inline `_idle_timeout_seconds is not None or _max_lifetime_seconds is not
None` clause. The waiter list is built by one comprehension per iteration
(rpc_idle_task changes between iterations) and the `finally` reuses the last
value; `_cancel_waiters` already skips done tasks. The docstring folds the
#17003 keepalive paragraph into the first paragraph instead of describing the
keepalive twice.
WHY: the dropped clause was the inverse of
`MCPServerHealthMixin._stdio_recycle_deadlines` re-derived by reaching into
health-mixin attributes. Without it, a stdio server with no limits and an
active RPC spawns one trivially-completing waiter per RPC; on lock release
the loop takes one extra iteration where `_recycle_if_due()` is False and
`_next_stdio_recycle_deadline()` is None, so it simply recomputes the timeout
and waits again — harmless.
After the stdio keepalive default became None, the lifecycle loop's
`if keepalive_interval is None: continue` skipped the only idle path to
`_mark_session_proven()`. `_session_proven` is otherwise set only on a
tool-call success or a suspect health-check pass, and each new session
resets it, so an idle default-config stdio server never became proven:
every later child death/reconnect charged the rapid-drop budget that a
180 s idle survival used to clear (#62212 regression).
While the session is unproven, keep the `asyncio.wait` timeout at
`_DEFAULT_KEEPALIVE_INTERVAL` for stdio too; when it fires with the child
still alive, mark the session proven without pinging. Once proven the
stdio timeout returns to None, so a healthy idle pipe is never probed.
Shutdown/reconnect precedence and the rpc_idle waiter are untouched.
Test: `_captured_interval` now runs two lifecycle cycles (first times
out, second shuts down) through a shared helper so the stdio test asserts
timeouts [180, None], zero pings and `_session_proven` set, instead of
copy-pasting the fake `asyncio.wait`. Docstring/comment updated to the
new rule.
_schedule_polling_recovery promised the gateway 'stays alive and will retry' for every error, but a _PollingStallError goes straight to _go_fatal_network (supervisor rebuild). Branch the wording on the error type, drop the watchdog's own pre-log so a stall yields exactly one error-level line (from _go_fatal_network), and carry stalled_for/generation in the stall error text instead. Test module docstring and test name updated to match the hand-off semantics.
Hoist the `_PollingStallError` check in `_handle_polling_network_error` to
right after the teardown/fatal guard, before the retry counter increment,
the exponential sleep, `_stop_updater_or_go_fatal` and both connection
drains. Use a plain `isinstance` (both raise sites construct the error
directly; nothing wraps it). The stall test now also asserts no sleep, no
retry-counter bump, no in-place `updater.stop()` and no drain.
WHY: the check sat after `await asyncio.sleep(delay)` (5-60 s), the
counter bump and a bounded `updater.stop()` (up to 15 s), so a gateway
already confirmed deaf stayed deaf 5-75 s longer and consumed a retry
slot for something that is not a retry.
The in-place `updater.stop()` before the handoff is dropped deliberately:
`_go_fatal_network` -> `_handoff_polling_fatal_error` -> supervisor
rebuild runs `disconnect()`, which performs the same bounded
`updater.stop()` (`_UPDATER_STOP_TIMEOUT`, falls through on timeout) plus
`app.stop()/shutdown()`, so stopping here only duplicated that work on the
slow path. Docstrings for `_handle_polling_network_error` and
`_check_polling_stall` no longer describe the stall as a reconnect-ladder
escalation.
`_check_polling_stall` hand-rolled the body of `_schedule_polling_recovery`
(set `_send_path_degraded`, `_mark_degraded()` when running, spawn
`_handle_polling_network_error`). Call the helper instead, matching the
post-reconnect verifier stall site.
WHY: one recovery entry point keeps degraded-state marking and the
"polling degraded (reason)" log line consistent across every scheduler.
The helper's early-return guards (`_teardown_started or has_fatal_error`,
`_recovery_in_flight()`) are already asserted at the top of
`_check_polling_stall` with no await in between, so they are no-ops here
and behaviour is unchanged.
Merge the watchdog and verifier stall tests into a single parametrized
`test_confirmed_stall_hands_off_before_reusing_updater` and assert the
adapter is also marked degraded, keeping two invariant tests for #113618
(stall -> fatal handoff; generic network error -> in-place reconnect).
Replace `_looks_like_polling_stall` (a three-substring classifier over
`str(error)`) with `_PollingStallError(RuntimeError)`. The watchdog and the
post-reconnect verifier raise it at their two stall sites; the reconnect
ladder checks `isinstance` through `_iter_exception_graph` before handing
the adapter to the supervisor.
WHY: a substring match couples recovery routing to log wording (rename a
message, silently lose the fatal handoff) and can misfire on unrelated
errors that quote the same words. A typed exception keeps #113657's
behaviour with no behavioural change beyond the classifier's source.
