Give the PR reviewer a run_tests tool

[sergical]

The review agent can now call run_tests, which runs the demo-pr vitest suite (about six minutes, dominated by the simulated integration tests) and reports failures in the Correctness section. The fixture gets a vitest project with unit and integration tests, the workflow installs its dependencies, and read_file resolves paths from the repo root so it no longer fails with ENOENT from apps/pr-reviewer.

Created with Claude Code

[socket-security]

Warning

Socket is a tool Sentry Security team deployed to defend against supply chain attack. Please provide any feedback you have in #discuss-security slack channel ❤️

Priority	Alert  (click "▶" to expand/collapse)	Action
	Critical CVE: Tinypool: Prototype Pollution gadget in worker options leads to Remote Code Execution CVE: GHSA-5gmw-xhrv-c9v3 Tinypool: Prototype Pollution gadget in worker options leads to Remote Code Execution (CRITICAL) Affected versions: < 2.1.1 Patched version: 2.1.1 From: apps/pr-reviewer/fixtures/demo-pr/package-lock.json → npm/[email protected] → npm/[email protected] ℹ Read more on: This package | This alert | What is a critical CVE? Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at [email protected]. Suggestion: Remove or replace dependencies that include known critical CVEs. Consumers can use dependency overrides or npm audit fix --force to remove vulnerable dependencies. Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/[email protected]. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.	Warn
	Critical CVE: Tinypool: Prototype Pollution Gadget to RCE in run() options CVE: GHSA-85c8-ppgw-ccpr Tinypool: Prototype Pollution Gadget to RCE in run() options (CRITICAL) Affected versions: < 2.1.2 Patched version: 2.1.2 From: apps/pr-reviewer/fixtures/demo-pr/package-lock.json → npm/[email protected] → npm/[email protected] ℹ Read more on: This package | This alert | What is a critical CVE? Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at [email protected]. Suggestion: Remove or replace dependencies that include known critical CVEs. Consumers can use dependency overrides or npm audit fix --force to remove vulnerable dependencies. Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/[email protected]. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.	Warn
	Critical CVE: When Vitest UI server is listening, arbitrary file can be read and executed CVE: GHSA-5xrq-8626-4rwp When Vitest UI server is listening, arbitrary file can be read and executed (CRITICAL) Affected versions: >= 4.0.0 < 4.1.0; < 3.2.6 Patched version: 3.2.6 From: apps/pr-reviewer/fixtures/demo-pr/package-lock.json → npm/[email protected] ℹ Read more on: This package | This alert | What is a critical CVE? Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at [email protected]. Suggestion: Remove or replace dependencies that include known critical CVEs. Consumers can use dependency overrides or npm audit fix --force to remove vulnerable dependencies. Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/[email protected]. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.	Warn

View full report

[github-actions]

Verdict

Request changes — the new run_tests tool has a real risk of CI timeout/failure due to cwd inconsistency and excessive sleep-based test time; tests pass locally but took ~360s, well into the danger zone for the 15‑minute job timeout.

Correctness

• apps/pr-reviewer/src/agents/review.ts (run_tests tool) — cwd: 'fixtures/demo-pr' is relative to process.cwd(), while every other tool in this same diff was just changed to resolve against repoRoot (process.env.GITHUB_WORKSPACE or two levels up) specifically to avoid depending on the process's working directory. If flue run is invoked from a directory other than apps/pr-reviewer, this execFileAsync call will fail with ENOENT/ENOTDIR where read_file would have succeeded. Should be path.resolve(repoRoot, 'apps/pr-reviewer/fixtures/demo-pr') (or equivalent) for consistency and robustness.

• CI timeout risk — The new checkout-flow.integration.test.ts adds 6 tests that each await settle() for a full 60 seconds with no real asynchronous work to justify it. Measured suite run time was ~360s (6 minutes) for sleeping alone. Added to the two npm ci invocations, diff/file reads, and two subagent LLM calls, this materially eats into the job's timeout-minutes: 15, risking the whole workflow being killed mid-review with no comment posted and no clear failure surfaced. The subsequent execFileAsync('npm', ['test'], ...) call also has no explicit timeout, so if it does hang, the only backstop is the GitHub Actions job timeout, which produces a much less useful failure mode.

• refund-window.test.ts — canRefund(daysAgo(-2), now) (a delivery date in the future) returns true, which the test asserts as correct. This locks in behavior where days goes negative and days <= 30 is trivially satisfied, meaning deliveries dated arbitrarily far in the future are always "refundable." That's arguably a latent bug in canRefund being codified as intended behavior rather than flagged.

• All other test assertions (cart-total, loyalty, refund-window arithmetic) check out against the implementations (cartTotal, pointsFor, canRefund) — no logic errors found there.

Test run: npm test in fixtures/demo-pr passed all 36 tests (4 files), but took 360.43s — driven almost entirely by the 6 integration tests' 60s sleeps. Worth flagging explicitly since it's the direct cause of the CI timeout risk above.

Style

• .github/workflows/review.yml: the new npm ci step (for fixtures/demo-pr) has no name:, inconsistent with neighboring named steps; add something like name: Install demo-pr fixture deps.
• Running npm ci for the fixture on every single PR review invocation (regardless of whether the fixture changed) adds fixed overhead with no caching (actions/setup-node's cache option isn't used); worth reconsidering given the timeout pressure above.
• checkout-flow.integration.test.ts: the rationale comment for the 60s settle() delay sits at module top rather than attached to the settle function itself (e.g., as a doc comment) — minor, but would keep the "why" next to the "what."
• DAY_MS is duplicated verbatim in both checkout-flow.integration.test.ts and refund-window.test.ts; consider a small shared test-util constant to avoid drift.
• run_tests's catch-block error cast (error as { code?: number; stdout?: string; stderr?: string }) could be named as a small local type for clarity/reuse.
• The numbered instructions in the ReviewLead prompt were renumbered to fit the new step — correct here, but worth double-checking no other stale "step N" references exist elsewhere in the prompt text.

Summary

Logic in the demo fixtures themselves is correct and well-tested. The main actionable issues are (1) the run_tests tool's cwd not using the newly-introduced repoRoot resolution like its sibling tool, and (2) the integration test file's unnecessary 60s-per-test sleeps pushing total CI time close to the job timeout — both should be addressed before merge. Style nits are minor and non-blocking.

[linear-code]

MAR-1676