diff --git a/src/gateway/http-utils.authorize-request.test.ts b/src/gateway/http-utils.authorize-request.test.ts index 95973ebbc7fa..283df6805037 100644 --- a/src/gateway/http-utils.authorize-request.test.ts +++ b/src/gateway/http-utils.authorize-request.test.ts @@ -27,9 +27,12 @@ vi.mock("../config/io.js", () => ({ })), })); +// Export every binding http-auth-utils.js imports from http-common.js so this +// factory stays safe under isolate:false regardless of which paths execute. vi.mock("./http-common.js", () => ({ sendGatewayAuthFailure: vi.fn(), sendJson: vi.fn(), + sendMissingScopeForbidden: vi.fn(), })); const { authorizeHttpGatewayConnect } = await import("./auth.js"); diff --git a/src/gateway/server-methods/agent.sessions-and-models.test-utils.ts b/src/gateway/server-methods/agent.sessions-and-models.test-utils.ts index 1a21579ef735..1e70a28219fd 100644 --- a/src/gateway/server-methods/agent.sessions-and-models.test-utils.ts +++ b/src/gateway/server-methods/agent.sessions-and-models.test-utils.ts @@ -2030,7 +2030,14 @@ describe("gateway agent handler", () => { const defaultRuntime = getDetachedTaskLifecycleRuntime(); const finalizeError = new Error("finalize boom"); + // The background run completes off-turn; signal finalize instead of + // polling for it so contended runners cannot outlast a fixed poll budget. + let signalFinalizeCalled: () => void = () => {}; + const finalizeCalled = new Promise((resolve) => { + signalFinalizeCalled = resolve; + }); const finalizeTaskRunByRunIdSpy = vi.fn(() => { + signalFinalizeCalled(); throw finalizeError; }); setDetachedTaskLifecycleRuntime({ @@ -2050,9 +2057,12 @@ describe("gateway agent handler", () => { { context, respond, reqId: "task-registry-finalize-throw" }, ); + // Event-driven wait bounded by the test timeout; the follow-up + // observations land in the same completion path right after finalize. + await finalizeCalled; + expect(finalizeTaskRunByRunIdSpy).toHaveBeenCalledTimes(1); await waitForAssertion(() => { // Finalize threw, but the run must still complete (second res frame with ok status). - expect(finalizeTaskRunByRunIdSpy).toHaveBeenCalledTimes(1); const completed = respond.mock.calls.some(([ok, payload]) => { return ok === true && (payload as { status?: string } | undefined)?.status === "ok"; }); diff --git a/src/gateway/session-message-events.test.ts b/src/gateway/session-message-events.test.ts index fd1b5e9a41ee..0b409eb9b87b 100644 --- a/src/gateway/session-message-events.test.ts +++ b/src/gateway/session-message-events.test.ts @@ -44,6 +44,10 @@ let subscribedOperatorWs: | Awaited>["openWs"]>> | undefined; +// No explicit hook timeout: the suite harness cold-imports the full gateway +// server graph, which can legitimately exceed 60s on contended CI runners. +// Sibling gateway suites rely on the shared project hookTimeout (120s, 180s on +// Windows) for the same boot; tightening it here caused flaky hook timeouts. beforeAll(async () => { harness = await createGatewaySuiteHarness(); subscribedOperatorWs = await harness.openWs(); @@ -52,7 +56,7 @@ beforeAll(async () => { timeoutMs: SETUP_RPC_TIMEOUT_MS, }); await rpcReq(subscribedOperatorWs, "sessions.subscribe", undefined, SETUP_RPC_TIMEOUT_MS); -}, 60_000); +}); afterAll(async () => { subscribedOperatorWs?.close(); diff --git a/test/non-isolated-runner.test.ts b/test/non-isolated-runner.test.ts new file mode 100644 index 000000000000..545ea83ebdcb --- /dev/null +++ b/test/non-isolated-runner.test.ts @@ -0,0 +1,120 @@ +// Regression coverage for the non-isolated runner's cross-file cleanup: when a +// sibling file fails during collection, vitest skips onAfterRunSuite for it, so +// cleanup must run from onAfterRunFiles or the crashed file's evaluated real +// modules stay cached and the next file's vi.mock factories silently never apply. +import { execFile } from "node:child_process"; +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { promisify } from "node:util"; +import { expect, it } from "vitest"; + +const execFileAsync = promisify(execFile); +const repoRoot = path.resolve(import.meta.dirname, ".."); + +function childEnv(): NodeJS.ProcessEnv { + const env: NodeJS.ProcessEnv = {}; + for (const [key, value] of Object.entries(process.env)) { + // Drop parent Vitest state so the child run resolves its own config, and + // drop GITHUB_ACTIONS so the child's github-actions reporter cannot emit + // ::error annotations that the parent CI job renders as its own failures. + if ( + key.startsWith("VITEST") || + key.startsWith("OPENCLAW_VITEST") || + key === "GITHUB_ACTIONS" || + key === "FORCE_COLOR" + ) { + continue; + } + env[key] = value; + } + // "CI" in env alone turns tinyrainbow colors on; NO_COLOR overrides every + // enable path, keeping the plain-text substring assertions below stable. + env.NO_COLOR = "1"; + return env; +} + +it("applies vi.mock factories after a sibling file fails during collection", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-non-isolated-runner-")); + try { + const write = (name: string, content: string) => + fs.writeFile(path.join(root, name), content, "utf-8"); + // The child project resolves vitest through the repository install. + await fs.symlink( + path.join(repoRoot, "node_modules"), + path.join(root, "node_modules"), + "junction", + ); + await write("dep.ts", 'export function flavor(): string {\n return "real";\n}\n'); + await write( + "mid.ts", + [ + 'import { flavor } from "./dep.js";', + "export function describeFlavor(): string {", + " return `flavor:${flavor()}`;", + "}", + "", + ].join("\n"), + ); + // Evaluates the real mid->dep chain into the shared module cache, then + // fails collection so runSuite early-returns without onAfterRunSuite. + await write( + "a-crash.test.ts", + 'import "./mid.js";\nthrow new Error("synthetic collect failure");\n', + ); + // Mocks the leaf module and observes it through the cached importer; with a + // poisoned cache this sees "flavor:real" instead of the mocked value. + await write( + "b-mock.test.ts", + [ + 'import { expect, it, vi } from "vitest";', + 'vi.mock("./dep.js", () => ({ flavor: () => "mocked" }));', + 'const { describeFlavor } = await import("./mid.js");', + 'it("sees the mocked module through its importer", () => {', + ' expect(describeFlavor()).toBe("flavor:mocked");', + "});", + "", + ].join("\n"), + ); + await write( + "vitest.config.ts", + [ + 'import { defineConfig } from "vitest/config";', + 'import { BaseSequencer } from "vitest/node";', + "// Alphabetical order keeps a-crash collected before b-mock regardless of", + "// the duration cache; the leak only reproduces in that order.", + "class AlphabeticalSequencer extends BaseSequencer {", + ' override async sort(files: Parameters[0]) {', + " return [...files].sort((a, b) => a.moduleId.localeCompare(b.moduleId));", + " }", + "}", + "export default defineConfig({", + ` cacheDir: ${JSON.stringify(path.join(root, ".vite"))},`, + " test: {", + " isolate: false,", + " fileParallelism: false,", + " maxWorkers: 1,", + " sequence: { sequencer: AlphabeticalSequencer },", + ` runner: ${JSON.stringify(path.join(repoRoot, "test", "non-isolated-runner.ts"))},`, + " },", + "});", + "", + ].join("\n"), + ); + + const vitestEntry = path.join(repoRoot, "node_modules", "vitest", "vitest.mjs"); + const result = await execFileAsync( + process.execPath, + [vitestEntry, "run", "--root", root, "--config", path.join(root, "vitest.config.ts")], + { cwd: repoRoot, env: childEnv(), maxBuffer: 16 * 1024 * 1024 }, + ).catch((error: unknown) => error as { stdout?: string; stderr?: string }); + const output = `${result.stdout ?? ""}\n${result.stderr ?? ""}`; + + // The crash file must fail collection first, and the mock file must still + // pass; a poisoned module cache turns b-mock into the second failure. + expect(output).toContain("synthetic collect failure"); + expect(output).toContain("1 failed | 1 passed"); + } finally { + await fs.rm(root, { recursive: true, force: true }); + } +}); diff --git a/test/non-isolated-runner.ts b/test/non-isolated-runner.ts index 46c69705da5d..2926fde15821 100644 --- a/test/non-isolated-runner.ts +++ b/test/non-isolated-runner.ts @@ -1,7 +1,7 @@ // Non-isolated runner helps execute tests without Vitest isolation. import fs from "node:fs"; import path from "node:path"; -import { TestRunner, type RunnerTask, type RunnerTestFile, type RunnerTestSuite, vi } from "vitest"; +import { TestRunner, type RunnerTask, type RunnerTestFile, vi } from "vitest"; type EvaluatedModuleNode = { promise?: unknown; @@ -251,15 +251,24 @@ export default class OpenClawNonIsolatedRunner extends TestRunner { super.onBeforeTryTask(test); } - override async onAfterRunSuite(suite: RunnerTestSuite) { - await super.onAfterRunSuite(suite); - if (this.config.isolate || !("filepath" in suite) || typeof suite.filepath !== "string") { + // Cross-file cleanup lives in onAfterRunFiles, not onAfterRunSuite: vitest + // early-returns runSuite for files that failed during collection (and for + // skipped file suites) without firing onAfterRunSuite, which used to leave + // the crashed file's evaluated real modules cached in the shared worker so + // the next file's vi.mock factories silently never applied. The worker loop + // calls startTests per file, so this hook runs after every file regardless + // of its collect/run outcome. + override onAfterRunFiles(files?: RunnerTestFile[]) { + super.onAfterRunFiles(); + if (this.config.isolate) { return; } const orderLogPath = process.env.OPENCLAW_VITEST_FILE_ORDER_LOG?.trim(); if (orderLogPath) { - fs.appendFileSync(orderLogPath, `END ${suite.filepath}\n`); + for (const file of files ?? []) { + fs.appendFileSync(orderLogPath, `END ${file.filepath}\n`); + } } // Mirror the missing cleanup from Vitest isolate mode so shared workers do diff --git a/test/scripts/tsdown-build.test.ts b/test/scripts/tsdown-build.test.ts index 6b993ab2afdb..37c50603d061 100644 --- a/test/scripts/tsdown-build.test.ts +++ b/test/scripts/tsdown-build.test.ts @@ -71,6 +71,23 @@ async function waitForFile(filePath: string, timeoutMs: number): Promise { throw new Error(`timed out waiting for ${filePath}`); } +// Pid files are written with plain writeFileSync, so an existence poll can +// observe the open-truncate 0-byte window and parse NaN (the #109140 flake +// class). Wait until the content parses to a real pid, not just for the file. +async function waitForPidFile(filePath: string, timeoutMs: number): Promise { + const deadlineAt = Date.now() + timeoutMs; + while (Date.now() < deadlineAt) { + if (fs.existsSync(filePath)) { + const pid = Number.parseInt(fs.readFileSync(filePath, "utf8"), 10); + if (Number.isInteger(pid) && pid > 0) { + return pid; + } + } + await sleep(5); + } + throw new Error(`timed out waiting for pid in ${filePath}`); +} + async function waitForDead(pid: number, timeoutMs: number): Promise { const deadlineAt = Date.now() + timeoutMs; while (Date.now() < deadlineAt) { @@ -909,8 +926,7 @@ describe("runTsdownBuildInvocation", () => { }, ); - await waitForFile(childPidPath, timeoutMs); - childPid = Number.parseInt(fs.readFileSync(childPidPath, "utf8"), 10); + childPid = await waitForPidFile(childPidPath, timeoutMs); expect(isProcessAlive(childPid)).toBe(true); const result = await runPromise; @@ -976,7 +992,7 @@ describe("runTsdownBuildInvocation", () => { ); await waitForFile(readyPath, 2_000); - childPid = Number.parseInt(fs.readFileSync(childPidPath, "utf8"), 10); + childPid = await waitForPidFile(childPidPath, 2_000); const result = await runPromise; expect(result.timedOut).toBe(true); @@ -1029,8 +1045,7 @@ describe("runTsdownBuildInvocation", () => { }); await waitForFile(readyPath, 2_000); - await waitForFile(childPidPath, 2_000); - childPid = Number.parseInt(fs.readFileSync(childPidPath, "utf8"), 10); + childPid = await waitForPidFile(childPidPath, 2_000); expect(isProcessAlive(childPid)).toBe(true); runner.kill("SIGTERM"); diff --git a/ui/src/pages/chat/chat-responsive.browser.test.ts b/ui/src/pages/chat/chat-responsive.browser.test.ts index 1099e72447ec..4f25435d9b26 100644 --- a/ui/src/pages/chat/chat-responsive.browser.test.ts +++ b/ui/src/pages/chat/chat-responsive.browser.test.ts @@ -20,6 +20,11 @@ const VIEWPORTS = [ [1440, 900], ] as const; const TOUCH_TARGET_MIN_PX = 43.5; +// Real-app cases boot through a cold Vite dev server that transforms the whole +// Control UI module graph on first request; with 6 Vitest workers sharing an +// 8vCPU CI runner that first render can starve well past 10s. Budget the +// first-render waits for contention while staying inside the 60s testTimeout. +const APP_FIRST_RENDER_TIMEOUT_MS = 30_000; const LONG_SIDE_CHAT_BODY = Array.from( { length: 80 }, (_, index) => `

