fix(agents): keep the provider you picked across an auth-method change

src/components/settings/acp-agent-settings.provider-rebind.test.tsx

@@ -0,0 +1,206 @@
+/**
+ * Leaving "model_provider" auth mode drops the draft's provider binding (a save
+ * in another mode must not persist one), so coming back falls into the
+ * auto-select. That auto-select used to take the head of the provider list —
+ * the OLDEST provider, since the list is ordered by row id — and the rebind is
+ * provider-authoritative: it rewrites the model fields, the env text and the
+ * config text from whichever provider it lands on. These drive the real panel
+ * through that round trip and pin where it lands: the user's own last pick,
+ * then the binding saved on the agent, and only then the head.
+ */
+import { render, screen, waitFor, within } from "@testing-library/react"
+import userEvent from "@testing-library/user-event"
+import { NextIntlClientProvider } from "next-intl"
+import { beforeEach, describe, expect, it, vi } from "vitest"
+
+import enMessages from "@/i18n/messages/en.json"
+import {
+  acpListAgents,
+  acpUpdateAgentConfig,
+  acpUpdateAgentEnv,
+  listModelProviders,
+} from "@/lib/api"
+import type { AcpAgentInfo, ModelProviderInfo } from "@/lib/types"
+
+import { AcpAgentSettings } from "./acp-agent-settings"
+
+vi.mock("@/lib/api", async (importOriginal) => {
+  const actual = await importOriginal<Record<string, unknown>>()
+  // Every function is stubbed so nothing here can reach the real transport;
+  // the calls the panel makes on mount that these tests don't need (preflight,
+  // catalogs) simply never settle.
+  return Object.fromEntries(
+    Object.entries(actual).map(([name, value]) => [
+      name,
+      typeof value === "function" ? vi.fn(() => new Promise(() => {})) : value,
+    ])
+  )
+})
+vi.mock("next/navigation", () => {
+  const params = new URLSearchParams()
+  return { useSearchParams: () => params }
+})
+
+function claudeAgent(overrides: Partial<AcpAgentInfo> = {}): AcpAgentInfo {
+  return {
+    agent_type: "claude_code",
+    skills_capable: true,
+    registry_id: "claude-code",
+    registry_version: "1.0.0",
+    supports_custom_version: false,
+    name: "Claude Code",
+    description: "",
+    available: true,
+    distribution_type: "npx",
+    is_acp_adapter: true,
+    custom_source: null,
+    enabled: true,
+    sort_order: 0,
+    installed_version: null,
+    host_tools_agent_mode: false,
+    env: {},
+    config_json: null,
+    config_file_path: null,
+    opencode_auth_json: null,
+    codex_auth_json: null,
+    cline_secrets_json: null,
+    codex_config_toml: null,
+    codex_model_catalog: null,
+    codex_sandbox_settings: null,
+    grok_config_toml: null,
+    grok_settings: null,
+    hermes_config_yaml: null,
+    cursor_cli_config_json: null,
+    cursor_settings: null,
+    model_provider_id: null,
+    icon_url: null,
+    ...overrides,
+  }
+}
+
+function provider(id: number, letter: string): ModelProviderInfo {
+  return {
+    id,
+    name: `Provider ${letter}`,
+    api_url: `https://gateway-${letter.toLowerCase()}.test`,
+    api_key: `key-${letter.toLowerCase()}`,
+    api_key_masked: "",
+    agent_type: "claude_code",
+    model: JSON.stringify({ main: `model-${letter.toLowerCase()}` }),
+    created_at: "",
+    updated_at: "",
+  }
+}
+
+// Row-id order, as `list_all` returns them: A is the oldest, so the head.
+const PROVIDERS = [provider(1, "A"), provider(2, "B"), provider(3, "C")]
+
+function renderPanel() {
+  return render(
+    <NextIntlClientProvider locale="en" messages={enMessages}>
+      <AcpAgentSettings />
+    </NextIntlClientProvider>
+  )
+}
+
+/** The select that sits under a visible `<label>` in the config card. */
+function selectUnder(label: string): HTMLElement {
+  const labelEl = screen.getByText(label, { selector: "label" })
+  return within(labelEl.parentElement as HTMLElement).getByRole("combobox")
+}
+
+async function choose(
+  user: ReturnType<typeof userEvent.setup>,
+  label: string,
+  option: string
+) {
+  await user.click(selectUnder(label))
+  const list = await screen.findByRole("listbox")
+  await user.click(within(list).getByRole("option", { name: option }))
+}
+
+async function expectBoundTo(letter: string) {
+  await waitFor(() =>
+    expect(selectUnder("Select Model Provider")).toHaveTextContent(
+      `Provider ${letter}`
+    )
+  )
+  const labelEl = screen.getByText("Native JSON Config", { selector: "label" })
+  const config = within(labelEl.parentElement as HTMLElement).getByRole(
+    "textbox"
+  ) as HTMLTextAreaElement
+  expect(JSON.parse(config.value).env.ANTHROPIC_MODEL).toBe(
+    `model-${letter.toLowerCase()}`
+  )
+}
+
+async function openPanel(agent: AcpAgentInfo) {
+  vi.mocked(acpListAgents).mockResolvedValue([agent])
+  renderPanel()
+  await screen.findByText("Native JSON Config", { selector: "label" })
+}
+
+describe("AcpAgentSettings — provider rebind across an auth-mode round trip", () => {
+  beforeEach(() => {
+    vi.mocked(listModelProviders).mockResolvedValue(PROVIDERS)
+    vi.mocked(acpUpdateAgentEnv).mockResolvedValue(0)
+    vi.mocked(acpUpdateAgentConfig).mockResolvedValue(0)
+  })
+
+  it("returns to an unsaved pick, and Save persists that provider", async () => {
+    const user = userEvent.setup()
+    await openPanel(claudeAgent())
+
+    // A first-time pick has nothing to return to, so it takes the head.
+    await choose(user, "Auth Mode", "Model Provider")
+    await expectBoundTo("A")
+    await choose(user, "Select Model Provider", "Provider B")
+    await expectBoundTo("B")
+
+    await choose(user, "Auth Mode", "Custom Endpoint")
+    await choose(user, "Auth Mode", "Model Provider")
+    await expectBoundTo("B")
+
+    await user.click(
+      screen.getByRole("button", { name: "Save Config Management" })
+    )
+    await waitFor(() => expect(acpUpdateAgentConfig).toHaveBeenCalledTimes(1))
+    expect(acpUpdateAgentEnv).toHaveBeenCalledTimes(1)
+    expect(acpUpdateAgentEnv).toHaveBeenCalledWith(
+      "claude_code",
+      expect.objectContaining({
+        modelProviderId: 2,
+        env: expect.objectContaining({ ANTHROPIC_MODEL: "model-b" }),
+      })
+    )
+    const [, configPatch] = vi.mocked(acpUpdateAgentConfig).mock.calls[0]
+    expect(
+      JSON.parse(configPatch.config_json ?? "{}").env.ANTHROPIC_MODEL
+    ).toBe("model-b")
+  })
+
+  it("returns to the saved binding when nothing was picked", async () => {
+    const user = userEvent.setup()
+    await openPanel(claudeAgent({ model_provider_id: 2 }))
+    await waitFor(() =>
+      expect(selectUnder("Select Model Provider")).toHaveTextContent(
+        "Provider B"
+      )
+    )
+
+    await choose(user, "Auth Mode", "Official Subscription")
+    await choose(user, "Auth Mode", "Model Provider")
+    await expectBoundTo("B")
+  })
+
+  it("prefers the last pick over the saved binding", async () => {
+    const user = userEvent.setup()
+    await openPanel(claudeAgent({ model_provider_id: 2 }))
+    await choose(user, "Select Model Provider", "Provider C")
+    await expectBoundTo("C")
+
+    await choose(user, "Auth Mode", "Custom Endpoint")
+    await choose(user, "Auth Mode", "Model Provider")
+    await expectBoundTo("C")
+  })
+})

