Skip to content

Commit 63f8cce

Browse files
committed
fix(appkit): tool() name optional, execute returns unknown, docs accurate
Address two P0 findings from the PR #306 agentic review. 1. tool() runtime crash on the template `npx @databricks/appkit init` produces a helper agent whose tools call `tool({ description, schema, execute })` with no `name` field (because the record key — `current_time`, `count_words` — already names the tool). The previous shape required `name: string`, so `isFunctionTool` rejected the returned FunctionTool and agent registration failed with "Agent 'helper' tool 'current_time' has an unrecognized shape" — the very first `pnpm dev` after init. Make `name` optional on both ToolConfig and FunctionTool, drop the `typeof obj.name === "string"` check from `isFunctionTool`, and widen `execute` from `Promise<string> | string` to `unknown | Promise<unknown>` since the template's tools naturally return objects (`{ now: ... }`). The agents plugin already overrides `name` with the record key at index-build time, so the original field was functionally redundant in keyed-record contexts. Zod error messages fall back to a generic "tool" label when the name is omitted. Downstream: agents plugin already normalises non-string results via `normalizeToolResult`; DatabricksAdapter already `JSON.stringify`s non-string returns at line 438-439. No further changes needed. 2. Level 1 docs lied about auto-inherit `docs/docs/plugins/agents.md` Level 1 said "auto-inherits every registered ToolProvider plugin's tools" but the Configuration Reference correctly states `autoInheritTools` defaults to `{ file: false, code: false }`. New users following Level 1 hit zero tools and confusion. Rewrite Level 1 narrative to be truthful: "The agent starts with no tools. Tools are opt-in — declare them in frontmatter (Level 2) or opt into auto-inherit explicitly with `agents({ autoInheritTools: { file: true } })`. See 'Auto-inherit posture' below." Same doc still had stale `fromPlugin(...)` examples in Level 3 + the standalone runAgent section. Rewritten to use the `tools(plugins) => ...` function form that v5 actually ships, matching the rest of the codebase. The runAgent section also gets a sentence on the new init contract (eager attachContext + setup, shared cache across sub-agents) and the trust-boundary caveat (no OBO, no approval gate). Tests: +3 covering the new shape — name optional, non-string returns, zod-error fallback label. Updated the "returns false when name is missing" guard test to assert the new "returns true" since the record key wins. Signed-off-by: MarioCadenas <MarioCadenas@users.noreply.github.com>
1 parent b933498 commit 63f8cce

7 files changed

Lines changed: 165 additions & 54 deletions

File tree

docs/docs/api/appkit/Interface.FunctionTool.md

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,12 @@ optional description: string | null;
2828
### execute()
2929

3030
```ts
31-
execute: (args: Record<string, unknown>) => string | Promise<string>;
31+
execute: (args: Record<string, unknown>) => unknown;
3232
```
3333

34+
Returns any shape; downstream `normalizeToolResult` serializes to a
35+
string before handing the value to the LLM.
36+
3437
#### Parameters
3538

3639
| Parameter | Type |
@@ -39,16 +42,22 @@ execute: (args: Record<string, unknown>) => string | Promise<string>;
3942

4043
#### Returns
4144

42-
`string` \| `Promise`\<`string`\>
45+
`unknown`
4346

4447
***
4548

46-
### name
49+
### name?
4750

4851
```ts
49-
name: string;
52+
optional name: string;
5053
```
5154

55+
Optional. When this tool is placed in a keyed record
56+
(`tools: { my_tool: ... }` or the function form), the agents plugin
57+
overrides this with the record key at index-build time. Only set it
58+
explicitly when constructing a `FunctionTool` outside any
59+
keyed-record context.
60+
5261
***
5362

5463
### parameters?

docs/docs/api/appkit/Interface.ToolConfig.md

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,16 @@ optional description: string;
3434
### execute()
3535

3636
```ts
37-
execute: (args: output<S>) => string | Promise<string>;
37+
execute: (args: output<S>) => unknown;
3838
```
3939