Line ${index + 1}: keep the complete side result readable.

`, @@ -617,7 +622,9 @@ describeBrowserLayout.concurrent("chat responsive browser layout", () => { ], }); await page.goto(`${realChatServer.baseUrl}chat`); - await page.getByText("Context hover regression fixture.").waitFor({ timeout: 10_000 }); + await page + .getByText("Context hover regression fixture.") + .waitFor({ timeout: APP_FIRST_RENDER_TIMEOUT_MS }); const details = page.locator("details.msg-meta"); const context = page.locator(".msg-meta__details"); @@ -635,7 +642,9 @@ describeBrowserLayout.concurrent("chat responsive browser layout", () => { // the group is hovered, so enter through the message body first. await page.locator(".chat-text").first().hover(); await page.locator(".msg-meta__summary").hover(); - expect(await context.isVisible()).toBe(true); + // The reveal is state-driven, so the re-render can lag the hover event + // under CPU contention; poll instead of a one-shot visibility read. + await context.waitFor({ state: "visible", timeout: 10_000 }); const hoverLayout = await page.evaluate(() => { const footer = document.querySelector(".chat-group-footer")!; const group = document.querySelector(".chat-group")!; @@ -653,13 +662,14 @@ describeBrowserLayout.concurrent("chat responsive browser layout", () => { expect(hoverLayout.contextBottom).toBeLessThanOrEqual(hoverLayout.summaryTop + 4); await page.mouse.move(0, 0); - expect(await context.isVisible()).toBe(false); + await context.waitFor({ state: "hidden", timeout: 10_000 }); await page.locator(".chat-text").first().hover(); await page.locator(".msg-meta__summary").click(); await page.mouse.move(0, 0); + // Click-to-open must survive the pointer leaving the message group. + await context.waitFor({ state: "visible", timeout: 10_000 }); expect(await details.getAttribute("open")).toBe(""); - expect(await context.isVisible()).toBe(true); } finally { await closeBrowserPage(page); } @@ -694,7 +704,9 @@ describeBrowserLayout.concurrent("chat responsive browser layout", () => { const image = page.locator("img.chat-message-image"); const video = page.locator("video"); - await image.waitFor({ timeout: 10_000 }); + // First wait absorbs the cold-app render; both elements land in the same + // history render pass, so the video follows immediately after. + await image.waitFor({ timeout: APP_FIRST_RENDER_TIMEOUT_MS }); await video.waitFor({ timeout: 10_000 }); expect(await image.getAttribute("src")).toBe(imageUrl); expect(await video.getAttribute("src")).toBe(videoUrl); @@ -1886,7 +1898,7 @@ describeBrowserLayout.concurrent("chat responsive browser layout", () => { await page.goto(`${realChatServer.baseUrl}chat`); await page .getByText("Short landscape slash command keyboard regression fixture.") - .waitFor({ timeout: 10_000 }); + .waitFor({ timeout: APP_FIRST_RENDER_TIMEOUT_MS }); const textarea = page.locator(".agent-chat__composer-combobox > textarea"); await textarea.fill("/"); await textarea.focus();