From 46991b520f107d55b45c1e47fa1e6fba75b5b5a5 Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Thu, 15 Oct 2026 14:57:29 +0200 Subject: [PATCH 1/2] fix: fall back to a copy when a save's rename hits EXDEV The Microsoft Store build is an MSIX package, and Windows redirects its writes under AppData into the package's private copy of the folder. A rename between that copy and the real folder fails with EXDEV even inside one directory, so saving the project of a fresh recording failed. renameWithRetry moves to electron/atomic-rename.ts, copies then deletes on EXDEV (never for a case-only rename), and gains a synchronous twin. Every temp-then-rename save in electron/ now goes through them, including the copy style-preset-service kept of the helper. Part of #1255 --- electron/ai-edition/chat-persistence.ts | 2 +- electron/ai-edition/document-service.ts | 34 ++------- electron/ai-edition/style-preset-service.ts | 20 +----- electron/app-settings.ts | 5 +- electron/atomic-rename.test.ts | 70 +++++++++++++++++++ electron/atomic-rename.ts | 54 ++++++++++++++ electron/cli/cliMain.ts | 5 +- electron/mcp/app-channel.ts | 5 +- electron/media/mediaLinksRegistry.ts | 3 +- electron/permissions/index.ts | 5 +- electron/recording/recordingsLocationStore.ts | 3 +- electron/recording/webm-duration.ts | 5 +- electron/recording/webm-seek-index.ts | 3 +- electron/stt/modelManager.ts | 5 +- 14 files changed, 156 insertions(+), 63 deletions(-) create mode 100644 electron/atomic-rename.test.ts create mode 100644 electron/atomic-rename.ts diff --git a/electron/ai-edition/chat-persistence.ts b/electron/ai-edition/chat-persistence.ts index eb5ee144b7..79d6d90792 100644 --- a/electron/ai-edition/chat-persistence.ts +++ b/electron/ai-edition/chat-persistence.ts @@ -7,7 +7,7 @@ import { existsSync, readFileSync } from "node:fs"; import fs from "node:fs/promises"; import path from "node:path"; import { z } from "zod"; -import { renameWithRetry } from "./document-service"; +import { renameWithRetry } from "../atomic-rename"; const toolCallSchema = z.object({ name: z.string(), summary: z.string() }); const messageSchema = z.object({ diff --git a/electron/ai-edition/document-service.ts b/electron/ai-edition/document-service.ts index 0d44274af4..52e1bb97e6 100644 --- a/electron/ai-edition/document-service.ts +++ b/electron/ai-edition/document-service.ts @@ -10,7 +10,7 @@ // ponytail: Phase 1 surface area is intentionally narrow (list / get / create // / save / addAsset / removeAsset). ops/history/agent runtime land in Phase 6. -import { mkdirSync, renameSync, rmSync, writeFileSync } from "node:fs"; +import { mkdirSync, rmSync, writeFileSync } from "node:fs"; import fs, { type FileHandle } from "node:fs/promises"; import path from "node:path"; import { createId } from "../../src/lib/ai-edition/document/ids"; @@ -27,6 +27,7 @@ import { type StylePresetAppearance, stylePresetLegacyEditor, } from "../../src/lib/ai-edition/stylePresets"; +import { renameSyncWithFallback, renameWithRetry } from "../atomic-rename"; import { ensureDocumentExtensions } from "../media/extensionClip"; import { relinkProjectMedia } from "../media/projectMediaRelinker"; @@ -239,27 +240,6 @@ function parseLoadedDocument(raw: string): AxcutDocument { return documentSchema.parse(migrateRawDocumentToCurrent(JSON.parse(raw))); } -/** - * Windows fails a rename onto an open file with EPERM/EBUSY: an indexer, an - * antivirus or a backup agent can hold the destination for a few milliseconds - * after we last touched it. The write itself is sound, so retry briefly rather - * than surface a save error the user cannot act on. POSIX renames don't hit this - * and take the first attempt. - */ -export async function renameWithRetry(from: string, to: string): Promise { - const RETRYABLE = new Set(["EPERM", "EACCES", "EBUSY"]); - for (let attempt = 0; ; attempt++) { - try { - await fs.rename(from, to); - return; - } catch (error) { - const code = (error as NodeJS.ErrnoException)?.code ?? ""; - if (attempt >= 5 || !RETRYABLE.has(code)) throw error; - await new Promise((resolve) => setTimeout(resolve, 10 * 2 ** attempt)); - } - } -} - export class DocumentService { private readonly projectsRoot: string; private readonly mediaRegistryDir: string; @@ -325,11 +305,9 @@ export class DocumentService { await fs.access(to); await fs.unlink(from); } catch { - await fs - .rename(from, to) - .catch((err) => - console.warn(`[ai-edition] failed to migrate ${from} -> ${to}:`, err), - ); + await renameWithRetry(from, to).catch((err) => + console.warn(`[ai-edition] failed to migrate ${from} -> ${to}:`, err), + ); } }), ); @@ -370,7 +348,7 @@ export class DocumentService { // Temp + rename: a reader never sees a half-written marker. const temp = `${marker}.tmp-${process.pid}`; writeFileSync(temp, JSON.stringify({ pid: process.pid, projectId }), "utf8"); - renameSync(temp, marker); + renameSyncWithFallback(temp, marker); } catch (error) { console.warn("[document-service] could not record the project open in the editor:", error); } diff --git a/electron/ai-edition/style-preset-service.ts b/electron/ai-edition/style-preset-service.ts index 2dcaf671bb..ae6ac88128 100644 --- a/electron/ai-edition/style-preset-service.ts +++ b/electron/ai-edition/style-preset-service.ts @@ -20,6 +20,7 @@ import { serializeStylePresetFile, stylePresetFileBaseName, } from "../../src/lib/ai-edition/stylePresets"; +import { renameWithRetry } from "../atomic-rename"; export type StylePresetErrorCode = "NAME_TAKEN" | "NOT_FOUND" | "INVALID_ID"; @@ -39,25 +40,6 @@ export interface StylePresetRevealTarget { path: string; } -/** - * Local copy of document-service's `renameWithRetry`: Windows fails a rename onto a file an - * indexer, antivirus or backup agent holds open for a few milliseconds. Copied rather than - * imported because importing document-service drags the whole project schema along. - */ -async function renameWithRetry(from: string, to: string): Promise { - const RETRYABLE = new Set(["EPERM", "EACCES", "EBUSY"]); - for (let attempt = 0; ; attempt++) { - try { - await fs.rename(from, to); - return; - } catch (error) { - const code = (error as NodeJS.ErrnoException)?.code ?? ""; - if (attempt >= 5 || !RETRYABLE.has(code)) throw error; - await new Promise((resolve) => setTimeout(resolve, 10 * 2 ** attempt)); - } - } -} - /** * Names the preset new projects start from: one line, the preset's id. It sits in the presets * folder so it travels with them, and has no preset extension so `list` never reads it. diff --git a/electron/app-settings.ts b/electron/app-settings.ts index bef025bf3a..81d334260c 100644 --- a/electron/app-settings.ts +++ b/electron/app-settings.ts @@ -1,4 +1,4 @@ -import { readFileSync, renameSync, rmSync, writeFileSync } from "node:fs"; +import { readFileSync, rmSync, writeFileSync } from "node:fs"; import path from "node:path"; import { DEFAULT_WEBCAM_QUALITY, @@ -7,6 +7,7 @@ import { webcamQualityFrom, } from "../src/hooks/webcamCaptureTarget"; import type { CursorCaptureMode } from "../src/lib/recordingSession"; +import { renameSyncWithFallback } from "./atomic-rename"; export interface RecordingPreferences { micEnabled: boolean; @@ -91,7 +92,7 @@ function atomicWrite(userData: string, value: RawSettings): void { const temporary = `${destination}.${process.pid}.${Date.now()}.tmp`; try { writeFileSync(temporary, `${JSON.stringify(value, null, 2)}\n`, "utf8"); - renameSync(temporary, destination); + renameSyncWithFallback(temporary, destination); } finally { rmSync(temporary, { force: true }); } diff --git a/electron/atomic-rename.test.ts b/electron/atomic-rename.test.ts new file mode 100644 index 0000000000..a6d4d091c4 --- /dev/null +++ b/electron/atomic-rename.test.ts @@ -0,0 +1,70 @@ +import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { renameSyncWithFallback, renameWithRetry } from "./atomic-rename"; + +// No test machine has two layers of one folder, so the MSIX virtualization (#1255) is +// played by a rename that always answers EXDEV. +const exdev = vi.hoisted(() => ({ + on: false, + error: () => + Object.assign(new Error("EXDEV: cross-device link not permitted"), { code: "EXDEV" }), +})); + +vi.mock("node:fs/promises", async (importOriginal) => { + const actual = await importOriginal(); + const rename: typeof actual.rename = (from, to) => + exdev.on ? Promise.reject(exdev.error()) : actual.rename(from, to); + return { ...actual, rename, default: { ...actual, rename } }; +}); + +vi.mock("node:fs", async (importOriginal) => { + const actual = await importOriginal(); + const renameSync: typeof actual.renameSync = (from, to) => { + if (exdev.on) throw exdev.error(); + actual.renameSync(from, to); + }; + return { ...actual, renameSync, default: { ...actual, renameSync } }; +}); + +describe("atomic rename across an MSIX-virtualized folder", () => { + let dir: string; + let temp: string; + let target: string; + + beforeEach(() => { + dir = mkdtempSync(path.join(os.tmpdir(), "atomic-rename-")); + temp = path.join(dir, "proj.openscreen.tmp-1"); + target = path.join(dir, "proj.openscreen"); + writeFileSync(temp, "new", "utf8"); + writeFileSync(target, "old", "utf8"); + exdev.on = true; + }); + + afterEach(() => { + exdev.on = false; + rmSync(dir, { recursive: true, force: true }); + }); + + it("copies over the destination and removes the temp file", async () => { + await renameWithRetry(temp, target); + expect(readFileSync(target, "utf8")).toBe("new"); + expect(existsSync(temp)).toBe(false); + }); + + it("does the same synchronously", () => { + renameSyncWithFallback(temp, target); + expect(readFileSync(target, "utf8")).toBe("new"); + expect(existsSync(temp)).toBe(false); + }); + + it("never copies a case-only rename onto itself", async () => { + const preset = path.join(dir, "Studio.json"); + writeFileSync(preset, "preset", "utf8"); + await expect(renameWithRetry(preset, path.join(dir, "studio.json"))).rejects.toMatchObject({ + code: "EXDEV", + }); + expect(readFileSync(preset, "utf8")).toBe("preset"); + }); +}); diff --git a/electron/atomic-rename.ts b/electron/atomic-rename.ts new file mode 100644 index 0000000000..0a70e7ba3c --- /dev/null +++ b/electron/atomic-rename.ts @@ -0,0 +1,54 @@ +import { copyFileSync, renameSync, rmSync } from "node:fs"; +import fs from "node:fs/promises"; + +/** + * The Microsoft Store build is an MSIX package: Windows redirects its writes under AppData + * into the package's private copy of the folder. A rename between that copy and the real + * folder fails with EXDEV, even inside one directory (#1255). Copying then deleting is the + * only way across. It is not atomic, so it is taken only when the rename cannot be. + * + * Never for a case-only rename: on a case-insensitive disk both names are the same file, + * and copying it onto itself then deleting the source would lose it. + */ +function mustCopyInstead(error: unknown, from: string, to: string): boolean { + return ( + (error as NodeJS.ErrnoException)?.code === "EXDEV" && from.toLowerCase() !== to.toLowerCase() + ); +} + +/** + * The rename half of a temp-then-rename save. Windows fails a rename onto an open file with + * EPERM/EBUSY: an indexer, an antivirus or a backup agent can hold the destination for a few + * milliseconds after we last touched it. The write itself is sound, so retry briefly rather + * than surface a save error the user cannot act on. POSIX renames don't hit this and take + * the first attempt. EXDEV falls back to a copy, see `mustCopyInstead`. + */ +export async function renameWithRetry(from: string, to: string): Promise { + const RETRYABLE = new Set(["EPERM", "EACCES", "EBUSY"]); + for (let attempt = 0; ; attempt++) { + try { + await fs.rename(from, to); + return; + } catch (error) { + if (mustCopyInstead(error, from, to)) { + await fs.copyFile(from, to); + await fs.rm(from, { force: true }); + return; + } + const code = (error as NodeJS.ErrnoException)?.code ?? ""; + if (attempt >= 5 || !RETRYABLE.has(code)) throw error; + await new Promise((resolve) => setTimeout(resolve, 10 * 2 ** attempt)); + } + } +} + +/** `renameSync` with the same EXDEV fallback, for the saves that must stay synchronous. */ +export function renameSyncWithFallback(from: string, to: string): void { + try { + renameSync(from, to); + } catch (error) { + if (!mustCopyInstead(error, from, to)) throw error; + copyFileSync(from, to); + rmSync(from, { force: true }); + } +} diff --git a/electron/cli/cliMain.ts b/electron/cli/cliMain.ts index 113815f680..fc1750968d 100644 --- a/electron/cli/cliMain.ts +++ b/electron/cli/cliMain.ts @@ -7,6 +7,7 @@ import path from "node:path"; import readline from "node:readline"; import { app, BrowserWindow, ipcMain, Menu, session, systemPreferences } from "electron"; import type { CliProgressEvent, CliSourcesResult } from "../../src/lib/cliContracts"; +import { renameWithRetry } from "../atomic-rename"; import { isDiagnosticModeEnabled } from "../diagnostics/main-log-buffer"; import { getSelectedDesktopSource, registerIpcHandlers } from "../ipc/handlers"; import { openVideoInHiddenWindow } from "../mcp/hidden-video"; @@ -495,7 +496,7 @@ export function runCli(command: CliCommand): void { `${command.projectPath} changed while it was being captioned, so nothing was written. Run captions again.`, ); } - await fs.rename(tmpPath, target); + await renameWithRetry(tmpPath, target); } finally { await fs.rm(tmpPath, { force: true }).catch(() => undefined); } @@ -521,7 +522,7 @@ export function runCli(command: CliCommand): void { const tmpOutPath = `${command.jsonOutPath}.${process.pid}.tmp`; try { await fs.writeFile(tmpOutPath, `${JSON.stringify(result.sources, null, 2)}\n`, "utf8"); - await fs.rename(tmpOutPath, command.jsonOutPath); + await renameWithRetry(tmpOutPath, command.jsonOutPath); } catch (writeError) { // Best-effort: the original error is what the caller needs to see. await fs.rm(tmpOutPath, { force: true }).catch(() => undefined); diff --git a/electron/mcp/app-channel.ts b/electron/mcp/app-channel.ts index 9da5585cf7..c81be24de6 100644 --- a/electron/mcp/app-channel.ts +++ b/electron/mcp/app-channel.ts @@ -24,7 +24,7 @@ // server's switches say: those guard the HTTP port, which is not ours to bypass. import { randomBytes, randomUUID, timingSafeEqual } from "node:crypto"; -import { chmodSync, renameSync, rmSync, writeFileSync } from "node:fs"; +import { chmodSync, rmSync, writeFileSync } from "node:fs"; import fs from "node:fs/promises"; import net from "node:net"; import path from "node:path"; @@ -34,6 +34,7 @@ import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js" import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import { isMutatingTool } from "../ai-edition/agent-tools"; import { isProcessAlive } from "../ai-edition/document-service"; +import { renameSyncWithFallback } from "../atomic-rename"; import { FORWARDED_POINTER_TRACK_SCHEMA, POINTER_TRACK_TOOL, @@ -169,7 +170,7 @@ export async function serveAppChannel(options: { encoding: "utf8", mode: 0o600, }); - renameSync(temp, file); + renameSyncWithFallback(temp, file); return { endpoint, diff --git a/electron/media/mediaLinksRegistry.ts b/electron/media/mediaLinksRegistry.ts index ec0ad76de5..92f3c0b197 100644 --- a/electron/media/mediaLinksRegistry.ts +++ b/electron/media/mediaLinksRegistry.ts @@ -20,6 +20,7 @@ import fs from "node:fs/promises"; import path from "node:path"; import type { CursorCaptureMode } from "../../src/lib/recordingSession"; +import { renameWithRetry } from "../atomic-rename"; // ponytail: `baseDir` is passed in by every caller (RECORDINGS_DIR in // electron/ipc/handlers.ts) rather than imported here, so this module has no @@ -150,7 +151,7 @@ async function writeRegistry(baseDir: string, file: MediaLinksRegistryFile): Pro const target = registryPath(baseDir); const tmpPath = `${target}.tmp-${process.pid}-${Date.now()}`; await fs.writeFile(tmpPath, JSON.stringify(file, null, 2), "utf-8"); - await fs.rename(tmpPath, target); + await renameWithRetry(tmpPath, target); } // Single-process, single-user desktop app — an in-process promise chain per diff --git a/electron/permissions/index.ts b/electron/permissions/index.ts index ec1d6dc005..0e3888e987 100644 --- a/electron/permissions/index.ts +++ b/electron/permissions/index.ts @@ -1,5 +1,5 @@ import { spawn } from "node:child_process"; -import { readFileSync, renameSync, rmSync, writeFileSync } from "node:fs"; +import { readFileSync, rmSync, writeFileSync } from "node:fs"; import path from "node:path"; import { app, @@ -9,6 +9,7 @@ import { shell, systemPreferences, } from "electron"; +import { renameSyncWithFallback } from "../atomic-rename"; import { macSystemPickerEnabled } from "../native-bridge/screen/macPickerSession"; import { findMacScreenAccessHelperPath, @@ -59,7 +60,7 @@ function createFileStore(userData: string): PermissionsStore { const temporary = `${file}.${process.pid}.tmp`; try { writeFileSync(temporary, `${JSON.stringify(state, null, 2)}\n`, "utf8"); - renameSync(temporary, file); + renameSyncWithFallback(temporary, file); } catch (error) { // Best effort: the in-memory note still gets the rest of this launch right. console.warn("[permissions] failed to persist the permissions note:", error); diff --git a/electron/recording/recordingsLocationStore.ts b/electron/recording/recordingsLocationStore.ts index 390c249e9b..1fb5f1d531 100644 --- a/electron/recording/recordingsLocationStore.ts +++ b/electron/recording/recordingsLocationStore.ts @@ -4,6 +4,7 @@ import { readFileSync } from "node:fs"; import fs from "node:fs/promises"; import path from "node:path"; +import { renameWithRetry } from "../atomic-rename"; interface RecordingsLocationConfig { recordingsDir: string | null; @@ -56,7 +57,7 @@ export class RecordingsLocationStore { const tmpPath = `${this.configPath}.tmp-${process.pid}-${Date.now()}`; try { await fs.writeFile(tmpPath, JSON.stringify(nextConfig, null, 2), "utf8"); - await fs.rename(tmpPath, this.configPath); + await renameWithRetry(tmpPath, this.configPath); this.config = nextConfig; } catch (error) { // Best-effort: don't let a cleanup failure hide the real error, and diff --git a/electron/recording/webm-duration.ts b/electron/recording/webm-duration.ts index d9b4b61979..dbca70a86d 100644 --- a/electron/recording/webm-duration.ts +++ b/electron/recording/webm-duration.ts @@ -3,6 +3,7 @@ import fs from "node:fs/promises"; import { pipeline } from "node:stream/promises"; import { fixParsedWebmDuration } from "@fix-webm-duration/fix"; import { WebmFile } from "@fix-webm-duration/parser"; +import { renameWithRetry } from "../atomic-rename"; export type DurationPatchResult = | { patched: true } @@ -143,7 +144,7 @@ export async function patchWebmDurationOnDisk( ws.destroy(); } - await fs.rename(tmpPath, filePath); + await renameWithRetry(tmpPath, filePath); return { patched: true }; } catch (error) { console.error(`[webm-duration] failed to patch ${filePath} using optimized method:`, error); @@ -187,7 +188,7 @@ async function patchWebmDurationInMemory( ); try { await fs.writeFile(tmpPath, patchedBytes); - await fs.rename(tmpPath, filePath); + await renameWithRetry(tmpPath, filePath); return { patched: true }; } catch (writeError) { console.error(`[webm-duration] failed to write patched ${filePath}:`, writeError); diff --git a/electron/recording/webm-seek-index.ts b/electron/recording/webm-seek-index.ts index 06430d290f..749805d7a8 100644 --- a/electron/recording/webm-seek-index.ts +++ b/electron/recording/webm-seek-index.ts @@ -1,4 +1,5 @@ import fs from "node:fs/promises"; +import { renameWithRetry } from "../atomic-rename"; import { CompositorViewService } from "../native-bridge/services/compositorViewService"; export type ReindexResult = @@ -90,7 +91,7 @@ export async function reindexRecordingOnDisk( return { reindexed: false, reason: "empty-output" }; } - await fs.rename(tmpPath, filePath); + await renameWithRetry(tmpPath, filePath); return { reindexed: true, packets: stats.packets, diff --git a/electron/stt/modelManager.ts b/electron/stt/modelManager.ts index d9d4aef32a..663909bb79 100644 --- a/electron/stt/modelManager.ts +++ b/electron/stt/modelManager.ts @@ -1,9 +1,10 @@ import { createHash } from "node:crypto"; import { createReadStream, existsSync } from "node:fs"; -import { mkdir, rename, rm, stat } from "node:fs/promises"; +import { mkdir, rm, stat } from "node:fs/promises"; import path from "node:path"; import { Readable } from "node:stream"; import { pipeline } from "node:stream/promises"; +import { renameWithRetry } from "../atomic-rename"; /** * Manages the lifetime of the on-disk model artifact used by the STT stack. @@ -380,7 +381,7 @@ async function ensureFile( ); } } - await rename(tmp, filePath); + await renameWithRetry(tmp, filePath); } export interface EnsureModelsOptions { From 441c9f84be06b8e3441d08b776a410f2a0f2a7b9 Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Thu, 15 Oct 2026 15:06:15 +0200 Subject: [PATCH 2/2] docs: say why the EXDEV fallback copies straight to the destination Part of #1255 --- electron/atomic-rename.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/electron/atomic-rename.ts b/electron/atomic-rename.ts index 0a70e7ba3c..cde01b225c 100644 --- a/electron/atomic-rename.ts +++ b/electron/atomic-rename.ts @@ -5,7 +5,9 @@ import fs from "node:fs/promises"; * The Microsoft Store build is an MSIX package: Windows redirects its writes under AppData * into the package's private copy of the folder. A rename between that copy and the real * folder fails with EXDEV, even inside one directory (#1255). Copying then deleting is the - * only way across. It is not atomic, so it is taken only when the rename cannot be. + * only way across. It is not atomic, so it is taken only when the rename cannot be. A + * second staging copy beside the destination would not restore atomicity: the source + * already is one, and its rename is the one that failed. * * Never for a case-only rename: on a case-insensitive disk both names are the same file, * and copying it onto itself then deleting the source would lose it.