From e9654cdb51778726723150871a80625cbbbfd957 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Mon, 29 Jun 2026 21:56:02 +0800 Subject: [PATCH 01/18] fix(cli): handle ACP read_file local roots Allow ACP read_file calls to fall back to local reads for explicitly permitted local roots when the serve workspace boundary rejects them, and preserve useful messages for plain object read errors. Co-authored-by: Qwen-Coder --- packages/cli/src/acp-integration/acpAgent.ts | 14 ++ .../service/filesystem.test.ts | 134 +++++++++++++++++- .../src/acp-integration/service/filesystem.ts | 70 ++++++++- packages/core/src/utils/errors.test.ts | 14 ++ packages/core/src/utils/errors.ts | 11 ++ packages/core/src/utils/fileUtils.test.ts | 23 +++ packages/core/src/utils/fileUtils.ts | 4 +- 7 files changed, 266 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/acp-integration/acpAgent.ts b/packages/cli/src/acp-integration/acpAgent.ts index 76c96e27f03..d737d8c4c17 100644 --- a/packages/cli/src/acp-integration/acpAgent.ts +++ b/packages/cli/src/acp-integration/acpAgent.ts @@ -20,6 +20,7 @@ import { findProviderById, getAllGeminiMdFilenames, getAutoMemoryRoot, + getUserAutoMemoryRoot, getDefaultBaseUrlForProtocol, getDefaultModelIds, getScopedEnvContents, @@ -130,6 +131,7 @@ import { pipeline } from 'node:stream/promises'; import * as fs from 'node:fs/promises'; import { createRequire } from 'node:module'; import * as path from 'node:path'; +import * as os from 'node:os'; import { createGunzip } from 'node:zlib'; import type { LoadedSettings } from '../config/settings.js'; import { @@ -7627,6 +7629,18 @@ class QwenAgent implements Agent { config.getSessionId(), this.clientCapabilities.fs, config.getFileSystemService(), + { + localReadRoots: [ + config.storage.getProjectTempDir(), + path.join(config.storage.getProjectDir(), 'subagents'), + Storage.getGlobalTempDir(), + os.tmpdir(), + getAutoMemoryRoot(config.getTargetDir()), + getUserAutoMemoryRoot(), + ...config.storage.getUserSkillsDirs(), + Storage.getUserExtensionsDir(), + ], + }, ); config.setFileSystemService(acpFileSystemService); } diff --git a/packages/cli/src/acp-integration/service/filesystem.test.ts b/packages/cli/src/acp-integration/service/filesystem.test.ts index c6b60abae5f..92176b250cb 100644 --- a/packages/cli/src/acp-integration/service/filesystem.test.ts +++ b/packages/cli/src/acp-integration/service/filesystem.test.ts @@ -8,6 +8,8 @@ import { describe, expect, it, vi } from 'vitest'; import type { FileSystemService } from '@qwen-code/qwen-code-core'; import { AcpFileSystemService } from './filesystem.js'; import type { AgentSideConnection } from '@agentclientprotocol/sdk'; +import os from 'node:os'; +import path from 'node:path'; const RESOURCE_NOT_FOUND_CODE = -32002; const INTERNAL_ERROR_CODE = -32603; @@ -73,7 +75,7 @@ describe('AcpFileSystemService', () => { }); }); - it('re-throws other errors unchanged', async () => { + it('preserves code and message for other read errors', async () => { const otherError = { code: INTERNAL_ERROR_CODE, message: 'Internal error', @@ -97,6 +99,136 @@ describe('AcpFileSystemService', () => { }); }); + it('normalizes plain object ACP errors to Error instances with the original message', async () => { + const otherError = { + code: INTERNAL_ERROR_CODE, + message: 'Internal error', + }; + const client = { + readTextFile: vi.fn().mockRejectedValue(otherError), + } as unknown as AgentSideConnection; + + const svc = new AcpFileSystemService( + client, + 'session-2b', + { readTextFile: true, writeTextFile: true }, + createFallback(), + ); + + const err = await svc + .readTextFile({ path: '/some/file.txt' }) + .catch((e: unknown) => e); + + expect(err).toBeInstanceOf(Error); + expect(err).toMatchObject({ + code: INTERNAL_ERROR_CODE, + message: 'Internal error', + }); + expect(String(err)).toContain('Internal error'); + expect(String(err)).not.toContain('[object Object]'); + }); + + it('falls back to local reads for allowed local roots when ACP rejects them as outside the workspace', async () => { + const skillRoot = path.join(os.homedir(), '.qwen', 'skills'); + const filePath = path.join( + skillRoot, + 'dataworks-di-data-processor', + 'instructions', + 'interaction_norms.md', + ); + const pathOutsideWorkspaceError = { + code: INTERNAL_ERROR_CODE, + message: `path escapes workspace: ${filePath}`, + data: { + errorKind: 'path_outside_workspace', + status: 400, + }, + }; + const client = { + readTextFile: vi.fn().mockRejectedValue(pathOutsideWorkspaceError), + } as unknown as AgentSideConnection; + const fallback = createFallback(); + (fallback.readTextFile as ReturnType).mockResolvedValue({ + content: 'skill instructions', + _meta: { bom: false, encoding: 'utf-8' }, + }); + + const svc = new AcpFileSystemService( + client, + 'session-2c', + { readTextFile: true, writeTextFile: true }, + fallback, + { localReadRoots: [skillRoot] }, + ); + + await expect(svc.readTextFile({ path: filePath })).resolves.toEqual({ + content: 'skill instructions', + _meta: { bom: false, encoding: 'utf-8' }, + }); + expect(fallback.readTextFile).toHaveBeenCalledWith({ path: filePath }); + }); + + it('does not fall back to local reads outside configured roots', async () => { + const localRoot = path.join(os.tmpdir(), 'acp-local-read-root'); + const filePath = path.join(os.tmpdir(), 'outside-local-root.md'); + const pathOutsideWorkspaceError = { + code: INTERNAL_ERROR_CODE, + message: `path escapes workspace: ${filePath}`, + data: { + errorKind: 'path_outside_workspace', + status: 400, + }, + }; + const client = { + readTextFile: vi.fn().mockRejectedValue(pathOutsideWorkspaceError), + } as unknown as AgentSideConnection; + const fallback = createFallback(); + + const svc = new AcpFileSystemService( + client, + 'session-2e', + { readTextFile: true, writeTextFile: true }, + fallback, + { localReadRoots: [localRoot] }, + ); + + await expect(svc.readTextFile({ path: filePath })).rejects.toMatchObject({ + code: INTERNAL_ERROR_CODE, + message: `path escapes workspace: ${filePath}`, + }); + expect(fallback.readTextFile).not.toHaveBeenCalled(); + }); + + it('ignores empty configured local read roots', async () => { + const filePath = path.join(process.cwd(), 'outside-workspace.md'); + const pathOutsideWorkspaceError = { + code: INTERNAL_ERROR_CODE, + message: `path escapes workspace: ${filePath}`, + data: { + errorKind: 'path_outside_workspace', + status: 400, + }, + }; + const client = { + readTextFile: vi.fn().mockRejectedValue(pathOutsideWorkspaceError), + } as unknown as AgentSideConnection; + const fallback = createFallback(); + + const svc = new AcpFileSystemService( + client, + 'session-2f', + { readTextFile: true, writeTextFile: true }, + fallback, + { localReadRoots: [''] }, + ); + + await expect(svc.readTextFile({ path: filePath })).rejects.toMatchObject({ + code: INTERNAL_ERROR_CODE, + message: `path escapes workspace: ${filePath}`, + }); + expect(fallback.readTextFile).not.toHaveBeenCalled(); + }); + it('uses fallback when readTextFile capability is disabled', async () => { const client = { readTextFile: vi.fn(), diff --git a/packages/cli/src/acp-integration/service/filesystem.ts b/packages/cli/src/acp-integration/service/filesystem.ts index 1990c26503a..00ce3a66d25 100644 --- a/packages/cli/src/acp-integration/service/filesystem.ts +++ b/packages/cli/src/acp-integration/service/filesystem.ts @@ -16,8 +16,19 @@ import type { FileSystemService, ReadTextFileResponse, } from '@qwen-code/qwen-code-core'; +import { getErrorMessage } from '../../utils/errors.js'; +import path from 'node:path'; const RESOURCE_NOT_FOUND_CODE = -32002; +const PATH_OUTSIDE_WORKSPACE_KIND = 'path_outside_workspace'; + +interface AcpFileSystemServiceOptions { + localReadRoots?: readonly string[]; +} + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null; +} function getErrorCode(error: unknown): unknown { if (error instanceof RequestError) { @@ -31,6 +42,37 @@ function getErrorCode(error: unknown): unknown { return undefined; } +function getErrorData(error: unknown): Record | undefined { + const data = isRecord(error) ? error['data'] : undefined; + return isRecord(data) ? data : undefined; +} + +function getErrorKind(error: unknown): unknown { + const data = getErrorData(error); + if (data && typeof data['errorKind'] === 'string') { + return data['errorKind']; + } + if (isRecord(error) && typeof error['errorKind'] === 'string') { + return error['errorKind']; + } + return undefined; +} + +function normalizeError(error: unknown): Error { + if (error instanceof Error) return error; + + const normalized = new Error(getErrorMessage(error)) as Error & + Record; + if (isRecord(error)) { + for (const [key, value] of Object.entries(error)) { + if (key !== 'message') { + normalized[key] = value; + } + } + } + return normalized; +} + function createEnoentError(filePath: string): NodeJS.ErrnoException { const err = new Error(`File not found: ${filePath}`) as NodeJS.ErrnoException; err.code = 'ENOENT'; @@ -39,12 +81,25 @@ function createEnoentError(filePath: string): NodeJS.ErrnoException { return err; } +function isPathWithinRoot(filePath: string, root: string): boolean { + if (!root.trim()) return false; + + const relative = path.relative(path.resolve(root), path.resolve(filePath)); + return ( + relative === '' || + (!relative.startsWith(`..${path.sep}`) && + relative !== '..' && + !path.isAbsolute(relative)) + ); +} + export class AcpFileSystemService implements FileSystemService { constructor( private readonly connection: AgentSideConnection, private readonly sessionId: string, private readonly capabilities: FileSystemCapability, private readonly fallback: FileSystemService, + private readonly options: AcpFileSystemServiceOptions = {}, ) {} async readTextFile( @@ -67,7 +122,14 @@ export class AcpFileSystemService implements FileSystemService { throw createEnoentError(params.path); } - throw error; + if ( + getErrorKind(error) === PATH_OUTSIDE_WORKSPACE_KIND && + this.isLocalReadFallbackPath(params.path) + ) { + return this.fallback.readTextFile(params); + } + + throw normalizeError(error); } return response; @@ -97,4 +159,10 @@ export class AcpFileSystemService implements FileSystemService { findFiles(fileName: string, searchPaths: readonly string[]): string[] { return this.fallback.findFiles(fileName, searchPaths); } + + private isLocalReadFallbackPath(filePath: string): boolean { + return (this.options.localReadRoots ?? []).some((root) => + isPathWithinRoot(filePath, root), + ); + } } diff --git a/packages/core/src/utils/errors.test.ts b/packages/core/src/utils/errors.test.ts index 36a098ca6a3..d23b4e790d1 100644 --- a/packages/core/src/utils/errors.test.ts +++ b/packages/core/src/utils/errors.test.ts @@ -42,6 +42,20 @@ describe('getErrorMessage cause unwrapping', () => { const err = new Error('same', { cause: new Error('same') }); expect(getErrorMessage(err)).toBe('same'); }); + + it('uses the message from plain error-like objects', () => { + expect( + getErrorMessage({ + code: -32603, + message: 'path escapes workspace: /root/.qwen/skills/example.md', + data: { errorKind: 'path_outside_workspace' }, + }), + ).toBe('path escapes workspace: /root/.qwen/skills/example.md'); + }); + + it('stringifies plain objects without a message', () => { + expect(getErrorMessage({ code: -32603 })).toBe('{"code":-32603}'); + }); }); describe('isAbortError', () => { diff --git a/packages/core/src/utils/errors.ts b/packages/core/src/utils/errors.ts index 114aa8d2913..5e2483e018b 100644 --- a/packages/core/src/utils/errors.ts +++ b/packages/core/src/utils/errors.ts @@ -88,6 +88,17 @@ export function getErrorMessage(error: unknown): string { } return error.message; } + if (error !== null && typeof error === 'object') { + const message = (error as { message?: unknown }).message; + if (typeof message === 'string' && message.trim()) { + return message; + } + try { + return JSON.stringify(error) ?? String(error); + } catch { + return String(error); + } + } try { return String(error); } catch { diff --git a/packages/core/src/utils/fileUtils.test.ts b/packages/core/src/utils/fileUtils.test.ts index 9bbcc607043..315f6efa50a 100644 --- a/packages/core/src/utils/fileUtils.test.ts +++ b/packages/core/src/utils/fileUtils.test.ts @@ -1055,6 +1055,29 @@ describe('fileUtils', () => { expect(result.returnDisplay).toContain('Simulated read error'); }); + it('should surface messages from plain object text read errors', async () => { + actualNodeFs.writeFileSync(testTextFilePath, 'content'); + vi.spyOn(fsService, 'readTextFile').mockRejectedValueOnce({ + code: -32603, + message: + 'path escapes workspace: /root/.qwen/skills/dataworks-di-data-processor/instructions/interaction_norms.md', + data: { + errorKind: 'path_outside_workspace', + status: 400, + }, + }); + + const result = await processSingleFileContent( + testTextFilePath, + mockConfig, + ); + + expect(result.error).toContain('path escapes workspace'); + expect(result.returnDisplay).toContain('path escapes workspace'); + expect(result.error).not.toContain('[object Object]'); + expect(result.returnDisplay).not.toContain('[object Object]'); + }); + it('should handle read errors for image/pdf files', async () => { actualNodeFs.writeFileSync(testImageFilePath, 'content'); // File must exist mockMimeGetType.mockReturnValue('image/png'); diff --git a/packages/core/src/utils/fileUtils.ts b/packages/core/src/utils/fileUtils.ts index 6e36c60c6d6..2897c160f14 100644 --- a/packages/core/src/utils/fileUtils.ts +++ b/packages/core/src/utils/fileUtils.ts @@ -18,7 +18,7 @@ import { ToolErrorType } from '../tools/tool-error.js'; import { BINARY_EXTENSIONS } from './ignorePatterns.js'; import type { Config } from '../config/config.js'; import { createDebugLogger } from './debugLogger.js'; -import { isNodeError } from './errors.js'; +import { getErrorMessage, isNodeError } from './errors.js'; import type { InputModalities } from '../core/contentGenerator.js'; import { detectEncodingFromBuffer } from './systemEncoding.js'; import { extractPDFText, parsePDFPageRange } from './pdf.js'; @@ -1242,7 +1242,7 @@ export async function processSingleFileContent( } } } catch (error) { - const errorMessage = error instanceof Error ? error.message : String(error); + const errorMessage = getErrorMessage(error); const displayPath = path .relative(rootDirectory, filePath) .replace(/\\/g, '/'); From f431d589464515a0baef2cbb1657c161fed92615 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Mon, 29 Jun 2026 22:42:36 +0800 Subject: [PATCH 02/18] codex: address PR review feedback (#6021) Add the missing getUserAutoMemoryRoot export to the acpAgent test core mock so the updated acpAgent import resolves under Vitest. Co-authored-by: Qwen-Coder --- packages/cli/src/acp-integration/acpAgent.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/cli/src/acp-integration/acpAgent.test.ts b/packages/cli/src/acp-integration/acpAgent.test.ts index 355c3843dbf..79d175013b4 100644 --- a/packages/cli/src/acp-integration/acpAgent.test.ts +++ b/packages/cli/src/acp-integration/acpAgent.test.ts @@ -294,6 +294,7 @@ vi.mock('@qwen-code/qwen-code-core', () => ({ getAutoMemoryRoot: vi.fn( (projectRoot: string) => `${projectRoot}/.qwen/memory`, ), + getUserAutoMemoryRoot: vi.fn(() => '/tmp/user-memory'), QwenOAuth2Event: {}, qwenOAuth2Events: { on: vi.fn(), off: vi.fn() }, MCPDiscoveryState: { From 6c51fadddcb012f7abf66bd9cbca44fcb4589a34 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Mon, 29 Jun 2026 23:38:59 +0800 Subject: [PATCH 03/18] codex: address PR review feedback (#6021) Co-authored-by: Qwen-Coder --- packages/cli/src/acp-integration/acpAgent.ts | 2 + .../service/filesystem.test.ts | 298 +++++++++++++++--- .../src/acp-integration/service/filesystem.ts | 86 ++++- packages/core/src/tools/read-file.ts | 2 + 4 files changed, 324 insertions(+), 64 deletions(-) diff --git a/packages/cli/src/acp-integration/acpAgent.ts b/packages/cli/src/acp-integration/acpAgent.ts index d737d8c4c17..2805830745b 100644 --- a/packages/cli/src/acp-integration/acpAgent.ts +++ b/packages/cli/src/acp-integration/acpAgent.ts @@ -7630,6 +7630,8 @@ 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'), diff --git a/packages/cli/src/acp-integration/service/filesystem.test.ts b/packages/cli/src/acp-integration/service/filesystem.test.ts index 92176b250cb..1371adad4dc 100644 --- a/packages/cli/src/acp-integration/service/filesystem.test.ts +++ b/packages/cli/src/acp-integration/service/filesystem.test.ts @@ -8,11 +8,44 @@ import { describe, expect, it, vi } from 'vitest'; import type { FileSystemService } from '@qwen-code/qwen-code-core'; import { AcpFileSystemService } from './filesystem.js'; import type { AgentSideConnection } from '@agentclientprotocol/sdk'; +import { promises as fs } from 'node:fs'; import os from 'node:os'; import path from 'node:path'; const RESOURCE_NOT_FOUND_CODE = -32002; const INTERNAL_ERROR_CODE = -32603; +type LocalReadFallbackErrorKind = 'path_outside_workspace' | 'symlink_escape'; + +async function withTempRoot( + callback: (tempRoot: string) => Promise, +): Promise { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), 'acp-local-read-')); + + try { + return await callback(tempRoot); + } finally { + await fs.rm(tempRoot, { recursive: true, force: true }); + } +} + +function createLocalReadFallbackError( + filePath: string, + errorKind: LocalReadFallbackErrorKind = 'path_outside_workspace', +) { + const reason = + errorKind === 'symlink_escape' + ? 'path escapes workspace via symlink' + : 'path escapes workspace'; + + return { + code: INTERNAL_ERROR_CODE, + message: `${reason}: ${filePath}`, + data: { + errorKind, + status: 400, + }, + }; +} const createFallback = (): FileSystemService => ({ readTextFile: vi.fn().mockResolvedValue({ @@ -129,56 +162,198 @@ describe('AcpFileSystemService', () => { }); it('falls back to local reads for allowed local roots when ACP rejects them as outside the workspace', async () => { - const skillRoot = path.join(os.homedir(), '.qwen', 'skills'); - const filePath = path.join( - skillRoot, - 'dataworks-di-data-processor', - 'instructions', - 'interaction_norms.md', - ); - const pathOutsideWorkspaceError = { - code: INTERNAL_ERROR_CODE, - message: `path escapes workspace: ${filePath}`, - data: { - errorKind: 'path_outside_workspace', - status: 400, - }, - }; - const client = { - readTextFile: vi.fn().mockRejectedValue(pathOutsideWorkspaceError), - } as unknown as AgentSideConnection; - const fallback = createFallback(); - (fallback.readTextFile as ReturnType).mockResolvedValue({ - content: 'skill instructions', - _meta: { bom: false, encoding: 'utf-8' }, + await withTempRoot(async (tempRoot) => { + const skillRoot = path.join(tempRoot, 'skills'); + const filePath = path.join( + skillRoot, + 'dataworks-di-data-processor', + 'instructions', + 'interaction_norms.md', + ); + await fs.mkdir(path.dirname(filePath), { recursive: true }); + await fs.writeFile(filePath, 'skill instructions', 'utf8'); + + const pathOutsideWorkspaceError = + createLocalReadFallbackError(filePath); + const client = { + readTextFile: vi.fn().mockRejectedValue(pathOutsideWorkspaceError), + } as unknown as AgentSideConnection; + const fallback = createFallback(); + (fallback.readTextFile as ReturnType).mockResolvedValue({ + content: 'skill instructions', + _meta: { bom: false, encoding: 'utf-8' }, + }); + + const svc = new AcpFileSystemService( + client, + 'session-2c', + { readTextFile: true, writeTextFile: true }, + fallback, + { localReadRoots: [skillRoot] }, + ); + + await expect(svc.readTextFile({ path: filePath })).resolves.toEqual({ + content: 'skill instructions', + _meta: { bom: false, encoding: 'utf-8' }, + }); + expect(fallback.readTextFile).toHaveBeenCalledWith({ path: filePath }); }); + }); - const svc = new AcpFileSystemService( - client, - 'session-2c', - { readTextFile: true, writeTextFile: true }, - fallback, - { localReadRoots: [skillRoot] }, - ); + it.skipIf(process.platform === 'win32')( + 'does not follow symlink paths that resolve outside configured local roots', + async () => { + await withTempRoot(async (tempRoot) => { + const localRoot = path.join(tempRoot, 'allowed'); + const outsideRoot = path.join(tempRoot, 'outside'); + await fs.mkdir(localRoot, { recursive: true }); + await fs.mkdir(outsideRoot, { recursive: true }); + + const outsideFile = path.join(outsideRoot, 'secret.md'); + const symlinkPath = path.join(localRoot, 'secret.md'); + await fs.writeFile(outsideFile, 'secret', 'utf8'); + await fs.symlink(outsideFile, symlinkPath); + + const pathOutsideWorkspaceError = + createLocalReadFallbackError(symlinkPath); + const client = { + readTextFile: vi.fn().mockRejectedValue(pathOutsideWorkspaceError), + } as unknown as AgentSideConnection; + const fallback = createFallback(); + + const svc = new AcpFileSystemService( + client, + 'session-2d', + { readTextFile: true, writeTextFile: true }, + fallback, + { localReadRoots: [localRoot] }, + ); + + await expect( + svc.readTextFile({ path: symlinkPath }), + ).rejects.toMatchObject({ + code: INTERNAL_ERROR_CODE, + message: `path escapes workspace: ${symlinkPath}`, + }); + expect(fallback.readTextFile).not.toHaveBeenCalled(); + }); + }, + ); + + it.skipIf(process.platform === 'win32')( + 'allows local roots and files that resolve to the same real path tree', + async () => { + await withTempRoot(async (tempRoot) => { + const realRoot = path.join(tempRoot, 'real-root'); + const rootAlias = path.join(tempRoot, 'root-alias'); + const filePath = path.join(realRoot, 'instructions.md'); + await fs.mkdir(realRoot, { recursive: true }); + await fs.writeFile(filePath, 'instructions', 'utf8'); + await fs.symlink(realRoot, rootAlias, 'dir'); + + const pathOutsideWorkspaceError = + createLocalReadFallbackError(filePath); + const client = { + readTextFile: vi.fn().mockRejectedValue(pathOutsideWorkspaceError), + } as unknown as AgentSideConnection; + const fallback = createFallback(); + (fallback.readTextFile as ReturnType).mockResolvedValue( + { + content: 'instructions', + _meta: { bom: false, encoding: 'utf-8' }, + }, + ); + + const svc = new AcpFileSystemService( + client, + 'session-2d-realpath', + { readTextFile: true, writeTextFile: true }, + fallback, + { localReadRoots: [rootAlias] }, + ); + + await expect(svc.readTextFile({ path: filePath })).resolves.toEqual({ + content: 'instructions', + _meta: { bom: false, encoding: 'utf-8' }, + }); + expect(fallback.readTextFile).toHaveBeenCalledWith({ + path: filePath, + }); + }); + }, + ); + + it('falls back to local reads for allowed local roots when ACP rejects them as symlink escapes', async () => { + await withTempRoot(async (tempRoot) => { + const localRoot = path.join(tempRoot, 'skills'); + const filePath = path.join(localRoot, 'instructions.md'); + await fs.mkdir(localRoot, { recursive: true }); + await fs.writeFile(filePath, 'instructions', 'utf8'); + + const symlinkEscapeError = createLocalReadFallbackError( + filePath, + 'symlink_escape', + ); + const client = { + readTextFile: vi.fn().mockRejectedValue(symlinkEscapeError), + } as unknown as AgentSideConnection; + const fallback = createFallback(); + (fallback.readTextFile as ReturnType).mockResolvedValue({ + content: 'instructions', + _meta: { bom: false, encoding: 'utf-8' }, + }); + + const svc = new AcpFileSystemService( + client, + 'session-2d-symlink', + { readTextFile: true, writeTextFile: true }, + fallback, + { localReadRoots: [localRoot] }, + ); + + await expect(svc.readTextFile({ path: filePath })).resolves.toEqual({ + content: 'instructions', + _meta: { bom: false, encoding: 'utf-8' }, + }); + expect(fallback.readTextFile).toHaveBeenCalledWith({ path: filePath }); + }); + }); - await expect(svc.readTextFile({ path: filePath })).resolves.toEqual({ - content: 'skill instructions', - _meta: { bom: false, encoding: 'utf-8' }, + it('preserves the original ACP error when local read fallback fails', async () => { + await withTempRoot(async (tempRoot) => { + const localRoot = path.join(tempRoot, 'skills'); + const filePath = path.join(localRoot, 'instructions.md'); + await fs.mkdir(localRoot, { recursive: true }); + await fs.writeFile(filePath, 'instructions', 'utf8'); + + const pathOutsideWorkspaceError = + createLocalReadFallbackError(filePath); + const client = { + readTextFile: vi.fn().mockRejectedValue(pathOutsideWorkspaceError), + } as unknown as AgentSideConnection; + const fallback = createFallback(); + (fallback.readTextFile as ReturnType).mockRejectedValue( + new Error('local read failed'), + ); + + const svc = new AcpFileSystemService( + client, + 'session-2d-fallback-fail', + { readTextFile: true, writeTextFile: true }, + fallback, + { localReadRoots: [localRoot] }, + ); + + await expect(svc.readTextFile({ path: filePath })).rejects.toThrow( + `Local fallback read failed for ${filePath}: local read failed (original ACP error: path escapes workspace: ${filePath})`, + ); }); - expect(fallback.readTextFile).toHaveBeenCalledWith({ path: filePath }); }); it('does not fall back to local reads outside configured roots', async () => { const localRoot = path.join(os.tmpdir(), 'acp-local-read-root'); const filePath = path.join(os.tmpdir(), 'outside-local-root.md'); - const pathOutsideWorkspaceError = { - code: INTERNAL_ERROR_CODE, - message: `path escapes workspace: ${filePath}`, - data: { - errorKind: 'path_outside_workspace', - status: 400, - }, - }; + const pathOutsideWorkspaceError = createLocalReadFallbackError(filePath); const client = { readTextFile: vi.fn().mockRejectedValue(pathOutsideWorkspaceError), } as unknown as AgentSideConnection; @@ -201,14 +376,7 @@ describe('AcpFileSystemService', () => { it('ignores empty configured local read roots', async () => { const filePath = path.join(process.cwd(), 'outside-workspace.md'); - const pathOutsideWorkspaceError = { - code: INTERNAL_ERROR_CODE, - message: `path escapes workspace: ${filePath}`, - data: { - errorKind: 'path_outside_workspace', - status: 400, - }, - }; + const pathOutsideWorkspaceError = createLocalReadFallbackError(filePath); const client = { readTextFile: vi.fn().mockRejectedValue(pathOutsideWorkspaceError), } as unknown as AgentSideConnection; @@ -368,5 +536,37 @@ describe('AcpFileSystemService', () => { }); expect(client.writeTextFile).not.toHaveBeenCalled(); }); + + it('normalizes plain object ACP write errors to Error instances with the original message', async () => { + const writeError = { + code: INTERNAL_ERROR_CODE, + message: 'Write failed', + }; + const client = { + writeTextFile: vi.fn().mockRejectedValue(writeError), + } as unknown as AgentSideConnection; + + const svc = new AcpFileSystemService( + client, + 'session-8', + { readTextFile: true, writeTextFile: true }, + createFallback(), + ); + + const err = await svc + .writeTextFile({ + path: '/some/file.txt', + content: 'hello', + }) + .catch((e: unknown) => e); + + expect(err).toBeInstanceOf(Error); + expect(err).toMatchObject({ + code: INTERNAL_ERROR_CODE, + message: 'Write failed', + }); + expect(String(err)).toContain('Write failed'); + expect(String(err)).not.toContain('[object Object]'); + }); }); }); diff --git a/packages/cli/src/acp-integration/service/filesystem.ts b/packages/cli/src/acp-integration/service/filesystem.ts index 00ce3a66d25..fcfa8fd3fef 100644 --- a/packages/cli/src/acp-integration/service/filesystem.ts +++ b/packages/cli/src/acp-integration/service/filesystem.ts @@ -16,11 +16,19 @@ import type { FileSystemService, ReadTextFileResponse, } from '@qwen-code/qwen-code-core'; +import { createDebugLogger } from '@qwen-code/qwen-code-core'; import { getErrorMessage } from '../../utils/errors.js'; +import { realpath } from 'node:fs/promises'; import path from 'node:path'; const RESOURCE_NOT_FOUND_CODE = -32002; const PATH_OUTSIDE_WORKSPACE_KIND = 'path_outside_workspace'; +const SYMLINK_ESCAPE_KIND = 'symlink_escape'; +const LOCAL_READ_FALLBACK_ERROR_KINDS = new Set([ + PATH_OUTSIDE_WORKSPACE_KIND, + SYMLINK_ESCAPE_KIND, +]); +const debugLogger = createDebugLogger('ACP_FILE_SYSTEM'); interface AcpFileSystemServiceOptions { localReadRoots?: readonly string[]; @@ -81,10 +89,34 @@ function createEnoentError(filePath: string): NodeJS.ErrnoException { return err; } -function isPathWithinRoot(filePath: string, root: string): boolean { - if (!root.trim()) return false; +function isLocalReadFallbackErrorKind(errorKind: unknown): boolean { + return ( + typeof errorKind === 'string' && + LOCAL_READ_FALLBACK_ERROR_KINDS.has(errorKind) + ); +} + +async function resolveRealPath(value: string): Promise { + if (!value.trim()) return undefined; - const relative = path.relative(path.resolve(root), path.resolve(filePath)); + try { + return await realpath(path.resolve(value)); + } catch { + return undefined; + } +} + +async function isPathWithinRoot( + filePath: string, + root: string, +): Promise { + const [realFilePath, realRoot] = await Promise.all([ + resolveRealPath(filePath), + resolveRealPath(root), + ]); + if (!realFilePath || !realRoot) return false; + + const relative = path.relative(realRoot, realFilePath); return ( relative === '' || (!relative.startsWith(`..${path.sep}`) && @@ -122,11 +154,30 @@ export class AcpFileSystemService implements FileSystemService { throw createEnoentError(params.path); } + const errorKind = getErrorKind(error); if ( - getErrorKind(error) === PATH_OUTSIDE_WORKSPACE_KIND && - this.isLocalReadFallbackPath(params.path) + isLocalReadFallbackErrorKind(errorKind) && + (await this.isLocalReadFallbackPath(params.path)) ) { - return this.fallback.readTextFile(params); + debugLogger.debug('Falling back to local read after ACP error', { + path: params.path, + errorKind, + error: getErrorMessage(error), + }); + try { + return await this.fallback.readTextFile(params); + } catch (fallbackError) { + debugLogger.warn('Local read fallback failed after ACP error', { + path: params.path, + errorKind, + originalError: getErrorMessage(error), + fallbackError: getErrorMessage(fallbackError), + }); + throw new Error( + `Local fallback read failed for ${params.path}: ${getErrorMessage(fallbackError)} (original ACP error: ${getErrorMessage(error)})`, + { cause: fallbackError }, + ); + } } throw normalizeError(error); @@ -147,11 +198,15 @@ export class AcpFileSystemService implements FileSystemService { ? '\uFEFF' + params.content : params.content; - await this.connection.writeTextFile({ - ...params, - content: finalContent, - sessionId: this.sessionId, - }); + try { + await this.connection.writeTextFile({ + ...params, + content: finalContent, + sessionId: this.sessionId, + }); + } catch (error) { + throw normalizeError(error); + } return { _meta: params._meta }; } @@ -160,9 +215,10 @@ export class AcpFileSystemService implements FileSystemService { return this.fallback.findFiles(fileName, searchPaths); } - private isLocalReadFallbackPath(filePath: string): boolean { - return (this.options.localReadRoots ?? []).some((root) => - isPathWithinRoot(filePath, root), - ); + private async isLocalReadFallbackPath(filePath: string): Promise { + for (const root of this.options.localReadRoots ?? []) { + if (await isPathWithinRoot(filePath, root)) return true; + } + return false; } } diff --git a/packages/core/src/tools/read-file.ts b/packages/core/src/tools/read-file.ts index 5f31ce4f2c8..54c925aa0cc 100644 --- a/packages/core/src/tools/read-file.ts +++ b/packages/core/src/tools/read-file.ts @@ -106,6 +106,8 @@ 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. const allowedRoots = [ this.config.storage.getProjectTempDir(), // Background subagent transcripts live under /subagents/ and From df90d1f03fe432fcf2821ddbc7cb2757ed19b914 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Tue, 30 Jun 2026 00:06:45 +0800 Subject: [PATCH 04/18] codex: address PR review feedback (#6021) Co-authored-by: Qwen-Coder --- .../cli/src/acp-integration/acpAgent.test.ts | 76 ++++++++++++++++++ .../service/filesystem.test.ts | 80 +++++++++++++++++++ .../src/acp-integration/service/filesystem.ts | 11 +-- packages/core/src/tools/edit.test.ts | 23 ++++++ packages/core/src/tools/edit.ts | 8 +- packages/core/src/tools/write-file.test.ts | 21 +++++ packages/core/src/tools/write-file.ts | 8 +- packages/core/src/utils/errors.test.ts | 7 ++ packages/core/src/utils/errors.ts | 14 +++- 9 files changed, 232 insertions(+), 16 deletions(-) diff --git a/packages/cli/src/acp-integration/acpAgent.test.ts b/packages/cli/src/acp-integration/acpAgent.test.ts index 79d175013b4..16f9b91caf4 100644 --- a/packages/cli/src/acp-integration/acpAgent.test.ts +++ b/packages/cli/src/acp-integration/acpAgent.test.ts @@ -348,6 +348,8 @@ vi.mock('@qwen-code/qwen-code-core', () => ({ SessionService: vi.fn(), Storage: { getGlobalQwenDir: vi.fn(() => '/tmp/qwen-global-test'), + getGlobalTempDir: vi.fn(() => '/tmp/qwen-global-temp'), + getUserExtensionsDir: vi.fn(() => '/tmp/qwen-extensions'), }, parseRule: vi.fn((raw: string) => { const trimmed = raw.trim(); @@ -598,6 +600,7 @@ import { } from '../config/permission-settings.js'; import { loadCliConfig } from '../config/config.js'; import { createLoadedSettingsAdapter } from '../config/loadedSettingsAdapter.js'; +import { AcpFileSystemService } from './service/filesystem.js'; import { Session, buildAvailableCommandsSnapshot } from './session/Session.js'; import { SERVE_STATUS_EXT_METHODS, @@ -1216,6 +1219,79 @@ describe('QwenAgent MCP SSE/HTTP support', () => { await agentPromise; }); + 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 agentPromise = runAcpAgent( + mockConfig, + makeSessionSettings(), + mockArgv, + ); + await vi.waitFor(() => expect(capturedAgentFactory).toBeDefined()); + + 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, + 'session-with-fs', + fsCapabilities, + fallbackFileSystem, + { + localReadRoots: [ + '/project/.qwen/tmp', + '/project/subagents', + '/tmp/qwen-global-temp', + os.tmpdir(), + '/project/.qwen/memory', + '/tmp/user-memory', + '/home/test/.qwen/skills', + '/tmp/qwen-extensions', + ], + }, + ); + expect(innerConfig.setFileSystemService).toHaveBeenCalled(); + + mockConnectionState.resolve(); + await agentPromise; + }); + it('does not return discontinued qwen-oauth as the only ACP auth option', async () => { vi.mocked(buildAuthMethods).mockReturnValue([ { diff --git a/packages/cli/src/acp-integration/service/filesystem.test.ts b/packages/cli/src/acp-integration/service/filesystem.test.ts index 1371adad4dc..b43f5d948c9 100644 --- a/packages/cli/src/acp-integration/service/filesystem.test.ts +++ b/packages/cli/src/acp-integration/service/filesystem.test.ts @@ -161,6 +161,51 @@ describe('AcpFileSystemService', () => { expect(String(err)).not.toContain('[object Object]'); }); + it('preserves stack traces from plain object ACP errors', async () => { + const otherError = { + code: INTERNAL_ERROR_CODE, + message: 'Internal error', + stack: 'Original ACP stack', + }; + const client = { + readTextFile: vi.fn().mockRejectedValue(otherError), + } as unknown as AgentSideConnection; + + const svc = new AcpFileSystemService( + client, + 'session-2b-stack', + { readTextFile: true, writeTextFile: true }, + createFallback(), + ); + + const err = await svc + .readTextFile({ path: '/some/file.txt' }) + .catch((e: unknown) => e); + + expect(err).toBeInstanceOf(Error); + expect((err as Error).stack).toBe('Original ACP stack'); + }); + + it('does not copy array entries onto normalized ACP errors', async () => { + const client = { + readTextFile: vi.fn().mockRejectedValue(['Internal error']), + } as unknown as AgentSideConnection; + + const svc = new AcpFileSystemService( + client, + 'session-2b-array', + { readTextFile: true, writeTextFile: true }, + createFallback(), + ); + + const err = await svc + .readTextFile({ path: '/some/file.txt' }) + .catch((e: unknown) => e); + + expect(err).toBeInstanceOf(Error); + expect(Object.prototype.hasOwnProperty.call(err, '0')).toBe(false); + }); + it('falls back to local reads for allowed local roots when ACP rejects them as outside the workspace', async () => { await withTempRoot(async (tempRoot) => { const skillRoot = path.join(tempRoot, 'skills'); @@ -200,6 +245,41 @@ describe('AcpFileSystemService', () => { }); }); + it('does not use top-level errorKind fields for local read fallback', async () => { + await withTempRoot(async (tempRoot) => { + const localRoot = path.join(tempRoot, 'skills'); + const filePath = path.join(localRoot, 'instructions.md'); + await fs.mkdir(localRoot, { recursive: true }); + await fs.writeFile(filePath, 'instructions', 'utf8'); + + const topLevelErrorKindError = { + code: INTERNAL_ERROR_CODE, + message: `top-level errorKind only: ${filePath}`, + errorKind: 'path_outside_workspace', + }; + const client = { + readTextFile: vi.fn().mockRejectedValue(topLevelErrorKindError), + } as unknown as AgentSideConnection; + const fallback = createFallback(); + + const svc = new AcpFileSystemService( + client, + 'session-2c-top-level-kind', + { readTextFile: true, writeTextFile: true }, + fallback, + { localReadRoots: [localRoot] }, + ); + + await expect( + svc.readTextFile({ path: filePath }), + ).rejects.toMatchObject({ + code: INTERNAL_ERROR_CODE, + message: `top-level errorKind only: ${filePath}`, + }); + expect(fallback.readTextFile).not.toHaveBeenCalled(); + }); + }); + it.skipIf(process.platform === 'win32')( 'does not follow symlink paths that resolve outside configured local roots', async () => { diff --git a/packages/cli/src/acp-integration/service/filesystem.ts b/packages/cli/src/acp-integration/service/filesystem.ts index fcfa8fd3fef..7631cc0dc0e 100644 --- a/packages/cli/src/acp-integration/service/filesystem.ts +++ b/packages/cli/src/acp-integration/service/filesystem.ts @@ -35,7 +35,7 @@ interface AcpFileSystemServiceOptions { } function isRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null; + return typeof value === 'object' && value !== null && !Array.isArray(value); } function getErrorCode(error: unknown): unknown { @@ -43,8 +43,8 @@ function getErrorCode(error: unknown): unknown { return error.code; } - if (typeof error === 'object' && error !== null && 'code' in error) { - return (error as { code?: unknown }).code; + if (isRecord(error)) { + return error['code']; } return undefined; @@ -55,14 +55,11 @@ function getErrorData(error: unknown): Record | undefined { return isRecord(data) ? data : undefined; } -function getErrorKind(error: unknown): unknown { +function getErrorKind(error: unknown): string | undefined { const data = getErrorData(error); if (data && typeof data['errorKind'] === 'string') { return data['errorKind']; } - if (isRecord(error) && typeof error['errorKind'] === 'string') { - return error['errorKind']; - } return undefined; } diff --git a/packages/core/src/tools/edit.test.ts b/packages/core/src/tools/edit.test.ts index 3c1c126eac6..83daf1db3a7 100644 --- a/packages/core/src/tools/edit.test.ts +++ b/packages/core/src/tools/edit.test.ts @@ -1100,6 +1100,29 @@ describe('EditTool', () => { const result = await invocation.execute(new AbortController().signal); expect(result.error?.type).toBe(ToolErrorType.FILE_WRITE_FAILURE); }); + + it('should surface plain object write error messages without object stringification', async () => { + fs.writeFileSync(filePath, 'content', 'utf8'); + seedPriorRead(filePath); + + vi.spyOn(fsService, 'writeTextFile').mockRejectedValueOnce({ + message: 'Plain object edit error', + }); + + const params: EditToolParams = { + file_path: filePath, + old_string: 'content', + new_string: 'new content', + }; + const invocation = tool.build(params); + const result = await invocation.execute(new AbortController().signal); + + expect(result.error?.type).toBe(ToolErrorType.FILE_WRITE_FAILURE); + expect(result.llmContent).toContain( + 'Error executing edit: Plain object edit error', + ); + expect(result.llmContent).not.toContain('[object Object]'); + }); }); describe('getDescription', () => { diff --git a/packages/core/src/tools/edit.ts b/packages/core/src/tools/edit.ts index 0542d3aa1bb..49f03753e5e 100644 --- a/packages/core/src/tools/edit.ts +++ b/packages/core/src/tools/edit.ts @@ -18,7 +18,7 @@ import type { PermissionDecision } from '../permissions/types.js'; import { BaseDeclarativeTool, Kind, ToolConfirmationOutcome } from './tools.js'; import { ToolErrorType } from './tool-error.js'; import { makeRelative, shortenPath, unescapePath } from '../utils/paths.js'; -import { isNodeError } from '../utils/errors.js'; +import { getErrorMessage, isNodeError } from '../utils/errors.js'; import type { Config } from '../config/config.js'; import { ApprovalMode } from '../config/config.js'; import { isAnyAutoMemPath, isTeamAutoMemPath } from '../memory/paths.js'; @@ -416,7 +416,7 @@ class EditToolInvocation implements ToolInvocation { if (abortSignal.aborted) { throw error; } - const errorMsg = error instanceof Error ? error.message : String(error); + const errorMsg = getErrorMessage(error); throw new Error(`Error preparing edit: ${errorMsg}`); } @@ -484,7 +484,7 @@ class EditToolInvocation implements ToolInvocation { if (signal.aborted) { throw error; } - const errorMsg = error instanceof Error ? error.message : String(error); + const errorMsg = getErrorMessage(error); return { llmContent: `Error preparing edit: ${errorMsg}`, returnDisplay: `Error preparing edit: ${errorMsg}`, @@ -725,7 +725,7 @@ class EditToolInvocation implements ToolInvocation { returnDisplay: displayResult, }; } catch (error) { - const errorMsg = error instanceof Error ? error.message : String(error); + const errorMsg = getErrorMessage(error); return { llmContent: `Error executing edit: ${errorMsg}`, returnDisplay: `Error writing file: ${errorMsg}`, diff --git a/packages/core/src/tools/write-file.test.ts b/packages/core/src/tools/write-file.test.ts index 76487771642..d4f3bdb7ec7 100644 --- a/packages/core/src/tools/write-file.test.ts +++ b/packages/core/src/tools/write-file.test.ts @@ -846,6 +846,27 @@ describe('WriteFileTool', () => { 'Error writing to file: Generic write error', ); }); + + it('should surface plain object write error messages without object stringification', async () => { + const filePath = path.join(rootDir, 'plain_object_error_file.txt'); + const content = 'test content'; + + vi.restoreAllMocks(); + + vi.spyOn(fsService, 'writeTextFile').mockRejectedValueOnce({ + message: 'Plain object write error', + }); + + const params = { file_path: filePath, content }; + const invocation = tool.build(params); + const result = await invocation.execute(abortSignal); + + expect(result.error?.type).toBe(ToolErrorType.FILE_WRITE_FAILURE); + expect(result.llmContent).toContain( + 'Error writing to file: Plain object write error', + ); + expect(result.llmContent).not.toContain('[object Object]'); + }); }); describe('BOM preservation (Issue #1672)', () => { diff --git a/packages/core/src/tools/write-file.ts b/packages/core/src/tools/write-file.ts index 742e61ccbf4..5f8b2d2cb6a 100644 --- a/packages/core/src/tools/write-file.ts +++ b/packages/core/src/tools/write-file.ts @@ -615,7 +615,7 @@ class WriteFileToolInvocation extends BaseToolInvocation< } else if (error instanceof Error) { errorMsg = `Error writing to file: ${error.message}`; } else { - errorMsg = `Error writing to file: ${String(error)}`; + errorMsg = `Error writing to file: ${getErrorMessage(error)}`; } return { @@ -689,9 +689,9 @@ The user has the ability to modify \`content\`. If modified, this will be stated } } } catch (statError: unknown) { - return `Error accessing path properties for validation: ${filePath}. Reason: ${ - statError instanceof Error ? statError.message : String(statError) - }`; + return `Error accessing path properties for validation: ${filePath}. Reason: ${getErrorMessage( + statError, + )}`; } const teamMemoryError = checkTeamMemorySecrets( diff --git a/packages/core/src/utils/errors.test.ts b/packages/core/src/utils/errors.test.ts index d23b4e790d1..34e3142cbdf 100644 --- a/packages/core/src/utils/errors.test.ts +++ b/packages/core/src/utils/errors.test.ts @@ -56,6 +56,13 @@ describe('getErrorMessage cause unwrapping', () => { it('stringifies plain objects without a message', () => { expect(getErrorMessage({ code: -32603 })).toBe('{"code":-32603}'); }); + + it('bounds stringified plain objects without a message', () => { + const message = getErrorMessage({ detail: 'x'.repeat(2000) }); + + expect(message.length).toBeLessThanOrEqual(1000); + expect(message).toContain('"detail"'); + }); }); describe('isAbortError', () => { diff --git a/packages/core/src/utils/errors.ts b/packages/core/src/utils/errors.ts index 5e2483e018b..d4c175f9772 100644 --- a/packages/core/src/utils/errors.ts +++ b/packages/core/src/utils/errors.ts @@ -10,6 +10,8 @@ interface GaxiosError { }; } +const MAX_STRINGIFIED_ERROR_MESSAGE_LENGTH = 1000; + export function isNodeError(error: unknown): error is NodeJS.ErrnoException { return error instanceof Error && 'code' in error; } @@ -80,6 +82,13 @@ function describeSingleError(err: unknown): string | undefined { return str && str !== '[object Object]' ? str : undefined; } +function truncateStringifiedErrorMessage(message: string): string { + if (message.length <= MAX_STRINGIFIED_ERROR_MESSAGE_LENGTH) { + return message; + } + return `${message.slice(0, MAX_STRINGIFIED_ERROR_MESSAGE_LENGTH - 3)}...`; +} + export function getErrorMessage(error: unknown): string { if (error instanceof Error) { const detail = describeErrorCause(error.cause); @@ -94,7 +103,10 @@ export function getErrorMessage(error: unknown): string { return message; } try { - return JSON.stringify(error) ?? String(error); + const serialized = JSON.stringify(error); + return serialized + ? truncateStringifiedErrorMessage(serialized) + : String(error); } catch { return String(error); } From 1a2729065fb6425eb2e97d462907fc97b8063d34 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Tue, 30 Jun 2026 00:17:58 +0800 Subject: [PATCH 05/18] fix(acp): Narrow local read fallback temp roots Remove the broad OS temp directory from default read_file allow roots and ACP local read fallback roots. Keep qwen-managed temp roots readable and reuse the shared isSubpath helper after realpath resolution for ACP fallback containment. Co-authored-by: Qwen-Coder --- .../cli/src/acp-integration/acpAgent.test.ts | 1 - packages/cli/src/acp-integration/acpAgent.ts | 2 -- .../src/acp-integration/service/filesystem.ts | 14 ++++------- packages/core/src/tools/read-file.test.ts | 23 +++++++++++++++++-- packages/core/src/tools/read-file.ts | 6 ++--- 5 files changed, 27 insertions(+), 19 deletions(-) diff --git a/packages/cli/src/acp-integration/acpAgent.test.ts b/packages/cli/src/acp-integration/acpAgent.test.ts index 16f9b91caf4..ae098dbe426 100644 --- a/packages/cli/src/acp-integration/acpAgent.test.ts +++ b/packages/cli/src/acp-integration/acpAgent.test.ts @@ -1278,7 +1278,6 @@ describe('QwenAgent MCP SSE/HTTP support', () => { '/project/.qwen/tmp', '/project/subagents', '/tmp/qwen-global-temp', - os.tmpdir(), '/project/.qwen/memory', '/tmp/user-memory', '/home/test/.qwen/skills', diff --git a/packages/cli/src/acp-integration/acpAgent.ts b/packages/cli/src/acp-integration/acpAgent.ts index 2805830745b..3002ddddc9b 100644 --- a/packages/cli/src/acp-integration/acpAgent.ts +++ b/packages/cli/src/acp-integration/acpAgent.ts @@ -131,7 +131,6 @@ import { pipeline } from 'node:stream/promises'; import * as fs from 'node:fs/promises'; import { createRequire } from 'node:module'; import * as path from 'node:path'; -import * as os from 'node:os'; import { createGunzip } from 'node:zlib'; import type { LoadedSettings } from '../config/settings.js'; import { @@ -7636,7 +7635,6 @@ class QwenAgent implements Agent { config.storage.getProjectTempDir(), path.join(config.storage.getProjectDir(), 'subagents'), Storage.getGlobalTempDir(), - os.tmpdir(), getAutoMemoryRoot(config.getTargetDir()), getUserAutoMemoryRoot(), ...config.storage.getUserSkillsDirs(), diff --git a/packages/cli/src/acp-integration/service/filesystem.ts b/packages/cli/src/acp-integration/service/filesystem.ts index 7631cc0dc0e..1512209805c 100644 --- a/packages/cli/src/acp-integration/service/filesystem.ts +++ b/packages/cli/src/acp-integration/service/filesystem.ts @@ -16,7 +16,7 @@ import type { FileSystemService, ReadTextFileResponse, } from '@qwen-code/qwen-code-core'; -import { createDebugLogger } from '@qwen-code/qwen-code-core'; +import { createDebugLogger, isSubpath } from '@qwen-code/qwen-code-core'; import { getErrorMessage } from '../../utils/errors.js'; import { realpath } from 'node:fs/promises'; import path from 'node:path'; @@ -103,7 +103,7 @@ async function resolveRealPath(value: string): Promise { } } -async function isPathWithinRoot( +async function isRealPathWithinRoot( filePath: string, root: string, ): Promise { @@ -113,13 +113,7 @@ async function isPathWithinRoot( ]); if (!realFilePath || !realRoot) return false; - const relative = path.relative(realRoot, realFilePath); - return ( - relative === '' || - (!relative.startsWith(`..${path.sep}`) && - relative !== '..' && - !path.isAbsolute(relative)) - ); + return isSubpath(realRoot, realFilePath); } export class AcpFileSystemService implements FileSystemService { @@ -214,7 +208,7 @@ export class AcpFileSystemService implements FileSystemService { private async isLocalReadFallbackPath(filePath: string): Promise { for (const root of this.options.localReadRoots ?? []) { - if (await isPathWithinRoot(filePath, root)) return true; + if (await isRealPathWithinRoot(filePath, root)) return true; } return false; } diff --git a/packages/core/src/tools/read-file.test.ts b/packages/core/src/tools/read-file.test.ts index f90287a75df..fb55741b5b7 100644 --- a/packages/core/src/tools/read-file.test.ts +++ b/packages/core/src/tools/read-file.test.ts @@ -13,6 +13,7 @@ import os from 'node:os'; import fs from 'node:fs'; import fsp from 'node:fs/promises'; import type { Config } from '../config/config.js'; +import { Storage } from '../config/storage.js'; import { FileDiscoveryService } from '../services/fileDiscoveryService.js'; import { FileReadCache } from '../services/fileReadCache.js'; import { StandardFileSystemService } from '../services/fileSystemService.js'; @@ -114,7 +115,7 @@ describe('ReadFileTool', () => { expect(typeof result).not.toBe('string'); }); - it('should allow access to files in OS temp directory', () => { + it('should build an invocation for files in the OS temp directory', () => { const params: ReadFileToolParams = { file_path: path.join(os.tmpdir(), 'pr-review-context.md'), }; @@ -212,6 +213,24 @@ describe('ReadFileTool', () => { expect(permission).toBe('allow'); }); + it('should return allow for paths within the global qwen temp directory', async () => { + const params: ReadFileToolParams = { + file_path: path.join(Storage.getGlobalTempDir(), 'temp-file.txt'), + }; + const invocation = tool.build(params); + const permission = await invocation.getDefaultPermission(); + expect(permission).toBe('allow'); + }); + + it('should return ask for paths directly under the OS temp directory', async () => { + const params: ReadFileToolParams = { + file_path: path.join(os.tmpdir(), 'pr-review-context.md'), + }; + const invocation = tool.build(params); + const permission = await invocation.getDefaultPermission(); + expect(permission).toBe('ask'); + }); + it('should return allow for paths within the subagent transcripts dir', async () => { const params: ReadFileToolParams = { file_path: path.join( @@ -648,7 +667,7 @@ describe('ReadFileTool', () => { expect(result.returnDisplay).toBe(''); }); - it('should successfully read files from OS temp directory', async () => { + it('should read OS temp files after the invocation is executed', async () => { const osTempFile = await fsp.mkdtemp( path.join(os.tmpdir(), 'read-file-test-'), ); diff --git a/packages/core/src/tools/read-file.ts b/packages/core/src/tools/read-file.ts index 54c925aa0cc..ff75476bdb8 100644 --- a/packages/core/src/tools/read-file.ts +++ b/packages/core/src/tools/read-file.ts @@ -4,7 +4,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -import os from 'node:os'; import path from 'node:path'; import fs from 'node:fs/promises'; import type { Stats } from 'node:fs'; @@ -99,8 +98,8 @@ class ReadFileToolInvocation extends BaseToolInvocation< } /** - * Returns 'ask' for paths outside the workspace/temp/userSkills directories, - * so that external file reads require user confirmation. + * Returns 'ask' for paths outside the workspace/qwen-managed temp/userSkills + * directories, so that external file reads require user confirmation. */ override async getDefaultPermission(): Promise { const filePath = path.resolve(this.params.file_path); @@ -114,7 +113,6 @@ class ReadFileToolInvocation extends BaseToolInvocation< // are advertised to the model as polling targets via read_file. path.join(this.config.storage.getProjectDir(), 'subagents'), Storage.getGlobalTempDir(), - os.tmpdir(), ...this.config.storage.getUserSkillsDirs(), Storage.getUserExtensionsDir(), ]; From 0144af09996e45f43b6758484c7e3c46f10989bd Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Tue, 30 Jun 2026 00:31:48 +0800 Subject: [PATCH 06/18] codex: fix CI failure on PR #6021 Add the serve fast-path bundle check script and root npm script that the current CI workflow invokes. This mirrors the already-merged mainline check without pulling unrelated workflow or test config changes into this PR. Co-authored-by: Qwen-Coder --- package.json | 1 + scripts/check-serve-fast-path-bundle.js | 315 ++++++++++++++++++++++++ 2 files changed, 316 insertions(+) create mode 100644 scripts/check-serve-fast-path-bundle.js diff --git a/package.json b/package.json index 4d04da224fe..72d9016af27 100644 --- a/package.json +++ b/package.json @@ -80,6 +80,7 @@ "telemetry": "node scripts/telemetry.js", "check:lockfile": "node scripts/check-lockfile.js", "check:desktop-isolation": "node scripts/check-desktop-isolation.js", + "check:serve-fast-path-bundle": "npm run build -- --cli-only && cross-env DEV=true npm run bundle && node scripts/check-serve-fast-path-bundle.js", "desktop-openwork-sync": "bun run scripts/desktop-openwork-sync.ts", "clean": "node scripts/clean.js", "pre-commit": "node scripts/pre-commit.js" diff --git a/scripts/check-serve-fast-path-bundle.js b/scripts/check-serve-fast-path-bundle.js new file mode 100644 index 00000000000..e05c8e24198 --- /dev/null +++ b/scripts/check-serve-fast-path-bundle.js @@ -0,0 +1,315 @@ +#!/usr/bin/env node +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { existsSync, readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const DEFAULT_METAFILE_PATH = resolve('dist/esbuild.json'); +const METAFILE_BUILD_COMMAND = + 'npm run build -- --cli-only && npx cross-env DEV=true npm run bundle'; +const SERVE_PRE_LISTEN_ROOTS = [ + { + label: 'serve fast path entry', + suffixes: [ + 'packages/cli/src/serve/fast-path.ts', + 'packages/cli/dist/src/serve/fast-path.js', + ], + }, + { + label: 'serve fast path settings', + suffixes: [ + 'packages/cli/src/serve/fast-path-settings.ts', + 'packages/cli/dist/src/serve/fast-path-settings.js', + ], + }, + { + label: 'run qwen serve entry', + suffixes: [ + 'packages/cli/src/serve/run-qwen-serve.ts', + 'packages/cli/dist/src/serve/run-qwen-serve.js', + ], + }, +]; + +const FORBIDDEN_SOURCE_INPUTS = [ + { + label: 'Serve ACP compatibility shim', + suffixes: [ + 'packages/cli/src/serve/acp-session-bridge.ts', + 'packages/cli/dist/src/serve/acp-session-bridge.js', + ], + }, + { + label: 'ACP bridge runtime', + suffixes: [ + 'packages/acp-bridge/src/bridge.ts', + 'packages/acp-bridge/dist/bridge.js', + ], + }, + { + label: 'ACP bridge client runtime', + suffixes: [ + 'packages/acp-bridge/src/bridgeClient.ts', + 'packages/acp-bridge/dist/bridgeClient.js', + ], + }, + { + label: 'ACP spawnChannel runtime', + suffixes: [ + 'packages/acp-bridge/src/spawnChannel.ts', + 'packages/acp-bridge/dist/spawnChannel.js', + ], + }, + { + label: 'ACP permission mediator runtime', + suffixes: [ + 'packages/acp-bridge/src/permissionMediator.ts', + 'packages/acp-bridge/dist/permissionMediator.js', + ], + }, + { + label: 'ACP compaction engine runtime', + suffixes: [ + 'packages/acp-bridge/src/compactionEngine.ts', + 'packages/acp-bridge/dist/compactionEngine.js', + ], + }, + { + label: 'Core shell tool runtime', + suffixes: [ + 'packages/core/src/tools/shell.ts', + 'packages/core/dist/src/tools/shell.js', + ], + }, +]; + +const FORBIDDEN_VENDOR_PACKAGES = [ + { label: 'glob vendor package', packageName: 'glob' }, + { label: 'chokidar vendor package', packageName: 'chokidar' }, + { label: '@iarna/toml vendor package', packageName: '@iarna/toml' }, + { label: 'fzf vendor package', packageName: 'fzf' }, +]; + +export function normalizeMetafilePath(filePath) { + return filePath.replace(/\\/g, '/').replace(/^\.\//, ''); +} + +function inputMatchesSuffix(input, suffix) { + const normalizedInput = normalizeMetafilePath(input); + return normalizedInput === suffix || normalizedInput.endsWith(`/${suffix}`); +} + +function inputMatchesAnySuffix(input, suffixes) { + return suffixes.some((suffix) => inputMatchesSuffix(input, suffix)); +} + +function inputMatchesPackage(input, packageName) { + const normalizedInput = normalizeMetafilePath(input); + const marker = `node_modules/${packageName}/`; + return ( + normalizedInput === `node_modules/${packageName}` || + normalizedInput.includes(marker) + ); +} + +function normalizeOutputs(metafile) { + return new Map( + Object.entries(metafile.outputs ?? {}).map(([outputPath, output]) => [ + normalizeMetafilePath(outputPath), + output, + ]), + ); +} + +function findServePreListenRootOutputs(outputs) { + const rootOutputs = []; + const missingRoots = []; + + for (const root of SERVE_PRE_LISTEN_ROOTS) { + let matchedOutput; + for (const [outputPath, output] of outputs) { + for (const input of Object.keys(output.inputs ?? {})) { + if (inputMatchesAnySuffix(input, root.suffixes)) { + matchedOutput = outputPath; + break; + } + } + if (matchedOutput) break; + } + + if (matchedOutput) { + rootOutputs.push(matchedOutput); + } else { + missingRoots.push(`${root.label} (${root.suffixes.join(' or ')})`); + } + } + + if (missingRoots.length > 0) { + throw new Error( + 'Could not find bundled outputs for serve pre-listen roots:\n' + + missingRoots.map((root) => `- ${root}`).join('\n') + + `\nRun \`${METAFILE_BUILD_COMMAND}\` to produce the metafile.`, + ); + } + + return [...new Set(rootOutputs)]; +} + +function collectStaticClosure(outputs, entryOutputs) { + const queue = [...entryOutputs]; + const closure = new Set(queue); + const parent = new Map(); + + for (let i = 0; i < queue.length; i++) { + const outputPath = queue[i]; + const output = outputs.get(outputPath); + for (const bundledImport of output?.imports ?? []) { + if (bundledImport.external) continue; + if (bundledImport.kind === 'dynamic-import') continue; + + const importedOutput = normalizeMetafilePath(bundledImport.path); + if (!outputs.has(importedOutput) || closure.has(importedOutput)) { + continue; + } + + closure.add(importedOutput); + parent.set(importedOutput, outputPath); + queue.push(importedOutput); + } + } + + return { closure, parent }; +} + +function buildImportPath(entryOutputs, outputPath, parent) { + const roots = new Set(entryOutputs); + const reversed = [outputPath]; + let current = outputPath; + while (!roots.has(current)) { + current = parent.get(current); + if (!current) break; + reversed.push(current); + } + return reversed.reverse(); +} + +export function findServeFastPathBundleOffenders(metafile) { + const outputs = normalizeOutputs(metafile); + const entryOutputs = findServePreListenRootOutputs(outputs); + const { closure, parent } = collectStaticClosure(outputs, entryOutputs); + const offenders = []; + const seen = new Set(); + + for (const outputPath of closure) { + const output = outputs.get(outputPath); + const inputs = Object.keys(output?.inputs ?? {}); + + for (const input of inputs) { + const sourceMatch = FORBIDDEN_SOURCE_INPUTS.find(({ suffixes }) => + inputMatchesAnySuffix(input, suffixes), + ); + if (sourceMatch) { + addOffender( + sourceMatch.label, + normalizeMetafilePath(input), + outputPath, + ); + } + + const vendorMatch = FORBIDDEN_VENDOR_PACKAGES.find(({ packageName }) => + inputMatchesPackage(input, packageName), + ); + if (vendorMatch) { + addOffender( + vendorMatch.label, + normalizeMetafilePath(input), + outputPath, + ); + } + } + } + + return offenders; + + function addOffender(label, matchedInput, outputPath) { + const key = `${label}\0${matchedInput}\0${outputPath}`; + if (seen.has(key)) return; + seen.add(key); + offenders.push({ + label, + matchedInput, + outputPath, + bytes: outputs.get(outputPath)?.bytes ?? 0, + importPath: buildImportPath(entryOutputs, outputPath, parent), + }); + } +} + +export function formatServeFastPathBundleOffenders(offenders) { + return offenders + .map((offender) => { + const importPath = offender.importPath.join(' -> '); + return [ + `- ${offender.label}`, + ` input: ${offender.matchedInput}`, + ` output: ${offender.outputPath} (${offender.bytes} bytes)`, + ` static path: ${importPath}`, + ].join('\n'); + }) + .join('\n'); +} + +export function checkServeFastPathBundle({ + metafilePath = DEFAULT_METAFILE_PATH, +} = {}) { + if (!existsSync(metafilePath)) { + throw new Error( + `Missing esbuild metafile at ${metafilePath}. ` + + `Run \`${METAFILE_BUILD_COMMAND}\` to produce it.`, + ); + } + + let metafile; + try { + metafile = JSON.parse(readFileSync(metafilePath, 'utf8')); + } catch (error) { + const reason = error instanceof Error ? error.message : String(error); + throw new Error( + `Invalid esbuild metafile at ${metafilePath}: ${reason}. ` + + `Run \`${METAFILE_BUILD_COMMAND}\` to regenerate it.`, + ); + } + const offenders = findServeFastPathBundleOffenders(metafile); + return { ok: offenders.length === 0, offenders }; +} + +function main() { + try { + const result = checkServeFastPathBundle(); + if (result.ok) { + console.log('Serve fast-path bundle closure check passed.'); + return; + } + + console.error( + 'Serve fast-path bundle closure includes pre-listen runtime modules:\n' + + formatServeFastPathBundleOffenders(result.offenders), + ); + process.exitCode = 1; + } catch (error) { + console.error(error instanceof Error ? error.message : String(error)); + process.exitCode = 1; + } +} + +if ( + process.argv[1] && + fileURLToPath(import.meta.url) === resolve(process.argv[1]) +) { + main(); +} From e6b98c259b4e55138df6a98d0f4552a946d7376f Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Tue, 30 Jun 2026 02:21:36 +0800 Subject: [PATCH 07/18] fix(cli): address ACP read error review feedback Co-authored-by: Qwen-Coder --- .../service/filesystem.test.ts | 57 +++++++++++++------ .../src/acp-integration/service/filesystem.ts | 11 +++- packages/core/src/utils/fileUtils.test.ts | 21 +++++++ packages/core/src/utils/fileUtils.ts | 2 +- 4 files changed, 69 insertions(+), 22 deletions(-) diff --git a/packages/cli/src/acp-integration/service/filesystem.test.ts b/packages/cli/src/acp-integration/service/filesystem.test.ts index b43f5d948c9..da4ca8520d5 100644 --- a/packages/cli/src/acp-integration/service/filesystem.test.ts +++ b/packages/cli/src/acp-integration/service/filesystem.test.ts @@ -108,7 +108,7 @@ describe('AcpFileSystemService', () => { }); }); - it('preserves code and message for other read errors', async () => { + it('preserves message for other read errors', async () => { const otherError = { code: INTERNAL_ERROR_CODE, message: 'Internal error', @@ -127,12 +127,11 @@ describe('AcpFileSystemService', () => { await expect( svc.readTextFile({ path: '/some/file.txt' }), ).rejects.toMatchObject({ - code: INTERNAL_ERROR_CODE, message: 'Internal error', }); }); - it('normalizes plain object ACP errors to Error instances with the original message', async () => { + it('normalizes plain object ACP errors without exposing numeric codes as Node error codes', async () => { const otherError = { code: INTERNAL_ERROR_CODE, message: 'Internal error', @@ -154,9 +153,10 @@ describe('AcpFileSystemService', () => { expect(err).toBeInstanceOf(Error); expect(err).toMatchObject({ - code: INTERNAL_ERROR_CODE, + cause: otherError, message: 'Internal error', }); + expect(Object.prototype.hasOwnProperty.call(err, 'code')).toBe(false); expect(String(err)).toContain('Internal error'); expect(String(err)).not.toContain('[object Object]'); }); @@ -270,12 +270,16 @@ describe('AcpFileSystemService', () => { { localReadRoots: [localRoot] }, ); - await expect( - svc.readTextFile({ path: filePath }), - ).rejects.toMatchObject({ - code: INTERNAL_ERROR_CODE, + const err = await svc + .readTextFile({ path: filePath }) + .catch((e: unknown) => e); + + expect(err).toBeInstanceOf(Error); + expect(err).toMatchObject({ + cause: topLevelErrorKindError, message: `top-level errorKind only: ${filePath}`, }); + expect(Object.prototype.hasOwnProperty.call(err, 'code')).toBe(false); expect(fallback.readTextFile).not.toHaveBeenCalled(); }); }); @@ -309,12 +313,16 @@ describe('AcpFileSystemService', () => { { localReadRoots: [localRoot] }, ); - await expect( - svc.readTextFile({ path: symlinkPath }), - ).rejects.toMatchObject({ - code: INTERNAL_ERROR_CODE, + const err = await svc + .readTextFile({ path: symlinkPath }) + .catch((e: unknown) => e); + + expect(err).toBeInstanceOf(Error); + expect(err).toMatchObject({ + cause: pathOutsideWorkspaceError, message: `path escapes workspace: ${symlinkPath}`, }); + expect(Object.prototype.hasOwnProperty.call(err, 'code')).toBe(false); expect(fallback.readTextFile).not.toHaveBeenCalled(); }); }, @@ -447,10 +455,16 @@ describe('AcpFileSystemService', () => { { localReadRoots: [localRoot] }, ); - await expect(svc.readTextFile({ path: filePath })).rejects.toMatchObject({ - code: INTERNAL_ERROR_CODE, + const err = await svc + .readTextFile({ path: filePath }) + .catch((e: unknown) => e); + + expect(err).toBeInstanceOf(Error); + expect(err).toMatchObject({ + cause: pathOutsideWorkspaceError, message: `path escapes workspace: ${filePath}`, }); + expect(Object.prototype.hasOwnProperty.call(err, 'code')).toBe(false); expect(fallback.readTextFile).not.toHaveBeenCalled(); }); @@ -470,10 +484,16 @@ describe('AcpFileSystemService', () => { { localReadRoots: [''] }, ); - await expect(svc.readTextFile({ path: filePath })).rejects.toMatchObject({ - code: INTERNAL_ERROR_CODE, + const err = await svc + .readTextFile({ path: filePath }) + .catch((e: unknown) => e); + + expect(err).toBeInstanceOf(Error); + expect(err).toMatchObject({ + cause: pathOutsideWorkspaceError, message: `path escapes workspace: ${filePath}`, }); + expect(Object.prototype.hasOwnProperty.call(err, 'code')).toBe(false); expect(fallback.readTextFile).not.toHaveBeenCalled(); }); @@ -617,7 +637,7 @@ describe('AcpFileSystemService', () => { expect(client.writeTextFile).not.toHaveBeenCalled(); }); - it('normalizes plain object ACP write errors to Error instances with the original message', async () => { + it('normalizes plain object ACP write errors without exposing numeric codes as Node error codes', async () => { const writeError = { code: INTERNAL_ERROR_CODE, message: 'Write failed', @@ -642,9 +662,10 @@ describe('AcpFileSystemService', () => { expect(err).toBeInstanceOf(Error); expect(err).toMatchObject({ - code: INTERNAL_ERROR_CODE, + cause: writeError, message: 'Write failed', }); + expect(Object.prototype.hasOwnProperty.call(err, 'code')).toBe(false); expect(String(err)).toContain('Write failed'); expect(String(err)).not.toContain('[object Object]'); }); diff --git a/packages/cli/src/acp-integration/service/filesystem.ts b/packages/cli/src/acp-integration/service/filesystem.ts index 1512209805c..bb6ba4ed060 100644 --- a/packages/cli/src/acp-integration/service/filesystem.ts +++ b/packages/cli/src/acp-integration/service/filesystem.ts @@ -66,11 +66,16 @@ function getErrorKind(error: unknown): string | undefined { function normalizeError(error: unknown): Error { if (error instanceof Error) return error; - const normalized = new Error(getErrorMessage(error)) as Error & - Record; + const normalized = new Error(getErrorMessage(error), { + cause: error, + }) as Error & Record; if (isRecord(error)) { for (const [key, value] of Object.entries(error)) { - if (key !== 'message') { + if ( + key !== 'message' && + key !== 'cause' && + (key !== 'code' || typeof value === 'string') + ) { normalized[key] = value; } } diff --git a/packages/core/src/utils/fileUtils.test.ts b/packages/core/src/utils/fileUtils.test.ts index 315f6efa50a..a7844d47de1 100644 --- a/packages/core/src/utils/fileUtils.test.ts +++ b/packages/core/src/utils/fileUtils.test.ts @@ -1078,6 +1078,27 @@ describe('fileUtils', () => { expect(result.returnDisplay).not.toContain('[object Object]'); }); + it('should surface messages from plain object notebook read errors', async () => { + const notebookPath = path.join(tempRootDir, 'analysis.ipynb'); + actualNodeFs.writeFileSync(notebookPath, '{}'); + vi.spyOn(fs.promises, 'readFile').mockRejectedValueOnce({ + code: -32603, + message: 'notebook is outside allowed roots', + data: { + errorKind: 'path_outside_workspace', + status: 400, + }, + }); + + const result = await processSingleFileContent(notebookPath, mockConfig); + + expect(result.error).toContain('notebook is outside allowed roots'); + expect(result.returnDisplay).toContain('Error reading notebook'); + expect(result.llmContent).toContain('notebook is outside allowed roots'); + expect(result.error).not.toContain('[object Object]'); + expect(result.llmContent).not.toContain('[object Object]'); + }); + it('should handle read errors for image/pdf files', async () => { actualNodeFs.writeFileSync(testImageFilePath, 'content'); // File must exist mockMimeGetType.mockReturnValue('image/png'); diff --git a/packages/core/src/utils/fileUtils.ts b/packages/core/src/utils/fileUtils.ts index 2897c160f14..3dec25cf4b9 100644 --- a/packages/core/src/utils/fileUtils.ts +++ b/packages/core/src/utils/fileUtils.ts @@ -1222,7 +1222,7 @@ export async function processSingleFileContent( stats, }; } catch (e: unknown) { - const msg = e instanceof Error ? e.message : String(e); + const msg = getErrorMessage(e); return { llmContent: `Error parsing notebook ${relativePathForDisplay}: ${msg}`, returnDisplay: `Error reading notebook: ${relativePathForDisplay}`, From 8f7808efd37b8437e7dddc6ddb9e2ac5de081029 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Tue, 30 Jun 2026 03:43:07 +0800 Subject: [PATCH 08/18] fix(cli): harden ACP error normalization Co-authored-by: Qwen-Coder --- .../service/filesystem.test.ts | 32 +++- .../src/acp-integration/service/filesystem.ts | 16 +- packages/core/src/tools/write-file.test.ts | 24 +++ packages/core/src/tools/write-file.ts | 2 - .../check-serve-fast-path-bundle.test.js | 171 ++++++++++++++++++ 5 files changed, 224 insertions(+), 21 deletions(-) create mode 100644 scripts/tests/check-serve-fast-path-bundle.test.js diff --git a/packages/cli/src/acp-integration/service/filesystem.test.ts b/packages/cli/src/acp-integration/service/filesystem.test.ts index da4ca8520d5..bfa82705f19 100644 --- a/packages/cli/src/acp-integration/service/filesystem.test.ts +++ b/packages/cli/src/acp-integration/service/filesystem.test.ts @@ -161,12 +161,20 @@ describe('AcpFileSystemService', () => { expect(String(err)).not.toContain('[object Object]'); }); - it('preserves stack traces from plain object ACP errors', async () => { - const otherError = { - code: INTERNAL_ERROR_CODE, + it('does not copy unsafe properties from plain object ACP errors', async () => { + const otherError: Record = { + code: 'ABORT_ERR', message: 'Internal error', - stack: 'Original ACP stack', + stack: 'Remote ACP stack', + name: 'AbortError', + constructor: 'RemoteConstructor', + toString: 'not callable', + valueOf: 'not callable', }; + Object.defineProperty(otherError, '__proto__', { + value: { remotePrototype: true }, + enumerable: true, + }); const client = { readTextFile: vi.fn().mockRejectedValue(otherError), } as unknown as AgentSideConnection; @@ -183,7 +191,21 @@ describe('AcpFileSystemService', () => { .catch((e: unknown) => e); expect(err).toBeInstanceOf(Error); - expect((err as Error).stack).toBe('Original ACP stack'); + expect((err as Error).name).toBe('Error'); + expect((err as Error).stack).toContain('Internal error'); + expect(Object.prototype.hasOwnProperty.call(err, 'code')).toBe(false); + expect(Object.prototype.hasOwnProperty.call(err, 'constructor')).toBe( + false, + ); + expect(Object.prototype.hasOwnProperty.call(err, 'toString')).toBe(false); + expect(Object.prototype.hasOwnProperty.call(err, 'valueOf')).toBe(false); + expect(Object.prototype.hasOwnProperty.call(err, '__proto__')).toBe( + false, + ); + expect( + (err as Record)['remotePrototype'], + ).toBeUndefined(); + expect(String(err)).toContain('Internal error'); }); it('does not copy array entries onto normalized ACP errors', async () => { diff --git a/packages/cli/src/acp-integration/service/filesystem.ts b/packages/cli/src/acp-integration/service/filesystem.ts index bb6ba4ed060..91e2e8d0203 100644 --- a/packages/cli/src/acp-integration/service/filesystem.ts +++ b/packages/cli/src/acp-integration/service/filesystem.ts @@ -66,21 +66,9 @@ function getErrorKind(error: unknown): string | undefined { function normalizeError(error: unknown): Error { if (error instanceof Error) return error; - const normalized = new Error(getErrorMessage(error), { + return new Error(getErrorMessage(error), { cause: error, - }) as Error & Record; - if (isRecord(error)) { - for (const [key, value] of Object.entries(error)) { - if ( - key !== 'message' && - key !== 'cause' && - (key !== 'code' || typeof value === 'string') - ) { - normalized[key] = value; - } - } - } - return normalized; + }); } function createEnoentError(filePath: string): NodeJS.ErrnoException { diff --git a/packages/core/src/tools/write-file.test.ts b/packages/core/src/tools/write-file.test.ts index d4f3bdb7ec7..a8a265badd4 100644 --- a/packages/core/src/tools/write-file.test.ts +++ b/packages/core/src/tools/write-file.test.ts @@ -847,6 +847,30 @@ describe('WriteFileTool', () => { ); }); + it('should include cause details for non-Node write errors', async () => { + const filePath = path.join(rootDir, 'write_error_with_cause.txt'); + const content = 'test content'; + + vi.restoreAllMocks(); + + const cause = Object.assign(new Error(''), { code: 'ECONNREFUSED' }); + vi.spyOn(fsService, 'writeTextFile').mockRejectedValueOnce( + new TypeError('fetch failed', { cause }), + ); + + const params = { file_path: filePath, content }; + const invocation = tool.build(params); + const result = await invocation.execute(abortSignal); + + expect(result.error?.type).toBe(ToolErrorType.FILE_WRITE_FAILURE); + expect(result.llmContent).toContain( + 'Error writing to file: fetch failed (cause: ECONNREFUSED)', + ); + expect(result.returnDisplay).toContain( + 'Error writing to file: fetch failed (cause: ECONNREFUSED)', + ); + }); + it('should surface plain object write error messages without object stringification', async () => { const filePath = path.join(rootDir, 'plain_object_error_file.txt'); const content = 'test content'; diff --git a/packages/core/src/tools/write-file.ts b/packages/core/src/tools/write-file.ts index 5f8b2d2cb6a..c0a0951b6e9 100644 --- a/packages/core/src/tools/write-file.ts +++ b/packages/core/src/tools/write-file.ts @@ -612,8 +612,6 @@ class WriteFileToolInvocation extends BaseToolInvocation< if (this.config.getDebugMode() && error.stack) { debugLogger.debug('Write file error stack:', error.stack); } - } else if (error instanceof Error) { - errorMsg = `Error writing to file: ${error.message}`; } else { errorMsg = `Error writing to file: ${getErrorMessage(error)}`; } diff --git a/scripts/tests/check-serve-fast-path-bundle.test.js b/scripts/tests/check-serve-fast-path-bundle.test.js new file mode 100644 index 00000000000..a0e5280aa52 --- /dev/null +++ b/scripts/tests/check-serve-fast-path-bundle.test.js @@ -0,0 +1,171 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { describe, it, expect, afterEach } from 'vitest'; +import { + checkServeFastPathBundle, + findServeFastPathBundleOffenders, + formatServeFastPathBundleOffenders, + normalizeMetafilePath, +} from '../check-serve-fast-path-bundle.js'; + +const SERVE_ROOT_INPUTS = { + 'packages/cli/src/serve/fast-path.ts': { bytesInOutput: 1 }, + 'packages/cli/src/serve/fast-path-settings.ts': { bytesInOutput: 1 }, + 'packages/cli/src/serve/run-qwen-serve.ts': { bytesInOutput: 1 }, +}; + +function output({ inputs = {}, imports = [], bytes = 10 } = {}) { + return { inputs, imports, bytes }; +} + +function metafile(outputs) { + return { outputs }; +} + +describe('check-serve-fast-path-bundle', () => { + const tempDirs = []; + + afterEach(() => { + for (const tempDir of tempDirs.splice(0)) { + rmSync(tempDir, { recursive: true, force: true }); + } + }); + + it('normalizes metafile paths to slash-separated relative paths', () => { + expect(normalizeMetafilePath('.\\dist\\chunk.js')).toBe('dist/chunk.js'); + }); + + it('accepts a clean serve fast-path static closure', () => { + const offenders = findServeFastPathBundleOffenders( + metafile({ + 'dist/serve-fast-path.js': output({ inputs: SERVE_ROOT_INPUTS }), + }), + ); + + expect(offenders).toEqual([]); + }); + + it('reports forbidden source files in the static closure', () => { + const offenders = findServeFastPathBundleOffenders( + metafile({ + 'dist/serve-fast-path.js': output({ + inputs: SERVE_ROOT_INPUTS, + imports: [{ path: 'dist/chunk.js', kind: 'import-statement' }], + }), + 'dist/chunk.js': output({ + inputs: { + 'packages/core/src/tools/shell.ts': { bytesInOutput: 1 }, + }, + bytes: 42, + }), + }), + ); + + expect(offenders).toMatchObject([ + { + label: 'Core shell tool runtime', + matchedInput: 'packages/core/src/tools/shell.ts', + outputPath: 'dist/chunk.js', + bytes: 42, + importPath: ['dist/serve-fast-path.js', 'dist/chunk.js'], + }, + ]); + }); + + it('reports forbidden vendor packages pulled in transitively', () => { + const offenders = findServeFastPathBundleOffenders( + metafile({ + 'dist/serve-fast-path.js': output({ + inputs: SERVE_ROOT_INPUTS, + imports: [{ path: 'dist/a.js', kind: 'import-statement' }], + }), + 'dist/a.js': output({ + imports: [{ path: 'dist/b.js', kind: 'import-statement' }], + }), + 'dist/b.js': output({ + inputs: { + 'node_modules/glob/dist/esm/index.js': { bytesInOutput: 1 }, + }, + }), + }), + ); + + expect(offenders).toMatchObject([ + { + label: 'glob vendor package', + matchedInput: 'node_modules/glob/dist/esm/index.js', + outputPath: 'dist/b.js', + importPath: ['dist/serve-fast-path.js', 'dist/a.js', 'dist/b.js'], + }, + ]); + }); + + it('ignores forbidden modules behind dynamic imports', () => { + const offenders = findServeFastPathBundleOffenders( + metafile({ + 'dist/serve-fast-path.js': output({ + inputs: SERVE_ROOT_INPUTS, + imports: [{ path: 'dist/lazy.js', kind: 'dynamic-import' }], + }), + 'dist/lazy.js': output({ + inputs: { + 'packages/acp-bridge/src/bridge.ts': { bytesInOutput: 1 }, + }, + }), + }), + ); + + expect(offenders).toEqual([]); + }); + + it('throws a descriptive error when serve pre-listen roots are missing', () => { + expect(() => findServeFastPathBundleOffenders(metafile({}))).toThrow( + /Could not find bundled outputs for serve pre-listen roots/, + ); + }); + + it('formats offenders with the matched input, output, and static path', () => { + const formatted = formatServeFastPathBundleOffenders([ + { + label: 'Core shell tool runtime', + matchedInput: 'packages/core/src/tools/shell.ts', + outputPath: 'dist/chunk.js', + bytes: 42, + importPath: ['dist/serve-fast-path.js', 'dist/chunk.js'], + }, + ]); + + expect(formatted).toContain('- Core shell tool runtime'); + expect(formatted).toContain('input: packages/core/src/tools/shell.ts'); + expect(formatted).toContain('output: dist/chunk.js (42 bytes)'); + expect(formatted).toContain( + 'static path: dist/serve-fast-path.js -> dist/chunk.js', + ); + }); + + it('checks a metafile from disk', () => { + const tempDir = mkdtempSync(join(tmpdir(), 'qwen-code-metafile-')); + tempDirs.push(tempDir); + const metafilePath = join(tempDir, 'esbuild.json'); + writeFileSync( + metafilePath, + JSON.stringify( + metafile({ + 'dist/serve-fast-path.js': output({ inputs: SERVE_ROOT_INPUTS }), + }), + ), + ); + + expect(checkServeFastPathBundle({ metafilePath })).toEqual({ + ok: true, + offenders: [], + }); + }); +}); From 95a71d9ea54732e71f00527ccdd301b4f3e26d08 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Tue, 30 Jun 2026 06:31:43 +0800 Subject: [PATCH 09/18] codex: address PR review feedback (#6021) Co-authored-by: Qwen-Coder --- .../service/filesystem.test.ts | 73 +++++++++++++++++-- .../src/acp-integration/service/filesystem.ts | 52 +++++++------ packages/core/src/utils/errors.test.ts | 4 + packages/core/src/utils/errors.ts | 2 +- 4 files changed, 100 insertions(+), 31 deletions(-) diff --git a/packages/cli/src/acp-integration/service/filesystem.test.ts b/packages/cli/src/acp-integration/service/filesystem.test.ts index bfa82705f19..6d674fd5bc6 100644 --- a/packages/cli/src/acp-integration/service/filesystem.test.ts +++ b/packages/cli/src/acp-integration/service/filesystem.test.ts @@ -263,10 +263,57 @@ describe('AcpFileSystemService', () => { content: 'skill instructions', _meta: { bom: false, encoding: 'utf-8' }, }); - expect(fallback.readTextFile).toHaveBeenCalledWith({ path: filePath }); + expect(fallback.readTextFile).toHaveBeenCalledWith({ + path: await fs.realpath(filePath), + }); }); }); + it.skipIf(process.platform === 'win32')( + 'uses the resolved real path for local read fallback', + async () => { + await withTempRoot(async (tempRoot) => { + const localRoot = path.join(tempRoot, 'skills'); + const realFilePath = path.join(localRoot, 'instructions.md'); + const symlinkPath = path.join(localRoot, 'instructions-link.md'); + await fs.mkdir(localRoot, { recursive: true }); + await fs.writeFile(realFilePath, 'instructions', 'utf8'); + await fs.symlink(realFilePath, symlinkPath); + + const pathOutsideWorkspaceError = + createLocalReadFallbackError(symlinkPath); + const client = { + readTextFile: vi.fn().mockRejectedValue(pathOutsideWorkspaceError), + } as unknown as AgentSideConnection; + const fallback = createFallback(); + (fallback.readTextFile as ReturnType).mockResolvedValue( + { + content: 'instructions', + _meta: { bom: false, encoding: 'utf-8' }, + }, + ); + + const svc = new AcpFileSystemService( + client, + 'session-2c-real-fallback-path', + { readTextFile: true, writeTextFile: true }, + fallback, + { localReadRoots: [localRoot] }, + ); + + await expect( + svc.readTextFile({ path: symlinkPath }), + ).resolves.toEqual({ + content: 'instructions', + _meta: { bom: false, encoding: 'utf-8' }, + }); + expect(fallback.readTextFile).toHaveBeenCalledWith({ + path: await fs.realpath(realFilePath), + }); + }); + }, + ); + it('does not use top-level errorKind fields for local read fallback', async () => { await withTempRoot(async (tempRoot) => { const localRoot = path.join(tempRoot, 'skills'); @@ -387,7 +434,7 @@ describe('AcpFileSystemService', () => { _meta: { bom: false, encoding: 'utf-8' }, }); expect(fallback.readTextFile).toHaveBeenCalledWith({ - path: filePath, + path: await fs.realpath(filePath), }); }); }, @@ -425,7 +472,9 @@ describe('AcpFileSystemService', () => { content: 'instructions', _meta: { bom: false, encoding: 'utf-8' }, }); - expect(fallback.readTextFile).toHaveBeenCalledWith({ path: filePath }); + expect(fallback.readTextFile).toHaveBeenCalledWith({ + path: await fs.realpath(filePath), + }); }); }); @@ -442,8 +491,9 @@ describe('AcpFileSystemService', () => { readTextFile: vi.fn().mockRejectedValue(pathOutsideWorkspaceError), } as unknown as AgentSideConnection; const fallback = createFallback(); + const fallbackError = new Error('local read failed'); (fallback.readTextFile as ReturnType).mockRejectedValue( - new Error('local read failed'), + fallbackError, ); const svc = new AcpFileSystemService( @@ -454,9 +504,18 @@ describe('AcpFileSystemService', () => { { localReadRoots: [localRoot] }, ); - await expect(svc.readTextFile({ path: filePath })).rejects.toThrow( - `Local fallback read failed for ${filePath}: local read failed (original ACP error: path escapes workspace: ${filePath})`, - ); + const err = await svc + .readTextFile({ path: filePath }) + .catch((e: unknown) => e); + + expect(err).toBeInstanceOf(Error); + expect(err).toMatchObject({ + message: `Local fallback read failed for ${filePath}: local read failed (original ACP error: path escapes workspace: ${filePath})`, + cause: { + fallbackError, + acpError: pathOutsideWorkspaceError, + }, + }); }); }); diff --git a/packages/cli/src/acp-integration/service/filesystem.ts b/packages/cli/src/acp-integration/service/filesystem.ts index 91e2e8d0203..61c44cc8bb2 100644 --- a/packages/cli/src/acp-integration/service/filesystem.ts +++ b/packages/cli/src/acp-integration/service/filesystem.ts @@ -91,24 +91,17 @@ async function resolveRealPath(value: string): Promise { try { return await realpath(path.resolve(value)); - } catch { + } catch (error) { + if (getErrorCode(error) !== 'ENOENT') { + debugLogger.warn('realpath failed during ACP local read fallback check', { + path: value, + error: getErrorMessage(error), + }); + } return undefined; } } -async function isRealPathWithinRoot( - filePath: string, - root: string, -): Promise { - const [realFilePath, realRoot] = await Promise.all([ - resolveRealPath(filePath), - resolveRealPath(root), - ]); - if (!realFilePath || !realRoot) return false; - - return isSubpath(realRoot, realFilePath); -} - export class AcpFileSystemService implements FileSystemService { constructor( private readonly connection: AgentSideConnection, @@ -139,27 +132,34 @@ export class AcpFileSystemService implements FileSystemService { } const errorKind = getErrorKind(error); - if ( - isLocalReadFallbackErrorKind(errorKind) && - (await this.isLocalReadFallbackPath(params.path)) - ) { + const shouldTryLocalReadFallback = + isLocalReadFallbackErrorKind(errorKind); + const fallbackPath = shouldTryLocalReadFallback + ? await this.getLocalReadFallbackPath(params.path) + : undefined; + if (shouldTryLocalReadFallback && fallbackPath) { debugLogger.debug('Falling back to local read after ACP error', { path: params.path, + resolvedPath: fallbackPath, errorKind, error: getErrorMessage(error), }); try { - return await this.fallback.readTextFile(params); + return await this.fallback.readTextFile({ + ...params, + path: fallbackPath, + }); } catch (fallbackError) { debugLogger.warn('Local read fallback failed after ACP error', { path: params.path, + resolvedPath: fallbackPath, errorKind, originalError: getErrorMessage(error), fallbackError: getErrorMessage(fallbackError), }); throw new Error( `Local fallback read failed for ${params.path}: ${getErrorMessage(fallbackError)} (original ACP error: ${getErrorMessage(error)})`, - { cause: fallbackError }, + { cause: { fallbackError, acpError: error } }, ); } } @@ -199,10 +199,16 @@ export class AcpFileSystemService implements FileSystemService { return this.fallback.findFiles(fileName, searchPaths); } - private async isLocalReadFallbackPath(filePath: string): Promise { + private async getLocalReadFallbackPath( + filePath: string, + ): Promise { + const realFilePath = await resolveRealPath(filePath); + if (!realFilePath) return undefined; + for (const root of this.options.localReadRoots ?? []) { - if (await isRealPathWithinRoot(filePath, root)) return true; + const realRoot = await resolveRealPath(root); + if (realRoot && isSubpath(realRoot, realFilePath)) return realFilePath; } - return false; + return undefined; } } diff --git a/packages/core/src/utils/errors.test.ts b/packages/core/src/utils/errors.test.ts index 34e3142cbdf..954fd2b1a48 100644 --- a/packages/core/src/utils/errors.test.ts +++ b/packages/core/src/utils/errors.test.ts @@ -63,6 +63,10 @@ describe('getErrorMessage cause unwrapping', () => { expect(message.length).toBeLessThanOrEqual(1000); expect(message).toContain('"detail"'); }); + + it('uses String formatting for arrays', () => { + expect(getErrorMessage([1, 2, 3])).toBe('1,2,3'); + }); }); describe('isAbortError', () => { diff --git a/packages/core/src/utils/errors.ts b/packages/core/src/utils/errors.ts index d4c175f9772..4edf85fe8c5 100644 --- a/packages/core/src/utils/errors.ts +++ b/packages/core/src/utils/errors.ts @@ -97,7 +97,7 @@ export function getErrorMessage(error: unknown): string { } return error.message; } - if (error !== null && typeof error === 'object') { + if (error !== null && typeof error === 'object' && !Array.isArray(error)) { const message = (error as { message?: unknown }).message; if (typeof message === 'string' && message.trim()) { return message; From 7e13010d9dcc3d47fc0cbb292163d538eb72241e Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Tue, 30 Jun 2026 07:35:52 +0800 Subject: [PATCH 10/18] codex: address PR review feedback (#6021) Co-authored-by: Qwen-Coder --- package.json | 2 +- packages/core/src/utils/errors.test.ts | 15 +++++++++++++++ packages/core/src/utils/errors.ts | 12 ++++++++++-- scripts/tests/package-scripts.test.js | 25 +++++++++++++++++++++++++ 4 files changed, 51 insertions(+), 3 deletions(-) create mode 100644 scripts/tests/package-scripts.test.js diff --git a/package.json b/package.json index 72d9016af27..faa3d0d0c61 100644 --- a/package.json +++ b/package.json @@ -38,7 +38,7 @@ "build:sandbox": "node scripts/build_sandbox.js", "bundle": "npm run generate && node esbuild.config.js && node scripts/copy_bundle_assets.js", "test": "cross-env NODE_OPTIONS=\"--max-old-space-size=3072\" npm run test --workspaces --if-present --parallel", - "test:ci": "cross-env NODE_OPTIONS=\"--max-old-space-size=3072\" npm run test:ci --workspaces --if-present --parallel && npm run test:scripts", + "test:ci": "cross-env NODE_OPTIONS=\"--max-old-space-size=3072\" npm run test:ci --workspaces --if-present --parallel && npm run test:scripts && npm run check:serve-fast-path-bundle", "test:scripts": "vitest run --config ./scripts/tests/vitest.config.ts", "test:e2e": "cross-env VERBOSE=true KEEP_OUTPUT=true npm run test:integration:sandbox:none", "test:integration:all": "npm run test:integration:sandbox:none && npm run test:integration:sandbox:docker && npm run test:integration:sandbox:podman", diff --git a/packages/core/src/utils/errors.test.ts b/packages/core/src/utils/errors.test.ts index 954fd2b1a48..b18bfb80550 100644 --- a/packages/core/src/utils/errors.test.ts +++ b/packages/core/src/utils/errors.test.ts @@ -53,6 +53,21 @@ describe('getErrorMessage cause unwrapping', () => { ).toBe('path escapes workspace: /root/.qwen/skills/example.md'); }); + it('surfaces cause details from plain error-like objects', () => { + expect( + getErrorMessage({ + message: 'fetch failed', + cause: { code: 'ECONNREFUSED' }, + }), + ).toBe('fetch failed (cause: ECONNREFUSED)'); + }); + + it('bounds long messages from plain error-like objects', () => { + const message = getErrorMessage({ message: 'x'.repeat(2000) }); + + expect(message).toBe(`${'x'.repeat(997)}...`); + }); + it('stringifies plain objects without a message', () => { expect(getErrorMessage({ code: -32603 })).toBe('{"code":-32603}'); }); diff --git a/packages/core/src/utils/errors.ts b/packages/core/src/utils/errors.ts index 4edf85fe8c5..1421e83bd1f 100644 --- a/packages/core/src/utils/errors.ts +++ b/packages/core/src/utils/errors.ts @@ -98,9 +98,17 @@ export function getErrorMessage(error: unknown): string { return error.message; } if (error !== null && typeof error === 'object' && !Array.isArray(error)) { - const message = (error as { message?: unknown }).message; + const { message, cause } = error as { + message?: unknown; + cause?: unknown; + }; if (typeof message === 'string' && message.trim()) { - return message; + const detail = describeErrorCause(cause); + const result = + detail && detail !== message + ? `${message} (cause: ${detail})` + : message; + return truncateStringifiedErrorMessage(result); } try { const serialized = JSON.stringify(error); diff --git a/scripts/tests/package-scripts.test.js b/scripts/tests/package-scripts.test.js new file mode 100644 index 00000000000..366db5758a8 --- /dev/null +++ b/scripts/tests/package-scripts.test.js @@ -0,0 +1,25 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { readFileSync } from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { describe, expect, it } from 'vitest'; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const root = path.resolve(__dirname, '../..'); + +describe('package scripts', () => { + it('runs the serve fast-path bundle check in CI tests', () => { + const packageJson = JSON.parse( + readFileSync(path.join(root, 'package.json'), 'utf8'), + ); + + expect(packageJson.scripts['test:ci']).toContain( + 'npm run check:serve-fast-path-bundle', + ); + }); +}); From 29ae1598cc6ba5d7e044ae55c418b0772460becf Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Tue, 30 Jun 2026 09:25:31 +0800 Subject: [PATCH 11/18] codex: address PR review feedback (#6021) Co-authored-by: Qwen-Coder --- .../service/filesystem.test.ts | 28 +++++++++++++++++++ .../src/acp-integration/service/filesystem.ts | 3 ++ scripts/check-serve-fast-path-bundle.js | 2 +- .../check-serve-fast-path-bundle.test.js | 11 ++++++++ 4 files changed, 43 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/acp-integration/service/filesystem.test.ts b/packages/cli/src/acp-integration/service/filesystem.test.ts index 6d674fd5bc6..3514ae2f648 100644 --- a/packages/cli/src/acp-integration/service/filesystem.test.ts +++ b/packages/cli/src/acp-integration/service/filesystem.test.ts @@ -750,5 +750,33 @@ describe('AcpFileSystemService', () => { expect(String(err)).toContain('Write failed'); expect(String(err)).not.toContain('[object Object]'); }); + + it('converts RESOURCE_NOT_FOUND write errors to ENOENT', async () => { + const resourceNotFoundError = { + code: RESOURCE_NOT_FOUND_CODE, + message: 'File not found', + }; + const client = { + writeTextFile: vi.fn().mockRejectedValue(resourceNotFoundError), + } as unknown as AgentSideConnection; + + const svc = new AcpFileSystemService( + client, + 'session-9', + { readTextFile: true, writeTextFile: true }, + createFallback(), + ); + + await expect( + svc.writeTextFile({ + path: '/some/file.txt', + content: 'hello', + }), + ).rejects.toMatchObject({ + code: 'ENOENT', + errno: -2, + path: '/some/file.txt', + }); + }); }); }); diff --git a/packages/cli/src/acp-integration/service/filesystem.ts b/packages/cli/src/acp-integration/service/filesystem.ts index 61c44cc8bb2..abaed09a7db 100644 --- a/packages/cli/src/acp-integration/service/filesystem.ts +++ b/packages/cli/src/acp-integration/service/filesystem.ts @@ -189,6 +189,9 @@ export class AcpFileSystemService implements FileSystemService { sessionId: this.sessionId, }); } catch (error) { + if (getErrorCode(error) === RESOURCE_NOT_FOUND_CODE) { + throw createEnoentError(params.path); + } throw normalizeError(error); } diff --git a/scripts/check-serve-fast-path-bundle.js b/scripts/check-serve-fast-path-bundle.js index e05c8e24198..c8be9c9731c 100644 --- a/scripts/check-serve-fast-path-bundle.js +++ b/scripts/check-serve-fast-path-bundle.js @@ -11,7 +11,7 @@ import { fileURLToPath } from 'node:url'; const DEFAULT_METAFILE_PATH = resolve('dist/esbuild.json'); const METAFILE_BUILD_COMMAND = - 'npm run build -- --cli-only && npx cross-env DEV=true npm run bundle'; + 'npm run build -- --cli-only && cross-env DEV=true npm run bundle'; const SERVE_PRE_LISTEN_ROOTS = [ { label: 'serve fast path entry', diff --git a/scripts/tests/check-serve-fast-path-bundle.test.js b/scripts/tests/check-serve-fast-path-bundle.test.js index a0e5280aa52..0ef0d0c5a37 100644 --- a/scripts/tests/check-serve-fast-path-bundle.test.js +++ b/scripts/tests/check-serve-fast-path-bundle.test.js @@ -168,4 +168,15 @@ describe('check-serve-fast-path-bundle', () => { offenders: [], }); }); + + it('throws a descriptive error for invalid JSON metafiles', () => { + const tempDir = mkdtempSync(join(tmpdir(), 'qwen-code-bad-meta-')); + tempDirs.push(tempDir); + const metafilePath = join(tempDir, 'esbuild.json'); + writeFileSync(metafilePath, 'not json'); + + expect(() => checkServeFastPathBundle({ metafilePath })).toThrow( + /Invalid esbuild metafile.*Run `npm run build -- --cli-only && cross-env DEV=true npm run bundle` to regenerate it\./s, + ); + }); }); From 65733cdcb84cdee30415e3e750271f9dd905dad6 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Tue, 30 Jun 2026 10:24:59 +0800 Subject: [PATCH 12/18] codex: address PR review feedback (#6021) Co-authored-by: Qwen-Coder --- .../service/filesystem.test.ts | 144 ++++++++++++------ .../src/acp-integration/service/filesystem.ts | 30 +++- packages/core/src/tools/edit.test.ts | 46 ++++++ 3 files changed, 172 insertions(+), 48 deletions(-) diff --git a/packages/cli/src/acp-integration/service/filesystem.test.ts b/packages/cli/src/acp-integration/service/filesystem.test.ts index 3514ae2f648..18922266a4e 100644 --- a/packages/cli/src/acp-integration/service/filesystem.test.ts +++ b/packages/cli/src/acp-integration/service/filesystem.test.ts @@ -519,63 +519,117 @@ describe('AcpFileSystemService', () => { }); }); + it('falls back to local reads for missing files under allowed local roots', async () => { + await withTempRoot(async (tempRoot) => { + const localRoot = path.join(tempRoot, 'skills'); + const filePath = path.join(localRoot, 'missing.md'); + await fs.mkdir(localRoot, { recursive: true }); + + const pathOutsideWorkspaceError = + createLocalReadFallbackError(filePath); + const client = { + readTextFile: vi.fn().mockRejectedValue(pathOutsideWorkspaceError), + } as unknown as AgentSideConnection; + const fallback = createFallback(); + const missingFileError = Object.assign( + new Error(`File not found: ${filePath}`), + { + code: 'ENOENT', + errno: -2, + path: filePath, + }, + ); + (fallback.readTextFile as ReturnType).mockRejectedValue( + missingFileError, + ); + + const svc = new AcpFileSystemService( + client, + 'session-2d-missing-local-file', + { readTextFile: true, writeTextFile: true }, + fallback, + { localReadRoots: [localRoot] }, + ); + + await expect(svc.readTextFile({ path: filePath })).rejects.toBe( + missingFileError, + ); + expect(fallback.readTextFile).toHaveBeenCalledWith({ + path: filePath, + }); + }); + }); + it('does not fall back to local reads outside configured roots', async () => { - const localRoot = path.join(os.tmpdir(), 'acp-local-read-root'); - const filePath = path.join(os.tmpdir(), 'outside-local-root.md'); - const pathOutsideWorkspaceError = createLocalReadFallbackError(filePath); - const client = { - readTextFile: vi.fn().mockRejectedValue(pathOutsideWorkspaceError), - } as unknown as AgentSideConnection; - const fallback = createFallback(); + await withTempRoot(async (tempRoot) => { + const localRoot = path.join(tempRoot, 'allowed'); + const outsideRoot = path.join(tempRoot, 'outside'); + const filePath = path.join(outsideRoot, 'outside-local-root.md'); + await fs.mkdir(localRoot, { recursive: true }); + await fs.mkdir(outsideRoot, { recursive: true }); + await fs.writeFile(filePath, 'outside local root', 'utf8'); - const svc = new AcpFileSystemService( - client, - 'session-2e', - { readTextFile: true, writeTextFile: true }, - fallback, - { localReadRoots: [localRoot] }, - ); + const pathOutsideWorkspaceError = + createLocalReadFallbackError(filePath); + const client = { + readTextFile: vi.fn().mockRejectedValue(pathOutsideWorkspaceError), + } as unknown as AgentSideConnection; + const fallback = createFallback(); - const err = await svc - .readTextFile({ path: filePath }) - .catch((e: unknown) => e); + const svc = new AcpFileSystemService( + client, + 'session-2e', + { readTextFile: true, writeTextFile: true }, + fallback, + { localReadRoots: [localRoot] }, + ); - expect(err).toBeInstanceOf(Error); - expect(err).toMatchObject({ - cause: pathOutsideWorkspaceError, - message: `path escapes workspace: ${filePath}`, + const err = await svc + .readTextFile({ path: filePath }) + .catch((e: unknown) => e); + + expect(err).toBeInstanceOf(Error); + expect(err).toMatchObject({ + cause: pathOutsideWorkspaceError, + message: `path escapes workspace: ${filePath}`, + }); + expect(Object.prototype.hasOwnProperty.call(err, 'code')).toBe(false); + expect(fallback.readTextFile).not.toHaveBeenCalled(); }); - expect(Object.prototype.hasOwnProperty.call(err, 'code')).toBe(false); - expect(fallback.readTextFile).not.toHaveBeenCalled(); }); it('ignores empty configured local read roots', async () => { - const filePath = path.join(process.cwd(), 'outside-workspace.md'); - const pathOutsideWorkspaceError = createLocalReadFallbackError(filePath); - const client = { - readTextFile: vi.fn().mockRejectedValue(pathOutsideWorkspaceError), - } as unknown as AgentSideConnection; - const fallback = createFallback(); + await withTempRoot(async (tempRoot) => { + const filePath = path.join(tempRoot, 'outside-workspace.md'); + await fs.writeFile(filePath, 'outside workspace', 'utf8'); - const svc = new AcpFileSystemService( - client, - 'session-2f', - { readTextFile: true, writeTextFile: true }, - fallback, - { localReadRoots: [''] }, - ); + const pathOutsideWorkspaceError = + createLocalReadFallbackError(filePath); + const client = { + readTextFile: vi.fn().mockRejectedValue(pathOutsideWorkspaceError), + } as unknown as AgentSideConnection; + const fallback = createFallback(); - const err = await svc - .readTextFile({ path: filePath }) - .catch((e: unknown) => e); + const svc = new AcpFileSystemService( + client, + 'session-2f', + { readTextFile: true, writeTextFile: true }, + fallback, + { localReadRoots: [''] }, + ); - expect(err).toBeInstanceOf(Error); - expect(err).toMatchObject({ - cause: pathOutsideWorkspaceError, - message: `path escapes workspace: ${filePath}`, + const err = await svc + .readTextFile({ path: filePath }) + .catch((e: unknown) => e); + + expect(err).toBeInstanceOf(Error); + expect(err).toMatchObject({ + cause: pathOutsideWorkspaceError, + message: `path escapes workspace: ${filePath}`, + }); + expect(Object.prototype.hasOwnProperty.call(err, 'code')).toBe(false); + expect(fallback.readTextFile).not.toHaveBeenCalled(); }); - expect(Object.prototype.hasOwnProperty.call(err, 'code')).toBe(false); - expect(fallback.readTextFile).not.toHaveBeenCalled(); }); it('uses fallback when readTextFile capability is disabled', async () => { diff --git a/packages/cli/src/acp-integration/service/filesystem.ts b/packages/cli/src/acp-integration/service/filesystem.ts index abaed09a7db..74fdfa877d6 100644 --- a/packages/cli/src/acp-integration/service/filesystem.ts +++ b/packages/cli/src/acp-integration/service/filesystem.ts @@ -102,6 +102,21 @@ async function resolveRealPath(value: string): Promise { } } +async function resolveNearestExistingParent( + filePath: string, +): Promise { + let current = path.dirname(path.resolve(filePath)); + + while (true) { + const realCurrent = await resolveRealPath(current); + if (realCurrent) return realCurrent; + + const parent = path.dirname(current); + if (parent === current) return undefined; + current = parent; + } +} + export class AcpFileSystemService implements FileSystemService { constructor( private readonly connection: AgentSideConnection, @@ -150,6 +165,10 @@ export class AcpFileSystemService implements FileSystemService { path: fallbackPath, }); } catch (fallbackError) { + if (getErrorCode(fallbackError) === 'ENOENT') { + throw fallbackError; + } + debugLogger.warn('Local read fallback failed after ACP error', { path: params.path, resolvedPath: fallbackPath, @@ -205,12 +224,17 @@ export class AcpFileSystemService implements FileSystemService { private async getLocalReadFallbackPath( filePath: string, ): Promise { - const realFilePath = await resolveRealPath(filePath); - if (!realFilePath) return undefined; + const normalizedFilePath = path.resolve(filePath); + const realFilePath = await resolveRealPath(normalizedFilePath); + const comparisonPath = + realFilePath ?? (await resolveNearestExistingParent(normalizedFilePath)); + if (!comparisonPath) return undefined; for (const root of this.options.localReadRoots ?? []) { const realRoot = await resolveRealPath(root); - if (realRoot && isSubpath(realRoot, realFilePath)) return realFilePath; + if (realRoot && isSubpath(realRoot, comparisonPath)) { + return realFilePath ?? normalizedFilePath; + } } return undefined; } diff --git a/packages/core/src/tools/edit.test.ts b/packages/core/src/tools/edit.test.ts index 83daf1db3a7..15fb31ddcb5 100644 --- a/packages/core/src/tools/edit.test.ts +++ b/packages/core/src/tools/edit.test.ts @@ -451,6 +451,30 @@ describe('EditTool', () => { ).rejects.toThrow(); }); + it('should surface plain object read errors without object stringification', async () => { + fs.writeFileSync(filePath, 'some old content here'); + seedPriorRead(filePath); + vi.spyOn(fsService, 'readTextFile').mockRejectedValueOnce({ + message: 'Plain object read error', + }); + + const params: EditToolParams = { + file_path: filePath, + old_string: 'old', + new_string: 'new', + }; + const invocation = tool.build(params); + const err = await invocation + .getConfirmationDetails(new AbortController().signal) + .catch((error: unknown) => error); + + expect(err).toBeInstanceOf(Error); + expect((err as Error).message).toContain( + 'Error preparing edit: Plain object read error', + ); + expect((err as Error).message).not.toContain('[object Object]'); + }); + it('should request confirmation for creating a new file (empty old_string)', async () => { const newFileName = 'new_file.txt'; const newFilePath = path.join(rootDir, newFileName); @@ -1074,6 +1098,28 @@ describe('EditTool', () => { expect(result.error?.type).toBe(ToolErrorType.EDIT_NO_CHANGE); }); + it('should return EDIT_PREPARATION_FAILURE with plain object read error messages', async () => { + fs.writeFileSync(filePath, 'content', 'utf8'); + seedPriorRead(filePath); + vi.spyOn(fsService, 'readTextFile').mockRejectedValueOnce({ + message: 'Plain object read error', + }); + + const params: EditToolParams = { + file_path: filePath, + old_string: 'content', + new_string: 'new content', + }; + const invocation = tool.build(params); + const result = await invocation.execute(new AbortController().signal); + + expect(result.error?.type).toBe(ToolErrorType.EDIT_PREPARATION_FAILURE); + expect(result.llmContent).toContain( + 'Error preparing edit: Plain object read error', + ); + expect(result.llmContent).not.toContain('[object Object]'); + }); + it('should throw INVALID_PARAMETERS error for relative path', async () => { const params: EditToolParams = { file_path: 'relative/path.txt', From 11405196b0dbe8c4da81ad3a1f7d3aebf20fca5c Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Tue, 30 Jun 2026 11:56:20 +0800 Subject: [PATCH 13/18] codex: address PR review feedback (#6021) Co-authored-by: Qwen-Coder --- .../service/filesystem.test.ts | 136 +++++++++++++++++- .../src/acp-integration/service/filesystem.ts | 27 +++- packages/core/src/tools/read-file.test.ts | 13 ++ 3 files changed, 170 insertions(+), 6 deletions(-) diff --git a/packages/cli/src/acp-integration/service/filesystem.test.ts b/packages/cli/src/acp-integration/service/filesystem.test.ts index 18922266a4e..233b21cc793 100644 --- a/packages/cli/src/acp-integration/service/filesystem.test.ts +++ b/packages/cli/src/acp-integration/service/filesystem.test.ts @@ -4,11 +4,29 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { describe, expect, it, vi } from 'vitest'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const mockDebugLogger = vi.hoisted(() => ({ + debug: vi.fn(), + warn: vi.fn(), +})); + +vi.mock('@qwen-code/qwen-code-core', async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + createDebugLogger: vi.fn(() => mockDebugLogger), + }; +}); + +vi.mock('node:fs/promises', { spy: true }); + import type { FileSystemService } from '@qwen-code/qwen-code-core'; import { AcpFileSystemService } from './filesystem.js'; import type { AgentSideConnection } from '@agentclientprotocol/sdk'; import { promises as fs } from 'node:fs'; +import { realpath as fsRealpath } from 'node:fs/promises'; import os from 'node:os'; import path from 'node:path'; @@ -57,6 +75,12 @@ const createFallback = (): FileSystemService => ({ }); describe('AcpFileSystemService', () => { + beforeEach(() => { + mockDebugLogger.debug.mockClear(); + mockDebugLogger.warn.mockClear(); + vi.mocked(fsRealpath).mockClear(); + }); + describe('readTextFile', () => { it('reads through ACP and returns response', async () => { const mockResponse = { @@ -228,6 +252,34 @@ describe('AcpFileSystemService', () => { expect(Object.prototype.hasOwnProperty.call(err, '0')).toBe(false); }); + it('includes cause details from plain object ACP errors', async () => { + const otherError = { + code: INTERNAL_ERROR_CODE, + message: 'fetch failed', + cause: { code: 'ECONNREFUSED' }, + }; + const client = { + readTextFile: vi.fn().mockRejectedValue(otherError), + } as unknown as AgentSideConnection; + + const svc = new AcpFileSystemService( + client, + 'session-2b-cause', + { readTextFile: true, writeTextFile: true }, + createFallback(), + ); + + const err = await svc + .readTextFile({ path: '/some/file.txt' }) + .catch((e: unknown) => e); + + expect(err).toBeInstanceOf(Error); + expect(err).toMatchObject({ + cause: otherError, + message: 'fetch failed (cause: ECONNREFUSED)', + }); + }); + it('falls back to local reads for allowed local roots when ACP rejects them as outside the workspace', async () => { await withTempRoot(async (tempRoot) => { const skillRoot = path.join(tempRoot, 'skills'); @@ -598,6 +650,88 @@ describe('AcpFileSystemService', () => { }); }); + it('logs when a local read fallback is eligible but skipped', async () => { + await withTempRoot(async (tempRoot) => { + const localRoot = path.join(tempRoot, 'allowed'); + const outsideRoot = path.join(tempRoot, 'outside'); + const filePath = path.join(outsideRoot, 'outside-local-root.md'); + await fs.mkdir(localRoot, { recursive: true }); + await fs.mkdir(outsideRoot, { recursive: true }); + await fs.writeFile(filePath, 'outside local root', 'utf8'); + + const pathOutsideWorkspaceError = + createLocalReadFallbackError(filePath); + const client = { + readTextFile: vi.fn().mockRejectedValue(pathOutsideWorkspaceError), + } as unknown as AgentSideConnection; + + const svc = new AcpFileSystemService( + client, + 'session-2e-skipped-log', + { readTextFile: true, writeTextFile: true }, + createFallback(), + { localReadRoots: [localRoot] }, + ); + + await svc.readTextFile({ path: filePath }).catch(() => undefined); + + expect(mockDebugLogger.debug).toHaveBeenCalledWith( + 'Local read fallback skipped - no matching root', + { + path: filePath, + errorKind: 'path_outside_workspace', + }, + ); + }); + }); + + it('resolves configured local read roots only once across fallback reads', async () => { + await withTempRoot(async (tempRoot) => { + const localRoot = path.join(tempRoot, 'skills'); + const firstFilePath = path.join(localRoot, 'first.md'); + const secondFilePath = path.join(localRoot, 'second.md'); + await fs.mkdir(localRoot, { recursive: true }); + await fs.writeFile(firstFilePath, 'first', 'utf8'); + await fs.writeFile(secondFilePath, 'second', 'utf8'); + + const client = { + readTextFile: vi + .fn() + .mockRejectedValueOnce(createLocalReadFallbackError(firstFilePath)) + .mockRejectedValueOnce( + createLocalReadFallbackError(secondFilePath), + ), + } as unknown as AgentSideConnection; + const fallback = createFallback(); + (fallback.readTextFile as ReturnType) + .mockResolvedValueOnce({ + content: 'first', + _meta: { bom: false, encoding: 'utf-8' }, + }) + .mockResolvedValueOnce({ + content: 'second', + _meta: { bom: false, encoding: 'utf-8' }, + }); + + const svc = new AcpFileSystemService( + client, + 'session-2e-root-cache', + { readTextFile: true, writeTextFile: true }, + fallback, + { localReadRoots: [localRoot] }, + ); + + await svc.readTextFile({ path: firstFilePath }); + await svc.readTextFile({ path: secondFilePath }); + + const resolvedLocalRoot = path.resolve(localRoot); + const localRootRealpathCalls = vi + .mocked(fsRealpath) + .mock.calls.filter(([value]) => value === resolvedLocalRoot); + expect(localRootRealpathCalls).toHaveLength(1); + }); + }); + it('ignores empty configured local read roots', async () => { await withTempRoot(async (tempRoot) => { const filePath = path.join(tempRoot, 'outside-workspace.md'); diff --git a/packages/cli/src/acp-integration/service/filesystem.ts b/packages/cli/src/acp-integration/service/filesystem.ts index 74fdfa877d6..4b765df816e 100644 --- a/packages/cli/src/acp-integration/service/filesystem.ts +++ b/packages/cli/src/acp-integration/service/filesystem.ts @@ -16,8 +16,11 @@ import type { FileSystemService, ReadTextFileResponse, } from '@qwen-code/qwen-code-core'; -import { createDebugLogger, isSubpath } from '@qwen-code/qwen-code-core'; -import { getErrorMessage } from '../../utils/errors.js'; +import { + createDebugLogger, + getErrorMessage, + isSubpath, +} from '@qwen-code/qwen-code-core'; import { realpath } from 'node:fs/promises'; import path from 'node:path'; @@ -118,6 +121,8 @@ async function resolveNearestExistingParent( } export class AcpFileSystemService implements FileSystemService { + private resolvedLocalReadRoots?: Promise; + constructor( private readonly connection: AgentSideConnection, private readonly sessionId: string, @@ -152,6 +157,12 @@ export class AcpFileSystemService implements FileSystemService { const fallbackPath = shouldTryLocalReadFallback ? await this.getLocalReadFallbackPath(params.path) : undefined; + if (shouldTryLocalReadFallback && !fallbackPath) { + debugLogger.debug('Local read fallback skipped - no matching root', { + path: params.path, + errorKind, + }); + } if (shouldTryLocalReadFallback && fallbackPath) { debugLogger.debug('Falling back to local read after ACP error', { path: params.path, @@ -221,6 +232,13 @@ export class AcpFileSystemService implements FileSystemService { return this.fallback.findFiles(fileName, searchPaths); } + private getResolvedLocalReadRoots(): Promise { + this.resolvedLocalReadRoots ??= Promise.all( + (this.options.localReadRoots ?? []).map(resolveRealPath), + ).then((roots) => roots.filter((root): root is string => Boolean(root))); + return this.resolvedLocalReadRoots; + } + private async getLocalReadFallbackPath( filePath: string, ): Promise { @@ -230,9 +248,8 @@ export class AcpFileSystemService implements FileSystemService { realFilePath ?? (await resolveNearestExistingParent(normalizedFilePath)); if (!comparisonPath) return undefined; - for (const root of this.options.localReadRoots ?? []) { - const realRoot = await resolveRealPath(root); - if (realRoot && isSubpath(realRoot, comparisonPath)) { + for (const realRoot of await this.getResolvedLocalReadRoots()) { + if (isSubpath(realRoot, comparisonPath)) { return realFilePath ?? normalizedFilePath; } } diff --git a/packages/core/src/tools/read-file.test.ts b/packages/core/src/tools/read-file.test.ts index fb55741b5b7..08fe5edcb89 100644 --- a/packages/core/src/tools/read-file.test.ts +++ b/packages/core/src/tools/read-file.test.ts @@ -222,6 +222,19 @@ describe('ReadFileTool', () => { expect(permission).toBe('allow'); }); + it('should return allow for paths within the user extensions directory', async () => { + const params: ReadFileToolParams = { + file_path: path.join( + Storage.getUserExtensionsDir(), + 'my-ext', + 'index.js', + ), + }; + const invocation = tool.build(params); + const permission = await invocation.getDefaultPermission(); + expect(permission).toBe('allow'); + }); + it('should return ask for paths directly under the OS temp directory', async () => { const params: ReadFileToolParams = { file_path: path.join(os.tmpdir(), 'pr-review-context.md'), From 91b1d72e6e604f8a958e975f55a82486f43bb0a3 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Tue, 30 Jun 2026 13:37:29 +0800 Subject: [PATCH 14/18] codex: fix Windows CI path expectation (#6021) Co-authored-by: Qwen-Coder --- packages/cli/src/acp-integration/acpAgent.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/src/acp-integration/acpAgent.test.ts b/packages/cli/src/acp-integration/acpAgent.test.ts index ae098dbe426..6b0d739bf96 100644 --- a/packages/cli/src/acp-integration/acpAgent.test.ts +++ b/packages/cli/src/acp-integration/acpAgent.test.ts @@ -1276,7 +1276,7 @@ describe('QwenAgent MCP SSE/HTTP support', () => { { localReadRoots: [ '/project/.qwen/tmp', - '/project/subagents', + path.join('/project', 'subagents'), '/tmp/qwen-global-temp', '/project/.qwen/memory', '/tmp/user-memory', From ead2067daa0838799bf480dd9fe27192c9c5b636 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Tue, 30 Jun 2026 14:26:37 +0800 Subject: [PATCH 15/18] codex: address PR review feedback (#6021) Co-authored-by: Qwen-Coder --- .../service/filesystem.test.ts | 138 +++++++++++++++--- .../src/acp-integration/service/filesystem.ts | 35 +---- 2 files changed, 126 insertions(+), 47 deletions(-) diff --git a/packages/cli/src/acp-integration/service/filesystem.test.ts b/packages/cli/src/acp-integration/service/filesystem.test.ts index 233b21cc793..dc3334f7493 100644 --- a/packages/cli/src/acp-integration/service/filesystem.test.ts +++ b/packages/cli/src/acp-integration/service/filesystem.test.ts @@ -571,7 +571,7 @@ describe('AcpFileSystemService', () => { }); }); - it('falls back to local reads for missing files under allowed local roots', async () => { + it('does not fall back to local reads for missing files under allowed local roots', async () => { await withTempRoot(async (tempRoot) => { const localRoot = path.join(tempRoot, 'skills'); const filePath = path.join(localRoot, 'missing.md'); @@ -583,17 +583,6 @@ describe('AcpFileSystemService', () => { readTextFile: vi.fn().mockRejectedValue(pathOutsideWorkspaceError), } as unknown as AgentSideConnection; const fallback = createFallback(); - const missingFileError = Object.assign( - new Error(`File not found: ${filePath}`), - { - code: 'ENOENT', - errno: -2, - path: filePath, - }, - ); - (fallback.readTextFile as ReturnType).mockRejectedValue( - missingFileError, - ); const svc = new AcpFileSystemService( client, @@ -603,12 +592,16 @@ describe('AcpFileSystemService', () => { { localReadRoots: [localRoot] }, ); - await expect(svc.readTextFile({ path: filePath })).rejects.toBe( - missingFileError, - ); - expect(fallback.readTextFile).toHaveBeenCalledWith({ - path: filePath, + const err = await svc + .readTextFile({ path: filePath }) + .catch((e: unknown) => e); + + expect(err).toBeInstanceOf(Error); + expect(err).toMatchObject({ + cause: pathOutsideWorkspaceError, + message: `path escapes workspace: ${filePath}`, }); + expect(fallback.readTextFile).not.toHaveBeenCalled(); }); }); @@ -676,7 +669,7 @@ describe('AcpFileSystemService', () => { await svc.readTextFile({ path: filePath }).catch(() => undefined); expect(mockDebugLogger.debug).toHaveBeenCalledWith( - 'Local read fallback skipped - no matching root', + 'Local read fallback skipped - no safe local path', { path: filePath, errorKind: 'path_outside_workspace', @@ -685,7 +678,7 @@ describe('AcpFileSystemService', () => { }); }); - it('resolves configured local read roots only once across fallback reads', async () => { + it('resolves configured local read roots for each fallback read', async () => { await withTempRoot(async (tempRoot) => { const localRoot = path.join(tempRoot, 'skills'); const firstFilePath = path.join(localRoot, 'first.md'); @@ -728,7 +721,112 @@ describe('AcpFileSystemService', () => { const localRootRealpathCalls = vi .mocked(fsRealpath) .mock.calls.filter(([value]) => value === resolvedLocalRoot); - expect(localRootRealpathCalls).toHaveLength(1); + expect(localRootRealpathCalls).toHaveLength(2); + }); + }); + + it('allows lazily-created local read roots on later fallback reads', async () => { + await withTempRoot(async (tempRoot) => { + const localRoot = path.join(tempRoot, 'skills'); + const firstFilePath = path.join(localRoot, 'missing.md'); + const secondFilePath = path.join(localRoot, 'instructions.md'); + + const client = { + readTextFile: vi + .fn() + .mockRejectedValueOnce(createLocalReadFallbackError(firstFilePath)) + .mockRejectedValueOnce( + createLocalReadFallbackError(secondFilePath), + ), + } as unknown as AgentSideConnection; + const fallback = createFallback(); + (fallback.readTextFile as ReturnType).mockResolvedValue({ + content: 'instructions', + _meta: { bom: false, encoding: 'utf-8' }, + }); + + const svc = new AcpFileSystemService( + client, + 'session-2e-lazy-root', + { readTextFile: true, writeTextFile: true }, + fallback, + { localReadRoots: [localRoot] }, + ); + + await svc.readTextFile({ path: firstFilePath }).catch(() => undefined); + expect(fallback.readTextFile).not.toHaveBeenCalled(); + + await fs.mkdir(localRoot, { recursive: true }); + await fs.writeFile(secondFilePath, 'instructions', 'utf8'); + + await expect( + svc.readTextFile({ path: secondFilePath }), + ).resolves.toEqual({ + content: 'instructions', + _meta: { bom: false, encoding: 'utf-8' }, + }); + expect(fallback.readTextFile).toHaveBeenCalledWith({ + path: await fs.realpath(secondFilePath), + }); + }); + }); + + it('logs and excludes local read roots when realpath fails with non-ENOENT', async () => { + await withTempRoot(async (tempRoot) => { + const localRoot = path.join(tempRoot, 'skills'); + const filePath = path.join(localRoot, 'instructions.md'); + await fs.mkdir(localRoot, { recursive: true }); + await fs.writeFile(filePath, 'instructions', 'utf8'); + + const actualFsPromises = + await vi.importActual( + 'node:fs/promises', + ); + vi.mocked(fsRealpath).mockImplementation(async (value) => { + if (value === path.resolve(localRoot)) { + const err = new Error('permission denied') as NodeJS.ErrnoException; + err.code = 'EACCES'; + throw err; + } + return actualFsPromises.realpath(value); + }); + + try { + const pathOutsideWorkspaceError = + createLocalReadFallbackError(filePath); + const client = { + readTextFile: vi.fn().mockRejectedValue(pathOutsideWorkspaceError), + } as unknown as AgentSideConnection; + const fallback = createFallback(); + + const svc = new AcpFileSystemService( + client, + 'session-2e-root-realpath-failure', + { readTextFile: true, writeTextFile: true }, + fallback, + { localReadRoots: [localRoot] }, + ); + + const err = await svc + .readTextFile({ path: filePath }) + .catch((e: unknown) => e); + + expect(err).toBeInstanceOf(Error); + expect(err).toMatchObject({ + cause: pathOutsideWorkspaceError, + message: `path escapes workspace: ${filePath}`, + }); + expect(fallback.readTextFile).not.toHaveBeenCalled(); + expect(mockDebugLogger.warn).toHaveBeenCalledWith( + 'realpath failed during ACP local read fallback check', + { + path: localRoot, + error: 'permission denied', + }, + ); + } finally { + vi.mocked(fsRealpath).mockRestore(); + } }); }); diff --git a/packages/cli/src/acp-integration/service/filesystem.ts b/packages/cli/src/acp-integration/service/filesystem.ts index 4b765df816e..95fb1d14b64 100644 --- a/packages/cli/src/acp-integration/service/filesystem.ts +++ b/packages/cli/src/acp-integration/service/filesystem.ts @@ -105,24 +105,7 @@ async function resolveRealPath(value: string): Promise { } } -async function resolveNearestExistingParent( - filePath: string, -): Promise { - let current = path.dirname(path.resolve(filePath)); - - while (true) { - const realCurrent = await resolveRealPath(current); - if (realCurrent) return realCurrent; - - const parent = path.dirname(current); - if (parent === current) return undefined; - current = parent; - } -} - export class AcpFileSystemService implements FileSystemService { - private resolvedLocalReadRoots?: Promise; - constructor( private readonly connection: AgentSideConnection, private readonly sessionId: string, @@ -158,7 +141,7 @@ export class AcpFileSystemService implements FileSystemService { ? await this.getLocalReadFallbackPath(params.path) : undefined; if (shouldTryLocalReadFallback && !fallbackPath) { - debugLogger.debug('Local read fallback skipped - no matching root', { + debugLogger.debug('Local read fallback skipped - no safe local path', { path: params.path, errorKind, }); @@ -232,11 +215,11 @@ export class AcpFileSystemService implements FileSystemService { return this.fallback.findFiles(fileName, searchPaths); } - private getResolvedLocalReadRoots(): Promise { - this.resolvedLocalReadRoots ??= Promise.all( + private async getResolvedLocalReadRoots(): Promise { + const roots = await Promise.all( (this.options.localReadRoots ?? []).map(resolveRealPath), - ).then((roots) => roots.filter((root): root is string => Boolean(root))); - return this.resolvedLocalReadRoots; + ); + return roots.filter((root): root is string => Boolean(root)); } private async getLocalReadFallbackPath( @@ -244,13 +227,11 @@ export class AcpFileSystemService implements FileSystemService { ): Promise { const normalizedFilePath = path.resolve(filePath); const realFilePath = await resolveRealPath(normalizedFilePath); - const comparisonPath = - realFilePath ?? (await resolveNearestExistingParent(normalizedFilePath)); - if (!comparisonPath) return undefined; + if (!realFilePath) return undefined; for (const realRoot of await this.getResolvedLocalReadRoots()) { - if (isSubpath(realRoot, comparisonPath)) { - return realFilePath ?? normalizedFilePath; + if (isSubpath(realRoot, realFilePath)) { + return realFilePath; } } return undefined; From a80b0d23a75d21b7086c41139e12e67a853b0f1f Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Tue, 30 Jun 2026 16:03:52 +0800 Subject: [PATCH 16/18] codex: address PR review feedback (#6021) Co-authored-by: Qwen-Coder --- .../service/filesystem.test.ts | 43 +++++++++++++++++++ packages/core/src/utils/errors.test.ts | 9 ++++ packages/core/src/utils/errors.ts | 21 +++++++-- 3 files changed, 70 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/acp-integration/service/filesystem.test.ts b/packages/cli/src/acp-integration/service/filesystem.test.ts index dc3334f7493..d920ba3af70 100644 --- a/packages/cli/src/acp-integration/service/filesystem.test.ts +++ b/packages/cli/src/acp-integration/service/filesystem.test.ts @@ -571,6 +571,49 @@ describe('AcpFileSystemService', () => { }); }); + it('re-throws ENOENT from local read fallback without wrapping it', async () => { + await withTempRoot(async (tempRoot) => { + const localRoot = path.join(tempRoot, 'skills'); + const filePath = path.join(localRoot, 'instructions.md'); + await fs.mkdir(localRoot, { recursive: true }); + await fs.writeFile(filePath, 'instructions', 'utf8'); + + const pathOutsideWorkspaceError = + createLocalReadFallbackError(filePath); + const client = { + readTextFile: vi.fn().mockRejectedValue(pathOutsideWorkspaceError), + } as unknown as AgentSideConnection; + const fallback = createFallback(); + const fallbackError = Object.assign( + new Error(`File not found: ${filePath}`), + { + code: 'ENOENT', + errno: -2, + path: filePath, + }, + ); + (fallback.readTextFile as ReturnType).mockRejectedValue( + fallbackError, + ); + + const svc = new AcpFileSystemService( + client, + 'session-2d-fallback-enoent', + { readTextFile: true, writeTextFile: true }, + fallback, + { localReadRoots: [localRoot] }, + ); + + await expect(svc.readTextFile({ path: filePath })).rejects.toBe( + fallbackError, + ); + expect(fallback.readTextFile).toHaveBeenCalledWith({ + path: await fs.realpath(filePath), + }); + expect(mockDebugLogger.warn).not.toHaveBeenCalled(); + }); + }); + it('does not fall back to local reads for missing files under allowed local roots', async () => { await withTempRoot(async (tempRoot) => { const localRoot = path.join(tempRoot, 'skills'); diff --git a/packages/core/src/utils/errors.test.ts b/packages/core/src/utils/errors.test.ts index b18bfb80550..5bd58f69e8d 100644 --- a/packages/core/src/utils/errors.test.ts +++ b/packages/core/src/utils/errors.test.ts @@ -62,6 +62,15 @@ describe('getErrorMessage cause unwrapping', () => { ).toBe('fetch failed (cause: ECONNREFUSED)'); }); + it('surfaces message and numeric code from plain object causes', () => { + expect( + getErrorMessage({ + message: 'fetch failed', + cause: { code: -32603, message: 'connection refused' }, + }), + ).toBe('fetch failed (cause: -32603: connection refused)'); + }); + it('bounds long messages from plain error-like objects', () => { const message = getErrorMessage({ message: 'x'.repeat(2000) }); diff --git a/packages/core/src/utils/errors.ts b/packages/core/src/utils/errors.ts index 1421e83bd1f..6635f168b8f 100644 --- a/packages/core/src/utils/errors.ts +++ b/packages/core/src/utils/errors.ts @@ -74,9 +74,24 @@ function describeSingleError(err: unknown): string | undefined { } return msg || codeStr || (err.name !== 'Error' ? err.name : undefined); } - if (err && typeof err === 'object' && 'code' in err) { - const code = (err as { code?: unknown }).code; - if (typeof code === 'string' && code) return code; + if (err && typeof err === 'object' && !Array.isArray(err)) { + const rec = err as Record; + const code = rec['code']; + const codeStr = + typeof code === 'string' && code + ? code + : typeof code === 'number' + ? String(code) + : undefined; + const message = rec['message']; + const msg = + typeof message === 'string' && message.trim() + ? message.trim() + : undefined; + if (msg && codeStr && !msg.includes(codeStr)) { + return `${codeStr}: ${msg}`; + } + return msg || codeStr; } const str = String(err); return str && str !== '[object Object]' ? str : undefined; From e2ffd48549fcc9cf91ed869f0add8c4e8f952ec1 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Tue, 30 Jun 2026 18:19:09 +0800 Subject: [PATCH 17/18] codex: address PR review feedback (#6021) Co-authored-by: Qwen-Coder --- .../service/filesystem.test.ts | 18 +++++++++++++++++ packages/core/src/utils/errors.test.ts | 20 +++++++++++++++++++ packages/core/src/utils/errors.ts | 7 +++++-- 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/acp-integration/service/filesystem.test.ts b/packages/cli/src/acp-integration/service/filesystem.test.ts index d920ba3af70..362c9e877a5 100644 --- a/packages/cli/src/acp-integration/service/filesystem.test.ts +++ b/packages/cli/src/acp-integration/service/filesystem.test.ts @@ -185,6 +185,24 @@ describe('AcpFileSystemService', () => { expect(String(err)).not.toContain('[object Object]'); }); + it('passes Error instances through without wrapping them', async () => { + const upstreamError = new Error('upstream failure'); + const client = { + readTextFile: vi.fn().mockRejectedValue(upstreamError), + } as unknown as AgentSideConnection; + + const svc = new AcpFileSystemService( + client, + 'session-2b-error', + { readTextFile: true, writeTextFile: true }, + createFallback(), + ); + + await expect(svc.readTextFile({ path: '/some/file.txt' })).rejects.toBe( + upstreamError, + ); + }); + it('does not copy unsafe properties from plain object ACP errors', async () => { const otherError: Record = { code: 'ABORT_ERR', diff --git a/packages/core/src/utils/errors.test.ts b/packages/core/src/utils/errors.test.ts index 5bd58f69e8d..dfc64c4a38f 100644 --- a/packages/core/src/utils/errors.test.ts +++ b/packages/core/src/utils/errors.test.ts @@ -38,6 +38,19 @@ describe('getErrorMessage cause unwrapping', () => { expect(getErrorMessage(err)).toBe('outer (cause: inner detail)'); }); + it('bounds Error messages that include long cause details', () => { + const expectedPrefix = 'outer (cause: '; + const err = new Error('outer', { + cause: { message: 'x'.repeat(2000) }, + }); + const message = getErrorMessage(err); + + expect(message).toBe( + `${expectedPrefix}${'x'.repeat(1000 - expectedPrefix.length - 3)}...`, + ); + expect(message.length).toBe(1000); + }); + it('does not append a redundant cause equal to the message', () => { const err = new Error('same', { cause: new Error('same') }); expect(getErrorMessage(err)).toBe('same'); @@ -88,6 +101,13 @@ describe('getErrorMessage cause unwrapping', () => { expect(message).toContain('"detail"'); }); + it('uses plain object code when JSON stringification fails', () => { + const circular: Record = { code: -32603 }; + circular['self'] = circular; + + expect(getErrorMessage(circular)).toBe('-32603'); + }); + it('uses String formatting for arrays', () => { expect(getErrorMessage([1, 2, 3])).toBe('1,2,3'); }); diff --git a/packages/core/src/utils/errors.ts b/packages/core/src/utils/errors.ts index 6635f168b8f..c5d2e99002c 100644 --- a/packages/core/src/utils/errors.ts +++ b/packages/core/src/utils/errors.ts @@ -108,7 +108,9 @@ export function getErrorMessage(error: unknown): string { if (error instanceof Error) { const detail = describeErrorCause(error.cause); if (detail && detail !== error.message) { - return `${error.message} (cause: ${detail})`; + return truncateStringifiedErrorMessage( + `${error.message} (cause: ${detail})`, + ); } return error.message; } @@ -131,7 +133,8 @@ export function getErrorMessage(error: unknown): string { ? truncateStringifiedErrorMessage(serialized) : String(error); } catch { - return String(error); + const detail = describeSingleError(error); + return detail ? truncateStringifiedErrorMessage(detail) : String(error); } } try { From 339cde1c833b67036559b3635f18c950b036579d Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Tue, 30 Jun 2026 18:28:28 +0800 Subject: [PATCH 18/18] codex: address PR review feedback (#6021) Co-authored-by: Qwen-Coder --- .../service/filesystem.test.ts | 13 +++++++++--- packages/core/src/utils/errors.test.ts | 21 +++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/acp-integration/service/filesystem.test.ts b/packages/cli/src/acp-integration/service/filesystem.test.ts index 362c9e877a5..d53c512a456 100644 --- a/packages/cli/src/acp-integration/service/filesystem.test.ts +++ b/packages/cli/src/acp-integration/service/filesystem.test.ts @@ -622,9 +622,16 @@ describe('AcpFileSystemService', () => { { localReadRoots: [localRoot] }, ); - await expect(svc.readTextFile({ path: filePath })).rejects.toBe( - fallbackError, - ); + const err = await svc + .readTextFile({ path: filePath }) + .catch((e: unknown) => e); + + expect(err).toBe(fallbackError); + expect(err).toMatchObject({ + code: 'ENOENT', + errno: -2, + path: filePath, + }); expect(fallback.readTextFile).toHaveBeenCalledWith({ path: await fs.realpath(filePath), }); diff --git a/packages/core/src/utils/errors.test.ts b/packages/core/src/utils/errors.test.ts index dfc64c4a38f..9ddf157b7e3 100644 --- a/packages/core/src/utils/errors.test.ts +++ b/packages/core/src/utils/errors.test.ts @@ -84,6 +84,15 @@ describe('getErrorMessage cause unwrapping', () => { ).toBe('fetch failed (cause: -32603: connection refused)'); }); + it('surfaces message-only plain object causes', () => { + expect( + getErrorMessage({ + message: 'fetch failed', + cause: { message: 'connection refused' }, + }), + ).toBe('fetch failed (cause: connection refused)'); + }); + it('bounds long messages from plain error-like objects', () => { const message = getErrorMessage({ message: 'x'.repeat(2000) }); @@ -108,9 +117,21 @@ describe('getErrorMessage cause unwrapping', () => { expect(getErrorMessage(circular)).toBe('-32603'); }); + it('uses String formatting when circular plain objects have no error details', () => { + const circular: Record = {}; + circular['self'] = circular; + + expect(getErrorMessage(circular)).toBe('[object Object]'); + }); + it('uses String formatting for arrays', () => { expect(getErrorMessage([1, 2, 3])).toBe('1,2,3'); }); + + it('uses String formatting for null and undefined', () => { + expect(getErrorMessage(null)).toBe('null'); + expect(getErrorMessage(undefined)).toBe('undefined'); + }); }); describe('isAbortError', () => {