Skip to content

feat(auxiliary): give short auxiliary calls their own lane, off the conversation's KV slot - #340

Merged
OmarB97 merged 8 commits into
mainfrom
feat/aux-dedicated-route-fork-20260802
Aug 2, 2026
Merged

feat(auxiliary): give short auxiliary calls their own lane, off the conversation's KV slot#340
OmarB97 merged 8 commits into
mainfrom
feat/aux-dedicated-route-fork-20260802

Conversation

@OmarB97

@OmarB97 OmarB97 commented Aug 2, 2026

Copy link
Copy Markdown
Owner

Every auxiliary.<task>.provider ships as "auto", and _resolve_auto's first
priority is "the user's main provider + main model". On a local-only setup that
is not a preference, it is a constraint: every auxiliary call lands on the
same endpoint as the conversation itself.

On a single-slot local server (llama.cpp -np 1, vLLM with one slot) that is
actively harmful. A goal judge or approval classifier sends 2-6K tokens; the
conversation is 95K. The short call takes the slot, the long conversation's KV
cache is evicted, and the next main turn re-prefills from zero. Measured on the
affected lane: 8/8 prompt-cache misses against a 3% baseline. The request
bytes are provably stable (tests/agent/test_request_prefix_stability.py, #334),
so eviction is what is left.

The same contention shows up a second way, which is what makes this worth fixing
rather than documenting. The approval classifier is itself an aux call, so it
queues behind the very turn it is meant to gate — the session self-contends,
the classifier times out, and _smart_approve falls back to asking a human. In
an unattended delegated run there is no human, so the run stops. One cause, two
symptoms: a cold cache and a stalled agent.

What this adds

auxiliary.route — one lane that every aux task falls back to:

auxiliary.<task>.*   >   auxiliary.route.*   >   "auto" (main lane)

Shipped empty, so with no config written the behavior is byte-identical to
today. Point it at a second endpoint and the short calls go there instead.

The overlay lives at exactly one site, _resolve_task_provider_model
(agent/auxiliary_client.py), which all five aux resolution entry points funnel
through — call_llm, async_call_llm, get_text_auxiliary_client,
get_async_text_auxiliary_client, resolve_vision_provider_client. It is
deliberately not at _resolve_auto, because agent/agent_init.py builds the
main agent's client through that same function and a hook there would hijack
the main lane.

Because "auto" now means "the route", the per-task escape back to the live chat
model is the already-supported auxiliary.<task>.provider: main.

The trap this had to defeat first

An earlier attempt at this change was abandoned because it stalls unattended
turns, and the mechanism is worth stating precisely, since it is not obvious.

tools/approval.py::_smart_approve fails closed: any exception becomes
return "escalate", logged at debug. Separately, call_llm's fallback gate is

if should_fallback and (is_auto or is_capacity_error):

Setting a route makes resolved_provider concrete, so is_auto goes false. An
aux endpoint that is down raises a connection error, which counts as a
capacity error, so it falls back to the main lane and everything works. An aux
endpoint that is up but returns 401 raises an auth error, which is not in
the capacity set — so the entire fallback block is skipped, the exception reaches
_smart_approve, and every smart auto-approval silently becomes a human prompt.
In a gateway-backed unattended run that is a 60s approval timeout and then a hard
block, per flagged command. Silently, because the only trace is a debug line.

So the gate now reads (is_auto or is_capacity_error or route_derived), in both
the sync and async paths. is_auto exists to respect a user's explicit per-task
pin; a provider that arrived from the global route is a routing default, not that
pin. With this, the same 401 dud costs one attributed warning and the main lane
answers.

Four supporting guarantees, so that pointing aux at an endpoint which may be
down, unkeyed, or slow can never be worse than today:

  • Quarantine. _mark_provider_unhealthy was only ever called for payment
    errors. A dead routed lane now pays its penalty once per TTL, not once per call.
  • Bounded budget. auxiliary.approval.timeout is 30s with 2 transient
    retries — about 93s, which out-waits the 60s human approval timeout. Routed
    calls get their own timeout and transient_retries (default 0), floored under
    the human budget for the approval task.
  • Loud. The debug swallow in _smart_approve is now a warning, and routed
    failures go through the existing AIAgent._emit_auxiliary_failure, so the
    operator sees one attributed line in chat instead of nothing.
  • Preflight. The route is resolved once at agent init and warns loudly if it
    yields no client or the no-key-required placeholder.

The silent 401, root-caused

That placeholder deserves its own commit, because it is a real bug independent of
this feature. hermes_cli/runtime_provider.py's new-style providers: dict path
resolves key_env internally but does not return it — only the legacy
custom_providers: list path does. So the second-chance os.getenv(key_env)
lookup in auxiliary_client could never rescue a providers: entry, and a client
was built with the literal string "no-key-required" and the request sent anyway.
key_env is now carried out of both paths.

Deliberately out of scope

background_review and curator resolve their own model and stay main-model-first
on purpose — they replay the full transcript because it is already warm in the
main prompt cache, and agent/background_review.py already switches to a compact
digest when routed away "because a different model can't reuse the main prompt
cache anyway". Routing them would force a digest replay to save a cache they were
built to exploit. plugin_llm and trajectory_compressor pass no task and stay on
auto.

Also worth stating plainly: with fallback_policy: "any" (the default), an
unreachable aux lane silently re-routes back to the main lane. Prompt-cache
protection is best-effort by design. That fallback is load-bearing — removing it
is what converts a slow degradation into the approval trap above.

Testing

scripts/run_tests.sh across the auxiliary client, aux routing, image routing,
runtime cache key, smart approval, goals, aux config, plugin aux tasks, vision
routing and config suites: 705 tests, 0 failures (and 237 more across the
config/vision/approval-cluster set).

The existing approval and goal tests all stub at the call_llm / _smart_approve
/ judge_goal boundary, so none of them can observe a routing regression — which
is precisely why this trap survived an earlier attempt. The new
tests/agent/test_auxiliary_route.py drives the route end to end instead of at
that seam, and its headline case is the one that matters: route set, aux
endpoint returns 401, _smart_approve must not escalate
— the call falls back
to the main lane and answers.

Also covered: route unset resolves byte-identically to today; a task's own
explicit provider still beats the route; provider: main still returns the live
chat model; connection errors quarantine the routed provider; the routed approval
budget is provably under approvals.timeout; a providers: entry with key_env
resolves a real key; and the goal judge still fails open on a routed lane,
with its five-consecutive-transport-failure auto-pause intact.

Omar Baradei and others added 8 commits August 2, 2026 11:51
`first_chunk_timeout_seconds` was documented only in
cli-config.yaml.example. The timeout table in the configuration guide is
where users actually look when a local turn is being killed, and it listed
every other layer but this one.

Also states the rule that trips people up: these keys go under the NAMED
provider entry, not under `custom`. Every user-declared endpoint resolves to
the billing class `custom` at runtime, and Hermes maps it back to the entry
that owns the endpoint URL — so a `providers.custom:` block would never be
consulted.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A `providers:` entry resolves its `key_env` internally but never returned
the name, so a consumer that received an empty `api_key` had no second
chance. `resolve_provider_client` has one — it re-reads
`os.getenv(entry["key_env"])` before giving up — but only the legacy
`custom_providers:` list path ever populated that field. A `providers:`
entry whose key could not be read (profile-scoped secret read failing
closed, or the env var exported after this process cached its config)
therefore fell through to the literal `no-key-required` placeholder and
the request was sent anyway, producing a silent 401.

Also accept the `api_key_env` spelling here. The auxiliary resolver
already honours both names; an entry written with the other one resolved
to an empty key without a word.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Every auxiliary task defaults to `provider: auto`, and `_resolve_auto`
priority 1 is "main provider + main model". On a single-slot local server
that means a ~2-6K-token approval or title call evicts the KV slot holding
a 95K-token conversation: measured 8/8 prompt-cache misses against a 3%
baseline, with request bytes proven stable by
tests/agent/test_request_prefix_stability.py. `auxiliary.route` gives those
calls one alternative endpoint so the main conversation's slot is left
alone.

Precedence: auxiliary.<task>.*  >  auxiliary.route.*  >  "auto".
The default is empty. Nothing is written to config.yaml, `_config_version`
is untouched, and with every field blank resolution is identical to having
no route at all.

One insertion point, in `_resolve_task_provider_model` — all five auxiliary
entry points funnel through it. NOT `_resolve_auto` (agent_init builds the
MAIN agent's client through `resolve_provider_client(provider or "auto")`,
so a hook there hijacks the main lane) and NOT `_get_auxiliary_task_config`
(image_routing and computer_use/vision_routing read RAW config to choose
native-vs-aux image handling and would flip).

The rails, because the previous attempt at this stalled unattended turns:

- A route-derived provider now falls back to the main lane on ANY failure,
  not only a capacity error. Without this, an endpoint that is UP but
  returns 401 is not a capacity error, `is_auto` is False because the
  provider is concrete, the whole fallback block is skipped, the exception
  reaches `_smart_approve`, and every smart auto-approval silently becomes
  a human prompt — a full `approvals.timeout` of dead time per flagged
  command in a gateway-backed run. `is_auto` exists to respect a user's
  explicit per-task pin; a provider that came from the global route is a
  routing default, not that pin. The signal is carried by a cheap
  pure-config predicate rather than by widening
  `_resolve_task_provider_model`'s 5-tuple, which ~20 tests patch by arity.
- An unreachable routed lane is quarantined for 10 minutes, so it costs its
  penalty once per TTL instead of once per call. Deliberately a separate
  cache from `_aux_unhealthy_until`: `_try_main_agent_model_fallback` bails
  when the MAIN provider is marked unhealthy, so writing a route's label
  into that shared cache could disable the very fallback that keeps
  unattended turns moving.
- Routed lanes get their own `timeout` and `transient_retries` (default 0),
  and a routed `approval` is capped below `approvals.timeout` so the aux
  answer — or its fallback — always lands before the human prompt gives up.
  Today's un-routed budget is 30s x 3 attempts + 3s backoff = 93s against a
  60s human deadline.
- Failures are loud. `_smart_approve`'s swallow is a warning instead of a
  debug line (it fails CLOSED, so silence there is how the trap hid), and a
  routed failure is surfaced through the agent's existing chat-visible
  auxiliary-failure channel.
- The route is preflighted at agent init, mirroring
  `check_compression_model_feasibility`: resolve it once and say loudly if
  it yields no client or only the `no-key-required` placeholder. Inert when
  no route is configured.

Not in v1, on purpose: `background_review` and `curator` resolve their own
runtime from raw config and are main-model-first BY DESIGN — they replay a
transcript that is already warm in the main prompt cache (background_review
already switches to a compact digest when routed "because a different model
can't reuse the main prompt cache anyway"). `plugin_llm` and
`trajectory_compressor` pass no task and stay on auto. `vision` is excluded:
an image payload needs a multimodal model and the vision path has its own
capability-aware chain; `auxiliary.vision` remains the pin.

With `fallback_policy: any` (the default) an unreachable aux lane silently
re-routes back to main, so cache protection is best-effort by design. That
fallback is not optional — removing it converts a slow degradation into the
approval trap.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Every existing approval / goal test stubs at the `_smart_approve` /
`judge_goal` / `call_llm` boundary, so none of them can observe a routing
regression — which is precisely how the "routed lane 401s, every approval
escalates" trap survived. These drive the real resolution path against a
temp HERMES_HOME with a real config.yaml and mock only the HTTP client.

Verified negative: reverting just the `or route_derived` term in the two
fallback gates fails test_routed_401_does_not_escalate_smart_approval,
test_routed_failure_is_reported_to_the_operator, and
test_judge_answers_from_the_main_lane_when_the_route_is_a_dud.

Covered: unset route resolves byte-identically for seven tasks; the route
owns unpinned tasks; per-task provider / base_url / model each beat it;
`provider: main` returns the main lane; vision is excluded and the raw
`auxiliary.vision` config that image routing reads is untouched; a routed
401 answers from the main lane while an explicitly pinned 401 still fails
closed; a connection error quarantines the lane without touching the shared
provider-health cache; the routed approval budget is under `approvals.timeout`
while today's un-routed one is not; a `providers:` entry's `key_env`
resolves and preflight warns when it cannot; the goal judge still fails
OPEN and still auto-pauses after five consecutive transport failures.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
What it is for (prompt-cache / KV-slot protection, not just cost), the
precedence chain, the empty default, every field, the `provider: main`
per-task escape, and the best-effort caveat: with `fallback_policy: any` an
unreachable lane silently re-routes back to main, because smart approvals
fail closed and a lane that simply errored would stall unattended runs.

Also notes the two deliberate exclusions — vision needs a multimodal model,
and background review / curator replay a transcript that is already warm in
the main model's prompt cache.

cli-config.yaml.example keeps its auxiliary section commented.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The example named a real host slug from the author's own fleet. Public
docs should not carry one operator's infrastructure names.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The sink is a module global. Registering a bound method pinned the agent
that registered it for the life of the process; a weakref closure gives the
same chat-visible warning without keeping a finished agent alive.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A MoA slot is already a per-slot model pin, and the ensemble's whole value
is model diversity — one global lane must never be able to collapse it.
agent/moa_loop.py always passes an explicit provider so the route would not
fire in practice; naming the two tasks makes that structural instead of
incidental, covering a preset slot with a blank provider.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@OmarB97
OmarB97 merged commit 7293578 into main Aug 2, 2026
32 checks passed
OmarB97 pushed a commit that referenced this pull request Aug 2, 2026
…anch

main advanced while this branch was in flight, and #338 ("wire the local
stream stale ceiling the config already promises") landed in the same
stale-timeout logic. Three conflicts, none of them a behavior collision:

* agent/chat_completion_helpers.py — docstring only. Both sides appended to
  the same paragraph in _dflash_prefill_scaled_timeout; main's text is a
  superset of ours, adding a cross-reference to _generic_local_stale_timeout.
  Took main's. Verified that function exists in the merged tree (line 630) and
  that it really is widened by the per-1k prefill term, so the reference it
  adds is accurate rather than aspirational.

* website/docs/user-guide/configuration.md — same sentence on both sides,
  differing only in the example provider name. Took main's `my-local-lane`:
  it is the entry actually used in cli-config.yaml.example, whereas
  `ai-router` appeared nowhere else in the docs.

* tests/agent/test_local_stream_timeout.py — a tail add/add. Our side
  contributes no lines at the conflict point; main appends
  TestGenericLocalStreamStaleCeiling. Kept both bodies by dropping the
  markers, so our first-chunk resolver tests above the seam and main's new
  class below it both survive.

No production logic was resolved away — the only code hunk was a docstring.

Verified on the merge result, not on either parent:
  pytest tests/hermes_cli/test_timeouts.py \
         tests/agent/test_request_prefix_stability.py \
         tests/run_agent/test_provider_parity.py \
         tests/agent/test_local_stream_timeout.py \
         tests/agent/test_reasoning_stale_timeout_floor.py \
         tests/agent/test_stream_read_timeout_floor.py -q
  343 passed

Merged rather than rebased on purpose: another session is active on this
branch with uncommitted work, and a force-push would have destroyed it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
OmarB97 added a commit that referenced this pull request Aug 2, 2026
… no model (#348)

`auxiliary.<task>.provider: my-local-lane` with no `model:` sent the MAIN
chat model's id to that endpoint. `resolve_provider_client` fills a blank
model for every concrete provider from `_read_main_model()`, so a task that
named one of the user's own `providers:`/`custom_providers:` entries got the
main model paired with the entry's base_url — a 404 from a local backend
serving a different model set, not an answer.

The entry's own `default_model` is what "use that provider" reads as, so it
now fills the blank first, in `_resolve_task_provider_model` — the one
function every auxiliary entry point funnels through (call_llm,
async_call_llm, get_text_auxiliary_client, get_async_text_auxiliary_client,
resolve_vision_provider_client), which is also where the raw config string
still exists.

`auto`, `main` and bare `custom` are excluded: they mean "the lane the main
runtime already resolved", so they keep inheriting the main chat model, and
`_read_main_model()` stays the last resort for a lane that declares no model
of its own.

PR #340 added exactly this rule for `auxiliary.route` and deliberately left
the per-task path alone to keep its blast radius small. Its inline lookup is
now the shared `_named_provider_default_model`, which normalises both config
shapes (`providers.<name>.default_model` and `custom_providers[].model`).

Co-authored-by: Omar Baradei <omar@kostudios.io>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
OmarB97 added a commit that referenced this pull request Aug 2, 2026
…n the approval classifier is down (#349)

`tools/approval.py::_smart_approve` fails CLOSED: every exception out of its
`call_llm(task="approval")` becomes "escalate", which means ask a human. With a
person at the keyboard that is right. In a delegated run there is nobody, so
each flagged command spends the full `approvals.timeout` waiting for an
approval that is never coming and is then BLOCKED — or returns
`status: "pending_approval"` and halts the operation outright. Observed live on
2026-08-02 in a `hermes desktop spawn --delegated` session. `auxiliary.route`
(#340) removed two of the triggers; it did not change the shape of the failure,
which is that classifier UNAVAILABILITY degrades to "ask a human" rather than
to a declared policy.

A session may now carry an allowlist declared in its brief — commands plus the
worktree they are scoped to. When the classifier cannot be reached, a command
that matches is auto-approved by policy with the reason logged; everything else
keeps escalating exactly as it does today.

Scoped as narrowly as the failure:

  * Only when the classifier was UNREACHABLE. A working classifier that says
    ESCALATE is unsure, not absent, and still wants a human; DENY still denies.
    _smart_approve's return alphabet is unchanged — "was never consulted"
    travels beside the verdict on a thread-local the caller clears before each
    call, so every stubbed _smart_approve reports "available".
  * Not in interactive CLI, where the prompt reaches someone immediately.
  * Only for env_type == "local"; a declared root names a path on this machine.
  * Not for execute_code, which is a Python script with no argv to judge.

Matching is on parsed argv plus the resolved working directory, never a regex
over shell text. A character allowlist runs first, so chaining, command
substitution, redirection, expansion, globbing, escapes and newlines are
refused before parsing. Then every token must resolve inside the declared root,
following symlinks, as must the cwd — and program-bearing commands (sh, python,
sudo, env, ssh, docker, xargs, timeout) cannot be declared at all, because they
run whatever their arguments name.

Declared per delegation via `hermes desktop spawn --allow-command /
--allow-command-root` (CLI -> spawn-control -> renderer -> session.create,
mirroring the --toolsets/--goal pins, and failing the create rather than
handing back a session that silently has no allowlist), or profile-wide via
`approvals.delegated_allowlist` in config.yaml. Both empty by default: with no
declaration the fail-closed path is byte-identical to today's.

terminal_tool now resolves the run directory before the guard instead of after
it, so an explicit `workdir=` is what a scoped rule is judged against.

Co-authored-by: Omar Baradei <omar@kostudios.io>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant