diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 0bdab578b..2ae66e666 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -28,6 +28,7 @@ - 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)) - Fixed grep match lines, codemode output, compaction summaries of tool results, and MCP error and server summaries cutting an emoji or other non-BMP character in half when truncating, which left an invalid lone surrogate in the text ([#7121](https://github.com/earendil-works/pi/issues/7121)) +- Fixed the find tool on Windows returning no results for every path pattern, such as `src/**/*.ts`, with fd 10.5.1, which pi downloads now. The workaround for older fd is now used only with fd before 10.5.1. With fd 10.5.1, `src/**/*.ts` also finds files directly in `src/`, `src/*.ts` no longer finds files in subdirectories, and patterns with Windows separators such as `src\**\*.ts` work ([#9262](https://github.com/earendil-works/pi/issues/9262)) ## [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 c3453e5c6..bc0b8f232 100644 --- a/packages/coding-agent/src/core/tools/find.ts +++ b/packages/coding-agent/src/core/tools/find.ts @@ -1,5 +1,5 @@ import { createInterface } from "node:readline"; -import { spawn } from "child_process"; +import { execFile, spawn } from "child_process"; import path from "path"; import { type Static, Type } from "typebox"; import { ensureTool } from "../../utils/tools-manager.ts"; @@ -22,6 +22,28 @@ export function relativizeFindResultPath( return hadTrailingSeparator && !posixPath.endsWith("/") ? `${posixPath}/` : posixPath; } +// Before 10.5.1, fd on Windows matched --full-path globs against paths with native `\` separators, so a +// glob separator `/` never matched (sharkdp/fd#2067). Version per fd binary path, probed once. +const legacyWindowsFullPathGlobByFdPath = new Map>(); + +/** Whether this fd binary predates the Windows --full-path glob fix (sharkdp/fd#2067, fd 10.5.1). */ +export function usesLegacyWindowsFullPathGlob(fdPath: string): Promise { + let result = legacyWindowsFullPathGlobByFdPath.get(fdPath); + if (!result) { + result = new Promise((resolve) => { + execFile(fdPath, ["--version"], { timeout: 5000 }, (_error, stdout) => { + const match = /(\d+)\.(\d+)\.(\d+)/.exec(stdout); + // Unknown versions are treated as fixed: pi downloads the latest fd release. + if (!match) return resolve(false); + const [major, minor, patch] = match.slice(1).map(Number); + resolve(major !== 10 ? major < 10 : minor !== 5 ? minor < 5 : patch < 1); + }); + }); + legacyWindowsFullPathGlobByFdPath.set(fdPath, result); + } + return result; +} + const findSchema = Type.Object({ pattern: Type.String({ description: "Glob pattern to match files, e.g. '*.ts', '**/*.json', or 'src/**/*.spec.ts'", @@ -173,6 +195,8 @@ export function createFindToolDefinition( // Default implementation uses fd. const fdPath = await ensureTool("fd"); + const legacyWindowsGlob = + process.platform === "win32" && !!fdPath && (await usesLegacyWindowsFullPathGlob(fdPath)); if (signal?.aborted) { settle(() => reject(new Error("Operation aborted"))); return; @@ -205,15 +229,24 @@ export function createFindToolDefinition( // 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 // pattern like 'src/**/*.spec.ts' needs a leading '**/' to match anything. - let effectivePattern = pattern; - if (pattern.includes("/")) { + // On Windows `\` is a path separator (fd does not treat it as a glob escape there), so + // native patterns like 'src\**\*.ts' are normalized to '/' (#9262). On POSIX `\` stays an + // escape character. + let effectivePattern = process.platform === "win32" ? pattern.replaceAll("\\", "/") : pattern; + if (effectivePattern.includes("/")) { args.push("--full-path"); - if (!pattern.startsWith("/") && !pattern.startsWith("**/") && pattern !== "**") { - effectivePattern = `**/${pattern}`; + if ( + !effectivePattern.startsWith("/") && + !effectivePattern.startsWith("**/") && + effectivePattern !== "**" + ) { + effectivePattern = `**/${effectivePattern}`; } - // fd matches full paths using native separators on Windows. - if (process.platform === "win32") + // Best-effort workaround for old fd on Windows. It must not be applied to fixed fd: + // '**[/\\]' is no longer a recursive wildcard there, so every path glob matches nothing. + if (legacyWindowsGlob) { effectivePattern = effectivePattern.replaceAll("/", String.raw`[/\\]`); + } } args.push("--", effectivePattern, searchPath); diff --git a/packages/coding-agent/test/suite/regressions/3302-find-path-glob.test.ts b/packages/coding-agent/test/suite/regressions/3302-find-path-glob.test.ts index 8c8e02a1f..72cbdef56 100644 --- a/packages/coding-agent/test/suite/regressions/3302-find-path-glob.test.ts +++ b/packages/coding-agent/test/suite/regressions/3302-find-path-glob.test.ts @@ -2,7 +2,13 @@ import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; -import { createFindToolDefinition } from "../../../src/core/tools/find.ts"; +import { createFindToolDefinition, usesLegacyWindowsFullPathGlob } from "../../../src/core/tools/find.ts"; +import { getToolPath } from "../../../src/utils/tools-manager.ts"; + +// fd before 10.5.1 on Windows only gets a best-effort workaround (sharkdp/fd#2067): it misses files +// directly under the prefix and lets '*' cross directories, so exact-result cases skip there. +const fdPath = getToolPath("fd"); +const exactPathGlobs = !(process.platform === "win32" && fdPath && (await usesLegacyWindowsFullPathGlob(fdPath))); /** * Regression test for https://github.com/earendil-works/pi/issues/3302 @@ -26,6 +32,7 @@ describe("issue #3302 find returns no results for path-based glob patterns", () writeFileSync(join(tempRoot, "some", "parent", "child", "file.ext"), ""); writeFileSync(join(tempRoot, "some", "parent", "child", "test.spec.ts"), ""); writeFileSync(join(tempRoot, "src", "foo", "bar", "example.spec.ts"), ""); + writeFileSync(join(tempRoot, "src", "direct.spec.ts"), ""); }); afterEach(() => { @@ -49,7 +56,11 @@ describe("issue #3302 find returns no results for path-based glob patterns", () it("basename pattern still matches (regression-safe)", async () => { const files = await runFind("*.spec.ts"); - expect(files.sort()).toEqual(["some/parent/child/test.spec.ts", "src/foo/bar/example.spec.ts"]); + expect(files.sort()).toEqual([ + "some/parent/child/test.spec.ts", + "src/direct.spec.ts", + "src/foo/bar/example.spec.ts", + ]); }); it("directory-prefixed pattern with ** tail matches subtree", async () => { @@ -65,8 +76,40 @@ describe("issue #3302 find returns no results for path-based glob patterns", () expect(files.sort()).toContain("some/parent/child/test.spec.ts"); }); - it("src/**/*.spec.ts matches nested spec file", async () => { + it.runIf(exactPathGlobs)("src/**/*.spec.ts matches direct and nested spec files", async () => { const files = await runFind("src/**/*.spec.ts"); - expect(files).toEqual(["src/foo/bar/example.spec.ts"]); + expect(files.sort()).toEqual(["src/direct.spec.ts", "src/foo/bar/example.spec.ts"]); + }); + + it.runIf(exactPathGlobs)("src/*.spec.ts does not cross directories", async () => { + const files = await runFind("src/*.spec.ts"); + expect(files).toEqual(["src/direct.spec.ts"]); + }); + + // https://github.com/earendil-works/pi/issues/9262: on Windows, `\` is a path separator. + // On POSIX it is a glob escape character, so these cases are Windows-only. + it.runIf(process.platform === "win32" && exactPathGlobs)( + "Windows backslash patterns match like forward-slash patterns", + async () => { + expect((await runFind(String.raw`src\**\*.spec.ts`)).sort()).toEqual([ + "src/direct.spec.ts", + "src/foo/bar/example.spec.ts", + ]); + expect(await runFind(String.raw`src\*.spec.ts`)).toEqual(["src/direct.spec.ts"]); + expect((await runFind(String.raw`**\parent\child\*`)).sort()).toEqual([ + "some/parent/child/file.ext", + "some/parent/child/test.spec.ts", + ]); + expect((await runFind(String.raw`src\**/*.spec.ts`)).sort()).toEqual([ + "src/direct.spec.ts", + "src/foo/bar/example.spec.ts", + ]); + }, + ); + + it.runIf(process.platform !== "win32")("POSIX backslash still escapes glob characters", async () => { + writeFileSync(join(tempRoot, "src", "*.ts"), ""); + writeFileSync(join(tempRoot, "src", "other.ts"), ""); + expect(await runFind(String.raw`src/\*.ts`)).toEqual(["src/*.ts"]); }); });