From 8126ca618a3f3a83fadf9afcab8085fac70241c7 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Tue, 2 Jun 2026 19:09:55 +0200 Subject: [PATCH] fix(channels): harden message action schema discovery --- .../plugins/message-action-discovery.ts | 211 +++++++++++++++--- src/channels/plugins/message-actions.test.ts | 184 ++++++++++++++- 2 files changed, 366 insertions(+), 29 deletions(-) diff --git a/src/channels/plugins/message-action-discovery.ts b/src/channels/plugins/message-action-discovery.ts index 2d73a7ced6e3..d2d663d6dbb7 100644 --- a/src/channels/plugins/message-action-discovery.ts +++ b/src/channels/plugins/message-action-discovery.ts @@ -79,11 +79,12 @@ export function createMessageActionDiscoveryContext( function logMessageActionError(params: { pluginId: string; - operation: "describeMessageTool"; + operation: "describeMessageTool" | "readMessageToolDiscovery"; + field?: string; error: unknown; }) { const message = formatErrorMessage(params.error); - const key = `${params.pluginId}:${params.operation}:${message}`; + const key = `${params.pluginId}:${params.operation}:${params.field ?? ""}:${message}`; // Discovery runs while building tool schemas, so log each plugin/error pair // once and let the agent continue with the remaining channel capabilities. if (loggedMessageActionErrors.has(key)) { @@ -91,8 +92,9 @@ function logMessageActionError(params: { } loggedMessageActionErrors.add(key); const stack = params.error instanceof Error && params.error.stack ? params.error.stack : null; + const field = params.field ? `.${params.field}` : ""; defaultRuntime.error?.( - `[message-action-discovery] ${params.pluginId}.actions.${params.operation} failed: ${stack ?? message}`, + `[message-action-discovery] ${params.pluginId}.actions.${params.operation}${field} failed: ${stack ?? message}`, ); } @@ -113,10 +115,30 @@ function describeMessageToolSafely(params: { } } +function readMessageToolDiscoveryValue(params: { + pluginId: string; + field: string; + read: () => T; + fallback: T; +}): T { + try { + return params.read(); + } catch (error) { + logMessageActionError({ + pluginId: params.pluginId, + operation: "readMessageToolDiscovery", + field: params.field, + error, + }); + return params.fallback; + } +} + /** * Normalizes plugin schema contributions into a list for merge callers. */ function normalizeToolSchemaContributions( + pluginId: string, value: | ChannelMessageToolSchemaContribution | ChannelMessageToolSchemaContribution[] @@ -126,7 +148,30 @@ function normalizeToolSchemaContributions( if (!value) { return []; } - return Array.isArray(value) ? value : [value]; + if (!Array.isArray(value)) { + return [value]; + } + const length = readMessageToolDiscoveryValue({ + pluginId, + field: "schema.length", + fallback: 0, + read: () => value.length, + }); + const contributions: ChannelMessageToolSchemaContribution[] = []; + for (let index = 0; index < length; index += 1) { + const contribution = readMessageToolDiscoveryValue< + ChannelMessageToolSchemaContribution | undefined + >({ + pluginId, + field: `schema.${index}`, + fallback: undefined, + read: () => value[index], + }); + if (contribution) { + contributions.push(contribution); + } + } + return contributions; } type ResolvedChannelMessageActionDiscovery = { @@ -161,6 +206,64 @@ function normalizeMessageToolMediaSourceParams( ); } +type SchemaContributionActionsRead = + | { status: "ok"; hasActions: true; actions: unknown } + | { status: "ok"; hasActions: false } + | { status: "unreadable" }; + +function readSchemaContributionVisibility( + pluginId: string, + contribution: ChannelMessageToolSchemaContribution, +): ChannelMessageToolSchemaContribution["visibility"] { + return readMessageToolDiscoveryValue({ + pluginId, + field: "schema.visibility", + fallback: "current-channel", + read: () => contribution.visibility ?? "current-channel", + }); +} + +function readSchemaContributionActions( + pluginId: string, + contribution: ChannelMessageToolSchemaContribution, +): SchemaContributionActionsRead { + const hasActions = readMessageToolDiscoveryValue({ + pluginId, + field: "schema.actions", + fallback: null, + read: () => Object.hasOwn(contribution, "actions"), + }); + if (hasActions === null) { + return { status: "unreadable" }; + } + if (!hasActions) { + return { status: "ok", hasActions: false }; + } + const actions = readMessageToolDiscoveryValue({ + pluginId, + field: "schema.actions", + fallback: null, + read: () => contribution.actions, + }); + return actions === null ? { status: "unreadable" } : { status: "ok", hasActions: true, actions }; +} + +function readSchemaContributionProperties( + pluginId: string, + contribution: ChannelMessageToolSchemaContribution, +): Record | undefined { + return readMessageToolDiscoveryValue({ + pluginId, + field: "schema.properties", + fallback: undefined, + read: () => contribution.properties, + }); +} + +function formatMessageToolDiscoveryField(base: string, key: PropertyKey): string { + return `${base}.${typeof key === "symbol" ? String(key) : key}`; +} + /** * Finds the lightest available message-tool discovery adapter for one channel. */ @@ -225,20 +328,41 @@ export function resolveMessageActionDiscoveryForPlugin(params: { context: params.context, describeMessageTool: adapter.describeMessageTool, }); + const actions = params.includeActions + ? readMessageToolDiscoveryValue({ + pluginId: params.pluginId, + field: "actions", + fallback: [], + read: () => (Array.isArray(described?.actions) ? [...described.actions] : []), + }) + : []; + const capabilities = params.includeCapabilities + ? readMessageToolDiscoveryValue({ + pluginId: params.pluginId, + field: "capabilities", + fallback: [], + read: () => (Array.isArray(described?.capabilities) ? described.capabilities : []), + }) + : []; + const schemaContributions = params.includeSchema + ? readMessageToolDiscoveryValue({ + pluginId: params.pluginId, + field: "schema", + fallback: [], + read: () => normalizeToolSchemaContributions(params.pluginId, described?.schema), + }) + : []; + const mediaSourceParams = readMessageToolDiscoveryValue({ + pluginId: params.pluginId, + field: "mediaSourceParams", + fallback: [], + read: () => normalizeMessageToolMediaSourceParams(described?.mediaSourceParams, params.action), + }); return { - actions: - params.includeActions && Array.isArray(described?.actions) ? [...described.actions] : [], - capabilities: - params.includeCapabilities && Array.isArray(described?.capabilities) - ? described.capabilities - : [], - schemaContributions: params.includeSchema - ? normalizeToolSchemaContributions(described?.schema) - : [], - mediaSourceParams: normalizeMessageToolMediaSourceParams( - described?.mediaSourceParams, - params.action, - ), + actions, + capabilities, + schemaContributions, + mediaSourceParams, }; } @@ -287,13 +411,16 @@ export function listCrossChannelSchemaSupportedMessageActions( for (const contribution of resolved.schemaContributions) { // Current-channel-only schema params are not safe for cross-channel tool // calls unless the plugin explicitly leaves an action without that schema. - if ((contribution.visibility ?? "current-channel") !== "current-channel") { + if ( + readSchemaContributionVisibility(pluginActions.pluginId, contribution) !== "current-channel" + ) { continue; } - if (!Object.hasOwn(contribution, "actions")) { + const actionsRead = readSchemaContributionActions(pluginActions.pluginId, contribution); + if (actionsRead.status === "unreadable" || !actionsRead.hasActions) { return []; } - const actions = contribution.actions; + const { actions } = actionsRead; if (!Array.isArray(actions)) { return []; } @@ -351,12 +478,28 @@ export function listChannelMessageCapabilitiesForChannel( function mergeToolSchemaProperties( target: Record, source: Record | undefined, + pluginId: string, ) { - if (!source) { + if (!source || typeof source !== "object" || Array.isArray(source)) { return; } - for (const [name, schema] of Object.entries(source)) { - if (!(name in target)) { + const keys = readMessageToolDiscoveryValue({ + pluginId, + field: "schema.properties", + fallback: [], + read: () => Reflect.ownKeys(source), + }); + for (const name of keys) { + if (typeof name !== "string" || name in target) { + continue; + } + const schema = readMessageToolDiscoveryValue({ + pluginId, + field: formatMessageToolDiscoveryField("schema.properties", name), + fallback: undefined, + read: () => Reflect.get(source, name), + }); + if (schema) { target[name] = schema; } } @@ -384,14 +527,22 @@ export function resolveChannelMessageToolSchemaProperties( context: discoveryBase, includeSchema: true, }).schemaContributions) { - const visibility = contribution.visibility ?? "current-channel"; + const visibility = readSchemaContributionVisibility(plugin.id, contribution); if (currentChannel) { if (visibility === "all-configured" || plugin.id === currentChannel) { - mergeToolSchemaProperties(properties, contribution.properties); + mergeToolSchemaProperties( + properties, + readSchemaContributionProperties(plugin.id, contribution), + plugin.id, + ); } continue; } - mergeToolSchemaProperties(properties, contribution.properties); + mergeToolSchemaProperties( + properties, + readSchemaContributionProperties(plugin.id, contribution), + plugin.id, + ); } } if (currentChannel && !seenPluginIds.has(currentChannel)) { @@ -405,9 +556,13 @@ export function resolveChannelMessageToolSchemaProperties( context: discoveryBase, includeSchema: true, }).schemaContributions) { - const visibility = contribution.visibility ?? "current-channel"; + const visibility = readSchemaContributionVisibility(currentActions.pluginId, contribution); if (visibility === "all-configured" || currentActions.pluginId === currentChannel) { - mergeToolSchemaProperties(properties, contribution.properties); + mergeToolSchemaProperties( + properties, + readSchemaContributionProperties(currentActions.pluginId, contribution), + currentActions.pluginId, + ); } } } diff --git a/src/channels/plugins/message-actions.test.ts b/src/channels/plugins/message-actions.test.ts index e747607444b0..630141e6c023 100644 --- a/src/channels/plugins/message-actions.test.ts +++ b/src/channels/plugins/message-actions.test.ts @@ -19,7 +19,7 @@ import { resolveChannelMessageToolSchemaProperties, } from "./message-action-discovery.js"; import type { ChannelMessageCapability } from "./message-capabilities.js"; -import type { ChannelPlugin } from "./types.js"; +import type { ChannelMessageToolSchemaContribution, ChannelPlugin } from "./types.js"; const emptyRegistry = createTestRegistry([]); @@ -193,6 +193,141 @@ describe("message action capability checks", () => { ).toHaveProperty("components"); }); + it("skips unreadable schema contributions without dropping healthy fields", () => { + const unreadableSchema = {} as ChannelMessageToolSchemaContribution; + Object.defineProperty(unreadableSchema, "visibility", { + value: "all-configured", + }); + Object.defineProperty(unreadableSchema, "properties", { + get() { + throw new Error("schema properties exploded"); + }, + }); + const schemaPlugin: ChannelPlugin = { + ...createChannelTestPluginBase({ + id: "demo-schema-read", + label: "Demo Schema Read", + capabilities: { chatTypes: ["direct", "group"] }, + config: { + listAccountIds: () => ["default"], + }, + }), + actions: { + describeMessageTool: () => ({ + schema: [ + unreadableSchema, + { + visibility: "all-configured", + properties: { + safeField: Type.Optional(Type.String()), + }, + }, + ], + }), + }, + }; + setActivePluginRegistry( + createTestRegistry([{ pluginId: "demo-schema-read", source: "test", plugin: schemaPlugin }]), + ); + + expect( + resolveChannelMessageToolSchemaProperties({ + cfg: {} as OpenClawConfig, + channel: "demo-schema-read", + }), + ).toHaveProperty("safeField"); + expect(errorSpy).toHaveBeenCalledOnce(); + }); + + it("skips unreadable schema array entries without dropping healthy entries", () => { + const schemaEntries = [ + undefined, + { + visibility: "all-configured", + properties: { + safeField: Type.Optional(Type.String()), + }, + }, + ] as ChannelMessageToolSchemaContribution[]; + Object.defineProperty(schemaEntries, "0", { + enumerable: true, + get() { + throw new Error("schema array entry exploded"); + }, + }); + const schemaPlugin: ChannelPlugin = { + ...createChannelTestPluginBase({ + id: "demo-schema-array-read", + label: "Demo Schema Array Read", + capabilities: { chatTypes: ["direct", "group"] }, + config: { + listAccountIds: () => ["default"], + }, + }), + actions: { + describeMessageTool: () => ({ + schema: schemaEntries, + }), + }, + }; + setActivePluginRegistry( + createTestRegistry([ + { pluginId: "demo-schema-array-read", source: "test", plugin: schemaPlugin }, + ]), + ); + + expect( + resolveChannelMessageToolSchemaProperties({ + cfg: {} as OpenClawConfig, + channel: "demo-schema-array-read", + }), + ).toHaveProperty("safeField"); + expect(errorSpy).toHaveBeenCalledOnce(); + }); + + it("skips unreadable schema property fields without dropping healthy siblings", () => { + const properties = { + safeField: Type.Optional(Type.String()), + } as NonNullable; + Object.defineProperty(properties, "badField", { + enumerable: true, + get() { + throw new Error("schema property field exploded"); + }, + }); + const schemaPlugin: ChannelPlugin = { + ...createChannelTestPluginBase({ + id: "demo-schema-field-read", + label: "Demo Schema Field Read", + capabilities: { chatTypes: ["direct", "group"] }, + config: { + listAccountIds: () => ["default"], + }, + }), + actions: { + describeMessageTool: () => ({ + schema: { + visibility: "all-configured", + properties, + }, + }), + }, + }; + setActivePluginRegistry( + createTestRegistry([ + { pluginId: "demo-schema-field-read", source: "test", plugin: schemaPlugin }, + ]), + ); + + expect( + resolveChannelMessageToolSchemaProperties({ + cfg: {} as OpenClawConfig, + channel: "demo-schema-field-read", + }), + ).toHaveProperty("safeField"); + expect(errorSpy).toHaveBeenCalledOnce(); + }); + it("filters only actions that depend on current-channel-only schema", () => { const scopedSchemaPlugin: ChannelPlugin = { ...createChannelTestPluginBase({ @@ -229,6 +364,53 @@ describe("message action capability checks", () => { ).toEqual(["read", "list-pins"]); }); + it("fails closed when scoped schema action lists are unreadable", () => { + const unreadableSchema = { + visibility: "current-channel", + properties: { + pinnedMessageId: Type.Optional(Type.String()), + }, + } as ChannelMessageToolSchemaContribution; + Object.defineProperty(unreadableSchema, "actions", { + get() { + throw new Error("schema actions exploded"); + }, + }); + const scopedSchemaPlugin: ChannelPlugin = { + ...createChannelTestPluginBase({ + id: "demo-unreadable-scoped-schema", + label: "Demo Unreadable Scoped Schema", + capabilities: { chatTypes: ["direct", "group"] }, + config: { + listAccountIds: () => ["default"], + }, + }), + actions: { + describeMessageTool: () => ({ + actions: ["read", "unpin"], + schema: unreadableSchema, + }), + }, + }; + setActivePluginRegistry( + createTestRegistry([ + { + pluginId: "demo-unreadable-scoped-schema", + source: "test", + plugin: scopedSchemaPlugin, + }, + ]), + ); + + expect( + listCrossChannelSchemaSupportedMessageActions({ + cfg: {} as OpenClawConfig, + channel: "demo-unreadable-scoped-schema", + }), + ).toStrictEqual([]); + expect(errorSpy).toHaveBeenCalledOnce(); + }); + it("keeps unscoped current-channel schema conservative for cross-channel actions", () => { const unscopedSchemaPlugin: ChannelPlugin = { ...createChannelTestPluginBase({