From 727458c8022e84d8e88dc8d8beb356228c536ba3 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Wed, 14 Oct 2026 21:59:32 -0700 Subject: [PATCH 1/4] refactor(tools): share arg guards, bounded reads, and process spawn --- src/io/bounded-read.ts | 340 +++++++++++++++ src/plugins/bounded-grep-fallback.ts | 70 +--- src/plugins/delete-file-plugin.ts | 59 ++- src/plugins/edit-file-line-range-plugin.ts | 9 +- src/plugins/evidence-archive-search-plugin.ts | 110 ++--- src/plugins/path-escape-plugin.ts | 134 ++---- src/plugins/read-file-guard-plugin.ts | 394 ++---------------- src/plugins/ripgrep-plugin.test.ts | 2 +- src/plugins/ripgrep-plugin.ts | 64 ++- src/plugins/search-files-fail-closed.test.ts | 2 +- .../secret-guard-config-denylist.test.ts | 2 +- src/plugins/shell-guard-plugin.ts | 77 +--- src/plugins/test-helpers.ts | 2 +- src/plugins/tool-args.ts | 40 ++ src/plugins/tool-output-uri-plugin.ts | 21 +- src/plugins/tool-time-budget.ts | 29 +- src/plugins/verify-plugin.ts | 5 +- src/{plugins => search}/rg-output.test.ts | 0 src/{plugins => search}/rg-output.ts | 4 +- src/{plugins => search}/rg-run.test.ts | 2 +- src/{plugins => search}/rg-run.ts | 21 +- src/shell/background-shell.ts | 24 +- src/shell/spawn-tree.ts | 49 +++ src/util/glob-regex.ts | 50 +++ 24 files changed, 733 insertions(+), 777 deletions(-) create mode 100644 src/io/bounded-read.ts create mode 100644 src/plugins/tool-args.ts rename src/{plugins => search}/rg-output.test.ts (100%) rename src/{plugins => search}/rg-output.ts (96%) rename src/{plugins => search}/rg-run.test.ts (98%) rename src/{plugins => search}/rg-run.ts (88%) create mode 100644 src/shell/spawn-tree.ts create mode 100644 src/util/glob-regex.ts diff --git a/src/io/bounded-read.ts b/src/io/bounded-read.ts new file mode 100644 index 000000000..88b3a0131 --- /dev/null +++ b/src/io/bounded-read.ts @@ -0,0 +1,340 @@ +import { Readable } from "node:stream"; +import { StringDecoder } from "node:string_decoder"; + +import { formatReadFileTimeoutMessage } from "../plugins/tool-time-budget.js"; + +// The bounded read engine behind read_file's guard: streams a source in one +// pass, emitting up to `limit` line-numbered lines while never buffering more +// than the byte budget plus one chunk. Filesystem and blob adapters (the +// read-file-guard plugin, archive search) feed it their own stream and error +// mapping; policy like the paging default stays with the caller. + +export const READ_FILE_MAX_BYTES = 50 * 1024; +export const READ_FILE_MAX_LINE_LENGTH = 2000; +// Absolute ceiling on bytes scanned from disk past the requested offset, so +// an emission window stays time-bounded. Bytes skipped to reach a nonzero +// offset do not count: continuation past the ceiling must read through to +// the end, not dead-end at the limit. +export const READ_FILE_MAX_SCAN_BYTES = 8 * 1024 * 1024; +// Headroom reserved out of the byte budget for the continuation notice, so +// the payload plus notice stays under READ_FILE_MAX_BYTES. +const NOTICE_RESERVE_BYTES = 256; + +const LINE_TRUNC_SUFFIX = ` ... [line truncated at ${READ_FILE_MAX_LINE_LENGTH} chars; full line remains in the file — use grep to match within the line]`; +const TOOL_OUTPUT_CHUNK_BYTES = 64 * 1024; + +export type TruncReason = "lines" | "bytes" | "scan"; + +export interface BoundedRead { + content: string; + isError?: boolean; +} + +// A truncated read tells the model to continue with the same path and the +// explicit next offset from the notice. There is no continuation handle: +// every read is a stateless, idempotent ranged read, so following a notice +// verbatim works on first use, replay, and a fresh plugin instance after +// compaction or session resume. Chunked reads carry rising offsets, so +// call-keying detectors see one ranged read per window, not a same-path +// loop. + + +function numberLine(lineNo: number, text: string): string { + return `${String(lineNo).padStart(6, " ")}\t${text}`; +} + + +/** + * Streams UTF-8 from `stream`, emitting up to `limit` line-numbered lines + * after skipping `offset` lines (zero-based); never decodes the full text in + * one pass. With `wrapLongLines`, overlong lines split into numbered windows + * so a giant JSON line pages like a multi-line file. With `windowHugeLines` + * instead, only lines that alone exceed the output budget are windowed, so + * plain path+offset pagination stays line-aligned. + */ +export function readStreamBounded( + stream: Readable, + displayPath: string, + offset: number, + limit: number, + signal: AbortSignal, + options: { + mapStreamError?: (err: NodeJS.ErrnoException) => Error; + wrapLongLines?: boolean; + windowHugeLines?: boolean; + diagnoseFirstChunk?: (chunk: Buffer) => string | undefined; + maxScanBytes?: number; + } = {}, +): Promise { + return new Promise((resolveP, rejectP) => { + const { + mapStreamError, + wrapLongLines = false, + windowHugeLines = false, + diagnoseFirstChunk, + maxScanBytes, + } = options; + const scanCeiling = maxScanBytes ?? READ_FILE_MAX_SCAN_BYTES; + const decoder = new StringDecoder("utf8"); + const contentBudget = READ_FILE_MAX_BYTES - NOTICE_RESERVE_BYTES; + + let pending = ""; + let pendingOverflow = false; + let firstChunk = true; + let lineNo = 0; + let scanned = 0; + let skipDone = offset <= 0; + let outBytes = 0; + let emitted = 0; + let lastEmittedLine = 0; + let truncReason: TruncReason | undefined; + let endReached = false; + let settled = false; + + const out: string[] = []; + + const onAbort = () => { + if (settled) return; + settled = true; + stream.destroy(); + const partial = out.length > 0 ? out.join("\n") : undefined; + rejectP(new Error(formatReadFileTimeoutMessage(displayPath, partial))); + }; + signal.addEventListener("abort", onAbort, { once: true }); + if (signal.aborted) { + onAbort(); + return; + } + + const done = (result: BoundedRead) => { + if (settled) return; + settled = true; + stream.destroy(); + signal.removeEventListener("abort", onAbort); + resolveP(result); + }; + + const handleLine = (raw: string, overflow: boolean): boolean => { + lineNo++; + if (lineNo <= offset) return true; + skipDone = true; + if (emitted >= limit) { + truncReason = "lines"; + return false; + } + const tooLong = overflow || raw.length > READ_FILE_MAX_LINE_LENGTH; + const text = tooLong + ? raw.slice(0, READ_FILE_MAX_LINE_LENGTH) + LINE_TRUNC_SUFFIX + : raw; + const numbered = numberLine(lineNo, text); + const bytes = Buffer.byteLength(numbered, "utf8") + 1; + if (emitted > 0 && outBytes + bytes > contentBudget) { + truncReason = "bytes"; + return false; + } + out.push(numbered); + outBytes += bytes; + emitted++; + lastEmittedLine = lineNo; + return true; + }; + + const emitWrapped = (raw: string, keepTail: boolean): boolean => { + let rest = raw; + while (rest.length > READ_FILE_MAX_LINE_LENGTH) { + if (!handleLine(rest.slice(0, READ_FILE_MAX_LINE_LENGTH), false)) + return false; + rest = rest.slice(READ_FILE_MAX_LINE_LENGTH); + } + if (keepTail) return handleLine(rest, false); + pending = rest; + return true; + }; + + const drainPending = (): boolean => { + for (;;) { + const nl = pending.indexOf("\n"); + if (nl === -1) { + if (wrapLongLines) return emitWrapped(pending, false); + if (pending.length > READ_FILE_MAX_LINE_LENGTH && !windowHugeLines) { + pending = pending.slice(0, READ_FILE_MAX_LINE_LENGTH); + pendingOverflow = true; + } + // windowHugeLines keeps the full pending: the scan trip ends the + // stream, and flushRemainder windows the remainder so every scanned + // byte stays reachable through offset continuation. + return true; + } + const line = pending.slice(0, nl); + pending = pending.slice(nl + 1); + if (wrapLongLines) { + if (!emitWrapped(line, true)) return false; + } else { + const overflow = pendingOverflow; + pendingOverflow = false; + if (!overflow && windowHugeLines && line.length > contentBudget) { + if (!emitWrapped(line, true)) return false; + } else if (!handleLine(line, overflow)) return false; + } + } + }; + + const flushRemainder = (): void => { + if (pending.length === 0) return; + if (wrapLongLines) { + emitWrapped(pending, true); + return; + } + // Windowed even when scan-capped: every window burns a line number, + // so the footer's offset resumes at the next window instead of promising + // continuation that skips the unshown middle of an overlong line. + if ( + !pendingOverflow && + windowHugeLines && + pending.length > contentBudget + ) { + emitWrapped(pending, true); + return; + } + handleLine(pending, pendingOverflow); + }; + + const finishOk = () => { + if (emitted === 0) { + if (lineNo === 0 && endReached) { + done({ content: "" }); + return; + } + // Skip bytes are not scanned, so a dead offset on a file larger than + // the ceiling still reaches EOF. Report the true range, not a scan + // limit that would hide a reachable EOF. + if (endReached) { + done({ + content: `[offset ${offset} is beyond end of file ${displayPath} (${lineNo} lines); valid offsets 0-${lineNo - 1}]`, + isError: true, + }); + } else { + done({ + content: `[reached the ${ + scanCeiling / (1024 * 1024) + }MB scan limit before offset ${offset}; the file is larger than read_file scans in one pass. Use a smaller offset or grep to locate content.]`, + isError: true, + }); + } + return; + } + + let content = out.join("\n"); + if (truncReason !== undefined) { + content += `\n\n${continuationNotice(truncReason, offset + 1, lastEmittedLine, limit, scanCeiling)}`; + } + done({ content }); + }; + + stream.on("data", (chunk: Buffer) => { + if (settled) return; + if (firstChunk) { + firstChunk = false; + const diagnosed = diagnoseFirstChunk?.(chunk); + if (diagnosed !== undefined) { + done({ + content: diagnosed, + isError: true, + }); + return; + } + if (chunk.includes(0)) { + done({ + content: `refusing to read binary file: ${displayPath}`, + isError: true, + }); + return; + } + } + if (skipDone) scanned += chunk.length; + pending += decoder.write(chunk); + if (!drainPending()) { + finishOk(); + return; + } + if (scanned >= scanCeiling) { + flushRemainder(); + if (truncReason === undefined) truncReason = "scan"; + finishOk(); + } + }); + + stream.on("end", () => { + if (settled) return; + endReached = true; + pending += decoder.end(); + flushRemainder(); + finishOk(); + }); + + stream.on("error", (err: NodeJS.ErrnoException) => { + if (settled) return; + settled = true; + signal.removeEventListener("abort", onAbort); + if (mapStreamError !== undefined) rejectP(mapStreamError(err)); + else rejectP(err); + }); + }); +} + + +/** + * Bounded read over an in-memory UTF-8 blob (tool-output spills). Feeds the + * buffer in chunks so offset/limit never require a full-text split; overlong + * lines wrap into numbered windows instead of being truncated. Callers pass + * a high `limit` so the byte budget, not the source-file 2000-line cap, + * pages the spill. + */ +export function readBytesBounded( + bytes: Uint8Array, + offset: number, + limit: number, + signal: AbortSignal, + displayPath: string, +): Promise { + async function* byteChunks(): AsyncGenerator { + for (let i = 0; i < bytes.length; i += TOOL_OUTPUT_CHUNK_BYTES) { + yield Buffer.from( + bytes.subarray(i, Math.min(i + TOOL_OUTPUT_CHUNK_BYTES, bytes.length)), + ); + } + } + return readStreamBounded( + Readable.from(byteChunks()), + displayPath, + offset, + limit, + signal, + { + wrapLongLines: true, + }, + ); +} + + +function continuationNotice( + reason: TruncReason, + firstLine: number, + lastLine: number, + limit: number, + scanCeiling: number, +): string { + const next = `Use offset=${lastLine} to continue.`; + if (reason === "lines") { + return `[Showing lines ${firstLine}-${lastLine}; stopped at the ${limit}-line limit. ${next}]`; + } + if (reason === "bytes") { + return `[Showing lines ${firstLine}-${lastLine}; stopped at the ${ + READ_FILE_MAX_BYTES / 1024 + }KB output limit. ${next}]`; + } + return `[Showing lines ${firstLine}-${lastLine}; stopped at the ${ + scanCeiling / (1024 * 1024) + }MB scan limit. ${next}]`; +} + diff --git a/src/plugins/bounded-grep-fallback.ts b/src/plugins/bounded-grep-fallback.ts index a0ed6f238..8fbc00d28 100644 --- a/src/plugins/bounded-grep-fallback.ts +++ b/src/plugins/bounded-grep-fallback.ts @@ -3,9 +3,23 @@ import { readFile, readdir, stat } from "node:fs/promises"; import { basename, join, relative, resolve } from "node:path"; import { hasCode } from "@intx/types"; +import { compileUserRegex, globToRegExp } from "../util/glob-regex.js"; +import { NARROW_SEARCH_HINT } from "./tool-time-budget.js"; const SKIP_SEGMENTS = new Set(["node_modules", ".git"]); +// brace expansion is caller-local semantics the shared glob translation does +// not carry: this surface refuses `{a,b}` instead of accepting the literal. +function rejectBraceExpansion(pattern: string): string { + if (/\{[^}]+\}/.test(pattern)) { + throw new Error( + `brace expansion is not supported: "${pattern}". Use separate searches or a ** pattern instead.`, + ); + } + return pattern; +} + + /** Max file paths considered per search (directory walk). */ export const BOUNDED_GREP_MAX_DIRECTORY_ENTRIES = 25_000; @@ -50,45 +64,6 @@ function shouldSkip(relativePath: string): boolean { return segments.some((s) => SKIP_SEGMENTS.has(s)); } -function globToRegex(pattern: string): RegExp { - if (/\{[^}]+\}/.test(pattern)) { - throw new Error( - `brace expansion is not supported: "${pattern}". Use separate searches or a ** pattern instead.`, - ); - } - - let regex = ""; - let i = 0; - - while (i < pattern.length) { - const c = pattern.charAt(i); - - if (c === "*" && pattern[i + 1] === "*") { - i += 2; - if (pattern[i] === "/") { - i++; - regex += "(?:.+/)?"; - } else { - regex += ".*"; - } - } else if (c === "*") { - regex += "[^/]*"; - i++; - } else if (c === "?") { - regex += "[^/]"; - i++; - } else if (".+^${}()|[]\\".includes(c)) { - regex += "\\" + c; - i++; - } else { - regex += c; - i++; - } - } - - return new RegExp("^" + regex + "$"); -} - function isBinary(buf: Buffer): boolean { return buf.includes(0); } @@ -252,14 +227,7 @@ export async function runBoundedGrep( ): Promise { signal.throwIfAborted(); - let regex: RegExp; - try { - regex = new RegExp(args.pattern); - } catch (err) { - throw new Error( - `invalid regex: ${err instanceof Error ? err.message : String(err)}`, - ); - } + const regex = compileUserRegex(rejectBraceExpansion(args.pattern)); const maxDirectoryEntries = limits.maxDirectoryEntries ?? BOUNDED_GREP_MAX_DIRECTORY_ENTRIES; @@ -269,7 +237,7 @@ export async function runBoundedGrep( const basePath = resolve(baseCwd, args.path ?? "."); const contextLines = args.context ?? 0; const maxResults = args.max_results ?? BOUNDED_GREP_DEFAULT_MAX_RESULTS; - const globFilter = args.glob !== undefined ? globToRegex(args.glob) : null; + const globFilter = args.glob !== undefined ? globToRegExp(rejectBraceExpansion(args.glob)) : null; let info; try { @@ -329,7 +297,7 @@ export async function runBoundedGrep( output += `\n... (${maxResults} of ${totalMatches} matches shown)`; } if (walkTruncated) { - output += `\n... (directory walk capped at ${maxDirectoryEntries} files; narrow path/glob)`; + output += `\n... (directory walk capped at ${maxDirectoryEntries} files; ${NARROW_SEARCH_HINT})`; } return output; } @@ -347,7 +315,7 @@ export async function runBoundedSearchFiles( const basePath = resolve(baseCwd, args.path ?? "."); const maxResults = args.max_results ?? BOUNDED_SEARCH_DEFAULT_MAX_RESULTS; - const regex = globToRegex(args.pattern); + const regex = globToRegExp(rejectBraceExpansion(args.pattern)); let info; try { @@ -393,7 +361,7 @@ export async function runBoundedSearchFiles( result += `\n... (${maxResults} of ${totalMatches} matches shown)`; } if (walkTruncated) { - result += `\n... (directory walk capped at ${maxDirectoryEntries} files; narrow path/glob)`; + result += `\n... (directory walk capped at ${maxDirectoryEntries} files; ${NARROW_SEARCH_HINT})`; } return result; } diff --git a/src/plugins/delete-file-plugin.ts b/src/plugins/delete-file-plugin.ts index bddf23e79..158d4f755 100644 --- a/src/plugins/delete-file-plugin.ts +++ b/src/plugins/delete-file-plugin.ts @@ -5,8 +5,10 @@ import type { ExtraTool, ToolPlugin } from "@intx/tools-posix"; import type { ToolCall, ToolResult } from "@intx/types/runtime"; import { resolveWorkspacePath } from "../permission/path-restriction.js"; import type { RootsProvider } from "../permission/worktree-roots.js"; +import { hasCode } from "@intx/types"; import { toolInputSchema } from "../util/tool-input-schema.js"; import { formatChangeDiff } from "./change-diff.js"; +import { denyResult, resolveAllowOutside } from "./tool-args.js"; const DeleteFileArgs = type({ path: [ @@ -26,25 +28,6 @@ export const DELETE_FILE_DEFINITION = { inputSchema: toolInputSchema(DeleteFileArgs), }; -function errorResult(callId: string, message: string): ToolResult { - return { - callId, - content: `Failed to delete file: ${message}`, - isError: true, - }; -} - -function errorCode(error: unknown): string | undefined { - if (!(error instanceof Error) || !("code" in error)) return undefined; - return typeof error.code === "string" ? error.code : undefined; -} - -function failureDetail(error: unknown): string { - if (!(error instanceof Error)) return String(error); - const code = errorCode(error); - return code === undefined ? error.message : `${code}: ${error.message}`; -} - export interface DeleteFilePluginOptions { // When true (yolo / --dangerously-skip-permissions), delete outside the // working directory. A getter is resolved per call so `/yolo` mid-session @@ -55,13 +38,6 @@ export interface DeleteFilePluginOptions { rootsProvider?: RootsProvider; } -function resolveAllowOutside( - value: boolean | (() => boolean) | undefined, -): boolean { - if (typeof value === "function") return value(); - return value === true; -} - // Above this size, skip reading the file into memory just to show a diff — // the diff output is already char-capped (MAX_DIFF_CHARS), so buffering a // large file for it is pure waste. @@ -76,7 +52,7 @@ export function deleteFilePlugin( handler: async (call: ToolCall): Promise => { const args = DeleteFileArgs(call.arguments); if (args instanceof type.errors) { - return errorResult(call.id, "delete_file requires a non-empty path"); + return denyResult(call.id, "Failed to delete file: delete_file requires a non-empty path"); } const allowOutside = resolveAllowOutside(options.allowOutside); @@ -96,17 +72,17 @@ export function deleteFilePlugin( options.rootsProvider ?? (() => []), ) === undefined ) { - return errorResult( + return denyResult( call.id, - `${args.path} resolves outside the working directory`, + `Failed to delete file: ${args.path} resolves outside the working directory`, ); } try { const info = await lstat(target); if (info.isDirectory()) { - return errorResult( + return denyResult( call.id, - `${args.path} is a directory; delete_file only deletes files`, + `Failed to delete file: ${args.path} is a directory; delete_file only deletes files`, ); } // Best-effort content capture before removal, so the result can show @@ -114,8 +90,8 @@ export function deleteFilePlugin( // (binary, permissions) never blocks the delete. Large files skip the // read entirely (see MAX_DELETE_PREVIEW_BYTES) and get a byte-count // summary instead. - let before: string | undefined; const tooLargeToPreview = info.size > MAX_DELETE_PREVIEW_BYTES; + let before: string | undefined; if (!tooLargeToPreview) { try { before = await readFile(target, "utf8"); @@ -140,10 +116,27 @@ export function deleteFilePlugin( content: `File already absent: ${args.path} (no action needed)`, }; } - return errorResult(call.id, failureDetail(error)); + return denyResult(call.id, `Failed to delete file: ${failureDetail(error)}`); } }, }; return { tools: [tool] }; } + +function failureDetail(error: unknown): string { + if (error instanceof Error) { + const code = + hasCode(error) && typeof error.code === "string" + ? error.code + : undefined; + return code === undefined ? error.message : `${code}: ${error.message}`; + } + return String(error); +} + +function errorCode(error: unknown): string | undefined { + return hasCode(error) && typeof error.code === "string" + ? error.code + : undefined; +} diff --git a/src/plugins/edit-file-line-range-plugin.ts b/src/plugins/edit-file-line-range-plugin.ts index 7b750734f..6c351aa62 100644 --- a/src/plugins/edit-file-line-range-plugin.ts +++ b/src/plugins/edit-file-line-range-plugin.ts @@ -3,6 +3,7 @@ import { parseEditFileMode, runEditFileLineRange, } from "./edit-file-line-range.js"; +import { denyResult, errorMessage } from "./tool-args.js"; /** * Short-circuits stock tools-posix edit_file for line-range mode (shell-guard pattern). @@ -16,7 +17,7 @@ export function editFileLineRangePlugin(): ToolPlugin { const parsed = parseEditFileMode(call.arguments); if (parsed.kind === "invalid") { - return { callId: call.id, content: parsed.message, isError: true }; + return denyResult(call.id, parsed.message); } if (parsed.kind === "substring") { return next(call, signal); @@ -26,11 +27,7 @@ export function editFileLineRangePlugin(): ToolPlugin { const content = await runEditFileLineRange(parsed, signal); return { callId: call.id, content }; } catch (err) { - return { - callId: call.id, - content: err instanceof Error ? err.message : String(err), - isError: true, - }; + return denyResult(call.id, errorMessage(err)); } }, }; diff --git a/src/plugins/evidence-archive-search-plugin.ts b/src/plugins/evidence-archive-search-plugin.ts index 8c3747b09..14e477f45 100644 --- a/src/plugins/evidence-archive-search-plugin.ts +++ b/src/plugins/evidence-archive-search-plugin.ts @@ -1,10 +1,8 @@ import type { ToolDefinition } from "@intx/types/runtime"; import type { ToolPlugin } from "@intx/tools-posix"; -import { - READ_FILE_DEFAULT_MAX_LINES, - readBytesBounded, -} from "./read-file-guard-plugin.js"; +import { READ_FILE_DEFAULT_MAX_LINES } from "./read-file-guard-plugin.js"; +import { readBytesBounded } from "../io/bounded-read.js"; import type { CompactionArchive } from "../session/compaction-archive.js"; import type { ArchiveOccurrence } from "../session/compaction-archive-schema.js"; import { @@ -12,50 +10,17 @@ import { isArchiveLike, parseArchiveTarget, } from "../session/compaction-archive.js"; +import { + denyResult, + errorMessage, + optionalNumber, + optionalString, +} from "./tool-args.js"; +import { compileUserRegex, globToRegExp } from "../util/glob-regex.js"; const SEARCH_DEFAULT_MAX = 1000; const GREP_DEFAULT_MAX = 500; -function str(value: unknown): string | undefined { - return typeof value === "string" && value.length > 0 ? value : undefined; -} - -function num(value: unknown): number | undefined { - return typeof value === "number" && Number.isFinite(value) - ? value - : undefined; -} - -function globToRegExp(pattern: string): RegExp { - let regex = ""; - let i = 0; - while (i < pattern.length) { - const c = pattern.charAt(i); - if (c === "*" && pattern[i + 1] === "*") { - i += 2; - if (pattern[i] === "/") { - i++; - regex += "(?:.+/)?"; - } else { - regex += ".*"; - } - } else if (c === "*") { - regex += "[^/]*"; - i++; - } else if (c === "?") { - regex += "[^/]"; - i++; - } else if (".+^${}()|[]\\".includes(c)) { - regex += `\\${c}`; - i++; - } else { - regex += c; - i++; - } - } - return new RegExp(`^${regex}$`); -} - function matchesArchiveName(pattern: string, occ: ArchiveOccurrence): boolean { const matcher = globToRegExp(pattern); return ( @@ -156,16 +121,15 @@ export function evidenceArchiveSearchPlugin( ) { return next(call, signal); } - const path = str(call.arguments.path); + const path = optionalString(call.arguments.path); if (path === undefined || !isArchiveLike(path)) return next(call, signal); const archive = getArchive(); if (archive === undefined) { - return { - callId: call.id, - content: "Error: evidence archive is not available in this session.", - isError: true, - }; + return denyResult( + call.id, + "Error: evidence archive is not available in this session.", + ); } const target = parseArchiveTarget(path); @@ -184,16 +148,15 @@ export function evidenceArchiveSearchPlugin( }; } if (call.name === "search_files") { - const pattern = str(call.arguments.pattern); + const pattern = optionalString(call.arguments.pattern); if (pattern === undefined) { - return { - callId: call.id, - content: "Error: search_files requires pattern (string).", - isError: true, - }; + return denyResult( + call.id, + "Error: search_files requires pattern (string).", + ); } const maxResults = - num(call.arguments.max_results) ?? SEARCH_DEFAULT_MAX; + optionalNumber(call.arguments.max_results) ?? SEARCH_DEFAULT_MAX; return { callId: call.id, content: await searchArchiveFiles( @@ -205,17 +168,13 @@ export function evidenceArchiveSearchPlugin( ), }; } - const pattern = str(call.arguments.pattern); + const pattern = optionalString(call.arguments.pattern); if (pattern === undefined) { - return { - callId: call.id, - content: "Error: grep requires pattern (string).", - isError: true, - }; + return denyResult(call.id, "Error: grep requires pattern (string)."); } - const maxResults = num(call.arguments.max_results) ?? GREP_DEFAULT_MAX; - const glob = str(call.arguments.glob); - const contextArg = num(call.arguments.context); + const maxResults = optionalNumber(call.arguments.max_results) ?? GREP_DEFAULT_MAX; + const glob = optionalString(call.arguments.glob); + const contextArg = optionalNumber(call.arguments.context); const context = contextArg !== undefined && contextArg > 0 ? Math.floor(contextArg) @@ -233,11 +192,7 @@ export function evidenceArchiveSearchPlugin( ), }; } catch (err) { - return { - callId: call.id, - content: `Error: ${err instanceof Error ? err.message : String(err)}`, - isError: true, - }; + return denyResult(call.id, `Error: ${errorMessage(err)}`); } }, }; @@ -253,10 +208,10 @@ async function readArchiveOccurrence( throw new Error("read_file archive path must be archive:///{occurrenceId}"); } const text = await archive.readAuthorizedPayload(occurrenceId); - const offsetArg = num(args.offset); + const offsetArg = optionalNumber(args.offset); const offset = offsetArg !== undefined && offsetArg > 0 ? Math.floor(offsetArg) : 0; - const limitArg = num(args.limit); + const limitArg = optionalNumber(args.limit); const limit = limitArg !== undefined && limitArg > 0 ? Math.floor(limitArg) @@ -348,14 +303,7 @@ async function grepArchive( signal: AbortSignal, ): Promise { signal.throwIfAborted(); - let regex: RegExp; - try { - regex = new RegExp(pattern); - } catch (err) { - throw new Error( - `invalid regex: ${err instanceof Error ? err.message : String(err)}`, - ); - } + const regex = compileUserRegex(pattern); const occurrences = await selectOccurrences(archive, occurrenceId); const hits: string[] = []; let matchCount = 0; diff --git a/src/plugins/path-escape-plugin.ts b/src/plugins/path-escape-plugin.ts index decf051f1..b67ac721b 100644 --- a/src/plugins/path-escape-plugin.ts +++ b/src/plugins/path-escape-plugin.ts @@ -8,6 +8,7 @@ import { type RootsProvider, } from "../permission/worktree-roots.js"; import { canonicalToolName } from "../agent/canonical-tool-name.js"; +import { denyResult, errorMessage, resolveAllowOutside } from "./tool-args.js"; export interface PathEscapeOptions { // When true (yolo / --dangerously-skip-permissions), paths outside the @@ -22,13 +23,6 @@ export interface PathEscapeOptions { trustedPluginRoots?: RootsProvider; } -function resolveAllowOutside( - value: boolean | (() => boolean) | undefined, -): boolean { - if (typeof value === "function") return value(); - return value === true; -} - const PLUGIN_READ_TOOLS = new Set([ "read_file", "grep", @@ -71,8 +65,7 @@ export function pathEscapePlugin( options.trustedPluginRoots, ); } catch (err) { - const message = err instanceof Error ? err.message : String(err); - return { callId: call.id, content: message, isError: true }; + return denyResult(call.id, errorMessage(err)); } return next({ ...call, arguments: escaped }, signal); }, @@ -87,56 +80,37 @@ function escapeArgs( toolName: string, trustedPluginRoots?: RootsProvider, ): Record { - if (!allowOutside) { - const reason = pathEscapeBlockReason( - args, - cwd, - rootsProvider, - toolName, - trustedPluginRoots, - ); - if (reason !== undefined) throw new Error(reason); - } - const combined = rootsForEscape(toolName, rootsProvider, trustedPluginRoots); - return escapeValue( + // One fused walk: sanitizePath throws the same deny strings + // pathEscapeBlockReason computes, so a surviving call is exactly one the + // block-reason leg would have passed. + const roots = rootsForEscape(toolName, rootsProvider, trustedPluginRoots); + return walkPathArgs( args, - cwd, - combined, - allowOutside, undefined, - toolName, + (value) => sanitizePath(value, cwd, roots, allowOutside, toolName), ) as Record; } -function escapeValue( +/** + * Deep-walk shared by every path-argument pass: strings under path-keyed + * arguments (innermost key wins; array entries inherit the array key) go to + * `visit`, everything else passes through untouched. + */ +function walkPathArgs( value: unknown, - cwd: string, - rootsProvider: RootsProvider, - allowOutside: boolean, key: string | undefined, - toolName: string, + visit: (value: string) => unknown, ): unknown { if (typeof value === "string") { - return key !== undefined && looksLikePath(key) - ? sanitizePath(value, cwd, rootsProvider, allowOutside, toolName) - : value; + return key !== undefined && looksLikePath(key) ? visit(value) : value; } if (Array.isArray(value)) { - return value.map((entry) => - escapeValue(entry, cwd, rootsProvider, allowOutside, key, toolName), - ); + return value.map((entry) => walkPathArgs(entry, key, visit)); } if (typeof value === "object" && value !== null) { const out: Record = {}; for (const [entryKey, entryValue] of Object.entries(value)) { - out[entryKey] = escapeValue( - entryValue, - cwd, - rootsProvider, - allowOutside, - entryKey, - toolName, - ); + out[entryKey] = walkPathArgs(entryValue, entryKey, visit); } return out; } @@ -194,12 +168,16 @@ export function looksLikePath(key: string): boolean { const TOOL_OUTPUT_URI_TOOL = "read_file"; const ARCHIVE_URI_TOOLS = new Set(["read_file", "grep", "search_files"]); +function toolOutputUriDenial(toolName: string, value: string): string { + return `cannot ${toolName} a tool-output:// URI: ${value}. Use read_file with that URI to read the spilled output instead.`; +} + // "skip" when this tool may receive the virtual ref, a block message when it // may not, undefined when the value is an ordinary filesystem path. An omitted // toolName denies rather than skips: both production callers (the middleware // and the permission gate) always pass a name, so an omission is a caller bug // and must fail closed instead of silently skipping the deny. -function virtualRefVerdict( +export function virtualRefVerdict( value: string, toolName: string | undefined, ): "skip" | string | undefined { @@ -210,7 +188,7 @@ function virtualRefVerdict( if (toolName === undefined) { return `cannot use a tool-output:// URI without a tool identity: ${value}. Use read_file with that URI to read the spilled output instead.`; } - return `cannot ${toolName} a tool-output:// URI: ${value}. Use read_file with that URI to read the spilled output instead.`; + return toolOutputUriDenial(toolName, value); } if (isArchiveLike(value)) { if (toolName !== undefined && ARCHIVE_URI_TOOLS.has(toolName)) { @@ -260,70 +238,42 @@ export function normalizePathArguments( args, cwd, rootsForEscape("read_file", rootsProvider, trustedPluginRoots), - ) as Record; + ); } function normalizeValue( - value: unknown, + args: Record, cwd: string, rootsProvider: RootsProvider, - key?: string, -): unknown { - if (typeof value === "string") { - if (key === undefined || !looksLikePath(key)) return value; +): Record { + return walkPathArgs(args, undefined, (value) => { if (isToolOutputLike(value) || isArchiveLike(value)) return value; return resolveWorkspacePath(cwd, value, rootsProvider) ?? value; - } - if (Array.isArray(value)) { - return value.map((entry) => normalizeValue(entry, cwd, rootsProvider, key)); - } - if (typeof value === "object" && value !== null) { - const out: Record = {}; - for (const [entryKey, entryValue] of Object.entries(value)) { - out[entryKey] = normalizeValue(entryValue, cwd, rootsProvider, entryKey); - } - return out; - } - return value; + }) as Record; } function blockReasonFor( - value: unknown, + args: Record, cwd: string, rootsProvider: RootsProvider, key: string | undefined, toolName: string, ): string | undefined { - if (typeof value === "string") { - if (key === undefined || !looksLikePath(key)) return undefined; + let reason: string | undefined = undefined; + walkPathArgs(args, key, (value) => { + if (reason !== undefined) return value; const verdict = virtualRefVerdict(value, toolName); - if (verdict === "skip") return undefined; - if (typeof verdict === "string") return verdict; - if (resolveWorkspacePath(cwd, value, rootsProvider) === undefined) { - return `Path escapes working directory: ${value}`; + if (verdict === "skip") return value; + if (typeof verdict === "string") { + reason = verdict; + return value; } - return undefined; - } - if (Array.isArray(value)) { - for (const entry of value) { - const reason = blockReasonFor(entry, cwd, rootsProvider, key, toolName); - if (reason !== undefined) return reason; - } - return undefined; - } - if (typeof value === "object" && value !== null) { - for (const [entryKey, entryValue] of Object.entries(value)) { - const reason = blockReasonFor( - entryValue, - cwd, - rootsProvider, - entryKey, - toolName, - ); - if (reason !== undefined) return reason; + if (resolveWorkspacePath(cwd, value, rootsProvider) === undefined) { + reason = `Path escapes working directory: ${value}`; } - } - return undefined; + return value; + }); + return reason; } function sanitizePath( diff --git a/src/plugins/read-file-guard-plugin.ts b/src/plugins/read-file-guard-plugin.ts index 46579f063..abfb1741d 100644 --- a/src/plugins/read-file-guard-plugin.ts +++ b/src/plugins/read-file-guard-plugin.ts @@ -1,48 +1,44 @@ import { createReadStream } from "node:fs"; import { stat } from "node:fs/promises"; import { resolve } from "node:path"; -import { Readable } from "node:stream"; -import { StringDecoder } from "node:string_decoder"; import type { ToolPlugin } from "@intx/tools-posix"; import type { BlobReader } from "@intx/types/runtime"; import { canonicalToolOutputUri, isToolOutputLike, } from "../util/tool-output-uri.js"; -import { formatReadFileTimeoutMessage } from "./tool-time-budget.js"; +import { + denyResult, + errorMessage, + optionalNumber, +} from "./tool-args.js"; import { diagnoseBlockedFileInspection, inspectionKindFromFirstChunk, PDF_EXTRACTOR_BIN, } from "./file-inspection-diagnosis.js"; -// Corbits Code-side guard for read_file. Stock @intx/tools-posix read-file -// buffers the whole file and returns every line with no cap, so a deep read -// can pull multi-MB files into context and OOM the host. +import { + type BoundedRead, + READ_FILE_MAX_SCAN_BYTES, + readBytesBounded, + readStreamBounded, +} from "../io/bounded-read.js"; + +export { + READ_FILE_MAX_BYTES, + READ_FILE_MAX_LINE_LENGTH, + READ_FILE_MAX_SCAN_BYTES, + readBytesBounded, +} from "../io/bounded-read.js"; -export const READ_FILE_MAX_BYTES = 50 * 1024; export const READ_FILE_DEFAULT_MAX_LINES = 2000; -export const READ_FILE_MAX_LINE_LENGTH = 2000; -// Absolute ceiling on bytes scanned from disk past the requested offset, so -// an emission window stays time-bounded. Bytes skipped to reach a nonzero -// offset do not count: continuation past the ceiling must read through to -// the end, not dead-end at the limit. -export const READ_FILE_MAX_SCAN_BYTES = 8 * 1024 * 1024; /** Refuse tool-output blobs larger than this before bounded paging. */ export const READ_FILE_MAX_TOOL_OUTPUT_BYTES = READ_FILE_MAX_SCAN_BYTES; -// Headroom reserved out of the byte budget for the continuation notice, so -// the payload plus notice stays under READ_FILE_MAX_BYTES. -const NOTICE_RESERVE_BYTES = 256; -const LINE_TRUNC_SUFFIX = ` ... [line truncated at ${READ_FILE_MAX_LINE_LENGTH} chars; full line remains in the file — use grep to match within the line]`; -const TOOL_OUTPUT_CHUNK_BYTES = 64 * 1024; - -type TruncReason = "lines" | "bytes" | "scan"; - -interface BoundedRead { - content: string; - isError?: boolean; -} +// Corbits Code-side guard for read_file. Stock @intx/tools-posix read-file +// buffers the whole file and returns every line with no cap, so a deep read +// can pull multi-MB files into context and OOM the host. export interface ReadFileGuardPluginOptions { blobReader?: BlobReader; @@ -66,24 +62,6 @@ export interface ReadFileGuardPluginOptions { maxScanBytes?: number; } -// A truncated read tells the model to continue with the same path and the -// explicit next offset from the notice. There is no continuation handle: -// every read is a stateless, idempotent ranged read, so following a notice -// verbatim works on first use, replay, and a fresh plugin instance after -// compaction or session resume. Chunked reads carry rising offsets, so -// call-keying detectors see one ranged read per window, not a same-path -// loop. - -function numArg(value: unknown): number | undefined { - return typeof value === "number" && Number.isFinite(value) - ? value - : undefined; -} - -function numberLine(lineNo: number, text: string): string { - return `${String(lineNo).padStart(6, " ")}\t${text}`; -} - function mapFilesystemStreamError( displayPath: string, err: NodeJS.ErrnoException, @@ -116,244 +94,6 @@ function resolveExtractorProbe( return probe() !== null; } -/** - * Streams UTF-8 from `stream`, emitting up to `limit` line-numbered lines - * after skipping `offset` lines (zero-based); never decodes the full text in - * one pass. With `wrapLongLines`, overlong lines split into numbered windows - * so a giant JSON line pages like a multi-line file. With `windowHugeLines` - * instead, only lines that alone exceed the output budget are windowed, so - * plain path+offset pagination stays line-aligned. - */ -function readStreamBounded( - stream: Readable, - displayPath: string, - offset: number, - limit: number, - signal: AbortSignal, - options: { - mapStreamError?: (err: NodeJS.ErrnoException) => Error; - wrapLongLines?: boolean; - windowHugeLines?: boolean; - diagnoseFirstChunk?: (chunk: Buffer) => string | undefined; - maxScanBytes?: number; - } = {}, -): Promise { - return new Promise((resolveP, rejectP) => { - const { - mapStreamError, - wrapLongLines = false, - windowHugeLines = false, - diagnoseFirstChunk, - maxScanBytes, - } = options; - const scanCeiling = maxScanBytes ?? READ_FILE_MAX_SCAN_BYTES; - const decoder = new StringDecoder("utf8"); - const contentBudget = READ_FILE_MAX_BYTES - NOTICE_RESERVE_BYTES; - - let pending = ""; - let pendingOverflow = false; - let firstChunk = true; - let lineNo = 0; - let scanned = 0; - let skipDone = offset <= 0; - let outBytes = 0; - let emitted = 0; - let lastEmittedLine = 0; - let truncReason: TruncReason | undefined; - let endReached = false; - let settled = false; - - const out: string[] = []; - - const onAbort = () => { - if (settled) return; - settled = true; - stream.destroy(); - const partial = out.length > 0 ? out.join("\n") : undefined; - rejectP(new Error(formatReadFileTimeoutMessage(displayPath, partial))); - }; - signal.addEventListener("abort", onAbort, { once: true }); - if (signal.aborted) { - onAbort(); - return; - } - - const done = (result: BoundedRead) => { - if (settled) return; - settled = true; - stream.destroy(); - signal.removeEventListener("abort", onAbort); - resolveP(result); - }; - - const handleLine = (raw: string, overflow: boolean): boolean => { - lineNo++; - if (lineNo <= offset) return true; - skipDone = true; - if (emitted >= limit) { - truncReason = "lines"; - return false; - } - const tooLong = overflow || raw.length > READ_FILE_MAX_LINE_LENGTH; - const text = tooLong - ? raw.slice(0, READ_FILE_MAX_LINE_LENGTH) + LINE_TRUNC_SUFFIX - : raw; - const numbered = numberLine(lineNo, text); - const bytes = Buffer.byteLength(numbered, "utf8") + 1; - if (emitted > 0 && outBytes + bytes > contentBudget) { - truncReason = "bytes"; - return false; - } - out.push(numbered); - outBytes += bytes; - emitted++; - lastEmittedLine = lineNo; - return true; - }; - - const emitWrapped = (raw: string, keepTail: boolean): boolean => { - let rest = raw; - while (rest.length > READ_FILE_MAX_LINE_LENGTH) { - if (!handleLine(rest.slice(0, READ_FILE_MAX_LINE_LENGTH), false)) - return false; - rest = rest.slice(READ_FILE_MAX_LINE_LENGTH); - } - if (keepTail) return handleLine(rest, false); - pending = rest; - return true; - }; - - const drainPending = (): boolean => { - for (;;) { - const nl = pending.indexOf("\n"); - if (nl === -1) { - if (wrapLongLines) return emitWrapped(pending, false); - if (pending.length > READ_FILE_MAX_LINE_LENGTH && !windowHugeLines) { - pending = pending.slice(0, READ_FILE_MAX_LINE_LENGTH); - pendingOverflow = true; - } - // windowHugeLines keeps the full pending: the scan trip ends the - // stream, and flushRemainder windows the remainder so every scanned - // byte stays reachable through offset continuation. - return true; - } - const line = pending.slice(0, nl); - pending = pending.slice(nl + 1); - if (wrapLongLines) { - if (!emitWrapped(line, true)) return false; - } else { - const overflow = pendingOverflow; - pendingOverflow = false; - if (!overflow && windowHugeLines && line.length > contentBudget) { - if (!emitWrapped(line, true)) return false; - } else if (!handleLine(line, overflow)) return false; - } - } - }; - - const flushRemainder = (): void => { - if (pending.length === 0) return; - if (wrapLongLines) { - emitWrapped(pending, true); - return; - } - // Windowed even when scan-capped: every window burns a line number, - // so the footer's offset resumes at the next window instead of promising - // continuation that skips the unshown middle of an overlong line. - if ( - !pendingOverflow && - windowHugeLines && - pending.length > contentBudget - ) { - emitWrapped(pending, true); - return; - } - handleLine(pending, pendingOverflow); - }; - - const finishOk = () => { - if (emitted === 0) { - if (lineNo === 0 && endReached) { - done({ content: "" }); - return; - } - // Skip bytes are not scanned, so a dead offset on a file larger than - // the ceiling still reaches EOF. Report the true range, not a scan - // limit that would hide a reachable EOF. - if (endReached) { - done({ - content: `[offset ${offset} is beyond end of file ${displayPath} (${lineNo} lines); valid offsets 0-${lineNo - 1}]`, - isError: true, - }); - } else { - done({ - content: `[reached the ${ - scanCeiling / (1024 * 1024) - }MB scan limit before offset ${offset}; the file is larger than read_file scans in one pass. Use a smaller offset or grep to locate content.]`, - isError: true, - }); - } - return; - } - - let content = out.join("\n"); - if (truncReason !== undefined) { - content += `\n\n${continuationNotice(truncReason, offset + 1, lastEmittedLine, limit, scanCeiling)}`; - } - done({ content }); - }; - - stream.on("data", (chunk: Buffer) => { - if (settled) return; - if (firstChunk) { - firstChunk = false; - const diagnosed = diagnoseFirstChunk?.(chunk); - if (diagnosed !== undefined) { - done({ - content: diagnosed, - isError: true, - }); - return; - } - if (chunk.includes(0)) { - done({ - content: `refusing to read binary file: ${displayPath}`, - isError: true, - }); - return; - } - } - if (skipDone) scanned += chunk.length; - pending += decoder.write(chunk); - if (!drainPending()) { - finishOk(); - return; - } - if (scanned >= scanCeiling) { - flushRemainder(); - if (truncReason === undefined) truncReason = "scan"; - finishOk(); - } - }); - - stream.on("end", () => { - if (settled) return; - endReached = true; - pending += decoder.end(); - flushRemainder(); - finishOk(); - }); - - stream.on("error", (err: NodeJS.ErrnoException) => { - if (settled) return; - settled = true; - signal.removeEventListener("abort", onAbort); - if (mapStreamError !== undefined) rejectP(mapStreamError(err)); - else rejectP(err); - }); - }); -} - /** * Streams a file, emitting up to `limit` line-numbered lines starting at * 1-indexed `offset`, holding at most one chunk plus the capped output in @@ -397,60 +137,6 @@ export function readFileBounded( ); } -/** - * Bounded read over an in-memory UTF-8 blob (tool-output spills). Feeds the - * buffer in chunks so offset/limit never require a full-text split; overlong - * lines wrap into numbered windows instead of being truncated. Callers pass - * a high `limit` so the byte budget, not the source-file 2000-line cap, - * pages the spill. - */ -export function readBytesBounded( - bytes: Uint8Array, - offset: number, - limit: number, - signal: AbortSignal, - displayPath: string, -): Promise { - async function* byteChunks(): AsyncGenerator { - for (let i = 0; i < bytes.length; i += TOOL_OUTPUT_CHUNK_BYTES) { - yield Buffer.from( - bytes.subarray(i, Math.min(i + TOOL_OUTPUT_CHUNK_BYTES, bytes.length)), - ); - } - } - return readStreamBounded( - Readable.from(byteChunks()), - displayPath, - offset, - limit, - signal, - { - wrapLongLines: true, - }, - ); -} - -function continuationNotice( - reason: TruncReason, - firstLine: number, - lastLine: number, - limit: number, - scanCeiling: number, -): string { - const next = `Use offset=${lastLine} to continue.`; - if (reason === "lines") { - return `[Showing lines ${firstLine}-${lastLine}; stopped at the ${limit}-line limit. ${next}]`; - } - if (reason === "bytes") { - return `[Showing lines ${firstLine}-${lastLine}; stopped at the ${ - READ_FILE_MAX_BYTES / 1024 - }KB output limit. ${next}]`; - } - return `[Showing lines ${firstLine}-${lastLine}; stopped at the ${ - scanCeiling / (1024 * 1024) - }MB scan limit. ${next}]`; -} - function resolveReadFilePaging( call: { arguments: Record }, defaultLimit = READ_FILE_DEFAULT_MAX_LINES, @@ -458,8 +144,8 @@ function resolveReadFilePaging( offset: number; limit: number; } { - const offsetArg = numArg(call.arguments.offset); - const limitArg = numArg(call.arguments.limit); + const offsetArg = optionalNumber(call.arguments.offset); + const limitArg = optionalNumber(call.arguments.limit); const offset = offsetArg !== undefined && offsetArg > 0 ? Math.floor(offsetArg) : 0; const limit = @@ -506,11 +192,10 @@ export function readFileGuardPlugin( } if (blobReader === undefined) { - return { - callId: call.id, - content: `cannot read ${rawPath}: no blob reader is configured for tool-output spills`, - isError: true, - }; + return denyResult( + call.id, + `cannot read ${rawPath}: no blob reader is configured for tool-output spills`, + ); } try { signal.throwIfAborted(); @@ -523,14 +208,10 @@ export function readFileGuardPlugin( uri, ); return res.isError - ? { callId: call.id, content: res.content, isError: true } + ? denyResult(call.id, res.content) : { callId: call.id, content: res.content }; } catch (err) { - return { - callId: call.id, - content: err instanceof Error ? err.message : String(err), - isError: true, - }; + return denyResult(call.id, errorMessage(err)); } } @@ -541,16 +222,15 @@ export function readFileGuardPlugin( if (info.isDirectory()) return next(call, signal); } catch (err) { if (isAccessDenied(err)) { - return { - callId: call.id, - content: diagnoseBlockedFileInspection({ + return denyResult( + call.id, + diagnoseBlockedFileInspection({ path: absolutePath, kind: "unreadable", extractorAvailable: false, canExecuteHostCommands: false, }).message, - isError: true, - }; + ); } return next(call, signal); } @@ -561,14 +241,10 @@ export function readFileGuardPlugin( displayPath: rawPath, }); return res.isError - ? { callId: call.id, content: res.content, isError: true } + ? denyResult(call.id, res.content) : { callId: call.id, content: res.content }; } catch (err) { - return { - callId: call.id, - content: err instanceof Error ? err.message : String(err), - isError: true, - }; + return denyResult(call.id, errorMessage(err)); } }, }; diff --git a/src/plugins/ripgrep-plugin.test.ts b/src/plugins/ripgrep-plugin.test.ts index a937e3af2..9e92b5267 100644 --- a/src/plugins/ripgrep-plugin.test.ts +++ b/src/plugins/ripgrep-plugin.test.ts @@ -11,7 +11,7 @@ import { buildCorePosixToolPlugins } from "../agent/posix-tool-plugins.js"; import { createPermissionGate } from "../permission/gate.js"; import { defined } from "../../testkit/defined.js"; import { withTempDir } from "../../testkit/temporary-dirs.js"; -import type { SpawnRg } from "./rg-run.js"; +import type { SpawnRg } from "../search/rg-run.js"; import { scriptedRgSpawn, stalledRgSpawn } from "./test-helpers.js"; // Repo root derived from this file, not process.cwd(): these cases search real diff --git a/src/plugins/ripgrep-plugin.ts b/src/plugins/ripgrep-plugin.ts index 49551ff49..e0c0085be 100644 --- a/src/plugins/ripgrep-plugin.ts +++ b/src/plugins/ripgrep-plugin.ts @@ -7,17 +7,24 @@ import { runBoundedSearchFiles, type BoundedGrepArgs, } from "./bounded-grep-fallback.js"; -import { createRgCollector } from "./rg-output.js"; +import { createRgCollector } from "../search/rg-output.js"; import { MAX_OUTPUT_BYTES, runRg, type RgLimits, type SpawnRg, -} from "./rg-run.js"; +} from "../search/rg-run.js"; import { + NARROW_SEARCH_HINT, formatUnboundedSearchMessage, isUnboundedRootSearch, } from "./tool-time-budget.js"; +import { + denyResult, + errorMessage, + optionalNumber, + optionalString, +} from "./tool-args.js"; import { createExtraDeniedPathMatcher } from "./secret-guard-plugin.js"; // A grep over a large tree with the pure-TypeScript walker enumerates the whole @@ -38,7 +45,7 @@ const DEFAULT_SEARCH_MAX = 1000; function capLines(text: string, max: number): string { const lines = text.split("\n").filter((line) => line.length > 0); if (lines.length <= max) return lines.join("\n"); - return `${lines.slice(0, max).join("\n")}\n... (showing first ${max} of ${lines.length}+ matches; narrow path/glob)`; + return `${lines.slice(0, max).join("\n")}\n... (showing first ${max} of ${lines.length}+ matches; ${NARROW_SEARCH_HINT})`; } // Mirrors read_file's truncate-and-offer behavior: a cap or timeout still @@ -72,16 +79,6 @@ function boundedContent( return partialContent(breach.stdout, maxResults, breach.notice); } -function str(value: unknown): string | undefined { - return typeof value === "string" && value.length > 0 ? value : undefined; -} - -function num(value: unknown): number | undefined { - return typeof value === "number" && Number.isFinite(value) - ? value - : undefined; -} - // rg prints paths relative to its cwd when the search target is ".", matching // the posix tool's directory-relative output. For a single file we search from // its parent so the printed path stays short and relative. @@ -159,12 +156,12 @@ export function ripgrepPlugin( return { middleware: (next) => async (call, signal) => { if (call.name === "grep") { - const pattern = str(call.arguments.pattern); + const pattern = optionalString(call.arguments.pattern); if (pattern === undefined) return next(call, signal); - const path = str(call.arguments.path) ?? cwd; - const context = num(call.arguments.context) ?? 0; - const glob = str(call.arguments.glob); - const maxResults = num(call.arguments.max_results) ?? DEFAULT_GREP_MAX; + const path = optionalString(call.arguments.path) ?? cwd; + const context = optionalNumber(call.arguments.context) ?? 0; + const glob = optionalString(call.arguments.glob); + const maxResults = optionalNumber(call.arguments.max_results) ?? DEFAULT_GREP_MAX; const { cwd: rgCwd, target } = searchLocation(path, cwd); const rgArgs = ["--line-number", "--no-heading", "--color", "never"]; @@ -194,18 +191,14 @@ export function ripgrepPlugin( ), }; } catch (err) { - return { - callId: call.id, - content: err instanceof Error ? err.message : String(err), - isError: true, - }; + return denyResult(call.id, errorMessage(err)); } } if (result.kind === "no-match") { return { callId: call.id, content: `no matches for /${pattern}/` }; } if (result.kind === "error") { - return { callId: call.id, content: result.message, isError: true }; + return denyResult(call.id, result.message); } if (result.kind === "partial") { return { @@ -224,7 +217,7 @@ export function ripgrepPlugin( } if (call.name === "search_files") { - const pattern = str(call.arguments.pattern); + const pattern = optionalString(call.arguments.pattern); if (pattern === undefined) return next(call, signal); // Unbounded workspace-root walks hang until the shell-guard budget // fires, and a timeout reads as a false negative. Refuse up front @@ -235,15 +228,14 @@ export function ripgrepPlugin( ? call.arguments.path : undefined; if (isUnboundedRootSearch({ path: rawPath, pattern, cwd })) { - return { - callId: call.id, - content: formatUnboundedSearchMessage("search_files", pattern), - isError: true, - }; + return denyResult( + call.id, + formatUnboundedSearchMessage("search_files", pattern), + ); } - const path = str(call.arguments.path) ?? cwd; + const path = optionalString(call.arguments.path) ?? cwd; const maxResults = - num(call.arguments.max_results) ?? DEFAULT_SEARCH_MAX; + optionalNumber(call.arguments.max_results) ?? DEFAULT_SEARCH_MAX; const { cwd: rgCwd, target } = searchLocation(path, cwd); const result = await runRg( @@ -269,18 +261,14 @@ export function ripgrepPlugin( ), }; } catch (err) { - return { - callId: call.id, - content: err instanceof Error ? err.message : String(err), - isError: true, - }; + return denyResult(call.id, errorMessage(err)); } } if (result.kind === "no-match") { return { callId: call.id, content: `no files matching "${pattern}"` }; } if (result.kind === "error") { - return { callId: call.id, content: result.message, isError: true }; + return denyResult(call.id, result.message); } if (result.kind === "partial") { return { diff --git a/src/plugins/search-files-fail-closed.test.ts b/src/plugins/search-files-fail-closed.test.ts index 3bdf28d16..ce9f5bbc3 100644 --- a/src/plugins/search-files-fail-closed.test.ts +++ b/src/plugins/search-files-fail-closed.test.ts @@ -11,7 +11,7 @@ import { TIMEOUT_PREFIX, } from "./tool-time-budget.js"; import { ripgrepPlugin } from "./ripgrep-plugin.js"; -import type { RgChild, SpawnRg } from "./rg-run.js"; +import type { RgChild, SpawnRg } from "../search/rg-run.js"; /** * search_files must fail closed on unbounded root walks instead of timing diff --git a/src/plugins/secret-guard-config-denylist.test.ts b/src/plugins/secret-guard-config-denylist.test.ts index 67a2c3dc3..8dafa097d 100644 --- a/src/plugins/secret-guard-config-denylist.test.ts +++ b/src/plugins/secret-guard-config-denylist.test.ts @@ -8,7 +8,7 @@ import { createPermissionGate } from "../permission/gate.js"; import { buildCorePosixToolPlugins } from "../agent/posix-tool-plugins.js"; import { createExtraDeniedPathMatcher } from "./secret-guard-plugin.js"; import { ripgrepPlugin } from "./ripgrep-plugin.js"; -import type { RgChild, SpawnRg } from "./rg-run.js"; +import type { RgChild, SpawnRg } from "../search/rg-run.js"; /** * An operator-chosen --config path inside the workspace is model-readable/ diff --git a/src/plugins/shell-guard-plugin.ts b/src/plugins/shell-guard-plugin.ts index 98614dc78..f19d86b8d 100644 --- a/src/plugins/shell-guard-plugin.ts +++ b/src/plugins/shell-guard-plugin.ts @@ -3,10 +3,8 @@ import { realpathSync } from "node:fs"; import { StringDecoder } from "node:string_decoder"; import type { ToolPlugin } from "@intx/tools-posix"; import type { ShellOutputFeedMap } from "../session/shell-output-feed.js"; -import { - killProcessTree, - type BackgroundShellRegistry, -} from "../shell/background-shell.js"; +import type { BackgroundShellRegistry } from "../shell/background-shell.js"; +import { killProcessTree } from "../shell/spawn-tree.js"; import { formatSearchTimeoutMessage, TIMEOUT_PREFIX, @@ -18,6 +16,12 @@ import { withTimeout, } from "../util/budget-race.js"; import type { ToolDefinition, ToolResult } from "@intx/types/runtime"; +import { + denyResult, + errorMessage, + optionalNumber, + resolveAllowOutside, +} from "./tool-args.js"; import { assertShellCwdUsable, isShellCwdWithinSession, @@ -510,12 +514,6 @@ export async function runGuardedShell( }); } -function optionalNumber(value: unknown): number | undefined { - return typeof value === "number" && Number.isFinite(value) - ? value - : undefined; -} - /** * Options for shellGuardPlugin: a hard-capped run_shell plus a 10s * wall-clock budget on grep/search_files. Short-circuits before the base @@ -534,13 +532,6 @@ export interface ShellGuardPluginOptions { getShellOutputFeeds?: () => ShellOutputFeedMap | undefined; } -function resolveAllowOutsideCwd( - value: boolean | (() => boolean) | undefined, -): boolean { - if (typeof value === "function") return value(); - return value === true; -} - export function shellGuardPlugin( cwd: string, timeoutConfig?: ShellTimeoutConfig, @@ -600,20 +591,14 @@ export function shellGuardPlugin( return enqueueShell(async () => { const command = call.arguments.command; if (typeof command !== "string" || command.length === 0) { - return { - callId: call.id, - content: 'argument "command" is required', - isError: true, - }; + return denyResult(call.id, 'argument "command" is required'); } const perCallCwdRaw = typeof call.arguments.cwd === "string" && call.arguments.cwd.length > 0 ? call.arguments.cwd : undefined; - const allowOutsideCwd = resolveAllowOutsideCwd( - options.allowOutsideCwd, - ); + const allowOutsideCwd = resolveAllowOutside(options.allowOutsideCwd); let executionCwd = retainedShellCwd; if (perCallCwdRaw !== undefined) { try { @@ -625,21 +610,13 @@ export function shellGuardPlugin( }, ); } catch (err) { - return { - callId: call.id, - content: err instanceof Error ? err.message : String(err), - isError: true, - }; + return denyResult(call.id, errorMessage(err)); } } try { assertShellCwdUsable(executionCwd); } catch (err) { - return { - callId: call.id, - content: err instanceof Error ? err.message : String(err), - isError: true, - }; + return denyResult(call.id, errorMessage(err)); } const requested = optionalNumber(call.arguments.timeout); const effectiveTimeout = resolveShellTimeoutMs({ @@ -653,11 +630,10 @@ export function shellGuardPlugin( if (call.arguments.background === true) { const registry = options.getBackgroundShellRegistry?.(); if (registry === undefined) { - return { - callId: call.id, - content: "background shell is not available in this session", - isError: true, - }; + return denyResult( + call.id, + "background shell is not available in this session", + ); } const started = registry.start({ command, @@ -668,9 +644,7 @@ export function shellGuardPlugin( maxOutputBytes, ...(env !== undefined ? { env } : {}), }); - if ("error" in started) { - return { callId: call.id, content: started.error, isError: true }; - } + if ("error" in started) return denyResult(call.id, started.error); // No pwd probe or retained-cwd mutation: the background shell runs // in its own process, so a `cd` inside it affects only itself. return { @@ -711,11 +685,10 @@ export function shellGuardPlugin( !allowOutsideCwd && !isShellCwdWithinSession(sessionRoot, parsed.finalCwd) ) { - return { - callId: call.id, - content: shellCwdEscapesSessionMessage(parsed.finalCwd), - isError: true, - }; + return denyResult( + call.id, + shellCwdEscapesSessionMessage(parsed.finalCwd), + ); } retainedShellCwd = parsed.finalCwd; } @@ -735,17 +708,13 @@ export function shellGuardPlugin( } return { callId: call.id, content }; } catch (err) { - return { - callId: call.id, - content: err instanceof Error ? err.message : String(err), - isError: true, - }; + return denyResult(call.id, errorMessage(err)); } finally { feeds?.drop(call.id); } }).catch((err: unknown) => ({ callId: call.id, - content: err instanceof Error ? err.message : String(err), + content: errorMessage(err), isError: true, })); } diff --git a/src/plugins/test-helpers.ts b/src/plugins/test-helpers.ts index e54d0cca0..0df800d66 100644 --- a/src/plugins/test-helpers.ts +++ b/src/plugins/test-helpers.ts @@ -1,7 +1,7 @@ import type { ToolCall } from "@intx/types/runtime"; import type { Middleware, ToolHandler, ToolPlugin } from "@intx/tools-posix"; -import type { RgChild, SpawnRg } from "./rg-run.js"; +import type { RgChild, SpawnRg } from "../search/rg-run.js"; export function neverAbort(): AbortSignal { return new AbortController().signal; diff --git a/src/plugins/tool-args.ts b/src/plugins/tool-args.ts new file mode 100644 index 000000000..e022c48c7 --- /dev/null +++ b/src/plugins/tool-args.ts @@ -0,0 +1,40 @@ +import type { ToolResult } from "@intx/types/runtime"; + +/** One error-text rendering: message when an Error, String(err) otherwise. */ +export function errorMessage(err: unknown): string { + return err instanceof Error ? err.message : String(err); +} + +// Scalar tool-argument guards, the yolo early-out, and the deny-result shape +// shared by the tool/plugins stack. Per-tool semantics stay with each caller; +// this only pins the shared vocabulary so the shape cannot drift between +// plugins answering the same way. + +/** Finite number argument, else undefined. Non-numbers and NaN/Infinity miss. */ +export function optionalNumber(value: unknown): number | undefined { + return typeof value === "number" && Number.isFinite(value) + ? value + : undefined; +} + +/** Non-empty string argument, else undefined. */ +export function optionalString(value: unknown): string | undefined { + return typeof value === "string" && value.length > 0 ? value : undefined; +} + +/** + * Resolve an `allowOutside` option passed as `boolean | (() => boolean)`: + * yolo is a live flag, so the getter is re-read per call and a mid-session + * toggle applies without rebuilding the plugin stack. + */ +export function resolveAllowOutside( + value: boolean | (() => boolean) | undefined, +): boolean { + if (typeof value === "function") return value(); + return value === true; +} + +/** The one deny result shape: swallowed error text in the model's call id. */ +export function denyResult(callId: string, message: string): ToolResult { + return { callId, content: message, isError: true }; +} diff --git a/src/plugins/tool-output-uri-plugin.ts b/src/plugins/tool-output-uri-plugin.ts index 78a5808d1..537332457 100644 --- a/src/plugins/tool-output-uri-plugin.ts +++ b/src/plugins/tool-output-uri-plugin.ts @@ -1,8 +1,7 @@ import type { ToolPlugin } from "@intx/tools-posix"; -import { - isToolOutputLike, - normalizeToolOutputUri, -} from "../util/tool-output-uri.js"; +import { isToolOutputLike, normalizeToolOutputUri } from "../util/tool-output-uri.js"; +import { virtualRefVerdict } from "./path-escape-plugin.js"; +import { denyResult } from "./tool-args.js"; /** * Normalize read_file tool-output URIs before the posix handler or read-file @@ -17,12 +16,14 @@ export function toolOutputUriPlugin(): ToolPlugin { if (typeof path !== "string" || !isToolOutputLike(path)) { return next(call, signal); } - if (call.name !== "read_file") { - return { - callId: call.id, - content: `cannot ${call.name} a tool-output:// URI: ${path}. Use read_file with that URI to read the spilled output instead.`, - isError: true, - }; + // path-escape's verdict is the single wording home for the + // tool-output:// deny; this plugin just applies it earlier in the stack. + const verdict = virtualRefVerdict(path, call.name); + if (verdict !== "skip") { + if (typeof verdict === "string") { + return denyResult(call.id, verdict); + } + return next(call, signal); } const normalized = normalizeToolOutputUri(path); if (normalized === path) { diff --git a/src/plugins/tool-time-budget.ts b/src/plugins/tool-time-budget.ts index 8b7d86d25..ccbb0821d 100644 --- a/src/plugins/tool-time-budget.ts +++ b/src/plugins/tool-time-budget.ts @@ -6,6 +6,22 @@ export type ScopedSearchTool = "grep" | "search_files"; export const TIMEOUT_PREFIX = "[timed out before completing]"; +// The single walk-caveat hint surfaced by every search cap/timeout notice. +export const NARROW_SEARCH_HINT = "narrow path/glob"; + +const NOT_A_NO_MATCH_TAIL = 'This is not the same as "no matches".'; + +// Timeout notices carry whatever partial output already streamed so the agent +// never loses the bytes it did get; empty partials attach the bare notice. +function withPartialResult( + notice: string, + partialResult?: string, +): string { + const trimmed = partialResult?.trim(); + if (trimmed === undefined || trimmed.length === 0) return notice; + return `${trimmed}\n\n${notice}`; +} + export function scopedSearchRetryHints(tool: ScopedSearchTool): string { const base = "Scope to a subdirectory (narrow `path`), add a `glob` filter, or use a more specific pattern"; @@ -19,12 +35,10 @@ export function formatSearchTimeoutMessage( tool: ScopedSearchTool, partialResult?: string, ): string { - const notice = - `${tool} ${TIMEOUT_PREFIX} — ${scopedSearchRetryHints(tool)}. ` + - `This is not the same as "no matches".`; - const trimmed = partialResult?.trim(); - if (trimmed === undefined || trimmed.length === 0) return notice; - return `${trimmed}\n\n${notice}`; + return withPartialResult( + `${tool} ${TIMEOUT_PREFIX} — ${scopedSearchRetryHints(tool)}. ${NOT_A_NO_MATCH_TAIL}`, + partialResult, + ); } // Whole-tree search_files patterns: recursive descent or a bare star with no @@ -87,8 +101,7 @@ export function formatUnboundedSearchMessage( ): string { return ( `${tool} refused an unbounded workspace-root walk for pattern "${pattern}" — ` + - `${scopedSearchRetryHints(tool)}. ` + - `This is not the same as "no matches".` + `${scopedSearchRetryHints(tool)}. ${NOT_A_NO_MATCH_TAIL}` ); } diff --git a/src/plugins/verify-plugin.ts b/src/plugins/verify-plugin.ts index 06919b4d0..8f48d6202 100644 --- a/src/plugins/verify-plugin.ts +++ b/src/plugins/verify-plugin.ts @@ -7,6 +7,7 @@ import { parseEditFileMode, } from "./edit-file-line-range.js"; import { formatChangeDiff } from "./change-diff.js"; +import { errorMessage } from "./tool-args.js"; function mutationPath(call: ToolCall): string | undefined { if (call.name !== "edit_file" && call.name !== "write_file") return undefined; @@ -63,7 +64,7 @@ export function verifyPlugin(): ToolPlugin { } catch (err) { return { callId: call.id, - content: `Write verification failed: could not re-read file: ${err instanceof Error ? err.message : String(err)}`, + content: `Write verification failed: could not re-read file: ${errorMessage(err)}`, isError: true, }; } @@ -107,7 +108,7 @@ export function verifyPlugin(): ToolPlugin { } catch (err) { return { callId: call.id, - content: `Edit verification failed: could not re-read file: ${err instanceof Error ? err.message : String(err)}`, + content: `Edit verification failed: could not re-read file: ${errorMessage(err)}`, isError: true, }; } diff --git a/src/plugins/rg-output.test.ts b/src/search/rg-output.test.ts similarity index 100% rename from src/plugins/rg-output.test.ts rename to src/search/rg-output.test.ts diff --git a/src/plugins/rg-output.ts b/src/search/rg-output.ts similarity index 96% rename from src/plugins/rg-output.ts rename to src/search/rg-output.ts index a7e1d59fe..d1a6b6ff2 100644 --- a/src/plugins/rg-output.ts +++ b/src/search/rg-output.ts @@ -1,3 +1,5 @@ +import { NARROW_SEARCH_HINT } from "../plugins/tool-time-budget.js"; + // Collects a ripgrep run's stdout and decides its outcome. `data`, `close` // and the timeout fire in a platform-dependent order, so the decision lives // here rather than in the handlers: the collector owns the accumulated bytes @@ -79,7 +81,7 @@ export function createRgCollector(maxOutputBytes: number): RgCollector { return settle({ kind: "partial", stdout, - notice: `ripgrep timed out after ${timeoutMs}ms — showing partial results; narrow path/glob`, + notice: `ripgrep timed out after ${timeoutMs}ms — showing partial results; ${NARROW_SEARCH_HINT}`, }); }, }; diff --git a/src/plugins/rg-run.test.ts b/src/search/rg-run.test.ts similarity index 98% rename from src/plugins/rg-run.test.ts rename to src/search/rg-run.test.ts index 8b192c5e1..fc3314a4b 100644 --- a/src/plugins/rg-run.test.ts +++ b/src/search/rg-run.test.ts @@ -5,7 +5,7 @@ import { scriptedRgSpawn, stalledRgSpawn, type RgScript, -} from "./test-helpers.js"; +} from "../plugins/test-helpers.js"; const line = "big.txt:1:match line here\n"; diff --git a/src/plugins/rg-run.ts b/src/search/rg-run.ts similarity index 88% rename from src/plugins/rg-run.ts rename to src/search/rg-run.ts index 9ef65e787..05094a407 100644 --- a/src/plugins/rg-run.ts +++ b/src/search/rg-run.ts @@ -1,6 +1,10 @@ import { spawn } from "node:child_process"; import { createRgCollector, type RgOutcome } from "./rg-output.js"; +import { + killProcessTree, + processGroupLeaderSpawnOptions, +} from "../shell/spawn-tree.js"; const RG_TIMEOUT_MS = 10_000; // Cap collected stdout so a runaway pattern cannot OOM the process before the @@ -41,8 +45,7 @@ const spawnRg: SpawnRg = (rgArgs, cwd, signal) => spawn("rg", rgArgs, { cwd, signal, - // Process-group leader so a kill reaches any grandchildren. - detached: process.platform !== "win32", + ...processGroupLeaderSpawnOptions(), }) as unknown as RgChild; export function runRg( @@ -60,19 +63,7 @@ export function runRg( let stderr = ""; let settled = false; - const killTree = (): void => { - if (child.pid === undefined) return; - try { - if (process.platform === "win32") child.kill("SIGKILL"); - else process.kill(-child.pid, "SIGKILL"); - } catch { - try { - child.kill("SIGKILL"); - } catch { - // already dead - } - } - }; + const killTree = (): void => killProcessTree(child); // Every path settles here, so whichever of data/close/timeout arrives first // stops the timer and reaps the child exactly once. diff --git a/src/shell/background-shell.ts b/src/shell/background-shell.ts index 6c3d793b7..1f945e628 100644 --- a/src/shell/background-shell.ts +++ b/src/shell/background-shell.ts @@ -1,4 +1,6 @@ import { spawn, type ChildProcess } from "node:child_process"; + +import { killProcessTree } from "./spawn-tree.js"; import { randomUUID } from "node:crypto"; import { BoundedShellOutput, @@ -15,28 +17,6 @@ export const MAX_RUNNING_BACKGROUND_SHELLS = 8; // holding the pipe must not delay the completion message after the child exits. const STDIO_DRAIN_MS = 100; -/** - * Signals the whole process group. With detached:true the shell is the group - * leader, so grandchildren (build watchers, test runners' workers) die too. - */ -export function killProcessTree(child: ChildProcess): void { - if (child.pid === undefined) return; - try { - if (process.platform === "win32") { - child.kill("SIGKILL"); - } else { - // Negative PID signals the whole process group. - process.kill(-child.pid, "SIGKILL"); - } - } catch { - try { - child.kill("SIGKILL"); - } catch { - // already exited - } - } -} - export interface StartBackgroundShellArgs { command: string; cwd: string; diff --git a/src/shell/spawn-tree.ts b/src/shell/spawn-tree.ts new file mode 100644 index 000000000..1ae5e44f7 --- /dev/null +++ b/src/shell/spawn-tree.ts @@ -0,0 +1,49 @@ +import { spawn, type ChildProcess, type SpawnOptions } from "node:child_process"; + +// One process-group-kill policy for every spawned tool subprocess: the child +// is spawned detached so it becomes a group leader, and the kill reaches the +// whole group (open-ended walks cannot outlive their parent on POSIX). + +/** The child subset a tree kill needs; ChildProcess satisfies it. */ +export interface TreeChild { + pid?: number | undefined; + kill: (signal?: NodeJS.Signals) => unknown; +} + +/** + * Signals the whole process group. With detached:true the spawned child is the + * group leader, so grandchildren (build watchers, test runners' workers) die too. + */ +export function killProcessTree(child: TreeChild): void { + if (child.pid === undefined) return; + try { + if (process.platform === "win32") { + child.kill("SIGKILL"); + } else { + // Negative PID signals the whole process group. + process.kill(-child.pid, "SIGKILL"); + } + } catch { + try { + child.kill("SIGKILL"); + } catch { + // already exited + } + } +} + +/** Spawn options making the child a process-group leader (POSIX only). */ +export function processGroupLeaderSpawnOptions(): Pick { + return { detached: process.platform !== "win32" }; +} + +export function spawnProcessGroupLeader( + command: string, + args: readonly string[], + options: SpawnOptions = {}, +): ChildProcess { + return spawn(command, [...args], { + ...processGroupLeaderSpawnOptions(), + ...options, + }); +} diff --git a/src/util/glob-regex.ts b/src/util/glob-regex.ts new file mode 100644 index 000000000..2a37063d2 --- /dev/null +++ b/src/util/glob-regex.ts @@ -0,0 +1,50 @@ +// Glob and user-regex patterns are accepted by several search surfaces; the +// translation rules and the "invalid regex" denial live in one place so the +// behavior cannot drift between them. Caller-specific semantics (e.g. brace +// expansion rejection) stay with the caller. + +// Cut the escaped set at `$`: no multiline/end anchors inside a single path +// token, and the wrap below anchors both ends itself. +const REGEX_SPECIALS = ".+^${}()|[]\\"; + +/** One glob segment language -> anchored RegExp (`*` stops at `/`, `**` does not). */ +export function globToRegExp(pattern: string): RegExp { + let regex = ""; + let i = 0; + while (i < pattern.length) { + const c = pattern.charAt(i); + if (c === "*" && pattern[i + 1] === "*") { + i += 2; + if (pattern[i] === "/") { + i++; + regex += "(?:.+/)?"; + } else { + regex += ".*"; + } + } else if (c === "*") { + regex += "[^/]*"; + i++; + } else if (c === "?") { + regex += "[^/]"; + i++; + } else if (REGEX_SPECIALS.includes(c)) { + regex += `\\${c}`; + i++; + } else { + regex += c; + i++; + } + } + return new RegExp(`^${regex}$`); +} + +/** Compile a model-supplied regex, turning a bad pattern into the shared denial. */ +export function compileUserRegex(pattern: string): RegExp { + try { + return new RegExp(pattern); + } catch (err) { + throw new Error( + `invalid regex: ${err instanceof Error ? err.message : String(err)}`, + ); + } +} From 64b93c8cb007501801e7acffc3e2aca826b4f923 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Wed, 14 Oct 2026 22:03:37 -0700 Subject: [PATCH 2/4] fix(tools): keep regex quantifiers in bounded grep Brace rejection stays on globs and search_files. Drop the test-only bounded-read re-export from the read-file guard. --- src/plugins/bounded-grep-fallback.test.ts | 29 ++++++++++++++++++++++ src/plugins/bounded-grep-fallback.ts | 2 +- src/plugins/read-file-guard-plugin.test.ts | 6 +++-- src/plugins/read-file-guard-plugin.ts | 7 ------ 4 files changed, 34 insertions(+), 10 deletions(-) diff --git a/src/plugins/bounded-grep-fallback.test.ts b/src/plugins/bounded-grep-fallback.test.ts index 543c4e2cc..6d71d42ee 100644 --- a/src/plugins/bounded-grep-fallback.test.ts +++ b/src/plugins/bounded-grep-fallback.test.ts @@ -97,6 +97,35 @@ describe("runBoundedGrep", () => { ).rejects.toThrow(); }); }); + + test("accepts regex quantifier braces and still rejects glob braces", async () => { + await withTempDir(async (dir) => { + await writeFile(join(dir, "q.txt"), "foo\nfooo\n"); + const out = await runBoundedGrep( + { pattern: "foo{1,3}", path: "." }, + new AbortController().signal, + dir, + ); + expect(out).toContain("q.txt"); + expect(out).not.toContain("brace expansion"); + + await expect( + runBoundedGrep( + { pattern: "foo", path: ".", glob: "*.{ts,txt}" }, + new AbortController().signal, + dir, + ), + ).rejects.toThrow(/brace expansion is not supported/); + + await expect( + runBoundedSearchFiles( + { pattern: "*.{ts,txt}", path: "." }, + new AbortController().signal, + dir, + ), + ).rejects.toThrow(/brace expansion is not supported/); + }); + }); }); describe("runBoundedSearchFiles", () => { diff --git a/src/plugins/bounded-grep-fallback.ts b/src/plugins/bounded-grep-fallback.ts index 8fbc00d28..c40063372 100644 --- a/src/plugins/bounded-grep-fallback.ts +++ b/src/plugins/bounded-grep-fallback.ts @@ -227,7 +227,7 @@ export async function runBoundedGrep( ): Promise { signal.throwIfAborted(); - const regex = compileUserRegex(rejectBraceExpansion(args.pattern)); + const regex = compileUserRegex(args.pattern); const maxDirectoryEntries = limits.maxDirectoryEntries ?? BOUNDED_GREP_MAX_DIRECTORY_ENTRIES; diff --git a/src/plugins/read-file-guard-plugin.test.ts b/src/plugins/read-file-guard-plugin.test.ts index 35ed33bab..3f75f12fe 100644 --- a/src/plugins/read-file-guard-plugin.test.ts +++ b/src/plugins/read-file-guard-plugin.test.ts @@ -11,12 +11,14 @@ import type { ToolResult, } from "@intx/types/runtime"; import { - READ_FILE_DEFAULT_MAX_LINES, READ_FILE_MAX_BYTES, READ_FILE_MAX_LINE_LENGTH, READ_FILE_MAX_SCAN_BYTES, - READ_FILE_MAX_TOOL_OUTPUT_BYTES, readBytesBounded, +} from "../io/bounded-read.js"; +import { + READ_FILE_DEFAULT_MAX_LINES, + READ_FILE_MAX_TOOL_OUTPUT_BYTES, readFileBounded, readFileGuardPlugin, } from "./read-file-guard-plugin.js"; diff --git a/src/plugins/read-file-guard-plugin.ts b/src/plugins/read-file-guard-plugin.ts index abfb1741d..f00c88679 100644 --- a/src/plugins/read-file-guard-plugin.ts +++ b/src/plugins/read-file-guard-plugin.ts @@ -25,13 +25,6 @@ import { readStreamBounded, } from "../io/bounded-read.js"; -export { - READ_FILE_MAX_BYTES, - READ_FILE_MAX_LINE_LENGTH, - READ_FILE_MAX_SCAN_BYTES, - readBytesBounded, -} from "../io/bounded-read.js"; - export const READ_FILE_DEFAULT_MAX_LINES = 2000; /** Refuse tool-output blobs larger than this before bounded paging. */ export const READ_FILE_MAX_TOOL_OUTPUT_BYTES = READ_FILE_MAX_SCAN_BYTES; From 80974e5935ad80053ef8335cd04cc78b7e22757c Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Wed, 14 Oct 2026 22:55:31 -0700 Subject: [PATCH 3/4] style(plugins): format shared tool helpers --- src/io/bounded-read.ts | 5 ----- src/plugins/bounded-grep-fallback.ts | 6 ++++-- src/plugins/delete-file-plugin.ts | 14 +++++++++----- src/plugins/evidence-archive-search-plugin.ts | 3 ++- src/plugins/path-escape-plugin.ts | 6 ++---- src/plugins/read-file-guard-plugin.ts | 6 +----- src/plugins/ripgrep-plugin.ts | 3 ++- src/plugins/tool-output-uri-plugin.ts | 5 ++++- src/plugins/tool-time-budget.ts | 5 +---- src/shell/spawn-tree.ts | 11 +++++++++-- 10 files changed, 34 insertions(+), 30 deletions(-) diff --git a/src/io/bounded-read.ts b/src/io/bounded-read.ts index 88b3a0131..d6b6a7a73 100644 --- a/src/io/bounded-read.ts +++ b/src/io/bounded-read.ts @@ -38,12 +38,10 @@ export interface BoundedRead { // call-keying detectors see one ranged read per window, not a same-path // loop. - function numberLine(lineNo: number, text: string): string { return `${String(lineNo).padStart(6, " ")}\t${text}`; } - /** * Streams UTF-8 from `stream`, emitting up to `limit` line-numbered lines * after skipping `offset` lines (zero-based); never decodes the full text in @@ -282,7 +280,6 @@ export function readStreamBounded( }); } - /** * Bounded read over an in-memory UTF-8 blob (tool-output spills). Feeds the * buffer in chunks so offset/limit never require a full-text split; overlong @@ -316,7 +313,6 @@ export function readBytesBounded( ); } - function continuationNotice( reason: TruncReason, firstLine: number, @@ -337,4 +333,3 @@ function continuationNotice( scanCeiling / (1024 * 1024) }MB scan limit. ${next}]`; } - diff --git a/src/plugins/bounded-grep-fallback.ts b/src/plugins/bounded-grep-fallback.ts index c40063372..8761eb742 100644 --- a/src/plugins/bounded-grep-fallback.ts +++ b/src/plugins/bounded-grep-fallback.ts @@ -19,7 +19,6 @@ function rejectBraceExpansion(pattern: string): string { return pattern; } - /** Max file paths considered per search (directory walk). */ export const BOUNDED_GREP_MAX_DIRECTORY_ENTRIES = 25_000; @@ -237,7 +236,10 @@ export async function runBoundedGrep( const basePath = resolve(baseCwd, args.path ?? "."); const contextLines = args.context ?? 0; const maxResults = args.max_results ?? BOUNDED_GREP_DEFAULT_MAX_RESULTS; - const globFilter = args.glob !== undefined ? globToRegExp(rejectBraceExpansion(args.glob)) : null; + const globFilter = + args.glob !== undefined + ? globToRegExp(rejectBraceExpansion(args.glob)) + : null; let info; try { diff --git a/src/plugins/delete-file-plugin.ts b/src/plugins/delete-file-plugin.ts index 158d4f755..f722f9190 100644 --- a/src/plugins/delete-file-plugin.ts +++ b/src/plugins/delete-file-plugin.ts @@ -52,7 +52,10 @@ export function deleteFilePlugin( handler: async (call: ToolCall): Promise => { const args = DeleteFileArgs(call.arguments); if (args instanceof type.errors) { - return denyResult(call.id, "Failed to delete file: delete_file requires a non-empty path"); + return denyResult( + call.id, + "Failed to delete file: delete_file requires a non-empty path", + ); } const allowOutside = resolveAllowOutside(options.allowOutside); @@ -116,7 +119,10 @@ export function deleteFilePlugin( content: `File already absent: ${args.path} (no action needed)`, }; } - return denyResult(call.id, `Failed to delete file: ${failureDetail(error)}`); + return denyResult( + call.id, + `Failed to delete file: ${failureDetail(error)}`, + ); } }, }; @@ -127,9 +133,7 @@ export function deleteFilePlugin( function failureDetail(error: unknown): string { if (error instanceof Error) { const code = - hasCode(error) && typeof error.code === "string" - ? error.code - : undefined; + hasCode(error) && typeof error.code === "string" ? error.code : undefined; return code === undefined ? error.message : `${code}: ${error.message}`; } return String(error); diff --git a/src/plugins/evidence-archive-search-plugin.ts b/src/plugins/evidence-archive-search-plugin.ts index 14e477f45..1cca83891 100644 --- a/src/plugins/evidence-archive-search-plugin.ts +++ b/src/plugins/evidence-archive-search-plugin.ts @@ -172,7 +172,8 @@ export function evidenceArchiveSearchPlugin( if (pattern === undefined) { return denyResult(call.id, "Error: grep requires pattern (string)."); } - const maxResults = optionalNumber(call.arguments.max_results) ?? GREP_DEFAULT_MAX; + const maxResults = + optionalNumber(call.arguments.max_results) ?? GREP_DEFAULT_MAX; const glob = optionalString(call.arguments.glob); const contextArg = optionalNumber(call.arguments.context); const context = diff --git a/src/plugins/path-escape-plugin.ts b/src/plugins/path-escape-plugin.ts index b67ac721b..a02a7bcf9 100644 --- a/src/plugins/path-escape-plugin.ts +++ b/src/plugins/path-escape-plugin.ts @@ -84,10 +84,8 @@ function escapeArgs( // pathEscapeBlockReason computes, so a surviving call is exactly one the // block-reason leg would have passed. const roots = rootsForEscape(toolName, rootsProvider, trustedPluginRoots); - return walkPathArgs( - args, - undefined, - (value) => sanitizePath(value, cwd, roots, allowOutside, toolName), + return walkPathArgs(args, undefined, (value) => + sanitizePath(value, cwd, roots, allowOutside, toolName), ) as Record; } diff --git a/src/plugins/read-file-guard-plugin.ts b/src/plugins/read-file-guard-plugin.ts index f00c88679..f225368e2 100644 --- a/src/plugins/read-file-guard-plugin.ts +++ b/src/plugins/read-file-guard-plugin.ts @@ -7,11 +7,7 @@ import { canonicalToolOutputUri, isToolOutputLike, } from "../util/tool-output-uri.js"; -import { - denyResult, - errorMessage, - optionalNumber, -} from "./tool-args.js"; +import { denyResult, errorMessage, optionalNumber } from "./tool-args.js"; import { diagnoseBlockedFileInspection, inspectionKindFromFirstChunk, diff --git a/src/plugins/ripgrep-plugin.ts b/src/plugins/ripgrep-plugin.ts index e0c0085be..f2e11bf4c 100644 --- a/src/plugins/ripgrep-plugin.ts +++ b/src/plugins/ripgrep-plugin.ts @@ -161,7 +161,8 @@ export function ripgrepPlugin( const path = optionalString(call.arguments.path) ?? cwd; const context = optionalNumber(call.arguments.context) ?? 0; const glob = optionalString(call.arguments.glob); - const maxResults = optionalNumber(call.arguments.max_results) ?? DEFAULT_GREP_MAX; + const maxResults = + optionalNumber(call.arguments.max_results) ?? DEFAULT_GREP_MAX; const { cwd: rgCwd, target } = searchLocation(path, cwd); const rgArgs = ["--line-number", "--no-heading", "--color", "never"]; diff --git a/src/plugins/tool-output-uri-plugin.ts b/src/plugins/tool-output-uri-plugin.ts index 537332457..567574e60 100644 --- a/src/plugins/tool-output-uri-plugin.ts +++ b/src/plugins/tool-output-uri-plugin.ts @@ -1,5 +1,8 @@ import type { ToolPlugin } from "@intx/tools-posix"; -import { isToolOutputLike, normalizeToolOutputUri } from "../util/tool-output-uri.js"; +import { + isToolOutputLike, + normalizeToolOutputUri, +} from "../util/tool-output-uri.js"; import { virtualRefVerdict } from "./path-escape-plugin.js"; import { denyResult } from "./tool-args.js"; diff --git a/src/plugins/tool-time-budget.ts b/src/plugins/tool-time-budget.ts index ccbb0821d..124760c28 100644 --- a/src/plugins/tool-time-budget.ts +++ b/src/plugins/tool-time-budget.ts @@ -13,10 +13,7 @@ const NOT_A_NO_MATCH_TAIL = 'This is not the same as "no matches".'; // Timeout notices carry whatever partial output already streamed so the agent // never loses the bytes it did get; empty partials attach the bare notice. -function withPartialResult( - notice: string, - partialResult?: string, -): string { +function withPartialResult(notice: string, partialResult?: string): string { const trimmed = partialResult?.trim(); if (trimmed === undefined || trimmed.length === 0) return notice; return `${trimmed}\n\n${notice}`; diff --git a/src/shell/spawn-tree.ts b/src/shell/spawn-tree.ts index 1ae5e44f7..a37a2a152 100644 --- a/src/shell/spawn-tree.ts +++ b/src/shell/spawn-tree.ts @@ -1,4 +1,8 @@ -import { spawn, type ChildProcess, type SpawnOptions } from "node:child_process"; +import { + spawn, + type ChildProcess, + type SpawnOptions, +} from "node:child_process"; // One process-group-kill policy for every spawned tool subprocess: the child // is spawned detached so it becomes a group leader, and the kill reaches the @@ -33,7 +37,10 @@ export function killProcessTree(child: TreeChild): void { } /** Spawn options making the child a process-group leader (POSIX only). */ -export function processGroupLeaderSpawnOptions(): Pick { +export function processGroupLeaderSpawnOptions(): Pick< + SpawnOptions, + "detached" +> { return { detached: process.platform !== "win32" }; } From abc7db75957e5d6145489ebb8af68ed62dd7fd97 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Wed, 14 Oct 2026 23:12:23 -0700 Subject: [PATCH 4/4] fix(shell): drop unused spawnProcessGroupLeader export The shared spawn helper had no callers, so the dead-export gate failed static analysis. --- src/shell/spawn-tree.ts | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/src/shell/spawn-tree.ts b/src/shell/spawn-tree.ts index a37a2a152..da23f37c3 100644 --- a/src/shell/spawn-tree.ts +++ b/src/shell/spawn-tree.ts @@ -1,8 +1,4 @@ -import { - spawn, - type ChildProcess, - type SpawnOptions, -} from "node:child_process"; +import type { SpawnOptions } from "node:child_process"; // One process-group-kill policy for every spawned tool subprocess: the child // is spawned detached so it becomes a group leader, and the kill reaches the @@ -43,14 +39,3 @@ export function processGroupLeaderSpawnOptions(): Pick< > { return { detached: process.platform !== "win32" }; } - -export function spawnProcessGroupLeader( - command: string, - args: readonly string[], - options: SpawnOptions = {}, -): ChildProcess { - return spawn(command, [...args], { - ...processGroupLeaderSpawnOptions(), - ...options, - }); -}