Skip to content

Commit 2f434d4

Browse files
fix(pipeline): harden ADR preservation and add regression test (#516)
Addresses review on #539. - Free p->saved_adr in cbm_pipeline_free so it is not leaked on error paths that exit before the restore in dump_and_persist_hashes (e.g. a cbm_gbuf_dump_to_sqlite failure). - Check cbm_store_adr_store's return value on restore and log an error instead of silently dropping the ADR (the original #516 symptom). - Add reproduce-first test pipeline_adr_survives_full_reindex: index, store an ADR, force a full re-index by adding files, assert the ADR survives unchanged. Passes against the fix. Signed-off-by: RithvikReddy0-0 <rithvikreddymukkara@gmail.com>
1 parent 7b6c063 commit 2f434d4

2 files changed

Lines changed: 78 additions & 2 deletions

File tree

src/pipeline/pipeline.c

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,9 @@ void cbm_pipeline_free(cbm_pipeline_t *p) {
180180
p->excluded_dirs = NULL;
181181
p->excluded_count = 0;
182182
free(p->branch_qn);
183+
free(p->saved_adr); /* freed here too: error paths can exit before the
184+
* restore in dump_and_persist_hashes runs. Issue #516. */
185+
p->saved_adr = NULL;
183186
cbm_git_context_free(&p->git_ctx);
184187
/* gbuf, store, registry freed during/after run */
185188
/* Defensively free userconfig in case run() was never called or panicked */
@@ -862,9 +865,12 @@ static int dump_and_persist_hashes(cbm_pipeline_t *p, const cbm_file_info_t *fil
862865
if (hash_store) {
863866
cbm_store_delete_file_hashes(hash_store, p->project_name);
864867

865-
/* Restore the ADR captured before the dump. Issue #516. */
868+
/* Restore the ADR captured before the dump. Surface a failed restore
869+
* rather than silently dropping the ADR (the original #516 symptom). */
866870
if (p->saved_adr) {
867-
cbm_store_adr_store(hash_store, p->project_name, p->saved_adr);
871+
if (cbm_store_adr_store(hash_store, p->project_name, p->saved_adr) != CBM_STORE_OK) {
872+
cbm_log_error("pipeline.err", "phase", "adr_restore", "project", p->project_name);
873+
}
868874
}
869875
for (int i = 0; i < file_count; i++) {
870876
struct stat fst;

tests/test_pipeline.c

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,75 @@ TEST(pipeline_structure_nodes) {
272272
PASS();
273273
}
274274

275+
/* Issue #516: an ADR stored via manage_adr (project_summaries) must survive a
276+
* full re-index. A full re-index deletes the DB and rebuilds it from the graph
277+
* buffer, which writes an empty project_summaries table; the fix captures the
278+
* ADR before the delete and restores it after the rebuild. Reproduce-first:
279+
* index, store an ADR, force a full re-index by adding files, assert the ADR
280+
* is still present and unchanged. */
281+
TEST(pipeline_adr_survives_full_reindex) {
282+
char tmp[256];
283+
snprintf(tmp, sizeof(tmp), "/tmp/cbm_adr_XXXXXX");
284+
if (!cbm_mkdtemp(tmp)) {
285+
FAIL("failed to create temp dir");
286+
}
287+
288+
char db_path[512];
289+
snprintf(db_path, sizeof(db_path), "%s/test.db", tmp);
290+
291+
/* Initial index with a single source file. */
292+
char path[512];
293+
snprintf(path, sizeof(path), "%s/main.py", tmp);
294+
FILE *f = fopen(path, "w");
295+
ASSERT_NOT_NULL(f);
296+
fprintf(f, "def foo():\n pass\n");
297+
fclose(f);
298+
299+
cbm_pipeline_t *p1 = cbm_pipeline_new(tmp, db_path, CBM_MODE_FULL);
300+
ASSERT_NOT_NULL(p1);
301+
ASSERT_EQ(cbm_pipeline_run(p1), 0);
302+
const char *project = cbm_pipeline_project_name(p1);
303+
char project_copy[256];
304+
snprintf(project_copy, sizeof(project_copy), "%s", project);
305+
cbm_pipeline_free(p1);
306+
307+
/* Store an ADR. */
308+
const char *adr_text = "# Decision\nWe chose X over Y.";
309+
cbm_store_t *s1 = cbm_store_open_path(db_path);
310+
ASSERT_NOT_NULL(s1);
311+
ASSERT_EQ(cbm_store_adr_store(s1, project_copy, adr_text), CBM_STORE_OK);
312+
cbm_store_close(s1);
313+
314+
/* Force a full re-index: add enough files to exceed the incremental
315+
* threshold so the DB is deleted and rebuilt. */
316+
for (int i = 0; i < 4; i++) {
317+
snprintf(path, sizeof(path), "%s/extra%d.py", tmp, i);
318+
f = fopen(path, "w");
319+
ASSERT_NOT_NULL(f);
320+
fprintf(f, "def g%d():\n return %d\n", i, i);
321+
fclose(f);
322+
}
323+
324+
cbm_pipeline_t *p2 = cbm_pipeline_new(tmp, db_path, CBM_MODE_FULL);
325+
ASSERT_NOT_NULL(p2);
326+
ASSERT_EQ(cbm_pipeline_run(p2), 0);
327+
cbm_pipeline_free(p2);
328+
329+
/* The ADR must still be present and unchanged. */
330+
cbm_store_t *s2 = cbm_store_open_path(db_path);
331+
ASSERT_NOT_NULL(s2);
332+
cbm_adr_t adr = {0};
333+
int rc = cbm_store_adr_get(s2, project_copy, &adr);
334+
ASSERT_EQ(rc, CBM_STORE_OK);
335+
ASSERT_NOT_NULL(adr.content);
336+
ASSERT_STR_EQ(adr.content, adr_text);
337+
cbm_store_adr_free(&adr);
338+
cbm_store_close(s2);
339+
340+
rm_rf(tmp);
341+
PASS();
342+
}
343+
275344
TEST(pipeline_structure_edges) {
276345
if (setup_test_repo() != 0) {
277346
FAIL("failed to create temp dir");
@@ -5897,6 +5966,7 @@ SUITE(pipeline) {
58975966
RUN_TEST(store_bulk_persistence);
58985967
/* Integration: structure pass */
58995968
RUN_TEST(pipeline_structure_nodes);
5969+
RUN_TEST(pipeline_adr_survives_full_reindex);
59005970
RUN_TEST(pipeline_structure_edges);
59015971
RUN_TEST(pipeline_branch_root_structure);
59025972
RUN_TEST(pipeline_project_name_derived);

0 commit comments

Comments
 (0)