fix(acp): align permission flow across clients

packages/cli/src/acp-integration/acpAgent.ts

@@ -57,7 +57,7 @@ import { buildAuthMethods } from './authMethods.js';
 import { AcpFileSystemService } from './service/filesystem.js';
 import { Readable, Writable } from 'node:stream';
 import type { LoadedSettings } from '../config/settings.js';
-import { SettingScope } from '../config/settings.js';
+import { loadSettings, SettingScope } from '../config/settings.js';
 import type { ApprovalModeValue } from './session/types.js';
 import { z } from 'zod';
 import type { CliArgs } from '../config/config.js';
@@ -223,30 +223,18 @@ class QwenAgent implements Agent {
         return sessionService.sessionExists(params.sessionId);
       },
     );
-    if (!exists) {
-      throw RequestError.invalidParams(
-        undefined,
-        `Session not found for id: ${params.sessionId}`,
-      );
-    }
 
     const config = await this.newSessionConfig(
       params.cwd,
       params.mcpServers,
       params.sessionId,
+      exists,
     );
     await this.ensureAuthenticated(config);
     this.setupFileSystem(config);
 
     const sessionData = config.getResumedSessionData();
-    if (!sessionData) {
-      throw RequestError.internalError(
-        undefined,
-        `Failed to load session data for id: ${params.sessionId}`,
-      );
-    }
-
-    await this.createAndStoreSession(config, sessionData.conversation);
+    await this.createAndStoreSession(config, sessionData?.conversation);
 
     const modesData = this.buildModesData(config);
     const availableModels = this.buildAvailableModels(config);
