diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 81003838a..80a3853f2 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog ## [Unreleased] + ### Added - Added `isolated` option to run tasks in isolated git worktrees @@ -9,6 +10,12 @@ ### Changed +- Updated edit tool parameters from camelCase to snake_case (oldText → old_text, newText → new_text) +- Updated grep tool parameters from camelCase to snake_case (ignoreCase → ignore_case, caseSensitive → case_sensitive, outputMode → output_mode, headLimit → head_limit) +- Updated python tool parameters from camelCase to snake_case (timeoutMs → timeout_ms) +- Updated todo-write tool parameters from camelCase to snake_case (activeForm → active_form) +- Updated MCP tool name parsing to handle redundant server name prefixes +- Marked read tool as non-abortable to improve performance - Simplified tool parameter descriptions across all tools for brevity - Updated find tool to always sort results by modification time - Changed web-fetch timeout default from 20s to 45s maximum diff --git a/packages/coding-agent/docs/hooks.md b/packages/coding-agent/docs/hooks.md index f83b19d14..3288371bc 100644 --- a/packages/coding-agent/docs/hooks.md +++ b/packages/coding-agent/docs/hooks.md @@ -345,10 +345,10 @@ Tool inputs: - `bash`: `{ command, timeout? }` - `read`: `{ path, offset?, limit? }` - `write`: `{ path, content }` -- `edit`: `{ path, oldText, newText }` +- `edit`: `{ path, old_text, new_text }` - `ls`: `{ path?, limit? }` - `find`: `{ pattern, path?, limit? }` -- `grep`: `{ pattern, path?, glob?, ignoreCase?, literal?, context?, limit? }` +- `grep`: `{ pattern, path?, glob?, ignore_case?, literal?, context?, limit? }` #### tool_result diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index 294e100eb..4affb0147 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -587,7 +587,7 @@ export class AgentSession { const args = toolCall.arguments; if (!args || typeof args !== "object" || Array.isArray(args)) return; - if ("oldText" in args || "newText" in args) return; + if ("old_text" in args || "new_text" in args) return; const path = typeof args.path === "string" ? args.path : undefined; if (!path) return; @@ -632,7 +632,7 @@ export class AgentSession { const args = toolCall.arguments; if (!args || typeof args !== "object" || Array.isArray(args)) return; - if ("oldText" in args || "newText" in args) return; + if ("old_text" in args || "new_text" in args) return; const path = typeof args.path === "string" ? args.path : undefined; const diff = typeof args.diff === "string" ? args.diff : undefined; diff --git a/packages/coding-agent/src/core/cursor/exec-bridge.ts b/packages/coding-agent/src/core/cursor/exec-bridge.ts index 7b57c35f7..af1b18eca 100644 --- a/packages/coding-agent/src/core/cursor/exec-bridge.ts +++ b/packages/coding-agent/src/core/cursor/exec-bridge.ts @@ -164,11 +164,11 @@ export class CursorExecHandlers implements ICursorExecHandlers { pattern: args.pattern, path: args.path || undefined, glob: args.glob || undefined, - outputMode: args.outputMode || undefined, + output_mode: args.outputMode || undefined, context: args.context ?? args.contextBefore ?? args.contextAfter ?? undefined, - ignoreCase: args.caseInsensitive || undefined, + ignore_case: args.caseInsensitive || undefined, type: args.type || undefined, - headLimit: args.headLimit ?? undefined, + head_limit: args.headLimit ?? undefined, multiline: args.multiline || undefined, }); return toolResultMessage; diff --git a/packages/coding-agent/src/core/mcp/tool-bridge.ts b/packages/coding-agent/src/core/mcp/tool-bridge.ts index 709c094ad..8a9f8a38b 100644 --- a/packages/coding-agent/src/core/mcp/tool-bridge.ts +++ b/packages/coding-agent/src/core/mcp/tool-bridge.ts @@ -66,21 +66,38 @@ function formatMCPContent(content: MCPContent[]): string { /** * Create a unique tool name for an MCP tool. - * Prefixes with server name to avoid conflicts. + * + * Prefixes with server name to avoid conflicts. If the tool name already + * starts with the server name (e.g., server "puppeteer" with tool + * "puppeteer_screenshot"), strips the redundant prefix to produce + * "mcp_puppeteer_screenshot" instead of "mcp_puppeteer_puppeteer_screenshot". */ export function createMCPToolName(serverName: string, toolName: string): string { - // Use underscore separator since tool names can't have special chars - return `mcp_${serverName}_${toolName}`; + // Strip redundant server name prefix from tool name if present + const prefixWithUnderscore = `${serverName}_`; + const prefixWithHyphen = `${serverName}-`; + + let normalizedToolName = toolName; + if (toolName.startsWith(prefixWithUnderscore)) { + normalizedToolName = toolName.slice(prefixWithUnderscore.length); + } else if (toolName.startsWith(prefixWithHyphen)) { + normalizedToolName = toolName.slice(prefixWithHyphen.length); + } + + return `mcp_${serverName}_${normalizedToolName}`; } /** * Parse an MCP tool name back to server and tool components. + * + * Note: This returns the normalized tool name (with server prefix stripped). + * The original MCP tool name may have had the server name as a prefix. */ export function parseMCPToolName(name: string): { serverName: string; toolName: string } | null { if (!name.startsWith("mcp_")) return null; const rest = name.slice(4); - const underscoreIdx = rest.lastIndexOf("_"); + const underscoreIdx = rest.indexOf("_"); if (underscoreIdx === -1) return null; return { diff --git a/packages/coding-agent/src/core/settings-manager.ts b/packages/coding-agent/src/core/settings-manager.ts index d362d4c4b..adc708c93 100644 --- a/packages/coding-agent/src/core/settings-manager.ts +++ b/packages/coding-agent/src/core/settings-manager.ts @@ -120,7 +120,7 @@ export interface PythonSettings { export interface EditSettings { fuzzyMatch?: boolean; // default: true (accept high-confidence fuzzy matches for whitespace/indentation) fuzzyThreshold?: number; // default: 0.95 (similarity threshold for fuzzy matching) - patchMode?: boolean; // default: true (use codex-style apply-patch format instead of oldText/newText) + patchMode?: boolean; // default: true (use codex-style apply-patch format instead of old_text/new_text) streamingAbort?: boolean; // default: false (abort streaming edit tool calls when patch preview fails) } diff --git a/packages/coding-agent/src/core/tools/grep.ts b/packages/coding-agent/src/core/tools/grep.ts index a0653ac12..bbc4a7f9b 100644 --- a/packages/coding-agent/src/core/tools/grep.ts +++ b/packages/coding-agent/src/core/tools/grep.ts @@ -29,18 +29,18 @@ const grepSchema = Type.Object({ path: Type.Optional(Type.String({ description: "Directory or file to search (default: cwd)" })), glob: Type.Optional(Type.String({ description: "Glob filter, e.g. '*.ts', '**/*.spec.ts'" })), type: Type.Optional(Type.String({ description: "File type filter, e.g. 'ts', 'py', 'rust'" })), - ignoreCase: Type.Optional(Type.Boolean({ description: "Force case-insensitive (default: smart-case)" })), - caseSensitive: Type.Optional(Type.Boolean({ description: "Force case-sensitive (default: smart-case)" })), + ignore_case: Type.Optional(Type.Boolean({ description: "Force case-insensitive (default: smart-case)" })), + case_sensitive: Type.Optional(Type.Boolean({ description: "Force case-sensitive (default: smart-case)" })), literal: Type.Optional(Type.Boolean({ description: "Treat pattern as literal, not regex (default: false)" })), multiline: Type.Optional(Type.Boolean({ description: "Match across line boundaries (default: false)" })), context: Type.Optional(Type.Number({ description: "Lines of context before/after match (default: 0)" })), limit: Type.Optional(Type.Number({ description: "Max matches to return (default: 100)" })), - outputMode: Type.Optional( + output_mode: Type.Optional( StringEnum(["content", "files_with_matches", "count"], { description: "Output format (default: content)", }), ), - headLimit: Type.Optional(Type.Number({ description: "Truncate output to first N results" })), + head_limit: Type.Optional(Type.Number({ description: "Truncate output to first N results" })), offset: Type.Optional(Type.Number({ description: "Skip first N results (default: 0)" })), }); @@ -88,14 +88,14 @@ interface GrepParams { path?: string; glob?: string; type?: string; - ignoreCase?: boolean; - caseSensitive?: boolean; + ignore_case?: boolean; + case_sensitive?: boolean; literal?: boolean; multiline?: boolean; context?: number; limit?: number; - outputMode?: "content" | "files_with_matches" | "count"; - headLimit?: number; + output_mode?: "content" | "files_with_matches" | "count"; + head_limit?: number; offset?: number; } @@ -154,14 +154,14 @@ export class GrepTool implements AgentTool { path: searchDir, glob, type, - ignoreCase, - caseSensitive, + ignore_case, + case_sensitive, literal, multiline, context, limit, - outputMode, - headLimit, + output_mode, + head_limit, offset, } = params; @@ -195,9 +195,9 @@ export class GrepTool implements AgentTool { } const contextValue = context && context > 0 ? context : 0; const effectiveLimit = Math.max(1, limit ?? DEFAULT_LIMIT); - const effectiveOutputMode = outputMode ?? "content"; + const effectiveOutputMode = output_mode ?? "content"; const effectiveOffset = offset && offset > 0 ? offset : 0; - const hasHeadLimit = headLimit !== undefined && headLimit > 0; + const hasHeadLimit = head_limit !== undefined && head_limit > 0; const formatPath = (filePath: string): string => { if (isDirectory) { @@ -237,9 +237,9 @@ export class GrepTool implements AgentTool { args.push("--json", "--line-number", "--color=never", "--hidden"); } - if (caseSensitive) { + if (case_sensitive) { args.push("--case-sensitive"); - } else if (ignoreCase) { + } else if (ignore_case) { args.push("--ignore-case"); } else { args.push("--smart-case"); @@ -321,13 +321,13 @@ export class GrepTool implements AgentTool { }; } - // Apply offset and headLimit + // Apply offset and head_limit let processedLines = lines; if (effectiveOffset > 0) { processedLines = processedLines.slice(effectiveOffset); } if (hasHeadLimit) { - processedLines = processedLines.slice(0, headLimit); + processedLines = processedLines.slice(0, head_limit); } let simpleMatchCount = 0; @@ -395,7 +395,7 @@ export class GrepTool implements AgentTool { })), mode: effectiveOutputMode, truncated: truncatedByHeadLimit, - headLimitReached: truncatedByHeadLimit ? headLimit : undefined, + headLimitReached: truncatedByHeadLimit ? head_limit : undefined, }, }; } @@ -412,7 +412,7 @@ export class GrepTool implements AgentTool { files: simpleFileList, mode: effectiveOutputMode, truncated: truncatedByHeadLimit, - headLimitReached: truncatedByHeadLimit ? headLimit : undefined, + headLimitReached: truncatedByHeadLimit ? head_limit : undefined, }, }; } @@ -528,13 +528,13 @@ export class GrepTool implements AgentTool { }; } - // Apply offset and headLimit to output lines + // Apply offset and head_limit to output lines let processedLines = outputLines; if (effectiveOffset > 0) { processedLines = processedLines.slice(effectiveOffset); } if (hasHeadLimit) { - processedLines = processedLines.slice(0, headLimit); + processedLines = processedLines.slice(0, head_limit); } // Apply byte truncation (no line limit since we already have match limit) @@ -554,7 +554,7 @@ export class GrepTool implements AgentTool { })), mode: effectiveOutputMode, truncated: matchLimitReached || truncation.truncated || truncatedByHeadLimit, - headLimitReached: truncatedByHeadLimit ? headLimit : undefined, + headLimitReached: truncatedByHeadLimit ? head_limit : undefined, }; // Build notices @@ -598,13 +598,13 @@ interface GrepRenderArgs { path?: string; glob?: string; type?: string; - ignoreCase?: boolean; - caseSensitive?: boolean; + ignore_case?: boolean; + case_sensitive?: boolean; literal?: boolean; multiline?: boolean; context?: number; limit?: number; - outputMode?: string; + output_mode?: string; } const COLLAPSED_LIST_LIMIT = PREVIEW_LIMITS.COLLAPSED_ITEMS; @@ -621,10 +621,10 @@ export const grepToolRenderer = { if (args.path) meta.push(`in ${args.path}`); if (args.glob) meta.push(`glob:${args.glob}`); if (args.type) meta.push(`type:${args.type}`); - if (args.outputMode && args.outputMode !== "files_with_matches") meta.push(`mode:${args.outputMode}`); - if (args.caseSensitive) { + if (args.output_mode && args.output_mode !== "files_with_matches") meta.push(`mode:${args.output_mode}`); + if (args.case_sensitive) { meta.push("case:sensitive"); - } else if (args.ignoreCase) { + } else if (args.ignore_case) { meta.push("case:insensitive"); } if (args.literal) meta.push("literal"); diff --git a/packages/coding-agent/src/core/tools/patch/index.ts b/packages/coding-agent/src/core/tools/patch/index.ts index 35ff6e557..a127a26da 100644 --- a/packages/coding-agent/src/core/tools/patch/index.ts +++ b/packages/coding-agent/src/core/tools/patch/index.ts @@ -91,8 +91,8 @@ export { ApplyPatchError, EditMatchError, ParseError } from "./types"; const replaceEditSchema = Type.Object({ path: Type.String({ description: "File path (relative or absolute)" }), - oldText: Type.String({ description: "Text to find (fuzzy whitespace matching enabled)" }), - newText: Type.String({ description: "Replacement text" }), + old_text: Type.String({ description: "Text to find (fuzzy whitespace matching enabled)" }), + new_text: Type.String({ description: "Replacement text" }), all: Type.Optional(Type.Boolean({ description: "Replace all occurrences (default: unique match required)" })), }); @@ -107,7 +107,7 @@ const patchEditSchema = Type.Object({ diff: Type.Optional(Type.String({ description: "Diff hunks (update) or full content (create)" })), }); -export type ReplaceParams = { path: string; oldText: string; newText: string; all?: boolean }; +export type ReplaceParams = { path: string; old_text: string; new_text: string; all?: boolean }; export type PatchParams = { path: string; op?: string; rename?: string; diff?: string }; // ═══════════════════════════════════════════════════════════════════════════ @@ -346,14 +346,14 @@ export class EditTool implements AgentTool { // ───────────────────────────────────────────────────────────────── // Replace mode execution // ───────────────────────────────────────────────────────────────── - const { path, oldText, newText, all } = params as ReplaceParams; + const { path, old_text, new_text, all } = params as ReplaceParams; if (path.endsWith(".ipynb")) { throw new Error("Cannot edit Jupyter notebooks with the Edit tool. Use the NotebookEdit tool instead."); } - if (oldText.length === 0) { - throw new Error("oldText must not be empty."); + if (old_text.length === 0) { + throw new Error("old_text must not be empty."); } const absolutePath = resolveToCwd(path, this.session.cwd); @@ -367,8 +367,8 @@ export class EditTool implements AgentTool { const { bom, text: content } = stripBom(rawContent); const originalEnding = detectLineEnding(content); const normalizedContent = normalizeToLF(content); - const normalizedOldText = normalizeToLF(oldText); - const normalizedNewText = normalizeToLF(newText); + const normalizedOldText = normalizeToLF(old_text); + const normalizedNewText = normalizeToLF(new_text); const result = replaceText(normalizedContent, normalizedOldText, normalizedNewText, { fuzzy: this.allowFuzzy, diff --git a/packages/coding-agent/src/core/tools/python.ts b/packages/coding-agent/src/core/tools/python.ts index 2a323a607..26ff14165 100644 --- a/packages/coding-agent/src/core/tools/python.ts +++ b/packages/coding-agent/src/core/tools/python.ts @@ -46,7 +46,7 @@ export const pythonSchema = Type.Object({ }), { description: "Cells to execute sequentially in persistent kernel" }, ), - timeoutMs: Type.Optional(Type.Number({ description: "Timeout in ms (default: 30000)" })), + timeout_ms: Type.Optional(Type.Number({ description: "Timeout in ms (default: 30000)" })), cwd: Type.Optional(Type.String({ description: "Working directory (default: cwd)" })), reset: Type.Optional(Type.Boolean({ description: "Restart kernel before execution" })), }); @@ -166,7 +166,7 @@ export class PythonTool implements AgentTool { throw new Error("Python tool requires a session when not using proxy executor"); } - const { cells, timeoutMs = 30000, cwd, reset } = params; + const { cells, timeout_ms = 30000, cwd, reset } = params; const controller = new AbortController(); const onAbort = () => controller.abort(); signal?.addEventListener("abort", onAbort, { once: true }); @@ -256,7 +256,7 @@ export class PythonTool implements AgentTool { const sessionId = sessionFile ? `session:${sessionFile}:cwd:${commandCwd}` : `cwd:${commandCwd}`; const baseExecutorOptions: Omit = { cwd: commandCwd, - timeoutMs, + timeoutMs: timeout_ms, signal: controller.signal, sessionId, kernelMode: this.session.settings?.getPythonKernelMode?.() ?? "session", diff --git a/packages/coding-agent/src/core/tools/read.ts b/packages/coding-agent/src/core/tools/read.ts index d5c12ae86..db94301c5 100644 --- a/packages/coding-agent/src/core/tools/read.ts +++ b/packages/coding-agent/src/core/tools/read.ts @@ -4,7 +4,7 @@ import type { AgentTool, AgentToolContext, AgentToolResult, AgentToolUpdateCallb import type { ImageContent, TextContent } from "@oh-my-pi/pi-ai"; import type { Component } from "@oh-my-pi/pi-tui"; import { Text } from "@oh-my-pi/pi-tui"; -import { ptree, untilAborted } from "@oh-my-pi/pi-utils"; +import { ptree } from "@oh-my-pi/pi-utils"; import { Type } from "@sinclair/typebox"; import { CONFIG_DIR_NAME } from "../../config"; import type { Theme } from "../../modes/interactive/theme/theme"; @@ -388,6 +388,7 @@ export class ReadTool implements AgentTool { public readonly label = "Read"; public readonly description: string; public readonly parameters = readSchema; + public readonly nonAbortable = true; private readonly session: ToolSession; private readonly autoResizeImages: boolean; @@ -414,248 +415,236 @@ export class ReadTool implements AgentTool { const { path: readPath, offset, limit, lines } = params; const absolutePath = resolveReadPath(readPath, this.session.cwd); - return untilAborted(signal, async () => { - let isDirectory = false; - let fileSize = 0; - try { - const stat = await Bun.file(absolutePath).stat(); - fileSize = stat.size; - isDirectory = stat.isDirectory(); - } catch (error) { - if (isNotFoundError(error)) { - let message = `File not found: ${readPath}`; + let isDirectory = false; + let fileSize = 0; + try { + const stat = await Bun.file(absolutePath).stat(); + fileSize = stat.size; + isDirectory = stat.isDirectory(); + } catch (error) { + if (isNotFoundError(error)) { + let message = `File not found: ${readPath}`; - // Skip fuzzy matching for remote mounts (sshfs) to avoid hangs - if (!isRemoteMountPath(absolutePath)) { - const suggestions = await findReadPathSuggestions(readPath, this.session.cwd, signal); + // Skip fuzzy matching for remote mounts (sshfs) to avoid hangs + if (!isRemoteMountPath(absolutePath)) { + const suggestions = await findReadPathSuggestions(readPath, this.session.cwd, signal); - if (suggestions?.suggestions.length) { - const scopeLabel = suggestions.scopeLabel ? ` in ${suggestions.scopeLabel}` : ""; - message += `\n\nClosest matches${scopeLabel}:\n${suggestions.suggestions.map((match) => `- ${match}`).join("\n")}`; - if (suggestions.truncated) { - message += `\n[Search truncated to first ${MAX_FUZZY_CANDIDATES} paths. Refine the path if the match isn't listed.]`; - } - } else if (suggestions?.error) { - message += `\n\nFuzzy match failed: ${suggestions.error}`; - } else if (suggestions?.scopeLabel) { - message += `\n\nNo similar paths found in ${suggestions.scopeLabel}.`; + if (suggestions?.suggestions.length) { + const scopeLabel = suggestions.scopeLabel ? ` in ${suggestions.scopeLabel}` : ""; + message += `\n\nClosest matches${scopeLabel}:\n${suggestions.suggestions.map((match) => `- ${match}`).join("\n")}`; + if (suggestions.truncated) { + message += `\n[Search truncated to first ${MAX_FUZZY_CANDIDATES} paths. Refine the path if the match isn't listed.]`; } + } else if (suggestions?.error) { + message += `\n\nFuzzy match failed: ${suggestions.error}`; + } else if (suggestions?.scopeLabel) { + message += `\n\nNo similar paths found in ${suggestions.scopeLabel}.`; } - - throw new Error(message); } - throw error; + + throw new Error(message); } + throw error; + } - if (isDirectory) { - const lsResult = await this.lsTool.execute(toolCallId, { path: readPath, limit }, signal); - return { - content: lsResult.content, - details: { redirectedTo: "ls", truncation: lsResult.details?.truncation }, - }; - } + if (isDirectory) { + const lsResult = await this.lsTool.execute(toolCallId, { path: readPath, limit }, signal); + return { + content: lsResult.content, + details: { redirectedTo: "ls", truncation: lsResult.details?.truncation }, + }; + } - const mimeType = await detectSupportedImageMimeTypeFromFile(absolutePath); - const ext = path.extname(absolutePath).toLowerCase(); + const mimeType = await detectSupportedImageMimeTypeFromFile(absolutePath); + const ext = path.extname(absolutePath).toLowerCase(); - // Read the file based on type - let content: (TextContent | ImageContent)[]; - let details: ReadToolDetails | undefined; + // Read the file based on type + let content: (TextContent | ImageContent)[]; + let details: ReadToolDetails | undefined; - if (mimeType) { - if (fileSize > MAX_IMAGE_SIZE) { - const sizeStr = formatSize(fileSize); + if (mimeType) { + if (fileSize > MAX_IMAGE_SIZE) { + const sizeStr = formatSize(fileSize); + const maxStr = formatSize(MAX_IMAGE_SIZE); + throw new Error(`Image file too large: ${sizeStr} exceeds ${maxStr} limit.`); + } else { + // Read as image (binary) + const file = Bun.file(absolutePath); + const buffer = await file.arrayBuffer(); + + // Check actual buffer size after reading to prevent OOM during serialization + if (buffer.byteLength > MAX_IMAGE_SIZE) { + const sizeStr = formatSize(buffer.byteLength); const maxStr = formatSize(MAX_IMAGE_SIZE); - content = [ - { - type: "text", - text: `[Image file too large: ${sizeStr} exceeds ${maxStr} limit. Use an image viewer or resize the image.]`, - }, - ]; + throw new Error(`Image file too large: ${sizeStr} exceeds ${maxStr} limit.`); } else { - // Read as image (binary) - const file = Bun.file(absolutePath); - const buffer = await file.arrayBuffer(); + const base64 = Buffer.from(buffer).toString("base64"); - // Check actual buffer size after reading to prevent OOM during serialization - if (buffer.byteLength > MAX_IMAGE_SIZE) { - const sizeStr = formatSize(buffer.byteLength); - const maxStr = formatSize(MAX_IMAGE_SIZE); - content = [ - { - type: "text", - text: `[Image file too large: ${sizeStr} exceeds ${maxStr} limit. Use an image viewer or resize the image.]`, - }, - ]; - } else { - const base64 = Buffer.from(buffer).toString("base64"); + if (this.autoResizeImages) { + // Resize image if needed - catch errors from WASM + try { + const resized = await resizeImage({ type: "image", data: base64, mimeType }); + const dimensionNote = formatDimensionNote(resized); - if (this.autoResizeImages) { - // Resize image if needed - catch errors from WASM - try { - const resized = await resizeImage({ type: "image", data: base64, mimeType }); - const dimensionNote = formatDimensionNote(resized); - - let textNote = `Read image file [${resized.mimeType}]`; - if (dimensionNote) { - textNote += `\n${dimensionNote}`; - } - - content = [ - { type: "text", text: textNote }, - { type: "image", data: resized.data, mimeType: resized.mimeType }, - ]; - } catch { - // Fall back to original image on resize failure - content = [ - { type: "text", text: `Read image file [${mimeType}]` }, - { type: "image", data: base64, mimeType }, - ]; + let textNote = `Read image file [${resized.mimeType}]`; + if (dimensionNote) { + textNote += `\n${dimensionNote}`; } - } else { + + content = [ + { type: "text", text: textNote }, + { type: "image", data: resized.data, mimeType: resized.mimeType }, + ]; + } catch { + // Fall back to original image on resize failure content = [ { type: "text", text: `Read image file [${mimeType}]` }, { type: "image", data: base64, mimeType }, ]; } - } - } - } else if (CONVERTIBLE_EXTENSIONS.has(ext)) { - // Convert document via markitdown - const result = await convertWithMarkitdown(absolutePath, signal); - if (result.ok) { - // Apply truncation to converted content - const truncation = truncateHead(result.content); - let outputText = truncation.content; - - if (truncation.truncated) { - outputText += `\n\n[Document converted via markitdown. Output truncated to ${formatSize(DEFAULT_MAX_BYTES)}]`; - details = { truncation }; - } - - content = [{ type: "text", text: outputText }]; - } else if (result.error) { - // markitdown not available or failed - const errorMsg = - result.error === "markitdown not found" - ? `markitdown not installed. Install with: pip install markitdown` - : result.error || "conversion failed"; - content = [{ type: "text", text: `[Cannot read ${ext} file: ${errorMsg}]` }]; - } else { - content = [{ type: "text", text: `[Cannot read ${ext} file: conversion failed]` }]; - } - } else { - // Read as text - const file = Bun.file(absolutePath); - const textContent = await file.text(); - const allLines = textContent.split("\n"); - const totalFileLines = allLines.length; - - // Apply offset if specified (1-indexed to 0-indexed) - const startLine = offset ? Math.max(0, offset - 1) : 0; - const startLineDisplay = startLine + 1; // For display (1-indexed) - - // Check if offset is out of bounds - return graceful message instead of throwing - if (startLine >= allLines.length) { - const suggestion = - allLines.length === 0 - ? "The file is empty." - : `Use offset=1 to read from the start, or offset=${allLines.length} to read the last line.`; - return { - content: [ - { - type: "text", - text: `Offset ${offset} is beyond end of file (${allLines.length} lines total). ${suggestion}`, - }, - ], - }; - } - - // If limit is specified by user, use it; otherwise we'll let truncateHead decide - let selectedContent: string; - let userLimitedLines: number | undefined; - if (limit !== undefined) { - const endLine = Math.min(startLine + limit, allLines.length); - selectedContent = allLines.slice(startLine, endLine).join("\n"); - userLimitedLines = endLine - startLine; - } else { - selectedContent = allLines.slice(startLine).join("\n"); - } - - // Apply truncation (respects both line and byte limits) - const truncation = truncateHead(selectedContent); - - // Add line numbers if requested (uses setting default if not specified) - const shouldAddLineNumbers = lines ?? this.defaultLineNumbers; - const prependLineNumbers = (text: string, startNum: number): string => { - const textLines = text.split("\n"); - const lastLineNum = startNum + textLines.length - 1; - const padWidth = String(lastLineNum).length; - return textLines - .map((line, i) => { - const lineNum = String(startNum + i).padStart(padWidth, " "); - return `${lineNum}\t${line}`; - }) - .join("\n"); - }; - - let outputText: string; - - if (truncation.firstLineExceedsLimit) { - const firstLine = allLines[startLine] ?? ""; - const firstLineBytes = Buffer.byteLength(firstLine, "utf-8"); - const snippet = truncateStringToBytesFromStart(firstLine, DEFAULT_MAX_BYTES); - const shownSize = formatSize(snippet.bytes); - - outputText = shouldAddLineNumbers ? prependLineNumbers(snippet.text, startLineDisplay) : snippet.text; - if (snippet.text.length > 0) { - outputText += `\n\n[Line ${startLineDisplay} is ${formatSize( - firstLineBytes, - )}, exceeds ${formatSize(DEFAULT_MAX_BYTES)} limit. Showing first ${shownSize} of the line.]`; } else { - outputText = `[Line ${startLineDisplay} is ${formatSize( - firstLineBytes, - )}, exceeds ${formatSize(DEFAULT_MAX_BYTES)} limit. Unable to display a valid UTF-8 snippet.]`; + content = [ + { type: "text", text: `Read image file [${mimeType}]` }, + { type: "image", data: base64, mimeType }, + ]; } + } + } + } else if (CONVERTIBLE_EXTENSIONS.has(ext)) { + // Convert document via markitdown + const result = await convertWithMarkitdown(absolutePath, signal); + if (result.ok) { + // Apply truncation to converted content + const truncation = truncateHead(result.content); + let outputText = truncation.content; + + if (truncation.truncated) { + outputText += `\n\n[Document converted via markitdown. Output truncated to ${formatSize(DEFAULT_MAX_BYTES)}]`; details = { truncation }; - } else if (truncation.truncated) { - // Truncation occurred - build actionable notice - const endLineDisplay = startLineDisplay + truncation.outputLines - 1; - const nextOffset = endLineDisplay + 1; - - outputText = shouldAddLineNumbers - ? prependLineNumbers(truncation.content, startLineDisplay) - : truncation.content; - - if (truncation.truncatedBy === "lines") { - outputText += `\n\n[Showing lines ${startLineDisplay}-${endLineDisplay} of ${totalFileLines}. Use offset=${nextOffset} to continue]`; - } else { - outputText += `\n\n[Showing lines ${startLineDisplay}-${endLineDisplay} of ${totalFileLines} (${formatSize( - DEFAULT_MAX_BYTES, - )} limit). Use offset=${nextOffset} to continue]`; - } - details = { truncation }; - } else if (userLimitedLines !== undefined && startLine + userLimitedLines < allLines.length) { - // User specified limit, there's more content, but no truncation - const remaining = allLines.length - (startLine + userLimitedLines); - const nextOffset = startLine + userLimitedLines + 1; - - outputText = shouldAddLineNumbers - ? prependLineNumbers(truncation.content, startLineDisplay) - : truncation.content; - outputText += `\n\n[${remaining} more lines in file. Use offset=${nextOffset} to continue]`; - } else { - // No truncation, no user limit exceeded - outputText = shouldAddLineNumbers - ? prependLineNumbers(truncation.content, startLineDisplay) - : truncation.content; } content = [{ type: "text", text: outputText }]; + } else if (result.error) { + // markitdown not available or failed + const errorMsg = + result.error === "markitdown not found" + ? `markitdown not installed. Install with: pip install markitdown` + : result.error || "conversion failed"; + content = [{ type: "text", text: `[Cannot read ${ext} file: ${errorMsg}]` }]; + } else { + content = [{ type: "text", text: `[Cannot read ${ext} file: conversion failed]` }]; + } + } else { + // Read as text + const file = Bun.file(absolutePath); + const textContent = await file.text(); + const allLines = textContent.split("\n"); + const totalFileLines = allLines.length; + + // Apply offset if specified (1-indexed to 0-indexed) + const startLine = offset ? Math.max(0, offset - 1) : 0; + const startLineDisplay = startLine + 1; // For display (1-indexed) + + // Check if offset is out of bounds - return graceful message instead of throwing + if (startLine >= allLines.length) { + const suggestion = + allLines.length === 0 + ? "The file is empty." + : `Use offset=1 to read from the start, or offset=${allLines.length} to read the last line.`; + return { + content: [ + { + type: "text", + text: `Offset ${offset} is beyond end of file (${allLines.length} lines total). ${suggestion}`, + }, + ], + }; } - return { content, details }; - }); + // If limit is specified by user, use it; otherwise we'll let truncateHead decide + let selectedContent: string; + let userLimitedLines: number | undefined; + if (limit !== undefined) { + const endLine = Math.min(startLine + limit, allLines.length); + selectedContent = allLines.slice(startLine, endLine).join("\n"); + userLimitedLines = endLine - startLine; + } else { + selectedContent = allLines.slice(startLine).join("\n"); + } + + // Apply truncation (respects both line and byte limits) + const truncation = truncateHead(selectedContent); + + // Add line numbers if requested (uses setting default if not specified) + const shouldAddLineNumbers = lines ?? this.defaultLineNumbers; + const prependLineNumbers = (text: string, startNum: number): string => { + const textLines = text.split("\n"); + const lastLineNum = startNum + textLines.length - 1; + const padWidth = String(lastLineNum).length; + return textLines + .map((line, i) => { + const lineNum = String(startNum + i).padStart(padWidth, " "); + return `${lineNum}\t${line}`; + }) + .join("\n"); + }; + + let outputText: string; + + if (truncation.firstLineExceedsLimit) { + const firstLine = allLines[startLine] ?? ""; + const firstLineBytes = Buffer.byteLength(firstLine, "utf-8"); + const snippet = truncateStringToBytesFromStart(firstLine, DEFAULT_MAX_BYTES); + const shownSize = formatSize(snippet.bytes); + + outputText = shouldAddLineNumbers ? prependLineNumbers(snippet.text, startLineDisplay) : snippet.text; + if (snippet.text.length > 0) { + outputText += `\n\n[Line ${startLineDisplay} is ${formatSize( + firstLineBytes, + )}, exceeds ${formatSize(DEFAULT_MAX_BYTES)} limit. Showing first ${shownSize} of the line.]`; + } else { + outputText = `[Line ${startLineDisplay} is ${formatSize( + firstLineBytes, + )}, exceeds ${formatSize(DEFAULT_MAX_BYTES)} limit. Unable to display a valid UTF-8 snippet.]`; + } + details = { truncation }; + } else if (truncation.truncated) { + // Truncation occurred - build actionable notice + const endLineDisplay = startLineDisplay + truncation.outputLines - 1; + const nextOffset = endLineDisplay + 1; + + outputText = shouldAddLineNumbers + ? prependLineNumbers(truncation.content, startLineDisplay) + : truncation.content; + + if (truncation.truncatedBy === "lines") { + outputText += `\n\n[Showing lines ${startLineDisplay}-${endLineDisplay} of ${totalFileLines}. Use offset=${nextOffset} to continue]`; + } else { + outputText += `\n\n[Showing lines ${startLineDisplay}-${endLineDisplay} of ${totalFileLines} (${formatSize( + DEFAULT_MAX_BYTES, + )} limit). Use offset=${nextOffset} to continue]`; + } + details = { truncation }; + } else if (userLimitedLines !== undefined && startLine + userLimitedLines < allLines.length) { + // User specified limit, there's more content, but no truncation + const remaining = allLines.length - (startLine + userLimitedLines); + const nextOffset = startLine + userLimitedLines + 1; + + outputText = shouldAddLineNumbers + ? prependLineNumbers(truncation.content, startLineDisplay) + : truncation.content; + outputText += `\n\n[${remaining} more lines in file. Use offset=${nextOffset} to continue]`; + } else { + // No truncation, no user limit exceeded + outputText = shouldAddLineNumbers + ? prependLineNumbers(truncation.content, startLineDisplay) + : truncation.content; + } + + content = [{ type: "text", text: outputText }]; + } + + return { content, details }; } } diff --git a/packages/coding-agent/src/core/tools/task/worker.ts b/packages/coding-agent/src/core/tools/task/worker.ts index cd663f662..808a5dca5 100644 --- a/packages/coding-agent/src/core/tools/task/worker.ts +++ b/packages/coding-agent/src/core/tools/task/worker.ts @@ -363,7 +363,7 @@ function createMCPProxyTool(metadata: MCPToolMetadata): CustomTool { } function getPythonCallTimeoutMs(params: PythonToolParams): number | undefined { - const timeout = params.timeoutMs; + const timeout = params.timeout_ms; if (typeof timeout === "number" && Number.isFinite(timeout) && timeout > 0) { return Math.max(1000, Math.round(timeout * 1000) + 1000); } diff --git a/packages/coding-agent/src/core/tools/todo-write.ts b/packages/coding-agent/src/core/tools/todo-write.ts index 1240d53c5..0883710a4 100644 --- a/packages/coding-agent/src/core/tools/todo-write.ts +++ b/packages/coding-agent/src/core/tools/todo-write.ts @@ -18,7 +18,7 @@ const todoWriteSchema = Type.Object({ Type.Object({ id: Type.Optional(Type.String({ description: "Stable todo id" })), content: Type.String({ description: "Imperative task description (e.g., 'Run tests')" }), - activeForm: Type.String({ description: "Present continuous form (e.g., 'Running tests')" }), + active_form: Type.String({ description: "Present continuous form (e.g., 'Running tests')" }), status: StringEnum(["pending", "in_progress", "completed"]), }), { description: "The updated todo list" }, @@ -30,7 +30,7 @@ type TodoStatus = "pending" | "in_progress" | "completed"; export interface TodoItem { id: string; content: string; - activeForm: string; + active_form: string; status: TodoStatus; } @@ -47,7 +47,7 @@ export interface TodoWriteToolDetails { const TODO_FILE_NAME = "todos.json"; -type TodoWriteParams = { todos: Array<{ id?: string; content?: string; activeForm?: string; status?: string }> }; +type TodoWriteParams = { todos: Array<{ id?: string; content?: string; active_form?: string; status?: string }> }; function normalizeTodoStatus(status?: string): TodoStatus { switch (status) { @@ -63,24 +63,24 @@ function normalizeTodoStatus(status?: string): TodoStatus { } function normalizeTodos( - items: Array<{ id?: string; content?: string; activeForm?: string; status?: string }>, + items: Array<{ id?: string; content?: string; active_form?: string; status?: string }>, ): TodoItem[] { return items.map((item) => { - if (!item.content || !item.activeForm) { - throw new Error("Todo content and activeForm are required."); + if (!item.content || !item.active_form) { + throw new Error("Todo content and active_form are required."); } const content = item.content.trim(); - const activeForm = item.activeForm.trim(); + const active_form = item.active_form.trim(); if (!content) { throw new Error("Todo content cannot be empty."); } - if (!activeForm) { - throw new Error("Todo activeForm cannot be empty."); + if (!active_form) { + throw new Error("Todo active_form cannot be empty."); } return { id: item.id && item.id.trim().length > 0 ? item.id : randomUUID(), content, - activeForm, + active_form, status: normalizeTodoStatus(item.status), }; }); @@ -144,7 +144,7 @@ function formatTodoSummary(todos: TodoItem[]): string { function formatTodoLine(item: TodoItem, uiTheme: Theme, prefix: string): string { const checkbox = uiTheme.checkbox; const displayText = - item.status === "in_progress" && item.activeForm !== item.content ? item.activeForm : item.content; + item.status === "in_progress" && item.active_form !== item.content ? item.active_form : item.content; switch (item.status) { case "completed": return uiTheme.fg("success", `${prefix}${checkbox.checked} ${chalk.strikethrough(item.content)}`); @@ -222,7 +222,7 @@ export class TodoWriteTool implements AgentTool; + todos?: Array<{ id?: string; content?: string; active_form?: string; status?: string }>; } export const todoWriteToolRenderer = { diff --git a/packages/coding-agent/src/modes/interactive/components/settings-defs.ts b/packages/coding-agent/src/modes/interactive/components/settings-defs.ts index 97ab8f50d..c8c32008c 100644 --- a/packages/coding-agent/src/modes/interactive/components/settings-defs.ts +++ b/packages/coding-agent/src/modes/interactive/components/settings-defs.ts @@ -307,7 +307,7 @@ export const SETTINGS_DEFS: SettingDef[] = [ tab: "tools", type: "boolean", label: "Edit patch mode", - description: "Use codex-style apply-patch format instead of oldText/newText for edits", + description: "Use codex-style apply-patch format instead of old_text/new_text for edits", get: (sm) => sm.getEditPatchMode(), set: (sm, v) => sm.setEditPatchMode(v), }, diff --git a/packages/coding-agent/src/modes/interactive/components/tool-execution.ts b/packages/coding-agent/src/modes/interactive/components/tool-execution.ts index 9e92c4437..31750f8f4 100644 --- a/packages/coding-agent/src/modes/interactive/components/tool-execution.ts +++ b/packages/coding-agent/src/modes/interactive/components/tool-execution.ts @@ -223,8 +223,8 @@ export class ToolExecutionComponent extends Container { return; } - const oldText = this.args?.oldText; - const newText = this.args?.newText; + const oldText = this.args?.old_text; + const newText = this.args?.new_text; const all = this.args?.all; // Need all three params to compute diff diff --git a/packages/coding-agent/src/prompts/tools/replace.md b/packages/coding-agent/src/prompts/tools/replace.md index 0dc5bdba1..36b9ce9aa 100644 --- a/packages/coding-agent/src/prompts/tools/replace.md +++ b/packages/coding-agent/src/prompts/tools/replace.md @@ -4,7 +4,7 @@ Performs string replacements in files with fuzzy whitespace matching. - Use the smallest edit that uniquely identifies the change -- If `oldText` is not unique, expand to include more context or use `all: true` to replace all occurrences +- If `old_text` is not unique, expand to include more context or use `all: true` to replace all occurrences - You must use your read tool at least once in the conversation before editing. This tool will error if you attempt an edit without reading the file. - Fuzzy matching handles minor whitespace/indentation differences automatically - Prefer editing existing files over creating new ones diff --git a/packages/coding-agent/src/prompts/tools/todo-write.md b/packages/coding-agent/src/prompts/tools/todo-write.md index 1b0f7a7ea..d98ddab16 100644 --- a/packages/coding-agent/src/prompts/tools/todo-write.md +++ b/packages/coding-agent/src/prompts/tools/todo-write.md @@ -148,7 +148,7 @@ The assistant did not use the todo list because this is a single command executi **IMPORTANT**: Task descriptions must have two forms: - content: The imperative form describing what needs to be done (e.g., "Run tests", "Build the project") - - activeForm: The present continuous form shown during execution (e.g., "Running tests", "Building the project") + - active_form: The present continuous form shown during execution (e.g., "Running tests", "Building the project") 2. **Task Management**: @@ -175,7 +175,7 @@ The assistant did not use the todo list because this is a single command executi - Use clear, descriptive task names - Always provide both forms: - content: "Fix authentication bug" - - activeForm: "Fixing authentication bug" + - active_form: "Fixing authentication bug" When in doubt, use this tool. Being proactive with task management demonstrates attentiveness and ensures you complete all requirements successfully. diff --git a/packages/coding-agent/test/tools.test.ts b/packages/coding-agent/test/tools.test.ts index 6244597b8..452f1cba5 100644 --- a/packages/coding-agent/test/tools.test.ts +++ b/packages/coding-agent/test/tools.test.ts @@ -44,7 +44,7 @@ describe("Coding Agent Tools", () => { let originalEditVariant: string | undefined; beforeEach(() => { - // Force replace mode for edit tool tests using oldText/newText + // Force replace mode for edit tool tests using old_text/new_text originalEditVariant = process.env.OMP_EDIT_VARIANT; process.env.OMP_EDIT_VARIANT = "replace"; @@ -260,8 +260,8 @@ describe("Coding Agent Tools", () => { const result = await editTool.execute("test-call-5", { path: testFile, - oldText: "world", - newText: "testing", + old_text: "world", + new_text: "testing", }); expect(getTextOutput(result)).toContain("Successfully replaced"); @@ -279,8 +279,8 @@ describe("Coding Agent Tools", () => { await expect( editTool.execute("test-call-6", { path: testFile, - oldText: "nonexistent", - newText: "testing", + old_text: "nonexistent", + new_text: "testing", }), ).rejects.toThrow(/Could not find/); }); @@ -293,8 +293,8 @@ describe("Coding Agent Tools", () => { await expect( editTool.execute("test-call-7", { path: testFile, - oldText: "foo", - newText: "bar", + old_text: "foo", + new_text: "bar", }), ).rejects.toThrow(/Found 3 occurrences/); }); @@ -305,8 +305,8 @@ describe("Coding Agent Tools", () => { const result = await editTool.execute("test-all-1", { path: testFile, - oldText: "foo", - newText: "qux", + old_text: "foo", + new_text: "qux", all: true, }); @@ -337,8 +337,8 @@ function b() { await expect( editTool.execute("test-all-fuzzy", { path: testFile, - oldText: "if (x) {\n doThing();\n}", - newText: "if (y) {\n doOther();\n}", + old_text: "if (x) {\n doThing();\n}", + new_text: "if (y) {\n doOther();\n}", all: true, }), ).rejects.toThrow(/Found 2 high-confidence matches/); @@ -351,8 +351,8 @@ function b() { await expect( editTool.execute("test-all-nomatch", { path: testFile, - oldText: "nonexistent", - newText: "bar", + old_text: "nonexistent", + new_text: "bar", all: true, }), ).rejects.toThrow(/Could not find/); @@ -364,8 +364,8 @@ function b() { const result = await editTool.execute("test-all-multiline", { path: testFile, - oldText: "foo\nbar", - newText: "replaced", + old_text: "foo\nbar", + new_text: "replaced", all: true, }); @@ -380,8 +380,8 @@ function b() { const result = await editTool.execute("test-all-single", { path: testFile, - oldText: "world", - newText: "universe", + old_text: "world", + new_text: "universe", all: true, }); @@ -531,7 +531,7 @@ describe("edit tool CRLF handling", () => { let originalEditVariant: string | undefined; beforeEach(() => { - // Force replace mode for edit tool tests using oldText/newText + // Force replace mode for edit tool tests using old_text/new_text originalEditVariant = process.env.OMP_EDIT_VARIANT; process.env.OMP_EDIT_VARIANT = "replace"; @@ -551,15 +551,15 @@ describe("edit tool CRLF handling", () => { } }); - it("should match LF oldText against CRLF file content", async () => { + it("should match LF old_text against CRLF file content", async () => { const testFile = join(testDir, "crlf-test.txt"); writeFileSync(testFile, "line one\r\nline two\r\nline three\r\n"); const result = await editTool.execute("test-crlf-1", { path: testFile, - oldText: "line two\n", - newText: "replaced line\n", + old_text: "line two\n", + new_text: "replaced line\n", }); expect(getTextOutput(result)).toContain("Successfully replaced"); @@ -571,8 +571,8 @@ describe("edit tool CRLF handling", () => { await editTool.execute("test-crlf-2", { path: testFile, - oldText: "second\n", - newText: "REPLACED\n", + old_text: "second\n", + new_text: "REPLACED\n", }); const content = readFileSync(testFile, "utf-8"); @@ -585,8 +585,8 @@ describe("edit tool CRLF handling", () => { await editTool.execute("test-lf-1", { path: testFile, - oldText: "second\n", - newText: "REPLACED\n", + old_text: "second\n", + new_text: "REPLACED\n", }); const content = readFileSync(testFile, "utf-8"); @@ -601,8 +601,8 @@ describe("edit tool CRLF handling", () => { await expect( editTool.execute("test-crlf-dup", { path: testFile, - oldText: "hello\nworld\n", - newText: "replaced\n", + old_text: "hello\nworld\n", + new_text: "replaced\n", }), ).rejects.toThrow(/Found 2 occurrences/); }); @@ -614,8 +614,8 @@ describe("edit tool CRLF handling", () => { await editTool.execute("test-bom", { path: testFile, - oldText: "second\n", - newText: "REPLACED\n", + old_text: "second\n", + new_text: "REPLACED\n", }); const content = readFileSync(testFile, "utf-8"); diff --git a/packages/coding-agent/test/tools/python-execution.test.ts b/packages/coding-agent/test/tools/python-execution.test.ts index e4589b4e1..93eaa1492 100644 --- a/packages/coding-agent/test/tools/python-execution.test.ts +++ b/packages/coding-agent/test/tools/python-execution.test.ts @@ -40,7 +40,7 @@ describe("python tool execution", () => { const tool = new PythonTool(createSession(tempDir.path)); const result = await tool.execute( "call-id", - { cells: [{ code: "print('hi')" }], timeoutMs: 5000, cwd: tempDir.path, reset: true }, + { cells: [{ code: "print('hi')" }], timeout_ms: 5000, cwd: tempDir.path, reset: true }, undefined, undefined, undefined, diff --git a/packages/coding-agent/test/tools/python.test.ts b/packages/coding-agent/test/tools/python.test.ts index 9e7ddf3aa..b0a7e63cf 100644 --- a/packages/coding-agent/test/tools/python.test.ts +++ b/packages/coding-agent/test/tools/python.test.ts @@ -57,7 +57,7 @@ describe("python tool schema", () => { expect(schema.type).toBe("object"); expect(schema.properties.cells.type).toBe("array"); - expect(schema.properties.timeoutMs.type).toBe("number"); + expect(schema.properties.timeout_ms.type).toBe("number"); expect(schema.properties.cwd.type).toBe("string"); expect(schema.properties.reset.type).toBe("boolean"); expect(schema.required).toEqual(["cells"]);