From 33f235dd620809622a151ddd038c6b738b5e6458 Mon Sep 17 00:00:00 2001
From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
Date: Sat, 18 Jul 2026 12:36:56 -0700
Subject: [PATCH] fix: reserve design variant destinations atomically

Destination labels were derived from the manifest's attempt count before
generation, so two concurrent runs could pick the same
variant-iteration-<label> path and overwrite each other's variant.

Claim each destination atomically with an exclusive create (open "wx"),
advancing to the next label on EEXIST, and guard the round-manifest
read-modify-write with a lock file plus a temp-file + rename so
concurrent updates don't drop an entry. Add concurrency and
failure/restart tests asserting distinct labels and that no variant is
overwritten.
---
 design/src/cli.ts                        | 94 ++++++++++++++++++------
 design/test/variant-preservation.test.ts | 35 +++++++++
 2 files changed, 108 insertions(+), 21 deletions(-)

diff --git a/design/src/cli.ts b/design/src/cli.ts
index 046801c0a..6b0207622 100644
--- a/design/src/cli.ts
+++ b/design/src/cli.ts
@@ -321,12 +321,15 @@ async function main(argv = process.argv): Promise<void> {
 }
 
 const ROUND_MANIFEST = ".gstack-design-rounds.json";
+const ROUND_MANIFEST_LOCK = ".gstack-design-rounds.lock";
+const ROUND_RESERVATION_LIMIT = 1000;
 
 interface RoundAttempt {
   label: string;
   path: string;
   success: boolean;
   error?: string;
+  reserved?: boolean;
 }
 
 type RoundManifest = Record<string, RoundAttempt[]>;
