diff --git a/src/pipeline/artifact.c b/src/pipeline/artifact.c index 7be8f6950c..f3b6098efa 100644 --- a/src/pipeline/artifact.c +++ b/src/pipeline/artifact.c @@ -1130,6 +1130,28 @@ int cbm_artifact_export(const char *db_path, const char *repo_path, const char * /* ── Import ──────────────────────────────────────────────────────── */ +/* Restore the imported ADR (carried in the graph DB's project_summaries) into + * the per-project sidecar so it arrives with the shared artifact — even for a + * teammate who already indexed the project (a marked sidecar with no ADR), + * which the marker-gated migrate-once would skip. INSERT OR IGNORE means a + * local ADR is never overwritten. Best-effort: the graph DB is already + * installed when this runs, so a failed restore never fails the import, but it + * is logged with the store's reason so a team ADR that did not arrive is + * diagnosable instead of silently missing. */ +static void artifact_import_restore_adr(const char *cache_db_path) { + cbm_store_t *restore = cbm_store_open_path_existing(cache_db_path); + if (!restore) { + cbm_log_warn("artifact.import", "err", "adr_restore", "detail", "reopen_imported_db", + "path", cache_db_path); + return; + } + if (cbm_store_adr_restore_from_graph_if_absent(restore) != CBM_STORE_OK) { + cbm_log_warn("artifact.import", "err", "adr_restore", "detail", cbm_store_error(restore), + "path", cache_db_path); + } + cbm_store_close(restore); +} + int cbm_artifact_import(const char *repo_path, const char *cache_db_path) { if (!repo_path || !cache_db_path) { return CBM_NOT_FOUND; @@ -1257,16 +1279,7 @@ int cbm_artifact_import(const char *repo_path, const char *cache_db_path) { cbm_unlink(wal); cbm_unlink(shm); - /* Restore the imported ADR (carried in the graph DB's project_summaries) into - * the per-project sidecar so it arrives with the shared artifact — even for a - * teammate who already indexed the project (a marked sidecar with no ADR), - * which the marker-gated migrate-once would skip. INSERT OR IGNORE means a - * local ADR is never overwritten. Best-effort. */ - cbm_store_t *restore = cbm_store_open_path_existing(cache_db_path); - if (restore) { - (void)cbm_store_adr_restore_from_graph_if_absent(restore); - cbm_store_close(restore); - } + artifact_import_restore_adr(cache_db_path); cbm_log_info("artifact.import", "db", cache_db_path, "size_mb", itoa_buf((int)((size_t)dlen / ART_BYTES_PER_MB))); diff --git a/src/store/store.c b/src/store/store.c index 9e6b6fc32c..ed06236a9d 100644 --- a/src/store/store.c +++ b/src/store/store.c @@ -12010,13 +12010,6 @@ static bool adr_migrated(sqlite3 *adb) { return done; } -/* Copy any legacy ADR rows from the graph DB's project_summaries into the - * sidecar (INSERT OR IGNORE, so a local row is never clobbered), then set the - * one-time marker. The marker check, copy and marker insert run in ONE - * BEGIN IMMEDIATE transaction on the sidecar, so two processes migrating at - * once cannot re-copy a row the other just deleted. Returns CBM_STORE_ERR on - * any failure (transaction rolled back) so callers can fail closed and preserve - * the old generation. No-op once the marker is set. */ /* Copy every ADR row from the graph DB's project_summaries into the sidecar with * INSERT OR IGNORE (so a row already present locally is never overwritten). Must * run inside a transaction on the sidecar connection. Returns OK/ERR; does not @@ -12070,6 +12063,13 @@ static int adr_copy_rows(cbm_store_t *s, sqlite3 *adb) { return (rc == SQLITE_DONE) ? CBM_STORE_OK : CBM_STORE_ERR; } +/* Copy any legacy ADR rows from the graph DB's project_summaries into the + * sidecar (INSERT OR IGNORE, so a local row is never clobbered), then set the + * one-time marker. The marker check, copy and marker insert run in ONE + * BEGIN IMMEDIATE transaction on the sidecar, so two processes migrating at + * once cannot re-copy a row the other just deleted. Returns CBM_STORE_ERR on + * any failure (transaction rolled back) so callers can fail closed and preserve + * the old generation. No-op once the marker is set. */ static int adr_copy_legacy_once(cbm_store_t *s, sqlite3 *adb) { if (adr_migrated(adb)) { return CBM_STORE_OK; /* fast path: already migrated, no lock needed */ @@ -12106,19 +12106,22 @@ static int adr_copy_legacy_once(cbm_store_t *s, sqlite3 *adb) { * DB's project_summaries, and a teammate who already indexed the project (so the * sidecar is marked, but has no ADR) must still receive it. INSERT OR IGNORE * guarantees a local ADR is never clobbered; running ungated by the marker is - * what distinguishes this from the one-time legacy migration. */ + * what distinguishes this from the one-time legacy migration. It never writes + * the marker: `adb` comes from adr_conn_write, which hands out a sidecar handle + * only after adr_copy_legacy_once committed it, and nothing deletes it. On + * failure the store error names the step (the import logs it). */ static int adr_restore_if_absent(cbm_store_t *s, sqlite3 *adb) { if (sqlite3_exec(adb, "BEGIN IMMEDIATE;", NULL, NULL, NULL) != SQLITE_OK) { + store_set_error_sqlite_on(s, adb, "adr restore begin"); return CBM_STORE_ERR; } if (adr_copy_rows(s, adb) != CBM_STORE_OK) { + store_set_error(s, "adr restore copy failed"); (void)sqlite3_exec(adb, "ROLLBACK;", NULL, NULL, NULL); return CBM_STORE_ERR; } - /* Mark migrated too, so a later reindex's before-delete migration is a no-op. */ - (void)sqlite3_exec(adb, "INSERT OR IGNORE INTO adr_meta (key, value) VALUES ('migrated','1');", - NULL, NULL, NULL); if (sqlite3_exec(adb, "COMMIT;", NULL, NULL, NULL) != SQLITE_OK) { + store_set_error_sqlite_on(s, adb, "adr restore commit"); (void)sqlite3_exec(adb, "ROLLBACK;", NULL, NULL, NULL); return CBM_STORE_ERR; } diff --git a/tests/test_artifact.c b/tests/test_artifact.c index bb7990c9ed..abf1415da6 100644 --- a/tests/test_artifact.c +++ b/tests/test_artifact.c @@ -252,6 +252,21 @@ TEST(artifact_export_roundtrip_keeps_adr) { cbm_store_adr_free(&adr); cbm_store_close(dst); + /* The fresh destination sidecar is marked migrated after the import, so a + * later reindex's migrate-before-delete is a no-op. Opening the sidecar for + * the restore already commits the marker; the restore itself adds none. */ + char side[1100]; + snprintf(side, sizeof(side), "%s.adr.db", import_db); + sqlite3 *raw = NULL; + ASSERT_EQ(sqlite3_open_v2(side, &raw, SQLITE_OPEN_READONLY, NULL), SQLITE_OK); + sqlite3_stmt *st = NULL; + ASSERT_EQ( + sqlite3_prepare_v2(raw, "SELECT value FROM adr_meta WHERE key='migrated';", -1, &st, NULL), + SQLITE_OK); + ASSERT_EQ(sqlite3_step(st), SQLITE_ROW); + sqlite3_finalize(st); + sqlite3_close(raw); + cleanup_dir(g_tmpdir); PASS(); } @@ -391,6 +406,42 @@ TEST(artifact_import_restores_adr_when_local_absent) { PASS(); } +/* A failed ADR restore must not be silent (#2315 follow-up). A DIRECTORY at + * ".adr.db" makes the sidecar open fail deterministically (no hook). The + * graph DB is already installed when the restore runs, so the import still + * succeeds (best-effort, unchanged), but the team ADR that did not arrive is + * logged with the store's reason instead of being dropped without a trace. */ +TEST(artifact_import_logs_failed_adr_restore) { + setup_artifact_test(); + create_test_db(g_db); + + cbm_store_t *src = cbm_store_open_path(g_db); + ASSERT_NOT_NULL(src); + ASSERT_EQ(cbm_store_adr_store(src, "test-proj", "# Decision\nshared A"), CBM_STORE_OK); + cbm_store_close(src); + ASSERT_EQ(cbm_artifact_export(g_db, g_repo, "test-proj", CBM_ARTIFACT_FAST), 0); + + char import_db[1024]; + snprintf(import_db, sizeof(import_db), "%s/imported.db", g_tmpdir); + char adr_dir[1100]; + snprintf(adr_dir, sizeof(adr_dir), "%s.adr.db", import_db); + ASSERT_EQ(cbm_mkdir(adr_dir), 0); + + capture_logs_start(); + int rc = cbm_artifact_import(g_repo, import_db); + const char *logs = capture_logs_end(); + + ASSERT_EQ(rc, 0); /* the graph DB is installed; the restore stays best-effort */ + ASSERT_TRUE(cbm_file_exists(import_db)); + const char *line = strstr(logs, "level=warn msg=artifact.import err=adr_restore"); + ASSERT_NOT_NULL(line); + /* The store's reason is carried (the text log format maps spaces to '_'). */ + ASSERT(strstr(line, "detail=adr_sidecar_open") != NULL); + + cleanup_dir(g_tmpdir); + PASS(); +} + TEST(artifact_export_best_roundtrip) { setup_artifact_test(); create_test_db(g_db); @@ -1361,6 +1412,7 @@ SUITE(artifact) { RUN_TEST(artifact_export_without_adr_does_not_resurrect_legacy_row); RUN_TEST(artifact_import_does_not_clobber_local_adr); RUN_TEST(artifact_import_restores_adr_when_local_absent); + RUN_TEST(artifact_import_logs_failed_adr_restore); RUN_TEST(artifact_export_best_roundtrip); RUN_TEST(artifact_exists_check); RUN_TEST(artifact_commit_hash);