diff --git a/CHANGELOG.md b/CHANGELOG.md index 7260d01dcfb..3f3071986ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -141,6 +141,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- fix(auto-reply): gate inline skill tool dispatch [AI]. (#78517) Thanks @pgondhi987. - Canvas plugin: keep legacy root `canvasHost` configs valid until `openclaw doctor --fix` migrates them into `plugins.entries.canvas.config.host`, move Canvas/A2UI clients to gateway protocol v4 plugin surfaces, and refresh the generated A2UI bundle hash so normal builds stay clean. - feishu: honor config write policy for dynamic agents [AI]. (#78520) Thanks @pgondhi987. - fix(skill-workshop): honor pending approval for tool suggestions [AI]. (#78516) Thanks @pgondhi987. diff --git a/src/agents/openclaw-tools.ts b/src/agents/openclaw-tools.ts index 7d133a2a643..add96458449 100644 --- a/src/agents/openclaw-tools.ts +++ b/src/agents/openclaw-tools.ts @@ -22,9 +22,15 @@ import { collectPresentOpenClawTools, isUpdatePlanToolEnabledForOpenClawTools, } from "./openclaw-tools.registration.js"; +import { + type HookContext, + isToolWrappedWithBeforeToolCallHook, + wrapToolWithBeforeToolCallHook, +} from "./pi-tools.before-tool-call.js"; import type { SandboxFsBridge } from "./sandbox/fs-bridge.js"; import type { SpawnedToolContext } from "./spawned-context.js"; import type { ToolFsPolicy } from "./tool-fs-policy.js"; +import { resolveToolLoopDetectionConfig } from "./tool-loop-detection-config.js"; import { createAgentsListTool } from "./tools/agents-list-tool.js"; import type { AnyAgentTool } from "./tools/common.js"; import { createCronTool } from "./tools/cron-tool.js"; @@ -117,6 +123,14 @@ export function createOpenClawTools( enableHeartbeatTool?: boolean; /** If true, skip plugin tool resolution and return only shipped core tools. */ disablePluginTools?: boolean; + /** + * Wrap returned tools with the before_tool_call hook at construction time. + * Defaults to true; callers that already enforce the hook at a later shared + * boundary should opt out explicitly. + */ + wrapBeforeToolCallHook?: boolean; + /** Override or extend the default hook context used by construction-time wrapping. */ + beforeToolCallHookContext?: HookContext; /** Records hot-path tool-prep stages for reply startup diagnostics. */ recordToolPrepStage?: (name: string) => void; /** Trusted sender id from inbound context (not tool args). */ @@ -413,23 +427,45 @@ export function createOpenClawTools( ...collectPresentOpenClawTools([webSearchTool, webFetchTool, imageTool, pdfTool]), ]; options?.recordToolPrepStage?.("openclaw-tools:core-tool-list"); - - if (options?.disablePluginTools) { - return tools; + let allTools = tools; + if (!options?.disablePluginTools) { + const existingToolNames = new Set(); + for (const tool of tools) { + existingToolNames.add(tool.name); + } + allTools = [ + ...tools, + ...resolveOpenClawPluginToolsForOptions({ + options, + resolvedConfig, + existingToolNames, + }), + ]; + options?.recordToolPrepStage?.("openclaw-tools:plugin-tools"); } - const existingToolNames = new Set(); - for (const tool of tools) { - existingToolNames.add(tool.name); + if (options?.wrapBeforeToolCallHook === false) { + return allTools; } - const wrappedPluginTools = resolveOpenClawPluginToolsForOptions({ - options, - resolvedConfig, - existingToolNames, - }); - options?.recordToolPrepStage?.("openclaw-tools:plugin-tools"); - - return [...tools, ...wrappedPluginTools]; + const hookAgentId = options?.requesterAgentIdOverride ?? sessionAgentId; + const defaultHookContext: HookContext = { + ...(hookAgentId ? { agentId: hookAgentId } : {}), + ...(resolvedConfig ? { config: resolvedConfig } : {}), + ...(options?.agentSessionKey ? { sessionKey: options.agentSessionKey } : {}), + ...(options?.sessionId ? { sessionId: options.sessionId } : {}), + ...(options?.currentChannelId ? { channelId: options.currentChannelId } : {}), + loopDetection: resolveToolLoopDetectionConfig({ cfg: resolvedConfig, agentId: hookAgentId }), + }; + const hookContext = { + ...defaultHookContext, + ...options?.beforeToolCallHookContext, + }; + options?.recordToolPrepStage?.("openclaw-tools:tool-hooks"); + return allTools.map((tool) => + isToolWrappedWithBeforeToolCallHook(tool) + ? tool + : wrapToolWithBeforeToolCallHook(tool, hookContext), + ); } export const __testing = { diff --git a/src/agents/openclaw-tools.update-plan.test.ts b/src/agents/openclaw-tools.update-plan.test.ts index 2b3a6b43d00..e6d8c8bddc3 100644 --- a/src/agents/openclaw-tools.update-plan.test.ts +++ b/src/agents/openclaw-tools.update-plan.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import type { OpenClawConfig } from "../config/config.js"; import { createOpenClawTools } from "./openclaw-tools.js"; import { isUpdatePlanToolEnabledForOpenClawTools } from "./openclaw-tools.registration.js"; +import { isToolWrappedWithBeforeToolCallHook } from "./pi-tools.before-tool-call.js"; import { createUpdatePlanTool } from "./tools/update-plan-tool.js"; type UpdatePlanGatingParams = Parameters[0]; @@ -51,6 +52,27 @@ describe("openclaw-tools update_plan gating", () => { expect(emptyAllowlistTools.some((tool) => tool.name === "update_plan")).toBe(false); }); + it("wraps constructed tools with before-tool-call hooks by default", () => { + const tools = createOpenClawTools({ + config: {} as OpenClawConfig, + disablePluginTools: true, + }); + const unwrappedTools = createOpenClawTools({ + config: {} as OpenClawConfig, + disablePluginTools: true, + wrapBeforeToolCallHook: false, + }); + + expect( + isToolWrappedWithBeforeToolCallHook(tools.find((tool) => tool.name === "sessions_list")!), + ).toBe(true); + expect( + isToolWrappedWithBeforeToolCallHook( + unwrappedTools.find((tool) => tool.name === "sessions_list")!, + ), + ).toBe(false); + }); + it("registers update_plan when explicitly enabled", () => { const config = { tools: { diff --git a/src/agents/pi-tools.ts b/src/agents/pi-tools.ts index 445444e1a4a..96453e50d54 100644 --- a/src/agents/pi-tools.ts +++ b/src/agents/pi-tools.ts @@ -746,6 +746,7 @@ export function createOpenClawCodingTools(options?: { disableMessageTool: options?.disableMessageTool, enableHeartbeatTool, disablePluginTools: !includePluginTools, + wrapBeforeToolCallHook: false, ...(cronSelfRemoveOnlyJobId ? { cronSelfRemoveOnlyJobId } : {}), requesterAgentIdOverride: agentId, requesterSenderId: options?.senderId, diff --git a/src/auto-reply/reply/get-reply-inline-actions.skip-when-config-empty.test.ts b/src/auto-reply/reply/get-reply-inline-actions.skip-when-config-empty.test.ts index b16d1a39f95..d08034c5944 100644 --- a/src/auto-reply/reply/get-reply-inline-actions.skip-when-config-empty.test.ts +++ b/src/auto-reply/reply/get-reply-inline-actions.skip-when-config-empty.test.ts @@ -687,6 +687,112 @@ describe("handleInlineActions", () => { senderIsOwner: true, }), ); - expect(toolExecute).toHaveBeenCalled(); + expect(toolExecute).toHaveBeenCalledWith( + expect.stringMatching(/^cmd_/), + { + command: "display name", + commandName: "set_profile", + skillName: "matrix-profile", + }, + undefined, + ); + }); + + it("honors construction-time before-tool-call blocks for inline tool dispatch", async () => { + const typing = createTypingController(); + const abortController = new AbortController(); + const toolExecute = vi.fn(async () => ({ + content: [{ type: "text", text: "denied by policy" }], + details: { + status: "blocked", + deniedReason: "plugin-before-tool-call", + reason: "denied by policy", + }, + })); + createOpenClawToolsMock.mockReturnValue([ + { + name: "message", + execute: toolExecute, + }, + ]); + + const ctx = buildTestCtx({ + Body: "/set_profile display name", + CommandBody: "/set_profile display name", + }); + const skillCommands: SkillCommandSpec[] = [ + { + name: "set_profile", + skillName: "matrix-profile", + description: "Set Matrix profile", + dispatch: { + kind: "tool", + toolName: "message", + argMode: "raw", + }, + sourceFilePath: "/tmp/plugin/commands/set-profile.md", + }, + ]; + + const result = await handleInlineActions( + createHandleInlineActionsInput({ + ctx, + typing, + cleanedBody: "/set_profile display name", + command: { + isAuthorizedSender: true, + senderId: "sender-1", + senderIsOwner: true, + abortKey: "sender-1", + rawBodyNormalized: "/set_profile display name", + commandBodyNormalized: "/set_profile display name", + }, + overrides: { + cfg: { + commands: { text: true }, + tools: { + loopDetection: { + enabled: true, + }, + }, + }, + agentId: "main", + allowTextCommands: true, + opts: { abortSignal: abortController.signal }, + skillCommands, + sessionEntry: { + sessionId: "wrapper-session", + updatedAt: 0, + }, + sessionStore: { + "s:main": { + sessionId: "target-session", + updatedAt: 0, + }, + }, + }, + }), + ); + + expect(result).toEqual({ + kind: "reply", + reply: { text: "❌ Tool call blocked: denied by policy" }, + }); + expect(createOpenClawToolsMock).toHaveBeenCalledWith( + expect.objectContaining({ + sessionId: "target-session", + currentChannelId: "whatsapp", + }), + ); + expect(toolExecute).toHaveBeenCalledWith( + expect.stringMatching(/^cmd_/), + { + command: "display name", + commandName: "set_profile", + skillName: "matrix-profile", + }, + abortController.signal, + ); + expect(typing.cleanup).toHaveBeenCalled(); }); }); diff --git a/src/auto-reply/reply/get-reply-inline-actions.ts b/src/auto-reply/reply/get-reply-inline-actions.ts index 4864a749616..900cf09b89c 100644 --- a/src/auto-reply/reply/get-reply-inline-actions.ts +++ b/src/auto-reply/reply/get-reply-inline-actions.ts @@ -144,6 +144,22 @@ function extractTextFromToolResult(result: unknown): string | null { return trimmed ? trimmed : null; } +function extractBlockedToolReason(result: unknown): string | null { + if (!result || typeof result !== "object") { + return null; + } + const details = (result as { details?: unknown }).details; + if (!details || typeof details !== "object") { + return null; + } + const status = (details as { status?: unknown }).status; + if (status !== "blocked") { + return null; + } + const reason = (details as { reason?: unknown }).reason; + return typeof reason === "string" && reason.trim() ? reason.trim() : null; +} + export async function handleInlineActions(params: { ctx: MsgContext; sessionCtx: TemplateContext; @@ -246,6 +262,7 @@ export async function handleInlineActions(params: { skillFilter, }) : []; + const targetSessionEntry = sessionStore?.[sessionKey] ?? sessionEntry; const skillInvocation = allowTextCommands && skillCommands.length > 0 @@ -285,6 +302,8 @@ export async function handleInlineActions(params: { config: cfg, allowGatewaySubagentBinding: true, senderIsOwner: command.senderIsOwner, + sessionId: targetSessionEntry?.sessionId, + currentChannelId: command.channelId, }); const authorizedTools = applyOwnerOnlyToolPolicy(tools, command.senderIsOwner); @@ -301,7 +320,15 @@ export async function handleInlineActions(params: { commandName: skillInvocation.command.name, skillName: skillInvocation.command.skillName, }; - const result = await tool.execute(toolCallId, toolArgs); + const result = await tool.execute(toolCallId, toolArgs, opts?.abortSignal); + const blockedReason = extractBlockedToolReason(result); + if (blockedReason) { + typing.cleanup(); + return { + kind: "reply", + reply: { text: `❌ Tool call blocked: ${blockedReason}` }, + }; + } const text = extractTextFromToolResult(result) ?? "✅ Done."; typing.cleanup(); return { kind: "reply", reply: { text } }; @@ -342,7 +369,6 @@ export async function handleInlineActions(params: { }; const isStopLikeInbound = isAbortRequestText(command.rawBodyNormalized); - const targetSessionEntry = sessionStore?.[sessionKey] ?? sessionEntry; if (!isStopLikeInbound && targetSessionEntry) { const cutoff = readAbortCutoffFromSessionEntry(targetSessionEntry); const incoming = resolveAbortCutoffFromContext(ctx); diff --git a/src/gateway/tool-resolution.ts b/src/gateway/tool-resolution.ts index 37c2e690c6d..96afcedc312 100644 --- a/src/gateway/tool-resolution.ts +++ b/src/gateway/tool-resolution.ts @@ -95,6 +95,7 @@ export function resolveGatewayScopedTools(params: { allowGatewaySubagentBinding: params.allowGatewaySubagentBinding, allowMediaInvokeCommands: params.allowMediaInvokeCommands, disablePluginTools: params.disablePluginTools, + wrapBeforeToolCallHook: false, senderIsOwner: params.senderIsOwner, config: params.cfg, workspaceDir,