Repository navigation
Description
Related to #2001, which quotes this same hint text. That report is about an unreadable
directory aborting a run that should have continued. This one is about the shape of the
response after an abort that is legitimate. Different cause, different fix, so I filed it
separately rather than adding to that thread.
What happens
cbm_pipeline_run publishes by renaming a fully validated staging database over the
destination. Every abort before that rename leaves the existing database in place and
still serving. The pipeline says so in its return value: it has two distinct failure
codes, and it returns them from many places.
handle_index_repository throws that away. It tests rc == 0 and puts everything else
into one branch:
} 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.");
}So a caller sees status: "error" and a hint naming repo_path, when repo_path was
fine and the previous index is intact.
Why it costs the reader time
Two things go wrong at once.
The hint sends people to check a path that is not the problem. The usual real cause is
that files changed while the run was in flight, or a discovery or manifest phase failed
transiently. Neither has anything to do with repo_path.
The reader also cannot tell the two failures apart, and the right next step differs:
- Aborted before publication. The old index is intact and still answering queries. Retry.
- Publication failed. The staging database was validated but could not be put in place.
The old index may have been rolled back. Check free disk space and permissions on the
cache directory before retrying.
Today both arrive as the same string.
The header already promises this
src/pipeline/pipeline.h:59-68 tells callers they may distinguish the failures:
Treating any non-zero as "the run failed" is always correct. Callers that 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.
The two constants live at src/pipeline/pipeline_internal.h:35 and :37. A public caller
should not include that header, so the contract invites callers to read a value it does
not give them a name for.
Verified on c38aa35c
| Fact | Where |
|---|---|
return CBM_PIPELINE_ABORT_PRESERVE_DB — 13 sites |
src/pipeline/pipeline.c |
return CBM_PIPELINE_PERSIST_FAILED — 29 sites |
src/pipeline/pipeline.c |
| The two codes, defined privately | src/pipeline/pipeline_internal.h:35, :37 |
| The contract that names them | src/pipeline/pipeline.h:59-68 |
handle_index_repository |
src/mcp/mcp.c:8720 |
if (rc == 0), and the one else that collapses 42 return sites |
src/mcp/mcp.c:8934, :8948-8953 |
src/mcp/mcp.c contains no reference to CBM_PIPELINE_ABORT_PRESERVE_DB,
CBM_PIPELINE_PERSIST_FAILED, or any status string other than "error" on this path.
A fix already exists on a branch
77eb7eb3 ("fix(mcp): tell the truth about pre-publication index aborts") on
feat/atlas-r1 adds the two else if branches and moves the constants to the public
header. It is not in main, and main does not contain the change by any other route.
I am happy to port it to main as a pull request if you want it. I did not open one,
because #2001 sits on adjacent ground and you may want to sequence the two.
Activity
fix(mcp): name the cause when an index attempt stops before publication
fix(mcp): name the cause when an index attempt stops before publication