From a0d0521fcba680ca9f21ad3dc55425b25ec29f48 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Mon, 2 Nov 2026 18:12:22 +0100 Subject: [PATCH] fix(store): keep the indexed checkout outside the digested tables index_coverage_meta is one of the tables the graph digest pins column by column, so the new indexed_checkout_sha column made the digest reject every live store with a schema error: the live-schema compatibility test failed, and test-impact, which digests the graph it builds for HEAD, fell back to running every test for every change. Record the checkout in a table of its own instead: index_checkout (project primary key referencing projects ON DELETE CASCADE, indexed_checkout_sha NOT NULL). A checkout is source identity, which the graph digest deliberately does not bind, and the store already keeps other per-project data (doc_link_*, observed_*) outside the digested tables. index_coverage_meta keeps exactly the shape the digest expects, so the digest needs no change and no version bump, and a new table needs no migration of existing databases. The row is written and cleared in the same coverage transaction, so a failed replace keeps the previous identity, and it is removed with its project. The store API is unchanged: cbm_coverage_meta_t carries indexed_checkout_sha as before. Read-only opens of databases without the table report no identity, as before. store_graph_digest_indexed_checkout_identity_is_not_bound writes a store through the real coverage API with and without a checkout SHA and asserts that it still digests and that the SHA leaves the digest unchanged. Signed-off-by: Martin Vogel --- src/store/store.c | 159 ++++++++++++++++++++------------ src/store/store.h | 6 +- tests/test_store_graph_digest.c | 40 ++++++++ 3 files changed, 143 insertions(+), 62 deletions(-) diff --git a/src/store/store.c b/src/store/store.c index 41fcbc4fb..ed8462d5a 100644 --- a/src/store/store.c +++ b/src/store/store.c @@ -324,29 +324,22 @@ bool cbm_store_edges_schema_current(cbm_store_t *s) { return true; } -/* Best-effort probe for the BT-240 indexed_checkout_sha column. Returns true - * only when the index_coverage_meta table exists AND exposes the column. - * Writable opens run init_schema (which adds it), so false is expected only on - * read-only legacy DBs opened via the query path — those must keep working and - * simply report no indexed-checkout identity. */ -static bool coverage_meta_has_checkout_column(cbm_store_t *s) { +/* Best-effort probe for the BT-240 index_checkout table. Writable opens run + * init_schema (which creates it), so false is expected only on read-only + * legacy DBs opened via the query path — those must keep working and simply + * report no indexed-checkout identity. */ +static bool index_checkout_table_exists(cbm_store_t *s) { if (!s || !s->db) { return false; } sqlite3_stmt *stmt = NULL; - if (sqlite3_prepare_v2(s->db, "PRAGMA table_info(index_coverage_meta);", CBM_NOT_FOUND, &stmt, - NULL) != SQLITE_OK) { + if (sqlite3_prepare_v2(s->db, + "SELECT 1 FROM sqlite_master WHERE type = 'table' " + "AND name = 'index_checkout';", + CBM_NOT_FOUND, &stmt, NULL) != SQLITE_OK) { return false; } - bool has = false; - int rc; - while ((rc = sqlite3_step(stmt)) == SQLITE_ROW) { - const char *name = (const char *)sqlite3_column_text(stmt, SKIP_ONE); - if (name && strcmp(name, "indexed_checkout_sha") == 0) { - has = true; - break; - } - } + bool has = sqlite3_step(stmt) == SQLITE_ROW; sqlite3_finalize(stmt); return has; } @@ -490,8 +483,15 @@ static int init_schema(cbm_store_t *s) { " ignored_files_stored INTEGER NOT NULL DEFAULT 0," " ignored_files_total INTEGER NOT NULL DEFAULT 0," " coverage_version INTEGER NOT NULL DEFAULT 1," - " hash_records_complete INTEGER NOT NULL DEFAULT 0," - " indexed_checkout_sha TEXT" + " hash_records_complete INTEGER NOT NULL DEFAULT 0" + ");" + /* BT-240: the checkout an index was built from. A table of its own, + * not an index_coverage_meta column: it is source identity, which the + * pinned graph-digest tables deliberately do not bind, and a new + * table needs no migration of existing databases. */ + "CREATE TABLE IF NOT EXISTS index_checkout (" + " project TEXT PRIMARY KEY REFERENCES projects(name) ON DELETE CASCADE," + " indexed_checkout_sha TEXT NOT NULL" ");"; int rc = exec_sql(s, ddl); @@ -499,16 +499,6 @@ static int init_schema(cbm_store_t *s) { return rc; } - /* BT-240 migration: CREATE TABLE IF NOT EXISTS cannot add a column to an - * existing legacy index_coverage_meta, so add it idempotently for any - * writable DB that predates it. Read-only query opens skip init_schema - * entirely and meta_get probes the column separately. */ - if (!coverage_meta_has_checkout_column(s) && - exec_sql(s, "ALTER TABLE index_coverage_meta ADD COLUMN indexed_checkout_sha TEXT;") != - CBM_STORE_OK) { - return CBM_STORE_ERR; - } - /* Schema-compat probe (#768): DBs created before the local_name_gen * discriminator still enforce UNIQUE(source_id,target_id,type) and lack * the column — the widened upsert in cbm_store_insert_edge can neither @@ -3065,6 +3055,7 @@ int cbm_store_delete_project(cbm_store_t *s, const char *name) { static const char *cleanup_sql[] = { "DELETE FROM index_coverage WHERE project = ?1;", "DELETE FROM index_coverage_meta WHERE project = ?1;", + "DELETE FROM index_checkout WHERE project = ?1;", "DELETE FROM projects WHERE name = ?1 || '::missed';", }; /* doc_link_unresolved exists only in databases published by a build that @@ -4773,6 +4764,34 @@ static int cov_rebuild_shadow_graph(cbm_store_t *s, const char *project) { return CBM_STORE_OK; } +/* Record (or, with no sha, clear) the checkout a project's index was built + * from. Runs inside the caller's coverage transaction, so a failed replace + * keeps the previous identity. */ +static int index_checkout_write(cbm_store_t *s, const char *project, const char *sha) { + bool record = sha && sha[0]; + sqlite3_stmt *stmt = NULL; + if (sqlite3_prepare_v2(s->db, + record ? "INSERT INTO index_checkout (project, indexed_checkout_sha) " + "VALUES (?1, ?2) ON CONFLICT(project) DO UPDATE SET " + "indexed_checkout_sha=?2;" + : "DELETE FROM index_checkout WHERE project = ?1;", + CBM_NOT_FOUND, &stmt, NULL) != SQLITE_OK) { + store_set_error_sqlite(s, "index checkout write prepare"); + return CBM_STORE_ERR; + } + bind_text(stmt, SKIP_ONE, project); + if (record) { + bind_text(stmt, ST_COL_2, sha); + } + int rc = sqlite3_step(stmt); + sqlite3_finalize(stmt); + if (rc != SQLITE_DONE) { + store_set_error_sqlite(s, "index checkout write"); + return CBM_STORE_ERR; + } + return CBM_STORE_OK; +} + int cbm_store_coverage_replace_ex(cbm_store_t *s, const char *project, const cbm_coverage_row_t *rows, int count, const cbm_coverage_meta_t *meta) { @@ -4894,12 +4913,11 @@ int cbm_store_coverage_replace_ex(cbm_store_t *s, const char *project, "INSERT INTO index_coverage_meta " "(project, generation, index_mode, recorded_at, recording_status, " " ignored_files_stored, ignored_files_total, coverage_version, " - " hash_records_complete, indexed_checkout_sha) " - "VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10) " + " hash_records_complete) " + "VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9) " "ON CONFLICT(project) DO UPDATE SET generation=?2, index_mode=?3, " "recorded_at=?4, recording_status=?5, ignored_files_stored=?6, " - "ignored_files_total=?7, coverage_version=?8, hash_records_complete=?9, " - "indexed_checkout_sha=?10;", + "ignored_files_total=?7, coverage_version=?8, hash_records_complete=?9;", CBM_NOT_FOUND, &up_meta, NULL) != SQLITE_OK) { store_set_error_sqlite(s, "coverage meta upsert prepare"); (void)exec_sql(s, "ROLLBACK;"); @@ -4914,11 +4932,6 @@ int cbm_store_coverage_replace_ex(cbm_store_t *s, const char *project, sqlite3_bind_int(up_meta, 7, ignored_total); sqlite3_bind_int(up_meta, 8, coverage_version); sqlite3_bind_int(up_meta, 9, meta->hash_records_complete ? 1 : 0); - if (meta->indexed_checkout_sha && meta->indexed_checkout_sha[0]) { - bind_text(up_meta, 10, meta->indexed_checkout_sha); - } else { - sqlite3_bind_null(up_meta, 10); - } int meta_rc = sqlite3_step(up_meta); sqlite3_finalize(up_meta); if (meta_rc != SQLITE_DONE) { @@ -4926,6 +4939,10 @@ int cbm_store_coverage_replace_ex(cbm_store_t *s, const char *project, (void)exec_sql(s, "ROLLBACK;"); return CBM_STORE_ERR; } + if (index_checkout_write(s, project, meta->indexed_checkout_sha) != CBM_STORE_OK) { + (void)exec_sql(s, "ROLLBACK;"); + return CBM_STORE_ERR; + } } else { sqlite3_stmt *del_meta = NULL; if (sqlite3_prepare_v2(s->db, "DELETE FROM index_coverage_meta WHERE project = ?1;", @@ -4942,6 +4959,10 @@ int cbm_store_coverage_replace_ex(cbm_store_t *s, const char *project, (void)exec_sql(s, "ROLLBACK;"); return CBM_STORE_ERR; } + if (index_checkout_write(s, project, NULL) != CBM_STORE_OK) { + (void)exec_sql(s, "ROLLBACK;"); + return CBM_STORE_ERR; + } } /* Rebuild the derived miss-graph view from the now-authoritative table @@ -5076,6 +5097,41 @@ void cbm_store_coverage_meta_clear(cbm_coverage_meta_t *meta) { memset(meta, 0, sizeof(*meta)); } +/* Fill out->indexed_checkout_sha from index_checkout. A legacy read-only DB + * (query opens skip init_schema) may lack the table: no identity, not an + * error. On failure out is cleared. */ +static int index_checkout_read(cbm_store_t *s, const char *project, cbm_coverage_meta_t *out) { + if (!index_checkout_table_exists(s)) { + return CBM_STORE_OK; + } + sqlite3_stmt *stmt = NULL; + if (sqlite3_prepare_v2(s->db, + "SELECT indexed_checkout_sha FROM index_checkout WHERE project = ?1;", + CBM_NOT_FOUND, &stmt, NULL) != SQLITE_OK) { + store_set_error_sqlite(s, "index checkout read prepare"); + cbm_store_coverage_meta_clear(out); + return CBM_STORE_ERR; + } + bind_text(stmt, SKIP_ONE, project); + int rc = sqlite3_step(stmt); + if (rc == SQLITE_ROW) { + const char *sha = (const char *)sqlite3_column_text(stmt, 0); + if (sha && sha[0]) { + out->indexed_checkout_sha = cbm_mem_strdup(CBM_MEM_CLASS_STORE, sha); + rc = out->indexed_checkout_sha ? SQLITE_DONE : SQLITE_NOMEM; + } else { + rc = SQLITE_DONE; + } + } + sqlite3_finalize(stmt); + if (rc != SQLITE_DONE) { + store_set_error_sqlite(s, "index checkout read"); + cbm_store_coverage_meta_clear(out); + return CBM_STORE_ERR; + } + return CBM_STORE_OK; +} + int cbm_store_coverage_meta_get(cbm_store_t *s, const char *project, cbm_coverage_meta_t *out) { if (!out) { return CBM_STORE_ERR; @@ -5085,21 +5141,11 @@ int cbm_store_coverage_meta_get(cbm_store_t *s, const char *project, cbm_coverag return CBM_STORE_ERR; } sqlite3_stmt *stmt = NULL; - /* Legacy read-only DBs (query opens skip init_schema) lack the - * indexed_checkout_sha column — probe and fall back so they still read and - * report no identity instead of erroring. */ - static const char SQL_FULL[] = - "SELECT project, generation, index_mode, recorded_at, recording_status, " - "ignored_files_stored, ignored_files_total, coverage_version, " - "hash_records_complete, indexed_checkout_sha FROM index_coverage_meta " - "WHERE project = ?1;"; - static const char SQL_LEGACY[] = - "SELECT project, generation, index_mode, recorded_at, recording_status, " - "ignored_files_stored, ignored_files_total, coverage_version, " - "hash_records_complete FROM index_coverage_meta WHERE project = ?1;"; - bool has_checkout_column = coverage_meta_has_checkout_column(s); - if (sqlite3_prepare_v2(s->db, has_checkout_column ? SQL_FULL : SQL_LEGACY, CBM_NOT_FOUND, &stmt, - NULL) != SQLITE_OK) { + if (sqlite3_prepare_v2(s->db, + "SELECT project, generation, index_mode, recorded_at, recording_status, " + "ignored_files_stored, ignored_files_total, coverage_version, " + "hash_records_complete FROM index_coverage_meta WHERE project = ?1;", + CBM_NOT_FOUND, &stmt, NULL) != SQLITE_OK) { store_set_error_sqlite(s, "coverage meta get prepare"); return CBM_STORE_ERR; } @@ -5115,18 +5161,13 @@ int cbm_store_coverage_meta_get(cbm_store_t *s, const char *project, cbm_coverag out->ignored_files_total = sqlite3_column_int(stmt, 6); out->coverage_version = sqlite3_column_int(stmt, 7); out->hash_records_complete = sqlite3_column_int(stmt, 8) != 0; - if (has_checkout_column) { - const char *sha = (const char *)sqlite3_column_text(stmt, 9); - out->indexed_checkout_sha = - sha && sha[0] ? cbm_mem_strdup(CBM_MEM_CLASS_STORE, sha) : NULL; - } sqlite3_finalize(stmt); if (!out->project || !out->generation || !out->index_mode || !out->recorded_at || !out->recording_status) { cbm_store_coverage_meta_clear(out); return CBM_STORE_ERR; } - return CBM_STORE_OK; + return index_checkout_read(s, project, out); } sqlite3_finalize(stmt); if (rc != SQLITE_DONE) { diff --git a/src/store/store.h b/src/store/store.h index 30662b221..43f1a21f0 100644 --- a/src/store/store.h +++ b/src/store/store.h @@ -791,9 +791,9 @@ typedef struct { int coverage_version; bool hash_records_complete; /* Git checkout identity frozen at the successful staged-generation - * boundary (BT-240). NULL/"" when the run had no git context (non-git - * repo) or the DB predates the column. Owned by the row: meta_get - * allocates it, meta_clear frees it. */ + * boundary (BT-240), stored in the index_checkout table. NULL/"" when the + * run had no git context (non-git repo) or the DB predates that table. + * Owned by the row: meta_get allocates it, meta_clear frees it. */ const char *indexed_checkout_sha; } cbm_coverage_meta_t; diff --git a/tests/test_store_graph_digest.c b/tests/test_store_graph_digest.c index abb32ba7c..d2add2adf 100644 --- a/tests/test_store_graph_digest.c +++ b/tests/test_store_graph_digest.c @@ -594,6 +594,45 @@ TEST(store_graph_digest_concurrent_isolation_with_held_callback_gates) { cbm_store_close(a);cbm_store_close(b);PASS(); } +/* BT-240 (#1561): the indexed checkout is source identity, which the digest + * never binds, so it lives outside the digested tables. A store written through + * the real coverage API, with and without a checkout SHA, keeps passing the + * live-schema check (test-impact falls back to running everything otherwise), + * and recording or changing the SHA leaves the digest unchanged. */ +TEST(store_graph_digest_indexed_checkout_identity_is_not_bound) { + cbm_store_t *s = cbm_store_open_memory(); + ASSERT_NOT_NULL(s); + ASSERT_EQ(cbm_store_upsert_project(s, "p", "/live"), CBM_STORE_OK); + cbm_coverage_meta_t meta = {.generation = "g", + .index_mode = "full", + .recorded_at = "t", + .recording_status = "complete", + .coverage_version = 1, + .hash_records_complete = true}; + cbm_store_graph_digest_t none = {0}, first = {0}, second = {0}; + ASSERT_EQ(cbm_store_coverage_replace_ex(s, "p", NULL, 0, &meta), CBM_STORE_OK); + bool none_ok = gd_p(s, &none); + meta.indexed_checkout_sha = "0123456789abcdef0123456789abcdef01234567"; + ASSERT_EQ(cbm_store_coverage_replace_ex(s, "p", NULL, 0, &meta), CBM_STORE_OK); + bool first_ok = gd_p(s, &first); + meta.indexed_checkout_sha = "89abcdef0123456789abcdef0123456789abcdef"; + ASSERT_EQ(cbm_store_coverage_replace_ex(s, "p", NULL, 0, &meta), CBM_STORE_OK); + bool second_ok = gd_p(s, &second); + cbm_coverage_meta_t got = {0}; + int got_rc = cbm_store_coverage_meta_get(s, "p", &got); + bool sha_kept = got_rc == CBM_STORE_OK && got.indexed_checkout_sha && + strcmp(got.indexed_checkout_sha, "89abcdef0123456789abcdef0123456789abcdef") == 0; + cbm_store_coverage_meta_clear(&got); + cbm_store_close(s); + ASSERT_TRUE(none_ok); + ASSERT_TRUE(first_ok); + ASSERT_TRUE(second_ok); + ASSERT_TRUE(sha_kept); + ASSERT_TRUE(gd_equal(&none, &first)); + ASSERT_TRUE(gd_equal(&first, &second)); + PASS(); +} + SUITE(store_graph_digest) { RUN_TEST(store_graph_digest_independent_canonical_golden); RUN_TEST(store_graph_digest_logical_order_and_excluded_storage_are_stable); @@ -607,4 +646,5 @@ SUITE(store_graph_digest) { RUN_TEST(store_graph_digest_cancellation_large_blob_never_publishes_prefix); RUN_TEST(store_graph_digest_sql_errors_cleanup_no_mutation_and_close_discard); RUN_TEST(store_graph_digest_concurrent_isolation_with_held_callback_gates); + RUN_TEST(store_graph_digest_indexed_checkout_identity_is_not_bound); }