diff --git a/packages/cli/src/commands/chat.ts b/packages/cli/src/commands/chat.ts index cae59a3..adbc01c 100644 --- a/packages/cli/src/commands/chat.ts +++ b/packages/cli/src/commands/chat.ts @@ -26,9 +26,9 @@ */ import { createInterface, type Interface } from "node:readline"; import type { Command } from "commander"; -import { createAgent, userText } from "@prismshadow/penguin-core"; +import { createAgent, userText, VERSION } from "@prismshadow/penguin-core"; import type { ApprovalDecision, OmniMessage, ToolCallPayload } from "@prismshadow/penguin-core"; -import { StreamRenderer, dim, renderHistory } from "../render.js"; +import { StreamRenderer, dim, renderHistory, sessionMetaTools } from "../render.js"; import { runTask } from "../task-loop.js"; import { parseApprovalAnswer, resolveApprovalMode } from "../approval.js"; import { LineComposer, PasteFilter } from "../input.js"; @@ -113,9 +113,11 @@ export function registerChatCommand(program: Command, t: Messages): void { } const renderer = new StreamRenderer(out, t); + // The assembled tool schemas decide each tool's call-line preview path (see render.ts). + renderer.useToolSchemas(sessionMetaTools(session)); out.write( - `${t.header("chat", agent.state.agentId, session.workspaceDir, session.modelId)}\n` + + `${t.header("chat", VERSION, agent.state.agentId, session.workspaceDir, session.modelId)}\n` + `${t.chatHints()}\n`, ); // On resume, first render the history messages of the current context per Trace diff --git a/packages/cli/src/commands/run.ts b/packages/cli/src/commands/run.ts index 5617aee..c3eecd7 100644 --- a/packages/cli/src/commands/run.ts +++ b/packages/cli/src/commands/run.ts @@ -13,8 +13,8 @@ * Docs: /docs/cli § "penguin run". */ import type { Command } from "commander"; -import { createAgent, userText } from "@prismshadow/penguin-core"; -import { StreamRenderer } from "../render.js"; +import { createAgent, userText, VERSION } from "@prismshadow/penguin-core"; +import { StreamRenderer, sessionMetaTools } from "../render.js"; import { runTask } from "../task-loop.js"; import { denyActivePrompt, resolveApprovalMode } from "../approval.js"; import type { Messages } from "../i18n.js"; @@ -53,7 +53,9 @@ export function registerRunCommand(program: Command, t: Messages): void { }); const out = process.stdout; - out.write(`${t.header("run", agent.state.agentId, session.workspaceDir, session.modelId)}\n`); + out.write( + `${t.header("run", VERSION, agent.state.agentId, session.workspaceDir, session.modelId)}\n`, + ); const controller = new AbortController(); const onSigint = () => { @@ -65,6 +67,8 @@ export function registerRunCommand(program: Command, t: Messages): void { process.on("SIGINT", onSigint); const renderer = new StreamRenderer(out, t); + // The assembled tool schemas decide each tool's call-line preview path (see render.ts). + renderer.useToolSchemas(sessionMetaTools(session)); try { const result = await runTask(session, [userText(opts.message)], { mode, diff --git a/packages/cli/src/i18n.ts b/packages/cli/src/i18n.ts index fce84f4..4187be4 100644 --- a/packages/cli/src/i18n.ts +++ b/packages/cli/src/i18n.ts @@ -114,7 +114,14 @@ export interface Messages { }; // —— Runtime output —— - header(kind: "chat" | "run", agentId: string, workspace: string, model: string): string; + /** Startup banner: product + subcommand + CLI version on the first line, then Agent / Workspace / Model each on its own line. */ + header( + kind: "chat" | "run", + version: string, + agentId: string, + workspace: string, + model: string, + ): string; chatHints(): string; confirmExit(): string; taskInterrupted(): string; @@ -187,8 +194,34 @@ export interface Messages { webTimeout(url: string): string; } -function header(kind: "chat" | "run", agentId: string, workspace: string, model: string): string { - return `PenguinHarness ${kind} — agent=${agentId} workspace=${workspace} model=${model}`; +function headerEn( + kind: "chat" | "run", + version: string, + agentId: string, + workspace: string, + model: string, +): string { + return [ + `PenguinHarness ${kind} v${version}`, + `Agent: ${agentId}`, + `Workspace: ${workspace}`, + `Model: ${model}`, + ].join("\n"); +} + +function headerZh( + kind: "chat" | "run", + version: string, + agentId: string, + workspace: string, + model: string, +): string { + return [ + `PenguinHarness ${kind} v${version}`, + `Agent：${agentId}`, + `Workspace：${workspace}`, + `模型：${model}`, + ].join("\n"); } const en: Messages = { @@ -308,7 +341,7 @@ const en: Messages = { installerFetchFailed: (url) => `Could not download the installer from ${url}.`, }, - header, + header: headerEn, chatHints: () => "Type a message to start a conversation; end a line with \\; /compact to compact the context; /exit to quit; and Ctrl-C interrupts the current conversation.", confirmExit: () => "Exit penguin? [y/N] ", @@ -473,7 +506,7 @@ const zh: Messages = { installerFetchFailed: (url) => `无法从 ${url} 下载安装脚本。`, }, - header, + header: headerZh, chatHints: () => "输入消息发起对话；行尾 \\ 续行；/compact 压缩上下文；/exit 退出；Ctrl-C 中断对话。", confirmExit: () => "确认退出 penguin？[y/N] ", diff --git a/packages/cli/src/render.ts b/packages/cli/src/render.ts index b8b06b8..3996676 100644 --- a/packages/cli/src/render.ts +++ b/packages/cli/src/render.ts @@ -21,13 +21,15 @@ * - when the head of the queue is held, the holder's own subsequent messages are let * through first (preserving in-segment order), avoiding deadlock. * - * **Pairing tags**: a tool call and its output may be separated by several segments, so - * both are tagged with a shared word for pairing: the call line reads - * `[tool-653] $ cmd`, the output line `[tool-653] >> ...` (653 being the last 3 - * characters of tool_call_id); nested (subagent) tools use - * `[agent-f2a-tool-653] $ cmd` (f2a being the last 3 characters of the direct child - * Session id). Approval lines carry no tag (they immediately follow the matching call - * line, so context makes the pairing clear): `[approved]`. + * **Call/output pairing**: both sides carry the same `[tool-653] ` prefix + * (653 being the last 3 characters of tool_call_id) — the call line reads + * `[tool-653] exec_command <- $ cmd`, each output line `[tool-653] exec_command -> ...`; + * nested (subagent) tools use `[agent-f2a-tool-653] …` (f2a being the last 3 characters + * of the direct child Session id). The output-side tool name is resolved from the + * preceding call via a tool_call_id → name map (the call always precedes its output; if + * no call was seen, the bare `[tool-653]` tag remains). Approval lines carry no tag + * (they immediately follow the matching call line, so context makes the pairing clear): + * `[approved]`. * * **Nested sub-session messages** (those carrying an origin) are handled separately: * child tool calls (so the user can see what the subagent is calling before approval) @@ -52,14 +54,18 @@ import type { PartialToolCallPayload, PartialToolCallOutputPayload, RequestEndPayload, + SessionMetaPayload, TokenUsagePayload, ToolCallPayload, + ToolDefinition, } from "@prismshadow/penguin-core"; -import { renderPartialToolCall } from "./tool-render.js"; +import { renderFileToolApprovalPayload, renderPartialToolCall } from "./tool-render.js"; import { defaultMessages } from "./i18n.js"; import type { Messages } from "./i18n.js"; const DIM = "\x1b[2m"; +const GREEN = "\x1b[32m"; +const RED = "\x1b[31m"; const CYAN = "\x1b[36m"; const RESET = "\x1b[0m"; @@ -67,6 +73,17 @@ export function dim(text: string): string { return `${DIM}${text}${RESET}`; } +/** The two file tools whose outputs carry git-style diffs; their `+`/`-`/`@@` lines get colored. */ +const DIFF_OUTPUT_TOOLS = new Set(["edit_file", "write_file"]); + +/** Color for one diff-output line, picked from its first character (null = plain). */ +function diffLineColor(firstChar: string | undefined): string | null { + if (firstChar === "+") return GREEN; + if (firstChar === "-") return RED; + if (firstChar === "@") return DIM; + return null; +} + /** Colors a tool call line cyan, distinguishing it from body text/thinking (review comment #5). */ function cyan(text: string): string { return `${CYAN}${text}${RESET}`; @@ -123,6 +140,15 @@ export function formatAbort(p: AbortPayload, t: Messages): string { return dim(t.abortLabel(p.reason ?? undefined)); } +/** + * The Session's assembled tool schemas, read off its `session_meta` — the definitions + * actually exposed to the model, so the per-tool `call_description` switch is already + * applied. Feeds `StreamRenderer.useToolSchemas`. + */ +export function sessionMetaTools(session: { metaMessage: OmniMessage }): readonly ToolDefinition[] { + return (session.metaMessage.payload as SessionMetaPayload).tools ?? []; +} + /** * Statically renders resumed history messages (`--resume`: full-message semantics, no * partial_*, including interrupted messages and their markers). Uses the @@ -135,6 +161,10 @@ export function renderHistory( out: NodeJS.WritableStream, t: Messages = defaultMessages(), ): void { + // tool_call_id -> tool name (keyed with the origin chain: parent/child ids may collide), + // so output lines can be labeled with the tool name of their preceding call. + const toolNames = new Map(); + const nameKey = (msg: OmniMessage, id: string): string => `${msg.origin?.join("/") ?? ""}:${id}`; for (const msg of messages) { if (isEventMessage(msg)) { const p = msg.payload as { type?: string } & AbortPayload; @@ -167,19 +197,31 @@ export function renderHistory( out.write(`${dim(p.thinking ?? "")}${marker}\n`); break; case "tool_call": { + if (p.name) toolNames.set(nameKey(msg, p.tool_call_id ?? ""), p.name); const preview = - renderPartialToolCall(p.name ?? "", p.arguments ?? "") ?? `${p.name} ${p.arguments}`; + renderPartialToolCall(p.name ?? "", p.arguments ?? "", { final: true }) ?? + `${p.name} ${p.arguments}`; out.write(`${cyan(`[${callTag(p.tool_call_id ?? "")}] ${preview}`)}${marker}\n`); break; } case "tool_call_output": { - const tag = callTag(p.tool_call_id ?? ""); + // Output lines carry the pairing tag plus the tool name (the bare tag when the + // transcript has no matching call); file-tool diff lines are colored like git's. + const tag = `[${callTag(p.tool_call_id ?? "")}]`; + const name = toolNames.get(nameKey(msg, p.tool_call_id ?? "")); + const label = name ? `${tag} ${name}` : tag; + const colorDiff = name !== undefined && DIFF_OUTPUT_TOOLS.has(name); for (const line of (p.output ?? "").split("\n")) { - out.write(`${DIM}[${tag}] >> ${RESET}${line}\n`); + const color = colorDiff ? diffLineColor(line[0]) : null; + out.write( + color + ? `${DIM}${label} -> ${RESET}${color}${line}${RESET}\n` + : `${DIM}${label} -> ${RESET}${line}\n`, + ); } // Attached images aren't rendered by the terminal; print one placeholder line per image. for (const _ of p.images ?? []) { - out.write(`${DIM}[${tag}] >> [image]${RESET}\n`); + out.write(`${DIM}${label} -> [image]${RESET}\n`); } break; } @@ -238,6 +280,22 @@ export class StreamRenderer { private inDim = false; /** Whether tool-call output is at the start of a line (decides whether the gutter needs to be written). */ private toolOutLineStart = true; + + /** + * tool_call_id -> tool name for the current task's parent-session calls (nested tool + * outputs are not gutter-rendered), so output lines can be prefixed with the tool name + * of the call that produced them. Populated from partial/complete call messages (the + * call always precedes its output); cleared with the other per-task registrations. + */ + private toolNames = new Map(); + /** + * Names of the tools whose assembled schema carries the `description` argument (from + * `session_meta.tools`, i.e. after the per-tool `call_description` switch has been + * applied). Decides the preview path before a call's arguments stream: awaiting the + * description, or streaming the plain form right away (see tool-render.ts). An unknown + * tool falls back to "no description". + */ + private describedTools = new Set(); /** Buffer for partial_tool_call; each delta streams out the newly appended suffix of the preview. */ private partialToolCalls = new Map< string, @@ -318,6 +376,21 @@ export class StreamRenderer { beginUserPrompt(toolCall?: OmniMessage): void { if (toolCall) this.ensureAdjacentCallLine(toolCall); this.finishLine(); + // File-tool approvals: the one-line preview shows only the (shortened) path, but the + // user is approving a concrete rewrite — print the decoded payload + // (old_string/new_string/content), bounded with an explicit elision note, right before + // the prompt. + if (toolCall) { + const payload = renderFileToolApprovalPayload( + toolCall.payload.name, + toolCall.payload.arguments, + ); + if (payload !== null) { + for (const line of payload.split("\n")) this.out.write(`${dim(line)}\n`); + // lastLineKey stays on the call's key: the payload lines belong to this call, so + // the later noteApprovalDecision must not re-render the call line as "not adjacent". + } + } this.promptActive = true; this.promptKey = toolCall ? this.callLineKey(toolCall.payload.tool_call_id, toolCall.origin) @@ -384,7 +457,11 @@ export class StreamRenderer { key: string, ): void { this.ensuredCallLines.add(key); - const preview = renderPartialToolCall(p.name, p.arguments) ?? `${p.name} ${p.arguments}`; + // Parent-session calls feed the output-gutter name map (nested outputs are not + // gutter-rendered, and a child id could collide with a parent id). + if (!origin || origin.length === 0) this.toolNames.set(p.tool_call_id, p.name); + const preview = + renderPartialToolCall(p.name, p.arguments, { final: true }) ?? `${p.name} ${p.arguments}`; this.finishLine(); this.out.write(`${cyan(`[${callTag(p.tool_call_id, origin)}] ${preview}`)}\n`); this.lastLineKey = key; @@ -500,7 +577,14 @@ export class StreamRenderer { case "partial_tool_call_output": this.handlePartialToolOutput(payload as PartialToolCallOutputPayload); return; - // Complete (non-streaming) model_msg is never rendered (including image_url/inline_*); the content has already been shown by partial_*. + // Complete (non-streaming) model_msg is never rendered (including image_url/inline_*); + // the content has already been shown by partial_*. A complete tool_call still feeds + // the name map so its output gutter can carry the tool name. + case "tool_call": { + const p = payload as ToolCallPayload; + this.toolNames.set(p.tool_call_id, p.name); + return; + } default: return; } @@ -603,7 +687,26 @@ export class StreamRenderer { } return; } - // session_meta: not rendered. + // session_meta: not rendered, but its tool list settles each tool's preview path. + if (msg.type === "session_meta") { + this.useToolSchemas((msg.payload as SessionMetaPayload).tools); + } + } + + /** + * Registers the Session's assembled tool schemas (`session_meta.tools`), which decide each + * tool's preview path before its arguments stream (see `describedTools`). The host calls + * this as soon as the Session exists; a `session_meta` flowing through the stream (resume, + * sub-sessions) registers the same way. + */ + useToolSchemas(tools: readonly ToolDefinition[]): void { + for (const tool of tools) { + const properties = (tool.parameters as { properties?: Record } | undefined) + ?.properties; + if (properties && Object.hasOwn(properties, "description")) + this.describedTools.add(tool.name); + else this.describedTools.delete(tool.name); + } } /** @@ -677,6 +780,25 @@ export class StreamRenderer { } } + /** + * Renders a call line whose preview was withheld for the whole stream (the arguments never + * settled — e.g. the turn was interrupted mid-arguments — so a description could still have + * arrived, see tool-render.ts). Called once at `stop` with the fragment marked final, so an + * in-flight call is never left invisible. + */ + private renderWithheldCallLine( + toolCallId: string, + partial: { name: string; arguments: string }, + ): void { + const key = this.callLineKey(toolCallId); + if (this.ensuredCallLines.has(key)) return; + const preview = renderPartialToolCall(partial.name, partial.arguments, { final: true }); + if (preview === null) return; + this.finishLine(); + this.out.write(`${cyan(`[${callTag(toolCallId)}] ${preview}`)}\n`); + this.lastLineKey = key; + } + private handlePartialToolCall(p: PartialToolCallPayload): void { // The call line was already rendered in place from the complete message at approval time: skip the whole late-arriving streaming copy (clean up the buffer on stop). if (this.ensuredCallLines.has(this.callLineKey(p.tool_call_id))) { @@ -689,13 +811,18 @@ export class StreamRenderer { partial = { name: p.name, arguments: "", lastPreview: "" }; this.partialToolCalls.set(p.tool_call_id, partial); } - if (p.name) partial.name = p.name; + if (p.name) { + partial.name = p.name; + // Remember the name for this call's output gutter (` -> …`). + this.toolNames.set(p.tool_call_id, p.name); + } if (p.arguments) { partial.arguments += p.arguments; } if (p.event_type === "stop") { if (partial.lastPreview) this.finishLine(); + else this.renderWithheldCallLine(p.tool_call_id, partial); this.partialToolCalls.delete(p.tool_call_id); return; } @@ -703,7 +830,9 @@ export class StreamRenderer { if (!p.arguments) return; if (this.inDim) this.finishLine(); - const preview = renderPartialToolCall(partial.name, partial.arguments); + const preview = renderPartialToolCall(partial.name, partial.arguments, { + expectDescription: this.describedTools.has(partial.name), + }); if (preview === null) return; // The line starts with a pairing tag [tool-], matching the output line that follows. @@ -731,40 +860,59 @@ export class StreamRenderer { return; } if (this.inDim) this.finishLine(); - if (p.output) this.writeToolOutput(p.output, callTag(p.tool_call_id)); + const label = this.outputLabel(p.tool_call_id); + const name = this.toolNames.get(p.tool_call_id); + const colorDiff = name !== undefined && DIFF_OUTPUT_TOOLS.has(name); + if (p.output) this.writeToolOutput(p.output, label, colorDiff); // Image delta (carried whole in a single delta): the terminal doesn't render the - // image itself, so print one placeholder line per image, using the same pairing tag - // as the output gutter. + // image itself, so print one placeholder line per image, using the same gutter label + // as the text output. if (p.images && p.images.length > 0) { this.finishLine(); - const tag = callTag(p.tool_call_id); for (const _ of p.images) { - this.out.write(`${DIM}[${tag}] >> [image]${RESET}\n`); + this.out.write(`${DIM}${label} -> [image]${RESET}\n`); } this.lastLineKey = null; } } + /** Output-gutter label: the `[tool-xxx]` pairing tag plus the tool name of the preceding call (the bare tag when no call was seen). */ + private outputLabel(toolCallId: string): string { + const tag = `[${callTag(toolCallId)}]`; + const name = this.toolNames.get(toolCallId); + return name ? `${tag} ${name}` : tag; + } + /** * Writes tool-call **output** line by line, each line starting with the dim gutter - * `[tool-] >> `, paired with the call line (cyan `[tool-xxx] $ - * cmd`). Streaming chunks arrive incrementally; whether to write the gutter is - * decided by the current line-start state. + * `[tool-xxx] -> ` (the same prefix as the call line, cyan + * `[tool-xxx] <- …`; the bare tag when no call was seen). Streaming chunks + * arrive incrementally; whether to write the gutter is decided by the current + * line-start state. */ - private writeToolOutput(chunk: string, tag: string): void { + private writeToolOutput(chunk: string, label: string, colorDiff: boolean): void { let i = 0; while (i < chunk.length) { + let lineColor: string | null = null; if (this.toolOutLineStart) { - this.out.write(`${DIM}[${tag}] >> ${RESET}`); + this.out.write(`${DIM}${label} -> ${RESET}`); this.toolOutLineStart = false; this.inLine = true; + // Diff coloring keys off the line's first character. File-tool outputs arrive as + // one delta of whole lines, so the first character is always in this chunk; a + // line continued from a previous chunk stays plain. + if (colorDiff) lineColor = diffLineColor(chunk[i]); } const nl = chunk.indexOf("\n", i); + const end = nl === -1 ? chunk.length : nl; + const segment = chunk.slice(i, end); + if (segment) { + this.out.write(lineColor ? `${lineColor}${segment}${RESET}` : segment); + } if (nl === -1) { - this.out.write(chunk.slice(i)); i = chunk.length; } else { - this.out.write(chunk.slice(i, nl + 1)); + this.out.write("\n"); this.toolOutLineStart = true; this.inLine = false; i = nl + 1; @@ -833,6 +981,7 @@ export class StreamRenderer { this.ensuredCallLines.clear(); this.renderedDecisions.clear(); this.partialToolCalls.clear(); + this.toolNames.clear(); } /** diff --git a/packages/cli/src/tool-render.ts b/packages/cli/src/tool-render.ts index ad4ca76..6a3a9cb 100644 --- a/packages/cli/src/tool-render.ts +++ b/packages/cli/src/tool-render.ts @@ -1,22 +1,52 @@ /** * Streaming tool-call rendering (CLI side). * - * The CLI only consumes `partial_tool_call` for visible rendering. exec_command is shown as - * `$ ` as early as possible; input_command / input_subagent show the target session id, - * with a non-empty payload (chars / prompt) appended as `<< ` — the payload is - * critical for approval and later audit (writing to stdin is equivalent to running a command), - * so the session id alone is not enough; run_subagent shows the prompt; other tools fall back - * to `name(args-prefix)`. + * The CLI only consumes `partial_tool_call` for visible rendering. Formats (the `<-` marker + * reads "input to the tool", paired with the `->` output gutter in render.ts): + * - exec_command: `exec_command <- $ {cmd}`, or with a model-written description + * `exec_command <- {description} ($ {cmd})`; + * - input_command: `input_command <- {process_id} << {chars}` (`<< …` only when writing; + * an empty payload just polls), or `input_command <- {description} ({process_id} << {chars})`; + * - run_subagent: `run_subagent <- {prompt}` or `run_subagent <- {description} ({prompt})`; + * - input_subagent: `input_subagent <- {subagent_id} << {prompt}` or + * `input_subagent <- {description} ({subagent_id} << {prompt})`; + * - file tools (read_file / edit_file / write_file): `{name} {shortened path}` — the path is + * shortened to at most one parent directory (`…/parent/file.ts`); the full path stays in + * the arguments. + * The payload (chars / prompt) is critical for approval and later audit (writing to stdin + * is equivalent to running a command), so the session id alone is never enough; other tools + * fall back to `name(args-prefix)`. * - * The render layer streams by appending to the preview (see render.ts), so the preview format - * must stay append-only: rendering only starts once the target id has fully appeared, the - * payload is only appended at the end, and the preview stops growing once it hits the - * truncation limit. + * The render layer streams by appending to the preview (see render.ts), so the preview must + * stay append-only — a preview that is not an extension of the previous one costs a fresh + * line, leaving the superseded one on screen. Which of the two forms a call will take is + * therefore decided **before** its arguments stream, from the tool's assembled schema + * (`expectDescription`, taken from `session_meta.tools` — the per-tool `call_description` + * switch decides whether the argument exists at all; see the docs on tool configuration): + * - schema without the argument (and the unknown case) → the plain form streams immediately, + * character by character, and any stray `description` is ignored; + * - schema with it → the description is awaited: it streams live as it grows + * (`name <- desc…`) and the payload is appended once its value completes + * (`name <- desc ({payload…` → `)`), so the plain form never reaches the screen whichever + * order the model emits its arguments in. The wait is bounded: the argument is declared + * **required**, so a schema-abiding model always sends one, and it is asked to send it + * first. + * A `final` fragment (stream ended, or arguments from a complete message) is settled by + * definition and renders whichever form the arguments actually carry — which is also what + * catches a model that violates the schema and omits the required argument. + * File-tool paths render only once complete — shortening a still-growing path would rewrite + * the line. */ -/** Max length of the single-line preview for a payload (chars / prompt); truncated with an ellipsis beyond this, after which the preview stops growing. */ +/** Max length of the single-line preview for a payload (chars / prompt / description); truncated with an ellipsis beyond this, after which the preview stops growing. */ const MAX_PAYLOAD_PREVIEW = 120; +/** Max lines of a file-tool payload printed before the approval prompt; the rest is elided with a count. */ +const MAX_APPROVAL_PAYLOAD_LINES = 24; + +/** Max characters per printed approval-payload line (the full text stays in the trace). */ +const MAX_APPROVAL_PAYLOAD_LINE = 200; + /** Collapse to a single line: newlines/runs of whitespace become a single space, and leading/trailing whitespace is trimmed. */ function toSingleLine(text: string): string { return text.replace(/\s+/g, " ").trim(); @@ -44,8 +74,14 @@ function capPreview(text: string): string { return text.length > MAX_PAYLOAD_PREVIEW ? `${text.slice(0, MAX_PAYLOAD_PREVIEW)}…` : text; } -/** Extract the current value of a string field from a possibly-incomplete JSON object string. */ -function extractPartialStringField(argsJson: string, field: string): string | null { +/** A string field extracted from possibly-incomplete JSON: its value so far, and whether the closing quote was seen. */ +interface PartialField { + value: string; + complete: boolean; +} + +/** Extract a string field from a possibly-incomplete JSON object string, reporting completeness. */ +function extractField(argsJson: string, field: string): PartialField | null { const key = `"${field}"`; const keyIndex = argsJson.indexOf(key); if (keyIndex === -1) return null; @@ -89,7 +125,7 @@ function extractPartialStringField(argsJson: string, field: string): string | nu // emitting the incomplete hex as a literal would cause a rollback once the next // increment completes it (breaking append-only preview); the render layer falls // back to a new line in that case. - if (i + 5 > argsJson.length) return out; + if (i + 5 > argsJson.length) return { value: out, complete: false }; const hex = argsJson.slice(i + 1, i + 5); if (/^[0-9a-fA-F]{4}$/.test(hex)) { out += String.fromCharCode(Number.parseInt(hex, 16)); @@ -108,44 +144,220 @@ function extractPartialStringField(argsJson: string, field: string): string | nu escaped = true; continue; } - if (ch === '"') return out; + if (ch === '"') return { value: out, complete: true }; out += ch; } - return out; + return { value: out, complete: false }; +} + +/** Extract the current value of a string field from a possibly-incomplete JSON object string. */ +function extractPartialStringField(argsJson: string, field: string): string | null { + return extractField(argsJson, field)?.value ?? null; +} + +/** The three file tools: previewed as ` `. */ +const FILE_TOOL_NAMES = new Set(["read_file", "edit_file", "write_file"]); + +/** + * Shortens a path for one-line display: at most one parent directory plus the filename + * (`…/parent/file.ts`); paths already within that shape are shown as-is. The full path + * stays in the argument JSON (and the expanded web card). + */ +export function shortenPath(p: string): string { + const segments = p.split("/").filter((s) => s.length > 0); + if (segments.length <= 2) return p; + return `…/${segments[segments.length - 2]}/${segments[segments.length - 1]}`; +} + +/** How a tool call is previewed while its arguments stream (see the module header). */ +export interface ToolCallPreviewOptions { + /** + * Whether this tool's assembled schema carries the `description` argument (from + * `session_meta.tools`). Unknown ⇒ `false`: fall back to the plain form, matching a + * configuration with the argument switched off. + */ + expectDescription?: boolean; + /** The call's last fragment (its stream ended) or arguments from a complete message: settled, so render whichever form they carry. */ + final?: boolean; } /** - * Streaming argument preview: exec_command shows `$ ` once cmd can be read; input_command / - * input_subagent show `⌨ → ` once the target id is available, with a non-empty - * chars / prompt appended as `<< ` (an empty payload just means polling, left as-is); - * run_subagent shows `run_subagent << ` once prompt can be read; other tools fall back - * to name(args-prefix). + * State of the model-written `description` argument within a possibly-incomplete arguments + * fragment: + * - `pending`: it is expected but hasn't produced anything showable yet — nothing renders; + * - `none`: no usable description (not expected, or the settled arguments carry none), so + * the plain form is correct; + * - `{ text, complete }`: the description's value so far, single-lined and capped. Payload + * is appended only once `complete`, keeping the preview append-only whichever order the + * model emits its arguments in. */ -export function renderPartialToolCall(name: string, argsJson: string): string | null { +type DescriptionState = "pending" | "none" | { text: string; complete: boolean }; + +function describedState(argsJson: string, opts: ToolCallPreviewOptions): DescriptionState { + const settled = opts.final === true || argsComplete(argsJson); + // Not expected and not settled: the schema has no such argument, so stream the plain form + // right away. Settled fragments are read for real — a complete call renders what it carries. + if (!settled && opts.expectDescription !== true) return "none"; + const field = extractField(argsJson, "description"); + if (field === null) return settled ? "none" : "pending"; + const text = toSingleLine(field.value); + // An empty description (still opening, or explicitly "") carries nothing to show: once + // settled it means "no description", otherwise keep waiting for its first characters. + if (!text) return settled ? "none" : "pending"; + return { text: capPreview(text), complete: field.complete }; +} + +/** Whether the whole argument JSON parses (i.e. argument streaming is finished). */ +function argsComplete(argsJson: string): boolean { + try { + JSON.parse(argsJson); + return true; + } catch { + return false; + } +} + +/** + * Wraps a payload preview in the description form: `{name} <- {description} ({payload…}`, + * closing the parenthesis once `closed`. The open parenthesis mid-stream keeps the preview + * append-only while the payload grows. + */ +function describedForm(name: string, desc: string, payload: string, closed: boolean): string { + return `${name} <- ${desc} (${payload}${closed ? ")" : ""}`; +} + +/** + * Streaming argument preview (formats documented in the module header). Returns null while + * nothing presentable has streamed in yet — including a call whose schema carries the + * `description` argument (`opts.expectDescription`) whose value hasn't started streaming. + */ +export function renderPartialToolCall( + name: string, + argsJson: string, + opts: ToolCallPreviewOptions = {}, +): string | null { if (!argsJson) return null; if (name === "exec_command") { - const cmd = extractPartialStringField(argsJson, "cmd"); - if (cmd !== null) return `$ ${toSingleLine(cmd)}`; + const desc = describedState(argsJson, opts); + if (desc === "pending") return null; + const cmd = extractField(argsJson, "cmd"); + if (desc !== "none") { + if (!desc.complete || cmd === null) return `${name} <- ${desc.text}`; + return describedForm(name, desc.text, `$ ${toSingleLine(cmd.value)}`, cmd.complete); + } + if (cmd !== null) return `${name} <- $ ${toSingleLine(cmd.value)}`; return null; } if (name === "run_subagent") { - const prompt = extractPartialStringField(argsJson, "prompt"); - if (prompt !== null) return `run_subagent << ${capPreview(toSingleLine(prompt))}`; + const desc = describedState(argsJson, opts); + if (desc === "pending") return null; + const prompt = extractField(argsJson, "prompt"); + if (desc !== "none") { + if (!desc.complete || prompt === null) return `${name} <- ${desc.text}`; + return describedForm( + name, + desc.text, + capPreview(toSingleLine(prompt.value)), + prompt.complete, + ); + } + if (prompt !== null) return `${name} <- ${capPreview(toSingleLine(prompt.value))}`; return null; } if (name === "input_command") { - const pid = extractPartialStringField(argsJson, "process_id"); - if (pid === null) return null; + const desc = describedState(argsJson, opts); + if (desc === "pending") return null; + const pid = extractField(argsJson, "process_id"); + if (desc === "none" && pid === null) return null; const chars = extractPartialStringField(argsJson, "chars"); - const payload = chars ? ` << ${capPreview(visualizeControlChars(chars))}` : ""; - return `⌨ input_command → ${toSingleLine(pid)}${payload}`; + const payloadSuffix = chars ? ` << ${capPreview(visualizeControlChars(chars))}` : ""; + if (desc !== "none") { + if (!desc.complete || pid === null) return `${name} <- ${desc.text}`; + return describedForm( + name, + desc.text, + `${toSingleLine(pid.value)}${payloadSuffix}`, + argsComplete(argsJson), + ); + } + return `${name} <- ${toSingleLine(pid!.value)}${payloadSuffix}`; } if (name === "input_subagent") { - const sid = extractPartialStringField(argsJson, "subagent_id"); - if (sid === null) return null; + const desc = describedState(argsJson, opts); + if (desc === "pending") return null; + const sid = extractField(argsJson, "subagent_id"); + if (desc === "none" && sid === null) return null; const prompt = extractPartialStringField(argsJson, "prompt"); - const payload = prompt ? ` << ${capPreview(toSingleLine(prompt))}` : ""; - return `⌨ input_subagent → ${toSingleLine(sid)}${payload}`; + const payloadSuffix = prompt ? ` << ${capPreview(toSingleLine(prompt))}` : ""; + if (desc !== "none") { + if (!desc.complete || sid === null) return `${name} <- ${desc.text}`; + return describedForm( + name, + desc.text, + `${toSingleLine(sid.value)}${payloadSuffix}`, + argsComplete(argsJson), + ); + } + return `${name} <- ${toSingleLine(sid!.value)}${payloadSuffix}`; + } + if (FILE_TOOL_NAMES.has(name)) { + // Path rendered only once complete: shortening a still-growing path would rewrite the line. + const filePath = extractField(argsJson, "file_path"); + if (filePath !== null && filePath.complete) { + return `${name} ${shortenPath(toSingleLine(filePath.value))}`; + } + return null; } return `${name || "tool_call"}(${toSingleLine(argsJson)}`; } + +/** + * File-tool payload for the interactive approval prompt: the full decoded arguments + * (old_string / new_string / content …), bounded to MAX_APPROVAL_PAYLOAD_LINES lines with + * an explicit elision note — under always-ask/read-only approval the user must see what + * they are approving, not just the file path. Returns null for other tools or unparseable + * arguments (arguments are complete by approval time). + */ +export function renderFileToolApprovalPayload(name: string, argsJson: string): string | null { + if (!FILE_TOOL_NAMES.has(name)) return null; + let parsed: unknown; + try { + parsed = JSON.parse(argsJson); + } catch { + return null; + } + if (parsed === null || typeof parsed !== "object") return null; + const args = parsed as Record; + const lines: string[] = []; + const pushField = (label: string, value: unknown): void => { + if (value === undefined) return; + if (typeof value === "string" && value.includes("\n")) { + lines.push(`${label}:`); + for (const line of value.split("\n")) lines.push(` | ${line}`); + } else { + lines.push(`${label}: ${typeof value === "string" ? value : JSON.stringify(value)}`); + } + }; + pushField("file_path", args["file_path"]); + if (name === "read_file") { + pushField("offset", args["offset"]); + pushField("limit", args["limit"]); + } else if (name === "edit_file") { + pushField("old_string", args["old_string"]); + pushField("new_string", args["new_string"]); + if (args["replace_all"] === true) pushField("replace_all", true); + } else if (name === "write_file") { + pushField("content", args["content"]); + } + let shown = lines; + let elided = 0; + if (shown.length > MAX_APPROVAL_PAYLOAD_LINES) { + elided = shown.length - MAX_APPROVAL_PAYLOAD_LINES; + shown = shown.slice(0, MAX_APPROVAL_PAYLOAD_LINES); + } + const capped = shown.map((l) => + l.length > MAX_APPROVAL_PAYLOAD_LINE ? `${l.slice(0, MAX_APPROVAL_PAYLOAD_LINE)}…` : l, + ); + if (elided > 0) capped.push(`[… ${elided} more line${elided === 1 ? "" : "s"} not shown]`); + return capped.join("\n"); +} diff --git a/packages/cli/test/i18n.test.ts b/packages/cli/test/i18n.test.ts index d90f6ce..3061840 100644 --- a/packages/cli/test/i18n.test.ts +++ b/packages/cli/test/i18n.test.ts @@ -48,10 +48,16 @@ describe("getMessages", () => { expect(getMessages("zh").langInvalid("fr")).toContain("fr"); }); - it("header order is agent → workspace → model", () => { - const h = getMessages("en").header("run", "ag", "/ws", "mod"); - expect(h.indexOf("agent=ag")).toBeLessThan(h.indexOf("workspace=/ws")); - expect(h.indexOf("workspace=/ws")).toBeLessThan(h.indexOf("model=mod")); + it("header shows the version and Agent / Workspace / Model on their own lines", () => { + for (const lang of ["en", "zh"] as const) { + const lines = getMessages(lang).header("run", "1.2.3", "ag", "/ws", "mod").split("\n"); + expect(lines).toHaveLength(4); + expect(lines[0]).toContain("run"); + expect(lines[0]).toContain("v1.2.3"); + expect(lines[1]).toContain("ag"); + expect(lines[2]).toContain("/ws"); + expect(lines[3]).toContain("mod"); + } }); }); diff --git a/packages/cli/test/render.test.ts b/packages/cli/test/render.test.ts index 999f08e..816f2b2 100644 --- a/packages/cli/test/render.test.ts +++ b/packages/cli/test/render.test.ts @@ -117,19 +117,165 @@ describe("StreamRenderer", () => { r.handle(partialToolCall({ eventType: "stop", name: "", toolCallId: "c4" })); r.handle(toolCall({ name: "exec_command", arguments: '{"cmd":"ls"}', toolCallId: "c4" })); // The call line carries a [tool-] pairing tag matching the output line. - expect(stripAnsi(text())).toBe("[tool-c4] $ ls\n"); + expect(stripAnsi(text())).toBe("[tool-c4] exec_command <- $ ls\n"); }); - it("streams partial_tool_call_output with a tagged gutter and skips the complete tool_call_output", () => { + it("renders one call line when the description arrives after the command", () => { const { stream, text } = collector(); const r = new StreamRenderer(stream, t); + // The assembled schema carries the description argument, so the preview waits for it: + // with payload-first emission (models don't always honour schema order) the plain form + // must never reach the screen, or it would be stranded above the described one. + r.useToolSchemas([ + { + name: "exec_command", + description: "run a command", + parameters: { type: "object", properties: { description: {}, cmd: {} } }, + }, + ]); + r.handle(partialToolCall({ eventType: "start", name: "exec_command", toolCallId: "c9" })); + r.handle( + partialToolCall({ + eventType: "delta", + name: "", + arguments: '{"cmd":"ls -la",', + toolCallId: "c9", + }), + ); + r.handle( + partialToolCall({ + eventType: "delta", + name: "", + arguments: '"description":"列出当前目录的文件"}', + toolCallId: "c9", + }), + ); + r.handle(partialToolCall({ eventType: "stop", name: "", toolCallId: "c9" })); + expect(stripAnsi(text())).toBe("[tool-c9] exec_command <- 列出当前目录的文件 ($ ls -la)\n"); + }); + + it("streams the command live when the schema has no description argument", () => { + const { stream, text } = collector(); + const r = new StreamRenderer(stream, t); + // call_description switched off for this tool: nothing can supersede the plain form, so + // it streams as the arguments arrive rather than waiting for them to settle. + r.useToolSchemas([ + { + name: "exec_command", + description: "run a command", + parameters: { type: "object", properties: { cmd: {} } }, + }, + ]); + r.handle(partialToolCall({ eventType: "start", name: "exec_command", toolCallId: "c7" })); + r.handle( + partialToolCall({ eventType: "delta", name: "", arguments: '{"cmd":"ls', toolCallId: "c7" }), + ); + expect(stripAnsi(text())).toBe("[tool-c7] exec_command <- $ ls"); + r.handle( + partialToolCall({ eventType: "delta", name: "", arguments: ' -la"}', toolCallId: "c7" }), + ); + r.handle(partialToolCall({ eventType: "stop", name: "", toolCallId: "c7" })); + expect(stripAnsi(text())).toBe("[tool-c7] exec_command <- $ ls -la\n"); + }); + + it("still renders a call line whose arguments never settled", () => { + const { stream, text } = collector(); + const r = new StreamRenderer(stream, t); + // Interrupted mid-arguments while awaiting a description: the call must not vanish. + r.useToolSchemas([ + { + name: "exec_command", + description: "run a command", + parameters: { type: "object", properties: { description: {}, cmd: {} } }, + }, + ]); + r.handle(partialToolCall({ eventType: "start", name: "exec_command", toolCallId: "c8" })); + r.handle( + partialToolCall({ eventType: "delta", name: "", arguments: '{"cmd":"sle', toolCallId: "c8" }), + ); + r.handle(partialToolCall({ eventType: "stop", name: "", toolCallId: "c8" })); + expect(stripAnsi(text())).toBe("[tool-c8] exec_command <- $ sle\n"); + }); + + it("prefixes streamed tool output with the tool name and skips the complete tool_call_output", () => { + const { stream, text } = collector(); + const r = new StreamRenderer(stream, t); + // The call precedes its output and supplies the gutter's tool name. + r.handle(partialToolCall({ eventType: "start", name: "exec_command", toolCallId: "c3" })); + r.handle( + partialToolCall({ + eventType: "delta", + name: "", + arguments: '{"cmd":"ls"}', + toolCallId: "c3", + }), + ); + r.handle(partialToolCall({ eventType: "stop", name: "", toolCallId: "c3" })); r.handle(partialToolCallOutput({ eventType: "start", toolCallId: "c3" })); r.handle(partialToolCallOutput({ eventType: "delta", output: "line1\n", toolCallId: "c3" })); r.handle(partialToolCallOutput({ eventType: "delta", output: "line2", toolCallId: "c3" })); r.handle(partialToolCallOutput({ eventType: "stop", toolCallId: "c3" })); r.handle(toolCallOutput({ output: "line1\nline2", toolCallId: "c3" })); // must not be re-rendered - // Each line starts with a tagged gutter (no indent) matching the call line. - expect(stripAnsi(text())).toBe("[tool-c3] >> line1\n[tool-c3] >> line2\n"); + // Call line first, then each output line repeats the `[tool-xxx] ` prefix. + expect(stripAnsi(text())).toBe( + "[tool-c3] exec_command <- $ ls\n[tool-c3] exec_command -> line1\n[tool-c3] exec_command -> line2\n", + ); + }); + + it("colors edit_file diff output lines green/red and dims hunk headers", () => { + const { stream, text } = collector(); + const r = new StreamRenderer(stream, t); + r.handle(partialToolCall({ eventType: "start", name: "edit_file", toolCallId: "d1" })); + r.handle( + partialToolCall({ + eventType: "delta", + name: "", + arguments: '{"file_path":"x.ts","old_string":"old","new_string":"new"}', + toolCallId: "d1", + }), + ); + r.handle(partialToolCall({ eventType: "stop", name: "", toolCallId: "d1" })); + r.handle(partialToolCallOutput({ eventType: "start", toolCallId: "d1" })); + r.handle( + partialToolCallOutput({ + eventType: "delta", + output: 'Replaced 1 occurrence in "x.ts".\n@@ -1,1 +1,1 @@\n-old\n+new\n', + toolCallId: "d1", + }), + ); + r.handle(partialToolCallOutput({ eventType: "stop", toolCallId: "d1" })); + const raw = text(); + // Diff lines are wrapped in green/red; the hunk header is dimmed; the summary line stays plain. + expect(raw).toContain("\x1b[32m+new\x1b[0m"); + expect(raw).toContain("\x1b[31m-old\x1b[0m"); + expect(raw).toContain("\x1b[2m@@ -1,1 +1,1 @@\x1b[0m"); + // The stripped view still reads as labeled gutter lines. + const plain = stripAnsi(raw); + expect(plain).toContain("[tool-d1] edit_file -> -old"); + expect(plain).toContain("[tool-d1] edit_file -> +new"); + }); + + it("does not diff-color non-file-tool output", () => { + const { stream, text } = collector(); + const r = new StreamRenderer(stream, t); + r.handle(partialToolCall({ eventType: "start", name: "exec_command", toolCallId: "d2" })); + r.handle( + partialToolCall({ eventType: "delta", name: "", arguments: '{"cmd":"x"}', toolCallId: "d2" }), + ); + r.handle(partialToolCall({ eventType: "stop", name: "", toolCallId: "d2" })); + r.handle(partialToolCallOutput({ eventType: "start", toolCallId: "d2" })); + r.handle(partialToolCallOutput({ eventType: "delta", output: "+plus\n", toolCallId: "d2" })); + r.handle(partialToolCallOutput({ eventType: "stop", toolCallId: "d2" })); + expect(text()).not.toContain("\x1b[32m"); + }); + + it("falls back to the pairing tag on output whose call was never seen", () => { + const { stream, text } = collector(); + const r = new StreamRenderer(stream, t); + r.handle(partialToolCallOutput({ eventType: "start", toolCallId: "c3" })); + r.handle(partialToolCallOutput({ eventType: "delta", output: "line1", toolCallId: "c3" })); + r.handle(partialToolCallOutput({ eventType: "stop", toolCallId: "c3" })); + expect(stripAnsi(text())).toBe("[tool-c3] -> line1\n"); }); it("prints the retry line only when the retry request actually begins", () => { @@ -165,10 +311,11 @@ describe("StreamRenderer", () => { r.handle(partialText("start", "")); r.handle(partialText("delta", "hello")); r.handle(partialToolCallOutput({ eventType: "delta", output: "a2\n", toolCallId: "tA" })); - expect(stripAnsi(text())).toBe("[tool-tA] >> a1\n[tool-tA] >> a2\n"); // hello is still queued + // No call preceded tA in this stream: the gutter falls back to the pairing tag. + expect(stripAnsi(text())).toBe("[tool-tA] -> a1\n[tool-tA] -> a2\n"); // hello is still queued r.handle(partialToolCallOutput({ eventType: "stop", toolCallId: "tA" })); r.handle(partialText("stop", "", "completed")); - expect(stripAnsi(text())).toBe("[tool-tA] >> a1\n[tool-tA] >> a2\nhello\n"); + expect(stripAnsi(text())).toBe("[tool-tA] -> a1\n[tool-tA] -> a2\nhello\n"); }); it("queues everything while a user prompt is active and flushes after it ends", () => { @@ -411,10 +558,30 @@ describe("StreamRenderer", () => { r.beginUserPrompt(tc); r.noteApprovalDecision(tc, "allow"); r.endUserPrompt(); - expect(stripAnsi(text())).toBe("[tool-p8] $ pwd\n✓ [approved]\n"); + expect(stripAnsi(text())).toBe("[tool-p8] exec_command <- $ pwd\n✓ [approved]\n"); // A late approval_decision event is deduped by key and not re-rendered. r.handle(approvalDecision("allow", "p8")); - expect(stripAnsi(text())).toBe("[tool-p8] $ pwd\n✓ [approved]\n"); + expect(stripAnsi(text())).toBe("[tool-p8] exec_command <- $ pwd\n✓ [approved]\n"); + }); + + it("prints the decoded file-tool payload before the approval prompt, without duplicating the call line", () => { + const { stream, text } = collector(); + const r = new StreamRenderer(stream, t); + const tc = toolCall({ + name: "edit_file", + arguments: JSON.stringify({ file_path: "src/x.ts", old_string: "a", new_string: "b" }), + toolCallId: "fp1", + }); + r.beginUserPrompt(tc); + r.noteApprovalDecision(tc, "allow"); + r.endUserPrompt(); + // Call line, payload lines (what the user is approving), then the result — with no + // duplicated call line after the payload. + expect(stripAnsi(text())).toBe( + "[tool-fp1] edit_file src/x.ts\n" + + "file_path: src/x.ts\nold_string: a\nnew_string: b\n" + + "✓ [approved]\n", + ); }); it("re-renders a half-streamed call line at approval and suppresses its late tail deltas", () => { @@ -447,7 +614,7 @@ describe("StreamRenderer", () => { // At approval time, the full call line is re-rendered in place from the complete message, right next to // the result; after unlocking, the late tail is deduped and must not start a duplicate call line after // the result line. - expect(s).toContain("[tool-h7] $ git status\n✓ [approved]\n"); + expect(s).toContain("[tool-h7] exec_command <- $ git status\n✓ [approved]\n"); expect(s.slice(s.indexOf("[approved]"))).not.toContain("[tool-h7]"); }); @@ -531,7 +698,9 @@ describe("StreamRenderer", () => { toolCall({ name: "exec_command", arguments: '{"cmd":"ls"}', toolCallId: "c5" }), "allow", ); - expect(stripAnsi(text())).toBe("[tool-c5] $ ls\nhi\n[tool-c5] $ ls\n✓ [approved]\n"); + expect(stripAnsi(text())).toBe( + "[tool-c5] exec_command <- $ ls\nhi\n[tool-c5] exec_command <- $ ls\n✓ [approved]\n", + ); }); it("does not re-render the call line when it is already adjacent to the decision", () => { @@ -551,7 +720,7 @@ describe("StreamRenderer", () => { toolCall({ name: "exec_command", arguments: '{"cmd":"ls"}', toolCallId: "c6" }), "deny", ); - expect(stripAnsi(text())).toBe("[tool-c6] $ ls\n× [denied]\n"); + expect(stripAnsi(text())).toBe("[tool-c6] exec_command <- $ ls\n× [denied]\n"); }); }); @@ -574,7 +743,7 @@ describe("StreamRenderer — nested (origin-tagged) subagent messages", () => { ), ); r.handle(withOrigin(approvalDecision("allow", "cc1"), hop)); - expect(stripAnsi(text())).toBe("[agent-ild-tool-cc1] $ ls\n✓ [approved]\n"); + expect(stripAnsi(text())).toBe("[agent-ild-tool-cc1] exec_command <- $ ls\n✓ [approved]\n"); }); it("renders the pending nested tool call at approval time when its stream copy has not arrived; dedupes the late copy", () => { @@ -586,11 +755,11 @@ describe("StreamRenderer — nested (origin-tagged) subagent messages", () => { ); // The approval callback arrives before the forwarded message: beginUserPrompt renders the call line directly from the complete message. r.beginUserPrompt(tc); - expect(stripAnsi(text())).toBe("[agent-ild-tool-cc9] $ ls\n"); + expect(stripAnsi(text())).toBe("[agent-ild-tool-cc9] exec_command <- $ ls\n"); r.endUserPrompt(); // The late forwarded copy is deduped by key and not re-rendered. r.handle(tc); - expect(stripAnsi(text())).toBe("[agent-ild-tool-cc9] $ ls\n"); + expect(stripAnsi(text())).toBe("[agent-ild-tool-cc9] exec_command <- $ ls\n"); }); it("renders the pending parent tool call at approval time and suppresses its late partial stream", () => { @@ -611,7 +780,7 @@ describe("StreamRenderer — nested (origin-tagged) subagent messages", () => { }), ); r.handle(partialToolCall({ eventType: "stop", name: "", toolCallId: "p7" })); - expect(stripAnsi(text())).toBe("[tool-p7] $ pwd\n"); + expect(stripAnsi(text())).toBe("[tool-p7] exec_command <- $ pwd\n"); }); it("adds nested token_usage request totals to the task delta and the session total", () => { @@ -691,9 +860,9 @@ describe("renderHistory (resume)", () => { expect(s).toContain("> hello"); expect(s).toContain("pondering"); expect(s).toContain("hi there"); - expect(s).toContain("[tool-653] $ ls"); - expect(s).toContain("[tool-653] >> a.txt"); - expect(s).toContain("[tool-653] >> b.txt"); + expect(s).toContain("[tool-653] exec_command <- $ ls"); + expect(s).toContain("[tool-653] exec_command -> a.txt"); + expect(s).toContain("[tool-653] exec_command -> b.txt"); // An interrupted message carries a marker (rendering includes the interrupted turn). expect(s).toContain("half answer [aborted]"); }); diff --git a/packages/cli/test/tool-render.test.ts b/packages/cli/test/tool-render.test.ts index 9da989b..6b93939 100644 --- a/packages/cli/test/tool-render.test.ts +++ b/packages/cli/test/tool-render.test.ts @@ -1,72 +1,174 @@ import { describe, expect, it } from "vitest"; -import { renderPartialToolCall } from "../src/tool-render.js"; +import { + renderFileToolApprovalPayload, + renderPartialToolCall, + shortenPath, +} from "../src/tool-render.js"; -describe("renderPartialToolCall", () => { - it("renders partial exec_command args as $ ", () => { +describe("renderPartialToolCall — exec_command", () => { + it("streams `exec_command <- $ {cmd}` when the schema has no description argument", () => { + // The default path: the switch is off (or the tool is unknown), so nothing can supersede + // the plain form and it streams character by character. expect(renderPartialToolCall("exec_command", '{"cmd":')).toBeNull(); - expect(renderPartialToolCall("exec_command", '{"cmd":"l')).toBe("$ l"); - expect(renderPartialToolCall("exec_command", '{"cmd":"ls"}')).toBe("$ ls"); - expect(renderPartialToolCall("exec_command", '{"cmd":"echo \\"hi\\"')).toBe('$ echo "hi"'); - }); - - it("renders run_subagent as run_subagent << , folded to one line", () => { - expect(renderPartialToolCall("run_subagent", '{"prompt":')).toBeNull(); - expect(renderPartialToolCall("run_subagent", '{"prompt":"analy')).toBe("run_subagent << analy"); - expect(renderPartialToolCall("run_subagent", '{"prompt":"line1\\nline2"}')).toBe( - "run_subagent << line1 line2", + expect(renderPartialToolCall("exec_command", '{"cmd":"l')).toBe("exec_command <- $ l"); + expect(renderPartialToolCall("exec_command", '{"cmd":"ls"}')).toBe("exec_command <- $ ls"); + expect(renderPartialToolCall("exec_command", '{"cmd":"echo \\"hi\\"')).toBe( + 'exec_command <- $ echo "hi"', ); }); - it("renders input_command polls (empty chars) without a payload", () => { - expect(renderPartialToolCall("input_command", '{"process_id":')).toBeNull(); - expect(renderPartialToolCall("input_command", '{"process_id":"proc-1a2b3c4d"}')).toBe( - "⌨ input_command → proc-1a2b3c4d", + it("waits for the description when the schema carries the argument", () => { + const described = { expectDescription: true }; + // Nothing renders while the description could still be the first thing shown... + expect(renderPartialToolCall("exec_command", '{"cmd":"ls -la', described)).toBeNull(); + // ...until the arguments settle without one, or the fragment is final. + expect(renderPartialToolCall("exec_command", '{"cmd":"ls -la"}', described)).toBe( + "exec_command <- $ ls -la", ); expect( - renderPartialToolCall("input_command", '{"process_id":"proc-1a2b3c4d","chars":""}'), - ).toBe("⌨ input_command → proc-1a2b3c4d"); + renderPartialToolCall("exec_command", '{"cmd":"ls -la', { ...described, final: true }), + ).toBe("exec_command <- $ ls -la"); }); - it("renders non-empty input_command chars with visible control characters", () => { + it("renders `exec_command <- {description} ($ {cmd})` when a description is present", () => { expect( - renderPartialToolCall("input_command", '{"process_id":"proc-1a2b3c4d","chars":"y\\n"}'), - ).toBe("⌨ input_command → proc-1a2b3c4d << y\\n"); - // U+0003 (Ctrl-C) is rendered in caret notation. + renderPartialToolCall("exec_command", '{"description":"List files","cmd":"ls -la"}'), + ).toBe("exec_command <- List files ($ ls -la)"); + // Same final form regardless of the model's property order. expect( - renderPartialToolCall("input_command", '{"process_id":"proc-1a2b3c4d","chars":"\\u0003"}'), - ).toBe("⌨ input_command → proc-1a2b3c4d << ^C"); - // Disambiguates literal backslash escapes: chars "a", "\", "n" render as a\\n, distinct from a real newline \n. - expect( - renderPartialToolCall("input_command", '{"process_id":"proc-1a2b3c4d","chars":"a\\\\n"}'), - ).toBe("⌨ input_command → proc-1a2b3c4d << a\\\\n"); + renderPartialToolCall("exec_command", '{"cmd":"ls -la","description":"List files"}'), + ).toBe("exec_command <- List files ($ ls -la)"); + // Multi-line descriptions fold to one line; an empty description falls back to the plain form. + expect(renderPartialToolCall("exec_command", '{"cmd":"ls","description":"a\\nb"}')).toBe( + "exec_command <- a b ($ ls)", + ); + expect(renderPartialToolCall("exec_command", '{"cmd":"ls","description":""}')).toBe( + "exec_command <- $ ls", + ); }); - it("keeps input_command previews append-only across \\uXXXX delta boundaries", () => { + it("streams the description form append-only when the description arrives first", () => { const stages = [ - '{"process_id":"proc-1a2b3c4d","chars":"y', - '{"process_id":"proc-1a2b3c4d","chars":"y\\u0', - '{"process_id":"proc-1a2b3c4d","chars":"y\\u0003', + '{"description":"List fi', // description streams live + '{"description":"List files"', // description complete + '{"description":"List files","cmd":"ls', // cmd streaming inside the open parenthesis + '{"description":"List files","cmd":"ls -la"}', // cmd complete: parenthesis closes ]; - const previews = stages.map((s) => renderPartialToolCall("input_command", s)!); - expect(previews[0]).toBe("⌨ input_command → proc-1a2b3c4d << y"); - // An incomplete \u escape is treated as "stop here" rather than emitting the raw hex as literal text. - expect(previews[1]).toBe("⌨ input_command → proc-1a2b3c4d << y"); - expect(previews[2]).toBe("⌨ input_command → proc-1a2b3c4d << y^C"); + const previews = stages.map((s) => + renderPartialToolCall("exec_command", s, { expectDescription: true }), + ); + expect(previews[0]).toBe("exec_command <- List fi"); + expect(previews[1]).toBe("exec_command <- List files"); + expect(previews[2]).toBe("exec_command <- List files ($ ls"); + expect(previews[3]).toBe("exec_command <- List files ($ ls -la)"); for (let i = 1; i < previews.length; i++) { expect(previews[i]!.startsWith(previews[i - 1]!)).toBe(true); } }); - it("renders input_subagent polls without a payload and follow-up prompts with one", () => { + it("never shows the plain form first when the model emits the payload before the description", () => { + // The regression this guards: a plain line followed by a described one for the same call. + const stages = [ + '{"cmd":"ls -la', // withheld: the schema says a description is coming + '{"cmd":"ls -la","description":"List fi', // description streams; payload waits for it + '{"cmd":"ls -la","description":"List files"}', // settled: payload appended + ]; + const previews = stages.map((s) => + renderPartialToolCall("exec_command", s, { expectDescription: true }), + ); + expect(previews[0]).toBeNull(); + expect(previews[1]).toBe("exec_command <- List fi"); + expect(previews[2]).toBe("exec_command <- List files ($ ls -la)"); + expect(previews[2]!.startsWith(previews[1]!)).toBe(true); + expect(previews.some((p) => p === "exec_command <- $ ls -la")).toBe(false); + }); +}); + +describe("renderPartialToolCall — run_subagent", () => { + it("renders `run_subagent <- {prompt}` and the description form", () => { + expect(renderPartialToolCall("run_subagent", '{"prompt":')).toBeNull(); + expect(renderPartialToolCall("run_subagent", '{"prompt":"analy')).toBe("run_subagent <- analy"); + expect(renderPartialToolCall("run_subagent", '{"prompt":"line1\\nline2"}')).toBe( + "run_subagent <- line1 line2", + ); + expect( + renderPartialToolCall( + "run_subagent", + '{"description":"Delegating research","prompt":"do the thing"}', + ), + ).toBe("run_subagent <- Delegating research (do the thing)"); + }); +}); + +describe("renderPartialToolCall — input_command / input_subagent", () => { + it("renders polls (empty chars) without a payload", () => { + expect(renderPartialToolCall("input_command", '{"process_id":')).toBeNull(); + expect(renderPartialToolCall("input_command", '{"process_id":"proc-1a2b3c4d"}')).toBe( + "input_command <- proc-1a2b3c4d", + ); + expect( + renderPartialToolCall("input_command", '{"process_id":"proc-1a2b3c4d","chars":""}'), + ).toBe("input_command <- proc-1a2b3c4d"); + }); + + it("renders non-empty chars with visible control characters", () => { + expect( + renderPartialToolCall("input_command", '{"process_id":"proc-1a2b3c4d","chars":"y\\n"}'), + ).toBe("input_command <- proc-1a2b3c4d << y\\n"); + // U+0003 (Ctrl-C) is rendered in caret notation. + expect( + renderPartialToolCall("input_command", '{"process_id":"proc-1a2b3c4d","chars":"\\u0003"}'), + ).toBe("input_command <- proc-1a2b3c4d << ^C"); + // Disambiguates literal backslash escapes: chars "a", "\", "n" render as a\\n, distinct from a real newline \n. + expect( + renderPartialToolCall("input_command", '{"process_id":"proc-1a2b3c4d","chars":"a\\\\n"}'), + ).toBe("input_command <- proc-1a2b3c4d << a\\\\n"); + }); + + it("wraps the payload in parentheses after the description", () => { + expect( + renderPartialToolCall( + "input_command", + '{"description":"Confirm the prompt","process_id":"proc-1a2b3c4d","chars":"y\\n"}', + ), + ).toBe("input_command <- Confirm the prompt (proc-1a2b3c4d << y\\n)"); + expect( + renderPartialToolCall( + "input_subagent", + '{"description":"Poll for progress","subagent_id":"subagent-9f8e7d6c"}', + ), + ).toBe("input_subagent <- Poll for progress (subagent-9f8e7d6c)"); + }); + + it("keeps input_command previews append-only across \\uXXXX delta boundaries", () => { + // Schema order (description first) keeps the whole call streaming live. + const stages = [ + '{"description":"Confirm","process_id":"proc-1a2b3c4d","chars":"y', + '{"description":"Confirm","process_id":"proc-1a2b3c4d","chars":"y\\u0', + '{"description":"Confirm","process_id":"proc-1a2b3c4d","chars":"y\\u0003', + ]; + const previews = stages.map((s) => + renderPartialToolCall("input_command", s, { expectDescription: true })!, + ); + expect(previews[0]).toBe("input_command <- Confirm (proc-1a2b3c4d << y"); + // An incomplete \u escape is treated as "stop here" rather than emitting the raw hex as literal text. + expect(previews[1]).toBe("input_command <- Confirm (proc-1a2b3c4d << y"); + expect(previews[2]).toBe("input_command <- Confirm (proc-1a2b3c4d << y^C"); + for (let i = 1; i < previews.length; i++) { + expect(previews[i]!.startsWith(previews[i - 1]!)).toBe(true); + } + }); + + it("renders input_subagent polls and follow-up prompts", () => { expect( renderPartialToolCall("input_subagent", '{"subagent_id":"subagent-9f8e7d6c","prompt":""}'), - ).toBe("⌨ input_subagent → subagent-9f8e7d6c"); + ).toBe("input_subagent <- subagent-9f8e7d6c"); expect( renderPartialToolCall( "input_subagent", '{"subagent_id":"subagent-9f8e7d6c","prompt":"continue with the tests"}', ), - ).toBe("⌨ input_subagent → subagent-9f8e7d6c << continue with the tests"); + ).toBe("input_subagent <- subagent-9f8e7d6c << continue with the tests"); }); it("truncates long payload previews and stops growing afterwards", () => { @@ -75,15 +177,95 @@ describe("renderPartialToolCall", () => { "input_subagent", `{"subagent_id":"subagent-9f8e7d6c","prompt":"${long}"}`, ); - expect(capped).toBe(`⌨ input_subagent → subagent-9f8e7d6c << ${"x".repeat(120)}…`); + expect(capped).toBe(`input_subagent <- subagent-9f8e7d6c << ${"x".repeat(120)}…`); const longer = renderPartialToolCall( "input_subagent", `{"subagent_id":"subagent-9f8e7d6c","prompt":"${long}yyy"}`, ); expect(longer).toBe(capped); }); +}); +describe("renderPartialToolCall — file tools", () => { + it("renders ` ` only once the path is complete", () => { + expect(renderPartialToolCall("read_file", '{"file_path":')).toBeNull(); + // A still-streaming path is withheld: shortening a growing path would rewrite the line. + expect(renderPartialToolCall("read_file", '{"file_path":"src/ap')).toBeNull(); + expect(renderPartialToolCall("read_file", '{"file_path":"src/app.py","offset":10}')).toBe( + "read_file src/app.py", + ); + expect( + renderPartialToolCall("edit_file", '{"file_path":"a.txt","old_string":"x","new_string":"y"}'), + ).toBe("edit_file a.txt"); + expect( + renderPartialToolCall("write_file", '{"file_path":"packages/core/src/state/out.ts"}'), + ).toBe("write_file …/state/out.ts"); + }); +}); + +describe("renderPartialToolCall — fallback", () => { it("falls back to name(args-prefix) for unknown tools", () => { expect(renderPartialToolCall("search", '{"q":"hi')).toBe('search({"q":"hi'); }); }); + +describe("shortenPath", () => { + it("keeps at most one parent directory plus the filename", () => { + expect(shortenPath("file.ts")).toBe("file.ts"); + expect(shortenPath("src/file.ts")).toBe("src/file.ts"); + expect(shortenPath("/etc/hosts")).toBe("/etc/hosts"); + expect(shortenPath("packages/core/src/state/default-config.ts")).toBe( + "…/state/default-config.ts", + ); + expect(shortenPath("/home/user/project/src/app.py")).toBe("…/src/app.py"); + }); +}); + +describe("renderFileToolApprovalPayload", () => { + it("prints the decoded edit_file payload with gutters for multi-line fields", () => { + const payload = renderFileToolApprovalPayload( + "edit_file", + JSON.stringify({ + file_path: "src/app.py", + old_string: "a\nb", + new_string: "a\nc", + }), + ); + expect(payload).toBe( + [ + "file_path: src/app.py", + "old_string:", + " | a", + " | b", + "new_string:", + " | a", + " | c", + ].join("\n"), + ); + }); + + it("prints write_file content and read_file window arguments", () => { + expect( + renderFileToolApprovalPayload("write_file", '{"file_path":"out.md","content":"hello"}'), + ).toBe(["file_path: out.md", "content: hello"].join("\n")); + expect( + renderFileToolApprovalPayload("read_file", '{"file_path":"a.txt","offset":3,"limit":5}'), + ).toBe(["file_path: a.txt", "offset: 3", "limit: 5"].join("\n")); + }); + + it("bounds the payload to a line count with an explicit elision note", () => { + const content = Array.from({ length: 60 }, (_, i) => `line-${i + 1}`).join("\n"); + const payload = renderFileToolApprovalPayload( + "write_file", + JSON.stringify({ file_path: "big.txt", content }), + )!; + const lines = payload.split("\n"); + // 24 shown lines + the elision note. + expect(lines).toHaveLength(25); + expect(lines[lines.length - 1]).toMatch(/\[… \d+ more lines not shown\]/); + }); + + it("returns null for non-file tools", () => { + expect(renderFileToolApprovalPayload("exec_command", '{"cmd":"ls"}')).toBeNull(); + }); +}); diff --git a/packages/core/src/environment/index.ts b/packages/core/src/environment/index.ts index 00e58d7..646ed3b 100644 --- a/packages/core/src/environment/index.ts +++ b/packages/core/src/environment/index.ts @@ -5,6 +5,9 @@ export { Environment } from "./environment.js"; export type { BuiltinTool, ToolExecutionContext } from "./tools/types.js"; export { BUILTIN_TOOL_FACTORIES } from "./tools/registry.js"; export type { BuiltinToolFactory } from "./tools/registry.js"; +export { createReadFileTool, READ_FILE_NAME } from "./tools/read-file.js"; +export { createEditFileTool, EDIT_FILE_NAME } from "./tools/edit-file.js"; +export { createWriteFileTool, WRITE_FILE_NAME } from "./tools/write-file.js"; export { createExecCommandTool, EXEC_COMMAND_NAME } from "./tools/exec-command.js"; export { createInputCommandTool, INPUT_COMMAND_NAME } from "./tools/input-command.js"; export { createSubagentTool, SUBAGENT_NAME } from "./tools/run-subagent.js"; diff --git a/packages/core/src/environment/tools/diff.ts b/packages/core/src/environment/tools/diff.ts new file mode 100644 index 0000000..7db1b84 --- /dev/null +++ b/packages/core/src/environment/tools/diff.ts @@ -0,0 +1,380 @@ +/** + * Unified-diff rendering for the file tools' outputs (edit_file / write_file), modeled on + * git's presentation: `@@ -oldStart,oldCount +newStart,newCount @@` hunk headers, ` ` + * context lines, `-` removed and `+` added lines, 3 lines of context, nearby changes + * merged into one hunk. + * + * Two hunk builders share the renderer: + * - `buildReplacementHunks` (edit_file): exact by construction — the replacement sites are + * known, so each hunk is derived by re-applying the replacement to the affected line + * region; no diff algorithm, cost independent of file size. + * - `buildLineDiffHunks` (write_file): a real line diff (LCS over the middle remaining + * after common prefix/suffix trimming) for arbitrary old/new contents, with a size guard + * that falls back to a `+X/−Y` summary when the middle is too large to diff cheaply. + * + * Display safety: diff lines are capped in length and trailing `\r` is stripped (a raw CR + * would glitch terminal rendering); CRLF-vs-LF differences therefore do not show as + * whole-file rewrites in the display, while counts stay based on the true content. + */ + +/** Context lines shown on each side of a change (git default). */ +export const DIFF_CONTEXT_LINES = 3; + +/** Max characters kept of a single diff line; the rest is replaced by a truncation marker. */ +const MAX_DIFF_LINE_LENGTH = 2000; + +/** DP cell cap for the LCS line diff: above this the write_file diff falls back to a summary. */ +const MAX_LCS_CELLS = 250_000; + +/** One unified hunk: header numbers plus already-prefixed (` `/`-`/`+`) display lines. */ +export interface DiffHunk { + oldStart: number; + oldCount: number; + newStart: number; + newCount: number; + lines: string[]; +} + +/** Caps a diff line for display and strips a trailing `\r` (CRLF files). */ +function capDiffLine(prefix: string, content: string): string { + const noCr = content.endsWith("\r") ? content.slice(0, -1) : content; + const capped = + noCr.length > MAX_DIFF_LINE_LENGTH + ? `${noCr.slice(0, MAX_DIFF_LINE_LENGTH)}… [line truncated]` + : noCr; + return `${prefix}${capped}`; +} + +/** Renders one hunk in git's unified format (a count of 0 backs the start up by one, as git does). */ +export function renderHunk(h: DiffHunk): string { + const oldStart = h.oldCount === 0 ? h.oldStart - 1 : h.oldStart; + const newStart = h.newCount === 0 ? h.newStart - 1 : h.newStart; + return [`@@ -${oldStart},${h.oldCount} +${newStart},${h.newCount} @@`, ...h.lines].join("\n"); +} + +/** Splits content into lines the way git counts them: a trailing newline terminates the last line instead of adding an empty one. */ +export function contentLines(content: string): string[] { + if (content === "") return []; + const lines = content.split("\n"); + if (lines[lines.length - 1] === "") lines.pop(); + return lines; +} + +/** 1-based line number containing the character at `pos` (a newline belongs to the line it terminates). */ +function lineOfChar(content: string, pos: number): number { + let line = 1; + for (let i = 0; i < pos && i < content.length; i += 1) { + if (content.charCodeAt(i) === 10) line += 1; + } + return line; +} + +/** Char offset where 1-based line `line` starts. */ +function lineStartOffset(content: string, line: number): number { + let current = 1; + for (let i = 0; i < content.length; i += 1) { + if (current === line) return i; + if (content.charCodeAt(i) === 10) current += 1; + } + return content.length; +} + +/** Char offset just past the end of 1-based line `line` (including its terminating newline when present). */ +function lineEndOffset(content: string, line: number): number { + const start = lineStartOffset(content, line); + const nl = content.indexOf("\n", start); + return nl === -1 ? content.length : nl + 1; +} + +export interface ReplacementDiff { + /** Hunks in file order, each with the number of replacement occurrences it covers. */ + hunks: { hunk: DiffHunk; sites: number }[]; +} + +/** + * Builds unified hunks for edit_file: one hunk per replacement site, with sites whose + * context ranges touch merged into a single hunk. Each hunk's `+` side is derived by + * re-applying the replacement to the affected region, so it is exact by construction + * (including several occurrences on one line). `maxHunks` caps the output; each hunk + * reports how many occurrences it covers so the caller can note the uncovered rest. + */ +export function buildReplacementHunks( + oldContent: string, + oldString: string, + newString: string, + replaceAll: boolean, + maxHunks: number, +): ReplacementDiff { + // Occurrence positions in the old content (non-overlapping, left to right). + const positions: number[] = []; + let idx = oldContent.indexOf(oldString); + while (idx !== -1) { + positions.push(idx); + if (!replaceAll) break; + idx = oldContent.indexOf(oldString, idx + oldString.length); + } + const oldLines = contentLines(oldContent); + const total = oldLines.length; + + // Line range touched by each site, then merge sites whose context ranges touch. + const ranges = positions.map((pos) => { + const a1 = lineOfChar(oldContent, pos); + const a2 = lineOfChar(oldContent, pos + Math.max(0, oldString.length - 1)); + return { a1, a2, sites: 1 }; + }); + const merged: { a1: number; a2: number; sites: number }[] = []; + for (const r of ranges) { + const last = merged[merged.length - 1]; + if (last && r.a1 - DIFF_CONTEXT_LINES <= last.a2 + DIFF_CONTEXT_LINES) { + last.a2 = Math.max(last.a2, r.a2); + last.sites += r.sites; + } else { + merged.push({ ...r }); + } + } + + const hunks: { hunk: DiffHunk; sites: number }[] = []; + let lineDelta = 0; // Cumulative new-minus-old line shift from earlier hunks + for (const region of merged) { + if (hunks.length >= maxHunks) break; + // Exact regional replacement: extract the affected whole-line region and re-apply. + const regionText = oldContent.slice( + lineStartOffset(oldContent, region.a1), + lineEndOffset(oldContent, region.a2), + ); + const newRegionText = regionText.split(oldString).join(newString); + let minus = contentLines(regionText); + let plus = contentLines(newRegionText); + // Shared leading/trailing lines become context instead of noise. + let leadShift = 0; + while (minus.length > 0 && plus.length > 0 && minus[0] === plus[0]) { + minus = minus.slice(1); + plus = plus.slice(1); + leadShift += 1; + } + let tailShift = 0; + while ( + minus.length > 0 && + plus.length > 0 && + minus[minus.length - 1] === plus[plus.length - 1] + ) { + minus = minus.slice(0, -1); + plus = plus.slice(0, -1); + tailShift += 1; + } + const changeStart = region.a1 + leadShift; // First old line actually changed + const changeEnd = region.a2 - tailShift; + const preFrom = Math.max(1, changeStart - DIFF_CONTEXT_LINES); + const postTo = Math.min(total, changeEnd + DIFF_CONTEXT_LINES); + const lines: string[] = []; + for (let n = preFrom; n < changeStart; n += 1) lines.push(capDiffLine(" ", oldLines[n - 1]!)); + for (const l of minus) lines.push(capDiffLine("-", l)); + for (const l of plus) lines.push(capDiffLine("+", l)); + for (let n = changeEnd + 1; n <= postTo; n += 1) lines.push(capDiffLine(" ", oldLines[n - 1]!)); + const contextCount = changeStart - preFrom + (postTo - changeEnd); + hunks.push({ + hunk: { + oldStart: preFrom, + oldCount: contextCount + minus.length, + newStart: preFrom + lineDelta, + newCount: contextCount + plus.length, + lines, + }, + sites: region.sites, + }); + lineDelta += plus.length - minus.length; + } + return { hunks }; +} + +export type LineDiffResult = + | { kind: "hunks"; hunks: DiffHunk[]; plus: number; minus: number } + | { kind: "too-large"; plus: number; minus: number } + | { kind: "identical" }; + +/** + * Line diff between two full contents (write_file overwrite): trims the common + * prefix/suffix, LCS-diffs the middle, and groups changes into context-3 hunks. When the + * middle is too large to diff cheaply, reports the middle sizes as a `+X/−Y` summary + * instead. + */ +export function buildLineDiffHunks(oldContent: string, newContent: string): LineDiffResult { + if (oldContent === newContent) return { kind: "identical" }; + const a = contentLines(oldContent); + const b = contentLines(newContent); + let prefix = 0; + while (prefix < a.length && prefix < b.length && a[prefix] === b[prefix]) prefix += 1; + let suffix = 0; + while ( + suffix < a.length - prefix && + suffix < b.length - prefix && + a[a.length - 1 - suffix] === b[b.length - 1 - suffix] + ) { + suffix += 1; + } + const midA = a.slice(prefix, a.length - suffix); + const midB = b.slice(prefix, b.length - suffix); + if (midA.length * midB.length > MAX_LCS_CELLS) { + return { kind: "too-large", plus: midB.length, minus: midA.length }; + } + + // LCS table over the middles, then backtrack into per-line ops. + const n = midA.length; + const m = midB.length; + const width = m + 1; + const table = new Int32Array((n + 1) * width); + for (let i = n - 1; i >= 0; i -= 1) { + for (let j = m - 1; j >= 0; j -= 1) { + table[i * width + j] = + midA[i] === midB[j] + ? table[(i + 1) * width + j + 1]! + 1 + : Math.max(table[(i + 1) * width + j]!, table[i * width + j + 1]!); + } + } + // Ops over the middle region: "keep" advances both sides, "del"/"ins" one side. + const ops: ("keep" | "del" | "ins")[] = []; + let i = 0; + let j = 0; + while (i < n && j < m) { + if (midA[i] === midB[j]) { + ops.push("keep"); + i += 1; + j += 1; + } else if (table[(i + 1) * width + j]! >= table[i * width + j + 1]!) { + ops.push("del"); + i += 1; + } else { + ops.push("ins"); + j += 1; + } + } + while (i < n) { + ops.push("del"); + i += 1; + } + while (j < m) { + ops.push("ins"); + j += 1; + } + + // Group changed ops into hunks with DIFF_CONTEXT_LINES of context (merging when the + // context ranges touch). Walk ops tracking old/new line cursors over the FULL files. + interface Pending { + startOld: number; // 1-based old line of the first display line + startNew: number; + lines: string[]; + oldCount: number; + newCount: number; + plus: number; + minus: number; + trailingContext: number; + } + const hunks: DiffHunk[] = []; + let totalPlus = 0; + let totalMinus = 0; + let pending: Pending | null = null; + let oldLine = prefix + 1; // 1-based cursors positioned at the middle's start + let newLine = prefix + 1; + // Context ring of the most recent unchanged lines before a change. + const ring: { text: string; oldLine: number; newLine: number }[] = []; + + const flush = (): void => { + if (!pending) return; + // Drop surplus trailing context beyond the window. + while (pending.trailingContext > DIFF_CONTEXT_LINES) { + pending.lines.pop(); + pending.oldCount -= 1; + pending.newCount -= 1; + pending.trailingContext -= 1; + } + hunks.push({ + oldStart: pending.startOld, + oldCount: pending.oldCount, + newStart: pending.startNew, + newCount: pending.newCount, + lines: pending.lines, + }); + pending = null; + }; + + const ensurePending = (): Pending => { + if (pending) { + pending.trailingContext = 0; + return pending; + } + const ctx = ring.slice(-DIFF_CONTEXT_LINES); + pending = { + startOld: ctx.length > 0 ? ctx[0]!.oldLine : oldLine, + startNew: ctx.length > 0 ? ctx[0]!.newLine : newLine, + lines: ctx.map((c) => capDiffLine(" ", c.text)), + oldCount: ctx.length, + newCount: ctx.length, + plus: 0, + minus: 0, + trailingContext: 0, + }; + return pending; + }; + + const onKeep = (text: string): void => { + if (pending) { + if (pending.trailingContext >= 2 * DIFF_CONTEXT_LINES) { + // Far enough from the last change: close the hunk (surplus context trimmed in + // flush; the trimmed lines are not re-fed into the ring, so two changes 8-13 + // lines apart may show slightly less than full leading context on the second + // hunk — a presentation nuance only, the numbers stay exact). + flush(); + ring.length = 0; + } else { + pending.lines.push(capDiffLine(" ", text)); + pending.oldCount += 1; + pending.newCount += 1; + pending.trailingContext += 1; + } + } + if (!pending) { + ring.push({ text, oldLine, newLine }); + if (ring.length > DIFF_CONTEXT_LINES) ring.shift(); + } + oldLine += 1; + newLine += 1; + }; + + // Prefix/suffix lines are unchanged context feeding the ring only near the middle; + // seed the ring with the tail of the common prefix. + for (let k = Math.max(0, prefix - DIFF_CONTEXT_LINES); k < prefix; k += 1) { + ring.push({ text: a[k]!, oldLine: k + 1, newLine: k + 1 }); + } + + let ai = prefix; + let bi = prefix; + for (const op of ops) { + if (op === "keep") { + onKeep(a[ai]!); + ai += 1; + bi += 1; + } else if (op === "del") { + const h = ensurePending(); + h.lines.push(capDiffLine("-", a[ai]!)); + h.oldCount += 1; + h.minus += 1; + totalMinus += 1; + ai += 1; + oldLine += 1; + } else { + const h = ensurePending(); + h.lines.push(capDiffLine("+", b[bi]!)); + h.newCount += 1; + h.plus += 1; + totalPlus += 1; + bi += 1; + newLine += 1; + } + } + // Trailing suffix lines: at most the context window matters. + for (let k = 0; k < Math.min(suffix, DIFF_CONTEXT_LINES + 1); k += 1) { + onKeep(a[a.length - suffix + k]!); + } + flush(); + return { kind: "hunks", hunks, plus: totalPlus, minus: totalMinus }; +} diff --git a/packages/core/src/environment/tools/edit-file.ts b/packages/core/src/environment/tools/edit-file.ts new file mode 100644 index 0000000..5e762a8 --- /dev/null +++ b/packages/core/src/environment/tools/edit-file.ts @@ -0,0 +1,200 @@ +/** + * edit_file — exact-string file editing tool, a builtin tool implementation (BuiltinTool). + * + * Replaces `old_string` with `new_string` in an existing file. `old_string` must match the + * file content exactly (including whitespace/indentation) and, unless `replace_all` is set, + * occur exactly once — zero or multiple occurrences fail with an explanation telling the + * model to fix the match or widen the context. On success the output confirms the + * replacement count and shows a git-style unified diff of the changed regions (one hunk + * per replacement site, nearby sites merged; capped for replace_all storms), so both the + * model and the user can verify exactly what changed without re-reading the file. The + * write is atomic (temp file + rename, preserving the original permission bits), so a + * crash mid-write cannot leave the file half-edited. Relative paths resolve against the + * Workspace; absolute paths are allowed (tools run with the user's full permissions, same + * as the shell tool). + * + * Division of responsibility with Environment (see environment.ts): non-streaming — yields + * one final text delta; failures are explanatory text finalized as `failed`; anything + * unexpected that still throws is caught by Environment and likewise finalized as failed. + * If interrupted, only reports `aborted` — the interruption note is appended by + * Environment. + * Docs: /docs/tools § "File tools". + */ +import path from "node:path"; +import { readFile, stat } from "node:fs/promises"; +import { partialToolCallOutput } from "../../omnimessage/index.js"; +import type { OmniMessage } from "../../omnimessage/index.js"; +import type { ToolDefinitionConfig } from "../../interfaces.js"; +import type { BuiltinTool, ToolExecutionContext, ToolResult } from "./types.js"; +import { atomicWriteFile } from "./file-utils.js"; +import { buildReplacementHunks, renderHunk } from "./diff.js"; + +/** Tool name constant (used only within this tool module, never exposed to Environment). */ +export const EDIT_FILE_NAME = "edit_file"; + +/** Max diff hunks shown in the result (replace_all over a large file stays readable). */ +const MAX_DIFF_HUNKS = 5; + +/** Output-budget headroom reserved for the trailing "…and N more replacements" note. */ +const NOTE_RESERVE = 120; + +/** Fallback output budget when the definition carries no maxOutputLength (mirrors the default config entry). */ +const DEFAULT_OUTPUT_BUDGET = 16000; + +/** Counts non-overlapping occurrences of `needle` in `haystack` (needle is non-empty here). */ +function countOccurrences(haystack: string, needle: string): number { + let count = 0; + let index = haystack.indexOf(needle); + while (index !== -1) { + count += 1; + index = haystack.indexOf(needle, index + needle.length); + } + return count; +} + +/** + * edit_file builtin tool: reads the file, validates the uniqueness of `old_string`, + * writes the replaced content back atomically, and reports a unified diff of the change. + * `definition` is overridden by Environment at construction time with the same-named entry + * from ToolConfig (description/arguments/permissions/limits). + */ +export function createEditFileTool(definition: ToolDefinitionConfig): BuiltinTool { + return { + name: definition.name, + definition, + async *execute( + args: Record, + ctx: ToolExecutionContext, + ): AsyncGenerator { + const { toolCallId, signal } = ctx; + const delta = (output: string): OmniMessage => + partialToolCallOutput({ eventType: "delta", output, toolCallId }); + + const filePath = args["file_path"]; + if (typeof filePath !== "string" || filePath.length === 0) { + yield delta(`Missing required argument "file_path" for ${definition.name}.`); + return { stopReason: "failed" }; + } + const oldString = args["old_string"]; + if (typeof oldString !== "string") { + yield delta(`Missing required argument "old_string" for ${definition.name}.`); + return { stopReason: "failed" }; + } + if (oldString.length === 0) { + yield delta( + "old_string must not be empty — edit_file replaces existing text. To create a file or rewrite it wholesale, use write_file.", + ); + return { stopReason: "failed" }; + } + const newString = args["new_string"]; + if (typeof newString !== "string") { + yield delta(`Missing required argument "new_string" for ${definition.name}.`); + return { stopReason: "failed" }; + } + if (oldString === newString) { + yield delta( + "old_string and new_string are identical — nothing to change. Make new_string the desired replacement text.", + ); + return { stopReason: "failed" }; + } + const replaceAll = args["replace_all"] === true; + + const resolved = path.resolve(ctx.workspaceDir, filePath); + let content: string; + let fileMode: number | undefined; + try { + const st = await stat(resolved); + if (st.isDirectory()) { + yield delta(`Cannot edit "${filePath}": it is a directory.`); + return { stopReason: "failed" }; + } + fileMode = st.mode & 0o777; + content = await readFile(resolved, { encoding: "utf8", ...(signal ? { signal } : {}) }); + } catch (err) { + if (signal?.aborted) return { stopReason: "aborted" }; + const code = (err as NodeJS.ErrnoException).code; + if (code === "ENOENT") { + yield delta( + `File not found: "${filePath}". edit_file only edits existing files — check the path, or use write_file to create it.`, + ); + } else { + const message = err instanceof Error ? err.message : String(err); + yield delta(`Failed to read "${filePath}": ${message}`); + } + return { stopReason: "failed" }; + } + if (signal?.aborted) return { stopReason: "aborted" }; + + const occurrences = countOccurrences(content, oldString); + if (occurrences === 0) { + // A CRLF file is the classic silent mismatch: text copied from read_file's display + // has bare \n while the file has \r\n — say so explicitly. + const crlfHint = content.includes("\r\n") + ? " Note: the file uses CRLF (\\r\\n) line endings — a multi-line old_string must include the \\r characters." + : ""; + yield delta( + `old_string not found in "${filePath}". Make sure it matches the file content exactly, including whitespace and indentation.${crlfHint}`, + ); + return { stopReason: "failed" }; + } + if (occurrences > 1 && !replaceAll) { + yield delta( + `old_string occurs ${occurrences} times in "${filePath}". Add surrounding context to make it unique, or set replace_all to true to replace every occurrence.`, + ); + return { stopReason: "failed" }; + } + + const replaceStart = content.indexOf(oldString); + const newContent = replaceAll + ? content.split(oldString).join(newString) + : content.slice(0, replaceStart) + + newString + + content.slice(replaceStart + oldString.length); + try { + await atomicWriteFile(resolved, newContent, { + ...(fileMode !== undefined ? { mode: fileMode } : {}), + ...(signal ? { signal } : {}), + }); + } catch (err) { + if (signal?.aborted) return { stopReason: "aborted" }; + const message = err instanceof Error ? err.message : String(err); + yield delta(`Failed to write "${filePath}": ${message}`); + return { stopReason: "failed" }; + } + + const replaced = replaceAll ? occurrences : 1; + // Git-style unified diff of the changed regions, self-budgeted below the tool's + // output cap so the leading summary line (and the elision note) always survive + // Environment's front-keep truncation. + const { hunks } = buildReplacementHunks( + content, + oldString, + newString, + replaceAll, + MAX_DIFF_HUNKS, + ); + const budget = + definition.maxOutputLength !== undefined && definition.maxOutputLength > 0 + ? definition.maxOutputLength + : DEFAULT_OUTPUT_BUDGET; + const out: string[] = [ + `Replaced ${replaced} occurrence${replaced === 1 ? "" : "s"} in "${filePath}".`, + ]; + let used = out[0]!.length; + let shownSites = 0; + for (const { hunk, sites } of hunks) { + const rendered = renderHunk(hunk); + if (used + 1 + rendered.length > budget - NOTE_RESERVE) break; + out.push(rendered); + used += 1 + rendered.length; + shownSites += sites; + } + const omitted = replaced - shownSites; + if (omitted > 0) { + out.push(`…and ${omitted} more replacement${omitted === 1 ? "" : "s"}`); + } + yield delta(out.join("\n")); + return; + }, + }; +} diff --git a/packages/core/src/environment/tools/exec-command.ts b/packages/core/src/environment/tools/exec-command.ts index 8f443f7..f4ec85e 100644 --- a/packages/core/src/environment/tools/exec-command.ts +++ b/packages/core/src/environment/tools/exec-command.ts @@ -33,7 +33,9 @@ export const EXEC_COMMAND_NAME = "exec_command"; * exec_command built-in tool: parses arguments, resolves workdir, and delegates to * `CommandSessionManager` to spawn the process and collect output. * `definition` is overridden by Environment at construction time with the matching entry - * from ToolConfig (description/parameters/permission/limits). + * from ToolConfig (description/parameters/permission/limits); the runtime tool name and + * self-referential messages follow `definition.name` (the config entry is the single + * source of truth for naming). * `services.commandSessions` is injected by Environment (shares the same registry with * input_command). */ @@ -43,7 +45,7 @@ export function createExecCommandTool( ): BuiltinTool { const manager = services?.commandSessions; return { - name: EXEC_COMMAND_NAME, + name: definition.name, definition, async *execute( args: Record, @@ -54,13 +56,13 @@ export function createExecCommandTool( partialToolCallOutput({ eventType: "delta", output, toolCallId }); if (!manager) { - yield delta("[exec_command unavailable: no command session manager configured]"); + yield delta(`[${definition.name} unavailable: no command session manager configured]`); return { stopReason: "failed" }; } const cmd = args["cmd"]; if (typeof cmd !== "string" || cmd.length === 0) { - yield delta('Missing required argument "cmd" for exec_command.'); + yield delta(`Missing required argument "cmd" for ${definition.name}.`); return { stopReason: "failed" }; } // workdir defaults to workspaceDir; relative paths are resolved against workspaceDir. diff --git a/packages/core/src/environment/tools/file-utils.ts b/packages/core/src/environment/tools/file-utils.ts new file mode 100644 index 0000000..0034119 --- /dev/null +++ b/packages/core/src/environment/tools/file-utils.ts @@ -0,0 +1,33 @@ +/** + * Shared helpers for the file tools (edit_file / write_file). + */ +import path from "node:path"; +import { rename, rm, writeFile } from "node:fs/promises"; + +/** + * Atomic file write: writes to a temp file in the target's directory, then renames it over + * the target — a crash or full disk mid-write can no longer leave the target half-written. + * `mode` preserves an existing file's permission bits (rename replaces the inode, which + * would otherwise reset them); the temp file is removed on failure (best effort). + */ +export async function atomicWriteFile( + target: string, + content: string, + opts: { mode?: number; signal?: AbortSignal } = {}, +): Promise { + const tmp = path.join( + path.dirname(target), + `.${path.basename(target)}.tmp-${process.pid}-${Math.random().toString(36).slice(2, 8)}`, + ); + try { + await writeFile(tmp, content, { + encoding: "utf8", + ...(opts.mode !== undefined ? { mode: opts.mode } : {}), + ...(opts.signal ? { signal: opts.signal } : {}), + }); + await rename(tmp, target); + } catch (err) { + await rm(tmp, { force: true }).catch(() => undefined); + throw err; + } +} diff --git a/packages/core/src/environment/tools/read-file.ts b/packages/core/src/environment/tools/read-file.ts new file mode 100644 index 0000000..e1b7c09 --- /dev/null +++ b/packages/core/src/environment/tools/read-file.ts @@ -0,0 +1,365 @@ +/** + * read_file — text-file reading tool, a builtin tool implementation (BuiltinTool). + * + * Reads a text file and returns it in `cat -n` style (line number, tab, content), so the + * model can quote exact lines back to edit_file. Relative paths resolve against the + * Workspace; absolute paths are allowed (tools run with the user's full permissions, same + * as the shell tool). An optional 1-based `offset` and a `limit` (default 2000 lines) form + * a window for paging through long files. + * + * Robustness properties: + * - **Bounded read**: the file is scanned incrementally through a file handle, never loaded + * whole — a multi-GB log cannot balloon the process. A hard scan cap (8MB) turns + * pathological requests (offsets deeper than the cap, files with no newlines) into a + * clean failure with guidance; a window that already produced lines when the cap hits is + * returned as a partial result with a lower-bound line count instead. + * - **Self-budgeted output**: the rendered window is trimmed to the tool's own + * maxOutputLength before Environment's front-keep truncation could cut the trailing + * continuation note — the note (and the shown range it reports) always survives. + * - Overlong single lines are truncated with a marker; CRLF files display without the `\r` + * and get an explicit note; binary content (NUL bytes) is rejected with advice; the + * secret stores (.vault.toml / .project_config.toml) are refused outright. + * + * Division of responsibility with Environment (see environment.ts): non-streaming — yields + * the whole numbered listing as one delta; failures (missing file, directory, binary + * content, scan cap) are explanatory text finalized as `failed`; anything unexpected that + * still throws is caught by Environment and likewise finalized as failed. If interrupted, + * only reports `aborted` — the interruption note is appended by Environment. + * Docs: /docs/tools § "File tools". + */ +import path from "node:path"; +import { open, stat } from "node:fs/promises"; +import { partialToolCallOutput } from "../../omnimessage/index.js"; +import type { OmniMessage } from "../../omnimessage/index.js"; +import type { ToolDefinitionConfig } from "../../interfaces.js"; +import type { BuiltinTool, ToolExecutionContext, ToolResult } from "./types.js"; + +/** Tool name constant (used only within this tool module, never exposed to Environment). */ +export const READ_FILE_NAME = "read_file"; + +/** Default max number of lines returned per call (overridable via the `limit` argument). */ +export const DEFAULT_READ_FILE_LIMIT = 2000; + +/** Max characters kept of a single line; the rest is replaced by a truncation marker. */ +export const MAX_LINE_LENGTH = 2000; + +/** Hard cap on bytes scanned per call: beyond it the tool stops instead of grinding through a huge file. */ +export const READ_FILE_SCAN_CAP_BYTES = 8 * 1024 * 1024; + +/** Bytes read per file-handle read (scan granularity; also the abort-signal check interval). */ +const CHUNK_BYTES = 256 * 1024; + +/** Per-line byte retention cap: at 4 bytes/char worst-case UTF-8 this always decodes to >= MAX_LINE_LENGTH chars, so char truncation stays exact. */ +const LINE_BYTE_CAP = MAX_LINE_LENGTH * 4; + +/** Output-budget headroom reserved for the trailing notes (continuation / CRLF), so they survive self-budget trimming. */ +const NOTE_RESERVE = 256; + +/** Fallback output budget when the definition carries no maxOutputLength (mirrors the default config entry). */ +const DEFAULT_OUTPUT_BUDGET = 64000; + +/** + * Secret stores the system prompt bans the model from reading; read_file refuses them by + * basename regardless of directory. The tool needs its own guard because `permission: "r"` + * makes it auto-approved under read-only approval (the shell tool stays rw-gated). + */ +const SECRET_BASENAMES = new Set([".vault.toml", ".project_config.toml"]); + +/** Renders one `cat -n` style line: 6-column right-aligned line number, tab, content. */ +export function numberedLine(lineNo: number, content: string): string { + const capped = + content.length > MAX_LINE_LENGTH + ? `${content.slice(0, MAX_LINE_LENGTH)}… [line truncated]` + : content; + return `${String(lineNo).padStart(6)}\t${capped}`; +} + +/** Coerces a count argument (offset/limit): numbers and numeric strings are accepted; anything else is rejected. */ +function coerceCount( + value: unknown, + fallback: number, +): { ok: true; value: number } | { ok: false } { + if (value === undefined || value === null) return { ok: true, value: fallback }; + if (typeof value === "number" && Number.isFinite(value)) { + return { ok: true, value: Math.floor(value) }; + } + if (typeof value === "string" && value.trim() !== "") { + const n = Number(value.trim()); + if (Number.isFinite(n)) return { ok: true, value: Math.floor(n) }; + } + return { ok: false }; +} + +/** Result of the bounded window scan. */ +interface ScanOutcome { + /** Decoded window lines (trailing \r already stripped), in order starting at `offset`. */ + lines: string[]; + /** Total line count — exact when `totalKnown`, otherwise a lower bound (lines seen before the scan cap). */ + total: number; + totalKnown: boolean; + /** Whether any CRLF (\r\n) line ending was seen in the scanned range. */ + sawCRLF: boolean; + /** Whether a NUL byte was seen (binary content). */ + binary: boolean; + /** Scan cap hit before the window produced any line: the request cannot be served. */ + capBeforeWindow: boolean; + aborted: boolean; +} + +/** + * Incremental windowed scan: reads the file in chunks through a file handle, splitting on + * `\n` at the byte level (always safe in UTF-8), storing only lines inside + * [offset, offset+limit-1] (each retained up to LINE_BYTE_CAP bytes) and counting the rest. + * After the window fills, it keeps counting lines until EOF or the scan cap so the + * continuation note can report an exact total when cheap and a lower bound otherwise. + */ +async function scanWindow( + filePath: string, + offset: number, + limit: number, + signal?: AbortSignal, +): Promise { + const end = offset + limit - 1; + const out: ScanOutcome = { + lines: [], + total: 0, + totalKnown: false, + sawCRLF: false, + binary: false, + capBeforeWindow: false, + aborted: false, + }; + let completedLines = 0; // Lines terminated by \n so far + let currentHasBytes = false; // Whether the in-progress line has any content + let currentParts: Buffer[] = []; // Retained bytes of the in-progress line (window lines only) + let currentBytes = 0; + let scanned = 0; + + const inWindow = (): boolean => completedLines + 1 >= offset && completedLines + 1 <= end; + + const appendRun = (buf: Buffer, from: number, to: number): void => { + if (currentBytes >= LINE_BYTE_CAP) return; // Overlong line: keep only the head (display truncates anyway) + const take = Math.min(LINE_BYTE_CAP - currentBytes, to - from); + currentParts.push(Buffer.from(buf.subarray(from, from + take))); + currentBytes += take; + }; + + const finishLine = (): void => { + if (inWindow()) { + let text = Buffer.concat(currentParts).toString("utf8"); + if (text.endsWith("\r")) text = text.slice(0, -1); + out.lines.push(text); + } + completedLines += 1; + currentParts = []; + currentBytes = 0; + currentHasBytes = false; + }; + + const fd = await open(filePath, "r"); + try { + const chunk = Buffer.alloc(CHUNK_BYTES); + let prevByte = -1; // For CRLF detection across chunk boundaries + for (;;) { + if (signal?.aborted) { + out.aborted = true; + return out; + } + const toRead = Math.min(CHUNK_BYTES, READ_FILE_SCAN_CAP_BYTES - scanned); + if (toRead <= 0) { + // Scan cap: with no window line produced the request cannot be served (fails with + // guidance); with a partial window, return it plus a lower-bound total. + out.capBeforeWindow = out.lines.length === 0; + out.total = completedLines + (currentHasBytes ? 1 : 0); + return out; + } + const { bytesRead } = await fd.read(chunk, 0, toRead, scanned); + if (bytesRead === 0) { + // EOF: a final line without a trailing newline still counts. + if (currentHasBytes) finishLine(); + out.total = completedLines; + out.totalKnown = true; + return out; + } + scanned += bytesRead; + let from = 0; + for (let i = 0; i < bytesRead; i += 1) { + const b = chunk[i]!; + if (b === 0) { + out.binary = true; + return out; + } + if (b === 0x0a) { + if (prevByte === 0x0d) out.sawCRLF = true; + if (i > from) { + currentHasBytes = true; + if (inWindow()) appendRun(chunk, from, i); + } + finishLine(); + from = i + 1; + } + prevByte = b; + } + if (from < bytesRead) { + currentHasBytes = true; + if (inWindow()) appendRun(chunk, from, bytesRead); + } + } + } finally { + await fd.close(); + } +} + +/** + * read_file builtin tool: resolves the path against the Workspace, validates it is a + * readable text file, and outputs the requested line window with line numbers. + * `definition` is overridden by Environment at construction time with the same-named entry + * from ToolConfig (description/arguments/permissions/limits). + */ +export function createReadFileTool(definition: ToolDefinitionConfig): BuiltinTool { + return { + name: definition.name, + definition, + async *execute( + args: Record, + ctx: ToolExecutionContext, + ): AsyncGenerator { + const { toolCallId, signal } = ctx; + const delta = (output: string): OmniMessage => + partialToolCallOutput({ eventType: "delta", output, toolCallId }); + + const filePath = args["file_path"]; + if (typeof filePath !== "string" || filePath.length === 0) { + yield delta(`Missing required argument "file_path" for ${definition.name}.`); + return { stopReason: "failed" }; + } + const offsetArg = coerceCount(args["offset"], 1); + if (!offsetArg.ok) { + yield delta(`Invalid "offset": expected a number (got ${JSON.stringify(args["offset"])}).`); + return { stopReason: "failed" }; + } + const offset = Math.max(1, offsetArg.value); + const limitArg = coerceCount(args["limit"], DEFAULT_READ_FILE_LIMIT); + if (!limitArg.ok || limitArg.value <= 0) { + yield delta( + `Invalid "limit": expected a positive number (got ${JSON.stringify(args["limit"])}).`, + ); + return { stopReason: "failed" }; + } + const limit = limitArg.value; + + const resolved = path.resolve(ctx.workspaceDir, filePath); + // Secret stores are refused by name: read_file is auto-approved under read-only + // approval, so it needs its own guard (aligned with the system prompt's ban). + if (SECRET_BASENAMES.has(path.basename(resolved))) { + yield delta( + `Refusing to read "${filePath}": ${path.basename(resolved)} holds the user's secrets and must never enter the conversation.`, + ); + return { stopReason: "failed" }; + } + + let size: number; + try { + const st = await stat(resolved); + if (st.isDirectory()) { + yield delta( + `Cannot read "${filePath}": it is a directory. Pass the path of a file inside it.`, + ); + return { stopReason: "failed" }; + } + size = st.size; + } catch (err) { + if (signal?.aborted) return { stopReason: "aborted" }; + const code = (err as NodeJS.ErrnoException).code; + if (code === "ENOENT") { + yield delta( + `File not found: "${filePath}". Check the path — relative paths resolve against the workspace (${ctx.workspaceDir}).`, + ); + } else { + const message = err instanceof Error ? err.message : String(err); + yield delta(`Failed to read "${filePath}": ${message}`); + } + return { stopReason: "failed" }; + } + if (signal?.aborted) return { stopReason: "aborted" }; + if (size === 0) { + yield delta(`"${filePath}" is an empty file (0 lines).`); + return; + } + + let scan: ScanOutcome; + try { + scan = await scanWindow(resolved, offset, limit, signal); + } catch (err) { + if (signal?.aborted) return { stopReason: "aborted" }; + const message = err instanceof Error ? err.message : String(err); + yield delta(`Failed to read "${filePath}": ${message}`); + return { stopReason: "failed" }; + } + if (scan.aborted || signal?.aborted) return { stopReason: "aborted" }; + if (scan.binary) { + yield delta( + `"${filePath}" looks like a binary file (contains NUL bytes). Use shell commands to inspect it, or read_image if it is an image.`, + ); + return { stopReason: "failed" }; + } + if (scan.capBeforeWindow) { + const mb = Math.round(READ_FILE_SCAN_CAP_BYTES / (1024 * 1024)); + yield delta( + `Stopped after scanning ${mb} MB of "${filePath}" without reaching the requested window ` + + `(scanned ${scan.total} line${scan.total === 1 ? "" : "s"}). The offset is too deep or the file has extremely long ` + + `lines — use shell commands (e.g. sed -n '${offset},${offset + limit - 1}p') for this file.`, + ); + return { stopReason: "failed" }; + } + if (scan.totalKnown && offset > scan.total) { + yield delta( + `Offset ${offset} is past the end of "${filePath}" (${scan.total} line${scan.total === 1 ? "" : "s"} total).`, + ); + return { stopReason: "failed" }; + } + + // Render the window, self-budgeted below the tool's output cap so the trailing notes + // are never cut by Environment's front-keep truncation. + const budget = + definition.maxOutputLength !== undefined && definition.maxOutputLength > 0 + ? definition.maxOutputLength + : DEFAULT_OUTPUT_BUDGET; + const contentBudget = Math.max(1, budget - NOTE_RESERVE); + const rows: string[] = []; + let used = 0; + let shownEnd = offset - 1; + let budgetTrimmed = false; + for (let i = 0; i < scan.lines.length; i += 1) { + const row = numberedLine(offset + i, scan.lines[i]!); + const cost = row.length + (rows.length > 0 ? 1 : 0); + if (used + cost > contentBudget) { + if (rows.length === 0) { + // Even the first row overflows a (tiny custom) budget: hard-cut it so the notes survive. + rows.push(row.slice(0, contentBudget)); + shownEnd = offset + i; + } + budgetTrimmed = true; + break; + } + used += cost; + rows.push(row); + shownEnd = offset + i; + } + + const moreRemains = budgetTrimmed || !scan.totalKnown || shownEnd < scan.total; + if (scan.sawCRLF) rows.push("(file uses CRLF line endings)"); + if (moreRemains) { + const trimmedInfix = budgetTrimmed ? " (output limit reached)" : ""; + const totalPart = scan.totalKnown + ? `file has ${scan.total} lines total` + : `file has more than ${scan.total} lines`; + rows.push( + `[${totalPart}; showing ${offset}-${shownEnd}${trimmedInfix} — call again with offset to continue]`, + ); + } + yield delta(rows.join("\n")); + return; + }, + }; +} diff --git a/packages/core/src/environment/tools/registry.ts b/packages/core/src/environment/tools/registry.ts index a9855b5..f9cef06 100644 --- a/packages/core/src/environment/tools/registry.ts +++ b/packages/core/src/environment/tools/registry.ts @@ -4,7 +4,7 @@ * Environment uses this table to assemble entries from ToolConfig into BuiltinTool instances: * a tool is only assembled if its name is in the table (i.e. a supported built-in tool); the * description/parameters/permission/maxOutputLength from config are injected into the tool's - * `definition` by each factory. + * `definition` by each factory, and the runtime tool name follows the config entry's name. * When adding a new built-in tool, just register one factory entry here — no changes to * Environment needed. * @@ -13,6 +13,9 @@ */ import type { EnvironmentServices, ToolDefinitionConfig } from "../../interfaces.js"; import type { BuiltinTool } from "./types.js"; +import { READ_FILE_NAME, createReadFileTool } from "./read-file.js"; +import { EDIT_FILE_NAME, createEditFileTool } from "./edit-file.js"; +import { WRITE_FILE_NAME, createWriteFileTool } from "./write-file.js"; import { EXEC_COMMAND_NAME, createExecCommandTool } from "./exec-command.js"; import { INPUT_COMMAND_NAME, createInputCommandTool } from "./input-command.js"; import { SUBAGENT_NAME, createSubagentTool } from "./run-subagent.js"; @@ -32,6 +35,9 @@ export type BuiltinToolFactory = ( /** Tool name -> factory. */ export const BUILTIN_TOOL_FACTORIES: Record = { + [READ_FILE_NAME]: createReadFileTool, + [EDIT_FILE_NAME]: createEditFileTool, + [WRITE_FILE_NAME]: createWriteFileTool, [EXEC_COMMAND_NAME]: createExecCommandTool, [INPUT_COMMAND_NAME]: createInputCommandTool, [SUBAGENT_NAME]: createSubagentTool, diff --git a/packages/core/src/environment/tools/write-file.ts b/packages/core/src/environment/tools/write-file.ts new file mode 100644 index 0000000..2592f63 --- /dev/null +++ b/packages/core/src/environment/tools/write-file.ts @@ -0,0 +1,161 @@ +/** + * write_file — whole-file writing tool, a builtin tool implementation (BuiltinTool). + * + * Writes `content` to a file, creating it (including missing parent directories) or + * overwriting it entirely; an empty string is a valid content (creates an empty file). + * The output distinguishes "Created" from "Overwrote" and reports the size written, so + * the model notices when it clobbered an existing file; an overwrite additionally shows a + * git-style unified diff against the previous content when the change is small, and a + * one-line `+X/−Y lines` summary otherwise (created files carry no diff — the model just + * supplied the content). The write is atomic (temp file + rename, preserving an + * overwritten file's permission bits), so a crash mid-write cannot leave the target + * half-written. Relative paths resolve against the Workspace; absolute + * paths are allowed (tools run with the user's full permissions, same as the shell tool). + * For surgical changes to an existing file, edit_file is the better tool — this one + * replaces the whole content. + * + * Division of responsibility with Environment (see environment.ts): non-streaming — yields + * one final text delta; failures (path is a directory, permission errors) are explanatory + * text finalized as `failed`; anything unexpected that still throws is caught by + * Environment and likewise finalized as failed. If interrupted, only reports `aborted` — + * the interruption note is appended by Environment. + * Docs: /docs/tools § "File tools". + */ +import path from "node:path"; +import { mkdir, readFile, stat } from "node:fs/promises"; +import { atomicWriteFile } from "./file-utils.js"; +import { buildLineDiffHunks, renderHunk } from "./diff.js"; +import { partialToolCallOutput } from "../../omnimessage/index.js"; +import type { OmniMessage } from "../../omnimessage/index.js"; +import type { ToolDefinitionConfig } from "../../interfaces.js"; +import type { BuiltinTool, ToolExecutionContext, ToolResult } from "./types.js"; + +/** Tool name constant (used only within this tool module, never exposed to Environment). */ +export const WRITE_FILE_NAME = "write_file"; + +/** Max bytes of previous content read back for the overwrite diff; larger files get no diff. */ +const DIFF_SOURCE_CAP_BYTES = 1024 * 1024; + +/** Max rendered diff lines (headers included) shown inline; larger diffs collapse to a +X/−Y summary. */ +const MAX_DIFF_DISPLAY_LINES = 60; + +/** Output-budget headroom kept below the tool's maxOutputLength when appending the diff. */ +const NOTE_RESERVE = 120; + +/** Fallback output budget when the definition carries no maxOutputLength (mirrors the default config entry). */ +const DEFAULT_OUTPUT_BUDGET = 16000; + +/** Counts content lines the way `cat -n` numbers them: a trailing newline ends the last line instead of adding an empty one. */ +function countLines(content: string): number { + if (content === "") return 0; + const lines = content.split("\n"); + return lines[lines.length - 1] === "" ? lines.length - 1 : lines.length; +} + +/** + * write_file builtin tool: creates parent directories as needed and writes the full + * content, reporting created/overwrote plus the written size. + * `definition` is overridden by Environment at construction time with the same-named entry + * from ToolConfig (description/arguments/permissions/limits). + */ +export function createWriteFileTool(definition: ToolDefinitionConfig): BuiltinTool { + return { + name: definition.name, + definition, + async *execute( + args: Record, + ctx: ToolExecutionContext, + ): AsyncGenerator { + const { toolCallId, signal } = ctx; + const delta = (output: string): OmniMessage => + partialToolCallOutput({ eventType: "delta", output, toolCallId }); + + const filePath = args["file_path"]; + if (typeof filePath !== "string" || filePath.length === 0) { + yield delta(`Missing required argument "file_path" for ${definition.name}.`); + return { stopReason: "failed" }; + } + // An empty string is valid content (creates an empty file); only a missing/non-string + // value is an argument error. + const content = args["content"]; + if (typeof content !== "string") { + yield delta(`Missing required argument "content" for ${definition.name}.`); + return { stopReason: "failed" }; + } + + const resolved = path.resolve(ctx.workspaceDir, filePath); + // Determine created-vs-overwrote before writing; also reject directories up front + // (writeFile's raw EISDIR is not model-friendly). + let existed = false; + let fileMode: number | undefined; + let previous: string | null = null; // Previous content, for the overwrite diff + try { + const st = await stat(resolved); + if (st.isDirectory()) { + yield delta(`Cannot write "${filePath}": it is a directory.`); + return { stopReason: "failed" }; + } + existed = true; + fileMode = st.mode & 0o777; // Preserved across the atomic temp-file + rename write + // Read the old content back for the diff — bounded: a huge or unreadable/binary + // previous file simply gets no diff (never a failure). + if (st.size <= DIFF_SOURCE_CAP_BYTES) { + const bytes = await readFile(resolved); + if (!bytes.includes(0)) previous = bytes.toString("utf8"); + } + } catch { + // Missing file (or unstatable path): proceed to create; real write errors surface below. + } + if (signal?.aborted) return { stopReason: "aborted" }; + + try { + await mkdir(path.dirname(resolved), { recursive: true }); + await atomicWriteFile(resolved, content, { + ...(fileMode !== undefined ? { mode: fileMode } : {}), + ...(signal ? { signal } : {}), + }); + } catch (err) { + if (signal?.aborted) return { stopReason: "aborted" }; + const message = err instanceof Error ? err.message : String(err); + yield delta(`Failed to write "${filePath}": ${message}`); + return { stopReason: "failed" }; + } + + const lines = countLines(content); + const bytes = Buffer.byteLength(content, "utf8"); + const out: string[] = [ + `${existed ? "Overwrote" : "Created"} "${filePath}" (${lines} line${lines === 1 ? "" : "s"}, ${bytes} byte${bytes === 1 ? "" : "s"}).`, + ]; + // Overwrites show what changed, Claude Code style: a small unified diff inline, a + // one-line +X/−Y summary otherwise. Self-budgeted below the tool's output cap so + // the leading summary line always survives Environment's front-keep truncation. + if (existed && previous !== null) { + const diff = buildLineDiffHunks(previous, content); + if (diff.kind === "identical") { + out.push("(content unchanged)"); + } else if (diff.kind === "too-large") { + out.push( + `+${diff.plus}/−${diff.minus} lines vs the previous content (diff too large to show)`, + ); + } else { + const rendered = diff.hunks.map(renderHunk); + const budget = + definition.maxOutputLength !== undefined && definition.maxOutputLength > 0 + ? definition.maxOutputLength + : DEFAULT_OUTPUT_BUDGET; + const totalLines = rendered.reduce((acc, h) => acc + h.split("\n").length, 0); + const totalChars = rendered.reduce((acc, h) => acc + h.length + 1, out[0]!.length); + if (totalLines > MAX_DIFF_DISPLAY_LINES || totalChars > budget - NOTE_RESERVE) { + out.push( + `+${diff.plus}/−${diff.minus} lines vs the previous content (diff too large to show)`, + ); + } else { + out.push(...rendered); + } + } + } + yield delta(out.join("\n")); + return; + }, + }; +} diff --git a/packages/core/src/interfaces.ts b/packages/core/src/interfaces.ts index 0b40b58..b418674 100644 --- a/packages/core/src/interfaces.ts +++ b/packages/core/src/interfaces.ts @@ -56,6 +56,16 @@ export interface ToolDefinitionConfig { timeoutMs?: number; /** Max length of tool output; Environment truncates from the front (keeping the head) if exceeded; <=0 disables it. */ maxOutputLength?: number; + /** + * Per-tool toggle for the optional `description` call argument (a model-written sentence + * shown to the user while the call runs). The argument itself is declared as a normal + * property in this entry's `parameters` (editable config is the single source of truth); + * setting `call_description: false` filters that property out of the schema handed to the + * LLM at assembly time (in-memory only — the stored YAML is never rewritten). Missing = + * true (the property stays). No effect on entries whose parameters declare no + * `description` property. + */ + call_description?: boolean; } export interface MCPServerConfig { diff --git a/packages/core/src/state/agent-state.ts b/packages/core/src/state/agent-state.ts index 3334698..98e7bfd 100644 --- a/packages/core/src/state/agent-state.ts +++ b/packages/core/src/state/agent-state.ts @@ -455,11 +455,37 @@ export function selectBuiltinToolsForModel( return tools.filter((t) => t.forModel === undefined || t.forModel === kind); } +/** + * Applies a tool entry's per-tool `call_description` toggle: the `description` call argument + * is declared as a normal property in the entry's `parameters` (editable config is the + * single source of truth) and is **required** there, so a tool that offers it always gets + * one — the frontends can then pick a call's display form up front instead of guessing while + * the arguments stream. When the entry sets `call_description: false`, the property is + * filtered out of the schema handed to the LLM, `required` along with it — on an in-memory + * clone only, the stored YAML is never rewritten. Missing/true, entries without a parameter + * schema, and entries whose properties declare no `description` all pass through unchanged + * (old configs predating the field are a no-op). + */ +function applyCallDescriptionToggle(def: ToolDefinitionConfig): ToolDefinitionConfig { + if (def.call_description !== false) return def; + const params = def.parameters; + if (params === undefined) return def; + const properties = params["properties"]; + if (properties === null || typeof properties !== "object") return def; + if (!("description" in (properties as Record))) return def; + const { description: _dropped, ...rest } = properties as Record; + const required = params["required"]; + const trimmed = Array.isArray(required) + ? { required: required.filter((name) => name !== "description") } + : {}; + return { ...def, parameters: { ...params, properties: rest, ...trimmed } }; +} + export function buildToolConfig(state: AgentState): ToolConfig { const systemTools = state.systemConfig.tools; const builtin = systemTools?.builtin ?? defaultSystemConfig().tools?.builtin ?? []; return { - customTools: builtin, + customTools: builtin.map(applyCallDescriptionToggle), mcpServers: systemTools?.mcpServers ?? [], }; } diff --git a/packages/core/src/state/default-config.ts b/packages/core/src/state/default-config.ts index 3caf0b6..7e2c48d 100644 --- a/packages/core/src/state/default-config.ts +++ b/packages/core/src/state/default-config.ts @@ -73,7 +73,7 @@ export interface SystemConfig { /** Context compaction (enabled by default, max_context_length 128k, mode summarize). */ compaction?: CompactionConfig; tools?: { - /** Built-in system tool configuration. */ + /** Built-in system tool configuration (per-entry fields incl. the `call_description` toggle live on ToolDefinitionConfig). */ builtin?: ToolDefinitionConfig[]; /** MCP Server configuration. */ mcpServers?: MCPServerConfig[]; @@ -140,7 +140,7 @@ The vault holds this agent's per-agent secrets (agent_state/.vault.toml). Each e {{VAULT_KEYS}} # Skills -Skills are reusable instruction packages stored under /agents//agent_state/skills//SKILL.md. There is no skill tool: when a task matches an installed skill below, or the user asks to use one (a message may start with a block listing skill names), first read that skill's SKILL.md in full with a shell command, then follow it. If a request only names a skill without a concrete task, ask the user what they need before starting. +Skills are reusable instruction packages stored under /agents//agent_state/skills//SKILL.md. There is no skill tool: when a task matches an installed skill below, or the user asks to use one (a message may start with a block listing skill names), first read that skill's SKILL.md in full with the read_file tool, then follow it. If a request only names a skill without a concrete task, ask the user what they need before starting. {{SKILL_METADATA}} # Environment @@ -169,21 +169,115 @@ export const DEFAULT_COMPACTION_PROMPT = "tools while writing the summary; respond with text only."; /** - * Default built-in system tools: bash execution and subagent spawning. + * Default built-in system tools: file reading/editing/writing first, then bash execution + * and subagent spawning. * Docs: /docs/tools § "Built-in tools". */ function defaultBuiltinTools(): ToolDefinitionConfig[] { return [ + { + name: "read_file", + description: + "Read a text file and return its content with line numbers (cat -n style) — the preferred " + + "way to inspect a file. Returns up to 2000 lines starting at the given offset; for longer " + + "files call again with offset to continue. Use the image tools for images and the shell " + + "tool for binary files.", + parameters: { + type: "object", + properties: { + file_path: { + type: "string", + description: "Path to the file to read; absolute, or relative to the workspace.", + }, + offset: { + type: "number", + description: "1-based line number to start reading from; defaults to 1.", + }, + limit: { + type: "number", + description: "Max lines to read; defaults to 2000.", + }, + }, + required: ["file_path"], + }, + permission: "r", + timeoutMs: 30000, + // Wider than the other tools' cap: a 2000-line window of code rarely fits in 16k characters. + maxOutputLength: 64000, + }, + { + name: "edit_file", + description: + "Edit a file by exact string replacement — the preferred way to make a precise change. " + + "old_string must match the file content exactly (including whitespace) and be unique " + + "unless replace_all is set; read the file first to copy the text verbatim. The result " + + "echoes a line-numbered snippet around the change for verification.", + parameters: { + type: "object", + properties: { + file_path: { + type: "string", + description: "Path to the file to edit; absolute, or relative to the workspace.", + }, + old_string: { + type: "string", + description: "Exact text to replace, including whitespace/indentation.", + }, + new_string: { + type: "string", + description: "Replacement text; must differ from old_string.", + }, + replace_all: { + type: "boolean", + description: "Replace every occurrence of old_string; defaults to false.", + }, + }, + required: ["file_path", "old_string", "new_string"], + }, + permission: "rw", + timeoutMs: 30000, + maxOutputLength: 16000, + }, + { + name: "write_file", + description: + "Create or overwrite a file with the given content, creating parent directories as " + + "needed. Use it for new files or full rewrites; for precise changes to an existing file " + + "prefer edit_file.", + parameters: { + type: "object", + properties: { + file_path: { + type: "string", + description: "Path to the file to write; absolute, or relative to the workspace.", + }, + content: { + type: "string", + description: "Full file content to write; an empty string creates an empty file.", + }, + }, + required: ["file_path", "content"], + }, + permission: "rw", + timeoutMs: 30000, + maxOutputLength: 16000, + }, { name: "exec_command", description: - "Run a shell command in the workspace to read, write, edit files and run programs. " + + "Run a shell command in the workspace to run programs, search, install dependencies, and " + + "everything the file tools don't cover. " + "Run long-lived commands (servers, watchers, builds) in the foreground: past yield_time_ms " + "they keep running in the background with a process_id. Do not background them with `&` — " + "the whole process group is cleaned up when the foreground command exits.", parameters: { type: "object", properties: { + description: { + type: "string", + description: + "Required, and emit it first, before the other arguments: it is shown to the user while the call runs. One short sentence describing what this call is doing and why, written in the user's language.", + }, cmd: { type: "string", description: "Shell command to execute.", @@ -199,9 +293,10 @@ function defaultBuiltinTools(): ToolDefinitionConfig[] { "How long to wait for the command before yielding. If it is still running when this elapses, the tool returns the output so far plus a process_id, and the command keeps running in the background (drive it with input_command). Defaults to 60000; minimum 250, capped below the tool timeout.", }, }, - required: ["cmd"], + required: ["description", "cmd"], }, permission: "rw", + call_description: true, timeoutMs: 120000, maxOutputLength: 16000, }, @@ -212,6 +307,11 @@ function defaultBuiltinTools(): ToolDefinitionConfig[] { parameters: { type: "object", properties: { + description: { + type: "string", + description: + "Required, and emit it first, before the other arguments: it is shown to the user while the call runs. One short sentence describing what this call is doing and why, written in the user's language.", + }, process_id: { type: "string", description: "The process_id returned by exec_command for the running command session.", @@ -227,9 +327,10 @@ function defaultBuiltinTools(): ToolDefinitionConfig[] { "How long to wait for new output or exit before returning. Non-empty writes default to 250; empty polls default to 5000. Minimum 250, capped below the tool timeout.", }, }, - required: ["process_id"], + required: ["description", "process_id"], }, permission: "rw", + call_description: true, // An empty poll can wait out a build/test run (the yield ceiling is derived from timeoutMs, clamped inside the tool). timeoutMs: 130000, maxOutputLength: 16000, @@ -242,6 +343,11 @@ function defaultBuiltinTools(): ToolDefinitionConfig[] { parameters: { type: "object", properties: { + description: { + type: "string", + description: + "Required, and emit it first, before the other arguments: it is shown to the user while the call runs. One short sentence describing what this call is doing and why, written in the user's language.", + }, prompt: { type: "string", description: @@ -268,9 +374,10 @@ function defaultBuiltinTools(): ToolDefinitionConfig[] { "How long to wait for the subagent before yielding. If it is still working when this elapses, the tool returns the output so far plus a subagent_id, and the subagent keeps running in the background (drive it with input_subagent). Defaults to 300000; minimum 250, capped below the tool timeout.", }, }, - required: ["prompt"], + required: ["description", "prompt"], }, permission: "rw", + call_description: true, // Subagent tasks typically run far longer than a single command, so the timeout ceiling is raised accordingly. timeoutMs: 600000, maxOutputLength: 16000, @@ -282,6 +389,11 @@ function defaultBuiltinTools(): ToolDefinitionConfig[] { parameters: { type: "object", properties: { + description: { + type: "string", + description: + "Required, and emit it first, before the other arguments: it is shown to the user while the call runs. One short sentence describing what this call is doing and why, written in the user's language.", + }, subagent_id: { type: "string", description: "The subagent_id returned by run_subagent for the background subagent.", @@ -297,9 +409,10 @@ function defaultBuiltinTools(): ToolDefinitionConfig[] { "How long to wait for new output or completion before returning. Follow-up prompts default to 300000; empty polls default to 10000. Minimum 250, capped below the tool timeout.", }, }, - required: ["subagent_id"], + required: ["description", "subagent_id"], }, permission: "rw", + call_description: true, // Same generous timeout tier as run_subagent: an empty poll can wait a long time for the subagent to wrap up. timeoutMs: 600000, maxOutputLength: 16000, diff --git a/packages/core/test/file-tools.test.ts b/packages/core/test/file-tools.test.ts new file mode 100644 index 0000000..fecf5cf --- /dev/null +++ b/packages/core/test/file-tools.test.ts @@ -0,0 +1,600 @@ +/** + * Behavior tests for the file tools (read_file / edit_file / write_file): happy paths, + * every failure case, workspace-relative path resolution, offset/limit windows, + * replace_all semantics, and parent-directory creation. Directly drives + * BuiltinTool.execute and captures the generator's return value (same approach as + * read-image.test.ts); Environment-side framing is covered by environment.test.ts. + */ +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { chmod, mkdir, mkdtemp, readdir, readFile, rm, stat, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { + DEFAULT_READ_FILE_LIMIT, + MAX_LINE_LENGTH, + READ_FILE_NAME, + READ_FILE_SCAN_CAP_BYTES, + createReadFileTool, +} from "../src/environment/tools/read-file.js"; +import { EDIT_FILE_NAME, createEditFileTool } from "../src/environment/tools/edit-file.js"; +import { WRITE_FILE_NAME, createWriteFileTool } from "../src/environment/tools/write-file.js"; +import type { BuiltinTool, ToolResult } from "../src/environment/tools/types.js"; +import type { OmniMessage } from "../src/omnimessage/index.js"; +import type { ToolDefinitionConfig } from "../src/interfaces.js"; + +function def(name: string, permission: "r" | "rw"): ToolDefinitionConfig { + return { name, description: "test", permission }; +} + +/** Runs one tool execution: concatenates text deltas and captures the generator's return value. */ +async function run(tool: BuiltinTool, args: Record, workspaceDir: string) { + const gen = tool.execute(args, { workspaceDir, toolCallId: "c1" }); + const messages: OmniMessage[] = []; + let result: ToolResult | void; + for (;;) { + const res = await gen.next(); + if (res.done) { + result = res.value; + break; + } + messages.push(res.value); + } + const text = messages.map((m) => (m.payload as { output?: string }).output ?? "").join(""); + return { result, text }; +} + +let tmp: string; + +beforeEach(async () => { + tmp = await mkdtemp(path.join(tmpdir(), "penguin-filetools-")); +}); + +afterEach(async () => { + await rm(tmp, { recursive: true, force: true }); +}); + +describe("read_file", () => { + const tool = () => createReadFileTool(def(READ_FILE_NAME, "r")); + + it("reads a file by workspace-relative path in cat -n style", async () => { + await writeFile(path.join(tmp, "a.txt"), "alpha\nbeta\ngamma\n"); + const { result, text } = await run(tool(), { file_path: "a.txt" }, tmp); + expect(result?.stopReason).toBeUndefined(); // Defaults to completed + expect(text.split("\n")).toEqual([" 1\talpha", " 2\tbeta", " 3\tgamma"]); + }); + + it("reads an absolute path outside the workspace", async () => { + const outside = await mkdtemp(path.join(tmpdir(), "penguin-filetools-out-")); + try { + const abs = path.join(outside, "abs.txt"); + await writeFile(abs, "outside\n"); + const { result, text } = await run(tool(), { file_path: abs }, tmp); + expect(result?.stopReason).toBeUndefined(); + expect(text).toContain("\toutside"); + } finally { + await rm(outside, { recursive: true, force: true }); + } + }); + + it("windows the output with offset/limit and points at the continuation", async () => { + const lines = Array.from({ length: 10 }, (_, i) => `line-${i + 1}`).join("\n"); + await writeFile(path.join(tmp, "many.txt"), `${lines}\n`); + const { result, text } = await run(tool(), { file_path: "many.txt", offset: 3, limit: 4 }, tmp); + expect(result?.stopReason).toBeUndefined(); + const rows = text.split("\n"); + expect(rows[0]).toBe(" 3\tline-3"); + expect(rows[3]).toBe(" 6\tline-6"); + expect(rows[4]).toBe( + "[file has 10 lines total; showing 3-6 — call again with offset to continue]", + ); + }); + + it("caps a missing limit at the default window and reports the total", async () => { + const total = DEFAULT_READ_FILE_LIMIT + 5; + const lines = Array.from({ length: total }, (_, i) => `l${i + 1}`).join("\n"); + await writeFile(path.join(tmp, "long.txt"), lines); + const { text } = await run(tool(), { file_path: "long.txt" }, tmp); + expect(text).toContain(`[file has ${total} lines total; showing 1-${DEFAULT_READ_FILE_LIMIT}`); + }); + + it("truncates a single overlong line with a marker", async () => { + await writeFile(path.join(tmp, "wide.txt"), `${"x".repeat(MAX_LINE_LENGTH + 50)}\nshort\n`); + const { text } = await run(tool(), { file_path: "wide.txt" }, tmp); + expect(text).toContain("… [line truncated]"); + expect(text).toContain("\tshort"); + expect(text).not.toContain("x".repeat(MAX_LINE_LENGTH + 1)); + }); + + it("reports an empty file as a note, not a failure", async () => { + await writeFile(path.join(tmp, "empty.txt"), ""); + const { result, text } = await run(tool(), { file_path: "empty.txt" }, tmp); + expect(result?.stopReason).toBeUndefined(); + expect(text).toContain("empty file"); + }); + + it("fails with a path hint when the file does not exist", async () => { + const { result, text } = await run(tool(), { file_path: "missing.txt" }, tmp); + expect(result?.stopReason).toBe("failed"); + expect(text).toContain("File not found"); + expect(text).toContain("missing.txt"); + expect(text).toContain("workspace"); + }); + + it("fails when the path is a directory", async () => { + await mkdir(path.join(tmp, "subdir")); + const { result, text } = await run(tool(), { file_path: "subdir" }, tmp); + expect(result?.stopReason).toBe("failed"); + expect(text).toContain("directory"); + }); + + it("fails on binary content (NUL bytes) and advises other tools", async () => { + await writeFile(path.join(tmp, "bin.dat"), Buffer.from([0x89, 0x50, 0x00, 0x0a, 0x42])); + const { result, text } = await run(tool(), { file_path: "bin.dat" }, tmp); + expect(result?.stopReason).toBe("failed"); + expect(text).toContain("binary"); + expect(text).toContain("read_image"); + }); + + it("fails when offset is past the end of the file", async () => { + await writeFile(path.join(tmp, "two.txt"), "a\nb\n"); + const { result, text } = await run(tool(), { file_path: "two.txt", offset: 5 }, tmp); + expect(result?.stopReason).toBe("failed"); + expect(text).toContain("past the end"); + expect(text).toContain("2 lines"); + }); + + it("fails when file_path is missing", async () => { + const { result, text } = await run(tool(), {}, tmp); + expect(result?.stopReason).toBe("failed"); + expect(text).toContain('"file_path"'); + }); +}); + +describe("edit_file", () => { + const tool = () => createEditFileTool(def(EDIT_FILE_NAME, "rw")); + + it("replaces a unique occurrence and echoes a git-style unified diff", async () => { + const file = path.join(tmp, "src.ts"); + await writeFile(file, "const a = 1;\nconst b = 2;\nconst c = 3;\n"); + const { result, text } = await run( + tool(), + { file_path: "src.ts", old_string: "const b = 2;", new_string: "const b = 20;" }, + tmp, + ); + expect(result?.stopReason).toBeUndefined(); + expect(text).toBe( + [ + 'Replaced 1 occurrence in "src.ts".', + "@@ -1,3 +1,3 @@", + " const a = 1;", + "-const b = 2;", + "+const b = 20;", + " const c = 3;", + ].join("\n"), + ); + expect(await readFile(file, "utf8")).toBe("const a = 1;\nconst b = 20;\nconst c = 3;\n"); + }); + + it("replaces every occurrence with replace_all and diffs same-line hits in one hunk", async () => { + const file = path.join(tmp, "multi.txt"); + await writeFile(file, "foo bar foo baz foo\n"); + const { result, text } = await run( + tool(), + { file_path: "multi.txt", old_string: "foo", new_string: "qux", replace_all: true }, + tmp, + ); + expect(result?.stopReason).toBeUndefined(); + expect(text).toBe( + [ + 'Replaced 3 occurrences in "multi.txt".', + "@@ -1,1 +1,1 @@", + "-foo bar foo baz foo", + "+qux bar qux baz qux", + ].join("\n"), + ); + expect(await readFile(file, "utf8")).toBe("qux bar qux baz qux\n"); + }); + + it("fails when old_string is not found", async () => { + await writeFile(path.join(tmp, "f.txt"), "hello\n"); + const { result, text } = await run( + tool(), + { file_path: "f.txt", old_string: "goodbye", new_string: "farewell" }, + tmp, + ); + expect(result?.stopReason).toBe("failed"); + expect(text).toContain('old_string not found in "f.txt"'); + expect(await readFile(path.join(tmp, "f.txt"), "utf8")).toBe("hello\n"); // Untouched + }); + + it("fails with the count when old_string is ambiguous and replace_all is unset", async () => { + await writeFile(path.join(tmp, "dup.txt"), "x\nx\nx\n"); + const { result, text } = await run( + tool(), + { file_path: "dup.txt", old_string: "x", new_string: "y" }, + tmp, + ); + expect(result?.stopReason).toBe("failed"); + expect(text).toContain("3 times"); + expect(text).toContain("replace_all"); + expect(await readFile(path.join(tmp, "dup.txt"), "utf8")).toBe("x\nx\nx\n"); + }); + + it("fails when old_string equals new_string", async () => { + await writeFile(path.join(tmp, "same.txt"), "abc\n"); + const { result, text } = await run( + tool(), + { file_path: "same.txt", old_string: "abc", new_string: "abc" }, + tmp, + ); + expect(result?.stopReason).toBe("failed"); + expect(text).toContain("identical"); + }); + + it("fails and suggests write_file when the file does not exist", async () => { + const { result, text } = await run( + tool(), + { file_path: "nope.txt", old_string: "a", new_string: "b" }, + tmp, + ); + expect(result?.stopReason).toBe("failed"); + expect(text).toContain("File not found"); + expect(text).toContain("write_file"); + }); + + it("fails when the path is a directory", async () => { + await mkdir(path.join(tmp, "d")); + const { result, text } = await run( + tool(), + { file_path: "d", old_string: "a", new_string: "b" }, + tmp, + ); + expect(result?.stopReason).toBe("failed"); + expect(text).toContain("directory"); + }); + + it("fails when required arguments are missing", async () => { + const missingPath = await run(tool(), { old_string: "a", new_string: "b" }, tmp); + expect(missingPath.result?.stopReason).toBe("failed"); + expect(missingPath.text).toContain('"file_path"'); + const missingOld = await run(tool(), { file_path: "f", new_string: "b" }, tmp); + expect(missingOld.result?.stopReason).toBe("failed"); + expect(missingOld.text).toContain('"old_string"'); + const missingNew = await run(tool(), { file_path: "f", old_string: "a" }, tmp); + expect(missingNew.result?.stopReason).toBe("failed"); + expect(missingNew.text).toContain('"new_string"'); + }); +}); + +describe("write_file", () => { + const tool = () => createWriteFileTool(def(WRITE_FILE_NAME, "rw")); + + it("creates a new file (workspace-relative) and reports lines/bytes", async () => { + const { result, text } = await run( + tool(), + { file_path: "out.txt", content: "one\ntwo\n" }, + tmp, + ); + expect(result?.stopReason).toBeUndefined(); + expect(text).toContain('Created "out.txt"'); + expect(text).toContain("2 lines"); + expect(text).toContain("8 bytes"); + expect(await readFile(path.join(tmp, "out.txt"), "utf8")).toBe("one\ntwo\n"); + }); + + it("creates missing parent directories", async () => { + const { result, text } = await run( + tool(), + { file_path: "a/b/c/deep.txt", content: "deep" }, + tmp, + ); + expect(result?.stopReason).toBeUndefined(); + expect(text).toContain("Created"); + expect(await readFile(path.join(tmp, "a/b/c/deep.txt"), "utf8")).toBe("deep"); + expect((await stat(path.join(tmp, "a/b"))).isDirectory()).toBe(true); + }); + + it("reports Overwrote when the file already exists", async () => { + await writeFile(path.join(tmp, "exists.txt"), "old"); + const { result, text } = await run( + tool(), + { file_path: "exists.txt", content: "new content" }, + tmp, + ); + expect(result?.stopReason).toBeUndefined(); + expect(text).toContain('Overwrote "exists.txt"'); + expect(await readFile(path.join(tmp, "exists.txt"), "utf8")).toBe("new content"); + }); + + it("accepts an empty string as content", async () => { + const { result, text } = await run(tool(), { file_path: "blank.txt", content: "" }, tmp); + expect(result?.stopReason).toBeUndefined(); + expect(text).toContain("0 lines, 0 bytes"); + expect(await readFile(path.join(tmp, "blank.txt"), "utf8")).toBe(""); + }); + + it("fails when the path is a directory", async () => { + await mkdir(path.join(tmp, "adir")); + const { result, text } = await run(tool(), { file_path: "adir", content: "x" }, tmp); + expect(result?.stopReason).toBe("failed"); + expect(text).toContain("directory"); + }); + + it("fails when a parent path component is an existing file", async () => { + await writeFile(path.join(tmp, "plain.txt"), "x"); + const { result, text } = await run( + tool(), + { file_path: "plain.txt/child.txt", content: "y" }, + tmp, + ); + expect(result?.stopReason).toBe("failed"); + expect(text).toContain("Failed to write"); + }); + + it("fails when content is missing (but not when it is empty)", async () => { + const { result, text } = await run(tool(), { file_path: "nocontent.txt" }, tmp); + expect(result?.stopReason).toBe("failed"); + expect(text).toContain('"content"'); + }); +}); + +describe("read_file — argument coercion, CRLF, secret guard", () => { + const tool = () => createReadFileTool(def(READ_FILE_NAME, "r")); + + it("accepts numeric strings for offset/limit", async () => { + const lines = Array.from({ length: 10 }, (_, i) => `line-${i + 1}`).join("\n"); + await writeFile(path.join(tmp, "many.txt"), `${lines}\n`); + const { result, text } = await run( + tool(), + { file_path: "many.txt", offset: "3", limit: "4" }, + tmp, + ); + expect(result?.stopReason).toBeUndefined(); + expect(text).toContain(" 3\tline-3"); + expect(text).toContain("showing 3-6"); + }); + + it("fails on non-numeric offset/limit instead of silently defaulting", async () => { + await writeFile(path.join(tmp, "a.txt"), "x\n"); + const badOffset = await run(tool(), { file_path: "a.txt", offset: "abc" }, tmp); + expect(badOffset.result?.stopReason).toBe("failed"); + expect(badOffset.text).toContain('Invalid "offset"'); + const badLimit = await run(tool(), { file_path: "a.txt", limit: 0 }, tmp); + expect(badLimit.result?.stopReason).toBe("failed"); + expect(badLimit.text).toContain('Invalid "limit"'); + }); + + it("strips trailing \\r from displayed lines and notes CRLF line endings", async () => { + await writeFile(path.join(tmp, "crlf.txt"), "alpha\r\nbeta\r\n"); + const { result, text } = await run(tool(), { file_path: "crlf.txt" }, tmp); + expect(result?.stopReason).toBeUndefined(); + expect(text).toContain(" 1\talpha\n"); + expect(text).not.toContain("\r"); + expect(text).toContain("(file uses CRLF line endings)"); + }); + + it("refuses the secret stores by basename regardless of directory", async () => { + await writeFile(path.join(tmp, ".vault.toml"), "SECRET=1\n"); + await mkdir(path.join(tmp, "sub")); + await writeFile(path.join(tmp, "sub", ".project_config.toml"), "key=1\n"); + const vault = await run(tool(), { file_path: ".vault.toml" }, tmp); + expect(vault.result?.stopReason).toBe("failed"); + expect(vault.text).toContain("Refusing to read"); + const cfg = await run(tool(), { file_path: "sub/.project_config.toml" }, tmp); + expect(cfg.result?.stopReason).toBe("failed"); + expect(cfg.text).toContain("Refusing to read"); + }); +}); + +describe("read_file — bounded scan and output budget", () => { + it("self-budgets the window so the continuation note survives a small maxOutputLength", async () => { + const lines = Array.from({ length: 10 }, (_, i) => `line-${i + 1}`).join("\n"); + await writeFile(path.join(tmp, "many.txt"), `${lines}\n`); + const tool = createReadFileTool({ + name: READ_FILE_NAME, + description: "test", + permission: "r", + maxOutputLength: 300, + }); + const { result, text } = await run(tool, { file_path: "many.txt" }, tmp); + expect(result?.stopReason).toBeUndefined(); + // The whole output fits under the tool's own cap, so Environment cannot cut the note. + expect(text.length).toBeLessThanOrEqual(300); + expect(text).toMatch( + /\[file has 10 lines total; showing 1-\d+ \(output limit reached\) — call again with offset to continue\]/, + ); + expect(text).not.toContain("line-10"); + }); + + it("fails with guidance when the scan cap is hit before the window (one huge line)", async () => { + await writeFile(path.join(tmp, "huge-line.txt"), "x".repeat(READ_FILE_SCAN_CAP_BYTES + 1024)); + const tool = () => createReadFileTool(def(READ_FILE_NAME, "r")); + const { result, text } = await run(tool(), { file_path: "huge-line.txt" }, tmp); + expect(result?.stopReason).toBe("failed"); + expect(text).toContain("Stopped after scanning 8 MB"); + expect(text).toContain("sed -n"); + }); + + it("reports a lower-bound total when the file outruns the scan cap after the window", async () => { + // > 8MB of two-byte lines: the window (1-2000) completes early, counting stops at the cap. + await writeFile(path.join(tmp, "long.txt"), "x\n".repeat(READ_FILE_SCAN_CAP_BYTES / 2 + 4096)); + const tool = () => createReadFileTool(def(READ_FILE_NAME, "r")); + const { result, text } = await run(tool(), { file_path: "long.txt" }, tmp); + expect(result?.stopReason).toBeUndefined(); + expect(text).toMatch( + /\[file has more than \d+ lines; showing 1-2000 — call again with offset to continue\]/, + ); + }); +}); + +describe("edit_file — review follow-ups", () => { + const tool = () => createEditFileTool(def(EDIT_FILE_NAME, "rw")); + + it("gives an explicitly-empty old_string its own message", async () => { + await writeFile(path.join(tmp, "f.txt"), "hello\n"); + const { result, text } = await run( + tool(), + { file_path: "f.txt", old_string: "", new_string: "x" }, + tmp, + ); + expect(result?.stopReason).toBe("failed"); + expect(text).toContain("old_string must not be empty"); + expect(text).toContain("write_file"); + }); + + it("mentions CRLF in the not-found error when the file uses \\r\\n", async () => { + await writeFile(path.join(tmp, "crlf.txt"), "alpha\r\nbeta\r\n"); + const { result, text } = await run( + tool(), + { file_path: "crlf.txt", old_string: "alpha\nbeta", new_string: "gamma" }, + tmp, + ); + expect(result?.stopReason).toBe("failed"); + expect(text).toContain("old_string not found"); + expect(text).toContain("CRLF"); + }); + + it("erasing the whole content diffs to a pure removal hunk", async () => { + const file = path.join(tmp, "erase.txt"); + await writeFile(file, "abc"); + const { result, text } = await run( + tool(), + { file_path: "erase.txt", old_string: "abc", new_string: "" }, + tmp, + ); + expect(result?.stopReason).toBeUndefined(); + expect(text).toBe('Replaced 1 occurrence in "erase.txt".\n@@ -1,1 +0,0 @@\n-abc'); + expect(await readFile(file, "utf8")).toBe(""); + }); + + it("writes atomically: no temp files are left behind", async () => { + await writeFile(path.join(tmp, "at.txt"), "a b a\n"); + await run(tool(), { file_path: "at.txt", old_string: "b", new_string: "c" }, tmp); + const entries = await readdir(tmp); + expect(entries.filter((e) => e.includes(".tmp-"))).toEqual([]); + }); +}); + +describe("write_file — review follow-ups", () => { + const tool = () => createWriteFileTool(def(WRITE_FILE_NAME, "rw")); + + it("preserves the permission bits of an overwritten file (atomic rename)", async () => { + const file = path.join(tmp, "mode.txt"); + await writeFile(file, "old"); + await chmod(file, 0o600); + const { result } = await run(tool(), { file_path: "mode.txt", content: "new" }, tmp); + expect(result?.stopReason).toBeUndefined(); + expect((await stat(file)).mode & 0o777).toBe(0o600); + expect(await readFile(file, "utf8")).toBe("new"); + }); + + it("writes atomically: no temp files are left behind", async () => { + await run(tool(), { file_path: "fresh.txt", content: "x" }, tmp); + const entries = await readdir(tmp); + expect(entries.filter((e) => e.includes(".tmp-"))).toEqual([]); + }); +}); + +describe("edit_file — diff output caps", () => { + const tool = () => createEditFileTool(def(EDIT_FILE_NAME, "rw")); + + it("caps replace_all storms at a handful of hunks plus an elision note", async () => { + // 8 far-apart sites (gaps wider than twice the context) -> 8 hunks, capped at 5. + const block = ["target", ...Array.from({ length: 9 }, (_, i) => `filler-${i}`)].join("\n"); + await writeFile( + path.join(tmp, "cap.txt"), + `${Array.from({ length: 8 }, () => block).join("\n")}\n`, + ); + const { result, text } = await run( + tool(), + { file_path: "cap.txt", old_string: "target", new_string: "changed", replace_all: true }, + tmp, + ); + expect(result?.stopReason).toBeUndefined(); + expect(text).toContain('Replaced 8 occurrences in "cap.txt".'); + expect(text.match(/@@ /g)).toHaveLength(5); + expect(text).toContain("…and 3 more replacements"); + }); + + it("self-budgets under a tiny maxOutputLength: the summary and note survive, hunks are dropped", async () => { + await writeFile(path.join(tmp, "tiny.txt"), "a\nb\na\n"); + const budgetTool = createEditFileTool({ + name: EDIT_FILE_NAME, + description: "test", + permission: "rw", + maxOutputLength: 150, + }); + const { result, text } = await run( + budgetTool, + { file_path: "tiny.txt", old_string: "a", new_string: "z", replace_all: true }, + tmp, + ); + expect(result?.stopReason).toBeUndefined(); + expect(text.length).toBeLessThanOrEqual(150); + expect(text).toContain('Replaced 2 occurrences in "tiny.txt".'); + expect(text).not.toContain("@@"); + expect(text).toContain("…and 2 more replacements"); + }); + + it("strips \\r from diff lines on CRLF files", async () => { + await writeFile(path.join(tmp, "crlfdiff.txt"), "alpha\r\nbeta\r\n"); + const { result, text } = await run( + tool(), + { file_path: "crlfdiff.txt", old_string: "alpha", new_string: "gamma" }, + tmp, + ); + expect(result?.stopReason).toBeUndefined(); + expect(text).toContain("-alpha"); + expect(text).toContain("+gamma"); + expect(text).toContain(" beta"); + expect(text).not.toContain("\r"); + }); +}); + +describe("write_file — overwrite diffs", () => { + const tool = () => createWriteFileTool(def(WRITE_FILE_NAME, "rw")); + + it("appends a small unified diff when overwriting", async () => { + await writeFile(path.join(tmp, "d.txt"), "one\ntwo\nthree\n"); + const { result, text } = await run( + tool(), + { file_path: "d.txt", content: "one\nTWO\nthree\n" }, + tmp, + ); + expect(result?.stopReason).toBeUndefined(); + expect(text).toBe( + [ + 'Overwrote "d.txt" (3 lines, 14 bytes).', + "@@ -1,3 +1,3 @@", + " one", + "-two", + "+TWO", + " three", + ].join("\n"), + ); + }); + + it("notes an unchanged overwrite", async () => { + await writeFile(path.join(tmp, "same.txt"), "keep\n"); + const { text } = await run(tool(), { file_path: "same.txt", content: "keep\n" }, tmp); + expect(text).toContain('Overwrote "same.txt"'); + expect(text).toContain("(content unchanged)"); + expect(text).not.toContain("@@"); + }); + + it("collapses a large rewrite to a one-line +X/−Y summary", async () => { + const oldContent = Array.from({ length: 200 }, (_, i) => `a${i}`).join("\n"); + const newContent = Array.from({ length: 200 }, (_, i) => `b${i}`).join("\n"); + await writeFile(path.join(tmp, "big.txt"), oldContent); + const { text } = await run(tool(), { file_path: "big.txt", content: newContent }, tmp); + expect(text).toContain('Overwrote "big.txt"'); + expect(text).toContain("+200/−200 lines vs the previous content (diff too large to show)"); + expect(text).not.toContain("@@"); + }); + + it("created files carry no diff", async () => { + const { text } = await run(tool(), { file_path: "fresh2.txt", content: "x\ny\n" }, tmp); + expect(text).toContain('Created "fresh2.txt"'); + expect(text).not.toContain("@@"); + }); +}); diff --git a/packages/core/test/state.test.ts b/packages/core/test/state.test.ts index 31c8838..3d2854b 100644 --- a/packages/core/test/state.test.ts +++ b/packages/core/test/state.test.ts @@ -45,6 +45,7 @@ import { toolsDir, type ModelRef, type ProjectConfig, + type SystemConfig, } from "../src/state/index.js"; import { sessionEnvironment } from "../src/internal/session-support.js"; @@ -170,7 +171,7 @@ describe("loadOrInitAgentState", () => { expect(second.systemConfig.system_prompt).toContain("PenguinHarness"); expect(second.agentsMd).toBe(first.agentsMd); // The tool config is fully preserved on the load path. - expect(second.systemConfig.tools?.builtin?.[0]?.name).toBe("exec_command"); + expect(second.systemConfig.tools?.builtin?.[0]?.name).toBe("read_file"); }); it("respects custom agentId / projectId", async () => { @@ -182,11 +183,14 @@ describe("loadOrInitAgentState", () => { }); describe("buildToolConfig", () => { - it("exposes exec/input command, run/input subagent (rw) and read_image (r)", async () => { + it("exposes command, file, subagent (rw) and image (r) tools", async () => { const state = await loadOrInitAgentState(); const cfg = buildToolConfig(state); expect(cfg.mcpServers).toEqual([]); expect(cfg.customTools.map((t) => t.name)).toEqual([ + "read_file", + "edit_file", + "write_file", "exec_command", "input_command", "run_subagent", @@ -198,16 +202,55 @@ describe("buildToolConfig", () => { expect(exec.permission).toBe("rw"); expect(exec.timeoutMs).toBe(120000); expect(exec.maxOutputLength).toBe(16000); - expect((exec.parameters as { required?: string[] }).required).toEqual(["cmd"]); + expect((exec.parameters as { required?: string[] }).required).toEqual(["description", "cmd"]); + // The four command/subagent tools declare the description call argument in config, + // toggled by the per-entry call_description field (default true). + expect(exec.call_description).toBe(true); + expect( + Object.keys((exec.parameters as { properties: Record }).properties), + ).toEqual(["description", "cmd", "workdir", "yield_time_ms"]); const write = cfg.customTools.find((t) => t.name === "input_command")!; expect(write.permission).toBe("rw"); - expect((write.parameters as { required?: string[] }).required).toEqual(["process_id"]); + expect(write.call_description).toBe(true); + expect((write.parameters as { required?: string[] }).required).toEqual([ + "description", + "process_id", + ]); + // File tools: read_file is read-only with a wider output cap; edit/write are rw. + const readFile = cfg.customTools.find((t) => t.name === "read_file")!; + expect(readFile.permission).toBe("r"); + expect(readFile.timeoutMs).toBe(30000); + expect(readFile.maxOutputLength).toBe(64000); + expect((readFile.parameters as { required?: string[] }).required).toEqual(["file_path"]); + expect(Object.keys((readFile.parameters as { properties: object }).properties)).toEqual([ + "file_path", + "offset", + "limit", + ]); + const editFile = cfg.customTools.find((t) => t.name === "edit_file")!; + expect(editFile.permission).toBe("rw"); + expect(editFile.timeoutMs).toBe(30000); + expect(editFile.maxOutputLength).toBe(16000); + expect((editFile.parameters as { required?: string[] }).required).toEqual([ + "file_path", + "old_string", + "new_string", + ]); + const writeFile = cfg.customTools.find((t) => t.name === "write_file")!; + expect(writeFile.permission).toBe("rw"); + expect((writeFile.parameters as { required?: string[] }).required).toEqual([ + "file_path", + "content", + ]); const sub = cfg.customTools.find((t) => t.name === "run_subagent")!; expect(sub.permission).toBe("rw"); - expect((sub.parameters as { required?: string[] }).required).toEqual(["prompt"]); + expect((sub.parameters as { required?: string[] }).required).toEqual(["description", "prompt"]); const writeSub = cfg.customTools.find((t) => t.name === "input_subagent")!; expect(writeSub.permission).toBe("rw"); - expect((writeSub.parameters as { required?: string[] }).required).toEqual(["subagent_id"]); + expect((writeSub.parameters as { required?: string[] }).required).toEqual([ + "description", + "subagent_id", + ]); // Both image-reading tool entries are explicitly in the config, each declaring its // applicable model kind via the forModel annotation. const readImage = cfg.customTools.find((t) => t.name === "read_image")!; @@ -273,6 +316,9 @@ describe("buildToolConfig", () => { }; const cfg = buildToolConfig(state); expect(cfg.customTools.map((t) => t.name)).toEqual([ + "read_file", + "edit_file", + "write_file", "exec_command", "input_command", "run_subagent", @@ -283,6 +329,105 @@ describe("buildToolConfig", () => { }); }); +describe("buildToolConfig — per-tool call_description filter", () => { + const makeState = (tools: NonNullable) => ({ + root: tmpRoot, + projectId: DEFAULT_PROJECT_ID, + agentId: DEFAULT_AGENT_ID, + stateDir: agentStateDir(tmpRoot, DEFAULT_PROJECT_ID, DEFAULT_AGENT_ID), + systemConfig: { system_prompt: "x", tools }, + agentsMd: "y", + }); + const properties = (t: { parameters?: Record }) => + (t.parameters as { properties: Record }).properties; + const required = (t: { parameters?: Record }) => + (t.parameters as { required?: string[] }).required; + + it("keeps the config-declared description property when call_description is missing or true (defaults)", async () => { + const state = await loadOrInitAgentState(); + const cfg = buildToolConfig(state); + for (const name of ["exec_command", "input_command", "run_subagent", "input_subagent"]) { + const tool = cfg.customTools.find((t) => t.name === name)!; + const desc = properties(tool)["description"] as { type?: string; description?: string }; + expect(desc.type).toBe("string"); + expect(desc.description).toContain("shown to the user"); + // Required whenever the tool offers it, so a call always carries one: the frontends + // pick the call's display form from the schema instead of guessing mid-stream. + expect(required(tool)).toContain("description"); + } + // The file tools' path argument is self-describing: no description parameter in config. + for (const name of ["read_file", "edit_file", "write_file", "read_image", "describe_image"]) { + const tool = cfg.customTools.find((t) => t.name === name)!; + expect(properties(tool)["description"]).toBeUndefined(); + } + }); + + it("filters the description property out when call_description is false, without mutating the stored config", () => { + const builtin = [ + { + name: "exec_command", + description: "shell", + call_description: false, + parameters: { + type: "object", + properties: { description: { type: "string" }, cmd: { type: "string" } }, + required: ["description", "cmd"], + }, + }, + ]; + const state = makeState({ builtin }); + const cfg = buildToolConfig(state); + const assembled = cfg.customTools[0]!; + expect(properties(assembled)["description"]).toBeUndefined(); + expect(properties(assembled)["cmd"]).toBeDefined(); + // The property and its `required` entry go together: a filtered-out argument must not + // stay mandatory. + expect(required(assembled)).toEqual(["cmd"]); + expect((builtin[0]!.parameters.required as string[]).includes("description")).toBe(true); + // In-memory clone only: the stored entry still declares the property. + expect(properties(builtin[0]!)["description"]).toBeDefined(); + }); + + it("is a no-op for entries without the property, a schema, or with the toggle on", () => { + const builtin = [ + { + name: "exec_command", + description: "shell", + call_description: false, + parameters: { + type: "object", + properties: { description: { type: "string" }, cmd: { type: "string" } }, + required: ["cmd"], + }, + }, + // call_description without a matching property (old config shape): no-op. + { + name: "input_command", + description: "no description property", + call_description: false, + parameters: { type: "object", properties: { process_id: { type: "string" } } }, + }, + // No parameter schema at all: no-op. + { name: "run_subagent", description: "no schema", call_description: false }, + // call_description true keeps the declared property. + { + name: "input_subagent", + description: "kept", + call_description: true, + parameters: { + type: "object", + properties: { description: { type: "string" }, subagent_id: { type: "string" } }, + }, + }, + ]; + const cfg = buildToolConfig(makeState({ builtin })); + expect(properties(cfg.customTools[0]!)["description"]).toBeUndefined(); + expect(properties(cfg.customTools[1]!)["process_id"]).toBeDefined(); + expect(cfg.customTools[2]!.parameters).toBeUndefined(); + expect(properties(cfg.customTools[3]!)["description"]).toBeDefined(); + }); +}); + describe("assembleSystemPrompt", () => { it("renders default system prompt placeholders", async () => { const state = await loadOrInitAgentState(); diff --git a/packages/docs/content/architecture.en.md b/packages/docs/content/architecture.en.md index 6ffa1af..24f8be2 100644 --- a/packages/docs/content/architecture.en.md +++ b/packages/docs/content/architecture.en.md @@ -81,7 +81,7 @@ packages/ │ ├── engine/context-engine.ts # ReAct loop orchestration: turn lifecycle, approvals, carry-over, reconnect, compaction │ ├── omnimessage/ # types.ts protocol types · builders.ts constructors · aggregate.ts partial aggregation │ ├── llm/ # generative-model.ts AgentHub adapter · tool-call-ids.ts id uniqueness -│ ├── environment/ # environment.ts execution close-out · tools/ registry, 6 builtin tools, background sessions +│ ├── environment/ # environment.ts execution close-out · tools/ registry, 9 builtin tools, background sessions │ ├── state/ # paths · default-config · project-config · model-catalog │ │ # agent-state (Skill install, prompt assembly) · agent-vault · builtin-agents │ ├── trace/ # writer.ts append-only JSONL · resume.ts replay-based recovery diff --git a/packages/docs/content/architecture.zh.md b/packages/docs/content/architecture.zh.md index 0824a83..3cd7ed6 100644 --- a/packages/docs/content/architecture.zh.md +++ b/packages/docs/content/architecture.zh.md @@ -81,7 +81,7 @@ packages/ │ ├── engine/context-engine.ts # ReAct 循环编排:轮生命周期、审批、补发、重连、压缩 │ ├── omnimessage/ # types.ts 协议类型 · builders.ts 构造函数 · aggregate.ts 分片聚合 │ ├── llm/ # generative-model.ts AgentHub 适配 · tool-call-ids.ts id 唯一化 -│ ├── environment/ # environment.ts 执行与收尾 · tools/ 注册表、6 个内置工具、后台会话 +│ ├── environment/ # environment.ts 执行与收尾 · tools/ 注册表、9 个内置工具、后台会话 │ ├── state/ # paths · default-config · project-config · model-catalog │ │ # agent-state(Skill 安装、提示词装配)· agent-vault · builtin-agents │ ├── trace/ # writer.ts 追加式 JSONL · resume.ts 回放恢复 diff --git a/packages/docs/content/configuration.en.md b/packages/docs/content/configuration.en.md index bb290a7..6101d6f 100644 --- a/packages/docs/content/configuration.en.md +++ b/packages/docs/content/configuration.en.md @@ -101,7 +101,7 @@ Edit this file via the CLI (`penguin config model …`) or the Web Models page | `compaction.max_session_turns` | `-1` | Cumulative Session turn threshold (`-1` = unlimited) | | `compaction.mode` | `summarize` | `summarize` / `discard` | | `compaction.prompt` | built-in template | Prompt used for summarize compaction | -| `tools.builtin` | full default toolset when omitted | Tool entries: `name` / `description` / `parameters` / `permission` (`r` or `rw`) / `forModel` / `timeoutMs` / `maxOutputLength`; once written it replaces the default list wholesale | +| `tools.builtin` | full default toolset when omitted | Tool entries: `name` / `description` / `parameters` / `permission` (`r` or `rw`) / `forModel` / `timeoutMs` / `maxOutputLength` / `call_description` (per-tool toggle for the `description` call argument, required while on; missing = kept); once written it replaces the default list wholesale | | `tools.mcpServers` | `[]` | MCP Server configuration (`name` + `config`); reserved for the MCP adapter layer | Tool permissions and approval semantics are covered in [Tools & Approval](/tools). diff --git a/packages/docs/content/configuration.zh.md b/packages/docs/content/configuration.zh.md index 746c44b..aaa969d 100644 --- a/packages/docs/content/configuration.zh.md +++ b/packages/docs/content/configuration.zh.md @@ -101,7 +101,7 @@ output = 0.857143 | `compaction.max_session_turns` | `-1` | Session 累计轮数阈值（`-1` 不限制） | | `compaction.mode` | `summarize` | `summarize` / `discard` | | `compaction.prompt` | 内置模板 | summarize 压缩使用的 Prompt | -| `tools.builtin` | 缺省时为完整默认工具集 | 工具条目：`name` / `description` / `parameters` / `permission`（`r` 或 `rw`）/ `forModel` / `timeoutMs` / `maxOutputLength`；一旦写出即整体替换默认列表 | +| `tools.builtin` | 缺省时为完整默认工具集 | 工具条目：`name` / `description` / `parameters` / `permission`（`r` 或 `rw`）/ `forModel` / `timeoutMs` / `maxOutputLength` / `call_description`（条目级开关：控制 `description` 调用参数，开启时为必填，缺省保留）；一旦写出即整体替换默认列表 | | `tools.mcpServers` | `[]` | MCP Server 配置（`name` + `config`），预留给 MCP 适配层 | 工具权限与审批语义见[工具与审批](/tools)。 diff --git a/packages/docs/content/introduction.en.md b/packages/docs/content/introduction.en.md index 42cbe46..ee43e8e 100644 --- a/packages/docs/content/introduction.en.md +++ b/packages/docs/content/introduction.en.md @@ -32,7 +32,7 @@ One install gives you four layers that share a single data directory and a singl These principles run through every component; the design pages keep coming back to them: -- **A minimal toolset**: the shell is the universal interface — file reads, writes and edits all go through `exec_command`. See [Tools & Approval](/tools). +- **A minimal toolset**: dedicated file tools (`read_file` / `edit_file` / `write_file`) for precise reading and editing, with the shell (`exec_command`) as the general-purpose fallback for everything else. See [Tools & Approval](/tools). - **Agents are editable data**: prompts, Skills and config are editable files on disk, not hardcoded constants — what you can see, an Agent can improve. See the [Configuration Reference](/configuration). - **Everything observable**: every request, tool call and approval decision is appended to the [Trace](/sessions-and-traces); a Session restores fully from it. - **Errors converge into messages**: model and tool failures never throw — they become messages the model can react to. See [The Agent Loop](/agent-loop). diff --git a/packages/docs/content/introduction.zh.md b/packages/docs/content/introduction.zh.md index e88c746..fa8910b 100644 --- a/packages/docs/content/introduction.zh.md +++ b/packages/docs/content/introduction.zh.md @@ -32,7 +32,7 @@ PenguinHarness 的能力围绕三个递进的概念展开——消息协议、SD 这些原则贯穿所有组件，后续每一页设计文档都会反复引用： -- **极简工具集**:shell 是通用接口，文件读写与命令执行统一经 `exec_command` 完成，见[工具与审批](/tools)。 +- **极简工具集**:专门的文件工具（`read_file` / `edit_file` / `write_file`）负责精确读写，shell（`exec_command`）作为通用兜底接口负责其余一切，见[工具与审批](/tools)。 - **Agent 是可编辑的数据**:Prompt、Skill、配置都是磁盘上的可编辑文件，而非硬编码——你能看到的，Agent 就能改进，见[配置参考](/configuration)。 - **全量可观测**：每一次请求、工具调用与审批决策都以追加方式写入 [Trace](/sessions-and-traces),Session 可从 Trace 完整恢复。 - **错误收敛为消息**：模型与工具的错误不抛异常，而是变成模型可以继续处理的消息，见 [Agent 运行循环](/agent-loop)。 diff --git a/packages/docs/content/skills.en.md b/packages/docs/content/skills.en.md index 4dc21e0..9f1a039 100644 --- a/packages/docs/content/skills.en.md +++ b/packages/docs/content/skills.en.md @@ -36,7 +36,7 @@ Parsing is tolerant: only `key: value` scalar lines inside the first `---` block ## Progressive loading -Skills follow an "index first, body on demand" design: the system prompt injects only each installed Skill's metadata (name + description) through the `{{SKILL_METADATA}}` placeholder, and instructs the model to read the matching `SKILL.md` in full via the shell before following it. There is no dedicated skill tool — reading the body is just one `exec_command` call (see [Tools & Approval](/tools)). +Skills follow an "index first, body on demand" design: the system prompt injects only each installed Skill's metadata (name + description) through the `{{SKILL_METADATA}}` placeholder, and instructs the model to read the matching `SKILL.md` in full via the shell before following it. There is no dedicated skill tool — reading the body is just one `read_file` or shell call (see [Tools & Approval](/tools)). Chat can also pin skills explicitly: the message then starts with a `` block listing the skill names. diff --git a/packages/docs/content/skills.zh.md b/packages/docs/content/skills.zh.md index 8c994bb..a98ab84 100644 --- a/packages/docs/content/skills.zh.md +++ b/packages/docs/content/skills.zh.md @@ -36,7 +36,7 @@ updated: 2026-07-17 ## 渐进式加载 -Skill 采用「先索引、后正文」的设计：系统 Prompt 经 `{{SKILL_METADATA}}` 占位符只注入每个已安装 Skill 的元数据(name + description)，并指示模型在任务匹配某个 Skill 时，先用 Shell 完整读取对应的 `SKILL.md`，再遵循执行。系统不设专门的 Skill 工具，读取正文就是一次 `exec_command` 调用(见 [工具与审批](/tools))。 +Skill 采用「先索引、后正文」的设计：系统 Prompt 经 `{{SKILL_METADATA}}` 占位符只注入每个已安装 Skill 的元数据(name + description)，并指示模型在任务匹配某个 Skill 时，先用 Shell 完整读取对应的 `SKILL.md`，再遵循执行。系统不设专门的 Skill 工具，读取正文就是一次 `read_file` 或 Shell 调用(见 [工具与审批](/tools))。 对话中也可以显式指定 Skill：此时消息以 `` 块开头，列出要使用的 Skill 名。 diff --git a/packages/docs/content/tools.en.md b/packages/docs/content/tools.en.md index 8d356e4..e928491 100644 --- a/packages/docs/content/tools.en.md +++ b/packages/docs/content/tools.en.md @@ -5,7 +5,7 @@ description: The deliberately minimal built-in toolset, its execution contract w ## Design -PenguinHarness ships a deliberately minimal built-in toolset: the shell is the universal interface, and reading, writing and editing files all go through `exec_command` — there are no separate file tools. Fewer tools mean fewer schema tokens and fewer wrong calls. +PenguinHarness ships a deliberately minimal built-in toolset: dedicated file tools (`read_file` / `edit_file` / `write_file`) cover precise reading and editing — line-numbered output and exact-string replacement beat quoting `sed` one-liners — while the shell (`exec_command`) remains the general-purpose fallback for everything else: running programs, searching, installing dependencies. Every tool that remains earns its schema tokens. ## Execution contract @@ -58,20 +58,30 @@ Each tool is described by one `ToolDefinitionConfig`: | `forModel` | `"vision"` / `"text-only"`: selected by the Session model's class; omitted = available to all models | | `timeoutMs` | Per-call timeout (ms), default 120000; `<=0` disables | | `maxOutputLength` | Output length cap (characters); `<=0` disables | +| `call_description` | Per-tool toggle for the `description` call argument declared in `parameters` (required while on); missing = kept, `false` filters it and its `required` entry out of the schema at assembly | ## Built-in tools -There are 6 built-in tools (assembled via `packages/core/src/environment/tools/registry.ts`): +There are 9 built-in tools (assembled via `packages/core/src/environment/tools/registry.ts`): | Tool | Permission | Timeout (ms) | Purpose | | --- | --- | --- | --- | | `exec_command` | rw | 120000 | Run a shell command in the Workspace via `bash -lc`, streaming stdout/stderr | | `input_command` | rw | 130000 | Drive a running command by `process_id`: write stdin, send Ctrl-C, poll output | +| `read_file` | r | 30000 | Read a text file as a line-numbered (`cat -n`) window, paged by offset/limit | +| `edit_file` | rw | 30000 | Exact-string replacement in an existing file, echoing a verification snippet | +| `write_file` | rw | 30000 | Create or overwrite a whole file, creating parent directories as needed | | `run_subagent` | rw | 600000 | Delegate a self-contained subtask to a child Agent in the same Workspace | | `input_subagent` | rw | 600000 | Poll a background subagent, or send a follow-up prompt once it is idle | | `read_image` | r | 60000 | Read an image and return it as image content (vision models) | | `describe_image` | r | 90000 | Have the configured `vision_model` read the image and answer in text (text-only models) | +Note that an existing agent's persisted `tools.builtin` list is frozen as written (the settings UI edits rows but adds none): agents created before this toolset do not pick up the file tools automatically — hand-edit the agent's `system_config.yaml` and add the new entries (copy them from the default definitions in `packages/core/src/state/default-config.ts`) to adopt them. + +### Call descriptions + +The command/subagent tools (`exec_command`, `input_command`, `run_subagent`, `input_subagent`) take a `description` argument: one model-written sentence about what the call is doing, shown by the CLI and Web UI while the call runs. The argument is declared as a normal `description` property in each entry's `parameters` in `system_config.yaml` (tool schemas live entirely in the editable config), and it is **required** there — a tool that offers the argument always gets one, so the frontends can pick a call's display form from the schema instead of guessing while the arguments stream; the model is also asked to emit it first. The per-entry `call_description` field toggles the whole thing — missing = kept, `call_description: false` filters the property (and its `required` entry) out of the schema at assembly time (in-memory only, the YAML is never rewritten). The file tools don't take it — their `file_path` argument is self-describing. + ### Command sessions `exec_command` waits in the foreground first; if the command outruns `yield_time_ms` it moves to the background and the call returns the output so far plus a `process_id`, driven from then on by `input_command`: @@ -94,6 +104,7 @@ Both tools' arguments (explicit keys): cmd: string; // required: the shell command to run workdir?: string; // working directory; defaults to the Workspace root, relative paths resolve against it yield_time_ms?: number; // foreground wait; default 60000, minimum 250, capped below the tool timeout + description: string; // required while call_description is on: one sentence shown to the user while the call runs, emitted first } // input_command @@ -101,6 +112,40 @@ Both tools' arguments (explicit keys): process_id: string; // required: the command-session id returned by exec_command chars?: string; // characters for stdin; send "\u0003" alone to deliver Ctrl-C; empty = poll only yield_time_ms?: number; // wait; defaults 250 for writes, 5000 for empty polls + description: string; // required while call_description is on +} +``` + +### File tools + +`read_file` / `edit_file` / `write_file` run with the user's full permissions, same as the shell tool; relative paths resolve against the Workspace and absolute paths are allowed. They are non-streaming (a single final output) and never throw — failures come back as explanatory text with `stop_reason: failed`. + +```ts +// read_file — cat -n style output (line number, tab, content); overlong single lines are +// truncated, and binary content (NUL bytes) is rejected with advice to use shell/image tools. +{ + file_path: string; // required: absolute, or relative to the Workspace + offset?: number; // 1-based line to start from; default 1 + limit?: number; // max lines returned; default 2000 — a trailing note points at the continuation +} + +// edit_file — the file must exist; old_string must occur exactly once (or set replace_all); +// success echoes "Replaced N occurrence(s)" plus a git-style unified diff of the changed +// regions (one hunk per site, nearby sites merged; replace_all storms are capped at a few +// hunks plus an "…and N more replacements" note). +{ + file_path: string; // required + old_string: string; // required: exact text to replace, including whitespace/indentation + new_string: string; // required: must differ from old_string + replace_all?: boolean; // replace every occurrence; default false +} + +// write_file — creates parent directories as needed; reports "Created" vs "Overwrote" with +// lines/bytes. An overwrite also shows a small unified diff against the previous content, +// or a one-line +X/−Y summary when the change is large. +{ + file_path: string; // required + content: string; // required: full file content; an empty string creates an empty file } ``` @@ -115,6 +160,7 @@ Both tools' arguments (explicit keys): agent_id?: string; // the child Agent; defaults to the current Agent model_id?: string; // the child Session's model; inherits the parent Session's model when omitted yield_time_ms?: number; // foreground wait; default 300000 + description: string; // required while call_description is on } // input_subagent @@ -122,6 +168,7 @@ Both tools' arguments (explicit keys): subagent_id: string; // required: the background Subagent id returned by run_subagent prompt?: string; // follow-up task, accepted only while the child Session is idle; empty = poll only yield_time_ms?: number; // wait; defaults 300000 with a prompt, 10000 for empty polls + description: string; // required while call_description is on } ``` @@ -182,6 +229,9 @@ tools: - name: exec_command description: Run a shell command in the workspace. permission: rw + # Optional per-tool toggle: false filters the `description` call argument + # (declared in parameters.properties) out of the schema (missing = kept). + call_description: false timeoutMs: 120000 maxOutputLength: 16000 # parameters: the complete JSON Schema is required (see the default definition diff --git a/packages/docs/content/tools.zh.md b/packages/docs/content/tools.zh.md index 095f976..e0ce45c 100644 --- a/packages/docs/content/tools.zh.md +++ b/packages/docs/content/tools.zh.md @@ -5,7 +5,7 @@ description: 极简内置工具集的设计与执行契约、Environment 统一 ## 设计取向 -PenguinHarness 刻意维持一个极小的内置工具集：Shell 是通用接口，文件的读取、写入、编辑全部经由 `exec_command` 完成，不设专门的文件工具。工具越少，注入的 schema 越少，Token 开销越小，模型误调用的概率也越低。 +PenguinHarness 刻意维持一个极小的内置工具集：文件的精确读取与编辑交给专门的文件工具（`read_file` / `edit_file` / `write_file`）——带行号的输出与精确字符串替换比拼 `sed` 命令更可靠；Shell（`exec_command`）仍是通用兜底接口，负责运行程序、搜索、装依赖等其余一切。保留下来的每个工具都对得起它占用的 schema Token。 ## 执行契约 @@ -58,20 +58,30 @@ interface ToolResult { | `forModel` | `"vision"` / `"text-only"`：按 Session 模型类别装配；缺省对所有模型可用 | | `timeoutMs` | 单次调用超时(ms)，默认 120000;`<=0` 关闭 | | `maxOutputLength` | 输出长度上限(字符);`<=0` 关闭 | +| `call_description` | 条目级开关：控制 `parameters` 中声明的 `description` 调用参数（开启时为必填）；缺省保留，`false` 时装配阶段将其连同 `required` 项从 schema 滤除 | ## 内置工具 -共 6 个内置工具(装配入口 `packages/core/src/environment/tools/registry.ts`): +共 9 个内置工具(装配入口 `packages/core/src/environment/tools/registry.ts`): | 工具 | 权限 | 超时(ms) | 用途 | | --- | --- | --- | --- | | `exec_command` | rw | 120000 | 在 Workspace 内以 `bash -lc` 运行命令，流式返回 stdout/stderr | | `input_command` | rw | 130000 | 按 `process_id` 驱动运行中的命令：写 stdin、发 Ctrl-C、轮询输出 | +| `read_file` | r | 30000 | 按 `cat -n` 风格带行号读取文本文件，以 offset/limit 分页 | +| `edit_file` | rw | 30000 | 对既有文件做精确字符串替换，回显校验片段 | +| `write_file` | rw | 30000 | 新建或整体覆写文件，按需创建父目录 | | `run_subagent` | rw | 600000 | 把自包含子任务委派给同 Workspace 的子 Agent | | `input_subagent` | rw | 600000 | 轮询后台 Subagent，或在其空闲时追加后续 Prompt | | `read_image` | r | 60000 | 读取图片并作为图像内容返回(vision 模型) | | `describe_image` | r | 90000 | 由 `vision_model` 代读图片并返回文字回答(text-only 模型) | +注意：既有 Agent 已落盘的 `tools.builtin` 列表按原样冻结（设置页只能编辑行、不能增行）：本工具集之前创建的 Agent 不会自动获得文件工具——需手工编辑该 Agent 的 `system_config.yaml`，把新条目补进去（可从 `packages/core/src/state/default-config.ts` 的默认定义复制）。 + +### 调用描述 + +命令 / Subagent 类工具（`exec_command`、`input_command`、`run_subagent`、`input_subagent`）带 `description` 参数：由模型写一句"本次调用在做什么"，CLI 与 Web 在调用运行期间展示给用户。该参数作为普通的 `description` 属性直接写在各条目的 `parameters` 中（工具 schema 完全存于可编辑配置），并且是**必填**的——提供该参数的工具每次调用都会带上它，前端据 schema 即可确定这次调用的展示形态，无需在参数流式过程中猜测；同时要求模型最先输出它。整个参数由条目级 `call_description` 字段控制——缺省保留，写 `call_description: false` 时装配阶段将该属性连同其 `required` 项一起从 schema 中滤除（仅内存内，不改写 YAML）。文件工具不带此参数——其 `file_path` 参数本身已说明用途。 + ### 命令会话 `exec_command` 先在前台等待；命令超过 `yield_time_ms` 仍未结束时转入后台，返回已有输出和一个 `process_id`，之后用 `input_command` 驱动： @@ -93,6 +103,7 @@ exec_command(cmd) cmd: string; // 必填:要执行的 shell 命令 workdir?: string; // 工作目录;缺省为 Workspace 根,相对路径按其解析 yield_time_ms?: number; // 前台等待时长;默认 60000,最小 250,上限受工具超时约束 + description: string; // 开关开启时必填:一句话说明,最先输出,调用运行期间展示给用户 } // input_command @@ -100,6 +111,39 @@ exec_command(cmd) process_id: string; // 必填:exec_command 返回的命令会话 id chars?: string; // 写入 stdin 的字符;单独发送 "\u0003" 传递 Ctrl-C;缺省仅轮询 yield_time_ms?: number; // 等待时长;有写入默认 250,空轮询默认 5000 + description: string; // 开关开启时必填 +} +``` + +### 文件工具 + +`read_file` / `edit_file` / `write_file` 与 Shell 工具一样以用户完整权限运行：相对路径按 Workspace 解析，也接受绝对路径。三者均为非流式（一次性输出最终结果），从不抛异常——失败以解释性文本收尾，`stop_reason` 为 `failed`。 + +```ts +// read_file — cat -n 风格输出(行号、制表符、内容);超长单行会被截断, +// 含 NUL 字节的二进制内容被拒绝并提示改用 Shell / 图像工具。 +{ + file_path: string; // 必填:绝对路径,或相对 Workspace 的路径 + offset?: number; // 起始行号(1 起);默认 1 + limit?: number; // 最多返回的行数;默认 2000——未读完时尾部注记提示续读 +} + +// edit_file — 文件必须已存在;old_string 必须恰好出现一次(或设 replace_all); +// 成功时回显 "Replaced N occurrence(s)" 及改动区域的 git 风格 unified diff +// (每个替换点一个 hunk,相邻替换点合并;replace_all 大量命中时截断为少量 hunk +// 并附 "…and N more replacements" 注记)。 +{ + file_path: string; // 必填 + old_string: string; // 必填:要替换的原文,须与文件内容(含空白/缩进)完全一致 + new_string: string; // 必填:替换文本,须与 old_string 不同 + replace_all?: boolean; // 替换全部出现处;默认 false +} + +// write_file — 按需创建父目录;报告 "Created" 或 "Overwrote" 及行数/字节数。 +// 覆写时还会附上与旧内容的小型 unified diff;改动过大时改为一行 +X/−Y 摘要。 +{ + file_path: string; // 必填 + content: string; // 必填:完整文件内容;空字符串创建空文件 } ``` @@ -114,6 +158,7 @@ exec_command(cmd) agent_id?: string; // 子 Agent;缺省复用当前 Agent model_id?: string; // 子 Session 模型;缺省继承父 Session 的模型 yield_time_ms?: number; // 前台等待时长;默认 300000 + description: string; // 开关开启时必填 } // input_subagent @@ -121,6 +166,7 @@ exec_command(cmd) subagent_id: string; // 必填:run_subagent 返回的后台 Subagent id prompt?: string; // 追加任务,仅在子 Session 空闲时接受;缺省仅轮询 yield_time_ms?: number; // 等待时长;有追加默认 300000,空轮询默认 10000 + description: string; // 开关开启时必填 } ``` @@ -180,6 +226,9 @@ tools: - name: exec_command description: Run a shell command in the workspace. permission: rw + # 可选的条目级开关:false 时从 schema 滤除 parameters.properties 里声明的 + # description 调用参数(缺省保留)。 + call_description: false timeoutMs: 120000 maxOutputLength: 16000 # parameters: 必须携带完整 JSON Schema(默认定义见 diff --git a/packages/docs/content/web-app.en.md b/packages/docs/content/web-app.en.md index ddb952c..3ebc825 100644 --- a/packages/docs/content/web-app.en.md +++ b/packages/docs/content/web-app.en.md @@ -64,7 +64,7 @@ The list page creates and deletes Agents; clicking through opens the `/agents/:a | Overview | Basic info, export / import of Agent State snapshots, and restoring the default configuration (overwrites customizations, keeping only name/description) | | Prompt | AGENTS.md and system_prompt | | Runtime | Runtime parameters such as max_turns, model.*, compaction.* | -| Tools | Built-in tool table and MCP server JSON configuration | +| Tools | Built-in tool table (incl. per-tool call_description switches) and MCP server JSON configuration | | Vault | Environment-variable entries with masked values | | Schedule | Scheduled tasks (TOML-defined): create, edit, toggle, delete | diff --git a/packages/docs/content/web-app.zh.md b/packages/docs/content/web-app.zh.md index 169d22c..80fb78b 100644 --- a/packages/docs/content/web-app.zh.md +++ b/packages/docs/content/web-app.zh.md @@ -64,7 +64,7 @@ penguin web | Overview | 基本信息、Agent State 快照的导出 / 导入，以及还原为默认配置（覆盖自定义内容，仅保留名称与描述） | | Prompt | AGENTS.md 与 system_prompt | | Runtime | max_turns、model.*、compaction.* 等运行参数 | -| Tools | 内置工具表格与 MCP Server 的 JSON 配置 | +| Tools | 内置工具表格（含条目级 call_description 开关）与 MCP Server 的 JSON 配置 | | Vault | 环境变量条目，值以掩码显示 | | Schedule | 定时任务（TOML 定义）：创建、编辑、启停、删除 | diff --git a/packages/server/src/http/validate.ts b/packages/server/src/http/validate.ts index 5dd7838..0504c9e 100644 --- a/packages/server/src/http/validate.ts +++ b/packages/server/src/http/validate.ts @@ -187,6 +187,17 @@ export function optionalNumber( return v; } +export function optionalBoolean( + obj: Record, + key: string, + label = key, +): boolean | undefined { + const v = obj[key]; + if (v === undefined) return undefined; + if (typeof v !== "boolean") throw badRequest(`${label} must be a boolean.`); + return v; +} + /** Validate a yyyy-mm-dd query parameter (defaults to undefined). */ export function optionalDateParam(value: string | undefined, label: string): string | undefined { if (value === undefined || value === "") return undefined; diff --git a/packages/server/src/services/agent-config-service.ts b/packages/server/src/services/agent-config-service.ts index 99fd92f..432a186 100644 --- a/packages/server/src/services/agent-config-service.ts +++ b/packages/server/src/services/agent-config-service.ts @@ -35,7 +35,13 @@ import type { VaultUpdateRequest, } from "../api/types.js"; import { HttpError } from "../http/errors.js"; -import { badRequest, optionalEnum, optionalNumber, optionalString } from "../http/validate.js"; +import { + badRequest, + optionalBoolean, + optionalEnum, + optionalNumber, + optionalString, +} from "../http/validate.js"; import { maskApiKey } from "./project-config-service.js"; const THINKING_LEVELS: readonly ThinkingLevelName[] = ["none", "low", "medium", "high", "xhigh"]; @@ -328,6 +334,7 @@ function validateToolsBuiltin(value: unknown): ToolDefinitionConfig[] { if (t.forModel !== undefined && t.forModel !== "vision" && t.forModel !== "text-only") { throw badRequest(`toolsBuiltin[${i}].forModel must be one of vision / text-only.`); } + optionalBoolean(t, "call_description", `toolsBuiltin[${i}].call_description`); optionalNumber(t, "timeoutMs", { integer: true, positiveOrMinusOne: true, diff --git a/packages/server/test/agent-config-call-description.test.ts b/packages/server/test/agent-config-call-description.test.ts new file mode 100644 index 0000000..b4c4d46 --- /dev/null +++ b/packages/server/test/agent-config-call-description.test.ts @@ -0,0 +1,86 @@ +/** + * Agent config route: the per-tool `call_description` field on toolsBuiltin rows. The + * default config writes `call_description: true` (plus the `description` property in + * parameters) on the four command/subagent tools; PUT round-trips a flipped `false` into + * system_config.yaml (preserving the rest of the file); a non-boolean value is a 400. + */ +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import fs from "node:fs/promises"; +import { systemConfigPath } from "@prismshadow/penguin-core"; +import type { ToolDefinitionConfig } from "@prismshadow/penguin-core"; +import type { AgentConfigResponse, ProjectCreateResponse } from "../src/api/types.js"; +import { apiClient, createTestApp, provisionUser } from "./helpers.js"; +import type { TestApp } from "./helpers.js"; + +describe("agent config: per-tool call_description", () => { + let t: TestApp; + let owner: ReturnType; + let projectId: string; + let configPath: string; + + beforeEach(async () => { + t = await createTestApp(); + const a = await provisionUser(t.app, "owner_cd"); + owner = apiClient(t.app, a.cookie); + const created = (await ( + await owner.post("/api/projects", { projectId: "owner_cd-calldesc", name: "cd project" }) + ).json()) as ProjectCreateResponse; + projectId = created.project.projectId; + configPath = `/api/projects/${projectId}/agents/default_agent/config`; + }); + + afterEach(async () => { + await t.cleanup(); + }); + + it("defaults carry call_description: true and the description property; PUT false round-trips", async () => { + const initial = (await (await owner.get(configPath)).json()) as AgentConfigResponse; + const names = initial.config.toolsBuiltin.map((tool) => tool.name); + // File tools lead the default toolset; the renamed shell tool follows. + expect(names.slice(0, 4)).toEqual(["read_file", "edit_file", "write_file", "exec_command"]); + const propsOf = (tool: ToolDefinitionConfig): Record => + (tool.parameters as { properties: Record }).properties; + for (const name of ["exec_command", "input_command", "run_subagent", "input_subagent"]) { + const tool = initial.config.toolsBuiltin.find((row) => row.name === name)!; + expect(tool.call_description).toBe(true); + expect(propsOf(tool)["description"]).toBeDefined(); + } + for (const name of ["read_file", "edit_file", "write_file"]) { + const tool = initial.config.toolsBuiltin.find((row) => row.name === name)!; + expect(tool.call_description).toBeUndefined(); + expect(propsOf(tool)["description"]).toBeUndefined(); + } + + // Flip exec_command's toggle off via the whole-table PUT. + const tools = initial.config.toolsBuiltin.map((row) => + row.name === "exec_command" ? { ...row, call_description: false } : row, + ); + const putRes = await owner.put(configPath, { config: { toolsBuiltin: tools } }); + expect(putRes.status).toBe(200); + const updated = (await putRes.json()) as AgentConfigResponse; + expect( + updated.config.toolsBuiltin.find((row) => row.name === "exec_command")!.call_description, + ).toBe(false); + // Written into the YAML itself; the rest of the config is preserved. + const yaml = await fs.readFile(systemConfigPath(t.root, projectId, "default_agent"), "utf8"); + expect(yaml).toContain("call_description: false"); + expect(updated.config.systemPrompt).toBe(initial.config.systemPrompt); + // The property stays declared in the stored parameters (filtering happens at assembly, not in the config). + expect( + propsOf(updated.config.toolsBuiltin.find((row) => row.name === "exec_command")!)[ + "description" + ], + ).toBeDefined(); + }); + + it("rejects a non-boolean call_description with 400", async () => { + const initial = (await (await owner.get(configPath)).json()) as AgentConfigResponse; + const tools = initial.config.toolsBuiltin.map((row) => + row.name === "exec_command" ? { ...row, call_description: "yes" } : row, + ); + const res = await owner.put(configPath, { config: { toolsBuiltin: tools } }); + expect(res.status).toBe(400); + const body = (await res.json()) as { error?: { message?: string } }; + expect(JSON.stringify(body)).toContain("call_description"); + }); +}); diff --git a/packages/web/src/features/agents/agent-settings-page.tsx b/packages/web/src/features/agents/agent-settings-page.tsx index 1961e6d..566f81d 100644 --- a/packages/web/src/features/agents/agent-settings-page.tsx +++ b/packages/web/src/features/agents/agent-settings-page.tsx @@ -29,6 +29,7 @@ import { Button } from "../../components/ui/button"; import { toastError, toastInfo, toastSuccess } from "../../components/ui/toast"; import { Input, Textarea } from "../../components/ui/input"; import { OptionMenu, type OptionMenuChoice } from "../../components/ui/option-menu"; +import { Switch } from "../../components/ui/switch"; import { ConfirmModal, useSaveConfirm } from "../../components/ui/confirm-modal"; import { Skeleton } from "../../components/ui/skeleton"; import { VaultTab } from "./vault-tab"; @@ -755,6 +756,12 @@ function ToolsTab({ data, onSave }: { data: AgentConfigResponse; onSave: SaveFn errs[`${i}-maxOutputLength`] = S.agent.toolFieldInvalid(row.base.name, "maxOutputLength"); } else tool.maxOutputLength = n; } + // call_description: missing = true, so flipping a stored-missing row back to on + // rewinds to "not written" instead of writing the default explicitly. + const origRow = data.config.toolsBuiltin[i]; + if (tool.call_description === true && origRow?.call_description === undefined) { + delete tool.call_description; + } tools.push(tool); } if (Object.keys(errs).length > 0) { @@ -772,7 +779,8 @@ function ToolsTab({ data, onSave }: { data: AgentConfigResponse; onSave: SaveFn return ( t.permission !== o.permission || t.timeoutMs !== o.timeoutMs || - t.maxOutputLength !== o.maxOutputLength + t.maxOutputLength !== o.maxOutputLength || + t.call_description !== o.call_description ); }); if (!dirty) { @@ -782,16 +790,24 @@ function ToolsTab({ data, onSave }: { data: AgentConfigResponse; onSave: SaveFn requestSave(() => void onSave({ config: { toolsBuiltin: tools } })); }; + /** Whether a tool's config schema declares the optional `description` call argument (only then does the per-row switch make sense). */ + const hasDescriptionProperty = (t: ToolDefinitionConfig): boolean => { + const props = (t.parameters as { properties?: Record } | undefined) + ?.properties; + return props !== undefined && props !== null && props["description"] !== undefined; + }; + return (
- +
+ @@ -829,11 +845,25 @@ function ToolsTab({ data, onSave }: { data: AgentConfigResponse; onSave: SaveFn onChange={(e) => update(i, { maxOutputLength: e.target.value })} /> + ))}
{S.common.name} {S.agent.toolPermission} {S.agent.toolTimeout} {S.agent.toolMaxOutput}{S.agent.toolCallDescription}
+ {/* Per-tool call_description switch (missing = on): shown only for tools whose + config schema actually declares the description argument. */} + {hasDescriptionProperty(row.base) ? ( + update(i, { base: { ...row.base, call_description: v } })} + aria-label={`${row.base.name} ${S.agent.toolCallDescription}`} + /> + ) : ( + — + )} +
+

