fix(codemode): stop leaking undefined into tool arguments (#37652)
This commit is contained in:
parent
0d68b0bb20
commit
33f1b269e9
8 changed files with 97 additions and 20 deletions
|
|
@ -19,7 +19,7 @@ ultimate source of truth.
|
|||
- [x] Top-level `await` and `return` through the program's implicit async-function scope.
|
||||
- [x] Explicit `return`, final top-level expression as a REPL-style result, and `null` when no value is produced.
|
||||
- [x] Program results use JSON-like boundaries, with `undefined` and non-finite numbers normalized to `null`. Tool
|
||||
arguments remain subject to their schema and the outbound-handling gap listed below.
|
||||
arguments follow JSON serialization semantics before their schema applies (see the tools section).
|
||||
- [x] Live Date, RegExp, Map, Set, URL, and URLSearchParams values inside CodeMode.
|
||||
- [x] Tool calls through the host-provided `tools` tree only.
|
||||
- [x] The global `search(...)` built-in: synchronous tool discovery that counts as an admitted tool call and is
|
||||
|
|
@ -157,8 +157,11 @@ ultimate source of truth.
|
|||
- [x] Dotted tool names are canonicalized into namespace paths; a path can be both callable and a namespace, and the
|
||||
last definition supplied for a canonical path wins.
|
||||
- [x] Tool path segments may be named `constructor`, `prototype`, or `__proto__` because paths use inert Map keys.
|
||||
- [ ] Reject `undefined` and non-finite numbers in outbound tool arguments before render-only and OpenAPI tools run;
|
||||
retain null normalization for program results and JSON serialization.
|
||||
- [x] Outbound tool arguments follow JSON serialization semantics, like `JSON.stringify`: object properties with
|
||||
`undefined` values are dropped, `undefined` array elements and non-finite numbers become `null`, and sparse
|
||||
arrays densify. Tools never receive `undefined` inside their input object, though a bare `tools.t(undefined)`
|
||||
argument still reaches schema decoding as `undefined`. Program results keep the stricter
|
||||
normalization where every `undefined` becomes `null`.
|
||||
- [ ] Tokenize and case-fold non-ASCII tool paths, descriptions, and queries for tool search.
|
||||
|
||||
## Objects and properties
|
||||
|
|
|
|||
|
|
@ -44,7 +44,7 @@ export const normalizeError = (error: unknown): Diagnostic => {
|
|||
message = (value as { message: string }).message
|
||||
} else {
|
||||
try {
|
||||
message = JSON.stringify(copyOut(value)) ?? String(value)
|
||||
message = JSON.stringify(copyOut(value, "json")) ?? String(value)
|
||||
} catch {
|
||||
message = String(value)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -52,7 +52,7 @@ export const executeWithLimits = <const Provided extends Record<string, unknown>
|
|||
logs,
|
||||
)
|
||||
const value = yield* interpreter.run(program)
|
||||
const result = copyOut(copyIn(value, "Execution result"), true) as DataValue
|
||||
const result = copyOut(copyIn(value, "Execution result"), "nullify") as DataValue
|
||||
returned = { value: result, promises }
|
||||
const warnings = yield* promises.interrupt()
|
||||
return {
|
||||
|
|
|
|||
|
|
@ -89,7 +89,7 @@ const formatConsoleTable = (value: unknown, columnsArgument: unknown): string =>
|
|||
const consoleTableColumns = (value: unknown): ReadonlyArray<string> | undefined => {
|
||||
if (value === undefined) return undefined
|
||||
if (containsRuntimeReference(value)) return undefined
|
||||
const columns = copyOut(copyIn(value, "console.table columns"), true)
|
||||
const columns = copyOut(copyIn(value, "console.table columns"), "nullify")
|
||||
return Array.isArray(columns) ? columns.map((column) => String(column)) : undefined
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -18,7 +18,7 @@ export const invokeJsonMethod = (name: string, args: Array<unknown>, node: AstNo
|
|||
}
|
||||
const space = args[2]
|
||||
const indent = typeof space === "number" || typeof space === "string" ? space : undefined
|
||||
return JSON.stringify(copyOut(copyIn(args[0], "JSON.stringify value")), null, indent)
|
||||
return JSON.stringify(copyOut(copyIn(args[0], "JSON.stringify value"), "json"), null, indent)
|
||||
}
|
||||
case "parse": {
|
||||
const text = args[0]
|
||||
|
|
|
|||
|
|
@ -118,8 +118,7 @@ export class ToolRuntimeError extends Error {
|
|||
}
|
||||
}
|
||||
|
||||
const isDefinition = <R>(value: Definition<R> | Tools<R>): value is Definition<R> =>
|
||||
isToolDefinition<R>(value)
|
||||
const isDefinition = <R>(value: Definition<R> | Tools<R>): value is Definition<R> => isToolDefinition<R>(value)
|
||||
|
||||
const runHost = <A, E, R>(effect: Effect.Effect<A, E, R>): Effect.Effect<A, ToolError, R> =>
|
||||
effect.pipe(
|
||||
|
|
@ -257,18 +256,31 @@ const copyBounded = (
|
|||
return copied
|
||||
}
|
||||
|
||||
export const copyOut = (value: unknown, undefinedAsNull = false): unknown => {
|
||||
if (value === undefined && undefinedAsNull) return null
|
||||
// "json" mirrors JSON.stringify (undefined object values drop, undefined array elements become
|
||||
// null, a bare undefined passes through): use it wherever data leaves as JSON, like tool
|
||||
// arguments and stringify-style formatting. "nullify" turns every undefined, including a bare
|
||||
// one, into null: use it for program results, where the consumer must never see undefined.
|
||||
export type CopyOutMode = "json" | "nullify"
|
||||
|
||||
export const copyOut = (value: unknown, mode: CopyOutMode): unknown => {
|
||||
if (value === undefined && mode === "nullify") return null
|
||||
if (typeof value === "number" && !Number.isFinite(value)) {
|
||||
return null
|
||||
}
|
||||
if (Array.isArray(value)) {
|
||||
// Array.from densifies holes so sparse arrays normalize at the boundary like JSON does.
|
||||
return Array.from(value, (item) => copyOut(item, undefinedAsNull))
|
||||
return Array.from(value, (item) => {
|
||||
const copied = copyOut(item, mode)
|
||||
return copied === undefined && mode === "json" ? null : copied
|
||||
})
|
||||
}
|
||||
|
||||
if (value !== null && typeof value === "object" && !(value instanceof ToolReference)) {
|
||||
return Object.fromEntries(Object.entries(value).map(([key, item]) => [key, copyOut(item, undefinedAsNull)]))
|
||||
return Object.fromEntries(
|
||||
Object.entries(value)
|
||||
.map(([key, item]) => [key, copyOut(item, mode)] as const)
|
||||
.filter(([, item]) => !(item === undefined && mode === "json")),
|
||||
)
|
||||
}
|
||||
|
||||
return value
|
||||
|
|
@ -696,13 +708,13 @@ export const make = <R>(
|
|||
invokeDefinition(
|
||||
"search",
|
||||
searchTool,
|
||||
args.map((arg) => copyOut(copyIn(arg, "Arguments for tool 'search'"))),
|
||||
args.map((arg) => copyOut(copyIn(arg, "Arguments for tool 'search'"), "json")),
|
||||
),
|
||||
),
|
||||
invoke: (path, args) =>
|
||||
Effect.gen(function* () {
|
||||
const name = canonicalSegments(path).join(".")
|
||||
const externalArgs = args.map((arg) => copyOut(copyIn(arg, `Arguments for tool '${name}'`)))
|
||||
const externalArgs = args.map((arg) => copyOut(copyIn(arg, `Arguments for tool '${name}'`), "json"))
|
||||
const tool = resolve(root, path)
|
||||
return yield* invokeDefinition(name, tool, externalArgs)
|
||||
}),
|
||||
|
|
|
|||
|
|
@ -453,6 +453,56 @@ describe("CodeMode schema flexibility", () => {
|
|||
expect(observed).toStrictEqual([{ id: 42 }])
|
||||
})
|
||||
|
||||
test("outbound tool arguments follow JSON serialization semantics", async () => {
|
||||
const observed: Array<unknown> = []
|
||||
const call = Tool.make({
|
||||
description: "Observe raw input",
|
||||
input: { type: "object" },
|
||||
run: (input) =>
|
||||
Effect.sync(() => {
|
||||
observed.push(input)
|
||||
return "ok"
|
||||
}),
|
||||
})
|
||||
const runtime = CodeMode.make({ tools: { adapter: { call } } })
|
||||
|
||||
const result = await Effect.runPromise(
|
||||
runtime.execute(
|
||||
`return await tools.adapter.call({ q: undefined, limit: 0 / 0, rate: 1 / 0, items: [1, undefined, 2], holes: [1, , 3] })`,
|
||||
),
|
||||
)
|
||||
expect(result.ok).toBe(true)
|
||||
const received = observed[0] as Record<string, unknown>
|
||||
expect(received).toStrictEqual({ limit: null, rate: null, items: [1, null, 2], holes: [1, null, 3] })
|
||||
// The undefined-valued property is dropped like JSON.stringify, not delivered as undefined.
|
||||
expect(Object.hasOwn(received, "q")).toBe(false)
|
||||
})
|
||||
|
||||
test("dropping undefined values lets optionalKey schemas accept conditional arguments", async () => {
|
||||
const observed: Array<unknown> = []
|
||||
const find = Tool.make({
|
||||
description: "Find things",
|
||||
input: Schema.Struct({ query: Schema.optionalKey(Schema.String), limit: Schema.optionalKey(Schema.Number) }),
|
||||
run: (input) =>
|
||||
Effect.sync(() => {
|
||||
observed.push(input)
|
||||
return "ok"
|
||||
}),
|
||||
})
|
||||
const runtime = CodeMode.make({ tools: { things: { find } } })
|
||||
|
||||
// The `cond ? value : undefined` idiom: optionalKey rejects a present undefined, so the
|
||||
// JSON boundary must drop the key before the schema decodes.
|
||||
const result = await Effect.runPromise(
|
||||
runtime.execute(`return await tools.things.find({ query: undefined, limit: 5 })`),
|
||||
)
|
||||
expect(result.ok).toBe(true)
|
||||
expect(observed).toStrictEqual([{ limit: 5 }])
|
||||
|
||||
const search = await Effect.runPromise(runtime.execute(`return (await search({ query: undefined })).items.length`))
|
||||
expect(search.ok).toBe(true)
|
||||
})
|
||||
|
||||
test("renders JSON Schema outputs and $defs references", async () => {
|
||||
const lookup = Tool.make({
|
||||
description: "Look up a user",
|
||||
|
|
|
|||
|
|
@ -258,11 +258,23 @@ describe("H1: NaN/Infinity flow as intermediates and normalize to null at the bo
|
|||
|
||||
test("copyOut normalizes non-finite numbers to null (the shared return + tool-arg boundary)", () => {
|
||||
// Tool-call arguments funnel through copyOut too, so this one function pins both boundaries.
|
||||
expect(ToolRuntime.copyOut(NaN)).toBeNull()
|
||||
expect(ToolRuntime.copyOut(Infinity)).toBeNull()
|
||||
expect(ToolRuntime.copyOut(-Infinity)).toBeNull()
|
||||
expect(ToolRuntime.copyOut(42)).toBe(42)
|
||||
expect(ToolRuntime.copyOut({ a: NaN, b: [Infinity, 1] })).toEqual({ a: null, b: [null, 1] })
|
||||
expect(ToolRuntime.copyOut(NaN, "json")).toBeNull()
|
||||
expect(ToolRuntime.copyOut(Infinity, "json")).toBeNull()
|
||||
expect(ToolRuntime.copyOut(-Infinity, "nullify")).toBeNull()
|
||||
expect(ToolRuntime.copyOut(42, "json")).toBe(42)
|
||||
expect(ToolRuntime.copyOut({ a: NaN, b: [Infinity, 1] }, "json")).toEqual({ a: null, b: [null, 1] })
|
||||
})
|
||||
})
|
||||
|
||||
describe("copyOut undefined handling per boundary mode", () => {
|
||||
test("json mode mirrors JSON.stringify for undefined", () => {
|
||||
expect(ToolRuntime.copyOut({ q: undefined, keep: 1 }, "json")).toStrictEqual({ keep: 1 })
|
||||
expect(ToolRuntime.copyOut([1, undefined, 2], "json")).toStrictEqual([1, null, 2])
|
||||
expect(ToolRuntime.copyOut({ nested: { a: undefined, b: [undefined] } }, "json")).toStrictEqual({
|
||||
nested: { b: [null] },
|
||||
})
|
||||
expect(ToolRuntime.copyOut(undefined, "json")).toBeUndefined()
|
||||
expect(ToolRuntime.copyOut({ a: undefined }, "nullify")).toStrictEqual({ a: null })
|
||||
})
|
||||
})
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue