The new entry-side guard in delete_session/delete_sessions called
_write_guards_reject without allow_closed_compression_parent=True, so
_check_transcript_write_guards raised CompressionSessionClosedError for any
row with end_reason='compression'. That type is not caught by
_write_guards_reject, so every user-facing delete of a compressed parent
500'd and a bulk delete containing one rolled back the whole batch.
Pass the flag at both sites, matching prune (hermes_state_maintenance.py).
The lease is keyed on the lineage root, so a live turn on the tip still
blocks deleting its ancestor. Test 1 gains a compression-ended case (red on
the pre-fold file).
delete_sessions(exclude_active_write_guards=True) dropped guarded rows
silently: the web bulk-delete endpoint returned only a count and the
dashboard removed every selected row optimistically, so refused rows
reappeared on the next reload with no explanation.
The store now appends refused ids to an optional skipped_ids list inside
the same write transaction, the endpoint returns them as skipped_active,
and SessionsPage keeps those rows listed. Also hoists the
SessionActiveWriteGuardError imports to module top (hermes_state_errors
is stdlib-only) and drops the assertion-less lineage comment in the test.
Refactor entry-side deletion refusal to execute in-transaction via
`_write_guards_reject(conn, sid)` (#123583), per maintainer review:
- Underlying `delete_session` and `delete_sessions` now accept an opt-in
kwarg `exclude_active_write_guards=True` running inside `_do` write
transaction, eliminating the race condition where a turn acquires the lease
between check and delete.
- Raises `SessionActiveWriteGuardError` when refusing single delete, leaving
the row untouched; `delete_sessions` atomically skips active rows.
- Checks both active turn leases and compression locks via the existing
reclaim-aware `_write_guards_reject` helper.
- Covers all user-facing delete sinks:
* Web `DELETE /api/sessions/{id}` -> 409 Conflict
* Web `POST /api/sessions/bulk-delete` -> skips active rows
* Web / CLI `prune` -> passes `exclude_active_write_guards=True` so lineage
parents of active conversations are not pruned
* API Server `DELETE /api/sessions/{id}` -> 409 session_active_turn
* CLI `hermes sessions delete` & `export --delete-after-verified` -> exits 1
* CLI browse picker -> refuses active delete
* TUI Gateway `session.delete` -> 4023 error
- Conforms to rubric with 2 targeted invariant tests in
`tests/hermes_state/test_delete_session_write_guards.py`.
- Updates user guide and web dashboard docs for 409 / exit 1.
(cherry picked from commit 2c037a7a79dc211b49bacc72e3140951ccf900cf)
Both reconnect paths (gateway/run_adapters.py watcher and multiplex
secondary) build a FRESH SimplexAdapter before connect(is_reconnect=True),
so the per-instance _allowlist_warned flag never suppressed anything: a
daemon-down cold boot re-logged the warning on every backoff retry. Gating
on `not is_reconnect` would instead lose the warning when the first connect
fails. Dedup at module level keyed on (hermes_home_key(), frozenset(names)),
still checked before the connectivity probe. No shared warn-once helper
exists (plugin_compat.warn_once is compat-specific).
Read the value with platform_gate_env (the reader authz uses; differs from
get_scoped_secret when a scope is installed with multiplex off) and decode
JSON list literals with decode_json_list_literal like _coerce_allow_set, so
'["4","9"]' written by `hermes config set` no longer warns that valid IDs
are ignored.
The caplog test now builds two fresh adapters (first connect fails, second
succeeds) and asserts exactly one warning naming only 'alice'; it fails with
2 warnings against the pre-fold adapter.
The two SimpleX allowlist tests differed only in the allowlist value and
expected verdict; one parametrized test keeps both invariants and holds
the stack at two tests after the connect()-warning test was added.
The name-entry warning in connect() read SIMPLEX_ALLOWED_USERS via raw
os.getenv, while authz reads it profile-scoped. Under multiplexing a
secondary profile would warn about (or stay silent on) the default
profile's list rather than the one actually enforced. Use the module's
_get_scoped_secret + _parse_comma_list like __init__ does.
It also only fired on a successful non-reconnect connect: if the daemon
was down at cold boot the first connect() failed and every retry came in
with is_reconnect=True, so the warning never appeared. Evaluate it before
the connectivity probe, once per adapter via an instance flag.
Test: two connects (first fails) -> exactly one warning naming only
'alice' for scoped '4, alice' while os.environ holds 'bob'. Red on the
pre-fold adapter (0 warnings) and on a raw-os.getenv variant (names bob).
After #44729 SIMPLEX_ALLOWED_USERS matches only the numeric contactId, but
the docs still told operators display names work, and existing name
entries would silently stop matching. Update the docs and log a one-time
warning at first connect listing non-numeric entries that are now ignored.
Drop test_simplex_allowlist_rejects_colliding_display_name: it passes on
the unfixed base (the allowlist held the contactId, not the colliding
name), so it never guarded #44729. Drop the setup-prompt string check as a
change-detector. Keep rejects_display_name_only (red on base) and
accepts_numeric_contact_id (contactId path still works).
The SimpleX sender allowlist (SIMPLEX_ALLOWED_USERS) previously matched
against both the stable numeric contactId (user_id) and the mutable
display name (user_name). Since any SimpleX contact can change their
localDisplayName / profile.displayName to match another user's, this
allowed an unauthorized contact to bypass the allowlist by setting a
colliding display name.
Remove the user_name check so that SIMPLEX_ALLOWED_USERS only matches
on the immutable contactId. Operators must use numeric contact IDs in
the allowlist.
Fixes#44729
(cherry picked from commit b4aa29da1567d45920f79aabdb36b44c5f87bde5)
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.
_insert_message_rows stamps _row_id and the stored-row digest onto the
caller's dicts inside the write transaction. Only append_messages_batch
restored that state on rollback. archive_and_compact, replace_messages
and the rotation handoff left the rolled-back id + digest on the dicts;
SQLite reuses the id, so a later flush found a digest mismatch on the
foreign row, adopted it and silently dropped the user's message.
Move the capture/restore into _execute_transcript_write, used by every
caller that inserts caller-owned dicts: each attempt starts from the
caller's state and a final failure restores it before re-raising.
(Rewind replacement and import insert dicts built inside the txn.)
Also: bind _message_row_params directly on insert instead of the
serialized-dict round-trip, import the public DB_ROW_SNAPSHOT /
CANONICAL_ROW names, set adopt=False once, and reuse target_row
instead of re-SELECTing when nothing was written.
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.
Same-process writers (set_message_reaction, display-kind stamping, api_content
backfills, codex reasoning update) change stored columns after a flush without
refreshing the live row digest. The next sanitize + re-flush treated that as a
concurrent winner and copied the lossy durable projection over live multimodal
content, dropping image parts and shifting the prompt-cache prefix. On adoption
we now keep live content when the stored content is just the durable form of
it and sync metadata only; a real concurrent content winner is still adopted.
The legacy blank-assistant path (dict with _row_id but no digest, blank DB row)
again only fills the row from live content, as on main, instead of running the
full canonical sync that wiped live reasoning_content/finish_reason/tool_calls.
Adoption on the legacy path is limited to a non-blank row.
Also: transcript_row_snapshot returns str (the partial-row branch had no
caller), serialization only runs on the digest-match branch that reads it,
stamping reuses hermes_state_common._id_chunks, and _MESSAGE_WRITE_COLUMNS is a
plain top-level import (hermes_state_messages imports this module lazily, so
there is no cycle).
Gateway/TUI/CLI callers pass their live dicts straight to
append_messages_batch, so a concurrent-winner adoption leaves the
decoded durable row (_canonical_row) on a dict that may later be sent
to the model. Treat it as persistence-only like _row_id and the digest
so the outbound builder and token estimator both drop it.
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.
transcript_row_snapshot annotates Optional, which was only reachable via the
PLUGIN-COMPAT re-export block at the bottom of the module. Internal code must
not depend on that revert-scheduled block, so import it with the other typing names.
Review cleanups on the drain back-off:
- `_drain_after` now requires `guard`: a None default silently meant "release
whatever guard is current", which is the guard-swap bug the parameter fixes.
- The identity rule is stated once (docstring, wrapped), and the redundant
`pending_event is dispatched_event` disjunct is dropped: the same object
always has an equal message_id, and when that id is empty its timestamp
equals itself, so the remaining comparison already covers it.
- Tests drop the `_Adapter` alias and cut the hot-loop matrix from 6 to 4
explicit cases (plain, rewrite with id, rewrite without id, steer). The
demotion route does not interact with the identity axis, and each case
spends a fixed 1s measuring window.
Test 2 waited a fixed 0.6s before cancelling, which under load landed mid
first-handler and raised KeyError; poll for a real back-off instead. Route
chained follow-ups through the runner mid-turn and assert they dispatch
immediately, which fails (0.25s/0.5s gaps) with the identity check disabled.
Add an id-less rewrite-copy case to the hot-loop parametrization and drop the
typing assertion that could never fire in this harness.
The back-off drain's slot-empty exit released whatever guard was current, so a
/stop, /new or /reset guard swapped in during the sleep was deleted, defeating
the #48300 guard-swap protection. Capture the guard owned by the drain at spawn
and release only that; also flush the text debounce buffer before popping the
slot, like every other task exit, so a debounced text isn't orphaned.
A pre_gateway_dispatch rewrite copy of an event with no message_id was never
matched to the dispatched event, so it still hot-looped (#123229). Fall back to
the copied timestamp when message_id is empty (a genuine new message gets a
fresh one); a plain message_id==/timestamp== form breaks the runner's re-queue
of a new object with the same id.
Cap the back-off at 1s: nothing wakes the sleep, so a genuine message merged
into the slot meanwhile waited up to 5s; 1 dispatch/s is still ~250x below the
unbounded loop and needs no new wake plumbing.
Measure the 1s window after the first dispatch so a cold first handler call under load
cannot false-green the no-fix mutation; parametrize over interrupt demotion and steer
fallback; cover cancel during back-off keeping the event queued and the /stop tail
replaying it. Reuse RestartTestAdapter instead of a duplicate adapter.
The _busy_requeued tag was reset only by an untagged drain, so chained queue/steer
follow-ups reaching the runner fast path (split-brain adapter, multiplex routes) backed off
exponentially again (0.05->2s gaps vs ~0.01 on main). Key the back-off on identity instead:
the drained event must be the one this task just dispatched (same object or same message_id
for rewrite-hook copies). That covers every demotion site (interrupt demotion, queue, steer
fallback, Telegram grace queue, agent-starting merge) with no per-site tagging, so the tag
and _hm_tag_busy_requeue are dropped.
A backed-off event now stays in _pending_messages during the sleep and is popped after it,
so a cancel needs no put-back and can no longer drop the older message when a newer one
took the slot. jittered_backoff import moved to module top (agent.retry_utils is stdlib-only).
The drain back-off keyed on `response is None`, but None is the normal
return for every streamed turn and for queue/steer busy modes, so
ordinary chained follow-ups were delayed 0.25s -> 5s forever. The busy
fast-path in run_inbound now tags the adapter's pending head
(`_busy_requeued`) where it demotes the same inbound event (or its
rewrite-hook copy) back into the queue (interrupt demotion, queue mode,
steer fallback); the drain backs off only for a tagged event and
otherwise resets the counter and dispatches at once — single reset owner.
Also: clear _requeue_counts on cancel_session_processing, stale-lock
heal, session end and shutdown; restore the pending event if cancelled
during the back-off sleep; reuse agent.retry_utils.jittered_backoff.
The regression test now chains 3 genuine follow-ups after a streamed
(None) turn and requires each to dispatch in <0.1s (red on pre-fold).
The runner busy-demotion puts the event back into the adapter pending slot and
returns None; the in-band drain re-dispatched it at once, ~400 times/s for the
whole busy window (typing churn, log flood, platform connection storm).
Count consecutive unanswered re-queues per session on the adapter (not by event
identity, so a pre_gateway_dispatch rewrite via dataclasses.replace is still
caught), reset when a handler returns a response. First re-queue stays
immediate (restart auto-resume self-bounce); later ones back off 0.25s..5s.
The delay is slept inside the new drain task before its processing
try/finally, so cancelling during the back-off cannot reach
_finish_session_task late-arrival respawn (no concurrent handler, no
untracked task on shutdown).
Fixes#123229
Co-authored-by: ahisblessed <ahisblessed@users.noreply.github.com>
A busy-demoted event that the runner puts back into the adapter pending slot
is re-dispatched by the in-band drain with zero delay (~400/s). Salvaged from
PR #123259 (test only; the fix is rebuilt separately).
Refs #123229
(cherry picked from commit f83f4c958514c0be6e3f37af9d6ac5afc62fdf16, test file only)
Quitting the Desktop app wrote "[boot] Restarting desktop connection" to
desktop.log and pushed the same message to the renderer over
hermes:boot-progress. Nothing was restarting: the quit coordinator called
teardownPrimaryBackendAndWait() with the default soft=false, and soft=false
is what makes resetHermesConnectionState() call
resetBootProgressForReconnect().
Name the two teardown intents in quit-teardown.ts and use them at every
deliberate primary teardown: a teardown that re-homes ('reconnect', update
hand-off and bundle swap) keeps the announcement; one that brings nothing
back ('quit', the quit coordinator and the uninstall teardown) stays silent.
Fixes#123437.
