[serge-ivo]

Closes #106.

Commits

• 62701d7 fix(exec): record a run that hits its time limit as a timeout.
  • ToolRun.status can now be "timeout", with timedOut and timeoutMs alongside it.
  • Inside run() the only change is in the catch block, which now records the timeout.
  • New recordToolRun() lets a runner that executes a command itself write to the same tool-run log.
• 6fbb074 fix(testing): treat a test-run timeout as not run, not failed.
  • Each project's run goes through runWithTreeKill() (new src/runners/exec-tree.ts). The test command runs under a small Node supervisor in its own process group. On timeout the supervisor sends the group SIGTERM, then SIGKILL after 2 s, and waits until the group is gone. On a normal exit, leftover child processes are killed too. Windows uses taskkill /T /F (untested).
  • A timed-out project gets status timeout ("timed out after N s") and a test-run-timeout warning, not an error.
  • Execution scores the same 10/20 as --skip-tests when no project produced results. If other projects did, it is scored on their pass rate alone.
  • No coverage is read for the timed-out project. Coverage files at the repo root are also skipped when any project timed out.
  • Check-level status is worked out as before; there is no new check status.
  • Configurable limit: --test-timeout <ms> overrides checks.testing.settings.timeoutMs, which overrides the default of 120000. The programmatic equivalent is scan(cwd, { testTimeoutMs }).
    • An invalid flag value exits with code 2.
    • An invalid config value falls back to the default and adds an invalid-test-timeout-setting info issue.
    • The effective limit and its source are recorded in check details as testTimeoutMs / testTimeoutSource.
  • Config shape: Analyzer settings schema: pass per-analyzer config through scans and record effective settings #30 is not in yet, so checks.<name>.settings is added here as a plain object, with a getCheckSettings() helper.
  • Complexity: runTesting was already over the complexity limit, and this change would have raised it. Two helpers were extracted, which brings it from 71 down to 58.

Verification

• pnpm build: clean. pnpm lint: 0 errors (62 warnings, same as the baseline on main).
• pnpm test: 65 files, 746/746 passing (main baseline 739, plus 7 new).
• src/runners/test-timeout.test.ts uses a fake vitest in local node_modules, so npx downloads nothing. It sleeps past a 2 s limit and starts a sleep grandchild.
  • Asserts: timedOut: true on the run record; project status timeout; a warning and no error; execution scored the same as --skip-tests; a stale coverage file on disk is not read.
  • Asserts that neither the fake's pid nor the grandchild's pid is alive after scan() returns. A deliberately broken supervisor, which killed only its direct child, failed this assertion.
• With --test-timeout 15000 the same fixture passes, overriding a config limit of 2000.
• exec.test.ts: a real timeout is recorded as timeout; an ordinary failure is not.

Not in this PR

• The docs PR on vibecodeqa/vibecodeqa for --test-timeout and settings.timeoutMs. It will be linked on the issue.
• The CLAUDE.md flags list does not mention --test-timeout yet.

🤖 Generated with Claude Code

https://claude.ai/code/session_01SmtSK7qDTyXhimJ9grap88

[vibecode-qa]

VibeCode QA

PR #110 (fix/106-test-timeout → main)

No workflow found. Add this to enable automatic PR scanning:

name: VibeCode QA
on:
  pull_request:
  workflow_dispatch:
permissions:
  contents: read
  pull-requests: write
  security-events: write
jobs:
  scan:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - uses: vibecodeqa/action@v1
        with:
          fail-under: "70"

Setup guide | Dashboard

[serge-ivo]

Adversarial review, round 1 (COMMENT only)

What I ran. I checked out head 6fbb074 in a throwaway worktree: build clean, lint 0 errors (62 warnings), 746/746 tests pass. I wrote repro scripts against the built dist/runners/exec-tree.js and compared them with main's dist/runners/exec.js. All on macOS.

The timeout classification, the test-run-timeout warning, skipping the stale coverage file, and the config/flag precedence all behave as described. The fixture test would fail on main for the right reason: main ignores settings.timeoutMs, so the run hangs past the test's own limit.

Findings

