From 1d4fdbad2658bc81cbbe1afa05a1fa224a5ebc57 Mon Sep 17 00:00:00 2001 From: Mario Zechner Date: Thu, 16 Apr 2026 23:03:33 +0200 Subject: [PATCH] fix(coding-agent): scope nested .gitignore rules to their subtree in find The find tool previously collected every .gitignore under the search path and passed them to fd via --ignore-file. fd treats --ignore-file entries as a single global ignore source, so rules from a/.gitignore also filtered files under sibling b/. Drop the manual collection and pass --no-require-git instead, which makes fd apply hierarchical .gitignore semantics whether or not a git repo is present. closes #3303 --- packages/coding-agent/CHANGELOG.md | 1 + packages/coding-agent/src/core/tools/find.ts | 31 +------ .../3303-find-nested-gitignore.test.ts | 83 +++++++++++++++++++ 3 files changed, 88 insertions(+), 27 deletions(-) create mode 100644 packages/coding-agent/test/suite/regressions/3303-find-nested-gitignore.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index f63d4f33..af115492 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -6,6 +6,7 @@ - Fixed `--verbose` startup output to begin with expanded startup help and loaded resource listings after the compact startup header change ([#3147](https://github.com/badlogic/pi-mono/issues/3147)) - Fixed `find` tool returning no results for path-based glob patterns such as `src/**/*.spec.ts` or `some/parent/child/**` by switching fd into full-path mode and normalizing the pattern when it contains a `/` ([#3302](https://github.com/badlogic/pi-mono/issues/3302)) +- Fixed `find` tool applying nested `.gitignore` rules across sibling directories (e.g. rules from `a/.gitignore` hiding matching files under `b/`) by dropping the manual `--ignore-file` collection and delegating to fd's hierarchical `.gitignore` handling via `--no-require-git` ([#3303](https://github.com/badlogic/pi-mono/issues/3303)) ## [0.67.5] - 2026-04-16 diff --git a/packages/coding-agent/src/core/tools/find.ts b/packages/coding-agent/src/core/tools/find.ts index 92afc275..0a8cbc2a 100644 --- a/packages/coding-agent/src/core/tools/find.ts +++ b/packages/coding-agent/src/core/tools/find.ts @@ -4,7 +4,6 @@ import { Text } from "@mariozechner/pi-tui"; import { type Static, Type } from "@sinclair/typebox"; import { spawn } from "child_process"; import { existsSync } from "fs"; -import { glob } from "glob"; import path from "path"; import { keyHint } from "../../modes/interactive/components/keybinding-hints.js"; import { ensureTool } from "../../utils/tools-manager.js"; @@ -225,39 +224,17 @@ export function createFindToolDefinition( return; } - // Build fd arguments. + // Build fd arguments. --no-require-git makes fd apply hierarchical .gitignore + // semantics whether or not the search path is inside a git repository, without + // leaking sibling-directory rules the way --ignore-file (a global source) would. const args: string[] = [ "--glob", "--color=never", "--hidden", + "--no-require-git", "--max-results", String(effectiveLimit), ]; - // Include .gitignore files from the search tree. - const gitignoreFiles = new Set(); - const rootGitignore = path.join(searchPath, ".gitignore"); - if (existsSync(rootGitignore)) gitignoreFiles.add(rootGitignore); - try { - const nestedGitignores = await glob("**/.gitignore", { - cwd: searchPath, - dot: true, - absolute: true, - ignore: ["**/node_modules/**", "**/.git/**"], - signal, - }); - for (const file of nestedGitignores) gitignoreFiles.add(file); - } catch { - if (signal?.aborted) { - settle(() => reject(new Error("Operation aborted"))); - return; - } - // ignore - } - if (signal?.aborted) { - settle(() => reject(new Error("Operation aborted"))); - return; - } - for (const gitignorePath of gitignoreFiles) args.push("--ignore-file", gitignorePath); // 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 diff --git a/packages/coding-agent/test/suite/regressions/3303-find-nested-gitignore.test.ts b/packages/coding-agent/test/suite/regressions/3303-find-nested-gitignore.test.ts new file mode 100644 index 00000000..15c8658a --- /dev/null +++ b/packages/coding-agent/test/suite/regressions/3303-find-nested-gitignore.test.ts @@ -0,0 +1,83 @@ +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.js"; + +/** + * Regression test for https://github.com/badlogic/pi-mono/issues/3303 + * + * The `find` tool previously collected every `.gitignore` under the search + * path and passed them to `fd` via `--ignore-file`. fd treats `--ignore-file` + * entries as a single global ignore source, so rules from `a/.gitignore` + * also filtered files under sibling `b/`. The fix switches to fd's + * hierarchical `.gitignore` handling via `--no-require-git` and drops the + * manual collection. + */ +describe("issue #3303 nested .gitignore rules leak into sibling directories", () => { + let tempRoot: string; + + async function runFind(pattern: string): Promise { + const def = createFindToolDefinition(tempRoot); + const ctx = {} as Parameters[4]; + const result = (await def.execute("call-1", { pattern }, undefined, undefined, ctx)) as { + content: Array<{ type: string; text?: string }>; + }; + const text = result.content[0]?.text ?? ""; + if (text === "No files found matching pattern") return []; + return text + .split("\n") + .map((l) => l.trim()) + .filter((l) => l.length > 0 && !l.startsWith("[")) + .sort(); + } + + afterEach(() => { + if (tempRoot) rmSync(tempRoot, { recursive: true, force: true }); + }); + + describe("flat sibling case", () => { + beforeEach(() => { + tempRoot = mkdtempSync(join(tmpdir(), "pi-3303-flat-")); + mkdirSync(join(tempRoot, "a"), { recursive: true }); + mkdirSync(join(tempRoot, "b"), { recursive: true }); + writeFileSync(join(tempRoot, "a", ".gitignore"), "ignored.txt\n"); + writeFileSync(join(tempRoot, "a", "ignored.txt"), ""); + writeFileSync(join(tempRoot, "a", "kept.txt"), ""); + writeFileSync(join(tempRoot, "b", "ignored.txt"), ""); + writeFileSync(join(tempRoot, "b", "kept.txt"), ""); + writeFileSync(join(tempRoot, "root.txt"), ""); + }); + + it("applies a/.gitignore only inside a/ and leaves b/ untouched", async () => { + const files = await runFind("**/*.txt"); + expect(files).toEqual(["a/kept.txt", "b/ignored.txt", "b/kept.txt", "root.txt"]); + }); + }); + + describe("deeply nested case", () => { + beforeEach(() => { + tempRoot = mkdtempSync(join(tmpdir(), "pi-3303-deep-")); + mkdirSync(join(tempRoot, "a", "deep"), { recursive: true }); + mkdirSync(join(tempRoot, "b"), { recursive: true }); + writeFileSync(join(tempRoot, "a", ".gitignore"), "ignored.txt\n"); + writeFileSync(join(tempRoot, "a", "deep", ".gitignore"), "secret.txt\n"); + writeFileSync(join(tempRoot, "a", "ignored.txt"), ""); + writeFileSync(join(tempRoot, "a", "kept.txt"), ""); + writeFileSync(join(tempRoot, "a", "deep", "ignored.txt"), ""); + writeFileSync(join(tempRoot, "a", "deep", "secret.txt"), ""); + writeFileSync(join(tempRoot, "a", "deep", "kept.txt"), ""); + writeFileSync(join(tempRoot, "b", "ignored.txt"), ""); + writeFileSync(join(tempRoot, "b", "kept.txt"), ""); + writeFileSync(join(tempRoot, "root.txt"), ""); + }); + + it("scopes each .gitignore to its own subtree", async () => { + const files = await runFind("**/*.txt"); + // a/.gitignore ignores 'ignored.txt' within a/ and a/deep/. + // a/deep/.gitignore additionally ignores 'secret.txt' within a/deep/. + // b/ is untouched by either. + expect(files).toEqual(["a/deep/kept.txt", "a/kept.txt", "b/ignored.txt", "b/kept.txt", "root.txt"]); + }); + }); +});