fix(core): add ToolUseBlock.getSuggestedRules() implementation by ShuiMu-peng · Pull Request #2916 · agentscope-ai/agentscope-java

ShuiMu-peng · 2026-08-31T04:10:08Z

实现了 ToolUseBlock.getSuggestedRules() 的完整功能，该功能已在文档中描述但尚未实现。

变更内容
关闭: #1592
核心实现

ToolUseBlock.java:
- 添加 suggestedRules 字段用于存储权限引擎建议的规则
- 实现 getSuggestedRules() 方法
- 实现 withSuggestedRules() 和 withStateAndSuggestedRules() 方法
- 支持 JSON 序列化/反序列化（字段名：suggested_rules）
ReActAgent.java:
- 在权限检查流程中调用 decision.getSuggestedRules()
- 将建议的规则附加到 ToolUseBlock 上
  测试
ToolUseBlockTest.java:
- 添加 6 个新测试用例覆盖 suggestedRules 相关功能
- 测试默认值、设置规则、状态保持、JSON 序列化等场景

示例

PermissionHITLExample.java:
更新示例代码使用 getSuggestedRules() 方法

测试

已通过的测试

mvn test -Dtest=ToolUseBlockTest
mvn spotless:check

所有新功能都包含了单元测试，代码格式符合项目规范。

Codecov Report

❌ Patch coverage is 72.72727% with 12 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...e/src/main/java/io/agentscope/core/ReActAgent.java	62.96%	9 Missing and 1 partial ⚠️
.../java/io/agentscope/core/message/ToolUseBlock.java	88.23%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!

dailingtao · 2026-09-01T14:41:56Z

+                                    List<PermissionRule> suggested = decision.getSuggestedRules();
+                                    ToolUseBlock toolUseBlock = use;
+                                    if (suggested != null && !suggested.isEmpty()) {
+                                        toolUseBlock = use.withSuggestedRules(suggested);


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

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

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

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

已经处理，麻烦再review下

… rules

…ules-final' into fix/add-tooluseblock-suggested-rules-final

… rules

dailingtao

LGTM

oss-maintainer · 2026-09-17T09:46:22Z

⚠️ Merge conflict detected

This PR currently conflicts with main and cannot be merged. Please rebase (or merge main in) and resolve the conflicts:

git fetch upstream
git checkout fix/add-tooluseblock-suggested-rules-final
git rebase upstream/main
# resolve conflicts, then:
git push --force-with-lease

This is a one-time reminder. Once the conflicts are resolved, comment @oss-maintainer please re-review and I will pick it up again.

Automated notification by github-manager-bot

oss-maintainer

Summary

Plumbs the permission engine / tool suggested rules through to ToolUseBlock so an approved ASK can register rules without a second prompt, and keeps withState(...) preserving the new field. API additions are non-breaking (the old constructors are kept and delegate), JSON round-trip is covered, and the default-to-empty-list normalisation is the right call.

Findings

[Warning] updateToolCallStatesAndRules clears suggestedRules for updated ids that are not pending (and via the single-id convenience overload) — see inline comment.
[Info] Collectors.toMap is NPE-sensitive; the safety depends on a normalisation one class away.
[Info] naming + a test gap on the context write-back path.

Suggestions

The core fix in evaluateOne (both engine and lightweight paths now carry withSuggestedRulesIfExist) looks correct and is the part the issue was about.

Verdict

Comment only — one behavioural edge to settle before approval. The PR also still conflicts with main.

Automated review by github-manager-bot

oss-maintainer · 2026-09-22T01:02:04Z

                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());


getOrDefault(id, List.of()) overwrites the field unconditionally via withStateAndSuggestedRules, so every gated-but-not-pending block (the ALLOWED branch of stateUpdates) has any rules it previously carried erased. Before this change the same call site used withState(...), which preserved every other field. The same happens from updateToolCallState(String, ToolCallState) (now delegating with an empty map), so a single-id state transition also clears rules. Suggest only writing rules when the id is actually present, e.g. keep t.withState(...) for ids missing from pendToolMap, so "no information" and "no rules" stay distinguishable. If clearing is really intended, a test that transitions an ASKING block with rules to ALLOWED through this method would make that explicit.

oss-maintainer · 2026-09-22T01:02:04Z

