fix(pi-fff): canonicalize find path constraints (#427)

closes #427

Canonicalize structured find path constraints before building the compact FFF query string. Absolute paths inside the active workspace now become repo-relative constraints, absolute paths outside the workspace fail clearly, and simple trailing recursive directory globs become directgory-prefix constraints while file globs remain unchanged.

This keeps the invariant at the pi-fff adapter boundary, where structured tool parameters are translated into FFF query syntax, and avoids changing core indexing, ranking, parser, or matcher behavior.

Author: edxeth

Makefile

@@ -78,6 +78,7 @@ prepare-node: build
 
 test-bun: prepare-bun
 	cd packages/fff-bun && bun test src/
+	cd packages/pi-fff && bun test test/
 
 test-node: prepare-node
 	cd packages/fff-node && npm run build && node test/e2e.mjs

packages/pi-fff/package.json

@@ -36,6 +36,7 @@
     "access": "public"
   },
   "scripts": {
+    "test": "bun test test/",
     "typecheck": "tsc --noEmit"
   },
   "dependencies": {

packages/pi-fff/src/index.ts

@@ -21,6 +21,7 @@ import type {
   SearchResult,
   MixedItem,
 } from "@ff-labs/fff-node";
+import { buildQuery } from "./query";
 
 // ---------------------------------------------------------------------------
 // Constants
@@ -104,65 +105,6 @@ function getFindCursor(id: string): FindCursor | undefined {
   return findCursorCache.get(id);
 }
 
-// ---------------------------------------------------------------------------
-// Query building helpers
-// ---------------------------------------------------------------------------
-
-function normalizePathConstraint(path: string): string | null {
-  let trimmed = path.trim();
-  if (!trimmed) return trimmed;
-  if (trimmed === "." || trimmed === "./") return null;
-  // Strip a leading `./` so `./**/*.rs` and `**/*.rs` behave identically.
-  if (trimmed.startsWith("./")) trimmed = trimmed.slice(2);
-  // 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.
-  if (/[*?\[{]/.test(trimmed)) return trimmed;
-  // Filename with extension (`main.rs`, `config.json`) → FilePath constraint.
-  const lastSegment = trimmed.split("/").pop() ?? "";
-  if (/\.[a-zA-Z][a-zA-Z0-9]{0,9}$/.test(lastSegment)) return trimmed;
-  // Bare directory prefix → append `/` so the parser sees a PathSegment.
-  return `${trimmed}/`;
-}
-
-// Exclusions are emitted as `!<constraint>` tokens, which the Rust parser
-// understands (crates/fff-query-parser/src/parser.rs). We normalize each one
-// the same way as the include path so bare dirs become PathSegment excludes.
-// Tolerate callers passing already-negated forms like `!src/` by stripping
-// the leading `!` before normalizing so we never double-negate (`!!src/`).
-function normalizeExcludes(exclude: string | string[] | undefined): string[] {
-  if (!exclude) return [];
-  const list = Array.isArray(exclude) ? exclude : [exclude];
-  const out: string[] = [];
-  for (const raw of list) {
-    const parts = raw
-      .split(/[,\s]+/)
-      .map((s) => s.trim())
-      .filter(Boolean);
-    for (const p of parts) {
-      const stripped = p.startsWith("!") ? p.slice(1) : p;
-      const normalized = normalizePathConstraint(stripped);
-      if (normalized) out.push(`!${normalized}`);
-    }
-  }
-  return out;
-}
-
-function buildQuery(
-  path: string | undefined,
-  pattern: string,
-  exclude?: string | string[],
-): string {
-  const parts: string[] = [];
-  if (path) {
-    const pathConstraint = normalizePathConstraint(path);
-    if (pathConstraint) parts.push(pathConstraint);
-  }
-  parts.push(...normalizeExcludes(exclude));
-  parts.push(pattern);
-  return parts.join(" ");
-}
-
 // ---------------------------------------------------------------------------
 // Output formatting helpers
 // ---------------------------------------------------------------------------
@@ -650,7 +592,7 @@ export default function fffExtension(pi: ExtensionAPI) {
 
       const f = await ensureFinder(activeCwd);
       const effectiveLimit = Math.max(1, params.limit ?? DEFAULT_GREP_LIMIT);
-      const query = buildQuery(params.path, params.pattern, params.exclude);
+      const 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.
@@ -829,7 +771,7 @@ export default function fffExtension(pi: ExtensionAPI) {
         : Math.max(1, params.limit ?? DEFAULT_FIND_LIMIT);
       const query = resumed
         ? resumed.query
-        : buildQuery(params.path, params.pattern, params.exclude);
+        : buildQuery(params.path, params.pattern, params.exclude, activeCwd);
       const pattern = resumed ? resumed.pattern : params.pattern;
       const pageIndex = resumed?.nextPageIndex ?? 0;
 

packages/pi-fff/src/query.ts

@@ -0,0 +1,87 @@
+import path from "node:path";
+
+export function normalizePathConstraint(
+  pathConstraint: string,
+  cwd = process.cwd(),
+): string | null {
+  let trimmed = pathConstraint.trim();
+  if (!trimmed) return trimmed;
+
+  if (path.isAbsolute(trimmed)) {
+    const relative = path.relative(cwd, trimmed).replaceAll(path.sep, "/");
+    if (relative === "") return null;
+    if (relative.startsWith("../") || relative === ".." || path.isAbsolute(relative)) {
+      throw new Error(
+        `Path constraint must be relative to the workspace: ${pathConstraint}`,
+      );
+    }
+    trimmed = relative;
+  }
+
+  if (trimmed === "." || trimmed === "./") return null;
+  // Strip a leading `./` so `./**/*.rs` and `**/*.rs` behave identically.
+  if (trimmed.startsWith("./")) trimmed = trimmed.slice(2);
+
+  // 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.
+  if (/[*?[{]/.test(trimmed)) return trimmed;
+  // Filename with extension (`main.rs`, `config.json`) → FilePath constraint.
+  const lastSegment = trimmed.split("/").pop() ?? "";
+  if (/\.[a-zA-Z][a-zA-Z0-9]{0,9}$/.test(lastSegment)) return trimmed;
+  // Bare directory prefix → append `/` so the parser sees a PathSegment.
+  return `${trimmed}/`;
+}
+
+// Exclusions are emitted as `!<constraint>` tokens, which the Rust parser
+// understands (crates/fff-query-parser/src/parser.rs). We normalize each one
+// the same way as the include path so bare dirs become PathSegment excludes.
+// Tolerate callers passing already-negated forms like `!src/` by stripping
+// the leading `!` before normalizing so we never double-negate (`!!src/`).
+export function normalizeExcludes(
+  exclude: string | string[] | undefined,
+  cwd = process.cwd(),
+): string[] {
+  if (!exclude) return [];
+  const list = Array.isArray(exclude) ? exclude : [exclude];
+  const out: string[] = [];
+  for (const raw of list) {
+    const parts = raw
+      .split(/[,\s]+/)
+      .map((s) => s.trim())
+      .filter(Boolean);
+    for (const p of parts) {
+      const stripped = p.startsWith("!") ? p.slice(1) : p;
+      const normalized = normalizePathConstraint(stripped, cwd);
+      if (normalized) out.push(`!${normalized}`);
+    }
+  }
+  return out;
+}
+
+export function buildQuery(
+  path: string | undefined,
+  pattern: string,
+  exclude?: string | string[],
+  cwd = process.cwd(),
+): string {
+  const parts: string[] = [];
+  if (path) {
+    const pathConstraint = normalizePathConstraint(path, cwd);
+    if (pathConstraint) parts.push(pathConstraint);
+  }
+  parts.push(...normalizeExcludes(exclude, cwd));
+  parts.push(pattern);
+  return parts.join(" ");
+}

packages/pi-fff/test/query.test.ts

@@ -0,0 +1,32 @@
+import { describe, expect, test } from "bun:test";
+import { buildQuery, normalizePathConstraint } from "../src/query";
+
+const cwd = "/tmp/workspace";
+
+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/plans/**", cwd)).toBe(
+      ".agents/plans/",
+    );
+  });
+
+  test("rejects absolute paths outside the workspace", () => {
+    expect(() => normalizePathConstraint("/tmp/other/.agents/**", cwd)).toThrow(
+      "Path constraint must be relative to the workspace",
+    );
+  });
+
+  test("collapses only simple trailing recursive directory globs", () => {
+    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}/**");
+  });
+
+  test("builds find queries with normalized include and exclude constraints", () => {
+    expect(
+      buildQuery("/tmp/workspace/.agents/**", "*", "/tmp/workspace/test/**", cwd),
+    ).toBe(".agents/ !test/ *");
+  });
+});