Third correction, and the load-bearing one. The previous commit put the
"did compaction clear the threshold?" verdict inside should_compress(). But
conversation_loop calls should_compress() TWICE per turn with two different
measures (turn_context.py / conversation_loop.py:1033 and :4789):
* pre-API : request_pressure_tokens -- a rough estimate that can dip BELOW
the threshold
* post-API: real prompt tokens -- which stay above it
So the rough reading reset the strike every turn and the loop never stopped.
Reproduced: 8 compactions in 8 turns under the real two-call pattern, even
with the previous fix applied. (My earlier repro only called should_compress()
once per turn, which is why it looked contained.)
Move the verdict to update_from_response(), the one place that sees the
provider's real prompt_tokens for the just-compacted conversation, guarded by
the existing awaiting_real_usage_after_compression flag so it fires exactly
once per compaction. Real-vs-real: it cannot be fooled by a rough sub-threshold
reading, and (from the previous commit's lesson) never subtracts an estimate
from a real count. should_compress() goes back to a plain threshold test plus
the pre-existing cooldown and anti-thrash guards.
New test test_rough_preflight_reading_does_not_reopen_the_loop drives the real
two-call-per-turn pattern and fails on the prior should_compress()-based commit.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Follow-up to the previous commit, whose futility check was unsound:
incompressible_floor = max(0, display_tokens - pre_estimate)
`display_tokens` is the provider's real prompt count; `pre_estimate` is
`estimate_messages_tokens_rough(messages)`. Subtracting an estimate from a real
count folds the tokenizer skew into "floor" and misreads it as incompressible
overhead. With a 1.6x skew on a 200K window (threshold 150K, true floor 30K):
rough_msgs=253,804 real_prompt=436,086
computed floor = 182,282 <-- mostly skew; exceeds the threshold
after compaction: 401 -> 77 msgs, real prompt = 106,361 (CLEARS 150,000)
verdict: ineffective_count = 1 <-- false positive
Two such passes would permanently disable compaction on a healthy session --
worse than the loop this PR set out to fix.
Move the check into should_compress(), where both sides of the comparison are
the caller's own token count:
* prompt under the threshold -> not thrashing; reset the counter
* a compaction just ran and we are STILL over -> one strike
Real-vs-real, so tokenizer skew can never be mistaken for a floor, and nothing
subtracts an estimate from a real count. compress() now only ever increments the
counter; the reset lives with the one measure the trigger uses.
Adds `test_no_false_positive_under_tokenizer_skew` (the case above) and
`test_counter_resets_once_the_prompt_fits_again` (one failed pass must not
disable compaction forever). Against upstream, 5 of the 7 cases fail; the 2 that
pass are the regression guards, which is the intended shape.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`should_compress()` documents anti-thrashing protection ("if the last two
compressions each saved less than 10%, skip compression to avoid infinite
loops"). In practice `_ineffective_compression_count` reset on every pass,
so the guard was dead code and a mis-sized context window presented as a
hung CLI instead of a warning.
Two defects:
1. Mixed measurement bases. Effectiveness was
`(current_tokens - estimate(compressed)) / current_tokens`, where
`current_tokens` is the provider's FULL prompt (system prompt + tool
schemas + messages) but `estimate(compressed)` covers messages only.
Every compaction therefore reported ~96% savings and reset the counter.
Savings is now scored messages-vs-messages.
2. Message shrinkage is the wrong yardstick. `should_compress()` trips on
the full prompt, but compaction can only shrink messages -- the system
prompt and tool schemas are an incompressible floor. When that floor
alone meets the threshold, each pass shrinks messages by a healthy
margin, legitimately resets the counter, and still leaves the prompt
over the line; the next turn compacts again, forever. Observed in the
wild: 45+ consecutive compactions, one auxiliary-LLM call each, zero
progress. Effectiveness is now scored against the goal -- did the
projected prompt get under the threshold? -- and a futile pass warns
with the numbers that prove it.
Also record an ineffective pass on the "only N messages (need > M)" early
return, which previously returned the transcript unchanged without moving
any anti-thrash state -- the same class of bug the neighbouring
"no compressable window" branch was already fixed for.
Tests: 4 of the 5 new cases fail on main and pass here; the fifth pins
that effective compaction still resets the counter (121 -> 15 messages,
88.9% savings).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Preserve deletability, route identity, stored costs, aggregate reconciliation,
and zero-usage Codex route accounting on top of the salvaged per-model usage
work.
The `sessions` table records only the initial (model, billing_provider)
for a session, so when a user switches models mid-session (via `/model`
or programmatically) every token — including the switched model's — is
attributed to the first model. Insights/billing reports then hide the
cost of the new model entirely (e.g. a session that started on deepseek
and switched to opus shows $0 for opus).
Add a `session_model_usage` table keyed (session_id, model,
billing_provider) that accumulates each per-API-call delta under the
model active at the time of the call. `update_token_counts()` is the
single chokepoint every per-call delta flows through (CLI, gateway,
cron, delegated, codex), so recording there captures accurate
attribution on every platform. Only the incremental path records — the
gateway's `absolute=True` summary overwrite is skipped to avoid
double-counting cumulative totals that can't be split per model. When a
call omits the model, it falls back to the session's recorded model,
matching the existing COALESCE-from-session summary behaviour.
Insights `_compute_model_breakdown` now aggregates tokens and cost from
`session_model_usage`, so a switched session splits correctly across
models, with a defensive fallback to the per-session aggregate for any
session lacking usage rows. A v17 migration backfills one usage row per
existing token-bearing session from its aggregate totals (idempotent via
INSERT OR IGNORE), validated lossless against a 1.3 GB production DB.
Tests: per-model recording, mid-session split, model fallback, absolute
no-double-count, v17 backfill, and an insights-level switch breakdown.
Fixes#51607.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
_CodexCompletionsAdapter (agent/auxiliary_client.py) is a second,
independent producer of Codex Responses input — used by auxiliary
calls (context compression, flush_memories, MoA aggregation,
session_search) that route through CodexAuxiliaryClient instead of
the main agent's ResponsesApiTransport.build_kwargs. It calls
_chat_messages_to_responses_input() directly without is_github_responses,
so the previous commit's fix didn't cover it: an auxiliary call made
against a Copilot-backed session could still replay a connection-scoped
codex_message_items id and hit the same HTTP 401.
Detect the Copilot host from the adapter's own client.base_url (same
check the adapter already does further down for prompt_cache_key
opt-out) and pass is_github_responses through, closing the gap.
Still #32716.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Copilot (api.githubcopilot.com/responses) binds replayed assistant
codex_message_items ids to a specific backend "connection". Credential-
pool rotation, a gateway restart, or routine load-balancer churn between
turns all invalidate that binding, and Copilot rejects the stale id with
HTTP 401 "input item ID does not belong to this connection" — even for
short ids well under the #27038 64-char length cap, since this is a
connection-scope problem, not a length problem. Once a session captures
one of these ids it is persisted and replayed forever, permanently
bricking the session.
Thread an is_github_responses flag from build_kwargs/convert_messages
into _chat_messages_to_responses_input and drop the id unconditionally
on that path, mirroring how reasoning items already strip id on replay.
phase/status/content are still replayed so cache-relevant signal isn't
lost — only the connection-scoped id is unsafe to reuse.
Written to apply independently of the #27038 length-cap fix so the two
PRs don't block each other; they touch adjacent conditions in the same
block and merge cleanly in either order.
Fixes#32716
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Cron jobs in the gateway process wedged before HTTP on later non-streaming
API calls because interruptible_api_call spawned a daemon worker inside
nested cron thread pools. Route cron platform turns through direct_api_call
on the conversation thread instead.
Codex assigns assistant message items server-side ids that can run
400+ chars (base64 encrypted blobs), but the Responses API caps
input[].id at 64 chars and rejects the whole request with a
non-retryable HTTP 400. Once a session captures one of these long
ids, every subsequent turn replays it and 400s forever, since the
history persists it in codex_message_items.
Add a 64-char length guard at both replay sites — the history-to-
input converter and the final preflight gate — so oversized ids are
dropped while short ids (msg_...) are kept for prefix-cache hits.
Mirrors the existing pattern for reasoning items, which already
strip their id before replay because store=False means the API
can't resolve ids server-side anyway.
Fixes#27038
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
force_close_tcp_sockets stayed shutdown-only after #29507 to avoid
cross-thread FD recycle. That left CLOSED sockets unreclaimed when
httpx.close() skipped already-shutdown sockets under long-lived
gateways (~1 CLOSED fd / 6 min via proxy).
Add release_fds= for the owning-thread dispose path only; abort still
defaults to shutdown-only.
When web_search results are passed directly to web_extract, the URLs
field contains dict objects (e.g., {"url": "...", "title": "..."})
rather than plain URL strings. Two code paths assumed URLs were always
strings and crashed:
- agent/display.py get_cute_tool_message for web_extract: tried to call
url.replace() on a dict, causing AttributeError
- tools/web_tools.py web_extract_tool loop: tried regex search on a dict,
causing TypeError
Both now extract the URL string from dict objects (url or href field) or
fall back to empty string, preserving the cosmetic display and allowing
the tool to process the URLs correctly.
Fixes#61693
Curator review forks now pass credential_pool and request_overrides from resolve_runtime_provider into AIAgent so pool-backed custom providers can rotate credentials on 401 like main chat.
Add a token-free, curated affection matcher (agent/reactions.py) — the single
source of truth for detecting user "vibes" (ily / <3 / good bot / heart emoji).
No model call, no tokens. Generalized to return a reaction *kind* so future
reactions can ride the same signal.
Wire an opt-in AIAgent.reaction_callback that fires from build_turn_context on
the incoming user message. It never touches the conversation (cache-safe) and
never fatal — a purely cosmetic side-beat each host can consume.
Only restore held verification text when the loop genuinely ends through budget exhaustion. Preserve later interrupts and failures, keep generated-summary fragment explanations, and add regression coverage for both contracts.
Treat intent-ack continuation text as non-final so last-turn exhaustion requests a real summary instead of surfacing a premature promise. Keep iteration-limit fallback text free of the abnormal-fragment explainer.
Name the continuation fallback for its actual verification-only provenance so unrelated continuation paths cannot accidentally inherit its cron-delivery semantics.
Track held-back verification responses explicitly so budget exhaustion returns the composed report without a second model call. Keep unrelated error and recovery exit reasons intact, preserve Kanban timeout accounting, and cover the real run_conversation paths.
Clear stale final_response before verify-on-stop/pre_verify loop
continues, and normalize iteration-limit exit reasons in turn_finalizer
when a composed answer survives with unknown/budget_exhausted.
Fixes#61631
Sibling sites of the salvaged #55997 fix, all reading user-editable
config values through .get(key, '').method(): MoA slot provider/model
labels, gateway quick-command alias targets (2 sites), gateway.proxy_url,
and gateway.relay_url. Regression tests for the contributor's two sites
plus the MoA labels.
dict.get(key, default) returns None (not the default) when the key
EXISTS with value None. The default only applies when the key is ABSENT.
Chained method calls (.strip(), .upper(), .count()) crash with
AttributeError on NoneType.
Fix two confirmed hits:
- auxiliary_client.py: custom provider base_url/api_key (config null)
- anthropic_adapter.py: text block content (API null response)
Pattern: .get(key, "").method() → (.get(key) or "").method()
* test: deflake CI and dev-machine flaky tests in bulk
Fixes ten distinct flake sources found by mining recent CI failures and
running the full suite on a dev machine with real user state:
CI-observed races:
- tests/conftest.py live-system guard: allow signal 0 (pure liveness
probe) through _guarded_kill/_guarded_killpg. psutil.pid_exists()
probes a just-killed grandchild reparented to init; the subtree check
fails for it and the guard RuntimeError'd
test_entire_tree_is_sigkilled_not_just_parent intermittently on
unrelated PRs.
Hermeticity flakes (fail on dev machines with real state, pass on CI):
- agent/coding_context.py: _marker_root() now skips the shared temp
root (tempfile.gettempdir()) like it skips $HOME — a stray
/tmp/package.json flipped every tmp_path test into the coding
posture (9 failures in test_coding_context.py).
- test_agent_guardrails.py: pin MAX_CONCURRENT_CHILDREN=3 via autouse
monkeypatch instead of freezing the user's real config value at
import time (import-time vs call-time config mismatch).
- test_web_tools_config.py: TestCheckWebApiKey now neutralizes the
ddgs package probe and registry providers — the optional ddgs
package in a dev venv lit up the fallback backend.
- test_credential_pool.py: block claude_code/hermes-oauth credential
autodiscovery in the two pool-merge tests that assert exact id
lists (a real ~/.claude/.credentials.json seeded an extra entry).
- test_modal_sandbox_fixes.py: clear _permanent_approved /
_session_approved — the user's real command_allowlist silently
approved the guard-escalation commands under test.
- test_setup_irc.py: stub prompt_checklist to select only the IRC row;
the non-TTY cancel fallback re-ran the real configured platforms'
interactive setup_fn, which hit input() under captured stdin.
- test_doctor.py: TestGitHubTokenCheck now patches the module-level
HERMES_HOME constant (the file's established pattern) instead of
only setenv — doctor was running PRAGMA integrity_check against the
real multi-GB state.db and blowing the 300s per-file budget.
Latent atexit-duplication (same _enter_buffered_busy class as #34217):
- test_undo_command.py: drop importlib.reload(tui_gateway.server) in
fixture teardown; reload re-registers the module's atexit hooks.
- test_session_platform_resolution.py: drop per-test reload of
tui_gateway.server; every resolver reads env at call time.
* test: sentinel model value in ignore-user-config fallback assertion
With HERMES_IGNORE_USER_CONFIG=1, load_cli_config() falls back to the
repo-root cli-config.yaml (untracked, gitignored). On a dev machine that
file can legitimately set the same popular model the test hardcoded
(anthropic/claude-sonnet-4.6), flipping the != assertion locally while
CI (no cli-config.yaml) stayed green. Use an impossible sentinel model
name instead.
_load_context_cache() returned None when context_length_cache.yaml
contained 'context_lengths:' (no value) — YAML parses this as
{'context_lengths': None} and dict.get(key, default) only returns
the default when the key is absent, not when the value is None.
This caused AttributeError in every downstream caller (issue #47135).
Fix: use 'or {}' instead of default= so both absent key and
None value return an empty dict.
Fixes#47135
switch_model() rebuilds _client_kwargs from scratch (api_key + base_url)
but does not call _apply_client_headers_for_base_url(), so provider-
specific headers like OpenRouter HTTP-Referer and X-Title are lost.
Subsequent requests show "Unknown" in OpenRouter dashboard logs.
Call _apply_client_headers_for_base_url() after rebuilding _client_kwargs
and before creating the new client.
Fixes#61099
switch_model() unconditionally set agent.provider but only set
agent.base_url when the resolved value was truthy. When a real
provider change resolved an empty base_url (e.g. minimax after
copilot), the agent ended up with provider="minimax" but
base_url still pointing at api.githubcopilot.com. That incoherent
pair then got snapshotted into agent._primary_runtime, so it kept
re-applying on every subsequent turn via restore_primary_runtime()
until the process restarted.
try_activate_fallback() and _swap_credential() were audited and
confirmed unaffected: both always derive base_url from an actually
constructed client, never from a possibly-empty resolver hint.
Fix: when base_url is empty AND the provider is genuinely changing,
raise ValueError instead of silently keeping the old provider's URL.
This routes through switch_model()'s existing snapshot/rollback
path, and callers (tui_gateway/server.py's _apply_model_switch)
already catch and surface a clean "switch failed, staying on X"
message. Re-selecting the SAME provider with an empty base_url
(credential-only refresh) still keeps the current URL, unchanged.
Fixes#47828
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Close the remaining end-to-end gaps so the full gpt-5.6 family (sol/
terra/luna + their -pro high-effort modes, 6 slugs) works on every
surface a user can reach them through:
- agent/auxiliary_client.py: the Codex OAuth backend hard-caps context
at 272K for gpt-5.6 exactly as it does for 5.4/5.5, but the default
50% compaction trigger would summarize at ~136K and waste half the
usable window. Extend the existing _is_codex_gpt54_or_gpt55 chokepoint
(single enforced predicate feeding _compression_threshold_for_model)
to match gpt-5.6* on the openai-codex route so those sessions get the
same 0.85 auto-raise. Direct-API/OpenRouter routes (full 1.05M window)
are unaffected; the historical codex_gpt55_autoraise opt-out still
applies. The one-time notice banner is model-dynamic and already
renders the correct slug/cap.
- hermes_cli/config.py, agent/agent_init.py: refresh the autoraise
comments/notice to mention the 5.6 family.
- hermes_cli/codex_models.py: add the -pro variants to DEFAULT_CODEX_MODELS
+ forward-compat so ChatGPT-OAuth (openai-codex) Pro users see the full
family in /model, not just the base tiers.
Supersedes the earlier commit's note that 5.6 was intentionally kept out
of the codex catalog: the slugs are confirmed routable (OpenRouter live
+ codex backend), so they belong there like every other codex-capable
gpt-5.x slug.
E2E verified across all 6 slugs: direct-API ctx 1.05M, codex ctx 272K,
pricing reachable from openai + openai-api routes, codex compaction
override 0.85 (and None on direct-API + when opted out), present in
openai-api picker + codex catalog, /model gpt resolves to sol on both
native routes. Guard tests added for the compaction route matrix.
PR #61587 adds sol-pro/terra-pro/luna-pro to the aggregator lists.
Complete those on the native surfaces the same way this PR completes
the base tiers:
- hermes_cli/models.py: -pro variants in _PROVIDER_MODELS[openai-api].
- agent/usage_pricing.py: alias ("openai", "gpt-5.6-*-pro") onto the
base-tier PricingEntry rows — the -pro high-effort modes bill at the
SAME per-token rates (verified against OpenRouter live pricing
2026-07-09: identical prompt/completion prices for base and -pro);
they cost more per task by consuming more tokens, not a higher rate.
- Context lengths need no new entries: "gpt-5.6-sol" et al. are
substrings of their -pro variants and both lookup tables match
longest-key-first (verified: sol-pro -> 1.05M direct / 272K codex).
- model_switch sort: -pro variants parse as suffix "sol-pro" (rank 1),
so /model gpt still defaults to base sol — pinned by test.
- Not added to DEFAULT_CODEX_MODELS: only confirmed routable via API/
OpenRouter so far; codex live discovery will surface them if ChatGPT
exposes them, same policy as other unconfirmed codex slugs.
Tests: invariant tests extended (pro aliases share base entries, base
sol outranks sol-pro); 191 targeted tests pass.
Phase-2 review findings addressed:
- resolve_billing_route: normalize the "openai-api" picker slug to the
"openai" billing provider — without this the ("openai", <model>)
_OFFICIAL_DOCS_PRICING keys (incl. every pre-existing gpt-4o/gpt-4.1
entry, not just 5.6) were unreachable when the provider is openai-api.
- pricing_version: drop the "preview" tag (GA 2026-07-09 at same rates).
- model_metadata comment: dict order is cosmetic — lookups length-sort
keys at match time; the old comment implied a positional invariant.
- model_switch comment: note "sol" is a series codename, not a generic
quality word.
- tests/hermes_cli/test_gpt56_registration.py: behavior contracts (no
list snapshots) — sol > terra/luna > 5.5 sort invariant, pricing
reachability from both openai and openai-api routes, cache-write
1.25x / cache-read 0.10x input relation.
