Skip to content

Commit d21b95e

Browse files
vincentkocvincentkoc
committed
fix(agents): quarantine invalid session tools
1 parent de3ee3d commit d21b95e

3 files changed

Lines changed: 492 additions & 32 deletions

File tree

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

Lines changed: 280 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import type {
3232
TextContent,
3333
} from "../../llm/types.js";
3434
import { isContextOverflow } from "../../llm/utils/overflow.js";
35+
import { createSubsystemLogger } from "../../logging/subsystem.js";
3536
import type {
3637
Agent,
3738
AgentEvent,
@@ -51,6 +52,10 @@ import {
5152
prepareCompaction,
5253
shouldCompact,
5354
} from "../runtime/index.js";
55+
import {
56+
filterRuntimeCompatibleTools,
57+
type RuntimeToolSchemaDiagnostic,
58+
} from "../tool-schema-projection.js";
5459
import { stripFrontmatter } from "../utils/frontmatter.js";
5560
import { sleep } from "../utils/sleep.js";
5661
import { formatNoApiKeyFoundMessage, formatNoModelSelectedMessage } from "./auth-guidance.js";
@@ -95,6 +100,8 @@ import { createLocalBashOperations } from "./tools/bash.js";
95100
import { createAllToolDefinitions } from "./tools/index.js";
96101
import { createToolDefinitionFromAgentTool } from "./tools/tool-definition-wrapper.js";
97102

103+
const log = createSubsystemLogger("agents/sessions");
104+
98105
function unwrapCoreResult<T>(result: { ok: true; value: T } | { ok: false; error: Error }): T {
99106
if (result.ok) {
100107
return result.value;
@@ -122,6 +129,201 @@ function normalizeBranchSummaryResult(
122129
return { error: result.error.message };
123130
}
124131

132+
function readToolDefinitionName(definition: ToolDefinition, fallback: string): string {
133+
try {
134+
return typeof definition.name === "string" && definition.name ? definition.name : fallback;
135+
} catch {
136+
return fallback;
137+
}
138+
}
139+
140+
function readToolDefinitionField(
141+
definition: ToolDefinition,
142+
field: keyof ToolDefinition,
143+
): { ok: true; value: unknown } | { ok: false } {
144+
try {
145+
return { ok: true, value: definition[field] };
146+
} catch {
147+
return { ok: false };
148+
}
149+
}
150+
151+
function toolDefinitionDiagnostic(
152+
toolIndex: number,
153+
toolName: string,
154+
violation: string,
155+
): RuntimeToolSchemaDiagnostic {
156+
return {
157+
toolName,
158+
toolIndex,
159+
violations: [violation],
160+
};
161+
}
162+
163+
function formatToolSchemaDiagnostic(diagnostic: RuntimeToolSchemaDiagnostic): string {
164+
return `${diagnostic.toolName}: ${diagnostic.violations.join(", ")}`;
165+
}
166+
167+
function formatToolSource(sourceInfo: SourceInfo | undefined): string {
168+
return sourceInfo?.path ?? "unknown source";
169+
}
170+
171+
function materializeToolDefinitionEntry(
172+
entry: ToolDefinitionEntry,
173+
toolIndex: number,
174+
): ToolDefinitionRead {
175+
const fallbackName = `tool[${toolIndex}]`;
176+
const nameRead = readToolDefinitionField(entry.definition, "name");
177+
const toolName =
178+
nameRead.ok && typeof nameRead.value === "string" && nameRead.value
179+
? nameRead.value
180+
: fallbackName;
181+
if (!nameRead.ok) {
182+
return {
183+
ok: false,
184+
sourceInfo: entry.sourceInfo,
185+
diagnostic: toolDefinitionDiagnostic(toolIndex, toolName, `${toolName}.name is unreadable`),
186+
};
187+
}
188+
if (typeof nameRead.value !== "string" || !nameRead.value) {
189+
return {
190+
ok: false,
191+
sourceInfo: entry.sourceInfo,
192+
diagnostic: toolDefinitionDiagnostic(
193+
toolIndex,
194+
toolName,
195+
`${toolName}.name must be a non-empty string`,
196+
),
197+
};
198+
}
199+
200+
const labelRead = readToolDefinitionField(entry.definition, "label");
201+
if (!labelRead.ok || typeof labelRead.value !== "string") {
202+
return {
203+
ok: false,
204+
sourceInfo: entry.sourceInfo,
205+
diagnostic: toolDefinitionDiagnostic(
206+
toolIndex,
207+
toolName,
208+
`${toolName}.label must be a string`,
209+
),
210+
};
211+
}
212+
213+
const descriptionRead = readToolDefinitionField(entry.definition, "description");
214+
if (!descriptionRead.ok || typeof descriptionRead.value !== "string") {
215+
return {
216+
ok: false,
217+
sourceInfo: entry.sourceInfo,
218+
diagnostic: toolDefinitionDiagnostic(
219+
toolIndex,
220+
toolName,
221+
`${toolName}.description must be a string`,
222+
),
223+
};
224+
}
225+
226+
const parametersRead = readToolDefinitionField(entry.definition, "parameters");
227+
if (!parametersRead.ok) {
228+
return {
229+
ok: false,
230+
sourceInfo: entry.sourceInfo,
231+
diagnostic: toolDefinitionDiagnostic(
232+
toolIndex,
233+
toolName,
234+
`${toolName}.parameters is unreadable`,
235+
),
236+
};
237+
}
238+
239+
const parameters = parametersRead.value === undefined ? {} : parametersRead.value;
240+
const executeRead = readToolDefinitionField(entry.definition, "execute");
241+
if (!executeRead.ok || typeof executeRead.value !== "function") {
242+
return {
243+
ok: false,
244+
sourceInfo: entry.sourceInfo,
245+
diagnostic: toolDefinitionDiagnostic(
246+
toolIndex,
247+
toolName,
248+
`${toolName}.execute must be a function`,
249+
),
250+
};
251+
}
252+
253+
const definition: ToolDefinition = {
254+
name: nameRead.value,
255+
label: labelRead.value,
256+
description: descriptionRead.value,
257+
parameters: parameters as ToolDefinition["parameters"],
258+
execute: executeRead.value.bind(entry.definition) as ToolDefinition["execute"],
259+
};
260+
261+
const promptSnippetRead = readToolDefinitionField(entry.definition, "promptSnippet");
262+
if (promptSnippetRead.ok && typeof promptSnippetRead.value === "string") {
263+
definition.promptSnippet = promptSnippetRead.value;
264+
}
265+
const promptGuidelinesRead = readToolDefinitionField(entry.definition, "promptGuidelines");
266+
if (
267+
promptGuidelinesRead.ok &&
268+
Array.isArray(promptGuidelinesRead.value) &&
269+
promptGuidelinesRead.value.every(
270+
(guideline): guideline is string => typeof guideline === "string",
271+
)
272+
) {
273+
definition.promptGuidelines = promptGuidelinesRead.value;
274+
}
275+
const renderShellRead = readToolDefinitionField(entry.definition, "renderShell");
276+
if (
277+
renderShellRead.ok &&
278+
(renderShellRead.value === "default" || renderShellRead.value === "self")
279+
) {
280+
definition.renderShell = renderShellRead.value;
281+
}
282+
const prepareArgumentsRead = readToolDefinitionField(entry.definition, "prepareArguments");
283+
if (prepareArgumentsRead.ok && typeof prepareArgumentsRead.value === "function") {
284+
definition.prepareArguments = prepareArgumentsRead.value.bind(
285+
entry.definition,
286+
) as ToolDefinition["prepareArguments"];
287+
}
288+
const executionModeRead = readToolDefinitionField(entry.definition, "executionMode");
289+
if (
290+
executionModeRead.ok &&
291+
(executionModeRead.value === "sequential" || executionModeRead.value === "parallel")
292+
) {
293+
definition.executionMode = executionModeRead.value;
294+
}
295+
const renderCallRead = readToolDefinitionField(entry.definition, "renderCall");
296+
if (renderCallRead.ok && typeof renderCallRead.value === "function") {
297+
definition.renderCall = renderCallRead.value.bind(
298+
entry.definition,
299+
) as ToolDefinition["renderCall"];
300+
}
301+
const renderResultRead = readToolDefinitionField(entry.definition, "renderResult");
302+
if (renderResultRead.ok && typeof renderResultRead.value === "function") {
303+
definition.renderResult = renderResultRead.value.bind(
304+
entry.definition,
305+
) as ToolDefinition["renderResult"];
306+
}
307+
308+
return {
309+
ok: true,
310+
entry: {
311+
definition,
312+
sourceInfo: entry.sourceInfo,
313+
},
314+
};
315+
}
316+
317+
function createToolDefinitionSchemaCandidate(
318+
entry: ToolDefinitionEntry,
319+
): ToolDefinitionSchemaCandidate {
320+
return {
321+
entry,
322+
name: entry.definition.name,
323+
parameters: entry.definition.parameters,
324+
};
325+
}
326+
125327
// ============================================================================
126328
// Skill Block Parsing
127329
// ============================================================================
@@ -283,6 +485,23 @@ interface ToolDefinitionEntry {
283485
sourceInfo: SourceInfo;
284486
}
285487

488+
type ToolDefinitionSchemaCandidate = {
489+
readonly entry: ToolDefinitionEntry;
490+
readonly name: string;
491+
readonly parameters: ToolDefinition["parameters"];
492+
};
493+
494+
type ToolDefinitionRead =
495+
| {
496+
readonly ok: true;
497+
readonly entry: ToolDefinitionEntry;
498+
}
499+
| {
500+
readonly ok: false;
501+
readonly diagnostic: RuntimeToolSchemaDiagnostic;
502+
readonly sourceInfo: SourceInfo;
503+
};
504+
286505
type ActiveToolPromptMetadata = {
287506
validToolNames: string[];
288507
toolSnippets: Record<string, string>;
@@ -2377,6 +2596,48 @@ export class AgentSession {
23772596
);
23782597
}
23792598

2599+
private reportToolSchemaDiagnostics(
2600+
candidates: readonly ToolDefinitionSchemaCandidate[],
2601+
diagnostics: readonly RuntimeToolSchemaDiagnostic[],
2602+
): void {
2603+
for (const diagnostic of diagnostics) {
2604+
const sourceInfo = candidates[diagnostic.toolIndex]?.entry.sourceInfo;
2605+
const source = formatToolSource(sourceInfo);
2606+
const message = `quarantined unsupported session tool schema from ${source}: ${formatToolSchemaDiagnostic(diagnostic)}`;
2607+
log.warn(message);
2608+
this.currentExtensionRunner.emitError({
2609+
extensionPath: source,
2610+
event: "tool_schema",
2611+
error: message,
2612+
});
2613+
}
2614+
}
2615+
2616+
private filterRuntimeCompatibleToolDefinitions(
2617+
entries: readonly ToolDefinitionEntry[],
2618+
): ToolDefinitionEntry[] {
2619+
const reads = entries.map(materializeToolDefinitionEntry);
2620+
const materializedEntries: ToolDefinitionEntry[] = [];
2621+
for (const read of reads) {
2622+
if (read.ok) {
2623+
materializedEntries.push(read.entry);
2624+
continue;
2625+
}
2626+
const message = `quarantined unsupported session tool schema from ${formatToolSource(read.sourceInfo)}: ${formatToolSchemaDiagnostic(read.diagnostic)}`;
2627+
log.warn(message);
2628+
this.currentExtensionRunner.emitError({
2629+
extensionPath: formatToolSource(read.sourceInfo),
2630+
event: "tool_schema",
2631+
error: message,
2632+
});
2633+
}
2634+
2635+
const candidates = materializedEntries.map(createToolDefinitionSchemaCandidate);
2636+
const projection = filterRuntimeCompatibleTools(candidates);
2637+
this.reportToolSchemaDiagnostics(candidates, projection.diagnostics);
2638+
return projection.tools.map((candidate) => candidate.entry);
2639+
}
2640+
23802641
private refreshToolRegistry(options?: {
23812642
activeToolNames?: string[];
23822643
includeAllExtensionTools?: boolean;
@@ -2388,15 +2649,28 @@ export class AgentSession {
23882649
this.disableBuiltInTools && this.baseToolDefinitions.has(name);
23892650
const isAllowedTool = (name: string): boolean =>
23902651
!isDisabledBuiltInToolName(name) && (!allowedToolNames || allowedToolNames.has(name));
2652+
const shouldInspectCustomTool = (tool: ToolDefinitionEntry): boolean => {
2653+
const name = readToolDefinitionName(tool.definition, "");
2654+
if (name) {
2655+
return isAllowedTool(name);
2656+
}
2657+
return !allowedToolNames;
2658+
};
23912659

23922660
const registeredTools = this.currentExtensionRunner.getAllRegisteredTools();
2393-
const allCustomTools = [
2661+
const candidateCustomTools = [
23942662
...registeredTools,
2395-
...this.customTools.map((definition) => ({
2396-
definition,
2397-
sourceInfo: createSyntheticSourceInfo(`<sdk:${definition.name}>`, { source: "sdk" }),
2398-
})),
2399-
].filter((tool) => isAllowedTool(tool.definition.name));
2663+
...this.customTools.map((definition, index) => {
2664+
const name = readToolDefinitionName(definition, `tool[${index}]`);
2665+
return {
2666+
definition,
2667+
sourceInfo: createSyntheticSourceInfo(`<sdk:${name}>`, { source: "sdk" }),
2668+
};
2669+
}),
2670+
];
2671+
const allCustomTools = this.filterRuntimeCompatibleToolDefinitions(
2672+
candidateCustomTools.filter(shouldInspectCustomTool),
2673+
);
24002674
const definitionRegistry = new Map<string, ToolDefinitionEntry>(
24012675
Array.from(this.baseToolDefinitions.entries())
24022676
.filter(([name]) => isAllowedTool(name))

0 commit comments

Comments
 (0)