Skip to content

fix(mcp): name the artifact phase when a post-publish export fails - #2247

Open
DeusData wants to merge 1 commit into
mainfrom
distill/1789-artifact-error-surfacing
Open

DeusData wants to merge 1 commit into
mainfrom
distill/1789-artifact-error-surfacing

Conversation

@DeusData

Copy link
Copy Markdown
Owner

Distilled from #1789 by @umi008 (Ulises Millan Guerrero), carried here with Co-authored-by. #1789 stays open until this merges.

The fix

When artifact export failed after the graph database was already published, the MCP error path returned the generic hint regardless:

Pipeline failed. Check repo_path exists and contains source files.

So a caller whose export failed — a read-only checkout, typically — was told to check a repository path that was perfectly fine, while the one log line naming the real phase sat somewhere they were not looking. The pipeline already recorded its own export error; it simply never reached the caller.

The failing phase is now carried out of the pipeline into the MCP error, so the message names artifact export when that is what failed. The existing fail-hard contract is unchanged: this changes what the caller is told, not whether the run fails.

Why a distill rather than the original

Five of the nine files in #1789 were openspec/ spec-framework scaffolding. openspec/ does not exist on main, no other contributor uses it, and adopting a spec framework is a project-level decision rather than something that rides along with a bug fix. That was explained on 2026-09-02, and the code half was approved on merit the same day — the scaffolding is the only reason it did not land then.

This PR is that code half, unchanged, rebased onto current main: 4 files, +110 lines, no openspec/.

Verification

  • build/c/test-runner pipeline mcp597 passed, 1 failed.
  • The single failure is tool_check_index_coverage_freshness_uses_indexer_mtime_source_issue1714, which fails identically on unmodified main (confirmed by reverting these four files in the same worktree and re-running: 317 passed, 1 failed, same test). It is unrelated to this change and is being tracked separately.
  • Co-authored-by verified with git log --format='%(trailers:key=Co-authored-by)' rather than grep, since a blank line hides a trailer from git's parser while grep still finds it.

Credit for the diagnosis, the fix and the test coverage is entirely @umi008's.

Distilled from #1789 by Ulises Millan Guerrero, unchanged except for the
five openspec/ scaffolding files that could not land.

The indexing pipeline already logged its own phase error when artifact
export failed after publication, but the MCP failure path returned the
generic "Pipeline failed. Check repo_path" regardless. A caller whose
export failed was told to check a repository path that was fine, while the
one log line naming the real phase sat somewhere they were not looking.

The fix carries the failing phase out of the pipeline and into the MCP
error, so the message names artifact export when that is what failed. The
existing fail-hard contract is preserved -- this changes what the caller is
told, not whether the run fails.

Why this is a distill rather than the original: five of the nine files in
on main, no other contributor uses it, and adopting a spec framework is a
project-level decision rather than something that rides in on a bug fix.
That was explained on 2026-09-02 and the code half was approved on merit
the same day; the scaffolding is the only reason it did not land then.

Co-authored-by: Ulises Millan Guerrero <ulises.millanguerrero@gmail.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
@DeusData
DeusData force-pushed the distill/1789-artifact-error-surfacing branch from 9b1dfda to 9af73b3 Compare September 20, 2026 15:37

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant