From 7de025eacd8ca6b2338c4a77066a9cb5369ed9c1 Mon Sep 17 00:00:00 2001 From: Dallin Romney Date: Fri, 29 May 2026 20:38:52 -0700 Subject: [PATCH] fix: route explicit vitest files through project runner (#88127) --- .agents/skills/openclaw-testing/SKILL.md | 4 +- AGENTS.md | 1 + scripts/AGENTS.md | 1 + scripts/run-vitest.mjs | 116 +++++++++++++++++++++++ scripts/test-projects.test-support.mjs | 13 +++ test/scripts/run-vitest.test.ts | 67 +++++++++++++ test/scripts/test-projects.test.ts | 49 ++++++++++ 7 files changed, 250 insertions(+), 1 deletion(-) diff --git a/.agents/skills/openclaw-testing/SKILL.md b/.agents/skills/openclaw-testing/SKILL.md index 2b22d570031c..374f223db6da 100644 --- a/.agents/skills/openclaw-testing/SKILL.md +++ b/.agents/skills/openclaw-testing/SKILL.md @@ -75,7 +75,9 @@ OPENCLAW_VITEST_MAX_WORKERS=1 pnpm test ``` Use targeted file paths whenever possible. Avoid raw `vitest`; use the repo -`pnpm test` wrapper so project routing, workers, and setup stay correct. +`pnpm test` wrapper so project routing, workers, and setup stay correct. If raw +Vitest is unavoidable, use `vitest run ...`; bare `vitest ...` starts local watch +mode and will not exit on its own. When the checkout is a Codex worktree, prefer the direct node harness instead: ```bash diff --git a/AGENTS.md b/AGENTS.md index 5b2cfe2ecba9..44bc0f2afd58 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -93,6 +93,7 @@ Skills own workflows; root owns hard policy and routing. - Install: `pnpm install` (keep Bun lock/patches aligned if touched). - CLI: `pnpm openclaw ...` or `pnpm dev`; build: `pnpm build`. - Tests in a normal source checkout: `pnpm test [vitest args...]`, `pnpm test:changed`, `pnpm test:serial`, `pnpm test:coverage`; never raw `vitest`. +- If raw Vitest is unavoidable, use `vitest run ...`; bare `vitest ...` starts local watch mode and will not exit on its own. - Tests in a Codex worktree or linked/sparse checkout: avoid direct local `pnpm test*`; use `node scripts/run-vitest.mjs ` for tiny explicit-file proof, or Crabbox/Testbox for anything broader. - Checks in a normal source checkout: `pnpm check:changed`; lanes: `pnpm changed:lanes --json`; staged: `pnpm check:changed --staged`; full: `pnpm check`. - Checks in a Codex worktree or linked/sparse checkout: avoid direct local `pnpm check*`; use `node scripts/crabbox-wrapper.mjs run ... --shell -- "pnpm check:changed"` so pnpm runs inside Testbox, not locally. diff --git a/scripts/AGENTS.md b/scripts/AGENTS.md index fc3b224040f1..4df5e7890aad 100644 --- a/scripts/AGENTS.md +++ b/scripts/AGENTS.md @@ -6,6 +6,7 @@ This directory owns local tooling, script wrappers, and generated-artifact helpe - Prefer existing wrappers over raw tool entrypoints when the repo already has a curated seam. - For tests, prefer `scripts/run-vitest.mjs` or the root `pnpm test ...` entrypoints over raw `vitest run` calls. +- Never use bare `vitest ...` in automation; it starts local watch mode unless `run` or `--run` is explicit. - For lint/typecheck flows, prefer `scripts/run-oxlint.mjs` and `scripts/run-tsgo.mjs` when adding or editing package scripts or CI steps that should honor repo-local runtime behavior. - For changed-file verification, prefer `scripts/check-changed.mjs` and keep lane classification in `scripts/changed-lanes.mjs`. Do not copy path-scope rules into new hooks or ad hoc CI snippets. diff --git a/scripts/run-vitest.mjs b/scripts/run-vitest.mjs index 800cde10b27d..a53de1c27313 100644 --- a/scripts/run-vitest.mjs +++ b/scripts/run-vitest.mjs @@ -81,6 +81,7 @@ const VITEST_DOTTED_OPTIONS_WITH_VALUE_PREFIXES = [ ]; const require = createRequire(import.meta.url); const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); +const testProjectsRunnerPath = path.join(repoRoot, "scripts", "test-projects.mjs"); function isTruthyEnvValue(value) { return TRUTHY_ENV_VALUES.has(value?.trim().toLowerCase() ?? ""); @@ -348,6 +349,93 @@ function hasAlternateVitestRootArg(argv) { ); } +function hasExplicitVitestProjectArg(argv) { + return argv.some((arg) => arg === "--project" || arg.startsWith("--project=")); +} + +function hasExplicitDisabledRunFlag(argv) { + for (let index = 0; index < argv.length; index += 1) { + const arg = argv[index]; + if (arg === "--") { + break; + } + const runFlag = resolveBooleanModeFlag(argv, index, "run"); + if (!runFlag) { + if (optionConsumesNextArg(arg)) { + index += 1; + } + continue; + } + if (runFlag.consumedNext) { + index += 1; + } + if (!runFlag.value) { + return true; + } + } + return false; +} + +function hasSeparateVitestOptionValueArg(argv) { + for (let index = 0; index < argv.length; index += 1) { + const arg = argv[index]; + if (arg === "--") { + return false; + } + if (optionConsumesNextArg(arg)) { + return true; + } + } + return false; +} + +function stripRunSubcommand(argv) { + const stripped = []; + let canRemoveRunSubcommand = true; + for (let index = 0; index < argv.length; index += 1) { + const arg = argv[index]; + if (arg === "--") { + stripped.push(arg); + canRemoveRunSubcommand = false; + continue; + } + if (canRemoveRunSubcommand && optionConsumesNextArg(arg)) { + stripped.push(arg); + if (index + 1 < argv.length) { + index += 1; + stripped.push(argv[index]); + } + continue; + } + if (canRemoveRunSubcommand && arg.startsWith("-")) { + stripped.push(arg); + continue; + } + if (canRemoveRunSubcommand && arg === "run") { + canRemoveRunSubcommand = false; + continue; + } + canRemoveRunSubcommand = false; + stripped.push(arg); + } + return stripped; +} + +export function resolveTestProjectsDelegationArgs(argv) { + if ( + hasExplicitVitestConfigArg(argv) || + hasAlternateVitestRootArg(argv) || + hasExplicitVitestProjectArg(argv) || + resolveExplicitVitestMode(argv) === "watch" || + hasExplicitDisabledRunFlag(argv) || + hasSeparateVitestOptionValueArg(argv) || + collectExplicitTestFileArgs(argv).length === 0 + ) { + return null; + } + return stripRunSubcommand(argv); +} + export function resolveMissingExplicitTestFiles(argv, cwd = process.cwd(), fsImpl = fs) { if (hasExplicitVitestConfigArg(argv) || hasAlternateVitestRootArg(argv)) { return []; @@ -573,6 +661,17 @@ export function spawnWatchedVitestProcess({ }; } +export function resolveTestProjectsRunnerEnv(env) { + return resolveVitestSpawnEnv(env); +} + +function spawnTestProjectsRunner(argv, env) { + return spawn(process.execPath, [testProjectsRunnerPath, ...argv], { + env: resolveTestProjectsRunnerEnv(env), + stdio: "inherit", + }); +} + function main(argv = process.argv.slice(2), env = process.env) { if (argv.length === 0) { console.error("usage: node scripts/run-vitest.mjs "); @@ -590,6 +689,23 @@ function main(argv = process.argv.slice(2), env = process.env) { process.exit(1); } + const delegatedArgs = resolveTestProjectsDelegationArgs(argv); + if (delegatedArgs) { + const child = spawnTestProjectsRunner(delegatedArgs, env); + child.on("exit", (code, signal) => { + if (signal) { + process.kill(process.pid, signal); + return; + } + process.exit(code ?? 1); + }); + child.on("error", (error) => { + console.error(error); + process.exit(1); + }); + return; + } + const vitestArgs = resolveImplicitVitestArgs(argv); const guardedVitestArgs = resolveExplicitTestFileNoPassArgs(vitestArgs); const spawnEnv = resolveRunVitestSpawnEnv(env, guardedVitestArgs); diff --git a/scripts/test-projects.test-support.mjs b/scripts/test-projects.test-support.mjs index 3e8321aaf60a..b65ec89318f9 100644 --- a/scripts/test-projects.test-support.mjs +++ b/scripts/test-projects.test-support.mjs @@ -1740,9 +1740,20 @@ export function parseTestProjectsArgs(args, cwd = process.cwd()) { const forwardedArgs = []; const targetArgs = []; let watchMode = false; + let passthrough = false; for (const arg of args) { if (arg === "--") { + if (targetArgs.length > 0) { + passthrough = true; + } + continue; + } + if (passthrough) { + if (arg === "--watch") { + watchMode = true; + } + forwardedArgs.push(arg); continue; } if (arg === "--watch") { @@ -1874,7 +1885,9 @@ export function buildVitestRunPlans( "utils", "wizard", "e2e", + "extensionActiveMemory", "extensionAcpx", + "extensionCodex", "extensionDiffs", "extensionBrowser", "extensionDiscord", diff --git a/test/scripts/run-vitest.test.ts b/test/scripts/run-vitest.test.ts index 59f1bdb14bc6..6ba68f86556b 100644 --- a/test/scripts/run-vitest.test.ts +++ b/test/scripts/run-vitest.test.ts @@ -8,6 +8,8 @@ import { resolveMissingVitestDependencyMessage, resolveMissingExplicitTestFiles, resolveRunVitestSpawnEnv, + resolveTestProjectsDelegationArgs, + resolveTestProjectsRunnerEnv, resolveVitestCliEntry, resolveVitestNodeArgs, resolveVitestNoOutputTimeoutMs, @@ -134,6 +136,56 @@ describe("scripts/run-vitest", () => { expect(resolveExplicitTestFileNoPassArgs(argv)).toBe(argv); }); + it("delegates bare explicit test files to the project router", () => { + const file = "test/scripts/run-vitest.test.ts"; + for (const [argv, expected] of [ + [[file], [file]], + [["run", file], [file]], + [ + ["run", file, "--reporter=verbose"], + [file, "--reporter=verbose"], + ], + [ + ["--reporter=verbose", "run", file], + ["--reporter=verbose", file], + ], + [ + ["run", file, "--", "--watch"], + [file, "--", "--watch"], + ], + [ + ["run", file, "--", "--reporter=verbose"], + [file, "--", "--reporter=verbose"], + ], + ] as const) { + expect(resolveTestProjectsDelegationArgs([...argv])).toEqual(expected); + } + }); + + it("keeps direct Vitest runs when project routing could change option semantics", () => { + const directArgvCases = [ + [ + "run", + "--config", + "test/vitest/vitest.tooling.config.ts", + "test/scripts/run-vitest.test.ts", + ], + ["--root", "packages/example", "src/example.test.ts"], + ["--project", "tooling", "test/scripts/run-vitest.test.ts"], + ["watch", "test/scripts/run-vitest.test.ts"], + ["dev", "test/scripts/run-vitest.test.ts"], + ["--watch", "test/scripts/run-vitest.test.ts"], + ["--run=false", "test/scripts/run-vitest.test.ts"], + ["--no-run", "test/scripts/run-vitest.test.ts"], + ["--run", "false", "test/scripts/run-vitest.test.ts"], + ["--testNamePattern", "run", "test/scripts/run-vitest.test.ts"], + ["run", "test/scripts/run-vitest.test.ts", "-t", "src"], + ]; + for (const argv of directArgvCases) { + expect(resolveTestProjectsDelegationArgs(argv)).toBeNull(); + } + }); + it("reports missing explicit test files before Vitest can silently ignore them", () => { const fsImpl = { existsSync: (filePath: string) => @@ -324,6 +376,21 @@ describe("scripts/run-vitest", () => { }); }); + it("does not force the stall watchdog into delegated runner environments", () => { + expect(resolveTestProjectsRunnerEnv({ PATH: "/usr/bin" })).toEqual({ + PATH: "/usr/bin", + }); + expect( + resolveTestProjectsRunnerEnv({ + OPENCLAW_VITEST_NO_OUTPUT_TIMEOUT_MS: "2500", + PATH: "/usr/bin", + }), + ).toEqual({ + OPENCLAW_VITEST_NO_OUTPUT_TIMEOUT_MS: "2500", + PATH: "/usr/bin", + }); + }); + it("spawns vitest in a detached process group on Unix hosts", () => { expect(resolveVitestSpawnParams({ PATH: "/usr/bin" }, "darwin")).toEqual({ env: { PATH: "/usr/bin" }, diff --git a/test/scripts/test-projects.test.ts b/test/scripts/test-projects.test.ts index e6269e9b4ad7..5ed9466a92c5 100644 --- a/test/scripts/test-projects.test.ts +++ b/test/scripts/test-projects.test.ts @@ -384,6 +384,33 @@ describe("scripts/test-projects changed-target routing", () => { ]); }); + it("preserves post-separator Vitest args without parsing them as targets", () => { + for (const [arg, watchMode] of [ + ["--reporter=verbose", false], + ["--watch", true], + ] as const) { + expect(buildVitestRunPlans(["test/scripts/run-vitest.test.ts", "--", arg])).toEqual([ + { + config: "test/vitest/vitest.tooling.config.ts", + forwardedArgs: [arg], + includePatterns: ["test/scripts/run-vitest.test.ts"], + watchMode, + }, + ]); + } + }); + + it("keeps pnpm-style leading separators out of target routing", () => { + expect(buildVitestRunPlans(["--", "test/scripts/run-vitest.test.ts"])).toEqual([ + { + config: "test/vitest/vitest.tooling.config.ts", + forwardedArgs: [], + includePatterns: ["test/scripts/run-vitest.test.ts"], + watchMode: false, + }, + ]); + }); + it("allows explicit split Vitest config targets without treating them as unmatched tests", () => { expect( findUnmatchedExplicitTestTargets( @@ -640,6 +667,28 @@ describe("scripts/test-projects changed-target routing", () => { ]); }); + it("routes explicit active-memory and Codex extension tests to their shards", () => { + expect( + buildVitestRunPlans([ + "extensions/active-memory/index.test.ts", + "extensions/codex/index.test.ts", + ]), + ).toEqual([ + { + config: "test/vitest/vitest.extension-active-memory.config.ts", + forwardedArgs: [], + includePatterns: ["extensions/active-memory/index.test.ts"], + watchMode: false, + }, + { + config: "test/vitest/vitest.extension-codex.config.ts", + forwardedArgs: [], + includePatterns: ["extensions/codex/index.test.ts"], + watchMode: false, + }, + ]); + }); + it("routes the top-level extensions target to every extension shard", () => { expect(buildVitestRunPlans(["extensions"], process.cwd())).toEqual( listFullExtensionVitestProjectConfigs().map((config) => ({