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 1e13ef190..1ab2f7fcf 100644 --- a/packages/server/src/server/agent/providers/acp-agent.test.ts +++ b/packages/server/src/server/agent/providers/acp-agent.test.ts @@ -65,6 +65,11 @@ interface ACPModelSelectionInternals { interface ACPConfiguredOverrideInternals { sessionId: string | null; connection: { + prompt: (input: { + sessionId: string; + messageId: string; + prompt: Array<{ type: "text"; text: string }>; + }) => Promise; setSessionMode: (input: { sessionId: string; modeId: string }) => Promise; setSessionConfigOption: (input: { sessionId: string; @@ -253,9 +258,11 @@ function prepareConfiguredOverrideSession( const setSessionConfigOption = vi.fn(async () => ({ configOptions: options.configOptions ?? [], })); + const prompt = vi.fn(async () => ({ stopReason: "end_turn" }) as PromptResponse); const internals = asInternals(session); internals.sessionId = "session-1"; internals.connection = { + prompt, setSessionMode, setSessionConfigOption, unstable_setSessionModel: unstableSetSessionModel, @@ -1001,6 +1008,100 @@ describe("ACPAgentSession Zed parity", () => { expect(events.some((event) => event.type === "permission_requested")).toBe(false); }); + test("falls back to Copilot's allow-all slash command when allow_all config is not ready", async () => { + const setSessionConfigOption = vi.fn(async () => { + throw { + code: -32603, + message: + "Failed to set config option 'allow_all' to 'on': Error: Permission service is unavailable for this session.", + }; + }); + const prompt = vi.fn(async () => ({ stopReason: "end_turn" }) as PromptResponse); + const setSessionMode = vi.fn(async () => undefined); + const session = createCopilotSessionWithConfig(COPILOT_ALLOW_ALL_MODE_ID); + const { internals } = prepareConfiguredOverrideSession(session, { + currentMode: "https://agentclientprotocol.com/protocol/session-modes#agent", + availableModes: COPILOT_MODES, + configOptions: [ + copilotModeConfigOption("https://agentclientprotocol.com/protocol/session-modes#agent"), + copilotAllowAllConfigOption("off"), + ], + connection: { prompt, setSessionConfigOption, setSessionMode }, + }); + + await internals.applyConfiguredOverrides(); + + expect(setSessionConfigOption).toHaveBeenCalledWith({ + sessionId: "session-1", + configId: "allow_all", + value: "on", + }); + expect(prompt).toHaveBeenCalledWith({ + sessionId: "session-1", + messageId: expect.any(String), + prompt: [{ type: "text", text: "/allow-all" }], + }); + expect(setSessionMode).not.toHaveBeenCalled(); + await expect(session.getCurrentMode()).resolves.toBe(COPILOT_ALLOW_ALL_MODE_ID); + }); + + test("refuses Copilot /allow-all fallback while a foreground turn is active", async () => { + const permissionError = { + code: -32603, + message: + "Failed to set config option 'allow_all' to 'on': Error: Permission service is unavailable for this session.", + }; + const setSessionConfigOption = vi.fn(async () => { + throw permissionError; + }); + const prompt = vi.fn(async () => ({ stopReason: "end_turn" }) as PromptResponse); + const setSessionMode = vi.fn(async () => undefined); + const session = createCopilotSessionWithConfig(COPILOT_ALLOW_ALL_MODE_ID); + const { internals } = prepareConfiguredOverrideSession(session, { + currentMode: "https://agentclientprotocol.com/protocol/session-modes#agent", + availableModes: COPILOT_MODES, + configOptions: [ + copilotModeConfigOption("https://agentclientprotocol.com/protocol/session-modes#agent"), + copilotAllowAllConfigOption("off"), + ], + connection: { prompt, setSessionConfigOption, setSessionMode }, + }); + (internals as unknown as { activeForegroundTurnId: string | null }).activeForegroundTurnId = + "turn-in-flight"; + + await expect(internals.applyConfiguredOverrides()).rejects.toBe(permissionError); + expect(prompt).not.toHaveBeenCalled(); + expect(setSessionMode).not.toHaveBeenCalled(); + }); + + test("surfaces the original error if Copilot /allow-all fallback ends with a non-end_turn stop reason", async () => { + const permissionError = { + code: -32603, + message: + "Failed to set config option 'allow_all' to 'on': Error: Permission service is unavailable for this session.", + }; + const setSessionConfigOption = vi.fn(async () => { + throw permissionError; + }); + const prompt = vi.fn(async () => ({ stopReason: "refusal" }) as PromptResponse); + const setSessionMode = vi.fn(async () => undefined); + const session = createCopilotSessionWithConfig(COPILOT_ALLOW_ALL_MODE_ID); + const { internals } = prepareConfiguredOverrideSession(session, { + currentMode: "https://agentclientprotocol.com/protocol/session-modes#agent", + availableModes: COPILOT_MODES, + configOptions: [ + copilotModeConfigOption("https://agentclientprotocol.com/protocol/session-modes#agent"), + copilotAllowAllConfigOption("off"), + ], + connection: { prompt, setSessionConfigOption, setSessionMode }, + }); + + await expect(internals.applyConfiguredOverrides()).rejects.toBe(permissionError); + expect(prompt).toHaveBeenCalledTimes(1); + expect(setSessionMode).not.toHaveBeenCalled(); + await expect(session.getCurrentMode()).resolves.not.toBe(COPILOT_ALLOW_ALL_MODE_ID); + }); + test("accepts Copilot's legacy autopilot mode ID as Allow All", async () => { const setSessionConfigOption = vi.fn(async () => ({ configOptions: [ diff --git a/packages/server/src/server/agent/providers/acp-agent.ts b/packages/server/src/server/agent/providers/acp-agent.ts index 5b5f3657c..9ed5ceeb0 100644 --- a/packages/server/src/server/agent/providers/acp-agent.ts +++ b/packages/server/src/server/agent/providers/acp-agent.ts @@ -410,6 +410,8 @@ export interface ACPProviderModeWriterContext { selection: ACPModeSelection; configOptions: SessionConfigOption[]; logger: Logger; + hasActiveTurn: boolean; + suppressUserEcho: (params: { messageId: string; text: string }) => void; } export interface ACPProviderModeWriteResult { @@ -1337,6 +1339,11 @@ export class ACPAgentSession implements AgentSession, ACPClient { selection, configOptions: this.configOptions, logger: this.logger, + hasActiveTurn: this.activeForegroundTurnId !== null, + suppressUserEcho: ({ messageId, text }) => { + this.suppressUserEchoMessageId = messageId; + this.suppressUserEchoText = text; + }, }; } diff --git a/packages/server/src/server/agent/providers/copilot-acp-agent.ts b/packages/server/src/server/agent/providers/copilot-acp-agent.ts index d6a8fadf9..607297081 100644 --- a/packages/server/src/server/agent/providers/copilot-acp-agent.ts +++ b/packages/server/src/server/agent/providers/copilot-acp-agent.ts @@ -1,4 +1,5 @@ import type { Logger } from "pino"; +import { randomUUID } from "node:crypto"; import { homedir } from "node:os"; import type { SessionConfigOption } from "@agentclientprotocol/sdk"; @@ -43,6 +44,8 @@ export const COPILOT_ALLOW_ALL_MODE_ID = "allow-all"; const COPILOT_ALLOW_ALL_CONFIG_ID = "allow_all"; const COPILOT_ALLOW_ALL_ON = "on"; const COPILOT_ALLOW_ALL_OFF = "off"; +const COPILOT_ALLOW_ALL_COMMAND = "/allow-all"; +const COPILOT_PERMISSION_SERVICE_UNAVAILABLE = "Permission service is unavailable for this session"; type SelectConfigOption = Extract; export const COPILOT_MODES: AgentMode[] = [ @@ -224,15 +227,53 @@ export async function writeCopilotProviderMode( if (!requestsAllowAll) { return { handled: false }; } - const response = await context.connection.setSessionConfigOption({ - sessionId: context.sessionId, - configId: COPILOT_ALLOW_ALL_CONFIG_ID, - value: COPILOT_ALLOW_ALL_ON, - }); + let configOptions: SessionConfigOption[] | undefined; + try { + const response = await context.connection.setSessionConfigOption({ + sessionId: context.sessionId, + configId: COPILOT_ALLOW_ALL_CONFIG_ID, + value: COPILOT_ALLOW_ALL_ON, + }); + configOptions = response.configOptions; + } catch (error) { + if (!isCopilotPermissionServiceUnavailableError(error)) { + throw error; + } + if (context.hasActiveTurn) { + // Issuing a second concurrent prompt on the same ACP session is unsafe; let the caller retry once the turn settles. + context.logger.warn( + { err: error }, + "Copilot allow_all config write failed and a foreground turn is active; refusing /allow-all fallback", + ); + throw error; + } + context.logger.warn( + { err: error }, + "Copilot allow_all config write failed before permission service was ready; falling back to /allow-all", + ); + const fallbackMessageId = randomUUID(); + context.suppressUserEcho({ + messageId: fallbackMessageId, + text: COPILOT_ALLOW_ALL_COMMAND, + }); + const promptResponse = await context.connection.prompt({ + sessionId: context.sessionId, + messageId: fallbackMessageId, + prompt: [{ type: "text", text: COPILOT_ALLOW_ALL_COMMAND }], + }); + if (promptResponse.stopReason !== "end_turn") { + context.logger.warn( + { stopReason: promptResponse.stopReason }, + "Copilot /allow-all fallback returned an unexpected stop reason; surfacing the original error", + ); + throw error; + } + configOptions = setCopilotAllowAllConfigValue(context.configOptions, COPILOT_ALLOW_ALL_ON); + } return { handled: true, currentModeId: COPILOT_ALLOW_ALL_MODE_ID, - configOptions: response.configOptions, + configOptions, }; } @@ -261,3 +302,34 @@ function isCopilotAllowAllEnabled(configOptions: SessionConfigOption[]): boolean option.currentValue === COPILOT_ALLOW_ALL_ON, ); } + +function setCopilotAllowAllConfigValue( + configOptions: SessionConfigOption[], + value: typeof COPILOT_ALLOW_ALL_ON | typeof COPILOT_ALLOW_ALL_OFF, +): SessionConfigOption[] { + return configOptions.map((option) => { + if (option.type !== "select" || option.id !== COPILOT_ALLOW_ALL_CONFIG_ID) { + return option; + } + return { ...option, currentValue: value }; + }); +} + +function isCopilotPermissionServiceUnavailableError(error: unknown): boolean { + const message = getErrorMessage(error); + return message?.includes(COPILOT_PERMISSION_SERVICE_UNAVAILABLE) ?? false; +} + +function getErrorMessage(error: unknown): string | null { + if (error instanceof Error) { + return error.message; + } + if (!isRecord(error)) { + return null; + } + return typeof error.message === "string" ? error.message : null; +} + +function isRecord(value: unknown): value is Record { + return value != null && typeof value === "object" && !Array.isArray(value); +}