diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index d6c7bd4686..f524564344 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -11414,9 +11414,25 @@ static char *handle_index_repository(cbm_mcp_server_t *srv, const char *args) { yyjson_mut_obj_add_strcpy(doc, root, "message", message); } else { yyjson_mut_obj_add_str(doc, root, "status", "error"); - yyjson_mut_obj_add_str(doc, root, "hint", - "Pipeline failed. Check repo_path exists and contains source files. " - "Try mode='fast' for a quicker diagnostic run."); + /* #1665: a post-publish artifact export failure (read-only repo, etc.) + * must not be blamed on the repository path. The pipeline snapshots the + * export error of THIS run, so its presence names the phase exactly — + * the graph database was already published when the export ran. */ + const char *export_error = cbm_pipeline_export_error(p); + if (export_error && export_error[0]) { + char hint[CBM_SZ_1K]; + (void)snprintf( + hint, sizeof(hint), + "Index database was published, but the persistence artifact export failed for " + "repo_path/.codebase-memory (%s). Use a writable checkout, or re-run with " + "--persistence false if the shared artifact is not needed.", + export_error); + yyjson_mut_obj_add_strcpy(doc, root, "hint", hint); + } else { + yyjson_mut_obj_add_str(doc, root, "hint", + "Pipeline failed. Check repo_path exists and contains source " + "files. Try mode='fast' for a quicker diagnostic run."); + } } char *json = yy_doc_to_str(doc); diff --git a/src/pipeline/pipeline.c b/src/pipeline/pipeline.c index fa62441a11..c808cb1b47 100644 --- a/src/pipeline/pipeline.c +++ b/src/pipeline/pipeline.c @@ -193,6 +193,13 @@ struct cbm_pipeline { cbm_index_resource_policy_t resource_policy; cbm_index_resource_violation_t resource_violation; + /* Snapshot of the artifact export failure of THIS run (set only by + * export_after_publish failure, zeroed at run start, cleared on success). + * The MCP layer reads it to attribute a failed run to artifact export + * without consulting cbm_artifact_export_last_error() directly — that + * global can still hold a PREVIOUS run's error. */ + char export_error[CBM_SZ_1K]; + /* Indexing state (set during run) */ cbm_gbuf_t *gbuf; cbm_registry_t *registry; @@ -389,6 +396,10 @@ void cbm_pipeline_get_resource_violation(const cbm_pipeline_t *p, } } +const char *cbm_pipeline_export_error(const cbm_pipeline_t *p) { + return p ? p->export_error : ""; +} + bool cbm_pipeline_set_project_name(cbm_pipeline_t *p, const char *name) { if (!p || !name || !name[0]) { return false; @@ -3162,6 +3173,12 @@ static int export_after_publish(cbm_pipeline_t *p, const char *final_path) { if (rc != 0) { const char *err = cbm_artifact_export_last_error(); cbm_log_error("pipeline.err", "phase", "artifact_export", "err", err ? err : "unknown"); + /* #1665: snapshot the error of THIS run so the MCP layer can + * attribute the failure truthfully, instead of re-reading the + * process-global export error (which may describe a previous run) + * and instead of the generic "Pipeline failed" hint that blames + * repo_path for a write-permission failure. */ + (void)snprintf(p->export_error, sizeof(p->export_error), "%s", err ? err : "unknown"); } return rc; } @@ -3397,6 +3414,7 @@ int cbm_pipeline_run(cbm_pipeline_t *p) { if (!p) { return CBM_NOT_FOUND; } + p->export_error[0] = '\0'; char *final_path = resolve_db_path(p); if (!final_path || !ensure_db_parent(final_path)) { free(final_path); diff --git a/src/pipeline/pipeline.h b/src/pipeline/pipeline.h index e5a672cb0b..c0fddc7e77 100644 --- a/src/pipeline/pipeline.h +++ b/src/pipeline/pipeline.h @@ -60,6 +60,13 @@ void cbm_pipeline_set_resource_policy(cbm_pipeline_t *p, const cbm_index_resourc void cbm_pipeline_get_resource_violation(const cbm_pipeline_t *p, cbm_index_resource_violation_t *violation); +/* Snapshot of the artifact export failure of the last cbm_pipeline_run, or "" + * when the run succeeded / did not reach post-publish export. Used to + * truthfully attribute a failed run to the persistence export (#1665) instead + * of the generic pipeline-error hint. Valid until the next cbm_pipeline_run or + * cbm_pipeline_free(). Returns "" for NULL p. */ +const char *cbm_pipeline_export_error(const cbm_pipeline_t *p); + /* Free a pipeline and all its internal state. NULL-safe. */ void cbm_pipeline_free(cbm_pipeline_t *p); diff --git a/tests/test_pipeline.c b/tests/test_pipeline.c index 798c6556ad..50840af3a4 100644 --- a/tests/test_pipeline.c +++ b/tests/test_pipeline.c @@ -477,6 +477,71 @@ TEST(pipeline_structure_nodes) { * ADR before the delete and restores it after the rebuild. Reproduce-first: * index, store an ADR, force a full re-index by adding files, assert the ADR * is still present and unchanged. */ +/* #1665: a persistence artifact export failure must not be reported as a + * repo_path problem. Create the artifact directory as a regular FILE so + * cbm_mkdir_p(.codebase-memory) fails deterministically (works as root too), + * run with persistence enabled, and assert: the run fails, the DB was + * published (publish happens before export), the pipeline snapshots the export + * error, and a clean re-run leaves the snapshot empty. */ +TEST(pipeline_export_error_snapshot_on_artifact_failure) { + char tmp[256]; + snprintf(tmp, sizeof(tmp), "/tmp/cbm_export1665_XXXXXX"); + if (!cbm_mkdtemp(tmp)) { + FAIL("failed to create temp dir"); + } + + char path[512]; + snprintf(path, sizeof(path), "%s/main.py", tmp); + FILE *f = fopen(path, "w"); + ASSERT_NOT_NULL(f); + fprintf(f, "def foo():\n pass\n"); + fclose(f); + + char db_path[512]; + snprintf(db_path, sizeof(db_path), "%s/test.db", tmp); + + /* Block the artifact directory: a FILE at .codebase-memory makes the + * export's mkdir_p fail with a deterministic error on every platform. */ + char art_block[512]; + snprintf(art_block, sizeof(art_block), "%s/.codebase-memory", tmp); + f = fopen(art_block, "w"); + ASSERT_NOT_NULL(f); + fprintf(f, "block\n"); + fclose(f); + + cbm_pipeline_t *p = cbm_pipeline_new(tmp, db_path, CBM_MODE_FULL); + ASSERT_NOT_NULL(p); + cbm_pipeline_set_persistence(p, true); + + int rc = cbm_pipeline_run(p); + ASSERT_NEQ(rc, 0); + + /* The export runs after publish, so the DB must exist despite the failure. */ + struct stat db_st; + ASSERT_EQ(stat(db_path, &db_st), 0); + + const char *export_error = cbm_pipeline_export_error(p); + ASSERT_NOT_NULL(export_error); + ASSERT_TRUE(export_error[0] != '\0'); + ASSERT_NOT_NULL(strstr(export_error, "prepare_artifact_dir")); + cbm_pipeline_free(p); + + /* Control: remove the blocker; a fresh run succeeds and leaves no snapshot. */ + (void)cbm_unlink(art_block); + cbm_pipeline_t *p2 = cbm_pipeline_new(tmp, db_path, CBM_MODE_FULL); + ASSERT_NOT_NULL(p2); + cbm_pipeline_set_persistence(p2, true); + rc = cbm_pipeline_run(p2); + ASSERT_EQ(rc, 0); + const char *export_error2 = cbm_pipeline_export_error(p2); + ASSERT_NOT_NULL(export_error2); + ASSERT_TRUE(export_error2[0] == '\0'); + cbm_pipeline_free(p2); + + rm_rf(tmp); + PASS(); +} + TEST(pipeline_adr_survives_full_reindex) { char tmp[256]; snprintf(tmp, sizeof(tmp), "/tmp/cbm_adr_XXXXXX"); @@ -15151,6 +15216,7 @@ SUITE(pipeline) { RUN_TEST(pipeline_structure_nodes); RUN_TEST(pipeline_committed_counts_match_persisted); RUN_TEST(pipeline_adr_survives_full_reindex); + RUN_TEST(pipeline_export_error_snapshot_on_artifact_failure); RUN_TEST(pipeline_structure_edges); RUN_TEST(pipeline_branch_root_structure); RUN_TEST(pipeline_project_name_derived);