From 271f67ef2770216ceabfd2d44d11b47f6e42df4e Mon Sep 17 00:00:00 2001 From: reusu Date: Sat, 28 Mar 2026 23:49:46 +0800 Subject: [PATCH] feat: enable load_image for subagents via MediaResolver in RunToolLoop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- pkg/agent/loop.go | 15 +++++++++------ pkg/tools/load_image_test.go | 23 +++++++++++++++++++++++ pkg/tools/registry.go | 12 ------------ pkg/tools/subagent.go | 19 +++++++++++++++++++ pkg/tools/toolloop.go | 28 ++++++++++++++++++++++++++-- 5 files changed, 77 insertions(+), 20 deletions(-) diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index ea3a37f7a..5af562c12 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -299,6 +299,14 @@ func registerSharedTools( subagentManager := tools.NewSubagentManager(provider, agent.Model, agent.Workspace) 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 subagentManager.SetSpawner(func( ctx context.Context, @@ -363,12 +371,7 @@ func registerSharedTools( // tools registered so far (file, web, etc.) but NOT spawn/ // spawn_status which are added below — preventing recursive // subagent spawning. - subagentTools := 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) + subagentManager.SetTools(agent.Tools.Clone()) if spawnEnabled { spawnTool := tools.NewSpawnTool(subagentManager) spawnTool.SetSpawner(NewSubTurnSpawner(al)) diff --git a/pkg/tools/load_image_test.go b/pkg/tools/load_image_test.go index 33820bba7..5f9658dc7 100644 --- a/pkg/tools/load_image_test.go +++ b/pkg/tools/load_image_test.go @@ -8,6 +8,7 @@ import ( "github.com/sipeed/picoclaw/pkg/config" "github.com/sipeed/picoclaw/pkg/media" + "github.com/sipeed/picoclaw/pkg/providers" ) func TestLoadImage_PathRequired(t *testing.T) { @@ -75,3 +76,25 @@ func TestLoadImage_FileTooLarge(t *testing.T) { 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") + } +} diff --git a/pkg/tools/registry.go b/pkg/tools/registry.go index bf2adb179..56af8d695 100644 --- a/pkg/tools/registry.go +++ b/pkg/tools/registry.go @@ -395,18 +395,6 @@ func (r *ToolRegistry) Clone() *ToolRegistry { 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. func (r *ToolRegistry) Count() int { r.mu.RLock() diff --git a/pkg/tools/subagent.go b/pkg/tools/subagent.go index 9a1a8b802..ada89efb7 100644 --- a/pkg/tools/subagent.go +++ b/pkg/tools/subagent.go @@ -67,6 +67,12 @@ type SubagentManager struct { hasTemperature bool nextID int 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( @@ -90,6 +96,17 @@ func (sm *SubagentManager) SetSpawner(spawner SpawnSubTurnFunc) { 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. func (sm *SubagentManager) SetLLMOptions(maxTokens int, temperature float64) { sm.mu.Lock() @@ -177,6 +194,7 @@ func (sm *SubagentManager) runTask( temperature := sm.temperature hasMaxTokens := sm.hasMaxTokens hasTemperature := sm.hasTemperature + mediaResolver := sm.mediaResolver sm.mu.RUnlock() var result *ToolResult @@ -223,6 +241,7 @@ After completing the task, provide a clear summary of what was done.` Tools: tools, MaxIterations: maxIter, LLMOptions: llmOptions, + MediaResolver: mediaResolver, }, messages, task.OriginChannel, task.OriginChatID) if err == nil { diff --git a/pkg/tools/toolloop.go b/pkg/tools/toolloop.go index 1704efeff..d6966867f 100644 --- a/pkg/tools/toolloop.go +++ b/pkg/tools/toolloop.go @@ -24,6 +24,11 @@ type ToolLoopConfig struct { Tools *ToolRegistry MaxIterations int 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. @@ -63,8 +68,27 @@ func RunToolLoop( if llmOpts == nil { 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 { logger.ErrorCF("toolloop", "LLM call failed", map[string]any{