diff --git a/CHANGELOG.md b/CHANGELOG.md index 656ab4918..6daffcea1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,7 @@ - Auto-reply: restore 300-char heartbeat ack limit and keep >300 char replies instead of dropping them; adjust long heartbeat test content accordingly. - Gateway: `agents.list` now honors explicit `agents.list` config without pulling stray agents from disk; GitHub Copilot CLI auth path uses the updated provider build. - Google: apply patched pi-ai `google-gemini-cli` function call handling (strips ids) after upgrading to pi-ai 0.43.0. +- Tools: allow Gemini/Claude-style aliases (`file_path`, `old_string`, `new_string`) in read/write/edit schemas while still enforcing required params at runtime to avoid validation loops. - Auto-reply: elevated/reasoning toggles now enqueue system events so the model sees the mode change immediately. - Tools: keep `image` available in sandbox and fail over when image models return empty output (fixes “(no text returned)”). - Discord: add per-channel `autoThread` to auto-create threads for top-level messages. (#800) — thanks @davidguttman. diff --git a/src/agents/pi-tools.test.ts b/src/agents/pi-tools.test.ts index 76ff871e9..384c12c37 100644 --- a/src/agents/pi-tools.test.ts +++ b/src/agents/pi-tools.test.ts @@ -3,7 +3,8 @@ import os from "node:os"; import path from "node:path"; import sharp from "sharp"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; +import type { AgentTool } from "@mariozechner/pi-agent-core"; import type { ClawdbotConfig } from "../config/config.js"; import { __testing, createClawdbotCodingTools } from "./pi-tools.js"; import { createBrowserTool } from "./tools/browser-tool.js"; @@ -355,6 +356,71 @@ describe("createClawdbotCodingTools", () => { } }); + describe("Claude/Gemini alias support", () => { + it("adds Claude-style aliases to schemas without dropping metadata", () => { + const base: AgentTool = { + name: "write", + description: "test", + parameters: { + type: "object", + required: ["path", "content"], + properties: { + path: { type: "string", description: "Path" }, + content: { type: "string", description: "Body" }, + }, + }, + execute: vi.fn(), + }; + + const patched = __testing.patchToolSchemaForClaudeCompatibility(base); + const params = patched.parameters as { + properties?: Record; + required?: string[]; + }; + const props = params.properties ?? {}; + + expect(props.file_path).toEqual(props.path); + expect(params.required ?? []).not.toContain("path"); + expect(params.required ?? []).not.toContain("file_path"); + }); + + it("normalizes file_path to path and enforces required groups at runtime", async () => { + const execute = vi.fn(async (_id, args) => args); + const tool: AgentTool = { + name: "write", + description: "test", + parameters: { + type: "object", + required: ["path", "content"], + properties: { + path: { type: "string" }, + content: { type: "string" }, + }, + }, + execute, + }; + + const wrapped = __testing.wrapToolParamNormalization(tool, [ + { keys: ["path", "file_path"] }, + ]); + + await wrapped.execute("tool-1", { file_path: "foo.txt", content: "x" }); + expect(execute).toHaveBeenCalledWith( + "tool-1", + { path: "foo.txt", content: "x" }, + undefined, + undefined, + ); + + await expect( + wrapped.execute("tool-2", { content: "x" }), + ).rejects.toThrow(/Missing required parameter/); + await expect( + wrapped.execute("tool-3", { file_path: " ", content: "x" }), + ).rejects.toThrow(/Missing required parameter/); + }); + }); + it("filters tools by sandbox policy", () => { const sandbox = { enabled: true, diff --git a/src/agents/pi-tools.ts b/src/agents/pi-tools.ts index 77fbdc926..fcc260bab 100644 --- a/src/agents/pi-tools.ts +++ b/src/agents/pi-tools.ts @@ -429,6 +429,96 @@ function wrapSandboxPathGuard(tool: AnyAgentTool, root: string): AnyAgentTool { }; } +type RequiredParamGroup = { + keys: string[]; + allowEmpty?: boolean; + label?: string; +}; + +const CLAUDE_PARAM_GROUPS = { + read: [{ keys: ["path", "file_path"], label: "path (path or file_path)" }], + write: [{ keys: ["path", "file_path"], label: "path (path or file_path)" }], + edit: [ + { keys: ["path", "file_path"], label: "path (path or file_path)" }, + { keys: ["oldText", "old_string"], label: "oldText (oldText or old_string)" }, + { keys: ["newText", "new_string"], label: "newText (newText or new_string)" }, + ], +} as const; + +function patchToolSchemaForClaudeCompatibility( + tool: AnyAgentTool, +): AnyAgentTool { + const schema = + tool.parameters && typeof tool.parameters === "object" + ? (tool.parameters as Record) + : undefined; + + if (!schema || !schema.properties || typeof schema.properties !== "object") { + return tool; + } + + const properties = { ...(schema.properties as Record) }; + const required = Array.isArray(schema.required) + ? schema.required.filter((key): key is string => typeof key === "string") + : []; + let changed = false; + + const aliasPairs: Array<{ original: string; alias: string }> = [ + { original: "path", alias: "file_path" }, + { original: "oldText", alias: "old_string" }, + { original: "newText", alias: "new_string" }, + ]; + + for (const { original, alias } of aliasPairs) { + if (!(original in properties)) continue; + if (!(alias in properties)) { + properties[alias] = properties[original]; + changed = true; + } + const idx = required.indexOf(original); + if (idx !== -1) { + required.splice(idx, 1); + changed = true; + } + } + + if (!changed) return tool; + + return { + ...tool, + parameters: { + ...schema, + properties, + ...(required.length > 0 ? { required } : {}), + }, + }; +} + +function assertRequiredParams( + record: Record | undefined, + groups: readonly RequiredParamGroup[], + toolName: string, +): void { + if (!record || typeof record !== "object") { + throw new Error(`Missing parameters for ${toolName}`); + } + + for (const group of groups) { + const satisfied = group.keys.some((key) => { + if (!(key in record)) return false; + const value = record[key]; + if (typeof value !== "string") return false; + if (group.allowEmpty) return true; + return value.trim().length > 0; + }); + + if (!satisfied) { + const label = group.label ?? group.keys.join(" or "); + throw new Error(`Missing required parameter: ${label}`); + } + } +} + function createSandboxedReadTool(root: string) { const base = createReadTool(root); return wrapSandboxPathGuard(createClawdbotReadTool(base), root); @@ -436,12 +526,18 @@ function createSandboxedReadTool(root: string) { function createSandboxedWriteTool(root: string) { const base = createWriteTool(root); - return wrapSandboxPathGuard(wrapToolParamNormalization(base), root); + return wrapSandboxPathGuard( + wrapToolParamNormalization(base, CLAUDE_PARAM_GROUPS.write), + root, + ); } function createSandboxedEditTool(root: string) { const base = createEditTool(root); - return wrapSandboxPathGuard(wrapToolParamNormalization(base), root); + return wrapSandboxPathGuard( + wrapToolParamNormalization(base, CLAUDE_PARAM_GROUPS.edit), + root, + ); } // Normalize tool parameters from Claude Code conventions to pi-coding-agent conventions. // Claude Code uses file_path/old_string/new_string while pi-coding-agent uses path/oldText/newText. @@ -471,26 +567,44 @@ function normalizeToolParams( } // Generic wrapper to normalize parameters for any tool -function wrapToolParamNormalization(tool: AnyAgentTool): AnyAgentTool { +function wrapToolParamNormalization( + tool: AnyAgentTool, + requiredParamGroups?: readonly RequiredParamGroup[], +): AnyAgentTool { + const patched = patchToolSchemaForClaudeCompatibility(tool); return { - ...tool, + ...patched, execute: async (toolCallId, params, signal, onUpdate) => { const normalized = normalizeToolParams(params); + const record = + normalized ?? + (params && typeof params === "object" + ? (params as Record) + : undefined); + if (requiredParamGroups?.length) { + assertRequiredParams(record, requiredParamGroups, tool.name); + } return tool.execute(toolCallId, normalized ?? params, signal, onUpdate); }, }; } function createClawdbotReadTool(base: AnyAgentTool): AnyAgentTool { + const patched = patchToolSchemaForClaudeCompatibility(base); return { - ...base, + ...patched, execute: async (toolCallId, params, signal) => { const normalized = normalizeToolParams(params); + const record = + normalized ?? + (params && typeof params === "object" + ? (params as Record) + : undefined); + assertRequiredParams(record, CLAUDE_PARAM_GROUPS.read, base.name); const result = (await base.execute( toolCallId, normalized ?? params, signal, )) as AgentToolResult; - const record = normalized ?? (params as Record); const filePath = typeof record?.path === "string" ? String(record.path) : ""; const normalizedResult = await normalizeReadImageResult(result, filePath); @@ -501,6 +615,10 @@ function createClawdbotReadTool(base: AnyAgentTool): AnyAgentTool { export const __testing = { cleanToolSchemaForGemini, + normalizeToolParams, + patchToolSchemaForClaudeCompatibility, + wrapToolParamNormalization, + assertRequiredParams, } as const; function throwAbortError(): never { @@ -618,12 +736,22 @@ export function createClawdbotCodingTools(options?: { if (tool.name === "write") { if (sandboxRoot) return []; // Wrap with param normalization for Claude Code compatibility - return [wrapToolParamNormalization(createWriteTool(workspaceRoot))]; + return [ + wrapToolParamNormalization( + createWriteTool(workspaceRoot), + CLAUDE_PARAM_GROUPS.write, + ), + ]; } if (tool.name === "edit") { if (sandboxRoot) return []; // Wrap with param normalization for Claude Code compatibility - return [wrapToolParamNormalization(createEditTool(workspaceRoot))]; + return [ + wrapToolParamNormalization( + createEditTool(workspaceRoot), + CLAUDE_PARAM_GROUPS.edit, + ), + ]; } return [tool as AnyAgentTool]; }); diff --git a/src/config/zod-schema.ts b/src/config/zod-schema.ts index e92329441..b6a912e54 100644 --- a/src/config/zod-schema.ts +++ b/src/config/zod-schema.ts @@ -241,6 +241,13 @@ const ExecutableTokenSchema = z .string() .refine(isSafeExecutableValue, "expected safe executable name or path"); +const ToolsAudioTranscriptionSchema = z + .object({ + args: z.array(z.string()).optional(), + timeoutSeconds: z.number().int().positive().optional(), + }) + .optional(); + const NativeCommandsSettingSchema = z.union([z.boolean(), z.literal("auto")]); const ProviderCommandsSchema = z @@ -249,25 +256,6 @@ const ProviderCommandsSchema = z }) .optional(); -const CommandsSchema = z - .object({ - native: NativeCommandsSettingSchema.optional().default("auto"), - text: z.boolean().optional(), - config: z.boolean().optional(), - debug: z.boolean().optional(), - restart: z.boolean().optional(), - useAccessGroups: z.boolean().optional(), - }) - .optional() - .default({ native: "auto" }); - -const ToolsAudioTranscriptionSchema = z - .object({ - args: z.array(z.string()).optional(), - timeoutSeconds: z.number().int().positive().optional(), - }) - .optional(); - const TelegramTopicSchema = z.object({ requireMention: z.boolean().optional(), skills: z.array(z.string()).optional(), @@ -732,6 +720,18 @@ const MessagesSchema = z }) .optional(); +const CommandsSchema = z + .object({ + native: NativeCommandsSettingSchema.optional().default("auto"), + text: z.boolean().optional(), + config: z.boolean().optional(), + debug: z.boolean().optional(), + restart: z.boolean().optional(), + useAccessGroups: z.boolean().optional(), + }) + .optional() + .default({ native: "auto" }); + const HeartbeatSchema = z .object({ every: z.string().optional(), @@ -801,7 +801,6 @@ const SandboxDockerSchema = z apparmorProfile: z.string().optional(), dns: z.array(z.string()).optional(), extraHosts: z.array(z.string()).optional(), - binds: z.array(z.string()).optional(), }) .optional();