diff --git a/packages/server/src/server/agent/providers/acp-agent.mode-namespace.test.ts b/packages/server/src/server/agent/providers/acp-agent.mode-namespace.test.ts new file mode 100644 index 0000000000..cfbf45d3f3 --- /dev/null +++ b/packages/server/src/server/agent/providers/acp-agent.mode-namespace.test.ts @@ -0,0 +1,152 @@ +import { describe, expect, test, vi } from "vitest"; +import type { + ClientSideConnection, + SessionConfigOption, + SessionUpdate, +} from "@agentclientprotocol/sdk"; +import { ACPAgentSession, type SessionStateResponse } from "./acp-agent.js"; +import type { AgentStreamEvent } from "../agent-sdk-types.js"; +import { asInternals } from "../../test-utils/class-mocks.js"; +import { createTestLogger } from "../../../test-utils/test-logger.js"; + +interface ModeSessionInternals { + sessionId: string | null; + connection: Pick; + applySessionState(response: SessionStateResponse): void; + translateSessionUpdate(update: SessionUpdate): AgentStreamEvent[]; +} + +const modes = { + currentModeId: "accept_edits", + availableModes: [ + { id: "default", name: "Default" }, + { id: "accept_edits", name: "Accept Edits" }, + { id: "dont_ask", name: "Don't Ask" }, + ], +}; +function editOptions(currentValue = "workspace_session"): SessionConfigOption[] { + return [ + { + id: "edit_approval_policy", + name: "Edits", + category: "mode", + type: "select", + currentValue, + options: [ + { value: "ask", name: "Default" }, + { value: "workspace_session", name: "Accept Edits" }, + { value: "session", name: "Don't Ask" }, + ], + }, + ]; +} +function fixture(withSessionModes = true) { + const session = new ACPAgentSession( + { provider: "hermes", cwd: process.cwd() }, + { + provider: "hermes", + logger: createTestLogger(), + defaultCommand: ["not-spawned"], + defaultModes: [], + capabilities: { + supportsStreaming: true, + supportsSessionPersistence: true, + supportsDynamicModes: true, + supportsMcpServers: true, + supportsReasoningStream: true, + supportsToolInvocations: true, + }, + }, + ); + const setSessionMode = vi.fn(async ({ modeId }: { sessionId: string; modeId: string }) => { + if (!modes.availableModes.some((mode) => mode.id === modeId)) throw new Error("invalid_params"); + return {}; + }); + const setSessionConfigOption = vi.fn( + async ({ value }: { sessionId: string; configId: string; value: string }) => ({ + configOptions: editOptions(value), + }), + ); + const internals = asInternals(session); + internals.sessionId = "isolated-session"; + internals.connection = { setSessionMode, setSessionConfigOption }; + internals.applySessionState({ + sessionId: "isolated-session", + modes: withSessionModes ? modes : undefined, + configOptions: editOptions(), + }); + return { session, internals, setSessionMode, setSessionConfigOption }; +} + +describe("ACP mode and config-value namespaces", () => { + test("keeps a pushed current mode when config values describe a different namespace", async () => { + const { session, internals } = fixture(); + internals.translateSessionUpdate({ + sessionUpdate: "current_mode_update", + currentModeId: "dont_ask", + }); + internals.translateSessionUpdate({ + sessionUpdate: "config_option_update", + configOptions: editOptions("session"), + }); + expect(await session.getCurrentMode()).toBe("dont_ask"); + internals.translateSessionUpdate({ sessionUpdate: "config_option_update", configOptions: [] }); + expect(await session.getCurrentMode()).toBe("dont_ask"); + expect((await session.getAvailableModes()).map((mode) => mode.id)).toEqual([ + "default", + "accept_edits", + "dont_ask", + ]); + }); + + test("does not send a config policy value to the mode endpoint or silently fall back", async () => { + const { session, internals, setSessionMode, setSessionConfigOption } = fixture(); + internals.translateSessionUpdate({ + sessionUpdate: "config_option_update", + configOptions: editOptions(), + }); + await session.setMode("session"); + expect(setSessionMode).not.toHaveBeenCalled(); + expect(setSessionConfigOption).not.toHaveBeenCalled(); + expect(await session.getCurrentMode()).toBe("accept_edits"); + }); + + test("keeps the confirmed mode when the mode request fails", async () => { + const { session, setSessionMode, setSessionConfigOption } = fixture(); + setSessionMode.mockRejectedValueOnce(new Error("mode denied")); + await expect(session.setMode("dont_ask")).rejects.toThrow("mode denied"); + expect(await session.getCurrentMode()).toBe("accept_edits"); + expect(setSessionConfigOption).not.toHaveBeenCalled(); + }); + + test("uses the config endpoint when a provider only advertises config choices", async () => { + const { session, setSessionMode, setSessionConfigOption } = fixture(false); + await session.setMode("session"); + expect(setSessionMode).not.toHaveBeenCalled(); + expect(setSessionConfigOption).toHaveBeenCalledExactlyOnceWith({ + sessionId: "isolated-session", + configId: "edit_approval_policy", + value: "session", + }); + expect(await session.getCurrentMode()).toBe("session"); + }); + + test("keeps real mode IDs after a config update and sends Don't Ask to the mode endpoint", async () => { + const { session, internals, setSessionMode, setSessionConfigOption } = fixture(); + internals.translateSessionUpdate({ + sessionUpdate: "config_option_update", + configOptions: editOptions(), + }); + expect(await session.getAvailableModes()).toEqual( + modes.availableModes.map(({ id, name }) => ({ id, label: name, description: undefined })), + ); + expect(await session.getCurrentMode()).toBe("accept_edits"); + await session.setMode("dont_ask"); + expect(setSessionMode).toHaveBeenCalledExactlyOnceWith({ + sessionId: "isolated-session", + modeId: "dont_ask", + }); + expect(setSessionConfigOption).not.toHaveBeenCalled(); + expect(await session.getCurrentMode()).toBe("dont_ask"); + }); +}); diff --git a/packages/server/src/server/agent/providers/acp-agent.test.ts b/packages/server/src/server/agent/providers/acp-agent.test.ts index ea0c1702e9..873e72b7aa 100644 --- a/packages/server/src/server/agent/providers/acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/acp-agent.test.ts @@ -140,6 +140,7 @@ interface ACPModelSelectionInternals { } interface ACPConfiguredOverrideInternals { + applySessionState(response: SessionStateResponse): void; sessionId: string | null; connection: { setSessionMode: (input: { sessionId: string; modeId: string }) => Promise; @@ -444,6 +445,24 @@ function prepareConfiguredOverrideSession( configOptions: options.configOptions ?? [], })); const internals = asInternals(session); + const availableModes = options.availableModes ?? []; + const modes = + availableModes.length > 0 + ? { + currentModeId: options.currentMode ?? availableModes[0]!.id, + availableModes: availableModes.map((mode) => ({ + id: mode.id, + label: mode.label, + description: mode.description, + name: mode.label, + })), + } + : undefined; + internals.applySessionState({ + sessionId: "session-1", + modes, + configOptions: options.configOptions ?? [], + }); internals.sessionId = "session-1"; internals.connection = { setSessionMode, @@ -1756,6 +1775,62 @@ describe("ACPAgentSession Zed parity", () => { }); }); + test("tracks initialized Copilot Allow All notifications and disables them before switching away", async () => { + const agentMode = "https://agentclientprotocol.com/protocol/session-modes#agent"; + const session = createCopilotSessionWithConfig(); + const prepared = prepareConfiguredOverrideSession(session, { + currentMode: agentMode, + availableModes: COPILOT_MODES, + configOptions: [copilotModeConfigOption(agentMode), copilotAllowAllConfigOption("off")], + }); + prepared.internals.applySessionState({ + sessionId: "session-1", + modes: { + currentModeId: agentMode, + availableModes: COPILOT_MODES.map((mode) => ({ id: mode.id, name: mode.label })), + }, + configOptions: [copilotModeConfigOption(agentMode), copilotAllowAllConfigOption("off")], + }); + asInternals(session).translateSessionUpdate({ + sessionUpdate: "config_option_update", + configOptions: [copilotModeConfigOption(agentMode), copilotAllowAllConfigOption("on")], + }); + await expect(session.getCurrentMode()).resolves.toBe(COPILOT_ALLOW_ALL_MODE_ID); + await session.setMode(agentMode); + expect(prepared.setSessionConfigOption).toHaveBeenCalledExactlyOnceWith({ + sessionId: "session-1", + configId: "allow_all", + value: "off", + }); + expect(prepared.setSessionMode).toHaveBeenCalledExactlyOnceWith({ + sessionId: "session-1", + modeId: agentMode, + }); + expect(prepared.setSessionConfigOption.mock.invocationCallOrder[0]).toBeLessThan( + prepared.setSessionMode.mock.invocationCallOrder[0]!, + ); + }); + + test("tracks initialized Copilot Allow All being turned off through config", async () => { + const agentMode = "https://agentclientprotocol.com/protocol/session-modes#agent"; + const session = createCopilotSessionWithConfig(); + const prepared = prepareConfiguredOverrideSession(session, { availableModes: COPILOT_MODES }); + prepared.internals.applySessionState({ + sessionId: "session-1", + modes: { + currentModeId: agentMode, + availableModes: COPILOT_MODES.map((mode) => ({ id: mode.id, name: mode.label })), + }, + configOptions: [copilotModeConfigOption(agentMode), copilotAllowAllConfigOption("on")], + }); + await expect(session.getCurrentMode()).resolves.toBe(COPILOT_ALLOW_ALL_MODE_ID); + asInternals(session).translateSessionUpdate({ + sessionUpdate: "config_option_update", + configOptions: [copilotModeConfigOption(agentMode), copilotAllowAllConfigOption("off")], + }); + await expect(session.getCurrentMode()).resolves.toBe(agentMode); + }); + test("trusts Copilot allow_all config updates as the current mode source", async () => { const session = createCopilotSessionWithConfig(); const internals = asInternals(session); diff --git a/packages/server/src/server/agent/providers/acp-agent.ts b/packages/server/src/server/agent/providers/acp-agent.ts index 09662aaa1c..4b67d827dd 100644 --- a/packages/server/src/server/agent/providers/acp-agent.ts +++ b/packages/server/src/server/agent/providers/acp-agent.ts @@ -1769,6 +1769,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { private readonly earlySessionUpdates: SessionNotification[] = []; private currentMode: string | null = null; private availableModes: AgentMode[]; + private sessionModeState: SessionStateResponse["modes"] = null; private currentModel: string | null = null; private availableModels: AvailableACPModel[] | null = null; private thinkingOptionId: string | null = null; @@ -2148,11 +2149,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { throw new Error("ACP session not initialized"); } - const selection = resolveACPModeSelection({ - modeId, - availableModes: this.availableModes, - configOptions: this.configOptions, - }); + const selection = this.resolveModeSelection(modeId); await this.setModeWithSelection({ modeId, selection }); } @@ -2178,7 +2175,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { if (providerResult.configOptions) { this.configOptions = this.transformConfigOptions(providerResult.configOptions); } - this.availableModes = deriveModesFromACP(this.defaultModes, null, this.configOptions).modes; + this.availableModes = this.modesFromConfig().modes; this.pushEvent({ type: "mode_changed", provider: this.provider, @@ -2257,7 +2254,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { requestedValue: modeId, label: "mode", }); - this.availableModes = deriveModesFromACP(this.defaultModes, null, this.configOptions).modes; + this.availableModes = this.modesFromConfig().modes; this.pushEvent({ type: "mode_changed", provider: this.provider, @@ -3010,6 +3007,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { : response; this.configOptions = this.transformConfigOptions(transformed.configOptions ?? []); + this.sessionModeState = transformed.modes ?? null; const modeInfo = deriveModesFromACP(this.defaultModes, transformed.modes, this.configOptions); this.availableModes = modeInfo.modes; @@ -3035,11 +3033,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { private async applyConfiguredOverrides(): Promise { const configuredModeId = this.config.modeId; if (configuredModeId && configuredModeId !== this.currentMode) { - const selection = resolveACPModeSelection({ - modeId: configuredModeId, - availableModes: this.availableModes, - configOptions: this.configOptions, - }); + const selection = this.resolveModeSelection(configuredModeId); await this.setModeWithSelection({ modeId: configuredModeId, selection }); } const configuredModelId = this.config.model; @@ -3296,9 +3290,37 @@ export class ACPAgentSession implements AgentSession, ACPClient { this.currentMode = this.transformModeId(update.currentModeId); } + private modesFromConfig() { + let modes = this.sessionModeState; + if (modes) { + let currentModeId = this.currentMode ?? modes.currentModeId; + const configMode = findSelectConfigOption({ + configOptions: this.configOptions, + category: "mode", + }); + // Copilot config updates name real modes; Hermes config values are a separate policy namespace. + if (configMode && modes.availableModes.some((mode) => mode.id === configMode.currentValue)) { + currentModeId = configMode.currentValue; + } + modes = { ...modes, currentModeId }; + } + return deriveModesFromACP(this.defaultModes, modes, this.configOptions); + } + + private resolveModeSelection(modeId: string): ACPModeSelection { + const hasSessionModes = Boolean(this.sessionModeState?.availableModes.length); + const modeConfig = findSelectConfigOption({ + configOptions: this.configOptions, + category: "mode", + }); + const configOnly = !hasSessionModes && modeConfig !== null; + const availableModes = configOnly ? [] : this.availableModes; + return resolveACPModeSelection({ modeId, availableModes, configOptions: this.configOptions }); + } + private handleConfigOptionUpdate(update: ConfigOptionUpdate): AgentStreamEvent[] { this.configOptions = this.transformConfigOptions(update.configOptions); - const modeInfo = deriveModesFromACP(this.defaultModes, null, this.configOptions); + const modeInfo = this.modesFromConfig(); const nextMode = modeInfo.currentModeId; const nextModel = deriveCurrentConfigValue(this.configOptions, "model"); const nextThinkingOptionId = deriveCurrentConfigValue(this.configOptions, "thought_level");