diff --git a/integration-tests/ripgrep-real.test.ts b/integration-tests/ripgrep-real.test.ts index 6b2aff905a..3ac8a0f16e 100644 --- a/integration-tests/ripgrep-real.test.ts +++ b/integration-tests/ripgrep-real.test.ts @@ -11,6 +11,7 @@ import * as os from 'node:os'; import { RipGrepTool } from '../packages/core/src/tools/ripGrep.js'; import { Config } from '../packages/core/src/config/config.js'; import { WorkspaceContext } from '../packages/core/src/utils/workspaceContext.js'; +import { createMockMessageBus } from '../packages/core/src/test-utils/mock-message-bus.js'; // Mock Config to provide necessary context class MockConfig { @@ -66,7 +67,7 @@ describe('ripgrep-real-direct', () => { await fs.writeFile(path.join(tempDir, 'file3.txt'), 'goodbye moon\n'); const config = new MockConfig(tempDir) as unknown as Config; - tool = new RipGrepTool(config); + tool = new RipGrepTool(config, createMockMessageBus()); }); afterAll(async () => { @@ -108,4 +109,24 @@ describe('ripgrep-real-direct', () => { expect(result.llmContent).toContain('script.js'); expect(result.llmContent).not.toContain('file1.txt'); }); + + it('should support context parameters', async () => { + // Create a file with multiple lines + await fs.writeFile( + path.join(tempDir, 'context.txt'), + 'line1\nline2\nline3 match\nline4\nline5\n', + ); + + const invocation = tool.build({ + pattern: 'match', + context: 1, + }); + const result = await invocation.execute(new AbortController().signal); + + expect(result.llmContent).toContain('Found 1 match'); + expect(result.llmContent).toContain('context.txt'); + expect(result.llmContent).toContain('L2- line2'); + expect(result.llmContent).toContain('L3: line3 match'); + expect(result.llmContent).toContain('L4- line4'); + }); }); diff --git a/packages/core/src/tools/ripGrep.test.ts b/packages/core/src/tools/ripGrep.test.ts index 2e1171d11c..f3c780603d 100644 --- a/packages/core/src/tools/ripGrep.test.ts +++ b/packages/core/src/tools/ripGrep.test.ts @@ -1408,42 +1408,45 @@ describe('RipGrepTool', () => { expect(result.llmContent).toContain('L1: HELLO world'); }); - it.each([ - { - name: 'fixed_strings parameter', - params: { pattern: 'hello.world', fixed_strings: true }, - mockOutput: { - path: { text: 'fileA.txt' }, - line_number: 1, - lines: { text: 'hello.world\n' }, - }, - expectedArgs: ['--fixed-strings'], - expectedPattern: 'hello.world', - }, - ])( - 'should handle $name', - async ({ params, mockOutput, expectedArgs, expectedPattern }) => { - mockSpawn.mockImplementationOnce( - createMockSpawn({ - outputData: - JSON.stringify({ type: 'match', data: mockOutput }) + '\n', - exitCode: 0, - }), - ); + it('should handle fixed_strings parameter', async () => { + mockSpawn.mockImplementationOnce( + createMockSpawn({ + outputData: + JSON.stringify({ + type: 'match', + data: { + path: { text: 'fileA.txt' }, + line_number: 1, + lines: { text: 'hello.world\n' }, + }, + }) + '\n', + exitCode: 0, + }), + ); - const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const invocation = grepTool.build({ + pattern: 'hello.world', + fixed_strings: true, + }); + const result = await invocation.execute(abortSignal); - expect(mockSpawn).toHaveBeenLastCalledWith( - expect.anything(), - expect.arrayContaining(expectedArgs), - expect.anything(), - ); - expect(result.llmContent).toContain( - `Found 1 match for pattern "${expectedPattern}"`, - ); - }, - ); + expect(mockSpawn).toHaveBeenLastCalledWith( + expect.anything(), + expect.arrayContaining(['--fixed-strings']), + expect.anything(), + ); + expect(result.llmContent).toContain( + 'Found 1 match for pattern "hello.world"', + ); + }); + + it('should allow invalid regex patterns when fixed_strings is true', () => { + const params: RipGrepToolParams = { + pattern: '[[', + fixed_strings: true, + }; + expect(grepTool.validateToolParams(params)).toBeNull(); + }); it('should handle no_ignore parameter', async () => { mockSpawn.mockImplementationOnce( @@ -1681,19 +1684,42 @@ describe('RipGrepTool', () => { mockSpawn.mockImplementationOnce( createMockSpawn({ outputData: + JSON.stringify({ + type: 'context', + data: { + path: { text: 'fileA.txt' }, + line_number: 1, + lines: { text: 'hello world\n' }, + }, + }) + + '\n' + JSON.stringify({ type: 'match', data: { path: { text: 'fileA.txt' }, line_number: 2, lines: { text: 'second line with world\n' }, - lines_before: [{ text: 'hello world\n' }], - lines_after: [ - { text: 'third line\n' }, - { text: 'fourth line\n' }, - ], }, - }) + '\n', + }) + + '\n' + + JSON.stringify({ + type: 'context', + data: { + path: { text: 'fileA.txt' }, + line_number: 3, + lines: { text: 'third line\n' }, + }, + }) + + '\n' + + JSON.stringify({ + type: 'context', + data: { + path: { text: 'fileA.txt' }, + line_number: 4, + lines: { text: 'fourth line\n' }, + }, + }) + + '\n', exitCode: 0, }), ); @@ -1721,9 +1747,10 @@ describe('RipGrepTool', () => { ); expect(result.llmContent).toContain('Found 1 match for pattern "world"'); expect(result.llmContent).toContain('File: fileA.txt'); + expect(result.llmContent).toContain('L1- hello world'); expect(result.llmContent).toContain('L2: second line with world'); - // Note: Ripgrep JSON output for context lines doesn't include line numbers for context lines directly - // The current parsing only extracts the matched line, so we only assert on that. + expect(result.llmContent).toContain('L3- third line'); + expect(result.llmContent).toContain('L4- fourth line'); }); }); diff --git a/packages/core/src/tools/ripGrep.ts b/packages/core/src/tools/ripGrep.ts index 68fa8cfb20..ebf022472c 100644 --- a/packages/core/src/tools/ripGrep.ts +++ b/packages/core/src/tools/ripGrep.ts @@ -140,6 +140,7 @@ interface GrepMatch { filePath: string; lineNumber: number; line: string; + isContext?: boolean; } class GrepToolInvocation extends BaseToolInvocation< @@ -267,8 +268,6 @@ class GrepToolInvocation extends BaseToolInvocation< return { llmContent: noMatchMsg, returnDisplay: `No matches found` }; } - const wasTruncated = allMatches.length >= totalMaxMatches; - const matchesByFile = allMatches.reduce( (acc, match) => { const fileKey = match.filePath; @@ -282,16 +281,19 @@ class GrepToolInvocation extends BaseToolInvocation< {} as Record, ); - const matchCount = allMatches.length; + const matchesOnly = allMatches.filter((m) => !m.isContext); + const matchCount = matchesOnly.length; const matchTerm = matchCount === 1 ? 'match' : 'matches'; + const wasTruncated = matchCount >= totalMaxMatches; + let llmContent = `Found ${matchCount} ${matchTerm} for pattern "${this.params.pattern}" ${searchLocationDescription}${this.params.include ? ` (filter: "${this.params.include}")` : ''}${wasTruncated ? ` (results limited to ${totalMaxMatches} matches for performance)` : ''}:\n---\n`; for (const filePath in matchesByFile) { llmContent += `File: ${filePath}\n`; matchesByFile[filePath].forEach((match) => { - const trimmedLine = match.line.trim(); - llmContent += `L${match.lineNumber}: ${trimmedLine}\n`; + const separator = match.isContext ? '-' : ':'; + llmContent += `L${match.lineNumber}${separator} ${match.line}\n`; }); llmContent += '---\n'; } @@ -402,11 +404,15 @@ class GrepToolInvocation extends BaseToolInvocation< allowedExitCodes: [0, 1], }); + let matchesFound = 0; for await (const line of generator) { const match = this.parseRipgrepJsonLine(line, absolutePath); if (match) { results.push(match); - if (results.length >= maxMatches) { + if (!match.isContext) { + matchesFound++; + } + if (matchesFound >= maxMatches) { break; } } @@ -425,11 +431,11 @@ class GrepToolInvocation extends BaseToolInvocation< ): GrepMatch | null { try { const json = JSON.parse(line); - if (json.type === 'match') { - const match = json.data; + if (json.type === 'match' || json.type === 'context') { + const data = json.data; // Defensive check: ensure text properties exist (skips binary/invalid encoding) - if (match.path?.text && match.lines?.text) { - const absoluteFilePath = path.resolve(basePath, match.path.text); + if (data.path?.text && data.lines?.text) { + const absoluteFilePath = path.resolve(basePath, data.path.text); const relativeCheck = path.relative(basePath, absoluteFilePath); if ( relativeCheck === '..' || @@ -443,8 +449,9 @@ class GrepToolInvocation extends BaseToolInvocation< return { filePath: relativeFilePath || path.basename(absoluteFilePath), - lineNumber: match.line_number, - line: match.lines.text.trimEnd(), + lineNumber: data.line_number, + line: data.lines.text.trimEnd(), + isContext: json.type === 'context', }; } } @@ -573,10 +580,12 @@ export class RipGrepTool extends BaseDeclarativeTool< protected override validateToolParamValues( params: RipGrepToolParams, ): string | null { - try { - new RegExp(params.pattern); - } catch (error) { - return `Invalid regular expression pattern provided: ${params.pattern}. Error: ${getErrorMessage(error)}`; + if (!params.fixed_strings) { + try { + new RegExp(params.pattern); + } catch (error) { + return `Invalid regular expression pattern provided: ${params.pattern}. Error: ${getErrorMessage(error)}`; + } } // Only validate path if one is provided