diff --git a/apps/desktop/src/app/DesktopConnectionCatalogStore.ts b/apps/desktop/src/app/DesktopConnectionCatalogStore.ts index 3240df3636e38..e96bdbaa21a4b 100644 --- a/apps/desktop/src/app/DesktopConnectionCatalogStore.ts +++ b/apps/desktop/src/app/DesktopConnectionCatalogStore.ts @@ -12,6 +12,7 @@ import { } from "@t3tools/client-runtime/platform"; import type { PersistedSavedEnvironmentRecord } from "@t3tools/contracts"; import { fromLenientJson } from "@t3tools/shared/schemaJson"; +import { renameIntoPlace } from "@t3tools/shared/atomicWrite"; import * as Context from "effect/Context"; import * as SecureRandom from "effect/crypto/SecureRandom"; import * as Effect from "effect/Effect"; @@ -256,7 +257,8 @@ const writeDocument = Effect.fn("desktop.connectionCatalogStore.writeDocument")( }), ), ); - yield* input.fileSystem.rename(tempPath, input.catalogPath).pipe( + yield* renameIntoPlace(tempPath, input.catalogPath).pipe( + Effect.provideService(FileSystem.FileSystem, input.fileSystem), Effect.mapError( (cause) => new DesktopConnectionCatalogStoreWriteError({ diff --git a/apps/desktop/src/settings/DesktopAppSettings.test.ts b/apps/desktop/src/settings/DesktopAppSettings.test.ts index 7e4b530083097..8fed05c680b02 100644 --- a/apps/desktop/src/settings/DesktopAppSettings.test.ts +++ b/apps/desktop/src/settings/DesktopAppSettings.test.ts @@ -1,6 +1,7 @@ import * as NodeCrypto from "@effect/platform-node/NodeCrypto"; import * as NodeServices from "@effect/platform-node/NodeServices"; import { assert, describe, it } from "@effect/vitest"; +import * as HostProcess from "@t3tools/shared/HostProcess"; import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; @@ -274,7 +275,9 @@ describe("DesktopSettings", () => { recursive: true, }); - const error = yield* settings.setServerExposureMode("network-accessible").pipe(Effect.flip); + const error = yield* settings + .setServerExposureMode("network-accessible") + .pipe(Effect.provideService(HostProcess.Platform, "linux"), Effect.flip); assert.equal(error.operation, "replace-settings-file"); assert.deepEqual( diff --git a/apps/desktop/src/settings/DesktopAppSettings.ts b/apps/desktop/src/settings/DesktopAppSettings.ts index 6cd34322d98c2..5191ed19aa068 100644 --- a/apps/desktop/src/settings/DesktopAppSettings.ts +++ b/apps/desktop/src/settings/DesktopAppSettings.ts @@ -23,6 +23,7 @@ import { type LinuxPasswordStorePreference, } from "../linuxSecretStorage.ts"; import { desktopUpdateChannelOf } from "@t3tools/shared/cliRelease"; +import { renameIntoPlace } from "@t3tools/shared/atomicWrite"; import { isValidDistroName } from "../wsl/wslPathParsing.ts"; export interface DesktopSettings { @@ -453,7 +454,8 @@ const writeSettings = Effect.fn("desktop.settings.writeSettings")(function* (inp }), ), ); - yield* input.fileSystem.rename(tempPath, targetPath).pipe( + yield* renameIntoPlace(tempPath, targetPath).pipe( + Effect.provideService(FileSystem.FileSystem, input.fileSystem), Effect.mapError( (cause) => new DesktopSettingsWriteError({ diff --git a/apps/desktop/src/settings/DesktopClientSettings.test.ts b/apps/desktop/src/settings/DesktopClientSettings.test.ts index 0dab1c8d83cc6..83fe8c54f26ee 100644 --- a/apps/desktop/src/settings/DesktopClientSettings.test.ts +++ b/apps/desktop/src/settings/DesktopClientSettings.test.ts @@ -6,12 +6,16 @@ import { DEFAULT_CLIENT_SETTINGS, type ClientSettings, } from "@t3tools/contracts"; +import * as HostProcess from "@t3tools/shared/HostProcess"; +import * as Deferred from "effect/Deferred"; import * as Effect from "effect/Effect"; +import * as Fiber from "effect/Fiber"; import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; import * as PlatformError from "effect/PlatformError"; import * as Schema from "effect/Schema"; +import * as TestClock from "effect/testing/TestClock"; import * as DesktopConfig from "../app/DesktopConfig.ts"; import * as DesktopEnvironment from "../app/DesktopEnvironment.ts"; @@ -77,6 +81,32 @@ const clientSettings: ClientSettings = { }; const decodeClientSettingsJson = Schema.decodeEffect(Schema.fromJsonString(ClientSettingsSchema)); +const renameLockedOnce = (fileSystem: FileSystem.FileSystem, locked: Deferred.Deferred) => { + let renames = 0; + const busy = Object.assign(new Error("EBUSY, rename"), { code: "EBUSY" }); + return FileSystem.FileSystem.of({ + ...fileSystem, + rename: (from, to) => + Effect.suspend(() => + ++renames === 1 + ? Deferred.succeed(locked, undefined).pipe( + Effect.andThen( + Effect.fail( + PlatformError.systemError({ + _tag: "Unknown", + module: "FileSystem", + method: "rename", + pathOrDescriptor: to, + cause: busy, + }), + ), + ), + ) + : fileSystem.rename(from, to), + ), + }); +}; + function layer(baseDir: string) { const layerEnvironment = DesktopEnvironment.layer({ dirname: "/repo/apps/desktop/src", @@ -154,7 +184,9 @@ describe("DesktopClientSettings", () => { recursive: true, }); - const error = yield* settings.set(clientSettings).pipe(Effect.flip); + const error = yield* settings + .set(clientSettings) + .pipe(Effect.provideService(HostProcess.Platform, "linux"), Effect.flip); assert.equal(error.operation, "replace-settings-file"); assert.deepEqual( @@ -167,6 +199,35 @@ describe("DesktopClientSettings", () => { ), ); + it.effect("saves on Windows once a scanner lets go of the settings file", () => + withClientSettings( + Effect.gen(function* () { + const environment = yield* DesktopEnvironment.DesktopEnvironment; + const fileSystem = yield* FileSystem.FileSystem; + const locked = yield* Deferred.make(); + yield* fileSystem.makeDirectory(environment.stateDir, { recursive: true }); + yield* fileSystem.writeFileString(environment.clientSettingsPath, "{}\n"); + const settings = yield* DesktopClientSettings.make.pipe( + Effect.provideService(FileSystem.FileSystem, renameLockedOnce(fileSystem, locked)), + ); + + const save = yield* settings + .set(clientSettings) + .pipe(Effect.provideService(HostProcess.Platform, "win32"), Effect.forkChild); + yield* Deferred.await(locked); + yield* TestClock.adjust("100 millis"); + yield* Fiber.join(save); + + assert.deepEqual( + yield* decodeClientSettingsJson( + yield* fileSystem.readFileString(environment.clientSettingsPath), + ), + clientSettings, + ); + }), + ), + ); + it.effect.each([ { label: "permission", reason: "PermissionDenied" }, { label: "I/O", reason: "Unknown" }, diff --git a/apps/desktop/src/settings/DesktopClientSettings.ts b/apps/desktop/src/settings/DesktopClientSettings.ts index b840bb7a058e3..46da7b5628999 100644 --- a/apps/desktop/src/settings/DesktopClientSettings.ts +++ b/apps/desktop/src/settings/DesktopClientSettings.ts @@ -1,6 +1,7 @@ import { ClientSettingsSchema, type ClientSettings } from "@t3tools/contracts"; import { fromLenientJson } from "@t3tools/shared/schemaJson"; import { resolveSymlinkTarget } from "@t3tools/shared/symlink"; +import { renameIntoPlace } from "@t3tools/shared/atomicWrite"; import * as Context from "effect/Context"; import * as SecureRandom from "effect/crypto/SecureRandom"; import * as Effect from "effect/Effect"; @@ -173,7 +174,8 @@ const writeClientSettings = Effect.fnUntraced(function* (input: { }), ), ); - yield* input.fileSystem.rename(tempPath, targetPath).pipe( + yield* renameIntoPlace(tempPath, targetPath).pipe( + Effect.provideService(FileSystem.FileSystem, input.fileSystem), Effect.mapError( (cause) => new DesktopClientSettingsWriteError({ diff --git a/packages/shared/src/atomicWrite.ts b/packages/shared/src/atomicWrite.ts index e48755047398f..20ab0f8bd9e3b 100644 --- a/packages/shared/src/atomicWrite.ts +++ b/packages/shared/src/atomicWrite.ts @@ -17,6 +17,45 @@ export const isWindowsRenameLock = (error: PlatformError.PlatformError) => { const LOCKED_RENAME_RETRIES = 20; const LOCKED_RENAME_RETRY_DELAY = "100 millis"; +/** Renames a written temp file over its target, waiting out brief Windows locks on the target. */ +export const renameIntoPlace = Effect.fn("atomicWrite.renameIntoPlace")(function* ( + tempPath: string, + targetPath: string, +) { + const fs = yield* FileSystem.FileSystem; + const platform = yield* HostProcess.Platform; + if (platform !== "win32") return yield* fs.rename(tempPath, targetPath); + + // Stop retrying if another save changes the target; read-only targets also return permanent EPERM. + const targetVersion = fs.stat(targetPath).pipe( + Effect.map((info) => ({ + id: `${Option.getOrUndefined(info.ino)}:${info.size}:${Option.getOrUndefined(info.mtime)?.getTime()}`, + readOnly: (info.mode & 0o200) === 0, + })), + Effect.catchIf( + (error) => error.reason._tag === "NotFound", + () => Effect.succeed({ id: "missing", readOnly: false }), + ), + Effect.option, + ); + const before = yield* targetVersion; + const firstAttempt = yield* Effect.result(fs.rename(tempPath, targetPath)); + if (Result.isSuccess(firstAttempt)) return; + const lockError = firstAttempt.failure; + if (!isWindowsRenameLock(lockError) || Option.isNone(before) || before.value.readOnly) { + return yield* lockError; + } + for (let retry = 0; retry < LOCKED_RENAME_RETRIES; retry += 1) { + yield* Effect.sleep(LOCKED_RENAME_RETRY_DELAY); + const current = yield* targetVersion; + if (Option.isNone(current) || current.value.id !== before.value.id) break; + const attempt = yield* Effect.result(fs.rename(tempPath, targetPath)); + if (Result.isSuccess(attempt)) return; + if (!isWindowsRenameLock(attempt.failure)) return yield* attempt.failure; + } + return yield* lockError; +}); + /** Resolves symlink targets before renaming so writes preserve the link. */ export const writeFileStringAtomically = (input: { readonly filePath: string; @@ -26,7 +65,6 @@ export const writeFileStringAtomically = (input: { Effect.gen(function* () { const fs = yield* FileSystem.FileSystem; const path = yield* Path.Path; - const platform = yield* HostProcess.Platform; const targetPath = yield* resolveSymlinkTarget(input.filePath); const targetDirectory = path.dirname(targetPath); @@ -42,35 +80,6 @@ export const writeFileStringAtomically = (input: { const tempPath = path.join(tempDirectory, "contents.tmp"); yield* fs.writeFileString(tempPath, input.contents); - if (platform !== "win32") return yield* fs.rename(tempPath, targetPath); - - // Stop retrying if another save changes the target; read-only targets also return permanent EPERM. - const targetVersion = fs.stat(targetPath).pipe( - Effect.map((info) => ({ - id: `${Option.getOrUndefined(info.ino)}:${info.size}:${Option.getOrUndefined(info.mtime)?.getTime()}`, - readOnly: (info.mode & 0o200) === 0, - })), - Effect.catchIf( - (error) => error.reason._tag === "NotFound", - () => Effect.succeed({ id: "missing", readOnly: false }), - ), - Effect.option, - ); - const before = yield* targetVersion; - const firstAttempt = yield* Effect.result(fs.rename(tempPath, targetPath)); - if (Result.isSuccess(firstAttempt)) return; - const lockError = firstAttempt.failure; - if (!isWindowsRenameLock(lockError) || Option.isNone(before) || before.value.readOnly) { - return yield* lockError; - } - for (let retry = 0; retry < LOCKED_RENAME_RETRIES; retry += 1) { - yield* Effect.sleep(LOCKED_RENAME_RETRY_DELAY); - const current = yield* targetVersion; - if (Option.isNone(current) || current.value.id !== before.value.id) break; - const attempt = yield* Effect.result(fs.rename(tempPath, targetPath)); - if (Result.isSuccess(attempt)) return; - if (!isWindowsRenameLock(attempt.failure)) return yield* attempt.failure; - } - return yield* lockError; + yield* renameIntoPlace(tempPath, targetPath); }), );