Skip to content

fix(agui): eliminate concurrent toolkit pollution via per-call RuntimeContext toolkit - #2463

Open
xzxiaoshan wants to merge 1 commit into
agentscope-ai:mainfrom
xzxiaoshan:fix/percall-toolkit-injection
Open

fix(agui): eliminate concurrent toolkit pollution via per-call RuntimeContext toolkit#2463
xzxiaoshan wants to merge 1 commit into
agentscope-ai:mainfrom
xzxiaoshan:fix/percall-toolkit-injection

Conversation

@xzxiaoshan

Copy link
Copy Markdown

问题

AguiAgentAdapter 在运行时直接对 Agent 的共享 toolkit 执行 registerAgentTool / removeTool,再通过 ToolInjection.close() + doFinally 做"借-还"恢复。该机制只保证单流自洽——同一 Agent 单例并发执行时,各流的注入与恢复会相互污染共享 toolkit 状态。

方案

三层改动,从根源上消除并发污染(共享 toolkit 全程只读):

改动
RuntimeContext 新增 toolkit 属性(getter/setter/builder),支持运行时携带 per-call toolkit 覆盖
ReActAgent.CallExecution 新增 activeToolkit 字段,beforeAgentExecution 一次性解析(优先 RuntimeContext,fallback 共享字段),内部全部 toolkit 访问切换
AguiAgentAdapter injectFrontendToolsbuildPerCallToolkit:在 toolkit.copy() 副本上构建,设入 RuntimeContext,移除 ToolInjection 借-还机制

改动文件

  • RuntimeContext.java — 新增 toolkit 字段及 Builder 方法
  • ReActAgent.javaCallExecution 新增 activeToolkit,5 处 toolkit 引用收口
  • AguiAgentAdapter.java — 重构为 per-call 副本模式,删除 ToolInjection 内部类
  • 对应测试更新(5 个重写 + 1 个新增)

Test Plan

  • mvn test -pl agentscope-core -Dtest="ReActAgent*,Toolkit*,RuntimeContext*" — 75 passed
  • mvn test -pl agentscope-extensions-agui -Dtest=AguiAgentAdapterTest — 45 passed
  • Spotless 格式检查通过
  • 新增 perCallToolkitOverrideUsedByCallExecution 验证 RuntimeContext toolkit 在 CallExecution 层生效

…ll toolkit via RuntimeContext, which now supports runtime toolkit override
@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@CLAassistant

CLAassistant commented Jul 28, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@oss-maintainer oss-maintainer left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review: Approved ✅

This is a clean, well-architected fix for the concurrent toolkit pollution problem identified in #2459.

Design Assessment

The shift from borrow-return mutation (ToolInjection.close() + doFinally) to per-call immutable copy via RuntimeContext.toolkit is the correct approach:

  • Shared toolkit: Before = mutated per call (register/remove), After = read-only throughout
  • Concurrency safety: Before = single-flow self-consistent only, After = fully isolated per call
  • Cleanup: Before = fragile doFinally + ToolInjection.close(), After = not needed — copy is GC'd with the call
  • Frontend tool visibility: Before = leaked into shared toolkit between cleanup cycles, After = scoped to activeToolkit in CallExecution

Key Changes

  1. RuntimeContext.toolkit — Clean per-call override mechanism. Getter/setter + Builder support. null semantics (fallback to shared field) is well-documented.

  2. ReActAgent.CallExecution.activeToolkit — Resolved once in beforeAgentExecution, stable for the whole call. All internal references correctly migrated from toolkit to activeToolkit.

  3. AguiAgentAdapterToolInjection pattern fully removed. buildPerCallToolkit() deep-copies the shared toolkit and layers frontend tools on the copy. The shared toolkit is never mutated.

  4. Tests — Updated to assert toolkit.getTool("frontend_lookup") is null (shared toolkit untouched) and perCallToolkit.getTool(...) carries the frontend tool. Good coverage of the invariant.

Minor Note

CI builds are still in progress — will confirm once green.

Fixes #2459.

@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.88889% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
.../java/io/agentscope/core/agent/RuntimeContext.java 71.42% 2 Missing ⚠️
...e/src/main/java/io/agentscope/core/ReActAgent.java 85.71% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@oss-maintainer oss-maintainer left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review: fix(agui): eliminate concurrent toolkit pollution via per-call RuntimeContext toolkit

Verdict: Approved ✅

Excellent architectural improvement. This PR eliminates the root cause of cross-stream toolkit pollution by replacing the fragile borrow-restore (ToolInjection) pattern with a clean per-call copy approach.

Key changes

  1. RuntimeContext.toolkit field — new per-call toolkit override with getter/setter/builder support. null means fall back to the agent's shared field (backward compatible).
  2. CallExecution.activeToolkit — resolved once in beforeAgentExecution, stable for the entire call. All 5 toolkit access points in CallExecution now read activeToolkit instead of the shared field.
  3. AguiAgentAdapter refactorinjectFrontendToolsbuildPerCallToolkit operates on Toolkit.copy(), sets it into RuntimeContext. The entire ToolInjection inner class (borrow-restore + cleanup) is removed — 39 lines of fragile state management eliminated.
  4. from() builder method — correctly copies the toolkit field when deriving a context.

Test quality

All 5 existing tests are properly rewritten to verify shared toolkit isolation (asserting the shared toolkit is never mutated). The new perCallToolkitOverrideUsedByCallExecution test validates the end-to-end flow with a tool that only exists on the per-call toolkit.

Why this is better than the old approach

Old (ToolInjection) New (per-call copy)
Mutates shared toolkit, restores on doFinally Shared toolkit never touched
Race condition if two streams overlap Fully isolated per call
Complex cleanup logic (reverse-order remove, restore map) Zero cleanup needed
GhostToolName edge cases in cleanup Copy handles it naturally

CI all green. LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants