diff --git a/cmd/picoclaw/internal/onboard/command.go b/cmd/picoclaw/internal/onboard/command.go index bf8f4104f..4fb5b8e23 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 000000000..fcedcbba7 --- /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_test.go b/pkg/agent/agent_test.go index c3562bdaa..dd13995b8 100644 --- a/pkg/agent/agent_test.go +++ b/pkg/agent/agent_test.go @@ -4401,7 +4401,90 @@ 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" +} + +func TestAgentLoop_VisionUnsupportedErrorReturnsClearFailure(t *testing.T) { workspace := t.TempDir() cfg := &config.Config{ @@ -4436,17 +4519,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,37 +4540,161 @@ 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_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) } - timeoutCtx2, cancel2 := context.WithTimeout(context.Background(), responseTimeout) - defer cancel2() + 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"}, + }, + } - resp2, err := al.processMessage(timeoutCtx2, testInboundMessage(bus.InboundMessage{ + 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: "m2", + MessageID: "m1", }, - Content: "hello again", - SessionKey: sessionKey, + Content: "describe the image you load", + SessionKey: "agent:main:telegram:direct:user1", })) if err != nil { - t.Fatalf("processMessage() second call error = %v", err) + t.Fatalf("processMessage() error = %v", err) } - if resp2 != "ok" { - t.Fatalf("second response = %q, want %q", resp2, "ok") + if resp != "vision answer" { + t.Fatalf("response = %q, want %q", resp, "vision answer") } - if provider.calls != 3 { - t.Fatalf("calls after second turn = %d, want %d", provider.calls, 3) + if planner.calls != 1 { + t.Fatalf("planner calls = %d, want %d", planner.calls, 1) } - 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 %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) } } diff --git a/pkg/agent/instance.go b/pkg/agent/instance.go index cc69c8177..ed2dd2a0b 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 31692174b..043df9eb1 100644 --- a/pkg/agent/llm_media.go +++ b/pkg/agent/llm_media.go @@ -1,8 +1,10 @@ package agent import ( + "fmt" "strings" + "github.com/sipeed/picoclaw/pkg/logger" "github.com/sipeed/picoclaw/pkg/providers" ) @@ -65,3 +67,112 @@ 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 (p *Pipeline) routeMediaTurn(ts *turnState, exec *turnExecution) error { + if p == nil || ts == nil || ts.agent == nil || exec == nil || !hasMediaRefs(exec.callMessages) { + 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_llm.go b/pkg/agent/pipeline_llm.go index 4f8a06132..8eebe45dd 100644 --- a/pkg/agent/pipeline_llm.go +++ b/pkg/agent/pipeline_llm.go @@ -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())