/proc/<pid>/fd reports the kernel-resolved dentry, but
_watched_sqlite_sidecar_paths built its canonical watch keys with
abspath, which never resolves symlinks. With a symlinked HERMES_HOME
every deleted state.db-wal/-shm generation was invisible to the scan,
so refuse_deleted_wal_generation never fired and a second opener could
mint a replacement WAL -- the split-brain the guard exists to prevent.
foreign_state_db_holders and the macOS libproc leg already compare
against realpath'd paths for exactly this reason; only the Linux
deleted-sidecar scan was still on abspath. Watch both resolved
spellings per sidecar -- the realpath'd parent plus the literal
basename, and the fully resolved path -- because SQLite canonicalizes
a symlinked database file before naming its sidecars while older
versions name them after the path opened.
Squashed integration of the user-facing message audit for this surface set.
Full per-finding receipts: /tmp/ux-audit/lanes/*-receipt.md (campaign artifacts).
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>
`_fd_is_truly_unlinked` stats `/proc/<pid>/fd/<n>` after the scan has
already read the readlink target. Between those two steps the descriptor
can be closed (ENOENT, handled by the previous commit) or the whole
process can exit (ESRCH). Both mean the descriptor can no longer keep a
retired WAL/SHM generation alive, so neither is evidence of a live holder
and neither should make the guard refuse to open the database.
Match the sibling scan in hermes_state_holders, which already skips both
errnos, by branching on `exc.errno in (ENOENT, ESRCH)`; any other OSError
still fails closed. The existing closed-descriptor test is parametrized
over both errnos, injecting the failure at `os.stat` so the ESRCH path is
exercised on every platform.
`count_db_holders` returned None on every non-Linux host, so `hermes doctor`
on macOS (every reporter in the deleted-WAL cluster) printed no
"N process(es) holding the DB open" row. The libproc enumeration that #110544
landed for the sidecar scan already yields `(pid, fd, path, (st_dev, st_ino))`;
count distinct PIDs whose fd identity matches state.db's inode. Identity, not
pathname: libproc reports the path as the opener spelled it (case, symlinked
prefix), which is what the sidecar leg had to case-fold around.
Carries the surviving delta from #110023 (@kshitijk4poor), whose libproc leg
otherwise landed via #110544.
`hermes_state_dbfile._canonical_sqlite_path` was a byte-identical copy of
`hermes_state_holders.canonical_sqlite_path`; keep the public one and repoint
the two hermes_state call sites. No import cycle: holders is stdlib+psutil.
Three gaps in the #110544 guard, all reported in its review and reproduced:
- A writer reopened by _reopen_after_close_locked (teardown/worker race,
#94736) came back with no guard: the next stray close + foreign close
deleted its WAL again.
- _try_wal_checkpoint refreshed the guard outside self._lock; landing after
close() it pinned an OFD lock with no connection behind it, so a foreign
`PRAGMA journal_mode=DELETE` saw `database is locked` forever.
- Refcounts keyed on (fd, inode) treated a recycled fd number as a surviving
lock: A+B live, close A, C reuses A's fd, close B left C recorded as guarded
while a foreign EXCLUSIVE succeeded.
The guard now counts handles per inode, re-locks every matching descriptor on
each hold (OFD re-lock is idempotent), and unlocks on the last handle only;
the reopen path holds it; the checkpoint refresh runs under self._lock and
skips a closed handle. The macOS holder scan folds case so a case-only alias
of the sidecar path on APFS still matches.
iter_deleted_sqlite_sidecar_holders() returned [] on every non-Linux
platform, so refuse_deleted_wal_generation() -- the pre-connect refusal
that stops a second opener from minting a replacement WAL under a live
writer -- was a permanent no-op on macOS. The reporter of #109641 hit
exactly that: after an update/restart took the sidecars away, a fresh
opener minted a new generation at the path, the still-live writer's next
write raised DeletedWalGenerationError, and each event copied the whole
database (16 halts / 13 minutes / 4.2 GB of captures).
macOS has no /proc and never reports a " (deleted)" suffix, which is why
the scan was restricted to Linux, but libproc does describe other
processes' descriptors: proc_pidinfo(PROC_PIDLISTFDS) lists a process's
fds and proc_pidfdinfo(PROC_PIDFDVNODEPATHINFO) returns each vnode fd's
(st_dev, st_ino) plus the vnode's last pathname -- for same-user
processes, without elevation. Both survive unlink, which is also why
psutil.Process.open_files() cannot stand in for it (it hides unlinked
descriptors, so the retired generation is structurally invisible).
The judgement itself is unchanged and now shared: a descriptor counts
only when it names a watched sidecar path while its identity no longer
matches what that path holds, i.e. _fd_is_truly_unlinked()'s identity
test (#108082), never a path suffix or a link count. Only the source of
that identity differs per platform -- readlink on /proc for Linux,
libproc for macOS -- and the darwin side resolves symlinks before
comparing paths because libproc reports the kernel's path
(/private/var/... where the caller opened /var/...).
Scope is this one function: the enumeration legs, the gate (Windows
still returns [] -- it cannot unlink a held sidecar) and the stale
docstring reason. Enumeration failures keep the existing fail-open
behaviour (logged at debug, no holders), and the new tests are marked
macos_only so the existing Linux-only ones stay untouched.
Cost, measured on macOS 26.4 (darwin 25.4.0) with 721 processes /
4383 vnode descriptors: ~20 ms per full enumeration, versus the Linux
leg's ~11 ms / ~4.4k syscalls measured in #108910 -- the same order,
paid once per open, on the platform where the guard previously did
nothing at all.
(cherry picked from commit f1501dfe7141e3c2521c5192857c37d7b60a922b)
SQLite protects a WAL generation with per-PROCESS POSIX locks (SHARED on
state.db, DMS byte on -shm). Any in-process open()/close() of either file
cancels both (sqlite.org/howtocorrupt.html §2.2); the next last-connection
close in ANY process then checkpoints and unlinks -wal/-shm, and the holder
sticky-halts with DeletedWalGenerationError. #109841 removed one such
close (mode tightening) but the class is open-ended: raw header probes,
plugins, tool reads of ~/.hermes, any library that touches the files.
hermes_state_lockguard re-holds the same two ranges as OFD locks
(F_OFD_SETLK) on private descriptors for as long as a writer handle is
open. OFD locks belong to the open file description, so a stray close()
cannot cancel them, and they conflict with the EXCLUSIVE a sibling needs
for the close-time reset exactly like SQLite's own. Released before the
handle's own close so a true last close still ends the generation; the
descriptors are closed only once no connection to the path remains, so a
holder scan from another process never counts them. Works on Python 3.11
(where sqlite3 cannot arm SQLITE_DBCONFIG_NO_CKPT_ON_CLOSE) and on macOS
(F_OFD_SETLK=90 per XNU bsd/sys/fcntl.h); no-op on Windows.
Live repro (Linux, Python 3.11.15, SQLite 3.53.1): holder = SessionDB
writer; in-process os.open/os.close of state.db and -shm; then a foreign
sqlite3.connect()+close(). Before: -wal unlinked, holder write raises
DeletedWalGenerationError. After: -wal keeps its inode, holder writes.
st_nlink == 0 alone cannot distinguish a genuine orphan from one that
still has a surviving hard link (e.g. a backup) after the watched
sidecar path itself was removed or replaced — that left st_nlink >= 1
on a truly orphaned generation, letting a new opener through while a
live writer still owned the old one. Compare (st_dev, st_ino) between
the fd and the current watched sidecar path instead: only an exact
match means they're the same live file, so any mismatch or unstattable
watched path still fails closed.
iter_deleted_sqlite_sidecar_holders() and SessionDB._wal_generation_was_lost()
both treated a `` (deleted)`` suffix on a /proc/<pid>/fd/* target as proof that
state.db-wal or state.db-shm was unlinked. On OpenZFS that suffix is not proof:
a live, still-linked file whose dentry was unhashed is reported the same way,
with st_nlink still 1 and the same (dev, ino) as the path. The guard then fires
permanently and the gateway falls back to JSONL forever, because the WAL was
never actually deleted.
Add _fd_is_truly_unlinked(), which confirms via os.stat(fd_path).st_nlink == 0
before a target counts as an orphaned generation. An unstattable descriptor
still counts as deleted, so the guard keeps failing closed. _iter_proc_fd_targets()
and _proc_fd_targets() now also yield the /proc fd path itself so both call
sites (open-path and the sticky write-path probe) can run the check.
- capture_retired_wal_generation: drop the main_image_max_bytes kwarg (no caller passes it; read the module constant).
- replace _write_json_durably with utils.atomic_json_write (late import keeps the capture dependency-light).
- clean the abandoned .partial staging dir on capture failure instead of leaving it for the retry to work around.
- _disable_close_time_checkpoint: document that it is the per-instance twin of _close_time_checkpoint_configurable and must agree with __init__'s capability decision.
- _close_quietly kept on the late import (module-level import from hermes_state_dbfile risks future cycles).
- hermes backup excluded *.db-wal by suffix but not the retired-wal capture dirs, so it would ship the capture's main-image copy while dropping the captured -wal that is the artifact's point; exclude <name>.retired-wal-* dirs whole (they must move as manifest+image+wal unit).
- _copy_range treated a short read as success, yielding a truncated copy with a valid manifest while the unlinked inode still dies at exit; raise RetiredGenerationCaptureError and clean the .part file.
- _quarantine_reason docstring no longer claims close() checks replaced before generation loss (close evaluates loss first and skips quarantine when lost).
_copy_descriptor and _copy_main_image were the same 17-line pread/sha256/fsync/rename loop
differing only in the byte source; both now delegate to _copy_range(read, dest, size=).
The header parser reuses _STATE_DB_APPLICATION_ID_OFFSET instead of re-spelling 68, and
_fsync_path keys off the module's _IS_WINDOWS like the rest of hermes_state_dbfile.
With the retired generation captured durably, closing a lost-generation
handle is safe wherever SQLITE_DBCONFIG_NO_CKPT_ON_CLOSE took effect: the
frames are preserved and sqlite3_close no longer checkpoints them into the
newer generation. On Python 3.11, where sqlite3 has no setconfig, closing
still runs SQLite's internal checkpoint over the newer main file, so only
there the exact quarantined connection is retained instead of closed: one
public Py_IncRef reference via ctypes.pythonapi (no struct-layout access),
bound before the writer opens, taken in close() after the capture. Runtimes
with setconfig never touch ctypes; a writable SessionDB requires CPython
with ctypes only where retention is the sole guard.
Read-only handles and every other quarantine reason close as before.
Regressions cover both branches: where retention applies, the retired inode
stays readable after close() and GC and an independent deleted WAL under
the same pathname is untouched; elsewhere the capture is the surviving copy.
A separate process's newer generation survives close, GC and normal exit
in both cases. The mock-based close-time regression originally written for
Refs #105670
Co-authored-by: fangliquanflq <fangliquan@qq.com>
After DeletedWalGenerationError the retired frames exist only in an
unlinked -wal inode that this process keeps open. #106315 stops them from
being checkpointed under wrong page numbers, but the canonical remediation
("stop the gateway, dashboard and cron writers, then reopen") lets the
kernel drop the inode with them: committed transactions whose disposition
is unknown were destroyed by the prescribed recovery path itself.
Capture the exact retired generation next to the database at the first
halt, or at close() if that sees the loss first. The WAL is located by the
(st_dev, st_ino) recorded at open among this process's own descriptors,
never by pathname, so a sibling's deleted WAL or a newer sidecar minted at
the same path cannot be mistaken for it; it is read with pread and no
descriptor is closed, moved or truncated. The artifact holds the WAL, the
-shm when still ours, the main image (or its header past a size cap) and a
manifest with identities, digests and the sidecar generation found at the
path at capture time. Nothing is merged: whether the frames belong on top
of the file now at the path stays an operator decision. close() refuses to
settle without the capture: it raises and leaves the handle open.
Regressions: capture at halt and at close, refusal to settle on capture
failure, inode-not-pathname selection with a second deleted WAL under the
same name, refusal to guess without a recorded identity, and a subprocess
control where rows committed only in the retired WAL are recovered from
the capture alone after the writer process has exited.
Refs #105670
`is_zeroed_state_db` and `has_invalid_sqlite_header_preopen` shared the
is_file / stat / live-connection / read_header preamble; `_preopen_header`
owns it now (zeroed is the NUL subset of "no SQLite header"). The
quarantine message also names `hermes sessions recover --source <bak>` so
the preserved bytes are actionable, not just parked.
Startup only quarantined a 0-byte / all-NUL state.db. A file whose first page
was clobbered with record bytes (#102198) went straight to sqlite3.connect,
which raised "file is not a database" and deleted the -wal sidecar — the one
piece of evidence that could have been recovered.
`has_invalid_sqlite_header_preopen` generalises the zeroed probe (zeroed is a
subset of "no SQLite header"; same live-connection contract, never raises).
`quarantine_invalid_state_db` moves the file AND its -wal/-shm aside as
`state.db.<zeroed|notadb>-<ts>-<pid>.bak` before anything opens it; a fresh
DB is created as before.
Re-authored on current main (the quarantine helpers moved to
hermes_state_dbfile.py in d15c61b5dc); one regression test proves the
notadb case is quarantined with its sidecars and the new DB passes
integrity_check.
Refs #102198 (the write-after-SIGTERM that clobbers page 0 is not addressed
here; this preserves the evidence instead of destroying it).
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).
For each issue anchor present in BASE 63279301bc non-test .py and absent on HEAD, the BASE comment/docstring block was re-attached at the HEAD location of the code it explained (matched by the distinctive code line / enclosing def). Sentences already covered by an existing HEAD comment were deduped; the issue number always survives. Insert-only: no code lines changed.
hermes_state_maintenance.py 557->418, hermes_state_dbfile.py 545->404.
- _placeholders() replaces 4 inline ','.join('?'...) builders (identical output)
- _write_guards_reject() unifies the lease/lock probe in sweep_orphaned_sessions
and prune_sessions (same kwargs, same exception set; prune keeps set -= order)
- _page_pragmas() absorbs the try/except-debug shape of logical_size_bytes and
_freelist_ratio (log texts unchanged); _try_checkpoint() for the two WAL
checkpoints in vacuum(); _seconds_since() for the two state_meta float parses
- archived tri-state -> f'string' clause (byte-identical SQL)
- dbfile: contextlib.suppress for pass-only excepts, lock closures collapsed,
unreachable size<0 branch dropped, is_zeroed tail folded to one predicate
- docstrings/comments compacted by hand; every WHY/lock-safety invariant kept
SQL PARITY OK (1120 stmts), MSG PARITY OK, import smoke OK.
