Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ Tries dedicated tools first, falls back to built-in:

## Flags

`--skip-tests`, `--ci`, `--fail-under N`, `--json`, `--badge`, `--sarif`, `--upload`, `--top [N]`, `--diff [base]`, `--watch`, `-v`, `-h`
`--skip-tests`, `--test-timeout MS`, `--ci`, `--fail-under N`, `--json`, `--badge`, `--sarif`, `--upload`, `--top [N]`, `--diff [base]`, `--watch`, `-v`, `-h`

## Testing

Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ Auto-detects: TypeScript/JavaScript/Dart, React/Vue/Svelte/Flutter, Vite/Webpack
| Flag | Description |
|------|-------------|
| `--skip-tests` | Skip test execution (fast mode) |
| `--test-timeout MS` | Per-project test run limit in ms, 1–2147483647 (default 120000; overrides `checks.testing.settings.timeoutMs` in `.vcqa.json`) |
| `--ci` | CI mode (exit 1 if score < `failUnder` from config, else 60) |
| `--fail-under N` | Exit 1 if score < N; overrides config and `--ci` |
| `--json` | JSON output |
Expand Down
12 changes: 12 additions & 0 deletions src/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,18 @@ describe("CLI flags", () => {
}
}, 30_000);

it("--test-timeout rejects a value a timer cannot hold, or no value (#106)", () => {
for (const value of ["3000000000", "0", ""]) {
try {
execSync(`node ${CLI} --json --test-timeout ${value} .`, { encoding: "utf-8", timeout: 30_000, cwd: TMP, stdio: "pipe" });
expect.unreachable(`--test-timeout ${value} should exit 2`);
} catch (e: any) {
expect(e.status).toBe(2);
expect(String(e.stderr)).toContain("from 1 to 2147483647");
}
}
}, 30_000);

