fix(project): race-safe batch scene preview import and close

Serialize updateProject and preview finalize, drain before pack/close, buffer zip entries, and abort finalize cleanly when leaving the project to avoid ENOENT on multi-image drops. Co-authored-by: Cursor <cursoragent@cursor.com>
2026-08-04 09:44:45 +08:00
parent f75444a5dd
commit 4e6f7321b8
5 changed files with 210 additions and 59 deletions
@@ -33,3 +33,19 @@ void test('generateScenePreviewThumbnailBytes: image scales to max edge', async

  await fs.rm(tmp, { recursive: true, force: true });
 });
+
+void test('generateScenePreviewThumbnailBytes: accepts image Buffer', async () => {
+  const png = await sharp({
+    create: {
+      width: 120,
+      height: 80,
+      channels: 3,
+      background: { r: 10, g: 20, b: 30 },
+    },
+  })
+    .png()
+    .toBuffer();
+  const buf = await generateScenePreviewThumbnailBytes(png, 'image');
+  assert.ok(buf !== null);
+  assert.ok(buf.length > 0);
+});
@@ -14,14 +14,15 @@ export const SCENE_PREVIEW_THUMB_MAX_PX = 320;

 /**
 * Builds a small WebP still for graph/list previews. Returns null if generation fails (import still succeeds).
+ * For images, prefer a Buffer so callers are not racy with reconcileAssetFiles unlinking paths.
 */
 export async function generateScenePreviewThumbnailBytes(
-  sourceAbsPath: string,
+  source: string | Buffer,
  kind: 'image' | 'video',
 ): Promise<Buffer | null> {
  try {
    if (kind === 'image') {
-      return await sharp(sourceAbsPath)
+      return await sharp(source)
        .rotate()
        .resize(SCENE_PREVIEW_THUMB_MAX_PX, SCENE_PREVIEW_THUMB_MAX_PX, {
          fit: 'inside',
@@ -31,6 +32,9 @@ export async function generateScenePreviewThumbnailBytes(
        .toBuffer();
    }

+    const sourceAbsPath = typeof source === 'string' ? source : null;
+    if (!sourceAbsPath) return null;
+
    const ffmpegPath = ffmpegStatic;
    if (!ffmpegPath) return null;

@@ -50,6 +50,17 @@ void test('zipStore: pack and open operations are serialized', () => {
  assert.match(src, /enqueueProjectSwitch/);
 });

+void test('zipStore: project updates and preview finalize are serialized', () => {
+  const src = fs.readFileSync(path.join(here, 'zipStore.ts'), 'utf8');
+  assert.match(src, /private projectUpdateChain: Promise<unknown>/);
+  assert.match(src, /private previewFinalizeChain: Promise<unknown>/);
+  assert.match(src, /enqueueProjectUpdate/);
+  assert.match(src, /enqueuePreviewFinalize/);
+  assert.match(src, /finalizeScenePreviewImportInner/);
+  assert.match(src, /drainProjectMutations/);
+  assert.match(src, /addBuffer/);
+});
+
 void test('zipStore: closeOpenProject is serialized with open on projectSwitchChain', () => {
  const src = fs.readFileSync(path.join(here, 'zipStore.ts'), 'utf8');
  assert.match(src, /async closeOpenProject\(\): Promise<void> \{[\s\S]*enqueueProjectSwitch/);
@@ -130,12 +130,44 @@ export class ZipProjectStore {
  private projectSession = 0;
  /** Serializes project.json writes — parallel renames caused ENOENT on Windows. */
  private projectWriteChain: Promise<void> = Promise.resolve();
+  /**
+   * Serializes project mutations (mutator + reconcileAssetFiles).
+   * Parallel updateProject during batch scene-preview import caused lost updates and
+   * reconcileAssetFiles deleting preview files still referenced after a stale write (ENOENT).
+   */
+  private projectUpdateChain: Promise<unknown> = Promise.resolve();
+  /** Serializes background preview optimize/thumb — avoids parallel sharp + orphan races. */
+  private previewFinalizeChain: Promise<unknown> = Promise.resolve();
  /** Serializes zip pack operations — parallel yazl/yauzl caused «unexpected number of bytes». */
  private packChain: Promise<void> = Promise.resolve();
  /** Serializes open/close/unzip — concurrent IPC caused ghost open projects and deadlocks. */
  private projectSwitchChain: Promise<void> = Promise.resolve();
  private saveDebounceTimer: ReturnType<typeof setTimeout> | null = null;

+  private enqueueProjectUpdate<T>(fn: () => Promise<T>): Promise<T> {
+    const task = this.projectUpdateChain.then(() => fn());
+    this.projectUpdateChain = task.then(
+      () => undefined,
+      () => undefined,
+    );
+    return task;
+  }
+
+  private enqueuePreviewFinalize<T>(fn: () => Promise<T>): Promise<T> {
+    const task = this.previewFinalizeChain.then(() => fn());
+    this.previewFinalizeChain = task.then(
+      () => undefined,
+      () => undefined,
+    );
+    return task;
+  }
+
+  /** Wait until preview optimize/thumb and project mutators finish (before pack/close). */
+  private async drainProjectMutations(): Promise<void> {
+    await this.previewFinalizeChain;
+    await this.projectUpdateChain;
+  }
+
  private enqueuePack(cacheDir: string, zipPath: string): Promise<void> {
    const next = this.packChain.then(async () => {
      await this.packZipFromCache(cacheDir, zipPath);
@@ -163,6 +195,8 @@ export class ZipProjectStore {
      clearTimeout(this.saveDebounceTimer);
      this.saveDebounceTimer = null;
    }
+    // Finish preview finalizers before flush/pack — otherwise yazl ENOENT on deleted assets.
+    await this.drainProjectMutations();
    if (this.saveQueued) {
      this.saveQueued = false;
      await this.flushSave();
@@ -172,6 +206,7 @@ export class ZipProjectStore {
    }
    await this.packChain;
    await this.projectWriteChain;
+    await this.drainProjectMutations();
  }

  async ensureRoots(): Promise<void> {
@@ -457,32 +492,68 @@ export class ZipProjectStore {
    sceneId: SceneId,
    assetId: AssetId,
  ): Promise<{ project: Project; changed: boolean }> {
+    return this.enqueuePreviewFinalize(() => this.finalizeScenePreviewImportInner(sceneId, assetId));
+  }
+
+  private async finalizeScenePreviewImportInner(
+    sceneId: SceneId,
+    assetId: AssetId,
+  ): Promise<{ project: Project; changed: boolean }> {
+    const sessionAtStart = this.projectSession;
    const open = this.openProject;
-    if (!open) throw new Error('No open project');
-    const sceneAtStart = open.project.scenes[sceneId];
+    if (!open) {
+      // Queued finalize after close — nothing to apply.
+      return { project: undefined as unknown as Project, changed: false };
+    }
+    const projectAtStart = open.project;
+    const sceneAtStart = projectAtStart.scenes[sceneId];
    if (sceneAtStart?.previewAssetId !== assetId) {
-      return { project: open.project, changed: false };
+      return { project: projectAtStart, changed: false };
    }
-    const sourceAsset = open.project.assets[assetId];
+    const sourceAsset = projectAtStart.assets[assetId];
    if (!sourceAsset || (sourceAsset.type !== 'image' && sourceAsset.type !== 'video')) {
-      return { project: open.project, changed: false };
+      return { project: projectAtStart, changed: false };
    }

+    const cacheDir = open.cacheDir;
+    const stillOpen = () =>
+      this.openProject !== null &&
+      this.projectSession === sessionAtStart &&
+      this.openProject.cacheDir === cacheDir;
+
    const generatedRelPaths: string[] = [];
    let finalAsset = sourceAsset;
    let finalAssetId = assetId;
-    let finalAbs = path.join(open.cacheDir, sourceAsset.relPath);
+    let finalAbs = path.join(cacheDir, sourceAsset.relPath);
+    /** Image bytes kept in memory so thumb/optimize do not re-open a path that reconcile may unlink. */
+    let imageBytes: Buffer | null = null;

    if (sourceAsset.type === 'image') {
-      const input = await fs.readFile(finalAbs);
-      const opt = await optimizeImageBufferVisuallyLossless(input);
+      try {
+        imageBytes = await fs.readFile(finalAbs);
+      } catch (e) {
+        const err = e as NodeJS.ErrnoException;
+        if (err?.code === 'ENOENT') {
+          const latest = this.getOpenProject();
+          return { project: latest ?? projectAtStart, changed: false };
+        }
+        throw e;
+      }
+      if (!stillOpen()) {
+        return { project: this.getOpenProject() ?? projectAtStart, changed: false };
+      }
+      const opt = await optimizeImageBufferVisuallyLossless(imageBytes);
+      if (!stillOpen()) {
+        return { project: this.getOpenProject() ?? projectAtStart, changed: false };
+      }
      if (!opt.passthrough) {
        finalAssetId = asAssetId(this.randomId());
        const optimizedName = `${path.parse(sourceAsset.originalName).name}.${opt.ext}`;
        const safeOptimizedName = sanitizeFileName(optimizedName);
        const optimizedRelPath = `assets/${finalAssetId}_${safeOptimizedName}`;
-        finalAbs = path.join(open.cacheDir, optimizedRelPath);
+        finalAbs = path.join(cacheDir, optimizedRelPath);
        const optimizedBuffer = Buffer.from(opt.buffer);
+        imageBytes = optimizedBuffer;
        await fs.writeFile(finalAbs, optimizedBuffer);
        generatedRelPaths.push(optimizedRelPath);
        const optimizedAsset = buildMediaAsset(
@@ -500,14 +571,26 @@ export class ZipProjectStore {
      }
    }

+    if (!stillOpen()) {
+      await Promise.all(
+        generatedRelPaths.map((relPath) =>
+          fs.unlink(path.join(cacheDir, relPath)).catch(() => undefined),
+        ),
+      );
+      return { project: this.getOpenProject() ?? projectAtStart, changed: false };
+    }
+
    const thumbKind = finalAsset.type === 'image' ? 'image' : 'video';
-    const thumbBytes = await generateScenePreviewThumbnailBytes(finalAbs, thumbKind);
+    const thumbBytes =
+      thumbKind === 'image' && imageBytes
+        ? await generateScenePreviewThumbnailBytes(imageBytes, 'image')
+        : await generateScenePreviewThumbnailBytes(finalAbs, thumbKind);
    let thumbAsset: MediaAsset | null = null;
    let thumbId: AssetId | null = null;
    if (thumbBytes !== null && thumbBytes.length > 0) {
      thumbId = asAssetId(this.randomId());
      const thumbRelPath = `assets/${thumbId}_preview_thumb.webp`;
-      const thumbAbs = path.join(open.cacheDir, thumbRelPath);
+      const thumbAbs = path.join(cacheDir, thumbRelPath);
      await fs.writeFile(thumbAbs, thumbBytes);
      generatedRelPaths.push(thumbRelPath);
      const thumbOrigName = `${path.parse(finalAsset.originalName).name}_preview_thumb.webp`;
@@ -521,46 +604,67 @@ export class ZipProjectStore {
      );
    }

-    await this.updateProject((p) => {
-      const scene = p.scenes[sceneId];
-      if (scene?.previewAssetId !== assetId) {
-        return p;
-      }
-      const assets: Record<AssetId, MediaAsset> = { ...p.assets, [finalAssetId]: finalAsset };
-      if (thumbAsset !== null && thumbId !== null) {
-        assets[thumbId] = thumbAsset;
-      }
-      return {
-        ...p,
-        assets,
-        scenes: {
-          ...p.scenes,
-          [sceneId]: {
-            ...scene,
-            previewAssetId: finalAssetId,
-            previewAssetType: finalAsset.type,
-            previewThumbAssetId: thumbId,
-            previewVideoAutostart: finalAsset.type === 'video' ? scene.previewVideoAutostart : false,
+    if (!stillOpen()) {
+      await Promise.all(
+        generatedRelPaths.map((relPath) =>
+          fs.unlink(path.join(cacheDir, relPath)).catch(() => undefined),
+        ),
+      );
+      return { project: this.getOpenProject() ?? projectAtStart, changed: false };
+    }
+
+    let applied = false;
+    try {
+      await this.updateProject((p) => {
+        const scene = p.scenes[sceneId];
+        if (scene?.previewAssetId !== assetId) {
+          return p;
+        }
+        applied = true;
+        const assets: Record<AssetId, MediaAsset> = { ...p.assets, [finalAssetId]: finalAsset };
+        if (finalAssetId !== assetId) {
+          delete assets[assetId];
+        }
+        if (thumbAsset !== null && thumbId !== null) {
+          assets[thumbId] = thumbAsset;
+        }
+        return {
+          ...p,
+          assets,
+          scenes: {
+            ...p.scenes,
+            [sceneId]: {
+              ...scene,
+              previewAssetId: finalAssetId,
+              previewAssetType: finalAsset.type,
+              previewThumbAssetId: thumbId,
+              previewVideoAutostart: finalAsset.type === 'video' ? scene.previewVideoAutostart : false,
+            },
          },
-        },
-      };
-    });
+        };
+      });
+    } catch (e) {
+      if (!stillOpen() || (e instanceof Error && e.message === 'No open project')) {
+        await Promise.all(
+          generatedRelPaths.map((relPath) =>
+            fs.unlink(path.join(cacheDir, relPath)).catch(() => undefined),
+          ),
+        );
+        return { project: this.getOpenProject() ?? projectAtStart, changed: false };
+      }
+      throw e;
+    }

    const latest = this.getOpenProject();
-    if (!latest) throw new Error('No open project');
-    const latestScene = latest.scenes[sceneId];
-    const applied =
-      latestScene?.previewAssetId === finalAssetId &&
-      (thumbId === null || latestScene.previewThumbAssetId === thumbId);
    if (!applied) {
      await Promise.all(
        generatedRelPaths.map((relPath) =>
-          fs.unlink(path.join(open.cacheDir, relPath)).catch(() => undefined),
+          fs.unlink(path.join(cacheDir, relPath)).catch(() => undefined),
        ),
      );
    }

-    return { project: latest, changed: applied };
+    return { project: latest ?? projectAtStart, changed: applied };
  }

  async clearScenePreview(sceneId: SceneId): Promise<Project> {
@@ -601,15 +705,17 @@ export class ZipProjectStore {
  }

  async updateProject(mutator: (draft: Project) => Project): Promise<Project> {
-    const open = this.openProject;
-    if (!open) throw new Error('No open project');
-    const prev = open.project;
-    let next = mutator(prev);
-    next = await reconcileAssetFiles(prev, next, open.cacheDir);
-    open.project = next;
-    await this.writeCacheProject(open.cacheDir, next);
-    this.queueSave();
-    return next;
+    return this.enqueueProjectUpdate(async () => {
+      const open = this.openProject;
+      if (!open) throw new Error('No open project');
+      const prev = open.project;
+      let next = mutator(prev);
+      next = await reconcileAssetFiles(prev, next, open.cacheDir);
+      open.project = next;
+      await this.writeCacheProject(open.cacheDir, next);
+      this.queueSave();
+      return next;
+    });
  }

  async updateScene(sceneId: SceneId, patch: ScenePatch): Promise<Scene> {
@@ -1653,14 +1759,18 @@ export class ZipProjectStore {
  async saveNow(): Promise<void> {
    const open = this.openProject;
    if (!open) return;
+    // Let background preview finalizers finish before packing cache → zip.
+    await this.drainProjectMutations();
    await this.projectWriteChain;
    await this.enqueuePack(open.cacheDir, open.zipPath);
  }

  async closeOpenProject(): Promise<void> {
    return this.enqueueProjectSwitch(async () => {
+      // Invalidate in-flight finalize early; then wait for queue to settle.
      this.projectSession += 1;
      if (!this.openProject) return;
+      await this.drainProjectMutations();
      await this.saveNow();
      await this.drainSavePipeline();
      this.saveQueued = false;
@@ -2752,7 +2862,16 @@ async function zipDir(srcDir: string, outZipPath: string): Promise<void> {
  const all = await listFilesRecursive(srcDir);
  for (const abs of all) {
    const rel = path.relative(srcDir, abs).replace(/\\/gu, '/');
-    zipfile.addFile(abs, rel, zipOptionsForRelativeEntry(rel));
+    // Bufferize: yazl.addFile opens lazily and can ENOENT if reconcile unlinks mid-pack.
+    let buf: Buffer;
+    try {
+      buf = await fs.readFile(abs);
+    } catch (e) {
+      const err = e as NodeJS.ErrnoException;
+      if (err?.code === 'ENOENT') continue;
+      throw e;
+    }
+    zipfile.addBuffer(buf, rel, zipOptionsForRelativeEntry(rel));
  }
  await fs.mkdir(path.dirname(outZipPath), { recursive: true });
  const out = fssync.createWriteStream(outZipPath);