From 27c1544839230e38251f07b341ba52864a3a20df Mon Sep 17 00:00:00 2001 From: Mario Zechner Date: Mon, 20 Apr 2026 21:53:07 +0200 Subject: [PATCH] fix(coding-agent): use tool-name allowlists and remove cwd-bound singletons - treat tools as a global allowlist across built-in, extension, and SDK tools - remove process-cwd singleton tool usage from SDK and CLI paths - add regression coverage for extension tool filtering closes #3452 closes #2835 --- packages/coding-agent/CHANGELOG.md | 1 + packages/coding-agent/docs/extensions.md | 14 +- .../coding-agent/examples/sdk/05-tools.ts | 42 ++--- .../examples/sdk/12-full-control.ts | 12 +- packages/coding-agent/src/cli/args.ts | 28 +--- .../src/core/agent-session-services.ts | 3 +- .../coding-agent/src/core/agent-session.ts | 48 ++++-- packages/coding-agent/src/core/sdk.ts | 39 ++--- packages/coding-agent/src/core/tools/index.ts | 155 +++++++++--------- packages/coding-agent/src/main.ts | 11 +- .../test/agent-session-branching.test.ts | 4 +- .../test/agent-session-runtime-events.test.ts | 1 - .../test/compaction-thinking-model.test.ts | 6 +- .../test/sdk-session-manager.test.ts | 31 +++- .../test/suite/agent-session-runtime.test.ts | 5 +- ...-allowlist-filters-extension-tools.test.ts | 94 +++++++++++ 16 files changed, 295 insertions(+), 199 deletions(-) create mode 100644 packages/coding-agent/test/suite/regressions/2835-tools-allowlist-filters-extension-tools.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 434b5b18..c3e220be 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -13,6 +13,7 @@ ### Fixed +- Fixed `ctx.ui.setWorkingIndicator()` custom frames to render verbatim instead of forcing the theme accent color, so extensions now own working-indicator coloring when they customize it ([#3467](https://github.com/badlogic/pi-mono/issues/3467)) - Fixed `pi update` reinstalling npm packages that are already at the latest published version by checking the installed package version before running `npm install @latest` ([#3000](https://github.com/badlogic/pi-mono/issues/3000)) - Fixed built-in tool wrapping to use the same extension-runner context path as extension tools, so built-in tools receive execution context and `read` can warn when the current model does not support images ([#3429](https://github.com/badlogic/pi-mono/issues/3429)) - Fixed threaded `/resume` session relationships and current-session detection to canonicalize symlinked session paths during selector comparisons, so shared session directories no longer break parent-child matching or active-session delete protection ([#3364](https://github.com/badlogic/pi-mono/issues/3364)) diff --git a/packages/coding-agent/docs/extensions.md b/packages/coding-agent/docs/extensions.md index 09b58d31..356561c6 100644 --- a/packages/coding-agent/docs/extensions.md +++ b/packages/coding-agent/docs/extensions.md @@ -2049,8 +2049,16 @@ ctx.ui.setWorkingMessage("Thinking deeply..."); ctx.ui.setWorkingMessage(); // Restore default // Working indicator (shown during streaming) -ctx.ui.setWorkingIndicator({ frames: ["●"] }); // Static dot -ctx.ui.setWorkingIndicator({ frames: ["·", "•", "●", "•"], intervalMs: 120 }); +ctx.ui.setWorkingIndicator({ frames: [ctx.ui.theme.fg("accent", "●")] }); // Static dot +ctx.ui.setWorkingIndicator({ + frames: [ + ctx.ui.theme.fg("dim", "·"), + ctx.ui.theme.fg("muted", "•"), + ctx.ui.theme.fg("accent", "●"), + ctx.ui.theme.fg("muted", "•"), + ], + intervalMs: 120, +}); ctx.ui.setWorkingIndicator({ frames: [] }); // Hide indicator ctx.ui.setWorkingIndicator(); // Restore default spinner @@ -2098,6 +2106,8 @@ ctx.ui.setTheme(lightTheme!); // Or switch by Theme object ctx.ui.theme.fg("accent", "styled text"); // Access current theme ``` +Custom working-indicator frames are rendered verbatim. If you want colors, add them to the frame strings yourself, for example with `ctx.ui.theme.fg(...)`. + ### Custom Components For complex UI, use `ctx.ui.custom()`. This temporarily replaces the editor with your component until `done()` is called: diff --git a/packages/coding-agent/examples/sdk/05-tools.ts b/packages/coding-agent/examples/sdk/05-tools.ts index 57648bb0..c24cb2a0 100644 --- a/packages/coding-agent/examples/sdk/05-tools.ts +++ b/packages/coding-agent/examples/sdk/05-tools.ts @@ -1,56 +1,44 @@ /** * Tools Configuration * - * Use built-in tool sets or individual tools. + * Use tool names to choose which built-in tools are enabled. * - * IMPORTANT: When using a custom `cwd`, you must use the tool factory functions - * (createCodingTools, createReadOnlyTools, createReadTool, etc.) to ensure - * tools resolve paths relative to your cwd, not process.cwd(). + * Tool names are matched against all available tools. If you use a custom `cwd`, + * createAgentSession() applies that cwd when it builds the actual built-in tools. * - * For custom tools, see 06-extensions.ts - custom tools are now registered - * via the extensions system using pi.registerTool(). + * For custom tools, see 06-extensions.ts - custom tools are registered via the + * extensions system using pi.registerTool(). */ -import { - bashTool, - createAgentSession, - createBashTool, - createCodingTools, - createGrepTool, - createReadTool, - grepTool, - readOnlyTools, - readTool, - SessionManager, -} from "@mariozechner/pi-coding-agent"; +import { createAgentSession, SessionManager } from "@mariozechner/pi-coding-agent"; -// Read-only mode (no edit/write) - uses process.cwd() +// Read-only mode (no edit/write) await createAgentSession({ - tools: readOnlyTools, + tools: ["read", "grep", "find", "ls"], sessionManager: SessionManager.inMemory(), }); console.log("Read-only session created"); -// Custom tool selection - uses process.cwd() +// Custom tool selection await createAgentSession({ - tools: [readTool, bashTool, grepTool], + tools: ["read", "bash", "grep"], sessionManager: SessionManager.inMemory(), }); console.log("Custom tools session created"); -// With custom cwd - MUST use factory functions! +// With custom cwd const customCwd = "/path/to/project"; await createAgentSession({ cwd: customCwd, - tools: createCodingTools(customCwd), // Tools resolve paths relative to customCwd - sessionManager: SessionManager.inMemory(), + tools: ["read", "bash", "edit", "write"], + sessionManager: SessionManager.inMemory(customCwd), }); console.log("Custom cwd session created"); // Or pick specific tools for custom cwd await createAgentSession({ cwd: customCwd, - tools: [createReadTool(customCwd), createBashTool(customCwd), createGrepTool(customCwd)], - sessionManager: SessionManager.inMemory(), + tools: ["read", "bash", "grep"], + sessionManager: SessionManager.inMemory(customCwd), }); console.log("Specific tools with custom cwd session created"); diff --git a/packages/coding-agent/examples/sdk/12-full-control.ts b/packages/coding-agent/examples/sdk/12-full-control.ts index 2387fa3a..55b019ed 100644 --- a/packages/coding-agent/examples/sdk/12-full-control.ts +++ b/packages/coding-agent/examples/sdk/12-full-control.ts @@ -2,19 +2,13 @@ * Full Control * * Replace everything - no discovery, explicit configuration. - * - * IMPORTANT: When providing `tools` with a custom `cwd`, use the tool factory - * functions (createReadTool, createBashTool, etc.) to ensure tools resolve - * paths relative to your cwd. */ import { getModel } from "@mariozechner/pi-ai"; import { AuthStorage, createAgentSession, - createBashTool, createExtensionRuntime, - createReadTool, ModelRegistry, type ResourceLoader, SessionManager, @@ -41,7 +35,6 @@ const settingsManager = SettingsManager.inMemory({ retry: { enabled: true, maxRetries: 2 }, }); -// When using a custom cwd with explicit tools, use the factory functions const cwd = process.cwd(); const resourceLoader: ResourceLoader = { @@ -65,9 +58,8 @@ const { session } = await createAgentSession({ authStorage, modelRegistry, resourceLoader, - // Use factory functions with the same cwd to ensure path resolution works correctly - tools: [createReadTool(cwd), createBashTool(cwd)], - sessionManager: SessionManager.inMemory(), + tools: ["read", "bash"], + sessionManager: SessionManager.inMemory(cwd), settingsManager, }); diff --git a/packages/coding-agent/src/cli/args.ts b/packages/coding-agent/src/cli/args.ts index a77e4de3..a541c199 100644 --- a/packages/coding-agent/src/cli/args.ts +++ b/packages/coding-agent/src/cli/args.ts @@ -6,7 +6,6 @@ import type { ThinkingLevel } from "@mariozechner/pi-agent-core"; import chalk from "chalk"; import { APP_NAME, CONFIG_DIR_NAME, ENV_AGENT_DIR } from "../config.js"; import type { ExtensionFlag } from "../core/extensions/types.js"; -import { allTools, type ToolName } from "../core/tools/index.js"; export type Mode = "text" | "json" | "rpc"; @@ -27,7 +26,7 @@ export interface Args { fork?: string; sessionDir?: string; models?: string[]; - tools?: ToolName[]; + tools?: string[]; noTools?: boolean; extensions?: string[]; noExtensions?: boolean; @@ -104,19 +103,10 @@ export function parseArgs(args: string[]): Args { } else if (arg === "--no-tools") { result.noTools = true; } else if (arg === "--tools" && i + 1 < args.length) { - const toolNames = args[++i].split(",").map((s) => s.trim()); - const validTools: ToolName[] = []; - for (const name of toolNames) { - if (name in allTools) { - validTools.push(name as ToolName); - } else { - result.diagnostics.push({ - type: "warning", - message: `Unknown tool "${name}". Valid tools: ${Object.keys(allTools).join(", ")}`, - }); - } - } - result.tools = validTools; + result.tools = args[++i] + .split(",") + .map((s) => s.trim()) + .filter((name) => name.length > 0); } else if (arg === "--thinking" && i + 1 < args.length) { const level = args[++i]; if (isValidThinkingLevel(level)) { @@ -231,9 +221,9 @@ ${chalk.bold("Options:")} --no-session Don't save session (ephemeral) --models Comma-separated model patterns for Ctrl+P cycling Supports globs (anthropic/*, *sonnet*) and fuzzy matching - --no-tools Disable all built-in tools - --tools Comma-separated list of tools to enable (default: read,bash,edit,write) - Available: read, bash, edit, write, grep, find, ls + --no-tools Disable all tools by default (built-in and extension) + --tools Comma-separated allowlist of tool names to enable + Applies to built-in and extension tools --thinking Set thinking level: off, minimal, low, medium, high, xhigh --extension, -e Load an extension file (can be used multiple times) --no-extensions, -ne Disable extension discovery (explicit -e paths still work) @@ -332,7 +322,7 @@ ${chalk.bold("Environment Variables:")} PI_SHARE_VIEWER_URL - Base URL for /share command (default: https://pi.dev/session/) PI_AI_ANTIGRAVITY_VERSION - Override Antigravity User-Agent version (e.g., 1.23.0) -${chalk.bold("Available Tools (default: read, bash, edit, write):")} +${chalk.bold("Built-in Tool Names:")} read - Read file contents bash - Execute bash commands edit - Edit files with find/replace diff --git a/packages/coding-agent/src/core/agent-session-services.ts b/packages/coding-agent/src/core/agent-session-services.ts index e6f9843b..8a9fe0e1 100644 --- a/packages/coding-agent/src/core/agent-session-services.ts +++ b/packages/coding-agent/src/core/agent-session-services.ts @@ -9,7 +9,6 @@ import { DefaultResourceLoader, type DefaultResourceLoaderOptions, type Resource import { type CreateAgentSessionResult, createAgentSession } from "./sdk.js"; import type { SessionManager } from "./session-manager.js"; import { SettingsManager } from "./settings-manager.js"; -import type { Tool } from "./tools/index.js"; /** * Non-fatal issues collected while creating services or sessions. @@ -53,7 +52,7 @@ export interface CreateAgentSessionFromServicesOptions { model?: Model; thinkingLevel?: ThinkingLevel; scopedModels?: Array<{ model: Model; thinkingLevel?: ThinkingLevel }>; - tools?: Tool[]; + tools?: string[]; customTools?: ToolDefinition[]; } diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index 64c16c59..86a9c3f0 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -150,6 +150,8 @@ export interface AgentSessionConfig { modelRegistry: ModelRegistry; /** Initial active built-in tool names. Default: [read, bash, edit, write] */ initialActiveToolNames?: string[]; + /** Optional allowlist of tool names. When provided, only these tool names are exposed. */ + allowedToolNames?: string[]; /** * Override base tools (useful for custom runtimes). * @@ -278,6 +280,7 @@ export class AgentSession { private _cwd: string; private _extensionRunnerRef?: { current?: ExtensionRunner }; private _initialActiveToolNames?: string[]; + private _allowedToolNames?: Set; private _baseToolsOverride?: Record; private _sessionStartEvent: SessionStartEvent; private _extensionUIContext?: ExtensionUIContext; @@ -309,6 +312,7 @@ export class AgentSession { this._modelRegistry = config.modelRegistry; this._extensionRunnerRef = config.extensionRunnerRef; this._initialActiveToolNames = config.initialActiveToolNames; + this._allowedToolNames = config.allowedToolNames ? new Set(config.allowedToolNames) : undefined; this._baseToolsOverride = config.baseToolsOverride; this._sessionStartEvent = config.sessionStartEvent ?? { type: "session_start", reason: "startup" }; @@ -2221,6 +2225,8 @@ export class AgentSession { private _refreshToolRegistry(options?: { activeToolNames?: string[]; includeAllExtensionTools?: boolean }): void { const previousRegistryNames = new Set(this._toolRegistry.keys()); const previousActiveToolNames = this.getActiveToolNames(); + const allowedToolNames = this._allowedToolNames; + const isAllowedTool = (name: string): boolean => !allowedToolNames || allowedToolNames.has(name); const registeredTools = this._extensionRunner.getAllRegisteredTools(); const allCustomTools = [ @@ -2229,15 +2235,17 @@ export class AgentSession { definition, sourceInfo: createSyntheticSourceInfo(``, { source: "sdk" }), })), - ]; + ].filter((tool) => isAllowedTool(tool.definition.name)); const definitionRegistry = new Map( - Array.from(this._baseToolDefinitions.entries()).map(([name, definition]) => [ - name, - { - definition, - sourceInfo: createSyntheticSourceInfo(``, { source: "builtin" }), - }, - ]), + Array.from(this._baseToolDefinitions.entries()) + .filter(([name]) => isAllowedTool(name)) + .map(([name, definition]) => [ + name, + { + definition, + sourceInfo: createSyntheticSourceInfo(``, { source: "builtin" }), + }, + ]), ); for (const tool of allCustomTools) { definitionRegistry.set(tool.definition.name, { @@ -2265,10 +2273,12 @@ export class AgentSession { const runner = this._extensionRunner; const wrappedExtensionTools = wrapRegisteredTools(allCustomTools, runner); const wrappedBuiltInTools = wrapRegisteredTools( - Array.from(this._baseToolDefinitions.values()).map((definition) => ({ - definition, - sourceInfo: createSyntheticSourceInfo(``, { source: "builtin" }), - })), + Array.from(this._baseToolDefinitions.values()) + .filter((definition) => isAllowedTool(definition.name)) + .map((definition) => ({ + definition, + sourceInfo: createSyntheticSourceInfo(``, { source: "builtin" }), + })), runner, ); @@ -2278,11 +2288,17 @@ export class AgentSession { } this._toolRegistry = toolRegistry; - const nextActiveToolNames = options?.activeToolNames - ? [...options.activeToolNames] - : [...previousActiveToolNames]; + const nextActiveToolNames = ( + options?.activeToolNames ? [...options.activeToolNames] : [...previousActiveToolNames] + ).filter((name) => isAllowedTool(name)); - if (options?.includeAllExtensionTools) { + if (allowedToolNames) { + for (const toolName of this._toolRegistry.keys()) { + if (allowedToolNames.has(toolName)) { + nextActiveToolNames.push(toolName); + } + } + } else if (options?.includeAllExtensionTools) { for (const tool of wrappedExtensionTools) { nextActiveToolNames.push(tool.name); } diff --git a/packages/coding-agent/src/core/sdk.ts b/packages/coding-agent/src/core/sdk.ts index 1646bed2..ba5b20a3 100644 --- a/packages/coding-agent/src/core/sdk.ts +++ b/packages/coding-agent/src/core/sdk.ts @@ -16,9 +16,6 @@ import { SettingsManager } from "./settings-manager.js"; import { isInstallTelemetryEnabled } from "./telemetry.js"; import { time } from "./timings.js"; import { - allTools, - bashTool, - codingTools, createBashTool, createCodingTools, createEditTool, @@ -28,16 +25,8 @@ import { createReadOnlyTools, createReadTool, createWriteTool, - editTool, - findTool, - grepTool, - lsTool, - readOnlyTools, - readTool, - type Tool, type ToolName, withFileMutationQueue, - writeTool, } from "./tools/index.js"; export interface CreateAgentSessionOptions { @@ -58,8 +47,14 @@ export interface CreateAgentSessionOptions { /** Models available for cycling (Ctrl+P in interactive mode) */ scopedModels?: Array<{ model: Model; thinkingLevel?: ThinkingLevel }>; - /** Built-in tools to use. Default: codingTools [read, bash, edit, write] */ - tools?: Tool[]; + /** + * Optional allowlist of tool names. + * + * When omitted, pi enables the default built-in tools (read, bash, edit, write) + * and leaves extension/custom tools enabled. + * When provided, only the listed tool names are enabled. + */ + tools?: string[]; /** Custom tools to register (in addition to built-in tools). */ customTools?: ToolDefinition[]; @@ -102,17 +97,6 @@ export type { Skill } from "./skills.js"; export type { Tool } from "./tools/index.js"; export { - // Pre-built tools (use process.cwd()) - readTool, - bashTool, - editTool, - writeTool, - grepTool, - findTool, - lsTool, - codingTools, - readOnlyTools, - allTools as allBuiltInTools, withFileMutationQueue, // Tool factories (for custom cwd) createCodingTools, @@ -185,7 +169,7 @@ function getOpenRouterAttributionHeaders( * ``` */ export async function createAgentSession(options: CreateAgentSessionOptions = {}): Promise { - const cwd = options.cwd ?? process.cwd(); + const cwd = options.cwd ?? options.sessionManager?.getCwd() ?? process.cwd(); const agentDir = options.agentDir ?? getDefaultAgentDir(); let resourceLoader = options.resourceLoader; @@ -261,9 +245,7 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} } const defaultActiveToolNames: ToolName[] = ["read", "bash", "edit", "write"]; - const initialActiveToolNames: ToolName[] = options.tools - ? options.tools.map((t) => t.name).filter((n): n is ToolName => n in allTools) - : defaultActiveToolNames; + const initialActiveToolNames: string[] = options.tools ? [...options.tools] : defaultActiveToolNames; let agent: Agent; @@ -384,6 +366,7 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} customTools: options.customTools, modelRegistry, initialActiveToolNames, + allowedToolNames: options.tools, extensionRunnerRef, sessionStartEvent: options.sessionStartEvent, }); diff --git a/packages/coding-agent/src/core/tools/index.ts b/packages/coding-agent/src/core/tools/index.ts index 6c853345..1043cc85 100644 --- a/packages/coding-agent/src/core/tools/index.ts +++ b/packages/coding-agent/src/core/tools/index.ts @@ -5,8 +5,6 @@ export { type BashToolDetails, type BashToolInput, type BashToolOptions, - bashTool, - bashToolDefinition, createBashTool, createBashToolDefinition, createLocalBashOperations, @@ -18,8 +16,6 @@ export { type EditToolDetails, type EditToolInput, type EditToolOptions, - editTool, - editToolDefinition, } from "./edit.js"; export { withFileMutationQueue } from "./file-mutation-queue.js"; export { @@ -29,8 +25,6 @@ export { type FindToolDetails, type FindToolInput, type FindToolOptions, - findTool, - findToolDefinition, } from "./find.js"; export { createGrepTool, @@ -39,8 +33,6 @@ export { type GrepToolDetails, type GrepToolInput, type GrepToolOptions, - grepTool, - grepToolDefinition, } from "./grep.js"; export { createLsTool, @@ -49,8 +41,6 @@ export { type LsToolDetails, type LsToolInput, type LsToolOptions, - lsTool, - lsToolDefinition, } from "./ls.js"; export { createReadTool, @@ -59,8 +49,6 @@ export { type ReadToolDetails, type ReadToolInput, type ReadToolOptions, - readTool, - readToolDefinition, } from "./read.js"; export { DEFAULT_MAX_BYTES, @@ -78,80 +66,90 @@ export { type WriteOperations, type WriteToolInput, type WriteToolOptions, - writeTool, - writeToolDefinition, } from "./write.js"; import type { AgentTool } from "@mariozechner/pi-agent-core"; import type { ToolDefinition } from "../extensions/types.js"; -import { - type BashToolOptions, - bashTool, - bashToolDefinition, - createBashTool, - createBashToolDefinition, -} from "./bash.js"; -import { createEditTool, createEditToolDefinition, editTool, editToolDefinition } from "./edit.js"; -import { createFindTool, createFindToolDefinition, findTool, findToolDefinition } from "./find.js"; -import { createGrepTool, createGrepToolDefinition, grepTool, grepToolDefinition } from "./grep.js"; -import { createLsTool, createLsToolDefinition, lsTool, lsToolDefinition } from "./ls.js"; -import { - createReadTool, - createReadToolDefinition, - type ReadToolOptions, - readTool, - readToolDefinition, -} from "./read.js"; -import { createWriteTool, createWriteToolDefinition, writeTool, writeToolDefinition } from "./write.js"; +import { type BashToolOptions, createBashTool, createBashToolDefinition } from "./bash.js"; +import { createEditTool, createEditToolDefinition, type EditToolOptions } from "./edit.js"; +import { createFindTool, createFindToolDefinition, type FindToolOptions } from "./find.js"; +import { createGrepTool, createGrepToolDefinition, type GrepToolOptions } from "./grep.js"; +import { createLsTool, createLsToolDefinition, type LsToolOptions } from "./ls.js"; +import { createReadTool, createReadToolDefinition, type ReadToolOptions } from "./read.js"; +import { createWriteTool, createWriteToolDefinition, type WriteToolOptions } from "./write.js"; export type Tool = AgentTool; export type ToolDef = ToolDefinition; - -export const codingTools: Tool[] = [readTool, bashTool, editTool, writeTool]; -export const readOnlyTools: Tool[] = [readTool, grepTool, findTool, lsTool]; - -export const allTools = { - read: readTool, - bash: bashTool, - edit: editTool, - write: writeTool, - grep: grepTool, - find: findTool, - ls: lsTool, -}; - -export const allToolDefinitions = { - read: readToolDefinition, - bash: bashToolDefinition, - edit: editToolDefinition, - write: writeToolDefinition, - grep: grepToolDefinition, - find: findToolDefinition, - ls: lsToolDefinition, -}; - -export type ToolName = keyof typeof allTools; +export type ToolName = "read" | "bash" | "edit" | "write" | "grep" | "find" | "ls"; +export const allToolNames: Set = new Set(["read", "bash", "edit", "write", "grep", "find", "ls"]); export interface ToolsOptions { read?: ReadToolOptions; bash?: BashToolOptions; + write?: WriteToolOptions; + edit?: EditToolOptions; + grep?: GrepToolOptions; + find?: FindToolOptions; + ls?: LsToolOptions; +} + +export function createToolDefinition(toolName: ToolName, cwd: string, options?: ToolsOptions): ToolDef { + switch (toolName) { + case "read": + return createReadToolDefinition(cwd, options?.read); + case "bash": + return createBashToolDefinition(cwd, options?.bash); + case "edit": + return createEditToolDefinition(cwd, options?.edit); + case "write": + return createWriteToolDefinition(cwd, options?.write); + case "grep": + return createGrepToolDefinition(cwd, options?.grep); + case "find": + return createFindToolDefinition(cwd, options?.find); + case "ls": + return createLsToolDefinition(cwd, options?.ls); + default: + throw new Error(`Unknown tool name: ${toolName}`); + } +} + +export function createTool(toolName: ToolName, cwd: string, options?: ToolsOptions): Tool { + switch (toolName) { + case "read": + return createReadTool(cwd, options?.read); + case "bash": + return createBashTool(cwd, options?.bash); + case "edit": + return createEditTool(cwd, options?.edit); + case "write": + return createWriteTool(cwd, options?.write); + case "grep": + return createGrepTool(cwd, options?.grep); + case "find": + return createFindTool(cwd, options?.find); + case "ls": + return createLsTool(cwd, options?.ls); + default: + throw new Error(`Unknown tool name: ${toolName}`); + } } export function createCodingToolDefinitions(cwd: string, options?: ToolsOptions): ToolDef[] { return [ createReadToolDefinition(cwd, options?.read), createBashToolDefinition(cwd, options?.bash), - createEditToolDefinition(cwd), - createWriteToolDefinition(cwd), + createEditToolDefinition(cwd, options?.edit), + createWriteToolDefinition(cwd, options?.write), ]; } export function createReadOnlyToolDefinitions(cwd: string, options?: ToolsOptions): ToolDef[] { return [ createReadToolDefinition(cwd, options?.read), - createGrepToolDefinition(cwd), - createFindToolDefinition(cwd), - createLsToolDefinition(cwd), + createGrepToolDefinition(cwd, options?.grep), + createFindToolDefinition(cwd, options?.find), + createLsToolDefinition(cwd, options?.ls), ]; } @@ -159,11 +157,11 @@ export function createAllToolDefinitions(cwd: string, options?: ToolsOptions): R return { read: createReadToolDefinition(cwd, options?.read), bash: createBashToolDefinition(cwd, options?.bash), - edit: createEditToolDefinition(cwd), - write: createWriteToolDefinition(cwd), - grep: createGrepToolDefinition(cwd), - find: createFindToolDefinition(cwd), - ls: createLsToolDefinition(cwd), + edit: createEditToolDefinition(cwd, options?.edit), + write: createWriteToolDefinition(cwd, options?.write), + grep: createGrepToolDefinition(cwd, options?.grep), + find: createFindToolDefinition(cwd, options?.find), + ls: createLsToolDefinition(cwd, options?.ls), }; } @@ -171,23 +169,28 @@ export function createCodingTools(cwd: string, options?: ToolsOptions): Tool[] { return [ createReadTool(cwd, options?.read), createBashTool(cwd, options?.bash), - createEditTool(cwd), - createWriteTool(cwd), + createEditTool(cwd, options?.edit), + createWriteTool(cwd, options?.write), ]; } export function createReadOnlyTools(cwd: string, options?: ToolsOptions): Tool[] { - return [createReadTool(cwd, options?.read), createGrepTool(cwd), createFindTool(cwd), createLsTool(cwd)]; + return [ + createReadTool(cwd, options?.read), + createGrepTool(cwd, options?.grep), + createFindTool(cwd, options?.find), + createLsTool(cwd, options?.ls), + ]; } export function createAllTools(cwd: string, options?: ToolsOptions): Record { return { read: createReadTool(cwd, options?.read), bash: createBashTool(cwd, options?.bash), - edit: createEditTool(cwd), - write: createWriteTool(cwd), - grep: createGrepTool(cwd), - find: createFindTool(cwd), - ls: createLsTool(cwd), + edit: createEditTool(cwd, options?.edit), + write: createWriteTool(cwd, options?.write), + grep: createGrepTool(cwd, options?.grep), + find: createFindTool(cwd, options?.find), + ls: createLsTool(cwd, options?.ls), }; } diff --git a/packages/coding-agent/src/main.ts b/packages/coding-agent/src/main.ts index a8b7e8c8..2fa1d390 100644 --- a/packages/coding-agent/src/main.ts +++ b/packages/coding-agent/src/main.ts @@ -39,7 +39,6 @@ import { import { SessionManager } from "./core/session-manager.js"; import { SettingsManager } from "./core/settings-manager.js"; import { printTimings, resetTimings, time } from "./core/timings.js"; -import { allTools } from "./core/tools/index.js"; import { runMigrations, showDeprecationWarnings } from "./migrations.js"; import { InteractiveMode, runPrintMode, runRpcMode } from "./modes/index.js"; import { ExtensionSelectorComponent } from "./modes/interactive/components/extension-selector.js"; @@ -368,14 +367,10 @@ function buildSessionOptions( // Tools if (parsed.noTools) { // --no-tools: start with no built-in tools - // --tools can still add specific ones back - if (parsed.tools && parsed.tools.length > 0) { - options.tools = parsed.tools.map((name) => allTools[name]); - } else { - options.tools = []; - } + // --tools can still add specific ones back, including extension tools. + options.tools = parsed.tools && parsed.tools.length > 0 ? [...parsed.tools] : []; } else if (parsed.tools) { - options.tools = parsed.tools.map((name) => allTools[name]); + options.tools = [...parsed.tools]; } return { options, cliThinkingFromModel, diagnostics }; diff --git a/packages/coding-agent/test/agent-session-branching.test.ts b/packages/coding-agent/test/agent-session-branching.test.ts index 128699ea..2d43b8b2 100644 --- a/packages/coding-agent/test/agent-session-branching.test.ts +++ b/packages/coding-agent/test/agent-session-branching.test.ts @@ -22,7 +22,6 @@ import { } from "../src/core/agent-session-runtime.js"; import { AuthStorage } from "../src/core/auth-storage.js"; import { SessionManager } from "../src/core/session-manager.js"; -import { codingTools } from "../src/core/tools/index.js"; import { API_KEY } from "./utilities.js"; describe.skipIf(!API_KEY)("AgentSession forking", () => { @@ -40,7 +39,6 @@ describe.skipIf(!API_KEY)("AgentSession forking", () => { if (runtimeHost) { await runtimeHost.dispose(); } - process.chdir(tmpdir()); if (tempDir && existsSync(tempDir)) { rmSync(tempDir, { recursive: true }); } @@ -73,7 +71,7 @@ describe.skipIf(!API_KEY)("AgentSession forking", () => { sessionManager, sessionStartEvent, model, - tools: codingTools, + tools: ["read", "bash", "edit", "write"], })), services, diagnostics: services.diagnostics, diff --git a/packages/coding-agent/test/agent-session-runtime-events.test.ts b/packages/coding-agent/test/agent-session-runtime-events.test.ts index d47dce97..06a33d49 100644 --- a/packages/coding-agent/test/agent-session-runtime-events.test.ts +++ b/packages/coding-agent/test/agent-session-runtime-events.test.ts @@ -76,7 +76,6 @@ describe("AgentSessionRuntime session lifecycle events", () => { cleanups.push(async () => { await runtimeHost.dispose(); faux.unregister(); - process.chdir(tmpdir()); if (existsSync(tempDir)) { rmSync(tempDir, { recursive: true, force: true }); } diff --git a/packages/coding-agent/test/compaction-thinking-model.test.ts b/packages/coding-agent/test/compaction-thinking-model.test.ts index 9e4ce7dc..2417ca82 100644 --- a/packages/coding-agent/test/compaction-thinking-model.test.ts +++ b/packages/coding-agent/test/compaction-thinking-model.test.ts @@ -18,7 +18,7 @@ import { AgentSession } from "../src/core/agent-session.js"; import { ModelRegistry } from "../src/core/model-registry.js"; import { SessionManager } from "../src/core/session-manager.js"; import { SettingsManager } from "../src/core/settings-manager.js"; -import { codingTools } from "../src/core/tools/index.js"; +import { createCodingTools } from "../src/core/tools/index.js"; import { API_KEY, createTestResourceLoader, @@ -70,7 +70,7 @@ describe.skipIf(!HAS_ANTIGRAVITY_AUTH)("Compaction with thinking models (Antigra initialState: { model, systemPrompt: "You are a helpful assistant. Be concise.", - tools: codingTools, + tools: createCodingTools(process.cwd()), thinkingLevel, }, }); @@ -168,7 +168,7 @@ describe.skipIf(!HAS_ANTHROPIC_AUTH)("Compaction with thinking models (Anthropic initialState: { model, systemPrompt: "You are a helpful assistant. Be concise.", - tools: codingTools, + tools: createCodingTools(process.cwd()), thinkingLevel, }, }); diff --git a/packages/coding-agent/test/sdk-session-manager.test.ts b/packages/coding-agent/test/sdk-session-manager.test.ts index 554660b5..e0557ea7 100644 --- a/packages/coding-agent/test/sdk-session-manager.test.ts +++ b/packages/coding-agent/test/sdk-session-manager.test.ts @@ -1,4 +1,4 @@ -import { existsSync, mkdirSync, rmSync } from "node:fs"; +import { existsSync, mkdirSync, realpathSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { getModel } from "@mariozechner/pi-ai"; @@ -63,4 +63,33 @@ describe("createAgentSession session manager defaults", () => { session.dispose(); }); + + it("derives cwd from an explicit sessionManager when cwd is omitted", async () => { + const model = getModel("anthropic", "claude-sonnet-4-5"); + expect(model).toBeTruthy(); + + const sessionCwd = join(tempDir, "session-project"); + mkdirSync(sessionCwd, { recursive: true }); + const sessionManager = SessionManager.inMemory(sessionCwd); + const { session } = await createAgentSession({ + agentDir, + model: model!, + sessionManager, + }); + + expect(session.sessionManager).toBe(sessionManager); + expect(session.systemPrompt).toContain(`Current working directory: ${sessionCwd}`); + + const bashTool = session.agent.state.tools.find((tool) => tool.name === "bash"); + expect(bashTool).toBeTruthy(); + const result = await bashTool!.execute("test", { command: "pwd" }); + const output = result.content + .filter((item): item is { type: "text"; text: string } => item.type === "text") + .map((item) => item.text) + .join(""); + + expect(realpathSync(output.trim())).toBe(realpathSync(sessionCwd)); + + session.dispose(); + }); }); diff --git a/packages/coding-agent/test/suite/agent-session-runtime.test.ts b/packages/coding-agent/test/suite/agent-session-runtime.test.ts index 55de188e..ed88c7d9 100644 --- a/packages/coding-agent/test/suite/agent-session-runtime.test.ts +++ b/packages/coding-agent/test/suite/agent-session-runtime.test.ts @@ -28,7 +28,6 @@ describe("AgentSessionRuntime characterization", () => { while (cleanups.length > 0) { await cleanups.pop()?.(); } - process.chdir(tmpdir()); }); async function createRuntimeForTest( @@ -391,7 +390,7 @@ describe("AgentSessionRuntime characterization", () => { await expect(runtime.fork("missing-entry")).rejects.toThrow("Invalid entry ID for forking"); }); - it("updates process.cwd() on cross-cwd session replacement", async () => { + it("updates the runtime session cwd on cross-cwd session replacement", async () => { const firstDir = join(tmpdir(), `pi-runtime-cwd-a-${Date.now()}-${Math.random().toString(36).slice(2)}`); const secondDir = join(tmpdir(), `pi-runtime-cwd-b-${Date.now()}-${Math.random().toString(36).slice(2)}`); mkdirSync(firstDir, { recursive: true }); @@ -459,8 +458,8 @@ describe("AgentSessionRuntime characterization", () => { await runtime.switchSession(otherSessionFile); - expect(realpathSync(process.cwd())).toBe(realpathSync(secondDir)); expect(realpathSync(runtime.session.sessionManager.getCwd())).toBe(realpathSync(secondDir)); + expect(realpathSync(runtime.cwd)).toBe(realpathSync(secondDir)); }); it("restores model and thinking state from the destination session", async () => { diff --git a/packages/coding-agent/test/suite/regressions/2835-tools-allowlist-filters-extension-tools.test.ts b/packages/coding-agent/test/suite/regressions/2835-tools-allowlist-filters-extension-tools.test.ts new file mode 100644 index 00000000..333bbf06 --- /dev/null +++ b/packages/coding-agent/test/suite/regressions/2835-tools-allowlist-filters-extension-tools.test.ts @@ -0,0 +1,94 @@ +import { existsSync, mkdirSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { getModel } from "@mariozechner/pi-ai"; +import { Type } from "@sinclair/typebox"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { DefaultResourceLoader } from "../../../src/core/resource-loader.js"; +import { createAgentSession } from "../../../src/core/sdk.js"; +import { SessionManager } from "../../../src/core/session-manager.js"; +import { SettingsManager } from "../../../src/core/settings-manager.js"; + +describe("regression #2835: tool allowlists filter extension tools", () => { + let tempDir: string; + let agentDir: string; + + beforeEach(() => { + tempDir = join(tmpdir(), `pi-tools-filter-${Date.now()}-${Math.random().toString(36).slice(2)}`); + agentDir = join(tempDir, "agent"); + mkdirSync(agentDir, { recursive: true }); + }); + + afterEach(() => { + if (tempDir && existsSync(tempDir)) { + rmSync(tempDir, { recursive: true, force: true }); + } + }); + + async function createSession(allowedToolNames?: string[]) { + const settingsManager = SettingsManager.create(tempDir, agentDir); + const sessionManager = SessionManager.inMemory(tempDir); + const resourceLoader = new DefaultResourceLoader({ + cwd: tempDir, + agentDir, + settingsManager, + extensionFactories: [ + (pi) => { + pi.on("session_start", () => { + pi.registerTool({ + name: "dynamic_tool", + label: "Dynamic Tool", + description: "Tool registered from session_start", + promptSnippet: "Run dynamic test behavior", + parameters: Type.Object({}), + execute: async () => ({ + content: [{ type: "text", text: "ok" }], + details: {}, + }), + }); + }); + }, + ], + }); + await resourceLoader.reload(); + + const { session } = await createAgentSession({ + cwd: tempDir, + agentDir, + model: getModel("anthropic", "claude-sonnet-4-5")!, + settingsManager, + sessionManager, + resourceLoader, + tools: allowedToolNames, + }); + await session.bindExtensions({}); + return session; + } + + it("allows only explicitly listed built-in and extension tools", async () => { + const session = await createSession(["read", "dynamic_tool"]); + + expect( + session + .getAllTools() + .map((tool) => tool.name) + .sort(), + ).toEqual(["dynamic_tool", "read"]); + expect(session.getActiveToolNames().sort()).toEqual(["dynamic_tool", "read"]); + expect(session.systemPrompt).toContain("- read: Read file contents"); + expect(session.systemPrompt).toContain("- dynamic_tool: Run dynamic test behavior"); + expect(session.systemPrompt).not.toContain("- bash:"); + expect(session.systemPrompt).not.toContain("- edit:"); + session.dispose(); + }); + + it("disables all tools when the allowlist is empty", async () => { + const session = await createSession([]); + + expect(session.getAllTools()).toEqual([]); + expect(session.getActiveToolNames()).toEqual([]); + expect(session.systemPrompt).toContain("Available tools:\n(none)"); + expect(session.systemPrompt).not.toContain("dynamic_tool"); + session.dispose(); + }); +});