From 22334b94c893ef81e0ac8e24ccc2a8f498d86e4b Mon Sep 17 00:00:00 2001 From: Aiden Cline <63023139+rekram1-node@users.noreply.github.com> Date: Wed, 15 Jul 2026 11:09:09 -0500 Subject: [PATCH] fix(codemode): canonicalize dotted tool paths (#36994) --- packages/codemode/README.md | 4 + packages/codemode/interpreter-support.md | 10 +- packages/codemode/src/interpreter/runtime.ts | 4 +- packages/codemode/src/tool-runtime.ts | 94 ++++++----- packages/codemode/src/tool.ts | 7 +- packages/codemode/test/enumeration.test.ts | 3 +- packages/codemode/test/tool-paths.test.ts | 164 +++++++++++++++++++ 7 files changed, 242 insertions(+), 44 deletions(-) create mode 100644 packages/codemode/test/tool-paths.test.ts diff --git a/packages/codemode/README.md b/packages/codemode/README.md index 22af0b701b..51634d91ba 100644 --- a/packages/codemode/README.md +++ b/packages/codemode/README.md @@ -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`. 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({ ...options, code })` runs once and is equivalent to `CodeMode.make(options).execute(code)`. A diff --git a/packages/codemode/interpreter-support.md b/packages/codemode/interpreter-support.md index f32f6c258e..a950eaa72f 100644 --- a/packages/codemode/interpreter-support.md +++ b/packages/codemode/interpreter-support.md @@ -168,7 +168,8 @@ ultimate source of truth. - [ ] `Object.is`; runtime and tool-reference identity semantics need to be defined first. - [ ] `Object.groupBy`. - [ ] 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 @@ -312,7 +313,12 @@ ultimate source of truth. 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`. -- [ ] 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 `null` in render-only or OpenAPI tool calls. - [ ] Make regular-expression execution genuinely timeout-safe, or narrow the timeout guarantee explicitly. diff --git a/packages/codemode/src/interpreter/runtime.ts b/packages/codemode/src/interpreter/runtime.ts index 68fa83aa1b..b9ed33fd61 100644 --- a/packages/codemode/src/interpreter/runtime.ts +++ b/packages/codemode/src/interpreter/runtime.ts @@ -1759,8 +1759,8 @@ export class Interpreter { : self.toPropertyKey(yield* self.evaluateExpression(propertyNode), propertyNode) if (objectValue instanceof ToolReference) { - if (typeof key !== "string" || isBlockedMember(key)) { - throw new InterpreterRuntimeError("Tool paths must use safe string property names.", propertyNode) + if (typeof key !== "string") { + throw new InterpreterRuntimeError("Tool paths must use string property names.", propertyNode) } return new ToolReference([...objectValue.path, key]) } diff --git a/packages/codemode/src/tool-runtime.ts b/packages/codemode/src/tool-runtime.ts index d4ed4ea221..e4e365cb4e 100644 --- a/packages/codemode/src/tool-runtime.ts +++ b/packages/codemode/src/tool-runtime.ts @@ -274,15 +274,41 @@ export const copyOut = (value: unknown, undefinedAsNull = false): unknown => { return value } +// Dots in tool names are namespace separators; the last definition for a canonical path wins. +type ToolNode = { + definition?: Definition + readonly children: Map> +} + +const toolTrie = (tools: Tools): ToolNode => { + const root: ToolNode = { children: new Map() } + const insert = (node: ToolNode, group: Tools): 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): ReadonlyArray => + path.flatMap((segment) => segment.split(".")) + const definitions = ( - tools: Tools, + node: ToolNode, path: ReadonlyArray = [], -): Array<{ path: string; definition: Definition }> => - Object.entries(tools).flatMap(([name, value]) => { - const next = [...path, name] - if (isDefinition(value)) return [{ path: next.join("."), definition: value }] - return definitions(value, next) - }) +): Array<{ path: string; definition: Definition }> => [ + ...(node.definition === undefined ? [] : [{ path: path.join("."), definition: node.definition }]), + ...Array.from(node.children, ([name, child]) => definitions(child, [...path, name])).flat(), +] const describeDefinition = (path: string, definition: Definition): ToolDescription => ({ path, @@ -291,7 +317,7 @@ const describeDefinition = (path: string, definition: Definition): ToolDes }) const visibleDefinitions = (tools: Tools) => - definitions(tools).map(({ path, definition }) => ({ + definitions(toolTrie(tools)).map(({ path, definition }) => ({ path, definition, description: describeDefinition(path, definition), @@ -555,37 +581,30 @@ export const prepare = (tools: Tools, catalogBudget = defaultCatalogBudget } } -const namespaceKeys = (tools: Tools, path: ReadonlyArray): ReadonlyArray => { - let value: Definition | Tools = tools - for (const segment of path) { - if (isBlockedMember(segment) || isDefinition(value) || !Object.hasOwn(value, segment)) { - throw new ToolRuntimeError("UnknownTool", `Unknown tool namespace '${path.join(".")}'.`, [ - "Object.keys(tools) lists the available namespaces; search({ query }) finds described tools.", - ]) - } - value = value[segment] as Definition | Tools +const lookup = (root: ToolNode, segments: ReadonlyArray): ToolNode | undefined => + segments.reduce | undefined>((node, segment) => node?.children.get(segment), root) + +const namespaceKeys = (root: ToolNode, path: ReadonlyArray): ReadonlyArray => { + const segments = canonicalSegments(path) + const node = lookup(root, segments) + if (node === undefined) { + throw new ToolRuntimeError("UnknownTool", `Unknown tool namespace '${segments.join(".")}'.`) } - if (isDefinition(value)) return [] - return Object.keys(value) + return Array.from(node.children.keys()) } -const resolve = (tools: Tools, path: ReadonlyArray): Definition => { - let value: Definition | Tools = tools - - for (const segment of path) { - if (isBlockedMember(segment) || isDefinition(value) || !Object.hasOwn(value, segment)) { - throw new ToolRuntimeError("UnknownTool", `Unknown tool '${path.join(".")}'.`, [ - "Use search({ query }) to find available described tools.", - ]) - } - value = value[segment] as Definition | Tools +const resolve = (root: ToolNode, path: ReadonlyArray): Definition => { + const segments = canonicalSegments(path) + const node = lookup(root, segments) + if (node === undefined) { + throw new ToolRuntimeError("UnknownTool", `Unknown tool '${segments.join(".")}'.`, [ + "Use search({ query }) to find available described tools.", + ]) } - - if (!isDefinition(value)) { - throw new ToolRuntimeError("UnknownTool", `Tool '${path.join(".")}' is not callable.`) + if (node.definition === undefined) { + throw new ToolRuntimeError("UnknownTool", `Tool '${segments.join(".")}' is not callable.`) } - - return value + return node.definition } export type ToolRuntime = { @@ -603,6 +622,7 @@ export const make = ( hooks?: ToolCallHooks, ): ToolRuntime => { const calls: Array = [] + const root = toolTrie(tools) const searchTool = makeSearchTool(searchIndex) // End hooks observe settled success or failure; interruption emits neither outcome. @@ -670,7 +690,7 @@ export const make = ( return { root: new ToolReference([]), calls, - keys: (path) => namespaceKeys(tools, path), + keys: (path) => namespaceKeys(root, path), search: (args) => Effect.suspend(() => invokeDefinition( @@ -681,9 +701,9 @@ export const make = ( ), invoke: (path, args) => 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 tool = resolve(tools, path) + const tool = resolve(root, path) return yield* invokeDefinition(name, tool, externalArgs) }), } diff --git a/packages/codemode/src/tool.ts b/packages/codemode/src/tool.ts index 4d07a6398c..e75fa7ba26 100644 --- a/packages/codemode/src/tool.ts +++ b/packages/codemode/src/tool.ts @@ -50,8 +50,13 @@ export type Options) => Effect.Effect, unknown, R> } +// Object.hasOwn: an inherited _tag must not classify a namespace as a Definition. export const isDefinition = (value: unknown): value is Definition => - 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.*`. diff --git a/packages/codemode/test/enumeration.test.ts b/packages/codemode/test/enumeration.test.ts index 0cd81e9c7e..116fbab0d9 100644 --- a/packages/codemode/test/enumeration.test.ts +++ b/packages/codemode/test/enumeration.test.ts @@ -56,11 +56,10 @@ describe("Object.keys over tool references", () => { 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)`) expect(failure.kind).toBe("UnknownTool") 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 () => { diff --git a/packages/codemode/test/tool-paths.test.ts b/packages/codemode/test/tool-paths.test.ts new file mode 100644 index 0000000000..c92739c5df --- /dev/null +++ b/packages/codemode/test/tool-paths.test.ts @@ -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") + }) +})