Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 45 additions & 4 deletions packages/codemode/src/tool-runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -673,7 +673,42 @@ const namespaceKeys = <R>(tools: HostTools<R>, path: ReadonlyArray<string>): Rea
return Object.keys(value)
}

const resolve = <R>(tools: HostTools<R>, path: ReadonlyArray<string>): HostTool<R> | Definition<R> => {
/** Walks a path to a callable tool, returning undefined instead of throwing when it misses. */
const lookup = <R>(tools: HostTools<R>, path: ReadonlyArray<string>): HostTool<R> | Definition<R> | undefined => {
let value: HostTool<R> | Definition<R> | HostTools<R> = tools
for (const segment of path) {
if (
isBlockedMember(segment) ||
typeof value === "function" ||
isDefinition(value) ||
!Object.hasOwn(value, segment)
)
return undefined
value = value[segment]
}
return typeof value === "function" || isDefinition(value) ? value : undefined
}

type Resolved<R> = { readonly tool: HostTool<R> | Definition<R>; readonly path: ReadonlyArray<string> }

/** Real tools a missed path plausibly meant: its namespace repeated inside the leaf, or the leaf under another namespace. */
const recoveryCandidates = <R>(tools: HostTools<R>, path: ReadonlyArray<string>): Array<Resolved<R>> => {
const leaf = path.at(-1)
const parent = path.at(-2)
if (leaf === undefined || parent === undefined) return []
const guesses: Array<ReadonlyArray<string>> = []
if (leaf.startsWith(`${parent}_`)) guesses.push([...path.slice(0, -1), leaf.slice(parent.length + 1)])
if (path.length === 2) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: When a tool is nested below multiple namespaces, a wrong root namespace still returns UnknownTool even when the leaf has one unique match elsewhere. The path.length === 2 guard limits recovery to root namespace/leaf calls; recursively search the callable tree while preserving the remaining path.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/codemode/src/tool-runtime.ts, line 701:

<comment>When a tool is nested below multiple namespaces, a wrong root namespace still returns `UnknownTool` even when the leaf has one unique match elsewhere. The `path.length === 2` guard limits recovery to root namespace/leaf calls; recursively search the callable tree while preserving the remaining path.</comment>

<file context>
@@ -673,7 +673,42 @@ const namespaceKeys = <R>(tools: HostTools<R>, path: ReadonlyArray<string>): Rea
+  if (leaf === undefined || parent === undefined) return []
+  const guesses: Array<ReadonlyArray<string>> = []
+  if (leaf.startsWith(`${parent}_`)) guesses.push([...path.slice(0, -1), leaf.slice(parent.length + 1)])
+  if (path.length === 2) {
+    for (const key of Object.keys(tools)) if (key !== parent) guesses.push([key, leaf])
+  }
</file context>

for (const key of Object.keys(tools)) if (key !== parent) guesses.push([key, leaf])
}
return guesses.flatMap((guess) => {
const tool = lookup(tools, guess)
return tool === undefined ? [] : [{ tool, path: guess }]
})
}

/** Resolves a written path to its tool, recovering a unique convention miss under the real path. */
const resolve = <R>(tools: HostTools<R>, path: ReadonlyArray<string>): Resolved<R> => {
let value: HostTool<R> | Definition<R> | HostTools<R> = tools

for (const segment of path) {
Expand All @@ -683,7 +718,12 @@ const resolve = <R>(tools: HostTools<R>, path: ReadonlyArray<string>): HostTool<
isDefinition(value) ||
!Object.hasOwn(value, segment)
) {
const candidates = recoveryCandidates(tools, path)
if (candidates.length === 1 && candidates[0] !== undefined) return candidates[0]
throw new ToolRuntimeError("UnknownTool", `Unknown tool '${path.join(".")}'.`, [
...(candidates.length > 1
? [`Did you mean ${candidates.map((candidate) => `tools.${candidate.path.join(".")}`).join(" or ")}?`]
: []),
"Use tools.$codemode.search({ query }) to find available described tools.",
])
}
Expand All @@ -694,7 +734,7 @@ const resolve = <R>(tools: HostTools<R>, path: ReadonlyArray<string>): HostTool<
throw new ToolRuntimeError("UnknownTool", `Tool '${path.join(".")}' is not callable.`)
}

return value
return { tool: value, path }
}

export type ToolRuntime<R = never> = {
Expand Down Expand Up @@ -758,15 +798,16 @@ export const make = <R>(
keys: (path) => namespaceKeys(callableTools, path),
invoke: (path, args) =>
Effect.gen(function* () {
const name = path.join(".")
const { tool, path: resolvedPath } = resolve(callableTools, path)
// The resolved path, not the written one, so telemetry names the tool that actually ran.
const name = resolvedPath.join(".")
const externalArgs = args.map((arg) => copyOut(copyIn(arg, `Arguments for tool '${name}'`)))
const call = { name }
const recordAndObserve = (input: unknown) =>
Effect.sync(() => {
recordCall(call)
return calls.length - 1
}).pipe(Effect.tap((index) => hooks?.onToolCallStart?.({ index, name, input }) ?? Effect.void))
const tool = resolve(callableTools, path)
let describedInput: unknown
if (isDefinition(tool)) {
if (externalArgs.length !== 1)
Expand Down
53 changes: 53 additions & 0 deletions packages/codemode/test/codemode.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1161,3 +1161,56 @@ describe("CodeMode public contract", () => {
expect(() => CodeMode.make({ tools: { $codemode: { lookup } } })).toThrow(/reserved for CodeMode discovery tools/)
})
})

describe("tool name recovery", () => {
const echo = Tool.make({
description: "Echo the value",
input: Schema.Struct({ value: Schema.String }),
output: Schema.String,
run: ({ value }) => Effect.succeed(value),
})
const tools = {
bedrock: { generate_image: echo, bedrock_database_query_records: echo, shopify_products_search: echo },
alpha: { shared_tool: echo },
beta: { shared_tool: echo },
}

test("recovers convention-missed tool names and records the tool that actually ran", async () => {
const calls: Array<string> = []
const runtime = CodeMode.make({
tools,
onToolCallStart: ({ name }) =>
Effect.sync(() => {
calls.push(name)
}),
})

const result = await Effect.runPromise(
runtime.execute(`return [
await tools.bedrock.bedrock_generate_image({ value: "a" }),
await tools.shopify.shopify_products_search({ value: "b" }),
await tools.bedrock.bedrock_database_query_records({ value: "c" }),
]`),
)

expect(result.ok).toBe(true)
if (result.ok) expect(result.value).toStrictEqual(["a", "b", "c"])
expect(calls).toStrictEqual([
"bedrock.generate_image",
"bedrock.shopify_products_search",
"bedrock.bedrock_database_query_records",
])
})

test("refuses to guess between equally plausible tools and names them", async () => {
const result = await Effect.runPromise(
CodeMode.execute({ tools, code: `return await tools.gamma.shared_tool({ value: "x" })` }),
)

expect(result.ok).toBe(false)
if (!result.ok) {
expect(result.error.kind).toBe("UnknownTool")
expect(result.error.suggestions?.join(" ")).toContain("tools.alpha.shared_tool or tools.beta.shared_tool")
}
})
})
Loading