mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-07-09 17:19:02 +00:00
fix(cli): Handle ACP read_file for managed local paths (#6021)
* 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 <qwen-coder@alibabacloud.com> * 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 <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6021) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6021) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * 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 <qwen-coder@alibabacloud.com> * 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 <qwen-coder@alibabacloud.com> * fix(cli): address ACP read error review feedback Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): harden ACP error normalization Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6021) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6021) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6021) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6021) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6021) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: fix Windows CI path expectation (#6021) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6021) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6021) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6021) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#6021) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
This commit is contained in:
jinye
GitHub
parent
f37c64c7f2
commit
ea536d361f
18 changed files with 1515 additions and 38 deletions
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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: {
|
||||
|
|
@ -347,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();
|
||||
|
|
@ -597,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,
|
||||
|
|
@ -1215,6 +1219,78 @@ 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<typeof Session>,
|
||||
);
|
||||
|
||||
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',
|
||||
path.join('/project', 'subagents'),
|
||||
'/tmp/qwen-global-temp',
|
||||
'/project/.qwen/memory',
|
||||
'/tmp/user-memory',
|
||||
'/home/test/.qwen/skills',
|
||||
'/tmp/qwen-extensions',
|
||||
],
|
||||
},
|
||||
);
|
||||
expect(innerConfig.setFileSystemService).toHaveBeenCalled();
|
||||
|
||||
mockConnectionState.resolve();
|
||||
await agentPromise;
|
||||
});
|
||||
|
||||
it('does not return discontinued qwen-oauth as the only ACP auth option', async () => {
|
||||
vi.mocked(buildAuthMethods).mockReturnValue([
|
||||
{
|
||||
|
|
|
|||
|
|
@ -20,6 +20,7 @@ import {
|
|||
findProviderById,
|
||||
getAllGeminiMdFilenames,
|
||||
getAutoMemoryRoot,
|
||||
getUserAutoMemoryRoot,
|
||||
getDefaultBaseUrlForProtocol,
|
||||
getDefaultModelIds,
|
||||
getScopedEnvContents,
|
||||
|
|
@ -7651,6 +7652,19 @@ class QwenAgent implements Agent {
|
|||
config.getSessionId(),
|
||||
this.clientCapabilities.fs,
|
||||
config.getFileSystemService(),
|
||||
{
|
||||
// SYNC: Mirrors ReadFileTool's default allowed local roots, including
|
||||
// auto-memory roots, so ACP-local read fallback follows the same policy.
|
||||
localReadRoots: [
|
||||
config.storage.getProjectTempDir(),
|
||||
path.join(config.storage.getProjectDir(), 'subagents'),
|
||||
Storage.getGlobalTempDir(),
|
||||
getAutoMemoryRoot(config.getTargetDir()),
|
||||
getUserAutoMemoryRoot(),
|
||||
...config.storage.getUserSkillsDirs(),
|
||||
Storage.getUserExtensionsDir(),
|
||||
],
|
||||
},
|
||||
);
|
||||
config.setFileSystemService(acpFileSystemService);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -4,13 +4,66 @@
|
|||
* 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<typeof import('@qwen-code/qwen-code-core')>();
|
||||
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';
|
||||
|
||||
const RESOURCE_NOT_FOUND_CODE = -32002;
|
||||
const INTERNAL_ERROR_CODE = -32603;
|
||||
type LocalReadFallbackErrorKind = 'path_outside_workspace' | 'symlink_escape';
|
||||
|
||||
async function withTempRoot<T>(
|
||||
callback: (tempRoot: string) => Promise<T>,
|
||||
): Promise<T> {
|
||||
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({
|
||||
|
|
@ -22,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 = {
|
||||
|
|
@ -73,7 +132,7 @@ describe('AcpFileSystemService', () => {
|
|||
});
|
||||
});
|
||||
|
||||
it('re-throws other errors unchanged', async () => {
|
||||
it('preserves message for other read errors', async () => {
|
||||
const otherError = {
|
||||
code: INTERNAL_ERROR_CODE,
|
||||
message: 'Internal error',
|
||||
|
|
@ -92,8 +151,784 @@ describe('AcpFileSystemService', () => {
|
|||
await expect(
|
||||
svc.readTextFile({ path: '/some/file.txt' }),
|
||||
).rejects.toMatchObject({
|
||||
message: 'Internal error',
|
||||
});
|
||||
});
|
||||
|
||||
it('normalizes plain object ACP errors without exposing numeric codes as Node error codes', 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({
|
||||
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]');
|
||||
});
|
||||
|
||||
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<string, unknown> = {
|
||||
code: 'ABORT_ERR',
|
||||
message: 'Internal error',
|
||||
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;
|
||||
|
||||
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).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<string, unknown>)['remotePrototype'],
|
||||
).toBeUndefined();
|
||||
expect(String(err)).toContain('Internal error');
|
||||
});
|
||||
|
||||
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('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');
|
||||
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<typeof vi.fn>).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: 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<typeof vi.fn>).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');
|
||||
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] },
|
||||
);
|
||||
|
||||
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();
|
||||
});
|
||||
});
|
||||
|
||||
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] },
|
||||
);
|
||||
|
||||
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();
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
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<typeof vi.fn>).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: await fs.realpath(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<typeof vi.fn>).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: await fs.realpath(filePath),
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
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();
|
||||
const fallbackError = new Error('local read failed');
|
||||
(fallback.readTextFile as ReturnType<typeof vi.fn>).mockRejectedValue(
|
||||
fallbackError,
|
||||
);
|
||||
|
||||
const svc = new AcpFileSystemService(
|
||||
client,
|
||||
'session-2d-fallback-fail',
|
||||
{ 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({
|
||||
message: `Local fallback read failed for ${filePath}: local read failed (original ACP error: path escapes workspace: ${filePath})`,
|
||||
cause: {
|
||||
fallbackError,
|
||||
acpError: pathOutsideWorkspaceError,
|
||||
},
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
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<typeof vi.fn>).mockRejectedValue(
|
||||
fallbackError,
|
||||
);
|
||||
|
||||
const svc = new AcpFileSystemService(
|
||||
client,
|
||||
'session-2d-fallback-enoent',
|
||||
{ readTextFile: true, writeTextFile: true },
|
||||
fallback,
|
||||
{ localReadRoots: [localRoot] },
|
||||
);
|
||||
|
||||
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),
|
||||
});
|
||||
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');
|
||||
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 svc = new AcpFileSystemService(
|
||||
client,
|
||||
'session-2d-missing-local-file',
|
||||
{ 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();
|
||||
});
|
||||
});
|
||||
|
||||
it('does not fall back to local reads outside configured roots', 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 fallback = createFallback();
|
||||
|
||||
const svc = new AcpFileSystemService(
|
||||
client,
|
||||
'session-2e',
|
||||
{ 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(Object.prototype.hasOwnProperty.call(err, 'code')).toBe(false);
|
||||
expect(fallback.readTextFile).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
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 safe local path',
|
||||
{
|
||||
path: filePath,
|
||||
errorKind: 'path_outside_workspace',
|
||||
},
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
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');
|
||||
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<typeof vi.fn>)
|
||||
.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(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<typeof vi.fn>).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<typeof import('node:fs/promises')>(
|
||||
'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();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
it('ignores empty configured local read roots', async () => {
|
||||
await withTempRoot(async (tempRoot) => {
|
||||
const filePath = path.join(tempRoot, 'outside-workspace.md');
|
||||
await fs.writeFile(filePath, 'outside workspace', 'utf8');
|
||||
|
||||
const pathOutsideWorkspaceError =
|
||||
createLocalReadFallbackError(filePath);
|
||||
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: [''] },
|
||||
);
|
||||
|
||||
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();
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -236,5 +1071,66 @@ describe('AcpFileSystemService', () => {
|
|||
});
|
||||
expect(client.writeTextFile).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
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',
|
||||
};
|
||||
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({
|
||||
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]');
|
||||
});
|
||||
|
||||
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',
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -16,21 +16,64 @@ import type {
|
|||
FileSystemService,
|
||||
ReadTextFileResponse,
|
||||
} from '@qwen-code/qwen-code-core';
|
||||
import {
|
||||
createDebugLogger,
|
||||
getErrorMessage,
|
||||
isSubpath,
|
||||
} from '@qwen-code/qwen-code-core';
|
||||
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[];
|
||||
}
|
||||
|
||||
function isRecord(value: unknown): value is Record<string, unknown> {
|
||||
return typeof value === 'object' && value !== null && !Array.isArray(value);
|
||||
}
|
||||
|
||||
function getErrorCode(error: unknown): unknown {
|
||||
if (error instanceof RequestError) {
|
||||
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;
|
||||
}
|
||||
|
||||
function getErrorData(error: unknown): Record<string, unknown> | undefined {
|
||||
const data = isRecord(error) ? error['data'] : undefined;
|
||||
return isRecord(data) ? data : undefined;
|
||||
}
|
||||
|
||||
function getErrorKind(error: unknown): string | undefined {
|
||||
const data = getErrorData(error);
|
||||
if (data && typeof data['errorKind'] === 'string') {
|
||||
return data['errorKind'];
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
function normalizeError(error: unknown): Error {
|
||||
if (error instanceof Error) return error;
|
||||
|
||||
return new Error(getErrorMessage(error), {
|
||||
cause: error,
|
||||
});
|
||||
}
|
||||
|
||||
function createEnoentError(filePath: string): NodeJS.ErrnoException {
|
||||
const err = new Error(`File not found: ${filePath}`) as NodeJS.ErrnoException;
|
||||
err.code = 'ENOENT';
|
||||
|
|
@ -39,12 +82,36 @@ function createEnoentError(filePath: string): NodeJS.ErrnoException {
|
|||
return err;
|
||||
}
|
||||
|
||||
function isLocalReadFallbackErrorKind(errorKind: unknown): boolean {
|
||||
return (
|
||||
typeof errorKind === 'string' &&
|
||||
LOCAL_READ_FALLBACK_ERROR_KINDS.has(errorKind)
|
||||
);
|
||||
}
|
||||
|
||||
async function resolveRealPath(value: string): Promise<string | undefined> {
|
||||
if (!value.trim()) return undefined;
|
||||
|
||||
try {
|
||||
return await realpath(path.resolve(value));
|
||||
} catch (error) {
|
||||
if (getErrorCode(error) !== 'ENOENT') {
|
||||
debugLogger.warn('realpath failed during ACP local read fallback check', {
|
||||
path: value,
|
||||
error: getErrorMessage(error),
|
||||
});
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
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 +134,50 @@ export class AcpFileSystemService implements FileSystemService {
|
|||
throw createEnoentError(params.path);
|
||||
}
|
||||
|
||||
throw error;
|
||||
const errorKind = getErrorKind(error);
|
||||
const shouldTryLocalReadFallback =
|
||||
isLocalReadFallbackErrorKind(errorKind);
|
||||
const fallbackPath = shouldTryLocalReadFallback
|
||||
? await this.getLocalReadFallbackPath(params.path)
|
||||
: undefined;
|
||||
if (shouldTryLocalReadFallback && !fallbackPath) {
|
||||
debugLogger.debug('Local read fallback skipped - no safe local path', {
|
||||
path: params.path,
|
||||
errorKind,
|
||||
});
|
||||
}
|
||||
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,
|
||||
path: fallbackPath,
|
||||
});
|
||||
} catch (fallbackError) {
|
||||
if (getErrorCode(fallbackError) === 'ENOENT') {
|
||||
throw 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, acpError: error } },
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
throw normalizeError(error);
|
||||
}
|
||||
|
||||
return response;
|
||||
|
|
@ -85,11 +195,18 @@ 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) {
|
||||
if (getErrorCode(error) === RESOURCE_NOT_FOUND_CODE) {
|
||||
throw createEnoentError(params.path);
|
||||
}
|
||||
throw normalizeError(error);
|
||||
}
|
||||
|
||||
return { _meta: params._meta };
|
||||
}
|
||||
|
|
@ -97,4 +214,26 @@ export class AcpFileSystemService implements FileSystemService {
|
|||
findFiles(fileName: string, searchPaths: readonly string[]): string[] {
|
||||
return this.fallback.findFiles(fileName, searchPaths);
|
||||
}
|
||||
|
||||
private async getResolvedLocalReadRoots(): Promise<string[]> {
|
||||
const roots = await Promise.all(
|
||||
(this.options.localReadRoots ?? []).map(resolveRealPath),
|
||||
);
|
||||
return roots.filter((root): root is string => Boolean(root));
|
||||
}
|
||||
|
||||
private async getLocalReadFallbackPath(
|
||||
filePath: string,
|
||||
): Promise<string | undefined> {
|
||||
const normalizedFilePath = path.resolve(filePath);
|
||||
const realFilePath = await resolveRealPath(normalizedFilePath);
|
||||
if (!realFilePath) return undefined;
|
||||
|
||||
for (const realRoot of await this.getResolvedLocalReadRoots()) {
|
||||
if (isSubpath(realRoot, realFilePath)) {
|
||||
return realFilePath;
|
||||
}
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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',
|
||||
|
|
@ -1100,6 +1146,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', () => {
|
||||
|
|
|
|||
|
|
@ -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<EditToolParams, ToolResult> {
|
|||
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<EditToolParams, ToolResult> {
|
|||
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<EditToolParams, ToolResult> {
|
|||
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}`,
|
||||
|
|
|
|||
|
|
@ -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,37 @@ 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 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'),
|
||||
};
|
||||
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 +680,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-'),
|
||||
);
|
||||
|
|
|
|||
|
|
@ -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,20 +98,21 @@ 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<PermissionDecision> {
|
||||
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 <projectDir>/subagents/ and
|
||||
// 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(),
|
||||
];
|
||||
|
|
|
|||
|
|
@ -846,6 +846,51 @@ describe('WriteFileTool', () => {
|
|||
'Error writing to file: Generic write error',
|
||||
);
|
||||
});
|
||||
|
||||
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';
|
||||
|
||||
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)', () => {
|
||||
|
|
|
|||
|
|
@ -612,10 +612,8 @@ 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: ${String(error)}`;
|
||||
errorMsg = `Error writing to file: ${getErrorMessage(error)}`;
|
||||
}
|
||||
|
||||
return {
|
||||
|
|
@ -689,9 +687,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(
|
||||
|
|
|
|||
|
|
@ -38,10 +38,100 @@ 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');
|
||||
});
|
||||
|
||||
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('surfaces cause details from plain error-like objects', () => {
|
||||
expect(
|
||||
getErrorMessage({
|
||||
message: 'fetch failed',
|
||||
cause: { code: 'ECONNREFUSED' },
|
||||
}),
|
||||
).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('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) });
|
||||
|
||||
expect(message).toBe(`${'x'.repeat(997)}...`);
|
||||
});
|
||||
|
||||
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"');
|
||||
});
|
||||
|
||||
it('uses plain object code when JSON stringification fails', () => {
|
||||
const circular: Record<string, unknown> = { code: -32603 };
|
||||
circular['self'] = circular;
|
||||
|
||||
expect(getErrorMessage(circular)).toBe('-32603');
|
||||
});
|
||||
|
||||
it('uses String formatting when circular plain objects have no error details', () => {
|
||||
const circular: Record<string, unknown> = {};
|
||||
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', () => {
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -72,22 +74,69 @@ 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<string, unknown>;
|
||||
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;
|
||||
}
|
||||
|
||||
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);
|
||||
if (detail && detail !== error.message) {
|
||||
return `${error.message} (cause: ${detail})`;
|
||||
return truncateStringifiedErrorMessage(
|
||||
`${error.message} (cause: ${detail})`,
|
||||
);
|
||||
}
|
||||
return error.message;
|
||||
}
|
||||
if (error !== null && typeof error === 'object' && !Array.isArray(error)) {
|
||||
const { message, cause } = error as {
|
||||
message?: unknown;
|
||||
cause?: unknown;
|
||||
};
|
||||
if (typeof message === 'string' && message.trim()) {
|
||||
const detail = describeErrorCause(cause);
|
||||
const result =
|
||||
detail && detail !== message
|
||||
? `${message} (cause: ${detail})`
|
||||
: message;
|
||||
return truncateStringifiedErrorMessage(result);
|
||||
}
|
||||
try {
|
||||
const serialized = JSON.stringify(error);
|
||||
return serialized
|
||||
? truncateStringifiedErrorMessage(serialized)
|
||||
: String(error);
|
||||
} catch {
|
||||
const detail = describeSingleError(error);
|
||||
return detail ? truncateStringifiedErrorMessage(detail) : String(error);
|
||||
}
|
||||
}
|
||||
try {
|
||||
return String(error);
|
||||
} catch {
|
||||
|
|
|
|||
|
|
@ -1055,6 +1055,50 @@ 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 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');
|
||||
|
|
|
|||
|
|
@ -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';
|
||||
|
|
@ -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}`,
|
||||
|
|
@ -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, '/');
|
||||
|
|
|
|||
|
|
@ -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',
|
||||
|
|
|
|||
25
scripts/tests/package-scripts.test.js
Normal file
25
scripts/tests/package-scripts.test.js
Normal file
|
|
@ -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',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
@ -238,7 +238,7 @@ describe('serve fast-path bundle check', () => {
|
|||
/Could not find bundled outputs for serve pre-listen roots/,
|
||||
);
|
||||
expect(() => findServeFastPathBundleOffenders(metafile)).toThrow(
|
||||
/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/,
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -313,7 +313,7 @@ describe('serve fast-path bundle check', () => {
|
|||
metafilePath: join(tempDir, 'dist', 'esbuild.json'),
|
||||
}),
|
||||
).toThrow(
|
||||
/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/,
|
||||
);
|
||||
} finally {
|
||||
rmSync(tempDir, { recursive: true, force: true });
|
||||
|
|
@ -331,7 +331,7 @@ describe('serve fast-path bundle check', () => {
|
|||
/Invalid esbuild metafile at .*dist[/\\]esbuild\.json/,
|
||||
);
|
||||
expect(() => checkServeFastPathBundle({ metafilePath })).toThrow(
|
||||
/Run `npm run build -- --cli-only && npx cross-env DEV=true npm run bundle` to regenerate it/,
|
||||
/Run `npm run build -- --cli-only && cross-env DEV=true npm run bundle` to regenerate it/,
|
||||
);
|
||||
} finally {
|
||||
rmSync(tempDir, { recursive: true, force: true });
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue