From 7f42eb98572fd62fb909d1e6ff571ef69a713d6e Mon Sep 17 00:00:00 2001 From: Hiep Le <69354317+hieptl@users.noreply.github.com> Date: Wed, 10 Jun 2026 20:46:11 +0700 Subject: [PATCH] chore: remove dead upload-path remnants (#1236) --- __tests__/api/workspace-upload-path.test.ts | 11 -- specs/workspace-upload-path.md | 2 +- src/api/workspace-upload-path.ts | 15 --- .../mutation/use-conversation-upload-files.ts | 104 ------------------ 4 files changed, 1 insertion(+), 131 deletions(-) delete mode 100644 src/hooks/mutation/use-conversation-upload-files.ts diff --git a/__tests__/api/workspace-upload-path.test.ts b/__tests__/api/workspace-upload-path.test.ts index 76cebd2d63..b1797d82e9 100644 --- a/__tests__/api/workspace-upload-path.test.ts +++ b/__tests__/api/workspace-upload-path.test.ts @@ -4,7 +4,6 @@ import { getSafeUploadFileName, resolveAbsoluteWorkspacePath, resolveConversationUploadWorkingDir, - toAbsoluteWorkspacePath, } from "#/api/workspace-upload-path"; import { clearAgentServerHomeDirCache } from "#/api/agent-server-home"; @@ -42,16 +41,6 @@ beforeEach(() => { }); describe("workspace-upload-path", () => { - // @spec WUP-001 — legacy helper kept for cosmetic uses only. - it("toAbsoluteWorkspacePath is a naive `/`-prefix and stays sync", () => { - expect(toAbsoluteWorkspacePath("workspace/project")).toBe( - "/workspace/project", - ); - expect(toAbsoluteWorkspacePath("/already/absolute")).toBe( - "/already/absolute", - ); - }); - // @spec WUP-001 — resolver anchors relative paths against /api/file/home. it("resolveAbsoluteWorkspacePath joins relative dirs to the agent-server home", async () => { const resolved = await resolveAbsoluteWorkspacePath("workspace/project"); diff --git a/specs/workspace-upload-path.md b/specs/workspace-upload-path.md index 37b46d7621..ebcf480d09 100644 --- a/specs/workspace-upload-path.md +++ b/specs/workspace-upload-path.md @@ -8,7 +8,7 @@ - [x] When the working dir is already absolute (e.g. POSIX `/foo`, Windows `C:\foo`, or the explicit selection from `search_subdirs`), the resolver shall pass it through unchanged. - [x] The home-directory lookup shall be cached per backend host so concurrent uploads share a single in-flight `/api/file/home` request, and a cached value is reused for subsequent uploads. - [x] A failed lookup shall not be cached so the next call retries fresh. -- [x] The legacy `toAbsoluteWorkspacePath` helper that naively prepends `/` shall remain only for cosmetic/log use; it shall not be used to construct upload paths. +- [x] Upload paths shall never be constructed by naively prepending `/`; the legacy `toAbsoluteWorkspacePath` helper that did so was removed once its last callers disappeared (recoverable from git history), and the home-anchored resolver is the only sanctioned mechanism. ### Why this exists - The agent-server's `/api/file/upload` endpoint requires an absolute path and `mkdir -p`s the parent of the destination. Naively prepending `/` to the default `workspace/project/` produces `/workspace/project/`. On macOS and on fresh Docker images that mount only `/home/` as writable, the filesystem root is read-only, so the upload fails with `OSError: [Errno 30] Read-only file system: '/workspace'`. diff --git a/src/api/workspace-upload-path.ts b/src/api/workspace-upload-path.ts index 4548b734db..7708af21a1 100644 --- a/src/api/workspace-upload-path.ts +++ b/src/api/workspace-upload-path.ts @@ -23,21 +23,6 @@ export function getSafeUploadFileName(fileName: string): string { return safeName; } -/** - * @deprecated The old `prepend `/` if not already absolute` behaviour is the - * root cause of #XXXX (relative `workspace/project` → `/workspace/project`, - * which is on a read-only mount on macOS / fresh containers). Callers that - * need an actual absolute path must use {@link resolveAbsoluteWorkspacePath} - * so the relative leg is anchored against `/api/file/home` instead. - * - * Kept for the rare callers that genuinely just want a leading-slash - * normaliser on a value already known to be agent-server-rooted (e.g. - * cosmetic display, log strings). - */ -export function toAbsoluteWorkspacePath(path: string): string { - return path.startsWith("/") ? path : `/${path}`; -} - /** * Resolve `workingDir` to an absolute path the agent-server's file APIs * accept. Relative paths are joined against `/api/file/home` (cached per diff --git a/src/hooks/mutation/use-conversation-upload-files.ts b/src/hooks/mutation/use-conversation-upload-files.ts deleted file mode 100644 index 59df9d0624..0000000000 --- a/src/hooks/mutation/use-conversation-upload-files.ts +++ /dev/null @@ -1,104 +0,0 @@ -import { useMutation } from "@tanstack/react-query"; -import { RemoteWorkspace } from "@openhands/typescript-client/workspace/remote-workspace"; -import { getAgentServerClientOptions } from "#/api/agent-server-client-options"; -import { - buildWorkspaceUploadPath, - getSafeUploadFileName, -} from "#/api/workspace-upload-path"; -import { FileUploadSuccessResponse } from "#/api/open-hands.types"; - -interface UploadFilesVariables { - conversationUrl: string | null | undefined; - sessionApiKey: string | null | undefined; - workingDir: string; - files: File[]; -} - -/** - * Hook to upload multiple files in parallel to V1 conversations - * Uploads files concurrently using Promise.allSettled and aggregates results - * - * @returns Mutation hook with mutateAsync function - */ -export const useConversationUploadFiles = () => - useMutation({ - mutationKey: ["v1-upload-files"], - mutationFn: async ( - variables: UploadFilesVariables, - ): Promise => { - const { conversationUrl, sessionApiKey, workingDir, files } = variables; - - const uploadPromises = files.map(async (file) => { - // @spec WUP-001 — Resolve once so both the success and failure - // branches report the same absolute path. - let filePath: string; - try { - filePath = await buildWorkspaceUploadPath(file.name, workingDir, { - conversationUrl, - sessionApiKey, - }); - } catch (error) { - return { - success: false as const, - fileName: file.name, - filePath: file.name, - error: error instanceof Error ? error.message : "Unknown error", - }; - } - - try { - const safeName = getSafeUploadFileName(file.name); - await new RemoteWorkspace( - getAgentServerClientOptions({ - conversationUrl, - sessionApiKey, - workingDir, - }), - ).fileUpload(file, filePath); - return { success: true as const, fileName: safeName, filePath }; - } catch (error) { - return { - success: false as const, - fileName: file.name, - filePath, - error: error instanceof Error ? error.message : "Unknown error", - }; - } - }); - - // Wait for all uploads to complete (both successful and failed) - const results = await Promise.allSettled(uploadPromises); - - // Aggregate the results - const uploadedFiles: string[] = []; - const skippedFiles: { name: string; reason: string }[] = []; - - results.forEach((result) => { - if (result.status === "fulfilled") { - if (result.value.success) { - // Return the absolute file path for V1 - uploadedFiles.push(result.value.filePath); - } else { - skippedFiles.push({ - name: result.value.fileName, - reason: result.value.error, - }); - } - } else { - // Promise was rejected (shouldn't happen since we catch errors above) - skippedFiles.push({ - name: "unknown", - reason: result.reason?.message || "Upload failed", - }); - } - }); - - return { - uploaded_files: uploadedFiles, - skipped_files: skippedFiles, - }; - }, - meta: { - disableToast: true, - }, - });