steveandClaude Opus 5
8670ed22be
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]>
87 lines
3.2 KiB
Go
87 lines
3.2 KiB
Go
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)
|
|
}
|
|
}
|