fix(mcp): tell the truth about pre-publication index aborts

An aborted incremental run preserves the previous index by design, but
the response said generic 'error — check repo_path exists', sending
people to debug a path that was fine. The abort and persist-failure
codes move to the public pipeline header (the contract always said
callers may distinguish them) and index_repository now answers with
what actually happened: aborted_previous_preserved (retry; the old
index still serves) vs persist_failed (disk/permissions) vs the
generic failure.

The incremental test helper goes fail-closed on error responses,
surfacing the real cause once at the source — a transient abort used
to slip past the non-NULL assert and cascade into dozens of mysterious
count failures downstream (the 99-test Windows pile-up).

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
This commit is contained in:
Martin Vogel
2026-08-28 02:07:53 +02:00
parent 762fbe2d55
commit 77eb7eb357
4 changed files with 40 additions and 8 deletions
+19
View File
@@ -8353,6 +8353,25 @@ static char *handle_index_repository(cbm_mcp_server_t *srv, const char *args) {
srv, doc, root, project_name, repo_path, persistence, p, excluded_dirs, excluded_count,
file_errors, file_error_count, has_logfile ? logfile_path : NULL);
yyjson_mut_obj_add_str(doc, root, "status", degraded ? "degraded" : "indexed");
} else if (rc == CBM_PIPELINE_ABORT_PRESERVE_DB) {
/* The truthful abort message: the old generic "check repo_path" hint
* sent people debugging a path that was fine, when the run aborted
* pre-publication (semantic inputs changed mid-run, or a discovery/
* manifest phase failed transiently) and the previous index is
* intact. A 99-test cascade on the Windows leg traced back to
* exactly this — the response said error, but not which kind. */
yyjson_mut_obj_add_str(doc, root, "status", "aborted_previous_preserved");
yyjson_mut_obj_add_str(doc, root, "hint",
"Indexing aborted before publication; the previous index is "
"intact and still serving. Causes: files changed while the run "
"was in flight, or a discovery/manifest phase failed "
"transiently. Retry; if it repeats, check the run log.");
} else if (rc == CBM_PIPELINE_PERSIST_FAILED) {
yyjson_mut_obj_add_str(doc, root, "status", "persist_failed");
yyjson_mut_obj_add_str(doc, root, "hint",
"The validated staging database could not be published. Check "
"free disk space and permissions on the cache directory; the "
"previous index may have been rolled back.");
} else {
yyjson_mut_obj_add_str(doc, root, "status", "error");
yyjson_mut_obj_add_str(doc, root, "hint",
+3 -3
View File
@@ -63,9 +63,9 @@ void cbm_pipeline_free(cbm_pipeline_t *p);
* need to know whether the PREVIOUS generation survived can distinguish the
* failures by value: the run publishes by renaming a fully validated staging
* database over the destination, so every abort before that rename leaves the
* existing database in place. Those codes (CBM_PIPELINE_ABORT_PRESERVE_DB and
* CBM_PIPELINE_PERSIST_FAILED) are defined in pipeline_internal.h alongside the
* stages that raise them. */
* existing database in place. */
#define CBM_PIPELINE_ABORT_PRESERVE_DB (-2) /* aborted pre-publication; previous DB intact */
#define CBM_PIPELINE_PERSIST_FAILED (-4) /* staging/rollback failure during publication */
int cbm_pipeline_run(cbm_pipeline_t *p);
/* Request cancellation of a running pipeline (thread-safe). */
+4 -4
View File
@@ -29,12 +29,12 @@
/* Route node QN buffer size (must fit __route__METHOD__/full/url/path) */
#define CBM_ROUTE_QN_SIZE 768
/* Incremental integrity failure: abort the run and preserve the existing DB.
* Distinct from CBM_NOT_FOUND, which the orchestrator uses as the normal
/* CBM_PIPELINE_ABORT_PRESERVE_DB / CBM_PIPELINE_PERSIST_FAILED moved to
* pipeline.h — callers legitimately distinguish them (the header's contract
* always said so). FORCE_FULL_REINDEX never escapes the orchestrator and
* stays internal. All three are distinct from CBM_NOT_FOUND, the normal
* "no incremental route; continue with a full index" sentinel. */
#define CBM_PIPELINE_ABORT_PRESERVE_DB (-2)
#define CBM_PIPELINE_FORCE_FULL_REINDEX (-3)
#define CBM_PIPELINE_PERSIST_FAILED (-4)
/* Canonicalize route-path parameter placeholders (":id", "{id}", "<id>",
* "${...}") to a single "{}" token so that client call sites and server
+14 -1
View File
@@ -107,7 +107,20 @@ static int reformat_files(const char *subdir, int max_files) {
static char *index_repo(void) {
char args[512];
snprintf(args, sizeof(args), "{\"repo_path\":\"%s\"}", g_repodir);
return cbm_mcp_handle_tool(g_srv, "index_repository", args);
char *resp = cbm_mcp_handle_tool(g_srv, "index_repository", args);
/* Fail-closed at the source: every caller asserts resp != NULL, but an
* error response (e.g. a transient pre-publication abort that preserves
* the previous index) used to slip through and cascade into dozens of
* mysterious count/query failures downstream — a 99-test pile-up on the
* Windows leg traced back to exactly this. Surface the real cause once,
* here, and let the existing non-NULL asserts fail loudly. */
if (resp && strstr(resp, "\"isError\":true")) {
fprintf(stderr, " index_repo: index_repository returned an error response:\n %.400s\n",
resp);
free(resp);
return NULL;
}
return resp;
}
/* Timed index: returns response, sets *elapsed_ms and *peak_rss_mb */