@@ -380,7 +368,9 @@ class QwenAgent implements Agent {
     cwd: string,
     mcpServers: McpServer[],
     sessionId?: string,
+    resume?: boolean,
   ): Promise<Config> {
+    this.settings = loadSettings(cwd);
     const mergedMcpServers = { ...this.settings.merged.mcpServers };
 
     for (const server of mcpServers) {
@@ -402,11 +392,11 @@ class QwenAgent implements Agent {
     const settings = { ...this.settings.merged, mcpServers: mergedMcpServers };
     const argvForSession = {
       ...this.argv,
-      resume: sessionId,
+      ...(resume ? { resume: sessionId } : { sessionId }),
       continue: false,
     };
 
-    const config = await loadCliConfig(settings, argvForSession, cwd);
+    const config = await loadCliConfig(settings, argvForSession, cwd, []);
     await config.initialize();
     return config;
   }

packages/cli/src/acp-integration/session/Session.test.ts

@@ -34,6 +34,7 @@ describe('Session', () => {
   let currentAuthType: AuthType;
   let switchModelSpy: ReturnType<typeof vi.fn>;
   let getAvailableCommandsSpy: ReturnType<typeof vi.fn>;
+  let mockToolRegistry: { getTool: ReturnType<typeof vi.fn> };
 
   beforeEach(() => {
     currentModel = 'qwen3-code-plus';
@@ -50,7 +51,7 @@ describe('Session', () => {
       addHistory: vi.fn(),
     } as unknown as GeminiChat;
 
-    const toolRegistry = { getTool: vi.fn() };
+    mockToolRegistry = { getTool: vi.fn() };
     const fileService = { shouldGitIgnoreFile: vi.fn().mockReturnValue(false) };
 
     mockConfig = {
@@ -65,8 +66,9 @@ describe('Session', () => {
       getChatRecordingService: vi.fn().mockReturnValue({
         recordUserMessage: vi.fn(),
         recordUiTelemetryEvent: vi.fn(),
+        recordToolResult: vi.fn(),
       }),
-      getToolRegistry: vi.fn().mockReturnValue(toolRegistry),
+      getToolRegistry: vi.fn().mockReturnValue(mockToolRegistry),
       getFileService: vi.fn().mockReturnValue(fileService),
       getFileFilteringRespectGitIgnore: vi.fn().mockReturnValue(true),
       getEnableRecursiveFileSearch: vi.fn().mockReturnValue(false),
@@ -275,5 +277,204 @@ describe('Session', () => {
         expect.any(Function),
       );
     });
+
+    it('hides allow-always options when confirmation already forbids them', async () => {
+      const executeSpy = vi.fn().mockResolvedValue({
+        llmContent: 'ok',
+        returnDisplay: 'ok',
+      });
+      const onConfirmSpy = vi.fn().mockResolvedValue(undefined);
+      const invocation = {
+        params: { path: '/tmp/file.txt' },
+        getDefaultPermission: vi.fn().mockResolvedValue('ask'),
+        getConfirmationDetails: vi.fn().mockResolvedValue({
+          type: 'info',
+          title: 'Need permission',
+          prompt: 'Allow?',
+          hideAlwaysAllow: true,
+          onConfirm: onConfirmSpy,
+        }),
+        getDescription: vi.fn().mockReturnValue('Inspect file'),
+        toolLocations: vi.fn().mockReturnValue([]),
+        execute: executeSpy,
+      };
+      const tool = {
+        name: 'read_file',
+        kind: core.Kind.Read,
+        build: vi.fn().mockReturnValue(invocation),
+      };
+
+      mockToolRegistry.getTool.mockReturnValue(tool);
+      mockConfig.getApprovalMode = vi
+        .fn()
+        .mockReturnValue(ApprovalMode.DEFAULT);
+      mockConfig.getPermissionManager = vi.fn().mockReturnValue(null);
+      mockChat.sendMessageStream = vi.fn().mockResolvedValue(
+        (async function* () {
+          yield {
+            type: core.StreamEventType.CHUNK,
+            value: {
+              functionCalls: [
+                {
+                  id: 'call-1',
+                  name: 'read_file',
+                  args: { path: '/tmp/file.txt' },
+                },
+              ],
+            },
+          };
+        })(),
+      );
+
+      await session.prompt({
+        sessionId: 'test-session-id',
+        prompt: [{ type: 'text', text: 'run tool' }],
+      });
+
+      expect(mockClient.requestPermission).toHaveBeenCalledWith(
+        expect.objectContaining({
+          options: [
+            expect.objectContaining({ kind: 'allow_once' }),
+            expect.objectContaining({ kind: 'reject_once' }),
+          ],
+        }),
+      );
+      const options = (mockClient.requestPermission as ReturnType<typeof vi.fn>)
+        .mock.calls[0][0].options as Array<{ kind: string }>;
+      expect(options.some((option) => option.kind === 'allow_always')).toBe(
+        false,
+      );
+    });
+
+    it('returns permission error for disabled tools (L1 isToolEnabled check)', async () => {
+      const executeSpy = vi.fn();
+      const invocation = {
+        params: { path: '/tmp/file.txt' },
+        getDefaultPermission: vi.fn().mockResolvedValue('ask'),
+        getConfirmationDetails: vi.fn().mockResolvedValue({
+          type: 'info',
+          title: 'Need permission',
+          prompt: 'Allow?',
+          onConfirm: vi.fn(),
+        }),
+        getDescription: vi.fn().mockReturnValue('Write file'),
+        toolLocations: vi.fn().mockReturnValue([]),
+        execute: executeSpy,
+      };
+      const tool = {
+        name: 'write_file',
+        kind: core.Kind.Edit,
+        build: vi.fn().mockReturnValue(invocation),
+      };
+
+      mockToolRegistry.getTool.mockReturnValue(tool);
+      mockConfig.getApprovalMode = vi
+        .fn()
+        .mockReturnValue(ApprovalMode.DEFAULT);
+      // Mock a PermissionManager that denies the tool
+      mockConfig.getPermissionManager = vi.fn().mockReturnValue({
+        isToolEnabled: vi.fn().mockResolvedValue(false),
+      });
+      mockChat.sendMessageStream = vi.fn().mockResolvedValue(
+        (async function* () {
+          yield {
+            type: core.StreamEventType.CHUNK,
+            value: {
+              functionCalls: [
+                {
+                  id: 'call-denied',
+                  name: 'write_file',
+                  args: { path: '/tmp/file.txt' },
+                },
+              ],
+            },
+          };
+        })(),
+      );
+
+      await session.prompt({
+        sessionId: 'test-session-id',
+        prompt: [{ type: 'text', text: 'write something' }],
+      });
+
+      // Tool should NOT have been executed
+      expect(executeSpy).not.toHaveBeenCalled();
+      // No permission dialog should have been opened
+      expect(mockClient.requestPermission).not.toHaveBeenCalled();
+    });
+
+    it('respects permission-request hook allow decisions without opening ACP permission dialog', async () => {
+      const hookSpy = vi
+        .spyOn(core, 'firePermissionRequestHook')
+        .mockResolvedValue({
+          hasDecision: true,
+          shouldAllow: true,
+          updatedInput: { path: '/tmp/updated.txt' },
+          denyMessage: undefined,
+        });
+      const executeSpy = vi.fn().mockResolvedValue({
+        llmContent: 'ok',
+        returnDisplay: 'ok',
+      });
+      const onConfirmSpy = vi.fn().mockResolvedValue(undefined);
+      const invocation = {
+        params: { path: '/tmp/original.txt' },
+        getDefaultPermission: vi.fn().mockResolvedValue('ask'),
+        getConfirmationDetails: vi.fn().mockResolvedValue({
+          type: 'info',
+          title: 'Need permission',
+          prompt: 'Allow?',
+          onConfirm: onConfirmSpy,
+        }),
+        getDescription: vi.fn().mockReturnValue('Inspect file'),
+        toolLocations: vi.fn().mockReturnValue([]),
+        execute: executeSpy,
+      };
+      const tool = {
+        name: 'read_file',
+        kind: core.Kind.Read,
+        build: vi.fn().mockReturnValue(invocation),
+      };
+
+      mockToolRegistry.getTool.mockReturnValue(tool);
+      mockConfig.getApprovalMode = vi
+        .fn()
+        .mockReturnValue(ApprovalMode.DEFAULT);
+      mockConfig.getPermissionManager = vi.fn().mockReturnValue(null);
+      mockConfig.getEnableHooks = vi.fn().mockReturnValue(true);
+      mockConfig.getMessageBus = vi.fn().mockReturnValue({});
+      mockChat.sendMessageStream = vi.fn().mockResolvedValue(
+        (async function* () {
+          yield {
+            type: core.StreamEventType.CHUNK,
+            value: {
+              functionCalls: [
+                {
+                  id: 'call-2',
+                  name: 'read_file',
+                  args: { path: '/tmp/original.txt' },
+                },
+              ],
+            },
+          };
+        })(),
+      );
+
+      try {
+        await session.prompt({
+          sessionId: 'test-session-id',
+          prompt: [{ type: 'text', text: 'run tool' }],
+        });
+      } finally {
+        hookSpy.mockRestore();
+      }
+
+      expect(mockClient.requestPermission).not.toHaveBeenCalled();
+      expect(onConfirmSpy).toHaveBeenCalledWith(
+        core.ToolConfirmationOutcome.ProceedOnce,
+      );
+      expect(invocation.params).toEqual({ path: '/tmp/updated.txt' });
+      expect(executeSpy).toHaveBeenCalled();
+    });
   });
 });