From 192484ed0ae94b92e54cd78dc5b4df04e3ebef56 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 31 Mar 2026 15:49:40 +0100 Subject: [PATCH] fix: log malformed tool parameters on failure --- ...pi-tool-definition-adapter.logging.test.ts | 67 +++++++++++++++++++ src/agents/pi-tool-definition-adapter.ts | 61 ++++++++++++++++- 2 files changed, 127 insertions(+), 1 deletion(-) create mode 100644 src/agents/pi-tool-definition-adapter.logging.test.ts diff --git a/src/agents/pi-tool-definition-adapter.logging.test.ts b/src/agents/pi-tool-definition-adapter.logging.test.ts new file mode 100644 index 00000000000..cc442abc3b9 --- /dev/null +++ b/src/agents/pi-tool-definition-adapter.logging.test.ts @@ -0,0 +1,67 @@ +import type { AgentTool } from "@mariozechner/pi-agent-core"; +import { Type } from "@sinclair/typebox"; +import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + logDebug: vi.fn(), + logError: vi.fn(), +})); + +vi.mock("../logger.js", () => ({ + logDebug: mocks.logDebug, + logError: mocks.logError, +})); + +let toToolDefinitions: typeof import("./pi-tool-definition-adapter.js").toToolDefinitions; +let wrapToolParamNormalization: typeof import("./pi-tools.params.js").wrapToolParamNormalization; +let CLAUDE_PARAM_GROUPS: typeof import("./pi-tools.params.js").CLAUDE_PARAM_GROUPS; +let logError: typeof import("../logger.js").logError; + +type ToolExecute = ReturnType< + typeof import("./pi-tool-definition-adapter.js").toToolDefinitions +>[number]["execute"]; +const extensionContext = {} as Parameters[4]; + +describe("pi tool definition adapter logging", () => { + beforeAll(async () => { + ({ toToolDefinitions } = await import("./pi-tool-definition-adapter.js")); + ({ wrapToolParamNormalization, CLAUDE_PARAM_GROUPS } = await import("./pi-tools.params.js")); + ({ logError } = await import("../logger.js")); + }); + + beforeEach(() => { + vi.mocked(logError).mockReset(); + mocks.logDebug.mockReset(); + }); + + it("logs raw malformed edit params when required aliases are missing", async () => { + const baseTool = { + name: "edit", + label: "Edit", + description: "edits files", + parameters: Type.Object({ + path: Type.String(), + oldText: Type.String(), + newText: Type.String(), + }), + execute: async () => ({ + content: [{ type: "text" as const, text: "ok" }], + details: { ok: true }, + }), + } satisfies AgentTool; + + const tool = wrapToolParamNormalization(baseTool, CLAUDE_PARAM_GROUPS.edit); + const [def] = toToolDefinitions([tool]); + if (!def) { + throw new Error("missing tool definition"); + } + + await def.execute("call-edit-1", { path: "notes.txt" }, undefined, undefined, extensionContext); + + expect(logError).toHaveBeenCalledWith( + expect.stringContaining( + '[tools] edit failed: Missing required parameters: oldText alias, newText alias. Supply correct parameters before retrying. raw_params={"path":"notes.txt"}', + ), + ); + }); +}); diff --git a/src/agents/pi-tool-definition-adapter.ts b/src/agents/pi-tool-definition-adapter.ts index da345a74ea8..b6ae8f7cd2c 100644 --- a/src/agents/pi-tool-definition-adapter.ts +++ b/src/agents/pi-tool-definition-adapter.ts @@ -5,7 +5,9 @@ import type { } from "@mariozechner/pi-agent-core"; import type { ToolDefinition } from "@mariozechner/pi-coding-agent"; import { logDebug, logError } from "../logger.js"; +import { redactToolDetail } from "../logging/redact.js"; import { isPlainObject } from "../utils.js"; +import { sanitizeForConsole } from "./console-sanitize.js"; import type { ClientToolDefinition } from "./pi-embedded-runner/run/params.js"; import type { HookContext } from "./pi-tools.before-tool-call.js"; import { @@ -35,6 +37,7 @@ type ToolExecuteArgs = ToolDefinition["execute"] extends (...args: infer P) => u ? P : ToolExecuteArgsCurrent; type ToolExecuteArgsAny = ToolExecuteArgs | ToolExecuteArgsLegacy | ToolExecuteArgsCurrent; +const TOOL_ERROR_PARAM_PREVIEW_MAX_CHARS = 600; function isAbortSignal(value: unknown): value is AbortSignal { return typeof value === "object" && value !== null && "aborted" in value; @@ -60,6 +63,58 @@ function describeToolExecutionError(err: unknown): { return { message: String(err) }; } +function serializeToolParams(value: unknown): string { + if (value === undefined) { + return ""; + } + if (typeof value === "string") { + return value; + } + if ( + value === null || + typeof value === "number" || + typeof value === "boolean" || + typeof value === "bigint" + ) { + return String(value); + } + try { + const serialized = JSON.stringify(value); + if (typeof serialized === "string") { + return serialized; + } + } catch { + // Fall through to String(value). + } + if (typeof value === "function") { + return value.name ? `[Function ${value.name}]` : "[Function anonymous]"; + } + if (typeof value === "symbol") { + return value.description ? `Symbol(${value.description})` : "Symbol()"; + } + return Object.prototype.toString.call(value); +} + +function formatToolParamPreview(label: string, value: unknown): string { + const serialized = serializeToolParams(value); + const redacted = redactToolDetail(serialized); + const preview = sanitizeForConsole(redacted, TOOL_ERROR_PARAM_PREVIEW_MAX_CHARS) ?? ""; + return `${label}=${preview}`; +} + +function describeToolFailureInputs(params: { + rawParams: unknown; + effectiveParams: unknown; +}): string { + const parts = [formatToolParamPreview("raw_params", params.rawParams)]; + const rawSerialized = serializeToolParams(params.rawParams); + const effectiveSerialized = serializeToolParams(params.effectiveParams); + if (effectiveSerialized !== rawSerialized) { + parts.push(formatToolParamPreview("effective_params", params.effectiveParams)); + } + return parts.join(" "); +} + function normalizeToolExecutionResult(params: { toolName: string; result: unknown; @@ -160,7 +215,11 @@ export function toToolDefinitions(tools: AnyAgentTool[]): ToolDefinition[] { if (described.stack && described.stack !== described.message) { logDebug(`tools: ${normalizedName} failed stack:\n${described.stack}`); } - logError(`[tools] ${normalizedName} failed: ${described.message}`); + const inputPreview = describeToolFailureInputs({ + rawParams: params, + effectiveParams: executeParams, + }); + logError(`[tools] ${normalizedName} failed: ${described.message} ${inputPreview}`); return buildToolExecutionErrorResult({ toolName: normalizedName,