_same_server_route compared config_fingerprint alone, which by design excludes
env/headers/auth (so the schema cache survives a token rotation). Profile B with the
same URL but different headers/env therefore adopted profile A's live connection and
called tools as A. _connection_identity = route fingerprint + env + headers + auth mode,
used by both the adopt and the stale-removal checks.
Also collapses the three writers of _server_tool_scopes to two: the adoption loop
re-implemented in mcp_tool_discovery._select_new_servers is dropped —
register_connected_into_current_scope (which runs first in register_mcp_servers) is
the single adopter, and _register_candidates records scope for freshly registered tools.
The salvaged regression test exercised only repair_message_sequence and
reanchor_current_turn_user_idx — pre-existing helpers — so reverting the fix left it
green. It now drives prepare_iteration on a real AIAgent with adjacent user rows and
asserts the returned index addresses this turn's row and mirrors into
_persist_user_message_idx (red without the re-anchor: IndexError).
Both re-anchor sites (repair and compression restart) call
turn_context_compaction._reanchor instead of inlining "reanchor + mirror", so they
cannot drift. The export tests fold into one parametrized invariant plus the
run_conversation envelope test; the WHAT-restating comment shrinks to the WHY.
Hosts that settle their own transcript by index (hermes-webui) cannot prove which
row of result["messages"] is the current user turn once this loop rewrote history
(alternation repair, compaction, post-turn micro-compaction): the instance-side
_persist_user_message_idx predates those rewrites, and a text match relabels an
identical historical prompt and claims its old answer. Only the producer can
assert the coordinate against the exact list it returns.
run_conversation now wraps the turn (_run_conversation_turn) and stamps the pair
through export_current_turn_boundary on every envelope that leaves the loop
(success, partial/error, interrupt, retry-exhausted, tool-limit, preflight
timeout, codex runtime), computed on the final messages after finalize_turn and
micro-compaction. The pair is exported only when the addressed row is this turn's
user message verbatim (reanchor's last-match rule); a rewritten row exports
nothing so hosts fail closed. The final index is mirrored into
_persist_user_message_idx for the persist override.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013gp366ijf39n4UUtJhZuMh
prepare_iteration() runs repair_message_sequence_with_cursor() before each API
call; the repair merges adjacent user rows in place (after a compaction, the
role=user summary sits next to the protected first user message). The loop's
current_turn_user_idx was recorded at turn start, so after a merge it points
past the current user row: the per-turn context injection (prefetch/plugin
context) silently misses it, and hosts that settle the transcript by this index
(hermes-webui) write the current user turn to the FRONT of the context —
rewriting the prompt's leading messages every turn (0% prefix-cache hits at
200K+ tokens, ~100 s re-prefill per turn) and duplicating the user's question.
The in-loop compression restart path already re-anchors; do the same after a
repair that changed the list: reanchor_current_turn_user_idx (last user row
carrying this turn's text), return the index through the IterationPrep verdict
so the loop state picks it up, and mirror it into agent._persist_user_message_idx,
which hosts read when the result carries no index. The new phase parameters
default to None so direct callers keep their signature.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013gp366ijf39n4UUtJhZuMh
The salvaged commit forked an explicit /refine under a new "refine_review" origin so
the memory delete gate would not treat it as unattended. But is_background_review()
is the key for every other review guard — skill_manager_guards (curator-owned-only,
read-before-write), skill_manager_tool (archive instead of rmtree), skill_ledger
actor, write_approval staging, the [auto] tag — so a /refine fork silently escaped
all of them.
Carry attendedness separately: the fork keeps origin "background_review" and sets
_review_attended; turn_context binds it beside the origin ContextVar; the memory
gate keys on the new is_unattended_review(). Also run the gate AFTER
_validate_single_op / the operations list check, as memory_tool's own docstring
requires, so an invalid replace is rejected now rather than staged and failed at
approve time.
Review follow-up on #105944 (#105921):
- explicit /refine forks now run under the refine_review write origin
(explicit flows from the CLI/gateway handlers through
_spawn_background_review_now and spawn_background_review_thread down
to build_cache_parity_fork), so a user-requested review keeps the
full memory operation set; only automatic reviews stay behind the
unattended delete gate.
- the unattended delete gate now stages the denied replace/remove (or
whole batch) into the pending store instead of dropping it: the
fork's own review summary is never published, so a plain denial lost
the consolidation request with no surfacing path. The staged proposal
carries a proposal_staged marker that summarize surfaces as an action
line, and a staging failure still fails closed to a plain denial.
- regression tests: explicit-path origin pass-through, refine_review
keeping replace working, near-limit denial end to end (add rejected
by budget -> replace staged -> proposal surfaces, store unchanged).
The review fork's tool whitelist granted the whole memory toolset
whenever the profile had memory enabled, regardless of which nudge
fired, so a skill-nudge fork held remove/replace on MEMORY.md it was
never asked to use; combined with the memory tool's near-limit
'consolidate now' hint, an unattended fork deleted standing rules with
no user in the loop.
- Pass review_memory from spawn_background_review_thread through
_run_review_in_thread/_run_review_fork into _review_tool_whitelist;
a skill-only review no longer gets the memory tool at all.
- Fail-closed operation gate in memory_tool: a background-review fork
may add, never replace/remove (single or in a batch) — consolidation
decisions reach a human via the review summary instead.
- Keep the deny/prompt wording in sync with the whitelist so a
memory-less review doesn't advertise memory.
Fold the identical heappush(...) into PeriodicScheduler._requeue; notify() instead of
notify_all() now that the scheduler thread is the only condition waiter; drop the
PR-history paragraph from the module docstring (the commit carries it).
Tests: the blocked-sibling test asserted `sibling_ran.wait(0.30)` — a wall-clock bound
under the repo's ≥2 s flake floor; it now asserts the sibling fired while the blocker
still held its worker. The worker-start-failure fake keys on this scheduler's own
_run_callback rather than the global thread-name prefix so a leaked handle on _DEFAULT
cannot consume the single injected failure. The base-green no-overlap test is dropped
(it does not prove the fix).
oMLX/MLX prefill memory-guard rejections name an allocation peak in BYTES but
close with "Reduce context length", so _CONTEXT_OVERFLOW_PATTERNS claims them
and the turn enters the compress-and-shrink loop. Compression cannot lower a
prefill peak — the prompt is usually far below the window — so it burns the
compression budget, re-hits the wedged server on every attempt and ends in
"Cannot compress further" plus a destructive session reset.
Classify them as `overloaded` instead: retry with backoff, no compression, no
session reset (mirrors 503/529 recovery).
The guard runs before the overflow check AND before the usage-limit
disambiguation. The second ordering matters more than it looks: "memory limit
exceeded" contains "limit exceeded", so a status-less rejection — a proxy that
flattened the body — is currently classified as `billing` and rotates a
healthy credential.
Sites covered:
- _OVERFLOW_AS_5XX_RULES, which _400_TAIL_RULES extends → 400, 500, 502,
503, 529
- _MESSAGE_HEAD_RULES for the status-less path (ahead of usage-limit)
- _ERROR_CODE_VERDICTS for the structured oMLX codes
- _classify_400, because _by_status runs before _by_error_code, so a body
whose wording a proxy stripped would otherwise fall through to
format_error
Every pattern names memory/allocation in bytes, never a token or window count,
so the list stays disjoint from _CONTEXT_OVERFLOW_PATTERNS. Both oMLX wordings
are kept: 0.5.6 says "Prefill would require ~13.87 GB peak", 0.5.7 reworded it
to "predicted peak would require/exceed" and both are in the field. A test
pins that a genuine window overflow still compresses.
Refs #52261. Supersedes #52289, which predates the classifier rewrite and can
no longer be merged.
Second entry of the same bug class: POST /api/cron/jobs/{id}/trigger →
_fire_cron_job_for_profile → CronScheduler.fire_due → claim_fire built its claim
without `manual`, so an off-tick run from the web UI stamped the future slot exactly
like the tools path #105704 fixes. fire_due/claim_fire gain `manual` (forwarded only
when set, mirroring `force`, so third-party providers keep working) and the dashboard
trigger passes it when the provider's signature accepts it. Webhook and misfire
catch-up fires run the slot that is due and keep the stamp.
Also drops the base-green tick-stamp test (the same contract is pinned by
tests/cron/test_scheduled_occurrence.py) and documents `manual` vs `force`.
claim_job_for_fire() derives the occurrence identity from next_run_at
before the same function advances it. On a scheduler tick next_run_at is
the occurrence being run, which is correct; on an off-tick manual run it
is the NEXT occurrence, so the execution is stamped with the identity of
a slot that has not happened yet. _job_is_due() then finds a completed
execution carrying that identity and skips the real slot, returning
before the last_dispatch write — no error, no log line, no dispatch
record.
The manual flag already guards this and both _job_is_due() and
claim_job_for_fire() honour it; the agent-facing run-now path never
declared itself. Add a keyword-only manual= parameter and pass it from
_claim_for_manual_run(). Deliberately not force=True: force also calls
_activate_job_record(), which would resume a paused or disabled job, and
the run-now tool depends on continuing to refuse those.
The local flag is renamed to manual_fire so the new parameter is not
shadowed inside the apply closure, which would raise UnboundLocalError.
Three existing tests in tests/tools/ pinned the old call signature via
assert_called_once_with; they now pin manual=True, so dropping the flag
again fails loudly rather than silently reintroducing the skip.
Restores the intent stated in #104790 — the column records the scheduled
instant an execution was claimed for, and an off-tick manual run was
claimed for none.
Fixes#105690
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A profile config with a bare `display:` key (present-but-null) made
`user_config.get("display", {})` return None — the {} default only
applies when the key is missing — so the chained
`.get("memory_notifications")` in _wire_turn_agent_callbacks raised
AttributeError on every real gateway turn (Discord / cron). Oneshot
turns bypass this wiring, which masked the crash during smoke tests.
Use the same `or {}` guard the other gateway display readers
(display_config.py, runtime_footer.py) already apply, and fall back to
the documented default "on".
Fixes#105674
Execute the selected no-op rather than freeze its spelling, while rejecting
/bin/true to model the NixOS failure. Mark the regression Linux-only and
retain the current user-bus environment handling.
Consolidates the earlier NixOS scope-probe report and fix in #102587 with
the PATH-independent payload from #105436. The fallback resolver is not
needed when /bin/sh is used directly.
Co-authored-by: Scott Garrand <sgarrand@gmail.com>
Scott Garrand (@sgarrand) identified the NixOS /bin/true systemd-probe bug
first in #102587; the salvage of #105436 credits him with a Co-authored-by
trailer, so the release script needs his mapping.
The compaction-refresh and empty-surface tests pinned
_tool_snapshot_generation == _FROZEN_TOOL_SNAPSHOT_GENERATION next to
the behavioural assertion (refresh returns set(), tools unchanged). The
behaviour is the contract; the constant is the mechanism.
agent.tools is always a list (agent_init assigns it from
get_tool_definitions) and every entry is a well-formed function schema,
so the isinstance ladder over parent/entry/function/name guarded shapes
that cannot reach this helper. Use the same two lines agent_init uses;
`or []` keeps the empty-surface contract from the previous commit.
Docstring cut to the WHY (the between-turn refresh note described the
other guard). Tests trimmed to the two invariants: inherited tools
survive the compaction-boundary refresh (deep-copy isolation folded in),
and an empty parent surface is copied and frozen. Literal sentinel
asserts replaced with the constant.
Copy and freeze review_agent._tool_snapshot_generation even when parent.tools is an empty list ([]). Previously, the truthiness check (\
ot parent_tools\) caused an empty parent tool surface to be skipped, allowing newly available late MCP or plugin tools to be retained on the review fork and leaving its snapshot generation unfrozen. This broke the byte-parity contract when no tools were active on the parent.
Returning early only when parent_tools is not an instance of list or tuple guarantees that an empty tool snapshot is faithfully inherited and frozen. Adds dedicated regression test test_unrouted_review_fork_inherits_empty_tool_surface.
Freezes review_agent._tool_snapshot_generation to _FROZEN_TOOL_SNAPSHOT_GENERATION
(2_147_483_647) when inheriting the parent tool surface for same-model cache parity.
When in-place compaction boundaries trigger refresh_agent_mcp_tools(content_aware=True),
the staleness guard in _publish_tool_snapshot refuses the rebuild (snapshot_generation < published_gen),
preventing agent.tools from being reconstructed from the raw registry and preserving
inherited memory-provider and late tools across compaction boundaries (#103579).
Adds unit regression test verifying tool preservation across content_aware refresh.
Co-authored-by: joaomarcos <joaomarcosdias444@gmail.com>
Ensure unrouted background_review forks inherit the parent's full advertised
tools[] surface. Without this, skip_memory=True caused memory-provider tools
(e.g. fact_store/fact_feedback) and dynamically injected plugin/late MCP tools
to be omitted from the fork's tools array, breaking byte-exact prefix-cache parity
and incurring full cold-read costs on providers where tools are part of the cache key.
Inheriting the full parent tools array preserves complete prefix cache parity
while execution dispatch remains strictly bounded by the thread tool whitelist.
_session_db is always a SessionDB (agent_init / delegate_tool), so the
"fail closed on a store wrapper" hasattr was defense around code that
cannot fail; the store's own guard binds the value into SQL, so the
prologue only needs the sibling idiom isinstance(_row_id, int) that
session_persistence and transcript_repair already use. The positional
hazard is explained once, on set_latest_user_api_content. The in-place
compaction test duplicated test_api_content_sidecar's
test_inplace_compaction_backfills_sidecar_into_db verbatim (its row_id
parameter was never varied); dropped, as was the positional-helper tail
of test_older_identical_row_is_untouched already covered there.
The turn-start stamp had grown its own copy of the "what does the current
user row hold" rule (persist override = clean transcript, live content =
wire bytes = sidecar when they differ) that _db_flush_row already
implements. Two copies drift; extract durable_user_row_content() in
session_persistence and call it from both.
Also: reuse _persist_lock() instead of a third open-coded lock/nullcontext
ladder; drop the hasattr guard on set_latest_user_api_content (it predates
this fix and exists on every SessionDB); cut the comment to the WHY;
trim the new test file from 18 cases to the 7 invariants (real close
flush E2E, repeated-"ok" positional protection, API-only pre-flushed
turn, normal path writes nothing, compaction keeps positional, store
guards). Still 3 red / 4 green when agent/turn_context.py is swapped
for main's copy.
_stamp_api_content_sidecar read _row_id without holding
_session_persist_lock. A close/early flush holds that lock while it
commits the row and only afterwards writes _row_id back onto the live
dict; a stamp that ran in between saw no id and skipped the backfill,
the flush finished with api_content = NULL and marked the message
persisted, and the turn-start persist skipped it — the row kept the
wrong bytes with no writer left to fix it.
Run the _row_id read and the DB backfill under the (re-entrant) lock,
re-checking _row_id after acquiring it. Race reported by @ehz0ah on
Co-authored-by: sal <141555468+salch-cred@users.noreply.github.com>
#102411; same fix shape as @salch-cred's follow-up on #103721.
The api_content sidecar ('persist what you send') preserves prompt-cache
stability across turn boundaries by persisting the exact API-bound bytes
(including memory-manager prefetch, plugin injections, and API-only notes)
and substituting them on replay.
When a user turn was already materialized in the database before the
sidecar could be composed (in-place preflight compaction or a close/early
flush racing the prologue on the CLI path), the turn-start crash persist
marker-skips that message. Previously, the backfill was gated strictly on
in-place compaction (_preflight_compressed and _last_compaction_in_place),
so racing CLI flushes left api_content = NULL in SQLite and broke prompt
caching on subsequent turns (#102194).
Positional approaches (such as #102239 and #102286) using LIMIT 1 on the
newest active user row are unsafe: repeated common inputs ('ok', 'yes',
'continue') cause the backfill to match and overwrite the PREVIOUS turn's
row with the new turn's sidecar, corrupting history and breaking cache parity.
Resolve all landing blockers and review feedback from #102411:
1. Bounded state owner (Sahilvishnaliya):
Add SessionDB.set_message_api_content(session_id, row_id, content, api_content)
to SessionMessagesMixin in hermes_state_messages.py instead of growing
hermes_state.py. Update set_latest_user_api_content docstring with durable
warning on the positional hazard.
2. API-only turns & durable content selection (ehz0ah):
When a pre-flushed clean input has an API-only difference (e.g. voice
prefix or model-switch note):
- Retain the differing API-facing bytes as api_content even when no
new memory or plugin context was injected.
- Derive the durable content guard using _override_replaces_content so
the SQL 'content IS ?' guard matches the clean override text stored
in the DB row rather than the restored wire text.
3. Turn prologue gating (_row_id) & fail-closed store duck-typing (ehz0ah):
In agent/turn_context.py::_stamp_api_content_sidecar: check _row_id on
the live user dict (stamped by _insert_message_rows and synced by
sync_flushed_message_markers). If valid (positive int, not bool), address
by exact ID. Do NOT fall back to positional matching when a row ID is
present: if an external or custom wrapper lacks set_message_api_content,
fail closed and skip rather than corrupting a neighbouring row. If absent
but in-place compacted, fall back to positional update. On normal turns,
skip the backfill entirely (single atomic INSERT).
4. Real lifecycle test coverage (salch-cred, ehz0ah):
Comprehensive tests in tests/agent/test_api_content_row_addressed_backfill.py
covering store guards, surrogate scrubbing, gate non-arming, older identical
row protection, real close-flush row_id synchronization, API-only clean
override preservation with exact wire replay, and duck-typed store fail-closed
verification when set_message_api_content is absent.
Fixes#102194.
Closes#102411.
- _transcript_row_texts re-implemented agent.message_content.flatten_message_text
and the api_content sidecar rule; the note can only land on a user row,
so the transcript scan now skips assistant/tool rows (the bulk of the bytes).
- Three sites computed "names of agent.tools"; tools.mcp_tool_agent gains
agent_tool_names() used by the switch note and conversation_loop, which
also stops importing the private _def_name across modules. The name list
is only captured when a switch was announced.
- split_runtime_boundary() is the single owner of the runtime-block
rpartition/END check for both identity_line_value and
_stored_prompt_matches_runtime.
- platform_surface_hint was a public alias of _platform_hint; the function is
now platform_hint (its docstring pointed at the pre-move module).
- consume_gateway_turn_context_notes and consume_surface_switch_note share
_pop_turn_note so the two one-shot channels have identical semantics.
- platform check hoisted above the transcript scan.
Move the six surface-switch helpers out of the conversation_loop facade
into agent/surface_switch.py (AGENTS.md: new behaviour goes in a topical
sibling), and fold the review findings on #104494:
- MoA and codex_app_server turns never stamp the api_content sidecar, so
the staged note could not be read back from the transcript and was
re-sent on every turn after a switch. Those modes now skip the note
(stored prompt still reused).
- The announced surface was parsed with split(".") — a plugin platform
with a dot in its name would never compare equal and re-stage the note
every turn. The note now closes the name with a fixed terminator.
- One identity-line parser (identity_line_value) shared by
_stored_prompt_matches_runtime and the switch detector instead of two
copies of the runtime-boundary/rpartition logic; tool names via the
existing tools.mcp_tool_agent._def_name; the transcript scan is bounded
to the last 200 rows (it ran every turn over the whole history).
- consume_surface_switch_note reduced to a plain pop; developer-guide
prompt-assembly.md updated (Platform is no longer an identity field);
17 new tests trimmed to 10 (same-shape pin/retire variants folded).
Restoring Platform as an identity field still turns 5 tests red.
When Bot Chat capability refresh rebuilds the system prompt for the current
surface, call _stage_surface_switch_note() so any earlier switch note sitting in
the transcript is retired instead of overriding the rebuilt prompt.
Also isolate _stored_prompt_platform() to parse only the authoritative identity
portion before '# Hermes runtime environment' (with legacy fallback for prompts
without the boundary), preventing embedder prose or HERMES_ENVIRONMENT_HINT decoys
from shadowing the real platform and falsely suppressing surface switch announcements.
Credit to @ehz0ah, who identified both correctness gaps on current main and
verified the regression scenarios.
The announcing turn used to skip the tools freeze and re-persist the array the new
surface had just built. That is the one mutation this fix cannot afford: tools[] is
serialized ahead of the system prompt, so rebuilding it moves the request at token 0
and re-prefills everything behind it — the exact cost #104414 measured (1% cache hit
on a 220K session), spent on the very turn the fix exists to make cheap. On a
`desktop -> tui` switch with a configured toolset selection (`_gui_surface_toolsets`
gives desktop `desktop_ui`, the TUI nothing), skipping the pin dropped ~a dozen tools
and bought back the whole miss.
The pin now holds. `_merge_preserving_prefix` still appends what the new surface
brought, so a `tui -> desktop` switch pays a break no freeze could have avoided, and
the tools it carries FORWARD are named at the end of the surface note instead of being
silently advertised: a `focus_pane` a terminal turn can only answer with
`tool_error("desktop only")` now reads as unavailable rather than as live capability.
The toolset converges at the next real rebuild boundary, where the break is already
paid.
Credit to @StanleyStetson, who caught that the tool array is evaluated ahead of the
system prompt and that the bypass reintroduced the miss this PR is about.
Two holes the first cut left open, both created by the note itself.
Switching BACK to the surface the prompt was built for (desktop -> tui -> desktop) left the
`Platform:` trailer agreeing with the runtime, so nothing was staged — while the newest note
in the transcript still told the model it was on tui. And a rebuild for an unrelated reason
(a model switch) refreshed the prompt but not that note, leaving the same contradiction from
the other side.
Compare the runtime surface against what the model was last TOLD — the newest surface note
when one exists, else the prompt's own trailer — and stage from the rebuild path too. The
full surface guidance rides along only when the prompt itself is out of date; when the prompt
already describes the current surface the note just retires the stale one and points at it.
The first cut gated the saved tool_names pin on "the surface drifted", which stays true
for as long as the stored prompt names the old surface — i.e. until the next compaction.
On the gateway path, where a fresh AIAgent is built per turn, that left the tools freeze
off for every remaining turn, so a check_fn that flaps could reorder `tools[]` and break
the tool cache block on its own.
Gate it on the turn that actually ANNOUNCES the switch instead, and persist the fresh,
toolset-correct names there. The next turn's row already holds this surface's tools, so
the pin resumes immediately: skipped once, not disabled.
`_stored_prompt_matches_runtime` treated `Platform` as a runtime-identity field, so
answering a live session from another surface — desktop -> TUI, or a resume after a
dashboard restart whose chat is a PTY TUI child — declared the stored prompt stale and
rebuilt it. The system prompt is the first thing in the request, so changing any byte of
it moves the first divergent byte to the head of a 220K-token request and the entire
conversation behind it re-prefills: a session that was hitting 240000/240287 came back
at 1536/219861.
The guard was not wrong about correctness — a desktop-built prompt on a terminal session
advertises inline widgets and a MEDIA: channel the TUI does not have — but the surface is
advisory metadata about the renderer, not a cache domain. Model/provider and cwd drift
change what the prompt should SAY; the surface changes only one paragraph.
Reuse the stored bytes across a surface switch and correct the paragraph where it costs
nothing to cache: `_stage_surface_switch_note` stages a one-shot note carrying the CURRENT
surface's guidance on the same per-turn user-message channel the gateway's must-deliver
notes use. It lands after the cached prefix and is stamped into the byte-stable
`api_content` sidecar, so later turns replay it instead of re-prefilling, and the prompt
converges at the next compaction — a boundary that already breaks the cache.
The saved tool_names prefix is not pinned across a switch: the tool registry is
process-global, so `_merge_preserving_prefix` would carry a saved-but-unloaded tool
forward (under `coding_context: focus` desktop gets a desktop_ui toolset the TUI cannot
run). On the same surface the tools freeze is untouched.
