feat: use agent server APIs for settings persistence

AGENTS.md

@@ -3,7 +3,7 @@
 - This repository is a near-direct port of the OpenHands frontend, adapted to talk straight to `software-agent-sdk` / `agent_server` without the usual OpenHands app backend.
 - Frontend API adaptation lives mainly in `src/api/`:
   - `option-service` fabricates an OSS web-client config and reads models/providers from `agent_server` LLM endpoints.
-  - `settings-service` stores settings locally in browser localStorage and reads schemas from `agent_server` `/api/settings/*` endpoints.
+  - `settings-service` uses agent server `/api/settings` endpoints for persistence; reads schemas from `/api/settings/agent-schema` and `/api/settings/conversation-schema`, fetches settings with optional `X-Expose-Secrets: encrypted` header for conversation start payloads, and saves settings via PATCH with diffs.
   - `v1-conversation-service`, `event-service`, `git-service`, and `skills-service` are mapped directly to `agent_server` REST endpoints.
   - `open-hands-axios` injects the optional `X-Session-API-Key` from env/local config for all requests.
 - Supported env vars for deployment:
@@ -35,6 +35,8 @@
 - `npm run dev:mock` needs MSW handlers for the direct agent-server routes used by the adapted frontend, not the original OpenHands mock paths. Key routes that must stay covered are:
   - bootstrap/model loading: `/server_info`, `/api/llm/models/verified`, `/api/llm/providers`
   - settings schemas: `/api/settings/agent-schema`, `/api/settings/conversation-schema`
+  - settings CRUD: `GET /api/settings`, `PATCH /api/settings`
+  - secrets CRUD: `GET /api/settings/secrets` (list), `GET /api/settings/secrets/:name` (value), `PUT /api/settings/secrets` (upsert), `DELETE /api/settings/secrets/:name`
   - conversation browsing/loading: `/api/conversations/search`, `/api/conversations?ids=...`, `/api/conversations/:id`, `/api/conversations/:id/events/*`
   - runtime git panels: `/api/git/changes`, `/api/git/diff`
 - Static mock verification needs a build created with `VITE_MOCK_API=true` (use `npm run build:mock`); the client must start MSW whenever that flag is enabled, even in production/static builds, otherwise routes like `/settings` and the conversations pane fall through to the static server and crash on undefined `.filter`/`.map` assumptions.
@@ -65,14 +67,25 @@
   - 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.
+- Git provider token persistence note: `src/api/secrets-service.ts` stores git provider tokens in TWO places:
+  1. **Agent-server secrets API** (`PUT /api/settings/secrets`) with naming convention `GIT_PROVIDER_{PROVIDER}_TOKEN` - for agent runtime use
+  2. **localStorage** (`openhands-agent-server-git-provider-tokens`) - for frontend git API calls (repo search, branches, etc.)
+  The `addGitProvider` method stores to server FIRST (must succeed), then updates localStorage. This ensures server-side persistence is the source of truth.
 - 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.
 
+- **SDK Dependency for Settings Persistence (PR #98)**: The settings persistence API changes depend on [software-agent-sdk PR #3060](https://github.com/OpenHands/software-agent-sdk/pull/3060) which adds:
+  - `/api/settings` GET/PATCH with `X-Expose-Secrets: encrypted` header support
+  - `/api/settings/secrets` CRUD endpoints for custom secrets
+  - `OH_SECRET_KEY` environment variable for encryption
+
+  **IMPORTANT**: Until PR #3060 is merged and released, `npm run dev` must use `OH_AGENT_SERVER_GIT_REF=main` to point at the SDK main branch (or the feature branch), not a released PyPI version. The dev scripts now default to `main` for this reason. Once released, update `dev-safe.mjs` to use the minimum required version.
+
 - 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 uv, optional `.env`, run `npm run dev`).
 - Keep README user-focused and move contributor/developer-specific workflows (`dev:safe`, mock mode, detailed env vars/build-test notes) into `DEVELOPMENT.md`.
 - `scripts/dev-safe.mjs` uses `uvx` for temporary agent-server installation — no permanent `uv tool install` needed. Environment variables:
   - `OH_AGENT_SERVER_VERSION` — specific PyPI version (e.g., "1.18.0")
   - `OH_AGENT_SERVER_GIT_REF` — git commit SHA or branch name (takes precedence over version)
