Merge pull request #7362 from paperclipai/pap-10195-dev-runner-race

[codex] Fix dev runner snapshot race

scripts/dev-runner-snapshot.mjs

@@ -0,0 +1,100 @@
+import { existsSync, readdirSync, statSync } from "node:fs";
+import path from "node:path";
+import { shouldTrackDevServerPath } from "./dev-runner-paths.mjs";
+
+const defaultFileSystem = {
+  existsSync,
+  readdirSync,
+  statSync,
+};
+
+export function isMissingPathError(error) {
+  return (
+    typeof error === "object" &&
+    error !== null &&
+    "code" in error &&
+    (error.code === "ENOENT" || error.code === "ENOTDIR")
+  );
+}
+
+function toRelativePath(repoRoot, absolutePath) {
+  return path.relative(repoRoot, absolutePath).split(path.sep).join("/");
+}
+
+export function readSignature(absolutePath, fileSystem = defaultFileSystem) {
+  try {
+    const stats = fileSystem.statSync(absolutePath);
+    return `${Math.trunc(stats.mtimeMs)}:${stats.size}`;
+  } catch (error) {
+    if (isMissingPathError(error)) return null;
+    throw error;
+  }
+}
+
+export function addFileToSnapshot(snapshot, absolutePath, options) {
+  const relativePath = toRelativePath(options.repoRoot, absolutePath);
+  if (options.ignoredRelativePaths?.has(relativePath)) return;
+  if (!shouldTrackDevServerPath(relativePath)) return;
+
+  const signature = readSignature(absolutePath, options.fileSystem ?? defaultFileSystem);
+  if (signature === null) return;
+  snapshot.set(relativePath, signature);
+}
+
+export function walkDirectory(snapshot, absoluteDirectory, options) {
+  const fileSystem = options.fileSystem ?? defaultFileSystem;
+  if (!fileSystem.existsSync(absoluteDirectory)) return;
+
+  let entries;
+  try {
+    entries = fileSystem.readdirSync(absoluteDirectory, { withFileTypes: true });
+  } catch (error) {
+    if (isMissingPathError(error)) return;
+    throw error;
+  }
+
+  for (const entry of entries) {
+    if (options.ignoredDirectoryNames?.has(entry.name)) continue;
+
+    const absolutePath = path.join(absoluteDirectory, entry.name);
+    if (entry.isDirectory()) {
+      walkDirectory(snapshot, absolutePath, options);
+      continue;
+    }
+    if (entry.isFile() || entry.isSymbolicLink()) {
+      addFileToSnapshot(snapshot, absolutePath, options);
+    }
+  }
+}
+
+export function collectWatchedSnapshot(options) {
+  const fileSystem = options.fileSystem ?? defaultFileSystem;
+  const snapshot = new Map();
+
+  for (const absoluteDirectory of options.watchedDirectories) {
+    walkDirectory(snapshot, absoluteDirectory, options);
+  }
+  for (const absoluteFile of options.watchedFiles) {
+    if (!fileSystem.existsSync(absoluteFile)) continue;
+    addFileToSnapshot(snapshot, absoluteFile, options);
+  }
+
+  return snapshot;
+}
+
+export function diffSnapshots(previous, next) {
+  const changed = new Set();
+
+  for (const [relativePath, signature] of next) {
+    if (previous.get(relativePath) !== signature) {
+      changed.add(relativePath);
+    }
+  }
+  for (const relativePath of previous.keys()) {
+    if (!next.has(relativePath)) {
+      changed.add(relativePath);
+    }
+  }
+
+  return [...changed].sort();
+}

scripts/dev-runner.ts

@@ -1,12 +1,12 @@
 #!/usr/bin/env -S node --import tsx
 import { spawn } from "node:child_process";
 import { randomUUID } from "node:crypto";
-import { existsSync, mkdirSync, readdirSync, rmSync, statSync, writeFileSync } from "node:fs";
+import { existsSync, mkdirSync, rmSync, writeFileSync } from "node:fs";
 import path from "node:path";
 import { createInterface } from "node:readline/promises";
 import { stdin, stdout } from "node:process";
 import { createCapturedOutputBuffer, parseJsonResponseWithLimit } from "./dev-runner-output.ts";
