Gate r2 Low cleanups (house rule: no aliases/shims):
- Drop the is_live_database_file alias; its point-in-time caveat now lives on
has_live_connection.
- _refuse_live_database reuses offline_file_access's message (via _serve_offline),
so a download 409 on state.db-shm names the main database like the read path;
the verb is "serve" so it fits read/download/stream.
- /api/files/read reads whole files in-process, so _read_base64_file now holds
offline_file_access through close (409 on a live DB; OSError stays 500). Only
the streamed FileResponse routes keep the point-in-time check.
- That check takes the global _live_lock, which other threads hold across
whole-file reads, so fs_download and the managed stream routes run it via
asyncio.to_thread instead of stalling the event loop.
- _managed_readable_file docstring no longer claims a size cap;
_read_file_reference returns (early, text) instead of a str|Expansion union
sniffed with isinstance.
Co-authored-by: Benjamin PERRY <benjaminperry6@yahoo.fr>
FileResponse opens and closes the file in the dashboard process, so
downloading a live state.db (or its -shm/-wal) via /api/fs/download or
the managed-file read/download/media routes still cancelled the
connection's POSIX locks. Both now return 409 via is_live_database_file;
the registry lock is not held across the streamed response.
The main-or-WAL-sidecar rule now lives in one _live_main_key helper used
by offline_file_access, has_live_connection and read_header_bytes_preopen,
and the sidecar refusal names the main database the connection is open on.
@file previews hold _live_lock only for the raw read; token counting and
formatting run after release. The Linux lock test gains requires_wal
(Hermes uses DELETE mode on WAL-reset-vulnerable SQLite), covers the
download refusal, and drops an ambiguous conditional assert.
Co-authored-by: Benjamin PERRY <benjaminperry6@yahoo.fr>
The PR predates the removal of the linux_only marker; the collection hook
now rejects it outright, so the whole module errored at collection.
The test reads /proc/locks and is genuinely Linux-only.
Keep one case per guarded route (@file, @folder, desktop fs_read_text)
plus one WAL-sidecar refusal (-shm); the remaining alias/wal variants
exercise the same offline_file_access keying path and only add runtime.
The rotation handoff now stamps each child row's stored-row digest, so
the exact-dict comparison must ignore DB_ROW_SNAPSHOT. Assert every
compressed dict carries one: this pins the non-flush restamp, which
otherwise only probes covered.
A legacy (no-digest) dict over a non-blank assistant row adopted the whole
decoded DB row: tool_calls / reasoning* / codex_* were overwritten with the
stored JSON (which still holds the escaped lone surrogate the sanitizer just
fixed, re-injecting it into the provider payload) and live-only fields were
popped. Resumed dicts (_rows_to_conversation stamps _row_id without a
digest) and compaction clones hit this path. Adopt content only, as before
this stack, via a content-only canonical handled like the metadata-only one.
_insert_message_rows dropped a clone's parent digest but only the flush
path restamped it, so clones made by archive_and_compact / replace /
rotation handoff / import reached the legacy path and the first live edit
after a clone was not persisted. Stamp the stored-row digest inside
_insert_message_rows (one batched SELECT, cold paths only; the flush path
statement count is unchanged) and drop the duplicate call in
append_messages_batch.
Define the _db_row_snapshot / _canonical_row keys once in
agent/message_metadata.py and import them everywhere instead of repeating
the literals.
The row digest hashed every repair column, so a same-process metadata write
(reaction, display-kind stamp, api_content / codex reasoning backfill,
platform message id) made our own row look like a foreign winner. The
re-flush then adopted the stale DB row: a later live edit (the non-ASCII
strip recovery) was reverted, and unsanitized tool_calls/reasoning were
copied back onto the live dict.
The digest now covers only the owned (non-metadata) columns: it means "the
row is still what we last committed". Match -> write the live owned values
and hand over only presentation metadata the live dict lacks; mismatch ->
genuine other writer, adopt as before. The r3 "stored content equals the
durable form of live" special case is subsumed and removed.
Also:
- _insert_message_rows drops a carried digest when it assigns a new row id
(compaction/replace/import clones carried the parent's version).
- append_messages_batch restores each message's _row_id / digest /
timestamp and pops the adopted row at the top of every _execute_write
attempt, so a rolled-back attempt cannot resolve to a foreign row.
- message_id is no longer synced onto live (int -> str flip, spurious
platform_message_id).
- The JSONL divert strips both bookkeeping keys via one frozenset.
Extend the kept active-row test: a reaction between flushes must not replace
the live multimodal user content with its text projection (red on the previous
tip at the image assertion), and the reaction metadata is synced. The user row
carries an int message_id so the stored-row (TEXT affinity) digest path is
exercised.
The row-addressed repair stamped the decoded durable row on every
resolved message, so after our own rewrite the sync copied the lossy
durable projection (image parts -> "text\n[screenshot]") back onto the
live dict: multimodal user/tool messages lost their images and the
prompt-cache prefix changed. Adopt the DB row only when another writer
won (digest mismatch) or on the legacy assistant path, as BASE did.
The insert-time digest hashed Python bind values, but SQLite affinity
rewrites them on storage (int message_id -> TEXT, float token_count ->
INTEGER), so live and DB digests never matched and in-place edits were
silently dropped. Hash the stored rows instead, only on the
append_messages_batch flush path that reads the digest (one SELECT per
batch), incrementally (type tag + length prefix) instead of via JSON.
Also skip the no-op UPDATE, fix the _write_columns comment/spacing and
drop the duplicate top-level Optional import (F811).
The CAS row snapshot was a full copy of each message's durable payload
riding on the live dict. The rough token estimator priced it (about 2x
estimates -> premature compaction) and it doubled transcript memory.
Replace it with a 16-byte blake2b digest of the repair columns. The
compare now runs in Python against the target row already read inside
the BEGIN IMMEDIATE transaction, followed by a plain UPDATE. Also:
- add _db_row_snapshot to PERSISTENCE_ONLY_MESSAGE_FIELDS so the
estimator and the outbound request builder both drop it
- derive _REPAIR_COLUMNS/_SYNC_FIELDS from _MESSAGE_WRITE_COLUMNS
- use hermes_state_common._placeholders
- drop the dead resume-path stamp (the SELECT has no token_count, so it
was always None) and the dead tool name assignment in
_decoded_repair_row
- keep the digest out of divert JSONL
The kept active-row test now pins estimate stability across a flush and
the survival of a concurrent writer's row. It goes red on the old
prod files and red when the digest compare is removed.
Keep one test per invariant: active user/tool rows whose _db_persisted marker
was popped by the outbound sanitizer keep their _row_id and are not re-inserted
(the #123462 desktop/serve path), and archived user/tool rows are repaired in
place instead of appended. Both fail on b4410b4bad; the other four PR tests
covered edge branches and are dropped per the <=2 invariant-test budget.
62ceddd342 cut raw args to HEAD+4096 before redaction to save time. The
PEM redaction pattern only matches a complete BEGIN...END block. A long key
whose END fell past the cut stayed unredacted, and once an earlier key was
redacted and the text shrank, its body landed in the 1200-char head that
goes into the persisted summary. Go back to the BASE order: redact the full
args, then apply the MAX/HEAD cut. This is a cold path (once per summarized
call per compaction), and _SUMMARY_INPUT_MAX_CHARS still bounds the prompt.
Extend the kept canonical-args test with a two-PEM input that leaks on
62ceddd342 and passes now.
Follow-ups to making tool-call args byte-exact:
- _record_compression_regions measured canonical_messages slices while
compress_start/compress_end are indices into the pruned copy that head/tail
are assembled from; measure the pruned rows actually sent, as before. This
also removes the only canonical slicing, so blank-echo classification drift
between the two copies can no longer misalign anything.
- _render_tool_call_for_summary redacted the full (now unbounded) args before
cutting to 1200 chars; cut to head+4096 first. Output unchanged for args
within that window.
- pressure_hits always equalled demoted once arg truncation left; fold it.
- Drop the fixture-only tautological assert in the guardrail test helper.
- Reword stale compress()/compression_marker docstrings that still described
canonical head/tail and compressor-written arg markers.
The arg-truncation removal deleted the only uses of the marker constants in
agent/context_compressor.py (ruff F401), and the test module had been
importing _COMPRESSION_MARKER_PREFIX through it, so test_context_compressor
line 137 raised NameError. Import it from its home, agent.compression_marker.
The PR's two tests passed on the base code (the prune boundary never
reached their calls). Replace them with one invariant test that is red on
base: six old 3000-char write_file arguments must survive the prune
unchanged. Drop the PR's canonical-state test file (tail-canonical shape
contradicts #61932's pressure demotion).
heal_pool_rows gates on _is_forkable_pool_row (refresh_token present for
nous) since 364a29d40e, but no test covered it: reverting to the plain
OAuth-payload check left the suite green while the heal deleted the
profile's agent_key-only row that shares root's id. Extend the existing
nous strip/heal test with that shape; it fails with the gate removed.
A profile left with only an agent_key nous row after the fork strip/heal still
"owns" nous but has no local providers.nous block. The next load_pool('nous')
fell back to the global root block in _seed_nous_singleton and upserted root's
single-use refresh token into the profile pool, recreating the fork one load
later (both device_code and manual:* ak-row shapes). Skip seeding from the
global-root fallback when the profile owns local nous rows; borrowing profiles
(no local rows) are unaffected.
Also drop the redundant try/except around _global_auth_file_path() in
_profile_owns_pool_provider; that function already handles its own failures.
The existing nous strip test now reloads the pool after strip and asserts no
profile row carries root's refresh token.
Adding nous to SINGLE_USE_REFRESH_POOL_PROVIDERS made the clone strip drop
every nous oauth pool row, including agent_key-only ones, while the
refresh_token-gated block strip kept the matching agent_key-only
providers.nous block. The profile then borrowed root's pool rows and its next
load_pool('nous') seeded its own block over root's shared row, writing the
profile's agent key into root (and every borrowing sibling).
Strip (and heal) a nous pool row only when it carries a refresh_token, the
same predicate the providers-block strip uses. The heal test now also covers
a fork that lives only in providers.nous (flat tokens), which previously had
no test coverage.
With nous in SINGLE_USE_REFRESH_POOL_PROVIDERS the pool row is stripped,
but providers.nous still carried the same single-use refresh token, and
nous load_pool/refresh re-seed from that block, so the profile still
forked the grant. Add nous to _DEVICE_CODE_BLOCK_PROVIDERS, read the
flat Nous token shape as well as the nested tokens shape, and only
strip/heal a block that carries a refresh token so an agent_key-only
nous block survives.
Refs #121649
Co-authored-by: salch-cred <salch-cred@users.noreply.github.com>
is_todo_tool_name returns False for non-string names (a malformed list/dict
name used to raise TypeError where the old check returned False), and the
kept regression test imports tui_gateway.server at module level so it no
longer depends on another test importing it first. Docstrings updated.
Co-authored-by: JoaoMarcos44 <joaomarcosdias444@gmail.com>
The parametrized todo_list hydrate test only exercised
AIAgent._hydrate_todo_store. Reverting the TUI files left every test
green. It now also asserts that tui_gateway.server._todo_state_from_history
returns the same todos for the direct and bridged cases.
Co-authored-by: JoaoMarcos44 <joaomarcosdias444@gmail.com>
The standalone test file carried an issue number in its name (AGENTS.md
forbids that) and duplicated TestHydrateTodoStore's fixture and assistant
helper. Give _assistant_todo_call name/arguments params and cover the
direct todo_list name plus the tool_call-bridged form in one parametrized
test. The legacy "todo" case is already covered by the existing class tests.
Co-authored-by: JoaoMarcos44 <joaomarcosdias444@gmail.com>
The prior check only guarded one of the three quote-forcing directives the
fix removed; a partial revert of '<exact latest user request>' or 'write the
reverse signal verbatim' would reintroduce long-quote stalls unnoticed.
Also call the classmethod directly and hoist the patch import to module level.
A deterministic fallback summary replaced the older handoff in the transcript but never updated _previous_summary. The next compaction kept the stale in-memory summary and dropped the fallback row from its window, so the fallback's user asks, files and last dropped turns never reached the summarizer. Store the fallback body in _previous_summary the same way a normal summary is stored.
(cherry picked from commit 35417d2e1ffbb775c3eaff17b26623896afa56c1)
Why: the test parametrized a label string that a ternary in the body mapped
back to an exception, and its name still said "api_timeout" although it also
covers an APIConnectionError carrying the stall marker. Parametrize the two
exception instances directly (with ids) and rename the test to
test_transport_errors_stay_terminal_network_failure. No behaviour change; still
one parametrized test.
The isinstance(e, TimeoutError) guard was untested: an APIConnectionError
whose text contains the stall marker must stay a terminal network failure
(#29559/#94448). Parametrize the existing api-timeout test with that input
(red when the guard is removed), and move both #124077 tests into
TestStreamingClosedFailure reusing _fail_on_main instead of a duplicate helper.
The zoneinfo/pydantic plugin warm-up imports were author-machine workarounds
that ran at collection time in CI; remove them. Collapse the class to one
helper and two tests: the Codex stall takes the timeout ladder without the
terminal network-failure flag, and APITimeoutError still sets it.
Co-authored-by: happy5318 <5318happy@users.noreply.github.com>
(cherry picked from commit 1cda73e0ac2d073066741a964b5166b16e9caf34)
## Thinking Path
When the Codex auxiliary stream guard aborts a compaction summary mid-stream
it raises `TimeoutError("Codex auxiliary Responses stream stalled: no new
output for 60.0s ...")`. The message contains neither "timeout" nor
"timed out", so `_classify_summary_failure` returned `timeout=False` while
`_is_connection_error` (which matches the type name "Timeout") returned
`streaming_closed=True`. The terminal network-failure flag then armed an
unconditional abort (`_TERMINAL_SUMMARY_FAILURES`), bypassing the retry
ladder and the deterministic fallback summary — on turn-start preflight
compression that ends in "Auto-resetting session after compression
exhaustion", wiping the session.
### What Changed
`agent/context_compressor.py` `_classify_summary_failure`:
- `timeout` is now computed first, and additionally matches `isinstance(e,
TimeoutError)` and the "stalled" message shape (the actual text the Codex
guard emits).
- `streaming_closed` is `_is_connection_error(e) and not timeout` — a
timeout keeps its retry-ladder semantics and can never arm the terminal
network-failure abort.
### Tests
New `TestSummaryFailureClassification124077` in
`tests/agent/test_context_compressor.py`:
- classify: Codex stall → `timeout=True, streaming_closed=False`.
- classify: plain `ConnectionError` stays `streaming_closed=True` (no
regression on the premature-close class).
- classify: a "timed out" message on a non-TimeoutError type stays a
timeout and is excluded from `streaming_closed`.
- integration: the stalled-summary path in `_generate_summary` does NOT arm
`_last_summary_network_failure`.
### Verification
- RED/GREEN double proof via git stash: pre-fix 3 failed, post-fix 4/4 pass.
- Regression: 15 existing failure-classification tests pass
(network_failure / premature_stream / empty_content / auth / truncation).
- ruff clean on changed files.
### Notes
Local test env: this checkout's venv python is a symlink into
`<home>/hermes-agent/.hermes-runtime/...`, so stdlib `zoneinfo` first-import
and pydantic's plugin `distributions()` scan under the real-home IO guard
needed the collection-time warmups at the top of the test file. CI
interpreters are not symlinked into the home — those two warm-up blocks are
no-ops there.
## Related
#124077 (issue). Family: #124078 (the same stall's template trigger),
#108104 (`auxiliary.compression.no_progress_timeout`).
(cherry picked from commit c5399fbdee44f2db1172f22632cbf348a2c7cab5)
(cherry picked from commit bce26a99791c09911d8a008a1922c512f5c1fcea)
The byte-stability test for the handoff block only re-asserted determinism of
a constant gated on tool-name membership; stable-tier rebuild stability is
already covered by test_system_prompt_restore and test_skills_auto_load. Its
one unique check (block appears exactly once) moves into the positive branch
of the parametrized injection test, and the _prompt helper now takes only the
tool names since every caller used the same model/gates.
The rationale comment lived twice (prompt_builder constant and the
system_prompt call site); keep only the call-site ordering note.
Keep the stack at <=2 invariant tests: the guidance is injected only when
delegate_task is in the toolset (and after the generic keep-working
blocks), and the stable prompt tier stays byte-identical across rebuilds
so the prompt-cache prefix does not drift.
The category filter dropped every zero-token category from the breakdown
payload. For mcp/memory/skills that is right — zero means "not
configured" and the row's absence says so. For conversation it hid the
row on any session whose transcript was empty or pre-turn, so the
Desktop Context usage panel showed System prompt/Tools/Memory but no
Conversation until the first turn completed (#87903): "the transcript
is empty" rendered identically to "the breakdown never measured it".
Zero for the conversation is a MEASUREMENT of something every session
has, so it is exempted from the drop via _ALWAYS_REPORTED; the
membership rule is documented at the constant so later additions argue
from the same principle. Structurally absent categories stay dropped.
Test: an empty-transcript breakdown reports conversation at 0 while
unconfigured optional categories remain omitted.
The desktop half (retained pre-turn snapshot) is already fixed on main:
useContextBreakdown nulls the snapshot mid-turn and the statusbar gauge
falls back to the streamed usage.
Python half salvaged from PR #87925 (author credited).
Fixes#87903
Routing the live display through reasoning_details bypassed
separate_glued_reasoning_blocks, so a reasoning-summary model's
head-to-tail bold headings glued into '**One****Two**' in the live box
while the persisted reasoning_content stayed repaired. De-glue the
detail-derived text against its own display accumulator (not
reasoning_parts — mirrored fields would insert a spurious break on the
first chunk), so display and history agree.
The streaming reasoning display read only reasoning_content/reasoning deltas;
reasoning_details deltas were accumulated for replay continuity but their text
was never routed to the live display, so models that stream thought solely via
reasoning_details (xiaomi/mimo-v2.6-pro via OpenRouter) showed no thinking
display at all. The non-streaming extract_reasoning already reads detail text —
this closes the asymmetry.
One representation per chunk is emitted: detail text when the details carry
it, otherwise the plain reasoning field. Persisted/replayed fields are not
rewritten; opaque replay material (encrypted/unknown types) stays unexposed.
A callback that raises (or is None) no longer breaks the stream.
Fixes#118851
Salvaged from #118888 by Wenfengcheng
Desktop terminal batching pre-collects the approval for every command in
a run before any of them executes, so the user consents to a batch in
which all commands are expected to run. When an earlier command then
fails (or is denied/blocked), that informed consent no longer describes
the world the later commands will run in — yet the executor still
consumed the pre-made decision as though nothing had happened.
- _TerminalBatch.failure_seen: set by the sequential publisher after a
slot's failed (or blocked) result is committed — i.e. after the failure
the model actually sees, never from a wedged worker's late result.
- consume_prepared_guard drops the prepared decision for any later slot
once a failure is published and returns None, so the guard runs its
live flow again: tirith scan, allowlist, and a fresh human approval
request when the command still warrants one. Nothing is auto-denied
and an explicit denial of the failing command remains authoritative.
- The flag is sticky for the batch: a later success must not un-stale an
approval collected before an even earlier failure.
Success-path batching is unchanged: with no failure, prepared decisions
are consumed exactly as before (byte-for-byte the same flow).
A quoted (space-bearing) @file:/@folder: reference may carry a :start[-end]
line range; the canonical parser (context_references.REFERENCE_PATTERN)
claims the whole token. The guard's local copy stopped at the closing quote,
leaving ":3" as residual "prose", so a quoted, line-ranged attachment-only
opener kept a path-derived title (#92068 regression found in review of #122000).
Mirror the canonical value shape and pin all three quote styles (#122000).