@@ -368,7 +371,37 @@ function readRoundManifest(dir: string): RoundManifest {
 
 function writeRoundManifest(dir: string, manifest: RoundManifest): void {
   fs.mkdirSync(dir, { recursive: true });
-  fs.writeFileSync(path.join(dir, ROUND_MANIFEST), JSON.stringify(manifest, null, 2));
+  const manifestPath = path.join(dir, ROUND_MANIFEST);
+  const tempPath = path.join(dir, `${ROUND_MANIFEST}.${process.pid}.${Date.now()}.tmp`);
+  fs.writeFileSync(tempPath, JSON.stringify(manifest, null, 2));
+  fs.renameSync(tempPath, manifestPath);
+}
+
+function withRoundManifestLock<T>(dir: string, action: () => T): T {
+  fs.mkdirSync(dir, { recursive: true });
+  const lockPath = path.join(dir, ROUND_MANIFEST_LOCK);
+  let lockFd: number | undefined;
+
+  for (let attempt = 0; attempt < ROUND_RESERVATION_LIMIT; attempt++) {
+    try {
+      lockFd = fs.openSync(lockPath, "wx");
+      break;
+    } catch (err: any) {
+      if (err.code !== "EEXIST") throw err;
+      Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 1);
+    }
+  }
+
+  if (lockFd === undefined) {
+    throw new Error(`Timed out reserving design round artifacts in ${dir}`);
+  }
+
+  try {
+    return action();
+  } finally {
+    fs.closeSync(lockFd);
+    fs.unlinkSync(lockPath);
+  }
 }
 
 function planRoundArtifacts(outputPath: string): RoundArtifactPlan | null {
@@ -376,32 +409,50 @@ function planRoundArtifacts(outputPath: string): RoundArtifactPlan | null {
   if (!baseName) return null;
 
   const dir = path.dirname(outputPath);
-  const manifest = readRoundManifest(dir);
   const key = roundKey(outputPath, baseName);
-  const attempts = manifest[key] || [];
-  const label = labelForIndex(attempts.length);
+  return withRoundManifestLock(dir, () => {
+    const manifest = readRoundManifest(dir);
+    const attempts = manifest[key] || [];
 
-  return {
-    aliasOutput: outputPath,
-    primaryOutput: path.join(dir, `${baseName}-${label}.png`),
-    roundKey: key,
-    label,
-  };
+    for (let index = attempts.length; index < attempts.length + ROUND_RESERVATION_LIMIT; index++) {
+      const label = labelForIndex(index);
+      const primaryOutput = path.join(dir, `${baseName}-${label}.png`);
+      try {
+        fs.closeSync(fs.openSync(primaryOutput, "wx"));
+      } catch (err: any) {
+        if (err.code === "EEXIST") continue;
+        throw err;
+      }
+
+      attempts.push({ label, path: primaryOutput, success: false, reserved: true });
+      manifest[key] = attempts;
+      writeRoundManifest(dir, manifest);
+      return { aliasOutput: outputPath, primaryOutput, roundKey: key, label };
+    }
+
+    throw new Error(`Unable to reserve a design round artifact after ${ROUND_RESERVATION_LIMIT} attempts`);
+  });
 }
 
 function recordRoundAttempt(plan: RoundArtifactPlan, success: boolean, error?: string): void {
   const dir = path.dirname(plan.aliasOutput);
-  const manifest = readRoundManifest(dir);
-  const attempts = manifest[plan.roundKey] || [];
-  const existing = attempts.find(attempt => attempt.label === plan.label);
-  const attempt = { label: plan.label, path: plan.primaryOutput, success, error };
-  if (existing) {
-    Object.assign(existing, attempt);
-  } else {
-    attempts.push(attempt);
-  }
-  manifest[plan.roundKey] = attempts;
-  writeRoundManifest(dir, manifest);
+  withRoundManifestLock(dir, () => {
+    const manifest = readRoundManifest(dir);
+    const attempts = manifest[plan.roundKey] || [];
+    const existing = attempts.find(attempt => attempt.label === plan.label);
+    const attempt = { label: plan.label, path: plan.primaryOutput, success, error };
+    if (existing) {
+      Object.assign(existing, attempt);
+    } else {
+      attempts.push(attempt);
+    }
+    manifest[plan.roundKey] = attempts;
+    writeRoundManifest(dir, manifest);
+
+    if (!success && fs.existsSync(plan.primaryOutput) && fs.statSync(plan.primaryOutput).size === 0) {
+      fs.unlinkSync(plan.primaryOutput);
+    }
+  });
 }
 
 function copyRoundAlias(plan: RoundArtifactPlan): void {
@@ -630,5 +681,6 @@ export {
   main,
   resolveImagePaths,
   planRoundArtifacts,
+  recordRoundAttempt,
   resolveRoundImageAlias,
 };
diff --git a/design/test/variant-preservation.test.ts b/design/test/variant-preservation.test.ts
index da9ed79cd..fc53d9993 100644
--- a/design/test/variant-preservation.test.ts
+++ b/design/test/variant-preservation.test.ts
@@ -4,6 +4,7 @@ import os from "os";
 import path from "path";
 import {
   planRoundArtifacts,
+  recordRoundAttempt,
   resolveImagePaths,
 } from "../src/cli";
 
@@ -51,6 +52,40 @@ describe("plan-design-review round variant preservation", () => {
     expect(second?.primaryOutput).toBe(path.join(tmpDir, "variant-recommended-B.png"));
   });
 
+  test("concurrent round reservations claim distinct paths and preserve every variant", async () => {
+    const alias = path.join(tmpDir, "variant-recommended.png");
+    const plans = await Promise.all(
+      Array.from({ length: 3 }, async () => {
+        const plan = planRoundArtifacts(alias)!;
+        await Promise.resolve();
+        writePng(plan.primaryOutput);
+        recordRoundAttempt(plan, true);
+        return plan;
+      }),
+    );
+
+    expect(plans.map(plan => plan.label).sort()).toEqual(["A", "B", "C"]);
+    expect(new Set(plans.map(plan => plan.primaryOutput)).size).toBe(3);
+    expect(plans.every(plan => fs.existsSync(plan.primaryOutput))).toBe(true);
+    await expect(resolveImagePaths(alias)).resolves.toEqual(
+      plans.map(plan => plan.primaryOutput),
+    );
+  });
+
+  test("failed reservations are recorded without blocking a later variant", () => {
+    const alias = path.join(tmpDir, "variant-recommended.png");
+    const failed = planRoundArtifacts(alias)!;
+    recordRoundAttempt(failed, false, "API error");
+
+    const retry = planRoundArtifacts(alias)!;
+    writePng(retry.primaryOutput);
+    recordRoundAttempt(retry, true);
+
+    expect(retry.label).toBe("B");
+    expect(fs.existsSync(failed.primaryOutput)).toBe(false);
+    expect(fs.existsSync(retry.primaryOutput)).toBe(true);
+  });
+
   test("recommended round alias expands to every successful generated candidate", async () => {
     const alias = path.join(tmpDir, "variant-recommended.png");
     const variantA = path.join(tmpDir, "variant-recommended-A.png");