cron/scheduler.py (8535 -> 6353):
- _deliver_result split into per-target helpers: _resolve_target_transport (live/relay/standalone
transport + enablement), _deliver_via_live_adapter (_live_route_metadata for Telegram DM-topic
vs forum routing, _live_send_text with the cancel()-based timeout disambiguation,
_live_send_media, _seed_live_delivery_sessions), _standalone_send/_deliver_standalone. The
three interpreter-shutdown skip branches and the repeated log+append+continue pattern collapse
into one _note_target_error / one shutdown message; _TargetDelivery carries per-target state.
- Thread/channel session seeding unified into _seed_cron_session (was two near-identical
functions).
- run_job decomposed into _run_no_agent_job, _apply_monitor_gate, _load_cron_job_config
(_CronJobConfig), _resolve_job_runtime, _check_model_drift, _open_cron_session_db,
_run_agent_with_watchdog, _finalize_cron_session, plus one _run_doc_header and one _audit
closure for the success/failure paths.
- _run_one_job_body: ownership-lost bookkeeping, delivery composition and outcome classification
extracted; tick: _acquire_tick_lock/_release_tick_lock, _maybe_reap_dead_owners,
_sweep_stale_inflight_for_tick, _process_due_job, _submit_with_guard.
- _build_job_prompt: context_from injection and skill loading extracted; one
_prepend_context_block for the four fenced-data blocks.
- One _start_heartbeat_thread for the script- and fire-claim heartbeat threads.
- Dropped unreachable return in SharedRouteAdapters.get; boolean-return and nested-if shapes
collapsed.
- Comments/docstrings compacted by hand, rationale kept (fd-leak reason for the late SessionDB
close callback, title-persistence rules, no_agent classification gate, inactivity-vs-provider
timeout ordering, stale-claim force-release, interruption token keying).
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.
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.
Two assertions per offloaded site:
- a loop probe, where the stubbed callee records whether an event loop is
running in its own thread — the idiom already used by
tests/hermes_cli/test_cron_dashboard_off_loop.py; and
- a concurrency proof, where the stubbed callee blocks on a threading.Event
while an unrelated request is timed. On the unfixed handlers that request
waits out the whole block; served off the loop it returns in
milliseconds.
The concurrency proof needs a single event loop across requests, so the
client fixture enters the TestClient context manager: that pins one
blocking portal for the whole fixture, where a bare TestClient(app) would
spin up a fresh loop per request and pass even unfixed.
Also covers the status-code mapping through the executor hop (404 on a
missing profile, 400 on a rename collision, 404 from the resolve that stays
on the loop ahead of describe-auto) and the _MISSING sentinel cases: a
desktop.json holding `null` still reports exists=true, an absent one
reports exists=false, and an empty SOUL.md is still distinguishable from a
missing one.
The client fixtures read web_server._SESSION_TOKEN from the module rather
than pinning a literal. web_server resolves that token once at import, so
whichever test file imports it first fixes the value for the session and a
later monkeypatch.setenv is silently ignored — two files hardcoding
different tokens would 401 depending on collection order.
Extends the off-loop suite to the third and last blocking method on the
`UpstreamAdapter` contract, the 401/429 rotation.
As with the two existing pairs, the primary assertion is **thread identity**,
not latency: a latency assertion measured by an HTTP client on the blocked
loop is vacuous, because the client's own timer cannot advance until the
block ends and it therefore reports a fast response on provably frozen code.
* `test_get_retry_credential_runs_off_the_event_loop` records
`threading.get_ident()` inside the fake adapter and compares it to the
loop thread, and checks the rotation still works end to end (rejected
bearer forwarded first, rotated bearer second).
* `test_event_loop_keeps_running_while_the_retry_credential_resolves`
samples a loop-side heartbeat counter from inside the stalled adapter. On
the unfixed handler it records exactly 0 loop iterations across a 0.5s
rotation.
* `test_retry_credential_failure_still_returns_the_upstream_rejection`
guards the error contract the change must leave alone: a raising rotation
is still swallowed and the upstream's own 401 is streamed back, with no
second forward.
A new `_build_rejecting_upstream` harness drives the `status in {401, 429}`
branch by rejecting every bearer except the rotated one.
Extends `test_proxy_off_loop.py` with the `/health` half, using the same
two-assertion shape as the credential tests:
- `test_is_authenticated_runs_off_the_event_loop` compares the thread the
adapter's `is_authenticated` ran on against the loop thread. Before the
fix they are the same ident.
- `test_event_loop_keeps_running_while_health_resolves_auth_state` reads a
loop-side heartbeat counter sampled by the adapter across its own stall.
Before the fix exactly 0 iterations run across 0.5s.
Both also assert the response is unchanged (`200`, `authenticated: true`),
so the offload cannot quietly alter what `/health` reports.
Adds `tests/hermes_cli/test_proxy_off_loop.py`, mirroring the harness in
`test_proxy.py`: the proxy and a fake upstream run as real aiohttp
servers on ephemeral ports under a single `asyncio.run`, guarded by
`pytest.importorskip("aiohttp")` — no pytest-aiohttp dependency.
The primary assertion is thread identity, not latency. A latency
assertion measured with an HTTP client on the blocked loop is vacuous:
the client's own timer cannot advance until the block ends, so it reports
a fast response on code that was provably frozen.
- `test_get_credential_runs_off_the_event_loop` records
`threading.get_ident()` inside the adapter and compares it to the loop
thread. Before the fix both are the same ident.
- `test_event_loop_keeps_running_while_credentials_resolve` runs a
heartbeat task on the loop and has the adapter sample its counter on
entry and exit, so the reading is taken from the loop rather than
through a client that shares it. Before the fix exactly 0 iterations
run across a 0.5s stall; after it, ~50.
- `test_credential_failure_still_maps_to_401` pins the error contract
across the change of call form. It is deliberately not in the
red-before set — it guards behaviour the fix must leave alone.
Submodule and worktree checkouts store .git as a file (gitdir: pointer);
the carry-over only looked at directory names, so that form was still lost
on rollback. Handle files with the same guard. Docstring now states the
deliberate limit: an excluded entry whose skill dir the target snapshot
lacks is dropped with staging (no orphan .git) and is not undoable via the
safety snapshot, which excludes these paths as well.
Excluding nested .git from snapshots has a side effect on rollback: the
staging move takes the whole live skill dir (including its .git) into
.rollback-staging-*, the extract restores the snapshot without it, and the
staging dir is then deleted — so a skill that is itself a git checkout lost
its .git on any rollback. Reproduced: main preserves it, the exclusion-only
branch did not.
After a successful extract, move excluded subtrees from the staged copy back
under their restored skill dir (mirroring how a top-level .git survives by
never being staged). Regression test included.
_safe_skills_path() is the sibling of iter_skills_files(): when a symlink in
skills/ forces a sanitized copy for mount-based backends (Docker/Singularity),
it rglob-copied the whole tree — .hub, .curator_backups, node_modules and all.
Prune EXCLUDED_SKILL_DIRS before descending, same rule as the sync generator,
so the mounted copy never carries (or walks) the bookkeeping trees either.
iter_skills_files() walked the skills tree with a bare rglob("*"), so the
.hub download cache, .archive, curator backups, and any node_modules/.git
under a skill package were uploaded to the sandbox on every sync. The
sandbox never reads them: skill content is resolved host-side.
EXCLUDED_SKILL_DIRS is already the canonical exclusion set, honoured by
discovery and backup. Apply it to the sync path too, across all three
roots iter_skills_files() walks (local, external, project-local), and add
.curator_backups to the set.
Measured on a local install: 900 files / 67.3 MB -> 771 files / 8.4 MB.
This is not just wasted bandwidth on the SSH backend, where the oversized
payload can exceed the 120s _ssh_bulk_upload deadline and surface as the
agent hanging on every tool call.
The filter intentionally does not reuse is_excluded_skill_path(), which
also prunes references/, templates/, assets/ and scripts/ -- those hold
support files and bundled scripts the sandbox does read and execute.
f50b5bb0fa taught the sync retry site to keep the cheap same-provider retry
when a Codex stream dies inside the 60s no-progress window (zero output),
skipping straight to fallback only on a stall or hard-ceiling timeout. The
async site never got that carve-out, so after widening the skip to vision
(#97572) an async vision call on a stillborn stream would have jumped to
fallback where the sync path retries. Both sites now apply the same rule.
Adds the async twin of the vision-skip test and a no-progress-still-retries
guard for the async site.
Issue #54465 established that a same-provider retry after a full-budget
timeout costs a second whole `timeout` window before the fallback chain is
reached, doubling the user-visible stall, and that compression must not pay
it because it sits on a critical path. The guard added for that is spelled
`task == "compression"`, so vision — which sits on the interactive path —
still retries.
The cost is the same and the stall is more visible: the turn holding the
image cannot answer, and because turns are serialised the following user
messages queue behind it. Two sequential full-budget timeouts on an
unhealthy vision provider is a long stall for something the fallback chain
could have served immediately.
Replaces the string comparison at both retry sites (sync `call_llm` and
`async_call_llm`) with `_TIMEOUT_NO_RETRY_TASKS = {"compression", "vision"}`,
so the two paths cannot drift again. Behaviour is unchanged for every other
task: fast blips (a streaming-close or a 5xx) still retry, and only
full-budget timeouts on those two tasks skip straight to fallback.
Tests: vision now falls straight through to fallback with the primary tried
exactly once, and a non-critical task still gets its one same-provider
retry, so the change stays scoped. Reverting the source change fails the
vision test and leaves the scoping test green.
Not the same as #51513, which fixes five separate defects in the vision
fallback chain (capability detection, sync/async client misuse, geo-block
and RemoteProtocolError classification, and chain iteration). This is about
what happens before that chain is reached.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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).
A goal quality gate is a shell command persisted by `/goal gate add` and
later executed with `subprocess.run(shell=True)` at every goal turn
boundary (run_gate), with no approval prompt. On the messaging gateway,
slash access is backward-compatible: with no `allow_admin_from` list
configured (the default), every *allowed* chat user is treated as
unrestricted. So an allowed but non-admin remote sender could add an
arbitrary shell command and get authenticated RCE as the Hermes process
account.
Gate ONLY the shell-creating operation (`gate add`) behind the existing
fail-closed explicit-admin check (`_resume_caller_is_admin`, the same one
that guards cross-origin `/resume`). `gate list` / `remove` / `clear`
stay open so a non-admin can still inspect and recover. CLI/TUI/Desktop
`gate add` is local (the user already has host access) and is unchanged.
Salvaged from #91677 by @unsupportedpastels — narrowed to the single
shell-creating op and reusing the existing admin helper rather than
renaming it. Contributor's regression tests preserved.
Co-authored-by: unsupportedpastels <theoldwizard123@pm.me>
On the default Nous config, call_llm acquires the auxiliary client via
_get_cached_client(resolved_model=None), so the cache key's model
element is "". On a 401, _refresh_nous_auxiliary_client rebuilt the
client but keyed the new entry on the resolved wire model (final_model,
e.g. "Hermes-4-405B"). The fresh client therefore landed under a
different key than the lookup, and the stale expired-credential client
under "" was never overwritten: every auxiliary call kept hitting the
dead client, 401ing and forcing a credential portal round-trip on each
request instead of self-healing after the first refresh.
The auto-provider dimensions had the same divergence: call_llm and
async_call_llm dropped task at both acquisition sites, and the async
path additionally dropped main_runtime at acquisition and at both of
its refresh sites, so the refreshed client shadowed the stale one under
a divergent (provider, task, model, runtime) key.
Pass the original lookup model (which may be None) into the refresh as
a separate lookup_model argument used only to build the cache key,
while the resolved model is still stored as the entry's usable model
and returned to the caller. Thread task into both acquisition sites and
main_runtime into the async acquisition and both async refresh sites,
so sync and async compute the same cache key on acquire and on refresh.
The stale client is now overwritten in place instead of lingering under
an orphaned key, preserving the per-model cache keying introduced in
on the default config.
Add end-to-end regression tests that drive the real call_llm and
async_call_llm through the real client cache; the existing 401 tests
patch _get_cached_client wholesale and so cannot observe
acquire/refresh key divergence.
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.
Interleaved subagent fan-outs were tagged with the first 4 hex chars of the
delegation id ([b2ac 3/9]), which is attributable but unreadable. Batches are
now numbered in order of appearance per process: [set 1 · 3/9], [set 2 · 1/7].
Desktop /agents already labels groups "Delegation N", so its duplicate hex
badge is dropped.
Assert the invariant — every display read of one session returns the same
transcript — plus the two things that must not grow with it: the model-fed
projection stays compressed, and Undo/Rewind rows stay hidden. Six of these
fail on the unfixed read paths.
Cuts the 41 contributor tests down to 8 pinning the before/after contracts
(out-of-tree provider resolves end to end, copilot-acp unchanged, broken
plugin falls through, flat-install discovery + non-provider kinds untouched).
Adds the create_client hook and process_* fields to the model-provider
plugin developer guide.
`hermes plugins install owner/repo` — and the plugin index behind
`hermes plugins search` — clones into `$HERMES_HOME/plugins/<name>/`, flat, one
directory per plugin. Provider discovery only ever scanned
`$HERMES_HOME/plugins/model-providers/<name>/`.
Nothing joined the two. `PluginManager` does not close the gap either: it
classifies `kind: model-provider` and deliberately skips importing it, because
provider lifecycle is owned by `providers/__init__.py` — which was not looking
in the directory the installer writes to.
So the documented install path half-worked. The CLI reported success, wrote its
install metadata, and the provider silently did not exist: `hermes -m <it>` said
"Unknown provider" and `/model` never listed it. Verified before the fix — a
plugin at `~/.hermes/plugins/<name>/` was NOT FOUND while the identical plugin
at `~/.hermes/plugins/model-providers/<name>/` was discovered.
Discovery now also walks the flat directory, importing only entries whose
manifest declares `kind: model-provider`. Everything else there belongs to
`PluginManager`, which owns its lifecycle and consent flow — importing it here
would run third-party code behind its back, so the tests assert we don't (with
fixtures that register on import, since a fixture that merely raised would be
swallowed by `_import_plugin_dir` and prove nothing).
Manifests are parsed with PyYAML when present and a line scan otherwise, so
provider discovery gains no hard dependency; an unreadable manifest is skipped
rather than allowed to blank the registry.
Co-Authored-By: Junie <junie@jetbrains.com>
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>
``create_openai_client`` was a hardcoded if-ladder: copilot-acp builds an ACP
stdio shim, gemini builds a native client, everything else gets an
``openai.OpenAI``. There was no extension point, so a provider whose wire
protocol is not OpenAI-over-HTTP could only be added by editing this function —
which is exactly why an ACP provider cannot ship outside this tree today, even
though ``providers/__init__.py`` has discovered out-of-tree profiles from
``~/.hermes/plugins/model-providers/`` and pip entry points for a while.
``ProviderProfile.create_client(**client_kwargs)`` closes that gap. It returns
``None`` by default, so every provider that wants the standard client is
unaffected and the existing ladder still runs as the fallback. copilot-acp is
migrated onto it — its hardcoded branch is gone and its profile supplies the
client in three lines, which is the same three lines an external package writes.
Resolution goes by provider name first, then by ``base_url`` prefix, so a
runtime configured only by URL still reaches its profile — matching what the
replaced ``startswith("acp://copilot")`` branch did. A profile that raises is
logged and skipped: a third-party plugin can fail to provide a client, but it
cannot take the turn down.
Also replaces the two ``isinstance`` checks in ``agent/auxiliary_client.py``
that mean "this client is complete, do not wrap it" with capability flags the
client class declares — ``HERMES_SKIP_TRANSPORT_WRAP`` and
``HERMES_SKIP_ASYNC_WRAP``, mirroring ``SUPPORTS_HERMES_TOOL_CALLS`` in
``background_review.py``. Two in-tree consumers (the ACP shim and the Gemini
native client), an out-of-tree client is covered by the same declaration, and
the hot path no longer imports those modules just to type-test.
Co-Authored-By: Junie <junie@jetbrains.com>
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.
Use the ACP v1 session config contract advertised by session/new: locate the category=model option and apply the selected value through session/set_config_option. Retain session/set_model only as compatibility fallback for pre-configOptions agents. Reject unknown and policy-disabled values before prompting.
Verified against the installed Copilot ACP server: its model config option advertises the account-authorized choices, session/set_config_option returns the updated state, and live prompts route gpt-5.6-terra to Terra and claude-sonnet-5 to Sonnet 5.
Follow-up to the session/set_model wiring, caught in live use: picking an
org-policy-disabled model (claude-fable-5) produced a response claiming to
BE that model while Copilot actually served its default (Claude Sonnet 5).
Two causes:
1. The prompt preamble injected 'Hermes requested model hint: <id>', so
whatever model actually served the session parroted the requested name
back as its identity. Remove the line entirely — the model is applied
for real via session/set_model now, and identity must come from the
backend, not prompt suggestion.
2. session/new advertises policy-disabled ids alongside enabled ones
(_meta.copilotEnablement: 'disabled'); selecting one is accepted but
silently serves the default. Exclude disabled ids from the offered set
so the degrade-with-warning path handles them.
Verified live: requesting claude-fable-5 logs the does-not-offer warning
listing the 23 genuinely enabled models, serves the default, and the
response truthfully self-identifies as Claude Sonnet 5.
Selecting a model on the copilot-acp provider had no effect: the model id
never left Hermes. _create_chat_completion() dropped the model argument
before _run_prompt(), so the selection survived only as prompt text
('Hermes requested model hint: ...') and Copilot answered with its own
session default — a user picking gpt-5.6-terra visibly got Claude Sonnet 5.
Live-probing 'copilot --acp --stdio' shows the CLI validates but IGNORES
its --model spawn flag in ACP mode, while session/new advertises
models.availableModels and the ACP-native session/set_model call actually
switches the session. Wire that in: forward the model into _run_prompt,
and after session/new send session/set_model when the id is advertised
(or the server reports no list). Unknown ids degrade to the session
default with a warning instead of failing the turn; the provider-level
virtual slug 'copilot-acp' is never forwarded.
Verified live against the real CLI: requesting gpt-5.6-terra answers as
GPT-5.6 Terra and claude-sonnet-5 answers as Claude Sonnet 5.
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.
Pin the fix class from the previous commits: auth_type-based dispatch in
get_auth_status(), positive-only auth_verified semantics (supported env
token yes, classic ghp_* PAT no, populated hosts.json yes, empty store no),
and the Accounts-tab cli_command (valid 'copilot login' default, configured
executable substitution, non-external providers untouched).
Review follow-up: the sweeper is right that the fixture's premise leaked.
It clears the tokens and both command variables, but get_auth_status()
treats an `acp+tcp://` base URL as configured on its own — no executable
required — so on a host that sets COPILOT_ACP_BASE_URL the
missing-executable test was answering a question about the host instead
of about the code.
Verified by handing the test the hostile value it was vulnerable to:
with COPILOT_ACP_BASE_URL=acp+tcp://127.0.0.1:9999 in the environment,
test_copilot_acp_hidden_when_executable_missing fails before this commit
and all three tests pass after it.
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
andrexibiza's review on #101118 pointed out that the timeout branch
still ran the SessionDB close/checkpoint even when _shutdown_executor()
reported a live worker -- the exact sequence that produces the
wrong-page-number corruption in #101093. The close block now only runs
when _exec_live == 0; a surviving worker skips it entirely and leaves
the handle open for SQLite to recover from its own WAL on next open.
Adds test_stuck_worker_skips_the_session_db_close to prove the converse
of the existing ordering test: a worker that outlives the budget must
never be raced by close().
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019JujDvCo2vfpEiizidAS2U
`_shutdown_executor()` ran *after* the SessionDB close block in `_stop_impl`,
and it never waited. That left two ways for blocking DB work to outlive
`SessionDB.close()`:
(a) `_executor_closing` was still False during the close, so a coroutine
reaching `_run_in_executor_with_context` minted a brand-new pool and ran
more blocking DB work against handles that had just been closed;
(b) `cancel_futures` only drops work that has not started, and cancelling
`self._background_tasks` does not stop the worker thread behind a
`run_in_executor` future that is already running.
`SessionDB.close()` checkpoints the WAL and lets SQLite unlink the sidecar. A
write that lands after it silently reopens the handle (#94736) and mints a
fresh WAL generation behind that checkpoint, so teardown checkpoints the same
file a second time from a connection the shutdown log never accounts for --
the close-time page-write damage in #101093 and the split WAL generation in
#101064.
The quiesce now runs before the close and waits for the running workers. The
wait is bounded by `_EXECUTOR_QUIESCE_TIMEOUT` (2s) and clamped to what is left
of the shutdown watchdog leash minus a second for the close itself, so a stuck
worker can never cost the post-close cleanup window (#82161). Workers still
alive after the budget are logged as a warning instead of being waited on.
`_shutdown_executor()` keeps its no-argument fire-and-forget contract and now
returns the number of workers still running.
Refs #101093
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DGpGPnvz5FFFH999i59Xeb
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).
Review findings (Salt, NS-788):
B1: delivery_outcome classification, unresolved_origin, and incident
'alerted' marking all read the deliver lane while the notice itself was
routed through failure_deliver — a silenced failure recorded
delivery_outcome='delivered' and marked its incident alerted (corrupting
the 'failure seen' vs 'operator was pinged' distinction the incident
store documents), and a failure delivered via failure_deliver over an
unresolvable deliver=origin recorded 'not_configured'. New
_delivery_lane_value() helper feeds the SAME lane to routing and
bookkeeping at all five sites (both classifiers, both unresolved_origin
computations, both zero-target checks). Three regression tests assert
outcome + alerted-marking; verified to bite on the pre-fix classifier.
S1: failure_deliver now goes through _resolve_cron_context_deliver on
tool create/update, matching deliver — a job created from inside a cron
run can no longer store literal 'origin' in its failure lane.
S2/T1: corrected the false 'same helper' comment in create_job; the
str/list flatten mirrors the tool layer for direct callers.
Full cron suite + interrupt tests: 87 files, 1112 passed, 0 failed.
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.
Flip the state.db retention defaults per Teknium's decision on #54189:
- sessions.auto_prune: false -> true. A stock install now prunes ENDED
sessions inactive for retention_days at CLI/gateway/cron startup
(at most once per min_interval_hours). Open, pinned and mid-turn
sessions are never deleted; the only open rows touched are stale
automation sessions (#100903 sweep), which are closed, not deleted,
and aged a further full window before removal.
- sessions.retention_days stays 90 (already the default; verified).
- Auto-VACUUM is now additionally gated on the reclaimable fraction of
the file: PRAGMA freelist_count / page_count must exceed 25%
(AUTO_VACUUM_MIN_FREELIST_RATIO) on top of the existing
min_vacuum_interval_days throttle. Pruning a few small sessions on a
dense multi-GB DB no longer rewrites the whole file to reclaim a few MB.
Unknown ratio (pragma read failure) falls back to the time throttle.
Existing installs that explicitly set any sessions.* key keep their
values (load_config deep-merges DEFAULT_CONFIG under user YAML); only
unset keys pick up the new defaults. No _config_version bump needed.
cli-config.yaml.example documents the section commented-out so
installers that copy it verbatim never pin these as explicit settings.
Tests: ratio gate (below/above/at-threshold/unknown/override), real-DB
freelist ratio, default assertions, fresh-config startup hook reaches
the prune call, explicit opt-out respected, template-does-not-pin-keys.
Policy: availability-gated tools (check_fn probes — Docker, HASS_TOKEN,
OAuth…) are frozen for the life of a session. tools[] only changes on
/new, /reload-mcp, or compaction. Two doors remained after #100638:
* Gateway agent-cache eviction (LRU/idle sweep/cross-process invalidation)
rebuilds a fresh AIAgent for the SAME session and agent_init re-derives
agent.tools from live probes with no predecessor to preserve. Persist
the session's resolved tool-name order in a new `sessions.tool_names`
JSON column (declarative reconciliation, SCHEMA_VERSION 28), written
alongside the system prompt and re-pinned on every published refresh
(so /reload-mcp and compaction naturally reset it; /new mints a new
row). On restore-for-existing-session the fresh definitions are folded
onto the saved order via the SAME `_merge_preserving_prefix` helper —
a probe-flipped tool is carried forward from the registry schema, a
deregistered one dropped, new tools appended at the tail.
* /reload-mcp (CLI, gateway, TUI RPC) now also calls
`reprobe_tool_availability()` — drops the check_fn verdict cache and the
get_tool_definitions memo — so a user can consciously pick up a
credential/daemon that appeared mid-session. Docs updated.
The per-turn MCP refresh re-derives `agent.tools` from live availability and
publishes the result wholesale. Two kinds of bytes move as a result:
* a tool whose `check_fn` merely flapped (headless browser probe, expired
credential, docker blip) disappears from the array, and
* a late-landing MCP tool splices into sorted position, which can be index 0.
Providers that render `tools` ahead of the messages re-prefill the entire
history behind any moved byte, so either case costs a full re-prefill of the
session — the measured 2% cache hit in #100336. The caller's own comment
claimed the refresh "only ever extends a fresh request prefix"; it did not.
`refresh_agent_mcp_tools(..., preserve_prefix=True)` makes that claim true.
The live order becomes authoritative: existing tools keep their slot (fresh
schemas still land), a tool that is still registered but momentarily
unavailable is carried forward, a tool that genuinely left the registry is
still dropped, and new tools are appended at the tail. Explicit `/reload-mcp`
and the compaction boundary keep the plain rebuild.
Refs #100336
_start_one_profile_adapters skipped only Platform.RELAY as shared
process-level ingress. WhatsApp is the same shape: the bridge is one
authenticated session tied to a single phone number, so a secondary
profile has no credential of its own to bring; constructing an adapter
for it only produced a connect/retry loop that stalled startup for every
profile queued behind it. Treat WhatsApp like Relay -- the active profile
owns the connection and route-stamped source.profile fans inbound turns
out to secondary profiles.
Salvage of #69042 (narrowed by its author to this one behavioral line);
test re-expressed on the current secondary-startup fixtures.
Co-authored-by: sshawn <28279366+lsshawn@users.noreply.github.com>
Teknium