refactor: gadfly round 3 — one table owns the OpenAI-compat contract
Three findings, and the first two are the same recurring shape. envKeyForProvider (env.go) is now the single definition of the LLM_<NAME> form. It lived in two places — lazy resolution in registry.go and the missing-key hint in openaiCompatScheme — with a comment on the second asserting it matched the first. A comment is not enforcement: if either had drifted, a keyless DSN target would have named a variable that does nothing, and nothing would have failed. The kimi and qwen test files had become near-identical, which is round 1's finding at the level above it: I deduped the fixtures, then left two parallel suites asserting the same four things. They are now ONE table in builtin_openaicompat_test.go — endpoint + credential, missing key fails closed naming its own variable and never reaching the network, the name:// DSN reaching another host, and a keyless DSN naming LLM_<NAME> instead of the built-in's key. Adding an OpenAI-compat built-in is a table row that immediately owes all four; builtin_kimi_test.go is deleted because the table covers it. Only genuinely qwen-specific tests remain in the qwen file: the reverse credential leak and the reasoning_effort wire claim ADR-0027 rests on. Also trimmed ProviderQwen's doc comment, which restated the ADR-0027 rationale already given at the registration site. The break-check suite caught its own rot again — two mutations went stale when these tests were renamed, and the landed-check reported them loudly instead of passing them off as green. Now 9 cases, including one that drifts envKeyForProvider to prove the shared helper is load-bearing. 9/9 apply and are caught. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This commit is contained in:
+3
-7
@@ -2,7 +2,6 @@ package majordomo
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"strings"
|
||||
|
||||
"gitea.stevedudenhoeffer.com/steve/majordomo/llm"
|
||||
"gitea.stevedudenhoeffer.com/steve/majordomo/provider/anthropic"
|
||||
@@ -22,11 +21,8 @@ const (
|
||||
// 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. Why OpenAI-compat and not the Anthropic-compat endpoint
|
||||
// Model Studio also exposes: see ADR-0027 — the OpenAI surface is the
|
||||
// first-class one there (reasoning_effort, json_schema structured output,
|
||||
// cached_tokens accounting all ride it), while the Anthropic shim exists
|
||||
// mainly to host Claude Code.
|
||||
// qwenBaseURL. ADR-0027 records why the OpenAI surface and not the
|
||||
// Anthropic-compatible one Model Studio also exposes.
|
||||
ProviderQwen = "qwen"
|
||||
ProviderAnthropic = "anthropic"
|
||||
ProviderGoogle = "google"
|
||||
@@ -67,7 +63,7 @@ func openaiCompatScheme(wrap func(...openai.Option) []openai.Option) SchemeFacto
|
||||
openai.WithName(name),
|
||||
openai.WithBaseURL(dsn.BaseURL()),
|
||||
openai.WithAPIKey(dsn.Token),
|
||||
openai.WithAPIKeyName("LLM_"+strings.ToUpper(strings.ReplaceAll(name, "-", "_"))),
|
||||
openai.WithAPIKeyName(envKeyForProvider(name)),
|
||||
)...), nil
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,142 +0,0 @@
|
||||
package majordomo
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"net/http"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"gitea.stevedudenhoeffer.com/steve/majordomo/llm"
|
||||
)
|
||||
|
||||
// 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: chatCompletionOK}
|
||||
r := newTestRegistry(t,
|
||||
WithEnvLookup(singleKeyEnv("KIMI_API_KEY", "kimi-secret")),
|
||||
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: chatCompletionOK}
|
||||
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: chatCompletionOK}
|
||||
r := newTestRegistry(t, WithHTTPClient(&http.Client{Transport: rt}))
|
||||
if err := r.LoadEnv(map[string]string{
|
||||
"LLM_KCN": "kimi://[email protected]/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_<NAME> env var, never KIMI_API_KEY (which does nothing for a
|
||||
// DSN-defined provider).
|
||||
func TestKimiSchemeMissingToken(t *testing.T) {
|
||||
rt := &captureRT{body: chatCompletionOK}
|
||||
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")
|
||||
}
|
||||
}
|
||||
@@ -1,15 +1,21 @@
|
||||
package majordomo
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"io"
|
||||
"net/http"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"gitea.stevedudenhoeffer.com/steve/majordomo/llm"
|
||||
)
|
||||
|
||||
// Shared fixtures 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 openaiCompatScheme exists on the production side.
|
||||
// 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
|
||||
@@ -56,3 +62,179 @@ func singleKeyEnv(key, value string) func(string) string {
|
||||
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_<dsnVar> 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_<NAME> 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")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
+8
-137
@@ -3,88 +3,22 @@ package majordomo
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"net/http"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"gitea.stevedudenhoeffer.com/steve/majordomo/llm"
|
||||
)
|
||||
|
||||
// TestQwenBuiltin: the built-in "qwen" provider resolves in Parse, targets
|
||||
// Model Studio's international OpenAI-compatible endpoint, and authenticates
|
||||
// with QWEN_API_KEY.
|
||||
func TestQwenBuiltin(t *testing.T) {
|
||||
rt := &captureRT{body: chatCompletionOK}
|
||||
r := newTestRegistry(t,
|
||||
WithEnvLookup(singleKeyEnv("QWEN_API_KEY", "qwen-secret")),
|
||||
WithHTTPClient(&http.Client{Transport: rt}),
|
||||
)
|
||||
|
||||
if p, ok := r.Provider(ProviderQwen); !ok {
|
||||
t.Fatal("built-in qwen provider not registered")
|
||||
} else if p.Name() != ProviderQwen {
|
||||
t.Errorf("name = %q, want %q", p.Name(), ProviderQwen)
|
||||
}
|
||||
|
||||
m, err := r.Parse("qwen/qwen3.8-max")
|
||||
if err != nil {
|
||||
t.Fatalf("Parse: %v", err)
|
||||
}
|
||||
if got := targetsOf(t, m); len(got) != 1 || got[0] != "qwen/qwen3.8-max" {
|
||||
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://dashscope-intl.aliyuncs.com/compatible-mode/v1/chat/completions"; rt.req.URL.String() != want {
|
||||
t.Errorf("URL = %q, want %q", rt.req.URL.String(), want)
|
||||
}
|
||||
if want := "Bearer qwen-secret"; rt.req.Header.Get("Authorization") != want {
|
||||
t.Errorf("Authorization = %q, want %q", rt.req.Header.Get("Authorization"), want)
|
||||
}
|
||||
}
|
||||
|
||||
// TestQwenBuiltinMissingKey: with no QWEN_API_KEY the built-in fails fast with
|
||||
// a synthetic 401 whose hint names QWEN_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 TestQwenBuiltinMissingKey(t *testing.T) {
|
||||
rt := &captureRT{body: chatCompletionOK}
|
||||
r := newTestRegistry(t, 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")}})
|
||||
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, "QWEN_API_KEY") {
|
||||
t.Errorf("message = %q, want it to name QWEN_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")
|
||||
}
|
||||
}
|
||||
// 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 fallthrough guard
|
||||
// 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.
|
||||
// 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
|
||||
@@ -114,69 +48,6 @@ func TestQwenBuiltinKeyDoesNotLeakToOpenAI(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestQwenScheme: a qwen:// LLM_* DSN defines a named provider on any Model
|
||||
// Studio host (here the China endpoint) that is first-class in Parse and
|
||||
// carries the DSN token as its bearer credential.
|
||||
func TestQwenScheme(t *testing.T) {
|
||||
rt := &captureRT{body: chatCompletionOK}
|
||||
r := newTestRegistry(t, WithHTTPClient(&http.Client{Transport: rt}))
|
||||
if err := r.LoadEnv(map[string]string{
|
||||
"LLM_QCN": "qwen://[email protected]/compatible-mode/v1",
|
||||
}); err != nil {
|
||||
t.Fatalf("LoadEnv: %v", err)
|
||||
}
|
||||
|
||||
m, err := r.Parse("qcn/qwen3.7-plus")
|
||||
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://dashscope.aliyuncs.com/compatible-mode/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)
|
||||
}
|
||||
}
|
||||
|
||||
// TestQwenSchemeMissingToken: a qwen:// DSN with no token is fixed by adding
|
||||
// one to the DSN, not by setting QWEN_API_KEY — so the missing-key hint names
|
||||
// the defining LLM_<NAME> env var, never QWEN_API_KEY (which does nothing for a
|
||||
// DSN-defined provider).
|
||||
func TestQwenSchemeMissingToken(t *testing.T) {
|
||||
rt := &captureRT{body: chatCompletionOK}
|
||||
r := newTestRegistry(t, WithHTTPClient(&http.Client{Transport: rt}))
|
||||
if err := r.LoadEnv(map[string]string{
|
||||
"LLM_QCN": "qwen://dashscope.aliyuncs.com/compatible-mode/v1", // no token
|
||||
}); err != nil {
|
||||
t.Fatalf("LoadEnv: %v", err)
|
||||
}
|
||||
|
||||
m, err := r.Parse("qcn/qwen3.7-plus")
|
||||
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_QCN") {
|
||||
t.Errorf("message = %q, want it to name LLM_QCN", apiErr.Message)
|
||||
}
|
||||
if strings.Contains(apiErr.Message, "QWEN_API_KEY") {
|
||||
t.Errorf("message = %q, must not name QWEN_API_KEY for a DSN provider", apiErr.Message)
|
||||
}
|
||||
if rt.req != nil {
|
||||
t.Error("network was hit despite missing token")
|
||||
}
|
||||
}
|
||||
|
||||
// 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
|
||||
|
||||
@@ -40,6 +40,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 (LLM_M1 → "m1", so "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.
|
||||
|
||||
+1
-1
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user