feat: enable load_image for subagents via MediaResolver in RunToolLoop

Instead of removing load_image from sub-agent tools (28f69e71), inject a
MediaResolver into the legacy RunToolLoop fallback path so media:// refs
are resolved to base64 before each LLM call — matching the main agent
loop behavior.

- Add MediaResolver field to ToolLoopConfig and call it on iteration > 1
- Add SubagentManager.SetMediaResolver() and wire it through runTask
- Remove ToolRegistry.Unregister() (no longer needed)
- Restore load_image in sub-agent tool set (revert Clone+Unregister)
- Add TestSubagentManager_SetMediaResolver_StoresResolver
This commit is contained in:
reusu 2026-03-28 23:49:46 +08:00
parent 28f69e71cc
commit 271f67ef27
5 changed files with 77 additions and 20 deletions

View file

@ -299,6 +299,14 @@ func registerSharedTools(
subagentManager := tools.NewSubagentManager(provider, agent.Model, agent.Workspace) subagentManager := tools.NewSubagentManager(provider, agent.Model, agent.Workspace)
subagentManager.SetLLMOptions(agent.MaxTokens, agent.Temperature) subagentManager.SetLLMOptions(agent.MaxTokens, agent.Temperature)
// Inject a media resolver so the legacy RunToolLoop fallback path can
// resolve media:// refs in the same way the main AgentLoop does.
// This keeps subagent vision support working even when the optimized
// sub-turn spawner path is unavailable.
subagentManager.SetMediaResolver(func(msgs []providers.Message) []providers.Message {
return resolveMediaRefs(msgs, al.mediaStore, cfg.Agents.Defaults.GetMaxMediaSize())
})
// Set the spawner that links into AgentLoop's turnState // Set the spawner that links into AgentLoop's turnState
subagentManager.SetSpawner(func( subagentManager.SetSpawner(func(
ctx context.Context, ctx context.Context,
@ -363,12 +371,7 @@ func registerSharedTools(
// tools registered so far (file, web, etc.) but NOT spawn/ // tools registered so far (file, web, etc.) but NOT spawn/
// spawn_status which are added below — preventing recursive // spawn_status which are added below — preventing recursive
// subagent spawning. // subagent spawning.
subagentTools := agent.Tools.Clone() subagentManager.SetTools(agent.Tools.Clone())
// load_image depends on resolveMediaRefs which only runs in
// the main agent loop, not in RunToolLoop. Remove it from
// sub-agent tools so the LLM won't call it in vain.
subagentTools.Unregister("load_image")
subagentManager.SetTools(subagentTools)
if spawnEnabled { if spawnEnabled {
spawnTool := tools.NewSpawnTool(subagentManager) spawnTool := tools.NewSpawnTool(subagentManager)
spawnTool.SetSpawner(NewSubTurnSpawner(al)) spawnTool.SetSpawner(NewSubTurnSpawner(al))

View file

@ -8,6 +8,7 @@ import (
"github.com/sipeed/picoclaw/pkg/config" "github.com/sipeed/picoclaw/pkg/config"
"github.com/sipeed/picoclaw/pkg/media" "github.com/sipeed/picoclaw/pkg/media"
"github.com/sipeed/picoclaw/pkg/providers"
) )
func TestLoadImage_PathRequired(t *testing.T) { func TestLoadImage_PathRequired(t *testing.T) {
@ -75,3 +76,25 @@ func TestLoadImage_FileTooLarge(t *testing.T) {
t.Fatal("expected error for oversized file") t.Fatal("expected error for oversized file")
} }
} }
func TestSubagentManager_SetMediaResolver_StoresResolver(t *testing.T) {
manager := NewSubagentManager(nil, "gpt-test", "/tmp")
called := false
manager.SetMediaResolver(func(msgs []providers.Message) []providers.Message {
called = true
return msgs
})
manager.mu.RLock()
got := manager.mediaResolver
manager.mu.RUnlock()
if got == nil {
t.Fatal("expected mediaResolver to be set")
}
if called {
t.Fatal("resolver should not be called during SetMediaResolver")
}
}

View file

@ -395,18 +395,6 @@ func (r *ToolRegistry) Clone() *ToolRegistry {
return clone return clone
} }
// Unregister removes a tool by name. Returns true if the tool was found and removed.
func (r *ToolRegistry) Unregister(name string) bool {
r.mu.Lock()
defer r.mu.Unlock()
if _, exists := r.tools[name]; !exists {
return false
}
delete(r.tools, name)
r.version.Add(1)
return true
}
// Count returns the number of registered tools. // Count returns the number of registered tools.
func (r *ToolRegistry) Count() int { func (r *ToolRegistry) Count() int {
r.mu.RLock() r.mu.RLock()

View file

@ -67,6 +67,12 @@ type SubagentManager struct {
hasTemperature bool hasTemperature bool
nextID int nextID int
spawner SpawnSubTurnFunc spawner SpawnSubTurnFunc
// mediaResolver resolves media:// refs in tool-loop messages before
// each LLM call in the legacy RunToolLoop fallback path.
// This lets subagents reuse the same media handling behavior as the
// main agent loop without importing pkg/agent and creating a cycle.
mediaResolver func([]providers.Message) []providers.Message
} }
func NewSubagentManager( func NewSubagentManager(
@ -90,6 +96,17 @@ func (sm *SubagentManager) SetSpawner(spawner SpawnSubTurnFunc) {
sm.spawner = spawner sm.spawner = spawner
} }
// SetMediaResolver injects a message preprocessor that resolves media:// refs
// into LLM-ready content before each tool-loop iteration.
// This is only used by the legacy RunToolLoop fallback path.
func (sm *SubagentManager) SetMediaResolver(
resolver func([]providers.Message) []providers.Message,
) {
sm.mu.Lock()
defer sm.mu.Unlock()
sm.mediaResolver = resolver
}
// SetLLMOptions sets max tokens and temperature for subagent LLM calls. // SetLLMOptions sets max tokens and temperature for subagent LLM calls.
func (sm *SubagentManager) SetLLMOptions(maxTokens int, temperature float64) { func (sm *SubagentManager) SetLLMOptions(maxTokens int, temperature float64) {
sm.mu.Lock() sm.mu.Lock()
@ -177,6 +194,7 @@ func (sm *SubagentManager) runTask(
temperature := sm.temperature temperature := sm.temperature
hasMaxTokens := sm.hasMaxTokens hasMaxTokens := sm.hasMaxTokens
hasTemperature := sm.hasTemperature hasTemperature := sm.hasTemperature
mediaResolver := sm.mediaResolver
sm.mu.RUnlock() sm.mu.RUnlock()
var result *ToolResult var result *ToolResult
@ -223,6 +241,7 @@ After completing the task, provide a clear summary of what was done.`
Tools: tools, Tools: tools,
MaxIterations: maxIter, MaxIterations: maxIter,
LLMOptions: llmOptions, LLMOptions: llmOptions,
MediaResolver: mediaResolver,
}, messages, task.OriginChannel, task.OriginChatID) }, messages, task.OriginChannel, task.OriginChatID)
if err == nil { if err == nil {

View file

@ -24,6 +24,11 @@ type ToolLoopConfig struct {
Tools *ToolRegistry Tools *ToolRegistry
MaxIterations int MaxIterations int
LLMOptions map[string]any LLMOptions map[string]any
// MediaResolver resolves media:// refs in messages before each LLM call.
// This is optional and is mainly used by subagent legacy fallback execution
// so subagents can reuse the same multimodal media handling as the main loop.
MediaResolver func(messages []providers.Message) []providers.Message
} }
// ToolLoopResult contains the result of running the tool loop. // ToolLoopResult contains the result of running the tool loop.
@ -63,8 +68,27 @@ func RunToolLoop(
if llmOpts == nil { if llmOpts == nil {
llmOpts = map[string]any{} llmOpts = map[string]any{}
} }
// 3. Call LLM
response, err := config.Provider.Chat(ctx, messages, providerToolDefs, config.Model, llmOpts) // 3. Resolve media:// refs and Call LLM.
// Tools like load_image produce media:// refs in their result messages.
// Without this step, the LLM would receive raw "media://uuid" strings
// instead of base64-encoded image data URLs.
//
// We build a separate callMessages slice so that:
// (a) the resolver output is used for the LLM call only,
// (b) the original `messages` slice keeps the unresolved refs for
// subsequent iterations — the resolver is idempotent but working
// on the original avoids double-encoding issues.
//
// On iteration 1 the initial user messages typically have no media://
// refs (they come from plain text), so this is effectively a no-op;
// it becomes relevant from iteration 2 onward when tool results may
// contain media refs.
callMessages := messages
if config.MediaResolver != nil && iteration > 1 {
callMessages = config.MediaResolver(messages)
}
response, err := config.Provider.Chat(ctx, callMessages, providerToolDefs, config.Model, llmOpts)
if err != nil { if err != nil {
logger.ErrorCF("toolloop", "LLM call failed", logger.ErrorCF("toolloop", "LLM call failed",
map[string]any{ map[string]any{