Let Copilot implement plans after selecting Allow All (#7234)

Selecting Allow All now moves Copilot to Agent mode before enabling allow_all, so a session that was in Plan can implement the plan.

Closes #3855.

Co-authored-by: KiraPC <pasqualecarmine.carbone@gmail.com>

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

@@ -1617,7 +1617,12 @@ describe("ACPAgentSession Zed parity", () => {
       configId: "allow_all",
       value: "on",
     });
-    expect(setSessionMode).not.toHaveBeenCalled();
+    // Allow All is agent mode with permissions pre-granted, so it has to leave
+    // whatever mode the session is in, not just flip the flag.
+    expect(setSessionMode).toHaveBeenCalledWith({
+      sessionId: "session-1",
+      modeId: "https://agentclientprotocol.com/protocol/session-modes#agent",
+    });
     await expect(session.getCurrentMode()).resolves.toBe(COPILOT_ALLOW_ALL_MODE_ID);
     expect(events.some((event) => event.type === "permission_requested")).toBe(false);
   });
@@ -1648,7 +1653,12 @@ describe("ACPAgentSession Zed parity", () => {
       configId: "allow_all",
       value: "on",
     });
-    expect(setSessionMode).not.toHaveBeenCalled();
+    // Allow All is agent mode with permissions pre-granted, so it has to leave
+    // whatever mode the session is in, not just flip the flag.
+    expect(setSessionMode).toHaveBeenCalledWith({
+      sessionId: "session-1",
+      modeId: "https://agentclientprotocol.com/protocol/session-modes#agent",
+    });
     await expect(session.getCurrentMode()).resolves.toBe(COPILOT_ALLOW_ALL_MODE_ID);
   });
 

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

@@ -209,6 +209,10 @@ export async function writeCopilotProviderMode(
   if (!requestsAllowAll) {
     return { handled: false };
   }
+  await context.connection.setSessionMode({
+    sessionId: context.sessionId,
+    modeId: COPILOT_AGENT_MODE_ID,
+  });
   const response = await context.connection.setSessionConfigOption({
     sessionId: context.sessionId,
     configId: COPILOT_ALLOW_ALL_CONFIG_ID,

packages/server/src/server/agent/providers/copilot-allow-all-mode.real.e2e.test.ts

@@ -0,0 +1,82 @@
+import { beforeAll, describe, expect, test } from "vitest";
+import { spawn } from "node:child_process";
+import { mkdtemp, rm } from "node:fs/promises";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { Readable, Writable } from "node:stream";
+import { ClientSideConnection, ndJsonStream, PROTOCOL_VERSION } from "@agentclientprotocol/sdk";
+
+import { isCommandAvailable } from "../../../executable-resolution/executable-resolution.js";
+import { createTestLogger } from "../../../test-utils/test-logger.js";
+import { terminateWithTreeKill } from "../../../utils/tree-kill.js";
+import { resolveACPModeSelection } from "./acp-agent.js";
+import {
+  COPILOT_ALLOW_ALL_MODE_ID,
+  COPILOT_MODES,
+  writeCopilotProviderMode,
+} from "./copilot-acp-agent.js";
+
+const AGENT_MODE_ID = "https://agentclientprotocol.com/protocol/session-modes#agent";
+const PLAN_MODE_ID = "https://agentclientprotocol.com/protocol/session-modes#plan";
+
+describe("Copilot ACP provider (real) Allow All mode", () => {
+  let canRun = false;
+
+  beforeAll(async () => {
+    canRun = await isCommandAvailable("copilot");
+  });
+
+  test("selecting Allow All from Plan leaves the real provider in Agent mode", async (context) => {
+    if (!canRun) {
+      context.skip();
+    }
+    const cwd = await mkdtemp(join(tmpdir(), "copilot-allow-all-real-"));
+    const child = spawn("copilot", ["--acp"], { cwd, stdio: ["pipe", "pipe", "ignore"] });
+    const connection = new ClientSideConnection(
+      () => ({
+        sessionUpdate: async () => {},
+        requestPermission: async () => ({ outcome: { outcome: "cancelled" } }),
+      }),
+      ndJsonStream(Writable.toWeb(child.stdin), Readable.toWeb(child.stdout)),
+    );
+
+    try {
+      await connection.initialize({ protocolVersion: PROTOCOL_VERSION, clientCapabilities: {} });
+      const session = await connection.newSession({ cwd, mcpServers: [] });
+      const plan = await connection.setSessionConfigOption({
+        sessionId: session.sessionId,
+        configId: "mode",
+        value: PLAN_MODE_ID,
+      });
+      expect(plan.configOptions).toContainEqual(
+        expect.objectContaining({ id: "mode", currentValue: PLAN_MODE_ID }),
+      );
+
+      // This is the same provider writer called by ACPAgentSession. Assert its
+      // untransformed ACP response, since Paseo's displayed mode hides this bug.
+      const result = await writeCopilotProviderMode({
+        connection,
+        sessionId: session.sessionId,
+        requestedModeId: COPILOT_ALLOW_ALL_MODE_ID,
+        currentModeId: PLAN_MODE_ID,
+        selection: resolveACPModeSelection({
+          modeId: COPILOT_ALLOW_ALL_MODE_ID,
+          availableModes: COPILOT_MODES,
+          configOptions: plan.configOptions,
+        }),
+        configOptions: plan.configOptions,
+        logger: createTestLogger(),
+      });
+
+      expect(result.configOptions).toContainEqual(
+        expect.objectContaining({ id: "mode", currentValue: AGENT_MODE_ID }),
+      );
+      expect(result.configOptions).toContainEqual(
+        expect.objectContaining({ id: "allow_all", currentValue: "on" }),
+      );
+    } finally {
+      await terminateWithTreeKill(child, { gracefulTimeoutMs: 1000, forceTimeoutMs: 1000 });
+      await rm(cwd, { recursive: true, force: true });
+    }
+  }, 60_000);
+});