Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 15 additions & 14 deletions apps/web/src/components/files/FilePreviewPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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),
});
}
}
/>
Expand Down Expand Up @@ -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 ? (
Expand Down
211 changes: 211 additions & 0 deletions apps/web/src/components/files/changeMarkdownTask.test.ts
Original file line number Diff line number Diff line change
@@ -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.AsyncResult<ProjectReadFileResult, never>>(
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();
});
});
34 changes: 34 additions & 0 deletions apps/web/src/components/files/changeMarkdownTask.ts
Original file line number Diff line number Diff line change
@@ -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);
}
27 changes: 27 additions & 0 deletions apps/web/src/components/files/projectFilesQueryState.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
});
});
19 changes: 17 additions & 2 deletions apps/web/src/components/files/projectFilesQueryState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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,
Expand Down
Loading
Loading