Skip to content

Commit 39705ed

Browse files
vincentkocvincentkoc
committed
fix(doctor): preserve tool quarantine ownership
(cherry picked from commit 27ef876)
1 parent 20205c4 commit 39705ed

4 files changed

Lines changed: 168 additions & 21 deletions

File tree

‎src/agents/tools-effective-inventory-build.ts‎

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -105,12 +105,17 @@ function buildUnsupportedToolSchemaNotices(params: {
105105
diagnostics: readonly RuntimeToolSchemaDiagnostic[];
106106
tools: readonly AnyAgentTool[];
107107
rawToolsByName: ReadonlyMap<string, AnyAgentTool>;
108+
fallbackToolsByIndex?: readonly AnyAgentTool[];
108109
}): EffectiveToolInventoryNotice[] {
109110
return params.diagnostics.map((diagnostic) =>
110111
buildUnsupportedToolSchemaNotice({
111112
diagnostic,
112113
tool: readMatchingTool(params.tools, diagnostic),
113-
fallbackTool: params.rawToolsByName.get(diagnostic.toolName),
114+
fallbackTool:
115+
params.rawToolsByName.get(diagnostic.toolName) ??
116+
(params.fallbackToolsByIndex
117+
? readToolByIndex(params.fallbackToolsByIndex, diagnostic.toolIndex)
118+
: undefined),
114119
}),
115120
);
116121
}
@@ -127,6 +132,17 @@ function readMatchingTool(
127132
}
128133
}
129134

