diff --git a/.env.example b/.env.example index 955c225..b9dd263 100644 --- a/.env.example +++ b/.env.example @@ -7,6 +7,7 @@ OLLAMA_API_KEY=your-ollama-cloud-key-here # Built-in provider keys (each optional; only needed for the providers you use). #OPENAI_API_KEY=sk-... #KIMI_API_KEY=sk-... # Moonshot AI (Kimi); provider name "kimi" +#QWEN_API_KEY=sk-... # Alibaba Model Studio (Qwen); provider name "qwen" #ANTHROPIC_API_KEY=sk-ant-... #GOOGLE_API_KEY=... diff --git a/README.md b/README.md index 0ecaf6b..9e5324c 100644 --- a/README.md +++ b/README.md @@ -122,6 +122,7 @@ Chains are health-tracked per target: |----------|-----------|-------------|------------------| | OpenAI (+compatible) | `openai` | `OPENAI_API_KEY` | https://api.openai.com/v1 | | Kimi (Moonshot AI) | `kimi` | `KIMI_API_KEY` | https://api.moonshot.ai/v1 | +| Qwen (Alibaba) | `qwen` | `QWEN_API_KEY` | https://dashscope-intl.aliyuncs.com/compatible-mode/v1 | | Anthropic (+compatible) | `anthropic` | `ANTHROPIC_API_KEY` | https://api.anthropic.com | | Google (Gemini) | `google` | `GOOGLE_API_KEY` / `GEMINI_API_KEY` | Gemini API (official SDK) | | Ollama Cloud | `ollama-cloud` | `OLLAMA_API_KEY` | https://ollama.com | @@ -134,6 +135,19 @@ the openai client (like llama-swap). The `kimi` built-in defaults to the international endpoint; reach the China endpoint (or any other host) with a `kimi://` DSN, e.g. `LLM_KCN=kimi://token@api.moonshot.cn/v1`. +Qwen is the same shape: Alibaba Model Studio's OpenAI-compatible mode, reusing +the openai client. The `qwen` built-in defaults to the international +(Singapore) host; reach the China host or a workspace-scoped regional one with +a `qwen://` DSN, e.g. +`LLM_QCN=qwen://token@dashscope.aliyuncs.com/compatible-mode/v1`. Model Studio +also fronts the same models with an Anthropic-compatible `/v1/messages` shim — +majordomo does **not** use it, because on that surface `reasoning_effort` is +dropped, `Request.Schema` stops being enforced, and cached-token accounting +disappears; see [ADR-0027](docs/adr/0027-qwen-builtin.md). Two Alibaba-side +quirks are worth knowing: thinking is on by default for some models (e.g. +`qwen3.7-plus`), and the Qwen3 open-source models require streaming while +thinking, so buffered `Generate` calls want a Max/Plus model. + OpenAI-compatible / Anthropic-compatible endpoints: construct the provider with a name and base URL and register it — @@ -165,7 +179,7 @@ m, _ := reg.Parse("m5/qwen3:30b,m1/qwen3:30b,thinking") ``` DSN format: `scheme://[token@]host[/path]`, scheme ∈ `foreman`, `ollama`, -`ollama-cloud`, `openai`, `kimi`, `anthropic`, `google`/`gemini`, `llama-swap`, +`ollama-cloud`, `openai`, `kimi`, `qwen`, `anthropic`, `google`/`gemini`, `llama-swap`, `llama-swaps`, or any scheme you add with `RegisterScheme`. The token is the credential (bearer token / API key); the base URL is always `https://host[/path]` — except `llama-swap`, which builds `http://host[:port]` since it's local-first @@ -420,6 +434,7 @@ to build one. |----------------------|:---:|:---:|:---:|:---:|:---:|:---:|:---:| | OpenAI (+compatible) | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | Kimi (Moonshot AI) | ✅ | ✅ | ✅ | ✅ | ✅ | ✅³ | ✅ | +| Qwen (Alibaba) | ✅ | ✅ | ✅ | ✅ | ✅⁴ | ✅⁴ | ✅ | | Anthropic (+compat) | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | Google (Gemini) | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | Ollama Cloud | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | @@ -444,6 +459,13 @@ probe and management methods on `*llamaswap.Provider`. level; whether a call succeeds depends on the Moonshot model — only the vision variants (e.g. `moonshot-v1-8k-vision-preview`) accept images. +⁴ Qwen also reuses the openai client (ADR-0027), so both columns are present at +the client level and gated by the Model Studio model you name: `json_schema` +structured output is on the Max/Plus families, image inputs on the `qwen-vl-*` +/ `qwen3-vl-*` models. `reasoning_effort` rides through as a top-level field — +one reason the built-in speaks OpenAI-compat rather than Model Studio's +Anthropic-compat shim. + Notes: Ollama has no native tool_choice — `"none"` drops the tools; `"required"`/named choices are best-effort ignored there. Ollama Cloud ignores the `format` field (verified live), so the provider also states diff --git a/builtin.go b/builtin.go index 1b39c05..b0301b1 100644 --- a/builtin.go +++ b/builtin.go @@ -2,7 +2,6 @@ package majordomo import ( "net/http" - "strings" "gitea.stevedudenhoeffer.com/steve/majordomo/llm" "gitea.stevedudenhoeffer.com/steve/majordomo/provider/anthropic" @@ -18,7 +17,13 @@ const ( // ProviderKimi is Moonshot AI's Kimi models over their OpenAI-compatible // Chat Completions endpoint. Reuses the openai client (like llama-swap); // keyed by KIMI_API_KEY, default base URL kimiBaseURL. - ProviderKimi = "kimi" + ProviderKimi = "kimi" + // ProviderQwen is Alibaba's Qwen models over Model Studio's + // OpenAI-compatible Chat Completions endpoint. Reuses the openai client + // (like kimi and llama-swap); keyed by QWEN_API_KEY, default base URL + // qwenBaseURL. ADR-0027 records why the OpenAI surface and not the + // Anthropic-compatible one Model Studio also exposes. + ProviderQwen = "qwen" ProviderAnthropic = "anthropic" ProviderGoogle = "google" ProviderOllama = "ollama" @@ -37,6 +42,55 @@ const ( // China endpoint (api.moonshot.cn/v1) is reachable via a kimi:// LLM_* DSN. const kimiBaseURL = "https://api.moonshot.ai/v1" +// qwenBaseURL is Alibaba Model Studio's international (Singapore) endpoint in +// OpenAI-compatible mode. The China endpoint +// (dashscope.aliyuncs.com/compatible-mode/v1) and any regional host are +// reachable via a qwen:// LLM_* DSN. +const qwenBaseURL = "https://dashscope-intl.aliyuncs.com/compatible-mode/v1" + +// openaiCompatScheme builds the DSN factory shared by every built-in that is +// "the openai client pointed somewhere else" (kimi, qwen, ...). The provider +// is named after the LLM_ var that defined it, takes its credential from +// the DSN token — not the built-in's own env var, which does nothing for a +// DSN-defined provider — and so names that same LLM_ var in the +// missing-key hint, matching the lazy-resolution key form in providerFor. +// +// wrap is the caller's option-decorator (it injects the registry's HTTP +// client), so a DSN provider is built exactly like the eager built-ins. +func openaiCompatScheme(wrap func(...openai.Option) []openai.Option) SchemeFactory { + return func(name string, dsn DSN) (llm.Provider, error) { + return openai.New(wrap( + openai.WithName(name), + openai.WithBaseURL(dsn.BaseURL()), + openai.WithAPIKey(dsn.Token), + openai.WithAPIKeyName(envKeyForProvider(name)), + )...), nil + } +} + +// registerOpenAICompatBuiltin installs BOTH halves of an OpenAI-compat +// built-in: the eager provider under name (credential from keyEnv) and the +// matching name:// DSN scheme. Why both in one call: the two halves are a pair +// — a built-in whose scheme is missing resolves as a spec but not from an +// LLM_* DSN, and the credential rules below have to hold identically in each. +// Adding the next one is a single line rather than six lines to copy. +// +// The two credential rules, holding by construction for every caller: +// - WithAPIKey is passed UNCONDITIONALLY, even when the lookup comes back +// empty. openai.New defaults its key to OPENAI_API_KEY, so anything less +// lets an unset keyEnv silently authenticate as OpenAI. +// - WithAPIKeyName makes the synthetic-401 hint name keyEnv, so a keyless +// call tells the operator the variable that actually fixes it. +func registerOpenAICompatBuiltin(r *Registry, wrap func(...openai.Option) []openai.Option, name, baseURL, keyEnv string) { + r.providers[name] = openai.New(wrap( + openai.WithName(name), + openai.WithBaseURL(baseURL), + openai.WithAPIKey(r.envLookup(keyEnv)), + openai.WithAPIKeyName(keyEnv), + )...) + r.schemes[name] = openaiCompatScheme(wrap) +} + // registerBuiltins installs the built-in providers and env-DSN scheme // factories into a fresh registry. httpClient, when non-nil, is used by // every provider and factory the registry itself constructs. @@ -83,32 +137,19 @@ func registerBuiltins(r *Registry, httpClient *http.Client) { )...), nil } - // Kimi (Moonshot AI): OpenAI-compatible Chat Completions, so it reuses the - // openai client (like llama-swap). Defaults to Moonshot's international - // endpoint and the KIMI_API_KEY credential. WithAPIKey is passed - // unconditionally — even empty — so an unset KIMI_API_KEY can never fall - // through to the openai client's OPENAI_API_KEY default; WithAPIKeyName - // makes the missing-key error name KIMI_API_KEY. - r.providers[ProviderKimi] = openai.New(openaiOpts( - openai.WithName(ProviderKimi), - openai.WithBaseURL(kimiBaseURL), - openai.WithAPIKey(r.envLookup("KIMI_API_KEY")), - openai.WithAPIKeyName("KIMI_API_KEY"), - )...) - // kimi:// DSN scheme: an OpenAI-compatible target labeled kimi, base URL - // from the DSN host (e.g. kimi://tok@api.moonshot.cn/v1 for China). Its - // credential is the DSN token, not KIMI_API_KEY, so the missing-key hint - // names the LLM_ env var that defines this provider (matching the - // lazy-resolution key form in providerFor) — the fix for a keyless target - // here is adding a token to that DSN. - r.schemes[ProviderKimi] = func(name string, dsn DSN) (llm.Provider, error) { - return openai.New(openaiOpts( - openai.WithName(name), - openai.WithBaseURL(dsn.BaseURL()), - openai.WithAPIKey(dsn.Token), - openai.WithAPIKeyName("LLM_"+strings.ToUpper(strings.ReplaceAll(name, "-", "_"))), - )...), nil - } + // Third-party endpoints that ARE the openai client at another base URL — + // no new package, mirroring llama-swap's chat path. Each gets the eager + // built-in plus its name:// DSN scheme, and the credential rules hold by + // construction (see registerOpenAICompatBuiltin). + // + // kimi (ADR-0026): Moonshot's international endpoint; China host via + // kimi://tok@api.moonshot.cn/v1. + registerOpenAICompatBuiltin(r, openaiOpts, ProviderKimi, kimiBaseURL, "KIMI_API_KEY") + // qwen (ADR-0027): Alibaba Model Studio's international host. Model Studio + // also exposes an Anthropic-compatible endpoint; the ADR records why the + // OpenAI one is the built-in. China / workspace-scoped regional hosts via + // qwen://tok@dashscope.aliyuncs.com/compatible-mode/v1. + registerOpenAICompatBuiltin(r, openaiOpts, ProviderQwen, qwenBaseURL, "QWEN_API_KEY") // llama-swap: OpenAI-compatible chat + image generation + management // endpoints over a model-swapping proxy. Chat reuses the openai client diff --git a/builtin_kimi_test.go b/builtin_kimi_test.go deleted file mode 100644 index 7140ba0..0000000 --- a/builtin_kimi_test.go +++ /dev/null @@ -1,171 +0,0 @@ -package majordomo - -import ( - "context" - "errors" - "io" - "net/http" - "strings" - "testing" - - "gitea.stevedudenhoeffer.com/steve/majordomo/llm" -) - -// kimiResponse is a minimal valid Chat Completions body so Generate returns a -// non-empty response (an empty one would trigger failover, not a clean pass). -const kimiResponse = `{"id":"c1","object":"chat.completion","choices":[` + - `{"index":0,"message":{"role":"assistant","content":"ok"},"finish_reason":"stop"}]}` - -// captureRT records the last request and returns a canned response without -// touching the network, so these tests stay hermetic while still exercising -// the real openai client the kimi built-in reuses (base URL + auth header). -type captureRT struct { - req *http.Request - body string -} - -func (c *captureRT) RoundTrip(r *http.Request) (*http.Response, error) { - c.req = r - return &http.Response{ - StatusCode: http.StatusOK, - Body: io.NopCloser(strings.NewReader(c.body)), - Header: make(http.Header), - Request: r, - }, nil -} - -// TestKimiBuiltin: the built-in "kimi" provider resolves in Parse, targets -// Moonshot's default endpoint, and authenticates with KIMI_API_KEY. -func TestKimiBuiltin(t *testing.T) { - rt := &captureRT{body: kimiResponse} - r := newTestRegistry(t, - WithEnvLookup(func(k string) string { - if k == "KIMI_API_KEY" { - return "kimi-secret" - } - return "" - }), - WithHTTPClient(&http.Client{Transport: rt}), - ) - - if p, ok := r.Provider(ProviderKimi); !ok { - t.Fatal("built-in kimi provider not registered") - } else if p.Name() != ProviderKimi { - t.Errorf("name = %q, want %q", p.Name(), ProviderKimi) - } - - m, err := r.Parse("kimi/kimi-k2-0711-preview") - if err != nil { - t.Fatalf("Parse: %v", err) - } - if got := targetsOf(t, m); len(got) != 1 || got[0] != "kimi/kimi-k2-0711-preview" { - t.Fatalf("targets = %v", got) - } - - if _, err := m.Generate(context.Background(), llm.Request{Messages: []llm.Message{llm.UserText("hi")}}); err != nil { - t.Fatalf("Generate: %v", err) - } - if rt.req == nil { - t.Fatal("no request captured") - } - if want := "https://api.moonshot.ai/v1/chat/completions"; rt.req.URL.String() != want { - t.Errorf("URL = %q, want %q", rt.req.URL.String(), want) - } - if want := "Bearer kimi-secret"; rt.req.Header.Get("Authorization") != want { - t.Errorf("Authorization = %q, want %q", rt.req.Header.Get("Authorization"), want) - } -} - -// TestKimiBuiltinMissingKey: with no KIMI_API_KEY the built-in fails fast with a -// synthetic 401 whose hint names KIMI_API_KEY — never OPENAI_API_KEY (proving -// the credential does not fall through to the openai client's default), and -// without hitting the network. -func TestKimiBuiltinMissingKey(t *testing.T) { - rt := &captureRT{body: kimiResponse} - r := newTestRegistry(t, WithHTTPClient(&http.Client{Transport: rt})) - - m, err := r.Parse("kimi/kimi-k2-0711-preview") - if err != nil { - t.Fatalf("Parse: %v", err) - } - _, err = m.Generate(context.Background(), llm.Request{Messages: []llm.Message{llm.UserText("hi")}}) - apiErr, ok := errors.AsType[*llm.APIError](err) - if !ok { - t.Fatalf("err = %v (%T), want *llm.APIError", err, err) - } - if apiErr.Status != http.StatusUnauthorized || apiErr.Code != "missing_api_key" { - t.Errorf("Status/Code = %d/%q, want 401/missing_api_key", apiErr.Status, apiErr.Code) - } - if !strings.Contains(apiErr.Message, "KIMI_API_KEY") { - t.Errorf("message = %q, want it to name KIMI_API_KEY", apiErr.Message) - } - if strings.Contains(apiErr.Message, "OPENAI_API_KEY") { - t.Errorf("message = %q, must not name OPENAI_API_KEY", apiErr.Message) - } - if rt.req != nil { - t.Error("network was hit despite missing key") - } -} - -// TestKimiScheme: a kimi:// LLM_* DSN defines a named provider on any Moonshot -// host (here the China endpoint) that is first-class in Parse and carries the -// DSN token as its bearer credential. -func TestKimiScheme(t *testing.T) { - rt := &captureRT{body: kimiResponse} - r := newTestRegistry(t, WithHTTPClient(&http.Client{Transport: rt})) - if err := r.LoadEnv(map[string]string{ - "LLM_KCN": "kimi://tok@api.moonshot.cn/v1", - }); err != nil { - t.Fatalf("LoadEnv: %v", err) - } - - m, err := r.Parse("kcn/moonshot-v1-8k") - if err != nil { - t.Fatalf("Parse: %v", err) - } - if _, err := m.Generate(context.Background(), llm.Request{Messages: []llm.Message{llm.UserText("hi")}}); err != nil { - t.Fatalf("Generate: %v", err) - } - if rt.req == nil { - t.Fatal("no request captured") - } - if want := "https://api.moonshot.cn/v1/chat/completions"; rt.req.URL.String() != want { - t.Errorf("URL = %q, want %q", rt.req.URL.String(), want) - } - if want := "Bearer tok"; rt.req.Header.Get("Authorization") != want { - t.Errorf("Authorization = %q, want %q", rt.req.Header.Get("Authorization"), want) - } -} - -// TestKimiSchemeMissingToken: a kimi:// DSN with no token is fixed by adding one -// to the DSN, not by setting KIMI_API_KEY — so the missing-key hint names the -// defining LLM_ env var, never KIMI_API_KEY (which does nothing for a -// DSN-defined provider). -func TestKimiSchemeMissingToken(t *testing.T) { - rt := &captureRT{body: kimiResponse} - r := newTestRegistry(t, WithHTTPClient(&http.Client{Transport: rt})) - if err := r.LoadEnv(map[string]string{ - "LLM_KCN": "kimi://api.moonshot.cn/v1", // no token - }); err != nil { - t.Fatalf("LoadEnv: %v", err) - } - - m, err := r.Parse("kcn/moonshot-v1-8k") - if err != nil { - t.Fatalf("Parse: %v", err) - } - _, err = m.Generate(context.Background(), llm.Request{Messages: []llm.Message{llm.UserText("hi")}}) - apiErr, ok := errors.AsType[*llm.APIError](err) - if !ok { - t.Fatalf("err = %v (%T), want *llm.APIError", err, err) - } - if !strings.Contains(apiErr.Message, "LLM_KCN") { - t.Errorf("message = %q, want it to name LLM_KCN", apiErr.Message) - } - if strings.Contains(apiErr.Message, "KIMI_API_KEY") { - t.Errorf("message = %q, must not name KIMI_API_KEY for a DSN provider", apiErr.Message) - } - if rt.req != nil { - t.Error("network was hit despite missing token") - } -} diff --git a/builtin_openaicompat_test.go b/builtin_openaicompat_test.go new file mode 100644 index 0000000..1290e2e --- /dev/null +++ b/builtin_openaicompat_test.go @@ -0,0 +1,240 @@ +package majordomo + +import ( + "context" + "errors" + "io" + "net/http" + "strings" + "testing" + + "gitea.stevedudenhoeffer.com/steve/majordomo/llm" +) + +// Shared fixtures and the shared contract for the built-ins that are "the +// openai client pointed somewhere else" (kimi, qwen, ...). They live here +// rather than in any one provider's test file so a new OpenAI-compat built-in +// has nothing to copy — the same reason registerOpenAICompatBuiltin exists on +// the production side. + +// chatCompletionOK is a minimal valid Chat Completions body, so Generate +// returns a non-empty response (an empty one would trigger failover, not a +// clean pass). +const chatCompletionOK = `{"id":"c1","object":"chat.completion","choices":[` + + `{"index":0,"message":{"role":"assistant","content":"ok"},"finish_reason":"stop"}]}` + +// captureRT records the last request (and the bytes of its body) and returns a +// canned response without touching the network, so these tests stay hermetic +// while still exercising the real openai client the built-ins reuse: base URL, +// auth header, and the JSON actually put on the wire. +type captureRT struct { + req *http.Request + reqBody []byte + body string +} + +func (c *captureRT) RoundTrip(r *http.Request) (*http.Response, error) { + c.req = r + // Drain and close the request body: a RoundTripper owns it, and those + // bytes are what wire-shape assertions read. + c.reqBody = nil + if r.Body != nil { + c.reqBody, _ = io.ReadAll(r.Body) + _ = r.Body.Close() + } + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(strings.NewReader(c.body)), + Header: make(http.Header), + Request: r, + }, nil +} + +// singleKeyEnv builds a WithEnvLookup function that knows exactly one variable +// and returns "" for everything else. The empty default has teeth: a built-in +// that reached for any other variable name gets nothing, so the request 401s +// and the test fails rather than quietly authenticating off the wrong key. +func singleKeyEnv(key, value string) func(string) string { + return func(k string) string { + if k == key { + return value + } + return "" + } +} + +// openAICompatBuiltin describes one built-in for the shared contract below. +// Adding an OpenAI-compat built-in means adding a row here — not copying a +// test file, which is how kimi's and qwen's suites became near-identical. +type openAICompatBuiltin struct { + name string // registry name and spec prefix + keyEnv string // the credential variable this built-in reads + model string // a current model id for that endpoint + wantURL string // chat-completions URL the default endpoint must produce + + // The name:// DSN case: an alternate host (regional/China endpoint) + // reached through an LLM_ definition. + dsnVar string + dsnHost string + wantDSNURL string +} + +var openAICompatBuiltins = []openAICompatBuiltin{ + { + name: ProviderKimi, + keyEnv: "KIMI_API_KEY", + model: "kimi-k2-0711-preview", + wantURL: "https://api.moonshot.ai/v1/chat/completions", + dsnVar: "LLM_KCN", + dsnHost: "api.moonshot.cn/v1", + wantDSNURL: "https://api.moonshot.cn/v1/chat/completions", + }, + { + name: ProviderQwen, + keyEnv: "QWEN_API_KEY", + model: "qwen3.8-max", + wantURL: "https://dashscope-intl.aliyuncs.com/compatible-mode/v1/chat/completions", + dsnVar: "LLM_QCN", + dsnHost: "dashscope.aliyuncs.com/compatible-mode/v1", + wantDSNURL: "https://dashscope.aliyuncs.com/compatible-mode/v1/chat/completions", + }, +} + +// TestOpenAICompatBuiltins is the whole contract an OpenAI-compat built-in +// owes, asserted identically for every one of them: it resolves in Parse and +// targets its own endpoint with its own key; a missing key fails closed naming +// the right variable and never reaching the network; its name:// DSN reaches +// any other host on the DSN token; and a keyless DSN names the LLM_ that +// actually fixes it rather than the built-in's variable, which does nothing +// for a DSN-defined provider. +func TestOpenAICompatBuiltins(t *testing.T) { + for _, tc := range openAICompatBuiltins { + t.Run(tc.name+"/builtin", func(t *testing.T) { + rt := &captureRT{body: chatCompletionOK} + secret := tc.name + "-secret" + r := newTestRegistry(t, + WithEnvLookup(singleKeyEnv(tc.keyEnv, secret)), + WithHTTPClient(&http.Client{Transport: rt}), + ) + + if p, ok := r.Provider(tc.name); !ok { + t.Fatalf("built-in %q not registered", tc.name) + } else if p.Name() != tc.name { + t.Errorf("name = %q, want %q", p.Name(), tc.name) + } + + spec := tc.name + "/" + tc.model + m, err := r.Parse(spec) + if err != nil { + t.Fatalf("Parse(%q): %v", spec, err) + } + if got := targetsOf(t, m); len(got) != 1 || got[0] != spec { + t.Fatalf("targets = %v, want [%q]", got, spec) + } + + if _, err := m.Generate(context.Background(), llm.Request{Messages: []llm.Message{llm.UserText("hi")}}); err != nil { + t.Fatalf("Generate: %v", err) + } + if rt.req == nil { + t.Fatal("no request captured") + } + if rt.req.URL.String() != tc.wantURL { + t.Errorf("URL = %q, want %q", rt.req.URL.String(), tc.wantURL) + } + if want := "Bearer " + secret; rt.req.Header.Get("Authorization") != want { + t.Errorf("Authorization = %q, want %q", rt.req.Header.Get("Authorization"), want) + } + }) + + t.Run(tc.name+"/builtin missing key", func(t *testing.T) { + rt := &captureRT{body: chatCompletionOK} + r := newTestRegistry(t, WithHTTPClient(&http.Client{Transport: rt})) + + m, err := r.Parse(tc.name + "/" + tc.model) + if err != nil { + t.Fatalf("Parse: %v", err) + } + _, err = m.Generate(context.Background(), llm.Request{Messages: []llm.Message{llm.UserText("hi")}}) + apiErr, ok := errors.AsType[*llm.APIError](err) + if !ok { + t.Fatalf("err = %v (%T), want *llm.APIError", err, err) + } + if apiErr.Status != http.StatusUnauthorized || apiErr.Code != "missing_api_key" { + t.Errorf("Status/Code = %d/%q, want 401/missing_api_key", apiErr.Status, apiErr.Code) + } + if !strings.Contains(apiErr.Message, tc.keyEnv) { + t.Errorf("message = %q, want it to name %s", apiErr.Message, tc.keyEnv) + } + // The load-bearing half: openai.New defaults its key to + // OPENAI_API_KEY, so a built-in that stopped passing WithAPIKey + // unconditionally would authenticate as OpenAI instead of failing. + if strings.Contains(apiErr.Message, "OPENAI_API_KEY") { + t.Errorf("message = %q, must not name OPENAI_API_KEY", apiErr.Message) + } + if rt.req != nil { + t.Error("network was hit despite missing key") + } + }) + + t.Run(tc.name+"/dsn scheme", func(t *testing.T) { + rt := &captureRT{body: chatCompletionOK} + r := newTestRegistry(t, WithHTTPClient(&http.Client{Transport: rt})) + if err := r.LoadEnv(map[string]string{ + tc.dsnVar: tc.name + "://tok@" + tc.dsnHost, + }); err != nil { + t.Fatalf("LoadEnv: %v", err) + } + + dsnName := strings.ToLower(strings.TrimPrefix(tc.dsnVar, "LLM_")) + m, err := r.Parse(dsnName + "/" + tc.model) + if err != nil { + t.Fatalf("Parse: %v", err) + } + if _, err := m.Generate(context.Background(), llm.Request{Messages: []llm.Message{llm.UserText("hi")}}); err != nil { + t.Fatalf("Generate: %v", err) + } + if rt.req == nil { + t.Fatal("no request captured") + } + if rt.req.URL.String() != tc.wantDSNURL { + t.Errorf("URL = %q, want %q", rt.req.URL.String(), tc.wantDSNURL) + } + if want := "Bearer tok"; rt.req.Header.Get("Authorization") != want { + t.Errorf("Authorization = %q, want %q", rt.req.Header.Get("Authorization"), want) + } + }) + + t.Run(tc.name+"/dsn scheme missing token", func(t *testing.T) { + rt := &captureRT{body: chatCompletionOK} + r := newTestRegistry(t, WithHTTPClient(&http.Client{Transport: rt})) + if err := r.LoadEnv(map[string]string{ + tc.dsnVar: tc.name + "://" + tc.dsnHost, // no token + }); err != nil { + t.Fatalf("LoadEnv: %v", err) + } + + dsnName := strings.ToLower(strings.TrimPrefix(tc.dsnVar, "LLM_")) + m, err := r.Parse(dsnName + "/" + tc.model) + if err != nil { + t.Fatalf("Parse: %v", err) + } + _, err = m.Generate(context.Background(), llm.Request{Messages: []llm.Message{llm.UserText("hi")}}) + apiErr, ok := errors.AsType[*llm.APIError](err) + if !ok { + t.Fatalf("err = %v (%T), want *llm.APIError", err, err) + } + // A keyless DSN is fixed by adding a token to that DSN, so the + // hint must name the defining variable — never the built-in's own + // key, which does nothing for a DSN-defined provider. + if !strings.Contains(apiErr.Message, tc.dsnVar) { + t.Errorf("message = %q, want it to name %s", apiErr.Message, tc.dsnVar) + } + if strings.Contains(apiErr.Message, tc.keyEnv) { + t.Errorf("message = %q, must not name %s for a DSN provider", apiErr.Message, tc.keyEnv) + } + if rt.req != nil { + t.Error("network was hit despite missing token") + } + }) + } +} diff --git a/builtin_qwen_test.go b/builtin_qwen_test.go new file mode 100644 index 0000000..0419e30 --- /dev/null +++ b/builtin_qwen_test.go @@ -0,0 +1,86 @@ +package majordomo + +import ( + "context" + "encoding/json" + "net/http" + "testing" + + "gitea.stevedudenhoeffer.com/steve/majordomo/llm" +) + +// The contract qwen shares with every other OpenAI-compat built-in (endpoint, +// credential isolation, its qwen:// DSN) is asserted by the table in +// builtin_openaicompat_test.go. What remains here is qwen-specific: the +// reverse-leak direction, and the wire claim ADR-0027 turns on. + +// TestQwenBuiltinKeyDoesNotLeakToOpenAI: QWEN_API_KEY is the qwen built-in's +// credential and nothing else's. Why this direction too: the shared table's +// missing-key case only proves qwen never borrows OPENAI_API_KEY; this proves +// the reverse — a registry that can see QWEN_API_KEY must not hand it to the +// openai built-in, which would send an Alibaba key to api.openai.com. +func TestQwenBuiltinKeyDoesNotLeakToOpenAI(t *testing.T) { + // Set before newTestRegistry: the openai built-in reads OPENAI_API_KEY at + // construction. Giving it a real key is what keeps this test honest — a + // keyless openai target would 401 before any request, and the assertion + // below would pass without a single byte reaching the wire. + t.Setenv("OPENAI_API_KEY", "openai-secret") + + rt := &captureRT{body: chatCompletionOK} + r := newTestRegistry(t, + WithEnvLookup(singleKeyEnv("QWEN_API_KEY", "qwen-secret")), + WithHTTPClient(&http.Client{Transport: rt}), + ) + + m, err := r.Parse("openai/gpt-4o-mini") + if err != nil { + t.Fatalf("Parse: %v", err) + } + if _, err := m.Generate(context.Background(), llm.Request{Messages: []llm.Message{llm.UserText("hi")}}); err != nil { + t.Fatalf("Generate: %v", err) + } + if rt.req == nil { + t.Fatal("no request captured") + } + if want := "Bearer openai-secret"; rt.req.Header.Get("Authorization") != want { + t.Errorf("Authorization = %q, want %q — the qwen credential must not reach the openai built-in", + rt.req.Header.Get("Authorization"), want) + } +} + +// TestQwenReasoningEffortReachesWire is the load-bearing test for ADR-0027's +// central claim: Model Studio's OpenAI-compatible surface takes reasoning as a +// top-level "reasoning_effort" body field, which the openai client already +// sends — so llm.WithReasoningEffort survives the trip on qwen with no +// qwen-specific code. Routing qwen through the anthropic client instead would +// drop it silently (provider/anthropic ignores ReasoningEffort by design), and +// that difference would be invisible without asserting on the wire body. +func TestQwenReasoningEffortReachesWire(t *testing.T) { + rt := &captureRT{body: chatCompletionOK} + r := newTestRegistry(t, + WithEnvLookup(singleKeyEnv("QWEN_API_KEY", "qwen-secret")), + WithHTTPClient(&http.Client{Transport: rt}), + ) + + m, err := r.Parse("qwen/qwen3.8-max") + if err != nil { + t.Fatalf("Parse: %v", err) + } + _, err = m.Generate(context.Background(), llm.Request{ + Messages: []llm.Message{llm.UserText("hi")}, + ReasoningEffort: "high", + }) + if err != nil { + t.Fatalf("Generate: %v", err) + } + if rt.reqBody == nil { + t.Fatal("no request body captured") + } + var sent map[string]any + if err := json.Unmarshal(rt.reqBody, &sent); err != nil { + t.Fatalf("decode request body: %v", err) + } + if got := sent["reasoning_effort"]; got != "high" { + t.Errorf("reasoning_effort = %v, want %q (body: %s)", got, "high", rt.reqBody) + } +} diff --git a/docs/adr/0027-qwen-builtin.md b/docs/adr/0027-qwen-builtin.md new file mode 100644 index 0000000..3a495d0 --- /dev/null +++ b/docs/adr/0027-qwen-builtin.md @@ -0,0 +1,102 @@ +# ADR-0027: Qwen (Alibaba) built-in provider — OpenAI-compat, not Anthropic-compat + +**Status:** Accepted — 2026-08-12 + +## Context + +Alibaba's Qwen models (`qwen3.8-max`, `qwen3.7-plus`, the `qwen3-vl-*` vision +variants, …) are served from Model Studio / DashScope, and mort wants them as a +first-class failover tier with a dedicated `QWEN_API_KEY` — the same ergonomics +ADR-0026 gave Kimi. + +Unlike Kimi, Model Studio exposes the same models over **two** protocols: + +| | OpenAI-compatible | Anthropic-compatible | +|---|---|---| +| Base URL | `https://dashscope-intl.aliyuncs.com/compatible-mode/v1` | `https://dashscope-intl.aliyuncs.com/apps/anthropic` | +| Endpoints | full Chat Completions surface | `/v1/messages` only (no `/v1/models`) | +| Purpose | the documented developer API | a shim, documented around hosting Claude Code | + +So the question this ADR answers is not "which client do we reuse" but +"which of Alibaba's two wire protocols does the built-in speak". + +## Decision + +**The `qwen` built-in and the `qwen://` DSN scheme speak OpenAI-compat**, over +`provider/openai` — no new package, mirroring ADR-0026 (kimi) and ADR-0015 +(llama-swap chat). Default base URL is the international host; the China host +(`dashscope.aliyuncs.com/compatible-mode/v1`) and workspace-scoped regional +hosts are reachable with a `qwen://` DSN. + +Credential handling is copied from kimi verbatim, because both of its rules +are load-bearing: `WithAPIKey` is passed unconditionally (even empty) so an +unset `QWEN_API_KEY` can never fall through to `openai.New`'s `OPENAI_API_KEY` +default, and `WithAPIKeyName("QWEN_API_KEY")` makes the synthetic-401 hint name +the variable the operator actually has to set. + +The kimi and qwen DSN factories were identical, so they now share one +`openaiCompatScheme` helper — the next OpenAI-compat built-in gets the +credential and key-hint rules by construction rather than by copy. + +### Why not the Anthropic-compatible endpoint + +Every concrete difference favors OpenAI-compat *for this codebase*: + +- **Reasoning survives the trip.** Model Studio takes `reasoning_effort` as a + top-level field on the OpenAI surface, which `provider/openai` already sends + — `llm.WithReasoningEffort` works on qwen with zero qwen-specific code + (`TestQwenReasoningEffortReachesWire` asserts it on the wire). Down the + anthropic client it would be dropped in silence: `provider/anthropic` + deliberately ignores `Request.ReasoningEffort`, because first-party Claude + has no such knob. +- **Structured output would regress.** `provider/anthropic` implements + `Request.Schema` with the first-party GA `output_config.format` mechanism. + Alibaba's shim does not implement it; a compat endpoint that ignores an + unknown field returns unconstrained prose while still reporting success. + The OpenAI path sends `response_format: json_schema`, which Model Studio + supports natively on the Max/Plus families. +- **Cache accounting already lands.** Model Studio's implicit prefix cache + reports hits in `usage.prompt_tokens_details.cached_tokens`, which the openai + client already maps to `llm.Usage.CacheReadTokens`. The anthropic client + reads `cache_read_input_tokens`, a field the shim has no reason to emit. +- **Thinking content is discarded on the anthropic path anyway.** + `provider/anthropic` skips `thinking` blocks in both the buffered and + streaming decoders, so the shim's headline feature — first-class + `thinking: {type: "enabled", budget_tokens: N}` — buys majordomo nothing + today. +- **Smaller blast radius.** The anthropic client has no `WithAPIKeyName` + option, so a keyless qwen would tell the operator to set `ANTHROPIC_API_KEY`; + fixing that means changing the first-party Anthropic client to serve a + third-party shim. +- **It is the less-exercised surface.** The Anthropic endpoint is documented as + Messages-only, with a temperature range that differs from Anthropic's own + ([0, 2) vs [0.0, 1.0]) — i.e. it is Qwen semantics wearing an Anthropic + envelope, not an Anthropic-equivalent target. + +The one thing the Anthropic surface offers that OpenAI-compat does not is +explicit `cache_control` breakpoints reached through `Request.PromptCache`. +That is not a reason to route Qwen through it: Model Studio's implicit cache is +automatic and already metered, and if explicit breakpoints ever matter they +belong in `provider/openai` (Model Studio accepts `cache_control` on content +blocks there too), where every OpenAI-compat target would get them. + +## Consequences + +- `qwen/` is first-class in Parse, chains, aliases, and health/failover + with no consumer wiring; model ids pass through verbatim (no catalog). +- Chat, streaming, tools, structured output, reasoning effort, and cached-token + accounting all ride the openai client and inherit its fixes. +- Image *inputs* work at the client level, but only the `qwen-vl-*` / + `qwen3-vl-*` models accept them (matrix footnote ⁴; ³ is kimi's). +- Two model-side quirks are Alibaba's, not majordomo's, and are left to the + caller rather than papered over: thinking is **on by default** on some models + (e.g. `qwen3.7-plus`), and Qwen3 *open-source* models require streaming when + thinking is enabled — a buffered `Generate` against one of those needs a + model that supports non-streaming thinking (the Max/Plus families do). +- If a future consumer genuinely needs the Anthropic surface, it is reachable + today without library changes: + `LLM_QWEN_ANTHROPIC=anthropic://token@dashscope-intl.aliyuncs.com/apps/anthropic` + — with the reasoning/structured-output caveats above. +- Second third-party built-in after kimi. The ADR-0026 bar still holds: a named + consumer needs it in-config. `RegisterProvider`/`LLM_*` remain the path for + everything else. diff --git a/docs/adr/README.md b/docs/adr/README.md index b75b582..5965921 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -30,3 +30,4 @@ One decision per file, append-only; supersede rather than rewrite. | [0024](0024-audio-wave3-surfaces.md) | Wave-3 audio surfaces (stems, SFX, speech enhance, voice clone, translate) | Accepted | | [0025](0025-videogen-wave3-surfaces.md) | Wave-3 video surfaces (lipsync, video matte, video upscale, chain jobs) | Accepted | | [0026](0026-kimi-builtin.md) | Kimi (Moonshot AI) built-in provider — reuse openai client, KIMI_API_KEY | Accepted | +| [0027](0027-qwen-builtin.md) | Qwen (Alibaba) built-in provider — OpenAI-compat, not Model Studio's Anthropic-compat endpoint | Accepted | diff --git a/env.go b/env.go index bf7d9a8..20c274b 100644 --- a/env.go +++ b/env.go @@ -26,8 +26,9 @@ var ErrUnknownProvider = errors.New("unknown provider") // authenticated with the bearer token "test-token". type DSN struct { // Scheme selects the provider implementation: "foreman", "ollama", - // "ollama-cloud", "openai", "kimi", "anthropic", "google"/"gemini", or - // any custom scheme registered with RegisterScheme. + // "ollama-cloud", "openai", "kimi", "qwen", "anthropic", + // "google"/"gemini", "llama-swap"/"llama-swaps", or any custom scheme + // registered with RegisterScheme. Scheme string // Token is the provider secret (bearer token or API key); empty = none. Token string @@ -40,6 +41,19 @@ type DSN struct { // env-defined providers always speak TLS). func (d DSN) BaseURL() string { return "https://" + d.Host } +// envKeyForProvider returns the LLM_* variable that defines the provider named +// name: "m1" → LLM_M1, "my-prov" → LLM_MY_PROV. +// +// This is the single definition on purpose. Two call sites need byte-identical +// output and would drift apart in silence: lazy resolution reads this variable +// to find an unregistered provider, and openaiCompatScheme names it in the +// missing-key hint so a keyless DSN target tells the operator which variable to +// set. Those two were separate copies with a comment asserting they matched — +// a comment is not enforcement, this function is. +func envKeyForProvider(name string) string { + return "LLM_" + strings.ToUpper(strings.ReplaceAll(name, "-", "_")) +} + // ParseDSN parses a raw DSN string. The algorithm matches go-llm exactly: // split on "://", then an optional "@" separates the token from the host; // trailing slashes on the host are trimmed. diff --git a/progress.md b/progress.md index 300080d..e69f874 100644 --- a/progress.md +++ b/progress.md @@ -285,3 +285,39 @@ tests flush out. (footnote ³), `.env.example`, ADR-0026 (+ index; also backfilled the missing 0024/0025 index rows). - Consumer: mort names Kimi as a failover tier. + +## 2026-08-12 — Qwen (Alibaba) built-in provider (ADR-0027) + +- New built-in `qwen` provider + `qwen://` DSN scheme over Alibaba Model + Studio's OpenAI-compatible mode, reusing `provider/openai` (no new client, + mirrors kimi/llama-swap). Default base URL + `https://dashscope-intl.aliyuncs.com/compatible-mode/v1`; China/regional + hosts via `LLM_QCN=qwen://token@dashscope.aliyuncs.com/compatible-mode/v1`. +- Credential is `QWEN_API_KEY` (via the registry's injected envLookup). + `WithAPIKey` passed unconditionally so an unset key cannot fall through to + `OPENAI_API_KEY`; `WithAPIKeyName` names `QWEN_API_KEY` in the 401 hint. +- **Chose OpenAI-compat over Model Studio's Anthropic-compatible + `/apps/anthropic` shim** (ADR-0027): on the anthropic client + `ReasoningEffort` is ignored by design, `Request.Schema` rides + `output_config.format` (which the shim does not implement), and cached-token + accounting reads Anthropic-only usage fields. The shim is still reachable + ad hoc via an `anthropic://` DSN. +- `registerOpenAICompatBuiltin` installs BOTH halves of an OpenAI-compat + built-in (eager provider + `name://` DSN scheme via the shared + `openaiCompatScheme`), so the two credential rules — unconditional + `WithAPIKey`, and `WithAPIKeyName` naming that same variable — hold by + construction. kimi and qwen are one line each. +- `envKeyForProvider` is the single definition of the `LLM_` form, + shared by lazy resolution (`registry.go`) and the DSN missing-key hint. They + were separate copies with a comment asserting they matched. +- The shared contract is ONE table (`builtin_openaicompat_test.go`), run + identically for every OpenAI-compat built-in: endpoint + bearer, missing key + fails closed naming its own variable with no network hit, the `name://` DSN + reaching another host, and a keyless DSN naming `LLM_` rather than the + built-in's key. Adding a built-in is a table row that immediately owes all + four; `builtin_kimi_test.go` was retired into it. Qwen-only tests: the + reverse credential leak, and `reasoning_effort` asserted on the wire body + (the ADR's load-bearing claim). +- Docs in sync: README built-in table + Qwen paragraph + DSN scheme list + + support matrix (footnote ⁴), `.env.example`, ADR-0027 (+ index). +- Consumer: mort wants Qwen as a failover tier. diff --git a/registry.go b/registry.go index e657e2a..f5bd162 100644 --- a/registry.go +++ b/registry.go @@ -263,7 +263,7 @@ func (r *Registry) providerFor(name string) (llm.Provider, error) { return nil, envErr } - envKey := "LLM_" + strings.ToUpper(strings.ReplaceAll(name, "-", "_")) + envKey := envKeyForProvider(name) envVal := r.envLookup(envKey) if envVal == "" { return nil, fmt.Errorf("%w: %q (checked registry and %s env var)", ErrUnknownProvider, name, envKey)