fix(server): ACP reports the selects it moved during open or apply as that call's result

A config_option_update that arrived before the write response was emitted
as an event on top of the result the manager stores. Notifications during
open or apply now fold into the open patch or apply result, and a rejected
apply reports what the agent moved since nothing is stored. Live options
are validated with the creation schema. Tests drive a stdio ACP agent.

packages/server/src/server/agent/providers/acp-agent.test.ts

@@ -4,7 +4,7 @@ import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises";
 import { createRequire } from "node:module";
 import { tmpdir } from "node:os";
 import path from "node:path";
-import { pathToFileURL } from "node:url";
+import { fileURLToPath, pathToFileURL } from "node:url";
 import { afterEach, beforeEach, describe, expect, test, vi } from "vitest";
 import {
   AgentSideConnection,
@@ -301,6 +301,7 @@ function selectConfigOption(
 function createCopilotSessionWithConfig(
   modeId?: string | null,
   providerOptions?: Record<string, unknown>,
+  defaultCommand: string[] = ["copilot", "--acp"],
 ): ACPAgentSession {
   return new ACPAgentSession(
     {
@@ -312,7 +313,7 @@ function createCopilotSessionWithConfig(
     {
       provider: "copilot",
       logger: createTestLogger(),
-      defaultCommand: ["copilot", "--acp"],
+      defaultCommand,
       defaultModes: COPILOT_MODES,
       sessionResponseTransformer: transformCopilotSessionResponse,
       configOptionsTransformer: transformCopilotConfigOptions,
@@ -1802,73 +1803,6 @@ describe("ACPAgentSession Zed parity", () => {
     ]);
   });
 
-  test("a select the provider opened differently from the options is the session's open patch, not an event", async () => {
-    const setSessionConfigOption = vi.fn(async () => ({
-      configOptions: [copilotAgentConfigOption("")],
-    }));
-    const session = createCopilotSessionWithConfig(null, { agent: "Probe Agent" });
-    const { internals } = prepareConfiguredOverrideSession(session, {
-      configOptions: [copilotAgentConfigOption("")],
-      connection: { setSessionConfigOption },
-    });
-    const emitted: AgentStreamEvent[] = [];
-    session.subscribe((event) => emitted.push(event));
-
-    await internals.applyConfiguredOverrides();
-
-    expect(session.openOptionsPatch).toEqual({ agent: "" });
-    expect(emitted.filter((event) => event.type === "options_changed")).toEqual([]);
-  });
-
-  test("applying a patch returns the selects that ended up different and emits nothing", async () => {
-    const setSessionConfigOption = vi.fn(async () => ({
-      configOptions: [copilotAgentConfigOption("")],
-    }));
-    const session = createCopilotSessionWithConfig();
-    prepareConfiguredOverrideSession(session, {
-      configOptions: [copilotAgentConfigOption("")],
-      connection: { setSessionConfigOption },
-    });
-    const emitted: AgentStreamEvent[] = [];
-    session.subscribe((event) => emitted.push(event));
-
-    await expect(session.applyOptions({ agent: "Probe Agent" })).resolves.toEqual({ agent: "" });
-    expect(emitted.filter((event) => event.type === "options_changed")).toEqual([]);
-  });
-
-  test("a removal writes the select's empty choice and returns nothing when that took", async () => {
-    const setSessionConfigOption = vi.fn(async () => ({
-      configOptions: [copilotAgentConfigOption("")],
-    }));
-    const session = createCopilotSessionWithConfig();
-    prepareConfiguredOverrideSession(session, {
-      configOptions: [copilotAgentConfigOption("Probe Agent")],
-      connection: { setSessionConfigOption },
-    });
-
-    await expect(session.applyOptions({ agent: null })).resolves.toEqual({});
-    expect(setSessionConfigOption).toHaveBeenCalledWith(expect.objectContaining({ value: "" }));
-  });
-
-  test("forwards a select the provider changes on its own", async () => {
-    const session = createCopilotSessionWithConfig();
-    prepareConfiguredOverrideSession(session, {
-      configOptions: [copilotAgentConfigOption("Probe Agent")],
-      connection: { setSessionConfigOption: vi.fn() },
-    });
-    const emitted: AgentStreamEvent[] = [];
-    session.subscribe((event) => emitted.push(event));
-
-    asInternals<ACPSessionInternals>(session).translateSessionUpdate({
-      sessionUpdate: "config_option_update",
-      configOptions: [copilotAgentConfigOption("")],
-    });
-
-    expect(emitted.filter((event) => event.type === "options_changed")).toEqual([
-      { type: "options_changed", provider: "copilot", patch: { agent: "" } },
-    ]);
-  });
-
   test("sets Copilot custom agent through ACP config options", async () => {
     const setSessionConfigOption = vi.fn(async () => ({
       configOptions: [copilotAgentConfigOption("Probe Agent")],
@@ -1893,6 +1827,130 @@ describe("ACPAgentSession Zed parity", () => {
   });
 });
 
+describe("ACPAgentSession options against an ACP agent", () => {
+  const fakeAgentPath = fileURLToPath(new URL("./test-utils/fake-acp-agent.mjs", import.meta.url));
+  const sessions: ACPAgentSession[] = [];
+
+  afterEach(async () => {
+    await Promise.all(sessions.splice(0).map((session) => session.close()));
+  });
+
+  function thoughtOption(currentValue: string): SessionConfigOption {
+    return {
+      id: "thought",
+      name: "Thinking",
+      category: "thought_level",
+      type: "select",
+      currentValue,
+      options: [
+        { value: "low", name: "Low" },
+        { value: "high", name: "High" },
+      ],
+    };
+  }
+
+  async function openSession(input: {
+    agent?: string;
+    scenario?: Record<string, unknown>;
+    providerOptions?: Record<string, unknown>;
+  }): Promise<{ session: ACPAgentSession; optionEvents: AgentStreamEvent[] }> {
+    const scenario = {
+      options: [copilotAgentConfigOption(input.agent ?? ""), thoughtOption("low")],
+      ...input.scenario,
+    };
+    await mkdir("/tmp/paseo-acp-test", { recursive: true });
+    const session = createCopilotSessionWithConfig(null, input.providerOptions, [
+      process.execPath,
+      fakeAgentPath,
+      JSON.stringify(scenario),
+    ]);
+    sessions.push(session);
+    const optionEvents: AgentStreamEvent[] = [];
+    session.subscribe((event) => {
+      if (event.type === "options_changed") optionEvents.push(event);
+    });
+    await session.initializeNewSession();
+    return { session, optionEvents };
+  }
+
+  test("a select the agent opened differently from the options is the open patch, not an event", async () => {
+    const { session, optionEvents } = await openSession({
+      providerOptions: { agent: "Probe Agent" },
+      scenario: { answers: { agent: "" } },
+    });
+
+    expect(session.openOptionsPatch).toEqual({ agent: "" });
+    expect(optionEvents).toEqual([]);
+  });
+
+  test("a notification that arrives before the open write is answered is not an event", async () => {
+    const { session, optionEvents } = await openSession({
+      providerOptions: { agent: "Probe Agent" },
+      scenario: { onWrite: "notify-first" },
+    });
+
+    expect(session.openOptionsPatch).toEqual({});
+    expect(optionEvents).toEqual([]);
+  });
+
+  test("applying a patch returns the selects that ended up different and emits nothing", async () => {
+    const { session, optionEvents } = await openSession({ scenario: { answers: { agent: "" } } });
+
+    await expect(session.applyOptions({ agent: "Probe Agent" })).resolves.toEqual({ agent: "" });
+    expect(optionEvents).toEqual([]);
+  });
+
+  test("a notification that arrives before the apply write is answered is not an event", async () => {
+    const { session, optionEvents } = await openSession({ scenario: { onWrite: "notify-first" } });
+
+    await expect(session.applyOptions({ agent: "Probe Agent" })).resolves.toEqual({});
+    expect(optionEvents).toEqual([]);
+  });
+
+  test("a rejected apply reports what the agent moved before refusing, since nothing is stored", async () => {
+    const { session, optionEvents } = await openSession({
+      scenario: { onWrite: "notify-then-reject" },
+    });
+
+    await expect(session.applyOptions({ agent: "Probe Agent" })).rejects.toThrow(/write refused/);
+    expect(optionEvents).toEqual([
+      { type: "options_changed", provider: "copilot", patch: { agent: "Probe Agent" } },
+    ]);
+  });
+
+  test("a removal writes the select's empty choice and returns nothing when that took", async () => {
+    const { session, optionEvents } = await openSession({ agent: "Probe Agent" });
+
+    await expect(session.applyOptions({ agent: null })).resolves.toEqual({});
+    expect(optionEvents).toEqual([]);
+  });
+
+  test("rejects a live auto_accept the creation schema would reject, and writes nothing", async () => {
+    const { session, optionEvents } = await openSession({
+      scenario: { onWrite: "notify-then-reject" },
+    });
+
+    await expect(
+      session.applyOptions({ auto_accept: "yes", agent: "Probe Agent" }),
+    ).rejects.toThrow(/auto_accept/);
+    expect(optionEvents).toEqual([]);
+  });
+
+  test("forwards a select the agent changes on its own", async () => {
+    const { session, optionEvents } = await openSession({
+      scenario: { moves: { thought: { agent: "Probe Agent" } } },
+    });
+
+    await session.setThinkingOption("high");
+
+    await vi.waitFor(() =>
+      expect(optionEvents).toEqual([
+        { type: "options_changed", provider: "copilot", patch: { agent: "Probe Agent" } },
+      ]),
+    );
+  });
+});
+
 describe("deriveModelDefinitionsFromACP", () => {
   test("attaches shared thinking options to ACP model state", () => {
     const result = deriveModelDefinitionsFromACP(

packages/server/src/server/agent/providers/acp-agent.ts

@@ -1,5 +1,6 @@
 import type { ProviderOptions } from "@getpaseo/protocol/agent-types";
 import { ACPProviderOptionsSchema } from "./acp-options.js";
+import { applyProviderOptionsPatch } from "../option-controls.js";
 import { type ChildProcess, type ChildProcessWithoutNullStreams } from "node:child_process";
 import { randomUUID } from "node:crypto";
 import fs from "node:fs/promises";
@@ -1757,6 +1758,8 @@ export class ACPAgentSession implements AgentSession, ACPClient {
   private configOptions: SessionConfigOption[] = [];
   /** How the opened session differs from the options it was given. */
   openOptionsPatch: Record<string, unknown> = {};
+  /** Selects the provider moves while Paseo is opening or applying options belong to that result, not to events. */
+  private optionsTransactionDepth = 0;
   private cachedCommands: AgentSlashCommand[] = [];
   /** When the last command batch arrived; null while nothing has been published. */
   private commandsUpdatedAt: number | null = null;
@@ -2397,19 +2400,31 @@ export class ACPAgentSession implements AgentSession, ACPClient {
     });
   }
 
-  /** Applies only the options the patch sets; returns the selects that ended up different. */
+  /**
+   * Applies only the options the patch sets; returns the selects that ended up different.
+   * Provider notifications that arrive meanwhile are part of that result. When a write is
+   * rejected, what the provider moved is reported as an event, since nothing is stored.
+   */
   async applyOptions(options: ProviderOptions): Promise<Record<string, unknown>> {
+    ACPProviderOptionsSchema.parse(applyProviderOptionsPatch(this.config.providerOptions, options));
     const before = this.featureValues();
     const requested: Record<string, unknown> = {};
-    for (const [id, value] of Object.entries(options)) {
-      const featureOption = this.configFeatureOptions.find((candidate) => candidate.id === id);
-      if (!featureOption) continue;
-      // A removal asks for the select's empty choice; a provider without one rejects it.
-      requested[id] = value ?? "";
-      const option = findSelectConfigFeatureOption(this.configOptions, featureOption);
-      if (!option || requested[id] !== option.currentValue) {
-        await this.setConfigFeature(id, value);
-      }
+    try {
+      await this.inOptionsTransaction(async () => {
+        for (const [id, value] of Object.entries(options)) {
+          const featureOption = this.configFeatureOptions.find((candidate) => candidate.id === id);
+          if (!featureOption) continue;
+          // A removal asks for the select's empty choice; a provider without one rejects it.
+          requested[id] = value ?? "";
+          const option = findSelectConfigFeatureOption(this.configOptions, featureOption);
+          if (!option || requested[id] !== option.currentValue) {
+            await this.setConfigFeature(id, value);
+          }
+        }
+      });
+    } catch (error) {
+      this.reportFeatureChanges(before);
+      throw error;
     }
     // Auto Accept is Paseo's own setting, so it changes only once every write has been accepted.
     if (ACP_AUTO_ACCEPT_FEATURE_ID in options) {
@@ -2421,6 +2436,15 @@ export class ACPAgentSession implements AgentSession, ACPClient {
     return this.featureDifferences({ ...before, ...requested });
   }
 
+  private async inOptionsTransaction<T>(run: () => Promise<T>): Promise<T> {
+    this.optionsTransactionDepth += 1;
+    try {
+      return await run();
+    } finally {
+      this.optionsTransactionDepth -= 1;
+    }
+  }
+
   /** The selects as the provider reports them now. */
   private featureValues(): Record<string, string> {
     const values: Record<string, string> = {};
@@ -2445,6 +2469,7 @@ export class ACPAgentSession implements AgentSession, ACPClient {
 
   /** Reports the selects the provider moved on its own, compared with their values `before`. */
   private reportFeatureChanges(before: Record<string, string>): void {
+    if (this.optionsTransactionDepth > 0) return;
     const patch = this.featureDifferences(before);
     if (Object.keys(patch).length > 0) {
       this.pushEvent({ type: "options_changed", provider: this.provider, patch });
@@ -3041,7 +3066,11 @@ export class ACPAgentSession implements AgentSession, ACPClient {
     return this.modeIdTransformer ? this.modeIdTransformer(modeId) : modeId;
   }
 
-  private async applyConfiguredOverrides(): Promise<void> {
+  private applyConfiguredOverrides(): Promise<void> {
+    return this.inOptionsTransaction(() => this.writeConfiguredOverrides());
+  }
+
+  private async writeConfiguredOverrides(): Promise<void> {
     const configuredModeId = this.config.modeId;
     if (configuredModeId && configuredModeId !== this.currentMode) {
       const selection = resolveACPModeSelection({

packages/server/src/server/agent/providers/acp-options.ts

@@ -2,6 +2,8 @@ import { z } from "zod";
 
 export const ACPProviderOptionsSchema = z
   .object({
+    // Composer control: approves permission prompts without asking.
+    auto_accept: z.boolean().optional(),
     supportsMcpServers: z.boolean().optional(),
     clientCapabilities: z
       .object({

packages/server/src/server/agent/providers/test-utils/fake-acp-agent.mjs

@@ -0,0 +1,57 @@
+// A stdio ACP agent for tests. argv[2] is a JSON scenario:
+//   options: the session's config options
+//   answers: { [configId]: value } the agent ends up with instead of the requested value
+//   onWrite: "notify-first" sends config_option_update before answering the write;
+//            "notify-then-reject" does the same and then fails the write
+//   moves:   { [configId]: { [id]: value } } writes that also move another select, announced
+//            by a config_option_update after the write is answered
+import { Readable, Writable } from "node:stream";
+import { AgentSideConnection, ndJsonStream } from "@agentclientprotocol/sdk";
+
+const scenario = JSON.parse(process.argv[2] ?? "{}");
+let options = scenario.options ?? [];
+
+function set(id, value) {
+  options = options.map((option) =>
+    option.id === id ? { ...option, currentValue: value } : option,
+  );
+}
+
+const stream = ndJsonStream(Writable.toWeb(process.stdout), Readable.toWeb(process.stdin));
+export const connection = new AgentSideConnection(
+  (agentConnection) => ({
+    async initialize() {
+      return { protocolVersion: 1, agentCapabilities: {} };
+    },
+    async newSession() {
+      return { sessionId: "session-1", modes: null, models: null, configOptions: options };
+    },
+    async setSessionConfigOption({ sessionId, configId, value }) {
+      set(configId, scenario.answers?.[configId] ?? value);
+      const announce = () =>
+        agentConnection.sessionUpdate({
+          sessionId,
+          update: { sessionUpdate: "config_option_update", configOptions: options },
+        });
+      if (scenario.onWrite === "notify-first" || scenario.onWrite === "notify-then-reject") {
+        await announce();
+        if (scenario.onWrite === "notify-then-reject") throw new Error("write refused");
+        return { configOptions: options };
+      }
+      const moves = scenario.moves?.[configId];
+      if (moves) {
+        for (const [id, moved] of Object.entries(moves)) set(id, moved);
+        setTimeout(() => void announce(), 20);
+      }
+      return { configOptions: options };
+    },
+    async authenticate() {
+      return {};
+    },
+    async prompt() {
+      return { stopReason: "end_turn" };
+    },
+    async cancel() {},
+  }),
+  stream,
+);