fix(artifact): reconcile imported hashes against git on bootstrap (1/3)

[moofone]

Tracking issue: #867 (Refs #867). This is part 1 of 3 stacked PRs addressing incremental-reindex cost. Review only commit fix(artifact): reconcile imported hashes against git on bootstrap. Parts 2 (#869) and 3 (#870) stack on top of this one.

What & why

After artifact bootstrap, try_artifact_bootstrap byte-copies a teammate's DB into the local cache, but the imported file_hashes rows carry the exporter's mtime_ns. A fresh clone stamps every file with checkout time, so classify_files (mtime+size only) marks ~every file changed and the first incremental run re-parses the whole repo — the exact run artifact bootstrap exists to make cheap.

Change

Add cbm_artifact_reconcile_hashes() in src/pipeline/artifact.c (+ artifact.h), called from try_artifact_bootstrap immediately after a successful cbm_artifact_import. It re-stamps the file_hashes rows of files git reports unchanged between the artifact's commit and the local working tree, using local stat() values. The existing, untouched classify_files logic then classifies correctly. Zero changes to the incremental pipeline itself.

Reuses two pieces of existing dead code: cbm_artifact_commit() (never called in production) and cbm_store_upsert_file_hash_batch() (existed, never called in production — reconciliation persists all restamped rows in one transaction).

