diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 062dc9ebb..f9ca79449 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -19,6 +19,7 @@ - Fixed changelog notes repeating on every fresh launch when the installed package version has a prerelease suffix, such as `0.1.5-20261020.7`: the saved version was read as `0.1.0`, so the `0.1.1` to `0.1.5` notes counted as new each time. A saved version without `major.minor.patch` now shows no notes and is replaced by the installed version, instead of showing the whole changelog ([#11703](https://github.com/earendil-works/pi/issues/11703)) - Fixed RPC mode cutting off shutdown cleanup, such as a pending MCP token refresh, when stdin closed and SIGTERM arrived together, which process managers like Paseo and systemd do. Pi lost the refreshed token and the MCP server revoked the sign-in. Once a shutdown has started, a stdin EOF or a single signal no longer interrupts it; a second signal still exits at once. Print mode likewise no longer dies from a signal that arrives during its final cleanup ([#11599](https://github.com/earendil-works/pi/issues/11599)) - Fixed the grep tool reading every matched file into memory when `context` was set, which ran out of memory on large files; context now comes from ripgrep, which also fixes UTF-16 files and lone CRs in matched lines, and the match limit notice no longer appears when exactly `limit` matches exist ([#9276](https://github.com/earendil-works/pi/issues/9276), [#11423](https://github.com/earendil-works/pi/issues/11423)) +- Fixed the find tool reporting the result limit when exactly `limit` files match, and `limit: 0` in find and ls returning every result or an empty listing; limits below 1 now mean 1 ([#7121](https://github.com/earendil-works/pi/issues/7121)) ## [1.5.0] - 2026-11-02 diff --git a/packages/coding-agent/src/core/tools/find.ts b/packages/coding-agent/src/core/tools/find.ts index 77ba1b356..c3453e5c6 100644 --- a/packages/coding-agent/src/core/tools/find.ts +++ b/packages/coding-agent/src/core/tools/find.ts @@ -108,7 +108,8 @@ export function createFindToolDefinition( (async () => { try { const searchPath = resolveToCwd(searchDir || ".", ctx?.cwd || cwd); - const effectiveLimit = limit ?? DEFAULT_LIMIT; + // fd treats --max-results 0 as unlimited, so a limit below 1 means 1. + const effectiveLimit = Math.max(1, Math.floor(limit ?? DEFAULT_LIMIT)); const ops = customOps ?? defaultFindOperations; // If custom operations provide glob(), use that instead of fd. @@ -121,9 +122,10 @@ export function createFindToolDefinition( settle(() => reject(new Error("Operation aborted"))); return; } + // One result past the limit tells "exactly N matches" apart from "more than N". const results = await ops.glob(pattern, searchPath, { ignore: ["**/node_modules/**", "**/.git/**"], - limit: effectiveLimit, + limit: effectiveLimit + 1, }); if (signal?.aborted) { settle(() => reject(new Error("Operation aborted"))); @@ -139,9 +141,11 @@ export function createFindToolDefinition( return; } + const resultLimitReached = results.length > effectiveLimit; // Relativize paths against the search root for stable output. - const relativized = results.map((p) => relativizeFindResultPath(p, searchPath)); - const resultLimitReached = relativized.length >= effectiveLimit; + const relativized = results + .slice(0, effectiveLimit) + .map((p) => relativizeFindResultPath(p, searchPath)); const rawOutput = relativized.join("\n"); const truncation = truncateHead(rawOutput, { maxLines: Number.MAX_SAFE_INTEGER }); let resultOutput = truncation.content; @@ -195,7 +199,8 @@ export function createFindToolDefinition( current = parent; } if (!insideGitRepo) args.push("--no-require-git"); - args.push("--max-results", String(effectiveLimit)); + // One result past the limit tells "exactly N matches" apart from "more than N" (#7121). + args.push("--max-results", String(effectiveLimit + 1)); // fd --glob matches against the basename unless --full-path is set; in --full-path // mode it matches against the absolute candidate path, so a path-containing @@ -215,7 +220,13 @@ export function createFindToolDefinition( const child = spawn(fdPath, args, { stdio: ["ignore", "pipe", "pipe"] }); const rl = createInterface({ input: child.stdout }); let stderr = ""; - const lines: string[] = []; + // Results within the limit, stored only up to the first one past the byte cap: + // truncateHead drops the rest anyway, so they are counted but not kept. + const storedResults: string[] = []; + let resultCount = 0; + // Byte size of all results within the limit, each counted with a trailing newline. + let outputBytes = 0; + let resultLimitReached = false; stopChild = () => { if (!child.killed) { @@ -231,8 +242,17 @@ export function createFindToolDefinition( stderr += chunk.toString(); }); - rl.on("line", (line) => { - lines.push(line); + rl.on("line", (rawLine) => { + const line = rawLine.replace(/\r$/, "").trim(); + if (!line) return; + if (resultCount === effectiveLimit) { + resultLimitReached = true; + return; + } + const result = relativizeFindResultPath(line, searchPath); + resultCount++; + if (outputBytes - 1 <= DEFAULT_MAX_BYTES) storedResults.push(result); + outputBytes += Buffer.byteLength(result, "utf-8") + 1; }); child.on("error", (error) => { @@ -246,15 +266,12 @@ export function createFindToolDefinition( settle(() => reject(new Error("Operation aborted"))); return; } - const output = lines.join("\n"); - if (code !== 0) { + if (code !== 0 && resultCount === 0) { const errorMsg = stderr.trim() || `fd exited with code ${code}`; - if (!output) { - settle(() => reject(new Error(errorMsg))); - return; - } + settle(() => reject(new Error(errorMsg))); + return; } - if (!output) { + if (resultCount === 0) { settle(() => resolve({ content: [{ type: "text", text: "No files found matching pattern" }], @@ -264,16 +281,13 @@ export function createFindToolDefinition( return; } - const relativized: string[] = []; - for (const rawLine of lines) { - const line = rawLine.replace(/\r$/, "").trim(); - if (!line) continue; - relativized.push(relativizeFindResultPath(line, searchPath)); - } - - const resultLimitReached = relativized.length >= effectiveLimit; - const rawOutput = relativized.join("\n"); - const truncation = truncateHead(rawOutput, { maxLines: Number.MAX_SAFE_INTEGER }); + // Stored results extend past the byte cap when more exist, so truncateHead decides + // truncation as for the full output; only the totals need the unstored rest. + const truncation = { + ...truncateHead(storedResults.join("\n"), { maxLines: Number.MAX_SAFE_INTEGER }), + totalLines: resultCount, + totalBytes: outputBytes - 1, + }; let resultOutput = truncation.content; const details: FindToolDetails = {}; const notices: string[] = []; diff --git a/packages/coding-agent/src/core/tools/ls.ts b/packages/coding-agent/src/core/tools/ls.ts index 4f5efc8a7..a06a50820 100644 --- a/packages/coding-agent/src/core/tools/ls.ts +++ b/packages/coding-agent/src/core/tools/ls.ts @@ -80,7 +80,8 @@ export function createLsToolDefinition( (async () => { try { const dirPath = resolveToCwd(path || ".", ctx?.cwd || cwd); - const effectiveLimit = limit ?? DEFAULT_LIMIT; + // A limit of 0 would list nothing and report "(empty directory)", so it means 1. + const effectiveLimit = Math.max(1, Math.floor(limit ?? DEFAULT_LIMIT)); // Check if path exists. if (!(await ops.exists(dirPath))) { diff --git a/packages/coding-agent/test/tools.test.ts b/packages/coding-agent/test/tools.test.ts index 26e2f9382..0fc908486 100644 --- a/packages/coding-agent/test/tools.test.ts +++ b/packages/coding-agent/test/tools.test.ts @@ -13,10 +13,11 @@ import { } from "../src/core/tools/bash.ts"; import { createEditToolDefinition } from "../src/core/tools/edit.ts"; import { computeEditsDiff } from "../src/core/tools/edit-diff.ts"; -import { createFindToolDefinition } from "../src/core/tools/find.ts"; +import { createFindToolDefinition, type FindOperations } from "../src/core/tools/find.ts"; import { createGrepToolDefinition } from "../src/core/tools/grep.ts"; import { createLsToolDefinition } from "../src/core/tools/ls.ts"; import { createReadToolDefinition } from "../src/core/tools/read.ts"; +import { truncateHead } from "../src/core/tools/truncate.ts"; import { createWriteToolDefinition } from "../src/core/tools/write.ts"; import { createEditTool, @@ -1163,6 +1164,93 @@ describe("Coding Agent Tools", () => { expect(getTextOutput(result)).toContain("No files found matching pattern"); }); + + // #7121: exactly `limit` matches must not report that the limit was reached. + it("should report the result limit only when more results exist", async () => { + for (const name of ["a.txt", "b.txt", "c.txt"]) writeFileSync(join(testDir, name), name); + + const exact = await findTool.execute("test-call-find-exact-limit", { + pattern: "*.txt", + path: testDir, + limit: 3, + }); + expect(getTextOutput(exact).split("\n").sort()).toEqual(["a.txt", "b.txt", "c.txt"]); + expect(exact.details).toBeUndefined(); + + const over = await findTool.execute("test-call-find-over-limit", { + pattern: "*.txt", + path: testDir, + limit: 2, + }); + const [listing, notice] = getTextOutput(over).split("\n\n"); + expect(listing.split("\n")).toHaveLength(2); + expect(notice).toBe("[2 results limit reached. Use limit=4 for more, or refine pattern]"); + expect(over.details).toEqual({ resultLimitReached: 2 }); + }); + + it("should treat limits below 1 as 1 and floor fractional limits", async () => { + for (const name of ["a.txt", "b.txt", "c.txt"]) writeFileSync(join(testDir, name), name); + + for (const limit of [0, -5, 0.5, 1.9]) { + const result = await findTool.execute("test-call-find-small-limit", { + pattern: "*.txt", + path: testDir, + limit, + }); + const [listing, notice] = getTextOutput(result).split("\n\n"); + expect(listing.split("\n")).toHaveLength(1); + expect(notice).toBe("[1 results limit reached. Use limit=2 for more, or refine pattern]"); + expect(result.details).toEqual({ resultLimitReached: 1 }); + } + }); + + it("should report full output totals past the byte cap", async () => { + // 300 names of 200 bytes join to about 60KB; equal lengths make the kept prefix independent of fd's order. + const names = Array.from({ length: 300 }, (_, i) => `${String(i).padStart(3, "0")}${"x".repeat(193)}.txt`); + for (const name of names) writeFileSync(join(testDir, name), ""); + + const result = await findTool.execute("test-call-find-byte-cap", { pattern: "*.txt", path: testDir }); + const expected = truncateHead(names.join("\n"), { maxLines: Number.MAX_SAFE_INTEGER }); + expect(result.details?.truncation).toEqual({ ...expected, content: result.details?.truncation?.content }); + expect(result.details?.truncation?.content.split("\n")).toHaveLength(expected.outputLines); + expect(result.details?.resultLimitReached).toBeUndefined(); + expect(getTextOutput(result)).toMatch(/\n\n\[50\.0KB limit reached\]$/); + }); + + it("should ask custom glob operations for one result past the limit", async () => { + const requestedLimits: number[] = []; + const createTool = (results: string[]) => { + const operations: FindOperations = { + exists: () => true, + glob: (_pattern, _cwd, options) => { + requestedLimits.push(options.limit); + return results.slice(0, options.limit); + }, + }; + return createFindToolDefinition(testDir, { operations }); + }; + + const exact = await createTool(["a.txt", "b.txt"]).execute( + "test-call-find-custom-exact", + { pattern: "*.txt", limit: 2 }, + undefined, + undefined, + fakeCtx(testDir), + ); + expect(getTextOutput(exact)).toBe("a.txt\nb.txt"); + expect(exact.details).toBeUndefined(); + + const over = await createTool(["a.txt", "b.txt", "c.txt"]).execute( + "test-call-find-custom-over", + { pattern: "*.txt", limit: 0 }, + undefined, + undefined, + fakeCtx(testDir), + ); + expect(getTextOutput(over)).toBe("a.txt\n\n[1 results limit reached]"); + expect(over.details).toEqual({ resultLimitReached: 1 }); + expect(requestedLimits).toEqual([3, 2]); + }); }); describe("ls tool", () => { @@ -1176,6 +1264,24 @@ describe("Coding Agent Tools", () => { expect(output).toContain(".hidden-file"); expect(output).toContain(".hidden-dir/"); }); + + it("should treat limits below 1 as 1 and floor fractional limits", async () => { + for (const name of ["a.txt", "b.txt", "c.txt"]) writeFileSync(join(testDir, name), name); + + for (const limit of [0, -5, 0.5, 1.9]) { + const result = await lsTool.execute("test-call-ls-small-limit", { path: testDir, limit }); + expect(getTextOutput(result)).toBe("a.txt\n\n[1 entries limit reached. Use limit=2 for more]"); + expect(result.details).toEqual({ entryLimitReached: 1 }); + } + }); + + it("should report the entry limit only when more entries exist", async () => { + for (const name of ["a.txt", "b.txt", "c.txt"]) writeFileSync(join(testDir, name), name); + + const result = await lsTool.execute("test-call-ls-exact-limit", { path: testDir, limit: 3 }); + expect(getTextOutput(result)).toBe("a.txt\nb.txt\nc.txt"); + expect(result.details).toBeUndefined(); + }); }); });