fix: prevent silent agent profile downgrade (#16523)

Co-authored-by: neubig <neubig@users.noreply.github.com>
Co-authored-by: openhands <openhands@all-hands.dev>

__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)", () => {

__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",
       });
     });
 

__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", () => ({

__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 () => {

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", () => {

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<string> = 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

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);
     }

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`

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<string, AgentProfile>();
+
+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,
+    });
+  }),
+];

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 };

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;