mirror of
https://github.com/badlogic/pi-mono.git
synced 2026-11-05 23:37:25 +00:00
fix(coding-agent): match find path globs with fd 10.5.1 on Windows
fd 10.5.1 fixes --full-path globs on Windows (sharkdp/fd#2067). The old [/\] separator rewrite breaks every path glob there, so it now applies only to fd before 10.5.1. Backslashes in Windows patterns are normalized to '/'. fixes #9262
This commit is contained in:
parent
5ff8c02708
commit
7cce3078b2
3 changed files with 88 additions and 11 deletions
|
|
@ -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 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 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 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
|
## [1.5.0] - 2026-11-02
|
||||||
|
|
|
||||||
|
|
@ -1,5 +1,5 @@
|
||||||
import { createInterface } from "node:readline";
|
import { createInterface } from "node:readline";
|
||||||
import { spawn } from "child_process";
|
import { execFile, spawn } from "child_process";
|
||||||
import path from "path";
|
import path from "path";
|
||||||
import { type Static, Type } from "typebox";
|
import { type Static, Type } from "typebox";
|
||||||
import { ensureTool } from "../../utils/tools-manager.ts";
|
import { ensureTool } from "../../utils/tools-manager.ts";
|
||||||
|
|
@ -22,6 +22,28 @@ export function relativizeFindResultPath(
|
||||||
return hadTrailingSeparator && !posixPath.endsWith("/") ? `${posixPath}/` : posixPath;
|
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<string, Promise<boolean>>();
|
||||||
|
|
||||||
|
/** Whether this fd binary predates the Windows --full-path glob fix (sharkdp/fd#2067, fd 10.5.1). */
|
||||||
|
export function usesLegacyWindowsFullPathGlob(fdPath: string): Promise<boolean> {
|
||||||
|
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({
|
const findSchema = Type.Object({
|
||||||
pattern: Type.String({
|
pattern: Type.String({
|
||||||
description: "Glob pattern to match files, e.g. '*.ts', '**/*.json', or 'src/**/*.spec.ts'",
|
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.
|
// Default implementation uses fd.
|
||||||
const fdPath = await ensureTool("fd");
|
const fdPath = await ensureTool("fd");
|
||||||
|
const legacyWindowsGlob =
|
||||||
|
process.platform === "win32" && !!fdPath && (await usesLegacyWindowsFullPathGlob(fdPath));
|
||||||
if (signal?.aborted) {
|
if (signal?.aborted) {
|
||||||
settle(() => reject(new Error("Operation aborted")));
|
settle(() => reject(new Error("Operation aborted")));
|
||||||
return;
|
return;
|
||||||
|
|
@ -205,15 +229,24 @@ export function createFindToolDefinition(
|
||||||
// fd --glob matches against the basename unless --full-path is set; in --full-path
|
// 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
|
// mode it matches against the absolute candidate path, so a path-containing
|
||||||
// pattern like 'src/**/*.spec.ts' needs a leading '**/' to match anything.
|
// pattern like 'src/**/*.spec.ts' needs a leading '**/' to match anything.
|
||||||
let effectivePattern = pattern;
|
// On Windows `\` is a path separator (fd does not treat it as a glob escape there), so
|
||||||
if (pattern.includes("/")) {
|
// 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");
|
args.push("--full-path");
|
||||||
if (!pattern.startsWith("/") && !pattern.startsWith("**/") && pattern !== "**") {
|
if (
|
||||||
effectivePattern = `**/${pattern}`;
|
!effectivePattern.startsWith("/") &&
|
||||||
|
!effectivePattern.startsWith("**/") &&
|
||||||
|
effectivePattern !== "**"
|
||||||
|
) {
|
||||||
|
effectivePattern = `**/${effectivePattern}`;
|
||||||
}
|
}
|
||||||
// fd matches full paths using native separators on Windows.
|
// Best-effort workaround for old fd on Windows. It must not be applied to fixed fd:
|
||||||
if (process.platform === "win32")
|
// '**[/\\]' is no longer a recursive wildcard there, so every path glob matches nothing.
|
||||||
|
if (legacyWindowsGlob) {
|
||||||
effectivePattern = effectivePattern.replaceAll("/", String.raw`[/\\]`);
|
effectivePattern = effectivePattern.replaceAll("/", String.raw`[/\\]`);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
args.push("--", effectivePattern, searchPath);
|
args.push("--", effectivePattern, searchPath);
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -2,7 +2,13 @@ import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
||||||
import { tmpdir } from "node:os";
|
import { tmpdir } from "node:os";
|
||||||
import { join } from "node:path";
|
import { join } from "node:path";
|
||||||
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
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
|
* 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", "file.ext"), "");
|
||||||
writeFileSync(join(tempRoot, "some", "parent", "child", "test.spec.ts"), "");
|
writeFileSync(join(tempRoot, "some", "parent", "child", "test.spec.ts"), "");
|
||||||
writeFileSync(join(tempRoot, "src", "foo", "bar", "example.spec.ts"), "");
|
writeFileSync(join(tempRoot, "src", "foo", "bar", "example.spec.ts"), "");
|
||||||
|
writeFileSync(join(tempRoot, "src", "direct.spec.ts"), "");
|
||||||
});
|
});
|
||||||
|
|
||||||
afterEach(() => {
|
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 () => {
|
it("basename pattern still matches (regression-safe)", async () => {
|
||||||
const files = await runFind("*.spec.ts");
|
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 () => {
|
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");
|
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");
|
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"]);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue