Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 30 additions & 18 deletions agentscope-core/src/main/java/io/agentscope/core/ReActAgent.java
Original file line number Diff line number Diff line change
Expand Up @@ -2830,6 +2830,14 @@ Flux<AgentEvent> actingStream(
.flatMapMany(
gate -> {
List<ToolUseBlock> pending = gate.pendingAsk();
Map<String, List<PermissionRule>> pendToolMap =
pending.stream()
.collect(
Collectors.toMap(
ToolUseBlock::getId,
ToolUseBlock::getSuggestedRules,
(a, b) -> a));

Set<String> autoDenied = gate.autoDeniedIds();

// Mark ToolUseBlock.state in context for every gated tool. ALLOWED
Expand All @@ -2844,17 +2852,11 @@ Flux<AgentEvent> 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(
Expand Down Expand Up @@ -3186,12 +3188,20 @@ private Mono<PermissionVerdict> 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
List<PermissionRule> suggested = decision.getSuggestedRules();
ToolUseBlock toolUseBlock = use;
if (suggested != null && !suggested.isEmpty()) {
toolUseBlock = use.withSuggestedRules(suggested);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 轻量权限路径也需要传递 suggested rules

这里的复制只发生在 useEngine == true 的分支。默认构建的 ReActAgent 使用 trivial PermissionContextState,会进入下面的轻量权限路径;如果自定义 ToolBase.checkPermissions() 返回 PermissionDecision.ask(...).withSuggestedRules(...)case ASK 仍用原始 use 构造 PermissionVerdict,这些规则会被静默丢失。

我用针对性测试复现后,完整 PermissionEngine 路径可以拿到 1 条规则,但默认轻量路径得到的是 0 条。这样本次 HITL 确认虽然仍能进行,暂停返回的 MsgRequireUserConfirmEventgetSuggestedRules() 却为空,调用方无法接受工具生成的规则,后续同类调用仍会重复询问。

建议让轻量路径的 ASK 分支也把 decision.getSuggestedRules() 附加到 ToolUseBlock,并补一个默认 agent 下的端到端回归测试。

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已经处理,麻烦再review下

}
return new PermissionVerdict(
toolUseBlock, decision.getBehavior());
});
}
return tb.checkPermissions(input, state.getPermissionContext())
.map(
Expand Down Expand Up @@ -3902,11 +3912,12 @@ private List<ToolUseBlock> 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<String, ToolCallState> updates) {
private void updateToolCallStatesAndRules(
Map<String, ToolCallState> updates, Map<String, List<PermissionRule>> pendToolMap) {
if (updates == null || updates.isEmpty()) {
return;
}
Expand All @@ -3928,7 +3939,8 @@ private void updateToolCallStates(Map<String, ToolCallState> updates) {
List<ContentBlock> 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<PermissionRule> rules = pendToolMap.getOrDefault(t.getId(), List.of());
rebuilt.add(t.withStateAndSuggestedRules(updates.get(t.getId()), rules));
} else {
rebuilt.add(block);
}
Expand All @@ -3940,7 +3952,7 @@ private void updateToolCallStates(Map<String, ToolCallState> 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. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -42,6 +45,7 @@ public final class ToolUseBlock extends ContentBlock {
private final String content; // Raw content for streaming tool calls
private final Map<String, Object> metadata; // Provider-specific metadata
private final ToolCallState state;
private final List<PermissionRule> suggestedRules; // Rules suggested when gated by ASK

/**
* Creates a new tool use block for JSON deserialization.
Expand All @@ -53,7 +57,7 @@ public final class ToolUseBlock extends ContentBlock {
*/
public ToolUseBlock(
String id, String name, Map<String, Object> input, Map<String, Object> metadata) {
this(id, name, input, null, metadata, null);
this(id, name, input, null, metadata, null, null);
}

/**
Expand All @@ -64,7 +68,7 @@ public ToolUseBlock(
* @param input Input parameters for the tool (will be defensively copied)
*/
public ToolUseBlock(String id, String name, Map<String, Object> input) {
this(id, name, input, null, null, null);
this(id, name, input, null, null, null, null);
}

/**
Expand All @@ -82,7 +86,7 @@ public ToolUseBlock(
Map<String, Object> input,
String content,
Map<String, Object> metadata) {
this(id, name, input, content, metadata, null);
this(id, name, input, content, metadata, null, null);
}

/**
Expand All @@ -95,14 +99,36 @@ 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<String, Object> input,
String content,
Map<String, Object> 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,
@JsonProperty("name") String name,
@JsonProperty("input") Map<String, Object> input,
@JsonProperty("content") String content,
@JsonProperty("metadata") Map<String, Object> metadata,
@JsonProperty("state") ToolCallState state) {
@JsonProperty("state") ToolCallState state,
@JsonProperty("suggested_rules") List<PermissionRule> suggestedRules) {
this.id = id;
this.name = name;
// Defensive copy to prevent external modifications
Expand All @@ -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));
}

/**
Expand Down Expand Up @@ -175,14 +205,67 @@ public ToolCallState getState() {
return state;
}

/**
* Gets the permission rules suggested for this tool call.
*
* <p>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<PermissionRule> getSuggestedRules() {
return suggestedRules;
}

/**
* Returns a copy of this block with the given state.
*
* @param state The new state
* @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<PermissionRule> 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<PermissionRule> suggestedRules) {
return new ToolUseBlock(
this.id, this.name, this.input, this.content, this.metadata, state, suggestedRules);
}

/**
Expand All @@ -204,6 +287,7 @@ public static class Builder {
private String content;
private Map<String, Object> metadata;
private ToolCallState state;
private List<PermissionRule> suggestedRules;

/**
* Sets the unique identifier for the tool call.
Expand Down Expand Up @@ -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<PermissionRule> 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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,9 @@ private static void runInteractive(ReActAgent agent) {

// Build ConfirmResult from the extracted ToolUseBlocks and resume
List<ConfirmResult> confirmResults =
askingTools.stream().map(t -> new ConfirmResult(approved, t)).toList();
askingTools.stream()
.map(t -> new ConfirmResult(approved, t, t.getSuggestedRules()))
.toList();
Map<String, Object> meta = new HashMap<>();
meta.put(Msg.METADATA_CONFIRM_RESULTS, confirmResults);
Msg resumeMsg =
Expand Down
Loading