Repository navigation
storage: opening a live sessions.db with the sqlite3 CLI makes the daemon report disk I/O errors / corrupt the file #4513
Description
Observed
During the #4497 verification (isolated daemon, release build), opening a live sessions.db with the sqlite3 CLI while the daemon was running was followed each time by the daemon failing with disk I/O error, and once the file later reported file is not a database.
Why it matters
Users and agents routinely inspect stores with sqlite3 while the daemon runs. A WAL-mode SQLite database should tolerate a concurrent external reader. If the daemon's locking/VFS/WAL settings (exclusive locking mode, custom VFS, checkpoint/truncate behavior, PRAGMA choices) make an external reader corrupt the store, that is data loss.
Expected
Either an external read-only sqlite3 session is safe, or the daemon refuses it with a clear lock error; it must never corrupt the store. Reproduce with a controlled test and identify which setting allows it. Related: #4152, #4481 (corruption warnings).
Activity
fix(storage): preserve SQLite locks during pathname identity checks
ScriptedAlchemy commentedon Nov 3, 2026
Found a concrete lock-loss defect in the current default branch and opened #4720.
OpenedDatabaseFile::verify_current_pathopens/closes a raw database descriptor; on Unix that close discards process-owned SQLite record locks. Separate-process Linux probes reproduced both a second rollback-mode writer being admitted and a WAL journal-mode change being admitted while the original connection was live. Metadata-only identity checks preserved exclusion, normal release, committed data, and integrity in those probes.The PR keeps the pinned identity and both
HAS_MOVED-adjacent checks, and adds three Rust regressions plus a subprocess helper. Rust compilation/test execution remains pending hosted CI; the successful Python/native-SQLite probes are not Rust-suite results.Keeping this issue open: the exact reported external read-only sessions.db workflow has not been proven safe end-to-end. Retained raw descriptor clone/release/drop lifetimes (including
VerifiedReader::into_parts) still need auditing against overlapping connections, followed by stress/integrity validation with the repository's SQLite build and macOS coverage.fix(sqlite): retain verified reader pins through connection close
fix(sqlite): inspect live worker identity without closing inode FDs
fix(sqlite): share pinned descriptors across live workers
fix(sqlite): refuse reader locators after shared pin release
fix(storage): avoid POSIX lock loss in daemon size reports
fix(sqlite): preserve live writer locks across verified readers
ScriptedAlchemy commentedon Nov 6, 2026
A second lock-loss path was found in the default-branch
open_verified_reader: on Unix, a verified read snapshot/immutable reader independentlyFile::open-pins the same SQLite inode and closes that pin when the reader is dropped. POSIX closes can release an unrelated live SQLite writer's process-owned locks. Separate-process native SQLite probes reproduced the failure for both rollback/WAL modes and both reader variants (all four combinations: an external writer or journal-mode switch was admitted after dropping the raw pin, despite the original transaction remaining live). The metadata-only/no-extra-fd analog retained external exclusion in all four combinations.Staged 6459ca2 on the sole application integration PR #4776 (Refs #4513). Unix verified readers now use pathname identity checks around
SQLITE_FCNTL_HAS_MOVED, fail closed if the VFS cannot supply that proof, and leave only SQLite's own descriptor open; Windows retains its existing pinned file handle. Added a real cross-process Rust regression for both journal modes and both reader types. The Linux/Python probes are independent analogs, not Rust/Bazel passes; repository Bazel tests have not run locally. Current-head hosted CI is queued, not passed.Issue remains OPEN: staged code is not on
master; exact reported external sqlite3sessions.dbreader stability, durability, and native macOS regression evidence still need end-to-end qualification. PR #4776 is held from master merge because it also contains user-excluded Kev/decision-model work.fix(storage): align verified-reader lifetime test with Unix locking
ScriptedAlchemy commentedon Nov 7, 2026
Additional independent SQLite failed-initialization cleanup data-loss risk found on current
master91c72aba7750a398dd28e83d64bf0c6d6fef4172and still present on held #47760566171ba967173809cc52c4e8cb42d0f43b0d64. This is distinct from the POSIX lock-loss paths already discussed here.crates/tracedecay-rusqlite-runtime/src/connection/mod.rsOpenedDatabaseFile::discard_createdverifies the pathname's identity once, drops its owned descriptor, then removes-wal,-shm,-journal, and the DB file with independent path-basedremove_filecalls. A replacement between verification and any unlink is not fenced.repository_start_failurecalls this cleanup after a newly created attachment fails; theinitialize -> attach_openedpath is a real production caller.store.dbas the owned file; between that identity check and cleanup, rename it aside and create an unrelated real file at the exactstore.dbpathname. Applying the production code's orderedPath::remove_fileoperations deletes the unrelated file, while the originally checked file survives at its new name. Both are ordinary files, not symlinks, so leaf no-follow alone would not close this race. This is a deterministic schedule analog, not a repository Rust/Bazel test, a native macOS result, or evidence of a production incident.Repair must bind cleanup authority to the created inode and an owned parent capability across failed attachment teardown, including sidecar identity/ownership, and fail closed when that cannot be proven. A second pathname recheck does not make an unlink conditional on inode identity. Add a deterministic Rust adversarial replacement regression for the failed-start path (and normal successful cleanup), run repository Bazel-only tests plus native macOS, and retain all peer edits. Until qualified, do not treat any existing SQLite shared-pin change as complete acceptance for #4513. No source or PR branch mutation in this investigation; no issue closure.
ScriptedAlchemy commentedon Nov 7, 2026
Refs #4513 — additional failed-initialization sidecar ownership / data-loss risk without any pathname race, distinct from the separately documented post-verification replacement window.
On current default
master91c72aba7750a398dd28e83d64bf0c6d6fef4172and sole held application PR #47760566171ba967173809cc52c4e8cb42d0f43b0d64:OpenedDatabaseFile::create_newusescreate_newfor the database file only. It does not refuse or record provenance of existing sibling-wal,-shm, or-journalfiles.discard_createdverifies only the just-created database inode, then unconditionally unlinks all three sidecar pathnames before unlinking the database.repository_start_failureis a real failed-initialization caller. There is no sidecar provenance check.Independent disposable Linux filesystem proof: for each suffix separately, create a pre-existing nonempty sentinel
fresh.db{suffix}whilefresh.dbdoes not exist;O_CREAT|O_EXCL-create and identity-checkfresh.db; run the exact source-ordered unlink operations with no swaps or concurrency. All 3 nonzero assertions passed: the pre-existing sidecar was deleted and the newly created database was removed. This is a filesystem schedule/operation analog, not execution of repository Rust/Bazel tests, native macOS or Windows testing, or evidence of a production incident.Repair requirement: a failed initialization must not delete a pre-existing sidecar, symlink, or newly substituted foreign entry merely because its filename matches a SQLite suffix. Define a provable sidecar ownership boundary at admission/creation and through cleanup, refuse ambiguous orphan sidecars, and add actual Rust regressions for all suffixes plus cross-process replacement; cover Windows and native macOS with Bazel/hosted tests. A preflight
exists()check alone is racy and not a complete fix. Keep #4513 OPEN: externalsqlite3-reader durability and the earlier path-replacement/lock-loss failures are separately unqualified onmaster. No source change or PR merge was made in this investigation; #4776 remains strictly held due to excluded Kev/decision-model work.ScriptedAlchemy commentedon Nov 8, 2026
Refs #4513 — new additional exposure on the successful initialization path, not just failed-start cleanup.
Live code on default
master91c72aba7750a398dd28e83d64bf0c6d6fef4172and held #4776453a19b2e644b0bcdc15ff40739002e7849bf111:OpenedDatabaseFile::create_newincrates/tracedecay-rusqlite-runtime/src/connection/mod.rsexclusively creates only the main DB, without owning or refusing an existing-wal,-shm, or-journalpathname.RepositoryPhysicalAttachmentFactory::initializethen starts the SQLite writer, which selects WAL mode. This exposure occurs beforediscard_createdcould run. Therefore simply removing/guarding cleanup's three sidecar unlinks does not prevent existing sidecar data from being overwritten by a successful first SQLite open.Independent actual SQLite reproduction, 3/3 nonzero assertions passed on Linux with Python sqlite3 (SQLite 3.46.1): For each suffix separately, in a disposable directory: (1) create a nonempty unique sentinel at
sessions.db{suffix}whilesessions.dbis absent; (2) createsessions.dbusing exclusive file creation; (3) open SQLite, executePRAGMA journal_mode=WAL,CREATE TABLE smoke(...), and one committed INSERT; (4) verifyPRAGMA quick_check=ok, then verify sentinel bytes differ or the sidecar disappeared. All three suffixes lost the original sentinel, with no concurrent mutation and no failed initialization.-wal/-shmwere replaced with SQLite contents and-journaldisappeared in this run. This is an OS/native-SQLite behavioral analog, NOT a TraceDecay Rust/Bazel test, native macOS/Windows result, or observed production incident. The sidecars were synthetic sentinels, not genuine recoverable WAL contents.Correctness requirement widened: Bind exclusive ownership of the entire SQLite basename/sidecar namespace before SQLite first opens it, with a trusted parent capability and refusal on any ambiguous pre-existing member; retain that ownership through both normal initialization and failure cleanup, without pathname-only TOCTOU. Regression must cover successful WAL-mode initialization against pre-existing
-wal/-shm/-journalas well as failed-start cleanup, external read-only client durability, replacement races, and native macOS/Windows with Bazel tests. Existing #4751 external-reader test onmasterdoes not cover this. No source change or merge made in this observation; issue stays OPEN, and mixed Kev-containing #4776 remains DRAFT and held.fix(sqlite): avoid raw verified reader pins on Unix
ScriptedAlchemy commentedon Nov 8, 2026
Refs #4513 — published the separately reviewable Unix verified-reader lock-loss fix as PR #4803, commit
83253b18e2d7, based onmaster91c72aba7750a398dd28e83d64bf0c6d6fef4172.This is a one-parent, three-file SQLite-only extraction of the relevant patch and cross-process regression from the held draft #4776. Unix verified readers no longer close an independent raw database descriptor that could discard unrelated process-wide POSIX SQLite locks; pathname dev/inode and
SQLITE_FCNTL_HAS_MOVEDevidence is checked around SQLite opening and unknown VFS support refuses the unpinned reader. Windows retains its original pin behavior. The Rust regression covers rollback/WAL × snapshot/immutable readers and checks external lock exclusion and final integrity.Publication is not acceptance: the newly created PR's hosted Bazel checks are queued, not passed; no local repository Bazel run, nonzero Rust test result, or native macOS qualification is claimed. The independently reproduced successful-initialization sidecar overwrite and failed-initialization cleanup ownership/TOCTOU cases are not addressed by #4803. They need safe entire-basename ownership before first SQLite open and defensible teardown, with dedicated regressions. Exact daemon + external sqlite3 durability, restart and native macOS end-to-end tests remain required. Keep #4513 OPEN. #4776 is still draft, not merged or modified, and its Kev/decision-model content remains excluded. Ordinary merge decisions belong to the existing TraceDecay PR-maintenance task.
23 remaining items
fix(sqlite): retire exact graph verification marker on reset
fix(sqlite): verify prepared contents before recovery publication
style(sqlite): clarify prepared-content generation witness
fix(sqlite): preserve POSIX locks for verified readers (#4803)
test(sqlite): include reserved journals in reset journeys
fix(sqlite): await writer capacity to finalize prospective schema
fix(sqlite): close staging readers before final WAL checkpoint
style(sqlite): format initialization reader shutdown barrier
fix(sqlite): close publication pins before releasing handoff
style(sqlite): format initialization handoff regressions
fix(sqlite): claim reader shutdown ownership atomically
style(sqlite): format initialization reader shutdown check
fix(storage): confine publication renames to pinned parents
fix(sqlite): refuse unverifiable worker VFS identity
style(sqlite): satisfy exact-head Bazel rustfmt gates