Run only the tests the diff touches

[sergical]

run_tests now reads the changed files from fixtures/latest.diff and runs vitest related for them instead of the whole suite. It removes the previous report before each run, and returns passed, failed, the number of test files run and the failing test names, or "no related tests" when nothing in the diff has a test.

Created with Claude Code

[github-actions]

Verdict

Approve with minor follow-ups — the vitest related rework is a solid improvement, but there's one real correctness gap (hardcoded diff path desync) worth fixing before merge, plus several maintainability nits.

Correctness

1. Hardcoded fixtures/latest.diff path can desync from the diff actually under review (run_tests tool, readFile('fixtures/latest.diff', 'utf8')). The read_diff tool used earlier in the workflow accepts an arbitrary path argument, but run_tests always re-reads the fixed path fixtures/latest.diff instead of reusing the path/content that was already loaded. If the lead agent is ever given a diff at a different location, run_tests will silently fall back to noRelatedTests (via the .catch(() => '')) rather than surfacing an error — a silent false negative that could hide a legitimately broken PR. Low risk today since the workflow always uses fixtures/latest.diff, but it's a latent trap.

2. Inconsistent path resolution base: fixtures/latest.diff is read relative to process.cwd(), while changedFiles are resolved against repoRoot. If these two differ (plausible, since the vitest invocation itself runs with cwd: 'fixtures/demo-pr'), the diff read can fail silently and again produce a false "no related tests" result instead of an explicit error.

3. Regex ^diff --git a\/(\S+) b\/\S+$ has known gaps: it won't match filenames containing spaces (quoted diff headers) and doesn't special-case renames, so the "old" path is used rather than the new one. Not fatal (vitest will just skip missing paths), but worth a comment/TODO since it can silently under-report which files are considered "changed."

4. Parse-failure vs. no-tests conflation: On JSON.parse failure the code returns noRelatedTests whenever exitCode === 0. If vitest related --passWithNoTests exits 0 but writes a malformed/truncated report (reporter crash, etc.), this is indistinguishable from "no related tests," which could mask a real tooling failure as a clean/empty result.

None of these are blocking for the stated goal (speeding up run_tests by scoping to related files), but #1/#2 should be tightened if read_diff's path ever becomes dynamic in practice.

Tests: run_tests was executed and returned {"passed":0,"failed":0,"ran":0,"note":"no related tests"} — consistent with the new "no related tests" fallback behavior for this diff; no failures to report.

Style / Maintainability

• noRelatedTests is named like a boolean but holds a pre-serialized JSON string — rename to something like noRelatedTestsPayload for clarity at call sites.
• The 'fixtures/latest.diff' string and the diff-header regex are both inlined magic values with no named constant/helper or comment explaining the diff-format assumptions; pulling them into a named constant / small extractChangedFilesFromDiff(diff) helper would aid future readers.
• The early-return branch (changedFiles.length === 0) duplicates the shape of the final log.info('tests finished', …) call with a hardcoded exitCode: 0; consider funneling both paths through one log call.
• Reusing noRelatedTests for the "report failed to parse but exit code was 0" case overloads one constant with two different real conditions (see correctness [lab 5 test] Eight files, reviewer with the per-file fan-out #4) — worth a distinct message for debuggability.
• reportPath is a single fixed tmpdir()/vitest.json location with an explicit rm before each run as a workaround; a unique per-run path (e.g., fs.mkdtemp or a random suffix) would be more robust against concurrent invocations, though this isn't a regression from the prior /tmp/vitest.json behavior.
• The vitest arg array now interleaves positional file paths with flags (...changedFiles, '--run', '--passWithNoTests', ...), which is a bit harder to scan than the previous flat flag list; grouping/commenting file args vs. option flags would help readability.

Overall: good functional direction (scoping tests to the diff, adding ran count, handling "no tests" case), but tighten the diff-path coupling and clean up the minor duplication/naming before considering this final.

[linear-code]

MAR-1676