From 9f14133e70785b4b1f9265e22090d52e6c538c39 Mon Sep 17 00:00:00 2001 From: Valera Brizhatiuk <19537764+plombeer31@users.noreply.github.com> Date: Mon, 14 Sep 2026 23:27:23 +0300 Subject: [PATCH 1/8] fix(tools): F33 argument errors show the keys received and expected MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A tool's argument error named the key it wanted and nothing else, so a model that had sent `patternes`, `"path"` (quotes included) or a key fused with prompt markup retried the same call blind. - batch-executor: an error a tool throws on its arguments now ends with `received keys: …; expected: …` (expected from the tool's args schema) and `did you mean X instead of Y?` for any received key within two edits of an accepted one. Keys only, never values. Runtime failures (ENOENT, timeouts) get no key report. - coerce-tool-args: before dispatch, a quoted key (`"path"`, `\"path\"`) and a fused label fragment (`,limit`) are renamed to the schema key they were meant to be — only when the clean key is in the schema and the model did not also send it. --- src/agent/batch-executor.test.ts | 54 ++++++++++++ src/agent/batch-executor.ts | 22 ++++- src/tools/argument-error-hint.test.ts | 92 ++++++++++++++++++++ src/tools/argument-error-hint.ts | 121 ++++++++++++++++++++++++++ src/tools/coerce-tool-args.test.ts | 57 +++++++++++- src/tools/coerce-tool-args.ts | 52 ++++++++++- 6 files changed, 392 insertions(+), 6 deletions(-) create mode 100644 src/tools/argument-error-hint.test.ts create mode 100644 src/tools/argument-error-hint.ts diff --git a/src/agent/batch-executor.test.ts b/src/agent/batch-executor.test.ts index c2f93a7a..89c08400 100644 --- a/src/agent/batch-executor.test.ts +++ b/src/agent/batch-executor.test.ts @@ -320,6 +320,60 @@ describe("executeBatch", () => { expect(out.cancelled).toBe(false); }); + it("appends the received and expected keys to a thrown argument error (F33)", async () => { + const registry = new ToolRegistry(); + registry.register({ + name: "os.fs.read", + description: "r", + readonly: true, + run: async () => { + throw new Error("os.fs.read: `path` must be a non-empty string"); + }, + }); + const inputs = toBatchInputs([ + { tool: "os.fs.read", args: { patth: "secret-value.txt" } }, + ]); + const out = await executeBatch( + inputs, + registry, + ctx(new AbortController().signal), + ); + const result = out.results[0]!.compressed!; + expect(result.status).toBe("error"); + expect(result.summary).toContain( + "os.fs.read: `path` must be a non-empty string — received keys: patth; expected: path, maxBytes, offset, limit, lineNumbers; did you mean `path` instead of `patth`?", + ); + expect(result.summary).not.toContain("secret-value"); + expect(result.details.receivedKeys).toEqual(["patth"]); + expect(result.details.expectedKeys).toEqual([ + "path", + "maxBytes", + "offset", + "limit", + "lineNumbers", + ]); + }); + + it("leaves a thrown runtime error without a key report", async () => { + const registry = new ToolRegistry(); + registry.register({ + name: "os.fs.read", + description: "r", + readonly: true, + run: async () => { + throw new Error("ENOENT: no such file or directory, open 'a'"); + }, + }); + const out = await executeBatch( + toBatchInputs([{ tool: "os.fs.read", args: { path: "a" } }]), + registry, + ctx(new AbortController().signal), + ); + const result = out.results[0]!.compressed!; + expect(result.summary).toBe("ENOENT: no such file or directory, open 'a'"); + expect(result.details.receivedKeys).toBeUndefined(); + }); + it("preserves batch-index order in the returned slots", async () => { const registry = new ToolRegistry(); registry.register({ diff --git a/src/agent/batch-executor.ts b/src/agent/batch-executor.ts index 996ff3b1..e5e00845 100644 --- a/src/agent/batch-executor.ts +++ b/src/agent/batch-executor.ts @@ -11,6 +11,7 @@ import { } from "../compressor/result-compressor.js"; import type { ToolRegistry } from "../tools/tool-registry.js"; import type { ToolRole } from "../tools/tool-roles.js"; +import { describeArgumentError } from "../tools/argument-error-hint.js"; import { CancelledError } from "../llm/index.js"; import { isParallelWithinGroup, @@ -436,11 +437,28 @@ export async function executeBatch( ); } const cause = err instanceof Error ? err : new Error(String(err)); + // An argument error names the key the tool wanted; the model also + // needs the keys it actually sent (`patternes`, `"path"`) and the + // closest accepted one, or it retries the same call blind. Keys + // only — never values. + const hint = describeArgumentError({ + tool: input.call.tool, + args: input.call.args, + message: cause.message, + }); compressed = compressToolResult({ tool: input.call.tool, status: "error", - output: cause.message, - details: { errorName: cause.name }, + output: hint?.message ?? cause.message, + details: { + errorName: cause.name, + ...(hint !== null + ? { + receivedKeys: hint.receivedKeys, + expectedKeys: hint.expectedKeys, + } + : {}), + }, }); } const durationMs = Date.now() - startedAt; diff --git a/src/tools/argument-error-hint.test.ts b/src/tools/argument-error-hint.test.ts new file mode 100644 index 00000000..60e088b1 --- /dev/null +++ b/src/tools/argument-error-hint.test.ts @@ -0,0 +1,92 @@ +import { describe, expect, it } from "vitest"; +import { describeArgumentError, nearestKey } from "./argument-error-hint.js"; + +describe("describeArgumentError", () => { + it("appends the received keys, the schema keys and the nearest key", () => { + // A local worker sent `patternes` to os.fs.grep. + const hint = describeArgumentError({ + tool: "os.fs.grep", + args: { patternes: ".add(", path: "js" }, + message: "os.fs.grep: `pattern` must be a non-empty string", + }); + expect(hint).not.toBeNull(); + expect(hint!.receivedKeys).toEqual(["patternes", "path"]); + expect(hint!.expectedKeys[0]).toBe("pattern"); + expect(hint!.expectedKeys).toContain("glob"); + expect(hint!.nearest).toEqual([ + { received: "patternes", expected: "pattern" }, + ]); + expect(hint!.message).toBe( + "os.fs.grep: `pattern` must be a non-empty string — " + + `received keys: patternes, path; expected: ${hint!.expectedKeys.join(", ")}; ` + + "did you mean `pattern` instead of `patternes`?", + ); + }); + + it("echoes keys only, never values", () => { + const hint = describeArgumentError({ + tool: "os.fs.write", + args: { pth: "a.js", content: "SECRET CONTENT" }, + message: "os.fs.write: `path` must be a non-empty string", + }); + expect(hint!.message).not.toContain("SECRET"); + expect(hint!.message).toContain("received keys: pth, content"); + expect(hint!.message).toContain("did you mean `path` instead of `pth`?"); + }); + + it("reports no expected keys for a tool without a schema", () => { + const hint = describeArgumentError({ + tool: "mcp.some.server.tool", + args: { q: 1 }, + message: "`query` is required", + }); + expect(hint!.expectedKeys).toEqual([]); + expect(hint!.nearest).toEqual([]); + expect(hint!.message).toBe("`query` is required — received keys: q"); + }); + + it("says (none) when no keys arrived", () => { + const hint = describeArgumentError({ + tool: "os.fs.read", + args: {}, + message: "os.fs.read: `path` must be a non-empty string", + }); + expect(hint!.message).toContain("received keys: (none)"); + }); + + it("leaves a runtime failure alone", () => { + expect( + describeArgumentError({ + tool: "os.fs.read", + args: { path: "nope.txt" }, + message: "ENOENT: no such file or directory, open 'nope.txt'", + }), + ).toBeNull(); + expect( + describeArgumentError({ + tool: "os.fs.grep", + args: { pattern: "x", path: "/gone" }, + message: "os.fs.grep: path does not exist: /gone", + }), + ).toBeNull(); + }); + + it("does not suggest a key that is further than two edits away", () => { + const hint = describeArgumentError({ + tool: "os.fs.read", + args: { filename: "a" }, + message: "os.fs.read: `path` must be a non-empty string", + }); + expect(hint!.nearest).toEqual([]); + expect(hint!.message).not.toContain("did you mean"); + }); +}); + +describe("nearestKey", () => { + it("matches case-insensitively within two edits, first in schema order on a tie", () => { + expect(nearestKey("Path", ["path", "offset"])).toBe("path"); + expect(nearestKey("limits", ["path", "limit"])).toBe("limit"); + expect(nearestKey("ab", ["abc", "abd"])).toBe("abc"); + expect(nearestKey("content", ["path", "offset"])).toBeNull(); + }); +}); diff --git a/src/tools/argument-error-hint.ts b/src/tools/argument-error-hint.ts new file mode 100644 index 00000000..c8b29ef5 --- /dev/null +++ b/src/tools/argument-error-hint.ts @@ -0,0 +1,121 @@ +import { getDefaultArgsJsonSchema } from "../prompt/default-tool-args-schemas.js"; + +/** + * What a tool's argument error should also say: which keys arrived, + * which the tool accepts, and the closest accepted key for any that it + * does not. + * + * A worker once sent `patternes` to `os.fs.grep`, another sent the keys + * `"\"path\""` and `",limit"` to `os.fs.read`. Each got + * "`path` must be a non-empty string" back — true, and useless: the + * message names the missing key but not the key the model actually + * used, so it retried the same call blind. The keys are echoed, never + * the values: a value can be a whole file. + */ +export interface ArgumentErrorHint { + /** The original message with the key report appended. */ + readonly message: string; + readonly receivedKeys: readonly string[]; + /** Empty when the tool has no registered args schema (MCP tools). */ + readonly expectedKeys: readonly string[]; + /** Received keys the tool does not accept, with their closest accepted key. */ + readonly nearest: ReadonlyArray<{ received: string; expected: string }>; +} + +/** Largest edit distance at which a received key still "means" an expected one. */ +export const NEAREST_KEY_MAX_DISTANCE = 2; + +/** + * The wording tools use when they reject their arguments. A backticked + * identifier is the strongest signal (every built-in names the field + * that way); the phrases cover the few that do not. + */ +const ARGUMENT_ERROR_WORDING = + /`[^`]+`|\bmust be\b|\bis required\b|\brequired\b|\bprovide (?:either|a|an|the|one)\b|\bmissing\b|\bunknown (?:arg|argument|field|key|option)\b|\bnot allowed\b/i; + +/** + * The hint for an error a tool threw on its arguments, or `null` when + * the message does not read as an argument error — a runtime failure + * (ENOENT, a timeout, an approval refusal) gets no key report, which + * would only be noise there. + */ +export function describeArgumentError(input: { + tool: string; + args: Record; + message: string; +}): ArgumentErrorHint | null { + if (!ARGUMENT_ERROR_WORDING.test(input.message)) return null; + const receivedKeys = Object.keys(input.args); + const expectedKeys = expectedKeysFor(input.tool); + const expectedSet = new Set(expectedKeys); + const nearest: Array<{ received: string; expected: string }> = []; + for (const received of receivedKeys) { + if (expectedSet.has(received)) continue; + const match = nearestKey(received, expectedKeys); + if (match !== null) nearest.push({ received, expected: match }); + } + const parts = [ + `received keys: ${receivedKeys.length > 0 ? receivedKeys.join(", ") : "(none)"}`, + ]; + if (expectedKeys.length > 0) parts.push(`expected: ${expectedKeys.join(", ")}`); + for (const { received, expected } of nearest) { + parts.push(`did you mean \`${expected}\` instead of \`${received}\`?`); + } + return { + message: `${input.message} — ${parts.join("; ")}`, + receivedKeys, + expectedKeys, + nearest, + }; +} + +function expectedKeysFor(tool: string): string[] { + const schema = getDefaultArgsJsonSchema(tool); + const properties = schema?.properties; + if ( + properties === null || + typeof properties !== "object" || + Array.isArray(properties) + ) { + return []; + } + return Object.keys(properties as Record); +} + +/** + * The expected key closest to `received` within + * `NEAREST_KEY_MAX_DISTANCE`, comparing case-insensitively; ties go to + * the first in schema order. + */ +export function nearestKey( + received: string, + expected: readonly string[], +): string | null { + let best: { key: string; distance: number } | null = null; + const needle = received.toLowerCase(); + for (const key of expected) { + const distance = editDistance(needle, key.toLowerCase()); + if (distance > NEAREST_KEY_MAX_DISTANCE) continue; + if (best === null || distance < best.distance) best = { key, distance }; + } + return best?.key ?? null; +} + +/** Levenshtein distance; keys are short, so the plain two-row form is enough. */ +function editDistance(a: string, b: string): number { + if (a === b) return 0; + if (a.length === 0) return b.length; + if (b.length === 0) return a.length; + let previous = Array.from({ length: b.length + 1 }, (_, i) => i); + for (let i = 1; i <= a.length; i += 1) { + const current = [i]; + for (let j = 1; j <= b.length; j += 1) { + const substitution = previous[j - 1]! + (a[i - 1] === b[j - 1] ? 0 : 1); + current.push( + Math.min(previous[j]! + 1, current[j - 1]! + 1, substitution), + ); + } + previous = current; + } + return previous[b.length]!; +} diff --git a/src/tools/coerce-tool-args.test.ts b/src/tools/coerce-tool-args.test.ts index 568e82dc..f786a67f 100644 --- a/src/tools/coerce-tool-args.test.ts +++ b/src/tools/coerce-tool-args.test.ts @@ -4,7 +4,7 @@ import { type ToolContext, type ToolDefinition, } from "./tool-registry.js"; -import { coerceToolArgs } from "./coerce-tool-args.js"; +import { coerceToolArgs, normalizeKey } from "./coerce-tool-args.js"; const ctx: ToolContext = { workingDir: "/w", @@ -243,3 +243,58 @@ describe("ToolRegistry.invoke integration", () => { ).rejects.toThrow(/tool not registered/); }); }); + +describe("coerceToolArgs — mangled keys (F33)", () => { + it("unquotes a key wrapped in its own quotes", async () => { + // A cloud worker sent `"path"` (quotes included) to os.fs.read. + const seen = await invokeWith("os.fs.read", { '"path"': "a.txt" }); + expect(seen).toEqual({ path: "a.txt" }); + }); + + it("keeps the real key of a fused label fragment", async () => { + const seen = await invokeWith("os.fs.read", { + path: "a.txt", + ",limit": 20, + }); + expect(seen).toEqual({ path: "a.txt", limit: 20 }); + }); + + it("still coerces the value under a repaired key", async () => { + const seen = await invokeWith("os.fs.read", { + path: "a.txt", + "'limit'": "20", + }); + expect(seen).toEqual({ path: "a.txt", limit: 20 }); + }); + + it("never overwrites a key the model also sent cleanly", async () => { + const args = { path: "a.txt", '"path"': "b.txt" }; + expect(await invokeWith("os.fs.read", args)).toEqual(args); + }); + + it("leaves a key alone when its cleaned form is not in the schema", async () => { + const args = { path: "a.txt", '"nope"': 1 }; + expect(await invokeWith("os.fs.read", args)).toEqual(args); + }); + + it("returns the same object when nothing needed repair", () => { + const args = { path: "a.txt", limit: 3 }; + expect(coerceToolArgs("os.fs.read", args)).toBe(args); + }); +}); + +describe("normalizeKey", () => { + it.each([ + ['"path"', "path"], + ["'path'", "path"], + ["`path`", "path"], + ['"\\"path\\""', "path"], + [",limit", "limit"], + [" offset ", "offset"], + ["path,", null], + ["a b", null], + ["", null], + ])("%j → %j", (raw, expected) => { + expect(normalizeKey(raw)).toBe(expected); + }); +}); diff --git a/src/tools/coerce-tool-args.ts b/src/tools/coerce-tool-args.ts index 6a9abdff..e400140d 100644 --- a/src/tools/coerce-tool-args.ts +++ b/src/tools/coerce-tool-args.ts @@ -28,8 +28,9 @@ export function coerceToolArgs( const properties = argsProperties(name); if (!properties) return args; + const normalised = normalizeArgKeys(args, properties); let coerced: Record | null = null; - for (const [key, value] of Object.entries(args)) { + for (const [key, value] of Object.entries(normalised)) { if (typeof value !== "string") continue; const schema = asSchema(properties[key]); if (!schema) continue; @@ -37,10 +38,55 @@ export function coerceToolArgs( const candidate = tryCoerce(value, schema); if (candidate === undefined) continue; - coerced ??= { ...args }; + coerced ??= { ...normalised }; coerced[key] = candidate; } - return coerced ?? args; + return coerced ?? normalised; +} + +/** + * Repairs argument *keys* that arrived mangled, the way models mangle + * them: a key wrapped in its own quotes (`"\"path\""`), and a key fused + * with a fragment of the prompt's markup (`,limit`) — + * both seen from a cloud worker, both rejected as "`path` must be a + * non-empty string" while the value sat under the mangled key. + * + * Do-no-harm again: a key is renamed only when it is not itself in the + * schema, its cleaned form is, and the model did not also send the + * clean key. Anything else is left for the tool to report. + */ +function normalizeArgKeys( + args: Record, + properties: Schema, +): Record { + let fixed: Record | null = null; + for (const [key, value] of Object.entries(args)) { + if (Object.hasOwn(properties, key)) continue; + const clean = normalizeKey(key); + if (clean === null || clean === key) continue; + if (!Object.hasOwn(properties, clean) || Object.hasOwn(args, clean)) { + continue; + } + fixed ??= { ...args }; + delete fixed[key]; + fixed[clean] = value; + } + return fixed ?? args; +} + +const IDENTIFIER = /^[A-Za-z_$][\w$]*$/; + +/** The identifier a mangled key was meant to be, or null when there is none. */ +export function normalizeKey(key: string): string | null { + let clean = key.trim(); + // A fused fragment ends with the real key after the last comma. + const comma = clean.lastIndexOf(","); + if (comma !== -1) clean = clean.slice(comma + 1).trim(); + // Markup that leaked in from the prompt. + clean = clean.replace(/<[^<>]*>/g, "").trim(); + // Quotes of the model's own JSON, one level or several, escaped or not. + clean = clean.replace(/^(?:\\?["'`])+|(?:\\?["'`])+$/g, "").trim(); + return IDENTIFIER.test(clean) ? clean : null; } /** From e4a353585fdba327708cf20758262b424b072564 Mon Sep 17 00:00:00 2001 From: Valera Brizhatiuk <19537764+plombeer31@users.noreply.github.com> Date: Mon, 14 Sep 2026 23:28:30 +0300 Subject: [PATCH 2/8] feat(tools): F34 literal search in os.fs.grep `os.fs.grep` always ran the pattern as a regex, so a code fragment such as `.add(` failed with a raw ripgrep "regex parse error: unclosed group" and the model had to guess at escaping. - `literal?: boolean` maps to `rg -F` (fixed-string search); added to the descriptor's argsSchema and the native-tools JSON schema. - A ripgrep regex parse error now ends with a hint naming `literal: true` (also in `details.hint`); nothing is added when literal was already set or the failure is something else. --- src/prompt/default-tool-args-schemas.ts | 1 + src/prompt/default-tool-descriptors-a.ts | 2 +- src/tools/os/fs-grep.test.ts | 68 +++++++++++++++++++++++- src/tools/os/fs-grep.ts | 27 ++++++++-- 4 files changed, 92 insertions(+), 6 deletions(-) diff --git a/src/prompt/default-tool-args-schemas.ts b/src/prompt/default-tool-args-schemas.ts index 7806a0bf..b65f3b20 100644 --- a/src/prompt/default-tool-args-schemas.ts +++ b/src/prompt/default-tool-args-schemas.ts @@ -206,6 +206,7 @@ const DEFAULT_TOOL_ARGS_SCHEMAS: ReadonlyMap = new Map< anyOf: [stringSchema, stringArraySchema], }, type: stringSchema, + literal: booleanSchema, caseInsensitive: booleanSchema, multiline: booleanSchema, outputMode: { diff --git a/src/prompt/default-tool-descriptors-a.ts b/src/prompt/default-tool-descriptors-a.ts index 8b91c1a2..52657ec2 100644 --- a/src/prompt/default-tool-descriptors-a.ts +++ b/src/prompt/default-tool-descriptors-a.ts @@ -91,7 +91,7 @@ export const DEFAULT_TOOL_DESCRIPTORS_A: readonly ToolDescriptor[] = [ summary: "Regex ripgrep for text search (content, files_with_matches, count). Best on source/text trees. Avoid tree-wide runs with glob *.pdf (or similar) over huge dirs—slow, binary-heavy, often flaky; prefer os.fs.glob by filename + os.fs.read_document on a small candidate set.", argsSchema: - "{ pattern: string, path?: string, glob?: string | string[], type?: string, caseInsensitive?: boolean, multiline?: boolean, outputMode?: 'content' | 'files_with_matches' | 'count', contextBefore?: number, contextAfter?: number, contextAround?: number, headLimit?: number, offset?: number, showLineNumbers?: boolean, timeoutMs?: number }", + "{ pattern: string, path?: string, glob?: string | string[], type?: string, literal?: boolean, caseInsensitive?: boolean, multiline?: boolean, outputMode?: 'content' | 'files_with_matches' | 'count', contextBefore?: number, contextAfter?: number, contextAround?: number, headLimit?: number, offset?: number, showLineNumbers?: boolean, timeoutMs?: number }", }, { name: "os.fs.edit", diff --git a/src/tools/os/fs-grep.test.ts b/src/tools/os/fs-grep.test.ts index 6d5bd7b1..196cfe92 100644 --- a/src/tools/os/fs-grep.test.ts +++ b/src/tools/os/fs-grep.test.ts @@ -7,7 +7,7 @@ import type { CommandResult, } from "../../sandbox/command-runner.js"; import type { ToolContext } from "../tool-registry.js"; -import { buildOsFsGrepTool, parseRipgrepJson } from "./fs-grep.js"; +import { buildOsFsGrepTool, LITERAL_HINT, parseRipgrepJson } from "./fs-grep.js"; // The grep runner is mocked in these tests, but the tool now stats the // requested path to decide the child process cwd, so the fixture must exist @@ -406,3 +406,69 @@ describe("os.fs.grep", () => { expect(ran).toBe(false); }); }); + +describe("os.fs.grep literal search (F34)", () => { + it("passes -F to ripgrep when literal is set", async () => { + let capturedArgs: string[] = []; + const tool = buildOsFsGrepTool({ + resolveRgPath: () => "/fake/rg", + runCommand: async (_cmd, args) => { + capturedArgs = args; + return makeCommandResult({ stdout: "" }); + }, + }); + await tool.run({ pattern: ".add(", literal: true }, makeCtx()); + expect(capturedArgs).toContain("-F"); + expect(capturedArgs.slice(-2)).toEqual([".add(", "."]); + }); + + it("does not pass -F by default", async () => { + let capturedArgs: string[] = []; + const tool = buildOsFsGrepTool({ + resolveRgPath: () => "/fake/rg", + runCommand: async (_cmd, args) => { + capturedArgs = args; + return makeCommandResult({ stdout: "" }); + }, + }); + await tool.run({ pattern: "foo" }, makeCtx()); + expect(capturedArgs).not.toContain("-F"); + }); + + it("suggests literal: true on a ripgrep regex parse error", async () => { + // What `.add(` produced live: an unclosed group. + const stderr = + "regex parse error:\n .add(\n ^\nerror: unclosed group\n"; + const tool = buildOsFsGrepTool({ + resolveRgPath: () => "/fake/rg", + runCommand: async () => makeCommandResult({ exitCode: 2, stderr }), + }); + const result = await tool.run({ pattern: ".add(" }, makeCtx()); + expect(result.status).toBe("error"); + expect(result.summary).toContain("unclosed group"); + expect(result.summary).toContain(`hint: ${LITERAL_HINT}`); + expect(result.details.hint).toBe(LITERAL_HINT); + }); + + it("adds no literal hint to other ripgrep failures, or when literal is already set", async () => { + const other = buildOsFsGrepTool({ + resolveRgPath: () => "/fake/rg", + runCommand: async () => + makeCommandResult({ exitCode: 2, stderr: "rg: some.file: Permission denied" }), + }); + const otherResult = await other.run({ pattern: "x" }, makeCtx()); + expect(otherResult.summary).not.toContain("literal"); + expect(otherResult.details.hint).toBeUndefined(); + + const literal = buildOsFsGrepTool({ + resolveRgPath: () => "/fake/rg", + runCommand: async () => + makeCommandResult({ exitCode: 2, stderr: "regex parse error: x" }), + }); + const literalResult = await literal.run( + { pattern: "x", literal: true }, + makeCtx(), + ); + expect(literalResult.summary).not.toContain("hint:"); + }); +}); diff --git a/src/tools/os/fs-grep.ts b/src/tools/os/fs-grep.ts index e4b46055..271f3a0a 100644 --- a/src/tools/os/fs-grep.ts +++ b/src/tools/os/fs-grep.ts @@ -26,6 +26,8 @@ interface GrepArgs { type: string | undefined; caseInsensitive: boolean; multiline: boolean; + /** Search for the pattern as a fixed string (`rg -F`), not a regex. */ + literal: boolean; outputMode: GrepOutputMode; contextBefore: number; contextAfter: number; @@ -51,6 +53,16 @@ interface RgFileRecord { const DEFAULT_TIMEOUT_MS = 30_000; const DEFAULT_HEAD_LIMIT = 200; +/** + * ripgrep's wording when the pattern is not a valid regex — `.add(`, + * `foo[`, `a**`. Models reach for grep with a code fragment more often + * than with a regex, so the error names the flag that searches for the + * fragment as written. + */ +const REGEX_PARSE_ERROR = /regex parse error/i; +export const LITERAL_HINT = + "the pattern was parsed as a regex; to search for it as a fixed string, set `literal: true`"; + export function buildOsFsGrepTool( deps: Partial = {}, ): ToolDefinition { @@ -60,7 +72,7 @@ export function buildOsFsGrepTool( return { name: "os.fs.grep", description: - "Fast regex search across files using bundled ripgrep. Supports three output modes (`content`, `files_with_matches`, `count`), glob filtering, file-type filtering, multiline mode, context lines, and pagination. Read-only.", + "Fast regex search across files using bundled ripgrep (`literal: true` searches for the pattern as a fixed string). Supports three output modes (`content`, `files_with_matches`, `count`), glob filtering, file-type filtering, multiline mode, context lines, and pagination. Read-only.", readonly: true, async run(rawArgs, ctx) { const args = parseArgs(rawArgs, ctx.workingDir); @@ -110,18 +122,22 @@ export function buildOsFsGrepTool( }); } if (result.exitCode !== 0) { + const stderr = result.stderr.trim(); const reason = - result.stderr.trim().length > 0 - ? result.stderr.trim() + stderr.length > 0 + ? stderr : `ripgrep exited with code ${result.exitCode}`; + const hint = + !args.literal && REGEX_PARSE_ERROR.test(stderr) ? LITERAL_HINT : null; return compressToolResult({ tool: "os.fs.grep", status: "error", - output: reason, + output: hint === null ? reason : `${reason}\nhint: ${hint}`, details: { exitCode: result.exitCode, command: [rgPath, ...rgArgs], path: args.path, + ...(hint === null ? {} : { hint }), }, }); } @@ -160,6 +176,7 @@ function parseArgs( : undefined; const caseInsensitive = rawArgs.caseInsensitive === true; const multiline = rawArgs.multiline === true; + const literal = rawArgs.literal === true; const modeRaw = rawArgs.outputMode; const outputMode: GrepOutputMode = modeRaw === "files_with_matches" || modeRaw === "count" @@ -200,6 +217,7 @@ function parseArgs( type, caseInsensitive, multiline, + literal, outputMode, contextBefore, contextAfter, @@ -255,6 +273,7 @@ async function resolveSearchTarget( function buildRgArgs(args: GrepArgs, searchTarget: string): string[] { const rg: string[] = ["--json"]; if (args.caseInsensitive) rg.push("-i"); + if (args.literal) rg.push("-F"); if (args.multiline) { rg.push("-U"); rg.push("--multiline-dotall"); From 6e03b113c2ba1d3f87f05678aee5e8f788c18a8e Mon Sep 17 00:00:00 2001 From: Valera Brizhatiuk <19537764+plombeer31@users.noreply.github.com> Date: Mon, 14 Sep 2026 23:35:00 +0300 Subject: [PATCH 3/8] feat(tools): F24 write-time warnings for HTML, transcript markup and double-escaping Patch 4's parse check knew `.js`/`.json` only. A local model then wrote an `index.html` with 16,627 bytes of serialized tool-call text after ``, and a `game.js` whose newlines were all literal `\n`; both were found many steps later. New `fs-content-check.ts`, warn-only like the parse check, composed with it through one entry point (`checkChangedFile`) that `os.fs.write`, `os.fs.edit` and `os.fs.patch` now call: - `.html`/`.htm`: every inline classic `', + '', + '', + "", + "", + "", +].join("\n"); + +/** The same page with the inline script closed: nothing to warn about. */ +const PAGE_OK = PAGE.replace(" },\n", " },\n };\n"); + +describe("checkWrittenContent — HTML", () => { + it("parses inline classic scripts and reports the line in the file", () => { + const warnings = checkWrittenContent("/w/index.html", PAGE); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toMatchObject({ kind: "html_script", key: "script4" }); + // The block opens on line 8; the unclosed object is reported at its + // end, line 13 in the file — not line 6 inside the block. + expect(warnings[0]!.message).toBe( + "inline