diff --git a/apps/mobile/src/Stack.tsx b/apps/mobile/src/Stack.tsx index 14fbc8b82f51..69ba831c27bd 100644 --- a/apps/mobile/src/Stack.tsx +++ b/apps/mobile/src/Stack.tsx @@ -33,6 +33,7 @@ import { useAgentNotificationNavigation } from "./features/agent-awareness/notif import { ConnectOnboardingRouteScreen } from "./features/cloud/ConnectOnboardingRouteScreen"; import { useConnectOnboardingNavigation } from "./features/cloud/connectOnboardingNavigation"; import { AttachmentFileScreen } from "./features/files/AttachmentFileScreen"; +import { FileEditorRouteScreen } from "./features/files/FileEditorRouteScreen"; import { ThreadFilesTreeScreen, ThreadFileScreen } from "./features/files/ThreadFilesRouteScreen"; import { AdaptiveWorkspaceLayout } from "./features/layout/AdaptiveWorkspaceLayout"; import { @@ -551,6 +552,7 @@ const WORKSPACE_OVERLAY_ROUTES = new Set([ "ThreadReviewComment", "ThreadDevicePreview", "ThreadBrowserPreview", + "ThreadFileEdit", "ThreadSettingsSheet", ]); @@ -748,6 +750,19 @@ const RootStackConfig = createNativeStackNavigator({ linking: `${THREAD_LINKING_PREFIX}/files/:path*`, options: SOLID_HEADER_OPTIONS, }), + // Deliberately has no `linking:` path. The editor carries a live draft and + // the read revision the save is guarded against — state no URL can + // reconstruct — so a path would only produce a link that cannot open it. + // Reached from `ThreadFile` (or `NewTaskFile` in a draft), which do link. + ThreadFileEdit: createNativeStackScreen({ + screen: FileEditorRouteScreen, + options: { + presentation: "fullScreenModal", + headerShown: true, + gestureEnabled: false, + ...SOLID_HEADER_OPTIONS, + }, + }), ThreadMcpApp: createNativeStackScreen({ screen: McpAppFullscreenScreen, linking: `${THREAD_LINKING_PREFIX}/apps/:itemId`, diff --git a/apps/mobile/src/features/files/FileEditorRouteScreen.tsx b/apps/mobile/src/features/files/FileEditorRouteScreen.tsx new file mode 100644 index 000000000000..aab4fae9cc17 --- /dev/null +++ b/apps/mobile/src/features/files/FileEditorRouteScreen.tsx @@ -0,0 +1,278 @@ +import { useAtomValue } from "@effect/atom-react"; +import { useNavigation, usePreventRemove, type StaticScreenProps } from "@react-navigation/native"; +import { squashAtomCommandFailure } from "@t3tools/client-runtime/state/runtime"; +import { + EnvironmentId, + ProjectWriteFileError, + type ProjectReadFileResult, +} from "@t3tools/contracts"; +import * as Cause from "effect/Cause"; +import * as Schema from "effect/Schema"; +import { useCallback, useEffect, useMemo, useState } from "react"; +import { Alert, Platform, TextInput, View } from "react-native"; +import { KeyboardAvoidingView } from "react-native-keyboard-controller"; +import { useSafeAreaInsets } from "react-native-safe-area-context"; + +import { EmptyState } from "../../components/EmptyState"; +import { ScreenHeader } from "../../components/ScreenHeader"; +import { projectEnvironment } from "../../state/projects"; +import { useEnvironmentQuery } from "../../state/query"; +import { useAtomCommand } from "../../state/use-atom-command"; +import { withNativeGlassHeaderItem } from "../layout/native-glass-header-items"; +import { REVIEW_MONO_FONT_FAMILY } from "../review/reviewDiffRendering"; +import { useAppearanceCodeSurface } from "../settings/appearance/useAppearanceCodeSurface"; +import { FilePreviewLoading, FilePreviewNotice } from "./FilePreviewFeedback"; +import { + canEditWorkspaceFile, + fromEditorText, + MAX_EDITABLE_FILE_BYTES, + toEditorText, + type EditorLineEnding, +} from "./fileEditing"; +import { basename } from "./filePath"; + +type FileEditorRouteScreenProps = StaticScreenProps<{ + readonly environmentId: string; + /** Absent for a project draft, which has no thread yet. */ + readonly threadId?: string; + readonly cwd: string; + readonly path: string[]; +}>; + +/** + * The loaded file, plus the text a save would write. `loadedText` and + * `revision` stay pinned to the read the user started from, so a background + * refresh can neither hide their changes nor weaken the guarded write. + */ +interface FileEditorDraft { + readonly text: string; + readonly loadedText: string; + readonly lineEnding: EditorLineEnding; + readonly hasUtf8Bom: boolean; + readonly revision: string; +} + +const NOT_EDITABLE_DETAIL = `Only complete UTF-8 workspace text files under ${ + MAX_EDITABLE_FILE_BYTES / 1024 +} KB can be edited on mobile.`; + +const isProjectWriteFileError = Schema.is(ProjectWriteFileError); + +/** The typed write failure behind an RPC error, whatever else the cause wraps. */ +function writeFileFailure(result: { + readonly cause: Cause.Cause; +}): ProjectWriteFileError | null { + const error = squashAtomCommandFailure(result); + return isProjectWriteFileError(error) ? error : null; +} + +function loadedDraft(input: { + readonly relativePath: string; + readonly file: ProjectReadFileResult | null; +}): FileEditorDraft | null { + const { file } = input; + if (file === null || file.revision === undefined || !canEditWorkspaceFile(input)) { + return null; + } + const converted = toEditorText(file.contents); + return { + text: converted.text, + loadedText: converted.text, + lineEnding: converted.lineEnding, + hasUtf8Bom: converted.hasUtf8Bom, + revision: file.revision, + }; +} + +export function FileEditorRouteScreen(props: FileEditorRouteScreenProps) { + const navigation = useNavigation(); + const insets = useSafeAreaInsets(); + const { codeSurface } = useAppearanceCodeSurface(); + const environmentId = EnvironmentId.make(props.route.params.environmentId); + const cwd = props.route.params.cwd; + const relativePath = props.route.params.path.join("/"); + const canWriteFiles = useAtomValue(projectEnvironment.writeFile.permissionAtom(environmentId)); + const fileQuery = useEnvironmentQuery( + projectEnvironment.readFile({ environmentId, input: { cwd, relativePath } }), + ); + const readResult = fileQuery.data as ProjectReadFileResult | null; + const writeFile = useAtomCommand(projectEnvironment.writeFile, { + label: "workspace file save", + reportFailure: false, + }); + // The viewer already cached this read, so the editor opens with the text in hand. + const loaded = useMemo( + () => loadedDraft({ relativePath, file: readResult }), + [readResult, relativePath], + ); + // The editor owns a draft only once the user types; until then the read is the + // draft, and "Discard and reload" simply hands it back to the read. + const [edited, setEdited] = useState(null); + const [saving, setSaving] = useState(false); + const [saved, setSaved] = useState(false); + // The input is uncontrolled so typing never round-trips the whole file through React; + // bumping this remounts it with the reloaded text. + const [reloadCount, setReloadCount] = useState(0); + const draft = edited ?? loaded; + const dirty = edited !== null && edited.text !== edited.loadedText; + + const save = async (options?: { readonly overwrite?: boolean }) => { + if (edited === null || saving) return; + setSaving(true); + const result = await writeFile({ + environmentId, + input: { + cwd, + relativePath, + contents: fromEditorText(edited.text, edited.lineEnding, edited.hasUtf8Bom), + ...(options?.overwrite === true ? {} : { expectedRevision: edited.revision }), + }, + }); + setSaving(false); + if (result._tag === "Success") { + // The viewer shares this query, so refreshing it shows the saved bytes. + fileQuery.refresh(); + setSaved(true); + return; + } + const failure = writeFileFailure(result); + if (failure?.failure === "file_changed") { + Alert.alert( + "File changed", + `${relativePath} changed on the server since you started editing.`, + [ + { text: "Keep editing", style: "cancel" }, + { + text: "Discard and reload", + style: "destructive", + onPress: () => { + setEdited(null); + setReloadCount((count) => count + 1); + fileQuery.refresh(); + }, + }, + { text: "Overwrite", onPress: () => void save({ overwrite: true }) }, + ], + ); + return; + } + // Any other failure keeps the draft, so Save can be retried. + Alert.alert("Couldn't save", String(squashAtomCommandFailure(result))); + }; + + const preventRemove = !saved && (dirty || saving); + usePreventRemove(preventRemove, ({ data }) => { + if (saving) { + Alert.alert("Saving file", "Wait for the file to finish saving before leaving."); + return; + } + Alert.alert("Discard changes?", "Your unsaved changes will be lost.", [ + { text: "Keep editing", style: "cancel" }, + { + text: "Discard changes", + style: "destructive", + onPress: () => navigation.dispatch(data.action), + }, + ]); + }); + useEffect(() => { + if (!saved) return; + // Let the native removal guard turn off before popping the saved file. + const frame = requestAnimationFrame(() => { + if (navigation.isFocused()) navigation.goBack(); + }); + return () => cancelAnimationFrame(frame); + }, [navigation, saved]); + + const close = useCallback(() => navigation.goBack(), [navigation]); + const handleTextChange = useCallback( + (text: string) => { + setEdited((current) => + current !== null ? { ...current, text } : loaded === null ? null : { ...loaded, text }, + ); + }, + [loaded], + ); + // Save is meaningful only for a changed draft on a connection that may write. + const canSave = dirty && !saving && canWriteFiles; + + return ( + + void save(), + }, + ]} + options={{ + headerBackVisible: false, + gestureEnabled: !preventRemove, + // The system back button begins its native pop before the removal + // guard runs. Dispatch from a bar action so the guard runs first. + // Android's in-flow header carries its own close control. + ...(Platform.OS === "ios" + ? { + unstable_headerLeftItems: () => [ + withNativeGlassHeaderItem({ + type: "button", + label: "", + accessibilityLabel: "Cancel", + icon: { type: "sfSymbol", name: "xmark" }, + onPress: close, + }), + ], + } + : undefined), + }} + /> + {canWriteFiles ? null : ( + This connection can't edit files. + )} + {draft !== null ? ( + + + + ) : readResult === null && fileQuery.error === null ? ( + + ) : ( + + + + )} + + ); +} diff --git a/apps/mobile/src/features/files/ThreadFilesRouteScreen.tsx b/apps/mobile/src/features/files/ThreadFilesRouteScreen.tsx index e0421e2a587c..ab4b588b7cdd 100644 --- a/apps/mobile/src/features/files/ThreadFilesRouteScreen.tsx +++ b/apps/mobile/src/features/files/ThreadFilesRouteScreen.tsx @@ -1,4 +1,5 @@ import { resolveFilesystemReadAccess } from "@t3tools/client-runtime/state/filesystem"; +import { useAtomValue } from "@effect/atom-react"; import { environmentSession } from "../../state/session"; import { NativeStackScreenOptions } from "../../native/StackHeader"; import { StackActions, useNavigation, type StaticScreenProps } from "@react-navigation/native"; @@ -53,6 +54,7 @@ import { WorkspaceFileImagePreview } from "./WorkspaceFileImagePreview"; import { WorkspaceFilePreviewError } from "./WorkspaceFilePreviewError"; import { WorkspaceFileVideoPreview } from "./WorkspaceFileVideoPreview"; import { WorkspaceFileWebPreview } from "./WorkspaceFileWebPreview"; +import { canEditWorkspaceFile } from "./fileEditing"; import { basename, fileHeaderSubtitle, @@ -105,6 +107,8 @@ function FileHeader(props: { readonly iconColor: string; readonly activeMode: string; readonly fileInspectorSupported: boolean; + readonly canEditFile: boolean; + readonly onEditFile: () => void; readonly onBack: () => void; readonly onReturnToThread: () => void; readonly actions: ReadonlyArray<{ @@ -134,16 +138,29 @@ function FileHeader(props: { : undefined } actions={ - props.fileInspectorSupported + props.canEditFile || props.fileInspectorSupported ? [ - { - accessibilityLabel: panes.auxiliaryPaneVisible - ? "Hide file navigator" - : "Show file navigator", - icon: "sidebar.right", - selected: panes.auxiliaryPaneVisible, - onPress: toggleAuxiliaryPane, - }, + ...(props.canEditFile + ? [ + { + accessibilityLabel: "Edit file", + icon: "pencil" as const, + onPress: props.onEditFile, + }, + ] + : []), + ...(props.fileInspectorSupported + ? [ + { + accessibilityLabel: panes.auxiliaryPaneVisible + ? "Hide file navigator" + : "Show file navigator", + icon: "sidebar.right" as const, + selected: panes.auxiliaryPaneVisible, + onPress: toggleAuxiliaryPane, + }, + ] + : []), ] : undefined } @@ -704,6 +721,23 @@ export function ThreadFileScreen(props: ThreadFileRouteScreenProps) { : null, ); const fileData = fileQuery.data as ProjectReadFileResult | null; + const canWriteFiles = useAtomValue(projectEnvironment.writeFile.permissionAtom(environmentId)); + const canEditFile = + canWriteFiles && + relativePath !== null && + canEditWorkspaceFile({ relativePath, file: fileData }); + + const handleEditFile = useCallback(() => { + if (environmentId === null || cwd === null || relativePath === null) { + return; + } + navigation.navigate("ThreadFileEdit", { + environmentId: String(environmentId), + ...(threadId === null ? {} : { threadId: String(threadId) }), + cwd, + path: relativePath.split("/").filter(Boolean), + }); + }, [cwd, environmentId, navigation, relativePath, threadId]); const handleSelectFile = useCallback( (path: string) => { @@ -931,6 +965,8 @@ export function ThreadFileScreen(props: ThreadFileRouteScreenProps) { iconColor={iconColor} activeMode={resolvedActiveMode} fileInspectorSupported={fileInspector.supported} + canEditFile={canEditFile} + onEditFile={handleEditFile} onBack={handleBack} onReturnToThread={handleReturnToThread} actions={fileMenuActions} diff --git a/apps/mobile/src/features/files/fileEditing.test.ts b/apps/mobile/src/features/files/fileEditing.test.ts new file mode 100644 index 000000000000..8d0b0b8c55aa --- /dev/null +++ b/apps/mobile/src/features/files/fileEditing.test.ts @@ -0,0 +1,102 @@ +import type { ProjectReadFileResult } from "@t3tools/contracts"; +import { describe, expect, it } from "vite-plus/test"; + +import { + canEditWorkspaceFile, + fromEditorText, + MAX_EDITABLE_FILE_BYTES, + toEditorText, +} from "./fileEditing"; + +function readResult(overrides: Partial = {}): ProjectReadFileResult { + return { + relativePath: "src/main.ts", + contents: "const main = 1;\n", + byteLength: 16, + truncated: false, + revision: "revision-1", + ...overrides, + }; +} + +function canEdit(relativePath: string, file: ProjectReadFileResult | null = readResult()): boolean { + return canEditWorkspaceFile({ relativePath, file }); +} + +describe("editor text line endings", () => { + it("leaves an LF file unchanged", () => { + expect(toEditorText("one\ntwo\n")).toEqual({ + text: "one\ntwo\n", + lineEnding: "\n", + hasUtf8Bom: false, + }); + expect(fromEditorText("one\ntwo\n", "\n")).toBe("one\ntwo\n"); + }); + + it("round-trips a CRLF file byte-identical", () => { + const contents = "one\r\ntwo\r\n"; + const editor = toEditorText(contents); + + expect(editor).toEqual({ text: "one\ntwo\n", lineEnding: "\r\n", hasUtf8Bom: false }); + expect(fromEditorText(editor.text, editor.lineEnding)).toBe(contents); + }); + + it("keeps a missing trailing newline missing", () => { + const contents = "one\r\ntwo"; + const editor = toEditorText(contents); + + expect(fromEditorText(editor.text, editor.lineEnding)).toBe(contents); + expect(toEditorText("one\ntwo").text).toBe("one\ntwo"); + }); + + it("keeps the UTF-8 BOM outside the editable text and restores it with CRLF", () => { + const contents = "\uFEFFone\r\ntwo\r\n"; + const editor = toEditorText(contents); + + expect(editor.text).toBe("one\ntwo\n"); + expect(editor.hasUtf8Bom).toBe(true); + expect(fromEditorText(editor.text, editor.lineEnding, editor.hasUtf8Bom)).toBe(contents); + expect(fromEditorText("edited\n", editor.lineEnding, editor.hasUtf8Bom)).toBe( + "\uFEFFedited\r\n", + ); + }); + + it("does not double the CR of a CRLF the editor already holds", () => { + expect(fromEditorText("one\r\ntwo\n", "\r\n")).toBe("one\r\ntwo\r\n"); + }); +}); + +describe("canEditWorkspaceFile", () => { + it("accepts loaded source and Markdown files", () => { + expect(canEdit("src/main.ts")).toBe(true); + expect(canEdit("docs/readme.md")).toBe(true); + expect(canEdit("assets/page.html")).toBe(true); + }); + + it("rejects a file outside the workspace", () => { + expect(canEdit("/tmp/report.md")).toBe(false); + expect(canEdit("C:\\repo\\main.ts")).toBe(false); + }); + + it("rejects a read the server could not complete", () => { + expect(canEdit("src/main.ts", null)).toBe(false); + expect(canEdit("src/main.ts", readResult({ truncated: true }))).toBe(false); + const { revision: _revision, ...withoutRevision } = readResult(); + expect(canEdit("src/main.ts", withoutRevision)).toBe(false); + }); + + it("rejects a file past the editable size cap", () => { + expect(canEdit("src/main.ts", readResult({ byteLength: MAX_EDITABLE_FILE_BYTES }))).toBe(true); + expect(canEdit("src/main.ts", readResult({ byteLength: MAX_EDITABLE_FILE_BYTES + 1 }))).toBe( + false, + ); + }); + + it("rejects preview formats", () => { + expect(canEdit("assets/photo.png")).toBe(false); + expect(canEdit("assets/diagram.svg")).toBe(false); + expect(canEdit("docs/report.pdf")).toBe(false); + expect(canEdit("assets/clip.mp4")).toBe(false); + expect(canEdit("assets/voice.m4a")).toBe(false); + }); +}); diff --git a/apps/mobile/src/features/files/fileEditing.ts b/apps/mobile/src/features/files/fileEditing.ts new file mode 100644 index 000000000000..2b5083cda0f9 --- /dev/null +++ b/apps/mobile/src/features/files/fileEditing.ts @@ -0,0 +1,67 @@ +import type { ProjectReadFileResult } from "@t3tools/contracts"; +import { isWorkspaceImagePreviewPath } from "@t3tools/shared/filePreview"; + +import { isPdfFile } from "../../lib/filePreview"; +import { isAbsolutePath, isAudioPreviewFile, isVideoPreviewFile } from "./filePath"; + +/** + * A single native TextInput gets slow with large text on Android, so files past + * this size stay read-only. + */ +export const MAX_EDITABLE_FILE_BYTES = 256 * 1024; + +export type EditorLineEnding = "\r\n" | "\n"; + +/** + * Only a complete, workspace-relative text file can be saved back. A host file + * outside the workspace, a preview format, or a read the server could not + * complete (no revision, truncated, too large) has no editable draft. + */ +export function canEditWorkspaceFile(input: { + readonly relativePath: string; + readonly file: ProjectReadFileResult | null; +}): boolean { + const { file, relativePath } = input; + return ( + !isAbsolutePath(relativePath) && + file !== null && + !file.truncated && + // Servers that predate guarded writes send no revision, so there is nothing + // to detect a concurrent edit with. + file.revision !== undefined && + file.byteLength <= MAX_EDITABLE_FILE_BYTES && + !isVideoPreviewFile(relativePath) && + !isAudioPreviewFile(relativePath) && + !isWorkspaceImagePreviewPath(relativePath) && + !isPdfFile({ name: relativePath }) + ); +} + +/** + * The editor works in LF text. Remembering the file's line endings here is what + * lets `fromEditorText` write a CRLF file back byte-identical. + */ +export function toEditorText(contents: string): { + readonly text: string; + readonly lineEnding: EditorLineEnding; + readonly hasUtf8Bom: boolean; +} { + const hasUtf8Bom = contents.startsWith("\uFEFF"); + const text = hasUtf8Bom ? contents.slice(1) : contents; + return { + text: text.replaceAll("\r\n", "\n"), + lineEnding: text.includes("\r\n") ? "\r\n" : "\n", + hasUtf8Bom, + }; +} + +export function fromEditorText( + text: string, + lineEnding: EditorLineEnding, + hasUtf8Bom = false, +): string { + // Normalize first: an input method can insert its own CRLF into the LF text. + const contents = + lineEnding === "\r\n" ? text.replaceAll("\r\n", "\n").replaceAll("\n", "\r\n") : text; + return (hasUtf8Bom ? "\uFEFF" : "") + contents; +} diff --git a/apps/server/src/workspace/WorkspaceFileSystem.test.ts b/apps/server/src/workspace/WorkspaceFileSystem.test.ts index 9a22a27bd37c..0a78157f3b9f 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.test.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.test.ts @@ -3,6 +3,9 @@ import * as NodeChildProcess from "node:child_process"; import * as NodeServices from "@effect/platform-node/NodeServices"; import { it, describe, expect } from "@effect/vitest"; +import * as Deferred from "effect/Deferred"; +import * as Fiber from "effect/Fiber"; +import * as Exit from "effect/Exit"; import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; @@ -74,10 +77,93 @@ it.layer(layerTest, { excludeTestServices: true })("WorkspaceFileSystemLive", (i contents: "export const answer = 42;\n", byteLength: 26, truncated: false, + revision: expect.any(String), }); }), ); + it.effect( + "reports a revision that is stable across identical bytes and changes with contents", + () => + Effect.gen(function* () { + const workspaceFileSystem = yield* WorkspaceFileSystem.WorkspaceFileSystem; + const cwd = yield* makeTempDir; + yield* writeTextFile(cwd, "src/index.ts", "export const answer = 42;\n"); + + const first = yield* workspaceFileSystem.readFile({ cwd, relativePath: "src/index.ts" }); + const again = yield* workspaceFileSystem.readFile({ cwd, relativePath: "src/index.ts" }); + yield* writeTextFile(cwd, "src/index.ts", "export const answer = 43;\n"); + const changed = yield* workspaceFileSystem.readFile({ + cwd, + relativePath: "src/index.ts", + }); + + expect(first.revision).toEqual(expect.any(String)); + expect(again.revision).toBe(first.revision); + expect(changed.revision).not.toBe(first.revision); + }), + ); + + it.effect("keeps invalid UTF-8 readable without offering a revision for editing", () => + Effect.gen(function* () { + const workspaceFileSystem = yield* WorkspaceFileSystem.WorkspaceFileSystem; + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const cwd = yield* makeTempDir; + const bytes = Uint8Array.from([0x63, 0x61, 0x66, 0xe9, 0x0a]); + yield* fileSystem.writeFile(path.join(cwd, "legacy.txt"), bytes); + + const result = yield* workspaceFileSystem.readFile({ cwd, relativePath: "legacy.txt" }); + + expect(result.contents).toBe("caf\uFFFD\n"); + expect(result.truncated).toBe(false); + expect(result.revision).toBeUndefined(); + expect(Array.from(yield* fileSystem.readFile(path.join(cwd, "legacy.txt")))).toEqual( + Array.from(bytes), + ); + }), + ); + + it.effect("preserves a UTF-8 BOM through a guarded edit", () => + Effect.gen(function* () { + const workspaceFileSystem = yield* WorkspaceFileSystem.WorkspaceFileSystem; + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const cwd = yield* makeTempDir; + yield* writeTextFile(cwd, "bom.txt", "\uFEFFone\r\n"); + const original = yield* workspaceFileSystem.readFile({ cwd, relativePath: "bom.txt" }); + expect(original.contents).toBe("\uFEFFone\r\n"); + expect(original.revision).toEqual(expect.any(String)); + + yield* workspaceFileSystem.writeFile({ + cwd, + relativePath: "bom.txt", + contents: original.contents.replace("one", "two"), + expectedRevision: original.revision!, + }); + + expect(Array.from(yield* fileSystem.readFile(path.join(cwd, "bom.txt")))).toEqual( + Array.from(new TextEncoder().encode("\uFEFFtwo\r\n")), + ); + }), + ); + + it.effect("omits the revision for a truncated read", () => + Effect.gen(function* () { + const workspaceFileSystem = yield* WorkspaceFileSystem.WorkspaceFileSystem; + const cwd = yield* makeTempDir; + const oneMiB = 1024 * 1024; + yield* writeTextFile(cwd, "large.txt", "a".repeat(oneMiB + 1)); + + const result = yield* workspaceFileSystem.readFile({ cwd, relativePath: "large.txt" }); + + expect(result.truncated).toBe(true); + expect(result.byteLength).toBe(oneMiB + 1); + expect(result.contents.length).toBe(oneMiB); + expect("revision" in result).toBe(false); + }), + ); + it.effect("reads host files outside the workspace root by absolute path", () => Effect.gen(function* () { const workspaceFileSystem = yield* WorkspaceFileSystem.WorkspaceFileSystem; @@ -97,6 +183,7 @@ it.layer(layerTest, { excludeTestServices: true })("WorkspaceFileSystemLive", (i contents: "# Report\n", byteLength: 9, truncated: false, + revision: expect.any(String), }); }), ); @@ -337,5 +424,244 @@ it.layer(layerTest, { excludeTestServices: true })("WorkspaceFileSystemLive", (i expect(escapedStat).toBeNull(); }), ); + + it.effect("writes when the revision from a prior read still matches", () => + Effect.gen(function* () { + const workspaceFileSystem = yield* WorkspaceFileSystem.WorkspaceFileSystem; + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const cwd = yield* makeTempDir; + yield* writeTextFile(cwd, "src/index.ts", "export const answer = 42;\n"); + const revision = (yield* workspaceFileSystem.readFile({ + cwd, + relativePath: "src/index.ts", + })).revision; + expect(revision).toEqual(expect.any(String)); + + const result = yield* workspaceFileSystem.writeFile({ + cwd, + relativePath: "src/index.ts", + contents: "export const answer = 43;\n", + expectedRevision: revision!, + }); + + expect(result).toEqual({ relativePath: "src/index.ts" }); + const saved = yield* fileSystem + .readFileString(path.join(cwd, "src/index.ts")) + .pipe(Effect.orDie); + expect(saved).toBe("export const answer = 43;\n"); + }), + ); + + it.effect("rejects a stale revision without overwriting the changed file", () => + Effect.gen(function* () { + const workspaceFileSystem = yield* WorkspaceFileSystem.WorkspaceFileSystem; + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const cwd = yield* makeTempDir; + yield* writeTextFile(cwd, "src/index.ts", "export const answer = 42;\n"); + const revision = (yield* workspaceFileSystem.readFile({ + cwd, + relativePath: "src/index.ts", + })).revision; + expect(revision).toEqual(expect.any(String)); + yield* writeTextFile(cwd, "src/index.ts", "// changed on disk\n"); + + const error = yield* workspaceFileSystem + .writeFile({ + cwd, + relativePath: "src/index.ts", + contents: "export const answer = 43;\n", + expectedRevision: revision!, + }) + .pipe(Effect.flip); + + expect(error._tag).toBe("WorkspaceFileChangedError"); + expect(error).toMatchObject({ + workspaceRoot: cwd, + relativePath: "src/index.ts", + }); + const saved = yield* fileSystem + .readFileString(path.join(cwd, "src/index.ts")) + .pipe(Effect.orDie); + expect(saved).toBe("// changed on disk\n"); + }), + ); + + it.effect("rejects a revision for a file deleted after it was read", () => + Effect.gen(function* () { + const workspaceFileSystem = yield* WorkspaceFileSystem.WorkspaceFileSystem; + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const cwd = yield* makeTempDir; + yield* writeTextFile(cwd, "src/index.ts", "export const answer = 42;\n"); + const revision = (yield* workspaceFileSystem.readFile({ + cwd, + relativePath: "src/index.ts", + })).revision; + expect(revision).toEqual(expect.any(String)); + yield* fileSystem.remove(path.join(cwd, "src/index.ts")).pipe(Effect.orDie); + + const error = yield* workspaceFileSystem + .writeFile({ + cwd, + relativePath: "src/index.ts", + contents: "export const answer = 43;\n", + expectedRevision: revision!, + }) + .pipe(Effect.flip); + + expect(error._tag).toBe("WorkspaceFileChangedError"); + const stat = yield* fileSystem + .stat(path.join(cwd, "src/index.ts")) + .pipe(Effect.orElseSucceed(() => null)); + expect(stat).toBeNull(); + }), + ); + + it.effect("serializes guarded saves to one file without blocking another file", () => + Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const cwd = yield* makeTempDir; + yield* writeTextFile(cwd, "first.txt", "original"); + yield* writeTextFile(cwd, "other.txt", "original"); + const writeEntered = yield* Deferred.make(); + const releaseWrite = yield* Deferred.make(); + const service = yield* WorkspaceFileSystem.make.pipe( + Effect.provideService(FileSystem.FileSystem, { + ...fileSystem, + writeFileString: (filePath, contents, options) => + Effect.gen(function* () { + if (contents === "first save") { + yield* Deferred.succeed(writeEntered, undefined); + yield* Deferred.await(releaseWrite); + } + yield* fileSystem.writeFileString(filePath, contents, options); + }), + }), + ); + const original = yield* service.readFile({ cwd, relativePath: "first.txt" }); + const first = yield* service + .writeFile({ + cwd, + relativePath: "first.txt", + contents: "first save", + expectedRevision: original.revision!, + }) + .pipe(Effect.forkChild); + yield* Deferred.await(writeEntered); + const second = yield* service + .writeFile({ + cwd, + relativePath: "./first.txt", + contents: "second save", + expectedRevision: original.revision!, + }) + .pipe(Effect.flip, Effect.forkChild); + + // This must finish while the first file's write is still blocked. + yield* service.writeFile({ cwd, relativePath: "other.txt", contents: "independent" }); + expect(yield* fileSystem.readFileString(path.join(cwd, "other.txt"))).toBe("independent"); + yield* Deferred.succeed(releaseWrite, undefined); + yield* Fiber.join(first); + expect((yield* Fiber.join(second))._tag).toBe("WorkspaceFileChangedError"); + expect(yield* fileSystem.readFileString(path.join(cwd, "first.txt"))).toBe("first save"); + }), + ); + + it.effect.skipIf(!symlinksSupported)("guards concurrent saves through symlink aliases", () => + Effect.gen(function* () { + const service = yield* WorkspaceFileSystem.WorkspaceFileSystem; + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const cwd = yield* makeTempDir; + yield* writeTextFile(cwd, "file.txt", "original"); + yield* fs.symlink(path.join(cwd, "file.txt"), path.join(cwd, "link.txt")); + const original = yield* service.readFile({ cwd, relativePath: "file.txt" }); + const results = yield* Effect.forEach( + ["file.txt", "link.txt"], + (relativePath) => + service + .writeFile({ + cwd, + relativePath, + contents: relativePath, + expectedRevision: original.revision!, + }) + .pipe(Effect.exit), + { concurrency: "unbounded" }, + ); + expect(results.filter(Exit.isSuccess)).toHaveLength(1); + const saved = yield* fs.readFileString(path.join(cwd, "file.txt")); + expect(["file.txt", "link.txt"]).toContain(saved); + }), + ); + + it.effect("releases the file lock before refreshing the workspace index", () => + Effect.gen(function* () { + const entries = yield* WorkspaceEntries.WorkspaceEntries; + const cwd = yield* makeTempDir; + yield* writeTextFile(cwd, "file.txt", "original"); + const refreshEntered = yield* Deferred.make(); + const releaseRefresh = yield* Deferred.make(); + let firstRefresh = true; + const service = yield* WorkspaceFileSystem.make.pipe( + Effect.provideService(WorkspaceEntries.WorkspaceEntries, { + ...entries, + refresh: () => + Effect.gen(function* () { + if (firstRefresh) { + firstRefresh = false; + yield* Deferred.succeed(refreshEntered, undefined); + yield* Deferred.await(releaseRefresh); + } + }), + }), + ); + const first = yield* service + .writeFile({ + cwd, + relativePath: "file.txt", + contents: "first save", + }) + .pipe(Effect.forkChild); + yield* Deferred.await(refreshEntered); + const current = yield* service.readFile({ cwd, relativePath: "file.txt" }); + yield* service.writeFile({ + cwd, + relativePath: "file.txt", + contents: "second save", + expectedRevision: current.revision!, + }); + expect((yield* service.readFile({ cwd, relativePath: "file.txt" })).contents).toBe( + "second save", + ); + yield* Deferred.succeed(releaseRefresh, undefined); + yield* Fiber.join(first); + }), + ); + + it.effect("overwrites a changed file when no revision is expected", () => + Effect.gen(function* () { + const workspaceFileSystem = yield* WorkspaceFileSystem.WorkspaceFileSystem; + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const cwd = yield* makeTempDir; + yield* writeTextFile(cwd, "src/index.ts", "export const answer = 42;\n"); + yield* writeTextFile(cwd, "src/index.ts", "// changed on disk\n"); + + yield* workspaceFileSystem.writeFile({ + cwd, + relativePath: "src/index.ts", + contents: "export const answer = 43;\n", + }); + + const saved = yield* fileSystem + .readFileString(path.join(cwd, "src/index.ts")) + .pipe(Effect.orDie); + expect(saved).toBe("export const answer = 43;\n"); + }), + ); }); }); diff --git a/apps/server/src/workspace/WorkspaceFileSystem.ts b/apps/server/src/workspace/WorkspaceFileSystem.ts index 9ef3d4546d1d..354ff4e3c3af 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.ts @@ -9,6 +9,7 @@ * * @module WorkspaceFileSystem */ +import * as NodeBuffer from "node:buffer"; import * as NodeFS from "node:fs"; import * as NodeFSP from "node:fs/promises"; @@ -19,11 +20,14 @@ import type { ProjectWriteFileResult, } from "@t3tools/contracts"; import * as Context from "effect/Context"; +import * as Crypto from "effect/Crypto"; import * as Effect from "effect/Effect"; +import * as Hex from "effect/encoding/Hex"; import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import * as Path from "effect/Path"; import * as Schema from "effect/Schema"; +import * as KeyedLock from "@t3tools/shared/KeyedLock"; import * as WorkspaceEntries from "./WorkspaceEntries.ts"; import * as WorkspacePaths from "./WorkspacePaths.ts"; @@ -95,11 +99,25 @@ export class WorkspaceBinaryFileError extends Schema.TaggedError()( + "WorkspaceFileChangedError", + { + workspaceRoot: Schema.String, + relativePath: Schema.String, + resolvedPath: Schema.String, + }, +) { + override get message(): string { + return `Workspace file '${this.relativePath}' in '${this.workspaceRoot}' changed since it was read.`; + } +} + export const WorkspaceFileSystemError = Schema.Union([ WorkspaceFileSystemOperationError, WorkspaceFilePathEscapeError, WorkspacePathNotFileError, WorkspaceBinaryFileError, + WorkspaceFileChangedError, ]); export type WorkspaceFileSystemError = typeof WorkspaceFileSystemError.Type; @@ -121,7 +139,10 @@ export class WorkspaceFileSystem extends Context.Service< * Write a file relative to the workspace root. * * Creates parent directories as needed and rejects paths that escape the - * workspace root. + * workspace root. `expectedRevision` is a best-effort pre-write check, failing + * with `WorkspaceFileChangedError` on a mismatch. Service writes to the same + * file are serialized, but external changes after the check may be overwritten; + * this is not an atomic compare-and-write. */ readonly writeFile: ( input: ProjectWriteFileInput, @@ -138,6 +159,12 @@ export const make = Effect.gen(function* () { const path = yield* Path.Path; const workspacePaths = yield* WorkspacePaths.WorkspacePaths; const workspaceEntries = yield* WorkspaceEntries.WorkspaceEntries; + const crypto = yield* Crypto.Crypto; + const writeLocks = yield* KeyedLock.make(); + + /** The revision a complete read reports and a guarded write compares against. */ + const fileRevision = (bytes: Uint8Array) => + crypto.digest("SHA-256", bytes).pipe(Effect.map(Hex.encode), Effect.orDie); /** * Resolves the file a read targets. Workspace-relative paths must stay inside the @@ -279,11 +306,17 @@ export const make = Effect.gen(function* () { }); } + const truncated = stat.size > PROJECT_READ_FILE_MAX_BYTES; + // A short read holds only part of the file, so it gets no revision to write back with. + const complete = !truncated && bytesRead === stat.size; + // Lossy previews remain readable, but cannot become editable drafts. + const editable = complete && NodeBuffer.isUtf8(fileBytes); return { relativePath: target.relativePath, - contents: new TextDecoder("utf-8").decode(fileBytes), + contents: new TextDecoder("utf-8", { ignoreBOM: true }).decode(fileBytes), byteLength: stat.size, - truncated: stat.size > PROJECT_READ_FILE_MAX_BYTES, + truncated, + ...(editable ? { revision: yield* fileRevision(fileBytes) } : {}), }; }), (handle) => @@ -310,20 +343,9 @@ export const make = Effect.gen(function* () { relativePath: input.relativePath, }); - yield* fileSystem.makeDirectory(path.dirname(target.absolutePath), { recursive: true }).pipe( - Effect.mapError( - (cause) => - new WorkspaceFileSystemOperationError({ - workspaceRoot: input.cwd, - relativePath: input.relativePath, - resolvedPath: target.absolutePath, - operationPath: path.dirname(target.absolutePath), - operation: "make-directory", - cause, - }), - ), - ); - yield* fileSystem.writeFileString(target.absolutePath, input.contents).pipe( + // Canonicalize existing files so symlink aliases share the same lock. + const lockPath = yield* fileSystem.realPath(target.absolutePath).pipe( + Effect.catchReason("PlatformError", "NotFound", () => Effect.succeed(target.absolutePath)), Effect.mapError( (cause) => new WorkspaceFileSystemOperationError({ @@ -331,11 +353,71 @@ export const make = Effect.gen(function* () { relativePath: input.relativePath, resolvedPath: target.absolutePath, operationPath: target.absolutePath, - operation: "write-file", + operation: "realpath-target", cause, }), ), ); + yield* writeLocks.withLock( + lockPath, + Effect.gen(function* () { + if (input.expectedRevision !== undefined) { + const currentBytes = yield* fileSystem.readFile(target.absolutePath).pipe( + Effect.catchReason("PlatformError", "NotFound", () => Effect.succeed(null)), + Effect.mapError( + (cause) => + new WorkspaceFileSystemOperationError({ + workspaceRoot: input.cwd, + relativePath: input.relativePath, + resolvedPath: target.absolutePath, + operationPath: target.absolutePath, + operation: "read", + cause, + }), + ), + ); + if ( + currentBytes === null || + (yield* fileRevision(currentBytes)) !== input.expectedRevision + ) { + return yield* new WorkspaceFileChangedError({ + workspaceRoot: input.cwd, + relativePath: input.relativePath, + resolvedPath: target.absolutePath, + }); + } + } + + yield* fileSystem + .makeDirectory(path.dirname(target.absolutePath), { recursive: true }) + .pipe( + Effect.mapError( + (cause) => + new WorkspaceFileSystemOperationError({ + workspaceRoot: input.cwd, + relativePath: input.relativePath, + resolvedPath: target.absolutePath, + operationPath: path.dirname(target.absolutePath), + operation: "make-directory", + cause, + }), + ), + ); + yield* fileSystem.writeFileString(target.absolutePath, input.contents).pipe( + Effect.mapError( + (cause) => + new WorkspaceFileSystemOperationError({ + workspaceRoot: input.cwd, + relativePath: input.relativePath, + resolvedPath: target.absolutePath, + operationPath: target.absolutePath, + operation: "write-file", + cause, + }), + ), + ); + }), + ); yield* workspaceEntries.refresh(input.cwd); return { relativePath: target.relativePath }; }); diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index af3fc819a4c7..0fce902fead0 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -532,6 +532,8 @@ function projectFileFailureContext( return { failure: "path_not_file", resolvedPath: error.resolvedPath }; case "WorkspaceBinaryFileError": return { failure: "binary_file", resolvedPath: error.resolvedPath }; + case "WorkspaceFileChangedError": + return { failure: "file_changed", resolvedPath: error.resolvedPath }; default: return unexpectedCompatibilityError(error); } diff --git a/packages/client-runtime/src/state/commandPermissions.test.ts b/packages/client-runtime/src/state/commandPermissions.test.ts index b9d1ebf6b738..7824fe2c514c 100644 --- a/packages/client-runtime/src/state/commandPermissions.test.ts +++ b/packages/client-runtime/src/state/commandPermissions.test.ts @@ -7,6 +7,7 @@ import type { RpcSession } from "../rpc/session.ts"; import { describe, expect, it } from "@effect/vitest"; import { vi } from "vite-plus/test"; import { + AuthFilesystemWriteScope, AuthOrchestrationOperateScope, AuthSourceControlWriteScope, ThreadId, @@ -118,6 +119,29 @@ describe("command permissions", () => { }), ), ); + it.effect("requires filesystem:write for guarded file writes", () => + Effect.scoped( + Effect.gen(function* () { + const registry = yield* setup; + const writeFile = createCommandPermissions(runtime, WS_METHODS.projectsWriteFile); + registry.set(sessions(env), AsyncResult.success(grant(true))); + expect(registry.get(writeFile.permissionAtom(env))).toBe(false); + expect( + (yield* writeFile.authorize(registry, env).pipe(Effect.flip)).requiredPermission, + ).toBe(AuthFilesystemWriteScope); + registry.set( + sessions(env), + AsyncResult.success({ + ...grant(true), + scopes: [AuthFilesystemWriteScope], + permissions: [AuthFilesystemWriteScope], + }), + ); + expect(registry.get(writeFile.permissionAtom(env))).toBe(true); + yield* writeFile.authorize(registry, env); + }), + ), + ); it("rechecks permission after waiting in a serial command lane", async () => { const registry = AtomRegistry.make(); const unmount = registry.mount(sessions(env)); diff --git a/packages/contracts/src/clientRpcPermissions.ts b/packages/contracts/src/clientRpcPermissions.ts index 9605829e47d7..f2c85377e414 100644 --- a/packages/contracts/src/clientRpcPermissions.ts +++ b/packages/contracts/src/clientRpcPermissions.ts @@ -1,6 +1,7 @@ import * as Schema from "effect/Schema"; import { GitPreparePullRequestThreadInput } from "./git.ts"; import { + AuthFilesystemWriteScope, AuthOrchestrationOperateScope, AuthSourceControlWriteScope, type AuthEnvironmentScope, @@ -34,6 +35,8 @@ export const CLIENT_GUARDED_RPC_SCOPES = { [WS_METHODS.vcsSwitchRef]: AuthSourceControlWriteScope, [WS_METHODS.vcsInit]: AuthSourceControlWriteScope, + [WS_METHODS.projectsWriteFile]: AuthFilesystemWriteScope, + [WS_METHODS.scheduledTasksUpsert]: AuthOrchestrationOperateScope, [WS_METHODS.scheduledTasksSetEnabled]: AuthOrchestrationOperateScope, [WS_METHODS.scheduledTasksDelete]: AuthOrchestrationOperateScope, diff --git a/packages/contracts/src/project.ts b/packages/contracts/src/project.ts index d1d13e82ac37..4bef42786ef1 100644 --- a/packages/contracts/src/project.ts +++ b/packages/contracts/src/project.ts @@ -446,6 +446,12 @@ export const ProjectReadFileResult = Schema.Struct({ contents: Schema.String, byteLength: NonNegativeInt, truncated: Schema.Boolean, + /** + * Identifies the bytes read, for `ProjectWriteFileInput.expectedRevision`. Present only for a + * complete, valid UTF-8 read. Older servers never send one, so clients can use its presence + * to detect support for guarded writes. + */ + revision: Schema.optionalKey(TrimmedNonEmptyString), }); export type ProjectReadFileResult = typeof ProjectReadFileResult.Type; @@ -455,6 +461,8 @@ export const ProjectFileFailure = Schema.Literals([ "path_not_file", "binary_file", "operation_failed", + /** A guarded write found different contents, or no file, where it expected a revision. */ + "file_changed", ]); export type ProjectFileFailure = typeof ProjectFileFailure.Type; @@ -510,6 +518,13 @@ export const ProjectWriteFileInput = Schema.Struct({ cwd: TrimmedNonEmptyString, relativePath: TrimmedNonEmptyString.check(Schema.isMaxLength(PROJECT_WRITE_FILE_PATH_MAX_LENGTH)), contents: Schema.String, + /** + * Best-effort pre-write check against `ProjectReadFileResult.revision`; a mismatch fails + * with `file_changed`. Writes through this server to the same file are serialized, but this + * is not an atomic compare-and-write: external changes after the check may be overwritten. + * Omit it to overwrite unconditionally. + */ + expectedRevision: Schema.optionalKey(TrimmedNonEmptyString), }); export type ProjectWriteFileInput = typeof ProjectWriteFileInput.Type; @@ -558,7 +573,9 @@ export class ProjectWriteFileError extends Schema.TaggedError