Skip to content

Fix structural build failure exit status - #407

Open
shhrohan wants to merge 1 commit into
trailhq:mainfrom
shhrohan:fix/structural-build-exit-status
Open

shhrohan wants to merge 1 commit into
trailhq:mainfrom
shhrohan:fix/structural-build-exit-status

Conversation

@shhrohan

Copy link
Copy Markdown

Summary

Structural graph extraction failures currently leave a partial graph on disk, but the CLI still prints the normal successful wiring banner, emits build_completed, and exits 0. This makes parser/runtime failures look indistinguishable from a complete build.

This change:

  • exposes a structuralErrors count separately from optional meaning-tier errors;
  • prints wiring incomplete and exits 1 when structural errors occurred;
  • emits build_failed instead of build_completed for an incomplete structural build;
  • keeps detailed per-file errors and all healthy partial graph artifacts;
  • continues every workspace child, federates the available child graphs, identifies failed children, and exits 1 for an incomplete workspace;
  • preserves --allow-partial exclusively for degraded LLM meaning passes.

Why

A recoverable parser failure is collected in GraphBuildResult.errors, so engine.graph() resolves successfully and writes the healthy portion of the graph. The CLI previously treated any resolved result as success. Automation therefore received exit code 0 even when files were missing from the graph.

The partial artifacts are useful for diagnosis and recovery, so this PR does not turn recoverable extraction errors into thrown exceptions or roll back graph output. It changes only the command-level completion status and reporting.

Reproduction

The regression tests inject a deterministic web-tree-sitter parser failure in a child process, using only synthetic C# and TypeScript fixtures. Before this change, the command prints the successful wiring banner and exits 0. After this change, it:

  • retains the healthy TypeScript nodes in wiring.json;
  • reports the failed C# extraction;
  • suppresses the successful wiring banner;
  • exits 1.

Coverage also includes cached/replayed failures, --deep fallback, a real degraded deep pass with --allow-partial, successful builds, and workspace federation.

Relationship to #340

This is a companion to #340, not a replacement for it.

#340 addresses WASM parser/tree lifetime, poisoned parser recovery, container ownership, and retrying failed extraction-cache entries. This PR deliberately does not duplicate those parser-lifetime or cache-retry changes. It addresses the separate CLI/API contract issue: if structural errors remain in a build result for any reason, the command must not report success.

Behavior guarantees

  • Healthy graph data remains written and queryable after a recoverable structural failure.
  • Detailed errors remain local and are printed as before.
  • Telemetry sends only the existing fixed build_failed stage/code fields; no error text is added to the telemetry contract.
  • --allow-partial can still accept meaning-tier failures, but cannot reset a structural failure exit status.
  • Workspace builds do not stop at the first failed child and still write workspace.json.

Tests

  • npm run build
  • node --import tsx --test test\\cli-structural-failure.test.ts test\\cli-deep-failure.test.ts test\\graph-incremental.test.ts test\\generic-extract.test.ts test\\graph-workspace-include-dir.test.ts test\\telemetry-contract.test.ts
    • 64 passed, 0 failed, 1 pre-existing Windows chmod-permission skip
  • Final CLI/telemetry regression run: 25 passed, 0 failed

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: bb884a52-d8d5-43f1-be33-215167fe2e86
@trailhq-graft

trailhq-graft Bot commented Sep 16, 2026

Copy link
Copy Markdown

🌱 graft blast radius

2 areas changed → 6 areas can be affected. 15 dependent symbols, depth 2.
Tests: Graph Construction has tests the diff did not touch.
Tag: @anirudhkumar-nanonets — 6 of 8 areas · @shhdwi — 6 of 8 areas · @bhavesh-gupta-investis — Synchronous Claude Execution

flowchart TB
  A0(("Pull Request Review<br/>8 symbols"))
  A1(("Workspace Graph Freshness<br/>3 symbols"))
  A2(("Viewer Build<br/>1 symbol"))
  A3(("MCP Tool Invocation<br/>1 symbol"))
  A4(("Graph Engine<br/>1 symbol"))
  AX(("1 smaller area<br/>1 symbol"))
  classDef reached fill:#D9EDF3,stroke:#3AA7C9,stroke-width:1.5px,color:#0E313C;
  class A0,A1,A2,A3,A4 reached;
  classDef tail fill:#EEF2F3,stroke:#9AA4A9,stroke-width:1px,color:#3A4247;
  class AX tail;
