refactor(test): gadfly round 1 — share the OpenAI-compat test fixtures

Both findings were the same one, and both were fair: the PR that retires two
byte-identical DSN factories into openaiCompatScheme then copy-pasted the test
fixtures. qwenResponse was byte-identical to kimiResponse, and the single-key
env-lookup closure appeared three times in the new file (plus a fourth in the
kimi file, which neither reviewer was looking at).

Fixed for the class rather than for qwen: captureRT, the canned Chat
Completions body (now chatCompletionOK), and a new singleKeyEnv helper move to
builtin_openaicompat_test.go, owned by no single provider. The kimi tests adopt
them too, so the next OpenAI-compat built-in has nothing left to copy — the
same argument the production helper makes.

Also aligned the test model ids to the current Model Studio names
(qwen3.8-max / qwen3.7-plus), which the docs already cited. One reviewer called
those ids fictional and named the 2025 ones instead; they shipped 2026-08-03
and 2026-05-21 respectively, so that finding is stale model knowledge, not a
defect — but having tests and prose name the same models removes the smell that
prompted it. A dotted id also now proves it passes through verbatim.

Break-checked again after the refactor: all six mutations still fail their test.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

builtin_kimi_test.go

@@ -3,7 +3,6 @@ package majordomo
 import (
 	"context"
 	"errors"
-	"io"
 	"net/http"
 	"strings"
 	"testing"
@@ -11,49 +10,12 @@ import (
 	"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 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 kimi and qwen 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
-}
-
 // 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}
+	rt := &captureRT{body: chatCompletionOK}
 	r := newTestRegistry(t,
-		WithEnvLookup(func(k string) string {
+		WithEnvLookup(singleKeyEnv("KIMI_API_KEY", "kimi-secret")),
-			if k == "KIMI_API_KEY" {
-				return "kimi-secret"
-			}
-			return ""
-		}),
 		WithHTTPClient(&http.Client{Transport: rt}),
 	)
 
@@ -90,7 +52,7 @@ func TestKimiBuiltin(t *testing.T) {
 // 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}
+	rt := &captureRT{body: chatCompletionOK}
 	r := newTestRegistry(t, WithHTTPClient(&http.Client{Transport: rt}))
 
 	m, err := r.Parse("kimi/kimi-k2-0711-preview")
@@ -120,7 +82,7 @@ func TestKimiBuiltinMissingKey(t *testing.T) {
 // 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}
+	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",
@@ -151,7 +113,7 @@ func TestKimiScheme(t *testing.T) {
 // 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: kimiResponse}
+	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

builtin_openaicompat_test.go

@@ -0,0 +1,58 @@
+package majordomo
+
+import (
+	"io"
+	"net/http"
+	"strings"
+)
+
+// 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.
+
+// 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 ""
+	}
+}

builtin_qwen_test.go

@@ -11,23 +11,13 @@ import (
 	"gitea.stevedudenhoeffer.com/steve/majordomo/llm"
 )
 
-// qwenResponse 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 qwenResponse = `{"id":"c1","object":"chat.completion","choices":[` +
-	`{"index":0,"message":{"role":"assistant","content":"ok"},"finish_reason":"stop"}]}`
-
 // 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: qwenResponse}
+	rt := &captureRT{body: chatCompletionOK}
 	r := newTestRegistry(t,
-		WithEnvLookup(func(k string) string {
+		WithEnvLookup(singleKeyEnv("QWEN_API_KEY", "qwen-secret")),
-			if k == "QWEN_API_KEY" {
-				return "qwen-secret"
-			}
-			return ""
-		}),
 		WithHTTPClient(&http.Client{Transport: rt}),
 	)
 
@@ -37,11 +27,11 @@ func TestQwenBuiltin(t *testing.T) {
 		t.Errorf("name = %q, want %q", p.Name(), ProviderQwen)
 	}
 
-	m, err := r.Parse("qwen/qwen3-max")
+	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-max" {
+	if got := targetsOf(t, m); len(got) != 1 || got[0] != "qwen/qwen3.8-max" {
 		t.Fatalf("targets = %v", got)
 	}
 
@@ -64,10 +54,10 @@ func TestQwenBuiltin(t *testing.T) {
 // 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: qwenResponse}
+	rt := &captureRT{body: chatCompletionOK}
 	r := newTestRegistry(t, WithHTTPClient(&http.Client{Transport: rt}))
 
-	m, err := r.Parse("qwen/qwen3-max")
+	m, err := r.Parse("qwen/qwen3.8-max")
 	if err != nil {
 		t.Fatalf("Parse: %v", err)
 	}
@@ -102,14 +92,9 @@ func TestQwenBuiltinKeyDoesNotLeakToOpenAI(t *testing.T) {
 	// below would pass without a single byte reaching the wire.
 	t.Setenv("OPENAI_API_KEY", "openai-secret")
 
-	rt := &captureRT{body: qwenResponse}
+	rt := &captureRT{body: chatCompletionOK}
 	r := newTestRegistry(t,
-		WithEnvLookup(func(k string) string {
+		WithEnvLookup(singleKeyEnv("QWEN_API_KEY", "qwen-secret")),
-			if k == "QWEN_API_KEY" {
-				return "qwen-secret"
-			}
-			return ""
-		}),
 		WithHTTPClient(&http.Client{Transport: rt}),
 	)
 
@@ -133,7 +118,7 @@ func TestQwenBuiltinKeyDoesNotLeakToOpenAI(t *testing.T) {
 // 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: qwenResponse}
+	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",
@@ -141,7 +126,7 @@ func TestQwenScheme(t *testing.T) {
 		t.Fatalf("LoadEnv: %v", err)
 	}
 
-	m, err := r.Parse("qcn/qwen-plus")
+	m, err := r.Parse("qcn/qwen3.7-plus")
 	if err != nil {
 		t.Fatalf("Parse: %v", err)
 	}
@@ -164,7 +149,7 @@ func TestQwenScheme(t *testing.T) {
 // 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: qwenResponse}
+	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
@@ -172,7 +157,7 @@ func TestQwenSchemeMissingToken(t *testing.T) {
 		t.Fatalf("LoadEnv: %v", err)
 	}
 
-	m, err := r.Parse("qcn/qwen-plus")
+	m, err := r.Parse("qcn/qwen3.7-plus")
 	if err != nil {
 		t.Fatalf("Parse: %v", err)
 	}
@@ -200,18 +185,13 @@ func TestQwenSchemeMissingToken(t *testing.T) {
 // 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: qwenResponse}
+	rt := &captureRT{body: chatCompletionOK}
 	r := newTestRegistry(t,
-		WithEnvLookup(func(k string) string {
+		WithEnvLookup(singleKeyEnv("QWEN_API_KEY", "qwen-secret")),
-			if k == "QWEN_API_KEY" {
-				return "qwen-secret"
-			}
-			return ""
-		}),
 		WithHTTPClient(&http.Client{Transport: rt}),
 	)
 
-	m, err := r.Parse("qwen/qwen3-max")
+	m, err := r.Parse("qwen/qwen3.8-max")
 	if err != nil {
 		t.Fatalf("Parse: %v", err)
 	}