Fix: _LENGTH_CONTINUATION_DROPPED_TOOLS_PREFIX ended with '(' but
_get_continuation_prompt still had f'({tool_list})', producing
'((write_file)' instead of '(write_file)'. Removed the '(' from
the prefix constant — the parenthesis belongs in the interpolation.
Widened: promoted the empty-response nudge (line 6993,
'You just executed tool calls but returned an empty response...')
to _EMPTY_TOOL_RESPONSE_NUDGE constant and added it to the
classifier's recognition set. Same bug class — its
_empty_recovery_synthetic metadata flag doesn't survive SessionDB
projection either.
Test: added parametrize case for the empty-response nudge (7→8 cases).
E2E: verified byte-for-byte string equivalence for all nudge constants.
aed114a69 taught _is_synthetic_compression_user_turn to recognize the
max-iteration nudge as ephemeral runtime scaffolding rather than a human
turn, since its role="user" metadata flag doesn't survive SessionDB
projection and a crash/interrupt mid-turn can persist it durably — becoming
the compaction anchor / auto-focus topic in place of the real task.
conversation_loop.py's retry loop appends several more role="user" rows
with the exact same "ephemeral, metadata-tag-only" shape, none of them
recognized by the classifier:
- The three _get_continuation_prompt variants (length-continuation nudge,
tagged _length_continuation_nudge) — two fixed strings plus a third that
interpolates the dropped-tool-call list.
- _CODEX_INCOMPLETE_NUDGE (codex/responses reasoning-only retry).
- The codex ack-continuation nudge (acknowledgment-only reply re-prompt).
- The dropped-tool-call nudge (tagged _dropped_toolcall_nudge) — persisted
across up to 3 consecutive retries before the finalization pop-loop
strips it; an interrupt/crash before that pop can persist it same as the
max-iteration case.
Promote the previously-inline nudge strings to named module-level constants
in conversation_loop.py (single source of truth for both construction and
recognition), then extend the classifier to recognize all of them — exact
match for the five fixed-content nudges, a stable-prefix check for the
dropped-tool-call continuation variant (its tool list is interpolated so it
can't be exact-matched, same treatment TODO_INJECTION_HEADER already gets).
Imported lazily inside the classifier to avoid a module-load-order cycle —
conversation_loop.py already imports FROM context_compressor.py at call
time for the same reason.
Follow-ups from review of #82049:
- extract append_user_instruction() into agent/skill_commands so the
stable-prefix construction cannot drift between the skill and cron
builders (the registered prefix must stay a byte-prefix of the built
message); cron no longer imports the private _SINGLE_SKILL_INSTRUCTION
- add the startswith guard to the skill builder registration site,
matching the stronger cron guard
- rename _MAX_BYTES to _MAX_CHARS (sum(map(len, ...)) counts characters,
not bytes) and correct the comment
- collapse find_stable_prefix's two-lock dance into a single critical
section (scan is <=32 short-circuiting startswith calls, measured
2-4us; drops the snapshot copy and the TOCTOU re-check)
- document the split-shape lifetime (marked-endpoint window) in the
module docstring
- add a contract test for the helper's byte-prefix invariant
(mutation-checked)
Follow-up review of the builder-declared cache boundary (#81867) found three
ways the split could silently stop paying off, or keep paying more than it
should, on a long-lived gateway process.
Flattening no longer consults the registry. `strip_anthropic_cache_control`
matched the decorated split by looking the first block up in the prefix
registry, so a mid-turn failover that re-decorates a request built many
messages earlier (#72626) would fail to flatten once _MAX_ENTRIES newer
scaffolds had been registered in between, and would hand the next provider
the two-part shape instead of the canonical string. The split is now matched
by its shape: a marker on the *first* part of a user message is something no
other decoration produces (list content otherwise gets its marker on the last
part, and the two-part [static, volatile] split is role-gated to system), so
the ""-join stays provably byte-exact without any process state. This drops
`is_registered_stable_prefix` and one lock acquisition per stripped message.
Lookups now refresh LRU position. A scaffold fired every minute by cron could
be evicted by a burst of one-off skill invocations while still being the
hottest prefix in the process, silently reverting it to whole-message caching.
Registration now also evicts by total retained bytes (4 MiB). Entries hold
whole expanded skill bodies, so a 32-entry cap alone does not bound memory.
The newest entry is always kept, so a single oversized scaffold still gets a
boundary instead of disabling the split.
Tests: eviction-then-failover round-trip, LRU refresh on hit, byte-cap
eviction, and oversized-single-entry survival.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Webhook/cron skill invocations concatenate a large static scaffold
(activation note + expanded skill body) with a small volatile tail
(ticket payload, timestamps) into one user string, and the Anthropic
cache planner marked that whole string as a single atomic block — so a
few changed tail bytes forced a full cache rewrite on every invocation.
Instead of re-parsing scaffold marker strings out of the message at
request time (fragile when a payload or skill body quotes the marker),
the builders now register the exact stable-prefix bytes in a small
process-local LRU registry at construction time. The cache planner
splits a registered user string into [marked stable prefix, unmarked
volatile tail] request-locally; canonical session history stays a plain
string, and the failover stripper flattens the split back byte-exactly
via an O(1) registry lookup. Unregistered messages keep the existing
whole-message policy.
Covers the single-skill builder (webhook + slash command + TUI) and the
cron job prompt assembler (multi-skill, bundles, skipped-skill notice),
with registration guarded against injection-scanner sanitization.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Folds the model-switch fix in with the untitled retry. They answer
different halves and each is wrong alone: counting alone left a session
that merely opened with machinery nameless forever, because nothing
reconsidered it, and the stored title alone would never title at all on a
store too old to report one. Skip only when both agree — past the opening
turn, and already named.
Counting a turn now judges a multimodal one on its text, so "here's a
screenshot, fix the login" counts as the question it is rather than
reading as machinery and undercounting the conversation.
Co-authored-by: yy28 <yy28@vip.sina.com>
Switching models before sending the first real message titled the session
"[System: The active model for this chat has…" instead of the user's actual
question.
`_append_model_switch_marker` persists its notice with `role="user"` because
strict OpenAI-compatible providers reject a system message that is not first
(#48338). Titling had no way to tell that apart from a genuine opening turn,
which caused two distinct failures:
1. `_MACHINE_PREFIXES` did not cover the marker. Its `[System: ` prefix
matches none of `[CONTEXT COMPACTION`, `[Runtime note:`, or `[SYSTEM]`
(different case, no closing bracket), so `is_titleable_user_message()`
returned True and the marker was formatted into the title.
2. `maybe_auto_title()` counted the marker as a user message. With the marker
present, the first real question arrived at `user_msg_count == 2` and the
`> 1` guard returned early, so the session was never titled at all and its
`title` column stayed NULL. Fixing only (1) would therefore have traded a
wrong title for a permanently missing one.
Add the marker prefix to `_MACHINE_PREFIXES` (kept in sync with
`tui_gateway.server._MODEL_SWITCH_MARKER_PREFIX`) and count only titleable
user messages when detecting the opening turn.
The guard stays narrow: ordinary user text that happens to start with
"[System:" still titles normally.
Adds 6 regression tests, verified to fail without the fix.
Two lookalike gaps found auditing the titler.
_MACHINE_PREFIXES missed the compressor's legacy summary opener and the
"[System note:" injections, so a compacted or resumed session could be
named after the note that carried it. Take the summary prefix from the
compressor that emits it rather than keeping a fourth local copy.
The fast-model exclude list covered embedders but not the other non-chat
siblings a provider names after its chat model — "gpt-4o-mini-tts"
satisfies the "-mini" rung and cannot answer a prompt.
An opener is not always titleable — an image with no caption, a compaction
handoff, a bare slash command — and those sessions stayed unnamed for
life, because the guard that stops re-titling a named session also stopped
the nameless one from ever asking again. Let a later turn name a session
that still has no title.
The derived title also ran the collision dedupe inline on the turn.
It is a slice of the user's own words, so it collides constantly — people
open sessions with "hi" — and resolving "hi #47" is a widening scan on the
critical path for a name the model replaces a second later. Decline it
there and let the background stage, which can afford the scan, pick it up.
The fast-model picker reads /v1/models to find the small model a provider
currently serves, and it asked anonymously. Most of those endpoints need a
key, so the fetch 401'd and the empty result read as "this provider has no
small model" — the picker fell back to its curated list and never noticed.
Worse, a failed fetch cached its empty result forever, so one bad moment
during startup disabled live model discovery for the life of the process,
and the processes that read this run for weeks. Give the failure an expiry
and pass the provider's credentials.
The bare family rungs (-mini, -flash, haiku) also picked whichever id
sorted first, which is the oldest generation a provider still serves:
gpt-3.5-mini over gpt-5.4-mini, claude-3-haiku over claude-haiku-4.5.
Compare the digit runs as numbers so the rung meant to keep us current
does.
The turn prologue titles every session, and it is shared by every agent —
including the ones no person is reading. A cron job already names its own
session after the job in its finally block, so the titler spent a side-LLM
call per fire to write the delivery scaffolding over it for the length of
the run. A delegated child's session is hidden from every picker, so a
batch at max_concurrent_children paid N title calls for N names nobody
opens.
Both are the same class of run that already sets skip_memory to stay off
the auxiliary path, so keep the titler off it too.
_PROVIDER_PREFIXES was a hand-maintained frozenset, so providers that ship
as plugins (bundled like fireworks, or user plugins under
$HERMES_HOME/plugins/model-providers/) were never recognised as
provider: prefixes in model strings, and metadata/context-window lookups
received the unstripped string. Mirror the _URL_TO_PROVIDER auto-extend
that already sits below it: add each registered profile's name and
aliases after discovery. The _OLLAMA_TAG_PATTERN guard keeps model:tag
strings intact.
Fixes#66106
The native Relay pipeline binds its Futures to the event loop that
entered run_in_session_async. While a managed tool callback executes,
that loop is blocked until the callback returns — so a nested managed
relay call made from inside the callback (vision_analyze's auxiliary
LLM call on a worker-thread loop) awaits a Future that can never
complete: 'RuntimeError: Future attached to a different loop', or a
deadlock, plus 'Event loop is closed' at shutdown when the orphaned
future completes late. (#77244)
Fix: managed_callback_guard, a ContextVar depth marker set around every
Hermes callback the relay adapters hand to the native pipeline
(relay_tools.execute invoke, relay_llm execute/execute_async invoke,
ManagedLlmStream run_callback). resolve_execution_context returns the
no-relay triple while the marker is set, so nested calls run unmanaged.
The marker propagates through contextvars.copy_context() into the
worker threads tools use for their internal async work.
Top-level turn LLM calls and tool wraps stay fully managed — verified
live: vision_analyze works under active shared metrics while the main
turn still records managed llm.execute events.
Alternative fixes considered and rejected: removing
retain_managed_execution (kills the shared-metrics managed pipeline)
and gating on main-thread identity (managed tool wraps legitimately
run on the run_agent thread, so that gate disables relay everywhere).
The Anthropic SDK's streaming accumulator builds ParsedMessage snapshots
whose ParsedTextBlock content doesn't match the generic union pydantic
expects, so model_dump() on stream events (message_stop) emits
PydanticSerializationUnexpectedValue UserWarnings straight into the
user's CLI output mid-response.
Pass warnings=False at every helper that dumps arbitrary SDK models
(relay_llm/_jsonable, relay_tools/_jsonable, anthropic_adapter
_to_plain_data, run_agent _hook_jsonable, chat_completion_helpers
extra_content/reasoning_details sites, chat_completions transport),
with a TypeError fallback for duck-typed model_dump implementations.
Adds regression tests including a precondition test that proves the
fixture still trips the warning without suppression.
Per-turn .env adoption could rewrite agent.api_key while leaving
_credential_pool_entry_id on a previously rotated fallback. The next 429
then marked the healthy fallback exhausted via credential_id precedence
(#79156).
- Sync pool entry id after a successful env credential refresh
- First look does not stomp a pool-rotated key with the env primary
- mark_exhausted_and_rotate prefers api_key_hint when it disagrees with
credential_id
Fixes#79156
Generic thinking fields (reasoning / reasoning_content + the
reasoning_details text charge) are replayed for at most the NEWEST
assistant turn on every transport: Anthropic strips all-but-newest at
convert time, Bedrock Converse never replays thinking, and strict
chat-completions providers reject or one-space-pad the field. The tail
budget walks charged them on every message anyway, spending 19-24% of
the budget (per the issue's 1,025-message measurement) on bytes that
provably never reach the wire — so the tail cut landed early and each
compaction discarded more real transcript than configured.
_estimate_msg_budget_tokens now partitions the replay keys:
* _ALWAYS_REPLAYED_BUDGET_KEYS (codex_reasoning_items,
codex_message_items) — charged unconditionally. These ride the wire
on every retained turn (#55572), and codex_reasoning_items now also
carries native server-side compaction checkpoints (#81747).
* _NEWEST_TURN_ONLY_BUDGET_KEYS (reasoning, reasoning_content) + the
reasoning_details text charge — charged only for the newest assistant
turn via charge_stale_thinking, resolved by the three budget walks
(tail cut, raw-budget re-walk, proactive-prune boundary).
Default stays the conservative full charge for callers without
turn-position context. A partition invariant test pins that any future
_REPLAY_BUDGET_KEYS entry must be classified into exactly one class.
Direction credit: #73669 (@x7peeps) and #73730 (@webtecnica) both
attacked this; the keep_open reviews asked for provider/API-mode-aware
accounting that keeps Codex carriers charged — this implements that
shape.
Titling fired on the first response, so a session sat unnamed for the whole
opening turn - p50 151s, p90 1212s across real sessions, because a turn is
tool calls, not one round-trip. A turn that failed or was interrupted never
got a title at all. Four surfaces each carried their own copy of the call.
Move it into the shared turn prologue and split it in two: a deterministic
title derived from the user's opening message, written inline before the
model runs, then one small-model call that upgrades it. The response is
constrained to a JSON object so there is no preamble to strip, and control
wrappers are stripped rather than refused, so a slash command titles as
what the user asked for instead of the command itself.
Titling ran on the user's main chat model, so a five-word title was billed
to a frontier reasoning model and inherited its latency. Pinning a cheap
model id instead just moves the problem: the hardcoded default was already
dead upstream and every call paid a 404 before the retry net caught it.
Match model FAMILIES against the provider's live /v1/models catalog,
preferring rolling '-latest' aliases where a provider publishes them, and
order the families by measured latency. Nothing to bump when a provider
ships a new mini/flash/haiku. Opt-in per task, so compression, vision, and
search keep 'auto means my chat model'.
Two corrections on top of the #71077 base (the whole bug class):
1. Turn boundary = last USER message, not last assistant message. A Codex
turn spans several assistant messages (assistant+tool_calls -> tool ->
... -> final assistant) whose reasoning items must replay together; the
last-assistant boundary would strip reasoning mid-chain from the active
turn (the gap flagged in PR #71077 review).
2. type="compaction" checkpoints (native server-side compaction, PR #81747)
are exempt: they carry already-pruned history, not per-turn reasoning.
Pruning filters items instead of popping the sidecar key.
Sibling site fixed in the same class: the Codex incomplete-continuation
dedup path blind-overwrote codex_reasoning_items on visually-duplicate
interim messages, which would drop the only copy of a checkpoint captured
on the earlier response. Extracted merge_interim_reasoning_items() into
agent/native_compaction.py; newer reasoning wins, prior checkpoints are
preserved unless the newer payload carries its own.
Own the surrogate-crash class at three chokepoints instead of leaf sites:
- finalize_turn scrubs final_response once where model text leaves the
conversation loop — covers oneshot stdout (#80366), NIM/any-provider
responses (#19819), and every delivery consumer of the turn result.
- _sanitize_gateway_final_response scrubs at the gateway chat-surface
boundary — Telegram utf16_len (#55309) and Signal formatting (#55143)
can no longer see a lone surrogate; raw-text surfaces keep passthrough.
- run_conversation walks the fully-built api_kwargs with
_sanitize_structure_surrogates so tool descriptions (session_search,
#50959) and every other request-body leaf are JSON-encodable before
any provider sees them.
Regression tests pin all three chokepoints plus helper semantics.
Cherry-picked alongside #79240 (TheophilusChinomona) and #80374
(rainbowgore) whose commits precede this one with authorship preserved.
Opt-in via compression.codex_responses_native (default: false). When enabled,
gpt-5.6-family models on the direct OpenAI API (api.openai.com) or a ChatGPT
Codex subscription send context_management=[{type: compaction,
compact_threshold: N}] on Responses requests. OpenAI compacts server-side and
returns an encrypted compaction output item; Hermes captures it into the
existing codex_reasoning_items sidecar and replays it on later turns in place
of the pruned history — inheriting persistence, session replay, the
cross-issuer guard, and the encrypted-replay kill switch with zero new state.
Scope is deliberately hard-gated (agent/native_compaction.py, re-checked per
request): gpt-5.6 family only — gpt-5.1/5.2 fail server-side on the field
(HTTP 500 / stream stall, no structured rejection; live-verified) — and
direct OpenAI/Codex routes only; xAI, GitHub/Copilot, OpenRouter, relays,
and local servers never see the field.
Hermes' local compression stays armed as the fallback owner: the native
threshold is clamped ~8K tokens below the local trigger so the server
compacts first, and a structured provider rejection of context_management
disables native compaction for the session and retries without it
(one-shot guard in TurnRetryState).
Live-verified E2E on api.openai.com/gpt-5.6: server compaction fired at a
4K threshold, checkpoints captured and replayed, recall preserved across
3 turns; gpt-5.1 with the flag enabled stays clean (field never sent).
Direction credit: PR #76950 by @laryhorb explored native Responses
compaction; this is a minimal reimplementation on current main.
Fixes 13 issues found in PR #20774 review:
1. Wiring: engine selection moved from run_agent.py to agent/agent_init.py
(where init_agent lives on current main). Transform hook moved from
run_agent.py to agent/conversation_loop.py (where run_conversation lives).
2. Prompt caching: replace copy.deepcopy with copy-on-write (shallow list
copy + clone only messages that are mutated). Use last_prompt_tokens
from update_from_response instead of re-estimating tokens every call.
System extension injection is idempotent (one-time cache break).
3. Signature mismatch: _message_signature renamed to _content_signature
and now excludes tool_calls/tool_call_id from the hash. This prevents
mismatches when _canonicalize_api_tool_calls re-serializes argument
JSON with sort_keys=True on the API copy.
4. update_model: accepts api_mode parameter (required by agent_init.py).
5. Reconciled with select_context: transform_api_messages is a separate
hook that runs AFTER select_context and sanitization, before
prompt-cache marker placement. Both hooks coexist with clear ordering.
6. Dedup/purge: kept as DCP-specific strategies (different semantics from
ContextCompressor._prune_old_tool_results — DCP deduplicates by
tool+args signature, not by content hash).
7. Removed copy.deepcopy: replaced with shallow list copy + copy-on-write
via _clone_if_needed. Only messages that are actually mutated get
cloned.
8. Removed redundant _ensure_refs call: _match_api_messages_to_refs no
longer calls _ensure_refs (the caller already called it).
9. _message_key still uses index (needed for positional ref assignment),
but _content_signature is cached per id(msg) to avoid re-hashing.
10. _inject_nudge: only injects into user messages, never falls back to
non-user messages (prevents role semantics violations).
11. Memory: _evict_inactive_blocks bounds blocks_by_id to
_MAX_INACTIVE_BLOCKS (50) deactivated blocks.
12. Merged _range_tool_schema and _message_tool_schema into a single
_compress_tool_schema. Merged _handle_range_compress and
_handle_message_compress into _handle_compress.
13. Dropped DCP_CONTEXT_ENGINE_PR_SPEC.md (temporary file, not for tree).
Config defaults kept minimal in hermes_cli/config_defaults.py (only
the keys the engine actually reads, not the full DCP-compatible surface).
Closes#20717
4c2961c51 added referenced_skill_names() so the curator never archives a
skill a cron job depends on — paused jobs and infrequent schedules would
otherwise age their skills out and the next run fails to load them.
62972060c then taught the scheduler that jobs may store ABSOLUTE skill
paths, normalizing them through normalize_skill_lookup_name before
skill_view. The protection set kept returning the raw string, so it now
holds a full path while the curator matches it against bare skill names.
Those jobs silently lost their protection: the skill is archived, and the
next fire logs a warning and runs the job without its instructions.
Canonicalize each reference the same way the scheduler resolves it, with
a deferred import and a verbatim fallback so a resolver failure can never
drop a name (referenced_skill_names has exactly one caller, the curator's
protection lookup, so nothing else sees the change).
On Windows, truststore.inject_into_ssl() replaces ssl.SSLContext with an
OS-trust-store-backed context whose get_ca_certs() raises NotImplementedError
(empty message). The ssl_guard's _validate_bundle_path() called get_ca_certs()
unguarded, crashing every fresh agent init with an opaque
'Failed to initialize OpenAI client:' error.
create_default_context(cafile=...) already validates that the bundle is
parseable, so we skip the post-load introspection rather than treat the
NotImplementedError as a failure.
Cherry-picked from PR #49945 with comment trimmed.
Co-authored-by: WolftacDigital <jonathan@wolftacdigital.com>
- Wire the Pass-1 dedup floor (len < 200) to the shared _PRUNE_MIN_CHARS
constant it was already documented as matching, and use the constant in
the remaining test literal.
- Restructure the clarify 'resolved' computation (is_answer_shaped +
sentinel check) instead of compute-then-flip.
- Add a live producer->recognizer drift guard: the REAL oneshot no-user
callback's output must be recognized as a sentinel, so producer wording
drift fails a test instead of silently reintroducing false attribution.
- Document the any()-poisoning semantic for multi-select sentinel lists.
Follow-up to the salvaged #81244 commits:
- Timeout/no-user clarify callbacks (CLI timeout, gateway timeout and
delivery failure, oneshot no-user) embed sentinel prose as
user_response; quoting those as '[clarify] user responded: ...' would
be false attribution. Route them to the generic summary path.
- Extract the shared _PRUNE_MIN_CHARS = 200 floor (prune default +
proactive clamp) and cap the clarify summary at _PRUNE_MIN_CHARS - 1,
removing the knife-edge equality the summary's survival depended on
and keeping it out of the >=200-char dedup pass.
- Tests: 4 sentinel shapes + multi-select sentinel; mutation-checked.
handle_max_iterations() appends its runtime summary request as a plain
role="user" row, which SessionDB persists verbatim. On later compaction the
synthetic-turn filters only recognized compaction summaries, continuation
rows, and todo snapshots, so the nudge could be selected as the latest
actionable user turn — becoming the task snapshot / auto-focus input and
getting summarized as "User asked: ...", demoting the real human task.
Metadata flags do not survive SessionDB projection (the reason the existing
markers are content-based), so recognition must key off stable content.
Extract the nudge into a shared MAX_ITERATIONS_SUMMARY_REQUEST constant and
teach _is_synthetic_compression_user_turn() to recognize it, mirroring the
continuation/todo markers. Every _is_actionable_user_turn call site already
pairs the synthetic guard, so the single recognizer change covers anchor
selection, auto-focus, and real-user-turn detection.
Fixes#78580
Guard both extra lookups with isinstance(dict) before merging, so a
truthy non-mapping `extra` value (e.g. `extra: "true"`) degrades to the
base Telegram hint instead of raising TypeError and aborting
system-prompt construction. Keep the narrowed except ImportError.
Add an integration test exercising the real config path (HERMES_HOME +
gateway.platforms.telegram.extra.rich_messages) and a regression test
for the malformed-extra fail-open path. The integration test fails on
main and passes with the fix.
Commit b45a217e0 gated the TELEGRAM_RICH_MESSAGES_HINT extension behind
a config read at the top-level ``platforms.telegram.extra.rich_messages``
key, but the Telegram adapter reads the same setting from the canonical
``gateway.platforms.telegram.extra.rich_messages`` path. When users set
the setting in the canonical location (the only one documented), the
lookup returned None and the extension never fired — the model degraded
pipe tables to bullet lists, task lists to plain dashes, and never
produced <details> blocks or block math.
Fix: merge both ``gateway.platforms.telegram.extra`` and the top-level
``platforms.telegram.extra`` with the same precedence the adapter uses
(top-level leaf wins), so config-wizard writes and dashboard-setup keys
are visible alongside the canonical gateway location. Narrow the
except-guard to ImportError so real config-stack failures surface.
- Fix _usage_audit_path() to use _get_hermes_home() instead of hardcoded
Path.home() / '.hermes' (profile-safe resolution, sweeper finding)
- Rewrite skip_background_review tests to exercise finalize_turn() directly
instead of duplicating the guard expression (sweeper finding)
- Fix response_silent audit field to use _is_cron_silence_response()
instead of the buggy SILENT_MARKER substring check it was meant to
replace (simplify-code review finding)
- Remove dead 'model' in locals() guard — model is always in scope
before the try block (simplify-code review finding)
- Extract _stub_agent_for_finalize() helper to eliminate ~40 lines of
copy-pasted agent stubbing in tests (simplify-code review finding)
- Clean up 'Phase 0.5' instrumentation comments
Phase 8 of the Hermes Agent token leak mitigation plan
(ralplan-hermes-token-leaks.md §3.9). Adds a boolean kwarg
`skip_background_review` (default False) to AIAgent.__init__ that
suppresses the end-of-turn _spawn_background_review fork.
Each background review fork instantiates a new AIAgent with its own
~15K input tokens + up to 8 LLM iterations, accumulating ~30K tokens
per event in the worst case. On cron sessions there is no
human-in-the-loop benefit from the review (no skill-creation pressure,
nobody curating MEMORY.md), so the cost is pure waste.
The end-of-turn guard now reads:
if (final_response and not interrupted
and not getattr(self, "skip_background_review", False)
and (_should_review_memory or _should_review_skills)):
skip_memory=True already disables the memory-review trigger; this
flag is the explicit single-switch off for both review paths.
Defaults to False, so behavior is unchanged for gateway/CLI callers
that omit the kwarg.
Tests: 5 new unit tests in tests/agent/test_skip_background_review.py
covering the default value, flag persistence, the gate short-circuit,
the gate fall-through, and a source-text assertion that the cron
scheduler sets the flag to True (separate commit).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reject malformed tasks=[...] batches before any child agent is spawned:
- exact-duplicate goals (case/whitespace-normalized), error names both
task indices
- placeholder goals: bare 'TODO', bare 'task N', unexpanded <...> or
{...} template markers, or goals shorter than 10 chars after strip
- 1-task batches, with an error pointing the model at the single
`goal` form instead
All checks are batch-only — the single-goal form is exempt by design
(short goals like goal="test" are valid there). Error strings are
actionable: each tells the model exactly how to fix the call.
Tool schema is unchanged (byte-stable); validation is runtime-only in
the existing batch-validation region.
Existing tests using terse batch goals ("A"/"B"/"C") updated to
realistic distinct goals per the new contract.
Inspired by: MoonshotAI/kimi-code agent-swarm.md validation rules (MIT)
Pin #80622 invariants: handoff alone must not drive a model call after
stop, pending real users are restored, and synthetic compaction rows are
never treated as user-originated turns. Also give micro-compaction
enough passes to pay back the longer SUMMARY_PREFIX marker overhead.
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.
Post-merge review of aecb9ca89 found the join guard over-broad: skipping
the join whenever ANY fragment self-matches _PREFIX_RE reopened a leak
for non-newline splits — sk-<15 chars>ESC<25 chars> masked only the
self-matching head and left the 25-char tail in cleartext (fully masked
before the guard; main never masked this shape at all, so the merged
state was still >= main, but the salvage's own coverage regressed).
Skip the join only when the span crosses a line boundary (\n / \r) —
that is the shape where adjacent legitimate text gets swallowed
(ghp_<token>-then-'button [ref=e3]' annotation bug). ESC/zero-width
controls never legitimately separate a token from prose, so joining
there is safe and restores full-tail masking.
Both legs mutation-checked: reverting to the unconditional skip fails
the new tail-mask test; removing the guard fails the annotation test.