Add POSTMORTEM.md: every real issue found, how, fix, and regression prevention

A detailed, permanent record covering the whole arc of this project's
development so far -- requested explicitly, as detailed as possible, to
prevent regression for ever. Seven sections:

1. Performance findings: the file-index O(n^2) treap rewrite, pack_write's
   O(n^2) dedup + latent hash-collision correctness bug, and the mount
   table's O(n^2) (confirmed, deliberately not fixed, with the reasoning).
2. Environment/tooling limitations: TSan's categorical block (every
   workaround actually tried and ruled out, not just the ones that
   worked), and both variants of the ASan/UBSan sandbox-startup flake.
3. Project professionalization: version API, SPDX, pkg-config, SECURITY.md,
   CHANGELOG.md, the Gitea-not-GitHub migration, and the CODE_OF_CONDUCT
   decision.
4. Git identity correction: the filter-branch rewrite, the tag-object
   tagger field it missed (found only by a full object-database sweep,
   not by re-reading git log), and the exhaustive re-verification.
5. Data-integrity fault injection: test_crash_consistency.c, the real bug
   in its own oracle (128 false failures before a progress side-channel
   fixed it), and the two real overlay.c bugs found while building the
   write-failure test (silent journal failure, torn-record poisoning of
   later records).
6. CI as a second, independent reviewer: the memory leak detect_leaks=0
   had been masking locally, and the set -e scripting bug -- including
   the mistake made while fixing it (reintroducing the same bug in a new
   shape), the follow-on bash-variable-blowup bug, the newly-discovered
   TSan segfault variant, and the retry-budget gap, each confirmed by
   direct reproduction under bash -eo pipefail, not reasoned about.
7. Distilled lessons: ten patterns extracted from the above, written to
   be applied to a new problem, not just recognized in this one.

Every figure cited was checked against BENCH.md/CLAUDE.md's own numbers
before writing this, not reproduced from memory.

Cross-referenced from CLAUDE.md's "Repository status", README.md's
"Contributing" section, and CHANGELOG.md -- including two entries CHANGELOG
itself was missing (the set -e CI fix from the previous commit, and this
document), the exact class of gap POSTMORTEM.md section 3 already
describes happening once before with the v0.1.0 tag.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UqJpkdJ6Njnt1pw3CbghzB

.gitea/workflows/ci.yml

@@ -30,28 +30,185 @@ jobs:
 
       - name: Build and run under AddressSanitizer + UndefinedBehaviorSanitizer
         run: |
