From ea16367b30b3b61ec7ca0a28cb3bf6c86ac70914 Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 07:12:49 +0800 Subject: [PATCH 01/54] feat(contracts): add projects.createUploadUrl for workspace uploads --- apps/server/src/auth/RpcAuthorization.ts | 1 + apps/server/src/ws.ts | 6 ++++ packages/contracts/src/project.ts | 40 ++++++++++++++++++++++++ packages/contracts/src/rpc.ts | 16 ++++++++++ 4 files changed, 63 insertions(+) diff --git a/apps/server/src/auth/RpcAuthorization.ts b/apps/server/src/auth/RpcAuthorization.ts index 28ceac4cec99..f64d14784aff 100644 --- a/apps/server/src/auth/RpcAuthorization.ts +++ b/apps/server/src/auth/RpcAuthorization.ts @@ -81,6 +81,7 @@ export const RPC_REQUIRED_SCOPES = { [WS_METHODS.projectsSearchContents]: AuthOrchestrationReadScope, [WS_METHODS.projectsSearchEntries]: AuthOrchestrationReadScope, [WS_METHODS.projectsWriteFile]: AuthOrchestrationOperateScope, + [WS_METHODS.projectsCreateUploadUrl]: AuthOrchestrationOperateScope, [WS_METHODS.shellOpenInEditor]: AuthOrchestrationOperateScope, [WS_METHODS.filesystemBrowse]: AuthOrchestrationReadScope, [WS_METHODS.assetsCreateUrl]: AuthOrchestrationReadScope, diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index 55b0be07c667..ff4d012bd6fd 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -34,6 +34,7 @@ import { OrchestrationGetTurnDiffError, ORCHESTRATION_WS_METHODS, type ProjectId, + ProjectCreateUploadUrlError, type ProjectEntriesFailure, type ProjectFileFailure, type ProjectFileOperation, @@ -1938,6 +1939,11 @@ const makeWsRpcLayer = ( ), { "rpc.aggregate": "workspace" }, ), + // Task 3 replaces this with the real handler. + [WS_METHODS.projectsCreateUploadUrl]: () => + Effect.fail( + new ProjectCreateUploadUrlError({ message: "Workspace uploads are not wired up yet." }), + ), [WS_METHODS.shellOpenInEditor]: (input) => observeRpcEffect(WS_METHODS.shellOpenInEditor, externalLauncher.launchEditor(input), { "rpc.aggregate": "workspace", diff --git a/packages/contracts/src/project.ts b/packages/contracts/src/project.ts index 757c000a065a..e036fb0ea387 100644 --- a/packages/contracts/src/project.ts +++ b/packages/contracts/src/project.ts @@ -298,3 +298,43 @@ export class ProjectWriteFileError extends Schema.TaggedErrorClass()( + "ProjectUploadTargetExistsError", + { + cwd: TrimmedNonEmptyString, + relativePath: TrimmedNonEmptyString, + }, +) { + override get message(): string { + return `A file already exists at '${this.relativePath}' in '${this.cwd}'.`; + } +} + +export class ProjectCreateUploadUrlError extends Schema.TaggedErrorClass()( + "ProjectCreateUploadUrlError", + { + cwd: Schema.optional(TrimmedNonEmptyString), + relativePath: Schema.optional(TrimmedNonEmptyString), + message: TrimmedNonEmptyString, + cause: Schema.optional(Schema.Defect()), + }, +) {} diff --git a/packages/contracts/src/rpc.ts b/packages/contracts/src/rpc.ts index 14363cfedff9..0e3d8600532c 100644 --- a/packages/contracts/src/rpc.ts +++ b/packages/contracts/src/rpc.ts @@ -112,6 +112,9 @@ import { RelayClientStatusSchema, } from "./relayClient.ts"; import { + ProjectCreateUploadUrlError, + ProjectCreateUploadUrlInput, + ProjectCreateUploadUrlResult, ProjectListEntriesError, ProjectListEntriesInput, ProjectListEntriesResult, @@ -124,6 +127,7 @@ import { ProjectSearchEntriesError, ProjectSearchEntriesInput, ProjectSearchEntriesResult, + ProjectUploadTargetExistsError, ProjectWriteFileError, ProjectWriteFileInput, ProjectWriteFileResult, @@ -216,6 +220,7 @@ export const WS_METHODS = { projectsSearchContents: "projects.searchContents", projectsSearchEntries: "projects.searchEntries", projectsWriteFile: "projects.writeFile", + projectsCreateUploadUrl: "projects.createUploadUrl", // Shell methods shellOpenInEditor: "shell.openInEditor", @@ -666,6 +671,16 @@ export const WsProjectsWriteFileRpc = Rpc.make(WS_METHODS.projectsWriteFile, { error: Schema.Union([ProjectWriteFileError, EnvironmentAuthorizationError]), }); +export const WsProjectsCreateUploadUrlRpc = Rpc.make(WS_METHODS.projectsCreateUploadUrl, { + payload: ProjectCreateUploadUrlInput, + success: ProjectCreateUploadUrlResult, + error: Schema.Union([ + ProjectCreateUploadUrlError, + ProjectUploadTargetExistsError, + EnvironmentAuthorizationError, + ]), +}); + export const WsShellOpenInEditorRpc = Rpc.make(WS_METHODS.shellOpenInEditor, { payload: LaunchEditorInput, error: Schema.Union([ExternalLauncherError, EnvironmentAuthorizationError]), @@ -1066,6 +1081,7 @@ export const WsRpcGroup = RpcGroup.make( WsProjectsSearchContentsRpc, WsProjectsSearchEntriesRpc, WsProjectsWriteFileRpc, + WsProjectsCreateUploadUrlRpc, WsShellOpenInEditorRpc, WsFilesystemBrowseRpc, WsAssetsCreateUrlRpc, From 552e1a0317e217ec4d8579b7032f02d8f80eca0c Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 07:18:56 +0800 Subject: [PATCH 02/54] feat(server): sign and store workspace file uploads --- .../src/workspace/WorkspaceUpload.test.ts | 216 ++++++++++++++++ apps/server/src/workspace/WorkspaceUpload.ts | 230 ++++++++++++++++++ 2 files changed, 446 insertions(+) create mode 100644 apps/server/src/workspace/WorkspaceUpload.test.ts create mode 100644 apps/server/src/workspace/WorkspaceUpload.ts diff --git a/apps/server/src/workspace/WorkspaceUpload.test.ts b/apps/server/src/workspace/WorkspaceUpload.test.ts new file mode 100644 index 000000000000..862be40e36e2 --- /dev/null +++ b/apps/server/src/workspace/WorkspaceUpload.test.ts @@ -0,0 +1,216 @@ +// @effect-diagnostics nodeBuiltinImport:off +import * as NodeFS from "node:fs"; +import * as NodePath from "node:path"; + +import * as NodeServices from "@effect/platform-node/NodeServices"; +import { describe, expect, it } from "@effect/vitest"; +import { PROJECT_UPLOAD_URL_TTL_MS } from "@t3tools/contracts"; +import * as Clock from "effect/Clock"; +import * as Effect from "effect/Effect"; +import * as FileSystem from "effect/FileSystem"; +import * as Layer from "effect/Layer"; +import * as Schema from "effect/Schema"; +import * as TestClock from "effect/testing/TestClock"; + +import { base64UrlEncode, signPayload } from "../auth/utils.ts"; +import * as ServerSecretStore from "../auth/ServerSecretStore.ts"; +import * as ServerConfig from "../config.ts"; +import * as VcsProcess from "../vcs/VcsProcess.ts"; +import * as WorkspaceEntries from "./WorkspaceEntries.ts"; +import * as WorkspacePaths from "./WorkspacePaths.ts"; +import { + WORKSPACE_UPLOAD_ROUTE_PREFIX, + issueWorkspaceUploadUrl, + storeWorkspaceUpload, + validateWorkspaceUploadToken, +} from "./WorkspaceUpload.ts"; + +const testLayer = Layer.empty.pipe( + Layer.provideMerge(ServerSecretStore.layer), + Layer.provideMerge(WorkspaceEntries.layer.pipe(Layer.provide(WorkspacePaths.layer))), + Layer.provideMerge(WorkspacePaths.layer), + Layer.provideMerge(VcsProcess.layer), + Layer.provide(ServerConfig.layerTest(process.cwd(), { prefix: "t3-workspace-upload-test-" })), + Layer.provideMerge(NodeServices.layer), +); + +const makeTempWorkspaceRoot = Effect.fn("makeTempWorkspaceRoot")(function* () { + const fileSystem = yield* FileSystem.FileSystem; + return yield* fileSystem.makeTempDirectoryScoped({ + prefix: "t3code-workspace-upload-", + }); +}); + +function tokenFromRelativeUrl(relativeUrl: string): string { + return relativeUrl.slice(`${WORKSPACE_UPLOAD_ROUTE_PREFIX}/`.length); +} + +const AttachmentUploadClaimsForTest = Schema.Struct({ + version: Schema.Literal(1), + kind: Schema.Literal("attachment-upload"), + attachmentId: Schema.String, + name: Schema.String, + mimeType: Schema.String, + sizeBytes: Schema.Number, + expiresAt: Schema.Number, +}); +const encodeAttachmentUploadClaimsForTest = Schema.encodeSync( + Schema.fromJsonString(AttachmentUploadClaimsForTest), +); + +describe("WorkspaceUpload", () => { + it.effect("mints, validates, and stores an upload roundtrip", () => + Effect.gen(function* () { + const cwd = yield* makeTempWorkspaceRoot(); + const bytes = new Uint8Array([1, 2, 3, 4]); + + const issued = yield* issueWorkspaceUploadUrl({ + cwd, + relativePath: "sub/dir/file.bin", + sizeBytes: bytes.byteLength, + }); + expect(issued.relativePath).toBe("sub/dir/file.bin"); + + const claims = yield* validateWorkspaceUploadToken(tokenFromRelativeUrl(issued.relativeUrl)); + if (!claims) { + throw new Error("Expected valid upload claims."); + } + + const result = yield* storeWorkspaceUpload(claims, bytes); + expect(result).toEqual({ ok: true, relativePath: "sub/dir/file.bin" }); + + const finalPath = NodePath.join(cwd, "sub/dir/file.bin"); + expect(NodeFS.existsSync(finalPath)).toBe(true); + expect(NodeFS.readFileSync(finalPath)).toEqual(Buffer.from(bytes)); + const siblingEntries = NodeFS.readdirSync(NodePath.dirname(finalPath)); + expect(siblingEntries.some((entry) => entry.endsWith(".part"))).toBe(false); + }).pipe(Effect.provide(testLayer)), + ); + + it.effect("rejects a mint target that escapes the workspace root", () => + Effect.gen(function* () { + const cwd = yield* makeTempWorkspaceRoot(); + + const error = yield* issueWorkspaceUploadUrl({ + cwd, + relativePath: "../outside.txt", + sizeBytes: 3, + }).pipe(Effect.flip); + + expect(error._tag).toBe("ProjectCreateUploadUrlError"); + }).pipe(Effect.provide(testLayer)), + ); + + it.effect("rejects mint on an existing file without overwrite, allows it with overwrite", () => + Effect.gen(function* () { + const cwd = yield* makeTempWorkspaceRoot(); + const fileSystem = yield* FileSystem.FileSystem; + yield* fileSystem.writeFileString(NodePath.join(cwd, "existing.txt"), "old"); + + const rejected = yield* issueWorkspaceUploadUrl({ + cwd, + relativePath: "existing.txt", + sizeBytes: 3, + }).pipe(Effect.flip); + expect(rejected._tag).toBe("ProjectUploadTargetExistsError"); + + const bytes = new Uint8Array([9, 9, 9]); + const issued = yield* issueWorkspaceUploadUrl({ + cwd, + relativePath: "existing.txt", + sizeBytes: bytes.byteLength, + overwrite: true, + }); + const claims = yield* validateWorkspaceUploadToken(tokenFromRelativeUrl(issued.relativeUrl)); + if (!claims) { + throw new Error("Expected valid upload claims."); + } + + const result = yield* storeWorkspaceUpload(claims, bytes); + expect(result).toEqual({ ok: true, relativePath: "existing.txt" }); + expect(NodeFS.readFileSync(NodePath.join(cwd, "existing.txt"))).toEqual(Buffer.from(bytes)); + }).pipe(Effect.provide(testLayer)), + ); + + it.effect("rejects a store body whose size does not match the claims", () => + Effect.gen(function* () { + const cwd = yield* makeTempWorkspaceRoot(); + const issued = yield* issueWorkspaceUploadUrl({ + cwd, + relativePath: "file.bin", + sizeBytes: 4, + }); + const claims = yield* validateWorkspaceUploadToken(tokenFromRelativeUrl(issued.relativeUrl)); + if (!claims) { + throw new Error("Expected valid upload claims."); + } + + const result = yield* storeWorkspaceUpload(claims, new Uint8Array([1, 2, 3])); + expect(result).toMatchObject({ ok: false, status: 400 }); + }).pipe(Effect.provide(testLayer)), + ); + + it.effect("rejects a store when the target appeared after mint without overwrite", () => + Effect.gen(function* () { + const cwd = yield* makeTempWorkspaceRoot(); + const fileSystem = yield* FileSystem.FileSystem; + const bytes = new Uint8Array([5, 6, 7]); + + const issued = yield* issueWorkspaceUploadUrl({ + cwd, + relativePath: "race.txt", + sizeBytes: bytes.byteLength, + }); + const claims = yield* validateWorkspaceUploadToken(tokenFromRelativeUrl(issued.relativeUrl)); + if (!claims) { + throw new Error("Expected valid upload claims."); + } + + yield* fileSystem.writeFileString(NodePath.join(cwd, "race.txt"), "raced"); + + const result = yield* storeWorkspaceUpload(claims, bytes); + expect(result).toMatchObject({ ok: false, status: 409 }); + }).pipe(Effect.provide(testLayer)), + ); + + it.effect("rejects tampered, malformed, expired, and cross-kind tokens", () => + Effect.gen(function* () { + const cwd = yield* makeTempWorkspaceRoot(); + const issued = yield* issueWorkspaceUploadUrl({ + cwd, + relativePath: "file.bin", + sizeBytes: 3, + }); + const token = tokenFromRelativeUrl(issued.relativeUrl); + const [payload, signature] = token.split("."); + + expect(yield* validateWorkspaceUploadToken(`${payload}x.${signature}`)).toBeNull(); + expect(yield* validateWorkspaceUploadToken("garbage")).toBeNull(); + + yield* TestClock.adjust(PROJECT_UPLOAD_URL_TTL_MS + 1); + expect(yield* validateWorkspaceUploadToken(token)).toBeNull(); + }).pipe(Effect.provide(testLayer)), + ); + + it.effect("rejects an attachment-upload token presented to the workspace validator", () => + Effect.gen(function* () { + const secretStore = yield* ServerSecretStore.ServerSecretStore; + const secret = yield* secretStore.getOrCreateRandom("asset-access-signing-key", 32); + const nowMs = yield* Clock.currentTimeMillis; + const encodedPayload = base64UrlEncode( + encodeAttachmentUploadClaimsForTest({ + version: 1, + kind: "attachment-upload", + attachmentId: "pending-00000000-0000-4000-8000-000000000000", + name: "file.png", + mimeType: "image/png", + sizeBytes: 3, + expiresAt: nowMs + 60_000, + }), + ); + const token = `${encodedPayload}.${signPayload(encodedPayload, secret)}`; + + expect(yield* validateWorkspaceUploadToken(token)).toBeNull(); + }).pipe(Effect.provide(testLayer)), + ); +}); diff --git a/apps/server/src/workspace/WorkspaceUpload.ts b/apps/server/src/workspace/WorkspaceUpload.ts new file mode 100644 index 000000000000..5f1a37bb6e6f --- /dev/null +++ b/apps/server/src/workspace/WorkspaceUpload.ts @@ -0,0 +1,230 @@ +// @effect-diagnostics nodeBuiltinImport:off +import * as NodeCrypto from "node:crypto"; + +import { + PROJECT_UPLOAD_URL_TTL_MS, + ProjectCreateUploadUrlError, + ProjectUploadTargetExistsError, + type ProjectCreateUploadUrlInput, +} from "@t3tools/contracts"; +import * as Clock from "effect/Clock"; +import * as Effect from "effect/Effect"; +import * as FileSystem from "effect/FileSystem"; +import * as Option from "effect/Option"; +import * as Path from "effect/Path"; +import * as Schema from "effect/Schema"; + +import { + base64UrlDecodeUtf8, + base64UrlEncode, + signPayload, + timingSafeEqualBase64Url, +} from "../auth/utils.ts"; +import * as ServerSecretStore from "../auth/ServerSecretStore.ts"; +import * as WorkspaceEntries from "./WorkspaceEntries.ts"; +import * as WorkspacePaths from "./WorkspacePaths.ts"; + +export const WORKSPACE_UPLOAD_ROUTE_PREFIX = "/api/workspace/upload"; + +// Asset download and attachment upload tokens share this key; the signed +// claim kind keeps the token spaces separate. +const SIGNING_SECRET_NAME = "asset-access-signing-key"; + +const WorkspaceUploadClaims = Schema.Struct({ + version: Schema.Literal(1), + kind: Schema.Literal("workspace-upload"), + cwd: Schema.String, + relativePath: Schema.String, + sizeBytes: Schema.Number, + overwrite: Schema.Boolean, + expiresAt: Schema.Number, +}); +export type WorkspaceUploadClaims = typeof WorkspaceUploadClaims.Type; + +const workspaceUploadClaimsJson = Schema.fromJsonString(WorkspaceUploadClaims); +const decodeWorkspaceUploadClaims = Schema.decodeUnknownOption(workspaceUploadClaimsJson); +const encodeWorkspaceUploadClaims = Schema.encodeSync(workspaceUploadClaimsJson); + +function decodeClaims(encodedPayload: string): WorkspaceUploadClaims | null { + try { + return Option.getOrNull(decodeWorkspaceUploadClaims(base64UrlDecodeUtf8(encodedPayload))); + } catch { + return null; + } +} + +const loadSigningSecret = Effect.gen(function* () { + const secretStore = yield* ServerSecretStore.ServerSecretStore; + return yield* secretStore.getOrCreateRandom(SIGNING_SECRET_NAME, 32); +}); + +export const issueWorkspaceUploadUrl = Effect.fn("WorkspaceUpload.issueUrl")(function* ( + input: ProjectCreateUploadUrlInput, +) { + const secret = yield* loadSigningSecret.pipe( + Effect.mapError( + (cause) => + new ProjectCreateUploadUrlError({ + cwd: input.cwd, + relativePath: input.relativePath, + message: "Failed to load the upload signing key.", + cause, + }), + ), + ); + + const workspacePaths = yield* WorkspacePaths.WorkspacePaths; + const target = yield* workspacePaths + .resolveRelativePathWithinRoot({ + workspaceRoot: input.cwd, + relativePath: input.relativePath, + }) + .pipe( + Effect.mapError( + (error) => + new ProjectCreateUploadUrlError({ + cwd: input.cwd, + relativePath: input.relativePath, + message: error.message, + cause: error, + }), + ), + ); + + const fileSystem = yield* FileSystem.FileSystem; + const targetExists = yield* fileSystem.exists(target.absolutePath).pipe( + Effect.mapError( + (cause) => + new ProjectCreateUploadUrlError({ + cwd: input.cwd, + relativePath: target.relativePath, + message: `Failed to check for an existing file at '${target.relativePath}' in '${input.cwd}'.`, + cause, + }), + ), + ); + if (targetExists && input.overwrite !== true) { + return yield* new ProjectUploadTargetExistsError({ + cwd: input.cwd, + relativePath: target.relativePath, + }); + } + + const nowMs = yield* Clock.currentTimeMillis; + const expiresAt = nowMs + PROJECT_UPLOAD_URL_TTL_MS; + const encodedPayload = base64UrlEncode( + encodeWorkspaceUploadClaims({ + version: 1, + kind: "workspace-upload", + cwd: input.cwd, + relativePath: target.relativePath, + sizeBytes: input.sizeBytes, + overwrite: input.overwrite === true, + expiresAt, + }), + ); + + return { + relativePath: target.relativePath, + relativeUrl: `${WORKSPACE_UPLOAD_ROUTE_PREFIX}/${encodedPayload}.${signPayload(encodedPayload, secret)}`, + expiresAt, + }; +}); + +export const validateWorkspaceUploadToken = Effect.fn("WorkspaceUpload.validateToken")(function* ( + token: string, +) { + const [encodedPayload, signature, unexpectedSegment] = token.split("."); + if (!encodedPayload || !signature || unexpectedSegment) { + return null; + } + + const secret = yield* loadSigningSecret.pipe( + Effect.tapError((cause) => + Effect.logError("Failed to load the workspace upload signing key.", { cause }), + ), + Effect.orElseSucceed(() => null), + ); + if (!secret || !timingSafeEqualBase64Url(signature, signPayload(encodedPayload, secret))) { + return null; + } + + const claims = decodeClaims(encodedPayload); + if (!claims || claims.expiresAt <= (yield* Clock.currentTimeMillis)) { + return null; + } + return claims; +}); + +export type StoreWorkspaceUploadResult = + | { readonly ok: true; readonly relativePath: string } + | { readonly ok: false; readonly status: number; readonly detail: string }; + +export const storeWorkspaceUpload = Effect.fn("WorkspaceUpload.store")(function* ( + claims: WorkspaceUploadClaims, + bytes: Uint8Array, +) { + if (bytes.byteLength !== claims.sizeBytes) { + return { + ok: false, + status: 400, + detail: `Body was ${bytes.byteLength} bytes, expected ${claims.sizeBytes}.`, + } satisfies StoreWorkspaceUploadResult; + } + + const workspacePaths = yield* WorkspacePaths.WorkspacePaths; + const target = yield* workspacePaths + .resolveRelativePathWithinRoot({ + workspaceRoot: claims.cwd, + relativePath: claims.relativePath, + }) + .pipe(Effect.catchTag("WorkspacePathOutsideRootError", () => Effect.succeed(null))); + if (!target) { + return { + ok: false, + status: 500, + detail: "Failed to resolve the workspace upload target.", + } satisfies StoreWorkspaceUploadResult; + } + + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const partPath = `${target.absolutePath}.${NodeCrypto.randomUUID()}.part`; + return yield* Effect.gen(function* () { + const targetExists = yield* fileSystem.exists(target.absolutePath); + if (targetExists && !claims.overwrite) { + return { + ok: false, + status: 409, + detail: "A file already exists at this path.", + } satisfies StoreWorkspaceUploadResult; + } + + yield* fileSystem.makeDirectory(path.dirname(target.absolutePath), { recursive: true }); + yield* fileSystem.writeFile(partPath, bytes); + yield* fileSystem.rename(partPath, target.absolutePath); + + const workspaceEntries = yield* WorkspaceEntries.WorkspaceEntries; + yield* workspaceEntries.refresh(claims.cwd); + + return { ok: true, relativePath: target.relativePath } satisfies StoreWorkspaceUploadResult; + }).pipe( + Effect.catch((cause) => + fileSystem.remove(partPath, { force: true }).pipe( + Effect.orElseSucceed(() => undefined), + Effect.andThen( + Effect.logError("Failed to persist workspace upload.", { + cwd: claims.cwd, + relativePath: claims.relativePath, + cause, + }), + ), + Effect.as({ + ok: false, + status: 500, + detail: "Failed to persist upload.", + } satisfies StoreWorkspaceUploadResult), + ), + ), + ); +}); From 774d836a86538df1655fad56014d97cf87d25a0a Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 07:22:45 +0800 Subject: [PATCH 03/54] docs(server): name workspace uploads among signing-key users --- apps/server/src/workspace/WorkspaceUpload.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/server/src/workspace/WorkspaceUpload.ts b/apps/server/src/workspace/WorkspaceUpload.ts index 5f1a37bb6e6f..8f89a93d4af5 100644 --- a/apps/server/src/workspace/WorkspaceUpload.ts +++ b/apps/server/src/workspace/WorkspaceUpload.ts @@ -26,8 +26,8 @@ import * as WorkspacePaths from "./WorkspacePaths.ts"; export const WORKSPACE_UPLOAD_ROUTE_PREFIX = "/api/workspace/upload"; -// Asset download and attachment upload tokens share this key; the signed -// claim kind keeps the token spaces separate. +// Asset download, attachment upload, and workspace upload tokens share this +// key; the signed claim kind keeps the token spaces separate. const SIGNING_SECRET_NAME = "asset-access-signing-key"; const WorkspaceUploadClaims = Schema.Struct({ From 04c5fe41c47be9767dc773750e1939a53ad2e44f Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 07:27:37 +0800 Subject: [PATCH 04/54] feat(server): serve workspace uploads over signed POST route --- apps/server/src/http.ts | 50 ++++++++++++++++++++++++ apps/server/src/server.test.ts | 70 ++++++++++++++++++++++++++++++++++ apps/server/src/server.ts | 2 + apps/server/src/ws.ts | 11 +++--- 4 files changed, 127 insertions(+), 6 deletions(-) diff --git a/apps/server/src/http.ts b/apps/server/src/http.ts index c3104e7bc420..73f71d80977a 100644 --- a/apps/server/src/http.ts +++ b/apps/server/src/http.ts @@ -33,6 +33,11 @@ import { storeAttachmentUpload, validateAttachmentUploadToken, } from "./assets/AttachmentUpload.ts"; +import { + WORKSPACE_UPLOAD_ROUTE_PREFIX, + storeWorkspaceUpload, + validateWorkspaceUploadToken, +} from "./workspace/WorkspaceUpload.ts"; import * as BrowserTraceCollector from "./observability/BrowserTraceCollector.ts"; import * as EnvironmentAuth from "./auth/EnvironmentAuth.ts"; import { traceRelayRequest } from "./cloud/traceRelayRequest.ts"; @@ -280,6 +285,51 @@ export const attachmentUploadRouteLayer = HttpRouter.add( }), ); +export const workspaceUploadRouteLayer = HttpRouter.add( + "POST", + `${WORKSPACE_UPLOAD_ROUTE_PREFIX}/*`, + Effect.gen(function* () { + const request = yield* HttpServerRequest.HttpServerRequest; + const url = HttpServerRequest.toURL(request); + if (Option.isNone(url)) { + return HttpServerResponse.text("Bad Request", { status: 400 }); + } + + const token = url.value.pathname.slice(`${WORKSPACE_UPLOAD_ROUTE_PREFIX}/`.length); + if (!token) { + return HttpServerResponse.text("Not Found", { status: 404 }); + } + const claims = yield* validateWorkspaceUploadToken(token); + if (!claims) { + return HttpServerResponse.text("Not Found", { status: 404 }); + } + + const contentLengthHeader = request.headers["content-length"]; + if ( + contentLengthHeader !== undefined && + (!Number.isInteger(Number(contentLengthHeader)) || + Number(contentLengthHeader) !== claims.sizeBytes) + ) { + return HttpServerResponse.text("Content-Length must match the upload size.", { + status: 400, + }); + } + + const body = yield* request.arrayBuffer.pipe( + Effect.provideService(HttpServerRequest.MaxBodySize, FileSystem.Size(claims.sizeBytes)), + Effect.orElseSucceed(() => null), + ); + if (body === null) { + return HttpServerResponse.text("Failed to read the upload body.", { status: 400 }); + } + + const stored = yield* storeWorkspaceUpload(claims, new Uint8Array(body)); + return stored.ok + ? HttpServerResponse.empty({ status: 204 }) + : HttpServerResponse.text(stored.detail, { status: stored.status }); + }), +); + export const staticAndDevRouteLayer = HttpRouter.add( "GET", "*", diff --git a/apps/server/src/server.test.ts b/apps/server/src/server.test.ts index 5e4f19172eff..92e046d2e9ca 100644 --- a/apps/server/src/server.test.ts +++ b/apps/server/src/server.test.ts @@ -133,6 +133,7 @@ import * as ServerEnvironment from "./environment/ServerEnvironment.ts"; import * as WorkspaceEntries from "./workspace/WorkspaceEntries.ts"; import * as WorkspaceFileSystem from "./workspace/WorkspaceFileSystem.ts"; import * as WorkspacePaths from "./workspace/WorkspacePaths.ts"; +import { WORKSPACE_UPLOAD_ROUTE_PREFIX } from "./workspace/WorkspaceUpload.ts"; import * as GitVcsDriver from "./vcs/GitVcsDriver.ts"; import * as VcsDriver from "./vcs/VcsDriver.ts"; import * as VcsStatusBroadcaster from "./vcs/VcsStatusBroadcaster.ts"; @@ -5127,6 +5128,75 @@ it.layer(NodeServices.layer)("server router seam", (it) => { }).pipe(Effect.provide(NodeHttpServer.layerTest)), ); + it.effect("uploads workspace file bytes through a signed URL issued by websocket rpc", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const workspaceDir = yield* fs.makeTempDirectoryScoped({ + prefix: "t3-ws-project-upload-", + }); + + yield* buildAppUnderTest(); + + const wsUrl = yield* getWsServerUrl("/ws"); + yield* Effect.scoped( + withWsRpcClient(wsUrl, (client) => + Effect.gen(function* () { + const issued = yield* client[WS_METHODS.projectsCreateUploadUrl]({ + cwd: workspaceDir, + relativePath: "uploaded/dropped.bin", + sizeBytes: 6, + overwrite: false, + }); + assert.equal(issued.relativePath, "uploaded/dropped.bin"); + + const badContentLength = yield* HttpClient.post(issued.relativeUrl, { + body: HttpBody.uint8Array(new Uint8Array([1, 2, 3]), "application/octet-stream"), + }); + assert.equal(badContentLength.status, 400); + + const response = yield* HttpClient.post(issued.relativeUrl, { + body: HttpBody.uint8Array( + new Uint8Array([1, 2, 3, 4, 5, 6]), + "application/octet-stream", + ), + }); + assert.equal(response.status, 204); + + const persisted = yield* fs.readFile( + path.join(workspaceDir, "uploaded", "dropped.bin"), + ); + assert.deepEqual(Array.from(persisted), [1, 2, 3, 4, 5, 6]); + + const notFoundResponse = yield* HttpClient.post( + `${WORKSPACE_UPLOAD_ROUTE_PREFIX}/not-a-real-token`, + { body: HttpBody.uint8Array(new Uint8Array([1, 2, 3, 4, 5, 6])) }, + ); + assert.equal(notFoundResponse.status, 404); + + const conflictTarget = yield* client[WS_METHODS.projectsCreateUploadUrl]({ + cwd: workspaceDir, + relativePath: "uploaded/conflict.bin", + sizeBytes: 6, + overwrite: false, + }); + yield* fs.writeFile( + path.join(workspaceDir, "uploaded", "conflict.bin"), + new Uint8Array([9, 9, 9, 9, 9, 9]), + ); + const conflictResponse = yield* HttpClient.post(conflictTarget.relativeUrl, { + body: HttpBody.uint8Array( + new Uint8Array([1, 2, 3, 4, 5, 6]), + "application/octet-stream", + ), + }); + assert.equal(conflictResponse.status, 409); + }), + ), + ); + }).pipe(Effect.provide(NodeHttpServer.layerTest)), + ); + it.effect("creates a missing workspace root during websocket project.create dispatch", () => Effect.gen(function* () { const fs = yield* FileSystem.FileSystem; diff --git a/apps/server/src/server.ts b/apps/server/src/server.ts index 0a31bf376dae..bf46f74c0908 100644 --- a/apps/server/src/server.ts +++ b/apps/server/src/server.ts @@ -18,6 +18,7 @@ import { staticAndDevRouteLayer, browserApiCorsLayer, httpCompressionLayer, + workspaceUploadRouteLayer, } from "./http.ts"; import { guardHttpResponseWriteErrors } from "./httpResponseErrorGuard.ts"; import { fixPath } from "./os-jank.ts"; @@ -458,6 +459,7 @@ export const makeRoutesLayer = Layer.mergeAll( otlpTracesProxyRouteLayer, assetRouteLayer, attachmentUploadRouteLayer, + workspaceUploadRouteLayer, staticAndDevRouteLayer, websocketRpcRouteLayer, ), diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index ff4d012bd6fd..ee5975f30ffc 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -34,7 +34,6 @@ import { OrchestrationGetTurnDiffError, ORCHESTRATION_WS_METHODS, type ProjectId, - ProjectCreateUploadUrlError, type ProjectEntriesFailure, type ProjectFileFailure, type ProjectFileOperation, @@ -97,6 +96,7 @@ import * as PreviewAutomationBroker from "./mcp/PreviewAutomationBroker.ts"; import * as PreviewManager from "./preview/Manager.ts"; import { issueAssetUrl } from "./assets/AssetAccess.ts"; import { deletePendingAttachment, issueAttachmentUploadUrl } from "./assets/AttachmentUpload.ts"; +import { issueWorkspaceUploadUrl } from "./workspace/WorkspaceUpload.ts"; import * as PortScanner from "./preview/PortScanner.ts"; import * as WorkspaceEntries from "./workspace/WorkspaceEntries.ts"; import * as WorkspaceFileSystem from "./workspace/WorkspaceFileSystem.ts"; @@ -1939,11 +1939,10 @@ const makeWsRpcLayer = ( ), { "rpc.aggregate": "workspace" }, ), - // Task 3 replaces this with the real handler. - [WS_METHODS.projectsCreateUploadUrl]: () => - Effect.fail( - new ProjectCreateUploadUrlError({ message: "Workspace uploads are not wired up yet." }), - ), + [WS_METHODS.projectsCreateUploadUrl]: (input) => + observeRpcEffect(WS_METHODS.projectsCreateUploadUrl, issueWorkspaceUploadUrl(input), { + "rpc.aggregate": "workspace", + }), [WS_METHODS.shellOpenInEditor]: (input) => observeRpcEffect(WS_METHODS.shellOpenInEditor, externalLauncher.launchEditor(input), { "rpc.aggregate": "workspace", From a0fd65d8bb3ff86778b9768c4db1e8a643251e8b Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 07:35:21 +0800 Subject: [PATCH 05/54] feat(web): queue workspace file uploads Adds the createUploadUrl command atom and a client-side upload queue for workspace files: FIFO pump capped at 3 concurrent uploads per environment, XHR-based byte upload with progress, an overwrite confirm flow for ProjectUploadTargetExistsError, and retry/cancel/dismiss for failed rows. --- apps/web/src/lib/workspaceUploadQueue.test.ts | 363 ++++++++++++++++++ apps/web/src/lib/workspaceUploadQueue.ts | 333 ++++++++++++++++ .../src/state/projectCommands.ts | 4 + 3 files changed, 700 insertions(+) create mode 100644 apps/web/src/lib/workspaceUploadQueue.test.ts create mode 100644 apps/web/src/lib/workspaceUploadQueue.ts diff --git a/apps/web/src/lib/workspaceUploadQueue.test.ts b/apps/web/src/lib/workspaceUploadQueue.test.ts new file mode 100644 index 000000000000..907364273107 --- /dev/null +++ b/apps/web/src/lib/workspaceUploadQueue.test.ts @@ -0,0 +1,363 @@ +import { EnvironmentId } from "@t3tools/contracts"; +import * as Cause from "effect/Cause"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; + +const mocks = vi.hoisted(() => ({ + createUploadUrl: Symbol("create-upload-url"), + runAtomCommand: vi.fn(), + readPreparedConnection: vi.fn(), + requestConfirmDialog: vi.fn(), +})); + +vi.mock("@t3tools/client-runtime/state/runtime", () => ({ + runAtomCommand: mocks.runAtomCommand, +})); + +vi.mock("../rpc/atomRegistry", () => ({ appAtomRegistry: {} })); + +vi.mock("../state/projects", () => ({ + projectEnvironment: { + createUploadUrl: mocks.createUploadUrl, + }, +})); + +vi.mock("../state/session", () => ({ + readPreparedConnection: mocks.readPreparedConnection, +})); + +vi.mock("../confirmDialog", () => ({ + requestConfirmDialog: mocks.requestConfirmDialog, +})); + +import { + cancelWorkspaceUpload, + dismissWorkspaceUpload, + retryWorkspaceUpload, + startWorkspaceUploads, + useWorkspaceUploadStore, +} from "./workspaceUploadQueue"; + +type ProgressListener = (event: { + readonly lengthComputable: boolean; + readonly loaded: number; + readonly total: number; +}) => void; + +class TestXmlHttpRequest { + static requests: TestXmlHttpRequest[] = []; + + status = 0; + timeout = 0; + method: string | null = null; + url: string | null = null; + readonly headers = new Map(); + readonly listeners = new Map void>(); + progressListener: ProgressListener | null = null; + + readonly upload = { + addEventListener: (_event: string, listener: ProgressListener) => { + this.progressListener = listener; + }, + }; + + constructor() { + TestXmlHttpRequest.requests.push(this); + } + + open(method: string, url: string): void { + this.method = method; + this.url = url; + } + + setRequestHeader(name: string, value: string): void { + this.headers.set(name, value); + } + + addEventListener(event: string, listener: () => void): void { + this.listeners.set(event, listener); + } + + send(): void {} + + abort(): void { + this.listeners.get("abort")?.(); + } + + progress(loaded: number, total: number): void { + this.progressListener?.({ lengthComputable: true, loaded, total }); + } + + complete(status = 204): void { + this.status = status; + this.listeners.get("load")?.(); + } +} + +const environmentId = EnvironmentId.make("environment-1"); +const cwd = "/workspace/project"; + +function makeFile(name: string): File { + return new File([new Uint8Array([1, 2, 3])], name); +} + +function mintedResult(relativePath: string) { + return { + _tag: "Success" as const, + value: { + relativePath, + relativeUrl: `/api/workspace/upload/token-${relativePath}`, + expiresAt: 1, + }, + }; +} + +function targetExistsFailure(relativePath: string) { + return { + _tag: "Failure" as const, + cause: Cause.fail({ + _tag: "ProjectUploadTargetExistsError", + cwd, + relativePath, + }), + }; +} + +function genericMintFailure() { + return { + _tag: "Failure" as const, + cause: Cause.fail({ + _tag: "ProjectCreateUploadUrlError", + message: "boom", + }), + }; +} + +function uploadsById() { + return useWorkspaceUploadStore.getState().uploadsById; +} + +function findUpload(name: string) { + return Object.entries(uploadsById()).find(([, upload]) => upload.name === name); +} + +describe("workspaceUploadQueue", () => { + beforeEach(() => { + TestXmlHttpRequest.requests = []; + mocks.runAtomCommand.mockReset(); + mocks.readPreparedConnection.mockReset(); + mocks.requestConfirmDialog.mockReset(); + mocks.readPreparedConnection.mockReturnValue({ httpBaseUrl: "https://environment.test/" }); + mocks.runAtomCommand.mockImplementation( + async ( + _registry: unknown, + command: unknown, + target: { readonly input: { readonly relativePath: string } }, + ) => { + if (command === mocks.createUploadUrl) { + return mintedResult(target.input.relativePath); + } + throw new Error("unexpected command"); + }, + ); + vi.stubGlobal("XMLHttpRequest", TestXmlHttpRequest); + }); + + afterEach(() => { + useWorkspaceUploadStore.setState({ uploadsById: {} }); + vi.unstubAllGlobals(); + }); + + it("uploads a file, reports progress, then removes the entry and calls onUploaded", async () => { + const onUploaded = vi.fn(); + const file = makeFile("notes.txt"); + startWorkspaceUploads({ environmentId, cwd, files: [file], onUploaded }); + await Promise.resolve(); + await Promise.resolve(); + + const request = TestXmlHttpRequest.requests[0]!; + expect(request.method).toBe("POST"); + expect(request.url).toBe("https://environment.test/api/workspace/upload/token-notes.txt"); + expect(request.headers.has("Content-Type")).toBe(false); + + request.progress(1, 2); + const [uploadId, uploading] = findUpload("notes.txt")!; + expect(uploading).toMatchObject({ + status: "uploading", + relativePath: "notes.txt", + environmentId, + cwd, + progress: 0.5, + }); + + request.complete(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + + expect(uploadsById()[uploadId]).toBeUndefined(); + expect(onUploaded).toHaveBeenCalledTimes(1); + }); + + it("marks the entry failed with a reason when minting fails", async () => { + mocks.runAtomCommand.mockResolvedValue(genericMintFailure()); + const file = makeFile("broken.txt"); + startWorkspaceUploads({ environmentId, cwd, files: [file], onUploaded: vi.fn() }); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + + const [, failed] = findUpload("broken.txt")!; + expect(failed).toMatchObject({ status: "failed", reason: "Upload could not start" }); + expect(TestXmlHttpRequest.requests).toHaveLength(0); + }); + + it("re-mints with overwrite and uploads when the user confirms replacing an existing file", async () => { + let call = 0; + mocks.runAtomCommand.mockImplementation( + async ( + _registry: unknown, + command: unknown, + target: { + readonly input: { readonly relativePath: string; readonly overwrite?: boolean }; + }, + ) => { + if (command !== mocks.createUploadUrl) throw new Error("unexpected command"); + call += 1; + if (call === 1) { + expect(target.input.overwrite).toBeUndefined(); + return targetExistsFailure(target.input.relativePath); + } + expect(target.input.overwrite).toBe(true); + return mintedResult(target.input.relativePath); + }, + ); + mocks.requestConfirmDialog.mockResolvedValue(true); + + const file = makeFile("existing.txt"); + startWorkspaceUploads({ environmentId, cwd, files: [file], onUploaded: vi.fn() }); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + + expect(mocks.requestConfirmDialog).toHaveBeenCalledWith( + "Replace existing.txt?\nA file named 'existing.txt' already exists in this project.", + ); + expect(TestXmlHttpRequest.requests).toHaveLength(1); + TestXmlHttpRequest.requests[0]!.complete(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + + expect(findUpload("existing.txt")).toBeUndefined(); + }); + + it("marks the entry failed with 'File already exists' when the user declines to replace", async () => { + mocks.runAtomCommand.mockImplementation( + async ( + _registry: unknown, + command: unknown, + target: { readonly input: { readonly relativePath: string } }, + ) => { + if (command !== mocks.createUploadUrl) throw new Error("unexpected command"); + return targetExistsFailure(target.input.relativePath); + }, + ); + mocks.requestConfirmDialog.mockResolvedValue(false); + + const file = makeFile("existing.txt"); + startWorkspaceUploads({ environmentId, cwd, files: [file], onUploaded: vi.fn() }); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + + const [, failed] = findUpload("existing.txt")!; + expect(failed).toMatchObject({ status: "failed", reason: "File already exists" }); + expect(TestXmlHttpRequest.requests).toHaveLength(0); + }); + + it("cancelWorkspaceUpload aborts the in-flight XHR and removes the entry", async () => { + const file = makeFile("cancel-me.txt"); + startWorkspaceUploads({ environmentId, cwd, files: [file], onUploaded: vi.fn() }); + await Promise.resolve(); + await Promise.resolve(); + + const request = TestXmlHttpRequest.requests[0]!; + const [uploadId] = findUpload("cancel-me.txt")!; + cancelWorkspaceUpload(uploadId); + + expect(uploadsById()[uploadId]).toBeUndefined(); + await Promise.resolve(); + await Promise.resolve(); + expect(request.listeners.has("abort")).toBe(true); + }); + + it("caps concurrent uploads at 3 per environment", async () => { + const files = ["a.txt", "b.txt", "c.txt", "d.txt"].map(makeFile); + startWorkspaceUploads({ environmentId, cwd, files, onUploaded: vi.fn() }); + await Promise.resolve(); + await Promise.resolve(); + + expect(TestXmlHttpRequest.requests).toHaveLength(3); + + for (const request of TestXmlHttpRequest.requests) { + request.complete(); + } + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + + expect(TestXmlHttpRequest.requests).toHaveLength(4); + }); + + it("retryWorkspaceUpload restarts a failed entry", async () => { + mocks.runAtomCommand.mockResolvedValueOnce(genericMintFailure()); + const file = makeFile("retry.txt"); + startWorkspaceUploads({ environmentId, cwd, files: [file], onUploaded: vi.fn() }); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + + const [uploadId, failed] = findUpload("retry.txt")!; + expect(failed).toMatchObject({ status: "failed" }); + + mocks.runAtomCommand.mockImplementation( + async ( + _registry: unknown, + command: unknown, + target: { readonly input: { readonly relativePath: string } }, + ) => { + if (command !== mocks.createUploadUrl) throw new Error("unexpected command"); + return mintedResult(target.input.relativePath); + }, + ); + retryWorkspaceUpload(uploadId); + await Promise.resolve(); + await Promise.resolve(); + + expect(TestXmlHttpRequest.requests).toHaveLength(1); + expect(uploadsById()[uploadId]).toMatchObject({ status: "uploading" }); + + TestXmlHttpRequest.requests[0]!.complete(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + + expect(uploadsById()[uploadId]).toBeUndefined(); + }); + + it("dismissWorkspaceUpload removes a failed entry", async () => { + mocks.runAtomCommand.mockResolvedValue(genericMintFailure()); + const file = makeFile("dismiss-me.txt"); + startWorkspaceUploads({ environmentId, cwd, files: [file], onUploaded: vi.fn() }); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + + const [uploadId] = findUpload("dismiss-me.txt")!; + dismissWorkspaceUpload(uploadId); + + expect(uploadsById()[uploadId]).toBeUndefined(); + }); +}); diff --git a/apps/web/src/lib/workspaceUploadQueue.ts b/apps/web/src/lib/workspaceUploadQueue.ts new file mode 100644 index 000000000000..5b266496e7ab --- /dev/null +++ b/apps/web/src/lib/workspaceUploadQueue.ts @@ -0,0 +1,333 @@ +import type { EnvironmentId } from "@t3tools/contracts"; +import { resolveAssetUrl } from "@t3tools/client-runtime/state/assets"; +import { runAtomCommand } from "@t3tools/client-runtime/state/runtime"; +import * as Cause from "effect/Cause"; +import { create } from "zustand"; + +import { requestConfirmDialog } from "../confirmDialog"; +import { appAtomRegistry } from "../rpc/atomRegistry"; +import { projectEnvironment } from "../state/projects"; +import { readPreparedConnection } from "../state/session"; +import { randomUUID } from "./utils"; + +const MAX_UPLOADS_PER_ENVIRONMENT = 3; +const UPLOAD_TIMEOUT_MS = 5 * 60_000; + +export type WorkspaceUploadState = + | { + readonly status: "uploading"; + readonly name: string; + readonly relativePath: string; + readonly environmentId: EnvironmentId; + readonly cwd: string; + readonly progress: number; + } + | { + readonly status: "failed"; + readonly name: string; + readonly relativePath: string; + readonly environmentId: EnvironmentId; + readonly cwd: string; + readonly reason: string; + }; + +interface WorkspaceUploadStore { + readonly uploadsById: Readonly>; +} + +export const useWorkspaceUploadStore = create(() => ({ + uploadsById: {}, +})); + +interface UploadJob { + readonly id: string; + readonly environmentId: EnvironmentId; + readonly cwd: string; + readonly relativePath: string; + readonly file: File; + readonly onUploaded: () => void; + overwrite: boolean; + cancelled: boolean; + abort: (() => void) | null; +} + +// Jobs stay here from queueing through a terminal state (failed) so retry can +// reuse the original File. Success and cancellation remove the entry. +const jobsById = new Map(); +const queue: UploadJob[] = []; +const activeUploadsByEnvironment = new Map(); + +function setUploadState(uploadId: string, upload: WorkspaceUploadState): void { + useWorkspaceUploadStore.setState((state) => ({ + uploadsById: { ...state.uploadsById, [uploadId]: upload }, + })); +} + +function clearUploadState(uploadId: string): void { + useWorkspaceUploadStore.setState((state) => { + if (!(uploadId in state.uploadsById)) { + return state; + } + const uploadsById = { ...state.uploadsById }; + delete uploadsById[uploadId]; + return { uploadsById }; + }); +} + +function failJob(job: UploadJob, reason: string): void { + setUploadState(job.id, { + status: "failed", + name: job.file.name, + relativePath: job.relativePath, + environmentId: job.environmentId, + cwd: job.cwd, + reason, + }); +} + +function uploadBytes(input: { + readonly url: string; + readonly file: File; + readonly onProgress: (progress: number) => void; +}): { readonly done: Promise; readonly abort: () => void } { + const xhr = new XMLHttpRequest(); + const done = new Promise((resolve, reject) => { + xhr.open("POST", input.url, true); + xhr.timeout = UPLOAD_TIMEOUT_MS; + xhr.upload.addEventListener("progress", (event) => { + if (event.lengthComputable && event.total > 0) { + input.onProgress(event.loaded / event.total); + } + }); + xhr.addEventListener("load", () => { + if (xhr.status >= 200 && xhr.status < 300) { + resolve(); + } else { + reject(new Error(`Upload rejected (${xhr.status})`)); + } + }); + xhr.addEventListener("error", () => reject(new Error("Upload failed"))); + xhr.addEventListener("timeout", () => reject(new Error("Upload timed out"))); + xhr.addEventListener("abort", () => reject(new Error("Upload cancelled"))); + xhr.send(input.file); + }); + + return { done, abort: () => xhr.abort() }; +} + +function mintUploadUrl(job: UploadJob) { + return runAtomCommand( + appAtomRegistry, + projectEnvironment.createUploadUrl, + { + environmentId: job.environmentId, + input: { + cwd: job.cwd, + relativePath: job.relativePath, + sizeBytes: job.file.size, + ...(job.overwrite ? { overwrite: true } : {}), + }, + }, + { reportFailure: false }, + ); +} + +function isTargetExistsFailure(cause: unknown): boolean { + return ( + typeof cause === "object" && + cause !== null && + "_tag" in cause && + cause._tag === "ProjectUploadTargetExistsError" + ); +} + +async function runUpload(job: UploadJob): Promise { + let minted = await mintUploadUrl(job); + if (job.cancelled) { + jobsById.delete(job.id); + return; + } + + if (minted._tag !== "Success") { + if (!isTargetExistsFailure(Cause.squash(minted.cause))) { + failJob(job, "Upload could not start"); + return; + } + + const confirmed = await requestConfirmDialog( + `Replace ${job.file.name}?\nA file named '${job.relativePath}' already exists in this project.`, + ); + if (job.cancelled) { + jobsById.delete(job.id); + return; + } + if (confirmed !== true) { + failJob(job, "File already exists"); + return; + } + + job.overwrite = true; + minted = await mintUploadUrl(job); + if (job.cancelled) { + jobsById.delete(job.id); + return; + } + if (minted._tag !== "Success") { + failJob(job, "Upload could not start"); + return; + } + } + + const connection = readPreparedConnection(job.environmentId); + const url = connection ? resolveAssetUrl(connection.httpBaseUrl, minted.value.relativeUrl) : null; + if (!url) { + failJob(job, "Not connected"); + return; + } + + let lastStep = -1; + const upload = uploadBytes({ + url, + file: job.file, + onProgress: (progress) => { + const step = Math.floor(progress * 20); + if (step === lastStep || job.cancelled) { + return; + } + lastStep = step; + setUploadState(job.id, { + status: "uploading", + name: job.file.name, + relativePath: job.relativePath, + environmentId: job.environmentId, + cwd: job.cwd, + progress, + }); + }, + }); + job.abort = upload.abort; + + try { + await upload.done; + if (job.cancelled) { + jobsById.delete(job.id); + return; + } + jobsById.delete(job.id); + clearUploadState(job.id); + job.onUploaded(); + } catch (error) { + if (job.cancelled) { + jobsById.delete(job.id); + return; + } + failJob(job, error instanceof Error ? error.message : "Upload failed"); + } finally { + job.abort = null; + } +} + +function pumpUploads(): void { + for (let index = 0; index < queue.length; ) { + const job = queue[index]!; + const active = activeUploadsByEnvironment.get(job.environmentId) ?? 0; + if (active >= MAX_UPLOADS_PER_ENVIRONMENT) { + index += 1; + continue; + } + + queue.splice(index, 1); + if (job.cancelled) { + continue; + } + activeUploadsByEnvironment.set(job.environmentId, active + 1); + void runUpload(job) + .catch(() => { + if (!job.cancelled) { + failJob(job, "Upload failed"); + } + }) + .finally(() => { + const remaining = (activeUploadsByEnvironment.get(job.environmentId) ?? 1) - 1; + if (remaining > 0) { + activeUploadsByEnvironment.set(job.environmentId, remaining); + } else { + activeUploadsByEnvironment.delete(job.environmentId); + } + pumpUploads(); + }); + } +} + +export function startWorkspaceUploads(input: { + readonly environmentId: EnvironmentId; + readonly cwd: string; + readonly files: ReadonlyArray; + readonly onUploaded: () => void; +}): void { + for (const file of input.files) { + const id = randomUUID(); + // Uploads land at the project root in v1. + const relativePath = file.name; + const job: UploadJob = { + id, + environmentId: input.environmentId, + cwd: input.cwd, + relativePath, + file, + onUploaded: input.onUploaded, + overwrite: false, + cancelled: false, + abort: null, + }; + jobsById.set(id, job); + queue.push(job); + setUploadState(id, { + status: "uploading", + name: file.name, + relativePath, + environmentId: input.environmentId, + cwd: input.cwd, + progress: 0, + }); + } + pumpUploads(); +} + +export function cancelWorkspaceUpload(uploadId: string): void { + const job = jobsById.get(uploadId); + if (!job) { + return; + } + job.cancelled = true; + jobsById.delete(uploadId); + const queuedIndex = queue.indexOf(job); + if (queuedIndex !== -1) { + queue.splice(queuedIndex, 1); + } + job.abort?.(); + clearUploadState(uploadId); +} + +export function retryWorkspaceUpload(uploadId: string): void { + const job = jobsById.get(uploadId); + if (!job) { + return; + } + job.cancelled = false; + setUploadState(job.id, { + status: "uploading", + name: job.file.name, + relativePath: job.relativePath, + environmentId: job.environmentId, + cwd: job.cwd, + progress: 0, + }); + queue.push(job); + pumpUploads(); +} + +export function dismissWorkspaceUpload(uploadId: string): void { + jobsById.delete(uploadId); + clearUploadState(uploadId); +} diff --git a/packages/client-runtime/src/state/projectCommands.ts b/packages/client-runtime/src/state/projectCommands.ts index 3defcc321547..ae5e7adf314a 100644 --- a/packages/client-runtime/src/state/projectCommands.ts +++ b/packages/client-runtime/src/state/projectCommands.ts @@ -102,5 +102,9 @@ export function createProjectEnvironmentAtoms( JSON.stringify([environmentId, input.cwd, input.relativePath]), }, }), + createUploadUrl: createEnvironmentRpcCommand(runtime, { + label: "environment-data:projects:create-upload-url", + tag: WS_METHODS.projectsCreateUploadUrl, + }), }; } From 493bdf4bdfae63ec8961e4002bc82ece5aee3b2d Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 07:46:20 +0800 Subject: [PATCH 06/54] feat(web): upload files from the files view --- .../src/components/files/FileBrowserPanel.tsx | 196 +++++++++++++++++- docs/user/files.md | 27 +++ 2 files changed, 220 insertions(+), 3 deletions(-) create mode 100644 docs/user/files.md diff --git a/apps/web/src/components/files/FileBrowserPanel.tsx b/apps/web/src/components/files/FileBrowserPanel.tsx index cbe20f4d3a8d..2dbb45752a0a 100644 --- a/apps/web/src/components/files/FileBrowserPanel.tsx +++ b/apps/web/src/components/files/FileBrowserPanel.tsx @@ -5,8 +5,9 @@ import type { import type { EnvironmentId, ProjectEntry } from "@t3tools/contracts"; import { FileTree, useFileTree, useFileTreeSearch } from "@pierre/trees/react"; import { serializeComposerFileLink } from "@t3tools/shared/composerTrigger"; -import { RotateCw } from "lucide-react"; -import { useEffect, useMemo, useRef } from "react"; +import { RotateCw, Upload, XIcon } from "lucide-react"; +import type { DragEvent as ReactDragEvent, ReactNode } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { Button } from "~/components/ui/button"; import { InputGroup, InputGroupInput } from "~/components/ui/input-group"; @@ -15,10 +16,19 @@ import { Tooltip, TooltipPopup, TooltipTrigger } from "~/components/ui/tooltip"; import { useComposerHandleContext } from "~/composerHandleContext"; import { writeTextToClipboard } from "~/hooks/useCopyToClipboard"; import { useTheme } from "~/hooks/useTheme"; +import { + cancelWorkspaceUpload, + dismissWorkspaceUpload, + retryWorkspaceUpload, + startWorkspaceUploads, + useWorkspaceUploadStore, + type WorkspaceUploadState, +} from "~/lib/workspaceUploadQueue"; import { cn } from "~/lib/utils"; import { readLocalApi } from "~/localApi"; import { T3_PIERRE_ICONS } from "~/pierre-icons"; +import { makeWorkspaceFileDropHandlers } from "../chat/workspaceFileDrop"; import { createFileTreeDragMentionController } from "./fileTreeDragMention"; import { useProjectEntriesQuery } from "./projectFilesQueryState"; @@ -71,6 +81,97 @@ function RefreshFilesButton(props: { isPending: boolean; onRefresh: () => void } ); } +function UploadFilesButton(props: { onClick: () => void }) { + return ( + + + } + > + + + Upload files + + ); +} + +function UploadRowButton(props: { label: string; icon: ReactNode; onClick: () => void }) { + return ( + + + } + > + {props.icon} + + {props.label} + + ); +} + +function UploadRow(props: { id: string; upload: WorkspaceUploadState }) { + const { id, upload } = props; + return ( +
+ + {upload.name}} + /> + {upload.name} + + {upload.status === "uploading" ? ( + <> + + {Math.round(upload.progress * 100)}% + + } + onClick={() => cancelWorkspaceUpload(id)} + /> + + ) : ( + <> + + + {upload.reason} + + } + /> + {upload.reason} + + } + onClick={() => retryWorkspaceUpload(id)} + /> + } + onClick={() => dismissWorkspaceUpload(id)} + /> + + )} +
+ ); +} + function FileSearchField(props: { ariaLabel: string; name: string; @@ -111,6 +212,56 @@ export default function FileBrowserPanel({ const { resolvedTheme } = useTheme(); const composerRef = useComposerHandleContext(); const entriesQuery = useProjectEntriesQuery(environmentId, cwd); + const [dragActive, setDragActive] = useState(false); + const fileInputRef = useRef(null); + const handleAddFiles = useCallback( + (files: File[]) => { + if (files.length === 0) return; + startWorkspaceUploads({ + environmentId, + cwd, + files, + onUploaded: () => entriesQuery.refresh(), + }); + }, + [cwd, entriesQuery, environmentId], + ); + // The shared drop handlers manage drag-active state, but their onDrop reads + // event.dataTransfer.files directly, which includes an unreadable stand-in + // File for a dropped directory. Filter with dataTransfer.items instead so + // directories never reach the upload queue. + const fileDropHandlers = useMemo( + () => makeWorkspaceFileDropHandlers({ setDragActive, addFiles: handleAddFiles }), + [handleAddFiles], + ); + const handleDrop = useCallback( + (event: ReactDragEvent) => { + if (!event.dataTransfer.types.includes("Files")) return; + event.preventDefault(); + setDragActive(false); + const items = Array.from(event.dataTransfer.items); + const supportsEntries = items.length > 0 && typeof items[0]?.webkitGetAsEntry === "function"; + const files = supportsEntries + ? items.flatMap((item) => { + if (item.kind !== "file") return []; + const entry = item.webkitGetAsEntry(); + if (entry !== null && !entry.isFile) return []; + const file = item.getAsFile(); + return file ? [file] : []; + }) + : Array.from(event.dataTransfer.files); + handleAddFiles(files); + }, + [handleAddFiles], + ); + const uploadsById = useWorkspaceUploadStore((state) => state.uploadsById); + const uploads = useMemo( + () => + Object.entries(uploadsById).filter( + ([, upload]) => upload.environmentId === environmentId && upload.cwd === cwd, + ), + [cwd, environmentId, uploadsById], + ); const entries = entriesQuery.data?.entries ?? []; const entryKinds = useMemo( () => new Map(entries.map((entry) => [entry.path, entry.kind] as const)), @@ -353,14 +504,33 @@ export default function FileBrowserPanel({ return (
+ {dragActive ? ( +
+
+
+
+ ) : null}
+ fileInputRef.current?.click()} /> + { + handleAddFiles(Array.from(event.target.files ?? [])); + event.target.value = ""; + }} + />
{entriesQuery.error && entriesQuery.data === null ? (
{entriesQuery.error}
@@ -382,6 +562,16 @@ export default function FileBrowserPanel({ }} /> )} + {uploads.length > 0 ? ( +
+ {uploads.map(([id, upload]) => ( + + ))} +
+ ) : null}
); } diff --git a/docs/user/files.md b/docs/user/files.md new file mode 100644 index 000000000000..bdecc6991d7a --- /dev/null +++ b/docs/user/files.md @@ -0,0 +1,27 @@ +# Files view + +The files view shows your project's folder tree: directories and files in one list you can +expand, collapse, and select. Click a file to open it in the preview pane. + +## Opening and searching + +Type in the search field to filter the tree to matching names. Press Escape to clear the search +and show the full tree again. + +Right-click a file for **Copy mention** and **Add to chat**, so you can reference it in your +message without leaving the tree. + +## Uploading files + +Drag files from your computer onto the files view, or select the upload button at the top of the +panel to choose files from a picker. Uploads land in the project's root folder. Dragging a folder +does not upload its contents; drop the files themselves. + +If a file with that name already exists, T3 Code asks before replacing it. Decline and the upload +is cancelled; the existing file is left as it was. + +While a file uploads, its row shows progress and a cancel button. A failed upload shows the reason +and gives you retry and dismiss buttons. + +Uploading works the same way whether you're connected locally, over a remote network, or through a +tunnel. From 4d68e1d7af67c189664606cfbc2c9f455780dd96 Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 07:49:24 +0800 Subject: [PATCH 07/54] perf(web): scope files-panel renders to its own uploads --- .../src/components/files/FileBrowserPanel.tsx | 27 ++++++++++++++----- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/apps/web/src/components/files/FileBrowserPanel.tsx b/apps/web/src/components/files/FileBrowserPanel.tsx index 2dbb45752a0a..bc546ff8e886 100644 --- a/apps/web/src/components/files/FileBrowserPanel.tsx +++ b/apps/web/src/components/files/FileBrowserPanel.tsx @@ -8,6 +8,7 @@ import { serializeComposerFileLink } from "@t3tools/shared/composerTrigger"; import { RotateCw, Upload, XIcon } from "lucide-react"; import type { DragEvent as ReactDragEvent, ReactNode } from "react"; import { useCallback, useEffect, useMemo, useRef, useState } from "react"; +import { useShallow } from "zustand/react/shallow"; import { Button } from "~/components/ui/button"; import { InputGroup, InputGroupInput } from "~/components/ui/input-group"; @@ -254,14 +255,26 @@ export default function FileBrowserPanel({ }, [handleAddFiles], ); - const uploadsById = useWorkspaceUploadStore((state) => state.uploadsById); - const uploads = useMemo( - () => - Object.entries(uploadsById).filter( - ([, upload]) => upload.environmentId === environmentId && upload.cwd === cwd, - ), - [cwd, environmentId, uploadsById], + // Flattened [id, state, id, state, ...] so the shallow compare sees stable + // string ids and per-upload state refs; uploads for other panels never + // re-render this one. + const uploadEntries = useWorkspaceUploadStore( + useShallow((state) => + Object.entries(state.uploadsById) + .filter(([, upload]) => upload.environmentId === environmentId && upload.cwd === cwd) + .flat(), + ), ); + const uploads = useMemo(() => { + const pairs: Array<[string, WorkspaceUploadState]> = []; + for (let index = 0; index < uploadEntries.length; index += 2) { + pairs.push([ + uploadEntries[index] as string, + uploadEntries[index + 1] as WorkspaceUploadState, + ]); + } + return pairs; + }, [uploadEntries]); const entries = entriesQuery.data?.entries ?? []; const entryKinds = useMemo( () => new Map(entries.map((entry) => [entry.path, entry.kind] as const)), From fed2a7a346555234688ca782d752248f9a7a24b5 Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 08:03:57 +0800 Subject: [PATCH 08/54] fix(web,server): harden workspace uploads after review Floor the workspace upload body limit at 1 byte so a 0-byte upload token can't disable NodeStream's max-body check for a chunked request with no Content-Length. Route the overwrite confirm dialog through readLocalApi() like every other caller instead of calling requestConfirmDialog directly. Extract the duplicated XHR upload helper (attachments, workspace) into apps/web/src/lib/uploadXhr.ts. Raise the workspace upload timeout to 10 minutes to match the 100 MiB max and the upload token TTL. Scope the files view upload docs to web and desktop. --- apps/server/src/http.ts | 6 ++- apps/server/src/server.test.ts | 50 +++++++++++++++++++ apps/web/src/lib/attachmentUploadQueue.ts | 36 ++----------- apps/web/src/lib/uploadXhr.ts | 34 +++++++++++++ apps/web/src/lib/workspaceUploadQueue.test.ts | 4 +- apps/web/src/lib/workspaceUploadQueue.ts | 42 +++------------- docs/user/files.md | 2 + 7 files changed, 105 insertions(+), 69 deletions(-) create mode 100644 apps/web/src/lib/uploadXhr.ts diff --git a/apps/server/src/http.ts b/apps/server/src/http.ts index 73f71d80977a..6bfc94a446fc 100644 --- a/apps/server/src/http.ts +++ b/apps/server/src/http.ts @@ -315,8 +315,12 @@ export const workspaceUploadRouteLayer = HttpRouter.add( }); } + // NodeStream.toArrayBuffer treats a falsy maxBytes as "no limit", so a + // 0-byte claim (empty files are a valid upload) would otherwise disable + // the body limit entirely. Floor it at 1 byte; an empty body still passes. + const maxBodySize = FileSystem.Size(Math.max(claims.sizeBytes, 1)); const body = yield* request.arrayBuffer.pipe( - Effect.provideService(HttpServerRequest.MaxBodySize, FileSystem.Size(claims.sizeBytes)), + Effect.provideService(HttpServerRequest.MaxBodySize, maxBodySize), Effect.orElseSucceed(() => null), ); if (body === null) { diff --git a/apps/server/src/server.test.ts b/apps/server/src/server.test.ts index 92e046d2e9ca..c74c45113eb2 100644 --- a/apps/server/src/server.test.ts +++ b/apps/server/src/server.test.ts @@ -5197,6 +5197,56 @@ it.layer(NodeServices.layer)("server router seam", (it) => { }).pipe(Effect.provide(NodeHttpServer.layerTest)), ); + it.effect("floors the workspace upload body limit for zero-byte claims", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const workspaceDir = yield* fs.makeTempDirectoryScoped({ + prefix: "t3-ws-project-upload-empty-", + }); + + yield* buildAppUnderTest(); + + const wsUrl = yield* getWsServerUrl("/ws"); + yield* Effect.scoped( + withWsRpcClient(wsUrl, (client) => + Effect.gen(function* () { + const emptyTarget = yield* client[WS_METHODS.projectsCreateUploadUrl]({ + cwd: workspaceDir, + relativePath: "empty.bin", + sizeBytes: 0, + overwrite: false, + }); + const emptyResponse = yield* HttpClient.post(emptyTarget.relativeUrl, { + body: HttpBody.uint8Array(new Uint8Array(0), "application/octet-stream"), + }); + assert.equal(emptyResponse.status, 204); + const emptyPath = path.join(workspaceDir, "empty.bin"); + assert.isTrue(yield* fs.exists(emptyPath)); + assert.equal((yield* fs.readFile(emptyPath)).byteLength, 0); + + // Mint a second zero-byte claim and post a chunked body (no + // Content-Length, so the header check above is skipped) that + // exceeds it. NodeStream.toArrayBuffer treats a falsy maxBytes + // as unlimited, so this only fails once the limit is floored at + // 1 byte. + const oversizedTarget = yield* client[WS_METHODS.projectsCreateUploadUrl]({ + cwd: workspaceDir, + relativePath: "empty-oversized.bin", + sizeBytes: 0, + overwrite: false, + }); + const oversizedResponse = yield* HttpClient.post(oversizedTarget.relativeUrl, { + body: HttpBody.stream(Stream.make(new Uint8Array([1, 2, 3, 4, 5, 6]))), + }); + assert.equal(oversizedResponse.status, 400); + assert.isFalse(yield* fs.exists(path.join(workspaceDir, "empty-oversized.bin"))); + }), + ), + ); + }).pipe(Effect.provide(NodeHttpServer.layerTest)), + ); + it.effect("creates a missing workspace root during websocket project.create dispatch", () => Effect.gen(function* () { const fs = yield* FileSystem.FileSystem; diff --git a/apps/web/src/lib/attachmentUploadQueue.ts b/apps/web/src/lib/attachmentUploadQueue.ts index 37eb924ca256..26602d8a6919 100644 --- a/apps/web/src/lib/attachmentUploadQueue.ts +++ b/apps/web/src/lib/attachmentUploadQueue.ts @@ -12,6 +12,7 @@ import { appAtomRegistry } from "../rpc/atomRegistry"; import { attachmentEnvironment } from "../state/attachments"; import { readPreparedConnection } from "../state/session"; import type { AttachmentUploadState, ReadyAttachmentUpload } from "./attachmentUploadState"; +import { uploadXhr } from "./uploadXhr"; const MAX_UPLOADS_PER_ENVIRONMENT = 3; const UPLOAD_TIMEOUT_MS = 5 * 60_000; @@ -69,37 +70,6 @@ function deletePendingUpload(environmentId: EnvironmentId, attachmentId: string) ); } -function uploadBytes(input: { - readonly url: string; - readonly file: File; - readonly onProgress: (progress: number) => void; -}): { readonly done: Promise; readonly abort: () => void } { - const xhr = new XMLHttpRequest(); - const done = new Promise((resolve, reject) => { - xhr.open("POST", input.url, true); - xhr.timeout = UPLOAD_TIMEOUT_MS; - xhr.setRequestHeader("Content-Type", input.file.type); - xhr.upload.addEventListener("progress", (event) => { - if (event.lengthComputable && event.total > 0) { - input.onProgress(event.loaded / event.total); - } - }); - xhr.addEventListener("load", () => { - if (xhr.status >= 200 && xhr.status < 300) { - resolve(); - } else { - reject(new Error(`Upload rejected (${xhr.status})`)); - } - }); - xhr.addEventListener("error", () => reject(new Error("Upload failed"))); - xhr.addEventListener("timeout", () => reject(new Error("Upload timed out"))); - xhr.addEventListener("abort", () => reject(new Error("Upload cancelled"))); - xhr.send(input.file); - }); - - return { done, abort: () => xhr.abort() }; -} - async function runUpload(job: UploadJob): Promise { const mimeType = PROVIDER_SEND_TURN_SUPPORTED_IMAGE_MIME_TYPES.find( (supportedMimeType) => supportedMimeType === job.image.mimeType.toLowerCase(), @@ -158,9 +128,11 @@ async function runUpload(job: UploadJob): Promise { } let lastStep = -1; - const upload = uploadBytes({ + const upload = uploadXhr({ url, file: job.image.file, + contentType: job.image.file.type, + timeoutMs: UPLOAD_TIMEOUT_MS, onProgress: (progress) => { const step = Math.floor(progress * 20); if (step === lastStep || job.cancelled) { diff --git a/apps/web/src/lib/uploadXhr.ts b/apps/web/src/lib/uploadXhr.ts new file mode 100644 index 000000000000..871fce25af3f --- /dev/null +++ b/apps/web/src/lib/uploadXhr.ts @@ -0,0 +1,34 @@ +export function uploadXhr(input: { + readonly url: string; + readonly file: File; + readonly contentType?: string; + readonly timeoutMs: number; + readonly onProgress: (progress: number) => void; +}): { readonly done: Promise; readonly abort: () => void } { + const xhr = new XMLHttpRequest(); + const done = new Promise((resolve, reject) => { + xhr.open("POST", input.url, true); + xhr.timeout = input.timeoutMs; + if (input.contentType !== undefined) { + xhr.setRequestHeader("Content-Type", input.contentType); + } + xhr.upload.addEventListener("progress", (event) => { + if (event.lengthComputable && event.total > 0) { + input.onProgress(event.loaded / event.total); + } + }); + xhr.addEventListener("load", () => { + if (xhr.status >= 200 && xhr.status < 300) { + resolve(); + } else { + reject(new Error(`Upload rejected (${xhr.status})`)); + } + }); + xhr.addEventListener("error", () => reject(new Error("Upload failed"))); + xhr.addEventListener("timeout", () => reject(new Error("Upload timed out"))); + xhr.addEventListener("abort", () => reject(new Error("Upload cancelled"))); + xhr.send(input.file); + }); + + return { done, abort: () => xhr.abort() }; +} diff --git a/apps/web/src/lib/workspaceUploadQueue.test.ts b/apps/web/src/lib/workspaceUploadQueue.test.ts index 907364273107..8c2922dadc35 100644 --- a/apps/web/src/lib/workspaceUploadQueue.test.ts +++ b/apps/web/src/lib/workspaceUploadQueue.test.ts @@ -25,8 +25,8 @@ vi.mock("../state/session", () => ({ readPreparedConnection: mocks.readPreparedConnection, })); -vi.mock("../confirmDialog", () => ({ - requestConfirmDialog: mocks.requestConfirmDialog, +vi.mock("../localApi", () => ({ + readLocalApi: () => ({ dialogs: { confirm: mocks.requestConfirmDialog } }), })); import { diff --git a/apps/web/src/lib/workspaceUploadQueue.ts b/apps/web/src/lib/workspaceUploadQueue.ts index 5b266496e7ab..17f0dfe72363 100644 --- a/apps/web/src/lib/workspaceUploadQueue.ts +++ b/apps/web/src/lib/workspaceUploadQueue.ts @@ -4,14 +4,17 @@ import { runAtomCommand } from "@t3tools/client-runtime/state/runtime"; import * as Cause from "effect/Cause"; import { create } from "zustand"; -import { requestConfirmDialog } from "../confirmDialog"; +import { readLocalApi } from "../localApi"; import { appAtomRegistry } from "../rpc/atomRegistry"; import { projectEnvironment } from "../state/projects"; import { readPreparedConnection } from "../state/session"; import { randomUUID } from "./utils"; +import { uploadXhr } from "./uploadXhr"; const MAX_UPLOADS_PER_ENVIRONMENT = 3; -const UPLOAD_TIMEOUT_MS = 5 * 60_000; +// Matches the upload token TTL (see PROJECT_UPLOAD_URL_TTL_MS), since +// workspace uploads allow files up to 100 MiB. +const UPLOAD_TIMEOUT_MS = 10 * 60_000; export type WorkspaceUploadState = | { @@ -85,36 +88,6 @@ function failJob(job: UploadJob, reason: string): void { }); } -function uploadBytes(input: { - readonly url: string; - readonly file: File; - readonly onProgress: (progress: number) => void; -}): { readonly done: Promise; readonly abort: () => void } { - const xhr = new XMLHttpRequest(); - const done = new Promise((resolve, reject) => { - xhr.open("POST", input.url, true); - xhr.timeout = UPLOAD_TIMEOUT_MS; - xhr.upload.addEventListener("progress", (event) => { - if (event.lengthComputable && event.total > 0) { - input.onProgress(event.loaded / event.total); - } - }); - xhr.addEventListener("load", () => { - if (xhr.status >= 200 && xhr.status < 300) { - resolve(); - } else { - reject(new Error(`Upload rejected (${xhr.status})`)); - } - }); - xhr.addEventListener("error", () => reject(new Error("Upload failed"))); - xhr.addEventListener("timeout", () => reject(new Error("Upload timed out"))); - xhr.addEventListener("abort", () => reject(new Error("Upload cancelled"))); - xhr.send(input.file); - }); - - return { done, abort: () => xhr.abort() }; -} - function mintUploadUrl(job: UploadJob) { return runAtomCommand( appAtomRegistry, @@ -154,7 +127,7 @@ async function runUpload(job: UploadJob): Promise { return; } - const confirmed = await requestConfirmDialog( + const confirmed = await readLocalApi()?.dialogs.confirm( `Replace ${job.file.name}?\nA file named '${job.relativePath}' already exists in this project.`, ); if (job.cancelled) { @@ -186,9 +159,10 @@ async function runUpload(job: UploadJob): Promise { } let lastStep = -1; - const upload = uploadBytes({ + const upload = uploadXhr({ url, file: job.file, + timeoutMs: UPLOAD_TIMEOUT_MS, onProgress: (progress) => { const step = Math.floor(progress * 20); if (step === lastStep || job.cancelled) { diff --git a/docs/user/files.md b/docs/user/files.md index bdecc6991d7a..a953804d070c 100644 --- a/docs/user/files.md +++ b/docs/user/files.md @@ -13,6 +13,8 @@ message without leaving the tree. ## Uploading files +Uploading is available in the web and desktop apps. The mobile files view does not support it. + Drag files from your computer onto the files view, or select the upload button at the top of the panel to choose files from a picker. Uploads land in the project's root folder. Dragging a folder does not upload its contents; drop the files themselves. From b298d644bddc6ab0745fe745336f7f51a4c57608 Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 09:09:40 +0800 Subject: [PATCH 09/54] fix(web,server): close upload races and dedupe the drop overlay Store the non-overwrite upload with an atomic hard link so a concurrent upload gets a 409 instead of silently replacing the file, and ignore a second retry click while the retried job is already uploading. Share one drop-overlay component between the chat and files views, reuse the attachment progress formatter, cap the uploads strip height, size the row buttons to the compact-row contract, and name the mint target in the resolve error message. --- apps/server/src/workspace/WorkspaceUpload.ts | 24 ++++++++++++++++-- apps/web/src/components/ChatView.tsx | 16 ++++-------- .../chat/WorkspaceFileDropOverlay.tsx | 22 ++++++++++++++++ .../src/components/files/FileBrowserPanel.tsx | 25 ++++++++----------- apps/web/src/lib/workspaceUploadQueue.test.ts | 18 +++++++++++++ apps/web/src/lib/workspaceUploadQueue.ts | 4 +++ 6 files changed, 81 insertions(+), 28 deletions(-) create mode 100644 apps/web/src/components/chat/WorkspaceFileDropOverlay.tsx diff --git a/apps/server/src/workspace/WorkspaceUpload.ts b/apps/server/src/workspace/WorkspaceUpload.ts index 8f89a93d4af5..0e289a8928c7 100644 --- a/apps/server/src/workspace/WorkspaceUpload.ts +++ b/apps/server/src/workspace/WorkspaceUpload.ts @@ -85,7 +85,7 @@ export const issueWorkspaceUploadUrl = Effect.fn("WorkspaceUpload.issueUrl")(fun new ProjectCreateUploadUrlError({ cwd: input.cwd, relativePath: input.relativePath, - message: error.message, + message: `Failed to resolve '${input.relativePath}' within '${input.cwd}'.`, cause: error, }), ), @@ -202,7 +202,27 @@ export const storeWorkspaceUpload = Effect.fn("WorkspaceUpload.store")(function* yield* fileSystem.makeDirectory(path.dirname(target.absolutePath), { recursive: true }); yield* fileSystem.writeFile(partPath, bytes); - yield* fileSystem.rename(partPath, target.absolutePath); + if (claims.overwrite) { + yield* fileSystem.rename(partPath, target.absolutePath); + } else { + // rename replaces a file created after the exists check above; link fails + // atomically instead, so concurrent non-overwrite uploads cannot clobber. + const conflict = yield* fileSystem.link(partPath, target.absolutePath).pipe( + Effect.as(false), + Effect.catchIf( + (error) => error.reason._tag === "AlreadyExists", + () => Effect.succeed(true), + ), + ); + yield* fileSystem.remove(partPath, { force: true }); + if (conflict) { + return { + ok: false, + status: 409, + detail: "A file already exists at this path.", + } satisfies StoreWorkspaceUploadResult; + } + } const workspaceEntries = yield* WorkspaceEntries.WorkspaceEntries; yield* workspaceEntries.refresh(claims.cwd); diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index cb1cf698535a..d650673fd78e 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -156,6 +156,7 @@ import { ThreadPreviewMiniPlayer } from "./preview/ThreadPreviewMiniPlayer"; import { subscribePreviewAction } from "./preview/previewActionBus"; import { getConfiguredPreviewUrls } from "./preview/previewEmptyStateLogic"; import { makeWorkspaceFileDropHandlers } from "./chat/workspaceFileDrop"; +import { WorkspaceFileDropOverlay } from "./chat/WorkspaceFileDropOverlay"; import { selectThreadPreviewMiniPlayer, usePreviewMiniPlayerStore, @@ -6669,18 +6670,11 @@ function ChatViewContent(props: ChatViewProps) { onDrop={workspaceFileDropHandlers.onDrop} > {isWorkspaceFileDragActive ? ( - + /> ) : null} {/* Provider status overlays the timeline without changing its content height. */}
diff --git a/apps/web/src/components/chat/WorkspaceFileDropOverlay.tsx b/apps/web/src/components/chat/WorkspaceFileDropOverlay.tsx new file mode 100644 index 000000000000..4d96ce250a04 --- /dev/null +++ b/apps/web/src/components/chat/WorkspaceFileDropOverlay.tsx @@ -0,0 +1,22 @@ +import type { ComponentPropsWithoutRef, ReactNode } from "react"; + +/** Full-surface drop treatment shared by the chat and files-view drop targets. */ +export function WorkspaceFileDropOverlay( + props: { icon: ReactNode; label: string } & ComponentPropsWithoutRef<"div">, +) { + const { icon, label, ...rest } = props; + return ( +
+
+ {icon} + {label} +
+
+ ); +} diff --git a/apps/web/src/components/files/FileBrowserPanel.tsx b/apps/web/src/components/files/FileBrowserPanel.tsx index bc546ff8e886..f3654e097853 100644 --- a/apps/web/src/components/files/FileBrowserPanel.tsx +++ b/apps/web/src/components/files/FileBrowserPanel.tsx @@ -17,6 +17,7 @@ import { Tooltip, TooltipPopup, TooltipTrigger } from "~/components/ui/tooltip"; import { useComposerHandleContext } from "~/composerHandleContext"; import { writeTextToClipboard } from "~/hooks/useCopyToClipboard"; import { useTheme } from "~/hooks/useTheme"; +import { formatAttachmentUploadProgress } from "~/lib/attachmentUploadState"; import { cancelWorkspaceUpload, dismissWorkspaceUpload, @@ -30,6 +31,7 @@ import { readLocalApi } from "~/localApi"; import { T3_PIERRE_ICONS } from "~/pierre-icons"; import { makeWorkspaceFileDropHandlers } from "../chat/workspaceFileDrop"; +import { WorkspaceFileDropOverlay } from "../chat/WorkspaceFileDropOverlay"; import { createFileTreeDragMentionController } from "./fileTreeDragMention"; import { useProjectEntriesQuery } from "./projectFilesQueryState"; @@ -110,8 +112,8 @@ function UploadRowButton(props: { label: string; icon: ReactNode; onClick: () => render={ + + + + + ); +} diff --git a/docs/user/files.md b/docs/user/files.md index a953804d070c..d8930c42aecc 100644 --- a/docs/user/files.md +++ b/docs/user/files.md @@ -27,3 +27,14 @@ and gives you retry and dismiss buttons. Uploading works the same way whether you're connected locally, over a remote network, or through a tunnel. + +## Renaming and deleting files + +Right-click a file for **Rename** and **Delete**. Renaming and deleting work on files only; +folders are not yet supported. + +Rename opens a dialog prefilled with the current name. The file keeps its place: renaming changes +the name, not the folder. If a file with the new name already exists, the rename is refused and +the existing file is left untouched. + +Delete asks for confirmation first, then permanently removes the file from the project. From e14ef6cda823bd4b29fa90b5d8278ea9e07fbffe Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 10:02:36 +0800 Subject: [PATCH 28/54] fix(server): keep rename from destroying data under concurrency Review of the rename path found three gaps. The rollback after a failed source removal deleted the link target even when the source was removed by someone else, destroying the only remaining copy; rollback now runs only for failures other than a missing source. An interrupt between the link and the removal stranded both names on disk; the pair now runs uninterruptibly. A case-only rename on a case-insensitive filesystem collided with the source itself; a same-inode conflict now falls back to rename, which applies the case change without clobber risk. --- .../src/workspace/WorkspaceFileSystem.test.ts | 27 ++++++ .../src/workspace/WorkspaceFileSystem.ts | 89 ++++++++++++++----- 2 files changed, 94 insertions(+), 22 deletions(-) diff --git a/apps/server/src/workspace/WorkspaceFileSystem.test.ts b/apps/server/src/workspace/WorkspaceFileSystem.test.ts index 95348306ab7e..dbc70f2344b7 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.test.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.test.ts @@ -347,6 +347,33 @@ it.layer(TestLayer, { excludeTestServices: true })("WorkspaceFileSystemLive", (i }), ); + // A hard-linked target reaches the same-inode path a case-only rename + // takes on a case-insensitive filesystem, deterministically on Linux. + it.effect("renames onto another name of the same file without destroying it", () => + 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/notes.md", "# Notes\n"); + yield* fileSystem + .link(path.join(cwd, "src/notes.md"), path.join(cwd, "src/Notes.md")) + .pipe(Effect.orDie); + + const result = yield* workspaceFileSystem.renameEntry({ + cwd, + relativePath: "src/notes.md", + newRelativePath: "src/Notes.md", + }); + + expect(result).toEqual({ relativePath: "src/Notes.md" }); + const renamed = yield* fileSystem + .readFileString(path.join(cwd, "src/Notes.md")) + .pipe(Effect.orDie); + expect(renamed).toBe("# Notes\n"); + }), + ); + it.effect("rejects renames that leave the source directory", () => 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 9f846d58854a..7aa128ee6756 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.ts @@ -27,6 +27,7 @@ import * as Context from "effect/Context"; import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; +import * as Option from "effect/Option"; import * as Path from "effect/Path"; import * as Schema from "effect/Schema"; @@ -344,6 +345,26 @@ export const make = Effect.gen(function* () { ); }); + // A link conflict can be the source itself seen under another name, either + // a case variant on a case-insensitive filesystem or a pre-existing hard + // link; the same device and inode identifies it. Stat failures count as a + // genuine conflict, the safe reading. + const isSameFile = Effect.fn(function* (leftPath: string, rightPath: string) { + const stats = yield* Effect.all([fileSystem.stat(leftPath), fileSystem.stat(rightPath)]).pipe( + Effect.orElseSucceed(() => null), + ); + if (stats === null) { + return false; + } + const [left, right] = stats; + return ( + left.dev === right.dev && + Option.isSome(left.ino) && + Option.isSome(right.ino) && + left.ino.value === right.ino.value + ); + }); + const renameEntry: WorkspaceFileSystem["Service"]["renameEntry"] = Effect.fn( "WorkspaceFileSystem.renameEntry", )(function* (input) { @@ -394,28 +415,52 @@ export const make = Effect.gen(function* () { } // rename would replace an existing target; link fails atomically instead, - // so a rename can never clobber another entry. - const conflict = yield* fileSystem.link(source.absolutePath, target.absolutePath).pipe( - Effect.as(false), - Effect.catchIf( - (error) => error.reason._tag === "AlreadyExists", - () => Effect.succeed(true), - ), - Effect.mapError((cause) => renameError("rename", cause)), - ); - if (conflict) { - return yield* new ProjectRenameEntryTargetExistsError({ - cwd: input.cwd, - relativePath: target.relativePath, - }); - } - yield* fileSystem.remove(source.absolutePath).pipe( - Effect.catch((cause) => - Effect.gen(function* () { - yield* fileSystem.remove(target.absolutePath, { force: true }).pipe(Effect.ignore); - return yield* renameError("rename", cause); - }), - ), + // so a rename can never clobber another entry. The link and the source + // removal form one critical section: an interrupt between them would + // strand both names on disk, so the pair runs uninterruptibly. + yield* Effect.uninterruptible( + Effect.gen(function* () { + const conflict = yield* fileSystem.link(source.absolutePath, target.absolutePath).pipe( + Effect.as(false), + Effect.catchIf( + (error) => error.reason._tag === "AlreadyExists", + () => Effect.succeed(true), + ), + Effect.mapError((cause) => renameError("rename", cause)), + ); + if (conflict) { + // On a case-insensitive filesystem a case-only rename collides with + // the source itself. A same-inode target is the source, so rename, + // which applies the case change and cannot clobber another entry. + const sameFile = yield* isSameFile(source.absolutePath, target.absolutePath); + if (!sameFile) { + return yield* new ProjectRenameEntryTargetExistsError({ + cwd: input.cwd, + relativePath: target.relativePath, + }); + } + return yield* fileSystem + .rename(source.absolutePath, target.absolutePath) + .pipe(Effect.mapError((cause) => renameError("rename", cause))); + } + yield* fileSystem.remove(source.absolutePath).pipe( + // A missing source means something else removed it after the link + // landed, leaving the target as the only copy of the data; rolling + // the link back would destroy it. Only real removal failures, where + // the source still exists, undo the link. + Effect.catchIf( + (error) => error.reason._tag === "NotFound", + () => Effect.void, + ), + Effect.catchTags({ + PlatformError: (cause) => + Effect.gen(function* () { + yield* fileSystem.remove(target.absolutePath, { force: true }).pipe(Effect.ignore); + return yield* renameError("rename", cause); + }), + }), + ); + }), ); yield* workspaceEntries.refresh(input.cwd); From f3ecbaece12193da6d42aad5a90b23dbb6956e72 Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 10:08:03 +0800 Subject: [PATCH 29/54] fix(server): remove the source entry when renaming onto a hard link POSIX rename over another name of the same file is a no-op, so the same-inode rename fallback left the source entry behind when the conflict was a pre-existing hard link pair rather than a case variant. The directory listing reports exact on-disk names and tells the shapes apart: both names listed removes the source entry, only the source listed renames to apply the case change, and otherwise the target name already holds the data. The hard-link test now asserts the source entry is gone. --- .../src/workspace/WorkspaceFileSystem.test.ts | 2 ++ .../src/workspace/WorkspaceFileSystem.ts | 33 ++++++++++++++++--- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/apps/server/src/workspace/WorkspaceFileSystem.test.ts b/apps/server/src/workspace/WorkspaceFileSystem.test.ts index dbc70f2344b7..58641de732e6 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.test.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.test.ts @@ -371,6 +371,8 @@ it.layer(TestLayer, { excludeTestServices: true })("WorkspaceFileSystemLive", (i .readFileString(path.join(cwd, "src/Notes.md")) .pipe(Effect.orDie); expect(renamed).toBe("# Notes\n"); + const sourceExists = yield* fileSystem.exists(path.join(cwd, "src/notes.md")); + expect(sourceExists).toBe(false); }), ); diff --git a/apps/server/src/workspace/WorkspaceFileSystem.ts b/apps/server/src/workspace/WorkspaceFileSystem.ts index 7aa128ee6756..7ccee08419b4 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.ts @@ -429,9 +429,6 @@ export const make = Effect.gen(function* () { Effect.mapError((cause) => renameError("rename", cause)), ); if (conflict) { - // On a case-insensitive filesystem a case-only rename collides with - // the source itself. A same-inode target is the source, so rename, - // which applies the case change and cannot clobber another entry. const sameFile = yield* isSameFile(source.absolutePath, target.absolutePath); if (!sameFile) { return yield* new ProjectRenameEntryTargetExistsError({ @@ -439,9 +436,35 @@ export const make = Effect.gen(function* () { relativePath: target.relativePath, }); } - return yield* fileSystem - .rename(source.absolutePath, target.absolutePath) + // The conflicting target is another name of the source's own inode. + // The directory listing reports exact on-disk names and tells the + // two shapes apart. Both names listed is a pre-existing hard link + // pair, where POSIX rename over another name of the same file is a + // no-op, so removing the source entry completes the rename. Only + // the source listed is the source under another casing on a + // case-insensitive filesystem, where rename applies the case + // change. Otherwise the source entry is gone or already carries + // the target casing, and the target name holds the data. + const siblingNames = yield* fileSystem + .readDirectory(path.dirname(source.absolutePath)) .pipe(Effect.mapError((cause) => renameError("rename", cause))); + const sourceListed = siblingNames.includes(path.basename(source.absolutePath)); + const targetListed = siblingNames.includes(path.basename(target.absolutePath)); + if (sourceListed && targetListed) { + return yield* fileSystem.remove(source.absolutePath).pipe( + Effect.catchIf( + (error) => error.reason._tag === "NotFound", + () => Effect.void, + ), + Effect.mapError((cause) => renameError("rename", cause)), + ); + } + if (sourceListed) { + return yield* fileSystem + .rename(source.absolutePath, target.absolutePath) + .pipe(Effect.mapError((cause) => renameError("rename", cause))); + } + return; } yield* fileSystem.remove(source.absolutePath).pipe( // A missing source means something else removed it after the link From 90b838ef2fa658da92e545f0405faf0b228abb59 Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 10:12:27 +0800 Subject: [PATCH 30/54] fix(server): treat renaming a file onto its own path as a no-op The hard-link-pair branch reads an identical source and target path as two listed names of one inode and removed the file's only directory entry while reporting success. The web dialog blocks unchanged names, but the RPC has no such gate. Identical resolved paths now return success before touching the filesystem, with a test pinning the file's survival. --- .../src/workspace/WorkspaceFileSystem.test.ts | 22 +++++++++++++++++++ .../src/workspace/WorkspaceFileSystem.ts | 7 ++++++ 2 files changed, 29 insertions(+) diff --git a/apps/server/src/workspace/WorkspaceFileSystem.test.ts b/apps/server/src/workspace/WorkspaceFileSystem.test.ts index 58641de732e6..6ff17a4b6068 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.test.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.test.ts @@ -347,6 +347,28 @@ it.layer(TestLayer, { excludeTestServices: true })("WorkspaceFileSystemLive", (i }), ); + it.effect("leaves the file untouched when renamed onto its own path", () => + 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/notes.md", "# Notes\n"); + + const result = yield* workspaceFileSystem.renameEntry({ + cwd, + relativePath: "src/notes.md", + newRelativePath: "src/notes.md", + }); + + expect(result).toEqual({ relativePath: "src/notes.md" }); + const contents = yield* fileSystem + .readFileString(path.join(cwd, "src/notes.md")) + .pipe(Effect.orDie); + expect(contents).toBe("# Notes\n"); + }), + ); + // A hard-linked target reaches the same-inode path a case-only rename // takes on a case-insensitive filesystem, deterministically on Linux. it.effect("renames onto another name of the same file without destroying it", () => diff --git a/apps/server/src/workspace/WorkspaceFileSystem.ts b/apps/server/src/workspace/WorkspaceFileSystem.ts index 7ccee08419b4..07af9e6764a2 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.ts @@ -414,6 +414,13 @@ export const make = Effect.gen(function* () { ); } + // Renaming a file onto its own exact path is a no-op. Without this guard + // the identical path would read as a hard link pair below and the source + // removal would delete the file's only directory entry. + if (source.relativePath === target.relativePath) { + return { relativePath: target.relativePath }; + } + // rename would replace an existing target; link fails atomically instead, // so a rename can never clobber another entry. The link and the source // removal form one critical section: an interrupt between them would From ad7423170db8a41b63bd6916a42fa766197b65c2 Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 10:59:24 +0800 Subject: [PATCH 31/54] fix(web): keep open file surfaces in sync with rename and delete Renaming or deleting a file from the tree only refreshed the listing. An open surface for the old path kept rendering stale contents, and its debounced saves could recreate the deleted file or the pre-rename name. Now a delete closes the file surface and a rename follows it to the new path, and the save coordinator discards unsaved edits for the removed path instead of flushing them on dispose. The delete confirm also uses the destructive dialog variant to match its menu item. --- .../src/components/files/FileBrowserPanel.tsx | 13 ++++++- .../src/components/files/FilePreviewPanel.tsx | 34 ++++++++++++++++++- .../components/files/RenameEntryDialog.tsx | 7 ++-- .../files/fileSaveCoordinator.test.ts | 22 ++++++++++++ .../components/files/fileSaveCoordinator.ts | 8 +++++ 5 files changed, 79 insertions(+), 5 deletions(-) diff --git a/apps/web/src/components/files/FileBrowserPanel.tsx b/apps/web/src/components/files/FileBrowserPanel.tsx index 0331d3e7efa4..58e55da28738 100644 --- a/apps/web/src/components/files/FileBrowserPanel.tsx +++ b/apps/web/src/components/files/FileBrowserPanel.tsx @@ -50,6 +50,10 @@ interface FileBrowserPanelProps { selectedPathRevealId: number; onOpenFile: (relativePath: string) => void; onRefreshSelectedFile?: () => void; + /** A rename succeeded; open surfaces for the old path should follow the file. */ + onEntryRenamed?: (relativePath: string, newRelativePath: string) => void; + /** A delete succeeded; open surfaces for the path should close. */ + onEntryDeleted?: (relativePath: string) => void; } const TREE_UNSAFE_CSS = ` @@ -216,6 +220,8 @@ export default function FileBrowserPanel({ selectedPathRevealId, onOpenFile, onRefreshSelectedFile, + onEntryRenamed, + onEntryDeleted, }: FileBrowserPanelProps) { const { resolvedTheme } = useTheme(); const composerRef = useComposerHandleContext(); @@ -311,6 +317,7 @@ export default function FileBrowserPanel({ const name = relativePath.split("/").at(-1) ?? relativePath; const confirmed = await readLocalApi()?.dialogs.confirm( `Delete ${name}?\nThis permanently deletes the file from the project.`, + { variant: "destructive" }, ); if (confirmed !== true) return; const result = await runAtomCommand( @@ -321,6 +328,7 @@ export default function FileBrowserPanel({ ); if (result._tag === "Success") { entriesQuery.refresh(); + onEntryDeleted?.(relativePath); return; } const failure = Cause.squash(result.cause); @@ -630,7 +638,10 @@ export default function FileBrowserPanel({ cwd={cwd} relativePath={renameTarget} onClose={() => setRenameTarget(null)} - onRenamed={() => entriesQuery.refresh()} + onRenamed={(newRelativePath) => { + entriesQuery.refresh(); + onEntryRenamed?.(renameTarget, newRelativePath); + }} /> ) : null}
diff --git a/apps/web/src/components/files/FilePreviewPanel.tsx b/apps/web/src/components/files/FilePreviewPanel.tsx index a8c364763c28..acdc6c0cbb4e 100644 --- a/apps/web/src/components/files/FilePreviewPanel.tsx +++ b/apps/web/src/components/files/FilePreviewPanel.tsx @@ -27,6 +27,7 @@ import { getLocalStorageItem, setLocalStorageItem, useLocalStorage } from "~/hoo import { DIFF_SURFACE_THEME_UNSAFE_CSS, resolveDiffThemeName } from "~/lib/diffRendering"; import { cn } from "~/lib/utils"; import { isPreviewSupportedInRuntime } from "~/previewStateStore"; +import { selectThreadRightPanelState, useRightPanelStore } from "~/rightPanelStore"; import { resolvePathLinkTarget } from "~/terminal-links"; import { ScrollArea } from "~/components/ui/scroll-area"; import { Toggle } from "~/components/ui/toggle"; @@ -393,6 +394,8 @@ interface EditableFileSurfaceProps { wordWrap: boolean; onPostRender: FilePostRender; onPendingChange: (relativePath: string, pending: boolean) => void; + /** Receives a callback that drops unsaved edits; used when the file is deleted or renamed away. */ + discardSavesRef?: { current: (() => void) | null } | undefined; } interface FileSelectionOverride { @@ -405,9 +408,10 @@ function useFileSaveCoordinator({ cwd, relativePath, onPendingChange, + discardSavesRef, }: Pick< EditableFileSurfaceProps, - "environmentId" | "cwd" | "relativePath" | "onPendingChange" + "environmentId" | "cwd" | "relativePath" | "onPendingChange" | "discardSavesRef" >): FileSaveCoordinator { const writeFile = useAtomCommand(projectEnvironment.writeFile); const coordinator = useMemo( @@ -428,6 +432,13 @@ function useFileSaveCoordinator({ ); useEffect(() => () => coordinator.dispose(), [coordinator]); + useEffect(() => { + if (!discardSavesRef) return; + discardSavesRef.current = () => coordinator.discard(); + return () => { + discardSavesRef.current = null; + }; + }, [coordinator, discardSavesRef]); return coordinator; } @@ -442,6 +453,7 @@ function EditableFileSurface({ wordWrap, onPostRender, onPendingChange, + discardSavesRef, }: EditableFileSurfaceProps) { const addReviewComment = useComposerDraftStore((store) => store.addReviewComment); const removeReviewComment = useComposerDraftStore((store) => store.removeReviewComment); @@ -462,6 +474,7 @@ function EditableFileSurface({ cwd, relativePath, onPendingChange, + discardSavesRef, }); const editor = useMemo( () => @@ -707,6 +720,7 @@ function RenderedMarkdownSurface({ contents, threadRef, onPendingChange, + discardSavesRef, }: Omit< EditableFileSurfaceProps, | "resolvedTheme" @@ -723,6 +737,7 @@ function RenderedMarkdownSurface({ cwd, relativePath, onPendingChange, + discardSavesRef, }); return ( @@ -797,6 +812,7 @@ export default function FilePreviewPanel({ null, ); const breadcrumbRef = useRef(null); + const discardActiveFileSavesRef = useRef<(() => void) | null>(null); const isMarkdown = relativePath ? isMarkdownPreviewFile(relativePath) : false; // A reveal still wins over the preference: the line only exists in the source. const renderMarkdown = @@ -1019,6 +1035,7 @@ export default function FilePreviewPanel({ threadRef={threadRef} contents={file.data.contents} onPendingChange={onPendingChange} + discardSavesRef={discardActiveFileSavesRef} /> ) : file.data.truncated ? ( ) ) : null} @@ -1081,6 +1099,20 @@ export default function FilePreviewPanel({ selectedPathRevealId={revealRequestId} onOpenFile={onOpenFile} {...(relativePath && !isImage ? { onRefreshSelectedFile: file.refresh } : {})} + onEntryDeleted={(path) => { + if (path === relativePath) discardActiveFileSavesRef.current?.(); + useRightPanelStore.getState().closeSurface(threadRef, `file:${path}`); + }} + onEntryRenamed={(from, to) => { + if (from === relativePath) discardActiveFileSavesRef.current?.(); + const store = useRightPanelStore.getState(); + const wasOpen = selectThreadRightPanelState( + store.byThreadKey, + threadRef, + ).surfaces.some((surface) => surface.id === `file:${from}`); + store.closeSurface(threadRef, `file:${from}`); + if (wasOpen) store.openFile(threadRef, to); + }} /> ) : null} diff --git a/apps/web/src/components/files/RenameEntryDialog.tsx b/apps/web/src/components/files/RenameEntryDialog.tsx index 11b92fd9869a..6ffe399a8a7b 100644 --- a/apps/web/src/components/files/RenameEntryDialog.tsx +++ b/apps/web/src/components/files/RenameEntryDialog.tsx @@ -37,7 +37,7 @@ export function RenameEntryDialog({ readonly cwd: string; readonly relativePath: string; readonly onClose: () => void; - readonly onRenamed: () => void; + readonly onRenamed: (newRelativePath: string) => void; }) { const lastSlash = relativePath.lastIndexOf("/"); const directoryPrefix = lastSlash === -1 ? "" : relativePath.slice(0, lastSlash + 1); @@ -76,19 +76,20 @@ export function RenameEntryDialog({ isRenamingRef.current = true; setIsRenaming(true); setRenameError(null); + const newRelativePath = `${directoryPrefix}${candidate}`; const result = await runAtomCommand( appAtomRegistry, projectEnvironment.renameEntry, { environmentId, - input: { cwd, relativePath, newRelativePath: `${directoryPrefix}${candidate}` }, + input: { cwd, relativePath, newRelativePath }, }, { reportFailure: false }, ); isRenamingRef.current = false; setIsRenaming(false); if (result._tag === "Success") { - onRenamed(); + onRenamed(newRelativePath); onClose(); return; } diff --git a/apps/web/src/components/files/fileSaveCoordinator.test.ts b/apps/web/src/components/files/fileSaveCoordinator.test.ts index 1acbb0c1d205..ce000c9d7d44 100644 --- a/apps/web/src/components/files/fileSaveCoordinator.test.ts +++ b/apps/web/src/components/files/fileSaveCoordinator.test.ts @@ -91,4 +91,26 @@ describe("FileSaveCoordinator", () => { expect(onPendingChange).toHaveBeenCalledWith(true); expect(onPendingChange).not.toHaveBeenCalledWith(false); }); + + it("discard drops unsaved edits instead of flushing them on dispose", async () => { + vi.useFakeTimers(); + const persist = vi + .fn<(contents: string) => Promise>>() + .mockResolvedValue(AsyncResult.success(undefined)); + const onPendingChange = vi.fn(); + const coordinator = new FileSaveCoordinator({ + debounceMs: 500, + persist, + onPendingChange, + onConfirmed: vi.fn(), + }); + + coordinator.change("doomed"); + coordinator.discard(); + coordinator.dispose(); + await vi.runAllTimersAsync(); + + expect(persist).not.toHaveBeenCalled(); + expect(onPendingChange.mock.calls.at(-1)).toEqual([false]); + }); }); diff --git a/apps/web/src/components/files/fileSaveCoordinator.ts b/apps/web/src/components/files/fileSaveCoordinator.ts index 138f01d360e3..9f6db61f6d4e 100644 --- a/apps/web/src/components/files/fileSaveCoordinator.ts +++ b/apps/web/src/components/files/fileSaveCoordinator.ts @@ -31,6 +31,14 @@ export class FileSaveCoordinator { if (this.latestRevision > 0) void this.persistLatest(); } + /** Drop unsaved edits without persisting; for files removed out from under the surface. */ + discard(): void { + this.disposed = true; + this.clearTimer(); + this.latestRevision = 0; + this.options.onPendingChange(false); + } + private schedule(delay: number): void { this.clearTimer(); this.timer = setTimeout(() => { From 9105b079e9ce8c96bcd50bc65b0c983b0e060f8c Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 10:59:32 +0800 Subject: [PATCH 32/54] fix(server): reject renaming onto a symlink that points at the source The same-file check followed symlinks, so a symlink at the target read as another name of the source's inode. The rename then removed the source and left the symlink dangling, losing the contents. The check now compares lstat identity, so a symlink is a distinct entry and the rename fails with the target-exists conflict. --- .../src/workspace/WorkspaceFileSystem.test.ts | 31 +++++++++++++++++++ .../src/workspace/WorkspaceFileSystem.ts | 23 +++++++------- 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/apps/server/src/workspace/WorkspaceFileSystem.test.ts b/apps/server/src/workspace/WorkspaceFileSystem.test.ts index 6ff17a4b6068..bd16dd210653 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.test.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.test.ts @@ -398,6 +398,37 @@ it.layer(TestLayer, { excludeTestServices: true })("WorkspaceFileSystemLive", (i }), ); + // A symlink at the target stats to the source's inode but is its own + // entry; treating it as the same file would delete the source and leave + // the symlink dangling. + it.effect("rejects renaming onto a symlink that points at the source", () => + 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/notes.md", "# Notes\n"); + yield* fileSystem + .symlink(path.join(cwd, "src/notes.md"), path.join(cwd, "src/link.md")) + .pipe(Effect.orDie); + + const error = yield* workspaceFileSystem + .renameEntry({ + cwd, + relativePath: "src/notes.md", + newRelativePath: "src/link.md", + }) + .pipe(Effect.flip); + + expect(error).toBeInstanceOf(ProjectRenameEntryTargetExistsError); + expect(error).toMatchObject({ cwd, relativePath: "src/link.md" }); + const source = yield* fileSystem + .readFileString(path.join(cwd, "src/notes.md")) + .pipe(Effect.orDie); + expect(source).toBe("# Notes\n"); + }), + ); + it.effect("rejects renames that leave the source directory", () => 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 07af9e6764a2..ada659f17d02 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.ts @@ -27,7 +27,6 @@ import * as Context from "effect/Context"; import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; -import * as Option from "effect/Option"; import * as Path from "effect/Path"; import * as Schema from "effect/Schema"; @@ -347,22 +346,22 @@ export const make = Effect.gen(function* () { // A link conflict can be the source itself seen under another name, either // a case variant on a case-insensitive filesystem or a pre-existing hard - // link; the same device and inode identifies it. Stat failures count as a - // genuine conflict, the safe reading. + // link; the same device and inode identifies it. lstat, not stat: a symlink + // pointing at the source is a distinct entry, and following it here would + // let the rename delete the source and leave the symlink dangling. Stat + // failures count as a genuine conflict, the safe reading. const isSameFile = Effect.fn(function* (leftPath: string, rightPath: string) { - const stats = yield* Effect.all([fileSystem.stat(leftPath), fileSystem.stat(rightPath)]).pipe( - Effect.orElseSucceed(() => null), - ); + const stats = yield* Effect.tryPromise(() => + Promise.all([ + NodeFSP.lstat(leftPath, { bigint: true }), + NodeFSP.lstat(rightPath, { bigint: true }), + ]), + ).pipe(Effect.orElseSucceed(() => null)); if (stats === null) { return false; } const [left, right] = stats; - return ( - left.dev === right.dev && - Option.isSome(left.ino) && - Option.isSome(right.ino) && - left.ino.value === right.ino.value - ); + return left.dev === right.dev && left.ino === right.ino; }); const renameEntry: WorkspaceFileSystem["Service"]["renameEntry"] = Effect.fn( From ec34b446a6ddd7b43b00a1ff3603ce2b638dad29 Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 11:13:41 +0800 Subject: [PATCH 33/54] fix(contracts): make cause optional on rename and delete errors The not-a-file and cross-directory stages are pure validation failures with no underlying error, so a required cause forced call sites to manufacture one. cause is now optional, matching ProjectWriteFileError, and validation stages construct without it. --- .../src/workspace/WorkspaceFileSystem.ts | 19 +++++-------------- packages/contracts/src/project.ts | 14 +++++++++----- 2 files changed, 14 insertions(+), 19 deletions(-) diff --git a/apps/server/src/workspace/WorkspaceFileSystem.ts b/apps/server/src/workspace/WorkspaceFileSystem.ts index ada659f17d02..28c678994c1f 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.ts @@ -367,7 +367,7 @@ export const make = Effect.gen(function* () { const renameEntry: WorkspaceFileSystem["Service"]["renameEntry"] = Effect.fn( "WorkspaceFileSystem.renameEntry", )(function* (input) { - const renameError = (stage: ProjectRenameEntryStage, cause: unknown) => + const renameError = (stage: ProjectRenameEntryStage, cause?: unknown) => new ProjectRenameEntryError({ cwd: input.cwd, relativePath: input.relativePath, @@ -386,20 +386,14 @@ export const make = Effect.gen(function* () { }), ]).pipe(Effect.mapError((cause) => renameError("resolve-path", cause))); if (path.dirname(source.relativePath) !== path.dirname(target.relativePath)) { - return yield* renameError( - "cross-directory", - `'${target.relativePath}' is outside the directory of '${source.relativePath}'.`, - ); + return yield* renameError("cross-directory"); } const sourceStat = yield* fileSystem .stat(source.absolutePath) .pipe(Effect.mapError((cause) => renameError("resolve-path", cause))); if (sourceStat.type !== "File") { - return yield* renameError( - "not-a-file", - `'${source.relativePath}' is a ${sourceStat.type}, not a file.`, - ); + return yield* renameError("not-a-file"); } const escapes = yield* directoryEscapesWorkspaceRoot( @@ -499,7 +493,7 @@ export const make = Effect.gen(function* () { const deleteEntry: WorkspaceFileSystem["Service"]["deleteEntry"] = Effect.fn( "WorkspaceFileSystem.deleteEntry", )(function* (input) { - const deleteError = (stage: ProjectDeleteEntryStage, cause: unknown) => + const deleteError = (stage: ProjectDeleteEntryStage, cause?: unknown) => new ProjectDeleteEntryError({ cwd: input.cwd, relativePath: input.relativePath, @@ -547,10 +541,7 @@ export const make = Effect.gen(function* () { return; } if (targetStat.type === "Directory") { - return yield* deleteError( - "not-a-file", - `'${target.relativePath}' is a directory; directories cannot be deleted from the files view yet.`, - ); + return yield* deleteError("not-a-file"); } yield* fileSystem diff --git a/packages/contracts/src/project.ts b/packages/contracts/src/project.ts index c9b3b84433a1..3389285c4ab3 100644 --- a/packages/contracts/src/project.ts +++ b/packages/contracts/src/project.ts @@ -418,7 +418,7 @@ type ProjectRenameEntryFailureContext = { readonly cwd: string; readonly relativePath: string; readonly stage: ProjectRenameEntryStage; - readonly cause: unknown; + readonly cause?: unknown; }; function projectRenameEntryStageMessage(props: ProjectRenameEntryFailureContext): string { @@ -441,7 +441,9 @@ export class ProjectRenameEntryError extends Schema.TaggedErrorClass Date: Tue, 25 Aug 2026 11:37:10 +0800 Subject: [PATCH 34/54] fix(web): keep editor state consistent through delete and rename Delete and rename now clear the optimistic file query so a reopened path refetches instead of showing stale bytes. The discard guard checks the file the editor shows now, not the render that created the callback, so a thread switch cannot drop the new thread's edits. Renaming a background tab no longer steals the active surface. --- .../src/components/files/FilePreviewPanel.tsx | 36 +++++++++++++++---- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/apps/web/src/components/files/FilePreviewPanel.tsx b/apps/web/src/components/files/FilePreviewPanel.tsx index acdc6c0cbb4e..43087d0295bb 100644 --- a/apps/web/src/components/files/FilePreviewPanel.tsx +++ b/apps/web/src/components/files/FilePreviewPanel.tsx @@ -60,6 +60,7 @@ import { fileBreadcrumbs } from "./filePath"; import { isMarkdownPreviewFile, setMarkdownTaskChecked } from "./filePreviewMode"; import { FileSaveCoordinator } from "./fileSaveCoordinator"; import { + clearProjectFileQueryData, confirmProjectFileQueryData, getOptimisticProjectFileQueryData, setProjectFileQueryData, @@ -813,6 +814,20 @@ export default function FilePreviewPanel({ ); const breadcrumbRef = useRef(null); const discardActiveFileSavesRef = useRef<(() => void) | null>(null); + // The delete and rename callbacks below can fire after a thread switch, when + // discardActiveFileSavesRef already points at the new thread's editor. This + // ref tracks what that editor shows now, so a stale callback cannot discard + // an unrelated file's pending edits. + const activeEditorFileRef = useRef({ environmentId, cwd, relativePath }); + useEffect(() => { + activeEditorFileRef.current = { environmentId, cwd, relativePath }; + }); + const editorShowsFile = (path: string) => { + const active = activeEditorFileRef.current; + return ( + active.environmentId === environmentId && active.cwd === cwd && active.relativePath === path + ); + }; const isMarkdown = relativePath ? isMarkdownPreviewFile(relativePath) : false; // A reveal still wins over the preference: the line only exists in the source. const renderMarkdown = @@ -1100,18 +1115,25 @@ export default function FilePreviewPanel({ onOpenFile={onOpenFile} {...(relativePath && !isImage ? { onRefreshSelectedFile: file.refresh } : {})} onEntryDeleted={(path) => { - if (path === relativePath) discardActiveFileSavesRef.current?.(); + if (editorShowsFile(path)) discardActiveFileSavesRef.current?.(); + clearProjectFileQueryData(environmentId, cwd, path); useRightPanelStore.getState().closeSurface(threadRef, `file:${path}`); }} onEntryRenamed={(from, to) => { - if (from === relativePath) discardActiveFileSavesRef.current?.(); + if (editorShowsFile(from)) discardActiveFileSavesRef.current?.(); + clearProjectFileQueryData(environmentId, cwd, from); const store = useRightPanelStore.getState(); - const wasOpen = selectThreadRightPanelState( - store.byThreadKey, - threadRef, - ).surfaces.some((surface) => surface.id === `file:${from}`); + const panel = selectThreadRightPanelState(store.byThreadKey, threadRef); + const wasOpen = panel.surfaces.some((surface) => surface.id === `file:${from}`); + const previousActiveId = panel.activeSurfaceId; store.closeSurface(threadRef, `file:${from}`); - if (wasOpen) store.openFile(threadRef, to); + if (!wasOpen) return; + store.openFile(threadRef, to); + // Renaming a background tab must not steal focus from the + // file the user is looking at. + if (previousActiveId !== null && previousActiveId !== `file:${from}`) { + store.activateSurface(threadRef, previousActiveId); + } }} /> From c5263de0dd1d4dfc68f2be165405dc71a2f8e4f5 Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 11:37:30 +0800 Subject: [PATCH 35/54] fix(contracts,server): typed escapes-root stage for rename and delete The symlink escape checks passed a formatted string as the error cause. Both stage unions gain an escapes-root literal with the message built in the contract, so the checks construct without a cause and clients get the same typed stage as every other failure. --- .../server/src/workspace/WorkspaceFileSystem.test.ts | 2 +- apps/server/src/workspace/WorkspaceFileSystem.ts | 10 ++-------- packages/contracts/src/project.ts | 12 +++++++++++- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/apps/server/src/workspace/WorkspaceFileSystem.test.ts b/apps/server/src/workspace/WorkspaceFileSystem.test.ts index bd16dd210653..05562d50a7b9 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.test.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.test.ts @@ -492,7 +492,7 @@ it.layer(TestLayer, { excludeTestServices: true })("WorkspaceFileSystemLive", (i .pipe(Effect.flip); expect(error).toBeInstanceOf(ProjectRenameEntryError); - expect(error).toMatchObject({ stage: "resolve-path" }); + expect(error).toMatchObject({ stage: "escapes-root" }); const untouched = yield* fileSystem .readFileString(path.join(outside, "owned.txt")) .pipe(Effect.orDie); diff --git a/apps/server/src/workspace/WorkspaceFileSystem.ts b/apps/server/src/workspace/WorkspaceFileSystem.ts index 28c678994c1f..41b1253f5c3b 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.ts @@ -401,10 +401,7 @@ export const make = Effect.gen(function* () { path.dirname(source.absolutePath), ).pipe(Effect.mapError((cause) => renameError("resolve-path", cause))); if (escapes) { - return yield* renameError( - "resolve-path", - `'${source.relativePath}' resolves outside the project.`, - ); + return yield* renameError("escapes-root"); } // Renaming a file onto its own exact path is a no-op. Without this guard @@ -524,10 +521,7 @@ export const make = Effect.gen(function* () { return; } if (escapes) { - return yield* deleteError( - "resolve-path", - `'${target.relativePath}' resolves outside the project.`, - ); + return yield* deleteError("escapes-root"); } const targetStat = yield* fileSystem.stat(target.absolutePath).pipe( diff --git a/packages/contracts/src/project.ts b/packages/contracts/src/project.ts index 3389285c4ab3..ff87509edeb9 100644 --- a/packages/contracts/src/project.ts +++ b/packages/contracts/src/project.ts @@ -408,6 +408,7 @@ export class ProjectRenameEntryTargetExistsError extends Schema.TaggedErrorClass export const ProjectRenameEntryStage = Schema.Literals([ "resolve-path", + "escapes-root", "not-a-file", "cross-directory", "rename", @@ -425,6 +426,8 @@ function projectRenameEntryStageMessage(props: ProjectRenameEntryFailureContext) switch (props.stage) { case "resolve-path": return `Failed to resolve '${props.relativePath}' within '${props.cwd}'.`; + case "escapes-root": + return `'${props.relativePath}' resolves outside the project.`; case "not-a-file": return `'${props.relativePath}' is not a file.`; case "cross-directory": @@ -458,7 +461,12 @@ export const ProjectDeleteEntryInput = Schema.Struct({ }); export type ProjectDeleteEntryInput = typeof ProjectDeleteEntryInput.Type; -export const ProjectDeleteEntryStage = Schema.Literals(["resolve-path", "not-a-file", "remove"]); +export const ProjectDeleteEntryStage = Schema.Literals([ + "resolve-path", + "escapes-root", + "not-a-file", + "remove", +]); export type ProjectDeleteEntryStage = typeof ProjectDeleteEntryStage.Type; type ProjectDeleteEntryFailureContext = { @@ -472,6 +480,8 @@ function projectDeleteEntryStageMessage(props: ProjectDeleteEntryFailureContext) switch (props.stage) { case "resolve-path": return `Failed to resolve '${props.relativePath}' within '${props.cwd}'.`; + case "escapes-root": + return `'${props.relativePath}' resolves outside the project.`; case "not-a-file": return `'${props.relativePath}' is not a file; only files can be deleted from the files view.`; case "remove": From a08d457dda715acb86a7337d969901fa8bb14d5b Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 11:37:44 +0800 Subject: [PATCH 36/54] refactor(contracts,web): derive the rename target-exists check from the schema RenameEntryDialog duck-typed the failure by _tag. The contracts package now exports a Schema.is predicate for ProjectRenameEntryTargetExistsError and the dialog uses it. --- apps/web/src/components/files/RenameEntryDialog.tsx | 13 ++----------- packages/contracts/src/project.ts | 2 ++ 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/apps/web/src/components/files/RenameEntryDialog.tsx b/apps/web/src/components/files/RenameEntryDialog.tsx index 6ffe399a8a7b..bf19b0f7a4a5 100644 --- a/apps/web/src/components/files/RenameEntryDialog.tsx +++ b/apps/web/src/components/files/RenameEntryDialog.tsx @@ -1,4 +1,4 @@ -import type { EnvironmentId } from "@t3tools/contracts"; +import { isProjectRenameEntryTargetExistsError, type EnvironmentId } from "@t3tools/contracts"; import { runAtomCommand } from "@t3tools/client-runtime/state/runtime"; import * as Cause from "effect/Cause"; import { useEffect, useId, useRef, useState } from "react"; @@ -17,15 +17,6 @@ import { Input } from "~/components/ui/input"; import { appAtomRegistry } from "~/rpc/atomRegistry"; import { projectEnvironment } from "~/state/projects"; -function isTargetExistsFailure(cause: unknown): boolean { - return ( - typeof cause === "object" && - cause !== null && - "_tag" in cause && - cause._tag === "ProjectRenameEntryTargetExistsError" - ); -} - export function RenameEntryDialog({ environmentId, cwd, @@ -94,7 +85,7 @@ export function RenameEntryDialog({ return; } setRenameError( - isTargetExistsFailure(Cause.squash(result.cause)) + isProjectRenameEntryTargetExistsError(Cause.squash(result.cause)) ? "A file with that name already exists." : "Rename failed. Try again.", ); diff --git a/packages/contracts/src/project.ts b/packages/contracts/src/project.ts index ff87509edeb9..5964ed0856ed 100644 --- a/packages/contracts/src/project.ts +++ b/packages/contracts/src/project.ts @@ -406,6 +406,8 @@ export class ProjectRenameEntryTargetExistsError extends Schema.TaggedErrorClass } } +export const isProjectRenameEntryTargetExistsError = Schema.is(ProjectRenameEntryTargetExistsError); + export const ProjectRenameEntryStage = Schema.Literals([ "resolve-path", "escapes-root", From aae757805cd4b473a81c9a3e8e86a424deb32a9f Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 12:00:08 +0800 Subject: [PATCH 37/54] fix(server): renaming onto a hard link reports target exists A case-only rename lists a single directory entry, so both names listed means the target name is genuinely occupied by another link to the same inode. Removing the source silently succeeded and dropped a real entry; report ProjectRenameEntryTargetExistsError like any other conflict. --- .../src/workspace/WorkspaceFileSystem.test.ts | 27 +++++++++++-------- .../src/workspace/WorkspaceFileSystem.ts | 25 ++++++++--------- 2 files changed, 27 insertions(+), 25 deletions(-) diff --git a/apps/server/src/workspace/WorkspaceFileSystem.test.ts b/apps/server/src/workspace/WorkspaceFileSystem.test.ts index 05562d50a7b9..70c7c0ea6bce 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.test.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.test.ts @@ -371,7 +371,9 @@ it.layer(TestLayer, { excludeTestServices: true })("WorkspaceFileSystemLive", (i // A hard-linked target reaches the same-inode path a case-only rename // takes on a case-insensitive filesystem, deterministically on Linux. - it.effect("renames onto another name of the same file without destroying it", () => + // Both names are genuine directory entries, so the target counts as + // occupied; only a true case change lists a single name. + it.effect("rejects renaming onto a hard link of the same file", () => Effect.gen(function* () { const workspaceFileSystem = yield* WorkspaceFileSystem.WorkspaceFileSystem; const fileSystem = yield* FileSystem.FileSystem; @@ -382,19 +384,22 @@ it.layer(TestLayer, { excludeTestServices: true })("WorkspaceFileSystemLive", (i .link(path.join(cwd, "src/notes.md"), path.join(cwd, "src/Notes.md")) .pipe(Effect.orDie); - const result = yield* workspaceFileSystem.renameEntry({ - cwd, - relativePath: "src/notes.md", - newRelativePath: "src/Notes.md", - }); + const error = yield* workspaceFileSystem + .renameEntry({ + cwd, + relativePath: "src/notes.md", + newRelativePath: "src/Notes.md", + }) + .pipe(Effect.flip); - expect(result).toEqual({ relativePath: "src/Notes.md" }); - const renamed = yield* fileSystem + expect(error).toBeInstanceOf(ProjectRenameEntryTargetExistsError); + expect(error).toMatchObject({ cwd, relativePath: "src/Notes.md" }); + const sourceExists = yield* fileSystem.exists(path.join(cwd, "src/notes.md")); + expect(sourceExists).toBe(true); + const targetContents = yield* fileSystem .readFileString(path.join(cwd, "src/Notes.md")) .pipe(Effect.orDie); - expect(renamed).toBe("# Notes\n"); - const sourceExists = yield* fileSystem.exists(path.join(cwd, "src/notes.md")); - expect(sourceExists).toBe(false); + expect(targetContents).toBe("# Notes\n"); }), ); diff --git a/apps/server/src/workspace/WorkspaceFileSystem.ts b/apps/server/src/workspace/WorkspaceFileSystem.ts index 41b1253f5c3b..0c922530b0ce 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.ts @@ -435,26 +435,23 @@ export const make = Effect.gen(function* () { } // The conflicting target is another name of the source's own inode. // The directory listing reports exact on-disk names and tells the - // two shapes apart. Both names listed is a pre-existing hard link - // pair, where POSIX rename over another name of the same file is a - // no-op, so removing the source entry completes the rename. Only - // the source listed is the source under another casing on a - // case-insensitive filesystem, where rename applies the case - // change. Otherwise the source entry is gone or already carries - // the target casing, and the target name holds the data. + // two shapes apart. A case change only ever lists one of the names, + // so both names listed is a pre-existing hard link pair, where the + // target name is occupied like any other conflict. Only the source + // listed is the source under another casing on a case-insensitive + // filesystem, where rename applies the case change. Otherwise the + // source entry is gone or already carries the target casing, and + // the target name holds the data. const siblingNames = yield* fileSystem .readDirectory(path.dirname(source.absolutePath)) .pipe(Effect.mapError((cause) => renameError("rename", cause))); const sourceListed = siblingNames.includes(path.basename(source.absolutePath)); const targetListed = siblingNames.includes(path.basename(target.absolutePath)); if (sourceListed && targetListed) { - return yield* fileSystem.remove(source.absolutePath).pipe( - Effect.catchIf( - (error) => error.reason._tag === "NotFound", - () => Effect.void, - ), - Effect.mapError((cause) => renameError("rename", cause)), - ); + return yield* new ProjectRenameEntryTargetExistsError({ + cwd: input.cwd, + relativePath: target.relativePath, + }); } if (sourceListed) { return yield* fileSystem From 3c1a9f4e91b45fdf109059f79795f8439fce4567 Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 12:00:09 +0800 Subject: [PATCH 38/54] fix(web): drop pending editor saves before rename or delete runs Saves, renames, and deletes share one serial per-path command queue, so a debounced save that fires while the mutation is in flight enqueues behind it and recreates the file after a delete or at the old path after a rename. Discard the editor's pending saves when the mutation starts instead of after it succeeds, closing the queue-ordering window. The post-success discard stays as a net for a mid-flight file switch. --- apps/web/src/components/files/FileBrowserPanel.tsx | 10 ++++++++++ apps/web/src/components/files/FilePreviewPanel.tsx | 3 +++ apps/web/src/components/files/RenameEntryDialog.tsx | 4 ++++ 3 files changed, 17 insertions(+) diff --git a/apps/web/src/components/files/FileBrowserPanel.tsx b/apps/web/src/components/files/FileBrowserPanel.tsx index 58e55da28738..d2be88027c11 100644 --- a/apps/web/src/components/files/FileBrowserPanel.tsx +++ b/apps/web/src/components/files/FileBrowserPanel.tsx @@ -50,6 +50,13 @@ interface FileBrowserPanelProps { selectedPathRevealId: number; onOpenFile: (relativePath: string) => void; onRefreshSelectedFile?: () => void; + /** + * A rename or delete is about to run. Saves and mutations share one serial + * per-path command queue, so a save enqueued during the mutation would land + * after it and recreate the file; pending saves for the path must be + * dropped before the mutation enqueues. + */ + onEntryMutationStart?: (relativePath: string) => void; /** A rename succeeded; open surfaces for the old path should follow the file. */ onEntryRenamed?: (relativePath: string, newRelativePath: string) => void; /** A delete succeeded; open surfaces for the path should close. */ @@ -220,6 +227,7 @@ export default function FileBrowserPanel({ selectedPathRevealId, onOpenFile, onRefreshSelectedFile, + onEntryMutationStart, onEntryRenamed, onEntryDeleted, }: FileBrowserPanelProps) { @@ -320,6 +328,7 @@ export default function FileBrowserPanel({ { variant: "destructive" }, ); if (confirmed !== true) return; + onEntryMutationStart?.(relativePath); const result = await runAtomCommand( appAtomRegistry, projectEnvironment.deleteEntry, @@ -638,6 +647,7 @@ export default function FileBrowserPanel({ cwd={cwd} relativePath={renameTarget} onClose={() => setRenameTarget(null)} + onRenameStart={() => onEntryMutationStart?.(renameTarget)} onRenamed={(newRelativePath) => { entriesQuery.refresh(); onEntryRenamed?.(renameTarget, newRelativePath); diff --git a/apps/web/src/components/files/FilePreviewPanel.tsx b/apps/web/src/components/files/FilePreviewPanel.tsx index 43087d0295bb..5c0ae4c462a2 100644 --- a/apps/web/src/components/files/FilePreviewPanel.tsx +++ b/apps/web/src/components/files/FilePreviewPanel.tsx @@ -1114,6 +1114,9 @@ export default function FilePreviewPanel({ selectedPathRevealId={revealRequestId} onOpenFile={onOpenFile} {...(relativePath && !isImage ? { onRefreshSelectedFile: file.refresh } : {})} + onEntryMutationStart={(path) => { + if (editorShowsFile(path)) discardActiveFileSavesRef.current?.(); + }} onEntryDeleted={(path) => { if (editorShowsFile(path)) discardActiveFileSavesRef.current?.(); clearProjectFileQueryData(environmentId, cwd, path); diff --git a/apps/web/src/components/files/RenameEntryDialog.tsx b/apps/web/src/components/files/RenameEntryDialog.tsx index bf19b0f7a4a5..0feadcf98d5d 100644 --- a/apps/web/src/components/files/RenameEntryDialog.tsx +++ b/apps/web/src/components/files/RenameEntryDialog.tsx @@ -22,12 +22,15 @@ export function RenameEntryDialog({ cwd, relativePath, onClose, + onRenameStart, onRenamed, }: { readonly environmentId: EnvironmentId; readonly cwd: string; readonly relativePath: string; readonly onClose: () => void; + /** The rename is about to run; pending saves for the path must not enqueue behind it. */ + readonly onRenameStart?: () => void; readonly onRenamed: (newRelativePath: string) => void; }) { const lastSlash = relativePath.lastIndexOf("/"); @@ -68,6 +71,7 @@ export function RenameEntryDialog({ setIsRenaming(true); setRenameError(null); const newRelativePath = `${directoryPrefix}${candidate}`; + onRenameStart?.(); const result = await runAtomCommand( appAtomRegistry, projectEnvironment.renameEntry, From 740090b7b38a3819496b98ca0a39bff2c53e03a0 Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 12:26:12 +0800 Subject: [PATCH 39/54] fix(server): rename files on volumes that reject hard links --- .../src/workspace/WorkspaceFileSystem.test.ts | 99 +++++++++++++++++++ .../src/workspace/WorkspaceFileSystem.ts | 45 ++++++++- 2 files changed, 139 insertions(+), 5 deletions(-) diff --git a/apps/server/src/workspace/WorkspaceFileSystem.test.ts b/apps/server/src/workspace/WorkspaceFileSystem.test.ts index 70c7c0ea6bce..e5737302ace8 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.test.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.test.ts @@ -9,6 +9,7 @@ import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import * as Path from "effect/Path"; +import * as PlatformError from "effect/PlatformError"; import * as ServerConfig from "../config.ts"; import * as VcsDriverRegistry from "../vcs/VcsDriverRegistry.ts"; @@ -554,3 +555,101 @@ it.layer(TestLayer, { excludeTestServices: true })("WorkspaceFileSystemLive", (i ); }); }); + +const linkRejections: Array = []; +const linklessFileSystemLayer = Layer.effect( + FileSystem.FileSystem, + Effect.gen(function* () { + const real = yield* FileSystem.FileSystem; + return FileSystem.FileSystem.of({ + ...real, + link: (fromPath, toPath) => { + linkRejections.push(toPath); + return Effect.fail( + PlatformError.systemError({ + _tag: "PermissionDenied", + module: "FileSystem", + method: "link", + syscall: "link", + pathOrDescriptor: toPath, + description: "EPERM: the volume rejects hard links", + }), + ); + }, + }); + }), +); + +const LinklessTestLayer = Layer.empty.pipe( + Layer.provideMerge(ProjectLayer), + Layer.provideMerge(WorkspaceEntries.layer.pipe(Layer.provide(WorkspacePaths.layer))), + Layer.provideMerge(WorkspacePaths.layer), + Layer.provideMerge(VcsDriverRegistry.layer.pipe(Layer.provide(VcsProcess.layer))), + Layer.provide( + ServerConfig.ServerConfig.layerTest(process.cwd(), { + prefix: "t3-workspace-files-test-", + }), + ), + Layer.provideMerge(linklessFileSystemLayer), + Layer.provideMerge(NodeServices.layer), +); + +it.layer(LinklessTestLayer, { excludeTestServices: true })( + "WorkspaceFileSystemLive without hard links", + (it) => { + it.effect("renames a file when the volume rejects hard links", () => + 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/notes.md", "# Notes\n"); + + const result = yield* workspaceFileSystem.renameEntry({ + cwd, + relativePath: "src/notes.md", + newRelativePath: "src/journal.md", + }); + + expect(result).toEqual({ relativePath: "src/journal.md" }); + expect(linkRejections.length).toBeGreaterThan(0); + const renamed = yield* fileSystem + .readFileString(path.join(cwd, "src/journal.md")) + .pipe(Effect.orDie); + expect(renamed).toBe("# Notes\n"); + const sourceExists = yield* fileSystem.exists(path.join(cwd, "src/notes.md")); + expect(sourceExists).toBe(false); + }), + ); + + it.effect("still rejects renaming onto an existing entry without hard links", () => + 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/source.md", "source\n"); + yield* writeTextFile(cwd, "src/taken.md", "taken\n"); + + const error = yield* workspaceFileSystem + .renameEntry({ + cwd, + relativePath: "src/source.md", + newRelativePath: "src/taken.md", + }) + .pipe(Effect.flip); + + expect(error).toBeInstanceOf(ProjectRenameEntryTargetExistsError); + expect(error).toMatchObject({ cwd, relativePath: "src/taken.md" }); + const source = yield* fileSystem + .readFileString(path.join(cwd, "src/source.md")) + .pipe(Effect.orDie); + expect(source).toBe("source\n"); + const taken = yield* fileSystem + .readFileString(path.join(cwd, "src/taken.md")) + .pipe(Effect.orDie); + expect(taken).toBe("taken\n"); + }), + ); + }, +); diff --git a/apps/server/src/workspace/WorkspaceFileSystem.ts b/apps/server/src/workspace/WorkspaceFileSystem.ts index 0c922530b0ce..b06ca435cf80 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.ts @@ -417,15 +417,50 @@ export const make = Effect.gen(function* () { // strand both names on disk, so the pair runs uninterruptibly. yield* Effect.uninterruptible( Effect.gen(function* () { - const conflict = yield* fileSystem.link(source.absolutePath, target.absolutePath).pipe( - Effect.as(false), + const claim = yield* fileSystem.link(source.absolutePath, target.absolutePath).pipe( + Effect.as("linked" as const), Effect.catchIf( (error) => error.reason._tag === "AlreadyExists", - () => Effect.succeed(true), + () => Effect.succeed("conflict" as const), ), - Effect.mapError((cause) => renameError("rename", cause)), + // FAT and exFAT volumes reject hard links; a real failure of any + // other kind resurfaces from the fallback rename below. + Effect.catch(() => Effect.succeed("unsupported" as const)), ); - if (conflict) { + if (claim === "unsupported") { + // Without hard links there is no atomic no-clobber primitive, so an + // exact-name listing plus the volume's own name resolution guard the + // target before a plain rename. exists finding the target while the + // basenames differ case-insensitively means another entry owns the + // name under different casing on a case-insensitive volume; equal + // basenames mean the hit is the source itself and the rename is a + // case change. + const siblingNames = yield* fileSystem + .readDirectory(path.dirname(source.absolutePath)) + .pipe(Effect.mapError((cause) => renameError("rename", cause))); + if (siblingNames.includes(path.basename(target.absolutePath))) { + return yield* new ProjectRenameEntryTargetExistsError({ + cwd: input.cwd, + relativePath: target.relativePath, + }); + } + const targetOccupied = yield* fileSystem + .exists(target.absolutePath) + .pipe(Effect.mapError((cause) => renameError("rename", cause))); + const caseChangeOnly = + path.basename(source.absolutePath).toLowerCase() === + path.basename(target.absolutePath).toLowerCase(); + if (targetOccupied && !caseChangeOnly) { + return yield* new ProjectRenameEntryTargetExistsError({ + cwd: input.cwd, + relativePath: target.relativePath, + }); + } + return yield* fileSystem + .rename(source.absolutePath, target.absolutePath) + .pipe(Effect.mapError((cause) => renameError("rename", cause))); + } + if (claim === "conflict") { const sameFile = yield* isSameFile(source.absolutePath, target.absolutePath); if (!sameFile) { return yield* new ProjectRenameEntryTargetExistsError({ From e6cc66f974e025b92f32953c8b2201b5c167a43a Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 12:54:21 +0800 Subject: [PATCH 40/54] fix(web): failed rename or delete no longer drops pending edits Rename and delete discarded pending edits up front, so a failed mutation silently lost whatever the editor still showed. The coordinator now suspends saves for the mutation and the outcome decides what follows: success discards, failure resumes and persists the held edits. Dispose while suspended skips the flush so a save cannot land behind the mutation on the shared serial queue. --- .../src/components/files/FileBrowserPanel.tsx | 7 +- .../src/components/files/FilePreviewPanel.tsx | 30 ++++++-- .../components/files/RenameEntryDialog.tsx | 4 ++ .../files/fileSaveCoordinator.test.ts | 68 +++++++++++++++++++ .../components/files/fileSaveCoordinator.ts | 21 +++++- 5 files changed, 121 insertions(+), 9 deletions(-) diff --git a/apps/web/src/components/files/FileBrowserPanel.tsx b/apps/web/src/components/files/FileBrowserPanel.tsx index d2be88027c11..05735a24d841 100644 --- a/apps/web/src/components/files/FileBrowserPanel.tsx +++ b/apps/web/src/components/files/FileBrowserPanel.tsx @@ -54,9 +54,11 @@ interface FileBrowserPanelProps { * A rename or delete is about to run. Saves and mutations share one serial * per-path command queue, so a save enqueued during the mutation would land * after it and recreate the file; pending saves for the path must be - * dropped before the mutation enqueues. + * held before the mutation enqueues. */ onEntryMutationStart?: (relativePath: string) => void; + /** The mutation failed and the file is still in place; held saves may run again. */ + onEntryMutationFailed?: (relativePath: string) => void; /** A rename succeeded; open surfaces for the old path should follow the file. */ onEntryRenamed?: (relativePath: string, newRelativePath: string) => void; /** A delete succeeded; open surfaces for the path should close. */ @@ -228,6 +230,7 @@ export default function FileBrowserPanel({ onOpenFile, onRefreshSelectedFile, onEntryMutationStart, + onEntryMutationFailed, onEntryRenamed, onEntryDeleted, }: FileBrowserPanelProps) { @@ -340,6 +343,7 @@ export default function FileBrowserPanel({ onEntryDeleted?.(relativePath); return; } + onEntryMutationFailed?.(relativePath); const failure = Cause.squash(result.cause); toastManager.add({ type: "error", @@ -648,6 +652,7 @@ export default function FileBrowserPanel({ relativePath={renameTarget} onClose={() => setRenameTarget(null)} onRenameStart={() => onEntryMutationStart?.(renameTarget)} + onRenameFailed={() => onEntryMutationFailed?.(renameTarget)} onRenamed={(newRelativePath) => { entriesQuery.refresh(); onEntryRenamed?.(renameTarget, newRelativePath); diff --git a/apps/web/src/components/files/FilePreviewPanel.tsx b/apps/web/src/components/files/FilePreviewPanel.tsx index 5c0ae4c462a2..9043976e5245 100644 --- a/apps/web/src/components/files/FilePreviewPanel.tsx +++ b/apps/web/src/components/files/FilePreviewPanel.tsx @@ -395,8 +395,17 @@ interface EditableFileSurfaceProps { wordWrap: boolean; onPostRender: FilePostRender; onPendingChange: (relativePath: string, pending: boolean) => void; - /** Receives a callback that drops unsaved edits; used when the file is deleted or renamed away. */ - discardSavesRef?: { current: (() => void) | null } | undefined; + /** + * Receives the active editor's save controls: suspend before a rename or + * delete, then discard on success or resume on failure. + */ + discardSavesRef?: { current: FileSaveControls | null } | undefined; +} + +interface FileSaveControls { + suspend: () => void; + resume: () => void; + discard: () => void; } interface FileSelectionOverride { @@ -435,7 +444,11 @@ function useFileSaveCoordinator({ useEffect(() => () => coordinator.dispose(), [coordinator]); useEffect(() => { if (!discardSavesRef) return; - discardSavesRef.current = () => coordinator.discard(); + discardSavesRef.current = { + suspend: () => coordinator.suspend(), + resume: () => coordinator.resume(), + discard: () => coordinator.discard(), + }; return () => { discardSavesRef.current = null; }; @@ -813,7 +826,7 @@ export default function FilePreviewPanel({ null, ); const breadcrumbRef = useRef(null); - const discardActiveFileSavesRef = useRef<(() => void) | null>(null); + const discardActiveFileSavesRef = useRef(null); // The delete and rename callbacks below can fire after a thread switch, when // discardActiveFileSavesRef already points at the new thread's editor. This // ref tracks what that editor shows now, so a stale callback cannot discard @@ -1115,15 +1128,18 @@ export default function FilePreviewPanel({ onOpenFile={onOpenFile} {...(relativePath && !isImage ? { onRefreshSelectedFile: file.refresh } : {})} onEntryMutationStart={(path) => { - if (editorShowsFile(path)) discardActiveFileSavesRef.current?.(); + if (editorShowsFile(path)) discardActiveFileSavesRef.current?.suspend(); + }} + onEntryMutationFailed={(path) => { + if (editorShowsFile(path)) discardActiveFileSavesRef.current?.resume(); }} onEntryDeleted={(path) => { - if (editorShowsFile(path)) discardActiveFileSavesRef.current?.(); + if (editorShowsFile(path)) discardActiveFileSavesRef.current?.discard(); clearProjectFileQueryData(environmentId, cwd, path); useRightPanelStore.getState().closeSurface(threadRef, `file:${path}`); }} onEntryRenamed={(from, to) => { - if (editorShowsFile(from)) discardActiveFileSavesRef.current?.(); + if (editorShowsFile(from)) discardActiveFileSavesRef.current?.discard(); clearProjectFileQueryData(environmentId, cwd, from); const store = useRightPanelStore.getState(); const panel = selectThreadRightPanelState(store.byThreadKey, threadRef); diff --git a/apps/web/src/components/files/RenameEntryDialog.tsx b/apps/web/src/components/files/RenameEntryDialog.tsx index 0feadcf98d5d..94af03219b37 100644 --- a/apps/web/src/components/files/RenameEntryDialog.tsx +++ b/apps/web/src/components/files/RenameEntryDialog.tsx @@ -23,6 +23,7 @@ export function RenameEntryDialog({ relativePath, onClose, onRenameStart, + onRenameFailed, onRenamed, }: { readonly environmentId: EnvironmentId; @@ -31,6 +32,8 @@ export function RenameEntryDialog({ readonly onClose: () => void; /** The rename is about to run; pending saves for the path must not enqueue behind it. */ readonly onRenameStart?: () => void; + /** The rename failed and the file kept its name; held saves may run again. */ + readonly onRenameFailed?: () => void; readonly onRenamed: (newRelativePath: string) => void; }) { const lastSlash = relativePath.lastIndexOf("/"); @@ -88,6 +91,7 @@ export function RenameEntryDialog({ onClose(); return; } + onRenameFailed?.(); setRenameError( isProjectRenameEntryTargetExistsError(Cause.squash(result.cause)) ? "A file with that name already exists." diff --git a/apps/web/src/components/files/fileSaveCoordinator.test.ts b/apps/web/src/components/files/fileSaveCoordinator.test.ts index ce000c9d7d44..8d6c3288b162 100644 --- a/apps/web/src/components/files/fileSaveCoordinator.test.ts +++ b/apps/web/src/components/files/fileSaveCoordinator.test.ts @@ -113,4 +113,72 @@ describe("FileSaveCoordinator", () => { expect(persist).not.toHaveBeenCalled(); expect(onPendingChange.mock.calls.at(-1)).toEqual([false]); }); + + it("suspend holds saves and resume persists the held edits", async () => { + vi.useFakeTimers(); + const persist = vi + .fn<(contents: string) => Promise>>() + .mockResolvedValue(AsyncResult.success(undefined)); + const onPendingChange = vi.fn(); + const coordinator = new FileSaveCoordinator({ + debounceMs: 500, + persist, + onPendingChange, + onConfirmed: vi.fn(), + }); + + coordinator.change("held"); + coordinator.suspend(); + await vi.advanceTimersByTimeAsync(5_000); + expect(persist).not.toHaveBeenCalled(); + + coordinator.resume(); + await vi.runAllTimersAsync(); + expect(persist).toHaveBeenCalledOnce(); + expect(persist).toHaveBeenCalledWith("held"); + expect(onPendingChange.mock.calls.at(-1)).toEqual([false]); + }); + + it("suspend then discard drops the held edits", async () => { + vi.useFakeTimers(); + const persist = vi + .fn<(contents: string) => Promise>>() + .mockResolvedValue(AsyncResult.success(undefined)); + const onPendingChange = vi.fn(); + const coordinator = new FileSaveCoordinator({ + debounceMs: 500, + persist, + onPendingChange, + onConfirmed: vi.fn(), + }); + + coordinator.change("doomed"); + coordinator.suspend(); + coordinator.discard(); + coordinator.dispose(); + await vi.runAllTimersAsync(); + + expect(persist).not.toHaveBeenCalled(); + expect(onPendingChange.mock.calls.at(-1)).toEqual([false]); + }); + + it("dispose while suspended does not flush behind a pending mutation", async () => { + vi.useFakeTimers(); + const persist = vi + .fn<(contents: string) => Promise>>() + .mockResolvedValue(AsyncResult.success(undefined)); + const coordinator = new FileSaveCoordinator({ + debounceMs: 500, + persist, + onPendingChange: vi.fn(), + onConfirmed: vi.fn(), + }); + + coordinator.change("unflushed"); + coordinator.suspend(); + coordinator.dispose(); + await vi.runAllTimersAsync(); + + expect(persist).not.toHaveBeenCalled(); + }); }); diff --git a/apps/web/src/components/files/fileSaveCoordinator.ts b/apps/web/src/components/files/fileSaveCoordinator.ts index 9f6db61f6d4e..c87ef66f91a7 100644 --- a/apps/web/src/components/files/fileSaveCoordinator.ts +++ b/apps/web/src/components/files/fileSaveCoordinator.ts @@ -14,6 +14,7 @@ export class FileSaveCoordinator { private lastChangeAt = 0; private saving = false; private disposed = false; + private suspended = false; constructor(private readonly options: FileSaveCoordinatorOptions) {} @@ -34,11 +35,29 @@ export class FileSaveCoordinator { /** Drop unsaved edits without persisting; for files removed out from under the surface. */ discard(): void { this.disposed = true; + this.suspended = false; this.clearTimer(); this.latestRevision = 0; this.options.onPendingChange(false); } + /** + * Hold pending edits while a rename or delete runs, so a save cannot land + * mid-mutation. The mutation's outcome decides what follows: discard() on + * success, resume() on failure. + */ + suspend(): void { + this.suspended = true; + this.clearTimer(); + } + + /** Reinstate saving after a failed rename or delete left the file in place. */ + resume(): void { + if (!this.suspended) return; + this.suspended = false; + if (this.latestRevision > 0) this.schedule(0); + } + private schedule(delay: number): void { this.clearTimer(); this.timer = setTimeout(() => { @@ -54,7 +73,7 @@ export class FileSaveCoordinator { } private async persistLatest(): Promise { - if (this.saving || this.latestRevision === 0) return; + if (this.suspended || this.saving || this.latestRevision === 0) return; this.saving = true; const contents = this.latestContents; From 9a543284cadd4444c126709656f05ef857807a48 Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 12:54:22 +0800 Subject: [PATCH 41/54] fix(server): close rename fallback races and delete symlinks by entry The linkless rename fallback checked the target with a listing and exists(), leaving a window where a rival file created between check and rename was replaced, and a dangling symlink under other casing escaped both checks. An empty O_EXCL create now claims the target name, so the rename only ever replaces this rename's own claim; when the name is held by the source's own inode the rename is a case change and needs no claim. deleteEntry now lstats the entry, so a dangling symlink is removed instead of reading as already gone, and anything that is not a regular file or a symlink is refused, matching renameEntry's file-only contract. --- .../src/workspace/WorkspaceFileSystem.test.ts | 145 ++++++++++++++++++ .../src/workspace/WorkspaceFileSystem.ts | 82 ++++++---- 2 files changed, 194 insertions(+), 33 deletions(-) diff --git a/apps/server/src/workspace/WorkspaceFileSystem.test.ts b/apps/server/src/workspace/WorkspaceFileSystem.test.ts index e5737302ace8..b078e7d416c8 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.test.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.test.ts @@ -534,6 +534,47 @@ it.layer(TestLayer, { excludeTestServices: true })("WorkspaceFileSystemLive", (i }), ); + // stat follows the link and reads NotFound, so without lstat the delete + // would report success while the entry stays on disk. + it.effect("removes a dangling symlink instead of reporting it already gone", () => + Effect.gen(function* () { + const workspaceFileSystem = yield* WorkspaceFileSystem.WorkspaceFileSystem; + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const cwd = yield* makeTempDir; + yield* fileSystem.makeDirectory(path.join(cwd, "src")); + yield* fileSystem.symlink( + path.join(cwd, "src/missing.md"), + path.join(cwd, "src/broken.md"), + ); + + yield* workspaceFileSystem.deleteEntry({ cwd, relativePath: "src/broken.md" }); + + const names = yield* fileSystem.readDirectory(path.join(cwd, "src")); + expect(names).not.toContain("broken.md"); + }), + ); + + it.effect("deletes a symlink without touching its target", () => + 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/notes.md", "# Notes\n"); + yield* fileSystem.symlink(path.join(cwd, "src/notes.md"), path.join(cwd, "src/alias.md")); + + yield* workspaceFileSystem.deleteEntry({ cwd, relativePath: "src/alias.md" }); + + const aliasExists = yield* fileSystem.exists(path.join(cwd, "src/alias.md")); + expect(aliasExists).toBe(false); + const contents = yield* fileSystem + .readFileString(path.join(cwd, "src/notes.md")) + .pipe(Effect.orDie); + expect(contents).toBe("# Notes\n"); + }), + ); + it.effect("rejects deleting a directory", () => Effect.gen(function* () { const workspaceFileSystem = yield* WorkspaceFileSystem.WorkspaceFileSystem; @@ -651,5 +692,109 @@ it.layer(LinklessTestLayer, { excludeTestServices: true })( expect(taken).toBe("taken\n"); }), ); + + // exists() follows the link and reads false for a dangling one; the + // O_EXCL claim fails on the entry itself, so the symlink survives. + it.effect("rejects renaming onto a dangling symlink without hard links", () => + 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/source.md", "source\n"); + yield* fileSystem.symlink( + path.join(cwd, "src/missing.md"), + path.join(cwd, "src/broken.md"), + ); + + const error = yield* workspaceFileSystem + .renameEntry({ + cwd, + relativePath: "src/source.md", + newRelativePath: "src/broken.md", + }) + .pipe(Effect.flip); + + expect(error).toBeInstanceOf(ProjectRenameEntryTargetExistsError); + const names = yield* fileSystem.readDirectory(path.join(cwd, "src")); + expect(names).toContain("broken.md"); + expect(names).toContain("source.md"); + }), + ); + }, +); + +const brokenRenameFileSystemLayer = Layer.effect( + FileSystem.FileSystem, + Effect.gen(function* () { + const real = yield* FileSystem.FileSystem; + return FileSystem.FileSystem.of({ + ...real, + link: (_fromPath, toPath) => + Effect.fail( + PlatformError.systemError({ + _tag: "PermissionDenied", + module: "FileSystem", + method: "link", + syscall: "link", + pathOrDescriptor: toPath, + description: "EPERM: the volume rejects hard links", + }), + ), + rename: (_oldPath, newPath) => + Effect.fail( + PlatformError.systemError({ + _tag: "PermissionDenied", + module: "FileSystem", + method: "rename", + syscall: "rename", + pathOrDescriptor: newPath, + description: "EACCES: the volume failed the rename", + }), + ), + }); + }), +); + +const BrokenRenameTestLayer = Layer.empty.pipe( + Layer.provideMerge(ProjectLayer), + Layer.provideMerge(WorkspaceEntries.layer.pipe(Layer.provide(WorkspacePaths.layer))), + Layer.provideMerge(WorkspacePaths.layer), + Layer.provideMerge(VcsDriverRegistry.layer.pipe(Layer.provide(VcsProcess.layer))), + Layer.provide( + ServerConfig.ServerConfig.layerTest(process.cwd(), { + prefix: "t3-workspace-files-test-", + }), + ), + Layer.provideMerge(brokenRenameFileSystemLayer), + Layer.provideMerge(NodeServices.layer), +); + +it.layer(BrokenRenameTestLayer, { excludeTestServices: true })( + "WorkspaceFileSystemLive when the fallback rename fails", + (it) => { + it.effect("reclaims the target name so a retry does not read it as taken", () => + 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/source.md", "source\n"); + + const error = yield* workspaceFileSystem + .renameEntry({ + cwd, + relativePath: "src/source.md", + newRelativePath: "src/renamed.md", + }) + .pipe(Effect.flip); + + expect(error).toBeInstanceOf(ProjectRenameEntryError); + expect(error).toMatchObject({ stage: "rename" }); + const names = yield* fileSystem.readDirectory(path.join(cwd, "src")); + expect(names).toContain("source.md"); + expect(names).not.toContain("renamed.md"); + }), + ); }, ); diff --git a/apps/server/src/workspace/WorkspaceFileSystem.ts b/apps/server/src/workspace/WorkspaceFileSystem.ts index b06ca435cf80..0a9148e52212 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.ts @@ -428,37 +428,44 @@ export const make = Effect.gen(function* () { Effect.catch(() => Effect.succeed("unsupported" as const)), ); if (claim === "unsupported") { - // Without hard links there is no atomic no-clobber primitive, so an - // exact-name listing plus the volume's own name resolution guard the - // target before a plain rename. exists finding the target while the - // basenames differ case-insensitively means another entry owns the - // name under different casing on a case-insensitive volume; equal - // basenames mean the hit is the source itself and the rename is a - // case change. - const siblingNames = yield* fileSystem - .readDirectory(path.dirname(source.absolutePath)) - .pipe(Effect.mapError((cause) => renameError("rename", cause))); - if (siblingNames.includes(path.basename(target.absolutePath))) { - return yield* new ProjectRenameEntryTargetExistsError({ - cwd: input.cwd, - relativePath: target.relativePath, - }); - } - const targetOccupied = yield* fileSystem - .exists(target.absolutePath) - .pipe(Effect.mapError((cause) => renameError("rename", cause))); - const caseChangeOnly = - path.basename(source.absolutePath).toLowerCase() === - path.basename(target.absolutePath).toLowerCase(); - if (targetOccupied && !caseChangeOnly) { - return yield* new ProjectRenameEntryTargetExistsError({ - cwd: input.cwd, - relativePath: target.relativePath, - }); + // Without hard links an empty O_EXCL create claims the target name, + // so the rename below can only ever replace this rename's own claim + // and no window exists between the conflict check and the rename. + // The create also fails on entries exists() cannot see, such as a + // dangling symlink. When the name is occupied by the source's own + // inode, the volume resolves names case-insensitively and the + // rename is a case change, which needs no claim: any rival create + // at the target fails against the source until the rename lands. + const fallbackClaim = yield* fileSystem + .writeFile(target.absolutePath, new Uint8Array(0), { flag: "wx" }) + .pipe( + Effect.as("claimed" as const), + Effect.catchIf( + (error) => error.reason._tag === "AlreadyExists", + () => Effect.succeed("occupied" as const), + ), + Effect.mapError((cause) => renameError("rename", cause)), + ); + if (fallbackClaim === "occupied") { + const sameFile = yield* isSameFile(source.absolutePath, target.absolutePath); + if (!sameFile) { + return yield* new ProjectRenameEntryTargetExistsError({ + cwd: input.cwd, + relativePath: target.relativePath, + }); + } + return yield* fileSystem + .rename(source.absolutePath, target.absolutePath) + .pipe(Effect.mapError((cause) => renameError("rename", cause))); } - return yield* fileSystem - .rename(source.absolutePath, target.absolutePath) - .pipe(Effect.mapError((cause) => renameError("rename", cause))); + return yield* fileSystem.rename(source.absolutePath, target.absolutePath).pipe( + // A failed rename leaves the empty claim at the target; reclaim + // it so a retry does not read the name as taken. + Effect.tapError(() => + fileSystem.remove(target.absolutePath, { force: true }).pipe(Effect.ignore), + ), + Effect.mapError((cause) => renameError("rename", cause)), + ); } if (claim === "conflict") { const sameFile = yield* isSameFile(source.absolutePath, target.absolutePath); @@ -556,9 +563,15 @@ export const make = Effect.gen(function* () { return yield* deleteError("escapes-root"); } - const targetStat = yield* fileSystem.stat(target.absolutePath).pipe( + // lstat examines the directory entry itself, so a dangling symlink is + // still found and removed; stat would follow it, read NotFound, and + // report success while the entry stays on disk. + const targetStat = yield* Effect.tryPromise({ + try: () => NodeFSP.lstat(target.absolutePath), + catch: (cause) => cause as NodeJS.ErrnoException, + }).pipe( Effect.catchIf( - (error) => error.reason._tag === "NotFound", + (error) => error.code === "ENOENT", () => Effect.succeed(null), ), Effect.mapError((cause) => deleteError("resolve-path", cause)), @@ -566,7 +579,10 @@ export const make = Effect.gen(function* () { if (targetStat === null) { return; } - if (targetStat.type === "Directory") { + // remove never follows a symlink, so deleting one drops only the link. + // Everything else must be a regular file: directories, FIFOs, sockets, + // and device nodes stay out of reach, matching renameEntry. + if (!targetStat.isFile() && !targetStat.isSymbolicLink()) { return yield* deleteError("not-a-file"); } From 87512cec93f2bfd14e19d71b08bf908eca18f275 Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 13:19:05 +0800 Subject: [PATCH 42/54] fix(web): one mutation lane per project and reset previews on overwrite Rename keyed its serial lane on the source path, so a delete of the rename's target ran on another lane and could remove the freshly renamed file. writeFile, renameEntry, and deleteEntry now share one serial lane per project, which also keeps saves ordered with mutations of the same file. An overwrite upload onto the open file left the pre-upload contents in the preview, and a later debounced save wrote that stale snapshot over the upload. The upload now clears the optimistic overlay, resets pending edits, and refetches the file. A discarded coordinator also ignores late editor changes, so the cache-key rotation after a delete can no longer revive the revision and recreate the file. --- .../src/components/files/FileBrowserPanel.tsx | 17 +++++-- .../src/components/files/FilePreviewPanel.tsx | 11 +++++ .../files/fileSaveCoordinator.test.ts | 46 +++++++++++++++++++ .../components/files/fileSaveCoordinator.ts | 12 +++++ .../src/state/projectCommands.ts | 14 +++--- 5 files changed, 90 insertions(+), 10 deletions(-) diff --git a/apps/web/src/components/files/FileBrowserPanel.tsx b/apps/web/src/components/files/FileBrowserPanel.tsx index 05735a24d841..8ee15eca4d65 100644 --- a/apps/web/src/components/files/FileBrowserPanel.tsx +++ b/apps/web/src/components/files/FileBrowserPanel.tsx @@ -52,8 +52,8 @@ interface FileBrowserPanelProps { onRefreshSelectedFile?: () => void; /** * A rename or delete is about to run. Saves and mutations share one serial - * per-path command queue, so a save enqueued during the mutation would land - * after it and recreate the file; pending saves for the path must be + * per-project command queue, so a save enqueued during the mutation would + * land after it and recreate the file; pending saves for the path must be * held before the mutation enqueues. */ onEntryMutationStart?: (relativePath: string) => void; @@ -63,6 +63,11 @@ interface FileBrowserPanelProps { onEntryRenamed?: (relativePath: string, newRelativePath: string) => void; /** A delete succeeded; open surfaces for the path should close. */ onEntryDeleted?: (relativePath: string) => void; + /** + * An upload landed at the path, possibly replacing an open file; stale + * cached contents and pending edits for it should be dropped. + */ + onEntryUploaded?: (relativePath: string) => void; } const TREE_UNSAFE_CSS = ` @@ -233,6 +238,7 @@ export default function FileBrowserPanel({ onEntryMutationFailed, onEntryRenamed, onEntryDeleted, + onEntryUploaded, }: FileBrowserPanelProps) { const { resolvedTheme } = useTheme(); const composerRef = useComposerHandleContext(); @@ -246,10 +252,13 @@ export default function FileBrowserPanel({ environmentId, cwd, files, - onUploaded: () => entriesQuery.refresh(), + onUploaded: (relativePath) => { + entriesQuery.refresh(); + onEntryUploaded?.(relativePath); + }, }); }, - [cwd, entriesQuery, environmentId], + [cwd, entriesQuery, environmentId, onEntryUploaded], ); // The shared drop handlers manage drag-active state, but their onDrop reads // event.dataTransfer.files directly, which includes an unreadable stand-in diff --git a/apps/web/src/components/files/FilePreviewPanel.tsx b/apps/web/src/components/files/FilePreviewPanel.tsx index 9043976e5245..6f7236a1f824 100644 --- a/apps/web/src/components/files/FilePreviewPanel.tsx +++ b/apps/web/src/components/files/FilePreviewPanel.tsx @@ -406,6 +406,7 @@ interface FileSaveControls { suspend: () => void; resume: () => void; discard: () => void; + reset: () => void; } interface FileSelectionOverride { @@ -448,6 +449,7 @@ function useFileSaveCoordinator({ suspend: () => coordinator.suspend(), resume: () => coordinator.resume(), discard: () => coordinator.discard(), + reset: () => coordinator.reset(), }; return () => { discardSavesRef.current = null; @@ -1130,6 +1132,15 @@ export default function FilePreviewPanel({ onEntryMutationStart={(path) => { if (editorShowsFile(path)) discardActiveFileSavesRef.current?.suspend(); }} + onEntryUploaded={(path) => { + // An overwrite upload replaced the bytes on disk; drop the + // stale optimistic overlay and pending edits so a later save + // cannot write the pre-upload snapshot over the upload. + clearProjectFileQueryData(environmentId, cwd, path); + if (!editorShowsFile(path) || isImage) return; + discardActiveFileSavesRef.current?.reset(); + file.refresh(); + }} onEntryMutationFailed={(path) => { if (editorShowsFile(path)) discardActiveFileSavesRef.current?.resume(); }} diff --git a/apps/web/src/components/files/fileSaveCoordinator.test.ts b/apps/web/src/components/files/fileSaveCoordinator.test.ts index 8d6c3288b162..9f22981ccc52 100644 --- a/apps/web/src/components/files/fileSaveCoordinator.test.ts +++ b/apps/web/src/components/files/fileSaveCoordinator.test.ts @@ -114,6 +114,52 @@ describe("FileSaveCoordinator", () => { expect(onPendingChange.mock.calls.at(-1)).toEqual([false]); }); + it("ignores changes made after discard", async () => { + vi.useFakeTimers(); + const persist = vi + .fn<(contents: string) => Promise>>() + .mockResolvedValue(AsyncResult.success(undefined)); + const onPendingChange = vi.fn(); + const coordinator = new FileSaveCoordinator({ + debounceMs: 500, + persist, + onPendingChange, + onConfirmed: vi.fn(), + }); + + coordinator.discard(); + coordinator.change("late editor churn"); + await vi.runAllTimersAsync(); + + expect(persist).not.toHaveBeenCalled(); + expect(onPendingChange).not.toHaveBeenCalledWith(true); + }); + + it("reset drops pending edits but later changes still save", async () => { + vi.useFakeTimers(); + const persist = vi + .fn<(contents: string) => Promise>>() + .mockResolvedValue(AsyncResult.success(undefined)); + const onPendingChange = vi.fn(); + const coordinator = new FileSaveCoordinator({ + debounceMs: 500, + persist, + onPendingChange, + onConfirmed: vi.fn(), + }); + + coordinator.change("stale pre-upload edit"); + coordinator.reset(); + await vi.advanceTimersByTimeAsync(5_000); + expect(persist).not.toHaveBeenCalled(); + expect(onPendingChange.mock.calls.at(-1)).toEqual([false]); + + coordinator.change("fresh edit"); + await vi.runAllTimersAsync(); + expect(persist).toHaveBeenCalledOnce(); + expect(persist).toHaveBeenCalledWith("fresh edit"); + }); + it("suspend holds saves and resume persists the held edits", 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 c87ef66f91a7..91390ffb2d73 100644 --- a/apps/web/src/components/files/fileSaveCoordinator.ts +++ b/apps/web/src/components/files/fileSaveCoordinator.ts @@ -19,6 +19,10 @@ export class FileSaveCoordinator { constructor(private readonly options: FileSaveCoordinatorOptions) {} change(contents: string): void { + // After discard() the file is gone from disk; a late editor change (for + // example the cache-key rotation replacing the contents) must not revive + // the revision and recreate the file. + if (this.disposed) return; this.latestContents = contents; this.latestRevision += 1; this.lastChangeAt = Date.now(); @@ -35,6 +39,14 @@ export class FileSaveCoordinator { /** Drop unsaved edits without persisting; for files removed out from under the surface. */ discard(): void { this.disposed = true; + this.reset(); + } + + /** + * Drop unsaved edits but keep saving alive; for files replaced on disk, + * where the surface reloads the new contents and editing continues. + */ + reset(): void { this.suspended = false; this.clearTimer(); this.latestRevision = 0; diff --git a/packages/client-runtime/src/state/projectCommands.ts b/packages/client-runtime/src/state/projectCommands.ts index 7ce1c61fc267..39309ce8ae50 100644 --- a/packages/client-runtime/src/state/projectCommands.ts +++ b/packages/client-runtime/src/state/projectCommands.ts @@ -92,14 +92,18 @@ export function createProjectEnvironmentAtoms( scheduler: projectScheduler, concurrency: projectConcurrency, }), + // Saves, renames, and deletes share one serial lane per project. A rename + // and a delete key on different paths when the rename targets the deleted + // path, so per-path lanes would let the delete run concurrently and + // remove the freshly renamed file; one lane orders every pair, including + // a save racing a mutation of its own path. writeFile: createEnvironmentRpcCommand(runtime, { label: "environment-data:projects:write-file", tag: WS_METHODS.projectsWriteFile, scheduler: fileScheduler, concurrency: { mode: "serial", - key: ({ environmentId, input }) => - JSON.stringify([environmentId, input.cwd, input.relativePath]), + key: ({ environmentId, input }) => JSON.stringify([environmentId, input.cwd]), }, }), createUploadUrl: createEnvironmentRpcCommand(runtime, { @@ -112,8 +116,7 @@ export function createProjectEnvironmentAtoms( scheduler: fileScheduler, concurrency: { mode: "serial", - key: ({ environmentId, input }) => - JSON.stringify([environmentId, input.cwd, input.relativePath]), + key: ({ environmentId, input }) => JSON.stringify([environmentId, input.cwd]), }, }), deleteEntry: createEnvironmentRpcCommand(runtime, { @@ -122,8 +125,7 @@ export function createProjectEnvironmentAtoms( scheduler: fileScheduler, concurrency: { mode: "serial", - key: ({ environmentId, input }) => - JSON.stringify([environmentId, input.cwd, input.relativePath]), + key: ({ environmentId, input }) => JSON.stringify([environmentId, input.cwd]), }, }), }; From afe9a828ae5d865011080693400a8ffef6a4f425 Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 13:50:17 +0800 Subject: [PATCH 43/54] fix(server): guard rename reclaim by inode and resolve third-casing renames --- .../src/workspace/WorkspaceFileSystem.test.ts | 72 ++++++++++++++++--- .../src/workspace/WorkspaceFileSystem.ts | 40 +++++++++-- 2 files changed, 98 insertions(+), 14 deletions(-) diff --git a/apps/server/src/workspace/WorkspaceFileSystem.test.ts b/apps/server/src/workspace/WorkspaceFileSystem.test.ts index b078e7d416c8..bd5d13c0e987 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.test.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.test.ts @@ -1,3 +1,6 @@ +// @effect-diagnostics nodeBuiltinImport:off +import * as NodeFS from "node:fs"; + import * as NodeServices from "@effect/platform-node/NodeServices"; import { it, describe, expect } from "@effect/vitest"; import { @@ -724,6 +727,10 @@ it.layer(LinklessTestLayer, { excludeTestServices: true })( }, ); +// Simulates a linkless volume whose rename also fails; when rivalBytesOnRename +// holds bytes, the rename first replaces the target with them, standing in for +// a confirmed overwrite landing between the claim and the rename. +const rivalBytesOnRename: { current: Uint8Array | null } = { current: null }; const brokenRenameFileSystemLayer = Layer.effect( FileSystem.FileSystem, Effect.gen(function* () { @@ -742,15 +749,27 @@ const brokenRenameFileSystemLayer = Layer.effect( }), ), rename: (_oldPath, newPath) => - Effect.fail( - PlatformError.systemError({ - _tag: "PermissionDenied", - module: "FileSystem", - method: "rename", - syscall: "rename", - pathOrDescriptor: newPath, - description: "EACCES: the volume failed the rename", - }), + Effect.sync(() => { + const rival = rivalBytesOnRename.current; + if (rival) { + // A real overwrite renames the rival's staged part onto the + // target, replacing the inode; remove-then-write reproduces that. + NodeFS.rmSync(newPath, { force: true }); + NodeFS.writeFileSync(newPath, rival); + } + }).pipe( + Effect.andThen( + Effect.fail( + PlatformError.systemError({ + _tag: "PermissionDenied", + module: "FileSystem", + method: "rename", + syscall: "rename", + pathOrDescriptor: newPath, + description: "EACCES: the volume failed the rename", + }), + ), + ), ), }); }), @@ -796,5 +815,40 @@ it.layer(BrokenRenameTestLayer, { excludeTestServices: true })( expect(names).not.toContain("renamed.md"); }), ); + + // A zero-byte rival is indistinguishable from the claim by size, so this + // pins the reclaim's inode comparison. + it.effect("keeps a rival's zero-byte overwrite when the fallback rename fails", () => + Effect.gen(function* () { + const workspaceFileSystem = yield* WorkspaceFileSystem.WorkspaceFileSystem; + const path = yield* Path.Path; + const cwd = yield* makeTempDir; + yield* writeTextFile(cwd, "src/source.md", "source\n"); + rivalBytesOnRename.current = new Uint8Array(0); + + const error = yield* workspaceFileSystem + .renameEntry({ + cwd, + relativePath: "src/source.md", + newRelativePath: "src/renamed.md", + }) + .pipe(Effect.flip); + + expect(error).toBeInstanceOf(ProjectRenameEntryError); + expect(error).toMatchObject({ stage: "rename" }); + // The rival replaced the claim before the failed rename; the reclaim + // must not delete it. + const target = path.join(cwd, "src/renamed.md"); + expect(NodeFS.existsSync(target)).toBe(true); + expect(NodeFS.readFileSync(target).byteLength).toBe(0); + expect(NodeFS.existsSync(path.join(cwd, "src/source.md"))).toBe(true); + }).pipe( + Effect.ensuring( + Effect.sync(() => { + rivalBytesOnRename.current = null; + }), + ), + ), + ); }, ); diff --git a/apps/server/src/workspace/WorkspaceFileSystem.ts b/apps/server/src/workspace/WorkspaceFileSystem.ts index 0a9148e52212..8a0212141f12 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.ts @@ -27,6 +27,7 @@ import * as Context from "effect/Context"; import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; +import * as Option from "effect/Option"; import * as Path from "effect/Path"; import * as Schema from "effect/Schema"; @@ -458,11 +459,26 @@ export const make = Effect.gen(function* () { .rename(source.absolutePath, target.absolutePath) .pipe(Effect.mapError((cause) => renameError("rename", cause))); } + // A rival's confirmed overwrite can replace the claim before the + // rename and can legitimately be zero bytes, so the failed-rename + // reclaim below requires the inode captured at claim time, with the + // size check alone only where the platform reports no inode. + const claimInode = yield* fileSystem.stat(target.absolutePath).pipe( + Effect.map((info) => Option.getOrNull(info.ino)), + Effect.mapError((cause) => renameError("rename", cause)), + ); return yield* fileSystem.rename(source.absolutePath, target.absolutePath).pipe( // A failed rename leaves the empty claim at the target; reclaim // it so a retry does not read the name as taken. Effect.tapError(() => - fileSystem.remove(target.absolutePath, { force: true }).pipe(Effect.ignore), + fileSystem.stat(target.absolutePath).pipe( + Effect.flatMap((info) => + info.size === FileSystem.Size(0) && Option.getOrNull(info.ino) === claimInode + ? fileSystem.remove(target.absolutePath, { force: true }) + : Effect.void, + ), + Effect.ignore, + ), ), Effect.mapError((cause) => renameError("rename", cause)), ); @@ -481,9 +497,7 @@ export const make = Effect.gen(function* () { // so both names listed is a pre-existing hard link pair, where the // target name is occupied like any other conflict. Only the source // listed is the source under another casing on a case-insensitive - // filesystem, where rename applies the case change. Otherwise the - // source entry is gone or already carries the target casing, and - // the target name holds the data. + // filesystem, where rename applies the case change. const siblingNames = yield* fileSystem .readDirectory(path.dirname(source.absolutePath)) .pipe(Effect.mapError((cause) => renameError("rename", cause))); @@ -500,7 +514,23 @@ export const make = Effect.gen(function* () { .rename(source.absolutePath, target.absolutePath) .pipe(Effect.mapError((cause) => renameError("rename", cause))); } - return; + // The file can also sit on disk under a third casing that matches + // neither typed name, which the listing checks above cannot see. + // Resolve the entry that folds to the source name and rename from + // it so the case change still lands. No such entry means the name + // already carries the target casing or the data moved on, and the + // rename is done. + const sourceFold = path.basename(source.absolutePath).toLowerCase(); + const targetName = path.basename(target.absolutePath); + const onDiskName = siblingNames.find( + (name) => name !== targetName && name.toLowerCase() === sourceFold, + ); + if (onDiskName === undefined) { + return; + } + return yield* fileSystem + .rename(path.join(path.dirname(source.absolutePath), onDiskName), target.absolutePath) + .pipe(Effect.mapError((cause) => renameError("rename", cause))); } yield* fileSystem.remove(source.absolutePath).pipe( // A missing source means something else removed it after the link From 43fffaf29e11238d3f7dfcc2ccb3d046f1e1e8da Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 13:50:18 +0800 Subject: [PATCH 44/54] fix(web): hold saves during overwrite uploads and refresh image previews --- .../src/components/files/FileBrowserPanel.tsx | 33 +++++++++++++++---- .../src/components/files/FilePreviewPanel.tsx | 23 ++++++++++++- apps/web/src/lib/workspaceUploadQueue.ts | 17 ++++++++++ 3 files changed, 66 insertions(+), 7 deletions(-) diff --git a/apps/web/src/components/files/FileBrowserPanel.tsx b/apps/web/src/components/files/FileBrowserPanel.tsx index 8ee15eca4d65..a75134a9b94a 100644 --- a/apps/web/src/components/files/FileBrowserPanel.tsx +++ b/apps/web/src/components/files/FileBrowserPanel.tsx @@ -51,13 +51,17 @@ interface FileBrowserPanelProps { onOpenFile: (relativePath: string) => void; onRefreshSelectedFile?: () => void; /** - * A rename or delete is about to run. Saves and mutations share one serial - * per-project command queue, so a save enqueued during the mutation would - * land after it and recreate the file; pending saves for the path must be - * held before the mutation enqueues. + * A rename, delete, or confirmed overwrite upload is about to run. Saves + * and mutations share one serial per-project command queue, so a save + * enqueued during the mutation would land after it and recreate the file, + * and uploads bypass the queue entirely; pending saves for the path must + * be held before the mutation runs. */ onEntryMutationStart?: (relativePath: string) => void; - /** The mutation failed and the file is still in place; held saves may run again. */ + /** + * The mutation failed, or the upload settled, and the file is still in + * place; held saves may run again. + */ onEntryMutationFailed?: (relativePath: string) => void; /** A rename succeeded; open surfaces for the old path should follow the file. */ onEntryRenamed?: (relativePath: string, newRelativePath: string) => void; @@ -252,13 +256,30 @@ export default function FileBrowserPanel({ environmentId, cwd, files, + // A confirmed overwrite replaces the open file's bytes outside the + // serial save lane, so pending saves hold from the confirmation until + // the job settles. A successful upload re-arms saves through + // onEntryUploaded's reset, which makes the settle release a no-op. + onOverwriteStart: (relativePath) => { + onEntryMutationStart?.(relativePath); + }, + onSettled: (relativePath) => { + onEntryMutationFailed?.(relativePath); + }, onUploaded: (relativePath) => { entriesQuery.refresh(); onEntryUploaded?.(relativePath); }, }); }, - [cwd, entriesQuery, environmentId, onEntryUploaded], + [ + cwd, + entriesQuery, + environmentId, + onEntryMutationFailed, + onEntryMutationStart, + onEntryUploaded, + ], ); // The shared drop handlers manage drag-active state, but their onDrop reads // event.dataTransfer.files directly, which includes an unreadable stand-in diff --git a/apps/web/src/components/files/FilePreviewPanel.tsx b/apps/web/src/components/files/FilePreviewPanel.tsx index 6f7236a1f824..e1c2025e2149 100644 --- a/apps/web/src/components/files/FilePreviewPanel.tsx +++ b/apps/web/src/components/files/FilePreviewPanel.tsx @@ -27,6 +27,7 @@ import { getLocalStorageItem, setLocalStorageItem, useLocalStorage } from "~/hoo import { DIFF_SURFACE_THEME_UNSAFE_CSS, resolveDiffThemeName } from "~/lib/diffRendering"; import { cn } from "~/lib/utils"; import { isPreviewSupportedInRuntime } from "~/previewStateStore"; +import { appAtomRegistry } from "~/rpc/atomRegistry"; import { selectThreadRightPanelState, useRightPanelStore } from "~/rightPanelStore"; import { resolvePathLinkTarget } from "~/terminal-links"; import { ScrollArea } from "~/components/ui/scroll-area"; @@ -1137,7 +1138,27 @@ export default function FilePreviewPanel({ // stale optimistic overlay and pending edits so a later save // cannot write the pre-upload snapshot over the upload. clearProjectFileQueryData(environmentId, cwd, path); - if (!editorShowsFile(path) || isImage) return; + if (!editorShowsFile(path)) return; + if (isImage) { + // The image preview renders a signed asset URL whose query + // stays fresh for minutes; re-mint it so the overwritten + // bytes replace the stale bitmap immediately. + if (absolutePath) { + appAtomRegistry.refresh( + assetEnvironment.createUrl({ + environmentId, + input: { + resource: { + _tag: "workspace-file", + threadId: threadRef.threadId, + path: absolutePath, + }, + }, + }), + ); + } + return; + } discardActiveFileSavesRef.current?.reset(); file.refresh(); }} diff --git a/apps/web/src/lib/workspaceUploadQueue.ts b/apps/web/src/lib/workspaceUploadQueue.ts index 7a451ee5d3e7..3caa127bbf60 100644 --- a/apps/web/src/lib/workspaceUploadQueue.ts +++ b/apps/web/src/lib/workspaceUploadQueue.ts @@ -49,6 +49,8 @@ interface UploadJob { readonly relativePath: string; readonly file: File; readonly onUploaded: (relativePath: string) => void; + readonly onOverwriteStart: ((relativePath: string) => void) | undefined; + readonly onSettled: ((relativePath: string) => void) | undefined; overwrite: boolean; cancelled: boolean; abort: (() => void) | null; @@ -132,6 +134,9 @@ async function runUpload(job: UploadJob): Promise { } job.overwrite = true; + // The upload replaces the target's bytes outside the serial save lane, so + // pending saves of the file hold from here until the job settles. + job.onOverwriteStart?.(job.relativePath); minted = await mintUploadUrl(job); if (job.cancelled) { jobsById.delete(job.id); @@ -220,6 +225,12 @@ function pumpUploads(): void { } }) .finally(() => { + try { + job.onSettled?.(job.relativePath); + } catch (error) { + // A throwing settle callback must not stall the queue. + console.error(error); + } const remaining = (activeUploadsByEnvironment.get(job.environmentId) ?? 1) - 1; if (remaining > 0) { activeUploadsByEnvironment.set(job.environmentId, remaining); @@ -236,6 +247,10 @@ export function startWorkspaceUploads(input: { readonly cwd: string; readonly files: ReadonlyArray; readonly onUploaded: (relativePath: string) => void; + /** An overwrite of the path was confirmed and is about to run. */ + readonly onOverwriteStart?: (relativePath: string) => void; + /** The job reached a terminal state: stored, failed, or cancelled. */ + readonly onSettled?: (relativePath: string) => void; }): void { for (const file of input.files) { const id = randomUUID(); @@ -250,6 +265,8 @@ export function startWorkspaceUploads(input: { relativePath, file, onUploaded: input.onUploaded, + onOverwriteStart: input.onOverwriteStart, + onSettled: input.onSettled, overwrite: false, cancelled: false, abort: null, From 69027195f6ca0f2ad8c7baa8a498f72b1546b993 Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 14:14:04 +0800 Subject: [PATCH 45/54] fix(server): keep a rival's file when it lands under the source mid-rename Two rename gaps. After the hard link landed, the source removal ran unconditionally, so a concurrent writer's new file under the source name was deleted; the removal now runs only while the source still names the linked inode. And a stat failure between the fallback claim and the inode capture stranded the empty claim, making every retry read the name as taken; the claim is now reclaimed before the error surfaces. Tests cover both: a link-rival layer that replaces the source after the link, and a path-scoped stat failure hook. --- .../src/workspace/WorkspaceFileSystem.test.ts | 123 +++++++++++++++++- .../src/workspace/WorkspaceFileSystem.ts | 14 ++ 2 files changed, 136 insertions(+), 1 deletion(-) diff --git a/apps/server/src/workspace/WorkspaceFileSystem.test.ts b/apps/server/src/workspace/WorkspaceFileSystem.test.ts index bd5d13c0e987..d67615e49d64 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.test.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.test.ts @@ -729,14 +729,30 @@ it.layer(LinklessTestLayer, { excludeTestServices: true })( // Simulates a linkless volume whose rename also fails; when rivalBytesOnRename // holds bytes, the rename first replaces the target with them, standing in for -// a confirmed overwrite landing between the claim and the rename. +// a confirmed overwrite landing between the claim and the rename. When +// statErrorPath names a path, stat of that exact path fails, standing in for +// a volume fault between the claim and the inode capture. const rivalBytesOnRename: { current: Uint8Array | null } = { current: null }; +const statErrorPath: { current: string | null } = { current: null }; const brokenRenameFileSystemLayer = Layer.effect( FileSystem.FileSystem, Effect.gen(function* () { const real = yield* FileSystem.FileSystem; return FileSystem.FileSystem.of({ ...real, + stat: (statPath) => + statPath === statErrorPath.current + ? Effect.fail( + PlatformError.systemError({ + _tag: "Unknown", + module: "FileSystem", + method: "stat", + syscall: "stat", + pathOrDescriptor: statPath, + description: "EIO: the volume failed the stat", + }), + ) + : real.stat(statPath), link: (_fromPath, toPath) => Effect.fail( PlatformError.systemError({ @@ -850,5 +866,110 @@ it.layer(BrokenRenameTestLayer, { excludeTestServices: true })( ), ), ); + + // A failed claim-inode stat surfaces before any rename runs; without the + // reclaim the empty claim would make every retry read the name as taken. + it.effect("reclaims the target name when the claim-inode stat fails", () => + Effect.gen(function* () { + const workspaceFileSystem = yield* WorkspaceFileSystem.WorkspaceFileSystem; + const path = yield* Path.Path; + const cwd = yield* makeTempDir; + yield* writeTextFile(cwd, "src/source.md", "source\n"); + statErrorPath.current = path.join(cwd, "src/renamed.md"); + + const error = yield* workspaceFileSystem + .renameEntry({ + cwd, + relativePath: "src/source.md", + newRelativePath: "src/renamed.md", + }) + .pipe(Effect.flip); + + expect(error).toBeInstanceOf(ProjectRenameEntryError); + expect(error).toMatchObject({ stage: "rename" }); + expect(NodeFS.existsSync(path.join(cwd, "src/renamed.md"))).toBe(false); + expect(NodeFS.existsSync(path.join(cwd, "src/source.md"))).toBe(true); + }).pipe( + Effect.ensuring( + Effect.sync(() => { + statErrorPath.current = null; + }), + ), + ), + ); + }, +); + +// The link lands, then a concurrent writer replaces the source name with a +// new file before the source removal runs. +const linkRivalBytes: { current: Uint8Array | null } = { current: null }; +const linkRivalFileSystemLayer = Layer.effect( + FileSystem.FileSystem, + Effect.gen(function* () { + const real = yield* FileSystem.FileSystem; + return FileSystem.FileSystem.of({ + ...real, + link: (fromPath, toPath) => + real.link(fromPath, toPath).pipe( + Effect.andThen( + Effect.sync(() => { + const rival = linkRivalBytes.current; + if (rival) { + // A real writer renames its own staged file onto the source + // name, replacing the inode; remove-then-write reproduces it. + NodeFS.rmSync(fromPath, { force: true }); + NodeFS.writeFileSync(fromPath, rival); + } + }), + ), + ), + }); + }), +); + +const LinkRivalTestLayer = Layer.empty.pipe( + Layer.provideMerge(ProjectLayer), + Layer.provideMerge(WorkspaceEntries.layer.pipe(Layer.provide(WorkspacePaths.layer))), + Layer.provideMerge(WorkspacePaths.layer), + Layer.provideMerge(VcsDriverRegistry.layer.pipe(Layer.provide(VcsProcess.layer))), + Layer.provide( + ServerConfig.ServerConfig.layerTest(process.cwd(), { + prefix: "t3-workspace-files-test-", + }), + ), + Layer.provideMerge(linkRivalFileSystemLayer), + Layer.provideMerge(NodeServices.layer), +); + +it.layer(LinkRivalTestLayer, { excludeTestServices: true })( + "WorkspaceFileSystemLive when a writer replaces the source after the link", + (it) => { + it.effect("keeps the writer's file instead of removing the source name", () => + Effect.gen(function* () { + const workspaceFileSystem = yield* WorkspaceFileSystem.WorkspaceFileSystem; + const path = yield* Path.Path; + const cwd = yield* makeTempDir; + yield* writeTextFile(cwd, "src/source.md", "source\n"); + linkRivalBytes.current = new TextEncoder().encode("rival\n"); + + const result = yield* workspaceFileSystem.renameEntry({ + cwd, + relativePath: "src/source.md", + newRelativePath: "src/renamed.md", + }); + + expect(result).toEqual({ relativePath: "src/renamed.md" }); + // The rename lands under the new name and the writer's file survives + // under the old one, the shape a plain rename race would also leave. + expect(NodeFS.readFileSync(path.join(cwd, "src/renamed.md"), "utf8")).toBe("source\n"); + expect(NodeFS.readFileSync(path.join(cwd, "src/source.md"), "utf8")).toBe("rival\n"); + }).pipe( + Effect.ensuring( + Effect.sync(() => { + linkRivalBytes.current = null; + }), + ), + ), + ); }, ); diff --git a/apps/server/src/workspace/WorkspaceFileSystem.ts b/apps/server/src/workspace/WorkspaceFileSystem.ts index 8a0212141f12..8741837f4ea1 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.ts @@ -465,6 +465,11 @@ export const make = Effect.gen(function* () { // size check alone only where the platform reports no inode. const claimInode = yield* fileSystem.stat(target.absolutePath).pipe( Effect.map((info) => Option.getOrNull(info.ino)), + // A stat failure here strands the empty claim, so every later + // attempt reads the name as taken; reclaim before surfacing. + Effect.tapError(() => + fileSystem.remove(target.absolutePath, { force: true }).pipe(Effect.ignore), + ), Effect.mapError((cause) => renameError("rename", cause)), ); return yield* fileSystem.rename(source.absolutePath, target.absolutePath).pipe( @@ -532,6 +537,15 @@ export const make = Effect.gen(function* () { .rename(path.join(path.dirname(source.absolutePath), onDiskName), target.absolutePath) .pipe(Effect.mapError((cause) => renameError("rename", cause))); } + // A concurrent writer can replace the source name after the link + // lands; removing it then would destroy the newer data. The source is + // only removed while it still names the linked inode. Otherwise the + // writer's file stays under the source name, the same shape a plain + // rename leaves when the source is recreated mid-flight. + const sourceStillLinked = yield* isSameFile(source.absolutePath, target.absolutePath); + if (!sourceStillLinked) { + return; + } yield* fileSystem.remove(source.absolutePath).pipe( // A missing source means something else removed it after the link // landed, leaving the target as the only copy of the data; rolling From 1d3ba6b1cb255941f4b09195d77339fd15a0ec24 Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 14:14:17 +0800 Subject: [PATCH 46/54] fix(web): hold saves from conflict discovery and track saved revisions Three save-coordination gaps around overwrite uploads. The overwrite hold now starts when the conflict is discovered, before the confirm dialog opens, so a debounced save cannot land while the dialog is up. The coordinator counts overlapping holds instead of a boolean, so a rename finishing early cannot release an upload's hold. And resume compares against a persisted-revision watermark, so a snapshot that already saved is not written again over what the upload put on disk. Settle callbacks pair with the hold: only jobs that entered the overwrite phase fire onSettled. --- .../src/components/files/FileBrowserPanel.tsx | 9 +- .../files/fileSaveCoordinator.test.ts | 71 ++++++++++++++ .../components/files/fileSaveCoordinator.ts | 36 ++++--- apps/web/src/lib/workspaceUploadQueue.test.ts | 96 +++++++++++++++++++ apps/web/src/lib/workspaceUploadQueue.ts | 30 ++++-- 5 files changed, 216 insertions(+), 26 deletions(-) diff --git a/apps/web/src/components/files/FileBrowserPanel.tsx b/apps/web/src/components/files/FileBrowserPanel.tsx index a75134a9b94a..d95bb2d4adde 100644 --- a/apps/web/src/components/files/FileBrowserPanel.tsx +++ b/apps/web/src/components/files/FileBrowserPanel.tsx @@ -256,10 +256,11 @@ export default function FileBrowserPanel({ environmentId, cwd, files, - // A confirmed overwrite replaces the open file's bytes outside the - // serial save lane, so pending saves hold from the confirmation until - // the job settles. A successful upload re-arms saves through - // onEntryUploaded's reset, which makes the settle release a no-op. + // An overwrite upload replaces the open file's bytes outside the + // serial save lane, so pending saves hold from conflict discovery, + // before the confirm dialog opens, until the job settles. A successful + // upload re-arms saves through onEntryUploaded's reset, which makes + // the settle release a no-op. onOverwriteStart: (relativePath) => { onEntryMutationStart?.(relativePath); }, diff --git a/apps/web/src/components/files/fileSaveCoordinator.test.ts b/apps/web/src/components/files/fileSaveCoordinator.test.ts index 9f22981ccc52..d550d2f97668 100644 --- a/apps/web/src/components/files/fileSaveCoordinator.test.ts +++ b/apps/web/src/components/files/fileSaveCoordinator.test.ts @@ -208,6 +208,77 @@ describe("FileSaveCoordinator", () => { expect(onPendingChange.mock.calls.at(-1)).toEqual([false]); }); + it("resume does not re-persist a snapshot that already saved", async () => { + vi.useFakeTimers(); + const persist = vi + .fn<(contents: string) => Promise>>() + .mockResolvedValue(AsyncResult.success(undefined)); + const coordinator = new FileSaveCoordinator({ + debounceMs: 500, + persist, + onPendingChange: vi.fn(), + onConfirmed: vi.fn(), + }); + + coordinator.change("saved before the mutation"); + await vi.runAllTimersAsync(); + expect(persist).toHaveBeenCalledOnce(); + + coordinator.suspend(); + coordinator.resume(); + await vi.runAllTimersAsync(); + expect(persist).toHaveBeenCalledOnce(); + }); + + it("resume persists an edit made after the last successful save", async () => { + vi.useFakeTimers(); + const persist = vi + .fn<(contents: string) => Promise>>() + .mockResolvedValue(AsyncResult.success(undefined)); + const coordinator = new FileSaveCoordinator({ + debounceMs: 500, + persist, + onPendingChange: vi.fn(), + onConfirmed: vi.fn(), + }); + + coordinator.change("saved"); + await vi.runAllTimersAsync(); + expect(persist).toHaveBeenCalledOnce(); + + coordinator.change("edited during the mutation window"); + coordinator.suspend(); + coordinator.resume(); + await vi.runAllTimersAsync(); + expect(persist).toHaveBeenCalledTimes(2); + expect(persist).toHaveBeenLastCalledWith("edited during the mutation window"); + }); + + it("overlapping suspends hold saves until the last resume", async () => { + vi.useFakeTimers(); + const persist = vi + .fn<(contents: string) => Promise>>() + .mockResolvedValue(AsyncResult.success(undefined)); + const coordinator = new FileSaveCoordinator({ + debounceMs: 500, + persist, + onPendingChange: vi.fn(), + onConfirmed: vi.fn(), + }); + + coordinator.change("held"); + coordinator.suspend(); + coordinator.suspend(); + coordinator.resume(); + await vi.advanceTimersByTimeAsync(5_000); + expect(persist).not.toHaveBeenCalled(); + + coordinator.resume(); + await vi.runAllTimersAsync(); + expect(persist).toHaveBeenCalledOnce(); + expect(persist).toHaveBeenCalledWith("held"); + }); + it("dispose while suspended does not flush behind a pending mutation", 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 91390ffb2d73..29731f18efd5 100644 --- a/apps/web/src/components/files/fileSaveCoordinator.ts +++ b/apps/web/src/components/files/fileSaveCoordinator.ts @@ -11,10 +11,11 @@ export class FileSaveCoordinator { private timer: ReturnType | null = null; private latestContents = ""; private latestRevision = 0; + private persistedRevision = 0; private lastChangeAt = 0; private saving = false; private disposed = false; - private suspended = false; + private suspendCount = 0; constructor(private readonly options: FileSaveCoordinatorOptions) {} @@ -33,7 +34,7 @@ export class FileSaveCoordinator { dispose(): void { this.disposed = true; this.clearTimer(); - if (this.latestRevision > 0) void this.persistLatest(); + if (this.latestRevision > this.persistedRevision) void this.persistLatest(); } /** Drop unsaved edits without persisting; for files removed out from under the surface. */ @@ -47,27 +48,35 @@ export class FileSaveCoordinator { * where the surface reloads the new contents and editing continues. */ reset(): void { - this.suspended = false; + this.suspendCount = 0; this.clearTimer(); this.latestRevision = 0; + this.persistedRevision = 0; this.options.onPendingChange(false); } /** - * Hold pending edits while a rename or delete runs, so a save cannot land - * mid-mutation. The mutation's outcome decides what follows: discard() on - * success, resume() on failure. + * Hold pending edits while a rename, delete, or overwrite upload runs, so a + * save cannot land mid-mutation. Holds count: overlapping mutations of the + * same file each take one, and saving stays held until every one has been + * released by discard(), reset(), or resume(). */ suspend(): void { - this.suspended = true; + this.suspendCount += 1; this.clearTimer(); } - /** Reinstate saving after a failed rename or delete left the file in place. */ + /** + * Release one hold after a failed mutation left the file in place. Only + * edits newer than the last successful save reschedule; a snapshot that + * already persisted must not overwrite what another writer put on disk + * since. + */ resume(): void { - if (!this.suspended) return; - this.suspended = false; - if (this.latestRevision > 0) this.schedule(0); + if (this.suspendCount === 0) return; + this.suspendCount -= 1; + if (this.suspendCount > 0) return; + if (this.latestRevision > this.persistedRevision) this.schedule(0); } private schedule(delay: number): void { @@ -85,7 +94,9 @@ export class FileSaveCoordinator { } private async persistLatest(): Promise { - if (this.suspended || this.saving || this.latestRevision === 0) return; + if (this.suspendCount > 0 || this.saving || this.latestRevision <= this.persistedRevision) { + return; + } this.saving = true; const contents = this.latestContents; @@ -93,6 +104,7 @@ export class FileSaveCoordinator { const result = await this.options.persist(contents); const succeeded = result._tag === "Success"; if (succeeded) { + this.persistedRevision = revision; this.options.onConfirmed(contents); } diff --git a/apps/web/src/lib/workspaceUploadQueue.test.ts b/apps/web/src/lib/workspaceUploadQueue.test.ts index 29e7ba3f7d66..776f71eece7f 100644 --- a/apps/web/src/lib/workspaceUploadQueue.test.ts +++ b/apps/web/src/lib/workspaceUploadQueue.test.ts @@ -297,6 +297,102 @@ describe("workspaceUploadQueue", () => { expect(TestXmlHttpRequest.requests).toHaveLength(0); }); + it("fires onOverwriteStart at conflict discovery, before the confirm dialog opens", async () => { + const events: string[] = []; + mocks.runAtomCommand.mockImplementation( + async ( + _registry: unknown, + command: unknown, + target: { readonly input: { readonly relativePath: string } }, + ) => { + if (command !== mocks.createUploadUrl) throw new Error("unexpected command"); + return targetExistsFailure(target.input.relativePath); + }, + ); + let resolveConfirm!: (value: boolean) => void; + mocks.requestConfirmDialog.mockImplementation(() => { + events.push("dialog"); + return new Promise((resolve) => { + resolveConfirm = resolve; + }); + }); + + startWorkspaceUploads({ + environmentId, + cwd, + files: [makeFile("existing.txt")], + onUploaded: vi.fn(), + onOverwriteStart: () => events.push("overwrite-start"), + onSettled: () => events.push("settled"), + }); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + expect(events).toEqual(["overwrite-start", "dialog"]); + + resolveConfirm(false); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + expect(events).toEqual(["overwrite-start", "dialog", "settled"]); + }); + + it("does not fire onSettled for uploads that never hit a conflict", async () => { + const onSettled = vi.fn(); + startWorkspaceUploads({ + environmentId, + cwd, + files: [makeFile("notes.txt")], + onUploaded: vi.fn(), + onSettled, + }); + await Promise.resolve(); + await Promise.resolve(); + + TestXmlHttpRequest.requests[0]!.complete(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + + expect(findUpload("notes.txt")).toBeUndefined(); + expect(onSettled).not.toHaveBeenCalled(); + }); + + it("retry leaves the overwrite phase, so a conflict-free retry does not settle again", async () => { + mocks.runAtomCommand.mockResolvedValueOnce(targetExistsFailure("existing.txt")); + mocks.requestConfirmDialog.mockResolvedValue(false); + const onSettled = vi.fn(); + startWorkspaceUploads({ + environmentId, + cwd, + files: [makeFile("existing.txt")], + onUploaded: vi.fn(), + onSettled, + }); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + expect(onSettled).toHaveBeenCalledTimes(1); + + const [uploadId] = findUpload("existing.txt")!; + retryWorkspaceUpload(uploadId); + await Promise.resolve(); + await Promise.resolve(); + + TestXmlHttpRequest.requests[0]!.complete(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + + expect(uploadsById()[uploadId]).toBeUndefined(); + expect(onSettled).toHaveBeenCalledTimes(1); + }); + it("cancelWorkspaceUpload aborts the in-flight XHR and removes the entry", async () => { const file = makeFile("cancel-me.txt"); startWorkspaceUploads({ environmentId, cwd, files: [file], onUploaded: vi.fn() }); diff --git a/apps/web/src/lib/workspaceUploadQueue.ts b/apps/web/src/lib/workspaceUploadQueue.ts index 3caa127bbf60..a80c6ff17423 100644 --- a/apps/web/src/lib/workspaceUploadQueue.ts +++ b/apps/web/src/lib/workspaceUploadQueue.ts @@ -52,6 +52,7 @@ interface UploadJob { readonly onOverwriteStart: ((relativePath: string) => void) | undefined; readonly onSettled: ((relativePath: string) => void) | undefined; overwrite: boolean; + overwriteStarted: boolean; cancelled: boolean; abort: (() => void) | null; } @@ -120,6 +121,12 @@ async function runUpload(job: UploadJob): Promise { return; } + // The overwrite phase begins at conflict discovery rather than at + // confirmation: a debounced save could otherwise fire while the confirm + // dialog is open and land after the upload replaces the bytes. + job.overwriteStarted = true; + job.onOverwriteStart?.(job.relativePath); + const confirmed = await readLocalApi()?.dialogs.confirm( `Replace ${job.relativePath}?\nA file named '${job.relativePath}' already exists in this project.`, { variant: "destructive" }, @@ -134,9 +141,6 @@ async function runUpload(job: UploadJob): Promise { } job.overwrite = true; - // The upload replaces the target's bytes outside the serial save lane, so - // pending saves of the file hold from here until the job settles. - job.onOverwriteStart?.(job.relativePath); minted = await mintUploadUrl(job); if (job.cancelled) { jobsById.delete(job.id); @@ -225,11 +229,15 @@ function pumpUploads(): void { } }) .finally(() => { - try { - job.onSettled?.(job.relativePath); - } catch (error) { - // A throwing settle callback must not stall the queue. - console.error(error); + // Settle pairs with onOverwriteStart: only jobs that entered the + // overwrite phase took a save hold, so only those release one. + if (job.overwriteStarted) { + try { + job.onSettled?.(job.relativePath); + } catch (error) { + // A throwing settle callback must not stall the queue. + console.error(error); + } } const remaining = (activeUploadsByEnvironment.get(job.environmentId) ?? 1) - 1; if (remaining > 0) { @@ -247,9 +255,9 @@ export function startWorkspaceUploads(input: { readonly cwd: string; readonly files: ReadonlyArray; readonly onUploaded: (relativePath: string) => void; - /** An overwrite of the path was confirmed and is about to run. */ + /** The path collided with an existing file; the confirm dialog is about to open. */ readonly onOverwriteStart?: (relativePath: string) => void; - /** The job reached a terminal state: stored, failed, or cancelled. */ + /** A job that fired onOverwriteStart reached a terminal state: stored, failed, or cancelled. */ readonly onSettled?: (relativePath: string) => void; }): void { for (const file of input.files) { @@ -268,6 +276,7 @@ export function startWorkspaceUploads(input: { onOverwriteStart: input.onOverwriteStart, onSettled: input.onSettled, overwrite: false, + overwriteStarted: false, cancelled: false, abort: null, }; @@ -317,6 +326,7 @@ export function retryWorkspaceUpload(uploadId: string): void { // The target may have changed since the original confirmation, so a retry // re-confirms an overwrite instead of carrying the earlier answer over. job.overwrite = false; + job.overwriteStarted = false; setUploadState(job.id, { status: "uploading", name: job.file.name, From 2e35887c6d4586077f97020926de44c1a4fdd11a Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 14:32:27 +0800 Subject: [PATCH 47/54] fix(server): guard the rename claim reclaim against a rival overwrite --- .../src/workspace/WorkspaceFileSystem.test.ts | 74 +++++++++++++++---- .../src/workspace/WorkspaceFileSystem.ts | 15 +++- 2 files changed, 73 insertions(+), 16 deletions(-) diff --git a/apps/server/src/workspace/WorkspaceFileSystem.test.ts b/apps/server/src/workspace/WorkspaceFileSystem.test.ts index d67615e49d64..84e848d7957c 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.test.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.test.ts @@ -730,10 +730,14 @@ it.layer(LinklessTestLayer, { excludeTestServices: true })( // Simulates a linkless volume whose rename also fails; when rivalBytesOnRename // holds bytes, the rename first replaces the target with them, standing in for // a confirmed overwrite landing between the claim and the rename. When -// statErrorPath names a path, stat of that exact path fails, standing in for -// a volume fault between the claim and the inode capture. +// statErrorPath names a path, the next stat of that exact path fails once, +// standing in for a transient volume fault between the claim and the inode +// capture; when rivalBytesOnStatError holds bytes, the failing stat first +// replaces the path with them, standing in for a confirmed overwrite landing +// in that same window. const rivalBytesOnRename: { current: Uint8Array | null } = { current: null }; const statErrorPath: { current: string | null } = { current: null }; +const rivalBytesOnStatError: { current: Uint8Array | null } = { current: null }; const brokenRenameFileSystemLayer = Layer.effect( FileSystem.FileSystem, Effect.gen(function* () { @@ -741,18 +745,27 @@ const brokenRenameFileSystemLayer = Layer.effect( return FileSystem.FileSystem.of({ ...real, stat: (statPath) => - statPath === statErrorPath.current - ? Effect.fail( - PlatformError.systemError({ - _tag: "Unknown", - module: "FileSystem", - method: "stat", - syscall: "stat", - pathOrDescriptor: statPath, - description: "EIO: the volume failed the stat", - }), - ) - : real.stat(statPath), + Effect.suspend(() => { + if (statPath !== statErrorPath.current) { + return real.stat(statPath); + } + statErrorPath.current = null; + const rival = rivalBytesOnStatError.current; + if (rival) { + NodeFS.rmSync(statPath, { force: true }); + NodeFS.writeFileSync(statPath, rival); + } + return Effect.fail( + PlatformError.systemError({ + _tag: "Unknown", + module: "FileSystem", + method: "stat", + syscall: "stat", + pathOrDescriptor: statPath, + description: "EIO: the volume failed the stat", + }), + ); + }), link: (_fromPath, toPath) => Effect.fail( PlatformError.systemError({ @@ -897,6 +910,39 @@ it.layer(BrokenRenameTestLayer, { excludeTestServices: true })( ), ), ); + + // A confirmed overwrite can replace the claim in the same window the stat + // fault covers; the reclaim must not delete the rival's stored file. + it.effect("keeps a rival's overwrite when the claim-inode stat fails", () => + Effect.gen(function* () { + const workspaceFileSystem = yield* WorkspaceFileSystem.WorkspaceFileSystem; + const path = yield* Path.Path; + const cwd = yield* makeTempDir; + yield* writeTextFile(cwd, "src/source.md", "source\n"); + statErrorPath.current = path.join(cwd, "src/renamed.md"); + rivalBytesOnStatError.current = new TextEncoder().encode("rival\n"); + + const error = yield* workspaceFileSystem + .renameEntry({ + cwd, + relativePath: "src/source.md", + newRelativePath: "src/renamed.md", + }) + .pipe(Effect.flip); + + expect(error).toBeInstanceOf(ProjectRenameEntryError); + expect(error).toMatchObject({ stage: "rename" }); + expect(NodeFS.readFileSync(path.join(cwd, "src/renamed.md"), "utf8")).toBe("rival\n"); + expect(NodeFS.existsSync(path.join(cwd, "src/source.md"))).toBe(true); + }).pipe( + Effect.ensuring( + Effect.sync(() => { + statErrorPath.current = null; + rivalBytesOnStatError.current = null; + }), + ), + ), + ); }, ); diff --git a/apps/server/src/workspace/WorkspaceFileSystem.ts b/apps/server/src/workspace/WorkspaceFileSystem.ts index 8741837f4ea1..983f335673e3 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.ts @@ -466,9 +466,20 @@ export const make = Effect.gen(function* () { const claimInode = yield* fileSystem.stat(target.absolutePath).pipe( Effect.map((info) => Option.getOrNull(info.ino)), // A stat failure here strands the empty claim, so every later - // attempt reads the name as taken; reclaim before surfacing. + // attempt reads the name as taken. With no inode to identify the + // claim by, the reclaim re-stats and removes only a zero-byte + // file, so a rival's non-empty overwrite is never deleted; when + // the fault persists the claim stays, trading a retryable + // conflict for zero data loss. Effect.tapError(() => - fileSystem.remove(target.absolutePath, { force: true }).pipe(Effect.ignore), + fileSystem.stat(target.absolutePath).pipe( + Effect.flatMap((info) => + info.size === FileSystem.Size(0) + ? fileSystem.remove(target.absolutePath, { force: true }) + : Effect.void, + ), + Effect.ignore, + ), ), Effect.mapError((cause) => renameError("rename", cause)), ); From c0ff793299a883e4224eb408e0b5bda5cdfe74f7 Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 14:32:28 +0800 Subject: [PATCH 48/54] fix(web): keep scroll target and panel state when a rename swaps the tab --- .../src/components/files/FilePreviewPanel.tsx | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/apps/web/src/components/files/FilePreviewPanel.tsx b/apps/web/src/components/files/FilePreviewPanel.tsx index e1c2025e2149..1ffb96917eb6 100644 --- a/apps/web/src/components/files/FilePreviewPanel.tsx +++ b/apps/web/src/components/files/FilePreviewPanel.tsx @@ -1175,16 +1175,28 @@ export default function FilePreviewPanel({ clearProjectFileQueryData(environmentId, cwd, from); const store = useRightPanelStore.getState(); const panel = selectThreadRightPanelState(store.byThreadKey, threadRef); - const wasOpen = panel.surfaces.some((surface) => surface.id === `file:${from}`); + const fromSurface = panel.surfaces.find((surface) => surface.id === `file:${from}`); const previousActiveId = panel.activeSurfaceId; + const wasPanelOpen = panel.isOpen; store.closeSurface(threadRef, `file:${from}`); - if (!wasOpen) return; - store.openFile(threadRef, to); + if (!fromSurface) return; + // The surface a rename replaces keeps its scroll target; a + // tab opened at a line reopens at that line, not at the top. + const revealLine = + fromSurface.kind === "file" && fromSurface.revealLine !== null + ? fromSurface.revealLine + : undefined; + store.openFile(threadRef, to, revealLine); // Renaming a background tab must not steal focus from the // file the user is looking at. if (previousActiveId !== null && previousActiveId !== `file:${from}`) { store.activateSurface(threadRef, previousActiveId); } + // The rename RPC can finish after the user closed the panel; + // swapping the surface must not override that close. + if (!wasPanelOpen) { + store.close(threadRef); + } }} /> From a258b873fc9d2fa3c4af471cc977512bb5552758 Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 14:43:36 +0800 Subject: [PATCH 49/54] fix(web): drop in-flight save results once reset replaces the file --- .../files/fileSaveCoordinator.test.ts | 60 +++++++++++++++++++ .../components/files/fileSaveCoordinator.ts | 17 +++++- 2 files changed, 75 insertions(+), 2 deletions(-) diff --git a/apps/web/src/components/files/fileSaveCoordinator.test.ts b/apps/web/src/components/files/fileSaveCoordinator.test.ts index d550d2f97668..370e4cca979e 100644 --- a/apps/web/src/components/files/fileSaveCoordinator.test.ts +++ b/apps/web/src/components/files/fileSaveCoordinator.test.ts @@ -160,6 +160,66 @@ describe("FileSaveCoordinator", () => { expect(persist).toHaveBeenCalledWith("fresh edit"); }); + it("a save resolving after reset neither confirms nor blocks later edits", async () => { + vi.useFakeTimers(); + const inFlight = deferred(); + const persist = vi + .fn<(contents: string) => Promise>>() + .mockReturnValueOnce(inFlight.promise) + .mockResolvedValueOnce(AsyncResult.success(undefined)); + const onConfirmed = vi.fn(); + const coordinator = new FileSaveCoordinator({ + debounceMs: 500, + persist, + onPendingChange: vi.fn(), + onConfirmed, + }); + + coordinator.change("pre-upload snapshot"); + await vi.advanceTimersByTimeAsync(500); + expect(persist).toHaveBeenCalledOnce(); + + coordinator.reset(); + inFlight.resolve(AsyncResult.success(undefined)); + await vi.runAllTimersAsync(); + expect(onConfirmed).not.toHaveBeenCalled(); + + coordinator.change("post-upload edit"); + await vi.runAllTimersAsync(); + expect(persist).toHaveBeenCalledTimes(2); + expect(persist).toHaveBeenLastCalledWith("post-upload edit"); + expect(onConfirmed).toHaveBeenCalledWith("post-upload edit"); + }); + + it("an edit made after reset saves once the stale write resolves", async () => { + vi.useFakeTimers(); + const inFlight = deferred(); + const persist = vi + .fn<(contents: string) => Promise>>() + .mockReturnValueOnce(inFlight.promise) + .mockResolvedValueOnce(AsyncResult.success(undefined)); + const onConfirmed = vi.fn(); + const coordinator = new FileSaveCoordinator({ + debounceMs: 500, + persist, + onPendingChange: vi.fn(), + onConfirmed, + }); + + coordinator.change("pre-upload snapshot"); + await vi.advanceTimersByTimeAsync(500); + expect(persist).toHaveBeenCalledOnce(); + + coordinator.reset(); + coordinator.change("edited while the stale write was in flight"); + inFlight.resolve(AsyncResult.success(undefined)); + await vi.runAllTimersAsync(); + expect(persist).toHaveBeenCalledTimes(2); + expect(persist).toHaveBeenLastCalledWith("edited while the stale write was in flight"); + expect(onConfirmed).toHaveBeenCalledOnce(); + expect(onConfirmed).toHaveBeenCalledWith("edited while the stale write was in flight"); + }); + it("suspend holds saves and resume persists the held edits", 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 29731f18efd5..b92ca4c4bbfe 100644 --- a/apps/web/src/components/files/fileSaveCoordinator.ts +++ b/apps/web/src/components/files/fileSaveCoordinator.ts @@ -16,6 +16,7 @@ export class FileSaveCoordinator { private saving = false; private disposed = false; private suspendCount = 0; + private generation = 0; constructor(private readonly options: FileSaveCoordinatorOptions) {} @@ -48,6 +49,10 @@ export class FileSaveCoordinator { * where the surface reloads the new contents and editing continues. */ reset(): void { + // A save already on the wire belongs to the replaced file; bumping the + // generation makes its completion drop the result instead of advancing + // the zeroed watermark or confirming stale contents. + this.generation += 1; this.suspendCount = 0; this.clearTimer(); this.latestRevision = 0; @@ -101,15 +106,23 @@ export class FileSaveCoordinator { this.saving = true; const contents = this.latestContents; const revision = this.latestRevision; + const generation = this.generation; const result = await this.options.persist(contents); - const succeeded = result._tag === "Success"; + // A reset while the write was on the wire replaced the file under this + // snapshot: the result describes bytes that no longer exist, so it must + // not advance the watermark or confirm the pre-replacement contents. + const stale = generation !== this.generation; + const succeeded = !stale && result._tag === "Success"; if (succeeded) { this.persistedRevision = revision; this.options.onConfirmed(contents); } this.saving = false; - if (revision === this.latestRevision) { + if (stale && this.latestRevision <= this.persistedRevision) { + return; + } + if (!stale && revision === this.latestRevision) { if (succeeded) this.options.onPendingChange(false); return; } From 13f02b6936065197845bafb331dff39cc4caf057 Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 14:56:34 +0800 Subject: [PATCH 50/54] fix(web): keep overlapping mutation holds when reset settles one --- .../files/fileSaveCoordinator.test.ts | 25 +++++++++++++++++++ .../components/files/fileSaveCoordinator.ts | 5 +++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/apps/web/src/components/files/fileSaveCoordinator.test.ts b/apps/web/src/components/files/fileSaveCoordinator.test.ts index 370e4cca979e..25d383df5aa4 100644 --- a/apps/web/src/components/files/fileSaveCoordinator.test.ts +++ b/apps/web/src/components/files/fileSaveCoordinator.test.ts @@ -339,6 +339,31 @@ describe("FileSaveCoordinator", () => { expect(persist).toHaveBeenCalledWith("held"); }); + it("reset releases only the settling mutation's hold", async () => { + vi.useFakeTimers(); + const persist = vi + .fn<(contents: string) => Promise>>() + .mockResolvedValue(AsyncResult.success(undefined)); + const coordinator = new FileSaveCoordinator({ + debounceMs: 500, + persist, + onPendingChange: vi.fn(), + onConfirmed: vi.fn(), + }); + + coordinator.suspend(); + coordinator.suspend(); + coordinator.reset(); + coordinator.change("edited while the other mutation still writes"); + await vi.advanceTimersByTimeAsync(5_000); + expect(persist).not.toHaveBeenCalled(); + + coordinator.resume(); + await vi.runAllTimersAsync(); + expect(persist).toHaveBeenCalledOnce(); + expect(persist).toHaveBeenCalledWith("edited while the other mutation still writes"); + }); + it("dispose while suspended does not flush behind a pending mutation", 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 b92ca4c4bbfe..b7b84c2b5bf0 100644 --- a/apps/web/src/components/files/fileSaveCoordinator.ts +++ b/apps/web/src/components/files/fileSaveCoordinator.ts @@ -53,7 +53,10 @@ export class FileSaveCoordinator { // generation makes its completion drop the result instead of advancing // the zeroed watermark or confirming stale contents. this.generation += 1; - this.suspendCount = 0; + // Only the settling mutation's own hold is released; an overlapping + // mutation still writing keeps its hold, so a later edit cannot save + // mid-mutation and overwrite that writer's result. + if (this.suspendCount > 0) this.suspendCount -= 1; this.clearTimer(); this.latestRevision = 0; this.persistedRevision = 0; From 2364ae108f5b61c9631e05d1edb0bab59b1089ee Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 17:00:36 +0800 Subject: [PATCH 51/54] test(server): stage rival rename fixtures Creating the rival after deleting the claim lets ext4 recycle the claim inode and model a state that the real staged rename cannot produce. --- apps/server/src/workspace/WorkspaceFileSystem.test.ts | 9 ++++----- apps/server/src/workspace/WorkspaceUpload.test.ts | 9 ++++----- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/apps/server/src/workspace/WorkspaceFileSystem.test.ts b/apps/server/src/workspace/WorkspaceFileSystem.test.ts index 84e848d7957c..8ed790b89abb 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.test.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.test.ts @@ -780,11 +780,10 @@ const brokenRenameFileSystemLayer = Layer.effect( rename: (_oldPath, newPath) => Effect.sync(() => { const rival = rivalBytesOnRename.current; - if (rival) { - // A real overwrite renames the rival's staged part onto the - // target, replacing the inode; remove-then-write reproduces that. - NodeFS.rmSync(newPath, { force: true }); - NodeFS.writeFileSync(newPath, rival); + if (rival !== null) { + const rivalPath = `${newPath}.rival`; + NodeFS.writeFileSync(rivalPath, rival, { flag: "wx" }); + NodeFS.renameSync(rivalPath, newPath); } }).pipe( Effect.andThen( diff --git a/apps/server/src/workspace/WorkspaceUpload.test.ts b/apps/server/src/workspace/WorkspaceUpload.test.ts index b61359836f7b..8f622d53c09d 100644 --- a/apps/server/src/workspace/WorkspaceUpload.test.ts +++ b/apps/server/src/workspace/WorkspaceUpload.test.ts @@ -536,11 +536,10 @@ const brokenRenameFileSystemLayer = Layer.effect( rename: (_fromPath, toPath) => Effect.sync(() => { const rival = rivalBytesOnRename.current; - if (rival) { - // A real overwrite renames the rival's staged part onto the - // target, replacing the inode; remove-then-write reproduces that. - NodeFS.rmSync(toPath, { force: true }); - NodeFS.writeFileSync(toPath, rival); + if (rival !== null) { + const rivalPath = `${toPath}.rival`; + NodeFS.writeFileSync(rivalPath, rival, { flag: "wx" }); + NodeFS.renameSync(rivalPath, toPath); } }).pipe( Effect.andThen( From 10d81362c0ef3d1f3505bb0e91518568274c6255 Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 17:00:48 +0800 Subject: [PATCH 52/54] fix(server): type lstat failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Keep the original lstat cause inside Effect tryPromise’s tagged unknown error so ENOENT remains recoverable without an untagged error channel. --- apps/server/src/workspace/WorkspaceFileSystem.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/apps/server/src/workspace/WorkspaceFileSystem.ts b/apps/server/src/workspace/WorkspaceFileSystem.ts index 983f335673e3..62066cdaeb24 100644 --- a/apps/server/src/workspace/WorkspaceFileSystem.ts +++ b/apps/server/src/workspace/WorkspaceFileSystem.ts @@ -621,15 +621,16 @@ export const make = Effect.gen(function* () { // lstat examines the directory entry itself, so a dangling symlink is // still found and removed; stat would follow it, read NotFound, and // report success while the entry stays on disk. - const targetStat = yield* Effect.tryPromise({ - try: () => NodeFSP.lstat(target.absolutePath), - catch: (cause) => cause as NodeJS.ErrnoException, - }).pipe( + const targetStat = yield* Effect.tryPromise(() => NodeFSP.lstat(target.absolutePath)).pipe( Effect.catchIf( - (error) => error.code === "ENOENT", + (error) => + typeof error.cause === "object" && + error.cause !== null && + "code" in error.cause && + error.cause.code === "ENOENT", () => Effect.succeed(null), ), - Effect.mapError((cause) => deleteError("resolve-path", cause)), + Effect.mapError((error) => deleteError("resolve-path", error.cause)), ); if (targetStat === null) { return; From 90e95275ece3ea20fd694ae52508e0979666b778 Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 19:34:26 +0800 Subject: [PATCH 53/54] fix(web): a lost upload race prompts to replace instead of failing --- apps/web/src/lib/uploadXhr.ts | 9 +- apps/web/src/lib/workspaceUploadQueue.test.ts | 58 ++++++ apps/web/src/lib/workspaceUploadQueue.ts | 175 ++++++++++-------- 3 files changed, 168 insertions(+), 74 deletions(-) diff --git a/apps/web/src/lib/uploadXhr.ts b/apps/web/src/lib/uploadXhr.ts index 871fce25af3f..8e6986660ac8 100644 --- a/apps/web/src/lib/uploadXhr.ts +++ b/apps/web/src/lib/uploadXhr.ts @@ -1,3 +1,10 @@ +/** A non-2xx response; carries the status so callers can react to specific codes. */ +export class UploadRejectedError extends Error { + constructor(readonly status: number) { + super(`Upload rejected (${status})`); + } +} + export function uploadXhr(input: { readonly url: string; readonly file: File; @@ -21,7 +28,7 @@ export function uploadXhr(input: { if (xhr.status >= 200 && xhr.status < 300) { resolve(); } else { - reject(new Error(`Upload rejected (${xhr.status})`)); + reject(new UploadRejectedError(xhr.status)); } }); xhr.addEventListener("error", () => reject(new Error("Upload failed"))); diff --git a/apps/web/src/lib/workspaceUploadQueue.test.ts b/apps/web/src/lib/workspaceUploadQueue.test.ts index 29e7ba3f7d66..a918a86c966b 100644 --- a/apps/web/src/lib/workspaceUploadQueue.test.ts +++ b/apps/web/src/lib/workspaceUploadQueue.test.ts @@ -273,6 +273,64 @@ describe("workspaceUploadQueue", () => { expect(findUpload("existing.txt")).toBeUndefined(); }); + it("prompts to replace and retries with overwrite when the commit loses the race with 409", async () => { + const seenOverwrites: Array = []; + mocks.runAtomCommand.mockImplementation( + async ( + _registry: unknown, + command: unknown, + target: { + readonly input: { readonly relativePath: string; readonly overwrite?: boolean }; + }, + ) => { + if (command !== mocks.createUploadUrl) throw new Error("unexpected command"); + seenOverwrites.push(target.input.overwrite); + return mintedResult(target.input.relativePath); + }, + ); + mocks.requestConfirmDialog.mockResolvedValue(true); + + const file = makeFile("raced.txt"); + startWorkspaceUploads({ environmentId, cwd, files: [file], onUploaded: vi.fn() }); + await Promise.resolve(); + await Promise.resolve(); + + TestXmlHttpRequest.requests[0]!.complete(409); + for (let tick = 0; tick < 6; tick += 1) await Promise.resolve(); + + expect(mocks.requestConfirmDialog).toHaveBeenCalledWith( + "Replace raced.txt?\nA file named 'raced.txt' already exists in this project.", + { variant: "destructive" }, + ); + expect(seenOverwrites).toEqual([undefined, true]); + expect(TestXmlHttpRequest.requests).toHaveLength(2); + expect(findUpload("raced.txt")![1]).toMatchObject({ status: "uploading" }); + + TestXmlHttpRequest.requests[1]!.complete(); + for (let tick = 0; tick < 4; tick += 1) await Promise.resolve(); + + expect(findUpload("raced.txt")).toBeUndefined(); + }); + + it("marks the entry failed when the user declines to replace after a commit-time 409", async () => { + mocks.requestConfirmDialog.mockResolvedValue(false); + + const file = makeFile("raced.txt"); + startWorkspaceUploads({ environmentId, cwd, files: [file], onUploaded: vi.fn() }); + await Promise.resolve(); + await Promise.resolve(); + + TestXmlHttpRequest.requests[0]!.complete(409); + for (let tick = 0; tick < 6; tick += 1) await Promise.resolve(); + + expect(mocks.requestConfirmDialog).toHaveBeenCalledTimes(1); + expect(TestXmlHttpRequest.requests).toHaveLength(1); + expect(findUpload("raced.txt")![1]).toMatchObject({ + status: "failed", + reason: "File already exists", + }); + }); + it("marks the entry failed with 'File already exists' when the user declines to replace", async () => { mocks.runAtomCommand.mockImplementation( async ( diff --git a/apps/web/src/lib/workspaceUploadQueue.ts b/apps/web/src/lib/workspaceUploadQueue.ts index 7a451ee5d3e7..fd40e79e4d1c 100644 --- a/apps/web/src/lib/workspaceUploadQueue.ts +++ b/apps/web/src/lib/workspaceUploadQueue.ts @@ -9,7 +9,7 @@ import { appAtomRegistry } from "../rpc/atomRegistry"; import { projectEnvironment } from "../state/projects"; import { readPreparedConnection } from "../state/session"; import { randomUUID } from "./utils"; -import { uploadXhr } from "./uploadXhr"; +import { UploadRejectedError, uploadXhr } from "./uploadXhr"; const MAX_UPLOADS_PER_ENVIRONMENT = 3; // Matches the upload token TTL (see PROJECT_UPLOAD_URL_TTL_MS), since @@ -105,97 +105,126 @@ function mintUploadUrl(job: UploadJob) { ); } -async function runUpload(job: UploadJob): Promise { - let minted = await mintUploadUrl(job); - if (job.cancelled) { - jobsById.delete(job.id); - return; - } - - if (minted._tag !== "Success") { - if (!isProjectUploadTargetExistsError(Cause.squash(minted.cause))) { - failJob(job, "Upload could not start"); - return; - } - - const confirmed = await readLocalApi()?.dialogs.confirm( +function confirmReplace(job: UploadJob): Promise { + return Promise.resolve( + readLocalApi()?.dialogs.confirm( `Replace ${job.relativePath}?\nA file named '${job.relativePath}' already exists in this project.`, { variant: "destructive" }, - ); - if (job.cancelled) { - jobsById.delete(job.id); - return; - } - if (confirmed !== true) { - failJob(job, "File already exists"); - return; - } + ), + ); +} - job.overwrite = true; - minted = await mintUploadUrl(job); +// The mint-time existence check is only advisory: a rival upload can land +// between it and the server's atomic commit, which then answers 409. Both +// conflict stages funnel into the same replace confirmation, and the overwrite +// flag bounds the loop to one retry. +async function runUpload(job: UploadJob): Promise { + for (;;) { + let minted = await mintUploadUrl(job); if (job.cancelled) { jobsById.delete(job.id); return; } + if (minted._tag !== "Success") { - failJob(job, "Upload could not start"); - return; - } - } + if (!isProjectUploadTargetExistsError(Cause.squash(minted.cause))) { + failJob(job, "Upload could not start"); + return; + } - const connection = readPreparedConnection(job.environmentId); - const url = connection ? resolveAssetUrl(connection.httpBaseUrl, minted.value.relativeUrl) : null; - if (!url) { - failJob(job, "Not connected"); - return; - } + const confirmed = await confirmReplace(job); + if (job.cancelled) { + jobsById.delete(job.id); + return; + } + if (confirmed !== true) { + failJob(job, "File already exists"); + return; + } - let lastStep = -1; - const upload = uploadXhr({ - url, - file: job.file, - timeoutMs: UPLOAD_TIMEOUT_MS, - onProgress: (progress) => { - const step = Math.floor(progress * 20); - if (step === lastStep || job.cancelled) { + job.overwrite = true; + minted = await mintUploadUrl(job); + if (job.cancelled) { + jobsById.delete(job.id); return; } - lastStep = step; - setUploadState(job.id, { - status: "uploading", - name: job.file.name, - relativePath: job.relativePath, - environmentId: job.environmentId, - cwd: job.cwd, - progress, - }); - }, - }); - job.abort = upload.abort; + if (minted._tag !== "Success") { + failJob(job, "Upload could not start"); + return; + } + } - try { - await upload.done; - if (job.cancelled) { - jobsById.delete(job.id); + const connection = readPreparedConnection(job.environmentId); + const url = connection + ? resolveAssetUrl(connection.httpBaseUrl, minted.value.relativeUrl) + : null; + if (!url) { + failJob(job, "Not connected"); return; } - jobsById.delete(job.id); - clearUploadState(job.id); + + let lastStep = -1; + const upload = uploadXhr({ + url, + file: job.file, + timeoutMs: UPLOAD_TIMEOUT_MS, + onProgress: (progress) => { + const step = Math.floor(progress * 20); + if (step === lastStep || job.cancelled) { + return; + } + lastStep = step; + setUploadState(job.id, { + status: "uploading", + name: job.file.name, + relativePath: job.relativePath, + environmentId: job.environmentId, + cwd: job.cwd, + progress, + }); + }, + }); + job.abort = upload.abort; + try { - job.onUploaded(job.relativePath); - } catch (error) { - // The upload itself succeeded; a throwing refresh callback must not - // resurrect the cleared entry as an unretryable failure. - console.error(error); - } - } catch (error) { - if (job.cancelled) { + await upload.done; + if (job.cancelled) { + jobsById.delete(job.id); + return; + } jobsById.delete(job.id); + clearUploadState(job.id); + try { + job.onUploaded(job.relativePath); + } catch (error) { + // The upload itself succeeded; a throwing refresh callback must not + // resurrect the cleared entry as an unretryable failure. + console.error(error); + } + return; + } catch (error) { + if (job.cancelled) { + jobsById.delete(job.id); + return; + } + if (error instanceof UploadRejectedError && error.status === 409 && !job.overwrite) { + const confirmed = await confirmReplace(job); + if (job.cancelled) { + jobsById.delete(job.id); + return; + } + if (confirmed !== true) { + failJob(job, "File already exists"); + return; + } + job.overwrite = true; + continue; + } + failJob(job, error instanceof Error ? error.message : "Upload failed"); return; + } finally { + job.abort = null; } - failJob(job, error instanceof Error ? error.message : "Upload failed"); - } finally { - job.abort = null; } } From b83839e05410c1e8d4c5e776ab65235c3e1946ca Mon Sep 17 00:00:00 2001 From: Mark S <49048586+msegec@users.noreply.github.com> Date: Tue, 25 Aug 2026 19:34:26 +0800 Subject: [PATCH 54/54] fix(web): a lost upload race prompts to replace instead of failing --- apps/web/src/lib/uploadXhr.ts | 9 +- apps/web/src/lib/workspaceUploadQueue.test.ts | 58 ++++++ apps/web/src/lib/workspaceUploadQueue.ts | 193 +++++++++++------- 3 files changed, 181 insertions(+), 79 deletions(-) diff --git a/apps/web/src/lib/uploadXhr.ts b/apps/web/src/lib/uploadXhr.ts index 871fce25af3f..8e6986660ac8 100644 --- a/apps/web/src/lib/uploadXhr.ts +++ b/apps/web/src/lib/uploadXhr.ts @@ -1,3 +1,10 @@ +/** A non-2xx response; carries the status so callers can react to specific codes. */ +export class UploadRejectedError extends Error { + constructor(readonly status: number) { + super(`Upload rejected (${status})`); + } +} + export function uploadXhr(input: { readonly url: string; readonly file: File; @@ -21,7 +28,7 @@ export function uploadXhr(input: { if (xhr.status >= 200 && xhr.status < 300) { resolve(); } else { - reject(new Error(`Upload rejected (${xhr.status})`)); + reject(new UploadRejectedError(xhr.status)); } }); xhr.addEventListener("error", () => reject(new Error("Upload failed"))); diff --git a/apps/web/src/lib/workspaceUploadQueue.test.ts b/apps/web/src/lib/workspaceUploadQueue.test.ts index 776f71eece7f..13ed09f62e3f 100644 --- a/apps/web/src/lib/workspaceUploadQueue.test.ts +++ b/apps/web/src/lib/workspaceUploadQueue.test.ts @@ -273,6 +273,64 @@ describe("workspaceUploadQueue", () => { expect(findUpload("existing.txt")).toBeUndefined(); }); + it("prompts to replace and retries with overwrite when the commit loses the race with 409", async () => { + const seenOverwrites: Array = []; + mocks.runAtomCommand.mockImplementation( + async ( + _registry: unknown, + command: unknown, + target: { + readonly input: { readonly relativePath: string; readonly overwrite?: boolean }; + }, + ) => { + if (command !== mocks.createUploadUrl) throw new Error("unexpected command"); + seenOverwrites.push(target.input.overwrite); + return mintedResult(target.input.relativePath); + }, + ); + mocks.requestConfirmDialog.mockResolvedValue(true); + + const file = makeFile("raced.txt"); + startWorkspaceUploads({ environmentId, cwd, files: [file], onUploaded: vi.fn() }); + await Promise.resolve(); + await Promise.resolve(); + + TestXmlHttpRequest.requests[0]!.complete(409); + for (let tick = 0; tick < 6; tick += 1) await Promise.resolve(); + + expect(mocks.requestConfirmDialog).toHaveBeenCalledWith( + "Replace raced.txt?\nA file named 'raced.txt' already exists in this project.", + { variant: "destructive" }, + ); + expect(seenOverwrites).toEqual([undefined, true]); + expect(TestXmlHttpRequest.requests).toHaveLength(2); + expect(findUpload("raced.txt")![1]).toMatchObject({ status: "uploading" }); + + TestXmlHttpRequest.requests[1]!.complete(); + for (let tick = 0; tick < 4; tick += 1) await Promise.resolve(); + + expect(findUpload("raced.txt")).toBeUndefined(); + }); + + it("marks the entry failed when the user declines to replace after a commit-time 409", async () => { + mocks.requestConfirmDialog.mockResolvedValue(false); + + const file = makeFile("raced.txt"); + startWorkspaceUploads({ environmentId, cwd, files: [file], onUploaded: vi.fn() }); + await Promise.resolve(); + await Promise.resolve(); + + TestXmlHttpRequest.requests[0]!.complete(409); + for (let tick = 0; tick < 6; tick += 1) await Promise.resolve(); + + expect(mocks.requestConfirmDialog).toHaveBeenCalledTimes(1); + expect(TestXmlHttpRequest.requests).toHaveLength(1); + expect(findUpload("raced.txt")![1]).toMatchObject({ + status: "failed", + reason: "File already exists", + }); + }); + it("marks the entry failed with 'File already exists' when the user declines to replace", async () => { mocks.runAtomCommand.mockImplementation( async ( diff --git a/apps/web/src/lib/workspaceUploadQueue.ts b/apps/web/src/lib/workspaceUploadQueue.ts index a80c6ff17423..0dd7f0d0a0af 100644 --- a/apps/web/src/lib/workspaceUploadQueue.ts +++ b/apps/web/src/lib/workspaceUploadQueue.ts @@ -9,7 +9,7 @@ import { appAtomRegistry } from "../rpc/atomRegistry"; import { projectEnvironment } from "../state/projects"; import { readPreparedConnection } from "../state/session"; import { randomUUID } from "./utils"; -import { uploadXhr } from "./uploadXhr"; +import { UploadRejectedError, uploadXhr } from "./uploadXhr"; const MAX_UPLOADS_PER_ENVIRONMENT = 3; // Matches the upload token TTL (see PROJECT_UPLOAD_URL_TTL_MS), since @@ -108,103 +108,140 @@ function mintUploadUrl(job: UploadJob) { ); } -async function runUpload(job: UploadJob): Promise { - let minted = await mintUploadUrl(job); - if (job.cancelled) { - jobsById.delete(job.id); +function confirmReplace(job: UploadJob): Promise { + return Promise.resolve( + readLocalApi()?.dialogs.confirm( + `Replace ${job.relativePath}?\nA file named '${job.relativePath}' already exists in this project.`, + { variant: "destructive" }, + ), + ); +} + +// The overwrite phase begins at conflict discovery rather than at +// confirmation: a debounced save could otherwise fire while the confirm +// dialog is open and land after the upload replaces the bytes. +function startOverwritePhase(job: UploadJob): void { + if (job.overwriteStarted) { return; } + job.overwriteStarted = true; + job.onOverwriteStart?.(job.relativePath); +} - if (minted._tag !== "Success") { - if (!isProjectUploadTargetExistsError(Cause.squash(minted.cause))) { - failJob(job, "Upload could not start"); - return; - } - - // The overwrite phase begins at conflict discovery rather than at - // confirmation: a debounced save could otherwise fire while the confirm - // dialog is open and land after the upload replaces the bytes. - job.overwriteStarted = true; - job.onOverwriteStart?.(job.relativePath); - - const confirmed = await readLocalApi()?.dialogs.confirm( - `Replace ${job.relativePath}?\nA file named '${job.relativePath}' already exists in this project.`, - { variant: "destructive" }, - ); +// The mint-time existence check is only advisory: a rival upload can land +// between it and the server's atomic commit, which then answers 409. Both +// conflict stages funnel into the same replace confirmation, and the overwrite +// flag bounds the loop to one retry. +async function runUpload(job: UploadJob): Promise { + for (;;) { + let minted = await mintUploadUrl(job); if (job.cancelled) { jobsById.delete(job.id); return; } - if (confirmed !== true) { - failJob(job, "File already exists"); - return; - } - job.overwrite = true; - minted = await mintUploadUrl(job); - if (job.cancelled) { - jobsById.delete(job.id); - return; - } if (minted._tag !== "Success") { - failJob(job, "Upload could not start"); - return; - } - } + if (!isProjectUploadTargetExistsError(Cause.squash(minted.cause))) { + failJob(job, "Upload could not start"); + return; + } - const connection = readPreparedConnection(job.environmentId); - const url = connection ? resolveAssetUrl(connection.httpBaseUrl, minted.value.relativeUrl) : null; - if (!url) { - failJob(job, "Not connected"); - return; - } + startOverwritePhase(job); - let lastStep = -1; - const upload = uploadXhr({ - url, - file: job.file, - timeoutMs: UPLOAD_TIMEOUT_MS, - onProgress: (progress) => { - const step = Math.floor(progress * 20); - if (step === lastStep || job.cancelled) { + const confirmed = await confirmReplace(job); + if (job.cancelled) { + jobsById.delete(job.id); + return; + } + if (confirmed !== true) { + failJob(job, "File already exists"); return; } - lastStep = step; - setUploadState(job.id, { - status: "uploading", - name: job.file.name, - relativePath: job.relativePath, - environmentId: job.environmentId, - cwd: job.cwd, - progress, - }); - }, - }); - job.abort = upload.abort; - try { - await upload.done; - if (job.cancelled) { - jobsById.delete(job.id); + job.overwrite = true; + minted = await mintUploadUrl(job); + if (job.cancelled) { + jobsById.delete(job.id); + return; + } + if (minted._tag !== "Success") { + failJob(job, "Upload could not start"); + return; + } + } + + const connection = readPreparedConnection(job.environmentId); + const url = connection + ? resolveAssetUrl(connection.httpBaseUrl, minted.value.relativeUrl) + : null; + if (!url) { + failJob(job, "Not connected"); return; } - jobsById.delete(job.id); - clearUploadState(job.id); + + let lastStep = -1; + const upload = uploadXhr({ + url, + file: job.file, + timeoutMs: UPLOAD_TIMEOUT_MS, + onProgress: (progress) => { + const step = Math.floor(progress * 20); + if (step === lastStep || job.cancelled) { + return; + } + lastStep = step; + setUploadState(job.id, { + status: "uploading", + name: job.file.name, + relativePath: job.relativePath, + environmentId: job.environmentId, + cwd: job.cwd, + progress, + }); + }, + }); + job.abort = upload.abort; + try { - job.onUploaded(job.relativePath); - } catch (error) { - // The upload itself succeeded; a throwing refresh callback must not - // resurrect the cleared entry as an unretryable failure. - console.error(error); - } - } catch (error) { - if (job.cancelled) { + await upload.done; + if (job.cancelled) { + jobsById.delete(job.id); + return; + } jobsById.delete(job.id); + clearUploadState(job.id); + try { + job.onUploaded(job.relativePath); + } catch (error) { + // The upload itself succeeded; a throwing refresh callback must not + // resurrect the cleared entry as an unretryable failure. + console.error(error); + } + return; + } catch (error) { + if (job.cancelled) { + jobsById.delete(job.id); + return; + } + if (error instanceof UploadRejectedError && error.status === 409 && !job.overwrite) { + startOverwritePhase(job); + const confirmed = await confirmReplace(job); + if (job.cancelled) { + jobsById.delete(job.id); + return; + } + if (confirmed !== true) { + failJob(job, "File already exists"); + return; + } + job.overwrite = true; + continue; + } + failJob(job, error instanceof Error ? error.message : "Upload failed"); return; + } finally { + job.abort = null; } - failJob(job, error instanceof Error ? error.message : "Upload failed"); - } finally { - job.abort = null; } }