Skip to content

Commit 1c7b5aa

Browse files
vincentkocvincentkoc
committed
fix(sessions): snapshot registered tool definitions
(cherry picked from commit 8c92f6b)
1 parent 815a7c0 commit 1c7b5aa

7 files changed

Lines changed: 495 additions & 26 deletions

File tree

‎src/agents/sessions/agent-session.ts‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,10 @@ import { type BuildSystemPromptOptions, buildSystemPrompt } from "./system-promp
9393
import type { BashOperations } from "./tools/bash-operations.js";
9494
import { createLocalBashOperations } from "./tools/bash.js";
9595
import { createAllToolDefinitions } from "./tools/index.js";
96-
import { createToolDefinitionFromAgentTool } from "./tools/tool-definition-wrapper.js";
96+
import {
97+
createToolDefinitionsFromAgentTools,
98+
snapshotSessionToolDefinitions,
99+
} from "./tools/tool-definition-wrapper.js";
97100

98101
function unwrapCoreResult<T>(result: { ok: true; value: T } | { ok: false; error: Error }): T {
99102
if (result.ok) {
@@ -383,7 +386,7 @@ export class AgentSession {
383386
this.settingsManager = config.settingsManager;
384387
this.scopedModelEntries = config.scopedModels ?? [];
385388
this.sessionResourceLoader = config.resourceLoader;
386-
this.customTools = config.customTools ?? [];
389+
this.customTools = snapshotSessionToolDefinitions(config.customTools ?? []);
387390
this.cwd = config.cwd;
388391
this.sessionModelRegistry = config.modelRegistry;
389392
this.extensionRunnerRef = config.extensionRunnerRef;
@@ -2485,12 +2488,7 @@ export class AgentSession {
24852488
const shellCommandPrefix = this.settingsManager.getShellCommandPrefix();
24862489
const shellPath = this.settingsManager.getShellPath();
24872490
const baseToolDefinitions = this.baseToolsOverride
2488-
? Object.fromEntries(
2489-
Object.entries(this.baseToolsOverride).map(([name, tool]) => [
2490-
name,
2491-
createToolDefinitionFromAgentTool(tool),
2492-
]),
2493-
)
2491+
? createToolDefinitionsFromAgentTools(this.baseToolsOverride)
24942492
: createAllToolDefinitions(this.cwd, {
24952493
read: { autoResizeImages },
24962494
bash: { commandPrefix: shellCommandPrefix, shellPath },
@@ -2521,7 +2519,7 @@ export class AgentSession {
25212519
this.applyExtensionBindings(this.currentExtensionRunner);
25222520

25232521
const defaultActiveToolNames = this.baseToolsOverride
2524-
? Object.keys(this.baseToolsOverride)
2522+
? Array.from(this.baseToolDefinitions.keys())
25252523
: ["read", "bash", "edit", "write"];
25262524
const baseActiveToolNames = options.activeToolNames ?? defaultActiveToolNames;
25272525
this.refreshToolRegistry({

‎src/agents/sessions/extensions/loader.test.ts‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,4 +47,43 @@ export default async function(api) {
4747
expect(result.extensions).toHaveLength(1);
4848
expect(result.extensions[0]?.commands.has("sdk-subpath-probe")).toBe(true);
4949
});
50+
51+
it("skips invalid registered tool schemas without failing extension load", async () => {
52+
const dir = await mkdtemp(join(tmpdir(), "openclaw-extension-tool-schema-"));
53+
tempDirs.push(dir);
54+
const extensionPath = join(dir, "extension.ts");
55+
await writeFile(
56+
extensionPath,
57+
`
58+
export default async function(api) {
59+
api.registerTool({
60+
name: "bad_lookup",
61+
label: "Bad Lookup",
62+
description: "bad",
63+
get parameters() {
64+
throw new Error("revoked schema");
65+
},
66+
async execute() {
67+
return { content: [{ type: "text", text: "bad" }], details: {} };
68+
},
69+
});
70+
api.registerTool({
71+
name: "healthy_lookup",
72+
label: "Healthy Lookup",
73+
description: "healthy",
74+
parameters: { type: "object", properties: {} },
75+
async execute() {
76+
return { content: [{ type: "text", text: "ok" }], details: {} };
77+
},
78+
});
79+
}
80+
`,
81+
);
82+
83+
const result = await loadExtensions([extensionPath], dir);
84+
85+
expect(result.errors).toEqual([]);
86+
expect(result.extensions).toHaveLength(1);
87+
expect(Array.from(result.extensions[0]?.tools.keys() ?? [])).toEqual(["healthy_lookup"]);
88+
});
5089
});

‎src/agents/sessions/extensions/loader.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import type { ExecOptions } from "../exec.js";
2929
import { execCommand } from "../exec.js";
3030
import * as bundledAgentSessions from "../extension-sdk.js";
3131
import { createSyntheticSourceInfo } from "../source-info.js";
32+
import { snapshotSessionToolDefinition } from "../tools/tool-definition-wrapper.js";
3233
import type {
3334
Extension,
3435
ExtensionAPI,
@@ -226,8 +227,12 @@ function createExtensionAPI(
226227

227228
registerTool(tool: ToolDefinition): void {
228229
runtime.assertActive();
229-
extension.tools.set(tool.name, {
230-
definition: tool,
230+
const definition = snapshotSessionToolDefinition(tool);
231+
if (!definition) {
232+
return;
233+
}
234+
extension.tools.set(definition.name, {
235+
definition,
231236
sourceInfo: extension.sourceInfo,
232237
});
233238
runtime.refreshTools();

‎src/agents/sessions/sdk.test.ts‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,46 @@ describe("createAgentSession tool defaults", () => {
121121
expect(session.getActiveToolNames()).toEqual(["custom_lookup"]);
122122
});
123123

124+
it("skips unreadable custom tool schemas while preserving healthy custom tools", async () => {
125+
const badTool = {
126+
name: "bad_lookup",
127+
label: "Bad Lookup",
128+
description: "Bad custom lookup.",
129+
execute: async () => ({
130+
content: [{ type: "text" as const, text: "bad" }],
131+
details: {},
132+
}),
133+
} as ToolDefinition;
134+
Object.defineProperty(badTool, "parameters", {
135+
get: () => {
136+
throw new Error("revoked schema");
137+
},
138+
});
139+
const healthyTool: ToolDefinition = {
140+
name: "custom_lookup",
141+
label: "Custom Lookup",
142+
description: "Looks up a test value.",
143+
parameters: Type.Object({}),
144+
execute: async () => ({
145+
content: [{ type: "text", text: "ok" }],
146+
details: {},
147+
}),
148+
};
149+
150+
const { session } = await createAgentSession({
151+
model: testModel,
152+
noTools: "builtin",
153+
customTools: [badTool, healthyTool],
154+
resourceLoader: createEmptyResourceLoader(),
155+
sessionManager: SessionManager.inMemory(),
156+
settingsManager: SettingsManager.inMemory(),
157+
modelRegistry: ModelRegistry.inMemory(AuthStorage.inMemory()),
158+
});
159+
160+
expect(session.getActiveToolNames()).toEqual(["custom_lookup"]);
161+
expect(session.getAllTools().map((tool) => tool.name)).toEqual(["custom_lookup"]);
162+
});
163+
124164
it("preserves an exact base system prompt when active tools change", async () => {
125165
const customTool: ToolDefinition = {
126166
name: "custom_lookup",

‎src/agents/sessions/sdk.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import {
4040
type ToolName,
4141
withFileMutationQueue,
4242
} from "./tools/index.js";
43+
import { snapshotSessionToolDefinitions } from "./tools/tool-definition-wrapper.js";
4344

4445
export interface CreateAgentSessionOptions {
4546
/** Working directory for project-local discovery. Default: process.cwd() */
@@ -288,7 +289,8 @@ export async function createAgentSession(
288289
}
289290

290291
const defaultActiveToolNames: ToolName[] = ["read", "bash", "edit", "write"];
291-
const customToolNames = options.customTools?.map((tool) => tool.name) ?? [];
292+
const customTools = snapshotSessionToolDefinitions(options.customTools ?? []);
293+
const customToolNames = customTools.map((tool) => tool.name);
292294
const allowedToolNames = options.tools ?? (options.noTools === "all" ? [] : undefined);
293295
const disableBuiltInTools = !options.tools && options.noTools === "builtin";
294296
const initialActiveToolNames: string[] = options.tools
@@ -431,7 +433,7 @@ export async function createAgentSession(
431433
cwd,
432434
scopedModels: options.scopedModels,
433435
resourceLoader,
434-
customTools: options.customTools,
436+
customTools,
435437
modelRegistry,
436438
initialActiveToolNames,
437439
allowedToolNames,

‎src/agents/sessions/tools/tool-definition-wrapper.test.ts‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
import { Type } from "typebox";
2+
import { describe, expect, it } from "vitest";
3+
import type { AgentTool } from "../../runtime/index.js";
4+
import type { ToolDefinition } from "../extensions/types.js";
5+
import {
6+
createToolDefinitionsFromAgentTools,
7+
snapshotSessionToolDefinitions,
8+
wrapToolDefinitions,
9+
} from "./tool-definition-wrapper.js";
10+
11+
function createUnreadableParametersDefinition(name: string): ToolDefinition {
12+
const definition = {
13+
name,
14+
label: name,
15+
description: "bad schema",
16+
execute: async () => ({
17+
content: [{ type: "text" as const, text: "bad" }],
18+
}),
19+
} as ToolDefinition;
20+
Object.defineProperty(definition, "parameters", {
21+
get: () => {
22+
throw new Error("revoked schema");
23+
},
24+
});
25+
return definition;
26+
}
27+
28+
describe("session tool definition wrapper", () => {
29+
it("skips unreadable ToolDefinition schemas while preserving healthy siblings", () => {
30+
const healthy = {
31+
name: "healthy_lookup",
32+
label: "Healthy Lookup",
33+
description: "survives bad siblings",
34+
parameters: Type.Object({ query: Type.String() }),
35+
execute: async () => ({
36+
content: [{ type: "text" as const, text: "ok" }],
37+
}),
38+
} satisfies ToolDefinition;
39+
40+
const tools = wrapToolDefinitions([
41+
createUnreadableParametersDefinition("bad_lookup"),
42+
healthy,
43+
]);
44+
45+
expect(tools.map((tool) => tool.name)).toEqual(["healthy_lookup"]);
46+
});
47+
48+
it("snapshots schemas without stripping TypeBox metadata", () => {
49+
const parameters = Type.Object({ query: Type.String() });
50+
const [snapshot] = snapshotSessionToolDefinitions([
51+
{
52+
name: "search",
53+
label: "Search",
54+
description: "searches",
55+
parameters,
56+
execute: async () => ({
57+
content: [{ type: "text" as const, text: "ok" }],
58+
}),
59+
},
60+
]);
61+
if (!snapshot) {
62+
throw new Error("missing snapshot");
63+
}
64+
(parameters.properties.query as Record<string, unknown>).type = "number";
65+
66+
expect(snapshot.parameters).not.toBe(parameters);
67+
expect(snapshot.parameters).toMatchObject({
68+
type: "object",
69+
properties: { query: { type: "string" } },
70+
});
71+
expect(Object.getOwnPropertyDescriptor(snapshot.parameters, "~kind")).toMatchObject({
72+
value: "Object",
73+
enumerable: false,
74+
});
75+
});
76+
77+
it("keeps tools that intentionally omit a parameter schema", () => {
78+
const [tool] = wrapToolDefinitions([
79+
{
80+
name: "no_args",
81+
label: "No Args",
82+
description: "accepts no arguments",
83+
parameters: undefined as never,
84+
execute: async () => ({
85+
content: [{ type: "text" as const, text: "ok" }],
86+
}),
87+
},
88+
]);
89+
90+
expect(tool?.name).toBe("no_args");
91+
expect(tool?.parameters).toBeUndefined();
92+
});
93+
94+
it("skips unreadable AgentTool schemas while preserving healthy base overrides", () => {
95+
const badTool = {
96+
name: "bad_override",
97+
label: "Bad Override",
98+
description: "bad schema",
99+
execute: async () => ({
100+
content: [{ type: "text" as const, text: "bad" }],
101+
}),
102+
} as AgentTool;
103+
Object.defineProperty(badTool, "parameters", {
104+
get: () => {
105+
throw new Error("revoked schema");
106+
},
107+
});
108+
const healthyTool = {
109+
name: "healthy_override",
110+
label: "Healthy Override",
111+
description: "survives bad overrides",
112+
parameters: Type.Object({ query: Type.String() }),
113+
execute: async () => ({
114+
content: [{ type: "text" as const, text: "ok" }],
115+
}),
116+
} satisfies AgentTool;
117+
118+
const definitions = createToolDefinitionsFromAgentTools({
119+
bad_override: badTool,
120+
healthy_override: healthyTool,
121+
});
122+
123+
expect(Object.keys(definitions)).toEqual(["healthy_override"]);
124+
});
125+
});

0 commit comments

Comments
 (0)