From 9a0722625bcd3e929da56019259060ef06b060d9 Mon Sep 17 00:00:00 2001 From: gbbosak <51209748+gbbosak@users.noreply.github.com> Date: Fri, 22 Aug 2025 15:49:35 -0500 Subject: [PATCH] Fix crash when encountering an included directory which doesn't exist (#6497) Co-authored-by: Gal Zahavi <38544478+galz10@users.noreply.github.com> --- docs/cli/configuration.md | 2 +- packages/cli/src/config/config.ts | 0 packages/cli/src/config/settingsSchema.ts | 3 +- .../ui/hooks/slashCommandProcessor.test.ts | 1 + .../core/src/utils/workspaceContext.test.ts | 65 ++++++++++++++----- packages/core/src/utils/workspaceContext.ts | 18 +++-- 6 files changed, 65 insertions(+), 24 deletions(-) mode change 100644 => 100755 packages/cli/src/config/config.ts mode change 100644 => 100755 packages/core/src/utils/workspaceContext.ts diff --git a/docs/cli/configuration.md b/docs/cli/configuration.md index a8d0414240..78eeefa35e 100644 --- a/docs/cli/configuration.md +++ b/docs/cli/configuration.md @@ -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 diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts old mode 100644 new mode 100755 diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index 19c4c06d6c..dafc5a32cb 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -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: { diff --git a/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts b/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts index c35a4aef8a..83a60ffd46 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts @@ -27,6 +27,7 @@ vi.mock('node:process', () => { const mockProcess: Partial = { exit: mockProcessExit, platform: 'sunos', + cwd: () => '/fake/dir', } as unknown as NodeJS.Process; return { ...mockProcess, diff --git a/packages/core/src/utils/workspaceContext.test.ts b/packages/core/src/utils/workspaceContext.test.ts index 30c9ae1633..096661f2ab 100644 --- a/packages/core/src/utils/workspaceContext.test.ts +++ b/packages/core/src/utils/workspaceContext.test.ts @@ -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(); + }); +}); diff --git a/packages/core/src/utils/workspaceContext.ts b/packages/core/src/utils/workspaceContext.ts old mode 100644 new mode 100755 index 924cd60147..144f7337af --- a/packages/core/src/utils/workspaceContext.ts +++ b/packages/core/src/utils/workspaceContext.ts @@ -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(