test(acp): verify mode identity through public sessions

Exercise initialization, queued notifications, mode selection, and rejection over stdio instead of replacing private connection state. Co-authored-by: a-sh <846704+a-sh@users.noreply.github.com>
2026-11-06 08:24:46 +00:00 · 2026-11-06 06:46:13 +00:00
parent 787410d94b
commit 94ae0e1eda
2 changed files with 254 additions and 164 deletions
@@ -1,21 +1,12 @@
-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 { afterEach, describe, expect, test } from "vitest";
+import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises";
+import path from "node:path";
+import type { SessionConfigOption, SessionUpdate } from "@agentclientprotocol/sdk";
+import { ACPAgentClient, type SessionStateResponse } from "./acp-agent.js";
+import { CopilotACPAgentClient, COPILOT_ALLOW_ALL_MODE_ID } from "./copilot-acp-agent.js";
+import type { AgentSession } from "../agent-sdk-types.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: [
@@ -24,6 +15,7 @@ const modes = {
    { id: "dont_ask", name: "Don't Ask" },
  ],
 };
+
 function editOptions(currentValue = "workspace_session"): SessionConfigOption[] {
  return [
    {
@@ -40,113 +32,267 @@ function editOptions(currentValue = "workspace_session"): SessionConfigOption[]
    },
  ];
 }
-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 };
+
+// A controlled ACP process, speaking the same stdio protocol as a provider.
+// Notifications precede session/new's response to exercise queued initialization updates.
+const modePeerScript = String.raw`
+import { createInterface } from "node:readline";
+import { readFileSync, appendFileSync } from "node:fs";
+const scenario = JSON.parse(readFileSync(process.argv[2], "utf8"));
+let configOptions = scenario.state.configOptions;
+const send = (message) => process.stdout.write(JSON.stringify(message) + "\n");
+createInterface({ input: process.stdin }).on("line", (line) => {
+  const request = JSON.parse(line);
+  const reply = (result) => send({jsonrpc: "2.0", id: request.id, result});
+  const reject = (message) => send({jsonrpc: "2.0", id: request.id, error: {code: -32602, message}});
+  switch (request.method) {
+    case "initialize":
+      reply({protocolVersion: 1, agentCapabilities: {}});
+      break;
+    case "session/new":
+      for (const update of scenario.updates) {
+        send({jsonrpc: "2.0", method: "session/update", params: {sessionId: "mode-session", update}});
+      }
+      reply({sessionId: "mode-session", ...scenario.state});
+      break;
+    case "session/set_mode":
+      appendFileSync(process.argv[3], JSON.stringify(request) + "\n");
+      if (request.params.modeId === scenario.rejectedMode) {
+        reject("mode denied");
+      } else if (!scenario.state.modes?.availableModes.some(mode => mode.id === request.params.modeId)) {
+        reject("invalid_params");
+      } else {
+        reply({});
+      }
+      break;
+    case "session/set_config_option":
+      appendFileSync(process.argv[3], JSON.stringify(request) + "\n");
+      configOptions = configOptions.map(option => option.id === request.params.configId
+        ? {...option, currentValue: request.params.value} : option);
+      reply({configOptions});
+      break;
+    default:
+      reject("Unexpected request: " + request.method);
+  }
+});
+`;
+
+interface ModeCall {
+  method: string;
+  params: { sessionId: string; modeId?: string; configId?: string; value?: string };
 }