1. major, CONFIRMED: src/runners/exec-tree.ts:29. Ctrl-C now orphans the test run; on main it does not. The command runs detached, in its own process group, so the terminal's SIGINT no longer reaches it. The supervisor installs no signal handlers, so it dies on that SIGINT without signalling the group.
   Repro: run runWithTreeKill("echo $$ > pid; exec sleep 300", dir, 60000) from a Node script, then kill -INT -<pgid of that script>.

   • PR: sleep 300 is still alive afterwards, reparented to PID 1 (PID 36423 PGID 36423 PPID 1).
   • main (run() / execSync): the same repro leaves nothing running.
     The same path applies when anything kills the CLI from outside. One example is the desktop monitor's 120 s execFile timeout on the whole CLI (src/monitor.tsx:175); I have not reproduced it there. This is the orphan class that #106 exists to remove.
     Fix: in the supervisor, trap SIGINT/SIGTERM/SIGHUP and run signalTree("SIGTERM") then SIGKILL before exiting. Also handle the parent dying outright: give the supervisor a stdin pipe from the parent, and kill the group on its end/close.

2. minor, CONFIRMED: src/runners/testing.ts:79 and src/cli.ts:68. A timeout above 2147483647 ms times out instantly. isValidTimeout accepts any positive integer. Node clamps an oversized setTimeout to 1 ms (TimeoutOverflowWarning).
   Repro: resolveTestTimeout({settings:{timeoutMs:3e9}}) gives {ms:3000000000, source:'config'}. runWithTreeKill("sleep 1; echo done", cwd, 3e9) then returns timedOut: true after 52 ms. Someone who writes a huge value to mean "no limit" gets every project marked timed out.
   Fix: reject or clamp values above 2**31-1, in both the flag and the setting.

3. minor, CONFIRMED: src/runners/exec-tree.ts:29 and :83. A process outside the group that holds the inherited stdout turns a finished run into a "timeout" after limit + 30 s. stdout and stderr are inherited straight from spawnSync's pipes. A test setup that spawns a detached helper (its own process group, so the group kill misses it) keeps the pipe open after the command exits, and spawnSync waits for the 30 s backstop.
   Repro: a command that spawns sleep 45 detached with inherited stdio, prints a report and exits.

   • PR: { ok: false, timedOut: true } after 33002 ms, with a 3000 ms limit.
   • main run(): returns after 3002 ms.
     The command itself finished in milliseconds and its stdout was complete, yet the project is reported as timed out.
     Fix: pipe the child's stdio through the supervisor and close it when the child exits, and treat the supervisor's exit status as authoritative over the backstop's ETIMEDOUT.

4. minor, CONFIRMED from code: src/runners/testing.ts:1250. A timeout hides a command failure in another project. executionPoints returns 10 whenever execution is null and any project timed out. Take a monorepo where project A is command-failed and project B timed out: main scores execution 0, this PR scores 10.
   Relatedly, when some projects do report, a timed-out project is dropped from the pass rate. A slow project with failing tests therefore stops counting against the score. That goes beyond the issue's rule ("execution scored as for --skip-tests").
   Fix: give 10 only when every non-skipped project timed out. Otherwise keep the old rule.

5. minor, CONFIRMED from code: src/monitor.tsx:175. The monitor kills the whole CLI scan after 120 s, so a checks.testing.settings.timeoutMs above 120000 can never take effect there. The scan is killed first, and with finding 1 the test group is orphaned.
   Fix: derive the monitor timeout from the effective test timeout, or drop it when tests run.

6. nit, PLAUSIBLE: output over 64 MB orphans the group. spawnSync hits maxBuffer, gets ENOBUFS, and SIGKILLs the supervisor. The group survives because the supervisor never runs its cleanup. Finding 1's fix (killing the group when the parent pipe closes) covers this too.

7. nit, PLAUSIBLE: a full 10 s wait on every timeout in containers whose PID 1 does not reap. There, orphaned group members stay as zombies, kill(-pgid, 0) keeps succeeding, and the supervisor waits out its 10 s ceiling.