-import { shouldTrackDevServerPath } from "./dev-runner-paths.mjs";
+import { collectWatchedSnapshot as collectDevServerWatchedSnapshot, diffSnapshots } from "./dev-runner-snapshot.mjs";
 import { createDevServiceIdentity, repoRoot } from "./dev-service-profile.ts";
 import { bootstrapDevRunnerWorktreeEnv } from "../server/src/dev-runner-worktree.ts";
 import {
@@ -261,68 +261,14 @@ function exitForSignal(signal: NodeJS.Signals) {
   process.exit(1);
 }
 
-function toRelativePath(absolutePath: string) {
-  return path.relative(repoRoot, absolutePath).split(path.sep).join("/");
-}
-
-function readSignature(absolutePath: string) {
-  const stats = statSync(absolutePath);
-  return `${Math.trunc(stats.mtimeMs)}:${stats.size}`;
-}
-
-function addFileToSnapshot(snapshot: Map<string, string>, absolutePath: string) {
-  const relativePath = toRelativePath(absolutePath);
-  if (ignoredRelativePaths.has(relativePath)) return;
-  if (!shouldTrackDevServerPath(relativePath)) return;
-  snapshot.set(relativePath, readSignature(absolutePath));
-}
-
-function walkDirectory(snapshot: Map<string, string>, absoluteDirectory: string) {
-  if (!existsSync(absoluteDirectory)) return;
-
-  for (const entry of readdirSync(absoluteDirectory, { withFileTypes: true })) {
-    if (ignoredDirectoryNames.has(entry.name)) continue;
-
-    const absolutePath = path.join(absoluteDirectory, entry.name);
-    if (entry.isDirectory()) {
-      walkDirectory(snapshot, absolutePath);
-      continue;
-    }
-    if (entry.isFile() || entry.isSymbolicLink()) {
-      addFileToSnapshot(snapshot, absolutePath);
-    }
-  }
-}
-
 function collectWatchedSnapshot() {
-  const snapshot = new Map<string, string>();
+  return collectDevServerWatchedSnapshot({
-
+    repoRoot,
-  for (const absoluteDirectory of watchedDirectories) {
+    watchedDirectories,
-    walkDirectory(snapshot, absoluteDirectory);
+    watchedFiles,
-  }
+    ignoredDirectoryNames,
-  for (const absoluteFile of watchedFiles) {
+    ignoredRelativePaths,
-    if (!existsSync(absoluteFile)) continue;
+  }) as Map<string, string>;
-    addFileToSnapshot(snapshot, absoluteFile);
-  }
-
-  return snapshot;
-}
-
-function diffSnapshots(previous: Map<string, string>, next: Map<string, string>) {
-  const changed = new Set<string>();
-
-  for (const [relativePath, signature] of next) {
-    if (previous.get(relativePath) !== signature) {
-      changed.add(relativePath);
-    }
-  }
-  for (const relativePath of previous.keys()) {
-    if (!next.has(relativePath)) {
-      changed.add(relativePath);
-    }
-  }
-
-  return [...changed].sort();
 }
 
 function ensureDevStatusDirectory() {

server/src/__tests__/dev-runner-snapshot.test.ts

@@ -0,0 +1,101 @@
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+import { afterEach, describe, expect, it } from "vitest";
+import {
+  collectWatchedSnapshot,
+  diffSnapshots,
+  readSignature,
+} from "../../../scripts/dev-runner-snapshot.mjs";
+
+const tempRoots = new Set<string>();
+
+afterEach(() => {
+  for (const root of tempRoots) {
+    fs.rmSync(root, { recursive: true, force: true });
+  }
+  tempRoots.clear();
+});
+
+function createTempRoot(prefix: string): string {
+  const root = fs.mkdtempSync(path.join(os.tmpdir(), prefix));
+  tempRoots.add(root);
+  return root;
+}
+
+function createSnapshotOptions(root: string) {
+  return {
+    repoRoot: root,
+    watchedDirectories: [path.join(root, "server")],
+    watchedFiles: [path.join(root, "package.json")],
+    ignoredDirectoryNames: new Set(["node_modules"]),
+    ignoredRelativePaths: new Set([".paperclip/dev-server-status.json"]),
+  };
+}
+
+describe("dev-runner watched snapshot", () => {
+  it("skips files that disappear between directory listing and stat", () => {
+    const root = createTempRoot("paperclip-dev-runner-snapshot-file-race-");
+    const serverDir = path.join(root, "server");
+    const sourcePath = path.join(serverDir, "index.ts");
+    fs.mkdirSync(serverDir, { recursive: true });
+    fs.writeFileSync(sourcePath, "console.log('boot');\n", "utf8");
+
+    const fileSystem = {
+      existsSync: fs.existsSync,
+      readdirSync: fs.readdirSync,
+      statSync(target: fs.PathLike, options?: fs.StatOptions) {
+        if (target === sourcePath) {
+          fs.rmSync(sourcePath, { force: true });
+        }
+        return fs.statSync(target, options);
+      },
+    };
+
+    const snapshot = collectWatchedSnapshot({ ...createSnapshotOptions(root), fileSystem });
+
+    expect(snapshot.has("server/index.ts")).toBe(false);
+  });
+
+  it("skips directories that disappear before they can be read", () => {
+    const root = createTempRoot("paperclip-dev-runner-snapshot-dir-race-");
+    const serverDir = path.join(root, "server");
+    const routesDir = path.join(serverDir, "routes");
+    fs.mkdirSync(routesDir, { recursive: true });
+    fs.writeFileSync(path.join(routesDir, "health.ts"), "export const ok = true;\n", "utf8");
+
+    const fileSystem = {
+      existsSync: fs.existsSync,
+      readdirSync(target: fs.PathLike, options?: fs.ObjectEncodingOptions & { withFileTypes: true }) {
+        if (target === routesDir) {
+          fs.rmSync(routesDir, { recursive: true, force: true });
+        }
+        return fs.readdirSync(target, options);
+      },
+      statSync: fs.statSync,
+    };
+
+    const snapshot = collectWatchedSnapshot({ ...createSnapshotOptions(root), fileSystem });
+
+    expect(snapshot.has("server/routes/health.ts")).toBe(false);
+  });
+
+  it("returns null for missing file signatures and not-directory races", () => {
+    const root = createTempRoot("paperclip-dev-runner-snapshot-diff-");
+    const missingPath = path.join(root, "server", "deleted.ts");
+    const fileSystem = {
+      statSync() {
+        const error = new Error("not a directory") as NodeJS.ErrnoException;
+        error.code = "ENOTDIR";
+        throw error;
+      },
+    };
+
+    expect(readSignature(missingPath)).toBeNull();
+    expect(readSignature(missingPath, fileSystem)).toBeNull();
+  });
+
+  it("reports deleted paths in diffs", () => {
+    expect(diffSnapshots(new Map([["server/deleted.ts", "1:1"]]), new Map())).toEqual(["server/deleted.ts"]);
+  });
+});