fix: 同步动态 skill tool 激活状态#1735
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
AgentScopeJavaBot
left a comment
There was a problem hiding this comment.
🤖 AI Review
This PR fixes a real and well-identified issue: dynamic skill tool groups activated during onSystemPrompt middleware or tool execution were not visible to the current/next reasoning round because the call-scoped AgentState was stale relative to the live Toolkit. The fix adds two strategically placed syncToolkitToState() calls (after system prompt consumption and after acting) and introduces tool group activation logic in SkillLoadTool with a sensible fallback chain (param.getAgent().getToolkit() → toolkitRef). The SkillRuntime.install() now refreshes the toolkitRef to prevent stale references. Test coverage is solid, covering all three sync scenarios. The implementation is clean, minimal, and correctly handles null safety throughout. No blocking issues found.
AgentScopeJavaBot
left a comment
There was a problem hiding this comment.
🤖 AI Review
This PR fixes a real and well-identified issue: dynamic skill tool groups activated during onSystemPrompt middleware or tool execution were not visible to the current/next reasoning round because the call-scoped AgentState was stale relative to the live Toolkit. The fix adds two strategically placed syncToolkitToState() calls (after system prompt consumption and after acting) and introduces tool group activation logic in SkillLoadTool with a sensible fallback chain (param.getAgent().getToolkit() → toolkitRef). The SkillRuntime.install() now refreshes the toolkitRef to prevent stale references. Test coverage is solid, covering all three sync scenarios. The implementation is clean, minimal, and correctly handles null safety throughout. No blocking issues found.
| if (toolkit == null && toolkitRef != null) { | ||
| toolkit = toolkitRef.get(); | ||
| } | ||
| if (toolkit == null) { |
There was a problem hiding this comment.
[nit] When the toolkit cannot be resolved (both param.getAgent() and toolkitRef fallback return null), the method returns silently. Consider adding a log.debug("activateSkillTools: no toolkit available for skill {}", entry.skill().getSkillId()) here to aid production debugging of skill activation issues.
问题背景
Issue #1715 中,动态 skill 在两处会出现工具不可见的问题:
onSystemPrompt阶段新注册或激活的 tool group 没有同步到当前 call state,导致首轮 reasoning 看不到动态工具。load_skill_through_path执行后,toolkit 中对应的 skill tool group 已经激活,但ReActAgent下一轮 reasoning 仍然读取旧的 state active groups,导致 skill-bound tools 仍不可见。变更内容
ReActAgent中补充 toolkit active groups 到当前 call state 的同步时机:consumeSystemMsgAfterPreCall(...)后同步一次,保证首轮 reasoning 可见;SkillLoadTool成功加载 skill 后,优先使用param.getAgent().getToolkit(),否则 fallback 到 runtime 持有的 toolkit;<skillId>_skill_tools,则立即激活对应 group。SkillRuntime.install(...)中,每次安装时刷新 runtime 持有的 toolkit 引用。onSystemPrompt激活的 group 在首轮 reasoning 可见;验证
已执行并通过以下测试:
mvn -pl agentscope-core '-Dtest=ReActAgentToolGroupSyncTest' '-Dsurefire.failIfNoSpecifiedTests=false' testmvn -pl agentscope-core '-Dtest=SkillRuntimeIntegrationTest,ReActAgentMiddlewareIntegrationTest' '-Dsurefire.failIfNoSpecifiedTests=false' testmvn -pl agentscope-harness -am '-Dtest=SkillRuntimeTest' '-Dsurefire.failIfNoSpecifiedTests=false' testmvn -pl agentscope-harness -am '-Dtest=SkillLoadToolFrontmatterViewTest' '-Dsurefire.failIfNoSpecifiedTests=false' test