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 f2b532d1b8..b94e26dab6 100644 --- a/agentscope-core/src/main/java/io/agentscope/core/ReActAgent.java +++ b/agentscope-core/src/main/java/io/agentscope/core/ReActAgent.java @@ -96,6 +96,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; @@ -2830,6 +2831,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 @@ -2844,17 +2853,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( @@ -3186,12 +3189,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( @@ -3199,18 +3207,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( @@ -3902,11 +3929,12 @@ private List extractPendingToolCalls() { // ==================== 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; } @@ -3928,7 +3956,8 @@ private void updateToolCallStates(Map updates) { List rebuilt = new ArrayList<>(m.getContent().size()); for (ContentBlock block : m.getContent()) { if (block instanceof ToolUseBlock t && updates.containsKey(t.getId())) { - rebuilt.add(t.withState(updates.get(t.getId()))); + List rules = pendToolMap.getOrDefault(t.getId(), List.of()); + rebuilt.add(t.withStateAndSuggestedRules(updates.get(t.getId()), rules)); } else { rebuilt.add(block); } @@ -3940,7 +3969,7 @@ private void updateToolCallStates(Map updates) { /** Convenience overload for a single tool call. */ private void updateToolCallState(String toolCallId, ToolCallState newState) { - updateToolCallStates(Map.of(toolCallId, newState)); + updateToolCallStatesAndRules(Map.of(toolCallId, newState), Map.of()); } /** Whether any ToolUseBlock in the last assistant Msg is in ASKING state. */ 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 eb0fbe7fe9..70521578b0 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 @@ -17,8 +17,11 @@ import com.fasterxml.jackson.annotation.JsonCreator; 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; /** @@ -42,6 +45,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 for JSON deserialization. @@ -53,7 +57,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); } /** @@ -64,7 +68,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); } /** @@ -82,7 +86,7 @@ public ToolUseBlock( Map input, String content, Map metadata) { - this(id, name, input, content, metadata, null); + this(id, name, input, content, metadata, null, null); } /** @@ -95,6 +99,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, @@ -102,7 +127,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 @@ -116,6 +142,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)); } /** @@ -175,6 +205,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; + } + /** * Returns a copy of this block with the given state. * @@ -182,7 +229,43 @@ public ToolCallState getState() { * @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); } /** @@ -204,6 +287,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. @@ -274,13 +358,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 ad708896a4..fe9b2784ea 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 @@ -656,4 +656,75 @@ void allowingToolBypassesHitlEntirely() { (ToolResultEndEvent) events.get(indexOf(events, ToolResultEndEvent.class)); 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()); + } + } } 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 f91a51958e..d981a0d5bc 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; @@ -293,4 +296,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 8eac8a2906..20519c90eb 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 =