From 359f6dedc270af7aec7d178b7a9859c64ad4cfe3 Mon Sep 17 00:00:00 2001 From: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Date: Tue, 27 Oct 2026 14:58:12 -0700 Subject: [PATCH] fix(task-cache): the hook test waits for every flusher it starts Outside GitHub Actions the run hook drains telemetry in a detached, unref'd flusher that creates its queue and lock under HOME even when there is nothing to send. The integration test ran most `vp` steps that way with HOME set to its temp root, so a late flusher could recreate directories while cleanup removed the root and fail with ENOTEMPTY. Run every integration step with inline flushing, so each `vp run` exits only after its telemetry is sent, and assert the report count after each step. Cover the detached path with an in-process test that collects the flusher from Node's child_process diagnostics channel, checks finish() returns before delivery, and awaits the flusher before cleanup. Co-Authored-By: Claude Opus 5.5 --- infra/task-cache/src/runHook.test.ts | 112 +++++++++++++++++++-------- 1 file changed, 80 insertions(+), 32 deletions(-) diff --git a/infra/task-cache/src/runHook.test.ts b/infra/task-cache/src/runHook.test.ts index 175092bd36a4f..95ea7ac4bb0b1 100644 --- a/infra/task-cache/src/runHook.test.ts +++ b/infra/task-cache/src/runHook.test.ts @@ -1,28 +1,17 @@ // @effect-diagnostics nodeBuiltinImport:off - exercises the real native runner and local HTTP collector. import * as NodeChildProcess from "node:child_process"; +import * as NodeDiagnosticsChannel from "node:diagnostics_channel"; import * as NodeFSP from "node:fs/promises"; import * as NodeHttp from "node:http"; import * as NodeOS from "node:os"; import * as NodePath from "node:path"; -import { expect, it } from "vite-plus/test"; +import { expect, it, vi } from "vite-plus/test"; +import startRunHook from "./runHook.ts"; it("uses one machine hook across fresh worktrees without modifying Vite+", async () => { const root = await NodeFSP.mkdtemp(NodePath.join(NodeOS.tmpdir(), "task-cache-hook-")); const unrelated = await NodeFSP.mkdtemp(NodePath.join(NodeOS.tmpdir(), "unrelated-project-")); const reports: { authorization: string | undefined; body: string }[] = []; - let firstExit!: () => void; - const firstExited = new Promise((resolve) => { - firstExit = resolve; - }); - let completedReports = 0; - let notified: (() => void) | undefined; - let firstDisconnected = false; - const report = (count: number) => - completedReports >= count - ? Promise.resolve() - : new Promise((resolve) => { - notified = resolve; - }); const server = NodeHttp.createServer(async (request, response) => { if (!request.url?.endsWith("/telemetry")) { response.writeHead(404).end(); @@ -31,14 +20,7 @@ it("uses one machine hook across fresh worktrees without modifying Vite+", async let body = ""; for await (const chunk of request) body += chunk; reports.push({ authorization: request.headers.authorization, body }); - if (reports.length === 1) { - await firstExited; - firstDisconnected = response.destroyed; - } response.writeHead(204).end(); - completedReports++; - notified?.(); - notified = undefined; }); try { await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); @@ -110,7 +92,7 @@ it("uses one machine hook across fresh worktrees without modifying Vite+", async }), ); const originalEntrypoint = await NodeFSP.readFile(entrypoint, "utf8"); - const run = (task: string, cwd = root, ci = false, cache = "--cache") => + const run = (task: string, cwd = root, cache = "--cache") => new Promise((resolve, reject) => { const child = NodeChildProcess.spawn( process.execPath, @@ -120,7 +102,8 @@ it("uses one machine hook across fresh worktrees without modifying Vite+", async env: { ...process.env, HOME: root, - GITHUB_ACTIONS: ci ? "true" : undefined, + // Flush inline so no detached flusher can write under root while it is removed. + GITHUB_ACTIONS: "true", T3_TASK_CACHE_TELEMETRY: undefined, T3_TASK_CACHE_HOST: undefined, VITE_CACHE_PATH: undefined, @@ -139,32 +122,30 @@ it("uses one machine hook across fresh worktrees without modifying Vite+", async }); child.once("error", reject); child.once("close", (code) => { - firstExit(); if (code !== 0 && task !== "fail") reject(new Error(stderr)); else resolve(code); }); }); expect(await run("build")).toBe(0); - await report(1); + expect(reports).toHaveLength(1); expect(await run("--last-details")).toBe(0); expect( JSON.parse(await NodeFSP.readFile(NodePath.join(root, "environment.json"), "utf8")), ).toMatchObject({ url: null }); - expect(firstDisconnected).toBe(false); expect(await run("build")).toBe(0); - await report(2); + expect(reports).toHaveLength(2); expect(await run("fail")).toBe(7); - await report(3); + expect(reports).toHaveLength(3); const secondWorktree = NodePath.join(root, "second-worktree"); await NodeFSP.mkdir(secondWorktree); for (const file of ["package.json", ".task-cache.json", "vite.config.ts", "build.cjs"]) await NodeFSP.copyFile(NodePath.join(root, file), NodePath.join(secondWorktree, file)); expect(await run("build", secondWorktree)).toBe(0); - await report(4); - expect(await run("build", secondWorktree, true)).toBe(0); - await report(5); + expect(reports).toHaveLength(4); + expect(await run("build", secondWorktree)).toBe(0); + expect(reports).toHaveLength(5); expect(await NodeFSP.readFile(entrypoint, "utf8")).toBe(originalEntrypoint); - expect(await run("nested", root, false, "--no-cache")).toBe(0); + expect(await run("nested", root, "--no-cache")).toBe(0); expect(await run("build", nested)).toBe(0); expect(reports).toHaveLength(5); expect( @@ -187,6 +168,73 @@ it("uses one machine hook across fresh worktrees without modifying Vite+", async } }); +it("sends telemetry from a detached flusher that the task runner does not wait for", async () => { + const root = await NodeFSP.mkdtemp(NodePath.join(NodeOS.tmpdir(), "task-cache-flusher-")); + const reports: string[] = []; + let finishReturned!: () => void; + const returned = new Promise((resolve) => { + finishReturned = resolve; + }); + let disconnected = false; + const server = NodeHttp.createServer(async (request, response) => { + let body = ""; + for await (const chunk of request) body += chunk; + reports.push(body); + await returned; + disconnected = response.destroyed; + response.writeHead(204).end(); + }); + // The hook detaches and unrefs its flusher, so only this channel lets cleanup wait for it. + const flushers: Promise[] = []; + const onSpawn = (message: unknown) => { + const { process: flusher } = message as { process: NodeChildProcess.ChildProcess }; + flushers.push(new Promise((resolve) => flusher.once("close", () => resolve()))); + }; + NodeDiagnosticsChannel.subscribe("child_process", onSpawn); + try { + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + const address = server.address(); + if (!address || typeof address === "string") throw new Error("Missing HTTP address"); + const endpoint = `http://127.0.0.1:${address.port}/projects/test`; + await NodeFSP.writeFile(NodePath.join(root, ".task-cache.json"), JSON.stringify({ endpoint })); + await NodeFSP.mkdir(NodePath.join(root, ".config/t3-task-cache"), { recursive: true }); + await NodeFSP.writeFile( + NodePath.join(root, ".config/t3-task-cache/credentials.json"), + JSON.stringify({ [endpoint]: { name: "maria", token: "secret" } }), + ); + vi.stubEnv("HOME", root); + vi.stubEnv("GITHUB_ACTIONS", undefined); + vi.stubEnv("T3_TASK_CACHE_TELEMETRY", undefined); + vi.stubEnv("VITE_CACHE_PATH", undefined); + const finish = startRunHook({ root }); + const summary = NodePath.join(root, "node_modules/.vite/task-cache/run/last-summary.json"); + await NodeFSP.mkdir(NodePath.dirname(summary), { recursive: true }); + await NodeFSP.writeFile( + summary, + JSON.stringify({ + tasks: [{ package_name: "fixture", task_name: "build", result: { Spawned: {} } }], + }), + ); + await finish?.(); + finishReturned(); + await Promise.all(flushers); + expect(flushers).toHaveLength(1); + expect(reports.map((report) => JSON.parse(report).tasks)).toEqual([ + [{ package: "fixture", task: "build", source: "executed", savedMs: 0 }], + ]); + expect(disconnected).toBe(false); + } finally { + finishReturned(); + await Promise.all(flushers); + NodeDiagnosticsChannel.unsubscribe("child_process", onSpawn); + vi.unstubAllEnvs(); + await new Promise((resolve, reject) => + server.close((error) => (error ? reject(error) : resolve())), + ); + await NodeFSP.rm(root, { recursive: true, force: true }); + } +}); + it("rejects invalid setup credentials before installing anything and never prints the token", async () => { const home = await NodeFSP.mkdtemp(NodePath.join(NodeOS.tmpdir(), "task-cache-setup-")); const token = "private-credential-that-must-not-appear-in-output";