8. nit, CONFIRMED: docs and merge order.

   • README.md:256 and the CLAUDE.md flags list do not mention --test-timeout.
   • This PR conflicts with #109 in the src/cli.ts help block.

Verdict: fix finding 1 before merge. The PR's guarantee is that no process outlives the run, and Ctrl-C now breaks it where main did not.

[serge-ivo]

Round-1 review addressed in six new commits; no force-push. Head is now 40b1884.

1. MAJOR: Ctrl-C orphan. Fixed in ef3db86. The supervisor kills the command's whole process group however the run ends:
   • It traps SIGINT, SIGTERM and SIGHUP.
   • It holds a pipe (fd 4) that only the parent keeps open, and kills the group on EOF there, which covers the parent dying outright. process.ppid is fixed at startup in Node 26, so watching it would not work.
   • On a maxBuffer overrun or the backstop timeout, the parent now sends SIGTERM rather than SIGKILL, so the supervisor still cleans up.
   • Tests in exec-tree.test.ts: "Ctrl-C (SIGINT to the parent's process group) leaves nothing running", the SIGTERM case (exit 143) and "kills the group when the parent dies outright". Disabling the handlers or the pipe one at a time fails the matching test. Your repro was re-run against the build and left nothing alive.
2. Timeout above 2^31-1. Fixed in 6769e14. The flag rejects it (exit 2, message gives the range) and config falls back to the default with an info issue; the runner clamps as a backstop. Accepted range: 1–2147483647 ms.
3. Detached helper holding stdout. Fixed in ef3db86. Output is relayed through the supervisor, which stops waiting 0.2 s after output goes quiet once the command exits (2 s at most). Its fd-3 exit report wins over pipe closure. Test: a backgrounded stdout holder returns ok in under 5 s against a 10 s limit (previously a timeout after about 33 s).
4. Timeout credit hiding a failed project. Fixed in 293f88e. The 10/20 credit applies only when every non-skipped project timed out. Otherwise execution is the pass rate of projects with results, or 0 as on main when none have results, and a failed project keeps its error. Covered by testExecutionPoints unit tests and a two-project scan test.
5. Monitor's fixed 120 s. Fixed in 5530a7c. With tests on, the limit is 120 s plus the test timeout for each test project, capped at 2^31-1. scanTimeoutWithTests has unit tests; the monitor wiring itself has none.
6. Nits. The README and CLAUDE.md flag lists include --test-timeout (40b1884). The over-64 MB output case kills the group, tested by "kills the group when the parent gives up on its output (maxBuffer)".

• 32bf027: run() and recordToolRun() now build entries through one toolRunEntry() helper. A test asserts both paths produce identical records.

Left:

• A project whose test command fails (not times out) still reads its old coverage file, as on main; that is outside A test run that hits the 120 s limit scores worse than --skip-tests, leaves vitest running, and may score stale coverage #106.
• The monitor counts test projects without the scan's source-root detection, so it may undercount outside declared workspaces.
• Nit 7, the 10 s wait under a container init that never cleans up dead children.
• The help-block conflict with fix(ci): honour config failUnder under --ci; print the gate's source (#108) #109 is resolved by whichever merges second.

Verified at 40b1884: pnpm build is clean; pnpm lint has 0 errors (62 warnings, same as main); pnpm test passes 66 files, 763/763.

[vibecode-qa]

VibeCode QA

PR #110 (fix/106-test-timeout → main)

No workflow found. Add this to enable automatic PR scanning:

name: VibeCode QA
on:
  pull_request:
  workflow_dispatch:
permissions:
  contents: read
  pull-requests: write
  security-events: write
jobs:
  scan:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - uses: vibecodeqa/action@v1
        with:
          fail-under: "70"

Setup guide | Dashboard

[serge-ivo]

Adversarial review, round 2 (final, COMMENT only)

What I ran, at head 40b1884. I used a fresh worktree. Build is clean and lint shows 0 errors (62 warnings). pnpm test passes 66 files, 763/763, when run on its own. A merge of #110 with #111 builds and passes 780/780. I re-ran every round-1 repro against dist/ on macOS, then ran a signal matrix on Linux in Docker (node:22-bookworm, with and without --init). I also scanned a copy of a real project (@vibecodeqa/schema, with @vitest/coverage-v8 added so vitest runs and passes) with both main and this head, then diffed the normalized report.json.

