From 77596e758cf0547c99daee19f24abdd6284966fc Mon Sep 17 00:00:00 2001 From: openhands Date: Wed, 10 Jun 2026 04:27:47 +0000 Subject: [PATCH] Move per-conversation metadata from localStorage to agent-server tags Eliminate the `openhands-agent-server-conversation-metadata` localStorage blob. The five fields it held (selected_repository, selected_branch, git_provider, selected_workspace, active_profile) now ride on each conversation's server-side `tags` map, with the agent-server as the source of truth. Pairs with software-agent-sdk PR #3621, which relaxes the tag-key regex from `^[a-z0-9]+$` to `^[a-z0-9_]+$` so snake_case keys validate. Wire-shape changes: - POST /api/conversations now sends the five metadata fields as `tags` alongside the existing `acpserver` tag (via `mergeMetadataIntoTags`). - PATCH /api/conversations/{id} is used to update single fields: `updateConversationRepository` (existing, now tag-based) and the new `updateConversationActiveProfile`. Both preserve unrelated tags. - `AppConversation` keeps the same flat shape; `toAppConversation` reads from server tags first and falls back to legacy localStorage for un-migrated conversations. Migration: - `conversation-metadata-store.ts` is now `@deprecated` and only retained for the read-side fallback above. - A new `conversation-metadata-migration.ts` runs lazily inside the `usePaginatedConversations` queryFn: for each conversation in the current page, if a legacy localStorage entry exists, PATCH any fields the server is missing onto its tags and clear the entry. Best-effort and retried on the next list refresh on failure. Tests: - New `__tests__/api/conversation-metadata-migration.test.ts` covers the migration helper (PATCH-and-clear, no-op when mirrored, preserve unrelated tags, retry on failure, empty-list no-op). - Rewrote `use-switch-llm-profile-and-log` and conversation-websocket tests to assert against `updateConversationActiveProfile` calls instead of localStorage state. - Updated `use-create-conversation-metadata` to assert against the new POST tags payload; remaining create-conversation call-shape tests pick up the new `active_profile` field on the metadata arg. AGENTS.md: dedupe Files-tab note, add migration note. Co-authored-by: openhands --- AGENTS.md | 4 +- .../conversation-metadata-migration.test.ts | 155 ++++++++++++++++ .../use-create-conversation-metadata.test.ts | 50 +++-- .../new-conversation-button-cloud.test.tsx | 2 + .../features/home/home-chat-launcher.test.tsx | 1 + .../features/home/task-card.test.tsx | 1 + .../conversation-websocket-context.test.tsx | 34 +++- .../mutation/use-create-conversation.test.tsx | 8 +- .../use-switch-llm-profile-and-log.test.tsx | 54 +++--- src/api/agent-server-adapter.ts | 173 ++++++++++++++++-- src/api/conversation-metadata-migration.ts | 105 +++++++++++ src/api/conversation-metadata-store.ts | 16 ++ .../agent-server-conversation-service.api.ts | 107 ++++++++--- ...agent-server-conversation-service.types.ts | 10 + .../conversation-websocket-context.tsx | 21 +-- src/hooks/mutation/use-create-conversation.ts | 48 ++--- .../use-switch-llm-profile-and-log.ts | 25 +-- .../query/use-paginated-conversations.ts | 8 + 18 files changed, 670 insertions(+), 152 deletions(-) create mode 100644 __tests__/api/conversation-metadata-migration.test.ts create mode 100644 src/api/conversation-metadata-migration.ts diff --git a/AGENTS.md b/AGENTS.md index 4ec6835ef..03c667f91 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -630,9 +630,9 @@ When adding code that needs a new string, decide up front which rule it falls un - 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. -- Files tab diff-view default logic: keyed off `useHasAttachedSource()` (`src/hooks/use-has-attached-source.ts`), which is true when the user explicitly attached _either_ a repo (`conversation.selected_repository`) _or_ a local workspace (`getStoredConversationMetadata(id).selected_workspace`, persisted by `createConversation` when `workingDirOverride` is supplied). The agent-server pre-initialises every conversation workspace as a git worktree for its own change tracking, so do NOT use a filesystem probe (`git status` / `useUnifiedGetGitChanges`) as the attachment signal — that was tried in earlier iterations and made every fresh no-attachment conversation incorrectly default to diff view. The companion `useHasGitCommits` probe (`src/hooks/query/use-has-git-commits.ts`) then suppresses diff view for attached-but-empty cases (unborn HEAD, non-git workspace). +- Files tab diff-view default logic: keyed off `useHasAttachedSource()` (`src/hooks/use-has-attached-source.ts`), which is true when the user explicitly attached _either_ a repo (`conversation.selected_repository`) _or_ a local workspace (`conversation.selected_workspace`). Both fields are populated by `toAppConversation` from the agent-server's per-conversation `tags` map; `createConversation` writes the `selected_workspace` tag when `workingDirOverride` is supplied. The agent-server pre-initialises every conversation workspace as a git worktree for its own change tracking, so do NOT use a filesystem probe (`git status` / `useUnifiedGetGitChanges`) as the attachment signal — that was tried in earlier iterations and made every fresh no-attachment conversation incorrectly default to diff view. The companion `useHasGitCommits` probe (`src/hooks/query/use-has-git-commits.ts`) then suppresses diff view for attached-but-empty cases (unborn HEAD, non-git workspace). -- Files tab diff-view default logic: keyed off `useHasAttachedSource()` (`src/hooks/use-has-attached-source.ts`), which is true when the user explicitly attached _either_ a repo (`conversation.selected_repository`) _or_ a local workspace (`getStoredConversationMetadata(id).selected_workspace`, persisted by `createConversation` when `workingDirOverride` is supplied). The agent-server pre-initialises every conversation workspace as a git worktree for its own change tracking, so do NOT use a filesystem probe (`git status` / `useUnifiedGetGitChanges`) as the attachment signal — that was tried in earlier iterations and made every fresh no-attachment conversation incorrectly default to diff view. The companion `useHasGitCommits` probe (`src/hooks/query/use-has-git-commits.ts`) then suppresses diff view for attached-but-empty cases (unborn HEAD, non-git workspace). +- Per-conversation metadata (`selected_repository`, `selected_branch`, `git_provider`, `selected_workspace`, `active_profile`) now lives in agent-server `tags` instead of the legacy `openhands-agent-server-conversation-metadata` localStorage blob. The five tag keys are declared in `AGENT_CANVAS_METADATA_TAG_KEYS` (`src/api/agent-server-adapter.ts`); the agent-server validator constrains tag keys to `^[a-z0-9_]+$` (SDK PR #3621 relaxed the previous `^[a-z0-9]+$` to allow underscores). Writers: `createConversation` stamps the full set on `POST /api/conversations` via `mergeMetadataIntoTags`; `updateConversationRepository` and `updateConversationActiveProfile` PATCH single fields and preserve unrelated tags (e.g. `acpserver`). Readers: `toAppConversation` reads server tags first and falls back to the legacy localStorage entry for un-migrated conversations. The legacy store (`src/api/conversation-metadata-store.ts`) is `@deprecated` and only kept alive by the fallback + a one-shot lazy migration (`src/api/conversation-metadata-migration.ts`) wired into `usePaginatedConversations`'s `queryFn`: for each listed conversation it PATCHes legacy values onto missing server tags, then clears the localStorage entry. Once one upgrade cycle has passed, the store, the fallback branches, and the migration helper can all be deleted. - Collapsible thinking: `ThinkAction` events and LLM extended reasoning (`reasoning_content` / `thinking_blocks` on `ActionEvent`) are rendered as collapsible sections via `CollapsibleThinking` (`src/components/conversation-events/chat/event-message-components/collapsible-thinking.tsx`). Collapsed by default to keep the chat compact — the thinking is often in English regardless of the user's conversation language. The `getReasoningContent()` helper in `event-thought-helpers.ts` extracts the content, preferring `reasoning_content` (plain string) and falling back to Anthropic `thinking_blocks`. i18n keys: `THINKING$TITLE`, `THINKING$EXPAND`, `THINKING$COLLAPSE`. Tests: `__tests__/components/conversation-events/chat/event-message-think-action.test.tsx`. diff --git a/__tests__/api/conversation-metadata-migration.test.ts b/__tests__/api/conversation-metadata-migration.test.ts new file mode 100644 index 000000000..70b783015 --- /dev/null +++ b/__tests__/api/conversation-metadata-migration.test.ts @@ -0,0 +1,155 @@ +import { ConversationClient } from "@openhands/typescript-client/clients"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { migrateLegacyConversationMetadata } from "#/api/conversation-metadata-migration"; +import { + getStoredConversationMetadata, + setStoredConversationMetadata, +} from "#/api/conversation-metadata-store"; +import type { AppConversation } from "#/api/conversation-service/agent-server-conversation-service.types"; + +const updateConversation = vi.fn(); + +vi.mock("@openhands/typescript-client/clients", async () => { + const actual = await vi.importActual< + typeof import("@openhands/typescript-client/clients") + >("@openhands/typescript-client/clients"); + return { + ...actual, + ConversationClient: vi.fn(function ConversationClientMock() { + return { updateConversation }; + }), + }; +}); + +vi.mock("#/api/agent-server-client-options", () => ({ + getAgentServerClientOptions: vi.fn(() => ({ + host: "http://localhost:18000", + apiKey: "test-key", + workingDir: "workspace/project", + })), +})); + +vi.mock("#/api/backend-registry/active-store", () => ({ + getActiveBackend: vi.fn(() => ({ + backend: { kind: "local", id: "default-local" }, + })), +})); + +const makeConversation = ( + id: string, + tags: Record | null = null, +): AppConversation => + ({ + id, + title: "x", + createdAt: "2024-01-01T00:00:00Z", + updatedAt: "2024-01-01T00:00:00Z", + selected_repository: null, + selected_branch: null, + git_provider: null, + selected_workspace: null, + active_profile: null, + tags, + }) as unknown as AppConversation; + +describe("migrateLegacyConversationMetadata", () => { + beforeEach(() => { + vi.clearAllMocks(); + window.localStorage.clear(); + updateConversation.mockResolvedValue(undefined); + vi.mocked(ConversationClient).mockClear(); + }); + + it("PATCHes legacy localStorage entries onto server tags and clears the entry", async () => { + setStoredConversationMetadata("conv-1", { + selected_repository: "octocat/hello-world", + selected_branch: "main", + git_provider: "github", + selected_workspace: null, + active_profile: "team-default", + }); + + // Server has no agent-canvas tags yet — pure legacy state. + await migrateLegacyConversationMetadata([makeConversation("conv-1", null)]); + + expect(updateConversation).toHaveBeenCalledTimes(1); + expect(updateConversation).toHaveBeenCalledWith("conv-1", { + tags: { + selected_repository: "octocat/hello-world", + selected_branch: "main", + git_provider: "github", + active_profile: "team-default", + }, + }); + expect(getStoredConversationMetadata("conv-1")).toBeNull(); + }); + + it("skips fields the server already has and clears the localStorage entry without PATCHing when fully mirrored", async () => { + setStoredConversationMetadata("conv-2", { + selected_repository: "octocat/hello-world", + selected_branch: "main", + git_provider: "github", + }); + + // The server already mirrors every field — nothing to migrate, just + // drop the stale legacy entry. + await migrateLegacyConversationMetadata([ + makeConversation("conv-2", { + selected_repository: "octocat/hello-world", + selected_branch: "main", + git_provider: "github", + }), + ]); + + expect(updateConversation).not.toHaveBeenCalled(); + expect(getStoredConversationMetadata("conv-2")).toBeNull(); + }); + + it("preserves unrelated server tags (e.g. acpserver) when merging in legacy values", async () => { + setStoredConversationMetadata("conv-3", { + selected_repository: "octocat/hello-world", + selected_branch: null, + git_provider: "github", + }); + + await migrateLegacyConversationMetadata([ + makeConversation("conv-3", { acpserver: "claude-code" }), + ]); + + expect(updateConversation).toHaveBeenCalledTimes(1); + const [, body] = updateConversation.mock.calls[0]; + expect(body.tags).toEqual({ + acpserver: "claude-code", + selected_repository: "octocat/hello-world", + git_provider: "github", + }); + }); + + it("leaves the legacy entry in place if the PATCH fails so the next refresh retries", async () => { + setStoredConversationMetadata("conv-4", { + selected_repository: "octocat/hello-world", + selected_branch: null, + git_provider: "github", + }); + updateConversation.mockRejectedValueOnce(new Error("boom")); + + await migrateLegacyConversationMetadata([makeConversation("conv-4", null)]); + + expect(getStoredConversationMetadata("conv-4")).not.toBeNull(); + }); + + it("is a no-op when no conversations are passed", async () => { + setStoredConversationMetadata("conv-5", { + selected_repository: "octocat/hello-world", + selected_branch: null, + git_provider: "github", + }); + + await migrateLegacyConversationMetadata([]); + + expect(updateConversation).not.toHaveBeenCalled(); + // Untouched — the entry will be migrated next time the list query + // returns conv-5. + expect(getStoredConversationMetadata("conv-5")).not.toBeNull(); + }); +}); diff --git a/__tests__/api/use-create-conversation-metadata.test.ts b/__tests__/api/use-create-conversation-metadata.test.ts index 5fae5835c..22010583d 100644 --- a/__tests__/api/use-create-conversation-metadata.test.ts +++ b/__tests__/api/use-create-conversation-metadata.test.ts @@ -7,7 +7,25 @@ import { renderHook, waitFor } from "@testing-library/react"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import React from "react"; import { useCreateConversation } from "#/hooks/mutation/use-create-conversation"; -import { getStoredConversationMetadata } from "#/api/conversation-metadata-store"; + +/** + * The metadata that USED to live in `conversation-metadata-store` localStorage + * (and was tested against that store here) now rides on the + * `POST /api/conversations` payload's `tags` field. These tests assert that + * wire shape directly — the localStorage shim is deprecated, and the + * agent-server is the canonical source. See `agent-server-adapter.ts` + * (AGENT_CANVAS_METADATA_TAG_KEYS) and SDK PR #3621. + */ +const lastCreatePayload = (): Record => { + const calls = mockHttpPost.mock.calls; + expect(calls.length).toBeGreaterThan(0); + return calls[calls.length - 1][1] as Record; +}; + +const tagsFromLastCreate = (): Record | undefined => { + const payload = lastCreatePayload(); + return payload.tags as Record | undefined; +}; const { mockHttpPost, @@ -123,7 +141,7 @@ describe("useCreateConversation persists selected repository metadata", () => { window.localStorage.clear(); }); - it("stores the selected repo/branch/provider in the metadata store after a successful create", async () => { + it("sends selected repo/branch/provider tags on POST /api/conversations", async () => { const { result } = renderHook(() => useCreateConversation(), { wrapper }); result.current.mutate({ @@ -137,15 +155,14 @@ describe("useCreateConversation persists selected repository metadata", () => { await waitFor(() => expect(result.current.isSuccess).toBe(true)); - expect(getStoredConversationMetadata("conv-new")).toEqual({ + expect(tagsFromLastCreate()).toEqual({ selected_repository: "octocat/hello-world", selected_branch: "main", git_provider: "github", - selected_workspace: null, }); }); - it("stores the selected workspace path when only a workspace (no repo) is attached", async () => { + it("sends the selected_workspace tag when only a workspace (no repo) is attached", async () => { const { result } = renderHook(() => useCreateConversation(), { wrapper }); result.current.mutate({ @@ -155,27 +172,26 @@ describe("useCreateConversation persists selected repository metadata", () => { await waitFor(() => expect(result.current.isSuccess).toBe(true)); - // We persist the workspace path so `useHasAttachedSource` can default - // the Files tab to diff view even when no repo was picked. - expect(getStoredConversationMetadata("conv-new")).toEqual({ - selected_repository: null, - selected_branch: null, - git_provider: null, + // `useHasAttachedSource` reads `selected_workspace` from server tags + // to default the Files tab to diff view even when no repo was picked. + expect(tagsFromLastCreate()).toEqual({ selected_workspace: "/home/me/code/some-project", }); }); - it("does not write metadata when neither a repository nor a workspace is attached", async () => { + it("omits the tags field entirely when neither a repository nor a workspace is attached", async () => { const { result } = renderHook(() => useCreateConversation(), { wrapper }); result.current.mutate({ query: "scratch session" }); await waitFor(() => expect(result.current.isSuccess).toBe(true)); - expect(getStoredConversationMetadata("conv-new")).toBeNull(); + // No metadata + no ACP server → buildStartConversationRequest leaves + // `tags` off the wire entirely (same shape as the original behaviour). + expect(lastCreatePayload().tags).toBeUndefined(); }); - it("stamps the active LLM profile even when no repo or workspace is attached (#1082)", async () => { + it("stamps the active_profile tag even when no repo or workspace is attached (#1082)", async () => { mockUseLlmProfiles.mockReturnValue({ data: { active_profile: "team-default" }, }); @@ -186,11 +202,7 @@ describe("useCreateConversation persists selected repository metadata", () => { await waitFor(() => expect(result.current.isSuccess).toBe(true)); - expect(getStoredConversationMetadata("conv-new")).toEqual({ - selected_repository: null, - selected_branch: null, - git_provider: null, - selected_workspace: null, + expect(tagsFromLastCreate()).toEqual({ active_profile: "team-default", }); }); diff --git a/__tests__/components/features/conversation-panel/new-conversation-button-cloud.test.tsx b/__tests__/components/features/conversation-panel/new-conversation-button-cloud.test.tsx index 158eee273..7b328916a 100644 --- a/__tests__/components/features/conversation-panel/new-conversation-button-cloud.test.tsx +++ b/__tests__/components/features/conversation-panel/new-conversation-button-cloud.test.tsx @@ -155,6 +155,7 @@ describe("NewConversationButton (cloud)", () => { selected_repository: "octo/dog", selected_branch: "trunk", git_provider: "github", + active_profile: null, }, undefined, undefined, @@ -197,6 +198,7 @@ describe("NewConversationButton (cloud)", () => { selected_repository: "octo/no-main", selected_branch: "main", git_provider: "github", + active_profile: null, }, undefined, undefined, diff --git a/__tests__/components/features/home/home-chat-launcher.test.tsx b/__tests__/components/features/home/home-chat-launcher.test.tsx index b86f6b2b0..8f84d11c4 100644 --- a/__tests__/components/features/home/home-chat-launcher.test.tsx +++ b/__tests__/components/features/home/home-chat-launcher.test.tsx @@ -382,6 +382,7 @@ describe("HomeChatLauncher", () => { selected_repository: "org/repo", selected_branch: "main", git_provider: "github", + active_profile: null, }, undefined, undefined, diff --git a/__tests__/components/features/home/task-card.test.tsx b/__tests__/components/features/home/task-card.test.tsx index 82ca47187..dea5594c7 100644 --- a/__tests__/components/features/home/task-card.test.tsx +++ b/__tests__/components/features/home/task-card.test.tsx @@ -138,6 +138,7 @@ describe("TaskCard", () => { selected_repository: MOCK_TASK_1.repo, selected_branch: null, git_provider: MOCK_TASK_1.git_provider, + active_profile: null, }, undefined, undefined, diff --git a/__tests__/contexts/conversation-websocket-context.test.tsx b/__tests__/contexts/conversation-websocket-context.test.tsx index 462212b33..c71df13cb 100644 --- a/__tests__/contexts/conversation-websocket-context.test.tsx +++ b/__tests__/contexts/conversation-websocket-context.test.tsx @@ -8,9 +8,25 @@ import { useOptimisticUserMessageStore } from "#/stores/optimistic-user-message- import { useBrowserStore } from "#/stores/browser-store"; import { useUserConversation } from "#/hooks/query/use-user-conversation"; import EventService from "#/api/event-service/event-service.api"; -import { getStoredConversationMetadata } from "#/api/conversation-metadata-store"; import type { MessageEvent } from "#/types/agent-server/core"; +// The websocket context now PATCHes the `active_profile` server tag through +// the conversation service when a model switch arrives over the socket. +// Mock the static method so this test asserts the call shape without +// touching the agent-server client. The mock object must be created via +// `vi.hoisted` because `vi.mock` is hoisted above plain top-level decls. +const { updateActiveProfileMock } = vi.hoisted(() => ({ + updateActiveProfileMock: vi.fn().mockResolvedValue(undefined), +})); +vi.mock( + "#/api/conversation-service/agent-server-conversation-service.api", + () => ({ + default: { + updateConversationActiveProfile: updateActiveProfileMock, + }, + }), +); + // Captures the main socket's `onMessage` (`handleMainMessage`) so tests can // drive the live message path without a real WebSocket. Only the main socket // gets a non-empty url (planning stays ""), so url presence discriminates it. @@ -123,7 +139,7 @@ describe("ConversationWebSocketProvider — conversation-scoped event store", () }, }); - it("stamps active_profile on a successful agent-triggered model switch so it survives reload", async () => { + it("PATCHes active_profile on a successful agent-triggered model switch so it survives reload", async () => { // Arrange: open a conversation with a real ws url so the main socket's // onMessage (handleMainMessage) is wired and captured. render( @@ -145,11 +161,15 @@ describe("ConversationWebSocketProvider — conversation-scoped event store", () }); }); - // Assert: the profile identity is persisted to stored metadata — the same - // field the chat-header switcher reads after a reload (#1082). Without the - // stamp this stays null and the header falls back to ambiguous matching. - expect(getStoredConversationMetadata("conv-switch")?.active_profile).toBe( - "fast-opus", + // Assert: the profile identity is PATCHed onto the conversation's + // `active_profile` server tag — the same value the chat-header + // switcher reads after a reload (#1082). Without this the header + // falls back to ambiguous matching on `llm.model`. + await waitFor(() => + expect(updateActiveProfileMock).toHaveBeenCalledWith( + "conv-switch", + "fast-opus", + ), ); }); diff --git a/__tests__/hooks/mutation/use-create-conversation.test.tsx b/__tests__/hooks/mutation/use-create-conversation.test.tsx index ebc562b16..24b50d477 100644 --- a/__tests__/hooks/mutation/use-create-conversation.test.tsx +++ b/__tests__/hooks/mutation/use-create-conversation.test.tsx @@ -82,15 +82,19 @@ describe("useCreateConversation", () => { }); await waitFor(() => { + // The metadata object now also carries `active_profile` (null when + // no profile is active) — see `useCreateConversation` and the + // deprecation of `conversation-metadata-store`. The mocked + // `useLlmProfiles` in this test returns no data, so the field is null. expect(createConversationSpy).toHaveBeenCalledWith( "Please address the comments", "Focus on review comments", undefined, - { + expect.objectContaining({ selected_repository: "owner/repo", selected_branch: "main", git_provider: "github", - }, + }), undefined, undefined, undefined, diff --git a/__tests__/hooks/mutation/use-switch-llm-profile-and-log.test.tsx b/__tests__/hooks/mutation/use-switch-llm-profile-and-log.test.tsx index 421fb74f8..97024e9bc 100644 --- a/__tests__/hooks/mutation/use-switch-llm-profile-and-log.test.tsx +++ b/__tests__/hooks/mutation/use-switch-llm-profile-and-log.test.tsx @@ -20,50 +20,56 @@ vi.mock("#/utils/custom-toast-handlers", () => ({ displayErrorToast: vi.fn(), })); +// The hook now PATCHes the `active_profile` server tag through the +// conversation service instead of stashing it in localStorage. We mock the +// static method so the test asserts the call shape without touching the +// network or the agent-server client. The mock object must be created via +// `vi.hoisted` because `vi.mock` is hoisted above plain top-level decls. +const { updateActiveProfileMock } = vi.hoisted(() => ({ + updateActiveProfileMock: vi.fn().mockResolvedValue(undefined), +})); +vi.mock( + "#/api/conversation-service/agent-server-conversation-service.api", + () => ({ + default: { + updateConversationActiveProfile: updateActiveProfileMock, + }, + }), +); + import { useSwitchLlmProfileAndLog } from "#/hooks/mutation/use-switch-llm-profile-and-log"; -import { - getStoredConversationMetadata, - setStoredConversationMetadata, -} from "#/api/conversation-metadata-store"; describe("useSwitchLlmProfileAndLog", () => { beforeEach(() => { vi.clearAllMocks(); - window.localStorage.clear(); }); - it("stamps the switched-to profile onto the conversation metadata, preserving repo/workspace (#1082)", () => { + it("PATCHes the switched-to profile onto the conversation tag (#1082)", () => { switchMutateMock.mockImplementation((_vars, opts) => opts?.onSuccess?.()); - // Repo metadata persisted at creation must survive the merge. - setStoredConversationMetadata("conv-1", { - selected_repository: "octocat/hello-world", - selected_branch: "main", - git_provider: "github", - }); const { result } = renderHook(() => useSwitchLlmProfileAndLog()); result.current.switchAndLog("conv-1", "claude-sonnet-4.6"); - expect(getStoredConversationMetadata("conv-1")).toEqual({ - selected_repository: "octocat/hello-world", - selected_branch: "main", - git_provider: "github", - selected_workspace: null, - active_profile: "claude-sonnet-4.6", - }); + // The merge with existing repo/workspace tags happens server-side in + // `updateConversationActiveProfile`; this test just asserts the wrapper + // routed the switch through the service with the right arguments. + expect(updateActiveProfileMock).toHaveBeenCalledWith( + "conv-1", + "claude-sonnet-4.6", + ); }); - it("does not stamp metadata for the home-page activate path (conversationId === null)", () => { + it("does not PATCH a tag on the home-page activate path (conversationId === null)", () => { switchMutateMock.mockImplementation((_vars, opts) => opts?.onSuccess?.()); const { result } = renderHook(() => useSwitchLlmProfileAndLog()); result.current.switchAndLog(null, "claude-sonnet-4.6"); - // No conversation to scope the stamp to → nothing written. - expect(getStoredConversationMetadata("conv-1")).toBeNull(); + // No conversation to scope the stamp to → no PATCH. + expect(updateActiveProfileMock).not.toHaveBeenCalled(); }); - it("does not stamp metadata when the switch fails", () => { + it("does not PATCH a tag when the switch fails", () => { switchMutateMock.mockImplementation((_vars, opts) => opts?.onError?.(new Error("boom")), ); @@ -71,6 +77,6 @@ describe("useSwitchLlmProfileAndLog", () => { const { result } = renderHook(() => useSwitchLlmProfileAndLog()); result.current.switchAndLog("conv-1", "claude-sonnet-4.6"); - expect(getStoredConversationMetadata("conv-1")).toBeNull(); + expect(updateActiveProfileMock).not.toHaveBeenCalled(); }); }); diff --git a/src/api/agent-server-adapter.ts b/src/api/agent-server-adapter.ts index 385dde907..c80901829 100644 --- a/src/api/agent-server-adapter.ts +++ b/src/api/agent-server-adapter.ts @@ -2,7 +2,7 @@ import { ACP_SETTINGS_KEYS } from "@openhands/typescript-client"; import { SKILLS_CATALOG } from "@openhands/extensions/skills"; import { DEFAULT_SETTINGS } from "#/services/settings"; import { ExecutionStatus } from "#/types/agent-server/core"; -import { Settings, SettingsValue } from "#/types/settings"; +import { Provider, Settings, SettingsValue } from "#/types/settings"; import { getAcpPreferredDefaultModel, getAcpProvider, @@ -62,11 +62,13 @@ export interface DirectConversationInfo { } | null; /** * Arbitrary string-keyed conversation tags surfaced by the agent-server - * (see ``ConversationInfo.tags``). Canvas only consumes one key today — - * ``ACP_SERVER_TAG_KEY`` ("acpserver") — but the field is typed as a - * generic record so future readers don't need another wire-shape change. - * Keys are constrained to ``^[a-z0-9]+$`` by the agent-server validator; - * values are opaque strings. + * (see ``ConversationInfo.tags``). Canvas writes six keys today: + * ``ACP_SERVER_TAG_KEY`` ("acpserver") and the five agent-canvas UI + * metadata fields (``selected_repository``, ``selected_branch``, + * ``git_provider``, ``selected_workspace``, ``active_profile``) declared + * in ``AGENT_CANVAS_METADATA_TAG_KEYS``. Keys are constrained to + * ``^[a-z0-9_]+$`` by the agent-server validator (SDK PR #3621); + * values are opaque strings up to 256 chars. */ tags?: Record | null; } @@ -288,7 +290,16 @@ export function getDefaultConversationTitle(conversationId: string): string { export function toAppConversation( info: DirectConversationInfo, ): AppConversation { - const metadata = getStoredConversationMetadata(info.id); + // Server tags are the canonical home for per-conversation, UI-only + // metadata (`selected_repository`, `selected_branch`, `git_provider`, + // `selected_workspace`, `active_profile`) since SDK PR #3621 relaxed the + // tag-key validator. Fall back to the legacy `conversation-metadata-store` + // localStorage blob for conversations created before this migration — + // `conversation-metadata-migration.ts` lazily pushes those into server + // tags in the background, after which the legacy entry is cleared and + // this branch goes unused. + const fromTags = readMetadataFromTags(info.tags); + const legacy = getStoredConversationMetadata(info.id); // ACPAgent conversations carry a sentinel ``llm`` on older SDKs. Prefer the // runtime model fields when available, then the configured ``acp_model`` that // Canvas saves for built-in providers. ``agent_kind`` still gates model @@ -302,11 +313,18 @@ export function toAppConversation( return { id: info.id, created_by_user_id: null, - selected_repository: metadata?.selected_repository ?? null, - selected_branch: metadata?.selected_branch ?? null, - git_provider: metadata?.git_provider ?? null, - selected_workspace: metadata?.selected_workspace ?? null, - active_profile: metadata?.active_profile ?? null, + selected_repository: + fromTags.selected_repository ?? legacy?.selected_repository ?? null, + selected_branch: + fromTags.selected_branch ?? legacy?.selected_branch ?? null, + git_provider: + (fromTags.git_provider as Provider | null | undefined) ?? + legacy?.git_provider ?? + null, + selected_workspace: + fromTags.selected_workspace ?? legacy?.selected_workspace ?? null, + active_profile: fromTags.active_profile ?? legacy?.active_profile ?? null, + tags: info.tags ?? null, title: info.title?.trim() ? info.title : getDefaultConversationTitle(info.id), @@ -406,6 +424,117 @@ type ConversationSettingsPayload = SettingsRecord & { export const ACP_SERVER_TAG_KEY = "acpserver"; +/** + * Conversation tag keys agent-canvas uses to round-trip per-conversation, + * UI-only metadata through the agent-server's existing `tags` field — see + * `software-agent-sdk` PR #3621 which relaxed the key validator from + * `^[a-z0-9]+$` to `^[a-z0-9_]+$` so we could use natural snake_case names + * here instead of mashed-together strings like `selectedworkspace`. + * + * Together these five keys deprecate `conversation-metadata-store.ts` (the + * old per-conversation localStorage blob). Migration is graceful: when + * `toAppConversation` finds an entry in localStorage but no value in + * server tags, it surfaces the legacy value while + * `conversation-metadata-migration.ts` runs in the background to PATCH the + * tags onto the live conversation and clear the localStorage entry on + * success. + * + * NOTE: Keys are kept snake_case to match the wire shape on `AppConversation` + * (e.g. `selected_repository`). Do not rename them — the relaxed SDK + * validator requires the agent-server to be at least the SDK release that + * shipped PR #3621. + */ +export const SELECTED_REPOSITORY_TAG_KEY = "selected_repository"; +export const SELECTED_BRANCH_TAG_KEY = "selected_branch"; +export const GIT_PROVIDER_TAG_KEY = "git_provider"; +export const SELECTED_WORKSPACE_TAG_KEY = "selected_workspace"; +export const ACTIVE_PROFILE_TAG_KEY = "active_profile"; + +/** + * The full set of tag keys agent-canvas writes. Used by tag-merge helpers + * to scrub the legacy localStorage value off the wire once the server has + * a canonical answer, and by tests to assert the wire shape. + */ +export const AGENT_CANVAS_METADATA_TAG_KEYS = [ + SELECTED_REPOSITORY_TAG_KEY, + SELECTED_BRANCH_TAG_KEY, + GIT_PROVIDER_TAG_KEY, + SELECTED_WORKSPACE_TAG_KEY, + ACTIVE_PROFILE_TAG_KEY, +] as const; + +/** + * Per-conversation, UI-only metadata round-tripped through server tags. + * Mirrors the legacy `ConversationMetadata` shape but consumers should treat + * it as a generic record; the migration helper handles both shapes. + */ +export interface ConversationMetadataTags { + selected_repository?: string | null; + selected_branch?: string | null; + git_provider?: string | null; + selected_workspace?: string | null; + active_profile?: string | null; +} + +/** + * Read the five agent-canvas metadata fields out of a server tag map. + * Returns the tags _projected_ onto the metadata shape, dropping any other + * tags (e.g. `acpserver`) so callers can treat the result as a typed view. + */ +export function readMetadataFromTags( + tags: Record | null | undefined, +): ConversationMetadataTags { + if (!tags) return {}; + return { + selected_repository: tags[SELECTED_REPOSITORY_TAG_KEY] ?? null, + selected_branch: tags[SELECTED_BRANCH_TAG_KEY] ?? null, + git_provider: tags[GIT_PROVIDER_TAG_KEY] ?? null, + selected_workspace: tags[SELECTED_WORKSPACE_TAG_KEY] ?? null, + active_profile: tags[ACTIVE_PROFILE_TAG_KEY] ?? null, + }; +} + +/** + * Build the agent-canvas metadata subset of a server tag map. Empty/null + * values are dropped so we never persist `selected_branch: ""` over a valid + * `null` — keeps the wire format identical to what a fresh write would + * produce. + */ +export function metadataToTagPatch( + metadata: ConversationMetadataTags, +): Record { + const patch: Record = {}; + for (const key of AGENT_CANVAS_METADATA_TAG_KEYS) { + const value = metadata[key]; + if (typeof value === "string" && value.length > 0) { + patch[key] = value; + } + } + return patch; +} + +/** + * Merge a metadata patch into an existing tag map, preserving any tags + * agent-canvas does not own (e.g. `acpserver` for ACP conversations). + * + * The agent-server's `PATCH /api/conversations/{id}` with `{tags: ...}` + * REPLACES the entire tag map (see `software-agent-sdk` `conversation_service.update_conversation`), + * so every PATCH must include the tags we want to preserve. + */ +export function mergeMetadataIntoTags( + existing: Record | null | undefined, + metadata: ConversationMetadataTags, +): Record { + const next: Record = { ...(existing ?? {}) }; + // Drop our own keys first so a metadata value of `null`/`""` clears the + // tag rather than leaving the stale one in place. + for (const key of AGENT_CANVAS_METADATA_TAG_KEYS) { + delete next[key]; + } + Object.assign(next, metadataToTagPatch(metadata)); + return next; +} + const CONVERSATION_SETTINGS_METADATA_KEYS = new Set([ "schema_version", "agent_settings", @@ -826,6 +955,13 @@ export interface StartConversationOptions { encryptedConversationSettings?: Record; secretsEncrypted?: boolean; customSecrets?: Array<{ name: string; description?: string }>; + /** + * UI-only per-conversation metadata to stamp onto the new conversation's + * server tags at creation time (`selected_repository`, `selected_branch`, + * `git_provider`, `selected_workspace`, `active_profile`). Mixed in + * alongside the ACP `acpserver` tag — see `mergeMetadataIntoTags`. + */ + metadata?: ConversationMetadataTags; } export function buildStartConversationRequest( @@ -869,8 +1005,18 @@ export function buildStartConversationRequest( worktree: true, }; + // Stamp all conversation tags at creation: the ACP server identifier + // (if applicable) plus the five agent-canvas UI metadata fields the chat + // page reads back via `toAppConversation` / `readMetadataFromTags`. + const tags: Record = {}; if (acpServerTag) { - payload.tags = { [ACP_SERVER_TAG_KEY]: acpServerTag }; + tags[ACP_SERVER_TAG_KEY] = acpServerTag; + } + if (options.metadata) { + Object.assign(tags, metadataToTagPatch(options.metadata)); + } + if (Object.keys(tags).length > 0) { + payload.tags = tags; } // ``secrets_encrypted`` makes the agent-server run every request secret through @@ -954,6 +1100,7 @@ export async function buildStartConversationRequestWithEncryptedSettings(options plugins?: PluginSpec[]; conversationId?: string; workingDir?: string; + metadata?: ConversationMetadataTags; }): Promise> { const { SecretsService } = await import("./secrets-service"); diff --git a/src/api/conversation-metadata-migration.ts b/src/api/conversation-metadata-migration.ts new file mode 100644 index 000000000..d989af325 --- /dev/null +++ b/src/api/conversation-metadata-migration.ts @@ -0,0 +1,105 @@ +import { ConversationClient } from "@openhands/typescript-client/clients"; +import { + AGENT_CANVAS_METADATA_TAG_KEYS, + ConversationMetadataTags, + mergeMetadataIntoTags, + readMetadataFromTags, +} from "./agent-server-adapter"; +import { getAgentServerClientOptions } from "./agent-server-client-options"; +import { getActiveBackend } from "./backend-registry/active-store"; +import { + getStoredConversationMetadata, + removeStoredConversationMetadata, +} from "./conversation-metadata-store"; +import type { AppConversation } from "./conversation-service/agent-server-conversation-service.types"; + +/** + * One-shot lazy migration of the legacy + * `openhands-agent-server-conversation-metadata` localStorage blob onto the + * agent-server's `tags` field on each conversation. + * + * Triggered by the conversation list query (see `use-paginated-conversations`) + * with the conversations the user just loaded. For each conversation that: + * + * 1. has a legacy localStorage entry, + * 2. and is missing the corresponding tag on the server, + * + * we PATCH the merged tag map onto the conversation and (on success) drop + * the localStorage entry. We deliberately do NOT overwrite a tag that + * already exists on the server — once a conversation has been migrated, + * the server is the source of truth and the legacy entry is just stale. + * + * Best-effort: any individual failure is swallowed (logged through console) + * so a bad network/permissions hiccup doesn't break the conversation list. + * Subsequent calls will retry until the legacy entry is gone. + * + * Cloud conversations are skipped — local profile/workspace metadata never + * lived in the cloud localStorage blob in the first place. + */ +const inFlight = new Set(); + +export async function migrateLegacyConversationMetadata( + conversations: AppConversation[], +): Promise { + if (getActiveBackend().backend.kind === "cloud") return; + if (typeof window === "undefined") return; + if (conversations.length === 0) return; + + const client = new ConversationClient(getAgentServerClientOptions()); + + await Promise.all( + conversations.map(async (conversation) => { + if (inFlight.has(conversation.id)) return; + + const legacy = getStoredConversationMetadata(conversation.id); + if (!legacy) return; + + const serverTags = conversation.tags ?? null; + const fromServer = readMetadataFromTags(serverTags); + + // Pick up only the fields the legacy entry has that the server is + // missing. If every legacy field is already mirrored on the server, + // there's nothing left to migrate — just drop the localStorage entry. + const merged: ConversationMetadataTags = {}; + let needsPatch = false; + for (const key of AGENT_CANVAS_METADATA_TAG_KEYS) { + const legacyValue = legacy[key as keyof typeof legacy]; + const serverValue = fromServer[key]; + if ( + typeof legacyValue === "string" && + legacyValue.length > 0 && + !serverValue + ) { + merged[key] = legacyValue; + needsPatch = true; + } + } + + if (!needsPatch) { + removeStoredConversationMetadata(conversation.id); + return; + } + + inFlight.add(conversation.id); + try { + const nextTags = mergeMetadataIntoTags(serverTags, { + ...fromServer, + ...merged, + }); + await client.updateConversation(conversation.id, { tags: nextTags }); + removeStoredConversationMetadata(conversation.id); + } catch (error) { + // Leave the localStorage entry in place so the next list refresh + // retries. Silent for the user — the worst case is the legacy + // entry just keeps surfacing through the fallback in + // `toAppConversation`. + console.warn( + `[conversation-metadata-migration] failed to migrate ${conversation.id}`, + error, + ); + } finally { + inFlight.delete(conversation.id); + } + }), + ); +} diff --git a/src/api/conversation-metadata-store.ts b/src/api/conversation-metadata-store.ts index a9de5aa7b..31d001c1f 100644 --- a/src/api/conversation-metadata-store.ts +++ b/src/api/conversation-metadata-store.ts @@ -1,5 +1,21 @@ import { Provider } from "#/types/settings"; +/** + * @deprecated Replaced by agent-server `tags` on each conversation + * (see `AGENT_CANVAS_METADATA_TAG_KEYS` in `agent-server-adapter.ts` and + * the conversation-tag SDK PR #3621). This module is kept only to: + * + * 1. Allow `toAppConversation` to read legacy values as a fallback when + * the server has not yet been migrated. + * 2. Allow `conversation-metadata-migration.ts` to lazily push legacy + * values onto server tags and then clear the localStorage entry. + * + * Do NOT add new readers or writers. New per-conversation metadata + * belongs in server tags via `mergeMetadataIntoTags`. Once an upgrade + * cycle has passed and the localStorage blob is unlikely to still exist + * on user machines, this whole module can be deleted along with the + * fallback branches in `toAppConversation` and the migration helper. + */ const STORAGE_KEY = "openhands-agent-server-conversation-metadata"; export interface ConversationMetadata { 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 bb2f57b1c..b713218f8 100644 --- a/src/api/conversation-service/agent-server-conversation-service.api.ts +++ b/src/api/conversation-service/agent-server-conversation-service.api.ts @@ -9,7 +9,7 @@ import { VSCodeClient, } from "@openhands/typescript-client/clients"; import { v4 as uuidv4 } from "uuid"; -import { Provider } from "#/types/settings"; + import type { ConversationRuntimeContext } from "#/api/conversation-file-upload.api"; import { buildHttpBaseUrl } from "#/utils/websocket-url"; import { @@ -33,10 +33,13 @@ import { updateCloudConversationPublicFlag, } from "../cloud/conversation-service.api"; import { + ConversationMetadataTags, DirectConversationInfo, buildStartConversationRequestWithEncryptedSettings, emptyHooksResponse, getDefaultConversationTitle, + mergeMetadataIntoTags, + readMetadataFromTags, toAppConversation, toConversationPage, } from "../agent-server-adapter"; @@ -48,9 +51,7 @@ import { import SettingsService from "../settings-service/settings-service.api"; import { ConversationMetadata, - getStoredConversationMetadata, removeStoredConversationMetadata, - setStoredConversationMetadata, } from "../conversation-metadata-store"; import type { GetHooksResponse, @@ -386,6 +387,22 @@ class AgentServerConversationService { workingDirOverride ?? buildConversationWorkingDir(conversationId), ); + // Build the metadata payload once. Conversations the user did NOT + // attach a repo/workspace to send no metadata tags and create an empty + // tag map on the server — same on-the-wire shape as before the + // localStorage shim. `active_profile` is the LLM-profile snapshot the + // caller (e.g. `useCreateConversation`) supplied at creation time; + // several saved profiles can share the same model (#1082), so the chat + // switcher needs the profile name explicitly rather than recovering it + // from `llm.model`. + const tagMetadata: ConversationMetadataTags = { + selected_repository: metadata?.selected_repository ?? null, + selected_branch: metadata?.selected_branch ?? null, + git_provider: metadata?.git_provider ?? null, + selected_workspace: workingDirOverride ?? null, + active_profile: metadata?.active_profile ?? null, + }; + // Use encrypted settings to avoid exposing secrets in the browser const payload = await buildStartConversationRequestWithEncryptedSettings({ settings, @@ -394,6 +411,7 @@ class AgentServerConversationService { plugins, conversationId, workingDir, + metadata: tagMetadata, }); const data = await new ConversationClient( @@ -402,21 +420,6 @@ class AgentServerConversationService { const localBackend = getEffectiveLocalBackend(); if (!localBackend) throw new NoBackendAvailableError(); - if (metadata?.selected_repository || workingDirOverride) { - // The agent-server runtime has no concept of selected repo/branch/ - // workspace, so persist the home-page selection client-side. - // `toAppConversation` reads the repo/branch fields back to hydrate - // the chat-page badges; `useHasAttachedSource` reads - // `selected_workspace` to default the Files tab to Diff mode when - // the user explicitly attached a local workspace. - setStoredConversationMetadata(data.id, { - selected_repository: metadata?.selected_repository ?? null, - selected_branch: metadata?.selected_branch ?? null, - git_provider: metadata?.git_provider ?? null, - selected_workspace: workingDirOverride ?? null, - }); - } - return { id: data.id, created_by_user_id: null, @@ -518,17 +521,29 @@ class AgentServerConversationService { branch?: string | null, gitProvider?: string | null, ): Promise { - if (repository) { - const existing = getStoredConversationMetadata(conversationId); - setStoredConversationMetadata(conversationId, { - ...(existing ?? {}), - selected_repository: repository, - selected_branch: branch ?? null, - git_provider: (gitProvider as Provider | null | undefined) ?? null, - }); - } else { - removeStoredConversationMetadata(conversationId); - } + // Round-trip selected_repository / selected_branch / git_provider through + // server tags via PATCH /api/conversations/{id} so the values persist + // server-side and survive a localStorage wipe. `updateConversation` + // replaces the entire `tags` field, so we merge against the current + // server tag map to preserve unrelated tags (e.g. `acpserver`). + const [current] = await this.batchGetAppConversations([conversationId]); + const existing = requireAppConversation(current, conversationId); + + const mergedTags = mergeMetadataIntoTags(existing.tags ?? null, { + ...readMetadataFromTags(existing.tags ?? null), + selected_repository: repository, + selected_branch: branch ?? null, + git_provider: gitProvider ?? null, + }); + + await new ConversationClient( + getAgentServerClientOptions(), + ).updateConversation(conversationId, { tags: mergedTags }); + + // Also clear any legacy localStorage entry — once we've successfully + // written to server tags it is the canonical source. + removeStoredConversationMetadata(conversationId); + const [conversation] = await this.batchGetAppConversations([ conversationId, ]); @@ -667,6 +682,40 @@ class AgentServerConversationService { return requireAppConversation(conversation, conversationId); } + /** + * Stamp the `active_profile` server tag on a conversation — used by the + * chat-header profile switcher to remember which named LLM profile the + * user picked. Several saved profiles can share a model (#1082), so this + * is the only way to recover the profile name after a reload. Other + * agent-canvas tags (selected_repository, …) and unrelated tags + * (`acpserver`) are preserved via `mergeMetadataIntoTags`. + */ + static async updateConversationActiveProfile( + conversationId: string, + profileName: string | null, + ): Promise { + if (getActiveBackend().backend.kind === "cloud") { + // Cloud conversations don't use local profiles — switching is a + // settings change rather than a tag patch. No-op here. + return; + } + + const [current] = await this.batchGetAppConversations([conversationId]); + const existing = requireAppConversation(current, conversationId); + + const mergedTags = mergeMetadataIntoTags(existing.tags ?? null, { + ...readMetadataFromTags(existing.tags ?? null), + active_profile: profileName, + }); + + await new ConversationClient( + getAgentServerClientOptions(), + ).updateConversation(conversationId, { tags: mergedTags }); + + // Same legacy-cleanup pattern as updateConversationRepository. + removeStoredConversationMetadata(conversationId); + } + /** * Switches the LLM profile for the running conversation when one is open * (POST /switch_profile — per-conversation swap, doesn't change the user's diff --git a/src/api/conversation-service/agent-server-conversation-service.types.ts b/src/api/conversation-service/agent-server-conversation-service.types.ts index 4db37da00..faf1df5a6 100644 --- a/src/api/conversation-service/agent-server-conversation-service.types.ts +++ b/src/api/conversation-service/agent-server-conversation-service.types.ts @@ -180,6 +180,16 @@ export interface AppConversation { * older client) — consumers fall back to model-matching. */ active_profile?: string | null; + /** + * Raw conversation tags as returned by the agent-server (`ConversationInfo.tags`). + * Includes the five agent-canvas metadata tags (see `AGENT_CANVAS_METADATA_TAG_KEYS`) + * as well as any other tags the conversation carries — most commonly + * `acpserver` for ACP conversations. Surfaced so callers updating a single + * tag can merge against the full existing map and `PATCH` it back without + * dropping unrelated tags (the agent-server's `update_conversation` replaces + * the entire `tags` field when provided). + */ + tags?: Record | null; public?: boolean; sub_conversation_ids: string[]; } diff --git a/src/contexts/conversation-websocket-context.tsx b/src/contexts/conversation-websocket-context.tsx index c3c8389fd..6714f5db3 100644 --- a/src/contexts/conversation-websocket-context.tsx +++ b/src/contexts/conversation-websocket-context.tsx @@ -66,10 +66,7 @@ import { invalidateConversationQueries, updateConversationLlmModelInCache, } from "#/hooks/mutation/conversation-mutation-utils"; -import { - getStoredConversationMetadata, - setStoredConversationMetadata, -} from "#/api/conversation-metadata-store"; +import AgentServerConversationService from "#/api/conversation-service/agent-server-conversation-service.api"; export type WebSocketConnectionState = | "CONNECTING" @@ -593,15 +590,13 @@ export function ConversationWebSocketProvider({ // Mirror the user-driven `/model` path: persist the profile so the // chat-header switcher shows the right name after a reload, even - // when several profiles share a model (#1082). - const prevMetadata = getStoredConversationMetadata(conversationId); - setStoredConversationMetadata(conversationId, { - selected_repository: prevMetadata?.selected_repository ?? null, - selected_branch: prevMetadata?.selected_branch ?? null, - git_provider: prevMetadata?.git_provider ?? null, - selected_workspace: prevMetadata?.selected_workspace ?? null, - active_profile: switchLLMObservation.observation.profile_name, - }); + // when several profiles share a model (#1082). Best-effort PATCH + // to the conversation's `active_profile` tag; a failed write + // only loses the cosmetic profile name on reload. + void AgentServerConversationService.updateConversationActiveProfile( + conversationId, + switchLLMObservation.observation.profile_name, + ).catch(() => undefined); if (switchLLMObservation.observation.active_model) { updateConversationLlmModelInCache( diff --git a/src/hooks/mutation/use-create-conversation.ts b/src/hooks/mutation/use-create-conversation.ts index 41fedff3c..2fdaf3032 100644 --- a/src/hooks/mutation/use-create-conversation.ts +++ b/src/hooks/mutation/use-create-conversation.ts @@ -5,10 +5,6 @@ import { SuggestedTask } from "#/utils/types"; import { Provider } from "#/types/settings"; import { useTracking } from "#/hooks/use-tracking"; import { useLlmProfiles } from "#/hooks/query/use-llm-profiles"; -import { - getStoredConversationMetadata, - setStoredConversationMetadata, -} from "#/api/conversation-metadata-store"; interface CreateConversationVariables { query?: string; @@ -55,40 +51,36 @@ export const useCreateConversation = () => { agentType, } = variables; + // Stamp the active LLM profile onto the new conversation so the chat + // switcher shows the exact profile even when several profiles share a + // model (#1082). Passed through `metadata.active_profile` so the + // conversation service writes it as a server tag in the same request + // as the repo/workspace selection — one round-trip, no localStorage. + // Keep the metadata arg `null` when there's nothing meaningful to + // stamp (no repo, no profile) so the wire-shape matches a pre-tag + // create — tests assert call args against `null` in that case. + const activeProfile = llmProfiles?.active_profile ?? null; + const metadata = + repository || activeProfile + ? { + selected_repository: repository?.name ?? null, + selected_branch: repository?.branch ?? null, + git_provider: repository?.gitProvider ?? null, + active_profile: activeProfile, + } + : null; + const conversation = await AgentServerConversationService.createConversation( query, conversationInstructions, plugins, - repository - ? { - selected_repository: repository.name, - selected_branch: repository.branch ?? null, - git_provider: repository.gitProvider, - } - : null, + metadata, workingDir, parentConversationId, agentType, ); - // Stamp the active LLM profile onto the (local) conversation so the - // chat switcher shows the exact profile even when several profiles - // share a model (#1082). Cloud conversations don't use local profiles - // (app_conversation_id stays null until the sandbox is READY). Merge so - // the repo/workspace metadata the service just persisted is preserved. - const localConversationId = conversation.app_conversation_id; - if (localConversationId && llmProfiles?.active_profile) { - const prev = getStoredConversationMetadata(localConversationId); - setStoredConversationMetadata(localConversationId, { - selected_repository: prev?.selected_repository ?? null, - selected_branch: prev?.selected_branch ?? null, - git_provider: prev?.git_provider ?? null, - selected_workspace: prev?.selected_workspace ?? null, - active_profile: llmProfiles.active_profile, - }); - } - // OpenHands cloud pattern: when the start task isn't immediately // READY (cloud sandbox is still provisioning), // app_conversation_id is null. We return a `task-{id}` URL so the diff --git a/src/hooks/mutation/use-switch-llm-profile-and-log.ts b/src/hooks/mutation/use-switch-llm-profile-and-log.ts index 752594f26..45301a20c 100644 --- a/src/hooks/mutation/use-switch-llm-profile-and-log.ts +++ b/src/hooks/mutation/use-switch-llm-profile-and-log.ts @@ -3,10 +3,7 @@ import { useTranslation } from "react-i18next"; import { getLastRenderableEventId } from "#/hooks/chat/model-command-event-anchor"; import { recordModelSwitchMessage } from "#/hooks/chat/record-model-switch-message"; import { useSwitchLlmProfile } from "#/hooks/mutation/use-switch-llm-profile"; -import { - getStoredConversationMetadata, - setStoredConversationMetadata, -} from "#/api/conversation-metadata-store"; +import AgentServerConversationService from "#/api/conversation-service/agent-server-conversation-service.api"; import { displayErrorToast } from "#/utils/custom-toast-handlers"; import { I18nKey } from "#/i18n/declaration"; @@ -36,17 +33,15 @@ export function useSwitchLlmProfileAndLog() { profileName, anchorEventId, ); - // Keep the per-conversation profile identity fresh so the - // chat-header switcher shows the right name after a reload - // (the agent-server only round-trips the model string). #1082 - const prev = getStoredConversationMetadata(conversationId); - setStoredConversationMetadata(conversationId, { - selected_repository: prev?.selected_repository ?? null, - selected_branch: prev?.selected_branch ?? null, - git_provider: prev?.git_provider ?? null, - selected_workspace: prev?.selected_workspace ?? null, - active_profile: profileName, - }); + // Persist the profile identity on the conversation's server + // tags so the chat-header switcher recovers it after a reload + // even when several profiles share a model (#1082). Best-effort + // — a failed PATCH only loses the display, the runtime switch + // already succeeded. + void AgentServerConversationService.updateConversationActiveProfile( + conversationId, + profileName, + ).catch(() => undefined); } }, onError: (err: unknown) => { diff --git a/src/hooks/query/use-paginated-conversations.ts b/src/hooks/query/use-paginated-conversations.ts index a6d60be0f..7beba7fbd 100644 --- a/src/hooks/query/use-paginated-conversations.ts +++ b/src/hooks/query/use-paginated-conversations.ts @@ -4,6 +4,7 @@ import { useIsAuthed } from "./use-is-authed"; import { isNoBackend } from "#/api/backend-registry/active-store"; import { useActiveBackend } from "#/contexts/active-backend-context"; import { AppConversationPage } from "#/api/conversation-service/agent-server-conversation-service.types"; +import { migrateLegacyConversationMetadata } from "#/api/conversation-metadata-migration"; export const usePaginatedConversations = (limit: number = 20) => { const { data: userIsAuthenticated } = useIsAuthed(); @@ -29,6 +30,13 @@ export const usePaginatedConversations = (limit: number = 20) => { pageParam, ); + // Lazily migrate any legacy `conversation-metadata-store` localStorage + // entries for the conversations the user just loaded onto the + // agent-server's `tags` field. Best-effort, fire-and-forget — see + // `conversation-metadata-migration.ts`. We don't await so a slow + // PATCH never delays the list rendering. + void migrateLegacyConversationMetadata(result.items); + return result; }, enabled: !!userIsAuthenticated && hasBackend,