Gate review: `test_repair_path_has_no_bare_connects` pinned "the helper owns
exactly one sqlite3.connect(str(db_path))"; the helper now opens through
`connect_tracked`, so the guard is "no bare connect anywhere in the module".
Also drops the `connect_fn=sqlite3.connect` kwarg: `connect_tracked` late-binds
the same module attribute, so the kwarg (and its comment) said nothing true.
`hermes_cli` ships with every install that has `hermes_state_repair`, so the
"scaffold/embed installs without hermes_cli" fallback could never run and only
hid a broken import behind an untracked connection. Import directly; docstring
keeps the why (locks cancelled by a probe close, howtocorrupt 2.2) and drops
the restated mechanism.
_connect_repair_durable() -- the single entry point for every repair/probe
connection to state.db -- opened the database with a bare sqlite3.connect(),
outside the live-connection registry in hermes_cli/sqlite_safe_read.py. While
a repair connection was open, has_live_connection() reported false, so any
byte-level probe in the process (zeroed-file detector, header verification,
kanban's post-commit page check) was free to open()/close() the file --
cancelling every POSIX advisory lock the process holds on it (howtocorrupt
2.2) and letting an external writer commit into a database the repair still
believed it owned. These paths hold the strongest locks in the process:
_open_exclusive() keeps locking_mode=EXCLUSIVE across the whole snapshot ->
strategies -> promotion window.
Open through connect_tracked() instead: the fd stays registered for its whole
lifetime and is released on close(). The sqlite3.connect(str(db_path), ...)
call stays in this module so tests patching it keep control, and installs
without hermes_cli keep the durable (untracked) connection as before.
Verified on real files, no mocks: with a repair connection holding
BEGIN EXCLUSIVE, an external writer is BLOCKED, a byte probe now returns None
(refused instead of opening the fd), the same writer stays BLOCKED after it,
and the registry is empty again once the repair closes. A real #63386-damaged
database (stale B-tree index) still reports exit 1 on --check-only and repairs
via reindex_btree to integrity_check 'ok'.
Refs #63386
(cherry picked from commit a031f1be26b777bc09e744b3fa6081ce2af1f851)
hermes_state_common pulls in agent.* at import, so the URI builder moves to
hermes_state_holders (errno/os/sqlite3/pathlib only) where the gateway
readiness probe and backup can adopt it in a follow-up sweep. The doctor
structural-damage branch is one helper instead of two copies, the holder
scan goes through hermes_state_repair._live_writer_holds_db, the migration
hint uses _schema_not_built (the startswith("no such ") check also matched
"no such module: fts5"), and the hermes_state import is hoisted so an import
failure cannot mask itself as UnboundLocalError.
read_only_db_uri() replaces four inline mode=ro URI sites (two of which
still used the raw f-string that truncates on ?/# in the home path:
state_db_has_structural_damage and collect_state_db_stats). The doctor
write probe now applies the live-holder gate in both modes: a quiet store
is probed in place as on main, a held store is probed through a read-only
snapshot, and a held store over 1 GB is skipped with an info line unless
--fix is given (the unconditional copy cost one full DB write per plain
doctor run). Connect/backup failures propagate to the existing
classification instead of being reported as FTS write-health failures.
Observational sessions commands print a migration hint instead of a raw
traceback when a read-only opener meets an older schema.
Co-authored-by: Ahmett101 <Ahmett101@users.noreply.github.com>
`hermes doctor --fix`'s WAL checkpoint and `repair_state_db_schema`'s
preflight documented themselves as fail-OPEN: `live_writer_holds_db` only
refused on unknown/deleted/uninspectable holders and then trusted a
`BEGIN IMMEDIATE` probe, which is blind to a `journal_mode=DELETE` reader
(SHARED only) and cannot run on a malformed file — exactly the states repair
and checkpoint get invoked in. A repair in a second process then REINDEXed /
VACUUMed a file the gateway still held (#103339 item 2).
- `hermes_state_holders.live_writer_holds_db`: any foreign holder of the DB or
a sidecar is a live holder; the probe is only an additional positive signal.
- doctor `--fix`: the checkpoint runs on `_exclusive_repair_db_guard`'s
connection instead of a bare writable `sqlite3.connect`, so an opener
arriving after the scan is refused, not joined; `_session_count` is a
`mode=ro` reader.
- Normal SessionDB writers are untouched: gateway + dashboard in two processes
both keep writing (a process-wide flock on the write path — PR #109270's
shape — would break that).
Tests: the two-process repair race test releases the test process's own
header-probe fd (it is a genuine holder now); the mid-repair writer fixture
opens its connection after staging starts (a pre-existing holder is refused up
front, which is the point).
Refs #103339#100896
`_db_opens_cleanly` drove one probe row through the messages_fts* triggers
and rolled back. FTS5 only buffers that row in an in-memory segment until
commit, so the probe never wrote to `<fts>_idx`/`_data` and could not hit a
stale `messages_fts_trigram_idx` row waiting at the next segid — the class
where PRAGMA integrity_check, the FTS5 integrity-check command and MATCH all
report clean while every committed append fails with
`IntegrityError: constraint failed`. The probe also caught only
OperationalError; IntegrityError is a DatabaseError sibling, so even a
colliding probe would have escaped and been reported as healthy.
Now the probe issues `INSERT INTO <fts>(<fts>) VALUES('flush')` for every
FTS family inside the rolled-back transaction (capability / not-built errors
stay benign), catches sqlite3.DatabaseError, and always rolls back in a
finally. `hermes doctor` and `hermes sessions repair --check-only` surface
the corruption and `repair_state_db_schema` heals it via the FTS rebuild
strategy (verified with a real stale-segid fixture).
Refs #100227
Reported-by: #100227
The recovery commands rendered on structural corruption — the turn explainer's
`session_persistence_failed`/corrupt body, the gateway's home-channel state.db
warning, and hermes_state_repair._persistent_repair_exhausted_error — already
interpolate the active profile's state.db path, but every `hermes ...` verb in them
was bare. A bare `hermes` follows the sticky `active_profile` file, so an operator
running the pasted `hermes doctor --fix` (or `hermes sessions recover` with a
relative source) from a named-profile incident could inspect or repair a different
profile's database (#105887).
hermes_constants.profile_cli_selector() renders `-p <name> ` for a named profile
home (default home and custom roots outside the profile tree render nothing: the
default is what a bare `hermes` already means, and a custom root is only reachable
via HERMES_HOME). Every command in the three guidance sites now carries it, and
the new `fts_index` guidance inherits the same interpolation.
Live check with HERMES_HOME=<root>/profiles/research and active_profile=other:
before `1. Run \`hermes doctor --fix\`` (targets "other"); after
`1. Run \`hermes -p research doctor --fix\`` and `hermes -p research sessions
recover --source <root>/profiles/research/state.db --inspect-only`.
Refs #105887
Reported-by: Cuttingwater
`hermes doctor` reported every write-health-probe failure as "state.db FTS write
corruption" and `--fix` ran the FTS repair ladder — rebuild, REINDEX, sqlite_master
surgery + VACUUM — on the damaged file. When the damage is structural (canonical
tables/indexes), none of those rungs can fix it, each one writes to the torn file in
place, and the operator is then told to "restore from the backup copy beside
state.db": a `.malformed-backup` that is a snapshot of the same corrupt image.
`hermes sessions recover`, the tool that actually rebuilds canonical rows into a
fresh file, was never mentioned (#88587; the 1.7 GB field incident lost days to it).
Discriminate before mutating. hermes_state_repair.integrity_damage_is_structural
maps `PRAGMA integrity_check` output onto the file: a `Tree N` id resolved through
sqlite_master.rootpage, an index named in `row N missing from index X`, or a
`Freelist:` line is structural unless the object is a Hermes-owned messages_fts*
table/shadow (full-matched, so a user lookalike such as archive_fts_data is never
swept into the rebuildable set). state_db_has_structural_damage runs it read-only on
a fresh connection; an integrity_check that RAISES under the walk (torn root page)
is structural too — no FTS-only fixture does that while sessions/messages read
cleanly. doctor's state check consults it first: structural damage becomes a
manual issue naming `hermes [-p <profile>] sessions recover --source <this db>
--inspect-only` (profile pinned, #105887) and explicitly warning off the
.malformed-backup; nothing is mutated and no backup is written. FTS-only damage
keeps the existing in-place repair path.
Verified against real fixtures: a torn `sessions` root page (before: "FTS write
corruption", --fix wrote a 1:1 malformed-backup and failed; after: structural,
recover guidance, no writes) and the 16-byte DEADBEEF messages_fts_data stomp
(still repaired in place via rebuild_fts).
Salvaged from PR #88604 (liuhao1024) onto the split doctor_state.py; the
classifier lives beside the repair ladder in hermes_state_repair so the ladder
itself can consult it next.
Fixes#88587
hermes_state.py: delete every '# noqa: F401 (re-exported...)' import block (hermes_state_common/errors/guard/
readpool/sessions/fts/dbfile/wal/repair/registry + agent.context_compressor _DB_PERSISTED_MARKER_KEY); keep
only the names hermes_state.py itself uses, without noqa.
hermes_state_registry.py: drop get_shared_session_db/release_shared_session_db/close_shared_session_dbs
aliases; every caller (gateway/, tools/, tui_gateway/, cron/, mcp_serve, run_agent, tests) now imports
acquire/release/close_all/release_or_close from hermes_state_registry.
hermes_state_titles.py: drop set_auto_title_if_empty shim (title_generator keeps its getattr fallback).
Re-remove shim-only names restored by 34abf954bd: latest_user_message_row_id (tests call
latest_message_row_id(key, role='user'); role-targeting assertions kept) and get_session_activity (tests
build the snapshot via agent.session_activity.build_activity_snapshot over db.get_session(sid)).
hermes_state_wal._log_once resolves its dedupe sets as module globals instead of via hermes_state;
hermes_state_repair helpers call module globals directly (tests patch hermes_state_repair.<name>).
Frozen updater surface untouched (update_cmd_maint imports only SessionDB from hermes_state).
Follow-up to b8f99bfc43: the seam edits for hermes_state_repair.py and tools/approval_detection.py
were overwritten by a concurrent squad's write before that commit landed (only their docstring
restores got in). Re-apply: _repair_conn/_open_exclusive/_db_opens_cleanly look up
_connect_repair_durable via hermes_state at call time; detect_dangerous_command/
detect_hardline_command look up _command_detection_variants via tools.approval, so patching the
facade (as tests/state/test_state_db_wal_unlink_race.py and tests/hermes_cli/test_approvals_test.py
do) reaches the call again, as on BASE 63279301bc.
Reviewer P2 (kshitijk4poor): tests patch hermes_cli.models.get_cached_nous_inference_base_url
but models_pricing.pricing_cache_scope read its own module global, so the patch never reached
the call and the test passed on the default-endpoint fallback. Same seam-erosion class audited
across /tmp/rf/patch_traps.json (777 candidates) with an AST reachability check + a per-test
call-count probe (facade vs defining module) against BASE 63279301bcb; three seams actually
bypassed their patch on HEAD but not on BASE:
- hermes_cli.models.get_cached_nous_inference_base_url <- models_pricing.pricing_cache_scope
- hermes_state._connect_repair_durable <- hermes_state_repair._open_exclusive/_repair_conn/_db_opens_cleanly
- tools.approval._command_detection_variants <- approval_detection.detect_{dangerous,hardline}_command
Each now looks the name up through its facade at call time (the pattern hermes_state_repair
already used in live_writer_holds_db), restoring BASE's patchability.