-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",
+let session: AgentSession | undefined;
+let directory: string | undefined;
+
+afterEach(async () => {
+  try {
+    await session?.close();
+  } finally {
+    session = undefined;
+    if (directory) await rm(directory, { recursive: true, force: true });
+    directory = undefined;
+  }
+});
+
+async function fixture(
+  options: {
+    state?: Omit<SessionStateResponse, "sessionId">;
+    updates?: SessionUpdate[];
+    rejectedMode?: string;
+    startupMode?: string;
+    copilot?: boolean;
+  } = {},
+) {
+  const artifacts = path.resolve(".artifacts");
+  await mkdir(artifacts, { recursive: true });
+  directory = await mkdtemp(path.join(artifacts, "acp-mode-"));
+  const script = path.join(directory, "peer.mjs");
+  const scenario = path.join(directory, "scenario.json");
+  const calls = path.join(directory, "calls.jsonl");
+  await writeFile(script, modePeerScript);
+  await writeFile(calls, "");
+  await writeFile(
+    scenario,
+    JSON.stringify({
+      state: options.state ?? { modes, configOptions: editOptions() },
+      updates: options.updates ?? [
+        { sessionUpdate: "config_option_update", configOptions: editOptions() },
+      ],
+      rejectedMode: options.rejectedMode,
+    }),
+  );
+  const command: [string, ...string[]] = [process.execPath, script, scenario, calls];
+  const client = options.copilot
+    ? new CopilotACPAgentClient({
+        logger: createTestLogger(),
+        runtimeSettings: { command: { mode: "replace", argv: command } },
+      })
+    : new ACPAgentClient({
+        provider: "mode-peer",
+        logger: createTestLogger(),
+        defaultCommand: command,
+      });
+  session = await client.createSession({
+    provider: options.copilot ? "copilot" : "mode-peer",
+    cwd: directory,
+    modeId: options.startupMode,
+  });
+  return {
+    session,
+    async calls(): Promise<ModeCall[]> {
+      const lines = (await readFile(calls, "utf8")).trim();
+      return lines
+        ? lines.split("\n").map((line) => {
+            const { method, params } = JSON.parse(line) as ModeCall;
+            return { method, params };
+          })
+        : [];
+    },
+  };
+}
+
+const modeCall = (modeId: string) => ({
+  method: "session/set_mode",
+  params: { sessionId: "mode-session", modeId },
+});
+const configCall = (configId: string, value: string) => ({
+  method: "session/set_config_option",
+  params: { sessionId: "mode-session", configId, value },
+});
+
+describe("ACP mode and config-value namespaces over stdio", () => {
+  test("selects Don't Ask from the advertised catalogue after a queued config update", async () => {
+    const peer = await fixture();
+    const available = await peer.session.getAvailableModes();
+    const dontAsk = available.find((mode) => mode.label === "Don't Ask");
+    expect(dontAsk).toBeDefined();
+    await peer.session.setMode(dontAsk!.id);
+    expect(await peer.calls()).toEqual([modeCall("dont_ask")]);
+    expect(await peer.session.getCurrentMode()).toBe("dont_ask");
+    expect(available.map((mode) => mode.id)).toEqual(["default", "accept_edits", "dont_ask"]);
+  });
+
+  test("keeps a pushed current mode through separate-policy and empty config updates", async () => {
+    const peer = await fixture({
+      updates: [
+        { sessionUpdate: "current_mode_update", currentModeId: "dont_ask" },
+        { sessionUpdate: "config_option_update", configOptions: editOptions("session") },
+        { sessionUpdate: "config_option_update", configOptions: [] },
+      ],
    });
-    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([
+    expect(await peer.session.getCurrentMode()).toBe("dont_ask");
+    expect((await peer.session.getAvailableModes()).map((mode) => mode.id)).toEqual([
      "default",
      "accept_edits",
      "dont_ask",
    ]);
+    expect(await peer.calls()).toEqual([]);
  });

-  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("ignores policy values outside the advertised session modes without an RPC", async () => {
+    const peer = await fixture();
+    await peer.session.setMode("session");
+    expect(await peer.calls()).toEqual([]);
+    expect(await peer.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("retains confirmed state after rejection without falling back to a config write", async () => {
+    const peer = await fixture({ rejectedMode: "dont_ask", updates: [] });
+    await expect(peer.session.setMode("dont_ask")).rejects.toThrow("mode denied");
+    expect(await peer.session.getCurrentMode()).toBe("accept_edits");
+    expect(await peer.calls()).toEqual([modeCall("dont_ask")]);
  });

-  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("selects a config-only mode through its owning config option", async () => {
+    const peer = await fixture({ state: { configOptions: editOptions() } });
+    await peer.session.setMode("session");
+    expect(await peer.calls()).toEqual([configCall("edit_approval_policy", "session")]);
+    expect(await peer.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(),
+  test.each([true, false])("routes startup overrides with session modes: %s", async (withModes) => {
+    const peer = await fixture({
+      state: { modes: withModes ? modes : undefined, configOptions: editOptions() },
+      startupMode: withModes ? "dont_ask" : "session",
    });
-    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");
+    expect(await peer.calls()).toEqual([
+      withModes ? modeCall("dont_ask") : configCall("edit_approval_policy", "session"),
+    ]);
+    expect(await peer.session.getCurrentMode()).toBe(withModes ? "dont_ask" : "session");
+  });
+});
+
+const agentMode = "https://agentclientprotocol.com/protocol/session-modes#agent";
+const planMode = "https://agentclientprotocol.com/protocol/session-modes#plan";
+function copilotConfig(allowAll: string): SessionConfigOption[] {
+  return [
+    {
+      id: "mode",
+      name: "Mode",
+      category: "mode",
+      type: "select",
+      currentValue: agentMode,
+      options: [
+        { value: agentMode, name: "Agent" },
+        { value: planMode, name: "Plan" },
+      ],
+    },
+    {
+      id: "allow_all",
+      name: "Allow All",
+      type: "select",
+      currentValue: allowAll,
+      options: [
+        { value: "on", name: "On" },
+        { value: "off", name: "Off" },
+      ],
+    },
+  ];
+}
+function copilotFixture(initial: string, pushed: string) {
+  return fixture({
+    copilot: true,
+    state: {
+      modes: {
+        currentModeId: agentMode,
+        availableModes: [
+          { id: agentMode, name: "Agent" },
+          { id: planMode, name: "Plan" },
+        ],
+      },
+      configOptions: copilotConfig(initial),
+    },
+    updates: [{ sessionUpdate: "config_option_update", configOptions: copilotConfig(pushed) }],
+  });
+}
+
+describe("Copilot Allow All through the public client", () => {
+  test("adopts an Allow All notification and disables it before switching modes", async () => {
+    const peer = await copilotFixture("off", "on");
+    expect(await peer.session.getCurrentMode()).toBe(COPILOT_ALLOW_ALL_MODE_ID);
+    await peer.session.setMode(planMode);
+    expect(await peer.calls()).toEqual([configCall("allow_all", "off"), modeCall(planMode)]);
+    expect(await peer.session.getCurrentMode()).toBe(planMode);
+  });
+
+  test("adopts an Allow All off notification after initializing with it enabled", async () => {
+    const peer = await copilotFixture("on", "off");
+    expect(await peer.session.getCurrentMode()).toBe(agentMode);
+    expect(await peer.calls()).toEqual([]);
+  });
+
+  test("selects Allow All and switches away in provider RPC order", async () => {
+    const peer = await copilotFixture("off", "off");
+    await peer.session.setMode(COPILOT_ALLOW_ALL_MODE_ID);
+    expect(await peer.session.getCurrentMode()).toBe(COPILOT_ALLOW_ALL_MODE_ID);
+    await peer.session.setMode(planMode);
+    expect(await peer.calls()).toEqual([
+      modeCall(agentMode),
+      configCall("allow_all", "on"),
+      configCall("allow_all", "off"),
+      modeCall(planMode),
+    ]);
+    expect(await peer.session.getCurrentMode()).toBe(planMode);
  });
 });
@@ -1775,62 +1775,6 @@ 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);