test: stop the src/node_modules race between platform-binary and the source walk

packages/cli/test/api-deprecated-paths.test.ts

@@ -1,5 +1,5 @@
 import { readFile, readdir } from "node:fs/promises";
-import { join, dirname, relative, sep } from "node:path";
+import { join, dirname, relative } from "node:path";
 import { fileURLToPath } from "node:url";
 
 import { describe, it, expect } from "vitest";
@@ -48,23 +48,63 @@ function resourcePrefix(path: string): string {
   return prefix.replace(/\/+$/, "");
 }
 
+/** Whether `error` is a filesystem "no such file or directory". */
+function isMissing(error: unknown): boolean {
+  return (error as NodeJS.ErrnoException | undefined)?.code === "ENOENT";
+}
+
+/**
+ * Every `.ts` file under `directory`, walked by hand rather than with
+ * `readdir({ recursive: true })` so that two kinds of directory can be pruned
+ * before the walk descends into them:
+ *
+ * - `node_modules`, which is never source. A recursive `readdir` cannot skip
+ *   it, only filter its contents afterwards, and it lists a directory and then
+ *   descends into it as separate steps. When a parallel test created and
+ *   removed `src/node_modules` between those steps, the walk threw `ENOENT`
+ *   (taskless/cli#694).
+ * - `generated/`, the schema's own transcript: it necessarily names every
+ *   path, deprecated ones included, and is not a call site. Matched at any
+ *   depth, because that reasoning is about what a generated directory IS and
+ *   does not depend on where it sits.
+ *
+ * A directory that vanishes between being listed and being read is skipped
+ * rather than failing the walk: something that no longer exists cannot
+ * reference a deprecated path.
+ */
 async function typeScriptSources(directory: string): Promise<string[]> {
-  const entries = await readdir(directory, {
-    recursive: true,
-    withFileTypes: true,
-  });
-  return entries
-    .filter((entry) => entry.isFile() && entry.name.endsWith(".ts"))
-    .map((entry) => join(entry.parentPath, entry.name))
-    .filter(
-      // `generated/` is the schema's own transcript: it necessarily names every
-      // path, deprecated ones included, and is not a call site. Matched at any
-      // depth, because that reasoning is about what a generated directory IS
-      // and does not depend on where it sits. `startsWith` reached only a
-      // top-level `src/generated`, which is a no-op today and would have
-      // reported a nested one as a wall of false offenders.
-      (file) => !relative(directory, file).split(sep).includes("generated")
-    );
+  let entries;
+  try {
+    entries = await readdir(directory, { withFileTypes: true });
+  } catch (error) {
+    if (isMissing(error)) return [];
+    throw error;
+  }
+
+  const files: string[] = [];
+  for (const entry of entries) {
+    const path = join(directory, entry.name);
+    if (entry.isDirectory()) {
+      if (entry.name === "node_modules" || entry.name === "generated") continue;
+      files.push(...(await typeScriptSources(path)));
+    } else if (entry.isFile() && entry.name.endsWith(".ts")) {
+      files.push(path);
+    }
+  }
+  return files;
+}
+
+/**
+ * The file's contents, or `undefined` when it was removed after the walk
+ * listed it, for the same reason the walk tolerates a vanished directory.
+ */
+async function readSource(file: string): Promise<string | undefined> {
+  try {
+    return await readFile(file, "utf8");
+  } catch (error) {
+    if (isMissing(error)) return undefined;
+    throw error;
+  }
 }
 
 describe("deprecated API paths", () => {
@@ -88,7 +128,8 @@ describe("deprecated API paths", () => {
 
     const offenders: string[] = [];
     for (const file of await typeScriptSources(SOURCE_ROOT)) {
-      const source = await readFile(file, "utf8");
+      const source = await readSource(file);
+      if (source === undefined) continue;
       for (const prefix of deprecated) {
         // The negative lookahead keeps `/cli/api/rule` from matching
         // `/cli/api/rule-hash-vectors`, a live path that merely shares a stem.
@@ -122,7 +163,8 @@ describe("v1 API routes", () => {
     const pattern = /["'`}]\/cli\/api\/(?!v2\/)/;
     const offenders: string[] = [];
     for (const file of await typeScriptSources(SOURCE_ROOT)) {
-      const source = await readFile(file, "utf8");
+      const source = await readSource(file);
+      if (source === undefined) continue;
       for (const [index, line] of source.split("\n").entries()) {
         if (pattern.test(line)) {
           offenders.push(`${relative(SOURCE_ROOT, file)}:${String(index + 1)}`);

packages/cli/test/platform-binary.test.ts

@@ -9,7 +9,7 @@ import {
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 
-import { afterEach, describe, expect, it } from "vitest";
+import { afterEach, describe, expect, it, vi } from "vitest";
 
 import { AST_GREP_BINARY } from "../src/rules/ast-grep-binary";
 import {
@@ -20,6 +20,36 @@ import {
 } from "../src/rules/platform-binary";
 import { VALE_BINARY } from "../src/rules/vale/binary";
 
+/**
+ * Where the resolver believes its own module lives, when a test relocates it.
+ *
+ * `resolvePlatformBinary` finds its `node_modules/.bin` tier relative to its
+ * own file (`fileURLToPath(import.meta.url)`). Under vitest that file is
+ * `src/rules/platform-binary.ts`, so planting a binary at the real location
+ * meant creating `src/node_modules` inside the source tree, where a parallel
+ * test walking `src/` could list it and then hit ENOENT once it was removed
+ * (taskless/cli#694). Relocating the module path instead keeps the resolver's
+ * own `..`/`node_modules/.bin` computation under test while every file the
+ * test creates lives in a temp directory.
+ *
+ * Every other call passes straight through to the real `fileURLToPath`.
+ */
+const relocation = vi.hoisted(() => ({
+  modulePath: undefined as string | undefined,
+}));
+
+vi.mock("node:url", async (importOriginal) => {
+  const actual = await importOriginal<typeof import("node:url")>();
+  const fileURLToPath: typeof actual.fileURLToPath = (url, options) => {
+    const path = actual.fileURLToPath(url, options);
+    return relocation.modulePath !== undefined &&
+      /[/\\]rules[/\\]platform-binary\.ts$/.test(path)
+      ? relocation.modulePath
+      : path;
+  };
+  return { ...actual, default: { ...actual, fileURLToPath }, fileURLToPath };
+});
+
 /**
  * The two engines' packages are named differently, and the difference is not
  * cosmetic: ast-grep publishes a libc/toolchain suffix and Vale does not.
@@ -207,18 +237,14 @@ describe("candidate order", () => {
   onUnix(
     "reports node_modules/.bin as the source when that tier answers",
     () => {
-      // Mirrors the computation `resolvePlatformBinary` uses internally
-      // (relative to the compiled module's own directory) — this is the exact
-      // location that tier searches, not a stand-in for it.
-      const localBin = join(
-        import.meta.dirname,
-        "..",
-        "src",
-        "node_modules",
-        ".bin"
-      );
+      // Pretend the resolver's module sits at `<tmp>/rules/platform-binary.ts`,
+      // the same depth as `src/rules/platform-binary.ts`, so the tier it
+      // computes is `<tmp>/node_modules/.bin`. Nothing is written under `src/`.
+      const root = mkdtempSync(join(tmpdir(), "candidate-order-bin-"));
+      workspaces.push(root);
+      relocation.modulePath = join(root, "rules", "platform-binary.ts");
+      const localBin = join(root, "node_modules", ".bin");
       mkdirSync(localBin, { recursive: true });
-      workspaces.push(join(import.meta.dirname, "..", "src", "node_modules"));
       const binaryPath = join(localBin, "sg");
       writeFileSync(binaryPath, "#!/bin/sh\necho 'fake-tool 1.0.0'\n");
       chmodSync(binaryPath, 0o755);
@@ -233,6 +259,7 @@ describe("candidate order", () => {
         expect(resolution.source).toBe("node_modules/.bin");
       } finally {
         process.env.PATH = originalPath;
+        relocation.modulePath = undefined;
       }
     }
   );