Fix/incremental destructive overwrite

[petercoxphoto]

Summary

• index_repository was destructively overwriting a project's graph in the store on every call, regardless of mode. In a mode downgrade (full → fast), files inside FAST_SKIP_DIRS directories (tools, scripts, bin, build, docs, __tests__, migrations, …) were silently purged from the graph. Real incident 2026-04-13: a live Skyline graph lost 516 nodes mid-session when a concurrent process ran fast mode, including every file under packages/mcp/src/tools/.
• Fix: find_deleted_files now stat()s each stored-but-missing file. Files that exist on disk are mode-skipped (nodes and hash rows preserved); only files that truly don't exist are purged. Mode-skipped hash rows are threaded through dump_and_persist → persist_hashes and re-upserted into the rebuilt DB so subsequent reindexes can still classify them.
• Follow-up: persist_hashes now checks the return code of every cbm_store_upsert_file_hash call on both the current-files and mode-skipped loops, logs a warning on failure with scope/rel_path/rc, and continues (partial-failure policy). Without this, a silent upsert failure on the mode_skipped loop could revive the orphaned-node bug class the main fix is designed to prevent.

Why this matters

The upstream FAST_SKIP_DIRS heuristic in discover.c is reasonable in isolation — lesser index modes skip directories named tools/scripts/bin/etc. to index faster. The bug was that cbm_pipeline_run_incremental then classified the resulting absence as deletion, purging the nodes. Nothing in the index store distinguished "this file isn't in the current walk" from "this file doesn't exist anymore". One run of index_repository(mode='fast') on a project that had previously been indexed in full mode would silently destroy the full-mode data — no warning, no log above info level.

The additive-merge semantics were partially built: cbm_pipeline_run_incremental already loads the existing graph into a gbuf, mutates only the changed files, and dumps back. The missing piece was correctly classifying stored-but-absent files.

Fail-safe rules (preserve on uncertainty)

• repo_path NULL → log error, preserve everything. A NULL is a misconfigured pipeline, not a deletion signal.
• snprintf path truncation (repo_path + rel_path ≥ 4KB) → preserve. Can't reliably stat a truncated path.
• stat() errno != ENOENT/ENOTDIR (EACCES, EIO, ELOOP, transient NFS, …) → preserve. File may exist; we just can't see it right now.
• strdup OOM mid-record → drop the entry rather than persist a row with a NULL rel_path that would silently fail the NOT NULL constraint.

Test plan

• New test incremental_fast_preserves_mode_skipped_tools_dir in tests/test_pipeline.c walks the real failure mode end-to-end in four phases:
  1. Full mode index — both files present
  2. Fast mode reindex — tools/util.go nodes preserved (was 0 before fix)
  3. Mutate main.go and fast reindex — forces dump_and_persist to run instead of the noop early-return, exercising the hash-preservation path
  4. Delete tools/util.go from disk and full reindex — confirms the deletion-detection contract still holds (would fail with orphaned nodes without hash-row preservation)
• New test store_file_hash_upsert_rejects_null_required_fields in tests/test_store_nodes.c pins the upstream API contract that cbm_store_upsert_file_hash returns CBM_STORE_ERR on NULL required fields. If a future schema change relaxes any NOT NULL, this test catches it at the producer layer so the consumer's warning doesn't go silent.
• Full test suite (2722 tests) passes under ASan + UBSan + LeakSanitizer.
• Adversarial audit cycle 1: 4 must-fix findings (ENOENT-only stat semantics, snprintf truncation check, hash-row drift, NULL repo_path branch) — all fixed.
• Adversarial audit cycle 2: verified all 4 cycle-1 fixes in code and test, found 1 minor (strdup NULL check) — fixed inline.
• Adversarial audit on follow-up (return-code checking): zero CRITICAL/MAJOR/MINOR, 6 observations all confirmed design choices. Ship verdict.
• Live end-to-end verification: built production binary, ran against a synthetic repo with packages/mcp/src/tools/*.go files, observed log line incremental.classify changed=0 unchanged=1 deleted=0 mode_skipped=2 (deleted would have been 2 before the fix).

Note

This fix removes the destructive failure mode but does NOT relax any policy around always using mode='full'. Full mode remains the only mode that provides complete directory coverage (no FAST_SKIP_DIRS at discover time) and semantic edges (SIMILAR_TO, SEMANTICALLY_RELATED). This fix is defense-in-depth for the case where a session running with stale instructions or any future tooling accidentally calls a lesser mode.