diff --git a/cmd/picoclaw/internal/onboard/command.go b/cmd/picoclaw/internal/onboard/command.go index bf8f4104..4fb5b8e2 100644 --- a/cmd/picoclaw/internal/onboard/command.go +++ b/cmd/picoclaw/internal/onboard/command.go @@ -1,14 +1,12 @@ package onboard import ( - "embed" - "github.com/spf13/cobra" + + picoclaw "github.com/sipeed/picoclaw" ) -//go:generate go run ../../../../scripts/copydir.go ../../../../workspace ./workspace -//go:embed workspace -var embeddedFiles embed.FS +var embeddedFiles = picoclaw.OnboardWorkspace func NewOnboardCommand() *cobra.Command { var encrypt bool diff --git a/onboard_workspace_embed.go b/onboard_workspace_embed.go new file mode 100644 index 00000000..fcedcbba --- /dev/null +++ b/onboard_workspace_embed.go @@ -0,0 +1,12 @@ +package picoclaw + +import "embed" + +// OnboardWorkspace embeds the default onboarding workspace template. +// +// Keeping this embed at the module root lets us source files directly from the +// tracked `workspace/` tree, instead of relying on a generated copy inside +// `cmd/...` that may be absent in clean checkouts and CI lint runs. +// +//go:embed workspace +var OnboardWorkspace embed.FS diff --git a/pkg/agent/agent_init.go b/pkg/agent/agent_init.go index 17629892..23fd21b5 100644 --- a/pkg/agent/agent_init.go +++ b/pkg/agent/agent_init.go @@ -296,7 +296,7 @@ func registerSharedTools( // 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()) + return resolveMediaRefs(msgs, al.mediaStore, cfg.Agents.Defaults.GetMaxMediaSize(), 0) }) // Set the spawner that links into AgentLoop's turnState diff --git a/pkg/agent/agent_media.go b/pkg/agent/agent_media.go index c02c7392..c15e8c16 100644 --- a/pkg/agent/agent_media.go +++ b/pkg/agent/agent_media.go @@ -31,6 +31,21 @@ var ( filePlaceholderRegex = regexp.MustCompile(`\[file(:\s+[^\]]*)?\]`) ) +func normalizeCurrentTurnStart(messages []providers.Message, currentTurnStart int) int { + if currentTurnStart < 0 { + return 0 + } + if currentTurnStart > len(messages) { + return len(messages) + } + return currentTurnStart +} + +func currentTurnMessages(messages []providers.Message, currentTurnStart int) []providers.Message { + currentTurnStart = normalizeCurrentTurnStart(messages, currentTurnStart) + return messages[currentTurnStart:] +} + // resolveMediaRefs resolves media:// refs in messages. // For user messages: images get path tags only ([image:/path]) so the LLM // can decide whether to view them via load_image or operate on the file. @@ -38,12 +53,20 @@ var ( // user message only after the contiguous tool-message block ends, so we don't // break the tool-results-must-immediately-follow-assistant constraint that // LLM APIs enforce. +// Only tool messages from the current turn may emit the synthetic user +// follow-up; historical tool results stay as plain path-tagged history. // Non-image files always get path tags regardless of role. // Returns a new slice; original messages are not mutated. -func resolveMediaRefs(messages []providers.Message, store media.MediaStore, maxSize int) []providers.Message { +func resolveMediaRefs( + messages []providers.Message, + store media.MediaStore, + maxSize int, + currentTurnStart int, +) []providers.Message { if store == nil { return messages } + currentTurnStart = normalizeCurrentTurnStart(messages, currentTurnStart) result := make([]providers.Message, 0, len(messages)) var pendingToolImages []string @@ -52,22 +75,14 @@ func resolveMediaRefs(messages []providers.Message, store media.MediaStore, maxS // When leaving a tool-message block, flush any accumulated images // as a synthetic user message. if m.Role != "tool" && len(pendingToolImages) > 0 { - result = append(result, providers.Message{ - Role: "user", - Content: "[Loaded image from tool result above]", - Media: pendingToolImages, - }) + result = append(result, toolImageFollowUpPromptMessage(pendingToolImages)) pendingToolImages = nil } if len(m.Media) == 0 { result = append(result, m) if idx == len(messages)-1 && len(pendingToolImages) > 0 { - result = append(result, providers.Message{ - Role: "user", - Content: "[Loaded image from tool result above]", - Media: pendingToolImages, - }) + result = append(result, toolImageFollowUpPromptMessage(pendingToolImages)) pendingToolImages = nil } continue @@ -104,7 +119,7 @@ func resolveMediaRefs(messages []providers.Message, store media.MediaStore, maxS mime := detectMIME(localPath, meta) pathTags = append(pathTags, buildPathTag(mime, localPath)) - if m.Role == "tool" && strings.HasPrefix(mime, "image/") { + if m.Role == "tool" && idx >= currentTurnStart && strings.HasPrefix(mime, "image/") { dataURL := encodeImageToDataURL(localPath, mime, info, maxSize) if dataURL != "" { pendingToolImages = append(pendingToolImages, dataURL) @@ -120,11 +135,7 @@ func resolveMediaRefs(messages []providers.Message, store media.MediaStore, maxS // If this is the last message and we have pending images, flush them. if idx == len(messages)-1 && len(pendingToolImages) > 0 { - result = append(result, providers.Message{ - Role: "user", - Content: "[Loaded image from tool result above]", - Media: pendingToolImages, - }) + result = append(result, toolImageFollowUpPromptMessage(pendingToolImages)) pendingToolImages = nil } } diff --git a/pkg/agent/agent_test.go b/pkg/agent/agent_test.go index c3562bda..db616ad3 100644 --- a/pkg/agent/agent_test.go +++ b/pkg/agent/agent_test.go @@ -4401,7 +4401,250 @@ func (p *visionUnsupportedMediaProvider) GetDefaultModel() string { return "mock-fail-model" } -func TestAgentLoop_VisionUnsupportedErrorStripsSessionMedia(t *testing.T) { +type loadImagePlanningProvider struct { + path string + followUpErr error + calls int + models []string +} + +func (p *loadImagePlanningProvider) Chat( + ctx context.Context, + messages []providers.Message, + tools []providers.ToolDefinition, + model string, + opts map[string]any, +) (*providers.LLMResponse, error) { + p.calls++ + p.models = append(p.models, model) + + if p.calls == 1 { + return &providers.LLMResponse{ + Content: "Let me inspect the image.", + ToolCalls: []providers.ToolCall{{ + ID: "call_load_image_test", + Type: "function", + Name: "load_image", + Arguments: map[string]any{"path": p.path}, + }}, + }, nil + } + + if p.followUpErr != nil { + return nil, p.followUpErr + } + + return nil, fmt.Errorf("load_image follow-up should not be handled by the text model") +} + +func (p *loadImagePlanningProvider) GetDefaultModel() string { + return "load-image-planner" +} + +type visionAnswerProvider struct { + calls int + models []string + mediaSeen []bool +} + +func (p *visionAnswerProvider) Chat( + ctx context.Context, + messages []providers.Message, + tools []providers.ToolDefinition, + model string, + opts map[string]any, +) (*providers.LLMResponse, error) { + p.calls++ + p.models = append(p.models, model) + + hasMedia := false + for _, msg := range messages { + for _, ref := range msg.Media { + if strings.TrimSpace(ref) != "" { + hasMedia = true + break + } + } + if hasMedia { + break + } + } + p.mediaSeen = append(p.mediaSeen, hasMedia) + if !hasMedia { + return nil, fmt.Errorf("vision provider expected image media in follow-up request") + } + + return &providers.LLMResponse{ + Content: "vision answer", + ToolCalls: []providers.ToolCall{}, + }, nil +} + +func (p *visionAnswerProvider) GetDefaultModel() string { + return "vision-answer-model" +} + +type resolvedImagePathVisionProvider struct { + expectedPath string + calls int + models []string + pathTagSeen []bool + mediaSeen []bool +} + +func (p *resolvedImagePathVisionProvider) Chat( + ctx context.Context, + messages []providers.Message, + tools []providers.ToolDefinition, + model string, + opts map[string]any, +) (*providers.LLMResponse, error) { + p.calls++ + p.models = append(p.models, model) + + pathTag := "[image:" + p.expectedPath + "]" + hasPathTag := false + hasMedia := false + for _, msg := range messages { + if strings.Contains(msg.Content, pathTag) { + hasPathTag = true + } + for _, ref := range msg.Media { + if strings.TrimSpace(ref) != "" { + hasMedia = true + break + } + } + } + p.pathTagSeen = append(p.pathTagSeen, hasPathTag) + p.mediaSeen = append(p.mediaSeen, hasMedia) + + if !hasPathTag { + return nil, fmt.Errorf("vision provider expected resolved image path tag %q", pathTag) + } + if hasMedia { + return nil, fmt.Errorf("vision provider expected resolved attachment turn without raw media refs") + } + + return &providers.LLMResponse{ + Content: "vision direct answer", + ToolCalls: []providers.ToolCall{}, + }, nil +} + +func (p *resolvedImagePathVisionProvider) GetDefaultModel() string { + return "resolved-image-path-vision-model" +} + +type unexpectedTextAttachmentProvider struct { + calls int +} + +func (p *unexpectedTextAttachmentProvider) Chat( + ctx context.Context, + messages []providers.Message, + tools []providers.ToolDefinition, + model string, + opts map[string]any, +) (*providers.LLMResponse, error) { + p.calls++ + return &providers.LLMResponse{ + Content: "text model response", + ToolCalls: []providers.ToolCall{}, + }, nil +} + +func (p *unexpectedTextAttachmentProvider) GetDefaultModel() string { + return "unexpected-text-attachment-model" +} + +type unexpectedVisionProvider struct { + calls int + models []string +} + +func (p *unexpectedVisionProvider) Chat( + ctx context.Context, + messages []providers.Message, + tools []providers.ToolDefinition, + model string, + opts map[string]any, +) (*providers.LLMResponse, error) { + p.calls++ + p.models = append(p.models, model) + return nil, fmt.Errorf("vision provider should not be called for this turn") +} + +func (p *unexpectedVisionProvider) GetDefaultModel() string { + return "unexpected-vision-model" +} + +type loadImageThenTextFollowUpProvider struct { + path string + calls int + models []string + mediaSeen []bool + syntheticMsgSeen []bool +} + +func (p *loadImageThenTextFollowUpProvider) Chat( + ctx context.Context, + messages []providers.Message, + tools []providers.ToolDefinition, + model string, + opts map[string]any, +) (*providers.LLMResponse, error) { + p.calls++ + p.models = append(p.models, model) + + hasMedia := false + hasSynthetic := false + for _, msg := range messages { + if msg.Content == "[Loaded image from tool result above]" { + hasSynthetic = true + } + for _, ref := range msg.Media { + if strings.TrimSpace(ref) != "" { + hasMedia = true + break + } + } + } + p.mediaSeen = append(p.mediaSeen, hasMedia) + p.syntheticMsgSeen = append(p.syntheticMsgSeen, hasSynthetic) + + switch p.calls { + case 1: + return &providers.LLMResponse{ + Content: "Let me inspect the image.", + ToolCalls: []providers.ToolCall{{ + ID: "call_load_image_regression", + Type: "function", + Name: "load_image", + Arguments: map[string]any{"path": p.path}, + }}, + }, nil + case 2: + if hasMedia { + return nil, fmt.Errorf("text-only follow-up unexpectedly retained media from prior turn") + } + if hasSynthetic { + return nil, fmt.Errorf("text-only follow-up unexpectedly rebuilt synthetic tool image message from history") + } + return &providers.LLMResponse{ + Content: "text follow-up", + ToolCalls: []providers.ToolCall{}, + }, nil + default: + return nil, fmt.Errorf("unexpected extra text-model call %d", p.calls) + } +} + +func (p *loadImageThenTextFollowUpProvider) GetDefaultModel() string { + return "load-image-then-text-follow-up-model" +} + +func TestAgentLoop_VisionUnsupportedErrorReturnsClearFailure(t *testing.T) { workspace := t.TempDir() cfg := &config.Config{ @@ -4436,17 +4679,20 @@ func TestAgentLoop_VisionUnsupportedErrorStripsSessionMedia(t *testing.T) { Media: []string{"data:image/png;base64,abc123"}, SessionKey: sessionKey, })) - if err != nil { - t.Fatalf("processMessage() error = %v", err) + if err == nil { + t.Fatal("processMessage() error = nil, want vision unsupported failure") } - if resp != "ok" { - t.Fatalf("response = %q, want %q", resp, "ok") + if resp != "" { + t.Fatalf("response = %q, want empty response on error", resp) } - if provider.calls != 2 { - t.Fatalf("calls = %d, want %d (fail with media, then retry without media)", provider.calls, 2) + if !strings.Contains(err.Error(), `active model "test-model" does not support image input`) { + t.Fatalf("error = %q, want clear vision unsupported guidance", err.Error()) } - if !slices.Equal(provider.mediaSeen, []bool{true, false}) { - t.Fatalf("mediaSeen = %v, want %v", provider.mediaSeen, []bool{true, false}) + if provider.calls != 1 { + t.Fatalf("calls = %d, want %d (no retry without media)", provider.calls, 1) + } + if !slices.Equal(provider.mediaSeen, []bool{true}) { + t.Fatalf("mediaSeen = %v, want %v", provider.mediaSeen, []bool{true}) } agent := al.registry.GetDefaultAgent() @@ -4454,10 +4700,172 @@ func TestAgentLoop_VisionUnsupportedErrorStripsSessionMedia(t *testing.T) { t.Fatal("expected default agent") } history := agent.Sessions.GetHistory(sessionKey) - for i, msg := range history { - if len(msg.Media) > 0 { - t.Fatalf("history[%d].Media = %v, want no media after stripping", i, msg.Media) - } + if len(history) == 0 { + t.Fatal("expected user message to remain in session history") + } + if len(history[0].Media) == 0 { + t.Fatalf("history[0].Media = %v, want original media preserved", history[0].Media) + } +} + +func TestAgentLoop_UserAttachmentRoutesToImageModelAfterMediaResolution(t *testing.T) { + workspace := t.TempDir() + pngPath := filepath.Join(workspace, "sample.png") + pngBytes := []byte{ + 0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A, + 0x00, 0x00, 0x00, 0x0D, 0x49, 0x48, 0x44, 0x52, + 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x08, 0x02, + 0x00, 0x00, 0x00, 0x90, 0x77, 0x53, 0xDE, + } + if err := os.WriteFile(pngPath, pngBytes, 0o644); err != nil { + t.Fatalf("WriteFile() error = %v", err) + } + + cfg := &config.Config{ + Agents: config.AgentsConfig{ + Defaults: config.AgentDefaults{ + Workspace: workspace, + ModelName: "text-model", + ImageModel: "vision-model", + MaxTokens: 4096, + MaxToolIterations: 3, + }, + }, + ModelList: []*config.ModelConfig{ + {ModelName: "text-model", Model: "openai/text-model"}, + {ModelName: "vision-model", Model: "openai/vision-model"}, + }, + } + + msgBus := bus.NewMessageBus() + textProvider := &unexpectedTextAttachmentProvider{} + al := NewAgentLoop(cfg, msgBus, textProvider) + + store := media.NewFileMediaStore() + al.SetMediaStore(store) + ref, err := store.Store(pngPath, media.MediaMeta{ContentType: "image/png"}, "test:user-attachment") + if err != nil { + t.Fatalf("Store() error = %v", err) + } + + agent := al.registry.GetDefaultAgent() + if agent == nil { + t.Fatal("expected default agent") + } + if len(agent.ImageCandidates) != 1 { + t.Fatalf("len(ImageCandidates) = %d, want 1", len(agent.ImageCandidates)) + } + + visionProvider := &resolvedImagePathVisionProvider{expectedPath: pngPath} + agent.CandidateProviders[providers.ModelKey("openai", "vision-model")] = visionProvider + + timeoutCtx, cancel := context.WithTimeout(context.Background(), responseTimeout) + defer cancel() + + resp, err := al.processMessage(timeoutCtx, testInboundMessage(bus.InboundMessage{ + Context: bus.InboundContext{ + Channel: "telegram", + ChatID: "chat1", + ChatType: "direct", + SenderID: "user1", + MessageID: "m1", + }, + Content: "describe this image", + Media: []string{ref}, + SessionKey: "agent:main:telegram:direct:user1", + })) + if err != nil { + t.Fatalf("processMessage() error = %v", err) + } + if resp != "vision direct answer" { + t.Fatalf("response = %q, want %q", resp, "vision direct answer") + } + if textProvider.calls != 0 { + t.Fatalf("textProvider calls = %d, want 0", textProvider.calls) + } + if visionProvider.calls != 1 { + t.Fatalf("visionProvider calls = %d, want %d", visionProvider.calls, 1) + } + if !slices.Equal(visionProvider.models, []string{"vision-model"}) { + t.Fatalf("visionProvider models = %v, want %v", visionProvider.models, []string{"vision-model"}) + } + if !slices.Equal(visionProvider.pathTagSeen, []bool{true}) { + t.Fatalf("visionProvider pathTagSeen = %v, want %v", visionProvider.pathTagSeen, []bool{true}) + } + if !slices.Equal(visionProvider.mediaSeen, []bool{false}) { + t.Fatalf("visionProvider mediaSeen = %v, want %v", visionProvider.mediaSeen, []bool{false}) + } +} + +func TestAgentLoop_TextFollowUpAfterUserAttachmentStaysOnTextModel(t *testing.T) { + workspace := t.TempDir() + pngPath := filepath.Join(workspace, "sample.png") + pngBytes := []byte{ + 0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A, + 0x00, 0x00, 0x00, 0x0D, 0x49, 0x48, 0x44, 0x52, + 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x08, 0x02, + 0x00, 0x00, 0x00, 0x90, 0x77, 0x53, 0xDE, + } + if err := os.WriteFile(pngPath, pngBytes, 0o644); err != nil { + t.Fatalf("WriteFile() error = %v", err) + } + + cfg := &config.Config{ + Agents: config.AgentsConfig{ + Defaults: config.AgentDefaults{ + Workspace: workspace, + ModelName: "text-model", + ImageModel: "vision-model", + MaxTokens: 4096, + MaxToolIterations: 3, + }, + }, + ModelList: []*config.ModelConfig{ + {ModelName: "text-model", Model: "openai/text-model"}, + {ModelName: "vision-model", Model: "openai/vision-model"}, + }, + } + + msgBus := bus.NewMessageBus() + textProvider := &unexpectedTextAttachmentProvider{} + al := NewAgentLoop(cfg, msgBus, textProvider) + + store := media.NewFileMediaStore() + al.SetMediaStore(store) + ref, err := store.Store(pngPath, media.MediaMeta{ContentType: "image/png"}, "test:user-attachment-followup") + if err != nil { + t.Fatalf("Store() error = %v", err) + } + + agent := al.registry.GetDefaultAgent() + if agent == nil { + t.Fatal("expected default agent") + } + + visionProvider := &resolvedImagePathVisionProvider{expectedPath: pngPath} + agent.CandidateProviders[providers.ModelKey("openai", "vision-model")] = visionProvider + + sessionKey := "agent:main:telegram:direct:user1" + timeoutCtx, cancel := context.WithTimeout(context.Background(), responseTimeout) + defer cancel() + + resp1, err := al.processMessage(timeoutCtx, testInboundMessage(bus.InboundMessage{ + Context: bus.InboundContext{ + Channel: "telegram", + ChatID: "chat1", + ChatType: "direct", + SenderID: "user1", + MessageID: "m1", + }, + Content: "describe this image", + Media: []string{ref}, + SessionKey: sessionKey, + })) + if err != nil { + t.Fatalf("first processMessage() error = %v", err) + } + if resp1 != "vision direct answer" { + t.Fatalf("first response = %q, want %q", resp1, "vision direct answer") } timeoutCtx2, cancel2 := context.WithTimeout(context.Background(), responseTimeout) @@ -4471,20 +4879,336 @@ func TestAgentLoop_VisionUnsupportedErrorStripsSessionMedia(t *testing.T) { SenderID: "user1", MessageID: "m2", }, - Content: "hello again", + Content: "now summarize it in one sentence", SessionKey: sessionKey, })) if err != nil { - t.Fatalf("processMessage() second call error = %v", err) + t.Fatalf("second processMessage() error = %v", err) } - if resp2 != "ok" { - t.Fatalf("second response = %q, want %q", resp2, "ok") + if resp2 != "text model response" { + t.Fatalf("second response = %q, want %q", resp2, "text model response") } - if provider.calls != 3 { - t.Fatalf("calls after second turn = %d, want %d", provider.calls, 3) + if textProvider.calls != 1 { + t.Fatalf("textProvider calls = %d, want 1", textProvider.calls) } - if !slices.Equal(provider.mediaSeen, []bool{true, false, false}) { - t.Fatalf("mediaSeen = %v, want %v", provider.mediaSeen, []bool{true, false, false}) + if visionProvider.calls != 1 { + t.Fatalf("visionProvider calls = %d, want 1", visionProvider.calls) + } +} + +func TestAgentLoop_GenericImagePlaceholderDoesNotRouteToImageModel(t *testing.T) { + workspace := t.TempDir() + + cfg := &config.Config{ + Agents: config.AgentsConfig{ + Defaults: config.AgentDefaults{ + Workspace: workspace, + ModelName: "text-model", + ImageModel: "vision-model", + MaxTokens: 4096, + MaxToolIterations: 3, + }, + }, + ModelList: []*config.ModelConfig{ + {ModelName: "text-model", Model: "openai/text-model"}, + {ModelName: "vision-model", Model: "openai/vision-model"}, + }, + } + + msgBus := bus.NewMessageBus() + textProvider := &unexpectedTextAttachmentProvider{} + al := NewAgentLoop(cfg, msgBus, textProvider) + + agent := al.registry.GetDefaultAgent() + if agent == nil { + t.Fatal("expected default agent") + } + + visionProvider := &unexpectedVisionProvider{} + agent.CandidateProviders[providers.ModelKey("openai", "vision-model")] = visionProvider + + timeoutCtx, cancel := context.WithTimeout(context.Background(), responseTimeout) + defer cancel() + + resp, err := al.processMessage(timeoutCtx, testInboundMessage(bus.InboundMessage{ + Context: bus.InboundContext{ + Channel: "telegram", + ChatID: "chat1", + ChatType: "direct", + SenderID: "user1", + MessageID: "m1", + }, + Content: "this is only a placeholder [image: photo], answer in text", + SessionKey: "agent:main:telegram:direct:user1", + })) + if err != nil { + t.Fatalf("processMessage() error = %v", err) + } + if resp != "text model response" { + t.Fatalf("response = %q, want %q", resp, "text model response") + } + if textProvider.calls != 1 { + t.Fatalf("textProvider calls = %d, want 1", textProvider.calls) + } + if visionProvider.calls != 0 { + t.Fatalf("visionProvider calls = %d, want 0", visionProvider.calls) + } +} + +func TestAgentLoop_TextFollowUpAfterLoadImageStaysOnTextModel(t *testing.T) { + workspace := t.TempDir() + pngPath := filepath.Join(workspace, "sample.png") + pngBytes := []byte{ + 0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A, + 0x00, 0x00, 0x00, 0x0D, 0x49, 0x48, 0x44, 0x52, + 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x08, 0x02, + 0x00, 0x00, 0x00, 0x90, 0x77, 0x53, 0xDE, + } + if err := os.WriteFile(pngPath, pngBytes, 0o644); err != nil { + t.Fatalf("WriteFile() error = %v", err) + } + + cfg := &config.Config{ + Agents: config.AgentsConfig{ + Defaults: config.AgentDefaults{ + Workspace: workspace, + ModelName: "text-model", + ImageModel: "vision-model", + MaxTokens: 4096, + MaxToolIterations: 3, + }, + }, + Tools: config.ToolsConfig{ + LoadImage: config.ToolConfig{Enabled: true}, + }, + ModelList: []*config.ModelConfig{ + {ModelName: "text-model", Model: "openai/text-model"}, + {ModelName: "vision-model", Model: "openai/vision-model"}, + }, + } + + msgBus := bus.NewMessageBus() + textProvider := &loadImageThenTextFollowUpProvider{path: pngPath} + al := NewAgentLoop(cfg, msgBus, textProvider) + al.SetMediaStore(media.NewFileMediaStore()) + + agent := al.registry.GetDefaultAgent() + if agent == nil { + t.Fatal("expected default agent") + } + if len(agent.ImageCandidates) != 1 { + t.Fatalf("len(ImageCandidates) = %d, want 1", len(agent.ImageCandidates)) + } + + visionProvider := &visionAnswerProvider{} + agent.CandidateProviders[providers.ModelKey("openai", "vision-model")] = visionProvider + + sessionKey := "agent:main:telegram:direct:user1" + timeoutCtx, cancel := context.WithTimeout(context.Background(), responseTimeout) + defer cancel() + + resp1, err := al.processMessage(timeoutCtx, testInboundMessage(bus.InboundMessage{ + Context: bus.InboundContext{ + Channel: "telegram", + ChatID: "chat1", + ChatType: "direct", + SenderID: "user1", + MessageID: "m1", + }, + Content: "describe the image you load", + SessionKey: sessionKey, + })) + if err != nil { + t.Fatalf("first processMessage() error = %v", err) + } + if resp1 != "vision answer" { + t.Fatalf("first response = %q, want %q", resp1, "vision answer") + } + + timeoutCtx2, cancel2 := context.WithTimeout(context.Background(), responseTimeout) + defer cancel2() + + resp2, err := al.processMessage(timeoutCtx2, testInboundMessage(bus.InboundMessage{ + Context: bus.InboundContext{ + Channel: "telegram", + ChatID: "chat1", + ChatType: "direct", + SenderID: "user1", + MessageID: "m2", + }, + Content: "now summarize it in one sentence", + SessionKey: sessionKey, + })) + if err != nil { + t.Fatalf("second processMessage() error = %v", err) + } + if resp2 != "text follow-up" { + t.Fatalf("second response = %q, want %q", resp2, "text follow-up") + } + if textProvider.calls != 2 { + t.Fatalf("textProvider calls = %d, want %d", textProvider.calls, 2) + } + if !slices.Equal(textProvider.models, []string{"text-model", "text-model"}) { + t.Fatalf("textProvider models = %v, want %v", textProvider.models, []string{"text-model", "text-model"}) + } + if !slices.Equal(textProvider.mediaSeen, []bool{false, false}) { + t.Fatalf("textProvider mediaSeen = %v, want %v", textProvider.mediaSeen, []bool{false, false}) + } + if !slices.Equal(textProvider.syntheticMsgSeen, []bool{false, false}) { + t.Fatalf("textProvider syntheticMsgSeen = %v, want %v", textProvider.syntheticMsgSeen, []bool{false, false}) + } + if visionProvider.calls != 1 { + t.Fatalf("visionProvider calls = %d, want %d", visionProvider.calls, 1) + } +} + +func TestAgentLoop_LoadImageFollowUpRoutesToImageModel(t *testing.T) { + workspace := t.TempDir() + pngPath := filepath.Join(workspace, "sample.png") + pngBytes := []byte{ + 0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A, + 0x00, 0x00, 0x00, 0x0D, 0x49, 0x48, 0x44, 0x52, + 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x08, 0x02, + 0x00, 0x00, 0x00, 0x90, 0x77, 0x53, 0xDE, + } + if err := os.WriteFile(pngPath, pngBytes, 0o644); err != nil { + t.Fatalf("WriteFile() error = %v", err) + } + + cfg := &config.Config{ + Agents: config.AgentsConfig{ + Defaults: config.AgentDefaults{ + Workspace: workspace, + ModelName: "text-model", + ImageModel: "vision-model", + MaxTokens: 4096, + MaxToolIterations: 3, + }, + }, + Tools: config.ToolsConfig{ + LoadImage: config.ToolConfig{Enabled: true}, + }, + ModelList: []*config.ModelConfig{ + {ModelName: "text-model", Model: "openai/text-model"}, + {ModelName: "vision-model", Model: "openai/vision-model"}, + }, + } + + msgBus := bus.NewMessageBus() + planner := &loadImagePlanningProvider{path: pngPath} + al := NewAgentLoop(cfg, msgBus, planner) + al.SetMediaStore(media.NewFileMediaStore()) + + agent := al.registry.GetDefaultAgent() + if agent == nil { + t.Fatal("expected default agent") + } + if len(agent.ImageCandidates) != 1 { + t.Fatalf("len(ImageCandidates) = %d, want 1", len(agent.ImageCandidates)) + } + + visionProvider := &visionAnswerProvider{} + agent.CandidateProviders[providers.ModelKey("openai", "vision-model")] = visionProvider + + timeoutCtx, cancel := context.WithTimeout(context.Background(), responseTimeout) + defer cancel() + + resp, err := al.processMessage(timeoutCtx, testInboundMessage(bus.InboundMessage{ + Context: bus.InboundContext{ + Channel: "telegram", + ChatID: "chat1", + ChatType: "direct", + SenderID: "user1", + MessageID: "m1", + }, + Content: "describe the image you load", + SessionKey: "agent:main:telegram:direct:user1", + })) + if err != nil { + t.Fatalf("processMessage() error = %v", err) + } + if resp != "vision answer" { + t.Fatalf("response = %q, want %q", resp, "vision answer") + } + if planner.calls != 1 { + t.Fatalf("planner calls = %d, want %d", planner.calls, 1) + } + if visionProvider.calls != 1 { + t.Fatalf("visionProvider calls = %d, want %d", visionProvider.calls, 1) + } + if !slices.Equal(visionProvider.models, []string{"vision-model"}) { + t.Fatalf("visionProvider models = %v, want %v", visionProvider.models, []string{"vision-model"}) + } + if !slices.Equal(visionProvider.mediaSeen, []bool{true}) { + t.Fatalf("visionProvider mediaSeen = %v, want %v", visionProvider.mediaSeen, []bool{true}) + } +} + +func TestAgentLoop_LoadImageFollowUpWithoutImageModelFailsClearly(t *testing.T) { + workspace := t.TempDir() + pngPath := filepath.Join(workspace, "sample.png") + pngBytes := []byte{ + 0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A, + 0x00, 0x00, 0x00, 0x0D, 0x49, 0x48, 0x44, 0x52, + 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x08, 0x02, + 0x00, 0x00, 0x00, 0x90, 0x77, 0x53, 0xDE, + } + if err := os.WriteFile(pngPath, pngBytes, 0o644); err != nil { + t.Fatalf("WriteFile() error = %v", err) + } + + cfg := &config.Config{ + Agents: config.AgentsConfig{ + Defaults: config.AgentDefaults{ + Workspace: workspace, + ModelName: "text-model", + MaxTokens: 4096, + MaxToolIterations: 3, + }, + }, + Tools: config.ToolsConfig{ + LoadImage: config.ToolConfig{Enabled: true}, + }, + ModelList: []*config.ModelConfig{ + {ModelName: "text-model", Model: "openai/text-model"}, + }, + } + + msgBus := bus.NewMessageBus() + planner := &loadImagePlanningProvider{ + path: pngPath, + followUpErr: fmt.Errorf( + `API request failed: Status: 404 Body: {"error":{"message":"No endpoints found that support image input"}}`, + ), + } + al := NewAgentLoop(cfg, msgBus, planner) + al.SetMediaStore(media.NewFileMediaStore()) + + timeoutCtx, cancel := context.WithTimeout(context.Background(), responseTimeout) + defer cancel() + + resp, err := al.processMessage(timeoutCtx, testInboundMessage(bus.InboundMessage{ + Context: bus.InboundContext{ + Channel: "telegram", + ChatID: "chat1", + ChatType: "direct", + SenderID: "user1", + MessageID: "m1", + }, + Content: "describe the image you load", + SessionKey: "agent:main:telegram:direct:user1", + })) + if err == nil { + t.Fatal("processMessage() error = nil, want vision unsupported failure") + } + if resp != "" { + t.Fatalf("response = %q, want empty response on error", resp) + } + if !strings.Contains(err.Error(), `active model "text-model" does not support image input`) { + t.Fatalf("error = %q, want clear vision unsupported guidance", err.Error()) + } + if planner.calls != 2 { + t.Fatalf("planner calls = %d, want %d", planner.calls, 2) } } @@ -5719,7 +6443,7 @@ func TestResolveMediaRefs_ImageInjectsPathTag(t *testing.T) { messages := []providers.Message{ {Role: "user", Content: "describe this", Media: []string{ref}}, } - result := resolveMediaRefs(messages, store, config.DefaultMaxMediaSize) + result := resolveMediaRefs(messages, store, config.DefaultMaxMediaSize, 0) if len(result[0].Media) != 0 { t.Fatalf("expected 0 media (images use path tags), got %d", len(result[0].Media)) @@ -5750,9 +6474,9 @@ func TestResolveMediaRefs_ToolRoleImageAppendedAsUserMessage(t *testing.T) { ref, _ := store.Store(pngPath, media.MediaMeta{}, "test") messages := []providers.Message{ - {Role: "tool", Content: "Image loaded", Media: []string{ref}}, + toolResultPromptMessage("Image loaded", "call_tool_result_image", []string{ref}), } - result := resolveMediaRefs(messages, store, config.DefaultMaxMediaSize) + result := resolveMediaRefs(messages, store, config.DefaultMaxMediaSize, 0) // Tool message should have path tag but no base64 if len(result[0].Media) != 0 { @@ -5778,6 +6502,106 @@ func TestResolveMediaRefs_ToolRoleImageAppendedAsUserMessage(t *testing.T) { } } +func TestResolveMediaRefs_HistoricalToolRoleImageDoesNotAppendAsUserMessage(t *testing.T) { + store := media.NewFileMediaStore() + dir := t.TempDir() + + pngPath := filepath.Join(dir, "historical-tool-result.png") + pngHeader := []byte{ + 0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A, + 0x00, 0x00, 0x00, 0x0D, + 0x49, 0x48, 0x44, 0x52, + 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x08, 0x02, + 0x00, 0x00, 0x00, + 0x90, 0x77, 0x53, 0xDE, + } + if err := os.WriteFile(pngPath, pngHeader, 0o644); err != nil { + t.Fatal(err) + } + ref, _ := store.Store(pngPath, media.MediaMeta{}, "test") + + messages := []providers.Message{ + toolResultPromptMessage("Image loaded", "call_historical_tool_result_image", []string{ref}), + {Role: "user", Content: "now summarize it in one sentence"}, + } + result := resolveMediaRefs(messages, store, config.DefaultMaxMediaSize, 1) + + if len(result) != 2 { + t.Fatalf("expected historical tool message plus current follow-up, got %d messages", len(result)) + } + if len(result[0].Media) != 0 { + t.Fatalf("expected 0 media in historical tool message, got %d", len(result[0].Media)) + } + localPath, _, _ := store.ResolveWithMeta(ref) + if !strings.Contains(result[0].Content, "[image:"+localPath+"]") { + t.Fatalf("expected image path tag in historical tool content, got %q", result[0].Content) + } + if result[1].Role != "user" || result[1].Content != "now summarize it in one sentence" { + t.Fatalf("expected current follow-up user message to remain untouched, got %#v", result[1]) + } +} + +func TestResolveMediaRefs_HistoricalAndCurrentToolImagesOnlyRehydrateCurrentTurn(t *testing.T) { + store := media.NewFileMediaStore() + dir := t.TempDir() + + historicalPath := filepath.Join(dir, "historical.png") + currentPath := filepath.Join(dir, "current.png") + pngHeader := []byte{ + 0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A, + 0x00, 0x00, 0x00, 0x0D, + 0x49, 0x48, 0x44, 0x52, + 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x08, 0x02, + 0x00, 0x00, 0x00, + 0x90, 0x77, 0x53, 0xDE, + } + if err := os.WriteFile(historicalPath, pngHeader, 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(currentPath, pngHeader, 0o644); err != nil { + t.Fatal(err) + } + historicalRef, _ := store.Store(historicalPath, media.MediaMeta{}, "test") + currentRef, _ := store.Store(currentPath, media.MediaMeta{}, "test") + + messages := []providers.Message{ + toolResultPromptMessage("Historical image loaded", "call_hist_image", []string{historicalRef}), + {Role: "assistant", Content: "Now I will inspect a new image."}, + toolResultPromptMessage("Current image loaded", "call_current_image", []string{currentRef}), + } + result := resolveMediaRefs(messages, store, config.DefaultMaxMediaSize, 1) + + if len(result) != 4 { + t.Fatalf("expected 4 messages (historical tool + assistant + current tool + synthetic user), "+ + "got %d", len(result)) + } + if result[0].Role != "tool" || len(result[0].Media) != 0 { + t.Fatalf("historical tool message = %#v, want path-tagged tool without media", result[0]) + } + if !strings.Contains(result[0].Content, "[image:"+historicalPath+"]") { + t.Fatalf( + "expected historical tool content to contain %q, got %q", + "[image:"+historicalPath+"]", + result[0].Content, + ) + } + if result[1].Role != "assistant" || result[1].Content != "Now I will inspect a new image." { + t.Fatalf("assistant boundary message = %#v, want untouched assistant", result[1]) + } + if result[2].Role != "tool" || len(result[2].Media) != 0 { + t.Fatalf("current tool message = %#v, want path-tagged tool without media", result[2]) + } + if !strings.Contains(result[2].Content, "[image:"+currentPath+"]") { + t.Fatalf("expected current tool content to contain %q, got %q", "[image:"+currentPath+"]", result[2].Content) + } + if result[3].Role != "user" || len(result[3].Media) != 1 { + t.Fatalf("synthetic follow-up = %#v, want one-media user message", result[3]) + } + if !strings.HasPrefix(result[3].Media[0], "data:image/png;base64,") { + t.Fatalf("expected base64 image in synthetic follow-up, got %q", result[3].Media[0][:40]) + } +} + func TestResolveMediaRefs_MultiToolCallPreservesOrdering(t *testing.T) { store := media.NewFileMediaStore() dir := t.TempDir() @@ -5796,10 +6620,10 @@ func TestResolveMediaRefs_MultiToolCallPreservesOrdering(t *testing.T) { // Simulate: assistant called load_image + read_file, two tool results follow messages := []providers.Message{ {Role: "assistant", Content: "Let me load the image and read the file."}, - {Role: "tool", Content: "Image loaded [image: photo]", Media: []string{imgRef}}, - {Role: "tool", Content: "file contents here"}, + toolResultPromptMessage("Image loaded [image: photo]", "call_load_image_multi_tool", []string{imgRef}), + toolResultPromptMessage("file contents here", "call_read_file_multi_tool", nil), } - result := resolveMediaRefs(messages, store, config.DefaultMaxMediaSize) + result := resolveMediaRefs(messages, store, config.DefaultMaxMediaSize, 0) // assistant, tool#1, tool#2 must remain contiguous — no user in between if result[0].Role != "assistant" { @@ -5824,6 +6648,52 @@ func TestResolveMediaRefs_MultiToolCallPreservesOrdering(t *testing.T) { } } +func TestResolveMediaRefs_MultipleCurrentToolImagesShareSingleSyntheticFollowUp(t *testing.T) { + store := media.NewFileMediaStore() + dir := t.TempDir() + + firstPath := filepath.Join(dir, "first.png") + secondPath := filepath.Join(dir, "second.png") + pngHeader := []byte{ + 0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A, + 0x00, 0x00, 0x00, 0x0D, + 0x49, 0x48, 0x44, 0x52, + 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x08, 0x02, + 0x00, 0x00, 0x00, + 0x90, 0x77, 0x53, 0xDE, + } + if err := os.WriteFile(firstPath, pngHeader, 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(secondPath, pngHeader, 0o644); err != nil { + t.Fatal(err) + } + firstRef, _ := store.Store(firstPath, media.MediaMeta{}, "test") + secondRef, _ := store.Store(secondPath, media.MediaMeta{}, "test") + + messages := []providers.Message{ + {Role: "assistant", Content: "I loaded two images for comparison."}, + toolResultPromptMessage("First image loaded", "call_first_image", []string{firstRef}), + toolResultPromptMessage("Second image loaded", "call_second_image", []string{secondRef}), + } + result := resolveMediaRefs(messages, store, config.DefaultMaxMediaSize, 0) + + if len(result) != 4 { + t.Fatalf("expected assistant + 2 tool results + 1 synthetic follow-up, got %d messages", len(result)) + } + if result[3].Role != "user" { + t.Fatalf("synthetic follow-up role = %q, want user", result[3].Role) + } + if len(result[3].Media) != 2 { + t.Fatalf("synthetic follow-up media count = %d, want 2", len(result[3].Media)) + } + for i, ref := range result[3].Media { + if !strings.HasPrefix(ref, "data:image/png;base64,") { + t.Fatalf("synthetic follow-up media[%d] missing data URL prefix: %q", i, ref[:40]) + } + } +} + func TestResolveMediaRefs_OversizedImageSkipsBase64KeepsPathTag(t *testing.T) { store := media.NewFileMediaStore() dir := t.TempDir() @@ -5841,7 +6711,7 @@ func TestResolveMediaRefs_OversizedImageSkipsBase64KeepsPathTag(t *testing.T) { {Role: "user", Content: "hi", Media: []string{ref}}, } // Use a tiny limit (1KB) so the file is oversized - result := resolveMediaRefs(messages, store, 1024) + result := resolveMediaRefs(messages, store, 1024, 0) if len(result[0].Media) != 0 { t.Fatalf("expected 0 media (oversized), got %d", len(result[0].Media)) @@ -5866,7 +6736,7 @@ func TestResolveMediaRefs_UnknownTypeInjectsPath(t *testing.T) { messages := []providers.Message{ {Role: "user", Content: "hi", Media: []string{ref}}, } - result := resolveMediaRefs(messages, store, config.DefaultMaxMediaSize) + result := resolveMediaRefs(messages, store, config.DefaultMaxMediaSize, 0) if len(result[0].Media) != 0 { t.Fatalf("expected 0 media entries, got %d", len(result[0].Media)) @@ -5881,7 +6751,7 @@ func TestResolveMediaRefs_PassesThroughNonMediaRefs(t *testing.T) { messages := []providers.Message{ {Role: "user", Content: "hi", Media: []string{"https://example.com/img.png"}}, } - result := resolveMediaRefs(messages, nil, config.DefaultMaxMediaSize) + result := resolveMediaRefs(messages, nil, config.DefaultMaxMediaSize, 0) if len(result[0].Media) != 1 || result[0].Media[0] != "https://example.com/img.png" { t.Fatalf("expected passthrough of non-media:// URL, got %v", result[0].Media) @@ -5906,7 +6776,7 @@ func TestResolveMediaRefs_DoesNotMutateOriginal(t *testing.T) { } originalRef := original[0].Media[0] - resolveMediaRefs(original, store, config.DefaultMaxMediaSize) + resolveMediaRefs(original, store, config.DefaultMaxMediaSize, 0) if original[0].Media[0] != originalRef { t.Fatal("resolveMediaRefs mutated original message slice") @@ -5926,7 +6796,7 @@ func TestResolveMediaRefs_UsesMetaContentType(t *testing.T) { messages := []providers.Message{ {Role: "user", Content: "hi", Media: []string{ref}}, } - result := resolveMediaRefs(messages, store, config.DefaultMaxMediaSize) + result := resolveMediaRefs(messages, store, config.DefaultMaxMediaSize, 0) if len(result[0].Media) != 0 { t.Fatalf("expected 0 media (images use path tags), got %d", len(result[0].Media)) @@ -5950,7 +6820,7 @@ func TestResolveMediaRefs_PDFInjectsFilePath(t *testing.T) { messages := []providers.Message{ {Role: "user", Content: "report.pdf [file]", Media: []string{ref}}, } - result := resolveMediaRefs(messages, store, config.DefaultMaxMediaSize) + result := resolveMediaRefs(messages, store, config.DefaultMaxMediaSize, 0) if len(result[0].Media) != 0 { t.Fatalf("expected 0 media (non-image), got %d", len(result[0].Media)) @@ -5972,7 +6842,7 @@ func TestResolveMediaRefs_AudioInjectsAudioPath(t *testing.T) { messages := []providers.Message{ {Role: "user", Content: "voice.ogg [audio]", Media: []string{ref}}, } - result := resolveMediaRefs(messages, store, config.DefaultMaxMediaSize) + result := resolveMediaRefs(messages, store, config.DefaultMaxMediaSize, 0) if len(result[0].Media) != 0 { t.Fatalf("expected 0 media, got %d", len(result[0].Media)) @@ -5994,7 +6864,7 @@ func TestResolveMediaRefs_VideoInjectsVideoPath(t *testing.T) { messages := []providers.Message{ {Role: "user", Content: "clip.mp4 [video]", Media: []string{ref}}, } - result := resolveMediaRefs(messages, store, config.DefaultMaxMediaSize) + result := resolveMediaRefs(messages, store, config.DefaultMaxMediaSize, 0) if len(result[0].Media) != 0 { t.Fatalf("expected 0 media, got %d", len(result[0].Media)) @@ -6016,7 +6886,7 @@ func TestResolveMediaRefs_NoGenericTagAppendsPath(t *testing.T) { messages := []providers.Message{ {Role: "user", Content: "here is my data", Media: []string{ref}}, } - result := resolveMediaRefs(messages, store, config.DefaultMaxMediaSize) + result := resolveMediaRefs(messages, store, config.DefaultMaxMediaSize, 0) expected := "here is my data [file:" + csvPath + "]" if result[0].Content != expected { @@ -6108,7 +6978,7 @@ func TestResolveMediaRefs_JSONContentPrependsPathTag(t *testing.T) { messages := []providers.Message{ {Role: "user", Content: jsonContent, Media: []string{ref}}, } - result := resolveMediaRefs(messages, store, config.DefaultMaxMediaSize) + result := resolveMediaRefs(messages, store, config.DefaultMaxMediaSize, 0) want := "[image:" + pngPath + "]\n" + jsonContent if result[0].Content != want { @@ -6128,7 +6998,7 @@ func TestResolveMediaRefs_EmptyContentGetsPathTag(t *testing.T) { messages := []providers.Message{ {Role: "user", Content: "", Media: []string{ref}}, } - result := resolveMediaRefs(messages, store, config.DefaultMaxMediaSize) + result := resolveMediaRefs(messages, store, config.DefaultMaxMediaSize, 0) expected := "[file:" + docPath + "]" if result[0].Content != expected { @@ -6157,7 +7027,7 @@ func TestResolveMediaRefs_MixedImageAndFile(t *testing.T) { messages := []providers.Message{ {Role: "user", Content: "check these [file]", Media: []string{imgRef, fileRef}}, } - result := resolveMediaRefs(messages, store, config.DefaultMaxMediaSize) + result := resolveMediaRefs(messages, store, config.DefaultMaxMediaSize, 0) if len(result[0].Media) != 0 { t.Fatalf("expected 0 media (all types use path tags), got %d", len(result[0].Media)) diff --git a/pkg/agent/instance.go b/pkg/agent/instance.go index cc69c817..ed2dd2a0 100644 --- a/pkg/agent/instance.go +++ b/pkg/agent/instance.go @@ -43,6 +43,7 @@ type AgentInstance struct { SkillsFilter []string MCPServerAllowlist map[string]struct{} Candidates []providers.FallbackCandidate + ImageCandidates []providers.FallbackCandidate // Router is non-nil when model routing is configured and the light model // was successfully resolved. It scores each incoming message and decides @@ -198,9 +199,19 @@ func NewAgentInstance( // Resolve fallback candidates candidates := resolveModelCandidates(cfg, defaults.Provider, model, fallbacks) + imageCandidates := resolveModelCandidates( + cfg, + defaults.Provider, + defaults.ImageModel, + defaults.ImageModelFallbacks, + ) candidateProviders := make(map[string]providers.LLMProvider) populateCandidateProvidersFromNames(cfg, workspace, fallbacks, candidateProviders) + if strings.TrimSpace(defaults.ImageModel) != "" { + imageNames := append([]string{defaults.ImageModel}, defaults.ImageModelFallbacks...) + populateCandidateProvidersFromNames(cfg, workspace, imageNames, candidateProviders) + } // Model routing setup: pre-resolve light model candidates at creation time // to avoid repeated model_list lookups on every incoming message. @@ -265,6 +276,7 @@ func NewAgentInstance( SkillsFilter: skillsFilter, MCPServerAllowlist: agentMCPServerAllowlist, Candidates: candidates, + ImageCandidates: imageCandidates, Router: router, LightCandidates: lightCandidates, LightProvider: lightProvider, diff --git a/pkg/agent/llm_media.go b/pkg/agent/llm_media.go index 31692174..6e27a39e 100644 --- a/pkg/agent/llm_media.go +++ b/pkg/agent/llm_media.go @@ -1,11 +1,16 @@ package agent import ( + "fmt" + "regexp" "strings" + "github.com/sipeed/picoclaw/pkg/logger" "github.com/sipeed/picoclaw/pkg/providers" ) +var resolvedImagePathTagRegex = regexp.MustCompile(`\[image:[^\s\]][^\]]*\]`) + func messagesContainMedia(messages []providers.Message) bool { for _, msg := range messages { for _, ref := range msg.Media { @@ -65,3 +70,125 @@ func isVisionUnsupportedError(err error) bool { return false } + +func visionUnsupportedModelError(modelName string, imageModelConfigured bool) error { + modelName = strings.TrimSpace(modelName) + if imageModelConfigured { + if modelName != "" { + return fmt.Errorf( + "selected vision model %q does not support image input; update agents.defaults.image_model to a multimodal model", + modelName, + ) + } + return fmt.Errorf( + "selected vision model does not support image input; update agents.defaults.image_model to a multimodal model", + ) + } + if modelName != "" { + return fmt.Errorf( + "active model %q does not support image input; configure agents.defaults.image_model with a multimodal model", + modelName, + ) + } + return fmt.Errorf( + "the active model does not support image input; configure agents.defaults.image_model with a multimodal model", + ) +} + +func sameCandidateSet(a, b []providers.FallbackCandidate) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i].StableKey() != b[i].StableKey() { + return false + } + } + return true +} + +func messagesContainCurrentTurnMediaTurn(messages []providers.Message) bool { + for _, msg := range messages { + if len(msg.Media) > 0 { + return true + } + if resolvedImagePathTagRegex.MatchString(msg.Content) { + return true + } + } + return false +} + +func (p *Pipeline) routeMediaTurn(ts *turnState, exec *turnExecution) error { + if p == nil || ts == nil || ts.agent == nil || exec == nil || + !messagesContainCurrentTurnMediaTurn(currentTurnMessages(exec.callMessages, exec.currentTurnStart)) { + return nil + } + + var targetCandidates []providers.FallbackCandidate + var targetModelName string + var routeReason string + + switch { + case len(ts.agent.ImageCandidates) > 0: + targetCandidates = append([]providers.FallbackCandidate(nil), ts.agent.ImageCandidates...) + targetModelName = strings.TrimSpace(p.Cfg.Agents.Defaults.ImageModel) + routeReason = "configured_image_model" + case exec.usedLight && len(ts.agent.Candidates) > 0: + targetCandidates = append([]providers.FallbackCandidate(nil), ts.agent.Candidates...) + targetModelName = strings.TrimSpace(ts.agent.Model) + routeReason = "bypass_light_model_for_media" + default: + return nil + } + + if len(targetCandidates) == 0 { + return nil + } + + targetModel := resolvedCandidateModel(targetCandidates, targetModelName) + targetProvider := exec.activeProvider + firstCandidate := targetCandidates[0] + if provider, err := providerForFallbackCandidate( + ts.agent, + ts.agent.Provider, + targetCandidates, + firstCandidate.Provider, + firstCandidate.Model, + ); err != nil { + return err + } else if provider != nil { + targetProvider = provider + } + + resolvedModelName := resolvedCandidateModelName(targetCandidates, targetModelName) + if sameCandidateSet(exec.activeCandidates, targetCandidates) && + exec.activeModel == targetModel && + exec.llmModelName == resolvedModelName { + return nil + } + + exec.activeCandidates = targetCandidates + exec.activeModel = targetModel + exec.activeProvider = targetProvider + exec.activeModelConfig = resolveActiveModelConfig( + p.Cfg, + ts.agent.Workspace, + targetCandidates, + targetModel, + p.Cfg.Agents.Defaults.Provider, + ) + exec.llmModelName = resolvedModelName + exec.usedLight = false + + logger.InfoCF("agent", "Media turn routing selected model", map[string]any{ + "agent_id": ts.agent.ID, + "reason": routeReason, + "model": exec.activeModel, + "model_name": exec.llmModelName, + "candidates": len(exec.activeCandidates), + "messages_count": len(exec.callMessages), + }) + + return nil +} diff --git a/pkg/agent/pipeline_execute.go b/pkg/agent/pipeline_execute.go index 81fc06ec..59e33317 100644 --- a/pkg/agent/pipeline_execute.go +++ b/pkg/agent/pipeline_execute.go @@ -295,21 +295,16 @@ toolLoop: contentForLLM = al.cfg.FilterSensitiveData(contentForLLM) } - toolResultMsg := providers.Message{ - Role: "tool", - Content: contentForLLM, - ToolCallID: tc.ID, - } - + var toolResultMedia []string if len(hookResult.Media) > 0 && !hookResult.ResponseHandled { hookResult.ArtifactTags = buildArtifactTags(al.mediaStore, hookResult.Media) contentForLLM = hookResult.ContentForLLM() if al.cfg.Tools.IsFilterSensitiveDataEnabled() { contentForLLM = al.cfg.FilterSensitiveData(contentForLLM) } - toolResultMsg.Content = contentForLLM - toolResultMsg.Media = append(toolResultMsg.Media, hookResult.Media...) + toolResultMedia = append(toolResultMedia, hookResult.Media...) } + toolResultMsg := toolResultPromptMessage(contentForLLM, tc.ID, toolResultMedia) al.emitEvent( runtimeevents.KindAgentToolExecEnd, @@ -695,14 +690,11 @@ toolLoop: contentForLLM = al.cfg.FilterSensitiveData(contentForLLM) } - toolResultMsg := providers.Message{ - Role: "tool", - Content: contentForLLM, - ToolCallID: toolCallID, - } + var toolResultMedia []string if len(toolResult.Media) > 0 && !toolResult.ResponseHandled { - toolResultMsg.Media = append(toolResultMsg.Media, toolResult.Media...) + toolResultMedia = append(toolResultMedia, toolResult.Media...) } + toolResultMsg := toolResultPromptMessage(contentForLLM, toolCallID, toolResultMedia) al.emitEvent( runtimeevents.KindAgentToolExecEnd, ts.eventMeta("runTurn", "turn.tool.end"), diff --git a/pkg/agent/pipeline_llm.go b/pkg/agent/pipeline_llm.go index 4f8a0613..0961a3d6 100644 --- a/pkg/agent/pipeline_llm.go +++ b/pkg/agent/pipeline_llm.go @@ -31,7 +31,7 @@ func (p *Pipeline) CallLLM( // PreLLM: resolve media refs (except on iteration 1 where user media is already resolved) if iteration > 1 { - exec.messages = resolveMediaRefs(exec.messages, p.MediaStore, maxMediaSize) + exec.messages = resolveMediaRefs(exec.messages, p.MediaStore, maxMediaSize, exec.currentTurnStart) } // PreLLM: graceful terminal handling @@ -64,6 +64,9 @@ func (p *Pipeline) CallLLM( exec.providerToolDefs = nil ts.markGracefulTerminalUsed() } + if err := p.routeMediaTurn(ts, exec); err != nil { + return ControlBreak, err + } exec.llmOpts = map[string]any{ "max_tokens": ts.agent.MaxTokens, @@ -170,36 +173,62 @@ func (p *Pipeline) CallLLM( return response, streamErr } - if len(exec.activeCandidates) > 1 && p.Fallback != nil { - fbResult, fbErr := p.Fallback.ExecuteCandidate( - providerCtx, + runCandidate := func( + ctx context.Context, + candidate providers.FallbackCandidate, + ) (*providers.LLMResponse, error) { + candidateProvider, err := providerForFallbackCandidate( + ts.agent, + exec.activeProvider, exec.activeCandidates, - func(ctx context.Context, candidate providers.FallbackCandidate) (*providers.LLMResponse, error) { - candidateProvider, err := providerForFallbackCandidate( - ts.agent, - exec.activeProvider, - exec.activeCandidates, - candidate.Provider, - candidate.Model, - ) - if err != nil { - return nil, err - } - callOpts := shallowCloneLLMOptions(exec.llmOpts) - delete(callOpts, "thinking_level") - candidateCfg := resolveActiveModelConfig( - p.Cfg, - ts.agent.Workspace, - []providers.FallbackCandidate{candidate}, - candidate.Model, - p.Cfg.Agents.Defaults.Provider, - ) - candidateThinking := thinkingSettingsFromModelConfig(candidateCfg) - applyThinkingOption(callOpts, candidateProvider, candidateThinking, true, ts.agent.ID) - exec.suppressReasoning = shouldSuppressReasoningFor(candidateThinking) - return candidateProvider.Chat(ctx, messagesForCall, toolDefsForCall, candidate.Model, callOpts) - }, + candidate.Provider, + candidate.Model, ) + if err != nil { + return nil, err + } + callOpts := shallowCloneLLMOptions(exec.llmOpts) + delete(callOpts, "thinking_level") + candidateCfg := resolveActiveModelConfig( + p.Cfg, + ts.agent.Workspace, + []providers.FallbackCandidate{candidate}, + candidate.Model, + p.Cfg.Agents.Defaults.Provider, + ) + candidateThinking := thinkingSettingsFromModelConfig(candidateCfg) + applyThinkingOption(callOpts, candidateProvider, candidateThinking, true, ts.agent.ID) + exec.suppressReasoning = shouldSuppressReasoningFor(candidateThinking) + return candidateProvider.Chat(ctx, messagesForCall, toolDefsForCall, candidate.Model, callOpts) + } + + if len(exec.activeCandidates) > 1 && p.Fallback != nil { + var ( + fbResult *providers.FallbackResult + fbErr error + ) + if hasMediaRefs(messagesForCall) { + fbResult, fbErr = p.Fallback.ExecuteImage( + providerCtx, + exec.activeCandidates, + func(ctx context.Context, provider, model string) (*providers.LLMResponse, error) { + candidate := providers.FallbackCandidate{Provider: provider, Model: model} + for _, configured := range exec.activeCandidates { + if configured.Provider == provider && configured.Model == model { + candidate = configured + break + } + } + return runCandidate(ctx, candidate) + }, + ) + } else { + fbResult, fbErr = p.Fallback.ExecuteCandidate( + providerCtx, + exec.activeCandidates, + runCandidate, + ) + } if fbErr != nil { return nil, fbErr } @@ -250,33 +279,11 @@ func (p *Pipeline) CallLLM( break } - // Retry without media if vision is unsupported - if hasMediaRefs(exec.callMessages) && isVisionUnsupportedError(err) && retry < maxRetries { - al.emitEvent( - runtimeevents.KindAgentLLMRetry, - ts.eventMeta("runTurn", "turn.llm.retry"), - LLMRetryPayload{ - Attempt: retry + 1, - MaxRetries: maxRetries, - Reason: "vision_unsupported", - Error: err.Error(), - Backoff: 0, - }, + if hasMediaRefs(exec.callMessages) && isVisionUnsupportedError(err) { + return ControlBreak, visionUnsupportedModelError( + exec.llmModelName, + len(ts.agent.ImageCandidates) > 0, ) - logger.WarnCF("agent", "Vision unsupported, retrying without media", map[string]any{ - "error": err.Error(), - "retry": retry, - }) - exec.callMessages = stripMessageMedia(exec.callMessages) - if !ts.opts.NoHistory { - exec.history = stripMessageMedia(exec.history) - ts.agent.Sessions.SetHistory(ts.sessionKey, exec.history) - for i := range ts.persistedMessages { - ts.persistedMessages[i].Media = nil - } - ts.refreshRestorePointFromSession(ts.agent) - } - continue } errMsg := strings.ToLower(err.Error()) @@ -381,7 +388,13 @@ func (p *Pipeline) CallLLM( fullHistory := append(append([]providers.Message(nil), trimmedHistory...), protectedTurnTail...) rebuildPromptReq := promptBuildRequestForTurn(ts, fullHistory, exec.summary, "", nil, p.Cfg) rebuildPromptReq.ActiveSkills = append([]string(nil), contextualSkills...) - return ts.agent.ContextBuilder.BuildMessagesFromPrompt(rebuildPromptReq) + rebuilt := ts.agent.ContextBuilder.BuildMessagesFromPrompt(rebuildPromptReq) + return resolveMediaRefs( + rebuilt, + p.MediaStore, + maxMediaSize, + len(rebuilt)-len(protectedTurnTail), + ) } originalHistoryCount := len(exec.history) var fit bool @@ -401,6 +414,7 @@ func (p *Pipeline) CallLLM( ) exec.history = append(trimmedStableHistory, protectedTurnTail...) exec.messages = buildMessages(trimmedStableHistory) + exec.currentTurnStart = len(exec.messages) - len(protectedTurnTail) if exec.gracefulTerminal { msgs := append([]providers.Message(nil), exec.messages...) exec.callMessages = append(msgs, ts.interruptHintMessage()) diff --git a/pkg/agent/pipeline_setup.go b/pkg/agent/pipeline_setup.go index ea05968c..0911a3b6 100644 --- a/pkg/agent/pipeline_setup.go +++ b/pkg/agent/pipeline_setup.go @@ -39,8 +39,12 @@ func (p *Pipeline) SetupTurn(ctx context.Context, ts *turnState) (*turnExecution initialPromptReq := promptBuildRequestForTurn(ts, history, summary, ts.userMessage, ts.media, cfg) initialPromptReq.ActiveSkills = append([]string(nil), contextualSkills...) messages := ts.agent.ContextBuilder.BuildMessagesFromPrompt(initialPromptReq) + currentTurnStart := len(messages) + if strings.TrimSpace(ts.userMessage) != "" || len(ts.media) > 0 { + currentTurnStart = len(messages) - 1 + } - messages = resolveMediaRefs(messages, p.MediaStore, maxMediaSize) + messages = resolveMediaRefs(messages, p.MediaStore, maxMediaSize, currentTurnStart) if !ts.opts.NoHistory { toolDefs := filterToolsByTurnProfile(ts.agent.Tools.ToProviderDefs(), ts.profile) @@ -81,7 +85,11 @@ func (p *Pipeline) SetupTurn(ctx context.Context, ts *turnState) (*turnExecution ) rebuildPromptReq.ActiveSkills = append([]string(nil), contextualSkills...) rebuilt := ts.agent.ContextBuilder.BuildMessagesFromPrompt(rebuildPromptReq) - return resolveMediaRefs(rebuilt, p.MediaStore, maxMediaSize) + rebuiltCurrentTurnStart := len(rebuilt) + if strings.TrimSpace(ts.userMessage) != "" || len(ts.media) > 0 { + rebuiltCurrentTurnStart = len(rebuilt) - 1 + } + return resolveMediaRefs(rebuilt, p.MediaStore, maxMediaSize, rebuiltCurrentTurnStart) }, ts.agent.ContextWindow, toolDefs, @@ -137,6 +145,7 @@ func (p *Pipeline) SetupTurn(ctx context.Context, ts *turnState) (*turnExecution summary, messages, ) + exec.currentTurnStart = currentTurnStart exec.activeCandidates = activeCandidates exec.activeModel = activeModel exec.activeModelConfig = resolveActiveModelConfig( diff --git a/pkg/agent/prompt.go b/pkg/agent/prompt.go index 1c66fe69..6270dc32 100644 --- a/pkg/agent/prompt.go +++ b/pkg/agent/prompt.go @@ -37,6 +37,7 @@ const ( PromptSlotMessage PromptSlot = "message" PromptSlotSteering PromptSlot = "steering" PromptSlotSubTurn PromptSlot = "subturn" + PromptSlotToolResult PromptSlot = "tool_result" PromptSlotInterrupt PromptSlot = "interrupt" PromptSlotOutput PromptSlot = "output" ) @@ -60,6 +61,7 @@ const ( PromptSourceUserMessage PromptSourceID = "turn:user_message" PromptSourceSteering PromptSourceID = "turn:steering" PromptSourceSubTurnResult PromptSourceID = "turn:subturn_result" + PromptSourceToolResult PromptSourceID = "turn:tool_result" PromptSourceInterrupt PromptSourceID = "turn:interrupt" ) diff --git a/pkg/agent/prompt_turn.go b/pkg/agent/prompt_turn.go index 053eb846..d149b491 100644 --- a/pkg/agent/prompt_turn.go +++ b/pkg/agent/prompt_turn.go @@ -194,6 +194,29 @@ func userPromptMessage(content string, media []string) providers.Message { return promptMessageWithMetadata(msg, PromptLayerTurn, PromptSlotMessage, PromptSourceUserMessage) } +func toolResultPromptMessage(content, toolCallID string, media []string) providers.Message { + msg := providers.Message{ + Role: "tool", + Content: content, + ToolCallID: toolCallID, + } + if len(media) > 0 { + msg.Media = append([]string(nil), media...) + } + return promptMessageWithMetadata(msg, PromptLayerTurn, PromptSlotToolResult, PromptSourceToolResult) +} + +func toolImageFollowUpPromptMessage(media []string) providers.Message { + msg := providers.Message{ + Role: "user", + Content: "[Loaded image from tool result above]", + } + if len(media) > 0 { + msg.Media = append([]string(nil), media...) + } + return promptMessageWithMetadata(msg, PromptLayerTurn, PromptSlotToolResult, PromptSourceToolResult) +} + func steeringPromptMessage(msg providers.Message) providers.Message { return promptMessageWithDefaultMetadata(msg, PromptLayerTurn, PromptSlotSteering, PromptSourceSteering) } diff --git a/pkg/agent/turn_coord.go b/pkg/agent/turn_coord.go index 4e14c759..166ca955 100644 --- a/pkg/agent/turn_coord.go +++ b/pkg/agent/turn_coord.go @@ -150,7 +150,7 @@ func (al *AgentLoop) runTurn(ctx context.Context, ts *turnState, pipeline *Pipel // Inject pending steering messages if len(pendingMessages) > 0 { - resolvedPending := resolveMediaRefs(pendingMessages, al.mediaStore, maxMediaSize) + resolvedPending := resolveMediaRefs(pendingMessages, al.mediaStore, maxMediaSize, 0) totalContentLen := 0 for i, pm := range pendingMessages { messages = append(messages, resolvedPending[i]) @@ -431,7 +431,11 @@ func (al *AgentLoop) askSideQuestion( messages := agent.ContextBuilder.BuildMessagesFromPrompt(promptReq) maxMediaSize := al.GetConfig().Agents.Defaults.GetMaxMediaSize() - messages = resolveMediaRefs(messages, al.mediaStore, maxMediaSize) + currentTurnStart := len(messages) + if strings.TrimSpace(question) != "" || len(media) > 0 { + currentTurnStart = len(messages) - 1 + } + messages = resolveMediaRefs(messages, al.mediaStore, maxMediaSize, currentTurnStart) activeCandidates, activeModel, usedLight := al.selectCandidates(agent, question, messages) selectedModelName := sideQuestionModelName(agent, usedLight) diff --git a/pkg/agent/turn_state.go b/pkg/agent/turn_state.go index 5fc4dd40..deb653dc 100644 --- a/pkg/agent/turn_state.go +++ b/pkg/agent/turn_state.go @@ -114,10 +114,11 @@ type ActiveTurnInfo struct { type turnExecution struct { // Core message state (accumulates throughout the turn) - messages []providers.Message // built from ContextBuilder, grows per-iteration - pendingMessages []providers.Message // steering/SubTurn messages awaiting injection - history []providers.Message // from ContextManager.Assemble - summary string + messages []providers.Message // built from ContextBuilder, grows per-iteration + pendingMessages []providers.Message // steering/SubTurn messages awaiting injection + history []providers.Message // from ContextManager.Assemble + summary string + currentTurnStart int // Turn output finalContent string @@ -164,12 +165,13 @@ func newTurnExecution( messages []providers.Message, ) *turnExecution { return &turnExecution{ - history: history, - summary: summary, - messages: messages, - pendingMessages: append([]providers.Message(nil), opts.InitialSteeringMessages...), - iteration: 0, - phase: LLMPhaseSetup, + history: history, + summary: summary, + messages: messages, + pendingMessages: append([]providers.Message(nil), opts.InitialSteeringMessages...), + currentTurnStart: len(messages), + iteration: 0, + phase: LLMPhaseSetup, } }