src/components/settings/acp-agent-settings.test.tsx

@@ -23,6 +23,7 @@ import {
   patchCodexConfigTomlText,
   patchEnvByImportantKey,
   patchImportantConfigText,
+  providerToRebindTo,
   codexSandboxSeedsAcpPreset,
   rebaseDeepSeekDraft,
   setAdapterChannel,
@@ -35,6 +36,7 @@ import type {
   AcpAgentInfo,
   AdapterInfo,
   AgentType,
+  ModelProviderInfo,
   PreflightResult,
 } from "@/lib/types"
 
@@ -148,6 +150,56 @@ function codexSandboxDraft(
   }
 }
 
+// Providers arrive ordered by row id, so falling straight to the head of the
+// list rebound the agent to its OLDEST provider whenever the auth-mode dropdown
+// round-tripped through another mode, and the rebind copies that provider's
+// model names over the one the user was on. The rendered round trip is pinned
+// in acp-agent-settings.provider-rebind.test.tsx.
+describe("providerToRebindTo", () => {
+  function provider(id: number): ModelProviderInfo {
+    return {
+      id,
+      name: `provider-${id}`,
+      api_url: "",
+      api_key: "",
+      api_key_masked: "",
+      agent_type: "claude_code",
+      model: null,
+      created_at: "",
+      updated_at: "",
+    }
+  }
+  const available = [provider(1), provider(2), provider(3)]
+
+  it("returns to the user's last pick, not the head", () => {
+    expect(providerToRebindTo(available, 2, null)?.id).toBe(2)
+  })
+
+  it("prefers the last pick over the binding saved on the agent", () => {
+    expect(providerToRebindTo(available, 3, 2)?.id).toBe(3)
+  })
+
+  it("returns to the saved binding when nothing was picked in the panel", () => {
+    expect(providerToRebindTo(available, undefined, 2)?.id).toBe(2)
+  })
+
+  it("skips a last pick that is gone and returns to the saved binding", () => {
+    expect(providerToRebindTo(available, 9, 2)?.id).toBe(2)
+  })
+
+  it("falls back to the head for a first-time pick", () => {
+    expect(providerToRebindTo(available, undefined, null)?.id).toBe(1)
+  })
+
+  it("falls back to the head when every candidate is gone", () => {
+    expect(providerToRebindTo([provider(3), provider(4)], 2, 1)?.id).toBe(3)
+  })
+
+  it("has nothing to bind to when no provider exists", () => {
+    expect(providerToRebindTo([], 2, 2)).toBeNull()
+  })
+})
+
 describe("buildCodexSandboxConfig — Codex sandbox/approval save patch", () => {
   // The core contract. The panel also sends the raw config.toml text and the
   // backend applies this patch last, so a field the user did not move must not

src/components/settings/acp-agent-settings.tsx

@@ -3870,6 +3870,31 @@ export function buildAcpAdapterCheck(
   }
 }
 
+/**
+ * Which provider an agent should land on when it returns to "model_provider"
+ * auth mode with no binding in the draft.
+ *
+ * In order: `lastPick`, the user's own last pick for this agent in this panel;
+ * `savedBinding`, the provider the agent is bound to on disk; then the head of
+ * the list. Each candidate counts only while it is still listed (not deleted,
+ * not moved to another agent), so a stale pick falls through to a saved
+ * binding that is still good rather than straight to the head. Going straight
+ * to `available[0]` rebound the agent to its OLDEST provider (the list arrives
+ * ordered by row id) whenever the auth-mode dropdown round-tripped through
+ * another mode, and the rebind copies that provider's model names into the
+ * draft, the env text and the config text. The head stays the fallback for a
+ * first-time pick.
+ */
+export function providerToRebindTo(
+  available: readonly ModelProviderInfo[],
+  lastPick: number | null | undefined,
+  savedBinding: number | null | undefined
+): ModelProviderInfo | null {
+  const listed = (id: number | null | undefined) =>
+    id == null ? undefined : available.find((provider) => provider.id === id)
+  return listed(lastPick) ?? listed(savedBinding) ?? available[0] ?? null
+}
+
 // `uvReady` reports whether the uv runtime (uvx) is installed — only meaningful
 // for uvx agents (custom Python-package agents; built-in Hermes moved to the
 // npm bridge). Derived from the uv preflight check by the caller. uvx agents
@@ -5471,6 +5496,14 @@ export function AcpAgentSettings() {
     )
   }, [modelProviders, selectedAgent])
 
