From 3c5e01199c8e61afdf5f752a226ed03d27edd884 Mon Sep 17 00:00:00 2001 From: Bulat Yapparov Date: Wed, 21 Oct 2026 01:22:53 +0100 Subject: [PATCH 1/6] fix: align built-in request guidance with declared tools --- packages/cli/src/session/llm.ts | 23 +- packages/cli/src/session/prompt.ts | 4 +- packages/cli/src/session/system.ts | 49 +- packages/cli/src/tool/bash.ts | 7 +- packages/cli/src/tool/edit.ts | 2 + packages/cli/src/tool/glob.ts | 2 + packages/cli/src/tool/grep.ts | 2 + packages/cli/src/tool/guidance.ts | 122 +++++ packages/cli/src/tool/read.ts | 2 + packages/cli/src/tool/registry.ts | 14 +- packages/cli/src/tool/tool.ts | 2 + packages/cli/src/tool/write.ts | 2 + .../session/headless-prompt-budget.test.ts | 25 +- packages/cli/test/session/llm.test.ts | 6 +- .../cli/test/session/tool-guidance.test.ts | 501 ++++++++++++++++++ 15 files changed, 728 insertions(+), 35 deletions(-) create mode 100644 packages/cli/src/tool/guidance.ts create mode 100644 packages/cli/test/session/tool-guidance.test.ts diff --git a/packages/cli/src/session/llm.ts b/packages/cli/src/session/llm.ts index ce207999..109cc84b 100644 --- a/packages/cli/src/session/llm.ts +++ b/packages/cli/src/session/llm.ts @@ -23,6 +23,7 @@ import { SystemPrompt } from "./system" import { PermissionNext } from "@/permission/next" import { Auth } from "@/auth" import { ProviderTermination } from "@/provider/termination" +import { ToolGuidance } from "@/tool/guidance" export namespace LLM { const log = Log.create({ service: "llm" }) @@ -67,11 +68,14 @@ export namespace LLM { Auth.get(input.model.providerID), ]) const isCodex = provider.id === "openai" && auth?.type === "oauth" + const selected = await resolveTools(input) + const names = ToolGuidance.capabilities(selected) + const tools = ToolGuidance.render(selected, names) const system = [] system.push( [ - ...leadPrompt(input.agent, isCodex), + ...leadPrompt(input.agent, isCodex, names), // any custom prompt passed into this call ...input.system, // any custom prompt from last user message @@ -110,7 +114,7 @@ export namespace LLM { mergeDeep(variant), ) if (isCodex) { - options.instructions = SystemPrompt.instructions() + options.instructions = SystemPrompt.instructions(names) } const params = await Plugin.trigger( @@ -148,8 +152,6 @@ export namespace LLM { const maxOutputTokens = isCodex ? undefined : ProviderTransform.maxOutputTokens(input.model) - const tools = await resolveTools(input) - // LiteLLM and some Anthropic proxies require the tools parameter to be present // when message history contains tool calls, even if no tools are being used. // Add a dummy tool that is never called to satisfy this validation. @@ -269,22 +271,23 @@ export namespace LLM { * The prompt that opens the system message: the agent's own prompt, else the shared base prompt. * Codex sign-in sessions send no base prompt here, because they send the Codex header as `options.instructions`. */ - function leadPrompt(agent: Agent.Info, isCodex: boolean) { + function leadPrompt(agent: Agent.Info, isCodex: boolean, tools: ReadonlySet) { if (agent.prompt) return [agent.prompt] if (isCodex) return [] - return [SystemPrompt.base()] + return [SystemPrompt.base(tools)] } async function resolveTools(input: Pick) { // same ruleset tool calls are checked against, so a denied tool is never offered to the model const ruleset = PermissionNext.merge(input.agent.permission, input.permission ?? []) - const disabled = PermissionNext.disabled(Object.keys(input.tools), ruleset) - for (const tool of Object.keys(input.tools)) { + const tools = { ...input.tools } + const disabled = PermissionNext.disabled(Object.keys(tools), ruleset) + for (const tool of Object.keys(tools)) { if (input.user.tools?.[tool] === false || disabled.has(tool)) { - delete input.tools[tool] + delete tools[tool] } } - return input.tools + return tools } // Check if messages contain any tool-call content diff --git a/packages/cli/src/session/prompt.ts b/packages/cli/src/session/prompt.ts index 9e4727a0..b9b64660 100644 --- a/packages/cli/src/session/prompt.ts +++ b/packages/cli/src/session/prompt.ts @@ -21,6 +21,7 @@ import { Plugin } from "../plugin" import MAX_STEPS from "../session/prompt/max-steps.txt" import { defer } from "../util/defer" import { ToolRegistry } from "../tool/registry" +import { ToolGuidance } from "../tool/guidance" import { MCP } from "../mcp" import { ReadTool } from "../tool/read" import { FileTime } from "../file/time" @@ -941,7 +942,7 @@ export namespace SessionPrompt { input.session.id, )) { const schema = ProviderTransform.schema(input.model, z.toJSONSchema(item.parameters)) - tools[item.id] = tool({ + const definition = tool({ id: item.id as any, description: item.description, inputSchema: jsonSchema(schema as any), @@ -981,6 +982,7 @@ export namespace SessionPrompt { return output }, }) + tools[item.id] = ToolGuidance.bind(definition, item.builtin, item.renderDescription) } for (const [key, item] of Object.entries(await MCP.tools())) { diff --git a/packages/cli/src/session/system.ts b/packages/cli/src/session/system.ts index 42f18228..6d8b2c27 100644 --- a/packages/cli/src/session/system.ts +++ b/packages/cli/src/session/system.ts @@ -3,15 +3,56 @@ import { Instance } from "../project/instance" import PROMPT_DEFAULT from "./prompt/default.txt" import PROMPT_CODEX from "./prompt/codex_header.txt" import type { Provider } from "@/provider/provider" +import { ToolGuidance } from "../tool/guidance" export namespace SystemPrompt { - export function instructions() { - return PROMPT_CODEX.trim() + export function instructions(tools?: ReadonlySet) { + const patch = + "- Try to use apply_patch for single file edits, but it is fine to explore other options to make the edit if it does not work well. Do not use apply_patch for changes that are auto-generated (i.e. generating package.json or running a lint or format command like gofmt) or when scripting is more efficient (such as search and replacing a string across a codebase).\n" + const files = " - Use Read to view files, Edit to modify files, and Write only when needed.\n" + const search = " - Use Glob to find files by name and Grep to search file contents.\n" + const bash = "- Use Bash for terminal operations (git, bun, builds, tests, running scripts).\n" + const specialized = "- Prefer specialized tools over shell for file operations:\n" + const available = (name: string) => tools?.has(name) ?? true + const operations = [ + ...(available("read") ? ["Read to view files"] : []), + ...(available("edit") ? ["Edit to modify files"] : []), + ...(available("write") ? ["Write only when needed"] : []), + ] + const searches = [ + ...(available("glob") ? ["Glob to find files by name"] : []), + ...(available("grep") ? ["Grep to search file contents"] : []), + ] + const prompt = ToolGuidance.replace(PROMPT_CODEX.trim(), patch, available("apply_patch") ? patch : "") + const editing = ToolGuidance.replace( + prompt, + files, + operations.length === 3 ? files : operations.length ? ` - Use ${operations.join("; ")}.\n` : "", + ) + const searching = ToolGuidance.replace( + editing, + search, + searches.length === 2 ? search : searches.length ? ` - Use ${searches[0]}.\n` : "", + ) + return ToolGuidance.replace( + ToolGuidance.replace(searching, bash, available("bash") ? bash : ""), + specialized, + operations.length || searches.length ? specialized : "", + ) } /** The base system prompt, shared by every provider and model family. */ - export function base() { - return PROMPT_DEFAULT.trim() + export function base(tools?: ReadonlySet) { + const task = "- Use the Task tool when a search is broad and open-ended, or when asked to delegate.\n" + const bash = + "- When you have finished, run the lint and typecheck commands (for example npm run lint, npm run typecheck, ruff) with bash if they were provided to you.\n" + return ToolGuidance.replace( + ToolGuidance.replace(PROMPT_DEFAULT.trim(), task, tools && !tools.has("task") ? "" : task), + bash, + tools && !tools.has("bash") + ? "- When you have finished, verify your changes using the available capabilities. Report any provided lint or typecheck commands you could not run.\n" + : bash, + ) } export async function environment(model: Provider.Model) { diff --git a/packages/cli/src/tool/bash.ts b/packages/cli/src/tool/bash.ts index 79202e6d..3005f38e 100644 --- a/packages/cli/src/tool/bash.ts +++ b/packages/cli/src/tool/bash.ts @@ -1,6 +1,7 @@ import z from "zod" import { spawn } from "child_process" import { Tool } from "./tool" +import { ToolGuidance } from "./guidance" import path from "path" import DESCRIPTION from "./bash.txt" import { Log } from "../util/log" @@ -57,7 +58,10 @@ const SHELL_INTERPRETERS = new Set(["bash", "sh", "zsh", "dash", "ksh", "fish"]) const TOKEN_TYPES = new Set(["command_name", "word", "string", "raw_string", "concatenation"]) /** Extract token text from a command node, filtering to known token types. */ -function getCommandTokens(node: { childCount: number; child(i: number): { type: string; text: string } | null }): string[] { +function getCommandTokens(node: { + childCount: number + child(i: number): { type: string; text: string } | null +}): string[] { const tokens: string[] = [] for (let i = 0; i < node.childCount; i++) { const child = node.child(i) @@ -94,6 +98,7 @@ export const BashTool = Tool.define("bash", async () => { log.info("bash tool using shell", { shell }) return { + renderDescription: ToolGuidance.bash, // Replacer functions insert values literally, without `$` pattern expansion description: DESCRIPTION.replaceAll("${directory}", () => Instance.directory) .replaceAll("${maxLines}", () => String(Truncate.MAX_LINES)) diff --git a/packages/cli/src/tool/edit.ts b/packages/cli/src/tool/edit.ts index 0e47a830..669ea65e 100644 --- a/packages/cli/src/tool/edit.ts +++ b/packages/cli/src/tool/edit.ts @@ -6,6 +6,7 @@ import z from "zod" import * as path from "path" import { Tool } from "./tool" +import { ToolGuidance } from "./guidance" import { createTwoFilesPatch, diffLines } from "diff" import DESCRIPTION from "./edit.txt" import { File } from "../file" @@ -21,6 +22,7 @@ function normalizeLineEndings(text: string): string { export const EditTool = Tool.define("edit", { description: DESCRIPTION, + renderDescription: ToolGuidance.edit, parameters: z.object({ filePath: z.string().describe("The absolute path to the file to modify"), oldString: z.string().describe("The text to replace"), diff --git a/packages/cli/src/tool/glob.ts b/packages/cli/src/tool/glob.ts index a2611246..3ba2355c 100644 --- a/packages/cli/src/tool/glob.ts +++ b/packages/cli/src/tool/glob.ts @@ -1,6 +1,7 @@ import z from "zod" import path from "path" import { Tool } from "./tool" +import { ToolGuidance } from "./guidance" import { Filesystem } from "../util/filesystem" import DESCRIPTION from "./glob.txt" import { Ripgrep } from "../file/ripgrep" @@ -9,6 +10,7 @@ import { assertExternalDirectory } from "./external-directory" export const GlobTool = Tool.define("glob", { description: DESCRIPTION, + renderDescription: ToolGuidance.glob, parameters: z.object({ pattern: z.string().describe("The glob pattern to match files against"), path: z diff --git a/packages/cli/src/tool/grep.ts b/packages/cli/src/tool/grep.ts index 82e7ac16..511cc2d8 100644 --- a/packages/cli/src/tool/grep.ts +++ b/packages/cli/src/tool/grep.ts @@ -1,6 +1,7 @@ import z from "zod" import { text } from "node:stream/consumers" import { Tool } from "./tool" +import { ToolGuidance } from "./guidance" import { Filesystem } from "../util/filesystem" import { Ripgrep } from "../file/ripgrep" import { Process } from "../util/process" @@ -14,6 +15,7 @@ const MAX_LINE_LENGTH = 2000 export const GrepTool = Tool.define("grep", { description: DESCRIPTION, + renderDescription: ToolGuidance.grep, parameters: z.object({ pattern: z.string().describe("The regex pattern to search for in file contents"), path: z.string().optional().describe("The directory to search in. Defaults to the current working directory."), diff --git a/packages/cli/src/tool/guidance.ts b/packages/cli/src/tool/guidance.ts new file mode 100644 index 00000000..56bece3e --- /dev/null +++ b/packages/cli/src/tool/guidance.ts @@ -0,0 +1,122 @@ +import type { Tool } from "ai" + +export namespace ToolGuidance { + const builtin = Symbol("builtin") + const renderer = Symbol("description.renderer") + + export type Renderer = (description: string, tools: ReadonlySet) => string + type Owned = Tool & { [builtin]?: string; [renderer]?: Renderer } + + export function bind(tool: Tool, id?: string, render?: Renderer): Tool { + if (!id) return tool + return { ...tool, [builtin]: id, [renderer]: render } as Owned + } + + export function capabilities(tools: Record) { + return new Set( + Object.entries(tools).flatMap(([id, tool]) => (id !== "invalid" && (tool as Owned)[builtin] === id ? [id] : [])), + ) + } + + export function render(tools: Record, names: ReadonlySet): Record { + return Object.fromEntries( + Object.entries(tools).map(([id, tool]) => { + const render = (tool as Owned)[renderer] + if (!render) return [id, tool] + if (tool.description === undefined) throw new Error(`Built-in tool ${id} has no description`) + return [id, { ...tool, description: render(tool.description, names) }] + }), + ) + } + + export function replace(description: string, fragment: string, replacement: string) { + const index = description.indexOf(fragment) + if (index === -1 || index !== description.lastIndexOf(fragment)) { + throw new Error("Harness guidance fragment is missing or duplicated") + } + return description.replace(fragment, () => replacement) + } + + export function bash(description: string, tools: ReadonlySet) { + const files = [ + ["glob", "Glob (not find/ls)"], + ["grep", "Grep (not grep/rg)"], + ["read", "Read (not cat/head/tail)"], + ["edit", "Edit (not sed/awk/perl)"], + ["write", "Write (not echo/heredoc redirection)"], + ] + const selected = files.filter(([id]) => tools.has(id)).map(([, text]) => text) + const dedicated = `Do not use it for file operations; use the dedicated tools instead: ${files.map(([, text]) => text).join(", ")}.\n\n` + const output = + "; read it with Read (offset/limit) or search it with Grep. So do not limit output with `head`, `tail` or other truncation commands; run the command directly." + const readers = [ + ...(tools.has("read") ? ["read it with Read (offset/limit)"] : []), + ...(tools.has("grep") ? ["search it with Grep"] : []), + ] + return replace( + replace( + description, + dedicated, + selected.length === files.length + ? dedicated + : selected.length + ? `Use the available dedicated tools for their supported file operations: ${selected.join(", ")}.\n\n` + : "", + ), + output, + readers.length === 2 + ? output + : readers.length + ? `; ${readers.join(" or ")}. So do not limit output with \`head\`, \`tail\` or other truncation commands; run the command directly.` + : ".", + ) + } + + export function read(description: string, tools: ReadonlySet) { + const grep = "- Use the grep tool to find specific content in large files or files with long lines.\n" + const glob = + "- If you are unsure of the correct file path, use the glob tool to look up filenames by glob pattern.\n" + return replace(replace(description, grep, tools.has("grep") ? grep : ""), glob, tools.has("glob") ? glob : "") + } + + export function glob(description: string, tools: ReadonlySet) { + const task = + "- When you are doing an open-ended search that may require multiple rounds of globbing and grepping, use the Task tool instead\n" + return replace(description, task, tools.has("task") ? task : "") + } + + export function grep(description: string, tools: ReadonlySet) { + const bash = + "- If you need to identify/count the number of matches within files, use the Bash tool with `rg` (ripgrep) directly. Do NOT use `grep`.\n" + return glob(replace(description, bash, tools.has("bash") ? bash : ""), tools) + } + + export function edit(description: string, tools: ReadonlySet) { + const read = + "- You must use your `Read` tool at least once in the conversation before editing. This tool will error if you attempt an edit without reading the file. \n" + const output = "When editing text from Read tool output" + return replace( + replace( + description, + read, + tools.has("read") + ? read + : "- For a nonempty `oldString`, the file's current contents must already be recorded as read in this session. This tool fails if that record is missing or stale. An empty `oldString` uses the direct-write path without this prerequisite.\n", + ), + output, + tools.has("read") ? output : "When editing text from numbered file output", + ) + } + + export function write(description: string, tools: ReadonlySet) { + const read = + "- If this is an existing file, you MUST use the Read tool first to read the file's contents. This tool will fail if you did not read the file first.\n" + return replace( + description, + read, + tools.has("read") + ? read + : "- Replacing an existing file requires its current contents to already be recorded as read in this session. This tool fails if that record is missing or stale; creating a new file does not need that record.\n", + ) + } +} diff --git a/packages/cli/src/tool/read.ts b/packages/cli/src/tool/read.ts index 74831bb1..0c1c2f7e 100644 --- a/packages/cli/src/tool/read.ts +++ b/packages/cli/src/tool/read.ts @@ -4,6 +4,7 @@ import * as fs from "fs/promises" import * as path from "path" import { createInterface } from "readline" import { Tool } from "./tool" +import { ToolGuidance } from "./guidance" import { FileTime } from "../file/time" import DESCRIPTION from "./read.txt" import { Instance } from "../project/instance" @@ -40,6 +41,7 @@ async function emitSkillResourceEvent(sessionID: string, filepath: string) { export const ReadTool = Tool.define("read", { description: DESCRIPTION, + renderDescription: ToolGuidance.read, parameters: z.object({ filePath: z.string().describe("The absolute path to the file or directory to read"), offset: z.coerce.number().describe("The line number to start reading from (1-indexed)").optional(), diff --git a/packages/cli/src/tool/registry.ts b/packages/cli/src/tool/registry.ts index 6e4831ef..7a6f71b6 100644 --- a/packages/cli/src/tool/registry.ts +++ b/packages/cli/src/tool/registry.ts @@ -107,11 +107,11 @@ export namespace ToolRegistry { custom.push(tool) } - async function all(): Promise { + async function all(): Promise<{ info: Tool.Info; builtin: boolean }[]> { const custom = await state().then((x) => x.custom) const config = await Config.get() - return [ + const builtin = [ InvalidTool, BashTool, ReadTool, @@ -126,12 +126,12 @@ export namespace ToolRegistry { SkillTool, ApplyPatchTool, ...(config.experimental?.batch_tool === true ? [BatchTool] : []), - ...custom, ] + return [...builtin.map((info) => ({ info, builtin: true })), ...custom.map((info) => ({ info, builtin: false }))] } export async function ids() { - return all().then((x) => x.map((t) => t.id)) + return all().then((x) => x.map((t) => t.info.id)) } export async function tools( @@ -145,7 +145,7 @@ export namespace ToolRegistry { const tools = await all() const result = await Promise.all( tools - .filter((t) => { + .filter(({ info: t }) => { if (t.id === "codesearch" || t.id === "websearch") return Flag.AICTRL_ENABLE_EXA // use apply tool in same format as codex @@ -156,7 +156,7 @@ export namespace ToolRegistry { return true }) - .map(async (t) => { + .map(async ({ info: t, builtin }) => { using _ = log.time(t.id) const tool = await t.init({ agent, sessionID }) const output = { @@ -169,6 +169,8 @@ export namespace ToolRegistry { ...tool, description: output.description, parameters: output.parameters, + builtin: builtin ? t.id : undefined, + renderDescription: builtin && output.description === tool.description ? tool.renderDescription : undefined, } }), ) diff --git a/packages/cli/src/tool/tool.ts b/packages/cli/src/tool/tool.ts index 4405dc66..e76c1ca5 100644 --- a/packages/cli/src/tool/tool.ts +++ b/packages/cli/src/tool/tool.ts @@ -3,6 +3,7 @@ import type { MessageV2 } from "../session/message-v2" import type { Agent } from "../agent/agent" import type { PermissionNext } from "../permission/next" import { Truncate } from "./truncation" +import type { ToolGuidance } from "./guidance" export namespace Tool { interface Metadata { @@ -29,6 +30,7 @@ export namespace Tool { id: string init: (ctx?: InitContext) => Promise<{ description: string + renderDescription?: ToolGuidance.Renderer parameters: Parameters execute( args: z.infer, diff --git a/packages/cli/src/tool/write.ts b/packages/cli/src/tool/write.ts index 19891277..9e99e1bc 100644 --- a/packages/cli/src/tool/write.ts +++ b/packages/cli/src/tool/write.ts @@ -1,6 +1,7 @@ import z from "zod" import * as path from "path" import { Tool } from "./tool" +import { ToolGuidance } from "./guidance" import { createTwoFilesPatch } from "diff" import DESCRIPTION from "./write.txt" import { Bus } from "../bus" @@ -13,6 +14,7 @@ import { assertExternalDirectory } from "./external-directory" export const WriteTool = Tool.define("write", { description: DESCRIPTION, + renderDescription: ToolGuidance.write, parameters: z.object({ content: z.string().describe("The content to write to the file"), filePath: z.string().describe("The absolute path to the file to write (must be absolute, not relative)"), diff --git a/packages/cli/test/session/headless-prompt-budget.test.ts b/packages/cli/test/session/headless-prompt-budget.test.ts index cfe1df43..db2702b0 100644 --- a/packages/cli/test/session/headless-prompt-budget.test.ts +++ b/packages/cli/test/session/headless-prompt-budget.test.ts @@ -23,28 +23,32 @@ const HEADLESS_PREFIX_BUDGET = 15_000 type Piece = { name: string; chars: number } /** System prompt pieces, assembled the way SessionPrompt and LLM.stream assemble them. */ -async function systemPieces(model: Provider.Model): Promise { +async function systemPieces(model: Provider.Model, capabilities: ReadonlySet): Promise { const instructions = await InstructionPrompt.system() return [ - { name: "system: base prompt", chars: SystemPrompt.base().length }, + { name: "system: base prompt", chars: SystemPrompt.base(capabilities).length }, { name: "system: environment", chars: (await SystemPrompt.environment(model)).join("\n").length }, { name: "system: instructions", chars: instructions.join("\n").length }, ] } /** Tool definitions the model receives: the registry list minus what LLM.stream drops. */ -async function toolPieces(model: Provider.Model, agent: Agent.Info): Promise { +async function toolPieces(model: Provider.Model, agent: Agent.Info) { const tools = await ToolRegistry.tools({ providerID: model.providerID, modelID: model.api.id }, agent) const disabled = PermissionNext.disabled( tools.map((tool) => tool.id), agent.permission, ) - return tools - .filter((tool) => tool.id !== "invalid" && !disabled.has(tool.id)) - .map((tool) => ({ - name: `tool: ${tool.id}`, - chars: tool.id.length + tool.description.length + JSON.stringify(z.toJSONSchema(tool.parameters)).length, - })) + const selected = tools.filter((tool) => tool.id !== "invalid" && !disabled.has(tool.id)) + const capabilities = new Set(selected.flatMap((tool) => (tool.builtin === tool.id ? [tool.id] : []))) + const pieces = selected.map((tool) => ({ + name: `tool: ${tool.id}`, + chars: + tool.id.length + + (tool.renderDescription?.(tool.description, capabilities) ?? tool.description).length + + JSON.stringify(z.toJSONSchema(tool.parameters)).length, + })) + return { capabilities, pieces } } function breakdown(pieces: Piece[]) { @@ -75,7 +79,8 @@ describe("headless request prefix budget (#160)", () => { const model = await Provider.getModel("alibaba", "qwen-plus") const agent = await Agent.get("build") - const pieces = [...(await systemPieces(model)), ...(await toolPieces(model, agent))] + const rendered = await toolPieces(model, agent) + const pieces = [...(await systemPieces(model, rendered.capabilities)), ...rendered.pieces] const total = pieces.reduce((sum, piece) => sum + piece.chars, 0) const report = [ diff --git a/packages/cli/test/session/llm.test.ts b/packages/cli/test/session/llm.test.ts index 25080ce1..3efe84eb 100644 --- a/packages/cli/test/session/llm.test.ts +++ b/packages/cli/test/session/llm.test.ts @@ -11,7 +11,7 @@ import type { Agent } from "../../src/agent/agent" import type { PermissionNext } from "../../src/permission/next" import type { MessageV2 } from "../../src/session/message-v2" import { alibabaProvider } from "../fixture/providers" -import PROMPT_DEFAULT from "../../src/session/prompt/default.txt" +import { SystemPrompt } from "../../src/session/system" describe("session.llm.hasToolCalls", () => { test("returns false for empty messages array", () => { @@ -245,9 +245,9 @@ function createEventResponse(chunks: unknown[], includeDone = false) { }) } -/** Whether a request body carries the shared default system prompt, whatever the provider's payload shape. */ +/** Whether a request body carries the default prompt rendered for these empty/custom-only tool fixtures, whatever the provider's payload shape. */ function sendsBasePrompt(body: Record) { - return JSON.stringify(body).includes(JSON.stringify(PROMPT_DEFAULT.trim()).slice(1, -1)) + return JSON.stringify(body).includes(JSON.stringify(SystemPrompt.base(new Set())).slice(1, -1)) } describe("session.llm.stream", () => { diff --git a/packages/cli/test/session/tool-guidance.test.ts b/packages/cli/test/session/tool-guidance.test.ts new file mode 100644 index 00000000..cb845a2d --- /dev/null +++ b/packages/cli/test/session/tool-guidance.test.ts @@ -0,0 +1,501 @@ +import { afterAll, beforeAll, describe, expect, spyOn, test } from "bun:test" +import path from "node:path" + +// Install before runtime imports: every model request is local or an intercepted +// Codex request carrying inert auth. Never fall through to an external origin. +const originalFetch = globalThis.fetch +const transport = { origin: "", codex: false, captures: [] as Wire[] } +type WireTool = { name?: string; description?: string; parameters?: unknown; function?: WireTool } +type Wire = { + messages?: { role: string; content: unknown }[] + input?: unknown + instructions?: string + tools?: WireTool[] +} +function chat() { + const chunks = [ + { + id: "fixture", + object: "chat.completion.chunk", + choices: [{ index: 0, delta: { role: "assistant", content: "done" } }], + }, + { id: "fixture", object: "chat.completion.chunk", choices: [{ index: 0, delta: {}, finish_reason: "stop" }] }, + ] + return sse(chunks) +} +function responses() { + return sse([ + { type: "response.created", response: { id: "resp-fixture", created_at: 1, model: "gpt-5.2", service_tier: null } }, + { type: "response.output_item.added", output_index: 0, item: { type: "message", id: "item-fixture" } }, + { type: "response.output_text.delta", item_id: "item-fixture", delta: "done", logprobs: null }, + { type: "response.output_item.done", output_index: 0, item: { type: "message", id: "item-fixture" } }, + { + type: "response.completed", + response: { + incomplete_details: null, + usage: { input_tokens: 1, input_tokens_details: null, output_tokens: 1, output_tokens_details: null }, + service_tier: null, + }, + }, + ]) +} +function sse(chunks: unknown[]) { + return new Response( + chunks + .map((chunk) => `data: ${JSON.stringify(chunk)}`) + .concat("data: [DONE]") + .join("\n\n") + "\n\n", + { + headers: { "content-type": "text/event-stream" }, + }, + ) +} +globalThis.fetch = (async (input, init) => { + const url = new URL(input instanceof Request ? input.url : String(input)) + if (transport.codex && url.origin === "https://chatgpt.com" && url.pathname.endsWith("/responses")) { + transport.captures.push(JSON.parse(String(init?.body)) as Wire) + return responses() + } + if (url.origin !== transport.origin) throw new Error(`Blocked unexpected model transport: ${url.origin}`) + return originalFetch(input, init) +}) as typeof fetch + +const [ + { Tool }, + { default: z }, + { tool, jsonSchema }, + { Instance }, + { Session }, + { SessionPrompt }, + { LLM }, + { ToolRegistry }, + { Plugin }, + { MCP }, + { Auth }, + { HeadlessRun }, + { tmpdir }, +] = await Promise.all([ + import("../../src/tool/tool"), + import("zod"), + import("ai"), + import("../../src/project/instance"), + import("../../src/session"), + import("../../src/session/prompt"), + import("../../src/session/llm"), + import("../../src/tool/registry"), + import("../../src/plugin"), + import("../../src/mcp"), + import("../../src/auth"), + import("../../src/flag/headless-run"), + import("../fixture/fixture"), +]) + +type StreamInput = Parameters[0] +type PromptInput = Parameters[0] +type Fixture = { + prompt(input?: Partial): Promise + direct(input: StreamInput): Promise + input(): StreamInput +} + +function definitions(body: Wire) { + return (body.tools ?? []).map((item) => item.function ?? item) +} +function names(body: Wire) { + return definitions(body) + .map((item) => item.name) + .sort() +} +function description(body: Wire, name: string) { + const result = definitions(body).find((item) => item.name === name) + if (!result) throw new Error(`Expected declared tool ${name}`) + return result.description ?? "" +} +function system(body: Wire) { + return (body.messages ?? []) + .filter((item) => item.role === "system") + .map((item) => (typeof item.content === "string" ? item.content : JSON.stringify(item.content))) + .join("\n") +} +const full = ["bash", "edit", "glob", "grep", "read", "skill", "task", "webfetch", "write"] +const reduced = ["bash", "edit", "read", "write"] +const off = ["glob", "grep", "task", "webfetch", "skill"] +const denied = Object.fromEntries(off.map((name) => [name, "deny" as const])) + +async function fixture( + label: string, + options: { + permission?: Record + session?: StreamInput["permission"] + gpt?: boolean + codex?: boolean + buildPrompt?: string + } = {}, + fn: (context: Fixture) => Promise, +) { + transport.captures = [] + transport.codex = options.codex ?? false + const providerID = options.gpt || options.codex ? "openai" : "fixture" + const modelID = providerID === "openai" ? "gpt-5.2" : "fixture" + const server = Bun.serve({ + hostname: "127.0.0.1", + port: 0, + async fetch(request) { + transport.captures.push((await request.json()) as Wire) + return providerID === "openai" ? responses() : chat() + }, + }) + transport.origin = server.url.origin + const previousAuth = options.codex ? await Auth.get("openai") : undefined + if (options.codex) + await Auth.set("openai", { + type: "oauth", + access: "fixture-access", + refresh: "fixture-refresh", + expires: Date.now() + 60_000, + }) + const original = LLM.stream + let last: StreamInput | undefined + const spy = spyOn(LLM, "stream").mockImplementation(async (input) => { + last = input + return original(input) + }) + try { + await using tmp = await tmpdir({ + config: { + permission: { "*": "allow", ...options.permission }, + enabled_providers: [providerID], + provider: { + [providerID]: { + npm: providerID === "openai" ? "@ai-sdk/openai" : "@ai-sdk/openai-compatible", + options: { apiKey: "fixture", baseURL: `${server.url.origin}/v1` }, + ...(providerID === "fixture" + ? { models: { fixture: { name: "fixture", tool_call: true, limit: { context: 200_000, output: 1000 } } } } + : {}), + }, + }, + ...(options.buildPrompt ? { agent: { build: { prompt: options.buildPrompt } } } : {}), + }, + }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const session = await Session.create({ title: "Tool guidance fixture", permission: options.session }) + const context: Fixture = { + async prompt(input = {}) { + const before = transport.captures.length + const result = await SessionPrompt.prompt({ + sessionID: session.id, + model: { providerID, modelID }, + parts: [{ type: "text", text: "Say done without calling tools." }], + ...input, + }) + if (result.info.role !== "assistant" || result.info.error) throw new Error(JSON.stringify(result.info)) + expect(transport.captures.length - before).toBe(1) + return transport.captures[before] + }, + async direct(input) { + const before = transport.captures.length + const stream = await LLM.stream(input) + for await (const _ of stream.fullStream) { + /* consume actual SDK serialization */ + } + expect(transport.captures.length - before).toBe(1) + return transport.captures[before] + }, + input() { + if (!last) throw new Error("Prompt must capture production-built tools first") + return last + }, + } + await fn(context) + }, + }) + } finally { + const output = process.env.GUIDANCE_CAPTURE_DIR + if (output) + await Bun.write( + path.join(output, `${label}.json`), + JSON.stringify({ label, codex: options.codex ?? false, requests: transport.captures }, null, 2), + ) + spy.mockRestore() + server.stop(true) + transport.origin = "" + transport.codex = false + if (options.codex) { + if (previousAuth) await Auth.set("openai", previousAuth) + else await Auth.remove("openai") + } + } +} + +beforeAll(() => HeadlessRun.enable()) +afterAll(() => { + HeadlessRun.disable() + globalThis.fetch = originalFetch +}) + +describe("actual request tool guidance (#244)", () => { + test("full built-in catalog retains useful peer guidance", async () => { + await fixture("full", {}, async (context) => { + const body = await context.prompt() + expect(names(body)).toEqual(full) + expect(system(body)).toContain("Use the Task tool") + safety(body) + const prefix = + system(body).length + + definitions(body).reduce( + (sum, item) => + sum + (item.name?.length ?? 0) + (item.description?.length ?? 0) + JSON.stringify(item.parameters).length, + 0, + ) + expect(prefix).toBeLessThanOrEqual(15_000) + expect(description(body, "bash")).toContain("Glob (not find/ls)") + expect(description(body, "bash")).toContain("Grep (not grep/rg)") + expect(description(body, "read")).toContain("Use the grep tool") + expect(description(body, "read")).toContain("use the glob tool") + }) + }) + + test("four-tool permission profile omits absent Task Glob Grep recommendations", async () => { + await fixture("four-tools", { permission: denied }, async (context) => { + const body = await context.prompt() + expect(names(body)).toEqual(reduced) + expect(system(body)).not.toContain("Use the Task tool") + expect(description(body, "bash")).not.toContain("Glob (not find/ls)") + expect(description(body, "bash")).not.toContain("Grep (not grep/rg)") + expect(description(body, "read")).not.toContain("Use the grep tool") + expect(description(body, "read")).not.toContain("use the glob tool") + }) + }) +}) + +function custom(id: string, description: string) { + return Tool.define(id, { + description, + parameters: z.object({ value: z.string().optional() }), + execute: async () => ({ title: "fixture", metadata: {}, output: "fixture" }), + }) +} + +function safety(body: Wire) { + expect(system(body)).toContain("don't retry it or work around it") + expect(system(body)).toContain("when calls are independent") +} + +describe("final request capability and ownership boundaries (#244)", () => { + test("session-only denial removes delegation advice after agent allows Task", async () => { + await fixture( + "session-deny", + { session: [{ permission: "task", pattern: "*", action: "deny" }] }, + async (context) => { + const body = await context.prompt() + expect(names(body)).toEqual(full.filter((name) => name !== "task")) + expect(system(body)).not.toContain("Use the Task tool") + expect(description(body, "glob")).not.toContain("use the Task tool") + expect(description(body, "grep")).not.toContain("use the Task tool") + safety(body) + }, + ) + }) + + test("legacy prompt tools-false applies before rendering guidance", async () => { + await fixture("legacy-tools-false", {}, async (context) => { + const body = await context.prompt({ tools: Object.fromEntries(off.map((name) => [name, false])) }) + expect(names(body)).toEqual(reduced) + expect(system(body)).not.toContain("Use the Task tool") + expect(description(body, "bash")).not.toContain("Glob (not find/ls)") + expect(description(body, "read")).not.toContain("Use the grep tool") + safety(body) + }) + }) + + for (const absent of ["glob", "grep"] as const) { + test(`search capability ${absent} renders independently`, async () => { + await fixture(`mixed-search-${absent}`, { permission: { [absent]: "deny" } }, async (context) => { + const body = await context.prompt() + expect(names(body)).toEqual(full.filter((name) => name !== absent)) + const present = absent === "glob" ? "grep" : "glob" + expect(description(body, "bash")).not.toContain(absent === "glob" ? "Glob (not find/ls)" : "Grep (not grep/rg)") + expect(description(body, "bash")).toContain(present === "glob" ? "Glob (not find/ls)" : "Grep (not grep/rg)") + expect(description(body, "read")).not.toContain(absent === "glob" ? "use the glob tool" : "Use the grep tool") + }) + }) + } + + test("Grep does not recommend unavailable Bash for counts", async () => { + await fixture("no-bash", { permission: { bash: "deny" } }, async (context) => { + const body = await context.prompt() + expect(names(body)).not.toContain("bash") + expect(description(body, "grep")).not.toContain("Bash tool with `rg`") + expect(system(body)).not.toContain("with bash if they were provided") + }) + }) + + test("full reduced full reuses original tools without mutation or guidance leakage", async () => { + await fixture("full-reduced-full", {}, async (context) => { + const first = await context.prompt() + const input = context.input() + const originalTools = input.tools + const before = Object.fromEntries(Object.entries(originalTools).map(([name, value]) => [name, value.description])) + const middle = await context.direct({ + ...input, + permission: off.map((permission) => ({ permission, pattern: "*", action: "deny" })), + }) + expect(names(middle)).toEqual(reduced) + expect(system(middle)).not.toContain("Use the Task tool") + expect( + Object.fromEntries(Object.entries(originalTools).map(([name, value]) => [name, value.description])), + ).toEqual(before) + const last = await context.direct({ ...input, permission: [] }) + expect(names(first)).toEqual(full) + expect(names(last)).toEqual(full) + expect(description(last, "bash")).toEqual(description(first, "bash")) + expect(system(last)).toContain("Use the Task tool") + }) + }) + + test("same-name custom Task does not enable built-in delegation recommendations", async () => { + const text = "Custom Task is a ticket lookup. Literal use the Task tool stays verbatim." + await fixture("custom-task", {}, async (context) => { + await ToolRegistry.register(custom("task", text)) + const body = await context.prompt() + expect(names(body)).toEqual(full) + expect(description(body, "task")).toBe(text) + expect(system(body)).not.toContain("Use the Task tool") + expect(description(body, "glob")).not.toContain("use the Task tool") + expect(description(body, "grep")).not.toContain("use the Task tool") + }) + }) + + test("same-name custom Read preserves text but cannot enable built-in reading advice", async () => { + const text = "Custom Read queries tickets, not files. Read this exact text." + await fixture("custom-read", {}, async (context) => { + await ToolRegistry.register(custom("read", text)) + const body = await context.prompt() + expect(description(body, "read")).toBe(text) + expect(description(body, "bash")).not.toContain("Read (not cat/head/tail)") + expect(description(body, "edit")).not.toContain("use your `Read` tool") + expect(description(body, "edit")).toContain("oldString") + expect(description(body, "write")).not.toContain("use your `Read` tool") + }) + }) + + test("missing Read preserves truthful conditional Edit Write prerequisites", async () => { + await fixture("no-read-prerequisites", { permission: { read: "deny" } }, async (context) => { + const body = await context.prompt() + expect(names(body)).not.toContain("read") + expect(description(body, "edit")).not.toContain("use your `Read` tool") + expect(description(body, "edit")).toContain("oldString") + expect(description(body, "edit")).toContain("empty") + expect(description(body, "edit")).toContain("missing or stale") + expect(description(body, "edit")).toContain("session") + expect(description(body, "write")).not.toContain("use your `Read` tool") + expect(description(body, "write")).toContain("session") + expect(description(body, "write")).toContain("existing") + expect(description(body, "write")).toContain("new file") + expect(description(body, "write")).toContain("missing or stale") + }) + }) + + for (const mode of ["replace", "append", "parameters"] as const) { + test(`plugin ${mode} respects description ownership`, async () => { + const original = Plugin.trigger + let expected = "" + const spy = spyOn(Plugin, "trigger").mockImplementation((async (name, input, output) => { + await original(name, input, output) + if (name === "tool.definition" && (input as { toolID: string }).toolID === "bash") { + const definition = output as { description: string; parameters: import("zod").ZodType } + if (mode === "replace") definition.description = "Plugin replacement: use Glob and Task literally." + if (mode === "append") definition.description += "\nPlugin appendix: use Glob and Task literally." + if (mode === "parameters") definition.parameters = definition.parameters.describe("plugin parameters") + expected = definition.description + } + return output + }) as typeof Plugin.trigger) + try { + await fixture(`plugin-${mode}`, { permission: denied }, async (context) => { + const body = await context.prompt() + expect(names(body)).toEqual(reduced) + if (mode === "parameters") { + expect(description(body, "bash")).not.toContain("Glob (not find/ls)") + expect(description(body, "bash")).not.toBe(expected) + } else expect(description(body, "bash")).toBe(expected) + }) + } finally { + spy.mockRestore() + } + }) + } + + test("custom agent caller and MCP text remains verbatim", async () => { + const agent = "Custom agent explicitly mentions Task, Glob and Grep. Preserve this literal sentence." + const caller = "Caller text: use the Task tool even when absent. Preserve literally." + const mcpText = "MCP text: use the Task tool, Glob and Grep literally." + const spy = spyOn(MCP, "tools").mockResolvedValue({ + fixture_external: tool({ + description: mcpText, + inputSchema: jsonSchema({ type: "object", properties: {} }), + execute: async () => ({ content: [{ type: "text" as const, text: "fixture" }] }), + }), + }) + try { + await fixture("custom-text", { permission: denied, buildPrompt: agent }, async (context) => { + const body = await context.prompt({ system: caller }) + expect(names(body)).toEqual([...reduced, "fixture_external"].sort()) + expect(system(body)).toContain(agent) + expect(system(body)).toContain(caller) + expect(description(body, "fixture_external")).toBe(mcpText) + }) + } finally { + spy.mockRestore() + } + }) + + test("GPT catalog uses apply_patch and omits unavailable built-in Bash peers", async () => { + await fixture("gpt-api", { gpt: true }, async (context) => { + const body = await context.prompt() + expect(names(body)).toEqual(["apply_patch", ...full.filter((name) => name !== "edit" && name !== "write")].sort()) + expect(description(body, "bash")).not.toContain("Edit (not sed/awk/perl)") + expect(description(body, "bash")).not.toContain("Write (not echo/heredoc redirection)") + }) + }) + + test("Codex full GPT catalog retains surviving capabilities and omits Edit Write", async () => { + await fixture("codex-full", { codex: true }, async (context) => { + const body = await context.prompt() + expect(names(body)).toEqual(["apply_patch", ...full.filter((name) => name !== "edit" && name !== "write")].sort()) + expect(body.instructions).toContain("Try to use apply_patch") + expect(body.instructions).toContain("Use Read") + expect(body.instructions).toContain("Use Glob") + expect(body.instructions).toContain("Grep to search") + expect(body.instructions).not.toContain("Edit to modify") + expect(body.instructions).not.toContain("Write only when needed") + }) + }) + + test("Codex no-Bash instructions preserve safety without terminal recommendations", async () => { + await fixture("codex-no-bash", { codex: true, permission: { bash: "deny" } }, async (context) => { + const body = await context.prompt() + expect(names(body)).not.toContain("bash") + expect(body.instructions).not.toContain("Use Bash for terminal") + expect(body.instructions).toContain("Run tool calls in parallel") + expect(body.instructions).toContain("NEVER revert existing changes") + }) + }) + + test("Codex OAuth instructions use final surviving tools", async () => { + await fixture("codex-reduced", { codex: true, permission: { ...denied, apply_patch: "deny" } }, async (context) => { + const body = await context.prompt() + expect(names(body)).toEqual(["bash", "read"]) + expect(body.instructions).toBeString() + expect(body.instructions).not.toContain("Try to use apply_patch") + expect(body.instructions).not.toContain("Use Glob") + expect(body.instructions).not.toContain("Grep to search") + expect(body.instructions).not.toContain("Edit to modify") + expect(body.instructions).toContain("Use Read") + expect(body.instructions).toContain("Run tool calls in parallel") + expect(body.instructions).toContain("NEVER revert existing changes") + }) + }) +}) From 0dc3dd76ffab2e989716b1a43f144b611ed55d0f Mon Sep 17 00:00:00 2001 From: Bulat Yapparov Date: Wed, 21 Oct 2026 01:34:54 +0100 Subject: [PATCH 2/6] fix: remove unavailable-tool advice from runtime messages --- packages/cli/src/file/time.ts | 7 +- packages/cli/src/tool/truncation.ts | 13 +- .../cli/test/session/tool-guidance.test.ts | 263 +++++++++++++++++- .../cli/test/tool/runtime-guidance.test.ts | 210 ++++++++++++++ packages/cli/test/tool/truncation.test.ts | 12 +- 5 files changed, 481 insertions(+), 24 deletions(-) create mode 100644 packages/cli/test/tool/runtime-guidance.test.ts diff --git a/packages/cli/src/file/time.ts b/packages/cli/src/file/time.ts index f7fc1074..e8eed1e9 100644 --- a/packages/cli/src/file/time.ts +++ b/packages/cli/src/file/time.ts @@ -59,12 +59,15 @@ export namespace FileTime { } const time = get(sessionID, filepath) - if (!time) throw new Error(`You must read file ${filepath} before overwriting it. Use the Read tool first`) + if (!time) + throw new Error( + `You must read file ${filepath} before overwriting it. This session has no recorded read of its current contents.`, + ) const mtime = Filesystem.stat(filepath)?.mtime // Allow a 50ms tolerance for Windows NTFS timestamp fuzziness / async flushing if (mtime && mtime.getTime() > time.getTime() + 50) { throw new Error( - `File ${filepath} has been modified since it was last read.\nLast modification: ${mtime.toISOString()}\nLast read: ${time.toISOString()}\n\nPlease read the file again before modifying it.`, + `File ${filepath} has been modified since it was last read.\nLast modification: ${mtime.toISOString()}\nLast read: ${time.toISOString()}\n\nA current read must be recorded in this session before modifying the file.`, ) } } diff --git a/packages/cli/src/tool/truncation.ts b/packages/cli/src/tool/truncation.ts index 0ea27438..cf200260 100644 --- a/packages/cli/src/tool/truncation.ts +++ b/packages/cli/src/tool/truncation.ts @@ -2,7 +2,6 @@ import fs from "fs/promises" import path from "path" import { Global } from "../global" import { Identifier } from "../id/id" -import { PermissionNext } from "../permission/next" import type { Agent } from "../agent/agent" import { Scheduler } from "../scheduler" import { Filesystem } from "../util/filesystem" @@ -42,13 +41,7 @@ export namespace Truncate { } } - function hasTaskTool(agent?: Agent.Info): boolean { - if (!agent?.permission) return false - const rule = PermissionNext.evaluate("task", "*", agent.permission) - return rule.action !== "deny" - } - - export async function output(text: string, options: Options = {}, agent?: Agent.Info): Promise { + export async function output(text: string, options: Options = {}, _agent?: Agent.Info): Promise { const maxLines = options.maxLines ?? MAX_LINES const maxBytes = options.maxBytes ?? MAX_BYTES const direction = options.direction ?? "head" @@ -94,9 +87,7 @@ export namespace Truncate { const filepath = path.join(DIR, id) await Filesystem.write(filepath, text) - const hint = hasTaskTool(agent) - ? `The tool call succeeded but the output was truncated. Full output saved to: ${filepath}\nUse the Task tool to have explore agent process this file with Grep and Read (with offset/limit). Do NOT read the full file yourself - delegate to save context.` - : `The tool call succeeded but the output was truncated. Full output saved to: ${filepath}\nUse Grep to search the full content or Read with offset/limit to view specific sections.` + const hint = `The tool call succeeded but the output was truncated. Full output saved to: ${filepath}` const message = direction === "head" ? `${preview}\n\n...${removed} ${unit} truncated...\n\n${hint}` diff --git a/packages/cli/test/session/tool-guidance.test.ts b/packages/cli/test/session/tool-guidance.test.ts index cb845a2d..59a958ac 100644 --- a/packages/cli/test/session/tool-guidance.test.ts +++ b/packages/cli/test/session/tool-guidance.test.ts @@ -56,6 +56,8 @@ globalThis.fetch = (async (input, init) => { transport.captures.push(JSON.parse(String(init?.body)) as Wire) return responses() } + if (["data:image/png;base64,aW5lcnQ=", "data:application/octet-stream;base64,Ynl0ZXM="].includes(url.href)) + return originalFetch(input, init) if (url.origin !== transport.origin) throw new Error(`Blocked unexpected model transport: ${url.origin}`) return originalFetch(input, init) }) as typeof fetch @@ -96,6 +98,9 @@ type Fixture = { prompt(input?: Partial): Promise direct(input: StreamInput): Promise input(): StreamInput + directory: string + sessionID: string + messages(): ReturnType } function definitions(body: Wire) { @@ -130,6 +135,9 @@ async function fixture( gpt?: boolean codex?: boolean buildPrompt?: string + call?: () => { name: string; arguments: Record } + images?: boolean + init?: (directory: string) => Promise } = {}, fn: (context: Fixture) => Promise, ) { @@ -142,6 +150,36 @@ async function fixture( port: 0, async fetch(request) { transport.captures.push((await request.json()) as Wire) + if (options.call && transport.captures.length === 1) { + const call = options.call() + return sse([ + { + id: "fixture-call", + object: "chat.completion.chunk", + choices: [ + { + index: 0, + delta: { + role: "assistant", + tool_calls: [ + { + index: 0, + id: "runtime-call", + type: "function", + function: { name: call.name, arguments: JSON.stringify(call.arguments) }, + }, + ], + }, + }, + ], + }, + { + id: "fixture-call", + object: "chat.completion.chunk", + choices: [{ index: 0, delta: {}, finish_reason: "tool_calls" }], + }, + ]) + } return providerID === "openai" ? responses() : chat() }, }) @@ -156,12 +194,15 @@ async function fixture( }) const original = LLM.stream let last: StreamInput | undefined + let persisted: unknown + const persistedCheckpoints: unknown[] = [] const spy = spyOn(LLM, "stream").mockImplementation(async (input) => { last = input return original(input) }) try { await using tmp = await tmpdir({ + init: options.init, config: { permission: { "*": "allow", ...options.permission }, enabled_providers: [providerID], @@ -170,7 +211,16 @@ async function fixture( npm: providerID === "openai" ? "@ai-sdk/openai" : "@ai-sdk/openai-compatible", options: { apiKey: "fixture", baseURL: `${server.url.origin}/v1` }, ...(providerID === "fixture" - ? { models: { fixture: { name: "fixture", tool_call: true, limit: { context: 200_000, output: 1000 } } } } + ? { + models: { + fixture: { + name: "fixture", + tool_call: true, + ...(options.images ? { modalities: { input: ["text", "image"], output: ["text"] } } : {}), + limit: { context: 200_000, output: 1000 }, + }, + }, + } : {}), }, }, @@ -182,6 +232,9 @@ async function fixture( fn: async () => { const session = await Session.create({ title: "Tool guidance fixture", permission: options.session }) const context: Fixture = { + directory: tmp.path, + sessionID: session.id, + messages: () => Session.messages({ sessionID: session.id }), async prompt(input = {}) { const before = transport.captures.length const result = await SessionPrompt.prompt({ @@ -191,8 +244,24 @@ async function fixture( ...input, }) if (result.info.role !== "assistant" || result.info.error) throw new Error(JSON.stringify(result.info)) - expect(transport.captures.length - before).toBe(1) - return transport.captures[before] + expect(transport.captures.length - before).toBe(before === 0 && options.call ? 2 : 1) + const messages = await Session.messages({ sessionID: session.id }) + const savedOutputs = [] + for (const message of messages) + for (const part of message.parts) { + if (part.type !== "tool" || part.state.status !== "completed" || !part.state.metadata.outputPath) + continue + const outputPath = part.state.metadata.outputPath as string + const bytes = await Bun.file(outputPath).bytes() + savedOutputs.push({ + outputPath, + bytes: bytes.length, + sha256: new Bun.CryptoHasher("sha256").update(bytes).digest("hex"), + content: new TextDecoder().decode(bytes), + }) + } + persistedCheckpoints.push({ afterRequestCount: transport.captures.length, messages, savedOutputs }) + return transport.captures.at(-1)! }, async direct(input) { const before = transport.captures.length @@ -208,7 +277,11 @@ async function fixture( return last }, } - await fn(context) + try { + await fn(context) + } finally { + persisted = await context.messages() + } }, }) } finally { @@ -216,7 +289,11 @@ async function fixture( if (output) await Bun.write( path.join(output, `${label}.json`), - JSON.stringify({ label, codex: options.codex ?? false, requests: transport.captures }, null, 2), + JSON.stringify( + { label, codex: options.codex ?? false, requests: transport.captures, persisted, persistedCheckpoints }, + null, + 2, + ), ) spy.mockRestore() server.stop(true) @@ -499,3 +576,179 @@ describe("final request capability and ownership boundaries (#244)", () => { }) }) }) + +function toolParts(messages: Awaited>) { + return messages.flatMap((message) => message.parts).filter((part) => part.type === "tool") +} +function nextToolText(body: Wire) { + return (body.messages ?? []) + .filter((message) => message.role === "tool") + .map((message) => (typeof message.content === "string" ? message.content : JSON.stringify(message.content))) + .join("\n") +} +function noNewAdvice(text: string) { + expect(text).not.toContain("Use the Task tool") + expect(text).not.toContain("Use Grep to search") + expect(text).not.toContain("Read with offset/limit") + expect(text).not.toContain("Use the Read tool first") +} + +describe("runtime hints through actual tool loop and next request (#244)", () => { + test("real oversized Bash persists factual result and serializes it on the next request", async () => { + const original = "benign-runtime-line\n".repeat(3001) + await fixture( + "runtime-bash", + { + permission: { ...denied, read: "deny" }, + call: () => ({ + name: "bash", + arguments: { + command: `node -e 'process.stdout.write("benign-runtime-line\\n".repeat(3001))'`, + description: "Print benign oversized fixture output", + }, + }), + }, + async (context) => { + const body = await context.prompt() + expect(names(body)).toEqual(["bash", "edit", "write"]) + const part = toolParts(await context.messages()).find((part) => part.tool === "bash")! + expect(part.state.status).toBe("completed") + if (part.state.status !== "completed") throw new Error(JSON.stringify(part.state)) + const outputPath = part.state.metadata.outputPath as string + expect(await Bun.file(outputPath).text()).toBe(original) + expect(part.state.metadata.truncated).toBe(true) + expect(part.state.output).toContain(outputPath) + expect(nextToolText(body)).toContain(part.state.output) + noNewAdvice(part.state.output) + noNewAdvice(nextToolText(body)) + const historical = + "Historical result: Use the Task tool to have explore agent process this file with Grep and Read (with offset/limit)." + await Session.updatePart({ ...part, state: { ...part.state, output: historical } }) + const replay = await context.prompt() + expect(nextToolText(replay)).toContain(historical) + }, + ) + }) + + for (const name of ["edit", "write"] as const) { + test(`Read-denied real ${name} failure keeps bytes and replays accurate error`, async () => { + let filepath = "" + await fixture( + `runtime-${name}-unread`, + { + permission: { ...denied, read: "deny" }, + init: async (directory) => { + filepath = path.join(directory, "protected.txt") + await Bun.write(filepath, "original bytes\n") + }, + call: () => ({ + name, + arguments: + name === "edit" + ? { filePath: filepath, oldString: "original", newString: "changed" } + : { filePath: filepath, content: "changed bytes" }, + }), + }, + async (context) => { + const body = await context.prompt() + expect(names(body)).not.toContain("read") + expect(await Bun.file(filepath).text()).toBe("original bytes\n") + const part = toolParts(await context.messages()).find((part) => part.tool === name)! + expect(part.state.status).toBe("error") + if (part.state.status !== "error") throw new Error(JSON.stringify(part.state)) + expect(part.state.error).toContain(filepath) + expect(part.state.error).toMatch(/read/i) + expect(nextToolText(body)).toContain(part.state.error) + noNewAdvice(part.state.error) + noNewAdvice(nextToolText(body)) + const historical = `You must read file ${filepath} before overwriting it. Use the Read tool first` + await Session.updatePart({ ...part, state: { ...part.state, error: historical } }) + const replay = await context.prompt() + expect(nextToolText(replay)).toContain(historical) + }, + ) + }) + } + + test("custom plugin output retains literal advice while only harness suffix changes", async () => { + const original = "Plugin original: Use the Task tool, Grep and Read literally.\n" + "plugin-data\n".repeat(3001) + await fixture( + "runtime-plugin", + { + permission: { ...denied, read: "deny" }, + init: async (directory) => { + await Bun.write( + path.join(directory, ".aictrl/tool/oversized.ts"), + `export default { description: "Custom plugin fixture", args: {}, execute: async () => ${JSON.stringify(original)} }`, + ) + }, + call: () => ({ name: "oversized", arguments: {} }), + }, + async (context) => { + const body = await context.prompt() + const part = toolParts(await context.messages()).find((part) => part.tool === "oversized")! + if (part.state.status !== "completed") throw new Error(JSON.stringify(part.state)) + expect(await Bun.file(part.state.metadata.outputPath as string).text()).toBe(original) + expect(part.state.output).toStartWith("Plugin original: Use the Task tool, Grep and Read literally.") + const suffix = part.state.output.split("The tool call succeeded")[1] + expect(suffix).toBeString() + noNewAdvice(suffix) + expect(nextToolText(body)).toContain(part.state.output) + }, + ) + }) + + test("MCP original text content and attachment ordering survives truncation wrapper", async () => { + const first = "MCP original: Use the Task tool, Grep and Read literally.\n" + "mcp-data\n".repeat(3001) + const content = [ + { type: "text" as const, text: first }, + { type: "image" as const, mimeType: "image/png", data: "aW5lcnQ=" }, + { + type: "resource" as const, + resource: { uri: "fixture://notes", mimeType: "text/plain", text: "resource tail" }, + }, + { + type: "resource" as const, + resource: { uri: "fixture://blob", mimeType: "application/octet-stream", blob: "Ynl0ZXM=" }, + }, + ] + const spy = spyOn(MCP, "tools").mockImplementation(async () => ({ + fixture_mcp: tool({ + description: "Local MCP fixture", + inputSchema: jsonSchema({ type: "object", properties: {} }), + execute: async () => ({ content, metadata: { fixture: "preserve" } }), + }), + })) + try { + await fixture( + "runtime-mcp", + { images: true, permission: { ...denied, read: "deny" }, call: () => ({ name: "fixture_mcp", arguments: {} }) }, + async (context) => { + const body = await context.prompt() + const part = toolParts(await context.messages()).find((part) => part.tool === "fixture_mcp")! + if (part.state.status !== "completed") throw new Error(JSON.stringify(part.state)) + expect(await Bun.file(part.state.metadata.outputPath as string).text()).toBe(first + "\n\nresource tail") + expect(part.state.metadata.fixture).toBe("preserve") + expect(part.state.output).toStartWith("MCP original: Use the Task tool, Grep and Read literally.") + noNewAdvice(part.state.output.split("The tool call succeeded")[1]) + expect(part.state.attachments?.map((item) => item.mime)).toEqual(["image/png", "application/octet-stream"]) + expect(part.state.attachments?.map((item) => item.url)).toEqual([ + "data:image/png;base64,aW5lcnQ=", + "data:application/octet-stream;base64,Ynl0ZXM=", + ]) + expect(nextToolText(body)).toContain("MCP original: Use the Task tool, Grep and Read literally.") + expect(nextToolText(body)).not.toContain("resource tail") + const toolContent = JSON.parse(nextToolText(body)) + expect(toolContent.map((item: { type: string }) => item.type)).toEqual(["text", "media"]) + expect(toolContent[0].text).toBe(part.state.output) + expect(toolContent[1]).toEqual({ type: "media", mediaType: "application/octet-stream", data: "Ynl0ZXM=" }) + expect(JSON.stringify(body.messages)).toContain("data:image/png;base64,aW5lcnQ=") + expect(content[0].text).toBe(first) + expect(content[2].resource?.text).toBe("resource tail") + }, + ) + } finally { + spy.mockRestore() + } + }) +}) diff --git a/packages/cli/test/tool/runtime-guidance.test.ts b/packages/cli/test/tool/runtime-guidance.test.ts new file mode 100644 index 00000000..fb345c59 --- /dev/null +++ b/packages/cli/test/tool/runtime-guidance.test.ts @@ -0,0 +1,210 @@ +import { afterAll, describe, expect, test } from "bun:test" +import path from "node:path" +import fs from "node:fs/promises" + +const originalFetch = globalThis.fetch +globalThis.fetch = Object.assign( + async () => { + throw new Error("Runtime guidance unit fixture forbids network") + }, + { + preconnect: () => { + throw new Error("Runtime guidance unit fixture forbids preconnect") + }, + }, +) +const [{ Truncate }, { FileTime }, { Instance }, { tmpdir }, { EditTool }, { WriteTool }, { ReadTool }] = + await Promise.all([ + import("../../src/tool/truncation"), + import("../../src/file/time"), + import("../../src/project/instance"), + import("../fixture/fixture"), + import("../../src/tool/edit"), + import("../../src/tool/write"), + import("../../src/tool/read"), + ]) +afterAll(() => { + globalThis.fetch = originalFetch +}) + +describe("runtime guidance correctness (#244)", () => { + test("new truncation suffix reports facts without prescribing Task Grep Read", async () => { + const original = "first\nsecond\nthird" + const result = await Truncate.output(original, { maxLines: 1 }, { + permission: [{ permission: "task", pattern: "*", action: "allow" }], + } as any) + expect(result.truncated).toBe(true) + if (!result.truncated) throw new Error("Expected truncation") + expect(await Bun.file(result.outputPath).text()).toBe(original) + expect(result.content).toContain("...2 lines truncated...") + expect(result.content).toContain(result.outputPath) + expect(result.content).not.toMatch(/\b(?:Task|Grep|Read)\b/) + }) + + test("missing read record fails accurately without prescribing unavailable Read", async () => { + await using tmp = await tmpdir() + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const filepath = path.join(tmp.path, "original.txt") + await Bun.write(filepath, "original") + const error = await FileTime.assert("fixture-session", filepath).then( + () => { + throw new Error("Expected missing-read failure") + }, + (error: Error) => error, + ) + expect(error.message).toContain(filepath) + expect(error.message).toMatch(/read/i) + expect(error.message).not.toContain("Use the Read tool") + expect(await Bun.file(filepath).text()).toBe("original") + }, + }) + }) +}) + +const context = { + sessionID: "runtime-guidance-session", + messageID: "fixture", + callID: "fixture", + agent: "build", + abort: new AbortController().signal, + messages: [], + metadata: () => {}, + ask: async () => {}, +} + +describe("factual truncation controls (#244)", () => { + for (const direction of ["head", "tail"] as const) { + test(`${direction} keeps preview order exact line count and saved full bytes`, async () => { + const content = "zero\none\ntwo\nthree\nfour" + const result = await Truncate.output(content, { maxLines: 2, direction }) + if (!result.truncated) throw new Error("Expected truncation") + expect(await Bun.file(result.outputPath).text()).toBe(content) + expect(result.content).toContain("...3 lines truncated...") + expect(result.content).toContain(`Full output saved to: ${result.outputPath}`) + expect(result.content).not.toMatch(/\b(?:Task|Grep|Read)\b/) + if (direction === "head") { + expect(result.content).toStartWith("zero\none\n\n...") + expect(result.content).not.toContain("four") + } else { + expect(result.content).toStartWith("...3 lines truncated...") + expect(result.content).toEndWith("three\nfour") + } + }) + test(`${direction} counts UTF-8 omitted bytes without corrupting full saved text`, async () => { + const content = "éé\nzz\n你好" + const result = await Truncate.output(content, { maxLines: 99, maxBytes: 6, direction }) + if (!result.truncated) throw new Error("Expected truncation") + expect(Array.from(await Bun.file(result.outputPath).bytes())).toEqual( + Array.from(new TextEncoder().encode(content)), + ) + const kept = direction === "head" ? Buffer.byteLength("éé") : Buffer.byteLength("你好") + expect(result.content).toContain(`...${Buffer.byteLength(content) - kept} bytes truncated...`) + if (direction === "head") expect(result.content).toStartWith("éé\n\n...") + else expect(result.content).toEndWith("你好") + }) + } + test("exact limits and small content stay verbatim without saved-output metadata", async () => { + for (const content of ["", "abc", "a\nb"]) { + const result = await Truncate.output(content, { maxBytes: Math.max(Buffer.byteLength(content), 1), maxLines: 2 }) + expect(result).toEqual({ content, truncated: false }) + } + }) +}) + +describe("actual read edit write enforcement controls (#244)", () => { + for (const name of ["edit", "write"] as const) { + for (const state of ["unread", "stale"] as const) { + test(`${name} ${state} attempt rejects before mutating any original bytes`, async () => { + await using tmp = await tmpdir() + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const filepath = path.join(tmp.path, "protected.txt") + const original = new Uint8Array([111, 114, 105, 103, 105, 110, 97, 108, 10]) + await Bun.write(filepath, original) + let read: Date | undefined + let modified: Date | undefined + if (state === "stale") { + FileTime.read(context.sessionID, filepath) + read = FileTime.get(context.sessionID, filepath)! + await fs.utimes(filepath, read, new Date(read.getTime() + 1000)) + modified = (await fs.stat(filepath)).mtime + } + const tool = name === "edit" ? await EditTool.init() : await WriteTool.init() + const args = + name === "edit" + ? { filePath: filepath, oldString: "original", newString: "changed" } + : { filePath: filepath, content: "changed" } + let error: Error | undefined + try { + await tool.execute(args as any, context) + } catch (e) { + error = e as Error + } + expect(error).toBeInstanceOf(Error) + expect(error!.message).toContain(filepath) + expect(error!.message).not.toContain("Use the Read tool") + expect(error!.message).toMatch(/read/i) + if (state === "stale") { + expect(error!.message).toContain("modified since it was last read") + expect(error!.message).toContain(`Last modification: ${modified!.toISOString()}`) + expect(error!.message).toContain(`Last read: ${read!.toISOString()}`) + } + expect(await Bun.file(filepath).bytes()).toEqual(original) + }, + }) + }) + } + test(`${name} valid actual Read allows the existing-file update`, async () => { + await using tmp = await tmpdir() + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const filepath = path.join(tmp.path, "valid.txt") + await Bun.write(filepath, "original\n") + const read = await ReadTool.init() + await read.execute({ filePath: filepath }, context) + if (name === "edit") + await ( + await EditTool.init() + ).execute({ filePath: filepath, oldString: "original", newString: "changed" }, context) + else await (await WriteTool.init()).execute({ filePath: filepath, content: "changed\n" }, context) + expect(await Bun.file(filepath).text()).toBe("changed\n") + }, + }) + }) + test(`${name} new-file exception still works without a read record`, async () => { + await using tmp = await tmpdir() + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const filepath = path.join(tmp.path, "new.txt") + expect(FileTime.get(context.sessionID, filepath)).toBeUndefined() + if (name === "edit") + await ( + await EditTool.init() + ).execute({ filePath: filepath, oldString: "", newString: "new bytes" }, context) + else await (await WriteTool.init()).execute({ filePath: filepath, content: "new bytes" }, context) + expect(await Bun.file(filepath).text()).toBe("new bytes") + }, + }) + }) + } + test("empty-oldString direct overwrite retains its existing read exemption", async () => { + await using tmp = await tmpdir() + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const filepath = path.join(tmp.path, "direct.txt") + await Bun.write(filepath, "original") + expect(FileTime.get(context.sessionID, filepath)).toBeUndefined() + await ( + await EditTool.init() + ).execute({ filePath: filepath, oldString: "", newString: "direct overwrite" }, context) + expect(await Bun.file(filepath).text()).toBe("direct overwrite") + }, + }) + }) +}) diff --git a/packages/cli/test/tool/truncation.test.ts b/packages/cli/test/tool/truncation.test.ts index 69f69e84..7a7e8c3a 100644 --- a/packages/cli/test/tool/truncation.test.ts +++ b/packages/cli/test/tool/truncation.test.ts @@ -88,7 +88,7 @@ describe("Truncate", () => { expect(result.truncated).toBe(true) expect(result.content).toContain("The tool call succeeded but the output was truncated") - expect(result.content).toContain("Grep") + expect(result.content).not.toMatch(/\b(?:Task|Grep|Read)\b/) if (!result.truncated) throw new Error("expected truncated") expect(result.outputPath).toBeDefined() expect(result.outputPath).toContain("tool_") @@ -97,23 +97,23 @@ describe("Truncate", () => { expect(written).toBe(lines) }) - test("suggests Task tool when agent has task permission", async () => { + test("reports facts even when agent has task permission", async () => { const lines = Array.from({ length: 100 }, (_, i) => `line${i}`).join("\n") const agent = { permission: [{ permission: "task", pattern: "*", action: "allow" as const }] } const result = await Truncate.output(lines, { maxLines: 10 }, agent as any) expect(result.truncated).toBe(true) - expect(result.content).toContain("Grep") - expect(result.content).toContain("Task tool") + expect(result.content).not.toMatch(/\b(?:Task|Grep|Read)\b/) + expect(result.content).not.toContain("Task tool") }) - test("omits Task tool hint when agent lacks task permission", async () => { + test("reports facts when agent lacks task permission", async () => { const lines = Array.from({ length: 100 }, (_, i) => `line${i}`).join("\n") const agent = { permission: [{ permission: "task", pattern: "*", action: "deny" as const }] } const result = await Truncate.output(lines, { maxLines: 10 }, agent as any) expect(result.truncated).toBe(true) - expect(result.content).toContain("Grep") + expect(result.content).not.toMatch(/\b(?:Task|Grep|Read)\b/) expect(result.content).not.toContain("Task tool") }) From a77f494c35592923ce23d258157d1826e349ef1d Mon Sep 17 00:00:00 2001 From: Bulat Yapparov Date: Wed, 21 Oct 2026 01:41:34 +0100 Subject: [PATCH 3/6] test: load oversized plugin fixture output as data --- packages/cli/test/session/tool-guidance.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/cli/test/session/tool-guidance.test.ts b/packages/cli/test/session/tool-guidance.test.ts index 59a958ac..4c0e5c04 100644 --- a/packages/cli/test/session/tool-guidance.test.ts +++ b/packages/cli/test/session/tool-guidance.test.ts @@ -677,9 +677,13 @@ describe("runtime hints through actual tool loop and next request (#244)", () => { permission: { ...denied, read: "deny" }, init: async (directory) => { + await Bun.write(path.join(directory, "oversized-output.txt"), original) await Bun.write( path.join(directory, ".aictrl/tool/oversized.ts"), - `export default { description: "Custom plugin fixture", args: {}, execute: async () => ${JSON.stringify(original)} }`, + [ + 'import { readFile } from "node:fs/promises"', + 'export default { description: "Custom plugin fixture", args: {}, execute: async () => readFile(new URL("../../oversized-output.txt", import.meta.url), "utf8") }', + ].join("\n"), ) }, call: () => ({ name: "oversized", arguments: {} }), From 7b4aa88a5a2bc1657df565b8a6069f01a48e361c Mon Sep 17 00:00:00 2001 From: Bulat Yapparov Date: Wed, 21 Oct 2026 02:00:03 +0100 Subject: [PATCH 4/6] fix: align remaining owned tool guidance --- packages/cli/src/agent/agent.ts | 12 + packages/cli/src/session/llm.ts | 5 +- packages/cli/src/session/prompt.ts | 12 +- packages/cli/src/tool/batch.ts | 2 + packages/cli/src/tool/guidance.ts | 33 ++ .../cli/test/session/tool-guidance.test.ts | 339 +++++++++++++++++- 6 files changed, 390 insertions(+), 13 deletions(-) diff --git a/packages/cli/src/agent/agent.ts b/packages/cli/src/agent/agent.ts index 03ff2104..334f13f6 100644 --- a/packages/cli/src/agent/agent.ts +++ b/packages/cli/src/agent/agent.ts @@ -19,6 +19,7 @@ import path from "path" import { Plugin } from "@/plugin" import { Skill } from "../skill" import { Log } from "@/util/log" +import { ToolGuidance } from "@/tool/guidance" export namespace Agent { const log = Log.create({ service: "agent" }) @@ -50,6 +51,14 @@ export namespace Agent { }) export type Info = z.infer + const prompts = new WeakMap() + + export function renderPrompt(agent: Info, tools: ReadonlySet) { + const owned = prompts.get(agent) + if (!owned || agent.prompt !== owned.source) return agent.prompt + return owned.render(owned.source, tools) + } + const state = Instance.state(async () => { const cfg = await Config.get() @@ -171,6 +180,8 @@ export namespace Agent { }, } + prompts.set(result.explore, { source: PROMPT_EXPLORE, render: ToolGuidance.explore }) + for (const [key, value] of Object.entries(cfg.agent ?? {})) { // the plan agent was removed: an old override of it must not become a custom agent with full permissions if (key === "plan") { @@ -192,6 +203,7 @@ export namespace Agent { } if (value.model) item.model = Provider.parseModel(value.model) item.variant = value.variant ?? item.variant + if (value.prompt !== undefined) prompts.delete(item) item.prompt = value.prompt ?? item.prompt item.description = value.description ?? item.description item.temperature = value.temperature ?? item.temperature diff --git a/packages/cli/src/session/llm.ts b/packages/cli/src/session/llm.ts index 109cc84b..8925e2ea 100644 --- a/packages/cli/src/session/llm.ts +++ b/packages/cli/src/session/llm.ts @@ -16,7 +16,7 @@ import { import { mergeDeep, pipe } from "remeda" import { ProviderTransform } from "@/provider/transform" import { Config } from "@/config/config" -import type { Agent } from "@/agent/agent" +import { Agent } from "@/agent/agent" import type { MessageV2 } from "./message-v2" import { Plugin } from "@/plugin" import { SystemPrompt } from "./system" @@ -272,7 +272,8 @@ export namespace LLM { * Codex sign-in sessions send no base prompt here, because they send the Codex header as `options.instructions`. */ function leadPrompt(agent: Agent.Info, isCodex: boolean, tools: ReadonlySet) { - if (agent.prompt) return [agent.prompt] + const prompt = Agent.renderPrompt(agent, tools) + if (prompt) return [prompt] if (isCodex) return [] return [SystemPrompt.base(tools)] } diff --git a/packages/cli/src/session/prompt.ts b/packages/cli/src/session/prompt.ts index b9b64660..027bac11 100644 --- a/packages/cli/src/session/prompt.ts +++ b/packages/cli/src/session/prompt.ts @@ -1434,9 +1434,6 @@ export namespace SessionPrompt { } if (part.type === "agent") { - // Check if this agent would be denied by task permission - const perm = PermissionNext.evaluate("task", part.name, agent.permission) - const hint = perm.action === "deny" ? " . Invoked by user; guaranteed to exist." : "" return [ { ...part, @@ -1448,12 +1445,11 @@ export namespace SessionPrompt { sessionID: input.sessionID, type: "text", synthetic: true, - // An extra space is added here. Otherwise the 'Use' gets appended - // to user's last word; making a combined word + // Keep this fresh instruction neutral: final request tools are resolved later. text: - " Use the above message and context to generate a prompt and call the task tool with subagent: " + - part.name + - hint, + ` The user requested delegation to subagent: ${part.name}. ` + + "Use the preceding message and context to prepare its task. " + + "Delegate if an appropriate delegation tool is available; otherwise report that delegation is unavailable.", }, ] } diff --git a/packages/cli/src/tool/batch.ts b/packages/cli/src/tool/batch.ts index eecbfe29..aa91a53e 100644 --- a/packages/cli/src/tool/batch.ts +++ b/packages/cli/src/tool/batch.ts @@ -1,6 +1,7 @@ import z from "zod" import { Tool } from "./tool" import DESCRIPTION from "./batch.txt" +import { ToolGuidance } from "./guidance" const DISALLOWED = new Set(["batch"]) const FILTERED_FROM_SUGGESTIONS = new Set(["invalid", "patch", ...DISALLOWED]) @@ -8,6 +9,7 @@ const FILTERED_FROM_SUGGESTIONS = new Set(["invalid", "patch", ...DISALLOWED]) export const BatchTool = Tool.define("batch", async () => { return { description: DESCRIPTION, + renderDescription: ToolGuidance.batch, parameters: z.object({ tool_calls: z .array( diff --git a/packages/cli/src/tool/guidance.ts b/packages/cli/src/tool/guidance.ts index 56bece3e..176ecfb6 100644 --- a/packages/cli/src/tool/guidance.ts +++ b/packages/cli/src/tool/guidance.ts @@ -119,4 +119,37 @@ export namespace ToolGuidance { : "- Replacing an existing file requires its current contents to already be recorded as read in this session. This tool fails if that record is missing or stale; creating a new file does not need that record.\n", ) } + + export function explore(description: string, tools: ReadonlySet) { + const lines = [ + ["glob", "- Use Glob for broad file pattern matching\n"], + ["grep", "- Use Grep for searching file contents with regex\n"], + ["read", "- Use Read when you know the specific file path you need to read\n"], + ["bash", "- Use Bash for file operations like copying, moving, or listing directory contents\n"], + ] as const + return lines.reduce((text, [id, line]) => replace(text, line, tools.has(id) ? line : ""), description) + } + + export function batch(description: string, tools: ReadonlySet) { + const example = + 'Payload Format (JSON array):\n[{"tool": "read", "parameters": {"filePath": "src/index.ts", "limit": 350}},{"tool": "grep", "parameters": {"pattern": "Session\\\\.updatePart", "include": "src/**/*.ts"}},{"tool": "bash", "parameters": {"command": "git status", "description": "Shows working tree status"}}]' + const search = ["grep", "glob", "read"].filter((id) => tools.has(id)) + const cases = [ + ["- Read many files\n", tools.has("read") ? "- Read many files\n" : ""], + ["- grep + glob + read combos\n", search.length >= 2 ? `- ${search.join(" + ")} combos\n` : ""], + ["- Multiple bash commands\n", tools.has("bash") ? "- Multiple bash commands\n" : ""], + [ + "- Multi-part edits; on the same, or different files\n", + ["edit", "write"].some((id) => tools.has(id)) ? "- Multi-part edits; on the same, or different files\n" : "", + ], + ] as const + const rendered = cases.reduce((text, [line, replacement]) => replace(text, line, replacement), description) + return replace( + replace(rendered, "Good Use Cases:\n", cases.some(([, replacement]) => replacement) ? "Good Use Cases:\n" : ""), + example, + ["read", "grep", "bash"].every((id) => tools.has(id)) + ? example + : "Supply independent calls to tools available in this request through the `tool_calls` array. Each item has `tool` and `parameters` fields.", + ) + } } diff --git a/packages/cli/test/session/tool-guidance.test.ts b/packages/cli/test/session/tool-guidance.test.ts index 4c0e5c04..1c771d32 100644 --- a/packages/cli/test/session/tool-guidance.test.ts +++ b/packages/cli/test/session/tool-guidance.test.ts @@ -76,6 +76,8 @@ const [ { Auth }, { HeadlessRun }, { tmpdir }, + { Agent }, + { default: nativeExplorePrompt }, ] = await Promise.all([ import("../../src/tool/tool"), import("zod"), @@ -90,6 +92,8 @@ const [ import("../../src/auth"), import("../../src/flag/headless-run"), import("../fixture/fixture"), + import("../../src/agent/agent"), + import("../../src/agent/prompt/explore.txt"), ]) type StreamInput = Parameters[0] @@ -134,6 +138,10 @@ async function fixture( session?: StreamInput["permission"] gpt?: boolean codex?: boolean + permissionDefault?: "native" + explorePrompt?: string + exploreTemperature?: number + batch?: boolean buildPrompt?: string call?: () => { name: string; arguments: Record } images?: boolean @@ -204,7 +212,8 @@ async function fixture( await using tmp = await tmpdir({ init: options.init, config: { - permission: { "*": "allow", ...options.permission }, + permission: { ...(options.permissionDefault === "native" ? {} : { "*": "allow" }), ...options.permission }, + ...(options.batch ? { experimental: { batch_tool: true } } : {}), enabled_providers: [providerID], provider: { [providerID]: { @@ -224,7 +233,23 @@ async function fixture( : {}), }, }, - ...(options.buildPrompt ? { agent: { build: { prompt: options.buildPrompt } } } : {}), + ...(options.buildPrompt || options.explorePrompt !== undefined || options.exploreTemperature !== undefined + ? { + agent: { + ...(options.buildPrompt ? { build: { prompt: options.buildPrompt } } : {}), + ...(options.explorePrompt !== undefined || options.exploreTemperature !== undefined + ? { + explore: { + ...(options.explorePrompt !== undefined ? { prompt: options.explorePrompt } : {}), + ...(options.exploreTemperature !== undefined + ? { temperature: options.exploreTemperature } + : {}), + }, + } + : {}), + }, + } + : {}), }, }) await Instance.provide({ @@ -290,7 +315,14 @@ async function fixture( await Bun.write( path.join(output, `${label}.json`), JSON.stringify( - { label, codex: options.codex ?? false, requests: transport.captures, persisted, persistedCheckpoints }, + { + label, + fixtureOptions: options, + codex: options.codex ?? false, + requests: transport.captures, + persisted, + persistedCheckpoints, + }, null, 2, ), @@ -756,3 +788,304 @@ describe("runtime hints through actual tool loop and next request (#244)", () => } }) }) + +const explorePeers = ["glob", "grep", "read", "bash"] +const exploreDenials = explorePeers.map((permission) => ({ permission, pattern: "*", action: "deny" as const })) +const nativeExploreFull = ["bash", "glob", "grep", "read", "webfetch"] +const explicitAgentParts: NonNullable = [ + { type: "text", text: "Inspect the fixtures using the requested agent." }, + { type: "agent", name: "explore" }, +] +function syntheticDelegation(messages: Awaited>) { + return messages + .flatMap((message) => message.parts) + .filter((part) => part.type === "text" && part.synthetic && part.text.includes("explore")) +} + +describe("remaining owned guidance completeness (#244)", () => { + test("native Explore full catalog preserves specialized peer guidance", async () => { + await fixture("completeness-explore-full", { permissionDefault: "native" }, async (context) => { + const body = await context.prompt({ agent: "explore" }) + expect(names(body)).toEqual(nativeExploreFull) + expect(system(body)).toContain("Use Glob for broad file pattern matching") + expect(system(body)).toContain("Use Grep for searching file contents") + expect(system(body)).toContain("Use Read when you know the specific file path") + expect(system(body)).toContain("Use Bash for file operations") + }) + }) + + test("native Explore session-denied peers are absent from both schema and owned prompt", async () => { + await fixture( + "completeness-explore-reduced", + { permissionDefault: "native", session: exploreDenials }, + async (context) => { + const body = await context.prompt({ agent: "explore" }) + expect(names(body)).toEqual(["webfetch"]) + expect(system(body)).not.toContain("Use Glob for broad file pattern matching") + expect(system(body)).not.toContain("Use Grep for searching file contents") + expect(system(body)).not.toContain("Use Read when you know the specific file path") + expect(system(body)).not.toContain("Use Bash for file operations") + }, + ) + }) + + test("custom Explore override stays verbatim with the same session-denied peers", async () => { + const prompt = + "CUSTOM Explore owner text: Use Glob for broad file pattern matching. Use Grep, Read and Bash literally; preserve this exact sentence." + await fixture( + "completeness-explore-custom", + { permissionDefault: "native", session: exploreDenials, explorePrompt: prompt }, + async (context) => { + const body = await context.prompt({ agent: "explore" }) + expect(names(body)).toEqual(["webfetch"]) + expect(system(body)).toStartWith(prompt) + expect(system(body)).not.toContain("You are a file search specialist.") + }, + ) + }) + + test("experimental Batch full catalog retains concrete examples", async () => { + await fixture("completeness-batch-full", { batch: true }, async (context) => { + const body = await context.prompt() + expect(names(body)).toEqual(["batch", ...full].sort()) + const text = description(body, "batch") + for (const peer of ["read", "grep", "bash"]) expect(text).toContain(`"tool": "${peer}"`) + expect(text).toContain("grep + glob + read combos") + }) + }) + + for (const absent of explorePeers) { + test(`experimental Batch session-denied ${absent} peer is absent from its owned recommendations`, async () => { + await fixture( + `completeness-batch-no-${absent}`, + { batch: true, session: [{ permission: absent, pattern: "*", action: "deny" }] }, + async (context) => { + const body = await context.prompt() + expect(names(body)).toEqual(["batch", ...full.filter((name) => name !== absent)].sort()) + const text = description(body, "batch") + if (absent === "glob") expect(text).not.toContain("grep + glob + read combos") + else expect(text).not.toContain(`"tool": "${absent}"`) + }, + ) + }) + } + + test("fresh explicit-agent part with Task present generates a persisted delegation instruction", async () => { + await fixture("completeness-agent-task-present", {}, async (context) => { + const body = await context.prompt({ parts: explicitAgentParts }) + expect(names(body)).toEqual(full) + const parts = syntheticDelegation(await context.messages()) + expect(parts).toHaveLength(1) + if (parts[0].type !== "text") throw new Error("Expected synthetic text") + expect(parts[0].text).toContain("subagent: explore") + expect(parts[0].text).toContain("delegation tool is available") + expect(parts[0].text).toContain("report that delegation is unavailable") + expect(JSON.stringify(body.messages)).toContain(parts[0].text) + expect(JSON.stringify(body.messages)).toContain("Inspect the fixtures using the requested agent.") + }) + }) + + test("fresh explicit-agent part does not prescribe Task removed by final session denial", async () => { + await fixture( + "completeness-agent-task-denied", + { session: [{ permission: "task", pattern: "*", action: "deny" }] }, + async (context) => { + const body = await context.prompt({ parts: explicitAgentParts }) + expect(names(body)).toEqual(full.filter((name) => name !== "task")) + const parts = syntheticDelegation(await context.messages()) + expect(parts).toHaveLength(1) + if (parts[0].type !== "text") throw new Error("Expected synthetic text") + expect(JSON.stringify(body.messages)).toContain(parts[0].text) + expect(parts[0].text).not.toContain("call the task tool") + expect(parts[0].text).toContain("delegation tool is available") + expect(parts[0].text).toContain("report that delegation is unavailable") + }, + ) + }) + + test("historical generated delegation instruction is replayed verbatim with Task denied", async () => { + await fixture( + "completeness-agent-history", + { session: [{ permission: "task", pattern: "*", action: "deny" }] }, + async (context) => { + await context.prompt({ parts: explicitAgentParts }) + const generated = syntheticDelegation(await context.messages())[0] + if (generated.type !== "text") throw new Error("Expected generated synthetic text") + const historical = { + ...generated, + text: " Use the above message and context to generate a prompt and call the task tool with subagent: explore", + } + await Session.updatePart(historical) + const body = await context.prompt({ + parts: [{ type: "text", text: "Continue, preserving the stored history." }], + }) + expect(names(body)).not.toContain("task") + expect(JSON.stringify(body.messages)).toContain(historical.text) + const replay = (await context.messages()) + .flatMap((message) => message.parts) + .find((part) => part.id === historical.id) + expect(replay).toEqual(historical) + }, + ) + }) +}) + +describe("remaining guidance provenance and lifetime controls (#244)", () => { + test("canonical-text custom Explore override remains verbatim after final session denial", async () => { + await fixture( + "completeness-explore-identical-override", + { permissionDefault: "native", session: exploreDenials, explorePrompt: nativeExplorePrompt }, + async (context) => { + const body = await context.prompt({ agent: "explore" }) + expect(names(body)).toEqual(["webfetch"]) + expect(system(body)).toStartWith(nativeExplorePrompt) + for (const peer of ["Glob", "Grep", "Read", "Bash"]) expect(system(body)).toContain(`Use ${peer}`) + }, + ) + }) + + test("parameter-only native Explore configuration retains owned conditional guidance", async () => { + await fixture( + "completeness-explore-parameters", + { permissionDefault: "native", session: exploreDenials, exploreTemperature: 0.17 }, + async (context) => { + const body = await context.prompt({ agent: "explore" }) + expect(names(body)).toEqual(["webfetch"]) + expect((await Agent.get("explore")).temperature).toBe(0.17) + expect(system(body)).toContain("You are a file search specialist") + for (const peer of ["Glob", "Grep", "Read", "Bash"]) expect(system(body)).not.toContain(`Use ${peer}`) + }, + ) + }) + + for (const absent of explorePeers) { + test(`native Explore removes only session-denied ${absent} peer advice`, async () => { + await fixture( + `completeness-explore-no-${absent}`, + { permissionDefault: "native", session: [{ permission: absent, pattern: "*", action: "deny" }] }, + async (context) => { + const body = await context.prompt({ agent: "explore" }) + expect(names(body)).toEqual(nativeExploreFull.filter((name) => name !== absent)) + const title = absent[0].toUpperCase() + absent.slice(1) + expect(system(body)).not.toContain(`Use ${title}`) + for (const other of explorePeers.filter((name) => name !== absent)) + expect(system(body)).toContain(`Use ${other[0].toUpperCase() + other.slice(1)}`) + }, + ) + }) + } + + test("native Explore cached source survives full reduced full request rendering", async () => { + await fixture("completeness-explore-full-reduced-full", { permissionDefault: "native" }, async (context) => { + const first = await context.prompt({ agent: "explore" }) + const input = context.input() + const cached = await Agent.get("explore") + expect(input.agent).toBe(cached) + const source = cached.prompt + expect(source).toBe(nativeExplorePrompt) + const middle = await context.direct({ ...input, permission: exploreDenials }) + expect(names(middle)).toEqual(["webfetch"]) + for (const peer of ["Glob", "Grep", "Read", "Bash"]) expect(system(middle)).not.toContain(`Use ${peer}`) + expect(cached.prompt).toBe(source) + expect(await Agent.get("explore")).toBe(cached) + const last = await context.direct({ ...input, permission: [] }) + expect(names(last)).toEqual(nativeExploreFull) + expect(system(last)).toBe(system(first)) + expect(cached.prompt).toBe(source) + }) + }) +}) + +describe("remaining guidance ownership boundaries (#244)", () => { + test("runtime-changed native Explore text is preserved by source-validity guard", async () => { + const changed = "Runtime-owned Explore prompt: Use Glob, Grep, Read and Bash literally. Preserve verbatim." + await fixture( + "completeness-explore-runtime-change", + { permissionDefault: "native", session: exploreDenials }, + async (context) => { + const cached = await Agent.get("explore") + const original = cached.prompt + cached.prompt = changed + try { + const body = await context.prompt({ agent: "explore" }) + expect(names(body)).toEqual(["webfetch"]) + expect(system(body)).toStartWith(changed) + expect(cached.prompt).toBe(changed) + } finally { + cached.prompt = original + } + }, + ) + }) + + test("Batch with every illustrated capability denied has no empty recommendation section", async () => { + const unavailable = ["read", "grep", "glob", "bash", "edit", "write"] + await fixture( + "completeness-batch-no-illustrated-peers", + { batch: true, session: unavailable.map((permission) => ({ permission, pattern: "*", action: "deny" })) }, + async (context) => { + const body = await context.prompt() + expect(names(body)).toEqual(["batch", "skill", "task", "webfetch"]) + const text = description(body, "batch") + expect(text).not.toContain("Good Use Cases:") + expect(text).not.toContain("Read many files") + expect(text).not.toContain("Multiple bash commands") + expect(text).not.toContain("Multi-part edits") + expect(text).not.toContain('"tool": "read"') + expect(text).not.toContain('"tool": "grep"') + expect(text).not.toContain('"tool": "bash"') + expect(text).toContain("tools available in this request") + expect(text).toContain("Partial failures do not stop other tool calls") + }, + ) + }) +}) + +describe("Batch actual dispatch description boundaries (#244)", () => { + test("GPT Batch with only apply_patch cannot recommend batched edits absent from inner dispatch", async () => { + const unavailable = ["bash", "glob", "grep", "read", "task", "webfetch", "skill"] + await fixture( + "completeness-batch-gpt-apply-patch-only", + { + gpt: true, + batch: true, + session: unavailable.map((permission) => ({ permission, pattern: "*", action: "deny" })), + }, + async (context) => { + const body = await context.prompt() + expect(names(body)).toEqual(["apply_patch", "batch"]) + const inner = (await ToolRegistry.tools({ modelID: "", providerID: "" })).map((tool) => tool.id) + expect(inner).not.toContain("apply_patch") + expect(inner).toContain("edit") + expect(inner).toContain("write") + expect(description(body, "batch")).not.toContain("Multi-part edits") + expect(description(body, "batch")).not.toContain("Good Use Cases:") + }, + ) + }) + + test("plugin-appended Batch description remains entirely plugin-owned verbatim", async () => { + const original = Plugin.trigger + let expected = "" + const spy = spyOn(Plugin, "trigger").mockImplementation((async (name, input, output) => { + await original(name, input, output) + if (name === "tool.definition" && (input as { toolID: string }).toolID === "batch") { + const definition = output as { description: string } + definition.description += "\nPlugin-owned Batch appendix: Use Read, Grep and Bash literally." + expected = definition.description + } + return output + }) as typeof Plugin.trigger) + try { + await fixture("completeness-batch-plugin-owned", { batch: true, session: exploreDenials }, async (context) => { + const body = await context.prompt() + expect(names(body)).toEqual(["batch", "edit", "skill", "task", "webfetch", "write"]) + expect(description(body, "batch")).toBe(expected) + expect(description(body, "batch")).toContain('"tool": "read"') + expect(description(body, "batch")).toContain("Plugin-owned Batch appendix") + }) + } finally { + spy.mockRestore() + } + }) +}) From 273c73c15b17eb69967c3a00f771e13a4dd9d172 Mon Sep 17 00:00:00 2001 From: Bulat Yapparov Date: Wed, 21 Oct 2026 12:13:23 +0100 Subject: [PATCH 5/6] test(session): verify tool guidance through session behavior --- .../cli/test/session/tool-guidance.test.ts | 143 +++++++++++++++--- 1 file changed, 121 insertions(+), 22 deletions(-) diff --git a/packages/cli/test/session/tool-guidance.test.ts b/packages/cli/test/session/tool-guidance.test.ts index 1c771d32..1644a658 100644 --- a/packages/cli/test/session/tool-guidance.test.ts +++ b/packages/cli/test/session/tool-guidance.test.ts @@ -208,6 +208,16 @@ async function fixture( last = input return original(input) }) + let primaryFailure: { error: unknown } | undefined + const cleanupErrors: Error[] = [] + const diagnosticErrors: Error[] = [] + async function attempt(errors: Error[], operation: string, fn: () => unknown | Promise) { + try { + await fn() + } catch (cause) { + errors.push(new Error(operation, { cause })) + } + } try { await using tmp = await tmpdir({ init: options.init, @@ -305,37 +315,59 @@ async function fixture( try { await fn(context) } finally { - persisted = await context.messages() + await attempt(diagnosticErrors, "Failed to capture persisted fixture messages", async () => { + persisted = await context.messages() + }) } }, }) + } catch (error) { + primaryFailure = { error } } finally { + await attempt(cleanupErrors, "Failed to restore LLM.stream", () => spy.mockRestore()) + await attempt(cleanupErrors, "Failed to stop fixture server", () => server.stop(true)) + await attempt(cleanupErrors, "Failed to reset fixture transport", () => { + transport.origin = "" + transport.codex = false + }) + if (options.codex) { + await attempt(cleanupErrors, "Failed to restore fixture auth", async () => { + if (previousAuth) await Auth.set("openai", previousAuth) + else await Auth.remove("openai") + }) + } const output = process.env.GUIDANCE_CAPTURE_DIR if (output) - await Bun.write( - path.join(output, `${label}.json`), - JSON.stringify( - { - label, - fixtureOptions: options, - codex: options.codex ?? false, - requests: transport.captures, - persisted, - persistedCheckpoints, - }, - null, - 2, + await attempt(diagnosticErrors, "Failed to write fixture capture", () => + Bun.write( + path.join(output, `${label}.json`), + JSON.stringify( + { + label, + fixtureOptions: options, + codex: options.codex ?? false, + requests: transport.captures, + persisted, + persistedCheckpoints, + }, + null, + 2, + ), ), ) - spy.mockRestore() - server.stop(true) - transport.origin = "" - transport.codex = false - if (options.codex) { - if (previousAuth) await Auth.set("openai", previousAuth) - else await Auth.remove("openai") - } } + if (cleanupErrors.length || diagnosticErrors.length) { + throw new AggregateError( + [ + ...(primaryFailure ? [primaryFailure.error] : []), + ...(cleanupErrors.length ? [new AggregateError(cleanupErrors, "Fixture cleanup failed")] : []), + ...(diagnosticErrors.length ? [new AggregateError(diagnosticErrors, "Fixture diagnostics failed")] : []), + ], + `Tool guidance fixture ${label} failed`, + primaryFailure ? { cause: primaryFailure.error } : undefined, + ) + } + if (primaryFailure) throw primaryFailure.error } beforeAll(() => HeadlessRun.enable()) @@ -344,6 +376,73 @@ afterAll(() => { globalThis.fetch = originalFetch }) +describe("guidance fixture teardown", () => { + for (const callbackFails of [false, true]) { + test(`capture failure restores fixture resources${callbackFails ? " and preserves the callback error" : ""}`, async () => { + await using tmp = await tmpdir() + const output = path.join(tmp.path, "capture-path-conflict") + await Bun.write(output, "owned regular file, not a capture directory") + const previousCapture = process.env.GUIDANCE_CAPTURE_DIR + const previousStream = LLM.stream + const previousAuth = await Auth.get("openai") + const primary = new Error("Owned callback failure") + let origin = "" + let failure: unknown + process.env.GUIDANCE_CAPTURE_DIR = output + try { + await fixture("teardown-capture-conflict", { codex: true }, async () => { + origin = transport.origin + if (callbackFails) throw primary + }) + } catch (error) { + failure = error + } finally { + if (previousCapture === undefined) delete process.env.GUIDANCE_CAPTURE_DIR + else process.env.GUIDANCE_CAPTURE_DIR = previousCapture + } + expect(failure).toBeInstanceOf(AggregateError) + const aggregate = failure as AggregateError + if (callbackFails) { + expect(aggregate.cause).toBe(primary) + expect(aggregate.errors[0]).toBe(primary) + } else expect(aggregate.cause).toBeUndefined() + const diagnostic = aggregate.errors.find( + (error) => error instanceof AggregateError && error.message === "Fixture diagnostics failed", + ) as AggregateError + expect(diagnostic).toBeInstanceOf(AggregateError) + expect(diagnostic.errors.some((error) => error.message === "Failed to write fixture capture")).toBe(true) + expect(LLM.stream).toBe(previousStream) + expect(transport.origin).toBe("") + expect(transport.codex).toBe(false) + expect(await Auth.get("openai")).toEqual(previousAuth) + expect(origin).toStartWith("http://127.0.0.1:") + await expect(originalFetch(origin)).rejects.toThrow() + }) + } + + test("successful capture is written after fixture resources are restored", async () => { + await using tmp = await tmpdir() + const previousCapture = process.env.GUIDANCE_CAPTURE_DIR + const previousStream = LLM.stream + const previousAuth = await Auth.get("openai") + process.env.GUIDANCE_CAPTURE_DIR = tmp.path + try { + await fixture("teardown-capture-success", { codex: true }, async () => {}) + const capture = await Bun.file(path.join(tmp.path, "teardown-capture-success.json")).json() + expect(capture.label).toBe("teardown-capture-success") + expect(capture.codex).toBe(true) + expect(capture.requests).toEqual([]) + expect(LLM.stream).toBe(previousStream) + expect(transport.origin).toBe("") + expect(transport.codex).toBe(false) + expect(await Auth.get("openai")).toEqual(previousAuth) + } finally { + if (previousCapture === undefined) delete process.env.GUIDANCE_CAPTURE_DIR + else process.env.GUIDANCE_CAPTURE_DIR = previousCapture + } + }) +}) + describe("actual request tool guidance (#244)", () => { test("full built-in catalog retains useful peer guidance", async () => { await fixture("full", {}, async (context) => { From d623347b6a712f9b3def96131f93cd4b37f3fc33 Mon Sep 17 00:00:00 2001 From: Bulat Yapparov Date: Wed, 21 Oct 2026 12:51:55 +0100 Subject: [PATCH 6/6] test: clean up failed tool guidance fixture setup --- .../cli/test/session/tool-guidance.test.ts | 126 ++++++++++-------- 1 file changed, 68 insertions(+), 58 deletions(-) diff --git a/packages/cli/test/session/tool-guidance.test.ts b/packages/cli/test/session/tool-guidance.test.ts index 1644a658..ea284b45 100644 --- a/packages/cli/test/session/tool-guidance.test.ts +++ b/packages/cli/test/session/tool-guidance.test.ts @@ -1,4 +1,4 @@ -import { afterAll, beforeAll, describe, expect, spyOn, test } from "bun:test" +import { afterAll, beforeAll, describe, expect, spyOn, test, type Mock } from "bun:test" import path from "node:path" // Install before runtime imports: every model request is local or an intercepted @@ -149,65 +149,16 @@ async function fixture( } = {}, fn: (context: Fixture) => Promise, ) { - transport.captures = [] - transport.codex = options.codex ?? false const providerID = options.gpt || options.codex ? "openai" : "fixture" const modelID = providerID === "openai" ? "gpt-5.2" : "fixture" - const server = Bun.serve({ - hostname: "127.0.0.1", - port: 0, - async fetch(request) { - transport.captures.push((await request.json()) as Wire) - if (options.call && transport.captures.length === 1) { - const call = options.call() - return sse([ - { - id: "fixture-call", - object: "chat.completion.chunk", - choices: [ - { - index: 0, - delta: { - role: "assistant", - tool_calls: [ - { - index: 0, - id: "runtime-call", - type: "function", - function: { name: call.name, arguments: JSON.stringify(call.arguments) }, - }, - ], - }, - }, - ], - }, - { - id: "fixture-call", - object: "chat.completion.chunk", - choices: [{ index: 0, delta: {}, finish_reason: "tool_calls" }], - }, - ]) - } - return providerID === "openai" ? responses() : chat() - }, - }) - transport.origin = server.url.origin - const previousAuth = options.codex ? await Auth.get("openai") : undefined - if (options.codex) - await Auth.set("openai", { - type: "oauth", - access: "fixture-access", - refresh: "fixture-refresh", - expires: Date.now() + 60_000, - }) + let server: Bun.Server | undefined + let previousAuth: { value: Awaited> } | undefined + let authSwapAttempted = false const original = LLM.stream let last: StreamInput | undefined let persisted: unknown const persistedCheckpoints: unknown[] = [] - const spy = spyOn(LLM, "stream").mockImplementation(async (input) => { - last = input - return original(input) - }) + let spy: Mock | undefined let primaryFailure: { error: unknown } | undefined const cleanupErrors: Error[] = [] const diagnosticErrors: Error[] = [] @@ -219,6 +170,62 @@ async function fixture( } } try { + transport.captures = [] + transport.codex = options.codex ?? false + server = Bun.serve({ + hostname: "127.0.0.1", + port: 0, + async fetch(request) { + transport.captures.push((await request.json()) as Wire) + if (options.call && transport.captures.length === 1) { + const call = options.call() + return sse([ + { + id: "fixture-call", + object: "chat.completion.chunk", + choices: [ + { + index: 0, + delta: { + role: "assistant", + tool_calls: [ + { + index: 0, + id: "runtime-call", + type: "function", + function: { name: call.name, arguments: JSON.stringify(call.arguments) }, + }, + ], + }, + }, + ], + }, + { + id: "fixture-call", + object: "chat.completion.chunk", + choices: [{ index: 0, delta: {}, finish_reason: "tool_calls" }], + }, + ]) + } + return providerID === "openai" ? responses() : chat() + }, + }) + transport.origin = server.url.origin + if (options.codex) { + previousAuth = { value: await Auth.get("openai") } + authSwapAttempted = true + await Auth.set("openai", { + type: "oauth", + access: "fixture-access", + refresh: "fixture-refresh", + expires: Date.now() + 60_000, + }) + } + spy = spyOn(LLM, "stream") + spy.mockImplementation(async (input) => { + last = input + return original(input) + }) await using tmp = await tmpdir({ init: options.init, config: { @@ -324,15 +331,18 @@ async function fixture( } catch (error) { primaryFailure = { error } } finally { - await attempt(cleanupErrors, "Failed to restore LLM.stream", () => spy.mockRestore()) - await attempt(cleanupErrors, "Failed to stop fixture server", () => server.stop(true)) + const acquiredSpy = spy + if (acquiredSpy) await attempt(cleanupErrors, "Failed to restore LLM.stream", () => acquiredSpy.mockRestore()) + const acquiredServer = server + if (acquiredServer) await attempt(cleanupErrors, "Failed to stop fixture server", () => acquiredServer.stop(true)) await attempt(cleanupErrors, "Failed to reset fixture transport", () => { transport.origin = "" transport.codex = false }) - if (options.codex) { + if (authSwapAttempted && previousAuth) { + const snapshot = previousAuth.value await attempt(cleanupErrors, "Failed to restore fixture auth", async () => { - if (previousAuth) await Auth.set("openai", previousAuth) + if (snapshot) await Auth.set("openai", snapshot) else await Auth.remove("openai") }) }