diff --git a/agentscope-core/src/main/java/io/agentscope/core/ReActAgent.java b/agentscope-core/src/main/java/io/agentscope/core/ReActAgent.java index 5c35401474..2378bfa0ff 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/ReActAgent.java +++ b/agentscope-core/src/main/java/io/agentscope/core/ReActAgent.java @@ -99,6 +99,7 @@ import io.agentscope.core.model.ToolSchema; import io.agentscope.core.permission.PermissionBehavior; import io.agentscope.core.permission.PermissionContextState; +import io.agentscope.core.permission.PermissionDecision; import io.agentscope.core.permission.PermissionEngine; import io.agentscope.core.permission.PermissionMode; import io.agentscope.core.permission.PermissionRule; @@ -3070,6 +3071,14 @@ Flux actingStream( .flatMapMany( gate -> { List pending = gate.pendingAsk(); + Map> pendToolMap = + pending.stream() + .collect( + Collectors.toMap( + ToolUseBlock::getId, + ToolUseBlock::getSuggestedRules, + (a, b) -> a)); + Set autoDenied = gate.autoDeniedIds(); // Mark ToolUseBlock.state in context for every gated tool. ALLOWED @@ -3084,17 +3093,11 @@ Flux actingStream( } stateUpdates.put( tc.getId(), - pending.stream() - .anyMatch( - p -> - p.getId() - .equals( - tc - .getId())) + pendToolMap.containsKey(tc.getId()) ? ToolCallState.ASKING : ToolCallState.ALLOWED); } - updateToolCallStates(stateUpdates); + updateToolCallStatesAndRules(stateUpdates, pendToolMap); if (pending.isEmpty()) { return runToolBatch( @@ -3431,12 +3434,17 @@ private Mono evaluateOne(ToolUseBlock use, boolean useEngine) return permissionEngine .checkPermission(tb, input) .map( - decision -> - new PermissionVerdict( - use, - decision == null - ? PermissionBehavior.ASK - : decision.getBehavior())); + decision -> { + if (decision == null) { + return new PermissionVerdict(use, PermissionBehavior.ASK); + } + + // Carry the engine's suggested rules on the ToolUseBlock + ToolUseBlock toolUseBlock = + withSuggestedRulesIfExist(use, decision); + return new PermissionVerdict( + toolUseBlock, decision.getBehavior()); + }); } return tb.checkPermissions(input, state.getPermissionContext()) .map( @@ -3444,18 +3452,37 @@ private Mono evaluateOne(ToolUseBlock use, boolean useEngine) if (decision == null) { return new PermissionVerdict(use, PermissionBehavior.ALLOW); } + + // Carry the engine's suggested rules on the ToolUseBlock + ToolUseBlock newToolUse = withSuggestedRulesIfExist(use, decision); + // In the legacy lightweight path only an explicit ASK from the tool // gates execution; PASSTHROUGH and ALLOW both run, DENY is // honoured. return switch (decision.getBehavior()) { - case ASK -> new PermissionVerdict(use, PermissionBehavior.ASK); + case ASK -> + new PermissionVerdict( + newToolUse, PermissionBehavior.ASK); case DENY -> - new PermissionVerdict(use, PermissionBehavior.DENY); - default -> new PermissionVerdict(use, PermissionBehavior.ALLOW); + new PermissionVerdict( + newToolUse, PermissionBehavior.DENY); + default -> + new PermissionVerdict( + newToolUse, PermissionBehavior.ALLOW); }; }); } + private ToolUseBlock withSuggestedRulesIfExist( + ToolUseBlock use, PermissionDecision decision) { + // Carry the engine's suggested rules on the ToolUseBlock + List suggested = decision.getSuggestedRules(); + if (suggested == null || suggested.isEmpty()) { + return use; + } + return use.withSuggestedRules(suggested); + } + private record PermissionVerdict(ToolUseBlock use, PermissionBehavior behavior) {} private List getSuspendedToolCalls( @@ -4281,11 +4308,12 @@ private List extractRecentToolCalls() { // ==================== Tool call state helpers (Permission HITL) ==================== /** - * Locate the last assistant Msg in context and replace the {@code state} of every + * Locate the last assistant Msg in context and replace the {@code state,suggestedRules} of every * {@link ToolUseBlock} whose id matches the given map's key. Mirrors Python's * {@code _update_tool_call_state} but operates in bulk to minimise list rebuilds. */ - private void updateToolCallStates(Map updates) { + private void updateToolCallStatesAndRules( + Map updates, Map> pendToolMap) { if (updates == null || updates.isEmpty()) { return; } @@ -4297,12 +4325,20 @@ private void updateToolCallStates(Map updates) { for (ToolUseBlock toolUse : lastAssistant.getContentBlocks(ToolUseBlock.class)) { ToolCallState newState = updates.get(toolUse.getId()); if (newState != null) { - replacements.put(toolUse.getId(), toolUse.withState(newState)); + List rules = + pendToolMap.getOrDefault(toolUse.getId(), List.of()); + replacements.put( + toolUse.getId(), toolUse.withStateAndSuggestedRules(newState, rules)); } } MessageUtils.replaceToolUseBlocks(state.contextMutable(), replacements); } + /** Convenience overload for a single tool call. */ + private void updateToolCallState(String toolCallId, ToolCallState newState) { + updateToolCallStatesAndRules(Map.of(toolCallId, newState), Map.of()); + } + /** The ToolUseBlocks in the last assistant Msg that are in ASKING state (HITL pending). */ private List askingToolCalls() { Msg last = MessageUtils.lastAssistantMessage(state.contextMutable()); diff --git a/agentscope-core/src/main/java/io/agentscope/core/message/ToolUseBlock.java b/agentscope-core/src/main/java/io/agentscope/core/message/ToolUseBlock.java index ef187ba870..f0feb9cccf 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/message/ToolUseBlock.java +++ b/agentscope-core/src/main/java/io/agentscope/core/message/ToolUseBlock.java @@ -18,8 +18,11 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonProperty; +import io.agentscope.core.permission.PermissionRule; +import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; /** @@ -51,6 +54,7 @@ public final class ToolUseBlock extends ContentBlock { private final String content; // Raw content for streaming tool calls private final Map metadata; // Provider-specific metadata private final ToolCallState state; + private final List suggestedRules; // Rules suggested when gated by ASK /** * Creates a new tool use block with metadata (convenience constructor). @@ -62,7 +66,7 @@ public final class ToolUseBlock extends ContentBlock { */ public ToolUseBlock( String id, String name, Map input, Map metadata) { - this(id, name, input, null, metadata, null); + this(id, name, input, null, metadata, null, null); } /** @@ -73,7 +77,7 @@ public ToolUseBlock( * @param input Input parameters for the tool (will be defensively copied) */ public ToolUseBlock(String id, String name, Map input) { - this(id, name, input, null, null, null); + this(id, name, input, null, null, null, null); } /** @@ -91,7 +95,7 @@ public ToolUseBlock( Map input, String content, Map metadata) { - this(id, name, input, content, metadata, null); + this(id, name, input, content, metadata, null, null); } /** @@ -104,6 +108,27 @@ public ToolUseBlock( * @param metadata Provider-specific metadata (will be defensively copied) * @param state The tool call state, defaults to PENDING if null */ + public ToolUseBlock( + String id, + String name, + Map input, + String content, + Map metadata, + ToolCallState state) { + this(id, name, input, content, metadata, state, null); + } + + /** + * Creates a new tool use block with all fields. + * + * @param id Unique identifier for this tool call + * @param name Name of the tool to execute + * @param input Input parameters for the tool (will be defensively copied) + * @param content Raw content for streaming tool calls + * @param metadata Provider-specific metadata (will be defensively copied) + * @param state The tool call state, defaults to PENDING if null + * @param suggestedRules Permission rules suggested for this call (will be defensively copied) + */ @JsonCreator public ToolUseBlock( @JsonProperty("id") String id, @@ -111,7 +136,8 @@ public ToolUseBlock( @JsonProperty("input") Map input, @JsonProperty("content") String content, @JsonProperty("metadata") Map metadata, - @JsonProperty("state") ToolCallState state) { + @JsonProperty("state") ToolCallState state, + @JsonProperty("suggested_rules") List suggestedRules) { this.id = id; this.name = name; // Defensive copy to prevent external modifications @@ -125,6 +151,10 @@ public ToolUseBlock( ? Collections.emptyMap() : Collections.unmodifiableMap(new HashMap<>(metadata)); this.state = state != null ? state : ToolCallState.PENDING; + this.suggestedRules = + suggestedRules == null + ? Collections.emptyList() + : Collections.unmodifiableList(new ArrayList<>(suggestedRules)); } /** @@ -184,6 +214,23 @@ public ToolCallState getState() { return state; } + /** + * Gets the permission rules suggested for this tool call. + * + *

Populated when the permission engine gates this call with + * {@link io.agentscope.core.permission.PermissionBehavior#ASK}: the owning tool derives + * candidate rules from the actual invocation via + * {@link io.agentscope.core.tool.ToolBase#generateSuggestions}. Passing them back in a + * {@code ConfirmResult} registers them with the engine, so future matching calls resolve + * without another prompt. + * + * @return The suggested rules, or an empty list when none were suggested + */ + @JsonProperty("suggested_rules") + public List getSuggestedRules() { + return suggestedRules; + } + /** * Checks whether this tool call is executed by the provider on the server side (marked via * {@link #METADATA_SERVER_TOOL}). @@ -202,7 +249,43 @@ public boolean isServerTool() { * @return A new ToolUseBlock with the updated state */ public ToolUseBlock withState(ToolCallState state) { - return new ToolUseBlock(this.id, this.name, this.input, this.content, this.metadata, state); + return new ToolUseBlock( + this.id, + this.name, + this.input, + this.content, + this.metadata, + state, + this.suggestedRules); + } + + /** + * Returns a copy of this block with the given suggested rules. + * + * @param suggestedRules The suggested permission rules + * @return A new ToolUseBlock with the updated suggested rules + */ + public ToolUseBlock withSuggestedRules(List suggestedRules) { + return new ToolUseBlock( + this.id, + this.name, + this.input, + this.content, + this.metadata, + this.state, + suggestedRules); + } + + /** + * Returns a copy of this block with the given suggested rules. + * @param state The new state + * @param suggestedRules The suggested permission rules + * @return A new ToolUseBlock with the updated suggested rules and state + */ + public ToolUseBlock withStateAndSuggestedRules( + ToolCallState state, List suggestedRules) { + return new ToolUseBlock( + this.id, this.name, this.input, this.content, this.metadata, state, suggestedRules); } /** @@ -224,6 +307,7 @@ public static class Builder { private String content; private Map metadata; private ToolCallState state; + private List suggestedRules; /** * Sets the unique identifier for the tool call. @@ -294,13 +378,24 @@ public Builder state(ToolCallState state) { return this; } + /** + * Sets the permission rules suggested for this tool call. + * + * @param suggestedRules The suggested permission rules + * @return This builder for chaining + */ + public Builder suggestedRules(List suggestedRules) { + this.suggestedRules = suggestedRules; + return this; + } + /** * Builds a new ToolUseBlock with the configured properties. * * @return A new ToolUseBlock instance */ public ToolUseBlock build() { - return new ToolUseBlock(id, name, input, content, metadata, state); + return new ToolUseBlock(id, name, input, content, metadata, state, suggestedRules); } } } diff --git a/agentscope-core/src/test/java/io/agentscope/core/agent/ReActAgentHitlTest.java b/agentscope-core/src/test/java/io/agentscope/core/agent/ReActAgentHitlTest.java index b1f5a33de4..e382d89d74 100644 --- a/agentscope-core/src/test/java/io/agentscope/core/agent/ReActAgentHitlTest.java +++ b/agentscope-core/src/test/java/io/agentscope/core/agent/ReActAgentHitlTest.java @@ -834,6 +834,77 @@ void allowingToolBypassesHitlEntirely() { assertEquals(ToolResultState.SUCCESS, end.getState()); } + /** + * Regression test for issue where suggested rules were lost in the lightweight permission + * path. When a default agent (no PermissionEngine) uses a tool that returns + * PermissionDecision.ask().withSuggestedRules(...), the suggested rules must be carried + * through to the returned ToolUseBlock and the RequireUserConfirmEvent. + */ + @Test + void lightweightPathPreservesSuggestedRulesFromAskingTool() { + ChatModelBase model = + new ScriptedModel( + List.of(() -> Flux.just(toolUseResponse("tc1", "send_money", "test")))); + ReActAgent agent = buildAgent(model, toolkitWith(new SendMoneyTool())); + + // Use streamEvents to capture both the result and events + List events = agent.streamEvents(List.of()).collectList().block(); + assertNotNull(events); + + // Verify RequireUserConfirmEvent was emitted with suggested rules + int iReq = indexOf(events, RequireUserConfirmEvent.class); + assertTrue(iReq >= 0, "RequireUserConfirmEvent must be emitted"); + + RequireUserConfirmEvent req = (RequireUserConfirmEvent) events.get(iReq); + assertEquals(1, req.getToolCalls().size()); + ToolUseBlock eventBlock = req.getToolCalls().get(0); + assertNotNull( + eventBlock.getSuggestedRules(), + "suggested rules must be present in RequireUserConfirmEvent"); + assertEquals( + 1, + eventBlock.getSuggestedRules().size(), + "RequireUserConfirmEvent must carry the suggested rule"); + assertEquals( + "send_money", + eventBlock.getSuggestedRules().get(0).toolName(), + "rule tool name must match"); + } + + public static class SendMoneyTool extends ToolBase { + public SendMoneyTool() { + super( + ToolBase.builder() + .name("send_money") + .description("Sends a money to the user") + .inputSchema( + Map.of( + "type", "object", + "properties", + Map.of( + "user", Map.of("type", "string"), + "money", Map.of("type", "string")), + "required", List.of("user", "money")))); + } + + @Override + public Mono checkPermissions( + Map toolInput, PermissionContextState context) { + return Mono.just( + PermissionDecision.ask("Requesting permission for send_money tool") + .withSuggestedRules(generateSuggestions(toolInput))); + } + + @Override + public Mono callAsync(ToolCallParam param) { + return Mono.just( + ToolResultBlock.builder() + .name("send_money") + .output(TextBlock.builder().text("send success!").build()) + .build()); + } + } + @Test void deniedConfirmResultWithCustomReasonWritesReasonToContext() { ChatModelBase model = diff --git a/agentscope-core/src/test/java/io/agentscope/core/message/ToolUseBlockTest.java b/agentscope-core/src/test/java/io/agentscope/core/message/ToolUseBlockTest.java index ed46eb4d63..5beb27aa62 100644 --- a/agentscope-core/src/test/java/io/agentscope/core/message/ToolUseBlockTest.java +++ b/agentscope-core/src/test/java/io/agentscope/core/message/ToolUseBlockTest.java @@ -22,6 +22,9 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; +import io.agentscope.core.permission.PermissionBehavior; +import io.agentscope.core.permission.PermissionRule; +import java.util.List; import java.util.Map; import org.junit.jupiter.api.Test; @@ -350,4 +353,81 @@ void testEmptyMapsForNullInputAndMetadata() { assertTrue(toolUseBlock.getMetadata().isEmpty()); assertEquals(null, toolUseBlock.getContent()); } + + @Test + void testSuggestedRulesDefaultsToEmptyList() { + ToolUseBlock toolUseBlock = new ToolUseBlock("tool-1000", "no-rules", Map.of()); + + assertNotNull(toolUseBlock.getSuggestedRules()); + assertTrue(toolUseBlock.getSuggestedRules().isEmpty()); + } + + @Test + void testWithSuggestedRulesKeepsStateAndCopiesRules() { + PermissionRule rule = + new PermissionRule("calculator", "add(*)", PermissionBehavior.ALLOW, "tool"); + ToolUseBlock original = + ToolUseBlock.builder() + .id("tool-1001") + .name("calculator") + .state(ToolCallState.ASKING) + .build(); + + ToolUseBlock updated = original.withSuggestedRules(List.of(rule)); + + assertTrue(original.getSuggestedRules().isEmpty()); + assertEquals(ToolCallState.ASKING, updated.getState()); + assertEquals(List.of(rule), updated.getSuggestedRules()); + } + + @Test + void testWithStateAndSuggestedRules() { + PermissionRule rule = + new PermissionRule("calculator", "add(*)", PermissionBehavior.ALLOW, "tool"); + ToolUseBlock original = ToolUseBlock.builder().id("tool-1002").name("calculator").build(); + + ToolUseBlock updated = + original.withStateAndSuggestedRules(ToolCallState.ASKING, List.of(rule)); + + assertEquals(ToolCallState.ASKING, updated.getState()); + assertEquals(List.of(rule), updated.getSuggestedRules()); + } + + @Test + void testWithStatePreservesSuggestedRules() { + PermissionRule rule = + new PermissionRule("calculator", "add(*)", PermissionBehavior.ALLOW, "tool"); + ToolUseBlock asking = + ToolUseBlock.builder() + .id("tool-1003") + .name("calculator") + .state(ToolCallState.ASKING) + .suggestedRules(List.of(rule)) + .build(); + + ToolUseBlock allowed = asking.withState(ToolCallState.ALLOWED); + + assertEquals(ToolCallState.ALLOWED, allowed.getState()); + assertEquals(List.of(rule), allowed.getSuggestedRules()); + } + + @Test + void testSuggestedRulesRoundTrip() throws JsonProcessingException { + PermissionRule rule = + new PermissionRule("calculator", "add(*)", PermissionBehavior.ALLOW, "tool"); + ToolUseBlock toolUseBlock = + ToolUseBlock.builder() + .id("tool-1004") + .name("calculator") + .state(ToolCallState.ASKING) + .suggestedRules(List.of(rule)) + .build(); + + String json = objectMapper.writeValueAsString(toolUseBlock); + assertTrue(json.contains("\"suggested_rules\"")); + + ToolUseBlock parsed = objectMapper.readValue(json, ToolUseBlock.class); + assertEquals(List.of(rule), parsed.getSuggestedRules()); + assertEquals(ToolCallState.ASKING, parsed.getState()); + } } diff --git a/agentscope-examples/documentation/src/main/java/io/agentscope/examples/documentation2/hitl/PermissionHITLExample.java b/agentscope-examples/documentation/src/main/java/io/agentscope/examples/documentation2/hitl/PermissionHITLExample.java index c4ea00e3e6..09f23314d8 100644 --- a/agentscope-examples/documentation/src/main/java/io/agentscope/examples/documentation2/hitl/PermissionHITLExample.java +++ b/agentscope-examples/documentation/src/main/java/io/agentscope/examples/documentation2/hitl/PermissionHITLExample.java @@ -179,7 +179,9 @@ private static void runInteractive(ReActAgent agent) { // Build ConfirmResult from the extracted ToolUseBlocks and resume List confirmResults = - askingTools.stream().map(t -> new ConfirmResult(approved, t)).toList(); + askingTools.stream() + .map(t -> new ConfirmResult(approved, t, t.getSuggestedRules())) + .toList(); Map meta = new HashMap<>(); meta.put(Msg.METADATA_CONFIRM_RESULTS, confirmResults); Msg resumeMsg =