fix(desktop): harden project bundle replacement on windows

0a92cbb9 · AI-甘富林 · ecd9651d · 0a92cbb9 · 0a92cbb9 · 0a92cbb9
Commit 0a92cbb9 authored Apr 10, 2026 by AI-甘富林
3 changed files
--- a/apps/desktop/src/main/services/project-bundle.ts
+++ b/apps/desktop/src/main/services/project-bundle.ts
@@ -50,10 +50,18 @@ interface BundleReplacementOperation {
 interface CommittedBundleReplacementOperation extends BundleReplacementOperation {
  backupPath?: string;
+  backupMode?: "rename" | "copy";
  targetExisted: boolean;
  applied: boolean;
 }
+interface RetryableFsOperationLogContext {
+  action: string;
+  path: string;
+  backupPath?: string;
+  projectId?: string;
+}
 interface MaterializedBundleTransaction {
  skillIds: string[];
  sharedSkillEntries: string[];
@@ -87,6 +95,8 @@ const REDIRECT_STATUS_CODES = new Set([301, 302, 307, 308]);
 const HEAD_UNSUPPORTED_STATUS_CODES = new Set([403, 404, 405, 501]);
 const MAX_REDIRECTS = 5;
 const BUNDLE_REQUEST_IDLE_TIMEOUT_MS = 30_000;
+const WINDOWS_RETRYABLE_FS_ERROR_CODES = new Set(["EBUSY", "EPERM", "ENOTEMPTY"]);
+const WINDOWS_FS_RETRY_DELAYS_MS = [50, 120, 250, 500, 1000] as const;
 function nowIso(): string {
  return new Date().toISOString();
@@ -178,6 +188,22 @@ function logBundle(event: string, details: Record<string, unknown>): void {
  console.info("[bundle]", event, { ...details, ts: new Date().toISOString() });
 }
+function getFsErrorCode(error: unknown): string | undefined {
+  if (!error || typeof error !== "object") {
+    return undefined;
+  }
+  const code = (error as { code?: unknown }).code;
+  return typeof code === "string" ? code : undefined;
+}
+function isRetryableWindowsFsError(error: unknown): boolean {
+  return process.platform === "win32" && WINDOWS_RETRYABLE_FS_ERROR_CODES.has(getFsErrorCode(error) ?? "");
+}
+function delay(ms: number): Promise<void> {
+  return new Promise((resolve) => setTimeout(resolve, ms));
+}
 export class ProjectBundleService {
  private readonly configService: AppConfigService;
  private readonly projectStore: ProjectStoreService;
@@ -612,6 +638,138 @@ export class ProjectBundleService {
    };
  }
+  private async withWindowsFsRetry<T>(
+    context: RetryableFsOperationLogContext,
+    operation: () => Promise<T>
+  ): Promise<T> {
+    let attempt = 0;
+    while (true) {
+      try {
+        return await operation();
+      } catch (error) {
+        if (!isRetryableWindowsFsError(error) || attempt >= WINDOWS_FS_RETRY_DELAYS_MS.length) {
+          throw error;
+        }
+        const delayMs = WINDOWS_FS_RETRY_DELAYS_MS[attempt];
+        const errorCode = getFsErrorCode(error);
+        attempt += 1;
+        logBundle("bundle.fs.retry", {
+          ...context,
+          attempt,
+          delayMs,
+          errorCode,
+          error: error instanceof Error ? error.message : String(error)
+        });
+        await this.startupLogger?.warn("project-bundle", "fs.retry", "Retrying Windows bundle filesystem operation.", {
+          ...context,
+          attempt,
+          delayMs,
+          errorCode,
+          error: error instanceof Error ? error.message : String(error)
+        });
+        await delay(delayMs);
+      }
+    }
+  }
+  private async removeReplacementPath(targetPath: string, projectId?: string): Promise<void> {
+    await this.withWindowsFsRetry(
+      {
+        action: "remove-path",
+        path: targetPath,
+        projectId
+      },
+      async () => {
+        await rm(targetPath, { recursive: true, force: true });
+      }
+    ).catch(() => undefined);
+  }
+  private async moveReplacementPath(sourcePath: string, targetPath: string, action: string, projectId?: string): Promise<void> {
+    await this.withWindowsFsRetry(
+      {
+        action,
+        path: sourcePath,
+        backupPath: targetPath,
+        projectId
+      },
+      async () => {
+        await rename(sourcePath, targetPath);
+      }
+    );
+  }
+  private async backupReplacementTarget(
+    operation: CommittedBundleReplacementOperation,
+    projectId: string
+  ): Promise<void> {
+    if (!operation.backupPath) {
+      return;
+    }
+    await mkdir(path.dirname(operation.backupPath), { recursive: true });
+    await this.removeReplacementPath(operation.backupPath, projectId);
+    try {
+      await this.moveReplacementPath(operation.targetPath, operation.backupPath, "backup-rename", projectId);
+      operation.backupMode = "rename";
+      return;
+    } catch (error) {
+      if (!isRetryableWindowsFsError(error)) {
+        throw error;
+      }
+      const errorCode = getFsErrorCode(error);
+      logBundle("bundle.fs.backup_fallback", {
+        action: "backup-copy",
+        path: operation.targetPath,
+        backupPath: operation.backupPath,
+        projectId,
+        errorCode,
+        error: error instanceof Error ? error.message : String(error)
+      });
+      await this.startupLogger?.warn("project-bundle", "backup.fallback", "Falling back to copy-based project bundle backup.", {
+        action: "backup-copy",
+        path: operation.targetPath,
+        backupPath: operation.backupPath,
+        projectId,
+        errorCode,
+        error: error instanceof Error ? error.message : String(error)
+      });
+      await cp(operation.targetPath, operation.backupPath, { recursive: true, force: true });
+      await this.removeReplacementPath(operation.targetPath, projectId);
+      operation.backupMode = "copy";
+    }
+  }
+  private async restoreReplacementBackup(operation: CommittedBundleReplacementOperation, projectId?: string): Promise<void> {
+    if (!operation.backupPath || !(await pathExists(operation.backupPath))) {
+      return;
+    }
+    await mkdir(path.dirname(operation.targetPath), { recursive: true });
+    await this.removeReplacementPath(operation.targetPath, projectId);
+    if (operation.backupMode === "copy") {
+      logBundle("bundle.fs.rollback_restore", {
+        action: "rollback-copy-restore",
+        path: operation.targetPath,
+        backupPath: operation.backupPath,
+        projectId
+      });
+      await this.startupLogger?.info("project-bundle", "rollback.copy_restore", "Restoring project bundle backup from copied directory.", {
+        action: "rollback-copy-restore",
+        path: operation.targetPath,
+        backupPath: operation.backupPath,
+        projectId
+      });
+      await cp(operation.backupPath, operation.targetPath, { recursive: true, force: true });
+      return;
+    }
+    await this.moveReplacementPath(operation.backupPath, operation.targetPath, "rollback-restore", projectId);
+  }
  private async materializeBundle(
    workspaceRoot: string,
    tempRoot: string,
@@ -657,23 +815,22 @@ export class ProjectBundleService {
          ...operation,
          targetExisted: await pathExists(operation.targetPath),
          backupPath: undefined,
+          backupMode: undefined,
          applied: false
        };
        if (committedOperation.targetExisted) {
          committedOperation.backupPath = this.buildReplacementBackupPath(backupRoot, operation.targetPath);
-          await mkdir(path.dirname(committedOperation.backupPath), { recursive: true });
+          await this.backupReplacementTarget(committedOperation, metadata.projectId);
-          await rm(committedOperation.backupPath, { recursive: true, force: true }).catch(() => undefined);
-          await rename(operation.targetPath, committedOperation.backupPath);
        }
        committedOperations.push(committedOperation);
        await mkdir(path.dirname(operation.targetPath), { recursive: true });
-        await rename(operation.stagedPath, operation.targetPath);
+        await this.moveReplacementPath(operation.stagedPath, operation.targetPath, "apply-replacement", metadata.projectId);
        committedOperation.applied = true;
      }
    } catch (error) {
-      await this.rollbackReplacementOperations(committedOperations).catch(() => undefined);
+      await this.rollbackReplacementOperations(committedOperations, metadata.projectId).catch(() => undefined);
      throw error;
    }
@@ -686,14 +843,14 @@ export class ProjectBundleService {
          return;
        }
        settled = true;
-        await this.cleanupReplacementBackups(committedOperations);
+        await this.cleanupReplacementBackups(committedOperations, metadata.projectId);
      },
      rollback: async () => {
        if (settled) {
          return;
        }
        settled = true;
-        await this.rollbackReplacementOperations(committedOperations);
+        await this.rollbackReplacementOperations(committedOperations, metadata.projectId);
      }
    };
  }
