Skip to content

Commit 98c4fc3

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 7f622b1 commit 98c4fc3

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
@@ -186,6 +186,9 @@ void cbm_pipeline_free(cbm_pipeline_t *p) {
186186
p->excluded_dirs = NULL;
187187
p->excluded_count = 0;
188188
free(p->branch_qn);
189+
free(p->saved_adr); /* freed here too: error paths can exit before the
190+
* restore in dump_and_persist_hashes runs. Issue #516. */
191+
p->saved_adr = NULL;
189192
cbm_git_context_free(&p->git_ctx);
190193
/* gbuf, store, registry freed during/after run */
191194
/* Defensively free userconfig in case run() was never called or panicked */
@@ -890,9 +893,12 @@ static int dump_and_persist_hashes(cbm_pipeline_t *p, const cbm_file_info_t *fil
890893
if (hash_store) {
891894
cbm_store_delete_file_hashes(hash_store, p->project_name);
892895

893-
/* Restore the ADR captured before the dump. Issue #516. */
896+
/* Restore the ADR captured before the dump. Surface a failed restore
897+
* rather than silently dropping the ADR (the original #516 symptom). */
894898
if (p->saved_adr) {
895-
cbm_store_adr_store(hash_store, p->project_name, p->saved_adr);
899+
if (cbm_store_adr_store(hash_store, p->project_name, p->saved_adr) != CBM_STORE_OK) {
900+
cbm_log_error("pipeline.err", "phase", "adr_restore", "project", p->project_name);
901+
}
896902
}
897903
for (int i = 0; i < file_count; i++) {
898904
struct stat fst;

tests/test_pipeline.c

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

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

0 commit comments

Comments
 (0)