mirror of
https://github.com/Kilo-Org/kilocode.git
synced 2026-09-01 15:32:11 +08:00
fd60036e4a
* fix: make tool invalid-arguments errors clearly actionable to the model * fix: address kilo-code-bot review on schema error formatting Return a jargon-free actionable message when the schema formatter yields no issues (instead of the raw SchemaError), and match Effect's missing-key message case-insensitively so it survives library wording changes. Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> * Address review: cap rendered issues, isolate Kilo logic, harden the fallbacks - Move formatter/format/path/reason out of the shared upstream tool.ts into packages/opencode/src/kilocode/tool/tool.ts. - Cap output at 20 issues / 4 KiB with an '...and N more' suffix, since validation fails before the normal output truncation. - Return a readable message when the formatter yields zero issues instead of falling back to SchemaError jargon. - Match missing-key messages with a regex instead of an exact string compare. - Rewrite the changeset around the user-visible outcome. * chore: annotate the reformatted decode call with kilocode_change markers The multi-line reformat left lines 122-125 unannotated in a shared upstream file. Wrap the whole statement in a start/end block instead of per-line inline markers, and drop the two now-redundant inline ones. Comments only. --------- Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Co-authored-by: Christiaan Arnoldus <christiaan.arnoldus@outlook.com>
216 lines
7.2 KiB
TypeScript
216 lines
7.2 KiB
TypeScript
import { describe, expect } from "bun:test"
|
|
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"
|
|
import { Truncate } from "@/tool/truncate"
|
|
import { testEffect } from "../lib/effect"
|
|
|
|
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",
|
|
parameters: params,
|
|
execute() {
|
|
executeFn?.()
|
|
return Effect.succeed({ title: "test", output: "ok", metadata: {} })
|
|
},
|
|
}
|
|
}
|
|
|
|
// kilocode_change start
|
|
function invalid(exit: Exit.Exit<unknown, unknown>) {
|
|
expect(Exit.isFailure(exit)).toBe(true)
|
|
if (!Exit.isFailure(exit)) {
|
|
throw new Error("expected tool execution to fail")
|
|
}
|
|
const die = exit.cause.reasons.find(Cause.isDieReason)
|
|
const error = die?.defect
|
|
expect(error).toBeInstanceOf(Tool.InvalidArgumentsError)
|
|
return error as Tool.InvalidArgumentsError
|
|
}
|
|
// kilocode_change end
|
|
|
|
describe("Tool.define", () => {
|
|
it.effect("object-defined tool does not mutate the original init object", () =>
|
|
Effect.gen(function* () {
|
|
const original = makeTool("test")
|
|
const originalExecute = original.execute
|
|
|
|
const info = yield* Tool.define("test-tool", Effect.succeed(original))
|
|
|
|
yield* info.init()
|
|
yield* info.init()
|
|
yield* info.init()
|
|
|
|
expect(original.execute).toBe(originalExecute)
|
|
}),
|
|
)
|
|
|
|
it.effect("effect-defined tool returns fresh objects and is unaffected", () =>
|
|
Effect.gen(function* () {
|
|
const info = yield* Tool.define(
|
|
"test-fn-tool",
|
|
Effect.succeed(() => Effect.succeed(makeTool("test"))),
|
|
)
|
|
|
|
const first = yield* info.init()
|
|
const second = yield* info.init()
|
|
|
|
expect(first).not.toBe(second)
|
|
}),
|
|
)
|
|
|
|
it.effect("object-defined tool returns distinct objects per init() call", () =>
|
|
Effect.gen(function* () {
|
|
const info = yield* Tool.define("test-copy", Effect.succeed(makeTool("test")))
|
|
|
|
const first = yield* info.init()
|
|
const second = yield* info.init()
|
|
|
|
expect(first).not.toBe(second)
|
|
}),
|
|
)
|
|
|
|
it.effect("execute receives decoded parameters", () =>
|
|
Effect.gen(function* () {
|
|
const parameters = Schema.Struct({
|
|
count: Schema.NumberFromString.pipe(Schema.optional, Schema.withDecodingDefaultType(Effect.succeed(5))),
|
|
})
|
|
const calls: Array<Schema.Schema.Type<typeof parameters>> = []
|
|
const info = yield* Tool.define(
|
|
"test-decoded",
|
|
Effect.succeed({
|
|
description: "test tool",
|
|
parameters,
|
|
execute(args: Schema.Schema.Type<typeof parameters>) {
|
|
calls.push(args)
|
|
return Effect.succeed({ title: "test", output: "ok", metadata: { truncated: false } })
|
|
},
|
|
}),
|
|
)
|
|
const ctx = makeCtx()
|
|
const tool = yield* info.init()
|
|
const execute = tool.execute as unknown as (args: unknown, ctx: Tool.Context) => ReturnType<typeof tool.execute>
|
|
|
|
yield* execute({}, ctx)
|
|
yield* execute({ count: "7" }, ctx)
|
|
|
|
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)
|
|
const args = invalid(exit) // kilocode_change
|
|
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"]`)
|
|
}),
|
|
)
|
|
|
|
// kilocode_change start
|
|
it.effect("invalid args explain missing required scalar fields without SchemaError jargon", () =>
|
|
Effect.gen(function* () {
|
|
const parameters = Schema.Struct({
|
|
pattern: Schema.String,
|
|
})
|
|
const info = yield* Tool.define(
|
|
"grep",
|
|
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>
|
|
|
|
const exit = yield* execute({}, makeCtx()).pipe(Effect.exit)
|
|
const args = invalid(exit)
|
|
expect(args.message).toContain("grep tool was called with invalid arguments")
|
|
expect(args.message).toContain("Please rewrite the input")
|
|
expect(args.detail).toContain(`["pattern"]`)
|
|
expect(args.detail.toLowerCase()).toContain("missing")
|
|
expect(args.detail.toLowerCase()).toContain("required")
|
|
expect(args.message).not.toContain("SchemaError(")
|
|
}),
|
|
)
|
|
|
|
it.effect("invalid args enumerate multiple failing fields", () =>
|
|
Effect.gen(function* () {
|
|
const parameters = Schema.Struct({
|
|
pattern: Schema.String,
|
|
path: Schema.String,
|
|
})
|
|
const info = yield* Tool.define(
|
|
"multi",
|
|
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>
|
|
|
|
const exit = yield* execute({}, makeCtx()).pipe(Effect.exit)
|
|
const args = invalid(exit)
|
|
expect(args.detail).toContain(`["pattern"]`)
|
|
expect(args.detail).toContain(`["path"]`)
|
|
}),
|
|
)
|
|
// kilocode_change end
|
|
})
|