diff --git a/__tests__/api/agent-server-adapter.test.ts b/__tests__/api/agent-server-adapter.test.ts index c62576ef63..a403cde75a 100644 --- a/__tests__/api/agent-server-adapter.test.ts +++ b/__tests__/api/agent-server-adapter.test.ts @@ -4,6 +4,8 @@ import { LAUNCH_CHILD_CONVERSATION_TOOL_NAME } from "#/constants/child-conversat import { ACP_SERVER_TAG_KEY, + AGENT_CANVAS_SOURCE, + CLIENT_SOURCE_TAG_KEY, buildRuntimeServicesSystemSuffix, buildStartConversationRequest, fetchBackendRuntimeServicesInfo, @@ -1371,7 +1373,10 @@ describe("buildStartConversationRequest — ACP discriminator", () => { load_project_skills: true, }); expect(Array.isArray(acpAgentContext.skills)).toBe(true); - expect(payload.tags).toEqual({ [ACP_SERVER_TAG_KEY]: "claude-code" }); + expect(payload.tags).toEqual({ + [ACP_SERVER_TAG_KEY]: "claude-code", + [CLIENT_SOURCE_TAG_KEY]: AGENT_CANVAS_SOURCE, + }); }); it("forwards mcp_config to the ACP subprocess when servers are configured", () => { @@ -1441,7 +1446,9 @@ describe("buildStartConversationRequest — ACP discriminator", () => { 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(); + expect(payload.tags).toEqual({ + [CLIENT_SOURCE_TAG_KEY]: AGENT_CANVAS_SOURCE, + }); }); it("omits acp_model when the user clears it (null)", () => { diff --git a/__tests__/api/agent-server-conversation-service.test.ts b/__tests__/api/agent-server-conversation-service.test.ts index 6cc03433d1..0dbebf1a81 100644 --- a/__tests__/api/agent-server-conversation-service.test.ts +++ b/__tests__/api/agent-server-conversation-service.test.ts @@ -1060,7 +1060,7 @@ describe("AgentServerConversationService", () => { global.fetch = originalFetch; }); - it("forwards parent_conversation_id, agent_type, and sandbox_id to the cloud createConversation payload", async () => { + it("marks Canvas-created cloud conversations with the GUI trigger", async () => { // Arrange fetchMock.mockResolvedValueOnce( mockJsonResponse({ @@ -1092,6 +1092,7 @@ describe("AgentServerConversationService", () => { parent_conversation_id: "parent-conv-1", agent_type: "plan", sandbox_id: "sandbox-9", + trigger: "gui", }); }); diff --git a/__tests__/components/features/chat/skill-install-restart-banner.test.tsx b/__tests__/components/features/chat/skill-install-restart-banner.test.tsx index 9e4026c54c..b97cc2ac13 100644 --- a/__tests__/components/features/chat/skill-install-restart-banner.test.tsx +++ b/__tests__/components/features/chat/skill-install-restart-banner.test.tsx @@ -19,10 +19,14 @@ import type { // The restart mutation resolves the launch profile through these services // before creating the conversation; stub them so it deterministically takes -// the legacy agent_settings path (their absence is a supported fallback). +// the legacy agent_settings path (no active agent profile). vi.mock("#/api/agent-profiles-service/agent-profiles-service.api", () => ({ __esModule: true, - default: { listProfiles: vi.fn().mockRejectedValue(new Error("n/a")) }, + default: { + listProfiles: vi + .fn() + .mockResolvedValue({ profiles: [], active_agent_profile_id: null }), + }, WELL_KNOWN_DEFAULT_AGENT_PROFILE_NAME: "default", })); vi.mock("#/api/profiles-service/profiles-service.api", () => ({ diff --git a/__tests__/hooks/mutation/use-create-conversation.test.tsx b/__tests__/hooks/mutation/use-create-conversation.test.tsx index b0f30d1b4e..fd63adf2ac 100644 --- a/__tests__/hooks/mutation/use-create-conversation.test.tsx +++ b/__tests__/hooks/mutation/use-create-conversation.test.tsx @@ -200,7 +200,6 @@ describe("useCreateConversation", () => { await result.current.mutateAsync({ query: "hello" }); await waitFor(() => { - // sandboxId is never passed; the active profile id rides as agentProfileId. const call = createConversationSpy.mock.lastCall; expect(call?.[0]?.sandboxId).toBeUndefined(); expect(call?.[0]?.agentProfileId).toBe("profile-abc"); @@ -245,15 +244,14 @@ describe("useCreateConversation", () => { expect(call?.[0]?.agentProfileId).toBe("profile-late"); }); - it("falls back to the agent_settings launch when the profiles fetch fails", async () => { - listAgentProfilesMock.mockRejectedValue(new Error("not supported")); - const createConversationSpy = vi - .spyOn(AgentServerConversationService, "createConversation") - .mockResolvedValue({ - id: "task-id", - app_conversation_id: "conv-1", - agent_server_url: "http://agent-server.local", - } as never); + it("does not downgrade when the profiles fetch fails", async () => { + const profileError = new Error("profile endpoint unavailable"); + listAgentProfilesMock.mockRejectedValue(profileError); + const createConversationSpy = vi.spyOn( + AgentServerConversationService, + "createConversation", + ); + createConversationSpy.mockClear(); const { result } = renderHook(() => useCreateConversation(), { wrapper: ({ children }) => ( @@ -263,12 +261,10 @@ describe("useCreateConversation", () => { ), }); - // Resolves without stalling: the launch-path fetch is retry: false. - await result.current.mutateAsync({ query: "hello" }); - - // No profile tail — the create stays on the legacy agent_settings path. - const call = createConversationSpy.mock.lastCall; - expect(call?.[0]?.agentProfileId).toBeUndefined(); + await expect(result.current.mutateAsync({ query: "hello" })).rejects.toBe( + profileError, + ); + expect(createConversationSpy).not.toHaveBeenCalled(); }); it("invalidates the conversation list and start-tasks queries on success", async () => { diff --git a/src/api/agent-server-adapter.test.ts b/src/api/agent-server-adapter.test.ts index bfa5b3920a..c99f5f3722 100644 --- a/src/api/agent-server-adapter.test.ts +++ b/src/api/agent-server-adapter.test.ts @@ -3,7 +3,11 @@ import { CANVAS_UI_CLIENT_TOOL_NAME } from "#/constants/canvas-ui"; import { LAUNCH_CHILD_CONVERSATION_TOOL_NAME } from "#/constants/child-conversation"; import { DEFAULT_SETTINGS } from "#/services/settings"; import type { Settings } from "#/types/settings"; -import { buildStartConversationRequest } from "./agent-server-adapter"; +import { + AGENT_CANVAS_SOURCE, + CLIENT_SOURCE_TAG_KEY, + buildStartConversationRequest, +} from "./agent-server-adapter"; const encryptedValue = "gAAAAAencrypted-mcp-header"; @@ -210,12 +214,15 @@ describe("buildStartConversationRequest — agentProfileId path", () => { ).toBeDefined(); // ...but a profile launch resolves the server server-side, so the tag - // (which may not match the launched profile) is omitted. + // (which may not match the launched profile) is omitted while the client + // source telemetry tag is still stamped. const payload = buildStartConversationRequest({ settings: makeSettings(agentSettings), agentProfileId: "profile-xyz", }); - expect(payload.tags).toBeUndefined(); + expect(payload.tags).toEqual({ + [CLIENT_SOURCE_TAG_KEY]: AGENT_CANVAS_SOURCE, + }); }); it("suppresses secrets_encrypted when launching from a profile", () => { diff --git a/src/api/agent-server-adapter.ts b/src/api/agent-server-adapter.ts index 04da0998c1..ade417018a 100644 --- a/src/api/agent-server-adapter.ts +++ b/src/api/agent-server-adapter.ts @@ -433,6 +433,8 @@ type ConversationSettingsPayload = SettingsRecord & { }; export const ACP_SERVER_TAG_KEY = "acpserver"; +export const CLIENT_SOURCE_TAG_KEY = "clientsource"; +export const AGENT_CANVAS_SOURCE = "agentcanvas"; export const AUTOMATION_TRIGGER_TAG_KEY = "automationtrigger"; export const AUTOMATION_ID_TAG_KEY = "automationid"; @@ -456,6 +458,7 @@ export const AUTOMATION_TAG_KEYS: readonly string[] = [ * rows. Each is either already surfaced by a first-class UI source or is * internal routing data: * - ``acpserver`` → ACP provider chip + * - ``clientsource`` → telemetry attribution * - ``title`` → conversation card heading * - git / repo / branch / workspace stamps → repo-branch metadata + directory * footer / hovercard rows (``selected_repository``, ``selected_branch``, @@ -466,6 +469,7 @@ export const AUTOMATION_TAG_KEYS: readonly string[] = [ */ export const RESERVED_CONVERSATION_TAG_KEYS: ReadonlySet = new Set([ ACP_SERVER_TAG_KEY, + CLIENT_SOURCE_TAG_KEY, AUTOMATION_ID_TAG_KEY, AUTOMATION_RUN_ID_TAG_KEY, "title", @@ -1119,10 +1123,17 @@ export function buildStartConversationRequest( worktree: options.worktree ?? true, }; + // Stamp the client source tag so the agent-server can attribute the + // conversation to Canvas in telemetry (conversation_source = "canvas"). // A profile launch resolves the ACP server server-side, so don't stamp the // tag from current settings (it may not match the launched profile). if (!options.agentProfileId && acpServerTag) { - payload.tags = { [ACP_SERVER_TAG_KEY]: acpServerTag }; + payload.tags = { + [ACP_SERVER_TAG_KEY]: acpServerTag, + [CLIENT_SOURCE_TAG_KEY]: AGENT_CANVAS_SOURCE, + }; + } else { + payload.tags = { [CLIENT_SOURCE_TAG_KEY]: AGENT_CANVAS_SOURCE }; } // ``secrets_encrypted`` makes the agent-server decrypt request secrets at 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 f2366f0efc..507769cd4b 100644 --- a/src/api/conversation-service/agent-server-conversation-service.api.ts +++ b/src/api/conversation-service/agent-server-conversation-service.api.ts @@ -428,6 +428,7 @@ class AgentServerConversationService { agent_type: agentType, sandbox_id: sandboxId ?? null, agent_profile_id: agentProfileId ?? null, + trigger: "gui", }; return createCloudAppConversation(request); } diff --git a/src/hooks/mutation/use-create-conversation.ts b/src/hooks/mutation/use-create-conversation.ts index 4208bc3442..1cda1aee24 100644 --- a/src/hooks/mutation/use-create-conversation.ts +++ b/src/hooks/mutation/use-create-conversation.ts @@ -96,23 +96,18 @@ export const useCreateConversation = () => { // conversations (#3727), on both local and cloud (cloud gained // /api/agent-profiles in OpenHands #15060, #3730). Await the list from // the shared query cache: a send fired before the home query resolves - // must still launch from the active profile, not fall through to the - // agent_settings path. Degrades safely: if the fetch errors (older - // backend without the surface), this stays undefined and creation falls - // back to the encrypted agent_settings launch path. - let agentProfiles: AgentProfileListResponse | undefined; - try { - agentProfiles = await queryClient.ensureQueryData({ + // must still launch from the active profile. Do not fall back to the + // global agent_settings when profile discovery fails: activation is + // pointer-only, so those settings may describe a different agent. + const agentProfiles: AgentProfileListResponse = + await queryClient.ensureQueryData({ queryKey: [...AGENT_PROFILES_QUERY_KEYS.all, backend.id, orgId], queryFn: AgentProfilesService.listProfiles, ...AGENT_PROFILES_RETRY_OPTIONS, }); - } catch { - // Profiles unavailable → legacy agent_settings launch. - } const requestedAgentProfileId = - agentProfileId ?? agentProfiles?.active_agent_profile_id ?? undefined; + agentProfileId ?? agentProfiles.active_agent_profile_id ?? undefined; // Fall back to the legacy agent_settings launch when the resolved agent // profile can't resolve its LLM. The agent-server seeds a `default` diff --git a/src/mocks/agent-profiles-handlers.ts b/src/mocks/agent-profiles-handlers.ts new file mode 100644 index 0000000000..d98c9420ac --- /dev/null +++ b/src/mocks/agent-profiles-handlers.ts @@ -0,0 +1,154 @@ +import { http, HttpResponse } from "msw"; +import type { + AgentProfile, + AgentProfileSummary, +} from "@openhands/typescript-client"; + +/** + * In-memory agent-profile store for the mock agent-server API. Keyed by name + * (agent-server uses name-based lookups, not IDs). + * + * Imported as a live module so consumers can seed entries and reset between + * tests without re-registering handlers. + */ +const profiles = new Map(); + +let activeProfileId: string | null = null; + +/** Reset the in-memory store (called from test setup, never on main threads). */ +export function resetMockAgentProfiles(): void { + profiles.clear(); + activeProfileId = null; +} + +function toSummary(name: string, profile: AgentProfile): AgentProfileSummary { + return { + id: profile.id, + name, + agent_kind: profile.agent_kind, + revision: profile.revision, + llm_profile_ref: + profile.agent_kind === "openhands" ? profile.llm_profile_ref : null, + mcp_server_refs: profile.mcp_server_refs, + }; +} + +/** + * Mock handlers for the agent-server `/api/agent-profiles` endpoints (the same + * contract consumed by `AgentProfilesService` and the cloud proxy). Without + * these, `listProfiles`/`saveProfile` in non-mocked tests hit the real network + * and reject, which `useCreateConversation` now surfaces as a hard failure + * (no silent downgrade fallback — see PR #16523). + * + * Routes mirror `agent_profiles_router.py` in the agent-server. + */ +export const AGENT_PROFILES_HANDLERS = [ + // GET /api/agent-profiles - List all profiles + the active id. + http.get("*/api/agent-profiles", async ({ request }) => { + // Exclude requests carrying a :name segment (handled by the :name route). + const url = new URL(request.url); + const pathParts = url.pathname.split("/").filter(Boolean); + if (pathParts.length > 2) return undefined; + + const summaries = Array.from(profiles.entries()).map(([name, profile]) => + toSummary(name, profile), + ); + return HttpResponse.json({ + profiles: summaries, + active_agent_profile_id: activeProfileId, + }); + }), + + // GET /api/agent-profiles/:name - Fetch a single profile. + http.get("*/api/agent-profiles/:name", async ({ params }) => { + const { name } = params; + if (typeof name !== "string") { + return HttpResponse.json({ detail: "Invalid name" }, { status: 400 }); + } + const profile = profiles.get(name); + if (!profile) { + return HttpResponse.json( + { detail: `Agent profile '${name}' not found` }, + { status: 404 }, + ); + } + return HttpResponse.json({ name, profile }); + }), + + // POST /api/agent-profiles/:name - Create or overwrite a profile (upsert). + http.post("*/api/agent-profiles/:name", async ({ params, request }) => { + const { name } = params; + if (typeof name !== "string") { + return HttpResponse.json({ detail: "Invalid name" }, { status: 400 }); + } + const body = (await request.json()) as AgentProfile; + profiles.set(name, { ...body, id: body.id ?? name }); + return HttpResponse.json({ name, message: "Agent profile saved." }); + }), + + // DELETE /api/agent-profiles/:name - Delete by name (idempotent). + http.delete("*/api/agent-profiles/:name", async ({ params }) => { + const { name } = params; + if (typeof name !== "string") { + return HttpResponse.json({ detail: "Invalid name" }, { status: 400 }); + } + if (activeProfileId === profiles.get(name)?.id) { + activeProfileId = null; + } + profiles.delete(name); + return HttpResponse.json({ name, message: "Agent profile deleted." }); + }), + + // POST /api/agent-profiles/:name/rename - Rename a profile. + http.post( + "*/api/agent-profiles/:name/rename", + async ({ params, request }) => { + const { name } = params; + if (typeof name !== "string") { + return HttpResponse.json({ detail: "Invalid name" }, { status: 400 }); + } + const body = (await request.json()) as { new_name?: string } | null; + const newName = body?.new_name; + if (!newName) { + return HttpResponse.json( + { detail: "new_name is required" }, + { status: 422 }, + ); + } + const profile = profiles.get(name); + if (!profile) { + return HttpResponse.json( + { detail: `Agent profile '${name}' not found` }, + { status: 404 }, + ); + } + profiles.delete(name); + profiles.set(newName, profile); + return HttpResponse.json({ + name: newName, + message: "Agent profile renamed.", + }); + }, + ), + + // POST /api/agent-profiles/:id/activate - Activate by stable UUID (pointer-only). + http.post("*/api/agent-profiles/:id/activate", async ({ params }) => { + const { id } = params; + if (typeof id !== "string") { + return HttpResponse.json({ detail: "Invalid id" }, { status: 400 }); + } + const profile = Array.from(profiles.values()).find((p) => p.id === id); + if (!profile) { + return HttpResponse.json( + { detail: `Agent profile '${id}' not found` }, + { status: 404 }, + ); + } + activeProfileId = id; + return HttpResponse.json({ + id, + message: "Agent profile activated.", + agent_settings_applied: false, + }); + }), +]; diff --git a/src/mocks/handlers.ts b/src/mocks/handlers.ts index efcba4f0a6..1267556dd8 100644 --- a/src/mocks/handlers.ts +++ b/src/mocks/handlers.ts @@ -1,5 +1,9 @@ import { FILE_SERVICE_HANDLERS } from "./file-service-handlers"; import { SECRETS_HANDLERS } from "./secrets-handlers"; +import { + AGENT_PROFILES_HANDLERS, + resetMockAgentProfiles, +} from "./agent-profiles-handlers"; import { GIT_REPOSITORY_HANDLERS } from "./git-repository-handlers"; import { SETTINGS_HANDLERS, @@ -23,6 +27,7 @@ import { export const handlers = [ ...FILE_SERVICE_HANDLERS, ...SECRETS_HANDLERS, + ...AGENT_PROFILES_HANDLERS, ...GIT_REPOSITORY_HANDLERS, ...SETTINGS_HANDLERS, ...CONVERSATION_HANDLERS, @@ -40,3 +45,5 @@ export { resetAutomationMockData, resetMockWorkspaces, }; + +export { AGENT_PROFILES_HANDLERS, resetMockAgentProfiles }; diff --git a/vitest.setup.ts b/vitest.setup.ts index ab7e422a4e..79567b14a2 100644 --- a/vitest.setup.ts +++ b/vitest.setup.ts @@ -74,9 +74,12 @@ if (typeof requestAnimationFrame === "undefined") { // `afterAll` (which runs *before* jsdom teardown) so they settle while // `ProgressEvent` is still defined. See `afterAll` below. The getter below // stashes the live class as a light defense-in-depth for any callback that -// fires before teardown completes; it cannot help after teardown (the -// accessor is deleted there), which is exactly why the `afterAll` drain is -// the real fix. +// fires before teardown completes; it cannot help after teardown because +// Vitest deletes the accessor (part of its `LIVING_KEYS`) during per-file +// teardown — which is exactly why the `afterAll` drain is the real fix. +// `configurable: true` is required so Vitest *can* delete the accessor +// during teardown; `configurable: false` would prevent cleanup and leave +// a stale getter pointing at a torn-down jsdom environment. class MockProgressEvent extends Event { readonly lengthComputable: boolean;