refactor(question): tool-arg errors at the boundary, drop redundant inner decode (#28570)
This commit is contained in:
@@ -416,46 +416,6 @@ it.live("pending question rejects on instance dispose", () =>
|
||||
}),
|
||||
)
|
||||
|
||||
// Regression for #28438: when an invalid payload reaches `Question.ask`
|
||||
// (one that's missing a required field like `question`), the previous
|
||||
// `Schema.decodeUnknownSync` would throw uncaught and crash the whole
|
||||
// assistant turn. The fix routes the failure through `Effect.orDie` with a
|
||||
// "rewrite the input" Error so the surrounding tool wrap can hand it back to
|
||||
// the model as a tool-call error rather than killing the session.
|
||||
it.instance(
|
||||
"ask - invalid payload surfaces as a friendly defect, not a thrown SchemaError",
|
||||
() =>
|
||||
Effect.gen(function* () {
|
||||
const exit = yield* askEffect({
|
||||
sessionID: SessionID.make("ses_invalid"),
|
||||
// Cast: bypassing the public type to simulate an upstream caller
|
||||
// (or a future schema divergence) that lets a missing required
|
||||
// field reach the decode boundary.
|
||||
questions: [
|
||||
{
|
||||
header: "Pick mode",
|
||||
options: [
|
||||
{ label: "A", description: "x" },
|
||||
{ label: "B", description: "y" },
|
||||
],
|
||||
} as unknown as Question.Info,
|
||||
],
|
||||
}).pipe(Effect.exit)
|
||||
|
||||
expect(Exit.isFailure(exit)).toBe(true)
|
||||
if (Exit.isFailure(exit)) {
|
||||
const message = exit.cause.toString()
|
||||
// Friendly preamble the AI SDK feeds back to the model so it can retry.
|
||||
expect(message).toContain("invalid arguments")
|
||||
expect(message).toContain("Please rewrite the input")
|
||||
// The exact JSON path pinpointing the missing field, so the model
|
||||
// knows which question and which field to fix.
|
||||
expect(message).toContain(`["questions"][0]["question"]`)
|
||||
}
|
||||
}),
|
||||
{ git: true },
|
||||
)
|
||||
|
||||
it.live("pending question rejects on instance reload", () =>
|
||||
Effect.gen(function* () {
|
||||
const dir = yield* tmpdirScoped({ git: true })
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, expect } from "bun:test"
|
||||
import { Effect, Layer, Schema } from "effect"
|
||||
import { Cause, Effect, Exit, Layer, Schema } from "effect"
|
||||
import { Agent } from "../../src/agent/agent"
|
||||
import { MessageID, SessionID } from "../../src/session/schema"
|
||||
import { Tool } from "@/tool/tool"
|
||||
@@ -10,6 +10,22 @@ const it = testEffect(Layer.mergeAll(Truncate.defaultLayer, Agent.defaultLayer))
|
||||
|
||||
const params = Schema.Struct({ input: Schema.String })
|
||||
|
||||
function makeCtx(): Tool.Context {
|
||||
return {
|
||||
sessionID: SessionID.descending(),
|
||||
messageID: MessageID.ascending(),
|
||||
agent: "build",
|
||||
abort: new AbortController().signal,
|
||||
messages: [],
|
||||
metadata() {
|
||||
return Effect.void
|
||||
},
|
||||
ask() {
|
||||
return Effect.void
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
function makeTool(id: string, executeFn?: () => void) {
|
||||
return {
|
||||
description: "test tool",
|
||||
@@ -79,19 +95,7 @@ describe("Tool.define", () => {
|
||||
},
|
||||
}),
|
||||
)
|
||||
const ctx: Tool.Context = {
|
||||
sessionID: SessionID.descending(),
|
||||
messageID: MessageID.ascending(),
|
||||
agent: "build",
|
||||
abort: new AbortController().signal,
|
||||
messages: [],
|
||||
metadata() {
|
||||
return Effect.void
|
||||
},
|
||||
ask() {
|
||||
return Effect.void
|
||||
},
|
||||
}
|
||||
const ctx = makeCtx()
|
||||
const tool = yield* info.init()
|
||||
const execute = tool.execute as unknown as (args: unknown, ctx: Tool.Context) => ReturnType<typeof tool.execute>
|
||||
|
||||
@@ -101,4 +105,49 @@ describe("Tool.define", () => {
|
||||
expect(calls).toEqual([{ count: 5 }, { count: 7 }])
|
||||
}),
|
||||
)
|
||||
|
||||
// Regression for #28438: the wrap is the canonical "untyped → typed" boundary.
|
||||
// When the LLM emits a tool call with a payload that fails the parameter
|
||||
// schema, the wrap must surface a typed `Tool.InvalidArgumentsError` whose
|
||||
// `.message` is the actionable prose the AI SDK feeds back to the model.
|
||||
it.effect("invalid args surface as Tool.InvalidArgumentsError with friendly message and JSON path", () =>
|
||||
Effect.gen(function* () {
|
||||
const parameters = Schema.Struct({
|
||||
questions: Schema.Array(
|
||||
Schema.Struct({
|
||||
question: Schema.String,
|
||||
options: Schema.Array(Schema.String),
|
||||
}),
|
||||
),
|
||||
})
|
||||
const info = yield* Tool.define(
|
||||
"qtest",
|
||||
Effect.succeed({
|
||||
description: "test tool",
|
||||
parameters,
|
||||
execute() {
|
||||
return Effect.succeed({ title: "ok", output: "ok", metadata: { truncated: false } })
|
||||
},
|
||||
}),
|
||||
)
|
||||
const tool = yield* info.init()
|
||||
const execute = tool.execute as unknown as (args: unknown, ctx: Tool.Context) => ReturnType<typeof tool.execute>
|
||||
|
||||
// Missing required `question` field on the first questions[] entry.
|
||||
const exit = yield* execute({ questions: [{ options: ["a"] }] }, makeCtx()).pipe(Effect.exit)
|
||||
expect(Exit.isFailure(exit)).toBe(true)
|
||||
if (!Exit.isFailure(exit)) return
|
||||
|
||||
// The wrap ends with Effect.orDie, so the failure lives in the cause as a
|
||||
// defect. Recover the typed instance from there.
|
||||
const die = exit.cause.reasons.find(Cause.isDieReason)
|
||||
const error = die?.defect
|
||||
expect(error).toBeInstanceOf(Tool.InvalidArgumentsError)
|
||||
const args = error as Tool.InvalidArgumentsError
|
||||
expect(args.tool).toBe("qtest")
|
||||
expect(args.message).toContain("qtest tool was called with invalid arguments")
|
||||
expect(args.message).toContain("Please rewrite the input")
|
||||
expect(args.message).toContain(`["questions"][0]["question"]`)
|
||||
}),
|
||||
)
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user