fix(core): drop invalid optional tool inputs
This commit is contained in:
parent
1b39d364bd
commit
b8ee2396d5
3 changed files with 77 additions and 27 deletions
|
|
@ -1,7 +1,7 @@
|
|||
import type { ToolDefinition } from "@opencode-ai/ai"
|
||||
import { Tool } from "@opencode-ai/schema/tool"
|
||||
import type { StandardJSONSchemaV1, StandardSchemaV1 } from "@standard-schema/spec"
|
||||
import { Effect, JsonSchema, Schema } from "effect"
|
||||
import { Effect, JsonSchema, Schema, SchemaAST, SchemaIssue } from "effect"
|
||||
|
||||
export const definition = (tool: Tool.Info<any, any>): ToolDefinition => ({
|
||||
name: effectiveName(tool),
|
||||
|
|
@ -33,18 +33,52 @@ export const execute = (tool: Tool.Info<any, any>, input: unknown, context: Tool
|
|||
|
||||
const decodeInput = (schema: Tool.ValueSchema<any>, value: unknown) => {
|
||||
if (Schema.isSchema(schema))
|
||||
return Schema.decodeUnknownEffect(schema)(value).pipe(
|
||||
return Schema.decodeUnknownEffect(schema)(value, { errors: "all" }).pipe(
|
||||
Effect.catch((error) => {
|
||||
const input = dropInvalidOptionalFields(schema, value, error)
|
||||
return input === value ? Effect.fail(error) : Schema.decodeUnknownEffect(schema)(input)
|
||||
}),
|
||||
Effect.mapError((error) => new Tool.Error({ message: `Invalid tool input: ${error.message}` })),
|
||||
)
|
||||
if (isStandardSchema(schema)) return validateStandard(schema, value, "Invalid tool input")
|
||||
return Effect.succeed(value)
|
||||
}
|
||||
|
||||
const dropInvalidOptionalFields = (schema: Schema.Codec<any, any>, value: unknown, error: unknown) => {
|
||||
if (
|
||||
schema.ast._tag !== "Objects" ||
|
||||
typeof value !== "object" ||
|
||||
value === null ||
|
||||
Array.isArray(value) ||
|
||||
!Schema.isSchemaError(error)
|
||||
)
|
||||
return value
|
||||
const optional = new Set(
|
||||
schema.ast.propertySignatures
|
||||
.filter((field) => SchemaAST.isOptional(field.type))
|
||||
.map((field) => field.name)
|
||||
.filter((key): key is string => typeof key === "string"),
|
||||
)
|
||||
const invalid = new Set(
|
||||
issueKeys(error.issue).filter((key): key is string => typeof key === "string" && optional.has(key)),
|
||||
)
|
||||
if (invalid.size === 0) return value
|
||||
return Object.fromEntries(Object.entries(value).filter(([key]) => !invalid.has(key)))
|
||||
}
|
||||
|
||||
const issueKeys = (issue: SchemaIssue.Issue): ReadonlyArray<PropertyKey> => {
|
||||
if (issue._tag === "Pointer") return issue.path.slice(0, 1)
|
||||
if (issue._tag === "Composite" || issue._tag === "AnyOf") return issue.issues.flatMap(issueKeys)
|
||||
if (issue._tag === "Encoding" || issue._tag === "Filter") return issueKeys(issue.issue)
|
||||
return []
|
||||
}
|
||||
|
||||
const encodeOutput = (schema: Tool.ValueSchema<any>, value: unknown) => {
|
||||
if (Schema.isSchema(schema))
|
||||
return Schema.encodeEffect(schema)(value).pipe(
|
||||
Effect.mapError(
|
||||
(error) => new Tool.Error({ message: `Tool returned an invalid value for its output schema: ${error.message}` }),
|
||||
(error) =>
|
||||
new Tool.Error({ message: `Tool returned an invalid value for its output schema: ${error.message}` }),
|
||||
),
|
||||
)
|
||||
if (isStandardSchema(schema))
|
||||
|
|
|
|||
|
|
@ -60,13 +60,13 @@ test("portable schemas validate and describe typed tools", async () => {
|
|||
},
|
||||
},
|
||||
}
|
||||
const tool: Info = ({
|
||||
const tool: Info = {
|
||||
name: "portable",
|
||||
description: "Portable tool",
|
||||
input,
|
||||
output,
|
||||
execute: ({ count }) => Effect.succeed({ output: count + 1 }),
|
||||
})
|
||||
}
|
||||
|
||||
expect(definition(tool)).toEqual({
|
||||
name: "portable",
|
||||
|
|
@ -106,16 +106,43 @@ test("portable schema failures become tool failures", async () => {
|
|||
expect(error.toString()).toContain("Invalid tool input: expected a string")
|
||||
})
|
||||
|
||||
test("invalid optional Effect schema fields are dropped", async () => {
|
||||
const tool: Info = {
|
||||
name: "optional",
|
||||
description: "Optional",
|
||||
input: Schema.Struct({ value: Schema.String, limit: Schema.optional(Schema.Number) }),
|
||||
execute: (input) => Effect.succeed({ content: JSON.stringify(input) }),
|
||||
}
|
||||
|
||||
expect(await Effect.runPromise(execute(tool, { value: "ok", limit: "many" }, {} as Tool.Context))).toEqual({
|
||||
output: undefined,
|
||||
content: [{ type: "text", text: '{"value":"ok"}' }],
|
||||
})
|
||||
})
|
||||
|
||||
test("invalid required Effect schema fields still fail", async () => {
|
||||
const tool: Info = {
|
||||
name: "required",
|
||||
description: "Required",
|
||||
input: Schema.Struct({ value: Schema.String, limit: Schema.optional(Schema.Number) }),
|
||||
execute: () => Effect.succeed({ content: "unused" }),
|
||||
}
|
||||
|
||||
const error = await Effect.runPromiseExit(execute(tool, { value: 1, limit: "many" }, {} as Tool.Context))
|
||||
expect(error.toString()).toContain("Invalid tool input")
|
||||
expect(error.toString()).toContain('["value"]')
|
||||
})
|
||||
|
||||
test("canonical results carry metadata with typed output", async () => {
|
||||
const input = Schema.Struct({ value: Schema.String })
|
||||
const output = Schema.Struct({ value: Schema.String, internal: Schema.Boolean })
|
||||
const tool: Info = ({
|
||||
const tool: Info = {
|
||||
name: "annotated",
|
||||
description: "Annotated tool",
|
||||
input,
|
||||
output,
|
||||
execute: ({ value }) => Effect.succeed({ output: { value, internal: true }, metadata: { value }, content: value }),
|
||||
})
|
||||
}
|
||||
|
||||
expect(await Effect.runPromise(tool.execute({ value: "out" }, {} as Tool.Context))).toEqual({
|
||||
output: { value: "out", internal: true },
|
||||
|
|
@ -126,12 +153,12 @@ test("canonical results carry metadata with typed output", async () => {
|
|||
|
||||
test("raw JSON schemas are render-only and omitted output means model-only", async () => {
|
||||
const input = { type: "object", properties: { value: { type: "string" } } }
|
||||
const tool: Info = ({
|
||||
const tool: Info = {
|
||||
name: "raw",
|
||||
description: "Raw tool",
|
||||
input,
|
||||
execute: (input) => Effect.succeed({ content: JSON.stringify(input) }),
|
||||
})
|
||||
}
|
||||
|
||||
expect(definition(tool)).toEqual({
|
||||
name: "raw",
|
||||
|
|
|
|||
|
|
@ -24,14 +24,7 @@ import { executeTool, registerToolPlugin, toolIdentity } from "./lib/tool"
|
|||
const globToolNode = makeLocationNode({
|
||||
name: "test/glob-tool-plugin",
|
||||
layer: Layer.effectDiscard(registerToolPlugin(GlobTool.Plugin)),
|
||||
deps: [
|
||||
Tool.node,
|
||||
FSUtil.node,
|
||||
Ripgrep.node,
|
||||
Location.node,
|
||||
LocationMutation.node,
|
||||
Permission.node,
|
||||
],
|
||||
deps: [Tool.node, FSUtil.node, Ripgrep.node, Location.node, LocationMutation.node, Permission.node],
|
||||
})
|
||||
const grepToolNode = makeLocationNode({
|
||||
name: "test/grep-tool-plugin",
|
||||
|
|
@ -84,7 +77,7 @@ const call = (name: "glob" | "grep", input: unknown) => ({
|
|||
const it = testEffect(Layer.empty)
|
||||
|
||||
describe("search tools", () => {
|
||||
it.live("bounds omitted glob and grep limits", () =>
|
||||
it.live("bounds omitted and invalid optional search limits", () =>
|
||||
Effect.acquireUseRelease(
|
||||
Effect.promise(() => tmpdir()),
|
||||
(tmp) =>
|
||||
|
|
@ -99,9 +92,11 @@ describe("search tools", () => {
|
|||
yield* withTools(tmp.path, (registry) =>
|
||||
Effect.gen(function* () {
|
||||
const glob = yield* executeTool(registry, call("glob", { pattern: "*" }))
|
||||
const invalidGlob = yield* executeTool(registry, call("glob", { pattern: "*", limit: "many" }))
|
||||
const grep = yield* executeTool(registry, call("grep", { pattern: "needle" }))
|
||||
|
||||
expect(glob.metadata).toEqual({ count: FileSystem.DEFAULT_SEARCH_LIMIT, truncated: true })
|
||||
expect(invalidGlob.metadata).toEqual({ count: FileSystem.DEFAULT_SEARCH_LIMIT, truncated: true })
|
||||
expect(grep.metadata).toEqual({ matches: FileSystem.DEFAULT_SEARCH_LIMIT, truncated: true })
|
||||
expect(glob.content).toHaveLength(1)
|
||||
expect(grep.content).toHaveLength(1)
|
||||
|
|
@ -186,9 +181,7 @@ describe("search tools", () => {
|
|||
Effect.promise(() => tmpdir()),
|
||||
(tmp) =>
|
||||
Effect.promise(() => fs.writeFile(path.join(tmp.path, "file.txt"), "haystack\n")).pipe(
|
||||
Effect.andThen(
|
||||
withTools(tmp.path, (registry) => executeTool(registry, call("grep", { pattern: "needle" }))),
|
||||
),
|
||||
Effect.andThen(withTools(tmp.path, (registry) => executeTool(registry, call("grep", { pattern: "needle" })))),
|
||||
Effect.tap((result) =>
|
||||
Effect.sync(() => {
|
||||
expect(result).toMatchObject({
|
||||
|
|
@ -297,9 +290,7 @@ describe("search tools", () => {
|
|||
(tmp) =>
|
||||
Effect.promise(() => fs.writeFile(path.join(tmp.path, "file.txt"), "content\n")).pipe(
|
||||
Effect.andThen(
|
||||
withTools(tmp.path, (registry) =>
|
||||
executeTool(registry, call("glob", { path: "file.txt", pattern: "*" })),
|
||||
),
|
||||
withTools(tmp.path, (registry) => executeTool(registry, call("glob", { path: "file.txt", pattern: "*" }))),
|
||||
),
|
||||
Effect.tap((result) =>
|
||||
Effect.sync(() => {
|
||||
|
|
@ -331,9 +322,7 @@ describe("search tools", () => {
|
|||
Effect.sync(() => {
|
||||
expect(result.status).toBe("completed")
|
||||
expect(assertions.map((input) => input.action)).toEqual(["external_directory", "glob"])
|
||||
expect(assertions[0]?.resources).toEqual([
|
||||
path.join(outside.path, "*").replaceAll("\\", "/"),
|
||||
])
|
||||
expect(assertions[0]?.resources).toEqual([path.join(outside.path, "*").replaceAll("\\", "/")])
|
||||
}),
|
||||
),
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue