P1: on-disk state: close the residual schema and race gaps (durability contract ratified in #699)

[macanderson]

What decision this asks for

Ratify one durability contract for everything stella writes to disk — .stella/ state, the sidecar SQLite databases, the media manifest, and stella.storage.toml — and then implement it once rather than fifteen times. The maintainer has to make three calls up front: (a) is every durable write atomic (temp + fsync + rename), and what does the #[cfg(not(unix))] branch do when the hardened path is unavailable; (b) is a rename-over-target acceptable on the agent's primary source-mutation path, given it replaces the inode and drops mode/owner/hard links; (c) do the sidecar stores carry a schema-version row and real foreign keys, and what is the backfill/starting-value policy for stores already in the field. Two of these items are destructive (a migration that drops a dead seam, and a sweep that deletes LLM conversation history) and need an explicit go/no-go, not a default.

Once those three rulings exist, most of the items below collapse into one shared write helper plus one migration policy. Nothing here should land as a per-crate one-off.

Items

• Single-valued fact supersession closes only ONE prior belief. currently_valid_edge does ORDER BY id DESC LIMIT 1, so a single-valued assert leaves older live edges open. Fix is either close all live edges on supersession, or reject the assert as InvalidInput — mutually exclusive remedies, an owner call, and either changes facts_superseded counts and the belief set. stella-context/src/store.rs:975, stella-context/src/writeback.rs. Needs its own regression test and its own PR. (audit Batched audit cleanup: area:context — 15 items #558, deferred by chore(stella-context): document the public surface and cache store_kinds #589) — landed in store: one durability contract — atomic state writes, in-place source edits, stamped sidecars #699. Ruling taken: close all live edges. The lookup is now live_edges (moved into writeback.rs), the correction closes every one of them, links the newest as the SUPERSEDES predecessor, and counts facts_superseded += live.len(). Witnessed by a_single_valued_assert_closes_every_live_belief.
• Drop or harden edge.public_id. Uniqueness rests on a process-local AtomicU64 with no UNIQUE constraint (the edge table declares public_id TEXT NOT NULL, unlike node), so two stella processes writing one workspace db in the same clock second mint identical edg_… ids that coexist silently. Either branch needs a v4 migration whose backfill/downgrade story is unspecified. stella-context/src/store.rs:936. (audit Batched audit cleanup: area:context — 15 items #558, deferred by chore(stella-context): document the public surface and cache store_kinds #589)
• save_with_id silently overwrites and leaves two contradictory manifest rows. std::fs::write truncates any existing <id>.<ext>, then append_manifest adds a second row for the same path with a different sha256. Video ids are derived deterministically from the provider job id (artifact_id_for), so polling a completed job twice — the normal resume flow — triggers it. Remediation: OpenOptions::new().write(true).create_new(true), and on AlreadyExists either return the existing MediaArtifactRef on a sha256 match or fail with a named MediaError::Artifact; make append_manifest replace a same-path row instead of appending. stella-media/src/artifact.rs:118, stella-media/src/adapters/zai_video.rs:249. Deferred because it changes write semantics on a path stella-tools/stella-cli drive, and the return-vs-fail fork is open. (audit Batched audit cleanup: area:media — 10 items #565, deferred by chore(stella-media): fix the live-smoke gate, preview labels, branding #591) — landed in fix: the nine P0s from the backlog triage #658, which resolved the fork with create_new plus a sha256 match and made append_manifest replace the same-path row; store: one durability contract — atomic state writes, in-place source edits, stamped sidecars #699 added the missing directory fsync so a power loss cannot leave a manifest row pointing at a file that does not exist.
• append_manifest does read → append → temp-write → rename with no cross-writer serialization and a single fixed temp name. Two processes or two ArtifactStore handles over one root can both read the pre-image and lose a row, and can collide on .manifest.json.tmp mid-write. jobs.rs already solved this with an advisory File::lock plus pid+counter temp names. Remediation: lift JobStore::mutation_lock into a shared private helper, take it in append_manifest, and name the temp .manifest.json.tmp.<pid>.<seq>. stella-media/src/artifact.rs:200, stella-media/src/jobs.rs. Deferred as a cross-module refactor whose contended behavior could not be verified locally. (audit Batched audit cleanup: area:media — 10 items #565, deferred by chore(stella-media): fix the live-smoke gate, preview labels, branding #591) — landed in store: one durability contract — atomic state writes, in-place source edits, stamped sidecars #699: routed through the one stella_store::durable::write_atomic helper, so the temp name carries pid + a per-process counter and the parent directory is fsynced; the media manifest and jobs.json already hold an advisory lock across their read-modify-write.
• Add foreign keys from tasks/attempts/lineage/spend to runs in the fleet ledger, so orphan rows are not representable despite foreign_keys=ON. stella-fleet/src/ledger.rs:230. Deferred because the audit raised it only as a representability observation with no prescribed fix; retrofitting FKs onto four deployed tables is far larger than the UNIQUE constraint the bullet actually asked for, and orphan rows already on disk would fail the constraint with no stated policy. (audit Batched audit cleanup: area:fleet — 14 items #562, deferred by chore(stella-fleet): apply the area:fleet audit batch #593)
• Route project-scope settings writes through temp + fsync + rename to avoid truncated/corrupt settings on crash — currently a bare std::fs::write(path, bytes). stella-cli/src/settings/private.rs:25. Deferred because write_user_settings is #[cfg(unix)]-gated with a hard error elsewhere; the #[cfg(not(unix))] arm is never compiled locally or in CI, so the change cannot be verified on the platform it puts at risk. (audit Batched audit cleanup: area:cli — 30 items #557, deferred by chore(stella-cli): apply the area:cli audit batch (14 of 30 items) #597) — landed in store: one durability contract — atomic state writes, in-place source edits, stamped sidecars #699 via durable::write_atomic_preserving_mode. The #[cfg(not(unix))] arm now does temp + fsync + rename instead of failing closed; only the parent-directory fsync is unix-only, and that asymmetry is stated in the module header.
• Duplicate filing of the same atomic-write defect at stella-cli/src/settings/private.rs:17. The audit issue filed this defect twice; same fix, same reason for deferral (the #[cfg(not(unix))] arm is never compiled locally or in CI). Fold into the item above rather than fixing separately. (audit Batched audit cleanup: area:cli — 30 items #557, deferred by chore(stella-cli): apply the area:cli audit batch (14 of 30 items) #597) — landed in store: one durability contract — atomic state writes, in-place source edits, stamped sidecars #699 with the item above.
• Store::migrate concurrent-first-open race. The version read and any_store_table_exists both sit outside the deferred transaction, so two processes opening a fresh store can race. stella-store/src/lib.rs:627, spills into stella-context/. Code shape is confirmed; the failure is inferred from SQLite's BUSY_SNAPSHOT behavior and is not witness-testable without a genuinely racing second process. It restructures the highest-blast-radius function in the crate, including the PRAGMA foreign_keys dance that must stay outside the transaction. (audit Batched audit cleanup: area:store — 23 items #569, deferred by chore(stella-store): apply ten batched audit fixes from the area:store sweep #599)
• The dead graph_nodes/graph_edges seam — confirmed dead, the only callers are tests.rs. Either delete it (removing public API and shipping a destructive migration) or #[doc(hidden)] it and name the future issue that will populate it. stella-store/src/ddl.rs:74. Whether the seam is abandoned or planned is the owner's call. (audit Batched audit cleanup: area:store — 23 items #569, deferred by chore(stella-store): apply ten batched audit fixes from the area:store sweep #599) — note: this is arguably already decided in docs/design/storage-map.md; check it and tick this row on inspection rather than reopening the question. store: one durability contract — atomic state writes, in-place source edits, stamped sidecars #699 deliberately shipped no destructive migration, so nothing has been dropped.
• Create-vs-rename race in ensure_workspace_generated_ignore. stella-store/src/private.rs:113. The audit issue disclaims it: it restructures write_committable_atomic's contract for one caller, means hand-rolling a mode-0644 no-follow create with matching non-unix stubs, and the race is not witness-testable. (audit Batched audit cleanup: area:store — 23 items #569, deferred by chore(stella-store): apply ten batched audit fixes from the area:store sweep #599)
• Rewrite latest_resumable as a filter over list() instead of its own duplicated scan. stella-store/src/sessions.rs:271. Deferred because resumable() deliberately re-reads the stored status while list() applies presented_status — inlining changes which status the filter sees for a record whose pid died between the two reads. Choosing the correct semantics is owner judgment. (audit Batched audit cleanup: area:store — 23 items #569, deferred by chore(stella-store): apply ten batched audit fixes from the area:store sweep #599) — landed in audit(round 2): 155 fixes from the 17-dimension re-audit — 79 → 80 #700, which picked the semantics: the filter is now !r.status.is_live() && journal::has_state(sidecar_dir) over list(), i.e. the presented status, so a record whose pid died between the two reads is judged once.
• Orphan-sidecar sweep in SessionRegistry::prune — reap sidecar directories (append-only journal + history.json, the LLM conversation snapshot) left behind when their <id>.json record is missing. Only the sync = true durability half of that checklist item shipped. stella-store/src/sessions.rs:184 (second half). Destructive: the sweep recursively deletes directories holding the conversation snapshot on the inference that a missing <id>.json means the directory is dead — a false positive destroys a session's history irrecoverably. (audit Batched audit cleanup: area:store — 23 items #569, deferred by chore(stella-store): apply ten batched audit fixes from the area:store sweep #599)
• Serialize append_meaning's read-modify-write. It rewrites a shared file through a fixed temp name (stella.storage.toml.tmp, via path.with_extension("toml.tmp")), so two concurrent appends both read the pre-write text and the later rename wins, silently losing the earlier entry; interleaved writes to the shared temp path can publish a spliced file. Fix: unique temp name plus an advisory lock file (O_CREAT|O_EXCL with retry) guarding the whole read-modify-write. stella-graph/src/manifest.rs:305. Deferred because the fix "adds a locking protocol and new failure modes, which is a design decision rather than a cleanup" — and landing only the unique-temp-name half would leave the lost-update race untouched. (audit Batched audit cleanup: area:graph — 15 items #563, deferred by perf(stella-graph): bound the audit's unbounded loops, scans, and walks #600) — landed in two halves: store: one durability contract — atomic state writes, in-place source edits, stamped sidecars #699 gave it a unique temp name and a durable replace (and flagged in a comment that the race was not yet fixed), then perf/reliability/accuracy: eight audit-backlog fixes, reconciled onto main's new durability contract #703 added the locking protocol — an O_CREAT|O_EXCL advisory lock bounded on both sides (5s wait, 60s stale-break) around the whole read-modify-write. Witnessed by concurrent_appends_all_survive, verified failing without the lock.
• Schema versioning for codegraph.db — a code_graph_meta(key, value) schema-version row plus a stepwise migration function. Without it MIGRATION stays idempotent CREATE TABLE IF NOT EXISTS DDL only, so a later column add or backfill can never reach an existing store. stella-graph/src/store.rs:42. Only the index half of that bullet ((level, address) and parent_id indexes) landed. Deferred because it contradicts item 14 of the same audit issue, which argues the opposite for the same file ("For codegraph.db this is defensible — the index is fully rebuildable from the tree") and asks only for a doc note; and stamping a version onto stores already in the field has no obviously correct starting value. (audit Batched audit cleanup: area:graph — 15 items #563, deferred by perf(stella-graph): bound the audit's unbounded loops, scans, and walks #600) — landed in store: one durability contract — atomic state writes, in-place source edits, stamped sidecars #699 as ensure_converged_schema_version, which also answers the starting-value question: user_version == 0 with every declared table present is stamped, never rebuilt; a missing table is not stamped (a stamped lie is worse than no stamp); a version from the future fails closed. Applied to codegraph.db and the media operation journal; the writer stamps, the read path deliberately does not.
• Shared write-temp-then-rename helper for the CRUD tools (write_file, edit_file, apply_edits, save_memory), which currently truncate-then-write straight onto the target path with tokio::fs::write. stella-tools/src/write.rs:88. Deferred because rename-over-target replaces the inode, dropping the original file's mode (a script loses +x), its owner, and any hard links — on the agent's primary source-mutation path. Preserving that matrix is a behaviour decision, and it could not be verified across platforms locally. The narrow, self-contained instance (exploration.rs:629) was applied. (audit Batched audit cleanup: area:tools — 26 items #570, deferred by chore(stella-tools): bound tool output, harden sub-resource auth, prune the process table #603) — landed in fix: the nine P0s from the backlog triage #658 (atomic write) and then ruled the other way by store: one durability contract — atomic state writes, in-place source edits, stamped sidecars #699: write_file, edit_file, apply_edits and save_memory now write in place (open without O_TRUNC, write, set_len, fsync, mode re-applied afterwards) precisely so the inode survives with its mode, owner, hard links, ACLs, xattrs and any open editor/LSP handle. atomic_write.rs → durable_write.rs, because it no longer claims atomicity it does not have.

Spot-checked against main at b5a99b4f: items 1, 2, 3, 4, 6/7, 13, and 15 are all still present as described (ORDER BY id DESC LIMIT 1 at store.rs:984; edge.public_id TEXT NOT NULL with no UNIQUE; std::fs::write(&dest, bytes) in save_with_id; the fixed .manifest.json.tmp; the bare std::fs::write(path, bytes) on the project-scope settings branch; path.with_extension("toml.tmp"); tokio::fs::write(&full_path, content)). None appear to have been fixed since the audit.

Why these are together

Every item on this list changes durable data, and almost all of them are the same defect wearing different crate names: a write that truncates before it has the replacement bytes, a read-modify-write with no lock and a fixed temp path, or a store whose schema can never be migrated because nothing records its version. Fixing them one crate at a time produces four incompatible write helpers and four migration conventions, and the two destructive items (the dead-seam migration and the sidecar sweep) can only be judged against a contract that says what state is authoritative and what is rebuildable. One ruling, one helper, one migration policy — then the fifteen become mechanical.

_{Deferred during the #546 audit remediation. Put Closes #<this issue> in the PR description and as a commit trailer when it lands.}

[macanderson]

Triage 2026-07-26 — 3 of 15 landed, 1 has a false premise

PR #658 (63ba537d, trailer Refs #617) closed three rows. Verified at 558474c9:

• Item 3 (save_with_id silent overwrite) — BUILT. stella-media/src/artifact.rs:155-164 now uses .create_new(true) with the comment "create_new, not write: a plain write truncates whatever is…". Same-bytes is idempotent; a genuine collision fails by name.
• Item 4 (append_manifest unserialized RMW) — BUILT. Now upsert_manifest (artifact.rs:264), holding mutation_lock(&self.root.join(format!("{MANIFEST_NAME}.lock"))) at :265. The lock helper was lifted to a shared pub(crate) fn mutation_lock at :324 and stella-media/src/jobs.rs:193 delegates to it — exactly the prescribed refactor. Fixed temp name gone.
• Item 15 (shared write-temp-then-rename) — BUILT. New stella-tools/src/atomic_write.rs, replace_file_atomically at :52, encoding the full per-target strategy: mode captured and re-applied after the write for setuid/setgid, in-place rewrite when nlink > 1 or a foreign owner, O_NOFOLLOW/O_CLOEXEC temp, parent-dir fsync. Adopted by write.rs:83, edit.rs:199, memory.rs:94, and both the apply and rollback paths of apply_edits.rs:321,333.

Item 11 has a false premise and should be rewritten, not implemented. stella-store/src/sessions.rs:279-284 already reads self.list().into_iter().filter(…).max_by_key(…) — it is the filter-over-list() the row asks for, and has been since PR #188 (064d17c6), well before the audit. What genuinely remains is only the semantic question the deferral names: resumable() at :270-275 re-reads stored status via get() while list() applies presented_status. Suggest rewriting item 11 as "decide whether resumable() should see stored or presented status".

Item 7 is an explicit duplicate filing of item 6.

Net: 11 of 15 open → 10 distinct items. The "Spot-checked against main at b5a99b4f" line is now wrong for items 3, 4 and 15.

Partial credit worth noting on item 9: the crate doc at stella-store/src/lib.rs:48-50 now says the graph schema is "reserved as a future seam for a context plane; not written by any shipping command today" — but it names no future issue, and neither remedy branch (delete + destructive migration, or #[doc(hidden)]) was taken.