From c30840c2eb894c43dc2cd0122b05eb9ef30fc185 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Mon, 5 Oct 2026 18:49:43 +0200 Subject: [PATCH] fix(coding-agent): keep hidden tools out of prompt rules and skills hint Rules and the tool list follow the declared tools (active minus hiddenDeclarations). The skills hint names no tool when the reader is hidden. Codemode shows each tool's prompt guidelines with its declaration, via the new ToolLoadout.getPromptGuidelines(). closes #10343 --- packages/coding-agent/CHANGELOG.md | 1 + packages/coding-agent/docs/codemode.md | 2 +- packages/coding-agent/docs/extensions.md | 2 +- .../coding-agent/src/core/agent-session.ts | 12 ++++---- .../coding-agent/src/core/extensions/types.ts | 2 ++ packages/coding-agent/src/core/skills.ts | 9 ++++-- .../coding-agent/src/core/system-prompt.ts | 19 ++++++++++-- .../src/extensions/codemode/execute.ts | 5 +++- .../src/extensions/codemode/index.ts | 2 ++ .../src/extensions/codemode/tool.ts | 23 +++++++++++--- .../test/suite/agent-session-codemode.test.ts | 22 ++++++++++++++ .../coding-agent/test/system-prompt.test.ts | 30 +++++++++++++++++++ 12 files changed, 111 insertions(+), 18 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 9be34850c..a6a0be885 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -14,6 +14,7 @@ - Fixed MCP OAuth sign-in failing with `invalid_redirect_uri` on servers with OpenID Connect client registration, such as `mcp.modem.dev`: pi now registers as a native client ([#10493](https://github.com/earendil-works/pi/issues/10493)) - Fixed `--tools` removing MCP tools, which left `pi --tools codemode` without any MCP servers. `--tools` now keeps MCP tools unless an entry starts with `mcp__` - Fixed MCP session shutdown returning while a server was still connecting, leaving its transport open until the server answered or timed out ([#10249](https://github.com/earendil-works/pi/issues/10249)) +- Fixed system prompt rules and the skills hint naming tools hidden by `prepareLoadout`. Hidden tools are left out of the rules, the skills hint names no tool when the file reader is hidden, and `codemode` shows each tool's prompt guidelines with its declaration; `ToolLoadout` gains `getPromptGuidelines()` ([#10343](https://github.com/earendil-works/pi/issues/10343)) ## [1.0.3] - 2026-10-05 diff --git a/packages/coding-agent/docs/codemode.md b/packages/coding-agent/docs/codemode.md index 764413ce2..d79f987c9 100644 --- a/packages/coding-agent/docs/codemode.md +++ b/packages/coding-agent/docs/codemode.md @@ -49,7 +49,7 @@ A call that fails, is blocked, or gets invalid arguments rejects with an `Error` The `codemode` description lists tools with their TypeScript declarations, grouped by namespace (for example one MCP server). Tools with `deferred` exposure, which includes MCP tools with the default `codemode` exposure, are not listed, so the description stays the same while MCP servers connect. Listed declarations share a budget of 3000 estimated tokens (`codemode.inlineBudget` in [settings](settings.md#tools)). Scripts find the other tools with `searchTools()`, `describeTool()`, `describeNamespace()`, or by filtering `ALL_TOOLS`. -While `codemode` is active, `codemode.mode` in [settings](settings.md#tools) decides how the other tools are presented. With `on` (default) declared tools stay declared, and their descriptions say how to call them from scripts. With `only` they are hidden from the model and listed in the `codemode` description instead, so the model calls them through scripts. +While `codemode` is active, `codemode.mode` in [settings](settings.md#tools) decides how the other tools are presented. With `on` (default) declared tools stay declared, and their descriptions say how to call them from scripts. With `only` they are hidden from the model and listed in the `codemode` description instead, so the model calls them through scripts. Tool declarations in the `codemode` description, `describeTool()`, and `ALL_TOOLS` carry the tools' prompt guidelines, since the system prompt rules only cover declared tools. ## Store values diff --git a/packages/coding-agent/docs/extensions.md b/packages/coding-agent/docs/extensions.md index e389d0338..c350e6626 100644 --- a/packages/coding-agent/docs/extensions.md +++ b/packages/coding-agent/docs/extensions.md @@ -177,7 +177,7 @@ pi.on("tool_call", async (event, ctx) => { }); ``` -A tool that orchestrates other tools can adjust what the model sees while it is active with `prepareLoadout(loadout)`. It runs whenever the active tools change and receives the declared tools, the callable tools, and every registered tool with its exposure and namespace. It returns replacement `descriptions` for declared tools (including its own) and `hiddenDeclarations`: active tools whose declarations requests leave out while they stay active and callable. `codemode` uses only this hook, `exposure`, and `ctx.executeTool()`, so another tool can implement the same behavior under a different name. +A tool that orchestrates other tools can adjust what the model sees while it is active with `prepareLoadout(loadout)`. It runs whenever the active tools change and receives the declared tools, the callable tools, and every registered tool with its exposure, namespace, and prompt guidelines. It returns replacement `descriptions` for declared tools (including its own) and `hiddenDeclarations`: active tools whose declarations requests leave out while they stay active and callable. The default system prompt leaves hidden tools out of its tool list and rules, and names no tool in the skills hint when the file reader is hidden, so the orchestrating tool should present their guidelines itself. `codemode` uses only this hook, `exposure`, and `ctx.executeTool()`, so another tool can implement the same behavior under a different name. ### Activate tools dynamically diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index 6263a8860..95b96350d 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -1576,6 +1576,7 @@ export class AgentSession { registered: [...this._toolRegistry.values()], getExposure: (name) => this._getToolExposure(name), getNamespace: (name) => this._toolDefinitions.get(name)?.definition.namespace, + getPromptGuidelines: (name) => this._toolPromptGuidelines.get(name) ?? [], }; const descriptions = new Map(); for (const { definition, sourceInfo } of hooks) { @@ -1687,8 +1688,8 @@ export class AgentSession { const toolSnippets: Record = {}; for (const name of this._toolRegistry.keys()) { const snippet = this._toolPromptSnippets.get(name); - // Tools without a snippet are not listed. Hidden tools are only callable through another tool. - if (snippet && !this._hiddenDeclarations.has(name)) toolSnippets[name] = snippet; + // Tools without a snippet are not listed. + if (snippet) toolSnippets[name] = snippet; } const loaderSystemPrompt = this._resourceLoader.getSystemPrompt(); @@ -1704,6 +1705,7 @@ export class AgentSession { customPrompt: loaderSystemPrompt, appendSystemPrompt, selectedTools: validToolNames, + hiddenTools: [...this._hiddenDeclarations], toolSnippets, toolGuidelines: Object.fromEntries(this._toolPromptGuidelines), }); @@ -1724,10 +1726,8 @@ export class AgentSession { messages: AgentMessage[] = this.agent.state.messages, ): SystemMessage | undefined { options.selectedTools = this._applyToolLoadout(options.selectedTools).map((tool) => tool.name); - // The tool list must match the declarations the request carries, so hidden tools are not listed. - options.toolSnippets = Object.fromEntries( - Object.entries(options.toolSnippets).filter(([name]) => !this._hiddenDeclarations.has(name)), - ); + // The tool list and rules must match the declarations the request carries. + options.hiddenTools = [...this._hiddenDeclarations]; const sections = diffSystemPromptSections( getCurrentSystemMessage(messages)?.sections ?? {}, buildSystemPromptSections(options), diff --git a/packages/coding-agent/src/core/extensions/types.ts b/packages/coding-agent/src/core/extensions/types.ts index 7eb20f5ec..941cfe8cb 100644 --- a/packages/coding-agent/src/core/extensions/types.ts +++ b/packages/coding-agent/src/core/extensions/types.ts @@ -546,6 +546,8 @@ export interface ToolLoadout { readonly registered: readonly AgentTool[]; getExposure(name: string): ToolExposure; getNamespace(name: string): ToolNamespace | undefined; + /** A tool's `promptGuidelines`. Hidden declarations leave them out of the system prompt. */ + getPromptGuidelines(name: string): readonly string[]; } /** Changes {@link ToolDefinition.prepareLoadout} makes to what the model sees. */ diff --git a/packages/coding-agent/src/core/skills.ts b/packages/coding-agent/src/core/skills.ts index 744c1395a..3fbf2b926 100644 --- a/packages/coding-agent/src/core/skills.ts +++ b/packages/coding-agent/src/core/skills.ts @@ -351,8 +351,11 @@ function loadSkillFromFile( * * Skills with disableModelInvocation=true are excluded from the prompt * (they can only be invoked explicitly via /skill:name commands). + * + * `fileReadTool` names the tool that loads skill files; `indirect` names none, for a reader that + * is reachable only through another tool. */ -export function formatSkillsForPrompt(skills: Skill[], fileReadTool: "read" | "bash" = "read"): string { +export function formatSkillsForPrompt(skills: Skill[], fileReadTool: "read" | "bash" | "indirect" = "read"): string { const visibleSkills = skills.filter((s) => !s.disableModelInvocation); if (visibleSkills.length === 0) { @@ -363,7 +366,9 @@ export function formatSkillsForPrompt(skills: Skill[], fileReadTool: "read" | "b "\n\nThe following skills provide specialized instructions for specific tasks.", fileReadTool === "read" ? "Use the read tool to load a skill's file when the task matches its description." - : "Use bash to load a skill's file when the task matches its description.", + : fileReadTool === "bash" + ? "Use bash to load a skill's file when the task matches its description." + : "Load a skill's file when the task matches its description.", "When a skill file references a relative path, resolve it against the skill directory (parent of SKILL.md / dirname of the path) and use that absolute path in tool commands.", "", "", diff --git a/packages/coding-agent/src/core/system-prompt.ts b/packages/coding-agent/src/core/system-prompt.ts index cd02d6cd7..49bcb58ee 100644 --- a/packages/coding-agent/src/core/system-prompt.ts +++ b/packages/coding-agent/src/core/system-prompt.ts @@ -13,6 +13,11 @@ export interface BuildSystemPromptOptions { forceSystemPrompt?: string; /** Tools to include in prompt. Default: [read, bash, edit, write]. */ selectedTools?: string[]; + /** + * Selected tools whose declarations requests leave out (`prepareLoadout` hidden declarations). + * They are reachable only through another tool, so the tool list and rules leave them out too. + */ + hiddenTools?: string[]; /** Optional one-line tool snippets keyed by tool name. */ toolSnippets?: Record; /** Guideline bullets contributed by each tool, keyed by tool name. */ @@ -33,6 +38,7 @@ export interface BuildSystemPromptOptions { export type NormalizedBuildSystemPromptOptions = BuildSystemPromptOptions & { selectedTools: string[]; + hiddenTools: string[]; toolSnippets: Record; toolGuidelines: Record; promptGuidelines: string[]; @@ -56,6 +62,7 @@ export function normalizeBuildSystemPromptOptions(input: BuildSystemPromptOption customPrompt: input.customPrompt, forceSystemPrompt: input.forceSystemPrompt, selectedTools: [...(input.selectedTools ?? ["read", "bash", "edit", "write"])], + hiddenTools: [...(input.hiddenTools ?? [])], toolSnippets: { ...(input.toolSnippets ?? {}) }, toolGuidelines: Object.fromEntries( Object.entries(input.toolGuidelines ?? {}).map(([name, guidelines]) => [name, [...guidelines]]), @@ -123,6 +130,7 @@ export function buildSystemPromptSections(input: BuildSystemPromptOptions): Syst const { customPrompt, selectedTools, + hiddenTools, toolSnippets, toolGuidelines, promptGuidelines, @@ -139,17 +147,18 @@ export function buildSystemPromptSections(input: BuildSystemPromptOptions): Syst } } + const declaredTools = selectedTools.filter((name) => !hiddenTools.includes(name)); const promptSections: Record = {}; if (customPrompt) { promptSections.preamble = customPrompt; } else { promptSections.preamble = "You are an expert coding assistant operating inside pi, a coding agent harness. You help users by reading files, executing commands, editing code, and writing new files."; - const visibleTools = selectedTools.filter((name) => !!toolSnippets[name]); + const visibleTools = declaredTools.filter((name) => !!toolSnippets[name]); const tools = visibleTools.length > 0 ? visibleTools.map((name) => `- ${name}: ${toolSnippets[name]}`).join("\n") : "(none)"; promptSections.tools = `${tools}\n\nIn addition to the tools above, you may have access to other custom tools depending on the project.`; - promptSections.rules = buildRules(selectedTools, toolGuidelines, promptGuidelines); + promptSections.rules = buildRules(declaredTools, toolGuidelines, promptGuidelines); promptSections.docs = `Pi documentation (read only when the user asks about pi itself, its SDK, extensions, themes, skills, or TUI): - Main documentation: ${getReadmePath()} - Additional docs: ${getDocsPath()} @@ -162,7 +171,11 @@ export function buildSystemPromptSections(input: BuildSystemPromptOptions): Syst if (appendSystemPrompt) promptSections.addendum = appendSystemPrompt; if (contextFiles.length > 0) promptSections.project_context = renderProjectContext(contextFiles); - const skillFileReadTool = (["read", "bash"] as const).find((tool) => selectedTools.includes(tool)); + // A hidden reader is still reachable through another tool, so skills stay but the hint names no tool. + const readers = ["read", "bash"] as const; + const skillFileReadTool = + readers.find((tool) => declaredTools.includes(tool)) ?? + (readers.some((tool) => selectedTools.includes(tool)) ? "indirect" : undefined); if (skillFileReadTool && skills.length > 0) { const skillsPrompt = formatSkillsForPrompt(skills, skillFileReadTool).trim(); if (skillsPrompt) promptSections.skills = skillsPrompt; diff --git a/packages/coding-agent/src/extensions/codemode/execute.ts b/packages/coding-agent/src/extensions/codemode/execute.ts index 69af3758f..630f831eb 100644 --- a/packages/coding-agent/src/extensions/codemode/execute.ts +++ b/packages/coding-agent/src/extensions/codemode/execute.ts @@ -383,7 +383,10 @@ export async function executeCodemode( const callable = ctx ? getCodemodeCallableTools(ctx.tools) : []; // ALL_TOOLS entries carry the declaration. - const samples = new Map(callable.map((tool) => [tool.name, renderToolSample(toCodemodeDeclaration(tool))])); + const guidelines = options.getToolGuidelines?.(); + const samples = new Map( + callable.map((tool) => [tool.name, renderToolSample(toCodemodeDeclaration(tool, guidelines?.get(tool.name)))]), + ); const sandboxTools: CodemodeTool[] = callable.map((tool) => ({ name: tool.name, description: samples.get(tool.name), diff --git a/packages/coding-agent/src/extensions/codemode/index.ts b/packages/coding-agent/src/extensions/codemode/index.ts index 4b58b1b60..b88352001 100644 --- a/packages/coding-agent/src/extensions/codemode/index.ts +++ b/packages/coding-agent/src/extensions/codemode/index.ts @@ -35,6 +35,8 @@ export function createCodemodeExtension(options: CodemodeExtensionOptions = {}): appendEntry: (customType, data) => pi.appendEntry(customType, data), models: options.models ?? true, getToolNamespace: (name) => pi.getAllTools().find((tool) => tool.name === name)?.namespace, + getToolGuidelines: () => + new Map(pi.getAllTools().map((tool) => [tool.name, tool.promptGuidelines ?? []] as const)), getMode: () => options.mode ?? readMode(pi), getInlineBudget: () => options.inlineBudget ?? readInlineBudget(pi), }), diff --git a/packages/coding-agent/src/extensions/codemode/tool.ts b/packages/coding-agent/src/extensions/codemode/tool.ts index cab14b66a..2b6cef943 100644 --- a/packages/coding-agent/src/extensions/codemode/tool.ts +++ b/packages/coding-agent/src/extensions/codemode/tool.ts @@ -66,6 +66,8 @@ export type CodemodeModelRuntime = Pick< export interface CodemodeToolOptions { /** Namespace of a tool, for `searchTools()` ranking and its `namespace` filter. */ getToolNamespace?: (toolName: string) => ToolNamespace | undefined; + /** Prompt guidelines of every tool, by tool name, shown with declarations by `describeTool()` and `ALL_TOOLS`. */ + getToolGuidelines?: () => ReadonlyMap; /** * Expose the `models` namespace to scripts, backed by the session's model registry * (`ctx.modelRegistry`). Without it, `models` is not declared. @@ -155,11 +157,18 @@ export const DEFAULT_CODEMODE_INLINE_BUDGET = 3000; /** Characters per token when estimating the cost of a tool section. */ const CHARS_PER_TOKEN = 4; -/** What a script sees of a tool. Tools without an output schema resolve to their text output. */ -export function toCodemodeDeclaration(tool: AgentTool): Omit { +/** + * What a script sees of a tool: its description followed by its prompt guidelines, which the system + * prompt only has for declared tools. Tools without an output schema resolve to their text output. + */ +export function toCodemodeDeclaration( + tool: AgentTool, + guidelines: readonly string[] = [], +): Omit { + const bullets = guidelines.flatMap((guideline) => (guideline.trim() ? [`- ${guideline.trim()}`] : [])); return { name: tool.name, - description: tool.description, + description: bullets.length > 0 ? `${tool.description.trim()}\n\n${bullets.join("\n")}` : tool.description, inputSchema: tool.parameters as CodemodeJsonSchema, outputSchema: (tool.outputSchema as CodemodeJsonSchema | undefined) ?? TEXT_OUTPUT_SCHEMA, }; @@ -177,6 +186,8 @@ export interface CodemodeDescriptionOptions { namespaces?: ReadonlyMap; /** Tools that are callable but never listed with their declaration (`deferred` exposure). */ deferred?: ReadonlySet; + /** Prompt guidelines of each tool, by tool name, listed after its description. */ + guidelines?: ReadonlyMap; /** * Estimated tokens (characters / 4) the tool sections may use. Tools that do not fit are left * out, like deferred tools. Unset lists every tool that is not deferred. @@ -240,7 +251,7 @@ export function createCodemodeDescription( ): string { const declarations = getCodemodeCallableTools(tools) .filter((tool) => !options.deferred?.has(tool.name)) - .map(toCodemodeDeclaration); + .map((tool) => toCodemodeDeclaration(tool, options.guidelines?.get(tool.name))); const groups = new Map([["", { namespace: undefined, entries: [] }]]); for (const declaration of declarations) { const namespace = options.namespaces?.get(declaration.name); @@ -323,6 +334,8 @@ function describeScriptCall(tool: AgentTool): string { * - `only`: the codemode description lists every callable tool, and requests leave out the * declarations of active `direct` tools. * + * Listed tools carry their prompt guidelines, which the system prompt only has for declared tools. + * * Listing by exposure, not by the active set, keeps the codemode description unchanged when * `tool_search` loads a tool, so loads do not redeclare codemode. */ @@ -344,9 +357,11 @@ function prepareCodemodeLoadout(loadout: ToolLoadout, options: CodemodeToolOptio return namespace ? [[tool.name, namespace] as const] : []; }), ); + const guidelines = new Map(listed.map((tool) => [tool.name, loadout.getPromptGuidelines(tool.name)] as const)); descriptions[CODEMODE_TOOL_NAME] = createCodemodeDescription(listed, { models: options.models === true, namespaces, + guidelines, deferred: new Set( listed.filter((tool) => loadout.getExposure(tool.name) === "deferred").map((tool) => tool.name), ), diff --git a/packages/coding-agent/test/suite/agent-session-codemode.test.ts b/packages/coding-agent/test/suite/agent-session-codemode.test.ts index 58054a493..5da2d884e 100644 --- a/packages/coding-agent/test/suite/agent-session-codemode.test.ts +++ b/packages/coding-agent/test/suite/agent-session-codemode.test.ts @@ -175,12 +175,34 @@ describe("AgentSession codemode tool", () => { expect(requestPrompts[1]).not.toContain("\n- read: "); expect(requestPrompts[1]).toContain("\n- codemode: "); expect(harness.session.systemPrompt).not.toContain("\n- read: "); + // Hidden tools' guidelines move from the rules to their codemode sections (#10343). + expect(requestPrompts[1]).not.toContain("Use read to examine files"); + expect(description("codemode")).toContain("- Use read to examine files instead of cat or sed."); // Without codemode, tools keep their plain descriptions. harness.session.setActiveToolsByName(["echo"]); expect(description("echo")).toBe("Echo text back.\n\nSecond paragraph."); }); + // #10343 + it("shows the guidelines of tools that do not fit the inline budget through describeTool()", async () => { + const harness = await setup(); + harness.settingsManager.applyOverrides({ codemode: { mode: "only", inlineBudget: 0 } }); + harness.session.setActiveToolsByName(["read", "codemode"]); + const codemode = harness.session.agent.state.tools.find((tool) => tool.name === "codemode"); + expect(codemode?.description).not.toContain("### `read`"); + harness.setResponses([ + fauxAssistantMessage([fauxToolCall("codemode", { code: 'text(await describeTool("read"))' })], { + stopReason: "toolUse", + }), + fauxAssistantMessage("done"), + ]); + + await harness.session.prompt("go"); + + expect(resultText(codemodeResult(harness))).toContain("- Use read to examine files instead of cat or sed."); + }); + it("runs nested calls in parallel and returns only the script result", async () => { const harness = await setup(); harness.setResponses([ diff --git a/packages/coding-agent/test/system-prompt.test.ts b/packages/coding-agent/test/system-prompt.test.ts index e24175fc5..276cc63ff 100644 --- a/packages/coding-agent/test/system-prompt.test.ts +++ b/packages/coding-agent/test/system-prompt.test.ts @@ -205,4 +205,34 @@ describe("buildSystemPrompt", () => { expect(prompt).not.toContain(""); }); }); + + // #10343 + describe("hidden tools", () => { + const build = (hiddenTools: string[]) => + buildSystemPrompt({ + selectedTools: ["read", "bash", "run"], + hiddenTools, + toolSnippets: { read: "Read files", bash: "Run commands", run: "Run a task" }, + toolGuidelines: { read: ["Use read for files."], run: ["Prefer run."] }, + contextFiles: [], + skills: [testSkill], + cwd: process.cwd(), + }); + + test("leaves hidden tools out of the tool list and rules", () => { + const prompt = build(["read", "bash"]); + + expect(prompt).toContain("\n- run: Run a task\n"); + expect(prompt).not.toContain("- read: "); + expect(prompt).not.toContain("Use read for files."); + expect(prompt).not.toContain("Use bash for file operations"); + expect(prompt).toContain("- Prefer run."); + }); + + test("keeps skills without naming a hidden reader", () => { + expect(build(["read", "bash"])).toContain("\nLoad a skill's file when the task matches its description."); + expect(build(["read"])).toContain("Use bash to load a skill's file"); + expect(build([])).toContain("Use the read tool to load a skill's file"); + }); + }); });