fix(cache): the atexit stat-index flush must not resurrect a deleted corpus (#2974) - #3107
fix(cache): the atexit stat-index flush must not resurrect a deleted corpus (#2974)#3107abhay-codes07 wants to merge 1 commit into
Conversation
…corpus (Graphify-Labs#2974) A post-commit hook runs `graphify update . &` in a short-lived worktree; the branch merges and `git worktree remove` deletes the tree while the rebuild is still running. _flush_stat_index then did `mkdir -p` on the dead path and wrote graphify-out/cache/stat-index.json into it — a husk directory per worktree, 81 of them over a few weeks for the reporter. The index is a pure optimisation: when its root no longer exists it is simply not written (and the dirty flag is cleared so nothing retries). A root that still exists keeps the old behaviour — a first run writes the index before graphify-out/ exists, and the existing tests pin that.
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 4 advisory finding(s) below merit a look before merge.
Formal verification. 1 change(s) tested, no difference found (not proven).
Graphify review — findings
Guards the atexit stat-index flush in _flush_stat_index so it skips writing (and clears the dirty flag) when _stat_index_root no longer exists, instead of mkdir -p'ing the dead path back into a husk that holds only the index — fixing the case where graphify update . & outlives a git worktree remove (#2974). A live root still gets graphify-out/cache/ created on a first run as before.
Worth a look
- _flush_stat_index modifies module globals without declaring them global —
graphify/cache.py:388· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- _flush_stat_index references _stat_index_root without global declaration when assigning _stat_index_dirty —
graphify/cache.py:388· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- _flush_stat_index checks corpus root instead of cache output parent —
graphify/cache.py:388· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Stat-index flush still has a TOCTOU path that can recreate a deleted corpus —
graphify/cache.py:389· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1352 functions depend on the 90 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 498 callers, 42 callees - new:
_rebuild_code()— 98 callers, 50 callees - new:
detect()— 108 callers, 15 callees - new:
save_semantic_cache()— 58 callers, 9 callees - new:
load_cached()— 48 callers, 7 callees - new:
file_hash()— 50 callers, 6 callees - new:
extract_corpus_parallel()— 26 callers, 11 callees - new:
dispatch_command()— 2 callers, 122 callees - …and 18 more — each is listed as a finding
Verification — 1352 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 774 function(s) in the blast radius were not formally verified this run
Formal verification
No difference found (not proven): No behavior difference found in \_flush\_stat\_index (not a proof).
The verifier ran both versions of \_flush\_stat\_index on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
· 26 more finding(s) on lines outside this diff (see the check run).
Closes #2974.
The problem
_flush_stat_indexruns atatexitand didp.parent.mkdir(parents=True, exist_ok=True)before writinggraphify-out/cache/stat-index.json. If the analyzed directory is deleted while graphify is running, the exit-time flush recreates the whole path and leaves a husk containing nothing but the index.The trigger is ordinary: a post-commit hook runs
graphify update . >/dev/null 2>&1 &in a short-livedgit worktree; the branch merges andgit worktree removedeletes the tree while the rebuild is still going; the rebuild finishes and resurrects the dead path. The reporter found 81 such<repo>-<branch>directories over a few weeks.The change
The index is a pure optimisation, so when its root no longer exists it is simply not written — and the dirty flag is cleared so nothing retries. A root that still exists keeps the old behaviour exactly: a first run writes the index before
graphify-out/exists at all, and three existingtest_stat_index_portabilitytests pin that, so the check is deliberately on the root, not on the output directory.The issue's own deterministic repro (
file_hash→rmtree→_flush_stat_index()) now prints "ok".Tests
tests/test_stat_index_husk.py— 3 tests: a corpus deleted mid-run stays deleted (and nothing is left pending), a redirectedcache_rootthat vanished is not recreated, and a live run still writes the index into a not-yet-existinggraphify-out/. With the fix reverted, 2 of 3 fail.test_stat_index_portability,test_cache,test_word_count_cache,test_extract_cache_location,test_cli_exportandtest_incrementalare unchanged; the full suite matches thev8baseline.