diff --git a/apps/web/src/components/files/FilePreviewPanel.tsx b/apps/web/src/components/files/FilePreviewPanel.tsx index e2d3a2b3c8ca..56632b3f9646 100644 --- a/apps/web/src/components/files/FilePreviewPanel.tsx +++ b/apps/web/src/components/files/FilePreviewPanel.tsx @@ -82,15 +82,11 @@ import { projectFileCacheKey, projectFileEditorCacheKey } from "./fileContentRev import { isMarkdownPreviewFile, resolveFilePreviewPath, - setMarkdownTaskChecked, shouldShowFileExplorer, } from "./filePreviewMode"; +import { changeMarkdownTask } from "./changeMarkdownTask"; import { useFileSaveCoordinator } from "./useFileSaveCoordinator"; -import { - getOptimisticProjectFileQueryData, - setProjectFileQueryData, - useProjectFileQuery, -} from "./projectFilesQueryState"; +import { setProjectFileQueryData, useProjectFileQuery } from "./projectFilesQueryState"; interface FilePreviewPanelProps { environmentId: EnvironmentId; @@ -875,13 +871,16 @@ function RenderedMarkdownSurface({ readOnly ? undefined : ({ markerOffset, checked }) => { - const currentContents = - getOptimisticProjectFileQueryData(environmentId, cwd, relativePath)?.contents ?? - contents; - const nextContents = setMarkdownTaskChecked(currentContents, markerOffset, checked); - if (nextContents === currentContents) return; - setProjectFileQueryData(environmentId, cwd, relativePath, nextContents); - saveCoordinator.change(nextContents); + changeMarkdownTask({ + environmentId, + cwd, + relativePath, + readOnly, + contents, + markerOffset, + checked, + change: (nextContents) => saveCoordinator.change(nextContents), + }); } } /> @@ -1255,7 +1254,9 @@ export default function FilePreviewPanel({ relativePath={relativePath} threadRef={threadRef} contents={file.data.contents} - readOnly={isHostFile} + readOnly={ + file.data.truncated || isHostFile || file.error !== null || file.isPending + } onPendingChange={onPendingChange} /> ) : tableDelimiter && renderTable ? ( diff --git a/apps/web/src/components/files/changeMarkdownTask.test.ts b/apps/web/src/components/files/changeMarkdownTask.test.ts new file mode 100644 index 000000000000..2a186e9704ea --- /dev/null +++ b/apps/web/src/components/files/changeMarkdownTask.test.ts @@ -0,0 +1,211 @@ +import type { ProjectReadFileResult } from "@t3tools/contracts"; +import { EnvironmentId } from "@t3tools/contracts"; +import * as Cause from "effect/Cause"; +import * as Option from "effect/Option"; +import { AsyncResult } from "effect/unstable/reactivity"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; + +const { readAtom, optimisticAtom } = await vi.hoisted(async () => { + const { Atom, AsyncResult } = await import("effect/unstable/reactivity"); + return { + readAtom: Atom.make>( + AsyncResult.initial(false), + ), + optimisticAtom: Atom.make<{ + data: ProjectReadFileResult; + confirmedAgainst: unknown; + } | null>(null), + }; +}); +vi.mock("~/state/projects", () => ({ + projectEnvironment: { + readFile: () => readAtom, + optimisticFile: () => optimisticAtom, + }, +})); + +import { appAtomRegistry } from "~/rpc/atomRegistry"; +import { changeMarkdownTask } from "./changeMarkdownTask"; +import { FileSaveCoordinator } from "./fileSaveCoordinator"; +import { setProjectFileQueryData } from "./projectFilesQueryState"; + +const identity = { + environmentId: EnvironmentId.make("markdown-authority-test"), + cwd: "/disposable-workspace", + relativePath: "README.md", +}; + +function read(contents: string, truncated = false) { + appAtomRegistry.set( + readAtom, + AsyncResult.success({ ...identity, contents, truncated, byteLength: contents.length }), + ); +} + +function fixture(contents: string, readOnly = false) { + let persisted = contents; + let displayed = contents; + const persist = vi.fn(async (next: string) => { + persisted = next; + return AsyncResult.success(undefined); + }); + const coordinator = new FileSaveCoordinator({ + debounceMs: 500, + persist, + onPendingChange: vi.fn(), + onConfirmed: vi.fn(), + }); + return { + render: (next: string) => { + displayed = next; + }, + toggle: (markerOffset = 2, checked = true) => + changeMarkdownTask({ + ...identity, + readOnly, + contents: displayed, + markerOffset, + checked, + change: (next) => coordinator.change(next), + }), + persisted: () => persisted, + persist, + }; +} + +beforeEach(() => { + vi.useFakeTimers(); + appAtomRegistry.set(readAtom, AsyncResult.initial(false)); + appAtomRegistry.set(optimisticAtom, null); +}); +afterEach(() => { + vi.useRealTimers(); +}); + +describe("rendered Markdown write authority", () => { + it("preserves the whole file when the visible prefix is truncated", async () => { + const wholeFile = "- [ ] task\n" + "a".repeat(1024 * 1024) + "\nTAIL\n"; + read(wholeFile.slice(0, 1024 * 1024), true); + const file = fixture(wholeFile); + file.toggle(); + await vi.runAllTimersAsync(); + expect(file.persist).not.toHaveBeenCalled(); + expect(file.persisted()).toBe(wholeFile); + expect(appAtomRegistry.get(optimisticAtom)).toBeNull(); + }); + + it("rechecks a truncated refresh after a writable callback was created", async () => { + const original = "- [ ] task\noriginal tail\n"; + read(original); + const file = fixture(original); + setProjectFileQueryData(identity.environmentId, identity.cwd, identity.relativePath, original); + read("- [ ] task\n", true); + file.toggle(); + await vi.runAllTimersAsync(); + expect(file.persist).not.toHaveBeenCalled(); + expect(file.persisted()).toBe(original); + expect(appAtomRegistry.get(optimisticAtom)?.data.contents).toBe(original); + }); + + it("does not replace a complete read-only host file", async () => { + const original = "- [ ] host task\n"; + read(original); + const file = fixture(original, true); + file.toggle(); + await vi.runAllTimersAsync(); + expect(file.persist).not.toHaveBeenCalled(); + expect(file.persisted()).toBe(original); + expect(appAtomRegistry.get(optimisticAtom)).toBeNull(); + }); + + it("preserves other characters and accumulates edits from the latest displayed draft", async () => { + const original = "- [ ] first\r\n- [X] second\r\n尾\r\n"; + read(original); + const file = fixture(original); + file.toggle(); + file.render("- [x] first\r\n- [X] second\r\n尾\r\n"); + file.toggle(original.indexOf("[X]"), false); + await vi.runAllTimersAsync(); + expect(file.persist).toHaveBeenCalledExactlyOnceWith("- [x] first\r\n- [ ] second\r\n尾\r\n"); + expect(file.persisted()).toBe("- [x] first\r\n- [ ] second\r\n尾\r\n"); + }); + + it("does not authorize a write when the live read is unavailable", async () => { + const file = fixture("- [ ] task\n"); + file.toggle(); + await vi.runAllTimersAsync(); + expect(file.persist).not.toHaveBeenCalled(); + expect(appAtomRegistry.get(optimisticAtom)).toBeNull(); + }); + + it("does not authorize an optimistic draft without a live read", async () => { + const original = "- [ ] task\n"; + const file = fixture(original); + setProjectFileQueryData(identity.environmentId, identity.cwd, identity.relativePath, original); + file.toggle(); + await vi.runAllTimersAsync(); + expect(file.persist).not.toHaveBeenCalled(); + expect(file.persisted()).toBe(original); + expect(appAtomRegistry.get(optimisticAtom)?.data.contents).toBe(original); + }); + + it("does not write a retained success after a failed refresh", async () => { + const original = "- [ ] task\n"; + read(original); + const previous = appAtomRegistry.get(readAtom); + appAtomRegistry.set( + readAtom, + AsyncResult.failureWithPrevious(Cause.die(new Error("read failed")), { + previous: Option.some(previous), + }), + ); + const file = fixture(original); + file.toggle(); + await vi.runAllTimersAsync(); + expect(file.persist).not.toHaveBeenCalled(); + expect(appAtomRegistry.get(optimisticAtom)).toBeNull(); + }); + + it("waits for a pending refresh before authorizing another edit", async () => { + const original = "- [ ] task\n"; + appAtomRegistry.set( + readAtom, + AsyncResult.success( + { ...identity, contents: original, truncated: false, byteLength: original.length }, + { waiting: true }, + ), + ); + const file = fixture(original); + file.toggle(); + await vi.runAllTimersAsync(); + expect(file.persist).not.toHaveBeenCalled(); + read(original); + file.toggle(); + await vi.runAllTimersAsync(); + expect(file.persist).toHaveBeenCalledExactlyOnceWith("- [x] task\n"); + }); + + it("does not apply a displayed task offset to a different complete snapshot", async () => { + const original = "- [ ] intended task\n"; + read(original); + const file = fixture(original); + read("- [ ] replacement task\n"); + file.toggle(); + await vi.runAllTimersAsync(); + expect(file.persist).not.toHaveBeenCalled(); + expect(appAtomRegistry.get(optimisticAtom)).toBeNull(); + file.render("- [ ] replacement task\n"); + file.toggle(); + await vi.runAllTimersAsync(); + expect(file.persist).toHaveBeenCalledExactlyOnceWith("- [x] replacement task\n"); + }); + + it("does not save an invalid marker offset", async () => { + read("- [ ] task\n"); + const file = fixture("- [ ] task\n"); + file.toggle(0); + await vi.runAllTimersAsync(); + expect(file.persist).not.toHaveBeenCalled(); + expect(appAtomRegistry.get(optimisticAtom)).toBeNull(); + }); +}); diff --git a/apps/web/src/components/files/changeMarkdownTask.ts b/apps/web/src/components/files/changeMarkdownTask.ts new file mode 100644 index 000000000000..a4c993acd7c9 --- /dev/null +++ b/apps/web/src/components/files/changeMarkdownTask.ts @@ -0,0 +1,34 @@ +import type { EnvironmentId } from "@t3tools/contracts"; + +import { setMarkdownTaskChecked } from "./filePreviewMode"; +import { getProjectFileQueryData, setProjectFileQueryData } from "./projectFilesQueryState"; + +export function changeMarkdownTask({ + environmentId, + cwd, + relativePath, + readOnly, + contents, + markerOffset, + checked, + change, +}: { + environmentId: EnvironmentId; + cwd: string; + relativePath: string; + readOnly: boolean; + contents: string; + markerOffset: number; + checked: boolean; + change: (contents: string) => void; +}): void { + if (readOnly) return; + const file = getProjectFileQueryData(environmentId, cwd, relativePath); + // The marker offset belongs to the displayed snapshot, not a later refresh. + // Only a complete live read of that snapshot can authorize replacement. + if (!file || file.truncated || file.contents !== contents) return; + const nextContents = setMarkdownTaskChecked(file.contents, markerOffset, checked); + if (nextContents === file.contents) return; + setProjectFileQueryData(environmentId, cwd, relativePath, nextContents); + change(nextContents); +} diff --git a/apps/web/src/components/files/projectFilesQueryState.test.tsx b/apps/web/src/components/files/projectFilesQueryState.test.tsx index 8ac6f4e1cd26..d8c2091a17d8 100644 --- a/apps/web/src/components/files/projectFilesQueryState.test.tsx +++ b/apps/web/src/components/files/projectFilesQueryState.test.tsx @@ -309,4 +309,31 @@ describe("project query refresh", () => { atomHooks.registry = null; } }); + + it("does not let an optimistic draft hide a truncated read", async () => { + const truncatedRead: ProjectReadFileResult = { + ...file("partial"), + byteLength: 2_000_000, + truncated: true, + }; + const readAtom = Atom.make(Effect.succeed(truncatedRead)); + const draftAtom = Atom.make({ confirmedAgainst: undefined, data: file("draft") }); + const registry = AtomRegistry.make(); + const unmount = registry.mount(readAtom); + projectMocks.readFile.mockReturnValue(readAtom); + projectMocks.optimisticFile.mockReturnValue(draftAtom); + atomHooks.registry = registry; + + try { + await flushEffects(); + reactHooks.beginRender(); + const query = useProjectFileQuery(environmentId, "/repo", "src/preview.ts"); + expect(query.data?.truncated).toBe(true); + expect(query.data?.contents).toBe("partial"); + } finally { + unmount(); + registry.dispose(); + atomHooks.registry = null; + } + }); }); diff --git a/apps/web/src/components/files/projectFilesQueryState.ts b/apps/web/src/components/files/projectFilesQueryState.ts index a6a556231e18..e73bc6b2bacc 100644 --- a/apps/web/src/components/files/projectFilesQueryState.ts +++ b/apps/web/src/components/files/projectFilesQueryState.ts @@ -88,6 +88,19 @@ export function getOptimisticProjectFileQueryData( return appAtomRegistry.get(optimisticFileAtom(environmentId, cwd, relativePath))?.data ?? null; } +// A refreshed partial read must not gain write authority from an older draft. +export function getProjectFileQueryData( + environmentId: EnvironmentId, + cwd: string, + relativePath: string, +): ProjectReadFileResult | null { + const result = appAtomRegistry.get(getProjectFileQueryAtom(environmentId, cwd, relativePath)); + if (result._tag !== "Success" || result.waiting) return null; + const data = result.value; + if (data.truncated) return data; + return getOptimisticProjectFileQueryData(environmentId, cwd, relativePath) ?? data; +} + export function confirmProjectFileQueryData( environmentId: EnvironmentId, cwd: string, @@ -122,7 +135,8 @@ export function resolveProjectFileQueryData( relativePath: string | null, data: ProjectReadFileResult | null, ): ProjectReadFileResult | null { - if (relativePath === null) return data; + // A truncated read has no write authority; an optimistic draft must not hide it. + if (relativePath === null || data?.truncated) return data; return appAtomRegistry.get(optimisticFileAtom(environmentId, cwd, relativePath))?.data ?? data; } @@ -219,7 +233,8 @@ export function useProjectFileQuery( const cause = failureCause(result); return { - data: optimisticFile?.data ?? data, + // A truncated read has no write authority; an optimistic draft must not hide it. + data: data?.truncated ? data : (optimisticFile?.data ?? data), error: errorMessage(cause), isNotFile: isProjectReadFileError(cause) && cause.failure === "path_not_file", isPending: result.waiting, diff --git a/apps/web/src/components/files/useFileSaveCoordinator.test.tsx b/apps/web/src/components/files/useFileSaveCoordinator.test.tsx index 603f97cf1f26..8d961de923e5 100644 --- a/apps/web/src/components/files/useFileSaveCoordinator.test.tsx +++ b/apps/web/src/components/files/useFileSaveCoordinator.test.tsx @@ -4,13 +4,22 @@ import { act, StrictMode } from "react"; import { create, type ReactTestRenderer } from "react-test-renderer"; import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; -const { writeFile, confirmFile } = vi.hoisted(() => ({ +const { writeFile, confirmFile, readFile, readState, waitForRead } = vi.hoisted(() => ({ writeFile: vi.fn(), confirmFile: vi.fn(), + readFile: vi.fn(), + readState: vi.fn(), + waitForRead: vi.fn(), })); +vi.mock("@t3tools/client-runtime/state/runtime", () => ({ executeAtomQuery: waitForRead })); +vi.mock("~/rpc/atomRegistry", () => ({ appAtomRegistry: { get: readState } })); vi.mock("~/state/projects", () => ({ projectEnvironment: { writeFile: {} } })); vi.mock("~/state/use-atom-command", () => ({ useAtomCommand: () => writeFile })); -vi.mock("./projectFilesQueryState", () => ({ confirmProjectFileQueryData: confirmFile })); +vi.mock("./projectFilesQueryState", () => ({ + confirmProjectFileQueryData: confirmFile, + getProjectFileQueryData: readFile, + getProjectFileQueryAtom: () => ({}), +})); import { setMarkdownTaskChecked } from "./filePreviewMode"; import { useFileSaveCoordinator } from "./useFileSaveCoordinator"; @@ -54,6 +63,9 @@ beforeEach(() => { vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); writeFile.mockReset().mockResolvedValue(AsyncResult.success(undefined)); confirmFile.mockReset(); + readFile.mockReset().mockReturnValue({ truncated: false }); + readState.mockReset().mockReturnValue({ waiting: false }); + waitForRead.mockReset(); onPendingChange.mockReset(); }); @@ -64,6 +76,54 @@ afterEach(async () => { }); describe("file-save React lifecycle", () => { + it("waits for a pending read before flushing an edit on close", async () => { + let complete!: () => void; + waitForRead.mockReturnValue( + new Promise((resolve) => { + complete = resolve; + }), + ); + mount(); + changeHandler()("pending edit"); + readState.mockReturnValue({ waiting: true }); + await act(async () => renderer!.unmount()); + renderer = null; + expect(writeFile).not.toHaveBeenCalled(); + readState.mockReturnValue({ waiting: false }); + await act(async () => complete()); + expect(writeFile).toHaveBeenCalledTimes(1); + expect(writeFile.mock.calls[0]![0].input.contents).toBe("pending edit"); + }); + + it.each([null, { truncated: true }])( + "does not flush after read authority is lost: %j", + async (read) => { + mount(); + changeHandler()("pending edit"); + readFile.mockReturnValue(read); + await vi.advanceTimersByTimeAsync(500); + expect(writeFile).not.toHaveBeenCalled(); + expect(confirmFile).not.toHaveBeenCalled(); + expect(onPendingChange).not.toHaveBeenCalledWith("file.txt", false); + await act(async () => renderer!.unmount()); + renderer = null; + expect(writeFile).not.toHaveBeenCalled(); + }, + ); + + it("rechecks authority when a failed save is retried on close", async () => { + writeFile.mockResolvedValueOnce(AsyncResult.fail(new Error("write failed"))); + mount(); + changeHandler()("pending edit"); + await vi.advanceTimersByTimeAsync(500); + expect(writeFile).toHaveBeenCalledTimes(1); + readFile.mockReturnValue({ truncated: true }); + await act(async () => renderer!.unmount()); + renderer = null; + expect(writeFile).toHaveBeenCalledTimes(1); + expect(confirmFile).not.toHaveBeenCalled(); + }); + it("persists editor model changes after StrictMode setup replay", async () => { mount(); changeHandler()("AUDIT7907NATIVE\n"); diff --git a/apps/web/src/components/files/useFileSaveCoordinator.ts b/apps/web/src/components/files/useFileSaveCoordinator.ts index 852490cf5be4..ddc4f369fe57 100644 --- a/apps/web/src/components/files/useFileSaveCoordinator.ts +++ b/apps/web/src/components/files/useFileSaveCoordinator.ts @@ -1,11 +1,18 @@ +import { executeAtomQuery } from "@t3tools/client-runtime/state/runtime"; import type { EnvironmentId } from "@t3tools/contracts"; +import { AsyncResult } from "effect/unstable/reactivity"; import { createRef, useEffect, useMemo } from "react"; +import { appAtomRegistry } from "~/rpc/atomRegistry"; import { projectEnvironment } from "~/state/projects"; import { useAtomCommand } from "~/state/use-atom-command"; import { FileSaveCoordinator } from "./fileSaveCoordinator"; -import { confirmProjectFileQueryData } from "./projectFilesQueryState"; +import { + confirmProjectFileQueryData, + getProjectFileQueryAtom, + getProjectFileQueryData, +} from "./projectFilesQueryState"; const FILE_SAVE_DEBOUNCE_MS = 500; @@ -31,11 +38,24 @@ export function useFileSaveCoordinator({ const coordinator = new FileSaveCoordinator({ debounceMs: FILE_SAVE_DEBOUNCE_MS, onPendingChange: (pending) => onPendingChange(relativePath, pending), - persist: (nextContents) => - writeFile({ + persist: async (nextContents) => { + // Debouncing and disposal can outlive the read that authorized the edit. + const atom = getProjectFileQueryAtom(environmentId, cwd, relativePath); + if (appAtomRegistry.get(atom).waiting) { + await executeAtomQuery(appAtomRegistry, atom, { + reportFailure: false, + reportDefect: false, + }); + } + const file = getProjectFileQueryData(environmentId, cwd, relativePath); + if (!file || file.truncated) { + return AsyncResult.fail(new Error("Cannot save an incomplete file preview.")); + } + return writeFile({ environmentId, input: { cwd, relativePath, contents: nextContents }, - }), + }); + }, onConfirmed: (confirmedContents) => { confirmProjectFileQueryData(environmentId, cwd, relativePath, confirmedContents); },