diff --git a/__tests__/api/agent-server-adapter.test.ts b/__tests__/api/agent-server-adapter.test.ts index dadff5853..2f6ad67b7 100644 --- a/__tests__/api/agent-server-adapter.test.ts +++ b/__tests__/api/agent-server-adapter.test.ts @@ -58,7 +58,7 @@ beforeEach(() => { }); describe("buildStartConversationRequest", () => { - it("uses nested settings as the source of truth and keeps SDK tool names", () => { + it("uses nested settings as the source of truth and lets the SDK create the agent", () => { const payload = buildStartConversationRequest({ settings: { ...DEFAULT_SETTINGS, @@ -76,6 +76,7 @@ describe("buildStartConversationRequest", () => { enabled: true, max_size: 120, }, + enable_switch_llm_tool: true, }, conversation_settings: { ...DEFAULT_SETTINGS.conversation_settings, @@ -84,32 +85,28 @@ describe("buildStartConversationRequest", () => { }, query: "hello", }) as { - agent: Record & { + agent?: unknown; + agent_settings: Record & { llm: Record; tools: Array<{ name: string; params: Record }>; - include_default_tools: string[]; + agent_context: Record; }; workspace: { working_dir: string }; initial_message: { content: Array<{ text: string }> }; max_iterations: number; }; - expect(payload.agent.llm).toMatchObject({ + expect(payload.agent).toBeUndefined(); + expect(payload.agent_settings.llm).toMatchObject({ model: "nested-model", api_key: "nested-key", base_url: "https://nested.example.com", }); - expect(payload.agent.condenser).toEqual({ - kind: "LLMSummarizingCondenser", - llm: { - model: "nested-model", - api_key: "nested-key", - base_url: "https://nested.example.com", - usage_id: "condenser", - }, + expect(payload.agent_settings.condenser).toEqual({ + enabled: true, max_size: 120, }); - expect(payload.agent.tools).toEqual([ + expect(payload.agent_settings.tools).toEqual([ { name: "terminal", params: {} }, { name: "file_editor", params: {} }, { name: "task_tracker", params: {} }, @@ -117,15 +114,12 @@ describe("buildStartConversationRequest", () => { { name: "browser_tool_set", params: {} }, { name: "task_tool_set", params: {} }, ]); - expect(payload.agent.include_default_tools).toEqual([ - "FinishTool", - "ThinkTool", - ]); - expect(payload.agent.agent_context).toEqual({ + expect(payload.agent_settings.agent_context).toEqual({ load_public_skills: true, load_user_skills: true, }); - expect(payload.agent.agent).toBeUndefined(); + expect(payload.agent_settings.agent).toBe("CodeActAgent"); + expect(payload.agent_settings.enable_switch_llm_tool).toBe(true); expect(payload.workspace.working_dir).toBe( "/workspace/project/agent-canvas", ); @@ -133,7 +127,7 @@ describe("buildStartConversationRequest", () => { expect(payload.initial_message.content[0]?.text).toBe("hello"); }); - it("adds the SDK switch-LLM built-in when the agent-server setting is enabled", () => { + it("forwards the switch-LLM setting to SDK agent settings", () => { const payload = buildStartConversationRequest({ settings: { ...DEFAULT_SETTINGS, @@ -144,18 +138,16 @@ describe("buildStartConversationRequest", () => { }, }, }) as { - agent: { - include_default_tools: string[]; + agent?: unknown; + agent_settings: { enable_switch_llm_tool?: boolean; + include_default_tools?: unknown; }; }; - expect(payload.agent.include_default_tools).toEqual([ - "FinishTool", - "ThinkTool", - "SwitchLLMTool", - ]); - expect(payload.agent.enable_switch_llm_tool).toBeUndefined(); + expect(payload.agent).toBeUndefined(); + expect(payload.agent_settings.enable_switch_llm_tool).toBe(true); + expect(payload.agent_settings.include_default_tools).toBeUndefined(); }); it("omits browser_tool_set and task_tool_set when the server does not advertise them", () => { @@ -170,12 +162,12 @@ describe("buildStartConversationRequest", () => { }, }, }) as { - agent: { + agent_settings: { tools: Array<{ name: string; params: Record }>; }; }; - expect(payload.agent.tools).toEqual([ + expect(payload.agent_settings.tools).toEqual([ { name: "terminal", params: {} }, { name: "file_editor", params: {} }, { name: "task_tracker", params: {} }, @@ -198,12 +190,12 @@ describe("buildStartConversationRequest", () => { }, }, }) as { - agent: { + agent_settings: { tools: Array<{ name: string; params: Record }>; }; }; - expect(payload.agent.tools).toEqual([ + expect(payload.agent_settings.tools).toEqual([ { name: "terminal", params: {} }, { name: "file_editor", params: {} }, { name: "task_tracker", params: {} }, @@ -223,12 +215,12 @@ describe("buildStartConversationRequest", () => { }, }, }) as { - agent: { + agent_settings: { tools: Array<{ name: string; params: Record }>; }; }; - const toolNames = payload.agent.tools.map((t) => t.name); + const toolNames = payload.agent_settings.tools.map((t) => t.name); expect(toolNames).not.toContain("task_tool_set"); }); @@ -382,7 +374,7 @@ describe("buildStartConversationRequest", () => { }); }); - it("mirrors conversation secrets onto agent.agent_context.secrets for ACP", () => { + it("mirrors conversation secrets onto agent_settings.agent_context.secrets for ACP", () => { // Until canvas pins to an agent-server build that includes // software-agent-sdk PR #3299, the bare ``payload.secrets`` channel // only reaches ``secret_registry`` server-side — ``ACPAgent``'s @@ -390,7 +382,7 @@ describe("buildStartConversationRequest", () => { // from the registry, so a Settings → Secrets entry like // ``ANTHROPIC_API_KEY`` would silently fail to land in the ACP // CLI's environment. Mirror the same LookupSecret map onto - // ``agent.agent_context.secrets`` so the existing SDK loop picks + // ``agent_settings.agent_context.secrets`` so the existing SDK loop picks // it up. Mirrors OpenHands' app-server bridging. const payload = buildStartConversationRequest({ settings: { @@ -404,7 +396,7 @@ describe("buildStartConversationRequest", () => { }, customSecrets: [{ name: "ANTHROPIC_API_KEY" }], }) as { - agent: { agent_context?: { secrets?: Record } }; + agent_settings: { agent_context?: { secrets?: Record } }; secrets: Record; }; @@ -412,9 +404,9 @@ describe("buildStartConversationRequest", () => { // channel (for any non-ACP consumer / for SDK #3299 once it lands) // and the agent_context bridge (for current ACPAgent spawns). expect(payload.secrets.ANTHROPIC_API_KEY).toBeDefined(); - expect(payload.agent.agent_context?.secrets?.ANTHROPIC_API_KEY).toEqual( - payload.secrets.ANTHROPIC_API_KEY, - ); + expect( + payload.agent_settings.agent_context?.secrets?.ANTHROPIC_API_KEY, + ).toEqual(payload.secrets.ANTHROPIC_API_KEY); }); it("does not synthesize agent_context.secrets for ACP when no custom secrets are set", () => { @@ -432,10 +424,10 @@ describe("buildStartConversationRequest", () => { }, }, }) as { - agent: { agent_context?: { secrets?: Record } }; + agent_settings: { agent_context?: { secrets?: Record } }; }; - expect(payload.agent.agent_context?.secrets).toBeUndefined(); + expect(payload.agent_settings.agent_context?.secrets).toBeUndefined(); }); it("does not mirror conversation secrets onto agent_context for non-ACP conversations", () => { @@ -448,10 +440,10 @@ describe("buildStartConversationRequest", () => { settings: DEFAULT_SETTINGS, customSecrets: [{ name: "ANTHROPIC_API_KEY" }], }) as { - agent: { agent_context?: { secrets?: Record } }; + agent_settings: { agent_context?: { secrets?: Record } }; }; - expect(payload.agent.agent_context?.secrets).toBeUndefined(); + expect(payload.agent_settings.agent_context?.secrets).toBeUndefined(); }); describe("canvas_ui tool injection", () => { @@ -647,8 +639,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 +651,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 @@ -753,7 +742,7 @@ describe("buildRuntimeServicesSystemSuffix", () => { }); }); -describe("createAgentFromSettings runtime services suffix", () => { +describe("agent_settings runtime services suffix", () => { afterEach(() => { vi.unstubAllEnvs(); }); @@ -763,9 +752,9 @@ describe("createAgentFromSettings runtime services suffix", () => { settings: DEFAULT_SETTINGS, query: "hello", }) as { - agent: { agent_context: Record }; + agent_settings: { agent_context: Record }; }; - expect(payload.agent.agent_context).toEqual({ + expect(payload.agent_settings.agent_context).toEqual({ load_public_skills: true, load_user_skills: true, }); @@ -788,20 +777,20 @@ describe("createAgentFromSettings runtime services suffix", () => { settings: DEFAULT_SETTINGS, query: "hello", }) as { - agent: { agent_context: Record }; + agent_settings: { agent_context: Record }; }; - expect(payload.agent.agent_context).toMatchObject({ + expect(payload.agent_settings.agent_context).toMatchObject({ load_public_skills: true, load_user_skills: true, }); expect( - payload.agent.agent_context.system_message_suffix as string, + payload.agent_settings.agent_context.system_message_suffix as string, ).toContain(""); }); }); describe("buildStartConversationRequest — ACP discriminator", () => { - it("builds an ACPAgent payload when agent_kind is 'acp'", () => { + it("builds ACP agent settings when agent_kind is 'acp'", () => { const payload = buildStartConversationRequest({ settings: { ...DEFAULT_SETTINGS, @@ -811,7 +800,7 @@ describe("buildStartConversationRequest — ACP discriminator", () => { acp_server: "claude-code", acp_command: ["npx", "-y", "@agentclientprotocol/claude-agent-acp"], acp_model: "claude-opus-4-5", - // These fields are LLM-only and must NOT leak into the ACP payload. + // These fields are LLM-only and must NOT leak into ACP settings. agent: "CodeActAgent", llm: { model: "gpt-4", api_key: "should-not-appear" }, condenser: { enabled: true, max_size: 240 }, @@ -821,49 +810,35 @@ describe("buildStartConversationRequest — ACP discriminator", () => { }, }, }) as { - agent: Record & { - kind: string; + agent?: unknown; + agent_settings: Record & { acp_command?: string[]; acp_model?: string | null; - llm?: unknown; - condenser?: unknown; - tools?: unknown; agent_context?: unknown; }; tags?: Record; }; - expect(payload.agent.kind).toBe("ACPAgent"); - expect(payload.agent.acp_command).toEqual([ + expect(payload.agent).toBeUndefined(); + expect(payload.agent_settings.agent_kind).toBe("acp"); + expect(payload.agent_settings.acp_command).toEqual([ "npx", "-y", "@agentclientprotocol/claude-agent-acp", ]); - expect(payload.agent.acp_model).toBe("claude-opus-4-5"); - // LLM-only fields must not leak into the ACPAgent payload. - expect(payload.agent.llm).toBeUndefined(); - expect(payload.agent.condenser).toBeUndefined(); - expect(payload.agent.tools).toBeUndefined(); - // ``agent_context`` IS populated on the ACP payload — the SDK marks - // ``skills`` / ``system_message_suffix`` / ``load_*_skills`` as - // ``acp_compatible: true``, and the ACP CLI's system prompt renders - // them via ``ACPAgent._render_suffix``. Without seeding these, an - // ACP user would silently lose the skill catalog and the runtime- - // services awareness an OpenHands-driven conversation gets. - expect(payload.agent.agent_context).toEqual({ + expect(payload.agent_settings.acp_model).toBe("claude-opus-4-5"); + // LLM-only fields must not leak into the ACP settings payload. + expect(payload.agent_settings.llm).toBeUndefined(); + expect(payload.agent_settings.condenser).toBeUndefined(); + expect(payload.agent_settings.tools).toBeUndefined(); + expect(payload.agent_settings.agent_context).toEqual({ load_public_skills: true, load_user_skills: true, }); - // Conversation tags carry the ACP provider key for chip rendering. - // Agent-server validates tag keys against ``^[a-z0-9]+$``, so the - // snake_case ``acp_server`` form would be rejected — we use the - // flattened ``acpserver`` form instead. Asserted via the exported - // constant so a rename surfaces here as a compile error rather - // than a silent schema-mismatch at runtime. expect(payload.tags).toEqual({ [ACP_SERVER_TAG_KEY]: "claude-code" }); }); - it("does not include ACP fields in the OpenHands Agent payload", () => { + it("does not include ACP-only fields in OpenHands agent settings", () => { const payload = buildStartConversationRequest({ settings: { ...DEFAULT_SETTINGS, @@ -876,14 +851,18 @@ describe("buildStartConversationRequest — ACP discriminator", () => { }, }, }) as { - agent: Record & { kind: string }; + agent?: unknown; + agent_settings: Record & { + llm: Record; + }; tags?: Record; }; - expect(payload.agent.kind).toBe("Agent"); - expect(payload.agent.acp_command).toBeUndefined(); - expect(payload.agent.acp_server).toBeUndefined(); - expect(payload.agent.agent_kind).toBeUndefined(); + expect(payload.agent).toBeUndefined(); + expect(payload.agent_settings.agent_kind).toBe("openhands"); + expect(payload.agent_settings.acp_command).toBeUndefined(); + expect(payload.agent_settings.acp_server).toBeUndefined(); + expect(payload.agent_settings.llm.model).toBe("gpt-4"); expect(payload.tags).toBeUndefined(); }); @@ -899,20 +878,13 @@ describe("buildStartConversationRequest — ACP discriminator", () => { acp_model: null, }, }, - }) as { agent: Record }; + }) as { agent_settings: Record }; - expect((payload.agent as { kind: string }).kind).toBe("ACPAgent"); - expect(payload.agent.acp_model).toBeUndefined(); + expect(payload.agent_settings.agent_kind).toBe("acp"); + expect(payload.agent_settings.acp_model).toBeUndefined(); }); it("resolves an empty acp_command from the registry by acp_server", () => { - // The Settings → Agent page and onboarding both store ``acp_command: - // []`` for the default-preset path on the assumption that the agent- - // server resolves it from ``acp_server``. The agent-server's ACPAgent - // model does no such resolution — empty list crashes the spawn with - // ``IndexError: list index out of range`` (acp_agent.py:1013) and the - // conversation hangs in ``idle`` forever. The adapter has to expand - // the command before the payload leaves the client. const payload = buildStartConversationRequest({ settings: { ...DEFAULT_SETTINGS, @@ -925,10 +897,10 @@ describe("buildStartConversationRequest — ACP discriminator", () => { }, }, }) as { - agent: Record & { acp_command?: unknown[] }; + agent_settings: Record & { acp_command?: unknown[] }; }; - expect(payload.agent.acp_command).toEqual([ + expect(payload.agent_settings.acp_command).toEqual([ "npx", "-y", "@agentclientprotocol/claude-agent-acp", @@ -936,8 +908,6 @@ describe("buildStartConversationRequest — ACP discriminator", () => { }); it("resolves an absent acp_command for built-in providers too", () => { - // The acp_command field may also be omitted entirely (e.g. on an older - // settings shape that predates the field). Same fix applies. const payload = buildStartConversationRequest({ settings: { ...DEFAULT_SETTINGS, @@ -949,10 +919,10 @@ describe("buildStartConversationRequest — ACP discriminator", () => { }, }, }) as { - agent: Record & { acp_command?: unknown[] }; + agent_settings: Record & { acp_command?: unknown[] }; }; - expect(payload.agent.acp_command).toEqual([ + expect(payload.agent_settings.acp_command).toEqual([ "npx", "-y", "@zed-industries/codex-acp", @@ -960,10 +930,6 @@ describe("buildStartConversationRequest — ACP discriminator", () => { }); it("leaves acp_command alone when acp_server is 'custom'", () => { - // Custom servers carry the user's explicit command. If they submitted - // an empty one, that is their bug to see — the registry has no entry - // to fall back to, and silently inventing one would be worse than the - // explicit spawn error. const payload = buildStartConversationRequest({ settings: { ...DEFAULT_SETTINGS, @@ -976,19 +942,13 @@ describe("buildStartConversationRequest — ACP discriminator", () => { }, }, }) as { - agent: Record & { acp_command?: unknown[] }; + agent_settings: Record & { acp_command?: unknown[] }; }; - expect(payload.agent.acp_command).toEqual([]); + expect(payload.agent_settings.acp_command).toEqual([]); }); it("leaves acp_command alone for an unknown acp_server key", () => { - // Future SDK adds a new provider before canvas's local mirror picks - // it up: we don't recognise the key, so we can't expand the command - // — but we also don't crash, and don't silently substitute one of - // the known commands. The agent-server will produce the same - // IndexError as before, which is the correct surface for "your - // local canvas is out of date with the SDK." const payload = buildStartConversationRequest({ settings: { ...DEFAULT_SETTINGS, @@ -1001,18 +961,13 @@ describe("buildStartConversationRequest — ACP discriminator", () => { }, }, }) as { - agent: Record & { acp_command?: unknown[] }; + agent_settings: Record & { acp_command?: unknown[] }; }; - expect(payload.agent.acp_command).toEqual([]); + expect(payload.agent_settings.acp_command).toEqual([]); }); it("treats acp_model: '' (empty string) as 'no override'", () => { - // 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: ''``). const payload = buildStartConversationRequest({ settings: { ...DEFAULT_SETTINGS, @@ -1025,27 +980,13 @@ describe("buildStartConversationRequest — ACP discriminator", () => { }, }, }) as { - agent: Record & { acp_model?: unknown }; + agent_settings: 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_settings.acp_model).toBe(""); }); it("ACP → OpenHands → ACP round trip leaves no field leakage", () => { - // Toggling agent_kind via the UI should not let stale ``acp_*`` - // state pollute an OpenHands run, and (in the reverse direction) - // shouldn't let LLM/condenser/MCP state pollute an ACP payload. - // We exercise both legs here against the same starting settings - // shape so the round-trip is provable, not just inferred from the - // single-direction tests. - const baseAcpSettings = { ...DEFAULT_SETTINGS, agent_settings: { @@ -1055,15 +996,12 @@ describe("buildStartConversationRequest — ACP discriminator", () => { acp_command: [], acp_env: { ANTHROPIC_API_KEY: "user-set-via-api" }, acp_model: "claude-opus-4-5", - // LLM-only crud that would leak without the strip: agent: "CodeActAgent", llm: { model: "gpt-4o", api_key: "stale-from-prior-oh-run" }, condenser: { enabled: true, max_size: 200 }, }, }; - // Leg 1: ACP → OpenHands. The OpenHands branch must drop every - // acp_* field; the LLM block survives. const ohPayload = buildStartConversationRequest({ settings: { ...baseAcpSettings, @@ -1072,21 +1010,23 @@ describe("buildStartConversationRequest — ACP discriminator", () => { agent_kind: "openhands", }, }, - }) as { agent: Record & { llm: Record } }; + }) as { + agent_settings: Record & { + llm: Record; + }; + }; - expect(ohPayload.agent.kind).toBe("Agent"); - expect(ohPayload.agent.acp_command).toBeUndefined(); - expect(ohPayload.agent.acp_env).toBeUndefined(); - expect(ohPayload.agent.acp_model).toBeUndefined(); - expect(ohPayload.agent.acp_server).toBeUndefined(); - expect(ohPayload.agent.llm.model).toBe("gpt-4o"); + expect(ohPayload.agent_settings.agent_kind).toBe("openhands"); + expect(ohPayload.agent_settings.acp_command).toBeUndefined(); + expect(ohPayload.agent_settings.acp_env).toBeUndefined(); + expect(ohPayload.agent_settings.acp_model).toBeUndefined(); + expect(ohPayload.agent_settings.acp_server).toBeUndefined(); + expect(ohPayload.agent_settings.llm.model).toBe("gpt-4o"); - // Leg 2: OpenHands → ACP (back). The ACP branch must drop the - // llm/condenser/agent fields; the acp_* state survives. const acpPayload = buildStartConversationRequest({ settings: baseAcpSettings, }) as { - agent: Record & { + agent_settings: Record & { acp_command?: unknown; acp_env?: unknown; acp_model?: unknown; @@ -1095,17 +1035,17 @@ describe("buildStartConversationRequest — ACP discriminator", () => { }; }; - expect(acpPayload.agent.kind).toBe("ACPAgent"); - expect(acpPayload.agent.acp_command).toEqual([ + expect(acpPayload.agent_settings.agent_kind).toBe("acp"); + expect(acpPayload.agent_settings.acp_command).toEqual([ "npx", "-y", "@agentclientprotocol/claude-agent-acp", ]); - expect(acpPayload.agent.acp_model).toBe("claude-opus-4-5"); - expect(acpPayload.agent.acp_env).toEqual({ + expect(acpPayload.agent_settings.acp_model).toBe("claude-opus-4-5"); + expect(acpPayload.agent_settings.acp_env).toEqual({ ANTHROPIC_API_KEY: "user-set-via-api", }); - expect(acpPayload.agent.llm).toBeUndefined(); - expect(acpPayload.agent.condenser).toBeUndefined(); + expect(acpPayload.agent_settings.llm).toBeUndefined(); + expect(acpPayload.agent_settings.condenser).toBeUndefined(); }); }); diff --git a/__tests__/components/conversation-events/chat/event-content-helpers/should-render-event.test.ts b/__tests__/components/conversation-events/chat/event-content-helpers/should-render-event.test.ts index 67166d7e9..34f825360 100644 --- a/__tests__/components/conversation-events/chat/event-content-helpers/should-render-event.test.ts +++ b/__tests__/components/conversation-events/chat/event-content-helpers/should-render-event.test.ts @@ -7,6 +7,13 @@ import { createUserMessageEvent, } from "test-utils"; import { ACPToolCallEvent } from "#/types/agent-server/core/events/acp-tool-call-event"; +import { + ActionEvent, + ObservationEvent, + SecurityRisk, +} from "#/types/agent-server/core"; +import { SwitchLLMAction } from "#/types/agent-server/core/base/action"; +import { SwitchLLMObservation } from "#/types/agent-server/core/base/observation"; const makeACPEvent = ( overrides: Partial = {}, @@ -88,3 +95,66 @@ describe("shouldRenderEvent - ACPToolCallEvent", () => { expect(shouldRenderEvent(event)).toBe(false); }); }); + +describe("shouldRenderEvent - SwitchLLM", () => { + const switchAction: ActionEvent = { + id: "switch-action", + timestamp: "2024-01-01T00:00:00Z", + source: "agent", + thought: [], + thinking_blocks: [], + action: { + kind: "SwitchLLMAction", + profile_name: "haiku", + reason: "Use a cheaper model.", + }, + tool_name: "switch_llm", + tool_call_id: "tool-switch", + tool_call: { + id: "tool-switch", + type: "function", + function: { + name: "switch_llm", + arguments: JSON.stringify({ profile_name: "haiku" }), + }, + }, + llm_response_id: "response-switch", + security_risk: SecurityRisk.LOW, + }; + + const makeSwitchObservation = ( + overrides: Partial = {}, + ): ObservationEvent => ({ + id: "switch-observation", + timestamp: "2024-01-01T00:00:01Z", + source: "environment", + tool_name: "switch_llm", + tool_call_id: "tool-switch", + action_id: "switch-action", + observation: { + kind: "SwitchLLMObservation", + content: [{ type: "text", text: "Switched." }], + is_error: false, + profile_name: "haiku", + reason: "Use a cheaper model.", + active_model: "anthropic/claude-haiku-4-5", + ...overrides, + }, + }); + + it("hides switch actions and successful observations for the shared model UI", () => { + expect(shouldRenderEvent(switchAction)).toBe(false); + expect(shouldRenderEvent(makeSwitchObservation())).toBe(false); + }); + + it("keeps failed switch observations visible", () => { + expect( + shouldRenderEvent( + makeSwitchObservation({ + is_error: true, + content: [{ type: "text", text: "Profile was not found." }], + }), + ), + ).toBe(true); + }); +}); diff --git a/src/api/agent-server-adapter.ts b/src/api/agent-server-adapter.ts index 5feb1d611..a9d1f4287 100644 --- a/src/api/agent-server-adapter.ts +++ b/src/api/agent-server-adapter.ts @@ -4,7 +4,10 @@ import { Settings, SettingsValue } from "#/types/settings"; import { ACP_PROVIDERS } from "#/constants/acp-providers"; import { getAgentServerClientOptions } from "./agent-server-client-options"; import { isAgentServerToolAvailable } from "./agent-server-compatibility"; -import { getAgentServerWorkingDir } from "./agent-server-config"; +import { + getAgentServerWorkingDir, + shouldLoadPublicSkills, +} from "./agent-server-config"; import { getEffectiveLocalBackend } from "./backend-registry/active-store"; import { buildAuthHeaders } from "./backend-registry/auth"; import { @@ -80,8 +83,6 @@ const DEFAULT_TOOL_NAMES = [ ]; const BROWSER_TOOL_SET_NAME = "browser_tool_set"; const TASK_TOOL_SET_NAME = "task_tool_set"; -const DEFAULT_BUILT_IN_TOOL_NAMES = ["FinishTool", "ThinkTool"]; -const SWITCH_LLM_TOOL_NAME = "SwitchLLMTool"; function browserToolsEnabled() { return import.meta.env.VITE_ENABLE_BROWSER_TOOLS !== "false"; @@ -328,38 +329,33 @@ export function toConversationPage(data: { type SettingsRecord = Record; -// Keys we strip before forwarding ``agent_settings`` into the OpenHands -// ``Agent`` payload. ``agent_kind`` is *not* in this set — it is read by -// ``buildStartConversationRequest`` to decide whether to build an -// ``Agent`` or an ``ACPAgent`` payload, and stripped on the LLM branch. -const AGENT_SETTINGS_METADATA_KEYS = new Set(["schema_version", "agent"]); +interface AgentToolSpec { + name: string; + params: SettingsRecord; +} + +type AgentSettingsPayload = SettingsRecord & { + llm?: SettingsRecord; + agent_context: SettingsRecord; + tools?: AgentToolSpec[]; +}; + +interface LocalWorkspacePayload { + kind: "LocalWorkspace"; + working_dir: string; +} + +interface InitialMessagePayload { + role: "user"; + content: Array<{ type: "text"; text: string }>; + run: true; +} + +type ConversationSettingsPayload = SettingsRecord & { + workspace: LocalWorkspacePayload; + initial_message?: InitialMessagePayload; +}; -/** - * All ACPAgent-specific settings the adapter handles. Serves two opposite - * roles depending on the active ``agent_kind``: - * - * 1. **Allow-list for the ACP branch** — ``buildConfiguredAcpAgentSettings`` - * iterates this list to decide what to forward into the ACPAgent - * payload. Anything not in the list (``llm``, ``condenser``, - * ``mcp_config``, ``tools``, ``agent``, …) is dropped so the - * agent-server's pydantic model doesn't reject the create as a - * pydantic extra. - * - * 2. **Deny-list for the OpenHands branch** — ``buildConfiguredAgentSettings`` - * deletes these same keys to prevent leftover ACP state (set either - * from a previous ACP run via the UI, or via the raw API) from - * leaking into an Agent payload where pydantic would reject them. - * - * That's why the list intentionally covers fields that have no UI yet - * (``acp_args``, ``acp_env``, ``acp_session_mode``, ``acp_prompt_timeout``): - * trimming it to UI-visible fields would solve role (1) at the cost of - * silently leaking those API-set fields when the user toggles back to - * an OpenHands agent. Keep aligned with the ``acp_*`` fields on - * ``ACPAgentSettings`` in - * ``openhands-sdk/openhands/sdk/settings/model.py`` — there is no - * matching constant on the Python side, so this is hand-maintained - * (drift tracked in agent-canvas#587 alongside ``ACP_PROVIDERS``). - */ const ACP_SETTINGS_KEYS = [ "acp_command", "acp_args", @@ -369,12 +365,6 @@ const ACP_SETTINGS_KEYS = [ "acp_prompt_timeout", ] as const; -/** - * Conversation-tag key under which the ACP provider key (e.g. ``"codex"``, - * ``"claude-code"``) is stored. The agent-server validates tag keys against - * ``^[a-z0-9]+$``, so the snake_case ``acp_server`` form is unusable — - * keep this aligned with the validator regex. - */ export const ACP_SERVER_TAG_KEY = "acpserver"; const CONVERSATION_SETTINGS_METADATA_KEYS = new Set([ @@ -430,50 +420,67 @@ function getConversationSecurityAnalyzer(conversationSettings: SettingsRecord) { } } -function getAgentTools(agentSettings: SettingsRecord) { - const tools = DEFAULT_TOOL_NAMES.map((name) => ({ name, params: {} })); - if ( - browserToolsEnabled() && - isAgentServerToolAvailable(BROWSER_TOOL_SET_NAME) - ) { - tools.push({ name: BROWSER_TOOL_SET_NAME, params: {} }); - } - // Sub-agent delegation: only expose `task_tool_set` when the user has - // opted in via Settings > Agent. The agent server's tool_router preloads - // the tool and registers the built-in subagents (code-explorer, - // bash-runner, web-researcher, general-purpose) when it's requested. - // Older servers that don't advertise the tool in /api/server_info's - // `usable_tools` are skipped via the capability probe. - if ( - agentSettings.enable_sub_agents === true && - isAgentServerToolAvailable(TASK_TOOL_SET_NAME) - ) { - tools.push({ name: TASK_TOOL_SET_NAME, params: {} }); +function isToolRecord( + value: unknown, +): value is { name: string; params?: unknown } { + return ( + !!value && + typeof value === "object" && + !Array.isArray(value) && + typeof (value as { name?: unknown }).name === "string" + ); +} + +function shouldIncludeTool(name: string, agentSettings: SettingsRecord) { + if (name === BROWSER_TOOL_SET_NAME) { + return browserToolsEnabled() && isAgentServerToolAvailable(name); + } + + if (name === TASK_TOOL_SET_NAME) { + return ( + agentSettings.enable_sub_agents === true && + isAgentServerToolAvailable(name) + ); } - return tools; + + return true; } -function getBuiltInToolNames(agentSettings: SettingsRecord) { - const configured = Array.isArray(agentSettings.include_default_tools) - ? agentSettings.include_default_tools.filter( - (name): name is string => typeof name === "string" && name.length > 0, - ) - : DEFAULT_BUILT_IN_TOOL_NAMES; +function getAgentTools(agentSettings: SettingsRecord): AgentToolSpec[] { + const tools = new Map(); + + for (const name of DEFAULT_TOOL_NAMES) { + tools.set(name, { name, params: {} }); + } + + for (const name of [BROWSER_TOOL_SET_NAME, TASK_TOOL_SET_NAME]) { + if (shouldIncludeTool(name, agentSettings)) { + tools.set(name, { name, params: {} }); + } + } + const configuredTools = agentSettings.tools; if ( - agentSettings.enable_switch_llm_tool === true && - !configured.includes(SWITCH_LLM_TOOL_NAME) + Array.isArray(configuredTools) && + configuredTools.every((tool) => isToolRecord(tool)) ) { - return [...configured, SWITCH_LLM_TOOL_NAME]; + for (const tool of configuredTools) { + if (shouldIncludeTool(tool.name, agentSettings)) { + tools.set(tool.name, { + name: tool.name, + params: toRecord(tool.params), + }); + } + } } - return configured; + return Array.from(tools.values()); } function buildInitialMessage( query?: string, conversationInstructions?: string, -) { +): InitialMessagePayload | null { const parts = [query?.trim(), conversationInstructions?.trim()].filter( Boolean, ); @@ -488,34 +495,70 @@ function buildInitialMessage( }; } -function buildCondenserConfig( - llm: SettingsRecord, - rawCondenser: unknown, -): SettingsRecord | undefined { - const condenser = toRecord(rawCondenser); +function buildAgentContext(agentSettings: SettingsRecord): SettingsRecord { + const runtimeServicesSuffix = buildRuntimeServicesSystemSuffix(); + return { + ...toRecord(agentSettings.agent_context), + load_public_skills: shouldLoadPublicSkills(), + load_user_skills: true, + ...(runtimeServicesSuffix + ? { system_message_suffix: runtimeServicesSuffix } + : {}), + }; +} + +function isAcpAgent(settings: Settings): boolean { + const agentSettings = toRecord(settings.agent_settings); + return agentSettings.agent_kind === "acp"; +} - if (condenser.enabled !== true) { - return undefined; +function getAcpServerTag(settings: Settings): string | undefined { + const agentSettings = toRecord(settings.agent_settings); + const value = agentSettings.acp_server; + return typeof value === "string" && value.length > 0 ? value : undefined; +} + +function resolveAcpCommand(agentSettings: SettingsRecord): unknown { + const cmd = agentSettings.acp_command; + const isEmpty = Array.isArray(cmd) && cmd.length === 0; + const noCommand = cmd === undefined; + if (!isEmpty && !noCommand) { + return cmd; } - const condenserLlm = { - ...llm, - usage_id: "condenser", - }; + const serverKey = + typeof agentSettings.acp_server === "string" + ? agentSettings.acp_server + : undefined; + const provider = ACP_PROVIDERS.find(({ key }) => key === serverKey); + return provider ? [...provider.default_command] : cmd; +} - const config: SettingsRecord = { - kind: "LLMSummarizingCondenser", - llm: condenserLlm, +function buildConfiguredAcpAgentSettings( + settings: Settings, +): AgentSettingsPayload { + const agentSettings = toRecord(settings.agent_settings); + const payload: AgentSettingsPayload = { + agent_kind: "acp", + agent_context: buildAgentContext(agentSettings), }; - if (typeof condenser.max_size === "number") { - config.max_size = condenser.max_size; + for (const key of ACP_SETTINGS_KEYS) { + const value = + key === "acp_command" + ? resolveAcpCommand(agentSettings) + : agentSettings[key]; + if (value !== undefined && value !== null) { + payload[key] = value; + } } - return config; + return payload; } -function buildConfiguredAgentSettings(settings: Settings): SettingsRecord { +function buildConfiguredOpenHandsAgentSettings( + settings: Settings, +): AgentSettingsPayload { const agentSettings = toRecord(settings.agent_settings); const llm = toRecord(agentSettings.llm); @@ -536,147 +579,30 @@ function buildConfiguredAgentSettings(settings: Settings): SettingsRecord { delete llm.base_url; } - const condenser = buildCondenserConfig(llm, agentSettings.condenser); - const includeDefaultTools = getBuiltInToolNames(agentSettings); - - AGENT_SETTINGS_METADATA_KEYS.forEach((key) => delete agentSettings[key]); - delete agentSettings.enable_switch_llm_tool; - // Drop fields that only apply to the ACP path; do not let them leak into - // an OpenHands Agent payload where pydantic would reject extras. - delete agentSettings.agent_kind; - delete agentSettings.acp_server; - for (const key of ACP_SETTINGS_KEYS) { - delete agentSettings[key]; - } - const mcpConfig = toRecord(agentSettings.mcp_config); if (Object.keys(mcpConfig).length === 0 || !("mcpServers" in mcpConfig)) { delete agentSettings.mcp_config; } - if (condenser) { - agentSettings.condenser = condenser; - } else { - delete agentSettings.condenser; + delete agentSettings.acp_server; + for (const key of ACP_SETTINGS_KEYS) { + delete agentSettings[key]; } return { ...agentSettings, llm, + agent_context: buildAgentContext(agentSettings), tools: getAgentTools(agentSettings), - include_default_tools: includeDefaultTools, - }; -} - -function buildConfiguredAcpAgentSettings(settings: Settings): SettingsRecord { - const agentSettings = toRecord(settings.agent_settings); - - // Only forward fields the ACPAgent model knows about. Everything else - // (``llm``, ``condenser``, ``mcp_config``, ``agent``, ``schema_version``, - // ``tools``, ``agent_kind``) is irrelevant on this path; the agent-server - // would either ignore it or reject it as a pydantic extra. ``acp_server`` - // is a UI bookkeeping field — it does not belong in the agent payload - // either, but we surface it on the conversation tags instead (see - // ``buildStartConversationRequest``). - const payload: SettingsRecord = {}; - for (const key of ACP_SETTINGS_KEYS) { - if (agentSettings[key] !== undefined && agentSettings[key] !== null) { - payload[key] = agentSettings[key]; - } - } - - // The Settings → Agent page (and onboarding) stores ``acp_command: []`` - // for the "default preset" path, expecting the registry to resolve it. - // The agent-server's ACPAgent model takes only an explicit ``acp_command`` - // though — empty list means ``subprocess(command[0], ...)`` raises - // ``IndexError: list index out of range`` at spawn time, the agent loop - // dies silently, and the conversation hangs in ``idle``. Resolve the - // command from ``ACP_PROVIDERS`` here so the agent-server sees a real - // 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 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]; - } - } - - return payload; -} - -function createAgentFromSettings( - agentSettings: SettingsRecord, - options: { acp?: boolean } = {}, -) { - const runtimeServicesSuffix = buildRuntimeServicesSystemSuffix(); - // ``load_public_skills``, ``load_user_skills``, and - // ``system_message_suffix`` are all marked ``acp_compatible: true`` in - // the SDK's AgentContext model — they're rendered into the system - // prompt the ACP CLI receives via ``ACPAgent._render_suffix``, so - // building the same agent_context here means a Claude-Code / Codex - // user gets the same skill catalog and runtime-services awareness an - // OpenHands-driven conversation does. Leaving it off (the previous - // ACP branch returned ``{kind:"ACPAgent",...agentSettings}`` with no - // ``agent_context``) silently dropped both, matching neither the - // SDK contract nor OpenHands' own behaviour. - // - // The ``acp_compatible`` markers live on the SDK fields themselves - // in ``openhands-sdk/openhands/sdk/context/agent_context.py``: - // - ``system_message_suffix`` (Field, json_schema_extra at L66) - // - ``load_user_skills`` (Field, json_schema_extra at L80) - // - ``load_public_skills`` (Field, json_schema_extra at L89) - // ``AgentContext.validate_acp_compatibility`` rejects any field not - // tagged that way at ``ACPAgent`` init time. If a future SDK bump - // demotes one of these (drops the marker), the ACP conversation start - // will 422 here — at which point the right move is to drop the - // demoted field from this dict, not to wrap a workaround. - // - // ``secrets`` is filled in later by the secret bridge in - // ``buildStartConversationRequest`` (when ``customSecrets`` is set); - // we don't seed it here so non-secret start paths don't end up with - // an empty ``secrets: {}`` map. - const agentContext: Record = { - load_public_skills: true, - load_user_skills: true, - // When the dev launcher provided ``VITE_RUNTIME_SERVICES_INFO``, - // append a block to the system prompt so the - // agent knows which services exist in this dev stack (e.g. - // automation backend URL, ingress URL) instead of having to probe. - ...(runtimeServicesSuffix - ? { system_message_suffix: runtimeServicesSuffix } - : {}), - }; - if (options.acp) { - return { - kind: "ACPAgent", - ...agentSettings, - agent_context: agentContext, - }; - } - return { - kind: "Agent", - ...agentSettings, - agent_context: agentContext, }; } -function isAcpAgent(settings: Settings): boolean { - const agentSettings = toRecord(settings.agent_settings); - return agentSettings.agent_kind === "acp"; -} - -function getAcpServerTag(settings: Settings): string | undefined { - const agentSettings = toRecord(settings.agent_settings); - const value = agentSettings.acp_server; - return typeof value === "string" && value.length > 0 ? value : undefined; +function buildConfiguredAgentSettings( + settings: Settings, +): AgentSettingsPayload { + return isAcpAgent(settings) + ? buildConfiguredAcpAgentSettings(settings) + : buildConfiguredOpenHandsAgentSettings(settings); } function buildConfiguredConversationSettings(options: { @@ -685,7 +611,7 @@ function buildConfiguredConversationSettings(options: { conversationInstructions?: string; plugins?: PluginSpec[]; workingDir?: string; -}): SettingsRecord { +}): ConversationSettingsPayload { const { settings, query, conversationInstructions, plugins, workingDir } = options; const conversationSettings = toRecord(settings.conversation_settings); @@ -695,7 +621,7 @@ function buildConfiguredConversationSettings(options: { (key) => delete conversationSettings[key], ); - return { + const payload: ConversationSettingsPayload = { ...conversationSettings, workspace: { kind: "LocalWorkspace", @@ -712,13 +638,10 @@ function buildConfiguredConversationSettings(options: { } : {}), }; + + return payload; } -/** - * A secret looked up from the agent-server at runtime. - * This allows secrets configured in Settings > Secrets to be available - * to conversations without exposing values to the frontend. - */ interface LookupSecret { kind: "LookupSecret"; url: string; @@ -726,6 +649,23 @@ interface LookupSecret { description?: string; } +type StartConversationPayload = Record & { + agent_settings: AgentSettingsPayload; + workspace: LocalWorkspacePayload; + confirmation_policy: SettingsRecord; + security_analyzer?: SettingsRecord; + initial_message?: InitialMessagePayload; + max_iterations: number; + stuck_detection: true; + autotitle: true; + worktree: true; + secrets_encrypted?: true; + conversation_id?: string; + secrets?: Record; + tags?: Record; + tool_module_qualnames?: Record; +}; + export interface StartConversationOptions { settings: Settings; query?: string; @@ -733,47 +673,25 @@ export interface StartConversationOptions { plugins?: PluginSpec[]; conversationId?: string; workingDir?: string; - /** - * Pre-fetched agent settings with encrypted secrets. - * If provided, these will be used instead of settings.agent_settings. - */ encryptedAgentSettings?: Record; - /** - * Pre-fetched conversation settings with encrypted secrets. - * If provided, these will be used instead of settings.conversation_settings. - */ encryptedConversationSettings?: Record; - /** - * Whether the secrets in agent/conversation settings are encrypted. - * If true, the server will decrypt them before use. - */ secretsEncrypted?: boolean; - /** - * Custom secrets to include in the conversation. - * Each entry maps a secret name to metadata (description). - * The actual values are fetched at runtime via LookupSecret. - */ customSecrets?: Array<{ name: string; description?: string }>; } export function buildStartConversationRequest( options: StartConversationOptions, -) { - // Use encrypted settings if provided, otherwise fall back to regular settings +): StartConversationPayload { const sourceAgentSettings = options.encryptedAgentSettings ? { ...options.settings, agent_settings: options.encryptedAgentSettings } : options.settings; const acpMode = isAcpAgent(sourceAgentSettings); - const agentSettings = acpMode - ? buildConfiguredAcpAgentSettings(sourceAgentSettings) - : buildConfiguredAgentSettings(sourceAgentSettings); - const agent = createAgentFromSettings(agentSettings, { acp: acpMode }); + const agentSettings = buildConfiguredAgentSettings(sourceAgentSettings); const acpServerTag = acpMode ? getAcpServerTag(sourceAgentSettings) : undefined; - // For conversation settings, merge encrypted settings if provided const sourceConversationOptions = options.encryptedConversationSettings ? { ...options, @@ -788,8 +706,8 @@ export function buildStartConversationRequest( sourceConversationOptions, ); - const payload: Record = { - agent, + const payload: StartConversationPayload = { + agent_settings: agentSettings, workspace: conversationSettings.workspace, confirmation_policy: getConversationConfirmationPolicy(conversationSettings), @@ -802,19 +720,10 @@ export function buildStartConversationRequest( worktree: true, }; - // Stamp the ACP provider key onto the conversation so the chip can render - // a brand name from a single source of truth. The tag is purely - // informational — frontend looks it up against ``ACP_PROVIDERS``; the - // agent-server treats it as an opaque string. - // - // Tag *keys* must match ``^[a-z0-9]+$`` per agent-server validation — - // ``acp_server`` would be rejected with a 422. ``acpserver`` flattens - // the snake_case original into the allowed shape. if (acpServerTag) { payload.tags = { [ACP_SERVER_TAG_KEY]: acpServerTag }; } - // Add secrets_encrypted flag if secrets are encrypted if (options.secretsEncrypted) { payload.secrets_encrypted = true; } @@ -841,10 +750,6 @@ export function buildStartConversationRequest( payload.hook_config = conversationSettings.hook_config; } - // Always include the canvas_ui tool module so the agent-server imports it - // and registers the tool. User-supplied entries from conversationSettings - // take precedence on key conflict (the canvas_ui key is ours and shouldn't - // collide in practice). payload.tool_module_qualnames = { [CANVAS_UI_TOOL_NAME]: CANVAS_UI_TOOL_MODULE, ...((conversationSettings.tool_module_qualnames as @@ -856,10 +761,6 @@ export function buildStartConversationRequest( payload.agent_definitions = conversationSettings.agent_definitions; } - // Add custom secrets as LookupSecret entries. - // The agent-server fetches the value at runtime from - // `/api/settings/secrets/{name}` on its own host, so the URL stays - // host-relative; auth headers come from the active local backend. if (options.customSecrets && options.customSecrets.length > 0) { const backend = getEffectiveLocalBackend(); const headers = buildAuthHeaders(backend); @@ -881,49 +782,17 @@ export function buildStartConversationRequest( payload.secrets = secrets; - // ACPAgent bridge: mirror the same secrets onto - // ``agent.agent_context.secrets`` so the agent-server's existing - // ``ACPAgent._start_acp_server`` env-injection loop picks them up - // and writes them into the ACP subprocess environment. Without - // this, the OpenHands ``Conversation.update_secrets`` path - // populates ``secret_registry`` — which the LLM-driven Agent - // reads, but the ACP subprocess never sees, so secrets set in - // Settings → Secrets (e.g. ``ANTHROPIC_API_KEY``) silently fail - // to reach the ACP CLI. - // - // Mirrors what OpenHands' app-server does in - // ``_build_acp_start_conversation_request``: wraps the conversation - // secrets in an ``AgentContext(secrets=secrets)`` before constructing - // the ACPAgent. This shim becomes redundant once canvas pins to an - // agent-server build that includes software-agent-sdk PR #3299 - // (which makes ``ACPAgent`` read ``state.secret_registry`` itself); - // at that point this block can be deleted with no behaviour change. - // - // Merge into the existing ``agent_context`` (``createAgentFromSettings`` - // seeds ``load_public_skills`` / ``load_user_skills`` / optionally a - // ``system_message_suffix`` from the dev launcher's runtime-services - // info — all marked ``acp_compatible: true`` in the SDK). Overwriting - // would drop those. if (acpMode) { - const agentRecord = payload.agent as Record; - const existingContext = - (agentRecord.agent_context as Record | undefined) ?? - {}; - agentRecord.agent_context = { ...existingContext, secrets }; + payload.agent_settings.agent_context = { + ...payload.agent_settings.agent_context, + secrets, + }; } } return payload; } -/** - * Build a start conversation request using encrypted settings from the server. - * This is the recommended way to start conversations from the frontend, - * as it ensures secrets are never exposed in plaintext to the browser. - * - * Also fetches custom secrets from the settings store and adds them as - * LookupSecret entries so they're available to the conversation at runtime. - */ export async function buildStartConversationRequestWithEncryptedSettings(options: { settings: Settings; query?: string; @@ -932,10 +801,8 @@ export async function buildStartConversationRequestWithEncryptedSettings(options conversationId?: string; workingDir?: string; }): Promise> { - // Import SecretsService dynamically to avoid circular dependencies const { SecretsService } = await import("./secrets-service"); - // Fetch settings with encrypted secrets and custom secrets list in parallel const [settingsResult, customSecrets] = await Promise.all([ SettingsService.getSettingsForConversation(), SecretsService.getSecrets(), diff --git a/src/components/conversation-events/chat/event-content-helpers/get-event-content.tsx b/src/components/conversation-events/chat/event-content-helpers/get-event-content.tsx index 8e486c2de..51cf2ad13 100644 --- a/src/components/conversation-events/chat/event-content-helpers/get-event-content.tsx +++ b/src/components/conversation-events/chat/event-content-helpers/get-event-content.tsx @@ -226,6 +226,14 @@ const getObservationEventTitle = ( name: event.observation.skill_name, }; break; + case "SwitchLLMObservation": + observationKey = event.observation.is_error + ? "MODEL$SWITCH_FAILED" + : "MODEL$SWITCHED_TO_PROFILE"; + observationValues = { + name: event.observation.profile_name, + }; + break; case "BrowserObservation": observationKey = "OBSERVATION_MESSAGE$BROWSE"; break; diff --git a/src/components/conversation-events/chat/event-content-helpers/get-observation-content.ts b/src/components/conversation-events/chat/event-content-helpers/get-observation-content.ts index 0658aa411..74c1162ae 100644 --- a/src/components/conversation-events/chat/event-content-helpers/get-observation-content.ts +++ b/src/components/conversation-events/chat/event-content-helpers/get-observation-content.ts @@ -15,6 +15,7 @@ import { GlobObservation, GrepObservation, InvokeSkillObservation, + SwitchLLMObservation, } from "#/types/agent-server/core/base/observation"; // File Editor Observations @@ -171,6 +172,33 @@ const getInvokeSkillObservationContent = ( return content; }; +const getSwitchLLMObservationContent = ( + event: ObservationEvent, +): string => { + const { observation } = event; + + const textContent = observation.content + .filter((c) => c.type === "text") + .map((c) => c.text) + .join("\n"); + + if (observation.is_error) { + return textContent + ? `**Error:**\n${textContent}` + : `**Error:**\nFailed to switch LLM profile \`${observation.profile_name}\`.`; + } + + const parts = [`**Profile:** \`${observation.profile_name}\``]; + if (observation.active_model) { + parts.push(`**Active model:** \`${observation.active_model}\``); + } + if (observation.reason) { + parts.push(`**Reason:** ${observation.reason}`); + } + + return parts.join("\n"); +}; + // Complex Observations const getTaskTrackerObservationContent = ( event: ObservationEvent, @@ -373,6 +401,11 @@ export const getObservationContent = (event: ObservationEvent): string => { event as ObservationEvent, ); + case "SwitchLLMObservation": + return getSwitchLLMObservationContent( + event as ObservationEvent, + ); + default: return getDefaultEventContent(event); } diff --git a/src/components/conversation-events/chat/event-content-helpers/get-observation-result.ts b/src/components/conversation-events/chat/event-content-helpers/get-observation-result.ts index 41f229d68..08eb27361 100644 --- a/src/components/conversation-events/chat/event-content-helpers/get-observation-result.ts +++ b/src/components/conversation-events/chat/event-content-helpers/get-observation-result.ts @@ -51,6 +51,9 @@ export const getObservationResult = ( case "MCPToolObservation": if (observation.is_error) return "error"; return "success"; + case "SwitchLLMObservation": + if (observation.is_error) return "error"; + return "success"; default: return "success"; } diff --git a/src/components/conversation-events/chat/event-content-helpers/should-render-event.ts b/src/components/conversation-events/chat/event-content-helpers/should-render-event.ts index 2c252fe79..18efbbb4e 100644 --- a/src/components/conversation-events/chat/event-content-helpers/should-render-event.ts +++ b/src/components/conversation-events/chat/event-content-helpers/should-render-event.ts @@ -34,11 +34,27 @@ export const shouldRenderEvent = (event: OpenHandsEvent) => { return false; } + // The model switch tool reuses the same inline model message UI as + // `/model ` once the observation arrives. + if (actionType === "SwitchLLMAction") { + return false; + } + return true; } // Render observation events if (isObservationEvent(event)) { + // Successful model switches are rendered through ModelMessages so they + // look identical to `/model ` confirmations. Failed switches + // still render as observations so the error remains visible in chat. + if ( + event.observation.kind === "SwitchLLMObservation" && + !event.observation.is_error + ) { + return false; + } + return true; } diff --git a/src/contexts/conversation-websocket-context.tsx b/src/contexts/conversation-websocket-context.tsx index b6c6d7fd6..7ca9c9c89 100644 --- a/src/contexts/conversation-websocket-context.tsx +++ b/src/contexts/conversation-websocket-context.tsx @@ -34,6 +34,7 @@ import { isPlanningFileEditorObservationEvent, isBrowserObservationEvent, isBrowserNavigateActionEvent, + isSwitchLLMObservationEvent, isCanvasUIActionEvent, } from "#/types/agent-server/type-guards"; import { handleCanvasUIAction } from "#/services/canvas-ui"; @@ -56,6 +57,11 @@ import { useReadConversationFile } from "#/hooks/mutation/use-read-conversation- import useMetricsStore from "#/stores/metrics-store"; import { useConversationHistory } from "#/hooks/query/use-conversation-history"; import { setConversationState } from "#/utils/conversation-local-storage"; +import { recordModelSwitchMessage } from "#/hooks/chat/record-model-switch-message"; +import { + invalidateConversationQueries, + updateConversationLlmModelInCache, +} from "#/hooks/mutation/conversation-mutation-utils"; export type WebSocketConnectionState = | "CONNECTING" @@ -390,6 +396,13 @@ export function ConversationWebSocketProvider({ // Use type guard to validate v1 event structure if (isAgentServerEvent(event)) { + const isDuplicateEvent = useEventStore + .getState() + .eventIds.has(event.id); + const switchLLMObservation = + !isDuplicateEvent && isSwitchLLMObservationEvent(event) + ? event + : null; addEvent(event); // Handle displayable error events - show error banner @@ -499,6 +512,27 @@ export function ConversationWebSocketProvider({ useBrowserStore.getState().setUrl(event.action.url); } + if ( + conversationId && + switchLLMObservation && + !switchLLMObservation.observation.is_error + ) { + recordModelSwitchMessage( + conversationId, + switchLLMObservation.observation.profile_name, + ); + + if (switchLLMObservation.observation.active_model) { + updateConversationLlmModelInCache( + queryClient, + conversationId, + switchLLMObservation.observation.active_model, + ); + } + + invalidateConversationQueries(queryClient, conversationId); + } + // Handle canvas_ui custom-tool ActionEvents - drive the frontend // (navigate to a file, switch tabs, show a preview). The tool // executes server-side as a no-op; the actual UI change happens diff --git a/src/hooks/chat/record-model-switch-message.ts b/src/hooks/chat/record-model-switch-message.ts new file mode 100644 index 000000000..bd7e0a200 --- /dev/null +++ b/src/hooks/chat/record-model-switch-message.ts @@ -0,0 +1,12 @@ +import { getLastRenderableEventId } from "#/hooks/chat/model-command-event-anchor"; +import { useModelStore } from "#/stores/model-store"; + +export function recordModelSwitchMessage( + conversationId: string, + profileName: string, + anchorEventId: string | null = getLastRenderableEventId(), +) { + useModelStore + .getState() + .recordSwitch(conversationId, anchorEventId, profileName); +} diff --git a/src/hooks/mutation/conversation-mutation-utils.test.ts b/src/hooks/mutation/conversation-mutation-utils.test.ts index 894842acf..bf34c1b0a 100644 --- a/src/hooks/mutation/conversation-mutation-utils.test.ts +++ b/src/hooks/mutation/conversation-mutation-utils.test.ts @@ -1,6 +1,9 @@ import { describe, expect, it } from "vitest"; import { QueryClient } from "@tanstack/react-query"; -import { updateConversationExecutionStatusInCache } from "./conversation-mutation-utils"; +import { + updateConversationExecutionStatusInCache, + updateConversationLlmModelInCache, +} from "./conversation-mutation-utils"; import { ExecutionStatus } from "#/types/agent-server/core/base/common"; import { AppConversation } from "#/api/conversation-service/agent-server-conversation-service.types"; @@ -51,3 +54,56 @@ describe("updateConversationExecutionStatusInCache", () => { }); }); }); + +describe("updateConversationLlmModelInCache", () => { + it("updates active conversation and list cache entries", () => { + const queryClient = new QueryClient(); + const conversation = createConversation(); + const otherConversation = { ...createConversation(), id: "conversation-2" }; + + queryClient.setQueryData( + ["user", "conversation", conversation.id, "backend-1", null], + conversation, + ); + queryClient.setQueryData(["user", "conversations"], { + pages: [ + { + items: [conversation, otherConversation], + }, + ], + }); + + updateConversationLlmModelInCache( + queryClient, + conversation.id, + "anthropic/claude-haiku-4-5", + ); + + expect( + queryClient.getQueryData([ + "user", + "conversation", + conversation.id, + "backend-1", + null, + ]), + ).toMatchObject({ + llm_model: "anthropic/claude-haiku-4-5", + }); + + expect( + queryClient.getQueryData<{ + pages: Array<{ items: AppConversation[] }>; + }>(["user", "conversations"])?.pages[0].items, + ).toEqual([ + expect.objectContaining({ + id: conversation.id, + llm_model: "anthropic/claude-haiku-4-5", + }), + expect.objectContaining({ + id: otherConversation.id, + llm_model: null, + }), + ]); + }); +}); diff --git a/src/hooks/mutation/conversation-mutation-utils.ts b/src/hooks/mutation/conversation-mutation-utils.ts index a652ac377..647197f54 100644 --- a/src/hooks/mutation/conversation-mutation-utils.ts +++ b/src/hooks/mutation/conversation-mutation-utils.ts @@ -123,6 +123,12 @@ export const updateConversationExecutionStatusInCache = ( ): void => patchConversationInCache(queryClient, conversationId, { execution_status }); +export const updateConversationLlmModelInCache = ( + queryClient: QueryClient, + conversationId: string, + llm_model: string, +): void => patchConversationInCache(queryClient, conversationId, { llm_model }); + export const invalidateConversationQueries = ( queryClient: QueryClient, conversationId: string, 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 160b9c12e..28cb2197e 100644 --- a/src/hooks/mutation/use-switch-llm-profile-and-log.ts +++ b/src/hooks/mutation/use-switch-llm-profile-and-log.ts @@ -1,8 +1,8 @@ import { useCallback } from "react"; 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 { useModelStore } from "#/stores/model-store"; import { displayErrorToast } from "#/utils/custom-toast-handlers"; import { I18nKey } from "#/i18n/declaration"; @@ -14,7 +14,6 @@ import { I18nKey } from "#/i18n/declaration"; */ export function useSwitchLlmProfileAndLog() { const { mutate, isPending } = useSwitchLlmProfile(); - const recordSwitch = useModelStore((s) => s.recordSwitch); const { t } = useTranslation(); const switchAndLog = useCallback( @@ -28,7 +27,11 @@ export function useSwitchLlmProfileAndLog() { // The inline "Switched to" message is scoped to a conversation; // skip it when activating from the home page (no convo yet). if (conversationId) { - recordSwitch(conversationId, anchorEventId, profileName); + recordModelSwitchMessage( + conversationId, + profileName, + anchorEventId, + ); } }, onError: (err: unknown) => { @@ -42,7 +45,7 @@ export function useSwitchLlmProfileAndLog() { }, ); }, - [mutate, recordSwitch, t], + [mutate, t], ); return { switchAndLog, isPending }; diff --git a/src/hooks/mutation/use-switch-llm-profile.ts b/src/hooks/mutation/use-switch-llm-profile.ts index 23f30b653..a2e7b7494 100644 --- a/src/hooks/mutation/use-switch-llm-profile.ts +++ b/src/hooks/mutation/use-switch-llm-profile.ts @@ -5,6 +5,7 @@ import { LLM_PROFILES_QUERY_KEYS, SETTINGS_QUERY_KEYS, } from "#/hooks/query/query-keys"; +import { invalidateConversationQueries } from "./conversation-mutation-utils"; interface SwitchLlmProfileVars { /** @@ -34,9 +35,7 @@ export const useSwitchLlmProfile = () => { queryKey: LLM_PROFILES_QUERY_KEYS.all, }); if (conversationId) { - queryClient.invalidateQueries({ - queryKey: ["user", "conversation", conversationId], - }); + invalidateConversationQueries(queryClient, conversationId); } else { // Home-page activate path (same server endpoint as // useActivateLlmProfile): clear the SettingsService cache so the next diff --git a/src/types/agent-server/core/base/action.ts b/src/types/agent-server/core/base/action.ts index 70f035ac5..6c5eba9f4 100644 --- a/src/types/agent-server/core/base/action.ts +++ b/src/types/agent-server/core/base/action.ts @@ -277,6 +277,17 @@ export interface InvokeSkillAction extends ActionBase<"InvokeSkillAction"> { name: string; } +export interface SwitchLLMAction extends ActionBase<"SwitchLLMAction"> { + /** + * Name of the saved LLM profile to use for future agent steps. + */ + profile_name: string; + /** + * Brief reason why this profile is a better fit for the next step. + */ + reason: string; +} + /** * Frontend-injected custom tool. Emitted over the existing WebSocket as a * regular ActionEvent; intercepted client-side by handleCanvasUIAction. @@ -311,4 +322,5 @@ export type Action = | GlobAction | GrepAction | InvokeSkillAction + | SwitchLLMAction | CanvasUIAction; diff --git a/src/types/agent-server/core/base/base.ts b/src/types/agent-server/core/base/base.ts index 2159fbadc..b3bc4ee3a 100644 --- a/src/types/agent-server/core/base/base.ts +++ b/src/types/agent-server/core/base/base.ts @@ -8,7 +8,8 @@ type EventType = | "StrReplaceEditor" | "TaskTracker" | "PlanningFileEditor" - | "InvokeSkill"; + | "InvokeSkill" + | "SwitchLLM"; type ActionOnlyType = | "BrowserNavigate" diff --git a/src/types/agent-server/core/base/observation.ts b/src/types/agent-server/core/base/observation.ts index e41f541ac..38461811a 100644 --- a/src/types/agent-server/core/base/observation.ts +++ b/src/types/agent-server/core/base/observation.ts @@ -290,6 +290,29 @@ export interface InvokeSkillObservation extends ObservationBase<"InvokeSkillObse is_error?: boolean; } +export interface SwitchLLMObservation extends ObservationBase<"SwitchLLMObservation"> { + /** + * Content returned from the switch LLM tool. + */ + content: Array; + /** + * Whether the profile switch resulted in an error. + */ + is_error: boolean; + /** + * Name of the profile the agent attempted to activate. + */ + profile_name: string; + /** + * Reason the agent gave for the switch. + */ + reason: string | null; + /** + * Model configured by the activated profile, when available. + */ + active_model: string | null; +} + export type Observation = | MCPToolObservation | FinishObservation @@ -303,4 +326,5 @@ export type Observation = | PlanningFileEditorObservation | GlobObservation | GrepObservation - | InvokeSkillObservation; + | InvokeSkillObservation + | SwitchLLMObservation; diff --git a/src/types/agent-server/type-guards.ts b/src/types/agent-server/type-guards.ts index b549538fc..c65143657 100644 --- a/src/types/agent-server/type-guards.ts +++ b/src/types/agent-server/type-guards.ts @@ -9,6 +9,7 @@ import { TerminalObservation, BrowserObservation, BrowserNavigateAction, + SwitchLLMObservation, CanvasUIAction, } from "./core"; import { AgentErrorEvent } from "./core/events/observation-event"; @@ -146,6 +147,15 @@ export const isBrowserObservationEvent = ( ): event is ObservationEvent => isObservationEvent(event) && event.observation.kind === "BrowserObservation"; +/** + * Type guard function to check if an observation event is a SwitchLLMObservation + */ +export const isSwitchLLMObservationEvent = ( + event: OpenHandsEvent, +): event is ObservationEvent => + isObservationEvent(event) && + event.observation.kind === "SwitchLLMObservation"; + /** * Type guard function to check if an action event is a BrowserNavigateAction */