it("--fail-under does not exit when score is above threshold", () => {
const out = run("--skip-tests --json --fail-under 0 .");
const report = JSON.parse(out);
Expand Down
10 changes: 10 additions & 0 deletions src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { isPartialScan } from "./history.js";
import { postPRComment } from "./pr-comment.js";
import { generatePages } from "./report/html.js";
import { buildReportHistorySnapshot, withFreshAnalyzerSnapshots } from "./report-contract.js";
import { isValidTestTimeout, MAX_TEST_TIMEOUT_MS } from "./runners/testing.js";
import { computeTrend, formatTrend, type TrendDelta } from "./trend.js";
import type { VibeReport, WorkspaceInfo } from "./types.js";
import { buildReportUploadPayload, currentGitSha } from "./upload.js";
Expand All @@ -34,6 +35,7 @@ interface ParsedFlags {
jsonOnly: boolean;
ciMode: boolean;
skipTests: boolean;
testTimeoutMs: number | null;
watchMode: boolean;
badgeMode: boolean;
sarifMode: boolean;
Expand Down Expand Up @@ -67,6 +69,11 @@ function parseFlags(): ParsedFlags {

const topN = parseValueFlag("--top", 5) ?? 0;
const failUnder = parseValueFlag("--fail-under");
const testTimeoutMs = parseValueFlag("--test-timeout");
if (args.includes("--test-timeout") && !isValidTestTimeout(testTimeoutMs)) {
console.error(`--test-timeout needs a whole number of milliseconds from 1 to ${MAX_TEST_TIMEOUT_MS}, e.g. --test-timeout 300000`);
process.exit(2);
}

let diffBase: string | null = null;
const diffIdx = args.indexOf("--diff");
Expand All @@ -87,6 +94,7 @@ function parseFlags(): ParsedFlags {
jsonOnly: flags.has("--json"),
ciMode: flags.has("--ci"),
skipTests: flags.has("--skip-tests"),
testTimeoutMs,
watchMode: flags.has("--watch"),
badgeMode: flags.has("--badge"),
sarifMode: flags.has("--sarif"),
Expand Down Expand Up @@ -125,6 +133,7 @@ function printHelp(): void {

\x1b[1mFlags:\x1b[0m
--skip-tests Skip test execution (faster scan)
--test-timeout MS Per-project test run limit in ms (default: 120000)
--ci CI mode (exit 1 if score < failUnder from config, else 60)
--fail-under N Exit 1 if score below N; overrides config and --ci (e.g. --fail-under 80)
--json Output JSON only (no terminal UI)
Expand Down Expand Up @@ -633,6 +642,7 @@ async function main() {
// Run scan using core API with progress output
const report = await scan(cwd, {
skipTests,
testTimeoutMs: flags.testTimeoutMs ?? undefined,
config,
diffBase,
ciContext,
Expand Down
9 changes: 8 additions & 1 deletion src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { join } from "node:path";

export interface VcqaConfig {
/** Disable or configure individual checks */
checks?: Record<string, { enabled?: boolean; ignore?: string[] }>;
checks?: Record<string, { enabled?: boolean; ignore?: string[]; settings?: Record<string, unknown> }>;
/** Extra glob patterns to ignore, merged with the versioned default exclusion policy. */
ignore?: string[];
/** Default fail-under threshold (overridden by --fail-under flag) */
Expand Down Expand Up @@ -49,6 +49,13 @@ export function getCheckIgnore(config: VcqaConfig, checkName: string): string[]
return config.checks?.[checkName]?.ignore;
}

/** Get per-check settings (`checks.<name>.settings`). Values are unvalidated —
* the check that reads a setting validates it and falls back to its default. */
export function getCheckSettings(config: VcqaConfig, checkName: string): Record<string, unknown> {
const settings = config.checks?.[checkName]?.settings;
return settings && typeof settings === "object" && !Array.isArray(settings) ? settings : {};
}

function validate(raw: Record<string, unknown>): VcqaConfig {
const config: VcqaConfig = {};

Expand Down
13 changes: 11 additions & 2 deletions src/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { resolve } from "node:path";
import { buildAnalyzerSnapshots } from "./analyzer-snapshot.js";
import { CHECK_META, type CheckMeta, getCheckMeta } from "./check-meta.js";
import { type CiContext, detectCiContext, type ReportProvenanceMeta } from "./ci-context.js";
import { getCheckIgnore, isCheckEnabled, loadConfig, type VcqaConfig } from "./config.js";
import { getCheckIgnore, getCheckSettings, isCheckEnabled, loadConfig, type VcqaConfig } from "./config.js";
import { detectRepoUrl, detectStack, detectWorkspace } from "./detect.js";
import { buildFileInventory } from "./file-inventory.js";
import { setGlobalIgnore, setGlobalIgnoreNames, setGlobalScanPolicy, setGlobalSrcRoots } from "./fs-utils.js";
Expand Down Expand Up @@ -83,6 +83,8 @@ type ScoreMode = "available-scored" | "available-unscored" | "not-applicable" |
export interface ScanOptions {
/** Skip test execution (faster scan). Default: false */
skipTests?: boolean;
/** Time limit for each test run, in ms. Overrides `checks.testing.settings.timeoutMs`. Default: 120000 */
testTimeoutMs?: number;
/** Base ref the caller will filter issues against (`--diff`). Recorded in
* `meta.scan.diffBase` so the report says it is partial. Default: null */
diffBase?: string | null;
Expand Down Expand Up @@ -168,7 +170,14 @@ export async function scan(cwd: string, options: ScanOptions = {}): Promise<Vibe
{ name: "env-validation", fn: () => runEnvValidation(resolvedCwd) },
{ name: "git-hygiene", fn: () => runGitHygiene(resolvedCwd, fileInventory) },
{ name: "memory-safety", fn: () => runMemorySafety(resolvedCwd, workspace, fileInventory) },
{ name: "testing", fn: () => runTesting(resolvedCwd, stack, skipTests, srcRoots, workspace, fileInventory) },
{
name: "testing",
fn: () =>
runTesting(resolvedCwd, stack, skipTests, srcRoots, workspace, fileInventory, {
timeoutMs: options.testTimeoutMs,
settings: getCheckSettings(config, "testing"),
}),
},
{ name: "secrets", fn: () => runSecrets(resolvedCwd, fileInventory) },
{ name: "security", fn: () => runSecurity(resolvedCwd, fileInventory) },
{ name: "dependencies", fn: () => runDependencies(resolvedCwd, stack) },
Expand Down
17 changes: 16 additions & 1 deletion src/monitor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@ import { watch, existsSync, mkdirSync, writeFileSync, readFileSync } from "node:
import { execFile, execSync } from "node:child_process";
import { fileURLToPath } from "node:url";

import { getCheckSettings, loadConfig } from "./config.js";
import { detectStack, detectWorkspace } from "./detect.js";
import { loadHistory } from "./history.js";
import { resolveTestTimeout, SCAN_BASE_TIMEOUT_MS, scanTimeoutWithTests, testRunTargets } from "./runners/testing.js";
import type { CheckResult } from "./types.js";

const VERSION = JSON.parse(readFileSync(new URL("../package.json", import.meta.url), "utf-8")).version;
Expand Down Expand Up @@ -163,6 +165,19 @@ function getGitChanges(cwd: string): GitChange[] {

// ── Scan via child process — UI never freezes ──

/** The scan subprocess's limit: it must outlast the configured test timeout
* (checks.testing.settings.timeoutMs) for every test project, or the scan is
* killed before that setting can take effect. */
function scanTimeoutMs(cwd: string, skipTests: boolean): number {
if (skipTests) return SCAN_BASE_TIMEOUT_MS;
try {
const { ms } = resolveTestTimeout({ settings: getCheckSettings(loadConfig(cwd), "testing") });
return scanTimeoutWithTests(ms, testRunTargets(cwd, undefined, detectWorkspace(cwd)).length);
} catch {
return scanTimeoutWithTests(resolveTestTimeout().ms, 1);
}
}

function runScanProcess(
cwd: string,
skipTests: boolean,
Expand All @@ -172,7 +187,7 @@ function runScanProcess(
if (skipTests) args.unshift("--skip-tests");

execFile(process.execPath, [CLI_PATH, ...args], {
timeout: 120_000,
timeout: scanTimeoutMs(cwd, skipTests),
maxBuffer: 10 * 1024 * 1024,
env: { ...process.env, VCQA_NO_UPDATE_CHECK: "1" },
}, (err, stdout) => {
Expand Down
214 changes: 214 additions & 0 deletions src/runners/exec-tree.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,214 @@
/** vibecodeqa/cli#106 — the supervisor behind runWithTreeKill must leave no
* process behind however the run ends: timeout, Ctrl-C, a signal to the
* supervisor, the parent dying, or the parent giving up on its output. */

import { type ChildProcess, spawn } from "node:child_process";
import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { afterEach, describe, expect, it } from "vitest";
import { startToolRecording, takeToolRuns } from "./exec.js";
import { MAX_TIMER_MS, runWithTreeKill, supervisorSpawn } from "./exec-tree.js";

const posix = process.platform !== "win32";

// A separate parent process running the exact supervisor runWithTreeKill uses,
// so the test can signal or kill the parent (or the supervisor) mid-run.
const PARENT = `
const { spawnSync } = require("node:child_process");
const { writeFileSync } = require("node:fs");
const spec = JSON.parse(process.argv[1]);
const res = spawnSync(spec.file, spec.args, { ...spec.options, cwd: spec.cwd });
writeFileSync(spec.cwd + "/result.json", JSON.stringify({ status: res.status, signal: res.signal, marker: res.output && res.output[3] }));
`;

const dirs: string[] = [];
const leftovers: number[] = [];
afterEach(() => {
for (const pid of leftovers.splice(0)) {
try {
process.kill(pid, "SIGKILL");
} catch {}
}
for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true });
});

function tempDir(): string {
const dir = mkdtempSync(join(tmpdir(), "vcqa-tree-"));
dirs.push(dir);
return dir;
}

function isAlive(pid: number): boolean {
try {
process.kill(pid, 0);
return true;
} catch (e) {
return (e as NodeJS.ErrnoException).code === "EPERM";
}
}

async function waitFor(cond: () => boolean, ms = 5000): Promise<boolean> {
const until = Date.now() + ms;
while (Date.now() < until) {
if (cond()) return true;
await new Promise((r) => setTimeout(r, 25));
}
return cond();
}

/** Start a parent in its own process group (like a CLI in a terminal) running
* `cmd` under the supervisor; resolves once `cmd` has written its pids. */
async function startParent(dir: string, cmd: string): Promise<{ parent: ChildProcess; pids: number[] }> {
const spec = { ...supervisorSpawn(cmd, 60_000), cwd: dir };
const parent = spawn(process.execPath, ["-e", PARENT, JSON.stringify(spec)], { detached: true, stdio: "ignore" });
if (parent.pid) leftovers.push(parent.pid);
const pidFile = join(dir, "pids");
expect(await waitFor(() => existsSync(pidFile) && readFileSync(pidFile, "utf-8").trim().split(/\s+/).length >= 2)).toBe(true);
const pids = readFileSync(pidFile, "utf-8").trim().split(/\s+/).map(Number);
leftovers.push(...pids);
return { parent, pids };
}

// `$$` is the shell, which execs into `sleep`; `$PPID` is the supervisor.
const SLEEPER = "echo $$ $PPID > pids; exec sleep 300";

describe.skipIf(!posix)("runWithTreeKill", () => {
it("returns the output and exit status of a normal run", () => {
const dir = tempDir();
expect(runWithTreeKill("echo hello", dir, 10_000)).toEqual({ stdout: "hello\n", ok: true, timedOut: false });
startToolRecording();
const failed = runWithTreeKill("echo oops >&2; exit 3", dir, 10_000);
const [r] = takeToolRuns();
expect(failed).toMatchObject({ ok: false, timedOut: false, stdout: "oops" });
expect(r).toMatchObject({ status: "failed", exitCode: 3, ok: false });
});

it("on timeout kills the whole group and records a timeout", () => {
const dir = tempDir();
startToolRecording();
const res = runWithTreeKill("sleep 60 & echo $! > pids; exec sleep 60", dir, 300);
const [r] = takeToolRuns();
expect(res).toMatchObject({ ok: false, timedOut: true });
expect(r).toMatchObject({ status: "timeout", timedOut: true, timeoutMs: 300, exitCode: null });
expect(isAlive(Number(readFileSync(join(dir, "pids"), "utf-8")))).toBe(false);
});

it("is not held open by a process outside the group that keeps stdout", () => {
const dir = tempDir();
const helper =
"node -e \"const c=require('child_process').spawn('sleep',['45'],{detached:true,stdio:['ignore','inherit','inherit']});require('fs').writeFileSync('helper',String(c.pid));c.unref()\"";
const started = Date.now();
const res = runWithTreeKill(`${helper}; echo report`, dir, 10_000);
const elapsed = Date.now() - started;
leftovers.push(Number(readFileSync(join(dir, "helper"), "utf-8")));
expect(res).toEqual({ stdout: "report\n", ok: true, timedOut: false });
expect(elapsed).toBeLessThan(5000);
});

it("clamps a limit too large for a timer instead of timing out at once", () => {
const dir = tempDir();
expect(supervisorSpawn("true", 3e9).limit).toBe(MAX_TIMER_MS);
expect(runWithTreeKill("sleep 0.2; echo done", dir, 3e9)).toEqual({ stdout: "done\n", ok: true, timedOut: false });
});

it("kills the group when the parent gives up on its output (maxBuffer)", () => {
const dir = tempDir();
const res = runWithTreeKill("echo $$ > pids; exec yes", dir, 60_000);
expect(res).toMatchObject({ ok: false, timedOut: false });
expect(isAlive(Number(readFileSync(join(dir, "pids"), "utf-8")))).toBe(false);
}, 30_000);

it("Ctrl-C (SIGINT to the parent's process group) leaves nothing running", async () => {
const dir = tempDir();
const { parent, pids } = await startParent(dir, SLEEPER);
process.kill(-(parent.pid as number), "SIGINT");
expect(await waitFor(() => !isAlive(pids[0]))).toBe(true);
expect(await waitFor(() => !isAlive(pids[1]))).toBe(true);
});

it("a SIGTERM to the supervisor kills the group and is reported as a failure", async () => {
const dir = tempDir();
const { pids } = await startParent(dir, SLEEPER);
process.kill(pids[1], "SIGTERM");
expect(await waitFor(() => existsSync(join(dir, "result.json")))).toBe(true);
expect(isAlive(pids[0])).toBe(false);
const result = JSON.parse(readFileSync(join(dir, "result.json"), "utf-8"));
expect(result.status).toBe(143);
expect(result.marker).not.toContain("vcqa:timed-out");
});

it("Ctrl-\\ (SIGQUIT to the supervisor) kills the group too", async () => {
const dir = tempDir();
const { pids } = await startParent(dir, SLEEPER);
process.kill(pids[1], "SIGQUIT");
expect(await waitFor(() => existsSync(join(dir, "result.json")))).toBe(true);
expect(isAlive(pids[0])).toBe(false);
expect(JSON.parse(readFileSync(join(dir, "result.json"), "utf-8")).status).toBe(131);
});

it("records a command killed by a signal as 128 + its signal number", () => {
const dir = tempDir();
startToolRecording();
runWithTreeKill("kill -9 $$", dir, 10_000);
runWithTreeKill("kill -TERM $$", dir, 10_000);
const [killed, terminated] = takeToolRuns();
expect(killed).toMatchObject({ status: "failed", exitCode: 137, ok: false });
expect(terminated).toMatchObject({ status: "failed", exitCode: 143, ok: false });
});

it("kills the group when the parent dies outright", async () => {
const dir = tempDir();
const { parent, pids } = await startParent(dir, SLEEPER);
process.kill(parent.pid as number, "SIGKILL");
expect(await waitFor(() => !isAlive(pids[0]))).toBe(true);
expect(await waitFor(() => !isAlive(pids[1]))).toBe(true);
});
});

describe.skipIf(!posix)("runWithTreeKill records through the shared recorder", () => {
// A token-shaped value, assembled at runtime so no literal sits in the source.
const alnum = "ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz23456789";
const body = Array.from({ length: 36 }, (_, i) => alnum[(i * 7 + 3) % alnum.length]).join("");
const value = ["gh", "p_", body].join("");
const windows = Array.from({ length: body.length - 4 }, (_, i) => body.slice(i, i + 5));
const expectAbsent = (text: string) => {
for (const w of windows) expect(text).not.toContain(w);
};

it("processes the output of a successful run", () => {
const dir = tempDir();
writeFileSync(join(dir, "v.txt"), `before ${value} after\n`);
startToolRecording();
const res = runWithTreeKill("cat v.txt", dir, 10_000);
const [r] = takeToolRuns();
expect(res.ok).toBe(true);
expect(r).toMatchObject({ status: "success", ok: true });
expect(r.output).toContain("before");
expectAbsent(r.output);
expectAbsent(r.command);
});

it("processes the output of a timed-out run", () => {
const dir = tempDir();
writeFileSync(join(dir, "v.txt"), `${value}\n`);
startToolRecording();
const res = runWithTreeKill("cat v.txt; exec sleep 60", dir, 500);
const [r] = takeToolRuns();
expect(res.timedOut).toBe(true);
expect(r).toMatchObject({ status: "timeout", timedOut: true });
expectAbsent(r.output);
});

it("processes a value that straddles the output cap", () => {
const dir = tempDir();
// The value starts 10 characters before the 8000-character cap.
writeFileSync(join(dir, "v.txt"), `${"x".repeat(7990)}${value}${"y".repeat(100)}\n`);
startToolRecording();
runWithTreeKill("cat v.txt", dir, 10_000);
const [r] = takeToolRuns();
expect(r.output.length).toBeLessThanOrEqual(8000);
expect(r.output.startsWith("xxxx")).toBe(true);
expectAbsent(r.output);
});
});
Loading