From d2d159d8aebe0af24dac060f905be05816db59d8 Mon Sep 17 00:00:00 2001 From: amosblomqvist Date: Sun, 4 Oct 2026 20:21:10 +0300 Subject: [PATCH] isError results replace thrown ToolInputError (pi 1.0 convention) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Authoring refinements (quizInputProblems, nodeId checks, ask/explain refinements, submit-agent pipeline laws) now return invalidInputResult — same model-facing failure and retry semantics pi minted from the throw, but details keeps {tool, problems} as data instead of the throw path's empty {}. ToolInputError deleted; wire shape unchanged (verified against pi's own thrown-error conversion in agent-loop). --- src/agent/subagents/practice.ts | 2 +- src/agent/subagents/submit-agent.ts | 11 +++++--- src/agent/tool-execute.ts | 39 ++++++++++++++++++----------- src/agent/tools.ts | 12 ++++----- tests/question-tools.test.ts | 28 ++++++++++----------- tests/tool-abort.test.ts | 30 +++++++++------------- 6 files changed, 64 insertions(+), 58 deletions(-) diff --git a/src/agent/subagents/practice.ts b/src/agent/subagents/practice.ts index 84de49d..ae6a355 100644 --- a/src/agent/subagents/practice.ts +++ b/src/agent/subagents/practice.ts @@ -4,7 +4,7 @@ * Three single-submit workers on the shared runSubmitAgent seam (one * TypeBox-validated submit call each; a pi agent purely so the model can * retry a schema-invalid or LAW-violating call — the validate hook throws - * ToolInputError at the execute seam and pi turns it into an error tool + * an invalidInputResult isError result at the execute seam (pi 1.0 convention) * result the model sees). * * Roles: partitioner | problemGenerator | grader — all strong-tier diff --git a/src/agent/subagents/submit-agent.ts b/src/agent/subagents/submit-agent.ts index eb2486d..5448300 100644 --- a/src/agent/subagents/submit-agent.ts +++ b/src/agent/subagents/submit-agent.ts @@ -10,7 +10,7 @@ import type { Usage } from "@earendil-works/pi-ai"; import type { AgentTool } from "@earendil-works/pi-agent-core"; import type { TSchema } from "typebox"; -import { toToolResult, ToolInputError } from "../tool-execute.ts"; +import { invalidInputResult, toToolResult } from "../tool-execute.ts"; import type { ModelRole } from "../../../config.ts"; import type { PiEngine } from "../models.ts"; import { runSubagent } from "./pi-run.ts"; @@ -63,14 +63,16 @@ export async function runSubmitAgent(opts: { /** Turn budget for the loop (default 4 = retry-only; the auditor runs * an exploration loop instead — 16). */ maxTurns?: number; + /** Debug: raw provider-event recording key (LH_PROVIDER_EVENTS=1). */ + chatId?: string; /** Weak-model argument heal at pi's prepareArguments seam (runs BEFORE * TypeBox validation — the sanctioned shim point). Identity by * default; extraction wires healStringifiedArrays here. */ prepareArguments?: (args: unknown) => unknown; /** Semantic validation at the submit seam: the value is schema-valid * but may violate a PIPELINE law (coverage, echo-equality). Returning - * a problem string throws ToolInputError → pi turns it into an error - * tool result → the model retries within maxTurns. null = accept. */ + * a problem string yields an isError result (invalidInputResult) → + * the model retries within maxTurns. null = accept. */ validate?: (value: T) => string | null; }): Promise { let submitted: T | undefined; @@ -87,7 +89,7 @@ export async function runSubmitAgent(opts: { execute: async (_toolCallId, input) => { const value = opts.fill(input); const problem = opts.validate?.(value) ?? null; - if (problem) throw new ToolInputError(opts.toolName, [problem]); + if (problem) return invalidInputResult(opts.toolName, [problem]); submitted = value; return toToolResult(submitted); }, @@ -100,6 +102,7 @@ export async function runSubmitAgent(opts: { prompt: opts.prompt, maxTurns: opts.maxTurns ?? 4, abortSignal: opts.signal, + chatId: opts.chatId, onAssistantMessage: (message, step) => opts.onStep( message.usage, diff --git a/src/agent/tool-execute.ts b/src/agent/tool-execute.ts index bd693e8..bb4f260 100644 --- a/src/agent/tool-execute.ts +++ b/src/agent/tool-execute.ts @@ -9,10 +9,10 @@ import type { Static, TSchema } from "typebox"; * (lh-tool.ts, deleted) used to add around every execute: * * - the system-level per-call timeout + abort cascade (runWithAbortGuard), - * - the authoring-refinement failure shape (ToolInputError — thrown from - * execute; pi's loop converts the throw into an error tool result the - * model sees and retries on, exactly the old InvalidToolInputError - * semantics), + * - the authoring-refinement failure shape (invalidInputResult — + * returned from execute as an isError result; pi 1.0's convention, the + * model sees the same retryable problem list it always did, and + * `details` keeps the problems as data), * - the result mapping (string verbatim / object JSON / image results to * native content parts; details carries the record-facing value), * - the question-tool placeholder (quiz / ask_user_question): a synthetic @@ -107,17 +107,26 @@ function runWithAbortGuard( .finally(cleanup); } -/** lh's port of the SDK's invalid-input message: what the model retried on. - * Thrown from a tool's execute when the authoring refinements fail — - * pi's loop catches the throw and turns it into an error tool result. */ -export class ToolInputError extends Error { - constructor( - public readonly toolName: string, - public readonly problems: string[], - ) { - super(`Invalid tool input for tool "${toolName}":\n${problems.map((p) => `- ${p}`).join("\n")}`); - this.name = "ToolInputError"; - } +/** The 1.0-native authoring-failure result (pi convention, agent-core + * 1.0): return an isError result instead of throwing — the model sees the + * same retryable problem list it always did (pi minted exactly this shape + * from a thrown ToolInputError), but `details` now keeps the problems as + * DATA for the record and the UI instead of the throw path's empty {}. + * Content text is byte-identical to the old thrown message. */ +export function invalidInputResult( + toolName: string, + problems: string[], +): AgentToolResult<{ tool: string; problems: string[] }> { + return { + content: [ + { + type: "text", + text: `Invalid tool input for tool "${toolName}":\n${problems.map((p) => `- ${p}`).join("\n")}`, + }, + ], + isError: true, + details: { tool: toolName, problems }, + }; } /** diff --git a/src/agent/tools.ts b/src/agent/tools.ts index 36a6910..428817a 100644 --- a/src/agent/tools.ts +++ b/src/agent/tools.ts @@ -1,6 +1,6 @@ import { Type } from "typebox"; import type { AgentTool } from "@earendil-works/pi-agent-core"; -import { questionResult, serverExecute, ToolInputError } from "./tool-execute.ts"; +import { invalidInputResult, questionResult, serverExecute } from "./tool-execute.ts"; import { askInputSchema, askOptionProblems, @@ -88,7 +88,7 @@ export const askUserQuestionTool: AgentTool = { executionMode: "sequential", execute: async (_toolCallId, input) => { const problems = askOptionProblems(input.options ?? []); - if (problems.length > 0) throw new ToolInputError("ask_user_question", problems); + if (problems.length > 0) return invalidInputResult("ask_user_question", problems); return questionResult(); }, }; @@ -153,7 +153,7 @@ export const probeQuizTool: AgentTool = { executionMode: "sequential", execute: async (_toolCallId, input) => { const problems = quizInputProblems(input); - if (problems.length > 0) throw new ToolInputError("quiz", problems); + if (problems.length > 0) return invalidInputResult("quiz", problems); return questionResult(); }, }; @@ -174,7 +174,7 @@ export function teachExplainToolFor( const problems = [...explainInputProblems(input)]; const nodeIdProblemText = nodeIdProblem(ctx, input.nodeId); if (nodeIdProblemText) problems.push(nodeIdProblemText); - if (problems.length > 0) throw new ToolInputError("explain", problems); + if (problems.length > 0) return invalidInputResult("explain", problems); return questionResult(); }, }; @@ -191,7 +191,7 @@ export const probeExplainTool: AgentTool = { executionMode: "sequential", execute: async (_toolCallId, input) => { const problems = explainInputProblems(input); - if (problems.length > 0) throw new ToolInputError("explain", problems); + if (problems.length > 0) return invalidInputResult("explain", problems); return questionResult(); }, }; @@ -317,7 +317,7 @@ export function teachQuizToolFor(ctx: Pick 0) throw new ToolInputError("quiz", problems); + if (problems.length > 0) return invalidInputResult("quiz", problems); return questionResult(); }, }; diff --git a/tests/question-tools.test.ts b/tests/question-tools.test.ts index 3e6bc53..bd8c5b1 100644 --- a/tests/question-tools.test.ts +++ b/tests/question-tools.test.ts @@ -64,29 +64,29 @@ const validQuiz = { }; import type { AgentTool } from "@earendil-works/pi-agent-core"; -import { ToolInputError } from "../src/agent/tool-execute.ts"; +import type { AgentToolResult } from "@earendil-works/pi-agent-core"; /** The Wave A boundary predicate (2026-09-26): pi's loop structurally * validates against the TypeBox parameters, then execute re-runs the - * authoring refinements (the old superRefine) and throws ToolInputError - * on problems — the model sees both as retryable input errors. This - * helper replicates the old safeParse seam end to end. */ + * authoring refinements (the old superRefine) and returns an isError + * result on problems (pi 1.0 convention — invalidInputResult; the model + * sees both as retryable input errors). This helper replicates the old + * safeParse seam end to end. */ async function boundaryParse( // `input: never` param: contravariance lets any AgentTool's execute slot in. tool: { parameters: any; execute: (id: string, input: never) => Promise }, input: unknown, -): Promise<{ success: boolean; error?: ToolInputError }> { +): Promise<{ success: boolean; error?: AgentToolResult<{ tool: string; problems: string[] }> }> { if (!Value.Check(tool.parameters, input)) return { success: false }; - try { - await tool.execute("tc", input as never); - return { success: true }; - } catch (e) { - assert.ok(e instanceof ToolInputError, `expected ToolInputError, got: ${e}`); - return { success: false, error: e }; - } + const result = (await tool.execute("tc", input as never)) as AgentToolResult<{ + tool: string; + problems: string[]; + }>; + if (result.isError) return { success: false, error: result }; + return { success: true }; } -const problemLines = (e?: ToolInputError): string[] => - (e?.message.split("\n") ?? []).filter((l) => l.startsWith("- ")); +const problemLines = (r?: AgentToolResult<{ tool: string; problems: string[] }>): string[] => + r?.details.problems ?? []; describe("teachQuizToolFor: the dynamic plan-nodeId validation (position signal)", () => { it("accepts a nodeId that matches a node of the session's tree", async () => { diff --git a/tests/tool-abort.test.ts b/tests/tool-abort.test.ts index 8f390d8..ff3c353 100644 --- a/tests/tool-abort.test.ts +++ b/tests/tool-abort.test.ts @@ -15,7 +15,6 @@ import { questionResult, serverExecute, toolTimeoutMs, - ToolInputError, toToolResult, type ToolExecuteContext, } from "../src/agent/tool-execute.ts"; @@ -121,23 +120,18 @@ describe("serverExecute: cooperative settle, mapping parity, question bypass", ( assert.equal(result.terminate, true, "owner Option A: question results end the batch natively"); }); - it("authoring failures throw ToolInputError with the retryable problem list", async () => { - await assert.rejects( - askUserQuestionTool.execute("c7", { - question: "q", - options: [ - { label: "A", value: "same" }, - { label: "B", value: "same" }, - ], - }), - (e: unknown) => { - assert.ok(e instanceof ToolInputError); - assert.equal(e.toolName, "ask_user_question"); - assert.match(e.message, /Invalid tool input for tool "ask_user_question":/); - assert.match(e.message, /- .*same value/s); - return true; - }, - ); + it("authoring failures return an isError result with the retryable problem list", async () => { + const result = (await askUserQuestionTool.execute("c7", { + question: "q", + options: [ + { label: "A", value: "same" }, + { label: "B", value: "same" }, + ], + })) as { isError?: boolean; content: Array<{ text?: string }>; details?: { tool: string; problems: string[] } }; + assert.equal(result.isError, true, "pi 1.0 convention: returned, not thrown"); + assert.equal(result.details?.tool, "ask_user_question"); + assert.match(result.content[0]!.text!, /Invalid tool input for tool "ask_user_question":/); + assert.match(result.details!.problems.map((p) => `- ${p}`).join("\n"), /- .*same value/s); }); it("toToolResult parity: string verbatim, object as JSON, details rides the raw value", () => {