40+
Returning a non-string value is fine: the agent runtime serializes
41+
the result via `normalizeToolResult` before handing it to the LLM
42+
(strings pass through; `null` becomes `"null"`; everything else gets
43+
`JSON.stringify`'d; `undefined` becomes `""`). Return whatever shape
44+
is most natural for your tool — typically an object — and let the
45+
runtime handle the wire format.
46+
4047
#### Parameters
4148

4249
| Parameter | Type |
@@ -45,16 +52,23 @@ execute: (args: output<S>) => string | Promise<string>;
4552

4653
#### Returns
4754

48-
`string` \| `Promise`\<`string`\>
55+
`unknown`
4956

5057
***
5158

52-
### name
59+
### name?
5360

5461
```ts
55-
name: string;
62+
optional name: string;
5663
```
5764

65+
Optional. When the tool is placed in a keyed record (the standard
66+
`tools: { my_tool: tool({...}) }` form, or the function form
67+
`tools(plugins) => ({ my_tool: tool({...}) })`), the agents plugin
68+
overrides the tool's LLM-visible name with the record key. Set
69+
`name` explicitly only if you're constructing a `FunctionTool`
70+
outside any keyed-record context — otherwise the record key wins.
71+
5872
***
5973

6074
### schema

docs/docs/plugins/agents.md

Lines changed: 30 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,9 @@ On startup the plugin:
5454
1. Discovers `./config/agents/assistant/agent.md` and registers agent id `assistant`.
5555
2. Parses the YAML frontmatter and markdown body as the agent's `instructions`.
5656
3. Resolves the adapter from `endpoint` (or falls back to `DATABRICKS_AGENT_ENDPOINT`).
57-
4. Auto-inherits every registered ToolProvider plugin's tools (`analytics.*`, `files.*`, …).
58-
5. Mounts the agent at the default name (`assistant`).
57+
4. Mounts the agent at the default name (`assistant`).
58+
59+
The agent starts with **no tools**. Tools are opt-in — declare them in frontmatter (Level 2 below) or opt into auto-inherit explicitly with `agents({ autoInheritTools: { file: true } })`. See "Auto-inherit posture" further down for what that costs and why it's off by default.
5960

6061
Requests land at `POST /invocations` with an OpenAI Responses-compatible body. Every tool call runs through `asUser(req)` so SQL executes as the requesting user, file access respects Unity Catalog ACLs, and telemetry spans are created automatically.
6162

@@ -87,30 +88,23 @@ When any `tools:` is declared the auto-inherit default is turned off — the age
8788
## Level 3: code-defined agents
8889

8990
```ts
90-
import {
91-
agents,
92-
analytics,
93-
createAgent,
94-
createApp,
95-
files,
96-
fromPlugin,
97-
server,
98-
tool,
99-
} from "@databricks/appkit";
91+
import { agents, analytics, createApp, files, server } from "@databricks/appkit";
92+
import { createAgent, tool } from "@databricks/appkit/beta";
10093
import { z } from "zod";
10194

10295
const support = createAgent({
10396
instructions: "You help customers with data and files.",
104-
model: "databricks-claude-sonnet-4-5", // string sugar
105-
tools: {
106-
...fromPlugin(analytics), // all analytics tools
107-
...fromPlugin(files, { only: ["uploads.read"] }), // filtered subset
108-
get_weather: tool({
109-
name: "get_weather",
110-
description: "Weather",
111-
schema: z.object({ city: z.string() }),
112-
execute: async ({ city }) => `Sunny in ${city}`,
113-
}),
97+
model: "databricks-claude-sonnet-4-5", // string sugar
98+
tools(plugins) {
99+
return {
100+
...plugins.analytics.toolkit(), // all analytics tools
101+
...plugins.files.toolkit({ only: ["uploads.read"] }), // filtered subset
102+
get_weather: tool({
103+
description: "Weather",
104+
schema: z.object({ city: z.string() }),
105+
execute: async ({ city }) => `Sunny in ${city}`,
106+
}),
107+
};
114108
},
115109
});
116110

@@ -119,13 +113,15 @@ await createApp({
119113
});
120114
```
121115

122-
Code-defined agents start with no tools by default. `fromPlugin(factory)` is the primary way to pull in a plugin's tools — it returns a spread-friendly marker that the agents plugin resolves against registered `ToolProvider`s at setup time. No intermediate variable, no duplicate `plugins: [analyticsP, filesP, ...]` dance: you write the factory reference once inside `fromPlugin` and again in `plugins: [...]`.
116+
Code-defined agents start with no tools by default. The function form `tools(plugins) => Record<string, AgentTool>` is the primary way to pull in plugin tools: each plugin registered in `createApp({ plugins: [...] })` shows up on the `plugins` parameter, and you call `.toolkit(opts?)` on it to get a spread-friendly record. The runtime invokes the function once at agent setup and caches the result — every plugin is mentioned exactly once (in `createApp`), with no held variables or marker imports.
117+
118+
Inline `tool({...})` calls live in the same record. `name` is optional — the agents plugin overrides it with the record key (`get_weather` above).
123119

124120
The asymmetry (file: auto-inherit, code: strict) matches the personas: prompt authors want zero ceremony, engineers want no surprises.
125121

126122
### Scoping tools in code
127123

128-
`fromPlugin(factory, opts?)` accepts the same `ToolkitOptions` as markdown frontmatter:
124+
`plugins.<name>.toolkit(opts?)` accepts the same `ToolkitOptions` as markdown frontmatter:
129125

130126
| Option | Example | Meaning |
131127
|---|---|---|
@@ -134,7 +130,7 @@ The asymmetry (file: auto-inherit, code: strict) matches the personas: prompt au
134130
| `prefix` | `{ prefix: "" }` | Drop the `${pluginName}.` prefix |
135131
| `rename` | `{ rename: { query: "q" } }` | Remap specific local names |
136132

137-
For plugins that don't expose a `.toolkit()` method (e.g., third-party `ToolProvider` plugins authored with plain `toPlugin`), `fromPlugin` falls back to walking `getAgentTools()` and synthesizing namespaced keys (`${pluginName}.${localName}`). The fallback respects `only` / `except` / `rename` / `prefix` the same way.
133+
For plugins that don't expose a `.toolkit()` method (e.g., third-party `ToolProvider` plugins authored with plain `toPlugin`), the runtime falls back to walking `getAgentTools()` and synthesizing namespaced keys (`${pluginName}.${localName}`). The fallback respects `only` / `except` / `rename` / `prefix` the same way.
138134

139135
If a referenced plugin is not registered in `createApp({ plugins })`, the agents plugin throws at setup with an `Available: …` listing so you can fix the wiring before the first request.
140136

@@ -190,15 +186,18 @@ for (const ticket of tickets) {
190186
}
191187
```
192188

193-
`runAgent` drives the adapter without `createApp` or HTTP. Inline `tool()` calls work standalone as shown above. To use plugin tools in standalone mode, pass the plugin factories through `RunAgentInput.plugins` `runAgent` will resolve any `fromPlugin` markers in the def against that list:
189+
`runAgent` drives the adapter without `createApp` or HTTP. Inline `tool()` calls work standalone as shown above. To use plugin tools in standalone mode, pass the plugin factories through `RunAgentInput.plugins` and reach into them via the `tools(plugins)` function form:
194190

195191
```ts
196-
import { analytics, createAgent, fromPlugin, runAgent } from "@databricks/appkit";
192+
import { analytics } from "@databricks/appkit";
193+
import { createAgent, runAgent } from "@databricks/appkit/beta";
197194

198195
const classifier = createAgent({
199196
instructions: "Classify tickets. Use analytics.query for historical data.",
200197
model: "databricks-claude-sonnet-4-5",
201-
tools: { ...fromPlugin(analytics) },
198+
tools(plugins) {
199+
return { ...plugins.analytics.toolkit() };
200+
},
202201
});
203202

204203
const result = await runAgent(classifier, {
@@ -207,7 +206,9 @@ const result = await runAgent(classifier, {
207206
});
208207
```
209208

210-
Hosted tools (MCP) are still `agents()`-only since they require the live MCP client. Plugin tool dispatch in standalone mode runs as the service principal (no OBO) since there is no HTTP request.
209+
`runAgent` eagerly constructs each plugin in `RunAgentInput.plugins`, runs the standard `attachContext({})` + `await setup()` lifecycle, and shares the instances across the top-level run and every sub-agent dispatch. Plugins whose `setup()` requires `createApp`-only runtime (e.g. `WorkspaceClient`, `ServiceContext`) throw at standalone-init with a clear "use createApp instead" message rather than mid-stream.
210+
211+
Hosted tools (MCP) are still `agents()`-only since they require the live MCP client. Plugin tool dispatch in standalone mode runs as the service principal (no OBO) and **bypasses the agents-plugin approval gate** — treat standalone runAgent as a trusted-prompt environment (CI, batch eval, internal scripts), not as an exposed user-facing surface.
211212

212213
## Configuration reference
213214

packages/appkit/src/core/agent/tests/function-tool.test.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,14 @@ describe("isFunctionTool", () => {
4747
expect(isFunctionTool({ type: "function", name: "x" })).toBe(false);
4848
});
4949

50-
test("returns false when name is missing", () => {
51-
expect(isFunctionTool({ type: "function", execute: () => "y" })).toBe(
52-
false,
53-
);
50+
test("returns true when name is omitted (record key wins downstream)", () => {
51+
// Regression: previously `tool({ description, schema, execute })` (no
52+
// name) produced a FunctionTool whose `name: undefined` failed this
53+
// guard and broke registration with "unrecognized shape". The agents
54+
// plugin always overrides `name` with the record key from
55+
// `tools: { my_tool: tool({...}) }`, so requiring `name` here was
56+
// rejecting valid input.
57+
expect(isFunctionTool({ type: "function", execute: () => "y" })).toBe(true);
5458
});
5559
});
5660

packages/appkit/src/core/agent/tests/tool.test.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,50 @@ describe("tool()", () => {
8686
expect(t.description).toBe("my_tool");
8787
expect(t.parameters).toBeDefined();
8888
});
89+
90+
test("name is optional — agents plugin overrides it with the record key", () => {
91+
// Regression: PR #306 reviewer hit a runtime crash because the
92+
// template wrote `tool({ description, schema, execute })` (no name)
93+
// and the FunctionTool shape guard rejected the result. The agent
94+
// runtime always overrides `name` with the record key in
95+
// `tools: { my_tool: tool({...}) }`, so requiring it here was
96+
// mis-shaping a valid input.
97+
const t = tool({
98+
description: "Returns the current server time",
99+
schema: z.object({}),
100+
execute: () => "2026-05-11T00:00:00Z",
101+
});
102+
103+
expect(t.type).toBe("function");
104+
expect(t.name).toBeUndefined();
105+
expect(t.description).toBe("Returns the current server time");
106+
});
107+
108+
test("execute may return non-string shapes; downstream normalises", async () => {
109+
// Regression: `execute` was typed `Promise<string> | string` but the
110+
// template's tools naturally return objects. The runtime serialises
111+
// via `normalizeToolResult`; tighten typing to `unknown` and verify
112+
// the value flows through.
113+
const t = tool({
114+
name: "now",
115+
schema: z.object({}),
116+
execute: () => ({ now: "2026-05-11T00:00:00Z" }),
117+
});
118+
const result = (await t.execute({})) as { now: string };
119+
expect(result).toEqual({ now: "2026-05-11T00:00:00Z" });
120+
});
121+
122+
test("zod-error message uses a generic label when name is omitted", async () => {
123+
const t = tool({
124+
description: "needs a city",
125+
schema: z.object({ city: z.string() }),
126+
execute: () => "ok",
127+
});
128+
const result = await t.execute({});
129+
expect(typeof result).toBe("string");
130+
expect(result).toContain("Invalid arguments for tool");
131+
expect(result).toContain("city");
132+
});
89133
});
90134

91135
describe("formatZodError", () => {

packages/appkit/src/core/agent/tools/function-tool.ts

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,14 @@ import type { AgentToolDefinition, ToolAnnotations } from "shared";
22

33
export interface FunctionTool {
44
type: "function";
5-
name: string;
5+
/**
6+
* Optional. When this tool is placed in a keyed record
7+
* (`tools: { my_tool: ... }` or the function form), the agents plugin
8+
* overrides this with the record key at index-build time. Only set it
9+
* explicitly when constructing a `FunctionTool` outside any
10+
* keyed-record context.
11+
*/
12+
name?: string;
613
description?: string | null;
714
parameters?: Record<string, unknown> | null;
815
strict?: boolean | null;
@@ -16,25 +23,34 @@ export interface FunctionTool {
1623
* tool indexes.
1724
*/
1825
annotations?: ToolAnnotations;
19-
execute: (args: Record<string, unknown>) => Promise<string> | string;
26+
/**
27+
* Returns any shape; downstream `normalizeToolResult` serializes to a
28+
* string before handing the value to the LLM.
29+
*/
30+
execute: (args: Record<string, unknown>) => unknown | Promise<unknown>;
2031
}
2132

2233
export function isFunctionTool(value: unknown): value is FunctionTool {
2334
if (typeof value !== "object" || value === null) return false;
2435
const obj = value as Record<string, unknown>;
25-
return (
26-
obj.type === "function" &&
27-
typeof obj.name === "string" &&
28-
typeof obj.execute === "function"
29-
);
36+
// `name` is intentionally not required: the agents plugin overrides it
37+
// with the record key (`tools: { my_tool: tool({...}) }` -> "my_tool")
38+
// so requiring it on the FunctionTool shape rejects perfectly-valid
39+
// `tool({ description, schema, execute })` calls that omit the name.
40+
return obj.type === "function" && typeof obj.execute === "function";
3041
}
3142

3243
export function functionToolToDefinition(
3344
tool: FunctionTool,
3445
): AgentToolDefinition {
46+
// `name` is guaranteed to be overridden downstream by the record key
47+
// when the tool is registered through `AgentDefinition.tools`. Falling
48+
// back to an empty string here keeps the type honest without
49+
// surfacing a sentinel that could leak into a non-record context.
50+
const name = tool.name ?? "";
3551
return {
36-
name: tool.name,
37-
description: tool.description ?? tool.name,
52+
name,
53+
description: tool.description ?? name,
3854
parameters: (tool.parameters as AgentToolDefinition["parameters"]) ?? {
3955
type: "object",
4056
properties: {},

packages/appkit/src/core/agent/tools/tool.ts

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,15 @@ import type { FunctionTool } from "./function-tool";
44
import { toToolJSONSchema } from "./json-schema";
55

66
export interface ToolConfig<S extends z.ZodType> {
7-
name: string;
7+
/**
8+
* Optional. When the tool is placed in a keyed record (the standard
9+
* `tools: { my_tool: tool({...}) }` form, or the function form
10+
* `tools(plugins) => ({ my_tool: tool({...}) })`), the agents plugin
11+
* overrides the tool's LLM-visible name with the record key. Set
12+
* `name` explicitly only if you're constructing a `FunctionTool`
13+
* outside any keyed-record context — otherwise the record key wins.
14+
*/
15+
name?: string;
816
description?: string;
917
schema: S;
1018
/**
@@ -16,7 +24,15 @@ export interface ToolConfig<S extends z.ZodType> {
1624
* added this field.
1725
*/
1826
annotations?: ToolAnnotations;
19-
execute: (args: z.infer<S>) => Promise<string> | string;
27+
/**
28+
* Returning a non-string value is fine: the agent runtime serializes
29+
* the result via `normalizeToolResult` before handing it to the LLM
30+
* (strings pass through; `null` becomes `"null"`; everything else gets
31+
* `JSON.stringify`'d; `undefined` becomes `""`). Return whatever shape
32+
* is most natural for your tool — typically an object — and let the
33+
* runtime handle the wire format.
34+
*/
35+
execute: (args: z.infer<S>) => unknown | Promise<unknown>;
2036
}
2137

2238
/**
@@ -34,16 +50,23 @@ export function tool<S extends z.ZodType>(config: ToolConfig<S>): FunctionTool {
3450
unknown
3551
>;
3652

53+
// `name` is only used for the zod-validation error message and the
54+
// FunctionTool's `name` field; the agents plugin overrides the latter
55+
// with the record key (`tools: { my_tool: ... }` -> "my_tool") at
56+
// index-build time. Fall back to a generic label so errors are still
57+
// legible when `name` is omitted.
58+
const labelForErrors = config.name ?? "tool";
59+
3760
return {
3861
type: "function",
39-
name: config.name,
62+
...(config.name !== undefined ? { name: config.name } : {}),
4063
description: config.description ?? config.name,
4164
parameters,
4265
...(config.annotations ? { annotations: config.annotations } : {}),
4366
execute: async (args: Record<string, unknown>) => {
4467
const parsed = config.schema.safeParse(args);
4568
if (!parsed.success) {
46-
return formatZodError(parsed.error, config.name);
69+
return formatZodError(parsed.error, labelForErrors);
4770
}
4871
return config.execute(parsed.data as z.infer<S>);
4972
},

0 commit comments

Comments
 (0)