Review-pass findings on the lazy-init deferral:
- No-op guard: the codex app-server usage callback assigns
compressor.context_length on EVERY response (same window each time).
The setter unconditionally invalidated the derived budgets, wiping
runtime corrections applied directly to threshold_tokens /
tail_token_budget (aux-context threshold sync) — those persisted on
main's eager init. Same-value assignment is now a no-op.
- Re-floor on genuinely new window: the setter invalidates budgets but
previously kept the stale threshold_percent, so a codex window switch
recomputed threshold_tokens from the new window with the old model's
floored percent. Re-apply the raise-only small-context floor from
_base_threshold_percent so percent and tokens derive from the same
window (guarded with getattr for object.__new__ test instances).
- Init log extracted to _emit_init_summary_once() and also fired from
the setter path, so a consumer assigning context_length before any
read no longer strands the startup line forever.
- threshold_tokens getter resolves the window into a local before
reading threshold_percent — correctness no longer depends on
left-to-right argument evaluation order.
- reasoning_timeouts: fix inaccurate 'tuples are immutable' comment
(the container is a list; safety comes from build-once-at-import),
document why the slug stays in the tuple.
Adds TestContextLengthSetterCoherence (3 tests): same-value assignment
preserves overrides; new-window assignment re-floors both directions.
With the selective prompt-cache copy (#57046), un-marked messages on the
decorated api_messages list share their nested content parts with the
persistent conversation history — the per-message copy in
conversation_loop is shallow and decoration now deep-copies only the
marked messages. try_shrink_image_parts_in_messages previously wrote the
re-encoded image INTO the aliased part/source dicts, so an
image-too-large retry on an Anthropic route would silently replace the
original image bytes in agent.messages (and persist the degraded copy).
Replace the in-place writes with copy-on-write: rebuild the content list
with fresh part/source/image_url dicts and reassign msg['content'] — a
top-level write on the per-call copy that never reaches history.
Adds two regression tests simulating the aliasing; both fail against the
old in-place implementation (mutation-verified).
The lazy-init change in #32221 deferred get_model_context_length() out of
ContextCompressor.__init__, but the "Context compressor initialized" log
(emitted only when quiet_mode=False) reads self.context_length,
self.threshold_tokens and self.tail_token_budget. Those property reads
resolve the deferred value, so the synchronous model-metadata probe still
ran inside __init__ on the interactive-CLI path — the exact blocking the
PR removed, just narrowed to the non-quiet path.
Emit the informative line once, on first context-length resolution, so
construction stays non-blocking on every path. Add a regression test
asserting no probe fires in __init__ with quiet_mode=False and that the
init log is emitted exactly once on first access.
_match_any() was re-sorting _REASONING_STALE_TIMEOUT_FLOORS (21 elements)
on every call. This function runs per API turn via error_classifier,
chat_completion_helpers, and thinking_timeout_guidance.
Also fixes thread-safety: the old _PATTERN_CACHE was a mutable dict
accessed from multiple threads without locking. Pre-compiling all
patterns at module load time eliminates both the per-call sort and
the TOCTOU race condition. The resulting list is effectively
immutable after import, safe for free-threaded Python 3.13+.
(cherry picked from commit e8b006b853)
Replaces full deepcopy with selective shallow copy in apply_anthropic_cache_control.
Only the 4 messages that receive cache_control markers are deep-copied; the rest
stay as references.
Measured on a 100-message conversation (typical long session):
- Before: ~15ms per call
- After: ~2ms per call
- 7.5x faster, scales with conversation length
Memory impact is also significant — no need to duplicate dozens of unchanged
messages on every turn.
The contract is unchanged: callers still get an independent message list
they can mutate. Only messages we modify (by injecting cache markers) are
copied. The rest are shared references to immutable history entries, which
is safe since the agent never mutates past turns.
(cherry picked from commit 17892df6cc)
Every LLM call built a fresh openai.OpenAI wire client (new httpx pool,
TCP+TLS handshake, measured 19.2ms p50 / 35.5ms p95 per call at ~5 calls
per tool-loop turn) and closed it when the request finished. Cache ONE
reusable wire client on the agent, keyed by the effective client kwargs:
- _create_request_openai_client hands back the cached client when the
effective kwargs are identical; any change (credential rotation,
provider failover, vision default_headers) evicts and rebuilds.
- Only a request that produced a response reports a reuse close reason
(request_complete / stream_request_complete); error unwinds report
*_error_cleanup and really close, so a retry after a request error
always builds a fresh pool.
- Cross-thread aborts poison the slot: a pool whose sockets were
shutdown(SHUT_RDWR) from a stranger thread is never reused (#29507) —
the owner-thread close discards it and the next create rebuilds. The
holder read and the abort are atomic (under the holder lock) at all
three abort sites, so a late abort can never poison the NEXT request's
checked-out client.
- Worker-side interrupt breaks close the half-read SSE stream on the
owning thread before building the partial response; a failed close
poisons the slot (otherwise each interrupt leaked one checked-out
connection until the pool hit PoolTimeout). run_codex_stream gets the
same poison-on-close-failure handling.
- Single checked-out slot (in_use): a concurrent call gets an untracked
client with the old per-request lifecycle.
- release_clients() / close() really close the cached client when idle;
if a worker has it checked out they abort the sockets and detach the
slot, deferring the FD release to the worker's own close.
- MoA facade and Mock passthroughs never enter the cache; max_retries=0
is preserved on all request clients.
Every API call in the tool loop persisted its token/cost delta by
calling SessionDB.update_token_counts() synchronously on the turn
thread — a BEGIN IMMEDIATE sessions UPDATE plus a session_model_usage
upsert, measured in production at p50 3.3ms / p95 70.4ms per call and
up to 299ms against a cold multi-GB state.db. The tool loop stalls for
that long between calls, N times per multi-tool turn.
SessionDB gains queue_token_counts(): same signature and semantics as
update_token_counts(), but the critical path is a deque append plus a
condvar notify. A lazily started daemon thread applies deltas in
enqueue order through the existing update_token_counts ->
_execute_write path, so the established self._lock / BEGIN IMMEDIATE /
jitter-retry discipline is unchanged. When a backlog forms, adjacent
same-route incremental deltas coalesce into one UPDATE: token and
api-call fields sum, cost fields sum None-preservingly (an all-None
run stays None so COALESCE keeps the stored value), and absolute=True
deltas never merge and act as ordering barriers. Route equality is
required for a merge because those fields feed COALESCE backfill, the
last-non-None-wins status fields, and the per-model usage attribution
key — a merged apply is row-equivalent to sequential applies.
Correctness and durability:
- flush_token_counts() gives read-your-writes to token/cost readers
(get_session, list_sessions_rich, _get_session_rich_row,
list_gateway_sessions, InsightsEngine.generate) — a plain attribute
check when nothing is queued. The writer sets its busy flag before
popping the queue so the lock-free fast path can never miss an
in-flight batch.
- update_session_model / update_session_billing_route /
update_session_meta write the sessions row synchronously, bypassing
the queue, so they flush it first: a still-queued first-of-session
delta carries the pre-switch route, and applying it after the switch
UPDATE would trip the first_accounted_route branch (api_call_count
== 0 plus a route mismatch) and resurrect the old model/provider.
- AIAgent._persist_session flushes at turn finalize and every
error-exit persist point; close() stops and drains the writer before
the WAL checkpoint; an atexit hook (registered on first enqueue,
unregistered on close so closed instances are not pinned until
interpreter exit) drains at shutdown. Worst-case crash loss is the
in-flight call's delta — the same window as the old inline write.
- A flush trusts a live stop-flagged writer (its loop drains before
exiting) and only drains on the caller's thread when the writer is
dead or never started, claiming the same busy flag so concurrent
flushes wait instead of racing an in-flight batch.
- After close() has stopped the writer, queue_token_counts applies the
delta inline instead of parking it on a queue nothing will drain; a
closed-connection failure then raises at the call site, which
already guards for it, exactly like the old synchronous path.
- Writer apply failures are logged and never raise into a turn; the
writer thread survives and keeps applying.
Call sites switched to the queue: the per-call site in
agent/conversation_loop.py and both codex app-server sites in
agent/codex_runtime.py. In-memory per-turn counters
(agent.session_estimated_cost_usd etc.) stay synchronous, so live turn
displays never see the queue.
Tests: tests/agent/test_async_token_accounting.py (19 tests: enqueue
ordering, absolute-as-barrier, backlog coalescing with exact sums,
coalesced-vs-sequential row equivalence, merge unit rules, None-cost
preservation, read-your-writes, flush vs stop-flagged/concurrent
drains, inline apply after writer stop, close/atexit durability,
_persist_session drain, writer failure isolation);
tests/run_agent/test_token_persistence_non_cli.py updated to the
queue_token_counts contract.
With HTTP(S)_PROXY (and similar mounted transports), live connections sit
on client._mounts rather than the default _transport. force_close_tcp_sockets
only walked the default pool, so stranger-thread interrupt abort logged
tcp_force_closed=0 and left the request alive for minutes (#72975). Also
walk nested proxy _connection wrappers and WARNING when an abort finds no
sockets.
The auto-continue recovery note was typed only after run_conversation
returned, so its row sat untyped for the whole turn — and permanently
when the continuation was itself killed, which is the case it exists
for. persist_user_display_kind stamps the type on the live message
before the crash persist writes it, in the same insert as the content.
The flush also carries display_metadata through, which it was dropping.
A slash-skill invocation is persisted expanded — activation note plus the
entire skill body. _history_to_messages is the single display projection
every surface reads, so that payload rendered as a chat bubble anywhere a
session was resumed.
Project it here onto the invocation the user typed, and tag skill/bundle
dispatches with the same string so the live send matches. Rewind and
regenerate replay from what the transcript shows, so re-expand the
invocation server-side before running the turn: the replayed prompt is
identical to the original and no client ever holds the body.
A mid-stream steer persists an interrupted-turn checkpoint so the model knows
its reply was cut off. That scaffolding — "[This response was interrupted by a
user correction.]" and the "Visible response before the interruption:" header —
was written straight into message content, so every reload painted the raw
machinery as an assistant bubble (and merged it into the preceding tool-call
bubble). Steered transcripts became unreadable.
Reuse the existing display/replay split instead of inventing new surface:
- Carry the scaffolded form in the server-only api_content sidecar (the exact
bytes replayed to the provider), keep content the user's/agent's real words.
- When nothing reached the screen there is no clean form, so mark the row
display_kind=hidden — replayed to the model, dropped by every transcript
surface, exactly like compaction-reference rows.
- Honor display_kind=hidden in the gateway's _history_to_messages projection
(it only sniffed the [System: convention), so the checkpoint can't leak
through the live/resume path to the TUI/CLI either.
The model still receives the full interrupted context on the wire; the
transcript shows the partial reply and the user's correction.
A mid-stream steer/redirect cancels only the live model request and queues
the correction for a rebuild. But the retry-wait, error-handling, and
backoff-wait paths all treated the cancellation bit as a hard stop:
clear_interrupt() destroyed the pending correction and the turn died with
"Operation interrupted…" — the user's message silently lost. All three
sites now preserve the redirect and rebuild the iteration from it, exactly
like the InterruptedError handler.
tui_gateway also gets the two suppressions its sibling surfaces already
had: the "Operation interrupted: waiting for model response (…)" sentinel
is cancellation metadata and no longer ships as assistant prose in
message.complete (gateway/run.py and ACP already suppress it), and a
leftover pending_steer returned by the turn is requeued as the next prompt
instead of dropped (cli.py and gateway/run.py already do this).
session.steer now records the correction on the inflight turn like
session.redirect does, so a resume/reconnect mid-turn rebuilds the steered
user bubble instead of losing it.
Expanding an @file:/@folder: reference deleted the token from the message
it was typed in, leaving a hole in the sentence and no anchor for clients
to chip. The attached context block still names each ref, so nothing is
lost by leaving the token where the user put it.
The concept 'never send a turn that strict wire validation rejects as
empty' was forked across four sites, each with its own predicate and its
own blind spots:
1. build_assistant_message write-time ' ' pad — broke codex commentary
turns (content:'' is a designed state), and a DB-side pad can't
survive _rows_to_conversation's whitespace strip anyway. REMOVED.
2. conversation_loop send-time ' ' pad — main-loop only (summary path
uncovered), ordering-fragile (had to run after whitespace
normalization), assistant-only. REMOVED.
3. stream-stub '[response interrupted]' substitution — defeated the
loop's empty-stub guard (the stub no longer looked empty, entered
history, and the placeholder leaked into the stitched final
response via truncated_response_parts). REMOVED.
4. repair_empty_non_final_messages in sanitize_api_messages — the
unconditional pre-send chokepoint shared by the main loop AND the
summary path, covers user and assistant turns, non-final only,
copy-on-write. This is now the SINGLE OWNER.
The owner's payload predicate (_msg_has_payload) is extended to treat
codex_message_items / codex_reasoning_items as payload, so
designed-empty codex commentary turns are never rewritten on any
api_mode — the failure shape that broke site 1 in CI is encoded in the
owner, not special-cased at a call site.
Tests updated to pin the new contracts: builder stores textless turns
as-is; the empty stream stub stays recognizably empty for the loop
guard; poisoned resumed histories are repaired to the placeholder at
the send boundary; codex item carriers are never rewritten.
Sabotage-verified: unwiring the owner fails 3 regression tests.
Third layer of the empty-stub fix: full self-recovery. A poisoned transcript
(empty assistant stub or empty user turn already persisted before the write-
time guard, or fed in from a host history) previously 400'd every subsequent
request until it scrolled out — needing a manual DB edit + gateway restart.
sanitize_api_messages() (the unconditional pre-send chokepoint) now repairs
empty non-final messages on the per-call copy by substituting a minimal
'[response interrupted]' placeholder, so the session recovers itself IN MEMORY
on the very next send. The final message is left untouched (empty final
assistant is legal); stored history is never mutated; reasoning-only and
tool_call turns are preserved (negative controls).
Tests: production-shape repro (tool -> empty assistant -> user), empty-user
case, non-destructive guarantee, and negative controls. RED verified by
disabling the wire-in.
Add distinct, greppable log lines at both fix sites so the condition is
observable in the field instead of only inferable from the absence of the old
'Cannot compress further' spiral:
- chat_completion_helpers: warn when an empty partial-stream stub is replaced
with the placeholder (0 chars recovered, no tool call).
- error_classifier: warn when a malformed-body 400 is classified as
format_error rather than context overflow, with num_messages/approx_tokens.
Extend the litellm-shape classifier test to assert the warning is emitted.
A stream that dies after delivering deltas but with 0 recovered chars (and
no captured tool call) produced a content-less assistant stub. That empty
non-final message is invalid for the Anthropic schema, so every later request
400s with 'all messages must have non-empty content' (INVALID_REQUEST_BODY).
On a large session the 400 was misclassified as context overflow, dropping the
loop into a compression spiral that ends in 'Cannot compress further' — a
misleading context-size error on a session nowhere near its limit.
Root cause: substitute a minimal '[response interrupted]' placeholder so the
stub is never empty.
Misclassification: add _INVALID_MESSAGE_BODY_PATTERNS (checked before the
context-overflow heuristic) to classify these as non-retryable format_error,
and teach the body extractors the litellm/Bedrock errorMessage/errorCode/
errorArgs shape so descriptive proxy errors are not mistaken for generic ones.
Adds RED-verified regression tests for the stub path and three classifier tests
(including a guard that real overflows still compress).
Commentary-phase Codex turns persist with content:'' by design (their
text is delivered via the interim assistant callback), and the Responses
wire has no 'assistant must not be empty' validation — padding them
broke test_run_conversation_codex_continues_after_commentary_phase_message
in CI. Both the builder pad and the send-time pad now skip
api_mode=codex_responses. Keying on the ACTIVE api_mode preserves the
repair for codex-written sessions replayed through a strict
chat-completions provider.
A mid-tool-call stream drop with no delivered text produces a
partial-stream stub carrying content:'' and tool_calls=None. The
conversation loop's truncation path appended it to history as
{"role":"assistant","content":""} before the continuation nudge, and
strict providers (Moonshot/Kimi via OpenRouter) reject empty assistant
content with HTTP 400 ("the message at position N with role 'assistant'
must not be empty") on the next replay. Because the message is
persisted, every subsequent turn re-failed — the session was
unrecoverable.
Three layers, smallest blast radius first:
1. conversation_loop (length path): an EMPTY partial-stream stub is no
longer appended as an interim assistant message; only the
continuation user-message is. Stubs that delivered partial text are
still persisted so continuation stitching is unchanged.
2. chat_completion_helpers.build_assistant_message: never serialize a
textless assistant turn with content:'' — pad to a single space, the
same trick as the reasoning_content pad (#15250, #17400). Tool-call
turns are exempt (content:'' alongside tool_calls is accepted
everywhere).
3. conversation_loop send boundary: pad a textless assistant turn's
empty content to a single space AFTER all content-mutating passes
(surrogate sanitize, whitespace normalization, thinking-only drops),
before token estimation. This is the durable repair for sessions
ALREADY poisoned by older builds: the persisted stub rows are rebuilt
to '' on every reload (_rows_to_conversation strips whitespace, so a
DB-side pad can't survive) and only a send-time pad repairs them.
Verified: 485 tests pass across the four affected files; live replay of
a real poisoned session's resumed history against Moonshot via
OpenRouter returns HTTP 200 (was HTTP 400).
Direct tool HTTP calls already identified as Hermes-Agent, but the main
OpenAI-SDK chat path still sent OpenAI/Python. Set Hermes-Agent/<ver> for
api.x.ai clients (xai + xai-oauth) so normal text traffic is attributed correctly.
A /steer redirect during a thinking phase serialized the streamed
reasoning into the persisted assistant checkpoint ('Reasoning shown
before the interruption: ...'). An assistant turn exposing its own
chain-of-thought reads to Anthropic's output classifier as
reasoning-injection/prefill jailbreak, so every subsequent call on the
session deterministically returned 'Provider returned an empty
response' — and because the checkpoint is persisted and replayed, no
retry, nudge, or empty-recovery branch could ever escape it. Four
sessions were permanently bricked this way in the week of Jul 21-27
(42+ blocked calls; every reasoning-free checkpoint that week was
untouched — same mechanism as the prefill.json incident).
Class fix: streamed reasoning is now display-only state. The
_current_streamed_reasoning_text accumulator is removed entirely
(producer in _fire_reasoning_delta, resets, and init), so no future
path can serialize CoT into replayable content. The checkpoint keeps
only the visible response text; the model regenerates its reasoning on
the retried turn. Invariant documented in _apply_active_turn_redirect.
Regression tests: CoT never appears in either checkpoint shape,
reasoning-only interrupts produce a bare checkpoint, reasoning deltas
stay display-only.
The call-block decoration reads agent._use_prompt_caching / _cache_ttl /
_use_native_cache_layout directly; the redecoration helper wrapped each in
getattr with divergent defaults (e.g. or-'5m' vs verbatim _cache_ttl).
The flags are unconditionally initialized on AIAgent, so the defaults
served only test fixtures and would mask a real init bug as silent
cache-off. Align with the house style.
_peel_moa_guidance hand-implemented the inverse of moa_loop's
_attach_reference_guidance from a different module — a drifting separator
or shape would make the peel silently no-op and put the last cache
breakpoint on the turn-varying guidance block (the #72626 bug class).
Move the inverse into moa_loop.peel_reference_guidance directly adjacent
to the attach, keep a thin wrapper in conversation_loop, and pin the
contract with a round-trip test over all three attach shapes.
Also fix the empty-list residue: peeling a guidance-only content part now
drops the whole message (mirroring the appended-user-message shape)
instead of leaving an empty-content user turn behind.
guidance=None is a real prepared shape (all references failed / silent
degraded policy builds prepared_request without attaching guidance), and
the MoA facade sends prepared['messages'] — not api_kwargs['messages'].
Gating the rebase on 'and guidance' left the stale decoration in the
prepared object for the no-guidance MoA sub-path, so #72626 persisted
there. rebase_prepared_request already handles falsy guidance (copies
messages, skips the attach).
The static-prefix reconstruction pattern (build_system_prompt_parts ->
['stable'] -> startswith gate -> fail-open) existed in three copies:
session restore (conversation_loop), compression keep-prompt path
(conversation_compression), and the new failover redecoration helper.
Hoist it into agent/system_prompt.reconstruct_static_prefix and call it
from all three sites.
Also memoize failed rebuilds per stored prompt (_static_rebuild_failed_for):
the redecoration chokepoint runs at the top of every retry attempt, and a
persistent stable-tier mismatch (restored session whose SOUL.md/skills
changed since save) would otherwise re-run the full prompt build — SOUL.md,
context files, memory I/O — on every attempt of every API call for the
life of the session. A legitimately changed stored prompt retries once.
strip_anthropic_cache_control flattened ANY pure-text multi-part content
list with a separator-less join. Decoration only ever produces a single
text part or the 2-part [static, volatile] system split; organic
multi-part text (merged user turns, imported transcripts) got word-jammed
and parts carrying extra keys (citations) were silently dropped — on the
common no-failover path, since redecoration runs on every attempt.
Restrict the flatten to the exact decoration-produced shapes and make
marker removal copy-on-write on part dicts (the per-call message copy is
shallow, so parts alias the persistent history).
try_activate_fallback refreshes the cache policy flags for the new
provider, but the retry loop reused the primary's decorated api_messages.
Cache-off→cache-on shipped zero breakpoints; cache-on→cache-off left
stale markers. Strip and re-render at each retry attempt (same chokepoint
as reasoning-echo reapply), peel/rebase MoA guidance so the last marker
stays off the turn-varying block, and rebuild the static system prefix
when caching becomes active mid-turn (#72626).
Three code-reuse fixes applied during salvage:
1. Reuse _relative_time from hermes_cli/main.py instead of duplicating
the relative-time formatting logic in hermes_cli/status.py.
2. Extract _stamp_hygiene_compression_provenance helper in gateway/run.py
to deduplicate the two nearly-identical try/except blocks that stamp
compression timeout/abort provenance in the hygiene path.
3. Add ContextCompressor.record_timeout_failure() method and use it from
the in-agent compress_context timeout callback instead of re-implementing
the (60, 300, 900) cooldown ladder inline. The existing summary-LLM
exception handler already has this ladder — now both paths share one
method.
Three mechanisms to detect and notify when gateway sessions stall silently:
1. Mid-turn activity heartbeats stamped to SessionDB so hermes sessions list
and hermes status show progress during long turns without new message rows.
2. Stall watchdog: when a busy session has pending inbound and the shared
activity clock is idle past agent.session_stall_timeout (default 300),
log a WARNING and notify the user once to try /new. Notify-only; does
not kill the turn.
3. Compaction timeout: fenceless compress_context callers get a progress-aware
host budget (compression.context_timeout_seconds default 120 idle,
compression.context_total_ceiling_seconds default 600 ceiling). On timeout,
cancel via commit fence, skip compaction without dropping messages, and
continue the turn.
Closes#72016 (slices 1-3; slice 4 cumulative SSE stream-retry deadline
remains a follow-up).
Cherry-picked from PR #72424 by @fangliquanflq.
- codex app-server sibling path: surface a WARNING (was silent debug) when
the projected-message flush fails — same bug class as the main fix, but
codex output has already streamed so fail-closed and agent_persisted=False
are both wrong here (#860/#42039 duplicate-write hazard); loud durability
gap logging instead.
- map session_persistence_failed in _format_turn_completion_explanation so
the user sees an actionable reason instead of 'The request failed:
unknown error' + explainer test.
- contributors/emails mapping for elco@thedaoist.gg (attribution CI).
claude-fable-5 is a Mythos-class reasoning model (1M context, 128K output,
adaptive thinking per anthropic_adapter.py) but was missing from the
_REASONING_STALE_TIMEOUT_FLOORS table. Without a floor entry it got the
default 180s stale timeout (300s with context scaling), which is too short
for fable-5's thinking phase on large contexts.
Each stale kill bumped the cross-turn circuit breaker streak; after 5
consecutive kills _check_stale_giveup() fired immediately (elapsed: 0.00s),
aborting all calls with "Provider has been unresponsive for 5 consecutive
stale attempts." Users with 191K-token contexts hit this reliably.
Add ("claude-fable", 600) — deep-reasoning tier alongside o1/deepseek-r1/
nemotron-3-ultra. The claude-fable slug matches claude-fable-5 and future
variants via the existing right-anchor regex.