Trust gate (the design point flagged in #867)

cbm_artifact_export now writes an optional "reconcile_basis":"git-clean-head" field to the existing artifact.json, only when export can prove the DB matches a clean checked-out tree at commit:

1. commit is a valid hex OID (40/64-char) from git rev-parse HEAD;
2. the working tree has no tracked/staged/untracked changes outside .codebase-memory/ (:(exclude) pathspec);
3. every file_hashes row's on-disk mtime_ns + size matches the stored stamp (belt-and-suspenders that catches a stale/swapped DB even when the tree looks clean).

Reconciliation requires the marker and the commit to be present + hex-valid + resolvable locally, and skips (returns -1) on any doubt: no git, untrusted/missing marker, non-hex/unknown commit, shallow clone, or any popen/parse uncertainty. A skip leaves rows foreign → today's slow-safe full incremental. No schema_version bump (older binaries ignore the unknown field).

Security

commit is hex-validated before any command construction; repo_path is shell-validated via cbm_validate_shell_arg (same pattern as git_context.c / watcher.c); git output is parsed as NUL-delimited (-z) directly (never line-oriented), so paths with newlines/quotes are handled. A non-empty -z buffer not ending in NUL is treated as corrupt → skip. New cbm_popen call site added to scripts/security-allowlist.txt with justification.

Tests (tests/test_artifact.c, following the existing suite)

• artifact_export_marks_clean_basis — marker set on a clean export, dropped when the tree is dirty.
• artifact_reconcile_restamps_unchanged — full-index → export → clone → modify 2 + add 1 → import → reconcile restamps unchanged rows to local mtime, leaves changed rows foreign (restamped == 3).
• artifact_reconcile_skips_untrusted_metadata — strip the marker → -1, rows untouched (regression guard for old/manual/dirty/stale artifacts).
• artifact_reconcile_skips_unknown_commit — valid-hex-but-absent commit → -1.
• artifact_reconcile_skips_without_git — no .git → -1.
• null-arg safety on cbm_artifact_reconcile_hashes.

All fixtures generated at runtime in tmpdirs; no committed fixtures. scripts/test.sh (ASan+UBSan) green locally — full suite passes.

Benchmark (S2 = bootstrap + first incremental, K = 7 changed)

N	baseline wall / changed	candidate wall / changed
500	635 ms / 503	308 ms / 7
2000	4561 ms / 2003	735 ms / 7
8000	53691 ms / 8003	2563 ms / 7

Candidate S2 wall is 4.8 % of baseline at N=8000 (~21× faster). After S2, candidate node/edge counts exactly equal a from-scratch full index of the same tree (500/2000/8000). Full matrix + raw CSV kept outside the repo.

Checklist

• C only; conventional commit; DCO signed on every commit.
• scripts/test.sh (ASan/UBSan) green.
• New popen/cbm_popen site justified + added to scripts/security-allowlist.txt.
• scripts/lint.sh (clang-tidy/cppcheck/clang-format) — not installed locally; verified ≤100-col + style by hand, CI is the gate.

Seeking maintainer sign-off on the artifact.json trust-marker reuse (point (a) in #867). Happy to rework if a different convention is preferred.

[DeusData]

Thanks for splitting this out from #867. Triage: performance/correctness bug, review first in the #868 -> #869 -> #870 stack.

Because this touches artifact import and the security allowlist file scripts/security-allowlist.txt, review will be a bit cautious: git trust boundaries, path handling, unchanged-file detection, and the necessity/scope of the allowlist update all need to be checked. The focused test coverage in this PR is the right shape.

[moofone]

Thanks for the triage focus areas — here's a map of each to the code, plus one hardening commit pushed in response.

New commit 2e49906 — while re-auditing "unchanged-file detection" against your list, I found one hole and closed it: a file that is gitignored yet indexed (possible via a .cbmignore negation un-skipping a gitignored dir — the #500 feature) appears in file_hashes but is invisible to both git diff and ls-files --others --exclude-standard, so it would have been restamped as "unchanged" even though git can't vouch for its content. Reconciliation now also captures git ls-tree -r -z --name-only <commit> and only restamps rows tracked at the artifact commit and not in the changed set; everything git can't vouch for stays foreign → re-parsed. New regression test artifact_reconcile_skips_untracked_rows covers exactly this shape. The allowlist line was extended to mention ls-tree (same call path, same validation).

Concern → where it's handled:

Git trust boundaries

• Repo-controlled metadata never reaches a git command unless: the reconcile_basis marker equals the sole trusted value AND commit passes is_hex_oid (40/64-char hex) — both hard gates at the top of cbm_artifact_reconcile_hashes.
• repo_path is the only other interpolated component; it goes through cbm_validate_shell_arg (rejects '" ;|&$<> backticks, newlines, and \ on POSIX) in build_git_cmd, mirroring the existing git_context.c/watcher.c pattern, plus a %!^ rejection on Windows.
• Shallow-clone guard: cat-file -e <commit>^{commit} must succeed before any diff is trusted.
• Every uncertainty path (no git, popen failure, non-zero exit, OOM, truncated output) returns -1 → rows stay foreign → today's slow-safe full incremental. Tests: artifact_reconcile_skips_untrusted_metadata, _skips_unknown_commit, _skips_without_git, null-arg safety.

Path handling

• All three git listings are parsed as NUL-delimited (-z) buffers directly — no line-oriented parsing, so paths with newlines/quotes are handled.
• Parse-invariant guard: a non-empty -z buffer not ending in NUL is treated as truncated/corrupt → skip (-1), never a partial parse.
• Per-row snprintf overflow of repo_path + rel_path skips that row (stays foreign), counted in the logged skipped field.

Unchanged-file detection

• Changed set = diff -z --name-only --no-renames <commit> (tracked changes, staged or not, renames shown as add+delete) ∪ ls-files -z --others --exclude-standard (untracked). Rows in the changed set stay foreign; rows missing locally stay foreign and are purged by find_deleted_files; and per the new commit, rows not tracked at the commit stay foreign.
• The exporter side only emits the trust marker when the tree is clean outside .codebase-memory/ AND every hash row's on-disk mtime+size matches the DB (db_hashes_match_disk) — so a stale/swapped DB can't be marked reconcilable even from a clean tree. Test: artifact_export_marks_clean_basis.

Allowlist necessity/scope

• One line, covering the three (now four) read-only git subcommands on the single new call path, all funneled through build_git_cmd's validation. No new process-spawn mechanism — same cbm_popen wrapper as the two existing artifact.c entries.

Also filed #885 for the cross-path bootstrap limitation noted in #867's deferred list (pre-existing, caps this PR's team-sharing benefit until fixed).

Full ASan/UBSan suite green with the new commit. Happy to adjust the trust-marker convention if you'd prefer a different shape for point (a).