diff --git a/AGENTS.md b/AGENTS.md index 36f8da5dc..a95328f9e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -206,3 +206,5 @@ - Onboarding modal: `src/components/features/onboarding/onboarding-modal.tsx` is a 4-step welcome flow rendered by `` (mounted on the home route) and gated by the `openhands-onboarded` localStorage flag (`use-onboarding-completion.ts`). The four steps live under `steps/`: choose-agent (Step 0 – OpenHands selectable, Claude Code & Codex disabled with a "coming soon" note), check-backend (embeds the new `BackendForm` extracted from `backend-form-modal.tsx` plus a colored connection banner driven by `useBackendsHealth`), setup-llm (renders `` so the existing settings UI keeps owning validation), and say-hello (text input pre-filled from `ONBOARDING$HELLO_DEFAULT_MESSAGE`, launches a no-workspace conversation via `useCreateConversation` and closes the modal). Animation: all four panels are mounted as siblings inside a horizontal rail; advancing/retreating just sets `currentStep`, which translates the rail by `-(step * 100)%` for the slide effect. Progress is rendered by `OnboardingProgressBar` with `data-state` per segment (`completed` | `current` | `upcoming`). When extending, refactor `BackendFormModal` carefully — the inner `BackendForm` is the public surface used both by the modal and by `CheckBackendStep`; the modal version still owns dirty/save tracking so it keeps "Save"/"Cancel" footer behavior. - Worktree policy (this conversation): commits are made on the worktree branch and the user expects the worktree to stay attached to that branch. Do NOT run `git switch --detach` in the worktree and reattach the branch to the main workspace after each commit — only do that when the user explicitly asks. See `~/.openhands/skills/worktree-switch/SKILL.md` for the manual procedure the user invokes. + +- `src/api/typescript-client.ts` now exports `createConversationClient()` and `createFileClient()` factories on top of `resolveClientOptions(overrides)`. Prefer them over hand-written `createHttpClient().get/post/patch/delete` for any agent-server endpoint that the SDK's `ConversationClient` or `FileClient` already covers (PR #291). `SecretsService` (`src/api/secrets-service.ts`) and `cloud/organization-service.api.ts` are still on raw `createHttpClient` because the SDK doesn't ship typed clients for them yet — migrate the SDK first. diff --git a/__tests__/api/agent-server-config.test.ts b/__tests__/api/agent-server-config.test.ts index d2fdd6b28..f3a4b326b 100644 --- a/__tests__/api/agent-server-config.test.ts +++ b/__tests__/api/agent-server-config.test.ts @@ -62,6 +62,8 @@ describe("agent server config", () => { }); it("defaults the working dir to the relative workspace path", () => { + vi.stubEnv("VITE_WORKING_DIR", ""); + expect(getAgentServerWorkingDir()).toBe(DEFAULT_WORKING_DIR); }); diff --git a/__tests__/api/agent-server-conversation-service.test.ts b/__tests__/api/agent-server-conversation-service.test.ts index 573c2eb57..b1fdb5b5e 100644 --- a/__tests__/api/agent-server-conversation-service.test.ts +++ b/__tests__/api/agent-server-conversation-service.test.ts @@ -11,29 +11,51 @@ import AgentServerConversationService from "#/api/conversation-service/agent-ser vi.mock("axios"); const { - mockHttpGet, - mockHttpPost, - mockHttpDelete, + mockCreateConversation, + mockGetConversations, + mockDeleteConversation, + mockDownloadTrajectory, + mockDownloadTextFile, mockFileUpload, - mockCreateHttpClient, + mockCreateConversationClient, + mockCreateFileClient, mockCreateRemoteWorkspace, mockGetSettings, mockGetSettingsForConversation, } = vi.hoisted(() => ({ - mockHttpGet: vi.fn(), - mockHttpPost: vi.fn(), - mockHttpDelete: vi.fn(), + mockCreateConversation: vi.fn(), + mockGetConversations: vi.fn(), + mockDeleteConversation: vi.fn(), + mockDownloadTrajectory: vi.fn(), + mockDownloadTextFile: vi.fn(), mockFileUpload: vi.fn(), - mockCreateHttpClient: vi.fn(), + mockCreateConversationClient: vi.fn(), + mockCreateFileClient: vi.fn(), mockCreateRemoteWorkspace: vi.fn(), mockGetSettings: vi.fn(), mockGetSettingsForConversation: vi.fn(), })); vi.mock("#/api/typescript-client", () => ({ - createHttpClient: mockCreateHttpClient, + createConversationClient: mockCreateConversationClient, + createFileClient: mockCreateFileClient, createRemoteWorkspace: mockCreateRemoteWorkspace, createVSCodeClient: vi.fn(), + // Tests still patch the skills loader path indirectly via the adapter; + // returning a no-op SkillsClient is sufficient. + createSkillsClient: vi.fn(() => ({ + publicSkills: vi.fn().mockResolvedValue({ skills: [] }), + })), + // SecretsService.getSecrets still uses createHttpClient directly (not + // yet migrated to a typed SDK client). Without this stub the + // createConversation tests would hit retry+fallback timing. + createHttpClient: vi.fn(() => ({ + get: vi.fn().mockResolvedValue({ data: { secrets: [] } }), + post: vi.fn(), + put: vi.fn(), + patch: vi.fn(), + delete: vi.fn(), + })), })); vi.mock("#/api/agent-server-config", () => ({ @@ -58,16 +80,33 @@ vi.mock("#/api/settings-service/settings-service.api", () => ({ describe("AgentServerConversationService", () => { beforeEach(() => { vi.clearAllMocks(); - mockHttpGet.mockReset(); - mockHttpPost.mockReset(); - mockHttpDelete.mockReset(); + mockCreateConversation.mockReset(); + mockGetConversations.mockReset(); + mockDeleteConversation.mockReset(); + mockDownloadTrajectory.mockReset(); + mockDownloadTextFile.mockReset(); mockFileUpload.mockReset(); - mockCreateHttpClient.mockReturnValue({ - get: mockHttpGet, - post: mockHttpPost, - patch: vi.fn(), - delete: mockHttpDelete, + mockCreateConversationClient.mockReturnValue({ + createConversation: mockCreateConversation, + getConversations: mockGetConversations, + deleteConversation: mockDeleteConversation, + // The rest are unused by these tests but the client surface is + // typed in the consumer, so provide no-op stubs. + sendEvent: vi.fn(), + pauseConversation: vi.fn(), + runConversation: vi.fn(), + askAgent: vi.fn(), + getConversation: vi.fn(), + searchConversations: vi.fn(), + updateConversation: vi.fn(), + }); + mockCreateFileClient.mockReturnValue({ + downloadTrajectory: mockDownloadTrajectory, + downloadTextFile: mockDownloadTextFile, + downloadFile: vi.fn(), + searchSubdirectories: vi.fn(), + getHome: vi.fn(), }); mockCreateRemoteWorkspace.mockReturnValue({ fileUpload: mockFileUpload, @@ -76,37 +115,27 @@ describe("AgentServerConversationService", () => { describe("readConversationFile", () => { it("downloads the plan from the conversation's own working_dir when no filePath is provided", async () => { - const encodedPlan = new TextEncoder().encode("# PLAN content").buffer; - mockHttpGet.mockImplementation((url: string) => { - if (url === "/api/conversations") { - return Promise.resolve({ - data: [ - { - id: "conv-123", - created_at: "2024-01-01", - updated_at: "2024-01-01", - workspace: { - working_dir: "/workspace/project/agent-canvas/conv-123", - }, - }, - ], - }); - } - return Promise.resolve({ data: encodedPlan }); - }); + mockGetConversations.mockResolvedValue([ + { + id: "conv-123", + created_at: "2024-01-01", + updated_at: "2024-01-01", + workspace: { + working_dir: "/workspace/project/agent-canvas/conv-123", + }, + }, + ]); + mockDownloadTextFile.mockResolvedValue("# PLAN content"); const content = await AgentServerConversationService.readConversationFile("conv-123"); expect(content).toBe("# PLAN content"); - expect(mockHttpGet).toHaveBeenCalledWith( - "/api/file/download", - expect.objectContaining({ - params: { - path: "/workspace/project/agent-canvas/conv-123/.agents_tmp/PLAN.md", - }, - responseType: "arrayBuffer", - }), + // The conversation lookup picks the working_dir; the file path is + // {working_dir}/.agents_tmp/PLAN.md, which the SDK's + // FileClient.downloadTextFile sends as a `path` query param. + expect(mockDownloadTextFile).toHaveBeenCalledWith( + "/workspace/project/agent-canvas/conv-123/.agents_tmp/PLAN.md", ); }); }); @@ -122,24 +151,22 @@ describe("AgentServerConversationService", () => { conversationSettings: {}, secretsEncrypted: true, }); - mockHttpPost.mockResolvedValue({ - data: { - id: "ignored-server-id", - created_at: "2024-01-01", - updated_at: "2024-01-01", - }, + mockCreateConversation.mockResolvedValue({ + id: "ignored-server-id", + created_at: "2024-01-01", + updated_at: "2024-01-01", }); await AgentServerConversationService.createConversation(); await AgentServerConversationService.createConversation(); - expect(mockHttpPost).toHaveBeenCalledTimes(2); - const [firstCall, secondCall] = mockHttpPost.mock.calls; - const firstPayload = firstCall[1] as { + expect(mockCreateConversation).toHaveBeenCalledTimes(2); + const [firstCall, secondCall] = mockCreateConversation.mock.calls; + const firstPayload = firstCall[0] as { conversation_id: string; workspace: { working_dir: string }; }; - const secondPayload = secondCall[1] as { + const secondPayload = secondCall[0] as { conversation_id: string; workspace: { working_dir: string }; }; @@ -171,17 +198,14 @@ describe("AgentServerConversationService", () => { __resetActiveStoreForTests(); }); - it("hits the local /api/file/download-trajectory endpoint with responseType blob when active backend is local", async () => { + it("delegates to FileClient.downloadTrajectory with the conversation id when active backend is local", async () => { const zipBlob = new Blob(["zip-bytes"], { type: "application/zip" }); - mockHttpGet.mockResolvedValue({ data: zipBlob }); + mockDownloadTrajectory.mockResolvedValue(zipBlob); const result = await AgentServerConversationService.downloadConversation("conv-abc"); - expect(mockHttpGet).toHaveBeenCalledWith( - "/api/file/download-trajectory/conv-abc", - expect.objectContaining({ responseType: "blob" }), - ); + expect(mockDownloadTrajectory).toHaveBeenCalledWith("conv-abc"); expect(result).toBe(zipBlob); }); }); @@ -197,14 +221,12 @@ describe("AgentServerConversationService", () => { __resetActiveStoreForTests(); }); - it("hits the local /api/conversations/{id} endpoint when active backend is local", async () => { - mockHttpDelete.mockResolvedValue({ data: undefined }); + it("delegates to ConversationClient.deleteConversation when active backend is local", async () => { + mockDeleteConversation.mockResolvedValue(undefined); await AgentServerConversationService.deleteConversation("conv-abc"); - expect(mockHttpDelete).toHaveBeenCalledWith( - "/api/conversations/conv-abc", - ); + expect(mockDeleteConversation).toHaveBeenCalledWith("conv-abc"); }); }); diff --git a/__tests__/api/mock-conversation-handlers.test.ts b/__tests__/api/mock-conversation-handlers.test.ts index 29c526d27..30ffc5a1f 100644 --- a/__tests__/api/mock-conversation-handlers.test.ts +++ b/__tests__/api/mock-conversation-handlers.test.ts @@ -1,8 +1,16 @@ -import { describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import AgentServerConversationService from "#/api/conversation-service/agent-server-conversation-service.api"; import AgentServerGitService from "#/api/git-service/agent-server-git-service.api"; describe("mock conversation handlers", () => { + beforeEach(() => { + vi.stubEnv("VITE_WORKING_DIR", ""); + }); + + afterEach(() => { + vi.unstubAllEnvs(); + }); + it("returns adapted conversations for batch lookups", async () => { const [conversation] = await AgentServerConversationService.batchGetAppConversations([ "1", diff --git a/__tests__/api/use-create-conversation-metadata.test.ts b/__tests__/api/use-create-conversation-metadata.test.ts index a78892ff2..f385ba4de 100644 --- a/__tests__/api/use-create-conversation-metadata.test.ts +++ b/__tests__/api/use-create-conversation-metadata.test.ts @@ -6,21 +6,36 @@ import { useCreateConversation } from "#/hooks/mutation/use-create-conversation" import { getStoredConversationMetadata } from "#/api/conversation-metadata-store"; const { - mockHttpPost, - mockCreateHttpClient, + mockCreateConversation, + mockCreateConversationClient, mockGetSettings, mockGetSettingsForConversation, } = vi.hoisted(() => ({ - mockHttpPost: vi.fn(), - mockCreateHttpClient: vi.fn(), + mockCreateConversation: vi.fn(), + mockCreateConversationClient: vi.fn(), mockGetSettings: vi.fn(), mockGetSettingsForConversation: vi.fn(), })); vi.mock("#/api/typescript-client", () => ({ - createHttpClient: mockCreateHttpClient, + createConversationClient: mockCreateConversationClient, + createFileClient: vi.fn(), createRemoteWorkspace: vi.fn(), createVSCodeClient: vi.fn(), + createSkillsClient: vi.fn(() => ({ + publicSkills: vi.fn().mockResolvedValue({ skills: [] }), + })), + // SecretsService.getSecrets still uses createHttpClient directly (it's + // not part of the SDK clients yet); stub it as a returns-empty-data + // shape so its retry+fallback path returns [] quickly instead of + // hanging waitFor. + createHttpClient: vi.fn(() => ({ + get: vi.fn().mockResolvedValue({ data: { secrets: [] } }), + post: vi.fn(), + put: vi.fn(), + patch: vi.fn(), + delete: vi.fn(), + })), })); vi.mock("#/api/agent-server-config", () => ({ @@ -56,8 +71,8 @@ const wrapper = ({ children }: { children: React.ReactNode }) => { describe("useCreateConversation persists selected repository metadata", () => { beforeEach(() => { window.localStorage.clear(); - mockHttpPost.mockReset(); - mockCreateHttpClient.mockReset(); + mockCreateConversation.mockReset(); + mockCreateConversationClient.mockReset(); mockGetSettings.mockReset(); mockGetSettingsForConversation.mockReset(); mockGetSettings.mockResolvedValue({ @@ -69,18 +84,23 @@ describe("useCreateConversation persists selected repository metadata", () => { conversationSettings: {}, secretsEncrypted: true, }); - mockCreateHttpClient.mockReturnValue({ - get: vi.fn(), - post: mockHttpPost, - patch: vi.fn(), - delete: vi.fn(), + mockCreateConversationClient.mockReturnValue({ + createConversation: mockCreateConversation, + // Other methods on the client surface are unused here. + getConversation: vi.fn(), + getConversations: vi.fn(), + searchConversations: vi.fn(), + sendEvent: vi.fn(), + pauseConversation: vi.fn(), + runConversation: vi.fn(), + askAgent: vi.fn(), + updateConversation: vi.fn(), + deleteConversation: vi.fn(), }); - mockHttpPost.mockResolvedValue({ - data: { - id: "conv-new", - created_at: "2026-05-05T00:00:00Z", - updated_at: "2026-05-05T00:00:00Z", - }, + mockCreateConversation.mockResolvedValue({ + id: "conv-new", + created_at: "2026-05-05T00:00:00Z", + updated_at: "2026-05-05T00:00:00Z", }); }); diff --git a/src/api/agent-server-adapter.ts b/src/api/agent-server-adapter.ts index 16e5b2d32..881c7723a 100644 --- a/src/api/agent-server-adapter.ts +++ b/src/api/agent-server-adapter.ts @@ -15,7 +15,7 @@ import { AppConversation, AppConversationPage, } from "./conversation-service/agent-server-conversation-service.types"; -import { createHttpClient, createSkillsClient } from "./typescript-client"; +import { createFileClient, createSkillsClient } from "./typescript-client"; import SettingsService from "./settings-service/settings-service.api"; import { getStoredConversationMetadata } from "./conversation-metadata-store"; @@ -529,15 +529,7 @@ export async function buildStartConversationRequestWithEncryptedSettings(options } export async function downloadTextFile(path: string): Promise { - const response = await createHttpClient().get( - "/api/file/download", - { - params: { path }, - responseType: "arrayBuffer", - }, - ); - - return new TextDecoder().decode(response.data); + return createFileClient().downloadTextFile(path); } export async function loadSkillsForConversation( diff --git a/src/api/conversation-service/agent-server-conversation-service.api.ts b/src/api/conversation-service/agent-server-conversation-service.api.ts index e702aa856..e20e35a6d 100644 --- a/src/api/conversation-service/agent-server-conversation-service.api.ts +++ b/src/api/conversation-service/agent-server-conversation-service.api.ts @@ -1,4 +1,5 @@ import { v4 as uuidv4 } from "uuid"; +import { ConversationSortOrder } from "@openhands/typescript-client"; import { Provider } from "#/types/settings"; import { buildHttpBaseUrl } from "#/utils/websocket-url"; import { @@ -32,7 +33,8 @@ import { } from "../agent-server-adapter"; import { GetVSCodeUrlResponse } from "../open-hands.types"; import { - createHttpClient, + createConversationClient, + createFileClient, createRemoteWorkspace, createVSCodeClient, } from "../typescript-client"; @@ -60,14 +62,9 @@ class AgentServerConversationService { conversationId: string, message: SendMessageRequest, ): Promise { - await createHttpClient().post( - `/api/conversations/${conversationId}/events`, - { - ...message, - run: true, - }, - ); - + await createConversationClient().sendEvent(conversationId, message, { + run: true, + }); return message; } @@ -127,11 +124,10 @@ class AgentServerConversationService { worktree: !!metadata?.selected_repository, }); - const response = await createHttpClient().post( - "/api/conversations", - payload, - ); - const { data } = response; + const data = + await createConversationClient().createConversation( + payload as Record, + ); if (metadata?.selected_repository) { // The agent-server runtime has no concept of selected repo/branch, so @@ -232,11 +228,9 @@ class AgentServerConversationService { _conversationUrl: string | null | undefined, sessionApiKey?: string | null, ): Promise<{ success: boolean }> { - const response = await createHttpClient({ sessionApiKey }).post<{ - success: boolean; - }>(`/api/conversations/${conversationId}/pause`, {}); - - return response.data; + return createConversationClient({ sessionApiKey }).pauseConversation( + conversationId, + ); } static async askAgent( @@ -245,11 +239,10 @@ class AgentServerConversationService { question: string, sessionApiKey?: string | null, ): Promise<{ response: string }> { - const response = await createHttpClient({ sessionApiKey }).post<{ - response: string; - }>(`/api/conversations/${conversationId}/ask_agent`, { question }); - - return response.data; + return createConversationClient({ sessionApiKey }).askAgent( + conversationId, + question, + ); } static async resumeConversation( @@ -257,11 +250,9 @@ class AgentServerConversationService { _conversationUrl: string | null | undefined, sessionApiKey?: string | null, ): Promise<{ success: boolean }> { - const response = await createHttpClient({ sessionApiKey }).post<{ - success: boolean; - }>(`/api/conversations/${conversationId}/run`, {}); - - return response.data; + return createConversationClient({ sessionApiKey }).runConversation( + conversationId, + ); } static async batchGetAppConversations( @@ -273,11 +264,12 @@ class AgentServerConversationService { return batchGetCloudConversations(ids); } - const response = await createHttpClient().get< - (DirectConversationInfo | null)[] - >("/api/conversations", { params: { ids } }); + const data = + await createConversationClient().getConversations( + ids, + ); - return response.data.map((item) => (item ? toAppConversation(item) : null)); + return data.map((item) => (item ? toAppConversation(item) : null)); } static async uploadFile( @@ -352,14 +344,7 @@ class AgentServerConversationService { return downloadCloudConversation(conversationId); } - const response = await createHttpClient().get( - `/api/file/download-trajectory/${conversationId}`, - { - responseType: "blob", - }, - ); - - return response.data; + return createFileClient().downloadTrajectory(conversationId); } static async getSkills(conversationId: string): Promise { @@ -401,12 +386,10 @@ class AgentServerConversationService { authMode: "session-api-key", sessionApiKey, }) - : ( - await createHttpClient({ - conversationUrl, - sessionApiKey, - }).get(`/api/conversations/${conversationId}`) - ).data; + : await createConversationClient({ + conversationUrl, + sessionApiKey, + }).getConversation(conversationId); return { id: data.id, @@ -452,25 +435,28 @@ class AgentServerConversationService { return searchCloudConversations(limit, pageId); } - const response = await createHttpClient().get<{ - items: DirectConversationInfo[]; - next_page_id: string | null; - }>("/api/conversations/search", { - params: { - limit, - page_id: pageId, - sort_order: "UPDATED_AT_DESC", - }, + // ConversationClient.searchConversations returns the SDK's typed + // `ConversationInfo[]`; on the local agent-server the same JSON is also + // a `DirectConversationInfo[]` (which is what the rest of agent-canvas + // expects), so a cast through `unknown` is honest about the change of + // narrow type without re-walking the items. + const data = await createConversationClient().searchConversations({ + limit, + page_id: pageId, + sort_order: ConversationSortOrder.UPDATED_AT_DESC, }); - return toConversationPage(response.data); + return toConversationPage({ + items: data.items as unknown as DirectConversationInfo[], + next_page_id: data.next_page_id ?? null, + }); } static async deleteConversation(conversationId: string): Promise { if (getActiveBackend().backend.kind === "cloud") { await deleteCloudConversation(conversationId); } else { - await createHttpClient().delete(`/api/conversations/${conversationId}`); + await createConversationClient().deleteConversation(conversationId); } removeStoredConversationMetadata(conversationId); } @@ -479,7 +465,7 @@ class AgentServerConversationService { conversationId: string, title: string, ): Promise { - await createHttpClient().patch(`/api/conversations/${conversationId}`, { + await createConversationClient().updateConversation(conversationId, { title, }); const [conversation] = await this.batchGetAppConversations([ diff --git a/src/api/files-service/files-service.api.ts b/src/api/files-service/files-service.api.ts index 4de54e928..66f883f1e 100644 --- a/src/api/files-service/files-service.api.ts +++ b/src/api/files-service/files-service.api.ts @@ -1,4 +1,4 @@ -import { createHttpClient } from "../typescript-client"; +import { createFileClient } from "../typescript-client"; export interface SubdirectoryEntry { name: string; @@ -31,21 +31,26 @@ const FilesService = { path: string, options: SearchSubdirsOptions = {}, ): Promise { - const params: Record = { path }; - if (options.pageId) params.page_id = options.pageId; - if (typeof options.limit === "number") params.limit = options.limit; - - const response = await createHttpClient().get( - "/api/file/search_subdirs", - { params }, - ); - return response.data; + // SDK's `FileClient.searchSubdirectories` returns the same JSON shape + // (items + next_page_id); the `pageId ?? undefined` filters out the + // null-vs-undefined mismatch so the SDK doesn't serialize `page_id=null`. + const page = await createFileClient().searchSubdirectories(path, { + pageId: options.pageId ?? undefined, + limit: options.limit, + }); + return { + items: page.items, + next_page_id: page.next_page_id ?? null, + }; }, async getHome(): Promise { - const response = - await createHttpClient().get("/api/file/home"); - return response.data; + const home = await createFileClient().getHome(); + return { + favorites: [], + locations: [], + ...home, + }; }, }; diff --git a/src/api/typescript-client.ts b/src/api/typescript-client.ts index a3a0e4b20..143bf5611 100644 --- a/src/api/typescript-client.ts +++ b/src/api/typescript-client.ts @@ -1,4 +1,6 @@ import { + ConversationClient, + FileClient, LLMMetadataClient, ServerClient, SettingsClient, @@ -120,6 +122,28 @@ export function createHttpClient( }); } +export function createConversationClient( + overrides?: TypeScriptClientOverrides, +): ConversationClient { + const { host, apiKey } = resolveClientOptions(overrides); + return new ConversationClient({ + host, + ...(apiKey ? { apiKey } : {}), + ...(overrides?.timeout ? { timeout: overrides.timeout } : {}), + }); +} + +export function createFileClient( + overrides?: TypeScriptClientOverrides, +): FileClient { + const { host, apiKey } = resolveClientOptions(overrides); + return new FileClient({ + host, + ...(apiKey ? { apiKey } : {}), + ...(overrides?.timeout ? { timeout: overrides.timeout } : {}), + }); +} + export function createRemoteEventsList( conversationId: string, overrides?: TypeScriptClientOverrides,