Fix crash when encountering an included directory which doesn't exist (#6497)

Co-authored-by: Gal Zahavi <38544478+galz10@users.noreply.github.com>
This commit is contained in:
gbbosak
2025-08-22 15:49:35 -05:00
committed by GitHub
parent cfcf14fd06
commit 9a0722625b
6 changed files with 65 additions and 24 deletions

View File

@@ -281,7 +281,7 @@ If you are experiencing performance issues with file searching (e.g., with `@` c
```
- **`includeDirectories`** (array of strings):
- **Description:** Specifies an array of additional absolute or relative paths to include in the workspace context. This allows you to work with files across multiple directories as if they were one. Paths can use `~` to refer to the user's home directory. This setting can be combined with the `--include-directories` command-line flag.
- **Description:** Specifies an array of additional absolute or relative paths to include in the workspace context. Missing directories will be skipped with a warning by default. Paths can use `~` to refer to the user's home directory. This setting can be combined with the `--include-directories` command-line flag.
- **Default:** `[]`
- **Example:**
```json

0
packages/cli/src/config/config.ts Normal file → Executable file
View File

View File

@@ -468,7 +468,8 @@ export const SETTINGS_SCHEMA = {
category: 'General',
requiresRestart: false,
default: [] as string[],
description: 'Additional directories to include in the workspace context.',
description:
'Additional directories to include in the workspace context. Missing directories will be skipped with a warning.',
showInDialog: false,
},
loadMemoryFromIncludeDirectories: {

View File

@@ -27,6 +27,7 @@ vi.mock('node:process', () => {
const mockProcess: Partial<NodeJS.Process> = {
exit: mockProcessExit,
platform: 'sunos',
cwd: () => '/fake/dir',
} as unknown as NodeJS.Process;
return {
...mockProcess,

View File

@@ -48,13 +48,6 @@ describe('WorkspaceContext with real filesystem', () => {
expect(directories).toEqual([cwd, otherDir]);
});
it('should reject non-existent directories', () => {
const nonExistentDir = path.join(tempDir, 'does-not-exist');
expect(() => {
new WorkspaceContext(cwd, [nonExistentDir]);
}).toThrow('Directory does not exist');
});
it('should handle empty initialization', () => {
const workspaceContext = new WorkspaceContext(cwd, []);
const directories = workspaceContext.getDirectories();
@@ -81,15 +74,6 @@ describe('WorkspaceContext with real filesystem', () => {
expect(directories).toEqual([cwd, otherDir]);
});
it('should reject non-existent directories', () => {
const nonExistentDir = path.join(tempDir, 'does-not-exist');
const workspaceContext = new WorkspaceContext(cwd);
expect(() => {
workspaceContext.addDirectory(nonExistentDir);
}).toThrow('Directory does not exist');
});
it('should prevent duplicate directories', () => {
const workspaceContext = new WorkspaceContext(cwd);
workspaceContext.addDirectory(otherDir);
@@ -387,3 +371,52 @@ describe('WorkspaceContext with real filesystem', () => {
});
});
});
describe('WorkspaceContext with optional directories', () => {
let tempDir: string;
let cwd: string;
let existingDir1: string;
let existingDir2: string;
let nonExistentDir: string;
beforeEach(() => {
tempDir = fs.realpathSync(
fs.mkdtempSync(path.join(os.tmpdir(), 'workspace-context-optional-')),
);
cwd = path.join(tempDir, 'project');
existingDir1 = path.join(tempDir, 'existing-dir-1');
existingDir2 = path.join(tempDir, 'existing-dir-2');
nonExistentDir = path.join(tempDir, 'non-existent-dir');
fs.mkdirSync(cwd, { recursive: true });
fs.mkdirSync(existingDir1, { recursive: true });
fs.mkdirSync(existingDir2, { recursive: true });
vi.spyOn(console, 'warn').mockImplementation(() => {});
});
afterEach(() => {
fs.rmSync(tempDir, { recursive: true, force: true });
vi.restoreAllMocks();
});
it('should skip a missing optional directory and log a warning', () => {
const workspaceContext = new WorkspaceContext(cwd, [
nonExistentDir,
existingDir1,
]);
const directories = workspaceContext.getDirectories();
expect(directories).toEqual([cwd, existingDir1]);
expect(console.warn).toHaveBeenCalledTimes(1);
expect(console.warn).toHaveBeenCalledWith(
`[WARN] Skipping unreadable directory: ${nonExistentDir} (Directory does not exist: ${nonExistentDir})`,
);
});
it('should include an existing optional directory', () => {
const workspaceContext = new WorkspaceContext(cwd, [existingDir1]);
const directories = workspaceContext.getDirectories();
expect(directories).toEqual([cwd, existingDir1]);
expect(console.warn).not.toHaveBeenCalled();
});
});

18
packages/core/src/utils/workspaceContext.ts Normal file → Executable file
View File

@@ -7,6 +7,7 @@
import { isNodeError } from '../utils/errors.js';
import * as fs from 'fs';
import * as path from 'path';
import * as process from 'process';
export type Unsubscribe = () => void;
@@ -30,7 +31,6 @@ export class WorkspaceContext {
for (const additionalDirectory of additionalDirectories) {
this.addDirectory(additionalDirectory);
}
this.initialDirectories = new Set(this.directories);
}
@@ -64,12 +64,18 @@ export class WorkspaceContext {
* @param basePath Optional base path for resolving relative paths (defaults to cwd)
*/
addDirectory(directory: string, basePath: string = process.cwd()): void {
const resolved = this.resolveAndValidateDir(directory, basePath);
if (this.directories.has(resolved)) {
return;
try {
const resolved = this.resolveAndValidateDir(directory, basePath);
if (this.directories.has(resolved)) {
return;
}
this.directories.add(resolved);
this.notifyDirectoriesChanged();
} catch (err) {
console.warn(
`[WARN] Skipping unreadable directory: ${directory} (${err instanceof Error ? err.message : String(err)})`,
);
}
this.directories.add(resolved);
this.notifyDirectoriesChanged();
}
private resolveAndValidateDir(