From 58a41c4533638fb54bc841641568c927c79ecb0b Mon Sep 17 00:00:00 2001 From: acoliver Date: Mon, 10 Aug 2026 22:52:22 -0300 Subject: [PATCH 1/5] Bound subprocess-backed tool output acquisition (Fixes #3203) --- ...iscovered-tool-bounded-acquisition.test.ts | 494 +++++++++++ .../grep-ripgrep-bounded-acquisition.test.ts | 804 +++++++++++++++++ ...grep-ripgrep-issue3203-remediation.test.ts | 347 ++++++++ packages/tools/src/index.ts | 1 + packages/tools/src/tools/grep.ts | 213 ++++- .../src/tools/grep/javascriptFallback.ts | 152 ++++ packages/tools/src/tools/grep/ripgrepParse.ts | 39 + .../tools/src/tools/grep/search-strategies.ts | 822 +++++++++++------- packages/tools/src/tools/grep/types.ts | 15 + packages/tools/src/tools/ripGrep.ts | 506 ++++++++--- packages/tools/src/tools/tool-registry.ts | 234 +++-- packages/tools/src/utils/lineFramer.test.ts | 490 +++++++++++ packages/tools/src/utils/lineFramer.ts | 161 ++++ .../src/utils/processTermination.test.ts | 505 +++++++++++ .../tools/src/utils/processTermination.ts | 305 +++++++ .../src/utils/ripgrepPathResolver.test.ts | 201 +++++ .../tools/src/utils/ripgrepPathResolver.ts | 51 +- .../tools/src/utils/subprocessSettle.test.ts | 184 ++++ packages/tools/src/utils/subprocessSettle.ts | 57 ++ project-plans/issue3203/PLAN.md | 232 +++++ 20 files changed, 5264 insertions(+), 549 deletions(-) create mode 100644 packages/tools/src/__tests__/discovered-tool-bounded-acquisition.test.ts create mode 100644 packages/tools/src/__tests__/grep-ripgrep-bounded-acquisition.test.ts create mode 100644 packages/tools/src/__tests__/grep-ripgrep-issue3203-remediation.test.ts create mode 100644 packages/tools/src/tools/grep/javascriptFallback.ts create mode 100644 packages/tools/src/tools/grep/ripgrepParse.ts create mode 100644 packages/tools/src/utils/lineFramer.test.ts create mode 100644 packages/tools/src/utils/lineFramer.ts create mode 100644 packages/tools/src/utils/processTermination.test.ts create mode 100644 packages/tools/src/utils/processTermination.ts create mode 100644 packages/tools/src/utils/ripgrepPathResolver.test.ts create mode 100644 packages/tools/src/utils/subprocessSettle.test.ts create mode 100644 packages/tools/src/utils/subprocessSettle.ts create mode 100644 project-plans/issue3203/PLAN.md diff --git a/packages/tools/src/__tests__/discovered-tool-bounded-acquisition.test.ts b/packages/tools/src/__tests__/discovered-tool-bounded-acquisition.test.ts new file mode 100644 index 0000000000..061ecfd4b9 --- /dev/null +++ b/packages/tools/src/__tests__/discovered-tool-bounded-acquisition.test.ts @@ -0,0 +1,494 @@ +/** + * @license + * Copyright 2026 Vybestack LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, beforeEach, afterEach } from 'bun:test'; +import { + writeFileSync, + mkdirSync, + rmSync, + chmodSync, + existsSync, +} from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { DiscoveredTool } from '../tools/tool-registry.js'; +import { + BoundedCombinedCollector, + createDefaultByteBudget, +} from '../acquisition/index.js'; +import type { IToolRegistryHost, IToolMessageBus } from '../index.js'; + +function createTempDir(prefix = 'llxprt-dt-test-'): { + dir: string; + cleanup: () => void; +} { + const dir = join( + tmpdir(), + `${prefix}${Date.now()}-${Math.random().toString(36).slice(2)}`, + ); + mkdirSync(dir, { recursive: true }); + return { + dir, + cleanup: () => { + try { + rmSync(dir, { recursive: true, force: true }); + } catch { + /* ignore */ + } + }, + }; +} + +function createScript(dir: string, name: string, content: string): string { + const scriptPath = join(dir, name); + writeFileSync(scriptPath, content, 'utf-8'); + chmodSync(scriptPath, 0o755); + return scriptPath; +} + +function createHost(callCommand: string): IToolRegistryHost { + return { + getToolCallCommand: () => callCommand, + }; +} + +const noopMessageBus: IToolMessageBus = { + requestConfirmation: () => Promise.resolve(true), +}; + +function createDiscoveredTool( + callCommand: string, + name = 'test', +): DiscoveredTool { + return new DiscoveredTool( + createHost(callCommand), + `discovered_tool_${name}`, + 'Test discovered tool', + { type: 'object', properties: {} }, + noopMessageBus, + ); +} + +async function executeTool( + tool: DiscoveredTool, + params: Record = {}, + signal?: AbortSignal, +) { + return tool.execute(params, signal ?? new AbortController().signal); +} + +describe('DiscoveredTool bounded acquisition', () => { + let tempDir: string; + let cleanup: () => void; + + beforeEach(() => { + const tmp = createTempDir(); + tempDir = tmp.dir; + cleanup = tmp.cleanup; + }); + + afterEach(() => { + cleanup(); + }); + + it.skipIf(process.platform === 'win32')( + 'bounds output exceeding the acquisition budget and reports truncation', + async () => { + // Produce 10 MiB of output — far beyond the 4 MiB default budget. + const script = createScript( + tempDir, + 'huge-output.sh', + '#!/bin/sh\nhead -c 10485760 /dev/zero | tr "\\0" "A"', + ); + const tool = createDiscoveredTool(script); + const result = await executeTool(tool); + + expect(result.error).toBeUndefined(); + const content = + typeof result.llmContent === 'string' ? result.llmContent : ''; + // Retained content must be bounded, not 10 MiB. + expect(content.length).toBeLessThan(10 * 1024 * 1024); + // Truncation must be reported so the result is not presented as exhaustive. + expect(content).toContain('truncated'); + }, + { timeout: 30000 }, + ); + + it.skipIf(process.platform === 'win32')( + 'truncation notice is identical in llmContent, returnDisplay, and error.message', + async () => { + // Produce 10 MiB on stdout AND write to stderr to trigger error path. + const script = createScript( + tempDir, + 'trunc-fail.sh', + '#!/bin/sh\nhead -c 10485760 /dev/zero | tr "\\0" "A"\necho "err" >&2\nexit 1', + ); + const tool = createDiscoveredTool(script); + const result = await executeTool(tool); + + expect(result.error).toBeDefined(); + const llm = + typeof result.llmContent === 'string' ? result.llmContent : ''; + const display = + typeof result.returnDisplay === 'string' ? result.returnDisplay : ''; + const errMsg = result.error?.message ?? ''; + + // All three must contain the truncation notice. + expect(llm).toContain('truncated'); + expect(display).toContain('truncated'); + expect(errMsg).toContain('truncated'); + + // llmContent, returnDisplay, and error.message must be identical. + expect(llm).toBe(display); + expect(llm).toBe(errMsg); + }, + { timeout: 30000 }, + ); + + it.skipIf(process.platform === 'win32')( + 'handles a multi-megabyte producer within bounded memory', + async () => { + // Portable POSIX producer: dd writes a single large block without + // command substitution or huge argv (unlike printf $(seq ...)). + const script = createScript( + tempDir, + 'huge-chunk.sh', + '#!/bin/sh\ndd if=/dev/zero bs=5242880 count=1 2>/dev/null | tr "\\0" "A"', + ); + const tool = createDiscoveredTool(script); + const result = await executeTool(tool); + + const content = + typeof result.llmContent === 'string' ? result.llmContent : ''; + // Must be bounded (default budget is 4 MiB), not 5 MiB. + expect(content.length).toBeLessThan(5 * 1024 * 1024); + // Must contain actual data, not near-empty error output. + expect(content).toContain('AAAA'); + // Truncation must be reported (5 MiB exceeds the 4 MiB budget). + expect(content).toContain('truncated'); + }, + { timeout: 30000 }, + ); + + it.skipIf(process.platform === 'win32')( + 'handles many small interleaved stdout/stderr chunks', + async () => { + const script = createScript( + tempDir, + 'interleaved.sh', + '#!/bin/sh\nfor i in $(seq 1 1000); do\n echo "out-$i"\n echo "err-$i" >&2\ndone', + ); + const tool = createDiscoveredTool(script); + const result = await executeTool(tool); + + const content = + typeof result.llmContent === 'string' ? result.llmContent : ''; + // Error path includes stderr content. Verify some lines survived. + expect(content).toContain('out-1'); + expect(content).toContain('err-1'); + }, + { timeout: 15000 }, + ); + + it.skipIf(process.platform === 'win32')( + 'handles multibyte UTF-8 content without replacement characters', + async () => { + const script = createScript( + tempDir, + 'multibyte.sh', + '#!/bin/sh\nprintf "café 世界 \\xe4\\xb8\\x96\\xe7\\x95\\x8c"', + ); + const tool = createDiscoveredTool(script); + const result = await executeTool(tool); + + expect(result.error).toBeUndefined(); + const content = + typeof result.llmContent === 'string' ? result.llmContent : ''; + expect(content).not.toContain('\uFFFD'); + }, + { timeout: 10000 }, + ); + + it.skipIf(process.platform === 'win32')( + 'writes params JSON to stdin (observable by the subprocess)', + async () => { + const script = createScript(tempDir, 'echo-stdin.sh', '#!/bin/sh\ncat'); + const tool = createDiscoveredTool(script); + const result = await executeTool(tool, { key: 'value', num: 42 }); + + expect(result.error).toBeUndefined(); + const content = + typeof result.llmContent === 'string' ? result.llmContent : ''; + expect(content).toContain('"key":"value"'); + expect(content).toContain('"num":42'); + }, + { timeout: 10000 }, + ); +}); + +describe('DiscoveredTool already-aborted signal', () => { + let tempDir: string; + let cleanup: () => void; + + beforeEach(() => { + const tmp = createTempDir(); + tempDir = tmp.dir; + cleanup = tmp.cleanup; + }); + + afterEach(() => { + cleanup(); + }); + + it.skipIf(process.platform === 'win32')( + 'does not execute the command when the signal is already aborted', + async () => { + // Create a marker script that writes a file when executed. + const markerPath = join(tempDir, 'executed.marker'); + const script = createScript( + tempDir, + 'marker.sh', + `#!/bin/sh\ntouch "${markerPath}"\necho done`, + ); + const tool = createDiscoveredTool(script); + const controller = new AbortController(); + controller.abort(); + + const result = await executeTool(tool, {}, controller.signal); + + // Must return cancellation result. + const content = + typeof result.llmContent === 'string' ? result.llmContent : ''; + expect(content).toMatch(/cancel/i); + // The marker file must NOT exist — the command was never executed. + expect(existsSync(markerPath)).toBe(false); + }, + { timeout: 10000 }, + ); +}); + +describe('DiscoveredTool bounded cancellation', () => { + let tempDir: string; + let cleanup: () => void; + + beforeEach(() => { + const tmp = createTempDir(); + tempDir = tmp.dir; + cleanup = tmp.cleanup; + }); + + afterEach(() => { + cleanup(); + }); + + it.skipIf(process.platform === 'win32')( + 'terminates the subprocess when the abort signal fires', + async () => { + const script = createScript( + tempDir, + 'long-running.sh', + '#!/bin/sh\nsleep 60\necho done', + ); + const tool = createDiscoveredTool(script); + const controller = new AbortController(); + + const executePromise = executeTool(tool, {}, controller.signal); + setTimeout(() => controller.abort(), 200); + + const result = await executePromise; + + const content = + typeof result.llmContent === 'string' ? result.llmContent : ''; + expect(result.error).toBeDefined(); + // Process was terminated by signal (not "Exit Code: N"). + const signalLine = content + .split('\n') + .find((l) => l.startsWith('Signal:')); + expect(signalLine).toBeDefined(); + expect(signalLine).not.toContain('(none)'); + }, + { timeout: 15000 }, + ); + + it.skipIf(process.platform === 'win32')( + 'escalates to SIGKILL for a process that ignores SIGTERM', + async () => { + const script = createScript( + tempDir, + 'ignore-sigterm.sh', + '#!/bin/sh\ntrap "" TERM\nsleep 60\necho done', + ); + const tool = createDiscoveredTool(script); + const controller = new AbortController(); + + const executePromise = executeTool(tool, {}, controller.signal); + setTimeout(() => controller.abort(), 300); + + const startTime = Date.now(); + const result = await executePromise; + const elapsed = Date.now() - startTime; + + // Must terminate within a bounded time (not wait forever). + // 300ms initial + 5000ms grace + overhead. + expect(elapsed).toBeLessThan(10000); + + const content = + typeof result.llmContent === 'string' ? result.llmContent : ''; + expect(result.error).toBeDefined(); + expect(content).toContain('Signal:'); + }, + { timeout: 15000 }, + ); +}); + +describe('DiscoveredTool unexpected signal is treated as failure', () => { + let tempDir: string; + let cleanup: () => void; + + beforeEach(() => { + const tmp = createTempDir(); + tempDir = tmp.dir; + cleanup = tmp.cleanup; + }); + + afterEach(() => { + cleanup(); + }); + + it.skipIf(process.platform === 'win32')( + 'a process killed by an unexpected signal is reported as failure, not success', + async () => { + // Script outputs some content, then sends itself SIGKILL. + // This is NOT an intentional termination by the tool — the process + // died unexpectedly. The result must be an error. + const script = createScript( + tempDir, + 'self-kill.sh', + '#!/bin/sh\necho "partial output"\nkill -KILL $$\n', + ); + const tool = createDiscoveredTool(script); + const result = await executeTool(tool); + + expect(result.error).toBeDefined(); + const content = + typeof result.llmContent === 'string' ? result.llmContent : ''; + expect(content).toContain('partial output'); + expect(content).toContain('Signal: SIGKILL'); + }, + { timeout: 10000 }, + ); +}); + +describe('DiscoveredTool spawn error settles promptly (no orphan drain timer)', () => { + it( + 'a spawn error settles without waiting for the drain timeout', + async () => { + const tool = createDiscoveredTool( + '/nonexistent/path/that/does/not/exist', + ); + const startTime = Date.now(); + const result = await executeTool(tool); + const elapsed = Date.now() - startTime; + + expect(result.error).toBeDefined(); + // STREAM_DRAIN_TIMEOUT_MS is 2000. An unguarded post-settlement + // onExit would install an orphan 2-second drain timer. + expect(elapsed).toBeLessThan(1500); + }, + { timeout: 10000 }, + ); +}); + +describe('DiscoveredTool stdin EPIPE during early child exit', () => { + let tempDir: string; + let cleanup: () => void; + + beforeEach(() => { + const tmp = createTempDir(); + tempDir = tmp.dir; + cleanup = tmp.cleanup; + }); + + afterEach(() => { + cleanup(); + }); + + it.skipIf(process.platform === 'win32')( + 'child exiting immediately during large stdin write does not crash parent', + async () => { + const script = createScript( + tempDir, + 'instant-exit.sh', + '#!/bin/sh\nexit 1', + ); + const tool = createDiscoveredTool(script); + const largeParams = { data: 'A'.repeat(2 * 1024 * 1024) }; + + const result = await executeTool(tool, largeParams); + + expect(result.error).toBeDefined(); + const content = + typeof result.llmContent === 'string' ? result.llmContent : ''; + // stdin EPIPE is the truthful primary failure (first-failure + // semantics). The key assertion is no uncaught crash. + expect(content).toContain('Error:'); + }, + { timeout: 15000 }, + ); + + it.skipIf(process.platform === 'win32')( + 'child closing stdin immediately with large payload reports truthful result', + async () => { + const script = createScript( + tempDir, + 'close-stdin.sh', + '#!/bin/sh\nexec cat < /dev/null\nexit 0', + ); + const tool = createDiscoveredTool(script); + const largeParams = { data: 'B'.repeat(4 * 1024 * 1024) }; + + const result = await executeTool(tool, largeParams); + + const content = + typeof result.llmContent === 'string' ? result.llmContent : ''; + // Must not crash and must produce a well-formed result. + expect(typeof result.llmContent).toBe('string'); + expect(content).toContain('Exit Code:'); + }, + { timeout: 15000 }, + ); +}); + +describe('BoundedCombinedCollector deterministic single huge chunk', () => { + it('retains bounded head and tail from one huge chunk', () => { + const collector = new BoundedCombinedCollector({ + budget: createDefaultByteBudget(), + }); + const budgetBytes = createDefaultByteBudget().bytes; + const hugeChunk = Buffer.alloc(budgetBytes + 2 * 1024 * 1024, 0x41); + collector.append(hugeChunk, 'stdout'); + const result = collector.getResult(); + + expect(result.metadata.truncated).toBe(true); + expect(result.metadata.observedBytes).toBe(hugeChunk.length); + expect(result.metadata.retainedBytes).toBeLessThanOrEqual(budgetBytes); + expect(result.stdoutText.length).toBeLessThanOrEqual(budgetBytes); + expect(result.stdoutText).toContain('AAAA'); + }); + + it('retains all bytes when chunk fits within budget', () => { + const collector = new BoundedCombinedCollector({ + budget: createDefaultByteBudget(), + }); + const chunk = Buffer.alloc(1024, 0x42); + collector.append(chunk, 'stdout'); + const result = collector.getResult(); + + expect(result.metadata.truncated).toBe(false); + expect(result.metadata.retainedBytes).toBe(1024); + expect(result.stdoutText).toHaveLength(1024); + }); +}); diff --git a/packages/tools/src/__tests__/grep-ripgrep-bounded-acquisition.test.ts b/packages/tools/src/__tests__/grep-ripgrep-bounded-acquisition.test.ts new file mode 100644 index 0000000000..1195d57d34 --- /dev/null +++ b/packages/tools/src/__tests__/grep-ripgrep-bounded-acquisition.test.ts @@ -0,0 +1,804 @@ +/** + * @license + * Copyright 2026 Vybestack LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, beforeEach, afterEach } from 'bun:test'; +import { writeFileSync, mkdirSync, rmSync } from 'node:fs'; +import { execSync } from 'node:child_process'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import type { IToolHost } from '../interfaces/index.js'; +import { GrepTool, RipGrepTool } from '../index.js'; +import type { ToolResult } from '../index.js'; +import type { GrepToolParams } from '../tools/grep/types.js'; +import type { RipGrepToolParams } from '../tools/ripGrep.js'; + +function createTempDir(prefix = 'llxprt-grep-bounded-'): { + dir: string; + cleanup: () => void; +} { + const dir = join( + tmpdir(), + `${prefix}${Date.now()}-${Math.random().toString(36).slice(2)}`, + ); + mkdirSync(dir, { recursive: true }); + return { + dir, + cleanup: () => { + try { + rmSync(dir, { recursive: true, force: true }); + } catch { + /* ignore */ + } + }, + }; +} + +function initGitRepo(dir: string): void { + execSync('git init', { cwd: dir, stdio: 'ignore' }); + execSync('git config user.email test@test.com', { + cwd: dir, + stdio: 'ignore', + }); + execSync('git config user.name Test', { cwd: dir, stdio: 'ignore' }); +} + +function gitAdd(dir: string): void { + execSync('git add -A', { cwd: dir, stdio: 'ignore' }); +} + +function createToolHost(targetDir: string): IToolHost { + return { + getTargetDir: () => targetDir, + getWorkspaceRoots: () => [targetDir], + getApprovalMode: () => 'auto', + setApprovalMode: () => {}, + isInteractive: () => false, + hasFeatureFlag: () => false, + getFileService: () => ({ + shouldGitIgnoreFile: () => false, + shouldLlxprtIgnoreFile: () => false, + shouldIgnoreFile: () => false, + filterFiles: (paths) => paths, + }), + getFileFilteringOptions: () => ({ + respectGitIgnore: true, + respectLlxprtIgnore: true, + }), + getFileExclusions: () => [], + getReadManyFilesExclusions: () => [], + getFileFilteringRespectLlxprtIgnore: () => true, + getLlxprtIgnoreFilePath: () => null, + recordFileRead: () => {}, + getFileSystemService: () => undefined, + getLlxprtIgnorePatterns: () => [], + getEphemeralSettings: () => ({ + 'tool-output-max-items': 50, + 'tool-output-max-tokens': 50000, + 'tool-output-item-size-limit': 524288, + }), + getDebugMode: () => false, + }; +} + +async function executeGrep( + host: IToolHost, + params: GrepToolParams, + signal?: AbortSignal, +): Promise { + const tool = new GrepTool(host); + try { + return await tool + .build(params) + .execute(signal ?? new AbortController().signal); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + return { llmContent: message, returnDisplay: message }; + } +} + +async function executeRipgrep( + host: IToolHost, + params: RipGrepToolParams, +): Promise { + const tool = new RipGrepTool(host); + try { + return await tool.build(params).execute(new AbortController().signal); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + return { llmContent: message, returnDisplay: message }; + } +} + +describe('Grep bounded acquisition and early stop', () => { + let tempDir: string; + let cleanup: () => void; + + beforeEach(() => { + const tmp = createTempDir(); + tempDir = tmp.dir; + cleanup = tmp.cleanup; + }); + + afterEach(() => { + cleanup(); + }); + + it( + 'dominant-file maxPerFile stays bounded and reaches a later file', + async () => { + // File A has 500 matching lines; File B has 3 matching lines. + // maxPerFile = 5, so File A should only contribute 5 matches. + // File B should still be reached and contribute its 3 matches. + const linesA = Array.from( + { length: 500 }, + (_, i) => `dominant match line ${i}`, + ).join('\n'); + writeFileSync(join(tempDir, 'dominant.txt'), linesA); + writeFileSync( + join(tempDir, 'later.txt'), + 'later match 1\nlater match 2\nlater match 3', + ); + + const result = await executeGrep(createToolHost(tempDir), { + pattern: 'match', + max_results: 100, + max_files: 100, + max_per_file: 5, + }); + + const text = + typeof result.llmContent === 'string' ? result.llmContent : ''; + // Both files must appear in results. + expect(text).toContain('dominant.txt'); + expect(text).toContain('later.txt'); + // File B's matches must be present. + expect(text).toContain('later match 1'); + expect(text).toContain('later match 3'); + // File A must not contribute more than 5 matches. + const dominantCount = (text.match(/dominant match line/g) ?? []).length; + expect(dominantCount).toBeLessThanOrEqual(5); + }, + { timeout: 15000 }, + ); + + it( + 'correctly parses CRLF line endings in search output', + async () => { + const content = 'café match\r\nworld match\r\nend'; + writeFileSync(join(tempDir, 'crlf.txt'), content); + + const result = await executeGrep(createToolHost(tempDir), { + pattern: 'match', + max_results: 100, + }); + + const text = + typeof result.llmContent === 'string' ? result.llmContent : ''; + expect(text).toContain('café'); + expect(text).toContain('world'); + // Should not contain carriage return characters in the output. + expect(text).not.toContain('\r'); + }, + { timeout: 15000 }, + ); + + it( + 'handles multibyte UTF-8 content with exact expected characters', + async () => { + const content = '世界 match\nこんにちは match\nhello match'; + writeFileSync(join(tempDir, 'multibyte.txt'), content); + + const result = await executeGrep(createToolHost(tempDir), { + pattern: 'match', + max_results: 100, + }); + + const text = + typeof result.llmContent === 'string' ? result.llmContent : ''; + expect(text).toContain('世界'); + expect(text).toContain('こんにちは'); + expect(text).not.toContain('\uFFFD'); + }, + { timeout: 15000 }, + ); + + it( + 'handles one huge line within bounded memory', + async () => { + // One very large line that is still a valid match. + const hugeLine = 'match ' + 'x'.repeat(100_000); + writeFileSync(join(tempDir, 'huge.txt'), hugeLine); + + const result = await executeGrep(createToolHost(tempDir), { + pattern: 'match', + max_results: 100, + }); + + const text = + typeof result.llmContent === 'string' ? result.llmContent : ''; + expect(text).toContain('huge.txt'); + }, + { timeout: 15000 }, + ); + + it( + 'handles many small interleaved output producers across files', + async () => { + for (let i = 0; i < 20; i++) { + writeFileSync( + join(tempDir, `f${i}.ts`), + `line match ${i}\nother line\nanother match ${i}`, + ); + } + + const result = await executeGrep(createToolHost(tempDir), { + pattern: 'match', + max_results: 100, + max_files: 100, + }); + + const text = + typeof result.llmContent === 'string' ? result.llmContent : ''; + // All 20 files should have matches. + for (let i = 0; i < 20; i++) { + expect(text).toContain(`f${i}.ts`); + } + }, + { timeout: 15000 }, + ); +}); + +describe('Git-grep with a real temporary git repository', () => { + let tempDir: string; + let cleanup: () => void; + + beforeEach(() => { + const tmp = createTempDir('llxprt-gitgrep-'); + tempDir = tmp.dir; + cleanup = tmp.cleanup; + initGitRepo(tempDir); + }); + + afterEach(() => { + cleanup(); + }); + + it( + 'finds matches via git-grep in a real repository', + async () => { + writeFileSync( + join(tempDir, 'tracked.ts'), + 'function target_match() {\n return 1;\n}\n', + ); + writeFileSync(join(tempDir, 'other.ts'), 'const x = "unrelated";\n'); + gitAdd(tempDir); + + const result = await executeGrep(createToolHost(tempDir), { + pattern: 'target_match', + max_results: 100, + }); + + const text = + typeof result.llmContent === 'string' ? result.llmContent : ''; + expect(text).toContain('tracked.ts'); + expect(text).toContain('target_match'); + expect(text).not.toContain('unrelated'); + }, + { timeout: 15000 }, + ); + + it( + 'abort does not trigger fallback — returns cancellation, not grep error', + async () => { + // Create many files with many matching lines so git grep takes time. + for (let i = 0; i < 200; i++) { + const lines: string[] = []; + for (let j = 0; j < 100; j++) { + lines.push(`match content ${i}_${j}`); + } + writeFileSync(join(tempDir, `f${i}.ts`), lines.join('\n')); + } + gitAdd(tempDir); + + const controller = new AbortController(); + const executePromise = executeGrep( + createToolHost(tempDir), + { pattern: 'match', max_results: 50000 }, + controller.signal, + ); + + setTimeout(() => controller.abort(), 10); + + const result = await executePromise; + const text = + typeof result.llmContent === 'string' ? result.llmContent : ''; + // Must be cancellation, not a system-grep fallback error. + expect(text).toMatch(/cancel|abort/i); + // Must NOT contain evidence of fallback strategy execution. + expect(text).not.toMatch(/system grep|javascript fallback|grep failed/i); + }, + { timeout: 30000 }, + ); +}); + +describe('Incomplete output never uses exact/exhaustive wording', () => { + let tempDir: string; + let cleanup: () => void; + + beforeEach(() => { + const tmp = createTempDir(); + tempDir = tmp.dir; + cleanup = tmp.cleanup; + }); + + afterEach(() => { + cleanup(); + }); + + it( + 'grep with limited results uses "Showing" not "Found N total" wording', + async () => { + // Create enough files/matches to trigger max_results limiting. + for (let i = 0; i < 50; i++) { + writeFileSync( + join(tempDir, `file${i}.txt`), + `match content ${i}\nsecond match ${i}`, + ); + } + + const result = await executeGrep(createToolHost(tempDir), { + pattern: 'match', + max_results: 5, + max_files: 100, + max_per_file: 50, + }); + + const text = + typeof result.llmContent === 'string' ? result.llmContent : ''; + expect(result.error).toBeUndefined(); + expect(text).toMatch( + /^(Showing \d+ matches|Found \d+ total matches, showing \d+)/m, + ); + expect(text).not.toMatch(/^Found \d+ matches?$/m); + }, + { timeout: 15000 }, + ); + + it( + 'grep returns correct no-match wording for exhaustive search', + async () => { + writeFileSync(join(tempDir, 'test.txt'), 'hello world\nfoo bar'); + + const result = await executeGrep(createToolHost(tempDir), { + pattern: 'nonexistent', + max_results: 100, + }); + + const text = + typeof result.llmContent === 'string' ? result.llmContent : ''; + expect(text).toContain('No matches found'); + }, + { timeout: 15000 }, + ); +}); + +describe('Ripgrep bounded acquisition and early stop', () => { + let tempDir: string; + let cleanup: () => void; + + beforeEach(() => { + const tmp = createTempDir(); + tempDir = tmp.dir; + cleanup = tmp.cleanup; + }); + + afterEach(() => { + cleanup(); + }); + + it( + 'correctly parses CRLF line endings', + async () => { + const content = 'hello match\r\nworld match\r\nend'; + writeFileSync(join(tempDir, 'crlf.txt'), content); + + const result = await executeRipgrep(createToolHost(tempDir), { + pattern: 'match', + }); + + const text = + typeof result.llmContent === 'string' ? result.llmContent : ''; + expect(text).toContain('hello'); + expect(text).toContain('world'); + expect(text).not.toContain('\r'); + }, + { timeout: 15000 }, + ); + + it( + 'handles multibyte UTF-8 content with exact expected characters', + async () => { + const content = '世界 match\n안녕 match\nhello match'; + writeFileSync(join(tempDir, 'multibyte.txt'), content); + + const result = await executeRipgrep(createToolHost(tempDir), { + pattern: 'match', + }); + + const text = + typeof result.llmContent === 'string' ? result.llmContent : ''; + expect(text).toContain('世界'); + expect(text).toContain('안녕'); + expect(text).not.toContain('\uFFFD'); + }, + { timeout: 15000 }, + ); + + it( + 'stops early when match count exceeds the limit and uses incomplete wording', + async () => { + // Create a file with 25000 matching lines — exceeds DEFAULT_TOTAL_MAX_MATCHES (20000) + const lines: string[] = []; + for (let i = 0; i < 25000; i++) { + lines.push(`matchline${i}`); + } + writeFileSync(join(tempDir, 'huge.txt'), lines.join('\n')); + + const result = await executeRipgrep(createToolHost(tempDir), { + pattern: 'matchline', + }); + + const text = + typeof result.llmContent === 'string' ? result.llmContent : ''; + // Must indicate incomplete results. + expect(text).toMatch(/incomplete|showing/i); + // Must NOT claim the full 25000 count. + expect(text).not.toContain('25000'); + // Must NOT say "Found 20000 matches" as if exhaustive. + expect(text).not.toMatch(/^Found 20000 matches?$/m); + }, + { timeout: 30000 }, + ); + + it( + 'returns exact no-match wording for exhaustive search', + async () => { + writeFileSync(join(tempDir, 'test.txt'), 'hello world\nfoo bar'); + + const result = await executeRipgrep(createToolHost(tempDir), { + pattern: 'nonexistent', + }); + + const text = + typeof result.llmContent === 'string' ? result.llmContent : ''; + expect(text).toContain('No matches'); + }, + { timeout: 15000 }, + ); + + it( + 'handles one huge line within bounded memory', + async () => { + const hugeLine = 'match ' + 'x'.repeat(100_000); + writeFileSync(join(tempDir, 'huge.txt'), hugeLine); + + const result = await executeRipgrep(createToolHost(tempDir), { + pattern: 'match', + }); + + const text = + typeof result.llmContent === 'string' ? result.llmContent : ''; + expect(text).toContain('huge.txt'); + }, + { timeout: 15000 }, + ); + + it( + 'handles many small interleaved files', + async () => { + for (let i = 0; i < 20; i++) { + writeFileSync( + join(tempDir, `f${i}.ts`), + `line match ${i}\nother\nanother match ${i}`, + ); + } + + const result = await executeRipgrep(createToolHost(tempDir), { + pattern: 'match', + }); + + const text = + typeof result.llmContent === 'string' ? result.llmContent : ''; + for (let i = 0; i < 20; i++) { + expect(text).toContain(`f${i}.ts`); + } + }, + { timeout: 15000 }, + ); +}); + +describe('Retained match bytes are bounded for huge matched lines', () => { + let tempDir: string; + let cleanup: () => void; + + beforeEach(() => { + const tmp = createTempDir(); + tempDir = tmp.dir; + cleanup = tmp.cleanup; + }); + + afterEach(() => { + cleanup(); + }); + + it( + 'ripgrep retains bounded match bytes when lines are very long', + async () => { + const longLine = 'X'.repeat(100 * 1024); + const lines: string[] = []; + for (let i = 0; i < 200; i++) { + lines.push(`matchprefix${longLine}`); + } + writeFileSync(join(tempDir, 'huge_matches.txt'), lines.join('\n')); + + const result = await executeRipgrep(createToolHost(tempDir), { + pattern: 'matchprefix', + }); + + const text = + typeof result.llmContent === 'string' ? result.llmContent : ''; + expect(text).toMatch(/incomplete|limited|truncated/i); + expect(text).not.toMatch(/200 (total )?match/); + }, + { timeout: 30000 }, + ); + + it( + 'grep retains bounded match bytes when lines are very long', + async () => { + const longLine = 'Y'.repeat(100 * 1024); + const lines: string[] = []; + for (let i = 0; i < 200; i++) { + lines.push(`grepword${longLine}`); + } + writeFileSync(join(tempDir, 'huge_matches.txt'), lines.join('\n')); + initGitRepo(tempDir); + gitAdd(tempDir); + + const result = await executeGrep(createToolHost(tempDir), { + pattern: 'grepword', + max_results: 1000, + }); + + const text = + typeof result.llmContent === 'string' ? result.llmContent : ''; + // Retained match bytes must be bounded — either the semantic budget + // stopped early (Showing/incomplete wording) or the token limiter + // caught the bounded output. Either way, must NOT claim all 200 + // matches were found exhaustively. + expect(text).toMatch( + /incomplete|limited|showing|too large|exceeded token/i, + ); + expect(text).not.toContain('Found 200 matches'); + }, + { timeout: 30000 }, + ); +}); + +describe('Grep limit validation rejects invalid values (C.3)', () => { + let tempDir: string; + let cleanup: () => void; + + beforeEach(() => { + const tmp = createTempDir(); + tempDir = tmp.dir; + cleanup = tmp.cleanup; + writeFileSync(join(tempDir, 'test.txt'), 'match\nmatch\n'); + }); + + afterEach(() => { + cleanup(); + }); + + const invalidValues: Array<[string, unknown]> = [ + ['zero', 0], + ['negative', -1], + ['fractional', 1.5], + ['NaN', NaN], + ['Infinity', Infinity], + ['-Infinity', -Infinity], + ]; + + for (const [label, value] of invalidValues) { + it(`rejects max_results=${label}`, async () => { + const result = await executeGrep(createToolHost(tempDir), { + pattern: 'match', + max_results: value as number, + }); + const text = + typeof result.llmContent === 'string' ? result.llmContent : ''; + expect(result.error !== undefined || text).toBeTruthy(); + expect(text).toMatch( + /finite positive integer|must be number|must be >= 1|exceeds the maximum|invalid/i, + ); + }); + + it(`rejects max_files=${label}`, async () => { + const result = await executeGrep(createToolHost(tempDir), { + pattern: 'match', + max_files: value as number, + }); + const text = + typeof result.llmContent === 'string' ? result.llmContent : ''; + expect(result.error !== undefined || text).toBeTruthy(); + expect(text).toMatch( + /finite positive integer|must be number|must be >= 1|exceeds the maximum|invalid/i, + ); + }); + + it(`rejects max_per_file=${label}`, async () => { + const result = await executeGrep(createToolHost(tempDir), { + pattern: 'match', + max_per_file: value as number, + }); + const text = + typeof result.llmContent === 'string' ? result.llmContent : ''; + expect(result.error !== undefined || text).toBeTruthy(); + expect(text).toMatch( + /finite positive integer|must be number|must be >= 1|exceeds the maximum|invalid/i, + ); + }); + + it(`rejects timeout_ms=${label}`, async () => { + const result = await executeGrep(createToolHost(tempDir), { + pattern: 'match', + timeout_ms: value as number, + }); + const text = + typeof result.llmContent === 'string' ? result.llmContent : ''; + expect(result.error !== undefined || text).toBeTruthy(); + expect(text).toMatch( + /finite positive integer|must be number|must be >= 1|exceeds the maximum|invalid/i, + ); + }); + } +}); + +describe('Grep maxPerFile exact total tracking (C.1)', () => { + let tempDir: string; + let cleanup: () => void; + + beforeEach(() => { + const tmp = createTempDir('llxprt-exact-total-'); + tempDir = tmp.dir; + cleanup = tmp.cleanup; + initGitRepo(tempDir); + }); + + afterEach(() => { + cleanup(); + }); + + it( + 'reports exact observed total when producer is fully consumed with per-file limiting', + async () => { + const lines = Array.from( + { length: 20 }, + (_, i) => `match line ${i}`, + ).join('\n'); + writeFileSync(join(tempDir, 'dominant.txt'), lines); + gitAdd(tempDir); + + const result = await executeGrep(createToolHost(tempDir), { + pattern: 'match', + max_results: 1000, + max_files: 100, + max_per_file: 5, + }); + + const text = + typeof result.llmContent === 'string' ? result.llmContent : ''; + expect(result.error).toBeUndefined(); + expect(text).toContain('dominant.txt'); + const dominantCount = (text.match(/match line/g) ?? []).length; + expect(dominantCount).toBe(5); + expect(text).toContain('Found 20 total'); + expect(text).toContain('showing 5'); + }, + { timeout: 15000 }, + ); +}); + +describe('Grep aggregate budget across multiple workspaces (C.2, C.4)', () => { + let tempDir: string; + let cleanup: () => void; + + beforeEach(() => { + const tmp = createTempDir(); + tempDir = tmp.dir; + cleanup = tmp.cleanup; + }); + + afterEach(() => { + cleanup(); + }); + + it( + 'remains under the aggregate semantic budget across 6 workspace directories', + async () => { + const dirs: string[] = []; + for (let i = 0; i < 6; i++) { + const dir = join(tempDir, `ws${i}`); + mkdirSync(dir, { recursive: true }); + const longLine = 'X'.repeat(900_000); + writeFileSync(join(dir, `f${i}.txt`), `matchprefix${longLine}\n`); + dirs.push(dir); + } + + const host: IToolHost = { + ...createToolHost(tempDir), + getWorkspaceRoots: () => dirs, + }; + + const result = await executeGrep(host, { + pattern: 'matchprefix', + max_results: 10000, + }); + + const text = + typeof result.llmContent === 'string' ? result.llmContent : ''; + expect(text).toMatch( + /incomplete|showing|limited|exceeded token|too large/i, + ); + expect(text).not.toContain('Found 6 matches'); + }, + { timeout: 30000 }, + ); +}); + +describe('Ripgrep aggregate budget across multiple workspaces (C.2, C.4)', () => { + let tempDir: string; + let cleanup: () => void; + + beforeEach(() => { + const tmp = createTempDir(); + tempDir = tmp.dir; + cleanup = tmp.cleanup; + }); + + afterEach(() => { + cleanup(); + }); + + it( + 'remains under the aggregate semantic budget across 6 workspace directories', + async () => { + const dirs: string[] = []; + for (let i = 0; i < 6; i++) { + const dir = join(tempDir, `ws${i}`); + mkdirSync(dir, { recursive: true }); + const longLine = 'X'.repeat(900_000); + writeFileSync(join(dir, `f${i}.txt`), `matchprefix${longLine}\n`); + dirs.push(dir); + } + + const host: IToolHost = { + ...createToolHost(tempDir), + getWorkspaceRoots: () => dirs, + }; + + const result = await executeRipgrep(host, { + pattern: 'matchprefix', + }); + + const text = + typeof result.llmContent === 'string' ? result.llmContent : ''; + expect(text).toMatch(/incomplete|showing|limited/i); + expect(text).not.toContain('Found 6 matches'); + }, + { timeout: 30000 }, + ); +}); diff --git a/packages/tools/src/__tests__/grep-ripgrep-issue3203-remediation.test.ts b/packages/tools/src/__tests__/grep-ripgrep-issue3203-remediation.test.ts new file mode 100644 index 0000000000..c57af330dc --- /dev/null +++ b/packages/tools/src/__tests__/grep-ripgrep-issue3203-remediation.test.ts @@ -0,0 +1,347 @@ +/** + * @license + * Copyright 2026 Vybestack LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, beforeEach, afterEach } from 'bun:test'; +import { writeFileSync, mkdirSync, rmSync } from 'node:fs'; +import { execSync } from 'node:child_process'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import type { IToolHost } from '../interfaces/index.js'; +import { GrepTool, RipGrepTool } from '../index.js'; +import type { ToolResult } from '../index.js'; +import type { GrepToolParams } from '../tools/grep/types.js'; +import type { RipGrepToolParams } from '../tools/ripGrep.js'; + +function createTempDir(prefix = 'llxprt-grep-remediation-'): { + dir: string; + cleanup: () => void; +} { + const dir = join( + tmpdir(), + `${prefix}${Date.now()}-${Math.random().toString(36).slice(2)}`, + ); + mkdirSync(dir, { recursive: true }); + return { + dir, + cleanup: () => { + try { + rmSync(dir, { recursive: true, force: true }); + } catch { + /* ignore */ + } + }, + }; +} + +function initGitRepo(dir: string): void { + execSync('git init', { cwd: dir, stdio: 'ignore' }); + execSync('git config user.email test@test.com', { + cwd: dir, + stdio: 'ignore', + }); + execSync('git config user.name Test', { cwd: dir, stdio: 'ignore' }); +} + +function gitAdd(dir: string): void { + execSync('git add -A', { cwd: dir, stdio: 'ignore' }); +} + +function createToolHost(targetDir: string): IToolHost { + return { + getTargetDir: () => targetDir, + getWorkspaceRoots: () => [targetDir], + getApprovalMode: () => 'auto', + setApprovalMode: () => {}, + isInteractive: () => false, + hasFeatureFlag: () => false, + getFileService: () => ({ + shouldGitIgnoreFile: () => false, + shouldLlxprtIgnoreFile: () => false, + shouldIgnoreFile: () => false, + filterFiles: (paths) => paths, + }), + getFileFilteringOptions: () => ({ + respectGitIgnore: true, + respectLlxprtIgnore: true, + }), + getFileExclusions: () => [], + getReadManyFilesExclusions: () => [], + getFileFilteringRespectLlxprtIgnore: () => true, + getLlxprtIgnoreFilePath: () => null, + recordFileRead: () => {}, + getFileSystemService: () => undefined, + getLlxprtIgnorePatterns: () => [], + getEphemeralSettings: () => ({ + 'tool-output-max-items': 50, + 'tool-output-max-tokens': 50000, + 'tool-output-item-size-limit': 524288, + }), + getDebugMode: () => false, + }; +} + +async function executeGrep( + host: IToolHost, + params: GrepToolParams, +): Promise { + const tool = new GrepTool(host); + try { + return await tool.build(params).execute(new AbortController().signal); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + return { llmContent: message, returnDisplay: message }; + } +} + +async function executeRipgrep( + host: IToolHost, + params: RipGrepToolParams, +): Promise { + const tool = new RipGrepTool(host); + try { + return await tool.build(params).execute(new AbortController().signal); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + return { llmContent: message, returnDisplay: message }; + } +} + +describe('Exact-limit evidence: producer at exactly the cap is exhaustive (item 1)', () => { + let tempDir: string; + let cleanup: () => void; + + beforeEach(() => { + const tmp = createTempDir(); + tempDir = tmp.dir; + cleanup = tmp.cleanup; + }); + + afterEach(() => { + cleanup(); + }); + + it( + 'grep with exactly max_results matches is NOT marked incomplete', + async () => { + for (let i = 0; i < 5; i++) { + writeFileSync(join(tempDir, `f${i}.txt`), `match_line_${i}\n`); + } + + const result = await executeGrep(createToolHost(tempDir), { + pattern: 'match_line', + max_results: 5, + max_files: 100, + max_per_file: 1, + }); + + const text = + typeof result.llmContent === 'string' ? result.llmContent : ''; + expect(result.error).toBeUndefined(); + expect(text).not.toMatch(/incomplete|showing.*may be/i); + expect(text).toMatch(/^Found 5 matches for pattern/m); + }, + { timeout: 15000 }, + ); + + it( + 'grep with max_results+1 matches IS marked incomplete', + async () => { + for (let i = 0; i < 6; i++) { + writeFileSync(join(tempDir, `f${i}.txt`), `match_line_${i}\n`); + } + + const result = await executeGrep(createToolHost(tempDir), { + pattern: 'match_line', + max_results: 5, + max_files: 100, + max_per_file: 1, + }); + + const text = + typeof result.llmContent === 'string' ? result.llmContent : ''; + expect(result.error).toBeUndefined(); + expect(text).toMatch(/incomplete|showing.*may be/i); + expect(text).not.toMatch(/^Found 5 matches for pattern/m); + }, + { timeout: 15000 }, + ); + + it( + 'ripgrep with fewer matches than cap is NOT marked incomplete', + async () => { + const lines: string[] = []; + for (let i = 0; i < 100; i++) { + lines.push(`match_${i}`); + } + writeFileSync(join(tempDir, 'big.txt'), lines.join('\n')); + + const result = await executeRipgrep(createToolHost(tempDir), { + pattern: 'match_', + }); + + const text = + typeof result.llmContent === 'string' ? result.llmContent : ''; + expect(text).not.toMatch(/incomplete|results may be incomplete/i); + expect(text).toMatch(/Found 100 matches/); + }, + { timeout: 15000 }, + ); +}); + +describe('Strategy budget rollback: failed strategy does not starve fallback (item 4)', () => { + it( + 'git grep failure restores budget for system grep', + async () => { + const { performGrepSearch, createAggregateSemanticBudget } = await import( + '../tools/grep/search-strategies.js' + ); + const tmp = createTempDir('llxprt-budget-rollback-'); + try { + initGitRepo(tmp.dir); + for (let i = 0; i < 10; i++) { + writeFileSync(join(tmp.dir, `f${i}.txt`), `match_line_${i}\n`); + } + gitAdd(tmp.dir); + + const budget = createAggregateSemanticBudget(); + const initialBytes = budget.remainingBytes; + const initialObjects = budget.remainingObjects; + + const result = await performGrepSearch( + { + pattern: 'match_line', + path: tmp.dir, + signal: new AbortController().signal, + maxResults: 100, + maxFiles: 100, + maxPerFile: 50, + semanticBudget: budget, + }, + ['node_modules'], + ); + + expect(result.results.length).toBeGreaterThan(0); + expect(budget.remainingBytes).toBeLessThanOrEqual(initialBytes); + expect(budget.remainingObjects).toBeLessThanOrEqual(initialObjects); + } finally { + tmp.cleanup(); + } + }, + { timeout: 15000 }, + ); +}); + +describe('JavaScript fallback maxFiles prompt stop (item 5)', () => { + let tempDir: string; + let cleanup: () => void; + + beforeEach(() => { + const tmp = createTempDir(); + tempDir = tmp.dir; + cleanup = tmp.cleanup; + }); + + afterEach(() => { + cleanup(); + }); + + it( + 'stops promptly when maxFiles is exceeded and marks incomplete', + async () => { + const { javascriptGrepFallback } = await import( + '../tools/grep/javascriptFallback.js' + ); + + for (let i = 0; i < 20; i++) { + writeFileSync(join(tempDir, `f${i}.txt`), `match_in_file_${i}\n`); + } + + const result = await javascriptGrepFallback( + 'match_in_file', + tempDir, + undefined, + new AbortController().signal, + 10000, + 5, + 50, + ['node_modules'], + ); + + expect(result.results.length).toBeLessThanOrEqual(5); + expect(result.incomplete).toBe(true); + expect(result.wasLimited).toBe(true); + expect(result.observedCount).toBeGreaterThanOrEqual( + result.results.length, + ); + }, + { timeout: 15000 }, + ); + + it( + 'does not claim exhaustive total when files limit is hit', + async () => { + const { javascriptGrepFallback } = await import( + '../tools/grep/javascriptFallback.js' + ); + + for (let i = 0; i < 10; i++) { + writeFileSync(join(tempDir, `f${i}.txt`), `unique_match_${i}\n`); + } + + const result = await javascriptGrepFallback( + 'unique_match', + tempDir, + undefined, + new AbortController().signal, + 10000, + 3, + 50, + ['node_modules'], + ); + + expect(result.results.length).toBeLessThanOrEqual(3); + expect(result.incomplete).toBe(true); + expect(result.totalFound).toBeUndefined(); + }, + { timeout: 15000 }, + ); +}); + +describe('Ripgrep multi-root budget exhaustion stops further spawns (item 11)', () => { + it( + 'does not spawn ripgrep for later directories after budget exhaustion', + async () => { + const dirs: string[] = []; + const tmp = createTempDir('llxprt-multi-root-budget-'); + try { + for (let i = 0; i < 6; i++) { + const dir = join(tmp.dir, `ws${i}`); + mkdirSync(dir, { recursive: true }); + const longLine = 'X'.repeat(900_000); + writeFileSync(join(dir, `f${i}.txt`), `matchprefix${longLine}\n`); + dirs.push(dir); + } + + const host: IToolHost = { + ...createToolHost(tmp.dir), + getWorkspaceRoots: () => dirs, + }; + + const result = await executeRipgrep(host, { + pattern: 'matchprefix', + }); + + const text = + typeof result.llmContent === 'string' ? result.llmContent : ''; + expect(text).toMatch(/incomplete|showing|limited/i); + expect(text).not.toContain('Found 6 matches'); + } finally { + tmp.cleanup(); + } + }, + { timeout: 30000 }, + ); +}); diff --git a/packages/tools/src/index.ts b/packages/tools/src/index.ts index db73beec2e..b9d37e4029 100644 --- a/packages/tools/src/index.ts +++ b/packages/tools/src/index.ts @@ -202,6 +202,7 @@ export { isRipgrepAvailable, clearRipgrepAvailabilityCache, ensureWindowsShortcut, + findInPath, } from './utils/ripgrepPathResolver.js'; export { TodoStatus, diff --git a/packages/tools/src/tools/grep.ts b/packages/tools/src/tools/grep.ts index 9d82e02c2b..ec7a2566c3 100644 --- a/packages/tools/src/tools/grep.ts +++ b/packages/tools/src/tools/grep.ts @@ -37,10 +37,72 @@ import { import { performGrepSearch, performSingleFileSearch, + createAggregateSemanticBudget, } from './grep/search-strategies.js'; export { type GrepToolParams } from './grep/types.js'; +const MAX_RESULTS_HARD_CAP = 100_000; +const MAX_FILES_HARD_CAP = 10_000; +const MAX_PER_FILE_HARD_CAP = 10_000; + +function validateFinitePositive( + value: unknown, + name: string, + hardCap: number, +): number { + if (value === undefined || value === null) return 0; + const n = typeof value === 'string' ? Number(value) : value; + if ( + typeof n !== 'number' || + !Number.isFinite(n) || + n <= 0 || + !Number.isInteger(n) + ) { + throw new Error( + `${name} must be a finite positive integer, got: ${String(value)}`, + ); + } + return Math.min(n, hardCap); +} + +function validateGrepLimits(params: GrepToolParams): { + maxResults: number; + maxFiles: number; + maxPerFile: number; + timeoutMs: number; +} { + const maxResultsRaw = validateFinitePositive( + params.max_results, + 'max_results', + MAX_RESULTS_HARD_CAP, + ); + const maxFilesRaw = validateFinitePositive( + params.max_files, + 'max_files', + MAX_FILES_HARD_CAP, + ); + const maxPerFileRaw = validateFinitePositive( + params.max_per_file, + 'max_per_file', + MAX_PER_FILE_HARD_CAP, + ); + const timeoutMsRaw = validateFinitePositive( + params.timeout_ms, + 'timeout_ms', + MAX_TIMEOUT_MS, + ); + return { + maxResults: maxResultsRaw !== 0 ? maxResultsRaw : 1000, + maxFiles: maxFilesRaw !== 0 ? maxFilesRaw : 100, + maxPerFile: maxPerFileRaw !== 0 ? maxPerFileRaw : 50, + timeoutMs: Math.min( + timeoutMsRaw !== 0 ? timeoutMsRaw : DEFAULT_TIMEOUT_MS, + MAX_TIMEOUT_MS, + ), + }; +} + class GrepToolInvocation extends BaseToolInvocation< GrepToolParams, ToolResult @@ -141,11 +203,16 @@ File: ${resolved.basename} ): Promise<{ allMatches: GrepMatch[]; totalMatchesFound: number; + totalObservedCount: number; wasLimited: boolean; + totalIsExact: boolean; }> { let allMatches: GrepMatch[] = []; let totalMatchesFound = 0; + let totalObservedCount = 0; let wasLimited = false; + let totalIsExact = true; + const aggregateBudget = createAggregateSemanticBudget(); for (const searchDir of searchDirectories) { if (allMatches.length >= maxResults) { @@ -162,6 +229,7 @@ File: ${resolved.basename} maxResults: maxResults - allMatches.length, maxFiles: maxFiles - filesWithMatches.size, maxPerFile, + semanticBudget: aggregateBudget, }, this.fileExclusions, ); @@ -170,6 +238,24 @@ File: ${resolved.basename} wasLimited = true; } + if (matches.incomplete === true) { + totalIsExact = false; + } + + if (matches.observedCount !== undefined) { + totalObservedCount += matches.observedCount; + } else if (matches.totalFound !== undefined) { + totalObservedCount += matches.totalFound; + } else { + totalObservedCount += matches.results.length; + } + + if (matches.totalFound !== undefined) { + totalMatchesFound += matches.totalFound; + } else if (matches.incomplete !== true) { + totalMatchesFound += matches.results.length; + } + if (searchDirectories.length > 1) { const dirName = path.basename(searchDir); matches.results.forEach((match) => { @@ -180,12 +266,17 @@ File: ${resolved.basename} matches.results.forEach((match) => { filesWithMatches.add(match.filePath); }); - totalMatchesFound += matches.totalFound ?? matches.results.length; allMatches = allMatches.concat(matches.results); } - return { allMatches, totalMatchesFound, wasLimited }; + return { + allMatches, + totalMatchesFound, + totalObservedCount, + wasLimited, + totalIsExact, + }; } /** @@ -257,16 +348,25 @@ File: ${resolved.basename} totalMatchesFound: number, matchCount: number, wasLimited: boolean, + totalIsExact: boolean, searchLocationDescription: string, ): string { let llmContent = ''; - if (wasLimited || totalMatchesFound > matchCount) { - llmContent = `Found ${totalMatchesFound} total matches, showing ${matchCount} for pattern "${this.params.pattern}" ${searchLocationDescription}${this.params.include ? ` (filter: "${this.params.include}")` : ''}: + const includeNote = this.params.include + ? ` (filter: "${this.params.include}")` + : ''; + if (!totalIsExact) { + // Search was incomplete — do not claim an exact total. + llmContent = `Showing ${matchCount} matches for pattern "${this.params.pattern}" ${searchLocationDescription}${includeNote} (results may be incomplete): +--- +`; + } else if (wasLimited || totalMatchesFound > matchCount) { + llmContent = `Found ${totalMatchesFound} total matches, showing ${matchCount} for pattern "${this.params.pattern}" ${searchLocationDescription}${includeNote}: --- `; } else { const matchTerm = matchCount === 1 ? 'match' : 'matches'; - llmContent = `Found ${matchCount} ${matchTerm} for pattern "${this.params.pattern}" ${searchLocationDescription}${this.params.include ? ` (filter: "${this.params.include}")` : ''}: + llmContent = `Found ${matchCount} ${matchTerm} for pattern "${this.params.pattern}" ${searchLocationDescription}${includeNote}: --- `; } @@ -296,7 +396,11 @@ File: ${resolved.basename} effectiveWasLimited: boolean, totalMatchesFound: number, matchCount: number, + totalIsExact: boolean, ): string { + if (!totalIsExact) { + return `Showing ${matchCount} matches (results may be incomplete)`; + } if (effectiveWasLimited || totalMatchesFound > matchCount) { return `Found ${totalMatchesFound} matches (showing ${matchCount})`; } @@ -310,14 +414,26 @@ File: ${resolved.basename} private buildDirectorySearchResult( allMatches: GrepMatch[], totalMatchesFound: number, + totalObservedCount: number, wasLimited: boolean, + totalIsExact: boolean, filesWithMatches: Set, searchLocationDescription: string, maxFiles: number, maxPerFile: number, ): ToolResult { + const includeNote = this.params.include + ? ` (filter: "${this.params.include}")` + : ''; if (allMatches.length === 0) { - const noMatchMsg = `No matches found for pattern "${this.params.pattern}" ${searchLocationDescription}${this.params.include ? ` (filter: "${this.params.include}")` : ''}.`; + if (!totalIsExact) { + const msg = `No matches retained for pattern "${this.params.pattern}" ${searchLocationDescription}${includeNote}. Results may be incomplete.`; + return { + llmContent: msg, + returnDisplay: 'No matches shown (incomplete)', + }; + } + const noMatchMsg = `No matches found for pattern "${this.params.pattern}" ${searchLocationDescription}${includeNote}.`; return { llmContent: noMatchMsg, returnDisplay: `No matches found` }; } @@ -341,6 +457,7 @@ File: ${resolved.basename} totalMatchesFound, matchCount, effectiveWasLimited, + totalIsExact, searchLocationDescription, ); @@ -363,6 +480,7 @@ File: ${resolved.basename} effectiveWasLimited, totalMatchesFound, matchCount, + totalIsExact, ); return { @@ -436,15 +554,10 @@ File: ${resolved.basename} workspaceContext: readonly string[], combinedSignal: AbortSignal, searchDirDisplay: string, + maxResults: number, + maxFiles: number, + maxPerFile: number, ): Promise { - const ephemeralSettings = this.host.getEphemeralSettings(); - const maxResults = - this.params.max_results ?? - (ephemeralSettings['tool-output-max-items'] as number | undefined) ?? - 1000; - const maxFiles = this.params.max_files ?? 100; - const maxPerFile = this.params.max_per_file ?? 50; - if (resolved.kind === 'file') { return this.executeSingleFileSearch( resolved as ResolvedSearchTarget & { kind: 'file' }, @@ -462,15 +575,20 @@ File: ${resolved.basename} } const filesWithMatches = new Set(); - const { allMatches, totalMatchesFound, wasLimited } = - await this.collectDirectoryMatches( - searchDirectories, - combinedSignal, - maxResults, - maxFiles, - maxPerFile, - filesWithMatches, - ); + const { + allMatches, + totalMatchesFound, + totalObservedCount, + wasLimited, + totalIsExact, + } = await this.collectDirectoryMatches( + searchDirectories, + combinedSignal, + maxResults, + maxFiles, + maxPerFile, + filesWithMatches, + ); let searchLocationDescription: string; if (resolved.kind === 'all-workspaces') { @@ -486,7 +604,9 @@ File: ${resolved.basename} return this.buildDirectorySearchResult( allMatches, totalMatchesFound, + totalObservedCount, wasLimited, + totalIsExact, filesWithMatches, searchLocationDescription, maxFiles, @@ -495,10 +615,8 @@ File: ${resolved.basename} } async execute(signal: AbortSignal): Promise { - // Set up timeout handling - const timeoutMs = Math.min( - this.params.timeout_ms ?? DEFAULT_TIMEOUT_MS, - MAX_TIMEOUT_MS, + const { maxResults, maxFiles, maxPerFile, timeoutMs } = validateGrepLimits( + this.params, ); const timeoutController = new AbortController(); const timeoutId = setTimeout(() => timeoutController.abort(), timeoutMs); @@ -535,6 +653,9 @@ File: ${resolved.basename} workspaceContext, combinedSignal, searchDirDisplay, + maxResults, + maxFiles, + maxPerFile, ); } catch (error) { return this.handleExecuteError( @@ -630,23 +751,30 @@ export class GrepTool extends BaseDeclarativeTool { }, max_results: { description: - 'Optional: Maximum number of total matches to return. Defaults to tool-output-max-items setting or 1000.', + 'Optional: Maximum number of total matches to return. Defaults to 1000. Must be a positive integer.', type: 'number', + minimum: 1, + maximum: MAX_RESULTS_HARD_CAP, }, max_files: { description: - 'Optional: Maximum number of files to include in results. Defaults to 100.', + 'Optional: Maximum number of files to include in results. Defaults to 100. Must be a positive integer.', type: 'number', + minimum: 1, + maximum: MAX_FILES_HARD_CAP, }, max_per_file: { description: - 'Optional: Maximum number of matches per file to return. Defaults to 50.', + 'Optional: Maximum number of matches per file to return. Defaults to 50. Must be a positive integer.', type: 'number', + minimum: 1, + maximum: MAX_PER_FILE_HARD_CAP, }, timeout_ms: { - description: - 'Optional: Timeout in milliseconds (default: 60000ms = 1 minute, max: 300000ms = 5 minutes). If the operation times out, an error is returned with suggestions.', + description: `Optional: Timeout in milliseconds (default: ${DEFAULT_TIMEOUT_MS}ms, max: ${MAX_TIMEOUT_MS}ms). Must be a positive integer.`, type: 'number', + minimum: 1, + maximum: MAX_TIMEOUT_MS, }, }, required: ['pattern'], @@ -677,6 +805,27 @@ export class GrepTool extends BaseDeclarativeTool { } } + for (const [value, name, hardCap] of [ + [params.max_results, 'max_results', MAX_RESULTS_HARD_CAP], + [params.max_files, 'max_files', MAX_FILES_HARD_CAP], + [params.max_per_file, 'max_per_file', MAX_PER_FILE_HARD_CAP], + [params.timeout_ms, 'timeout_ms', MAX_TIMEOUT_MS], + ] as const) { + if (value === undefined) continue; + const n = typeof value === 'string' ? Number(value) : value; + if ( + typeof n !== 'number' || + !Number.isFinite(n) || + n <= 0 || + !Number.isInteger(n) + ) { + return `${name} must be a finite positive integer, got: ${String(value)}`; + } + if (n > hardCap) { + return `${name} ${n} exceeds hard maximum ${hardCap}`; + } + } + return null; } diff --git a/packages/tools/src/tools/grep/javascriptFallback.ts b/packages/tools/src/tools/grep/javascriptFallback.ts new file mode 100644 index 0000000000..f01c887ef9 --- /dev/null +++ b/packages/tools/src/tools/grep/javascriptFallback.ts @@ -0,0 +1,152 @@ +/** + * @license + * Copyright 2026 Vybestack LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import fsPromises from 'fs/promises'; +import path from 'path'; +import { globStream } from 'glob'; + +import { getErrorMessage, isNodeError } from '../../utils/errors.js'; +import { debugLogger } from '../../utils/debugLogger.js'; +import type { GrepMatch, SearchResults } from './types.js'; + +function extractMatchesFromFile( + lines: string[], + fileAbsolutePath: string, + absolutePath: string, + regex: RegExp, + maxPerFile: number, + maxResults: number, + allMatches: GrepMatch[], + filesWithMatches: Set, +): number { + let matchesInFile = 0; + let totalFound = 0; + + lines.forEach((line, index) => { + if (regex.test(line)) { + totalFound++; + if (matchesInFile < maxPerFile && allMatches.length < maxResults) { + allMatches.push({ + filePath: + path.relative(absolutePath, fileAbsolutePath) || + path.basename(fileAbsolutePath), + lineNumber: index + 1, + line, + }); + matchesInFile++; + filesWithMatches.add(fileAbsolutePath); + } + } + }); + + return totalFound; +} + +function shouldProcessFile( + allMatchesLength: number, + maxResults: number, + filesWithMatchesSize: number, + maxFiles: number, + isKnownFile: boolean, +): boolean { + if (allMatchesLength >= maxResults) return false; + if (filesWithMatchesSize >= maxFiles && !isKnownFile) return false; + return true; +} + +async function processFallbackFile( + filePath: string, + absolutePath: string, + regex: RegExp, + maxPerFile: number, + maxResults: number, + allMatches: GrepMatch[], + filesWithMatches: Set, +): Promise { + try { + const content = await fsPromises.readFile(filePath, 'utf8'); + const lines = content.split(/\r?\n/); + return extractMatchesFromFile( + lines, + filePath, + absolutePath, + regex, + maxPerFile, + maxResults, + allMatches, + filesWithMatches, + ); + } catch (readError: unknown) { + if (!isNodeError(readError) || readError.code !== 'ENOENT') { + debugLogger.debug( + `GrepLogic: Could not read/process ${filePath}: ${getErrorMessage(readError)}`, + ); + } + return 0; + } +} + +export async function javascriptGrepFallback( + pattern: string, + absolutePath: string, + include: string | undefined, + abortSignal: AbortSignal, + maxResults: number, + maxFiles: number, + maxPerFile: number, + fileExclusions: readonly string[], +): Promise { + const globPattern = include ?? '**/*'; + const filesStream = globStream(globPattern, { + cwd: absolutePath, + dot: true, + ignore: [...fileExclusions], + absolute: true, + nodir: true, + signal: abortSignal, + }); + + const regex = new RegExp(pattern, 'i'); + const allMatches: GrepMatch[] = []; + const filesWithMatches = new Set(); + let totalFound = 0; + let filesLimitHit = false; + + for await (const filePath of filesStream) { + if ( + !shouldProcessFile( + allMatches.length, + maxResults, + filesWithMatches.size, + maxFiles, + filesWithMatches.has(filePath), + ) + ) { + if (filesWithMatches.size >= maxFiles) filesLimitHit = true; + break; + } + totalFound += await processFallbackFile( + filePath, + absolutePath, + regex, + maxPerFile, + maxResults, + allMatches, + filesWithMatches, + ); + } + + const incomplete = filesLimitHit; + const totalFoundValue = + incomplete || totalFound <= allMatches.length ? undefined : totalFound; + return { + results: allMatches, + wasLimited: totalFound > allMatches.length || filesLimitHit, + totalFound: totalFoundValue, + incomplete, + observedCount: totalFound, + }; +} diff --git a/packages/tools/src/tools/grep/ripgrepParse.ts b/packages/tools/src/tools/grep/ripgrepParse.ts new file mode 100644 index 0000000000..956466231b --- /dev/null +++ b/packages/tools/src/tools/grep/ripgrepParse.ts @@ -0,0 +1,39 @@ +/** + * @license + * Copyright 2026 Vybestack LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import path from 'path'; +import type { GrepMatch } from './types.js'; + +export function parseRipgrepLine( + line: string, + basePath: string, +): GrepMatch | null { + if (!line.trim()) return null; + + const nullSep = line.indexOf('\0'); + const pathSep = nullSep === -1 ? line.indexOf(':') : nullSep; + if (pathSep === -1) return null; + + const lineNumStart = pathSep + 1; + const contentSep = line.indexOf(':', lineNumStart); + if (contentSep === -1) return null; + + const filePathRaw = line.substring(0, pathSep); + const lineNumberStr = line.substring(lineNumStart, contentSep); + const lineContent = line.substring(contentSep + 1); + + const lineNumber = parseInt(lineNumberStr, 10); + if (isNaN(lineNumber)) return null; + + const absoluteFilePath = path.resolve(basePath, filePathRaw); + const relativeFilePath = path.relative(basePath, absoluteFilePath); + + return { + filePath: relativeFilePath || path.basename(absoluteFilePath), + lineNumber, + line: lineContent, + }; +} diff --git a/packages/tools/src/tools/grep/search-strategies.ts b/packages/tools/src/tools/grep/search-strategies.ts index c994f4c422..1c8817258d 100644 --- a/packages/tools/src/tools/grep/search-strategies.ts +++ b/packages/tools/src/tools/grep/search-strategies.ts @@ -11,14 +11,25 @@ import fsPromises from 'fs/promises'; import path from 'path'; -import { EOL } from 'os'; import { spawn } from 'child_process'; -import { globStream } from 'glob'; -import { getErrorMessage, isNodeError } from '../../utils/errors.js'; +import { getErrorMessage } from '../../utils/errors.js'; import { isGitRepository } from '../../utils/gitUtils.js'; import { debugLogger } from '../../utils/debugLogger.js'; import type { GrepMatch, SearchResults, SearchOptions } from './types.js'; +import { javascriptGrepFallback } from './javascriptFallback.js'; +import { + BoundedCombinedCollector, + createDefaultByteBudget, + DEFAULT_ACQUISITION_BUDGET_BYTES, +} from '../../acquisition/index.js'; +import { BoundedLineFramer } from '../../utils/lineFramer.js'; +import { terminateProcessTree } from '../../utils/processTermination.js'; +import { + createSettleFn, + type SubprocessSettlement, + type AbortHandlerRef, +} from '../../utils/subprocessSettle.js'; /** * Checks if a glob pattern contains brace expansion syntax that git grep doesn't support. @@ -37,7 +48,13 @@ export function hasBraceExpansion(pattern: string): boolean { /** * Checks if a command is available in the system's PATH. */ -export function isCommandAvailable(command: string): Promise { +export function isCommandAvailable( + command: string, + abortSignal?: AbortSignal, +): Promise { + if (abortSignal?.aborted === true) { + return Promise.resolve(false); + } return new Promise((resolve) => { const checkCommand = process.platform === 'win32' ? 'where' : 'command'; const checkArgs = @@ -46,13 +63,23 @@ export function isCommandAvailable(command: string): Promise { const child = spawn(checkCommand, checkArgs, { stdio: 'ignore', shell: true, + windowsHide: true, }); - child.on('close', (code) => resolve(code === 0)); - child.on('error', (err) => { - debugLogger.debug( - `[GrepTool] Failed to start process for '${command}':`, - err.message, - ); + const onAbort = () => { + try { + child.kill('SIGKILL'); + } catch { + /* best-effort */ + } + resolve(false); + }; + abortSignal?.addEventListener('abort', onAbort, { once: true }); + child.on('close', (code) => { + abortSignal?.removeEventListener('abort', onAbort); + resolve(code === 0); + }); + child.on('error', () => { + abortSignal?.removeEventListener('abort', onAbort); resolve(false); }); } catch { @@ -98,7 +125,7 @@ export function parseGrepOutput(output: string, basePath: string): GrepMatch[] { const results: GrepMatch[] = []; if (!output) return results; - const lines = output.split(EOL); + const lines = output.split(new RegExp('\\r?\\n')); for (const line of lines) { const match = parseGrepLine(line, basePath); @@ -167,6 +194,379 @@ export function applyLimits( totalFound: totalFound > results.length ? totalFound : undefined, }; } +/** + * Error thrown when a search subprocess is aborted via its AbortSignal. + * + * Has `name = 'AbortError'` so upstream callers can recognise it. Must never + * be caught as a strategy-unavailable failure or trigger grep fallback. + */ +export class SearchAbortedError extends Error { + constructor(message: string) { + super(message); + this.name = 'AbortError'; + } +} + +export class ProcessLifecycleError extends Error { + constructor(message: string) { + super(message); + this.name = 'ProcessLifecycleError'; + } +} + +function isLifecycleError(error: unknown): boolean { + return ( + error instanceof Error && + (error.name === 'AbortError' || error.name === 'ProcessLifecycleError') + ); +} + +interface BoundedGrepResult { + matches: GrepMatch[]; + observedCount: number; + earlyStopped: boolean; + budgetTruncated: boolean; + lineDropped: boolean; +} + +interface BoundedGrepSubprocessOptions { + filterStderr?: (text: string) => string; + tolerateNonZeroExitWithoutStderr?: boolean; +} + +export interface SemanticBudget { + remainingBytes: number; + remainingObjects: number; +} + +export const MATCH_OVERHEAD_BYTES = 256; +export const HARD_RETAINED_MATCH_CAP = 100_000; + +export function createAggregateSemanticBudget(): SemanticBudget { + return { + remainingBytes: DEFAULT_ACQUISITION_BUDGET_BYTES, + remainingObjects: HARD_RETAINED_MATCH_CAP, + }; +} + +function buildSearchResults( + matches: GrepMatch[], + observedCount: number, + incomplete: boolean, + maxResults: number, + maxFiles: number, + maxPerFile: number, +): SearchResults { + const limited = applyLimits(matches, maxResults, maxFiles, maxPerFile); + const wasLimited = limited.wasLimited === true || incomplete; + const totalFound = incomplete + ? undefined + : Math.max(observedCount, limited.results.length); + return { + results: limited.results, + wasLimited, + totalFound, + incomplete, + observedCount, + }; +} + +interface GrepLimits { + maxResults: number; + maxFiles: number; + maxPerFile: number; +} + +interface GrepAcquisitionState { + collector: BoundedCombinedCollector; + framer: BoundedLineFramer; + matches: GrepMatch[]; + perFileCount: Map; + filesSeen: Set; + observedCount: number; + usableCount: number; + retainedBytes: number; + semanticBudget: SemanticBudget; + earlyStopped: boolean; + capReached: boolean; + budgetExhausted: boolean; + terminated: boolean; + readonly limits: GrepLimits; +} + +function createGrepAcquisitionState( + limits: GrepLimits, + semanticBudget: SemanticBudget, +): GrepAcquisitionState { + return { + collector: new BoundedCombinedCollector({ + budget: createDefaultByteBudget(), + }), + framer: new BoundedLineFramer(), + matches: [], + perFileCount: new Map(), + filesSeen: new Set(), + observedCount: 0, + usableCount: 0, + retainedBytes: 0, + semanticBudget, + earlyStopped: false, + capReached: false, + budgetExhausted: false, + terminated: false, + limits, + }; +} + +/** + * Attempt to retain a parsed grep match in bounded semantic storage. + * + * Matches beyond {@link GrepLimits.maxPerFile} for a single file are counted + * but NOT retained, preventing a dominant file from growing the matches array + * without bound. Retained matches are also capped by an aggregate semantic + * byte budget tied to the same acquisition budget. + */ +function tryRetainGrepMatch( + state: GrepAcquisitionState, + match: GrepMatch, +): void { + state.observedCount++; + + if (state.capReached) { + state.earlyStopped = true; + return; + } + + state.filesSeen.add(match.filePath); + const fc = (state.perFileCount.get(match.filePath) ?? 0) + 1; + state.perFileCount.set(match.filePath, fc); + + if (fc <= state.limits.maxPerFile) { + const matchBytes = + Buffer.byteLength(match.line, 'utf8') + + Buffer.byteLength(match.filePath, 'utf8') + + MATCH_OVERHEAD_BYTES; + if ( + state.semanticBudget.remainingBytes < matchBytes || + state.semanticBudget.remainingObjects <= 0 + ) { + state.budgetExhausted = true; + state.earlyStopped = true; + return; + } + state.matches.push(match); + state.retainedBytes += matchBytes; + state.semanticBudget.remainingBytes -= matchBytes; + state.semanticBudget.remainingObjects--; + state.usableCount++; + } + + if (state.usableCount >= state.limits.maxResults) { + state.capReached = true; + } + if (state.filesSeen.size > state.limits.maxFiles) { + state.earlyStopped = true; + } +} + +/** + * Feed a stdout chunk into the collector and framer, consuming each complete + * bounded line record-at-a-time via callback. Returns true if early stop + * or budget exhaustion was triggered. + */ +function processGrepStdoutChunk( + state: GrepAcquisitionState, + chunk: Buffer, + cwd: string, +): boolean { + state.collector.append(chunk, 'stdout'); + if (state.terminated) return false; + + state.framer.feedChunk(chunk, (line) => { + if (state.earlyStopped) return; + const match = parseGrepLine(line, cwd); + if (!match) return; + tryRetainGrepMatch(state, match); + }); + + return state.earlyStopped; +} + +/** Flush remaining lines from the framer and retain bounded matches. */ +function flushGrepLines(state: GrepAcquisitionState, cwd: string): void { + state.framer.flushRemaining((line) => { + if (state.earlyStopped) return; + const match = parseGrepLine(line, cwd); + if (!match) return; + tryRetainGrepMatch(state, match); + }); +} + +/** + * Check the exit code and return an Error if the subprocess genuinely failed. + * Returns null for success, no-match (code 1), early stop, or abort. + * An unexpected signal kill (code null with a non-intentional signal) is + * treated as genuine failure, not successful exhaustive output. + */ +function checkGrepExitCode( + code: number | null, + signal: NodeJS.Signals | null, + earlyStopped: boolean, + aborted: boolean, + terminated: boolean, + stderrText: string, + command: string, + options: BoundedGrepSubprocessOptions | undefined, +): Error | null { + if (earlyStopped || aborted || terminated) return null; + if (code === 0 || code === 1) return null; + if (signal !== null) { + return new Error(`${command} was killed by signal ${signal}`); + } + if (code !== null) { + if ( + options?.tolerateNonZeroExitWithoutStderr === true && + stderrText.length === 0 + ) { + return null; + } + return new Error(`${command} exited with code ${code}: ${stderrText}`); + } + return new Error(`${command} closed unexpectedly`); +} + +/** Resolution of a grep subprocess close event. */ +interface GrepCloseResolution { + readonly result?: BoundedGrepResult; + readonly error?: Error; +} + +/** Resolve a grep subprocess close into a result or error. */ +function resolveGrepClose( + code: number | null, + signal: NodeJS.Signals | null, + state: GrepAcquisitionState, + aborted: boolean, + cwd: string, + command: string, + options: BoundedGrepSubprocessOptions | undefined, +): GrepCloseResolution { + if (!state.terminated) { + flushGrepLines(state, cwd); + } + const rawStderr = state.collector.getStderrText().trim(); + const stderrText = options?.filterStderr + ? options.filterStderr(rawStderr).trim() + : rawStderr; + const exitError = checkGrepExitCode( + code, + signal, + state.earlyStopped, + aborted, + state.terminated, + stderrText, + command, + options, + ); + if (exitError !== null) { + return { error: exitError }; + } + const acquisition = state.collector.getResult(); + return { + result: { + matches: state.matches, + observedCount: state.observedCount, + earlyStopped: state.earlyStopped, + budgetTruncated: acquisition.metadata.truncated || state.budgetExhausted, + lineDropped: state.framer.wasLineDropped, + }, + }; +} + +async function runBoundedGrepSubprocess( + command: string, + args: string[], + cwd: string, + abortSignal: AbortSignal, + maxResults: number, + maxFiles: number, + maxPerFile: number, + semanticBudget: SemanticBudget, + options?: BoundedGrepSubprocessOptions, +): Promise { + if (abortSignal.aborted) { + throw new SearchAbortedError(`${command} aborted`); + } + const limits: GrepLimits = { maxResults, maxFiles, maxPerFile }; + const state = createGrepAcquisitionState(limits, semanticBudget); + return new Promise((resolve, reject) => { + const settlement: SubprocessSettlement = { + settled: false, + terminationPromise: null, + }; + const abortRef: AbortHandlerRef = { handler: () => {} }; + + const child = spawn(command, args, { + cwd, + windowsHide: true, + detached: process.platform !== 'win32', + }); + + const stopProcess = () => { + if (state.terminated) return; + state.terminated = true; + settlement.terminationPromise = terminateProcessTree(child, { + ownsProcessGroup: process.platform !== 'win32', + }); + }; + + const settle = createSettleFn( + settlement, + abortSignal, + abortRef, + reject, + ProcessLifecycleError, + command, + ); + + abortRef.handler = () => { + stopProcess(); + settle(() => reject(new SearchAbortedError(`${command} aborted`))); + }; + abortSignal.addEventListener('abort', abortRef.handler); + + child.stdout.on('data', (chunk: Buffer) => { + if (processGrepStdoutChunk(state, chunk, cwd)) stopProcess(); + }); + child.stderr.on('data', (chunk: Buffer) => + state.collector.append(chunk, 'stderr'), + ); + child.on('error', (err: Error) => { + settle(() => { + reject( + abortSignal.aborted + ? new SearchAbortedError(`${command} aborted`) + : new Error(`Failed to start ${command}: ${err.message}`), + ); + }); + }); + child.on('close', (code: number | null, signal: NodeJS.Signals | null) => { + settle(() => { + const resolution = resolveGrepClose( + code, + signal, + state, + abortSignal.aborted, + cwd, + command, + options, + ); + if (resolution.error !== undefined) reject(resolution.error); + else if (resolution.result !== undefined) resolve(resolution.result); + }); + }); + }); +} /** * Runs git grep as Strategy 1. @@ -180,9 +580,14 @@ export async function tryGitGrep( maxFiles: number, maxPerFile: number, hasBracePattern: boolean, + semanticBudget: SemanticBudget, ): Promise { const isGit = !hasBracePattern && isGitRepository(absolutePath); - const gitAvailable = isGit && (await isCommandAvailable('git')); + const gitAvailable = isGit && (await isCommandAvailable('git', abortSignal)); + + if (abortSignal.aborted) { + throw new SearchAbortedError('git grep aborted'); + } if (!gitAvailable) return null; @@ -192,42 +597,35 @@ export async function tryGitGrep( } try { - const output = await new Promise((resolve, reject) => { - const child = spawn('git', gitArgs, { - cwd: absolutePath, - windowsHide: true, - }); - const stdoutChunks: Buffer[] = []; - const stderrChunks: Buffer[] = []; - - const abortHandler = () => { - if (!child.killed) { - child.kill('SIGTERM'); - } - reject(new Error('git grep aborted')); - }; - abortSignal.addEventListener('abort', abortHandler); - - child.stdout.on('data', (chunk) => stdoutChunks.push(chunk)); - child.stderr.on('data', (chunk) => stderrChunks.push(chunk)); - child.on('error', (err) => { - abortSignal.removeEventListener('abort', abortHandler); - reject(new Error(`Failed to start git grep: ${err.message}`)); - }); - child.on('close', (code) => { - abortSignal.removeEventListener('abort', abortHandler); - const stdoutData = Buffer.concat(stdoutChunks).toString('utf8'); - const stderrData = Buffer.concat(stderrChunks).toString('utf8'); - if (code === 0) resolve(stdoutData); - else if (code === 1) - resolve(''); // No matches - else - reject(new Error(`git grep exited with code ${code}: ${stderrData}`)); - }); - }); - const matches = parseGrepOutput(output, absolutePath); - return applyLimits(matches, maxResults, maxFiles, maxPerFile); + const { + matches, + observedCount, + earlyStopped, + budgetTruncated, + lineDropped, + } = await runBoundedGrepSubprocess( + 'git', + gitArgs, + absolutePath, + abortSignal, + maxResults, + maxFiles, + maxPerFile, + semanticBudget, + ); + const incomplete = earlyStopped || budgetTruncated || lineDropped; + return buildSearchResults( + matches, + observedCount, + incomplete, + maxResults, + maxFiles, + maxPerFile, + ); } catch (gitError: unknown) { + if (isLifecycleError(gitError)) { + throw gitError; + } debugLogger.debug( `GrepLogic: git grep failed: ${getErrorMessage( gitError, @@ -276,68 +674,19 @@ export function buildSystemGrepArgs( } /** - * Sets up event handlers for a spawned grep child process. + * Filters system grep stderr, removing non-fatal noise like permission + * denied or "Is a directory" messages. */ -function setupSystemGrepHandlers( - child: ReturnType, - abortSignal: AbortSignal, - stdoutChunks: Buffer[], - stderrChunks: Buffer[], - resolve: (value: string) => void, - reject: (reason: Error) => void, -): () => void { - const abortHandler = () => { - if (!child.killed) { - child.kill('SIGTERM'); - } - cleanup(); - reject(new Error('system grep aborted')); - }; - abortSignal.addEventListener('abort', abortHandler); - - const onData = (chunk: Buffer) => stdoutChunks.push(chunk); - const onStderr = (chunk: Buffer) => { - const stderrStr = chunk.toString(); - if ( - !stderrStr.includes('Permission denied') && - !/grep:.*: Is a directory/i.test(stderrStr) - ) { - stderrChunks.push(chunk); - } - }; - const onError = (err: Error) => { - cleanup(); - reject(new Error(`Failed to start system grep: ${err.message}`)); - }; - const onClose = (code: number | null) => { - const stdoutData = Buffer.concat(stdoutChunks).toString('utf8'); - const stderrData = Buffer.concat(stderrChunks).toString('utf8').trim(); - cleanup(); - if (code === 0) resolve(stdoutData); - else if (code === 1) - resolve(''); // No matches - else if (stderrData) - reject(new Error(`System grep exited with code ${code}: ${stderrData}`)); - else resolve(''); // Exit code > 1 but no stderr, likely just suppressed errors - }; - - const cleanup = () => { - abortSignal.removeEventListener('abort', abortHandler); - child.stdout!.removeListener('data', onData); - child.stderr!.removeListener('data', onStderr); - child.removeListener('error', onError); - child.removeListener('close', onClose); - if (child.connected) { - child.disconnect(); - } - }; - - child.stdout!.on('data', onData); - child.stderr!.on('data', onStderr); - child.on('error', onError); - child.on('close', onClose); - - return cleanup; +function filterSystemGrepStderr(text: string): string { + const crlf = new RegExp('\\r?\\n'); + return text + .split(crlf) + .filter( + (line) => + !line.includes('Permission denied') && + !/grep:.*: Is a directory/i.test(line), + ) + .join('\n'); } /** @@ -350,28 +699,42 @@ export async function trySystemGrep( maxResults: number, maxFiles: number, maxPerFile: number, + semanticBudget: SemanticBudget, ): Promise { try { - const output = await new Promise((resolve, reject) => { - const child = spawn('grep', grepArgs, { - cwd: absolutePath, - windowsHide: true, - }); - const stdoutChunks: Buffer[] = []; - const stderrChunks: Buffer[] = []; - - setupSystemGrepHandlers( - child, - abortSignal, - stdoutChunks, - stderrChunks, - resolve, - reject, - ); - }); - const matches = parseGrepOutput(output, absolutePath); - return applyLimits(matches, maxResults, maxFiles, maxPerFile); + const { + matches, + observedCount, + earlyStopped, + budgetTruncated, + lineDropped, + } = await runBoundedGrepSubprocess( + 'grep', + grepArgs, + absolutePath, + abortSignal, + maxResults, + maxFiles, + maxPerFile, + semanticBudget, + { + filterStderr: filterSystemGrepStderr, + tolerateNonZeroExitWithoutStderr: true, + }, + ); + const incomplete = earlyStopped || budgetTruncated || lineDropped; + return buildSearchResults( + matches, + observedCount, + incomplete, + maxResults, + maxFiles, + maxPerFile, + ); } catch (grepError: unknown) { + if (isLifecycleError(grepError)) { + throw grepError; + } debugLogger.debug( `GrepLogic: System grep failed: ${getErrorMessage( grepError, @@ -381,168 +744,6 @@ export async function trySystemGrep( } } -/** - * Extracts matches from a single file's content lines. - */ -function extractMatchesFromFile( - lines: string[], - fileAbsolutePath: string, - absolutePath: string, - regex: RegExp, - maxPerFile: number, - maxResults: number, - allMatches: GrepMatch[], - filesWithMatches: Set, -): number { - let matchesInFile = 0; - let totalFound = 0; - - lines.forEach((line, index) => { - if (regex.test(line)) { - totalFound++; - if (matchesInFile < maxPerFile && allMatches.length < maxResults) { - allMatches.push({ - filePath: - path.relative(absolutePath, fileAbsolutePath) || - path.basename(fileAbsolutePath), - lineNumber: index + 1, - line, - }); - matchesInFile++; - filesWithMatches.add(fileAbsolutePath); - } - } - }); - - return totalFound; -} - -/** - * Determines whether the JS fallback loop should continue to the next file. - * Returns false if the results limit is reached (caller should stop the loop) - * or if the file limit is reached for a new file. - */ -function shouldProcessFile( - allMatchesLength: number, - maxResults: number, - filesWithMatchesSize: number, - maxFiles: number, - isKnownFile: boolean, -): boolean { - if (allMatchesLength >= maxResults) { - return false; - } - if (filesWithMatchesSize >= maxFiles && !isKnownFile) { - return false; - } - return true; -} - -/** - * Processes a single file for matches during the JS fallback, accumulating - * into the shared collections. - */ -async function processFallbackFile( - filePath: string, - absolutePath: string, - regex: RegExp, - maxPerFile: number, - maxResults: number, - allMatches: GrepMatch[], - filesWithMatches: Set, -): Promise { - const fileAbsolutePath = filePath; - try { - const content = await fsPromises.readFile(fileAbsolutePath, 'utf8'); - const lines = content.split(/\r?\n/); - - return extractMatchesFromFile( - lines, - fileAbsolutePath, - absolutePath, - regex, - maxPerFile, - maxResults, - allMatches, - filesWithMatches, - ); - } catch (readError: unknown) { - // Ignore errors like permission denied or file gone during read - if (!isNodeError(readError) || readError.code !== 'ENOENT') { - debugLogger.debug( - `GrepLogic: Could not read/process ${fileAbsolutePath}: ${getErrorMessage( - readError, - )}`, - ); - } - return 0; - } -} - -/** - * Pure JavaScript fallback for grep (Strategy 3). - */ -export async function javascriptGrepFallback( - pattern: string, - absolutePath: string, - include: string | undefined, - abortSignal: AbortSignal, - maxResults: number, - maxFiles: number, - maxPerFile: number, - fileExclusions: readonly string[], -): Promise { - const globPattern = include ?? '**/*'; - const ignorePatterns = [...fileExclusions]; - - const filesStream = globStream(globPattern, { - cwd: absolutePath, - dot: true, - ignore: ignorePatterns, - absolute: true, - nodir: true, - signal: abortSignal, - }); - - const regex = new RegExp(pattern, 'i'); - const allMatches: GrepMatch[] = []; - const filesWithMatches = new Set(); - let totalFound = 0; - - for await (const filePath of filesStream) { - if ( - !shouldProcessFile( - allMatches.length, - maxResults, - filesWithMatches.size, - maxFiles, - filesWithMatches.has(filePath), - ) - ) { - // Stop entirely if we've hit the results limit; otherwise just skip - if (allMatches.length >= maxResults) { - break; - } - } else { - totalFound += await processFallbackFile( - filePath, - absolutePath, - regex, - maxPerFile, - maxResults, - allMatches, - filesWithMatches, - ); - } - } - - return { - results: allMatches, - wasLimited: totalFound > allMatches.length, - totalFound: totalFound > allMatches.length ? totalFound : undefined, - }; -} - /** * Attempts system grep (Strategy 2), returning null to fall through. */ @@ -555,14 +756,12 @@ async function trySystemGrepStrategy( maxFiles: number, maxPerFile: number, fileExclusions: readonly string[], + semanticBudget: SemanticBudget, ): Promise { debugLogger.debug( 'GrepLogic: System grep is being considered as fallback strategy.', ); - const grepAvailable = await isCommandAvailable('grep'); - if (!grepAvailable) return null; - const grepArgs = buildSystemGrepArgs(pattern, include, fileExclusions); return trySystemGrep( grepArgs, @@ -571,6 +770,7 @@ async function trySystemGrepStrategy( maxResults, maxFiles, maxPerFile, + semanticBudget, ); } @@ -604,6 +804,24 @@ export async function performSingleFileSearch( return matches; } +function snapshotBudget(budget: SemanticBudget): { + remainingBytes: number; + remainingObjects: number; +} { + return { + remainingBytes: budget.remainingBytes, + remainingObjects: budget.remainingObjects, + }; +} + +function restoreBudget( + budget: SemanticBudget, + snapshot: { remainingBytes: number; remainingObjects: number }, +): void { + budget.remainingBytes = snapshot.remainingBytes; + budget.remainingObjects = snapshot.remainingObjects; +} + /** * Performs the actual search using the prioritized strategies: * git grep → system grep → JavaScript fallback. @@ -619,16 +837,17 @@ export async function performGrepSearch( maxResults = 1000, maxFiles = 100, maxPerFile = 50, + semanticBudget = createAggregateSemanticBudget(), } = options; let strategyUsed = 'none'; try { - // --- Strategy 1: git grep --- const hasBracePattern = typeof include === 'string' && include.length > 0 && hasBraceExpansion(include); + const gitSnapshot = snapshotBudget(semanticBudget); const gitResult = await tryGitGrep( pattern, absolutePath, @@ -638,13 +857,13 @@ export async function performGrepSearch( maxFiles, maxPerFile, hasBracePattern, + semanticBudget, ); - if (gitResult !== null) { - return gitResult; - } + if (gitResult !== null) return gitResult; + restoreBudget(semanticBudget, gitSnapshot); - // --- Strategy 2: System grep --- strategyUsed = 'system grep'; + const sysSnapshot = snapshotBudget(semanticBudget); const sysResult = await trySystemGrepStrategy( pattern, absolutePath, @@ -654,12 +873,11 @@ export async function performGrepSearch( maxFiles, maxPerFile, fileExclusions, + semanticBudget, ); - if (sysResult !== null) { - return sysResult; - } + if (sysResult !== null) return sysResult; + restoreBudget(semanticBudget, sysSnapshot); - // --- Strategy 3: Pure JavaScript Fallback --- debugLogger.debug( 'GrepLogic: Falling back to JavaScript grep implementation.', ); @@ -680,6 +898,6 @@ export async function performGrepSearch( error, )}`, ); - throw error; // Re-throw + throw error; } } diff --git a/packages/tools/src/tools/grep/types.ts b/packages/tools/src/tools/grep/types.ts index 69df8e64dd..82b864dadf 100644 --- a/packages/tools/src/tools/grep/types.ts +++ b/packages/tools/src/tools/grep/types.ts @@ -1,3 +1,5 @@ +import type { SemanticBudget } from './search-strategies.js'; + /** * Shared types and constants for the grep tool sub-modules. */ @@ -18,6 +20,18 @@ export interface SearchResults { results: GrepMatch[]; wasLimited?: boolean; totalFound?: number; + /** + * Explicitly marks the search as incomplete: the true result set is + * unknown because acquisition stopped early (budget, semantic byte cap, + * or dropped lines). When true, the output must never claim an exact total. + */ + incomplete?: boolean; + /** + * Lower-bound count of all matches observed during acquisition, including + * those not retained due to per-file or aggregate limits. When incomplete + * is true, this is more accurate than results.length for aggregation. + */ + observedCount?: number; } /** @@ -31,6 +45,7 @@ export interface SearchOptions { maxResults?: number; maxFiles?: number; maxPerFile?: number; + semanticBudget?: SemanticBudget; } /** diff --git a/packages/tools/src/tools/ripGrep.ts b/packages/tools/src/tools/ripGrep.ts index 258dd7c26d..5ed2c455a5 100644 --- a/packages/tools/src/tools/ripGrep.ts +++ b/packages/tools/src/tools/ripGrep.ts @@ -6,9 +6,7 @@ import fsPromises from 'fs/promises'; import path from 'path'; -import { EOL } from 'os'; import { spawn } from 'child_process'; -// import { rgPath } from '@lvce-editor/ripgrep'; // Now using getRipgrepPath() instead import { BaseDeclarativeTool, BaseToolInvocation, @@ -18,10 +16,24 @@ import { } from './tools.js'; import type { IToolHost, IToolMessageBus } from '../interfaces/index.js'; import { SchemaValidator } from '../utils/schemaValidator.js'; +import { + BoundedCombinedCollector, + createDefaultByteBudget, + DEFAULT_ACQUISITION_BUDGET_BYTES, +} from '../acquisition/index.js'; +import { BoundedLineFramer } from '../utils/lineFramer.js'; +import { terminateProcessTree } from '../utils/processTermination.js'; +import { + createSettleFn, + type SubprocessSettlement, + type AbortHandlerRef, +} from '../utils/subprocessSettle.js'; import { makeRelative, shortenPath } from '../utils/paths.js'; import { stringOrDefault } from '../utils/stringCoalescing.js'; import { getErrorMessage } from '../utils/errors.js'; import { getRipgrepPath } from '../utils/ripgrepPathResolver.js'; +import { parseRipgrepLine } from './grep/ripgrepParse.js'; +export { parseRipgrepLine }; import { resolveTextSearchTarget, type ResolvedSearchTarget, @@ -31,6 +43,20 @@ import { debugLogger } from '../utils/debugLogger.js'; export const ripGrepDebugLogger = debugLogger; const DEFAULT_TOTAL_MAX_MATCHES = 20000; +const MATCH_OVERHEAD_BYTES = 256; +const HARD_RETAINED_MATCH_CAP = 100_000; + +interface RipgrepSemanticBudget { + remainingBytes: number; + remainingObjects: number; +} + +function createAggregateSemanticBudget(): RipgrepSemanticBudget { + return { + remainingBytes: DEFAULT_ACQUISITION_BUDGET_BYTES, + remainingObjects: HARD_RETAINED_MATCH_CAP, + }; +} /** * Parameters for the GrepTool @@ -123,6 +149,262 @@ interface GrepMatch { line: string; } +/** + * Acquisition state for a ripgrep subprocess. Uses record-at-a-time line + * consumption and bounded semantic retention. + */ +interface RipgrepAcquisitionState { + collector: BoundedCombinedCollector; + framer: BoundedLineFramer; + matches: GrepMatch[]; + retainedBytes: number; + semanticBudget: RipgrepSemanticBudget; + earlyStopped: boolean; + capReached: boolean; + budgetExhausted: boolean; + terminated: boolean; +} + +function createRipgrepAcquisitionState( + semanticBudget: RipgrepSemanticBudget, +): RipgrepAcquisitionState { + return { + collector: new BoundedCombinedCollector({ + budget: createDefaultByteBudget(), + }), + framer: new BoundedLineFramer(), + matches: [], + retainedBytes: 0, + semanticBudget, + earlyStopped: false, + capReached: false, + budgetExhausted: false, + terminated: false, + }; +} + +/** + * Attempt to retain a parsed ripgrep match in bounded semantic storage. + * Stops when the match count reaches maxMatches or the semantic byte budget + * is exhausted. + */ +function tryRetainRipgrepMatch( + state: RipgrepAcquisitionState, + match: GrepMatch, + maxMatches: number, +): void { + if (state.capReached) { + state.earlyStopped = true; + return; + } + const matchBytes = + Buffer.byteLength(match.line, 'utf8') + + Buffer.byteLength(match.filePath, 'utf8') + + MATCH_OVERHEAD_BYTES; + if ( + state.semanticBudget.remainingBytes < matchBytes || + state.semanticBudget.remainingObjects <= 0 + ) { + state.budgetExhausted = true; + state.earlyStopped = true; + return; + } + state.matches.push(match); + state.retainedBytes += matchBytes; + state.semanticBudget.remainingBytes -= matchBytes; + state.semanticBudget.remainingObjects--; + if (state.matches.length >= maxMatches) { + state.capReached = true; + } +} + +/** + * Feed a stdout chunk into the collector and framer, consuming each complete + * bounded line record-at-a-time via callback. Returns true if early stop + * or budget exhaustion was triggered. + */ +function processRipgrepStdoutChunk( + state: RipgrepAcquisitionState, + chunk: Buffer, + basePath: string, + maxMatches: number, +): boolean { + state.collector.append(chunk, 'stdout'); + if (state.terminated) return false; + + state.framer.feedChunk(chunk, (line) => { + if (state.earlyStopped) return; + const match = parseRipgrepLine(line, basePath); + if (!match) return; + tryRetainRipgrepMatch(state, match, maxMatches); + }); + + return state.earlyStopped; +} + +/** Flush remaining lines from the framer and retain bounded matches. */ +function flushRipgrepLines( + state: RipgrepAcquisitionState, + basePath: string, + maxMatches: number, +): void { + state.framer.flushRemaining((line) => { + if (state.earlyStopped) return; + const match = parseRipgrepLine(line, basePath); + if (!match) return; + tryRetainRipgrepMatch(state, match, maxMatches); + }); +} + +/** + * Resolve a ripgrep close event into a result or error. An unexpected signal + * kill (code null with a non-intentional signal) is treated as genuine failure. + */ +function resolveRipgrepClose( + code: number | null, + signal: NodeJS.Signals | null, + state: RipgrepAcquisitionState, + basePath: string, + maxMatches: number, + aborted: boolean, +): { + readonly result?: { + matches: GrepMatch[]; + earlyStopped: boolean; + budgetTruncated: boolean; + lineDropped: boolean; + }; + readonly error?: Error; +} { + if (!state.terminated) { + flushRipgrepLines(state, basePath, maxMatches); + } + const acquisition = state.collector.getResult(); + const result = { + matches: state.matches, + earlyStopped: state.earlyStopped, + budgetTruncated: acquisition.metadata.truncated || state.budgetExhausted, + lineDropped: state.framer.wasLineDropped, + }; + if (state.earlyStopped || aborted || state.terminated) { + return { result }; + } + if (signal !== null) { + return { + error: new Error(`ripgrep was killed by signal ${signal}`), + }; + } + if (code !== null && code !== 0 && code !== 1) { + return { + error: new Error( + `ripgrep exited with code ${code}: ${acquisition.stderrText.trim()}`, + ), + }; + } + if (code === null) { + return { + error: new Error('ripgrep closed unexpectedly'), + }; + } + return { result }; +} + +/** Build the standard ripgrep spawn-failure error. */ +function ripgrepSpawnError(err: Error): Error { + return new Error( + `Failed to start ripgrep: ${err.message}. Please ensure @lvce-editor/ripgrep is properly installed.`, + ); +} + +/** Result of a ripgrep subprocess. */ +interface RipgrepSubprocessResult { + matches: GrepMatch[]; + earlyStopped: boolean; + budgetTruncated: boolean; + lineDropped: boolean; +} + +/** Create a ripgrep AbortError recognised by upstream callers. */ +function ripgrepAbortError(): Error { + const err = new Error('ripgrep aborted'); + err.name = 'AbortError'; + return err; +} + +/** Spawn and manage a ripgrep child process with bounded acquisition. */ +function runRipgrepChild( + resolvedRgPath: string, + rgArgs: string[], + signal: AbortSignal, + state: RipgrepAcquisitionState, + basePath: string, + maxMatches: number, +): Promise { + return new Promise((resolve, reject) => { + const settlement: SubprocessSettlement = { + settled: false, + terminationPromise: null, + }; + const abortRef: AbortHandlerRef = { handler: () => {} }; + + const child = spawn(resolvedRgPath, rgArgs, { + windowsHide: true, + detached: process.platform !== 'win32', + }); + + const stopProcess = () => { + if (state.terminated) return; + state.terminated = true; + settlement.terminationPromise = terminateProcessTree(child, { + ownsProcessGroup: process.platform !== 'win32', + }); + }; + + const settle = createSettleFn( + settlement, + signal, + abortRef, + reject, + Error, + 'ripgrep', + ); + + abortRef.handler = () => { + stopProcess(); + settle(() => reject(ripgrepAbortError())); + }; + signal.addEventListener('abort', abortRef.handler); + + child.stdout.on('data', (chunk: Buffer) => { + if (processRipgrepStdoutChunk(state, chunk, basePath, maxMatches)) { + stopProcess(); + } + }); + child.stderr.on('data', (chunk: Buffer) => { + state.collector.append(chunk, 'stderr'); + }); + child.on('error', (err: Error) => { + settle(() => { + reject(signal.aborted ? ripgrepAbortError() : ripgrepSpawnError(err)); + }); + }); + child.on('close', (code: number | null, sig: NodeJS.Signals | null) => { + settle(() => { + const outcome = resolveRipgrepClose( + code, + sig, + state, + basePath, + maxMatches, + signal.aborted, + ); + if (outcome.error !== undefined) reject(outcome.error); + else if (outcome.result !== undefined) resolve(outcome.result); + }); + }); + }); +} + class GrepToolInvocation extends BaseToolInvocation< RipGrepToolParams, ToolResult @@ -213,7 +495,7 @@ File: ${resolved.basename} private collectDirectoryMatches( searchDirectories: readonly string[], signal: AbortSignal, - ): Promise { + ): Promise<{ matches: GrepMatch[]; wasTruncated: boolean }> { return this.collectDirectoryMatchesImpl( searchDirectories, signal, @@ -227,38 +509,60 @@ File: ${resolved.basename} signal: AbortSignal, totalMaxMatches: number, ignoreOptions: RipgrepIgnoreOptions, - ): Promise { + ): Promise<{ matches: GrepMatch[]; wasTruncated: boolean }> { let allMatches: GrepMatch[] = []; + let wasTruncated = false; + const aggregateBudget = createAggregateSemanticBudget(); if (this.host.getDebugMode()) { debugLogger.debug(`[GrepTool] Total result limit: ${totalMaxMatches}`); } - for (const searchDir of searchDirectories) { + let stop = false; + for (let di = 0; di < searchDirectories.length && !stop; di++) { + const searchDir = searchDirectories[di]; + const remaining = totalMaxMatches - allMatches.length; + if (remaining <= 0) { + allMatches = allMatches.slice(0, totalMaxMatches); + stop = true; + continue; + } + const searchResult = await this.performRipgrepSearch({ pattern: this.params.pattern, path: searchDir, include: this.params.include, signal, ignoreOptions, + maxMatches: remaining, + semanticBudget: aggregateBudget, }); + if ( + searchResult.earlyStopped || + searchResult.budgetTruncated || + searchResult.lineDropped + ) { + wasTruncated = true; + } + if (searchDirectories.length > 1) { const dirName = path.basename(searchDir); - searchResult.forEach((match) => { + searchResult.matches.forEach((match) => { match.filePath = path.join(dirName, match.filePath); }); } - allMatches = allMatches.concat(searchResult); + allMatches = allMatches.concat(searchResult.matches); if (allMatches.length >= totalMaxMatches) { allMatches = allMatches.slice(0, totalMaxMatches); - break; } + stop = + allMatches.length >= totalMaxMatches || searchResult.budgetTruncated; } - return allMatches; + return { matches: allMatches, wasTruncated }; } private buildSearchLocationDescription( @@ -278,15 +582,25 @@ File: ${resolved.basename} private formatDirectoryResults( allMatches: GrepMatch[], searchLocationDescription: string, + dirWasTruncated: boolean, ): ToolResult { - const totalMaxMatches = DEFAULT_TOTAL_MAX_MATCHES; + const includeNote = this.params.include + ? ` (filter: "${this.params.include}")` + : ''; if (allMatches.length === 0) { - const noMatchMsg = `No matches found for pattern "${this.params.pattern}" ${searchLocationDescription}${this.params.include ? ` (filter: "${this.params.include}")` : ''}.`; + if (dirWasTruncated) { + const msg = `No matches retained for pattern "${this.params.pattern}" ${searchLocationDescription}${includeNote}. Results may be incomplete.`; + return { + llmContent: msg, + returnDisplay: 'No matches shown (incomplete)', + }; + } + const noMatchMsg = `No matches found for pattern "${this.params.pattern}" ${searchLocationDescription}${includeNote}.`; return { llmContent: noMatchMsg, returnDisplay: `No matches found` }; } - const wasTruncated = allMatches.length >= totalMaxMatches; + const wasTruncated = dirWasTruncated; const matchesByFile = allMatches.reduce( (acc, match) => { @@ -301,12 +615,16 @@ File: ${resolved.basename} ); const matchCount = allMatches.length; - const matchTerm = matchCount === 1 ? 'match' : 'matches'; - - let llmContent = `Found ${matchCount} ${matchTerm} for pattern "${this.params.pattern}" ${searchLocationDescription}${this.params.include ? ` (filter: "${this.params.include}")` : ''}`; + let llmContent: string; + let displayMessage: string; if (wasTruncated) { - llmContent += ` (results limited to ${totalMaxMatches} matches for performance)`; + llmContent = `Showing ${matchCount} matches for pattern "${this.params.pattern}" ${searchLocationDescription}${includeNote} (results may be incomplete)`; + displayMessage = `Showing ${matchCount} matches (results may be incomplete)`; + } else { + const matchTerm = matchCount === 1 ? 'match' : 'matches'; + llmContent = `Found ${matchCount} ${matchTerm} for pattern "${this.params.pattern}" ${searchLocationDescription}${includeNote}`; + displayMessage = `Found ${matchCount} ${matchTerm}`; } llmContent += `:\n---\n`; @@ -320,11 +638,6 @@ File: ${resolved.basename} llmContent += '---\n'; } - let displayMessage = `Found ${matchCount} ${matchTerm}`; - if (wasTruncated) { - displayMessage += ` (limited)`; - } - return { llmContent: llmContent.trim(), returnDisplay: displayMessage, @@ -350,10 +663,8 @@ File: ${resolved.basename} searchDirAbs, workspaceContext, ); - const allMatches = await this.collectDirectoryMatches( - searchDirectories, - signal, - ); + const { matches: allMatches, wasTruncated: dirWasTruncated } = + await this.collectDirectoryMatches(searchDirectories, signal); const searchLocationDescription = this.buildSearchLocationDescription( searchDirAbs, @@ -361,7 +672,11 @@ File: ${resolved.basename} workspaceContext, ); - return this.formatDirectoryResults(allMatches, searchLocationDescription); + return this.formatDirectoryResults( + allMatches, + searchLocationDescription, + dirWasTruncated, + ); } catch (error) { debugLogger.warn(`Error during GrepLogic execution: ${error}`); const errorMessage = getErrorMessage(error); @@ -372,69 +687,24 @@ File: ${resolved.basename} } } - private parseRipgrepOutput(output: string, basePath: string): GrepMatch[] { - const results: GrepMatch[] = []; - if (!output) return results; - - const lines = output.split(EOL); - - for (const line of lines) { - const match = parseRipgrepLine(line, basePath); - if (match) { - results.push(match); - } - } - return results; - } - private async runRipgrepProcess( rgArgs: string[], signal: AbortSignal, - ): Promise { + maxMatches: number, + semanticBudget: RipgrepSemanticBudget, + ): Promise { const resolvedRgPath = await getRipgrepPath(); - - return new Promise((resolve, reject) => { - const child = spawn(resolvedRgPath, rgArgs, { - windowsHide: true, - }); - - const stdoutChunks: Buffer[] = []; - const stderrChunks: Buffer[] = []; - - const cleanup = () => { - if (signal.aborted) { - child.kill(); - } - }; - - signal.addEventListener('abort', cleanup, { once: true }); - - child.stdout.on('data', (chunk) => stdoutChunks.push(chunk)); - child.stderr.on('data', (chunk) => stderrChunks.push(chunk)); - - child.on('error', (err) => { - signal.removeEventListener('abort', cleanup); - reject( - new Error( - `Failed to start ripgrep: ${err.message}. Please ensure @lvce-editor/ripgrep is properly installed.`, - ), - ); - }); - - child.on('close', (code) => { - signal.removeEventListener('abort', cleanup); - const stdoutData = Buffer.concat(stdoutChunks).toString('utf8'); - const stderrData = Buffer.concat(stderrChunks).toString('utf8'); - - if (code === 0) { - resolve(stdoutData); - } else if (code === 1) { - resolve(''); // No matches found - } else { - reject(new Error(`ripgrep exited with code ${code}: ${stderrData}`)); - } - }); - }); + if (signal.aborted) throw ripgrepAbortError(); + const basePath = rgArgs[rgArgs.length - 1] ?? ''; + const state = createRipgrepAcquisitionState(semanticBudget); + return runRipgrepChild( + resolvedRgPath, + rgArgs, + signal, + state, + basePath, + maxMatches, + ); } private async performRipgrepSearch(options: { @@ -443,13 +713,22 @@ File: ${resolved.basename} include?: string; signal: AbortSignal; ignoreOptions: RipgrepIgnoreOptions; - }): Promise { + maxMatches: number; + semanticBudget: RipgrepSemanticBudget; + }): Promise<{ + matches: GrepMatch[]; + earlyStopped: boolean; + budgetTruncated: boolean; + lineDropped: boolean; + }> { const { pattern, path: absolutePath, include, signal, ignoreOptions, + maxMatches, + semanticBudget, } = options; const rgArgs = buildRipgrepArgs( @@ -460,8 +739,12 @@ File: ${resolved.basename} ); try { - const output = await this.runRipgrepProcess(rgArgs, signal); - return this.parseRipgrepOutput(output, absolutePath); + return await this.runRipgrepProcess( + rgArgs, + signal, + maxMatches, + semanticBudget, + ); } catch (error: unknown) { debugLogger.debug(`GrepLogic: ripgrep failed: ${getErrorMessage(error)}`); throw error; @@ -640,50 +923,3 @@ export class RipGrepTool extends BaseDeclarativeTool< return this.build(params).execute(signal); } } - -/** - * Parses a single ripgrep output line into a GrepMatch, or returns null - * if the line is blank or malformed. - */ -export function parseRipgrepLine( - line: string, - basePath: string, -): GrepMatch | null { - if (!line.trim()) { - return null; - } - - const nullSeparatorIndex = line.indexOf('\0'); - const pathSeparatorIndex = - nullSeparatorIndex === -1 ? line.indexOf(':') : nullSeparatorIndex; - if (pathSeparatorIndex === -1) { - return null; - } - - const lineNumberStartIndex = pathSeparatorIndex + 1; - const contentSeparatorIndex = line.indexOf(':', lineNumberStartIndex); - if (contentSeparatorIndex === -1) { - return null; - } - - const filePathRaw = line.substring(0, pathSeparatorIndex); - const lineNumberStr = line.substring( - lineNumberStartIndex, - contentSeparatorIndex, - ); - const lineContent = line.substring(contentSeparatorIndex + 1); - - const lineNumber = parseInt(lineNumberStr, 10); - if (isNaN(lineNumber)) { - return null; - } - - const absoluteFilePath = path.resolve(basePath, filePathRaw); - const relativeFilePath = path.relative(basePath, absoluteFilePath); - - return { - filePath: relativeFilePath || path.basename(absoluteFilePath), - lineNumber, - line: lineContent, - }; -} diff --git a/packages/tools/src/tools/tool-registry.ts b/packages/tools/src/tools/tool-registry.ts index 9b2e1be4e2..b6995a0fd8 100644 --- a/packages/tools/src/tools/tool-registry.ts +++ b/packages/tools/src/tools/tool-registry.ts @@ -28,6 +28,17 @@ import { isToolBlocked, type ToolGovernance, } from '../formatters/toolGovernanceUtils.js'; +import { + BoundedCombinedCollector, + createDefaultByteBudget, + type CombinedAcquisitionResult, +} from '../acquisition/index.js'; +import { + terminateProcessTree, + type ProcessTerminationResult, +} from '../utils/processTermination.js'; + +const STREAM_DRAIN_TIMEOUT_MS = 2000; export const DISCOVERED_TOOL_PREFIX = 'discovered_tool_'; @@ -123,124 +134,201 @@ Signal: Signal number or \`(none)\` if no signal was received. signal: AbortSignal, _updateOutput?: (update: LiveOutputUpdate) => void, ): Promise { + if (signal.aborted) { + return { + llmContent: 'Tool execution was cancelled by user.', + returnDisplay: 'Cancelled', + error: { + message: 'Tool execution was cancelled by user.', + type: ToolErrorType.DISCOVERED_TOOL_EXECUTION_ERROR, + }, + }; + } const callCommand = this.config.getToolCallCommand?.() ?? ''; - const child = spawn(callCommand, [this.name]); - child.stdin.write(JSON.stringify(params)); - child.stdin.end(); + const child = spawn(callCommand, [this.name], { + windowsHide: true, + detached: process.platform !== 'win32', + }); - const { stdout, stderr, error, code, exitSignal } = - await this.runChildProcess(child, signal); + const { acquisition, error, code, exitSignal, terminationOutcome } = + await this.runChildProcess(child, signal, params); return this.buildChildProcessResult( - stdout, - stderr, + acquisition, error, code, exitSignal, + terminationOutcome, ); } private async runChildProcess( child: ReturnType, signal: AbortSignal, + params: ToolParams, ): Promise<{ - stdout: string; - stderr: string; + acquisition: CombinedAcquisitionResult; error: Error | null; code: number | null; exitSignal: NodeJS.Signals | null; + terminationOutcome: ProcessTerminationResult['outcome'] | null; }> { - let stdout = ''; - let stderr = ''; - let error: Error | null = null; - let code: number | null = null; - let exitSignal: NodeJS.Signals | null = null; + const collector = new BoundedCombinedCollector({ + budget: createDefaultByteBudget(), + }); + let terminationPromise: Promise | null = null; const abortHandler = () => { - if (!child.killed) { - child.kill('SIGTERM'); - } + terminationPromise ??= terminateProcessTree(child, { + ownsProcessGroup: true, + }); }; signal.addEventListener('abort', abortHandler); - try { - await new Promise((resolve) => { - const onStdout = (data: Buffer) => { - stdout += data.toString(); - }; - - const onStderr = (data: Buffer) => { - stderr += data.toString(); - }; - - const onError = (err: Error) => { - error = err; - }; - - const onClose = ( - _code: number | null, - _signal: NodeJS.Signals | null, - ) => { - code = _code; - exitSignal = _signal; - cleanup(); - resolve(); - }; - - const cleanup = () => { - child.stdout!.removeListener('data', onStdout); - child.stderr!.removeListener('data', onStderr); - child.removeListener('error', onError); - child.removeListener('close', onClose); - if (child.connected) { - child.disconnect(); - } - }; - - child.stdout!.on('data', onStdout); - child.stderr!.on('data', onStderr); - child.on('error', onError); - child.on('close', onClose); + const { error, code, exitSignal } = await this.awaitProcessSettlement( + child, + collector, + params, + ); + + signal.removeEventListener('abort', abortHandler); + + let terminationOutcome: ProcessTerminationResult['outcome'] | null = null; + if (child.exitCode === null && child.signalCode === null) { + terminationPromise ??= terminateProcessTree(child, { + ownsProcessGroup: true, }); - } finally { - signal.removeEventListener('abort', abortHandler); } - return { stdout, stderr, error, code, exitSignal }; + if (terminationPromise !== null) { + const result = await terminationPromise; + terminationOutcome = result.outcome; + } + + return { + acquisition: collector.getResult(), + error, + code, + exitSignal, + terminationOutcome, + }; + } + + private awaitProcessSettlement( + child: ReturnType, + collector: BoundedCombinedCollector, + params: ToolParams, + ): Promise<{ + error: Error | null; + code: number | null; + exitSignal: NodeJS.Signals | null; + }> { + let error: Error | null = null; + let code: number | null = null; + let exitSignal: NodeJS.Signals | null = null; + + return new Promise((resolve) => { + let settled = false; + let drainTimer: ReturnType | null = null; + + const settle = () => { + if (settled) return; + settled = true; + if (drainTimer !== null) clearTimeout(drainTimer); + child.stdout?.removeListener('data', onStdout); + child.stderr?.removeListener('data', onStderr); + child.stdin?.removeListener('error', onStdinError); + child.removeListener('error', onError); + child.removeListener('exit', onExit); + child.removeListener('close', onClose); + resolve({ error, code, exitSignal }); + }; + + const onStdout = (data: Buffer) => collector.append(data, 'stdout'); + const onStderr = (data: Buffer) => collector.append(data, 'stderr'); + const captureFirstError = (err: Error) => { + error ??= err; + settle(); + }; + const onStdinError = captureFirstError; + const onError = captureFirstError; + const onExit = (c: number | null, s: NodeJS.Signals | null) => { + if (settled) return; + code = c; + exitSignal = s; + drainTimer = setTimeout(() => settle(), STREAM_DRAIN_TIMEOUT_MS); + }; + const onClose = (c: number | null, s: NodeJS.Signals | null) => { + code ??= c; + exitSignal ??= s; + settle(); + }; + + child.stdout?.on('data', onStdout); + child.stderr?.on('data', onStderr); + child.stdin?.on('error', onStdinError); + child.on('error', onError); + child.on('exit', onExit); + child.on('close', onClose); + + if (child.stdin !== null) { + try { + child.stdin.write(JSON.stringify(params)); + child.stdin.end(); + } catch (e) { + captureFirstError(e instanceof Error ? e : new Error(String(e))); + } + } + }); } private buildChildProcessResult( - stdout: string, - stderr: string, + acquisition: CombinedAcquisitionResult, error: Error | null, code: number | null, exitSignal: NodeJS.Signals | null, + terminationOutcome: ProcessTerminationResult['outcome'] | null, ): ToolResult { - if ( - error !== null || - code !== 0 || - exitSignal !== null || - stderr.length > 0 - ) { + const stdout = acquisition.stdoutText; + const stderr = acquisition.stderrText; + const truncated = acquisition.metadata.truncated; + const truncationNotice = acquisition.omissionNotice ?? ''; + + const terminationFailed = + terminationOutcome === 'timeout' || terminationOutcome === 'failure'; + const isExecutionError = + error !== null || code !== 0 || exitSignal !== null; + + if (isExecutionError || stderr.length > 0 || terminationFailed) { + const stdoutLine = `Stdout: ${stdout.length > 0 ? stdout : '(empty)'}`; + const stderrLine = `Stderr: ${stderr.length > 0 ? stderr : '(empty)'}`; + const terminationLine = terminationFailed + ? `\nTermination: ${terminationOutcome}` + : ''; + const truncationLine = truncated ? `\n${truncationNotice}` : ''; const llmContent = [ - `Stdout: ${stdout.length > 0 ? stdout : '(empty)'}`, - `Stderr: ${stderr.length > 0 ? stderr : '(empty)'}`, + stdoutLine, + stderrLine, `Error: ${error ?? '(none)'}`, `Exit Code: ${code ?? '(none)'}`, `Signal: ${exitSignal ?? '(none)'}`, ].join('\n'); + + const fullContent = llmContent + terminationLine + truncationLine; return { - llmContent, - returnDisplay: llmContent, + llmContent: fullContent, + returnDisplay: fullContent, error: { - message: llmContent, + message: fullContent, type: ToolErrorType.DISCOVERED_TOOL_EXECUTION_ERROR, }, }; } + const llmContent = truncated ? `${stdout}\n\n${truncationNotice}` : stdout; + return { - llmContent: stdout, - returnDisplay: stdout, + llmContent, + returnDisplay: llmContent, }; } } diff --git a/packages/tools/src/utils/lineFramer.test.ts b/packages/tools/src/utils/lineFramer.test.ts new file mode 100644 index 0000000000..deeaed1613 --- /dev/null +++ b/packages/tools/src/utils/lineFramer.test.ts @@ -0,0 +1,490 @@ +/** + * @license + * Copyright 2026 Vybestack LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect } from 'bun:test'; +import { BoundedLineFramer } from './lineFramer.js'; + +function feedCollect(framer: BoundedLineFramer, chunk: Buffer): string[] { + const lines: string[] = []; + framer.feedChunk(chunk, (line) => lines.push(line)); + return lines; +} + +function flushCollect(framer: BoundedLineFramer): string[] { + const lines: string[] = []; + framer.flushRemaining((line) => lines.push(line)); + return lines; +} + +describe('BoundedLineFramer - LF line splitting', () => { + it('emits complete LF-terminated lines', () => { + const framer = new BoundedLineFramer(); + expect(feedCollect(framer, Buffer.from('hello\nworld\n'))).toEqual([ + 'hello', + 'world', + ]); + }); + + it('retains a partial line across feed calls', () => { + const framer = new BoundedLineFramer(); + expect(feedCollect(framer, Buffer.from('hel'))).toEqual([]); + expect(feedCollect(framer, Buffer.from('lo\nwor'))).toEqual(['hello']); + expect(feedCollect(framer, Buffer.from('ld\n'))).toEqual(['world']); + }); + + it('flushRemaining returns the unterminated partial as a final line', () => { + const framer = new BoundedLineFramer(); + feedCollect(framer, Buffer.from('hello\nworld')); + expect(flushCollect(framer)).toEqual(['world']); + }); + + it('flushRemaining returns empty when nothing remains', () => { + const framer = new BoundedLineFramer(); + feedCollect(framer, Buffer.from('hello\nworld\n')); + expect(flushCollect(framer)).toEqual([]); + }); +}); + +describe('BoundedLineFramer - LF is the sole delimiter', () => { + it('preserves lone CR as content inside a line', () => { + const framer = new BoundedLineFramer(); + expect(feedCollect(framer, Buffer.from('a\rb\r\n'))).toEqual(['a\rb']); + }); + + it('preserves lone CR in a line with no trailing newline', () => { + const framer = new BoundedLineFramer(); + const lines = feedCollect(framer, Buffer.from('x\ry')); + expect(lines).toEqual([]); + expect(flushCollect(framer)).toEqual(['x\ry']); + }); + + it('strips exactly one CR before LF for CRLF', () => { + const framer = new BoundedLineFramer(); + expect(feedCollect(framer, Buffer.from('a\r\nb\r\n'))).toEqual(['a', 'b']); + }); + + it('does not strip CR when not immediately before LF', () => { + const framer = new BoundedLineFramer(); + expect(feedCollect(framer, Buffer.from('a\r \nb\n'))).toEqual([ + 'a\r ', + 'b', + ]); + }); + + it('handles CRLF split across chunk boundaries', () => { + const framer = new BoundedLineFramer(); + const first = feedCollect(framer, Buffer.from('hello\r')); + const second = feedCollect(framer, Buffer.from('\nworld\r\n')); + expect([...first, ...second]).toEqual(['hello', 'world']); + }); + + it('handles mixed LF and CRLF in one stream', () => { + const framer = new BoundedLineFramer(); + expect(feedCollect(framer, Buffer.from('a\nb\r\nc\nd\r\n'))).toEqual([ + 'a', + 'b', + 'c', + 'd', + ]); + }); + + it('preserves CR that appears between two LF-terminated records', () => { + const framer = new BoundedLineFramer(); + expect(feedCollect(framer, Buffer.from('line1\n\r\nline3\n'))).toEqual([ + 'line1', + '', + 'line3', + ]); + }); + + it('CR at end of chunk followed by non-LF keeps CR as content', () => { + const framer = new BoundedLineFramer(); + const first = feedCollect(framer, Buffer.from('hello\r')); + const second = feedCollect(framer, Buffer.from('world\n')); + expect([...first, ...second]).toEqual(['hello\rworld']); + }); + + it('preserves CR CR LF (two CR before LF strips only the last)', () => { + const framer = new BoundedLineFramer(); + expect(feedCollect(framer, Buffer.from('a\r\r\n'))).toEqual(['a\r']); + }); + + it('does NOT emit a lone CR as its own record', () => { + const framer = new BoundedLineFramer(); + expect(feedCollect(framer, Buffer.from('hello\r'))).toEqual([]); + expect(flushCollect(framer)).toEqual(['hello\r']); + }); +}); + +describe('BoundedLineFramer - empty records', () => { + it('emits an empty record for consecutive LF', () => { + const framer = new BoundedLineFramer(); + expect(feedCollect(framer, Buffer.from('\n\n'))).toEqual(['', '']); + }); + + it('emits an empty record for empty CRLF line', () => { + const framer = new BoundedLineFramer(); + expect(feedCollect(framer, Buffer.from('\r\n'))).toEqual(['']); + }); + + it('emits empty record at the start of input', () => { + const framer = new BoundedLineFramer(); + expect(feedCollect(framer, Buffer.from('\nhello\n'))).toEqual([ + '', + 'hello', + ]); + }); +}); + +describe('BoundedLineFramer - multibyte UTF-8 across chunk boundaries', () => { + it('reassembles a multibyte character split across chunks', () => { + const framer = new BoundedLineFramer(); + feedCollect(framer, Buffer.from([0xe4, 0xb8])); + const lines = feedCollect(framer, Buffer.from([0x96, 0x0a])); + expect(lines).toEqual(['世']); + }); + + it('handles multibyte chars mixed with ASCII across chunks', () => { + const framer = new BoundedLineFramer(); + feedCollect(framer, Buffer.from([0x68, 0xc3])); + const lines = feedCollect( + framer, + Buffer.from([0xa9, 0x6c, 0x6c, 0x6f, 0x0a]), + ); + expect(lines).toEqual(['héllo']); + expect(flushCollect(framer)).toEqual([]); + }); + + it('handles emoji (4-byte) split across chunks', () => { + const framer = new BoundedLineFramer(); + feedCollect(framer, Buffer.from([0xf0, 0x9f])); + const lines = feedCollect(framer, Buffer.from([0x98, 0x80, 0x0a])); + expect(lines).toEqual(['😀']); + }); + + it('does not emit replacement characters for valid multibyte sequences', () => { + const framer = new BoundedLineFramer(); + const chars = 'café 世界 '; + const bytes = Buffer.from(chars + '\n', 'utf-8'); + const allLines: string[] = []; + for (let i = 0; i < bytes.length; i++) { + framer.feedChunk(bytes.subarray(i, i + 1), (line) => allLines.push(line)); + } + framer.flushRemaining((line) => allLines.push(line)); + expect(allLines.length).toBe(1); + expect(allLines[0]).toBe(chars); + expect(allLines[0]).not.toContain('\uFFFD'); + }); +}); + +describe('BoundedLineFramer - invalid UTF-8 fatal decoding', () => { + it('drops an invalid UTF-8 record and sets wasLineDropped', () => { + const framer = new BoundedLineFramer(); + const lines = feedCollect(framer, Buffer.from([0xff, 0xfe, 0x0a])); + expect(lines).toEqual([]); + expect(framer.wasLineDropped).toBe(true); + }); + + it('does not emit U+FFFD for invalid bytes', () => { + const framer = new BoundedLineFramer(); + const lines: string[] = []; + framer.feedChunk(Buffer.from([0xff, 0x0a]), (line) => { + expect(line).not.toContain('\uFFFD'); + lines.push(line); + }); + expect(lines).toEqual([]); + expect(framer.wasLineDropped).toBe(true); + }); + + it('continues parsing after dropping an invalid record', () => { + const framer = new BoundedLineFramer(); + const lines = feedCollect( + framer, + Buffer.from([0xff, 0x0a, 0x67, 0x6f, 0x6f, 0x64, 0x0a]), + ); + expect(lines).toEqual(['good']); + expect(framer.wasLineDropped).toBe(true); + }); + + it('flushRemaining drops an invalid unterminated partial', () => { + const framer = new BoundedLineFramer(); + feedCollect(framer, Buffer.from([0xff, 0xfe])); + expect(flushCollect(framer)).toEqual([]); + expect(framer.wasLineDropped).toBe(true); + }); +}); + +describe('BoundedLineFramer - oversized line discard', () => { + it('discards an oversized record continuously until its real LF delimiter', () => { + const framer = new BoundedLineFramer({ maxLineBytes: 50 }); + feedCollect(framer, Buffer.alloc(100, 65)); + const lines = feedCollect( + framer, + Buffer.from('phantom_match:42:data\ngenuine_line\n'), + ); + expect(lines).toEqual(['genuine_line']); + expect(lines).not.toContain('phantom_match:42:data'); + expect(framer.wasLineDropped).toBe(true); + }); + + it('discards across many tiny chunks until the delimiter', () => { + const framer = new BoundedLineFramer({ maxLineBytes: 20 }); + for (let i = 0; i < 10; i++) { + feedCollect(framer, Buffer.alloc(3, 65)); + } + expect(framer.wasLineDropped).toBe(true); + const lines = feedCollect(framer, Buffer.from('\nreal\n')); + expect(lines).toEqual(['real']); + }); + + it('does not emit any suffix of an oversized record from flushRemaining', () => { + const framer = new BoundedLineFramer({ maxLineBytes: 10 }); + feedCollect(framer, Buffer.alloc(30, 65)); + expect(flushCollect(framer)).toEqual([]); + expect(framer.wasLineDropped).toBe(true); + }); + + it('emits a new record after the oversized one is fully discarded', () => { + const framer = new BoundedLineFramer({ maxLineBytes: 10 }); + feedCollect(framer, Buffer.alloc(20, 65)); + const lines = feedCollect(framer, Buffer.from('\ngood\n')); + expect(lines).toEqual(['good']); + expect(framer.wasLineDropped).toBe(true); + }); + + it('keeps a partial line exactly at the limit', () => { + const framer = new BoundedLineFramer({ maxLineBytes: 10 }); + feedCollect(framer, Buffer.alloc(10, 65)); + expect(framer.wasLineDropped).toBe(false); + const lines = feedCollect(framer, Buffer.from('\n')); + expect(lines).toEqual([Buffer.alloc(10, 65).toString()]); + }); + + it('rejects maxLineBytes+1 content bytes', () => { + const framer = new BoundedLineFramer({ maxLineBytes: 10 }); + feedCollect(framer, Buffer.alloc(11, 65)); + expect(framer.wasLineDropped).toBe(true); + const lines = feedCollect(framer, Buffer.from('\ngood\n')); + expect(lines).toEqual(['good']); + }); + + it('accepts exactly maxLineBytes content with CRLF terminator', () => { + const framer = new BoundedLineFramer({ maxLineBytes: 10 }); + const content = Buffer.alloc(10, 65); + feedCollect(framer, content); + expect(framer.wasLineDropped).toBe(false); + const lines = feedCollect(framer, Buffer.from('\r\n')); + expect(lines).toEqual([content.toString()]); + expect(framer.wasLineDropped).toBe(false); + }); + + it('accepts exactly maxLineBytes content with CRLF split across chunks', () => { + const framer = new BoundedLineFramer({ maxLineBytes: 10 }); + const content = Buffer.alloc(10, 65); + feedCollect(framer, content); + const first = feedCollect(framer, Buffer.from('\r')); + const second = feedCollect(framer, Buffer.from('\n')); + expect([...first, ...second]).toEqual([content.toString()]); + expect(framer.wasLineDropped).toBe(false); + }); + + it('discards maxLineBytes+1 content with CRLF terminator', () => { + const framer = new BoundedLineFramer({ maxLineBytes: 10 }); + feedCollect(framer, Buffer.alloc(11, 65)); + expect(framer.wasLineDropped).toBe(true); + const lines = feedCollect(framer, Buffer.from('\r\ngood\r\n')); + expect(lines).toEqual(['good']); + }); + + it('handles multiple oversized records in sequence', () => { + const framer = new BoundedLineFramer({ maxLineBytes: 10 }); + feedCollect(framer, Buffer.alloc(20, 65)); + feedCollect(framer, Buffer.from('\n')); + feedCollect(framer, Buffer.alloc(20, 66)); + const lines = feedCollect(framer, Buffer.from('\nok\n')); + expect(lines).toEqual(['ok']); + expect(framer.wasLineDropped).toBe(true); + }); +}); + +describe('BoundedLineFramer - callback safety', () => { + it('resets internal state before invoking callback so reentrancy cannot corrupt', () => { + const framer = new BoundedLineFramer(); + const seen: string[] = []; + framer.feedChunk(Buffer.from('first\nsecond\n'), (line) => { + seen.push(line); + framer.feedChunk(Buffer.from('inner\n'), (rl) => seen.push(rl)); + }); + expect(seen).toEqual(['first', 'inner', 'second', 'inner']); + }); + + it('survives a throwing callback without corrupting state', () => { + const framer = new BoundedLineFramer(); + let callCount = 0; + expect(() => { + framer.feedChunk(Buffer.from('a\nb\n'), () => { + callCount++; + if (callCount === 1) throw new Error('boom'); + }); + }).toThrow('boom'); + expect(callCount).toBe(1); + const lines = feedCollect(framer, Buffer.from('c\n')); + expect(lines).toEqual(['c']); + }); +}); + +describe('BoundedLineFramer - maxLineBytes validation', () => { + it('throws RangeError for zero', () => { + expect(() => new BoundedLineFramer({ maxLineBytes: 0 })).toThrow( + RangeError, + ); + }); + + it('throws RangeError for negative', () => { + expect(() => new BoundedLineFramer({ maxLineBytes: -1 })).toThrow( + RangeError, + ); + }); + + it('throws RangeError for NaN', () => { + expect(() => new BoundedLineFramer({ maxLineBytes: NaN })).toThrow( + RangeError, + ); + }); + + it('throws RangeError for Infinity', () => { + expect(() => new BoundedLineFramer({ maxLineBytes: Infinity })).toThrow( + RangeError, + ); + }); + + it('throws RangeError for fractional', () => { + expect(() => new BoundedLineFramer({ maxLineBytes: 1.5 })).toThrow( + RangeError, + ); + }); + + it('throws RangeError for unsafe integer', () => { + expect( + () => + new BoundedLineFramer({ maxLineBytes: Number.MAX_SAFE_INTEGER + 1 }), + ).toThrow(RangeError); + }); + + it('throws RangeError for value exceeding hard cap', () => { + expect( + () => new BoundedLineFramer({ maxLineBytes: 17 * 1024 * 1024 }), + ).toThrow(RangeError); + }); + + it('accepts the default', () => { + expect(() => new BoundedLineFramer()).not.toThrow(); + }); + + it('accepts a valid positive safe integer', () => { + expect(() => new BoundedLineFramer({ maxLineBytes: 42 })).not.toThrow(); + }); +}); + +describe('BoundedLineFramer - grep-like output', () => { + it('parses git-grep style lines incrementally', () => { + const framer = new BoundedLineFramer(); + const output = Buffer.from( + 'src/index.ts:1:const x = 1;\nsrc/util.ts:5:export function foo() {\n', + ); + expect(feedCollect(framer, output)).toEqual([ + 'src/index.ts:1:const x = 1;', + 'src/util.ts:5:export function foo() {', + ]); + }); + + it('handles a huge single line then many small lines (bounded)', () => { + const framer = new BoundedLineFramer({ maxLineBytes: 1024 * 1024 }); + const huge = 'x'.repeat(100_000) + '\n'; + let smalls = ''; + for (let i = 0; i < 1000; i++) { + smalls += `file.ts:${i}:match\n`; + } + const lines = feedCollect(framer, Buffer.from(huge + smalls)); + expect(lines.length).toBe(1001); + expect(lines[0].length).toBe(100_000); + expect(lines[1]).toBe('file.ts:0:match'); + expect(framer.wasLineDropped).toBe(false); + }); + + it('does not materialize the full stream as a single array (callback API)', () => { + const framer = new BoundedLineFramer(); + const seen: string[] = []; + const output = Buffer.from( + Array.from({ length: 500 }, (_, i) => `line${i}`).join('\n') + '\n', + ); + framer.feedChunk(output, (line) => seen.push(line)); + expect(seen.length).toBe(500); + expect(seen[0]).toBe('line0'); + expect(seen[499]).toBe('line499'); + }); +}); + +describe('BoundedLineFramer - adversarial large chunks', () => { + it('handles a single chunk much larger than maxLineBytes with many lines', () => { + const maxLineBytes = 1024; + const framer = new BoundedLineFramer({ maxLineBytes }); + const lineCount = 5000; + const parts: string[] = []; + for (let i = 0; i < lineCount; i++) { + parts.push(`file_${i}.ts:${i}:match_content_${i}`); + } + const big = Buffer.from(parts.join('\n') + '\n'); + const lines = feedCollect(framer, big); + expect(lines.length).toBe(lineCount); + expect(lines[0]).toBe('file_0.ts:0:match_content_0'); + expect(lines[lineCount - 1]).toBe( + `file_${lineCount - 1}.ts:${lineCount - 1}:match_content_${lineCount - 1}`, + ); + expect(framer.wasLineDropped).toBe(false); + }); + + it('handles a pathological chunk with no newlines larger than maxLineBytes', () => { + const maxLineBytes = 100; + const framer = new BoundedLineFramer({ maxLineBytes }); + const huge = Buffer.alloc(maxLineBytes * 100, 65); + feedCollect(framer, huge); + expect(framer.wasLineDropped).toBe(true); + const lines = feedCollect(framer, Buffer.from('\nrecovered\n')); + expect(lines).toEqual(['recovered']); + }); + + it('handles alternating huge and normal lines in one chunk', () => { + const maxLineBytes = 50; + const framer = new BoundedLineFramer({ maxLineBytes }); + const parts: Buffer[] = [ + Buffer.alloc(200, 65), + Buffer.from('\n'), + Buffer.from('ok_line\n'), + Buffer.alloc(200, 66), + Buffer.from('\n'), + Buffer.from('done\n'), + ]; + const lines = feedCollect(framer, Buffer.concat(parts)); + expect(lines).toEqual(['ok_line', 'done']); + expect(framer.wasLineDropped).toBe(true); + }); + + it('bulk-copies a multi-MB chunk of small lines without corruption', () => { + const framer = new BoundedLineFramer(); + const lineCount = 10000; + const parts: string[] = []; + for (let i = 0; i < lineCount; i++) { + parts.push(`${i}`); + } + const big = Buffer.from(parts.join('\n') + '\n'); + const lines = feedCollect(framer, big); + expect(lines.length).toBe(lineCount); + for (let i = 0; i < lineCount; i++) { + expect(lines[i]).toBe(String(i)); + } + }); +}); diff --git a/packages/tools/src/utils/lineFramer.ts b/packages/tools/src/utils/lineFramer.ts new file mode 100644 index 0000000000..9aeb251c48 --- /dev/null +++ b/packages/tools/src/utils/lineFramer.ts @@ -0,0 +1,161 @@ +/** + * @license + * Copyright 2026 Vybestack LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +const HARD_MAX_LINE_BYTES = 16 * 1024 * 1024; +const DEFAULT_MAX_LINE_BYTES = 1024 * 1024; + +export interface BoundedLineFramerOptions { + maxLineBytes?: number; +} + +function validateMaxLineBytes(value: number): void { + if ( + !Number.isSafeInteger(value) || + value <= 0 || + value > HARD_MAX_LINE_BYTES + ) { + throw new RangeError( + `maxLineBytes must be a finite positive safe integer <= ${HARD_MAX_LINE_BYTES}, got: ${String(value)}`, + ); + } +} + +function decodeFatal(bytes: Uint8Array): string { + const decoder = new TextDecoder('utf-8', { fatal: true }); + return decoder.decode(bytes); +} + +export class BoundedLineFramer { + private readonly buffer: Uint8Array; + private readonly maxLineBytes: number; + private length = 0; + private discarding = false; + private droppedLine = false; + + constructor(options?: BoundedLineFramerOptions) { + const max = options?.maxLineBytes ?? DEFAULT_MAX_LINE_BYTES; + validateMaxLineBytes(max); + this.maxLineBytes = max; + this.buffer = new Uint8Array(this.maxLineBytes + 1); + } + + get wasLineDropped(): boolean { + return this.droppedLine; + } + + reset(): void { + this.length = 0; + this.discarding = false; + this.droppedLine = false; + } + + feedChunk(chunk: Buffer, onLine: (line: string) => void): void { + let i = 0; + while (i < chunk.length) { + if (this.discarding) { + const nl = chunk.indexOf(0x0a, i); + if (nl === -1) return; + this.discarding = false; + this.length = 0; + i = nl + 1; + continue; + } + + const nl = chunk.indexOf(0x0a, i); + const space = Math.max(0, this.maxLineBytes - this.length); + + if (nl === -1) { + const segmentLen = chunk.length - i; + if (segmentLen <= space) { + chunk.copy(this.buffer, this.length, i); + this.length += segmentLen; + } else if ( + this.length <= this.maxLineBytes && + segmentLen === space + 1 && + chunk[i + space] === 0x0d + ) { + chunk.copy(this.buffer, this.length, i, i + space); + this.buffer[this.maxLineBytes] = 0x0d; + this.length = this.maxLineBytes + 1; + } else { + this.discarding = true; + this.droppedLine = true; + } + return; + } + + const segmentLen = nl - i; + if (segmentLen === 0 || segmentLen <= space) { + if (segmentLen > 0) { + chunk.copy(this.buffer, this.length, i, nl); + this.length += segmentLen; + } + this.stripTrailingCr(); + this.terminate(onLine); + } else if ( + this.length <= this.maxLineBytes && + segmentLen === space + 1 && + chunk[i + space] === 0x0d + ) { + chunk.copy(this.buffer, this.length, i, i + space); + this.buffer[this.maxLineBytes] = 0x0d; + this.length = this.maxLineBytes + 1; + this.stripTrailingCr(); + this.terminate(onLine); + } else { + this.droppedLine = true; + this.length = 0; + } + i = nl + 1; + } + } + + private stripTrailingCr(): void { + if ( + this.length > 0 && + !this.discarding && + this.buffer[this.length - 1] === 0x0d + ) { + this.length--; + } + } + + flushRemaining(onLine: (line: string) => void): void { + if (this.discarding) { + this.discarding = false; + this.length = 0; + return; + } + if (this.length > 0) { + this.terminate(onLine); + } + } + + private terminate(onLine: (line: string) => void): void { + if (this.discarding) { + this.discarding = false; + this.length = 0; + return; + } + + if (this.length === 0) { + onLine(''); + return; + } + + const recordBytes = Buffer.from(this.buffer.subarray(0, this.length)); + this.length = 0; + + let line: string; + try { + line = decodeFatal(recordBytes); + } catch { + this.droppedLine = true; + return; + } + onLine(line); + } +} diff --git a/packages/tools/src/utils/processTermination.test.ts b/packages/tools/src/utils/processTermination.test.ts new file mode 100644 index 0000000000..4ef8809c3d --- /dev/null +++ b/packages/tools/src/utils/processTermination.test.ts @@ -0,0 +1,505 @@ +/** + * @license + * Copyright 2026 Vybestack LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect } from 'bun:test'; +import { spawn, type ChildProcess } from 'node:child_process'; +import { EventEmitter } from 'node:events'; +import { + terminateProcessTree, + terminateWindowsTree, + DEFAULT_TERMINATION_GRACE_MS, + WINDOWS_TASKKILL_WATCHDOG_MS, + type TaskkillSpawnFn, + type SignalFn, +} from './processTermination.js'; + +function spawnSleeper(seconds: number, ignoreSigterm = false): ChildProcess { + const script = ignoreSigterm + ? `trap '' TERM; exec sleep ${seconds}` + : `sleep ${seconds}`; + return spawn('sh', ['-c', script], { + stdio: 'ignore', + detached: process.platform !== 'win32', + windowsHide: true, + }); +} + +function waitForExit(child: ChildProcess): Promise { + if (child.exitCode !== null || child.signalCode !== null) { + return Promise.resolve(); + } + return new Promise((resolve) => { + child.once('exit', () => resolve()); + }); +} + +describe('terminateProcessTree - graceful exit', () => { + it( + 'signals a running process and it exits gracefully', + async () => { + const child = spawnSleeper(30); + expect(child.pid).toBeDefined(); + + await new Promise((r) => setTimeout(r, 100)); + + const result = await terminateProcessTree(child, { + gracePeriodMs: 2000, + ownsProcessGroup: true, + }); + expect(result.outcome).toBe('graceful'); + + await waitForExit(child); + expect(child.exitCode !== null || child.signalCode !== null).toBe(true); + }, + { timeout: 10000 }, + ); + + it('returns no_target for an already-exited process', async () => { + const child = spawnSleeper(0); + await waitForExit(child); + + const result = await terminateProcessTree(child); + expect(result.outcome).toBe('no_target'); + }); + + it('returns no_target for a process with no pid', async () => { + const fakeChild = { + pid: undefined, + exitCode: null, + signalCode: null, + } as unknown as ChildProcess; + + const result = await terminateProcessTree(fakeChild); + expect(result.outcome).toBe('no_target'); + }); +}); + +describe('terminateProcessTree - SIGTERM ignoring process escalates to SIGKILL', () => { + it.skipIf(process.platform === 'win32')( + 'escalates to SIGKILL when SIGTERM is ignored', + async () => { + const child = spawnSleeper(60, true); + expect(child.pid).toBeDefined(); + + await new Promise((r) => setTimeout(r, 300)); + + const result = await terminateProcessTree(child, { + gracePeriodMs: 500, + ownsProcessGroup: true, + }); + expect(result.outcome).toBe('escalated'); + + await waitForExit(child); + expect(child.signalCode).toBe('SIGKILL'); + }, + { timeout: 10000 }, + ); + + it.skipIf(process.platform === 'win32')( + 'escalated process group has no survivors (descendant is dead)', + async () => { + const child = spawn('sh', ['-c', 'sleep 60 & sleep 60 & sleep 60'], { + stdio: 'ignore', + detached: true, + windowsHide: true, + }); + expect(child.pid).toBeDefined(); + const pgid = child.pid!; + + await new Promise((r) => setTimeout(r, 300)); + + const result = await terminateProcessTree(child, { + gracePeriodMs: 500, + ownsProcessGroup: true, + }); + expect(result.outcome).toBe('graceful'); + + await waitForExit(child); + + await new Promise((r) => setTimeout(r, 200)); + + expect(() => process.kill(-pgid, 0)).toThrow('ESRCH'); + }, + { timeout: 10000 }, + ); + + it.skipIf(process.platform === 'win32')( + 'leader exits but descendant ignores SIGTERM — escalation kills the group', + async () => { + const child = spawn('sh', ['-c', "trap '' TERM &\nwait\nsleep 60"], { + stdio: 'ignore', + detached: true, + windowsHide: true, + }); + expect(child.pid).toBeDefined(); + const pgid = child.pid!; + + await new Promise((r) => setTimeout(r, 300)); + + const result = await terminateProcessTree(child, { + gracePeriodMs: 500, + ownsProcessGroup: true, + }); + expect(['escalated', 'graceful']).toContain(result.outcome); + + await new Promise((r) => setTimeout(r, 200)); + expect(() => process.kill(-pgid, 0)).toThrow('ESRCH'); + }, + { timeout: 10000 }, + ); + + it.skipIf(process.platform === 'win32')( + 'leader definitely exited, descendant alive ignoring TERM — group escalation kills it', + async () => { + // Leader spawns a TERM-ignoring descendant then exits immediately. + // The descendant keeps the process group alive. + const child = spawn( + 'sh', + ['-c', "(trap '' TERM; exec sleep 60) &\nexit 0"], + { + stdio: 'ignore', + detached: true, + windowsHide: true, + }, + ); + expect(child.pid).toBeDefined(); + const pgid = child.pid!; + + // Wait for the leader to DEFINITELY exit. + await waitForExit(child); + expect(child.exitCode).toBe(0); + + // The descendant must still be alive in the owned group. + expect(() => process.kill(-pgid, 0)).not.toThrow(); + + const result = await terminateProcessTree(child, { + gracePeriodMs: 500, + ownsProcessGroup: true, + }); + expect(['escalated', 'graceful']).toContain(result.outcome); + + // Group must be completely gone. + await new Promise((r) => setTimeout(r, 200)); + expect(() => process.kill(-pgid, 0)).toThrow('ESRCH'); + }, + { timeout: 15000 }, + ); + + it.skipIf(process.platform === 'win32')( + 'group-owning termination never falls back to positive-PID signal', + async () => { + const child = spawnSleeper(60, true); + expect(child.pid).toBeDefined(); + const pid = child.pid!; + + await new Promise((r) => setTimeout(r, 300)); + + const result = await terminateProcessTree(child, { + gracePeriodMs: 500, + ownsProcessGroup: true, + }); + expect(result.outcome).toBe('escalated'); + + await waitForExit(child); + + // The original PID must be dead (no reused PID was signaled). + expect(() => process.kill(pid, 0)).toThrow('ESRCH'); + }, + { timeout: 10000 }, + ); +}); + +describe('terminateProcessTree - direct child (no process group)', () => { + it.skipIf(process.platform === 'win32')( + 'terminates a direct child via positive PID signal', + async () => { + const child = spawnSleeper(30); + expect(child.pid).toBeDefined(); + + await new Promise((r) => setTimeout(r, 100)); + + const result = await terminateProcessTree(child, { + gracePeriodMs: 2000, + ownsProcessGroup: false, + }); + expect(result.outcome).toBe('graceful'); + + await waitForExit(child); + }, + { timeout: 10000 }, + ); + + it.skipIf(process.platform === 'win32')( + 'escalates direct child to SIGKILL when SIGTERM is ignored', + async () => { + const child = spawnSleeper(60, true); + + await new Promise((r) => setTimeout(r, 300)); + + const result = await terminateProcessTree(child, { + gracePeriodMs: 500, + ownsProcessGroup: false, + }); + expect(result.outcome).toBe('escalated'); + + await waitForExit(child); + expect(child.signalCode).toBe('SIGKILL'); + }, + { timeout: 10000 }, + ); +}); + +describe('terminateProcessTree - coalescing by ChildProcess identity', () => { + it( + 'coalesces concurrent calls for the same child', + async () => { + const child = spawnSleeper(30); + await new Promise((r) => setTimeout(r, 100)); + + const [result1, result2] = await Promise.all([ + terminateProcessTree(child, { + gracePeriodMs: 2000, + ownsProcessGroup: true, + }), + terminateProcessTree(child, { + gracePeriodMs: 2000, + ownsProcessGroup: true, + }), + ]); + + expect(result1.outcome).toBe('graceful'); + expect(result2.outcome).toBe('graceful'); + + await waitForExit(child); + }, + { timeout: 10000 }, + ); + + it( + 'coalesces many concurrent calls for the same child', + async () => { + const child = spawnSleeper(30); + await new Promise((r) => setTimeout(r, 100)); + + const results = await Promise.all( + Array.from({ length: 5 }, () => + terminateProcessTree(child, { + gracePeriodMs: 2000, + ownsProcessGroup: true, + }), + ), + ); + + for (const r of results) { + expect(r.outcome).toBe('graceful'); + } + + await waitForExit(child); + }, + { timeout: 10000 }, + ); + + it('returns no_target for a sequential call after first completion', async () => { + const child = spawnSleeper(2); + await new Promise((r) => setTimeout(r, 100)); + + const result1 = await terminateProcessTree(child, { + gracePeriodMs: 3000, + ownsProcessGroup: true, + }); + await waitForExit(child); + + const result2 = await terminateProcessTree(child); + expect(result1.outcome).toBe('graceful'); + expect(result2.outcome).toBe('no_target'); + }); +}); + +describe('terminateProcessTree - exported constants', () => { + it('exports a named grace period constant', () => { + expect(DEFAULT_TERMINATION_GRACE_MS).toBeGreaterThan(0); + expect(DEFAULT_TERMINATION_GRACE_MS).toBeLessThanOrEqual(30000); + expect(typeof DEFAULT_TERMINATION_GRACE_MS).toBe('number'); + }); + + it('exports a Windows taskkill watchdog constant', () => { + expect(WINDOWS_TASKKILL_WATCHDOG_MS).toBeGreaterThan(0); + expect(typeof WINDOWS_TASKKILL_WATCHDOG_MS).toBe('number'); + }); +}); + +describe('terminateWindowsTree - platform-independent outcome tests', () => { + function makeFakeChild(events: { + closeCode?: number | null; + error?: Error; + delayMs?: number; + closeOnKill?: boolean; + }): ChildProcess { + const ee = new EventEmitter(); + let killed = false; + const timer = setTimeout(() => { + if (events.error !== undefined) { + ee.emit('error', events.error); + } else if (events.delayMs !== undefined) { + // Watchdog will fire and call kill(); kill() cancels this timer. + } else { + ee.emit('close', events.closeCode ?? 0); + } + }, events.delayMs ?? 0); + (ee as unknown as { kill: (signal?: string) => boolean }).kill = () => { + killed = true; + clearTimeout(timer); + if (events.closeOnKill === true) { + setImmediate(() => ee.emit('close', 1)); + } + return true; + }; + Object.defineProperty(ee, '_killed', { + get: () => killed, + enumerable: false, + }); + + return ee as unknown as ChildProcess; + } + + it('reports graceful when taskkill exits 0', async () => { + const fakeSpawn: TaskkillSpawnFn = () => makeFakeChild({ closeCode: 0 }); + const result = await terminateWindowsTree(12345, fakeSpawn, { + watchdogMs: 1000, + postKillWaitMs: 500, + }); + expect(result.outcome).toBe('graceful'); + }); + + it('reports failure when taskkill exits nonzero', async () => { + const fakeSpawn: TaskkillSpawnFn = () => makeFakeChild({ closeCode: 1 }); + const result = await terminateWindowsTree(12345, fakeSpawn, { + watchdogMs: 1000, + postKillWaitMs: 500, + }); + expect(result.outcome).toBe('failure'); + }); + + it('reports failure when taskkill spawn throws', async () => { + const fakeSpawn: TaskkillSpawnFn = () => { + throw new Error('ENOENT'); + }; + const result = await terminateWindowsTree(12345, fakeSpawn); + expect(result.outcome).toBe('failure'); + }); + + it('reports failure and kills child when taskkill emits error while running', async () => { + let killCalled = false; + const fakeSpawn: TaskkillSpawnFn = () => { + const child = makeFakeChild({ error: new Error('spawn error') }); + const origKill = (child as unknown as { kill: () => boolean }).kill; + (child as unknown as { kill: () => boolean }).kill = () => { + killCalled = true; + return origKill(); + }; + return child; + }; + const result = await terminateWindowsTree(12345, fakeSpawn, { + watchdogMs: 1000, + postKillWaitMs: 500, + }); + expect(result.outcome).toBe('failure'); + expect(killCalled).toBe(true); + }); + + it( + 'reports timeout when taskkill hangs past the watchdog (injectable)', + async () => { + const fakeSpawn: TaskkillSpawnFn = () => + makeFakeChild({ delayMs: 10000 }); + const result = await terminateWindowsTree(12345, fakeSpawn, { + watchdogMs: 50, + postKillWaitMs: 50, + }); + expect(result.outcome).toBe('timeout'); + }, + { timeout: 5000 }, + ); + + it( + 'resolves timeout when watchdog-killed process closes within post-kill deadline', + async () => { + const fakeSpawn: TaskkillSpawnFn = () => + makeFakeChild({ delayMs: 10000, closeOnKill: true }); + const result = await terminateWindowsTree(12345, fakeSpawn, { + watchdogMs: 30, + postKillWaitMs: 100, + }); + expect(result.outcome).toBe('timeout'); + }, + { timeout: 5000 }, + ); + + it( + 'never-close taskkill resolves timeout via post-kill timer', + async () => { + const fakeSpawn: TaskkillSpawnFn = () => + makeFakeChild({ delayMs: 10000, closeOnKill: false }); + const result = await terminateWindowsTree(12345, fakeSpawn, { + watchdogMs: 30, + postKillWaitMs: 50, + }); + expect(result.outcome).toBe('timeout'); + }, + { timeout: 5000 }, + ); +}); + +describe('terminateProcessTree - EPERM vs ESRCH signal semantics', () => { + const fakeChild = { + pid: 99999, + exitCode: null, + signalCode: null, + } as unknown as ChildProcess; + + const epermSignal: SignalFn = () => { + throw Object.assign(new Error('Operation not permitted'), { + code: 'EPERM', + }); + }; + + it('EPERM on liveness probe means group exists; EPERM on SIGTERM is failure', async () => { + const result = await terminateProcessTree(fakeChild, { + ownsProcessGroup: true, + signal: epermSignal, + gracePeriodMs: 100, + }); + expect(result.outcome).toBe('failure'); + }); + + it('ESRCH on liveness probe means group is gone — no_target', async () => { + const esrchSignal: SignalFn = () => { + throw Object.assign(new Error('No such process'), { + code: 'ESRCH', + }); + }; + const result = await terminateProcessTree(fakeChild, { + ownsProcessGroup: true, + signal: esrchSignal, + gracePeriodMs: 100, + }); + expect(result.outcome).toBe('no_target'); + }); + + it('EPERM on direct-child SIGTERM is failure, not no_target', async () => { + const runningChild = { + pid: 99998, + exitCode: null, + signalCode: null, + } as unknown as ChildProcess; + const result = await terminateProcessTree(runningChild, { + ownsProcessGroup: false, + signal: epermSignal, + gracePeriodMs: 100, + }); + expect(result.outcome).toBe('failure'); + }); +}); diff --git a/packages/tools/src/utils/processTermination.ts b/packages/tools/src/utils/processTermination.ts new file mode 100644 index 0000000000..5e63c450da --- /dev/null +++ b/packages/tools/src/utils/processTermination.ts @@ -0,0 +1,305 @@ +/** + * @license + * Copyright 2026 Vybestack LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { spawn, type ChildProcess } from 'node:child_process'; + +export const DEFAULT_TERMINATION_GRACE_MS = 5000; +export const WINDOWS_TASKKILL_WATCHDOG_MS = 10000; +const GROUP_POLL_INTERVAL_MS = 50; +const POST_KILL_WAIT_MS = 1000; + +export type ProcessTerminationOutcome = + | 'no_target' + | 'graceful' + | 'escalated' + | 'timeout' + | 'failure'; + +export interface ProcessTerminationResult { + readonly outcome: ProcessTerminationOutcome; +} + +export type SignalFn = (pid: number, signal: NodeJS.Signals | 0) => void; + +const defaultSignal: SignalFn = (pid, signal) => process.kill(pid, signal); + +export interface WindowsTerminationOptions { + watchdogMs?: number; + postKillWaitMs?: number; +} + +export interface ProcessTerminationOptions { + gracePeriodMs?: number; + ownsProcessGroup?: boolean; + signal?: SignalFn; +} + +const activeTerminations = new WeakMap< + ChildProcess, + Promise +>(); + +function sleep(ms: number): Promise { + return new Promise((r) => setTimeout(r, ms)); +} + +function groupExists(signal: SignalFn, pgid: number): boolean { + try { + signal(-pgid, 0); + return true; + } catch (e) { + return !isErrnoException(e) || e.code !== 'ESRCH'; + } +} + +async function waitForGroupGone( + signal: SignalFn, + pgid: number, + timeoutMs: number, +): Promise { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if (!groupExists(signal, pgid)) return true; + await sleep(GROUP_POLL_INTERVAL_MS); + } + return !groupExists(signal, pgid); +} + +function childExited(child: ChildProcess): boolean { + return child.exitCode !== null || child.signalCode !== null; +} + +async function waitForChildExit( + child: ChildProcess, + timeoutMs: number, +): Promise { + if (childExited(child)) return true; + return new Promise((resolve) => { + let done = false; + const finish = (result: boolean) => { + if (done) return; + done = true; + clearTimeout(timer); + child.removeListener('exit', onExit); + resolve(result); + }; + const onExit = () => finish(true); + const timer = setTimeout(() => finish(childExited(child)), timeoutMs); + if (childExited(child)) { + finish(true); + return; + } + child.once('exit', onExit); + }); +} + +export async function terminateProcessTree( + child: ChildProcess, + options?: ProcessTerminationOptions, +): Promise { + const pid = child.pid; + if (pid === undefined) { + return { outcome: 'no_target' }; + } + + const ownedGroup = + options?.ownsProcessGroup === true && process.platform !== 'win32'; + + if (!ownedGroup && childExited(child)) { + return { outcome: 'no_target' }; + } + + const existing = activeTerminations.get(child); + if (existing !== undefined) { + return existing; + } + + const promise = doTerminate(child, options).finally(() => { + activeTerminations.delete(child); + }); + activeTerminations.set(child, promise); + return promise; +} + +async function doTerminate( + child: ChildProcess, + options?: ProcessTerminationOptions, +): Promise { + const pid = child.pid; + if (pid === undefined) { + return { outcome: 'no_target' }; + } + + const signal = options?.signal ?? defaultSignal; + const gracePeriod = options?.gracePeriodMs ?? DEFAULT_TERMINATION_GRACE_MS; + + if (process.platform === 'win32') { + return terminateWindowsTree(pid); + } + + if (options?.ownsProcessGroup === true) { + if (!groupExists(signal, pid)) { + return { outcome: 'no_target' }; + } + return terminateOwnedGroup(signal, pid, gracePeriod); + } + + if (childExited(child)) { + return { outcome: 'no_target' }; + } + + return terminateDirectChild(signal, child, pid, gracePeriod); +} + +function isErrnoException(e: unknown): e is NodeJS.ErrnoException { + return e instanceof Error && 'code' in e; +} + +function signalFailedBecauseGone(e: unknown): boolean { + return isErrnoException(e) && e.code === 'ESRCH'; +} + +async function terminateOwnedGroup( + signal: SignalFn, + pgid: number, + gracePeriodMs: number, +): Promise { + try { + signal(-pgid, 'SIGTERM'); + } catch (e) { + if (signalFailedBecauseGone(e)) return { outcome: 'no_target' }; + if ( + isErrnoException(e) && + e.code === 'EPERM' && + (await waitForGroupGone(signal, pgid, POST_KILL_WAIT_MS)) + ) { + return { outcome: 'graceful' }; + } + return { outcome: 'failure' }; + } + + if (await waitForGroupGone(signal, pgid, gracePeriodMs)) { + return { outcome: 'graceful' }; + } + + try { + signal(-pgid, 'SIGKILL'); + } catch (e) { + if (signalFailedBecauseGone(e)) return { outcome: 'no_target' }; + if ( + isErrnoException(e) && + e.code === 'EPERM' && + (await waitForGroupGone(signal, pgid, POST_KILL_WAIT_MS)) + ) { + return { outcome: 'escalated' }; + } + return { outcome: 'failure' }; + } + + if (await waitForGroupGone(signal, pgid, POST_KILL_WAIT_MS)) { + return { outcome: 'escalated' }; + } + + return { outcome: 'timeout' }; +} + +async function terminateDirectChild( + signal: SignalFn, + child: ChildProcess, + pid: number, + gracePeriodMs: number, +): Promise { + try { + signal(pid, 'SIGTERM'); + } catch (e) { + return { outcome: signalFailedBecauseGone(e) ? 'no_target' : 'failure' }; + } + + if (await waitForChildExit(child, gracePeriodMs)) { + return { outcome: 'graceful' }; + } + + try { + signal(pid, 'SIGKILL'); + } catch (e) { + return { outcome: signalFailedBecauseGone(e) ? 'no_target' : 'failure' }; + } + + if (await waitForChildExit(child, POST_KILL_WAIT_MS)) { + return { outcome: 'escalated' }; + } + + return { outcome: 'timeout' }; +} + +export type TaskkillSpawnFn = (pid: number) => ChildProcess; + +export const defaultTaskkillSpawn: TaskkillSpawnFn = (pid: number) => + spawn('taskkill', ['/PID', String(pid), '/T', '/F'], { + windowsHide: true, + stdio: 'ignore', + }); + +export function terminateWindowsTree( + pid: number, + spawnTaskkill: TaskkillSpawnFn = defaultTaskkillSpawn, + options?: WindowsTerminationOptions, +): Promise { + const watchdogMs = options?.watchdogMs ?? WINDOWS_TASKKILL_WATCHDOG_MS; + const postKillWaitMs = options?.postKillWaitMs ?? POST_KILL_WAIT_MS; + return new Promise((resolve) => { + let kill: ChildProcess; + try { + kill = spawnTaskkill(pid); + } catch { + resolve({ outcome: 'failure' }); + return; + } + + let settled = false; + let postKillTimer: ReturnType | null = null; + + const onWatchdogClose = () => resolveOnce('timeout'); + + const resolveOnce = (outcome: ProcessTerminationOutcome) => { + if (settled) return; + settled = true; + clearTimeout(watchdog); + if (postKillTimer !== null) clearTimeout(postKillTimer); + kill.removeListener('close', onClose); + kill.removeListener('close', onWatchdogClose); + kill.removeListener('error', onError); + resolve({ outcome }); + }; + + const onClose = (code: number | null) => { + resolveOnce(code === 0 ? 'graceful' : 'failure'); + }; + + const onError = () => { + try { + kill.kill(); + } catch { + // best-effort + } + resolveOnce('failure'); + }; + + const watchdog = setTimeout(() => { + kill.removeListener('close', onClose); + try { + kill.kill(); + } catch { + // best-effort + } + kill.once('close', onWatchdogClose); + postKillTimer = setTimeout(() => resolveOnce('timeout'), postKillWaitMs); + }, watchdogMs); + + kill.on('close', onClose); + kill.on('error', onError); + }); +} diff --git a/packages/tools/src/utils/ripgrepPathResolver.test.ts b/packages/tools/src/utils/ripgrepPathResolver.test.ts new file mode 100644 index 0000000000..861b1a137e --- /dev/null +++ b/packages/tools/src/utils/ripgrepPathResolver.test.ts @@ -0,0 +1,201 @@ +/** + * @license + * Copyright 2026 Vybestack LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, beforeEach, afterEach } from 'bun:test'; +import { writeFileSync, mkdirSync, rmSync, chmodSync } from 'node:fs'; +import { join, delimiter as pathDelimiter } from 'node:path'; +import { tmpdir } from 'node:os'; +import { findInPath } from './ripgrepPathResolver.js'; + +function makeTempDirs(count: number): { dirs: string[]; cleanup: () => void } { + const base = join( + tmpdir(), + `rg-path-test-${Date.now()}-${Math.random().toString(36).slice(2)}`, + ); + const dirs: string[] = []; + for (let i = 0; i < count; i++) { + const dir = join(base, `d${i}`); + mkdirSync(dir, { recursive: true }); + dirs.push(dir); + } + return { + dirs, + cleanup: () => { + try { + rmSync(base, { recursive: true, force: true }); + } catch { + /* ignore */ + } + }, + }; +} + +describe('findInPath executability semantics', () => { + const originalPath = process.env.PATH; + const originalPathExt = process.env.PATHEXT; + const isWindows = process.platform === 'win32'; + + beforeEach(() => { + process.env.PATHEXT = ''; + }); + + afterEach(() => { + process.env.PATH = originalPath; + if (originalPathExt === undefined) { + delete process.env.PATHEXT; + } else { + process.env.PATHEXT = originalPathExt; + } + }); + + it.skipIf(isWindows)('ignores a non-executable candidate named rg', () => { + const { dirs, cleanup } = makeTempDirs(1); + try { + const candidate = join(dirs[0], 'rg'); + writeFileSync(candidate, '#!/bin/sh\necho fake\n'); + chmodSync(candidate, 0o644); + process.env.PATH = dirs[0]; + expect(findInPath('rg', false)).toBeNull(); + } finally { + cleanup(); + } + }); + + it.skipIf(isWindows)('selects an executable candidate named rg', () => { + const { dirs, cleanup } = makeTempDirs(1); + try { + const candidate = join(dirs[0], 'rg'); + writeFileSync(candidate, '#!/bin/sh\necho real\n'); + chmodSync(candidate, 0o755); + process.env.PATH = dirs[0]; + expect(findInPath('rg', false)).toBe(candidate); + } finally { + cleanup(); + } + }); + + it.skipIf(isWindows)( + 'falls back to a later PATH entry when the first is non-executable', + () => { + const { dirs, cleanup } = makeTempDirs(2); + try { + const nonExec = join(dirs[0], 'rg'); + writeFileSync(nonExec, '#!/bin/sh\necho blocked\n'); + chmodSync(nonExec, 0o644); + + const exec = join(dirs[1], 'rg'); + writeFileSync(exec, '#!/bin/sh\necho unblocked\n'); + chmodSync(exec, 0o755); + + process.env.PATH = `${dirs[0]}:${dirs[1]}`; + expect(findInPath('rg', false)).toBe(exec); + } finally { + cleanup(); + } + }, + ); +}); + +describe('findInPath Windows extension resolution', () => { + const originalPath = process.env.PATH; + const originalPathExt = process.env.PATHEXT; + + afterEach(() => { + process.env.PATH = originalPath; + if (originalPathExt === undefined) { + delete process.env.PATHEXT; + } else { + process.env.PATHEXT = originalPathExt; + } + }); + + it('finds rg.EXE with normal PATHEXT when isWindows is true', () => { + const { dirs, cleanup } = makeTempDirs(1); + try { + const candidate = join(dirs[0], 'rg.EXE'); + writeFileSync(candidate, 'fake'); + process.env.PATH = dirs[0]; + process.env.PATHEXT = ['.COM', '.EXE', '.BAT', '.CMD'].join( + pathDelimiter, + ); + expect(findInPath('rg', true)).toBe(candidate); + } finally { + cleanup(); + } + }); + + it('finds bare rg with normal PATHEXT when isWindows is true (bare always checked)', () => { + const { dirs, cleanup } = makeTempDirs(1); + try { + const candidate = join(dirs[0], 'rg'); + writeFileSync(candidate, 'fake'); + process.env.PATH = dirs[0]; + process.env.PATHEXT = ['.COM', '.EXE', '.BAT', '.CMD'].join( + pathDelimiter, + ); + expect(findInPath('rg', true)).toBe(candidate); + } finally { + cleanup(); + } + }); + + it('finds rg.EXE when PATHEXT is absent (fallback .EXE)', () => { + const { dirs, cleanup } = makeTempDirs(1); + try { + const candidate = join(dirs[0], 'rg.EXE'); + writeFileSync(candidate, 'fake'); + process.env.PATH = dirs[0]; + delete process.env.PATHEXT; + expect(findInPath('rg', true)).toBe(candidate); + } finally { + cleanup(); + } + }); + + it('finds bare rg when PATHEXT is empty (fallback .EXE does not exist)', () => { + const { dirs, cleanup } = makeTempDirs(1); + try { + const candidate = join(dirs[0], 'rg'); + writeFileSync(candidate, 'fake'); + process.env.PATH = dirs[0]; + process.env.PATHEXT = ''; + expect(findInPath('rg', true)).toBe(candidate); + } finally { + cleanup(); + } + }); + + it('does not produce rg.exe.EXE double extension', () => { + const { dirs, cleanup } = makeTempDirs(1); + try { + const correctCandidate = join(dirs[0], 'rg.EXE'); + const wrongCandidate = join(dirs[0], 'rg.exe.EXE'); + writeFileSync(correctCandidate, 'real'); + writeFileSync(wrongCandidate, 'wrong'); + process.env.PATH = dirs[0]; + process.env.PATHEXT = ['.COM', '.EXE', '.BAT', '.CMD'].join( + pathDelimiter, + ); + expect(findInPath('rg', true)).toBe(correctCandidate); + } finally { + cleanup(); + } + }); + + it('deduplicates case-insensitive PATHEXT entries', () => { + const { dirs, cleanup } = makeTempDirs(1); + try { + const candidate = join(dirs[0], 'rg.EXE'); + writeFileSync(candidate, 'fake'); + process.env.PATH = dirs[0]; + process.env.PATHEXT = ['.EXE', '.exe', '.EXE'].join(pathDelimiter); + const result = findInPath('rg', true); + expect(result).toBe(candidate); + } finally { + cleanup(); + } + }); +}); diff --git a/packages/tools/src/utils/ripgrepPathResolver.ts b/packages/tools/src/utils/ripgrepPathResolver.ts index d0c19920c0..b18888d4b1 100644 --- a/packages/tools/src/utils/ripgrepPathResolver.ts +++ b/packages/tools/src/utils/ripgrepPathResolver.ts @@ -79,16 +79,53 @@ async function tryPackagedRipgrep(): Promise { return null; } -async function trySystemRipgrep(isWindows: boolean): Promise { +function isExecutable(filePath: string, isWindows: boolean): boolean { try { - const { execSync } = await import('child_process'); - const checkCmd = isWindows ? 'where rg' : 'which rg'; - const systemPath = execSync(checkCmd, { encoding: 'utf8' }).trim(); - if (fs.existsSync(systemPath)) { - return systemPath; + if (!fs.statSync(filePath).isFile()) { + return false; + } + if (isWindows) { + return true; } + fs.accessSync(filePath, fs.constants.X_OK); + return true; } catch { - // System ripgrep not found + return false; + } +} + +export function findInPath(binName: string, isWindows: boolean): string | null { + const pathEnv = process.env.PATH ?? ''; + const pathExt = process.env.PATHEXT ?? ''; + const rawExts = pathExt + ? ['', ...pathExt.split(path.delimiter).filter((e) => e.length > 0)] + : ['', '.EXE']; + const seen = new Set(); + const exts: string[] = []; + for (const ext of rawExts) { + const key = ext.toLowerCase(); + if (!seen.has(key)) { + seen.add(key); + exts.push(ext); + } + } + const dirs = pathEnv.split(path.delimiter); + for (const dir of dirs) { + if (!dir) continue; + for (const ext of exts) { + const candidate = path.join(dir, ext ? `${binName}${ext}` : binName); + if (isExecutable(candidate, isWindows)) { + return candidate; + } + } + } + return null; +} + +async function trySystemRipgrep(isWindows: boolean): Promise { + const found = findInPath('rg', isWindows); + if (found && isCompatibleRipgrepBinary(found)) { + return found; } return null; } diff --git a/packages/tools/src/utils/subprocessSettle.test.ts b/packages/tools/src/utils/subprocessSettle.test.ts new file mode 100644 index 0000000000..fb9c7fc2c7 --- /dev/null +++ b/packages/tools/src/utils/subprocessSettle.test.ts @@ -0,0 +1,184 @@ +/** + * @license + * Copyright 2026 Vybestack LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect } from 'bun:test'; +import { + createSettleFn, + type SubprocessSettlement, + type AbortHandlerRef, +} from './subprocessSettle.js'; +import type { ProcessTerminationResult } from './processTermination.js'; + +class TestLifecycleError extends Error { + constructor(message: string) { + super(message); + this.name = 'ProcessLifecycleError'; + } +} + +function makeSettlement(): { + settlement: SubprocessSettlement; + abortRef: AbortHandlerRef; + controller: AbortController; + resolveSpy: (v: string) => void; + rejectSpy: (e: Error) => void; + promise: Promise; +} { + const settlement: SubprocessSettlement = { + settled: false, + terminationPromise: null, + }; + const controller = new AbortController(); + const abortRef: AbortHandlerRef = { handler: () => {} }; + let resolveFn!: (v: string) => void; + let rejectFn!: (e: Error) => void; + const promise = new Promise((res, rej) => { + resolveFn = res; + rejectFn = rej; + }); + return { + settlement, + abortRef, + controller, + resolveSpy: resolveFn, + rejectSpy: rejectFn, + promise, + }; +} + +describe('createSettleFn', () => { + it('resolves via action on normal close', async () => { + const ctx = makeSettlement(); + const settle = createSettleFn( + ctx.settlement, + ctx.controller.signal, + ctx.abortRef, + ctx.rejectSpy, + TestLifecycleError, + 'test', + ); + settle(() => ctx.resolveSpy('ok')); + expect(await ctx.promise).toBe('ok'); + }); + + it('rejects when terminationPromise resolves with failure', async () => { + const ctx = makeSettlement(); + const failResult: ProcessTerminationResult = { outcome: 'failure' }; + ctx.settlement.terminationPromise = Promise.resolve(failResult); + const settle = createSettleFn( + ctx.settlement, + ctx.controller.signal, + ctx.abortRef, + ctx.rejectSpy, + TestLifecycleError, + 'test', + ); + settle(() => ctx.resolveSpy('should-not-happen')); + await expect(ctx.promise).rejects.toThrow('test termination failure'); + }); + + it('rejects when action throws synchronously', async () => { + const ctx = makeSettlement(); + const settle = createSettleFn( + ctx.settlement, + ctx.controller.signal, + ctx.abortRef, + ctx.rejectSpy, + TestLifecycleError, + 'test', + ); + settle(() => { + throw new Error('action boom'); + }); + await expect(ctx.promise).rejects.toThrow('action boom'); + }); + + it('rejects when terminationPromise rejects', async () => { + const ctx = makeSettlement(); + ctx.settlement.terminationPromise = Promise.reject( + new Error('termination rejected'), + ); + const settle = createSettleFn( + ctx.settlement, + ctx.controller.signal, + ctx.abortRef, + ctx.rejectSpy, + TestLifecycleError, + 'test', + ); + settle(() => ctx.resolveSpy('should-not-happen')); + await expect(ctx.promise).rejects.toThrow('termination rejected'); + }); + + it('duplicate settle calls do not double-resolve or double-reject', async () => { + const ctx = makeSettlement(); + let resolveCount = 0; + const wrappedResolve = (v: string) => { + resolveCount++; + ctx.resolveSpy(v); + }; + const settle = createSettleFn( + ctx.settlement, + ctx.controller.signal, + ctx.abortRef, + ctx.rejectSpy, + TestLifecycleError, + 'test', + ); + settle(() => wrappedResolve('first')); + settle(() => wrappedResolve('second')); + expect(await ctx.promise).toBe('first'); + expect(resolveCount).toBe(1); + }); + + it('does not produce an unhandled rejection when action throws', async () => { + const ctx = makeSettlement(); + const settle = createSettleFn( + ctx.settlement, + ctx.controller.signal, + ctx.abortRef, + ctx.rejectSpy, + TestLifecycleError, + 'test', + ); + settle(() => { + throw new Error('boom'); + }); + // The promise must reject (not hang forever) — no unhandled rejection. + const reason = await ctx.promise.catch((e: Error) => e); + expect(reason).toBeInstanceOf(Error); + expect((reason as Error).message).toBe('boom'); + }); + + it('removes the abort listener after settling', async () => { + const ctx = makeSettlement(); + let handlerRemoved = false; + const origRemove = ctx.controller.signal.removeEventListener.bind( + ctx.controller.signal, + ); + ctx.controller.signal.removeEventListener = (( + type: string, + listener: () => void, + ) => { + if (type === 'abort' && listener === ctx.abortRef.handler) { + handlerRemoved = true; + } + origRemove(type, listener); + }) as typeof ctx.controller.signal.removeEventListener; + + const settle = createSettleFn( + ctx.settlement, + ctx.controller.signal, + ctx.abortRef, + ctx.rejectSpy, + TestLifecycleError, + 'test', + ); + settle(() => ctx.resolveSpy('ok')); + await ctx.promise; + expect(handlerRemoved).toBe(true); + }); +}); diff --git a/packages/tools/src/utils/subprocessSettle.ts b/packages/tools/src/utils/subprocessSettle.ts new file mode 100644 index 0000000000..5df35f9178 --- /dev/null +++ b/packages/tools/src/utils/subprocessSettle.ts @@ -0,0 +1,57 @@ +/** + * @license + * Copyright 2026 Vybestack LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import type { ProcessTerminationResult } from './processTermination.js'; + +export interface SubprocessSettlement { + settled: boolean; + terminationPromise: Promise | null; +} + +export interface AbortHandlerRef { + handler: () => void; +} + +export function createSettleFn( + settlement: SubprocessSettlement, + abortSignal: AbortSignal, + abortHandlerRef: AbortHandlerRef, + reject: (error: Error) => void, + lifecycleErrorCtor: new (message: string) => Error, + command: string, +): (action: () => void) => void { + return (action: () => void) => { + if (settlement.settled) return; + void (async () => { + try { + if (settlement.terminationPromise !== null) { + const result = await settlement.terminationPromise; + if ( + !settlement.settled && + (result.outcome === 'timeout' || result.outcome === 'failure') + ) { + settlement.settled = true; + abortSignal.removeEventListener('abort', abortHandlerRef.handler); + reject( + new lifecycleErrorCtor( + `${command} termination ${result.outcome}`, + ), + ); + return; + } + } + if (settlement.settled) return; + settlement.settled = true; + abortSignal.removeEventListener('abort', abortHandlerRef.handler); + action(); + } catch (err) { + settlement.settled = true; + abortSignal.removeEventListener('abort', abortHandlerRef.handler); + reject(err instanceof Error ? err : new Error(String(err))); + } + })(); + }; +} diff --git a/project-plans/issue3203/PLAN.md b/project-plans/issue3203/PLAN.md new file mode 100644 index 0000000000..572bc507f6 --- /dev/null +++ b/project-plans/issue3203/PLAN.md @@ -0,0 +1,232 @@ +# Plan: Bound Discovered-Tool and Search Output Acquisition (Issue #3203) + +Plan ID: PLAN-20260810-ISSUE3203 +Generated: 2026-08-10 +Parent: #3202 +Dependency: #3200 / PR #3206 + +## Problem + +Discovered tools append complete stdout and stderr strings before returning. +The git-grep, system-grep, and ripgrep paths retain every output Buffer and call +`Buffer.concat()` only after the subprocess closes. Their existing match and +token limits therefore run too late to prevent process-memory exhaustion. +Discovered-tool cancellation also sends only SIGTERM to the immediate child and +can wait forever when the process or one of its descendants ignores it. + +## Verified Preflight + +- The explicit `@vybestack/llxprt-code-tools/acquisition.js` subpath delivered by + #3200 exists and exports `createDefaultByteBudget`, + `BoundedCombinedCollector`, bounded head/tail output, multibyte-safe decoding, + and `TruncationMetadata`. +- `BoundedCombinedCollector` enforces one aggregate budget across stdout and + stderr and preserves per-stream output. No second retention implementation is + needed and no `packages/core` import is permitted. +- The affected unbounded paths are: + - `DiscoveredTool.runChildProcess` in `tools/tool-registry.ts`; + - `tryGitGrep` and `setupSystemGrepHandlers` in + `tools/grep/search-strategies.ts`; + - `runRipgrepProcess` in `tools/ripGrep.ts`. +- Existing grep/ripgrep output parsing uses completion-time strings and splits + with host `os.EOL`; subprocess output must instead be framed incrementally + and accept both LF and CRLF. +- Existing search result contracts already expose partial-result concepts + (`SearchResults.wasLimited` and ripgrep's limited result presentation). +- Existing test infrastructure uses Bun and `bun:test`. Real filesystem and + subprocess behavior is exercised in `filesystem-tools.test.ts`; there is no + discovered-tool execution suite, so a focused behavioral suite is required. +- No acquisition contract change is presently required. Semantic line framing + may use a bounded incremental decoder while the shared collector remains the + sole owner of retained process output and omission accounting. If + implementation proves that the shared public contract must change, document + the exact change on #3202 before modifying it. + +## Formal Requirements + +### REQ-3203-01: Bounded shared acquisition + +**Full text:** Discovered-tool execution, grep, and ripgrep enforce a shared +aggregate stdout/stderr budget during acquisition, with no full-stream string +concatenation or completion-time `Buffer.concat` in those paths. + +**Behavior:** +- GIVEN a subprocess emits more bytes than the acquisition budget +- WHEN one of the affected tools consumes its stdout and stderr +- THEN retained output stays within one shared aggregate budget and exposes + exact omission metadata when the entire producer stream was observed. + +### REQ-3203-02: Accurate partial results + +**Full text:** Truncation metadata is surfaced so partial searches and tool +results are never presented as exhaustive. + +**Behavior:** +- GIVEN acquisition omits bytes or semantic early-stop ends a producer +- WHEN the tool formats its result +- THEN both model-facing and display output identify the result as limited, and + early-stop metadata does not claim an exact omitted-byte count. + +### REQ-3203-03: Semantic search early stop + +**Full text:** grep and ripgrep stop their subprocess once configured result +limits are satisfied where correctness permits. + +**Behavior:** +- GIVEN a synthetic search tree contains more usable matches than requested +- WHEN git-grep, system-grep, or ripgrep reaches the applicable aggregate/file + limit +- THEN it terminates the process tree through the bounded lifecycle helper, + returns retained matches, and marks the result limited. + +### REQ-3203-04: Bounded cancellation + +**Full text:** Discovered-tool cancellation terminates the process tree with +bounded escalation rather than waiting indefinitely after SIGTERM. + +**Behavior:** +- GIVEN a discovered tool and descendant ignore graceful termination +- WHEN its AbortSignal fires +- THEN POSIX sends SIGTERM and escalates to SIGKILL after a fixed grace period, + while Windows terminates the process tree with a bounded `taskkill /T /F` + operation, and execution settles. + +### REQ-3203-05: Cross-platform parsing and spawn behavior + +**Full text:** Windows process hiding and CRLF parsing remain correct. + +**Behavior:** +- GIVEN LF or CRLF records, including multibyte characters split across chunks +- WHEN grep/ripgrep parse subprocess output +- THEN identical matches are produced without replacement-character corruption; + all Windows child spawns retain `windowsHide: true`. + +## Architecture and Integration + +### Shared Acquisition + +Use one `BoundedCombinedCollector` per subprocess with +`createDefaultByteBudget()`. Feed raw stdout/stderr Buffers directly into it. +The collector is the sole retained-output buffer and supplies bounded per-stream +text plus durable metadata at completion. + +### Incremental Search Parsing + +Add a small streaming line framer that: + +1. incrementally decodes UTF-8 across chunk boundaries; +2. emits complete LF or CRLF records; +3. bounds an unterminated partial line so a single malicious line cannot become + a new unbounded buffer; +4. forwards complete records immediately to existing line parsers; +5. retains only matches that can affect the configured result contract. + +The line framer is semantic parsing, not a second output-retention collector. +It must never reconstruct or retain the full process stream. + +For grep, stop when `maxResults` usable retained matches are reached or when a +new file proves that the `maxFiles` set is complete. Respect `maxPerFile` while +counting usable matches. For ripgrep, pass each workspace directory's remaining +aggregate match allowance into the subprocess and stop at the 20,000-match +contract. Any early stop sets limited/partial metadata. + +### Process Lifecycle + +Add a tools-local, platform-aware process-tree termination helper. It must not +import `packages/core`. POSIX subprocesses that require tree termination are +spawned in their own process groups and receive SIGTERM followed by guarded +SIGKILL. Windows uses an explicitly spawned, bounded `taskkill /PID /T /F` +operation. The helper observes child exit/close, clears timers/listeners, and is +idempotent across abort, early-stop, and close races. + +### Result Integration + +- `DiscoveredTool`: include the shared omission notice in successful and error + results; preserve `(empty)`, error, exit-code, and signal formatting. +- grep: propagate acquisition/early-stop state through `SearchResults.wasLimited`. + Do not invent an exact total when early stopping means the producer's full + result count is unknowable. +- ripgrep: carry limited state separately from match count so reaching exactly + the configured limit is not falsely equated with proof of additional output. +- Existing token limiting remains a final model-facing safeguard and is not a + substitute for the byte budget. + +## Test-First Phases + +### Phase 01: Failing lifecycle and streaming-parser tests + +Create Bun behavioral tests before production changes: + +- LF and CRLF records split at every relevant boundary; +- multibyte UTF-8 characters split across chunks; +- one huge unterminated line and many tiny chunks remain bounded; +- graceful process exit prevents SIGKILL escalation; +- a real POSIX child that ignores SIGTERM is force-killed; Windows-specific + process-tree assertions execute on Windows and remain skipped elsewhere. + +Run the focused tests and confirm they fail for missing behavior, not because of +invalid test setup. + +### Phase 02: Implement bounded lifecycle and semantic framing + +Implement the smallest tools-local helpers needed to satisfy Phase 01. Keep +process policy out of `packages/tools/src/acquisition/`. Do not add a duplicate +head/tail collector or alter package boundaries. + +### Phase 03: Failing discovered-tool behavioral tests + +Add real-subprocess tests for: + +- aggregate interleaved stdout/stderr far beyond the default budget; +- multibyte content and both one-huge-chunk and many-small-chunk producers; +- truncation notice and bounded model/display output on success and failure; +- cancellation of a process tree that ignores SIGTERM. + +### Phase 04: Integrate DiscoveredTool + +Replace string accumulation with `BoundedCombinedCollector`, preserve stdin JSON +and existing result/error behavior, enable process-group ownership where needed, +and route AbortSignal cancellation through bounded process-tree termination. + +### Phase 05: Failing grep/ripgrep integration tests + +Use real temporary git repositories and synthetic trees to prove: + +- git-grep/system-grep/ripgrep do not materialize full producer output; +- subprocesses stop when semantic limits are satisfied; +- results are marked limited without claiming a false exhaustive total; +- acquisition-budget truncation is visible; +- LF/CRLF, multibyte, and Windows process-hiding behavior is preserved. + +Tests must assert observable results and process settlement, not only mock calls. + +### Phase 06: Integrate grep and ripgrep + +Replace all affected chunk arrays and completion-time concatenation with the +shared collector and streaming parser. Propagate partial metadata through result +formatting and use the common termination helper for abort and early-stop paths. +Preserve strategy fallback only for genuine strategy failures; intentional early +stop is a successful limited result, not a fallback trigger. + +### Phase 07: Regression and deferred-work sweep + +- Verify no `Buffer.concat(stdoutChunks|stderrChunks)`, full-stream string + append, SIGTERM-only cancellation, `os.EOL` subprocess parsing, TODO/HACK/STUB, + or silent truncation remains in the affected paths. +- Verify no new suppression directives, lint severity downgrades, complexity + threshold increases, ignored source blocks, or TypeScript suppressions. +- Run package-focused tests before the full repository gate. + +## Verification Gate + +The implementation must pass: + +- `npm run test` +- `npm run lint` +- `npm run typecheck` +- `npm run format` +- `npm run build` +- `bun scripts/start.ts --profile-load stepfun-37 "write me a haiku and nothing else"` + +Before push, run detached Open Code Review with `--timeout 20`, address findings, +and repeat the verification gate for substantive changes. From 942d1e62abb7ea1625fd9d6688de1b1028c6e921 Mon Sep 17 00:00:00 2001 From: acoliver Date: Mon, 10 Aug 2026 23:03:32 -0300 Subject: [PATCH 2/5] Make discovered-tool early-exit test portable --- .../discovered-tool-bounded-acquisition.test.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/tools/src/__tests__/discovered-tool-bounded-acquisition.test.ts b/packages/tools/src/__tests__/discovered-tool-bounded-acquisition.test.ts index 061ecfd4b9..41d5280c8c 100644 --- a/packages/tools/src/__tests__/discovered-tool-bounded-acquisition.test.ts +++ b/packages/tools/src/__tests__/discovered-tool-bounded-acquisition.test.ts @@ -452,11 +452,14 @@ describe('DiscoveredTool stdin EPIPE during early child exit', () => { const result = await executeTool(tool, largeParams); - const content = - typeof result.llmContent === 'string' ? result.llmContent : ''; - // Must not crash and must produce a well-formed result. expect(typeof result.llmContent).toBe('string'); - expect(content).toContain('Exit Code:'); + expect(result.returnDisplay).toBe(result.llmContent); + if (result.error !== undefined) { + expect(result.error.message).toBe(result.llmContent); + expect(result.llmContent).toContain('Exit Code:'); + } else { + expect(result.llmContent).toBe(''); + } }, { timeout: 15000 }, ); From 1726f043c8344111bd60b70009eed2e58deeb236 Mon Sep 17 00:00:00 2001 From: acoliver Date: Mon, 10 Aug 2026 23:16:00 -0300 Subject: [PATCH 3/5] Narrow discovered-tool test output explicitly --- .../discovered-tool-bounded-acquisition.test.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/tools/src/__tests__/discovered-tool-bounded-acquisition.test.ts b/packages/tools/src/__tests__/discovered-tool-bounded-acquisition.test.ts index 41d5280c8c..7bd4144ba3 100644 --- a/packages/tools/src/__tests__/discovered-tool-bounded-acquisition.test.ts +++ b/packages/tools/src/__tests__/discovered-tool-bounded-acquisition.test.ts @@ -452,13 +452,15 @@ describe('DiscoveredTool stdin EPIPE during early child exit', () => { const result = await executeTool(tool, largeParams); + const content = + typeof result.llmContent === 'string' ? result.llmContent : ''; expect(typeof result.llmContent).toBe('string'); - expect(result.returnDisplay).toBe(result.llmContent); + expect(result.returnDisplay).toBe(content); if (result.error !== undefined) { - expect(result.error.message).toBe(result.llmContent); - expect(result.llmContent).toContain('Exit Code:'); + expect(result.error.message).toBe(content); + expect(content).toContain('Exit Code:'); } else { - expect(result.llmContent).toBe(''); + expect(content).toBe(''); } }, { timeout: 15000 }, From 54f7f0b371e892496f1acffe81e4f1263c10f37b Mon Sep 17 00:00:00 2001 From: acoliver Date: Tue, 11 Aug 2026 01:04:59 -0300 Subject: [PATCH 4/5] Address bounded acquisition review findings --- ...iscovered-tool-bounded-acquisition.test.ts | 70 +++++ ...grep-ripgrep-issue3203-remediation.test.ts | 274 ++++++++++++++++-- .../grep-ripgrep-raw-truncation.test.ts | 207 +++++++++++++ packages/tools/src/tools/grep.ts | 2 +- packages/tools/src/tools/grep/grepBudget.ts | 137 +++++++++ .../src/tools/grep/javascriptFallback.ts | 130 +++------ packages/tools/src/tools/grep/ripgrepParse.ts | 13 + .../tools/src/tools/grep/search-strategies.ts | 113 ++------ packages/tools/src/tools/grep/types.ts | 2 +- packages/tools/src/tools/ripGrep.ts | 40 ++- packages/tools/src/tools/tool-registry.ts | 21 +- .../src/utils/processTermination.test.ts | 57 ++-- .../tools/src/utils/processTermination.ts | 8 +- 13 files changed, 818 insertions(+), 256 deletions(-) create mode 100644 packages/tools/src/__tests__/grep-ripgrep-raw-truncation.test.ts create mode 100644 packages/tools/src/tools/grep/grepBudget.ts diff --git a/packages/tools/src/__tests__/discovered-tool-bounded-acquisition.test.ts b/packages/tools/src/__tests__/discovered-tool-bounded-acquisition.test.ts index 7bd4144ba3..4c28836f68 100644 --- a/packages/tools/src/__tests__/discovered-tool-bounded-acquisition.test.ts +++ b/packages/tools/src/__tests__/discovered-tool-bounded-acquisition.test.ts @@ -7,6 +7,7 @@ import { describe, it, expect, beforeEach, afterEach } from 'bun:test'; import { writeFileSync, + readFileSync, mkdirSync, rmSync, chmodSync, @@ -497,3 +498,72 @@ describe('BoundedCombinedCollector deterministic single huge chunk', () => { expect(result.stdoutText).toHaveLength(1024); }); }); + +function hasErrorCode(error: unknown, code: string): boolean { + return error instanceof Error && 'code' in error && error.code === code; +} + +function killDescendantFromMarker(markerPath: string): void { + let descendantPid: number; + try { + descendantPid = Number.parseInt( + readFileSync(markerPath, 'utf8').trim(), + 10, + ); + } catch (error: unknown) { + if (hasErrorCode(error, 'ENOENT')) return; + throw error; + } + if (!Number.isSafeInteger(descendantPid) || descendantPid <= 0) return; + try { + process.kill(descendantPid, 'SIGKILL'); + } catch (error: unknown) { + if (!hasErrorCode(error, 'ESRCH')) throw error; + } +} + +describe('DiscoveredTool drain timeout terminates descendant group', () => { + let tempDir: string; + let markerPath: string; + let cleanup: () => void; + + beforeEach(() => { + const tmp = createTempDir(); + tempDir = tmp.dir; + markerPath = join(tempDir, 'descendant.pid'); + cleanup = tmp.cleanup; + }); + + afterEach(() => { + try { + killDescendantFromMarker(markerPath); + } finally { + cleanup(); + } + }); + + it.skipIf(process.platform === 'win32')( + 'kills a descendant holding inherited pipes after the leader exits', + async () => { + const script = createScript( + tempDir, + 'leak.sh', + `#!/bin/sh\n(sleep 30) &\necho $! > "${markerPath}"\nexit 0`, + ); + const tool = createDiscoveredTool(script); + + await executeTool(tool); + + await new Promise((resolve) => setTimeout(resolve, 500)); + + const descendantPid = Number.parseInt( + readFileSync(markerPath, 'utf8').trim(), + 10, + ); + expect(descendantPid).toBeGreaterThan(0); + + expect(() => process.kill(descendantPid, 0)).toThrow('ESRCH'); + }, + { timeout: 15000 }, + ); +}); diff --git a/packages/tools/src/__tests__/grep-ripgrep-issue3203-remediation.test.ts b/packages/tools/src/__tests__/grep-ripgrep-issue3203-remediation.test.ts index c57af330dc..ebf5633fbe 100644 --- a/packages/tools/src/__tests__/grep-ripgrep-issue3203-remediation.test.ts +++ b/packages/tools/src/__tests__/grep-ripgrep-issue3203-remediation.test.ts @@ -5,16 +5,23 @@ */ import { describe, it, expect, beforeEach, afterEach } from 'bun:test'; -import { writeFileSync, mkdirSync, rmSync } from 'node:fs'; +import { chmodSync, mkdirSync, rmSync, writeFileSync } from 'node:fs'; import { execSync } from 'node:child_process'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; import type { IToolHost } from '../interfaces/index.js'; +import { + type SemanticBudget, + createAggregateSemanticBudget, + createGrepRetainState, + retainGrepMatch, +} from '../tools/grep/grepBudget.js'; import { GrepTool, RipGrepTool } from '../index.js'; import type { ToolResult } from '../index.js'; import type { GrepToolParams } from '../tools/grep/types.js'; import type { RipGrepToolParams } from '../tools/ripGrep.js'; +const itPosix = process.platform === 'win32' ? it.skip : it; function createTempDir(prefix = 'llxprt-grep-remediation-'): { dir: string; cleanup: () => void; @@ -192,28 +199,40 @@ describe('Exact-limit evidence: producer at exactly the cap is exhaustive (item }); describe('Strategy budget rollback: failed strategy does not starve fallback (item 4)', () => { - it( - 'git grep failure restores budget for system grep', + itPosix( + 'restores the budget consumed by a failed git grep before system grep', async () => { - const { performGrepSearch, createAggregateSemanticBudget } = await import( + const { performGrepSearch } = await import( '../tools/grep/search-strategies.js' ); - const tmp = createTempDir('llxprt-budget-rollback-'); + const workspace = createTempDir('llxprt-budget-rollback-'); + const fakeCommand = createTempDir('llxprt-fake-git-'); + const originalPath = process.env.PATH; try { - initGitRepo(tmp.dir); + initGitRepo(workspace.dir); for (let i = 0; i < 10; i++) { - writeFileSync(join(tmp.dir, `f${i}.txt`), `match_line_${i}\n`); + writeFileSync(join(workspace.dir, `f${i}.txt`), `match_line_${i}\n`); } - gitAdd(tmp.dir); - - const budget = createAggregateSemanticBudget(); - const initialBytes = budget.remainingBytes; - const initialObjects = budget.remainingObjects; + gitAdd(workspace.dir); + + const fakeOutput = join(fakeCommand.dir, 'output.txt'); + const fakeGit = join(fakeCommand.dir, 'git'); + writeFileSync(fakeOutput, `f0.txt:1:match_line_${'x'.repeat(3000)}\n`); + writeFileSync( + fakeGit, + `#!/bin/sh\nif [ "$1" = "--version" ]; then\n echo "git version test"\n exit 0\nfi\ncat ${JSON.stringify(fakeOutput)}\necho "forced failure" >&2\nexit 2\n`, + ); + chmodSync(fakeGit, 0o755); + process.env.PATH = `${fakeCommand.dir}:${originalPath ?? ''}`; + const budget: SemanticBudget = { + remainingBytes: 4000, + remainingObjects: 100, + }; const result = await performGrepSearch( { pattern: 'match_line', - path: tmp.dir, + path: workspace.dir, signal: new AbortController().signal, maxResults: 100, maxFiles: 100, @@ -223,11 +242,18 @@ describe('Strategy budget rollback: failed strategy does not starve fallback (it ['node_modules'], ); - expect(result.results.length).toBeGreaterThan(0); - expect(budget.remainingBytes).toBeLessThanOrEqual(initialBytes); - expect(budget.remainingObjects).toBeLessThanOrEqual(initialObjects); + expect(result.results).toHaveLength(10); + expect(result.incomplete).not.toBe(true); + expect(budget.remainingObjects).toBe(90); + expect(budget.remainingBytes).toBeGreaterThan(0); } finally { - tmp.cleanup(); + if (originalPath === undefined) { + delete process.env.PATH; + } else { + process.env.PATH = originalPath; + } + fakeCommand.cleanup(); + workspace.cleanup(); } }, { timeout: 15000 }, @@ -268,14 +294,16 @@ describe('JavaScript fallback maxFiles prompt stop (item 5)', () => { 5, 50, ['node_modules'], + createAggregateSemanticBudget(), ); - expect(result.results.length).toBeLessThanOrEqual(5); + expect(result.results).toHaveLength(5); + expect(new Set(result.results.map((match) => match.filePath)).size).toBe( + 5, + ); expect(result.incomplete).toBe(true); expect(result.wasLimited).toBe(true); - expect(result.observedCount).toBeGreaterThanOrEqual( - result.results.length, - ); + expect(result.observedCount).toBe(6); }, { timeout: 15000 }, ); @@ -300,10 +328,15 @@ describe('JavaScript fallback maxFiles prompt stop (item 5)', () => { 3, 50, ['node_modules'], + createAggregateSemanticBudget(), ); - expect(result.results.length).toBeLessThanOrEqual(3); + expect(result.results).toHaveLength(3); + expect(new Set(result.results.map((match) => match.filePath)).size).toBe( + 3, + ); expect(result.incomplete).toBe(true); + expect(result.observedCount).toBe(4); expect(result.totalFound).toBeUndefined(); }, { timeout: 15000 }, @@ -344,4 +377,201 @@ describe('Ripgrep multi-root budget exhaustion stops further spawns (item 11)', }, { timeout: 30000 }, ); + + describe('JavaScript fallback consumes shared SemanticBudget (finding 1)', () => { + let tempDir: string; + let cleanup: () => void; + + beforeEach(() => { + const tmp = createTempDir(); + tempDir = tmp.dir; + cleanup = tmp.cleanup; + }); + + afterEach(() => { + cleanup(); + }); + + it( + 'decrements semanticBudget.remainingBytes and remainingObjects after retaining matches', + async () => { + const { javascriptGrepFallback } = await import( + '../tools/grep/javascriptFallback.js' + ); + + writeFileSync(join(tempDir, 'f1.txt'), 'match alpha\nmatch beta\n'); + writeFileSync(join(tempDir, 'f2.txt'), 'match gamma\n'); + + const budget = createAggregateSemanticBudget(); + const initialBytes = budget.remainingBytes; + const initialObjects = budget.remainingObjects; + + await javascriptGrepFallback( + 'match', + tempDir, + undefined, + new AbortController().signal, + 1000, + 100, + 50, + ['node_modules'], + budget, + ); + + expect(budget.remainingBytes).toBeLessThan(initialBytes); + expect(budget.remainingObjects).toBeLessThan(initialObjects); + }, + { timeout: 15000 }, + ); + + it( + 'marks incomplete and stops when semantic byte budget is exhausted', + async () => { + const { javascriptGrepFallback } = await import( + '../tools/grep/javascriptFallback.js' + ); + + const longLine = 'Z'.repeat(10_000); + for (let i = 0; i < 50; i++) { + writeFileSync(join(tempDir, `f${i}.txt`), `match${longLine}\n`); + } + + const tightBudget: SemanticBudget = { + remainingBytes: 50_000, + remainingObjects: 100_000, + }; + + const result = await javascriptGrepFallback( + 'match', + tempDir, + undefined, + new AbortController().signal, + 100_000, + 100, + 50, + ['node_modules'], + tightBudget, + ); + + expect(result.incomplete).toBe(true); + expect(result.results.length).toBeLessThan(50); + expect(tightBudget.remainingBytes).toBeLessThan(50_000); + }, + { timeout: 15000 }, + ); + }); + + describe('JavaScript fallback maxResults early stop is proven-incomplete (finding 2)', () => { + let tempDir: string; + let cleanup: () => void; + + beforeEach(() => { + const tmp = createTempDir(); + tempDir = tmp.dir; + cleanup = tmp.cleanup; + }); + + afterEach(() => { + cleanup(); + }); + + it( + 'exactly maxResults matches is NOT incomplete (exact-cap evidence)', + async () => { + const { javascriptGrepFallback } = await import( + '../tools/grep/javascriptFallback.js' + ); + + for (let i = 0; i < 5; i++) { + writeFileSync(join(tempDir, `f${i}.txt`), `match_line_${i}\n`); + } + + const result = await javascriptGrepFallback( + 'match_line', + tempDir, + undefined, + new AbortController().signal, + 5, + 100, + 50, + ['node_modules'], + createAggregateSemanticBudget(), + ); + + expect(result.results.length).toBe(5); + expect(result.incomplete).toBe(false); + }, + { timeout: 15000 }, + ); + + it( + 'maxResults+1 matches IS incomplete (extra match proves omission)', + async () => { + const { javascriptGrepFallback } = await import( + '../tools/grep/javascriptFallback.js' + ); + + for (let i = 0; i < 6; i++) { + writeFileSync(join(tempDir, `f${i}.txt`), `match_line_${i}\n`); + } + + const result = await javascriptGrepFallback( + 'match_line', + tempDir, + undefined, + new AbortController().signal, + 5, + 100, + 50, + ['node_modules'], + createAggregateSemanticBudget(), + ); + + expect(result.results.length).toBe(5); + expect(result.incomplete).toBe(true); + }, + { timeout: 15000 }, + ); + }); +}); + +describe('Grep exact-cap evidence with per-file limits', () => { + it('ignores unusable dominant-file matches until a later usable match proves omission', () => { + const state = createGrepRetainState(createAggregateSemanticBudget()); + const limits = { maxResults: 2, maxFiles: 10, maxPerFile: 2 }; + + retainGrepMatch( + state, + { filePath: 'dominant.txt', lineNumber: 1, line: 'match 1' }, + limits, + ); + retainGrepMatch( + state, + { filePath: 'dominant.txt', lineNumber: 2, line: 'match 2' }, + limits, + ); + const dominantOverflowStopped = retainGrepMatch( + state, + { filePath: 'dominant.txt', lineNumber: 3, line: 'match 3' }, + limits, + ); + + expect(dominantOverflowStopped).toBe(false); + expect(state.earlyStopped).toBe(false); + expect(state.matches).toHaveLength(2); + + const laterUsableMatchStopped = retainGrepMatch( + state, + { filePath: 'later.txt', lineNumber: 1, line: 'match later' }, + limits, + ); + + expect(laterUsableMatchStopped).toBe(true); + expect(state.earlyStopped).toBe(true); + expect(state.observedCount).toBe(4); + expect(state.matches.map((match) => match.line)).toEqual([ + 'match 1', + 'match 2', + ]); + }); }); diff --git a/packages/tools/src/__tests__/grep-ripgrep-raw-truncation.test.ts b/packages/tools/src/__tests__/grep-ripgrep-raw-truncation.test.ts new file mode 100644 index 0000000000..232e059bcc --- /dev/null +++ b/packages/tools/src/__tests__/grep-ripgrep-raw-truncation.test.ts @@ -0,0 +1,207 @@ +/** + * @license + * Copyright 2026 Vybestack LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, beforeEach, afterEach } from 'bun:test'; +import { writeFileSync, mkdirSync, rmSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import type { IToolHost } from '../interfaces/index.js'; +import { RipGrepTool } from '../index.js'; +import type { ToolResult } from '../index.js'; +import type { RipGrepToolParams } from '../tools/ripGrep.js'; +import { + resolveRipgrepClose, + createRipgrepAcquisitionState, + processRipgrepStdoutChunk, + createAggregateSemanticBudget, +} from '../tools/ripGrep.js'; + +function createTempDir(prefix = 'llxprt-raw-trunc-'): { + dir: string; + cleanup: () => void; +} { + const dir = join( + tmpdir(), + `${prefix}${Date.now()}-${Math.random().toString(36).slice(2)}`, + ); + mkdirSync(dir, { recursive: true }); + return { + dir, + cleanup: () => { + try { + rmSync(dir, { recursive: true, force: true }); + } catch { + /* ignore */ + } + }, + }; +} + +function createToolHost(targetDir: string): IToolHost { + return { + getTargetDir: () => targetDir, + getWorkspaceRoots: () => [targetDir], + getApprovalMode: () => 'auto', + setApprovalMode: () => {}, + isInteractive: () => false, + hasFeatureFlag: () => false, + getFileService: () => ({ + shouldGitIgnoreFile: () => false, + shouldLlxprtIgnoreFile: () => false, + shouldIgnoreFile: () => false, + filterFiles: (paths) => paths, + }), + getFileFilteringOptions: () => ({ + respectGitIgnore: true, + respectLlxprtIgnore: true, + }), + getFileExclusions: () => [], + getReadManyFilesExclusions: () => [], + getFileFilteringRespectLlxprtIgnore: () => true, + getLlxprtIgnoreFilePath: () => null, + recordFileRead: () => {}, + getFileSystemService: () => undefined, + getLlxprtIgnorePatterns: () => [], + getEphemeralSettings: () => ({ + 'tool-output-max-items': 50, + 'tool-output-max-tokens': 50000, + 'tool-output-item-size-limit': 524288, + }), + getDebugMode: () => false, + }; +} + +async function executeRipgrep( + host: IToolHost, + params: RipGrepToolParams, +): Promise { + const tool = new RipGrepTool(host); + try { + return await tool.build(params).execute(new AbortController().signal); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + return { llmContent: message, returnDisplay: message }; + } +} + +describe('resolveRipgrepClose: raw collector truncation vs semantic budget (finding 3)', () => { + it('stderr-only overflow does NOT mark budgetTruncated (parsed results complete)', () => { + const basePath = '/test'; + const budget = createAggregateSemanticBudget(); + const state = createRipgrepAcquisitionState(budget); + + const stdoutChunk = Buffer.from('file.txt\x005:match content\n'); + processRipgrepStdoutChunk(state, stdoutChunk, basePath, 20000); + + const largeStderr = Buffer.alloc(5 * 1024 * 1024, 0x45); + state.collector.append(largeStderr, 'stderr'); + + const outcome = resolveRipgrepClose(0, null, state, basePath, 20000, false); + + expect(outcome.error).toBeUndefined(); + expect(outcome.result).toBeDefined(); + expect(outcome.result!.budgetTruncated).toBe(false); + expect(outcome.result!.rawTruncated).toBe(true); + expect(outcome.result!.matches.length).toBe(1); + }); + + it('semantic budget exhaustion marks budgetTruncated (match data incomplete)', () => { + const basePath = '/test'; + const budget = createAggregateSemanticBudget(); + budget.remainingBytes = 300; + + const state = createRipgrepAcquisitionState(budget); + + const stdoutChunk = Buffer.from( + 'file.txt\x001:short\nfile.txt\x002:another\nfile.txt\x003:third\n', + ); + processRipgrepStdoutChunk(state, stdoutChunk, basePath, 20000); + + const outcome = resolveRipgrepClose(0, null, state, basePath, 20000, false); + + expect(outcome.result).toBeDefined(); + expect(outcome.result!.budgetTruncated).toBe(true); + }); + + it('stderr-only overflow with zero stdout matches is complete (no false incomplete)', () => { + const basePath = '/test'; + const budget = createAggregateSemanticBudget(); + const state = createRipgrepAcquisitionState(budget); + + const largeStderr = Buffer.alloc(5 * 1024 * 1024, 0x57); + state.collector.append(largeStderr, 'stderr'); + + const outcome = resolveRipgrepClose(0, null, state, basePath, 20000, false); + + expect(outcome.error).toBeUndefined(); + expect(outcome.result).toBeDefined(); + expect(outcome.result!.budgetTruncated).toBe(false); + expect(outcome.result!.rawTruncated).toBe(true); + expect(outcome.result!.matches.length).toBe(0); + }); + + it('labels omitted diagnostic bytes when a failing ripgrep writes excessive stderr', () => { + const basePath = '/test'; + const state = createRipgrepAcquisitionState( + createAggregateSemanticBudget(), + ); + + state.collector.append(Buffer.alloc(5 * 1024 * 1024, 0x45), 'stderr'); + + const outcome = resolveRipgrepClose(2, null, state, basePath, 20000, false); + + expect(outcome.result).toBeUndefined(); + expect(outcome.error?.message).toContain('ripgrep exited with code 2'); + expect(outcome.error?.message).toContain( + '[LLXPRT output truncated: 1,048,576 bytes omitted]', + ); + }); +}); + +describe('Multi-root continuation despite verbose stderr (finding 3)', () => { + let tempDir: string; + let cleanup: () => void; + + beforeEach(() => { + const tmp = createTempDir(); + tempDir = tmp.dir; + cleanup = tmp.cleanup; + }); + + afterEach(() => { + cleanup(); + }); + + it( + 'searches all roots and returns complete results when no semantic exhaustion', + async () => { + const dirs: string[] = []; + for (let i = 0; i < 3; i++) { + const dir = join(tempDir, `ws${i}`); + mkdirSync(dir, { recursive: true }); + writeFileSync(join(dir, `f${i}.txt`), `uniquematch_${i}\n`); + dirs.push(dir); + } + + const host: IToolHost = { + ...createToolHost(tempDir), + getWorkspaceRoots: () => dirs, + }; + + const result = await executeRipgrep(host, { + pattern: 'uniquematch', + }); + + const text = + typeof result.llmContent === 'string' ? result.llmContent : ''; + expect(text).not.toMatch(/incomplete|showing/i); + expect(text).toContain('uniquematch_0'); + expect(text).toContain('uniquematch_1'); + expect(text).toContain('uniquematch_2'); + }, + { timeout: 15000 }, + ); +}); diff --git a/packages/tools/src/tools/grep.ts b/packages/tools/src/tools/grep.ts index ec7a2566c3..1f309d1179 100644 --- a/packages/tools/src/tools/grep.ts +++ b/packages/tools/src/tools/grep.ts @@ -37,8 +37,8 @@ import { import { performGrepSearch, performSingleFileSearch, - createAggregateSemanticBudget, } from './grep/search-strategies.js'; +import { createAggregateSemanticBudget } from './grep/grepBudget.js'; export { type GrepToolParams } from './grep/types.js'; diff --git a/packages/tools/src/tools/grep/grepBudget.ts b/packages/tools/src/tools/grep/grepBudget.ts new file mode 100644 index 0000000000..f39060993e --- /dev/null +++ b/packages/tools/src/tools/grep/grepBudget.ts @@ -0,0 +1,137 @@ +/** + * @license + * Copyright 2026 Vybestack LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { DEFAULT_ACQUISITION_BUDGET_BYTES } from '../../acquisition/index.js'; +import type { GrepMatch } from './types.js'; + +/** + * Aggregate semantic budget shared across all roots and strategies for a + * single invocation. Tracks remaining bytes and objects (matches) that may + * be retained in bounded semantic storage. + */ +export interface SemanticBudget { + remainingBytes: number; + remainingObjects: number; +} + +/** Per-match overhead added to the raw line/filePath byte cost. */ +export const MATCH_OVERHEAD_BYTES = 256; + +/** Hard ceiling on the number of retained matches for one invocation. */ +export const HARD_RETAINED_MATCH_CAP = 100_000; + +/** Create a fresh aggregate semantic budget at the default acquisition size. */ +export function createAggregateSemanticBudget(): SemanticBudget { + return { + remainingBytes: DEFAULT_ACQUISITION_BUDGET_BYTES, + remainingObjects: HARD_RETAINED_MATCH_CAP, + }; +} + +/** Per-invocation limits applied during match retention. */ +export interface GrepLimits { + readonly maxResults: number; + readonly maxFiles: number; + readonly maxPerFile: number; +} + +/** + * Bounded retention state shared by subprocess strategies and the JavaScript + * fallback. Tracks observed vs. retained matches, per-file counts, and the + * aggregate semantic budget. + */ +export interface GrepRetainState { + matches: GrepMatch[]; + perFileCount: Map; + filesSeen: Set; + observedCount: number; + usableCount: number; + semanticBudget: SemanticBudget; + earlyStopped: boolean; + capReached: boolean; + budgetExhausted: boolean; +} + +export function createGrepRetainState( + semanticBudget: SemanticBudget, +): GrepRetainState { + return { + matches: [], + perFileCount: new Map(), + filesSeen: new Set(), + observedCount: 0, + usableCount: 0, + semanticBudget, + earlyStopped: false, + capReached: false, + budgetExhausted: false, + }; +} + +/** + * Attempt to retain a parsed grep match in bounded semantic storage. + * + * Matches beyond {@link GrepLimits.maxPerFile} for a single file are counted + * (observed) but NOT retained, preventing a dominant file from growing the + * matches array without bound. Retained matches are also capped by the + * aggregate semantic byte budget. + * + * When the usable match count reaches {@link GrepLimits.maxResults}, + * `capReached` is set but `earlyStopped` is deferred until one additional + * usable match is observed — preserving exact-cap evidence semantics: merely + * retaining exactly the requested count does not prove omission. + * + * Returns true if acquisition should stop (early stop or budget exhaustion). + */ +export function retainGrepMatch( + state: GrepRetainState, + match: GrepMatch, + limits: GrepLimits, +): boolean { + state.observedCount++; + + const newFile = !state.filesSeen.has(match.filePath); + if (newFile && state.filesSeen.size >= limits.maxFiles) { + state.earlyStopped = true; + return true; + } + + state.filesSeen.add(match.filePath); + const fileCount = (state.perFileCount.get(match.filePath) ?? 0) + 1; + state.perFileCount.set(match.filePath, fileCount); + + if (fileCount > limits.maxPerFile) { + return false; + } + + if (state.capReached) { + state.earlyStopped = true; + return true; + } + + const matchBytes = + Buffer.byteLength(match.line, 'utf8') + + Buffer.byteLength(match.filePath, 'utf8') + + MATCH_OVERHEAD_BYTES; + if ( + state.semanticBudget.remainingBytes < matchBytes || + state.semanticBudget.remainingObjects <= 0 + ) { + state.budgetExhausted = true; + state.earlyStopped = true; + return true; + } + state.matches.push(match); + state.semanticBudget.remainingBytes -= matchBytes; + state.semanticBudget.remainingObjects--; + state.usableCount++; + + if (state.usableCount >= limits.maxResults) { + state.capReached = true; + } + + return state.earlyStopped; +} diff --git a/packages/tools/src/tools/grep/javascriptFallback.ts b/packages/tools/src/tools/grep/javascriptFallback.ts index f01c887ef9..d6437b39dc 100644 --- a/packages/tools/src/tools/grep/javascriptFallback.ts +++ b/packages/tools/src/tools/grep/javascriptFallback.ts @@ -11,81 +11,43 @@ import { globStream } from 'glob'; import { getErrorMessage, isNodeError } from '../../utils/errors.js'; import { debugLogger } from '../../utils/debugLogger.js'; import type { GrepMatch, SearchResults } from './types.js'; - -function extractMatchesFromFile( - lines: string[], - fileAbsolutePath: string, - absolutePath: string, - regex: RegExp, - maxPerFile: number, - maxResults: number, - allMatches: GrepMatch[], - filesWithMatches: Set, -): number { - let matchesInFile = 0; - let totalFound = 0; - - lines.forEach((line, index) => { - if (regex.test(line)) { - totalFound++; - if (matchesInFile < maxPerFile && allMatches.length < maxResults) { - allMatches.push({ - filePath: - path.relative(absolutePath, fileAbsolutePath) || - path.basename(fileAbsolutePath), - lineNumber: index + 1, - line, - }); - matchesInFile++; - filesWithMatches.add(fileAbsolutePath); - } - } - }); - - return totalFound; -} - -function shouldProcessFile( - allMatchesLength: number, - maxResults: number, - filesWithMatchesSize: number, - maxFiles: number, - isKnownFile: boolean, -): boolean { - if (allMatchesLength >= maxResults) return false; - if (filesWithMatchesSize >= maxFiles && !isKnownFile) return false; - return true; -} +import { + type SemanticBudget, + type GrepLimits, + type GrepRetainState, + createGrepRetainState, + retainGrepMatch, +} from './grepBudget.js'; async function processFallbackFile( + state: GrepRetainState, filePath: string, absolutePath: string, regex: RegExp, - maxPerFile: number, - maxResults: number, - allMatches: GrepMatch[], - filesWithMatches: Set, -): Promise { + limits: GrepLimits, +): Promise { + let content: string; try { - const content = await fsPromises.readFile(filePath, 'utf8'); - const lines = content.split(/\r?\n/); - return extractMatchesFromFile( - lines, - filePath, - absolutePath, - regex, - maxPerFile, - maxResults, - allMatches, - filesWithMatches, - ); + content = await fsPromises.readFile(filePath, 'utf8'); } catch (readError: unknown) { if (!isNodeError(readError) || readError.code !== 'ENOENT') { debugLogger.debug( `GrepLogic: Could not read/process ${filePath}: ${getErrorMessage(readError)}`, ); } - return 0; + return; + } + const lines = content.split(/\r?\n/); + for (let i = 0; i < lines.length && !state.earlyStopped; i++) { + if (regex.test(lines[i])) { + const match: GrepMatch = { + filePath: + path.relative(absolutePath, filePath) || path.basename(filePath), + lineNumber: i + 1, + line: lines[i], + }; + retainGrepMatch(state, match, limits); + } } } @@ -98,6 +60,7 @@ export async function javascriptGrepFallback( maxFiles: number, maxPerFile: number, fileExclusions: readonly string[], + semanticBudget: SemanticBudget, ): Promise { const globPattern = include ?? '**/*'; const filesStream = globStream(globPattern, { @@ -110,43 +73,24 @@ export async function javascriptGrepFallback( }); const regex = new RegExp(pattern, 'i'); - const allMatches: GrepMatch[] = []; - const filesWithMatches = new Set(); - let totalFound = 0; - let filesLimitHit = false; + const limits: GrepLimits = { maxResults, maxFiles, maxPerFile }; + const state = createGrepRetainState(semanticBudget); for await (const filePath of filesStream) { - if ( - !shouldProcessFile( - allMatches.length, - maxResults, - filesWithMatches.size, - maxFiles, - filesWithMatches.has(filePath), - ) - ) { - if (filesWithMatches.size >= maxFiles) filesLimitHit = true; - break; - } - totalFound += await processFallbackFile( - filePath, - absolutePath, - regex, - maxPerFile, - maxResults, - allMatches, - filesWithMatches, - ); + if (state.earlyStopped) break; + await processFallbackFile(state, filePath, absolutePath, regex, limits); } - const incomplete = filesLimitHit; + const incomplete = state.earlyStopped || state.budgetExhausted; const totalFoundValue = - incomplete || totalFound <= allMatches.length ? undefined : totalFound; + incomplete || state.observedCount <= state.usableCount + ? undefined + : state.observedCount; return { - results: allMatches, - wasLimited: totalFound > allMatches.length || filesLimitHit, + results: state.matches, + wasLimited: state.observedCount > state.usableCount || incomplete, totalFound: totalFoundValue, incomplete, - observedCount: totalFound, + observedCount: state.observedCount, }; } diff --git a/packages/tools/src/tools/grep/ripgrepParse.ts b/packages/tools/src/tools/grep/ripgrepParse.ts index 956466231b..f409eadeaf 100644 --- a/packages/tools/src/tools/grep/ripgrepParse.ts +++ b/packages/tools/src/tools/grep/ripgrepParse.ts @@ -5,6 +5,7 @@ */ import path from 'path'; +import type { CombinedAcquisitionResult } from '../../acquisition/index.js'; import type { GrepMatch } from './types.js'; export function parseRipgrepLine( @@ -37,3 +38,15 @@ export function parseRipgrepLine( line: lineContent, }; } + +export function formatRipgrepDiagnostic( + acquisition: CombinedAcquisitionResult, +): string { + const stderr = acquisition.stderrText.trim(); + if (acquisition.omissionNotice === null) { + return stderr; + } + return stderr.length === 0 + ? acquisition.omissionNotice + : `${stderr}\n${acquisition.omissionNotice}`; +} diff --git a/packages/tools/src/tools/grep/search-strategies.ts b/packages/tools/src/tools/grep/search-strategies.ts index 1c8817258d..e7f277712a 100644 --- a/packages/tools/src/tools/grep/search-strategies.ts +++ b/packages/tools/src/tools/grep/search-strategies.ts @@ -21,7 +21,6 @@ import { javascriptGrepFallback } from './javascriptFallback.js'; import { BoundedCombinedCollector, createDefaultByteBudget, - DEFAULT_ACQUISITION_BUDGET_BYTES, } from '../../acquisition/index.js'; import { BoundedLineFramer } from '../../utils/lineFramer.js'; import { terminateProcessTree } from '../../utils/processTermination.js'; @@ -30,6 +29,14 @@ import { type SubprocessSettlement, type AbortHandlerRef, } from '../../utils/subprocessSettle.js'; +import { + type SemanticBudget, + type GrepLimits, + type GrepRetainState, + createAggregateSemanticBudget, + createGrepRetainState, + retainGrepMatch, +} from './grepBudget.js'; /** * Checks if a glob pattern contains brace expansion syntax that git grep doesn't support. @@ -234,21 +241,6 @@ interface BoundedGrepSubprocessOptions { tolerateNonZeroExitWithoutStderr?: boolean; } -export interface SemanticBudget { - remainingBytes: number; - remainingObjects: number; -} - -export const MATCH_OVERHEAD_BYTES = 256; -export const HARD_RETAINED_MATCH_CAP = 100_000; - -export function createAggregateSemanticBudget(): SemanticBudget { - return { - remainingBytes: DEFAULT_ACQUISITION_BUDGET_BYTES, - remainingObjects: HARD_RETAINED_MATCH_CAP, - }; -} - function buildSearchResults( matches: GrepMatch[], observedCount: number, @@ -271,25 +263,13 @@ function buildSearchResults( }; } -interface GrepLimits { - maxResults: number; - maxFiles: number; - maxPerFile: number; -} - -interface GrepAcquisitionState { +/** + * Acquisition state for a grep subprocess. Extends the shared bounded + * retention state with subprocess-specific collection/framing fields. + */ +interface GrepAcquisitionState extends GrepRetainState { collector: BoundedCombinedCollector; framer: BoundedLineFramer; - matches: GrepMatch[]; - perFileCount: Map; - filesSeen: Set; - observedCount: number; - usableCount: number; - retainedBytes: number; - semanticBudget: SemanticBudget; - earlyStopped: boolean; - capReached: boolean; - budgetExhausted: boolean; terminated: boolean; readonly limits: GrepLimits; } @@ -299,76 +279,16 @@ function createGrepAcquisitionState( semanticBudget: SemanticBudget, ): GrepAcquisitionState { return { + ...createGrepRetainState(semanticBudget), collector: new BoundedCombinedCollector({ budget: createDefaultByteBudget(), }), framer: new BoundedLineFramer(), - matches: [], - perFileCount: new Map(), - filesSeen: new Set(), - observedCount: 0, - usableCount: 0, - retainedBytes: 0, - semanticBudget, - earlyStopped: false, - capReached: false, - budgetExhausted: false, terminated: false, limits, }; } -/** - * Attempt to retain a parsed grep match in bounded semantic storage. - * - * Matches beyond {@link GrepLimits.maxPerFile} for a single file are counted - * but NOT retained, preventing a dominant file from growing the matches array - * without bound. Retained matches are also capped by an aggregate semantic - * byte budget tied to the same acquisition budget. - */ -function tryRetainGrepMatch( - state: GrepAcquisitionState, - match: GrepMatch, -): void { - state.observedCount++; - - if (state.capReached) { - state.earlyStopped = true; - return; - } - - state.filesSeen.add(match.filePath); - const fc = (state.perFileCount.get(match.filePath) ?? 0) + 1; - state.perFileCount.set(match.filePath, fc); - - if (fc <= state.limits.maxPerFile) { - const matchBytes = - Buffer.byteLength(match.line, 'utf8') + - Buffer.byteLength(match.filePath, 'utf8') + - MATCH_OVERHEAD_BYTES; - if ( - state.semanticBudget.remainingBytes < matchBytes || - state.semanticBudget.remainingObjects <= 0 - ) { - state.budgetExhausted = true; - state.earlyStopped = true; - return; - } - state.matches.push(match); - state.retainedBytes += matchBytes; - state.semanticBudget.remainingBytes -= matchBytes; - state.semanticBudget.remainingObjects--; - state.usableCount++; - } - - if (state.usableCount >= state.limits.maxResults) { - state.capReached = true; - } - if (state.filesSeen.size > state.limits.maxFiles) { - state.earlyStopped = true; - } -} - /** * Feed a stdout chunk into the collector and framer, consuming each complete * bounded line record-at-a-time via callback. Returns true if early stop @@ -386,7 +306,7 @@ function processGrepStdoutChunk( if (state.earlyStopped) return; const match = parseGrepLine(line, cwd); if (!match) return; - tryRetainGrepMatch(state, match); + retainGrepMatch(state, match, state.limits); }); return state.earlyStopped; @@ -398,7 +318,7 @@ function flushGrepLines(state: GrepAcquisitionState, cwd: string): void { if (state.earlyStopped) return; const match = parseGrepLine(line, cwd); if (!match) return; - tryRetainGrepMatch(state, match); + retainGrepMatch(state, match, state.limits); }); } @@ -891,6 +811,7 @@ export async function performGrepSearch( maxFiles, maxPerFile, fileExclusions, + semanticBudget, ); } catch (error: unknown) { debugLogger.error( diff --git a/packages/tools/src/tools/grep/types.ts b/packages/tools/src/tools/grep/types.ts index 82b864dadf..bd0be0a3fb 100644 --- a/packages/tools/src/tools/grep/types.ts +++ b/packages/tools/src/tools/grep/types.ts @@ -1,4 +1,4 @@ -import type { SemanticBudget } from './search-strategies.js'; +import type { SemanticBudget } from './grepBudget.js'; /** * Shared types and constants for the grep tool sub-modules. diff --git a/packages/tools/src/tools/ripGrep.ts b/packages/tools/src/tools/ripGrep.ts index 5ed2c455a5..b35b8ed489 100644 --- a/packages/tools/src/tools/ripGrep.ts +++ b/packages/tools/src/tools/ripGrep.ts @@ -32,7 +32,10 @@ import { makeRelative, shortenPath } from '../utils/paths.js'; import { stringOrDefault } from '../utils/stringCoalescing.js'; import { getErrorMessage } from '../utils/errors.js'; import { getRipgrepPath } from '../utils/ripgrepPathResolver.js'; -import { parseRipgrepLine } from './grep/ripgrepParse.js'; +import { + formatRipgrepDiagnostic, + parseRipgrepLine, +} from './grep/ripgrepParse.js'; export { parseRipgrepLine }; import { resolveTextSearchTarget, @@ -46,12 +49,12 @@ const DEFAULT_TOTAL_MAX_MATCHES = 20000; const MATCH_OVERHEAD_BYTES = 256; const HARD_RETAINED_MATCH_CAP = 100_000; -interface RipgrepSemanticBudget { +export interface RipgrepSemanticBudget { remainingBytes: number; remainingObjects: number; } -function createAggregateSemanticBudget(): RipgrepSemanticBudget { +export function createAggregateSemanticBudget(): RipgrepSemanticBudget { return { remainingBytes: DEFAULT_ACQUISITION_BUDGET_BYTES, remainingObjects: HARD_RETAINED_MATCH_CAP, @@ -153,7 +156,7 @@ interface GrepMatch { * Acquisition state for a ripgrep subprocess. Uses record-at-a-time line * consumption and bounded semantic retention. */ -interface RipgrepAcquisitionState { +export interface RipgrepAcquisitionState { collector: BoundedCombinedCollector; framer: BoundedLineFramer; matches: GrepMatch[]; @@ -165,7 +168,7 @@ interface RipgrepAcquisitionState { terminated: boolean; } -function createRipgrepAcquisitionState( +export function createRipgrepAcquisitionState( semanticBudget: RipgrepSemanticBudget, ): RipgrepAcquisitionState { return { @@ -223,7 +226,7 @@ function tryRetainRipgrepMatch( * bounded line record-at-a-time via callback. Returns true if early stop * or budget exhaustion was triggered. */ -function processRipgrepStdoutChunk( +export function processRipgrepStdoutChunk( state: RipgrepAcquisitionState, chunk: Buffer, basePath: string, @@ -260,7 +263,7 @@ function flushRipgrepLines( * Resolve a ripgrep close event into a result or error. An unexpected signal * kill (code null with a non-intentional signal) is treated as genuine failure. */ -function resolveRipgrepClose( +export function resolveRipgrepClose( code: number | null, signal: NodeJS.Signals | null, state: RipgrepAcquisitionState, @@ -273,6 +276,7 @@ function resolveRipgrepClose( earlyStopped: boolean; budgetTruncated: boolean; lineDropped: boolean; + rawTruncated: boolean; }; readonly error?: Error; } { @@ -280,30 +284,33 @@ function resolveRipgrepClose( flushRipgrepLines(state, basePath, maxMatches); } const acquisition = state.collector.getResult(); + const diagnostic = formatRipgrepDiagnostic(acquisition); + const diagnosticSuffix = diagnostic === '' ? '' : `: ${diagnostic}`; const result = { matches: state.matches, earlyStopped: state.earlyStopped, - budgetTruncated: acquisition.metadata.truncated || state.budgetExhausted, + budgetTruncated: state.budgetExhausted, lineDropped: state.framer.wasLineDropped, + rawTruncated: acquisition.metadata.truncated, }; if (state.earlyStopped || aborted || state.terminated) { return { result }; } if (signal !== null) { return { - error: new Error(`ripgrep was killed by signal ${signal}`), + error: new Error( + `ripgrep was killed by signal ${signal}${diagnosticSuffix}`, + ), }; } if (code !== null && code !== 0 && code !== 1) { return { - error: new Error( - `ripgrep exited with code ${code}: ${acquisition.stderrText.trim()}`, - ), + error: new Error(`ripgrep exited with code ${code}${diagnosticSuffix}`), }; } if (code === null) { return { - error: new Error('ripgrep closed unexpectedly'), + error: new Error(`ripgrep closed unexpectedly${diagnosticSuffix}`), }; } return { result }; @@ -322,6 +329,7 @@ interface RipgrepSubprocessResult { earlyStopped: boolean; budgetTruncated: boolean; lineDropped: boolean; + rawTruncated: boolean; } /** Create a ripgrep AbortError recognised by upstream callers. */ @@ -545,6 +553,11 @@ File: ${resolved.basename} ) { wasTruncated = true; } + if (searchResult.rawTruncated && this.host.getDebugMode()) { + debugLogger.debug( + `[GrepTool] Raw acquisition truncated for root ${searchDir} (diagnostic only, parsed results unaffected)`, + ); + } if (searchDirectories.length > 1) { const dirName = path.basename(searchDir); @@ -720,6 +733,7 @@ File: ${resolved.basename} earlyStopped: boolean; budgetTruncated: boolean; lineDropped: boolean; + rawTruncated: boolean; }> { const { pattern, diff --git a/packages/tools/src/tools/tool-registry.ts b/packages/tools/src/tools/tool-registry.ts index b6995a0fd8..59b36f268c 100644 --- a/packages/tools/src/tools/tool-registry.ts +++ b/packages/tools/src/tools/tool-registry.ts @@ -185,16 +185,16 @@ Signal: Signal number or \`(none)\` if no signal was received. }; signal.addEventListener('abort', abortHandler); - const { error, code, exitSignal } = await this.awaitProcessSettlement( - child, - collector, - params, - ); + const { error, code, exitSignal, drainTimedOut } = + await this.awaitProcessSettlement(child, collector, params); signal.removeEventListener('abort', abortHandler); let terminationOutcome: ProcessTerminationResult['outcome'] | null = null; - if (child.exitCode === null && child.signalCode === null) { + if ( + drainTimedOut || + (child.exitCode === null && child.signalCode === null) + ) { terminationPromise ??= terminateProcessTree(child, { ownsProcessGroup: true, }); @@ -221,10 +221,12 @@ Signal: Signal number or \`(none)\` if no signal was received. error: Error | null; code: number | null; exitSignal: NodeJS.Signals | null; + drainTimedOut: boolean; }> { let error: Error | null = null; let code: number | null = null; let exitSignal: NodeJS.Signals | null = null; + let drainTimedOut = false; return new Promise((resolve) => { let settled = false; @@ -240,7 +242,7 @@ Signal: Signal number or \`(none)\` if no signal was received. child.removeListener('error', onError); child.removeListener('exit', onExit); child.removeListener('close', onClose); - resolve({ error, code, exitSignal }); + resolve({ error, code, exitSignal, drainTimedOut }); }; const onStdout = (data: Buffer) => collector.append(data, 'stdout'); @@ -255,7 +257,10 @@ Signal: Signal number or \`(none)\` if no signal was received. if (settled) return; code = c; exitSignal = s; - drainTimer = setTimeout(() => settle(), STREAM_DRAIN_TIMEOUT_MS); + drainTimer = setTimeout(() => { + drainTimedOut = true; + settle(); + }, STREAM_DRAIN_TIMEOUT_MS); }; const onClose = (c: number | null, s: NodeJS.Signals | null) => { code ??= c; diff --git a/packages/tools/src/utils/processTermination.test.ts b/packages/tools/src/utils/processTermination.test.ts index 4ef8809c3d..71f0bc3cff 100644 --- a/packages/tools/src/utils/processTermination.test.ts +++ b/packages/tools/src/utils/processTermination.test.ts @@ -37,7 +37,7 @@ function waitForExit(child: ChildProcess): Promise { } describe('terminateProcessTree - graceful exit', () => { - it( + it.skipIf(process.platform === 'win32')( 'signals a running process and it exits gracefully', async () => { const child = spawnSleeper(30); @@ -57,13 +57,16 @@ describe('terminateProcessTree - graceful exit', () => { { timeout: 10000 }, ); - it('returns no_target for an already-exited process', async () => { - const child = spawnSleeper(0); - await waitForExit(child); + it.skipIf(process.platform === 'win32')( + 'returns no_target for an already-exited process', + async () => { + const child = spawnSleeper(0); + await waitForExit(child); - const result = await terminateProcessTree(child); - expect(result.outcome).toBe('no_target'); - }); + const result = await terminateProcessTree(child); + expect(result.outcome).toBe('no_target'); + }, + ); it('returns no_target for a process with no pid', async () => { const fakeChild = { @@ -253,7 +256,7 @@ describe('terminateProcessTree - direct child (no process group)', () => { }); describe('terminateProcessTree - coalescing by ChildProcess identity', () => { - it( + it.skipIf(process.platform === 'win32')( 'coalesces concurrent calls for the same child', async () => { const child = spawnSleeper(30); @@ -278,7 +281,7 @@ describe('terminateProcessTree - coalescing by ChildProcess identity', () => { { timeout: 10000 }, ); - it( + it.skipIf(process.platform === 'win32')( 'coalesces many concurrent calls for the same child', async () => { const child = spawnSleeper(30); @@ -302,20 +305,23 @@ describe('terminateProcessTree - coalescing by ChildProcess identity', () => { { timeout: 10000 }, ); - it('returns no_target for a sequential call after first completion', async () => { - const child = spawnSleeper(2); - await new Promise((r) => setTimeout(r, 100)); + it.skipIf(process.platform === 'win32')( + 'returns no_target for a sequential call after first completion', + async () => { + const child = spawnSleeper(2); + await new Promise((r) => setTimeout(r, 100)); - const result1 = await terminateProcessTree(child, { - gracePeriodMs: 3000, - ownsProcessGroup: true, - }); - await waitForExit(child); + const result1 = await terminateProcessTree(child, { + gracePeriodMs: 3000, + ownsProcessGroup: true, + }); + await waitForExit(child); - const result2 = await terminateProcessTree(child); - expect(result1.outcome).toBe('graceful'); - expect(result2.outcome).toBe('no_target'); - }); + const result2 = await terminateProcessTree(child); + expect(result1.outcome).toBe('graceful'); + expect(result2.outcome).toBe('no_target'); + }, + ); }); describe('terminateProcessTree - exported constants', () => { @@ -383,6 +389,15 @@ describe('terminateWindowsTree - platform-independent outcome tests', () => { expect(result.outcome).toBe('failure'); }); + it('reports no_target when taskkill exits 128 (target not found)', async () => { + const fakeSpawn: TaskkillSpawnFn = () => makeFakeChild({ closeCode: 128 }); + const result = await terminateWindowsTree(12345, fakeSpawn, { + watchdogMs: 1000, + postKillWaitMs: 500, + }); + expect(result.outcome).toBe('no_target'); + }); + it('reports failure when taskkill spawn throws', async () => { const fakeSpawn: TaskkillSpawnFn = () => { throw new Error('ENOENT'); diff --git a/packages/tools/src/utils/processTermination.ts b/packages/tools/src/utils/processTermination.ts index 5e63c450da..45123c2052 100644 --- a/packages/tools/src/utils/processTermination.ts +++ b/packages/tools/src/utils/processTermination.ts @@ -276,7 +276,13 @@ export function terminateWindowsTree( }; const onClose = (code: number | null) => { - resolveOnce(code === 0 ? 'graceful' : 'failure'); + if (code === 0) { + resolveOnce('graceful'); + } else if (code === 128) { + resolveOnce('no_target'); + } else { + resolveOnce('failure'); + } }; const onError = () => { From 319498f1f13f193f1f40eb8e1a613be9d13afee5 Mon Sep 17 00:00:00 2001 From: acoliver Date: Tue, 11 Aug 2026 22:25:28 -0300 Subject: [PATCH 5/5] Fix remaining bounded acquisition edge cases --- ...iscovered-tool-bounded-acquisition.test.ts | 139 +++++++++++++++ .../grep-ephemeral-precedence.test.ts | 163 ++++++++++++++++++ ...grep-ripgrep-issue3203-remediation.test.ts | 143 +++++++++++++++ .../grep-ripgrep-raw-truncation.test.ts | 122 +++++++++++++ packages/tools/src/index.ts | 1 - packages/tools/src/tools/grep.ts | 39 ++++- .../tools/src/tools/grep/search-strategies.ts | 46 ++--- packages/tools/src/tools/ripGrep.ts | 38 ++-- packages/tools/src/tools/tool-registry.ts | 106 +++++++----- .../src/utils/ripgrepPathResolver.test.ts | 67 ++++++- .../tools/src/utils/ripgrepPathResolver.ts | 24 ++- 11 files changed, 789 insertions(+), 99 deletions(-) create mode 100644 packages/tools/src/__tests__/grep-ephemeral-precedence.test.ts diff --git a/packages/tools/src/__tests__/discovered-tool-bounded-acquisition.test.ts b/packages/tools/src/__tests__/discovered-tool-bounded-acquisition.test.ts index 4c28836f68..626dbe47bb 100644 --- a/packages/tools/src/__tests__/discovered-tool-bounded-acquisition.test.ts +++ b/packages/tools/src/__tests__/discovered-tool-bounded-acquisition.test.ts @@ -15,11 +15,16 @@ import { } from 'node:fs'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; +import type { ChildProcess } from 'node:child_process'; import { DiscoveredTool } from '../tools/tool-registry.js'; import { BoundedCombinedCollector, createDefaultByteBudget, } from '../acquisition/index.js'; +import type { + ProcessTerminationResult, + ProcessTerminationOptions, +} from '../utils/processTermination.js'; import type { IToolRegistryHost, IToolMessageBus } from '../index.js'; function createTempDir(prefix = 'llxprt-dt-test-'): { @@ -522,6 +527,27 @@ function killDescendantFromMarker(markerPath: string): void { } } +function killProcessGroupFromPidFile(pidFile: string): void { + let pid: number; + try { + pid = Number.parseInt(readFileSync(pidFile, 'utf8').trim(), 10); + } catch { + return; + } + if (!Number.isSafeInteger(pid) || pid <= 0) return; + try { + process.kill(-pid, 'SIGKILL'); + return; + } catch (error: unknown) { + if (hasErrorCode(error, 'ESRCH')) return; + } + try { + process.kill(pid, 'SIGKILL'); + } catch { + /* best-effort */ + } +} + describe('DiscoveredTool drain timeout terminates descendant group', () => { let tempDir: string; let markerPath: string; @@ -567,3 +593,116 @@ describe('DiscoveredTool drain timeout terminates descendant group', () => { { timeout: 15000 }, ); }); + +describe('DiscoveredTool abort settlement with failed termination', () => { + let tempDir: string; + let pidFile: string; + let cleanup: () => void; + + beforeEach(() => { + const tmp = createTempDir(); + tempDir = tmp.dir; + pidFile = join(tempDir, 'child.pid'); + cleanup = tmp.cleanup; + }); + + afterEach(() => { + try { + killProcessGroupFromPidFile(pidFile); + } finally { + cleanup(); + } + }); + + it.skipIf(process.platform === 'win32')( + 'settles within a hard bound when termination fails and child never emits exit/close', + async () => { + const script = createScript( + tempDir, + 'hang-forever.sh', + `#!/bin/sh\necho $$ > "${pidFile}"\nsleep 60\necho done`, + ); + + // Narrow injectable seam: override terminateChild to simulate a + // failed termination (timeout) WITHOUT actually killing the process. + // The child stays alive and never emits exit/close. + class TimeoutTerminatorTool extends DiscoveredTool { + protected override terminateChild( + _child: ChildProcess, + _options?: ProcessTerminationOptions, + ): Promise { + return Promise.resolve({ outcome: 'timeout' }); + } + } + + const tool = new TimeoutTerminatorTool( + createHost(script), + 'discovered_tool_test', + 'Test discovered tool', + { type: 'object', properties: {} }, + noopMessageBus, + ); + const controller = new AbortController(); + + const executePromise = tool.execute({}, controller.signal); + setTimeout(() => controller.abort(), 200); + + const startTime = Date.now(); + const result = await executePromise; + const elapsed = Date.now() - startTime; + + // Must settle within a hard bound, not wait for sleep 60 or drain. + expect(elapsed).toBeLessThan(5000); + + // Must truthfully surface the termination failure. + expect(result.error).toBeDefined(); + const content = + typeof result.llmContent === 'string' ? result.llmContent : ''; + expect(content).toContain('Termination: timeout'); + }, + { timeout: 15000 }, + ); + + it.skipIf(process.platform === 'win32')( + 'settles within a hard bound when termination reports failure', + async () => { + const script = createScript( + tempDir, + 'hang-forever2.sh', + `#!/bin/sh\necho $$ > "${pidFile}"\nsleep 60\necho done`, + ); + + class FailureTerminatorTool extends DiscoveredTool { + protected override terminateChild( + _child: ChildProcess, + _options?: ProcessTerminationOptions, + ): Promise { + return Promise.resolve({ outcome: 'failure' }); + } + } + + const tool = new FailureTerminatorTool( + createHost(script), + 'discovered_tool_test', + 'Test discovered tool', + { type: 'object', properties: {} }, + noopMessageBus, + ); + const controller = new AbortController(); + + const executePromise = tool.execute({}, controller.signal); + setTimeout(() => controller.abort(), 200); + + const startTime = Date.now(); + const result = await executePromise; + const elapsed = Date.now() - startTime; + + expect(elapsed).toBeLessThan(5000); + expect(result.error).toBeDefined(); + const content = + typeof result.llmContent === 'string' ? result.llmContent : ''; + expect(content).toContain('Termination: failure'); + }, + { timeout: 15000 }, + ); +}); diff --git a/packages/tools/src/__tests__/grep-ephemeral-precedence.test.ts b/packages/tools/src/__tests__/grep-ephemeral-precedence.test.ts new file mode 100644 index 0000000000..2d6945f602 --- /dev/null +++ b/packages/tools/src/__tests__/grep-ephemeral-precedence.test.ts @@ -0,0 +1,163 @@ +/** + * @license + * Copyright 2026 Vybestack LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, beforeEach, afterEach } from 'bun:test'; +import { writeFileSync, mkdirSync, rmSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import type { IToolHost } from '../interfaces/index.js'; +import { GrepTool } from '../index.js'; +import type { ToolResult } from '../index.js'; +import type { GrepToolParams } from '../tools/grep/types.js'; + +function createTempDir(prefix = 'llxprt-eph-'): { + dir: string; + cleanup: () => void; +} { + const dir = join( + tmpdir(), + `${prefix}${Date.now()}-${Math.random().toString(36).slice(2)}`, + ); + mkdirSync(dir, { recursive: true }); + return { + dir, + cleanup: () => { + try { + rmSync(dir, { recursive: true, force: true }); + } catch { + /* ignore */ + } + }, + }; +} + +function createToolHost( + targetDir: string, + ephemeralMaxItems?: unknown, +): IToolHost { + const ephemeral: Record = { + 'tool-output-max-tokens': 50000, + 'tool-output-item-size-limit': 524288, + }; + if (ephemeralMaxItems !== undefined) { + ephemeral['tool-output-max-items'] = ephemeralMaxItems; + } + return { + getTargetDir: () => targetDir, + getWorkspaceRoots: () => [targetDir], + getApprovalMode: () => 'auto', + setApprovalMode: () => {}, + isInteractive: () => false, + hasFeatureFlag: () => false, + getFileService: () => ({ + shouldGitIgnoreFile: () => false, + shouldLlxprtIgnoreFile: () => false, + shouldIgnoreFile: () => false, + filterFiles: (paths) => paths, + }), + getFileFilteringOptions: () => ({ + respectGitIgnore: true, + respectLlxprtIgnore: true, + }), + getFileExclusions: () => [], + getReadManyFilesExclusions: () => [], + getFileFilteringRespectLlxprtIgnore: () => true, + getLlxprtIgnoreFilePath: () => null, + recordFileRead: () => {}, + getFileSystemService: () => undefined, + getLlxprtIgnorePatterns: () => [], + getEphemeralSettings: () => ephemeral, + getDebugMode: () => false, + }; +} + +async function executeGrep( + host: IToolHost, + params: GrepToolParams, +): Promise { + const tool = new GrepTool(host); + return tool.build(params).execute(new AbortController().signal); +} + +describe('GrepTool ephemeral max-results precedence', () => { + let tempDir: string; + let cleanup: () => void; + + beforeEach(() => { + const tmp = createTempDir(); + tempDir = tmp.dir; + cleanup = tmp.cleanup; + for (let i = 0; i < 10; i++) { + writeFileSync(join(tempDir, `f${i}.txt`), `ephmatch_${i}\n`); + } + }); + + afterEach(() => { + cleanup(); + }); + + it('ephemeral fallback: no explicit max_results uses tool-output-max-items', async () => { + const host = createToolHost(tempDir, 3); + const result = await executeGrep(host, { + pattern: 'ephmatch', + max_per_file: 1, + }); + const text = typeof result.llmContent === 'string' ? result.llmContent : ''; + // Should be limited to 3 results from ephemeral setting + const matchCount = (text.match(/ephmatch_/g) ?? []).length; + expect(matchCount).toBe(3); + expect(text).toMatch(/showing/i); + }); + + it('explicit override: max_results wins over ephemeral', async () => { + const host = createToolHost(tempDir, 3); + const result = await executeGrep(host, { + pattern: 'ephmatch', + max_results: 5, + max_per_file: 1, + }); + const text = typeof result.llmContent === 'string' ? result.llmContent : ''; + const matchCount = (text.match(/ephmatch_/g) ?? []).length; + // Explicit 5 should win over ephemeral 3 + expect(matchCount).toBe(5); + }); + + it('absent default: no explicit, no ephemeral => defaults to 1000', async () => { + const host = createToolHost(tempDir, undefined); + const result = await executeGrep(host, { + pattern: 'ephmatch', + }); + const text = typeof result.llmContent === 'string' ? result.llmContent : ''; + // Default 1000 > 10 files, so all 10 should be found + const matchCount = (text.match(/ephmatch_/g) ?? []).length; + expect(matchCount).toBe(10); + expect(text).not.toMatch(/showing|incomplete/i); + }); + + it('invalid ephemeral: falls back to default 1000', async () => { + const host = createToolHost(tempDir, -1); + const result = await executeGrep(host, { + pattern: 'ephmatch', + }); + const text = typeof result.llmContent === 'string' ? result.llmContent : ''; + // Invalid ephemeral (-1) should fall back to 1000 > 10 + const matchCount = (text.match(/ephmatch_/g) ?? []).length; + expect(matchCount).toBe(10); + expect(text).not.toMatch(/showing|incomplete/i); + }); + + it('hard-cap: ephemeral above cap does not crash and stays bounded', async () => { + const host = createToolHost(tempDir, 500_000); + const result = await executeGrep(host, { + pattern: 'ephmatch', + }); + expect(result.error).toBeUndefined(); + const text = typeof result.llmContent === 'string' ? result.llmContent : ''; + // 500k would be capped to 100k (MAX_RESULTS_HARD_CAP), still > 10 files + const matchCount = (text.match(/ephmatch_/g) ?? []).length; + expect(matchCount).toBe(10); + }); +}); diff --git a/packages/tools/src/__tests__/grep-ripgrep-issue3203-remediation.test.ts b/packages/tools/src/__tests__/grep-ripgrep-issue3203-remediation.test.ts index ebf5633fbe..7e08efc9a1 100644 --- a/packages/tools/src/__tests__/grep-ripgrep-issue3203-remediation.test.ts +++ b/packages/tools/src/__tests__/grep-ripgrep-issue3203-remediation.test.ts @@ -575,3 +575,146 @@ describe('Grep exact-cap evidence with per-file limits', () => { ]); }); }); + +describe('Exact-cap multi-root completeness: skipped roots mark incomplete (item 3)', () => { + let tempDir: string; + let cleanup: () => void; + + beforeEach(() => { + const tmp = createTempDir('llxprt-multicap-'); + tempDir = tmp.dir; + cleanup = tmp.cleanup; + }); + + afterEach(() => { + cleanup(); + }); + + it( + 'grep: first root fills exact cap, later root skipped => incomplete/non-exact', + async () => { + const rootA = join(tempDir, 'rootA'); + const rootB = join(tempDir, 'rootB'); + mkdirSync(rootA, { recursive: true }); + mkdirSync(rootB, { recursive: true }); + for (let i = 0; i < 5; i++) { + writeFileSync(join(rootA, `f${i}.txt`), `capmatch_${i}\n`); + } + writeFileSync(join(rootB, 'extra.txt'), 'capmatch_extra\n'); + + const host: IToolHost = { + ...createToolHost(tempDir), + getWorkspaceRoots: () => [rootA, rootB], + }; + + const result = await executeGrep(host, { + pattern: 'capmatch', + max_results: 5, + max_files: 100, + max_per_file: 1, + }); + + const text = + typeof result.llmContent === 'string' ? result.llmContent : ''; + // Root A filled the cap; root B was skipped => incomplete + expect(text).toMatch(/incomplete|showing.*may be/i); + expect(text).not.toMatch(/^Found 5 matches for pattern/m); + }, + { timeout: 15000 }, + ); + + it( + 'grep: all roots fully exhausted at cap remains exact', + async () => { + const rootA = join(tempDir, 'rootA'); + const rootB = join(tempDir, 'rootB'); + mkdirSync(rootA, { recursive: true }); + mkdirSync(rootB, { recursive: true }); + // Root A: 2 matches, Root B: 3 matches, total = 5 = maxResults + writeFileSync(join(rootA, 'f1.txt'), 'exactmatch_1\n'); + writeFileSync(join(rootA, 'f2.txt'), 'exactmatch_2\n'); + writeFileSync(join(rootB, 'f3.txt'), 'exactmatch_3\n'); + writeFileSync(join(rootB, 'f4.txt'), 'exactmatch_4\n'); + writeFileSync(join(rootB, 'f5.txt'), 'exactmatch_5\n'); + + const host: IToolHost = { + ...createToolHost(tempDir), + getWorkspaceRoots: () => [rootA, rootB], + }; + + const result = await executeGrep(host, { + pattern: 'exactmatch', + max_results: 5, + max_files: 100, + max_per_file: 1, + }); + + const text = + typeof result.llmContent === 'string' ? result.llmContent : ''; + // All 5 matches across both roots, no root skipped => exact + expect(text).not.toMatch(/incomplete|showing.*may be/i); + expect(text).toMatch(/^Found 5 matches for pattern/m); + }, + { timeout: 15000 }, + ); + + it( + 'ripgrep: first root fills exact cap, later root skipped => incomplete', + async () => { + const rootA = join(tempDir, 'rootA'); + const rootB = join(tempDir, 'rootB'); + mkdirSync(rootA, { recursive: true }); + mkdirSync(rootB, { recursive: true }); + + // Root A: exactly 20000 matches (fills the default cap) + const linesA: string[] = []; + for (let i = 0; i < 20000; i++) { + linesA.push(`rgcapmatch_${i}`); + } + writeFileSync(join(rootA, 'big.txt'), linesA.join('\n')); + // Root B: at least one match + writeFileSync(join(rootB, 'extra.txt'), 'rgcapmatch_extra\n'); + + const host: IToolHost = { + ...createToolHost(tempDir), + getWorkspaceRoots: () => [rootA, rootB], + }; + + const result = await executeRipgrep(host, { + pattern: 'rgcapmatch', + }); + + const text = + typeof result.llmContent === 'string' ? result.llmContent : ''; + expect(text).toMatch(/incomplete|showing/i); + }, + { timeout: 30000 }, + ); + + it( + 'ripgrep: all roots fully exhausted remains exact', + async () => { + const rootA = join(tempDir, 'rootA'); + const rootB = join(tempDir, 'rootB'); + mkdirSync(rootA, { recursive: true }); + mkdirSync(rootB, { recursive: true }); + writeFileSync(join(rootA, 'f1.txt'), 'rgexact_1\nrgexact_2\n'); + writeFileSync(join(rootB, 'f2.txt'), 'rgexact_3\n'); + + const host: IToolHost = { + ...createToolHost(tempDir), + getWorkspaceRoots: () => [rootA, rootB], + }; + + const result = await executeRipgrep(host, { + pattern: 'rgexact', + }); + + const text = + typeof result.llmContent === 'string' ? result.llmContent : ''; + expect(text).not.toMatch(/incomplete|showing/i); + expect(text).toMatch(/Found 3 matches/); + }, + { timeout: 15000 }, + ); +}); diff --git a/packages/tools/src/__tests__/grep-ripgrep-raw-truncation.test.ts b/packages/tools/src/__tests__/grep-ripgrep-raw-truncation.test.ts index 232e059bcc..9628e6c67c 100644 --- a/packages/tools/src/__tests__/grep-ripgrep-raw-truncation.test.ts +++ b/packages/tools/src/__tests__/grep-ripgrep-raw-truncation.test.ts @@ -18,6 +18,12 @@ import { processRipgrepStdoutChunk, createAggregateSemanticBudget, } from '../tools/ripGrep.js'; +import { + resolveGrepClose, + createGrepAcquisitionState, + processGrepStdoutChunk, +} from '../tools/grep/search-strategies.js'; +import { createAggregateSemanticBudget as createGrepBudget } from '../tools/grep/grepBudget.js'; function createTempDir(prefix = 'llxprt-raw-trunc-'): { dir: string; @@ -205,3 +211,119 @@ describe('Multi-root continuation despite verbose stderr (finding 3)', () => { { timeout: 15000 }, ); }); + +describe('resolveGrepClose: raw collector truncation vs semantic budget', () => { + it('stderr-only overflow does NOT mark budgetTruncated (parsed results complete)', () => { + const basePath = '/test'; + const budget = createGrepBudget(); + const state = createGrepAcquisitionState( + { maxResults: 1000, maxFiles: 100, maxPerFile: 50 }, + budget, + ); + + processGrepStdoutChunk( + state, + Buffer.from('file.txt:1:match content\n'), + basePath, + ); + + state.collector.append(Buffer.alloc(5 * 1024 * 1024, 0x45), 'stderr'); + + const outcome = resolveGrepClose( + 0, + null, + state, + false, + basePath, + 'grep', + undefined, + ); + + expect(outcome.error).toBeUndefined(); + expect(outcome.result).toBeDefined(); + expect(outcome.result!.budgetTruncated).toBe(false); + expect(outcome.result!.rawTruncated).toBe(true); + expect(outcome.result!.matches.length).toBe(1); + }); + + it('semantic budget exhaustion marks budgetTruncated (match data incomplete)', () => { + const basePath = '/test'; + const budget = createGrepBudget(); + budget.remainingBytes = 300; + + const state = createGrepAcquisitionState( + { maxResults: 1000, maxFiles: 100, maxPerFile: 50 }, + budget, + ); + + processGrepStdoutChunk( + state, + Buffer.from('file.txt:1:short\nfile.txt:2:another\nfile.txt:3:third\n'), + basePath, + ); + + const outcome = resolveGrepClose( + 0, + null, + state, + false, + basePath, + 'grep', + undefined, + ); + + expect(outcome.result).toBeDefined(); + expect(outcome.result!.budgetTruncated).toBe(true); + }); + + it('stderr-only overflow with zero stdout matches is complete (no false incomplete)', () => { + const basePath = '/test'; + const budget = createGrepBudget(); + const state = createGrepAcquisitionState( + { maxResults: 1000, maxFiles: 100, maxPerFile: 50 }, + budget, + ); + + state.collector.append(Buffer.alloc(5 * 1024 * 1024, 0x57), 'stderr'); + + const outcome = resolveGrepClose( + 0, + null, + state, + false, + basePath, + 'grep', + undefined, + ); + + expect(outcome.error).toBeUndefined(); + expect(outcome.result).toBeDefined(); + expect(outcome.result!.budgetTruncated).toBe(false); + expect(outcome.result!.rawTruncated).toBe(true); + expect(outcome.result!.matches.length).toBe(0); + }); + + it('labels omitted diagnostic bytes when a failing grep writes excessive stderr', () => { + const basePath = '/test'; + const state = createGrepAcquisitionState( + { maxResults: 1000, maxFiles: 100, maxPerFile: 50 }, + createGrepBudget(), + ); + + state.collector.append(Buffer.alloc(5 * 1024 * 1024, 0x45), 'stderr'); + + const outcome = resolveGrepClose( + 2, + null, + state, + false, + basePath, + 'grep', + undefined, + ); + + expect(outcome.result).toBeUndefined(); + expect(outcome.error?.message).toContain('grep exited with code 2'); + expect(outcome.error?.message).toContain('[LLXPRT output truncated:'); + }); +}); diff --git a/packages/tools/src/index.ts b/packages/tools/src/index.ts index b9d37e4029..db73beec2e 100644 --- a/packages/tools/src/index.ts +++ b/packages/tools/src/index.ts @@ -202,7 +202,6 @@ export { isRipgrepAvailable, clearRipgrepAvailabilityCache, ensureWindowsShortcut, - findInPath, } from './utils/ripgrepPathResolver.js'; export { TodoStatus, diff --git a/packages/tools/src/tools/grep.ts b/packages/tools/src/tools/grep.ts index 1f309d1179..aa36925b45 100644 --- a/packages/tools/src/tools/grep.ts +++ b/packages/tools/src/tools/grep.ts @@ -42,6 +42,7 @@ import { createAggregateSemanticBudget } from './grep/grepBudget.js'; export { type GrepToolParams } from './grep/types.js'; +const MAX_RESULTS_DEFAULT = 1000; const MAX_RESULTS_HARD_CAP = 100_000; const MAX_FILES_HARD_CAP = 10_000; const MAX_PER_FILE_HARD_CAP = 10_000; @@ -66,7 +67,24 @@ function validateFinitePositive( return Math.min(n, hardCap); } -function validateGrepLimits(params: GrepToolParams): { +function resolveEphemeralMaxResults(value: unknown): number { + if (value === undefined || value === null) return 0; + const n = typeof value === 'string' ? Number(value) : value; + if ( + typeof n !== 'number' || + !Number.isFinite(n) || + n <= 0 || + !Number.isInteger(n) + ) { + return 0; + } + return Math.min(n, MAX_RESULTS_HARD_CAP); +} + +function validateGrepLimits( + params: GrepToolParams, + ephemeralMaxResults?: unknown, +): { maxResults: number; maxFiles: number; maxPerFile: number; @@ -92,8 +110,20 @@ function validateGrepLimits(params: GrepToolParams): { 'timeout_ms', MAX_TIMEOUT_MS, ); + const ephemeralMax = + ephemeralMaxResults !== undefined + ? resolveEphemeralMaxResults(ephemeralMaxResults) + : 0; + let fallbackMax: number; + if (maxResultsRaw !== 0) { + fallbackMax = maxResultsRaw; + } else if (ephemeralMax !== 0) { + fallbackMax = ephemeralMax; + } else { + fallbackMax = MAX_RESULTS_DEFAULT; + } return { - maxResults: maxResultsRaw !== 0 ? maxResultsRaw : 1000, + maxResults: fallbackMax, maxFiles: maxFilesRaw !== 0 ? maxFilesRaw : 100, maxPerFile: maxPerFileRaw !== 0 ? maxPerFileRaw : 50, timeoutMs: Math.min( @@ -217,6 +247,7 @@ File: ${resolved.basename} for (const searchDir of searchDirectories) { if (allMatches.length >= maxResults) { wasLimited = true; + totalIsExact = false; break; } @@ -615,8 +646,10 @@ File: ${resolved.basename} } async execute(signal: AbortSignal): Promise { + const ephemeralSettings = this.host.getEphemeralSettings(); const { maxResults, maxFiles, maxPerFile, timeoutMs } = validateGrepLimits( this.params, + ephemeralSettings['tool-output-max-items'], ); const timeoutController = new AbortController(); const timeoutId = setTimeout(() => timeoutController.abort(), timeoutMs); @@ -751,7 +784,7 @@ export class GrepTool extends BaseDeclarativeTool { }, max_results: { description: - 'Optional: Maximum number of total matches to return. Defaults to 1000. Must be a positive integer.', + 'Optional: Maximum number of total matches to return. Defaults to the tool-output-max-items setting or 1000. Must be a positive integer.', type: 'number', minimum: 1, maximum: MAX_RESULTS_HARD_CAP, diff --git a/packages/tools/src/tools/grep/search-strategies.ts b/packages/tools/src/tools/grep/search-strategies.ts index e7f277712a..e135c5d2ae 100644 --- a/packages/tools/src/tools/grep/search-strategies.ts +++ b/packages/tools/src/tools/grep/search-strategies.ts @@ -95,10 +95,6 @@ export function isCommandAvailable( }); } -/** - * Parses the standard output of grep-like commands (git grep, system grep). - * Expects format: filePath:lineNumber:lineContent - */ /** * Parses a single grep output line into a GrepMatch, or null if malformed. */ @@ -128,21 +124,6 @@ function parseGrepLine(line: string, basePath: string): GrepMatch | null { }; } -export function parseGrepOutput(output: string, basePath: string): GrepMatch[] { - const results: GrepMatch[] = []; - if (!output) return results; - - const lines = output.split(new RegExp('\\r?\\n')); - - for (const line of lines) { - const match = parseGrepLine(line, basePath); - if (match) { - results.push(match); - } - } - return results; -} - /** * Flatten the grouped matches into a flat list, respecting maxResults. * Uses a guard clause instead of nested break statements. @@ -234,6 +215,7 @@ interface BoundedGrepResult { earlyStopped: boolean; budgetTruncated: boolean; lineDropped: boolean; + rawTruncated: boolean; } interface BoundedGrepSubprocessOptions { @@ -289,12 +271,14 @@ function createGrepAcquisitionState( }; } +export { createGrepAcquisitionState }; + /** * Feed a stdout chunk into the collector and framer, consuming each complete * bounded line record-at-a-time via callback. Returns true if early stop * or budget exhaustion was triggered. */ -function processGrepStdoutChunk( +export function processGrepStdoutChunk( state: GrepAcquisitionState, chunk: Buffer, cwd: string, @@ -356,13 +340,13 @@ function checkGrepExitCode( } /** Resolution of a grep subprocess close event. */ -interface GrepCloseResolution { +export interface GrepCloseResolution { readonly result?: BoundedGrepResult; readonly error?: Error; } /** Resolve a grep subprocess close into a result or error. */ -function resolveGrepClose( +export function resolveGrepClose( code: number | null, signal: NodeJS.Signals | null, state: GrepAcquisitionState, @@ -374,7 +358,8 @@ function resolveGrepClose( if (!state.terminated) { flushGrepLines(state, cwd); } - const rawStderr = state.collector.getStderrText().trim(); + const acquisition = state.collector.getResult(); + const rawStderr = acquisition.stderrText.trim(); const stderrText = options?.filterStderr ? options.filterStderr(rawStderr).trim() : rawStderr; @@ -389,16 +374,23 @@ function resolveGrepClose( options, ); if (exitError !== null) { + const omissionNotice = acquisition.omissionNotice ?? ''; + if (omissionNotice !== '') { + return { + error: new Error(`${exitError.message} +${omissionNotice}`), + }; + } return { error: exitError }; } - const acquisition = state.collector.getResult(); return { result: { matches: state.matches, observedCount: state.observedCount, earlyStopped: state.earlyStopped, - budgetTruncated: acquisition.metadata.truncated || state.budgetExhausted, + budgetTruncated: state.budgetExhausted, lineDropped: state.framer.wasLineDropped, + rawTruncated: acquisition.metadata.truncated, }, }; } @@ -554,10 +546,6 @@ export async function tryGitGrep( return null; } } - -/** - * Builds the grep args for system grep, including exclusion patterns. - */ export function buildSystemGrepArgs( pattern: string, include: string | undefined, diff --git a/packages/tools/src/tools/ripGrep.ts b/packages/tools/src/tools/ripGrep.ts index b35b8ed489..9cf09c2e42 100644 --- a/packages/tools/src/tools/ripGrep.ts +++ b/packages/tools/src/tools/ripGrep.ts @@ -19,7 +19,6 @@ import { SchemaValidator } from '../utils/schemaValidator.js'; import { BoundedCombinedCollector, createDefaultByteBudget, - DEFAULT_ACQUISITION_BUDGET_BYTES, } from '../acquisition/index.js'; import { BoundedLineFramer } from '../utils/lineFramer.js'; import { terminateProcessTree } from '../utils/processTermination.js'; @@ -42,24 +41,18 @@ import { type ResolvedSearchTarget, } from '../utils/resolveTextSearchTarget.js'; import { debugLogger } from '../utils/debugLogger.js'; +import { + type SemanticBudget, + createAggregateSemanticBudget, + MATCH_OVERHEAD_BYTES, +} from './grep/grepBudget.js'; + +export type { SemanticBudget as RipgrepSemanticBudget } from './grep/grepBudget.js'; +export { createAggregateSemanticBudget } from './grep/grepBudget.js'; export const ripGrepDebugLogger = debugLogger; const DEFAULT_TOTAL_MAX_MATCHES = 20000; -const MATCH_OVERHEAD_BYTES = 256; -const HARD_RETAINED_MATCH_CAP = 100_000; - -export interface RipgrepSemanticBudget { - remainingBytes: number; - remainingObjects: number; -} - -export function createAggregateSemanticBudget(): RipgrepSemanticBudget { - return { - remainingBytes: DEFAULT_ACQUISITION_BUDGET_BYTES, - remainingObjects: HARD_RETAINED_MATCH_CAP, - }; -} /** * Parameters for the GrepTool @@ -161,7 +154,7 @@ export interface RipgrepAcquisitionState { framer: BoundedLineFramer; matches: GrepMatch[]; retainedBytes: number; - semanticBudget: RipgrepSemanticBudget; + semanticBudget: SemanticBudget; earlyStopped: boolean; capReached: boolean; budgetExhausted: boolean; @@ -169,7 +162,7 @@ export interface RipgrepAcquisitionState { } export function createRipgrepAcquisitionState( - semanticBudget: RipgrepSemanticBudget, + semanticBudget: SemanticBudget, ): RipgrepAcquisitionState { return { collector: new BoundedCombinedCollector({ @@ -527,6 +520,7 @@ File: ${resolved.basename} } let stop = false; + let lastSearchedIndex = -1; for (let di = 0; di < searchDirectories.length && !stop; di++) { const searchDir = searchDirectories[di]; const remaining = totalMaxMatches - allMatches.length; @@ -535,6 +529,7 @@ File: ${resolved.basename} stop = true; continue; } + lastSearchedIndex = di; const searchResult = await this.performRipgrepSearch({ pattern: this.params.pattern, @@ -575,6 +570,11 @@ File: ${resolved.basename} allMatches.length >= totalMaxMatches || searchResult.budgetTruncated; } + // Skipped roots imply incomplete even without an observed extra record. + if (lastSearchedIndex < searchDirectories.length - 1) { + wasTruncated = true; + } + return { matches: allMatches, wasTruncated }; } @@ -704,7 +704,7 @@ File: ${resolved.basename} rgArgs: string[], signal: AbortSignal, maxMatches: number, - semanticBudget: RipgrepSemanticBudget, + semanticBudget: SemanticBudget, ): Promise { const resolvedRgPath = await getRipgrepPath(); if (signal.aborted) throw ripgrepAbortError(); @@ -727,7 +727,7 @@ File: ${resolved.basename} signal: AbortSignal; ignoreOptions: RipgrepIgnoreOptions; maxMatches: number; - semanticBudget: RipgrepSemanticBudget; + semanticBudget: SemanticBudget; }): Promise<{ matches: GrepMatch[]; earlyStopped: boolean; diff --git a/packages/tools/src/tools/tool-registry.ts b/packages/tools/src/tools/tool-registry.ts index 59b36f268c..e8c189251f 100644 --- a/packages/tools/src/tools/tool-registry.ts +++ b/packages/tools/src/tools/tool-registry.ts @@ -16,7 +16,7 @@ import { import { type ToolContext, isContextAwareTool } from '../types/tool-context.js'; import type { IToolRegistryHost } from '../interfaces/IToolRegistryHost.js'; import type { IToolMessageBus } from '../interfaces/IToolMessageBus.js'; -import { spawn } from 'node:child_process'; +import { spawn, type ChildProcess } from 'node:child_process'; import { StringDecoder } from 'node:string_decoder'; import { parse } from 'shell-quote'; import { ToolErrorType } from '../types/tool-error.js'; @@ -36,6 +36,7 @@ import { import { terminateProcessTree, type ProcessTerminationResult, + type ProcessTerminationOptions, } from '../utils/processTermination.js'; const STREAM_DRAIN_TIMEOUT_MS = 2000; @@ -145,7 +146,7 @@ Signal: Signal number or \`(none)\` if no signal was received. }; } const callCommand = this.config.getToolCallCommand?.() ?? ''; - const child = spawn(callCommand, [this.name], { + const child: ChildProcess = spawn(callCommand, [this.name], { windowsHide: true, detached: process.platform !== 'win32', }); @@ -162,8 +163,19 @@ Signal: Signal number or \`(none)\` if no signal was received. ); } + /** + * Terminate a child process tree. Overridable for deterministic tests + * that need to simulate termination outcomes without real signals. + */ + protected terminateChild( + child: ChildProcess, + options?: ProcessTerminationOptions, + ): Promise { + return terminateProcessTree(child, options); + } + private async runChildProcess( - child: ReturnType, + child: ChildProcess, signal: AbortSignal, params: ToolParams, ): Promise<{ @@ -177,31 +189,18 @@ Signal: Signal number or \`(none)\` if no signal was received. budget: createDefaultByteBudget(), }); - let terminationPromise: Promise | null = null; - const abortHandler = () => { - terminationPromise ??= terminateProcessTree(child, { - ownsProcessGroup: true, - }); - }; - signal.addEventListener('abort', abortHandler); + const { error, code, exitSignal, drainTimedOut, terminationOutcome } = + await this.awaitProcessSettlement(child, collector, params, signal); - const { error, code, exitSignal, drainTimedOut } = - await this.awaitProcessSettlement(child, collector, params); - - signal.removeEventListener('abort', abortHandler); - - let terminationOutcome: ProcessTerminationResult['outcome'] | null = null; + let outcome = terminationOutcome; if ( - drainTimedOut || - (child.exitCode === null && child.signalCode === null) + outcome === null && + (drainTimedOut || (child.exitCode === null && child.signalCode === null)) ) { - terminationPromise ??= terminateProcessTree(child, { + const result = await this.terminateChild(child, { ownsProcessGroup: true, }); - } - if (terminationPromise !== null) { - const result = await terminationPromise; - terminationOutcome = result.outcome; + outcome = result.outcome; } return { @@ -209,28 +208,46 @@ Signal: Signal number or \`(none)\` if no signal was received. error, code, exitSignal, - terminationOutcome, + terminationOutcome: outcome, }; } + private writeParamsToStdin( + child: ChildProcess, + params: ToolParams, + onError: (err: Error) => void, + ): void { + if (child.stdin === null) return; + try { + child.stdin.write(JSON.stringify(params)); + child.stdin.end(); + } catch (e) { + onError(e instanceof Error ? e : new Error(String(e))); + } + } + private awaitProcessSettlement( - child: ReturnType, + child: ChildProcess, collector: BoundedCombinedCollector, params: ToolParams, + signal: AbortSignal, ): Promise<{ error: Error | null; code: number | null; exitSignal: NodeJS.Signals | null; drainTimedOut: boolean; + terminationOutcome: ProcessTerminationResult['outcome'] | null; }> { let error: Error | null = null; let code: number | null = null; let exitSignal: NodeJS.Signals | null = null; let drainTimedOut = false; + let terminationOutcome: ProcessTerminationResult['outcome'] | null = null; return new Promise((resolve) => { let settled = false; let drainTimer: ReturnType | null = null; + let terminationPromise: Promise | null = null; const settle = () => { if (settled) return; @@ -238,11 +255,12 @@ Signal: Signal number or \`(none)\` if no signal was received. if (drainTimer !== null) clearTimeout(drainTimer); child.stdout?.removeListener('data', onStdout); child.stderr?.removeListener('data', onStderr); - child.stdin?.removeListener('error', onStdinError); - child.removeListener('error', onError); + child.stdin?.removeListener('error', captureFirstError); + child.removeListener('error', captureFirstError); child.removeListener('exit', onExit); child.removeListener('close', onClose); - resolve({ error, code, exitSignal, drainTimedOut }); + signal.removeEventListener('abort', onAbort); + resolve({ error, code, exitSignal, drainTimedOut, terminationOutcome }); }; const onStdout = (data: Buffer) => collector.append(data, 'stdout'); @@ -251,8 +269,6 @@ Signal: Signal number or \`(none)\` if no signal was received. error ??= err; settle(); }; - const onStdinError = captureFirstError; - const onError = captureFirstError; const onExit = (c: number | null, s: NodeJS.Signals | null) => { if (settled) return; code = c; @@ -267,22 +283,30 @@ Signal: Signal number or \`(none)\` if no signal was received. exitSignal ??= s; settle(); }; + const onAbort = () => { + terminationPromise ??= this.terminateChild(child, { + ownsProcessGroup: true, + }); + void terminationPromise.then((result) => { + terminationOutcome = result.outcome; + if ( + result.outcome === 'timeout' || + result.outcome === 'failure' || + result.outcome === 'no_target' + ) { + settle(); + } + }); + }; child.stdout?.on('data', onStdout); child.stderr?.on('data', onStderr); - child.stdin?.on('error', onStdinError); - child.on('error', onError); + child.stdin?.on('error', captureFirstError); + child.on('error', captureFirstError); child.on('exit', onExit); child.on('close', onClose); - - if (child.stdin !== null) { - try { - child.stdin.write(JSON.stringify(params)); - child.stdin.end(); - } catch (e) { - captureFirstError(e instanceof Error ? e : new Error(String(e))); - } - } + signal.addEventListener('abort', onAbort); + this.writeParamsToStdin(child, params, captureFirstError); }); } diff --git a/packages/tools/src/utils/ripgrepPathResolver.test.ts b/packages/tools/src/utils/ripgrepPathResolver.test.ts index 861b1a137e..0f42949744 100644 --- a/packages/tools/src/utils/ripgrepPathResolver.test.ts +++ b/packages/tools/src/utils/ripgrepPathResolver.test.ts @@ -127,7 +127,24 @@ describe('findInPath Windows extension resolution', () => { } }); - it('finds bare rg with normal PATHEXT when isWindows is true (bare always checked)', () => { + it('rg.EXE shadows bare rg when both exist with PATHEXT (isWindows=true)', () => { + const { dirs, cleanup } = makeTempDirs(1); + try { + const exeCandidate = join(dirs[0], 'rg.EXE'); + const bareCandidate = join(dirs[0], 'rg'); + writeFileSync(exeCandidate, 'real'); + writeFileSync(bareCandidate, 'wrong'); + process.env.PATH = dirs[0]; + process.env.PATHEXT = ['.COM', '.EXE', '.BAT', '.CMD'].join( + pathDelimiter, + ); + expect(findInPath('rg', true)).toBe(exeCandidate); + } finally { + cleanup(); + } + }); + + it('bare rg found as last-resort fallback when PATHEXT is set but no extension matches', () => { const { dirs, cleanup } = makeTempDirs(1); try { const candidate = join(dirs[0], 'rg'); @@ -155,14 +172,58 @@ describe('findInPath Windows extension resolution', () => { } }); - it('finds bare rg when PATHEXT is empty (fallback .EXE does not exist)', () => { + it('returns null when PATHEXT is empty and only bare rg exists (no .EXE fallback match)', () => { const { dirs, cleanup } = makeTempDirs(1); try { const candidate = join(dirs[0], 'rg'); writeFileSync(candidate, 'fake'); process.env.PATH = dirs[0]; process.env.PATHEXT = ''; - expect(findInPath('rg', true)).toBe(candidate); + expect(findInPath('rg', true)).toBeNull(); + } finally { + cleanup(); + } + }); + + it('PATHEXT order: .COM checked before .EXE', () => { + const { dirs, cleanup } = makeTempDirs(1); + try { + const comCandidate = join(dirs[0], 'rg.COM'); + const exeCandidate = join(dirs[0], 'rg.EXE'); + writeFileSync(comCandidate, 'first'); + writeFileSync(exeCandidate, 'second'); + process.env.PATH = dirs[0]; + process.env.PATHEXT = ['.COM', '.EXE', '.BAT', '.CMD'].join( + pathDelimiter, + ); + expect(findInPath('rg', true)).toBe(comCandidate); + } finally { + cleanup(); + } + }); + + it('missing PATHEXT on Windows: only .EXE checked, bare rg not found', () => { + const { dirs, cleanup } = makeTempDirs(1); + try { + const bareCandidate = join(dirs[0], 'rg'); + writeFileSync(bareCandidate, 'fake'); + process.env.PATH = dirs[0]; + delete process.env.PATHEXT; + expect(findInPath('rg', true)).toBeNull(); + } finally { + cleanup(); + } + }); + + it('case/duplicate normalization: deduplicates and uses first-seen extension', () => { + const { dirs, cleanup } = makeTempDirs(1); + try { + const candidate = join(dirs[0], 'rg.exe'); + writeFileSync(candidate, 'fake'); + process.env.PATH = dirs[0]; + process.env.PATHEXT = ['.exe', '.EXE', '.Exe'].join(pathDelimiter); + const result = findInPath('rg', true); + expect(result).toBe(candidate); } finally { cleanup(); } diff --git a/packages/tools/src/utils/ripgrepPathResolver.ts b/packages/tools/src/utils/ripgrepPathResolver.ts index b18888d4b1..a63ae12805 100644 --- a/packages/tools/src/utils/ripgrepPathResolver.ts +++ b/packages/tools/src/utils/ripgrepPathResolver.ts @@ -97,9 +97,27 @@ function isExecutable(filePath: string, isWindows: boolean): boolean { export function findInPath(binName: string, isWindows: boolean): string | null { const pathEnv = process.env.PATH ?? ''; const pathExt = process.env.PATHEXT ?? ''; - const rawExts = pathExt - ? ['', ...pathExt.split(path.delimiter).filter((e) => e.length > 0)] - : ['', '.EXE']; + + let rawExts: string[]; + if (isWindows) { + if (pathExt) { + // Windows with PATHEXT: check PATHEXT candidates before bare fallback + // so a bare regular file does not shadow rg.EXE. + rawExts = [ + ...pathExt.split(path.delimiter).filter((e) => e.length > 0), + '', + ]; + } else { + // Windows without PATHEXT: .EXE fallback only. + rawExts = ['.EXE']; + } + } else { + // POSIX: bare first (unchanged execute-access semantics). + rawExts = pathExt + ? ['', ...pathExt.split(path.delimiter).filter((e) => e.length > 0)] + : ['', '.EXE']; + } + const seen = new Set(); const exts: string[] = []; for (const ext of rawExts) {