From 3905369bd59a58e8c6be19528506bbb7022d018d Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Fri, 19 Jun 2026 12:31:52 +0200 Subject: [PATCH 1/3] feat: add agent selection as ACP session config option Expose user/plugin/project-configured Claude Code agents as a selectable config option, rendered alongside Mode/Model/Effort. Unlike the original prototype (#653), switching agents is a live operation: `query.applyFlagSettings({ agent })` (or `agent: null` to reset) takes effect on the next turn, so no subprocess restart or session resume is needed (requires claude-agent-sdk >= 0.3.161). `supportedAgents()` always returns Claude Code's built-in subagents (Explore, Plan, etc.) even when the user has configured none of their own. Those aren't meaningful main-thread personas, so we filter them out and only surface the picker when at least one custom agent is configured. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/acp-agent.ts | 78 +++++++++++++++++- src/tests/acp-agent.test.ts | 157 ++++++++++++++++++++++++++++++++++++ 2 files changed, 234 insertions(+), 1 deletion(-) diff --git a/src/acp-agent.ts b/src/acp-agent.ts index d2f13aff..282d4d1c 100644 --- a/src/acp-agent.ts +++ b/src/acp-agent.ts @@ -47,6 +47,7 @@ import { StopReason, } from "@agentclientprotocol/sdk"; import { + AgentInfo, CanUseTool, deleteSession, getSessionMessages, @@ -231,6 +232,14 @@ type Session = { models: SessionModelState; modelInfos: ModelInfo[]; configOptions: SessionConfigOption[]; + /** Custom main-thread agent personas the user (or a plugin/project) has + * configured, discovered via `supportedAgents()` with Claude Code's built-in + * subagents filtered out. Empty when none are configured, in which case the + * "agent" config option is omitted entirely. */ + agents: AgentInfo[]; + /** The currently selected main-thread agent name, or "default" for the + * standard Claude Code agent (no `agent` flag applied). */ + currentAgent: string; abortController: AbortController; /** Signal the consumer races `query.next()` against. Aborted by cancel() * (after a grace period) to force the active turn to settle "cancelled" when @@ -2692,6 +2701,8 @@ export class ClaudeAcpAgent { session.models, session.modelInfos, currentEffort, + session.agents, + session.currentAgent, ); // Sync effort with the SDK if it changed after the model switch @@ -2727,6 +2738,14 @@ export class ClaudeAcpAgent { await session.query.applyFlagSettings({ effortLevel: toSdkEffortLevel(value), }); + } else if (configId === "agent") { + session.currentAgent = value; + // Live agent switch — no subprocess restart needed. Passing `null` + // clears the flag layer back to the standard Claude Code agent; the + // change takes effect on the next turn (SDK >= 0.3.161). + await session.query.applyFlagSettings({ + agent: value === "default" ? null : value, + }); } } } @@ -3141,11 +3160,16 @@ export class ClaudeAcpAgent { availableModes, }; + const agents = await discoverCustomAgents(q); + const currentAgent = userProvidedOptions?.agent ?? "default"; + const configOptions = buildConfigOptions( modes, models, allowedModels, settingsManager.getSettings().effortLevel, + agents, + currentAgent, ); // Apply the initial effort level to the SDK so it matches the UI default @@ -3176,6 +3200,8 @@ export class ClaudeAcpAgent { models, modelInfos: allowedModels, configOptions, + agents, + currentAgent, abortController, emitRawSDKMessages: sessionMeta?.claudeCode?.emitRawSDKMessages ?? false, contextWindowSize: @@ -3357,11 +3383,39 @@ function toSdkEffortLevel(value: string | undefined): Settings["effortLevel"] | return value === undefined || value === "default" ? null : (value as Settings["effortLevel"]); } -function buildConfigOptions( +// `supportedAgents()` always returns Claude Code's built-in subagents — the +// ones used for Task-tool delegation (Explore, Plan, etc.) — even when the user +// has configured none of their own. Those aren't meaningful *main-thread* +// personas, so we filter them out and only surface the Agent picker when the +// user (or a plugin/project) has configured custom agents. Update this set if +// the SDK's built-in roster changes. +const BUILTIN_AGENT_NAMES = new Set([ + "claude", + "general-purpose", + "Explore", + "Plan", + "statusline-setup", +]); + +/** Discover user/plugin/project-configured main-thread agents, excluding the + * built-in subagents. Returns an empty list if discovery fails so a flaky + * control request never blocks session creation. */ +export async function discoverCustomAgents(q: Query): Promise { + try { + const agents = await q.supportedAgents(); + return agents.filter((a) => !BUILTIN_AGENT_NAMES.has(a.name)); + } catch { + return []; + } +} + +export function buildConfigOptions( modes: SessionModeState, models: SessionModelState, modelInfos: ModelInfo[], currentEffortLevel?: string, + agents: AgentInfo[] = [], + currentAgent: string = "default", ): SessionConfigOption[] { const options: SessionConfigOption[] = [ { @@ -3425,6 +3479,28 @@ function buildConfigOptions( }); } + // Only surface the Agent picker when there's a real choice — i.e. the user + // has configured at least one custom agent (built-ins are filtered out in + // discoverCustomAgents). With none configured, "Default" would be the only + // entry, so we omit the option entirely. + if (agents.length > 0) { + options.push({ + id: "agent", + name: "Agent", + description: "Main-thread agent persona", + type: "select", + currentValue: currentAgent, + options: [ + { value: "default", name: "Default", description: "Standard Claude Code agent" }, + ...agents.map((a) => ({ + value: a.name, + name: a.name, + description: a.description || undefined, + })), + ], + }); + } + return options; } diff --git a/src/tests/acp-agent.test.ts b/src/tests/acp-agent.test.ts index bd748d38..d235df63 100644 --- a/src/tests/acp-agent.test.ts +++ b/src/tests/acp-agent.test.ts @@ -36,6 +36,8 @@ import { describeAlwaysAllow, streamEventToAcpNotifications, messageIdForGrouping, + buildConfigOptions, + discoverCustomAgents, type AcpClient, type SDKMessageFilter, } from "../acp-agent.js"; @@ -106,6 +108,8 @@ function mockSessionState(overrides: Record = {}) { cachedWriteTokens: 0, }, configOptions: [], + agents: [], + currentAgent: "default", abortController: new AbortController(), emitRawSDKMessages: false, contextWindowSize: 200000, @@ -2310,6 +2314,8 @@ describe("session/close", () => { cachedWriteTokens: 0, }, configOptions: [], + agents: [], + currentAgent: "default", abortController: new AbortController(), emitRawSDKMessages: false, contextWindowSize: 200000, @@ -2393,6 +2399,8 @@ describe("session/delete", () => { cachedWriteTokens: 0, }, configOptions: [], + agents: [], + currentAgent: "default", abortController: new AbortController(), emitRawSDKMessages: false, contextWindowSize: 200000, @@ -2493,6 +2501,8 @@ describe("getOrCreateSession param change detection", () => { cachedWriteTokens: 0, }, configOptions: [], + agents: [], + currentAgent: "default", abortController: new AbortController(), emitRawSDKMessages: false, contextWindowSize: 200000, @@ -4557,6 +4567,8 @@ describe("post-error recovery", () => { cachedWriteTokens: 0, }, configOptions: [], + agents: [], + currentAgent: "default", abortController: new AbortController(), emitRawSDKMessages: false, contextWindowSize: 200000, @@ -5125,6 +5137,8 @@ describe("session/cancel wedge recovery (issue #680)", () => { cachedWriteTokens: 0, }, configOptions: [], + agents: [], + currentAgent: "default", abortController: new AbortController(), emitRawSDKMessages: false, contextWindowSize: 200000, @@ -5470,3 +5484,146 @@ describe("messageIdForGrouping", () => { expect(messageIdForGrouping({ type: "assistant", uuid: "", message: {} })).toBeUndefined(); }); }); + +describe("agent selection config option", () => { + const baseModes = { currentModeId: "default", availableModes: [] }; + const baseModels = { currentModelId: "default", availableModels: [] }; + + describe("discoverCustomAgents", () => { + it("filters out Claude Code's built-in subagents", async () => { + const q = { + supportedAgents: async () => [ + { name: "claude", description: "catch-all" }, + { name: "Explore", description: "search" }, + { name: "general-purpose", description: "gp" }, + { name: "Plan", description: "architect" }, + { name: "statusline-setup", description: "status" }, + { name: "my-reviewer", description: "Reviews code" }, + { name: "my-writer", description: "Writes docs" }, + ], + } as any; + const agents = await discoverCustomAgents(q); + expect(agents.map((a) => a.name)).toEqual(["my-reviewer", "my-writer"]); + }); + + it("returns an empty list when discovery throws", async () => { + const q = { + supportedAgents: async () => { + throw new Error("control request failed"); + }, + } as any; + expect(await discoverCustomAgents(q)).toEqual([]); + }); + }); + + describe("buildConfigOptions agent option", () => { + it("omits the agent option when no custom agents are configured", () => { + const options = buildConfigOptions(baseModes, baseModels, [], undefined, [], "default"); + expect(options.find((o) => o.id === "agent")).toBeUndefined(); + }); + + it("adds an agent option with a synthetic Default entry when custom agents exist", () => { + const agents = [ + { name: "my-reviewer", description: "Reviews code" }, + // empty description should normalize to undefined, not "" + { name: "my-writer", description: "" }, + ]; + const options = buildConfigOptions( + baseModes, + baseModels, + [], + undefined, + agents, + "my-reviewer", + ); + const agentOption = options.find((o) => o.id === "agent"); + expect(agentOption).toBeDefined(); + expect(agentOption!.currentValue).toBe("my-reviewer"); + expect(agentOption!.type).toBe("select"); + const entries = (agentOption as any).options; + expect(entries.map((o: any) => o.value)).toEqual(["default", "my-reviewer", "my-writer"]); + expect(entries[2].description).toBeUndefined(); + }); + }); + + describe("switching the agent", () => { + function createMockAgent() { + const mockClient = { sessionUpdate: async () => {} } as unknown as AcpClient; + return new ClaudeAcpAgent(mockClient, { log: () => {}, error: () => {} }); + } + + const agents = [{ name: "my-reviewer", description: "Reviews code" }]; + + function injectSession(agent: ClaudeAcpAgent, sessionId: string) { + function* empty() {} + const applyFlagSettings = vi.fn(async () => {}); + const gen = Object.assign(empty(), { + interrupt: vi.fn(), + close: vi.fn(), + applyFlagSettings, + }); + agent.sessions[sessionId] = { + query: gen as any, + input: new Pushable(), + cancelled: false, + cwd: "/test", + sessionFingerprint: JSON.stringify({ cwd: "/test", mcpServers: [] }), + modes: { currentModeId: "default", availableModes: [] }, + models: { currentModelId: "default", availableModels: [] }, + modelInfos: [], + settingsManager: { dispose: vi.fn() } as any, + accumulatedUsage: { + inputTokens: 0, + outputTokens: 0, + cachedReadTokens: 0, + cachedWriteTokens: 0, + }, + configOptions: buildConfigOptions(baseModes, baseModels, [], undefined, agents, "default"), + agents, + currentAgent: "default", + abortController: new AbortController(), + emitRawSDKMessages: false, + contextWindowSize: 200000, + taskState: new Map(), + toolUseCache: {}, + messageIdToUuid: new Map(), + }; + return { session: agent.sessions[sessionId]!, applyFlagSettings }; + } + + it("applies the agent flag live without restarting the subprocess", async () => { + const agent = createMockAgent(); + const { session, applyFlagSettings } = injectSession(agent, "s1"); + + const result = await agent.setSessionConfigOption({ + sessionId: "s1", + configId: "agent", + value: "my-reviewer", + }); + + expect(applyFlagSettings).toHaveBeenCalledWith({ agent: "my-reviewer" }); + expect(session.currentAgent).toBe("my-reviewer"); + // The whole point of the SDK >= 0.3.161 approach: no process teardown. + expect(session.query.interrupt).not.toHaveBeenCalled(); + expect(session.abortController.signal.aborted).toBe(false); + expect(agent.sessions["s1"]).toBe(session); + const agentOption = result.configOptions.find((o) => o.id === "agent"); + expect(agentOption?.currentValue).toBe("my-reviewer"); + }); + + it("clears the flag (agent: null) when switching back to default", async () => { + const agent = createMockAgent(); + const { session, applyFlagSettings } = injectSession(agent, "s2"); + session.currentAgent = "my-reviewer"; + + await agent.setSessionConfigOption({ + sessionId: "s2", + configId: "agent", + value: "default", + }); + + expect(applyFlagSettings).toHaveBeenCalledWith({ agent: null }); + expect(session.currentAgent).toBe("default"); + }); + }); +}); From c17baea5ee5b4e2c040ef410c3ee0af99fcd41f3 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Fri, 19 Jun 2026 12:39:29 +0200 Subject: [PATCH 2/3] fix: harden agent selection edge cases + SDK roster guard Address two state-consistency edge cases from review: - Apply the SDK `agent` flag before mutating `currentAgent`/configOptions so a rejected control request can't leave the UI claiming an agent the SDK isn't running. - Only adopt a caller-provided `agent` as the selected value when it's one we actually surface; a built-in (filtered out) or unknown name would point the config option's currentValue at an entry missing from its own options list. Add an integration test (RUN_INTEGRATION_TESTS) that runs the real SDK and asserts BUILTIN_AGENT_NAMES exactly matches the default agent roster, so the hardcoded set is flagged if the SDK changes its built-ins. Plus a unit test covering the rejected-switch no-desync path. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/acp-agent.ts | 32 +++++++++++++++-------- src/tests/acp-agent.test.ts | 20 +++++++++++++++ src/tests/agent-discovery.test.ts | 42 +++++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 10 deletions(-) create mode 100644 src/tests/agent-discovery.test.ts diff --git a/src/acp-agent.ts b/src/acp-agent.ts index 282d4d1c..1764bcc9 100644 --- a/src/acp-agent.ts +++ b/src/acp-agent.ts @@ -2730,6 +2730,19 @@ export class ClaudeAcpAgent { }, }); } + } else if (configId === "agent") { + // Live agent switch — no subprocess restart needed. Apply the SDK flag + // first so a rejected control request leaves both `currentAgent` and the + // config option untouched (no UI/SDK desync). Passing `null` clears the + // flag layer back to the standard Claude Code agent; the change takes + // effect on the next turn (SDK >= 0.3.161). + await session.query.applyFlagSettings({ + agent: value === "default" ? null : value, + }); + session.currentAgent = value; + session.configOptions = session.configOptions.map((o) => + o.id === configId && typeof o.currentValue === "string" ? { ...o, currentValue: value } : o, + ); } else { session.configOptions = session.configOptions.map((o) => o.id === configId && typeof o.currentValue === "string" ? { ...o, currentValue: value } : o, @@ -2738,14 +2751,6 @@ export class ClaudeAcpAgent { await session.query.applyFlagSettings({ effortLevel: toSdkEffortLevel(value), }); - } else if (configId === "agent") { - session.currentAgent = value; - // Live agent switch — no subprocess restart needed. Passing `null` - // clears the flag layer back to the standard Claude Code agent; the - // change takes effect on the next turn (SDK >= 0.3.161). - await session.query.applyFlagSettings({ - agent: value === "default" ? null : value, - }); } } } @@ -3161,7 +3166,14 @@ export class ClaudeAcpAgent { }; const agents = await discoverCustomAgents(q); - const currentAgent = userProvidedOptions?.agent ?? "default"; + // Only adopt the requested agent as the selected value if it's one we + // actually surface in the picker. A built-in (filtered out above) or + // otherwise-unknown name would leave the config option's `currentValue` + // pointing at an entry not in its own `options` list, which clients render + // as a blank/invalid selection. + const requestedAgent = userProvidedOptions?.agent; + const currentAgent = + requestedAgent && agents.some((a) => a.name === requestedAgent) ? requestedAgent : "default"; const configOptions = buildConfigOptions( modes, @@ -3389,7 +3401,7 @@ function toSdkEffortLevel(value: string | undefined): Settings["effortLevel"] | // personas, so we filter them out and only surface the Agent picker when the // user (or a plugin/project) has configured custom agents. Update this set if // the SDK's built-in roster changes. -const BUILTIN_AGENT_NAMES = new Set([ +export const BUILTIN_AGENT_NAMES = new Set([ "claude", "general-purpose", "Explore", diff --git a/src/tests/acp-agent.test.ts b/src/tests/acp-agent.test.ts index d235df63..d14baa49 100644 --- a/src/tests/acp-agent.test.ts +++ b/src/tests/acp-agent.test.ts @@ -5625,5 +5625,25 @@ describe("agent selection config option", () => { expect(applyFlagSettings).toHaveBeenCalledWith({ agent: null }); expect(session.currentAgent).toBe("default"); }); + + it("leaves tracked state untouched when the live switch is rejected", async () => { + const agent = createMockAgent(); + const { session, applyFlagSettings } = injectSession(agent, "s3"); + applyFlagSettings.mockRejectedValueOnce(new Error("control channel closed")); + + await expect( + agent.setSessionConfigOption({ + sessionId: "s3", + configId: "agent", + value: "my-reviewer", + }), + ).rejects.toThrow("control channel closed"); + + // The flag never applied, so neither currentAgent nor the config option + // moves — no desync with the agent the SDK is actually running. + expect(session.currentAgent).toBe("default"); + const agentOption = session.configOptions.find((o) => o.id === "agent"); + expect(agentOption?.currentValue).toBe("default"); + }); }); }); diff --git a/src/tests/agent-discovery.test.ts b/src/tests/agent-discovery.test.ts new file mode 100644 index 00000000..551b2ebb --- /dev/null +++ b/src/tests/agent-discovery.test.ts @@ -0,0 +1,42 @@ +import { describe, it, expect } from "vitest"; +import { randomUUID } from "crypto"; +import { query } from "@anthropic-ai/claude-agent-sdk"; +import type { SDKUserMessage } from "@anthropic-ai/claude-agent-sdk"; +import { Pushable } from "../utils.js"; +import { discoverCustomAgents, BUILTIN_AGENT_NAMES } from "../acp-agent.js"; + +// `discoverCustomAgents` distinguishes user/plugin-configured agents from +// Claude Code's built-in subagents by name (BUILTIN_AGENT_NAMES), since the +// SDK's `AgentInfo` carries no built-in/source flag. That hardcoded set drifts +// silently if the SDK changes its default roster: a newly added built-in would +// leak into the ACP "Agent" picker as if the user had configured it. This +// integration test runs the real SDK so the set is flagged the moment it does. +describe.skipIf(!process.env.RUN_INTEGRATION_TESTS)("agent discovery (SDK)", () => { + it("BUILTIN_AGENT_NAMES exactly covers the SDK's default agent roster", async () => { + const input = new Pushable(); + const q = query({ + prompt: input, + options: { + systemPrompt: { type: "preset", preset: "claude_code" }, + sessionId: randomUUID(), + // No setting sources → no user/project/local custom agents are loaded, + // so `supportedAgents()` reports exactly the intrinsic built-ins + // regardless of what the developer has in ~/.claude/agents. + settingSources: [], + includePartialMessages: true, + }, + }); + + try { + const reported = (await q.supportedAgents()).map((a) => a.name).sort(); + expect(reported).toEqual([...BUILTIN_AGENT_NAMES].sort()); + + // With the built-ins being the only agents present, discovery surfaces + // nothing — the ACP "Agent" picker is correctly omitted. + expect(await discoverCustomAgents(q)).toEqual([]); + } finally { + input.end(); + q.return(undefined); + } + }, 30000); +}); From afade87df7eb4869ace20ac3570be2ba9bb8c83f Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Fri, 19 Jun 2026 12:45:02 +0200 Subject: [PATCH 3/3] fix: reserve "default" agent sentinel to prevent collision A custom agent named exactly "default" collided with the synthetic Default picker entry: two options shared the value, and selecting it silently routed to applyFlagSettings({ agent: null }) (the standard agent) instead of the user's persona, making it unreachable. Centralize the sentinel as DEFAULT_AGENT_ID and exclude any custom agent with that name from discovery, so the value is unambiguous everywhere it's used. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/acp-agent.ts | 24 +++++++++++++++++------- src/tests/acp-agent.test.ts | 11 +++++++++++ 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/src/acp-agent.ts b/src/acp-agent.ts index 1764bcc9..5a58970e 100644 --- a/src/acp-agent.ts +++ b/src/acp-agent.ts @@ -2737,7 +2737,7 @@ export class ClaudeAcpAgent { // flag layer back to the standard Claude Code agent; the change takes // effect on the next turn (SDK >= 0.3.161). await session.query.applyFlagSettings({ - agent: value === "default" ? null : value, + agent: value === DEFAULT_AGENT_ID ? null : value, }); session.currentAgent = value; session.configOptions = session.configOptions.map((o) => @@ -3173,7 +3173,9 @@ export class ClaudeAcpAgent { // as a blank/invalid selection. const requestedAgent = userProvidedOptions?.agent; const currentAgent = - requestedAgent && agents.some((a) => a.name === requestedAgent) ? requestedAgent : "default"; + requestedAgent && agents.some((a) => a.name === requestedAgent) + ? requestedAgent + : DEFAULT_AGENT_ID; const configOptions = buildConfigOptions( modes, @@ -3409,13 +3411,21 @@ export const BUILTIN_AGENT_NAMES = new Set([ "statusline-setup", ]); +// Value of the synthetic "Default" entry in the agent picker, which maps to the +// standard Claude Code agent (`applyFlagSettings({ agent: null })`). It is a +// reserved sentinel: a custom agent named exactly this would collide with it +// (two options sharing the value, selection silently routing to `null`), so we +// exclude that name from discovery. +export const DEFAULT_AGENT_ID = "default"; + /** Discover user/plugin/project-configured main-thread agents, excluding the - * built-in subagents. Returns an empty list if discovery fails so a flaky - * control request never blocks session creation. */ + * built-in subagents and the reserved "default" sentinel. Returns an empty + * list if discovery fails so a flaky control request never blocks session + * creation. */ export async function discoverCustomAgents(q: Query): Promise { try { const agents = await q.supportedAgents(); - return agents.filter((a) => !BUILTIN_AGENT_NAMES.has(a.name)); + return agents.filter((a) => !BUILTIN_AGENT_NAMES.has(a.name) && a.name !== DEFAULT_AGENT_ID); } catch { return []; } @@ -3427,7 +3437,7 @@ export function buildConfigOptions( modelInfos: ModelInfo[], currentEffortLevel?: string, agents: AgentInfo[] = [], - currentAgent: string = "default", + currentAgent: string = DEFAULT_AGENT_ID, ): SessionConfigOption[] { const options: SessionConfigOption[] = [ { @@ -3503,7 +3513,7 @@ export function buildConfigOptions( type: "select", currentValue: currentAgent, options: [ - { value: "default", name: "Default", description: "Standard Claude Code agent" }, + { value: DEFAULT_AGENT_ID, name: "Default", description: "Standard Claude Code agent" }, ...agents.map((a) => ({ value: a.name, name: a.name, diff --git a/src/tests/acp-agent.test.ts b/src/tests/acp-agent.test.ts index d14baa49..14e0d344 100644 --- a/src/tests/acp-agent.test.ts +++ b/src/tests/acp-agent.test.ts @@ -5506,6 +5506,17 @@ describe("agent selection config option", () => { expect(agents.map((a) => a.name)).toEqual(["my-reviewer", "my-writer"]); }); + it("excludes a custom agent named 'default' (reserved sentinel)", async () => { + const q = { + supportedAgents: async () => [ + { name: "default", description: "collides with the synthetic Default entry" }, + { name: "my-reviewer", description: "Reviews code" }, + ], + } as any; + const agents = await discoverCustomAgents(q); + expect(agents.map((a) => a.name)).toEqual(["my-reviewer"]); + }); + it("returns an empty list when discovery throws", async () => { const q = { supportedAgents: async () => {