+  // The provider each agent was last bound to, remembered across auth-mode
+  // changes. The auth-mode handlers drop `draft.modelProviderId` whenever the
+  // mode leaves "model_provider", so that a save in another mode cannot
+  // persist a binding. Without this memory, coming BACK to provider mode falls
+  // through to the auto-select below, which had no record of the user's own
+  // choice. See `providerToRebindTo`.
+  const lastBoundProviderRef = useRef<Partial<Record<AgentType, number>>>({})
+
   const selectedNeedsModelProvider = useMemo(() => {
     if (!selectedDraft) return false
     if (!selectedAgent) return false
@@ -5902,6 +5935,9 @@ export function AcpAgentSettings() {
     (providerIdStr: string) => {
       if (!selectedAgent || !selectedDraft) return
       const providerId = providerIdStr ? Number(providerIdStr) : null
+      if (providerId != null) {
+        lastBoundProviderRef.current[selectedAgent.agent_type] = providerId
+      }
       const provider = providerId
         ? modelProviders.find((p) => p.id === providerId)
         : null
@@ -6113,15 +6149,26 @@ export function AcpAgentSettings() {
     [selectedAgent, selectedDraft, modelProviders, updateSelectedDraft]
   )
 
-  // Auto-select the first available provider when the user switches an agent to
-  // "model_provider" auth mode and hasn't picked one yet. If the list is empty,
-  // the existing "noModelProviderAvailable" hint handles the empty state.
+  // Auto-select a provider when the user switches an agent to
+  // "model_provider" auth mode and the draft holds no binding. If the list is
+  // empty, the existing "noModelProviderAvailable" hint handles the empty
+  // state. The user's own last pick (then the saved binding) wins over the head
+  // of the list, because an auth-mode round trip lands here too and rebinding
+  // to whichever provider happens to be first copies ITS model names over the
+  // one the user was actually on.
   useEffect(() => {
     if (!selectedNeedsModelProvider) return
     if (selectedDraft?.modelProviderId != null) return
-    if (selectedModelProviders.length === 0) return
-    handleModelProviderSelect(String(selectedModelProviders[0].id))
+    if (!selectedAgent) return
+    const target = providerToRebindTo(
+      selectedModelProviders,
+      lastBoundProviderRef.current[selectedAgent.agent_type],
+      selectedAgent.model_provider_id
+    )
+    if (!target) return
+    handleModelProviderSelect(String(target.id))
   }, [
+    selectedAgent,
     selectedNeedsModelProvider,
     selectedDraft?.modelProviderId,
     selectedModelProviders,