+          # Two independent problems, both fixed below, confirmed by
+          # actually reproducing each rather than assumed fixed by
+          # inspection:
+          #
+          # 1. This step's default shell runs under `set -e`: a bare
+          #    `"./binary"` whose exit code is never captured (as an
+          #    earlier version of this loop did) aborts the *entire*
+          #    step the instant any single binary returns nonzero --
+          #    including the documented, non-fatal DEADLYSIGNAL sandbox-
+          #    startup flake (CONTRIBUTING.md), which would then read as
+          #    a real build failure. Confirmed directly: a two-line
+          #    reproduction of the same unwrapped-`OUT=$(cmd)` pattern
+          #    aborts under `-e` before the very next line, even one
+          #    that only reads $?, ever runs.
+          #
+          # 2. The flake's worse form is an *unbounded* repeating print
+          #    loop, not a clean single-line failure -- capturing that
+          #    into a shell variable via `OUT=$(cmd)` (an earlier version
+          #    of this fix did exactly that) is not just slow, it
+          #    reproduced as a genuine several-hundred-megabyte shell
+          #    string in one run, at which point this script's own
+          #    string classification stopped behaving reliably. Output is
+          #    redirected straight to a file instead (bounded disk I/O,
+          #    not an unbounded in-memory shell string), and only a
+          #    bounded prefix of that file is ever read back for
+          #    classification or printed -- generously sized (64 KiB) to
+          #    comfortably hold any real ASan/UBSan report this project
+          #    has actually produced (historically a few dozen lines),
+          #    while nowhere near what the runaway-loop flake produced.
           mkdir -p build/san
           for f in src/*.c; do
             cc -std=c11 -Wall -Wextra -O1 -g -fPIC -Iinclude -Isrc -D_GNU_SOURCE \
                -fsanitize=address,undefined -c "$f" -o "build/san/$(basename "${f%.c}").o"
           done
+          FAIL=0
           for t in tests/test_*.c; do
             name=$(basename "${t%.c}")
             cc -std=c11 -O1 -g -Iinclude -Isrc -D_GNU_SOURCE -fsanitize=address,undefined \
                "$t" build/san/*.o -lpthread -o "build/san/$name"
-            "./build/san/$name"
+            OK=0
+            LOG="build/san/$name.out"
+            for attempt in 1 2 3 4 5; do
+              if timeout 30 "./build/san/$name" > "$LOG" 2>&1; then
+                RC=0
+              else
+                RC=$?
+              fi
+              head -c 65536 "$LOG" > "$LOG.head"
+              if [ $RC -eq 0 ] && grep -q "^OK$" "$LOG.head"; then
+                cat "$LOG.head"
+                OK=1
+                break
+              fi
+              if grep -q "AddressSanitizer:DEADLYSIGNAL" "$LOG.head" \
+                 && ! grep -qE "ERROR: AddressSanitizer|runtime error:" "$LOG.head"; then
+                # 3 retries turned out not to be enough in practice: this
+                # sandbox's actual flake rate is meaningfully higher than
+                # the "roughly 1 in 5-10" CONTRIBUTING.md documents
+                # elsewhere (observed directly: 3 consecutive flake hits
+                # on the same binary, purely by chance, within just a
+                # handful of full-suite runs during this workflow's own
+                # development) -- 5 attempts reduces the chance of
+                # exhausting the budget on bad luck alone without making
+                # a real failure take meaningfully longer to report.
+                echo "::warning::$name attempt $attempt/5 hit the known DEADLYSIGNAL sandbox-startup flake (CONTRIBUTING.md), not a real finding — retrying"
+              else
+                echo "::error::$name — real ASan/UBSan failure (exit $RC), first 64KiB:"
+                cat "$LOG.head"
+                break
+              fi
             done
+            rm -f "$LOG" "$LOG.head"
+            if [ $OK -ne 1 ]; then
+              echo "::error::$name failed all 3 attempts (or failed with a real finding on one of them)"
+              FAIL=1
+            fi
+          done
+          if [ $FAIL -ne 0 ]; then
+            echo "One or more binaries had a real ASan/UBSan failure. Failing the build."
+            exit 1
+          fi
 
       - name: Build and run under ThreadSanitizer
         run: |
+          # TSan needs personality(ADDR_NO_RANDOMIZE) to disable ASLR for
+          # itself; some containerized runners (including this project's
+          # own registered Gitea runner, confirmed empirically, not just
+          # hypothesized — see CONTRIBUTING.md and CLAUDE.md) block that
+          # syscall outright via their default seccomp profile, in which
+          # case every TSan binary fails identically with `FATAL:
+          # ThreadSanitizer: unexpected memory mapping`, regardless of
+          # PackFS's own correctness. Treating that exact, specific
+          # failure signature as fatal would make CI permanently red for
+          # a reason that has nothing to do with the code under test — so
+          # this step distinguishes it from a real finding (a genuine data
+          # race, a crash, or any other failure) rather than either
+          # blanket-ignoring TSan failures (which would also hide a real
+          # race) or blanket-failing the build on an environment limitation
+          # this repository doesn't control.
+          #
+          # This step's default shell runs under `set -e`. The very first
+          # version of this script assigned OUT via a bare, unwrapped
+          # `OUT=$("./binary" 2>&1)` — under `-e`, that command's own
+          # nonzero exit status aborts the whole script immediately,
+          # *before* the next line (even `RC=$?`) ever runs, which means
+          # none of the classification logic below ever executed: the
+          # first binary to hit the FATAL/exit-66 case (all of them, on
+          # this runner) killed the step outright with that raw exit code
+          # as if it were a real failure, exactly the outcome this step
+          # exists to avoid. Confirmed directly (not just theorized): a
+          # two-line reproduction of the same pattern under `set -e`
+          # aborts before printing a line straight after it. Fixed by
+          # wrapping the invocation in `if ... ; then ... else RC=$?; fi`,
+          # which bash's `-e` rules exempt from triggering an abort.
+          #
+          # Output is also captured to a file and only a bounded prefix
+          # (64 KiB) is ever read back or printed, not the whole thing
+          # into a shell variable — the same fix the ASan/UBSan step
+          # above needed after that approach reproduced as a several-
+          # hundred-megabyte shell string when the flake's unbounded-
+          # repeating-loop form hit during this file's own development.
           mkdir -p build/tsan
           for f in src/*.c; do
             cc -std=c11 -Wall -Wextra -O1 -g -fPIC -Iinclude -Isrc -D_GNU_SOURCE \
                -fsanitize=thread -c "$f" -o "build/tsan/$(basename "${f%.c}").o"
           done
+          KNOWN_FLAKE=0
+          REAL_FAILURE=0
           for t in tests/test_*.c; do
             name=$(basename "${t%.c}")
             cc -std=c11 -O1 -g -Iinclude -Isrc -D_GNU_SOURCE -fsanitize=thread \
                "$t" build/tsan/*.o -lpthread -o "build/tsan/$name"
-            "./build/tsan/$name"
+            LOG="build/tsan/$name.out"
+            if timeout 30 "./build/tsan/$name" > "$LOG" 2>&1; then
+              RC=0
+            else
+              RC=$?
+            fi
+            head -c 65536 "$LOG" > "$LOG.head"
+            rm -f "$LOG"
+            # The known-limitation signature has two observed forms, both
+            # confirmed by direct, repeated reproduction on this runner
+            # (not assumed): (a) a clean `FATAL: ThreadSanitizer:
+            # unexpected memory mapping` message then exit 66, and (b)
+            # TSan's broken startup segfaulting outright instead of
+            # printing that message — confirmed to be the same underlying
+            # cause, not a real per-binary bug, by hitting three
+            # *different* binaries (test_journal_failure in one run,
+            # test_crash_consistency and test_dir together in another)
+            # across repeated full-suite runs, with zero reproductions
+            # tied to any specific binary's own code. Form (b) is
+            # recognized by the log containing only `timeout`'s own
+            # "dumped core" notice and nothing else — meaning the crash
+            # happened before any of the program's own output (or a real
+            # WARNING/SUMMARY ThreadSanitizer race report) had a chance
+            # to be written at all.
+            if [ $RC -eq 0 ]; then
+              cat "$LOG.head"
+            elif grep -q "FATAL: ThreadSanitizer: unexpected memory mapping" "$LOG.head" \
+                 && ! grep -qE "WARNING: ThreadSanitizer: |SUMMARY: ThreadSanitizer:" "$LOG.head"; then
+              echo "::warning::$name — known runner limitation (TSan can't start here), not a code finding:"
+              cat "$LOG.head"
+              KNOWN_FLAKE=1
+            elif grep -q "dumped core" "$LOG.head" \
+                 && [ "$(grep -vc "dumped core" "$LOG.head")" -eq 0 ]; then
+              echo "::warning::$name — known runner limitation (TSan's broken startup segfaulted instead of printing its usual FATAL message this time; same cause, confirmed by hitting other, unrelated binaries too), not a code finding:"
+              cat "$LOG.head"
+              KNOWN_FLAKE=1
+            else
+              echo "::error::$name — real ThreadSanitizer failure, first 64KiB:"
+              cat "$LOG.head"
+              REAL_FAILURE=1
+            fi
+            rm -f "$LOG.head"
           done
+          if [ $REAL_FAILURE -ne 0 ]; then
+            echo "One or more binaries failed ThreadSanitizer for a reason other than the known runner limitation. Failing the build."
+            exit 1
+          fi
+          if [ $KNOWN_FLAKE -ne 0 ]; then
+            echo "ThreadSanitizer could not run at all on this runner (known limitation, not a code issue) — this step is not treated as a build failure, but TSan verification did NOT happen this run. See CONTRIBUTING.md."
+          fi

CHANGELOG.md

@@ -12,6 +12,93 @@ This project is pre-release (`0.x`, an initial, partial implementation of
 tagged locally in this repository's own git history but has not been
 pushed to any remote — there is no public release yet, only a local one.
 
+## [Unreleased]
+
+### Fixed
+- **A failed journal write (disk full, quota, an I/O error) used to be
+  silently reported as success.** `journal_append_record` and its callers
+  were `void` and never checked `fwrite`/`fflush`/`fsync`'s return
+  values, so `vfs_write`/`vfs_mkdir`/`vfs_unlink`/`vfs_rename` on an
+  overlay-backed file all reported success even when the durable journal
+  append actually failed. Fixed by propagating success/failure through
+  the whole call chain; the affected call now returns `VFS_ERR_IO`.
+- **A second bug, found only by reproducing the first fix's edge case
+  directly: a partial write left a torn record in the journal that made
+  every later, individually-*successful* write unrecoverable on reopen
+  too**, not just the failed one — `journal_replay` stops at the first
+  record it can't fully read, regardless of what valid records follow it.
+  Fixed by rolling the journal file back to its exact pre-record length
+  whenever a record fails partway. Both bugs covered permanently by the
+  new `tests/test_journal_failure.c`, which forces a real write failure
+  via `RLIMIT_FSIZE` rather than a mock.
+- Neither bug was caught by an existing test — both were found by code
+  review while investigating a question about data-integrity
+  trustworthiness, then confirmed by direct reproduction before and after
+  each fix, not merely inspected and assumed correct.
+- **`tests/test_journal_failure.c` itself leaked memory on its normal,
+  expected code path** (`vfs_free(v2)` was only called on the unreached
+  `else` branch, in two separate reopen blocks) — invisible locally
+  because local sanitizer runs had been using
+  `ASAN_OPTIONS=detect_leaks=0`, caught by Gitea CI, which does not set
+  that option. Fixed, and the local/CI sanitizer-invocation mismatch that
+  let it go unnoticed is now called out explicitly in `CONTRIBUTING.md`.
+  Also fixed a `-Wunused-result` warning on an intentionally-ignored
+  `write()` return value in `tests/test_crash_consistency.c`'s progress
+  side-channel.
+
+### Added
+- `tests/test_crash_consistency.c`: real `fork()`+`SIGKILL` fault
+  injection (not a hand-truncated file) against journaled writes and
+  compaction, with an independent progress side-channel so the test's own
+  oracle can distinguish "never attempted before the kill" from
+  "attempted and lost" — a distinction its own first draft got wrong,
+  reporting 128 false failures before that side-channel was added. Zero
+  real corruption found across dozens of genuine interruptions per run,
+  both scenarios, on repeated runs including under ASan/UBSan.
+
+### Documented
+- The pack integrity checksum's (FNV-1a64) non-cryptographic nature —
+  already documented for the compaction-dedup bug — was never explicitly
+  connected to its *other* job, load-time pack integrity validation,
+  where it matters more (`SECURITY.md`).
+- TSan's inability to run is now confirmed in a *second*, independent
+  sandboxed environment (a separate remote cloud sandbox, tried
+  specifically to check whether the restriction was one environment's
+  fluke), not just this project's own — same root cause
+  (`personality(ADDR_NO_RANDOMIZE)` blocked by seccomp) confirmed both
+  generically and against this project's actual test suite.
+- **`POSTMORTEM.md`**: a detailed, permanent record of every real bug and
+  environment issue found across this project's development, how each was
+  actually diagnosed (including false starts), the final fix, and the
+  regression-prevention artifact for each — written so a future session
+  doesn't have to re-diagnose something already answered there.
+
+### Fixed (CI)
+- **`.gitea/workflows/ci.yml`'s ThreadSanitizer step was failing CI for an
+  environment reason, but the actual bug was in the CI script, not the
+  code under test.** Gitea Actions' `run:` steps execute under `set -e`;
+  an unwrapped `OUT=$("./binary" 2>&1)` aborts the whole script on the
+  first nonzero exit *before* the step's own classification logic (which
+  exists specifically to not fail the build over the already-documented
+  TSan-can't-start-here limitation) ever runs. Fixed by wrapping every
+  such invocation in `if cmd; then RC=0; else RC=$?; fi`. Fixing this
+  surfaced two more real issues, both also fixed: capturing unbounded
+  flake output into a bash variable (rather than a file with a bounded
+  read-back) caused a genuine misclassification at scale, and a second,
+  previously undocumented segfault variant of the same TSan-can't-start
+  flake (confirmed via hitting multiple unrelated binaries at random, not
+  a per-binary bug). The ASan/UBSan step's retry budget was also raised
+  from 3 to 5 after a real 3-in-a-row flake exhaustion was observed during
+  this same verification work. See `POSTMORTEM.md` §6.2 for the full
+  diagnosis, including the exact reproduction technique used to confirm
+  each fix (not just reasoned about).
+- A `-Wunused-result` warning on `tests/test_crash_consistency.c`'s
+  intentionally-ignored `write()` return value built clean locally (a
+  `(void)` cast) but still warned on the Gitea runner's own gcc — a real
+  toolchain configuration difference, confirmed by reproducing clean
+  locally with identical flags first. Fixed with an actual conditional
+  branch on the return value instead, which is portably honored.
+
 ## [0.1.0] - 2026-09-14
 
 ### Added

CLAUDE.md

@@ -4,7 +4,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
 
 ## Repository status
 
-This repository contains a working v0 implementation of the `concept.md` specification: `include/packfs.h` (public API), `src/*.c` (implementation), `tests/test_*.c` (test suite), and open-source project scaffolding (`README.md`, `LICENSE`, `CONTRIBUTING.md`, `CHANGELOG.md`, `SECURITY.md`, `packfs.pc.in`, `.gitea/workflows/ci.yml` — this project is hosted on Gitea, not GitHub), alongside the frozen `concept.md` and this file. Every file under `src/` and `include/` carries an `SPDX-License-Identifier: MIT` tag; `include/packfs.h`'s `PACKFS_VERSION_*` macros are the single source of truth for the project's version — `pfs_version()` (runtime) and `packfs.pc` (generated by `make install`) are both derived from them, never maintained separately.
+This repository contains a working v0 implementation of the `concept.md` specification: `include/packfs.h` (public API), `src/*.c` (implementation), `tests/test_*.c` (test suite), and open-source project scaffolding (`README.md`, `LICENSE`, `CONTRIBUTING.md`, `CHANGELOG.md`, `SECURITY.md`, `packfs.pc.in`, `.gitea/workflows/ci.yml` — this project is hosted on Gitea, not GitHub), alongside the frozen `concept.md`, this file, and `POSTMORTEM.md` (every real bug and environment issue found during development, how each was actually diagnosed, the final fix, and the regression-prevention artifact for each — read it before re-investigating something that might already be answered there, and add to it rather than letting a future finding go unrecorded). Every file under `src/` and `include/` carries an `SPDX-License-Identifier: MIT` tag; `include/packfs.h`'s `PACKFS_VERSION_*` macros are the single source of truth for the project's version — `pfs_version()` (runtime) and `packfs.pc` (generated by `make install`) are both derived from them, never maintained separately.
 
 ## Build, test, and lint commands
 
@@ -19,7 +19,7 @@ A single test: `make build/test_mem && ./build/test_mem` (substitute any `test_*
 
 Sanitizer builds are not wired into `make test` (they need per-file compilation with sanitizer flags plus `-D_GNU_SOURCE -Iinclude -Isrc`); see `.gitea/workflows/ci.yml` (Gitea Actions) for the exact invocation, which CI runs on every push. **Any change to `upper.c`, `overlay.c`, or `vfs.c` must be verified under `-fsanitize=address,undefined` and `-fsanitize=thread` before being considered done** — this is not a formality: exactly this process caught a real use-after-free in the snapshot-reclamation logic during initial development (see the `reclaim_gate` note below), which the plain build and even repeated plain test runs never surfaced.
 
-**If your environment cannot run ThreadSanitizer at all, say so rather than skipping it silently.** TSan needs `personality(ADDR_NO_RANDOMIZE)` to disable ASLR for itself; some sandboxes block that syscall outright, in which case *every* TSan build fails identically (`FATAL: ThreadSanitizer: unexpected memory mapping`), including a trivial unrelated pthread program — that is the confirming test, not a PackFS-specific symptom. In that situation, ASan/UBSan are still run and still matter (they are what actually caught the `reclaim_gate` bug above), but they do not perform TSan's happens-before race analysis and are not a substitute for it; a change is TSan-verified only once it has passed on a machine or CI run that can actually execute it, not merely because ASan/UBSan passed.
+**If your environment cannot run ThreadSanitizer at all, say so rather than skipping it silently.** TSan needs `personality(ADDR_NO_RANDOMIZE)` to disable ASLR for itself; some sandboxes block that syscall outright, in which case *every* TSan build fails identically (`FATAL: ThreadSanitizer: unexpected memory mapping`), including a trivial unrelated pthread program — that is the confirming test, not a PackFS-specific symptom. **This project's own CI runner is one of them, confirmed by its first real run** (see `CONTRIBUTING.md` for the details and for exactly how `.gitea/workflows/ci.yml`'s TSan step now tells that specific, known failure apart from a real finding rather than either hiding it or failing the build over it) — a green CI run is therefore not proof TSan executed. In that situation, ASan/UBSan are still run and still matter (they are what actually caught the `reclaim_gate` bug above), but they do not perform TSan's happens-before race analysis and are not a substitute for it; a change is TSan-verified only once it has actually passed on a machine confirmed able to execute it, never merely because ASan/UBSan passed or because a CI step reported success. **This has been actively investigated, not just accepted at face value:** in this sandbox, `personality(ADDR_NO_RANDOMIZE)` itself returns `EPERM` when called directly (confirmed via a raw syscall, not just observed via TSan's own error); `setarch -R` (which just calls the same syscall) fails identically; there is no `TSAN_OPTIONS` flag that relaxes the startup memory-mapping check (checked against the runtime's own `help=1` option listing); the process holds zero effective capabilities (`CapEff` all-zero) under an active seccomp filter (`Seccomp_filters: 1`) that also blocks `unshare --user`, ruling out working around it via a nested, less-restricted namespace. This is a categorical, syscall-level restriction with no userspace workaround available in this environment, not a configuration this project has simply failed to find yet.
 
 **A related but separate flake affects ASan/UBSan themselves in that kind of sandbox, not just TSan:** a sanitizer-built test binary can non-deterministically fail to start with `AddressSanitizer:DEADLYSIGNAL`, occasionally as an unbounded repeating loop rather than a single line — a sandbox startup race, confirmed by it hitting different, unrelated binaries across repeated runs, each of which then passes cleanly on retry. See `CONTRIBUTING.md`'s workflow section for the full description and the required mitigation (`timeout`-wrap sanitizer runs in such an environment; treat `DEADLYSIGNAL` alone, without an actual `ERROR: AddressSanitizer` or `runtime error:` string, as inconclusive and re-run rather than as a finding).
 
@@ -29,12 +29,12 @@ Sanitizer builds are not wired into `make test` (they need per-file compilation
 - `src/internal.h` — every internal type shared across `.c` files; read this first when touching implementation code.
 - `src/vfs.c` — the `Vfs` mount table itself (`MountSnapshot`, refcounted, atomically swapped), the public API's dispatch-by-longest-prefix-match, and `pfs_version()` (trivially returns `PACKFS_VERSION_STRING`, kept next to `vfs_new`/`vfs_free` as the other whole-library-lifecycle entry points).
 - `src/upper.c` — the writable layer shared by `mem`, `dir`, and the overlay's upper side: `UpperSnapshot`/`UpperEntry`/`MutCell`, the persistent-treap index (`TreapNode` and `treap_*`/`node_*` — see "Known performance characteristics" below for why it's a treap and not a flat array), the structural-write functions (`upper_create`/`upper_mkdir`/`upper_remove`/`upper_rename`/`upper_copy_up`), the content-write fast path (`upper_cell_read`/`upper_cell_write`), the range-query API (`upper_visit_range`, used by `readdir` in both this file and `overlay.c`), and the standalone `mem`/`dir` `Backend` (`backend_mem_new`/`backend_dir_new`).
-- `src/overlay.c` — composes a `Pack` (lower, read-only) with an `UpperStore` (upper): copy-up, whiteouts, the merged view (`overlay_stat`/`overlay_readdir`), the append journal, and `vfs_sync`'s compaction.
+- `src/overlay.c` — composes a `Pack` (lower, read-only) with an `UpperStore` (upper): copy-up, whiteouts, the merged view (`overlay_stat`/`overlay_readdir`), the append journal (`journal_append_record` reports and rolls back a failed write rather than leaving a torn record or silently reporting success — see "Known reliability characteristics" below), and `vfs_sync`'s compaction.
 - `src/pack.c` — the on-disk pack format: load-time integrity validation, binary-search lookup/range-query, the compaction writer (atomic rename, exact-duplicate elimination via a `DedupSlot` open-addressing hash table over `(hash, size)` plus a `memcmp` verification before ever reusing a `data_off` — see "Known performance characteristics" below for why it isn't a linear scan), and the standalone read-only `pack` `Backend` (`backend_pack_new`) for mounting a pack with no writable upper layer at all.
 - `src/containment.c` — `dir`-mount path containment (`openat2`/`O_NOFOLLOW` fallback) and opt-in Landlock hardening.
 - `src/path.c` — virtual-namespace `.`/`..` canonicalization.
 - `src/hash.c` — FNV-1a64, used for pack checksums and compaction dedup.
-- `tests/` — one binary per concern (`test_mem`, `test_dir`, `test_pack_overlay`, `test_concurrency`, `test_index_stress` — the treap's correctness under thousands of randomized structural operations, cross-checked against an independent reference model); `test_harness.h` is a small assertion-macro header, not a framework, consistent with the zero-dependency constraint.
+- `tests/` — one binary per concern (`test_mem`, `test_dir`, `test_pack_overlay`, `test_concurrency`, `test_index_stress` — the treap's correctness under thousands of randomized structural operations, cross-checked against an independent reference model; `test_pack_write_perf` — the `pack_write` dedup O(n²) regression tripwire; `test_crash_consistency` — real fork()+`SIGKILL` fault injection against a real process, not a hand-truncated file, verifying Section 4.3/4.4's crash-safety claims for both journaled writes and compaction, with an independent progress side-channel so the oracle can tell "never attempted before the kill" apart from "attempted and lost" — see its own file header before touching journal or compaction code; `test_journal_failure` — a real, forced journal-write I/O failure via `RLIMIT_FSIZE`, covering the two journal-durability bugs "Known reliability characteristics" below documents, that `test_crash_consistency` doesn't: a failed write silently reported as success, and a torn record left behind by that failure poisoning replay for every subsequent record too); `test_harness.h` is a small assertion-macro header, not a framework, consistent with the zero-dependency constraint.
 
 **One architectural fact spans every mutable structure and is easy to miss reading any single file in isolation:** `MountSnapshot` (`vfs.c`), `UpperSnapshot` (`upper.c`), and a `mem`-backed `MutCell`'s buffer (`upper.c`, Section 5.7) are all retired through the same two-step pattern — publish the replacement via `atomic_store_explicit(..., memory_order_release)`, then free the superseded object only under a dedicated `reclaim_gate` rwlock's *write* side, while every acquirer takes that same gate's *read* side around its own load-then-increment. This exists because a plain "load a pointer, then atomically increment its refcount" leaves a real gap between those two steps in which a concurrent writer can free the very object being acquired — confirmed by ASan as an actual heap-use-after-free during development, not a theoretical concern. See the comment on `UpperStore.reclaim_gate` in `internal.h` for the full reasoning. Any new refcounted, concurrently-reclaimed structure added to this codebase needs the same gate, not just an atomic pointer and a naive refcount. The one exception, and the reason it's an exception rather than a hole: `TreapNode`'s own per-node refcounting (`upper.c`) does *not* need its own `reclaim_gate`, because nothing ever acquires a `TreapNode*` independently the way `upper_acquire` acquires an `UpperSnapshot*` — a reader only ever reaches tree nodes by walking `s->root` after already holding a valid `UpperSnapshot` reference, which transitively keeps the whole tree alive for the reader's purposes; `node_ref`/`node_unref` are called only by the single writer (under `writer_lock`) and by `upper_release`'s cascade (already inside `reclaim_gate`'s protection at the snapshot level), never by a reader racing a writer the way the snapshot pointer itself is raced.
 
@@ -95,6 +95,24 @@ These are hard requirements stated in the spec, not stylistic suggestions — an
 - **RESOLVED: `pack_write`'s compaction-time exact-duplicate elimination (Section 9.2) was O(n²) in entry count, plus a latent correctness bug, both fixed together.** The dedup check was a linear scan of every previously-seen `(hash, size)` pair per entry, comparing only the hash and size — never the actual bytes — before trusting a match, so two different-content entries that happened to collide on `(hash, size)` under FNV-1a64 (explicitly not collision-resistant, `src/hash.c`) would have been silently merged, corrupting one of them; this was never observed in practice but was true of the code as written. `bench/bench.c`'s own compaction benchmark never surfaced either problem because its test files are byte-identical, which made every scan match on the first comparison (the scan's best case, not its worst case). Measuring with unique content instead (`pack_write` called directly, bypassing the journal/fsync path) showed 80,000 entries taking 1.74s, with a 20,000→80,000 (4x N) step showing 15.6x — matching O(n²)'s 16x prediction. Fixed with an open-addressing hash table (`DedupSlot`, load factor 1/2, linear probing) plus a `memcmp` verification before ever reusing a `data_off`, closing both the complexity issue and the correctness gap at once; post-fix, the same 80,000-entry case takes 0.044s (39.6x faster) and the 20,000→80,000 ratio drops to 3.35x, consistent with O(n). `BENCH.md`'s "Resolution #2" has the full before/after numbers. Correctness is covered permanently by `tests/test_pack_overlay.c` (asserts duplicate-content entries share one `data_off` and distinct-content entries do not); the performance fix is covered permanently by `tests/test_pack_write_perf.c` (a regression tripwire against 10,000 unique entries, run as part of `make test`).
 - **NOT FIXED, by deliberate decision: `vfs.c`'s mount table (`MountSnapshot`) is O(n²) in mount count, using the same full-array-copy-per-write pattern the file index used to.** Confirmed via `bench/bench.c` category 10 (500/2,000/8,000 mounts, both 4x-N steps showing 15–20x, matching O(n²)'s 16x prediction, for mount, resolve, and unmount alike). Unlike the file index, this is not being converted to a persistent tree: mount points are created by calls written into a program's own source code, not driven by workload or user data, so realistic mount counts do not reach the scale that made the file index's O(n²) an actual problem — `concept.md` Section 5.3's own stated trigger for needing a persistent structure ("not required until this assumption is empirically violated") has not been violated here the way it was for the file index. `BENCH.md`'s "Finding: mount table scaling" has the full numbers and reasoning; revisit only if a real use case needs thousands of runtime mount points (e.g. one mount per tenant).
 
+## Known reliability characteristics
+
+Section 4.3/4.4's crash-safety claims (self-checking journal records; atomic-rename compaction that leaves the existing pack + journal untouched on failure) were, until now, verified only by static reasoning about the design and by `test_pack_overlay.c`'s test of loading an *already*-corrupted pack file — never by actually killing a process mid-write and checking what a fresh reopen recovers. `tests/test_crash_consistency.c` closes that gap with real fault injection (`fork()` + `SIGKILL` at randomized, empirically-calibrated delays against an actual child process, not a hand-truncated file standing in for a crash):
+
+- **Journal-write crash injection** (25 trials, ~6ms/journaled-write on this build, delays sampled across the whole burst): kills a child mid-way through a sequence of individually-journaled, individually-fsynced writes. Verifies a strict "clean prefix" after reopening — every write the child actually completed is present with exact content, every write after the kill is cleanly absent, no gaps. 20–22 of 25 trials land a genuine interruption (`WIFSIGNALED`, not `WIFEXITED`) each run; **zero corruption or gaps found**.
+- **Compaction crash injection** (40 trials, delays sampled across a calibrated ~0.08s window covering pre-compaction durable writes through post-rename): kills a child mid-`vfs_sync`. Verifies that every write durably journaled *before* `vfs_sync` was even called survives regardless of whether compaction itself completed, and that reopening never fails. Uses an independent progress side-channel (a plain `write()`+`fsync()` on a dedicated file, entirely outside packfs) as ground truth for how many writes the child actually completed before dying — without it, a write the child simply never reached in time is indistinguishable from one that completed and was then lost, which is exactly the bug the test itself had on its first draft (see below). 40/40 trials land a genuine interruption each run; **zero corruption or loss found**.
+
+**A real finding from building this test, recorded here per this file's own "document literally all" standard: the test's first draft reported 128 failures, and every one of them was a bug in the test, not in PackFS.** It checked "is extra write *i* present after the crash" without distinguishing "the child was killed before it ever attempted write *i*" (expected, not a bug) from "write *i* completed and was then lost" (would be a real bug) — SIGKILL cannot be caught, so the child has no way to report its own progress through the normal API. Fixed by adding the independent progress side-channel described above; the corrected test has passed cleanly across every run so far (multiple full runs, plus 3 clean runs under ASan/UBSan). This is the same category of lesson as `BENCH.md`'s own history of findings: a claim (here, "PackFS's crash safety" — there, "the benchmark's numbers") is only as trustworthy as the thing measuring it, and that has to be checked too, not just the subject under test.
+
+**Two real, previously-unknown correctness bugs were found and fixed while investigating the write-failure case those two scenarios don't cover** (disk-full/`ENOSPC`/quota mid-write) — neither was found by a test catching it unprompted; both were found by code review while building `tests/test_journal_failure.c`, then confirmed by direct reproduction before being treated as real:
+
+- **A failed journal write was silently reported as success.** `journal_append_record` and everything that called it (`journal_append_put`/`_delete`/`_mkdir`, `journal_put_current`) used to be `void`, and none of `journal_append_record`'s `fwrite`/`fflush`/`fsync` calls had their return values checked — meaning `vfs_write`/`vfs_mkdir`/`vfs_unlink`/`vfs_rename` on an overlay-backed file all reported `VFS_OK`/a byte count even when the durable journal append actually failed (disk full, quota, an I/O error). This directly contradicted Section 4.4's premise: a caller that saw success had no way to know the write wasn't actually durable, and would only discover the loss on the next crash. Fixed by making the whole journal-append call chain return success/failure and propagating it: the affected `vfs_*` call now returns `VFS_ERR_IO` instead of silently succeeding — even though the in-memory content, already updated by that point, is left as-is and still visible to readers in the same process (the same asymmetry a real `write()`-succeeds-but-later-`fsync()`-fails has; there is no way to "undo" the in-memory update, and the return value's job is to report durability, not roll back visible state).
+- **Fixing the above was not sufficient by itself, and this was confirmed by direct reproduction, not just reasoned about: a partial write leaves a torn record in the middle of the journal file, which poisons every record appended *after* it too, even ones that complete successfully later.** `journal_replay` stops at the first record whose length doesn't fit the remaining file (Section 4.3, by design — this is correct behavior for replay itself). A torn record left behind by a *failed* write sits before every subsequent record, so replay never reaches them, regardless of whether they themselves wrote perfectly fine. Reproduced directly: a forced-failed write followed immediately by a genuinely successful one was unrecoverable after reopening — the good record existed in the file, replay just never got past the torn one in front of it — before this half of the fix. Closed by rolling the journal file back to its exact pre-record byte length whenever a record fails partway (`ftruncate` to a length captured via `ftell` before any of that record's bytes are written), so the on-disk journal stays valid-up-to-EOF even when an individual write fails, not only when every write succeeds.
+
+Both are covered permanently by `tests/test_journal_failure.c`, which forces a real write failure via `RLIMIT_FSIZE` + ignoring `SIGXFSZ` (so `write()` returns `EFBIG` instead of killing the process) — not a mock or a simulated error path — and checks all three properties: the failure is reported (not swallowed), a fresh reopen does not see the torn record as if valid, and a later genuinely-successful write on the same overlay session survives reopening (the exact scenario that was broken before the second half of the fix).
+
+**Still not covered, stated plainly:** cross-process contention (two processes crashing/racing against the same files — not implemented in v0, see "Load-bearing constraints" above), and a crash during journal *truncation* specifically (the second atomic-rename step after a successful compaction, `.jnl.tmp` → `.jnl`) as its own isolated, precisely-targeted case — investigated and deliberately not built as a dedicated test (it would need real concurrency, a second writer thread racing the kill, to reliably land that specific narrow window; the existing compaction-crash scenario above already covers it probabilistically, at whatever rate its delay sampling happens to hit that window, just not by design). TSan remains unable to run for the concurrency side of reliability — now confirmed in *two* independent sandboxed environments (this project's own, and a separate remote cloud sandbox tried specifically to check whether the restriction was environment-specific), both blocking `personality(ADDR_NO_RANDOMIZE)` identically and failing TSan on this project's actual test suite with the identical `FATAL: ThreadSanitizer: unexpected memory mapping` signature — see the sanitizer section above for the workarounds tried in each.
+
 ## Self-evaluation
 
 **Methodology.** This file was checked against the repository's actual state (`make test` passing, including two new tests, `test_pack_write_perf` and the dedup-verification block added to `test_pack_overlay`; `include/`, `src/`, `tests/`, `bench/` present and matching the description below; confirmed by directory listing and a live build), against `concept.md` as frozen (for factual consistency of the constraints and architecture summarized above), and against the documentation standard stated in this file. This revision followed a deliberate audit for other instances of the file index's O(n²) shape elsewhere in the codebase — prompted by a request to leave no caveats undocumented — which found two more real issues, not zero: `pack_write`'s compaction dedup (O(n²), plus a latent hash-collision correctness bug, both now fixed) and the mount table (O(n²), confirmed and deliberately left unfixed, with the reasoning recorded). The audit also found and fixed a CI gap the new tests exposed — the CI config's (now `.gitea/workflows/ci.yml`; at the time of this finding it lived at `.github/workflows/ci.yml`, before this project's scaffolding was set up for Gitea hosting instead of the GitHub-Actions convention it started from — this repository was never actually hosted on GitHub) sanitizer-build steps never passed `-D_GNU_SOURCE` when compiling test files, which was harmless while no test included `internal.h` and became a real link failure once two did (`internal.h` needs it for `pthread_rwlock_t`) — and documented a sandbox flake (ASan/UBSan's own `DEADLYSIGNAL` startup race, distinct from and in addition to the already-documented TSan limitation) observed directly during this audit's own sanitizer runs, in `CONTRIBUTING.md` and cross-referenced here. Every finding below was confirmed empirically (direct `pack_write` timing, `bench/bench.c`'s new mount-scaling category, repeated sanitizer runs with `timeout`), not asserted.

CONTRIBUTING.md

@@ -43,13 +43,22 @@ project's development, see the `reclaim_gate` note in `internal.h`) but do
 not do TSan's happens-before race analysis, so they are not a substitute
 for it. CI (`.gitea/workflows/ci.yml`, Gitea Actions — this project is
 hosted on Gitea, not GitHub) runs on this project's own self-hosted
-runner; whether that runner can execute TSan depends on that runner's own
-environment, which is not controlled by this repository the way a
-hosted-runner VM would be — do not assume CI's TSan step passing means
-what it would on an unrestricted machine without having actually checked
-what environment the registered runner provides. Treat a change as
-TSan-verified only once it has actually passed on a machine confirmed able
-to run it, not merely because a CI step reported success.
+runner. **Confirmed, not just anticipated: that runner cannot run TSan
+either.** The first real CI run on it failed the ThreadSanitizer step with
+the exact same signature described above (`FATAL: ThreadSanitizer:
+unexpected memory mapping`), consistent with the runner's job containers
+using a default seccomp profile that blocks `personality()` the same way
+some local sandboxes do. The CI step now detects this specific failure
+signature and does not fail the build over it (while still failing hard on
+any *other* TSan outcome — a real race, a crash, anything without that
+exact signature) — see the step's own comment in `ci.yml` for the
+detection logic. This means **CI's ThreadSanitizer step passing is not
+evidence that TSan actually ran** on a given push; it may just mean the
+runner couldn't start it and the step correctly didn't treat that as a
+failure. Treat a change as TSan-verified only once it has actually passed
+on a machine confirmed able to run it (a local machine or container with
+`personality(ADDR_NO_RANDOMIZE)` available), not merely because CI is
+green.
 
 **A second, separate environment quirk, also observed directly rather than
 assumed:** in the same kind of sandboxed environment, an ASan/UBSan-built
@@ -64,7 +73,14 @@ all, in another: `test_pack_overlay`), which — together with the fact that
 every one of those binaries passes cleanly on a repeat run — indicates a
 sandbox startup race (plausibly in the same family as the ASLR/mapping
 restrictions behind the TSan limitation above), not a bug in the binary
-being tested. **Always wrap sanitizer-build test runs in `timeout` in such
+being tested. **`tests/test_crash_consistency.c` hits this flake noticeably
+more often than the rest of the suite** (observed 2 failures in 5 runs,
+versus roughly 1-in-5–10 elsewhere) — expected, not a sign anything is
+wrong with that test specifically: it forks 60+ subprocesses per run
+(one per fault-injection trial), and each fork is an independent chance to
+hit the same startup race, so a test that forks this much will trip it
+proportionally more often. Re-run it in isolation before treating a
+failure there as a real finding, same as any other `DEADLYSIGNAL` exit. **Always wrap sanitizer-build test runs in `timeout` in such
 an environment** (e.g. `timeout 30 ./build/san/test_name`) so a stuck run
 fails loudly and bounded instead of hanging; treat a `DEADLYSIGNAL` exit as
 inconclusive, re-run, and only treat the run as a real finding if the output
@@ -75,6 +91,26 @@ response is to say this plainly and re-run until a clean pass is obtained
 (or hand off to CI, which runs on an unrestricted VM where this has not been
 observed), not to suppress or silently ignore a `DEADLYSIGNAL` exit.
 
+**A real gap this project's own process had, found the hard way: local
+sanitizer runs had been using `ASAN_OPTIONS=detect_leaks=0`, which masked
+a genuine memory leak that Gitea CI (which does not set that option) then
+caught.** The override wasn't unreasonable on its own terms — a
+`SIGKILL`ed forked child (as in `tests/test_crash_consistency.c`) never
+runs its own exit-time leak check, so its allocations were never actually
+the concern — but setting it for the *entire* test run also suppressed
+LeakSanitizer for the parent process's own code, where a real bug
+(`tests/test_journal_failure.c` was missing a `vfs_free(v2)` call on its
+normal, expected code path, in two separate reopen blocks) went
+undetected locally across many runs. **Do not add `detect_leaks=0` (or
+any other blanket sanitizer-weakening option) to a local verification
+habit without it also being in `.gitea/workflows/ci.yml`** — if CI and a
+local run check different things, a real finding can pass locally and
+only surface once it reaches CI, which is what happened here. If a fork
++`SIGKILL` test's own allocations genuinely need excluding, scope the
+exclusion narrowly (e.g. a leak-suppression file naming the specific
+allocation site) rather than disabling leak detection for the whole
+binary.
+
 A change to the index structure in `upper.c` (the `TreapNode`/`treap_*`/
 `node_*` functions, `snapshot_upsert`, `snapshot_remove`, or the
 `UpperSnapshot`/`UpperEntry` representation) should be run through

POSTMORTEM.md

@@ -0,0 +1,709 @@
+# PackFS development postmortem
+
+This document is a permanent record of every real issue found during this
+project's development from the initial O(n²) file-index finding through the
+`c414a27` CI fix — what happened, how it was actually diagnosed (including
+false starts and things that looked like fixes but weren't), the final fix,
+how it was verified, and what permanent artifact (a test, a CI change, a
+documentation update) now prevents it from recurring silently. It exists
+because several of the issues below were found only by *re-checking* a claim
+that had already been asserted as true — the purpose of writing this down is
+to make that re-checking unnecessary next time: read this first, not the git
+log, to find out whether something below already covers the question at
+hand.
+
+Every finding here follows the same shape this project's other documents
+already use (`CLAUDE.md`'s "Documentation standard"): a claim is not treated
+as verified until it has been reproduced directly, not merely reasoned
+about, and a revision extends the record rather than silently overwriting an
+earlier, wrong belief. Several entries below are explicitly about a *test*
+or a *fix* being wrong on its first attempt — those are kept, not deleted,
+because the wrong first attempt and why it was wrong is exactly the
+information that prevents the same mistake twice.
+
+Commit hashes below refer to `master` in this repository
+(`retoor.molodetz.nl/retoor/packfs`).
+
+## Contents
+
+1. [Performance findings](#1-performance-findings)
+2. [Environment and tooling limitations](#2-environment-and-tooling-limitations)
+3. [Project professionalization](#3-project-professionalization)
+4. [Git identity correction](#4-git-identity-correction)
+5. [Data-integrity fault injection](#5-data-integrity-fault-injection)
+6. [CI as a second, independent reviewer](#6-ci-as-a-second-independent-reviewer)
+7. [Distilled lessons](#7-distilled-lessons)
+
+---
+
+## 1. Performance findings
+
+### 1.1 File index O(n²) in bulk sequential structural writes (`64283d5`)
+
+**Symptom.** An early `make bench` run showed `mem`/`dir` bulk sequential
+`create`/`unlink`/`mkdir` running 9–22x *slower* than the raw host
+filesystem — the opposite of what an in-memory, in-process VFS should show.
+
+**Diagnosis.** `UpperSnapshot` (`src/upper.c`) was a flat, sorted array;
+every structural write copied the entire array before publishing the next
+immutable snapshot (Section 5.3's single-writer model). At N=20,000 entries,
+each `create` was doing an O(N) copy, making N sequential creates O(N²)
+total. `concept.md` Section 5.3 had already named the exact trigger for
+reconsidering this design ("a persistent (structurally shared) tree
+structure is not required until this assumption is empirically violated")
+— this was that violation, measured rather than hypothesized.
+
+**Fix.** Rewrote the index as a persistent treap (Seidel & Aragon 1996;
+Liljenzin arXiv:1301.3388 for the persistent-treap-as-MVCC-snapshot
+technique specifically) — a structural write now touches only the O(log n)
+nodes on the path to the change, sharing every other node (refcounted) with
+the snapshot it was built from.
+
+**Verification.** `mkdir`-at-N=4,000-vs-`create`-at-N=20,000 (same
+mechanism, 5x the N) showed a 7.15x ratio post-fix versus 25–28.4x pre-fix —
+O(n) predicts 5.0x, O(n log n) predicts 5.97x, the old O(n²) regime
+predicted and measured 25–28.4x. This is the complexity-*class*
+confirmation, not just "it got faster" (a constant-factor optimization could
+also produce a speedup without changing the class). `mem` create/unlink went
+from 9.5–22x slower than raw fs to 15–27x *faster*.
+
+**Regression prevention.** `tests/test_index_stress.c` (thousands of
+randomized, non-sequential creates/deletes/renames, cross-checked against an
+independent reference model — not just "the benchmark ran fast without
+crashing"). `BENCH.md`'s "Resolution" section carries the full before/after
+table and this exact complexity-class methodology, reusable for any future
+index change.
+
+### 1.2 `pack_write` compaction dedup: O(n²) plus a latent correctness bug (`684c6d9`)
+
+**Symptom.** None initially — this was found by deliberately auditing the
+codebase for *other* instances of the O(n²) shape §1.1 had just fixed, not
+by a benchmark regression.
+
+**Diagnosis, first pass (nearly missed).** `bench/bench.c`'s own compaction
+benchmark (category 8) never showed a problem, because its test files are
+all byte-identical — every entry's linear duplicate-scan matched on the very
+first comparison, which is the scan's *best* case, not its worst case. This
+was the actual trap: a benchmark whose own test data accidentally happens to
+avoid an algorithm's worst case will report a clean result for a genuinely
+quadratic function. Re-measuring with unique per-file content instead
+(calling `pack_write` directly, bypassing the journal/fsync path so the
+measurement isn't confounded by this sandbox's separately-documented slow
+`fsync` — see §2.2) surfaced it: 5,000/20,000/80,000 unique entries took
+0.0138s/0.1114s/1.7389s, and the 20,000→80,000 (4x N) step showed 15.61x,
+matching O(n²)'s 16x prediction.
+
+**A second, independent bug found in the same code while reading it to fix
+the first.** The dedup check compared only `(hash, size)` before declaring
+two entries duplicates and sharing their `data_off` — never the actual
+bytes. `src/hash.c`'s FNV-1a64 is explicitly documented as non-cryptographic
+and not collision-resistant. Two different-content entries whose
+`(hash, size)` happened to collide would have been silently merged,
+corrupting one of them. Never observed in practice (no test's data happened
+to collide), but true of the code as written — the kind of bug that stays
+latent until it doesn't.
+
+**Fix.** Replaced the linear scan with an open-addressing hash table
+(`DedupSlot`, load factor 1/2, linear probing) *plus* an actual `memcmp`
+against the candidate's stored data before ever reusing a `data_off` — a
+hash match is now only ever a candidate, never accepted as proof of
+equality. This closes both the complexity issue and the correctness gap
+with one change, since the fix required for one directly implies the fix
+for the other (the hash table alone, without `memcmp`, would still have
+the collision bug; the `memcmp` alone, without a hash table, would still be
+O(n²)).
+
+**Verification.** Post-fix, the same methodology: 80,000 unique entries in
+0.044s (39.6x faster), 20,000→80,000 ratio dropped to 3.35x (O(n) predicts
+4x — close, and nowhere near 16x).
+
+**Regression prevention.** `tests/test_pack_overlay.c` gained a white-box
+assertion (`#include "internal.h"`) that `/a.txt` and `/dup.txt` (identical
+content) share one `data_off` while `/b.txt` (different content) shares
+neither's — this catches *both* "dedup stopped happening" and "dedup
+over-matched," which a single boolean pass/fail on the benchmark could not
+distinguish. `tests/test_pack_write_perf.c` is a standing regression
+tripwire: 10,000 unique entries under a generous time bound, run as part of
+`make test`, so a future accidental reversion to a linear scan fails loudly
+and immediately rather than being rediscovered by a future audit.
+
+### 1.3 Mount table O(n²) — confirmed, deliberately not fixed (`684c6d9`)
+
+**Symptom.** None from user-facing behavior; found by extending the same
+audit that found §1.2, this time to `src/vfs.c`'s mount table.
+
+**Diagnosis.** `MountSnapshot` uses the identical full-array-copy-per-write
+pattern the file index used to (§1.1) — confirmed via a new `bench/bench.c`
+category (500/2,000/8,000 mounts): both 4x-N steps showed 15–20x, matching
+O(n²)'s 16x prediction, for `mount`, `resolve`, *and* `unmount` alike
+(`resolve` being O(n²) too was itself a finding — longest-prefix-match
+dispatch is a linear scan of the mount table per call, O(n) per call, O(n²)
+across N calls).
+
+**Decision: not fixed.** Unlike the file index, mount points are created by
+calls written into a program's own source code — bounded by how many lines
+of "mount this backend at this path" a person or build script is willing to
+write, not by user data or workload size. `concept.md` Section 5.3's own
+stated trigger for needing a persistent structure ("not required until this
+assumption is empirically violated") has not been violated here the way it
+was for the file index, and converting the mount table anyway would add
+real, permanent complexity for a case that doesn't occur in practice.
+
+**Regression prevention.** `BENCH.md`'s "Finding: mount table scaling"
+section and `CLAUDE.md`'s "Known performance characteristics" record the
+measured numbers and the reasoning, explicitly labeled "NOT FIXED, by
+deliberate decision" — so a future reader sees a considered decision, not
+an unexamined gap, and doesn't need to re-derive whether this is worth
+fixing from scratch.
+
+---
+
+## 2. Environment and tooling limitations
+
+### 2.1 ThreadSanitizer cannot run in this sandbox — confirmed in two independent environments
+
+**Symptom.** Every `-fsanitize=thread` build fails identically:
+`FATAL: ThreadSanitizer: unexpected memory mapping`.
+
+**Diagnosis.** TSan needs `personality(ADDR_NO_RANDOMIZE)` to disable ASLR
+for itself before it can set up its shadow-memory layout. Confirmed via a
+direct raw syscall (Python `ctypes`, `libc.personality(0x0040000)`) that
+this returns `-1`/`errno=1` (`EPERM`) — a hard, syscall-level refusal, not a
+PackFS-specific symptom (a trivial, unrelated two-thread pthread program
+fails identically).
+
+**Attempts that did *not* fix it, tried and ruled out rather than assumed
+unhelpful:**
+- `setarch $(uname -m) -R ./binary` — `setarch` just calls the same
+  `personality()` syscall internally; fails identically
+  (`Operation not permitted`).
+- Compiling `-no-pie -fno-pie` — theorized that a non-PIE binary's fixed
+  load address might sidestep the need for ASLR-disabling, since ASLR
+  mostly affects PIE base-address randomization. Tested directly: still
+  fails identically. (TSan's shadow-memory requirement is about the whole
+  process's mmap layout — heap, libraries, stack — not just the main
+  executable's own base address, so this was never going to work; worth
+  recording precisely *why* it doesn't, so it isn't tried again on the
+  same mistaken theory.)
+- Searching `TSAN_OPTIONS=help=1` for a flag that relaxes the startup
+  memory-mapping check — none exists.
+- Checking for a privilege-escalation path: `/proc/self/status` shows
+  `CapEff` all-zero (no effective capabilities) under an active seccomp
+  filter (`Seccomp_filters: 1`) that *also* blocks `unshare --user`
+  (`Operation not permitted`) — ruling out running TSan inside a nested,
+  less-restricted namespace.
+- Checking whether a genuinely different infrastructure provider has the
+  same restriction, via a dedicated remote-cloud-sandbox agent (not just
+  re-testing the same local machine): identical result —
+  `personality()` returns `EPERM`, a trivial pthread program fails TSan
+  identically, and all of this project's own test binaries fail with the
+  same `FATAL: ThreadSanitizer: unexpected memory mapping` signature.
+
+**Conclusion.** This is a categorical, syscall-level restriction with no
+userspace workaround available, confirmed in two independent sandboxed
+environments, not a configuration this project has simply failed to find
+yet. **As of this writing, TSan has not completed a single run in any
+environment this project has actually been built in** — this is stated
+plainly in `README.md`, `CONTRIBUTING.md`, and `CLAUDE.md` rather than left
+implied by CI showing green (see §2.2's segfault variant, and §6.2, for why
+a green TSan CI step specifically is *not* evidence TSan ran).
+
+**Regression prevention.** `CONTRIBUTING.md`'s sanitizer-testing section and
+`CLAUDE.md`'s equivalent paragraph state this explicitly, including the
+exact confirming tests, so a future session doesn't have to re-derive "is
+this fixable" from scratch — it's already been checked, thoroughly, and the
+answer is recorded along with the checking.
+
+### 2.2 ASan/UBSan sandbox-startup flake, two variants
+
+**Variant A: clean, bounded.** A sanitizer-built binary occasionally
+(non-deterministically) fails to start, printing `AddressSanitizer:
+DEADLYSIGNAL` once and exiting nonzero. Confirmed as a sandbox race, not a
+PackFS bug, by it hitting *different, unrelated* binaries across repeated
+runs (in one session: `test_dir` and `test_mem`; in another:
+`test_pack_overlay`; in another: nothing at all), with every affected binary
+passing cleanly on a repeat run.
+
+**Variant B: unbounded.** The same underlying race can instead manifest as
+an *unbounded repeating loop* of the same `AddressSanitizer:DEADLYSIGNAL`
+line — observed directly consuming CPU/memory for minutes and, in one
+extreme case during this project's own CI-script debugging (§6.2), writing
+enough output within a 30-second `timeout` window to reproduce as a
+264MB+ log.
+
+**Mitigation, not elimination:** always wrap sanitizer-build test runs in
+`timeout` (bounds variant B's wall-clock cost); treat a bare
+`DEADLYSIGNAL` exit as inconclusive and re-run; only treat it as a real
+finding if the output actually contains `ERROR: AddressSanitizer` or
+`runtime error:`. `tests/test_crash_consistency.c` hits this flake
+noticeably more often than the rest of the suite (it forks 60+ subprocesses
+per run — each fork is an independent chance to hit the same startup race)
+— this is expected, not a sign specific to that test, and is called out in
+`CONTRIBUTING.md` so a future reader doesn't misdiagnose it as a
+regression in that file.
+
+**Regression prevention.** `CONTRIBUTING.md` documents both variants, the
+required `timeout` mitigation, and the exact string check that separates a
+real finding from this flake. §6.2 covers a further, serious follow-on
+mistake made while automating this exact mitigation in CI — see there for
+why "wrap it in timeout" was necessary but not, on its own, sufficient.
+
+---
+
+## 3. Project professionalization
+
+Prompted by "do literally everything for this project to be taken
+seriously." A survey of what a serious C library project needs, each
+verified rather than assumed present:
+
+- **`SPDX-License-Identifier: MIT`** added to every file under `src/` and
+  `include/` (`include/packfs.h` already had one; the `src/*.c` files and
+  `src/internal.h` did not).
+- **Version API**: `PACKFS_VERSION_MAJOR`/`MINOR`/`PATCH`/`STRING` in
+  `include/packfs.h`, a runtime `pfs_version()` (`src/vfs.c`, next to
+  `vfs_new`/`vfs_free`). Verified with a new assertion in
+  `tests/test_mem.c` that the compile-time macro and the runtime function
+  never disagree — a version API that can silently drift between its two
+  forms is worse than not having one.
+- **`packfs.pc`** (pkg-config), generated by `make install` from a new
+  `packfs.pc.in`, with its `Version:` field derived from
+  `PACKFS_VERSION_STRING` via a Makefile-level `grep`/`sed` rather than
+  hand-maintained separately. Verified end-to-end with a scratch
+  `make install PREFIX=...` followed by an actual
+  `pkg-config --cflags --libs packfs` call and `make uninstall` — not just
+  by reading the Makefile rule.
+- **`SECURITY.md`**: states precisely what this project's containment and
+  pack-integrity code actually claims as a security boundary (`concept.md`
+  Section 6/7) versus what it explicitly does not (unenforced `mode`, no
+  cross-process concurrency) — including, after a later audit (§5), that
+  the pack integrity checksum (FNV-1a64) is non-cryptographic and not
+  tamper-evident against a deliberate adversary, a fact that was already
+  documented in the context of §1.2's dedup bug but had never been
+  explicitly connected to its *other* use, load-time integrity validation,
+  where it matters more.
+- **`CHANGELOG.md`** (Keep a Changelog format), built from the real git
+  history — corrected once already (see the `a914330` commit) when a prior
+  commit landed *after* the `v0.1.0` tag without ever getting its own
+  entry, leaving the changelog stale the moment it happened. The fix:
+  recognize that "I tagged a release" is not the same event as "I stopped
+  needing to update the changelog," and check for this specifically after
+  any tag.
+- **Considered and explicitly declined**: a `CODE_OF_CONDUCT.md`, per the
+  user's own choice when asked — recorded here so a future session doesn't
+  re-propose it as an oversight.
+- **Gitea, not GitHub**: on explicit direction, `.github/workflows/ci.yml`
+  moved to `.gitea/workflows/ci.yml` (Gitea Actions' convention), and every
+  doc that assumed GitHub-specific features was corrected — most notably
+  `SECURITY.md`, which had claimed "private security advisories" would be
+  available once hosted, a GitHub feature this project's actual host
+  (Gitea) was never confirmed to have; reporting is by direct email only.
+  `README.md`'s claim that the suite is "regularly run under
+  ThreadSanitizer" was also corrected at this point to reflect §2.1's
+  actual, confirmed status, rather than left as an aspirational statement
+  a reader could mistake for a tested one.
+
+**Regression prevention.** All of the above are either self-verifying
+(the version API test, the pkg-config round-trip) or are stated as
+documentation with an explicit "verified by X" attached, per this
+project's own documentation standard — the standard itself is the
+regression-prevention mechanism here: a claim without a verification note
+is treated as suspect on sight.
+
+---
+
+## 4. Git identity correction
+
+**Symptom.** None from the code; a direct user request ("My name is
+literary nowhere to find in the whole log anymore, not even in met a with
+grep?") revealed the actual bug: every commit's author/committer showed the
+user's real name, because commits had been made under whatever local git
+config was already set on the machine, without ever asking what identity
+this project should use — despite the user having already established
+`SECURITY.md`'s contact as "retoor."
+
+**Fix, first pass.** `git filter-branch --env-filter` rewrote every
+commit's author *and* committer across all 11 commits at the time, since
+nothing had been pushed to any remote yet (confirmed via `git remote -v`
+returning empty first) — making this a safe, local-only rewrite, not a
+history rewrite against shared state.
+
+**A real gap found only by verifying the "fixed" state exhaustively,
+not by re-checking `git log`.** `git log --format='%an <%ae>'` showed only
+the corrected identity — but a full object-database sweep (`git
+cat-file --batch-all-objects`, dumping and `grep`-ing every commit, tree,
+blob, *and tag* object, not just what `git log` surfaces) found the
+annotated `v0.1.0` tag object's own `tagger` field still showed the real
+name. `git filter-branch`'s tag-rewriting step recreates the tag object
+pointing at the new commit hash but does not apply the `--env-filter` to
+the tag object's own tagger line — a genuinely separate object type with
+its own separate metadata field, easy to miss if verification stops at
+"does `git log` look right."
+
+**Fix, second pass.** Deleted and recreated the `v0.1.0` tag (with the
+local git identity already corrected by that point), producing a fresh
+tag object with the correct tagger.
+
+**Verification, exhaustive rather than spot-checked:** dumped and grepped
+the content of all ~126 packed objects (case-insensitively, for the name,
+the surname alone, and the email) — zero matches; ran
+`git fsck --unreachable --dangling` after `gc --prune=now` — nothing left
+over from the rewrite; checked `.git/config`, `.git/packed-refs`, both
+reflog files' actual content (not just `git reflog show`), notes, and
+stash — all clean; raw byte-level `grep -a -r -i` across the entire
+`.git` directory — zero matches.
+
+**Regression prevention.** None needed as an ongoing mechanism (this was a
+one-time correction, not a recurring class of bug) — but the *methodology*
+is worth keeping: `git log` alone is not a complete verification of "is
+this identity anywhere in this repository," because it doesn't surface tag
+objects, reflogs, or dangling objects. A full object-database sweep is the
+actual bar for "confirmed gone," and is cheap enough (a few seconds for a
+repository this size) to just do rather than trust a partial check.
+
+---
+
+## 5. Data-integrity fault injection
+
+Prompted by a direct question ("How safe is packfs to use for data
+integrity?") that got an honest answer identifying two real, then-open
+gaps: no fault-injection testing of the documented crash-safety claims, and
+TSan having never actually verified the concurrency-safety claims (§2.1).
+The user's follow-up ("Do whatever you need to do to make you trust it")
+turned this from a documentation exercise into real engineering work.
+
+### 5.1 `tests/test_crash_consistency.c`: real `fork()`+`SIGKILL` fault injection (`e41bd23`)
+
+**What it does.** Forks a real child process, lets it run for an
+empirically-calibrated delay (measured via throwaway scripts against this
+build, not guessed — e.g. ~6ms per individually-journaled write, ~12ms for
+a 4,000-entry/2KB-payload compaction), then `SIGKILL`s it and checks what a
+fresh reopen recovers. Two scenarios: a burst of individually-journaled
+writes (25 trials), and a compaction (`vfs_sync`) call (40 trials).
+
+**A real bug in the test itself, found before trusting its results.** The
+first draft's compaction scenario reported 128 failures. Every one was a
+bug in the test's own oracle, not in PackFS: it checked "is extra write *i*
+present after the crash" without distinguishing "the child was killed
+before it ever attempted write *i*" (expected, not a bug) from "write *i*
+completed and was then lost" (would be a real bug) — `SIGKILL` cannot be
+caught, so the child has no way to report its own progress through the
+normal API once it's been killed. Fixed by adding an independent progress
+side-channel: a plain POSIX `write()`+`fsync()` on a dedicated file,
+entirely outside packfs, recording "N extras durably completed" after each
+one — giving the test's own verification step ground truth for how far the
+child actually got, independent of (and not trusting) the thing being
+tested.
+
+**Result, after the test's own bug was fixed.** 20–22/25 burst trials and
+40/40 compaction trials land a genuine interruption (`WIFSIGNALED`, not
+`WIFEXITED`) per run; zero corruption or gaps found across dozens of runs,
+including under ASan/UBSan.
+
+**Regression prevention.** This test itself, run as part of `make test` and
+CI on every push.
+
+### 5.2 Silent journal-write failure — a real, previously-unknown data-loss bug (`153d44e`)
+
+**How it was found.** Not by a test — by reading `src/overlay.c`'s journal
+code while building a fault-injection test for the "disk full mid-write"
+case §5.1 didn't cover.
+
+**The bug.** `journal_append_record` and everything that called it
+(`journal_append_put`/`_delete`/`_mkdir`, `journal_put_current`) were
+`void`, and none of `journal_append_record`'s `fwrite`/`fflush`/`fsync`
+calls had their return values checked. A real write failure (disk full,
+quota, an I/O error) was silently reported as success by
+`vfs_write`/`vfs_mkdir`/`vfs_unlink`/`vfs_rename` on an overlay-backed
+file — directly contradicting Section 4.4's premise that a successful
+journal append means the write is durable.
+
+**Fix.** Made the whole call chain return and propagate success/failure;
+the affected `vfs_*` call now returns `VFS_ERR_IO` instead of silently
+succeeding — while leaving the already-applied in-memory change as-is
+(readers in the same process still see it), the same asymmetry a real
+`write()`-succeeds-but-a-later-`fsync()`-fails has. There is no way to
+"undo" the in-memory update, and the return value's job is to report
+durability, not roll back visible state.
+
+**A second, deeper bug found only by reproducing the first fix's edge
+case, not by reasoning about it.** Fixing the silent-failure bug alone was
+not enough: a partial write leaves a torn record sitting in the middle of
+the journal file, and `journal_replay` correctly stops at the first record
+it can't fully read (Section 4.3, by design). A torn record left behind by
+a *failed* write therefore poisons every record appended *after* it too —
+including ones that themselves complete successfully later. Reproduced
+directly before fixing: a forced-failed write followed immediately by a
+genuinely successful one was unrecoverable after reopening — the good
+record existed in the file, replay just never got past the torn one sitting
+in front of it.
+
+**Fix.** Roll the journal file back to its exact pre-record byte length
+(`ftell` captured before any of the record's bytes are written, `ftruncate`
+on failure) whenever a record fails partway, keeping "the journal on disk
+is valid up to EOF" true even when an individual write fails.
+
+**A third bug, found by inspection while in the same code.**
+`journal_put_current` used to pass a `NULL` buffer into
+`journal_append_record`'s `memcpy` of a nonzero size when `malloc(size)`
+failed — an OOM-triggered NULL-pointer dereference. Closed with an
+explicit `if (size && !buf) return -1;` guard. Not test-triggered
+(reliably forcing `malloc()` failure in a portable, safe way isn't
+practical here) — verified by code inspection and stated as such, not
+claimed as tested.
+
+**Regression prevention.** `tests/test_journal_failure.c`, which forces a
+*real* write failure via `RLIMIT_FSIZE` + ignoring `SIGXFSZ` (so `write()`
+returns `EFBIG` instead of killing the process) rather than a mock, and
+checks all three properties in one place: the failure is reported (not
+swallowed), a fresh reopen does not see the torn record as if valid, and a
+later genuinely-successful write on the same overlay session survives
+reopening (the exact scenario that was broken before the rollback fix).
+
+---
+
+## 6. CI as a second, independent reviewer
+
+Two real bugs were found not by local testing but by Gitea CI running the
+exact same code under a genuinely different invocation — worth recording
+as its own category, because both were invisible locally for structural
+reasons, not bad luck.
+
+### 6.1 A real memory leak, masked by a local sanitizer habit (`4aad7d6`)
+
+**What CI reported.** `LeakSanitizer: detected memory leaks` — 256 bytes
+across 4 allocations, from `vfs_new`/`vfs_unmount` call sites.
+
+**Why local testing never caught it.** Local ASan/UBSan verification had
+been using `ASAN_OPTIONS=detect_leaks=0`. The reasoning behind that
+override was sound on its own terms: a `SIGKILL`ed forked child (§5.1)
+never runs its own exit-time leak check, so its allocations were never the
+actual concern. The mistake was applying it to the *entire* test binary's
+run, not just scoping it to the child's own allocations — this also
+suppressed LeakSanitizer for the *parent* process's own code, which is
+exactly where the real bug was sitting.
+
+**The actual bug.** `tests/test_journal_failure.c`'s two "reopen after the
+failure, verify recovery" blocks called
+`vfs_unmount`/`backend_free`/`backend_free` inside the `if (ov2)` branch
+(the normal, expected path, always taken since the code's own `CHECK`
+asserts `ov2 != NULL`) but `vfs_free(v2)` only on the `else` branch, which
+in practice is never reached.
+
+**Fix.** Moved `vfs_free(v2)` to run unconditionally after the `if`, in
+both blocks.
+
+**Verification.** Reproduced the leak first — dropped the
+`detect_leaks=0` override locally, matching CI's actual invocation exactly,
+*before* touching any code — then confirmed the fix the same way: all 8
+test binaries clean under ASan/UBSan with leak detection *on*, run
+multiple times.
+
+**Regression prevention.** `CONTRIBUTING.md` now states this gap
+explicitly: do not add `detect_leaks=0` (or any other blanket
+sanitizer-weakening option) to a local verification habit without it also
+being in `.gitea/workflows/ci.yml` — if local and CI check different
+things, a real finding can pass locally and only surface once it reaches
+CI, which is exactly what happened here.
+
+### 6.2 The `set -e` bug: CI's own script was the actual root cause of a reported failure (`c414a27`)
+
+This is the most involved finding in this document and the one most worth
+reading in full before touching `.gitea/workflows/ci.yml` again.
+
+**What was reported.** Gitea CI's "Build and run under ThreadSanitizer"
+step failing with `exitcode '66': failure`, after the build and test-suite
+steps passed. 66 is TSan's own raw exit code for the `FATAL:
+ThreadSanitizer: unexpected memory mapping` failure (§2.1) — the step's own
+classification logic (added specifically to *not* fail the build over that
+known, environment-caused failure) was supposed to catch this and turn it
+into a warning, not a build failure.
+
+**Root cause, confirmed by direct reproduction, not theorized.** The
+step's script assigned captured output via a bare, unwrapped
+`OUT=$("./binary" 2>&1)`. Gitea Actions' `run:` steps execute under `bash
+--noprofile --norc -eo pipefail {0}` — `set -e` is on by default. Under
+`-e`, a command substitution's own nonzero exit status counts as that
+simple command's failure, which aborts the whole script *immediately* —
+*before* the very next line, even one that only reads `$?`, ever runs.
+Reproduced directly with a two-line script:
+```sh
+set -e
+OUT=$(false)
+RC=$?
+echo "after"   # never printed
+```
+This meant the classification logic a few lines further down the real
+script never executed at all: the first TSan binary to hit the FATAL/
+exit-66 case (all of them, on this runner) killed the step outright with
+that raw exit code, exactly the outcome the classification logic existed
+to prevent.
+
+**Fix.** Wrap the invocation in `if cmd; then RC=0; else RC=$?; fi` —
+bash's `-e` rules specifically exempt a command used as an `if` condition
+from triggering an abort. Reproduced the fix working, the same way:
+```sh
+set -e
+if OUT=$(false); then RC=0; else RC=$?; fi
+echo "after, RC=$RC"   # prints: after, RC=1
+```
+
+**A follow-on bug, found only while verifying the fix under real
+conditions, not assumed absent.** With the `-e` bug fixed, re-running the
+corrected script repeatedly to verify it eventually hit §2.2 Variant B (the
+flake's unbounded-loop form) — and when it did, the classification logic
+misbehaved: an error line reported `(exit 0)`, which should have been
+impossible. Root cause: capturing hundreds of megabytes of repeated
+`AddressSanitizer:DEADLYSIGNAL` lines into a bash variable via
+`OUT=$(cmd)` is not just slow — at that scale, the shell's own
+string-handling stopped behaving reliably enough to trust the
+classification built on top of it. **Fixed by redirecting output straight
+to a file instead of a shell variable, and reading back only a bounded
+64 KiB prefix** for both classification and logging (chosen generously —
+this project's own real ASan/UBSan failure reports have historically been
+a few dozen lines, nowhere near 64 KiB, while the flake's worst case was
+hundreds of megabytes). This was applied to *both* the ASan/UBSan and TSan
+steps, since both had the same fragility once genuinely stressed.
+
+**A mistake made while applying that exact fix, caught by testing it, not
+by review.** The first attempt at the file-redirect fix wrote
+`timeout 30 "./binary" > "$LOG" 2>&1` as a bare statement — reintroducing
+the *exact same* `-e`-abort bug the whole exercise started from, just in a
+new shape (redirecting to a file doesn't change whether the command itself
+is subject to `-e`'s abort rule; only wrapping it in a tested context
+does). Caught immediately by re-running the same two-line reproduction
+technique against the new form before trusting it, not by inspection:
+```sh
+set -e
+timeout 5 false > /tmp/out.log 2>&1
+RC=$?
+echo "after"   # never printed -- same bug, new location
+```
+Fixed the same way as the first bug: `if timeout 30 "./binary" > "$LOG"
+2>&1; then RC=0; else RC=$?; fi`.
+
+**A second, previously undocumented flake variant, discovered while
+repeatedly reproducing the above.** TSan's broken startup on this runner
+does not always print the clean `FATAL: ThreadSanitizer: unexpected memory
+mapping` message — it sometimes segfaults outright instead (`timeout`
+reports "the monitored command dumped core"). Confirmed this is the *same*
+environmental cause, not a bug in any specific test file, by watching it
+hit three different, unrelated binaries across repeated full-suite runs
+(`test_journal_failure` in one run; `test_crash_consistency` and `test_dir`
+together in another) — a real bug in one file's own code would not migrate
+between files at random like that. The TSan step's classification now also
+recognizes this variant: a log containing only `timeout`'s own "dumped
+core" notice and nothing else (no program output, no real
+`WARNING`/`SUMMARY: ThreadSanitizer:` race report) is treated the same as
+the clean FATAL-message case.
+
+**A retry-budget gap, found by observing real failures during this same
+verification work, not estimated in advance.** The ASan/UBSan step retries
+a binary up to a fixed count before giving up; with a budget of 3, a real
+run during this exact debugging session hit the DEADLYSIGNAL flake three
+times in a row on the same binary purely by chance, exhausting the budget
+and failing the step even though every individual attempt was correctly
+classified as the known flake, not a real finding. This sandbox's actual
+flake rate is evidently higher in practice than the "roughly 1 in 5–10"
+`CONTRIBUTING.md` documents elsewhere. Fixed by raising the budget to 5,
+which reduces (does not eliminate — this is a probabilistic mitigation,
+not a fix for the underlying flake) the chance of exhausting it on bad luck
+alone.
+
+**Verification, cumulative, all under `bash -eo pipefail` locally
+(matching Gitea Actions' actual shell invocation) rather than assumed
+correct from reading the diff:**
+- The original `-e` bug: reproduced and fixed, confirmed with the two-line
+  repro above.
+- The TSan step: re-run 10 full-suite times (80 individual binary
+  executions) after the fixes above, with both flake variants recurring
+  naturally and both correctly classified as warnings, zero false
+  failures.
+- The ASan/UBSan step: re-run 10 full-suite times with the corrected
+  bounded-file logic and the 5-attempt budget, zero false failures.
+- A separate, unrelated `-Wunused-result` warning on an intentionally-
+  ignored `write()` return value in `tests/test_crash_consistency.c`
+  (§5.1's progress side-channel) was *also* flagged by this same CI run.
+  The `(void)` cast used to silence it built warning-free locally but
+  still warned on the Gitea runner's gcc — reproduced clean locally with
+  the exact same compiler flags first, confirming this is a real
+  toolchain version/configuration difference between the two machines,
+  not a local misconfiguration on either side. `(void)`-cast suppression
+  of `warn_unused_result` is documented as unreliable across gcc
+  configurations for exactly this reason; fixed with an actual
+  conditional branch on the return value (`if (write(...) < 0) { }`)
+  instead, which every gcc/clang version this project has been built with
+  honors.
+
+**Regression prevention.** `.gitea/workflows/ci.yml` itself now carries
+inline comments at each fixed site explaining exactly what would break and
+why, so a future edit to these scripts doesn't reintroduce the same class
+of bug without at least being warned by the comment sitting right there.
+This document is the second layer: read §6.2 in full before touching the
+sanitizer steps' shell scripts again, and re-run the exact two-line
+`set -e` reproduction technique above against any new command-substitution
+pattern before trusting it — that technique, cheap and fast, is what
+caught every bug in this section, including the one introduced while fixing
+the previous one.
+
+---
+
+## 7. Distilled lessons
+
+These are the patterns that actually caught something above, stated once
+here rather than only implicitly in each entry, so they can be applied to
+a *new* problem, not just recognized in hindsight on this one:
+
+1. **A benchmark or test's own data can accidentally avoid the exact case
+   being measured.** §1.2 was invisible because the benchmark's identical
+   test-file content made a linear scan's worst case never happen. When
+   auditing for a known bug shape elsewhere, deliberately construct the
+   adversarial input, don't just re-run the existing benchmark and trust a
+   clean result.
+2. **A fault-injection test's own oracle can be wrong, and has to be
+   checked with the same rigor as the code it's testing.** §5.1's first
+   128 "failures" were a bug in the test. The fix (an independent
+   ground-truth side-channel, entirely outside the system under test) is
+   the general pattern: when the thing being tested is also the thing
+   reporting whether it worked, the report can't be trusted.
+3. **Fixing one bug in a code path can leave a nearby, related bug
+   unfixed, discoverable only by reproducing the fix's own edge cases.**
+   §5.2's torn-record-poisoning-later-records bug was found only by
+   confirming the *first* fix actually solved the whole problem, not by
+   inspecting the diff and assuming it did.
+4. **A local verification shortcut that diverges from what CI actually
+   runs can hide a real bug indefinitely.** §6.1: `detect_leaks=0` was
+   reasonable for one specific reason, applied too broadly, and CI (which
+   didn't share the shortcut) caught what dozens of local runs couldn't.
+5. **`bash -e` does not do what it looks like it does around command
+   substitution.** §6.2, hit twice in the same debugging session (once as
+   the original bug, once as a mistake made while fixing it): `OUT=$(cmd)`
+   or `cmd > file` as a bare statement aborts the whole script on failure,
+   even one line before a line that reads `$?`. The fix is always
+   `if cmd; then ...; else RC=$?; fi`. Test this specific pattern with the
+   two-line reproduction in §6.2 before trusting any new script that
+   relies on inspecting a command's exit code under `set -e`.
+6. **Capturing genuinely unbounded output into a shell variable is not
+   just a performance concern — it can make downstream logic behave
+   incorrectly at scale**, not just slowly. §6.2's second bug. Redirect to
+   a file and bound what's ever read back.
+7. **A crash's signature is not necessarily unique — the same underlying
+   cause can manifest in more than one way, and the way to tell "same
+   cause, different symptom" from "different bug entirely" is to check
+   whether it's tied to specific code or migrates across unrelated
+   files/binaries at random.** §6.2's segfault variant of the already-known
+   TSan-can't-start issue was confirmed this way, not assumed.
+8. **A compiler-warning suppression that works locally is not guaranteed
+   to work on a different toolchain build, even nominally "the same
+   compiler."** §6.2's `(void)`-cast case. Prefer suppressions that are
+   specified to work (an actual branch on the value) over idioms that
+   merely happen to compile clean once.
+9. **Verifying "is this identity/string anywhere in this repository" means
+   more than `git log`.** §4: tag objects, reflogs, and dangling objects
+   all needed their own explicit check; a full object-database sweep is
+   cheap enough to just do.
+10. **When a claim can be reproduced directly, reproduce it — don't reason
+    about whether it's true.** Every fix in this document was confirmed by
+    actually re-running the failing case under the same conditions that
+    produced the failure (the same shell mode, the same compiler flags,
+    the same sandbox), not by reading the diff and concluding it must now
+    be correct. This is the one pattern underlying all the others above.

README.md

@@ -103,74 +103,100 @@ in `BENCH.md`'s "After" table, on the same environment described there. Not
 a replacement for `BENCH.md` — a spot-check confirming the documented
 numbers reproduce within normal single-run variance, per the methodology
 `BENCH.md` itself states ("illustrative of shape... not precise absolute
-figures").
+figures"). Every column from the raw `make bench` output is kept for both
+runs — time, throughput, and MB/s where the category reports one — not
+just a time delta, so every category (including the `pack` rows:
+compaction and random-access read) can be checked in full, not summarized
+away.
 
-| Category | Backend | Documented (BENCH.md) | New run | Delta |
-|---|---|---|---|---|
-| create 20,000 files | mem | 0.0400s | 0.0394s | -1.5% |
-| create 20,000 files | raw fs | 0.9943s | 1.0171s | +2.3% |
-| create 20,000 files | raw+fsync | 116.2147s | 116.7567s | +0.5% |
-| read 20,000 files | mem | 0.0099s | 0.0091s | -8.1% |
-| read 20,000 files | raw fs | 0.1606s | 0.1634s | +1.7% |
-| stat 20,000 files | mem | 0.0077s | 0.0078s | +1.3% |
-| stat 20,000 files | raw fs | 0.0699s | 0.0727s | +4.0% |
-| readdir (20,000 entries) | mem | 0.0041s | 0.0043s | +4.9% |
-| readdir (20,000 entries) | raw fs | 0.0069s | 0.0072s | +4.3% |
-| create 20,000 files | dir | 1.1418s | 1.2030s | +5.4% |
-| read 20,000 files | dir | 0.1530s | 0.1715s | +12.1% |
-| stat 20,000 files | dir | 0.1379s | 0.1546s | +12.1% |
-| readdir (20,000 entries) | dir | 0.0037s | 0.0041s | +10.8% |
-| unlink 20,000 files | mem | 0.0179s | 0.0182s | +1.7% |
-| unlink 20,000 files | dir | 0.4911s | 0.5978s | +21.7% |
-| unlink 20,000 files | raw fs | 0.4746s | 0.5062s | +6.7% |
-| mkdir 4,000 dirs | mem | 0.0056s | 0.0050s | -10.7% |
-| rmdir 4,000 dirs | mem | 0.0040s | 0.0039s | -2.5% |
-| mkdir 4,000 dirs | dir | 0.1710s | 0.1864s | +9.0% |
-| rmdir 4,000 dirs | dir | 0.1257s | 0.1169s | -7.0% |
-| mkdir 4,000 dirs | raw fs | 0.1374s | 0.1444s | +5.1% |
-| rmdir 4,000 dirs | raw fs | 0.0951s | 0.1005s | +5.7% |
-| write 1MB | mem | 0.0001s | 0.0001s | +0.0% |
-| read 1MB | mem | 0.0000s | 0.0000s | +0.0% |
-| write 16MB | mem | 0.0110s | 0.0110s | +0.0% |
-| read 16MB | mem | 0.0009s | 0.0010s | +11.1% |
-| write 64MB | mem | 0.0586s | 0.0621s | +6.0% |
-| read 64MB | mem | 0.0032s | 0.0036s | +12.5% |
-| write 1MB | raw fs | 0.0004s | 0.0004s | +0.0% |
-| read 1MB | raw fs | 0.0001s | 0.0001s | +0.0% |
-| write 16MB | raw fs | 0.0044s | 0.0047s | +6.8% |
-| read 16MB | raw fs | 0.0010s | 0.0012s | +20.0% |
-| write 64MB | raw fs | 0.0186s | 0.0189s | +1.6% |
-| read 64MB | raw fs | 0.0047s | 0.0050s | +6.4% |
-| write 1MB | raw+fsync | 0.0180s | 0.0340s | +88.9% |
-| read 1MB | raw+fsync | 0.0001s | 0.0001s | +0.0% |
-| write 16MB | raw+fsync | 0.0231s | 0.0242s | +4.8% |
-| read 16MB | raw+fsync | 0.0012s | 0.0012s | +0.0% |
-| write 64MB | raw+fsync | 0.0803s | 0.0835s | +4.0% |
-| read 64MB | raw+fsync | 0.0048s | 0.0049s | +2.1% |
-| compact 20,000 entries to pack | pack | 0.0267s | 0.0280s | +4.9% |
-| random-read 20,000 entries | pack (mmap'd) | 0.0082s | 0.0085s | +3.7% |
-| random-read 20,000 entries | raw fs | 0.1629s | 0.1912s | +17.4% |
-| concurrent create+read+unlink (8×4,000×3) | mem | 0.2750s | 0.2791s | +1.5% |
-| concurrent create+read+unlink (8×4,000×3) | raw fs | 5.6498s | 6.1583s | +9.0% |
-| mount 500 backends | vfs | 0.0054s | 0.0060s | +11.1% |
-| resolve, 500 mounts | vfs | 0.0020s | 0.0022s | +10.0% |
-| unmount 500 backends | vfs | 0.0046s | 0.0046s | +0.0% |
-| mount 2,000 backends | vfs | 0.0831s | 0.0863s | +3.9% |
-| resolve, 2,000 mounts | vfs | 0.0296s | 0.0329s | +11.1% |
-| unmount 2,000 backends | vfs | 0.0758s | 0.0780s | +2.9% |
-| mount 8,000 backends | vfs | 1.4739s | 1.5380s | +4.3% |
-| resolve, 8,000 mounts | vfs | 0.4938s | 0.5298s | +7.3% |
-| unmount 8,000 backends | vfs | 1.5172s | 1.5191s | +0.1% |
+| Category | Backend | Doc Time | Doc Throughput | Doc MB/s | New Time | New Throughput | New MB/s | Δ Time |
+|---|---|---|---|---|---|---|---|---|
+| create 20,000 files | mem | 0.0400s | 499,809 ops/s | 61.0 | 0.0373s | 536,419 ops/s | 65.5 | -6.8% |
+| create 20,000 files | raw fs | 0.9943s | 20,114 ops/s | 2.5 | 0.9883s | 20,237 ops/s | 2.5 | -0.6% |
+| create 20,000 files | raw+fsync | 116.2147s | 172 ops/s | 0.0 | 116.3080s | 172 ops/s | 0.0 | +0.1% |
+| read 20,000 files | mem | 0.0099s | 2,011,901 ops/s | 245.6 | 0.0116s | 1,723,405 ops/s | 210.4 | +17.2% |
+| read 20,000 files | raw fs | 0.1606s | 124,543 ops/s | 15.2 | 0.2040s | 98,030 ops/s | 12.0 | +27.0% |
+| stat 20,000 files | mem | 0.0077s | 2,598,241 ops/s | — | 0.0079s | 2,519,089 ops/s | — | +2.6% |
+| stat 20,000 files | raw fs | 0.0699s | 286,222 ops/s | — | 0.0954s | 209,541 ops/s | — | +36.5% |
+| readdir (20,000 entries) | mem | 0.0041s | 4,845,826 ops/s | — | 0.0043s | 4,675,035 ops/s | — | +4.9% |
+| readdir (20,000 entries) | raw fs | 0.0069s | 2,894,815 ops/s | — | 0.0071s | 2,808,607 ops/s | — | +2.9% |
+| create 20,000 files | dir | 1.1418s | 17,517 ops/s | 2.1 | 1.7006s | 11,760 ops/s | 1.4 | +48.9% |
+| read 20,000 files | dir | 0.1530s | 130,705 ops/s | 16.0 | 0.2238s | 89,371 ops/s | 10.9 | +46.3% |
+| stat 20,000 files | dir | 0.1379s | 145,056 ops/s | — | 0.1984s | 100,826 ops/s | — | +43.9% |
+| readdir (20,000 entries) | dir | 0.0037s | 5,465,162 ops/s | — | 0.0042s | 4,799,105 ops/s | — | +13.5% |
+| unlink 20,000 files | mem | 0.0179s | 1,117,742 ops/s | — | 0.0183s | 1,094,715 ops/s | — | +2.2% |
+| unlink 20,000 files | dir | 0.4911s | 40,724 ops/s | — | 0.7500s | 26,667 ops/s | — | +52.7% |
+| unlink 20,000 files | raw fs | 0.4746s | 42,141 ops/s | — | 0.6404s | 31,228 ops/s | — | +34.9% |
+| mkdir 4,000 dirs | mem | 0.0056s | 719,696 ops/s | — | 0.0051s | 791,918 ops/s | — | -8.9% |
+| rmdir 4,000 dirs | mem | 0.0040s | 993,724 ops/s | — | 0.0435s | 91,873 ops/s | — | +987.5% |
+| mkdir 4,000 dirs | dir | 0.1710s | 23,386 ops/s | — | 0.2123s | 18,838 ops/s | — | +24.2% |
+| rmdir 4,000 dirs | dir | 0.1257s | 31,827 ops/s | — | 0.1372s | 29,163 ops/s | — | +9.1% |
+| mkdir 4,000 dirs | raw fs | 0.1374s | 29,122 ops/s | — | 0.1837s | 21,772 ops/s | — | +33.7% |
+| rmdir 4,000 dirs | raw fs | 0.0951s | 42,043 ops/s | — | 0.1305s | 30,640 ops/s | — | +37.2% |
+| write 1MB | mem | 0.0001s | 7,058 ops/s | 7,058.1 | 0.0002s | 6,313 ops/s | 6,312.7 | +100.0% |
+| read 1MB | mem | 0.0000s | 50,051 ops/s | 50,050.9 | 0.0000s | 48,616 ops/s | 48,616.4 | +0.0% |
+| write 16MB | mem | 0.0110s | 91 ops/s | 1,458.2 | 0.0407s | 25 ops/s | 393.3 | +270.0% |
+| read 16MB | mem | 0.0009s | 1,058 ops/s | 16,920.1 | 0.0010s | 1,021 ops/s | 16,338.0 | +11.1% |
+| write 64MB | mem | 0.0586s | 17 ops/s | 1,091.3 | 0.0599s | 17 ops/s | 1,068.3 | +2.2% |
+| read 64MB | mem | 0.0032s | 313 ops/s | 20,056.5 | 0.0300s | 33 ops/s | 2,131.0 | +837.5% |
+| write 1MB | raw fs | 0.0004s | 2,759 ops/s | 2,759.4 | 0.0004s | 2,474 ops/s | 2,474.0 | +0.0% |
+| read 1MB | raw fs | 0.0001s | 16,812 ops/s | 16,812.4 | 0.0001s | 16,756 ops/s | 16,756.0 | +0.0% |
+| write 16MB | raw fs | 0.0044s | 229 ops/s | 3,656.4 | 0.0044s | 228 ops/s | 3,645.0 | +0.0% |
+| read 16MB | raw fs | 0.0010s | 989 ops/s | 15,827.9 | 0.0010s | 965 ops/s | 15,443.5 | +0.0% |
+| write 64MB | raw fs | 0.0186s | 54 ops/s | 3,449.7 | 0.0184s | 54 ops/s | 3,478.9 | -1.1% |
+| read 64MB | raw fs | 0.0047s | 213 ops/s | 13,633.1 | 0.0049s | 203 ops/s | 13,003.0 | +4.3% |
+| write 1MB | raw+fsync | 0.0180s | 56 ops/s | 55.7 | 0.0162s | 62 ops/s | 61.7 | -10.0% |
+| read 1MB | raw+fsync | 0.0001s | 9,059 ops/s | 9,058.8 | 0.0001s | 12,025 ops/s | 12,025.1 | +0.0% |
+| write 16MB | raw+fsync | 0.0231s | 43 ops/s | 691.7 | 0.0247s | 41 ops/s | 648.1 | +6.9% |
+| read 16MB | raw+fsync | 0.0012s | 855 ops/s | 13,672.2 | 0.0014s | 726 ops/s | 11,610.9 | +16.7% |
+| write 64MB | raw+fsync | 0.0803s | 12 ops/s | 797.2 | 0.1001s | 10 ops/s | 639.5 | +24.7% |
+| read 64MB | raw+fsync | 0.0048s | 209 ops/s | 13,393.5 | 0.0200s | 50 ops/s | 3,193.8 | +316.7% |
+| compact 20,000 entries to pack | pack | 0.0267s | 747,839 ops/s | 91.3 | 0.0250s | 800,791 ops/s | 97.8 | -6.4% |
+| random-read 20,000 entries | pack (mmap'd) | 0.0082s | 2,435,930 ops/s | 297.4 | 0.0083s | 2,397,682 ops/s | 292.7 | +1.2% |
+| random-read 20,000 entries | raw fs | 0.1629s | 122,749 ops/s | 15.0 | 0.1691s | 118,290 ops/s | 14.4 | +3.8% |
+| concurrent create+read+unlink (8×4,000×3) | mem | 0.2750s | 349,127 ops/s | — | 0.2981s | 322,041 ops/s | — | +8.4% |
+| concurrent create+read+unlink (8×4,000×3) | raw fs | 5.6498s | 16,992 ops/s | — | 6.3837s | 15,038 ops/s | — | +13.0% |
+| mount 500 backends | vfs | 0.0054s | 92,833 ops/s | — | 0.0054s | 91,871 ops/s | — | +0.0% |
+| resolve, 500 mounts | vfs | 0.0020s | 246,064 ops/s | — | 0.0019s | 262,671 ops/s | — | -5.0% |
+| unmount 500 backends | vfs | 0.0046s | 109,207 ops/s | — | 0.0045s | 110,348 ops/s | — | -2.2% |
+| mount 2,000 backends | vfs | 0.0831s | 24,081 ops/s | — | 0.0836s | 23,926 ops/s | — | +0.6% |
+| resolve, 2,000 mounts | vfs | 0.0296s | 67,458 ops/s | — | 0.0293s | 68,318 ops/s | — | -1.0% |
+| unmount 2,000 backends | vfs | 0.0758s | 26,397 ops/s | — | 0.0802s | 24,932 ops/s | — | +5.8% |
+| mount 8,000 backends | vfs | 1.4739s | 5,428 ops/s | — | 1.4905s | 5,367 ops/s | — | +1.1% |
+| resolve, 8,000 mounts | vfs | 0.4938s | 16,200 ops/s | — | 0.4616s | 17,332 ops/s | — | -6.5% |
+| unmount 8,000 backends | vfs | 1.5172s | 5,273 ops/s | — | 1.5176s | 5,271 ops/s | — | +0.0% |
 
-Almost every row sits within ±15% of the documented figures — consistent
-with the single-run jitter `BENCH.md` already warns about, not a
-regression. Two rows exceed that: `unlink 20,000 files (dir)` (+21.7%,
-plausible container/host I/O noise, same direction as the other `dir`
-metadata rows this run) and `write 1MB (raw+fsync)` (+88.9%, but this is a
-tiny absolute value — 18ms vs 34ms for one syscall — the single number most
-sensitive to one slow `fsync` on this container's overlay filesystem, not a
-meaningful regression at that scale). Total wall-clock: 4m7.7s this run vs
-5m23.9s documented, itself within the same single-run variance.
+The `pack`-backend rows are the most stable in the whole table (compaction
+-6.4%, random-access read +1.2%/+3.8%), consistent with `BENCH.md`'s point
+that `pack` random access is a flat `mmap`'d table lookup rather than
+anything sensitive to scheduling jitter the way syscall-heavy `dir`/raw-fs
+metadata operations are.
+
+Most rows sit within ±15% of the documented figures — single-run jitter,
+not a regression, exactly as `BENCH.md`'s stated methodology warns to
+expect. This run was noisier than the previous spot-check, though, with
+several rows well outside that band, worth naming rather than averaging
+away: every `dir`-backend metadata row (`create`/`read`/`stat`/`unlink`/
+`mkdir`) is elevated 24–53% together, pointing at host-side I/O contention
+during this run rather than anything PackFS-specific (the `mem` and `pack`
+rows, touching no host filesystem, mostly did not move); and two `mem`
+rows are outliers even by that standard — `rmdir 4,000 dirs` (+987.5%,
+0.0040s → 0.0435s) and `read 64MB` (+837.5%, 0.0032s → 0.0300s). Both are
+large relative jumps on a small absolute base (tens of milliseconds),
+exactly where a single scheduling stall has the most disproportionate
+effect on a percentage, and neither is corroborated by a neighboring
+row moving the same way (`mkdir` and `unlink` on `mem`, right next to
+`rmdir`, both stayed flat or improved; `read 16MB`/`write 64MB` on `mem`,
+right next to `read 64MB`, both stayed within a few percent) — the
+single-run, non-statistical methodology `BENCH.md` states up front means
+this can't be distinguished from noise without repeated runs, so it is
+reported here rather than either hidden or asserted as a regression.
+Total wall-clock: 4m10.1s this run vs 5m23.9s documented — faster overall
+despite the several elevated `dir`/raw-fs rows above, because the dominant
+cost throughout every run of this benchmark is the single `raw+fsync`
+20,000-file create test (~116s here too, unaffected — see `BENCH.md`'s
+"Environment" section for why that test in particular is unrelated to
+PackFS's own performance).
 
 ## Quick example
 
@@ -259,8 +285,20 @@ cell directly and never touch the snapshot. `mem`-backed buffer growth
 never mutates a buffer address a reader might be reading (Section 5.7):
 growth always allocates a new buffer and publishes it, never reallocates in
 place. `tests/test_concurrency.c` exercises this under concurrent reader and
-writer threads, and the suite is regularly run under ThreadSanitizer and
-AddressSanitizer (see `.gitea/workflows/ci.yml`).
+writer threads. The suite is regularly run under AddressSanitizer/
+UndefinedBehaviorSanitizer, both locally and in CI (see
+`.gitea/workflows/ci.yml`), and this is not aspirational — ASan caught a
+real heap-use-after-free in the snapshot-reclamation logic during
+development (see the `reclaim_gate` note in `src/internal.h`).
+ThreadSanitizer is configured the same way but, as of this writing, has
+not actually completed a run in either environment this project has been
+built and tested in so far — the local development sandbox and this
+project's own CI runner both block the `personality(ADDR_NO_RANDOMIZE)`
+syscall TSan needs to start (see `CONTRIBUTING.md` for the confirming
+tests in each case). CI's TSan step is written to not fail the build over
+that specific, known-benign failure, which means a green CI run is not
+evidence TSan actually executed — stated plainly here rather than left to
+be assumed from CI showing green.
 
 ## Security
 
@@ -283,7 +321,11 @@ security boundary, and how to report a vulnerability.
 See [`CONTRIBUTING.md`](CONTRIBUTING.md). Read `concept.md` and `CLAUDE.md`
 first — they are the project's actual specification and its enforced
 documentation standard, respectively, and every design decision in the code
-traces back to one of them.
+traces back to one of them. [`POSTMORTEM.md`](POSTMORTEM.md) is a detailed,
+permanent record of every real bug and environment issue found during this
+project's development — what happened, how it was diagnosed, the fix, and
+the regression-prevention artifact for each. Check it before re-diagnosing
+something that might already be answered there.
 
 ## License
 

SECURITY.md

@@ -38,10 +38,42 @@ rather than a generic "we take security seriously":
   externally-sourced pack additionally needs a checksum verified at load.
   Loading an attacker-supplied `.img` file that PackFS accepts without
   detecting corruption, or that causes an out-of-bounds read/write, is a
-  security bug.
+  security bug. **That checksum (FNV-1a64, `src/hash.c`) is explicitly
+  non-cryptographic and not collision-resistant** — stated plainly here
+  because this is the one place in this document where it matters most:
+  it is a real, effective check against *accidental* corruption (a
+  truncated copy, a bit flip, a bad transfer), but it is not a tamper-
+  evident guarantee against a deliberate adversary who can choose the
+  bytes of a `.img` file — someone with that capability could in
+  principle construct a corrupted pack whose checksum still matches. Do
+  not treat "checksum verified at load" as a substitute for verifying the
+  *source* of an externally-supplied pack through some other channel if
+  the threat model includes a deliberate adversary, not just accidental
+  corruption.
 - **Journal records are length/checksum self-describing** (Section 4.4),
   so replay stops cleanly at the first torn or corrupted record rather
   than reading past it.
+- **The crash-safety design above is verified by actual fault injection,
+  not only by reasoning about the design.** `tests/test_crash_consistency.c`
+  forks a real child process and `SIGKILL`s it at randomized points during
+  journaled writes and during compaction, then checks what a fresh reopen
+  recovers, across dozens of trials per run — not a hand-truncated file
+  standing in for a crash. See `CLAUDE.md`'s "Known reliability
+  characteristics" for the methodology and results, including a real bug
+  the *test itself* had on its first draft (conflating "never attempted
+  before the kill" with "lost after completing") — fixed before treating
+  the crash-safety claim as verified.
+- **A real, previously-unknown data-loss bug in this exact area was found
+  and fixed: a failed journal write (disk full, quota, an I/O error) used
+  to be silently reported as success**, and even after fixing that, a
+  partial write left behind a torn record that made every *later*,
+  individually-successful write unrecoverable on reopen too — not just
+  the one that actually failed. Both are fixed (`src/overlay.c`,
+  `journal_append_record`) and covered by `tests/test_journal_failure.c`,
+  which forces a real write failure via `RLIMIT_FSIZE` rather than
+  simulating one. See `CLAUDE.md`'s "Known reliability characteristics"
+  for the full story, including how each half of the bug was confirmed by
+  direct reproduction before and after the fix, not just inspection.
 
 ## What PackFS explicitly does *not* claim
 

src/overlay.c

@@ -61,19 +61,43 @@ static const char *dirrel(const char *p) { return p[0] == '/' ? p + 1 : p; }
 #define JOP_DELETE 1
 #define JOP_MKDIR  2
 
-static void journal_append_record(Overlay *ov, uint8_t op, const char *name,
+/*
+ * Returns 0 on success (including the intentional "no pack_path configured,
+ * no durability wanted" case — an embedder may pass NULL to
+ * backend_overlay_new's pack_path to run with no journal/compaction target
+ * at all, and that is not a failure), -1 if a durable append was actually
+ * attempted and failed at any step (open, an fwrite short of its full
+ * count, fflush, or fsync).
+ *
+ * This return value matters: every one of journal_append_record's callers
+ * used to ignore it entirely (this function used to be void, and every
+ * fwrite/fflush/fsync call above went unchecked) — meaning a disk-full or
+ * I/O-error condition during an ordinary vfs_write/vfs_mkdir/vfs_unlink/
+ * vfs_rename on an overlay-backed file silently reported success while the
+ * change was never actually made durable, a real data-loss bug this
+ * project's own crash-safety claims (Section 4.4) implicitly promise does
+ * not happen. Found via code review while building fault-injection tests
+ * for those claims, not by a test catching it first — there was no test
+ * that could have caught it, since nothing exercised a failing journal
+ * write before this fix. See tests/test_journal_failure.c.
+ */
+static int journal_append_record(Overlay *ov, uint8_t op, const char *name,
                                   int64_t mtime, uint64_t size, const void *data) {
-    if (!ov->pack_path) return;
+    if (!ov->pack_path) return 0; /* no journal configured: not an error */
     pthread_mutex_lock(&ov->journal_lock);
     if (!ov->journal_fp) {
         char jpath[PFS_PATH_MAX];
         snprintf(jpath, sizeof(jpath), "%s.jnl", ov->pack_path);
         ov->journal_fp = fopen(jpath, "ab");
     }
+    int ok = 0;
     if (ov->journal_fp) {
+        fseek(ov->journal_fp, 0, SEEK_END);
+        long pre_len = ftell(ov->journal_fp);
         uint32_t name_len = (uint32_t)strlen(name);
         uint32_t payload_len = 4 + name_len + (op == JOP_PUT ? (uint32_t)(8 + 8 + size) : 0);
         unsigned char *payload = (unsigned char *)malloc(payload_len);
+        if (payload) {
             size_t off = 0;
             memcpy(payload + off, &name_len, 4); off += 4;
             memcpy(payload + off, name, name_len); off += name_len;
@@ -83,30 +107,61 @@ static void journal_append_record(Overlay *ov, uint8_t op, const char *name,
                 if (size) memcpy(payload + off, data, size);
             }
             uint32_t checksum = (uint32_t)pfs_fnv1a64(payload, payload_len);
-        fwrite(&payload_len, 4, 1, ov->journal_fp);
-        fwrite(&checksum, 4, 1, ov->journal_fp);
-        fwrite(&op, 1, 1, ov->journal_fp);
-        fwrite(payload, 1, payload_len, ov->journal_fp);
-        fflush(ov->journal_fp);
-        fsync(fileno(ov->journal_fp));
+            ok = fwrite(&payload_len, 4, 1, ov->journal_fp) == 1
+              && fwrite(&checksum, 4, 1, ov->journal_fp) == 1
+              && fwrite(&op, 1, 1, ov->journal_fp) == 1
+              && fwrite(payload, 1, payload_len, ov->journal_fp) == payload_len
+              && fflush(ov->journal_fp) == 0
+              && fsync(fileno(ov->journal_fp)) == 0;
             free(payload);
+            /*
+             * A partial write here (e.g. ENOSPC or, as in
+             * tests/test_journal_failure.c, RLIMIT_FSIZE mid-record)
+             * would otherwise leave a torn record sitting in the middle
+             * of the journal file permanently: journal_replay stops at
+             * the first record whose length doesn't fit (Section 4.3),
+             * which means every record appended *after* this one —
+             * including ones that themselves complete successfully
+             * later — would be silently unreachable on every future
+             * reopen until the next compaction, not just this one
+             * failed write. Confirmed by direct reproduction before this
+             * fix: a forced failed write followed by a genuinely
+             * successful one was unrecoverable, because replay never got
+             * past the torn record to reach the good one. Rolling the
+             * file back to its exact pre-record length keeps "the
+             * journal on disk is valid up to EOF" true even when a
+             * write fails, not just when every write succeeds.
+             */
+            if (!ok && pre_len >= 0) {
+                fflush(ov->journal_fp);
+                if (ftruncate(fileno(ov->journal_fp), (off_t)pre_len) == 0) {
+                    fseek(ov->journal_fp, 0, SEEK_END);
+                }
+            }
+        }
     }
     pthread_mutex_unlock(&ov->journal_lock);
+    return ok ? 0 : -1;
 }
 
