diff --git a/src/acp-agent.ts b/src/acp-agent.ts index d2f13aff..5a58970e 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 @@ -2719,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_AGENT_ID ? 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, @@ -3141,11 +3165,25 @@ export class ClaudeAcpAgent { availableModes, }; + const agents = await discoverCustomAgents(q); + // 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_AGENT_ID; + 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 +3214,8 @@ export class ClaudeAcpAgent { models, modelInfos: allowedModels, configOptions, + agents, + currentAgent, abortController, emitRawSDKMessages: sessionMeta?.claudeCode?.emitRawSDKMessages ?? false, contextWindowSize: @@ -3357,11 +3397,47 @@ 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. +export const BUILTIN_AGENT_NAMES = new Set([ + "claude", + "general-purpose", + "Explore", + "Plan", + "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 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) && a.name !== DEFAULT_AGENT_ID); + } catch { + return []; + } +} + +export function buildConfigOptions( modes: SessionModeState, models: SessionModelState, modelInfos: ModelInfo[], currentEffortLevel?: string, + agents: AgentInfo[] = [], + currentAgent: string = DEFAULT_AGENT_ID, ): SessionConfigOption[] { const options: SessionConfigOption[] = [ { @@ -3425,6 +3501,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_AGENT_ID, 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..14e0d344 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,177 @@ 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("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 () => { + 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"); + }); + + 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); +});