135+
function readToolByIndex(
136+
tools: readonly AnyAgentTool[],
137+
toolIndex: number,
138+
): AnyAgentTool | undefined {
139+
try {
140+
return tools[toolIndex];
141+
} catch {
142+
return undefined;
143+
}
144+
}
145+
130146
// Raw tool arrays can contain getters/proxies from plugin boundaries. Read
131147
// defensively; projection diagnostics handle the exact unreadable entry later.
132148
function buildReadableRawToolsByName(
@@ -223,9 +239,10 @@ export function buildRuntimeCompatibleToolInventory(params: {
223239
} {
224240
const rawToolsByName = buildReadableRawToolsByName(params.tools);
225241
const preNormalizationProjection = filterProviderNormalizableTools(params.tools);
226-
const preNormalizationDiagnostics: RuntimeToolSchemaDiagnostic[] = [
242+
const sourceProjectionDiagnostics: RuntimeToolSchemaDiagnostic[] = [
227243
...preNormalizationProjection.diagnostics,
228244
];
245+
const normalizationDiagnostics: RuntimeToolSchemaDiagnostic[] = [];
229246
const normalizedTools = normalizeAgentRuntimeTools({
230247
// Schema normalization can replace tool definitions, so hand the runtime
231248
// policy a mutable copy while keeping this inventory API readonly.
@@ -237,17 +254,30 @@ export function buildRuntimeCompatibleToolInventory(params: {
237254
modelApi: params.modelApi ?? undefined,
238255
model: params.runtimeModel,
239256
onPreNormalizationSchemaDiagnostics: (diagnostics) =>
240-
preNormalizationDiagnostics.push(...diagnostics),
257+
normalizationDiagnostics.push(...diagnostics),
241258
});
242259
const projection = filterRuntimeCompatibleTools(normalizedTools);
243-
const diagnostics = [...preNormalizationDiagnostics, ...projection.diagnostics];
244260
return {
245261
entries: buildEffectiveToolInventoryEntries(projection.tools, rawToolsByName),
246-
notices: buildUnsupportedToolSchemaNotices({
247-
diagnostics,
248-
tools: normalizedTools,
249-
rawToolsByName,
250-
}),
262+
notices: [
263+
...buildUnsupportedToolSchemaNotices({
264+
diagnostics: sourceProjectionDiagnostics,
265+
tools: params.tools,
266+
rawToolsByName,
267+
fallbackToolsByIndex: params.tools,
268+
}),
269+
...buildUnsupportedToolSchemaNotices({
270+
diagnostics: normalizationDiagnostics,
271+
tools: preNormalizationProjection.tools,
272+
rawToolsByName,
273+
fallbackToolsByIndex: preNormalizationProjection.tools,
274+
}),
275+
...buildUnsupportedToolSchemaNotices({
276+
diagnostics: projection.diagnostics,
277+
tools: normalizedTools,
278+
rawToolsByName,
279+
}),
280+
],
251281
};
252282
}
253283

‎src/agents/tools-effective-inventory.test.ts‎

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ const effectiveInventoryState = vi.hoisted(() => ({
3030
mockTool({ name: "docs_lookup", label: "Docs Lookup", description: "Search docs" }),
3131
] as AnyAgentTool[],
3232
pluginMeta: {} as Record<string, { pluginId: string } | undefined>,
33+
pluginMetaByTool: new WeakMap<object, { pluginId: string }>(),
3334
channelMeta: {} as Record<string, { channelId: string } | undefined>,
3435
effectivePolicy: {} as { profile?: string; providerProfile?: string },
3536
normalizeToolsMock: vi.fn((options: { tools: AnyAgentTool[] }) => options.tools),
@@ -61,7 +62,9 @@ vi.mock("./agent-tools.js", () => ({
6162
}));
6263

6364
vi.mock("../plugins/tools.js", () => ({
64-
getPluginToolMeta: (tool: { name: string }) => effectiveInventoryState.pluginMeta[tool.name],
65+
getPluginToolMeta: (tool: { name: string }) =>
66+
effectiveInventoryState.pluginMetaByTool.get(tool) ??
67+
effectiveInventoryState.pluginMeta[tool.name],
6568
buildPluginToolMetadataKey: (pluginId: string, toolName: string) =>
6669
JSON.stringify([pluginId, toolName]),
6770
}));
@@ -115,6 +118,7 @@ async function loadHarness(options?: {
115118
tools?: AnyAgentTool[];
116119
createToolsMock?: typeof effectiveInventoryState.createToolsMock;
117120
pluginMeta?: Record<string, { pluginId: string } | undefined>;
121+
pluginMetaByTool?: WeakMap<object, { pluginId: string }>;
118122
channelMeta?: Record<string, { channelId: string } | undefined>;
119123
effectivePolicy?: { profile?: string; providerProfile?: string };
120124
normalizeToolsMock?: typeof effectiveInventoryState.normalizeToolsMock;
@@ -124,6 +128,7 @@ async function loadHarness(options?: {
124128
mockTool({ name: "docs_lookup", label: "Docs Lookup", description: "Search docs" }),
125129
];
126130
effectiveInventoryState.pluginMeta = options?.pluginMeta ?? {};
131+
effectiveInventoryState.pluginMetaByTool = options?.pluginMetaByTool ?? new WeakMap();
127132
effectiveInventoryState.channelMeta = options?.channelMeta ?? {};
128133
effectiveInventoryState.effectivePolicy = options?.effectivePolicy ?? {};
129134
effectiveInventoryState.normalizeToolsMock =
@@ -151,6 +156,7 @@ describe("resolveEffectiveToolInventory", () => {
151156
mockTool({ name: "docs_lookup", label: "Docs Lookup", description: "Search docs" }),
152157
];
153158
effectiveInventoryState.pluginMeta = {};
159+
effectiveInventoryState.pluginMetaByTool = new WeakMap();
154160
effectiveInventoryState.channelMeta = {};
155161
effectiveInventoryState.effectivePolicy = {};
156162
effectiveInventoryState.normalizeToolsMock = vi.fn((options) => options.tools);
@@ -380,6 +386,43 @@ describe("resolveEffectiveToolInventory", () => {
380386
]);
381387
});
382388

389+
it("preserves plugin ownership for unreadable pre-normalization tool names", async () => {
390+
const unreadable = mockTool({
391+
name: "fuzzplugin_move_angles",
392+
label: "Fuzzplugin Move Angles",
393+
description: "Move fixture joints",
394+
});
395+
Object.defineProperty(unreadable, "name", {
396+
enumerable: true,
397+
get() {
398+
throw new Error("fuzzplugin name getter exploded");
399+
},
400+
});
401+
const pluginMetaByTool = new WeakMap<object, { pluginId: string }>([
402+
[unreadable, { pluginId: "fuzzplugin" }],
403+
]);
404+
const { resolveEffectiveToolInventory: resolveEffectiveToolInventoryLocal14 } =
405+
await loadHarness({
406+
tools: [
407+
unreadable,
408+
mockTool({ name: "exec", label: "Exec", description: "Run shell commands" }),
409+
],
410+
pluginMetaByTool,
411+
});
412+
413+
const result = resolveEffectiveToolInventoryLocal14({ cfg: {} });
414+
415+
expect(result.groups.flatMap((group) => group.tools.map((tool) => tool.id))).toEqual(["exec"]);
416+
expect(result.notices).toEqual([
417+
{
418+
id: "unsupported-tool-schema:tool[0]",
419+
severity: "warning",
420+
message:
421+
'Tool "tool[0]" from plugin "fuzzplugin" has an unsupported runtime input schema (tool[0].name is unreadable) and was quarantined before model projection. Fix or disable the owner, or remove the tool from active allowlists.',
422+
},
423+
]);
424+
});
425+
383426
it("reports unreadable inventory tool entries without crashing", async () => {
384427
const healthy = mockTool({ name: "exec", label: "Exec", description: "Run shell commands" });
385428
const tools = new Proxy([healthy] as AnyAgentTool[], {

‎src/commands/doctor/shared/active-tool-schema-warnings.test.ts‎

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import type { AnyAgentTool } from "../../../agents/tools/common.js";
66
const toolState = vi.hoisted(() => ({
77
tools: [] as AnyAgentTool[],
88
pluginIds: {} as Record<string, string | undefined>,
9+
pluginIdsByTool: new WeakMap<object, string>(),
910
throwError: null as Error | null,
1011
runtimeModel: null as {
1112
id: string;
@@ -19,7 +20,12 @@ const toolState = vi.hoisted(() => ({
1920
resolveModel: vi.fn(),
2021
createTools: vi.fn<typeof createOpenClawCodingTools>(),
2122
normalizeTools: vi.fn(
22-
(options: { tools: AnyAgentTool[]; modelApi?: string; model?: unknown }) => options.tools,
23+
(options: {
24+
tools: AnyAgentTool[];
25+
modelApi?: string;
26+
model?: unknown;
27+
onPreNormalizationSchemaDiagnostics?: (diagnostics: unknown[], tools: AnyAgentTool[]) => void;
28+
}) => options.tools,
2329
),
2430
}));
2531

@@ -39,7 +45,8 @@ vi.mock("../../../agents/agent-tools.js", () => ({
3945

4046
vi.mock("../../../plugins/tools.js", () => ({
4147
getPluginToolMeta: (toolLocal: { name: string }) => {
42-
const pluginId = toolState.pluginIds[toolLocal.name];
48+
const pluginId =
49+
toolState.pluginIdsByTool.get(toolLocal) ?? toolState.pluginIds[toolLocal.name];
4350
return pluginId ? { pluginId, optional: false } : undefined;
4451
},
4552
}));
@@ -66,6 +73,7 @@ describe("active tool schema doctor warnings", () => {
6673
beforeEach(() => {
6774
toolState.tools = [];
6875
toolState.pluginIds = {};
76+
toolState.pluginIdsByTool = new WeakMap();
6977
toolState.throwError = null;
7078
toolState.runtimeModel = null;
7179
toolState.resolveModelError = null;
@@ -136,6 +144,50 @@ describe("active tool schema doctor warnings", () => {
136144
]);
137145
});
138146

147+
it("preserves plugin ownership for pre-normalization tools with unreadable names", () => {
148+
const unreadable = tool("fuzzplugin_move_angles", {
149+
type: "object",
150+
properties: {},
151+
});
152+
Object.defineProperty(unreadable, "name", {
153+
enumerable: true,
154+
get() {
155+
throw new Error("fuzzplugin name getter exploded");
156+
},
157+
});
158+
const healthy = tool("message", { type: "object", properties: {} });
159+
toolState.tools = [unreadable, healthy];
160+
toolState.pluginIdsByTool.set(unreadable, "fuzzplugin");
161+
toolState.normalizeTools.mockImplementation((options) => {
162+
options.onPreNormalizationSchemaDiagnostics?.(
163+
[
164+
{
165+
toolName: "tool[0]",
166+
toolIndex: 0,
167+
violations: ["tool[0].name is unreadable"],
168+
},
169+
],
170+
options.tools,
171+
);
172+
return [healthy];
173+
});
174+
175+
expect(
176+
collectActiveToolSchemaProjectionWarnings({
177+
cfg: {
178+
plugins: {
179+
entries: {
180+
fuzzplugin: { enabled: true },
181+
},
182+
},
183+
},
184+
env: { HOME: "/tmp/openclaw-test" },
185+
}),
186+
).toEqual([
187+
'- agents.main: active tool "tool[0]" from plugin "fuzzplugin" has unsupported runtime input schema (tool[0].name is unreadable). OpenClaw will quarantine this tool at runtime; fix or disable the plugin, or remove the tool from active allowlists.',
188+
]);
189+
});
190+
139191
it("does not validate disabled plugin mode", () => {
140192
toolState.tools = [
141193
tool("fuzzplugin_move_angles", { type: "array", items: { type: "number" } }),

‎src/commands/doctor/shared/active-tool-schema-warnings.ts‎

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,17 @@ function readPluginId(tool: AnyAgentTool | undefined): string | undefined {
115115
}
116116
}
117117

118+
function resolveDiagnosticPluginId(params: {
119+
tools: readonly AnyAgentTool[];
120+
rawToolsByName: ReadonlyMap<string, AnyAgentTool>;
121+
diagnostic: RuntimeToolSchemaDiagnostic;
122+
}): string | undefined {
123+
return (
124+
readPluginId(readToolByIndex(params.tools, params.diagnostic.toolIndex)) ??
125+
readPluginId(params.rawToolsByName.get(params.diagnostic.toolName))
126+
);
127+
}
128+
118129
/** Collect per-agent warnings for active plugin tools rejected by runtime schema projection. */
119130
export function collectActiveToolSchemaProjectionWarnings(params: {
120131
cfg: OpenClawConfig;
@@ -172,7 +183,10 @@ export function collectActiveToolSchemaProjectionWarnings(params: {
172183
}
173184

174185
const rawToolsByName = buildReadableToolsByName(tools);
175-
const preNormalizationDiagnostics: RuntimeToolSchemaDiagnostic[] = [];
186+
const preNormalizationDiagnostics: {
187+
diagnostic: RuntimeToolSchemaDiagnostic;
188+
sourceTools: readonly AnyAgentTool[];
189+
}[] = [];
176190
let normalizedTools: typeof tools;
177191
try {
178192
normalizedTools = normalizeAgentRuntimeTools({
@@ -184,8 +198,11 @@ export function collectActiveToolSchemaProjectionWarnings(params: {
184198
modelId: modelRef.model,
185199
modelApi: runtimeModelContext.modelApi,
186200
model: runtimeModelContext.model,
187-
onPreNormalizationSchemaDiagnostics: (diagnostics) =>
188-
preNormalizationDiagnostics.push(...diagnostics),
201+
onPreNormalizationSchemaDiagnostics: (diagnostics, sourceTools) => {
202+
for (const diagnostic of diagnostics) {
203+
preNormalizationDiagnostics.push({ diagnostic, sourceTools });
204+
}
205+
},
189206
});
190207
} catch (error) {
191208
warnings.push(
@@ -195,9 +212,12 @@ export function collectActiveToolSchemaProjectionWarnings(params: {
195212
);
196213
continue;
197214
}
198-
for (const diagnostic of preNormalizationDiagnostics) {
199-
const rawTool = rawToolsByName.get(diagnostic.toolName);
200-
const pluginId = readPluginId(rawTool);
215+
for (const { diagnostic, sourceTools } of preNormalizationDiagnostics) {
216+
const pluginId = resolveDiagnosticPluginId({
217+
tools: sourceTools,
218+
rawToolsByName,
219+
diagnostic,
220+
});
201221
warnings.push(
202222
formatDiagnostic({
203223
agentId,
@@ -208,9 +228,11 @@ export function collectActiveToolSchemaProjectionWarnings(params: {
208228
}
209229
const projection = filterRuntimeCompatibleTools(normalizedTools);
210230
for (const diagnostic of projection.diagnostics) {
211-
const tool = readToolByIndex(normalizedTools, diagnostic.toolIndex);
212-
const rawTool = rawToolsByName.get(diagnostic.toolName);
213-
const pluginId = readPluginId(tool) ?? readPluginId(rawTool);
231+
const pluginId = resolveDiagnosticPluginId({
232+
tools: normalizedTools,
233+
rawToolsByName,
234+
diagnostic,
235+
});
214236
warnings.push(
215237
formatDiagnostic({
216238
agentId,

0 commit comments

Comments
 (0)