Skip to content

fix(pi-fff): use exact aux finder for external paths - #713

Open
WodenJay wants to merge 1 commit into
dmtrKovalenko:mainfrom
WodenJay:fix/pi-fff-exact-aux-finder
Open

fix(pi-fff): use exact aux finder for external paths#713
WodenJay wants to merge 1 commit into
dmtrKovalenko:mainfrom
WodenJay:fix/pi-fff-exact-aux-finder

Conversation

@WodenJay

Copy link
Copy Markdown

Closes #711.

Root cause

resolveFinderForPath() reused any cached auxiliary finder whose root covered the requested route.root.

A broader finder such as ~/.pi can exclude nested directories like node_modules while building its index. Rebasing the query suffix relative to that broader root cannot recover files that were never indexed. When the exact search returned no matches, fuzzy fallback could also return unrelated files from the broader index.

Fix

Acquire the exact requested auxiliary root for explicit paths outside the workspace:

const aux = await auxPool.acquire(route.root, { exact: true });

Since exact acquisition guarantees that the finder is rooted at route.root, build the query directly from route.suffix and remove the obsolete ancestor-root rebasing logic.

Add an extension-level regression test that first caches a broader finder, then searches an absolute file path under its ignored node_modules subtree. The test verifies that a finder rooted at the target directory is created and the requested file is returned.

Steps to reproduce

Start Pi in a workspace outside ~/.pi, ensure an auxiliary finder rooted at ~/.pi has already been cached, then run:

{
  "pattern": "randomUUID",
  "path": "/Users/example/.pi/agent/npm/node_modules/@tintinweb/pi-subagents/src/agent-manager.ts",
  "limit": 3
}
  • Pre-fix: the cached ~/.pi finder is reused, but its index excludes node_modules; the requested file is missed and fuzzy fallback may return unrelated files.
  • Post-fix: a finder rooted at the target file's directory is created or reused, and matches from the requested file are returned.

How verified

  • cd packages/pi-fff && bun test test/extension.test.ts — 9 passed
  • cd packages/pi-fff && bun run typecheck

The full pi-fff suite on Windows reported 38 passed and 9 existing failures in aux-finders.test.ts and aux-pool.test.ts; those tests use hard-coded POSIX paths such as /tmp and /a/b.

Copilot AI review requested due to automatic review settings July 25, 2026 09:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes incorrect reuse of “covering” auxiliary finders for explicit external paths by requiring an exact-root aux finder, preventing false negatives (and unrelated fuzzy fallback results) when broader aux roots exclude the requested subtree (e.g. node_modules).

Changes:

  • Acquire auxiliary finders with { exact: true } for outside-workspace path routing and simplify query construction by removing ancestor-root rebasing.
  • Add a regression test that warms a broad aux finder, then verifies an explicit external file path forces creation/use of an aux finder rooted at the target directory.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
packages/pi-fff/src/index.ts Switches external-path routing to exact aux-root acquisition and removes rebasing logic so the query matches the requested root.
packages/pi-fff/test/extension.test.ts Adds an extension-level regression test ensuring explicit external file paths create/reuse an exact-root finder and return matches from the target file.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +361 to +363
const grep = tools.get("ffgrep");
await grep.execute("warmup", { pattern: "warmup", path: broadRoot });
const result = await grep.execute("repro", {
@dmtrKovalenko

Copy link
Copy Markdown
Owner

@gustav-fff review

@gustav-fff

Copy link
Copy Markdown
Collaborator

[triage-bot] DIRECTED:

Reviewed dc9301f.

Correctness. Fix is right for #711. Rebasing a suffix onto a broader cached finder can never recover files the broader index excluded via gitignore (node_modules under ~/.pi). Forcing exact: true at packages/pi-fff/src/index.ts:406 and dropping the rebase math is the correct call. find resume path at index.ts:846 uses the same exact: true — consistent.

Perf tradeoff worth flagging. With MAX_AUX = 3 (aux-finders.ts:7), every distinct route.root spawns a fresh finder + waitForScan. Previously one ~/.pi covered a whole subtree; now a workflow hopping across ~/.pi/projA/src/, ~/.pi/projB/lib/, ~/.pi/projC/pkg/ churns the pool (each acquire evicts oldest, rebuilds index, blocks on scan). Acceptable price for correctness, but if agents commonly grep 4+ sibling external roots in one session the cache thrashes. Consider bumping MAX_AUX or keeping a covering-finder path as fallback only when the exact scan is empty.

Nit (Copilot's point, valid). packages/pi-fff/test/extension.test.ts:361tools.get(\"ffgrep\") can return undefined; an explicit assertion before .execute makes future failures readable instead of a cannot read execute stack. Trivial change.

Cross-repo PR from a fork — cannot push. Suggested diff:

```ts
const grep = tools.get("ffgrep");
expect(grep).toBeDefined();
await grep!.execute("warmup", { pattern: "warmup", path: broadRoot });
```

Verified locally: `bun test test/extension.test.ts` — 9 pass. Typecheck skipped (`tsc` unavailable in triage env; PR author already ran it).

Honk-Honk 🪿

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: pi-fff reuses broader aux finder that excludes explicit absolute-path target

4 participants