Round-1 findings

#	Status	Evidence
1 major: Ctrl-C orphans the run	FIXED	runWithTreeKill("echo $$ > pid; exec sleep 300", …, 60000). I signalled either the parent's process group or the parent alone, with SIGINT, SIGTERM, SIGHUP and SIGKILL. Every case, on macOS and on Linux, left no sleep and no supervisor.
2 timeout above 2^31-1	FIXED	resolveTestTimeout({settings:{timeoutMs:3e9}}) returns {ms:120000, source:'default', invalidSetting:3e9}. runWithTreeKill("sleep 1; echo done", cwd, 3e9) returns ok after 1048 ms. --test-timeout 3000000000, 0 and abc all exit 2 with the range message.
3 detached helper holding stdout	FIXED	The round-1 repro returns ok in 287 ms (main: 3004 ms). A setsid sleep 20 & helper returns in 16 ms. Only fds 0-2 reach the command (/proc/$$/fd on Linux, lsof on macOS), so the fd 3/4 pipes cannot hold the parent either.
4 timeout credit hiding a failed project	FIXED	testExecutionPoints plus the two-project scan test. The test passes and asserts ["command-failed","timeout"]. A timed-out project is still left out of the pass rate when other projects report; the docs now say so.
5 monitor's fixed 120 s	FIXED (code read; the wiring has no test)	—
6 output over 64 MB	FIXED	The test passes. A 20 MB stdout relays intact (END marker present, same length as main).
7 10 s wait when PID 1 does not reap	NOT FIXED (acknowledged)	CONFIRMED on Linux: with node as PID 1, a timed-out (sleep 30 &); (sleep 30 &); sleep 30 at a 1000 ms limit returns after 11047 ms. Under --init it returns after 1041 ms. Optional fix: once SIGKILL has gone to the group, stop polling after a short grace instead of waiting for zombies to be reaped.
8 docs and merge order	FIXED (docs) / still open (merge)	git merge-tree pr109 pr110 now conflicts in README.md as well as src/cli.ts.

Shared record helper. For non-test runners every ToolRun field comes out as on main. The diff of the normalized schema report, main against this head, contains only:

• the new testTimeoutMs / testTimeoutSource details;
• timing-dependent order of the slowest tests;
• biome's internal duration value.

All 39 check scores are identical (overall 87). On the failing path (exit 3, command not found) status, exitCode and notFound match main.

New findings

1. minor, CONFIRMED: src/runners/exec-tree.ts:90. Ctrl-\ (SIGQUIT) still orphans the run; main does not. The trap list covers SIGINT, SIGTERM and SIGHUP. SIGQUIT's default action kills the supervisor before it can signal the group, and the fd-4 watch dies with it.
   Repro: the same harness, with kill -QUIT -<pgid>.

   • PR: sleep survives (PID 4250 PGID 4250 PPID 1).
   • main (run()): nothing survives.

   Fix: add ["SIGQUIT", 131] to the list.

2. nit, CONFIRMED: src/runners/exec-tree.ts:112. A command killed by a signal is recorded as exit code 128, whatever the signal. code ?? (signal ? 128 : 1) maps SIGKILL, SIGTERM and SIGSEGV all to 128. On main exitCode was null in that case.

   • kill -9 $$ gives exitCode 128; main gives null.
   • kill -TERM $$ gives exitCode 128 and empty output; main gives null and Error: Command failed: ….

   An OOM-killed vitest (137) cannot be told apart from a crash.
   Fix: 128 + (os.constants.signals[signal] ?? 0), or record null as main did.

