fix(state): stop the unbounded state.db repair loop (salvage of #88224) by kshitijk4poor · Pull Request #88425 · NousResearch/hermes-agent

kshitijk4poor · 2026-08-17T13:04:13Z

Summary

Stops the unbounded state.db repair loop that filled a disk (89 GB over 11 days in the reporting install) and hardens the forensic-backup path against reusing or publishing an incomplete recovery image. Salvage of @jirathip-k's #88224, rebased onto current main and composed with the merged repair-durability work (#91852).

Root cause of the loop: both #86747 guards keyed on size:mtime_ns, which is wrong for the malformed-schema class — that DB still opens and accepts writes (only sqlite_master is unreadable), so live writers, WAL checkpoints and the in-place repair strategies all move mtime between passes. Every pass looked like a new file: the attempt counter reset to 1 forever (cap never reached) and the backup dedupe never matched (a fresh full-size copy every pass).

Changes

Repair-epoch fingerprint (_db_fingerprint) → size + a bounded head/tail content sample with SQLite's commit-counter header fields masked out, so an ordinary write no longer re-keys the attempt ledger. POSIX-advisory-lock-safe (offline_file_access); returns None (never a differently-shaped key) when a live peer makes the read unsafe.
Attempt budget survives peer connections and liveness alternation across passes.
Forensic backup: atomic staging, proportional low-disk refusal (fail-closed, state.db repair/re-corrupt cascade: schema surgery is only serialized in-process, and sqlite_master edits never bump the schema cookie #69603), rollback journal included in the bundle, retention cap.

Data-integrity fixes (this revision — addressing @andrexibiza's review)

Forensic dedupe no longer reuses the repair-epoch fingerprint. _db_fingerprint answers "same damage epoch?" (masks counters, samples head/tail); reusing it as "same recovery image?" was wrong — a live writer committing rows into an interior page (size preserved, head/tail untouched) collided under it, so _backup_db_file could hand back a stale backup that predates real user data. New _backup_content_identity() digests the whole file + every sidecar (length-delimited, prefix-free) and the dedupe uses it. The O(n) read is only taken when a prior backup exists, and is cheaper than the O(n) copy it avoids on a hit.
Backup bundle is now published atomically. The old promotion loop os.replaced files one at a time (main first) and cleanup unlinked only staging sources — so a sidecar failure after the main promotion left a countable-but-incomplete bundle that passed the state.db repair/re-corrupt cascade: schema surgery is only serialized in-process, and sqlite_master edits never bump the schema cookie #69603 hard stop and deduped as legitimate next pass. Now sidecars publish first and the main DB last (its name is the commit marker _existing_malformed_backups counts), and any failure rolls back every already-published destination.

Validation

tests/test_state_db_repair_loop_mtime.py + repair/durability blast radius: 57 passed, 1 skipped (the WAL-only live-writer test auto-skips on WAL-reset-vulnerable SQLite).
Two new regressions, both mutation-checked (each fails on pre-fix code):
- test_backup_not_deduped_after_interior_page_write — interior-page write forces a fresh backup.
- test_publication_failure_leaves_no_countable_partial_bundle — a mid-publish os.replace failure leaves no countable main backup.
Composes cleanly with fix(state): apply macOS write barriers on every state.db repair connection #91852 (barriers + live-writer guard): shared offline_file_access liveness source, no conflicting repair-strategy edits. ruff clean.

Provenance / interlocks

@jirathip-k's two #88224 commits are preserved verbatim at the base of the stack (authorship intact); the fingerprint/budget/backup hardening and the two data-integrity fixes are on top. This is the corrective superseder for the malformed-schema blind spot in the #86867 persistent-repair-budget work, not a duplicate. #87409 (@cervantesh, scratch-DB repair so a failed repair can't destroy the original) is complementary at the same mutation boundary and is the natural next composition; this PR hardens the backup/ledger and does not close producer-side corruption causes (#80255, #73411).

Closes #88224.

Co-authored-by: jirathip-k 115384744+jirathip-k@users.noreply.github.com

andrexibiza

Reviewed exact head a9f94ce1aa28a605a2f78f863e6d10dbe5657e53 against recorded base / merge-base cf64ca20c5ab99ebf7e8ca272c69edc7ea0636ed and current main fab8479aa005e3c6f9e779a2b9da4e186a17db4a. This PR has no prior human review/comments on the exact head. It is currently 7 commits ahead and 573 commits behind main, non-mergeable, so everything below is a semantic review of the exact current branch plus a restack gate.

The branch closes several real failure modes from #88224: the mtime-based repair budget was structurally wrong for malformed-schema databases that still accept writes; the raw-fd lock cancellation issue is correctly avoided with offline_file_access; the liveness-alternation budget fix is directionally right; rollback journals belong in the forensic bundle; low-disk refusal is correctly fail-closed; and the staging prefix fix prevents copy-stage debris from being counted as a backup. The tests are unusually good at reproducing those specific failures.

I found two blocking data-integrity gaps in the new backup identity/publication model.

1. One fingerprint is being asked to mean two incompatible things

_db_fingerprint() is deliberately designed as a repair-epoch identity, not a byte-identity: it samples only head/tail, masks SQLite's ordinary commit counters, and the new test_ordinary_commit_does_not_rekey_the_fingerprint explicitly requires a normal database write to leave the fingerprint unchanged. That is the correct semantic for the attempt ledger: a live write should not mint a fresh automatic-repair budget.

But _backup_db_file() reuses the same fingerprint as proof that an existing forensic backup is "identical to the damaged DB" and can be reused. Those are different equivalence relations.

Concrete failure shape:

Take forensic backup A of a multi-page malformed-schema DB.
A live writer commits new transcript/session data into an interior page while preserving file size. The branch intentionally masks the header counters, and a middle-page update can leave the first/last 64 KiB unchanged.
_db_fingerprint(current) == _db_fingerprint(A) by design.
_backup_db_file() returns A instead of taking backup B.
This exact head still runs repair strategies in place. #87409 demonstrates that a failed schema repair can VACUUM away canonical tables and still return repaired=False. The only accepted forensic copy can therefore predate user data that existed immediately before the destructive pass.

Even if #87409 lands first and makes failed repairs non-destructive, the semantic bug remains: a forensic recovery point must not claim byte/state identity while knowingly ignoring ordinary durable writes. A later successful-but-lossy repair or manual restore would still hand the operator a stale snapshot while the log says it was "identical".

Required fix: split the identities. Keep a repair-epoch fingerprint that is stable across ordinary writes for the attempt ledger, and use a backup-content identity that detects every durable byte/state change relevant to recovery (including sidecars) for forensic dedupe. Since the expensive operation being avoided is another full write, an O(n) read/hash at backup time is a much safer trade than treating a 128 KiB sample as backup equality; hashing while copying or validating an already-copied candidate are both viable shapes. Add a regression with a DB larger than the sample windows: backup once, mutate an interior page without changing size/head/tail, then prove the next backup is not deduped.

This is the deeper class boundary: "same corruption incident" is not the same predicate as "same recovery image." They should not share one key.

2. The staged backup bundle is still not atomically published

The new staging name fixes copy-stage partials, but the promotion loop publishes the final bundle one file at a time:

for src, dst in staged:
    os.replace(src, dst)

staged contains the main DB first, then WAL/SHM/journal. If the main os.replace succeeds and a later sidecar replace fails (ENOSPC, antivirus/permission race, crash), the exception cleanup only unlinks the staging src paths. It does not remove destinations already promoted. The final-prefix main backup remains on disk.

On the next pass _existing_malformed_backups() sees that main file as a legitimate completed backup, and the dedupe checks only the main-file fingerprint before returning it. That recreates the exact class the staging work is trying to eliminate: an incomplete forensic bundle can become the official backup_path and pass the #69603 hard stop.

The existing test_failed_copy_leaves_no_countable_debris fails a sidecar during copy2, before any os.replace; it does not exercise the publication failure window.

Required fix: make the main backup name the commit marker, not the first published member. Publish/validate sidecars first, publish the main DB last, and on any failure remove every final destination already published. A small manifest/commit marker would also work if you prefer explicit bundle completeness. Add a regression that makes os.replace fail on the second or third promotion and proves (a) no countable main backup remains and (b) the next _backup_db_file() cannot dedupe/reuse the partial bundle.

Interlocks / merge order

#88224 / @jirathip-k is the source implementation being salvaged here. Keep that authorship/provenance exactly as the PR states.
#86867 / @teknium1 is the merged predecessor that introduced the persistent repair budget and backup cap after the #86747 / @jermynyee 89GB incident. #88425 is a corrective superseder for the malformed-schema blind spot, not a duplicate.
#87409 / @cervantesh is overlapping and complementary at the same mutation boundary: it moves repair strategies onto a scratch DB so a failed repair cannot destroy the original. I would land/compose that invariant before restacking this PR; otherwise #88425 is hardening the backup/ledger around an in-place repair architecture already proven capable of transcript destruction.
#89073 / @the3asic is complementary derived-index work: it keeps corrupt FTS rebuilds off the live gateway path and changes _db_opens_cleanly / repair semantics. It overlaps the same hermes_state.py region and needs semantic composition, not conflict-only resolution.

The clean target architecture is: canonical data remains untouched by failed automatic repair; derived FTS degradation stays online-safe; repair budgets identify the same damage epoch; forensic dedupe identifies the same recovery image; and a backup bundle is published only when complete.

CI / restack truth

Exact-head Docker is green. Hosted CI's substantive jobs shown on the run are green (Python, e2e, Windows/macOS, Ruff/ty, docs, supply-chain, OSV); the run is red because the review-label gate was missing, not because a test failed. That evidence is also old relative to current main. The branch is 573 commits behind and non-mergeable, so after the two blockers above are closed it needs a proof-preserving rebase/composition with #87409/#89073 as applicable and a fresh exact-head matrix.

Re-review gate: separate repair identity from forensic equality; make multi-file backup publication fail-atomic; compose the non-destructive/FTS repair topology; rebase current main; rerun exact-head CI.

@jirathip-k

…er guard Follow-up to the salvaged repair-durability commit. Scope corrections so this PR ships only the reachable, non-competing, WAL-mode-correct half: - Drop verify_state_db_integrity() + its 4 tests. Zero production callers here (dead code); the caller lives in the follow-up that wires it into SessionStore._open_session_db_for_active_scope() (PR NousResearch#91754). The function moves with its wiring. - Drop the _db_fingerprint change (size:mtime_ns -> dev:ino:size) + its 3 ledger tests. This is competing work: PR NousResearch#88425 (salvage of @jirathip-k's NousResearch#88224) already fixes the same size:mtime_ns budget-reset bug with a content-sample + volatile-header-mask that also handles the DELETE-mode commit-counter case, and carries @jirathip-k's diagnosis/credit. Landing a second, divergent fingerprint contract would stomp that lineage. Fingerprint stays with NousResearch#88425; this PR reverts _db_fingerprint to main's form. - Mark test_repair_refuses_while_another_connection_holds_the_db requires_wal. _live_writer_holds_db detects an out-of-process holder via the WAL-index exclusive lock, absent in journal_mode=DELETE (used on WAL-reset-vulnerable SQLite <3.51.3 incl. CI's 3.50.4, and on NFS/SMB). The test failed there; the conftest requires_wal gate auto-skips it. DELETE-mode limitation is now documented on the guard docstring: repair is serialised only by the cross-process repairer lock there. The reported incident was in WAL mode. - Map dhanesh@users.noreply.github.com -> dhanesh (contributors/emails) so the attribution CI gate passes. Net: this PR is repair-connection durability barriers + the live-writer guard. addresses @andrexibiza's NousResearch#90747 review (dead-code verifier + fingerprint interlock with NousResearch#88425).

@jirathip-k

…er guard Follow-up to the salvaged repair-durability commit. Scope corrections so this PR ships only the reachable, non-competing, WAL-mode-correct half: - Drop verify_state_db_integrity() + its 4 tests. Zero production callers here (dead code); the caller lives in the follow-up that wires it into SessionStore._open_session_db_for_active_scope() (PR #91754). The function moves with its wiring. - Drop the _db_fingerprint change (size:mtime_ns -> dev:ino:size) + its 3 ledger tests. This is competing work: PR #88425 (salvage of @jirathip-k's #88224) already fixes the same size:mtime_ns budget-reset bug with a content-sample + volatile-header-mask that also handles the DELETE-mode commit-counter case, and carries @jirathip-k's diagnosis/credit. Landing a second, divergent fingerprint contract would stomp that lineage. Fingerprint stays with #88425; this PR reverts _db_fingerprint to main's form. - Mark test_repair_refuses_while_another_connection_holds_the_db requires_wal. _live_writer_holds_db detects an out-of-process holder via the WAL-index exclusive lock, absent in journal_mode=DELETE (used on WAL-reset-vulnerable SQLite <3.51.3 incl. CI's 3.50.4, and on NFS/SMB). The test failed there; the conftest requires_wal gate auto-skips it. DELETE-mode limitation is now documented on the guard docstring: repair is serialised only by the cross-process repairer lock there. The reported incident was in WAL mode. - Map dhanesh@users.noreply.github.com -> dhanesh (contributors/emails) so the attribution CI gate passes. Net: this PR is repair-connection durability barriers + the live-writer guard. addresses @andrexibiza's #90747 review (dead-code verifier + fingerprint interlock with #88425).

@jirathip-k

…er guard Follow-up to the salvaged repair-durability commit. Scope corrections so this PR ships only the reachable, non-competing, WAL-mode-correct half: - Drop verify_state_db_integrity() + its 4 tests. Zero production callers here (dead code); the caller lives in the follow-up that wires it into SessionStore._open_session_db_for_active_scope() (PR NousResearch#91754). The function moves with its wiring. - Drop the _db_fingerprint change (size:mtime_ns -> dev:ino:size) + its 3 ledger tests. This is competing work: PR NousResearch#88425 (salvage of @jirathip-k's NousResearch#88224) already fixes the same size:mtime_ns budget-reset bug with a content-sample + volatile-header-mask that also handles the DELETE-mode commit-counter case, and carries @jirathip-k's diagnosis/credit. Landing a second, divergent fingerprint contract would stomp that lineage. Fingerprint stays with NousResearch#88425; this PR reverts _db_fingerprint to main's form. - Mark test_repair_refuses_while_another_connection_holds_the_db requires_wal. _live_writer_holds_db detects an out-of-process holder via the WAL-index exclusive lock, absent in journal_mode=DELETE (used on WAL-reset-vulnerable SQLite <3.51.3 incl. CI's 3.50.4, and on NFS/SMB). The test failed there; the conftest requires_wal gate auto-skips it. DELETE-mode limitation is now documented on the guard docstring: repair is serialised only by the cross-process repairer lock there. The reported incident was in WAL mode. - Map dhanesh@users.noreply.github.com -> dhanesh (contributors/emails) so the attribution CI gate passes. Net: this PR is repair-connection durability barriers + the live-writer guard. addresses @andrexibiza's NousResearch#90747 review (dead-code verifier + fingerprint interlock with NousResearch#88425).

A malformed-schema state.db sent Hermes into a repair loop that wrote a fresh full-size forensic backup every ~10s: 31 copies / 2.3GB in 20 minutes, free space heading to zero on a host running an agent fleet. The NousResearch#86747 guards for exactly this were already present and did not hold. Both keyed on `size:mtime_ns`: * `_db_fingerprint` -> the ledger's attempt counter reset to 1 on every pass, so `_MAX_PERSISTENT_REPAIR_ATTEMPTS` was never reached and the loop never terminated; * `_backup_db_file`'s dedupe compared mtime, so it never matched and each pass wrote another full-size copy. The assumption behind that key -- "nothing can successfully write to a damaged file" -- holds for the b-tree damage of NousResearch#86747 but not for the malformed-SCHEMA class: the DB still opens and accepts writes (only sqlite_master is unreadable), so live writers, WAL checkpoints and the in-place repair strategies themselves all move mtime between passes. Fixes: * fingerprint on size + a bounded head/tail content sample instead of mtime. Stable across passes that merely touch the file, still changes on genuine repair/truncation/restore (so recovery resets the budget), and stays O(1) on a multi-GB DB. * dedupe the forensic backup on that same fingerprint. * add the missing free-space guard: refuse the pre-repair copy when it would leave under 2GiB free, with an actionable error. The backup is a full raw copy of the damaged DB, so a repair loop is a disk amplifier that can take down every process on the host -- and the refusal path already hard-stops the repair (NousResearch#69603) rather than mutating the only remaining copy. Tests fail on the unfixed tree and pass here; the pre-existing failures in test_state_db_malformed_repair.py and TestFTS5Search are unrelated and reproduce on the base commit.

Follow-up to adversarial review of the first commit. Three findings, two confirmed by test and fixed here, one disproven and left alone. CONFIRMED — the free-space guard was a threshold, not cleanup. Prune runs only on the success path, so any copy that failed partway (ENOSPC, sidecar copy failure, kill mid-copy) left a file matching the `malformed-backup-` prefix that nothing ever removed. Measured on the unpatched tree: backups capped at 3 while copies succeed, but 13+ and climbing once copy2 raises — self-reinforcing, since each partial consumes the space that guarantees the next failure. Worse, partials sort newest-by-name, so a later successful prune KEPT the garbage and deleted the intact forensic copies. Fix: copy to a `.incomplete` staging name that does not match the backup prefix, os.replace into place only after every copy succeeds, unlink staging on failure, and sweep stale staging debris on entry. CONFIRMED — the 2GiB floor was a small-volume regression. A 50MB DB on a 10GB volume with 1.5GB free (30x headroom) was refused, and since a refused backup is a HARD STOP (NousResearch#69603) that silently converts "repair loops" into "repair never runs". Fix: require the copy itself (now including its -wal/-shm sidecars, which the old check ignored) plus proportional headroom — max(256MiB, 2% of volume). DISPROVEN — the review claimed a refused backup skips _record_repair_outcome so the loop never terminates. It does not: repair_state_db_schema records the outcome on the result returned by _repair_state_db_schema_locked, which is where the hard stop returns. Verified on a simulated low-disk host: terminal at pass 4 with zero backups written. No change made. Tests: 5 new (small-volume allow, proportional headroom, sidecar accounting, failed-copy leaves no countable debris + staging swept). 23 pass with the NousResearch#86747 suite; test_hermes_state.py 252 passed. Pre-existing unrelated failures unchanged.

…y locks The content fingerprint takes a raw descriptor, and close() on ANY descriptor cancels every POSIX advisory lock the process holds on that file. The exhaustion probe runs before _backup_db_file's has_live_connection guard, so the read happened even when a peer SessionDB held a write lock. Verified end-to-end (journal_mode=DELETE, gateway mid-turn write, peer in a subprocess): before peer BLOCKED -> repair -> peer BLOCKED, holder COMMIT ok unfixed peer BLOCKED -> repair -> peer STOLE the lock, holder COMMIT: disk I/O error WAL is immune (it coordinates through -shm), but DELETE is what Hermes falls back to on NFS/SMB/FUSE/ZFS and on SQLite builds vulnerable to the WAL-reset bug, so this is a real deployment shape. Run the read under offline_file_access and fall back to size:mtime_ns when a connection is live. That keeps the ledger counting instead of returning None (which reads as "not exhausted" and would restore the unbounded loop), and the content key stays load-bearing on the offline repair path -- the only path where surgery actually runs. Also fail the free-space guard CLOSED: a nearly-full volume is exactly where statvfs is likeliest to fail, and proceeding is the multi-GB copy that finishes off the disk.

The staging name was derived from the backup name (`<db>.malformed-backup-<stamp>.incomplete`), which still matches the prefix `_existing_malformed_backups` selects on -- it excludes only `-wal`/`-shm`. Three consequences, all reproduced: - it is COUNTED as a forensic backup; - it sorts NEWEST (`.incomplete` > the bare stamp), so prune's keep-3-newest slice retained partials and deleted intact copies -- the exact inversion the staging change was meant to prevent; - worst, the dedupe ran BEFORE the sweep, and a staging file orphaned by a kill mid-copy is a byte-identical copy of the damaged DB, so its fingerprint MATCHES and it was handed back as the official `backup_path`. Repair then passed the NousResearch#69603 hard-stop gate and ran destructive surgery believing a forensic copy existed, and the next pass's sweep deleted that very file. Move staging outside the prefix (`<db>.backup-staging-<stamp>`) and sweep before the dedupe. The sweep also matches the pre-merge `.incomplete` spelling so a host that ran the earlier build does not keep prefix-matching debris that sorts newest and survives prune forever. Before / after on the same fixture (orphaned staging + a later pass): before backup_path = ...malformed-backup-<stamp>.incomplete (staging!) pass-1 forensic copy deleted by the next sweep after backup_path = ...malformed-backup-<stamp> (real copy) debris swept, pass-1 forensic copy preserved

Self-review of the previous commit found it reintroduced the bug this PR exists to fix, by a different route. `_db_fingerprint` fell back to `size:mtime_ns` when a live connection made the content read unsafe. The ledger compares keys for EQUALITY, and the two keys have different SHAPES, so a gateway peer connecting between passes flipped the shape and the counter reset to 1 every time: pass 1 [offline] attempts=1 fp=8192:58c7924f0fba... pass 2 [LIVE ] attempts=1 fp=8192:1786972039271402096 pass 3 [offline] attempts=1 fp=8192:58c7924f0fba... ... never reaches _MAX_PERSISTENT_REPAIR_ATTEMPTS Return None instead, and teach the two ledger helpers to cope: - `_persistent_repair_attempts_exhausted` falls back to the recorded key's SIZE prefix (the one component both shapes share and that needs no raw read) rather than reading as "not exhausted" — otherwise a peer connection hides an exhausted budget on every pass, same loop. - `_record_repair_outcome` keeps the key already on record and still increments, rather than dropping the pass. pass 1 [offline] attempts=1 pass 2 [LIVE] attempts=2 pass 3 [offline] attempts=3 pass 4 [LIVE] BLOCKED Intra-pass flips were already safe (the probe and the record are both reached with the same liveness within one `repair_state_db_schema` call); it is the cross-pass change that desynced. Also drops two `type: ignore` directives `ty` flagged as unused, and replaces the `LiveConnectionError = ()` / `nullcontext()` shim with a real no-op contextmanager + exception class so the scaffold-install path is honest.

Third self-review pass found the content fingerprint was still defeated on rollback-journal deployments, by the same mechanism as the original mtime bug. The head sample starts at byte 0, so it covers the database header's file change counter (bytes 24-27) and version-valid-for (92-95). In DELETE mode a commit writes the main file directly and bumps both. A malformed-SCHEMA DB still accepts writes -- that is the whole premise of this PR -- so any ordinary session write between passes re-keyed the ledger: DELETE, 18MB db, one peer UPDATE between passes (before this commit) pass 1..6: attempts=1 every pass, exhausted=False -> unbounded loop after pass 1..3: attempts=1,2,3 pass 4: BLOCKED WAL is unaffected (commits land in -wal; the main header only moves on checkpoint), so this was invisible on a WAL host and reproducible on every NFS/SMB/FUSE/ZFS or WAL-reset-vulnerable host -- exactly the deployments the earlier lock-safety commit was written for. Mask the two volatile ranges out of the sample. Page 1's sqlite_master b-tree sits after byte 100 and stays in, so genuine recovery still resets the budget: verified schema rewrite, index rebuild, VACUUM and truncation all change the key, while a bare utime and an ordinary commit do not. Test-cost cleanup in the same file, since the new tests needed a larger-than-sample fixture and the file was already slow: - the two guard tests that allocated 450MB of os.urandom now use sparse truncate (both only ever read st_size), and the new fixtures use 600 rows rather than 40k; - file runtime 127s -> 35s.

The pre-repair copy took only -wal/-shm. In rollback-journal (DELETE) mode -- Hermes's fallback on NFS/SMB/FUSE/ZFS and on WAL-reset-vulnerable SQLite builds -- a hot <db>-journal exists on disk whenever a transaction was open, and that file is what rolls the damaged bytes back to a consistent state. A forensic copy without it cannot be recovered by hand, which is the entire purpose of taking the copy before destructive surgery. Verified the journal is really there: files while a txn is open: ['state.db', 'state.db-journal'] files after commit: ['state.db'] Add _DB_SIDECAR_SUFFIXES = ("-wal", "-shm", "-journal") and use it at the four sites that must agree: the disk-guard sizing, the staging copy, the backup-count exclusion in _existing_malformed_backups (so a copied journal is not itself counted as a forensic backup), and _prune_malformed_backups (which otherwise leaks one journal per pruned backup, quietly defeating the retention cap this PR is partly about). Matches the spelling hermes_cli/session_recovery.py:61 already uses for the same concept.

@andrexibiza

…int; publish backup bundle atomically Addresses two data-integrity gaps @andrexibiza flagged reviewing NousResearch#88425. 1. Forensic dedupe no longer reuses the repair-epoch fingerprint. _db_fingerprint masks SQLite's commit counters and samples only head/tail so an ordinary write does not re-key the repair budget — the right predicate for 'same damage epoch', the WRONG one for 'same recovery image'. A live writer committing rows into an interior page (size preserved, head/tail untouched) collided under it, so _backup_db_file handed back a STALE backup that predates real user data. New _backup_content_identity() digests the whole file + every sidecar; the dedupe uses it. The O(n) read is cheaper than the O(n) copy it avoids on a hit. 2. Backup bundle is now published atomically. The promotion loop replaced files one at a time (main first) and cleanup unlinked only staging srcs, so a sidecar os.replace failure after the main promotion left the final-prefix main backup on disk — a countable-but-incomplete bundle that passed the NousResearch#69603 hard stop and deduped as legitimate next pass. Now sidecars publish first and the main DB last (its name is the commit marker _existing_malformed_backups counts), and cleanup rolls back every already-published destination. Two regressions added (both mutation-checked — each fails on pre-fix code): - test_backup_not_deduped_after_interior_page_write - test_publication_failure_leaves_no_countable_partial_bundle tests/test_state_db_repair_loop_mtime.py: 28 passed.

kshitijk4poor · 2026-08-22T09:03:33Z

Thanks for the deep review — both blocking findings were real and are now fixed on the current head (dbf41d1201), and the branch is rebased onto current main (mergeable).

1. Fingerprint conflated two equivalence relations — fixed. You're right that _db_fingerprint is a repair-epoch identity (masks commit counters, samples head/tail) and must not double as forensic-backup equality. Added _backup_content_identity() — whole main file + every sidecar, length-delimited so the concatenation is prefix-free — and switched _backup_db_file's dedupe to it. New regression test_backup_not_deduped_after_interior_page_write builds a DB larger than the sample window, mutates an interior page (size + head/tail preserved), and proves the next backup is NOT deduped; it fails on the pre-fix code (mutation-checked).

2. Non-atomic bundle publication — fixed. The promotion loop published main-first and cleanup only removed staging sources, so a sidecar os.replace failure after the main promotion left a countable partial bundle. Now sidecars publish first and the main DB publishes last as the commit marker, and any failure rolls back every already-promoted destination. New regression test_publication_failure_leaves_no_countable_partial_bundle fails os.replace mid-publish and asserts no countable main backup survives (distinct from the existing copy-stage-failure test, which fails before any os.replace). Also mutation-checked.

On the interlocks: agreed that #87409 (scratch-DB repair) is the natural composition at this mutation boundary — this PR hardens the backup/ledger and explicitly does not claim to close the producer-side causes (#80255, #73411). @jirathip-k's two #88224 commits are preserved verbatim at the base of the stack.

cervantesh · 2026-08-22T23:56:20Z

Following up on the composition point in this resolution: #87409 is now rebased onto current main and implements the complementary non-destructive mutation boundary.

It preserves #88425''s split between repair-epoch identity and forensic-content identity, atomic sidecar-first/main-last backup publication, retention, rollback-journal coverage and fail-closed disk guard. Repair outcomes are now recorded under the same cross-process lock, with a second exhaustion check after lock acquisition; deterministic corruption consumes the budget while transient environmental aborts do not.

All repair strategies run on an Online Backup snapshot, and one retained SQLite guard excludes writers continuously through staging and transactional promotion. Current #87409 head: 7aeb0a45b2e5204311ce4ca4fcbcae918d642dc7; focused composed suite: 81 passed, 3 platform-gated skips.

* chore: AUTHOR_MAP — add samtcam@gmail.com → samclams For PR #91806 salvage (multiplex refusal exit code fix). * fix(gateway): multiplex refusal must exit EX_CONFIG (78), not 1 `_guard_named_profile_under_multiplexer` correctly refuses a named-profile gateway while the default gateway is multiplexing — starting a second one would double-bind that profile's platforms. The refusal is right; its exit code was not. The refusal is decided entirely by configuration (`multiplex_profiles` plus the allowlist), so it is permanent: no number of retries can change the answer. Exiting 1 made it look transient to a service manager. That matters because this module generates the systemd unit, and the template pairs `Restart=always` / `RestartSec=5` with `StartLimitIntervalSec=0` — it deliberately trades systemd's generic start-rate limiter for the specific `RestartPreventExitStatus=GATEWAY_FATAL_CONFIG_EXIT_CODE` backstop declared three lines below it. Returning 1 left that backstop unarmed with the limiter already disabled, so a correct, permanent refusal became an unbounded restart loop. Observed on a host running `multiplex_profiles: true` with a leftover per-profile unit: 136 refusals in ~13 minutes, stopped only by hand. `GATEWAY_FATAL_CONFIG_EXIT_CODE` (78, EX_CONFIG) is this codebase's existing answer for exactly this case — `gateway/restart.py` documents it as the fatal configuration error that the s6 finish script translates into 125 "permanent failure" (#51228). This adopts that contract rather than inventing one, so the fix also works on s6 hosts, not just systemd. After: one refusal, `status=78/CONFIG`, `NRestarts=0`, unit settles in `failed`. Also strengthens the two guard tests. They asserted `pytest.raises(SystemExit, match="1")`, but `match=` is a regex search over `str(exc)`, so it passed for 1, 21, 100 and 111 alike — it read like an exit-code assertion while pinning nothing. They now assert `excinfo.value.code == GATEWAY_FATAL_CONFIG_EXIT_CODE`. The exit code is the contract here: it is the only thing that tells a supervisor the failure is permanent. * fix(cron): nudge review of escaped-run failures too A recurring job that fails at the scheduler layer - an exception escaping run_one_job's body before the agent is ever constructed - has delivered a failure alert since 4668750fa. It has never carried the repeated-failure review nudge the normal agent-failure delivery carries: the nudge (#80752, 2026-08-06) predates that second delivery site by eight days and only ever composed the first one. The streak itself is layer-agnostic. mark_job_run increments failure_streak for an escaped failure exactly as it does for an agent failure, and the escape handler calls it. So the counter climbs correctly and shows up in `hermes cron list`, but the chat message that spends it is unreachable for a job whose failures ALL escape - a half-applied update leaving a bad import, a provider client that cannot construct. Those are precisely the failures that repeat identically on every tick, so the operator gets the same one-line error every 10 minutes indefinitely and is never told the automation itself is worth reviewing or pausing. Compose the nudge at the escape handler's delivery exactly as the normal path does. It stays config-gated and threshold-gated by the same helper, so a first-time escaped failure reads exactly as it did before. Docs said the streak counts "runs where the agent failed", which is what the reporter read and reasonably concluded their failures were out of scope. The counter never worked that way; correct the sentence to match the code. Tests: two cases on the escaped-failure delivery path - streak at threshold appends the nudge (fails on the unfixed handler with the bare summary), and streak below threshold delivers the unchanged one-liner, so the guard also proves the nudge is not unconditional. The existing nudge tests only ever exercised the helper in isolation, which is why the second delivery site could be added without it. Fixes #88655 * fix(telegram): rebuild after cancellation-shielded stop Use the existing wall-clock deadline helper for updater.stop() during network recovery. If PTB cleanup remains cancellation-shielded past the deadline, escalate to retryable fatal recovery so the runner builds a fresh adapter instead of calling start_polling() while the old Updater may still hold its lifecycle lock. Add regression coverage with stop() swallowing cancellation while holding the same lock start_polling() needs, and verify the old Updater is never reused. * fix(telegram): widen cancellation-shielded stop to sibling paths The network-error reconnect path (PR #91524) was the only site converted from asyncio.wait_for to _await_with_thread_deadline. The same cancellation-shielding vulnerability exists at two more updater.stop() sites: - Conflict-retry path: asyncio.wait_for could hang forever if PTB/AnyIO cleanup swallowed CancelledError, stalling the conflict-retry ladder. Now uses _await_with_thread_deadline and escalates to fatal on timeout (same reasoning: cannot safely reuse an Updater whose lifecycle lock may still be held). - Conflict-exhausted fatal path: asyncio.wait_for could hang before the fatal notification fired. Now uses _await_with_thread_deadline; the timeout handler already proceeds to fatal notify, so no behavior change beyond the deadline mechanism. All three asyncio.wait_for(updater.stop()) sites now use the thread-deadline helper consistently. * fix(zai): GLM-5.3 low/medium reasoning effort reaches the wire instead of clamping to high GLM-5.3 accepts a graded low/medium/high/max reasoning_effort scale (verified live in #91789: monotonic reasoning-token scaling, no 400s), but the effort mapper reused GLM-5.2's two-level vocabulary, silently rewriting low/medium to high. Adds GLM53_EFFORTS/GLM53_OVERRIDES and a per-model vocabulary pick in the zai plugin; 5.2 keeps its high/max clamp. Closes #91789. Also covers the gap noted when closing #86947 (credit @santhanakrishnan-d and @terje1965 for the graded-scale finding). * fix(telegram): keep DM-topic tables on sendRichMessage when drafts degrade #91241 stopped root-DM tables collapsing to bullets by keeping native draft transport when rich_drafts is off. Private Telegram topics still reject sendMessageDraft (string thread ids, forum-style thread fields), so the stream consumer falls back to edit-in-place. Telegram then rejects a rich edit of that plain MarkdownV2 preview and format_message permanently rewrites pipe tables into bullet lists — the remaining report after that merge. Route drafts through the same integer topic kwargs as send(), and on that degraded topic path prefer a fresh sendRichMessage (then delete the preview) instead of the table-to-bullets formatter. * test(telegram): cover DM-topic table streaming after draft degradation Pins integer topic routing on send_draft, a successful topic stream that finalizes through sendRichMessage, and the reporter path where sendMessageDraft and in-place rich edits both fail — the persistent payload must still be the raw pipe table, not convert_table_to_bullets. * fix(telegram): honor the direct-messages-topic alias in the fresh-final gate prefers_fresh_final_streaming read only the raw direct_messages_topic_id key; the adapter's canonical accessor _metadata_direct_messages_topic_id also accepts the documented telegram_direct_messages_topic_id alias (treated as equivalent in gateway/delivery.py), so an alias-only lane would still flatten tables. Route the gate through the accessor and pin the alias with a regression (mutation-checked: raw-key gate fails it). Also reshape the happy-path endpoint assertion into the actual invariant (sendRichMessage present, no rich draft frames) instead of a frozen call list. Surfaced during review of PR #91436. * feat(bedrock): support OpenAI Responses models Route Bedrock-hosted OpenAI GPT-5.5 through the Bedrock Mantle OpenAI Responses endpoint with SigV4 request signing. Keep native Bedrock Converse and Claude Bedrock routing unchanged, and add picker/runtime regression coverage. * fix(moa): keep Bedrock slots on provider runtime Preserve the Bedrock provider identity for MoA reference and aggregator slots so Bedrock OpenAI Responses models use the aws_sdk/SigV4 runtime instead of being downgraded to a generic custom endpoint. Add regression coverage for Bedrock GPT-5.5 MoA slots. * feat(bedrock): add OpenAI GPT-5.6 family (Sol/Terra/Luna) to Mantle Responses routing GPT-5.6 Sol, Terra, and Luna went GA on Amazon Bedrock on 2026-07-13. Like GPT-5.5, they are served exclusively from the Bedrock Mantle OpenAI-compatible Responses endpoint (the model cards list bedrock-runtime/Converse as unsupported), so they ride the allowlist routing introduced for GPT-5.5: - Add openai.gpt-5.6-{sol,terra,luna} to BEDROCK_OPENAI_RESPONSES_MODEL_IDS so runtime resolution, auxiliary calls, and MoA slots all take the SigV4/bearer Mantle Responses path. - Surface the family in the curated Bedrock picker list. - Record the 272K context window from the AWS model cards for all four Mantle OpenAI models (previously fell back to the 128K default). - Generalize picker tests from the hardcoded single-model checks to the BEDROCK_OPENAI_RESPONSES_MODEL_IDS allowlist so future Mantle model additions do not require test surgery; add routing, picker, and context-length coverage for the 5.6 family. Docs: https://docs.aws.amazon.com/bedrock/latest/userguide/model-cards-openai.html * fix(bedrock): align auxiliary region resolution with runtime + document Mantle route Address review feedback on #65076: - Add resolve_bedrock_runtime_region() to agent/bedrock_adapter.py: the config-first region resolution (bedrock.region in config.yaml, then AWS_REGION/AWS_DEFAULT_REGION/botocore profile/us-east-1) that the main runtime resolver uses, exposed as a shared helper. - Switch auxiliary client resolution (agent/auxiliary_client.py aws_sdk branch) to the new helper. Previously it derived its region with bare resolve_bedrock_region() (env-first), so when config.yaml pinned bedrock.region to a different region than the ambient AWS env, auxiliary calls (compression, memory, vision) left the primary runtime's region. Both the AnthropicBedrock/Converse path and the new Mantle OpenAI Responses path now resolve identically to the main runtime. - Add regression tests covering the bedrock.region-vs-AWS_REGION mismatch for both the Claude auxiliary path and the Mantle auxiliary path. - Update website/docs/guides/aws-bedrock.md: the guide claimed Hermes never uses the OpenAI-compatible endpoint, which the Mantle route made stale. Document the triple routing (AnthropicBedrock / Mantle OpenAI Responses / Converse), the Mantle auth model (bearer token or SigV4), and add the GPT-5.5/5.6 model IDs to the models table. * refactor(bedrock): make resolve_bedrock_runtime_region the single region chokepoint Follow-up structural pass on the review fix: - Runtime provider, auxiliary resolution, model validation (hermes_cli/models.py), live discovery (bedrock_model_ids_or_none), and the Mantle URL/SigV4 fallbacks all resolve their region through resolve_bedrock_runtime_region() — one canonical implementation of the config-first priority instead of three hand-rolled copies. - agent_init: drop the 'if "client_kwargs" in locals()' guard by initializing client_kwargs unconditionally at the top of the else branch; the Mantle kwargs hook is a documented no-op for non-Mantle base URLs. * test: trim salvage of #65076 to a lean regression set Drop the bulk test additions from the original PR; keep only mandatory picker-assertion adaptations (Mantle IDs join the discovery lists), one allowlist routing test covering all four Mantle model IDs, the 272K context check, and the two review-mandated auxiliary regressions (config-region-beats-env for the Mantle path, aux Responses client). * chore: map salvage contributor emails * fix: hermes update no longer strands non-interactive updates on a parked branch with unmerged commits A clean checkout parked on a feature branch now always switches to the update target. Unmerged commits are safe on the branch (git checkout never discards committed work) and get a loud 'kept' notice naming the branch, count, and the checkout command to resume the work. Previously the update hard-skipped with exit 1 — a dead end for the desktop update button, gateway /update, and cron, which have no way to resolve a skip. Dirty trees (uncommitted changes) still skip loudly, and the updates.auto_switch_parked_branch: false opt-out still pins the branch. * feat(update): update branches carrying unmerged commits in place instead of skipping The parked-branch guard (8ce8ffd429) distinguishes checkouts by what the branch carries, then treats both non-clean cases the same: a stale fully-merged leftover is switched back to the target (correct), but a branch with unmerged commits — a branch someone is actually working on — gets CODE UPDATE SKIPPED and exit 1. For anyone running a maintained custom branch on top of main, every update now refuses, and the guidance ('checkout main') abandons their branch. The guard's own reason codes already separate the cases, so use them: - fully merged -> switch back to the target (unchanged) - unmerged:N -> update the branch IN PLACE: fetch, then bring origin/<target> into the checkout. Fast-forward when possible; on divergence, a true merge behind a pre-update safety tag, stopping cleanly on conflict. The checkout never moves; local commits survive; the running code advances. - dirty/unverifiable/opted out -> skip loudly (unchanged) The post-pull success gate learns that an in-place update legitimately ends on a non-target branch: origin/<target> was merged INTO the checkout, so refusing to claim success there would fail every update that did exactly the right thing. Guard tests updated: the unmerged case now asserts the in-place outcome — target code arrives (b.txt from c3), the branch's own commit survives, and HEAD never moves. 18/18 guard tests, 20/20 with the diverged-update suite. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(update): --switch-branch opts an unmerged branch out of the in-place merge Review feedback on #89507: in-place merging suits a branch that tracks the target with a small patch set, but a long-lived feature branch (a PR branch hundreds of commits deep) does not want an update-driven merge commit written into its history. Reported against a checkout carrying 819 unmerged commits. --switch-branch routes the unmerged case to the switch path instead: the checkout moves to the update target and updates there, and the branch is left byte-identical — no merge, no commit, nothing written to it. The tree is known clean on that path (the guard checks dirty before cherry), so a dirty tree still gets the loud skip, unchanged. Opt-in: without the flag the default remains the in-place update, which is what keeps a small-patch-set branch's running code current. Tests: the flag switches and leaves the branch tip byte-identical; the default without it still updates in place. The first fails if the flag's branch is severed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(update): updates.parked_branch_strategy gates the in-place merge; switch stays the default Adapts the in-place branch update from PR #89507 (@willfrombr) onto the switch-by-default behavior: the deterministic switch path remains the default so non-interactive updates (desktop, gateway, cron) never dead-end on a merge conflict, and deliberate custom-branch users opt in with updates.parked_branch_strategy: update_in_place. --switch-branch overrides the in-place strategy for one run (deep feature branches that must not accumulate update merge commits). Docs + config comments + tests cover all three routes. Co-authored-by: Willian Santos <285090322+willfrombr@users.noreply.github.com> * fix: remove function-level 'import time as _time' that shadowed the module import The in-function import made _time local to all of _cmd_update_impl, so the orphan-backend reap path (which runs earlier in the function) hit UnboundLocalError before the import line executed. The module-level 'import time as _time' at the top of update_cmd.py already covers the divergence-merge safety tag. * feat(bot-mode): message_agent tool — structured, Bot-Chat-only agent-to-agent DMs Bot Mode agents now DM teammates through a real tool instead of hand-assembled shell commands. message_agent(target, message) validates the target against the live roster, applies the sender's attribution prefix server-side, and delivers over the existing proven transports (hermes -p ... --query-file for local teammates, hermes peer dm for peer gateways) as a tracked background process with notify-on-complete — fire-and-forget, the reply wakes the sender on a later turn. Containment: the schema is injected per-turn ONLY into a bot's canonical 'Bot Chat' session on Bot-Mode-managed installs (same gate as the protocol section); it is never registered in the tool registry or any toolset, and dispatch re-gates on the session title so a forged call from any other session refuses. The gate is session-stable, so the tool list stays byte-identical across turns (prompt-cache safe). The protocol section is rewritten to teach the tool and now carries the teammate roster WITH ROLES (Bot Mode title + profile description), so bots know who does what before picking a recipient. Roles and a protocol version salt join the capability fingerprint: existing eternal Bot Chats adopt the v2 protocol + tool with one epoch refresh, and a rename/description edit refreshes the roster on the next message. * feat(desktop): failed turns name the failing layer with recovery actions Turn errors now carry a structured {layer, code, retryable} descriptor (agent/error_surface.py) built from the same classifier the retry loop uses. The tui_gateway stamps it on terminal error frames, retained failed-turn snapshots, and resume replay; the Desktop error card renders the layer title (provider / endpoint / streaming / auth / billing / gateway / runtime / disk) plus matched actions: Retry, Switch provider, Open logs, Copy diagnostics. Older backends that omit the descriptor keep today's behavior (generic title, string-sniff fallbacks) — the field is advisory on both sides. * fix(desktop): error card renders router-free threads without crashing useNavigate() throws outside a <Router>; streaming.test.tsx renders the thread bare. Move the Settings deep-link into a SwitchProviderAction child gated on useInRouterContext(), which is safe in any tree. * feat(desktop): error card offers Nous support link on Portal-auth sessions Sessions running on provider 'nous' get a 'Nous support' action on the failed-turn card, opening the portal help hub (https://portal.nousresearch.com/help — docs, Discord, GitHub) in the external browser. All five locales + docs updated. * Revert "feat(desktop): error card offers Nous support link on Portal-auth sessions" This reverts commit 31872bfcf555cedb2501122a75e29328c0e90e80. * polish(desktop): rename error-card action to 'Copy error details' 'Copy diagnostics' was dev-speak; match the familiar OS-error phrasing. All five locales + docs updated. * fix(desktop): error card honors the classifier's retry verdict + failing-session identity (review feedback) Addresses @helix4u's review on #91493: - conversation_loop now stamps failure_retryable (the real ClassifiedError verdict) next to failure_reason; error_surface prefers it and only falls back to the reason set for older results. Fallback set corrected to match classify_api_error (auth, format_error, billing_unverified now non-retryable). - The descriptor carries the failing session's provider/model captured at classification time; Copy error details prefers them over the foreground composer atoms. - Open logs is labeled 'Open Desktop logs' on remote/cloud connections — the local folder holds transport logs, not the remote runtime's. - API-exception module allowlist widened to botocore/boto3/google/grpc/ requests/aiohttp so other adapter SDKs don't misclassify as gateway. * fix(state): defer FTS rebuild under foreign WAL holders * fix(state): guard gateway FTS rebuild + comment early flag-set Add the foreign-holder guard to gateway/session.py::_rebuild_fts_once(), the third FTS rebuild path that was not covered by the original fix. Also add a comment explaining why _fts_runtime_rebuild_attempted is set before the foreign-holder check: the fail-open path that follows persists FTS_STALE_KEY so the next startup retries via _recover_stale_fts. * fix(state): use /proc readlinks + cmdline fallback for holder detection Address review feedback from @jackulau on PR #90871: 1. psutil.open_files() silently drops '(deleted)' WAL sidecar entries on Linux because isfile_strict() stats the literal path including the suffix and fails. Switch to direct /proc/<pid>/fd readlinks which preserve the '(deleted)' suffix so _canonical can match. 2. psutil.process_iter() converts AccessDenied to None, which or-() skips silently — the fail-closed branch never runs. For the root-gateway vs user-desktop topology in the issue, the fd table is unreadable but /proc/<pid>/cmdline is world-readable. Add a cmdline fallback that flags uninspectable processes. Also keep the psutil path for macOS/BSD (no '(deleted)' convention). * fix(state): only flag uninspectable Hermes processes as holders The cmdline fallback was matching every system daemon with an unreadable fd table (init, systemd-journald, dockerd, etc.), causing FTS rebuilds to be skipped on every Linux system. Add _looks_like_hermes filter so only processes whose cmdline contains Hermes markers are flagged — matching @jackulau's suggestion of 'uninspectable AND identifiable as another Hermes process.' * fix(cli): guard empty message text in _display_resumed_history text.splitlines() returns [] for empty strings. Accessing msg_lines[0] then raises IndexError, making session resume crash when the session contains a message with empty or whitespace-only text (e.g. reasoning-only turns, tool-only assistant messages). Guard with `or [""]` in all three branches (user, assistant_last, regular assistant) so an empty message renders as a blank line. Fixes #59265 Co-authored-by: AlexFucuson9 <AlexFucuson9@users.noreply.github.com> * fix(state): apply macOS write barriers on every state.db repair connection state.db corrupted twice in two days with the torn-b-tree signature — repeated "2nd reference to page", "Rowid out of order", and long runs of "never used" pages in messages (rootpage 5) and idx_messages_session. macOS fsync() guarantees neither data-on-platter nor write ordering, which _enforce_macos_synchronous_full already documents: a rewrite interrupted by process or OS termination leaves half-written b-tree pages. The mitigation is per-connection (synchronous=FULL + checkpoint_fullfsync=1) and was applied only through apply_wal_with_fallback(). The repair path opened state.db with a bare sqlite3.connect() six times and then ran REINDEX, VACUUM and writable_schema surgery through it — the operations that rewrite nearly every page of the file — with no barrier at all. - _connect_repair_durable() routes every repair/probe connection through the barriers. Applying them is best-effort by necessity: SQLite loads the schema before any statement, so on a malformed schema even PRAGMA synchronous=FULL raises DatabaseError, and a malformed database is precisely this helper's input. _reapply_durability_barriers() retakes them before REINDEX and VACUUM, once the schema parses and they can stick. - verify_state_db_integrity() adds the proactive check that was missing. Repair only ever ran reactively, after a caller already hit a malformed error, so a database torn in pages no query happened to touch stayed live and kept accepting writes. On 2026-08-19 that gap was 11 hours across two restarts that both reported a clean start. Size-aware: degrades to an O(1) probe above 2 GiB rather than pegging a CPU at startup. Also restores two fixes lost when `hermes update` reset the tree to origin/main before they were committed: - _db_fingerprint keys the repair ledger on dev+inode+size instead of size+mtime_ns. The old form was justified as "stable for a file nothing can successfully write to"; that premise is false, because on FTS corruption this module deliberately keeps canonical writes enabled with FTS detached. mtime churned on every write, so each pass re-keyed the ledger and reset the counter to 1 — the cap could never be reached and the damaging surgery could retry forever. - _live_writer_holds_db() refuses surgery while another connection holds the database. The cross-process lock only serialises repairers against each other; it says nothing about the gateway, Desktop or a CLI. Rewriting b-tree pages under a concurrent writer is what spread the 2026-08-18/19 damage out of the FTS shadow tables and into the canonical ones. Fails open, so it cannot strand the self-heal path it protects. The guard's own tests built a two-table toy schema, so every repair aborted on "no such table: sessions" before reaching the guards under test — the assertions were passing over a code path that never ran. They now build through a real SessionDB. Targeted state/repair suites: 330 passed, 1 pre-existing unrelated failure. Broader sweep: 50 failed/1221 passed -> 46 failed/1225 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Dhanesh Purohit <dhanesh@users.noreply.github.com> * fix(state): scope salvage to repair-connection durability + live-writer guard Follow-up to the salvaged repair-durability commit. Scope corrections so this PR ships only the reachable, non-competing, WAL-mode-correct half: - Drop verify_state_db_integrity() + its 4 tests. Zero production callers here (dead code); the caller lives in the follow-up that wires it into SessionStore._open_session_db_for_active_scope() (PR #91754). The function moves with its wiring. - Drop the _db_fingerprint change (size:mtime_ns -> dev:ino:size) + its 3 ledger tests. This is competing work: PR #88425 (salvage of @jirathip-k's #88224) already fixes the same size:mtime_ns budget-reset bug with a content-sample + volatile-header-mask that also handles the DELETE-mode commit-counter case, and carries @jirathip-k's diagnosis/credit. Landing a second, divergent fingerprint contract would stomp that lineage. Fingerprint stays with #88425; this PR reverts _db_fingerprint to main's form. - Mark test_repair_refuses_while_another_connection_holds_the_db requires_wal. _live_writer_holds_db detects an out-of-process holder via the WAL-index exclusive lock, absent in journal_mode=DELETE (used on WAL-reset-vulnerable SQLite <3.51.3 incl. CI's 3.50.4, and on NFS/SMB). The test failed there; the conftest requires_wal gate auto-skips it. DELETE-mode limitation is now documented on the guard docstring: repair is serialised only by the cross-process repairer lock there. The reported incident was in WAL mode. - Map dhanesh@users.noreply.github.com -> dhanesh (contributors/emails) so the attribution CI gate passes. Net: this PR is repair-connection durability barriers + the live-writer guard. addresses @andrexibiza's #90747 review (dead-code verifier + fingerprint interlock with #88425). * chore(state): tidy post-salvage residue in state.db durability test Two leftovers from the #91852 descope (integrity-check tests removed but their scaffolding stayed): - Drop the now-unused `import pytest` (orphaned when the verify_state_db_integrity tests that used it were removed; no markers/raises/fixtures remain in this file). - Rename the `# Defect 1:` section label to just `# Repair-path write durability` — the sibling "Defect 2" section was descoped out, leaving the numbering dangling. Test-only, no behavior change. tests/test_state_db_write_durability.py: 4 passed. * fix(credits): suppress depleted banner on stealth-preview models Stealth-preview SKUs (e.g. stealth/ox-alpha) are free-tier but carry no :free suffix, so is_free_tier_model() returned False for them. On gateway sessions (which never run the model picker's pricing fetch), the free-model suppression of the credits.depleted banner never engaged, and any response carrying paid_access:false triggered a false "Credit access paused" notice. Add stealth/ prefix detection to is_free_tier_model() as a zero-network signal, same design as the existing :free suffix check. Fail-open to False (banner still shows) if the prefix changes — recoverable noise, never a masked depletion on a paid model. Closes #91843 * refactor(credits): fold review findings for stealth free-tier fix - credits_tracker: trim inline comment block (duplicated docstring) and correct its safety claim - a paid model under stealth/ would fail closed (suppressed banner), not open; state the trade-off honestly. - run_agent: update stale call-site comment to mention stealth/ prefix. - auxiliary_client: widen sibling free-SKU detector _is_free_model to recognize stealth/ prefix (same bug class as #91843: free_only=true wrongly skipped the OpenRouter fallback and the paid-lane warning fired spuriously for stealth models). - tests: bind the new sibling behavior (stealth/ox-alpha free, my-stealth/model not). * docs(credits): document naming-convention trust in aux free-SKU detector Mirror the credits_tracker caveat in _is_free_model (a paid stealth/ model would bypass the free_only gate and paid-lane warning) and fix the stale _warn_paid_lane_once docstring. * fix(telegram): omit topic routing from rich edits * fix(desktop): surface actionable error when Nous Cloud agent returns 503 (#85335) When a Hermes Desktop connects to a Nous-managed cloud agent (*.agents.nousresearch.com) and that backend returns HTTP 502/503/504, the previous error message was the opaque generic 'Hermes backend did not become ready: 503: ...' with no guidance that the cloud server itself is down. Add isServerSideHttpError and isNousCloudAgentUrl helpers and use them in waitForHermesReady to detect this exact scenario. When triggered, throw an error with the hostname, status code, and recovery paths: check the Nous Portal, switch to Local mode, or reach out on Discord. Also adds a isCloudBackendDown flag and statusCode property on the thrown error so the renderer overlay can render specialized UI if desired. * fix(desktop): surface Nous Cloud 503 at the OAuth ticket-mint boundary The original implementation classified 502/503/504 only inside the readiness loop, but for OAuth-backed Cloud connections the WebSocket-ticket mint runs before waitForHermesReady. A server fault there was wrapped by gatewayTicketFailure into a generic message and the Cloud-down classifier was never reached. This closes that boundary and fixes a latent regex defect. - isServerSideHttpError: structured-first (err.statusCode for 502/503/504), legacy 'NNN:' prefix as fallback, non-Error inputs rejected. Also fixes the committed '\d' (double-escaped, matched a literal backslash) that made the function never detect a status prefix. - makeNousCloudBackendDownError: single factory for the actionable Cloud-down error (isCloudBackendDown/statusCode/detail/cause), shared by both the ticket-mint boundary and readiness exhaustion. - main.ts: run the Cloud classifier at mintGatewayWsTicket before the gatewayTicketFailure wrap; 401/403 still route to reauth. - connection-config.ts: gatewayTicketFailure preserves an integer statusCode from the source error; auth semantics unchanged. - boot-progress/IPC: carry isCloudBackendDown and statusCode through DesktopBootProgress so the renderer overlay (a PR-body promise) can key on the structured result rather than re-classifying the message string. Tests: backend-health (structured detection, non-Error rejection, factory shape/cause/guards, legacy fallback), connection-config (statusCode preserve, 401/403 reauth, integer-only copy), and an OAuth ticket-mint integration regression (Cloud 503 -> actionable Cloud-down; 401 -> reauth). Connection- config suite 80/80 green; backend-health sync tests green; the async readiness loop tests cannot run on this host (pre-existing local-run limitation) and are the CI gate. PR #85373 (#85335). * fix(desktop): render the Nous Cloud-down recovery when a cloud backend fails (#85335) The electron boot path now classifies a Nous Cloud 502/503/504 at both the OAuth ticket-mint and readiness boundaries and carries isCloudBackendDown / statusCode through DesktopBootProgress, but the renderer never consumed the structured signal — a cloud-backend failure fell into the generic remote- failure recovery copy. Make BootFailureOverlay branch on isCloudBackendDown: lead with the cloud-specific title/description, drop the local-only Repair action, and surface the actionable portal / Local-mode / Discord guidance (the electron factory's full message is still shown in the error box). Adds the cloudDown i18n keys (en + ar/ja/zh/zh-hant) and a regression test asserting the cloud-down recovery renders and Repair is dropped. * style(desktop): satisfy perfectionist lint on the 503 electron files eslint --fix output: blank lines before statements and the import-order spacing in connection-config.test.ts that the check:lint gate rejects. Formatting only — no logic change. * polish(desktop): cloud-down overlay gets Portal/Discord action buttons Follow-up on the #85373 salvage: the portal and Discord URLs move out of the localized hint prose into dedicated action buttons (URLs live in code, translations can't drift them), matching the layered error card's action-row idiom from #91493. Overlay test updated to the button contract; all five locales updated. * test(desktop): advance the mock clock in the cloud-503 readiness tests The two waitForHermesReady cloud-503 tests froze now() at 0, so the readiness loop never crossed its deadline — the vitest electron project hung for the full 20-minute CI budget. Advance the clock per poll like the sibling readiness tests do. * feat(bot-mode): @mention middleware identifies, never delivers — the agent owns messaging The composer middleware is now identification-only: it resolves the user's @tags against the live roster and annotates the draft with who they refer to (profile, friendly title, device for cross-connection rows). The agent decides whether to contact them and does it through its message_agent tool — one send path, composed messages only. Deleted the renderer's entire parallel delivery transport: deliverRemoteRosterMentions / pollRemoteDmReply / ensureRemoteCanonicalChat and the injected shellout instructions ('[@mention handoff — run hermes -p …]' and 'Desktop is delivering … over Connections'). This retires the whole invocation bug class at the source instead of sanitizing it: no verbatim user text is ever forwarded by the renderer (#91397), and no shell command is ever composed from prompt text (#91304, #91339 shape). Tests: mention-identification.test.mjs replaces the two delivery-era files — identification note shape, no-shellout/no-delivery containment (sabotage-verified: re-adding a renderer delivery call fails 2 tests), poisoned-title inertness, pass-through for unknown @s, and a source contract pinning the deleted machinery. hide-bots + roster-cache-key harnesses re-pinned to the new contract. 390/390 green. * test(windows): on-demand live venv-holder E2E lane + probe suite (#91277) On-demand workflow (fires only on wine2e/** pushes, never on PRs/main) that runs a live venv-holder E2E on windows-latest: real spawned processes with Hermes argv shapes, real detection/classification/ message code against the live process table. Tests pin CORRECT behavior for the cluster issues (#90778 mislabeling, #78089 long-path exemption, #87594 ancestor-exclusion, #81774 serve premise), so unfixed bugs fail on the runner — empirical premise-check before the consolidation fix. * ci(windows-venv-e2e): drop --timeout (pytest-timeout not in dev-only sync) * fix(update): venv-holder labels parse the real subcommand; gateway ancestors stay visible to the scan #90778: _hermes_holder_subcommand() — token-based parse of the actual Hermes subcommand (profile selectors skipped, flags never matched), so 'hermes dashboard' stops being labeled as the Desktop backend and '--preserve-cache' stops matching 'serve'. Unknown argv gets no hint instead of a wrong one. #87594: ancestor-exclusion in _detect_venv_python_processes and _venv_launcher_ancestors now carves out GATEWAY ancestors (canonical looks_like_gateway_command_line): when /update runs as the gateway's child, the gateway stays visible to the scan so the pause machinery can stop it, while shells/terminals/own-venv ancestry stay excluded. 15 cross-platform classifier tests; live Windows E2E suite is the acceptance gate on this branch. * test(windows): realistic gateway-parent argv in the #87594 live probe (child code via file, one-line -c) * test(windows): diagnostics in the #87594 probe — parent cmdline/exe + matcher verdict * test(windows): #87594 probe asserts on the gateway ANCESTOR, not the direct parent Diagnostic run showed the venv shim makes every spawn a launcher/worker chain: the child's direct parent is its own launcher (python.exe child_scan.py), and the gateway-argv process is the grandparent. The probe now finds the gateway ancestor by argv — the same way the pause machinery would — and asserts THAT pid is visible to the scan. * fix(update): holder classifier derives value-flags from the real parser; de-flake goal-resume fixture Review on #91869 (@andrexibiza): the handwritten value_flags subset misparsed '--reasoning high serve' as subcommand 'high' and '-m dashboard serve' as 'dashboard' — recreating the wrong-hint class. _holder_value_flags() now introspects build_top_level_parser() (every option with nargs != 0, plus the pre-argparse profile selectors), with a static fallback for broken-tree updates, --flag=value handled. Regressions for --reasoning/-m/-t/--model=/-c per review. De-flake test_goal_resume_restart: the fixture only set the HERMES_HOME env var, but get_hermes_home() prefers the context-local override — an override leaked by any earlier test in the xdist worker pointed the goals DB at a dead tmp dir and resume enqueued nothing (the CI-only red). Fixture now pins the override via set/reset_hermes_home_override. Mechanism proven both ways: env-only fixture cannot beat a leaked override; pinned fixture immune. * fix(desktop): strip off-scheme paint from selection copies Chromium's native selection copy serializes the selection as text/html with every element's computed color inlined. Copied from a dark theme, body text lands on the clipboard as near-white (the app ink computes to color(srgb 0.902 0.929 0.953 / 0.94)); pasted into a light-background target such as an email, it is invisible. The renderer never writes rich text itself, so this payload can only come from Chromium's serializer — which runs after copy handlers decline, meaning clipboardData reads back empty inside the event. The new guard therefore decides from the live DOM: it scores the computed ink of the selected text against the rendered theme mode, and only when they are opposite schemes does it own the payload, writing text/plain plus a tag-structured text/html with no paint declarations. Structure (headings, lists, tables, links, bold/italic, code layout) survives; colors come from the paste target's defaults. A generic font-family anchor (sans-serif, monospace inside code) keeps receivers that convert HTML to rich text on their own compose font instead of the Times browser default. Same-scheme copies and selections starting inside editable fields pass through untouched. * fmt(js): `npm run fix` on merge (#92032) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * fmt(js): `npm run fix` on merge (#92034) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * fix(update): don't ZIP-fallback on dependency failures or dirty trees Surgical reapply of PR #87878 (@kshitijk4poor's salvage of #87327 by @liruixinch) onto current main — the receipt-boundary and summary changes from this session made the original commits conflict. - ZIP fallback now keys on git ACTUALLY having failed (_should_zip_fallback_on_update_error): a dependency-install failure after a successful pull can't be fixed by re-downloading source and would clobber the tree (#87331 cascade trigger, #87304). - _abort_zip_update_if_dirty_tree: refuse to overlay a dirty checkout (-uall so user gitconfig can't blind the guard) + pre-swap TOCTOU re-check with our own staging artifacts filtered (#91962, #87304). - Failure-stage naming (_format_update_failure_stage) + stderr tail so 'Git update failed' stops mislabeling pip/uv failures. - Receipt finalize preserved on the no-fallback failure path. Co-authored-by: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Co-authored-by: liruixinch <liruixinch@outlook.com> * fix(update): ZIP swap preserves the built desktop app (apps/desktop/release) The #70337/#87331 win-unpacked wipe half, from PR #70477 by @JonthanaHanh (reimplemented against the two-phase staged swap that postdates that branch — the live release/ dir is grafted into the staged apps copy BEFORE the atomic commit, so preservation rides the same rollback machinery instead of a post-hoc copy). Co-authored-by: JonthanaHanh <92574114+JonthanaHanh@users.noreply.github.com> * test(update): re-pin ZIP-fallback desktop test to the preserve-through-swap contract The old contract WAS the bug (#70337): exe deleted by the swap, then rebuilt from scratch. With the release-dir graft the exe survives the swap; the test now asserts survival + original bytes. * feat(desktop): Send Diagnostics — one-click redacted debug-bundle upload from the error card New diagnostics.share_nous RPC reuses the CLI --nous pipeline (collect_share_bundle → build_nous_bundle → share_to_nous) with redaction forced on; accepts redacted error context + client-side extra files (local desktop.log on remote connections) with sanitized labels and size caps. Desktop: Send Diagnostics action on the failed-turn error card → consent modal (privacy notice, explicit Upload) → private view link + GitHub Issues / Nous Portal Support / Discord handoff. CLI --nous success output gets the same three-destination pointer. i18n en/ja/zh/zh-hant/ar; docs updated. * fix(desktop): Send Diagnostics review fixes — consent accuracy, log-grade redaction, dismissal guard, linkless-success (review feedback) Addresses @helix4u's review on #92020: - Consent notice now matches the real --nous contract: full logs up to 512KB each, likely conversation content/tool outputs/file paths, viewable by Nous staff AND allowlisted Discord moderators (all 5 locales). - Client-supplied text (error_context + extra_files) rides _redact_log_text — the same upload-safe redactor as backend logs (secrets + email masking), not the weaker bare secret pass; regression test covers both. - ok:true without view_url or id becomes a structured failure; a returned id without a link renders an upload-ID fallback the user can quote. - Generation guard in the store: dismissal is immediate in every phase (incl. mid-upload); a stale completion can no longer resurrect or overwrite the dialog. Cancel button never disabled. * style: post-rebase lint fixes * fmt(js): `npm run fix` on merge (#92089) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * ci: run the work lanes on larger runners and merge the split jobs Every Linux lane that does real work ran on a 4-core `ubuntu-latest`. The Python suite and the JS checks were split into many small jobs to make that size usable. Each split job repeated the full setup. In most of the JS jobs the repeated setup cost more than the work. The work lanes move to larger runners. Then the splits that existed only to make small runners usable go away. Python tests: 12 slices become 1 job on a 96-core runner. Slicing cost a matrix job, a duration cache, a per-slice artifact and a merge job. 96 cores clear the floor that the slowest single test file sets, which is about 82s. A second slice divides work that is already at that floor, and adds a second setup. Duration data from run 32522943054 gives the numbers behind this: 3178 files, 11645s in series. The worker count is explicit, because `run_tests.sh` defaults to twice the core count. A later commit sets it from a measurement on this hardware. JS checks: 14 jobs become 1. The matrix paid about 371s of repeated setup to spread about 612s of work. One larger runner installs one time. The three UI shard scripts and `run-ui-shard.mjs` are therefore removed, because the unsharded `test:ui` covers the same tests. The unit of parallel work inside that job is a CHECK, and not a workspace. apps/desktop is most of the payload, and its own `check` is a serial && chain. A spread across workspaces alone therefore leaves that chain as the long pole. A package that declares `check:*` sub-scripts gives one unit for each sub-script. That is the same selection rule the matrix used. The loop lives in `.github/scripts/run-workspace-checks.mjs`, so the same sequence runs on a laptop. It runs 11 units together, buffers the output of each one, and fails at the end with the full list. Children that share one stdout interleave their lines and make a failure hard to read. `npm run --ws check` stops at the first workspace that fails. `check:test:plugins` joins the desktop `check` script. The matrix prefers `check:*` sub-scripts over the plain `check` script, so `check:test:plugins` ran only as its own leg. Without this change the merge drops that suite and the job stays green. node_modules is cached on the lockfile, and `npm ci` is skipped on an exact hit. The `cache: npm` option of `setup-node` caches only the ~/.npm tarball cache, which leaves the extract and the postinstalls to pay again. The arm64 image build stays on a native arm64 runner. A build of linux/arm64 on an x64 host uses emulation. The docker test lane caps its workers at the core count. Each of those tests drives a container, so the docker daemon sets the limit and not the processor. `.github/actionlint.yaml` declares the runner labels. actionlint knows the GitHub-hosted labels only, and an undeclared label reads as an error that hides the real findings. The `detect` job checks out one file through a sparse checkout, and its timeout drops to 1 minute. It reads `scripts/ci/classify_changes.py` and nothing else. Verification: - actionlint reports 9 findings across all workflows. An unmodified HEAD with the same config reports the same 9. This change adds none. - A wrong label still fails. actionlint reports `ubuntu-latest-32-cor` and `ubuntu-latest-32-arm-cores`. - Every changed workflow parses, and `name` parses as a string. - A replay of the `save-durations` merge step against a three-artifact layout returns all 3178 entries. - An expansion of the npm script graph gives the same leaf commands for the parallel units and for a plain `npm run check`, in both directions. Against the 13-leg matrix the count is 13 to 11, and the whole difference is the three UI shards that collapse into one unsharded `check:test:ui`. - `--list` reports the 11 units, and a full local run completes and reports the time of each unit. - The runner labels cannot be verified here. The first real run is the test. * fix(tests): remove four shared-state and lifetime faults at high concurrency The suite now runs as one job with high per-file concurrency. Four tests depend on state that they share with their siblings, or on a timer that outlives them. That was safe at 8 workers. It is not safe at 96 or more. Runs 32547184159 and 32551746525 show them. 1. Every pytest subprocess shared one temp root. pytest puts tmp_path under <temproot>/pytest-of-<user>/. At the end of a session it walks that directory with cleanup_dead_symlinks(). The walk lists the directory. Then it asks whether the `pytest-current` symlink resolves. Then it unlinks the symlink. A second process replaces that symlink between the question and the unlink. The first process then raises FileNotFoundError after all of its tests passed. Two files failed this way and passed on retry. scripts/run_tests_parallel.py now gives each subprocess its own temp root through PYTEST_DEBUG_TEMPROOT, and deletes it after the attempt. No two processes share a directory. The race has no shared object to act on. Proof: a direct driver of _pytest.pathlib.cleanup_dead_symlinks against one root, with a second thread that replaces the symlink, raises the same FileNotFoundError on 'pytest-current' as CI. A private root for each subprocess removes that condition. A separate check confirms that 5 subprocesses receive 5 distinct roots, that tmp_path lands inside the private root, and that no root survives the attempt. 2. The config read guard walked directories that other tests were writing. tests/hermes_cli/test_config_read_guard.py scanned the tree with rglob. rglob descends into every directory and filters after that, so it calls scandir() on __pycache__ trees that the guard never inspects. Sibling processes create and delete those entries during the run. A directory that disappears in the middle of a walk raises FileNotFoundError out of rglob. The scan now uses os.walk. It prunes excluded directories before it descends, and it ignores a directory that disappears. __pycache__ joins the excluded set, because bytecode is not source. The guard still catches what it exists to catch. With a planted raw yaml.safe_load of config.yaml in hermes_cli/, the test fails and names the planted file. With a clean tree it passes. 3. A PTY test waited for a file to exist, and not for its content. tests/tools/test_process_registry_write_stdin_surrogates.py spawns a child that runs open(out,'wb').write(sys.stdin.buffer.readline()). open() creates the file empty. The bytes arrive only after the PTY delivers the line. The wait stopped at out.exists(), which the empty file already satisfies, so the read returned b'' when the parent won that gap. This test failed both attempts in CI, and did not pass on retry. The test now waits for the expected bytes, with a bounded deadline. Proof: the old wait loses 6 times in 25 runs on an idle 16-core machine. The new wait loses 0 times in 25. 4. A dialog close timer outlived the test that started it. ConfirmDialog holds the "done" beat for 600ms after a successful confirm, then calls onClose. The timer had no cleanup, so an unmount inside that window left it armed. It then called onClose on a tree that is gone, which reaches setState in the parent. vitest can tear the environment down first, and React then reads `window` during the update: ReferenceError: window is not defined at resolveUpdatePriority (react-dom-client.development.js:1308) at dispatchSetState at Timeout.t4 [as _onTimeout] session-actions-menu.tsx:574 The frame at session-actions-menu.tsx:574 is the `onClose` prop of DeleteSessionDialog. The owner of the timer is ConfirmDialog, which now keeps the handle in a ref and clears it on unmount. Zoomable had the same fault, with a 1500ms timer that clears a "copied" flag. copy-button.tsx and tooltip.tsx already clear their timers. Proof: a new test confirms, unmounts inside the 600ms window, then advances the clock. Against the old code it fails with "expected onClose to not be called at all, but actually been called 1 times". Against the new code it passes. Verification: - The affected Python files and the tests of the runner itself pass under scripts/run_tests.sh. - The desktop ui suite passes: 566 files, 5382 tests, and no "window is not defined". - eslint reports 0 errors on apps/desktop. The 118 warnings are the state before this change. The two cleanup effects carry an eslint-disable line for the ref-mirror rule. They write a timer handle, and not a mirror of a reactive value. The rule permits this, and its own comment names the case. - The PTY test cannot run on the NixOS development machine. That machine has no python3 outside the nix store, and the test uses the literal `python3`. The child exits 127 there. The fix rests on the 25-run measurement above and on CI. * perf(ci): set python test workers to one for each core, from measurement `run_tests.sh` defaults to twice the core count, and the value this branch started with came from a rule of thumb of 1.5x cores plus a measurement on a 16-core machine. A sweep on the real runner disagrees with both. Run 32549672063 on the 96-core runner (EPYC 7763, 377GB) timed the whole suite at six worker counts, two repetitions for each. A warmup run came first, and retries were off: workers x cores rep 1 rep 2 mean 48 0.5x 138s 139s 138s 96 1.0x 127s 126s 126s <- fastest 144 1.5x 130s 134s 132s 192 2.0x 132s 133s 132s 240 2.5x 140s 139s 140s 288 3.0x 143s 142s 142s One worker for each core wins. Both repetitions agree on the order. The shape is the more useful result. The range is 126s to 142s across a 6x range of worker counts. The suite has sufficient concurrency at this machine size, so nothing above the core count buys anything. The remaining time belongs to the slowest individual files and to the setup. A future gain must come from those, and not from this number. The sweep ran from a temporary workflow that this branch does not keep. * fmt(js): `npm run fix` on merge (#92094) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * fix(windows): restore dedicated CLI launchers on update * fix(windows): preserve launcher layout invariants * fix(bot-mode): a bot row opens the bot's canonical Bot Chat (#92042) Partially reverts the newer-visible-session preference from #91791 (salvage of #91258), which made the pinned canonical Bot Chat unreachable. Fixes #92040. Canonical Bot Chats are ALWAYS hidden from the Sessions sidebar: session.create passes hidden:true unconditionally and hideOwnedBotSessions() sweeps any that were born visible (asserted in tests/hide-bot-chats.test.mjs). The bot row is therefore the ONLY entry point to a bot's forever-chat, so preferring the profile's freshest visible session did not re-order two equivalent doors — it removed the only one. Reported symptom: a 106-message bot-building conversation with no reachable entry point anywhere in the UI, while the row previewed one session and opened another (a regression of the preview/click identity #88200 established). The report behind #91791 was real but has a non-destructive answer: scratch sessions started via "New chat with this agent" are not plumbing-titled, so neither hideOwnedBotSessions() nor sweepBotProfileSessions() hides them (the sweep matches the exact titles 'Bot Chat' / 'Agent Inbox' / 'Group: …'). They stay listed in the Sessions sidebar and are reachable there; they simply are not what the bot row targets, which is by design. Changes: - openBotCanonicalChat: when the pin is alive and verified, open it directly. The newerVisibleBotChat preference is removed from that branch only; the helper stays for the dead-pin recovery path. - Drop the now-unused latestVisible parameter and its argument at the BotRow call site. The second call site already passed three args. - tests/bot-row-opens-latest.test.mjs -> tests/bot-row-opens-canonical-chat.test.mjs: the two tests that asserted the newer-session behaviour are rewritten rather than deleted, so the reasoning survives in the suite. Adds a source-level guard ("the healthy-pin branch never prefers a newer visible session") so this cannot silently regress. The deleted-newer-session fallback test covered a path that no longer exists; replaced with one asserting a failed open of a verified pin propagates instead of forking the forever-chat. The keepAllProfilesScope: false half of #91791 is untouched. Plugin suite: 392 pass, 0 fail. * docs(agents-md): update pipeline architecture, process-identity pitfall, gateway lifecycle contract, wine2e lane Captures the durable invariants from the fleet-update campaign (#91277) so contributors and the sweeper review against them: - Update Pipeline section: the transactional shape now on main (plan → snapshot → apply → restart-per-kind → verify → report), the per-stage invariants (no partial snapshot tiers, ZIP only on real git failure + dirty-tree refusal + release-dir graft, fleet-wide drain-first restarts, code-sha verify, exactly-once receipts), deployment kinds as first-class, and the #92091 socket direction. - Gateway lifecycle vs Desktop app: serve dies with the app by design, the detached gateway survives it; the Windows shim-unlock tree-kill is the known breach (#85265) and its replacement is pause-for-update — with the two anti-fix warnings. - Known Pitfall: process identity is never inferred from argv substrings (canonical matchers, parser-derived flag sets, ancestor carve-out, full-cmdline rule, socket-first for new heuristics). - Testing: the on-demand wine2e live Windows lane and its reproduce-first workflow. * fix(bot-mode): the canonical Bot Chat is found by NAME — session-id pins removed A bot's forever-chat now has exactly one identity: the session titled "Bot Chat" on that bot's profile. Core UNIQUE(title) makes (profile, 'Bot Chat') an exact registry, and every open consults it directly via session.list {title, include_hidden}. The stored-id pin (ui_meta['hermes-bots'].chat) and its entire verification apparatus — preferred_session_ids resolution, drifted-pin keep branches, last_session grandfathering, dead-pin recovery re-anchoring, newerVisibleBotChat — are removed, not deprecated. Legacy ui_meta.chat keys are ignored and dropped from merges on sight. Every lost-canonical-chat incident (#88146, #88200, #90524, #90705, and five hardening waves) traced to that pointer dangling or being stolen, then later guards welding the wrong session in. A name cannot dangle: corrupt pins self-heal on first click because the pointer is simply never read. Gateway: profiles.list now reports canonical_session per profile row (registry row resolved server-side by title — hidden rows resolve, deny-listed sources and archived rows do not, compression lineages resolve to the live tip), replacing the preferred_session_ids request contract. The roster preview, activity signals, and the /new→/compact guard all read canonical_session, so preview identity and click identity are the same row by construction. No migration shims: this IS the system. * docs: record the Bot Mode canonical-chat invariant in AGENTS.md * docs(agents-md): Bot Mode canonical-chat invariant is name-identity — corrections folded in The cherry-picked #92121 text documented the pin-first contract (#92042 era). Corrected to the registry contract this branch ships: identity is (profile, 'Bot Chat') via exact-title lookup; there is no session-id pin at any tier; reviewer corollaries and regression-test references updated to the surviving suites. * fix(state): stop unbounded state.db repair loop from filling the disk A malformed-schema state.db sent Hermes into a repair loop that wrote a fresh full-size forensic backup every ~10s: 31 copies / 2.3GB in 20 minutes, free space heading to zero on a host running an agent fleet. The #86747 guards for exactly this were already present and did not hold. Both keyed on `size:mtime_ns`: * `_db_fingerprint` -> the ledger's attempt counter reset to 1 on every pass, so `_MAX_PERSISTENT_REPAIR_ATTEMPTS` was never reached and the loop never terminated; * `_backup_db_file`'s dedupe compared mtime, so it never matched and each pass wrote another full-size copy. The assumption behind that key -- "nothing can successfully write to a damaged file" -- holds for the b-tree damage of #86747 but not for the malformed-SCHEMA class: the DB still opens and accepts writes (only sqlite_master is unreadable), so live writers, WAL checkpoints and the in-place repair strategies themselves all move mtime between passes. Fixes: * fingerprint on size + a bounded head/tail content sample instead of mtime. Stable across passes that merely touch the file, still changes on genuine repair/truncation/restore (so recovery resets the budget), and stays O(1) on a multi-GB DB. * dedupe the forensic backup on that same fingerprint. * add the missing free-space guard: refuse the pre-repair copy when it would leave under 2GiB free, with an actionable error. The backup is a full raw copy of the damaged DB, so a repair loop is a disk amplifier that can take down every process on the host -- and the refusal path already hard-stops the repair (#69603) rather than mutating the only remaining copy. Tests fail on the unfixed tree and pass here; the pre-existing failures in test_state_db_malformed_repair.py and TestFTS5Search are unrelated and reproduce on the base commit. * fix(state): make backup atomic and the disk guard proportional Follow-up to adversarial review of the first commit. Three findings, two confirmed by test and fixed here, one disproven and left alone. CONFIRMED — the free-space guard was a threshold, not cleanup. Prune runs only on the success path, so any copy that failed partway (ENOSPC, sidecar copy failure, kill mid-copy) left a file matching the `malformed-backup-` prefix that nothing ever removed. Measured on the unpatched tree: backups capped at 3 while copies succeed, but 13+ and climbing once copy2 raises — self-reinforcing, since each partial consumes the space that guarantees the next failure. Worse, partials sort newest-by-name, so a later successful prune KEPT the garbage and deleted the intact forensic copies. Fix: copy to a `.incomplete` staging name that does not match the backup prefix, os.replace into place only after every copy succeeds, unlink staging on failure, and sweep stale staging debris on entry. CONFIRMED — the 2GiB floor was a small-volume regression. A 50MB DB on a 10GB volume with 1.5GB free (30x headroom) was refused, and since a refused backup is a HARD STOP (#69603) that silently converts "repair loops" into "repair never runs". Fix: require the copy itself (now including its -wal/-shm sidecars, which the old check ignored) plus proportional headroom — max(256MiB, 2% of volume). DISPROVEN — the review claimed a refused backup skips _record_repair_outcome so the loop never terminates. It does not: repair_state_db_schema records the outcome on the result returned by _repair_state_db_schema_locked, which is where the hard stop returns. Verified on a simulated low-disk host: terminal at pass 4 with zero backups written. No change made. Tests: 5 new (small-volume allow, proportional headroom, sidecar accounting, failed-copy leaves no countable debris + staging swept). 23 pass with the #86747 suite; test_hermes_state.py 252 passed. Pre-existing unrelated failures unchanged. * fix(state): keep the repair fingerprint from cancelling POSIX advisory locks The content fingerprint takes a raw descriptor, and close() on ANY descriptor cancels every POSIX advisory lock the process holds on that file. The exhaustion probe runs before _backup_db_file's has_live_connection guard, so the read happened even when a peer SessionDB held a write lock. Verified end-to-end (journal_mode=DELETE, gateway mid-turn write, peer in a subprocess): before peer BLOCKED -> repair -> peer BLOCKED, holder COMMIT ok unfixed peer BLOCKED -> repair -> peer STOLE the lock, holder COMMIT: disk I/O error WAL is immune (it coordinates through -shm), but DELETE is what Hermes falls back to on NFS/SMB/FUSE/ZFS and on SQLite builds vulnerable to the WAL-reset bug, so this is a real deployment shape. Run the read under offline_file_access and fall back to size:mtime_ns when a connection is live. That keeps the ledger counting instead of returning None (which reads as "not exhausted" and would restore the unbounded loop), and the content key stays load-bearing on the offline repair path -- the only path where surgery actually runs. Also fail the free-space guard CLOSED: a nearly-full volume is exactly where statvfs is likeliest to fail, and proceeding is the multi-GB copy that finishes off the disk. * fix(state): stop backup staging from posing as a forensic copy The staging name was derived from the backup name (`<db>.malformed-bac…

Run every mutating schema/FTS repair strategy on a complete SQLite snapshot and promote a proven result through SQLite's transactional backup API. Failed strategies and interrupted promotions leave the original database intact, committed WAL frames are included, and existing readers keep a coherent inode-backed snapshot. Compose with NousResearch#88425 without changing its repair-epoch fingerprint, forensic content identity, atomic backup publication, retention, or durability barriers. Clean stale repair snapshots before any health/space decision and reserve disk for the snapshot, worst-case VACUUM, promotion, and headroom. Regression coverage includes destructive schema-btree failure, WAL completeness, live-reader promotion, interrupted rollback, promotion failure, cleanup ordering, and disk budgeting.

@jirathip-k

…er guard Follow-up to the salvaged repair-durability commit. Scope corrections so this PR ships only the reachable, non-competing, WAL-mode-correct half: - Drop verify_state_db_integrity() + its 4 tests. Zero production callers here (dead code); the caller lives in the follow-up that wires it into SessionStore._open_session_db_for_active_scope() (PR NousResearch#91754). The function moves with its wiring. - Drop the _db_fingerprint change (size:mtime_ns -> dev:ino:size) + its 3 ledger tests. This is competing work: PR NousResearch#88425 (salvage of @jirathip-k's NousResearch#88224) already fixes the same size:mtime_ns budget-reset bug with a content-sample + volatile-header-mask that also handles the DELETE-mode commit-counter case, and carries @jirathip-k's diagnosis/credit. Landing a second, divergent fingerprint contract would stomp that lineage. Fingerprint stays with NousResearch#88425; this PR reverts _db_fingerprint to main's form. - Mark test_repair_refuses_while_another_connection_holds_the_db requires_wal. _live_writer_holds_db detects an out-of-process holder via the WAL-index exclusive lock, absent in journal_mode=DELETE (used on WAL-reset-vulnerable SQLite <3.51.3 incl. CI's 3.50.4, and on NFS/SMB). The test failed there; the conftest requires_wal gate auto-skips it. DELETE-mode limitation is now documented on the guard docstring: repair is serialised only by the cross-process repairer lock there. The reported incident was in WAL mode. - Map dhanesh@users.noreply.github.com -> dhanesh (contributors/emails) so the attribution CI gate passes. Net: this PR is repair-connection durability barriers + the live-writer guard. addresses @andrexibiza's NousResearch#90747 review (dead-code verifier + fingerprint interlock with NousResearch#88425).

@andrexibiza

…int; publish backup bundle atomically Addresses two data-integrity gaps @andrexibiza flagged reviewing NousResearch#88425. 1. Forensic dedupe no longer reuses the repair-epoch fingerprint. _db_fingerprint masks SQLite's commit counters and samples only head/tail so an ordinary write does not re-key the repair budget — the right predicate for 'same damage epoch', the WRONG one for 'same recovery image'. A live writer committing rows into an interior page (size preserved, head/tail untouched) collided under it, so _backup_db_file handed back a STALE backup that predates real user data. New _backup_content_identity() digests the whole file + every sidecar; the dedupe uses it. The O(n) read is cheaper than the O(n) copy it avoids on a hit. 2. Backup bundle is now published atomically. The promotion loop replaced files one at a time (main first) and cleanup unlinked only staging srcs, so a sidecar os.replace failure after the main promotion left the final-prefix main backup on disk — a countable-but-incomplete bundle that passed the NousResearch#69603 hard stop and deduped as legitimate next pass. Now sidecars publish first and the main DB last (its name is the commit marker _existing_malformed_backups counts), and cleanup rolls back every already-published destination. Two regressions added (both mutation-checked — each fails on pre-fix code): - test_backup_not_deduped_after_interior_page_write - test_publication_failure_leaves_no_countable_partial_bundle tests/test_state_db_repair_loop_mtime.py: 28 passed.

@jirathip-k

…er guard Follow-up to the salvaged repair-durability commit. Scope corrections so this PR ships only the reachable, non-competing, WAL-mode-correct half: - Drop verify_state_db_integrity() + its 4 tests. Zero production callers here (dead code); the caller lives in the follow-up that wires it into SessionStore._open_session_db_for_active_scope() (PR NousResearch#91754). The function moves with its wiring. - Drop the _db_fingerprint change (size:mtime_ns -> dev:ino:size) + its 3 ledger tests. This is competing work: PR NousResearch#88425 (salvage of @jirathip-k's NousResearch#88224) already fixes the same size:mtime_ns budget-reset bug with a content-sample + volatile-header-mask that also handles the DELETE-mode commit-counter case, and carries @jirathip-k's diagnosis/credit. Landing a second, divergent fingerprint contract would stomp that lineage. Fingerprint stays with NousResearch#88425; this PR reverts _db_fingerprint to main's form. - Mark test_repair_refuses_while_another_connection_holds_the_db requires_wal. _live_writer_holds_db detects an out-of-process holder via the WAL-index exclusive lock, absent in journal_mode=DELETE (used on WAL-reset-vulnerable SQLite <3.51.3 incl. CI's 3.50.4, and on NFS/SMB). The test failed there; the conftest requires_wal gate auto-skips it. DELETE-mode limitation is now documented on the guard docstring: repair is serialised only by the cross-process repairer lock there. The reported incident was in WAL mode. - Map dhanesh@users.noreply.github.com -> dhanesh (contributors/emails) so the attribution CI gate passes. Net: this PR is repair-connection durability barriers + the live-writer guard. addresses @andrexibiza's NousResearch#90747 review (dead-code verifier + fingerprint interlock with NousResearch#88425).

@andrexibiza

…int; publish backup bundle atomically Addresses two data-integrity gaps @andrexibiza flagged reviewing NousResearch#88425. 1. Forensic dedupe no longer reuses the repair-epoch fingerprint. _db_fingerprint masks SQLite's commit counters and samples only head/tail so an ordinary write does not re-key the repair budget — the right predicate for 'same damage epoch', the WRONG one for 'same recovery image'. A live writer committing rows into an interior page (size preserved, head/tail untouched) collided under it, so _backup_db_file handed back a STALE backup that predates real user data. New _backup_content_identity() digests the whole file + every sidecar; the dedupe uses it. The O(n) read is cheaper than the O(n) copy it avoids on a hit. 2. Backup bundle is now published atomically. The promotion loop replaced files one at a time (main first) and cleanup unlinked only staging srcs, so a sidecar os.replace failure after the main promotion left the final-prefix main backup on disk — a countable-but-incomplete bundle that passed the NousResearch#69603 hard stop and deduped as legitimate next pass. Now sidecars publish first and the main DB last (its name is the commit marker _existing_malformed_backups counts), and cleanup rolls back every already-published destination. Two regressions added (both mutation-checked — each fails on pre-fix code): - test_backup_not_deduped_after_interior_page_write - test_publication_failure_leaves_no_countable_partial_bundle tests/test_state_db_repair_loop_mtime.py: 28 passed.

kshitijk4poor enabled auto-merge (rebase) August 17, 2026 13:19

kshitijk4poor mentioned this pull request Aug 17, 2026

fix(state): stop unbounded state.db repair loop from filling the disk #88224

Closed

kshitijk4poor disabled auto-merge August 17, 2026 13:24

kshitijk4poor enabled auto-merge (rebase) August 17, 2026 13:52

This was referenced Aug 18, 2026

fix(state): refuse in-file repair when state.db was replaced #89360

Closed

[Bug]: No file-identity guard on state.db — gateway limps through a total persistence outage when its DB is replaced out-of-band instead of failing loudly #89332

Closed

andrexibiza mentioned this pull request Aug 19, 2026

fix(gateway): startup-liveness watchdog for pre-event-loop deadlocks (OOF-298) #89750

Closed

andrexibiza reviewed Aug 20, 2026

View reviewed changes

andrexibiza mentioned this pull request Aug 20, 2026

fix(state): apply macOS write barriers on every state.db repair connection #90747

Closed

andrexibiza mentioned this pull request Aug 21, 2026

fix(state): fail closed on damaged state.db before gateway writes #91754

Open

kshitijk4poor mentioned this pull request Aug 21, 2026

fix(state): apply macOS write barriers on every state.db repair connection #91852

Merged

jirathip-k and others added 4 commits August 22, 2026 14:18

kshitijk4poor added 4 commits August 22, 2026 14:18

kshitijk4poor force-pushed the salvage/88224-state-db-repair-loop branch from a9f94ce to dbf41d1 Compare August 22, 2026 08:59

kshitijk4poor merged commit 1fe8683 into NousResearch:main Aug 22, 2026
35 checks passed

cervantesh mentioned this pull request Aug 22, 2026

fix(state): a failed schema repair must not destroy the database #87409

Closed

2 tasks

Haderach-Ram mentioned this pull request Aug 23, 2026

Ecosystem Digest — 2026-08-23 Haderach-Ram/openclaw-radar#119

Open

This was referenced Aug 23, 2026

[Bug]: state.db automatic repair can destroy canonical data before proving recovery #93064

Closed

[Bug]: gateway permanently caches unavailable SessionDB after init failure #93088

Closed

andrexibiza mentioned this pull request Aug 23, 2026

fix(state): make automatic repair non-destructive #93268

Closed

15 tasks

Navigation Menu

fix(state): stop the unbounded state.db repair loop (salvage of #88224) - #88425

fix(state): stop the unbounded state.db repair loop (salvage of #88224)#88425
kshitijk4poor merged 8 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/88224-state-db-repair-loop

kshitijk4poor commented Aug 17, 2026 •

edited

Loading

Uh oh!

andrexibiza left a comment

Uh oh!

Uh oh!

kshitijk4poor commented Aug 22, 2026

Uh oh!

cervantesh commented Aug 22, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

kshitijk4poor commented Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Data-integrity fixes (this revision — addressing @andrexibiza's review)

Validation

Provenance / interlocks

Uh oh!

andrexibiza left a comment

Choose a reason for hiding this comment

1. One fingerprint is being asked to mean two incompatible things

2. The staged backup bundle is still not atomically published

Interlocks / merge order

CI / restack truth

Uh oh!

Uh oh!

kshitijk4poor commented Aug 22, 2026

Uh oh!

cervantesh commented Aug 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kshitijk4poor commented Aug 17, 2026 •

edited

Loading

cervantesh commented Aug 22, 2026 •

edited

Loading