Baseline diff vs clean upstream/main (182 pre-existing environmental
failures on both sides) showed exactly 6 PR-caused failures, all the same
mechanism: tests construct ContextCompressor under a
get_model_context_length mock and read threshold_percent/threshold_tokens
after the with block, or build via object.__new__ and assign
context_length AFTER the derived budgets (the setter now resets the
lazily-cached budgets).
- test_compression_small_ctx_threshold_floor: resolve inside _make()
- test_cjk_token_estimation: resolve inside mock
- test_per_model_compression_threshold: resolve inside mock (2 tests)
- test_pre_compress_memory_context: assign context_length before
threshold/tail/summary budgets; add summary_target_ratio
TestThresholdTokensCap and TestLazyContextResolution landed on main after
the #38991 lazy-init base; they construct ContextCompressor under a
get_model_context_length patch and read threshold_tokens after the with
block. With deferred resolution the probe now fires lazily, so resolve
inside the mock (same pattern as the rest of the suite) and give the
lazy-resolution mock a real return_value.
Self-review found the deepcopy->shallow-copy change in
apply_anthropic_cache_control (#57046) had no test pinning the
"prompt caching is sacred / never mutate the caller's list" invariant.
The old test_returns_deep_copy only exercised the single marked message,
never an un-marked shared reference, so a regression that deep-copied too
little would pass the whole suite.
Add two tests: (1) caller list + every element left byte-identical after
the call, un-marked middle messages returned as shared references, marked
messages fresh copies, and mutating a returned marked message does not leak
upstream; (2) structural byte-equivalence vs a reference full-deepcopy
implementation across both native_anthropic modes and two TTLs.
Mutation-verified: neutering the per-message deepcopy makes test (1) fail.
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)
The structural AST walker in test_compression_session_id_persistence.py
only recursed into control-flow children found via iter_child_nodes(stmt).
When a session_entry.session_id assignment lives inside an `else` block
whose statements are all assigns (no nested If/Try/etc to trigger
_walk_node), the assignment was invisible to the walker and the test
florped with 'No assignments found'.
Walk the stmt itself when it is a control-flow node so its
body/orelse/finalbody (and Try handlers) are always expanded, regardless
of whether iter_child_nodes yields an inner control-flow child.
Manual /compress already persists the rotated child transcript first and
only then repoints the live session_entry; a False rewrite_transcript
return keeps the entry on the original session_id so the conversation
stays reachable. Session hygiene auto-compress did the opposite: it
rebound session_id (and lease/topic) first, then called rewrite_transcript
without checking the return value. On a failed write the live entry
already pointed at an empty child SID and the turn continued — permanent
silent conversation loss. Persist first; rebind only after success.
Phase 2 review found two sibling sites with the same bug class:
- truncate_before_user_ordinal in prompt.submit counted display_kind
timeline rows as user turns, shifting the truncation target
- rollback.restore used the old pop-loop pattern that would pop a
display_kind marker instead of the last real exchange
Both now use the same predicate (role==user and not display_kind)
matching list_recent_user_messages, /undo, /retry, and CLI resume.
Added tests for both paths.
list_recent_user_messages and the in-memory /retry + session.undo walkers
treated every role=user row as a real user turn. Timeline bookkeeping
(model_switch, async_delegation_complete, auto_continue, hidden) is stored
that way, so /undo soft-deleted from a marker and /retry re-sent opaque
bookkeeping text. Exclude display_kind the same way CLI resume counting and
the prompt.submit ordinal path do.
Replaces the O(N) list_sessions_rich histogram in /api/sessions/stats
with a single GROUP BY query, reducing response time from ~575ms to
<1ms on large databases.
Original PR #48921 by @liuhao1024. Salvage fixes based on review
feedback from teknium1 and @wernerhp:
1. Preserve try/except guard — a DB error still degrades to empty
by_source instead of failing the whole stats response.
2. GROUP BY COALESCE(source, 'cli') — the original GROUP BY source
could emit duplicate 'cli' keys (NULL group + literal 'cli' group)
that the dict comprehension silently dropped.
3. Add exclude_children=True — list_sessions_rich excludes subagent
runs, delegates, and compression continuations by default; the
bare GROUP BY counted all rows, inflating source counts.
Aggregate shape (exclude_children/include_archived/limit params)
adapted from closed duplicate #61120 by @mijanx.
Closes#48914
Co-authored-by: mijanx <mijanx@users.noreply.github.com>
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.
Follow-up to PR #64171. The _TOKEN_DELTA_* classification must exactly
cover update_token_counts' keyword surface: an unclassified kwarg is
silently kept only from the first delta of a merged run. Introspect the
live signature and fail with a pointed message when a future kwarg is
added without classification (or a classified field is removed).
Follow-up to PR #64171. The writer loop and flush_token_counts'
caller-drain both set _token_writer_busy BEFORE popping the queue —
that ordering is what makes flush's lock-free fast path (reads queue
then busy, no cond held) sound. _stop_token_writer's leftover drain did
it backwards (clear queue, then set busy), leaving a few-bytecode window
where a concurrent flush could observe 'empty and idle' and return True
with the popped batch still unapplied. Shutdown-only staleness, no data
loss — but the protocol now matches at all three drain sites.
Test: concurrent flush during a stop-drain mid-apply must time out
(False), never report drained.
Follow-up to PR #64171. Two writer-death hardening gaps:
- queue_token_counts only spawned the writer when the thread object was
None, so a writer that died from an unexpected exception could never be
replaced: deltas piled up on the uncapped deque until a reader's flush
drained them synchronously. Respawn on 'not thread.is_alive()' instead.
(atexit re-registration on respawn is safe: unregister removes all equal
bound-method registrations and the drain hook is idempotent.)
- _coalesce_token_deltas ran outside the per-delta try/except in
_apply_token_batch, so a merge bug (e.g. an unclassified future kwarg
summing None + 0) would escape and kill the writer thread. Wrap it and
fall back to applying the raw batch — coalescing is an optimization,
never load-bearing.
Tests: coalesce-failure fallback + dead-writer respawn.
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.
Hard 16 MB cap on readFileDataUrl blocked larger local attaches with no way to raise it. Settings -> Chat now has a free-form MB field. Main process owns the persisted value and clamps only absurd inputs.
Critical fixes to salvaged PR #73020:
- Use SessionDB.append_message instead of raw INSERT INTO messages.
The original used wrong column names (session_key/created_at vs
session_id/timestamp) and bypassed FTS indexing, session metadata
updates, display_kind, and all other columns append_message handles.
- Use get_hermes_home() instead of hardcoded Path.home()/'.hermes'.
Profile-aware path resolution under HERMES_HOME override and active
profile isolation.
- Add 11 tests covering flush, recovery, serialisation, edge cases.
When FTS5 index corruption prevents INSERT INTO messages, the gateway
accumulates messages in _pending_messages (memory-only). On shutdown,
.clear() discards the only surviving copy — permanent user data loss.
Changes:
- Add gateway/shutdown_flush.py with flush_pending_to_file() and
recover_pending_to_db() for two-phase data preservation.
- Patch gateway/run.py: flush runner._pending_messages before clear()
in _stop_impl_body, and recover on startup after runner.start().
- Patch gateway/platforms/base.py: flush adapter._pending_messages
before clear() in the adapter shutdown path.
Recovery behavior:
- Reads pending JSON files from ~/.hermes/pending_messages/
- Inserts messages into state.db directly
- Per-session isolation: one corrupt session doesn't block others
- Successful recovery deletes the flush file
- Failed recovery re-saves for next startup retry
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.
- Change await self._send_read_receipt to asyncio.create_task to avoid
blocking message dispatch on slow bridge responses (matches BlueBubbles
pattern). Up to 5s per-message latency eliminated.
- Update tests: assert_called_once_with instead of assert_awaited_once_with
since the receipt is now scheduled, not directly awaited.
- Add send_read_receipts documentation to whatsapp.md following the
BlueBubbles docs pattern.
- Replace _load_cron_jobs_for_config_warning with lazy import of
cron.jobs.load_jobs — picks up BOM handling, corruption repair,
and context-local store resolution for free
- Re-add model.name to axis mapping (was dropped during merge);
model.name is a legacy alias for model.default
- Fix grammar: '1 enabled unpinned cron job have' -> 'has'
- Pass user_config to cron_model_drift_guard_enabled in set_config_value
to avoid a redundant load_config() re-read of the file just written
When an operator changes the global model/provider config, warn that
unpinned cron jobs with stored snapshots will fail-closed on their next
run. Adds a cron.model_drift_guard config opt-out (default true) for
fleets that should deliberately track changing global defaults.
Addresses #59031. Original PR #59177 by @doncazper.
Skip hashing active copies when the bundled origin is unchanged, build rename and optional migration indexes lazily, and reuse one optional-skill directory index per sync.
Add regression coverage for unchanged copies, deferred modification detection, lazy rename recovery, and optional provenance scans.
A session is meant to be either the main thread or a tile, never both.
openSessionTile enforced this from the tile side (refusing to tile the
selected session), but the main side never dropped an existing tile — so
any path that routes main to a session that's already tiled (cold-start
remembered-session restore, a pasted/Cmd-K route, a notification jump)
painted the same transcript twice: the workspace pane from the route and
the tile pane in parallel, both fighting one runtime.
resumeSession is the single chokepoint every load-into-main path funnels
through, so close the redundant tile there once the session becomes the
selection. The warm cache/runtime binding survives for main to reuse.
Mirror the picker dropdown's provider collapse into the Edit Models dialog so
curating is one click per provider instead of scrolling through 30 models.
Each provider header is a full-width clickable label (same style as the
composer context-menu labels) with a DisclosureCaret next to the text and a
select-all Checkbox (indeterminate when partial). Model rows stay Switches.
The dropdown's collapse is fixed too: the current provider is now
collapsible (was forced open), and the label style matches the rest of the app.
Adds a search icon to the dialog input matching every other search field.
The subpath entry existed only so the TUI could import the client-side
projection fallback. The TUI spawns its gateway from this same checkout and
cannot version-skew with it, so that fallback was dead weight — it reads
`display` directly now. With its last caller gone the dispatch helper folds
back into the desktop, where an older backend is genuinely reachable.
apps/shared/package.json is byte-identical to main again.
Pass the display type into the turn so the row is born typed, and
recognize the note's fixed prefix in _history_to_messages so the
untyped rows already on disk stop painting as user bubbles.
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.
