fix(acp): preserve session mode identity across config updates

This commit is contained in:
a-sh
2026-11-05 10:53:03 +00:00
parent cdd7800fed
commit 787410d94b
3 changed files with 262 additions and 13 deletions
@@ -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<ClientSideConnection, "setSessionMode" | "setSessionConfigOption">;
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<ModeSessionInternals>(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");
});
});
@@ -140,6 +140,7 @@ interface ACPModelSelectionInternals {
}
interface ACPConfiguredOverrideInternals {
applySessionState(response: SessionStateResponse): void;
sessionId: string | null;
connection: {
setSessionMode: (input: { sessionId: string; modeId: string }) => Promise<void>;
@@ -444,6 +445,24 @@ function prepareConfiguredOverrideSession(
configOptions: options.configOptions ?? [],
}));
const internals = asInternals<ACPConfiguredOverrideInternals>(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<ACPSessionInternals>(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<ACPSessionInternals>(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<ACPSessionInternals>(session);
@@ -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<void> {
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");