diff --git a/packages/fff-bun/test/multi-session.test.ts b/packages/fff-bun/test/multi-session.test.ts index b6a4a0aa..35bd7f3d 100644 --- a/packages/fff-bun/test/multi-session.test.ts +++ b/packages/fff-bun/test/multi-session.test.ts @@ -153,6 +153,7 @@ function startSession( const events = new Map(); const tools = new Map(); const notifications: Array<{ message: string; level?: string }> = []; + let activeTools: string[] = []; const flags: Record = { "fff-frecency-db": dbs.frecencyDbPath, @@ -165,6 +166,10 @@ function startSession( registerCommand: () => undefined, registerFlag: () => undefined, registerTool: (tool: RegisteredTool) => tools.set(tool.name, tool), + getActiveTools: () => activeTools, + setActiveTools: (names: string[]) => { + activeTools = names; + }, appendEntry: () => undefined, }; diff --git a/packages/pi-fff/README.md b/packages/pi-fff/README.md index 6456d55b..ef860111 100644 --- a/packages/pi-fff/README.md +++ b/packages/pi-fff/README.md @@ -116,7 +116,7 @@ Parameters: - `/fff-health` — show FFF status (indexed files, git info, frecency/history DB status) - `/fff-rescan` — trigger a file rescan -- `/fff-mode ` — switch mode (tool name change requires restart) +- `/fff-mode ` — switch mode (tool name changes require `/reload`) ## Modes @@ -124,12 +124,14 @@ Parameters: - `tools-only`: additional tools only; keep pi's default `@` autocomplete - `override`: replaces pi's built-in `find`, `grep` and adds `multi_grep` + FFF-backed `@` autocomplete -Mode precedence: +Startup mode precedence: 1. `--fff-mode ` CLI flag 2. `PI_FFF_MODE=` environment variable 3. `mode` in the global config file 4. default (`tools-and-ui`) +When a session resumes, its most recent `/fff-mode` selection takes precedence over the startup resolution above. Switching to or from `override` takes effect after `/reload`, when the tools are registered again. + ## Configuration For persistent global configuration, create `pi-fff.json` in pi's agent directory (`~/.pi/agent/pi-fff.json` by default; `PI_CODING_AGENT_DIR` is respected): diff --git a/packages/pi-fff/src/index.ts b/packages/pi-fff/src/index.ts index 120f2e06..2ca27337 100644 --- a/packages/pi-fff/src/index.ts +++ b/packages/pi-fff/src/index.ts @@ -6,7 +6,11 @@ */ import nodePath from "node:path"; -import type { ExtensionAPI } from "@earendil-works/pi-coding-agent"; +import type { + ExtensionAPI, + ExtensionContext, + ToolDefinition, +} from "@earendil-works/pi-coding-agent"; import { type AutocompleteItem, type AutocompleteProvider, @@ -20,7 +24,7 @@ import type { MixedItem, SearchResult, } from "@ff-labs/fff-node"; -import { Type } from "@sinclair/typebox"; +import { Type, type TSchema } from "@sinclair/typebox"; import { AuxFinderPool, routePathConstraint } from "./aux-finders"; import { type FffMode, loadConfig, VALID_MODES } from "./config"; import { FilePickerFactory } from "./file-picker"; @@ -328,53 +332,69 @@ export default function fffExtension(pi: ExtensionAPI) { return undefined; } - let currentMode = getConfigValue( - "fff-mode", - "PI_FFF_MODE", - config.mode, - "tools-and-ui", - ); - const toolNames = resolveToolNames(currentMode); - - const resolvedDbPaths = resolveDbPaths({ - frecency: getConfigValue( - "fff-frecency-db", - "FFF_FRECENCY_DB", - config.frecencyDbPath, - undefined, - ), - history: getConfigValue( - "fff-history-db", - "FFF_HISTORY_DB", - config.historyDbPath, - undefined, - ), - }); - - // Root scanning opt-in: FFF refuses to init at / unless this is set. - const enableFsRootScanning = getConfigValue( - "fff-enable-root-scan", - "FFF_ENABLE_ROOT_SCAN", - config.enableFsRootScanning, - false, - parseBoolean, - ); - // Home dir scanning is on by default (launching pi from $HOME is a normal - // flow), but configurable so users with huge $HOME trees can opt out. - const enableHomeDirScanning = getConfigValue( - "fff-enable-home-scan", - "FFF_ENABLE_HOME_SCAN", - config.enableHomeDirScanning, - true, - parseBoolean, - ); - - function getMode(): FffMode { - return currentMode; + function parseMode(value: unknown): FffMode | undefined { + return typeof value === "string" && VALID_MODES.includes(value as FffMode) + ? (value as FffMode) + : undefined; } + let currentMode: FffMode = "tools-and-ui"; + let toolNames = resolveToolNames(currentMode); + let resolvedDbPaths: ReturnType; + let enableFsRootScanning = false; + let enableHomeDirScanning = true; + function setMode(mode: FffMode): void { currentMode = mode; + toolNames = resolveToolNames(mode); + } + + function resolveStartupConfig(): void { + setMode( + getConfigValue( + "fff-mode", + "PI_FFF_MODE", + config.mode, + "tools-and-ui", + parseMode, + ), + ); + resolvedDbPaths = resolveDbPaths({ + frecency: getConfigValue( + "fff-frecency-db", + "FFF_FRECENCY_DB", + config.frecencyDbPath, + undefined, + ), + history: getConfigValue( + "fff-history-db", + "FFF_HISTORY_DB", + config.historyDbPath, + undefined, + ), + }); + + // Root scanning opt-in: FFF refuses to init at / unless this is set. + enableFsRootScanning = getConfigValue( + "fff-enable-root-scan", + "FFF_ENABLE_ROOT_SCAN", + config.enableFsRootScanning, + false, + parseBoolean, + ); + // Home dir scanning is on by default (launching pi from $HOME is a normal + // flow), but configurable so users with huge $HOME trees can opt out. + enableHomeDirScanning = getConfigValue( + "fff-enable-home-scan", + "FFF_ENABLE_HOME_SCAN", + config.enableHomeDirScanning, + true, + parseBoolean, + ); + } + + function getMode(): FffMode { + return currentMode; } function shouldEnableMentions(): boolean { @@ -398,22 +418,28 @@ export default function fffExtension(pi: ExtensionAPI) { ); } - const pickers = new FilePickerFactory({ - frecencyDbPath: resolvedDbPaths.frecency, - historyDbPath: resolvedDbPaths.history, - onDbFailure: (error) => - uiCtx?.ui.notify( - `(fff): Failed to open frecency/history database (${error}). Continuing without frecency persistence.`, - "error", - ), - }); + let pickers: FilePickerFactory | null = null; + let auxPool: AuxFinderPool | null = null; - const auxPool = new AuxFinderPool({ - enableFsRootScanning, - enableHomeDirScanning, - onHomeDirScan: warnHomeDirScan, - pickers, - }); + function initializeFinderFactories(): void { + if (pickers) return; + + pickers = new FilePickerFactory({ + frecencyDbPath: resolvedDbPaths.frecency, + historyDbPath: resolvedDbPaths.history, + onDbFailure: (error) => + uiCtx?.ui.notify( + `(fff): Failed to open frecency/history database (${error}). Continuing without frecency persistence.`, + "error", + ), + }); + auxPool = new AuxFinderPool({ + enableFsRootScanning, + enableHomeDirScanning, + onHomeDirScan: warnHomeDirScan, + pickers, + }); + } // in case cwd changes we need to figure this out function ensureFinder(cwd: string): Promise { @@ -431,6 +457,7 @@ export default function fffExtension(pi: ExtensionAPI) { // if the dbs can't be opened the factory falls back to a db-less picker, // e.g. when some other process corrupts the lock + if (!pickers) throw new Error("FFF picker factory is not initialized"); mainFinder = await pickers.create({ basePath: cwd, enableHomeDirScanning, @@ -485,9 +512,9 @@ export default function fffExtension(pi: ExtensionAPI) { finderCwd = null; } - if (auxPool) { - auxPool.destroy(); - } + auxPool?.destroy(); + auxPool = null; + pickers = null; } async function resolveFinderForPath( @@ -497,6 +524,7 @@ export default function fffExtension(pi: ExtensionAPI) { ): Promise<{ finder: FileFinderApi; query: string; root: string } | null> { const route = routePathConstraint(pathParam, activeCwd); if (!route) return null; + if (!auxPool) throw new Error("FFF auxiliary finder pool is not initialized"); const aux = await auxPool.acquire(route.root); // A broader covering picker may have been reused; rebase the suffix so the // constraint stays relative to the picker's actual root. @@ -577,6 +605,45 @@ export default function fffExtension(pi: ExtensionAPI) { }); } + type PendingToolDefinition< + TParams extends TSchema, + TDetails = unknown, + TState = any, + > = Omit< + ToolDefinition, + "name" | "label" | "promptGuidelines" + > & { + promptGuidelines?: (names: ToolNames) => string[]; + }; + + const pendingTools: (() => string)[] = []; + let toolsRegistered = false; + + function queueTool( + resolveName: () => string, + definition: PendingToolDefinition, + ): void { + pendingTools.push(() => { + const { promptGuidelines, ...tool } = definition; + const resolvedName = resolveName(); + pi.registerTool({ + ...tool, + name: resolvedName, + label: resolvedName, + promptGuidelines: promptGuidelines?.(toolNames), + }); + return resolvedName; + }); + } + + function registerPendingTools(): void { + if (toolsRegistered) return; + + const registeredNames = pendingTools.map((register) => register()); + pi.setActiveTools([...new Set([...pi.getActiveTools(), ...registeredNames])]); + toolsRegistered = true; + } + // --- Flags / lifecycle --- pi.registerFlag("fff-mode", { @@ -606,34 +673,48 @@ export default function fffExtension(pi: ExtensionAPI) { type: "boolean", }); - pi.on("session_start", async (_event, ctx) => { - try { - activeCwd = ctx.cwd; - uiCtx = ctx as unknown as typeof uiCtx; - - // Restore persisted mode from session entries. This handles session - // resume after process restart where env vars are lost, and ensures - // the env var is set for the next /reload in the same session. - const entries = ctx.sessionManager?.getEntries(); - if (entries) { - const modeEntry = [...entries] - .reverse() - .find( - (e: { type: string; customType?: string }) => - e.type === "custom" && e.customType === "fff-mode", - ); - if ( - modeEntry && - typeof (modeEntry as any).data?.mode === "string" && - VALID_MODES.includes((modeEntry as any).data.mode as FffMode) - ) { - const restored = (modeEntry as any).data.mode as FffMode; - if (restored !== currentMode) { - currentMode = restored; - } - } + function reportInitFailure(ctx: ExtensionContext, error: unknown): void { + ctx.ui.notify( + `FFF init failed: ${error instanceof Error ? error.message : String(error)}`, + "error", + ); + } + + function prepareSession(ctx: ExtensionContext): void { + activeCwd = ctx.cwd; + uiCtx = ctx; + if (toolsRegistered) return; + + // Pi populates extension flag values after loading extensions. + resolveStartupConfig(); + + // Restore persisted mode before registering tools so a saved override + // can safely change their names after /reload or session resume. + const entries = ctx.sessionManager?.getEntries(); + if (entries) { + const modeEntry = [...entries] + .reverse() + .find( + (e: { type: string; customType?: string }) => + e.type === "custom" && e.customType === "fff-mode", + ); + if ( + modeEntry && + typeof (modeEntry as any).data?.mode === "string" && + VALID_MODES.includes((modeEntry as any).data.mode as FffMode) + ) { + const restored = (modeEntry as any).data.mode as FffMode; + if (restored !== currentMode) setMode(restored); } + } + + initializeFinderFactories(); + registerPendingTools(); + } + pi.on("session_start", async (_event, ctx) => { + try { + prepareSession(ctx); registerAutocompleteProvider(ctx); await ensureFinder(activeCwd); @@ -651,11 +732,19 @@ export default function fffExtension(pi: ExtensionAPI) { // waitForScan() also resolves on timeout, so poll until the scan really // settles before clearing the footer. if (atHome) trackHomeScanStatus(); - } catch (e: unknown) { - ctx.ui.notify( - `FFF init failed: ${e instanceof Error ? e.message : String(e)}`, - "error", - ); + } catch (error: unknown) { + reportInitFailure(ctx, error); + } + }); + + // SDK callers can prompt without binding session_start. Prepare on the first + // agent turn as a fallback so the tools still reach that turn's tool set. + pi.on("before_agent_start", (_event, ctx) => { + if (toolsRegistered) return; + try { + prepareSession(ctx); + } catch (error: unknown) { + reportInitFailure(ctx, error); } }); @@ -731,16 +820,14 @@ export default function fffExtension(pi: ExtensionAPI) { ), }); - pi.registerTool({ - name: toolNames.grep, - label: toolNames.grep, + queueTool(() => toolNames.grep, { description: `Grep file contents. Smart-case, auto-detects regex vs literal, git-aware. Results are ranked by frecency (most-accessed files first); matches within a file stay in source order. Default limit ${DEFAULT_GREP_LIMIT}.`, promptSnippet: "Grep contents", - promptGuidelines: [ - `${toolNames.grep}: prefer bare identifiers as patterns. Literal queries are most efficient.`, - `${toolNames.grep}: use path for include ('src/', '*.ts') and exclude for noise ('test/,*.min.js').`, - `${toolNames.grep}: caseSensitive: true when you need exact case (smart-case otherwise).`, - `${toolNames.grep}: after 1-2 greps, read the top match instead of more greps.`, + promptGuidelines: (names) => [ + `${names.grep}: prefer bare identifiers as patterns. Literal queries are most efficient.`, + `${names.grep}: use path for include ('src/', '*.ts') and exclude for noise ('test/,*.min.js').`, + `${names.grep}: caseSensitive: true when you need exact case (smart-case otherwise).`, + `${names.grep}: after 1-2 greps, read the top match instead of more greps.`, ], parameters: grepSchema, @@ -922,18 +1009,16 @@ export default function fffExtension(pi: ExtensionAPI) { ), }); - pi.registerTool({ - name: toolNames.find, - label: toolNames.find, + queueTool(() => toolNames.find, { description: `Fuzzy path search and glob search. Matches against the whole repo-relative path, not just the filename. Frecency-ranked, git-aware. Multi-word = narrower (AND). Default limit ${DEFAULT_FIND_LIMIT}.`, promptSnippet: "Find files by path or glob", - promptGuidelines: [ - `${toolNames.find}: matches the WHOLE path, not just the filename — \`profile\` hits \`chrome/browser/profiles/x.cc\` too.`, - `${toolNames.find}: keep queries to 1-2 terms; extra words narrow.`, - `${toolNames.find}: use for paths, not content. Use ${toolNames.grep} for content.`, - `${toolNames.find}: for exact path matches use a glob in \`path\` — e.g. path: '**/profile.h' for exact filename, or path: 'src/**/profile.h' scoped to a subtree. Bare patterns are fuzzy.`, - `${toolNames.find}: to list everything inside a directory, pass path: 'dir/**' with an empty or wildcard pattern instead of using pattern alone.`, - `${toolNames.find}: use exclude: 'test/,*.min.js' to cut noise in large repos.`, + promptGuidelines: (names) => [ + `${names.find}: matches the WHOLE path, not just the filename — \`profile\` hits \`chrome/browser/profiles/x.cc\` too.`, + `${names.find}: keep queries to 1-2 terms; extra words narrow.`, + `${names.find}: use for paths, not content. Use ${names.grep} for content.`, + `${names.find}: for exact path matches use a glob in \`path\` — e.g. path: '**/profile.h' for exact filename, or path: 'src/**/profile.h' scoped to a subtree. Bare patterns are fuzzy.`, + `${names.find}: to list everything inside a directory, pass path: 'dir/**' with an empty or wildcard pattern instead of using pattern alone.`, + `${names.find}: use exclude: 'test/,*.min.js' to cut noise in large repos.`, ], parameters: findSchema, @@ -942,10 +1027,12 @@ export default function fffExtension(pi: ExtensionAPI) { // if resumed we use the same picker as before const resumed = params.cursor ? getFindCursor(params.cursor) : undefined; + const pool = auxPool; + if (!pool) throw new Error("FFF auxiliary finder pool is not initialized"); const aux = resumed ? resumed.auxRoot ? { - finder: (await auxPool.acquire(resumed.auxRoot, { exact: true })).finder, + finder: (await pool.acquire(resumed.auxRoot, { exact: true })).finder, root: resumed.auxRoot, } : null @@ -1062,16 +1149,14 @@ export default function fffExtension(pi: ExtensionAPI) { cursor: Type.Optional(Type.String({ description: "Pagination cursor" })), }); - pi.registerTool({ - name: toolNames.multiGrep, - label: toolNames.multiGrep, + queueTool(() => toolNames.multiGrep, { description: "Search file contents for ANY of multiple literal patterns (OR, SIMD Aho-Corasick). Faster than regex alternation.", promptSnippet: "Multi-pattern OR content search", - promptGuidelines: [ - `${toolNames.multiGrep}: use when searching for several identifiers at once.`, - `${toolNames.multiGrep}: include all naming-convention variants (snake/camel/Pascal).`, - `${toolNames.multiGrep}: patterns are literal. Use constraints for file filters.`, + promptGuidelines: (names) => [ + `${names.multiGrep}: use when searching for several identifiers at once.`, + `${names.multiGrep}: include all naming-convention variants (snake/camel/Pascal).`, + `${names.multiGrep}: patterns are literal. Use constraints for file filters.`, ], parameters: multiGrepSchema, @@ -1146,6 +1231,15 @@ export default function fffExtension(pi: ExtensionAPI) { pi.registerCommand("fff-mode", { description: "Show or set FFF mode: /fff-mode [tools-and-ui | tools-only | override]", handler: async (args, ctx) => { + if (!toolsRegistered) { + try { + prepareSession(ctx); + } catch (error: unknown) { + reportInitFailure(ctx, error); + return; + } + } + const arg = (args || "").trim(); // No args - show current mode @@ -1164,15 +1258,18 @@ export default function fffExtension(pi: ExtensionAPI) { const newMode = arg as FffMode; const oldMode = getMode(); - setMode(newMode); - pi.appendEntry("fff-mode", { mode: newMode }); - const note = - (oldMode === "override") !== (newMode === "override") - ? " (tool name change requires /reload)" - : ""; - ctx.ui.notify(`Mode changed: '${oldMode}' → '${newMode}'${note}`, "info"); + if ((oldMode === "override") !== (newMode === "override")) { + ctx.ui.notify( + `Mode '${newMode}' saved. Run /reload to apply the tool name change.`, + "info", + ); + return; + } + + setMode(newMode); + ctx.ui.notify(`Mode changed: '${oldMode}' → '${newMode}'`, "info"); }, }); diff --git a/packages/pi-fff/test/extension.test.ts b/packages/pi-fff/test/extension.test.ts index 9d1c3a5c..4021c3c9 100644 --- a/packages/pi-fff/test/extension.test.ts +++ b/packages/pi-fff/test/extension.test.ts @@ -102,19 +102,35 @@ type EventHandler = (...args: any[]) => unknown; function createPi(mode?: string, flags: Record = {}) { const events = new Map(); const commands = new Map(); + const registeredFlags = new Set(); + let flagsReady = false; const pi = { - getFlag: mock((name: string) => - name === "fff-mode" && mode !== undefined ? mode : flags[name], - ), + getFlag: mock((name: string) => { + if (!flagsReady || !registeredFlags.has(name)) return undefined; + return name === "fff-mode" && mode !== undefined ? mode : flags[name]; + }), on: mock((event: string, handler: EventHandler) => { - events.set(event, handler); + events.set(event, (...args) => { + flagsReady = true; + return handler(...args); + }); }), registerCommand: mock((name: string, command: any) => { - commands.set(name, command); + commands.set(name, { + ...command, + handler: (...args: any[]) => { + flagsReady = true; + return command.handler(...args); + }, + }); + }), + registerFlag: mock((name: string) => { + registeredFlags.add(name); }), - registerFlag: mock(() => undefined), registerTool: mock((_tool: any) => undefined), + getActiveTools: mock(() => ["read"] as string[]), + setActiveTools: mock((_names: string[]) => undefined), appendEntry: mock(() => undefined), }; @@ -124,6 +140,9 @@ function createPi(mode?: string, flags: Record = {}) { function createContext(cwd = "/tmp/workspace") { return { cwd, + sessionManager: { + getEntries: mock(() => [] as any[]), + }, // Signatures mirror the real pi UI surface so mock.calls stays typed. ui: { addAutocompleteProvider: mock((_factory: (current: any) => any) => undefined), @@ -258,12 +277,94 @@ describe("pi-fff global config", () => { }); await shutdown(setup); }); + + test("falls through invalid flag and environment modes", async () => { + writeConfig({ mode: "override" }); + process.env.PI_FFF_MODE = "invalid-env-mode"; + + const setup = await start("invalid-flag-mode"); + const toolNames = setup.pi.registerTool.mock.calls.map(([tool]) => tool.name); + + expect(toolNames).toContain("grep"); + expect(toolNames).toContain("find"); + expect(toolNames).not.toContain("ffgrep"); + await shutdown(setup); + }); }); function writeConfig(config: Record): void { fs.writeFileSync(configPath, JSON.stringify(config)); } +describe("pi-fff session mode", () => { + test("registers tools only after restoring the saved mode", async () => { + const setup = createPi("tools-and-ui"); + const ctx = createContext(); + ctx.sessionManager.getEntries.mockReturnValue([ + { type: "custom", customType: "fff-mode", data: { mode: "override" } }, + ]); + fffExtension(setup.pi as any); + + expect(setup.pi.registerTool).not.toHaveBeenCalled(); + await setup.events.get("session_start")?.({ reason: "startup" }, ctx); + + const tools = setup.pi.registerTool.mock.calls.map(([tool]) => tool); + const toolNames = tools.map((tool) => tool.name); + expect(toolNames).toContain("grep"); + expect(toolNames).toContain("find"); + expect(toolNames).not.toContain("ffgrep"); + expect(toolNames).not.toContain("fffind"); + const grepTool = tools.find((tool) => tool.name === "grep"); + expect(grepTool.promptGuidelines[0].startsWith("grep:")).toBe(true); + expect(setup.pi.setActiveTools).toHaveBeenCalledWith( + expect.arrayContaining(["read", "grep", "find"]), + ); + + await setup.commands.get("fff-mode").handler("", ctx); + expect(ctx.ui.notify).toHaveBeenLastCalledWith( + "Current mode: 'override' (flag: tools-and-ui)", + "info", + ); + await shutdown(setup); + }); + + test("registers tools before an unbound SDK session's first agent turn", async () => { + const setup = createPi("override"); + const ctx = createContext(); + fffExtension(setup.pi as any); + + expect(setup.pi.registerTool).not.toHaveBeenCalled(); + await setup.events.get("before_agent_start")?.({}, ctx); + + const toolNames = setup.pi.registerTool.mock.calls.map(([tool]) => tool.name); + expect(toolNames).toContain("grep"); + expect(toolNames).toContain("find"); + expect(createCalls).toHaveLength(0); + await shutdown(setup); + }); + + test("keeps the active mode unchanged until a tool-name switch is reloaded", async () => { + const setup = await start(); + + await setup.commands.get("fff-mode").handler("override", setup.ctx); + + expect(setup.pi.appendEntry).toHaveBeenCalledWith("fff-mode", { + mode: "override", + }); + expect(setup.ctx.ui.notify).toHaveBeenLastCalledWith( + "Mode 'override' saved. Run /reload to apply the tool name change.", + "info", + ); + + await setup.commands.get("fff-mode").handler("", setup.ctx); + expect(setup.ctx.ui.notify).toHaveBeenLastCalledWith( + "Current mode: 'tools-and-ui' (flag: unset)", + "info", + ); + await shutdown(setup); + }); +}); + // Regression for #743: launching from $HOME must be visible and interruptible. describe("pi-fff $HOME scan warning", () => { test("warns and pins a status when cwd is $HOME", async () => { @@ -477,7 +578,7 @@ describe("pi-fff autocomplete registration", () => { }); test("/fff-mode changes mention behavior without touching the editor", async () => { - const { commands, ctx } = await start(); + const { commands, ctx, pi } = await start(); const factory = ctx.ui.addAutocompleteProvider.mock.calls[0][0]; const current = currentProvider(); const provider = factory(current); @@ -485,6 +586,7 @@ describe("pi-fff autocomplete registration", () => { await commands.get("fff-mode").handler("tools-only", ctx); await provider.getSuggestions(["@src"], 0, 4, abortOptions()); + expect(pi.appendEntry).toHaveBeenCalledWith("fff-mode", { mode: "tools-only" }); expect(current.getSuggestions).toHaveBeenCalledTimes(1); expect(finders[0].mixedSearch).not.toHaveBeenCalled(); expect(ctx.ui.setEditorComponent).not.toHaveBeenCalled();