Review follow-ups on the live-inode import:
- A `.db` member is now page-restored into the live file, but a paired
`-wal`/`-shm`/`-journal` member from an old or hand-built archive still went
through the rename publish — installing a foreign WAL beside the restored
database (and over a live sidecar's inode). Skip them; current backups never
ship them (_EXCLUDED_SUFFIXES), now shared as _SQLITE_SIDECAR_SUFFIXES.
- restore_quick_snapshot ignored _safe_restore_db's False and counted a
refused restore as success; honour it like run_import does.
- _safe_restore_db docstring described the pre-#90950 unconditional fallback.
`hermes import` published every zip member, including `state.db`, with
`_extract_member_atomically` — a rename that swaps the file's inode. Any
gateway, dashboard, or WebUI process holding the database open keeps its
descriptor on the now-unlinked inode: it goes on serving pre-import pages
and writing sessions no other process can see, while the sidecar WAL left
beside the new file describes the database that was just unlinked. Nothing
raises, so the import prints "Import complete" and the sessions are simply
absent from the database everyone opens next.
The live-safe path already exists: `/snapshot restore` has routed `.db`
files through `_safe_restore_db()` since #65942, writing snapshot pages
into the existing file so every open connection converges. `hermes import`
— the disaster-recovery path, reached by users who already lost something
once — never got that treatment.
Route `.db` members through it. A target that does not exist yet has no
holders and no inode worth preserving, so it keeps the ordinary atomic
publish. A refused or failed live-safe restore now raises, so the import
reports a skipped file instead of counting a silent success, and the
existing database is left untouched.
Importing an older backup over newer work stays allowed but no longer
silent: the summary reports the session/message counts the import replaced,
the same before/after evidence `restore_cron_jobs_if_emptied` uses for
`cron/jobs.json`.
Closes#100960
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012ShTVU941HYygvypY9JMYE
(cherry picked from commit 8ff260a312341cb85bdf7cdd570de5ff888c6013)
The gateway broadcast and turn-failure explanation printed a literal
~/.hermes/state.db; now that the line is a command the operator is meant to
run as-is, interpolate _default_db_path() so profile / HERMES_HOME installs are
pointed at the store that actually failed. Also: split a comment that a merge
fused onto the logger line in session_lost_and_found.py, and fix an inverted
test docstring.
Refs #100368. The forensics thread established that a sqlite3 CLI with
the WAL-reset opener bug (fixed 3.51.3+ / backports 3.50.7 / 3.44.6;
Debian/Ubuntu system shells 3.45.1/3.46.1 are in the vulnerable band)
unlinks the live -wal/-shm pair when pointed at a live state.db whose
writer's DMS lock has been cancelled, splitting the store into two
concurrent generations whose acknowledged writes vanish while both
report integrity_check ok. Hermes' own corruption banners instructed
exactly that command.
- gateway corruption broadcast, run_agent corrupt-cause explanation,
hermes_state repair-budget and forensic-backup refusals, and the
kanban manual-recovery hint now route operators to
`hermes sessions recover --source <db>` (which snapshots the damaged
bundle before any shell touches it) and warn against a raw sqlite3
shell on the live file
- find_sqlite3_cli() now refuses a WAL-reset-vulnerable shell for the
page-level salvage lane even on the snapshot, reusing the canonical
gate from hermes_cli.sqlite_runtime so the embedded runtime and the
salvage shell can never disagree
- find_sqlite3_cli_refusal() records why a shell was refused so the
lost_and_found lane can tell the operator exactly what to install
instead of a generic "not found"
- regression tests cover the version gate (vulnerable/fixed matrix, the
mirror check), every refusal reason, and each guidance site
Test plan:
- scripts/run_tests.sh tests/hermes_cli/test_sqlite3_cli_salvage_gate.py
tests/test_state_db_repair_loop_cap.py
tests/run_agent/test_corruption_recovery_guidance.py
tests/hermes_cli/test_session_recovery_lost_and_found.py
tests/hermes_cli/test_session_recovery.py tests/test_sqlite_wal_reset_gate.py
tests/hermes_cli/test_sqlite_runtime.py - 91 passed, 1 skipped locally
(cherry picked from commit e62940d1021e80e9b7d6423ced1cbdfe7dd0c37d)
- Register meta-ai as a live-first picker provider so the /v1/models catalog
leads the picker; new models appear without a PR
- Override fetch_models to exclude non-chat models (muse-image-*, muse-voice-*)
from the picker; new chat model families pass through automatically
- Slim fallback_models to a single safety-net entry (muse-spark-1.2), shown
only when the live fetch fails
- Make data-policy contributor warning model-generic (not hardcoded to 1.2)
so it covers any future -contributor model
- Update test assertion to match generic warning text
LOCAL ONLY — pre-launch, not for push.
The stampede fix added a `stale_access_token` hint to
resolve_nous_runtime_credentials() so a process whose bearer just 401'd
adopts a token a sibling already rotated instead of re-POSTing the shared
grant — but only the credential-pool caller passed it. The main agent's
401 path (run_agent._try_refresh_nous_client_credentials), the auxiliary
client rebuild, and the proxy adapter all called force_refresh=True with
no hint, so `_already_rotated_by_peer` could never fire: N subagents
hitting hourly expiry still issued N serialized refreshes, each one
invalidating the token a sibling had just adopted.
Live 12-process A/B against a fake Portal: 12 refresh POSTs / 9 distinct
final tokens before, 1 POST / 1 token after.
Each TUI instance spawned three stdio MCP server copies: one in the
CLI wrapper, one in tui_gateway.entry, one in the slash worker. The
wrapper's copy is dead weight — _launch_tui blocks in subprocess.call
until the TUI exits, so its registered MCP tools are never invoked,
yet the server process (35-85 MB) lives for the whole session.
Root cause: _is_tui_chat_launch() only detected --tui / HERMES_TUI=1,
so bare `hermes` with display.interface: tui fell through to
background MCP discovery in the wrapper while the TUI gateway
(spawned moments later) ran a second discovery.
Fix: _is_tui_chat_launch() now consults _resolve_use_tui() — the exact
TUI-vs-classic decision cmd_chat makes — for chat commands only
(command in {None, "chat"}), leaving mcp serve / gateway / acp / cron
discovery behavior untouched.
Verified: unit tests (RED->GREEN); E2E with a canary stdio MCP server
in a scratch HERMES_HOME counted 2 spawned copies pre-fix vs 1
post-fix (gateway's only), and the wrapper's RSS dropped ~43 MB.
Related: #71928 (same per-process duplication class), #11115 (lazy
non-core discovery).
Follow-up on the #101437 salvage (#101415). `_own_live_lease_ids()` snapshots
the server's session records under `_sessions_lock`, but the registry file
lock is taken afterwards, and a sibling session attaches its lease to its
record only after `try_acquire_active_session` has already written the entry.
A finalize racing that acquire would read the brand-new lease as an orphan
and drop it. Entries this process wrote inside a 30 s grace window are kept
regardless of the snapshot; real orphans are minutes old.
The plausibility gate excluded stub rows by a hard-coded title literal that
session_lost_and_found builds inline at two sites; rewording either would have
turned every stub into a mis-mapped row. One STUB_TITLE_PREFIX constant.
Follow-ups on the #101423 salvage (#101409):
- `stub_missing_parent_sessions` legitimately writes `started_at = 0.0` when
no timestamped message survived; a salvage where only stubs remain is
depleted, not mis-mapped. Stub rows leave the sessions denominator.
- Reuse `_EPOCH_LOW` from session_lost_and_found instead of a second
2001-epoch constant; collapse the two per-table blocks into one loop.
- Tests: a real upgraded-layout source (started_at physically appended) with
page 1 zeroed goes through the real `.recover` lane and the report comes
back `verified: False`; a stub-only output is not flagged; a mapped row
with a NULL title (the mis-mapped shape) still counts as mapped.
The lost_and_found salvage lane maps cells positionally onto the
destination template's declared column order, but a source upgraded
via ALTER TABLE has its columns in the order they were added, which
differs from SCHEMA_SQL whenever a column was inserted mid-definition
(#101409). Every row still inserts, so integrity/FK/FTS/count checks
stay green and the report ends up verified: true — while all 1,875
sessions in the reporter's DB got started_at = 0.0 with counters and
URLs shifted into the wrong columns.
Add a semantic plausibility gate to the salvage lane: when every
salvaged sessions.started_at or messages.timestamp is NULL or before
2001-09, the cells were mapped onto the wrong columns and the
recovery is reported with errors and healthy: false, so verified no
longer claims a mis-mapped output. A partially damaged column (torn
cells on some rows) does not trip the gate — only a systematic
violation does.
Fixing the positional mapping itself needs a historical-layouts
table derived from schema-version history; that design decision is
left to maintainers (suggested fix 2 in the issue).
Follow-ups on the #101586 salvage (#101356):
- `_is_same_auth_store` uses `Path.samefile` instead of a hand-rolled
st_dev/st_ino compare.
- The same-store check now runs after the mtime fingerprint short-circuit and
records the clean mark, so a symlinked-profile process pays the resolve +
stat pair once per file change instead of on every `load_pool()` call.
- A profile `.anthropic_oauth.json` aliased to root's singleton is one shared
grant too; the singleton block no longer self-compares and unlinks it.
The forked-grant heal treated a profile auth.json that is an *alias* of the
root store (symlink, hardlink, bind-mount) as a forked copy: it loaded the
same file as both the profile store and the root store, matched every OAuth
row against itself, stripped the profile rows / providers.<id> block, and
_save_auth_store() wrote that strip back through the alias — deleting the
shared credential (openai-codex reported).
#100339 / PR #100929 fixed *copied* stores; a shared store has no other side
to consolidate. Guard the heal with _is_same_auth_store(), which reuses the
resolving _same_path() (covers symlinks and equivalent paths) and falls back
to device+inode identity (covers hardlinks), and return None without writing.
Regression tests cover an openai-codex pool row plus providers block behind a
symlinked and a hardlinked profile auth.json; the copied-fork heals from
#100339 are unchanged.
Fixes#101356
The cherry-picked commit added the allowed_mcp_names filter to
discover_mcp_tools(). Since then CLI startup grew a second discovery path —
start_background_mcp_discovery / the deferred desktop start in
hermes_cli/mcp_startup.py — so wiring the filter only into the inline call
would leave `hermes chat -t terminal` (the default backgrounded path) still
spawning every server.
Store the filter once in mcp_startup (set_mcp_server_filter, called from
_prepare_agent_startup from args.toolsets; `all`/`*`/empty clears it) and
have both the inline and the background discovery honor it. The unfiltered
call shape is unchanged so zero-arg test stubs keep working.
Dropped from the original PR: the atexit/SIGINT/SIGTERM oneshot MCP reap —
main already does this in _cleanup_oneshot_runtime() -> shutdown_mcp_servers().
E2E (3 configured stdio servers, real subprocesses, 5 runs median):
no filter 3 spawned / 2.0 s; `-t terminal` 0 spawned / 1 ms.
Expose the read deadline as a top-level config.yaml key beside
context_file_max_chars (same load_config_readonly resolution shape), default
5s, documented in context-files.md. Narrow the reader thread's catch from
BaseException to Exception: control-flow exceptions can't originate inside
read_text on a worker thread, and re-raising one would bypass the sites'
except Exception / except (OSError, UnicodeDecodeError) handlers.
Findings from the efficiency review pass on the two salvages, applied as one
small follow-up:
- copilot_auth: check the negative cache BEFORE taking the per-fingerprint
exchange lock. During the 60 s post-failure window, dashboard polls now
raise immediately instead of parking an executor thread behind the
in-flight holder (up to ~50 s) to learn the same answer. Test hangs
without the check (timeout 124), passes with it.
- buzz _localize_inbound_media: download_path.read_bytes() was still
evaluated on the loop as the argument to the offloaded cache call — up to
the 128 MiB inbound cap. Read off the loop too.
- test_list_credential_pool_keeps_loop_responsive: 0.5 s block / 0.25 s
threshold (2x margin) so runner descheduling cannot false-fail it while a
real regression still trips it.
Follow-up to the off-loop move: once the credential-pool handlers run on
worker threads, the dashboard's periodic /api/credentials/pool polls can
overlap, and during a DNS outage each poll would have started its own
exchange and abandoned its own hung resolver thread.
- Per-fingerprint threading.Lock around the exchange: concurrent callers
wait on the one in-flight attempt, then hit the positive or negative
cache (bounded worker count, no duplicate network calls).
- _urlopen_bounded: when the hard cap fires and the abandoned worker later
succeeds, close the HTTPResponse instead of leaking the socket.
- Tests (none shipped with the original PR): hard cap + late-close,
single-flight success and failure paths, and the pool endpoint running
off-loop / keeping the loop responsive under a 200 ms blocking read.
Network off (unplugged) froze the backend 17 minutes: the async
/api/credentials/pool endpoints (GET/POST/DELETE) called load_pool()
synchronously on the event-loop thread -> Copilot token exchange ->
blocking urlopen -> getaddrinfo stuck in C for 1016s, immune to
urlopen(timeout=10). WS dropped (1006), sessions detached, even log
writes stalled.
Fix 1 (web_server.py): move the three endpoint bodies into
asyncio.to_thread, matching the file's existing _run pattern.
Fix 2 (copilot_auth.py): _urlopen_bounded() runs the request in a
daemon thread with a wall-clock hard cap (timeout+5s) so DNS hangs
can no longer block any caller indefinitely.
Measured: simulated DNS hang now raises TimeoutError after 6.0s
instead of freezing the loop.
The module already binds run_in_threadpool (used by list_profiles_endpoint)
and every sibling router uses the same starlette helper; the nine new
loop.run_in_executor(None, _run) sites now go through that alias so the
file has one offload idiom. Behaviour-identical (both hand the callable to
a worker thread).
Also sweeps the one endpoint the PR left synchronous:
update_profile_model_endpoint's _write_profile_model reads and rewrites
the profile's config.yaml on the event loop.
The remaining in-scope handlers in this router read and write profile
documents inline on the ASGI event loop:
- GET /api/profiles/{name}/soul reads SOUL.md
- PUT /api/profiles/{name}/soul atomic_write_text(SOUL.md)
- PUT /api/profiles/{name}/description write_profile_meta(profile.yaml)
- GET /api/profiles/{name}/desktop-overlay reads desktop.json
The persona save is the sharpest of the four: atomic_write_text() writes a
temp file, fsyncs it and replaces the original, so the loop is parked for
however long the filesystem takes to durably commit — unbounded on a slow
or contended disk, and paid on every Save in the editor.
Each handler keeps its existing status-code mapping. The reads probe and
load in a single executor hop rather than two, which also avoids widening
the gap between the existence check and the read.
Both readers return a _MISSING sentinel rather than None for an absent
file. desktop.json may legitimately contain the document `null`; collapsing
that onto None would newly report an existing-but-empty overlay as absent.
The same distinction is what the SOUL.md durability tests rely on, where
"file missing" and "file empty" must not both read as never-set.
_resolve_profile_dir() stays on the loop in all four, as it does in the
rest of this sweep: it is a name check plus one stat, and it owns the
400/404 responses.
Three more handlers in this router did filesystem work inline on the ASGI
event loop:
- PATCH /api/profiles/{name} calls rename_profile(), which stops a running
gateway through the same 10-second _stop_gateway_process() poll that
delete uses, then renames the profile directory, rewrites the Honcho
host blocks and regenerates the wrapper script.
- GET /api/profiles/active reads the active_profile state file and
resolves HERMES_HOME against the profiles root. The sidebar polls it.
- POST /api/profiles/active stats the target profile, creates the state
directory and writes active_profile via a temp file plus replace.
Rename carries the same worst case as delete and belongs off the loop for
the same reason. The two active-profile handlers are individually cheap,
but they are the routes the dashboard polls, so they are the ones most
likely to be queued behind something slower — and leaving them inline is
what made the router inconsistent with list_profiles_endpoint, which
already offloads a plain directory listing eight lines above.
The two reads in GET share one executor hop rather than taking one each.
POST /api/profiles/{name}/describe-auto called
profile_describer.describe_profile() inline. That function is a plain def;
it reaches agent.auxiliary_client.call_llm(), also a plain def, which makes
a synchronous provider request with a 60-second ceiling.
Held on the ASGI event loop that is six times the 10-second WebSocket
ready-probe threshold web_server.py records as the point where the desktop
app gives up (GH-73083). A single describe-auto on a slow or unreachable
auxiliary provider therefore takes the whole dashboard offline for up to a
minute, including the /api/ws and /api/pty sockets the desktop app and the
Chat tab run on.
Move the import and call into the default executor. _resolve_profile_dir()
deliberately stays on the loop ahead of the hop: it is a name validation
plus a single stat, and it owns the 400/404 responses that the handler's
`except Exception` would otherwise turn into a 500.
DELETE /api/profiles/{name} called profiles.delete_profile() inline on the
ASGI event loop. When the target profile has a gateway running, that call
stops it via _stop_gateway_process(), which polls the PID every 500 ms for
up to 10 s before escalating to a force kill, and then removes the profile
tree.
For the whole of that window the dashboard process serves nothing else.
web_server.py's own notes record what that costs: a stall of this length
"caus[ed] the Desktop's 10-second WebSocket ready-probe to time out
(GH-73083)", and both the desktop app and the dashboard's Chat tab drive the
agent over those WebSockets. Deleting a profile whose gateway is up is a
routine action that reliably reaches the full ten seconds — the handler's
own output announces "Gateway is running - it will be stopped".
Move the call into the default executor via run_in_executor, matching
list_profiles_endpoint, export_profile_endpoint and import_profile_endpoint
in this same module. The exception-to-status mapping is unchanged:
FileNotFoundError/ValueError are raised inside the worker and re-raised by
the await, so they still map to 404/400.
`handle_proxy` already offloads the two credential reads on the happy path,
but the rotation inside the `upstream_resp.status in {401, 429}` branch still
called `adapter.get_retry_credential` inline on the event loop.
That is the most expensive of the three blocking methods on the
`UpstreamAdapter` contract, not the cheapest:
* `NousPortalAdapter.get_retry_credential` routes into
`_get_credential(force_refresh=True)`, so the token-refresh POST that
`get_credential` performs only near expiry is unconditional here — and it
runs under the same `_auth_store_lock()`, a cross-process advisory lock
with a 15s timeout.
* `XAIGrokAdapter.get_retry_credential` loads the key pool off disk and
calls `try_refresh_current` / `mark_exhausted_and_rotate` under its lock.
So every upstream 401 or 429 froze the proxy's single event loop — and with
it every other in-flight streaming completion — for the whole rotation. A 429
is exactly when the proxy is busiest, which is the worst moment to stall.
Wrap it in `asyncio.to_thread`, matching the two sites above. The error
contract is unchanged: `to_thread` re-raises the worker's exception in the
awaiting frame, so the existing `except Exception -> retry_cred = None` still
swallows a failed rotation and streams the upstream's own rejection back.
`handle_health` called `adapter.is_authenticated()` inline from an
`async def`. `UpstreamAdapter.is_authenticated` is documented as
"Should be cheap — no network calls. Used by `proxy start` for a clear
up-front error before binding a port." (`adapters/base.py`), and that is
true of the `proxy start` preflight, which runs in a plain synchronous
CLI function. It is not true on the event loop:
`NousPortalAdapter.is_authenticated` goes through `_read_state()`, which
takes `_auth_store_lock()` — the same cross-process lock with a 15s
timeout as credential resolution — and `XAIGrokAdapter` reads its key
pool off disk.
`/health` is precisely what a supervisor, systemd unit, container
healthcheck or load balancer polls, on a fixed interval, so it is the
endpoint least able to afford a lock wait; and a wait here freezes every
concurrent proxied stream, not just the healthcheck.
Offload it with `asyncio.to_thread`. The response body is byte-identical;
only the scheduling changes.
`create_app` registers `handle_proxy` as an `async def`, and it called
`adapter.get_credential()` directly on the aiohttp event loop.
`UpstreamAdapter` is a synchronous contract (`adapters/base.py` — every
method is a plain `def`), and the shipped adapters implement it with
blocking I/O. `NousPortalAdapter.get_credential` takes
`_auth_store_lock()` — a cross-process advisory lock with
`AUTH_LOCK_TIMEOUT_SECONDS = 15.0` (`hermes_cli/auth.py:110`) — reads
`auth.json` off disk, and may issue a token-refresh POST; on a terminal
`AuthError` it takes that lock a second time to persist the quarantined
state. `XAIGrokAdapter.get_credential` reads its key pool off disk.
The proxy is one process with one loop, and `handle_proxy` streams with
`sock_read=300`, so long-lived completions are the normal case. Blocking
inside credential resolution therefore freezes *every* concurrent
in-flight stream mid-token for the duration — a concurrent `hermes auth`
command holding the auth-store lock is enough to do it. Every proxied
request goes through this path.
Dispatch through `asyncio.to_thread` instead. This is a pure scheduling
change: `to_thread` re-raises the worker's exception in the awaiting
frame, so the existing `except Exception` -> 401 `upstream_auth_failed`
mapping is unchanged, and the adapters' own `self._lock` still serialises
concurrent resolutions exactly as before. Fixing it at the handler also
leaves the synchronous `UpstreamAdapter` ABC untouched, so it covers
every adapter without conflicting with in-flight work that subclasses it.
GitHub answers anonymous fetches with HTTP 401 during outages (and for
renamed/private repos). git then prompts `Username for 'https://github.com':`
on the inherited terminal and `hermes update` sits there — users read it as
Hermes demanding a GitHub login.
Every network git call in the updater (fetch/pull/push, apply + --check +
fork sync) now runs with GIT_TERMINAL_PROMPT=0 / stdin=DEVNULL, so the 401
fails fast into the fetch-failure classifier, which now reports it as a
GitHub-side rejection (likely outage) rather than blaming the user's
credentials. Credential helpers/askpass are left configured so private-fork
origins still authenticate.
Live repro: PTY-attached update --check against a 401 origin hung 15s+ on
the prompt before; exits rc=1 in 0.2s with the diagnosis after.
Same class as #73751 (@Frowtek, pre-main.py decomposition); passive banner
half salvaged from #101421 (@RobbertC5).
Hermes gathers workspace context by running git against the session
directory automatically — the coding-workspace snapshot, gateway
project-tree build, /diff, @diff|@staged context refs, goal-gate
fingerprint, and -w startup worktree add — before any prompt, tool call,
approval, or trust gate. Those probes ran the system git without
stripping the repository's own config, so a repo delivered as files with
its .git directory intact (a shared zip, sync folder, or USB stick;
git clone never transfers .git/config) could set an execution-sink git
setting and get arbitrary host code execution as the user with nothing
on screen.
- core.fsmonitor / core.hooksPath / pager / editor / credential helper:
neutralized by routing every automatic probe through
noninteractive_git_env(), which pins those keys to inert values via
GIT_CONFIG_* and ignores global/system config. bounded_git_probe (the
reported sink, coding_context._git + tui_gateway.git_probe) now defaults
to that env; worktree-add, working_diff, web_git, context_references,
goals, and subagent_worktree route through it too.
- Attribute-scoped [diff "x"] command=/textconv= drivers: the attacker
names the driver in .gitattributes, so GIT_CONFIG_KEY overrides can't
enumerate them. Added harden_git_argv(), which inserts
--no-ext-diff --no-textconv on diff-rendering subcommands (diff/show/
log/blame) only — status et al reject the flags. Both flags required
(verified empirically; each alone leaves the other live).
Builds on the noninteractive_git_env config-scrubbing from the
gemini-cli #28792 port. Real-git E2E regression suite arms a malicious
repo and asserts every automatic path neutralizes fsmonitor, hooks,
external-diff, and textconv; a baseline test proves the repo is armed.
Addresses review feedback on #84928.
The tick interval was exposed as HERMES_NOUS_KEEPALIVE_INTERVAL_SECONDS.
AGENTS.md reserves .env for credentials and puts behavioural thresholds in
config.yaml, so the knob moves to `nous.keepalive_interval_seconds`,
following the existing `vertex:` section's precedent for non-secret
provider settings. The env var is dropped rather than bridged: it was never
released, so nothing depends on it. Adding a key to a new section is handled
by the deep-merge, so no _config_version bump is required.
test_keepalive_interval_fits_inside_the_token_lifetime asserted
`900 < 899 * 4 - 120`, which is true for any realistic interval and could
never fail. It also tested the wrong value: the configured constant is only
a ceiling, while the schedule that ships is the derived tick. Replaced with
an assertion over the derived tick for each observed lifetime, which does
fail if the derivation constants regress -- verified against both
TICKS_PER_LIFETIME=1 and MIN_INTERVAL_SECONDS=5000.
Also adds coverage for an unreadable config.yaml, which must fall back to
the module default rather than take the keepalive thread down.
pytest tests/hermes_cli/test_nous_auth_keepalive.py -> 9 passed
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Follow-up to the interval fix: ticking faster narrowed the gap but did not
close it, and the hardcoded interval was wrong for short-lifetime accounts.
Two independent problems, both needed:
1. Lifetime is not a constant. Installs have been observed issuing ~3594s
and ~899s (see #35752, which reports expires_in: 899 while this account
reports 3594). Any hardcoded interval is right for one and wrong for the
other. The tick now derives from the lifetime the server actually issued,
capped by the configured interval and floored at 60s. The access token and
the invoke agent key carry separate lifetimes, so the shorter one governs.
2. Ticking faster alone never closes the gap. The refresh only fires once a
credential is within ACCESS_TOKEN_REFRESH_SKEW_SECONDS (120s) of expiry,
so a tick spaced wider than that window steps straight over it. With a 900s
tick against a 3594s lifetime the last tick before expiry still saw 894s
remaining, declined to refresh, and the credential died 6s before the next
one. The keepalive now asks "will this outlive my next tick?" (tick + skew)
rather than reusing the request path's bare skew.
Measured against both observed lifetimes:
lifetime 3594s: was never refreshed proactively; now refreshes 900s early
lifetime 899s: was never refreshed proactively; now refreshes 227s early
The lifetime is re-read every pass rather than cached, since it can change
when the account, plan, or server-side policy does -- exactly the case the
keepalive exists to cover.
min_access_ttl_seconds is threaded through as an optional parameter, so the
request path keeps its existing 120s behaviour and only the keepalive widens
the window.
Nous Portal access tokens carry a one-hour lifetime, and the keepalive only
refreshes once a token is within ACCESS_TOKEN_REFRESH_SKEW_SECONDS (120s) of
expiry. The tick interval was 6 hours, so it could only land inside that
2-minute window by coincidence. In practice every hour rolled over untouched
and the next inference call paid a 401 plus a re-auth round trip.
Observed on a production install: 71 "refreshed Nous runtime credentials
after 401, retrying" events across current logs.
Drop the default interval to 15 minutes, which gives four ticks per token
lifetime and leaves ample margin under the TTL-minus-skew ceiling of 3480s.
Add HERMES_NOUS_KEEPALIVE_INTERVAL_SECONDS so the interval is tunable without
a source edit, matching the existing HERMES_NOUS_TIMEOUT_SECONDS convention.
Zero still disables the thread.
Callers pass no interval, so the default is what actually shipped; the
signature now resolves at call time rather than binding the constant at
import.
An external-process provider is an agent CLI Hermes drives over stdio rather
than an HTTP endpoint. Three things about it were spelled out for one vendor,
and each was a hard stop for any other:
* ``resolve_provider()`` gates on ``PROVIDER_REGISTRY``. Its auto-extend from
``providers/`` covered api-key providers only, so an external-process profile
never entered it and ``hermes -m <that provider>`` died with "Unknown
provider" before a client was ever built.
* ``resolve_runtime_provider()`` keyed the external-process branch on the
literal ``"copilot-acp"``, so anything else silently fell through to the
OpenRouter default instead of its own runtime.
* ``resolve_external_process_provider_credentials()`` hardcoded the binary
(``copilot``), the argv (``--acp --stdio``), the env var names and the
placeholder api_key — so a third-party provider would have been handed
another vendor's CLI.
Now the profile carries what only the provider knows — ``process_command``,
``process_args``, ``process_command_env_vars``, ``process_args_env_var`` — and
the three core paths key on ``auth_type == "external_process"`` instead of a
name. copilot-acp's values move into its profile verbatim, so
``HERMES_COPILOT_ACP_COMMAND`` / ``COPILOT_CLI_PATH`` /
``HERMES_COPILOT_ACP_ARGS`` and its ``copilot-acp`` api_key placeholder behave
exactly as before; the new tests assert that alongside the out-of-tree case at
every step.
The error for a missing binary now names the provider and its own env override
instead of telling every user to install GitHub Copilot CLI.
Co-Authored-By: Junie <junie@jetbrains.com>
Slots above gemini-3.7-flash (kept) in OPENROUTER_MODELS and
_PROVIDER_MODELS["nous"]; openrouter plugin fallback_models bumped
3.7 -> 3.8; model-catalog.json regenerated.
Verified live with test completions on both Nous Portal and OpenRouter
(model echo + billed). Same 1,048,576 window / 65,536 output / pricing
as 3.7-flash, so provider-agnostic metadata resolves via the existing
gemini entries and both routes bill live (official_models_api) — no
pricing snapshot needed.
Scoped to the two named providers: vertex/gemini/kilocode/gmi curated
lists, setup.py samples, and aux defaults untouched.
N processes sharing one Nous OAuth pool entry hit the hourly expiry
together; each force-refreshed, each rotation invalidated the token a
sibling had just adopted, and processes that lost the auth-store flock
race had their only entry benched ("matched no nous entry ... pool size
0") — ~120 sessions surfaced 401 'out of funds' on Sep 2 2026.
- resolve_nous_runtime_credentials(stale_access_token=): under the store
lock, skip the refresh POST when the on-disk token differs from the one
that failed and is usable (a peer already rotated) — adopt instead.
- credential_pool nous path: adopt a peer-rotated key after the pre-sync,
pass the failed bearer through, and treat a lock TimeoutError as
'retry later', never as an exhausted credential.
- Live 120-process stampede harness: 41 refreshes/9 unrecovered -> 1
refresh/0 unrecovered.
Three gaps between the copilot-acp picker row and what the user's
subscription actually serves (reported: picker showed the stale curated
list while the Copilot CLI offered Sonnet 5 / Opus 5 / GPT-5.6):
1. _resolve_copilot_catalog_api_key() never looked at the Copilot CLI's
own token store (~/.copilot/config.json copilotTokens). A user whose
only credential is 'copilot login' got no catalog key, the live fetch
401'd, and copilot-acp silently fell back to the stale curated list.
Add it as resolution source 3, JSONC-tolerant, with each candidate
validated and exchanged like pool entries.
2. The existing credential-pool branch unpacked exchange_copilot_token()
into two names, but it returns (api_token, expires_at, base_url) —
the ValueError was swallowed by the enclosing except, disabling that
entire resolution path. Latent since the base_url return was added.
3. GitHub now returns model_picker_enabled: false for EVERY model on
some accounts/token types, so honoring the flag rejected the whole
live catalog. Treat the flag as a display hint: when it empties the
result, refilter without it (chat/endpoint checks still exclude
embeddings and non-chat rows).
Verified live: catalog resolves 44 models for a copilot-login-only
account, matching the CLI's own picker (claude-sonnet-5, claude-opus-5,
gpt-5.6-sol/terra, gemini, kimi).
Two follow-up gaps found by actually running 'copilot login' end-to-end:
1. The CLI (without an OS keychain) stores its token in
~/.copilot/config.json under copilotTokens — a JSONC file with
//-comment header lines. Add it as an auth-evidence source in
_external_process_auth_evidence(), parsed comment-tolerantly and
counting only a non-empty copilotTokens map (config.json exists after
first launch even when logged out).
2. The desktop chat picker requests explicit_only rows, and
_filter_explicit_provider_rows() dropped copilot-acp because a CLI
login leaves no trace in active_provider, model.provider, or env vars
— exactly the Anthropic-OAuth carve-out case. Keep external_process
rows when their CLI credentials are verified (auth_verified), while
still dropping ambient executable-on-PATH-only rows so the filter's
narrower contract holds.
Net effect: after 'copilot login', copilot-acp appears in the desktop
picker and the Accounts card reads signed in; a machine with only the
binary installed keeps today's hidden-until-configured behavior.
The Accounts-tab card told users to run 'copilot /login', which is not a
valid invocation — slash-commands only exist inside an interactive session.
Use 'copilot login', the CLI's device-code login subcommand.
The card's status_fn also hardcoded logged_in: False with a static label.
Wire it to get_external_process_provider_status(): claim logged_in only on
positive credential evidence (auth_verified), show which executable Hermes
resolved when merely configured, and say so when the CLI is missing from
PATH entirely.
The rendered cli_command now substitutes the executable the user actually
configured (HERMES_COPILOT_ACP_COMMAND / COPILOT_CLI_PATH) so a custom
binary path gets a copy-pasteable command that matches what Hermes spawns.
get_auth_status() special-cased the literal slug 'copilot-acp'; any other
external_process provider (the pending kiro/devin/junie ACP backends) fell
through to {'logged_in': False}. Dispatch on
PROVIDER_REGISTRY[target].auth_type == 'external_process' instead so the
whole class gets a real status.
get_external_process_provider_status() equated 'logged_in' with 'the
executable resolves', which says nothing about whether the Copilot CLI is
actually signed in. Add auth_verified/auth_source: positive-only evidence
from supported env tokens (validated via copilot_auth, classic ghp_* PATs
excluded) or known on-disk GitHub Copilot credential stores. No evidence
means unknown — never presented as signed out, because the CLI may keep its
session in an OS keychain. Deliberately subprocess-free to avoid re-creating
the gh-auth-token cold-start stall (#60800).
The overlay loop in list_authenticated_providers() checks every way a
provider might be authenticated — env keys, the auth store, the
credential pool, even Claude Code's external token files — but never
asks the one question that matters for an external_process provider:
does the executable resolve? copilot-acp has no key or token by design
(the spawned `copilot --acp --stdio` brings its own auth), so has_creds
stayed False and the filter dropped it from every picker. Funny enough,
five lines further down the same loop has a dedicated copilot-acp
branch for fetching its model ids — it just never got a chance to run.
Availability now comes from get_auth_status(), the same source
`hermes model` and the auth status endpoints already use, so the CLI
and GUI agree on what 'configured' means for external-process
providers.
Fixes#63662
Follow-up to the failure_deliver salvage (#100375):
- _preflight_check_delivery also checks the failure lane, so a typo'd
failure_deliver platform blocks at config-validation time instead of
surfacing only when a failure occurs — exactly when the notice must
not be lost. Duplicate lanes are checked once.
- The dashboard cron-update normalizer treats failure_deliver like
deliver (text normalization; empty clears the optional override
instead of coalescing), closing the one update path that could write
an unnormalized value into jobs.json.
4 guard tests; both fixes mutation-checked (neutralize -> red, restore -> green).
Coatue FR (Frank Long): jobs delivering into shared channels publish
engine failure notices ('⚠️ Cron X failed…') to those channels with no
opt-out. Adds an optional per-job failure_deliver field sharing
deliver's grammar: on failure, targets resolve from failure_deliver
when set (local = structural silence; state still recorded in
last_status/last_error/run history). Success delivery is unchanged;
absent field = today's behavior byte-for-byte.
Honored by every failure-category engine notice: the run_job failure
summary (+streak nudge), the escaped-failure retry path, drift-skip and
blocked-config alerts (composed into the same delivery), and the
gateway-shutdown interrupted-run notice (_notify_interrupted_cron_jobs).
Surfaces: cronjob tool create/update (same bot-chat validation as
deliver; '' clears on update), hermes cron create/edit
--failure-deliver, docs tip in automate-with-cron.
Existing fake_deliver test doubles gained **kwargs for the new
for_failure keyword — signature-compat only, no behavior change.
Andrew Wikel