From d694897544461695356cf0ff7beb74787024fe70 Mon Sep 17 00:00:00 2001 From: gustav-fff <286169375+gustav-fff@users.noreply.github.com> Date: Sun, 21 Jun 2026 11:56:56 -0700 Subject: [PATCH] fix(pi-fff): route out-of-workspace path constraints to a rotating aux finder pool Hotfix prototype for #463. When the agent passes an absolute `path` outside the workspace cwd to ffgrep/fffind, spin up (or reuse) a FileFinder rooted at that path instead of throwing "Path constraint must be relative to the workspace". Pool keeps at most 3 aux finders, LRU-evicted, dropped after 5 minutes of inactivity. Find pagination cursors carry the aux root so resumes hit the same finder. --- packages/pi-fff/src/aux-finders.ts | 126 +++++++++++++++++++++++ packages/pi-fff/src/index.ts | 58 ++++++++++- packages/pi-fff/test/aux-finders.test.ts | 36 +++++++ 3 files changed, 215 insertions(+), 5 deletions(-) create mode 100644 packages/pi-fff/src/aux-finders.ts create mode 100644 packages/pi-fff/test/aux-finders.test.ts diff --git a/packages/pi-fff/src/aux-finders.ts b/packages/pi-fff/src/aux-finders.ts new file mode 100644 index 00000000..576d934f --- /dev/null +++ b/packages/pi-fff/src/aux-finders.ts @@ -0,0 +1,126 @@ +import fs from "node:fs"; +import path from "node:path"; +import { FileFinder } from "@ff-labs/fff-node"; + +// Hotfix prototype for issue #463: agent occasionally needs to grep/find +// outside the workspace cwd. Maintain a tiny rotating pool of additional +// FileFinder instances rooted at out-of-workspace paths. LRU-evicted at +// MAX_AUX, dropped after IDLE_TTL_MS of no use. + +export const MAX_AUX = 3; +export const IDLE_TTL_MS = 5 * 60 * 1000; + +interface AuxEntry { + root: string; + finder: FileFinder; + lastUsed: number; +} + +export interface AuxOpts { + frecencyDbPath?: string; + historyDbPath?: string; + enableFsRootScanning: boolean; +} + +export class AuxFinderPool { + private entries: AuxEntry[] = []; + constructor(private opts: AuxOpts) {} + + private sweepIdle(now = Date.now()): void { + const kept: AuxEntry[] = []; + for (const e of this.entries) { + if (now - e.lastUsed > IDLE_TTL_MS) { + if (!e.finder.isDestroyed) e.finder.destroy(); + } else { + kept.push(e); + } + } + this.entries = kept; + } + + async acquire(root: string): Promise { + this.sweepIdle(); + const existing = this.entries.find((e) => e.root === root); + if (existing && !existing.finder.isDestroyed) { + existing.lastUsed = Date.now(); + return existing.finder; + } + if (this.entries.length >= MAX_AUX) { + let oldest = this.entries[0]; + for (const e of this.entries) if (e.lastUsed < oldest.lastUsed) oldest = e; + if (!oldest.finder.isDestroyed) oldest.finder.destroy(); + this.entries = this.entries.filter((e) => e !== oldest); + } + const result = FileFinder.create({ + basePath: root, + frecencyDbPath: this.opts.frecencyDbPath, + historyDbPath: this.opts.historyDbPath, + aiMode: true, + enableHomeDirScanning: true, + enableFsRootScanning: this.opts.enableFsRootScanning, + }); + if (!result.ok) + throw new Error(`Failed to create aux file finder for ${root}: ${result.error}`); + await result.value.waitForScan(15000); + this.entries.push({ root, finder: result.value, lastUsed: Date.now() }); + return result.value; + } + + destroyAll(): void { + for (const e of this.entries) if (!e.finder.isDestroyed) e.finder.destroy(); + this.entries = []; + } + + // Exposed for tests/diagnostics. + size(): number { + this.sweepIdle(); + return this.entries.length; + } +} + +// Split an absolute path into the longest non-glob directory prefix (the aux +// root) and a remainder usable as a path constraint relative to that root. +// Returns null if the prefix doesn't exist on disk. +export function resolveAuxRoot( + absPath: string, +): { root: string; suffix: string } | null { + const normalized = path.normalize(absPath.trim()); + if (!path.isAbsolute(normalized)) return null; + const parts = normalized.split(path.sep); + const firstGlob = parts.findIndex((p) => /[*?[{]/.test(p)); + let rootPath: string; + let suffix: string; + if (firstGlob === -1) { + rootPath = normalized; + suffix = ""; + } else { + rootPath = parts.slice(0, firstGlob).join(path.sep) || path.sep; + suffix = parts.slice(firstGlob).join("/"); + } + const stripped = rootPath.replace(/\/+$/, "") || "/"; + let stat: fs.Stats; + try { + stat = fs.statSync(stripped); + } catch { + return null; + } + if (stat.isFile()) { + return { root: path.dirname(stripped), suffix: path.basename(stripped) }; + } + return { root: stripped, suffix }; +} + +// Decide whether a `path` parameter should route to the workspace finder or +// to an aux finder. An absolute path is "outside workspace" when the relative +// from cwd starts with `..`. Returns null to signal "no rerouting". +export function routePathConstraint( + pathConstraint: string | undefined, + cwd: string, +): { root: string; suffix: string } | null { + if (!pathConstraint) return null; + const trimmed = pathConstraint.trim(); + if (!trimmed || !path.isAbsolute(trimmed)) return null; + const rel = path.relative(cwd, trimmed); + if (!rel.startsWith("..") && rel !== "..") return null; + return resolveAuxRoot(trimmed); +} diff --git a/packages/pi-fff/src/index.ts b/packages/pi-fff/src/index.ts index 7c8091f4..684dc546 100644 --- a/packages/pi-fff/src/index.ts +++ b/packages/pi-fff/src/index.ts @@ -20,6 +20,7 @@ import type { } from "@ff-labs/fff-node"; import { FileFinder } from "@ff-labs/fff-node"; import { Type } from "@sinclair/typebox"; +import { AuxFinderPool, routePathConstraint } from "./aux-finders"; import { buildQuery } from "./query"; // --------------------------------------------------------------------------- @@ -85,6 +86,7 @@ interface FindCursor { pattern: string; pageSize: number; nextPageIndex: number; + auxRoot?: string; } const findCursorCache = new Map(); @@ -285,6 +287,7 @@ export default function fffExtension(pi: ExtensionAPI) { // deadlock at the native layer (issue #403). let finderPromise: Promise | null = null; let activeCwd = process.cwd(); + let auxPool: AuxFinderPool | null = null; // Mode resolution: flag > env > default let currentMode: FffMode = @@ -372,6 +375,36 @@ export default function fffExtension(pi: ExtensionAPI) { finder = null; finderCwd = null; } + if (auxPool) { + auxPool.destroyAll(); + auxPool = null; + } + } + + function getAuxPool(): AuxFinderPool { + if (!auxPool) { + auxPool = new AuxFinderPool({ + frecencyDbPath, + historyDbPath, + enableFsRootScanning, + }); + } + return auxPool; + } + + // Out-of-workspace path constraint -> route to a pooled aux FileFinder + // rooted at the requested directory. Returns null when the workspace + // finder should handle the call normally. + async function resolveFinderForPath( + pathParam: string | undefined, + pattern: string, + exclude: string | string[] | undefined, + ): Promise<{ finder: FileFinder; query: string; root: string } | null> { + const route = routePathConstraint(pathParam, activeCwd); + if (!route) return null; + const aux = await getAuxPool().acquire(route.root); + const query = buildQuery(route.suffix || undefined, pattern, exclude, route.root); + return { finder: aux, query, root: route.root }; } async function getMentionItems( @@ -587,9 +620,16 @@ export default function fffExtension(pi: ExtensionAPI) { async execute(_toolCallId, params, signal) { if (signal?.aborted) throw new Error("Operation aborted"); - const f = await ensureFinder(activeCwd); + const aux = await resolveFinderForPath( + params.path, + params.pattern, + params.exclude, + ); + const f = aux ? aux.finder : await ensureFinder(activeCwd); const effectiveLimit = Math.max(1, params.limit ?? DEFAULT_GREP_LIMIT); - const query = buildQuery(params.path, params.pattern, params.exclude, activeCwd); + const query = aux + ? aux.query + : buildQuery(params.path, params.pattern, params.exclude, activeCwd); // Auto-detect: regex if the pattern has regex metacharacters AND parses // as a valid regex, otherwise plain literal. The fuzzy fallback below // only kicks in for plain mode — regex queries are intentional. @@ -752,19 +792,26 @@ export default function fffExtension(pi: ExtensionAPI) { async execute(_toolCallId, params, signal) { if (signal?.aborted) throw new Error("Operation aborted"); - const f = await ensureFinder(activeCwd); - // Resume from a prior cursor if supplied — cursor owns query+pageSize so // the agent can't accidentally mix patterns across pages. const resumed = params.cursor ? getFindCursor(params.cursor) : undefined; + const aux = resumed + ? resumed.auxRoot + ? { finder: await getAuxPool().acquire(resumed.auxRoot), root: resumed.auxRoot } + : null + : await resolveFinderForPath(params.path, params.pattern, params.exclude); + const f = aux ? aux.finder : await ensureFinder(activeCwd); const effectiveLimit = resumed ? resumed.pageSize : Math.max(1, params.limit ?? DEFAULT_FIND_LIMIT); const query = resumed ? resumed.query - : buildQuery(params.path, params.pattern, params.exclude, activeCwd); + : aux && "query" in aux + ? (aux as { query: string }).query + : buildQuery(params.path, params.pattern, params.exclude, activeCwd); const pattern = resumed ? resumed.pattern : params.pattern; const pageIndex = resumed?.nextPageIndex ?? 0; + const auxRoot = resumed?.auxRoot ?? aux?.root; const searchResult = f.fileSearch(query, { pageIndex, @@ -796,6 +843,7 @@ export default function fffExtension(pi: ExtensionAPI) { pattern, pageSize: effectiveLimit, nextPageIndex: pageIndex + 1, + auxRoot, }); notices.push( `${remaining} more match${remaining === 1 ? "" : "es"} available. cursor="${cursorId}" to continue`, diff --git a/packages/pi-fff/test/aux-finders.test.ts b/packages/pi-fff/test/aux-finders.test.ts new file mode 100644 index 00000000..d64c32fe --- /dev/null +++ b/packages/pi-fff/test/aux-finders.test.ts @@ -0,0 +1,36 @@ +import { describe, expect, test } from "bun:test"; +import { resolveAuxRoot, routePathConstraint } from "../src/aux-finders"; + +describe("routePathConstraint", () => { + const cwd = "/tmp/workspace"; + + test("returns null for relative paths", () => { + expect(routePathConstraint("src/", cwd)).toBeNull(); + expect(routePathConstraint("**/*.ts", cwd)).toBeNull(); + expect(routePathConstraint(undefined, cwd)).toBeNull(); + }); + + test("returns null for absolute paths inside the workspace", () => { + expect(routePathConstraint("/tmp/workspace/src", cwd)).toBeNull(); + }); + + test("routes absolute paths outside the workspace", () => { + const route = routePathConstraint("/tmp", cwd); + expect(route).toEqual({ root: "/tmp", suffix: "" }); + }); + + test("splits glob suffix from existing dir prefix", () => { + const route = routePathConstraint("/tmp/**/*.ts", cwd); + expect(route).toEqual({ root: "/tmp", suffix: "**/*.ts" }); + }); + + test("returns null when prefix does not exist on disk", () => { + expect(routePathConstraint("/tmp/__nonexistent_dir_xyz__", cwd)).toBeNull(); + }); +}); + +describe("resolveAuxRoot", () => { + test("returns null for relative input", () => { + expect(resolveAuxRoot("src/")).toBeNull(); + }); +});