+  - `OH_SECRET_KEY` — secret key for settings encryption; uses a default value for local dev, override for production
   - Default: latest released version from PyPI
 - `scripts/dev-safe.mjs` should fail fast if `uvx` cannot be spawned (for example missing PATH entries).
 - Vite dev mode can black-screen on first load with `504 Outdated Optimize Dep` if core client-entry deps are not prebundled; keep `react`, `react/jsx-runtime`, `react-dom/client`, and `react-router/dom` in `optimizeDeps.include`.

__tests__/api/secrets-service.test.ts

@@ -1,8 +1,9 @@
 import { beforeEach, describe, expect, it } from "vitest";
-import SettingsService from "#/api/settings-service/settings-service.api";
 import { SecretsService } from "#/api/secrets-service";
 import { Provider, ProviderToken } from "#/types/settings";
 
+const GIT_PROVIDER_STORAGE_KEY = "openhands-agent-server-git-provider-tokens";
+
 const buildProviders = (
   overrides: Partial<Record<Provider, ProviderToken>> = {},
 ): Record<Provider, ProviderToken> => ({
@@ -20,22 +21,26 @@ describe("SecretsService", () => {
     window.localStorage.clear();
   });
 
-  it("stores connected Git providers in local settings", async () => {
-    await expect(
-      SecretsService.addGitProvider(
-        buildProviders({
-          github: {
-            token: "ghp_test_123",
-            host: "github.example.com",
-          },
-        }),
-      ),
-    ).resolves.toBe(true);
-
-    const settings = await SettingsService.getSettings();
+  it("stores connected Git providers in local cache and calls secrets API", async () => {
+    // The SecretsService stores git provider tokens via the secrets API
+    // and keeps a local cache for UI purposes (host mappings)
+    // Method returns void (throws on failure)
+    await SecretsService.addGitProvider(
+      buildProviders({
+        github: {
+          token: "ghp_test_123",
+          host: "github.example.com",
+        },
+      }),
+    );
 
-    expect(settings.provider_tokens_set).toEqual({
-      github: "github.example.com",
+    // Verify local cache was updated
+    const cached = JSON.parse(
+      window.localStorage.getItem(GIT_PROVIDER_STORAGE_KEY) || "{}",
+    );
+    expect(cached.github).toEqual({
+      token: "ghp_test_123",
+      host: "github.example.com",
     });
   });
 
@@ -49,6 +54,7 @@ describe("SecretsService", () => {
       }),
     );
 
+    // Update only the host, empty token means keep existing
     await SecretsService.addGitProvider(
       buildProviders({
         github: {
@@ -58,14 +64,17 @@ describe("SecretsService", () => {
       }),
     );
 
-    const settings = await SettingsService.getSettings();
-
-    expect(settings.provider_tokens_set).toEqual({
-      github: "github.internal.example.com",
+    // Verify local cache preserves the token and updates the host
+    const cached = JSON.parse(
+      window.localStorage.getItem(GIT_PROVIDER_STORAGE_KEY) || "{}",
+    );
+    expect(cached.github).toEqual({
+      token: "ghp_test_123",
+      host: "github.internal.example.com",
     });
   });
 
-  it("clears connected Git providers from local settings", async () => {
+  it("clears connected Git providers from local cache", async () => {
     await SecretsService.addGitProvider(
       buildProviders({
         github: {
@@ -75,10 +84,10 @@ describe("SecretsService", () => {
       }),
     );
 
-    await expect(SecretsService.deleteGitProviders()).resolves.toBe(true);
-
-    const settings = await SettingsService.getSettings();
+    // Method returns void (throws on failure)
+    await SecretsService.deleteGitProviders();
 
-    expect(settings.provider_tokens_set).toEqual({});
+    // Verify local cache was cleared
+    expect(window.localStorage.getItem(GIT_PROVIDER_STORAGE_KEY)).toBeNull();
   });
 });

__tests__/api/settings-service.test.ts

@@ -1,62 +1,30 @@
-import { beforeEach, describe, expect, it } from "vitest";
+import { beforeEach, describe, expect, it, vi } from "vitest";
 
 import SettingsService from "#/api/settings-service/settings-service.api";
-
-const STORAGE_KEY = "openhands-agent-server-settings";
+import { resetTestHandlersMockSettings } from "#/mocks/settings-handlers";
 
 describe("SettingsService", () => {
   beforeEach(() => {
+    // Clear localStorage and reset mock settings state
     window.localStorage.clear();
+    resetTestHandlersMockSettings();
+    // Invalidate the in-memory cache
+    SettingsService.invalidateCache();
   });
 
-  it("treats nested SDK settings as the source of truth when loading", async () => {
-    window.localStorage.setItem(
-      STORAGE_KEY,
-      JSON.stringify({
-        agent: "Agent",
-        llm_model: "stale-top-level-model",
-        llm_base_url: "https://stale.example.com",
-        llm_api_key: "stale-key",
-        enable_default_condenser: false,
-        condenser_max_size: 12,
-        confirmation_mode: false,
-        security_analyzer: null,
-        max_iterations: 5,
-        agent_settings: {
-          agent: "CodeActAgent",
-          llm: {
-            model: "nested-model",
-            base_url: "https://nested.example.com",
-            api_key: "nested-key",
-          },
-          condenser: {
-            enabled: true,
-            max_size: 321,
-          },
-        },
-        conversation_settings: {
-          confirmation_mode: true,
-          security_analyzer: "llm",
-          max_iterations: 77,
-        },
-      }),
-    );
-
+  it("fetches settings from the API and normalizes derived fields", async () => {
+    // The mock handler returns default settings
     const settings = await SettingsService.getSettings();
 
+    // Should have normalized settings with derived fields
     expect(settings.agent).toBe("CodeActAgent");
-    expect(settings.llm_model).toBe("nested-model");
-    expect(settings.llm_base_url).toBe("https://nested.example.com");
-    expect(settings.llm_api_key).toBe("nested-key");
-    expect(settings.enable_default_condenser).toBe(true);
-    expect(settings.condenser_max_size).toBe(321);
-    expect(settings.confirmation_mode).toBe(true);
+    expect(settings.llm_model).toBe("openhands/claude-opus-4-5-20251101");
+    expect(settings.confirmation_mode).toBe(false);
     expect(settings.security_analyzer).toBe("llm");
-    expect(settings.max_iterations).toBe(77);
-    expect(settings.agent_settings?.agent).toBe("CodeActAgent");
   });
 
-  it("keeps top-level mirrors in sync when saving nested settings diffs", async () => {
+  it("saves settings via PATCH API and invalidates cache", async () => {
+    // Save some settings
     await SettingsService.saveSettings({
       agent_settings_diff: {
         agent: "CodeActAgent",
@@ -73,23 +41,66 @@ describe("SettingsService", () => {
       },
     });
 
+    // Fetch settings again - should reflect the saved values
     const settings = await SettingsService.getSettings();
 
     expect(settings.llm_model).toBe("saved-model");
     expect(settings.llm_base_url).toBe("https://saved.example.com");
-    expect(settings.llm_api_key).toBe("saved-key");
+    // Note: api_key will be redacted when fetched without X-Expose-Secrets header
     expect(settings.confirmation_mode).toBe(true);
     expect(settings.security_analyzer).toBe("llm");
     expect(settings.max_iterations).toBe(33);
-    expect(settings.agent_settings?.llm).toMatchObject({
-      model: "saved-model",
-      base_url: "https://saved.example.com",
-      api_key: "saved-key",
-    });
-    expect(settings.conversation_settings).toMatchObject({
-      confirmation_mode: true,
-      security_analyzer: "llm",
-      max_iterations: 33,
+  });
+
+  it("returns encrypted secrets when using getSettingsForConversation", async () => {
+    // First save a key
+    await SettingsService.saveSettings({
+      agent_settings_diff: {
+        llm: {
+          api_key: "test-api-key",
+        },
+      },
     });
+
+    // Get settings for conversation (should have encrypted secrets)
+    const { agentSettings, secretsEncrypted } =
+      await SettingsService.getSettingsForConversation();
+
+    expect(secretsEncrypted).toBe(true);
+    // The mock returns an "encrypted" placeholder for the key
+    const llm = agentSettings.llm as Record<string, unknown> | undefined;
+    expect(llm?.api_key).toMatch(/^gAAAAA_mock_encrypted_/);
+  });
+
+  it("uses cache for repeated getSettings calls", async () => {
+    const fetchSpy = vi.spyOn(SettingsService, "fetchSettingsFromApi");
+
+    // First call - should fetch from API
+    await SettingsService.getSettings();
+    expect(fetchSpy).toHaveBeenCalledTimes(1);
+
+    // Second call - should use cache
+    await SettingsService.getSettings();
+    expect(fetchSpy).toHaveBeenCalledTimes(1);
+
+    // After invalidation - should fetch again
+    SettingsService.invalidateCache();
+    await SettingsService.getSettings();
+    expect(fetchSpy).toHaveBeenCalledTimes(2);
+
+    fetchSpy.mockRestore();
+  });
+
+  it("skips API call when no diffs are provided to saveSettings", async () => {
+    const fetchSpy = vi.spyOn(SettingsService, "fetchSettingsFromApi");
+
+    // Call with empty/no diffs
+    const result = await SettingsService.saveSettings({});
+
+    expect(result).toBe(true);
+    // No fetch should have been made (PATCH not called)
+    expect(fetchSpy).not.toHaveBeenCalled();
+
+    fetchSpy.mockRestore();
   });
 });

__tests__/api/use-create-conversation-metadata.test.ts

@@ -5,13 +5,17 @@ import React from "react";
 import { useCreateConversation } from "#/hooks/mutation/use-create-conversation";
 import { getStoredConversationMetadata } from "#/api/conversation-metadata-store";
 
-const { mockHttpPost, mockCreateHttpClient, mockGetSettings } = vi.hoisted(
-  () => ({
-    mockHttpPost: vi.fn(),
-    mockCreateHttpClient: vi.fn(),
-    mockGetSettings: vi.fn(),
-  }),
-);
+const {
+  mockHttpPost,
+  mockCreateHttpClient,
+  mockGetSettings,
+  mockGetSettingsForConversation,
+} = vi.hoisted(() => ({
+  mockHttpPost: vi.fn(),
+  mockCreateHttpClient: vi.fn(),
+  mockGetSettings: vi.fn(),
+  mockGetSettingsForConversation: vi.fn(),
+}));
 
 vi.mock("#/api/typescript-client", () => ({
   createHttpClient: mockCreateHttpClient,
@@ -31,7 +35,10 @@ vi.mock("#/api/agent-server-config", () => ({
 }));
 
 vi.mock("#/api/settings-service/settings-service.api", () => ({
-  default: { getSettings: mockGetSettings },
+  default: {
+    getSettings: mockGetSettings,
+    getSettingsForConversation: mockGetSettingsForConversation,
+  },
 }));
 
 vi.mock("#/hooks/use-tracking", () => ({
@@ -50,10 +57,17 @@ describe("useCreateConversation persists selected repository metadata", () => {
     window.localStorage.clear();
     mockHttpPost.mockReset();
     mockCreateHttpClient.mockReset();
+    mockGetSettings.mockReset();
+    mockGetSettingsForConversation.mockReset();
     mockGetSettings.mockResolvedValue({
       agent_settings: { llm: { model: "gpt-4o" } },
       conversation_settings: {},
     });
+    mockGetSettingsForConversation.mockResolvedValue({
+      agentSettings: { llm: { model: "gpt-4o" } },
+      conversationSettings: {},
+      secretsEncrypted: true,
+    });
     mockCreateHttpClient.mockReturnValue({
       get: vi.fn(),
       post: mockHttpPost,

__tests__/api/v1-conversation-service.test.ts

@@ -8,13 +8,15 @@ const {
   mockCreateHttpClient,
   mockCreateRemoteWorkspace,
   mockGetSettings,
+  mockGetSettingsForConversation,
 } = vi.hoisted(() => ({
   mockHttpGet: vi.fn(),
   mockHttpPost: vi.fn(),
   mockFileUpload: vi.fn(),
   mockCreateHttpClient: vi.fn(),
   mockCreateRemoteWorkspace: vi.fn(),
   mockGetSettings: vi.fn(),
+  mockGetSettingsForConversation: vi.fn(),
 }));
 
 vi.mock("#/api/typescript-client", () => ({
@@ -37,6 +39,7 @@ vi.mock("#/api/agent-server-config", () => ({
 vi.mock("#/api/settings-service/settings-service.api", () => ({
   default: {
     getSettings: mockGetSettings,
+    getSettingsForConversation: mockGetSettingsForConversation,
   },
 }));
 
@@ -101,6 +104,11 @@ describe("V1ConversationService", () => {
         agent_settings: { llm: { model: "gpt-4o" } },
         conversation_settings: {},
       });
+      mockGetSettingsForConversation.mockResolvedValue({
+        agentSettings: { llm: { model: "gpt-4o" } },
+        conversationSettings: {},
+        secretsEncrypted: true,
+      });
       mockHttpPost.mockResolvedValue({
         data: {
           id: "ignored-server-id",