diff --git a/pkg/agent/instance_test.go b/pkg/agent/instance_test.go index 790ab16ca..42bb53d86 100644 --- a/pkg/agent/instance_test.go +++ b/pkg/agent/instance_test.go @@ -229,6 +229,43 @@ func TestNewAgentInstance_PreservesDistinctLimiterIdentityForSharedResolvedModel } } +func TestNewAgentInstance_PreservesConfigIdentityForExplicitProviderModelRef(t *testing.T) { + tmpDir := t.TempDir() + + cfg := &config.Config{ + Agents: config.AgentsConfig{ + Defaults: config.AgentDefaults{ + Workspace: tmpDir, + ModelName: "nvidia/z-ai/glm-5.1", + }, + }, + ModelList: []*config.ModelConfig{ + { + ModelName: "nvidia-glm", + Provider: "nvidia", + Model: "z-ai/glm-5.1", + RPM: 7, + }, + }, + } + + agent := NewAgentInstance(nil, &cfg.Agents.Defaults, cfg, &mockProvider{}) + if len(agent.Candidates) != 1 { + t.Fatalf("len(Candidates) = %d, want 1", len(agent.Candidates)) + } + + candidate := agent.Candidates[0] + if candidate.Provider != "nvidia" || candidate.Model != "z-ai/glm-5.1" { + t.Fatalf("candidate = %s/%s, want nvidia/z-ai/glm-5.1", candidate.Provider, candidate.Model) + } + if candidate.IdentityKey != "model_name:nvidia-glm" { + t.Fatalf("identity key = %q, want %q", candidate.IdentityKey, "model_name:nvidia-glm") + } + if candidate.RPM != 7 { + t.Fatalf("RPM = %d, want 7", candidate.RPM) + } +} + func TestNewAgentInstance_AllowsMediaTempDirForReadListAndExec(t *testing.T) { workspace := t.TempDir() mediaDir := media.TempDir() diff --git a/pkg/agent/model_resolution.go b/pkg/agent/model_resolution.go index 1c23cee50..6065f6403 100644 --- a/pkg/agent/model_resolution.go +++ b/pkg/agent/model_resolution.go @@ -60,6 +60,12 @@ func lookupModelConfigByRef(cfg *config.Config, raw string) *config.ModelConfig return mc } + rawRef := providers.ParseModelRef(raw, "") + rawKey := "" + if rawRef != nil && strings.TrimSpace(rawRef.Provider) != "" && strings.TrimSpace(rawRef.Model) != "" { + rawKey = providers.ModelKey(rawRef.Provider, rawRef.Model) + } + for i := range cfg.ModelList { mc := cfg.ModelList[i] if mc == nil { @@ -72,10 +78,13 @@ func lookupModelConfigByRef(cfg *config.Config, raw string) *config.ModelConfig if fullModel == raw { return mc } - _, modelID := providers.ExtractProtocol(mc) + protocol, modelID := providers.ExtractProtocol(mc) if modelID == raw { return mc } + if rawKey != "" && providers.ModelKey(protocol, modelID) == rawKey { + return mc + } } return nil diff --git a/pkg/providers/factory_provider.go b/pkg/providers/factory_provider.go index 2f956c7a0..140d140e3 100644 --- a/pkg/providers/factory_provider.go +++ b/pkg/providers/factory_provider.go @@ -107,7 +107,13 @@ func ExtractProtocol(cfg *config.ModelConfig) (protocol, modelID string) { model := strings.TrimSpace(cfg.Model) if provider := strings.TrimSpace(cfg.Provider); provider != "" { - return NormalizeProvider(provider), model + normalized := NormalizeProvider(provider) + if prefix, rest, found := strings.Cut(model, "/"); found { + if NormalizeProvider(prefix) == normalized && strings.TrimSpace(rest) != "" { + return normalized, strings.TrimSpace(rest) + } + } + return normalized, model } if model == "" { return "", "" diff --git a/pkg/providers/factory_provider_test.go b/pkg/providers/factory_provider_test.go index 62b838acb..d691d2398 100644 --- a/pkg/providers/factory_provider_test.go +++ b/pkg/providers/factory_provider_test.go @@ -83,6 +83,18 @@ func TestExtractProtocol(t *testing.T) { wantProtocol: "nvidia", wantModelID: "z-ai/glm-5.1", }, + { + name: "explicit provider strips redundant matching prefix", + config: &config.ModelConfig{Provider: "openai", Model: "openai/gpt-4o"}, + wantProtocol: "openai", + wantModelID: "gpt-4o", + }, + { + name: "explicit provider strips redundant aliased prefix", + config: &config.ModelConfig{Provider: "qwen", Model: "qwen/qwen-plus"}, + wantProtocol: "qwen-portal", + wantModelID: "qwen-plus", + }, { name: "empty provider segment", config: &config.ModelConfig{Model: "/gpt-4o"}, @@ -152,6 +164,27 @@ func TestCreateProviderFromConfig_UsesExplicitProvider(t *testing.T) { } } +func TestCreateProviderFromConfig_StripsRedundantExplicitProviderPrefix(t *testing.T) { + cfg := &config.ModelConfig{ + ModelName: "test-openai", + Provider: "openai", + Model: "openai/gpt-4o", + APIBase: "https://api.example.com/v1", + } + cfg.SetAPIKey("test-key") + + provider, modelID, err := CreateProviderFromConfig(cfg) + if err != nil { + t.Fatalf("CreateProviderFromConfig() error = %v", err) + } + if provider == nil { + t.Fatal("CreateProviderFromConfig() returned nil provider") + } + if modelID != "gpt-4o" { + t.Fatalf("modelID = %q, want %q", modelID, "gpt-4o") + } +} + func TestCreateProviderFromConfig_DefaultAPIBase(t *testing.T) { tests := []struct { name string diff --git a/web/backend/api/models.go b/web/backend/api/models.go index e2ff19952..459bd7655 100644 --- a/web/backend/api/models.go +++ b/web/backend/api/models.go @@ -6,6 +6,7 @@ import ( "io" "net/http" "strconv" + "strings" "sync" "github.com/sipeed/picoclaw/pkg/config" @@ -50,6 +51,27 @@ type modelResponse struct { IsVirtual bool `json:"is_virtual"` } +func normalizeExplicitProviderModel(mc *config.ModelConfig) { + if mc == nil { + return + } + + provider := strings.TrimSpace(mc.Provider) + model := strings.TrimSpace(mc.Model) + if provider == "" || model == "" { + return + } + + prefix, rest, found := strings.Cut(model, "/") + if !found || strings.TrimSpace(rest) == "" { + return + } + + if providers.NormalizeProvider(prefix) == providers.NormalizeProvider(provider) { + mc.Model = strings.TrimSpace(rest) + } +} + // handleListModels returns all model_list entries with masked API keys. // // GET /api/models @@ -139,6 +161,7 @@ func (h *Handler) handleAddModel(w http.ResponseWriter, r *http.Request) { if mc.APIKey != "" { mc.ModelConfig.SetAPIKey(mc.APIKey) } + normalizeExplicitProviderModel(&mc.ModelConfig) cfg, err := config.LoadConfig(h.configPath) if err != nil { @@ -241,7 +264,31 @@ func (h *Handler) handleUpdateModel(w http.ResponseWriter, r *http.Request) { // the new field yet, while still allowing explicit clearing via "". if _, ok := rawFields["provider"]; !ok { mc.Provider = cfg.ModelList[idx].Provider + // Older clients still round-trip the legacy model field only. When the + // stored config encodes provider/model in Model and has no explicit + // Provider field yet, continue preserving that hidden provider prefix. + // This keeps provider-omitted updates backward-compatible even when an + // older client edits the visible model ID. + if strings.TrimSpace(cfg.ModelList[idx].Provider) == "" { + existingProtocol, existingModelID := providers.ExtractProtocol(cfg.ModelList[idx]) + existingRawModel := strings.TrimSpace(cfg.ModelList[idx].Model) + incomingModel := strings.TrimSpace(mc.Model) + if existingRawModel != "" && existingRawModel != existingModelID && incomingModel != "" { + if incomingModel == existingModelID { + mc.Model = existingRawModel + } else if strings.Contains(incomingModel, "/") && !strings.Contains(existingModelID, "/") { + // Older clients never saw the hidden provider prefix for simple + // legacy entries such as "openai/gpt-4o". If they now send an + // explicit provider/model string, treat it as the caller's full + // intent instead of re-applying the old hidden prefix. + mc.Model = incomingModel + } else if !strings.HasPrefix(incomingModel, existingProtocol+"/") { + mc.Model = existingProtocol + "/" + incomingModel + } + } + } } + normalizeExplicitProviderModel(&mc.ModelConfig) cfg.ModelList[idx] = &mc.ModelConfig diff --git a/web/backend/api/models_test.go b/web/backend/api/models_test.go index 5bb9769ce..b43b569c6 100644 --- a/web/backend/api/models_test.go +++ b/web/backend/api/models_test.go @@ -508,6 +508,41 @@ func TestHandleAddModel_PersistsProvider(t *testing.T) { } } +func TestHandleAddModel_NormalizesRedundantExplicitProviderPrefix(t *testing.T) { + configPath, cleanup := setupOAuthTestEnv(t) + defer cleanup() + + h := NewHandler(configPath) + mux := http.NewServeMux() + h.RegisterRoutes(mux) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/api/models", bytes.NewBufferString(`{ + "model_name":"openai-gpt", + "provider":"openai", + "model":"openai/gpt-4o-mini", + "api_key":"sk-openai" + }`)) + req.Header.Set("Content-Type", "application/json") + mux.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d, body=%s", rec.Code, http.StatusOK, rec.Body.String()) + } + + cfg, err := config.LoadConfig(configPath) + if err != nil { + t.Fatalf("LoadConfig() error = %v", err) + } + added := cfg.ModelList[len(cfg.ModelList)-1] + if got := added.Provider; got != "openai" { + t.Fatalf("provider = %q, want %q", got, "openai") + } + if got := added.Model; got != "gpt-4o-mini" { + t.Fatalf("model = %q, want %q", got, "gpt-4o-mini") + } +} + func TestHandleAddModel_PersistsCustomHeaders(t *testing.T) { configPath, cleanup := setupOAuthTestEnv(t) defer cleanup() @@ -699,6 +734,210 @@ func TestHandleUpdateModel_PreservesProviderWhenOmitted(t *testing.T) { } } +func TestHandleUpdateModel_NormalizesRedundantExplicitProviderPrefix(t *testing.T) { + configPath, cleanup := setupOAuthTestEnv(t) + defer cleanup() + + cfg, err := config.LoadConfig(configPath) + if err != nil { + t.Fatalf("LoadConfig() error = %v", err) + } + cfg.ModelList = []*config.ModelConfig{{ + ModelName: "editable", + Model: "gpt-4o", + Provider: "openai", + }} + if err := config.SaveConfig(configPath, cfg); err != nil { + t.Fatalf("SaveConfig() error = %v", err) + } + + h := NewHandler(configPath) + mux := http.NewServeMux() + h.RegisterRoutes(mux) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPut, "/api/models/0", bytes.NewBufferString(`{ + "model_name":"editable", + "provider":"openai", + "model":"openai/gpt-5.4" + }`)) + req.Header.Set("Content-Type", "application/json") + mux.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d, body=%s", rec.Code, http.StatusOK, rec.Body.String()) + } + + updated, err := config.LoadConfig(configPath) + if err != nil { + t.Fatalf("LoadConfig() error = %v", err) + } + if got := updated.ModelList[0].Provider; got != "openai" { + t.Fatalf("provider = %q, want %q", got, "openai") + } + if got := updated.ModelList[0].Model; got != "gpt-5.4" { + t.Fatalf("model = %q, want %q", got, "gpt-5.4") + } +} + +func TestHandleUpdateModel_PreservesLegacyModelPrefixWhenProviderOmitted(t *testing.T) { + configPath, cleanup := setupOAuthTestEnv(t) + defer cleanup() + + cfg, err := config.LoadConfig(configPath) + if err != nil { + t.Fatalf("LoadConfig() error = %v", err) + } + cfg.ModelList = []*config.ModelConfig{{ + ModelName: "legacy-openrouter", + Model: "openrouter/openai/gpt-5.4", + }} + if err := config.SaveConfig(configPath, cfg); err != nil { + t.Fatalf("SaveConfig() error = %v", err) + } + + h := NewHandler(configPath) + mux := http.NewServeMux() + h.RegisterRoutes(mux) + + // Simulate an older client: it reads GET /api/models, ignores the new + // provider field, then PUTs the visible model string back unchanged. + recList := httptest.NewRecorder() + reqList := httptest.NewRequest(http.MethodGet, "/api/models", nil) + mux.ServeHTTP(recList, reqList) + + if recList.Code != http.StatusOK { + t.Fatalf("list status = %d, want %d, body=%s", recList.Code, http.StatusOK, recList.Body.String()) + } + + var listResp struct { + Models []modelResponse `json:"models"` + } + if err := json.Unmarshal(recList.Body.Bytes(), &listResp); err != nil { + t.Fatalf("Unmarshal() error = %v", err) + } + if len(listResp.Models) != 1 { + t.Fatalf("len(models) = %d, want 1", len(listResp.Models)) + } + if got := listResp.Models[0].Provider; got != "openrouter" { + t.Fatalf("provider = %q, want %q", got, "openrouter") + } + if got := listResp.Models[0].Model; got != "openai/gpt-5.4" { + t.Fatalf("model = %q, want %q", got, "openai/gpt-5.4") + } + + recUpdate := httptest.NewRecorder() + reqUpdate := httptest.NewRequest(http.MethodPut, "/api/models/0", bytes.NewBufferString(`{ + "model_name":"legacy-openrouter", + "model":"openai/gpt-5.4" + }`)) + reqUpdate.Header.Set("Content-Type", "application/json") + mux.ServeHTTP(recUpdate, reqUpdate) + + if recUpdate.Code != http.StatusOK { + t.Fatalf("update status = %d, want %d, body=%s", recUpdate.Code, http.StatusOK, recUpdate.Body.String()) + } + + updated, err := config.LoadConfig(configPath) + if err != nil { + t.Fatalf("LoadConfig() error = %v", err) + } + if got := updated.ModelList[0].Provider; got != "" { + t.Fatalf("provider = %q, want empty", got) + } + if got := updated.ModelList[0].Model; got != "openrouter/openai/gpt-5.4" { + t.Fatalf("model = %q, want %q", got, "openrouter/openai/gpt-5.4") + } +} + +func TestHandleUpdateModel_PreservesLegacyModelPrefixWhenProviderOmittedAndModelChanges(t *testing.T) { + configPath, cleanup := setupOAuthTestEnv(t) + defer cleanup() + + cfg, err := config.LoadConfig(configPath) + if err != nil { + t.Fatalf("LoadConfig() error = %v", err) + } + cfg.ModelList = []*config.ModelConfig{{ + ModelName: "legacy-openrouter", + Model: "openrouter/openai/gpt-5.4", + }} + if err := config.SaveConfig(configPath, cfg); err != nil { + t.Fatalf("SaveConfig() error = %v", err) + } + + h := NewHandler(configPath) + mux := http.NewServeMux() + h.RegisterRoutes(mux) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPut, "/api/models/0", bytes.NewBufferString(`{ + "model_name":"legacy-openrouter", + "model":"openai/gpt-5.5" + }`)) + req.Header.Set("Content-Type", "application/json") + mux.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d, body=%s", rec.Code, http.StatusOK, rec.Body.String()) + } + + updated, err := config.LoadConfig(configPath) + if err != nil { + t.Fatalf("LoadConfig() error = %v", err) + } + if got := updated.ModelList[0].Provider; got != "" { + t.Fatalf("provider = %q, want empty", got) + } + if got := updated.ModelList[0].Model; got != "openrouter/openai/gpt-5.5" { + t.Fatalf("model = %q, want %q", got, "openrouter/openai/gpt-5.5") + } +} + +func TestHandleUpdateModel_OmittedProviderAllowsLegacyProviderChangeForSimpleVisibleModel(t *testing.T) { + configPath, cleanup := setupOAuthTestEnv(t) + defer cleanup() + + cfg, err := config.LoadConfig(configPath) + if err != nil { + t.Fatalf("LoadConfig() error = %v", err) + } + cfg.ModelList = []*config.ModelConfig{{ + ModelName: "legacy-openai", + Model: "openai/gpt-5.4", + }} + if err := config.SaveConfig(configPath, cfg); err != nil { + t.Fatalf("SaveConfig() error = %v", err) + } + + h := NewHandler(configPath) + mux := http.NewServeMux() + h.RegisterRoutes(mux) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPut, "/api/models/0", bytes.NewBufferString(`{ + "model_name":"legacy-openai", + "model":"anthropic/claude-sonnet-4.6" + }`)) + req.Header.Set("Content-Type", "application/json") + mux.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d, body=%s", rec.Code, http.StatusOK, rec.Body.String()) + } + + updated, err := config.LoadConfig(configPath) + if err != nil { + t.Fatalf("LoadConfig() error = %v", err) + } + if got := updated.ModelList[0].Provider; got != "" { + t.Fatalf("provider = %q, want empty", got) + } + if got := updated.ModelList[0].Model; got != "anthropic/claude-sonnet-4.6" { + t.Fatalf("model = %q, want %q", got, "anthropic/claude-sonnet-4.6") + } +} + func TestHandleListModels_ReturnsProviderField(t *testing.T) { configPath, cleanup := setupOAuthTestEnv(t) defer cleanup() diff --git a/web/frontend/src/components/models/edit-model-sheet.tsx b/web/frontend/src/components/models/edit-model-sheet.tsx index 23d057073..a56f67326 100644 --- a/web/frontend/src/components/models/edit-model-sheet.tsx +++ b/web/frontend/src/components/models/edit-model-sheet.tsx @@ -118,7 +118,7 @@ export function EditModelSheet({ try { await updateModel(model.index, { model_name: model.model_name, - provider: form.provider.trim() || undefined, + provider: form.provider.trim(), model: form.modelId.trim(), api_base: form.apiBase || undefined, api_key: form.apiKey || undefined,