fix(backup): isolate retry temp archives (#101449)

LiLan0125 · vincentkoc · web-flow · commit 686b7784689c · 2026-07-07T01:29:45.000-07:00
* fix(backup): isolate retry temp archives Closes #101382 * fix(backup): remove unused retry path initializer --------- Co-authored-by: Vincent Koc <vincentkoc@ieee.org>
diff --git a/src/commands/backup.atomic.test.ts b/src/commands/backup.atomic.test.ts
@@ -86,6 +86,56 @@ describe("backupCreateCommand atomic archive write", () => {
     }
   });
 
+  it("cleans intermediate retry temp archives after cleanup races", async () => {
+    const { archiveDir, outputPath, runtime } = await prepareAtomicBackupScenario({
+      archivePrefix: "openclaw-backup-retry-cleanup-",
+    });
+    const realRm = fs.rm.bind(fs);
+    const rmAttempts = new Map<string, number>();
+    const attemptFiles: string[] = [];
+    const rmSpy = vi.spyOn(fs, "rm").mockImplementation((async (
+      targetPath: Parameters<typeof fs.rm>[0],
+      options?: Parameters<typeof fs.rm>[1],
+    ) => {
+      const key = String(targetPath);
+      const attempt = (rmAttempts.get(key) ?? 0) + 1;
+      rmAttempts.set(key, attempt);
+      if ((key === attemptFiles[0] || key === attemptFiles[1]) && attempt === 1) {
+        throw Object.assign(new Error("resource busy"), { code: "EBUSY" });
+      }
+      await realRm(targetPath, options);
+    }) as typeof fs.rm);
+    try {
+      let tarAttempt = 0;
+      tarCreateMock.mockImplementation(async ({ file }: { file: string }) => {
+        tarAttempt += 1;
+        attemptFiles.push(file);
+        await fs.writeFile(file, `archive-attempt-${tarAttempt}`, "utf8");
+        if (tarAttempt < 3) {
+          throw Object.assign(new Error("did not encounter expected EOF"), {
+            path: path.join(tempHome.home, ".openclaw", "state.txt"),
+          });
+        }
+      });
+
+      const result = await backupCreateCommand(runtime, {
+        output: outputPath,
+      });
+
+      expect(result.archivePath).toBe(outputPath);
+      expect(attemptFiles).toStrictEqual([
+        attemptFiles[0],
+        `${attemptFiles[0]}.retry-2`,
+        `${attemptFiles[0]}.retry-3`,
+      ]);
+      expect(rmAttempts.get(attemptFiles[1])).toBeGreaterThanOrEqual(2);
+      expect((await fs.readdir(archiveDir)).toSorted()).toStrictEqual([path.basename(outputPath)]);
+    } finally {
+      rmSpy.mockRestore();
+      await fs.rm(archiveDir, { recursive: true, force: true });
+    }
+  });
+
   it("does not overwrite an archive created after readiness checks complete", async () => {
     const { archiveDir, outputPath, runtime } = await prepareAtomicBackupScenario({
       archivePrefix: "openclaw-backup-race-",
diff --git a/src/infra/backup-create.test.ts b/src/infra/backup-create.test.ts
@@ -223,6 +223,70 @@ describe("writeTarArchiveWithRetry", () => {
     expect(log).toHaveBeenCalledTimes(2);
   });
 
+  it("uses a fresh temp archive path when cleanup cannot remove a failed attempt", async () => {
+    const eofErr = Object.assign(new Error("did not encounter expected EOF"), {
+      path: "/state/sessions/s-abc/transcript.jsonl",
+    });
+    const tempArchivePath = "/tmp/backup.tar.gz.tmp";
+    const runTar = vi
+      .fn<(attemptTempArchivePath: string) => Promise<void>>()
+      .mockRejectedValueOnce(eofErr)
+      .mockResolvedValueOnce(undefined);
+    const log = vi.fn();
+    const sleep = vi.fn<(ms: number) => Promise<void>>().mockResolvedValue(undefined);
+    const rmSpy = vi.spyOn(fs, "rm").mockImplementation(async () => {
+      throw Object.assign(new Error("resource busy"), { code: "EBUSY" });
+    });
+
+    try {
+      const completedTempArchivePath = await backupCreateInternals.writeTarArchiveWithRetry({
+        tempArchivePath,
+        runTar,
+        log,
+        sleepMs: sleep,
+      });
+
+      expect(runTar).toHaveBeenNthCalledWith(1, tempArchivePath);
+      expect(runTar).toHaveBeenNthCalledWith(2, `${tempArchivePath}.retry-2`);
+      expect(completedTempArchivePath).toBe(`${tempArchivePath}.retry-2`);
+      expect(rmSpy).toHaveBeenCalledWith(tempArchivePath, { force: true });
+      expect(log).toHaveBeenCalledWith(
+        `Backup archiver could not remove temp archive ${tempArchivePath} between retries: EBUSY. Continuing.`,
+      );
+    } finally {
+      rmSpy.mockRestore();
+    }
+  });
+
+  it("cleans retry temp archive paths when a later attempt fails", async () => {
+    const eofErr = Object.assign(new Error("did not encounter expected EOF"), {
+      path: "/state/sessions/s-abc/transcript.jsonl",
+    });
+    const tempArchivePath = "/tmp/backup.tar.gz.tmp";
+    const runTar = vi
+      .fn<(attemptTempArchivePath: string) => Promise<void>>()
+      .mockRejectedValueOnce(eofErr)
+      .mockRejectedValueOnce(new Error("permission denied"));
+    const sleep = vi.fn<(ms: number) => Promise<void>>().mockResolvedValue(undefined);
+    const rmSpy = vi.spyOn(fs, "rm").mockResolvedValue(undefined);
+
+    try {
+      await expect(
+        backupCreateInternals.writeTarArchiveWithRetry({
+          tempArchivePath,
+          runTar,
+          sleepMs: sleep,
+        }),
+      ).rejects.toThrow(/permission denied/);
+
+      expect(runTar).toHaveBeenNthCalledWith(1, tempArchivePath);
+      expect(runTar).toHaveBeenNthCalledWith(2, `${tempArchivePath}.retry-2`);
+      expect(rmSpy).toHaveBeenCalledWith(`${tempArchivePath}.retry-2`, { force: true });
+    } finally {
+      rmSpy.mockRestore();
+    }
+  });
+
   it("surfaces the offending path and attempt count after exhausting retries", async () => {
     const eofErr = Object.assign(new Error("did not encounter expected EOF"), {
       path: "/state/logs/gateway.jsonl",
diff --git a/src/infra/backup-create.ts b/src/infra/backup-create.ts
@@ -170,30 +170,53 @@ function isTarEofRaceError(err: unknown): boolean {
 
 export type BackupTarRetryLogger = (message: string) => void;
 
+function resolveBackupTarAttemptTempPath(tempArchivePath: string, attempt: number): string {
+  return attempt === 1 ? tempArchivePath : `${tempArchivePath}.retry-${attempt}`;
+}
+
+function resolveBackupTarAttemptTempPaths(tempArchivePath: string): string[] {
+  return Array.from({ length: BACKUP_TAR_MAX_ATTEMPTS }, (_value, index) =>
+    resolveBackupTarAttemptTempPath(tempArchivePath, index + 1),
+  );
+}
+
+async function removeBackupTempArchiveBestEffort(tempArchivePath: string): Promise<void> {
+  await fs.rm(tempArchivePath, { force: true }).catch(() => undefined);
+}
+
 async function writeTarArchiveWithRetry(params: {
   tempArchivePath: string;
-  runTar: () => Promise<void>;
+  runTar: (tempArchivePath: string) => Promise<void>;
   log?: BackupTarRetryLogger;
   sleepMs?: (ms: number) => Promise<void>;
-}): Promise<void> {
+}): Promise<string> {
   const sleepFn = params.sleepMs ?? sleep;
   let lastErr: unknown;
+  const attemptTempArchivePaths: string[] = [];
   for (let attempt = 1; attempt <= BACKUP_TAR_MAX_ATTEMPTS; attempt += 1) {
+    const attemptTempArchivePath = resolveBackupTarAttemptTempPath(params.tempArchivePath, attempt);
+    attemptTempArchivePaths.push(attemptTempArchivePath);
     try {
-      await params.runTar();
-      return;
+      await params.runTar(attemptTempArchivePath);
+      for (const staleTempArchivePath of attemptTempArchivePaths.slice(0, -1)) {
+        await removeBackupTempArchiveBestEffort(staleTempArchivePath);
+      }
+      return attemptTempArchivePath;
     } catch (err) {
       lastErr = err;
       if (!isTarEofRaceError(err) || attempt === BACKUP_TAR_MAX_ATTEMPTS) {
+        for (const staleTempArchivePath of attemptTempArchivePaths) {
+          await removeBackupTempArchiveBestEffort(staleTempArchivePath);
+        }
         break;
       }
       try {
-        await fs.rm(params.tempArchivePath, { force: true });
+        await fs.rm(attemptTempArchivePath, { force: true });
       } catch (cleanupErr) {
         const code = (cleanupErr as NodeJS.ErrnoException).code;
         if (code && code !== "ENOENT") {
           params.log?.(
-            `Backup archiver could not remove temp archive ${params.tempArchivePath} between retries: ${code}. Continuing.`,
+            `Backup archiver could not remove temp archive ${attemptTempArchivePath} between retries: ${code}. Continuing.`,
           );
         }
       }
@@ -778,6 +801,7 @@ export async function createBackupArchive(
   const tempDir = await fs.mkdtemp(path.join(tempRoot, "openclaw-backup-"));
   const manifestPath = path.join(tempDir, "manifest.json");
   const tempArchivePath = buildTempArchivePath(outputPath);
+  const tempArchiveCleanupPaths = resolveBackupTarAttemptTempPaths(tempArchivePath);
   const stateAsset = result.assets.find((asset) => asset.kind === "state");
   try {
     const stateSqliteBackup = stateAsset
@@ -855,18 +879,18 @@ export async function createBackupArchive(
       }
       return true;
     };
-    await writeTarArchiveWithRetry({
+    const completedTempArchivePath = await writeTarArchiveWithRetry({
       tempArchivePath,
       log: opts.log,
-      runTar: async () => {
+      runTar: async (attemptTempArchivePath) => {
         // tar.c re-walks the tree (and thus re-invokes tarFilter) on every
         // attempt, so reset the closure counter here or retries would report
         // cumulative skip counts across attempts instead of the final one.
         skippedVolatileCount = 0;
         unexpectedSqliteSourcePaths.length = 0;
         await tar.c(
           {
-            file: tempArchivePath,
+            file: attemptTempArchivePath,
             gzip: true,
             portable: true,
             preservePaths: true,
@@ -904,9 +928,11 @@ export async function createBackupArchive(
         } (live sessions, cron logs, queues, sockets, pid/tmp).`,
       );
     }
-    await publishTempArchive({ tempArchivePath, outputPath });
+    await publishTempArchive({ tempArchivePath: completedTempArchivePath, outputPath });
   } finally {
-    await fs.rm(tempArchivePath, { force: true }).catch(() => undefined);
+    for (const cleanupPath of tempArchiveCleanupPaths) {
+      await removeBackupTempArchiveBestEffort(cleanupPath);
+    }
     await fs.rm(tempDir, { recursive: true, force: true }).catch(() => undefined);
   }
 
