From a80b8efa149c099f13e73932ab321d92b982b238 Mon Sep 17 00:00:00 2001 From: Sameer Reddy Date: Wed, 9 Sep 2026 13:58:04 -0700 Subject: [PATCH] Fix pi-fff path constraints and configurable excludes --- packages/pi-fff/pi-fff.schema.json | 6 +++++ packages/pi-fff/src/config.ts | 18 +++++++++++++++ packages/pi-fff/src/index.ts | 20 +++++++++++++--- packages/pi-fff/src/query.ts | 31 +++++++++++++++---------- packages/pi-fff/test/config.test.ts | 9 ++++++++ packages/pi-fff/test/query.test.ts | 36 ++++++++++++++++++++++++----- 6 files changed, 99 insertions(+), 21 deletions(-) diff --git a/packages/pi-fff/pi-fff.schema.json b/packages/pi-fff/pi-fff.schema.json index 5f2783a4e..748cecc2d 100644 --- a/packages/pi-fff/pi-fff.schema.json +++ b/packages/pi-fff/pi-fff.schema.json @@ -46,6 +46,12 @@ "type": "boolean", "default": true, "description": "Indexes through directory symlinks, e.g. a git worktree or stow layout whose files live behind links. Set to false to keep the walk inside the real tree." + }, + "defaultExcludes": { + "type": "array", + "items": { "type": "string", "minLength": 1 }, + "default": [], + "description": "Path constraints to exclude from every find and grep query unless the query path explicitly targets one of them." } } } diff --git a/packages/pi-fff/src/config.ts b/packages/pi-fff/src/config.ts index 8e12f9d0f..ffe1ef6ab 100644 --- a/packages/pi-fff/src/config.ts +++ b/packages/pi-fff/src/config.ts @@ -16,6 +16,7 @@ export interface FffConfig { enableHomeDirScanning?: boolean; warnOnHomeDirScan?: boolean; followSymlinks?: boolean; + defaultExcludes?: string[]; } const CONFIG_KEYS = new Set([ @@ -27,6 +28,7 @@ const CONFIG_KEYS = new Set([ "enableHomeDirScanning", "warnOnHomeDirScan", "followSymlinks", + "defaultExcludes", ]); export function loadConfig(agentDir = piDataDir()): FffConfig { @@ -70,6 +72,7 @@ export function loadConfig(agentDir = piDataDir()): FffConfig { validateBoolean(configPath, parsed, "enableHomeDirScanning"); validateBoolean(configPath, parsed, "warnOnHomeDirScan"); validateBoolean(configPath, parsed, "followSymlinks"); + validateStringArray(configPath, parsed, "defaultExcludes"); return parsed as FffConfig; } @@ -111,3 +114,18 @@ function validateBoolean( throw invalidConfig(configPath, `"${key}" must be a boolean`); } } + +function validateStringArray( + configPath: string, + config: Record, + key: "defaultExcludes", +): void { + const value = config[key]; + if (value === undefined) return; + if ( + !Array.isArray(value) || + value.some((item) => typeof item !== "string" || item.length === 0) + ) { + throw invalidConfig(configPath, `"${key}" must be an array of non-empty strings`); + } +} diff --git a/packages/pi-fff/src/index.ts b/packages/pi-fff/src/index.ts index 75d93cfe3..44aed6ebc 100644 --- a/packages/pi-fff/src/index.ts +++ b/packages/pi-fff/src/index.ts @@ -350,6 +350,7 @@ export default function fffExtension(pi: ExtensionAPI) { let enableHomeDirScanning = true; let warnOnHomeDirScan = true; let followSymlinks = true; + let defaultExcludes: string[] = []; function setMode(mode: FffMode): void { currentMode = mode; @@ -408,6 +409,7 @@ export default function fffExtension(pi: ExtensionAPI) { true, parseBoolean, ); + defaultExcludes = config.defaultExcludes ?? []; } function getMode(): FffMode { @@ -550,7 +552,13 @@ export default function fffExtension(pi: ExtensionAPI) { // constraint stays relative to the picker's actual root. const rebase = nodePath.relative(aux.root, route.root).replaceAll(nodePath.sep, "/"); const suffix = [rebase, route.suffix].filter(Boolean).join("/"); - const query = buildQuery(suffix || undefined, pattern, exclude, aux.root); + const query = buildQuery( + suffix || undefined, + pattern, + exclude, + aux.root, + defaultExcludes, + ); return { finder: aux.finder, query, root: aux.root }; } @@ -878,7 +886,7 @@ export default function fffExtension(pi: ExtensionAPI) { const context = clampContext(params.context); const query = aux ? aux.query - : buildQuery(params.path, pattern, params.exclude, activeCwd); + : buildQuery(params.path, pattern, params.exclude, activeCwd, defaultExcludes); // Auto-detect: regex if the pattern has regex metacharacters AND parses // as a valid regex, otherwise plain literal. The fuzzy fallback below @@ -1080,7 +1088,13 @@ export default function fffExtension(pi: ExtensionAPI) { ? resumed.query : aux && "query" in aux ? (aux as { query: string }).query - : buildQuery(params.path, params.pattern, params.exclude, activeCwd); + : buildQuery( + params.path, + params.pattern, + params.exclude, + activeCwd, + defaultExcludes, + ); const pattern = resumed ? resumed.pattern : params.pattern; const pageIndex = resumed?.nextPageIndex ?? 0; diff --git a/packages/pi-fff/src/query.ts b/packages/pi-fff/src/query.ts index 501d8333d..c82faf0a0 100644 --- a/packages/pi-fff/src/query.ts +++ b/packages/pi-fff/src/query.ts @@ -25,17 +25,6 @@ export function normalizePathConstraint( // wif we left with the ** it means anything so treat it as a cwd path if (trimmed === "**" || trimmed === "**/" || trimmed === "**/*") return null; - // FFF's glob matcher can treat a hidden directory root glob such as - // `.agents/**` as empty, while the tool contract says this means "inside - // this directory". Collapse simple trailing recursive directory globs to the - // directory-prefix constraint understood by the parser. Keep real file globs - // such as `src/**/*.ts` unchanged. - const recursiveDir = trimmed.match(/^(.*)\/\*\*(?:\/\*)?$/); - if (recursiveDir) { - const dir = recursiveDir[1]; - if (dir && !/[*?[{]/.test(dir)) return `${dir}/`; - } - // Already signals path-constraint syntax to the parser. if (trimmed.startsWith("/") || trimmed.endsWith("/")) return trimmed; // Globs (`*.ts`, `src/**/*.cc`, `{src,lib}`) are handled by the parser. @@ -73,18 +62,36 @@ export function normalizeExcludes( return out; } +function pathTargetsExcludedRoot( + pathConstraint: string | undefined, + excludes: string[], + cwd: string, +): boolean { + if (!pathConstraint || excludes.length === 0) return false; + const normalized = normalizePathConstraint(pathConstraint, cwd); + if (!normalized) return false; + return excludes.some( + (exclude) => normalized === exclude || normalized.startsWith(exclude), + ); +} + export function buildQuery( path: string | undefined, pattern: string, exclude?: string | string[], cwd = process.cwd(), + defaultExcludes: string[] = [], ): string { const parts: string[] = []; if (path) { const pathConstraint = normalizePathConstraint(path, cwd); if (pathConstraint) parts.push(pathConstraint); } - parts.push(...normalizeExcludes(exclude, cwd)); + const activeDefaultExcludes = pathTargetsExcludedRoot(path, defaultExcludes, cwd) + ? [] + : defaultExcludes; + const callerExcludes = exclude ? (Array.isArray(exclude) ? exclude : [exclude]) : []; + parts.push(...normalizeExcludes([...activeDefaultExcludes, ...callerExcludes], cwd)); parts.push(pattern); return parts.join(" "); } diff --git a/packages/pi-fff/test/config.test.ts b/packages/pi-fff/test/config.test.ts index e21f3442a..2248d0e27 100644 --- a/packages/pi-fff/test/config.test.ts +++ b/packages/pi-fff/test/config.test.ts @@ -33,6 +33,7 @@ describe("loadConfig", () => { enableHomeDirScanning: false, warnOnHomeDirScan: false, followSymlinks: true, + defaultExcludes: [".worktrees/", ".claude/worktrees/"], }; writeConfig(config); @@ -70,6 +71,14 @@ describe("loadConfig", () => { [{ enableHomeDirScanning: "false" }, '"enableHomeDirScanning" must be a boolean'], [{ warnOnHomeDirScan: "false" }, '"warnOnHomeDirScan" must be a boolean'], [{ followSymlinks: "true" }, '"followSymlinks" must be a boolean'], + [ + { defaultExcludes: "test/" }, + '"defaultExcludes" must be an array of non-empty strings', + ], + [ + { defaultExcludes: ["test/", ""] }, + '"defaultExcludes" must be an array of non-empty strings', + ], ]; for (const [config, message] of cases) { diff --git a/packages/pi-fff/test/query.test.ts b/packages/pi-fff/test/query.test.ts index 50e03741c..6ef5e5270 100644 --- a/packages/pi-fff/test/query.test.ts +++ b/packages/pi-fff/test/query.test.ts @@ -2,12 +2,13 @@ import { describe, expect, test } from "bun:test"; import { buildQuery, normalizePathConstraint } from "../src/query"; const cwd = "/tmp/workspace"; +const defaultExcludes = [".worktrees/", ".claude/worktrees/"]; describe("path constraint normalization", () => { test("converts absolute in-workspace paths to repo-relative constraints", () => { - expect(normalizePathConstraint("/tmp/workspace/.agents/**", cwd)).toBe(".agents/"); + expect(normalizePathConstraint("/tmp/workspace/.agents/**", cwd)).toBe(".agents/**"); expect(normalizePathConstraint("/tmp/workspace/.agents/plans/**", cwd)).toBe( - ".agents/plans/", + ".agents/plans/**", ); }); @@ -17,9 +18,9 @@ describe("path constraint normalization", () => { ); }); - test("collapses only simple trailing recursive directory globs", () => { - expect(normalizePathConstraint(".agents/**", cwd)).toBe(".agents/"); - expect(normalizePathConstraint("src/**/*", cwd)).toBe("src/"); + test("preserves recursive directory globs as root-relative constraints", () => { + expect(normalizePathConstraint(".agents/**", cwd)).toBe(".agents/**"); + expect(normalizePathConstraint("src/**/*", cwd)).toBe("src/**/*"); expect(normalizePathConstraint("src/**/*.ts", cwd)).toBe("src/**/*.ts"); expect(normalizePathConstraint("{src,lib}/**", cwd)).toBe("{src,lib}/**"); }); @@ -27,7 +28,7 @@ describe("path constraint normalization", () => { test("builds find queries with normalized include and exclude constraints", () => { expect( buildQuery("/tmp/workspace/.agents/**", "*", "/tmp/workspace/test/**", cwd), - ).toBe(".agents/ !test/ *"); + ).toBe(".agents/** !test/** *"); }); test("treats path='.' as workspace root (no constraint)", () => { @@ -77,3 +78,26 @@ describe("path constraint normalization", () => { expect(buildQuery("**", "needle", undefined, cwd)).toBe("needle"); }); }); + +describe("default excludes", () => { + test("adds configured excludes to ordinary queries", () => { + expect(buildQuery("src/**", "needle", undefined, cwd, defaultExcludes)).toBe( + "src/** !.worktrees/ !.claude/worktrees/ needle", + ); + }); + + test("combines configured and caller excludes", () => { + expect(buildQuery("src/**", "needle", "test/", cwd, defaultExcludes)).toBe( + "src/** !.worktrees/ !.claude/worktrees/ !test/ needle", + ); + }); + + test("does not apply an exclude when path explicitly targets it", () => { + expect( + buildQuery(".worktrees/demo/**", "needle", undefined, cwd, defaultExcludes), + ).toBe(".worktrees/demo/** needle"); + expect( + buildQuery(".claude/worktrees/demo/**", "needle", undefined, cwd, defaultExcludes), + ).toBe(".claude/worktrees/demo/** needle"); + }); +});