From fd4f3133c580bae3c06df544d383ccecddbfe0f5 Mon Sep 17 00:00:00 2001 From: Greg Tiller Date: Sat, 4 Jul 2026 20:13:02 -0300 Subject: [PATCH 1/7] fix(artifact): reconcile imported hashes against git on bootstrap 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_ns + 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. Add cbm_artifact_reconcile_hashes(), called from try_artifact_bootstrap right after a successful cbm_artifact_import. It re-stamps the hash rows of files git reports unchanged between the artifact's commit and the local working tree with local stat() values, so the existing, untouched classify_files logic then classifies correctly. Zero changes to the incremental pipeline itself. Trust gate (so a stale/corrupt artifact can never mark a genuinely changed file unchanged -> graph corruption): cbm_artifact_export now writes an optional "reconcile_basis":"git-clean-head" marker into the existing artifact.json ONLY when it can prove the DB matches a clean checked-out tree at `commit` -- commit is a validated hex OID, the working tree has no changes outside .codebase-memory, and every file_hashes row's on-disk mtime+size matches. Reconciliation requires the marker and skips (returns -1) on any doubt: no git, untrusted/missing marker, non-hex/unknown commit, shallow clone, or any popen or parse uncertainty. A skip leaves rows foreign and falls back to today's slow-safe full incremental. No schema_version bump (older binaries ignore the new optional field). Security: commit is hex-validated before command construction and 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 via line-oriented parsing, so paths with newlines/quotes are handled. New popen call site added to scripts/security-allowlist.txt. Reuses two pieces of existing dead/unused code: cbm_artifact_commit() (was never called in production) and cbm_store_upsert_file_hash_batch() (existed but was never called in production -- reconciliation persists all restamped rows in one transaction). Tests (tests/test_artifact.c, following the existing suite's structure): export sets the marker on a clean tree and drops it when dirty; reconcile restamps unchanged rows and leaves changed rows foreign; reconcile skips (-1, rows untouched) on untrusted metadata, unknown commit, and no-git, plus null-arg safety. Signed-off-by: Greg Tiller --- scripts/security-allowlist.txt | 1 + src/mcp/mcp.c | 8 +- src/pipeline/artifact.c | 408 ++++++++++++++++++++++++++++++++- src/pipeline/artifact.h | 9 + tests/test_artifact.c | 288 +++++++++++++++++++++++ 5 files changed, 706 insertions(+), 8 deletions(-) diff --git a/scripts/security-allowlist.txt b/scripts/security-allowlist.txt index 30d5126a8..183c28bf6 100644 --- a/scripts/security-allowlist.txt +++ b/scripts/security-allowlist.txt @@ -41,6 +41,7 @@ src/pipeline/pass_githistory.c:popen:via cbm_popen wrapper call # ── Pipeline: artifact persistence (git HEAD hash, merge driver config) ──── src/pipeline/artifact.c:cbm_popen:git rev-parse HEAD for artifact metadata (hardcoded cmd) src/pipeline/artifact.c:cbm_popen:git config merge.ours.driver for gitattributes (hardcoded cmd) +src/pipeline/artifact.c:cbm_popen:git diff/ls-files/cat-file for bootstrap hash reconciliation (commit hex-validated via is_hex_oid, repo path shell-validated via cbm_validate_shell_arg) src/pipeline/artifact.c:popen:via cbm_popen wrapper calls # ── UI: HTTP server process management ───────────────────────────────────── diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 413a9d7aa..eb2a8dffb 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -3246,7 +3246,13 @@ static void try_artifact_bootstrap(const char *project_name, const char *repo_pa project_db_path(project_name, db_buf, sizeof(db_buf)); if (cbm_file_size(db_buf) < 0 && cbm_artifact_exists(repo_path)) { cbm_log_info("index.artifact_bootstrap", "project", project_name); - cbm_artifact_import(repo_path, db_buf); + if (cbm_artifact_import(repo_path, db_buf) == 0) { + /* Reconcile imported (foreign-mtime) hash rows against the local + * working tree so the first incremental run classifies by actual + * git diff instead of re-parsing ~everything. Best-effort: a -1 + * return leaves rows foreign and falls back to today's behavior. */ + (void)cbm_artifact_reconcile_hashes(repo_path, db_buf, project_name); + } } } diff --git a/src/pipeline/artifact.c b/src/pipeline/artifact.c index 014f32b23..82c06a6cb 100644 --- a/src/pipeline/artifact.c +++ b/src/pipeline/artifact.c @@ -12,6 +12,7 @@ enum { ART_ZSTD_BEST = 9, ART_RATIO_SCALE = 10, /* multiply ratio by 10 for integer logging */ ART_NUL = 1, /* NUL terminator byte */ + ART_NS_PER_SEC = 1000000000, /* nanoseconds per second (mtime_ns helper) */ }; #define ART_BYTES_PER_MB ((size_t)1024 * 1024) @@ -21,6 +22,8 @@ enum { #include "foundation/compat_fs.h" #include "foundation/compat.h" #include "foundation/log.h" +#include "foundation/str_util.h" /* cbm_validate_shell_arg */ +#include "foundation/hash_table.h" /* CBMHashTable (reconcile changed-set) */ #include "zstd_store.h" @@ -234,6 +237,219 @@ static void iso_timestamp(char *buf, size_t bufsz) { (void)strftime(buf, bufsz, "%Y-%m-%dT%H:%M:%SZ", &tm); } +/* ── Git + trust helpers for bootstrap reconciliation ────────────── */ + +/* Non-NULL sentinel value stored in a membership hash set (key presence is all + * that matters; the value is never dereferenced). */ +static int g_reconcile_sentinel; + +/* Validate s is a hex git object id: 40 chars (SHA-1) or 64 chars (SHA-256). + * Repo-controlled strings reach this check, so it is a hard gate before any + * commit value is interpolated into a git command string. */ +static bool is_hex_oid(const char *s) { + if (!s) { + return false; + } + size_t len = strlen(s); + if (len != 40 && len != 64) { + return false; + } + for (size_t i = 0; i < len; i++) { + char c = s[i]; + if (!((c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F'))) { + return false; + } + } + return true; +} + +/* Portable mtime in nanoseconds. Duplicated locally to match the existing + * per-file idiom in pipeline.c / pipeline_incremental.c rather than introducing + * a third cross-file dependency for one helper. */ +static int64_t art_stat_mtime_ns(const struct stat *st) { +#ifdef __APPLE__ + return ((int64_t)st->st_mtimespec.tv_sec * ART_NS_PER_SEC) + + (int64_t)st->st_mtimespec.tv_nsec; +#elif defined(_WIN32) + return (int64_t)st->st_mtime * ART_NS_PER_SEC; +#else + return ((int64_t)st->st_mtim.tv_sec * ART_NS_PER_SEC) + (int64_t)st->st_mtim.tv_nsec; +#endif +} + +/* Build "git -C \"\" 2>" with a shell-validated repo path. + * repo_path is the only untrusted component; args is a trusted literal or a + * hex-validated commit string (never arbitrary data). Returns false on shell-arg + * validation failure or truncation. Mirrors the git_context.c / watcher.c pattern. */ +static bool build_git_cmd(char *buf, size_t bufsz, const char *repo_path, const char *args) { + if (!cbm_validate_shell_arg(repo_path)) { + return false; + } +#ifdef _WIN32 + for (const char *p = repo_path; *p; p++) { + if (*p == '%' || *p == '!' || *p == '^') { + return false; + } + } + const char *null_dev = "NUL"; +#else + const char *null_dev = "/dev/null"; +#endif + int n = snprintf(buf, bufsz, "git -C \"%s\" %s 2>%s", repo_path, args, null_dev); + return n >= 0 && (size_t)n < bufsz; +} + +/* Run a git command; return true iff it exits 0. Output is drained (not kept) + * so `diff --quiet` semantics work and large outputs don't SIGPIPE. */ +static bool git_run_ok(const char *repo_path, const char *args) { + char cmd[CBM_SZ_2K]; + if (!build_git_cmd(cmd, sizeof(cmd), repo_path, args)) { + return false; + } + FILE *fp = cbm_popen(cmd, "r"); + if (!fp) { + return false; + } + char drain[CBM_SZ_4K]; + while (fread(drain, 1, sizeof(drain), fp) > 0) { + } + return cbm_pclose(fp) == 0; +} + +/* Run a git command and capture the FULL stdout (NUL bytes preserved) into a + * growing malloc'd buffer. Empty output is success with *out_len = 0. Returns + * 0 on success, CBM_NOT_FOUND on popen / non-zero-exit / OOM. Caller frees *out. */ +static int git_capture_full(const char *repo_path, const char *args, char **out, size_t *out_len) { + *out = NULL; + *out_len = 0; + char cmd[CBM_SZ_2K]; + if (!build_git_cmd(cmd, sizeof(cmd), repo_path, args)) { + return CBM_NOT_FOUND; + } + FILE *fp = cbm_popen(cmd, "r"); + if (!fp) { + return CBM_NOT_FOUND; + } + size_t cap = CBM_SZ_4K; + size_t len = 0; + char *buf = malloc(cap); + if (!buf) { + (void)cbm_pclose(fp); + return CBM_NOT_FOUND; + } + size_t n; + while ((n = fread(buf + len, 1, cap - len, fp)) > 0) { + len += n; + if (len == cap) { + size_t ncap = cap * PAIR_LEN; + char *tmp = realloc(buf, ncap); + if (!tmp) { + free(buf); + (void)cbm_pclose(fp); + return CBM_NOT_FOUND; + } + buf = tmp; + cap = ncap; + } + } + int rc = cbm_pclose(fp); + if (rc != 0) { + free(buf); + return CBM_NOT_FOUND; + } + *out = buf; + *out_len = len; + return 0; +} + +/* True iff the working tree has no tracked/staged/untracked changes outside + * .codebase-memory/. The export itself writes .codebase-memory/, so a blanket + * dirty check would always fail; excluding that subtree is what makes the + * "clean export" invariant checkable. */ +static bool tree_clean_for_reconcile(const char *repo_path) { + /* Tracked (staged + unstaged) changes vs HEAD, excluding .codebase-memory. + * The pathspec is single-quoted so the shell passes the `:(exclude)` magic + * (which contains paren subshell metacharacters) literally to git. */ + if (!git_run_ok(repo_path, "diff --quiet HEAD -- . ':(exclude).codebase-memory'")) { + return false; + } + /* Untracked (non-ignored) files outside .codebase-memory. */ + static const char *const ls_args = + "ls-files -z --others --exclude-standard -- . ':(exclude).codebase-memory'"; + char *untracked = NULL; + size_t un_len = 0; + if (git_capture_full(repo_path, ls_args, &untracked, &un_len) != 0) { + return false; + } + bool clean = (un_len == 0); + free(untracked); + return clean; +} + +/* True iff every file_hashes row for project has an on-disk file whose + * mtime_ns + size matches the stored stamp. Any stat failure, path overflow, + * or mismatch makes the artifact untrusted for reconciliation — this is the + * belt-and-suspenders that catches a stale/swapped DB even when the tree looks + * clean. */ +static bool db_hashes_match_disk(const char *repo_path, const char *db_path, const char *project) { + cbm_store_t *s = cbm_store_open_path(db_path); + if (!s) { + return false; + } + cbm_file_hash_t *hashes = NULL; + int count = 0; + bool match = true; + if (cbm_store_get_file_hashes(s, project, &hashes, &count) != CBM_STORE_OK) { + cbm_store_close(s); + return false; + } + for (int i = 0; i < count; i++) { + char abs[CBM_SZ_4K]; + int n = snprintf(abs, sizeof(abs), "%s/%s", repo_path, hashes[i].rel_path); + if (n < 0 || n >= (int)sizeof(abs)) { + match = false; + break; + } + struct stat st; + if (stat(abs, &st) != 0 || art_stat_mtime_ns(&st) != hashes[i].mtime_ns || + (int64_t)st.st_size != hashes[i].size) { + match = false; + break; + } + } + cbm_store_free_file_hashes(hashes, count); + cbm_store_close(s); + return match; +} + +/* Read the optional reconcile_basis marker from artifact.json. True only when it + * is exactly "git-clean-head" (the sole trusted basis this code emits). */ +static bool read_metadata_reconcile_trusted(const char *repo_path) { + char meta_path[CBM_SZ_4K]; + if (!artifact_path(meta_path, sizeof(meta_path), repo_path, CBM_ARTIFACT_META)) { + return false; + } + size_t len = 0; + char *json = read_file_alloc(meta_path, &len); + if (!json) { + return false; + } + yyjson_doc *doc = yyjson_read(json, len, 0); + free(json); + if (!doc) { + return false; + } + yyjson_val *root = yyjson_doc_get_root(doc); + yyjson_val *val = yyjson_obj_get(root, "reconcile_basis"); + bool trusted = false; + if (val) { + const char *s = yyjson_get_str(val); + trusted = (s && strcmp(s, "git-clean-head") == 0); + } + yyjson_doc_free(doc); + return trusted; +} + /* ── Metadata read/write ─────────────────────────────────────────── */ /* Read schema_version from artifact.json. Returns -1 if missing/invalid. */ @@ -285,11 +501,9 @@ static size_t read_metadata_original_size(const char *repo_path) { } /* Write artifact.json metadata. */ -static int write_metadata(const char *repo_path, const char *project_name, int nodes, int edges, - size_t original_size, size_t compressed_size, int compression_level) { - char commit[CBM_SZ_64] = ""; - git_head_hash(repo_path, commit, sizeof(commit)); - +static int write_metadata(const char *repo_path, const char *project_name, const char *commit, + int nodes, int edges, size_t original_size, size_t compressed_size, + int compression_level, bool reconcile_trusted) { char ts[CBM_SZ_64]; iso_timestamp(ts, sizeof(ts)); @@ -306,6 +520,12 @@ static int write_metadata(const char *repo_path, const char *project_name, int n yyjson_mut_obj_add_uint(doc, root, "original_size", (uint64_t)original_size); yyjson_mut_obj_add_uint(doc, root, "compressed_size", (uint64_t)compressed_size); yyjson_mut_obj_add_int(doc, root, "compression_level", compression_level); + /* Optional clean-basis marker: present only when export verified the DB + * matches a clean checked-out tree at `commit` (see cbm_artifact_export). + * Older binaries ignore this unknown field, so no schema_version bump. */ + if (reconcile_trusted) { + yyjson_mut_obj_add_str(doc, root, "reconcile_basis", "git-clean-head"); + } size_t json_len = 0; char *json = yyjson_mut_write(doc, YYJSON_WRITE_PRETTY, &json_len); @@ -518,9 +738,19 @@ int cbm_artifact_export(const char *db_path, const char *repo_path, const char * cbm_store_close(count_store); } + /* Compute the optional clean-basis trust marker. An imported DB can be + * fast-reconciled against git only if export can prove it was built from a + * clean checked-out tree at a known commit. Any doubt omits the marker and + * reconciliation falls back to today's slow-safe full incremental. */ + char commit[CBM_SZ_64] = ""; + bool has_commit = git_head_hash(repo_path, commit, sizeof(commit)); + bool reconcile_trusted = has_commit && is_hex_oid(commit) && + tree_clean_for_reconcile(repo_path) && + db_hashes_match_disk(repo_path, db_path, project_name); + /* Write metadata */ - if (write_metadata(repo_path, project_name, nodes, edges, db_size, (size_t)clen, - compression_level) != 0) { + if (write_metadata(repo_path, project_name, commit, nodes, edges, db_size, (size_t)clen, + compression_level, reconcile_trusted) != 0) { cbm_unlink(zst_path); return CBM_NOT_FOUND; } @@ -711,3 +941,167 @@ char *cbm_artifact_commit(const char *repo_path) { yyjson_doc_free(doc); return result; } + +/* ── Bootstrap reconciliation ─────────────────────────────────────── */ + +/* Add every NUL-delimited entry in buf (length len) to the membership set ht. + * git -z output NUL-terminates every entry including the last, so each non-empty + * entry buf+i is already a C string terminated by its trailing NUL. Keys are + * borrowed from buf (the table does not copy keys), so buf must outlive ht. + * Empty entries (consecutive NULs) are skipped. */ +static void reconcile_add_nul_entries(CBMHashTable *ht, const char *buf, size_t len) { + if (!ht || !buf || len == 0) { + return; + } + size_t i = 0; + while (i < len) { + const char *entry = buf + i; + size_t j = i; + while (j < len && buf[j] != '\0') { + j++; + } + if (j > i) { + cbm_ht_set(ht, entry, &g_reconcile_sentinel); + } + i = (j < len) ? j + 1 : len; + } +} + +/* After a successful import of a trusted clean-basis artifact, re-stamp + * file_hashes rows for files whose content is unchanged between the artifact's + * commit and the local working tree, using local stat() values. The subsequent + * incremental run then classifies by actual git diff instead of re-parsing + * ~every file (whose stored mtime_ns is the exporter's, not local). + * + * Returns the number of rows re-stamped, or -1 when reconciliation was skipped + * (no git / untrusted metadata / unknown or non-hex commit / shallow clone / + * any popen or parse uncertainty). Best-effort: never fails the import — a -1 + * return leaves rows foreign and the first run falls back to today's slow-safe + * full incremental behavior. + * + * Windows/autocrlf note: on-disk bytes may differ from the exporter's while git + * reports "unchanged"; line numbers and parse results are equivalent, so + * re-stamping by git's diff is correct. */ +int cbm_artifact_reconcile_hashes(const char *repo_path, const char *cache_db_path, + const char *project_name) { + if (!repo_path || !cache_db_path || !project_name) { + return CBM_NOT_FOUND; + } + + /* 1. Trusted clean-basis marker must be present and commit must be a valid + * hex object id. Hard gates: repo-controlled metadata never reaches a + * git command otherwise. */ + if (!read_metadata_reconcile_trusted(repo_path)) { + return CBM_NOT_FOUND; + } + char *commit = cbm_artifact_commit(repo_path); + if (!commit || !is_hex_oid(commit)) { + free(commit); + return CBM_NOT_FOUND; + } + + /* 2. Commit must exist locally (shallow-clone guard). commit is hex → safe + * to interpolate into the command. */ + char catfile_args[CBM_SZ_128]; + snprintf(catfile_args, sizeof(catfile_args), "cat-file -e '%s^{commit}'", commit); + if (!git_run_ok(repo_path, catfile_args)) { + free(commit); + return CBM_NOT_FOUND; + } + + /* 3. Build the changed set relative to the artifact commit. NUL-delimited + * (-z) output is parsed directly; line-oriented parsing cannot handle + * paths containing newlines or quotes. */ + char diff_args[CBM_SZ_256]; + snprintf(diff_args, sizeof(diff_args), "diff -z --name-only --no-renames %s --", commit); + char *diff_out = NULL; + size_t diff_len = 0; + char *ls_out = NULL; + size_t ls_len = 0; + if (git_capture_full(repo_path, diff_args, &diff_out, &diff_len) != 0) { + free(commit); + return CBM_NOT_FOUND; + } + if (git_capture_full(repo_path, "ls-files -z --others --exclude-standard --", &ls_out, + &ls_len) != 0) { + free(diff_out); + free(commit); + return CBM_NOT_FOUND; + } + /* Parse-invariant guard: git -z NUL-terminates every entry including the + * last; a non-empty buffer not ending in NUL means truncated/corrupt output + * → untrusted → skip rather than risk misclassifying a changed file as + * unchanged (graph corruption). */ + if ((diff_len > 0 && diff_out[diff_len - 1] != '\0') || + (ls_len > 0 && ls_out[ls_len - 1] != '\0')) { + free(diff_out); + free(ls_out); + free(commit); + return CBM_NOT_FOUND; + } + + CBMHashTable *changed = cbm_ht_create(CBM_SZ_64); + reconcile_add_nul_entries(changed, diff_out, diff_len); + reconcile_add_nul_entries(changed, ls_out, ls_len); + int changed_count = (int)cbm_ht_count(changed); + + /* 4. Restamp every row whose rel_path is NOT in the changed set with local + * stat() values. Changed rows stay foreign → re-parsed; rows whose file + * is missing locally stay foreign → find_deleted_files purges them. */ + cbm_store_t *store = cbm_store_open_path(cache_db_path); + int restamped = 0; + int skipped = 0; + if (store) { + cbm_file_hash_t *stored = NULL; + int stored_count = 0; + if (cbm_store_get_file_hashes(store, project_name, &stored, &stored_count) == + CBM_STORE_OK && + stored_count > 0) { + cbm_file_hash_t *batch = malloc((size_t)stored_count * sizeof(cbm_file_hash_t)); + if (batch) { + int batch_n = 0; + for (int i = 0; i < stored_count; i++) { + if (cbm_ht_get(changed, stored[i].rel_path)) { + continue; + } + char abs[CBM_SZ_4K]; + int n = snprintf(abs, sizeof(abs), "%s/%s", repo_path, stored[i].rel_path); + if (n < 0 || n >= (int)sizeof(abs)) { + skipped++; + continue; + } + struct stat st; + if (stat(abs, &st) != 0) { + skipped++; + continue; + } + /* Borrow project from the caller and rel_path/sha256 from + * the row; all outlive the batch upsert call (which + * consumes them) and are freed afterward. */ + batch[batch_n].project = project_name; + batch[batch_n].rel_path = stored[i].rel_path; + batch[batch_n].sha256 = stored[i].sha256; + batch[batch_n].mtime_ns = art_stat_mtime_ns(&st); + batch[batch_n].size = (int64_t)st.st_size; + batch_n++; + } + if (batch_n > 0 && + cbm_store_upsert_file_hash_batch(store, batch, batch_n) == CBM_STORE_OK) { + restamped = batch_n; + } + free(batch); + } + } + cbm_store_free_file_hashes(stored, stored_count); + cbm_store_close(store); + } + + cbm_ht_free(changed); + free(diff_out); + free(ls_out); + free(commit); + + cbm_log_info("artifact.reconcile", "restamped", itoa_buf(restamped), "changed", + itoa_buf(changed_count), "skipped", itoa_buf(skipped)); + return restamped; +} diff --git a/src/pipeline/artifact.h b/src/pipeline/artifact.h index ecb658548..29df6b11c 100644 --- a/src/pipeline/artifact.h +++ b/src/pipeline/artifact.h @@ -53,4 +53,13 @@ bool cbm_artifact_exists(const char *repo_path); * Returns NULL if artifact doesn't exist or has no commit field. */ char *cbm_artifact_commit(const char *repo_path); +/* After a successful import of a trusted clean-basis artifact, re-stamp + * file_hashes rows for files whose content is unchanged between the artifact's + * commit and the local working tree, using local stat() values. + * Returns the number of rows re-stamped, or -1 when reconciliation was skipped + * (no git / untrusted metadata / unknown commit / any uncertainty). + * Best-effort: never fails the import. */ +int cbm_artifact_reconcile_hashes(const char *repo_path, const char *cache_db_path, + const char *project_name); + #endif /* CBM_ARTIFACT_H */ diff --git a/tests/test_artifact.c b/tests/test_artifact.c index 4dcffe0de..578337712 100644 --- a/tests/test_artifact.c +++ b/tests/test_artifact.c @@ -11,6 +11,8 @@ #include #include +#include +#include /* ── Helpers ─────────────────────────────────────────────────────── */ @@ -315,6 +317,287 @@ TEST(artifact_null_safety) { ASSERT_NEQ(cbm_artifact_import("/tmp", NULL), 0); ASSERT_FALSE(cbm_artifact_exists(NULL)); ASSERT_NULL(cbm_artifact_commit(NULL)); + ASSERT_EQ(cbm_artifact_reconcile_hashes(NULL, "/tmp/x.db", "p"), -1); + ASSERT_EQ(cbm_artifact_reconcile_hashes("/tmp", NULL, "p"), -1); + ASSERT_EQ(cbm_artifact_reconcile_hashes("/tmp", "/tmp/x.db", NULL), -1); + PASS(); +} + +/* ── Bootstrap reconciliation tests ───────────────────────────────── */ + +/* printf-formatted system() wrapper; returns the exit status. */ +static int runf(const char *fmt, ...) { + char cmd[4096]; + va_list ap; + va_start(ap, fmt); + int n = vsnprintf(cmd, sizeof(cmd), fmt, ap); + va_end(ap); + if (n < 0 || (size_t)n >= sizeof(cmd)) { + return -1; + } + return system(cmd); +} + +static void git_init(const char *repo) { + runf("git -C '%s' init -q", repo); + runf("git -C '%s' config user.email t@t.com", repo); + runf("git -C '%s' config user.name t", repo); + runf("git -C '%s' config commit.gpgsign false", repo); +} + +/* Portable local mtime_ns for assertions (mirrors art_stat_mtime_ns). */ +static int64_t t_mtime_ns(const char *path) { + struct stat st; + if (stat(path, &st) != 0) { + return -1; + } +#ifdef __APPLE__ + return (int64_t)st.st_mtimespec.tv_sec * 1000000000LL + (int64_t)st.st_mtimespec.tv_nsec; +#elif defined(__linux__) + return (int64_t)st.st_mtim.tv_sec * 1000000000LL + (int64_t)st.st_mtim.tv_nsec; +#else + return (int64_t)st.st_mtime * 1000000000LL; +#endif +} + +/* True iff /.codebase-memory/artifact.json contains substr. */ +static bool meta_contains(const char *repo, const char *substr) { + char path[1024]; + snprintf(path, sizeof(path), "%s/.codebase-memory/artifact.json", repo); + FILE *fp = fopen(path, "r"); + if (!fp) { + return false; + } + char buf[4096]; + size_t n = fread(buf, 1, sizeof(buf) - 1, fp); + fclose(fp); + buf[n] = '\0'; + return strstr(buf, substr) != NULL; +} + +static int64_t row_mtime(cbm_file_hash_t *rows, int n, const char *rel) { + for (int i = 0; i < n; i++) { + if (strcmp(rows[i].rel_path, rel) == 0) { + return rows[i].mtime_ns; + } + } + return -1; +} + +/* Build a git repo at g_repo with 5 .rs files, full-index + export (clean tree + * → reconcile_basis marker set), and commit .codebase-memory so a clone receives + * the artifact. Returns the derived project name (caller frees) or NULL. */ +static char *build_trusted_artifact_repo(void) { + git_init(g_repo); + const char *names[] = {"a.rs", "b.rs", "c.rs", "d.rs", "e.rs"}; + for (int i = 0; i < 5; i++) { + char p[1024]; + snprintf(p, sizeof(p), "%s/%s", g_repo, names[i]); + char body[128]; + snprintf(body, sizeof(body), "pub fn f%d() {}\n", i); + write_text_file(p, body); + } + if (runf("git -C '%s' add -A && git -C '%s' commit -qm init", g_repo, g_repo) != 0) { + return NULL; + } + cbm_pipeline_t *p = cbm_pipeline_new(g_repo, g_db, CBM_MODE_FAST); + if (!p) { + return NULL; + } + cbm_pipeline_set_persistence(p, true); + int rc = cbm_pipeline_run(p); + cbm_pipeline_free(p); + if (rc != 0) { + return NULL; + } + /* Clean source tree at export → marker must be present. */ + if (!meta_contains(g_repo, "\"reconcile_basis\"")) { + return NULL; + } + if (runf("git -C '%s' add -A && git -C '%s' commit -qm artifact", g_repo, g_repo) != 0) { + return NULL; + } + return cbm_project_name_from_path(g_repo); +} + +/* Clones g_repo (A) into /work/repo (B) so both share the basename "repo" + * and thus the derived project name — required for artifact bootstrap. */ +static bool clone_to_b(char *repoB_out, size_t repoB_sz) { + char work[1024]; + snprintf(work, sizeof(work), "%s/work", g_tmpdir); + cbm_mkdir_p(work, 0755); + snprintf(repoB_out, repoB_sz, "%s/repo", work); + return runf("git clone -q '%s' '%s'", g_repo, repoB_out) == 0; +} + +TEST(artifact_export_marks_clean_basis) { + setup_artifact_test(); + git_init(g_repo); + char src[1024]; + snprintf(src, sizeof(src), "%s/a.rs", g_repo); + write_text_file(src, "pub fn f0() {}\n"); + ASSERT_EQ(runf("git -C '%s' add -A && git -C '%s' commit -qm init", g_repo, g_repo), 0); + + char *proj = cbm_project_name_from_path(g_repo); + ASSERT_NOT_NULL(proj); + + /* Full index + export on a clean source tree → clean-basis marker set. */ + cbm_pipeline_t *p = cbm_pipeline_new(g_repo, g_db, CBM_MODE_FAST); + ASSERT_NOT_NULL(p); + cbm_pipeline_set_persistence(p, true); + ASSERT_EQ(cbm_pipeline_run(p), 0); + cbm_pipeline_free(p); + ASSERT(meta_contains(g_repo, "\"reconcile_basis\"")); + + /* Dirty the source tree (uncommitted edit). Re-export must omit the marker: + * the tree is non-clean outside .codebase-memory (and the a.rs hash row no + * longer matches disk). */ + write_text_file(src, "pub fn dirty() {}\n"); + ASSERT_EQ(cbm_artifact_export(g_db, g_repo, proj, CBM_ARTIFACT_FAST), 0); + ASSERT_FALSE(meta_contains(g_repo, "\"reconcile_basis\"")); + + free(proj); + cleanup_dir(g_tmpdir); + PASS(); +} + +TEST(artifact_reconcile_restamps_unchanged) { + setup_artifact_test(); + char *proj = build_trusted_artifact_repo(); + ASSERT_NOT_NULL(proj); + + char repoB[1024]; + ASSERT(clone_to_b(repoB, sizeof(repoB))); + + /* B: modify 2 files + add 1, commit. (Clone gave B fresh mtimes.) */ + char m1[1152], m2[1152], add[1152]; + snprintf(m1, sizeof(m1), "%s/a.rs", repoB); + snprintf(m2, sizeof(m2), "%s/b.rs", repoB); + snprintf(add, sizeof(add), "%s/added.rs", repoB); + write_text_file(m1, "pub fn f0() { /* changed */ }\n"); + write_text_file(m2, "pub fn f1() { /* changed */ }\n"); + write_text_file(add, "pub fn new_fn() {}\n"); + ASSERT_EQ(runf("git -C '%s' add -A && git -C '%s' commit -qm edits", repoB, repoB), 0); + + /* Import A's artifact into a fresh cache DB for B. */ + char dbB[1152]; + snprintf(dbB, sizeof(dbB), "%s/b.db", g_tmpdir); + ASSERT_EQ(cbm_artifact_import(repoB, dbB), 0); + + /* Reconcile: 5 rows; a.rs+b.rs changed (left foreign), 3 unchanged restamped. */ + int restamped = cbm_artifact_reconcile_hashes(repoB, dbB, proj); + ASSERT_EQ(restamped, 3); + + /* Unchanged row (c.rs) now carries B's local mtime; modified row (a.rs) + * still carries A's foreign mtime. */ + cbm_store_t *s = cbm_store_open_path(dbB); + ASSERT_NOT_NULL(s); + cbm_file_hash_t *rows = NULL; + int n = 0; + ASSERT_EQ(cbm_store_get_file_hashes(s, proj, &rows, &n), 0); + char c_path[1152], a_path[1152]; + snprintf(c_path, sizeof(c_path), "%s/c.rs", repoB); + snprintf(a_path, sizeof(a_path), "%s/a.rs", repoB); + ASSERT_EQ(row_mtime(rows, n, "c.rs"), t_mtime_ns(c_path)); + ASSERT_NEQ(row_mtime(rows, n, "a.rs"), t_mtime_ns(a_path)); + cbm_store_free_file_hashes(rows, n); + cbm_store_close(s); + + free(proj); + cleanup_dir(g_tmpdir); + PASS(); +} + +TEST(artifact_reconcile_skips_untrusted_metadata) { + setup_artifact_test(); + char *proj = build_trusted_artifact_repo(); + ASSERT_NOT_NULL(proj); + char repoB[1024]; + ASSERT(clone_to_b(repoB, sizeof(repoB))); + + char dbB[1152]; + snprintf(dbB, sizeof(dbB), "%s/b.db", g_tmpdir); + ASSERT_EQ(cbm_artifact_import(repoB, dbB), 0); + + /* Snapshot one foreign mtime, then strip the trust marker. */ + cbm_store_t *s = cbm_store_open_path(dbB); + cbm_file_hash_t *rows = NULL; + int n = 0; + cbm_store_get_file_hashes(s, proj, &rows, &n); + int64_t before = row_mtime(rows, n, "c.rs"); + cbm_store_free_file_hashes(rows, n); + cbm_store_close(s); + + char meta[1152]; + snprintf(meta, sizeof(meta), "%s/.codebase-memory/artifact.json", repoB); + /* Rewrite artifact.json without reconcile_basis (schema_version preserved). */ + FILE *fp = fopen(meta, "w"); + ASSERT_NOT_NULL(fp); + fprintf(fp, "{\"schema_version\":2,\"commit\":\"deadbeef\"," + "\"original_size\":1000,\"indexed_at\":\"2026-01-01T00:00:00Z\"}"); + fclose(fp); + + ASSERT_EQ(cbm_artifact_reconcile_hashes(repoB, dbB, proj), -1); + + /* Rows untouched: still the foreign value captured before. */ + s = cbm_store_open_path(dbB); + cbm_store_get_file_hashes(s, proj, &rows, &n); + ASSERT_EQ(row_mtime(rows, n, "c.rs"), before); + cbm_store_free_file_hashes(rows, n); + cbm_store_close(s); + + free(proj); + cleanup_dir(g_tmpdir); + PASS(); +} + +TEST(artifact_reconcile_skips_unknown_commit) { + setup_artifact_test(); + char *proj = build_trusted_artifact_repo(); + ASSERT_NOT_NULL(proj); + char repoB[1024]; + ASSERT(clone_to_b(repoB, sizeof(repoB))); + + char dbB[1152]; + snprintf(dbB, sizeof(dbB), "%s/b.db", g_tmpdir); + ASSERT_EQ(cbm_artifact_import(repoB, dbB), 0); + + /* Rewrite artifact.json: keep the marker but point commit at a hex-valid + * SHA that does not exist locally → cat-file -e fails. */ + char meta[1152]; + snprintf(meta, sizeof(meta), "%s/.codebase-memory/artifact.json", repoB); + FILE *fp = fopen(meta, "w"); + ASSERT_NOT_NULL(fp); + fprintf(fp, "{\"schema_version\":2,\"commit\":\"%s\"," + "\"original_size\":1000,\"reconcile_basis\":\"git-clean-head\"}", + "1111111111111111111111111111111111111111"); + fclose(fp); + + ASSERT_EQ(cbm_artifact_reconcile_hashes(repoB, dbB, proj), -1); + + free(proj); + cleanup_dir(g_tmpdir); + PASS(); +} + +TEST(artifact_reconcile_skips_without_git) { + setup_artifact_test(); + char *proj = build_trusted_artifact_repo(); + ASSERT_NOT_NULL(proj); + char repoB[1024]; + ASSERT(clone_to_b(repoB, sizeof(repoB))); + + char dbB[1152]; + snprintf(dbB, sizeof(dbB), "%s/b.db", g_tmpdir); + ASSERT_EQ(cbm_artifact_import(repoB, dbB), 0); + + /* Remove .git → not a repo. Marker + commit still present, but git fails. */ + ASSERT_EQ(runf("rm -rf '%s/.git'", repoB), 0); + + ASSERT_EQ(cbm_artifact_reconcile_hashes(repoB, dbB, proj), -1); + + free(proj); + cleanup_dir(g_tmpdir); PASS(); } @@ -329,4 +612,9 @@ SUITE(artifact) { RUN_TEST(artifact_export_rename_failure_logs_specific_error); RUN_TEST(pipeline_persistence_export_failure_returns_error); RUN_TEST(artifact_null_safety); + RUN_TEST(artifact_export_marks_clean_basis); + RUN_TEST(artifact_reconcile_restamps_unchanged); + RUN_TEST(artifact_reconcile_skips_untrusted_metadata); + RUN_TEST(artifact_reconcile_skips_unknown_commit); + RUN_TEST(artifact_reconcile_skips_without_git); } From e9d4190e750c721f74f3a4724205d19247134064 Mon Sep 17 00:00:00 2001 From: Greg Tiller Date: Sat, 4 Jul 2026 20:33:27 -0300 Subject: [PATCH 2/7] refactor(incremental): persist hashes in one batch from classify-time stamps persist_hashes re-stat()ed every discovered file AFTER the run and upserted rows one at a time (one implicit SQLite write transaction per file). Two problems: 1. Wasted work: classify_files had already stat()ed every file with a stored hash. persist_hashes stat()ed them all again. 2. Correctness: re-stat'ing AFTER the run records the post-run mtime, so a file edited mid-index is stamped "current" and the NEXT run misses the edit (classifies it unchanged). Using the classify-time stat is both faster and more correct. classify_files now outputs a cbm_file_stamp_t array (one stat per file at classify time, including for new files). persist_hashes builds one cbm_file_hash_t array from files[]+stamps[] plus mode_skipped[] and upserts it via the existing-but-unused cbm_store_upsert_file_hash_batch (single BEGIN...COMMIT). On batch failure (it rolls back on first row error) it falls back to a row-at-a-time loop so one bad row still can't nuke all persistence -- preserving the documented "partial preservation beats total loss" contract. Threading: stamps flows classify_files -> cbm_pipeline_run_incremental -> dump_and_persist -> persist_hashes (all static, file-local signature changes), freed at every return path (noop, load_db_failed, normal). Test: incremental_persist_batch_roundtrip exercises the batch path via the real pipeline route (full index -> modify -> incremental reindex -> noop) and asserts the persisted mtime equals the classify-time mtime. The existing incremental suite (incr_modify_file / incr_add_file / incr_delete_file / incr_noop_reindex, ~5900 cases) is the regression guard. Signed-off-by: Greg Tiller --- src/pipeline/pipeline_incremental.c | 189 +++++++++++++++++++--------- tests/test_artifact.c | 69 ++++++++++ 2 files changed, 199 insertions(+), 59 deletions(-) diff --git a/src/pipeline/pipeline_incremental.c b/src/pipeline/pipeline_incremental.c index 1fa320d86..805dc7ee1 100644 --- a/src/pipeline/pipeline_incremental.c +++ b/src/pipeline/pipeline_incremental.c @@ -70,13 +70,23 @@ static int64_t stat_mtime_ns(const struct stat *st) { #endif } +/* Classify-time file stamp captured exactly once per file. persist_hashes + * consumes these instead of re-stat()ing every file post-run (which both + * wasted work and masked edits made while indexing was running). */ +typedef struct { + int64_t mtime_ns; + int64_t size; + bool valid; +} cbm_file_stamp_t; + /* ── File classification ─────────────────────────────────────────── */ /* Classify discovered files against stored hashes using mtime+size. * Returns a boolean array: changed[i] = true if files[i] needs re-parsing. * Caller must free the returned array. */ static bool *classify_files(cbm_file_info_t *files, int file_count, cbm_file_hash_t *stored, - int stored_count, int *out_changed, int *out_unchanged) { + int stored_count, cbm_file_stamp_t *stamps, int *out_changed, + int *out_unchanged) { bool *changed = calloc((size_t)file_count, sizeof(bool)); if (!changed) { return NULL; @@ -92,7 +102,19 @@ static bool *classify_files(cbm_file_info_t *files, int file_count, cbm_file_has cbm_ht_set(ht, stored[i].rel_path, &stored[i]); } + /* Stat every file exactly once (at classify time) and record the stamp so + * persist_hashes can persist it without a second stat. The previous design + * re-stat()ed in persist_hashes, which both wasted work and masked mid-run + * edits (a post-run mtime hid a file edited while indexing was running). */ for (int i = 0; i < file_count; i++) { + struct stat st; + bool st_ok = (stat(files[i].path, &st) == 0); + if (stamps) { + stamps[i].valid = st_ok; + stamps[i].mtime_ns = st_ok ? stat_mtime_ns(&st) : 0; + stamps[i].size = st_ok ? (int64_t)st.st_size : 0; + } + cbm_file_hash_t *h = cbm_ht_get(ht, files[i].rel_path); if (!h) { /* New file */ @@ -101,14 +123,7 @@ static bool *classify_files(cbm_file_info_t *files, int file_count, cbm_file_has continue; } - struct stat st; - if (stat(files[i].path, &st) != 0) { - changed[i] = true; - n_changed++; - continue; - } - - if (stat_mtime_ns(&st) != h->mtime_ns || st.st_size != h->size) { + if (!st_ok || stat_mtime_ns(&st) != h->mtime_ns || (int64_t)st.st_size != h->size) { changed[i] = true; n_changed++; } else { @@ -433,33 +448,24 @@ static void incr_free_edge_capture(cbm_edge_capture_t *cap) { /* ── Persist file hashes ─────────────────────────────────────────── */ -/* Persist file hash rows for the current discovery and any mode-skipped - * files preserved from the previous DB. - * - * Partial-failure policy: an `upsert` failure on any single row is logged - * as a warning and the loop continues. We deliberately do NOT abort the - * whole reindex on a single bad row — partial preservation is better than - * total loss, and a transient failure on one file should not invalidate - * the entire incremental update. The trade-off is that a silently-failed - * row produces the same downstream effect as if the file were never - * indexed at all (forced re-parse on the next run for current-files, - * potential orphaned-node revival for mode_skipped). The warning surface - * is the only signal that something went wrong. */ -static void persist_hashes(cbm_store_t *store, const char *project, cbm_file_info_t *files, - int file_count, const cbm_file_hash_t *mode_skipped, - int mode_skipped_count) { +/* Row-at-a-time fallback used when the single-transaction batch upsert fails + * (the batch API rolls back on the first row error). Mirrors the historical + * path so one bad row still can't nuke all persistence. Uses classify-time + * stamps — no per-file re-stat (see persist_hashes for why that matters). */ +static void persist_hashes_row_by_row(cbm_store_t *store, const char *project, + cbm_file_info_t *files, int file_count, + const cbm_file_stamp_t *stamps, + const cbm_file_hash_t *mode_skipped, + int mode_skipped_count) { int current_failed = 0; int ms_failed = 0; - /* Current discovery: re-stat to capture any mtime/size that changed - * during the run, and write fresh hash rows for visited files. */ for (int i = 0; i < file_count; i++) { - struct stat st; - if (stat(files[i].path, &st) != 0) { + if (!stamps || !stamps[i].valid) { continue; } int rc = cbm_store_upsert_file_hash(store, project, files[i].rel_path, "", - stat_mtime_ns(&st), st.st_size); + stamps[i].mtime_ns, stamps[i].size); if (rc != CBM_STORE_OK) { cbm_log_warn("incremental.persist_hash_failed", "scope", "current", "rel_path", files[i].rel_path, "rc", itoa_buf(rc)); @@ -467,30 +473,16 @@ static void persist_hashes(cbm_store_t *store, const char *project, cbm_file_inf } } - /* Mode-skipped (preserved): re-upsert hash rows from the previous DB - * so the next reindex can still classify these files correctly. Without - * this, an orphaned-node bug emerges where: - * - full mode indexes everything - * - fast mode runs and drops mode-skipped hash rows - * - file is then deleted on disk - * - next reindex's stored hashes don't include the file → noop or - * can't detect the deletion → graph nodes for the deleted file - * remain forever (or until a destructive rebuild). - * - * A failure here is more serious than a current-files failure because - * it can revive the orphaned-node bug for that specific file. Logged - * with scope=mode_skipped so the warning is searchable. */ - if (mode_skipped) { - for (int i = 0; i < mode_skipped_count; i++) { - int rc = - cbm_store_upsert_file_hash(store, project, mode_skipped[i].rel_path, - mode_skipped[i].sha256 ? mode_skipped[i].sha256 : "", - mode_skipped[i].mtime_ns, mode_skipped[i].size); - if (rc != CBM_STORE_OK) { - cbm_log_warn("incremental.persist_hash_failed", "scope", "mode_skipped", "rel_path", - mode_skipped[i].rel_path, "rc", itoa_buf(rc)); - ms_failed++; - } + /* Mode-skipped rows are preserved verbatim so the next reindex can still + * classify them (see find_deleted_files for the orphaned-node guard). */ + for (int i = 0; i < mode_skipped_count; i++) { + int rc = cbm_store_upsert_file_hash(store, project, mode_skipped[i].rel_path, + mode_skipped[i].sha256 ? mode_skipped[i].sha256 : "", + mode_skipped[i].mtime_ns, mode_skipped[i].size); + if (rc != CBM_STORE_OK) { + cbm_log_warn("incremental.persist_hash_failed", "scope", "mode_skipped", "rel_path", + mode_skipped[i].rel_path, "rc", itoa_buf(rc)); + ms_failed++; } } @@ -500,6 +492,80 @@ static void persist_hashes(cbm_store_t *store, const char *project, cbm_file_inf } } +/* Persist file hash rows for the current discovery and any mode-skipped + * files preserved from the previous DB. + * + * Single transaction + one stat per file: classify_files already stat()ed + * every file (captured in `stamps`), so we build one cbm_file_hash_t array + * from files[]+stamps[] plus mode_skipped[] and upsert it via the batch API + * (one BEGIN…COMMIT). This replaces the prior row-at-a-time loop that + * re-stat()ed every file post-run — the re-stat was both wasted work AND a + * correctness bug: it stamped a file with its post-run mtime, masking any + * edit made while indexing was running so the next run missed it. + * Classify-time stamps fix that. + * + * Partial-failure policy: an `upsert` failure on any single row is logged + * as a warning and the loop continues. We deliberately do NOT abort the + * whole reindex on a single bad row — partial preservation is better than + * total loss, and a transient failure on one file should not invalidate + * the entire incremental update. The batch API rolls back on first failure, + * so on batch failure we fall back to persist_hashes_row_by_row to preserve + * that contract. The trade-off is that a silently-failed row produces the + * same downstream effect as if the file were never indexed at all (forced + * re-parse on the next run for current-files, potential orphaned-node + * revival for mode_skipped). The warning surface is the only signal that + * something went wrong. */ +static void persist_hashes(cbm_store_t *store, const char *project, cbm_file_info_t *files, + int file_count, const cbm_file_stamp_t *stamps, + const cbm_file_hash_t *mode_skipped, int mode_skipped_count) { + int total = file_count + mode_skipped_count; + if (total <= 0) { + return; + } + + cbm_file_hash_t *batch = malloc((size_t)total * sizeof(cbm_file_hash_t)); + if (!batch) { + /* OOM: fall back to row-at-a-time rather than dropping all persistence. */ + persist_hashes_row_by_row(store, project, files, file_count, stamps, mode_skipped, + mode_skipped_count); + return; + } + + int n = 0; + for (int i = 0; i < file_count; i++) { + if (!stamps || !stamps[i].valid) { + continue; /* couldn't stat at classify time → skip, re-parse next run */ + } + batch[n].project = project; + batch[n].rel_path = files[i].rel_path; + batch[n].sha256 = ""; /* column not yet populated at write sites */ + batch[n].mtime_ns = stamps[i].mtime_ns; + batch[n].size = stamps[i].size; + n++; + } + for (int i = 0; i < mode_skipped_count; i++) { + batch[n].project = project; + batch[n].rel_path = mode_skipped[i].rel_path; + batch[n].sha256 = mode_skipped[i].sha256 ? mode_skipped[i].sha256 : ""; + batch[n].mtime_ns = mode_skipped[i].mtime_ns; + batch[n].size = mode_skipped[i].size; + n++; + } + + if (n > 0) { + int rc = cbm_store_upsert_file_hash_batch(store, batch, n); + if (rc != CBM_STORE_OK) { + /* Batch rolls back on first row failure; fall back so one bad row + * can't lose all hash rows. */ + cbm_log_warn("incremental.persist_batch_failed", "rc", itoa_buf(rc), "rows", + itoa_buf(n)); + persist_hashes_row_by_row(store, project, files, file_count, stamps, mode_skipped, + mode_skipped_count); + } + } + free(batch); +} + /* ── Registry seed visitor ────────────────────────────────────────── */ /* Labels the full-index definition pass seeds into the registry @@ -630,8 +696,8 @@ static void run_postpasses(cbm_pipeline_ctx_t *ctx, cbm_file_info_t *changed_fil * not visited this pass". */ static void dump_and_persist(cbm_gbuf_t *gbuf, const char *db_path, const char *project, cbm_file_info_t *files, int file_count, - const cbm_file_hash_t *mode_skipped, int mode_skipped_count, - const char *repo_path) { + const cbm_file_stamp_t *stamps, const cbm_file_hash_t *mode_skipped, + int mode_skipped_count, const char *repo_path) { struct timespec t; cbm_clock_gettime(CLOCK_MONOTONIC, &t); @@ -649,7 +715,8 @@ static void dump_and_persist(cbm_gbuf_t *gbuf, const char *db_path, const char * cbm_store_t *hash_store = cbm_store_open_path(db_path); if (hash_store) { - persist_hashes(hash_store, project, files, file_count, mode_skipped, mode_skipped_count); + persist_hashes(hash_store, project, files, file_count, stamps, mode_skipped, + mode_skipped_count); /* FTS5 rebuild after incremental dump. The btree dump path bypasses * any triggers that could have kept nodes_fts synchronized, so we @@ -698,8 +765,9 @@ int cbm_pipeline_run_incremental(cbm_pipeline_t *p, const char *db_path, cbm_fil /* Classify files */ int n_changed = 0; int n_unchanged = 0; - bool *is_changed = - classify_files(files, file_count, stored, stored_count, &n_changed, &n_unchanged); + cbm_file_stamp_t *stamps = calloc((size_t)file_count, sizeof(cbm_file_stamp_t)); + bool *is_changed = classify_files(files, file_count, stored, stored_count, stamps, &n_changed, + &n_unchanged); /* Classify stored files absent from current discovery: truly-deleted * (purge) vs mode-skipped (preserve nodes AND hash rows). */ @@ -720,6 +788,7 @@ int cbm_pipeline_run_incremental(cbm_pipeline_t *p, const char *db_path, cbm_fil if (n_changed == 0 && deleted_count == 0) { cbm_log_info("incremental.noop", "reason", "no_changes"); free(is_changed); + free(stamps); free(deleted); free_mode_skipped(mode_skipped, mode_skipped_count); cbm_store_free_file_hashes(stored, stored_count); @@ -757,6 +826,7 @@ int cbm_pipeline_run_incremental(cbm_pipeline_t *p, const char *db_path, cbm_fil cbm_log_error("incremental.err", "msg", "load_db_failed"); cbm_gbuf_free(existing); free(changed_files); + free(stamps); for (int i = 0; i < deleted_count; i++) { free(deleted[i]); } @@ -866,8 +936,9 @@ int cbm_pipeline_run_incremental(cbm_pipeline_t *p, const char *db_path, cbm_fil * covers incremental reindexes, not just full ones. */ cbm_pipeline_set_committed_counts(p, cbm_gbuf_node_count(existing), cbm_gbuf_edge_count(existing)); - dump_and_persist(existing, db_path, project, files, file_count, mode_skipped, + dump_and_persist(existing, db_path, project, files, file_count, stamps, mode_skipped, mode_skipped_count, cbm_pipeline_repo_path(p)); + free(stamps); free_mode_skipped(mode_skipped, mode_skipped_count); cbm_gbuf_free(existing); diff --git a/tests/test_artifact.c b/tests/test_artifact.c index 578337712..3de9788a9 100644 --- a/tests/test_artifact.c +++ b/tests/test_artifact.c @@ -601,6 +601,74 @@ TEST(artifact_reconcile_skips_without_git) { PASS(); } +/* P2: persist_hashes now writes all hash rows in one batch transaction from + * classify-time stamps (no per-file re-stat). Verify the round-trip via the + * real pipeline route: index → modify → incremental reindex (batch persist) → + * the row reflects the new content, and a follow-up run classifies it as + * unchanged (proving the persisted stamps are usable). Offline synthetic repo. */ +TEST(incremental_persist_batch_roundtrip) { + setup_artifact_test(); + char src[1024]; + snprintf(src, sizeof(src), "%s/a.rs", g_repo); + write_text_file(src, "pub fn original() {}\n"); + + char *proj = cbm_project_name_from_path(g_repo); + ASSERT_NOT_NULL(proj); + + /* Run 1: full index → file_hashes populated (size = len(original)). */ + cbm_pipeline_t *p = cbm_pipeline_new(g_repo, g_db, CBM_MODE_FAST); + ASSERT_NOT_NULL(p); + ASSERT_EQ(cbm_pipeline_run(p), 0); + cbm_pipeline_free(p); + + cbm_store_t *s = cbm_store_open_path(g_db); + ASSERT_NOT_NULL(s); + cbm_file_hash_t *rows = NULL; + int n = 0; + ASSERT_EQ(cbm_store_get_file_hashes(s, proj, &rows, &n), 0); + ASSERT_EQ(n, 1); + int64_t size_before = rows[0].size; + cbm_store_free_file_hashes(rows, n); + cbm_store_close(s); + ASSERT_GT(size_before, 0); + + /* Modify the file (different size → detectable by classify on next run). */ + write_text_file(src, "pub fn original() { /* expanded with more content now */ }\n"); + + /* Run 2: routes to incremental → persist_hashes (batch) writes the new stamp. */ + p = cbm_pipeline_new(g_repo, g_db, CBM_MODE_FAST); + ASSERT_NOT_NULL(p); + ASSERT_EQ(cbm_pipeline_run(p), 0); + cbm_pipeline_free(p); + + s = cbm_store_open_path(g_db); + ASSERT_NOT_NULL(s); + ASSERT_EQ(cbm_store_get_file_hashes(s, proj, &rows, &n), 0); + ASSERT_EQ(n, 1); + int64_t size_after = rows[0].size; + /* Persisted mtime now equals the file's current (classify-time) mtime. */ + char c_path[1024]; + snprintf(c_path, sizeof(c_path), "%s/a.rs", g_repo); + ASSERT_EQ(rows[0].mtime_ns, t_mtime_ns(c_path)); + cbm_store_free_file_hashes(rows, n); + cbm_store_close(s); + ASSERT_NEQ(size_after, size_before); + + /* Run 3: no-op — the persisted classify-time stamp makes classify see no + * change, proving the batch-persisted row is correct and reusable. */ + capture_logs_start(); + p = cbm_pipeline_new(g_repo, g_db, CBM_MODE_FAST); + ASSERT_NOT_NULL(p); + ASSERT_EQ(cbm_pipeline_run(p), 0); + cbm_pipeline_free(p); + const char *logs = capture_logs_end(); + ASSERT(strstr(logs, "incremental.noop") != NULL); + + free(proj); + cleanup_dir(g_tmpdir); + PASS(); +} + SUITE(artifact) { RUN_TEST(artifact_export_fast_roundtrip); RUN_TEST(artifact_export_best_roundtrip); @@ -617,4 +685,5 @@ SUITE(artifact) { RUN_TEST(artifact_reconcile_skips_untrusted_metadata); RUN_TEST(artifact_reconcile_skips_unknown_commit); RUN_TEST(artifact_reconcile_skips_without_git); + RUN_TEST(incremental_persist_batch_roundtrip); } From 683bded361358712ea9a3246dc12712d1b07f929 Mon Sep 17 00:00:00 2001 From: Greg Tiller Date: Sat, 4 Jul 2026 20:54:31 -0300 Subject: [PATCH 3/7] feat(incremental): debounce artifact auto re-export MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit dump_and_persist re-exported the artifact on EVERY incremental run -- even a 1-file edit read the whole DB, zstd-compressed it, and rewrote .codebase-memory/graph.db.zst (+ metadata), churning the user's git working tree and burning O(graph) I/O/CPU proportional to the whole graph. Debounce: export only when changed_total (changed + deleted) >= CBM_ARTIFACT_EXPORT_MIN_CHANGES (default 10) OR the artifact's indexed_at is older than CBM_ARTIFACT_EXPORT_MAX_AGE_S (default 24h). Age comes from artifact.json's indexed_at via a new cbm_artifact_age_seconds() helper (parsed with a portable UTC epoch computation -- no libc timegm/timezone dependency), NOT the .zst filesystem mtime, because checkout/touch can make an old artifact look fresh. CBM_ARTIFACT_EXPORT_MIN_CHANGES=0 restores per-run export (today's behavior). The skip is logged (artifact.export_skipped changed= age_s=) so staleness stays observable ("no silent caps"). Safety argument for staleness: with P1's git reconcile in place, a stale artifact merely means bootstrap reconciles a slightly larger git diff -- cost degrades gracefully, correctness is unaffected. Tests: artifact_export_debounced (1 change → .zst inode mtime bit-identical + export_skipped logged) and artifact_export_forced_by_env (MIN_CHANGES=0 → .zst re-written). Both drive a real incremental run via the pipeline route. Signed-off-by: Greg Tiller --- src/pipeline/artifact.c | 61 ++++++++++++++++++ src/pipeline/artifact.h | 6 ++ src/pipeline/pipeline_incremental.c | 45 ++++++++++++-- tests/test_artifact.c | 96 +++++++++++++++++++++++++++++ 4 files changed, 204 insertions(+), 4 deletions(-) diff --git a/src/pipeline/artifact.c b/src/pipeline/artifact.c index 82c06a6cb..2271b13d6 100644 --- a/src/pipeline/artifact.c +++ b/src/pipeline/artifact.c @@ -237,6 +237,32 @@ static void iso_timestamp(char *buf, size_t bufsz) { (void)strftime(buf, bufsz, "%Y-%m-%dT%H:%M:%SZ", &tm); } +/* Parse a "YYYY-MM-DDTHH:MM:SSZ" (UTC) timestamp into unix epoch seconds. + * Returns -1 on any format/range mismatch. Computes epoch directly via + * days-from-civil so it is independent of libc timegm/timezone (portable + * across POSIX and Windows), matching the fixed format iso_timestamp emits. */ +static int64_t parse_iso8601_utc(const char *s) { + if (!s) { + return -1; + } + int Y, Mo, D, H, Mi, S; + char z = 0; + if (sscanf(s, "%d-%d-%dT%d:%d:%d%c", &Y, &Mo, &D, &H, &Mi, &S, &z) != 7 || z != 'Z') { + return -1; + } + if (Mo < 1 || Mo > 12 || D < 1 || D > 31 || H < 0 || H > 23 || Mi < 0 || Mi > 59 || S < 0 || + S > 60) { + return -1; + } + int64_t y = (int64_t)Y - (Mo <= 2); + int64_t era = (y >= 0 ? y : y - 399) / 400; + int64_t yoe = y - era * 400; + int64_t doy = (153 * (Mo + (Mo > 2 ? -3 : 9)) + 2) / 5 + D - 1; + int64_t doe = yoe * 365 + yoe / 4 - yoe / 100 + doy; + int64_t days = era * 146097 + doe - 719468; /* days since 1970-01-01 */ + return days * 86400 + (int64_t)H * 3600 + (int64_t)Mi * 60 + (int64_t)S; +} + /* ── Git + trust helpers for bootstrap reconciliation ────────────── */ /* Non-NULL sentinel value stored in a membership hash set (key presence is all @@ -905,6 +931,41 @@ bool cbm_artifact_exists(const char *repo_path) { /* ── Commit hash extraction ──────────────────────────────────────── */ +/* Age of the existing artifact in seconds (now - artifact.json indexed_at). + * Returns -1 when there is no artifact.json or indexed_at is absent/unparseable. + * Uses the metadata timestamp, NOT the .zst filesystem mtime, because + * checkout/touch operations can make an old artifact look fresh. */ +int64_t cbm_artifact_age_seconds(const char *repo_path) { + if (!repo_path) { + return -1; + } + char meta_path[CBM_SZ_4K]; + if (!artifact_path(meta_path, sizeof(meta_path), repo_path, CBM_ARTIFACT_META)) { + return -1; + } + size_t len = 0; + char *json = read_file_alloc(meta_path, &len); + if (!json) { + return -1; + } + yyjson_doc *doc = yyjson_read(json, len, 0); + free(json); + if (!doc) { + return -1; + } + yyjson_val *root = yyjson_doc_get_root(doc); + yyjson_val *val = yyjson_obj_get(root, "indexed_at"); + int64_t age = -1; + if (val) { + int64_t t = parse_iso8601_utc(yyjson_get_str(val)); + if (t >= 0) { + age = (int64_t)time(NULL) - t; + } + } + yyjson_doc_free(doc); + return age; +} + char *cbm_artifact_commit(const char *repo_path) { if (!repo_path) { return NULL; diff --git a/src/pipeline/artifact.h b/src/pipeline/artifact.h index 29df6b11c..b233bb921 100644 --- a/src/pipeline/artifact.h +++ b/src/pipeline/artifact.h @@ -10,6 +10,7 @@ #define CBM_ARTIFACT_H #include +#include /* Schema version — increment when DB schema changes (new tables/indexes). * Import refuses artifacts with schema_version > current. @@ -53,6 +54,11 @@ bool cbm_artifact_exists(const char *repo_path); * Returns NULL if artifact doesn't exist or has no commit field. */ char *cbm_artifact_commit(const char *repo_path); +/* Age of the existing artifact in seconds (now - artifact.json indexed_at). + * Returns -1 when there is no artifact.json or indexed_at is absent/unparseable. + * Uses the metadata timestamp, NOT the .zst filesystem mtime. */ +int64_t cbm_artifact_age_seconds(const char *repo_path); + /* After a successful import of a trusted clean-basis artifact, re-stamp * file_hashes rows for files whose content is unchanged between the artifact's * commit and the local working tree, using local stat() values. diff --git a/src/pipeline/pipeline_incremental.c b/src/pipeline/pipeline_incremental.c index 805dc7ee1..640ab9aea 100644 --- a/src/pipeline/pipeline_incremental.c +++ b/src/pipeline/pipeline_incremental.c @@ -79,6 +79,30 @@ typedef struct { bool valid; } cbm_file_stamp_t; +/* Artifact auto-export debounce (P3). A fast-mode artifact is re-exported on + * every incremental run by default; that churns the user's working tree and + * burns O(graph) I/O/CPU. Skip the export for small change sets unless the + * artifact's indexed_at is older than the max age. CBM_ARTIFACT_EXPORT_MIN_CHANGES=0 + * restores per-run export (today's behavior). */ +enum { + CBM_ARTIFACT_EXPORT_MIN_CHANGES_DEFAULT = 10, + CBM_ARTIFACT_EXPORT_MAX_AGE_S = 24 * 60 * 60, +}; + +static int export_min_changes(void) { + const char *raw = getenv("CBM_ARTIFACT_EXPORT_MIN_CHANGES"); + if (!raw || !raw[0]) { + return CBM_ARTIFACT_EXPORT_MIN_CHANGES_DEFAULT; + } + errno = 0; + char *end = NULL; + long v = strtol(raw, &end, 10); + if (errno != 0 || end == raw || *end != '\0' || v < 0) { + return CBM_ARTIFACT_EXPORT_MIN_CHANGES_DEFAULT; + } + return (int)v; +} + /* ── File classification ─────────────────────────────────────────── */ /* Classify discovered files against stored hashes using mtime+size. @@ -697,7 +721,7 @@ static void run_postpasses(cbm_pipeline_ctx_t *ctx, cbm_file_info_t *changed_fil static void dump_and_persist(cbm_gbuf_t *gbuf, const char *db_path, const char *project, cbm_file_info_t *files, int file_count, const cbm_file_stamp_t *stamps, const cbm_file_hash_t *mode_skipped, - int mode_skipped_count, const char *repo_path) { + int mode_skipped_count, const char *repo_path, int changed_total) { struct timespec t; cbm_clock_gettime(CLOCK_MONOTONIC, &t); @@ -735,9 +759,22 @@ static void dump_and_persist(cbm_gbuf_t *gbuf, const char *db_path, const char * cbm_store_close(hash_store); } - /* Auto-update artifact if one already exists (persistence was enabled previously) */ + /* Auto-update artifact if one already exists (persistence was enabled + * previously). Debounced: skip for small change sets unless the artifact's + * indexed_at is older than the max age. Staleness is cheap with P1's + * git reconcile (a slightly larger diff to reconcile on bootstrap); the + * skip is logged so staleness stays observable ("no silent caps"), and + * CBM_ARTIFACT_EXPORT_MIN_CHANGES=0 forces per-run export. */ if (repo_path && cbm_artifact_exists(repo_path)) { - cbm_artifact_export(db_path, repo_path, project, CBM_ARTIFACT_FAST); + int min_changes = export_min_changes(); + int64_t age_s = cbm_artifact_age_seconds(repo_path); + bool age_expired = (age_s >= 0 && age_s >= CBM_ARTIFACT_EXPORT_MAX_AGE_S); + if (changed_total >= min_changes || age_expired) { + cbm_artifact_export(db_path, repo_path, project, CBM_ARTIFACT_FAST); + } else { + cbm_log_info("artifact.export_skipped", "changed", itoa_buf(changed_total), "age_s", + itoa_buf((int)age_s)); + } } } @@ -937,7 +974,7 @@ int cbm_pipeline_run_incremental(cbm_pipeline_t *p, const char *db_path, cbm_fil cbm_pipeline_set_committed_counts(p, cbm_gbuf_node_count(existing), cbm_gbuf_edge_count(existing)); dump_and_persist(existing, db_path, project, files, file_count, stamps, mode_skipped, - mode_skipped_count, cbm_pipeline_repo_path(p)); + mode_skipped_count, cbm_pipeline_repo_path(p), n_changed + deleted_count); free(stamps); free_mode_skipped(mode_skipped, mode_skipped_count); cbm_gbuf_free(existing); diff --git a/tests/test_artifact.c b/tests/test_artifact.c index 3de9788a9..8e6f23b86 100644 --- a/tests/test_artifact.c +++ b/tests/test_artifact.c @@ -601,6 +601,100 @@ TEST(artifact_reconcile_skips_without_git) { PASS(); } +/* P3: artifact auto-export is debounced. With a small change set (< default + * threshold 10) and a fresh artifact (age < 24h) the incremental run must NOT + * re-export — the .zst inode mtime is unchanged and artifact.export_skipped is + * logged. CBM_ARTIFACT_EXPORT_MIN_CHANGES=0 forces per-run export (today's + * behavior). Nanosecond mtime compare needs no sleep: an untouched inode's + * mtime is bit-identical. */ +TEST(artifact_export_debounced) { + setup_artifact_test(); + unsetenv("CBM_ARTIFACT_EXPORT_MIN_CHANGES"); + git_init(g_repo); + char gi[1024]; + snprintf(gi, sizeof(gi), "%s/.gitignore", g_repo); + write_text_file(gi, ".codebase-memory/\n"); + const char *fnames[] = {"a.rs", "b.rs", "c.rs", "d.rs"}; + for (int i = 0; i < 4; i++) { + char p[1024]; + snprintf(p, sizeof(p), "%s/%s", g_repo, fnames[i]); + char body[64]; + snprintf(body, sizeof(body), "pub fn f%d() {}\n", i); + write_text_file(p, body); + } + char src[1024]; + snprintf(src, sizeof(src), "%s/a.rs", g_repo); + + /* Run 1: full index + export → artifact exists, indexed_at = now. */ + cbm_pipeline_t *p = cbm_pipeline_new(g_repo, g_db, CBM_MODE_FAST); + ASSERT_NOT_NULL(p); + cbm_pipeline_set_persistence(p, true); + ASSERT_EQ(cbm_pipeline_run(p), 0); + cbm_pipeline_free(p); + + char zst[1152]; + snprintf(zst, sizeof(zst), "%s/.codebase-memory/graph.db.zst", g_repo); + int64_t mt0 = t_mtime_ns(zst); + ASSERT_GTE(mt0, 0); + + /* Modify 1 file → incremental run, changed_total = 1 (< 10), age ~0 → skip. */ + write_text_file(src, "pub fn f0() { /* modified */ }\n"); + capture_logs_start(); + p = cbm_pipeline_new(g_repo, g_db, CBM_MODE_FAST); + ASSERT_NOT_NULL(p); + ASSERT_EQ(cbm_pipeline_run(p), 0); + cbm_pipeline_free(p); + const char *logs = capture_logs_end(); + + ASSERT_EQ(t_mtime_ns(zst), mt0); /* untouched */ + ASSERT(strstr(logs, "artifact.export_skipped") != NULL); + + cleanup_dir(g_tmpdir); + PASS(); +} + +TEST(artifact_export_forced_by_env) { + setup_artifact_test(); + git_init(g_repo); + char gi[1024]; + snprintf(gi, sizeof(gi), "%s/.gitignore", g_repo); + write_text_file(gi, ".codebase-memory/\n"); + const char *fnames[] = {"a.rs", "b.rs", "c.rs", "d.rs"}; + for (int i = 0; i < 4; i++) { + char p[1024]; + snprintf(p, sizeof(p), "%s/%s", g_repo, fnames[i]); + char body[64]; + snprintf(body, sizeof(body), "pub fn f%d() {}\n", i); + write_text_file(p, body); + } + char src[1024]; + snprintf(src, sizeof(src), "%s/a.rs", g_repo); + + cbm_pipeline_t *p = cbm_pipeline_new(g_repo, g_db, CBM_MODE_FAST); + ASSERT_NOT_NULL(p); + cbm_pipeline_set_persistence(p, true); + ASSERT_EQ(cbm_pipeline_run(p), 0); + cbm_pipeline_free(p); + + char zst[1152]; + snprintf(zst, sizeof(zst), "%s/.codebase-memory/graph.db.zst", g_repo); + int64_t mt0 = t_mtime_ns(zst); + + /* CBM_ARTIFACT_EXPORT_MIN_CHANGES=0 forces export even for 1 change. */ + ASSERT_EQ(setenv("CBM_ARTIFACT_EXPORT_MIN_CHANGES", "0", 1), 0); + write_text_file(src, "pub fn f0() { /* modified */ }\n"); + p = cbm_pipeline_new(g_repo, g_db, CBM_MODE_FAST); + ASSERT_NOT_NULL(p); + ASSERT_EQ(cbm_pipeline_run(p), 0); + cbm_pipeline_free(p); + unsetenv("CBM_ARTIFACT_EXPORT_MIN_CHANGES"); + + ASSERT_NEQ(t_mtime_ns(zst), mt0); /* re-exported */ + + cleanup_dir(g_tmpdir); + PASS(); +} + /* P2: persist_hashes now writes all hash rows in one batch transaction from * classify-time stamps (no per-file re-stat). Verify the round-trip via the * real pipeline route: index → modify → incremental reindex (batch persist) → @@ -686,4 +780,6 @@ SUITE(artifact) { RUN_TEST(artifact_reconcile_skips_unknown_commit); RUN_TEST(artifact_reconcile_skips_without_git); RUN_TEST(incremental_persist_batch_roundtrip); + RUN_TEST(artifact_export_debounced); + RUN_TEST(artifact_export_forced_by_env); } From 2e4990624d58f19fc41e58a1404a5103f57f8f05 Mon Sep 17 00:00:00 2001 From: Greg Tiller Date: Sun, 5 Jul 2026 09:08:37 -0300 Subject: [PATCH 4/7] fix(artifact): restrict reconcile restamp to commit-tracked paths git diff/ls-files are blind to gitignored files, but a gitignored file can still be indexed (.cbmignore negation un-skipping a generated dir, #500) and thus carry a file_hashes row. Without a tracked-set gate such a row would be restamped as "unchanged" even though git cannot vouch for its content, leaving stale graph data after bootstrap. Reconciliation now also captures git ls-tree -r -z --name-only and restamps only rows tracked at the artifact commit AND absent from the changed set; everything git cannot vouch for stays foreign and is re-parsed. Same validation funnel (hex-validated commit, shell-validated repo path, NUL- delimited parse with truncation guard); allowlist entry text extended to mention ls-tree. Adds regression test artifact_reconcile_skips_untracked_rows. Refs #867 Signed-off-by: Greg Tiller --- scripts/security-allowlist.txt | 2 +- src/pipeline/artifact.c | 44 +++++++++++++++++++++++------ tests/test_artifact.c | 51 ++++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 9 deletions(-) diff --git a/scripts/security-allowlist.txt b/scripts/security-allowlist.txt index dc552dab0..b413b13ba 100644 --- a/scripts/security-allowlist.txt +++ b/scripts/security-allowlist.txt @@ -41,7 +41,7 @@ src/pipeline/pass_githistory.c:popen:via cbm_popen wrapper call # ── Pipeline: artifact persistence (git HEAD hash, merge driver config) ──── src/pipeline/artifact.c:cbm_popen:git rev-parse HEAD for artifact metadata (hardcoded cmd) src/pipeline/artifact.c:cbm_popen:git config merge.ours.driver for gitattributes (hardcoded cmd) -src/pipeline/artifact.c:cbm_popen:git diff/ls-files/cat-file for bootstrap hash reconciliation (commit hex-validated via is_hex_oid, repo path shell-validated via cbm_validate_shell_arg) +src/pipeline/artifact.c:cbm_popen:git diff/ls-files/ls-tree/cat-file for bootstrap hash reconciliation (commit hex-validated via is_hex_oid, repo path shell-validated via cbm_validate_shell_arg) src/pipeline/artifact.c:popen:via cbm_popen wrapper calls # ── UI: HTTP server process management ───────────────────────────────────── diff --git a/src/pipeline/artifact.c b/src/pipeline/artifact.c index 82c06a6cb..aa64055af 100644 --- a/src/pipeline/artifact.c +++ b/src/pipeline/artifact.c @@ -973,6 +973,10 @@ static void reconcile_add_nul_entries(CBMHashTable *ht, const char *buf, size_t * incremental run then classifies by actual git diff instead of re-parsing * ~every file (whose stored mtime_ns is the exporter's, not local). * + * Only rows tracked at the artifact commit are eligible: files git ignores can + * still be indexed (.cbmignore negations, #500), and git cannot vouch for their + * content — they stay foreign and are re-parsed. + * * Returns the number of rows re-stamped, or -1 when reconciliation was skipped * (no git / untrusted metadata / unknown or non-hex commit / shallow clone / * any popen or parse uncertainty). Best-effort: never fails the import — a -1 @@ -1009,15 +1013,25 @@ int cbm_artifact_reconcile_hashes(const char *repo_path, const char *cache_db_pa return CBM_NOT_FOUND; } - /* 3. Build the changed set relative to the artifact commit. NUL-delimited - * (-z) output is parsed directly; line-oriented parsing cannot handle - * paths containing newlines or quotes. */ + /* 3. Build the changed set relative to the artifact commit, plus the set of + * paths tracked AT that commit. NUL-delimited (-z) output is parsed + * directly; line-oriented parsing cannot handle paths containing + * newlines or quotes. The tracked set exists because git diff/ls-files + * are blind to files git ignores: a gitignored-yet-indexed file (e.g. a + * .cbmignore negation un-skipping a generated dir, #500) would otherwise + * be restamped as "unchanged" while its content is not under git's + * control at all. Only rows tracked at the artifact commit are eligible + * for restamping; everything git can't vouch for stays foreign. */ char diff_args[CBM_SZ_256]; snprintf(diff_args, sizeof(diff_args), "diff -z --name-only --no-renames %s --", commit); + char lstree_args[CBM_SZ_256]; + snprintf(lstree_args, sizeof(lstree_args), "ls-tree -r -z --name-only %s --", commit); char *diff_out = NULL; size_t diff_len = 0; char *ls_out = NULL; size_t ls_len = 0; + char *tracked_out = NULL; + size_t tracked_len = 0; if (git_capture_full(repo_path, diff_args, &diff_out, &diff_len) != 0) { free(commit); return CBM_NOT_FOUND; @@ -1028,14 +1042,22 @@ int cbm_artifact_reconcile_hashes(const char *repo_path, const char *cache_db_pa free(commit); return CBM_NOT_FOUND; } + if (git_capture_full(repo_path, lstree_args, &tracked_out, &tracked_len) != 0) { + free(diff_out); + free(ls_out); + free(commit); + return CBM_NOT_FOUND; + } /* Parse-invariant guard: git -z NUL-terminates every entry including the * last; a non-empty buffer not ending in NUL means truncated/corrupt output * → untrusted → skip rather than risk misclassifying a changed file as * unchanged (graph corruption). */ if ((diff_len > 0 && diff_out[diff_len - 1] != '\0') || - (ls_len > 0 && ls_out[ls_len - 1] != '\0')) { + (ls_len > 0 && ls_out[ls_len - 1] != '\0') || + (tracked_len > 0 && tracked_out[tracked_len - 1] != '\0')) { free(diff_out); free(ls_out); + free(tracked_out); free(commit); return CBM_NOT_FOUND; } @@ -1044,10 +1066,13 @@ int cbm_artifact_reconcile_hashes(const char *repo_path, const char *cache_db_pa reconcile_add_nul_entries(changed, diff_out, diff_len); reconcile_add_nul_entries(changed, ls_out, ls_len); int changed_count = (int)cbm_ht_count(changed); + CBMHashTable *tracked = cbm_ht_create(CBM_SZ_64); + reconcile_add_nul_entries(tracked, tracked_out, tracked_len); - /* 4. Restamp every row whose rel_path is NOT in the changed set with local - * stat() values. Changed rows stay foreign → re-parsed; rows whose file - * is missing locally stay foreign → find_deleted_files purges them. */ + /* 4. Restamp every row whose rel_path IS tracked at the artifact commit and + * is NOT in the changed set, with local stat() values. Changed or + * untracked rows stay foreign → re-parsed; rows whose file is missing + * locally stay foreign → find_deleted_files purges them. */ cbm_store_t *store = cbm_store_open_path(cache_db_path); int restamped = 0; int skipped = 0; @@ -1061,7 +1086,8 @@ int cbm_artifact_reconcile_hashes(const char *repo_path, const char *cache_db_pa if (batch) { int batch_n = 0; for (int i = 0; i < stored_count; i++) { - if (cbm_ht_get(changed, stored[i].rel_path)) { + if (!cbm_ht_get(tracked, stored[i].rel_path) || + cbm_ht_get(changed, stored[i].rel_path)) { continue; } char abs[CBM_SZ_4K]; @@ -1097,8 +1123,10 @@ int cbm_artifact_reconcile_hashes(const char *repo_path, const char *cache_db_pa } cbm_ht_free(changed); + cbm_ht_free(tracked); free(diff_out); free(ls_out); + free(tracked_out); free(commit); cbm_log_info("artifact.reconcile", "restamped", itoa_buf(restamped), "changed", diff --git a/tests/test_artifact.c b/tests/test_artifact.c index 578337712..6f099a01e 100644 --- a/tests/test_artifact.c +++ b/tests/test_artifact.c @@ -508,6 +508,56 @@ TEST(artifact_reconcile_restamps_unchanged) { PASS(); } +TEST(artifact_reconcile_skips_untracked_rows) { + setup_artifact_test(); + char *proj = build_trusted_artifact_repo(); + ASSERT_NOT_NULL(proj); + char repoB[1024]; + ASSERT(clone_to_b(repoB, sizeof(repoB))); + + /* B: a gitignored-yet-indexed file (the .cbmignore-negation shape, #500). + * git diff/ls-files are both blind to it, so without the tracked-at-commit + * gate it would be restamped as "unchanged" even though git cannot vouch + * for its content. */ + char gen[1152], gi[1152]; + snprintf(gen, sizeof(gen), "%s/gen.rs", repoB); + snprintf(gi, sizeof(gi), "%s/.gitignore", repoB); + write_text_file(gen, "pub fn generated_local() {}\n"); + write_text_file(gi, "gen.rs\n"); + ASSERT_EQ(runf("git -C '%s' add .gitignore && git -C '%s' commit -qm ignore", repoB, repoB), + 0); + + char dbB[1152]; + snprintf(dbB, sizeof(dbB), "%s/b.db", g_tmpdir); + ASSERT_EQ(cbm_artifact_import(repoB, dbB), 0); + + /* Simulate the exporter having indexed gen.rs: insert a foreign-mtime row. */ + const int64_t foreign_mtime = 12345; + cbm_store_t *s = cbm_store_open_path(dbB); + ASSERT_NOT_NULL(s); + ASSERT_EQ(cbm_store_upsert_file_hash(s, proj, "gen.rs", "", foreign_mtime, 7), CBM_STORE_OK); + cbm_store_close(s); + + /* Reconcile: the 5 tracked unchanged rows restamp; gen.rs must not. */ + ASSERT_EQ(cbm_artifact_reconcile_hashes(repoB, dbB, proj), 5); + + s = cbm_store_open_path(dbB); + ASSERT_NOT_NULL(s); + cbm_file_hash_t *rows = NULL; + int n = 0; + ASSERT_EQ(cbm_store_get_file_hashes(s, proj, &rows, &n), CBM_STORE_OK); + ASSERT_EQ(row_mtime(rows, n, "gen.rs"), foreign_mtime); + char c_path[1152]; + snprintf(c_path, sizeof(c_path), "%s/c.rs", repoB); + ASSERT_EQ(row_mtime(rows, n, "c.rs"), t_mtime_ns(c_path)); + cbm_store_free_file_hashes(rows, n); + cbm_store_close(s); + + free(proj); + cleanup_dir(g_tmpdir); + PASS(); +} + TEST(artifact_reconcile_skips_untrusted_metadata) { setup_artifact_test(); char *proj = build_trusted_artifact_repo(); @@ -614,6 +664,7 @@ SUITE(artifact) { RUN_TEST(artifact_null_safety); RUN_TEST(artifact_export_marks_clean_basis); RUN_TEST(artifact_reconcile_restamps_unchanged); + RUN_TEST(artifact_reconcile_skips_untracked_rows); RUN_TEST(artifact_reconcile_skips_untrusted_metadata); RUN_TEST(artifact_reconcile_skips_unknown_commit); RUN_TEST(artifact_reconcile_skips_without_git); From bbdb97e0dad70a11e6c5d102641384e9cb86f607 Mon Sep 17 00:00:00 2001 From: Greg Tiller Date: Sun, 5 Jul 2026 09:15:05 -0300 Subject: [PATCH 5/7] fix(incremental): export on cumulative drift; add max-age env override MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A per-run change threshold alone has a staleness hole: many consecutive below-threshold runs (each < 10 changes) never re-export, leaving the artifact stale for up to the age cap despite a materially changed index. The debounce now also computes cumulative drift — discovered files whose classify-time mtime lands in a later second than the artifact's indexed_at — from the stamps already produced by classify_files (zero extra I/O, no persisted counter). Files touched since the last export keep local mtimes newer than indexed_at, so drift accumulates across runs; export fires when this run's changes >= threshold OR drift >= threshold OR age >= max. Over-counting (touched-but-unchanged files) only exports more often — the safe direction. Drift is included in the artifact.export_skipped log line. Also adds CBM_ARTIFACT_EXPORT_MAX_AGE_S (same strict parse as CBM_ARTIFACT_EXPORT_MIN_CHANGES) so both debounce knobs are overridable, and factors cbm_artifact_indexed_at_epoch out of cbm_artifact_age_seconds. Tests: artifact_export_forced_by_cumulative_drift, artifact_export_forced_by_max_age_env. Refs #867 Signed-off-by: Greg Tiller --- src/pipeline/artifact.c | 23 +++--- src/pipeline/artifact.h | 6 +- src/pipeline/pipeline_incremental.c | 63 ++++++++++++---- tests/test_artifact.c | 108 ++++++++++++++++++++++++++++ 4 files changed, 178 insertions(+), 22 deletions(-) diff --git a/src/pipeline/artifact.c b/src/pipeline/artifact.c index 2023eae0c..436f2a66d 100644 --- a/src/pipeline/artifact.c +++ b/src/pipeline/artifact.c @@ -931,11 +931,11 @@ bool cbm_artifact_exists(const char *repo_path) { /* ── Commit hash extraction ──────────────────────────────────────── */ -/* Age of the existing artifact in seconds (now - artifact.json indexed_at). +/* Unix epoch seconds of the artifact's indexed_at metadata timestamp. * Returns -1 when there is no artifact.json or indexed_at is absent/unparseable. * Uses the metadata timestamp, NOT the .zst filesystem mtime, because * checkout/touch operations can make an old artifact look fresh. */ -int64_t cbm_artifact_age_seconds(const char *repo_path) { +int64_t cbm_artifact_indexed_at_epoch(const char *repo_path) { if (!repo_path) { return -1; } @@ -955,15 +955,22 @@ int64_t cbm_artifact_age_seconds(const char *repo_path) { } yyjson_val *root = yyjson_doc_get_root(doc); yyjson_val *val = yyjson_obj_get(root, "indexed_at"); - int64_t age = -1; + int64_t epoch = -1; if (val) { - int64_t t = parse_iso8601_utc(yyjson_get_str(val)); - if (t >= 0) { - age = (int64_t)time(NULL) - t; - } + epoch = parse_iso8601_utc(yyjson_get_str(val)); } yyjson_doc_free(doc); - return age; + return epoch; +} + +/* Age of the existing artifact in seconds (now - artifact.json indexed_at). + * Returns -1 when the indexed_at epoch is unavailable. */ +int64_t cbm_artifact_age_seconds(const char *repo_path) { + int64_t epoch = cbm_artifact_indexed_at_epoch(repo_path); + if (epoch < 0) { + return -1; + } + return (int64_t)time(NULL) - epoch; } char *cbm_artifact_commit(const char *repo_path) { diff --git a/src/pipeline/artifact.h b/src/pipeline/artifact.h index b233bb921..e60fced72 100644 --- a/src/pipeline/artifact.h +++ b/src/pipeline/artifact.h @@ -54,9 +54,13 @@ bool cbm_artifact_exists(const char *repo_path); * Returns NULL if artifact doesn't exist or has no commit field. */ char *cbm_artifact_commit(const char *repo_path); -/* Age of the existing artifact in seconds (now - artifact.json indexed_at). +/* Unix epoch seconds of the artifact's indexed_at metadata timestamp. * Returns -1 when there is no artifact.json or indexed_at is absent/unparseable. * Uses the metadata timestamp, NOT the .zst filesystem mtime. */ +int64_t cbm_artifact_indexed_at_epoch(const char *repo_path); + +/* Age of the existing artifact in seconds (now - artifact.json indexed_at). + * Returns -1 when the indexed_at epoch is unavailable. */ int64_t cbm_artifact_age_seconds(const char *repo_path); /* After a successful import of a trusted clean-basis artifact, re-stamp diff --git a/src/pipeline/pipeline_incremental.c b/src/pipeline/pipeline_incremental.c index 640ab9aea..99e759684 100644 --- a/src/pipeline/pipeline_incremental.c +++ b/src/pipeline/pipeline_incremental.c @@ -27,6 +27,7 @@ enum { INCR_RING_BUF = 4, INCR_RING_MASK = 3, INCR_TS_BUF = 24, INCR_WAL_BUF = 1 #include "foundation/platform.h" #include +#include #include #include #include @@ -82,27 +83,41 @@ typedef struct { /* Artifact auto-export debounce (P3). A fast-mode artifact is re-exported on * every incremental run by default; that churns the user's working tree and * burns O(graph) I/O/CPU. Skip the export for small change sets unless the - * artifact's indexed_at is older than the max age. CBM_ARTIFACT_EXPORT_MIN_CHANGES=0 - * restores per-run export (today's behavior). */ + * cumulative drift since the last export crosses the same threshold or the + * artifact's indexed_at is older than the max age. + * CBM_ARTIFACT_EXPORT_MIN_CHANGES=0 restores per-run export (today's + * behavior); CBM_ARTIFACT_EXPORT_MAX_AGE_S overrides the age cap. */ enum { CBM_ARTIFACT_EXPORT_MIN_CHANGES_DEFAULT = 10, - CBM_ARTIFACT_EXPORT_MAX_AGE_S = 24 * 60 * 60, + CBM_ARTIFACT_EXPORT_MAX_AGE_S_DEFAULT = 24 * 60 * 60, }; -static int export_min_changes(void) { - const char *raw = getenv("CBM_ARTIFACT_EXPORT_MIN_CHANGES"); +/* Non-negative integer env override with a strict parse; any missing, empty, + * non-numeric, trailing-garbage, or negative value falls back to def. */ +static int env_override_nonneg(const char *name, int def) { + const char *raw = getenv(name); if (!raw || !raw[0]) { - return CBM_ARTIFACT_EXPORT_MIN_CHANGES_DEFAULT; + return def; } errno = 0; char *end = NULL; long v = strtol(raw, &end, 10); - if (errno != 0 || end == raw || *end != '\0' || v < 0) { - return CBM_ARTIFACT_EXPORT_MIN_CHANGES_DEFAULT; + if (errno != 0 || end == raw || *end != '\0' || v < 0 || v > INT_MAX) { + return def; } return (int)v; } +static int export_min_changes(void) { + return env_override_nonneg("CBM_ARTIFACT_EXPORT_MIN_CHANGES", + CBM_ARTIFACT_EXPORT_MIN_CHANGES_DEFAULT); +} + +static int export_max_age_s(void) { + return env_override_nonneg("CBM_ARTIFACT_EXPORT_MAX_AGE_S", + CBM_ARTIFACT_EXPORT_MAX_AGE_S_DEFAULT); +} + /* ── File classification ─────────────────────────────────────────── */ /* Classify discovered files against stored hashes using mtime+size. @@ -760,7 +775,8 @@ static void dump_and_persist(cbm_gbuf_t *gbuf, const char *db_path, const char * } /* Auto-update artifact if one already exists (persistence was enabled - * previously). Debounced: skip for small change sets unless the artifact's + * previously). Debounced: skip for small change sets unless cumulative + * drift since the last export crosses the same threshold or the artifact's * indexed_at is older than the max age. Staleness is cheap with P1's * git reconcile (a slightly larger diff to reconcile on bootstrap); the * skip is logged so staleness stays observable ("no silent caps"), and @@ -768,12 +784,33 @@ static void dump_and_persist(cbm_gbuf_t *gbuf, const char *db_path, const char * if (repo_path && cbm_artifact_exists(repo_path)) { int min_changes = export_min_changes(); int64_t age_s = cbm_artifact_age_seconds(repo_path); - bool age_expired = (age_s >= 0 && age_s >= CBM_ARTIFACT_EXPORT_MAX_AGE_S); - if (changed_total >= min_changes || age_expired) { + bool age_expired = (age_s >= 0 && age_s >= export_max_age_s()); + + /* Cumulative drift: files whose classify-time mtime lands in a later + * second than the artifact's indexed_at. A per-run change count alone + * would let many consecutive small runs leave the artifact stale + * forever short of the age cap; files touched since the last export + * keep mtimes newer than indexed_at, so this accumulates across runs + * with no persisted counter. Strictly-later-second comparison keeps + * files written within the export's own second (the whole tree during + * a fast full index) from counting as drift; over-counting + * (touched-but-unchanged files) only exports more often — safe. */ + int drift = 0; + int64_t exported_epoch = cbm_artifact_indexed_at_epoch(repo_path); + if (exported_epoch >= 0 && stamps) { + const int64_t next_sec_ns = (exported_epoch + 1) * CBM_NS_PER_SEC; + for (int i = 0; i < file_count; i++) { + if (stamps[i].valid && stamps[i].mtime_ns >= next_sec_ns) { + drift++; + } + } + } + + if (changed_total >= min_changes || drift >= min_changes || age_expired) { cbm_artifact_export(db_path, repo_path, project, CBM_ARTIFACT_FAST); } else { - cbm_log_info("artifact.export_skipped", "changed", itoa_buf(changed_total), "age_s", - itoa_buf((int)age_s)); + cbm_log_info("artifact.export_skipped", "changed", itoa_buf(changed_total), "drift", + itoa_buf(drift), "age_s", itoa_buf((int)age_s)); } } } diff --git a/tests/test_artifact.c b/tests/test_artifact.c index e95e946cc..64e015776 100644 --- a/tests/test_artifact.c +++ b/tests/test_artifact.c @@ -745,6 +745,112 @@ TEST(artifact_export_forced_by_env) { PASS(); } +/* P3: cumulative drift forces export. Many consecutive below-threshold runs + * must not leave the artifact stale: files touched since the last export keep + * mtimes newer than indexed_at, so their count accumulates across runs and + * crosses the same threshold a single big run would. */ +TEST(artifact_export_forced_by_cumulative_drift) { + setup_artifact_test(); + unsetenv("CBM_ARTIFACT_EXPORT_MIN_CHANGES"); + unsetenv("CBM_ARTIFACT_EXPORT_MAX_AGE_S"); + git_init(g_repo); + char gi[1024]; + snprintf(gi, sizeof(gi), "%s/.gitignore", g_repo); + write_text_file(gi, ".codebase-memory/\n"); + char paths[12][1024]; + for (int i = 0; i < 12; i++) { + snprintf(paths[i], sizeof(paths[i]), "%s/f%d.rs", g_repo, i); + char body[64]; + snprintf(body, sizeof(body), "pub fn f%d() {}\n", i); + write_text_file(paths[i], body); + } + + /* Full index + export → artifact exists, indexed_at = now. */ + cbm_pipeline_t *p = cbm_pipeline_new(g_repo, g_db, CBM_MODE_FAST); + ASSERT_NOT_NULL(p); + cbm_pipeline_set_persistence(p, true); + ASSERT_EQ(cbm_pipeline_run(p), 0); + cbm_pipeline_free(p); + + char zst[1152]; + snprintf(zst, sizeof(zst), "%s/.codebase-memory/graph.db.zst", g_repo); + int64_t mt0 = t_mtime_ns(zst); + ASSERT_GTE(mt0, 0); + + /* Drift compares mtime seconds strictly after indexed_at's second, so the + * edits must land in a later second than the export. */ + cbm_usleep(1100 * 1000); + + /* Run A: 5 changes (< 10), drift 5 (< 10) → skip. */ + for (int i = 0; i < 5; i++) { + char body[64]; + snprintf(body, sizeof(body), "pub fn f%d() { /* edit */ }\n", i); + write_text_file(paths[i], body); + } + p = cbm_pipeline_new(g_repo, g_db, CBM_MODE_FAST); + ASSERT_NOT_NULL(p); + ASSERT_EQ(cbm_pipeline_run(p), 0); + cbm_pipeline_free(p); + ASSERT_EQ(t_mtime_ns(zst), mt0); /* still debounced */ + + /* Run B: 6 different changes (< 10), but cumulative drift is 11 (>= 10) + * because run A's files still carry post-export mtimes → export fires. */ + for (int i = 5; i < 11; i++) { + char body[64]; + snprintf(body, sizeof(body), "pub fn f%d() { /* edit */ }\n", i); + write_text_file(paths[i], body); + } + p = cbm_pipeline_new(g_repo, g_db, CBM_MODE_FAST); + ASSERT_NOT_NULL(p); + ASSERT_EQ(cbm_pipeline_run(p), 0); + cbm_pipeline_free(p); + ASSERT_NEQ(t_mtime_ns(zst), mt0); /* re-exported by drift */ + + cleanup_dir(g_tmpdir); + PASS(); +} + +/* P3: CBM_ARTIFACT_EXPORT_MAX_AGE_S override. With the age cap forced to 0 a + * 1-change run must export despite being below the change/drift thresholds. */ +TEST(artifact_export_forced_by_max_age_env) { + setup_artifact_test(); + unsetenv("CBM_ARTIFACT_EXPORT_MIN_CHANGES"); + git_init(g_repo); + char gi[1024]; + snprintf(gi, sizeof(gi), "%s/.gitignore", g_repo); + write_text_file(gi, ".codebase-memory/\n"); + char src[1024]; + snprintf(src, sizeof(src), "%s/a.rs", g_repo); + write_text_file(src, "pub fn f0() {}\n"); + + cbm_pipeline_t *p = cbm_pipeline_new(g_repo, g_db, CBM_MODE_FAST); + ASSERT_NOT_NULL(p); + cbm_pipeline_set_persistence(p, true); + ASSERT_EQ(cbm_pipeline_run(p), 0); + cbm_pipeline_free(p); + + char zst[1152]; + snprintf(zst, sizeof(zst), "%s/.codebase-memory/graph.db.zst", g_repo); + int64_t mt0 = t_mtime_ns(zst); + ASSERT_GTE(mt0, 0); + + /* age >= 0 always holds once a second has elapsed, so MAX_AGE_S=0 expires + * immediately; sleep past indexed_at's 1s resolution to avoid age = -0. */ + ASSERT_EQ(setenv("CBM_ARTIFACT_EXPORT_MAX_AGE_S", "0", 1), 0); + cbm_usleep(1100 * 1000); + write_text_file(src, "pub fn f0() { /* modified */ }\n"); + p = cbm_pipeline_new(g_repo, g_db, CBM_MODE_FAST); + ASSERT_NOT_NULL(p); + ASSERT_EQ(cbm_pipeline_run(p), 0); + cbm_pipeline_free(p); + unsetenv("CBM_ARTIFACT_EXPORT_MAX_AGE_S"); + + ASSERT_NEQ(t_mtime_ns(zst), mt0); /* re-exported by age override */ + + cleanup_dir(g_tmpdir); + PASS(); +} + /* P2: persist_hashes now writes all hash rows in one batch transaction from * classify-time stamps (no per-file re-stat). Verify the round-trip via the * real pipeline route: index → modify → incremental reindex (batch persist) → @@ -833,4 +939,6 @@ SUITE(artifact) { RUN_TEST(incremental_persist_batch_roundtrip); RUN_TEST(artifact_export_debounced); RUN_TEST(artifact_export_forced_by_env); + RUN_TEST(artifact_export_forced_by_cumulative_drift); + RUN_TEST(artifact_export_forced_by_max_age_env); } From ba58244315719b4e96a0f5543ef73f4f086825bc Mon Sep 17 00:00:00 2001 From: Greg Tiller Date: Sun, 5 Jul 2026 09:34:21 -0300 Subject: [PATCH 6/7] style: clang-format artifact reconcile additions CI lint (clang-format-20 --dry-run --Werror) flagged comment alignment and line-break style in the P1 additions. Formatting only; no code changes. Refs #867 Signed-off-by: Greg Tiller --- src/pipeline/artifact.c | 12 +++++------- tests/test_artifact.c | 8 ++++---- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/pipeline/artifact.c b/src/pipeline/artifact.c index aa64055af..11571a3b2 100644 --- a/src/pipeline/artifact.c +++ b/src/pipeline/artifact.c @@ -10,8 +10,8 @@ enum { ART_DIR_PERMS = 0755, ART_ZSTD_FAST = 3, ART_ZSTD_BEST = 9, - ART_RATIO_SCALE = 10, /* multiply ratio by 10 for integer logging */ - ART_NUL = 1, /* NUL terminator byte */ + ART_RATIO_SCALE = 10, /* multiply ratio by 10 for integer logging */ + ART_NUL = 1, /* NUL terminator byte */ ART_NS_PER_SEC = 1000000000, /* nanoseconds per second (mtime_ns helper) */ }; #define ART_BYTES_PER_MB ((size_t)1024 * 1024) @@ -22,7 +22,7 @@ enum { #include "foundation/compat_fs.h" #include "foundation/compat.h" #include "foundation/log.h" -#include "foundation/str_util.h" /* cbm_validate_shell_arg */ +#include "foundation/str_util.h" /* cbm_validate_shell_arg */ #include "foundation/hash_table.h" /* CBMHashTable (reconcile changed-set) */ #include "zstd_store.h" @@ -268,8 +268,7 @@ static bool is_hex_oid(const char *s) { * a third cross-file dependency for one helper. */ static int64_t art_stat_mtime_ns(const struct stat *st) { #ifdef __APPLE__ - return ((int64_t)st->st_mtimespec.tv_sec * ART_NS_PER_SEC) + - (int64_t)st->st_mtimespec.tv_nsec; + return ((int64_t)st->st_mtimespec.tv_sec * ART_NS_PER_SEC) + (int64_t)st->st_mtimespec.tv_nsec; #elif defined(_WIN32) return (int64_t)st->st_mtime * ART_NS_PER_SEC; #else @@ -311,8 +310,7 @@ static bool git_run_ok(const char *repo_path, const char *args) { return false; } char drain[CBM_SZ_4K]; - while (fread(drain, 1, sizeof(drain), fp) > 0) { - } + while (fread(drain, 1, sizeof(drain), fp) > 0) {} return cbm_pclose(fp) == 0; } diff --git a/tests/test_artifact.c b/tests/test_artifact.c index 6f099a01e..17d752857 100644 --- a/tests/test_artifact.c +++ b/tests/test_artifact.c @@ -524,8 +524,7 @@ TEST(artifact_reconcile_skips_untracked_rows) { snprintf(gi, sizeof(gi), "%s/.gitignore", repoB); write_text_file(gen, "pub fn generated_local() {}\n"); write_text_file(gi, "gen.rs\n"); - ASSERT_EQ(runf("git -C '%s' add .gitignore && git -C '%s' commit -qm ignore", repoB, repoB), - 0); + ASSERT_EQ(runf("git -C '%s' add .gitignore && git -C '%s' commit -qm ignore", repoB, repoB), 0); char dbB[1152]; snprintf(dbB, sizeof(dbB), "%s/b.db", g_tmpdir); @@ -618,8 +617,9 @@ TEST(artifact_reconcile_skips_unknown_commit) { snprintf(meta, sizeof(meta), "%s/.codebase-memory/artifact.json", repoB); FILE *fp = fopen(meta, "w"); ASSERT_NOT_NULL(fp); - fprintf(fp, "{\"schema_version\":2,\"commit\":\"%s\"," - "\"original_size\":1000,\"reconcile_basis\":\"git-clean-head\"}", + fprintf(fp, + "{\"schema_version\":2,\"commit\":\"%s\"," + "\"original_size\":1000,\"reconcile_basis\":\"git-clean-head\"}", "1111111111111111111111111111111111111111"); fclose(fp); From d09366bc7f3b98254ab12fccbd5599d0ff7e0d9b Mon Sep 17 00:00:00 2001 From: Greg Tiller Date: Sun, 5 Jul 2026 09:35:59 -0300 Subject: [PATCH 7/7] style: clang-format batch persist additions CI lint (clang-format-20 --dry-run --Werror) flagged wrap style in the P2 signature changes. Formatting only; no code changes. Refs #867 Signed-off-by: Greg Tiller --- src/pipeline/pipeline_incremental.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/pipeline/pipeline_incremental.c b/src/pipeline/pipeline_incremental.c index 805dc7ee1..91988c782 100644 --- a/src/pipeline/pipeline_incremental.c +++ b/src/pipeline/pipeline_incremental.c @@ -455,8 +455,7 @@ static void incr_free_edge_capture(cbm_edge_capture_t *cap) { static void persist_hashes_row_by_row(cbm_store_t *store, const char *project, cbm_file_info_t *files, int file_count, const cbm_file_stamp_t *stamps, - const cbm_file_hash_t *mode_skipped, - int mode_skipped_count) { + const cbm_file_hash_t *mode_skipped, int mode_skipped_count) { int current_failed = 0; int ms_failed = 0; @@ -695,9 +694,9 @@ static void run_postpasses(cbm_pipeline_ctx_t *ctx, cbm_file_info_t *changed_fil * reindexes can correctly distinguish "never indexed" from "indexed but * not visited this pass". */ static void dump_and_persist(cbm_gbuf_t *gbuf, const char *db_path, const char *project, - cbm_file_info_t *files, int file_count, - const cbm_file_stamp_t *stamps, const cbm_file_hash_t *mode_skipped, - int mode_skipped_count, const char *repo_path) { + cbm_file_info_t *files, int file_count, const cbm_file_stamp_t *stamps, + const cbm_file_hash_t *mode_skipped, int mode_skipped_count, + const char *repo_path) { struct timespec t; cbm_clock_gettime(CLOCK_MONOTONIC, &t); @@ -716,7 +715,7 @@ static void dump_and_persist(cbm_gbuf_t *gbuf, const char *db_path, const char * cbm_store_t *hash_store = cbm_store_open_path(db_path); if (hash_store) { persist_hashes(hash_store, project, files, file_count, stamps, mode_skipped, - mode_skipped_count); + mode_skipped_count); /* FTS5 rebuild after incremental dump. The btree dump path bypasses * any triggers that could have kept nodes_fts synchronized, so we @@ -766,8 +765,8 @@ int cbm_pipeline_run_incremental(cbm_pipeline_t *p, const char *db_path, cbm_fil int n_changed = 0; int n_unchanged = 0; cbm_file_stamp_t *stamps = calloc((size_t)file_count, sizeof(cbm_file_stamp_t)); - bool *is_changed = classify_files(files, file_count, stored, stored_count, stamps, &n_changed, - &n_unchanged); + bool *is_changed = + classify_files(files, file_count, stored, stored_count, stamps, &n_changed, &n_unchanged); /* Classify stored files absent from current discovery: truly-deleted * (purge) vs mode-skipped (preserve nodes AND hash rows). */