3. nit, PLAUSIBLE: two new tests depend on timing. I ran the four suites (main, #109, #110, #111) at once on one machine. Two #110 tests failed there and passed on a serial re-run:

   • test-timeout.test.ts:93 (a 2000 ms limit): pids.json was never written before the limit hit.
   • :210 (1500 ms): the project meant to fail by command timed out instead, giving ['timeout','timeout'].

   The same heavy load also made main's --json produces valid JSON test time out, so this is weak evidence. A 2-vCPU CI runner still sits closer to that load than a dev laptop does.
   Fix: give the project that must finish a generous limit, separate from the one that must time out.

Verdict: READY TO MERGE. The round-1 blocker is fixed and holds on both macOS and Linux. Finding 1 is a one-line change worth adding before merge. Findings 2 and 3 can follow. Resolve the #109 conflict (src/cli.ts help block and README.md flags table) without dropping either side's rows.

[serge-ivo]

Round-2 findings addressed in two new commits; no force-push. Head is now 128a770.

1. SIGQUIT. Fixed in 1037bd3: ["SIGQUIT", 131] added to the trapped signals. Test: "Ctrl-\ (SIGQUIT to the supervisor) kills the group too" asserts the group is gone and the status is 131.
2. Signal exit codes. Fixed in the same commit: 128 + os.constants.signals[signal] instead of a flat 128. Test: kill -9 $$ is recorded as exitCode 137 and kill -TERM $$ as 143.
   • Both new tests fail against the previous exec-tree.ts; the SIGQUIT case orphaned its sleep, as in your repro.
3. Timing. Fixed in 128a770. The single-project test (:93) uses a 5 s limit, so the fake writes pids.json well before it. The two-project test (:210) shares one 5 s limit (the per-project limit is uniform): the 30 s sleeper still times out and the failing project has headroom. Its vitest timeout is 60 s.

The #109 conflict is left to be resolved by merging main in after #109 merges, keeping both sides' help and README rows.

Verified at 128a770: pnpm build is clean; pnpm lint has 0 errors (62 warnings, same as main); pnpm test passes 66 files, 765/765.

[vibecode-qa]

VibeCode QA

PR #110 (fix/106-test-timeout → main)

No workflow found. Add this to enable automatic PR scanning:

name: VibeCode QA
on:
  pull_request:
  workflow_dispatch:
permissions:
  contents: read
  pull-requests: write
  security-events: write
jobs:
  scan:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - uses: vibecodeqa/action@v1
        with:
          fail-under: "70"

Setup guide | Dashboard

[vibecode-qa]

VibeCode QA

PR #110 (fix/106-test-timeout → main)

No workflow found. Add this to enable automatic PR scanning:

name: VibeCode QA
on:
  pull_request:
  workflow_dispatch:
permissions:
  contents: read
  pull-requests: write
  security-events: write
jobs:
  scan:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - uses: vibecodeqa/action@v1
        with:
          fail-under: "70"

Setup guide | Dashboard

[serge-ivo]

Merged main (782f54a) in and recorded tool runs through the shared recorder. Head is now adacab2. Both are new commits; no rebase or force-push.

• b05944a, merge commit. Conflicts resolved:

  • src/runners/exec.ts: run() keeps this PR's recordToolRun(...) calls on both the success and failure paths. main's inline record({...}) blocks are dropped; recordToolRun builds its entry and hands it to main's record().
  • src/runners/testing.ts: imports keep MAX_TIMER_MS/runWithTreeKill and main's redactSecrets. The now-unused run import is dropped.
  • src/core.ts: both import sets, and both ScanOptions additions (testTimeoutMs, and diffBase/ciContext).
  • src/cli.ts and README.md: the --test-timeout row plus fix(ci): honour config failUnder under --ci; print the gate's source (#108) #109's --ci / --fail-under rows.

• adacab2. toolRunEntry passes the command and output to record() whole, instead of a trimmed, capped copy, because record() does its own processing and capping. New tests in exec-tree.test.ts ("runWithTreeKill records through the shared recorder") use a value assembled at runtime that the recorder recognises:

  • on a successful run and on a timed-out run, no 5-character window of the value's body appears in the recorded output or command
  • when the value straddles the 8000-character cap, none of it survives either

  The straddle test fails if toolRunEntry truncates first.

Verified at adacab2: pnpm build is clean; pnpm lint has 0 errors (61 warnings); pnpm test passes 72 files, 1003/1003. src/report-output-hygiene.test.ts and src/runners/redact.test.ts pass 62/62 on the merged branch.