diff --git a/frontend/bindings/github.com/MaimoryLab/OneAgent/internal/binding/models.ts b/frontend/bindings/github.com/MaimoryLab/OneAgent/internal/binding/models.ts index be58c2a2..fdc84dd6 100644 --- a/frontend/bindings/github.com/MaimoryLab/OneAgent/internal/binding/models.ts +++ b/frontend/bindings/github.com/MaimoryLab/OneAgent/internal/binding/models.ts @@ -105,6 +105,14 @@ export interface ModelsResponse { "retryable": boolean; "protocol"?: string | null; "protocols"?: { [_ in string]?: ProbeResponse } | null; + + /** + * Model is the ID that was probed, and AutoSelectedModel says OneAgent chose + * it rather than the user. A failure on a model we picked is not evidence + * about the user's key, and the UI has to be able to say which it is. + */ + "model"?: string; + "auto_selected_model"?: boolean; "models": string[] | null; } @@ -136,6 +144,14 @@ export interface ProbeResponse { "retryable": boolean; "protocol"?: string | null; "protocols"?: { [_ in string]?: ProbeResponse } | null; + + /** + * Model is the ID that was probed, and AutoSelectedModel says OneAgent chose + * it rather than the user. A failure on a model we picked is not evidence + * about the user's key, and the UI has to be able to say which it is. + */ + "model"?: string; + "auto_selected_model"?: boolean; } export interface ProviderIDRequest { diff --git a/frontend/src/components/ConnectionStatus.test.tsx b/frontend/src/components/ConnectionStatus.test.tsx new file mode 100644 index 00000000..3942d403 --- /dev/null +++ b/frontend/src/components/ConnectionStatus.test.tsx @@ -0,0 +1,50 @@ +import { render, screen } from "@testing-library/react"; +import { describe, expect, it } from "vitest"; + +import { I18nProvider } from "../i18n"; +import type { ProbeResponse } from "../types/api"; +import { ConnectionStatus } from "./ConnectionStatus"; + +function probe(overrides: Partial = {}): ProbeResponse { + return { + ok: false, reachable: true, status: 400, message: "模型请求被拒绝", + error_code: "PROTOCOL_UNSUPPORTED", retryable: false, ...overrides, + } as ProbeResponse; +} + +// jsdom reports navigator.language as en-US, so I18nProvider resolves to English +// and the assertions below match the translated strings rather than the zh keys. +function show(result: ProbeResponse) { + render(); +} + +describe("ConnectionStatus", () => { + // A Provider's catalogue is mostly image, video and audio generators. One of + // those rejecting a chat payload used to be indistinguishable from a bad key, + // so the user read a wrong verdict about their credentials during setup. + it("names a model it chose itself when the probe fails", () => { + show(probe({ model: "wan-ai/wan2.1-t2v-14b", auto_selected_model: true })); + expect(screen.getByText(/wan-ai\/wan2\.1-t2v-14b/)).toBeTruthy(); + expect(screen.getByText(/OneAgent chose the model/)).toBeTruthy(); + }); + + it("stays quiet about the model the user chose themselves", () => { + // Their override, so the failure is the answer they asked for; explaining our + // choice would be both wrong and confusing. + show(probe({ model: "kwai/kling-v1-video", auto_selected_model: false })); + expect(screen.getByText("模型请求被拒绝")).toBeTruthy(); + expect(screen.queryByText(/OneAgent chose the model/)).toBeNull(); + }); + + it("does not blame the model when the key itself was rejected", () => { + // A rejected key is about the key whatever model carried the request. + show(probe({ error_code: "API_KEY_REJECTED", message: "API Key 无效", model: "wan-ai/wan2.1-t2v-14b", auto_selected_model: true })); + expect(screen.getByText("API Key 无效")).toBeTruthy(); + expect(screen.queryByText(/OneAgent chose the model/)).toBeNull(); + }); + + it("reports a successful probe with the provider's own message", () => { + show(probe({ ok: true, status: 200, message: "连接正常", error_code: null })); + expect(screen.getByText("连接正常")).toBeTruthy(); + }); +}); diff --git a/frontend/src/components/ConnectionStatus.tsx b/frontend/src/components/ConnectionStatus.tsx index 5e1a49ae..3c0c1a5a 100644 --- a/frontend/src/components/ConnectionStatus.tsx +++ b/frontend/src/components/ConnectionStatus.tsx @@ -31,10 +31,21 @@ export function ConnectionStatus({ state, result }: { state: AsyncState; result: ); } const rejected = result?.error_code === "API_KEY_REJECTED"; + // A model OneAgent chose is named in the failure. A Provider's catalogue is + // mostly image, video and audio generators, and one of those rejecting a chat + // payload otherwise reads as a broken key -- a wrong verdict about the user's + // credentials. Not shown when the key itself was rejected, which is about the + // key whatever model was used, nor when the user named the model themselves. + const blamedModel = !rejected && result?.auto_selected_model ? result.model : ""; return (
{rejected ? : } - {result?.message || t("连接失败")} + + {result?.message || t("连接失败")} + {blamedModel ? ( + {t("测试使用的模型 {model} 由 OneAgent 自动选择，可能不支持对话。可在上方自定义模型名称后重试", { model: blamedModel })} + ) : null} +
); } diff --git a/frontend/src/i18n.tsx b/frontend/src/i18n.tsx index 189b9e28..c1323ca6 100644 --- a/frontend/src/i18n.tsx +++ b/frontend/src/i18n.tsx @@ -105,6 +105,10 @@ const english = { "尚未测试连接": "Connection not tested", "正在验证端点和 Key": "Validating endpoint and key", "连接失败": "Connection failed", + // Shown when the probe failed on a model OneAgent picked rather than one the + // user typed. Without it, a video or image model rejecting a chat request reads + // as a rejected API key. + "测试使用的模型 {model} 由 OneAgent 自动选择，可能不支持对话。可在上方自定义模型名称后重试": "OneAgent chose the model {model} for this test, and it may not support chat. Enter a model name above and try again", "查看安装日志": "View installation log", "任务中心": "Task center", "有任务正在运行": "A task is running", diff --git a/frontend/src/styles/app.css b/frontend/src/styles/app.css index b4fb2573..d9918f73 100644 --- a/frontend/src/styles/app.css +++ b/frontend/src/styles/app.css @@ -801,6 +801,28 @@ .inline-status.status-warning { color: var(--orange); background: transparent; } .inline-status.status-error { color: var(--red); } +/* A failure can carry a second line explaining that the probed model was chosen + for the user. align-items moves to the top so the icon stays beside the first + line rather than centring against a two-line block. */ +.inline-status:has(small) { + align-items: flex-start; + padding: 9px 0; +} + +.inline-status > span { + min-width: 0; + display: grid; + gap: 3px; +} + +/* Inherits neither the colour nor the weight of the error above it: the + explanation is context, not a second alarm. */ +.inline-status small { + color: var(--text-secondary); + font-size: 12px; + line-height: 1.45; +} + .model-picker { width: 100%; display: grid; diff --git a/internal/app/agent.go b/internal/app/agent.go index 300ebb1d..f41035cf 100644 --- a/internal/app/agent.go +++ b/internal/app/agent.go @@ -101,7 +101,9 @@ func (u *UseCases) activateAgentLocked(ctx context.Context, options ActivateAgen return ActivateAgentResult{}, err } - model, err := u.resolveProviderModel(ctx, target, apiKey, options.Model) + // The auto-selected flag only matters to the probe, which has to explain a + // failure; activation just needs a usable model. + model, _, err := u.resolveProviderModel(ctx, target, apiKey, options.Model) if err != nil { return ActivateAgentResult{}, err } diff --git a/internal/app/install.go b/internal/app/install.go index fbb2f80b..539a67ff 100644 --- a/internal/app/install.go +++ b/internal/app/install.go @@ -194,7 +194,7 @@ func (u *UseCases) validateInstall(ctx context.Context, manifest catalog.Manifes if u.provider == nil { return options, oneerrors.New(oneerrors.InternalError, "Model discovery is not configured", oneerrors.WithStatus(501)) } - model, err := u.resolveProviderModel(ctx, target, options.APIKey, "") + model, _, err := u.resolveProviderModel(ctx, target, options.APIKey, "") if err != nil { return options, err } diff --git a/internal/app/provider.go b/internal/app/provider.go index f2847da6..4859947e 100644 --- a/internal/app/provider.go +++ b/internal/app/provider.go @@ -3,6 +3,7 @@ package app import ( "context" "fmt" + "slices" "sort" "strings" "sync" @@ -25,6 +26,13 @@ type ProviderProbeOptions struct { type ProviderProbeResult struct { Primary provider.ProbeResult Protocols map[string]provider.ProbeResult + // Model is the ID actually probed, and AutoSelectedModel says we chose it + // rather than the user. Together they let a failure distinguish "your key is + // wrong" from "we picked a model this endpoint does not serve for chat" -- + // indistinguishable before, which is what made a bad auto-pick read as a + // credential problem. + Model string + AutoSelectedModel bool } // SaveProviderResult reports which Agents were rewritten after the edit so the @@ -50,7 +58,7 @@ func (u *UseCases) ProbeProvider(ctx context.Context, options ProviderProbeOptio if apiKey == "" { apiKey = target.APIKey } - model, err := u.resolveProviderModel(ctx, target, apiKey, options.Model) + model, autoSelected, err := u.resolveProviderModel(ctx, target, apiKey, options.Model) if err != nil { return ProviderProbeResult{}, err } @@ -73,7 +81,7 @@ func (u *UseCases) ProbeProvider(ctx context.Context, options ProviderProbeOptio } } primary.OK = allOK - return ProviderProbeResult{Primary: primary, Protocols: results}, nil + return ProviderProbeResult{Primary: primary, Protocols: results, Model: model, AutoSelectedModel: autoSelected}, nil } func (u *UseCases) probeProtocols(ctx context.Context, protocols []string, apiKey, model string, baseFor func(string) string) (map[string]provider.ProbeResult, error) { @@ -265,21 +273,37 @@ func (u *UseCases) DeleteProvider(ctx context.Context, providerID string) error return u.providers.Delete(ctx, providerID) } -func (u *UseCases) resolveProviderModel(ctx context.Context, target provider.Entry, apiKey, model string) (string, error) { +// resolveProviderModel decides which model the connection probe sends a chat +// payload to. The second return value reports whether the choice was ours rather +// than the user's, so a failure can say which of the two it is describing. +// +// Preference order, and why: a model the user typed wins outright, because the +// probe model is explicitly their override and a failure on it is the answer they +// asked for. Otherwise the Provider's manifest model wins over anything picked out +// of the live catalogue — it is a reviewed, known-chat ID for that Provider, while +// the catalogue of an aggregator is mostly video, image and audio generators whose +// names no denylist will ever fully enumerate. PickChatModel is the last resort, +// for a custom endpoint or a Provider whose manifest model it no longer serves. +func (u *UseCases) resolveProviderModel(ctx context.Context, target provider.Entry, apiKey, model string) (string, bool, error) { if model = strings.TrimSpace(model); model != "" { - return model, nil + return model, false, nil } if apiKey == "" { - return target.FallbackModel, nil + return target.FallbackModel, true, nil } listing, err := u.provider.ListModels(ctx, "custom", apiKey, target.BaseURL) if err != nil { - return "", err + return "", true, err } if listing.OK && len(listing.Models) > 0 { - return provider.PickChatModel(listing.Models), nil + // Only when the Provider actually serves it: probing a manifest model the + // endpoint has dropped would fail for a reason the user cannot act on. + if fallback := strings.TrimSpace(target.FallbackModel); fallback != "" && slices.Contains(listing.Models, fallback) { + return fallback, true, nil + } + return provider.PickChatModel(listing.Models), true, nil } - return target.FallbackModel, nil + return target.FallbackModel, true, nil } func protocolsForAgents(agentIDs []string) ([]string, error) { diff --git a/internal/app/provider_test.go b/internal/app/provider_test.go index 9eff56a7..4e15231e 100644 --- a/internal/app/provider_test.go +++ b/internal/app/provider_test.go @@ -304,3 +304,57 @@ func TestDeleteIgnoresStaleBindingWithoutAgentConfig(t *testing.T) { t.Fatalf("stale binding blocked Provider deletion: %v", err) } } + +// A built-in Provider's catalogue is mostly not chat models. With an empty probe +// model the live list used to win outright, so a video generator returned first +// became the model the connection test sent a chat payload to -- and the failure +// read as "your key is broken" during first-run setup. +func TestProbeProviderPrefersTheManifestModelOverAVideoModel(t *testing.T) { + var probed string + core := providerUseCases(t, appProviderDoer(func(request *http.Request) (*http.Response, error) { + if request.URL.Path == "/openai/v1/models" { + // Ordered as an aggregator really does: generators first. + return appProviderResponse(http.StatusOK, `{"data":[ + {"id":"wan-ai/wan2.1-t2v-14b"}, + {"id":"kwai/kling-v1-video"}, + {"id":"deepseek/deepseek-v4-flash"}, + {"id":"deepseek/deepseek-v4-pro"} + ]}`), nil + } + body, _ := io.ReadAll(request.Body) + probed = string(body) + return appProviderResponse(http.StatusNoContent, ""), nil + })) + result, err := core.ProbeProvider(context.Background(), ProviderProbeOptions{ + Provider: "ppio", APIKey: "key", AgentIDs: []string{"opencode"}, + }) + if err != nil || !result.Primary.OK { + t.Fatalf("probe = %#v, err=%v", result, err) + } + // The manifest's reviewed chat model, not the first survivor of the denylist. + if !strings.Contains(probed, "deepseek/deepseek-v4-flash") { + t.Fatalf("probed payload did not use the manifest model: %s", probed) + } +} + +// A model the user typed is their override and must be probed verbatim, even when +// it is one the denylist would reject: a failure they asked for is information. +func TestProbeProviderKeepsAModelTheUserTyped(t *testing.T) { + var probed string + core := providerUseCases(t, appProviderDoer(func(request *http.Request) (*http.Response, error) { + if request.URL.Path == "/openai/v1/models" { + t.Fatal("discovery ran even though the user named a model") + } + body, _ := io.ReadAll(request.Body) + probed = string(body) + return appProviderResponse(http.StatusNoContent, ""), nil + })) + if _, err := core.ProbeProvider(context.Background(), ProviderProbeOptions{ + Provider: "ppio", APIKey: "key", Model: "kwai/kling-v1-video", AgentIDs: []string{"opencode"}, + }); err != nil { + t.Fatal(err) + } + if !strings.Contains(probed, "kwai/kling-v1-video") { + t.Fatalf("user's model was replaced: %s", probed) + } +} diff --git a/internal/binding/services.go b/internal/binding/services.go index 53c16146..b1fdf21f 100644 --- a/internal/binding/services.go +++ b/internal/binding/services.go @@ -202,6 +202,10 @@ func (s *ProviderService) Probe(ctx context.Context, request ProbeRequest) (Prob return ProbeResponse{}, err } response := probeResponse(result.Primary) + // Set on the top-level response only: the per-protocol entries all probed the + // same model, so repeating it there would suggest they could differ. + response.Model = result.Model + response.AutoSelectedModel = result.AutoSelectedModel response.Protocols = make(map[string]ProbeResponse, len(result.Protocols)) for protocolID, protocolResult := range result.Protocols { response.Protocols[protocolID] = probeResponse(protocolResult) @@ -511,6 +515,11 @@ type ProbeResponse struct { Retryable bool `json:"retryable"` Protocol *string `json:"protocol,omitempty"` Protocols map[string]ProbeResponse `json:"protocols,omitempty"` + // Model is the ID that was probed, and AutoSelectedModel says OneAgent chose + // it rather than the user. A failure on a model we picked is not evidence + // about the user's key, and the UI has to be able to say which it is. + Model string `json:"model,omitempty"` + AutoSelectedModel bool `json:"auto_selected_model,omitempty"` } type ModelsResponse struct { diff --git a/internal/provider/provider.go b/internal/provider/provider.go index a7b65f02..6c6ce4f9 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -18,7 +18,13 @@ const ( ProtocolResponses = catalog.ProtocolResponses ) -var nonChatModel = regexp.MustCompile(`(?i)(^|[-_/.])(embed(ding)?s?|rerank(er)?s?|ocr|whisper|asr|tts|speech|vl|vision|image|sdx?|flux|guard(rail)?s?|moderation|sql)([-_/.]|$)`) +// nonChatModel matches IDs that clearly name a non-chat endpoint. It is defence +// in depth, not the primary guard: a denylist over third-party model IDs always +// lags the vendors, so callers that have a reviewed model for the Provider should +// prefer that (see app.resolveProviderModel). The video, audio and music terms +// were added after a t2v model returned first by an aggregator got probed with a +// chat payload and the failure read as a bad API key. +var nonChatModel = regexp.MustCompile(`(?i)(^|[-_/.])(embed(ding)?s?|rerank(er)?s?|ocr|whisper|asr|tts|speech|voice|audio|music|bark|vl|vision|image|img|sdx?|flux|video|t2v|i2v|v2v|t2i|i2t|t2a|sora|veo|kling|hailuo|seedance|cogvideox?|wan[0-9.]*|guard(rail)?s?|moderation|sql)([-_/.]|$)`) // ValidateBaseURL accepts an explicit HTTP(S) origin or path and rejects the // forms that could smuggle credentials or control characters into requests. diff --git a/internal/provider/provider_test.go b/internal/provider/provider_test.go index 8aee87a5..4e620b38 100644 --- a/internal/provider/provider_test.go +++ b/internal/provider/provider_test.go @@ -94,3 +94,35 @@ func TestInvalidProtocolErrorIsStable(t *testing.T) { t.Fatalf("invalid protocol error = %v", err) } } + +// The generator IDs the aggregators OneAgent ships actually return. Before the +// video and audio terms were added, every one of these was treated as a chat +// model, so an aggregator listing one first had it probed with a chat payload. +func TestPickChatModelSkipsGeneratorModels(t *testing.T) { + generators := []string{ + "wan-ai/wan2.1-t2v-14b", "kwai/kling-v1-video", "sora-2", "veo-3.0-generate-001", + "zai-org/cogvideox-5b", "tencent/hunyuan-video", "bytedance/seedance-1-0-pro", + "minimaxai/minimax-hailuo-02", "stabilityai/stable-video-diffusion", "suno/bark", + "black-forest-labs/flux-1-schnell", "qwen/qwen-image-edit", "minimaxai/minimax-speech-02", + } + for _, id := range generators { + if got := PickChatModel([]string{id, "deepseek/deepseek-v4-pro"}); got != "deepseek/deepseek-v4-pro" { + t.Errorf("PickChatModel picked the generator %q", got) + } + } +} + +// The other half of the denylist's contract, and the reason it cannot simply be +// made broader: a term like "wan" or "veo" that matched real chat model families +// would push the probe onto models[0] and reintroduce the bug from the other side. +func TestPickChatModelKeepsRealChatModels(t *testing.T) { + for _, id := range []string{ + "deepseek/deepseek-v4-pro", "deepseek/deepseek-v4-flash", "gpt-5.6-terra", "claude-fable-5", + "qwen/qwen3-235b-a22b", "moonshotai/kimi-k2", "meta-llama/llama-4-maverick", + "zai-org/glm-4.6", "minimaxai/minimax-m2", "openai/gpt-oss-120b", + } { + if got := PickChatModel([]string{id}); got != id { + t.Errorf("chat model %q was classified as non-chat", id) + } + } +}