Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 19 additions & 3 deletions src/mcp/mcp.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
18 changes: 18 additions & 0 deletions src/pipeline/pipeline.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
Expand Down
7 changes: 7 additions & 0 deletions src/pipeline/pipeline.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
66 changes: 66 additions & 0 deletions tests/test_pipeline.c
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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);
Expand Down
Loading