From 5d4eec597affd08aab6d418990c3c9f2c672e7f4 Mon Sep 17 00:00:00 2001 From: Alex Southwell Date: Sun, 20 Sep 2026 13:32:04 +1000 Subject: [PATCH] fix(web): prevent writes from incomplete file previews --- .../src/components/files/FilePreviewPanel.tsx | 31 ++-- .../files/changeMarkdownTask.test.ts | 152 ++++++++++++++++++ .../components/files/changeMarkdownTask.ts | 31 ++++ .../files/projectFilesQueryState.ts | 12 ++ 4 files changed, 208 insertions(+), 18 deletions(-) create mode 100644 apps/web/src/components/files/changeMarkdownTask.test.ts create mode 100644 apps/web/src/components/files/changeMarkdownTask.ts diff --git a/apps/web/src/components/files/FilePreviewPanel.tsx b/apps/web/src/components/files/FilePreviewPanel.tsx index 1d89d58bc67f..a26dfb854aa9 100644 --- a/apps/web/src/components/files/FilePreviewPanel.tsx +++ b/apps/web/src/components/files/FilePreviewPanel.tsx @@ -79,17 +79,10 @@ import SourceFilePreview from "./ReadOnlySourcePreview"; import { resolveCenteredFileLineScrollTop } from "./fileLineReveal"; import { DiffCommentAnnotation } from "../diffs/DiffCommentAnnotation"; import { projectFileCacheKey, projectFileEditorCacheKey } from "./fileContentRevision"; -import { - isMarkdownPreviewFile, - setMarkdownTaskChecked, - shouldShowFileExplorer, -} from "./filePreviewMode"; +import { isMarkdownPreviewFile, 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; @@ -874,13 +867,15 @@ 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, + markerOffset, + checked, + change: (nextContents) => saveCoordinator.change(nextContents), + }); } } /> @@ -1246,7 +1241,7 @@ export default function FilePreviewPanel({ relativePath={relativePath} threadRef={threadRef} contents={file.data.contents} - readOnly={isHostFile} + readOnly={file.data.truncated || isHostFile} 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..722c14547fc0 --- /dev/null +++ b/apps/web/src/components/files/changeMarkdownTask.test.ts @@ -0,0 +1,152 @@ +import type { ProjectReadFileResult } from "@t3tools/contracts"; +import { EnvironmentId } from "@t3tools/contracts"; +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; + 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 { + toggle: (markerOffset = 2, checked = true) => + changeMarkdownTask({ + ...identity, + readOnly, + 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 all other bytes and accumulates edits from the latest complete draft", async () => { + const original = "- [ ] first\r\n- [X] second\r\n尾\r\n"; + read(original); + const file = fixture(original); + file.toggle(); + 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 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..8f46e1d69142 --- /dev/null +++ b/apps/web/src/components/files/changeMarkdownTask.ts @@ -0,0 +1,31 @@ +import type { EnvironmentId } from "@t3tools/contracts"; + +import { setMarkdownTaskChecked } from "./filePreviewMode"; +import { getProjectFileQueryData, setProjectFileQueryData } from "./projectFilesQueryState"; + +export function changeMarkdownTask({ + environmentId, + cwd, + relativePath, + readOnly, + markerOffset, + checked, + change, +}: { + environmentId: EnvironmentId; + cwd: string; + relativePath: string; + readOnly: boolean; + markerOffset: number; + checked: boolean; + change: (contents: string) => void; +}): void { + if (readOnly) return; + const file = getProjectFileQueryData(environmentId, cwd, relativePath); + // Only a complete live read can authorize replacing the original file. + if (!file || file.truncated) 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.ts b/apps/web/src/components/files/projectFilesQueryState.ts index b9a880301831..9d19fd89316c 100644 --- a/apps/web/src/components/files/projectFilesQueryState.ts +++ b/apps/web/src/components/files/projectFilesQueryState.ts @@ -83,6 +83,18 @@ 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)); + const data = Option.getOrNull(AsyncResult.value(result)); + if (!data || data.truncated) return data; + return getOptimisticProjectFileQueryData(environmentId, cwd, relativePath) ?? data; +} + export function confirmProjectFileQueryData( environmentId: EnvironmentId, cwd: string,