Files
OpenHands/__tests__/api/conversation-service.test.ts
T
chuckbutkusandopenhands 2ed2c571e6 fix(upload): resolve relative working dirs against /api/file/home (#1106)
* fix(upload): resolve relative working dirs against /api/file/home

The agent-server's /api/file/upload endpoint requires an absolute path
and `mkdir -p`'s the parent. The frontend's `toAbsoluteWorkspacePath`
was naively prepending `/` to the default relative `workspace/project`
working dir, producing `/workspace/project/<hex>/...`. On macOS and
fresh Docker images that path lives under a read-only filesystem root,
so uploads failed with `OSError: [Errno 30] Read-only file system:
'/workspace'`.

Conversations themselves kept working because the agent-server resolves
relative `workspace.working_dir` against its own process CWD (which is
writable in dev), so the worktree landed elsewhere and only uploads
mistargeted the read-only root.

Fix: introduce `getAgentServerHomeDir` (cached per backend, backed by
`FileClient.getHome` → `GET /api/file/home`) and a
`resolveAbsoluteAgentServerPath` helper. Both the conversation-start
payload and the file-upload destination now go through this resolver,
so they always agree on a single absolute path anchored at the
agent-server's home directory (e.g. `~/workspace/project/<hex>`).
Absolute paths pass through unchanged, so explicit workspace selections
and `VITE_WORKING_DIR` overrides are unaffected.

Spec: WUP-001 in specs/workspace-upload-path.md.

Co-authored-by: openhands <openhands@all-hands.dev>

* docs: clarify resolveConversationUploadWorkingDir returns raw (possibly-relative) working dir

Add JSDoc explaining that callers must funnel the result through
buildWorkspaceUploadPath (which calls resolveAbsoluteWorkspacePath) to
get an absolute path for the upload endpoint.  Also document that the
UNC path case is already covered by the existing isAbsolutePath regex.

Addresses review comment on PR #1106.

Co-authored-by: openhands <openhands@all-hands.dev>

* test(e2e): add mock-llm image-upload test

Adds an end-to-end mock-LLM test that exercises the full image-attachment
pipeline:

1. Attaches a minimal 1×1 PNG to the home-page chat via the hidden file
   input (data-testid="upload-image-input") using Playwright's
   setInputFiles.
2. Submits "What is in this image?" — creating a conversation and sending
   the message via sendMessageWithAttachments.
3. Verifies the agent replies with IMAGE_REPLY_TOKEN in the chat UI.
4. Verifies the user MessageEvent in the conversation events API has
   image_urls populated (base64 data: URL).
5. Verifies at least one /v1/chat/completions call to the mock server
   contained an image_url content block — confirming the image was
   forwarded to the LLM as expected.

Supporting changes:
- mock-llm-server.py: add GET /admin/requests endpoint that exposes all
  captured completion request bodies since the last reset; reset also
  clears the history.
- mock-llm-helpers.ts: add getMockLLMRequests(), IMAGE_REPLY_TOKEN,
  and MINIMAL_PNG_BASE64 exports.
- AGENTS.md: document the new endpoint and test spec.

Co-authored-by: openhands <openhands@all-hands.dev>

* test(e2e): add padding response to image-upload trajectory

The agent-server makes one internal LLM call for skill-analysis before
the main agent loop starts. The original 1-response trajectory was
consumed by that internal call, leaving the agent with a 500 error and
retry storm.

Add 1 padding response (turn 0: empty text) + 1 safety buffer (turn 2)
following the same pattern as mock-llm-automation.spec.ts.

Co-authored-by: openhands <openhands@all-hands.dev>

* test(e2e): use gpt-4o model name for vision-capable LLM requests

litellm strips image_url content blocks for unknown model names like
'openai/mock-test-model'. Switch to 'openai/gpt-4o' so litellm knows
the model accepts vision content and includes base64 image_url blocks
in the completion request body.

The base_url still points at the local mock server; the model name is
only a hint to litellm's request formatter.

Also improve assertion diagnostics to print all captured LLM requests
(not just the first) when the assertion fails.

Co-authored-by: openhands <openhands@all-hands.dev>

---------

Co-authored-by: openhands <openhands@all-hands.dev>
2026-06-03 17:47:45 -04:00

144 lines
4.7 KiB
TypeScript

import { describe, expect, it, vi, beforeEach } from "vitest";
import { RemoteWorkspace } from "@openhands/typescript-client/workspace/remote-workspace";
import ConversationService from "#/api/conversation-service/conversation-service.api";
import { clearAgentServerHomeDirCache } from "#/api/agent-server-home";
const fileUploadMock = vi.fn();
const getHomeMock = vi.fn();
vi.mock("@openhands/typescript-client/workspace/remote-workspace", () => ({
RemoteWorkspace: vi.fn(function RemoteWorkspaceMock() {
return { fileUpload: fileUploadMock };
}),
}));
vi.mock("@openhands/typescript-client/clients", () => ({
FileClient: vi.fn(function FileClientMock() {
return { getHome: getHomeMock };
}),
}));
function makeFile(name: string) {
return new File(["content"], name, { type: "text/plain" });
}
describe("ConversationService", () => {
beforeEach(() => {
vi.clearAllMocks();
ConversationService.setCurrentConversation(null);
clearAgentServerHomeDirCache();
getHomeMock.mockResolvedValue({ home: "/Users/agent" });
});
describe("uploadFiles", () => {
// @spec WUP-001 — The default fallback working dir is relative
// (`workspace/project`); the upload path is resolved against the
// agent-server's home directory via /api/file/home.
it("uploads files through RemoteWorkspace and reports successes", async () => {
fileUploadMock.mockResolvedValue(undefined);
const result = await ConversationService.uploadFiles("conv-1", [
makeFile("a.txt"),
makeFile("b.txt"),
]);
expect(RemoteWorkspace).toHaveBeenCalledWith(
expect.objectContaining({
host: expect.any(String),
workingDir: expect.any(String),
}),
);
expect(fileUploadMock).toHaveBeenCalledWith(
expect.objectContaining({ name: "a.txt" }),
"/Users/agent/workspace/project/a.txt",
);
expect(fileUploadMock).toHaveBeenCalledWith(
expect.objectContaining({ name: "b.txt" }),
"/Users/agent/workspace/project/b.txt",
);
expect(result).toEqual({
uploaded_files: ["a.txt", "b.txt"],
skipped_files: [],
});
});
it("uploads using only the basename of user-provided file names", async () => {
fileUploadMock.mockResolvedValue(undefined);
const result = await ConversationService.uploadFiles("conv-1", [
makeFile("../../evil.txt"),
]);
expect(fileUploadMock).toHaveBeenCalledWith(
expect.objectContaining({ name: "../../evil.txt" }),
"/Users/agent/workspace/project/evil.txt",
);
expect(result).toEqual({
uploaded_files: ["evil.txt"],
skipped_files: [],
});
});
it("uploads into the active conversation workspace when set", async () => {
ConversationService.setCurrentConversation({
id: "conv-1",
workspace: { working_dir: "/workspace/project/my-app" },
} as never);
fileUploadMock.mockResolvedValue(undefined);
await ConversationService.uploadFiles("conv-1", [makeFile("doc.txt")]);
expect(fileUploadMock).toHaveBeenCalledWith(
expect.objectContaining({ name: "doc.txt" }),
"/workspace/project/my-app/doc.txt",
);
});
it("uses the current conversation session key and reports per-file failures", async () => {
ConversationService.setCurrentConversation({
id: "conv-1",
session_api_key: "session-key",
} as never);
fileUploadMock
.mockResolvedValueOnce(undefined)
.mockRejectedValueOnce(new Error("too large"));
const result = await ConversationService.uploadFiles("conv-1", [
makeFile("ok.txt"),
makeFile("bad.txt"),
]);
expect(RemoteWorkspace).toHaveBeenCalledWith(
expect.objectContaining({ apiKey: "session-key" }),
);
expect(result).toEqual({
uploaded_files: ["ok.txt"],
skipped_files: [{ name: "bad.txt", reason: "too large" }],
});
});
it("uploads files in bounded batches", async () => {
let activeUploads = 0;
let maxActiveUploads = 0;
fileUploadMock.mockImplementation(async () => {
activeUploads += 1;
maxActiveUploads = Math.max(maxActiveUploads, activeUploads);
await Promise.resolve();
activeUploads -= 1;
});
const files = Array.from({ length: 7 }, (_, index) =>
makeFile(`file-${String(index)}.txt`),
);
const result = await ConversationService.uploadFiles("conv-1", files);
expect(fileUploadMock).toHaveBeenCalledTimes(7);
expect(maxActiveUploads).toBe(5);
expect(result.uploaded_files).toEqual(files.map((file) => file.name));
expect(result.skipped_files).toEqual([]);
});
});
});