agentscope-ai / agentscope-ai/agentscope-java

[Bug]: ReActAgent.Builder.build() toolkit deep copy causes dynamically-registered middleware tools to be missing from the agent's actual toolkit

Abierto
#1,606 6 comentarios 0 reacciones 0 asignados Ver en GitHub
area/core/agent area/harness bug
Lenguaje dominante
Java
Estrellas
5.6k
Forks
1.3k
Merge medio
4 d 12 h
PR fusionados (30 d)
77

Descripción

### Summary

`ReActAgent.Builder.build()` performs a defensive `toolkit.copy()` (line 3077), which creates a second copy of the toolkit. When a middleware (e.g., `HarnessSkillMiddleware`) holds a reference to the first copy and dynamically registers tools onto it during `call()`, those tools are invisible to the agent because the agent uses the second copy. This results in `Tool not found` errors at runtime.

### Environment

- AgentScope Java: current main
- Component: `agentscope-core` (`ReActAgent`), `agentscope-harness` (`HarnessAgent`, `HarnessSkillMiddleware`)

### Root Cause

In `HarnessAgent.Builder.build()` (HarnessAgent.java ~L1520-1902):

```java
// Step 1: First defensive copy
Toolkit agentToolkit = this.toolkit.copy(); // ← Copy 1

// Step 2: Middleware receives Copy 1
inner.middleware(
new HarnessSkillMiddleware(
orderedSkillRepos,
agentToolkit, // ← Copy 1 passed to middleware
skillFilter, ...));

// Step 3: Pass Copy 1 to inner builder
inner.toolkit(agentToolkit); // ← Copy 1

// Step 4: inner.build() does ANOTHER copy
ReActAgent delegate = inner.build();
// → ReActAgent.Builder.build() line 3077:
// Toolkit agentToolkit = this.toolkit.copy(); // ← Copy 2 (the one the agent actually uses)
```

The lifecycle at `call()` time:

```
agent.call()
→ seedSystemMsg()
→ applySystemPromptMiddlewares()
→ HarnessSkillMiddleware.onSystemPrompt()
→ runtime.install(catalog, toolkit)
→ toolkit.registerAgentTool(loadTool) // ← Registered on Copy 1
→ ...
→ toolkit.getToolSchemas() // ← Reads from Copy 2 → load_skill_through_path is MISSING
→ LLM generates tool_call: load_skill_through_path
→ Toolkit.getTool("load_skill_through_path") // ← Looks up in Copy 2 → returns null
→ "Error: Tool not found: load_skill_through_path"
```

### Reproduction Steps

1. Build a `HarnessAgent` with `skillRepository(...)` configured
2. Call `agent.call()` with a user message
3. The LLM sees the `` prompt block (rendered by middleware) and generates a `load_skill_through_path` tool call
4. The framework returns `"Error: Tool not found: load_skill_through_path"`

### Debug Evidence

Setting breakpoints at the following locations confirms the issue:

| # | Class | Line | Observation |
|---|-------|------|-------------|
| 1 | `HarnessSkillMiddleware` | `mergeRepositories()` | `repo.getAllSkills()` returns 2 skills — **correct** |
| 2 | `HarnessSkillMiddleware` | `onSystemPrompt()` ~L177 | `catalog.ids()` contains 2 skill IDs — **correct** |
| 3 | `SkillLoadTool` | `getParameters()` | **Breakpoint never hit** — `ToolSchemaProvider` uses `registered.getExtendedParameters()` (captured at registration time), not `tool.getParameters()` |
| 4 | `ToolSchemaProvider` | `getToolSchemas()` | Returns 17 tools — `load_skill_through_path` is **absent** |
| 5 | `ReActAgent` | ~L1045 | `toolkit.getToolSchemas()` returns 17 tools — `load_skill_through_path` is **absent** |
| 6 | `Toolkit` | `getTool("load_skill_through_path")` | `toolRegistry` has 17 tools, `load_skill_through_path` is **not among them** |

The middleware registers `load_skill_through_path` on Copy 1, but the agent's `toolkit` (Copy 2) never receives it.

### Impact

- Any middleware that dynamically registers tools during `onSystemPrompt()` or `onReasoning()` is affected, not just `HarnessSkillMiddleware`
- The `DynamicSkillMiddleware` in `agentscope-core` (ReActAgent.java ~L3104-3110) has the same problem — it receives `agentToolkit` (which is already a copy) and registers tools onto it, but the agent uses a further copy
- This makes the entire skill-loading feature non-functional for `HarnessAgent`

### Suggested Fix

**Option A (Recommended):** In `ReActAgent.Builder.build()`, after creating the deep copy, propagate the copy back to any middleware that holds a reference to the old toolkit. For example, add a `rebindToolkit(Toolkit newToolkit)` method to `MiddlewareBase` (default no-op), and call it for each middleware after the copy:

```java
// ReActAgent.Builder.build()
Toolkit agentToolkit = this.toolkit.copy();

// Rebind middlewares to the new toolkit copy
for (MiddlewareBase mw : middlewares) {
mw.rebindToolkit(agentToolkit); // default no-op, HarnessSkillMiddleware overrides
}
```

**Option B:** In `HarnessAgent.Builder.build()`, skip the first copy and pass the builder's toolkit directly to `inner`, letting `ReActAgent.Builder.build()` be the single point of deep copy. Then pass the same `agentToolkit` reference (the one from `inner.build()`) to the middleware. This requires restructuring the build sequence.

**Option C (Minimal):** In `HarnessSkillMiddleware.onSystemPrompt()`, instead of registering the tool on the toolkit passed at construction time, resolve the agent's actual toolkit from the `Agent` parameter and register there:

```java
@Override
public Mono onSystemPrompt(Agent agent, String currentPrompt) {
// Resolve the agent's actual toolkit instead of using the construction-time reference
Toolkit actualToolkit = resolveToolkit(agent);
runtime.install(catalog, actualToolkit);
...
}
```

This requires `AgentBase` or `ReActAgent` to expose a `getToolkit()` method (which `HarnessAgent` already does, but `Agent` interface does not).

### Workaround

We currently work around this by using reflection after `agentBuilder.build()` to extract the `SkillLoadTool` from the middleware's `SkillRuntime` and register it onto the agent's actual toolkit:

```java
HarnessAgent agent = agentBuilder.build();
fixSkillLoadToolAfterBuild(agent);

private void fixSkillLoadToolAfterBuild(HarnessAgent agent) {
ReActAgent delegate = agent.getDelegate();
Field middlewaresField = ReActAgent.class.getDeclaredField("middlewares");
middlewaresField.setAccessible(true);
List middlewares = (List) middlewaresField.get(delegate);
for (MiddlewareBase mw : middlewares) {
if (mw.getClass().getSimpleName().equals("HarnessSkillMiddleware")) {
Method runtimeMethod = mw.getClass().getMethod("runtime");
Object runtime = runtimeMethod.invoke(mw);
Method loadToolMethod = runtime.getClass().getMethod("loadTool");
Object loadTool = loadToolMethod.invoke(runtime);
Toolkit actualToolkit = agent.getToolkit();
if (actualToolkit.getTool("load_skill_through_path") == null) {
actualToolkit.registerAgentTool((AgentTool) loadTool);
}
return;
}
}
}
```

This works because `SkillLoadTool` holds an `AtomicReference` shared with the middleware, so catalog updates from subsequent `onSystemPrompt()` calls are automatically visible.

Guía de contribución

Abrir la guía de contribución

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.