From d4ea75a80603212df5472e73fc43361691504eea Mon Sep 17 00:00:00 2001 From: matevip Date: Fri, 29 May 2026 18:03:13 +0800 Subject: [PATCH] fix(approval): deny approval-required tools in non-interactive runs --- .../graph/executor/ToolExecutionExecutor.java | 31 +++++++- ...ionExecutorNonInteractiveApprovalTest.java | 70 +++++++++++++++++++ 2 files changed, 99 insertions(+), 2 deletions(-) create mode 100644 mateclaw-server/src/test/java/vip/mate/agent/graph/executor/ToolExecutionExecutorNonInteractiveApprovalTest.java diff --git a/mateclaw-server/src/main/java/vip/mate/agent/graph/executor/ToolExecutionExecutor.java b/mateclaw-server/src/main/java/vip/mate/agent/graph/executor/ToolExecutionExecutor.java index d4472d38..f832af01 100644 --- a/mateclaw-server/src/main/java/vip/mate/agent/graph/executor/ToolExecutionExecutor.java +++ b/mateclaw-server/src/main/java/vip/mate/agent/graph/executor/ToolExecutionExecutor.java @@ -493,7 +493,7 @@ public class ToolExecutionExecutor { // 2. ToolGuard 安全检查（replay 模式跳过） if (!isReplay) { GuardDecision decision = evaluateGuard(toolCall, toolName, arguments, - conversationId, agentId, toolCalls, i, events, requesterId); + conversationId, agentId, toolCalls, i, events, requesterId, safeOrigin); if (decision.blocked) { allResponses.add(new ToolResponseMessage.ToolResponse( @@ -933,7 +933,8 @@ public class ToolExecutionExecutor { private GuardDecision evaluateGuard(AssistantMessage.ToolCall toolCall, String toolName, String arguments, String conversationId, String agentId, List allToolCalls, int currentIndex, - List events, String requesterId) { + List events, String requesterId, + ChatOrigin origin) { // Auto-grant requires BOTH the lookup cache and the resolver to be wired. // Legacy constructors leave them null; in that case we skip workspace // resolution and skip the resolver block, falling back to the original @@ -979,6 +980,13 @@ public class ToolExecutionExecutor { // requiresHuman → fall through to legacy human-approval path below. } + // No human can resolve an approval in a non-interactive (scheduled-job) + // run, so a pending request would hang the turn until it times out with + // no answer. Deny immediately with an actionable message instead. + if (origin != null && origin.cronOrigin()) { + return denyNonInteractiveApproval(toolCall, toolName, events); + } + List remaining = allToolCalls.subList(currentIndex + 1, allToolCalls.size()); String approvalResponse = ToolExecutionGuardHelper.handleToolApproval( toolCall, toolName, arguments, evaluation, @@ -998,6 +1006,9 @@ public class ToolExecutionExecutor { } if (guardResult.needsApproval()) { + if (origin != null && origin.cronOrigin()) { + return denyNonInteractiveApproval(toolCall, toolName, events); + } List remaining = allToolCalls.subList(currentIndex + 1, allToolCalls.size()); String approvalResponse = ToolExecutionGuardHelper.handleToolApprovalLegacy( toolCall, toolName, arguments, guardResult, @@ -1010,6 +1021,22 @@ public class ToolExecutionExecutor { return GuardDecision.allowed(); } + /** + * Deny an approval-required tool when the run is non-interactive (no human can + * approve), returning an actionable message so the agent falls back to a + * non-gated built-in tool instead of stalling on a pending nobody resolves. + */ + private GuardDecision denyNonInteractiveApproval(AssistantMessage.ToolCall toolCall, String toolName, + List events) { + String msg = "[审批不可用] 该工具需要人工审批，但当前为非交互（定时任务）运行，无人可批准，" + + "因此无法执行。请改用无需审批的内置工具完成本步骤（例如 PDF / XLSX / 文档技能、文件读写工具），" + + "或跳过该步骤并说明原因，不要反复重试同一命令。"; + log.info("[ToolExecutor] NON_INTERACTIVE_DENY: tool={} needs approval but origin is non-interactive (cron); " + + "denying to avoid an unresolvable pending", toolName); + events.add(GraphEventPublisher.toolComplete(toolCall.id(), toolName, msg, false)); + return GuardDecision.blocked(msg); + } + // ==================== 辅助方法 ==================== /** diff --git a/mateclaw-server/src/test/java/vip/mate/agent/graph/executor/ToolExecutionExecutorNonInteractiveApprovalTest.java b/mateclaw-server/src/test/java/vip/mate/agent/graph/executor/ToolExecutionExecutorNonInteractiveApprovalTest.java new file mode 100644 index 00000000..976dbe5d --- /dev/null +++ b/mateclaw-server/src/test/java/vip/mate/agent/graph/executor/ToolExecutionExecutorNonInteractiveApprovalTest.java @@ -0,0 +1,70 @@ +package vip.mate.agent.graph.executor; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.springframework.ai.chat.messages.AssistantMessage; +import org.springframework.ai.chat.messages.ToolResponseMessage; +import org.springframework.ai.chat.model.ToolContext; +import org.springframework.ai.tool.ToolCallback; +import org.springframework.ai.tool.definition.ToolDefinition; +import org.springframework.ai.tool.metadata.ToolMetadata; +import vip.mate.agent.AgentToolSet; +import vip.mate.agent.context.ChatOrigin; +import vip.mate.tool.guard.ToolGuard; +import vip.mate.tool.guard.ToolGuardResult; + +import java.util.List; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * A tool that requires human approval cannot be resolved in a non-interactive + * (scheduled-job) run — a pending request would hang the turn until it times out + * with no answer. The executor must deny such a tool immediately for a cron-origin + * invocation while still gating it normally for an interactive (web) origin. + */ +class ToolExecutionExecutorNonInteractiveApprovalTest { + + private ToolExecutionExecutor executorRequiringApproval(ToolCallback cb) { + AgentToolSet toolSet = AgentToolSet.fromCallbacks(List.of(), List.of(cb)); + ToolGuard needsApproval = (name, args) -> + ToolGuardResult.needsApproval("shell command execution requires approval", "shell_tool_default"); + return new ToolExecutionExecutor(toolSet, needsApproval, null, null); + } + + @Test + @DisplayName("cron origin denies an approval-required tool instead of creating an unresolvable pending") + void cronOriginDeniesApprovalRequiredTool() { + ToolExecutionExecutor executor = executorRequiringApproval( + stub("execute_shell_command", args -> "ran")); + AssistantMessage.ToolCall call = new AssistantMessage.ToolCall( + "c1", "function", "execute_shell_command", "{\"command\":\"ls\"}"); + + ChatOrigin cron = ChatOrigin.cron("conv_cron", null, null, null, null); + ToolExecutionExecutor.ToolExecutionResult result = + executor.execute(List.of(call), "conv_cron", "agent_x", false, "system", null, cron); + + assertFalse(result.awaitingApproval(), + "non-interactive origin must not create a pending approval"); + ToolResponseMessage.ToolResponse resp = result.responses().get(0); + assertTrue(resp.responseData().contains("[审批不可用]"), + "cron-origin approval-required tool should be denied with guidance, got: " + resp.responseData()); + } + + private static ToolCallback stub(String name, java.util.function.Function handler) { + ToolDefinition def = ToolDefinition.builder() + .name(name) + .description("test tool " + name) + .inputSchema("{\"type\":\"object\",\"properties\":{}}") + .build(); + ToolMetadata md = ToolMetadata.builder().returnDirect(false).build(); + return new ToolCallback() { + @Override public ToolDefinition getToolDefinition() { return def; } + @Override public ToolMetadata getToolMetadata() { return md; } + @Override public String call(String arguments) { return handler.apply(arguments); } + @Override public String call(String arguments, ToolContext toolContext) { + return handler.apply(arguments); + } + }; + } +}