Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions packages/core/src/tools/edit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,26 @@ describe('EditTool', () => {
expect(display.fileName).toBe(testFile);
});

it('should handle mixed CJK/Latin spacing mistakes in file paths', async () => {
const realDir = path.join(rootDir, 'image图片');
fs.mkdirSync(realDir, { recursive: true });
const realFilePath = path.join(realDir, 'target.txt');
fs.writeFileSync(realFilePath, 'old value', 'utf8');

const mangledFilePath = path.join(rootDir, 'image 图片', 'target.txt');
const params: EditToolParams = {
file_path: mangledFilePath,
old_string: 'old',
new_string: 'new',
};

const invocation = tool.build(params);
const result = await invocation.execute(new AbortController().signal);

expect(result.error).toBeUndefined();
expect(fs.readFileSync(realFilePath, 'utf8')).toBe('new value');
});

it('should create a new file if old_string is empty and file does not exist, and return created message', async () => {
const newFileName = 'brand_new_file.txt';
const newFilePath = path.join(rootDir, newFileName);
Expand Down
28 changes: 21 additions & 7 deletions packages/core/src/tools/edit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,11 @@ import type {
} from './tools.js';
import { BaseDeclarativeTool, Kind, ToolConfirmationOutcome } from './tools.js';
import { ToolErrorType } from './tool-error.js';
import { makeRelative, shortenPath } from '../utils/paths.js';
import {
makeRelative,
resolvePathWithMixedScriptSpacingFix,
shortenPath,
} from '../utils/paths.js';
import { isNodeError } from '../utils/errors.js';
import type { Config } from '../config/config.js';
import { ApprovalMode } from '../config/config.js';
Expand Down Expand Up @@ -548,8 +552,9 @@ Expectation for required parameters:
return `File path must be absolute: ${params.file_path}`;
}

const resolvedPath = resolvePathWithMixedScriptSpacingFix(params.file_path);
const workspaceContext = this.config.getWorkspaceContext();
if (!workspaceContext.isPathWithinWorkspace(params.file_path)) {
if (!workspaceContext.isPathWithinWorkspace(resolvedPath)) {
const directories = workspaceContext.getDirectories();
return `File path must be within one of the workspace directories: ${directories.join(', ')}`;
}
Expand All @@ -560,27 +565,36 @@ Expectation for required parameters:
protected createInvocation(
params: EditToolParams,
): ToolInvocation<EditToolParams, ToolResult> {
const resolvedPath = resolvePathWithMixedScriptSpacingFix(params.file_path);
if (resolvedPath !== params.file_path) {
params.file_path = resolvedPath;
}
return new EditToolInvocation(this.config, params);
}

getModifyContext(_: AbortSignal): ModifyContext<EditToolParams> {
return {
getFilePath: (params: EditToolParams) => params.file_path,
getFilePath: (params: EditToolParams) =>
resolvePathWithMixedScriptSpacingFix(params.file_path),
getCurrentContent: async (params: EditToolParams): Promise<string> => {
const resolvedPath = resolvePathWithMixedScriptSpacingFix(
params.file_path,
);
try {
return this.config
.getFileSystemService()
.readTextFile(params.file_path);
return this.config.getFileSystemService().readTextFile(resolvedPath);
} catch (err) {
if (!isNodeError(err) || err.code !== 'ENOENT') throw err;
return '';
}
},
getProposedContent: async (params: EditToolParams): Promise<string> => {
const resolvedPath = resolvePathWithMixedScriptSpacingFix(
params.file_path,
);
try {
const currentContent = await this.config
.getFileSystemService()
.readTextFile(params.file_path);
.readTextFile(resolvedPath);
return applyReplacement(
currentContent,
params.old_string,
Expand Down
13 changes: 13 additions & 0 deletions packages/core/src/tools/ls.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,19 @@ describe('LSTool', () => {
expect(result.returnDisplay).toBe('Listed 1 item(s).');
});

it('should handle mixed CJK/Latin spacing mistakes in directory paths', async () => {
const realDir = path.join(tempRootDir, 'image图片');
await fs.mkdir(realDir, { recursive: true });
await fs.writeFile(path.join(realDir, 'clipboard.txt'), 'x');

const mangledDir = path.join(tempRootDir, 'image 图片');
const invocation = lsTool.build({ path: mangledDir });
const result = await invocation.execute(abortSignal);

expect(result.llmContent).toContain('clipboard.txt');
expect(result.returnDisplay).toBe('Listed 1 item(s).');
});

it('should handle empty directories', async () => {
const emptyDir = path.join(tempRootDir, 'empty');
await fs.mkdir(emptyDir);
Expand Down
11 changes: 8 additions & 3 deletions packages/core/src/tools/ls.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import type { ToolInvocation, ToolResult } from './tools.js';
import { BaseDeclarativeTool, BaseToolInvocation, Kind } from './tools.js';
import { makeRelative, shortenPath } from '../utils/paths.js';
import { isSubpath } from '../utils/paths.js';
import { resolvePathWithMixedScriptSpacingFix } from '../utils/paths.js';
import type { Config } from '../config/config.js';
import { DEFAULT_FILE_FILTERING_OPTIONS } from '../config/constants.js';
import { ToolErrorType } from './tool-error.js';
Expand Down Expand Up @@ -311,16 +312,17 @@ export class LSTool extends BaseDeclarativeTool<LSToolParams, ToolResult> {
protected override validateToolParamValues(
params: LSToolParams,
): string | null {
const resolvedPath = resolvePathWithMixedScriptSpacingFix(params.path);
if (!path.isAbsolute(params.path)) {
return `Path must be absolute: ${params.path}`;
}

const userSkillsBase = this.config.storage.getUserSkillsDir();
const isUnderUserSkills = isSubpath(userSkillsBase, params.path);
const isUnderUserSkills = isSubpath(userSkillsBase, resolvedPath);

const workspaceContext = this.config.getWorkspaceContext();
if (
!workspaceContext.isPathWithinWorkspace(params.path) &&
!workspaceContext.isPathWithinWorkspace(resolvedPath) &&
!isUnderUserSkills
) {
const directories = workspaceContext.getDirectories();
Expand All @@ -334,6 +336,9 @@ export class LSTool extends BaseDeclarativeTool<LSToolParams, ToolResult> {
protected createInvocation(
params: LSToolParams,
): ToolInvocation<LSToolParams, ToolResult> {
return new LSToolInvocation(this.config, params);
return new LSToolInvocation(this.config, {
...params,
path: resolvePathWithMixedScriptSpacingFix(params.path),
});
}
}
22 changes: 22 additions & 0 deletions packages/core/src/tools/read-file.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,28 @@ describe('ReadFileTool', () => {
expect(result.returnDisplay).toBe('');
});

it('should handle mixed CJK/Latin spacing mistakes in path segments', async () => {
const realDir = path.join(tempRootDir, 'image图片');
await fsp.mkdir(realDir, { recursive: true });
const realFilePath = path.join(realDir, 'clipboard.txt');
await fsp.writeFile(realFilePath, 'content-from-cjk-path', 'utf-8');

const mangledPath = path.join(
tempRootDir,
'image 图片',
'clipboard.txt',
);
const params: ReadFileToolParams = { absolute_path: mangledPath };
const invocation = tool.build(params) as ToolInvocation<
ReadFileToolParams,
ToolResult
>;

const result = await invocation.execute(abortSignal);
expect(result.llmContent).toBe('content-from-cjk-path');
expect(result.returnDisplay).toBe('');
});

describe('with .qwenignore', () => {
beforeEach(async () => {
await fsp.writeFile(
Expand Down
17 changes: 12 additions & 5 deletions packages/core/src/tools/read-file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@ import { FileOperation } from '../telemetry/metrics.js';
import { getProgrammingLanguage } from '../telemetry/telemetry-utils.js';
import { logFileOperation } from '../telemetry/loggers.js';
import { FileOperationEvent } from '../telemetry/types.js';
import { isSubpath } from '../utils/paths.js';
import {
isSubpath,
resolvePathWithMixedScriptSpacingFix,
} from '../utils/paths.js';

/**
* Parameters for the ReadFile tool
Expand Down Expand Up @@ -174,6 +177,7 @@ export class ReadFileTool extends BaseDeclarativeTool<
params: ReadFileToolParams,
): string | null {
const filePath = params.absolute_path;
const resolvedPath = resolvePathWithMixedScriptSpacingFix(filePath);
if (params.absolute_path.trim() === '') {
return "The 'absolute_path' parameter must be non-empty.";
}
Expand All @@ -185,12 +189,12 @@ export class ReadFileTool extends BaseDeclarativeTool<
const workspaceContext = this.config.getWorkspaceContext();
const projectTempDir = this.config.storage.getProjectTempDir();
const userSkillsDir = this.config.storage.getUserSkillsDir();
const resolvedFilePath = path.resolve(filePath);
const resolvedFilePath = path.resolve(resolvedPath);
const isWithinTempDir = isSubpath(projectTempDir, resolvedFilePath);
const isWithinUserSkills = isSubpath(userSkillsDir, resolvedFilePath);

if (
!workspaceContext.isPathWithinWorkspace(filePath) &&
!workspaceContext.isPathWithinWorkspace(resolvedPath) &&
!isWithinTempDir &&
!isWithinUserSkills
) {
Expand All @@ -207,7 +211,7 @@ export class ReadFileTool extends BaseDeclarativeTool<
}

const fileService = this.config.getFileService();
if (fileService.shouldQwenIgnoreFile(params.absolute_path)) {
if (fileService.shouldQwenIgnoreFile(resolvedPath)) {
return `File path '${filePath}' is ignored by .qwenignore pattern(s).`;
}

Expand All @@ -217,6 +221,9 @@ export class ReadFileTool extends BaseDeclarativeTool<
protected createInvocation(
params: ReadFileToolParams,
): ToolInvocation<ReadFileToolParams, ToolResult> {
return new ReadFileToolInvocation(this.config, params);
return new ReadFileToolInvocation(this.config, {
...params,
absolute_path: resolvePathWithMixedScriptSpacingFix(params.absolute_path),
});
}
}
50 changes: 50 additions & 0 deletions packages/core/src/utils/paths.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
resolveAndValidatePath,
unescapePath,
isSubpath,
resolvePathWithMixedScriptSpacingFix,
shortenPath,
tildeifyPath,
} from './paths.js';
Expand Down Expand Up @@ -415,6 +416,55 @@ describe('resolvePath', () => {
});
});

describe('resolvePathWithMixedScriptSpacingFix', () => {
let tempRoot: string;

beforeAll(() => {
tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'mixed-script-path-'));
});

afterAll(() => {
fs.rmSync(tempRoot, { recursive: true, force: true });
});

it('returns corrected existing path when CJK/Latin spacing is accidental', () => {
const realDir = path.join(tempRoot, 'image图片');
const realFile = path.join(realDir, 'target.txt');
fs.mkdirSync(realDir, { recursive: true });
fs.writeFileSync(realFile, 'ok');

const mangled = path.join(tempRoot, 'image 图片', 'target.txt');
expect(resolvePathWithMixedScriptSpacingFix(mangled)).toBe(realFile);
});

it('returns corrected existing path for Japanese Kana/Latin spacing mistakes', () => {
const realDir = path.join(tempRoot, 'imageテスト');
const realFile = path.join(realDir, 'target.txt');
fs.mkdirSync(realDir, { recursive: true });
fs.writeFileSync(realFile, 'ok');

const mangled = path.join(tempRoot, 'image テスト', 'target.txt');
expect(resolvePathWithMixedScriptSpacingFix(mangled)).toBe(realFile);
});

it('returns corrected existing path for Hangul/Latin spacing mistakes', () => {
const realDir = path.join(tempRoot, 'image한글');
const realFile = path.join(realDir, 'target.txt');
fs.mkdirSync(realDir, { recursive: true });
fs.writeFileSync(realFile, 'ok');

const mangled = path.join(tempRoot, 'image 한글', 'target.txt');
expect(resolvePathWithMixedScriptSpacingFix(mangled)).toBe(realFile);
});

it('returns normalized original path when no corrected existing path is found', () => {
const mangled = path.join(tempRoot, 'does not exist', 'target.txt');
expect(resolvePathWithMixedScriptSpacingFix(mangled)).toBe(
path.normalize(mangled),
);
});
});

describe('validatePath', () => {
let workspaceRoot: string;
let config: Config;
Expand Down
75 changes: 75 additions & 0 deletions packages/core/src/utils/paths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,81 @@ export function resolvePath(
}
}

const CJK_SCRIPT_PATTERN =
'(?:\\p{Script=Han}|\\p{Script=Hiragana}|\\p{Script=Katakana}|\\p{Script=Hangul})';

const MIXED_SCRIPT_SPACE_PATTERNS = [
new RegExp(`(${CJK_SCRIPT_PATTERN})\\s+([A-Za-z0-9])`, 'gu'),
new RegExp(`([A-Za-z0-9])\\s+(${CJK_SCRIPT_PATTERN})`, 'gu'),
];

function collapseMixedScriptSpacing(segment: string): string {
let normalized = segment;
for (const pattern of MIXED_SCRIPT_SPACE_PATTERNS) {
normalized = normalized.replace(pattern, '$1$2');
}
return normalized;
}

/**
* Attempts to recover an existing path when an LLM accidentally inserts spaces
* between CJK and Latin characters within path segments.
*
* Example: `image 图片` -> `image图片`
*
* Returns the original normalized path when no existing corrected path can be
* found.
*/
export function resolvePathWithMixedScriptSpacingFix(filePath: string): string {
const normalizedPath = path.normalize(filePath);
if (fs.existsSync(normalizedPath)) {
return normalizedPath;
}

const parsed = path.parse(normalizedPath);
const rawSegments = normalizedPath
.slice(parsed.root.length)
.split(path.sep)
.filter((segment) => segment.length > 0);

if (rawSegments.length === 0) {
return normalizedPath;
}

let currentPath = parsed.root || '';
let usedCorrection = false;

for (const segment of rawSegments) {
const directPath = currentPath
? path.join(currentPath, segment)
: segment;
if (fs.existsSync(directPath)) {
currentPath = directPath;
continue;
}

const correctedSegment = collapseMixedScriptSpacing(segment);
if (correctedSegment !== segment) {
const correctedPath = currentPath
? path.join(currentPath, correctedSegment)
: correctedSegment;
if (fs.existsSync(correctedPath)) {
currentPath = correctedPath;
usedCorrection = true;
continue;
}
}

currentPath = directPath;
}

if (usedCorrection && fs.existsSync(currentPath)) {
return currentPath;
}

return normalizedPath;
}

export interface PathValidationOptions {
/**
* If true, allows both files and directories. If false (default), only allows directories.
Expand Down