Loading
Can be affected Symbols Nearest hop Reached from
Pull Request Review 8 src/app/brain-build.ts:L251-L358 readRepository — calls, depth 1 Graph Construction
Workspace Graph Freshness 3 src/graph/refresh.ts:L150-L227 ensureFreshGraph — calls, depth 1 Graph Construction
Viewer Build 1 scripts/build-viewer.mjs:L1-L45 build-viewer.mjs — calls, depth 2 Graph Construction
MCP Tool Invocation 1 src/mcp/tools.ts:L216-L244 callTool — calls, depth 2 Graph Construction
Graph Engine 1 src/engine.ts:L91-L101 graph — calls, depth 1 Graph Construction
Synchronous Claude Execution 1 src/claude/sync-run.ts:L19-L33 runSync — calls, depth 2 Graph Construction
Who knows this code — 3 people across 8 areas
Area Who knows it
Graph Construction · changed @anirudhkumar-nanonets — 18 commits, last 27d ago · @shhdwi — 7 commits, last 1mo ago
CLI Entry Point · changed @anirudhkumar-nanonets — 39 commits, last 6d ago · @shhdwi — 23 commits, last 1mo ago
Pull Request Review · affected @anirudhkumar-nanonets — 12 commits, last 6d ago
Workspace Graph Freshness · affected @anirudhkumar-nanonets — 7 commits, last 1mo ago · @shhdwi — 6 commits, last 1mo ago
Viewer Build · affected @shhdwi — 2 commits, last 1mo ago
MCP Tool Invocation · affected @shhdwi — 14 commits, last 1mo ago · @anirudhkumar-nanonets — 7 commits, last 14d ago
Graph Engine · affected @anirudhkumar-nanonets — 16 commits, last 2mo ago
Synchronous Claude Execution · affected @shhdwi — 3 commits, last 2mo ago · @bhavesh-gupta-investis — 1 commit, last 23d ago

Ownership is git history over each area's own files, weighted towards recent work (120-day half-life). Merge commits and bots are dropped, and you are dropped from your own PR. A name with no @ has no GitHub handle in its commit email — tag them by hand, or add a .mailmap entry. A suggestion from history, not a CODEOWNERS rule.

All 15 dependent symbols, grouped by area

Pull Request Review — 8 symbols in 6 files

  • src/app/brain-build.ts:L251-L358 — readRepository (calls, depth 1)
    254: const tag = `${job.owner}/${job.repo}`;
  • src/app/review.ts:L45-L99 — reviewPullRequest (calls, depth 1)
    51: const checkout = checkoutPullRequest({ owner: job.owner, repo: job.repo, number: job.number, baseRef: job.baseRef, token, log });
  • src/app/brain-build-worker.ts:L1-L83 — brain-build-worker.ts (calls, depth 2)
    12: import { readRepository, type BrainBuildJob, type RepoReadAuth } from "./brain-build.js";
  • src/app/brain-build-worker.ts:L29-L32 — DoneMessage (references, depth 2)
  • src/app/brain-build.ts:L237-L239 — buildRepoIntoBrain (calls, depth 2)
  • src/app/review-process.ts:L179-L183 — childReviewer (references, depth 2)
  • src/app/review-worker.ts:L67-L87 — run (calls, depth 2)
  • src/app/server.ts:L34-L46 — AppSeams (references, depth 2)

Workspace Graph Freshness — 3 symbols in 2 files

  • src/graph/refresh.ts:L150-L227 — ensureFreshGraph (calls, depth 1)
    165: // — run graft build" message is the right answer. Auto-building a whole repo
  • src/graph/workspace.ts:L709-L722 — splitWorkspace (calls, depth 1)
    712: buildChild: (childDir: string, childName: string) => Promise<void>,
  • src/graph/refresh.ts:L235-L261 — ensureFreshChildren (calls, depth 2)

Viewer Build — 1 symbol in 1 file

  • scripts/build-viewer.mjs:L1-L45 — build-viewer.mjs (calls, depth 2)
    3: * assets). Runs as part of `npm run build`; the bundle ships in the package

MCP Tool Invocation — 1 symbol in 1 file

  • src/mcp/tools.ts:L216-L244 — callTool (calls, depth 2)

Graph Engine — 1 symbol in 1 file

  • src/engine.ts:L91-L101 — graph (calls, depth 1)
    91: graph(dir: string, opts: GraphRunOptions = {}): Promise<GraphBuildResult> {

Synchronous Claude Execution — 1 symbol in 1 file

  • src/claude/sync-run.ts:L19-L33 — runSync (calls, depth 2)
Test signal per changed area — 1 ⚠ · 1 –

Reached = a node under a test path has a resolved edge into the changed symbol. It undercounts anything called indirectly — through a CLI, a spawned process or a dynamic import — so read a low ratio as “look here”, never as a coverage gate.

  • Graph Construction — 2 of 3 reached · 33 test files reach it, none changed here
    • not reached: buildChild
  • CLI Entry Point — no function, method or class changed here
33 test suites also reference this code

41 symbols, kept out of the diagram and the table so they cannot crowd out the areas a reviewer has to look at.

  • test/ask-index.test.ts
  • test/ask.test.ts
  • test/container-extract.test.ts
  • test/context-only-dir.test.ts
  • test/context.test.ts
  • test/covers.test.ts
  • test/generic-extract.test.ts
  • test/graph-go.test.ts
  • test/graph-incremental.test.ts
  • test/graph-invariants.test.ts
  • test/graph-java.test.ts
  • test/graph-languages.test.ts
  • test/graph-php.test.ts
  • test/graph-posix-paths.test.ts
  • test/graph-python.test.ts
  • test/graph-r-classes.test.ts
  • test/graph-r-phase3.test.ts
  • test/graph-r-phase4.test.ts
  • test/graph-r-phase5.test.ts
  • test/graph-r.test.ts
  • …13 more

⚠️ 1 changed file not in the graph (CHANGELOG.md) — no parser claims the extension, or the index predates the file.

graft blast · origin/main...HEAD · depth 2 · 7 changed files

Open the interactive graph → — click an area to see its dependent symbols at file:line.

github-actions Bot added a commit that referenced this pull request Sep 16, 2026
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