Skip to content

Commit 07d5abe

Browse files
tech0328davion-knight
authored andcommitted
fix(scripts): surface the real launch error in check-changelog's run()
When a command check-changelog runs can't even launch (e.g. git-cliff not on PATH), spawnSync returns status:null with the actual ENOENT/EACCES reason on result.error. run() read only result.stderr/stdout and fell back to a generic "<label> failed" string, discarding result.error and wasting debugging time. Surface result.error.message (labeled) when status is null and no output is available, keeping the existing stderr/stdout and generic-fallback paths unchanged. run() gains injectable spawn/onFailure params (defaults preserve CLI behavior) and the module's execution is guarded behind an entrypoint check so the function can be unit-tested without running the checks. Adds test/unit/check-changelog-script.test.ts covering the real-ENOENT launch failure plus the stderr, generic-fallback, and success paths. Closes #7772
1 parent e8577cb commit 07d5abe

2 files changed

Lines changed: 135 additions & 51 deletions

File tree

scripts/check-changelog.mjs

Lines changed: 66 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -2,73 +2,88 @@
22
import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
33
import { tmpdir } from "node:os";
44
import { join } from "node:path";
5+
import { pathToFileURL } from "node:url";
56
import { spawnSync } from "node:child_process";
67

7-
const requestedChecks = new Set(process.argv.slice(2));
8-
const validArgs = new Set(["--root", "--mcp"]);
9-
const invalidArgs = [...requestedChecks].filter((arg) => !validArgs.has(arg));
8+
function main() {
9+
const requestedChecks = new Set(process.argv.slice(2));
10+
const validArgs = new Set(["--root", "--mcp"]);
11+
const invalidArgs = [...requestedChecks].filter((arg) => !validArgs.has(arg));
1012

11-
if (invalidArgs.length > 0) {
12-
console.error(`Unknown changelog check option: ${invalidArgs.join(", ")}`);
13-
process.exit(1);
14-
}
13+
if (invalidArgs.length > 0) {
14+
console.error(`Unknown changelog check option: ${invalidArgs.join(", ")}`);
15+
process.exit(1);
16+
}
1517

16-
const tempDir = mkdtempSync(join(tmpdir(), "loopover-changelog-"));
18+
const tempDir = mkdtempSync(join(tmpdir(), "loopover-changelog-"));
1719

18-
try {
19-
const checks = [
20-
{
21-
label: "root changelog",
22-
output: "CHANGELOG.md",
23-
command: "npm run changelog:root",
24-
selector: "--root",
25-
runner: () => {
26-
const generatedPath = join(tempDir, "CHANGELOG.md");
27-
run(["git-cliff", "--config", "cliff.toml", "--output", generatedPath], "root changelog");
28-
return generatedPath;
20+
try {
21+
const checks = [
22+
{
23+
label: "root changelog",
24+
output: "CHANGELOG.md",
25+
command: "npm run changelog:root",
26+
selector: "--root",
27+
runner: () => {
28+
const generatedPath = join(tempDir, "CHANGELOG.md");
29+
run(["git-cliff", "--config", "cliff.toml", "--output", generatedPath], "root changelog");
30+
return generatedPath;
31+
},
2932
},
30-
},
31-
{
32-
label: "MCP package changelog",
33-
output: "packages/loopover-mcp/CHANGELOG.md",
34-
command: "npm run changelog:mcp",
35-
selector: "--mcp",
36-
runner: () => {
37-
const generatedPath = join(tempDir, "MCP_CHANGELOG.md");
38-
const version = JSON.parse(readFileSync("packages/loopover-mcp/package.json", "utf8")).version;
39-
writeFileSync(generatedPath, readFileSync("packages/loopover-mcp/CHANGELOG.md", "utf8"));
40-
run(["node", "scripts/generate-mcp-changelog.mjs", "--output", generatedPath, "--version", version], "MCP package changelog");
41-
return generatedPath;
33+
{
34+
label: "MCP package changelog",
35+
output: "packages/loopover-mcp/CHANGELOG.md",
36+
command: "npm run changelog:mcp",
37+
selector: "--mcp",
38+
runner: () => {
39+
const generatedPath = join(tempDir, "MCP_CHANGELOG.md");
40+
const version = JSON.parse(readFileSync("packages/loopover-mcp/package.json", "utf8")).version;
41+
writeFileSync(generatedPath, readFileSync("packages/loopover-mcp/CHANGELOG.md", "utf8"));
42+
run(["node", "scripts/generate-mcp-changelog.mjs", "--output", generatedPath, "--version", version], "MCP package changelog");
43+
return generatedPath;
44+
},
4245
},
43-
},
44-
].filter((check) => requestedChecks.size === 0 || requestedChecks.has(check.selector));
46+
].filter((check) => requestedChecks.size === 0 || requestedChecks.has(check.selector));
4547

46-
const failures = [];
47-
for (const check of checks) {
48-
const generatedPath = check.runner();
49-
const expected = readFileSync(generatedPath, "utf8");
50-
const actual = readFileSync(check.output, "utf8");
51-
if (normalize(actual) !== normalize(expected)) failures.push(`${check.output} is stale; run ${check.command}.`);
52-
}
48+
const failures = [];
49+
for (const check of checks) {
50+
const generatedPath = check.runner();
51+
const expected = readFileSync(generatedPath, "utf8");
52+
const actual = readFileSync(check.output, "utf8");
53+
if (normalize(actual) !== normalize(expected)) failures.push(`${check.output} is stale; run ${check.command}.`);
54+
}
5355

54-
if (failures.length > 0) {
55-
console.error(failures.join("\n"));
56-
process.exit(1);
57-
}
56+
if (failures.length > 0) {
57+
console.error(failures.join("\n"));
58+
process.exit(1);
59+
}
5860

59-
console.log(`${checks.map((check) => check.output).join(", ")} current`);
60-
} finally {
61-
rmSync(tempDir, { recursive: true, force: true });
61+
console.log(`${checks.map((check) => check.output).join(", ")} current`);
62+
} finally {
63+
rmSync(tempDir, { recursive: true, force: true });
64+
}
6265
}
6366

64-
function run(command, label) {
65-
const result = spawnSync(command[0], command.slice(1), { encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] });
67+
/** Run a command, failing the process with its real output on a non-zero status. `spawn` and `onFailure` are
68+
* injectable purely for testability; every real caller uses the defaults. When the command cannot even
69+
* launch (`status` is null, e.g. the binary is not on PATH), `result.error` holds the actual ENOENT/EACCES
70+
* reason -- surface its message (#7772) instead of the generic `${label} failed`, which wastes debugging time. */
71+
export function run(command, label, { spawn = spawnSync, onFailure = defaultOnFailure } = {}) {
72+
const result = spawn(command[0], command.slice(1), { encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] });
6673
if (result.status !== 0) {
67-
process.stderr.write(result.stderr || result.stdout || `${label} failed`);
68-
process.exit(result.status ?? 1);
74+
const message = result.stderr || result.stdout || (result.error ? `${label}: ${result.error.message}\n` : `${label} failed`);
75+
onFailure(message, result.status ?? 1);
6976
}
7077
}
7178

79+
function defaultOnFailure(message, code) {
80+
process.stderr.write(message);
81+
process.exit(code);
82+
}
83+
7284
function normalize(value) {
7385
return value.replace(/\r\n/g, "\n").trimEnd();
7486
}
87+
88+
/* v8 ignore next -- entrypoint guard: runs the checks only as a CLI, so importing `run` for tests is a no-op. */
89+
if (import.meta.url === pathToFileURL(process.argv[1] ?? "").href) main();
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
import { describe, expect, it, vi } from "vitest";
2+
// @ts-expect-error -- plain .mjs script with no type declarations
3+
import { run } from "../../scripts/check-changelog.mjs";
4+
5+
/**
6+
* #7772: when a command can't even launch (binary missing from PATH), spawnSync returns `status: null` with
7+
* the real ENOENT/EACCES reason on `result.error`. `run()` used to fall back to a generic `"<label> failed"`
8+
* string, discarding that reason. It now surfaces `result.error.message`, so a misconfigured dependency
9+
* produces an actionable error instead of a mystery.
10+
*/
11+
describe("check-changelog run() launch-failure reporting (#7772)", () => {
12+
it("surfaces the real spawn error when the command cannot launch (real ENOENT)", () => {
13+
let captured = "";
14+
let exitCode: number | undefined;
15+
run(["loopover-definitely-not-a-real-binary-xyz"], "root changelog", {
16+
onFailure: (message: string, code: number) => {
17+
captured = message;
18+
exitCode = code;
19+
},
20+
});
21+
22+
// The message carries the actual reason (spawn ENOENT ...), labeled -- not the generic "... failed".
23+
expect(captured).toContain("root changelog");
24+
expect(captured).toMatch(/ENOENT|spawn/);
25+
expect(captured).not.toBe("root changelog failed");
26+
expect(exitCode).toBe(1);
27+
});
28+
29+
it("still falls back to the generic label when there is no error and no output (nonzero exit only)", () => {
30+
// A command that ran but exited non-zero with empty streams and no launch error -> generic fallback.
31+
const spawn = vi.fn(() => ({ status: 2, stdout: "", stderr: "", error: undefined }));
32+
let captured = "";
33+
let exitCode: number | undefined;
34+
run(["anything"], "MCP package changelog", {
35+
spawn: spawn as unknown as typeof import("node:child_process").spawnSync,
36+
onFailure: (message: string, code: number) => {
37+
captured = message;
38+
exitCode = code;
39+
},
40+
});
41+
42+
expect(captured).toBe("MCP package changelog failed");
43+
expect(exitCode).toBe(2);
44+
});
45+
46+
it("prefers real stderr/stdout output over the launch-error fallback", () => {
47+
const spawn = vi.fn(() => ({ status: 1, stdout: "", stderr: "cliff: bad config\n", error: undefined }));
48+
let captured = "";
49+
run(["git-cliff"], "root changelog", {
50+
spawn: spawn as unknown as typeof import("node:child_process").spawnSync,
51+
onFailure: (message: string) => {
52+
captured = message;
53+
},
54+
});
55+
56+
expect(captured).toBe("cliff: bad config\n");
57+
});
58+
59+
it("does nothing on success (status 0)", () => {
60+
const spawn = vi.fn(() => ({ status: 0, stdout: "", stderr: "", error: undefined }));
61+
const onFailure = vi.fn();
62+
run(["git-cliff"], "root changelog", {
63+
spawn: spawn as unknown as typeof import("node:child_process").spawnSync,
64+
onFailure,
65+
});
66+
67+
expect(onFailure).not.toHaveBeenCalled();
68+
});
69+
});

0 commit comments

Comments
 (0)