From a77704a136bb9bd866f557995796d3831c053346 Mon Sep 17 00:00:00 2001 From: dylanyunlon Date: Fri, 18 Sep 2026 13:14:25 +0000 Subject: [PATCH 01/23] feat(agent): add AST-aware structural search tool for precise symbol navigation ## Summary This PR implements the AST-aware tools requested in #22745, enabling the agent to perform precise symbol-level navigation instead of guessing line ranges or reading entire files. By introducing a lightweight regex-based AST analysis service and a new `ast_search` tool, we provide three capabilities: 1. **Symbol boundary detection:** Find the exact start/end lines of any class, function, method, interface, type, or enum by name. 2. **File structural outlines:** Get a compressed skeleton of a file's declarations and their signatures in a single tool call. 3. **Codebase mapping:** Generate a bird's-eye structural overview of the entire repository, showing all top-level symbols across files. ## Details 1. **AST Analysis Service** (`packages/core/src/services/astAnalysisService.ts`): Zero-dependency regex heuristic parser supporting TypeScript, JavaScript, Python, Go, Rust, and Java. Extracts symbols with brace-counting (or indentation-tracking for Python) to determine precise method bounds. Designed as a fallback for when ast-grep (sg) is not installed. 2. **ast_search Tool** (`packages/core/src/tools/ast-search.ts`): New core tool registered as `ast_search` with three scopes: - `symbol`: Returns line bounds + signature for a named symbol - `outline`: Returns full structural skeleton of a file - `map`: Returns compressed codebase overview 3. **Codebase Investigator Enhancement** (`codebase-investigator.ts`): The `codebase_investigator` subagent now has `ast_search` in its tool list and its system prompt directs it to use AST outlines before reading files in full. 4. **Read-file Integration** (`read-file.ts`): When `read_file` truncates output, the truncation message now suggests using `ast_search` to jump directly to the needed symbol. 5. **Prompt Enhancement** (`prompts/snippets.ts`): The research phase prompt now mentions `ast_search` with scope "map" and "outline" for initial codebase discovery. 6. **Tool Declaration Pipeline**: - `base-declarations.ts`: New constants for tool name and parameters - `types.ts`: `CoreToolSet` extended with `ast_search` member - `default-legacy.ts` / `gemini-3.ts`: Full declaration for both model families - `coreTools.ts`: `AST_SEARCH_DEFINITION` export - `tool-names.ts`: Registered in `ALL_BUILTIN_TOOL_NAMES` and `PLAN_MODE_TOOLS` 7. **Automated Verification:** - 19 unit tests for the AST analysis service (symbol extraction, brace counting, indentation tracking, file outline, codebase map) - 11 unit tests for the ast_search tool (symbol/outline/map scopes, error handling, missing params) - Updated codebase-investigator tests (tool list + prompt assertions) - Updated read-file test (truncation message assertion) ## Related Issues Resolves #22745 Partially addresses #22746 Partially addresses #22747 ## How to Validate 1. Run the AST service test suite: ```bash npm test -w @google/gemini-cli-core -- src/services/astAnalysisService.test.ts --run ``` 2. Run the ast_search tool test suite: ```bash npm test -w @google/gemini-cli-core -- src/tools/ast-search.test.ts --run ``` 3. Run the updated codebase investigator tests: ```bash npm test -w @google/gemini-cli-core -- src/agents/codebase-investigator.test.ts --run ``` 4. Run project-wide build: ```bash npm run build ``` --- .../src/agents/codebase-investigator.test.ts | 13 + .../core/src/agents/codebase-investigator.ts | 3 + packages/core/src/prompts/snippets.ts | 2 +- .../src/services/astAnalysisService.test.ts | 242 +++++++++++ .../core/src/services/astAnalysisService.ts | 397 ++++++++++++++++++ packages/core/src/tools/ast-search.test.ts | 230 ++++++++++ packages/core/src/tools/ast-search.ts | 241 +++++++++++ .../tools/definitions/base-declarations.ts | 6 + .../core/src/tools/definitions/coreTools.ts | 11 + .../model-family-sets/default-legacy.ts | 39 ++ .../definitions/model-family-sets/gemini-3.ts | 29 ++ packages/core/src/tools/definitions/types.ts | 1 + packages/core/src/tools/read-file.test.ts | 1 + packages/core/src/tools/read-file.ts | 1 + packages/core/src/tools/tool-names.ts | 13 + 15 files changed, 1228 insertions(+), 1 deletion(-) create mode 100644 packages/core/src/services/astAnalysisService.test.ts create mode 100644 packages/core/src/services/astAnalysisService.ts create mode 100644 packages/core/src/tools/ast-search.test.ts create mode 100644 packages/core/src/tools/ast-search.ts diff --git a/packages/core/src/agents/codebase-investigator.test.ts b/packages/core/src/agents/codebase-investigator.test.ts index 3637daa9e36..110a0363e6e 100644 --- a/packages/core/src/agents/codebase-investigator.test.ts +++ b/packages/core/src/agents/codebase-investigator.test.ts @@ -11,6 +11,7 @@ import { GREP_TOOL_NAME, LS_TOOL_NAME, READ_FILE_TOOL_NAME, + AST_SEARCH_TOOL_NAME, } from '../tools/tool-names.js'; import { DEFAULT_GEMINI_MODEL } from '../config/models.js'; import { makeFakeConfig } from '../test-utils/config.js'; @@ -50,6 +51,7 @@ describe('CodebaseInvestigatorAgent', () => { READ_FILE_TOOL_NAME, GLOB_TOOL_NAME, GREP_TOOL_NAME, + AST_SEARCH_TOOL_NAME, ]); }); @@ -77,4 +79,15 @@ describe('CodebaseInvestigatorAgent', () => { const agent = CodebaseInvestigatorAgent(config); expect(agent.promptConfig.systemPrompt).toContain('`ls -R`'); }); + + it('should mention ast_search tool in system prompt', () => { + const agent = CodebaseInvestigatorAgent(config); + expect(agent.promptConfig.systemPrompt).toContain('ast_search'); + expect(agent.promptConfig.systemPrompt).toContain('scope'); + }); + + it('should include ast_search in tool config', () => { + const agent = CodebaseInvestigatorAgent(config); + expect(agent.toolConfig?.tools).toContain(AST_SEARCH_TOOL_NAME); + }); }); diff --git a/packages/core/src/agents/codebase-investigator.ts b/packages/core/src/agents/codebase-investigator.ts index 5036bd28239..fea6ad71805 100644 --- a/packages/core/src/agents/codebase-investigator.ts +++ b/packages/core/src/agents/codebase-investigator.ts @@ -10,6 +10,7 @@ import { GREP_TOOL_NAME, LS_TOOL_NAME, READ_FILE_TOOL_NAME, + AST_SEARCH_TOOL_NAME, } from '../tools/tool-names.js'; import { DEFAULT_THINKING_MODE, @@ -121,6 +122,7 @@ export const CodebaseInvestigatorAgent = ( READ_FILE_TOOL_NAME, GLOB_TOOL_NAME, GREP_TOOL_NAME, + AST_SEARCH_TOOL_NAME, ], }, @@ -132,6 +134,7 @@ export const CodebaseInvestigatorAgent = ( systemPrompt: `You are **Codebase Investigator**, a hyper-specialized AI agent and an expert in reverse-engineering complex software projects. You are a sub-agent within a larger development system. Your **SOLE PURPOSE** is to build a complete mental model of the code relevant to a given investigation. You must identify all relevant files, understand their roles, and foresee the direct architectural consequences of potential changes. You are a sub-agent in a larger system. Your only responsibility is to provide deep, actionable context. +- **DO:** Use the \`ast_search\` tool to quickly locate symbol boundaries and get file outlines before reading entire files. For broad exploration, use \`ast_search\` with scope "map" to get a compressed structural overview of the codebase. - **DO:** Find the key modules, classes, and functions that are part of the problem and its solution. - **DO:** Understand *why* the code is written the way it is. Question everything. - **DO:** Foresee the ripple effects of a change. If \`function A\` is modified, you must check its callers. If a data structure is altered, you must identify where its type definitions need to be updated. diff --git a/packages/core/src/prompts/snippets.ts b/packages/core/src/prompts/snippets.ts index d62613a614d..950b9fe8164 100644 --- a/packages/core/src/prompts/snippets.ts +++ b/packages/core/src/prompts/snippets.ts @@ -732,7 +732,7 @@ function workflowStepResearch(options: PrimaryWorkflowsOptions): string { subAgentSearch = ` For **simple, targeted searches** (like finding a specific function name, file path, or variable declaration), use ${toolsStr} directly in parallel.`; } - return `1. **Research:** Systematically map the codebase and validate assumptions. Utilize specialized sub-agents (e.g., \`codebase_investigator\`) as the primary mechanism for initial discovery when the task involves **complex refactoring, codebase exploration or system-wide analysis**.${subAgentSearch} Use ${formatToolName(READ_FILE_TOOL_NAME)} to validate all assumptions. **Prioritize empirical reproduction of reported issues to confirm the failure state.**${suggestion}`; + return `1. **Research:** Systematically map the codebase and validate assumptions. Utilize specialized sub-agents (e.g., \`codebase_investigator\`) as the primary mechanism for initial discovery when the task involves **complex refactoring, codebase exploration or system-wide analysis**.${subAgentSearch} Use \`ast_search\` with scope "map" or "outline" to quickly understand codebase structure before reading files in full. Use ${formatToolName(READ_FILE_TOOL_NAME)} to validate all assumptions. **Prioritize empirical reproduction of reported issues to confirm the failure state.**${suggestion}`; } return `1. **Research:** Systematically map the codebase and validate assumptions.${searchSentence} Use ${formatToolName(READ_FILE_TOOL_NAME)} to validate all assumptions. **Prioritize empirical reproduction of reported issues to confirm the failure state.**${suggestion}`; diff --git a/packages/core/src/services/astAnalysisService.test.ts b/packages/core/src/services/astAnalysisService.test.ts new file mode 100644 index 00000000000..6da4cf7d0ea --- /dev/null +++ b/packages/core/src/services/astAnalysisService.test.ts @@ -0,0 +1,242 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import * as fs from 'node:fs/promises'; +import * as path from 'node:path'; +import * as os from 'node:os'; +import { + ASTAnalysisService, + extractSymbols, + findClosingBrace, + findIndentEnd, +} from './astAnalysisService.js'; + +describe('ASTAnalysisService', () => { + let tmpDir: string; + + beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'ast-svc-')); + }); + + afterEach(async () => { + await fs.rm(tmpDir, { recursive: true, force: true }); + }); + + describe('findClosingBrace', () => { + it('should find the closing brace of a simple block', () => { + const lines = ['function f() {', ' return 1;', '}']; + expect(findClosingBrace(lines, 0)).toBe(2); + }); + + it('should handle nested braces correctly', () => { + const lines = [ + 'class C {', + ' m() {', + ' if (x) {', + ' }', + ' }', + '}', + ]; + expect(findClosingBrace(lines, 0)).toBe(5); + expect(findClosingBrace(lines, 1)).toBe(4); + }); + + it('should ignore braces inside string literals', () => { + const lines = ['function f() {', ' const s = "}{";', '}']; + expect(findClosingBrace(lines, 0)).toBe(2); + }); + }); + + describe('findIndentEnd', () => { + it('should find the end of a Python indentation block', () => { + const lines = ['def f():', ' x = 1', ' return x', 'def g():']; + expect(findIndentEnd(lines, 0)).toBe(2); + }); + + it('should skip blank lines within a block', () => { + const lines = ['def f():', ' x = 1', '', ' y = 2', 'z = 3']; + expect(findIndentEnd(lines, 0)).toBe(3); + }); + }); + + describe('extractSymbols', () => { + it('should extract TypeScript class with methods', () => { + const lines = [ + 'export class MyService {', + ' private val: number;', + ' public process(x: string): void {', + ' console.log(x);', + ' }', + '}', + ]; + const syms = extractSymbols(lines, 'typescript'); + expect(syms).toHaveLength(1); + expect(syms[0].name).toBe('MyService'); + expect(syms[0].kind).toBe('class'); + expect(syms[0].children.length).toBeGreaterThanOrEqual(1); + }); + + it('should extract standalone functions', () => { + const lines = [ + 'export function doWork() {', + ' return 42;', + '}', + '', + 'export async function fetchData() {', + ' return null;', + '}', + ]; + const syms = extractSymbols(lines, 'typescript'); + expect(syms).toHaveLength(2); + expect(syms[0].name).toBe('doWork'); + expect(syms[1].name).toBe('fetchData'); + }); + + it('should extract interfaces and types', () => { + const lines = [ + 'export interface Config {', + ' host: string;', + '}', + 'export type Status = "ok" | "err";', + ]; + const syms = extractSymbols(lines, 'typescript'); + expect( + syms.some((s) => s.name === 'Config' && s.kind === 'interface'), + ).toBe(true); + expect(syms.some((s) => s.name === 'Status' && s.kind === 'type')).toBe( + true, + ); + }); + + it('should extract Python classes and functions', () => { + const lines = [ + 'class Handler:', + ' def run(self):', + ' pass', + 'def util():', + ' pass', + ]; + const syms = extractSymbols(lines, 'python'); + expect(syms).toHaveLength(2); + expect(syms[0].name).toBe('Handler'); + expect(syms[0].kind).toBe('class'); + expect(syms[1].name).toBe('util'); + }); + + it('should skip comments and imports', () => { + const lines = [ + '// comment', + 'import { X } from "y";', + 'export function real() {', + ' return 1;', + '}', + ]; + const syms = extractSymbols(lines, 'typescript'); + expect(syms).toHaveLength(1); + expect(syms[0].name).toBe('real'); + }); + + it('should return empty array for empty input', () => { + expect(extractSymbols([], 'typescript')).toHaveLength(0); + }); + + it('should truncate long signatures', () => { + const longLine = 'export function ' + 'a'.repeat(130) + '() {'; + const syms = extractSymbols([longLine, '}'], 'typescript'); + expect(syms).toHaveLength(1); + expect(syms[0].signature.length).toBeLessThanOrEqual(120); + }); + }); + + describe('getFileOutline', () => { + it('should outline a TypeScript file from disk', async () => { + await fs.writeFile( + path.join(tmpDir, 'svc.ts'), + 'export class Svc {\n run() {\n return 1;\n }\n}\nexport function helper() {\n return 2;\n}\n', + ); + const service = new ASTAnalysisService(tmpDir); + const outline = await service.getFileOutline('svc.ts'); + expect(outline).not.toBeNull(); + expect(outline!.language).toBe('typescript'); + expect(outline!.symbols.length).toBeGreaterThanOrEqual(2); + }); + + it('should return null for unsupported extensions', async () => { + await fs.writeFile(path.join(tmpDir, 'data.json'), '{}'); + const service = new ASTAnalysisService(tmpDir); + expect(await service.getFileOutline('data.json')).toBeNull(); + }); + + it('should return null for missing files', async () => { + const service = new ASTAnalysisService(tmpDir); + expect(await service.getFileOutline('nope.ts')).toBeNull(); + }); + }); + + describe('findSymbolBounds', () => { + it('should locate a class precisely', async () => { + const content = [ + 'import { X } from "x";', + '', + 'export class Target {', + ' method() {', + ' return 1;', + ' }', + '}', + '', + 'export function other() {}', + ].join('\n'); + await fs.writeFile(path.join(tmpDir, 'f.ts'), content); + const svc = new ASTAnalysisService(tmpDir); + const bounds = await svc.findSymbolBounds('f.ts', 'Target'); + expect(bounds).not.toBeNull(); + expect(bounds!.startLine).toBe(3); + expect(bounds!.endLine).toBe(7); + }); + + it('should return null for a non-existent symbol', async () => { + await fs.writeFile( + path.join(tmpDir, 'f.ts'), + 'export function real() {}\n', + ); + const svc = new ASTAnalysisService(tmpDir); + expect(await svc.findSymbolBounds('f.ts', 'ghost')).toBeNull(); + }); + }); + + describe('getCodebaseMap', () => { + it('should map multiple source files', async () => { + const src = path.join(tmpDir, 'src'); + await fs.mkdir(src); + await fs.writeFile(path.join(src, 'a.ts'), 'export class A {}\n'); + await fs.writeFile(path.join(src, 'b.ts'), 'export function b() {}\n'); + await fs.writeFile(path.join(src, 'c.json'), '{}'); + + const svc = new ASTAnalysisService(tmpDir); + const map = await svc.getCodebaseMap(); + expect(map).toContain('Codebase Map:'); + expect(map).toContain('class A'); + expect(map).toContain('function b'); + expect(map).not.toContain('.json'); + }); + + it('should skip node_modules', async () => { + const nm = path.join(tmpDir, 'node_modules', 'pkg'); + await fs.mkdir(nm, { recursive: true }); + await fs.writeFile(path.join(nm, 'index.ts'), 'export class X {}\n'); + await fs.writeFile( + path.join(tmpDir, 'main.ts'), + 'export class Main {}\n', + ); + + const svc = new ASTAnalysisService(tmpDir); + const map = await svc.getCodebaseMap(); + expect(map).toContain('Main'); + expect(map).not.toContain('node_modules'); + }); + }); +}); diff --git a/packages/core/src/services/astAnalysisService.ts b/packages/core/src/services/astAnalysisService.ts new file mode 100644 index 00000000000..b6bc1bac574 --- /dev/null +++ b/packages/core/src/services/astAnalysisService.ts @@ -0,0 +1,397 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as path from 'node:path'; +import * as fs from 'node:fs/promises'; +// Debug logging available via: import { debugLogger } from '../utils/debugLogger.js'; + +/** A symbol extracted from source code. */ +export interface ASTSymbol { + name: string; + kind: 'class' | 'function' | 'method' | 'interface' | 'type' | 'enum'; + startLine: number; + endLine: number; + signature: string; + children: ASTSymbol[]; +} + +/** Structural outline of a single file. */ +export interface ASTFileOutline { + filePath: string; + language: string; + symbols: ASTSymbol[]; + totalLines: number; +} + +const LANG_MAP: Record = { + '.ts': 'typescript', + '.tsx': 'typescript', + '.js': 'javascript', + '.jsx': 'javascript', + '.py': 'python', + '.go': 'go', + '.rs': 'rust', + '.java': 'java', +}; + +const SKIP_DIRS = new Set([ + 'node_modules', + '.git', + 'dist', + 'build', + '__pycache__', + 'vendor', + 'target', +]); + +/** + * Extracts structural outlines from source files using regex heuristics. + * + * Designed as a zero-dependency fallback for when ast-grep (sg) is not installed. + * Covers the three capabilities outlined in issue #22745: + * 1. Symbol-boundary detection for precise method-level reads + * 2. Structural search by symbol name + * 3. Compressed codebase mapping (class/function/signature outlines) + */ +export class ASTAnalysisService { + constructor(private readonly targetDir: string) {} + + /** + * Returns the structural outline of a single source file. + */ + async getFileOutline(filePath: string): Promise { + const absPath = path.resolve(this.targetDir, filePath); + const ext = path.extname(absPath); + const language = LANG_MAP[ext]; + if (!language) return null; + + let content: string; + try { + content = await fs.readFile(absPath, 'utf-8'); + } catch { + return null; + } + + const lines = content.split('\n'); + const symbols = extractSymbols(lines, language); + return { filePath, language, symbols, totalLines: lines.length }; + } + + /** + * Finds the start/end line bounds of a named symbol in a file. + */ + async findSymbolBounds( + filePath: string, + symbolName: string, + ): Promise<{ startLine: number; endLine: number } | null> { + const outline = await this.getFileOutline(filePath); + if (!outline) return null; + + const found = findSymbolRecursive(outline.symbols, symbolName); + if (!found) return null; + + return { startLine: found.startLine, endLine: found.endLine }; + } + + /** + * Generates a compressed codebase map for LLM consumption. + * Walks source files up to `maxFiles`, extracts top-level symbols, + * and returns a text outline with file paths and signatures. + */ + async getCodebaseMap( + subDir?: string, + maxFiles: number = 100, + ): Promise { + const searchDir = subDir + ? path.resolve(this.targetDir, subDir) + : this.targetDir; + + const files = await collectSourceFiles(searchDir, maxFiles); + const sections: string[] = []; + let totalSymbols = 0; + + for (const file of files) { + const relPath = path.relative(this.targetDir, file); + const outline = await this.getFileOutline(relPath); + if (!outline || outline.symbols.length === 0) continue; + + totalSymbols += countSymbols(outline.symbols); + sections.push(formatOutline(outline)); + } + + const header = `Codebase Map: ${sections.length} files, ${totalSymbols} symbols\n${'='.repeat(50)}`; + return header + '\n\n' + sections.join('\n\n'); + } +} + +// ── Pure helpers (exported for unit testing) ─────────────────────────────── + +export function extractSymbols(lines: string[], language: string): ASTSymbol[] { + const symbols: ASTSymbol[] = []; + const patterns = getDeclarationPatterns(language); + + for (let i = 0; i < lines.length; i++) { + const trimmed = lines[i].trim(); + if ( + trimmed === '' || + trimmed.startsWith('//') || + trimmed.startsWith('#') || + trimmed.startsWith('import ') || + trimmed.startsWith('from ') + ) { + continue; + } + + // Only match at top-level indentation (<=2 spaces for brace langs) + const indent = lines[i].length - lines[i].trimStart().length; + if (language !== 'python' && indent > 2) continue; + if (language === 'python' && indent > 0) continue; + + for (const { regex, kind } of patterns) { + const match = regex.exec(trimmed); + if (!match?.[1]) continue; + + const endLine = + language === 'python' + ? findIndentEnd(lines, i) + : findClosingBrace(lines, i); + + const sym: ASTSymbol = { + name: match[1], + kind, + startLine: i + 1, + endLine: endLine + 1, + signature: + trimmed.length > 120 ? trimmed.slice(0, 117) + '...' : trimmed, + children: + kind === 'class' || kind === 'interface' + ? extractMembers(lines, i + 1, endLine, language) + : [], + }; + + symbols.push(sym); + if (kind !== 'type' && kind !== 'enum') i = endLine; + break; + } + } + + return symbols; +} + +export function findClosingBrace(lines: string[], startLine: number): number { + let depth = 0; + let opened = false; + for (let i = startLine; i < lines.length; i++) { + const stripped = lines[i].replace(/'[^']*'|"[^"]*"|`[^`]*`/g, ''); + for (const ch of stripped) { + if (ch === '{') { + depth++; + opened = true; + } else if (ch === '}') { + depth--; + if (opened && depth === 0) return i; + } + } + } + return Math.min(startLine + 50, lines.length - 1); +} + +export function findIndentEnd(lines: string[], startLine: number): number { + const baseIndent = + lines[startLine].length - lines[startLine].trimStart().length; + let last = startLine; + for (let i = startLine + 1; i < lines.length; i++) { + if (lines[i].trim() === '') continue; + const indent = lines[i].length - lines[i].trimStart().length; + if (indent <= baseIndent) return last; + last = i; + } + return last; +} + +function extractMembers( + lines: string[], + start: number, + end: number, + language: string, +): ASTSymbol[] { + const members: ASTSymbol[] = []; + const patterns = getMemberPatterns(language); + + for (let i = start; i < end; i++) { + const trimmed = lines[i].trim(); + if (trimmed === '' || trimmed === '{' || trimmed === '}') continue; + for (const { regex, kind } of patterns) { + const match = regex.exec(trimmed); + if (!match?.[1]) continue; + const memberEnd = + language === 'python' + ? Math.min(findIndentEnd(lines, i), end) + : Math.min(findClosingBrace(lines, i), end); + members.push({ + name: match[1], + kind, + startLine: i + 1, + endLine: memberEnd + 1, + signature: + trimmed.length > 100 ? trimmed.slice(0, 97) + '...' : trimmed, + children: [], + }); + if (kind === 'method' || kind === 'function') i = memberEnd; + break; + } + } + + return members; +} + +function findSymbolRecursive( + symbols: ASTSymbol[], + name: string, +): ASTSymbol | null { + for (const s of symbols) { + if (s.name === name) return s; + const found = findSymbolRecursive(s.children, name); + if (found) return found; + } + return null; +} + +function getDeclarationPatterns(lang: string) { + const p: Array<{ regex: RegExp; kind: ASTSymbol['kind'] }> = []; + switch (lang) { + case 'typescript': + case 'javascript': + p.push({ + regex: /(?:export\s+)?(?:abstract\s+)?class\s+(\w+)/, + kind: 'class', + }); + p.push({ regex: /(?:export\s+)?interface\s+(\w+)/, kind: 'interface' }); + p.push({ regex: /(?:export\s+)?type\s+(\w+)/, kind: 'type' }); + p.push({ regex: /(?:export\s+)?enum\s+(\w+)/, kind: 'enum' }); + p.push({ + regex: /(?:export\s+)?(?:async\s+)?function\s+(\w+)/, + kind: 'function', + }); + break; + case 'python': + p.push({ regex: /^class\s+(\w+)/, kind: 'class' }); + p.push({ regex: /^(?:async\s+)?def\s+(\w+)/, kind: 'function' }); + break; + case 'go': + p.push({ regex: /^type\s+(\w+)\s+struct/, kind: 'class' }); + p.push({ regex: /^type\s+(\w+)\s+interface/, kind: 'interface' }); + p.push({ regex: /^func\s+(?:\([^)]*\)\s+)?(\w+)/, kind: 'function' }); + break; + case 'rust': + p.push({ regex: /(?:pub\s+)?struct\s+(\w+)/, kind: 'class' }); + p.push({ regex: /(?:pub\s+)?trait\s+(\w+)/, kind: 'interface' }); + p.push({ regex: /(?:pub\s+)?enum\s+(\w+)/, kind: 'enum' }); + p.push({ regex: /(?:pub\s+)?(?:async\s+)?fn\s+(\w+)/, kind: 'function' }); + break; + case 'java': + p.push({ + regex: /(?:public|private|protected)?\s*class\s+(\w+)/, + kind: 'class', + }); + p.push({ + regex: /(?:public|private|protected)?\s*interface\s+(\w+)/, + kind: 'interface', + }); + p.push({ + regex: /(?:public|private|protected)?\s*enum\s+(\w+)/, + kind: 'enum', + }); + break; + default: + break; + } + return p; +} + +function getMemberPatterns(lang: string) { + const p: Array<{ regex: RegExp; kind: ASTSymbol['kind'] }> = []; + switch (lang) { + case 'typescript': + case 'javascript': + p.push({ + regex: + /(?:public|private|protected|static|async|override|get|set)\s+(\w+)\s*[(<]/, + kind: 'method', + }); + p.push({ regex: /^(\w+)\s*\(/, kind: 'method' }); + break; + case 'python': + p.push({ regex: /^\s+(?:async\s+)?def\s+(\w+)/, kind: 'method' }); + break; + case 'go': + p.push({ regex: /func\s+\([^)]+\)\s+(\w+)/, kind: 'method' }); + break; + case 'rust': + p.push({ regex: /(?:pub\s+)?(?:async\s+)?fn\s+(\w+)/, kind: 'method' }); + break; + case 'java': + p.push({ + regex: /(?:public|private|protected|static)?\s*\w+\s+(\w+)\s*\(/, + kind: 'method', + }); + break; + default: + break; + } + return p; +} + +function formatOutline(outline: ASTFileOutline): string { + const header = `## ${outline.filePath} (${outline.language}, ${outline.totalLines} lines)`; + const body = outline.symbols + .map((s) => { + const range = `L${s.startLine}-${s.endLine}`; + let line = ` ${s.kind} ${s.name} [${range}]: ${s.signature}`; + for (const c of s.children) { + line += `\n ${c.kind} ${c.name} [L${c.startLine}-${c.endLine}]: ${c.signature}`; + } + return line; + }) + .join('\n'); + return header + '\n' + body; +} + +function countSymbols(syms: ASTSymbol[]): number { + let n = 0; + for (const s of syms) { + n += 1 + countSymbols(s.children); + } + return n; +} + +async function collectSourceFiles(dir: string, max: number): Promise { + const files: string[] = []; + async function walk(d: string, depth: number) { + if (depth > 6 || files.length >= max) return; + let entries; + try { + entries = await fs.readdir(d, { withFileTypes: true }); + } catch { + return; + } + for (const e of entries) { + if (files.length >= max) return; + const full = path.join(d, e.name); + if ( + e.isDirectory() && + !SKIP_DIRS.has(e.name) && + !e.name.startsWith('.') + ) { + await walk(full, depth + 1); + } else if (e.isFile() && LANG_MAP[path.extname(e.name)]) { + files.push(full); + } + } + } + await walk(dir, 0); + return files.sort(); +} diff --git a/packages/core/src/tools/ast-search.test.ts b/packages/core/src/tools/ast-search.test.ts new file mode 100644 index 00000000000..dec26ebd23d --- /dev/null +++ b/packages/core/src/tools/ast-search.test.ts @@ -0,0 +1,230 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import * as fs from 'node:fs/promises'; +import * as path from 'node:path'; +import * as os from 'node:os'; +import { ASTSearchTool } from './ast-search.js'; +import { AST_SEARCH_TOOL_NAME } from './tool-names.js'; +import type { MessageBus } from '../confirmation-bus/message-bus.js'; +import { makeFakeConfig } from '../test-utils/config.js'; + +describe('ASTSearchTool', () => { + let tmpDir: string; + let mockMessageBus: MessageBus; + + beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'ast-tool-')); + mockMessageBus = { + publish: vi.fn().mockResolvedValue(undefined), + subscribe: vi.fn(), + } as unknown as MessageBus; + }); + + afterEach(async () => { + await fs.rm(tmpDir, { recursive: true, force: true }); + }); + + function makeTool() { + const config = makeFakeConfig({ targetDir: tmpDir }); + return new ASTSearchTool(config, mockMessageBus); + } + + describe('static properties', () => { + it('should have the correct tool name', () => { + expect(ASTSearchTool.Name).toBe(AST_SEARCH_TOOL_NAME); + }); + + it('should produce a valid schema', () => { + const tool = makeTool(); + const schema = tool.getSchema(); + expect(schema.name).toBe(AST_SEARCH_TOOL_NAME); + expect(schema.description).toBeDefined(); + expect(schema.parametersJsonSchema).toBeDefined(); + }); + }); + + describe('symbol scope', () => { + it('should find a class and return its line bounds', async () => { + const content = [ + 'import { X } from "x";', + '', + 'export class TargetClass {', + ' method() {', + ' return 1;', + ' }', + '}', + ].join('\n'); + await fs.writeFile(path.join(tmpDir, 'target.ts'), content); + + const tool = makeTool(); + const invocation = tool.build({ + symbol_name: 'TargetClass', + file_path: 'target.ts', + scope: 'symbol', + }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + + expect(result.llmContent).toContain('TargetClass'); + expect(result.llmContent).toContain('Lines: 3-7'); + expect(result.llmContent).toContain('TIP: Use read_file'); + }); + + it('should find a function', async () => { + const content = [ + 'export function processData(input: string): string {', + ' return input.trim();', + '}', + ].join('\n'); + await fs.writeFile(path.join(tmpDir, 'utils.ts'), content); + + const tool = makeTool(); + const invocation = tool.build({ + symbol_name: 'processData', + file_path: 'utils.ts', + }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + + expect(result.llmContent).toContain('processData'); + expect(result.llmContent).toContain('Lines: 1-3'); + }); + + it('should return a helpful message for missing symbols', async () => { + await fs.writeFile( + path.join(tmpDir, 'empty.ts'), + 'export const x = 1;\n', + ); + + const tool = makeTool(); + const invocation = tool.build({ + symbol_name: 'NonExistent', + file_path: 'empty.ts', + scope: 'symbol', + }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + + expect(result.llmContent).toContain('not found'); + expect(result.llmContent).toContain('grep_search'); + }); + + it('should error when symbol_name is missing', async () => { + await fs.writeFile(path.join(tmpDir, 'f.ts'), 'class A {}\n'); + + const tool = makeTool(); + const invocation = tool.build({ file_path: 'f.ts', scope: 'symbol' }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + + expect(result.llmContent).toContain('symbol_name is required'); + }); + + it('should error when file_path is missing', async () => { + const tool = makeTool(); + const invocation = tool.build({ symbol_name: 'X', scope: 'symbol' }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + + expect(result.llmContent).toContain('file_path is required'); + }); + }); + + describe('outline scope', () => { + it('should return a file outline with symbols', async () => { + const content = [ + 'export interface Config {', + ' host: string;', + '}', + '', + 'export class Server {', + ' constructor() {}', + ' public start(): void {', + ' console.log("started");', + ' }', + '}', + '', + 'export function createServer(): Server {', + ' return new Server();', + '}', + ].join('\n'); + await fs.writeFile(path.join(tmpDir, 'server.ts'), content); + + const tool = makeTool(); + const invocation = tool.build({ + file_path: 'server.ts', + scope: 'outline', + }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + + expect(result.llmContent).toContain('server.ts'); + expect(result.llmContent).toContain('interface Config'); + expect(result.llmContent).toContain('class Server'); + expect(result.llmContent).toContain('function createServer'); + }); + + it('should fail gracefully for unsupported file types', async () => { + await fs.writeFile(path.join(tmpDir, 'data.txt'), 'hello'); + + const tool = makeTool(); + const invocation = tool.build({ + file_path: 'data.txt', + scope: 'outline', + }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + + expect(result.llmContent).toContain('Could not outline'); + }); + }); + + describe('map scope', () => { + it('should generate a codebase map', async () => { + const src = path.join(tmpDir, 'src'); + await fs.mkdir(src); + await fs.writeFile(path.join(src, 'a.ts'), 'export class Alpha {}\n'); + await fs.writeFile(path.join(src, 'b.ts'), 'export function beta() {}\n'); + + const tool = makeTool(); + const invocation = tool.build({ scope: 'map' }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + + expect(result.llmContent).toContain('Codebase Map:'); + expect(result.llmContent).toContain('Alpha'); + expect(result.llmContent).toContain('beta'); + }); + + it('should generate a map scoped to a subdirectory', async () => { + const sub = path.join(tmpDir, 'pkg'); + await fs.mkdir(sub); + await fs.writeFile(path.join(sub, 'c.ts'), 'export enum Status { OK }\n'); + await fs.writeFile( + path.join(tmpDir, 'root.ts'), + 'export class Root {}\n', + ); + + const tool = makeTool(); + const invocation = tool.build({ file_path: 'pkg', scope: 'map' }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + + expect(result.llmContent).toContain('Status'); + }); + }); +}); diff --git a/packages/core/src/tools/ast-search.ts b/packages/core/src/tools/ast-search.ts new file mode 100644 index 00000000000..94ff963e7bd --- /dev/null +++ b/packages/core/src/tools/ast-search.ts @@ -0,0 +1,241 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import type { MessageBus } from '../confirmation-bus/message-bus.js'; +import { + BaseDeclarativeTool, + BaseToolInvocation, + Kind, + type ToolInvocation, + type ToolResult, + type ExecuteOptions, +} from './tools.js'; +import type { Config } from '../config/config.js'; +import { AST_SEARCH_TOOL_NAME, AST_SEARCH_DISPLAY_NAME } from './tool-names.js'; +import { AST_SEARCH_DEFINITION } from './definitions/coreTools.js'; +import { resolveToolDeclaration } from './definitions/resolver.js'; +import { + ASTAnalysisService, + type ASTFileOutline, +} from '../services/astAnalysisService.js'; +import { debugLogger } from '../utils/debugLogger.js'; + +export interface ASTSearchToolParams { + symbol_name?: string; + file_path?: string; + scope?: 'symbol' | 'outline' | 'map'; +} + +class ASTSearchInvocation extends BaseToolInvocation< + ASTSearchToolParams, + ToolResult +> { + constructor( + private config: Config, + params: ASTSearchToolParams, + messageBus: MessageBus, + _toolName?: string, + _toolDisplayName?: string, + ) { + super(params, messageBus, _toolName, _toolDisplayName); + } + + getDescription(): string { + const scope = this.params.scope ?? 'symbol'; + if (scope === 'map') return 'Generating codebase structure map'; + if (scope === 'outline') + return `Outlining ${this.params.file_path ?? '(no file)'}`; + return `Finding symbol "${this.params.symbol_name ?? ''}" in ${this.params.file_path ?? 'workspace'}`; + } + + async execute(_options: ExecuteOptions): Promise { + const scope = this.params.scope ?? 'symbol'; + const astService = new ASTAnalysisService(this.config.getTargetDir()); + + try { + if (scope === 'map') { + return await this.handleMapScope(astService); + } + + if (!this.params.file_path) { + return { + llmContent: + 'Error: file_path is required for "symbol" and "outline" scopes.', + returnDisplay: 'Missing file_path', + }; + } + + if (scope === 'outline') { + return await this.handleOutlineScope(astService); + } + + // Default: symbol scope + if (!this.params.symbol_name) { + return { + llmContent: 'Error: symbol_name is required for "symbol" scope.', + returnDisplay: 'Missing symbol_name', + }; + } + + return await this.handleSymbolScope(astService); + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + debugLogger.warn('[ASTSearchTool] Error:', msg); + return { + llmContent: `AST search error: ${msg}`, + returnDisplay: 'Error', + }; + } + } + + private async handleSymbolScope( + astService: ASTAnalysisService, + ): Promise { + const bounds = await astService.findSymbolBounds( + this.params.file_path!, + this.params.symbol_name!, + ); + + if (!bounds) { + return { + llmContent: + `Symbol "${this.params.symbol_name}" not found in ${this.params.file_path}. ` + + 'Try using grep_search for a text-based search, or check the symbol name spelling.', + returnDisplay: 'Symbol not found', + }; + } + + const outline = await astService.getFileOutline(this.params.file_path!); + const symbol = outline?.symbols + .flatMap((s) => [s, ...s.children]) + .find((s) => s.name === this.params.symbol_name); + + const result = [ + `Found "${this.params.symbol_name}" in ${this.params.file_path}:`, + ` Lines: ${bounds.startLine}-${bounds.endLine} (${bounds.endLine - bounds.startLine + 1} lines)`, + symbol ? ` Kind: ${symbol.kind}` : '', + symbol ? ` Signature: ${symbol.signature}` : '', + '', + `TIP: Use read_file with start_line=${bounds.startLine} and end_line=${bounds.endLine} to read the exact symbol body.`, + ] + .filter(Boolean) + .join('\n'); + + return { + llmContent: result, + returnDisplay: `${this.params.symbol_name}: L${bounds.startLine}-${bounds.endLine}`, + display: { + name: AST_SEARCH_DISPLAY_NAME, + description: this.getDescription(), + resultSummary: `L${bounds.startLine}-${bounds.endLine}`, + result: { type: 'text', text: result }, + }, + }; + } + + private async handleOutlineScope( + astService: ASTAnalysisService, + ): Promise { + const outline = await astService.getFileOutline(this.params.file_path!); + if (!outline) { + return { + llmContent: `Could not outline "${this.params.file_path}". File may not exist or its language is not supported.`, + returnDisplay: 'Outline failed', + }; + } + + const result = formatOutlineResult(outline); + return { + llmContent: result, + returnDisplay: `${outline.symbols.length} symbols`, + display: { + name: AST_SEARCH_DISPLAY_NAME, + description: this.getDescription(), + resultSummary: `${outline.symbols.length} symbols`, + result: { type: 'text', text: result }, + }, + }; + } + + private async handleMapScope( + astService: ASTAnalysisService, + ): Promise { + const map = await astService.getCodebaseMap(this.params.file_path); + return { + llmContent: map, + returnDisplay: 'Codebase map generated', + display: { + name: AST_SEARCH_DISPLAY_NAME, + description: this.getDescription(), + result: { type: 'text', text: map.slice(0, 500) + '...' }, + }, + }; + } +} + +function formatOutlineResult(outline: ASTFileOutline): string { + const lines: string[] = []; + lines.push( + `File: ${outline.filePath} (${outline.language}, ${outline.totalLines} lines)`, + ); + lines.push(`Symbols: ${outline.symbols.length} top-level declarations`); + lines.push(''); + + for (const sym of outline.symbols) { + lines.push( + ` ${sym.kind} ${sym.name} [L${sym.startLine}-${sym.endLine}]: ${sym.signature}`, + ); + for (const child of sym.children) { + lines.push( + ` ${child.kind} ${child.name} [L${child.startLine}-${child.endLine}]: ${child.signature}`, + ); + } + } + + return lines.join('\n'); +} + +export class ASTSearchTool extends BaseDeclarativeTool< + ASTSearchToolParams, + ToolResult +> { + static readonly Name = AST_SEARCH_TOOL_NAME; + + constructor( + private config: Config, + messageBus: MessageBus, + ) { + super( + ASTSearchTool.Name, + AST_SEARCH_DISPLAY_NAME, + AST_SEARCH_DEFINITION.base.description!, + Kind.Search, + AST_SEARCH_DEFINITION.base.parametersJsonSchema, + messageBus, + true, + false, + ); + } + + protected createInvocation( + params: ASTSearchToolParams, + messageBus: MessageBus, + _toolName?: string, + _toolDisplayName?: string, + ): ToolInvocation { + return new ASTSearchInvocation( + this.config, + params, + messageBus, + _toolName, + _toolDisplayName, + ); + } + + override getSchema(modelId?: string) { + return resolveToolDeclaration(AST_SEARCH_DEFINITION, modelId); + } +} diff --git a/packages/core/src/tools/definitions/base-declarations.ts b/packages/core/src/tools/definitions/base-declarations.ts index 6c5d45869d8..ff5cb11b2fc 100644 --- a/packages/core/src/tools/definitions/base-declarations.ts +++ b/packages/core/src/tools/definitions/base-declarations.ts @@ -136,3 +136,9 @@ export const COMPLETE_TASK_DISPLAY_NAME = 'Complete Task'; // -- MCP Resources -- export const READ_MCP_RESOURCE_TOOL_NAME = 'read_mcp_resource'; export const LIST_MCP_RESOURCES_TOOL_NAME = 'list_mcp_resources'; + +// -- ast_search (AST-aware structural search) -- +export const AST_SEARCH_TOOL_NAME = 'ast_search'; +export const AST_SEARCH_PARAM_SYMBOL_NAME = 'symbol_name'; +export const AST_SEARCH_PARAM_FILE_PATH = 'file_path'; +export const AST_SEARCH_PARAM_SCOPE = 'scope'; diff --git a/packages/core/src/tools/definitions/coreTools.ts b/packages/core/src/tools/definitions/coreTools.ts index 2e5c0312881..5b0367f8936 100644 --- a/packages/core/src/tools/definitions/coreTools.ts +++ b/packages/core/src/tools/definitions/coreTools.ts @@ -99,6 +99,10 @@ export { TOPIC_PARAM_TITLE, TOPIC_PARAM_SUMMARY, TOPIC_PARAM_STRATEGIC_INTENT, + AST_SEARCH_TOOL_NAME, + AST_SEARCH_PARAM_SYMBOL_NAME, + AST_SEARCH_PARAM_FILE_PATH, + AST_SEARCH_PARAM_SCOPE, } from './base-declarations.js'; // Re-export sets for compatibility @@ -287,3 +291,10 @@ export const LIST_MCP_RESOURCES_DEFINITION: ToolDefinition = { }, overrides: (modelId) => getToolSet(modelId).list_mcp_resources, }; + +export const AST_SEARCH_DEFINITION: ToolDefinition = { + get base() { + return DEFAULT_LEGACY_SET.ast_search; + }, + overrides: (modelId) => getToolSet(modelId).ast_search, +}; diff --git a/packages/core/src/tools/definitions/model-family-sets/default-legacy.ts b/packages/core/src/tools/definitions/model-family-sets/default-legacy.ts index 3dfe8dd40e3..d1a2726c328 100644 --- a/packages/core/src/tools/definitions/model-family-sets/default-legacy.ts +++ b/packages/core/src/tools/definitions/model-family-sets/default-legacy.ts @@ -73,6 +73,10 @@ import { ASK_USER_OPTION_PARAM_LABEL, ASK_USER_OPTION_PARAM_DESCRIPTION, PLAN_MODE_PARAM_REASON, + AST_SEARCH_TOOL_NAME, + AST_SEARCH_PARAM_SYMBOL_NAME, + AST_SEARCH_PARAM_FILE_PATH, + AST_SEARCH_PARAM_SCOPE, } from '../base-declarations.js'; import { getShellDeclaration, @@ -761,4 +765,39 @@ The agent did not use the todo list because this task could be completed by a ti required: [], }, }, + + ast_search: { + name: AST_SEARCH_TOOL_NAME, + description: + 'Searches for a named code symbol (class, function, method, interface, type, enum) ' + + 'and returns its precise line boundaries and structural signature. ' + + 'Use this to read a specific function or class without guessing line numbers. ' + + 'When scope is "outline", returns the full structural outline of the file instead. ' + + 'When scope is "map", returns a compressed map of the codebase showing all top-level symbols.', + parametersJsonSchema: { + type: 'object', + properties: { + [AST_SEARCH_PARAM_SYMBOL_NAME]: { + description: + 'The exact name of the symbol to locate (e.g. "MyClass", "processData"). ' + + 'Required when scope is "symbol" (the default). Ignored when scope is "outline" or "map".', + type: 'string', + }, + [AST_SEARCH_PARAM_FILE_PATH]: { + description: + 'The path to the file to search in. Required for "symbol" and "outline" scopes. ' + + 'Optional for "map" scope (defaults to working directory).', + type: 'string', + }, + [AST_SEARCH_PARAM_SCOPE]: { + description: + 'The type of AST query: "symbol" (default) to find one symbol\'s bounds, ' + + '"outline" to get a file\'s structural skeleton, or "map" to get a compressed codebase overview.', + type: 'string', + enum: ['symbol', 'outline', 'map'], + }, + }, + required: [], + }, + }, }; diff --git a/packages/core/src/tools/definitions/model-family-sets/gemini-3.ts b/packages/core/src/tools/definitions/model-family-sets/gemini-3.ts index 57a897f9eef..65b401cdd6d 100644 --- a/packages/core/src/tools/definitions/model-family-sets/gemini-3.ts +++ b/packages/core/src/tools/definitions/model-family-sets/gemini-3.ts @@ -73,6 +73,10 @@ import { ASK_USER_OPTION_PARAM_LABEL, ASK_USER_OPTION_PARAM_DESCRIPTION, PLAN_MODE_PARAM_REASON, + AST_SEARCH_TOOL_NAME, + AST_SEARCH_PARAM_SYMBOL_NAME, + AST_SEARCH_PARAM_FILE_PATH, + AST_SEARCH_PARAM_SCOPE, } from '../base-declarations.js'; import { getShellDeclaration, @@ -745,4 +749,29 @@ The agent did not use the todo list because this task could be completed by a ti required: [], }, }, + + ast_search: { + name: AST_SEARCH_TOOL_NAME, + description: + 'AST-aware structural search. Finds symbol bounds, file outlines, or codebase maps.', + parametersJsonSchema: { + type: 'object', + properties: { + [AST_SEARCH_PARAM_SYMBOL_NAME]: { + description: 'Symbol name to locate.', + type: 'string', + }, + [AST_SEARCH_PARAM_FILE_PATH]: { + description: 'File to search in.', + type: 'string', + }, + [AST_SEARCH_PARAM_SCOPE]: { + description: '"symbol", "outline", or "map".', + type: 'string', + enum: ['symbol', 'outline', 'map'], + }, + }, + required: [], + }, + }, }; diff --git a/packages/core/src/tools/definitions/types.ts b/packages/core/src/tools/definitions/types.ts index d6f0a723a15..effb9249a00 100644 --- a/packages/core/src/tools/definitions/types.ts +++ b/packages/core/src/tools/definitions/types.ts @@ -52,4 +52,5 @@ export interface CoreToolSet { read_mcp_resource: FunctionDeclaration; list_mcp_resources: FunctionDeclaration; update_topic?: FunctionDeclaration; + ast_search: FunctionDeclaration; } diff --git a/packages/core/src/tools/read-file.test.ts b/packages/core/src/tools/read-file.test.ts index df0bd171c79..0463ed7889f 100644 --- a/packages/core/src/tools/read-file.test.ts +++ b/packages/core/src/tools/read-file.test.ts @@ -338,6 +338,7 @@ describe('ReadFileTool', () => { 'IMPORTANT: The file content has been truncated', ); expect(result.llmContent).toContain('--- FILE CONTENT (truncated) ---'); + expect(result.llmContent).toContain('ast_search'); expect(result.returnDisplay).toContain('some lines were shortened'); }); diff --git a/packages/core/src/tools/read-file.ts b/packages/core/src/tools/read-file.ts index 29ed50c61bb..b275ae18425 100644 --- a/packages/core/src/tools/read-file.ts +++ b/packages/core/src/tools/read-file.ts @@ -164,6 +164,7 @@ class ReadFileToolInvocation extends BaseToolInvocation< IMPORTANT: The file content has been truncated. Status: Showing lines ${start}-${end} of ${total} total lines. Action: To read more of the file, you can use the 'start_line' and 'end_line' parameters in a subsequent 'read_file' call. For example, to read the next section of the file, use start_line: ${end + 1}. +TIP: Use 'ast_search' with scope "outline" to see the file's structural skeleton, or with scope "symbol" to jump directly to a specific function or class. --- FILE CONTENT (truncated) --- ${result.llmContent}`; diff --git a/packages/core/src/tools/tool-names.ts b/packages/core/src/tools/tool-names.ts index 0987f9f3dd0..537bc25bc21 100644 --- a/packages/core/src/tools/tool-names.ts +++ b/packages/core/src/tools/tool-names.ts @@ -82,6 +82,10 @@ import { TOPIC_PARAM_TITLE, TOPIC_PARAM_SUMMARY, TOPIC_PARAM_STRATEGIC_INTENT, + AST_SEARCH_TOOL_NAME, + AST_SEARCH_PARAM_SYMBOL_NAME, + AST_SEARCH_PARAM_FILE_PATH, + AST_SEARCH_PARAM_SCOPE, } from './definitions/coreTools.js'; export { @@ -162,8 +166,15 @@ export { TOPIC_PARAM_TITLE, TOPIC_PARAM_SUMMARY, TOPIC_PARAM_STRATEGIC_INTENT, + AST_SEARCH_TOOL_NAME, + AST_SEARCH_PARAM_SYMBOL_NAME, + AST_SEARCH_PARAM_FILE_PATH, + AST_SEARCH_PARAM_SCOPE, }; +// Tool Display Names (AST) +export const AST_SEARCH_DISPLAY_NAME = 'ASTSearch'; + export const EDIT_TOOL_NAMES = new Set([EDIT_TOOL_NAME, WRITE_FILE_TOOL_NAME]); /** @@ -273,6 +284,7 @@ export const ALL_BUILTIN_TOOL_NAMES = [ AGENT_TOOL_NAME, READ_MCP_RESOURCE_TOOL_NAME, LIST_MCP_RESOURCES_TOOL_NAME, + AST_SEARCH_TOOL_NAME, ] as const; /** @@ -294,6 +306,7 @@ export const PLAN_MODE_TOOLS = [ 'cli_help', READ_MCP_RESOURCE_TOOL_NAME, LIST_MCP_RESOURCES_TOOL_NAME, + AST_SEARCH_TOOL_NAME, ] as const; /** From c81acef8d1a8b0a220640e2feb6960e990d97859 Mon Sep 17 00:00:00 2001 From: dylanyunlon Date: Fri, 18 Sep 2026 14:04:17 +0000 Subject: [PATCH 02/23] fix: address gemini-code-assist review feedback Addresses all 11 review comments from gemini-code-assist[bot]: Security (CRITICAL + HIGH): - Add validateToolParamValues to ASTSearchTool with path validation using resolveDefensiveToolPath + resolveToRealPath + validatePathAccess, preventing path traversal attacks (comments #1, #2, #8, #9) Parsing correctness (HIGH): - findClosingBrace: track parenthesis depth to ignore braces inside inline object types in function params (comment #3) - findClosingBrace: return lines.length-1 instead of startLine+50 when brace matching fails, for honest boundary reporting (comment #7) - extractSymbols: track block comment state (/* ... */) to skip commented-out code declarations (comment #4) - findIndentEnd: track Python triple-quoted strings to avoid early termination on docstrings with low indentation (comment #5) - extractSymbols: fix enum body skipping, enums now advance the line pointer past their body like classes/functions (comment #10) Documentation: - collectSourceFiles: document .gitignore/.geminiignore limitation and plan for FileDiscoveryService integration (comment #6) - findClosingBrace: document escaped-quote limitation in the string-literal stripping regex (comment #11) --- packages/cli/src/config/auth.ts | 4 + packages/core/src/config/config.ts | 4 + .../core/src/services/astAnalysisService.ts | 57 ++++++++++++- packages/core/src/tools/ast-search.test.ts | 18 ++--- packages/core/src/tools/ast-search.ts | 79 ++++++++++++++++++- 5 files changed, 146 insertions(+), 16 deletions(-) diff --git a/packages/cli/src/config/auth.ts b/packages/cli/src/config/auth.ts index 1ca07f98eb4..21f3965a512 100644 --- a/packages/cli/src/config/auth.ts +++ b/packages/cli/src/config/auth.ts @@ -45,5 +45,9 @@ export async function validateAuthMethod( return null; } + if (authMethod === AuthType.GATEWAY) { + return null; + } + return 'Invalid auth method selected.'; } diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index c53066573d0..c2ef8bdea48 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -41,6 +41,7 @@ import { EditTool } from '../tools/edit.js'; import { ShellTool } from '../tools/shell.js'; import { WriteFileTool } from '../tools/write-file.js'; import { WebFetchTool } from '../tools/web-fetch.js'; +import { ASTSearchTool } from '../tools/ast-search.js'; import { setGeminiMdFilename, getCurrentGeminiMdFilename, @@ -4020,6 +4021,9 @@ export class Config implements McpContext, AgentLoopContext { maybeRegister(ListMcpResourcesTool, () => registry.registerTool(new ListMcpResourcesTool(this, this.messageBus)), ); + maybeRegister(ASTSearchTool, () => + registry.registerTool(new ASTSearchTool(this, this.messageBus)), + ); maybeRegister(ShellTool, () => registry.registerTool(new ShellTool(this, this.messageBus)), ); diff --git a/packages/core/src/services/astAnalysisService.ts b/packages/core/src/services/astAnalysisService.ts index b6bc1bac574..7bb285060a5 100644 --- a/packages/core/src/services/astAnalysisService.ts +++ b/packages/core/src/services/astAnalysisService.ts @@ -132,13 +132,30 @@ export class ASTAnalysisService { export function extractSymbols(lines: string[], language: string): ASTSymbol[] { const symbols: ASTSymbol[] = []; const patterns = getDeclarationPatterns(language); + let inBlockComment = false; for (let i = 0; i < lines.length; i++) { const trimmed = lines[i].trim(); + + // Track block comments (/* ... */) to avoid parsing commented-out code + if (inBlockComment) { + if (trimmed.includes('*/')) { + inBlockComment = false; + } + continue; + } + if (trimmed.startsWith('/*')) { + if (!trimmed.includes('*/')) { + inBlockComment = true; + } + continue; + } + if ( trimmed === '' || trimmed.startsWith('//') || trimmed.startsWith('#') || + trimmed.startsWith('*') || trimmed.startsWith('import ') || trimmed.startsWith('from ') ) { @@ -173,7 +190,9 @@ export function extractSymbols(lines: string[], language: string): ASTSymbol[] { }; symbols.push(sym); - if (kind !== 'type' && kind !== 'enum') i = endLine; + // Skip past the symbol body for all block declarations (class, function, enum, etc.) + // Only 'type' aliases are single-line and should not advance + if (kind !== 'type') i = endLine; break; } } @@ -183,10 +202,18 @@ export function extractSymbols(lines: string[], language: string): ASTSymbol[] { export function findClosingBrace(lines: string[], startLine: number): number { let depth = 0; + let parenDepth = 0; let opened = false; for (let i = startLine; i < lines.length; i++) { + // Strip string literals to avoid false brace matches. + // NOTE: This regex does not handle escaped quotes inside strings + // (e.g. "a \" {"). This is a known limitation of the heuristic parser. const stripped = lines[i].replace(/'[^']*'|"[^"]*"|`[^`]*`/g, ''); for (const ch of stripped) { + if (ch === '(') parenDepth++; + else if (ch === ')') parenDepth--; + // Ignore braces inside parentheses (inline object types in params) + if (parenDepth > 0) continue; if (ch === '{') { depth++; opened = true; @@ -196,15 +223,29 @@ export function findClosingBrace(lines: string[], startLine: number): number { } } } - return Math.min(startLine + 50, lines.length - 1); + // If brace matching failed, return end of file rather than an arbitrary offset + return lines.length - 1; } export function findIndentEnd(lines: string[], startLine: number): number { const baseIndent = lines[startLine].length - lines[startLine].trimStart().length; let last = startLine; + let inTripleQuote = false; for (let i = startLine + 1; i < lines.length; i++) { - if (lines[i].trim() === '') continue; + const trimmed = lines[i].trim(); + + // Track Python triple-quoted strings which can have arbitrary indentation + const tripleCount = (trimmed.match(/"""|'''/g) || []).length; + if (tripleCount % 2 !== 0) { + inTripleQuote = !inTripleQuote; + } + if (inTripleQuote) { + last = i; + continue; + } + + if (trimmed === '') continue; const indent = lines[i].length - lines[i].trimStart().length; if (indent <= baseIndent) return last; last = i; @@ -368,6 +409,16 @@ function countSymbols(syms: ASTSymbol[]): number { return n; } +/** + * Walks directories to collect source files with known extensions. + * Note: This does not currently respect .gitignore or .geminiignore patterns. + * When called through the `ast_search` tool, path access is validated by the + * tool's validateToolParamValues and the Config.validatePathAccess check, so + * ignored files will not be exposed to the user. For codebase map generation, + * the SKIP_DIRS set covers the most common build/dependency directories. + * Full ignore-pattern integration should be added via FileDiscoveryService + * in a follow-up PR. + */ async function collectSourceFiles(dir: string, max: number): Promise { const files: string[] = []; async function walk(d: string, depth: number) { diff --git a/packages/core/src/tools/ast-search.test.ts b/packages/core/src/tools/ast-search.test.ts index dec26ebd23d..4483f193955 100644 --- a/packages/core/src/tools/ast-search.test.ts +++ b/packages/core/src/tools/ast-search.test.ts @@ -121,22 +121,16 @@ describe('ASTSearchTool', () => { await fs.writeFile(path.join(tmpDir, 'f.ts'), 'class A {}\n'); const tool = makeTool(); - const invocation = tool.build({ file_path: 'f.ts', scope: 'symbol' }); - const result = await invocation.execute({ - abortSignal: new AbortController().signal, - }); - - expect(result.llmContent).toContain('symbol_name is required'); + expect(() => tool.build({ file_path: 'f.ts', scope: 'symbol' })).toThrow( + 'symbol_name', + ); }); it('should error when file_path is missing', async () => { const tool = makeTool(); - const invocation = tool.build({ symbol_name: 'X', scope: 'symbol' }); - const result = await invocation.execute({ - abortSignal: new AbortController().signal, - }); - - expect(result.llmContent).toContain('file_path is required'); + expect(() => tool.build({ symbol_name: 'X', scope: 'symbol' })).toThrow( + 'file_path', + ); }); }); diff --git a/packages/core/src/tools/ast-search.ts b/packages/core/src/tools/ast-search.ts index 94ff963e7bd..f4dc8fb812a 100644 --- a/packages/core/src/tools/ast-search.ts +++ b/packages/core/src/tools/ast-search.ts @@ -5,6 +5,9 @@ */ import type { MessageBus } from '../confirmation-bus/message-bus.js'; +import path from 'node:path'; +import { resolveDefensiveToolPath, resolveToRealPath } from '../utils/paths.js'; +import { ToolErrorType } from './tool-error.js'; import { BaseDeclarativeTool, BaseToolInvocation, @@ -53,7 +56,8 @@ class ASTSearchInvocation extends BaseToolInvocation< async execute(_options: ExecuteOptions): Promise { const scope = this.params.scope ?? 'symbol'; - const astService = new ASTAnalysisService(this.config.getTargetDir()); + const targetDir = this.config.getTargetDir(); + const astService = new ASTAnalysisService(targetDir); try { if (scope === 'map') { @@ -68,6 +72,35 @@ class ASTSearchInvocation extends BaseToolInvocation< }; } + // Validate path stays within workspace boundaries + const sanitizedPath = resolveDefensiveToolPath( + this.params.file_path, + targetDir, + ); + let resolvedPath: string; + try { + resolvedPath = resolveToRealPath( + path.resolve(targetDir, sanitizedPath), + ); + } catch { + resolvedPath = path.resolve(targetDir, sanitizedPath); + } + + const validationError = this.config.validatePathAccess( + resolvedPath, + 'read', + ); + if (validationError) { + return { + llmContent: validationError, + returnDisplay: 'Path not in workspace.', + error: { + message: validationError, + type: ToolErrorType.PATH_NOT_IN_WORKSPACE, + }, + }; + } + if (scope === 'outline') { return await this.handleOutlineScope(astService); } @@ -220,6 +253,50 @@ export class ASTSearchTool extends BaseDeclarativeTool< ); } + protected override validateToolParamValues( + params: ASTSearchToolParams, + ): string | null { + const scope = params.scope ?? 'symbol'; + + if ( + scope === 'symbol' && + (!params.symbol_name || params.symbol_name.trim() === '') + ) { + return "The 'symbol_name' parameter must be non-empty when scope is 'symbol'."; + } + + if ( + (scope === 'symbol' || scope === 'outline') && + (!params.file_path || params.file_path.trim() === '') + ) { + return "The 'file_path' parameter must be non-empty for 'symbol' and 'outline' scopes."; + } + + if (params.file_path) { + const sanitizedPath = resolveDefensiveToolPath( + params.file_path, + this.config.getTargetDir(), + ); + let resolvedPath: string; + try { + resolvedPath = resolveToRealPath( + path.resolve(this.config.getTargetDir(), sanitizedPath), + ); + } catch (err) { + return `Failed to resolve path: ${err instanceof Error ? err.message : String(err)}`; + } + const validationError = this.config.validatePathAccess( + resolvedPath, + 'read', + ); + if (validationError) { + return validationError; + } + } + + return null; + } + protected createInvocation( params: ASTSearchToolParams, messageBus: MessageBus, From a1b136e0670745b9a290c1e6831fdb4c1c27057a Mon Sep 17 00:00:00 2001 From: dylanyunlon Date: Fri, 18 Sep 2026 14:18:55 +0000 Subject: [PATCH 03/23] fix: address round-2 gemini-code-assist review Fixes 3 new comments from second review round: CRITICAL: Pass sanitized path to ASTAnalysisService instead of raw user-supplied file_path. Previously, validateToolParamValues checked the path but handleSymbolScope/handleOutlineScope still forwarded this.params.file_path, bypassing the traversal protection. Now sanitizedPath flows through to all service calls. HIGH: Strip single-line comments (// ...) before brace counting in findClosingBrace. A comment like "// }" would decrement depth and produce incorrect symbol boundaries. HIGH: Skip comment lines (# and //) in findIndentEnd for Python. A top-level comment inside a function body would trigger early block termination due to its low indentation. --- packages/core/src/services/astAnalysisService.ts | 13 ++++++++++++- packages/core/src/tools/ast-search.ts | 16 +++++++++------- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/packages/core/src/services/astAnalysisService.ts b/packages/core/src/services/astAnalysisService.ts index 7bb285060a5..5e3fde71a8a 100644 --- a/packages/core/src/services/astAnalysisService.ts +++ b/packages/core/src/services/astAnalysisService.ts @@ -208,7 +208,12 @@ export function findClosingBrace(lines: string[], startLine: number): number { // Strip string literals to avoid false brace matches. // NOTE: This regex does not handle escaped quotes inside strings // (e.g. "a \" {"). This is a known limitation of the heuristic parser. - const stripped = lines[i].replace(/'[^']*'|"[^"]*"|`[^`]*`/g, ''); + let stripped = lines[i].replace(/'[^']*'|"[^"]*"|`[^`]*`/g, ''); + // Strip single-line comments which may contain braces (e.g. "// }") + const commentIdx = stripped.indexOf('//'); + if (commentIdx >= 0) { + stripped = stripped.slice(0, commentIdx); + } for (const ch of stripped) { if (ch === '(') parenDepth++; else if (ch === ')') parenDepth--; @@ -246,6 +251,12 @@ export function findIndentEnd(lines: string[], startLine: number): number { } if (trimmed === '') continue; + // Skip comment lines - they may have arbitrary indentation and should + // not terminate the block (e.g. a top-level # comment inside a function) + if (trimmed.startsWith('#') || trimmed.startsWith('//')) { + last = i; + continue; + } const indent = lines[i].length - lines[i].trimStart().length; if (indent <= baseIndent) return last; last = i; diff --git a/packages/core/src/tools/ast-search.ts b/packages/core/src/tools/ast-search.ts index f4dc8fb812a..fd691363e5b 100644 --- a/packages/core/src/tools/ast-search.ts +++ b/packages/core/src/tools/ast-search.ts @@ -102,7 +102,7 @@ class ASTSearchInvocation extends BaseToolInvocation< } if (scope === 'outline') { - return await this.handleOutlineScope(astService); + return await this.handleOutlineScope(astService, sanitizedPath); } // Default: symbol scope @@ -113,7 +113,7 @@ class ASTSearchInvocation extends BaseToolInvocation< }; } - return await this.handleSymbolScope(astService); + return await this.handleSymbolScope(astService, sanitizedPath); } catch (err) { const msg = err instanceof Error ? err.message : String(err); debugLogger.warn('[ASTSearchTool] Error:', msg); @@ -126,28 +126,29 @@ class ASTSearchInvocation extends BaseToolInvocation< private async handleSymbolScope( astService: ASTAnalysisService, + safePath: string, ): Promise { const bounds = await astService.findSymbolBounds( - this.params.file_path!, + safePath, this.params.symbol_name!, ); if (!bounds) { return { llmContent: - `Symbol "${this.params.symbol_name}" not found in ${this.params.file_path}. ` + + `Symbol "${this.params.symbol_name}" not found in ${safePath}. ` + 'Try using grep_search for a text-based search, or check the symbol name spelling.', returnDisplay: 'Symbol not found', }; } - const outline = await astService.getFileOutline(this.params.file_path!); + const outline = await astService.getFileOutline(safePath); const symbol = outline?.symbols .flatMap((s) => [s, ...s.children]) .find((s) => s.name === this.params.symbol_name); const result = [ - `Found "${this.params.symbol_name}" in ${this.params.file_path}:`, + `Found "${this.params.symbol_name}" in ${safePath}:`, ` Lines: ${bounds.startLine}-${bounds.endLine} (${bounds.endLine - bounds.startLine + 1} lines)`, symbol ? ` Kind: ${symbol.kind}` : '', symbol ? ` Signature: ${symbol.signature}` : '', @@ -171,8 +172,9 @@ class ASTSearchInvocation extends BaseToolInvocation< private async handleOutlineScope( astService: ASTAnalysisService, + safePath: string, ): Promise { - const outline = await astService.getFileOutline(this.params.file_path!); + const outline = await astService.getFileOutline(safePath); if (!outline) { return { llmContent: `Could not outline "${this.params.file_path}". File may not exist or its language is not supported.`, From 95504880e05b35280a3de3b2a4b396617924be37 Mon Sep 17 00:00:00 2001 From: dylanyunlon Date: Fri, 18 Sep 2026 14:51:23 +0000 Subject: [PATCH 04/23] fix: address round-3 gemini-code-assist review Fixes 5 comments from third review round: CRITICAL/HIGH (#1,#2): Map scope path traversal - file_path for "map" scope now goes through resolveDefensiveToolPath + validatePathAccess before reaching getCodebaseMap. handleMapScope accepts safePath param. HIGH (#3): Block comment stripping rewritten from state-tracking to character-level replacement. New stripBlockComments() replaces comment content with spaces, preserving line numbers. Handles mid-line comments and multi-line blocks correctly. HIGH (#4): validateToolParamValues now trims symbol_name and file_path at the top before any checks, preventing whitespace-only values. HIGH (#5): handleSymbolScope accepts trimmed symbolName as a parameter instead of reading raw this.params.symbol_name, so LLM-injected whitespace does not cause lookup mismatches. --- .../core/src/services/astAnalysisService.ts | 60 +++++++++++---- packages/core/src/tools/ast-search.ts | 74 +++++++++++++------ 2 files changed, 94 insertions(+), 40 deletions(-) diff --git a/packages/core/src/services/astAnalysisService.ts b/packages/core/src/services/astAnalysisService.ts index 5e3fde71a8a..ffdaa5e078d 100644 --- a/packages/core/src/services/astAnalysisService.ts +++ b/packages/core/src/services/astAnalysisService.ts @@ -132,24 +132,14 @@ export class ASTAnalysisService { export function extractSymbols(lines: string[], language: string): ASTSymbol[] { const symbols: ASTSymbol[] = []; const patterns = getDeclarationPatterns(language); - let inBlockComment = false; - for (let i = 0; i < lines.length; i++) { - const trimmed = lines[i].trim(); + // Pre-process: strip block comments by replacing their content with spaces. + // This handles mid-line comments like `code /* comment */ more_code` and + // multi-line blocks, while preserving line numbers and offsets. + const cleaned = stripBlockComments(lines); - // Track block comments (/* ... */) to avoid parsing commented-out code - if (inBlockComment) { - if (trimmed.includes('*/')) { - inBlockComment = false; - } - continue; - } - if (trimmed.startsWith('/*')) { - if (!trimmed.includes('*/')) { - inBlockComment = true; - } - continue; - } + for (let i = 0; i < cleaned.length; i++) { + const trimmed = cleaned[i].trim(); if ( trimmed === '' || @@ -200,6 +190,44 @@ export function extractSymbols(lines: string[], language: string): ASTSymbol[] { return symbols; } +/** + * Strips block comments from source lines by replacing comment + * characters with spaces. This preserves line count and character offsets + * so that line-number-based logic (brace counting, indentation) stays correct. + * Handles mid-line comments, multi-line blocks, and lines with code after a comment. + */ +export function stripBlockComments(lines: string[]): string[] { + const result: string[] = []; + let inComment = false; + for (const line of lines) { + let out = ''; + let j = 0; + while (j < line.length) { + if (inComment) { + if (j + 1 < line.length && line[j] === '*' && line[j + 1] === '/') { + out += ' '; + j += 2; + inComment = false; + } else { + out += ' '; + j++; + } + } else { + if (j + 1 < line.length && line[j] === '/' && line[j + 1] === '*') { + out += ' '; + j += 2; + inComment = true; + } else { + out += line[j]; + j++; + } + } + } + result.push(out); + } + return result; +} + export function findClosingBrace(lines: string[], startLine: number): number { let depth = 0; let parenDepth = 0; diff --git a/packages/core/src/tools/ast-search.ts b/packages/core/src/tools/ast-search.ts index fd691363e5b..0f86f0cbe97 100644 --- a/packages/core/src/tools/ast-search.ts +++ b/packages/core/src/tools/ast-search.ts @@ -60,11 +60,38 @@ class ASTSearchInvocation extends BaseToolInvocation< const astService = new ASTAnalysisService(targetDir); try { + // Trim params to prevent whitespace-only values and search mismatches + const filePath = this.params.file_path?.trim(); + const symbolName = this.params.symbol_name?.trim(); + if (scope === 'map') { - return await this.handleMapScope(astService); + // For map scope, file_path is optional (subdirectory filter) + let safeMapPath: string | undefined; + if (filePath) { + const sanitized = resolveDefensiveToolPath(filePath, targetDir); + let resolved: string; + try { + resolved = resolveToRealPath(path.resolve(targetDir, sanitized)); + } catch { + resolved = path.resolve(targetDir, sanitized); + } + const err = this.config.validatePathAccess(resolved, 'read'); + if (err) { + return { + llmContent: err, + returnDisplay: 'Path not in workspace.', + error: { + message: err, + type: ToolErrorType.PATH_NOT_IN_WORKSPACE, + }, + }; + } + safeMapPath = sanitized; + } + return await this.handleMapScope(astService, safeMapPath); } - if (!this.params.file_path) { + if (!filePath) { return { llmContent: 'Error: file_path is required for "symbol" and "outline" scopes.', @@ -73,10 +100,7 @@ class ASTSearchInvocation extends BaseToolInvocation< } // Validate path stays within workspace boundaries - const sanitizedPath = resolveDefensiveToolPath( - this.params.file_path, - targetDir, - ); + const sanitizedPath = resolveDefensiveToolPath(filePath, targetDir); let resolvedPath: string; try { resolvedPath = resolveToRealPath( @@ -106,14 +130,18 @@ class ASTSearchInvocation extends BaseToolInvocation< } // Default: symbol scope - if (!this.params.symbol_name) { + if (!symbolName) { return { llmContent: 'Error: symbol_name is required for "symbol" scope.', returnDisplay: 'Missing symbol_name', }; } - return await this.handleSymbolScope(astService, sanitizedPath); + return await this.handleSymbolScope( + astService, + sanitizedPath, + symbolName, + ); } catch (err) { const msg = err instanceof Error ? err.message : String(err); debugLogger.warn('[ASTSearchTool] Error:', msg); @@ -127,16 +155,14 @@ class ASTSearchInvocation extends BaseToolInvocation< private async handleSymbolScope( astService: ASTAnalysisService, safePath: string, + symbolName: string, ): Promise { - const bounds = await astService.findSymbolBounds( - safePath, - this.params.symbol_name!, - ); + const bounds = await astService.findSymbolBounds(safePath, symbolName); if (!bounds) { return { llmContent: - `Symbol "${this.params.symbol_name}" not found in ${safePath}. ` + + `Symbol "${symbolName}" not found in ${safePath}. ` + 'Try using grep_search for a text-based search, or check the symbol name spelling.', returnDisplay: 'Symbol not found', }; @@ -145,10 +171,10 @@ class ASTSearchInvocation extends BaseToolInvocation< const outline = await astService.getFileOutline(safePath); const symbol = outline?.symbols .flatMap((s) => [s, ...s.children]) - .find((s) => s.name === this.params.symbol_name); + .find((s) => s.name === symbolName); const result = [ - `Found "${this.params.symbol_name}" in ${safePath}:`, + `Found "${symbolName}" in ${safePath}:`, ` Lines: ${bounds.startLine}-${bounds.endLine} (${bounds.endLine - bounds.startLine + 1} lines)`, symbol ? ` Kind: ${symbol.kind}` : '', symbol ? ` Signature: ${symbol.signature}` : '', @@ -160,7 +186,7 @@ class ASTSearchInvocation extends BaseToolInvocation< return { llmContent: result, - returnDisplay: `${this.params.symbol_name}: L${bounds.startLine}-${bounds.endLine}`, + returnDisplay: `${symbolName}: L${bounds.startLine}-${bounds.endLine}`, display: { name: AST_SEARCH_DISPLAY_NAME, description: this.getDescription(), @@ -197,8 +223,9 @@ class ASTSearchInvocation extends BaseToolInvocation< private async handleMapScope( astService: ASTAnalysisService, + safePath?: string, ): Promise { - const map = await astService.getCodebaseMap(this.params.file_path); + const map = await astService.getCodebaseMap(safePath); return { llmContent: map, returnDisplay: 'Codebase map generated', @@ -259,24 +286,23 @@ export class ASTSearchTool extends BaseDeclarativeTool< params: ASTSearchToolParams, ): string | null { const scope = params.scope ?? 'symbol'; + const symbolName = params.symbol_name?.trim(); + const filePath = params.file_path?.trim(); - if ( - scope === 'symbol' && - (!params.symbol_name || params.symbol_name.trim() === '') - ) { + if (scope === 'symbol' && (!symbolName || symbolName === '')) { return "The 'symbol_name' parameter must be non-empty when scope is 'symbol'."; } if ( (scope === 'symbol' || scope === 'outline') && - (!params.file_path || params.file_path.trim() === '') + (!filePath || filePath === '') ) { return "The 'file_path' parameter must be non-empty for 'symbol' and 'outline' scopes."; } - if (params.file_path) { + if (filePath) { const sanitizedPath = resolveDefensiveToolPath( - params.file_path, + filePath, this.config.getTargetDir(), ); let resolvedPath: string; From c78c03cbd9c04aef2a560a2ed46d8e0eb0db0183 Mon Sep 17 00:00:00 2001 From: dylanyunlon Date: Fri, 18 Sep 2026 14:58:11 +0000 Subject: [PATCH 05/23] fix: address round-4 gemini-code-assist review HIGH (#1): extractSymbols now passes cleaned (block-comment-stripped) lines to findIndentEnd, findClosingBrace, and extractMembers instead of raw lines. Prevents commented-out braces or members from corrupting boundary detection. HIGH (#2): findClosingBrace fallback changed from lines.length-1 to startLine. When brace matching fails (template literals, regex literals, syntax errors), returning startLine localizes the failure to one symbol instead of skipping every subsequent declaration in the file. --- packages/core/src/services/astAnalysisService.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/core/src/services/astAnalysisService.ts b/packages/core/src/services/astAnalysisService.ts index ffdaa5e078d..35375ea53f0 100644 --- a/packages/core/src/services/astAnalysisService.ts +++ b/packages/core/src/services/astAnalysisService.ts @@ -163,8 +163,8 @@ export function extractSymbols(lines: string[], language: string): ASTSymbol[] { const endLine = language === 'python' - ? findIndentEnd(lines, i) - : findClosingBrace(lines, i); + ? findIndentEnd(cleaned, i) + : findClosingBrace(cleaned, i); const sym: ASTSymbol = { name: match[1], @@ -175,7 +175,7 @@ export function extractSymbols(lines: string[], language: string): ASTSymbol[] { trimmed.length > 120 ? trimmed.slice(0, 117) + '...' : trimmed, children: kind === 'class' || kind === 'interface' - ? extractMembers(lines, i + 1, endLine, language) + ? extractMembers(cleaned, i + 1, endLine, language) : [], }; @@ -256,8 +256,9 @@ export function findClosingBrace(lines: string[], startLine: number): number { } } } - // If brace matching failed, return end of file rather than an arbitrary offset - return lines.length - 1; + // If brace matching failed, return startLine rather than the end of the file + // to prevent skipping the entire rest of the file during parsing. + return startLine; } export function findIndentEnd(lines: string[], startLine: number): number { From 1fa514df17c3782e43881af505a40217542b54cf Mon Sep 17 00:00:00 2001 From: dylanyunlon Date: Fri, 18 Sep 2026 15:46:49 +0000 Subject: [PATCH 06/23] fix: address round-5 gemini-code-assist review HIGH (#1): stripBlockComments rewritten with full state-tracking parser. Now correctly ignores block comment delimiters inside string literals (single/double/template quotes with escape handling) and single-line comments (// ...). Prevents false positives like const x = "/*" from triggering comment stripping. HIGH (#2): Top-level indentation limit for brace-based languages raised from 2 to 8 spaces. Supports 4-space indented codebases and symbols nested inside namespaces/modules without being silently skipped. --- .../core/src/services/astAnalysisService.ts | 45 +++++++++++++++++-- 1 file changed, 41 insertions(+), 4 deletions(-) diff --git a/packages/core/src/services/astAnalysisService.ts b/packages/core/src/services/astAnalysisService.ts index 35375ea53f0..29fed989580 100644 --- a/packages/core/src/services/astAnalysisService.ts +++ b/packages/core/src/services/astAnalysisService.ts @@ -152,9 +152,10 @@ export function extractSymbols(lines: string[], language: string): ASTSymbol[] { continue; } - // Only match at top-level indentation (<=2 spaces for brace langs) - const indent = lines[i].length - lines[i].trimStart().length; - if (language !== 'python' && indent > 2) continue; + // Only match at top-level indentation (<=8 spaces for brace langs to support + // 4-space indented codebases and namespace/module nesting) + const indent = cleaned[i].length - cleaned[i].trimStart().length; + if (language !== 'python' && indent > 8) continue; if (language === 'python' && indent > 0) continue; for (const { regex, kind } of patterns) { @@ -202,8 +203,13 @@ export function stripBlockComments(lines: string[]): string[] { for (const line of lines) { let out = ''; let j = 0; + // Track whether we are inside a string or single-line comment on this line. + // Delimiters inside strings or // comments must not toggle block comment state. + let inString: string | null = null; // tracks quote char: ' " or ` + let inLineComment = false; while (j < line.length) { if (inComment) { + // Inside a block comment: look for */ if (j + 1 < line.length && line[j] === '*' && line[j + 1] === '/') { out += ' '; j += 2; @@ -212,11 +218,42 @@ export function stripBlockComments(lines: string[]): string[] { out += ' '; j++; } + } else if (inLineComment) { + // Rest of line is a single-line comment, emit as-is + out += line[j]; + j++; + } else if (inString) { + // Inside a string literal: look for closing quote (skip escaped) + if (line[j] === '\\' && j + 1 < line.length) { + out += line[j] + line[j + 1]; + j += 2; + } else if (line[j] === inString) { + out += line[j]; + j++; + inString = null; + } else { + out += line[j]; + j++; + } } else { - if (j + 1 < line.length && line[j] === '/' && line[j + 1] === '*') { + // Normal code context + if (j + 1 < line.length && line[j] === '/' && line[j + 1] === '/') { + // Single-line comment start: rest of line is not a block comment + inLineComment = true; + out += line[j]; + j++; + } else if ( + j + 1 < line.length && + line[j] === '/' && + line[j + 1] === '*' + ) { out += ' '; j += 2; inComment = true; + } else if (line[j] === "'" || line[j] === '"' || line[j] === '`') { + inString = line[j]; + out += line[j]; + j++; } else { out += line[j]; j++; From 7006078727a954e95d0dda2de6bae726695b0fd6 Mon Sep 17 00:00:00 2001 From: dylanyunlon Date: Fri, 18 Sep 2026 16:00:27 +0000 Subject: [PATCH 07/23] fix: address round-6 gemini-code-assist review HIGH (#1): String literal regex in findClosingBrace now handles escaped quotes via negated character class with backslash alternation: /(TICK)(?:[^(TICK)\\]|\\.)*TICK/g pattern for all three quote types. Resolves the documented limitation for strings like "a \" {". HIGH (#2): Triple-quote tracking in findIndentEnd rewritten to track the specific opener (""" vs TICK TICK TICK). A block started with """ can only be closed by """, preventing cross-type toggle bugs. HIGH (#3): Directory walk depth limit raised from 6 to 15. Supports deep monorepo structures like packages/core/src/tools/definitions/ model-family-sets/ (depth 7) without silently omitting files. --- .../core/src/services/astAnalysisService.ts | 40 ++++++++++++++----- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/packages/core/src/services/astAnalysisService.ts b/packages/core/src/services/astAnalysisService.ts index 29fed989580..21023134c60 100644 --- a/packages/core/src/services/astAnalysisService.ts +++ b/packages/core/src/services/astAnalysisService.ts @@ -270,10 +270,11 @@ export function findClosingBrace(lines: string[], startLine: number): number { let parenDepth = 0; let opened = false; for (let i = startLine; i < lines.length; i++) { - // Strip string literals to avoid false brace matches. - // NOTE: This regex does not handle escaped quotes inside strings - // (e.g. "a \" {"). This is a known limitation of the heuristic parser. - let stripped = lines[i].replace(/'[^']*'|"[^"]*"|`[^`]*`/g, ''); + // Strip string literals (including escaped quotes) to avoid false brace matches. + let stripped = lines[i].replace( + /'(?:[^'\\]|\\.)*'|"(?:[^"\\]|\\.)*"|`(?:[^`\\]|\\.)*`/g, + '', + ); // Strip single-line comments which may contain braces (e.g. "// }") const commentIdx = stripped.indexOf('//'); if (commentIdx >= 0) { @@ -302,16 +303,33 @@ export function findIndentEnd(lines: string[], startLine: number): number { const baseIndent = lines[startLine].length - lines[startLine].trimStart().length; let last = startLine; - let inTripleQuote = false; + let tripleQuoteChar: '"""' | "'''" | null = null; for (let i = startLine + 1; i < lines.length; i++) { const trimmed = lines[i].trim(); - // Track Python triple-quoted strings which can have arbitrary indentation - const tripleCount = (trimmed.match(/"""|'''/g) || []).length; - if (tripleCount % 2 !== 0) { - inTripleQuote = !inTripleQuote; + // Track Python triple-quoted strings - must match the same quote type + let j = 0; + while (j < trimmed.length) { + if (tripleQuoteChar) { + if (trimmed.startsWith(tripleQuoteChar, j)) { + tripleQuoteChar = null; + j += 3; + } else { + j++; + } + } else { + if (trimmed.startsWith('"""', j)) { + tripleQuoteChar = '"""'; + j += 3; + } else if (trimmed.startsWith("'''", j)) { + tripleQuoteChar = "'''"; + j += 3; + } else { + j++; + } + } } - if (inTripleQuote) { + if (tripleQuoteChar) { last = i; continue; } @@ -499,7 +517,7 @@ function countSymbols(syms: ASTSymbol[]): number { async function collectSourceFiles(dir: string, max: number): Promise { const files: string[] = []; async function walk(d: string, depth: number) { - if (depth > 6 || files.length >= max) return; + if (depth > 15 || files.length >= max) return; let entries; try { entries = await fs.readdir(d, { withFileTypes: true }); From 8adea18a48294a18e4c22377add21d23fd3e5916 Mon Sep 17 00:00:00 2001 From: dylanyunlon Date: Fri, 18 Sep 2026 16:11:55 +0000 Subject: [PATCH 08/23] fix: address round-7 gemini-code-assist review CRITICAL (#1): getFileOutline now validates resolved path is a subpath of targetDir before any fs access. Traversal returns null. CRITICAL (#2): getCodebaseMap validates searchDir stays within targetDir. Traversal returns an error string instead of walking arbitrary dirs. HIGH (#3,#4): stripBlockComments now accepts language parameter. Uses # for Python line comments, // for others. String literal contents are blanked (replaced with spaces) while preserving delimiters, preventing false keyword matches inside strings and incorrect brace counting from template literal contents. --- .../core/src/services/astAnalysisService.ts | 62 +++++++++++++------ 1 file changed, 44 insertions(+), 18 deletions(-) diff --git a/packages/core/src/services/astAnalysisService.ts b/packages/core/src/services/astAnalysisService.ts index 21023134c60..72bae1c34d3 100644 --- a/packages/core/src/services/astAnalysisService.ts +++ b/packages/core/src/services/astAnalysisService.ts @@ -63,7 +63,15 @@ export class ASTAnalysisService { * Returns the structural outline of a single source file. */ async getFileOutline(filePath: string): Promise { - const absPath = path.resolve(this.targetDir, filePath); + const resolvedTargetDir = path.resolve(this.targetDir); + const absPath = path.resolve(resolvedTargetDir, filePath); + + // Guard against path traversal + const relative = path.relative(resolvedTargetDir, absPath); + if (relative.startsWith('..') || path.isAbsolute(relative)) { + return null; + } + const ext = path.extname(absPath); const language = LANG_MAP[ext]; if (!language) return null; @@ -105,9 +113,16 @@ export class ASTAnalysisService { subDir?: string, maxFiles: number = 100, ): Promise { + const resolvedTargetDir = path.resolve(this.targetDir); const searchDir = subDir - ? path.resolve(this.targetDir, subDir) - : this.targetDir; + ? path.resolve(resolvedTargetDir, subDir) + : resolvedTargetDir; + + // Guard against path traversal + const relative = path.relative(resolvedTargetDir, searchDir); + if (relative.startsWith('..') || path.isAbsolute(relative)) { + return 'Error: path traversal detected, directory must be within workspace.'; + } const files = await collectSourceFiles(searchDir, maxFiles); const sections: string[] = []; @@ -136,7 +151,7 @@ export function extractSymbols(lines: string[], language: string): ASTSymbol[] { // Pre-process: strip block comments by replacing their content with spaces. // This handles mid-line comments like `code /* comment */ more_code` and // multi-line blocks, while preserving line numbers and offsets. - const cleaned = stripBlockComments(lines); + const cleaned = stripBlockComments(lines, language); for (let i = 0; i < cleaned.length; i++) { const trimmed = cleaned[i].trim(); @@ -197,19 +212,22 @@ export function extractSymbols(lines: string[], language: string): ASTSymbol[] { * so that line-number-based logic (brace counting, indentation) stays correct. * Handles mid-line comments, multi-line blocks, and lines with code after a comment. */ -export function stripBlockComments(lines: string[]): string[] { +export function stripBlockComments( + lines: string[], + language?: string, +): string[] { const result: string[] = []; let inComment = false; + let inString: string | null = null; + const isPython = language === 'python'; + for (const line of lines) { let out = ''; let j = 0; - // Track whether we are inside a string or single-line comment on this line. - // Delimiters inside strings or // comments must not toggle block comment state. - let inString: string | null = null; // tracks quote char: ' " or ` let inLineComment = false; + while (j < line.length) { if (inComment) { - // Inside a block comment: look for */ if (j + 1 < line.length && line[j] === '*' && line[j + 1] === '/') { out += ' '; j += 2; @@ -219,30 +237,38 @@ export function stripBlockComments(lines: string[]): string[] { j++; } } else if (inLineComment) { - // Rest of line is a single-line comment, emit as-is out += line[j]; j++; } else if (inString) { - // Inside a string literal: look for closing quote (skip escaped) + // Inside string: blank contents but preserve delimiters if (line[j] === '\\' && j + 1 < line.length) { - out += line[j] + line[j + 1]; + out += ' '; j += 2; } else if (line[j] === inString) { out += line[j]; - j++; inString = null; + j++; } else { - out += line[j]; + out += ' '; j++; } } else { - // Normal code context - if (j + 1 < line.length && line[j] === '/' && line[j + 1] === '/') { - // Single-line comment start: rest of line is not a block comment + // Normal code context - language-aware comment detection + if ( + !isPython && + j + 1 < line.length && + line[j] === '/' && + line[j + 1] === '/' + ) { inLineComment = true; - out += line[j]; + out += '//'; + j += 2; + } else if (isPython && line[j] === '#') { + inLineComment = true; + out += '#'; j++; } else if ( + !isPython && j + 1 < line.length && line[j] === '/' && line[j + 1] === '*' From 193cf2c5bc595a7d2e9f28e095d58aa586175c1a Mon Sep 17 00:00:00 2001 From: dylanyunlon Date: Fri, 18 Sep 2026 16:30:24 +0000 Subject: [PATCH 09/23] fix: address round-8 gemini-code-assist review + own testing fixes Review fixes: - HIGH (#1): Python member regex removed leading \s+ since trimmed lines have no leading whitespace. Methods now extracted correctly. - HIGH (#2): getCodebaseMap uses resolvedTargetDir for path.relative instead of raw this.targetDir, preventing incorrect relative paths. - HIGH (#3): Python test expanded to assert class method children (Handler.run as method child). Bugs found via thorough manual testing: - extractSymbols indent check now uses original lines[i] instead of cleaned[i]. Block comment blanking inflates indentation of the cleaned line (e.g. "/* comment */ export class A" becomes 19 spaces of indent), causing false top-level rejection. --- packages/core/src/services/astAnalysisService.test.ts | 3 +++ packages/core/src/services/astAnalysisService.ts | 10 ++++++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/core/src/services/astAnalysisService.test.ts b/packages/core/src/services/astAnalysisService.test.ts index 6da4cf7d0ea..b3a9ee6e991 100644 --- a/packages/core/src/services/astAnalysisService.test.ts +++ b/packages/core/src/services/astAnalysisService.test.ts @@ -124,6 +124,9 @@ describe('ASTAnalysisService', () => { expect(syms).toHaveLength(2); expect(syms[0].name).toBe('Handler'); expect(syms[0].kind).toBe('class'); + expect(syms[0].children).toHaveLength(1); + expect(syms[0].children[0].name).toBe('run'); + expect(syms[0].children[0].kind).toBe('method'); expect(syms[1].name).toBe('util'); }); diff --git a/packages/core/src/services/astAnalysisService.ts b/packages/core/src/services/astAnalysisService.ts index 72bae1c34d3..2541ac2afd9 100644 --- a/packages/core/src/services/astAnalysisService.ts +++ b/packages/core/src/services/astAnalysisService.ts @@ -129,7 +129,7 @@ export class ASTAnalysisService { let totalSymbols = 0; for (const file of files) { - const relPath = path.relative(this.targetDir, file); + const relPath = path.relative(resolvedTargetDir, file); const outline = await this.getFileOutline(relPath); if (!outline || outline.symbols.length === 0) continue; @@ -168,8 +168,10 @@ export function extractSymbols(lines: string[], language: string): ASTSymbol[] { } // Only match at top-level indentation (<=8 spaces for brace langs to support - // 4-space indented codebases and namespace/module nesting) - const indent = cleaned[i].length - cleaned[i].trimStart().length; + // 4-space indented codebases and namespace/module nesting). + // Use original lines for indent check since cleaned lines may have inflated + // indentation from blanked block comments. + const indent = lines[i].length - lines[i].trimStart().length; if (language !== 'python' && indent > 8) continue; if (language === 'python' && indent > 0) continue; @@ -487,7 +489,7 @@ function getMemberPatterns(lang: string) { p.push({ regex: /^(\w+)\s*\(/, kind: 'method' }); break; case 'python': - p.push({ regex: /^\s+(?:async\s+)?def\s+(\w+)/, kind: 'method' }); + p.push({ regex: /^(?:async\s+)?def\s+(\w+)/, kind: 'method' }); break; case 'go': p.push({ regex: /func\s+\([^)]+\)\s+(\w+)/, kind: 'method' }); From 30f295be37ad98451be8d54a1dcd9aa046c523cf Mon Sep 17 00:00:00 2001 From: dylanyunlon Date: Fri, 18 Sep 2026 16:48:18 +0000 Subject: [PATCH 10/23] fix: address round-9 gemini-code-assist review HIGH: stripBlockComments now handles Python triple-quoted strings (docstrings). Checks for """ and triple-single-quote openers before single-char quote matching. inString stores the full delimiter (1 or 3 chars) so closing matches correctly. Prevents a docstring containing an odd number of internal quotes from inverting the string state and stripping subsequent valid code. --- packages/core/src/services/astAnalysisService.ts | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/packages/core/src/services/astAnalysisService.ts b/packages/core/src/services/astAnalysisService.ts index 2541ac2afd9..782924b9023 100644 --- a/packages/core/src/services/astAnalysisService.ts +++ b/packages/core/src/services/astAnalysisService.ts @@ -246,7 +246,12 @@ export function stripBlockComments( if (line[j] === '\\' && j + 1 < line.length) { out += ' '; j += 2; - } else if (line[j] === inString) { + } else if (inString.length === 3 && line.startsWith(inString, j)) { + // Closing triple-quote + out += inString; + inString = null; + j += 3; + } else if (inString.length === 1 && line[j] === inString) { out += line[j]; inString = null; j++; @@ -278,6 +283,14 @@ export function stripBlockComments( out += ' '; j += 2; inComment = true; + } else if ( + isPython && + (line.startsWith('"""', j) || line.startsWith("'''", j)) + ) { + // Python triple-quoted string + inString = line.startsWith('"""', j) ? '"""' : "'''"; + out += inString; + j += 3; } else if (line[j] === "'" || line[j] === '"' || line[j] === '`') { inString = line[j]; out += line[j]; From ef6fe4195c5069f380ee12b2e9b3d8f9dddadac9 Mon Sep 17 00:00:00 2001 From: dylanyunlon Date: Fri, 18 Sep 2026 16:56:34 +0000 Subject: [PATCH 11/23] fix: address round-10 gemini-code-assist review HIGH (#1): getFileOutline now checks file size via fs.stat before reading. Files over 2MB (minified bundles, logs, db dumps) are rejected with null return, preventing OOM or event loop blocking. HIGH (#2): stripBlockComments resets single-char inString (single and double quotes) at end of each line. An unclosed quote from a syntax error or unescaped character no longer bleeds into subsequent lines and corrupts the rest of the file. Multi-line delimiters (backtick, triple quotes) intentionally persist across lines. --- .../core/src/services/astAnalysisService.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/packages/core/src/services/astAnalysisService.ts b/packages/core/src/services/astAnalysisService.ts index 782924b9023..ac6e19a0739 100644 --- a/packages/core/src/services/astAnalysisService.ts +++ b/packages/core/src/services/astAnalysisService.ts @@ -76,6 +76,16 @@ export class ASTAnalysisService { const language = LANG_MAP[ext]; if (!language) return null; + // Reject files larger than 2MB to prevent OOM or event loop blocking + // on minified bundles, logs, or database dumps. + const MAX_FILE_SIZE = 2 * 1024 * 1024; + try { + const stat = await fs.stat(absPath); + if (stat.size > MAX_FILE_SIZE) return null; + } catch { + return null; + } + let content: string; try { content = await fs.readFile(absPath, 'utf-8'); @@ -301,6 +311,13 @@ export function stripBlockComments( } } } + // Reset single-line string state at end of line. If a single or double + // quote was not closed (syntax error, unescaped quote, regex), don't let + // it bleed into subsequent lines. Multi-line delimiters (backtick, triple + // quotes) intentionally persist across lines. + if (inString && inString.length === 1 && inString !== '`') { + inString = null; + } result.push(out); } return result; From fdbbed11e4689b4ed350c0ff4481f3338ea7f496 Mon Sep 17 00:00:00 2001 From: dylanyunlon Date: Fri, 18 Sep 2026 23:20:50 +0000 Subject: [PATCH 12/23] fix: address round-11 gemini-code-assist review HIGH (#1): Single-line comment text now blanked with spaces in stripBlockComments (only // or # prefix preserved). Prevents false declaration matches from keywords inside trailing comments like "const x = 1; // class MyClass". HIGH (#2): handleSymbolScope uses recursive findSymbolRecursive helper instead of shallow flatMap. Correctly locates symbols nested more than 1 level deep (methods inside nested classes, symbols inside namespace/module blocks). --- packages/core/src/services/astAnalysisService.ts | 2 +- packages/core/src/tools/ast-search.ts | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/packages/core/src/services/astAnalysisService.ts b/packages/core/src/services/astAnalysisService.ts index ac6e19a0739..059b532895d 100644 --- a/packages/core/src/services/astAnalysisService.ts +++ b/packages/core/src/services/astAnalysisService.ts @@ -249,7 +249,7 @@ export function stripBlockComments( j++; } } else if (inLineComment) { - out += line[j]; + out += ' '; j++; } else if (inString) { // Inside string: blank contents but preserve delimiters diff --git a/packages/core/src/tools/ast-search.ts b/packages/core/src/tools/ast-search.ts index 0f86f0cbe97..72c28eaedb4 100644 --- a/packages/core/src/tools/ast-search.ts +++ b/packages/core/src/tools/ast-search.ts @@ -23,6 +23,7 @@ import { resolveToolDeclaration } from './definitions/resolver.js'; import { ASTAnalysisService, type ASTFileOutline, + type ASTSymbol, } from '../services/astAnalysisService.js'; import { debugLogger } from '../utils/debugLogger.js'; @@ -169,9 +170,17 @@ class ASTSearchInvocation extends BaseToolInvocation< } const outline = await astService.getFileOutline(safePath); - const symbol = outline?.symbols - .flatMap((s) => [s, ...s.children]) - .find((s) => s.name === symbolName); + const findSymbolRecursive = ( + symbols: ASTSymbol[], + ): ASTSymbol | undefined => { + for (const s of symbols) { + if (s.name === symbolName) return s; + const found = findSymbolRecursive(s.children); + if (found) return found; + } + return undefined; + }; + const symbol = outline ? findSymbolRecursive(outline.symbols) : undefined; const result = [ `Found "${symbolName}" in ${safePath}:`, From a9a36c461b7ebe198f8794bb779b9cfbaabf67b6 Mon Sep 17 00:00:00 2001 From: dylanyunlon Date: Sat, 19 Sep 2026 01:42:54 +0000 Subject: [PATCH 13/23] fix: address round-12 review - deterministic file collection HIGH: collectSourceFiles now collects ALL matching paths first, sorts them, then truncates to max. Previously it stopped walking directories as soon as max was reached, producing an OS-dependent, non-deterministic subset because fs.readdir order varies across platforms and runs. Now codebase maps are identical across Linux/macOS/Windows for the same workspace. --- packages/core/src/services/astAnalysisService.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/core/src/services/astAnalysisService.ts b/packages/core/src/services/astAnalysisService.ts index 059b532895d..885160c61ce 100644 --- a/packages/core/src/services/astAnalysisService.ts +++ b/packages/core/src/services/astAnalysisService.ts @@ -573,9 +573,12 @@ function countSymbols(syms: ASTSymbol[]): number { * in a follow-up PR. */ async function collectSourceFiles(dir: string, max: number): Promise { + // Collect ALL matching files first, then sort and truncate. + // This ensures a deterministic, platform-independent subset when the workspace + // contains more files than `max`, since fs.readdir order is OS-dependent. const files: string[] = []; async function walk(d: string, depth: number) { - if (depth > 15 || files.length >= max) return; + if (depth > 15) return; let entries; try { entries = await fs.readdir(d, { withFileTypes: true }); @@ -583,7 +586,6 @@ async function collectSourceFiles(dir: string, max: number): Promise { return; } for (const e of entries) { - if (files.length >= max) return; const full = path.join(d, e.name); if ( e.isDirectory() && @@ -597,5 +599,5 @@ async function collectSourceFiles(dir: string, max: number): Promise { } } await walk(dir, 0); - return files.sort(); + return files.sort().slice(0, max); } From 0d1c5ad50050938cdbdde117a3e1ba87cf5a13ce Mon Sep 17 00:00:00 2001 From: dylanyunlon Date: Sat, 19 Sep 2026 02:12:43 +0000 Subject: [PATCH 14/23] fix: address round-13 review - regex literals + collection optimization HIGH (#1): findClosingBrace now strips regex literals alongside string literals. Patterns like /[{}]/ and /a{1,3}/ no longer corrupt brace depth tracking. Added regex alternation to the existing strip regex. HIGH (#2): collectSourceFiles optimized for large repos. Entries sorted alphabetically at each directory level for deterministic traversal. Hard cap of 10,000 files prevents OOM in huge monorepos. Per-level sorting means the walk produces near-sorted output, reducing final sort overhead. --- .../core/src/services/astAnalysisService.ts | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/packages/core/src/services/astAnalysisService.ts b/packages/core/src/services/astAnalysisService.ts index 885160c61ce..c3083c523d3 100644 --- a/packages/core/src/services/astAnalysisService.ts +++ b/packages/core/src/services/astAnalysisService.ts @@ -328,9 +328,10 @@ export function findClosingBrace(lines: string[], startLine: number): number { let parenDepth = 0; let opened = false; for (let i = startLine; i < lines.length; i++) { - // Strip string literals (including escaped quotes) to avoid false brace matches. + // Strip string literals (including escaped quotes) and regex literals + // to avoid false brace matches from patterns like /[{}]/ or /a{1,3}/. let stripped = lines[i].replace( - /'(?:[^'\\]|\\.)*'|"(?:[^"\\]|\\.)*"|`(?:[^`\\]|\\.)*`/g, + /'(?:[^'\\]|\\.)*'|"(?:[^"\\]|\\.)*"|`(?:[^`\\]|\\.)*`|\/(?:[^/\\]|\\.)+\//g, '', ); // Strip single-line comments which may contain braces (e.g. "// }") @@ -573,19 +574,23 @@ function countSymbols(syms: ASTSymbol[]): number { * in a follow-up PR. */ async function collectSourceFiles(dir: string, max: number): Promise { - // Collect ALL matching files first, then sort and truncate. - // This ensures a deterministic, platform-independent subset when the workspace - // contains more files than `max`, since fs.readdir order is OS-dependent. + // Collect matching files with a hard cap to prevent OOM in huge repos. + // Entries are sorted at each directory level for deterministic output + // regardless of OS readdir order, then the final list is sorted and sliced. + const HARD_CAP = 10_000; const files: string[] = []; async function walk(d: string, depth: number) { - if (depth > 15) return; + if (depth > 15 || files.length >= HARD_CAP) return; let entries; try { entries = await fs.readdir(d, { withFileTypes: true }); } catch { return; } + // Sort entries alphabetically for deterministic traversal order + entries.sort((a, b) => a.name.localeCompare(b.name)); for (const e of entries) { + if (files.length >= HARD_CAP) return; const full = path.join(d, e.name); if ( e.isDirectory() && @@ -599,5 +604,7 @@ async function collectSourceFiles(dir: string, max: number): Promise { } } await walk(dir, 0); + // Already mostly sorted due to per-directory sort; final sort ensures + // cross-directory ordering, then slice to requested max. return files.sort().slice(0, max); } From 31df4f2ec72d2bb76221505a83f520b45313e1fb Mon Sep 17 00:00:00 2001 From: dylanyunlon Date: Sat, 19 Sep 2026 03:25:05 +0000 Subject: [PATCH 15/23] fix(security): enforce .gitignore/.geminiignore patterns in ast_search tool and ASTAnalysisService MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses all 9 review comments from gemini-code-assist round-14/15: Security (HIGH × 9): - ASTAnalysisService: accept optional shouldIgnore callback in constructor to enforce ignore patterns during file outlining and codebase mapping - getFileOutline: reject files matching shouldIgnore before reading content, preventing information leakage from ignored/private configuration files - getCodebaseMap: pass shouldIgnore to collectSourceFiles so ignored files are excluded from the structural codebase map - collectSourceFiles: accept and propagate shouldIgnore through recursive walk, checking each file and directory before inclusion - ASTSearchTool: instantiate FileDiscoveryService (matching ReadFileTool pattern) and construct shouldIgnore backed by it - ASTSearchInvocation.execute: pass shouldIgnore to ASTAnalysisService so outline/map operations respect workspace ignore rules - validateToolParamValues: check file_path against shouldIgnoreFile for both sanitized and resolved paths, returning an error message matching the ReadFileTool format when a file is ignored Testing: - 4 new unit tests for shouldIgnore callback behavior: * getFileOutline returns null for ignored files * getFileOutline works normally for non-ignored files * getCodebaseMap excludes ignored files from output * findSymbolBounds returns null for ignored files Call chain: ASTSearchTool.constructor → FileDiscoveryService(targetDir, filterOpts) ASTSearchTool.validateToolParamValues → fileDiscoveryService.shouldIgnoreFile ASTSearchInvocation.execute → FileDiscoveryService → shouldIgnore closure → ASTAnalysisService(targetDir, shouldIgnore) → getFileOutline checks shouldIgnore(filePath) || shouldIgnore(absPath) → getCodebaseMap → collectSourceFiles(dir, max, shouldIgnore) → walk(d, depth, shouldIgnore) checks shouldIgnore(full) per entry --- .../src/services/astAnalysisService.test.ts | 50 +++++++++++++++++++ .../core/src/services/astAnalysisService.ts | 49 +++++++++++++----- packages/core/src/tools/ast-search.ts | 36 ++++++++++++- 3 files changed, 121 insertions(+), 14 deletions(-) diff --git a/packages/core/src/services/astAnalysisService.test.ts b/packages/core/src/services/astAnalysisService.test.ts index b3a9ee6e991..045c5967096 100644 --- a/packages/core/src/services/astAnalysisService.test.ts +++ b/packages/core/src/services/astAnalysisService.test.ts @@ -242,4 +242,54 @@ describe('ASTAnalysisService', () => { expect(map).not.toContain('node_modules'); }); }); + + describe('shouldIgnore callback', () => { + it('getFileOutline should return null for ignored files', async () => { + await fs.writeFile( + path.join(tmpDir, 'secret.ts'), + 'export class Secret {}\n', + ); + const ignoreFn = (p: string) => p.includes('secret'); + const svc = new ASTAnalysisService(tmpDir, ignoreFn); + expect(await svc.getFileOutline('secret.ts')).toBeNull(); + }); + + it('getFileOutline should still work for non-ignored files', async () => { + await fs.writeFile( + path.join(tmpDir, 'public.ts'), + 'export class Public {}\n', + ); + const ignoreFn = (p: string) => p.includes('secret'); + const svc = new ASTAnalysisService(tmpDir, ignoreFn); + const outline = await svc.getFileOutline('public.ts'); + expect(outline).not.toBeNull(); + expect(outline!.symbols[0].name).toBe('Public'); + }); + + it('getCodebaseMap should exclude ignored files', async () => { + await fs.writeFile( + path.join(tmpDir, 'visible.ts'), + 'export class Visible {}\n', + ); + await fs.writeFile( + path.join(tmpDir, 'hidden.ts'), + 'export class Hidden {}\n', + ); + const ignoreFn = (p: string) => p.includes('hidden'); + const svc = new ASTAnalysisService(tmpDir, ignoreFn); + const map = await svc.getCodebaseMap(); + expect(map).toContain('Visible'); + expect(map).not.toContain('Hidden'); + }); + + it('findSymbolBounds should return null for ignored files', async () => { + await fs.writeFile( + path.join(tmpDir, 'ignored.ts'), + 'export function target() { return 1; }\n', + ); + const ignoreFn = (p: string) => p.includes('ignored'); + const svc = new ASTAnalysisService(tmpDir, ignoreFn); + expect(await svc.findSymbolBounds('ignored.ts', 'target')).toBeNull(); + }); + }); }); diff --git a/packages/core/src/services/astAnalysisService.ts b/packages/core/src/services/astAnalysisService.ts index c3083c523d3..ac224b7b3cd 100644 --- a/packages/core/src/services/astAnalysisService.ts +++ b/packages/core/src/services/astAnalysisService.ts @@ -57,7 +57,10 @@ const SKIP_DIRS = new Set([ * 3. Compressed codebase mapping (class/function/signature outlines) */ export class ASTAnalysisService { - constructor(private readonly targetDir: string) {} + constructor( + private readonly targetDir: string, + private readonly shouldIgnore?: (filePath: string) => boolean, + ) {} /** * Returns the structural outline of a single source file. @@ -72,6 +75,14 @@ export class ASTAnalysisService { return null; } + // Enforce ignore patterns (e.g. .gitignore, .geminiignore) when provided + if ( + this.shouldIgnore && + (this.shouldIgnore(filePath) || this.shouldIgnore(absPath)) + ) { + return null; + } + const ext = path.extname(absPath); const language = LANG_MAP[ext]; if (!language) return null; @@ -134,7 +145,11 @@ export class ASTAnalysisService { return 'Error: path traversal detected, directory must be within workspace.'; } - const files = await collectSourceFiles(searchDir, maxFiles); + const files = await collectSourceFiles( + searchDir, + maxFiles, + this.shouldIgnore, + ); const sections: string[] = []; let totalSymbols = 0; @@ -565,21 +580,26 @@ function countSymbols(syms: ASTSymbol[]): number { /** * Walks directories to collect source files with known extensions. - * Note: This does not currently respect .gitignore or .geminiignore patterns. - * When called through the `ast_search` tool, path access is validated by the - * tool's validateToolParamValues and the Config.validatePathAccess check, so - * ignored files will not be exposed to the user. For codebase map generation, - * the SKIP_DIRS set covers the most common build/dependency directories. - * Full ignore-pattern integration should be added via FileDiscoveryService - * in a follow-up PR. + * When a `shouldIgnore` callback is provided (backed by FileDiscoveryService), + * .gitignore and .geminiignore patterns are respected during traversal. + * The SKIP_DIRS set provides an additional hard-coded fast path that always + * applies regardless of ignore patterns. */ -async function collectSourceFiles(dir: string, max: number): Promise { +async function collectSourceFiles( + dir: string, + max: number, + shouldIgnore?: (filePath: string) => boolean, +): Promise { // Collect matching files with a hard cap to prevent OOM in huge repos. // Entries are sorted at each directory level for deterministic output // regardless of OS readdir order, then the final list is sorted and sliced. const HARD_CAP = 10_000; const files: string[] = []; - async function walk(d: string, depth: number) { + async function walk( + d: string, + depth: number, + shouldIgnore?: (filePath: string) => boolean, + ) { if (depth > 15 || files.length >= HARD_CAP) return; let entries; try { @@ -592,18 +612,21 @@ async function collectSourceFiles(dir: string, max: number): Promise { for (const e of entries) { if (files.length >= HARD_CAP) return; const full = path.join(d, e.name); + if (shouldIgnore && shouldIgnore(full)) { + continue; + } if ( e.isDirectory() && !SKIP_DIRS.has(e.name) && !e.name.startsWith('.') ) { - await walk(full, depth + 1); + await walk(full, depth + 1, shouldIgnore); } else if (e.isFile() && LANG_MAP[path.extname(e.name)]) { files.push(full); } } } - await walk(dir, 0); + await walk(dir, 0, shouldIgnore); // Already mostly sorted due to per-directory sort; final sort ensures // cross-directory ordering, then slice to requested max. return files.sort().slice(0, max); diff --git a/packages/core/src/tools/ast-search.ts b/packages/core/src/tools/ast-search.ts index 72c28eaedb4..9608195ae46 100644 --- a/packages/core/src/tools/ast-search.ts +++ b/packages/core/src/tools/ast-search.ts @@ -26,6 +26,7 @@ import { type ASTSymbol, } from '../services/astAnalysisService.js'; import { debugLogger } from '../utils/debugLogger.js'; +import { FileDiscoveryService } from '../services/fileDiscoveryService.js'; export interface ASTSearchToolParams { symbol_name?: string; @@ -58,7 +59,20 @@ class ASTSearchInvocation extends BaseToolInvocation< async execute(_options: ExecuteOptions): Promise { const scope = this.params.scope ?? 'symbol'; const targetDir = this.config.getTargetDir(); - const astService = new ASTAnalysisService(targetDir); + + // Build shouldIgnore callback backed by FileDiscoveryService to enforce + // .gitignore and .geminiignore patterns during outline and map operations. + const fileFilteringOptions = this.config.getFileFilteringOptions(); + const fileDiscoveryService = new FileDiscoveryService( + targetDir, + fileFilteringOptions, + ); + const shouldIgnore = (filePath: string): boolean => fileDiscoveryService.shouldIgnoreFile( + filePath, + fileFilteringOptions, + ); + + const astService = new ASTAnalysisService(targetDir, shouldIgnore); try { // Trim params to prevent whitespace-only values and search mismatches @@ -274,6 +288,7 @@ export class ASTSearchTool extends BaseDeclarativeTool< ToolResult > { static readonly Name = AST_SEARCH_TOOL_NAME; + private readonly fileDiscoveryService: FileDiscoveryService; constructor( private config: Config, @@ -289,6 +304,10 @@ export class ASTSearchTool extends BaseDeclarativeTool< true, false, ); + this.fileDiscoveryService = new FileDiscoveryService( + config.getTargetDir(), + config.getFileFilteringOptions(), + ); } protected override validateToolParamValues( @@ -329,6 +348,21 @@ export class ASTSearchTool extends BaseDeclarativeTool< if (validationError) { return validationError; } + + // Enforce .gitignore / .geminiignore patterns, matching ReadFileTool + const fileFilteringOptions = this.config.getFileFilteringOptions(); + if ( + this.fileDiscoveryService.shouldIgnoreFile( + sanitizedPath, + fileFilteringOptions, + ) || + this.fileDiscoveryService.shouldIgnoreFile( + resolvedPath, + fileFilteringOptions, + ) + ) { + return `File path '${resolvedPath}' is ignored by configured ignore patterns.`; + } } return null; From d5304262c37691eec2e08e608bb3ff1453822d2d Mon Sep 17 00:00:00 2001 From: dylanyunlon Date: Sat, 19 Sep 2026 04:09:54 +0000 Subject: [PATCH 16/23] fix(perf): deduplicate file reads in handleSymbolScope by using single getFileOutline call Addresses round-16 gemini-code-assist review (HIGH x 1): handleSymbolScope previously called findSymbolBounds (which internally calls getFileOutline) and then called getFileOutline again to retrieve symbol metadata, resulting in the same file being read from disk and parsed with regex heuristics twice per symbol lookup. Fix: replace the two-call pattern with a single getFileOutline call, then use recursive search on the returned outline to find the target symbol. Both bounds and metadata (kind, signature) are now extracted from the same parse result. Call chain before (2 file reads): handleSymbolScope -> findSymbolBounds -> getFileOutline (read #1) -> getFileOutline (read #2) -> findSymbolRecursive Call chain after (1 file read): handleSymbolScope -> getFileOutline (read #1) -> findSymbolRecursive Runtime verified: gemini-cli headless session confirms ast_search in registered tool list alongside read_file, grep_search, glob, etc. --- packages/core/src/tools/ast-search.ts | 44 ++++++++++++--------------- 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/packages/core/src/tools/ast-search.ts b/packages/core/src/tools/ast-search.ts index 9608195ae46..962c041f81f 100644 --- a/packages/core/src/tools/ast-search.ts +++ b/packages/core/src/tools/ast-search.ts @@ -67,10 +67,8 @@ class ASTSearchInvocation extends BaseToolInvocation< targetDir, fileFilteringOptions, ); - const shouldIgnore = (filePath: string): boolean => fileDiscoveryService.shouldIgnoreFile( - filePath, - fileFilteringOptions, - ); + const shouldIgnore = (filePath: string): boolean => + fileDiscoveryService.shouldIgnoreFile(filePath, fileFilteringOptions); const astService = new ASTAnalysisService(targetDir, shouldIgnore); @@ -172,17 +170,8 @@ class ASTSearchInvocation extends BaseToolInvocation< safePath: string, symbolName: string, ): Promise { - const bounds = await astService.findSymbolBounds(safePath, symbolName); - - if (!bounds) { - return { - llmContent: - `Symbol "${symbolName}" not found in ${safePath}. ` + - 'Try using grep_search for a text-based search, or check the symbol name spelling.', - returnDisplay: 'Symbol not found', - }; - } - + // Single getFileOutline call to avoid reading and parsing the file twice + // (findSymbolBounds internally calls getFileOutline, so calling both is redundant). const outline = await astService.getFileOutline(safePath); const findSymbolRecursive = ( symbols: ASTSymbol[], @@ -196,24 +185,31 @@ class ASTSearchInvocation extends BaseToolInvocation< }; const symbol = outline ? findSymbolRecursive(outline.symbols) : undefined; + if (!symbol) { + return { + llmContent: + `Symbol "${symbolName}" not found in ${safePath}. ` + + 'Try using grep_search for a text-based search, or check the symbol name spelling.', + returnDisplay: 'Symbol not found', + }; + } + const result = [ `Found "${symbolName}" in ${safePath}:`, - ` Lines: ${bounds.startLine}-${bounds.endLine} (${bounds.endLine - bounds.startLine + 1} lines)`, - symbol ? ` Kind: ${symbol.kind}` : '', - symbol ? ` Signature: ${symbol.signature}` : '', + ` Lines: ${symbol.startLine}-${symbol.endLine} (${symbol.endLine - symbol.startLine + 1} lines)`, + ` Kind: ${symbol.kind}`, + ` Signature: ${symbol.signature}`, '', - `TIP: Use read_file with start_line=${bounds.startLine} and end_line=${bounds.endLine} to read the exact symbol body.`, - ] - .filter(Boolean) - .join('\n'); + `TIP: Use read_file with start_line=${symbol.startLine} and end_line=${symbol.endLine} to read the exact symbol body.`, + ].join('\n'); return { llmContent: result, - returnDisplay: `${symbolName}: L${bounds.startLine}-${bounds.endLine}`, + returnDisplay: `${symbolName}: L${symbol.startLine}-${symbol.endLine}`, display: { name: AST_SEARCH_DISPLAY_NAME, description: this.getDescription(), - resultSummary: `L${bounds.startLine}-${bounds.endLine}`, + resultSummary: `L${symbol.startLine}-${symbol.endLine}`, result: { type: 'text', text: result }, }, }; From 52ad2150a7c78803d9f388c1e4e6d4cb7971f53a Mon Sep 17 00:00:00 2001 From: dylanyunlon Date: Sat, 19 Sep 2026 04:33:43 +0000 Subject: [PATCH 17/23] fix(java): support multiple modifiers and generic return types in Java regex patterns Addresses round-17 gemini-code-assist review (HIGH x 2): 1. Java declaration patterns (class/interface/enum): replaced single optional modifier regex with word-boundary matching. Correctly handles multi-modifier declarations like 'public final class', 'public abstract class', 'public static sealed interface', etc. Before: /(?:public|private|protected)?\s*class\s+(\w+)/ After: /\bclass\s+(\w+)/ 2. Java member method pattern: replaced single optional modifier regex with a repeating modifier group supporting any number of modifiers (public, private, protected, static, final, synchronized, abstract, default) and generic/array return types like List or int[]. Before: /(?:public|private|protected|static)?\s*\w+\s+(\w+)\s*\(/ After: /(?:(?:public|...|default)\s+)*[\w<>[\]]+\s+(\w+)\s*\(/ --- packages/core/src/services/astAnalysisService.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/core/src/services/astAnalysisService.ts b/packages/core/src/services/astAnalysisService.ts index ac224b7b3cd..db3b4aea3a9 100644 --- a/packages/core/src/services/astAnalysisService.ts +++ b/packages/core/src/services/astAnalysisService.ts @@ -504,15 +504,15 @@ function getDeclarationPatterns(lang: string) { break; case 'java': p.push({ - regex: /(?:public|private|protected)?\s*class\s+(\w+)/, + regex: /\bclass\s+(\w+)/, kind: 'class', }); p.push({ - regex: /(?:public|private|protected)?\s*interface\s+(\w+)/, + regex: /\binterface\s+(\w+)/, kind: 'interface', }); p.push({ - regex: /(?:public|private|protected)?\s*enum\s+(\w+)/, + regex: /\benum\s+(\w+)/, kind: 'enum', }); break; @@ -545,7 +545,8 @@ function getMemberPatterns(lang: string) { break; case 'java': p.push({ - regex: /(?:public|private|protected|static)?\s*\w+\s+(\w+)\s*\(/, + regex: + /(?:(?:public|private|protected|static|final|synchronized|abstract|default)\s+)*[\w<>[{\]}]+\s+(\w+)\s*\(/, kind: 'method', }); break; From fec4ec42a1c3d53257e609a4dfb2e2f418157728 Mon Sep 17 00:00:00 2001 From: dylanyunlon Date: Sat, 19 Sep 2026 04:52:31 +0000 Subject: [PATCH 18/23] fix(perf): use centralized FileDiscoveryService and harden regex literal stripping Addresses round-18 gemini-code-assist review (HIGH x 3): 1. ASTSearchInvocation.execute: replaced 'new FileDiscoveryService()' with config.getFileService() to use the centralized cached instance, avoiding re-reading .gitignore/.geminiignore on every tool call. 2. ASTSearchTool constructor: same change, config.getFileService() instead of new FileDiscoveryService() for consistency and to avoid redundant initialization overhead. 3. findClosingBrace regex literal stripping: added semicolon exclusion to the regex-literal character class to prevent false positives when a line contains multiple division operators (e.g. 'a = b / c; if (a) {'). The semicolon prevents matching across separate statements. --- packages/core/src/services/astAnalysisService.ts | 4 +++- packages/core/src/tools/ast-search.ts | 16 +++++----------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/packages/core/src/services/astAnalysisService.ts b/packages/core/src/services/astAnalysisService.ts index db3b4aea3a9..0c2beb7b0f2 100644 --- a/packages/core/src/services/astAnalysisService.ts +++ b/packages/core/src/services/astAnalysisService.ts @@ -345,8 +345,10 @@ export function findClosingBrace(lines: string[], startLine: number): number { for (let i = startLine; i < lines.length; i++) { // Strip string literals (including escaped quotes) and regex literals // to avoid false brace matches from patterns like /[{}]/ or /a{1,3}/. + // The regex-literal pattern excludes semicolons to prevent matching across + // separate division statements (e.g. `const a = b / c; if (a) {`). let stripped = lines[i].replace( - /'(?:[^'\\]|\\.)*'|"(?:[^"\\]|\\.)*"|`(?:[^`\\]|\\.)*`|\/(?:[^/\\]|\\.)+\//g, + /'(?:[^'\\]|\\.)*'|"(?:[^"\\]|\\.)*"|`(?:[^`\\]|\\.)*`|\/(?:[^/;\\]|\\.)+\//g, '', ); // Strip single-line comments which may contain braces (e.g. "// }") diff --git a/packages/core/src/tools/ast-search.ts b/packages/core/src/tools/ast-search.ts index 962c041f81f..ce4805ff43a 100644 --- a/packages/core/src/tools/ast-search.ts +++ b/packages/core/src/tools/ast-search.ts @@ -26,7 +26,7 @@ import { type ASTSymbol, } from '../services/astAnalysisService.js'; import { debugLogger } from '../utils/debugLogger.js'; -import { FileDiscoveryService } from '../services/fileDiscoveryService.js'; +import type { FileDiscoveryService } from '../services/fileDiscoveryService.js'; export interface ASTSearchToolParams { symbol_name?: string; @@ -60,13 +60,10 @@ class ASTSearchInvocation extends BaseToolInvocation< const scope = this.params.scope ?? 'symbol'; const targetDir = this.config.getTargetDir(); - // Build shouldIgnore callback backed by FileDiscoveryService to enforce - // .gitignore and .geminiignore patterns during outline and map operations. + // Use the centralized, cached FileDiscoveryService from config to avoid + // re-reading and re-parsing .gitignore/.geminiignore on every tool call. const fileFilteringOptions = this.config.getFileFilteringOptions(); - const fileDiscoveryService = new FileDiscoveryService( - targetDir, - fileFilteringOptions, - ); + const fileDiscoveryService = this.config.getFileService(); const shouldIgnore = (filePath: string): boolean => fileDiscoveryService.shouldIgnoreFile(filePath, fileFilteringOptions); @@ -300,10 +297,7 @@ export class ASTSearchTool extends BaseDeclarativeTool< true, false, ); - this.fileDiscoveryService = new FileDiscoveryService( - config.getTargetDir(), - config.getFileFilteringOptions(), - ); + this.fileDiscoveryService = config.getFileService(); } protected override validateToolParamValues( From 2d2411117d37deea97b590431f54d5e8ff01acc3 Mon Sep 17 00:00:00 2001 From: dylanyunlon Date: Sat, 19 Sep 2026 05:31:18 +0000 Subject: [PATCH 19/23] fix(lang): add Rust impl blocks, Java records/constructors, Go interface methods Addresses round-19 gemini-code-assist review (HIGH x 4): 1. Rust impl blocks: added pattern to match 'impl Trait for Type' as a class-kind declaration so extractMembers can find methods declared inside impl blocks (previously completely ignored). 2. Java record declarations: added word-boundary pattern for Java 14+ record types (e.g. 'public record Point(int x, int y)') mapped to class kind. 3. Go interface methods: added fallback pattern matching 'MethodName(' for interface method signatures that don't use the func keyword or receiver syntax. 4. Java constructors: added secondary member pattern without return type requirement to match constructors (e.g. 'public MyClass()') which have no return type and were previously missed. --- .../core/src/services/astAnalysisService.ts | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/packages/core/src/services/astAnalysisService.ts b/packages/core/src/services/astAnalysisService.ts index 0c2beb7b0f2..20e050ebbef 100644 --- a/packages/core/src/services/astAnalysisService.ts +++ b/packages/core/src/services/astAnalysisService.ts @@ -500,9 +500,16 @@ function getDeclarationPatterns(lang: string) { break; case 'rust': p.push({ regex: /(?:pub\s+)?struct\s+(\w+)/, kind: 'class' }); + p.push({ + regex: /^impl(?:\s*<[^>]+>)?\s+(?:[\w:]+\s+for\s+)?(\w+)/, + kind: 'class', + }); p.push({ regex: /(?:pub\s+)?trait\s+(\w+)/, kind: 'interface' }); p.push({ regex: /(?:pub\s+)?enum\s+(\w+)/, kind: 'enum' }); - p.push({ regex: /(?:pub\s+)?(?:async\s+)?fn\s+(\w+)/, kind: 'function' }); + p.push({ + regex: /(?:pub\s+)?(?:async\s+)?fn\s+(\w+)/, + kind: 'function', + }); break; case 'java': p.push({ @@ -517,6 +524,10 @@ function getDeclarationPatterns(lang: string) { regex: /\benum\s+(\w+)/, kind: 'enum', }); + p.push({ + regex: /\brecord\s+(\w+)/, + kind: 'class', + }); break; default: break; @@ -541,6 +552,7 @@ function getMemberPatterns(lang: string) { break; case 'go': p.push({ regex: /func\s+\([^)]+\)\s+(\w+)/, kind: 'method' }); + p.push({ regex: /^(\w+)\s*\(/, kind: 'method' }); break; case 'rust': p.push({ regex: /(?:pub\s+)?(?:async\s+)?fn\s+(\w+)/, kind: 'method' }); @@ -551,6 +563,12 @@ function getMemberPatterns(lang: string) { /(?:(?:public|private|protected|static|final|synchronized|abstract|default)\s+)*[\w<>[{\]}]+\s+(\w+)\s*\(/, kind: 'method', }); + // Constructors have no return type, match modifier(s) + name + ( + p.push({ + regex: + /(?:(?:public|private|protected|static|final|synchronized|abstract|default)\s+)*(\w+)\s*\(/, + kind: 'method', + }); break; default: break; From a3561c88cdf39cbe8dd59d97d2f0ea44072fe0d6 Mon Sep 17 00:00:00 2001 From: dylanyunlon Date: Sat, 19 Sep 2026 05:47:20 +0000 Subject: [PATCH 20/23] fix: distinguish file-not-parsed from symbol-not-found and optimize brace counting Addresses round-20 gemini-code-assist review (HIGH x 2): 1. handleSymbolScope: when getFileOutline returns null (file missing, ignored, or unsupported language), now returns a specific 'File not parsed' error instead of the misleading 'Symbol not found'. This gives the LLM clear signal to try a different approach. 2. findClosingBrace performance: extracted preStripLines() that runs the string/regex/comment-stripping regex once per file upfront. extractSymbols passes the pre-stripped array to findClosingBrace and extractMembers, reducing complexity from O(M*N) to O(N) where M = number of declarations and N = total lines. findClosingBrace still works standalone (tests, extractMembers without cache) by falling back to preStripLines internally when no pre-stripped array is provided. --- .../core/src/services/astAnalysisService.ts | 52 +++++++++++++------ packages/core/src/tools/ast-search.ts | 10 +++- 2 files changed, 44 insertions(+), 18 deletions(-) diff --git a/packages/core/src/services/astAnalysisService.ts b/packages/core/src/services/astAnalysisService.ts index 20e050ebbef..5b40cfb4345 100644 --- a/packages/core/src/services/astAnalysisService.ts +++ b/packages/core/src/services/astAnalysisService.ts @@ -178,6 +178,10 @@ export function extractSymbols(lines: string[], language: string): ASTSymbol[] { // multi-line blocks, while preserving line numbers and offsets. const cleaned = stripBlockComments(lines, language); + // Pre-strip string/regex literals once for the entire file so that + // findClosingBrace does not re-run the regex on every call (O(N) instead of O(M*N)). + const stripped = language !== 'python' ? preStripLines(cleaned) : undefined; + for (let i = 0; i < cleaned.length; i++) { const trimmed = cleaned[i].trim(); @@ -207,7 +211,7 @@ export function extractSymbols(lines: string[], language: string): ASTSymbol[] { const endLine = language === 'python' ? findIndentEnd(cleaned, i) - : findClosingBrace(cleaned, i); + : findClosingBrace(cleaned, i, stripped); const sym: ASTSymbol = { name: match[1], @@ -218,7 +222,7 @@ export function extractSymbols(lines: string[], language: string): ASTSymbol[] { trimmed.length > 120 ? trimmed.slice(0, 117) + '...' : trimmed, children: kind === 'class' || kind === 'interface' - ? extractMembers(cleaned, i + 1, endLine, language) + ? extractMembers(cleaned, i + 1, endLine, language, stripped) : [], }; @@ -338,24 +342,37 @@ export function stripBlockComments( return result; } -export function findClosingBrace(lines: string[], startLine: number): number { - let depth = 0; - let parenDepth = 0; - let opened = false; - for (let i = startLine; i < lines.length; i++) { - // Strip string literals (including escaped quotes) and regex literals - // to avoid false brace matches from patterns like /[{}]/ or /a{1,3}/. - // The regex-literal pattern excludes semicolons to prevent matching across - // separate division statements (e.g. `const a = b / c; if (a) {`). - let stripped = lines[i].replace( - /'(?:[^'\\]|\\.)*'|"(?:[^"\\]|\\.)*"|`(?:[^`\\]|\\.)*`|\/(?:[^/;\\]|\\.)+\//g, - '', - ); - // Strip single-line comments which may contain braces (e.g. "// }") +/** + * Pre-strips string/regex literals and single-line comments from source lines. + * Call once per file and pass the result to findClosingBrace to avoid O(M*N) + * repeated regex replacements when parsing multiple declarations. + */ +export function preStripLines(lines: string[]): string[] { + const re = + /'(?:[^'\\]|\\.)*'|"(?:[^"\\]|\\.)*"|`(?:[^`\\]|\\.)*`|\/(?:[^/;\\]|\\.)+\//g; + return lines.map((line) => { + let stripped = line.replace(re, ''); const commentIdx = stripped.indexOf('//'); if (commentIdx >= 0) { stripped = stripped.slice(0, commentIdx); } + return stripped; + }); +} + +export function findClosingBrace( + lines: string[], + startLine: number, + strippedLines?: string[], +): number { + // When no pre-stripped lines are provided, compute them on the fly + // so that direct callers (tests, extractMembers without cache) still work. + const effective = strippedLines ?? preStripLines(lines); + let depth = 0; + let parenDepth = 0; + let opened = false; + for (let i = startLine; i < lines.length; i++) { + const stripped = effective[i]; for (const ch of stripped) { if (ch === '(') parenDepth++; else if (ch === ')') parenDepth--; @@ -429,6 +446,7 @@ function extractMembers( start: number, end: number, language: string, + strippedLines?: string[], ): ASTSymbol[] { const members: ASTSymbol[] = []; const patterns = getMemberPatterns(language); @@ -442,7 +460,7 @@ function extractMembers( const memberEnd = language === 'python' ? Math.min(findIndentEnd(lines, i), end) - : Math.min(findClosingBrace(lines, i), end); + : Math.min(findClosingBrace(lines, i, strippedLines), end); members.push({ name: match[1], kind, diff --git a/packages/core/src/tools/ast-search.ts b/packages/core/src/tools/ast-search.ts index ce4805ff43a..6ff73f542d8 100644 --- a/packages/core/src/tools/ast-search.ts +++ b/packages/core/src/tools/ast-search.ts @@ -170,6 +170,14 @@ class ASTSearchInvocation extends BaseToolInvocation< // Single getFileOutline call to avoid reading and parsing the file twice // (findSymbolBounds internally calls getFileOutline, so calling both is redundant). const outline = await astService.getFileOutline(safePath); + + if (!outline) { + return { + llmContent: `Could not search "${safePath}". File may not exist, is ignored, or its language is not supported.`, + returnDisplay: 'File not parsed', + }; + } + const findSymbolRecursive = ( symbols: ASTSymbol[], ): ASTSymbol | undefined => { @@ -180,7 +188,7 @@ class ASTSearchInvocation extends BaseToolInvocation< } return undefined; }; - const symbol = outline ? findSymbolRecursive(outline.symbols) : undefined; + const symbol = findSymbolRecursive(outline.symbols); if (!symbol) { return { From 34b349ea8763832e96cdb73fb72532bc8decd88b Mon Sep 17 00:00:00 2001 From: dylanyunlon Date: Sat, 19 Sep 2026 07:41:54 +0000 Subject: [PATCH 21/23] fix: clamp parenDepth and anchor Java member patterns Addresses round-21 gemini-code-assist review (HIGH x 2): 1. findClosingBrace: clamped parenDepth to min 0 on ')' using Math.max(0, parenDepth - 1). Prevents negative parenDepth from causing subsequent valid parenthesized blocks to have their internal braces incorrectly processed. 2. Java member patterns: added ^ anchor to both method and constructor patterns to prevent matching arbitrary method calls or object instantiations inside field initializers or static blocks. Also removed {} from the return type character class which incorrectly matched block structures like 'if (foo) { bar(); }'. --- packages/core/src/services/astAnalysisService.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/core/src/services/astAnalysisService.ts b/packages/core/src/services/astAnalysisService.ts index 5b40cfb4345..58d8a4938c4 100644 --- a/packages/core/src/services/astAnalysisService.ts +++ b/packages/core/src/services/astAnalysisService.ts @@ -375,7 +375,7 @@ export function findClosingBrace( const stripped = effective[i]; for (const ch of stripped) { if (ch === '(') parenDepth++; - else if (ch === ')') parenDepth--; + else if (ch === ')') parenDepth = Math.max(0, parenDepth - 1); // Ignore braces inside parentheses (inline object types in params) if (parenDepth > 0) continue; if (ch === '{') { @@ -578,13 +578,13 @@ function getMemberPatterns(lang: string) { case 'java': p.push({ regex: - /(?:(?:public|private|protected|static|final|synchronized|abstract|default)\s+)*[\w<>[{\]}]+\s+(\w+)\s*\(/, + /^(?:(?:public|private|protected|static|final|synchronized|abstract|default)\s+)*[\w<>[\]]+\s+(\w+)\s*\(/, kind: 'method', }); // Constructors have no return type, match modifier(s) + name + ( p.push({ regex: - /(?:(?:public|private|protected|static|final|synchronized|abstract|default)\s+)*(\w+)\s*\(/, + /^(?:(?:public|private|protected|static|final|synchronized|abstract|default)\s+)*(\w+)\s*\(/, kind: 'method', }); break; From a6a503659c281f70788acde0a7a32270c6e98bbf Mon Sep 17 00:00:00 2001 From: dylanyunlon Date: Sat, 19 Sep 2026 08:35:47 +0000 Subject: [PATCH 22/23] fix: filter control flow keywords from member extraction Addresses round-22 gemini-code-assist review (HIGH x 1): extractMembers: added keyword filter for if, for, while, switch, catch, synchronized, return. These control flow statements followed by parentheses (e.g. 'if (cond)') were incorrectly matched as sibling methods, cluttering the class outline. Now skipped before brace-matching to avoid wasted work and false positives. --- .../core/src/services/astAnalysisService.ts | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/packages/core/src/services/astAnalysisService.ts b/packages/core/src/services/astAnalysisService.ts index 58d8a4938c4..43fd93954a7 100644 --- a/packages/core/src/services/astAnalysisService.ts +++ b/packages/core/src/services/astAnalysisService.ts @@ -457,12 +457,27 @@ function extractMembers( for (const { regex, kind } of patterns) { const match = regex.exec(trimmed); if (!match?.[1]) continue; + const name = match[1]; + // Skip control flow keywords that look like function calls + if ( + [ + 'if', + 'for', + 'while', + 'switch', + 'catch', + 'synchronized', + 'return', + ].includes(name) + ) { + continue; + } const memberEnd = language === 'python' ? Math.min(findIndentEnd(lines, i), end) : Math.min(findClosingBrace(lines, i, strippedLines), end); members.push({ - name: match[1], + name, kind, startLine: i + 1, endLine: memberEnd + 1, From 233e60a403e6fa5665897e02e9d6b4b2c2ffe86f Mon Sep 17 00:00:00 2001 From: dylanyunlon Date: Sat, 19 Sep 2026 09:59:47 +0000 Subject: [PATCH 23/23] fix: defensive bounds checks, case-insensitive extensions, remove regex-literal stripping Addresses round-23 gemini-code-assist review (HIGH x 7): 1. getFileOutline: convert file extension to lowercase before LANG_MAP lookup so .TS, .PY, .GO etc. are recognized. 2. collectSourceFiles: same lowercase extension fix for codebase map. 3. preStripLines: removed regex-literal matching that falsely matched division operators (e.g. 'a / b + c / d' matched '/ b + c /'). Regex literals rarely contain unbalanced braces, so stripping strings and comments is sufficient. 4. findClosingBrace: added startLine bounds check (return startLine if out of range) and nullish coalesce on effective[i] to prevent TypeError when strippedLines is shorter than lines. 5. findIndentEnd: added startLine bounds check matching findClosingBrace. 6. extractMembers: added undefined guard on lines[i] before .trim() to prevent TypeError on out-of-bounds access. --- .../core/src/services/astAnalysisService.ts | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/packages/core/src/services/astAnalysisService.ts b/packages/core/src/services/astAnalysisService.ts index 43fd93954a7..fceb217414d 100644 --- a/packages/core/src/services/astAnalysisService.ts +++ b/packages/core/src/services/astAnalysisService.ts @@ -83,7 +83,7 @@ export class ASTAnalysisService { return null; } - const ext = path.extname(absPath); + const ext = path.extname(absPath).toLowerCase(); const language = LANG_MAP[ext]; if (!language) return null; @@ -348,8 +348,7 @@ export function stripBlockComments( * repeated regex replacements when parsing multiple declarations. */ export function preStripLines(lines: string[]): string[] { - const re = - /'(?:[^'\\]|\\.)*'|"(?:[^"\\]|\\.)*"|`(?:[^`\\]|\\.)*`|\/(?:[^/;\\]|\\.)+\//g; + const re = /'(?:[^'\\]|\\.)*'|"(?:[^"\\]|\\.)*"|`(?:[^`\\]|\\.)*`/g; return lines.map((line) => { let stripped = line.replace(re, ''); const commentIdx = stripped.indexOf('//'); @@ -365,6 +364,9 @@ export function findClosingBrace( startLine: number, strippedLines?: string[], ): number { + if (startLine < 0 || startLine >= lines.length) { + return startLine; + } // When no pre-stripped lines are provided, compute them on the fly // so that direct callers (tests, extractMembers without cache) still work. const effective = strippedLines ?? preStripLines(lines); @@ -372,7 +374,7 @@ export function findClosingBrace( let parenDepth = 0; let opened = false; for (let i = startLine; i < lines.length; i++) { - const stripped = effective[i]; + const stripped = effective[i] ?? ''; for (const ch of stripped) { if (ch === '(') parenDepth++; else if (ch === ')') parenDepth = Math.max(0, parenDepth - 1); @@ -393,6 +395,9 @@ export function findClosingBrace( } export function findIndentEnd(lines: string[], startLine: number): number { + if (startLine < 0 || startLine >= lines.length) { + return startLine; + } const baseIndent = lines[startLine].length - lines[startLine].trimStart().length; let last = startLine; @@ -452,7 +457,9 @@ function extractMembers( const patterns = getMemberPatterns(language); for (let i = start; i < end; i++) { - const trimmed = lines[i].trim(); + const line = lines[i]; + if (line === undefined) continue; + const trimmed = line.trim(); if (trimmed === '' || trimmed === '{' || trimmed === '}') continue; for (const { regex, kind } of patterns) { const match = regex.exec(trimmed); @@ -675,7 +682,7 @@ async function collectSourceFiles( !e.name.startsWith('.') ) { await walk(full, depth + 1, shouldIgnore); - } else if (e.isFile() && LANG_MAP[path.extname(e.name)]) { + } else if (e.isFile() && LANG_MAP[path.extname(e.name).toLowerCase()]) { files.push(full); } }