From 3820c16564aeeca284d1840ae52d010db045afca Mon Sep 17 00:00:00 2001 From: Orc Date: Fri, 3 Jul 2026 20:09:35 +0200 Subject: [PATCH 1/2] Fix: Windows FileFinder triggers OneDrive sync for cloud-only files --- FIX_SUMMARY.md | 77 +++++++++++++++++++++++++++++++++++++++ src/fff-runtime.ts | 18 +++++++++ tests/fff-runtime.test.ts | 9 +++++ 3 files changed, 104 insertions(+) create mode 100644 FIX_SUMMARY.md diff --git a/FIX_SUMMARY.md b/FIX_SUMMARY.md new file mode 100644 index 0000000..25e3ea4 --- /dev/null +++ b/FIX_SUMMARY.md @@ -0,0 +1,77 @@ +# Fix for Issue #6: Windows OneDrive Sync Problem + +## Problem Summary + +When pi opens a terminal on Windows and the working directory is `C:\Users\` (no `.git` found), pi-fff's FileFinder indexes the **entire home directory** as the project root. This causes: + +- Unwanted bandwidth consumption +- Disk space bloat (cloud-only files become local) +- Slow scanning while OneDrive downloads large files +- Unnecessary OneDrive sync activity + +## Root Cause + +1. `resolveProjectRoot()` walks up from cwd looking for `.git`. When none is found, it falls back to `cwd`. +2. `FileFinder.create({ basePath: projectRoot })` then starts scanning all files under the home directory. +3. On Windows, OneDrive Files On-Demand keeps cloud-only files as lightweight placeholders (reparse points). When FFF's background scanner/watcher accesses these files, the Windows filesystem triggers OneDrive's hydration — downloading the actual file content from the cloud. + +## Solution Implemented + +Added a check in the `initialize()` method of `FffRuntime` to prevent scanning the home directory: + +1. **New helper function** `isHomeDirectory(path: string): boolean` + - Compares the resolved path with the user's home directory + - Uses Node.js `homedir()` and `resolve()` for cross-platform compatibility + +2. **Validation in `initialize()` method** + - After resolving the project root, check if it's the home directory + - If it is, return an error with a helpful message + - Prevents FileFinder from being created and scanning the home directory + +3. **Error message** + - Clear and actionable: "Cannot index the home directory. Please navigate to a specific project directory instead." + - Uses existing `RuntimeInitializationError` type for consistency + +4. **Test coverage** + - Added test `initialize rejects home directory as project root` + - Verifies that initialization fails when cwd is the home directory + - Checks that the error message contains appropriate guidance + +## Changes Made + +### `src/fff-runtime.ts` +- Added `isHomeDirectory()` helper function (lines 68-71) +- Added home directory validation in `initialize()` method (lines 834-845) + +### `tests/fff-runtime.test.ts` +- Added test case for home directory rejection (lines 614-622) + +## Benefits + +1. **Prevents OneDrive sync issues**: On Windows, cloud-only files in OneDrive won't trigger unwanted downloads +2. **Performance**: Avoids indexing large home directories with thousands of files +3. **User experience**: Clear error message guides users to navigate to a proper project directory +4. **Cross-platform**: Works on Windows, macOS, and Linux (though the OneDrive issue is Windows-specific) +5. **Minimal code change**: Only 18 lines added, no breaking changes to existing functionality + +## Alternative Approaches Considered + +1. **Add `enableHomeDirScanning` option to FileFinder**: The `@ff-labs/fff-node` package already has this option, but it defaults to `false`. Our fix adds an explicit check in pi-fff to provide a better error message and prevent the issue at the application layer. + +2. **Exclude OneDrive directory**: This would require platform-specific logic and wouldn't solve the general problem of indexing the entire home directory. + +3. **Use `enableHomeDirScanning` option**: We could pass this option, but it's better to explicitly reject home directory scanning at the application level with a clear error message. + +## Testing + +All existing tests pass, plus the new test specifically for home directory rejection: +- Typecheck: ✓ +- All 29 tests: ✓ +- New test for home directory rejection: ✓ + +## Backward Compatibility + +This is a **breaking change** for users who currently use pi-fff from their home directory. However: +- This is an edge case that causes significant problems (OneDrive sync issues) +- The error message provides clear guidance on how to proceed +- The fix prevents a more serious problem (unwanted file downloads and performance issues) \ No newline at end of file diff --git a/src/fff-runtime.ts b/src/fff-runtime.ts index 528d4f7..079512d 100644 --- a/src/fff-runtime.ts +++ b/src/fff-runtime.ts @@ -65,6 +65,11 @@ async function getPathType(path: string): Promise<"file" | "directory" | null> { } } +function isHomeDirectory(path: string): boolean { + const home = homedir(); + return resolve(path) === resolve(home); +} + function normalizeSlashes(value: string): string { return value.replace(/\\/g, "/"); } @@ -826,6 +831,19 @@ export class FffRuntime { const projectRoot = this.options.projectRoot ?? await resolveProjectRoot(this.cwd); this.basePath = projectRoot; + + // Prevent scanning the home directory to avoid triggering OneDrive sync + // on Windows and to prevent indexing excessive files + if (isHomeDirectory(projectRoot)) { + return errResult( + new RuntimeInitializationError({ + cwd: this.cwd, + step: "validate project root", + cause: "Cannot index the home directory. Please navigate to a specific project directory instead.", + }), + ); + } + const paths = getProjectDatabasePaths(root, projectRoot); const dbDir = paths.dbDir; const dbResult = await Result.tryPromise({ diff --git a/tests/fff-runtime.test.ts b/tests/fff-runtime.test.ts index df47074..565cc5f 100644 --- a/tests/fff-runtime.test.ts +++ b/tests/fff-runtime.test.ts @@ -611,3 +611,12 @@ test("grepSearch treats dot scope as project root", async () => { assert.equal(result.value.items.length, 1); assert.equal(result.value.items[0]?.relativePath, "b/c"); }); + +test("initialize rejects home directory as project root", async () => { + const runtime = new FffRuntime(homedir()); + const result = await runtime.ensure(); + assert.equal(result.isOk(), false); + if (result.isOk()) assert.fail("Expected initialization to fail"); + assert.match(result.error.message, /Cannot index the home directory/i); + assert.match(result.error.message, /navigate to a specific project directory/i); +}); From 1f4f5a6423f66b2f742c5eaa21fe64754b84ba23 Mon Sep 17 00:00:00 2001 From: Orc Date: Fri, 3 Jul 2026 20:35:08 +0200 Subject: [PATCH 2/2] Fix: grep fails when target directory is outside of initial workspace --- FIX_SUMMARY_ISSUE_4.md | 116 ++++++++++++++++++++++++++++++++++++++ src/fff-runtime.ts | 18 +++++- src/fff-types.ts | 1 + src/register-tools.ts | 8 ++- tests/fff-runtime.test.ts | 34 +++++++++++ 5 files changed, 174 insertions(+), 3 deletions(-) create mode 100644 FIX_SUMMARY_ISSUE_4.md diff --git a/FIX_SUMMARY_ISSUE_4.md b/FIX_SUMMARY_ISSUE_4.md new file mode 100644 index 0000000..43eeec1 --- /dev/null +++ b/FIX_SUMMARY_ISSUE_4.md @@ -0,0 +1,116 @@ +# Fix for Issue #4: grep fails when target directory is outside of the initial workspace (ctx.cwd) + +## Problem Summary + +When pi is started in a specific directory (e.g., `d:\a\b\c`) and the user requests to search for files in a different directory (e.g., `d:\d`), the fff tool fails to find any results. This is because FileFinder is initialized with a specific `basePath` (the project root) and can only search within that indexed directory. + +### Example Scenario +```bash +# Start pi in d:\a\b\c +d:\a\b\c> pi + +# User asks to search in d:\d +"please search content hello in d:\d directory" +``` + +**Expected**: Search returns results from `d:\d` +**Actual**: Search returns no results because `d:\d` is outside the indexed `basePath` of `d:\a\b\c` + +## Root Cause + +1. FileFinder is initialized with a `basePath` that is the project root (e.g., `d:\a\b\c`) +2. FileFinder only indexes and searches files within this `basePath` +3. When a user provides an absolute path like `d:\d`, the code attempts to resolve it +4. The resolved path gets a `relativePath` relative to the `basePath`, but this doesn't make sense for paths outside the `basePath` +5. FileFinder's native constraints are built from this relative path, which doesn't match any files in the indexed directory +6. Result: No matches found, even though files exist in the target directory + +## Solution Implemented + +The fix detects when a search target is outside the current `basePath` and falls back to the built-in grep tool, which can handle arbitrary paths. + +### Changes Made + +1. **Added `isOutsideBasePath` property to `ResolvedPath` type** (`src/fff-types.ts`) + - Tracks whether a resolved path is outside the current indexed base path + +2. **Created `isPathOutsideBasePath()` helper function** (`src/fff-runtime.ts`) + - Uses Node.js `relative()` to check if a path is outside the base path + - Returns `true` if the relative path starts with ".." + +3. **Updated `resolveExistingPath()` method** (`src/fff-runtime.ts`) + - Returns `isOutsideBasePath` flag along with other path information + - Updated return type to include the new property + +4. **Updated `resolvePath()` method** (`src/fff-runtime.ts`) + - Passes `isOutsideBasePath` through to the `ResolvedPath` object + - All three places where `ResolvedPath` is created now include this flag + +5. **Modified grep tool registration** (`src/register-tools.ts`) + - When FFF grep returns results, checks if the scope is outside the base path + - If it is, falls back to the built-in grep which can handle external paths + - This ensures searches work correctly regardless of the target directory location + +6. **Added test coverage** (`tests/fff-runtime.test.ts`) + - Test verifies that `resolvePath` correctly marks paths outside the basePath + - Test verifies that paths inside the basePath are not marked as outside + +## How It Works + +1. User requests grep with a path query (e.g., `d:\d`) +2. The path is resolved and checked to see if it's outside the `basePath` +3. FFF grep is executed with the resolved scope +4. If the scope has `isOutsideBasePath = true`, the tool falls back to built-in grep +5. Built-in grep uses the absolute path and can search any directory +6. Results are returned to the user + +## Benefits + +1. ✅ **Works with external directories**: Users can search in directories outside the initial workspace +2. ✅ **Automatic fallback**: No manual intervention needed - the system automatically uses the right tool +3. ✅ **Maintains performance**: For paths within the basePath, FFF's fast indexed search is still used +4. ✅ **Cross-platform**: Works on Windows, macOS, and Linux +5. ✅ **Backward compatible**: No breaking changes to existing functionality +6. ✅ **Minimal code change**: Only 57 lines added across 4 files + +## Alternative Approaches Considered + +1. **Dynamic re-indexing** (as suggested in the issue): + - Would require adding a `chindex` method to change the indexed directory + - Limitations: FileFinder can only index one directory at a time + - Would require creating/destroying FileFinder instances + - More complex and could have performance implications + +2. **Indexing from drive root**: + - Would index entire drive (e.g., `d:\`) + - Problems: Very slow, uses excessive memory, not practical + - Could trigger issues like the OneDrive sync problem (Issue #6) + +3. **Multiple FileFinder instances**: + - Cache multiple FileFinder instances for different base paths + - Problems: Complex, resource-intensive, unclear when to clean up + +4. **Selected approach (automatic fallback)**: + - Simple, elegant, and effective + - Leverages existing built-in grep as a fallback + - No resource overhead + - Seamless user experience + +## Testing + +All tests pass: +- Typecheck: ✅ +- All 30 tests: ✅ +- New test for external path detection: ✅ + +## Backward Compatibility + +This is a **non-breaking change**. All existing functionality continues to work exactly as before. The only difference is that searches with external paths now work correctly instead of returning no results. + +## Edge Cases Handled + +1. **Absolute paths outside basePath**: Detected and handled correctly +2. **Relative paths inside basePath**: Work as before +3. **Mixed scenarios**: User can search both inside and outside the basePath in the same session +4. **Non-existent paths**: Continue to return appropriate errors +5. **Nested paths**: Correctly identifies parent/child relationships \ No newline at end of file diff --git a/src/fff-runtime.ts b/src/fff-runtime.ts index 079512d..28b5cc4 100644 --- a/src/fff-runtime.ts +++ b/src/fff-runtime.ts @@ -70,6 +70,14 @@ function isHomeDirectory(path: string): boolean { return resolve(path) === resolve(home); } +function isPathOutsideBasePath(basePath: string, targetPath: string): boolean { + const resolvedBase = resolve(basePath); + const resolvedTarget = resolve(targetPath); + const rel = relative(resolvedBase, resolvedTarget); + // If the relative path starts with "..", the target is outside the base path + return rel.startsWith(".."); +} + function normalizeSlashes(value: string): string { return value.replace(/\\/g, "/"); } @@ -417,6 +425,7 @@ export class FffRuntime { relativePath: direct.relativePath, pathType: direct.pathType, candidates: [], + isOutsideBasePath: direct.isOutsideBasePath, }); } } @@ -439,6 +448,7 @@ export class FffRuntime { pathType: direct.pathType, location: search.value.location, candidates: filtered, + isOutsideBasePath: direct.isOutsideBasePath, }); } @@ -452,6 +462,7 @@ export class FffRuntime { const absolutePath = top.item.path && isAbsolute(top.item.path) ? top.item.path : resolve(this.basePath, top.item.relativePath); const pathType = (await getPathType(absolutePath)) ?? "file"; + const isOutsideBasePath = isPathOutsideBasePath(this.basePath, absolutePath); return Result.ok({ kind: "resolved", query, @@ -460,6 +471,7 @@ export class FffRuntime { pathType, location: search.value.location, candidates: filtered, + isOutsideBasePath, }); } @@ -800,7 +812,7 @@ export class FffRuntime { } satisfies GrepSearchResponse); } - private async resolveExistingPath(query: string, allowDirectory: boolean): Promise | null> { + private async resolveExistingPath(query: string, allowDirectory: boolean): Promise | null> { const candidates = isAbsolute(query) ? [query] : query.startsWith("./") || query.startsWith("../") @@ -812,10 +824,12 @@ export class FffRuntime { const pathType = await getPathType(directPath); if (!pathType) continue; if (pathType === "directory" && !allowDirectory) continue; + const relativePath = relativeFrom(this.basePath, directPath); return { absolutePath: directPath, - relativePath: relativeFrom(this.basePath, directPath), + relativePath, pathType, + isOutsideBasePath: isPathOutsideBasePath(this.basePath, directPath), }; } return null; diff --git a/src/fff-types.ts b/src/fff-types.ts index 592a100..f91194a 100644 --- a/src/fff-types.ts +++ b/src/fff-types.ts @@ -30,6 +30,7 @@ export type ResolvedPath = { pathType: "file" | "directory"; location?: Location; candidates: FffFileCandidate[]; + isOutsideBasePath?: boolean; }; export type PathResolution = AppResult; diff --git a/src/register-tools.ts b/src/register-tools.ts index 428ea6e..be97311 100644 --- a/src/register-tools.ts +++ b/src/register-tools.ts @@ -147,7 +147,13 @@ export function registerTools(pi: ExtensionAPI, deps: ToolRegistrationDeps): voi } return textResult(buildGrepFailureMessage(error, params.path), buildGrepDetails(undefined, undefined, error)); }, - ok: async (value) => textResult(value.formatted, buildGrepDetails(value)), + ok: async (value) => { + // If the scope is outside the base path, fall back to built-in grep + if (value.scope?.isOutsideBasePath) { + return original.execute(toolCallId, builtinParams, signal, onUpdate); + } + return textResult(value.formatted, buildGrepDetails(value)); + }, }); }, }); diff --git a/tests/fff-runtime.test.ts b/tests/fff-runtime.test.ts index 565cc5f..1e02d9e 100644 --- a/tests/fff-runtime.test.ts +++ b/tests/fff-runtime.test.ts @@ -620,3 +620,37 @@ test("initialize rejects home directory as project root", async () => { assert.match(result.error.message, /Cannot index the home directory/i); assert.match(result.error.message, /navigate to a specific project directory/i); }); + +test("resolvePath marks paths outside basePath", async () => { + const root = await mkdtemp(join(tmpdir(), "pi-fff-")); + const sibling = await mkdtemp(join(tmpdir(), "pi-fff-sibling-")); + await writeFile(join(sibling, "test.txt"), "hello\n", "utf8"); + + const finder = createMockFinder({ + fileSearch() { + return ok({ + items: [], + totalFiles: 0, + totalMatched: 0, + scores: [], + }); + }, + }); + + const runtime = new FffRuntime(root, { finder }); + + // Test absolute path outside basePath + const result = await runtime.resolvePath(sibling); + assert.equal(result.isOk(), true); + if (result.isErr()) assert.fail(result.error.message); + assert.equal(result.value.isOutsideBasePath, true); + assert.equal(result.value.absolutePath, sibling); + + // Test path inside basePath + const insidePath = join(root, "inside"); + await mkdir(insidePath, { recursive: true }); + const insideResult = await runtime.resolvePath(insidePath); + assert.equal(insideResult.isOk(), true); + if (insideResult.isErr()) assert.fail(insideResult.error.message); + assert.equal(insideResult.value.isOutsideBasePath, false); +});