-static void journal_append_put(Overlay *ov, const char *name, const void *data, uint64_t size, int64_t mtime) {
-    journal_append_record(ov, JOP_PUT, name, mtime, size, data);
+static int journal_append_put(Overlay *ov, const char *name, const void *data, uint64_t size, int64_t mtime) {
+    return journal_append_record(ov, JOP_PUT, name, mtime, size, data);
 }
-static void journal_append_delete(Overlay *ov, const char *name) {
-    journal_append_record(ov, JOP_DELETE, name, 0, 0, NULL);
+static int journal_append_delete(Overlay *ov, const char *name) {
+    return journal_append_record(ov, JOP_DELETE, name, 0, 0, NULL);
 }
-static void journal_append_mkdir(Overlay *ov, const char *name) {
-    journal_append_record(ov, JOP_MKDIR, name, 0, 0, NULL);
+static int journal_append_mkdir(Overlay *ov, const char *name) {
+    return journal_append_record(ov, JOP_MKDIR, name, 0, 0, NULL);
 }
 
 /* journals the *current* full content of `path` after a write/rename,
- * matching the whole-value scheme described above. */
-static void journal_put_current(Overlay *ov, const char *path, MutCell *cell) {
+ * matching the whole-value scheme described above. Returns 0 on success,
+ * -1 on failure — including an allocation failure while staging the
+ * content to journal: `buf` used to be passed into journal_append_put
+ * (and from there into a memcpy of `size` bytes) even when malloc(size)
+ * had failed and left it NULL, an OOM-triggered NULL-pointer memcpy this
+ * fix also closes, not just the silently-ignored-failure issue above. */
+static int journal_put_current(Overlay *ov, const char *path, MutCell *cell) {
     UpperStore *us = upper_of(ov);
     pfs_usize size;
     void *buf = NULL;
@@ -121,14 +176,16 @@ static void journal_put_current(Overlay *ov, const char *path, MutCell *cell) {
     } else {
         VfsStat st;
         int e = 0;
-        if (pfs_dir_statat(us->root_fd, dirrel(path), &st, &e) < 0) return;
+        if (pfs_dir_statat(us->root_fd, dirrel(path), &st, &e) < 0) return -1;
         size = st.size;
         mtime = st.mtime;
         buf = size ? malloc(size) : NULL;
         if (buf) upper_cell_read(us, cell, path, 0, buf, size);
     }
-    journal_append_put(ov, path, buf, size, mtime);
+    if (size && !buf) return -1; /* allocation failed: nothing safe to journal */
+    int rc = journal_append_put(ov, path, buf, size, mtime);
     free(buf);
+    return rc;
 }
 
 static void journal_replay(Overlay *ov) {
@@ -291,7 +348,15 @@ static pfs_isize overlay_write(VfsFile *f, const void *buf, pfs_usize n) {
     pfs_isize w = upper_cell_write(upper_of(of->ov), of->cell, of->path, of->pos, buf, n, &err);
     if (w < 0) return err;
     of->pos += (pfs_usize)w;
-    journal_put_current(of->ov, of->path, of->cell);
+    /* The in-memory write above already succeeded and is visible to
+     * readers in this process regardless of what happens next — that
+     * cannot be undone, the same way a real write() into the page cache
+     * isn't undone by a later failed fsync(). What CAN still be reported
+     * is durability: if the journal append fails, this call reports
+     * VFS_ERR_IO instead of the byte count, so a caller relying on
+     * "success means durable" (Section 4.4's whole premise) finds out
+     * immediately rather than silently losing the write on a later crash. */
+    if (journal_put_current(of->ov, of->path, of->cell) < 0) return VFS_ERR_IO;
     return w;
 }
 
@@ -461,7 +526,7 @@ static int overlay_mkdir(Backend *b, const char *path) {
 
     int err = 0;
     if (upper_mkdir(us, path, &err) < 0) return err;
-    journal_append_mkdir(ov, path);
+    if (journal_append_mkdir(ov, path) < 0) return VFS_ERR_IO; /* see overlay_write's comment */
     return VFS_OK;
 }
 
@@ -513,7 +578,7 @@ static int overlay_unlink(Backend *b, const char *path) {
 
     int err = 0;
     if (upper_remove(us, path, had_lower, &err) < 0) return err;
-    journal_append_delete(ov, path);
+    if (journal_append_delete(ov, path) < 0) return VFS_ERR_IO; /* see overlay_write's comment */
     return VFS_OK;
 }
 
@@ -555,16 +620,24 @@ static int overlay_rename(Backend *b, const char *from, const char *to) {
         upper_remove(us, from, 1, &err2);
     }
 
-    journal_append_delete(ov, from);
+    /* Both journal calls below are attempted regardless of whether the
+     * first one failed — "from" being gone and "to" now holding the
+     * content are two independent durable facts, and a failure to
+     * journal one is not a reason to skip attempting the other. See
+     * overlay_write's comment on why a journal failure here reports
+     * VFS_ERR_IO despite the in-memory rename already having happened. */
+    int jerr = journal_append_delete(ov, from) < 0;
     if (kind == ENTRY_DIR_MARKER) {
-        journal_append_mkdir(ov, to);
+        jerr |= journal_append_mkdir(ov, to) < 0;
     } else {
         UpperSnapshot *s2 = upper_acquire(us);
         const UpperEntry *nue;
-        if (upper_lookup(s2, to, &nue) && nue->kind == ENTRY_FILE) journal_put_current(ov, to, nue->cell);
+        if (upper_lookup(s2, to, &nue) && nue->kind == ENTRY_FILE) {
+            jerr |= journal_put_current(ov, to, nue->cell) < 0;
+        }
         upper_release(s2);
     }
-    return VFS_OK;
+    return jerr ? VFS_ERR_IO : VFS_OK;
 }
 
 /* ---- compaction (Section 4.1 step 5, 5.3, 9.2) ---- */

tests/test_crash_consistency.c

@@ -0,0 +1,412 @@
+/*
+ * test_crash_consistency.c — real fault injection for Section 4.3/4.4's
+ * crash-safety claims (self-checking journal records, atomic-rename
+ * compaction), via an actual fork()+SIGKILL of a child mid-operation and
+ * a fresh reopen afterward, not a hand-truncated file standing in for a
+ * crash. The two scenarios below (journal writes, compaction) were the
+ * two concrete claims CLAUDE.md/concept.md make about crash behavior;
+ * before this file, neither had ever actually been exercised by killing
+ * a real process mid-write — only by static reasoning about the design
+ * and by test_pack_overlay.c's separate (unrelated) test of loading an
+ * already-corrupted pack file.
+ *
+ * Timing constants below are empirically calibrated for this project's
+ * own build (see the comment above each scenario), not arbitrary: the
+ * goal is to sample kill delays across the real duration of the
+ * operation under test, confirmed per-run by reporting how many trials
+ * actually landed a genuine mid-operation kill (WIFSIGNALED) versus how
+ * many the child outran (WIFEXITED) — a run reporting zero interrupted
+ * trials would mean this test verified nothing and should be treated as
+ * a test bug, not a pass; see the CHECK on INTERRUPTED_MIN below.
+ */
+
+#include <fcntl.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/wait.h>
+#include <time.h>
+#include <unistd.h>
+
+#include "packfs.h"
+#include "test_harness.h"
+
+static void sleep_sec(double s) {
+    struct timespec ts;
+    ts.tv_sec = (time_t)s;
+    ts.tv_nsec = (long)((s - (double)ts.tv_sec) * 1e9);
+    nanosleep(&ts, NULL);
+}
+
+/* fork the given child function, let it run for kill_delay seconds, then
+ * SIGKILL it. Returns 1 if the child was genuinely still running (killed
+ * by the signal), 0 if it had already exited on its own. */
+static int run_and_kill(void (*child_fn)(void *), void *arg, double kill_delay) {
+    pid_t pid = fork();
+    if (pid < 0) { perror("fork"); exit(1); }
+    if (pid == 0) {
+        child_fn(arg);
+        _exit(99); /* child_fn must _exit itself; this is a safety net */
+    }
+    sleep_sec(kill_delay);
+    kill(pid, SIGKILL);
+    int status = 0;
+    waitpid(pid, &status, 0);
+    return WIFSIGNALED(status) && WTERMSIG(status) == SIGKILL;
+}
+
+/* ================= scenario A: kill mid-burst of journaled writes ===== */
+/*
+ * Calibrated: ~6ms per individual journaled create+write+close on this
+ * build/environment (fopen+fwrite+fflush+fsync per record, Section 4.4).
+ * BURST=60 writes takes ~0.36s; sampling kill delays across [0, 0.40s]
+ * covers the whole burst including its very start and its tail.
+ */
+#define BURST_N       60
+#define BURST_TRIALS  25
+#define BURST_MAX_DELAY 0.40
+
+typedef struct { char pack_path[512]; } BurstArg;
+
+static void mk_burst_content(char *buf, size_t n, int i) {
+    snprintf(buf, n, "burst-content-%06d", i);
+}
+
+static void burst_child(void *arg_) {
+    BurstArg *arg = (BurstArg *)arg_;
+    Vfs *v = vfs_new();
+    Backend *mem = backend_mem_new();
+    int oerr = 0;
+    Backend *ov = backend_overlay_new(arg->pack_path, mem, &oerr);
+    if (!ov) _exit(2);
+    if (vfs_mount(v, "/", ov) != VFS_OK) _exit(3);
+
+    char name[64], content[64];
+    for (int i = 0; i < BURST_N; i++) {
+        snprintf(name, sizeof(name), "/f%05d.txt", i);
+        mk_burst_content(content, sizeof(content), i);
+        int err = 0;
+        VfsFile *f = vfs_open(v, name, VFS_O_WRONLY | VFS_O_CREAT, &err);
+        if (!f) _exit(4);
+        if (vfs_write(f, content, strlen(content)) != (pfs_isize)strlen(content)) _exit(5);
+        if (vfs_close(f) != VFS_OK) _exit(6);
+    }
+    _exit(0); /* all BURST_N writes durably committed */
+}
+
+static void test_journal_burst_crash(void) {
+    BurstArg arg;
+    snprintf(arg.pack_path, sizeof(arg.pack_path), "/tmp/packfs_test_crash_burst_%d.img", (int)getpid());
+    char jpath[600];
+    snprintf(jpath, sizeof(jpath), "%s.jnl", arg.pack_path);
+
+    int interrupted_count = 0;
+    for (int trial = 0; trial < BURST_TRIALS; trial++) {
+        unlink(arg.pack_path);
+        unlink(jpath);
+        double delay = (BURST_MAX_DELAY * (double)trial) / (double)(BURST_TRIALS - 1);
+        int interrupted = run_and_kill(burst_child, &arg, delay);
+        if (interrupted) interrupted_count++;
+
+        Vfs *v2 = vfs_new();
+        Backend *mem2 = backend_mem_new();
+        int oerr2 = 0;
+        Backend *ov2 = backend_overlay_new(arg.pack_path, mem2, &oerr2);
+        CHECK(ov2 != NULL); /* a crash mid-journal must never make reopening fail */
+        if (!ov2) { vfs_free(v2); backend_free(mem2); continue; }
+        CHECK_EQ_INT(vfs_mount(v2, "/", ov2), VFS_OK);
+
+        /* clean-prefix invariant: every present file has exactly correct
+         * content, and there is no gap (file i missing, file i+1 present) */
+        int last_present = -1;
+        char name[64], expect[64], buf[64];
+        for (int i = 0; i < BURST_N; i++) {
+            snprintf(name, sizeof(name), "/f%05d.txt", i);
+            int err = 0;
+            VfsFile *f = vfs_open(v2, name, VFS_O_RDONLY, &err);
+            if (!f) continue;
+            mk_burst_content(expect, sizeof(expect), i);
+            memset(buf, 0, sizeof(buf));
+            pfs_isize n = vfs_read(f, buf, sizeof(buf));
+            vfs_close(f);
+            if (n != (pfs_isize)strlen(expect) || memcmp(buf, expect, strlen(expect)) != 0) {
+                fprintf(stderr, "FAIL trial %d: %s has wrong/corrupt content after crash (delay=%.4f)\n",
+                        trial, name, delay);
+                pfs_test_failures++;
+            }
+            if (i != last_present + 1) {
+                fprintf(stderr, "FAIL trial %d: gap in journal replay prefix at %s (delay=%.4f)\n",
+                        trial, name, delay);
+                pfs_test_failures++;
+            }
+            last_present = i;
+        }
+
+        vfs_unmount(v2, "/");
+        backend_free(ov2);
+        backend_free(mem2);
+        vfs_free(v2);
+    }
+
+    unlink(arg.pack_path);
+    unlink(jpath);
+    fprintf(stderr, "journal-burst crash test: %d/%d trials genuinely interrupted mid-burst\n",
+            interrupted_count, BURST_TRIALS);
+    /* if this is ever 0, the delay schedule no longer matches this
+     * machine's write latency and the test isn't exercising the crash
+     * path at all -- that is itself a failure, not a quiet pass. */
+    CHECK(interrupted_count >= BURST_TRIALS / 4);
+}
+
+/* ================= scenario B: kill mid-compaction ===================== */
+/*
+ * Calibrated: a 100-file, 2KB-payload durable baseline (built once,
+ * untimed, then compacted once to get a clean golden pack.img) plus a
+ * per-trial 10-file durable "extras" batch takes ~0.058s to journal and
+ * the following compaction takes ~0.012s on this build/environment.
+ * Sampling kill delays across [0, 0.08s] covers extras-journaling,
+ * compaction start, mid-compaction, and post-rename.
+ *
+ * The invariant under test is not "pre-state XOR post-state" -- it's
+ * simpler and is the actual documented guarantee (concept.md's crash-
+ * safety constraints, CLAUDE.md's "if writing pack.img.tmp or its fsync
+ * fails, compaction aborts and the existing pack + journal are
+ * untouched"): anything durably journaled (fsynced) *before* vfs_sync is
+ * even called must survive a crash during that vfs_sync call, no matter
+ * where in it the crash lands -- either via journal replay (if
+ * compaction didn't finish) or by being included in the fresh pack (if
+ * it did).
+ */
+#define GOLDEN_N        100
+#define EXTRA_N         10
+#define PAYLOAD_SIZE    2048
+#define COMPACT_TRIALS  40
+#define COMPACT_MAX_DELAY 0.08
+
+typedef struct { char pack_path[512]; char golden_path[512]; char progress_path[512]; } CompactArg;
+
+static void mk_payload(char *buf, size_t n, const char *tag, int i) {
+    int len = snprintf(buf, n, "%s-%06d-", tag, i);
+    for (size_t j = (size_t)len; j < n; j++) buf[j] = (char)('a' + (int)(j % 26));
+}
+
+static int copy_file(const char *from, const char *to) {
+    FILE *in = fopen(from, "rb");
+    if (!in) return -1;
+    FILE *out = fopen(to, "wb");
+    if (!out) { fclose(in); return -1; }
+    char buf[65536];
+    size_t n;
+    while ((n = fread(buf, 1, sizeof(buf), in)) > 0) fwrite(buf, 1, n, out);
+    fclose(in);
+    fclose(out);
+    return 0;
+}
+
+/* Records "N extras durably completed" via a plain POSIX write+fsync on a
+ * dedicated file, entirely independent of packfs. This is the test's
+ * ground truth for how far the child actually got before SIGKILL landed
+ * -- without it, an extra that the child simply never reached in time
+ * (expected, not a bug) is indistinguishable from one that completed and
+ * was then lost (a real bug), since SIGKILL cannot be caught to report
+ * progress any other way. The one imprecision this leaves: the tiny gap
+ * between vfs_close() returning (extra i durably in the journal) and
+ * this progress write's own fsync landing can make the progress count
+ * undercount by one extra in the worst case -- which only makes the
+ * check *less* strict at the margin (skips verifying the most recent
+ * extra), never produces a false failure. */
+static void record_progress(const char *progress_path, int n_done) {
+    int fd = open(progress_path, O_WRONLY | O_CREAT | O_TRUNC, 0644);
+    if (fd < 0) return;
+    char buf[16];
+    int len = snprintf(buf, sizeof(buf), "%d", n_done);
+    /* Best-effort: a short write here only makes read_progress()'s count
+     * more conservative (see the imprecision note above), never wrong in
+     * the unsafe direction, so there is nothing more useful to do with a
+     * failed/partial write than note it happened. A bare `(void)` cast
+     * looked like the idiomatic way to silence write()'s
+     * warn_unused_result attribute, but proved unreliable in practice —
+     * it built warning-free here, then still warned under the Gitea
+     * runner's gcc (a real, observed toolchain-version/config
+     * difference, not a local misconfiguration on either side); an
+     * actual branch on the result, below, is honored by every gcc/clang
+     * version this project has been built with so far. */
+    if (write(fd, buf, (size_t)len) < 0) { /* best-effort; nothing more to do */ }
+    fsync(fd);
+    close(fd);
+}
+
+static int read_progress(const char *progress_path) {
+    FILE *fp = fopen(progress_path, "r");
+    if (!fp) return 0;
+    int n = 0;
+    if (fscanf(fp, "%d", &n) != 1) n = 0;
+    fclose(fp);
+    return n;
+}
+
+static void compact_child(void *arg_) {
+    CompactArg *arg = (CompactArg *)arg_;
+    record_progress(arg->progress_path, 0);
+    Vfs *v = vfs_new();
+    Backend *mem = backend_mem_new();
+    int oerr = 0;
+    Backend *ov = backend_overlay_new(arg->pack_path, mem, &oerr);
+    if (!ov) _exit(2);
+    if (vfs_mount(v, "/", ov) != VFS_OK) _exit(3);
+
+    char payload[PAYLOAD_SIZE];
+    for (int i = 0; i < EXTRA_N; i++) {
+        char name[64];
+        snprintf(name, sizeof(name), "/extra%04d.dat", i);
+        mk_payload(payload, sizeof(payload), "extra", i);
+        int err = 0;
+        VfsFile *f = vfs_open(v, name, VFS_O_WRONLY | VFS_O_CREAT, &err);
+        if (!f) _exit(4);
+        if (vfs_write(f, payload, sizeof(payload)) != (pfs_isize)sizeof(payload)) _exit(5);
+        if (vfs_close(f) != VFS_OK) _exit(6);
+        record_progress(arg->progress_path, i + 1); /* extra i is now durable */
+    }
+    /* every extra above is already durable (each fsynced individually);
+     * this is the operation actually being crash-tested */
+    if (vfs_sync(v, "/") != VFS_OK) _exit(7);
+    _exit(0);
+}
+
+static void build_golden_baseline(CompactArg *arg) {
+    unlink(arg->pack_path);
+    char jpath[600];
+    snprintf(jpath, sizeof(jpath), "%s.jnl", arg->pack_path);
+    unlink(jpath);
+
+    Vfs *v = vfs_new();
+    Backend *mem = backend_mem_new();
+    int oerr = 0;
+    Backend *ov = backend_overlay_new(arg->pack_path, mem, &oerr);
+    CHECK(ov != NULL);
+    CHECK_EQ_INT(vfs_mount(v, "/", ov), VFS_OK);
+
+    char payload[PAYLOAD_SIZE];
+    for (int i = 0; i < GOLDEN_N; i++) {
+        char name[64];
+        snprintf(name, sizeof(name), "/base%05d.dat", i);
+        mk_payload(payload, sizeof(payload), "base", i);
+        int err = 0;
+        VfsFile *f = vfs_open(v, name, VFS_O_WRONLY | VFS_O_CREAT, &err);
+        CHECK(f != NULL);
+        CHECK_EQ_INT(vfs_write(f, payload, sizeof(payload)), (pfs_isize)sizeof(payload));
+        CHECK_EQ_INT(vfs_close(f), VFS_OK);
+    }
+    CHECK_EQ_INT(vfs_sync(v, "/"), VFS_OK); /* clean, untimed compaction */
+
+    vfs_unmount(v, "/");
+    backend_free(ov);
+    backend_free(mem);
+    vfs_free(v);
+    unlink(jpath); /* golden state: compacted pack, no pending journal */
+
+    CHECK_EQ_INT(copy_file(arg->pack_path, arg->golden_path), 0);
+}
+
+static void test_compaction_crash(void) {
+    CompactArg arg;
+    snprintf(arg.pack_path, sizeof(arg.pack_path), "/tmp/packfs_test_crash_compact_%d.img", (int)getpid());
+    snprintf(arg.golden_path, sizeof(arg.golden_path), "/tmp/packfs_test_crash_golden_%d.img", (int)getpid());
+    snprintf(arg.progress_path, sizeof(arg.progress_path), "/tmp/packfs_test_crash_progress_%d.txt", (int)getpid());
+    char jpath[600];
+    snprintf(jpath, sizeof(jpath), "%s.jnl", arg.pack_path);
+
+    build_golden_baseline(&arg);
+
+    int interrupted_count = 0;
+    for (int trial = 0; trial < COMPACT_TRIALS; trial++) {
+        CHECK_EQ_INT(copy_file(arg.golden_path, arg.pack_path), 0);
+        unlink(jpath);
+
+        double delay = (COMPACT_MAX_DELAY * (double)trial) / (double)(COMPACT_TRIALS - 1);
+        int interrupted = run_and_kill(compact_child, &arg, delay);
+        if (interrupted) interrupted_count++;
+
+        Vfs *v2 = vfs_new();
+        Backend *mem2 = backend_mem_new();
+        int oerr2 = 0;
+        Backend *ov2 = backend_overlay_new(arg.pack_path, mem2, &oerr2);
+        CHECK(ov2 != NULL); /* a crash mid-compaction must never make reopening fail */
+        if (!ov2) { vfs_free(v2); backend_free(mem2); continue; }
+        CHECK_EQ_INT(vfs_mount(v2, "/", ov2), VFS_OK);
+
+        /* the pre-existing golden baseline must always survive */
+        char name[64], expect[PAYLOAD_SIZE], buf[PAYLOAD_SIZE];
+        for (int i = 0; i < GOLDEN_N; i++) {
+            snprintf(name, sizeof(name), "/base%05d.dat", i);
+            mk_payload(expect, sizeof(expect), "base", i);
+            int err = 0;
+            VfsFile *f = vfs_open(v2, name, VFS_O_RDONLY, &err);
+            if (!f) {
+                fprintf(stderr, "FAIL trial %d: baseline file %s LOST after compaction crash (delay=%.4f)\n",
+                        trial, name, delay);
+                pfs_test_failures++;
+                continue;
+            }
+            memset(buf, 0, sizeof(buf));
+            pfs_isize n = vfs_read(f, buf, sizeof(buf));
+            vfs_close(f);
+            if (n != (pfs_isize)sizeof(buf) || memcmp(buf, expect, sizeof(buf)) != 0) {
+                fprintf(stderr, "FAIL trial %d: baseline file %s CORRUPTED after compaction crash (delay=%.4f)\n",
+                        trial, name, delay);
+                pfs_test_failures++;
+            }
+        }
+
+        /* Every "extra" write the child actually completed (vfs_close
+         * returned VFS_OK) before it was killed was already durably
+         * journaled at that point -- it must survive regardless of
+         * whether the subsequent vfs_sync (compaction) itself completed.
+         * completed_extras, from the independent progress side-channel,
+         * is the ground truth for how many extras the child actually
+         * finished; extras beyond that were never attempted and their
+         * absence is expected, not a bug. */
+        int completed_extras = read_progress(arg.progress_path);
+        CHECK(completed_extras >= 0 && completed_extras <= EXTRA_N);
+        for (int i = 0; i < completed_extras; i++) {
+            snprintf(name, sizeof(name), "/extra%04d.dat", i);
+            mk_payload(expect, sizeof(expect), "extra", i);
+            int err = 0;
+            VfsFile *f = vfs_open(v2, name, VFS_O_RDONLY, &err);
+            if (!f) {
+                fprintf(stderr, "FAIL trial %d: durably-completed %s LOST after compaction crash "
+                        "(delay=%.4f, completed_extras=%d)\n", trial, name, delay, completed_extras);
+                pfs_test_failures++;
+                continue;
+            }
+            memset(buf, 0, sizeof(buf));
+            pfs_isize n = vfs_read(f, buf, sizeof(buf));
+            vfs_close(f);
+            if (n != (pfs_isize)sizeof(buf) || memcmp(buf, expect, sizeof(buf)) != 0) {
+                fprintf(stderr, "FAIL trial %d: durably-completed %s CORRUPTED after compaction crash "
+                        "(delay=%.4f, completed_extras=%d)\n", trial, name, delay, completed_extras);
+                pfs_test_failures++;
+            }
+        }
+
+        vfs_unmount(v2, "/");
+        backend_free(ov2);
+        backend_free(mem2);
+        vfs_free(v2);
+    }
+
+    unlink(arg.pack_path);
+    unlink(arg.golden_path);
+    unlink(arg.progress_path);
+    unlink(jpath);
+    fprintf(stderr, "compaction crash test: %d/%d trials genuinely interrupted mid-compaction\n",
+            interrupted_count, COMPACT_TRIALS);
+    CHECK(interrupted_count >= COMPACT_TRIALS / 4);
+}
+
+int main(void) {
+    test_journal_burst_crash();
+    test_compaction_crash();
+    TEST_MAIN_END();
+}

tests/test_journal_failure.c

@@ -0,0 +1,206 @@
+/*
+ * test_journal_failure.c — a real, forced journal-write I/O failure
+ * (RLIMIT_FSIZE + SIGXFSZ ignored, so write() fails with EFBIG instead of
+ * killing the process), verifying two real bugs found and fixed in
+ * overlay.c while building this test, not by this test catching them
+ * unprompted -- there was no test before this one that exercised a
+ * failing journal write at all:
+ *
+ * 1. journal_append_* and journal_put_current used to be void, and every
+ *    fwrite/fflush/fsync inside journal_append_record went unchecked --
+ *    a real write failure (disk full, quota, this test's RLIMIT_FSIZE)
+ *    was silently swallowed and vfs_write/vfs_mkdir/vfs_unlink/
+ *    vfs_rename all reported success on an overlay-backed file even
+ *    though the change was never made durable.
+ *
+ * 2. Fixing (1) by itself was not enough: a partial write leaves a torn
+ *    record sitting in the middle of the journal file, and
+ *    journal_replay stops at the first record it can't fully read
+ *    (Section 4.3) -- meaning every record appended *after* the failed
+ *    one, including ones that complete successfully later, was silently
+ *    unreachable on every future reopen. Confirmed by direct
+ *    reproduction before this half of the fix: a forced-failed write
+ *    followed by a genuinely successful one was unrecoverable, because
+ *    replay never got past the torn record in between. Fixed by rolling
+ *    the journal file back to its exact pre-record length on any failed
+ *    write, keeping "valid up to EOF" true even when a write fails.
+ *
+ * What this test does NOT cover: the parallel bug in journal_put_current
+ * (a malloc(size) failure used to leave a NULL buffer that got passed
+ * into journal_append_record's memcpy anyway -- an OOM-triggered
+ * NULL-pointer dereference, not a "write failed" one). Reliably forcing
+ * malloc() to fail in a portable, safe way isn't practical here; that
+ * half of the fix is verified by code inspection (the `if (size && !buf)
+ * return -1;` guard in journal_put_current), not by a test triggering it.
+ */
+
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/resource.h>
+#include <unistd.h>
+
+#include "packfs.h"
+#include "test_harness.h"
+
+static void reset_paths(const char *pack_path, const char *jpath) {
+    unlink(pack_path);
+    unlink(jpath);
+}
+
+static void set_fsize_limit(rlim_t bytes) {
+    struct rlimit rl;
+    rl.rlim_cur = bytes;
+    rl.rlim_max = RLIM_INFINITY;
+    CHECK_EQ_INT(setrlimit(RLIMIT_FSIZE, &rl), 0);
+}
+
+int main(void) {
+    /* SIGXFSZ's default action terminates the process; ignoring it makes
+     * the offending write() return -1/EFBIG instead, which is what this
+     * test needs to observe -- a failed write, not a killed process. */
+    signal(SIGXFSZ, SIG_IGN);
+
+    char pack_path[512];
+    snprintf(pack_path, sizeof(pack_path), "/tmp/packfs_test_journal_fail_%d.img", (int)getpid());
+    char jpath[600];
+    snprintf(jpath, sizeof(jpath), "%s.jnl", pack_path);
+
+    /* --- scenario 1: vfs_write's journal append fails; the failure must
+     * be reported (not swallowed), the in-memory content must still be
+     * readable in this process, and -- critically -- a fresh reopen must
+     * NOT see the torn/rolled-back record as if it were valid. --- */
+    reset_paths(pack_path, jpath);
+    {
+        Vfs *v = vfs_new();
+        Backend *mem = backend_mem_new();
+        int oerr = 0;
+        Backend *ov = backend_overlay_new(pack_path, mem, &oerr);
+        CHECK(ov != NULL);
+        CHECK_EQ_INT(vfs_mount(v, "/", ov), VFS_OK);
+
+        set_fsize_limit(32); /* far smaller than any real journal record */
+
+        int err = 0;
+        VfsFile *f = vfs_open(v, "/big.txt", VFS_O_WRONLY | VFS_O_CREAT, &err);
+        CHECK(f != NULL);
+        const char *payload = "this content is deliberately longer than 32 bytes";
+        pfs_isize wr = vfs_write(f, payload, strlen(payload));
+        CHECK_EQ_INT(wr, VFS_ERR_IO); /* durability failure must be reported */
+        CHECK_EQ_INT(vfs_close(f), VFS_OK);
+
+        /* in-memory: still readable in this process, same as a page-cache
+         * write a later fsync() fails on -- only durability was refused */
+        f = vfs_open(v, "/big.txt", VFS_O_RDONLY, &err);
+        CHECK(f != NULL);
+        if (f) {
+            char buf[128] = {0};
+            pfs_isize rd = vfs_read(f, buf, sizeof(buf));
+            CHECK_EQ_INT(rd, (pfs_isize)strlen(payload));
+            CHECK_EQ_INT(memcmp(buf, payload, strlen(payload)), 0);
+            CHECK_EQ_INT(vfs_close(f), VFS_OK);
+        }
+
+        vfs_unmount(v, "/");
+        backend_free(ov);
+        backend_free(mem);
+        vfs_free(v);
+        set_fsize_limit(RLIM_INFINITY); /* reopening below only reads, but be safe */
+
+        Vfs *v2 = vfs_new();
+        Backend *mem2 = backend_mem_new();
+        int oerr2 = 0;
+        Backend *ov2 = backend_overlay_new(pack_path, mem2, &oerr2);
+        CHECK(ov2 != NULL); /* must never fail to load, even with a rolled-back journal */
+        if (ov2) {
+            CHECK_EQ_INT(vfs_mount(v2, "/", ov2), VFS_OK);
+            int err2 = 0;
+            VfsFile *f2 = vfs_open(v2, "/big.txt", VFS_O_RDONLY, &err2);
+            /* the failed write must not have been replayed as if valid */
+            CHECK(f2 == NULL);
+            CHECK_EQ_INT(err2, VFS_ERR_NOENT);
+            if (f2) vfs_close(f2);
+            vfs_unmount(v2, "/");
+            backend_free(ov2);
+            backend_free(mem2);
+        }
+        vfs_free(v2);
+    }
+
+    /* --- scenario 2: vfs_mkdir's journal append fails --- */
+    reset_paths(pack_path, jpath);
+    {
+        Vfs *v = vfs_new();
+        Backend *mem = backend_mem_new();
+        int oerr = 0;
+        Backend *ov = backend_overlay_new(pack_path, mem, &oerr);
+        CHECK(ov != NULL);
+        CHECK_EQ_INT(vfs_mount(v, "/", ov), VFS_OK);
+        set_fsize_limit(4); /* smaller than even a bare mkdir record */
+        CHECK_EQ_INT(vfs_mkdir(v, "/a-directory-name-long-enough"), VFS_ERR_IO);
+        vfs_unmount(v, "/");
+        backend_free(ov);
+        backend_free(mem);
+        vfs_free(v);
+        set_fsize_limit(RLIM_INFINITY);
+    }
+
+    /* --- scenario 3: after a failed write, lifting the limit lets a
+     * later, unrelated write on the SAME overlay session succeed and
+     * survive a fresh reopen -- proving the rollback keeps the journal
+     * usable afterward, not merely "not corrupted." --- */
+    reset_paths(pack_path, jpath);
+    {
+        Vfs *v = vfs_new();
+        Backend *mem = backend_mem_new();
+        int oerr = 0;
+        Backend *ov = backend_overlay_new(pack_path, mem, &oerr);
+        CHECK(ov != NULL);
+        CHECK_EQ_INT(vfs_mount(v, "/", ov), VFS_OK);
+
+        set_fsize_limit(32);
+        int err = 0;
+        VfsFile *f = vfs_open(v, "/big.txt", VFS_O_WRONLY | VFS_O_CREAT, &err);
+        CHECK(f != NULL);
+        const char *payload = "this content is deliberately longer than 32 bytes";
+        CHECK_EQ_INT(vfs_write(f, payload, strlen(payload)), VFS_ERR_IO);
+        CHECK_EQ_INT(vfs_close(f), VFS_OK);
+
+        set_fsize_limit(RLIM_INFINITY);
+        f = vfs_open(v, "/after.txt", VFS_O_WRONLY | VFS_O_CREAT, &err);
+        CHECK(f != NULL);
+        CHECK_EQ_INT(vfs_write(f, "durable now", 11), 11);
+        CHECK_EQ_INT(vfs_close(f), VFS_OK);
+
+        vfs_unmount(v, "/");
+        backend_free(ov);
+        backend_free(mem);
+        vfs_free(v);
+
+        Vfs *v2 = vfs_new();
+        Backend *mem2 = backend_mem_new();
+        int oerr2 = 0;
+        Backend *ov2 = backend_overlay_new(pack_path, mem2, &oerr2);
+        CHECK(ov2 != NULL);
+        if (ov2) {
+            CHECK_EQ_INT(vfs_mount(v2, "/", ov2), VFS_OK);
+            int err2 = 0;
+            VfsFile *f2 = vfs_open(v2, "/after.txt", VFS_O_RDONLY, &err2);
+            CHECK(f2 != NULL); /* this is the exact case that was broken before the rollback fix */
+            if (f2) {
+                char buf[32] = {0};
+                CHECK_EQ_INT(vfs_read(f2, buf, sizeof(buf)), 11);
+                CHECK_EQ_INT(memcmp(buf, "durable now", 11), 0);
+                CHECK_EQ_INT(vfs_close(f2), VFS_OK);
+            }
+            vfs_unmount(v2, "/");
+            backend_free(ov2);
+            backend_free(mem2);
+        }
+        vfs_free(v2);
+    }
+
+    reset_paths(pack_path, jpath);
+    TEST_MAIN_END();
+}