From c5ec681a004576481c633cd6b4b70ffb7a6048e4 Mon Sep 17 00:00:00 2001 From: Kevin Rajan <7121943+kvnloo@users.noreply.github.com> Date: Fri, 16 Oct 2026 23:56:47 -0500 Subject: [PATCH 1/6] fix(web): retry a failed final file save after closing --- .../files/fileSaveCoordinator.test.ts | 67 ++++++++++++++++++- .../components/files/fileSaveCoordinator.ts | 8 ++- 2 files changed, 71 insertions(+), 4 deletions(-) diff --git a/apps/web/src/components/files/fileSaveCoordinator.test.ts b/apps/web/src/components/files/fileSaveCoordinator.test.ts index 9871553c81f5a..ae770aeeec7aa 100644 --- a/apps/web/src/components/files/fileSaveCoordinator.test.ts +++ b/apps/web/src/components/files/fileSaveCoordinator.test.ts @@ -5,9 +5,9 @@ import { AsyncResult } from "effect/reactivity"; import { FileSaveCoordinator } from "./fileSaveCoordinator"; -function deferred() { - let resolve!: (result: AtomCommandResult) => void; - const promise = new Promise>((resolvePromise) => { +function deferred() { + let resolve!: (result: AtomCommandResult) => void; + const promise = new Promise>((resolvePromise) => { resolve = resolvePromise; }); return { promise, resolve }; @@ -209,6 +209,67 @@ describe("FileSaveCoordinator", () => { expect(onPendingChange).not.toHaveBeenCalledWith(false); }); + it("retries a close-time flush when the in-flight write fails", async () => { + vi.useFakeTimers(); + const inFlight = deferred(); + const persist = vi + .fn() + .mockReturnValueOnce(inFlight.promise) + .mockResolvedValueOnce(AsyncResult.failure(Cause.fail(new Error("write failed")))) + .mockResolvedValue(AsyncResult.success(undefined)); + const onPendingChange = vi.fn(); + const onConfirmed = vi.fn(); + const coordinator = new FileSaveCoordinator({ + debounceMs: 500, + persist, + onPendingChange, + onConfirmed, + }); + + coordinator.change("only"); + await vi.advanceTimersByTimeAsync(500); + expect(persist).toHaveBeenCalledTimes(1); + + // The editor closes while the write is still in flight; the write then fails. + coordinator.dispose(); + inFlight.resolve(AsyncResult.failure(Cause.fail(new Error("write failed")))); + await vi.advanceTimersByTimeAsync(500); + // The failed close-time flush must be retried, not dropped. + expect(persist).toHaveBeenCalledTimes(2); + expect(persist).toHaveBeenLastCalledWith("only"); + + await vi.advanceTimersByTimeAsync(500); + expect(persist).toHaveBeenCalledTimes(3); + expect(onConfirmed).toHaveBeenCalledWith("only"); + expect(onPendingChange.mock.calls.at(-1)).toEqual([false]); + }); + + it("retries a dispose-time flush that fails after the editor closes", async () => { + vi.useFakeTimers(); + const persist = vi + .fn() + .mockResolvedValueOnce(AsyncResult.failure(Cause.fail(new Error("write failed")))) + .mockResolvedValueOnce(AsyncResult.failure(Cause.fail(new Error("write failed")))) + .mockResolvedValue(AsyncResult.success(undefined)); + const onPendingChange = vi.fn(); + const onConfirmed = vi.fn(); + const coordinator = new FileSaveCoordinator({ + debounceMs: 500, + persist, + onPendingChange, + onConfirmed, + }); + + coordinator.change("unsaved"); + coordinator.dispose(); + await vi.runAllTimersAsync(); + // First dispose flush failed; it must be retried instead of dropping the edit. + expect(persist.mock.calls.length).toBeGreaterThanOrEqual(2); + expect(persist).toHaveBeenLastCalledWith("unsaved"); + expect(onConfirmed).toHaveBeenCalledWith("unsaved"); + expect(onPendingChange.mock.calls.at(-1)).toEqual([false]); + }); + it("ignores editor changes emitted after disposal", async () => { vi.useFakeTimers(); const persist = vi diff --git a/apps/web/src/components/files/fileSaveCoordinator.ts b/apps/web/src/components/files/fileSaveCoordinator.ts index aa907cdc77536..6ea1a360739d1 100644 --- a/apps/web/src/components/files/fileSaveCoordinator.ts +++ b/apps/web/src/components/files/fileSaveCoordinator.ts @@ -68,7 +68,13 @@ export class FileSaveCoordinator { this.saving = false; if (revision === this.latestRevision) { - if (confirmed) this.options.onPendingChange(false); + if (confirmed) { + this.options.onPendingChange(false); + } else if (!succeeded && this.disposed) { + // Closing the editor must not discard an edit whose final save failed. + // Retry after the normal debounce; a successful write stops the retry. + this.schedule(this.options.debounceMs); + } return; } From da6e9d98f08b31769d1e86ffd930c0f4ef02c156 Mon Sep 17 00:00:00 2001 From: Kevin Rajan <7121943+kvnloo@users.noreply.github.com> Date: Sat, 17 Oct 2026 00:00:46 -0500 Subject: [PATCH 2/6] fix(web): skip stale close-time retry after a newer draft --- .../files/fileSaveCoordinator.test.ts | 26 +++++++++++++++++++ .../components/files/fileSaveCoordinator.ts | 12 +++++++++ .../files/useFileSaveCoordinator.ts | 2 ++ 3 files changed, 40 insertions(+) diff --git a/apps/web/src/components/files/fileSaveCoordinator.test.ts b/apps/web/src/components/files/fileSaveCoordinator.test.ts index ae770aeeec7aa..e2f1b4ca530a4 100644 --- a/apps/web/src/components/files/fileSaveCoordinator.test.ts +++ b/apps/web/src/components/files/fileSaveCoordinator.test.ts @@ -270,6 +270,32 @@ describe("FileSaveCoordinator", () => { expect(onPendingChange.mock.calls.at(-1)).toEqual([false]); }); + it("skips a closed editor retry after another edit replaces its draft", async () => { + vi.useFakeTimers(); + let currentDraft = "old"; + const persist = vi + .fn() + .mockResolvedValueOnce(AsyncResult.failure(Cause.fail(new Error("write failed")))) + .mockResolvedValue(AsyncResult.success(undefined)); + const shouldRetryAfterClose = vi.fn((contents: string) => contents === currentDraft); + const coordinator = new FileSaveCoordinator({ + debounceMs: 500, + persist, + onPendingChange: vi.fn(), + onConfirmed: vi.fn(), + shouldRetryAfterClose, + }); + + coordinator.change("old"); + coordinator.dispose(); + await vi.advanceTimersByTimeAsync(0); + currentDraft = "new"; + await vi.advanceTimersByTimeAsync(500); + + expect(shouldRetryAfterClose).toHaveBeenCalledWith("old"); + expect(persist).toHaveBeenCalledExactlyOnceWith("old"); + }); + it("ignores editor changes emitted after disposal", async () => { vi.useFakeTimers(); const persist = vi diff --git a/apps/web/src/components/files/fileSaveCoordinator.ts b/apps/web/src/components/files/fileSaveCoordinator.ts index 6ea1a360739d1..2e755b631c1a9 100644 --- a/apps/web/src/components/files/fileSaveCoordinator.ts +++ b/apps/web/src/components/files/fileSaveCoordinator.ts @@ -3,6 +3,8 @@ import type { AtomCommandResult } from "@t3tools/client-runtime/state/runtime"; export interface FileSaveCoordinatorOptions { readonly debounceMs: number; readonly canPersist?: () => boolean; + /** Refuse stale close-time retries after another editor replaces the draft. */ + readonly shouldRetryAfterClose?: (contents: string) => boolean; readonly persist: (contents: string) => Promise>; readonly onPendingChange: (pending: boolean) => void; /** Return false when another editor has newer unsaved contents. */ @@ -17,6 +19,7 @@ export class FileSaveCoordinator { private lastChangeAt = 0; private saving = false; private disposed = false; + private closeRetryPending = false; constructor(private readonly options: FileSaveCoordinatorOptions) {} @@ -55,6 +58,14 @@ export class FileSaveCoordinator { return; } + if ( + this.closeRetryPending && + this.options.shouldRetryAfterClose?.(this.latestContents) === false + ) { + return; + } + this.closeRetryPending = false; + this.saving = true; const contents = this.latestContents; const revision = this.latestRevision; @@ -73,6 +84,7 @@ export class FileSaveCoordinator { } else if (!succeeded && this.disposed) { // Closing the editor must not discard an edit whose final save failed. // Retry after the normal debounce; a successful write stops the retry. + this.closeRetryPending = true; this.schedule(this.options.debounceMs); } return; diff --git a/apps/web/src/components/files/useFileSaveCoordinator.ts b/apps/web/src/components/files/useFileSaveCoordinator.ts index 897e72627f039..1c7a7b04a74a2 100644 --- a/apps/web/src/components/files/useFileSaveCoordinator.ts +++ b/apps/web/src/components/files/useFileSaveCoordinator.ts @@ -85,6 +85,8 @@ function createFileSaveSession({ const coordinator = new FileSaveCoordinator({ debounceMs: FILE_SAVE_DEBOUNCE_MS, canPersist: () => readEnvironmentScope(environmentId, AuthFilesystemWriteScope), + shouldRetryAfterClose: (contents) => + getUnsavedProjectFileQueryData(environmentId, cwd, relativePath)?.contents === contents, onPendingChange: (pending) => onPendingChange(relativePath, pending), persist: (nextContents) => writeFile({ From e62985e0f5a40506c68cda87790db44b918317c1 Mon Sep 17 00:00:00 2001 From: Kevin Rajan <7121943+kvnloo@users.noreply.github.com> Date: Sat, 17 Oct 2026 01:14:52 -0500 Subject: [PATCH 3/6] fix(web): bound failed close-save retries with backoff Retain pending drafts on exhaustion or permission loss; add deterministic timer regressions. AI assistance: GPT-6 / ChatGPT. --- .../files/fileSaveCoordinator.test.ts | 58 ++++++++++++++++++- .../components/files/fileSaveCoordinator.ts | 20 +++++-- 2 files changed, 73 insertions(+), 5 deletions(-) diff --git a/apps/web/src/components/files/fileSaveCoordinator.test.ts b/apps/web/src/components/files/fileSaveCoordinator.test.ts index e2f1b4ca530a4..bf9befab784ba 100644 --- a/apps/web/src/components/files/fileSaveCoordinator.test.ts +++ b/apps/web/src/components/files/fileSaveCoordinator.test.ts @@ -238,7 +238,9 @@ describe("FileSaveCoordinator", () => { expect(persist).toHaveBeenCalledTimes(2); expect(persist).toHaveBeenLastCalledWith("only"); - await vi.advanceTimersByTimeAsync(500); + await vi.advanceTimersByTimeAsync(999); + expect(persist).toHaveBeenCalledTimes(2); + await vi.advanceTimersByTimeAsync(1); expect(persist).toHaveBeenCalledTimes(3); expect(onConfirmed).toHaveBeenCalledWith("only"); expect(onPendingChange.mock.calls.at(-1)).toEqual([false]); @@ -296,6 +298,60 @@ describe("FileSaveCoordinator", () => { expect(persist).toHaveBeenCalledExactlyOnceWith("old"); }); + it("caps close-time retries with backoff and preserves an unsaved draft", async () => { + vi.useFakeTimers(); + const onPendingChange = vi.fn(); + const persist = vi + .fn() + .mockResolvedValue(AsyncResult.failure(Cause.fail(new Error("write unavailable")))); + const coordinator = new FileSaveCoordinator({ + debounceMs: 500, + persist, + onPendingChange, + onConfirmed: vi.fn(), + }); + + coordinator.change("unsaved"); + coordinator.dispose(); + await vi.advanceTimersByTimeAsync(0); + expect(persist).toHaveBeenCalledTimes(1); + + await vi.advanceTimersByTimeAsync(500); + expect(persist).toHaveBeenCalledTimes(2); + await vi.advanceTimersByTimeAsync(999); + expect(persist).toHaveBeenCalledTimes(2); + await vi.advanceTimersByTimeAsync(1); + expect(persist).toHaveBeenCalledTimes(3); + await vi.advanceTimersByTimeAsync(1_999); + expect(persist).toHaveBeenCalledTimes(3); + await vi.advanceTimersByTimeAsync(1); + expect(persist).toHaveBeenCalledTimes(4); + await vi.advanceTimersByTimeAsync(30_000); + expect(persist).toHaveBeenCalledTimes(4); + expect(onPendingChange).toHaveBeenLastCalledWith(true); + }); + + it("stops retrying if write permission was revoked after the failure", async () => { + vi.useFakeTimers(); + let canWrite = true; + const persist = vi.fn().mockImplementation(async () => { + canWrite = false; + return AsyncResult.failure(Cause.fail(new Error("write forbidden"))); + }); + const coordinator = new FileSaveCoordinator({ + debounceMs: 500, + canPersist: () => canWrite, + persist, + onPendingChange: vi.fn(), + onConfirmed: vi.fn(), + }); + + coordinator.change("unsaved"); + coordinator.dispose(); + await vi.runAllTimersAsync(); + expect(persist).toHaveBeenCalledOnce(); + }); + it("ignores editor changes emitted after disposal", async () => { vi.useFakeTimers(); const persist = vi diff --git a/apps/web/src/components/files/fileSaveCoordinator.ts b/apps/web/src/components/files/fileSaveCoordinator.ts index 2e755b631c1a9..d39626287e78d 100644 --- a/apps/web/src/components/files/fileSaveCoordinator.ts +++ b/apps/web/src/components/files/fileSaveCoordinator.ts @@ -1,5 +1,8 @@ import type { AtomCommandResult } from "@t3tools/client-runtime/state/runtime"; +// Avoid unbounded retry traffic after a closed editor loses write access or its filesystem stays unavailable. +const MAX_CLOSE_RETRIES = 3; + export interface FileSaveCoordinatorOptions { readonly debounceMs: number; readonly canPersist?: () => boolean; @@ -20,6 +23,7 @@ export class FileSaveCoordinator { private saving = false; private disposed = false; private closeRetryPending = false; + private closeRetryFailures = 0; constructor(private readonly options: FileSaveCoordinatorOptions) {} @@ -80,12 +84,20 @@ export class FileSaveCoordinator { this.saving = false; if (revision === this.latestRevision) { if (confirmed) { + this.closeRetryFailures = 0; this.options.onPendingChange(false); - } else if (!succeeded && this.disposed) { - // Closing the editor must not discard an edit whose final save failed. - // Retry after the normal debounce; a successful write stops the retry. + } else if ( + !succeeded && + this.disposed && + this.closeRetryFailures < MAX_CLOSE_RETRIES && + this.options.canPersist?.() !== false + ) { + // A permanent failure must not leave a disposed editor retrying forever. + // Keep the unsaved draft pending after the bounded retry budget expires. this.closeRetryPending = true; - this.schedule(this.options.debounceMs); + const retryDelay = this.options.debounceMs * 2 ** this.closeRetryFailures; + this.closeRetryFailures += 1; + this.schedule(retryDelay); } return; } From 9702a1b86ef90e193d835a967ce4ee3409fa604e Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Mon, 26 Oct 2026 14:01:23 +0000 Subject: [PATCH 4/6] fix(files): reject stale final-save retries across clients --- .../src/workspace/WorkspaceFileSystem.test.ts | 44 +++++++++ .../src/workspace/WorkspaceFileSystem.ts | 87 +++++++++------- .../files/fileSaveCoordinator.test.ts | 35 +------ .../components/files/fileSaveCoordinator.ts | 22 ++--- .../files/projectFilesQueryState.ts | 12 +++ .../useFileSaveCoordinator.browser.test.tsx | 99 ++++++++++++++++++- .../files/useFileSaveCoordinator.ts | 33 ++++++- packages/contracts/src/project.ts | 4 + 8 files changed, 249 insertions(+), 87 deletions(-) diff --git a/apps/server/src/workspace/WorkspaceFileSystem.test.ts b/apps/server/src/workspace/WorkspaceFileSystem.test.ts index 0cf5524df4e17..8fc2120ddaa2c 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.test.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.test.ts @@ -181,6 +181,7 @@ it.layer(layerTest, { excludeTestServices: true })("WorkspaceFileSystemLive", (i contents: "# Report\n", byteLength: 9, truncated: false, + supportsConditionalWrite: true, }); }), ); @@ -202,6 +203,7 @@ it.layer(layerTest, { excludeTestServices: true })("WorkspaceFileSystemLive", (i contents: "# Notes\n", byteLength: 8, truncated: false, + supportsConditionalWrite: true, }); }), ); @@ -369,6 +371,48 @@ it.layer(layerTest, { excludeTestServices: true })("WorkspaceFileSystemLive", (i }); describe("writeFile", () => { + it.effect("refuses a retired save after another client has saved newer contents", () => + Effect.gen(function* () { + const service = yield* WorkspaceFileSystem.WorkspaceFileSystem; + const cwd = yield* makeTempDir; + yield* writeTextFile(cwd, "note.txt", "original"); + yield* service.writeFile({ cwd, relativePath: "note.txt", contents: "newer" }); + const retry = { + cwd, + relativePath: "note.txt", + contents: "retired", + expectedContents: "original", + }; + expect(yield* service.writeFile(retry)).toEqual({ + relativePath: "note.txt", + conflict: true, + }); + expect((yield* service.readFile({ cwd, relativePath: "note.txt" })).contents).toBe("newer"); + }), + ); + + it.effect("checks conditional saves in the same critical section as competing writes", () => + Effect.gen(function* () { + const service = yield* WorkspaceFileSystem.WorkspaceFileSystem; + const cwd = yield* makeTempDir; + yield* writeTextFile(cwd, "note.txt", "original"); + const results = yield* Effect.forEach( + ["first", "second"], + (contents) => { + const input = { cwd, relativePath: "note.txt", contents, expectedContents: "original" }; + return service.writeFile(input); + }, + { concurrency: "unbounded" }, + ); + expect(results).toEqual( + expect.arrayContaining([ + { relativePath: "note.txt" }, + { relativePath: "note.txt", conflict: true }, + ]), + ); + }), + ); + it.effect("rejects writes outside a known workspace", () => Effect.gen(function* () { const workspaceFileSystem = yield* WorkspaceFileSystem.WorkspaceFileSystem; diff --git a/apps/server/src/workspace/WorkspaceFileSystem.ts b/apps/server/src/workspace/WorkspaceFileSystem.ts index 7107cd99631a8..8b3d071bd2130 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.ts @@ -25,6 +25,7 @@ import * as KnownWorkspaces from "./KnownWorkspaces.ts"; import * as WorkspaceEntries from "./WorkspaceEntries.ts"; import * as WorkspacePaths from "./WorkspacePaths.ts"; import { fileHeaderMimeType } from "./fileHeaderMimeType.ts"; +import { withWorkspaceLease } from "./workspaceLease.ts"; const PROJECT_READ_FILE_MAX_BYTES = 1024 * 1024; @@ -344,6 +345,7 @@ export const make = Effect.gen(function* () { byteLength: 0, truncated: false, exists: false, + supportsConditionalWrite: true, }; const realTargetPath = target.realTargetPath; if (realTargetPath === null) return absentResult; @@ -423,6 +425,7 @@ export const make = Effect.gen(function* () { contents: new TextDecoder("utf-8").decode(fileBytes), byteLength: stat.size, truncated: stat.size > PROJECT_READ_FILE_MAX_BYTES, + supportsConditionalWrite: true, }; }), (handle) => @@ -532,41 +535,59 @@ export const make = Effect.gen(function* () { const target = yield* resolveWriteTarget(input); const realTargetPath = target.realTargetPath; - yield* fileSystem.makeDirectory(path.dirname(realTargetPath), { recursive: true }).pipe( - Effect.mapError( - (cause) => - new WorkspaceFileSystemOperationError({ - workspaceRoot: input.cwd, + return yield* withWorkspaceLease( + realTargetPath, + Effect.gen(function* () { + if (input.expectedContents !== undefined) { + const current = yield* readFile({ + cwd: input.cwd, relativePath: input.relativePath, - resolvedPath: realTargetPath, - operationPath: path.dirname(realTargetPath), - operation: "make-directory", - cause, - }), - ), + allowMissing: true, + }); + if ( + current.truncated || + (current.exists === false ? null : current.contents) !== input.expectedContents + ) { + return { relativePath: target.relativePath, conflict: true }; + } + } + yield* fileSystem.makeDirectory(path.dirname(realTargetPath), { recursive: true }).pipe( + Effect.mapError( + (cause) => + new WorkspaceFileSystemOperationError({ + workspaceRoot: input.cwd, + relativePath: input.relativePath, + resolvedPath: realTargetPath, + operationPath: path.dirname(realTargetPath), + operation: "make-directory", + cause, + }), + ), + ); + yield* Effect.tryPromise({ + // A symlink swapped in after the check is not followed. Windows lacks O_NOFOLLOW. + try: () => + NodeFSP.writeFile(realTargetPath, input.contents, { + flag: + NodeFS.constants.O_WRONLY | + NodeFS.constants.O_CREAT | + NodeFS.constants.O_TRUNC | + (NodeFS.constants.O_NOFOLLOW ?? 0), + }), + catch: (cause) => + new WorkspaceFileSystemOperationError({ + workspaceRoot: input.cwd, + relativePath: input.relativePath, + resolvedPath: realTargetPath, + operationPath: realTargetPath, + operation: "write-file", + cause, + }), + }); + yield* workspaceEntries.refresh(input.cwd); + return { relativePath: target.relativePath }; + }), ); - yield* Effect.tryPromise({ - // A symlink swapped in after the check is not followed. Windows lacks O_NOFOLLOW. - try: () => - NodeFSP.writeFile(realTargetPath, input.contents, { - flag: - NodeFS.constants.O_WRONLY | - NodeFS.constants.O_CREAT | - NodeFS.constants.O_TRUNC | - (NodeFS.constants.O_NOFOLLOW ?? 0), - }), - catch: (cause) => - new WorkspaceFileSystemOperationError({ - workspaceRoot: input.cwd, - relativePath: input.relativePath, - resolvedPath: realTargetPath, - operationPath: realTargetPath, - operation: "write-file", - cause, - }), - }); - yield* workspaceEntries.refresh(input.cwd); - return { relativePath: target.relativePath }; }); return WorkspaceFileSystem.of({ getMetadata, readFile, writeFile }); diff --git a/apps/web/src/components/files/fileSaveCoordinator.test.ts b/apps/web/src/components/files/fileSaveCoordinator.test.ts index 1ebfdcc20c562..83417f842ddd9 100644 --- a/apps/web/src/components/files/fileSaveCoordinator.test.ts +++ b/apps/web/src/components/files/fileSaveCoordinator.test.ts @@ -180,13 +180,11 @@ describe("FileSaveCoordinator", () => { await vi.advanceTimersByTimeAsync(500); expect(persist).toHaveBeenCalledTimes(1); - // The editor closes while the write is still in flight; the write then fails. coordinator.dispose(); inFlight.resolve(AsyncResult.failure(Cause.fail(new Error("write failed")))); await vi.advanceTimersByTimeAsync(500); - // The failed close-time flush must be retried, not dropped. expect(persist).toHaveBeenCalledTimes(2); - expect(persist).toHaveBeenLastCalledWith("only"); + expect(persist).toHaveBeenLastCalledWith("only", true); await vi.advanceTimersByTimeAsync(999); expect(persist).toHaveBeenCalledTimes(2); @@ -215,39 +213,12 @@ describe("FileSaveCoordinator", () => { coordinator.change("unsaved"); coordinator.dispose(); await vi.runAllTimersAsync(); - // First dispose flush failed; it must be retried instead of dropping the edit. - expect(persist.mock.calls.length).toBeGreaterThanOrEqual(2); - expect(persist).toHaveBeenLastCalledWith("unsaved"); + expect(persist).toHaveBeenCalledTimes(3); + expect(persist).toHaveBeenLastCalledWith("unsaved", true); expect(onConfirmed).toHaveBeenCalledWith("unsaved"); expect(onPendingChange.mock.calls.at(-1)).toEqual([false]); }); - it("skips a closed editor retry after another edit replaces its draft", async () => { - vi.useFakeTimers(); - let currentDraft = "old"; - const persist = vi - .fn() - .mockResolvedValueOnce(AsyncResult.failure(Cause.fail(new Error("write failed")))) - .mockResolvedValue(AsyncResult.success(undefined)); - const shouldRetryAfterClose = vi.fn((contents: string) => contents === currentDraft); - const coordinator = new FileSaveCoordinator({ - debounceMs: 500, - persist, - onPendingChange: vi.fn(), - onConfirmed: vi.fn(), - shouldRetryAfterClose, - }); - - coordinator.change("old"); - coordinator.dispose(); - await vi.advanceTimersByTimeAsync(0); - currentDraft = "new"; - await vi.advanceTimersByTimeAsync(500); - - expect(shouldRetryAfterClose).toHaveBeenCalledWith("old"); - expect(persist).toHaveBeenCalledExactlyOnceWith("old"); - }); - it("caps close-time retries with backoff and preserves an unsaved draft", async () => { vi.useFakeTimers(); const onPendingChange = vi.fn(); diff --git a/apps/web/src/components/files/fileSaveCoordinator.ts b/apps/web/src/components/files/fileSaveCoordinator.ts index d39626287e78d..a44f3ef9416d9 100644 --- a/apps/web/src/components/files/fileSaveCoordinator.ts +++ b/apps/web/src/components/files/fileSaveCoordinator.ts @@ -1,6 +1,5 @@ import type { AtomCommandResult } from "@t3tools/client-runtime/state/runtime"; -// Avoid unbounded retry traffic after a closed editor loses write access or its filesystem stays unavailable. const MAX_CLOSE_RETRIES = 3; export interface FileSaveCoordinatorOptions { @@ -8,7 +7,7 @@ export interface FileSaveCoordinatorOptions { readonly canPersist?: () => boolean; /** Refuse stale close-time retries after another editor replaces the draft. */ readonly shouldRetryAfterClose?: (contents: string) => boolean; - readonly persist: (contents: string) => Promise>; + readonly persist: (contents: string, retry?: boolean) => Promise>; readonly onPendingChange: (pending: boolean) => void; /** Return false when another editor has newer unsaved contents. */ readonly onConfirmed: (contents: string) => boolean | void; @@ -22,7 +21,6 @@ export class FileSaveCoordinator { private lastChangeAt = 0; private saving = false; private disposed = false; - private closeRetryPending = false; private closeRetryFailures = 0; constructor(private readonly options: FileSaveCoordinatorOptions) {} @@ -63,17 +61,18 @@ export class FileSaveCoordinator { } if ( - this.closeRetryPending && + this.closeRetryFailures > 0 && this.options.shouldRetryAfterClose?.(this.latestContents) === false ) { return; } - this.closeRetryPending = false; this.saving = true; const contents = this.latestContents; const revision = this.latestRevision; - const result = await this.options.persist(contents); + const result = await (this.closeRetryFailures > 0 + ? this.options.persist(contents, true) + : this.options.persist(contents)); const succeeded = result._tag === "Success"; let confirmed = false; if (succeeded) { @@ -84,17 +83,8 @@ export class FileSaveCoordinator { this.saving = false; if (revision === this.latestRevision) { if (confirmed) { - this.closeRetryFailures = 0; this.options.onPendingChange(false); - } else if ( - !succeeded && - this.disposed && - this.closeRetryFailures < MAX_CLOSE_RETRIES && - this.options.canPersist?.() !== false - ) { - // A permanent failure must not leave a disposed editor retrying forever. - // Keep the unsaved draft pending after the bounded retry budget expires. - this.closeRetryPending = true; + } else if (!succeeded && this.disposed && this.closeRetryFailures < MAX_CLOSE_RETRIES) { const retryDelay = this.options.debounceMs * 2 ** this.closeRetryFailures; this.closeRetryFailures += 1; this.schedule(retryDelay); diff --git a/apps/web/src/components/files/projectFilesQueryState.ts b/apps/web/src/components/files/projectFilesQueryState.ts index 3da7fdd6da850..e1a47705317d2 100644 --- a/apps/web/src/components/files/projectFilesQueryState.ts +++ b/apps/web/src/components/files/projectFilesQueryState.ts @@ -131,6 +131,18 @@ export function getUnsavedProjectFileQueryData( return optimistic?.confirmedAgainst === undefined ? (optimistic?.data ?? null) : null; } +export function getPersistedProjectFileQueryData( + environmentId: EnvironmentId, + cwd: string, + relativePath: string, +): ProjectReadFileResult | undefined { + return Option.getOrUndefined( + AsyncResult.value( + appAtomRegistry.get(getProjectFileQueryAtom(environmentId, cwd, relativePath)), + ), + ); +} + export function confirmProjectFileQueryData( environmentId: EnvironmentId, cwd: string, diff --git a/apps/web/src/components/files/useFileSaveCoordinator.browser.test.tsx b/apps/web/src/components/files/useFileSaveCoordinator.browser.test.tsx index b04512fa9f9db..5574f93b1f054 100644 --- a/apps/web/src/components/files/useFileSaveCoordinator.browser.test.tsx +++ b/apps/web/src/components/files/useFileSaveCoordinator.browser.test.tsx @@ -1,14 +1,16 @@ import { render, fakeTimers } from "../../../test/render"; import { EnvironmentId } from "@t3tools/contracts"; +import * as Cause from "effect/Cause"; import { AsyncResult } from "effect/reactivity"; import { StrictMode, useEffect } from "react"; import { describe, expect, it, vi } from "vite-plus/test"; -const { writeFile, confirmFile, readScope, getUnsavedFile } = vi.hoisted(() => ({ +const { writeFile, confirmFile, readScope, getUnsavedFile, getPersistedFile } = vi.hoisted(() => ({ writeFile: vi.fn(), confirmFile: vi.fn(), readScope: vi.fn(), getUnsavedFile: vi.fn(), + getPersistedFile: vi.fn(), })); vi.mock(import("~/state/session"), async (original) => ({ ...(await original()), @@ -23,6 +25,7 @@ vi.mock(import("./projectFilesQueryState"), async (original) => ({ ...(await original()), confirmProjectFileQueryData: confirmFile, getUnsavedProjectFileQueryData: getUnsavedFile, + getPersistedProjectFileQueryData: getPersistedFile, })); import { useFileSaveCoordinator } from "./useFileSaveCoordinator"; @@ -36,10 +39,13 @@ const defaultProps = { onPendingChange, }; function setup() { - writeFile.mockReset().mockResolvedValue(AsyncResult.success(undefined)); + writeFile.mockReset().mockResolvedValue(AsyncResult.success({ relativePath: "file.txt" })); confirmFile.mockReset(); readScope.mockReset().mockReturnValue(true); getUnsavedFile.mockReset().mockReturnValue(null); + getPersistedFile + .mockReset() + .mockReturnValue({ contents: "original", supportsConditionalWrite: true }); onPendingChange.mockReset(); } @@ -67,6 +73,95 @@ async function mount(props = defaultProps) { } describe("file-save React lifecycle", () => { + it("retains the draft and stops when the server refuses a stale retry", async () => { + setup(); + using _timers = fakeTimers(); + await using screen = await mount(); + getUnsavedFile.mockReturnValue({ contents: "pending edit" }); + writeFile + .mockResolvedValueOnce(AsyncResult.failure(Cause.fail(new Error("unavailable")))) + .mockResolvedValueOnce(AsyncResult.success({ relativePath: "file.txt", conflict: true })); + screen.changeHandler()("pending edit"); + await screen.unmount(); + await vi.runAllTimersAsync(); + expect(writeFile).toHaveBeenCalledTimes(2); + expect(confirmFile).not.toHaveBeenCalled(); + expect(onPendingChange).toHaveBeenLastCalledWith("file.txt", true); + }); + + it("does not retry against an older server without conditional writes", async () => { + setup(); + using _timers = fakeTimers(); + await using screen = await mount(); + getPersistedFile.mockReturnValue({ contents: "original" }); + getUnsavedFile.mockReturnValue({ contents: "pending edit" }); + writeFile.mockResolvedValueOnce(AsyncResult.failure(Cause.fail(new Error("unavailable")))); + screen.changeHandler()("pending edit"); + await screen.unmount(); + await vi.runAllTimersAsync(); + expect(writeFile).toHaveBeenCalledTimes(1); + expect(onPendingChange).toHaveBeenLastCalledWith("file.txt", true); + }); + + it("retries a failed final save after unmount and confirms the retained draft", async () => { + setup(); + using _timers = fakeTimers(); + await using screen = await mount(); + getUnsavedFile.mockReturnValue({ contents: "pending edit" }); + writeFile.mockResolvedValueOnce(AsyncResult.failure(Cause.fail(new Error("unavailable")))); + screen.changeHandler()("pending edit"); + await screen.unmount(); + await vi.advanceTimersByTimeAsync(500); + expect(writeFile).toHaveBeenCalledTimes(2); + expect(writeFile).toHaveBeenLastCalledWith({ + environmentId, + input: { + cwd: "/workspace", + relativePath: "file.txt", + contents: "pending edit", + expectedContents: "original", + }, + }); + expect(confirmFile).toHaveBeenCalledExactlyOnceWith( + environmentId, + "/workspace", + "file.txt", + "pending edit", + ); + expect(onPendingChange).toHaveBeenLastCalledWith("file.txt", false); + }); + + it.each([{ contents: "newer edit" }, null])( + "cancels a retired retry when its draft is replaced or confirmed: %j", + async (draft) => { + setup(); + using _timers = fakeTimers(); + await using screen = await mount(); + getUnsavedFile.mockReturnValue({ contents: "pending edit" }); + writeFile.mockResolvedValueOnce(AsyncResult.failure(Cause.fail(new Error("unavailable")))); + screen.changeHandler()("pending edit"); + await screen.unmount(); + await vi.advanceTimersByTimeAsync(0); + getUnsavedFile.mockReturnValue(draft); + await vi.runAllTimersAsync(); + expect(writeFile).toHaveBeenCalledTimes(1); + expect(confirmFile).not.toHaveBeenCalled(); + expect(onPendingChange).toHaveBeenLastCalledWith("file.txt", true); + }, + ); + + it("does not retry a successful final write declined by draft confirmation", async () => { + setup(); + using _timers = fakeTimers(); + await using screen = await mount(); + confirmFile.mockReturnValue(false); + screen.changeHandler()("pending edit"); + await screen.unmount(); + await vi.runAllTimersAsync(); + expect(writeFile).toHaveBeenCalledTimes(1); + expect(onPendingChange).toHaveBeenLastCalledWith("file.txt", true); + }); + it("persists editor model changes after StrictMode setup replay", async () => { setup(); using _timers = fakeTimers(); diff --git a/apps/web/src/components/files/useFileSaveCoordinator.ts b/apps/web/src/components/files/useFileSaveCoordinator.ts index 7ad5db0b7711c..252d9760d559b 100644 --- a/apps/web/src/components/files/useFileSaveCoordinator.ts +++ b/apps/web/src/components/files/useFileSaveCoordinator.ts @@ -8,6 +8,7 @@ import { useAtomCommand } from "~/state/use-atom-command"; import { FileSaveCoordinator } from "./fileSaveCoordinator"; import { confirmProjectFileQueryData, + getPersistedProjectFileQueryData, getUnsavedProjectFileQueryData, } from "./projectFilesQueryState"; @@ -79,18 +80,42 @@ function createFileSaveSession({ return { change: (contents: string) => coordinatorRef.current?.change(contents), setup: () => { + let conflict = false; + let expectedContents: string | null | undefined; const coordinator = new FileSaveCoordinator({ debounceMs: FILE_SAVE_DEBOUNCE_MS, canPersist: () => readEnvironmentScope(environmentId, AuthFilesystemWriteScope), shouldRetryAfterClose: (contents) => + expectedContents !== undefined && getUnsavedProjectFileQueryData(environmentId, cwd, relativePath)?.contents === contents, onPendingChange: (pending) => onPendingChange(relativePath, pending), - persist: (nextContents) => - writeFile({ + persist: async (nextContents, retry) => { + if (!retry) { + const base = getPersistedProjectFileQueryData(environmentId, cwd, relativePath); + expectedContents = + base?.supportsConditionalWrite && !base.truncated + ? base.exists === false + ? null + : base.contents + : undefined; + } + const result = await writeFile({ environmentId, - input: { cwd, relativePath, contents: nextContents }, - }), + input: { + cwd, + relativePath, + contents: nextContents, + ...(retry && expectedContents !== undefined ? { expectedContents } : {}), + }, + }); + if (result._tag === "Success") { + conflict = result.value.conflict === true; + if (!conflict && expectedContents !== undefined) expectedContents = nextContents; + } + return result; + }, onConfirmed: (confirmedContents) => + !conflict && confirmProjectFileQueryData(environmentId, cwd, relativePath, confirmedContents), }); coordinatorRef.current = coordinator; diff --git a/packages/contracts/src/project.ts b/packages/contracts/src/project.ts index 2647519df10aa..e8d7ff27f483d 100644 --- a/packages/contracts/src/project.ts +++ b/packages/contracts/src/project.ts @@ -447,6 +447,8 @@ export const ProjectReadFileResult = Schema.Struct({ truncated: Schema.Boolean, // Omitted by older servers; false only for an absent optional file. exists: Schema.optional(Schema.Boolean), + /** Older servers omit this and cannot safely retry closed-editor saves. */ + supportsConditionalWrite: Schema.optionalKey(Schema.Boolean), }); export type ProjectReadFileResult = typeof ProjectReadFileResult.Type; @@ -511,11 +513,13 @@ export const ProjectWriteFileInput = Schema.Struct({ cwd: TrimmedNonEmptyString, relativePath: TrimmedNonEmptyString.check(Schema.isMaxLength(PROJECT_WRITE_FILE_PATH_MAX_LENGTH)), contents: Schema.String, + expectedContents: Schema.optionalKey(Schema.NullOr(Schema.String)), }); export type ProjectWriteFileInput = typeof ProjectWriteFileInput.Type; export const ProjectWriteFileResult = Schema.Struct({ relativePath: TrimmedNonEmptyString, + conflict: Schema.optionalKey(Schema.Boolean), }); export type ProjectWriteFileResult = typeof ProjectWriteFileResult.Type; From 4b527aea132e12a979794f85af12c3e6872ce208 Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Mon, 26 Oct 2026 14:26:53 +0000 Subject: [PATCH 5/6] fix(web): retain acknowledged contents for final-save retries --- .../src/workspace/WorkspaceFileSystem.test.ts | 31 ++++++++++++++++- .../src/workspace/WorkspaceFileSystem.ts | 1 + .../useFileSaveCoordinator.browser.test.tsx | 33 +++++++++++++++++++ .../files/useFileSaveCoordinator.ts | 2 +- 4 files changed, 65 insertions(+), 2 deletions(-) diff --git a/apps/server/src/workspace/WorkspaceFileSystem.test.ts b/apps/server/src/workspace/WorkspaceFileSystem.test.ts index 8fc2120ddaa2c..90ddb8a5422e2 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.test.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.test.ts @@ -6,6 +6,8 @@ import * as NodePath from "@effect/platform-node/NodePath"; import * as NodeServices from "@effect/platform-node/NodeServices"; import { it, describe, expect } from "@effect/vitest"; import * as Effect from "effect/Effect"; +import * as Deferred from "effect/Deferred"; +import * as Fiber from "effect/Fiber"; import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import * as Path from "effect/Path"; @@ -23,7 +25,7 @@ import { vi } from "vite-plus/test"; vi.mock("node:fs/promises", async (importOriginal) => { const actual = await importOriginal(); - return { ...actual, realpath: vi.fn(actual.realpath) }; + return { ...actual, realpath: vi.fn(actual.realpath), writeFile: vi.fn(actual.writeFile) }; }); const UNKNOWN_WORKSPACE_PREFIX = "t3code-unknown-workspace-"; @@ -371,6 +373,33 @@ it.layer(layerTest, { excludeTestServices: true })("WorkspaceFileSystemLive", (i }); describe("writeFile", () => { + it.effect("waits for native writes to settle before cancellation releases the lease", () => + Effect.gen(function* () { + const service = yield* WorkspaceFileSystem.WorkspaceFileSystem; + const cwd = yield* makeTempDir; + const started = yield* Deferred.make(); + let finishWrite!: () => void; + vi.mocked(NodeFSP.writeFile).mockImplementationOnce( + () => + new Promise((resolve) => { + finishWrite = resolve; + Deferred.doneUnsafe(started, Effect.void); + }), + ); + const write = yield* service + .writeFile({ cwd, relativePath: "note.txt", contents: "retired" }) + .pipe(Effect.forkChild); + yield* Deferred.await(started); + const cancelling = yield* Fiber.interrupt(write).pipe( + Effect.forkChild({ startImmediately: true }), + ); + const releasedBeforeNativeWrite = cancelling.pollUnsafe() !== undefined; + finishWrite(); + yield* Fiber.join(cancelling); + expect(releasedBeforeNativeWrite).toBe(false); + }), + ); + it.effect("refuses a retired save after another client has saved newer contents", () => Effect.gen(function* () { const service = yield* WorkspaceFileSystem.WorkspaceFileSystem; diff --git a/apps/server/src/workspace/WorkspaceFileSystem.ts b/apps/server/src/workspace/WorkspaceFileSystem.ts index 8b3d071bd2130..aa7bb79d3bc5d 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.ts @@ -566,6 +566,7 @@ export const make = Effect.gen(function* () { ); yield* Effect.tryPromise({ // A symlink swapped in after the check is not followed. Windows lacks O_NOFOLLOW. + // A zero-argument try keeps Effect waiting for native IO before cancellation releases the lease. try: () => NodeFSP.writeFile(realTargetPath, input.contents, { flag: diff --git a/apps/web/src/components/files/useFileSaveCoordinator.browser.test.tsx b/apps/web/src/components/files/useFileSaveCoordinator.browser.test.tsx index 5574f93b1f054..2b65f66f1a1c0 100644 --- a/apps/web/src/components/files/useFileSaveCoordinator.browser.test.tsx +++ b/apps/web/src/components/files/useFileSaveCoordinator.browser.test.tsx @@ -73,6 +73,39 @@ async function mount(props = defaultProps) { } describe("file-save React lifecycle", () => { + it("retries the final draft against its earlier acknowledged save, not the stale query", async () => { + setup(); + using _timers = fakeTimers(); + await using screen = await mount(); + let finishFirst!: (result: ReturnType) => void; + writeFile + .mockImplementationOnce( + () => + new Promise((resolve) => { + finishFirst = resolve; + }), + ) + .mockResolvedValueOnce(AsyncResult.failure(Cause.fail(new Error("unavailable")))); + screen.changeHandler()("first"); + await vi.advanceTimersByTimeAsync(500); + screen.changeHandler()("final"); + getUnsavedFile.mockReturnValue({ contents: "final" }); + confirmFile.mockReturnValueOnce(false); + await screen.unmount(); + finishFirst(AsyncResult.success({ relativePath: "file.txt" })); + await vi.runAllTimersAsync(); + expect(writeFile).toHaveBeenLastCalledWith({ + environmentId, + input: { + cwd: "/workspace", + relativePath: "file.txt", + contents: "final", + expectedContents: "first", + }, + }); + expect(onPendingChange).toHaveBeenLastCalledWith("file.txt", false); + }); + it("retains the draft and stops when the server refuses a stale retry", async () => { setup(); using _timers = fakeTimers(); diff --git a/apps/web/src/components/files/useFileSaveCoordinator.ts b/apps/web/src/components/files/useFileSaveCoordinator.ts index 252d9760d559b..8cbd8be34be89 100644 --- a/apps/web/src/components/files/useFileSaveCoordinator.ts +++ b/apps/web/src/components/files/useFileSaveCoordinator.ts @@ -90,7 +90,7 @@ function createFileSaveSession({ getUnsavedProjectFileQueryData(environmentId, cwd, relativePath)?.contents === contents, onPendingChange: (pending) => onPendingChange(relativePath, pending), persist: async (nextContents, retry) => { - if (!retry) { + if (!retry && expectedContents === undefined) { const base = getPersistedProjectFileQueryData(environmentId, cwd, relativePath); expectedContents = base?.supportsConditionalWrite && !base.truncated From aa6670f624f43a9d24b0d5f47aca198218af3344 Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Mon, 26 Oct 2026 14:33:29 +0000 Subject: [PATCH 6/6] fix(server): hold file lease until cancelled writes settle --- apps/server/src/workspace/WorkspaceFileSystem.test.ts | 2 +- apps/server/src/workspace/WorkspaceFileSystem.ts | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/apps/server/src/workspace/WorkspaceFileSystem.test.ts b/apps/server/src/workspace/WorkspaceFileSystem.test.ts index 90ddb8a5422e2..42c662ea8678a 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.test.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.test.ts @@ -383,7 +383,7 @@ it.layer(layerTest, { excludeTestServices: true })("WorkspaceFileSystemLive", (i () => new Promise((resolve) => { finishWrite = resolve; - Deferred.doneUnsafe(started, Effect.void); + queueMicrotask(() => Deferred.doneUnsafe(started, Effect.void)); }), ); const write = yield* service diff --git a/apps/server/src/workspace/WorkspaceFileSystem.ts b/apps/server/src/workspace/WorkspaceFileSystem.ts index aa7bb79d3bc5d..0425ca90d7a8b 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.ts @@ -566,7 +566,6 @@ export const make = Effect.gen(function* () { ); yield* Effect.tryPromise({ // A symlink swapped in after the check is not followed. Windows lacks O_NOFOLLOW. - // A zero-argument try keeps Effect waiting for native IO before cancellation releases the lease. try: () => NodeFSP.writeFile(realTargetPath, input.contents, { flag: @@ -584,7 +583,7 @@ export const make = Effect.gen(function* () { operation: "write-file", cause, }), - }); + }).pipe(Effect.uninterruptible); yield* workspaceEntries.refresh(input.cwd); return { relativePath: target.relativePath }; }),