Skip to content

index_repository reports an abort that preserved the index as an indistinguishable error #2020

Open

Description

@CaptainMittens

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingpriority/highNeeds near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker.ux/behaviorDisplay bugs, docs, adoption UX

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions