Merge pull request #1324 from corbitsdev/cl-9892-latest-race

fix(session): make latest symlink update race-safe

Author: TheGreatAxios

src/session/index.ts

@@ -22,15 +22,6 @@ import { LOG_NAMESPACE_ROOT } from "../branding.js";
 
 const log = getLogger([LOG_NAMESPACE_ROOT, "session"]);
 
-function isENOENT(err: unknown): boolean {
-  return (
-    typeof err === "object" &&
-    err !== null &&
-    "code" in err &&
-    (err as { code?: unknown }).code === "ENOENT"
-  );
-}
-
 // ---------------------------------------------------------------------------
 // UUIDv7 generator (no external dependencies)
 // ---------------------------------------------------------------------------
@@ -179,19 +170,25 @@ export async function initSessionDir(
   const dir = await migrateLegacySessionIfNeeded(cwd, sessionId, home);
   await mkdir(join(dir, "context"), { recursive: true });
 
-  // Update the `latest` symlink to point to this session.
-  const linkPath = latestSymlinkPath(cwd, home);
-  await mkdir(dirname(linkPath), { recursive: true });
-
-  // Remove existing symlink first, then create new one.
-  await unlink(linkPath).catch((err: unknown) => {
-    if (isENOENT(err)) return;
-    log.debug("failed to replace latest symlink at {path}: {error}", {
-      path: linkPath,
+  // Best-effort `latest` symlink update. A temp symlink plus atomic rename
+  // keeps concurrent inits from racing on EEXIST; failures here must never
+  // abort session creation, so every error is swallowed below.
+  try {
+    const linkPath = latestSymlinkPath(cwd, home);
+    await mkdir(dirname(linkPath), { recursive: true });
+    const tmpPath = `${linkPath}.${process.pid}.${Math.random().toString(16).slice(2)}.tmp`;
+    try {
+      await symlink(sessionId, tmpPath);
+      await rename(tmpPath, linkPath);
+    } catch (err: unknown) {
+      await unlink(tmpPath).catch(() => undefined);
+      throw err;
+    }
+  } catch (err: unknown) {
+    log.debug("failed to update latest symlink: {error}", {
       error: err instanceof Error ? err.message : String(err),
     });
-  });
-  await symlink(sessionId, linkPath);
+  }
 
   return dir;
 }

src/session/session-dir.test.ts

@@ -1,7 +1,14 @@
 import { afterEach, beforeEach, expect, test } from "bun:test";
-import { mkdir, readFile, rm, writeFile } from "node:fs/promises";
+import {
+  mkdir,
+  readFile,
+  readdir,
+  readlink,
+  rm,
+  writeFile,
+} from "node:fs/promises";
 import { existsSync } from "node:fs";
-import { join } from "node:path";
+import { dirname, join } from "node:path";
 import { tmpdir } from "node:os";
 import { execFileSync } from "node:child_process";
 
@@ -82,6 +89,26 @@ test("listSessions finds legacy sessions and migrates them", async () => {
   expect(existsSync(legacy)).toBe(false);
 });
 
+test("concurrent initSessionDir does not throw on latest symlink race", async () => {
+  const ids = Array.from({ length: 16 }, () => generateSessionId());
+  const dirs = await Promise.all(
+    ids.map((id) => initSessionDir(cwd, id, home)),
+  );
+  for (const [i, dir] of dirs.entries()) {
+    expect(dir).toBe(sessionDir(cwd, ids[i] as string, home));
+    expect(existsSync(join(dir, "context"))).toBe(true);
+  }
+  // The winning `latest` symlink must resolve to one of the raced ids.
+  const projectRoot = dirname(dirs[0] as string);
+  const latestTarget = await readlink(join(projectRoot, "latest"));
+  expect(ids).toContain(latestTarget);
+  // No temp-symlink strays (`latest.<pid>.<rand>.tmp`) may be left behind.
+  const entries = await readdir(projectRoot);
+  expect(
+    entries.filter((e) => e.startsWith("latest.") && e.endsWith(".tmp")),
+  ).toEqual([]);
+});
+
 test("migrateLegacySessionIfNeeded does not migrate main-repo .agent-state from a worktree cwd", async () => {
   const main = join(cwd, "main");
   await mkdir(main, { recursive: true });