fix(codemode): canonicalize dotted tool paths (#36994)

This commit is contained in:
Aiden Cline
2026-07-15 11:09:09 -05:00
committed by GitHub
parent f5dd181443
commit 22334b94c8
7 changed files with 242 additions and 44 deletions
+4
View File
@@ -75,6 +75,10 @@ is decoded before `run` is invoked; an Effect Schema `output` is decoded and cop
Schemas only shape the model-visible signature. Without `output` the signature advertises `Promise<unknown>`. Schemas only shape the model-visible signature. Without `output` the signature advertises `Promise<unknown>`.
Descriptions and schemas are model-visible contract; keep authorization in `run`. Descriptions and schemas are model-visible contract; keep authorization in `run`.
Dots in tool names are namespace separators: `{ "issues.list": tool }` exposes `tools.issues.list(...)`, exactly like
`{ issues: { list: tool } }`. Other non-identifier characters render with bracket notation, e.g.
`tools.context7["resolve-library-id"](...)`.
### `CodeMode.execute` and `CodeMode.make` ### `CodeMode.execute` and `CodeMode.make`
`CodeMode.execute({ ...options, code })` runs once and is equivalent to `CodeMode.make(options).execute(code)`. A `CodeMode.execute({ ...options, code })` runs once and is equivalent to `CodeMode.make(options).execute(code)`. A
+8 -2
View File
@@ -168,7 +168,8 @@ ultimate source of truth.
- [ ] `Object.is`; runtime and tool-reference identity semantics need to be defined first. - [ ] `Object.is`; runtime and tool-reference identity semantics need to be defined first.
- [ ] `Object.groupBy`. - [ ] `Object.groupBy`.
- [ ] Object creation, descriptors, freezing/sealing, prototype APIs, and reflection APIs. - [ ] Object creation, descriptors, freezing/sealing, prototype APIs, and reflection APIs.
- [ ] A final policy for legal data/tool keys named `__proto__`, `constructor`, or `prototype`. - [ ] A final policy for legal data keys named `__proto__`, `constructor`, or `prototype` (tool path segments
already allow them; see known semantic gaps).
## Arrays ## Arrays
@@ -312,7 +313,12 @@ ultimate source of truth.
These are actionable implementation items. Check them off only when behavior and direct tests land. These are actionable implementation items. Check them off only when behavior and direct tests land.
- [x] Return real promises from `Promise.all`, `Promise.allSettled`, and `Promise.race`. - [x] Return real promises from `Promise.all`, `Promise.allSettled`, and `Promise.race`.
- [ ] Guarantee every advertised tool path is executable, including dotted and blocked path segments. - [x] Canonicalize dotted tool names into namespace paths so every advertised dotted path is executable, one
canonical path can be both a callable tool and a namespace, and the last definition supplied for a canonical
path wins.
- [x] Allow blocked member names (`constructor`, `prototype`, `__proto__`) as tool path segments: segments are Map
keys and inert strings, never plain-object property accesses, so every advertised path is executable. Blocked
member access on data values stays rejected. Tool names with empty segments are rejected at construction.
- [ ] Define safe outbound handling for non-finite numbers and `undefined` so invalid values cannot silently become - [ ] Define safe outbound handling for non-finite numbers and `undefined` so invalid values cannot silently become
`null` in render-only or OpenAPI tool calls. `null` in render-only or OpenAPI tool calls.
- [ ] Make regular-expression execution genuinely timeout-safe, or narrow the timeout guarantee explicitly. - [ ] Make regular-expression execution genuinely timeout-safe, or narrow the timeout guarantee explicitly.
+2 -2
View File
@@ -1759,8 +1759,8 @@ export class Interpreter<R> {
: self.toPropertyKey(yield* self.evaluateExpression(propertyNode), propertyNode) : self.toPropertyKey(yield* self.evaluateExpression(propertyNode), propertyNode)
if (objectValue instanceof ToolReference) { if (objectValue instanceof ToolReference) {
if (typeof key !== "string" || isBlockedMember(key)) { if (typeof key !== "string") {
throw new InterpreterRuntimeError("Tool paths must use safe string property names.", propertyNode) throw new InterpreterRuntimeError("Tool paths must use string property names.", propertyNode)
} }
return new ToolReference([...objectValue.path, key]) return new ToolReference([...objectValue.path, key])
} }
+57 -37
View File
@@ -274,15 +274,41 @@ export const copyOut = (value: unknown, undefinedAsNull = false): unknown => {
return value return value
} }
// Dots in tool names are namespace separators; the last definition for a canonical path wins.
type ToolNode<R> = {
definition?: Definition<R>
readonly children: Map<string, ToolNode<R>>
}
const toolTrie = <R>(tools: Tools<R>): ToolNode<R> => {
const root: ToolNode<R> = { children: new Map() }
const insert = (node: ToolNode<R>, group: Tools<R>): void => {
for (const [name, value] of Object.entries(group)) {
let current = node
for (const segment of name.split(".")) {
if (segment === "") throw new TypeError(`Tool name '${name}' contains an empty segment.`)
const child = current.children.get(segment) ?? { children: new Map() }
current.children.set(segment, child)
current = child
}
if (isDefinition(value)) current.definition = value
else insert(current, value)
}
}
insert(root, tools)
return root
}
const canonicalSegments = (path: ReadonlyArray<string>): ReadonlyArray<string> =>
path.flatMap((segment) => segment.split("."))
const definitions = <R>( const definitions = <R>(
tools: Tools<R>, node: ToolNode<R>,
path: ReadonlyArray<string> = [], path: ReadonlyArray<string> = [],
): Array<{ path: string; definition: Definition<R> }> => ): Array<{ path: string; definition: Definition<R> }> => [
Object.entries(tools).flatMap(([name, value]) => { ...(node.definition === undefined ? [] : [{ path: path.join("."), definition: node.definition }]),
const next = [...path, name] ...Array.from(node.children, ([name, child]) => definitions(child, [...path, name])).flat(),
if (isDefinition(value)) return [{ path: next.join("."), definition: value }] ]
return definitions(value, next)
})
const describeDefinition = <R>(path: string, definition: Definition<R>): ToolDescription => ({ const describeDefinition = <R>(path: string, definition: Definition<R>): ToolDescription => ({
path, path,
@@ -291,7 +317,7 @@ const describeDefinition = <R>(path: string, definition: Definition<R>): ToolDes
}) })
const visibleDefinitions = <R>(tools: Tools<R>) => const visibleDefinitions = <R>(tools: Tools<R>) =>
definitions(tools).map(({ path, definition }) => ({ definitions(toolTrie(tools)).map(({ path, definition }) => ({
path, path,
definition, definition,
description: describeDefinition(path, definition), description: describeDefinition(path, definition),
@@ -555,37 +581,30 @@ export const prepare = <R>(tools: Tools<R>, catalogBudget = defaultCatalogBudget
} }
} }
const namespaceKeys = <R>(tools: Tools<R>, path: ReadonlyArray<string>): ReadonlyArray<string> => { const lookup = <R>(root: ToolNode<R>, segments: ReadonlyArray<string>): ToolNode<R> | undefined =>
let value: Definition<R> | Tools<R> = tools segments.reduce<ToolNode<R> | undefined>((node, segment) => node?.children.get(segment), root)
for (const segment of path) {
if (isBlockedMember(segment) || isDefinition(value) || !Object.hasOwn(value, segment)) { const namespaceKeys = <R>(root: ToolNode<R>, path: ReadonlyArray<string>): ReadonlyArray<string> => {
throw new ToolRuntimeError("UnknownTool", `Unknown tool namespace '${path.join(".")}'.`, [ const segments = canonicalSegments(path)
"Object.keys(tools) lists the available namespaces; search({ query }) finds described tools.", const node = lookup(root, segments)
]) if (node === undefined) {
} throw new ToolRuntimeError("UnknownTool", `Unknown tool namespace '${segments.join(".")}'.`)
value = value[segment] as Definition<R> | Tools<R>
} }
if (isDefinition(value)) return [] return Array.from(node.children.keys())
return Object.keys(value)
} }
const resolve = <R>(tools: Tools<R>, path: ReadonlyArray<string>): Definition<R> => { const resolve = <R>(root: ToolNode<R>, path: ReadonlyArray<string>): Definition<R> => {
let value: Definition<R> | Tools<R> = tools const segments = canonicalSegments(path)
const node = lookup(root, segments)
for (const segment of path) { if (node === undefined) {
if (isBlockedMember(segment) || isDefinition(value) || !Object.hasOwn(value, segment)) { throw new ToolRuntimeError("UnknownTool", `Unknown tool '${segments.join(".")}'.`, [
throw new ToolRuntimeError("UnknownTool", `Unknown tool '${path.join(".")}'.`, [ "Use search({ query }) to find available described tools.",
"Use search({ query }) to find available described tools.", ])
])
}
value = value[segment] as Definition<R> | Tools<R>
} }
if (node.definition === undefined) {
if (!isDefinition(value)) { throw new ToolRuntimeError("UnknownTool", `Tool '${segments.join(".")}' is not callable.`)
throw new ToolRuntimeError("UnknownTool", `Tool '${path.join(".")}' is not callable.`)
} }
return node.definition
return value
} }
export type ToolRuntime<R = never> = { export type ToolRuntime<R = never> = {
@@ -603,6 +622,7 @@ export const make = <R>(
hooks?: ToolCallHooks<R>, hooks?: ToolCallHooks<R>,
): ToolRuntime<R> => { ): ToolRuntime<R> => {
const calls: Array<ToolCall> = [] const calls: Array<ToolCall> = []
const root = toolTrie(tools)
const searchTool = makeSearchTool(searchIndex) const searchTool = makeSearchTool(searchIndex)
// End hooks observe settled success or failure; interruption emits neither outcome. // End hooks observe settled success or failure; interruption emits neither outcome.
@@ -670,7 +690,7 @@ export const make = <R>(
return { return {
root: new ToolReference([]), root: new ToolReference([]),
calls, calls,
keys: (path) => namespaceKeys(tools, path), keys: (path) => namespaceKeys(root, path),
search: (args) => search: (args) =>
Effect.suspend(() => Effect.suspend(() =>
invokeDefinition( invokeDefinition(
@@ -681,9 +701,9 @@ export const make = <R>(
), ),
invoke: (path, args) => invoke: (path, args) =>
Effect.gen(function* () { Effect.gen(function* () {
const name = path.join(".") 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}'`)))
const tool = resolve(tools, path) const tool = resolve(root, path)
return yield* invokeDefinition(name, tool, externalArgs) return yield* invokeDefinition(name, tool, externalArgs)
}), }),
} }
+6 -1
View File
@@ -50,8 +50,13 @@ export type Options<I extends SchemaType, O extends SchemaType | undefined, R =
readonly run: (input: InputType<I>) => Effect.Effect<ResultType<O>, unknown, R> readonly run: (input: InputType<I>) => Effect.Effect<ResultType<O>, unknown, R>
} }
// Object.hasOwn: an inherited _tag must not classify a namespace as a Definition.
export const isDefinition = <R = never>(value: unknown): value is Definition<R> => export const isDefinition = <R = never>(value: unknown): value is Definition<R> =>
typeof value === "object" && value !== null && "_tag" in value && value._tag === "CodeModeTool" typeof value === "object" &&
value !== null &&
"_tag" in value &&
Object.hasOwn(value, "_tag") &&
value._tag === "CodeModeTool"
/** /**
* Defines one schema-described tool available to a CodeMode program through `tools.*`. * Defines one schema-described tool available to a CodeMode program through `tools.*`.
+1 -2
View File
@@ -56,11 +56,10 @@ describe("Object.keys over tool references", () => {
expect(await value(`return typeof search`)).toBe("function") expect(await value(`return typeof search`)).toBe("function")
}) })
test("an unknown namespace is an UnknownTool error pointing at the discovery idioms", async () => { test("an unknown namespace is an UnknownTool error", async () => {
const failure = await error(`return Object.keys(tools.nonexistent)`) const failure = await error(`return Object.keys(tools.nonexistent)`)
expect(failure.kind).toBe("UnknownTool") expect(failure.kind).toBe("UnknownTool")
expect(failure.message).toContain("Unknown tool namespace 'nonexistent'") expect(failure.message).toContain("Unknown tool namespace 'nonexistent'")
expect(failure.suggestions?.join(" ")).toContain("Object.keys(tools)")
}) })
test("Object.values/entries on a tool reference explain the working idioms", async () => { test("Object.values/entries on a tool reference explain the working idioms", async () => {
+164
View File
@@ -0,0 +1,164 @@
import { describe, expect, test } from "bun:test"
import { Effect, Schema } from "effect"
import { CodeMode, Tool } from "../src/index.js"
const echo = (description: string, result: string) =>
Tool.make({
description,
input: Schema.Struct({}),
output: Schema.String,
run: () => Effect.succeed(result),
})
const value = async (runtime: CodeMode.Runtime, code: string) => {
const result = await Effect.runPromise(runtime.execute(code))
if (!result.ok) throw new Error(`expected success, got ${result.error.kind}: ${result.error.message}`)
return result.value
}
const failure = async (runtime: CodeMode.Runtime, code: string) => {
const result = await Effect.runPromise(runtime.execute(code))
if (result.ok) throw new Error(`expected failure, got value ${JSON.stringify(result.value)}`)
return result.error
}
describe("dotted tool names", () => {
const runtime = CodeMode.make({ tools: { api: { "issues.list": echo("List issues", "listed") } } })
test("a dotted name becomes nested namespaces in the catalog", () => {
const catalog = runtime.catalog()
expect(catalog).toHaveLength(1)
expect(catalog[0]?.path).toBe("api.issues.list")
expect(catalog[0]?.signature).toStartWith("tools.api.issues.list(input:")
expect(runtime.instructions()).toContain("tools.api.issues.list(input:")
})
test("the advertised dotted path is executable", async () => {
expect(await value(runtime, `return await tools.api.issues.list({})`)).toBe("listed")
})
test("bracket access with a dotted segment spells the same canonical path", async () => {
expect(await value(runtime, `return await tools.api["issues.list"]({})`)).toBe("listed")
expect(await value(runtime, `return await tools["api.issues"].list({})`)).toBe("listed")
})
test("intermediate segments enumerate like ordinary namespaces", async () => {
expect(await value(runtime, `return [Object.keys(tools.api), Object.keys(tools.api.issues)]`)).toEqual([
["issues"],
["list"],
])
expect(await value(runtime, `return Object.keys(tools["api.issues"])`)).toEqual(["list"])
})
test("a top-level dotted name nests from the root", async () => {
const flat = CodeMode.make({ tools: { "issues.list": echo("List issues", "flat") } })
expect(flat.catalog()[0]?.path).toBe("issues.list")
expect(await value(flat, `return await tools.issues.list({})`)).toBe("flat")
})
})
describe("callable namespaces", () => {
const runtime = CodeMode.make({
tools: { issues: echo("All issues", "all"), "issues.list": echo("List issues", "list") },
})
test("a path can hold a tool and child tools at once", async () => {
expect(await value(runtime, `return await tools.issues({})`)).toBe("all")
expect(await value(runtime, `return await tools.issues.list({})`)).toBe("list")
expect(runtime.catalog().map((tool) => tool.path)).toEqual(["issues", "issues.list"])
})
test("a callable namespace enumerates its children", async () => {
expect(await value(runtime, `return Object.keys(tools.issues)`)).toEqual(["list"])
})
test("search returns executable paths for both", async () => {
const result = await value(runtime, `return search({ query: "", namespace: "issues" })`)
expect((result as { items: Array<{ path: string }> }).items.map((item) => item.path)).toEqual([
"tools.issues",
"tools.issues.list",
])
const exact = await value(runtime, `return search({ query: "tools.issues.list" })`)
expect((exact as { items: Array<{ path: string }> }).items.map((item) => item.path)).toEqual(["tools.issues.list"])
})
test("an unknown child under a callable tool is an UnknownTool error", async () => {
const diagnostic = await failure(runtime, `return await tools.issues.missing({})`)
expect(diagnostic.kind).toBe("UnknownTool")
expect(diagnostic.message).toContain("Unknown tool 'issues.missing'")
})
test("a namespace without its own definition stays non-callable", async () => {
const nested = CodeMode.make({ tools: { "issues.list": echo("List issues", "list") } })
const diagnostic = await failure(nested, `return await tools.issues({})`)
expect(diagnostic.kind).toBe("UnknownTool")
expect(diagnostic.message).toContain("Tool 'issues' is not callable")
})
})
describe("blocked member names on tool paths", () => {
const runtime = CodeMode.make({
tools: {
prototype: echo("Prototype tool", "proto"),
"issues.constructor": echo("Constructor tool", "ctor"),
nested: { ["__proto__"]: echo("Proto tool", "dunder") },
},
})
test("tools may use blocked member names because path segments never touch real properties", async () => {
expect(runtime.catalog().map((tool) => tool.path)).toEqual(["prototype", "issues.constructor", "nested.__proto__"])
expect(await value(runtime, `return await tools.prototype({})`)).toBe("proto")
expect(await value(runtime, `return await tools.issues.constructor({})`)).toBe("ctor")
expect(await value(runtime, `return await tools["issues.constructor"]({})`)).toBe("ctor")
expect(await value(runtime, `return await tools.nested.__proto__({})`)).toBe("dunder")
expect(await value(runtime, `return Object.keys(tools.issues)`)).toEqual(["constructor"])
})
test("a literal __proto__ key cannot poison a namespace into a fake definition", async () => {
const poisoned = CodeMode.make({
tools: { ns: { "__proto__": echo("Hidden", "hidden"), real: echo("Real tool", "real") } },
})
expect(poisoned.catalog().map((tool) => tool.path)).toEqual(["ns.real"])
expect(await value(poisoned, `return await tools.ns.real({})`)).toBe("real")
})
test("blocked member access on data values stays blocked", async () => {
const diagnostic = await failure(runtime, `const x = {}; return x.constructor`)
expect(diagnostic.message).toContain("constructor")
expect(Object.keys(Object.prototype)).toEqual([])
})
})
describe("empty segments", () => {
test("tool names with empty segments are rejected at make", () => {
for (const name of ["", "a..b", "trail.", ".lead"]) {
expect(() => CodeMode.make({ tools: { [name]: echo("Bad", "bad") } })).toThrow("empty segment")
}
})
})
describe("canonical path collisions", () => {
test("the last definition supplied for a canonical path wins", async () => {
const runtime = CodeMode.make({
tools: { "issues.list": echo("First", "first"), issues: { list: echo("Second", "second") } },
})
expect(await value(runtime, `return await tools.issues.list({})`)).toBe("second")
expect(runtime.catalog()).toHaveLength(1)
expect(runtime.catalog()[0]?.description).toBe("Second")
})
test("overriding one path keeps sibling tools from both shapes", async () => {
const runtime = CodeMode.make({
tools: {
"issues.list": echo("First list", "first"),
issues: { list: echo("Second list", "second"), get: echo("Get issue", "got") },
"issues.close": echo("Close issue", "closed"),
},
})
// Catalog order follows first appearance of each canonical path.
expect(runtime.catalog().map((tool) => tool.path)).toEqual(["issues.list", "issues.get", "issues.close"])
expect(await value(runtime, `return await tools.issues.list({})`)).toBe("second")
expect(await value(runtime, `return await tools.issues.get({})`)).toBe("got")
expect(await value(runtime, `return await tools.issues.close({})`)).toBe("closed")
})
})