fix: log malformed tool parameters on failure

This commit is contained in:
Peter Steinberger 2026-03-31 15:49:40 +01:00
parent 7dffd8160a
commit 192484ed0a
No known key found for this signature in database
2 changed files with 127 additions and 1 deletions

View File

@ -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<ToolExecute>[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"}',
),
);
});
});

View File

@ -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 "<undefined>";
}
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) ?? "<empty>";
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,