diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 287b835c..533c8eba 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -9,6 +9,7 @@ ### Fixed - Fixed crash on `/quit` when an extension registers a custom footer whose `render()` accesses `ctx`, by tearing down extension-provided UI before invalidating the extension runner during shutdown ([#3595](https://github.com/badlogic/pi-mono/issues/3595)) +- Fixed the CLI/SDK tool-selection split so `--no-builtin-tools` and `createAgentSession({ noTools: "builtin" })` disable only built-in default tools while keeping extension/custom tools enabled, instead of falling through to the same "disable everything" path as `--no-tools` ([#3592](https://github.com/badlogic/pi-mono/issues/3592)) - Fixed `pi-coding-agent` shipping `uuid@11`, which triggered `npm audit` moderate vulnerability reports for downstream installs; the package now depends on `uuid@14` ([#3577](https://github.com/badlogic/pi-mono/issues/3577)) - Fixed `ctx.ui.setWorkingMessage()` to persist across loader recreation, matching the behavior of `ctx.ui.setWorkingIndicator()` ([#3566](https://github.com/badlogic/pi-mono/issues/3566)) - Fixed coding-agent `fs.watch` error handling for theme and git-footer watchers to retry after transient watcher failures such as `EMFILE`, avoiding startup crashes in large repos ([#3564](https://github.com/badlogic/pi-mono/issues/3564)) diff --git a/packages/coding-agent/README.md b/packages/coding-agent/README.md index a6247083..6ed1b438 100644 --- a/packages/coding-agent/README.md +++ b/packages/coding-agent/README.md @@ -522,8 +522,9 @@ cat README.md | pi -p "Summarize this text" | Option | Description | |--------|-------------| -| `--tools ` | Enable specific built-in tools (default: `read,bash,edit,write`) | -| `--no-tools` | Disable all built-in tools (extension tools still work) | +| `--tools `, `-t ` | Allowlist specific tool names across built-in, extension, and custom tools | +| `--no-builtin-tools`, `-nbt` | Disable built-in tools by default but keep extension/custom tools enabled | +| `--no-tools`, `-nt` | Disable all tools by default | Available built-in tools: `read`, `bash`, `edit`, `write`, `grep`, `find`, `ls` diff --git a/packages/coding-agent/docs/extensions.md b/packages/coding-agent/docs/extensions.md index 3b07a369..56da258b 100644 --- a/packages/coding-agent/docs/extensions.md +++ b/packages/coding-agent/docs/extensions.md @@ -1785,10 +1785,10 @@ Extensions can override built-in tools (`read`, `bash`, `edit`, `write`, `grep`, pi -e ./tool-override.ts ``` -Alternatively, use `--no-tools` to start without any built-in tools: +Alternatively, use `--no-builtin-tools` to start without any built-in tools while keeping extension tools enabled: ```bash # No built-in tools, only extension tools -pi --no-tools -e ./my-extension.ts +pi --no-builtin-tools -e ./my-extension.ts ``` See [examples/extensions/tool-override.ts](../examples/extensions/tool-override.ts) for a complete example that overrides `read` with logging and access control. diff --git a/packages/coding-agent/src/cli/args.ts b/packages/coding-agent/src/cli/args.ts index 70f07ca8..08827c1a 100644 --- a/packages/coding-agent/src/cli/args.ts +++ b/packages/coding-agent/src/cli/args.ts @@ -28,6 +28,7 @@ export interface Args { models?: string[]; tools?: string[]; noTools?: boolean; + noBuiltinTools?: boolean; extensions?: string[]; noExtensions?: boolean; print?: boolean; @@ -100,9 +101,11 @@ export function parseArgs(args: string[]): Args { result.sessionDir = args[++i]; } else if (arg === "--models" && i + 1 < args.length) { result.models = args[++i].split(",").map((s) => s.trim()); - } else if (arg === "--no-tools") { + } else if (arg === "--no-tools" || arg === "-nt") { result.noTools = true; - } else if (arg === "--tools" && i + 1 < args.length) { + } else if (arg === "--no-builtin-tools" || arg === "-nbt") { + result.noBuiltinTools = true; + } else if ((arg === "--tools" || arg === "-t") && i + 1 < args.length) { result.tools = args[++i] .split(",") .map((s) => s.trim()) @@ -221,9 +224,10 @@ ${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 tools by default (built-in and extension) - --tools Comma-separated allowlist of tool names to enable - Applies to built-in and extension tools + --no-tools, -nt Disable all tools by default (built-in and extension) + --no-builtin-tools, -nbt Disable built-in tools by default but keep extension/custom tools enabled + --tools, -t Comma-separated allowlist of tool names to enable + Applies to built-in, extension, and custom 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) diff --git a/packages/coding-agent/src/core/sdk.ts b/packages/coding-agent/src/core/sdk.ts index ba5b20a3..58ce0af7 100644 --- a/packages/coding-agent/src/core/sdk.ts +++ b/packages/coding-agent/src/core/sdk.ts @@ -47,11 +47,19 @@ export interface CreateAgentSessionOptions { /** Models available for cycling (Ctrl+P in interactive mode) */ scopedModels?: Array<{ model: Model; thinkingLevel?: ThinkingLevel }>; + /** + * Optional default tool suppression mode when no explicit allowlist is provided. + * + * - "all": start with no tools enabled + * - "builtin": disable the default built-in tools (read, bash, edit, write) + * but keep extension/custom tools enabled + */ + noTools?: "all" | "builtin"; /** * Optional allowlist of tool names. * * When omitted, pi enables the default built-in tools (read, bash, edit, write) - * and leaves extension/custom tools enabled. + * and leaves extension/custom tools enabled unless `noTools` changes that default. * When provided, only the listed tool names are enabled. */ tools?: string[]; @@ -245,7 +253,12 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} } const defaultActiveToolNames: ToolName[] = ["read", "bash", "edit", "write"]; - const initialActiveToolNames: string[] = options.tools ? [...options.tools] : defaultActiveToolNames; + const allowedToolNames = options.tools ?? (options.noTools === "all" ? [] : undefined); + const initialActiveToolNames: string[] = options.tools + ? [...options.tools] + : options.noTools + ? [] + : defaultActiveToolNames; let agent: Agent; @@ -366,7 +379,7 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} customTools: options.customTools, modelRegistry, initialActiveToolNames, - allowedToolNames: options.tools, + allowedToolNames, extensionRunnerRef, sessionStartEvent: options.sessionStartEvent, }); diff --git a/packages/coding-agent/src/main.ts b/packages/coding-agent/src/main.ts index 2fa1d390..bb0e5e63 100644 --- a/packages/coding-agent/src/main.ts +++ b/packages/coding-agent/src/main.ts @@ -366,10 +366,11 @@ function buildSessionOptions( // Tools if (parsed.noTools) { - // --no-tools: start with no built-in 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.noTools = "all"; + } else if (parsed.noBuiltinTools) { + options.noTools = "builtin"; + } + if (parsed.tools) { options.tools = [...parsed.tools]; } diff --git a/packages/coding-agent/test/args.test.ts b/packages/coding-agent/test/args.test.ts index d1739c69..3e53d7f6 100644 --- a/packages/coding-agent/test/args.test.ts +++ b/packages/coding-agent/test/args.test.ts @@ -257,17 +257,48 @@ describe("parseArgs", () => { }); }); - describe("--no-tools flag", () => { + describe("tool flags", () => { test("parses --no-tools flag", () => { const result = parseArgs(["--no-tools"]); expect(result.noTools).toBe(true); }); + test("parses -nt shorthand", () => { + const result = parseArgs(["-nt"]); + expect(result.noTools).toBe(true); + }); + + test("parses --no-builtin-tools flag", () => { + const result = parseArgs(["--no-builtin-tools"]); + expect(result.noBuiltinTools).toBe(true); + }); + + test("parses -nbt shorthand", () => { + const result = parseArgs(["-nbt"]); + expect(result.noBuiltinTools).toBe(true); + }); + + test("parses --tools flag", () => { + const result = parseArgs(["--tools", "read,bash"]); + expect(result.tools).toEqual(["read", "bash"]); + }); + + test("parses -t shorthand", () => { + const result = parseArgs(["-t", "read,bash"]); + expect(result.tools).toEqual(["read", "bash"]); + }); + test("parses --no-tools with explicit --tools flags", () => { const result = parseArgs(["--no-tools", "--tools", "read,bash"]); expect(result.noTools).toBe(true); expect(result.tools).toEqual(["read", "bash"]); }); + + test("parses --no-builtin-tools with explicit --tools flags", () => { + const result = parseArgs(["--no-builtin-tools", "--tools", "read,bash"]); + expect(result.noBuiltinTools).toBe(true); + expect(result.tools).toEqual(["read", "bash"]); + }); }); describe("messages and file args", () => { diff --git a/packages/coding-agent/test/suite/regressions/3592-no-builtin-tools-keeps-extension-tools.test.ts b/packages/coding-agent/test/suite/regressions/3592-no-builtin-tools-keeps-extension-tools.test.ts new file mode 100644 index 00000000..3dda3bc6 --- /dev/null +++ b/packages/coding-agent/test/suite/regressions/3592-no-builtin-tools-keeps-extension-tools.test.ts @@ -0,0 +1,93 @@ +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 "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 #3592: no-builtin-tools keeps extension tools enabled", () => { + let tempDir: string; + let agentDir: string; + + beforeEach(() => { + tempDir = join(tmpdir(), `pi-no-builtin-tools-${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(options?: { noTools?: "all" | "builtin"; tools?: 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, + noTools: options?.noTools, + tools: options?.tools, + }); + await session.bindExtensions({}); + return session; + } + + it("keeps extension tools active when built-in defaults are disabled", async () => { + const session = await createSession({ noTools: "builtin" }); + + expect( + session + .getAllTools() + .map((tool) => tool.name) + .sort(), + ).toEqual(["bash", "dynamic_tool", "edit", "find", "grep", "ls", "read", "write"]); + expect(session.getActiveToolNames()).toEqual(["dynamic_tool"]); + expect(session.systemPrompt).toContain("- dynamic_tool: Run dynamic test behavior"); + expect(session.systemPrompt).not.toContain("- read:"); + expect(session.systemPrompt).not.toContain("- bash:"); + session.dispose(); + }); + + it("still disables all tools when noTools is all", async () => { + const session = await createSession({ noTools: "all" }); + + expect(session.getAllTools()).toEqual([]); + expect(session.getActiveToolNames()).toEqual([]); + expect(session.systemPrompt).toContain("Available tools:\n(none)"); + session.dispose(); + }); +});