Skip to content

Commit f9b7cef

Browse files
theCodeDriftclaude
andcommitted
fix(cli): report engine failures in JSON and scan configs concurrently
`warn()` is a no-op under `--json`, so an engine that died reached the machine consumer as `{"success":false,"results":[]}` — indistinguishable from a clean run. Carry `failures` and `notices` in the envelope, omitted when empty, the way `skipped` already is. Scan the ast-grep configs with `Promise.all` instead of awaiting each in turn, so a project holding both `sg/rules/` and the legacy `rules/` does not pay two subprocess latencies in series. Extract the "directory is absent" errno guard to `rules/errno.ts`, one definition shared rather than a private set per caller. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3
1 parent c57dcef commit f9b7cef

4 files changed

Lines changed: 108 additions & 15 deletions

File tree

‎packages/cli/src/commands/check.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,10 @@ export const checkCommand = defineCommand({
282282
const telemetry = await getTelemetry(cwd);
283283

284284
// Warnings/notices are advisory human output; suppress them under --json so
285-
// the machine output stays the { success, results, skipped? } shape.
285+
// the machine output stays the
286+
// { success, results, skipped?, failures?, notices? } shape. Engine
287+
// failures and notices are carried in that envelope instead, since a
288+
// machine consumer cannot read stderr prose.
286289
const warn = (message: string) => {
287290
if (!args.json) console.error(message);
288291
};
@@ -422,6 +425,12 @@ export const checkCommand = defineCommand({
422425
success: exitCode === 0,
423426
results,
424427
...(plan.skipped.length > 0 ? { skipped: plan.skipped } : {}),
428+
...(dispatched.failures.length > 0
429+
? { failures: dispatched.failures }
430+
: {}),
431+
...(dispatched.notices.length > 0
432+
? { notices: dispatched.notices }
433+
: {}),
425434
});
426435
console.log(JSON.stringify(output));
427436
} else {

‎packages/cli/src/rules/dispatch.ts‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,12 @@ import { join } from "node:path";
33

44
import type { CheckResult } from "../types/check";
55
import { dedupeFindings, ENGINE_LAYOUTS, type EngineName } from "./engines";
6+
import { isMissingDirectory } from "./errno";
67
import { executeRuntimeRules } from "./runtime/harness";
78
import type { RuntimeRule } from "./runtime/discover";
89
import { runAstGrepScan } from "./scan";
910
import { runVale } from "./vale/run";
1011

11-
/** Errno values that mean "the directory is not there", and nothing worse. */
12-
const ABSENT_DIRECTORY_CODES = new Set(["ENOENT", "ENOTDIR"]);
13-
1412
/**
1513
* Whether `.taskless/vale/rules/` holds anything to run.
1614
*
@@ -34,8 +32,7 @@ export async function hasValeRules(cwd: string): Promise<boolean> {
3432
);
3533
return entries.some((entry) => entry.endsWith(".yml"));
3634
} catch (error) {
37-
const code = (error as NodeJS.ErrnoException).code;
38-
if (code !== undefined && ABSENT_DIRECTORY_CODES.has(code)) return false;
35+
if (isMissingDirectory(error)) return false;
3936
throw error;
4037
}
4138
}
@@ -102,17 +99,22 @@ export interface DispatchResult {
10299
* `sg/rules/` and the legacy `.taskless/rules/` are scanned separately, so a
103100
* rule present in both reports twice; the finding is its own identity, so
104101
* identical matches collapse.
102+
*
103+
* The configs are scanned concurrently, for the same reason the engines are:
104+
* each is an independent subprocess over the same paths, and a project holding
105+
* both `sg/rules/` and the legacy `rules/` should not pay their latencies in
106+
* series. `Promise.all` preserves input order in its output, so the flattened
107+
* results are ordered by config exactly as the sequential loop left them.
105108
*/
106109
async function runAstGrepEngine(
107110
options: DispatchOptions
108111
): Promise<EngineOutcome> {
109-
const results: CheckResult[] = [];
110-
for (const configPath of options.astGrepConfigPaths) {
111-
const scan = await runAstGrepScan(options.cwd, options.paths, {
112-
configPath,
113-
});
114-
results.push(...scan.results);
115-
}
112+
const scans = await Promise.all(
113+
options.astGrepConfigPaths.map((configPath) =>
114+
runAstGrepScan(options.cwd, options.paths, { configPath })
115+
)
116+
);
117+
const results: CheckResult[] = scans.flatMap((scan) => scan.results);
116118
return { engine: "sg", results: dedupeFindings(results) };
117119
}
118120

@@ -221,4 +223,3 @@ export async function runEngines(
221223
: 0,
222224
};
223225
}
224-

‎packages/cli/src/schemas/check.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,17 @@ export const outputSchema = z.object({
3838
.array(skippedRuntimeRuleSchema)
3939
.optional()
4040
.describe("Runtime rules present but not executed"),
41+
// Human output for these is `warn()`, which is a no-op under `--json`. Absent
42+
// from the envelope, a machine consumer reads `{"success":false,"results":[]}`
43+
// for a Vale that died and cannot tell it from a clean run.
44+
failures: z
45+
.array(z.string())
46+
.optional()
47+
.describe("Engines that were present and failed"),
48+
notices: z
49+
.array(z.string())
50+
.optional()
51+
.describe("Advisory messages: engines that could not run"),
4152
});
4253

4354
/** Error schema for `taskless check --json` on failure */

‎packages/cli/test/vale-orchestration.test.ts‎

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { execFile } from "node:child_process";
12
import {
23
chmodSync,
34
mkdirSync,
@@ -6,7 +7,8 @@ import {
67
writeFileSync,
78
} from "node:fs";
89
import { tmpdir } from "node:os";
9-
import { join } from "node:path";
10+
import { join, resolve } from "node:path";
11+
import { promisify } from "node:util";
1012

1113
import { afterEach, describe, expect, it, vi } from "vitest";
1214

@@ -79,6 +81,41 @@ function makeMixedProject(options?: {
7981
/** The committed config `makeMixedProject` writes, as `check` would resolve it. */
8082
const sgConfigPaths = [".taskless/sg/sgconfig.yml"];
8183

84+
const execFileAsync = promisify(execFile);
85+
const binPath = resolve(import.meta.dirname, "../dist/index.js");
86+
87+
/** Run the built CLI, tolerating a non-zero exit. */
88+
async function runCli(
89+
args: string[]
90+
): Promise<{ stdout: string; exitCode: number }> {
91+
try {
92+
const { stdout } = await execFileAsync("node", [binPath, ...args]);
93+
return { stdout, exitCode: 0 };
94+
} catch (error) {
95+
const execError = error as { stdout: string; code: number };
96+
return { stdout: execError.stdout ?? "", exitCode: execError.code };
97+
}
98+
}
99+
100+
/** The `--json` line, ignoring any preceding migration notice. */
101+
function parseJson(stdout: string): {
102+
success: boolean;
103+
results: unknown[];
104+
failures?: string[];
105+
notices?: string[];
106+
} {
107+
const line = stdout
108+
.trim()
109+
.split("\n")
110+
.findLast((entry) => entry.trim().startsWith("{"));
111+
return JSON.parse(line ?? "{}") as {
112+
success: boolean;
113+
results: unknown[];
114+
failures?: string[];
115+
notices?: string[];
116+
};
117+
}
118+
82119
describe("hasValeRules", () => {
83120
it("is false for a scaffolded-but-empty rules directory", async () => {
84121
// The common state after `taskless init`. Spawning Vale per check to
@@ -295,3 +332,38 @@ describe("runEngines when Vale is unavailable", () => {
295332
}
296333
);
297334
});
335+
336+
describe("an engine failure under --json", () => {
337+
it("reaches the machine envelope, not only the suppressed warning", async () => {
338+
// `warn()` is a no-op under `--json`, so without a field for it the
339+
// consumer sees `{"success":false,"results":[]}` and cannot tell a broken
340+
// engine from a clean run. A CI script reading that treats a dead engine
341+
// as a pass.
342+
const cwd = makeMixedProject({ valeRules: false });
343+
// An unparseable rule file: ast-grep exits non-zero and the engine fails
344+
// with no findings, the exact shape that used to read as clean.
345+
writeFileSync(
346+
join(cwd, ".taskless", "sg", "rules", "broken.yml"),
347+
"id: broken\nlanguage: javascript\nrule:\n bogusKey: nope\n"
348+
);
349+
350+
const { stdout, exitCode } = await runCli(["check", "-d", cwd, "--json"]);
351+
const output = parseJson(stdout);
352+
353+
expect(exitCode).toBe(1);
354+
expect(output.success).toBe(false);
355+
expect(output.results).toEqual([]);
356+
expect(output.failures).toHaveLength(1);
357+
expect(output.failures?.[0]).toContain("sg engine failed");
358+
});
359+
360+
it("omits both fields when nothing failed, as `skipped` does", async () => {
361+
const cwd = makeMixedProject({ valeRules: false });
362+
const { stdout } = await runCli(["check", "-d", cwd, "doc.md", "--json"]);
363+
const output = parseJson(stdout);
364+
365+
expect(output.success).toBe(true);
366+
expect(output.failures).toBeUndefined();
367+
expect(output.notices).toBeUndefined();
368+
});
369+
});

0 commit comments

Comments
 (0)