fix: Address review comments for BOM encoding support

- Edit tool now respects defaultFileEncoding for new files
- Edit tool preserves BOM character for existing files without re-adding
- AcpFileSystemService detects BOM through ACP client with fallback
- Use line: null, limit: 1 for efficient BOM detection
- Add unit tests for AcpFileSystemService.detectFileBOM
- Add unit tests for EditTool BOM handling

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
This commit is contained in:
tanzhenxin 2026-02-01 11:23:02 +08:00
parent 831d74dbfe
commit 2d525d9fd0
4 changed files with 195 additions and 4 deletions

View file

@ -17,6 +17,93 @@ const createFallback = (): FileSystemService => ({
});
describe('AcpFileSystemService', () => {
describe('detectFileBOM', () => {
it('detects BOM through ACP client when content starts with U+FEFF', async () => {
const client = {
readTextFile: vi
.fn()
.mockResolvedValue({ content: '\ufeff// BOM file' }),
} as unknown as import('../acp.js').Client;
const svc = new AcpFileSystemService(
client,
'session-1',
{ readTextFile: true, writeTextFile: true },
createFallback(),
);
const result = await svc.detectFileBOM('/test/file.txt');
expect(result).toBe(true);
expect(client.readTextFile).toHaveBeenCalledWith({
path: '/test/file.txt',
sessionId: 'session-1',
line: null,
limit: 1,
});
});
it('detects no BOM through ACP client when content does not start with U+FEFF', async () => {
const client = {
readTextFile: vi.fn().mockResolvedValue({ content: '// No BOM file' }),
} as unknown as import('../acp.js').Client;
const svc = new AcpFileSystemService(
client,
'session-2',
{ readTextFile: true, writeTextFile: true },
createFallback(),
);
const result = await svc.detectFileBOM('/test/file.txt');
expect(result).toBe(false);
});
it('falls back to local filesystem when ACP client fails', async () => {
const client = {
readTextFile: vi.fn().mockRejectedValue(new Error('Network error')),
} as unknown as import('../acp.js').Client;
const fallback = createFallback();
(fallback.detectFileBOM as ReturnType<typeof vi.fn>).mockResolvedValue(
true,
);
const svc = new AcpFileSystemService(
client,
'session-3',
{ readTextFile: true, writeTextFile: true },
fallback,
);
const result = await svc.detectFileBOM('/test/file.txt');
expect(result).toBe(true);
expect(fallback.detectFileBOM).toHaveBeenCalledWith('/test/file.txt');
});
it('falls back to local filesystem when readTextFile capability is disabled', async () => {
const client = {
readTextFile: vi.fn(),
} as unknown as import('../acp.js').Client;
const fallback = createFallback();
(fallback.detectFileBOM as ReturnType<typeof vi.fn>).mockResolvedValue(
false,
);
const svc = new AcpFileSystemService(
client,
'session-4',
{ readTextFile: false, writeTextFile: true },
fallback,
);
const result = await svc.detectFileBOM('/test/file.txt');
expect(result).toBe(false);
expect(fallback.detectFileBOM).toHaveBeenCalledWith('/test/file.txt');
expect(client.readTextFile).not.toHaveBeenCalled();
});
});
describe('readTextFile ENOENT handling', () => {
it('converts RESOURCE_NOT_FOUND error to ENOENT', async () => {
const resourceNotFoundError = {

View file

@ -74,7 +74,22 @@ export class AcpFileSystemService implements FileSystemService {
}
async detectFileBOM(filePath: string): Promise<boolean> {
// Always use fallback for BOM detection
// Try to detect BOM through ACP client first by reading first line
if (this.capabilities.readTextFile) {
try {
const response = await this.client.readTextFile({
path: filePath,
sessionId: this.sessionId,
line: null,
limit: 1,
});
// Check if content starts with BOM character (U+FEFF)
return response.content.charCodeAt(0) === 0xfeff;
} catch {
// Fall through to fallback if ACP read fails
}
}
// Fall back to local filesystem detection
return this.fallback.detectFileBOM(filePath);
}