From 011652f6492926d529a522283d819bb2a3339acf Mon Sep 17 00:00:00 2001 From: Christoffer Niska Date: Wed, 25 Mar 2026 11:10:21 +0200 Subject: [PATCH 1/8] refactor(tools): remove partial read support from read-file --- src/file-ops.ts | 19 +++------- src/file-toolkit.ts | 87 +++++++++++++++++---------------------------- src/tool-guards.ts | 69 ++++------------------------------- 3 files changed, 43 insertions(+), 132 deletions(-) diff --git a/src/file-ops.ts b/src/file-ops.ts index 960ffd07..e3049344 100644 --- a/src/file-ops.ts +++ b/src/file-ops.ts @@ -15,7 +15,6 @@ import { ensurePathWithinAllowedRoots, isBinaryExtension, resolveSearchScopeFiles, - toInt, } from "./tool-utils"; export type FindReplaceEdit = { find: string; replace: string }; @@ -191,26 +190,18 @@ export async function searchFiles( return "No matches."; } -export async function readSnippet(workspace: string, pathInput: string, start?: string, end?: string): Promise { +export async function readFileContent(workspace: string, pathInput: string): Promise { const absPath = ensurePathWithinAllowedRoots(pathInput, "Read", workspace); const raw = await readFile(absPath, "utf8"); const lines = raw.split("\n"); - - const from = toInt(start, 1); - const to = Math.max(from, toInt(end, Math.min(from + 119, lines.length))); - const slice = lines.slice(from - 1, to); - const numbered = slice.map((line, idx) => `${from + idx}: ${line}`); - + const numbered = lines.map((line, idx) => `${idx + 1}: ${line}`); return [`File: ${absPath}`, ...numbered].join("\n"); } -export async function readSnippets( - workspace: string, - entries: Array<{ path: string; start?: string; end?: string }>, -): Promise { +export async function readFileContents(workspace: string, paths: string[]): Promise { const results: string[] = []; - for (const entry of entries) { - results.push(await readSnippet(workspace, entry.path, entry.start, entry.end)); + for (const path of paths) { + results.push(await readFileContent(workspace, path)); } return results.join("\n\n"); } diff --git a/src/file-toolkit.ts b/src/file-toolkit.ts index 7cc457ce..5207f9a2 100644 --- a/src/file-toolkit.ts +++ b/src/file-toolkit.ts @@ -1,6 +1,6 @@ import { isAbsolute, relative } from "node:path"; import { z } from "zod"; -import { deleteTextFile, editFile, findFiles, readSnippets, searchFiles, writeTextFile } from "./file-ops"; +import { deleteTextFile, editFile, findFiles, readFileContents, searchFiles, writeTextFile } from "./file-ops"; import { createTool, type ToolkitDeps, type ToolkitInput } from "./tool-contract"; import { runTool } from "./tool-execution"; import { compactToolOutput } from "./tool-output"; @@ -37,20 +37,16 @@ function formatDeletePaths(paths: string[]): string { return remaining > 0 ? `${shown} (+${remaining})` : shown; } -type ReadPathInput = { path: string; start?: number; end?: number }; -type NormalizedReadEntry = { path: string; start?: string; end?: string }; - -function normalizeReadEntries(paths: ReadPathInput[]): NormalizedReadEntry[] { - const deduped = new Map(); +function deduplicatePaths(paths: Array<{ path: string }>): string[] { + const seen = new Set(); + const result: string[] = []; for (const entry of paths) { const path = entry.path.trim(); - if (path.length === 0) continue; - const start = entry.start != null ? String(entry.start) : undefined; - const end = entry.end != null ? String(entry.end) : undefined; - const key = `${path}\u0000${start ?? ""}\u0000${end ?? ""}`; - if (!deduped.has(key)) deduped.set(key, { path, start, end }); + if (path.length === 0 || seen.has(path)) continue; + seen.add(path); + result.push(path); } - return Array.from(deduped.values()); + return result; } function createFindFilesTool(deps: ToolkitDeps, input: ToolkitInput) { @@ -188,62 +184,43 @@ function createReadFileTool(deps: ToolkitDeps, input: ToolkitInput) { category: "read", permissions: ["read"], description: - "Read one or more text files. Always pass `paths` as an array of {path, start?, end?} objects, even for a single file. Omit start/end to read the entire file (preferred). Only use line ranges for files over 500 lines. Never re-read a file you already have. Batch multiple files only while discovering scope or comparing targets.", - instruction: [ - "Use `read-file` to inspect code before editing.", - "Read whole files by default and use start/end only for very large files.", - "Batch reads while discovering scope; once you are editing named targets, read each target separately right before its edit, then continue directly to `edit-file` or `edit-code`.", - "For one named file with a repeated literal replacement, read it once, compute the edits from that text, and do not re-read the same file unless the edit fails or the direct read output was truncated and you need the remaining ranges.", - ].join(" "), + "Read one or more text files. Pass `paths` as an array of {path} objects. Never re-read a file you already have.", + instruction: + "Use `read-file` to inspect code before editing. Batch reads while discovering scope; once you are editing named targets, read each target separately right before its edit, then continue directly to `edit-file` or `edit-code`.", inputSchema: z.object({ - paths: z - .array( - z - .object({ - path: z.string().min(1), - start: z.number().int().min(1).optional(), - end: z.number().int().min(1).optional(), - }) - .refine((entry) => entry.start === undefined || entry.end === undefined || entry.start <= entry.end, { - message: "start must be less than or equal to end", - path: ["end"], - }), - ) - .min(1), + paths: z.array(z.object({ path: z.string().min(1) })).min(1), }), outputSchema: z.object({ kind: z.literal("read-file"), - paths: z.array(z.object({ path: z.string().min(1), start: z.string().optional(), end: z.string().optional() })), + paths: z.array(z.string().min(1)), output: z.string(), }), execute: async (toolInput, toolCallId) => { return runTool(input.session, "read-file", toolCallId, toolInput, async (callId) => { - const entries = normalizeReadEntries(toolInput.paths); - if (entries.length === 0) throw new Error("Read requires at least one non-empty path"); - const unique = Array.from(new Set(entries.map((entry) => toDisplayPath(entry.path, input.workspace)))); - if (unique.length > 0) { - const shown = unique.slice(0, TOOL_OUTPUT_LIMITS.inlineFiles); - const remaining = unique.length - shown.length; - input.onOutput({ - toolName: "read-file", - content: { - kind: "file-header", - labelKey: "tool.label.read", - count: unique.length, - targets: shown, - omitted: remaining > 0 ? remaining : undefined, - }, - toolCallId: callId, - }); - } + const paths = deduplicatePaths(toolInput.paths); + if (paths.length === 0) throw new Error("Read requires at least one non-empty path"); + const displayPaths = paths.map((p) => toDisplayPath(p, input.workspace)); + const shown = displayPaths.slice(0, TOOL_OUTPUT_LIMITS.inlineFiles); + const remaining = displayPaths.length - shown.length; + input.onOutput({ + toolName: "read-file", + content: { + kind: "file-header", + labelKey: "tool.label.read", + count: displayPaths.length, + targets: shown, + omitted: remaining > 0 ? remaining : undefined, + }, + toolCallId: callId, + }); const baseBudget = deps.outputBudget.read; - const count = entries.length; + const count = paths.length; const budget = { maxChars: Math.max(400, Math.floor(baseBudget.maxChars / count) * count), maxLines: Math.max(20, Math.floor(baseBudget.maxLines / count) * count), }; - const result = compactToolOutput(await readSnippets(input.workspace, entries), budget); - return { kind: "read-file", paths: entries, output: result }; + const result = compactToolOutput(await readFileContents(input.workspace, paths), budget); + return { kind: "read-file", paths, output: result }; }); }, }); diff --git a/src/tool-guards.ts b/src/tool-guards.ts index 6c297c9f..1f00bb2e 100644 --- a/src/tool-guards.ts +++ b/src/tool-guards.ts @@ -139,13 +139,6 @@ function editedPathsSinceLastVerify(session: SessionContext): string[] { type RedundantQueryKind = "narrower" | "scope-narrowing"; -type ReadRequestSignature = { - path: string; - signature: string; -}; - -const WHOLE_FILE_SENTINEL_END = 1_000_000; - function redundantQueryKind(input: { toolName: string; session: SessionContext; @@ -187,39 +180,6 @@ function normalizeGuardArgValue(value: unknown): unknown { return value; } -function extractReadRequestSignatures(args: Record): ReadRequestSignature[] { - const rawPaths = Array.isArray(args.paths) ? args.paths : []; - const signatures: ReadRequestSignature[] = []; - for (const entry of rawPaths) { - if (!entry || typeof entry !== "object") continue; - const pathValue = (entry as { path?: unknown }).path; - if (typeof pathValue !== "string") continue; - const path = normalizePath(pathValue.trim().toLowerCase()); - if (path.length === 0) continue; - const start = (entry as { start?: unknown }).start; - const end = (entry as { end?: unknown }).end; - const startValue = typeof start === "number" ? String(start) : ""; - const endValue = typeof end === "number" ? String(end) : ""; - signatures.push({ path, signature: `${path}\u0000${startValue}\u0000${endValue}` }); - } - return signatures; -} - -function isWholeFileReadOfPath(args: Record, path: string): boolean { - const rawPaths = Array.isArray(args.paths) ? args.paths : []; - if (rawPaths.length !== 1) return false; - const [entry] = rawPaths; - if (!entry || typeof entry !== "object") return false; - const pathValue = (entry as { path?: unknown }).path; - if (typeof pathValue !== "string") return false; - const normalizedPath = normalizePath(pathValue.trim().toLowerCase()); - if (normalizedPath !== path) return false; - const start = (entry as { start?: unknown }).start; - const end = (entry as { end?: unknown }).end; - if (start === undefined && end === undefined) return true; - return start === 1 && typeof end === "number" && end >= WHOLE_FILE_SENTINEL_END; -} - function readCountForPath(session: SessionContext, path: string): number { let count = 0; for (const entry of scopedCallLog(session)) { @@ -294,10 +254,7 @@ const fileChurnGuard: ToolGuard = { : [] : extractReadPaths(args, { normalize: true }); if (targetPaths.length === 0) return; - const requestedReadSignatures = - toolName === "read-file" ? extractReadRequestSignatures(args).map((entry) => entry.signature) : []; const pathCounts = new Map(); - const readSignaturesByPath = new Map>(); const countsForPath = (path: string): { readCount: number; editCount: number } => { const existing = pathCounts.get(path); if (existing) return existing; @@ -305,19 +262,11 @@ const fileChurnGuard: ToolGuard = { pathCounts.set(path, created); return created; }; - const signaturesForPath = (path: string): Set => { - const existing = readSignaturesByPath.get(path); - if (existing) return existing; - const created = new Set(); - readSignaturesByPath.set(path, created); - return created; - }; const sinceLastVerify = callsSinceLastVerify(session); for (const entry of sinceLastVerify) { if (entry.toolName === "read-file") { - for (const readEntry of extractReadRequestSignatures(entry.args)) { - countsForPath(readEntry.path).readCount += 1; - signaturesForPath(readEntry.path).add(readEntry.signature); + for (const readPath of extractReadPaths(entry.args, { normalize: true })) { + countsForPath(readPath).readCount += 1; } } else if ( entry.status !== "failed" && @@ -331,16 +280,10 @@ const fileChurnGuard: ToolGuard = { for (const target of targetPaths) { const { readCount, editCount } = countsForPath(target); - if ( - toolName === "read-file" && - editCount > 0 && - session.mode !== "verify" && - requestedReadSignatures.some((signature) => signaturesForPath(target).has(signature)) - ) { + if (toolName === "read-file" && editCount > 0 && session.mode !== "verify") { report("blocked", target); throw new Error( - `File "${target}" was already edited successfully in this turn, and this reread repeats an earlier read. ` + - "Use the diff you already have or read a different section if you need new context.", + `File "${target}" was already edited successfully in this turn. Use the diff you already have.`, ); } @@ -358,7 +301,7 @@ const fileChurnGuard: ToolGuard = { report("blocked", target); throw new Error( `Repeated read/edit loop detected for "${target}". Stop incremental tweaks. ` + - "Use one consolidated edit (line-range block or edit-code), then run verify.", + "Use one consolidated edit or edit-code, then run verify.", ); } }, @@ -446,7 +389,7 @@ const redundantSearchGuard = createRedundantDiscoveryGuard({ const calls = scopedCallLog(session); const prior = calls[calls.length - 1]; if (!prior || prior.toolName !== "read-file") return; - if (isWholeFileReadOfPath(prior.args, targetPath)) { + if (extractReadPaths(prior.args, { normalize: true }).includes(targetPath)) { report("blocked", targetPath); throw new Error( `File "${targetPath}" was already read directly in full. Do not search the same file before editing; ` + From 9441f49dd7e3d23d226a7e7c3a421ef55161fcf4 Mon Sep 17 00:00:00 2001 From: Christoffer Niska Date: Wed, 25 Mar 2026 11:22:35 +0200 Subject: [PATCH 2/8] test: update tests for partial read removal --- src/agent-instructions.test.ts | 2 -- src/agent-modes.ts | 2 -- src/cli-format.ts | 7 ----- src/cli-tool.ts | 12 ++++---- src/file-ops.test.ts | 12 ++++---- src/file-toolkit.test.ts | 37 ++++++++++-------------- src/tool-cache.test.ts | 26 +++++------------ src/tool-cache.ts | 25 +++++++--------- src/tool-guards.test.ts | 53 ++++------------------------------ src/tool-utils.ts | 7 ----- 10 files changed, 51 insertions(+), 132 deletions(-) diff --git a/src/agent-instructions.test.ts b/src/agent-instructions.test.ts index aedecb2e..4917476c 100644 --- a/src/agent-instructions.test.ts +++ b/src/agent-instructions.test.ts @@ -35,7 +35,6 @@ describe("createModeInstructions", () => { expect(out).toContain("repeated literal replacements in one known file"); expect(out).toContain("collect every visible requested occurrence"); expect(out).toContain("must cover all of those visible locations"); - expect(out).toContain("If a direct `read-file` result is truncated"); expect(out).toContain("if a named file has separated occurrences you have not yet pinned to exact snippets"); expect(out).toContain("do not signal completion after the first hit or first partial batch"); expect(out).toContain("make the requested change and stop"); @@ -99,7 +98,6 @@ describe("createInstructions", () => { expect(out).toContain('{ op: "replace", rule: { all: [{ kind: "call_expression" }'); expect(out).toContain("broadening the rewrite to unrelated matches"); expect(out).toContain("calling another write tool on that same file"); - expect(out).toContain("do not re-read the same file unless the edit fails or the direct read output was truncated"); expect(out).toContain("If that preview shows the requested bounded change, stop"); expect(out).toContain("stop instead of re-reading, searching, reviewing, or editing that same file again"); expect(out).toContain("use several small exact edits in one call rather than one oversized `find` block"); diff --git a/src/agent-modes.ts b/src/agent-modes.ts index 970b2337..701a97c6 100644 --- a/src/agent-modes.ts +++ b/src/agent-modes.ts @@ -14,7 +14,6 @@ export const agentModes: Record = { tools: toolIdsForGrants(["read", "write", "execute", "network"]), preamble: [ "If the target path is explicit, skip `find-files`/`search-files` and read that file directly.", - "Always read the full file without line ranges unless you know the file is very large.", "For 'add/update in file X' tasks, make `read-file` on X your first tool call.", "If the user names the files to change, limit reads and edits to those files plus directly referenced support files needed to complete the task.", "For explicit multi-file edit tasks, work one named file at a time: read the file you are about to change, edit it, then move to the next.", @@ -27,7 +26,6 @@ export const agentModes: Record = { "For small fixes in an existing file, use exact `find`/`replace` edits and keep the change as small as the request allows.", "For repeated literal replacements in one known file, do not use `search-files`, `scan-code`, or extra rereads after the initial direct read. Use that read to collect every visible requested occurrence and make one consolidated `edit-file` call.", "If the requested literal appears in multiple visible locations in the direct read of a named file, your `edit-file` call must cover all of those visible locations, not just the first contiguous block.", - "If a direct `read-file` result is truncated or omits part of a named file, gather the missing ranges with bounded `read-file` calls before finishing a repeated replacement task on that file.", "For multi-file rename or repeated replacement tasks, if a named file has separated occurrences you have not yet pinned to exact snippets, run one scoped `search-files` on that file before editing instead of guessing a larger `find` block.", "For bounded 'each'/'every'/'all' replacements in one named file, do not signal completion after the first hit or first partial batch; finish only when the latest file text and edit preview show no remaining requested matches in that file.", "For explicit bounded fixes, make the requested change and stop.", diff --git a/src/cli-format.ts b/src/cli-format.ts index dd3836e1..45719361 100644 --- a/src/cli-format.ts +++ b/src/cli-format.ts @@ -133,13 +133,6 @@ export function formatForTool(toolId: string, raw: string): string { return (TOOL_FORMATTERS[toolId] ?? formatReadOutput)(raw); } -export function formatReadDetail(pathInput: string, start?: string, end?: string): string { - if (!start && !end) return pathInput; - const from = start ?? "1"; - const to = end ?? "EOF"; - return `${pathInput}:${from}-${to}`; -} - export function formatRunSummary( label: string, tokenUsage: { usage: { inputTokens: number; outputTokens: number; totalTokens: number }; modelCalls?: number }[], diff --git a/src/cli-tool.ts b/src/cli-tool.ts index 22b2d38a..6fa88511 100644 --- a/src/cli-tool.ts +++ b/src/cli-tool.ts @@ -1,7 +1,7 @@ import { z } from "zod"; -import { formatReadDetail, printToolResult } from "./cli-format"; +import { printToolResult } from "./cli-format"; import { formatUsage } from "./cli-help"; -import { editFile, findFiles, readSnippet, searchFiles } from "./file-ops"; +import { editFile, findFiles, readFileContent, searchFiles } from "./file-ops"; import { gitDiff, gitStatusShort } from "./git-ops"; import { t } from "./i18n"; import { runShellCommand } from "./shell-ops"; @@ -74,14 +74,14 @@ function createToolHandlers(printError: (msg: string) => void): Record { - const [pathInput, start, end] = rest; + const [pathInput] = rest; if (!pathInput) { - printError(formatUsage("acolyte tool read-file [start] [end]")); + printError(formatUsage("acolyte tool read-file ")); process.exitCode = 1; return; } - const snippet = await readSnippet(process.cwd(), pathInput, start, end); - printToolResult("read-file", snippet, formatReadDetail(pathInput, start, end)); + const content = await readFileContent(process.cwd(), pathInput); + printToolResult("read-file", content, pathInput); }, "git-status": async () => { const result = await gitStatusShort(process.cwd()); diff --git a/src/file-ops.test.ts b/src/file-ops.test.ts index 85725624..9bf2edac 100644 --- a/src/file-ops.test.ts +++ b/src/file-ops.test.ts @@ -2,7 +2,7 @@ import { afterAll, describe, expect, test } from "bun:test"; import { mkdir, readFile, rm, writeFile } from "node:fs/promises"; import { join, resolve } from "node:path"; import { TOOL_ERROR_CODES } from "./error-contract"; -import { deleteTextFile, editFile, findFiles, readSnippet, searchFiles, writeTextFile } from "./file-ops"; +import { deleteTextFile, editFile, findFiles, readFileContent, searchFiles, writeTextFile } from "./file-ops"; import { testUuid } from "./test-utils"; const WORKSPACE = resolve(process.cwd()); @@ -15,8 +15,8 @@ afterAll(async () => { }); describe("path guards", () => { - test("readSnippet blocks paths outside workspace", async () => { - await expect(readSnippet(WORKSPACE, "/etc/hosts")).rejects.toThrow("restricted to the workspace or /tmp"); + test("readFileContent blocks paths outside workspace", async () => { + await expect(readFileContent(WORKSPACE, "/etc/hosts")).rejects.toThrow("restricted to the workspace or /tmp"); }); test("editFile blocks paths outside workspace", async () => { @@ -37,11 +37,11 @@ describe("path guards", () => { ); }); - test("readSnippet allows /tmp files", async () => { + test("readFileContent allows /tmp files", async () => { const filePath = `/tmp/acolyte-test-read-${testUuid()}.txt`; tempFiles.push(filePath); await writeFile(filePath, "hello from tmp", "utf8"); - const output = await readSnippet(WORKSPACE, filePath, "1", "1"); + const output = await readFileContent(WORKSPACE, filePath); expect(output).toContain("hello from tmp"); }); @@ -348,7 +348,7 @@ describe("deleteTextFile", () => { await writeFile(filePath, "alpha\nbeta\n", "utf8"); const result = await deleteTextFile({ workspace: WORKSPACE, path: filePath }); expect(result).toContain("bytes="); - await expect(readSnippet(WORKSPACE, filePath)).rejects.toThrow(); + await expect(readFileContent(WORKSPACE, filePath)).rejects.toThrow(); }); }); diff --git a/src/file-toolkit.test.ts b/src/file-toolkit.test.ts index aff2b201..8ec5fe8d 100644 --- a/src/file-toolkit.test.ts +++ b/src/file-toolkit.test.ts @@ -2,38 +2,33 @@ import { describe, expect, test } from "bun:test"; import { toolsForAgent } from "./tool-registry"; describe("read-file tool schema", () => { - test("rejects invalid range when start is greater than end", () => { + test("accepts single path", () => { const { tools } = toolsForAgent(); const schema = tools.readFile.inputSchema; - expect(() => schema.parse({ paths: [{ path: "src/agent.ts", start: 20, end: 10 }] })).toThrow( - "start must be less than or equal to end", - ); + expect(schema.parse({ paths: [{ path: "src/agent.ts" }] })).toEqual({ + paths: [{ path: "src/agent.ts" }], + }); }); - test("accepts bounded ranges and single-sided ranges", () => { + test("accepts multiple paths", () => { const { tools } = toolsForAgent(); const schema = tools.readFile.inputSchema; - expect(schema.parse({ paths: [{ path: "src/agent.ts", start: 10, end: 20 }] })).toEqual({ - paths: [{ path: "src/agent.ts", start: 10, end: 20 }], - }); - expect(schema.parse({ paths: [{ path: "src/agent.ts", start: 10 }] })).toEqual({ - paths: [{ path: "src/agent.ts", start: 10 }], - }); - expect(schema.parse({ paths: [{ path: "src/agent.ts", end: 20 }] })).toEqual({ - paths: [{ path: "src/agent.ts", end: 20 }], + expect(schema.parse({ paths: [{ path: "src/agent.ts" }, { path: "src/cli.ts" }] })).toEqual({ + paths: [{ path: "src/agent.ts" }, { path: "src/cli.ts" }], }); }); - test("accepts multiple paths", () => { + test("rejects empty paths array", () => { const { tools } = toolsForAgent(); const schema = tools.readFile.inputSchema; - expect( - schema.parse({ - paths: [{ path: "src/agent.ts", start: 1, end: 10 }, { path: "src/cli.ts" }], - }), - ).toEqual({ - paths: [{ path: "src/agent.ts", start: 1, end: 10 }, { path: "src/cli.ts" }], - }); + expect(() => schema.parse({ paths: [] })).toThrow(); + }); + + test("strips unknown properties", () => { + const { tools } = toolsForAgent(); + const schema = tools.readFile.inputSchema; + const result = schema.parse({ paths: [{ path: "src/agent.ts", start: 10, end: 20 }] }); + expect(result).toEqual({ paths: [{ path: "src/agent.ts" }] }); }); }); diff --git a/src/tool-cache.test.ts b/src/tool-cache.test.ts index 5ef46a36..254fbbde 100644 --- a/src/tool-cache.test.ts +++ b/src/tool-cache.test.ts @@ -148,35 +148,22 @@ describe("tool-cache", () => { test("populateSubEntries splits multi-file read into per-file cache entries", () => { const cache = createToolCache(CACHEABLE); const multiArgs = { paths: [{ path: "src/a.ts" }, { path: "src/b.ts" }] }; - const result = "File: /workspace/src/a.ts\n1: const a = 1;\n\nFile: /workspace/src/b.ts\n1: const b = 2;"; + const output = "File: /workspace/src/a.ts\n1: const a = 1;\n\nFile: /workspace/src/b.ts\n1: const b = 2;"; + const result = { kind: "read-file", paths: ["src/a.ts", "src/b.ts"], output }; cache.set("read-file", multiArgs, { result }); cache.populateSubEntries("read-file", multiArgs, result); const hitA = cache.get("read-file", { paths: [{ path: "src/a.ts" }] }); expect(hitA).toBeDefined(); - expect(hitA?.result).toBe("File: /workspace/src/a.ts\n1: const a = 1;"); + expect((hitA?.result as { output: string }).output).toBe("File: /workspace/src/a.ts\n1: const a = 1;"); const hitB = cache.get("read-file", { paths: [{ path: "src/b.ts" }] }); expect(hitB).toBeDefined(); - expect(hitB?.result).toBe("File: /workspace/src/b.ts\n1: const b = 2;"); - }); - - test("populateSubEntries creates sub-entries preserving start/end ranges", () => { - const cache = createToolCache(CACHEABLE); - const multiArgs = { paths: [{ path: "src/a.ts", start: 1, end: 5 }, { path: "src/b.ts" }] }; - const result = "File: /workspace/src/a.ts\n1: const a = 1;\n\nFile: /workspace/src/b.ts\n1: const b = 2;"; - cache.set("read-file", multiArgs, { result }); - cache.populateSubEntries("read-file", multiArgs, result); - // a.ts sub-entry keyed with its range - expect(cache.get("read-file", { paths: [{ path: "src/a.ts", start: 1, end: 5 }] })).toBeDefined(); - // a.ts without range — also populated as rangeless fallback - expect(cache.get("read-file", { paths: [{ path: "src/a.ts" }] })).toBeDefined(); - // b.ts had no range — sub-entry populated - expect(cache.get("read-file", { paths: [{ path: "src/b.ts" }] })).toBeDefined(); + expect((hitB?.result as { output: string }).output).toBe("File: /workspace/src/b.ts\n1: const b = 2;"); }); test("populateSubEntries does nothing for single-file reads", () => { const cache = createToolCache(CACHEABLE); const args = { paths: [{ path: "src/a.ts" }] }; - const result = "File: /workspace/src/a.ts\n1: const a = 1;"; + const result = { kind: "read-file", paths: ["src/a.ts"], output: "File: /workspace/src/a.ts\n1: const a = 1;" }; cache.set("read-file", args, { result }); cache.populateSubEntries("read-file", args, result); // Only the original entry exists, no extra sub-entries @@ -186,7 +173,8 @@ describe("tool-cache", () => { test("write invalidation evicts sub-entry for written file", () => { const cache = createToolCache(CACHEABLE); const multiArgs = { paths: [{ path: "src/a.ts" }, { path: "src/b.ts" }] }; - const result = "File: /workspace/src/a.ts\n1: const a = 1;\n\nFile: /workspace/src/b.ts\n1: const b = 2;"; + const output = "File: /workspace/src/a.ts\n1: const a = 1;\n\nFile: /workspace/src/b.ts\n1: const b = 2;"; + const result = { kind: "read-file", paths: ["src/a.ts", "src/b.ts"], output }; cache.set("read-file", multiArgs, { result }); cache.populateSubEntries("read-file", multiArgs, result); cache.invalidateForWrite("edit-file", { path: "src/a.ts" }); diff --git a/src/tool-cache.ts b/src/tool-cache.ts index 62479eef..9ca19952 100644 --- a/src/tool-cache.ts +++ b/src/tool-cache.ts @@ -130,26 +130,23 @@ export function createToolCache( if (toolName !== "read-file") return; const paths = args.paths; if (!Array.isArray(paths) || paths.length < 2) return; - if (typeof result !== "string") return; - const sections = result.split("\n\nFile: "); + const output = + typeof result === "string" + ? result + : typeof (result as { output?: unknown })?.output === "string" + ? (result as { output: string }).output + : null; + if (!output) return; + const sections = output.split("\n\nFile: "); if (sections.length < 2) return; const normalized = [sections[0], ...sections.slice(1).map((s) => `File: ${s}`)]; for (let i = 0; i < normalized.length && i < paths.length; i++) { const entry = paths[i]; if (!entry || typeof entry !== "object") continue; - const pathEntry = entry as Record; - const p = pathEntry.path; + const p = (entry as { path?: unknown }).path; if (typeof p !== "string") continue; - const subArg: Record = { path: p }; - if (pathEntry.start !== undefined) subArg.start = pathEntry.start; - if (pathEntry.end !== undefined) subArg.end = pathEntry.end; - this.set(toolName, { paths: [subArg] }, { result: normalized[i] }); - if (pathEntry.start !== undefined || pathEntry.end !== undefined) { - const rangelessKey = stableKey(toolName, { paths: [{ path: p }] }); - if (!cache.has(rangelessKey)) { - this.set(toolName, { paths: [{ path: p }] }, { result: normalized[i] }); - } - } + const subResult = { kind: "read-file", paths: [p], output: normalized[i] }; + this.set(toolName, { paths: [{ path: p }] }, { result: subResult }); } }, diff --git a/src/tool-guards.test.ts b/src/tool-guards.test.ts index e3302922..6a93bcdb 100644 --- a/src/tool-guards.test.ts +++ b/src/tool-guards.test.ts @@ -138,21 +138,13 @@ describe("file-churn guard", () => { runGuards({ toolName: "read-file", args: { paths: [{ path: "src/chat-commands.ts" }] }, session }), ).not.toThrow(); }); - test("allows a second read with a different range before any edit", () => { - const session = createSessionContext(); - recordCall(session, "read-file", { paths: [{ path: "src/foo.ts", start: 1, end: 40 }] }); - expect(() => - runGuards({ toolName: "read-file", args: { paths: [{ path: "src/foo.ts", start: 41, end: 80 }] }, session }), - ).not.toThrow(); - }); - - test("blocks repeating the same reread after a successful edit before verify", () => { + test("blocks rereading an edited file before verify", () => { const session = createSessionContext(); session.writeTools = new Set(["edit-file"]); recordCall(session, "read-file", { paths: [{ path: "src/foo.ts" }] }); recordCall(session, "edit-file", { path: "src/foo.ts" }); expect(() => runGuards({ toolName: "read-file", args: { paths: [{ path: "src/foo.ts" }] }, session })).toThrow( - /this reread repeats an earlier read/, + /already edited/, ); }); @@ -167,16 +159,6 @@ describe("file-churn guard", () => { ).not.toThrow(); }); - test("allows reading a different range of the same file after an edit", () => { - const session = createSessionContext(); - session.writeTools = new Set(["edit-file"]); - recordCall(session, "read-file", { paths: [{ path: "src/foo.ts", start: 1, end: 40 }] }); - recordCall(session, "edit-file", { path: "src/foo.ts" }); - expect(() => - runGuards({ toolName: "read-file", args: { paths: [{ path: "src/foo.ts", start: 41, end: 80 }] }, session }), - ).not.toThrow(); - }); - test("allows reading a different file after an edit", () => { const session = createSessionContext(); session.writeTools = new Set(["edit-file"]); @@ -195,7 +177,7 @@ describe("file-churn guard", () => { recordCall(session, "edit-file", { path: "src/foo.ts" }); } expect(() => runGuards({ toolName: "read-file", args: { paths: [{ path: "src/foo.ts" }] }, session })).toThrow( - /this reread repeats an earlier read/, + /already edited/, ); }); @@ -319,21 +301,9 @@ describe("redundant-search guard", () => { ).toThrow(/already read directly in full/i); }); - test("allows same-file search after a ranged read", () => { - const session = createSessionContext(); - recordCall(session, "read-file", { paths: [{ path: "src/a.ts", start: 1, end: 40 }] }); - expect(() => - runGuards({ - toolName: "search-files", - args: { patterns: ["return undefined;"], paths: ["src/a.ts"] }, - session, - }), - ).not.toThrow(); - }); - - test("blocks same-file search after a whole-file sentinel ranged read", () => { + test("blocks same-file search after reading that file", () => { const session = createSessionContext(); - recordCall(session, "read-file", { paths: [{ path: "src/a.ts", start: 1, end: Number.MAX_SAFE_INTEGER }] }); + recordCall(session, "read-file", { paths: [{ path: "src/a.ts" }] }); expect(() => runGuards({ toolName: "search-files", @@ -342,19 +312,6 @@ describe("redundant-search guard", () => { }), ).toThrow(/already read directly in full/i); }); - - test("blocks same-file search after multiple rereads of the same file", () => { - const session = createSessionContext(); - recordCall(session, "read-file", { paths: [{ path: "src/a.ts", start: 1, end: 40 }] }); - recordCall(session, "read-file", { paths: [{ path: "src/a.ts", start: 41, end: 80 }] }); - expect(() => - runGuards({ - toolName: "search-files", - args: { patterns: ["return undefined;"], paths: ["src/a.ts"] }, - session, - }), - ).toThrow(/already read multiple times/i); - }); }); describe("redundant-find guard", () => { diff --git a/src/tool-utils.ts b/src/tool-utils.ts index 1f4a86c9..53f21279 100644 --- a/src/tool-utils.ts +++ b/src/tool-utils.ts @@ -362,10 +362,3 @@ export function createUnifiedDeleteDiff(path: string, previous: string): string const removed = oldLines.map((line) => `-${line}`); return [...header, ...removed].join("\n"); } - -export function toInt(value: string | undefined, fallback: number): number { - if (!value) return fallback; - const parsed = Number.parseInt(value, 10); - if (Number.isNaN(parsed) || parsed < 1) return fallback; - return parsed; -} From 1d20715c2e1c1340a6a725526d51ada773264704 Mon Sep 17 00:00:00 2001 From: Christoffer Niska Date: Wed, 25 Mar 2026 11:54:55 +0200 Subject: [PATCH 3/8] refactor(tools): replace read output truncation with line limit guard --- src/app-config.ts | 2 +- src/code-toolkit.ts | 2 +- src/file-ops.ts | 11 ++++++++--- src/file-toolkit.ts | 12 +++--------- src/tool-contract.ts | 2 +- 5 files changed, 14 insertions(+), 15 deletions(-) diff --git a/src/app-config.ts b/src/app-config.ts index c873261e..ae4095a5 100644 --- a/src/app-config.ts +++ b/src/app-config.ts @@ -53,7 +53,7 @@ export const appConfig = { searchFiles: { maxChars: 2200, maxLines: 80 }, webSearch: { maxChars: 2400, maxLines: 80 }, webFetch: { maxChars: 2600, maxLines: 90 }, - read: { maxChars: 2600, maxLines: 120 }, + read: { maxLines: 2000 }, gitStatus: { maxChars: 1800, maxLines: 80 }, gitDiff: { maxChars: 3200, maxLines: 120 }, run: { maxChars: 2600, maxLines: 120 }, diff --git a/src/code-toolkit.ts b/src/code-toolkit.ts index 6b2aac00..fbdead57 100644 --- a/src/code-toolkit.ts +++ b/src/code-toolkit.ts @@ -95,7 +95,7 @@ function createScanCodeTool(deps: ToolkitDeps, input: ToolkitInput) { const baseBudget = deps.outputBudget.scanCode; const count = paths.length * toolInput.patterns.length; const budget = { - maxChars: Math.max(400, Math.floor(baseBudget.maxChars / count) * count), + maxChars: baseBudget.maxChars ? Math.max(400, Math.floor(baseBudget.maxChars / count) * count) : undefined, maxLines: Math.max(20, Math.floor(baseBudget.maxLines / count) * count), }; const rawScan = await scanCode({ diff --git a/src/file-ops.ts b/src/file-ops.ts index e3049344..e0e7ecae 100644 --- a/src/file-ops.ts +++ b/src/file-ops.ts @@ -190,18 +190,23 @@ export async function searchFiles( return "No matches."; } -export async function readFileContent(workspace: string, pathInput: string): Promise { +export async function readFileContent(workspace: string, pathInput: string, maxLines?: number): Promise { const absPath = ensurePathWithinAllowedRoots(pathInput, "Read", workspace); const raw = await readFile(absPath, "utf8"); const lines = raw.split("\n"); + if (maxLines && lines.length > maxLines) { + throw new Error( + `File "${pathInput}" is too large (${lines.length} lines). Use \`search-files\` or \`scan-code\` to find the relevant sections.`, + ); + } const numbered = lines.map((line, idx) => `${idx + 1}: ${line}`); return [`File: ${absPath}`, ...numbered].join("\n"); } -export async function readFileContents(workspace: string, paths: string[]): Promise { +export async function readFileContents(workspace: string, paths: string[], maxLines?: number): Promise { const results: string[] = []; for (const path of paths) { - results.push(await readFileContent(workspace, path)); + results.push(await readFileContent(workspace, path, maxLines)); } return results.join("\n\n"); } diff --git a/src/file-toolkit.ts b/src/file-toolkit.ts index 5207f9a2..f5983b3c 100644 --- a/src/file-toolkit.ts +++ b/src/file-toolkit.ts @@ -77,7 +77,7 @@ function createFindFilesTool(deps: ToolkitDeps, input: ToolkitInput) { const count = toolInput.patterns.length; const baseBudget = deps.outputBudget.findFiles; const budget = { - maxChars: Math.max(400, Math.floor(baseBudget.maxChars / count) * count), + maxChars: baseBudget.maxChars ? Math.max(400, Math.floor(baseBudget.maxChars / count) * count) : undefined, maxLines: Math.max(20, Math.floor(baseBudget.maxLines / count) * count), }; const result = compactToolOutput(await findFiles(input.workspace, toolInput.patterns, maxResults), budget); @@ -213,14 +213,8 @@ function createReadFileTool(deps: ToolkitDeps, input: ToolkitInput) { }, toolCallId: callId, }); - const baseBudget = deps.outputBudget.read; - const count = paths.length; - const budget = { - maxChars: Math.max(400, Math.floor(baseBudget.maxChars / count) * count), - maxLines: Math.max(20, Math.floor(baseBudget.maxLines / count) * count), - }; - const result = compactToolOutput(await readFileContents(input.workspace, paths), budget); - return { kind: "read-file", paths, output: result }; + const output = await readFileContents(input.workspace, paths, deps.outputBudget.read.maxLines); + return { kind: "read-file", paths, output }; }); }, }); diff --git a/src/tool-contract.ts b/src/tool-contract.ts index 61aaf8c7..3f654bda 100644 --- a/src/tool-contract.ts +++ b/src/tool-contract.ts @@ -18,7 +18,7 @@ export type ToolDefinition = { readonly labelKey?: string; }; -export type ToolOutputBudgetEntry = { maxChars: number; maxLines: number }; +export type ToolOutputBudgetEntry = { maxChars?: number; maxLines: number }; export type ToolOutputBudget = { findFiles: ToolOutputBudgetEntry; From ac52de7acd02854782ed11cf9bcc3192ee888b83 Mon Sep 17 00:00:00 2001 From: Christoffer Niska Date: Wed, 25 Mar 2026 12:09:58 +0200 Subject: [PATCH 4/8] fix(tools): address review findings for partial read removal --- src/app-config.ts | 2 +- src/code-toolkit.ts | 2 +- src/file-ops.test.ts | 36 +++++++++++++++++++++++++++++++++++- src/file-ops.ts | 2 +- src/file-toolkit.ts | 5 +++-- src/tool-cache.ts | 6 +----- src/tool-contract.ts | 2 +- 7 files changed, 43 insertions(+), 12 deletions(-) diff --git a/src/app-config.ts b/src/app-config.ts index ae4095a5..5ff80f3b 100644 --- a/src/app-config.ts +++ b/src/app-config.ts @@ -53,7 +53,7 @@ export const appConfig = { searchFiles: { maxChars: 2200, maxLines: 80 }, webSearch: { maxChars: 2400, maxLines: 80 }, webFetch: { maxChars: 2600, maxLines: 90 }, - read: { maxLines: 2000 }, + read: { maxChars: 80_000, maxLines: 2000 }, gitStatus: { maxChars: 1800, maxLines: 80 }, gitDiff: { maxChars: 3200, maxLines: 120 }, run: { maxChars: 2600, maxLines: 120 }, diff --git a/src/code-toolkit.ts b/src/code-toolkit.ts index fbdead57..6b2aac00 100644 --- a/src/code-toolkit.ts +++ b/src/code-toolkit.ts @@ -95,7 +95,7 @@ function createScanCodeTool(deps: ToolkitDeps, input: ToolkitInput) { const baseBudget = deps.outputBudget.scanCode; const count = paths.length * toolInput.patterns.length; const budget = { - maxChars: baseBudget.maxChars ? Math.max(400, Math.floor(baseBudget.maxChars / count) * count) : undefined, + maxChars: Math.max(400, Math.floor(baseBudget.maxChars / count) * count), maxLines: Math.max(20, Math.floor(baseBudget.maxLines / count) * count), }; const rawScan = await scanCode({ diff --git a/src/file-ops.test.ts b/src/file-ops.test.ts index 9bf2edac..da54dc18 100644 --- a/src/file-ops.test.ts +++ b/src/file-ops.test.ts @@ -2,7 +2,15 @@ import { afterAll, describe, expect, test } from "bun:test"; import { mkdir, readFile, rm, writeFile } from "node:fs/promises"; import { join, resolve } from "node:path"; import { TOOL_ERROR_CODES } from "./error-contract"; -import { deleteTextFile, editFile, findFiles, readFileContent, searchFiles, writeTextFile } from "./file-ops"; +import { + deleteTextFile, + editFile, + findFiles, + readFileContent, + readFileContents, + searchFiles, + writeTextFile, +} from "./file-ops"; import { testUuid } from "./test-utils"; const WORKSPACE = resolve(process.cwd()); @@ -45,6 +53,32 @@ describe("path guards", () => { expect(output).toContain("hello from tmp"); }); + test("readFileContent rejects files exceeding maxLines", async () => { + const filePath = `/tmp/acolyte-test-large-${testUuid()}.txt`; + tempFiles.push(filePath); + const lines = Array.from({ length: 11 }, (_, i) => `line ${i + 1}`).join("\n"); + await writeFile(filePath, lines, "utf8"); + await expect(readFileContent(WORKSPACE, filePath, 10)).rejects.toThrow(/too large/); + }); + + test("readFileContent allows files at exactly maxLines", async () => { + const filePath = `/tmp/acolyte-test-exact-${testUuid()}.txt`; + tempFiles.push(filePath); + const lines = Array.from({ length: 10 }, (_, i) => `line ${i + 1}`).join("\n"); + await writeFile(filePath, lines, "utf8"); + const output = await readFileContent(WORKSPACE, filePath, 10); + expect(output).toContain("line 1"); + }); + + test("readFileContents rejects batch when any file exceeds maxLines", async () => { + const small = `/tmp/acolyte-test-small-${testUuid()}.txt`; + const large = `/tmp/acolyte-test-large-${testUuid()}.txt`; + tempFiles.push(small, large); + await writeFile(small, "ok", "utf8"); + await writeFile(large, Array.from({ length: 11 }, (_, i) => `line ${i + 1}`).join("\n"), "utf8"); + await expect(readFileContents(WORKSPACE, [small, large], 10)).rejects.toThrow(/too large/); + }); + test("editFile allows /tmp files", async () => { const filePath = `/tmp/acolyte-test-edit-${testUuid()}.txt`; tempFiles.push(filePath); diff --git a/src/file-ops.ts b/src/file-ops.ts index e0e7ecae..6c383899 100644 --- a/src/file-ops.ts +++ b/src/file-ops.ts @@ -194,7 +194,7 @@ export async function readFileContent(workspace: string, pathInput: string, maxL const absPath = ensurePathWithinAllowedRoots(pathInput, "Read", workspace); const raw = await readFile(absPath, "utf8"); const lines = raw.split("\n"); - if (maxLines && lines.length > maxLines) { + if (maxLines !== undefined && lines.length > maxLines) { throw new Error( `File "${pathInput}" is too large (${lines.length} lines). Use \`search-files\` or \`scan-code\` to find the relevant sections.`, ); diff --git a/src/file-toolkit.ts b/src/file-toolkit.ts index f5983b3c..f062a074 100644 --- a/src/file-toolkit.ts +++ b/src/file-toolkit.ts @@ -77,7 +77,7 @@ function createFindFilesTool(deps: ToolkitDeps, input: ToolkitInput) { const count = toolInput.patterns.length; const baseBudget = deps.outputBudget.findFiles; const budget = { - maxChars: baseBudget.maxChars ? Math.max(400, Math.floor(baseBudget.maxChars / count) * count) : undefined, + maxChars: Math.max(400, Math.floor(baseBudget.maxChars / count) * count), maxLines: Math.max(20, Math.floor(baseBudget.maxLines / count) * count), }; const result = compactToolOutput(await findFiles(input.workspace, toolInput.patterns, maxResults), budget); @@ -213,7 +213,8 @@ function createReadFileTool(deps: ToolkitDeps, input: ToolkitInput) { }, toolCallId: callId, }); - const output = await readFileContents(input.workspace, paths, deps.outputBudget.read.maxLines); + const raw = await readFileContents(input.workspace, paths, deps.outputBudget.read.maxLines); + const output = compactToolOutput(raw, deps.outputBudget.read); return { kind: "read-file", paths, output }; }); }, diff --git a/src/tool-cache.ts b/src/tool-cache.ts index 9ca19952..7501702d 100644 --- a/src/tool-cache.ts +++ b/src/tool-cache.ts @@ -131,11 +131,7 @@ export function createToolCache( const paths = args.paths; if (!Array.isArray(paths) || paths.length < 2) return; const output = - typeof result === "string" - ? result - : typeof (result as { output?: unknown })?.output === "string" - ? (result as { output: string }).output - : null; + typeof (result as { output?: unknown })?.output === "string" ? (result as { output: string }).output : null; if (!output) return; const sections = output.split("\n\nFile: "); if (sections.length < 2) return; diff --git a/src/tool-contract.ts b/src/tool-contract.ts index 3f654bda..61aaf8c7 100644 --- a/src/tool-contract.ts +++ b/src/tool-contract.ts @@ -18,7 +18,7 @@ export type ToolDefinition = { readonly labelKey?: string; }; -export type ToolOutputBudgetEntry = { maxChars?: number; maxLines: number }; +export type ToolOutputBudgetEntry = { maxChars: number; maxLines: number }; export type ToolOutputBudget = { findFiles: ToolOutputBudgetEntry; From 583181fa75e782b66cce1ff563be5eef79644279 Mon Sep 17 00:00:00 2001 From: Christoffer Niska Date: Wed, 25 Mar 2026 12:32:01 +0200 Subject: [PATCH 5/8] fix(guards): allow re-reading edited files for follow-up edits --- src/tool-guards.test.ts | 26 +++++++++++++++++++++++--- src/tool-guards.ts | 28 ++++++++++++++++++++++++---- 2 files changed, 47 insertions(+), 7 deletions(-) diff --git a/src/tool-guards.test.ts b/src/tool-guards.test.ts index 6a93bcdb..a0338ebe 100644 --- a/src/tool-guards.test.ts +++ b/src/tool-guards.test.ts @@ -138,13 +138,33 @@ describe("file-churn guard", () => { runGuards({ toolName: "read-file", args: { paths: [{ path: "src/chat-commands.ts" }] }, session }), ).not.toThrow(); }); - test("blocks rereading an edited file before verify", () => { + test("allows first re-read of an edited file for follow-up edits", () => { const session = createSessionContext(); session.writeTools = new Set(["edit-file"]); recordCall(session, "read-file", { paths: [{ path: "src/foo.ts" }] }); recordCall(session, "edit-file", { path: "src/foo.ts" }); + expect(() => + runGuards({ toolName: "read-file", args: { paths: [{ path: "src/foo.ts" }] }, session }), + ).not.toThrow(); + }); + + test("blocks re-read of an edited file when already re-read since last edit", () => { + const session = createSessionContext(); + session.writeTools = new Set(["edit-file"]); + session.cache = { + isCacheable: () => true, + get: () => undefined, + set: () => {}, + populateSubEntries: () => {}, + invalidateForWrite: () => {}, + clear: () => {}, + stats: () => ({ hits: 0, misses: 0, invalidations: 0, evictions: 0, size: 0 }), + }; + recordCall(session, "read-file", { paths: [{ path: "src/foo.ts" }] }); + recordCall(session, "edit-file", { path: "src/foo.ts" }); + recordCall(session, "read-file", { paths: [{ path: "src/foo.ts" }] }); expect(() => runGuards({ toolName: "read-file", args: { paths: [{ path: "src/foo.ts" }] }, session })).toThrow( - /already edited/, + /already re-read/, ); }); @@ -177,7 +197,7 @@ describe("file-churn guard", () => { recordCall(session, "edit-file", { path: "src/foo.ts" }); } expect(() => runGuards({ toolName: "read-file", args: { paths: [{ path: "src/foo.ts" }] }, session })).toThrow( - /already edited/, + /read\/edit loop/, ); }); diff --git a/src/tool-guards.ts b/src/tool-guards.ts index 1f00bb2e..303fc6bf 100644 --- a/src/tool-guards.ts +++ b/src/tool-guards.ts @@ -242,6 +242,24 @@ const duplicateCallGuard: ToolGuard = { }, }; +function hasReadSinceLastEditOf(callLog: ToolCallRecord[], session: SessionContext, path: string): boolean { + for (let i = callLog.length - 1; i >= 0; i--) { + const entry = callLog[i]; + if (!entry) continue; + if ( + entry.status !== "failed" && + isWriteTool(session, entry.toolName) && + normalizePath(String(entry.args.path ?? "")) === path + ) { + return false; + } + if (entry.toolName === "read-file" && extractReadPaths(entry.args, { normalize: true }).includes(path)) { + return true; + } + } + return false; +} + const fileChurnGuard: ToolGuard = { id: "file-churn", description: "Block excessive read/edit churn on the same file to force a strategy change.", @@ -281,10 +299,12 @@ const fileChurnGuard: ToolGuard = { const { readCount, editCount } = countsForPath(target); if (toolName === "read-file" && editCount > 0 && session.mode !== "verify") { - report("blocked", target); - throw new Error( - `File "${target}" was already edited successfully in this turn. Use the diff you already have.`, - ); + if (hasReadSinceLastEditOf(sinceLastVerify, session, target)) { + report("blocked", target); + throw new Error( + `File "${target}" was already re-read after the last edit. Use the content you already have.`, + ); + } } if (toolName === "read-file" && editCount === 0 && readCount >= FILE_READ_ONLY_CHURN_MIN) { From d38de6a1750894d8b5073db276a7837a6cea0b43 Mon Sep 17 00:00:00 2001 From: Christoffer Niska Date: Wed, 25 Mar 2026 12:41:32 +0200 Subject: [PATCH 6/8] feat(guards): block lifecycle-managed commands in work mode --- src/agent-modes.ts | 2 +- src/lifecycle.ts | 1 + src/tool-guards.ts | 29 +++++++++++++++++++++++++++++ 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/agent-modes.ts b/src/agent-modes.ts index 701a97c6..3cd96005 100644 --- a/src/agent-modes.ts +++ b/src/agent-modes.ts @@ -42,7 +42,7 @@ export const agentModes: Record = { "Trust type signatures; do not add impossible null/undefined guards unless the declared types allow them.", "Never delete a file to recreate it — use `edit-file` to modify existing files.", "When a target file does not exist, say so instead of silently creating it.", - "Do not run verify, test, or build commands — the lifecycle handles format, lint, and verify automatically after your edits.", + "Do not run lint, format, build, or verify commands — the lifecycle runs them automatically after your edits. If you added or changed a test file, run only that specific test file (e.g. `bun test src/foo.test.ts`), not the full test suite.", "Do not signal done until the requested behavior is actually implemented. Updating help text, comments, or tests alone is not completing the task — the functional change must be in place.", "After the last tool call, use the lifecycle signal format from the base instructions and keep the user-facing outcome to one sentence.", "For multi-step tasks (3+ distinct steps), use `create-checklist` at the start to define a progress checklist. Use `update-checklist` to mark items as you complete each step.", diff --git a/src/lifecycle.ts b/src/lifecycle.ts index e9616081..6ed74c2d 100644 --- a/src/lifecycle.ts +++ b/src/lifecycle.ts @@ -235,6 +235,7 @@ export async function runLifecycle(input: LifecycleInput, deps: LifecycleDeps = ctxRef = ctx; attachToolOutputHandler(ctx); ctx.session.flags.totalStepLimit = policy.totalMaxSteps; + if (profile.ecosystem) ctx.session.workspaceProfile = profile; ctx.debug("lifecycle.start", { task_id: input.taskId ?? null, mode: initialMode, model }); await deps.phaseGenerate(ctx, { diff --git a/src/tool-guards.ts b/src/tool-guards.ts index 303fc6bf..dd280a54 100644 --- a/src/tool-guards.ts +++ b/src/tool-guards.ts @@ -10,6 +10,7 @@ import { WORKSPACE_SCOPE, } from "./tool-arg-paths"; import type { ToolCache } from "./tool-contract"; +import { formatWorkspaceCommand, type WorkspaceCommand, type WorkspaceProfile } from "./workspace-profile"; const DEFAULT_CYCLE_STEP_LIMIT = 80; const DEFAULT_TOTAL_STEP_LIMIT = 200; @@ -51,6 +52,7 @@ export type SessionContext = { onGuard?: (event: GuardEvent) => void; cache?: ToolCache; onDebug?: (event: `lifecycle.${string}`, data: Record) => void; + workspaceProfile?: WorkspaceProfile; }; const FILE_CHURN_MIN_COMBINED = 12; @@ -677,6 +679,32 @@ const shellBypassGuard: ToolGuard = { }, }; +function commandMatchesProfile(command: string, profile: WorkspaceProfile): boolean { + const commands = [profile.lintCommand, profile.formatCommand, profile.verifyCommand] + .filter((cmd): cmd is WorkspaceCommand => cmd !== undefined) + .map((cmd) => formatWorkspaceCommand(cmd)); + return commands.some((cmd) => command.includes(cmd)); +} + +const lifecycleCommandGuard: ToolGuard = { + id: "lifecycle-command", + description: "Block lint/format/verify commands in work mode — the lifecycle runs them automatically.", + tools: ["run-command"], + check({ args, session, report }) { + if (session.mode !== "work") return; + const profile = session.workspaceProfile; + if (!profile) return; + const command = typeof args.command === "string" ? args.command : ""; + if (!command) return; + if (commandMatchesProfile(command, profile)) { + report("blocked", command); + throw new Error( + "Lint, format, and verify commands run automatically after your edits. Do not run them manually.", + ); + } + }, +}; + const GUARDS: ToolGuard[] = [ circuitBreakerGuard, stepBudgetGuard, @@ -689,6 +717,7 @@ const GUARDS: ToolGuard[] = [ redundantVerifyGuard, postEditRedundancyGuard, shellBypassGuard, + lifecycleCommandGuard, ]; export function runGuards(input: Omit): void { From 1235751675bfaa9115070c3710e734276a5f8751 Mon Sep 17 00:00:00 2001 From: Christoffer Niska Date: Wed, 25 Mar 2026 12:50:34 +0200 Subject: [PATCH 7/8] fix(tui): deduplicate header on /clear to prevent duplicate key warning --- src/chat-promotion.test.tsx | 30 ++++++++++++++++++++++++++++++ src/chat-promotion.ts | 8 +++++++- 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/chat-promotion.test.tsx b/src/chat-promotion.test.tsx index 18be992d..82cf6069 100644 --- a/src/chat-promotion.test.tsx +++ b/src/chat-promotion.test.tsx @@ -63,6 +63,36 @@ describe("usePromotion hook", () => { unmount(); }); + test("clearTranscript replaces header without duplicating", async () => { + const session = createSession({ id: "sess_clear1" }); + const { result, unmount } = renderHook(() => + usePromotion({ + version: "1.0", + session, + currentSessionId: session.id, + rowsRef: { current: [] }, + setRows: () => {}, + }), + ); + + expect(result.current.promotedRows.filter((r) => r.id === "header_sess_clear1")).toHaveLength(1); + + result.current.clearTranscript(); + await wait(); + + const headers = result.current.promotedRows.filter((r) => r.kind === "header"); + // Original header + new header after clear (unique IDs, no duplicates) + expect(headers).toHaveLength(2); + expect(headers[0]?.id).toBe("header_sess_clear1"); + expect(headers[1]?.id).toMatch(/^header_sess_clear1_/); + + // No duplicate key warnings — all IDs are unique + const ids = result.current.promotedRows.map((r) => r.id); + expect(new Set(ids).size).toBe(ids.length); + + unmount(); + }); + test("promote moves live rows to promoted", async () => { const session = createSession({ id: "sess_p1" }); const liveRows: ChatRow[] = [{ id: "row_1", kind: "user", content: "hello" }]; diff --git a/src/chat-promotion.ts b/src/chat-promotion.ts index 68a9bfb0..a063e5b8 100644 --- a/src/chat-promotion.ts +++ b/src/chat-promotion.ts @@ -86,10 +86,16 @@ export function usePromotion(input: UsePromotionInput): UsePromotionResult { const currentSessionIdRef = useRef(input.currentSessionId); currentSessionIdRef.current = input.currentSessionId; + const clearCountRef = useRef(0); + const clearTranscript = useCallback( (sessionId?: string) => { clearScreen(); - setPromotedRows((prev) => [...prev, createHeaderItem(input.version, sessionId ?? currentSessionIdRef.current)]); + clearCountRef.current += 1; + const id = sessionId ?? currentSessionIdRef.current; + const header = createHeaderItem(input.version, id); + header.id = `${header.id}_${clearCountRef.current}`; + setPromotedRows((prev) => [...prev, header]); input.setRows(() => []); }, [input.version, input.setRows], From d027b1eb9dc4792b829abcef66a97a71fd28a598 Mon Sep 17 00:00:00 2001 From: Christoffer Niska Date: Wed, 25 Mar 2026 14:15:34 +0200 Subject: [PATCH 8/8] chore: add guard tests, update docs, type-safe cache extraction --- docs/comparison.md | 15 +----------- docs/glossary.md | 2 +- src/agent-modes.ts | 2 +- src/code-ops.ts | 4 +-- src/file-ops.ts | 12 ++++----- src/git-ops.ts | 8 +++--- src/tool-cache.ts | 21 +++++++++++----- src/tool-guards.test.ts | 54 +++++++++++++++++++++++++++++++++++++++++ src/tool-utils.ts | 6 ++--- 9 files changed, 87 insertions(+), 37 deletions(-) diff --git a/docs/comparison.md b/docs/comparison.md index bda6946a..262c9e93 100644 --- a/docs/comparison.md +++ b/docs/comparison.md @@ -88,20 +88,7 @@ Autonomous coding agents frequently enter degenerate loops: - Repeated searches - Verification cycles with no changes -Acolyte uses behavioral guards that run before every tool call. - -| Guard id | Purpose | -|---|---| -| `circuit-breaker` | Stop execution after too many consecutive blocked calls | -| `step-budget` | Enforce per-cycle and total step budgets | -| `duplicate-call` | Block near-duplicate tool calls with no state change in between | -| `ping-pong` | Block alternating tool call patterns indicating the model is stuck | -| `stale-result` | Block calls that repeatedly return the same result | -| `file-churn` | Detect read/edit loops against the same file | -| `redundant-find` | Block narrower file discovery after broader calls | -| `redundant-search` | Block redundant search-files calls | -| `redundant-verify` | Prevent verify when no write tools ran | -| `post-edit-redundancy` | Block redundant follow-up edits without fresh file evidence | +Acolyte uses behavioral guards that run before every tool call. Guards cover step budgets, duplicate/redundant calls, file churn, ping-pong loops, and lifecycle command enforcement. See `src/tool-guards.ts` for the full set. Only OpenClaw and OpenHands ship comparable runtime safeguards. diff --git a/docs/glossary.md b/docs/glossary.md index 6fb7dfd7..5a32e1a2 100644 --- a/docs/glossary.md +++ b/docs/glossary.md @@ -29,7 +29,7 @@ Naming conventions and core terms used across Acolyte code and docs. | Ecosystem Detector | Pluggable workspace detection rule that identifies project type and resolves available tooling | | Entry | Runtime/pipeline item used during processing; not necessarily persisted | | Evaluator | Post-generation rule that accepts or requests regeneration | -| Guard | Pre-tool execution rule that may block calls (step budget, file churn, duplicate call, redundant search/find/verify) | +| Guard | Pre-tool execution rule that may block calls; see `src/tool-guards.ts` for the full set | | Host | The runtime environment around the model that provides tools, lifecycle structure, memory, guards, and recovery behavior | | Lifecycle Feedback | Task-scoped runtime feedback emitted by evaluators or selected guard outcomes and consumed by the next matching lifecycle attempt | | Lifecycle Policy | Bounded execution controls for lifecycle behavior (timeouts, regeneration caps) | diff --git a/src/agent-modes.ts b/src/agent-modes.ts index 3cd96005..f7f8c9aa 100644 --- a/src/agent-modes.ts +++ b/src/agent-modes.ts @@ -42,7 +42,7 @@ export const agentModes: Record = { "Trust type signatures; do not add impossible null/undefined guards unless the declared types allow them.", "Never delete a file to recreate it — use `edit-file` to modify existing files.", "When a target file does not exist, say so instead of silently creating it.", - "Do not run lint, format, build, or verify commands — the lifecycle runs them automatically after your edits. If you added or changed a test file, run only that specific test file (e.g. `bun test src/foo.test.ts`), not the full test suite.", + "Do not run lint, format, build, or verify commands — the lifecycle runs them automatically after your edits. If you added or changed a test file, run only that specific test file, not the full test suite.", "Do not signal done until the requested behavior is actually implemented. Updating help text, comments, or tests alone is not completing the task — the functional change must be in place.", "After the last tool call, use the lifecycle signal format from the base instructions and keep the user-facing outcome to one sentence.", "For multi-step tasks (3+ distinct steps), use `create-checklist` at the start to define a progress checklist. Use `update-checklist` to mark items as you complete each step.", diff --git a/src/code-ops.ts b/src/code-ops.ts index 199dd8f9..84b17c68 100644 --- a/src/code-ops.ts +++ b/src/code-ops.ts @@ -571,7 +571,7 @@ export async function editCode(input: { path: string; edits: EditCodeEdit[]; }): Promise { - const absPath = ensurePathWithinAllowedRoots(input.path, "AST edit", input.workspace); + const absPath = ensurePathWithinAllowedRoots(input.path, input.workspace); const pathStats = await stat(absPath); if (pathStats.isDirectory()) { @@ -688,7 +688,7 @@ export async function scanCode(input: { let scanned = 0; const scanPath = async (rawPath: string) => { - const absPath = ensurePathWithinAllowedRoots(rawPath, "Scan", input.workspace); + const absPath = ensurePathWithinAllowedRoots(rawPath, input.workspace); const info = await stat(absPath); if (info.isFile()) { diff --git a/src/file-ops.ts b/src/file-ops.ts index 6c383899..cc2fb7f5 100644 --- a/src/file-ops.ts +++ b/src/file-ops.ts @@ -190,13 +190,13 @@ export async function searchFiles( return "No matches."; } -export async function readFileContent(workspace: string, pathInput: string, maxLines?: number): Promise { - const absPath = ensurePathWithinAllowedRoots(pathInput, "Read", workspace); +export async function readFileContent(workspace: string, path: string, maxLines?: number): Promise { + const absPath = ensurePathWithinAllowedRoots(path, workspace); const raw = await readFile(absPath, "utf8"); const lines = raw.split("\n"); if (maxLines !== undefined && lines.length > maxLines) { throw new Error( - `File "${pathInput}" is too large (${lines.length} lines). Use \`search-files\` or \`scan-code\` to find the relevant sections.`, + `File "${path}" is too large (${lines.length} lines). Use \`search-files\` or \`scan-code\` to find the relevant sections.`, ); } const numbered = lines.map((line, idx) => `${idx + 1}: ${line}`); @@ -217,7 +217,7 @@ export async function editFile(input: { edits: FileEdit[]; dryRun?: boolean; }): Promise { - const absPath = ensurePathWithinAllowedRoots(input.path, "Edit", input.workspace); + const absPath = ensurePathWithinAllowedRoots(input.path, input.workspace); const raw = await readFile(absPath, "utf8"); const lines = raw.split("\n"); @@ -368,7 +368,7 @@ export async function writeTextFile(input: { content: string; overwrite?: boolean; }): Promise { - const absPath = ensurePathWithinAllowedRoots(input.path, "Write", input.workspace); + const absPath = ensurePathWithinAllowedRoots(input.path, input.workspace); const overwrite = input.overwrite ?? true; let previousContent: string | null = null; @@ -394,7 +394,7 @@ export async function writeTextFile(input: { } export async function deleteTextFile(input: { workspace: string; path: string; dryRun?: boolean }): Promise { - const absPath = ensurePathWithinAllowedRoots(input.path, "Delete", input.workspace); + const absPath = ensurePathWithinAllowedRoots(input.path, input.workspace); const previousContent = await readFile(absPath, "utf8"); const dryRun = input.dryRun ?? false; if (!dryRun) await unlink(absPath); diff --git a/src/git-ops.ts b/src/git-ops.ts index 2adfe517..1a98ad4f 100644 --- a/src/git-ops.ts +++ b/src/git-ops.ts @@ -9,7 +9,7 @@ export async function gitStatusShort(workspace: string): Promise { export async function gitDiff(workspace: string, pathInput?: string, contextLines = 3): Promise { const args = ["git", "diff", `--unified=${contextLines}`]; if (pathInput) { - ensurePathWithinAllowedRoots(pathInput, "Diff", workspace); + ensurePathWithinAllowedRoots(pathInput, workspace); args.push("--", pathInput); } const { code, stdout, stderr } = await runCommand(args, workspace); @@ -21,7 +21,7 @@ export async function gitLog(workspace: string, options?: { path?: string; limit const limit = Math.max(1, Math.min(50, options?.limit ?? 10)); const args = ["git", "log", "--oneline", "--decorate", `-n`, String(limit)]; if (options?.path) { - ensurePathWithinAllowedRoots(options.path, "Log", workspace); + ensurePathWithinAllowedRoots(options.path, workspace); args.push("--", options.path); } const { code, stdout, stderr } = await runCommand(args, workspace); @@ -37,7 +37,7 @@ export async function gitShow( const ref = options?.ref?.trim() ? options.ref.trim() : "HEAD"; const args = ["git", "show", "--no-color", `--unified=${contextLines}`, ref]; if (options?.path) { - ensurePathWithinAllowedRoots(options.path, "Show", workspace); + ensurePathWithinAllowedRoots(options.path, workspace); args.push("--", options.path); } const { code, stdout, stderr } = await runCommand(args, workspace); @@ -50,7 +50,7 @@ export async function gitAdd(workspace: string, options?: { paths?: string[]; al const paths = (options?.paths ?? []).map((path) => path.trim()).filter((path) => path.length > 0); if (!all && paths.length === 0) throw new Error("git add requires at least one path when all=false"); if (all && paths.length > 0) throw new Error("git add cannot combine all=true with explicit paths"); - for (const pathInput of paths) ensurePathWithinAllowedRoots(pathInput, "Add", workspace); + for (const pathInput of paths) ensurePathWithinAllowedRoots(pathInput, workspace); const args = ["git", "add", ...(all ? ["-A"] : ["--", ...paths])]; const { code, stdout, stderr } = await runCommand(args, workspace); if (code !== 0) throw new Error(stderr.trim() || "git add failed"); diff --git a/src/tool-cache.ts b/src/tool-cache.ts index 7501702d..70ab0364 100644 --- a/src/tool-cache.ts +++ b/src/tool-cache.ts @@ -36,6 +36,16 @@ function extractCachedPaths(toolName: string, args: Record): st return []; } +type ReadFileResult = { kind: "read-file"; paths: string[]; output: string }; + +function asReadFileResult(result: unknown): ReadFileResult | null { + if (typeof result !== "object" || result === null) return null; + if (!("kind" in result) || result.kind !== "read-file") return null; + if (!("output" in result) || typeof result.output !== "string") return null; + if (!("paths" in result) || !Array.isArray(result.paths)) return null; + return { kind: "read-file", paths: result.paths, output: result.output }; +} + const DEFAULT_MAX_ENTRIES = 256; export function createToolCache( @@ -130,18 +140,17 @@ export function createToolCache( if (toolName !== "read-file") return; const paths = args.paths; if (!Array.isArray(paths) || paths.length < 2) return; - const output = - typeof (result as { output?: unknown })?.output === "string" ? (result as { output: string }).output : null; - if (!output) return; - const sections = output.split("\n\nFile: "); + const parsed = asReadFileResult(result); + if (!parsed) return; + const sections = parsed.output.split("\n\nFile: "); if (sections.length < 2) return; const normalized = [sections[0], ...sections.slice(1).map((s) => `File: ${s}`)]; for (let i = 0; i < normalized.length && i < paths.length; i++) { const entry = paths[i]; if (!entry || typeof entry !== "object") continue; - const p = (entry as { path?: unknown }).path; + const p = "path" in entry ? entry.path : undefined; if (typeof p !== "string") continue; - const subResult = { kind: "read-file", paths: [p], output: normalized[i] }; + const subResult: ReadFileResult = { kind: "read-file", paths: [p], output: normalized[i] ?? "" }; this.set(toolName, { paths: [{ path: p }] }, { result: subResult }); } }, diff --git a/src/tool-guards.test.ts b/src/tool-guards.test.ts index a0338ebe..068198ed 100644 --- a/src/tool-guards.test.ts +++ b/src/tool-guards.test.ts @@ -717,3 +717,57 @@ describe("shell-bypass guard", () => { expect(() => runGuards({ toolName: "run-command", args: { command: "git status" }, session })).not.toThrow(); }); }); + +describe("lifecycle-command guard", () => { + test("blocks verify command in work mode", () => { + const session = createSessionContext(); + session.mode = "work"; + session.workspaceProfile = { verifyCommand: { bin: "bun", args: ["run", "verify"] } }; + expect(() => runGuards({ toolName: "run-command", args: { command: "bun run verify" }, session })).toThrow( + /automatically/, + ); + }); + + test("blocks lint command in work mode", () => { + const session = createSessionContext(); + session.mode = "work"; + session.workspaceProfile = { lintCommand: { bin: "bunx", args: ["biome", "check"] } }; + expect(() => runGuards({ toolName: "run-command", args: { command: "bunx biome check" }, session })).toThrow( + /automatically/, + ); + }); + + test("allows lifecycle commands in verify mode", () => { + const session = createSessionContext(); + session.mode = "verify"; + session.workspaceProfile = { verifyCommand: { bin: "bun", args: ["run", "verify"] } }; + expect(() => runGuards({ toolName: "run-command", args: { command: "bun run verify" }, session })).not.toThrow(); + }); + + test("allows commands when no workspace profile", () => { + const session = createSessionContext(); + session.mode = "work"; + expect(() => runGuards({ toolName: "run-command", args: { command: "bun run verify" }, session })).not.toThrow(); + }); + + test("allows unrelated commands in work mode", () => { + const session = createSessionContext(); + session.mode = "work"; + session.workspaceProfile = { verifyCommand: { bin: "bun", args: ["run", "verify"] } }; + expect(() => + runGuards({ toolName: "run-command", args: { command: "bun test src/foo.test.ts" }, session }), + ).not.toThrow(); + }); +}); + +describe("file-churn guard with failed edits", () => { + test("failed edit does not block subsequent re-read", () => { + const session = createSessionContext(); + session.writeTools = new Set(["edit-file"]); + recordCall(session, "read-file", { paths: [{ path: "src/foo.ts" }] }); + recordCall(session, "edit-file", { path: "src/foo.ts" }, undefined, "failed"); + expect(() => + runGuards({ toolName: "read-file", args: { paths: [{ path: "src/foo.ts" }] }, session }), + ).not.toThrow(); + }); +}); diff --git a/src/tool-utils.ts b/src/tool-utils.ts index 53f21279..49cb7649 100644 --- a/src/tool-utils.ts +++ b/src/tool-utils.ts @@ -40,9 +40,9 @@ export function isAllowedPath(pathInput: string, workspace: string): boolean { return isWithinWorkspace(pathInput, workspace) || isWithinTempRoot(pathInput, workspace); } -export function ensurePathWithinAllowedRoots(pathInput: string, operation: string, workspace: string): string { +export function ensurePathWithinAllowedRoots(pathInput: string, workspace: string): string { const absPath = resolveAgentPath(pathInput, workspace); - if (!isAllowedPath(absPath, workspace)) throw new Error(`${operation} is restricted to the workspace or /tmp`); + if (!isAllowedPath(absPath, workspace)) throw new Error("Path is restricted to the workspace or /tmp"); return absPath; } @@ -189,7 +189,7 @@ export async function resolveSearchScopeFiles(workspace: string, paths: string[] if (normalizedPaths.length === 0) return allFiles; const include = new Set(); for (const rawPath of normalizedPaths) { - const absPath = ensurePathWithinAllowedRoots(rawPath, "Search", workspace); + const absPath = ensurePathWithinAllowedRoots(rawPath, workspace); if (!isWithinWorkspacePath(absPath, workspace)) throw new Error("Search paths must be within the workspace"); let entryStat: Awaited>; try {