From 5c8af1a1faf6331ece2b47c9867fe4bf6ce3b348 Mon Sep 17 00:00:00 2001 From: jinye Date: Mon, 6 Jul 2026 15:42:46 +0800 Subject: [PATCH] fix(cli): allow ACP local fallback reads from /tmp (#6370) Add POSIX /tmp to ACP local read fallback roots without changing read_file's default permission behavior. Also add QWEN_ACP_LOCAL_READ_ROOTS as an append-only absolute-path override for ACP fallback reads. Co-authored-by: Qwen-Coder --- .../cli/src/acp-integration/acpAgent.test.ts | 187 +++++++++++------- packages/cli/src/acp-integration/acpAgent.ts | 46 +++-- packages/core/src/tools/read-file.ts | 5 +- 3 files changed, 159 insertions(+), 79 deletions(-) diff --git a/packages/cli/src/acp-integration/acpAgent.test.ts b/packages/cli/src/acp-integration/acpAgent.test.ts index 2422b09ca0..631b5586cf 100644 --- a/packages/cli/src/acp-integration/acpAgent.test.ts +++ b/packages/cli/src/acp-integration/acpAgent.test.ts @@ -1230,6 +1230,7 @@ describe('QwenAgent MCP SSE/HTTP support', () => { let stdoutDestroySpy: MockInstance; const mockArgv = {} as CliArgs; + const acpLocalReadRootsEnv = 'QWEN_ACP_LOCAL_READ_ROOTS'; beforeEach(() => { vi.clearAllMocks(); @@ -1327,75 +1328,39 @@ describe('QwenAgent MCP SSE/HTTP support', () => { }); it('configures ACP file system fallback roots for read_file allowed local roots', async () => { - const fsCapabilities = { readTextFile: true, writeTextFile: true }; - const fallbackFileSystem = {}; - const innerConfig = { - ...makeInnerConfig(), - getTargetDir: vi.fn().mockReturnValue('/project'), - getSessionId: vi.fn().mockReturnValue('session-with-fs'), - getFileSystemService: vi.fn().mockReturnValue(fallbackFileSystem), - setFileSystemService: vi.fn(), - storage: { - getProjectTempDir: vi.fn().mockReturnValue('/project/.qwen/tmp'), - getProjectDir: vi.fn().mockReturnValue('/project'), - getUserSkillsDirs: vi.fn().mockReturnValue(['/home/test/.qwen/skills']), - }, - }; - vi.mocked(loadSettings).mockReturnValue(makeSessionSettings()); - vi.mocked(loadCliConfig).mockResolvedValue( - innerConfig as unknown as Config, - ); - vi.mocked(Session).mockImplementation( - () => - ({ - getId: vi.fn().mockReturnValue('session-with-fs'), - getConfig: vi.fn().mockReturnValue(innerConfig), - sendAvailableCommandsUpdate: vi.fn().mockResolvedValue(undefined), - replayHistory: vi.fn().mockResolvedValue(undefined), - installRewriter: vi.fn(), - startCronScheduler: vi.fn(), - dispose: vi.fn(), - }) as unknown as InstanceType, - ); + const previousRoots = process.env[acpLocalReadRootsEnv]; + delete process.env[acpLocalReadRootsEnv]; - const agentPromise = runAcpAgent( - mockConfig, - makeSessionSettings(), - mockArgv, - ); - await vi.waitFor(() => expect(capturedAgentFactory).toBeDefined()); + try { + await expectAcpLocalReadRoots( + 'session-with-fs', + expectedDefaultAcpLocalReadRoots(), + ); + } finally { + restoreOptionalEnv(acpLocalReadRootsEnv, previousRoots); + } + }); - const fakeConn = { - get closed() { - return mockConnectionState.promise; - }, - } as AgentSideConnectionLike; - const agent = capturedAgentFactory!(fakeConn) as AgentLike; + it('appends QWEN_ACP_LOCAL_READ_ROOTS absolute entries to ACP file system fallback roots', async () => { + const previousRoots = process.env[acpLocalReadRootsEnv]; + const envRootA = path.resolve('/custom/acp-a'); + const envRootB = path.resolve('/custom/acp-b'); + process.env[acpLocalReadRootsEnv] = [ + '', + ` ${envRootA} `, + 'relative-acp-root', + envRootB, + ' ', + ].join(path.delimiter); - await agent.initialize({ clientCapabilities: { fs: fsCapabilities } }); - await agent.newSession({ cwd: '/project', mcpServers: [] }); - - expect(AcpFileSystemService).toHaveBeenCalledWith( - fakeConn, - 'session-with-fs', - fsCapabilities, - fallbackFileSystem, - { - localReadRoots: [ - '/project/.qwen/tmp', - path.join('/project', 'subagents'), - '/tmp/qwen-global-temp', - '/project/.qwen/memory', - '/tmp/user-memory', - '/home/test/.qwen/skills', - '/tmp/qwen-extensions', - ], - }, - ); - expect(innerConfig.setFileSystemService).toHaveBeenCalled(); - - mockConnectionState.resolve(); - await agentPromise; + try { + await expectAcpLocalReadRoots( + 'session-with-fs-env', + [...expectedDefaultAcpLocalReadRoots(), envRootA, envRootB], + ); + } finally { + restoreOptionalEnv(acpLocalReadRootsEnv, previousRoots); + } }); it('passes each concurrent newSession its own workspace settings instance', async () => { @@ -1608,6 +1573,96 @@ describe('QwenAgent MCP SSE/HTTP support', () => { }; } + function expectedDefaultAcpLocalReadRoots(): string[] { + return [ + '/project/.qwen/tmp', + path.join('/project', 'subagents'), + '/tmp/qwen-global-temp', + '/project/.qwen/memory', + '/tmp/user-memory', + '/home/test/.qwen/skills', + '/tmp/qwen-extensions', + ...(process.platform === 'win32' ? [] : ['/tmp']), + ]; + } + + function restoreOptionalEnv(key: string, value: string | undefined): void { + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + } + + async function expectAcpLocalReadRoots( + sessionId: string, + expectedLocalReadRoots: string[], + ): Promise { + const fsCapabilities = { readTextFile: true, writeTextFile: true }; + const fallbackFileSystem: Record = {}; + const innerConfig = { + ...makeInnerConfig(), + getTargetDir: vi.fn().mockReturnValue('/project'), + getSessionId: vi.fn().mockReturnValue(sessionId), + getFileSystemService: vi.fn().mockReturnValue(fallbackFileSystem), + setFileSystemService: vi.fn(), + storage: { + getProjectTempDir: vi.fn().mockReturnValue('/project/.qwen/tmp'), + getProjectDir: vi.fn().mockReturnValue('/project'), + getUserSkillsDirs: vi.fn().mockReturnValue(['/home/test/.qwen/skills']), + }, + }; + vi.mocked(loadSettings).mockReturnValue(makeSessionSettings()); + vi.mocked(loadCliConfig).mockResolvedValue( + innerConfig as unknown as Config, + ); + vi.mocked(Session).mockImplementation( + () => + ({ + getId: vi.fn().mockReturnValue(sessionId), + getConfig: vi.fn().mockReturnValue(innerConfig), + sendAvailableCommandsUpdate: vi.fn().mockResolvedValue(undefined), + replayHistory: vi.fn().mockResolvedValue(undefined), + installRewriter: vi.fn(), + startCronScheduler: vi.fn(), + dispose: vi.fn(), + }) as unknown as InstanceType, + ); + + const agentPromise = runAcpAgent( + mockConfig, + makeSessionSettings(), + mockArgv, + ); + await vi.waitFor(() => expect(capturedAgentFactory).toBeDefined()); + + try { + const fakeConn = { + get closed() { + return mockConnectionState.promise; + }, + } as AgentSideConnectionLike; + const agent = capturedAgentFactory!(fakeConn) as AgentLike; + + await agent.initialize({ clientCapabilities: { fs: fsCapabilities } }); + await agent.newSession({ cwd: '/project', mcpServers: [] }); + + expect(AcpFileSystemService).toHaveBeenCalledWith( + fakeConn, + sessionId, + fsCapabilities, + fallbackFileSystem, + { + localReadRoots: expectedLocalReadRoots, + }, + ); + expect(innerConfig.setFileSystemService).toHaveBeenCalled(); + } finally { + mockConnectionState.resolve(); + await agentPromise; + } + } + function makeSessionSettings() { return { merged: { mcpServers: {} }, diff --git a/packages/cli/src/acp-integration/acpAgent.ts b/packages/cli/src/acp-integration/acpAgent.ts index e6aa73e3a0..f95b761612 100644 --- a/packages/cli/src/acp-integration/acpAgent.ts +++ b/packages/cli/src/acp-integration/acpAgent.ts @@ -263,12 +263,46 @@ import { import type { HistoryItemContextUsage } from '../ui/types.js'; const debugLogger = createDebugLogger('ACP_AGENT'); +const QWEN_ACP_LOCAL_READ_ROOTS_ENV = 'QWEN_ACP_LOCAL_READ_ROOTS'; +const POSIX_TMP_LOCAL_READ_ROOT = '/tmp'; // Must be less than SESSION_BTW_TIMEOUT_MS (60s) in bridge.ts so the child // aborts before the bridge's backstop timer fires. const BTW_CHILD_TIMEOUT_MS = 55_000; // Must be less than WORKSPACE_MEMORY_REMEMBER_TIMEOUT_MS (300s) in bridge.ts. const WORKSPACE_MEMORY_REMEMBER_CHILD_TIMEOUT_MS = 295_000; +function parseAcpLocalReadRootsEnv( + raw = process.env[QWEN_ACP_LOCAL_READ_ROOTS_ENV], +): string[] { + if (!raw) return []; + + return raw + .split(path.delimiter) + .map((entry) => entry.trim()) + .filter((entry) => entry.length > 0 && path.isAbsolute(entry)); +} + +function defaultAcpOnlyLocalReadRoots(): string[] { + return process.platform === 'win32' ? [] : [POSIX_TMP_LOCAL_READ_ROOT]; +} + +function buildAcpLocalReadRoots(config: Config): string[] { + return [ + // SYNC: The first group mirrors ReadFileTool's default allowed local roots, + // including auto-memory roots. The ACP-only additions below expand only + // local read fallback, not read_file's default permission. + config.storage.getProjectTempDir(), + path.join(config.storage.getProjectDir(), 'subagents'), + Storage.getGlobalTempDir(), + getAutoMemoryRoot(config.getTargetDir()), + getUserAutoMemoryRoot(), + ...config.storage.getUserSkillsDirs(), + Storage.getUserExtensionsDir(), + ...defaultAcpOnlyLocalReadRoots(), + ...parseAcpLocalReadRootsEnv(), + ]; +} + function isObjectRecord(value: unknown): value is Record { return typeof value === 'object' && value !== null && !Array.isArray(value); } @@ -8162,17 +8196,7 @@ class QwenAgent implements Agent { this.clientCapabilities.fs, config.getFileSystemService(), { - // SYNC: Mirrors ReadFileTool's default allowed local roots, including - // auto-memory roots, so ACP-local read fallback follows the same policy. - localReadRoots: [ - config.storage.getProjectTempDir(), - path.join(config.storage.getProjectDir(), 'subagents'), - Storage.getGlobalTempDir(), - getAutoMemoryRoot(config.getTargetDir()), - getUserAutoMemoryRoot(), - ...config.storage.getUserSkillsDirs(), - Storage.getUserExtensionsDir(), - ], + localReadRoots: buildAcpLocalReadRoots(config), }, ); config.setFileSystemService(acpFileSystemService); diff --git a/packages/core/src/tools/read-file.ts b/packages/core/src/tools/read-file.ts index 93c368f572..4dd0b53e7d 100644 --- a/packages/core/src/tools/read-file.ts +++ b/packages/core/src/tools/read-file.ts @@ -106,8 +106,9 @@ class ReadFileToolInvocation extends BaseToolInvocation< const filePath = path.resolve(this.params.file_path); const workspaceContext = this.config.getWorkspaceContext(); - // SYNC: Keep these roots and the auto-memory check below aligned with - // AcpAgent.setupFileSystem's localReadRoots. + // SYNC: Keep these base roots and the auto-memory check below aligned with + // AcpAgent.buildAcpLocalReadRoots' mirrored ReadFileTool group. ACP may + // append fallback-only roots after that group. const allowedRoots = [ this.config.storage.getProjectTempDir(), // Background subagent transcripts live under /subagents/ and