diff --git a/__tests__/api/agent-server-adapter.test.ts b/__tests__/api/agent-server-adapter.test.ts index dadff585..144b3c88 100644 --- a/__tests__/api/agent-server-adapter.test.ts +++ b/__tests__/api/agent-server-adapter.test.ts @@ -554,16 +554,83 @@ describe("toAppConversation", () => { expect(result.llm_model).toBe("claude-sonnet-4-6"); }); - it("marks ACP conversations and nulls llm_model so the chat UI can't mislead", () => { - // The SDK's ACPAgent carries a sentinel ``llm`` (``acp-managed``) for - // cost-attribution only; the *real* model lives on the ACP subprocess via - // ``acp_model`` and isn't surfaced on ``agent.llm.model``. Surfacing the - // sentinel as ``llm_model`` would let SwitchProfileButton render an - // affordance to "change the model" on a Claude-Code conversation while - // the running subprocess kept its own — a confusing silent no-op. + it("marks ACP conversations and surfaces the configured acp_model", () => { + // The SDK's ACPAgent may still carry a sentinel ``llm`` (``acp-managed``) + // for cost-attribution. Consumers should see the concrete ACP model Canvas + // configured, while SwitchProfileButton remains gated by agent_kind. const result = toAppConversation({ ...baseInfo, - agent: { kind: "ACPAgent", llm: { model: "acp-managed" } }, + agent: { + kind: "ACPAgent", + acp_model: "claude-sonnet-4-6", + llm: { model: "acp-managed" }, + }, + }); + expect(result.agent_kind).toBe("acp"); + expect(result.llm_model).toBe("claude-sonnet-4-6"); + }); + + it("prefers ACP runtime model fields over configured acp_model", () => { + const result = toAppConversation({ + ...baseInfo, + current_model_id: "claude-sonnet-4-6", + current_model_name: "Claude Sonnet 4.6", + agent: { + kind: "ACPAgent", + acp_model: "claude-opus-4-7", + llm: { model: "acp-managed" }, + }, + }); + expect(result.agent_kind).toBe("acp"); + expect(result.llm_model).toBe("Claude Sonnet 4.6"); + }); + + it("does not surface ACP default placeholders when a configured model exists", () => { + const result = toAppConversation({ + ...baseInfo, + current_model_id: "default", + current_model_name: "Default (recommended)", + agent: { + kind: "ACPAgent", + acp_model: "claude-sonnet-4-6", + llm: { model: "acp-managed" }, + }, + }); + expect(result.agent_kind).toBe("acp"); + expect(result.llm_model).toBe("claude-sonnet-4-6"); + }); + + it("falls back to a non-sentinel ACP llm.model for SDKs that mirror acp_model there", () => { + const result = toAppConversation({ + ...baseInfo, + agent: { kind: "ACPAgent", llm: { model: "claude-sonnet-4-6" } }, + }); + expect(result.agent_kind).toBe("acp"); + expect(result.llm_model).toBe("claude-sonnet-4-6"); + }); + + it("filters ACP default placeholders surfaced via the configured acp_model", () => { + // Older settings may have persisted the SDK's literal "default" string + // into ``acp_model``. Surfacing it on the chip would lie about what's + // running — the placeholder filter is applied to every candidate, not + // just the runtime fields. + const result = toAppConversation({ + ...baseInfo, + agent: { + kind: "ACPAgent", + acp_model: "Default (recommended)", + llm: { model: "acp-managed" }, + }, + }); + expect(result.agent_kind).toBe("acp"); + expect(result.llm_model).toBeNull(); + }); + + it("filters ACP default placeholders surfaced via agent.llm.model", () => { + // Same defense, one rung lower in the precedence chain. + const result = toAppConversation({ + ...baseInfo, + agent: { kind: "ACPAgent", llm: { model: "default" } }, }); expect(result.agent_kind).toBe("acp"); expect(result.llm_model).toBeNull(); @@ -647,8 +714,7 @@ describe("buildRuntimeServicesSystemSuffix", () => { url_from_agent: "http://localhost:18001", api_prefix: "/api/automation", docs_url: "http://localhost:18001/api/automation/docs", - openapi_url: - "http://localhost:18001/api/automation/openapi.json", + openapi_url: "http://localhost:18001/api/automation/openapi.json", auth_env_var: "OPENHANDS_AUTOMATION_API_KEY", }, }, @@ -660,9 +726,7 @@ describe("buildRuntimeServicesSystemSuffix", () => { expect(suffix).toContain("dev:automation"); expect(suffix).toContain("http://localhost:18000"); expect(suffix).toContain("http://localhost:18001"); - expect(suffix).toContain( - "http://localhost:18001/api/automation/docs", - ); + expect(suffix).toContain("http://localhost:18001/api/automation/docs"); expect(suffix).toContain("X-API-Key: $OPENHANDS_AUTOMATION_API_KEY"); expect(suffix).toContain(""); // The "don't guess" line should reference the actual agent-server URL @@ -1007,12 +1071,14 @@ describe("buildStartConversationRequest — ACP discriminator", () => { expect(payload.agent.acp_command).toEqual([]); }); - it("treats acp_model: '' (empty string) as 'no override'", () => { + it("seeds the provider default when settings contains an empty acp_model", () => { // The form may carry an empty string after a user clears the model - // input; the agent-server expects ``null`` for "use provider default." - // Empty strings would pass the spawn but bias model selection on - // some providers (e.g. claude-agent-acp's _meta would set - // ``model: ''``). + // input. Older behavior left ``acp_model`` absent and relied on the + // agent-server's own default; the registry-default path + // (resolveEffectiveAcpModel) is now authoritative on Canvas's side, + // so an empty string resolves to the provider's ``default_model`` + // before the request leaves the client. Keeps the displayed Settings + // → Agent default in sync with what the runtime actually starts. const payload = buildStartConversationRequest({ settings: { ...DEFAULT_SETTINGS, @@ -1028,14 +1094,30 @@ describe("buildStartConversationRequest — ACP discriminator", () => { agent: Record & { acp_model?: unknown }; }; - // ``buildConfiguredAcpAgentSettings`` already filters undefined + - // null; the empty string falls through, which is a known nit. The - // adapter's contract is "forward what settings says"; the - // canonicalisation belongs in the save path - // (``agent-settings.tsx::handleSave`` already does - // ``acpModel.trim() || null``). Pin the current behaviour so a - // future change to either side is a deliberate decision. - expect(payload.agent.acp_model).toBe(""); + expect(payload.agent.acp_model).toBe("claude-opus-4-7"); + }); + + it("omits acp_model for the custom preset when none is configured", () => { + // The Custom preset has no registered ``default_model``, so an empty + // ``acp_model`` falls through to ``undefined`` — the agent-server then + // applies its own default. Distinct from the built-in providers + // which substitute their registry default. + const payload = buildStartConversationRequest({ + settings: { + ...DEFAULT_SETTINGS, + agent_settings: { + schema_version: 1, + agent_kind: "acp", + acp_server: "custom", + acp_command: ["my-custom-acp"], + acp_model: "", + }, + }, + }) as { + agent: Record & { acp_model?: unknown }; + }; + + expect(payload.agent.acp_model).toBeUndefined(); }); it("ACP → OpenHands → ACP round trip leaves no field leakage", () => { diff --git a/__tests__/api/agent-server-conversation-service.test.ts b/__tests__/api/agent-server-conversation-service.test.ts index 14b460a3..74fbef56 100644 --- a/__tests__/api/agent-server-conversation-service.test.ts +++ b/__tests__/api/agent-server-conversation-service.test.ts @@ -502,6 +502,69 @@ describe("AgentServerConversationService", () => { ); }); + it("preserves the new ACP model fields through the wire normalizer", async () => { + // Direct adapter tests pass DirectConversationInfo objects in-process + // and so can't catch the case where the wire-format normalizer + // (``normalizeAgent`` + ``requireDirectConversationInfo``) drops the + // newly-added ACP fields. Exercises the full HTTP -> AppConversation + // path so the chip's model resolution actually has the inputs it + // needs on a real local-backend fetch. + mockHttpGet.mockResolvedValue({ + data: [ + { + id: "conv-acp-model-wire", + created_at: "2024-01-01", + updated_at: "2024-01-01", + agent: { + kind: "ACPAgent", + acp_model: "claude-opus-4-7", + llm: { model: "acp-managed" }, + }, + current_model_id: "claude-opus-4-7", + current_model_name: "Claude Opus 4.7", + tags: { acpserver: "claude-code" }, + }, + ], + }); + + const [conversation] = + await AgentServerConversationService.batchGetAppConversations([ + "conv-acp-model-wire", + ]); + + // ``current_model_name`` wins the precedence chain in the adapter. + expect(conversation?.agent_kind).toBe("acp"); + expect(conversation?.llm_model).toBe("Claude Opus 4.7"); + }); + + it("falls back to acp_model when SDK runtime fields are absent on the wire", async () => { + // Older agent-servers don't populate ``current_model_*``. The + // adapter must still surface a model on the chip — falling through + // to ``agent.acp_model`` (the Canvas-configured value). + mockHttpGet.mockResolvedValue({ + data: [ + { + id: "conv-acp-fallback", + created_at: "2024-01-01", + updated_at: "2024-01-01", + agent: { + kind: "ACPAgent", + acp_model: "claude-sonnet-4-6", + llm: { model: "acp-managed" }, + }, + tags: { acpserver: "claude-code" }, + }, + ], + }); + + const [conversation] = + await AgentServerConversationService.batchGetAppConversations([ + "conv-acp-fallback", + ]); + + expect(conversation?.llm_model).toBe("claude-sonnet-4-6"); + }); + it("extracts the acpserver tag from the wire payload for the sidebar chip", async () => { // The agent-server stamps ``tags.acpserver`` at conversation create // time (see ``buildStartConversationRequest``); the read path diff --git a/__tests__/components/features/chat/components/chat-input-actions.test.tsx b/__tests__/components/features/chat/components/chat-input-actions.test.tsx index 04bafd7e..a2ebc143 100644 --- a/__tests__/components/features/chat/components/chat-input-actions.test.tsx +++ b/__tests__/components/features/chat/components/chat-input-actions.test.tsx @@ -12,7 +12,13 @@ import type { Backend } from "#/api/backend-registry/types"; const useActiveConversationMock = vi.fn< () => { - data: { conversation_id: string; llm_model: string | null } | undefined; + data: + | { + conversation_id: string; + agent_kind?: "openhands" | "acp"; + llm_model: string | null; + } + | undefined; } >(() => ({ data: undefined })); @@ -78,6 +84,26 @@ describe("ChatInputActions", () => { ).not.toBeInTheDocument(); }); + it("renders the static model label for local ACP conversations", () => { + useActiveConversationMock.mockReturnValue({ + data: { + conversation_id: "test-conversation-id", + agent_kind: "acp", + llm_model: "claude-sonnet-4-6", + }, + }); + + renderWithProviders(); + + expect(screen.getByTestId("chat-input-llm-model")).toHaveAttribute( + "title", + "claude-sonnet-4-6", + ); + expect( + screen.queryByTestId("switch-profile-button-stub"), + ).not.toBeInTheDocument(); + }); + it("renders the active conversation model on a cloud backend", () => { setRegisteredBackends([cloudBackend]); setActiveSelection({ backendId: cloudBackend.id }); diff --git a/__tests__/components/features/chat/components/chat-input-model.test.tsx b/__tests__/components/features/chat/components/chat-input-model.test.tsx index 870e326e..3feab477 100644 --- a/__tests__/components/features/chat/components/chat-input-model.test.tsx +++ b/__tests__/components/features/chat/components/chat-input-model.test.tsx @@ -62,6 +62,42 @@ describe("ChatInputModel", () => { ).not.toBeInTheDocument(); }); + it("renders an ACP conversation model and links to Agent settings", () => { + useActiveConversationMock.mockReturnValue({ + data: { + conversation_id: "test-conversation-id", + agent_kind: "acp", + llm_model: "claude-sonnet-4-6", + }, + }); + + renderWithProviders(); + + const model = screen.getByTestId("chat-input-llm-model"); + expect(model).toHaveAttribute("title", "claude-sonnet-4-6"); + fireEvent.click(model); + expect(screen.getByRole("link")).toHaveAttribute("href", "/settings/agent"); + }); + + it("does not fall back to the OpenHands settings model for active ACP conversations", () => { + useActiveConversationMock.mockReturnValue({ + data: { + conversation_id: "test-conversation-id", + agent_kind: "acp", + llm_model: null, + }, + }); + useSettingsMock.mockReturnValue({ + data: { llm_model: "openai/gpt-4o" }, + }); + + renderWithProviders(); + + expect( + screen.queryByTestId("chat-input-llm-model"), + ).not.toBeInTheDocument(); + }); + it("falls back to the user's default model from settings when there is no active conversation", () => { // Arrange — home page render: no conversation yet, but the user has // a default model configured. The switcher should still show. @@ -80,6 +116,26 @@ describe("ChatInputModel", () => { ); }); + it("uses the ACP settings model on the home page when ACP is active", () => { + useActiveConversationMock.mockReturnValue({ data: undefined }); + useSettingsMock.mockReturnValue({ + data: { + llm_model: "openai/gpt-4o", + agent_settings: { + agent_kind: "acp", + acp_model: "gemini-2.5-pro", + }, + }, + }); + + renderWithProviders(); + + const model = screen.getByTestId("chat-input-llm-model"); + expect(model).toHaveAttribute("title", "gemini-2.5-pro"); + fireEvent.click(model); + expect(screen.getByRole("link")).toHaveAttribute("href", "/settings/agent"); + }); + it("renders nothing when neither the conversation nor settings provide an llm_model", () => { useActiveConversationMock.mockReturnValue({ data: undefined }); useSettingsMock.mockReturnValue({ data: undefined }); @@ -116,27 +172,29 @@ describe("ChatInputModel", () => { ).not.toBeInTheDocument(); }); - it("renders nothing on the home page when ACP is the default agent", () => { - // Home-screen gating: no active conversation, so the - // per-conversation ``agent_kind`` check can't catch this case. - // Fall back to ``settings.agent_settings.agent_kind`` — that's - // the kind the next-created conversation will inherit. Showing - // the LLM picker here would put up a control that becomes a - // silent no-op the moment the user sends their first message. + it("shows the provider default on the home page when ACP is the default agent and no model is saved", () => { + // Home-screen gating: no active conversation and no saved ``acp_model``. + // The next-created conversation will inherit the provider's + // ``default_model`` (see buildConfiguredAcpAgentSettings), so the picker + // shows that same default — matching what the runtime will actually + // start. Picker links to /settings/agent (not /settings) since + // ``settings.llm_model`` doesn't apply to ACP. useActiveConversationMock.mockReturnValue({ data: undefined }); useSettingsMock.mockReturnValue({ data: { agent_settings: { agent_kind: "acp", acp_server: "claude-code" }, - // settings.llm_model is still set (the user has an OpenHands - // default configured), but agent_kind=acp wins. + // settings.llm_model is set (user has an OpenHands default + // configured), but agent_kind=acp suppresses it. llm_model: "anthropic/claude-sonnet-4-20250514", }, }); renderWithProviders(); - expect( - screen.queryByTestId("chat-input-llm-model"), - ).not.toBeInTheDocument(); + const model = screen.getByTestId("chat-input-llm-model"); + // Claude Code's registered default; see CLAUDE_MODELS in acp-providers.ts. + expect(model).toHaveAttribute("title", "claude-opus-4-7"); + fireEvent.click(model); + expect(screen.getByRole("link")).toHaveAttribute("href", "/settings/agent"); }); }); diff --git a/__tests__/components/features/chat/switch-profile-button.test.tsx b/__tests__/components/features/chat/switch-profile-button.test.tsx index 7219fbf2..be022760 100644 --- a/__tests__/components/features/chat/switch-profile-button.test.tsx +++ b/__tests__/components/features/chat/switch-profile-button.test.tsx @@ -123,7 +123,7 @@ describe("SwitchProfileButton", () => { expect(screen.getByTestId("switch-profile-button")).toBeDisabled(); }); - it("renders nothing for ACP conversations even when profiles exist", () => { + it("renders nothing for ACP conversations even when profiles and a display model exist", () => { // ACPAgent conversations route prompts to a CLI subprocess whose model is // set via ``acp_model`` in Settings → Agent, not by the LLM-profile // picker. Letting the user "switch the LLM" here would silently no-op @@ -133,7 +133,7 @@ describe("SwitchProfileButton", () => { data: { id: "conv-1", agent_kind: "acp", - llm_model: null, + llm_model: "claude-sonnet-4-6", }, }); diff --git a/__tests__/components/features/conversation-panel/conversation-card.test.tsx b/__tests__/components/features/conversation-panel/conversation-card.test.tsx index f5687b9d..ac6c79db 100644 --- a/__tests__/components/features/conversation-panel/conversation-card.test.tsx +++ b/__tests__/components/features/conversation-panel/conversation-card.test.tsx @@ -597,12 +597,12 @@ describe("ConversationCard", () => { }); }); - describe("ACP agent badge", () => { - it("renders the resolved display name for a known ACP server", () => { - // ``claude-code`` resolves through the ACP_PROVIDERS registry to the - // human display name "Claude Code". The badge always renders for - // ACP conversations — it's identity info, not gated by the LLM- - // profile preference. + describe("Agent chip", () => { + it("renders the brand mark + model for an ACP conversation with a model", () => { + // PR 730 wires ``current_model_name``/``current_model_id``/configured + // ``acp_model`` into ``llm_model`` on the adapter so ACP conversations + // arrive at the card with a concrete model string. The chip shows the + // resolved Claude brand mark + that model text. renderWithProviders( { lastUpdatedAt="2021-10-01T12:00:00Z" agentKind="acp" acpServer="claude-code" + llmModel="sonnet" />, ); - const badge = screen.getByTestId("conversation-card-acp-badge"); - expect(badge).toHaveTextContent("Claude Code"); + const chip = screen.getByTestId("conversation-card-agent-chip"); + expect(chip).toHaveTextContent("sonnet"); + expect(chip).toHaveAttribute("title", "Claude Code · sonnet"); + expect( + within(chip).getByTestId("agent-brand-icon-claude-code"), + ).toBeInTheDocument(); }); - it("falls back to the generic 'ACP' label when the server key is unknown", () => { - // The Custom-command preset uses ``acp_server: "custom"`` (and - // future ACP servers Canvas's registry doesn't know about look the - // same here) — the resolver returns null and the chip shows the - // generic ``CONVERSATION$ACP_AGENT_GENERIC`` translation. + it("falls back to the provider display name for an ACP conversation with no model", () => { + // No ``llm_model`` (older agent-server, no SDK runtime fields, no + // configured ``acp_model``) — the chip still renders for identity, with + // the provider name as the text and the brand mark as the icon. + renderWithProviders( + , + ); + + const chip = screen.getByTestId("conversation-card-agent-chip"); + expect(chip).toHaveTextContent("Claude Code"); + expect(chip).toHaveAttribute("title", "Claude Code"); + expect( + within(chip).getByTestId("agent-brand-icon-claude-code"), + ).toBeInTheDocument(); + }); + + it("falls back to the generic terminal glyph when the server key is unknown", () => { + // ``custom`` (and any future ACP server Canvas doesn't know yet) maps + // to the fallback ``cli-generic`` icon and the generic "ACP" label. renderWithProviders( { />, ); - const badge = screen.getByTestId("conversation-card-acp-badge"); - expect(badge).toHaveTextContent("ACP"); + const chip = screen.getByTestId("conversation-card-agent-chip"); + expect(chip).toHaveTextContent("ACP"); + expect( + within(chip).getByTestId("agent-brand-icon-generic"), + ).toBeInTheDocument(); }); - it("falls back to the generic 'ACP' label when the server key is null", () => { + it("falls back to the generic terminal glyph when the server key is null", () => { // ACP conversations missing the ``acpserver`` tag (older clients, - // raw API writes) still get a chip — the goal is "this is an ACP - // conversation" first, exact provider second. + // raw API writes) still get a chip — identity first, exact provider + // second. renderWithProviders( { />, ); - const badge = screen.getByTestId("conversation-card-acp-badge"); - expect(badge).toHaveTextContent("ACP"); + const chip = screen.getByTestId("conversation-card-agent-chip"); + expect(chip).toHaveTextContent("ACP"); + expect( + within(chip).getByTestId("agent-brand-icon-generic"), + ).toBeInTheDocument(); }); - it("does not render the badge for OpenHands conversations", () => { - // The OpenHands rendering path must be untouched — even if a stray - // ``acp_server`` value somehow reaches the prop, the chip stays - // hidden because ``agentKind !== "acp"``. + it("renders the OpenHands logo + model name for native conversations", () => { + // OpenHands native conversations show the same chip pattern as ACP + // cards: the OpenHands logo + the raw ``agent.llm.model`` string. No + // user-preference gate — surfacing the model is identity info, same + // as the ACP path. A stray ``acp_server`` value on an OpenHands card + // must not flip the icon to the Claude/Codex/Gemini brand mark. renderWithProviders( { lastUpdatedAt="2021-10-01T12:00:00Z" agentKind="openhands" acpServer="claude-code" + llmModel="claude-sonnet-4" + />, + ); + + const chip = screen.getByTestId("conversation-card-agent-chip"); + expect(chip).toHaveTextContent("claude-sonnet-4"); + expect(chip).toHaveAttribute("title", "claude-sonnet-4"); + expect( + within(chip).getByTestId("agent-brand-icon-openhands"), + ).toBeInTheDocument(); + expect( + within(chip).queryByTestId("agent-brand-icon-claude-code"), + ).not.toBeInTheDocument(); + }); + + it("hides the chip for OpenHands conversations with no model", () => { + // No model string and no ACP server — nothing to display, so the + // chip collapses entirely rather than showing a bare logo. + renderWithProviders( + , ); expect( - screen.queryByTestId("conversation-card-acp-badge"), + screen.queryByTestId("conversation-card-agent-chip"), ).not.toBeInTheDocument(); }); }); diff --git a/__tests__/components/onboarding/choose-agent-step.test.tsx b/__tests__/components/onboarding/choose-agent-step.test.tsx index b7dcf9a4..fd16ae31 100644 --- a/__tests__/components/onboarding/choose-agent-step.test.tsx +++ b/__tests__/components/onboarding/choose-agent-step.test.tsx @@ -168,7 +168,7 @@ describe("ChooseAgentStep", () => { // ``acp_args`` can't survive and concatenate onto the spawn // command at conversation-create time. acp_args: [], - acp_model: null, + acp_model: "claude-opus-4-7", }); }); @@ -191,6 +191,9 @@ describe("ChooseAgentStep", () => { expect( (call.agent_settings_diff as Record).acp_server, ).toBe(expected); + expect( + (call.agent_settings_diff as Record).acp_model, + ).toBe(ACP_PROVIDERS.find(({ key }) => key === expected)?.default_model); }); it("rebuilds the diff cleanly when the user flips between ACP providers", async () => { diff --git a/__tests__/constants/acp-providers.test.ts b/__tests__/constants/acp-providers.test.ts index 8c9fe287..fb61c78c 100644 --- a/__tests__/constants/acp-providers.test.ts +++ b/__tests__/constants/acp-providers.test.ts @@ -1,5 +1,10 @@ import { describe, expect, it } from "vitest"; -import { getAcpProviderDisplayName } from "#/constants/acp-providers"; +import { + ACP_CUSTOM_PRESET_KEY, + ACP_PROVIDERS, + buildAcpAgentSettingsDiff, + getAcpProviderDisplayName, +} from "#/constants/acp-providers"; describe("getAcpProviderDisplayName", () => { it("resolves the three built-in registry keys to their human names", () => { @@ -28,3 +33,45 @@ describe("getAcpProviderDisplayName", () => { expect(getAcpProviderDisplayName("")).toBeNull(); }); }); + +describe("ACP provider registry", () => { + it("keeps every built-in default model in the UX suggestions", () => { + for (const provider of ACP_PROVIDERS) { + expect(provider.default_model, provider.key).toBeTruthy(); + expect(provider.available_models, provider.key).toBeTruthy(); + expect( + provider.available_models?.some( + (model) => model.id === provider.default_model, + ), + provider.key, + ).toBe(true); + } + }); + + it("does not suggest generic default model placeholders", () => { + for (const provider of ACP_PROVIDERS) { + for (const model of provider.available_models ?? []) { + expect(model.id.toLowerCase()).not.toBe("default"); + expect(model.label.toLowerCase()).not.toContain("default"); + } + } + }); + + it("seeds built-in ACP diffs with the provider default model", () => { + for (const provider of ACP_PROVIDERS) { + expect(buildAcpAgentSettingsDiff(provider.key)).toMatchObject({ + agent_kind: "acp", + acp_server: provider.key, + acp_model: provider.default_model, + }); + } + }); + + it("keeps custom ACP diffs model-optional", () => { + expect(buildAcpAgentSettingsDiff(ACP_CUSTOM_PRESET_KEY)).toMatchObject({ + agent_kind: "acp", + acp_server: ACP_CUSTOM_PRESET_KEY, + acp_model: null, + }); + }); +}); diff --git a/__tests__/routes/agent-settings.test.tsx b/__tests__/routes/agent-settings.test.tsx index 3c796851..ca1656fb 100644 --- a/__tests__/routes/agent-settings.test.tsx +++ b/__tests__/routes/agent-settings.test.tsx @@ -152,6 +152,10 @@ describe("AgentSettingsScreen", () => { }); it("shows the ACP form when the active agent_kind is acp", async () => { + // Use a model ID that isn't in CLAUDE_MODELS so the form falls through to + // the custom-input branch — that's the path this test is asserting (saved + // value round-trips into the visible input). Known IDs go through the + // dropdown instead and are covered separately. vi.spyOn(SettingsService, "getSettings").mockResolvedValue( buildSettings({ agent_settings: { @@ -159,7 +163,7 @@ describe("AgentSettingsScreen", () => { agent_kind: "acp", acp_server: "claude-code", acp_command: ["npx", "-y", "@agentclientprotocol/claude-agent-acp"], - acp_model: "claude-opus-4-5", + acp_model: "my-pinned-fork-model", }, }), ); @@ -174,7 +178,113 @@ describe("AgentSettingsScreen", () => { const modelInput = screen.getByTestId( "agent-model-input", ) as HTMLInputElement; - expect(modelInput.value).toBe("claude-opus-4-5"); + expect(modelInput.value).toBe("my-pinned-fork-model"); + }); + + it("defaults built-in ACP providers to a suggested model when none is saved", async () => { + vi.spyOn(SettingsService, "getSettings").mockResolvedValue( + buildSettings({ + agent_settings: { + schema_version: 1, + agent_kind: "acp", + acp_server: "claude-code", + acp_command: [], + acp_model: null, + }, + }), + ); + + renderAgentSettingsScreen(); + + await screen.findByTestId("agent-command-input"); + expect(screen.getByLabelText("SETTINGS$AGENT_MODEL")).toHaveValue( + "Claude Opus 4.7", + ); + }); + + it("saves the selected built-in ACP model", async () => { + const user = userEvent.setup(); + vi.spyOn(SettingsService, "getSettings").mockResolvedValue( + buildSettings({ + agent_settings: { + schema_version: 1, + agent_kind: "acp", + acp_server: "claude-code", + acp_command: [], + acp_model: null, + }, + }), + ); + const save = vi.spyOn(SettingsService, "saveSettings"); + + renderAgentSettingsScreen(); + await screen.findByTestId("agent-command-input"); + await user.click(screen.getByLabelText("SETTINGS$AGENT_MODEL")); + await user.click(await screen.findByText("Claude Haiku 4.5")); + await user.click(screen.getByTestId("agent-save-button")); + + await waitFor(() => { + expect(save).toHaveBeenCalledTimes(1); + }); + const call = save.mock.calls[0]?.[0] as { + agent_settings_diff?: Record; + }; + expect(call.agent_settings_diff?.acp_model).toBe("claude-haiku-4-5"); + }); + + it("clears the model when switching from a built-in provider to Custom", async () => { + // F3 from review: built-ins seed ``acp_model`` to their registered + // ``default_model`` on load. Picking Custom must not leak that built-in + // default into custom settings — otherwise a user choosing Custom from + // Claude Code would silently save ``acp_model: "claude-opus-4-7"`` on an + // unrelated wrapper. + const user = userEvent.setup(); + vi.spyOn(SettingsService, "getSettings").mockResolvedValue( + buildSettings({ + agent_settings: { + schema_version: 1, + agent_kind: "acp", + acp_server: "claude-code", + acp_command: [], + acp_model: null, + }, + }), + ); + const save = vi.spyOn(SettingsService, "saveSettings"); + + renderAgentSettingsScreen(); + await screen.findByTestId("agent-command-input"); + // Form loads with the Claude Code default visible. + expect(screen.getByLabelText("SETTINGS$AGENT_MODEL")).toHaveValue( + "Claude Opus 4.7", + ); + + // Switch to the Custom preset, then enter a different command — the + // form's ``selectedPreset`` re-derives from the command text, so the + // save path only treats it as Custom once the command no longer matches + // a built-in provider's default. + await user.click(screen.getByTestId("agent-preset-selector")); + await user.click( + await screen.findByRole("option", { name: "SETTINGS$AGENT_PRESET_CUSTOM" }), + ); + const commandInput = screen.getByTestId( + "agent-command-input", + ) as HTMLTextAreaElement; + await user.clear(commandInput); + await user.type(commandInput, "my-custom-acp --flag"); + + await user.click(screen.getByTestId("agent-save-button")); + await waitFor(() => { + expect(save).toHaveBeenCalledTimes(1); + }); + + const call = save.mock.calls[0]?.[0] as { + agent_settings_diff?: Record; + }; + // Custom preset has no registered default — saved acp_model must be null, + // not the inherited Claude Opus default. + expect(call.agent_settings_diff?.acp_server).toBe("custom"); + expect(call.agent_settings_diff?.acp_model).toBeNull(); }); it("saves an ACP diff when switching to ACP + Claude Code", async () => { @@ -205,6 +315,9 @@ describe("AgentSettingsScreen", () => { expect(commandInput.value).toBe( "npx -y @agentclientprotocol/claude-agent-acp", ); + expect(screen.getByLabelText("SETTINGS$AGENT_MODEL")).toHaveValue( + "Claude Opus 4.7", + ); await user.click(screen.getByTestId("agent-save-button")); @@ -225,7 +338,7 @@ describe("AgentSettingsScreen", () => { // ``acp_args`` can't survive and concatenate onto the spawn // command at conversation-create time. acp_args: [], - acp_model: null, + acp_model: "claude-opus-4-7", }); }); diff --git a/src/api/agent-server-adapter.ts b/src/api/agent-server-adapter.ts index 5feb1d61..f3d769ce 100644 --- a/src/api/agent-server-adapter.ts +++ b/src/api/agent-server-adapter.ts @@ -1,7 +1,10 @@ import { DEFAULT_SETTINGS } from "#/services/settings"; import { ExecutionStatus } from "#/types/agent-server/core"; import { Settings, SettingsValue } from "#/types/settings"; -import { ACP_PROVIDERS } from "#/constants/acp-providers"; +import { + ACP_PROVIDERS, + resolveEffectiveAcpModel, +} from "#/constants/acp-providers"; import { getAgentServerClientOptions } from "./agent-server-client-options"; import { isAgentServerToolAvailable } from "./agent-server-compatibility"; import { getAgentServerWorkingDir } from "./agent-server-config"; @@ -39,19 +42,18 @@ export interface DirectConversationInfo { } | null; agent?: { /** - * Pydantic discriminator from the SDK union. ``"ACPAgent"`` means the - * conversation runs an ACP CLI subprocess (model selection lives on - * the subprocess via ``acp_model``, not on ``agent.llm``); ``"Agent"`` - * means the conversation drives an LLM directly through litellm. - * Used by ``toAppConversation`` to null out ``llm_model`` for ACP - * conversations so the chat UI doesn't expose LLM-switch affordances - * that would silently no-op against the running ACP subprocess. + * Pydantic discriminator from the SDK union: ``"ACPAgent"`` for ACP CLI + * subprocesses (model lives on the subprocess via ``acp_model``), + * ``"Agent"`` for direct litellm. Read by {@link toAppConversation}. */ kind?: string | null; + acp_model?: string | null; llm?: { model?: string | null; } | null; } | null; + current_model_id?: string | null; + current_model_name?: string | null; workspace?: { working_dir?: string | null; } | null; @@ -246,14 +248,10 @@ export function toAppConversation( info: DirectConversationInfo, ): AppConversation { const metadata = getStoredConversationMetadata(info.id); - // ACPAgent conversations carry a dummy ``llm`` on the SDK side (the real - // model lives on the ACP subprocess via ``acp_model``), so surfacing - // ``agent.llm.model`` as the conversation's "active LLM" would lie to - // every consumer downstream — most visibly the chat header's - // SwitchProfileButton, which would otherwise let the user switch - // profiles on a Claude-Code conversation while the running subprocess - // keeps its own model. Null at the boundary so no consumer has to - // re-derive the rule. Mirrors OpenHands PR #14401. + // 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 + // switching, so surfacing this string is display-only. const isAcp = info.agent?.kind === "ACPAgent"; // Only surface ``acp_server`` for ACP conversations even if the wire // payload accidentally carries an ``acpserver`` tag on an OpenHands @@ -274,8 +272,17 @@ export function toAppConversation( pr_number: [], agent_kind: isAcp ? "acp" : "openhands", acp_server: acpServer, + // Chip path: no ``providerDefault`` — the chip must distinguish + // "no concrete model" (fall back to the provider display name in + // ConversationCardFooter) from "default" (would lie about what's + // running on the subprocess). llm_model: isAcp - ? null + ? resolveEffectiveAcpModel({ + runtimeName: info.current_model_name, + runtimeId: info.current_model_id, + configured: info.agent?.acp_model, + sdkLlm: info.agent?.llm?.model, + }) : (info.agent?.llm?.model ?? DEFAULT_SETTINGS.llm_model), metrics: info.metrics ? { @@ -580,6 +587,7 @@ function buildConfiguredAcpAgentSettings(settings: Settings): SettingsRecord { // ``buildStartConversationRequest``). const payload: SettingsRecord = {}; for (const key of ACP_SETTINGS_KEYS) { + if (key === "acp_model") continue; if (agentSettings[key] !== undefined && agentSettings[key] !== null) { payload[key] = agentSettings[key]; } @@ -595,18 +603,30 @@ function buildConfiguredAcpAgentSettings(settings: Settings): SettingsRecord { // command for every built-in preset, while leaving ``acp_server: custom`` // (and any unknown key) untouched — those genuinely require the user's // ``acp_command`` entry. + const serverKey = + typeof agentSettings.acp_server === "string" + ? agentSettings.acp_server + : undefined; + const provider = ACP_PROVIDERS.find(({ key }) => key === serverKey); const cmd = payload.acp_command; const isEmpty = Array.isArray(cmd) && cmd.length === 0; const noCommand = cmd === undefined; - if (isEmpty || noCommand) { - const serverKey = - typeof agentSettings.acp_server === "string" - ? agentSettings.acp_server - : undefined; - const provider = ACP_PROVIDERS.find(({ key }) => key === serverKey); - if (provider) { - payload.acp_command = [...provider.default_command]; - } + if ((isEmpty || noCommand) && provider) { + payload.acp_command = [...provider.default_command]; + } + + // Saved settings may carry ``acp_model: null`` (existing users predating + // the default-model registry, or savedfields that the agent-server stripped). + // Fall back to the provider's ``default_model`` so the conversation starts + // with whatever the Settings → Agent UI shows — without that, the form's + // displayed default would silently not take effect at runtime until the + // user re-saved the page. + const effectiveModel = resolveEffectiveAcpModel({ + configured: agentSettings.acp_model as string | null | undefined, + providerDefault: provider?.default_model, + }); + if (effectiveModel) { + payload.acp_model = effectiveModel; } return payload; 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 fe463c99..a0f70726 100644 --- a/src/api/conversation-service/agent-server-conversation-service.api.ts +++ b/src/api/conversation-service/agent-server-conversation-service.api.ts @@ -129,11 +129,17 @@ function normalizeAgent(value: unknown): DirectConversationInfo["agent"] { ? { model: stringOrNull(value.llm.model) } : null; // ``kind`` is the SDK's pydantic discriminator (``"Agent"`` vs ``"ACPAgent"``); - // ``toAppConversation`` reads it to derive ``agent_kind`` and to gate the - // ACP-server chip + ``llm_model`` null-out. Preserving it here makes the - // wire path agree with the unit-test path that builds ``DirectConversationInfo`` + // ``toAppConversation`` reads it to derive ``agent_kind``. ``acp_model`` is + // the Canvas-configured model on the ACPAgent — preserved so the conversation + // adapter and the conversation chip can fall back to it when the SDK runtime + // model fields aren't populated. Preserving these here makes the wire path + // agree with the unit-test path that builds ``DirectConversationInfo`` // directly (e.g. ``__tests__/api/agent-server-adapter.test.ts``). - return { kind: stringOrNull(value.kind), llm }; + return { + kind: stringOrNull(value.kind), + acp_model: stringOrNull(value.acp_model), + llm, + }; } function normalizeWorkspace( @@ -214,6 +220,12 @@ function requireDirectConversationInfo(item: unknown): DirectConversationInfo { agent: normalizeAgent(item.agent), workspace: normalizeWorkspace(item.workspace), tags: normalizeTags(item.tags), + // SDK-runtime ACP model fields (populated when the agent-server supports + // ``ConversationInfo.current_model_*``). Consumed by the conversation + // adapter to drive the per-card chip's model text. Older agent-servers + // omit these — adapter handles ``undefined`` / ``null`` gracefully. + current_model_id: stringOrNull(item.current_model_id), + current_model_name: stringOrNull(item.current_model_name), }; } 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 3261ed4c..09243fdf 100644 --- a/src/api/conversation-service/agent-server-conversation-service.types.ts +++ b/src/api/conversation-service/agent-server-conversation-service.types.ts @@ -127,8 +127,8 @@ export interface AppConversation { * driven Agent, ``"acp"`` for an ACPAgent that delegates to an external * ACP CLI subprocess. Consumers can use this to gate UI affordances that * only make sense for one kind (e.g. the LLM-profile switcher in the chat - * header is a no-op for ACP conversations because model selection lives - * on the subprocess via ``acp_model``, not on ``llm_model``). + * header is a no-op for ACP conversations even though ``llm_model`` may + * carry the ACP subprocess model for display). */ agent_kind?: "openhands" | "acp" | null; /** diff --git a/src/components/features/chat/components/chat-input-actions.tsx b/src/components/features/chat/components/chat-input-actions.tsx index 544cb81b..86acda0f 100644 --- a/src/components/features/chat/components/chat-input-actions.tsx +++ b/src/components/features/chat/components/chat-input-actions.tsx @@ -21,6 +21,7 @@ import { usePauseConversation } from "#/hooks/mutation/use-pause-conversation"; import { useResumeConversation } from "#/hooks/mutation/use-resume-conversation"; import { useActiveBackend } from "#/contexts/active-backend-context"; import { useActiveConversation } from "#/hooks/query/use-active-conversation"; +import { useSettings } from "#/hooks/query/use-settings"; import { useConversationNameContextMenu } from "#/hooks/use-conversation-name-context-menu"; import { useConversationStore } from "#/stores/conversation-store"; import { useAgentState } from "#/hooks/use-agent-state"; @@ -65,10 +66,26 @@ export function ChatInputActions({ const { conversationId } = useOptionalConversationId(); const { data: conversation } = useActiveConversation(); const { backend } = useActiveBackend(); + const { data: settings } = useSettings(); const isCloud = backend.kind === "cloud"; + const isAcpConversation = conversation?.agent_kind === "acp"; + // Home screen: no active conversation yet, but Settings → Agent may already + // be configured for ACP. The chat input on the home page should reflect + // that — the next-created conversation will inherit it — so route the model + // label to ChatInputModel (which knows how to show the ACP model) instead + // of SwitchProfileButton (which would hide itself for ACP and leave the + // home screen with no model affordance at all). + const isHomeAcp = + !conversation && settings?.agent_settings?.agent_kind === "acp"; + const isAcpContext = isAcpConversation || isHomeAcp; const llmDestinationLabel = t( - isCloud ? I18nKey.SETTINGS$LLM_SETTINGS : I18nKey.SETTINGS$LLM_PROFILES, + isAcpContext + ? I18nKey.SETTINGS$NAV_AGENT + : isCloud + ? I18nKey.SETTINGS$LLM_SETTINGS + : I18nKey.SETTINGS$LLM_PROFILES, ); + const modelDestinationPath = isAcpContext ? "/settings/agent" : "/settings"; const webSocketStatus = useUnifiedWebSocketStatus(); const { curAgentState } = useAgentState(); const { conversationMode, setConversationMode } = useConversationStore(); @@ -449,7 +466,7 @@ export function ChatInputActions({
  • @@ -491,7 +508,11 @@ export function ChatInputActions({ )}
    - {isCloud ? : } + {isCloud || isAcpContext ? ( + + ) : ( + + )}
    {hasOverflowItems && ( diff --git a/src/components/features/chat/components/chat-input-model.tsx b/src/components/features/chat/components/chat-input-model.tsx index 04b5af5d..86320514 100644 --- a/src/components/features/chat/components/chat-input-model.tsx +++ b/src/components/features/chat/components/chat-input-model.tsx @@ -8,6 +8,10 @@ import { ContextMenu } from "#/ui/context-menu"; import { Divider } from "#/ui/divider"; import { I18nKey } from "#/i18n/declaration"; import { useActiveBackend } from "#/contexts/active-backend-context"; +import { + ACP_PROVIDERS, + resolveEffectiveAcpModel, +} from "#/constants/acp-providers"; import { cn } from "#/utils/utils"; import React from "react"; import { useTranslation } from "react-i18next"; @@ -28,28 +32,41 @@ export function ChatInputModel() { // Home page has no active conversation; fall back to the user's default // model so the switcher renders consistently across both surfaces. const { data: settings } = useSettings(); - // ACPAgent conversations have no OpenHands LLM (the model lives on the - // ACP subprocess via ``acp_model``), so ``toAppConversation`` writes a - // null ``llm_model`` for them. Don't fall back to ``settings.llm_model`` - // here — that would resurrect the user's *default* OpenHands model on a - // Claude-Code conversation and link to /settings, both of which lie - // about what model is actually running. - // - // On the home screen ``conversation`` is undefined, so we also have to - // consult ``settings.agent_settings.agent_kind`` — that's the kind the - // next-created conversation will inherit. Without the fallback, ACP - // users would still see the LLM-profile control on the home page, - // contradicting the ACP nav gating elsewhere. - const isAcpActive = - conversation?.agent_kind === "acp" || - (!conversation && settings?.agent_settings?.agent_kind === "acp"); - const llmModel = isAcpActive - ? null - : (conversation?.llm_model ?? settings?.llm_model); + // ACP conversations do not use the OpenHands LLM profile. Resolve the model + // label through the shared helper so the displayed value matches what the + // conversation-creation path will actually send to the agent-server (the + // helper applies provider defaults + filters out the SDK ``"default"`` + // placeholders + the ``"acp-managed"`` sentinel). + const isActiveAcpConversation = conversation?.agent_kind === "acp"; + const isHomeAcp = + !conversation && settings?.agent_settings?.agent_kind === "acp"; + const acpProvider = isHomeAcp + ? ACP_PROVIDERS.find( + ({ key }) => key === settings?.agent_settings?.acp_server, + ) + : undefined; + let llmModel: string | null | undefined; + if (isActiveAcpConversation) { + llmModel = conversation?.llm_model; + } else if (isHomeAcp) { + llmModel = resolveEffectiveAcpModel({ + configured: + typeof settings?.agent_settings?.acp_model === "string" + ? settings.agent_settings.acp_model + : null, + providerDefault: acpProvider?.default_model, + }); + } else { + llmModel = conversation?.llm_model ?? settings?.llm_model; + } + const destinationPath = + isActiveAcpConversation || isHomeAcp ? "/settings/agent" : "/settings"; const llmDestinationLabel = t( - backend.kind === "cloud" - ? I18nKey.SETTINGS$LLM_SETTINGS - : I18nKey.SETTINGS$LLM_PROFILES, + isActiveAcpConversation || isHomeAcp + ? I18nKey.SETTINGS$NAV_AGENT + : backend.kind === "cloud" + ? I18nKey.SETTINGS$LLM_SETTINGS + : I18nKey.SETTINGS$LLM_PROFILES, ); const [isPopoverOpen, setIsPopoverOpen] = React.useState(false); @@ -105,7 +122,7 @@ export function ChatInputModel() {
  • setIsPopoverOpen(false)} className="flex h-[30px] items-center gap-2 rounded p-2 leading-5 text-white hover:bg-[var(--oh-interactive-hover)] transition-colors" > diff --git a/src/components/features/chat/switch-profile-button.tsx b/src/components/features/chat/switch-profile-button.tsx index c2f07b68..a113dc60 100644 --- a/src/components/features/chat/switch-profile-button.tsx +++ b/src/components/features/chat/switch-profile-button.tsx @@ -34,9 +34,8 @@ export function SwitchProfileButton() { // controlled by ``acp_model`` (set in Settings → Agent), not by the LLM // profile picker. Surfacing the switcher here would let the user "change // the model" while the running subprocess silently keeps its own — a - // confusing no-op. Hide the button instead. ``toAppConversation`` also - // nulls ``llm_model`` on this boundary so any other consumer that reads - // the model directly sees "no model" rather than a misleading value. + // confusing no-op. Hide the button even when ``llm_model`` carries an ACP + // display model for chips/headers. // // On the home screen ``conversation`` is undefined; fall back to // ``settings.agent_settings.agent_kind`` so the picker also hides when diff --git a/src/components/features/conversation-panel/compact-conversation-row.tsx b/src/components/features/conversation-panel/compact-conversation-row.tsx index 1875053f..ad32aa31 100644 --- a/src/components/features/conversation-panel/compact-conversation-row.tsx +++ b/src/components/features/conversation-panel/compact-conversation-row.tsx @@ -21,7 +21,6 @@ interface CompactConversationRowProps { onClose?: () => void; showRepositoryMetadata?: boolean; llmModel?: string | null; - showLlmProfiles?: boolean; agentKind?: "openhands" | "acp" | null; acpServer?: string | null; } @@ -44,7 +43,6 @@ export function CompactConversationRow({ onClose, showRepositoryMetadata = true, llmModel = null, - showLlmProfiles = false, agentKind = null, acpServer = null, }: CompactConversationRowProps) { @@ -70,7 +68,6 @@ export function CompactConversationRow({ workspaceWorkingDir={workspaceWorkingDir} showRepositoryMetadata={showRepositoryMetadata} llmModel={llmModel} - showLlmModel={showLlmProfiles} agentKind={agentKind} acpServer={acpServer} /> diff --git a/src/components/features/conversation-panel/conversation-card/conversation-card-footer.tsx b/src/components/features/conversation-panel/conversation-card/conversation-card-footer.tsx index 8ab26ab3..4d101a97 100644 --- a/src/components/features/conversation-panel/conversation-card/conversation-card-footer.tsx +++ b/src/components/features/conversation-panel/conversation-card/conversation-card-footer.tsx @@ -5,7 +5,14 @@ import { I18nKey } from "#/i18n/declaration"; import { RepositorySelection } from "#/api/open-hands.types"; import { ExecutionStatus } from "#/types/agent-server/core/base/common"; import { isExecutionPaused } from "#/utils/status"; -import { getAcpProviderDisplayName } from "#/constants/acp-providers"; +import { + getAcpProviderDisplayName, + resolveAcpProviderIcon, +} from "#/constants/acp-providers"; +import { + AgentBrandIcon, + type AgentBrandIconKind, +} from "#/components/shared/agent-brand-icon"; import { ConversationRepoLink } from "./conversation-repo-link"; import { NoRepository } from "./no-repository"; @@ -18,13 +25,11 @@ interface ConversationCardFooterProps { showRepositoryMetadata?: boolean; showTimestamp?: boolean; llmModel?: string | null; - showLlmModel?: boolean; /** - * High-level kind of the conversation's agent. The ACP-agent chip is - * only rendered when this is ``"acp"``. The OpenHands rendering path - * is intentionally untouched — for OpenHands conversations the chip is - * suppressed regardless of any ``acpServer`` value (defensive against - * stray wire tags on non-ACP conversations). + * High-level kind of the conversation's agent. Drives the chip's icon: + * the OpenHands logo for native conversations and the resolved ACP brand + * mark for ACP conversations. Defensive against stray ``acpServer`` + * values reaching an OpenHands card. */ agentKind?: "openhands" | "acp" | null; /** @@ -32,9 +37,7 @@ interface ConversationCardFooterProps { * ``"gemini-cli"`` / unknown / null). Resolved to a human display name * via {@link getAcpProviderDisplayName}; unknown / null falls back to * a generic "ACP" label so a Custom-command preset still produces a - * useful chip. Always shown for ACP conversations — this is identity - * info, not gated by the ``showLlmModel`` preference (which is about - * LLM model strings, an orthogonal concern). + * useful chip. */ acpServer?: string | null; } @@ -48,7 +51,6 @@ export function ConversationCardFooter({ showRepositoryMetadata = true, showTimestamp = true, llmModel, - showLlmModel = false, agentKind = null, acpServer = null, }: ConversationCardFooterProps) { @@ -56,11 +58,30 @@ export function ConversationCardFooter({ const isPaused = isExecutionPaused(executionStatus); - const acpDisplayName = - agentKind === "acp" - ? (getAcpProviderDisplayName(acpServer) ?? - t(I18nKey.CONVERSATION$ACP_AGENT_GENERIC)) - : null; + // Single inline chip per conversation: [brand mark] {model text}. Always + // shown when there's something meaningful to render — OpenHands shows the + // logo + ``agent.llm.model``; ACP shows the provider brand mark + model + // resolved through PR 730's adapter chain, falling back to the provider + // display name when no model is available so the chip never collapses to + // icon-only. + let chip: { + kind: AgentBrandIconKind; + text: string; + tooltip: string; + } | null = null; + if (agentKind === "acp") { + const providerName = + getAcpProviderDisplayName(acpServer) ?? + t(I18nKey.CONVERSATION$ACP_AGENT_GENERIC); + const text = llmModel ?? providerName; + chip = { + kind: resolveAcpProviderIcon(acpServer), + text, + tooltip: llmModel ? `${providerName} · ${llmModel}` : providerName, + }; + } else if (llmModel) { + chip = { kind: "openhands", text: llmModel, tooltip: llmModel }; + } return (
    - {acpDisplayName ? ( + {chip ? (
    - {acpDisplayName} + + {chip.text}
    ) : null} - {showLlmModel && llmModel ? ( - - {llmModel} - - ) : null}
    diff --git a/src/components/features/conversation-panel/conversation-panel.tsx b/src/components/features/conversation-panel/conversation-panel.tsx index 4c9c29fa..8f7f756a 100644 --- a/src/components/features/conversation-panel/conversation-panel.tsx +++ b/src/components/features/conversation-panel/conversation-panel.tsx @@ -408,7 +408,6 @@ export function ConversationPanel({ onClose={onClose} showRepositoryMetadata={showRepoBranchMetadata} llmModel={conversation.llm_model} - showLlmProfiles={showLlmProfiles} agentKind={conversation.agent_kind} acpServer={conversation.acp_server} /> @@ -451,7 +450,6 @@ export function ConversationPanel({ } showRepositoryMetadata={showRepoBranchMetadata} llmModel={conversation.llm_model} - showLlmProfiles={showLlmProfiles} agentKind={conversation.agent_kind} acpServer={conversation.acp_server} /> @@ -467,7 +465,6 @@ export function ConversationPanel({ onClose, openContextMenuId, showRepoBranchMetadata, - showLlmProfiles, ], ); diff --git a/src/components/features/onboarding/steps/choose-agent-step.tsx b/src/components/features/onboarding/steps/choose-agent-step.tsx index 313dcb58..4f9773d5 100644 --- a/src/components/features/onboarding/steps/choose-agent-step.tsx +++ b/src/components/features/onboarding/steps/choose-agent-step.tsx @@ -12,8 +12,8 @@ import { ACP_PROVIDER_FALLBACK_ICON, ACP_PROVIDERS, buildAcpAgentSettingsDiff, - type ACPProviderIcon, } from "#/constants/acp-providers"; +import { type AgentBrandIconKind } from "#/components/shared/agent-brand-icon"; import { displayErrorToast, displaySuccessToast, @@ -35,7 +35,7 @@ const CODEX_MARK_PATH = const GEMINI_MARK_PATH = "M12 0C12.904 6.056 17.944 11.096 24 12C17.944 12.904 12.904 17.944 12 24C11.096 17.944 6.056 12.904 0 12C6.056 11.096 11.096 6.056 12 0Z"; -function getAgentOptionIcon(id: string): ACPProviderIcon { +function getAgentOptionIcon(id: string): AgentBrandIconKind { if (id === "openhands") return "openhands"; return ( diff --git a/src/components/shared/agent-brand-icon.tsx b/src/components/shared/agent-brand-icon.tsx new file mode 100644 index 00000000..204dde00 --- /dev/null +++ b/src/components/shared/agent-brand-icon.tsx @@ -0,0 +1,108 @@ +import OpenHandsLogo from "#/assets/branding/openhands-logo.svg?react"; +import TerminalIcon from "#/icons/terminal.svg?react"; +import { + CLAUDE_CODE_MARK_PATH, + CLAUDE_CODE_VIEWBOX, + CODEX_MARK_PATH, + CODEX_VIEWBOX, + GEMINI_MARK_PATH, + GEMINI_VIEWBOX, +} from "#/constants/acp-brand-marks"; +import type { ACPProviderIcon } from "#/constants/acp-providers"; +import { cn } from "#/utils/utils"; + +/** + * Icons the conversation chip + onboarding tiles can render. Strictly broader + * than {@link ACPProviderIcon} — that type covers ACP CLI subprocesses only + * (Claude Code, Codex, Gemini, generic terminal fallback), whereas this type + * additionally includes the native OpenHands harness. + */ +export type AgentBrandIconKind = "openhands" | ACPProviderIcon; + +interface AgentBrandIconProps { + kind: AgentBrandIconKind; + size?: number; + className?: string; + "data-testid"?: string; +} + +export function AgentBrandIcon({ + kind, + size = 12, + className, + "data-testid": testId, +}: AgentBrandIconProps) { + if (kind === "openhands") { + // The shipped SVG paints every path ``fill="white"``. Override with + // ``fill-current`` on the descendant ```` nodes so the logo + // inherits the chip's text color. + return ( + + ); + } + if (kind === "claude-code") { + return ( + + + + ); + } + if (kind === "codex") { + return ( + + + + ); + } + if (kind === "gemini") { + return ( + + + + ); + } + return ( + + ); +} diff --git a/src/constants/acp-brand-marks.ts b/src/constants/acp-brand-marks.ts new file mode 100644 index 00000000..ea510d7d --- /dev/null +++ b/src/constants/acp-brand-marks.ts @@ -0,0 +1,18 @@ +// Brand-mark SVG path data for the agent harnesses we recognise on a +// conversation chip. Kept as raw path strings (drawn with ``currentColor`` +// so callers can size & colour them via Tailwind) rather than embedded +// ```` files to avoid an extra asset round-trip and so the icons can +// be reused inline at multiple sizes (onboarding tile, conversation chip, +// settings preview). + +export const CLAUDE_CODE_MARK_PATH = + "m19.6 66.5 19.7-11 .3-1-.3-.5h-1l-3.3-.2-11.2-.3L14 53l-9.5-.5-2.4-.5L0 49l.2-1.5 2-1.3 2.9.2 6.3.5 9.5.6 6.9.4L38 49.1h1.6l.2-.7-.5-.4-.4-.4L29 41l-10.6-7-5.6-4.1-3-2-1.5-2-.6-4.2 2.7-3 3.7.3.9.2 3.7 2.9 8 6.1L37 36l1.5 1.2.6-.4.1-.3-.7-1.1L33 25l-6-10.4-2.7-4.3-.7-2.6c-.3-1-.4-2-.4-3l3-4.2L28 0l4.2.6L33.8 2l2.6 6 4.1 9.3L47 29.9l2 3.8 1 3.4.3 1h.7v-.5l.5-7.2 1-8.7 1-11.2.3-3.2 1.6-3.8 3-2L61 2.6l2 2.9-.3 1.8-1.1 7.7L59 27.1l-1.5 8.2h.9l1-1.1 4.1-5.4 6.9-8.6 3-3.5L77 13l2.3-1.8h4.3l3.1 4.7-1.4 4.9-4.4 5.6-3.7 4.7-5.3 7.1-3.2 5.7.3.4h.7l12-2.6 6.4-1.1 7.6-1.3 3.5 1.6.4 1.6-1.4 3.4-8.2 2-9.6 2-14.3 3.3-.2.1.2.3 6.4.6 2.8.2h6.8l12.6 1 3.3 2 1.9 2.7-.3 2-5.1 2.6-6.8-1.6-16-3.8-5.4-1.3h-.8v.4l4.6 4.5 8.3 7.5L89 80.1l.5 2.4-1.3 2-1.4-.2-9.2-7-3.6-3-8-6.8h-.5v.7l1.8 2.7 9.8 14.7.5 4.5-.7 1.4-2.6 1-2.7-.6-5.8-8-6-9-4.7-8.2-.5.4-2.9 30.2-1.3 1.5-3 1.2-2.5-2-1.4-3 1.4-6.2 1.6-8 1.3-6.4 1.2-7.9.7-2.6v-.2H49L43 72l-9 12.3-7.2 7.6-1.7.7-3-1.5.3-2.8L24 86l10-12.8 6-7.9 4-4.6-.1-.5h-.3L17.2 77.4l-4.7.6-2-2 .2-3 1-1 8-5.5Z"; +export const CLAUDE_CODE_VIEWBOX = "0 0 100 100"; + +export const CODEX_MARK_PATH = + "M4.04286 0.228393C4.52451 0.0304495 5.0488 -0.0409817 5.56586 0.0208928C6.23236 0.0973928 6.82636 0.380893 7.34786 0.870893C7.35488 0.877545 7.36344 0.882351 7.37278 0.884881C7.38212 0.887412 7.39194 0.887588 7.40136 0.885393C8.10536 0.712393 8.78236 0.773393 9.43186 1.06839L9.46336 1.08339L9.54036 1.12139C10.2189 1.47289 10.7054 2.00639 10.9994 2.72039C11.1384 3.05989 11.2084 3.41439 11.2099 3.78339C11.2197 4.05816 11.1893 4.33288 11.1199 4.59889C11.1164 4.61245 11.1165 4.62665 11.12 4.6402C11.1235 4.65374 11.1303 4.66618 11.1399 4.67639C11.5329 5.0749 11.8063 5.57572 11.9289 6.12189C12.1214 7.07239 11.9239 7.92939 11.3374 8.69189L11.2464 8.80189C10.8579 9.24669 10.3481 9.56836 9.77936 9.72739C9.76694 9.73097 9.75556 9.73747 9.74617 9.74634C9.73677 9.75521 9.72964 9.7662 9.72536 9.77839C9.59786 10.1464 9.46986 10.4604 9.23186 10.7744C8.63236 11.5654 7.75086 12.0054 6.75786 11.9999C5.96636 11.9959 5.26486 11.7064 4.65286 11.1319C4.64358 11.1234 4.63225 11.1174 4.61998 11.1146C4.6077 11.1118 4.59491 11.1123 4.58286 11.1159C4.32386 11.1994 4.06286 11.2114 3.78086 11.2084C3.33033 11.2048 2.88658 11.0984 2.48336 10.8974C2.0613 10.688 1.6939 10.3831 1.41036 10.0069C1.30886 9.87239 1.20836 9.74589 1.13486 9.59639C1.03349 9.39033 0.95066 9.17565 0.887357 8.95489C0.754446 8.45324 0.751521 7.92599 0.878857 7.42289C0.882974 7.41102 0.884341 7.39837 0.882857 7.38589C0.88038 7.37348 0.873877 7.36223 0.864357 7.35389C0.556147 7.04213 0.320543 6.66619 0.174357 6.25289C0.0775698 5.99842 0.0213841 5.73031 0.00785682 5.45839C-0.0163229 5.10033 0.0153902 4.74069 0.101857 4.39239C0.326857 3.65039 0.756357 3.06839 1.39036 2.64589C1.53136 2.55189 1.66536 2.47889 1.79136 2.42689C1.93436 2.36689 2.07786 2.31689 2.22186 2.27489C2.23216 2.27184 2.24153 2.26626 2.24913 2.25866C2.25672 2.25107 2.2623 2.24169 2.26536 2.23139C2.37455 1.83888 2.56235 1.47264 2.81736 1.15489C3.15736 0.731893 3.56586 0.422893 4.04286 0.228393ZM3.64086 4.15339C3.58503 4.05573 3.49269 3.98424 3.38415 3.95465C3.27562 3.92507 3.15977 3.93981 3.06211 3.99564C2.96444 4.05147 2.89295 4.14381 2.86337 4.25235C2.83378 4.36088 2.84853 4.47673 2.90436 4.57439L3.75136 6.05689L2.90736 7.48089C2.85561 7.57738 2.84315 7.69014 2.87257 7.7956C2.902 7.90106 2.97104 7.99108 3.06526 8.04684C3.15949 8.1026 3.27162 8.1198 3.37823 8.09484C3.48484 8.06988 3.57768 8.00469 3.63736 7.91289L4.60736 6.27689C4.64561 6.21237 4.66609 6.13887 4.66671 6.06386C4.66732 5.98886 4.64805 5.91503 4.61086 5.84989L3.64086 4.15339ZM6.36386 7.27339C6.25583 7.27982 6.15434 7.32727 6.08012 7.40603C6.00591 7.48479 5.96458 7.58892 5.96458 7.69714C5.96458 7.80536 6.00591 7.90949 6.08012 7.98826C6.15434 8.06702 6.25583 8.11446 6.36386 8.12089H8.78786C8.89675 8.1156 8.99943 8.06862 9.07462 7.98969C9.14982 7.91075 9.19176 7.80591 9.19176 7.69689C9.19176 7.58787 9.14982 7.48303 9.07462 7.4041C8.99943 7.32516 8.89675 7.27818 8.78786 7.27289H6.36386V7.27339Z"; +export const CODEX_VIEWBOX = "0 0 12 12"; + +export const GEMINI_MARK_PATH = + "M12 0C12.904 6.056 17.944 11.096 24 12C17.944 12.904 12.904 17.944 12 24C11.096 17.944 6.056 12.904 0 12C6.056 11.096 11.096 6.056 12 0Z"; +export const GEMINI_VIEWBOX = "0 0 24 24"; diff --git a/src/constants/acp-providers.ts b/src/constants/acp-providers.ts index aa88a54f..f090069c 100644 --- a/src/constants/acp-providers.ts +++ b/src/constants/acp-providers.ts @@ -1,7 +1,6 @@ import { I18nKey } from "#/i18n/declaration"; export type ACPProviderIcon = - | "openhands" | "claude-code" | "codex" | "gemini" @@ -9,6 +8,64 @@ export type ACPProviderIcon = export const ACP_PROVIDER_FALLBACK_ICON: ACPProviderIcon = "cli-generic"; +// SDK placeholder strings the ACP wrapper returns before the user has +// chosen a real model — surfacing either would lie about what's running. +export const ACP_DEFAULT_PLACEHOLDERS = new Set([ + "default", + "default (recommended)", +]); + +// Sentinel ``agent.llm.model`` returned by older SDKs for ACP conversations +// in lieu of a real model. Suppressed at every consumer that resolves a +// display string. +export const ACP_MANAGED_SENTINEL = "acp-managed"; + +/** + * Filter for "real" ACP model strings — non-empty, not the SDK's "default" + * placeholder, not the legacy ``acp-managed`` sentinel. Returns the trimmed + * value on success, ``null`` otherwise. + */ +function realAcpModel(value: unknown): string | null { + if (typeof value !== "string") return null; + const trimmed = value.trim(); + if (!trimmed) return null; + if (ACP_DEFAULT_PLACEHOLDERS.has(trimmed.toLowerCase())) return null; + if (trimmed === ACP_MANAGED_SENTINEL) return null; + return trimmed; +} + +/** + * Single source of truth for resolving the model string to surface for an + * ACP conversation/settings context. Consumed by the conversation adapter + * (chip text), the conversation-creation path (concrete ``acp_model`` + * payload), the Settings → Agent form (initial value), and the chat-input + * model label. + * + * Precedence: SDK runtime fields → user-configured ``acp_model`` → + * legacy ``agent.llm.model`` → provider default (when ``providerDefault`` + * is passed). Pass ``providerDefault`` only on surfaces that should + * silently substitute the registry default; omit it for the conversation + * chip, which must distinguish "no concrete model" from "default". + */ +export function resolveEffectiveAcpModel(inputs: { + runtimeName?: string | null; + runtimeId?: string | null; + configured?: string | null; + sdkLlm?: string | null; + providerDefault?: string | null; +}): string | null { + for (const candidate of [ + inputs.runtimeName, + inputs.runtimeId, + inputs.configured, + inputs.sdkLlm, + ]) { + const value = realAcpModel(candidate); + if (value) return value; + } + return inputs.providerDefault ?? null; +} + /** * Built-in ACP (Agent Client Protocol) provider registry. * @@ -43,6 +100,14 @@ export interface ACPProviderConfig { * ``@zed-industries/codex-acp`` (the Zed-shipped wrapper) instead. */ default_command: string[]; + /** + * Canvas-local suggested ACP model IDs. These mirror the current runtime + * picker values for the built-in harnesses, but are not authoritative access + * checks; users can still enter a custom override in Settings -> Agent. + */ + available_models?: ACPModelOption[]; + /** Model ID preselected for built-in providers so Canvas never saves blank. */ + default_model?: string; /** * i18n key for the one-line provider description rendered under the * onboarding tile. Stored on the registry so adding a new ACP @@ -58,6 +123,83 @@ export interface ACPProviderConfig { icon?: ACPProviderIcon; } +export interface ACPModelOption { + /** Exact model ID sent as ``acp_model``. */ + id: string; + /** Human-readable label shown in Settings -> Agent. */ + label: string; +} + +// Canonical model IDs the Claude Code CLI binary's model registry recognises +// (verified by string-scanning v2.1.146 of the bundled ``claude`` native +// binary). ``[1m]`` is the SDK-documented 1M-context suffix; we use the +// version-agnostic alias so the option auto-tracks the newest 1M-capable +// model. ``opusplan`` routes planning to Opus and execution to Sonnet. +// Availability for any of these ultimately depends on the user's Anthropic +// plan tier — surfacing them here matches what the CLI *accepts*, not what +// every account can actually invoke. +const CLAUDE_MODELS: ACPModelOption[] = [ + { id: "claude-opus-4-7", label: "Claude Opus 4.7" }, + { id: "claude-opus-4-6", label: "Claude Opus 4.6" }, + { id: "opus[1m]", label: "Claude Opus 4.6 (1M)" }, + { id: "claude-opus-4-5", label: "Claude Opus 4.5" }, + { id: "claude-opus-4-1-20250805", label: "Claude Opus 4.1" }, + { id: "claude-sonnet-4-6", label: "Claude Sonnet 4.6" }, + { id: "sonnet[1m]", label: "Claude Sonnet 4.6 (1M)" }, + { id: "claude-sonnet-4-5", label: "Claude Sonnet 4.5" }, + { id: "claude-haiku-4-5", label: "Claude Haiku 4.5" }, + { id: "opusplan", label: "Opus (plan) + Sonnet (execute)" }, +]; + +// Model IDs accepted by the ``@zed-industries/codex-acp`` wrapper, mirroring +// the Codex CLI's own ``/model`` picker. Format is ``/`` +// where the trailing tier (``low``/``medium``/``high``/``xhigh``) hints the +// reasoning effort for that turn. Sourced from the Codex CLI's documented +// runtime options as of 2026-05-22 — see ``acp_model`` registry tracker +// in agent-canvas#740 for the long-term plan. +const CODEX_MODELS: ACPModelOption[] = [ + { id: "gpt-5.5/low", label: "GPT-5.5 (low)" }, + { id: "gpt-5.5/medium", label: "GPT-5.5 (medium)" }, + { id: "gpt-5.5/high", label: "GPT-5.5 (high)" }, + { id: "gpt-5.5/xhigh", label: "GPT-5.5 (xhigh)" }, + { id: "gpt-5.4/low", label: "GPT-5.4 (low)" }, + { id: "gpt-5.4/medium", label: "GPT-5.4 (medium)" }, + { id: "gpt-5.4/high", label: "GPT-5.4 (high)" }, + { id: "gpt-5.4/xhigh", label: "GPT-5.4 (xhigh)" }, + { id: "gpt-5.4-mini/low", label: "GPT-5.4 Mini (low)" }, + { id: "gpt-5.4-mini/medium", label: "GPT-5.4 Mini (medium)" }, + { id: "gpt-5.4-mini/high", label: "GPT-5.4 Mini (high)" }, + { id: "gpt-5.4-mini/xhigh", label: "GPT-5.4 Mini (xhigh)" }, + { id: "gpt-5.3-codex/low", label: "GPT-5.3 Codex (low)" }, + { id: "gpt-5.3-codex/medium", label: "GPT-5.3 Codex (medium)" }, + { id: "gpt-5.3-codex/high", label: "GPT-5.3 Codex (high)" }, + { id: "gpt-5.3-codex/xhigh", label: "GPT-5.3 Codex (xhigh)" }, + { id: "gpt-5.2/low", label: "GPT-5.2 (low)" }, + { id: "gpt-5.2/medium", label: "GPT-5.2 (medium)" }, + { id: "gpt-5.2/high", label: "GPT-5.2 (high)" }, + { id: "gpt-5.2/xhigh", label: "GPT-5.2 (xhigh)" }, +]; + +// Model IDs accepted by ``@google/gemini-cli --acp``. The ``auto-gemini-*`` +// entries delegate version selection to the CLI's router; the explicit +// ``gemini-3.1-*`` / ``gemini-2.5-*`` entries pin to a specific snapshot. +// Sourced from the Gemini CLI's documented model list as of 2026-05-22 — +// see agent-canvas#740 for the long-term plan to move this registry +// upstream. +const GEMINI_MODELS: ACPModelOption[] = [ + { id: "auto-gemini-3", label: "Auto (Gemini 3)" }, + { id: "auto-gemini-2.5", label: "Auto (Gemini 2.5)" }, + { id: "gemini-3.1-pro-preview", label: "Gemini 3.1 Pro (preview)" }, + { id: "gemini-3-flash-preview", label: "Gemini 3 Flash (preview)" }, + { + id: "gemini-3.1-flash-lite-preview", + label: "Gemini 3.1 Flash Lite (preview)", + }, + { id: "gemini-2.5-pro", label: "Gemini 2.5 Pro" }, + { id: "gemini-2.5-flash", label: "Gemini 2.5 Flash" }, + { id: "gemini-2.5-flash-lite", label: "Gemini 2.5 Flash Lite" }, +]; + // Each entry's ``default_command`` is the published-package npx // invocation that speaks the ACP JSON-RPC protocol on stdio. Verified // against the upstream npm registry on the date noted below — if a @@ -71,6 +213,8 @@ export const ACP_PROVIDERS: ACPProviderConfig[] = [ // Verified 2026-05-19. Official Anthropic-maintained ACP wrapper // around the Claude Code CLI. default_command: ["npx", "-y", "@agentclientprotocol/claude-agent-acp"], + available_models: CLAUDE_MODELS, + default_model: "claude-opus-4-7", description_key: I18nKey.ONBOARDING$AGENT_CLAUDE_CODE_DESCRIPTION, icon: "claude-code", }, @@ -82,6 +226,8 @@ export const ACP_PROVIDERS: ACPProviderConfig[] = [ // OpenAI Codex CLI — NOT ``@openai/codex acp`` (no ``acp`` // subcommand on that package). default_command: ["npx", "-y", "@zed-industries/codex-acp"], + available_models: CODEX_MODELS, + default_model: "gpt-5.5/medium", description_key: I18nKey.ONBOARDING$AGENT_CODEX_DESCRIPTION, icon: "codex", }, @@ -92,6 +238,8 @@ export const ACP_PROVIDERS: ACPProviderConfig[] = [ // Verified 2026-05-19. Official Google CLI; ``--acp`` switches it // into ACP server mode on stdio. default_command: ["npx", "-y", "@google/gemini-cli", "--acp"], + available_models: GEMINI_MODELS, + default_model: "gemini-2.5-pro", description_key: I18nKey.ONBOARDING$AGENT_GEMINI_CLI_DESCRIPTION, icon: "gemini", }, @@ -123,6 +271,23 @@ export function getAcpProviderDisplayName( return found ? found.display_name : null; } +/** + * Resolve an ACP provider registry key to the icon discriminator the + * conversation chip should render alongside the model text. + * + * Falls back to {@link ACP_PROVIDER_FALLBACK_ICON} for ``"custom"``, + * unknown keys, or a missing key — the chip then shows a neutral + * terminal glyph that still communicates "this is an ACP conversation" + * without claiming a brand identity we don't know. + */ +export function resolveAcpProviderIcon( + key: string | null | undefined, +): ACPProviderIcon { + if (!key) return ACP_PROVIDER_FALLBACK_ICON; + const found = ACP_PROVIDERS.find((p) => p.key === key); + return found?.icon ?? ACP_PROVIDER_FALLBACK_ICON; +} + /** * Build the ``agent_settings_diff`` payload PATCH /api/settings expects * for the agent-kind/provider choice the user just made. @@ -170,6 +335,11 @@ export function buildAcpAgentSettingsDiff( return null; } + const model = + options.model === undefined + ? (provider?.default_model ?? null) + : options.model; + // ``acp_args: []`` resets any API-set ``acp_args`` that would // otherwise survive and concatenate to ``acp_command`` at spawn time // (the agent-server merges the two before exec). Callers building the @@ -181,6 +351,6 @@ export function buildAcpAgentSettingsDiff( acp_server: providerKey, acp_command: options.command ?? [], acp_args: [], - acp_model: options.model ?? null, + acp_model: model ?? null, }; } diff --git a/src/i18n/translation.json b/src/i18n/translation.json index 15b28c2e..8cde8d7f 100644 --- a/src/i18n/translation.json +++ b/src/i18n/translation.json @@ -5133,22 +5133,39 @@ "uk": "Model", "ca": "Model" }, + "SETTINGS$AGENT_CUSTOM_MODEL": { + "en": "Custom model", + "ja": "カスタムモデル", + "zh-CN": "自定义模型", + "zh-TW": "自訂模型", + "ko-KR": "사용자 정의 모델", + "no": "Egendefinert modell", + "it": "Modello personalizzato", + "pt": "Modelo personalizado", + "es": "Modelo personalizado", + "ar": "نموذج مخصص", + "fr": "Modèle personnalisé", + "tr": "Özel model", + "de": "Benutzerdefiniertes Modell", + "uk": "Користувацька модель", + "ca": "Model personalitzat" + }, "SETTINGS$AGENT_MODEL_HINT": { - "en": "Optional model override forwarded to the ACP subprocess. Leave blank to use the provider's default.", - "ja": "Optional model override forwarded to the ACP subprocess. Leave blank to use the provider's default.", - "zh-CN": "Optional model override forwarded to the ACP subprocess. Leave blank to use the provider's default.", - "zh-TW": "Optional model override forwarded to the ACP subprocess. Leave blank to use the provider's default.", - "ko-KR": "Optional model override forwarded to the ACP subprocess. Leave blank to use the provider's default.", - "no": "Optional model override forwarded to the ACP subprocess. Leave blank to use the provider's default.", - "it": "Optional model override forwarded to the ACP subprocess. Leave blank to use the provider's default.", - "pt": "Optional model override forwarded to the ACP subprocess. Leave blank to use the provider's default.", - "es": "Optional model override forwarded to the ACP subprocess. Leave blank to use the provider's default.", - "ar": "Optional model override forwarded to the ACP subprocess. Leave blank to use the provider's default.", - "fr": "Optional model override forwarded to the ACP subprocess. Leave blank to use the provider's default.", - "tr": "Optional model override forwarded to the ACP subprocess. Leave blank to use the provider's default.", - "de": "Optional model override forwarded to the ACP subprocess. Leave blank to use the provider's default.", - "uk": "Optional model override forwarded to the ACP subprocess. Leave blank to use the provider's default.", - "ca": "Optional model override forwarded to the ACP subprocess. Leave blank to use the provider's default." + "en": "Choose a suggested ACP model or enter a custom model override. Built-in providers save a concrete model instead of leaving the model blank.", + "ja": "推奨される ACP モデルを選択するか、カスタムモデルの上書きを入力してください。組み込みプロバイダーはモデルを空白のままにせず、具体的なモデルを保存します。", + "zh-CN": "选择一个建议的 ACP 模型,或输入自定义模型覆盖。内置提供商会保存一个具体的模型,而不是将模型留空。", + "zh-TW": "選擇一個建議的 ACP 模型,或輸入自訂模型覆寫。內建供應商會儲存具體的模型,而不是將模型留空。", + "ko-KR": "추천 ACP 모델을 선택하거나 사용자 정의 모델 재정의를 입력하세요. 기본 제공 공급자는 모델을 비워두지 않고 구체적인 모델을 저장합니다.", + "no": "Velg en foreslått ACP-modell eller skriv inn en egendefinert modelloverstyring. Innebygde leverandører lagrer en konkret modell i stedet for å la modellen være tom.", + "it": "Scegli un modello ACP suggerito o inserisci una sostituzione personalizzata. I provider integrati salvano un modello concreto invece di lasciarlo vuoto.", + "pt": "Escolha um modelo ACP sugerido ou insira uma substituição de modelo personalizada. Os provedores integrados salvam um modelo concreto em vez de deixá-lo em branco.", + "es": "Elige un modelo ACP sugerido o introduce una sustitución de modelo personalizada. Los proveedores integrados guardan un modelo concreto en lugar de dejarlo en blanco.", + "ar": "اختر نموذج ACP مقترحًا أو أدخل تجاوزًا لنموذج مخصص. تقوم المزودات المدمجة بحفظ نموذج محدد بدلًا من ترك النموذج فارغًا.", + "fr": "Choisissez un modèle ACP suggéré ou saisissez un remplacement de modèle personnalisé. Les fournisseurs intégrés enregistrent un modèle concret plutôt que de laisser le champ vide.", + "tr": "Önerilen bir ACP modelini seçin veya özel bir model geçersiz kılma değeri girin. Yerleşik sağlayıcılar, modeli boş bırakmak yerine somut bir model kaydeder.", + "de": "Wähle ein vorgeschlagenes ACP-Modell aus oder gib eine benutzerdefinierte Modellüberschreibung ein. Integrierte Anbieter speichern ein konkretes Modell, anstatt das Modellfeld leer zu lassen.", + "uk": "Виберіть рекомендовану модель ACP або введіть користувацьке перевизначення моделі. Вбудовані постачальники зберігають конкретну модель замість того, щоб залишати поле порожнім.", + "ca": "Tria un model ACP suggerit o introdueix una substitució de model personalitzada. Els proveïdors integrats desen un model concret en lloc de deixar-lo en blanc." }, "SETTINGS$AGENT_DISABLED_TOOLTIP": { "en": "Disabled while {{agentName}} is the active agent", diff --git a/src/routes/agent-settings.tsx b/src/routes/agent-settings.tsx index bf0f2a05..75cd757b 100644 --- a/src/routes/agent-settings.tsx +++ b/src/routes/agent-settings.tsx @@ -34,6 +34,7 @@ type AgentType = "openhands" | "acp"; const ENABLE_SUB_AGENTS_FIELD_KEY = "enable_sub_agents"; const COMMAND_PLACEHOLDER_FALLBACK = "npx -y "; +const ACP_CUSTOM_MODEL_KEY = "__custom_model__"; function toStringArray(value: unknown): string[] { return Array.isArray(value) @@ -68,6 +69,15 @@ function getEnableSubAgentsValue( return field?.default === true; } +function isKnownAcpModel( + provider: ACPProviderConfig | undefined, + model: string, +): boolean { + return ( + provider?.available_models?.some(({ id }) => id === model.trim()) ?? false + ); +} + function AgentSettingsScreen() { const { t } = useTranslation("openhands"); const { data: settings, isLoading } = useSettings(); @@ -98,6 +108,7 @@ function AgentSettingsScreen() { const [agentType, setAgentType] = useState("openhands"); const [commandText, setCommandText] = useState(""); const [acpModel, setAcpModel] = useState(""); + const [isCustomAcpModel, setIsCustomAcpModel] = useState(false); const [isDirty, setIsDirty] = useState(false); const lastInitializedSettingsRef = useRef(null); @@ -134,13 +145,20 @@ function AgentSettingsScreen() { loadedCommandTextRef.current = renderedCommandText; const savedModel = settings.agent_settings?.acp_model; - setAcpModel(typeof savedModel === "string" ? savedModel : ""); + const normalizedSavedModel = + typeof savedModel === "string" ? savedModel.trim() : ""; + setAcpModel(normalizedSavedModel || provider?.default_model || ""); + setIsCustomAcpModel( + !!normalizedSavedModel && + (!provider || !isKnownAcpModel(provider, normalizedSavedModel)), + ); } else { setAgentType("openhands"); setCommandText(""); setAcpModel(""); loadedAcpServerRef.current = null; loadedCommandTextRef.current = ""; + setIsCustomAcpModel(false); } setIsDirty(false); }, [settings]); @@ -159,6 +177,13 @@ function AgentSettingsScreen() { const selectedProvider = ACP_PROVIDERS.find( ({ key }) => key === selectedPreset, ); + const modelSuggestions = selectedProvider?.available_models ?? []; + const hasModelSuggestions = modelSuggestions.length > 0; + const selectedModelIsSuggestion = isKnownAcpModel(selectedProvider, acpModel); + const selectedModelKey = + isCustomAcpModel || !selectedModelIsSuggestion + ? ACP_CUSTOM_MODEL_KEY + : acpModel; const isDefaultProviderCommand = !!selectedProvider && commandTokens.join(" ") === selectedProvider.default_command.join(" "); @@ -187,9 +212,12 @@ function AgentSettingsScreen() { : selectedProvider && isDefaultProviderCommand ? selectedProvider.key : ACP_CUSTOM_PRESET_KEY; + // ``model: undefined`` lets buildAcpAgentSettingsDiff seed the provider's + // ``default_model`` for built-in keys; for the custom preset it falls + // through to ``null`` since custom has no default. const agentSettingsDiff = buildAcpAgentSettingsDiff(providerKey, { command: useDefault ? [] : commandTokens, - model: acpModel.trim() || null, + model: acpModel.trim() || undefined, allowUnknownServer: preserveUnknownServer, }); @@ -277,7 +305,11 @@ function AgentSettingsScreen() { const preferred = ACP_PROVIDERS[0]; if (preferred) { setCommandText(formatCommand(preferred.default_command)); + setAcpModel(preferred.default_model ?? ""); + setIsCustomAcpModel(false); } + } else if (newType === "openhands") { + setIsCustomAcpModel(false); } setIsDirty(true); }} @@ -325,6 +357,15 @@ function AgentSettingsScreen() { const provider = ACP_PROVIDERS.find(({ key: k }) => k === preset); if (provider) { setCommandText(formatCommand(provider.default_command)); + setAcpModel(provider.default_model ?? ""); + setIsCustomAcpModel(false); + } else if (preset === ACP_CUSTOM_PRESET_KEY) { + // Switching to Custom must clear any built-in default the + // user just left — otherwise saving the custom command + // silently leaks e.g. ``claude-opus-4-7`` into + // ``acp_model`` for an unrelated wrapper. + setAcpModel(""); + setIsCustomAcpModel(true); } setIsDirty(true); }} @@ -350,18 +391,54 @@ function AgentSettingsScreen() {
    - { - setAcpModel(value); - setIsDirty(true); - }} - /> + {hasModelSuggestions && ( + ({ + key: model.id, + label: model.label, + })), + { + key: ACP_CUSTOM_MODEL_KEY, + label: t(I18nKey.SETTINGS$AGENT_PRESET_CUSTOM), + }, + ]} + selectedKey={selectedModelKey} + onSelectionChange={(key) => { + if (!key) return; + const modelKey = String(key); + if (modelKey === ACP_CUSTOM_MODEL_KEY) { + setIsCustomAcpModel(true); + setAcpModel(""); + } else { + setIsCustomAcpModel(false); + setAcpModel(modelKey); + } + setIsDirty(true); + }} + /> + )} + {(!hasModelSuggestions || isCustomAcpModel) && ( + { + setAcpModel(value); + setIsDirty(true); + }} + /> + )} {t(I18nKey.SETTINGS$AGENT_MODEL_HINT)}