Review-pass follow-ups (three parallel reviewers, findings verified):
- hermes_state_search.py list_recent_user_messages now drops legacy
standalone compaction handoffs in the decode loop (SQL can't see them:
durable role=user, no display_kind). Closes the /undo N pairing skew
where the in-memory count (new predicate) and the DB soft-delete pick
(old predicate) targeted different turns on legacy sessions. Fetches
with headroom so the requested limit is still honored. 3 new tests,
mutation-checked (no-op'ing the skip fails 2/3).
- _should_skip_model_call_for_reference_handoff: single drive-check scan
(was two — once inside the restore helper, once after); the restore
helper no longer re-scans and its return value now decides the verdict.
- _final_response_from_messages replaced by the _HANDOFF_SKIP_FINAL_RESPONSE
constant it always returned (parameter was unused).
- _handoff_carries_live_user_content delegates to the canonical
_strip_context_summary_handoff_message — also fixes the edge where a
merged-shaped row with an EMPTY preserved prior tail was wrongly
treated as carrying live content.
- Site-level guard test for rollback.restore with a legacy handoff row
(predicate-in-context, complements the unit tests).
Follow-ups on top of the salvaged #80696 fix (review findings):
- Sibling sites: rollback.restore, gateway /retry, CLI /retry and /undo N,
and both CLI resume turn counters now use is_user_originated_turn so
legacy-persisted standalone handoffs (durable role=user, no display_kind)
can never be truncation targets or counted as user turns (#80622
suggested regression 4, dispatcher-wide).
- Site-1 guard: hoist the api_call_count decrement + iteration-budget
refund above the break so a skipped turn no longer leaks a budget unit
and finalize_turn logs the true call count (matches the ollama early-exit
and the site-2 sibling).
- Site-2 guard: run the handoff guard BEFORE reanchoring so a restored
user ask is what the anchor lands on, not a stale pre-restore index.
- SUMMARY_PREFIX: add the mid-tool-loop carve-out the code-side guard
already implements, so a literal-minded model doesn't halt an in-flight
exchange after in-place compaction.
- Skip path returns a short compaction status instead of replaying the
previous turn's answer (finalize_turn would append it as a fresh
assistant row — duplicate prose in transcript and delivery).
After a completed assistant stop, a standalone CONTEXT COMPACTION handoff
could occupy the sole user slot and resume stale Historical Task Snapshot
work with no new human ask. Guard post-compaction continues, hide
standalone handoffs from session dispatch, and harden SUMMARY_PREFIX for
the empty-after-handoff case (#80622).
The rough preflight estimate intentionally overestimates, but not by a
fixed margin: CJK text is counted at ~1.7x its o200k cost and
Responses-mode reasoning replay blobs at several times their billed
cost. Heavy sessions show rough estimates 2-3x real usage and compact
at 35-55% of the real window, stalling turns for minutes and discarding
detail (churn), because the defer guard only tolerated 5% rough growth
and sessions that never compressed had no baseline at all.
Pair every request's rough estimate (note_request_rough_estimate,
recorded in the conversation loop right after the pressure estimate)
with the provider's real prompt_tokens in update_from_response(), then
defer preflight while projected real usage — last real + rough growth
since that reading — stays under the threshold. Rough growth is itself
an overestimate of real growth, so the projection is an upper bound and
deferring below the threshold is safe; the provider's context-overflow
handler remains the backstop.
The baseline no longer ratchets on defer: it is refreshed by the
response pairing, and advancing it without a matching real reading
would shrink apparent growth and defer on stale data.
The api_messages build used a shallow msg.copy(), decoupling only
top-level fields. Every nested container (tool_calls entries and their
function dicts, multimodal content-part lists, reasoning_details) stayed
aliased to the persisted history, so ANY in-place transform on the send
copy silently rewrote the stored transcript.
Probed every send-path transform against that aliasing shape on main:
content strip loop safe (top-level reassign)
_canonicalize_api_tool_calls (repair) LEAKED <- #80616's fix
_sanitize_messages_surrogates LEAKED (multimodal parts,
tc ids/args, reasoning)
_sanitize_messages_non_ascii LEAKED (multimodal parts)
_sanitize_api_messages safe
_drop_thinking_only_and_merge_users safe
The retry loop already believed the copies were independent - it
sanitizes messages AND api_messages separately (~L3555) - so the
aliasing was accidental everywhere.
Fix at the chokepoint: _clone_message_for_send clones every container
(dict/list) recursively while sharing immutable leaves, so every
downstream in-place transform - current and future - is safe by
construction. Cost is container-count, not string-bytes: 100KB argument
strings and base64 payloads are shared (measured ~0.5ms vs ~0.1ms per
1500-message build; noise next to one json round-trip). Same clone
applied to the prefill-message insert (same class, same pipeline).
The class-wide invariant test runs the full send-path transform
pipeline over an adversarial fixture (malformed args, surrogates,
non-ASCII, multimodal parts, reasoning fields) and asserts the history
stays byte-identical; an AST contract pins the build-site wiring so the
shallow copy can't quietly return. Both mutation-verified: reverting
the clone to shallow fails 4 isolation tests, unwiring the build site
fails the AST contract.
0xGr1mm's branch fix (previous commit) remains as defense in depth at
the exact site the #80498 incident hit; his regression tests and the
class-wide invariant give layered coverage.
`_canonicalize_api_tool_calls` promises copy-on-write in its own docstring
— "the persisted history is untouched" — and the call site repeats it:
"Operates on api_messages (the API copy) so the original conversation
history in `messages` is untouched."
The canonicalize branch keeps that promise (`tc = {**tc, "function": {...}}`).
The repair branch does not:
except Exception:
tc["function"]["arguments"] = _repair_tool_call_arguments(...)
`api_messages` is built with `msg.copy()` — a SHALLOW per-message copy — so
every `tool_calls` entry is the same dict object the persisted history
holds. Assigning into `tc["function"]` therefore writes through to the
stored turn. The sibling loop two lines above only touches `am["content"]`,
one level deep, which is why the aliasing never showed up there.
On the unrepairable path `_repair_tool_call_arguments` returns "{}", so
that write replaces the model's real arguments with an empty object in the
transcript. A stream that dies mid `write_file` loses the file content it
had already streamed — the reported symptom in #80498, where a chapter
draft was silently reduced to `{}` and only a WARNING remained:
Unrepairable tool_call arguments for write_file — replaced with empty
object (was: {"content": "# 骨架-第25章\n> 承接...)
Mirror the canonicalize branch: build a new tool-call dict instead of
assigning into the shared one. The API copy still carries "{}" — the
repair's whole purpose is to never ship broken JSON — but the history keeps
what the model actually sent, so the transcript, session persistence and
any later retry still have it.
The in-place write was not an oversight in isolation: it predates the memo
refactor, which preserved it deliberately for byte-parity. The existing
`test_history_not_mutated` asserts exactly this invariant but restricts
itself to valid arguments, and its docstring records the gap — "(Malformed
args take the in-place repair path — pre-existing behavior)". That is why
a test file whose header already claims "the persisted history is never
mutated (copy-on-write preserved)" stayed green through the bug.
Four tests close it: history keeps the original bytes, the send copy is
still repaired, a broken call does not disturb its siblings, and repeated
sends stay lossless. On unpatched main three of them fail; the parity and
complexity tests are unaffected because the difference is only observable
when the history list is separate from the send copy — which is the shape
production uses.
Refs #80498
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Selecting an NVIDIA NIM model whose id reached config without the nvidia/
prefix produced a bare "HTTP 404: 404 page not found" — retried three times,
never naming the model. It reads exactly like an outage or an auth failure,
which is where the Discord thread spent its time before the id was spotted.
normalize_model_for_provider() had no branch for nvidia, so a bare id passed
straight through to the API. Repair it from the provider's curated catalogue:
a bare name that matches exactly one entry modulo the prefix gets it back.
That's a lookup, not a guess — build.nvidia.com also fronts local NIM
containers and third-party models, and anything absent from the catalogue is
left alone. Because the repair runs on every runtime setup, an already-broken
config self-heals on the next turn and prints what it changed.
If a bare id still reaches the wire, the 404 now explains itself. The
classifier consults the same catalogue: a prefix-less id the provider only
serves as vendor/model is a deterministic failure, so it classifies as
model_not_found instead of burning three retries on a retryable "unknown",
and the error trace names the id to use.
Fixes#78796
* fix(agent): adopt .env credential/base-url edits at the turn boundary
A Settings save (desktop PUT /api/env, hermes setup) updates .env and
the saving process's os.environ, but a live session worker keeps the
base_url/api_key captured at agent init until restart — an open chat
silently kept calling the old endpoint (e.g. a local-server key sent to
api.openai.com, failing with an opaque 401).
Add AIAgent._try_refresh_env_client_credentials(), called at the start
of each conversation turn: re-resolve the provider's env-sourced
credentials (load_env() is mtime-memoized, so an unchanged file costs
one stat()) and rebuild the client via the existing
_replace_primary_openai_client machinery when the user edited them.
The refresh reacts only to env edits — resolved values changed since
the last look — never to mere divergence from the agent's current
values: credential-pool rotation and failover legitimately move the
session off the env credential, and stomping those back would flap.
Config model.base_url / pool custom endpoints keep precedence: edits
are only adopted while the session still runs on the registry default
or the previously-seen env value.
Lift _get_env_prefer_dotenv out of _seed_from_env to module level
(get_env_prefer_dotenv) so both the pool seeder and the per-turn
refresh share the same .env-over-os.environ resolution, including the
op:// indirection handling.
Fixes#67821
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(agent): address sweeper review on env credential refresh
- Cover named custom providers (#67935): provider="custom" has no
PROVIDER_REGISTRY entry, so resolve the config block's key_env through
the same lookup the runtime resolver uses.
- Make the edit baseline transactional: a failed client rebuild rolls the
agent back and leaves _env_creds_seen un-advanced so the unchanged edit
is retried next turn.
- Recompute route-derived TLS material and default headers on a base-url
change, via a _reapply_route_client_config helper shared with
credential-pool rotation so the two paths cannot drift.
- Rebase onto main: get_env_prefer_dotenv keeps the scoped _get_secret
semantics from the profile-isolation fix (no raw os.environ reads).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* chore: map jskang@lablup.com to rapsealk
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Brooklyn Nicholson <brooklyn.bb.nicholson@gmail.com>
The output-cap error handler already computes request_input_estimate at
line 4722 via estimate_request_tokens_rough(api_messages, tools=...).
The new compression block ~50 lines below was calling the same function
with the same inputs again. Reuse the existing local.
The output-cap retry loop reduced max_tokens by 64 tokens per attempt but
never called _compress_context(), so the compressor never fired. Input
growth (~65 tokens/attempt) canceled the savings, leaving the session
stuck at 200,001 tokens — 1 over the 200,000 ceiling.
The fix adds compression to the output-cap retry path. The compressor
drops the middle window, freeing ~50% of tokens. If compression makes
>=5% savings, the session continues; otherwise vision payloads are
stripped or the session ends with compression_exhausted=True.
Also adds CHANGELOG.md entry and bug fix report.
The bracketed-marker regex was inlined in conversation_loop.py as
re.fullmatch(r"\[...", ...) while hermes_state.py defines the same
pattern as _STALE_TOOL_CALL_MARKER_RE. Both must agree on what counts
as a stale marker — a drift here means the runtime guard silently
disagrees with the load-on-read repair and CLI purge in hermes_state.
Consolidate onto a single compiled constant (_STALE_MARKER_RE) at
module level in conversation_loop.py, with a comment noting it must
mirror _STALE_TOOL_CALL_MARKER_RE in hermes_state.py. A direct import
from hermes_state was tried first but caused a regression: hermes_state
initializes DEFAULT_DB_PATH = get_hermes_home() / 'state.db' at module
import time, which breaks tests that monkeypatch get_hermes_home() to
return a str (test_slash_worker_accepts_profile_home).
Follow-up to PR #78175 (@JoaoMarcos44).
Local tool-call templates can emit a bare bracketed token (e.g. "[memory]")
as assistant content alongside a function call. The loop treated that
protocol scaffolding as visible content: it got cached as the post-tool
fallback, and when the next turn came back empty, the marker was replayed
as the final response and written into the persisted transcript. Later
context compaction preserved that history, letting the model repeat the
marker in subsequent turns.
Detect content that is only a bracketed marker (`[name]`) when the
response also carries tool_calls, and drop it before it can be cached
or persisted. Scoped narrowly: only fires alongside tool_calls, so a
genuine final response of "[memory]" without a tool call is unaffected.
Empty content retries previously fired back-to-back with no delay,
wasting up to 3 rapid API calls, and could not be cancelled mid-wait.
Apply the same jittered_backoff() already used for rate-limit and
API-error retries, sleeping in small increments so a user interrupt
aborts the wait instead of blocking until it elapses.
Fixes#35230
Post-tool compression path passed context_compressor.last_prompt_tokens (0 in the no-usage
fallback) to _compress_context instead of the overhead-aware _real_tokens computed just above
— same tool-blind bug as the overflow handlers (upstream PR #77169 review, teknium1). Also adds
production-path regression tests asserting the 413, context-overflow (two wordings), and
Anthropic long-context recovery handlers pass estimate_request_tokens_rough(..., tools=...)
(sentinel-patched) to _compress_context.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Root fix (Option A) for design-session "Context compression exhausted" crashes. The three
compression-retry handlers after an API overflow/413/long-context error (conversation_loop.py
~4229/4488/4747) passed the tool-BLIND messages-only estimate (approx_tokens) to _compress_context,
so hermes-lcm's forced-overflow recovery armed on the message count and missed overflows driven by
tool-schema/system overhead. Now they pass estimate_request_tokens_rough(api_messages, tools=...)
— the same overhead-aware estimator already used at :4580 — so recovery arms on the TRUE request
size; LCM's _overflow_recovery_assembly_cap self-subtracts the overhead so the full request fits.
Empirically validated on real failed session 6dddf1a67b76 (LCM engine, floor=24000/cap=248000):
observed 256,359 >= 248,000 -> arms; recovery 231,313->208,559 msg-tokens -> full request
233,605 < 272,000 FITS. Prior messages-only path did NOT arm (231K < 248K) and crashed.
Durable copy: ~/.hermes/local-patches/optionA-overflow-overhead-aware.patch (survives hermes update
reset). Upstream PR pending. classify_api_error call at :3667 intentionally unchanged (not recovery).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review follow-up on the #76098 salvage: the 4096-entry count bound alone
doesn't bound MEMORY — write_file/patch argument strings run 100KB+, so
a long-lived gateway process under sustained heavy write workloads could
pin ~800MB of evicted-session strings. A 32MB byte budget extends the
existing FIFO eviction; common-case args (0.5-2KB) never hit it. New
guard test mutation-checked (fails with the byte leg disabled).
The pre-send normalization pass re-canonicalized every historical tool
call's argument JSON on every API-call iteration — quadratic in session
tool-call count. Route it through a bounded value-keyed memo (the
_MSG_TOKENS_CACHE idiom from agent/model_metadata.py): per-iteration
cost is now proportional to new tool calls, not all of them.
Measured (simulated growing session, repo venv): session-total
canonicalization cost 1056 ms -> 58 ms at 500 tool calls x 2 KB args
(18.3x), 5744 ms -> 79 ms at 500 x 16 KB (72.5x). Byte-parity with the
pre-fix logic is asserted at every iteration (unicode, nested,
malformed, empty, non-string args), and a call-count test proves
json.loads invocations went from K(K+1)/2 to K per session.
- Bound ALL reads of the on-disk JWT store through one _read_jwt_store()
helper (load, eviction, save-merge) — the 1 MiB cap previously only
covered the load path; eviction and save could still parse an
oversized/corrupt store and rewrite it back out (sweeper finding).
- Fix the class, not the site: the recovery gates checked the literal
provider == "copilot" while /model and profile configs can leave the
alias spelling in place (the reporter's own log shows provider=copilot
AND provider=github-copilot in one session — the aliased turns would
have silently skipped recovery). Single owner:
AIAgent._is_copilot_provider() (slug aliases + Copilot base-URL
fallback), used by both run_agent recovery methods and both
conversation_loop gates.
- Update the salvaged 401 test to current main's client-retirement
contract (release deferred to GC — no synchronous .close()).
- Add copilot_stale_cred_retry_attempted to the TurnRetryState field
contract test; add bounded-store and alias-gate regression tests.
Copilot degrades in two related ways that both abort a turn as non-retryable
and only clear on a gateway restart (a cold process re-runs the token exchange):
1. HTTP 400 model_not_available_for_integrator / model_not_supported — a
raw/degraded token routes to the restricted copilot-language-server
integrator whose allowlist omits enterprise-only models (e.g.
claude-opus-4.8). Because it is a 400 (not 401), the existing 401 refresh
path never fired. Prevented (retry-with-backoff exchange + on-disk JWT
persistence + header guard at the client chokepoint) and self-healed at
runtime (single-shot forced re-exchange + client rebuild + retry before
fallback).
2. HTTP 401 'IDE token expired: unauthorized: token expired' — the short-TTL
*exchanged* IDE token expires mid-turn. The clean-401 path DID fire and call
_try_refresh_copilot_client_credentials(), but that method only re-resolved
the stable raw ghu_ token and rebuilt the client — it never evicted the
cached exchanged JWT or forced a fresh exchange, so the retry put the SAME
expired token back on the wire, 401'd again, and the single-shot guard
aborted the turn. Fix: force a fresh IDE-token exchange (evict cached JWT via
evict_cached_exchanged_token + re-mint via get_copilot_api_token) before the
client rebuild, mirroring the merged auxiliary-path recovery (#59837) and the
400 recovery in this same PR. Graceful fallback to the resolved token if the
exchange endpoint is unreachable; picks up the enterprise base_url on
re-exchange.
Brings main-loop clean-401 recovery to parity with the merged auxiliary path
(#59837), using the newer on-disk-aware evict helper. Companion context: #58743
(this PR, expanded), #51313, #63204 (which assumed the 401 path already
recovered — it reached the method but the method was too weak).
Tests: exchange retry/persist round-trip, restart-blip disk reuse, stale-cred
400 classifier, 400 recovery, and 3 new 401 cases (fresh exchanged token on the
wire; network-blip fallback to resolved token). 58 copilot tests green on
current main.
Reactions live in the existing messages.display_metadata JSON column (no new
table), with iOS Tapback semantics enforced DB-side: one reaction per author
per message, re-tap retracts, different emoji replaces. The desktop catches up
to the reaction contract five platform adapters already ship.
- SessionDB: set/get_message_reaction, latest_message_row_id (role + offset +
require_text so invisible tool-call-only rows are never targeted),
take_unseen_reactions (announce-exactly-once), get_message_role
- message.react RPC: accepts row_id or newest_role for live messages that
haven't learned their durable id yet
- react_to_message tool: desktop-gated (check_fn), defaults to the user's
latest visible message, messages_back for retroactive reactions
- Model context rides run_message only (beside the speech-interrupted note):
the persisted prompt stays clean, so no [The user reacted …] scaffolding in
transcripts, and no cached prefix ever changes
- Resume projection forwards row_id + reactions; _row_id is stripped from
outgoing API copies next to display_metadata
Three provably-safe optimizations for O(n)-per-iteration history walks:
1. sanitize_tool_call_arguments: optional identity-keyed cursor (strong
refs to the exact validated message objects) skips re-json.loads-ing
already-validated history each loop iteration. Any list rewrite
(compression, repair, undo, steer) breaks the identity prefix match
and forces re-scan from the divergence point. Wired via a per-agent
cursor dict in conversation_loop.
2. estimate_messages_tokens_rough: per-message memo keyed on a deep
identity fingerprint (strings pinned by strong reference so id()
aliasing is impossible; scalars by value; dicts/lists structurally
with key order). Equal fingerprints imply identical str(shadow)
bytes, hence identical estimates. Unfingerprintable shapes fall
through to direct compute. Bounded FIFO cache (4096 entries).
3. _flush_messages_to_session_db_unlocked: bounded scan that skips the
identity-matched prefix of the previous successful flush's snapshot.
Snapshot only taken on full success; cleared on exception. Compression
rewrites use fresh copies, breaking identity and forcing full re-scan.
Parity proven in tests/agent/test_cursor_optimizations_parity.py:
500-message synthetic histories with tool calls, malformed args, unicode,
element-wise old==new across 3 iterations incl. simulated compression.
Measured (median of 5): sanitize 0.097ms->0.011ms, tokens 1.145ms->0.853ms,
persist-scan 179.5us->10.0us at 500 messages.
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.
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 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.
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.
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).
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.
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).
kshitij
HexLab98
Alan Hsu
kshitij
Gr1mmJ4w
Ryder Freeman
brooklyn!
Jeongseok Kang
BobClawblaw
Hermes Agent
joaomarcos
arimu1
LFDM Core
AlexFucuson9
spfcraze
Rod Boev
Xipong
Gille
Teknium
dstkwll
Alex Fournier
Soju06
Jeffrey Quesnelle
0xprincess
elcocoel
rob-maron