Cross-PR interaction fix: #77714 (salvage of #71775) changed
_available_entries to return (available, pending_refresh) while #77631
(salvage of #67642) added next_available_at() which still truthiness-
tests the bare return. A non-empty tuple is always truthy — even
([], []) — so the reset-aware gate silently returned None ('no wait
info') for every exhausted pool, disabling the feature #77631 shipped.
Unpack the tuple and test the available list.
Also adapts the lock-probe test for the RLock introduced by #77714
(same-thread non-blocking acquire always succeeds on an RLock; probe
from a helper thread instead).
```
Error: optimization failed: no such table: messages_fts_trigram
No data was lost. Re-run to resume.
```
on any install where the trigram FTS index is legitimately absent. The failure is
deterministic — re-running can never make progress, because the crash happens at the same
point every time — so the database is permanently stuck on the legacy high-footprint FTS
layout with no supported way forward.
Observed on a 5.4 GB production `state.db`. After the fix the same database optimized
successfully and shrank to 3.3 GB.
The trigram index is absent whenever the runtime cannot maintain it. On a SQLite build
without the `trigram` tokenizer, `_ensure_fts_schema()` returns `False`, so `__init__`
leaves `self._trigram_available = False` and no `messages_fts_trigram` table on disk. This
is a **supported degraded runtime**, not damage — CJK/substring search falls back to
`LIKE` and everything else works normally. `_is_fts5_unavailable_error()` and
`_warn_trigram_unavailable()` exist specifically to make this path graceful.
Two code paths write the boundary sweep for the deferred FTS rebuild, and only one of them
respects that flag:
| Function | Trigram `INSERT` guarded? |
|---|---|
| `fts_rebuild_step()` | ✅ `if include_trigram:` where `include_trigram = self._trigram_available` |
| `_fts_rebuild_finish()` | ❌ unconditional |
`_fts_rebuild_finish()` runs the boundary sweep at the *end* of the backfill. Its
unguarded `INSERT INTO messages_fts_trigram …` raises `OperationalError`, which propagates
out of `optimize_fts_storage()` and aborts the entire optimization — *after* the backfill
has already completed. Hence the characteristic output showing 100% progress immediately
before the error:
```
Rebuilding index: 100% (909,671/909,671)
Error: optimization failed: no such table: messages_fts_trigram
```
There is a second, quieter consequence. The teardown phase that reclaims the demoted
`fts_v22_trash_*` shadow tables runs *after* the backfill phase in
`optimize_fts_storage()`. Because the crash happens before teardown is ever reached, those
tables are never emptied or dropped — so the space the migration was supposed to reclaim
stays allocated indefinitely, and the leftover trash tables look (misleadingly) like
evidence of a half-finished migration.
Build a populated v23 database, set the deferred-rebuild markers, then reopen it on a
runtime where `_ensure_fts_schema('messages_fts_trigram', …)` returns `False` (exactly
what a SQLite build without the trigram tokenizer produces) and call
`optimize_fts_storage()`:
```
[precondition] trigram absent, _trigram_available=False, rebuild pending ✓
RED ✗ optimize_fts_storage raised OperationalError: no such table: messages_fts_trigram
```
With this patch applied, unchanged harness:
```
optimize_fts_storage returned {'ok': True, 'vacuumed': None}
GREEN ✓ optimize ok; markers cleared; base FTS 'zebra' -> 200 hits
```
Full harness and transcripts in `TEST-EVIDENCE.md`.
Gate the sweep on `self._trigram_available`, exactly as `fts_rebuild_step()` already does:
```python
include_trigram = self._trigram_available
def _do(conn):
...
if include_trigram:
conn.execute("INSERT INTO messages_fts_trigram(...) ...")
```
The base `messages_fts` sweep and the marker cleanup are untouched, so the rebuild still
finalizes correctly and the index remains complete for every row it is responsible for.
The fix does not disable or weaken search to dodge the error — the regression tests assert
that base FTS still returns results afterwards.
`TestFtsRebuildFinishWithoutTrigram` in `tests/test_hermes_state.py`:
- `test_rebuild_finish_skips_trigram_when_unavailable` — drives `_fts_rebuild_finish()`
directly on a trigram-less runtime; asserts it completes, clears both rebuild markers,
and leaves base FTS searchable.
- `test_optimize_fts_storage_succeeds_without_trigram` — end-to-end through the public
`optimize_fts_storage()` entry point; asserts `ok=True`, markers cleared, search intact.
Both use the existing `_NoTrigramConnection` helper already in the file. Both fail on
`main` with `no such table: messages_fts_trigram` and pass with this patch.
`tests/test_hermes_state.py` passes in full (463 tests → 465 with these two). `ruff` clean.
This PR is the crash only.
A companion PR narrows `_db_opens_cleanly()` so that
`hermes sessions repair --check-only` stops reporting a write-broken FTS schema as
healthy — the gap that makes this class of problem hard to diagnose in the first place.
The two are independent and can land in either order.
Review folds on the #71775 salvage (dossier findings 1+2):
- self._lock becomes an RLock and the mutation primitives
(_replace_entry, _persist) are now self-locking, so the deferred
single-use-token refresh path — which deliberately runs its
cross-process flock + OAuth network I/O OUTSIDE the pool lock —
still serializes its pool mutations against concurrent
select()/rotation. In-lock callers re-acquire reentrantly.
- Dropped _refresh_pending_entries' redundant second _replace_entry:
_refresh_entry already merges the refreshed entry internally.
Adds tests/agent/test_credential_pool_deferred_refresh.py pinning both
invariants: select() must NOT hold the lock during the refresh window
(the PR's whole point), and the post-refresh mutations MUST contend on
the lock (blocking-thread probe).
select() and acquire_lease() held self._lock during the entire
_available_entries() loop, which for openai-codex and xai-oauth providers
includes a cross-process file lock (_auth_store_lock) plus OAuth token
refresh HTTP POST. The lock timeout can exceed 20 seconds, blocking all
credential pool consumers across every gateway thread and subagent.
Collect single-use-token refresh entries under the lock, then execute the
refreshes outside it. On success the refreshed entry is merged back into
the pool and re-selected. Non-single-use providers (anthropic, nous)
continue refreshing inside the lock since their refresh is a simple HTTP
POST with no cross-process coordination.
Review fold on the #67642 salvage: next_available_at() called
_available_entries() — which prunes DEAD entries, syncs tokens, and
persists — and iterated self._entries with no lock, racing concurrent
select()/rotation exactly as has_available()'s comment warns. Wrap the
method body in self._lock and pin it with a non-blocking-acquire probe
test.
restore_primary_runtime retries the primary every turn once the 60s
transient cooldown clears. For subscription-window limits (Claude
Pro/Max 5h windows, Codex weekly caps) the reset is hours or days away,
so every retry is a guaranteed failure costing two provider switches
and two prompt-cache invalidations per turn.
Add CredentialPool.next_available_at() (earliest reset across exhausted
entries; None when available now or no reset info) and gate the restore
on it: skip while the primary's pool says nobody can serve, restore on
the first turn after the reset elapses. Fail-open: any gate error or
missing reset info falls through to the existing per-turn retry, so
recovery can never be later than today. Cross-provider fallbacks
consult the PRIMARY's pool (not the attached fallback pool), reusing
the loaded pool for the existing rebind to keep auth reads at one per
restore.
Every hashed bundle chunk under /assets/ was served with no caching
directives, so each dashboard load re-fetched (or at best revalidated)
every JS/CSS chunk. Those filenames carry a Vite content hash — the
bytes behind a given URL can never change; a rebuild mints new
filenames referenced by a freshly served index.html.
Mark them Cache-Control: public, max-age=31536000, immutable:
- the /assets StaticFiles mount, via a subclass that stamps the header
on 200s only (404s stay uncached — a rebuild can create the file),
- serve_css, preserving its X-Forwarded-Prefix url() rewrites for
/fonts/, /fonts-terminal/, /ds-assets/, /assets/.
index.html keeps no-store, no-cache, must-revalidate — it is the
mutable entry point that binds users to the current hashes.
The original PR also added hand-rolled per-request gzip compression of
asset responses; that part is deliberately dropped. This server is a
localhost-default dashboard backend: compressing every response on the
CPU to save loopback bandwidth is a pessimization, and callers that
front it with a real proxy already get compression there.
Salvaged from PR #28543 (idea by @sea-monsters; gzip groups dropped as
described above).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
le=100 would 422 real desktop callers: sessions-settings fetches
archived at limit=200, the command palette lists at 200, and the
electron remote-merge over-fetches limit+offset (exceeds 100 at
offset>=81, and its .catch(()=>null) silently drops remote sessions).
Clamp must sit above real client maxima. New test pins limit=200 w/
offset.
Re-derivation of aydnOktay's twin clamp PRs onto current main (the
session-list endpoints moved into web_routers/; the analytics endpoints
gained asyncio.to_thread wrappers since the originals):
- limit le=100 on /api/sessions, /api/sessions/search and the
/api/profiles/sessions fan-out (one unbounded request could drag every
session row + correlated-subquery preview work out of SQLite, times
every profile's state.db on the fan-out).
- days ge=1 le=365 on /api/analytics/usage + /api/analytics/models
(huge or non-positive values force full-history InsightsEngine work or
inverted windows; the UI only offers 7/30/90 presets).
FastAPI Query bounds reject at the validation layer (422). 8 new tests;
both clamp classes mutation-checked (clamp removed -> its tests fail).
The 2-line alias had zero production consumers (web_server calls
get_usage_breakdown directly). Tests rewired onto the real API; the
contracts they pin are unchanged. Stale test docstring fixed.
On dashboard-only sessions nothing else executes check_fn warmers (they
live only in the tool-schema build), so the hub's read-only cache lookup
would report auth_required=False forever. On a cache miss, schedule a
deduplicated daemon-thread probe off the request path; the short hub TTL
surfaces the verdict on the next fetch.
The curator LLM review loop (_run_llm_review) built its AIAgent without
enabled_toolsets, so it advertised the full default catalog (~30 tools plus the
context_engine lcm_* family) on every call. The fork uses only four tools, fixed
by its own system prompt, with no dispatch path to the rest, so ~26 tool schemas
shipped on every request as dead weight: ~7K input tokens per call on a loop that
makes 50-100 calls per consolidation pass.
Restrict the fork to enabled_toolsets=["skills", "terminal"], the same tools the
prompt already names. Behavior-neutral: the prompt held the model to these tools
and nothing routed calls to the others. Mirrors the background_review fork
(background_review.py:788-794). Call-site only; AIAgent already forwards the kwarg.
Adds test_review_fork_restricts_toolsets_to_skills_and_terminal (captures the
constructor kwarg) and test_review_fork_toolset_surface_is_skills_plus_terminal
(pins the resolved surface).
Rebase fold on the salvaged gate:
- is_trivial_prompt/TRIVIAL_PROMPT_RE move to agent/memory_provider (the
ABC both the core gate and providers already import) — one source of
truth; honcho's _TRIVIAL_PROMPT_RE now aliases it, turn_context and the
queue_prefetch_all warm path (a sibling site main grew after the PR's
base) both use it
- tests: gate tests at the prefetch call site (mutation-checked), shared
classifier tests incl. prefix-collision guards (k8s/yolo/note/supper),
and honcho dialectic-machinery tests re-driven with a substantive prompt
("hello" became trivial by design — those tests exercise thread cadence,
not the classifier)
Salvage of PR #25350 (commits 88ffede2d + 2b848a0b2 + 3136dc63a, squashed
and ported): the run_agent.py prefetch site the PR gated has since moved
into agent/turn_context.py's build_turn_context(), so the trivial-query
gate lands there instead.
- Gate the per-turn memory_manager.prefetch_all() on a trivial-prompt
check so greetings/acknowledgements ('hi!', 'thanks', 'ok') no longer
block the turn on provider network round-trips or inject stale context.
- Extend honcho's _TRIVIAL_PROMPT_RE with greetings and a trailing
punctuation class so 'hey!' / 'hello.' classify as trivial.
- Add honcho classifier tests for greeting forms.
Follow-ups on the salvaged bounded-read fix:
- refresh flow: the non-200 branch reads a STREAMED body, which fails
(ReadError/StreamClosed) once the httpx.Client context has exited —
moved inside the context. Repro + regression test use a real socket
server (MockTransport buffers in memory and cannot catch this).
- truncation guard: >limit bodies end with ...[truncated] (mutation-checked
against the is_stream_consumed fallback).
- test mocks now model the streamed-read surface (is_stream_consumed,
iter_bytes, client.send) so non-200 paths exercise the real bounded read.
The as_completed drain + `with` join made the parallel version WORSE than
sequential main in the common case (first endpoint succeeds fast, others
slow/unreachable): main returned at first success, the parallel version
waited for every straggler. Now: after each completion, walk endpoints in
priority order and return as soon as a success is unbeatable (all
higher-priority probes already finished); pool uses shutdown(wait=False) so
losers drain in the background. Mutation-checked: removing the early exit
makes the new timing test fail (8.2s vs <1.5s).
Rebase fold: the original PR predates ZAI_ENDPOINTS growing per-endpoint
probe_models lists; the parallel worker now preserves that candidate-model
fallback loop (was: scalar model). Tests (both mutation-checked):
- candidate-model fallback within one endpoint worker
- ZAI_ENDPOINTS priority order wins over completion order
- all-fail returns None
Follow-up on the salvaged pair: the original guard's `not msg_id` arm let an
id-less internal/synthetic event erase a tracking entry a concurrently-queued
id-bearing message's drain task still needs for recall matching (id-less
events never write entries in _dispatch_inbound_event, so they must never
pop). Tests cover: normal cleanup, id-less non-erasure, overwritten-entry
ownership handoff, TTL eviction + fresh-entry survival.
Adds two regression tests for the #59077 batch: (1) _get_session_rich_rows_batch(compact_rows=True) uses the schema-derived compact projection (no system_prompt, git_branch/git_repo_root kept); (2) list_sessions_rich(compact_rows=True) threads compact_rows through the compression-tip projection call site. Mutation-checked: hardcoding compact_rows=False at the call site fails test 2.
list_sessions_rich()'s compression-root projection called
_get_session_rich_row() once per root — a separate single-row query per
compression root on every session-list render. Resolve every tip id
first, then fetch all tip rows in one WHERE id IN (...) query via the
new _get_session_rich_rows_batch().
_get_session_rich_row() is now a thin wrapper over the batch method, so
the enriched SELECT (preview + last_active) lives in exactly one place —
future column changes (e.g. #42196's include_system_prompt) only touch
one query.
get_compression_tip()'s chain walk is untouched; it's a genuine
per-session graph walk with branch/delegate-exclusion and race handling,
and batching it safely is out of scope here.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The INDEXED BY pin is a hard dependency -- SQLite raises 'no such
index' when the named index is missing. That happens in production:
the web dashboard's usage analytics (_get_usage_analytics,
_get_models_analytics) open state.db read_only=True, which skips
_init_schema, so a DB last written by a pre-index version has no
idx_messages_assistant_calls_by_session and every insights call
crashes with OperationalError (reproduced E2E).
Probe sqlite_master once in __init__ and strip the pin from the four
prepared statements when absent -- identical rows, optimizer-chosen
plan, no crash. Replaces the change-detector test that froze the
crash as intended behavior with a fallback-equivalence test.
Review follow-up (#67341): on a freshly initialized state.db (before
ANALYZE has run) the source-filtered branches of _get_tool_usage /
_get_skill_usage did not select idx_messages_assistant_calls_by_session
— the optimizer drove from idx_sessions_source_id and probed each
session's messages via idx_messages_session_active, scanning non
tool-call rows. Pin the index with INDEXED BY on all four fixed-predicate
branches so the plan is deterministic for both the unfiltered and
source-filtered scopes without depending on statistics.
Safe because the index is declared in SCHEMA_SQL (created by every
read-write SessionDB._init_schema) and every InsightsEngine caller opens
a read-write SessionDB; read-only attachments (which skip schema init)
are never used for insights.
Extract the four queries into class constants and add tests: query-plan
coverage for both scopes without ANALYZE, row-level equivalence between
pinned and un-pinned forms, and an assertion that INDEXED BY fails loudly
if the index is absent.
InsightsEngine._get_tool_usage and _get_skill_usage scan messages for
role='assistant' AND tool_calls IS NOT NULL, but no index aligns with
that predicate, so SQLite scans the full messages table on a large
state.db. Add a partial index over exactly those rows.
role and tool_calls are base columns in the messages table, so the index
lives in SCHEMA_SQL (created on both fresh and existing databases via the
executescript on every open) rather than DEFERRED_INDEX_SQL.
Adds schema regression coverage (fresh + reopened DB, plan uses the index)
and an Insights regression test proving tool/skill output is identical with
and without the index present.
Fixes#67341
Two places were using SELECT COUNT(*) when they only needed a boolean:
- has_any_sessions() called session_count() > 1 (full table scan)
- delete_session() used SELECT COUNT(*) WHERE id=? (full matching scan)
Fix:
- Add session_count_ge(n) to SessionDB — short-circuits via
SELECT 1 FROM sessions LIMIT n, returns bool
- has_any_sessions() uses session_count_ge(2) instead of session_count() > 1
- delete_session() uses SELECT 1 ... LIMIT 1 with fetchone() is None
- Add tests for session_count_ge
CI exposed the whole class: feishu tests across MANY files (thread
routing, text batching, sdk executor, ...) inject a mock _client and
skip connect(), so the deferred import leaves the request-builder
globals None. Replace the single-file setUpModule with a session-scoped
autouse conftest fixture that binds the globals once when lark_oapi is
installed; when it isn't, the affected tests already skip via their own
skipUnless guards. Full tests/gateway run: zero failures beyond main's
pre-existing baseline (sorted failure-diff).
Salvage of #57657, ported onto the plugin layout (the adapter moved
from gateway/platforms/feishu.py to plugins/platforms/feishu/adapter.py
since the PR's base). lark_oapi takes seconds to import and holds the
GIL doing it; the module-level import made every gateway boot pay that
cost even with Feishu unconfigured.
- _load_lark_oapi() with double-checked locking binds the SDK globals
on first use; connect() and _standalone_send() call it via
asyncio.to_thread so the loop never blocks on the import.
- probe_bot() also calls _load_lark_oapi() (sync context) so the SDK
probe path is preserved rather than silently degrading to the HTTP
fallback before a first connect.
- check_feishu_requirements() is install-only and no longer rebinds
globals; test_feishu.py gets a setUpModule that binds them eagerly
for tests that inject fake clients.
Includes the dedicated lazy-import test file (check-does-not-import,
connect-loads-on-worker-thread).
Re-derivation of #33612 by @LeonSGP43 onto the rewritten scheduler (the
original is 10,692 commits behind; its tick() no longer exists in that
shape, so this is a fresh minimal fix crediting the PR's idea).
The gateway's built-in ticker calls tick(verbose=False) every 60s. The
idle early-return was gated on 'verbose and not due_jobs', so idle
GATEWAY ticks fell through to load_config() + worker-pool resolution
every minute. Return early on ANY idle tick; keep the post-tick MCP
orphan sweep (main intentionally reaps orphaned stdio children on idle
ticks).
3 new tests; mutation-checked (restoring the verbose-gated guard fails
the config-skip test). 66 scheduler tests green.
Re-derivation of PR #39399 onto current main: pass --prefer-offline to
the web-UI workspace install (both silent and verbose arms of
_install_web_deps) and to the update-time Node dependency refresh in
_update_node_dependencies, so npm reuses its local cache instead of
re-fetching metadata. Test expectations updated to match, mirroring the
PR's own test-update commit.
Review follow-up on the #56798 salvage: the gate shipped fully dormant
(no provider profile sets supports_prompt_cache_key, no production
caller passes it, and no plain 'openai' profile exists to set it on) —
AGENTS.md rejects dead code wired in without E2E proof.
Activate the one endpoint where the field is first-class: exact-host
api.openai.com (OpenAI documents prompt_cache_key; GPT-5.6+ docs
recommend it for cache routing). Deliberately NOT substring matching —
Azure/OpenAI-compat endpoints may reject unknown fields and stay
opt-in via the flag. 4 new tests (imply + 3 spoof/proxy/Azure
negatives); mutation-checked (substring-weakened host check fails the
spoof tests).
The runtime footer (`/footer`) shows what model ran and how full the context
is, but not how long the turn took. On a messaging platform there is no
progress bar and no shell timer — a turn that took 4 seconds and one that took
four minutes produce visually identical replies. Users comparing models,
providers, or reasoning levels have no at-a-glance signal for the one
dimension they most often care about, and "was that slow or did I imagine it?"
is unanswerable after the fact.
Adds a `latency` field to the existing footer machinery, rendering the
wall-clock duration of the agent run: `<1s`, `22s`, `1m05s`.
`gateway/run.py` measures with `time.monotonic()` immediately around the
`self._run_agent(...)` await in `_handle_message_with_agent` — the same
function that already builds the footer, so the value is the user-perceived
turn duration (monotonic, so it is immune to wall-clock/NTP adjustment).
`latency` is deliberately NOT in `_DEFAULT_FIELDS`. It is opt-in via
`display.runtime_footer.fields`. Every existing footer — and every footer a
user has today without touching config — renders byte-identically.
This is enforced by tests, not just asserted:
- `test_latency_not_in_default_fields` pins the default tuple.
- `test_resolve_footer_config_default_fields_exclude_latency` pins what
config resolution produces for an untouched config.
- `test_default_footer_renders_byte_identically` pins five exact output
strings for default-config renders **while supplying `turn_seconds`** —
proving that even when the caller measures timing, a default-configured
footer does not show it.
- `test_default_build_footer_line_ignores_turn_seconds` asserts
`build_footer_line(...) == build_footer_line(..., turn_seconds=125.0)`
under default fields.
Adding `latency` to `_DEFAULT_FIELDS` fails 11 of these tests.
No new config surface (reuses `display.runtime_footer.fields`), no new env
vars, no new core tool, no new model-facing schema. One new module-private
helper (`_format_latency`), one new keyword argument threaded through the two
existing footer functions, and 3 lines in `gateway/run.py`.
`turn_seconds` defaults to `None` and the field is skipped when it is `None`
or negative, so any call site that does not measure timing keeps working
unchanged.
`tests/gateway/test_runtime_footer.py` (+185): `_format_latency` boundary
table (sub-second, rounding at 59.4/59.6, the `m{:02d}s` zero-pad, 60m), the
render/skip/opt-in matrix, field-order placement, `build_footer_line`
threading, and the byte-stability block above.
RED-proved by mutation — each of these breaks tests:
- `latency` added to `_DEFAULT_FIELDS` → 11 failures
- dropping the `turn_seconds is not None and >= 0` guard → 2 failures
- `{sec:02d}` → `{sec}` → 6 failures
- `build_footer_line` not threading `turn_seconds` → 1 failure
51 passed in `tests/gateway/test_runtime_footer.py`; 54 passed across the
footer blast radius. `ruff check` clean.
The existing test only mutated the miss-path return; a mutation to
'return _PROVIDER_LIST_CACHE' (aliasing the global cache) survived the
suite. One line pins the cached-return copy. Mutation-checked.
The retry loop gates on real time.time() < sleep_end; with sleep mocked
to a no-op the test hot-spun 7.5 wall-clock seconds. Advance a fake
clock by each sleep amount instead (pattern precedent:
test_session_activity_persist.py).
Empty content retries previously fired back-to-back with no delay,
wasting up to 3 rapid API calls, and could not be cancelled mid-wait.
Apply the same jittered_backoff() already used for rate-limit and
API-error retries, sleeping in small increments so a user interrupt
aborts the wait instead of blocking until it elapses.
Fixes#35230
Review fold on the #76341 salvage: the substring test ('gh' in
source.lower()) classified GH_TOKEN and GITHUB_TOKEN as gh_cli, so a
user's env-var-specific suppression was silently bypassed (and
suppressing gh_cli silently dropped env tokens). Pre-existing bug on
main, but the PR's early gate makes the classification decide whether
the exchange runs at all. Match resolve_copilot_token's exact
'gh auth token' sentinel instead.
Adds 3 regression tests: env-var suppression gates the exchange,
gh_cli suppression doesn't swallow env tokens, all-sources suppression
skips the resolve subprocess entirely. Also corrects the ~13s comment
(actual worst case ~35s: 3x10s timeouts + 4.5s backoff).
The copilot branch of _seed_from_singletons ran the suppression gate
_after get_copilot_api_token(), which retries the network exchange 3x
with backoff (~13s worst case). A source the user already suppressed
(hermes auth remove copilot gh_cli) still burned the full exchange dead
time on every pool load — model picker open, /model, agent startup —
only to have the entry discarded afterwards.
Move the _is_suppressed() gate ahead of the network call, matching the
early-gate pattern every other singleton branch uses. Suppressed copilot
sources now skip the exchange entirely. Measured: model.options payload
build drops from ~13s to ~0.2-0.4s for a user with copilot suppressed.
Add regression test test_load_pool_skips_exchange_for_suppressed_copilot
asserting the exchange is never invoked for a suppressed source.
build_local_transcribe_kwargs read stt.local.no_speech_prob_threshold /
stt.local.logprob_threshold only for Hermes' post-filter
(_is_hallucinated_segment). faster-whisper's model.transcribe() never
received them, so its internal defaults (no_speech_threshold=0.6,
log_prob_threshold=-1.0) always applied and silently dropped
low-confidence segments before they reached the post-filter — making
those config knobs dead for the first gate.
Non-English speech decodes at a lower avg_logprob, so the English-tuned
defaults discard whole utterances (empty transcript despite correct
capture and language detection). Map the same config values through to
model.transcribe() so both gates stay in sync and the knobs work.
Defaults are unchanged, so behavior is identical unless a user tunes them.
Fixes#74178
Five review findings folded:
- schema cache writes via utils.atomic_json_write (fsync; was bare
tmp+replace), file moved to cache/mcp_schema_cache.json with 0o600
(sibling precedent: registry discovery cache)
- phantom-tool reconciliation: after a lazy server's first-use connect,
cached tools the live server no longer offers are deregistered (were
permanent registry ghosts burning circuit-breaker strikes on every
'Unknown tool' round-trip); stale fingerprint logged
- cache-load path now runs _scan_mcp_description like the eager path
(cache file is user-writable JSON; defense-in-depth)
- write-through skips the disk rewrite when the entry is unchanged
(a flapping stdio server was rewriting byte-identical JSON per
revival)
- _lazy_server_fingerprints no longer write-only dead state (consumed
by the reconciliation logging)
444 mcp tests green (440 pre-fold + 4 new guards); phantom-dereg and
write-skip mutation-checked.
Wires the fingerprint-keyed schema cache (previous commit, @Vansh5632's
design from #56832) into the startup path, re-derived onto main's
current connect machinery:
- register_mcp_servers: servers with mcp_servers.<name>.lazy=true whose
config fingerprint matches a valid cache entry register tools from
cache WITHOUT spawning; miss/stale falls back to eager connect.
- First tool use routes through _ensure_lazy_server_connected, which
composes with the connect cooldown (#50394) and _server_connecting
dedup rather than duplicating the connect path.
- resource/prompt utility handlers (list_resources/get_prompt) also
connect-on-first-use — closes the gap flagged in the original
sweeper review.
- Write-through: a live connect refreshes the cache entry.
Config gate is per-server, default OFF, matching the
idle_timeout_seconds key pattern. 24 lazy/cache tests + 440 mcp-wide
green; mutation-checked (cache-read disabled -> registration test
fails; connect bypassed -> 3 first-use tests fail).
kshitijk4poor