diff --git a/CLAUDE.md b/CLAUDE.md index 1193533..9cde598 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -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 diff --git a/README.md b/README.md index 78e03d6..dcc6a75 100644 --- a/README.md +++ b/README.md @@ -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 | diff --git a/src/cli.test.ts b/src/cli.test.ts index 66df006..e4c695b 100644 --- a/src/cli.test.ts +++ b/src/cli.test.ts @@ -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); diff --git a/src/cli.ts b/src/cli.ts index 9fc2c08..d2e6c71 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -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"; @@ -34,6 +35,7 @@ interface ParsedFlags { jsonOnly: boolean; ciMode: boolean; skipTests: boolean; + testTimeoutMs: number | null; watchMode: boolean; badgeMode: boolean; sarifMode: boolean; @@ -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"); @@ -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"), @@ -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) @@ -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, diff --git a/src/config.ts b/src/config.ts index 033359d..dc5c2f3 100644 --- a/src/config.ts +++ b/src/config.ts @@ -5,7 +5,7 @@ import { join } from "node:path"; export interface VcqaConfig { /** Disable or configure individual checks */ - checks?: Record; + checks?: Record }>; /** Extra glob patterns to ignore, merged with the versioned default exclusion policy. */ ignore?: string[]; /** Default fail-under threshold (overridden by --fail-under flag) */ @@ -49,6 +49,13 @@ export function getCheckIgnore(config: VcqaConfig, checkName: string): string[] return config.checks?.[checkName]?.ignore; } +/** Get per-check settings (`checks..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 { + const settings = config.checks?.[checkName]?.settings; + return settings && typeof settings === "object" && !Array.isArray(settings) ? settings : {}; +} + function validate(raw: Record): VcqaConfig { const config: VcqaConfig = {}; diff --git a/src/core.ts b/src/core.ts index 4cb5146..cf175fd 100644 --- a/src/core.ts +++ b/src/core.ts @@ -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"; @@ -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; @@ -168,7 +170,14 @@ export async function scan(cwd: string, options: ScanOptions = {}): Promise 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) }, diff --git a/src/monitor.tsx b/src/monitor.tsx index 85fc622..4cd4f45 100644 --- a/src/monitor.tsx +++ b/src/monitor.tsx @@ -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; @@ -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, @@ -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) => { diff --git a/src/runners/exec-tree.test.ts b/src/runners/exec-tree.test.ts new file mode 100644 index 0000000..26fcf16 --- /dev/null +++ b/src/runners/exec-tree.test.ts @@ -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 { + 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); + }); +}); diff --git a/src/runners/exec-tree.ts b/src/runners/exec-tree.ts new file mode 100644 index 0000000..67674fe --- /dev/null +++ b/src/runners/exec-tree.ts @@ -0,0 +1,191 @@ +/** Run a command with a time limit that kills its whole process tree. + * + * `execSync`'s timeout signals only the `/bin/sh` it spawned. Everything that + * shell started — `npx` → `vitest` → worker pools — is reparented and keeps + * running after the scan has given up on it (vibecodeqa/cli#106). + * + * The runners are synchronous and `spawnSync` cannot start a detached child, + * so the command runs under a tiny Node supervisor. It starts the command in + * its own process group (`detached`) and tears the group down — SIGTERM, then + * SIGKILL after a grace period, waiting until no member is left — whenever the + * run ends for any reason: + * + * - the time limit passes; + * - the supervisor is signalled (SIGINT/SIGTERM/SIGHUP). Ctrl-C at a terminal + * reaches the supervisor, which sits in the CLI's process group, but not the + * detached command, so the supervisor must pass it on; + * - the parent goes away: the supervisor holds one end of a pipe (fd 4) whose + * other end only the parent holds, and sees EOF on it when the parent dies; + * - the parent gives up on it (output past `maxBuffer`, or the backstop + * timeout): `spawnSync` then sends SIGTERM, which is handled as above. + * + * The command's output is relayed through the supervisor rather than inherited, + * so a process outside the group that keeps the command's stdout open (a + * `detached` helper) cannot hold the run open: the supervisor stops relaying + * shortly after the command exits, and its exit closes the parent's pipes. + * + * The supervisor reports on fd 3 — a timeout marker, or the command's exit + * code — so the outcome cannot be confused with anything the command prints. + */ + +import { spawnSync } from "node:child_process"; +import { recordToolRun, type ToolRunContext } from "./exec.js"; + +const MAX_BUFFER = 64 * 1024 * 1024; +const TIMEOUT_MARKER = "vcqa:timed-out"; +const EXIT_MARKER = "vcqa:exit:"; +/** Largest delay `setTimeout` honours; anything above fires after 1 ms. */ +export const MAX_TIMER_MS = 2 ** 31 - 1; + +// Runs as `node -e`. argv: [cmd, timeoutMs]. Plain JS — it is not compiled. +const SUPERVISOR = ` +const { spawn, execFileSync } = require("node:child_process"); +const { writeSync } = require("node:fs"); +const [cmd, timeoutArg] = process.argv.slice(1); +const timeoutMs = Number(timeoutArg); +const win = process.platform === "win32"; +const child = spawn(cmd, { shell: true, detached: !win, stdio: ["ignore", "pipe", "pipe"] }); +let ending = false; +function report(text) { try { writeSync(3, text); } catch {} } +function writeAll(fd, buf) { + let off = 0; + while (off < buf.length) { + try { off += writeSync(fd, buf, off); } + catch (e) { if (e.code !== "EAGAIN") throw e; } + } +} +function signalTree(sig) { + try { + if (win) execFileSync("taskkill", ["/pid", String(child.pid), "/T", "/F"], { stdio: "ignore" }); + else process.kill(-child.pid, sig); + } catch {} +} +function groupAlive() { + if (win) return false; + try { process.kill(-child.pid, 0); return true; } catch (e) { return e.code === "EPERM"; } +} +// Tear the group down and exit. The single exit path for every abnormal end. +function stop(code, timedOut) { + if (ending) return; + ending = true; + clearTimeout(timer); + signalTree("SIGTERM"); + const started = Date.now(); + let killed = false; + const finish = () => { if (timedOut) report("${TIMEOUT_MARKER}"); process.exit(code); }; + if (!child.pid) finish(); + const poll = setInterval(() => { + if (!groupAlive()) { clearInterval(poll); finish(); return; } + const waited = Date.now() - started; + if (!killed && waited >= 2000) { killed = true; signalTree("SIGKILL"); } + if (waited >= 10000) { clearInterval(poll); finish(); } + }, 20); +} +const relay = (fd) => (chunk) => { + try { writeAll(fd, chunk); } catch { stop(129, false); } // parent stopped reading +}; +child.stdout.on("data", relay(1)); +child.stderr.on("data", relay(2)); +const timer = setTimeout(() => stop(124, true), timeoutMs); +for (const [sig, code] of [["SIGINT", 130], ["SIGTERM", 143], ["SIGHUP", 129], ["SIGQUIT", 131]]) process.on(sig, () => stop(code, false)); +if (!win) { + try { + const watch = new (require("node:net").Socket)({ fd: 4, readable: true, writable: false }); + const gone = () => stop(129, false); + watch.on("end", gone); watch.on("close", gone); watch.on("error", gone); + } catch {} +} +child.on("error", (e) => { + if (ending) return; + ending = true; + clearTimeout(timer); + try { writeAll(2, Buffer.from(String(e && e.message))); } catch {} + report("${EXIT_MARKER}127"); + process.exit(127); +}); +child.on("exit", (code, signal) => { + if (ending) return; + ending = true; + clearTimeout(timer); + // Grandchildren that outlive the command (a stray watcher) go too. + signalTree("SIGKILL"); + const status = code ?? (signal ? 128 + (require("node:os").constants.signals[signal] ?? 0) : 1); + // Relay what is still buffered, but do not wait on a process outside the + // group that holds the pipes open: stop once they close, go quiet, or 2 s pass. + const done = () => { report("${EXIT_MARKER}" + status); process.exit(status); }; + const streams = [child.stdout, child.stderr].filter((s) => !s.closed); + if (streams.length === 0) done(); + let open = streams.length; + for (const s of streams) s.on("close", () => { if (--open === 0) done(); }); + let quietTimer = setTimeout(done, 200); + const busy = () => { clearTimeout(quietTimer); quietTimer = setTimeout(done, 200); }; + child.stdout.on("data", busy); + child.stderr.on("data", busy); + setTimeout(done, 2000); +}); +`; + +/** How the supervisor is started: `spawnSync(file, args, options)`. Exposed so + * tests can run the exact same supervisor from a separate parent process. */ +export function supervisorSpawn(cmd: string, timeoutMs: number) { + const limit = Math.min(Math.max(1, Math.floor(timeoutMs)), MAX_TIMER_MS); + return { + limit, + file: process.execPath, + args: ["-e", SUPERVISOR, cmd, String(limit)], + options: { + encoding: "utf-8" as const, + maxBuffer: MAX_BUFFER, + // fd 3: the supervisor's report. fd 4: held open by this process only, + // so the supervisor sees EOF on it if this process dies. + stdio: ["ignore", "pipe", "pipe", "pipe", "pipe"] as Array<"ignore" | "pipe">, + // Backstop only: the supervisor normally exits well before this. SIGTERM, + // not SIGKILL, so the supervisor still tears the group down. + timeout: limit + 30_000, + killSignal: "SIGTERM" as const, + }, + }; +} + +export interface TreeRunResult { + stdout: string; + ok: boolean; + timedOut: boolean; +} + +/** Like `run()` from exec.ts, but a timeout kills every process the command + * started, and is recorded as `status: "timeout"`. A `timeoutMs` above + * {@link MAX_TIMER_MS} is clamped to it. */ +export function runWithTreeKill(cmd: string, cwd: string, timeoutMs: number, context: ToolRunContext = {}): TreeRunResult { + const started = Date.now(); + const { limit, file, args, options } = supervisorSpawn(cmd, timeoutMs); + const res = spawnSync(file, args, { ...options, cwd }); + const stdout = typeof res.stdout === "string" ? res.stdout : ""; + const stderr = typeof res.stderr === "string" ? res.stderr : ""; + const marker = typeof res.output?.[3] === "string" ? res.output[3] : ""; + const exited = new RegExp(`${EXIT_MARKER}(\\d+)`).exec(marker); + const errorCode = (res.error as NodeJS.ErrnoException | undefined)?.code; + // The supervisor's own report wins; the backstop's ETIMEDOUT counts only + // when the supervisor never said how the command ended. + const timedOut = marker.includes(TIMEOUT_MARKER) || (!exited && errorCode === "ETIMEDOUT"); + const status = exited ? Number(exited[1]) : res.status; + const failedToRun = Boolean(res.error) && !(exited && errorCode === "ETIMEDOUT"); + const ok = !timedOut && !failedToRun && status === 0; + const combined = `${stdout}${stdout && stderr ? "\n" : ""}${stderr}`.trim() || (res.error ? String(res.error) : ""); + const output = ok ? stdout : combined; + + recordToolRun( + cmd, + cwd, + { + status: ok ? "success" : timedOut ? "timeout" : "failed", + exitCode: ok ? 0 : timedOut ? null : (status ?? null), + ok, + durationMs: Date.now() - started, + output, + ...(timedOut ? { timedOut: true, timeoutMs: limit } : {}), + }, + context, + ); + return { stdout: output, ok, timedOut }; +} diff --git a/src/runners/exec.test.ts b/src/runners/exec.test.ts index 4e0bdca..27d7ae1 100644 --- a/src/runners/exec.test.ts +++ b/src/runners/exec.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { filterToolRuns, run, startToolRecording, type ToolRun, takeToolRuns } from "./exec.js"; +import { filterToolRuns, recordToolRun, run, startToolRecording, type ToolRun, takeToolRuns } from "./exec.js"; /** Provenance is the mechanism that makes a report auditable — without it a * clean score cannot be distinguished from a tool that never ran, or one that @@ -71,6 +71,23 @@ describe("tool run provenance", () => { expect(r.exitCode).toBe(2); }); + it("records a timeout as a timeout, not as a failure (#106)", () => { + startToolRecording(); + const result = run("sleep 5", "/tmp", 200); + const [r] = takeToolRuns(); + expect(result.ok).toBe(false); + expect(r).toMatchObject({ status: "timeout", timedOut: true, timeoutMs: 200, ok: false, notFound: false }); + }); + + it("does not mark an ordinary failure as timed out", () => { + startToolRecording(); + run("sh -c 'exit 1'", "/tmp"); + const [r] = takeToolRuns(); + expect(r.status).toBe("failed"); + expect(r.timedOut).toBeUndefined(); + expect(r.timeoutMs).toBeUndefined(); + }); + it("names the package rather than npx for delegated tools", () => { startToolRecording(); run("npx --yes knip --reporter json", "/tmp", 1); @@ -101,6 +118,38 @@ describe("tool run provenance", () => { expect(r.output.length).toBeLessThanOrEqual(8000); }); + it("records an externally executed run exactly as run() would (#106)", () => { + const noisy = "node -e \"process.stdout.write('x'.repeat(20000))\""; + startToolRecording({ analyzerId: "testing" }); + run(noisy, "/tmp", 60_000, { projectId: "app" }); + const viaRun = takeToolRuns()[0]; + startToolRecording({ analyzerId: "testing" }); + recordToolRun( + noisy, + "/tmp", + { status: "success", exitCode: 0, ok: true, durationMs: viaRun.durationMs, output: `${"x".repeat(20000)}\n` }, + { projectId: "app" }, + ); + const viaRecord = takeToolRuns()[0]; + expect(viaRecord).toEqual(viaRun); + expect(viaRecord.output.length).toBeLessThan(20000); + }); + + it("does not call a timed-out run whose output mentions ENOENT a missing binary", () => { + startToolRecording(); + recordToolRun("npx vitest run", "/tmp", { + status: "timeout", + exitCode: null, + ok: false, + durationMs: 5, + output: "ENOENT: no such file", + timedOut: true, + timeoutMs: 5, + }); + const [r] = takeToolRuns(); + expect(r).toMatchObject({ tool: "vitest", status: "timeout", notFound: false }); + }); + it("filters tool runs by analyzer and project for page-specific logs", () => { const runs: ToolRun[] = [ { diff --git a/src/runners/exec.ts b/src/runners/exec.ts index 86bc2a4..43772ec 100644 --- a/src/runners/exec.ts +++ b/src/runners/exec.ts @@ -23,8 +23,12 @@ export interface ToolRun { analyzer?: string; projectId?: string; projectPath?: string; - status: "success" | "failed"; + status: "success" | "failed" | "timeout"; exitCode: number | null; + /** True when the run was killed for exceeding its time limit. */ + timedOut?: boolean; + /** The time limit that was exceeded; set only when `timedOut`. */ + timeoutMs?: number; ok: boolean; durationMs: number; /** Combined output, credential values redacted, trimmed and capped so reports stay a sane size. */ @@ -115,7 +119,6 @@ export function filterToolRuns(runs: ToolRun[], filter: ToolRunFilter = {}): Too export function run(cmd: string, cwd: string, timeout = 60_000, context: ToolRunContext = {}): { stdout: string; ok: boolean } { const started = Date.now(); - const runContext = normalizedContext({ ...defaultContext, ...context }); try { const stdout = execSync(cmd, { cwd, @@ -124,37 +127,52 @@ export function run(cmd: string, cwd: string, timeout = 60_000, context: ToolRun maxBuffer: MAX_BUFFER, stdio: ["pipe", "pipe", "pipe"], }); - record({ - tool: toolNameOf(cmd), - command: cmd, - cwd, - ...runContext, - status: "success", - exitCode: 0, - ok: true, - durationMs: Date.now() - started, - output: stdout, - notFound: false, - }); + recordToolRun(cmd, cwd, { status: "success", exitCode: 0, ok: true, durationMs: Date.now() - started, output: stdout }, context); return { stdout, ok: true }; } catch (e: any) { const output = outputOf(e); - record({ - tool: toolNameOf(cmd), - command: cmd, + const timedOut = e?.code === "ETIMEDOUT"; + recordToolRun( + cmd, cwd, - ...runContext, - status: "failed", - exitCode: typeof e?.status === "number" ? e.status : null, - ok: false, - durationMs: Date.now() - started, - output, - notFound: /not found|ENOENT|command not found/i.test(output), - }); + { + status: timedOut ? "timeout" : "failed", + ...(timedOut ? { timedOut: true, timeoutMs: timeout } : {}), + exitCode: typeof e?.status === "number" ? e.status : null, + ok: false, + durationMs: Date.now() - started, + output, + }, + context, + ); return { stdout: output, ok: false }; } } +type ToolRunResult = Pick; + +/** The one place a ToolRun entry is built: command, cwd, context and the + * not-found guess. Command and output are passed through whole — `record()` + * processes and caps them, so every recorded run is stored the same way. */ +function toolRunEntry(cmd: string, cwd: string, result: ToolRunResult, context: ToolRunContext): ToolRun { + return { + tool: toolNameOf(cmd), + command: cmd, + cwd, + ...normalizedContext({ ...defaultContext, ...context }), + ...result, + output: result.output, + notFound: !result.ok && !result.timedOut && /not found|ENOENT|command not found/i.test(result.output), + }; +} + +/** Record a finished run in the provenance log. `run()` records through this, + * and so does a runner that executes its command another way (e.g. one that + * must kill a whole process tree). */ +export function recordToolRun(cmd: string, cwd: string, result: ToolRunResult, context: ToolRunContext = {}): void { + record(toolRunEntry(cmd, cwd, result, context)); +} + export function runJSON(cmd: string, cwd: string, timeout = 60_000, context: ToolRunContext = {}): T | null { const { stdout } = run(cmd, cwd, timeout, context); try { diff --git a/src/runners/test-timeout.test.ts b/src/runners/test-timeout.test.ts new file mode 100644 index 0000000..7fee031 --- /dev/null +++ b/src/runners/test-timeout.test.ts @@ -0,0 +1,241 @@ +/** vibecodeqa/cli#106 — a test run that hits its time limit is "not run", not + * "failed", and leaves nothing behind. + * + * The fixture's `vitest` is a local fake (node_modules/vitest), so `npx vitest` + * resolves it without a download. It starts a grandchild (`sleep`), records + * both pids, then waits the number of ms in `fake-sleep-ms` before printing a + * passing vitest JSON report and a fresh coverage summary. A stale coverage + * summary is on disk before the run. + */ + +import { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { scan } from "../core.js"; +import { detectStack } from "../detect.js"; +import type { ToolRun } from "./exec.js"; +import { resolveTestTimeout, runTesting, scanTimeoutWithTests, testExecutionPoints } from "./testing.js"; + +const FAKE_VITEST = `#!/usr/bin/env node +const { spawn } = require("node:child_process"); +const { mkdirSync, readFileSync, writeFileSync } = require("node:fs"); +const mode = readFileSync("fake-sleep-ms", "utf-8").trim(); +if (mode === "fail") { console.error("boom"); process.exit(1); } +const sleepMs = Number(mode); +const grandchild = spawn("sleep", ["60"], { stdio: "ignore" }); +writeFileSync("pids.json", JSON.stringify({ fake: process.pid, grandchild: grandchild.pid })); +setTimeout(() => { + grandchild.kill("SIGKILL"); + mkdirSync("coverage", { recursive: true }); + const pct = { pct: 90 }; + writeFileSync("coverage/coverage-summary.json", JSON.stringify({ total: { statements: pct, lines: pct, branches: pct, functions: pct } })); + console.log(JSON.stringify({ numTotalTests: 1, numPassedTests: 1, numFailedTests: 0, testResults: [] })); +}, sleepMs); +`; + +const STALE_PCT = { pct: 42 }; +const STALE_COVERAGE = { total: { statements: STALE_PCT, lines: STALE_PCT, branches: STALE_PCT, functions: STALE_PCT } }; + +const dirs: string[] = []; +afterEach(() => { + for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true }); +}); + +function makeFixture(sleepMs: number | "fail", settings?: Record, into?: string): string { + const dir = into ?? mkdtempSync(join(tmpdir(), "vcqa-timeout-")); + if (!into) dirs.push(dir); + mkdirSync(dir, { recursive: true }); + writeFileSync(join(dir, "package.json"), JSON.stringify({ name: "timeout-fixture", devDependencies: { vitest: "1.0.0" } })); + if (settings) writeFileSync(join(dir, ".vcqa.json"), JSON.stringify({ checks: { testing: { settings } } })); + mkdirSync(join(dir, "src")); + writeFileSync(join(dir, "src/app.ts"), "export const x = 1;\n"); + writeFileSync( + join(dir, "src/app.test.ts"), + "import { expect, it } from 'vitest';\nimport { x } from './app';\nit('works as expected', () => { expect(x).toBe(1); });\n", + ); + mkdirSync(join(dir, "coverage")); + writeFileSync(join(dir, "coverage/coverage-summary.json"), JSON.stringify(STALE_COVERAGE)); + writeFileSync(join(dir, "fake-sleep-ms"), String(sleepMs)); + mkdirSync(join(dir, "node_modules/vitest"), { recursive: true }); + mkdirSync(join(dir, "node_modules/.bin"), { recursive: true }); + writeFileSync( + join(dir, "node_modules/vitest/package.json"), + JSON.stringify({ name: "vitest", version: "1.0.0", bin: { vitest: "cli.js" } }), + ); + writeFileSync(join(dir, "node_modules/vitest/cli.js"), FAKE_VITEST); + chmodSync(join(dir, "node_modules/vitest/cli.js"), 0o755); + symlinkSync("../vitest/cli.js", join(dir, "node_modules/.bin/vitest")); + 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 scanTesting(dir: string, testTimeoutMs?: number) { + const report = await scan(dir, { checks: ["testing"], testTimeoutMs }); + const check = report.checks.find((c) => c.name === "testing"); + if (!check) throw new Error("testing check missing"); + const details = check.details as Record; + return { check, details, toolRuns: (details.toolRuns ?? []) as ToolRun[] }; +} + +describe("test run timeout (#106)", () => { + it.skipIf(process.platform === "win32")( + "a run past the configured limit is a warning, scores like --skip-tests, reads no coverage and leaves no process", + async () => { + // 5 s, not less: the fake must start and write pids.json before the limit, + // even on a loaded CI runner. + const dir = makeFixture(30_000, { timeoutMs: 5000 }); + const { check, details, toolRuns } = await scanTesting(dir); + + const run = toolRuns.find((r) => r.tool === "vitest"); + expect(run).toMatchObject({ status: "timeout", timedOut: true, timeoutMs: 5000, ok: false }); + expect(run?.durationMs).toBeLessThan(15_000); + + expect(details.testTimeoutMs).toBe(5000); + expect(details.testTimeoutSource).toBe("config"); + expect(details.executionStatus).toBe("timeout"); + expect(details.testProjects).toHaveLength(1); + expect(details.testProjects[0]).toMatchObject({ + status: "timeout", + statusLabel: "timed out after 5 s", + timeoutMs: 5000, + coverageStatus: "not-reported", + coverage: null, + }); + // The stale 42% summary on disk was not read. + expect(details.coverage).toBeUndefined(); + + const timeoutIssues = check.issues.filter((i) => i.rule === "test-run-timeout"); + expect(timeoutIssues).toHaveLength(1); + expect(timeoutIssues[0].severity).toBe("warning"); + expect(check.issues.some((i) => i.rule === "test-run-command-failed")).toBe(false); + + // Execution scored as for --skip-tests: same fixture, same static + // points, and --skip-tests gets 10/20 for execution plus whatever the + // stale coverage file is worth (which the timed-out scan must not get). + const skipped = await scan(dir, { checks: ["testing"], skipTests: true }); + const skippedCheck = skipped.checks.find((c) => c.name === "testing"); + const avgStale = 42; + const staleCoveragePoints = Math.round((avgStale / 100) * 20); + expect(check.score).toBe((skippedCheck?.score ?? 0) - staleCoveragePoints); + + // Nothing from the timed-out run survives scan(). + const pids = JSON.parse(readFileSync(join(dir, "pids.json"), "utf-8")) as { fake: number; grandchild: number }; + expect(isAlive(pids.fake)).toBe(false); + expect(isAlive(pids.grandchild)).toBe(false); + }, + 30_000, + ); + + it.skipIf(process.platform === "win32")( + "the same fixture passes with a larger limit from --test-timeout", + async () => { + const dir = makeFixture(2500, { timeoutMs: 2000 }); + const { check, details, toolRuns } = await scanTesting(dir, 15_000); + + const run = toolRuns.find((r) => r.tool === "vitest"); + expect(run).toMatchObject({ status: "success", ok: true }); + expect(run?.timedOut).toBeUndefined(); + expect(details.testTimeoutMs).toBe(15_000); + expect(details.testTimeoutSource).toBe("flag"); + expect(details.testProjects[0]).toMatchObject({ status: "passed", coverageStatus: "reported" }); + expect(details.testProjects[0].timeoutMs).toBeUndefined(); + expect(details.coverage).toMatchObject({ stmts: 90 }); + expect(check.issues.some((i) => i.rule === "test-run-timeout")).toBe(false); + expect(existsSync(join(dir, "pids.json"))).toBe(true); + }, + 30_000, + ); +}); + +describe("resolveTestTimeout", () => { + it("defaults to 120 s", () => { + expect(resolveTestTimeout()).toEqual({ ms: 120_000, source: "default" }); + }); + + it("prefers the flag over the setting", () => { + expect(resolveTestTimeout({ timeoutMs: 5000, settings: { timeoutMs: 9000 } })).toEqual({ ms: 5000, source: "flag" }); + expect(resolveTestTimeout({ settings: { timeoutMs: 9000 } })).toEqual({ ms: 9000, source: "config" }); + }); + + it("falls back to the default for an invalid setting and says so", () => { + expect(resolveTestTimeout({ settings: { timeoutMs: "fast" } })).toEqual({ ms: 120_000, source: "default", invalidSetting: "fast" }); + expect(resolveTestTimeout({ settings: { timeoutMs: -1 } })).toMatchObject({ ms: 120_000, source: "default" }); + expect(resolveTestTimeout({ settings: { timeoutMs: 1.5 } })).toMatchObject({ ms: 120_000, source: "default" }); + }); + + it("accepts up to 2^31-1 ms and rejects anything a timer would fire at once", () => { + expect(resolveTestTimeout({ settings: { timeoutMs: 2 ** 31 - 1 } })).toEqual({ ms: 2 ** 31 - 1, source: "config" }); + expect(resolveTestTimeout({ settings: { timeoutMs: 3e9 } })).toEqual({ ms: 120_000, source: "default", invalidSetting: 3e9 }); + expect(resolveTestTimeout({ timeoutMs: 2 ** 31, settings: { timeoutMs: 9000 } })).toEqual({ ms: 9000, source: "config" }); + }); +}); + +describe("timeout scoring across projects (#106)", () => { + const timedOut = { status: "timeout" as const }; + + it("gives 10/20 only when every project that ran timed out", () => { + expect(testExecutionPoints(null, false, [timedOut])).toBe(10); + expect(testExecutionPoints(null, false, [timedOut, timedOut, { status: "skipped" }])).toBe(10); + expect(testExecutionPoints(null, false, [timedOut, { status: "command-failed" }])).toBe(0); + expect(testExecutionPoints(null, false, [timedOut, { status: "parse-failed" }])).toBe(0); + expect(testExecutionPoints(null, false, [])).toBe(0); + expect(testExecutionPoints(null, true, [])).toBe(10); + }); + + it("scores the projects that reported by pass rate, timed-out ones aside", () => { + expect(testExecutionPoints({ passed: 1, failed: 1, total: 2 }, false, [timedOut, { status: "failed" }])).toBe(10); + expect(testExecutionPoints({ passed: 0, failed: 3, total: 3 }, false, [timedOut, { status: "failed" }])).toBe(0); + }); + + it.skipIf(process.platform === "win32")( + "a timed-out project does not hide another project's failed command", + () => { + const run = (second: number | "fail") => { + const dir = mkdtempSync(join(tmpdir(), "vcqa-timeout-mono-")); + dirs.push(dir); + writeFileSync(join(dir, "package.json"), JSON.stringify({ name: "mono", private: true })); + makeFixture(30_000, undefined, join(dir, "packages/slow")); + makeFixture(second, undefined, join(dir, "packages/other")); + // Keep coverage out of the comparison: a failed run's coverage file is + // still read, as before #106, and would differ between the two runs. + rmSync(join(dir, "packages/other/coverage"), { recursive: true }); + const srcRoots = ["packages/slow/src", "packages/other/src"]; + // One limit for both projects: generous enough that the failing one + // always finishes under load; the 30 s sleeper still times out. + return runTesting(dir, detectStack(dir), false, srcRoots, undefined, undefined, { timeoutMs: 5000 }); + }; + const bothTimedOut = run(30_000); + const oneFailed = run("fail"); + const statuses = (check: typeof oneFailed) => + (check.details as Record).testProjects.map((p: { status: string }) => p.status).sort(); + + expect(statuses(bothTimedOut)).toEqual(["timeout", "timeout"]); + expect(statuses(oneFailed)).toEqual(["command-failed", "timeout"]); + expect(oneFailed.issues.some((i) => i.rule === "test-run-command-failed" && i.severity === "error")).toBe(true); + // Same static points; only execution differs: 10 when all timed out, 0 here. + expect(oneFailed.score).toBe(bothTimedOut.score - 10); + }, + 60_000, + ); +}); + +describe("scanTimeoutWithTests (monitor scan limit)", () => { + it("leaves room for the full test limit of every project", () => { + expect(scanTimeoutWithTests(120_000, 1)).toBe(240_000); + expect(scanTimeoutWithTests(600_000, 3)).toBe(120_000 + 1_800_000); + expect(scanTimeoutWithTests(600_000, 0)).toBe(720_000); + }); + + it("never exceeds what a timer can hold", () => { + expect(scanTimeoutWithTests(2 ** 31 - 1, 2)).toBe(2 ** 31 - 1); + }); +}); diff --git a/src/runners/testing.ts b/src/runners/testing.ts index 93f030b..f9ef956 100644 --- a/src/runners/testing.ts +++ b/src/runners/testing.ts @@ -16,7 +16,7 @@ import { inventorySourceFiles } from "../file-inventory.js"; import { getProductionFiles, normalizeToolPath, readDeps } from "../fs-utils.js"; import type { AnalyzerMetric, CheckResult, Issue, StackInfo, WorkspaceInfo } from "../types.js"; import { gradeFromScore } from "../types.js"; -import { run } from "./exec.js"; +import { MAX_TIMER_MS, runWithTreeKill } from "./exec-tree.js"; import { redactSecrets } from "./redact.js"; // ── Types ── @@ -49,7 +49,65 @@ interface CoverageData { } type TestCaseStatus = "passed" | "failed" | "skipped" | "todo" | "unknown"; -type TestProjectRunStatus = "passed" | "failed" | "partial" | "no-tests-matched" | "parse-failed" | "command-failed" | "skipped"; +type TestProjectRunStatus = + | "passed" + | "failed" + | "partial" + | "no-tests-matched" + | "parse-failed" + | "command-failed" + | "timeout" + | "skipped"; + +/** Default per-project test run limit. Override with `--test-timeout ` or + * `.vcqa.json` `checks.testing.settings.timeoutMs`. */ +export const DEFAULT_TEST_TIMEOUT_MS = 120_000; + +export interface TestingOptions { + /** From `--test-timeout`; wins over the config setting. */ + timeoutMs?: number; + /** `checks.testing.settings` from `.vcqa.json` / package.json#vcqa. */ + settings?: Record; +} + +export interface EffectiveTestTimeout { + ms: number; + source: "flag" | "config" | "default"; + /** Set when `settings.timeoutMs` was present but not a positive integer. */ + invalidSetting?: unknown; +} + +/** Largest accepted test timeout: 2^31-1 ms (~24.8 days), the most a Node + * timer can wait. Larger values are rejected, not clamped. */ +export const MAX_TEST_TIMEOUT_MS = MAX_TIMER_MS; + +/** A test timeout is a whole number of ms from 1 to {@link MAX_TEST_TIMEOUT_MS}. */ +export function isValidTestTimeout(value: unknown): value is number { + return typeof value === "number" && Number.isInteger(value) && value > 0 && value <= MAX_TEST_TIMEOUT_MS; +} + +export function resolveTestTimeout(options: TestingOptions = {}): EffectiveTestTimeout { + const configured = options.settings?.timeoutMs; + const invalid = configured !== undefined && !isValidTestTimeout(configured) ? { invalidSetting: configured } : {}; + if (isValidTestTimeout(options.timeoutMs)) return { ms: options.timeoutMs, source: "flag", ...invalid }; + if (isValidTestTimeout(configured)) return { ms: configured, source: "config" }; + return { ms: DEFAULT_TEST_TIMEOUT_MS, source: "default", ...invalid }; +} + +/** Base allowance for a whole scan, test runs aside. */ +export const SCAN_BASE_TIMEOUT_MS = 120_000; + +/** Time limit for a whole scan that runs tests (e.g. the monitor's scan + * subprocess): the base allowance plus one full test limit per test project, + * capped at what a timer can hold. A tighter limit would kill the scan before + * a configured test timeout could take effect. */ +export function scanTimeoutWithTests(testTimeoutMs: number, testProjects: number): number { + return Math.min(SCAN_BASE_TIMEOUT_MS + testTimeoutMs * Math.max(1, testProjects), MAX_TIMER_MS); +} + +function formatSeconds(ms: number): string { + return `${Number((ms / 1000).toFixed(1))} s`; +} type TestCoverageStatus = "reported" | "not-reported"; interface TestCaseReport { @@ -94,6 +152,8 @@ interface NormalizedTestProjectReport extends TestExecutionReport { reportParsed: boolean; coverageStatus: TestCoverageStatus; coverage: CoverageData | null; + /** Set when the run was killed for exceeding this limit (ms). */ + timeoutMs?: number; } // ── Classification rules ── @@ -484,7 +544,7 @@ function commandForTestTarget(target: TestRunTarget): string { : "npx jest --json --coverage --coverageReporters=json-summary"; } -function statusLabel(status: TestProjectRunStatus): string { +function statusLabel(status: TestProjectRunStatus, timeoutMs?: number): string { switch (status) { case "passed": return "passed"; @@ -498,6 +558,8 @@ function statusLabel(status: TestProjectRunStatus): string { return "report could not be parsed"; case "command-failed": return "command failed"; + case "timeout": + return timeoutMs ? `timed out after ${formatSeconds(timeoutMs)}` : "timed out"; case "skipped": return "skipped"; } @@ -518,8 +580,14 @@ export function normalizeTestProjectReport(input: { executionOk: boolean | null; coverage: CoverageData | null; skipped?: boolean; + /** The limit the run exceeded; marks the project as timed out. */ + timedOutAfterMs?: number; }): NormalizedTestProjectReport { - const status = input.skipped ? "skipped" : reportStatus(input.report, input.executionOk); + const status: TestProjectRunStatus = input.skipped + ? "skipped" + : input.timedOutAfterMs !== undefined + ? "timeout" + : reportStatus(input.report, input.executionOk); const base = input.report ?? { runner: input.target.runner, cwd: input.target.cwd, @@ -543,11 +611,12 @@ export function normalizeTestProjectReport(input: { path: input.target.projectPath ?? ".", command: input.command, status, - statusLabel: statusLabel(status), + statusLabel: statusLabel(status, input.timedOutAfterMs), executionOk: input.executionOk, reportParsed: Boolean(input.report), coverageStatus: input.coverage ? "reported" : "not-reported", coverage: input.coverage, + ...(status === "timeout" ? { timeoutMs: input.timedOutAfterMs } : {}), }; } @@ -558,6 +627,7 @@ export function summarizeTestProjectStatus(projects: Pick p.status === "passed" || p.status === "partial")) { return projects.every((p) => p.status === "passed") ? "passed" : "partial"; } + if (projects.some((p) => p.status === "timeout")) return "timeout"; if (projects.every((p) => p.status === "no-tests-matched")) return "no-tests-matched"; return "parse-failed"; } @@ -565,8 +635,9 @@ export function summarizeTestProjectStatus(projects: Pick 0) { - issues.push({ - severity: "warning", - message: `${report.path} has ${staticTestCount} discovered test file${staticTestCount === 1 ? "" : "s"}, but ${report.runner} matched 0 tests`, - rule: "test-run-no-tests-matched", - }); - } - if (report.status === "parse-failed" || report.status === "command-failed") { - issues.push({ - severity: report.status === "command-failed" ? "error" : "warning", - message: `${report.path} ${report.runner} ${report.statusLabel}`, - rule: `test-run-${report.status}`, - }); - } - } + issues.push(...projectRunIssues(testReports, testFiles)); if (coverage) { if (coverage.branches < 50) { @@ -946,14 +1029,7 @@ export function runTesting( score += pyramidScore; // Execution (20 points) - if (execution) { - const passRate = execution.total > 0 ? execution.passed / execution.total : 0; - score += Math.round(passRate * 20); - } else if (!skipExec) { - // Couldn't run tests - } else { - score += 10; // partial credit when skipped - } + score += testExecutionPoints(execution, skipExec, testReports); // Coverage (20 points) if (coverage) { @@ -1003,6 +1079,13 @@ export function runTesting( } : {}), executionStatus: summarizeTestProjectStatus(testReports), + ...(skipExec + ? {} + : { + testTimeoutMs: testTimeout.ms, + testTimeoutSource: testTimeout.source, + ...("invalidSetting" in testTimeout ? { testTimeoutInvalidSetting: testTimeout.invalidSetting } : {}), + }), ...(execution ? { passed: execution.passed, failed: execution.failed, total: execution.total } : {}), ...(testReports.length > 0 ? { testReports } : {}), ...(testReports.length > 0 @@ -1014,6 +1097,7 @@ export function runTesting( command: report.command, status: report.status, statusLabel: report.statusLabel, + ...(report.timeoutMs !== undefined ? { timeoutMs: report.timeoutMs } : {}), reportParsed: report.reportParsed, executionOk: report.executionOk, passed: report.passed, @@ -1140,3 +1224,52 @@ function staticTestFilesForProject(projectPath: string, testFiles: TestFile[]): const prefix = `${projectPath.replace(/^\/+|\/+$/g, "")}/`; return testFiles.filter((file) => file.path.startsWith(prefix)).length; } + +function projectRunIssues(testReports: NormalizedTestProjectReport[], testFiles: TestFile[]): Issue[] { + const issues: Issue[] = []; + for (const report of testReports) { + const staticTestCount = staticTestFilesForProject(report.path, testFiles); + if (report.status === "no-tests-matched" && staticTestCount > 0) { + issues.push({ + severity: "warning", + message: `${report.path} has ${staticTestCount} discovered test file${staticTestCount === 1 ? "" : "s"}, but ${report.runner} matched 0 tests`, + rule: "test-run-no-tests-matched", + }); + } + if (report.status === "timeout") { + issues.push({ + severity: "warning", + message: `${report.path} ${report.runner} ${report.statusLabel} — tests not scored; raise --test-timeout or checks.testing.settings.timeoutMs`, + rule: "test-run-timeout", + }); + } + if (report.status === "parse-failed" || report.status === "command-failed") { + issues.push({ + severity: report.status === "command-failed" ? "error" : "warning", + message: `${report.path} ${report.runner} ${report.statusLabel}`, + rule: `test-run-${report.status}`, + }); + } + } + return issues; +} + +/** Execution points (of 20). Pass rate over the projects that reported; 10 + * when tests were not run at all — `--skip-tests`, or EVERY project that ran + * stopped at the time limit (a timeout says how fast the machine is, not + * whether the tests pass); otherwise 0. A timeout never lifts a project whose + * command failed: one failed project and one timed-out project score 0. */ +export function testExecutionPoints( + execution: { passed: number; failed: number; total: number } | null, + skipExec: boolean, + testReports: Pick[], +): number { + if (execution) { + const passRate = execution.total > 0 ? execution.passed / execution.total : 0; + return Math.round(passRate * 20); + } + if (skipExec) return 10; + const ran = testReports.filter((report) => report.status !== "skipped"); + if (ran.length > 0 && ran.every((report) => report.status === "timeout")) return 10; + return 0; // couldn't run tests +}