{S.agent.callDescriptionHint}

{S.agent.mcpServers}

diff --git a/packages/web/src/features/chat/message-files-card.tsx b/packages/web/src/features/chat/message-files-card.tsx index 631dbb1..0a6b728 100644 --- a/packages/web/src/features/chat/message-files-card.tsx +++ b/packages/web/src/features/chat/message-files-card.tsx @@ -11,8 +11,8 @@ * it also doesn't render if none of the candidates exist — the heuristic extraction inevitably * matches error message examples, external paths, and other strings that can't actually be * opened, so this card is only responsible for "if you click it, it really opens". - * Doesn't include diff stats — the only built-in tool is exec_command (file writes happen inside - * the shell), so the protocol has no structured edit signal; this is just an aggregated view of + * Doesn't include diff stats — file writes may happen inside opaque exec_command shells, so + * the protocol has no reliable structured edit signal; this is just an aggregated view of * text references, hence the neutral "N files" title. */ import { useEffect, useMemo, useState } from "react"; diff --git a/packages/web/src/features/chat/tool-call-card.tsx b/packages/web/src/features/chat/tool-call-card.tsx index 9406375..3c92e79 100644 --- a/packages/web/src/features/chat/tool-call-card.tsx +++ b/packages/web/src/features/chat/tool-call-card.tsx @@ -28,19 +28,113 @@ import { LiveDuration } from "./live-duration"; import { SubagentCard } from "./subagent-card"; import type { StreamRenderContext } from "./message-stream"; +/** Tools that accept the optional model-written `description` argument. */ +const DESCRIBED_TOOLS = new Set([ + "exec_command", + "input_command", + "run_subagent", + "input_subagent", +]); + +/** The three file tools: previewed by their `file_path` argument. */ +const FILE_TOOLS = new Set(["read_file", "edit_file", "write_file"]); + +/** + * Shortens a path for one-line display: at most one parent directory plus the filename + * (`…/parent/file.ts`); paths already within that shape are shown as-is (same rule as the + * CLI's tool-render). The full path stays in the expanded arguments block. + */ +export function shortenPath(p: string): string { + const segments = p.split("/").filter((s) => s.length > 0); + if (segments.length <= 2) return p; + return `…/${segments[segments.length - 2]}/${segments[segments.length - 1]}`; +} + /** * Argument preview (same approach as the CLI's tool-render): exec_command shows `$ `, - * other tools show a single-line `name(args)` prefix. Arguments may be incomplete JSON - * (mid-stream), so extraction is done leniently. + * the file tools show their shortened file path, other tools show a single-line + * `name(args)` prefix. Arguments may be incomplete JSON (mid-stream), so extraction is done + * leniently. The preview deliberately keeps the real arguments (not the model-written + * description): it heads the approval row, and the user must approve the actual command, + * not the model's summary of it. */ -function previewArguments(name: string, argsJson: string): string { +export function previewArguments(name: string, argsJson: string): string { if (name === "exec_command") { const cmd = extractStringField(argsJson, "cmd"); if (cmd !== null) return `$ ${cmd.replace(/\s+/g, " ").trim()}`; } + if (FILE_TOOLS.has(name)) { + const filePath = extractStringField(argsJson, "file_path"); + if (filePath !== null) return shortenPath(filePath.replace(/\s+/g, " ").trim()); + } return argsJson.replace(/\s+/g, " ").trim(); } +/** + * Collapsed-header subtitle: the human-readable line next to the tool name — the + * model-written `description` argument for the command/subagent tools (declared in their + * config schema; per-tool `call_description: false` removes it, in which case the model + * never sends it), or the shortened file path for the file tools. Null when there is + * nothing beyond the raw arguments. + */ +export function headerSubtitle(name: string, argsJson: string): string | null { + if (DESCRIBED_TOOLS.has(name)) { + const desc = extractStringField(argsJson, "description"); + if (desc !== null) { + const line = desc.replace(/\s+/g, " ").trim(); + if (line) return line; + } + return null; + } + if (FILE_TOOLS.has(name)) { + const filePath = extractStringField(argsJson, "file_path"); + if (filePath !== null) { + const line = filePath.replace(/\s+/g, " ").trim(); + if (line) return shortenPath(line); + } + } + return null; +} + +/** + * Decoded file-tool payload for the pending-approval block: the user is approving a + * concrete rewrite (old_string/new_string/content), so the bare path is not enough — the + * actual arguments are rendered in the scrollable expanded style while the call is PENDING. + * Null for other tools or unparseable arguments (arguments are complete by approval time). + */ +export function pendingFilePayload(name: string, argsJson: string): string | null { + if (!FILE_TOOLS.has(name)) return null; + let parsed: unknown; + try { + parsed = JSON.parse(argsJson); + } catch { + return null; + } + if (parsed === null || typeof parsed !== "object") return null; + const args = parsed as Record; + const sections: string[] = []; + const push = (label: string, value: unknown): void => { + if (value === undefined) return; + if (typeof value === "string" && value.includes("\n")) { + sections.push(`${label}:\n${value}`); + } else { + sections.push(`${label}: ${typeof value === "string" ? value : JSON.stringify(value)}`); + } + }; + push("file_path", args["file_path"]); + if (name === "read_file") { + push("offset", args["offset"]); + push("limit", args["limit"]); + } else if (name === "edit_file") { + push("old_string", args["old_string"]); + push("new_string", args["new_string"]); + if (args["replace_all"] === true) push("replace_all", true); + } else if (name === "write_file") { + push("content", args["content"]); + } + return sections.join("\n"); +} + /** Extracts the current value of a string field from a possibly-incomplete JSON object string (a simplified version, good enough for preview purposes). */ function extractStringField(argsJson: string, field: string): string | null { const key = `"${field}"`; @@ -94,6 +188,7 @@ export function ToolCallCard({ item, ctx }: { item: ToolCallItem; ctx: StreamRen }, [hasNestedPending]); const preview = previewArguments(item.name, item.argumentsText); + const subtitle = headerSubtitle(item.name, item.argumentsText); // Executing = the call has finished streaming, output hasn't arrived yet, and it's not waiting on approval (approval wait time doesn't count toward execution). const executing = item.callComplete && !item.outputComplete && !pending; // Argument-generation segment (settled): the live execution timer accumulates on top of this as a baseline, so the displayed duration doesn't shrink back once output arrives. @@ -135,6 +230,12 @@ export function ToolCallCard({ item, ctx }: { item: ToolCallItem; ctx: StreamRen {item.name || S.chat.unknownTool} + {/* Human-readable subtitle: the model-written call description (command/subagent tools) or the file path (file tools). */} + {subtitle && ( + + {subtitle} + + )} {item.durationMs !== undefined ? ( humanizeDuration(item.durationMs) @@ -189,6 +290,17 @@ export function ToolCallCard({ item, ctx }: { item: ToolCallItem; ctx: StreamRen {preview}
+ {/* File tools: the one-line preview shows only the (shortened) path, but the user is + approving a concrete rewrite — render the decoded payload (old_string/new_string/ + content) in the scrollable expanded style while pending. */} + {(() => { + const payload = pendingFilePayload(item.name, item.argumentsText); + return payload !== null ? ( + 
+                {payload}
+
+ ) : null; + })()} ctx.onApprove(item.toolCallId, decision, ctx.origin)} /> diff --git a/packages/web/src/features/chat/use-files-panel.ts b/packages/web/src/features/chat/use-files-panel.ts index 24cf3eb..969278c 100644 --- a/packages/web/src/features/chat/use-files-panel.ts +++ b/packages/web/src/features/chat/use-files-panel.ts @@ -4,7 +4,7 @@ * tree" navigation command (driven by clicking a file chip inside a message). * * The panel's content is just WorkspaceBrowser's single directory-tree view — the protocol has no - * structured file-write signal at all (the only built-in tool is the opaque exec_command shell), + * structured file-write signal at all (file writes can happen inside the opaque exec_command shell), * so there's no "Agent output" list to maintain; a file clicked in a message jumps straight to * locating it in the tree. * diff --git a/packages/web/src/lib/strings-en.ts b/packages/web/src/lib/strings-en.ts index d083b8a..569df62 100644 --- a/packages/web/src/lib/strings-en.ts +++ b/packages/web/src/lib/strings-en.ts @@ -254,6 +254,9 @@ export const en: Strings = { "Can modify things. Needs manual confirmation when the approval mode is read-only.", toolTimeout: "timeoutMs", toolMaxOutput: "maxOutputLength", + toolCallDescription: "call_description", + callDescriptionHint: + "call_description: when on (the default), the tool's schema keeps the optional description argument — a model-written sentence about each call, shown to the user while it runs; when off, the argument is filtered out of the schema at assembly. Only tools whose parameters declare a description property can be toggled.", mcpServers: "MCP Servers (read-only)", defaultValue: "(default)", deleteAgent: "Delete agent", diff --git a/packages/web/src/lib/strings.ts b/packages/web/src/lib/strings.ts index 2e97196..1592515 100644 --- a/packages/web/src/lib/strings.ts +++ b/packages/web/src/lib/strings.ts @@ -232,6 +232,9 @@ export const zh = { permissionReadWriteDescription: "可修改。审批模式为 read-only 时需人工确认。", toolTimeout: "timeoutMs", toolMaxOutput: "maxOutputLength", + toolCallDescription: "call_description", + callDescriptionHint: + "call_description：开启（缺省）时该工具的 schema 保留可选的 description 参数——模型为每次调用写一句说明，运行期间展示给用户；关闭则装配时从 schema 滤除该参数。仅参数中定义了 description 属性的工具可切换。", mcpServers: "MCP Server（只读）", defaultValue: "（缺省）", /** Reset link next to the runtime dropdowns: rewinds the local pick back to "not overridden" (the menus offer no inherit row). */ diff --git a/packages/web/test/tool-call-preview.test.ts b/packages/web/test/tool-call-preview.test.ts new file mode 100644 index 0000000..9e6807e --- /dev/null +++ b/packages/web/test/tool-call-preview.test.ts @@ -0,0 +1,116 @@ +/** + * tool-call-card.tsx preview helpers: previewArguments keeps the real arguments (what heads + * the approval row), headerSubtitle surfaces the model-written `description` argument for + * the command/subagent tools and the shortened file path for the file tools, and + * pendingFilePayload decodes the file-tool arguments so a pending approval shows the actual + * rewrite. All must tolerate incomplete mid-stream JSON. + */ +import { describe, expect, it } from "vitest"; +import { + headerSubtitle, + pendingFilePayload, + previewArguments, + shortenPath, +} from "../src/features/chat/tool-call-card"; + +describe("previewArguments", () => { + it("renders exec_command as $ ", () => { + expect(previewArguments("exec_command", '{"cmd":"ls -la"}')).toBe("$ ls -la"); + // Mid-stream (incomplete JSON) still extracts the cmd prefix. + expect(previewArguments("exec_command", '{"cmd":"echo h')).toBe("$ echo h"); + }); + + it("renders the file tools by their shortened file_path", () => { + expect(previewArguments("read_file", '{"file_path":"src/app.py","offset":3}')).toBe( + "src/app.py", + ); + expect( + previewArguments("edit_file", '{"file_path":"a.txt","old_string":"x","new_string":"y"}'), + ).toBe("a.txt"); + expect(previewArguments("write_file", '{"file_path":"packages/core/src/state/out.ts"}')).toBe( + "…/state/out.ts", + ); + }); + + it("keeps the real command even when a description argument is present (approval fidelity)", () => { + expect( + previewArguments("exec_command", '{"cmd":"rm -rf build","description":"Clean caches"}'), + ).toBe("$ rm -rf build"); + }); + + it("falls back to the single-line raw arguments for other tools", () => { + expect(previewArguments("search", '{"q": "a\n b"}')).toBe('{"q": "a b"}'); + }); +}); + +describe("shortenPath", () => { + it("keeps at most one parent directory plus the filename", () => { + expect(shortenPath("file.ts")).toBe("file.ts"); + expect(shortenPath("src/file.ts")).toBe("src/file.ts"); + expect(shortenPath("/etc/hosts")).toBe("/etc/hosts"); + expect(shortenPath("packages/core/src/state/default-config.ts")).toBe( + "…/state/default-config.ts", + ); + }); +}); + +describe("headerSubtitle", () => { + it("shows the description for the command/subagent tools when present", () => { + expect( + headerSubtitle("exec_command", '{"cmd":"ls","description":"List workspace files"}'), + ).toBe("List workspace files"); + expect( + headerSubtitle("run_subagent", '{"prompt":"p","description":"Delegating research"}'), + ).toBe("Delegating research"); + expect( + headerSubtitle("input_command", '{"process_id":"proc-1","description":"Poll the build"}'), + ).toBe("Poll the build"); + expect( + headerSubtitle("input_subagent", '{"subagent_id":"s-1","description":"Follow up"}'), + ).toBe("Follow up"); + }); + + it("is null when the description is absent or empty (call_description off = the model never sends it)", () => { + expect(headerSubtitle("exec_command", '{"cmd":"ls"}')).toBeNull(); + expect(headerSubtitle("exec_command", '{"cmd":"ls","description":""}')).toBeNull(); + }); + + it("shows the shortened file path for the file tools and nothing for others", () => { + expect(headerSubtitle("read_file", '{"file_path":"src/app.py"}')).toBe("src/app.py"); + expect(headerSubtitle("edit_file", '{"file_path":"packages/web/src/a.tsx"}')).toBe( + "…/src/a.tsx", + ); + expect(headerSubtitle("write_file", '{"file_path":"out.md"}')).toBe("out.md"); + expect(headerSubtitle("search", '{"q":"x"}')).toBeNull(); + }); + + it("folds a multi-line description to one line", () => { + expect(headerSubtitle("exec_command", '{"cmd":"ls","description":"one\\ntwo"}')).toBe( + "one two", + ); + }); +}); + +describe("pendingFilePayload", () => { + it("decodes the edit_file rewrite so the approval block shows it", () => { + const payload = pendingFilePayload( + "edit_file", + JSON.stringify({ file_path: "src/app.py", old_string: "a\nb", new_string: "a\nc" }), + ); + expect(payload).toBe("file_path: src/app.py\nold_string:\na\nb\nnew_string:\na\nc"); + }); + + it("decodes write_file content and read_file window arguments", () => { + expect(pendingFilePayload("write_file", '{"file_path":"out.md","content":"hello"}')).toBe( + "file_path: out.md\ncontent: hello", + ); + expect(pendingFilePayload("read_file", '{"file_path":"a.txt","offset":3,"limit":5}')).toBe( + "file_path: a.txt\noffset: 3\nlimit: 5", + ); + }); + + it("returns null for non-file tools and incomplete JSON", () => { + expect(pendingFilePayload("exec_command", '{"cmd":"ls"}')).toBeNull(); + expect(pendingFilePayload("edit_file", '{"file_path":"a.txt","old_str')).toBeNull(); + }); +});