+                                Map<String, List<PermissionRule>> pendToolMap =
+                                        pending.stream()
+                                                .collect(
+                                                        Collectors.toMap(


Collectors.toMap throws NPE when the value mapper returns null. Today ToolUseBlock#getSuggestedRules normalises null to an empty list so this is safe, but it is only safe because of a guarantee one class away — an explicit loop (or Objects.requireNonNullElseGet) keeps this from breaking if a legacy/deserialized block ever yields null.

oss-maintainer · 2026-09-22T01:02:04Z

+     * @return The suggested rules, or an empty list when none were suggested
+     */
+    @JsonProperty("suggested_rules")
+    public List<PermissionRule> getSuggestedRules() {


Two small things: (1) the name pendToolMap reads as key→tool while it is key→rules — pendingSuggestedRules would match the new getter; (2) getSuggestedRules() documents "empty list when none were suggested", so the caller-side assertNotNull in ReActAgentHitlTest tests the contract of a method that cannot return null — asserting the context block (not only the RequireUserConfirmEvent payload) carries the rules would cover the write-back path in updateToolCallStatesAndRules, which is where the [Warning] above lives.

…k-suggested-rules-final # Conflicts: # agentscope-core/src/main/java/io/agentscope/core/ReActAgent.java # agentscope-core/src/main/java/io/agentscope/core/message/ToolUseBlock.java # agentscope-core/src/test/java/io/agentscope/core/agent/ReActAgentHitlTest.java

jujn · 2026-09-29T14:17:57Z

fixed in: #3293

dailingtao reviewed Sep 1, 2026

View reviewed changes

ShuiMu-peng and others added 5 commits September 1, 2026 23:13

Merge branch 'main' into fix/add-tooluseblock-suggested-rules-final

c5e3d05

fix(core): Guarantee the lightweight path preservation recommendation…

3ca4fca

… rules

Merge remote-tracking branch 'origin/fix/add-tooluseblock-suggested-r…

e9e9288

…ules-final' into fix/add-tooluseblock-suggested-rules-final

fix(core): Guarantee the lightweight path preservation recommendation…

47dc241

… rules

Merge branch 'main' into fix/add-tooluseblock-suggested-rules-final

0d56921

dailingtao approved these changes Sep 2, 2026

View reviewed changes

oss-maintainer reviewed Sep 22, 2026

View reviewed changes

oss-maintainer mentioned this pull request Sep 22, 2026

[Question] What is the best practice for obtaining permission suggestedRules in the ReActAgent HITL flow? #3243

Open

Alanyaeer mentioned this pull request Sep 22, 2026

fix(doc): replace getSuggestedRules with generationSuggestions in ToolBase #3238

Closed

jujn closed this Sep 29, 2026

Navigation Menu

Repository navigation

fix(core): add ToolUseBlock.getSuggestedRules() implementation - #2916

fix(core): add ToolUseBlock.getSuggestedRules() implementation#2916
ShuiMu-peng wants to merge 7 commits into
agentscope-ai:mainfrom
ShuiMu-peng:fix/add-tooluseblock-suggested-rules-final

ShuiMu-peng commented Aug 31, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Aug 31, 2026 •

edited

Loading

Uh oh!

dailingtao Sep 1, 2026

Uh oh!

ShuiMu-peng Sep 2, 2026

Uh oh!

dailingtao left a comment

Uh oh!

oss-maintainer commented Sep 17, 2026

Uh oh!

oss-maintainer left a comment

Uh oh!

oss-maintainer Sep 22, 2026

Uh oh!

oss-maintainer Sep 22, 2026

Uh oh!

oss-maintainer Sep 22, 2026

Uh oh!

jujn commented Sep 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

ShuiMu-peng commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

已通过的测试

Uh oh!

codecov Bot commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

dailingtao Sep 1, 2026

Choose a reason for hiding this comment

Uh oh!

ShuiMu-peng Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

dailingtao left a comment

Choose a reason for hiding this comment

Uh oh!

oss-maintainer commented Sep 17, 2026

Uh oh!

oss-maintainer left a comment

Choose a reason for hiding this comment

Summary

Findings

Suggestions

Verdict

Uh oh!

oss-maintainer Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

oss-maintainer Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

oss-maintainer Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

jujn commented Sep 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ShuiMu-peng commented Aug 31, 2026 •

edited

Loading

codecov Bot commented Aug 31, 2026 •

edited

Loading