Skip to content

Commit 815a7c0

Browse files
vincentkocvincentkoc
committed
fix(agents): snapshot session tool definitions
(cherry picked from commit b4cc7c7)
1 parent 5c53918 commit 815a7c0

2 files changed

Lines changed: 242 additions & 7 deletions

File tree

‎src/agents/agent-tool-definition-adapter.test.ts‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,96 @@ async function executeTool(tool: AgentTool, callId: string) {
4848
}
4949

5050
describe("agent tool definition adapter", () => {
51+
it("skips unreadable tool parameters while preserving healthy siblings", () => {
52+
const badTool = {
53+
name: "bad_schema",
54+
label: "Bad Schema",
55+
description: "throws while reading parameters",
56+
execute: async () => ({
57+
content: [{ type: "text" as const, text: "bad" }],
58+
}),
59+
} as AgentTool;
60+
Object.defineProperty(badTool, "parameters", {
61+
get: () => {
62+
throw new Error("revoked schema");
63+
},
64+
});
65+
const healthyTool = {
66+
name: "healthy_schema",
67+
label: "Healthy Schema",
68+
description: "survives a bad sibling",
69+
parameters: { type: "object", properties: { query: { type: "string" } } },
70+
execute: async () => ({
71+
content: [{ type: "text" as const, text: "ok" }],
72+
}),
73+
} satisfies AgentTool;
74+
75+
const defs = toToolDefinitions([badTool, healthyTool]);
76+
77+
expect(defs.map((def) => def.name)).toEqual(["healthy_schema"]);
78+
});
79+
80+
it("skips unreadable tool names before normalizing session definitions", () => {
81+
const badTool = {
82+
label: "Bad Name",
83+
description: "throws while reading name",
84+
parameters: { type: "object", properties: {} },
85+
execute: async () => ({
86+
content: [{ type: "text" as const, text: "bad" }],
87+
}),
88+
} as AgentTool;
89+
Object.defineProperty(badTool, "name", {
90+
get: () => {
91+
throw new Error("revoked name");
92+
},
93+
});
94+
const healthyTool = {
95+
name: "healthy_name",
96+
label: "Healthy Name",
97+
description: "survives a bad sibling",
98+
parameters: { type: "object", properties: {} },
99+
execute: async () => ({
100+
content: [{ type: "text" as const, text: "ok" }],
101+
}),
102+
} satisfies AgentTool;
103+
104+
const defs = toToolDefinitions([badTool, healthyTool]);
105+
106+
expect(defs.map((def) => def.name)).toEqual(["healthy_name"]);
107+
});
108+
109+
it("snapshots tool definition schemas before exposing session definitions", () => {
110+
const parameters = {
111+
type: "object",
112+
properties: {
113+
query: { type: "string" },
114+
},
115+
};
116+
const tool = {
117+
name: "search",
118+
label: "Search",
119+
description: "searches",
120+
parameters,
121+
execute: async () => ({
122+
content: [{ type: "text" as const, text: "ok" }],
123+
}),
124+
} satisfies AgentTool;
125+
126+
const [definition] = toToolDefinitions([tool]);
127+
if (!definition) {
128+
throw new Error("missing tool definition");
129+
}
130+
parameters.properties.query.type = "number";
131+
132+
expect(definition.parameters).toEqual({
133+
type: "object",
134+
properties: {
135+
query: { type: "string" },
136+
},
137+
});
138+
expect(definition.parameters).not.toBe(parameters);
139+
});
140+
51141
it("wraps tool errors into a tool result", async () => {
52142
const result = await executeThrowingTool("boom", "call1");
53143

‎src/agents/agent-tool-definition-adapter.ts‎

Lines changed: 152 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,30 @@ const TOOL_ERROR_PARAM_PREVIEW_MAX_CHARS = 600;
5858
const TOOL_ERROR_EXEC_COMMAND_HASH_CHARS = 16;
5959
const SENSITIVE_EXEC_ENV_VALUE = "[omitted exec env value]";
6060
const EXEC_COMMAND_PARAM_KEYS = new Set(["command", "cmd"]);
61+
const TOOL_DEFINITION_SCHEMA_MAX_DEPTH = 24;
62+
const TOOL_DEFINITION_SCHEMA_MAX_NODES = 1_000;
63+
64+
class InvalidToolDefinitionSchemaError extends Error {
65+
constructor() {
66+
super("parameters schema is not JSON-document-compatible");
67+
this.name = "InvalidToolDefinitionSchemaError";
68+
}
69+
}
70+
71+
type ToolDefinitionSchemaCloneState = {
72+
seen: WeakSet<object>;
73+
nodes: number;
74+
};
75+
76+
type ToolDefinitionSnapshot = {
77+
sourceTool: AnyAgentTool;
78+
name: string;
79+
normalizedName: string;
80+
label: string;
81+
description: string;
82+
parameters: ToolDefinition["parameters"];
83+
beforeHookWrapped: boolean;
84+
};
6185

6286
export type ClientToolCallRecorder =
6387
| ((toolName: string, params: Record<string, unknown>) => void)
@@ -359,15 +383,21 @@ export function toToolDefinitions(
359383
tools: AnyAgentTool[],
360384
hookContext?: HookContext,
361385
): ToolDefinition[] {
362-
return tools.map((tool) => {
363-
const name = tool.name || "tool";
364-
const normalizedName = normalizeToolName(name);
365-
const beforeHookWrapped = isToolWrappedWithBeforeToolCallHook(tool);
386+
return snapshotAgentToolDefinitions(tools).map((toolSnapshot) => {
387+
const {
388+
sourceTool: tool,
389+
name,
390+
normalizedName,
391+
label,
392+
description,
393+
parameters,
394+
beforeHookWrapped,
395+
} = toolSnapshot;
366396
return {
367397
name,
368-
label: tool.label ?? name,
369-
description: tool.description ?? "",
370-
parameters: tool.parameters,
398+
label,
399+
description,
400+
parameters,
371401
execute: async (...args: ToolExecuteArgs): Promise<AgentToolResult<unknown>> => {
372402
const { toolCallId, params, onUpdate, signal } = splitToolExecuteArgs(args);
373403
let executeParams = params;
@@ -454,6 +484,121 @@ export function toToolDefinitions(
454484
});
455485
}
456486

487+
function snapshotAgentToolDefinitions(tools: readonly AnyAgentTool[]): ToolDefinitionSnapshot[] {
488+
const snapshots: ToolDefinitionSnapshot[] = [];
489+
for (const tool of tools) {
490+
const snapshot = snapshotAgentToolDefinition(tool);
491+
if (snapshot) {
492+
snapshots.push(snapshot);
493+
}
494+
}
495+
return snapshots;
496+
}
497+
498+
function snapshotAgentToolDefinition(tool: AnyAgentTool): ToolDefinitionSnapshot | undefined {
499+
let name = "tool";
500+
try {
501+
const rawName = tool.name;
502+
if (typeof rawName === "string" && rawName.length > 0) {
503+
name = rawName;
504+
} else if (rawName != null && rawName !== "") {
505+
throw new Error(`tool name must be a string`);
506+
}
507+
const rawLabel = tool.label;
508+
const rawDescription = tool.description;
509+
const label = typeof rawLabel === "string" && rawLabel.length > 0 ? rawLabel : name;
510+
const description = typeof rawDescription === "string" ? rawDescription : "";
511+
const parameters = snapshotToolDefinitionSchema(tool.parameters);
512+
return {
513+
sourceTool: tool,
514+
name,
515+
normalizedName: normalizeToolName(name),
516+
label,
517+
description,
518+
parameters,
519+
beforeHookWrapped: isToolWrappedWithBeforeToolCallHook(tool),
520+
};
521+
} catch (err) {
522+
logError(
523+
`[tools] skipped invalid tool definition "${name}": ${describeToolDefinitionError(err)}`,
524+
);
525+
return undefined;
526+
}
527+
}
528+
529+
function describeToolDefinitionError(err: unknown): string {
530+
if (err instanceof Error && err.message) {
531+
return err.message;
532+
}
533+
return String(err);
534+
}
535+
536+
function snapshotToolDefinitionSchema(value: unknown): ToolDefinition["parameters"] {
537+
if (value === undefined) {
538+
return undefined as unknown as ToolDefinition["parameters"];
539+
}
540+
return cloneToolDefinitionSchemaValue(
541+
value,
542+
{
543+
seen: new WeakSet<object>(),
544+
nodes: 0,
545+
},
546+
0,
547+
) as ToolDefinition["parameters"];
548+
}
549+
550+
function cloneToolDefinitionSchemaValue(
551+
value: unknown,
552+
state: ToolDefinitionSchemaCloneState,
553+
depth: number,
554+
): unknown {
555+
if (value === null || typeof value === "string" || typeof value === "boolean") {
556+
return value;
557+
}
558+
if (typeof value === "number") {
559+
if (!Number.isFinite(value)) {
560+
throw new InvalidToolDefinitionSchemaError();
561+
}
562+
return value;
563+
}
564+
if (typeof value !== "object") {
565+
throw new InvalidToolDefinitionSchemaError();
566+
}
567+
if (depth > TOOL_DEFINITION_SCHEMA_MAX_DEPTH || state.seen.has(value)) {
568+
throw new InvalidToolDefinitionSchemaError();
569+
}
570+
state.nodes += 1;
571+
if (state.nodes > TOOL_DEFINITION_SCHEMA_MAX_NODES) {
572+
throw new InvalidToolDefinitionSchemaError();
573+
}
574+
state.seen.add(value);
575+
try {
576+
if (Array.isArray(value)) {
577+
return value.map((entry) => cloneToolDefinitionSchemaValue(entry, state, depth + 1));
578+
}
579+
if (!isPlainObject(value)) {
580+
throw new InvalidToolDefinitionSchemaError();
581+
}
582+
const cloned: Record<string, unknown> = {};
583+
for (const key of Object.keys(value)) {
584+
const clonedValue = cloneToolDefinitionSchemaValue(Reflect.get(value, key), state, depth + 1);
585+
if (key === "__proto__") {
586+
Object.defineProperty(cloned, key, {
587+
value: clonedValue,
588+
enumerable: true,
589+
configurable: true,
590+
writable: true,
591+
});
592+
} else {
593+
cloned[key] = clonedValue;
594+
}
595+
}
596+
return cloned;
597+
} finally {
598+
state.seen.delete(value);
599+
}
600+
}
601+
457602
/**
458603
* Coerce tool-call params into a plain object.
459604
*

0 commit comments

Comments
 (0)