@@ -796,23 +953,25 @@ export class ProjectBundleService {
    return path.join(backupRoot, `${path.basename(targetPath)}-${hashedTarget}`);
  }
-  private async rollbackReplacementOperations(operations: CommittedBundleReplacementOperation[]): Promise<void> {
+  private async rollbackReplacementOperations(
+    operations: CommittedBundleReplacementOperation[],
+    projectId?: string
+  ): Promise<void> {
    for (const operation of [...operations].reverse()) {
      if (operation.applied) {
-        await rm(operation.targetPath, { recursive: true, force: true }).catch(() => undefined);
+        await this.removeReplacementPath(operation.targetPath, projectId);
-      }
-      if (operation.backupPath && await pathExists(operation.backupPath)) {
-        await mkdir(path.dirname(operation.targetPath), { recursive: true });
-        await rm(operation.targetPath, { recursive: true, force: true }).catch(() => undefined);
-        await rename(operation.backupPath, operation.targetPath);
      }
+      await this.restoreReplacementBackup(operation, projectId);
    }
  }
-  private async cleanupReplacementBackups(operations: CommittedBundleReplacementOperation[]): Promise<void> {
+  private async cleanupReplacementBackups(
+    operations: CommittedBundleReplacementOperation[],
+    projectId?: string
+  ): Promise<void> {
    const backupPaths = uniqueStrings(operations.map((operation) => operation.backupPath ?? ""));
    for (const backupPath of backupPaths) {
-      await rm(backupPath, { recursive: true, force: true }).catch(() => undefined);
+      await this.removeReplacementPath(backupPath, projectId);
    }
  }

--- a/build/scripts/project-bundle-replacement-smoke.ps1
+++ b/build/scripts/project-bundle-replacement-smoke.ps1
@@ -10,6 +10,8 @@ $tempRoot = Join-Path $repoRoot '.tmp\project-bundle-replacement-smoke'
 $compileRoot = Join-Path $tempRoot 'compiled'
 $entryPath = Join-Path $compileRoot 'build\scripts\project-bundle-replacement-smoke.js'
 $compilePackagePath = Join-Path $compileRoot 'package.json'
+$compileNodeModulesPath = Join-Path $compileRoot 'node_modules'
+$desktopNodeModulesPath = Join-Path $repoRoot 'apps\desktop\node_modules'
 $bundleRootA = Join-Path $tempRoot 'bundle-src-a'
 $bundleRootB = Join-Path $tempRoot 'bundle-src-b'
 $bundleRootC = Join-Path $tempRoot 'bundle-src-c'
@@ -25,6 +27,23 @@ function Write-Utf8File {
  [System.IO.File]::WriteAllText($FilePath, $Content, $encoding)
 }
+function Ensure-NodeModulesLink {
+  param(
+    [string]$LinkPath,
+    [string]$TargetPath
+  )
+  if (Test-Path $LinkPath) {
+    Remove-Item $LinkPath -Recurse -Force
+  }
+  if (-not (Test-Path $TargetPath)) {
+    throw "Desktop node_modules was not found: $TargetPath"
+  }
+  New-Item -ItemType Junction -Path $LinkPath -Target $TargetPath -Force | Out-Null
+}
 function New-BundleFixture {
  param(
    [string]$Root,
@@ -104,6 +123,7 @@ Compress-Archive -Path (Join-Path $bundleRootB '*') -DestinationPath $bundleZipP
 Compress-Archive -Path (Join-Path $bundleRootC '*') -DestinationPath $bundleZipPathC -Force
 Write-Utf8File -FilePath $compilePackagePath -Content '{"type":"module"}'
+Ensure-NodeModulesLink -LinkPath $compileNodeModulesPath -TargetPath $desktopNodeModulesPath
 Write-Host 'Running project-bundle replacement smoke'
 node $entryPath $resolvedResultPath $bundleZipPathA $bundleZipPathB $bundleZipPathC $bundleRootA $bundleRootB $bundleRootC

--- a/build/scripts/project-bundle-replacement-smoke.ts
+++ b/build/scripts/project-bundle-replacement-smoke.ts
@@ -14,6 +14,12 @@ function assert(condition: unknown, message: string): asserts condition {
  }
 }
+function createRetryableWindowsError(message: string, code: "EBUSY" | "EPERM" | "ENOTEMPTY" = "EBUSY"): NodeJS.ErrnoException {
+  const error = new Error(message) as NodeJS.ErrnoException;
+  error.code = code;
+  return error;
+}
 async function pathExists(targetPath: string): Promise<boolean> {
  try {
    await stat(targetPath);
@@ -133,6 +139,11 @@ async function main(): Promise<void> {
    const projectBundleService = new ProjectBundleService(configService, projectStore);
    const projectBundleServiceWithTestHooks = projectBundleService as unknown as {
      extractZip(zipPath: string, destinationPath: string): Promise<void>;
+      withWindowsFsRetry?<T>(
+        context: { action: string; path: string; backupPath?: string; projectId?: string },
+        operation: () => Promise<T>
+      ): Promise<T>;
+      moveReplacementPath?(sourcePath: string, targetPath: string, action: string, projectId?: string): Promise<void>;
    };
    projectBundleServiceWithTestHooks.extractZip = async (_zipPath: string, destinationPath: string) => {
      const sourceRoot = activeVariant === "a"
@@ -165,8 +176,27 @@ async function main(): Promise<void> {
    assert(await pathExists(path.join(workspaceRoot, "skills", variantSkillEntry.a)), "Variant A skill entry was not materialized.");
    assert(await pathExists(path.join(workspaceRoot, "cron", variantCronEntry.a)), "Variant A cron entry was not materialized.");
+    const originalWithWindowsFsRetry = projectBundleServiceWithTestHooks.withWindowsFsRetry?.bind(projectBundleService);
+    assert(originalWithWindowsFsRetry, "withWindowsFsRetry hook was not available for smoke test.");
+    let injectedRetryCount = 0;
+    projectBundleServiceWithTestHooks.withWindowsFsRetry = async <T>(context, operation) => {
+      if (context.action === "apply-replacement" && injectedRetryCount === 0) {
+        let firstAttempt = true;
+        return originalWithWindowsFsRetry(context, async () => {
+          if (firstAttempt) {
+            firstAttempt = false;
+            injectedRetryCount += 1;
+            throw createRetryableWindowsError("Injected retryable apply-replacement failure.");
+          }
+          return operation();
+        });
+      }
+      return originalWithWindowsFsRetry(context, operation);
+    };
    activeVariant = "b";
    await projectBundleService.syncRemoteBundles([remoteAsset], configVersion, "sync");
+    assert(injectedRetryCount === 1, `Expected one injected retry before variant B replacement, received ${injectedRetryCount}.`);
    const manifestAfterB = (await readJsonFile<Record<string, {
      checksum?: string;
@@ -184,6 +214,17 @@ async function main(): Promise<void> {
    assert(!(await pathExists(path.join(workspaceRoot, "skills", variantSkillEntry.a))), "Variant A skill entry was not cleaned up after replacement with variant B.");
    assert(!(await pathExists(path.join(workspaceRoot, "cron", variantCronEntry.a))), "Variant A cron entry was not cleaned up after replacement with variant B.");
+    const originalMoveReplacementPath = projectBundleServiceWithTestHooks.moveReplacementPath?.bind(projectBundleService);
+    assert(originalMoveReplacementPath, "moveReplacementPath hook was not available for smoke test.");
+    let injectedFallbackCount = 0;
+    projectBundleServiceWithTestHooks.moveReplacementPath = async (sourcePath, targetPath, action, hookProjectId) => {
+      if (action === "backup-rename" && hookProjectId === projectId && injectedFallbackCount === 0) {
+        injectedFallbackCount += 1;
+        throw createRetryableWindowsError("Injected retryable backup rename failure.");
+      }
+      return originalMoveReplacementPath(sourcePath, targetPath, action, hookProjectId);
+    };
    const originalSyncBundleProject = projectStore.syncBundleProject.bind(projectStore);
    let injectedFailureCount = 0;
    (projectStore as ProjectStoreService & {
@@ -203,7 +244,10 @@ async function main(): Promise<void> {
    (projectStore as ProjectStoreService & {
      syncBundleProject: typeof originalSyncBundleProject;
    }).syncBundleProject = originalSyncBundleProject;
+    projectBundleServiceWithTestHooks.moveReplacementPath = originalMoveReplacementPath;
+    projectBundleServiceWithTestHooks.withWindowsFsRetry = originalWithWindowsFsRetry;
+    assert(injectedFallbackCount === 1, `Expected one injected fallback backup failure, received ${injectedFallbackCount}.`);
    assert(replacementFailure === "Simulated syncBundleProject failure after bundle replacement commit.", "Injected replacement failure did not surface the expected error.");
    const readmeAfterFailure = await readFile(readmePath, "utf8");
@@ -257,6 +301,8 @@ async function main(): Promise<void> {
      downloadUrl,
      configVersion,
      replacementFailure,
+      retryInjectionCount: injectedRetryCount,
+      fallbackInjectionCount: injectedFallbackCount,
      manifestRemoteEtagAfterB: recordAfterB.remoteEtag ?? null,
      manifestRemoteEtagAfterFailure: recordAfterFailure?.remoteEtag ?? null,
      manifestRemoteEtagAfterRecovery: recordAfterRecovery?.remoteEtag ?? null,