From c0786f0538c8a276ec71f70e2001bae60fa90bcc Mon Sep 17 00:00:00 2001 From: Rohan Shah Date: Wed, 16 Sep 2026 11:34:29 +0530 Subject: [PATCH 1/3] fix: fail builds with structural graph errors Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: bb884a52-d8d5-43f1-be33-215167fe2e86 --- CHANGELOG.md | 4 ++ src/cli.ts | 37 ++++++---- src/graph/build.ts | 4 ++ src/graph/workspace-cli.ts | 29 +++++++- test/cli-deep-failure.test.ts | 38 ++++++++--- test/cli-structural-failure.test.ts | 100 ++++++++++++++++++++++++++++ test/helpers.ts | 38 +++++++++++ 7 files changed, 226 insertions(+), 24 deletions(-) create mode 100644 test/cli-structural-failure.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 46786c861..23c2ca0ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,6 +59,10 @@ command, unchanged). Both keep getting every release. - **A brain refreshes its rules from upkeep** (#343), so a single empty pull no longer leaves it stuck without rules. +- **Structural graph failures now fail the build command** instead of printing a + successful wiring banner and exiting 0. Partial graph artifacts remain + available, workspace builds report failed children, and `--allow-partial` + continues to waive only incomplete LLM meaning passes. ## 0.17.0 diff --git a/src/cli.ts b/src/cli.ts index eb1397976..420bbb157 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -646,7 +646,16 @@ grouped(program.command("build"), GROUP.setup) throw err; }); process.stderr.write("\n"); - console.log(`✓ wiring: ${g.nodes} nodes (${fmt(g.byKind)}), ${g.edges} edges, ${g.cards} cards [${g.languages.join(", ")}]`); + const structuralFailed = g.structuralErrors > 0; + if (structuralFailed) { + console.error( + `✗ wiring incomplete: ${g.structuralErrors} structural error(s), ` + + `${g.nodes} nodes, ${g.edges} edges, ${g.cards} cards.`, + ); + process.exitCode = 1; + } else { + console.log(`✓ wiring: ${g.nodes} nodes (${fmt(g.byKind)}), ${g.edges} edges, ${g.cards} cards [${g.languages.join(", ")}]`); + } console.log(` parsed: ${g.parsed} of ${g.files} files (${g.reused} replayed from cache)`); // Worth one line: this build started from a graph the user never built *here*. if (g.seededFrom) console.log(` seeded: copied a starting graph from ${g.seededFrom} (git worktree)`); @@ -657,17 +666,21 @@ grouped(program.command("build"), GROUP.setup) console.log(` → ${g.contextDir}`); // The activation event. Everything here is a bucket or a fixed label: repo // scale rather than a file count, a language set rather than file names. - track( - "build_completed", - { - files_bucket: filesBucket(g.files), - langs: langsValue(g.languages), - mode: deep ? "deep" : "fast", - duration_bucket: durationBucket(Date.now() - buildStartedAt), - incremental: String(g.reused > 0), - }, - { repo: buildRoot }, - ); + if (structuralFailed) { + track("build_failed", { stage: "graph", code: errorCode(new Error(g.errors[0])) }, { repo: buildRoot }); + } else { + track( + "build_completed", + { + files_bucket: filesBucket(g.files), + langs: langsValue(g.languages), + mode: deep ? "deep" : "fast", + duration_bucket: durationBucket(Date.now() - buildStartedAt), + incremental: String(g.reused > 0), + }, + { repo: buildRoot }, + ); + } for (const e of g.errors) console.error(`✗ ${e}`); // A language whose native grammar didn't load was parsed by its WASM stand-in: // say which, and the install that fixes it. Nothing when everything loaded. diff --git a/src/graph/build.ts b/src/graph/build.ts index 178f7f58c..f8844a3a5 100644 --- a/src/graph/build.ts +++ b/src/graph/build.ts @@ -173,6 +173,9 @@ export interface GraphBuildResult { * and the packages to allow at install to fix it. Absent when everything loaded. */ nativeFallback?: NativeFallback; meaning: EnrichStats; + /** Recoverable Tier-1/build-output errors. These leave a partial graph on + * disk, but the CLI must not report the build as successful. */ + structuralErrors: number; errors: string[]; } @@ -467,6 +470,7 @@ export async function buildGraph( ? { nativeFallback: { languages: [...fellBackLabels].sort(), packages: missingNativePackages(fellBack) } } : {}), meaning, + structuralErrors: errors.length - meaning.errors.length, errors, }; } diff --git a/src/graph/workspace-cli.ts b/src/graph/workspace-cli.ts index 09755639f..4e443d3f7 100644 --- a/src/graph/workspace-cli.ts +++ b/src/graph/workspace-cli.ts @@ -12,6 +12,7 @@ import { brand, cmd, PACKAGE_FOR } from "../brand.js"; import { nativeFallbackNotice } from "./native.js"; import type { EngineConfig } from "../ai/providers.js"; import { formatAsk } from "../ask/ask.js"; +import { errorCode, track } from "../telemetry/index.js"; import type { Direction } from "./traverse.js"; import { federateAsk, @@ -53,6 +54,8 @@ export async function runWorkspaceBuild(root: string, opts: WorkspaceBuildOption // Native grammars load once per process, so the children share one answer: // collected here and printed once at the end, not once per repo. const fellBack = { languages: new Set(), packages: new Set() }; + const failedChildren: string[] = []; + let firstStructuralError: string | undefined; const buildChild = async (childDir: string, childName: string): Promise => { // Persisted BEFORE the child build itself runs, same as the single-repo // path in cli.ts, so this build and every later no-flag child build agree. @@ -72,7 +75,16 @@ export async function runWorkspaceBuild(root: string, opts: WorkspaceBuildOption const engine = new Graft({ ...opts.childConfig, contextDir: undefined }); if (opts.deep) await engine.init(childDir, { extensions: opts.extensions }); const g = await engine.graph(childDir, { llm: opts.deep, concurrency: opts.concurrency }); - console.log(`✓ ${childName}/: ${g.nodes} nodes, ${g.edges} edges, ${g.cards} cards [${g.languages.join(", ")}]`); + if (g.structuralErrors > 0) { + failedChildren.push(childName); + firstStructuralError ??= g.errors[0]; + console.error( + `✗ ${childName}/: wiring incomplete — ${g.structuralErrors} structural error(s), ` + + `${g.nodes} nodes, ${g.edges} edges, ${g.cards} cards.`, + ); + } else { + console.log(`✓ ${childName}/: ${g.nodes} nodes, ${g.edges} edges, ${g.cards} cards [${g.languages.join(", ")}]`); + } for (const e of g.errors) console.error(`✗ ${childName}/: ${e}`); for (const l of g.nativeFallback?.languages ?? []) fellBack.languages.add(l); for (const p of g.nativeFallback?.packages ?? []) fellBack.packages.add(p); @@ -90,7 +102,20 @@ export async function runWorkspaceBuild(root: string, opts: WorkspaceBuildOption // Each child self-ignored during its own build; the parent's federation // index (graft/workspace.json) is written outside buildGraph, so ignore it here too. ensureGitignored(root, contextDirFor(root, opts.override)); - console.log(`✓ workspace: ${children.length} repos federated → graft/workspace.json`); + if (failedChildren.length > 0) { + track( + "build_failed", + { stage: "graph", code: errorCode(new Error(firstStructuralError ?? "structural build failure")) }, + { repo: root }, + ); + console.error( + `✗ workspace incomplete: ${failedChildren.length} of ${children.length} repo(s) had structural errors ` + + `(${failedChildren.join(", ")}); partial graphs were federated → graft/workspace.json`, + ); + process.exitCode = 1; + } else { + console.log(`✓ workspace: ${children.length} repos federated → graft/workspace.json`); + } console.log(` graft/ is git-ignored — each teammate runs \`${cmd("graft build")}\` to regenerate it locally.`); const notice = nativeFallbackNotice( { languages: [...fellBack.languages].sort(), packages: [...fellBack.packages].sort() }, diff --git a/test/cli-deep-failure.test.ts b/test/cli-deep-failure.test.ts index a94069504..7a40beff7 100644 --- a/test/cli-deep-failure.test.ts +++ b/test/cli-deep-failure.test.ts @@ -14,7 +14,7 @@ import { createServer, type Server } from "node:http"; import { mkdirSync, writeFileSync } from "node:fs"; import { spawn } from "node:child_process"; import { join } from "node:path"; -import { tmpRepo } from "./helpers.js"; +import { tmpRepo, wasmParserFailureCliArgs } from "./helpers.js"; /** A gateway that answers every request the way an exhausted quota does. */ async function quotaExhaustedServer(): Promise<{ url: string; close: () => Promise; calls: () => number }> { @@ -56,18 +56,21 @@ async function runBuild( dir: string, baseUrl: string, extra: string[] = [], + injectWasmFailure = false, ): Promise<{ status: number | null; stderr: string; stdout: string }> { + const cliArgs = [ + "--provider", "openai", + "--api-key", "test-key", + "--base-url", baseUrl, + "--model", "test-model", + "build", dir, "--deep", "-j", "1", + ...extra, + ]; const child = spawn( process.execPath, - [ - "--import", "tsx", "src/cli.ts", - "--provider", "openai", - "--api-key", "test-key", - "--base-url", baseUrl, - "--model", "test-model", - "build", dir, "--deep", "-j", "1", - ...extra, - ], + injectWasmFailure + ? wasmParserFailureCliArgs(cliArgs) + : ["--import", "tsx", "src/cli.ts", ...cliArgs], { // No transport retries: the SDK's backoff is correct behaviour but would make // this test spend seconds waiting to learn what the first response already said. @@ -116,6 +119,21 @@ test("#127: --allow-partial keeps the same report but exits 0, for callers that } }); +test("--allow-partial cannot waive a structural failure during a degraded deep build", async () => { + const gateway = await quotaExhaustedServer(); + try { + const d = repo(5); + writeFileSync(join(d, "Broken.cs"), "public class Broken {}\n"); + const r = await runBuild(d, gateway.url, ["--allow-partial"], true); + + assert.equal(r.status, 1, `expected the structural failure to win\n${r.stderr}`); + assert.match(r.stderr, /wiring incomplete: 1 structural error\(s\)/); + assert.match(r.stderr, /the deep pass did not complete/); + } finally { + await gateway.close(); + } +}); + test("#127: the doomed calls stop instead of one per file", async () => { const gateway = await quotaExhaustedServer(); try { diff --git a/test/cli-structural-failure.test.ts b/test/cli-structural-failure.test.ts new file mode 100644 index 000000000..fd83d248c --- /dev/null +++ b/test/cli-structural-failure.test.ts @@ -0,0 +1,100 @@ +/** + * Recoverable extraction failures still write the healthy portion of the + * graph, but they must fail the command and must never print a success banner. + */ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { runCli, runCliWithWasmParserFailure, tmpRepo } from "./helpers.js"; + +function repo(tag: string): string { + const root = tmpRepo(tag); + writeFileSync(join(root, "Broken.cs"), "public class Broken {}\n"); + writeFileSync(join(root, "healthy.ts"), "export function healthy() { return 42; }\n"); + return root; +} + +function assertStructuralFailure( + r: ReturnType, + expectedErrors = 1, +): void { + assert.equal(r.status, 1, r.describe()); + assert.match(r.stderr, new RegExp(`wiring incomplete: ${expectedErrors} structural error\\(s\\)`), r.describe()); + assert.match(r.stderr, /Broken\.cs: parse failed/, r.describe()); + assert.doesNotMatch(r.stdout, /wiring:/, r.describe()); +} + +test("a structural parse failure writes the healthy graph but exits 1", () => { + const root = repo("cli-structural"); + const r = runCliWithWasmParserFailure(["build", root, "--no-gitignore", "--no-ignore"]); + + assertStructuralFailure(r); + const wiring = readFileSync(join(root, "graft", ".graph", "wiring.json"), "utf8"); + assert.match(wiring, /healthy/); +}); + +test("--allow-partial cannot waive structural failures", () => { + const root = repo("cli-structural-partial"); + const r = runCliWithWasmParserFailure([ + "build", root, "--allow-partial", "--no-gitignore", "--no-ignore", + ]); + + assertStructuralFailure(r); +}); + +test("--deep fallback without a key still fails for structural errors", () => { + const root = repo("cli-structural-deep-fallback"); + const r = runCliWithWasmParserFailure([ + "build", root, "--deep", "--allow-partial", "--no-gitignore", "--no-ignore", + ]); + + assertStructuralFailure(r); + assert.match(r.stderr, /falling back to the structural build/, r.describe()); +}); + +test("a replayed cached structural failure remains a failing build", () => { + const root = repo("cli-structural-incremental"); + assertStructuralFailure(runCliWithWasmParserFailure([ + "build", root, "--no-gitignore", "--no-ignore", + ])); + + const replay = runCli(["build", root, "--no-gitignore", "--no-ignore"]); + assert.equal(replay.status, 1, replay.describe()); + assert.match(replay.stderr, /wiring incomplete: 1 structural error\(s\)/, replay.describe()); + assert.match(replay.stdout, /parsed: 0 of 2 files \(2 replayed from cache\)/, replay.describe()); +}); + +test("a healthy structural build keeps the success banner and exits 0", () => { + const root = tmpRepo("cli-structural-success"); + writeFileSync(join(root, "healthy.ts"), "export function healthy() { return 42; }\n"); + + const r = runCli(["build", root, "--no-gitignore", "--no-ignore"]); + assert.equal(r.status, 0, r.describe()); + assert.match(r.stdout, /wiring:/, r.describe()); + assert.doesNotMatch(r.stderr, /wiring incomplete/, r.describe()); +}); + +test("a workspace preserves and federates partial child graphs but exits 1", () => { + const root = tmpRepo("cli-structural-workspace"); + const broken = join(root, "broken-repo"); + const healthy = join(root, "healthy-repo"); + mkdirSync(join(broken, ".git"), { recursive: true }); + mkdirSync(join(healthy, ".git"), { recursive: true }); + writeFileSync(join(broken, "Broken.cs"), "public class Broken {}\n"); + writeFileSync(join(broken, "healthy.ts"), "export const retained = 1;\n"); + writeFileSync(join(healthy, "healthy.ts"), "export const healthy = 1;\n"); + + const r = runCliWithWasmParserFailure([ + "build", root, "--no-gitignore", "--no-ignore", + ]); + + assert.equal(r.status, 1, r.describe()); + assert.match(r.stderr, /broken-repo\/: wiring incomplete/, r.describe()); + assert.match(r.stderr, /workspace incomplete: 1 of 2 repo\(s\)/, r.describe()); + assert.match(r.stdout, /healthy-repo\/:/, r.describe()); + assert.doesNotMatch(r.stdout, /workspace: 2 repos federated/, r.describe()); + assert.ok(existsSync(join(root, "graft", "workspace.json"))); + assert.ok(existsSync(join(broken, "graft", ".graph", "wiring.json"))); + assert.ok(existsSync(join(healthy, "graft", ".graph", "wiring.json"))); +}); diff --git a/test/helpers.ts b/test/helpers.ts index 73073bdb6..7557c2b79 100644 --- a/test/helpers.ts +++ b/test/helpers.ts @@ -92,6 +92,44 @@ export function runCli(args: string[], opts: { home?: string; timeoutMs?: number return res; } +/** + * Run the CLI after replacing the WASM parser's language setter with a + * deterministic failure. The real exhaustion crash depends on accumulated + * WASM state; this process-boundary seam exercises the same recoverable + * structural-error path without consuming unbounded memory. + */ +export function runCliWithWasmParserFailure( + args: string[], + opts: { home?: string; timeoutMs?: number } = {}, +): CliRun { + const res = spawnSync(process.execPath, wasmParserFailureCliArgs(args), { + encoding: "utf8", + timeout: opts.timeoutMs ?? 120_000, + env: opts.home ? homeEnv(opts.home) : process.env, + }) as CliRun; + res.describe = () => + [ + `graft ${args.join(" ")} (injected WASM failure)`, + ` status: ${res.status}${res.signal ? ` (signal ${res.signal})` : ""}`, + ` stdout: ${JSON.stringify(res.stdout ?? "")}`, + ` stderr: ${JSON.stringify(res.stderr ?? "")}`, + ].join("\n"); + return res; +} + +/** Node arguments for an async or sync CLI child with parser failure injected. */ +export function wasmParserFailureCliArgs(args: string[]): string[] { + const script = [ + 'import { Parser } from "web-tree-sitter";', + 'Parser.prototype.setLanguage = function () { throw new Error("injected WASM parse failure"); };', + // Commander detects eval mode and treats argv after the executable as user + // arguments, so there is no script-path slot in this process shape. + `process.argv = [process.execPath, ...${JSON.stringify(args)}];`, + 'await import("./src/cli.ts");', + ].join("\n"); + return ["--import", "tsx", "--input-type=module", "--eval", script]; +} + /** Summarizer that passes the source through unchanged, so tests control the text. */ export class PassthroughSummarizer implements Summarizer { async summarize(code: string): Promise { From 96409086f4187a12d5f706ce10fe2921ce98f02a Mon Sep 17 00:00:00 2001 From: shhrohan Date: Wed, 30 Sep 2026 23:45:05 +0530 Subject: [PATCH 2/3] fix: enforce en-US locale for consistent number formatting in tests --- src/ask/ask.ts | 6 +++--- src/claude/format.ts | 8 ++++---- src/claude/session-metrics.ts | 2 +- src/context/savings.ts | 6 +++--- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/ask/ask.ts b/src/ask/ask.ts index 561965428..756f8ef61 100644 --- a/src/ask/ask.ts +++ b/src/ask/ask.ts @@ -1557,9 +1557,9 @@ function askSavingsLine(r: AskResult, body: string): string { const saved = base - pack; const pct = Math.round((saved / base) * 100); return ( - `${tag()} tokens saved ≈ ${saved.toLocaleString()} (${pct}%) — this pack ≈ ` + - `${pack.toLocaleString()} tok vs reading the ${r.saved.files} source file(s) whole ≈ ` + - `${base.toLocaleString()} tok. Estimate (baseline = those files read in full).` + + `${tag()} tokens saved ≈ ${saved.toLocaleString("en-US")} (${pct}%) — this pack ≈ ` + + `${pack.toLocaleString("en-US")} tok vs reading the ${r.saved.files} source file(s) whole ≈ ` + + `${base.toLocaleString("en-US")} tok. Estimate (baseline = those files read in full).` + savingsTurnNudge(saved) ); } diff --git a/src/claude/format.ts b/src/claude/format.ts index d776dd787..b4803a45b 100644 --- a/src/claude/format.ts +++ b/src/claude/format.ts @@ -43,7 +43,7 @@ export function renderStatusline( // stands alone rather than carrying a rate nobody measured. const usd = dollarsSaved(saved, session?.inputCostMicros, session?.inputTokensBilled); const money = usd === null ? '' : ` · ~${formatDollars(usd)}`; - top.push(C.indigo(`~${saved.toLocaleString()} tok saved${money}`)); + top.push(C.indigo(`~${saved.toLocaleString("en-US")} tok saved${money}`)); } const bottom: string[] = []; @@ -186,9 +186,9 @@ export function formatRetrieval(ask: AskJson, cap = 5): string | null { const base = tokensOf(ask.saved!.baselineChars); const pct = Math.round((saved / base) * 100); return ( - `${body}\n${tag()} tokens saved ≈ ${saved.toLocaleString()} (${pct}%); this pack ≈ ` + - `${tokensOf(body.length).toLocaleString()} tok vs reading the ${ask.saved!.files} file(s) whole ≈ ` + - `${base.toLocaleString()} tok (estimate).` + `${body}\n${tag()} tokens saved ≈ ${saved.toLocaleString("en-US")} (${pct}%); this pack ≈ ` + + `${tokensOf(body.length).toLocaleString("en-US")} tok vs reading the ${ask.saved!.files} file(s) whole ≈ ` + + `${base.toLocaleString("en-US")} tok (estimate).` ); } diff --git a/src/claude/session-metrics.ts b/src/claude/session-metrics.ts index 3e9700245..3008b5b55 100644 --- a/src/claude/session-metrics.ts +++ b/src/claude/session-metrics.ts @@ -202,7 +202,7 @@ export function formatSessionStats(s: SessionSummary | null): string { ` graft reads: ${graft}`, ` source reads: ${source} (Read / Grep / Glob)`, ` mix: ${mix}`, - ` tokens saved: ~${saved.toLocaleString()}`, + ` tokens saved: ~${saved.toLocaleString("en-US")}`, ]; // Omitted, not zeroed, when no turn has been billed yet: a host whose hooks // name no transcript can't know what a token costs here, and a made-up rate diff --git a/src/context/savings.ts b/src/context/savings.ts index 75c3daf99..d29f15548 100644 --- a/src/context/savings.ts +++ b/src/context/savings.ts @@ -120,9 +120,9 @@ export function savingsLine(body: string, saved: Savings | undefined): string { const delta = base - pack; const pct = Math.round((delta / base) * 100); return ( - `${tag()} tokens saved ≈ ${delta.toLocaleString()} (${pct}%) — this output ≈ ` + - `${pack.toLocaleString()} tok vs reading the ${saved.files} file(s) it covers whole ≈ ` + - `${base.toLocaleString()} tok (estimate).` + + `${tag()} tokens saved ≈ ${delta.toLocaleString("en-US")} (${pct}%) — this output ≈ ` + + `${pack.toLocaleString("en-US")} tok vs reading the ${saved.files} file(s) it covers whole ≈ ` + + `${base.toLocaleString("en-US")} tok (estimate).` + savingsTurnNudge(delta) ); } From ecd4bed9c3fde7417f547b2c3c4291f17b42129d Mon Sep 17 00:00:00 2001 From: shhrohan Date: Wed, 30 Sep 2026 23:45:09 +0530 Subject: [PATCH 3/3] test: align process.argv injected by helper with commander expectations --- test/helpers.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/helpers.ts b/test/helpers.ts index 7557c2b79..a733bc887 100644 --- a/test/helpers.ts +++ b/test/helpers.ts @@ -124,7 +124,7 @@ export function wasmParserFailureCliArgs(args: string[]): string[] { 'Parser.prototype.setLanguage = function () { throw new Error("injected WASM parse failure"); };', // Commander detects eval mode and treats argv after the executable as user // arguments, so there is no script-path slot in this process shape. - `process.argv = [process.execPath, ...${JSON.stringify(args)}];`, + `process.argv = [process.execPath, "graft", ...${JSON.stringify(args)}];`, 'await import("./src/cli.ts");', ].join("\n"); return ["--import", "tsx", "--input-type=module", "--eval", script];