diff --git a/AGENTS.md b/AGENTS.md index a44b66db3..0e3706f32 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -61,13 +61,13 @@ - For local verification in this repo, setting `VITE_WORKING_DIR=/workspace/project/agent-server-gui` avoids initial Changes-tab 500s from pointing conversations at the non-repo parent `/workspace/project`. - A successful end-to-end live run in this environment required a real LLM config (`LLM_MODEL` + `LLM_API_KEY`). The default `litellm_proxy/...` model with no `llm_api_key` failed at runtime with a `litellm.AuthenticationError`. -- Agent-server recovery UX gotchas: - - Keep `/settings/agent-server` in the intermediate-page bypass path (`use-is-on-intermediate-page`) so `useConfig()`-driven layout/sidebar queries do not block the recovery screen behind a global spinner. - - `PostHogWrapper` should treat config-fetch failures as silent/optional (no user-facing toast), otherwise onboarding/recovery screens show a duplicate incompatible-server toast on top of the friendly guidance. - - Keep the settings route on the compact `AgentServerConnectionForm` variant with `showSectionHeader={false}` and no checklist; the blocked root onboarding should stay similarly minimal, with only the status card plus a single sentence that links to the repo setup instructions. - - For local screenshot/GIF capture of SPA routes, serve `build/` with an SPA fallback (for example `sirv build --single`) and restart the static server after each rebuild so hashed asset URLs stay in sync. - -- Git provider token persistence note: this direct-agent-server frontend now persists `Settings > Git` provider tokens locally in browser storage instead of posting to an app-backend secrets route. `src/api/secrets-service.ts` writes the token payload to localStorage, mirrors provider hosts into `provider_tokens_set` through `SettingsService.saveSettings()`, and `use-delete-git-providers` clears that local state. +- **Settings persistence architecture**: Settings are now persisted via the agent-server REST API (`/api/settings` endpoints) instead of relying solely on localStorage. The `SettingsService` in `src/api/settings-service/settings-service.api.ts` uses the `@openhands/typescript-client` `SettingsClient` for CRUD operations: + - `getSettings()` fetches from the agent-server with localStorage as fallback + - `saveSettings()` posts diffs to the server and updates the local cache + - The local cache key `openhands-agent-server-settings` remains for offline scenarios and faster initial loads + - The agent-server persists settings to `~/.openhands/settings.json` on disk +- **Secrets persistence**: Custom secrets (`Settings > Secrets`) are now managed via the agent-server's `SettingsClient.createSecret()`, `listSecrets()`, `deleteSecret()` methods. The server encrypts API keys using the SDK's Cipher utility and stores them at `~/.openhands/secrets.json`. +- Git provider token persistence note: this direct-agent-server frontend still persists `Settings > Git` provider tokens locally in browser storage (`openhands-agent-server-git-provider-tokens`) while mirroring host metadata into settings via `SettingsService.saveSettings()`. - Agent server connection settings now live at `Settings > Agent Server` (`/settings/agent-server`). The page reads deployment defaults from `VITE_BACKEND_BASE_URL` / `VITE_SESSION_API_KEY`, saves user overrides in the `openhands-agent-server-config` localStorage key, and must stay reachable even when the backend compatibility probe fails so users can recover from missing or wrong backend configuration. - README expectation: keep the first section as a concrete, chronological from-scratch quickstart for running this frontend against a real `openhands-agent-server` (clone, install backend, optional `.env`, run `npm run dev`). diff --git a/__tests__/components/shared/modals/settings/settings-form.test.tsx b/__tests__/components/shared/modals/settings/settings-form.test.tsx index 1ebdab88c..0a5881d73 100644 --- a/__tests__/components/shared/modals/settings/settings-form.test.tsx +++ b/__tests__/components/shared/modals/settings/settings-form.test.tsx @@ -1,4 +1,4 @@ -import { screen } from "@testing-library/react"; +import { screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { renderWithProviders } from "test-utils"; @@ -13,6 +13,8 @@ describe("SettingsForm", () => { beforeEach(() => { vi.clearAllMocks(); + // Mock saveSettings to resolve immediately (simulates server response) + saveSettingsSpy.mockResolvedValue(true); }); it("should save the user settings and close the modal when submitted outside a conversation route", async () => { @@ -26,16 +28,21 @@ describe("SettingsForm", () => { await user.click(screen.getByTestId("save-settings-button")); - expect(saveSettingsSpy).toHaveBeenCalledWith( - expect.objectContaining({ - agent_settings_diff: expect.objectContaining({ - llm: expect.objectContaining({ - model: getAgentSettingValue(DEFAULT_SETTINGS, "llm.model"), + await waitFor(() => { + expect(saveSettingsSpy).toHaveBeenCalledWith( + expect.objectContaining({ + agent_settings_diff: expect.objectContaining({ + llm: expect.objectContaining({ + model: getAgentSettingValue(DEFAULT_SETTINGS, "llm.model"), + }), }), }), - }), - ); - expect(onCloseMock).toHaveBeenCalled(); + ); + }); + + await waitFor(() => { + expect(onCloseMock).toHaveBeenCalled(); + }); }); it("should confirm before saving when submitted from a conversation route", async () => { @@ -60,15 +67,20 @@ describe("SettingsForm", () => { await user.click(confirmButton); - expect(saveSettingsSpy).toHaveBeenCalledWith( - expect.objectContaining({ - agent_settings_diff: expect.objectContaining({ - llm: expect.objectContaining({ - model: getAgentSettingValue(DEFAULT_SETTINGS, "llm.model"), + await waitFor(() => { + expect(saveSettingsSpy).toHaveBeenCalledWith( + expect.objectContaining({ + agent_settings_diff: expect.objectContaining({ + llm: expect.objectContaining({ + model: getAgentSettingValue(DEFAULT_SETTINGS, "llm.model"), + }), }), }), - }), - ); - expect(onCloseMock).toHaveBeenCalled(); + ); + }); + + await waitFor(() => { + expect(onCloseMock).toHaveBeenCalled(); + }); }); }); diff --git a/src/api/agent-server-adapter.ts b/src/api/agent-server-adapter.ts index aa577b9c5..adafd1c14 100644 --- a/src/api/agent-server-adapter.ts +++ b/src/api/agent-server-adapter.ts @@ -320,6 +320,14 @@ function buildConfiguredConversationSettings(options: { }; } +/** + * Build a start conversation request payload from settings. + * + * The frontend fetches settings with `X-Expose-Secrets: encrypted`, receiving + * cipher-encrypted secret values. When starting a conversation, we include + * `secrets_encrypted: true` to tell the server to decrypt the secrets + * (e.g., LLM API key) before use. + */ export function buildStartConversationRequest(options: { settings: Settings; query?: string; @@ -343,6 +351,9 @@ export function buildStartConversationRequest(options: { : 500, stuck_detection: true, autotitle: true, + // Tell server that secrets (e.g., LLM api_key) are cipher-encrypted + // and need to be decrypted before use + secrets_encrypted: true, }; if (options.conversationId) { diff --git a/src/api/agent-server-compatibility.ts b/src/api/agent-server-compatibility.ts index 60b836a2a..537e02ca9 100644 --- a/src/api/agent-server-compatibility.ts +++ b/src/api/agent-server-compatibility.ts @@ -6,7 +6,14 @@ const AGENT_SERVER_INFO_TIMEOUT_MS = 5000; const SEMVER_PATTERN = /^v?(\d+)\.(\d+)\.(\d+)(?:[-+].*)?$/; -const getServerVersion = (serverInfo: ServerInfo): string => serverInfo.version; +const getServerVersion = (serverInfo: ServerInfo): string => { + // Fall back to sdk_version when version is unknown/missing + // (common in dev builds or when build metadata isn't injected) + if (serverInfo.version && serverInfo.version !== "unknown") { + return serverInfo.version; + } + return serverInfo.sdk_version ?? serverInfo.version; +}; const parseSemver = ( version: string | null, diff --git a/src/api/secrets-service.ts b/src/api/secrets-service.ts index c855791de..e48c99595 100644 --- a/src/api/secrets-service.ts +++ b/src/api/secrets-service.ts @@ -1,13 +1,28 @@ import SettingsService from "./settings-service/settings-service.api"; -import { openHands } from "./open-hands-axios"; +import { createHttpClient } from "./typescript-client"; import { - CustomSecret, CustomSecretPage, CustomSecretWithoutValue, SearchSecretsParams, } from "./secrets-service.types"; import { Provider, ProviderOptions, ProviderToken } from "#/types/settings"; +/** + * Response from GET /api/settings/secrets + */ +interface SecretsListResponse { + secrets: Array<{ name: string; description?: string | null }>; +} + +/** + * Request for PUT /api/settings/secrets + */ +interface CreateSecretRequest { + name: string; + value: string; + description?: string | null; +} + const GIT_PROVIDER_STORAGE_KEY = "openhands-agent-server-git-provider-tokens"; type StoredGitProviderTokens = Partial>; @@ -97,32 +112,45 @@ const buildProviderTokensSet = ( export class SecretsService { /** * Search/list custom secrets with pagination support. - * Uses the new V1 API endpoint: GET /api/v1/secrets/search + * Uses the agent-server settings API for local persistence. */ static async searchSecrets( params: SearchSecretsParams = {}, ): Promise { - const queryParams = new URLSearchParams(); + try { + const client = createHttpClient(); + const response = await client.get( + "/api/settings/secrets", + ); + + // Filter by name if requested + let items = response.data.secrets.map((s) => ({ + name: s.name, + description: s.description ?? undefined, + })); + + if (params.name__contains) { + const query = params.name__contains.toLowerCase(); + items = items.filter((s) => s.name.toLowerCase().includes(query)); + } - if (params.name__contains) { - queryParams.set("name__contains", params.name__contains); - } - if (params.page_id) { - queryParams.set("page_id", params.page_id); + // Simple pagination (agent-server doesn't have built-in pagination) + const limit = params.limit ?? 100; + const startIndex = params.page_id ? parseInt(params.page_id, 10) : 0; + const paginatedItems = items.slice(startIndex, startIndex + limit); + const hasMore = startIndex + limit < items.length; + + return { + items: paginatedItems, + next_page_id: hasMore ? String(startIndex + limit) : null, + }; + } catch { + return { items: [], next_page_id: null }; } - if (params.limit) { - queryParams.set("limit", params.limit.toString()); - } - - const queryString = queryParams.toString(); - const url = `/api/v1/secrets/search${queryString ? `?${queryString}` : ""}`; - - const { data } = await openHands.get(url); - return data; } /** - * @deprecated Use searchSecrets instead. This method uses the deprecated V0 API. + * Get all secrets (names and descriptions only, no values). */ static async getSecrets(): Promise { const allSecrets: CustomSecretWithoutValue[] = []; @@ -141,30 +169,58 @@ export class SecretsService { return allSecrets; } + /** + * Create a new custom secret via the agent-server. + */ static async createSecret(name: string, value: string, description?: string) { - const secret: CustomSecret = { - name, - value, - description, - }; - - const { status } = await openHands.post("/api/v1/secrets", secret); - return status === 201; + try { + const client = createHttpClient(); + await client.put("/api/settings/secrets", { + name, + value, + description: description ?? null, + }); + return true; + } catch { + return false; + } } - static async updateSecret(id: string, name: string, description?: string) { - const secret: CustomSecretWithoutValue = { - name, - description, - }; - - const { status } = await openHands.put(`/api/v1/secrets/${id}`, secret); - return status === 200; + /** + * Update a secret's metadata (description). For agent-server, we need to + * re-create the secret with the same value since we can't update in place + * without the value. + */ + static async updateSecret(_id: string, name: string, description?: string) { + try { + // For agent-server, we can only update by re-creating with a new value + // This is a limitation - in practice, users should delete and recreate + const client = createHttpClient(); + const valueResponse = await client.get( + `/api/settings/secrets/${name}`, + ); + await client.put("/api/settings/secrets", { + name, + value: valueResponse.data, + description: description ?? null, + }); + return true; + } catch { + return false; + } } - static async deleteSecret(id: string) { - const { status } = await openHands.delete(`/api/v1/secrets/${id}`); - return status === 200; + /** + * Delete a custom secret via the agent-server. + */ + static async deleteSecret(name: string) { + try { + const client = createHttpClient(); + await client.delete(`/api/settings/secrets/${name}`); + return true; + } catch { + return false; + } } static async addGitProvider( diff --git a/src/api/settings-service/settings-service.api.ts b/src/api/settings-service/settings-service.api.ts index e29fe93d8..9867636d5 100644 --- a/src/api/settings-service/settings-service.api.ts +++ b/src/api/settings-service/settings-service.api.ts @@ -1,8 +1,17 @@ import { DEFAULT_SETTINGS } from "#/services/settings"; import { Settings, SettingsSchema, SettingsValue } from "#/types/settings"; -import { createSettingsClient } from "../typescript-client"; +import { createHttpClient, createSettingsClient } from "../typescript-client"; -const STORAGE_KEY = "openhands-agent-server-settings"; +const LOCAL_CACHE_KEY = "openhands-agent-server-settings"; + +/** + * Response from GET /api/settings + */ +interface SettingsResponse { + agent_settings: Record; + conversation_settings: Record; + llm_api_key_is_set: boolean; +} const deepClone = (value: T): T => JSON.parse(JSON.stringify(value)) as T; @@ -11,13 +20,16 @@ const mergeRecords = ( next: Record | null | undefined, ) => ({ ...(base ?? {}), ...(next ?? {}) }); -const readStoredSettings = (): Partial => { +/** + * Read cached settings from localStorage (used as fallback when server unavailable). + */ +const readLocalCache = (): Partial => { if (typeof window === "undefined") { return {}; } try { - const raw = window.localStorage.getItem(STORAGE_KEY); + const raw = window.localStorage.getItem(LOCAL_CACHE_KEY); if (!raw) return {}; return (JSON.parse(raw) as Partial) ?? {}; } catch { @@ -25,6 +37,19 @@ const readStoredSettings = (): Partial => { } }; +/** + * Write settings to localStorage as a local cache. + */ +const writeLocalCache = (settings: Settings) => { + if (typeof window === "undefined") return; + window.localStorage.setItem(LOCAL_CACHE_KEY, JSON.stringify(settings)); +}; + +/** + * Converts server response format to frontend Settings format. + * The server stores `agent_settings` and `conversation_settings` as nested records, + * but the frontend expects top-level fields like `llm_model`, `agent`, etc. + */ const syncDerivedSettings = (settings: Partial): Settings => { const agentSettings = mergeRecords( DEFAULT_SETTINGS.agent_settings ?? {}, @@ -94,14 +119,43 @@ const syncDerivedSettings = (settings: Partial): Settings => { return merged; }; -const writeStoredSettings = (settings: Settings) => { - if (typeof window === "undefined") return; - window.localStorage.setItem(STORAGE_KEY, JSON.stringify(settings)); -}; - class SettingsService { + /** + * Get settings from the agent server, with local cache fallback. + * The server persists settings to disk, while localStorage acts as a cache + * for faster initial loads and offline scenarios. + * + * Uses `X-Expose-Secrets: encrypted` header to receive cipher-encrypted + * secret values. These encrypted values can be safely stored client-side + * and passed back to the server when starting conversations (with + * `secrets_encrypted: true`), where they will be decrypted. + */ static async getSettings(): Promise { - return syncDerivedSettings(readStoredSettings()); + try { + const client = createHttpClient(); + const response = await client.get("/api/settings", { + headers: { + "X-Expose-Secrets": "encrypted", + }, + }); + + // Convert server response to frontend format + const fromServer: Partial = { + agent_settings: response.data.agent_settings, + conversation_settings: response.data.conversation_settings, + llm_api_key_set: response.data.llm_api_key_is_set, + }; + + const merged = syncDerivedSettings(fromServer); + + // Update local cache (contains encrypted secrets) + writeLocalCache(merged); + + return merged; + } catch { + // Fallback to local cache if server unavailable + return syncDerivedSettings(readLocalCache()); + } } static async getSettingsSchema(): Promise { @@ -112,6 +166,10 @@ class SettingsService { return (await createSettingsClient().getConversationSchema()) as SettingsSchema; } + /** + * Save settings to the agent server. + * Sends only the diff (changed fields) to the server, which merges with existing. + */ static async saveSettings( settings: Partial & Record, ): Promise { @@ -185,9 +243,25 @@ class SettingsService { delete nextSettings.agent_settings_diff; delete nextSettings.conversation_settings_diff; - const merged = syncDerivedSettings(nextSettings); - writeStoredSettings(merged); - return true; + try { + // Send to agent server for persistence + const client = createHttpClient(); + await client.patch("/api/settings", { + agent_settings_diff: agentSettingsDiff, + conversation_settings_diff: conversationSettingsDiff, + }); + + // Update local cache on success + const merged = syncDerivedSettings(nextSettings); + writeLocalCache(merged); + + return true; + } catch { + // Fallback: save to local cache only if server unavailable + const merged = syncDerivedSettings(nextSettings); + writeLocalCache(merged); + return true; + } } }