fix(core,cli): enable recursive directory access for (#17094)

This commit is contained in:
Gal Zahavi
2026-01-21 09:58:23 -08:00
committed by GitHub
parent acbef4cd31
commit 45d554ae2f
17 changed files with 410 additions and 135 deletions
+5
View File
@@ -801,6 +801,11 @@ export class Config {
}
this.initialized = true;
// Add pending directories to workspace context
for (const dir of this.pendingIncludeDirectories) {
this.workspaceContext.addDirectory(dir);
}
// Initialize centralized FileDiscoveryService
const discoverToolsHandle = startupProfiler.start('discover_tools');
this.getFileService();
+23 -8
View File
@@ -9,6 +9,9 @@ import { HookSystem } from './hookSystem.js';
import { Config } from '../config/config.js';
import { HookType } from './types.js';
import { spawn } from 'node:child_process';
import * as fs from 'node:fs';
import * as os from 'node:os';
import * as path from 'node:path';
import type { ChildProcessWithoutNullStreams } from 'node:child_process';
import type { Readable, Writable } from 'node:stream';
@@ -58,13 +61,16 @@ describe('HookSystem Integration', () => {
beforeEach(() => {
vi.resetAllMocks();
const testDir = path.join(os.tmpdir(), 'test-hooks');
fs.mkdirSync(testDir, { recursive: true });
// Create a real config with simple command hook configurations for testing
config = new Config({
model: 'gemini-1.5-flash',
targetDir: '/tmp/test-hooks',
targetDir: testDir,
sessionId: 'test-session',
debugMode: false,
cwd: '/tmp/test-hooks',
cwd: testDir,
hooks: {
BeforeTool: [
{
@@ -141,13 +147,16 @@ describe('HookSystem Integration', () => {
});
it('should handle initialization errors gracefully', async () => {
const invalidDir = path.join(os.tmpdir(), 'test-hooks-invalid');
fs.mkdirSync(invalidDir, { recursive: true });
// Create a config with invalid hooks to trigger initialization errors
const invalidConfig = new Config({
model: 'gemini-1.5-flash',
targetDir: '/tmp/test-hooks-invalid',
targetDir: invalidDir,
sessionId: 'test-session-invalid',
debugMode: false,
cwd: '/tmp/test-hooks-invalid',
cwd: invalidDir,
hooks: {
BeforeTool: [
{
@@ -254,13 +263,16 @@ describe('HookSystem Integration', () => {
describe('hook disabling via settings', () => {
it('should not execute disabled hooks from settings', async () => {
const disabledDir = path.join(os.tmpdir(), 'test-hooks-disabled');
fs.mkdirSync(disabledDir, { recursive: true });
// Create config with two hooks, one enabled and one disabled via settings
const configWithDisabled = new Config({
model: 'gemini-1.5-flash',
targetDir: '/tmp/test-hooks-disabled',
targetDir: disabledDir,
sessionId: 'test-session-disabled',
debugMode: false,
cwd: '/tmp/test-hooks-disabled',
cwd: disabledDir,
hooks: {
BeforeTool: [
{
@@ -322,13 +334,16 @@ describe('HookSystem Integration', () => {
describe('hook disabling via command', () => {
it('should disable hook when setHookEnabled is called', async () => {
const setEnabledDir = path.join(os.tmpdir(), 'test-hooks-setEnabled');
fs.mkdirSync(setEnabledDir, { recursive: true });
// Create config with a hook
const configForDisabling = new Config({
model: 'gemini-1.5-flash',
targetDir: '/tmp/test-hooks-setEnabled',
targetDir: setEnabledDir,
sessionId: 'test-session-setEnabled',
debugMode: false,
cwd: '/tmp/test-hooks-setEnabled',
cwd: setEnabledDir,
hooks: {
BeforeTool: [
{
+2
View File
@@ -480,6 +480,7 @@ describe('editor utils', () => {
});
it(`should allow ${editor} when not in sandbox mode`, () => {
vi.stubEnv('SANDBOX', '');
expect(allowEditorTypeInSandbox(editor)).toBe(true);
});
}
@@ -500,6 +501,7 @@ describe('editor utils', () => {
it('should return true for vscode when installed and not in sandbox mode', () => {
(execSync as Mock).mockReturnValue(Buffer.from('/usr/bin/code'));
vi.stubEnv('SANDBOX', '');
expect(isEditorAvailable('vscode')).toBe(true);
});
@@ -7,7 +7,6 @@
import * as fs from 'node:fs/promises';
import * as path from 'node:path';
import { isSubpath } from './paths.js';
import { marked, type Token } from 'marked';
import { debugLogger } from './debugLogger.js';
// Simple console logger for import processing
@@ -83,7 +82,7 @@ function hasMessage(err: unknown): err is { message: string } {
);
}
// Helper to find all code block and inline code regions using marked
// Helper to find all code block and inline code regions using regex
/**
* Finds all import statements in content without using regex
* @returns Array of {start, _end, path} objects for each import found
@@ -154,35 +153,13 @@ function isLetter(char: string): boolean {
function findCodeRegions(content: string): Array<[number, number]> {
const regions: Array<[number, number]> = [];
const tokens = marked.lexer(content);
let offset = 0;
function walk(token: Token, baseOffset: number) {
if (token.type === 'code' || token.type === 'codespan') {
regions.push([baseOffset, baseOffset + token.raw.length]);
}
if ('tokens' in token && token.tokens) {
let childOffset = 0;
for (const child of token.tokens) {
const childIndexInParent = token.raw.indexOf(child.raw, childOffset);
if (childIndexInParent === -1) {
logger.error(
`Could not find child token in parent raw content. Aborting parsing for this branch. Child raw: "${child.raw}"`,
);
break;
}
walk(child, baseOffset + childIndexInParent);
childOffset = childIndexInParent + child.raw.length;
}
}
// Regex to match code blocks (inline and multiline)
// Matches one or more backticks, content (lazy), and same number of backticks
const regex = /(`+)([\s\S]*?)\1/g;
let match;
while ((match = regex.exec(content)) !== null) {
regions.push([match.index, match.index + match[0].length]);
}
for (const token of tokens) {
walk(token, offset);
offset += token.raw.length;
}
return regions;
}
@@ -374,6 +374,75 @@ describe('WorkspaceContext with real filesystem', () => {
expect(dirs1).toEqual(dirs2);
});
});
describe('addDirectories', () => {
it('should add multiple directories and emit one event', () => {
const dir3 = path.join(tempDir, 'dir3');
fs.mkdirSync(dir3);
const workspaceContext = new WorkspaceContext(cwd);
const listener = vi.fn();
workspaceContext.onDirectoriesChanged(listener);
const result = workspaceContext.addDirectories([otherDir, dir3]);
expect(workspaceContext.getDirectories()).toContain(otherDir);
expect(workspaceContext.getDirectories()).toContain(dir3);
expect(listener).toHaveBeenCalledOnce();
expect(result.added).toHaveLength(2);
expect(result.failed).toHaveLength(0);
});
it('should handle partial failures', () => {
const workspaceContext = new WorkspaceContext(cwd);
const listener = vi.fn();
workspaceContext.onDirectoriesChanged(listener);
const loggerSpy = vi
.spyOn(debugLogger, 'warn')
.mockImplementation(() => {});
const nonExistent = path.join(tempDir, 'does-not-exist');
const result = workspaceContext.addDirectories([otherDir, nonExistent]);
expect(workspaceContext.getDirectories()).toContain(otherDir);
expect(workspaceContext.getDirectories()).not.toContain(nonExistent);
expect(listener).toHaveBeenCalledOnce();
expect(loggerSpy).toHaveBeenCalled();
expect(result.added).toEqual([otherDir]);
expect(result.failed).toHaveLength(1);
expect(result.failed[0].path).toBe(nonExistent);
expect(result.failed[0].error).toBeDefined();
loggerSpy.mockRestore();
});
it('should not emit event if no directories added', () => {
const workspaceContext = new WorkspaceContext(cwd);
const listener = vi.fn();
workspaceContext.onDirectoriesChanged(listener);
const loggerSpy = vi
.spyOn(debugLogger, 'warn')
.mockImplementation(() => {});
const nonExistent = path.join(tempDir, 'does-not-exist');
const result = workspaceContext.addDirectories([nonExistent]);
expect(listener).not.toHaveBeenCalled();
expect(result.added).toHaveLength(0);
expect(result.failed).toHaveLength(1);
loggerSpy.mockRestore();
});
});
describe('addDirectory', () => {
it('should throw error if directory fails to add', () => {
const workspaceContext = new WorkspaceContext(cwd);
const nonExistent = path.join(tempDir, 'does-not-exist');
expect(() => workspaceContext.addDirectory(nonExistent)).toThrow();
});
});
});
describe('WorkspaceContext with optional directories', () => {
+44 -14
View File
@@ -11,6 +11,11 @@ import { debugLogger } from './debugLogger.js';
export type Unsubscribe = () => void;
export interface AddDirectoriesResult {
added: string[];
failed: Array<{ path: string; error: Error }>;
}
/**
* WorkspaceContext manages multiple workspace directories and validates paths
* against them. This allows the CLI to operate on files from multiple directories
@@ -31,9 +36,7 @@ export class WorkspaceContext {
additionalDirectories: string[] = [],
) {
this.addDirectory(targetDir);
for (const additionalDirectory of additionalDirectories) {
this.addDirectory(additionalDirectory);
}
this.addDirectories(additionalDirectories);
this.initialDirectories = new Set(this.directories);
}
@@ -67,22 +70,49 @@ export class WorkspaceContext {
* Adds a directory to the workspace.
* @param directory The directory path to add (can be relative or absolute)
* @param basePath Optional base path for resolving relative paths (defaults to cwd)
* @throws Error if the directory cannot be added
*/
addDirectory(directory: string): void {
try {
const resolved = this.resolveAndValidateDir(directory);
if (this.directories.has(resolved)) {
return;
}
this.directories.add(resolved);
this.notifyDirectoriesChanged();
} catch (err) {
debugLogger.warn(
`[WARN] Skipping unreadable directory: ${directory} (${err instanceof Error ? err.message : String(err)})`,
);
const result = this.addDirectories([directory]);
if (result.failed.length > 0) {
throw result.failed[0].error;
}
}
/**
* Adds multiple directories to the workspace.
* Emits a single change event if any directories are added.
* @param directories The directory paths to add
* @returns Object containing successfully added directories and failures
*/
addDirectories(directories: string[]): AddDirectoriesResult {
const result: AddDirectoriesResult = { added: [], failed: [] };
let changed = false;
for (const directory of directories) {
try {
const resolved = this.resolveAndValidateDir(directory);
if (!this.directories.has(resolved)) {
this.directories.add(resolved);
changed = true;
}
result.added.push(directory);
} catch (err) {
const error = err instanceof Error ? err : new Error(String(err));
debugLogger.warn(
`[WARN] Skipping unreadable directory: ${directory} (${error.message})`,
);
result.failed.push({ path: directory, error });
}
}
if (changed) {
this.notifyDirectoriesChanged();
}
return result;
}
private resolveAndValidateDir(directory: string): string {
const absolutePath = path.resolve(this.targetDir, directory);