diff --git a/.gitea/workflows/build-image.yml b/.gitea/workflows/build-image.yml index fb8ddd9..d88e8e5 100644 --- a/.gitea/workflows/build-image.yml +++ b/.gitea/workflows/build-image.yml @@ -55,6 +55,12 @@ jobs: timeout-minutes: 15 steps: - uses: actions/checkout@v4 + with: + # Scrubbing the registry credential while leaving the checkout token + # in .git/config would just move the prize: `go test` below runs + # repository code with the workspace readable. Nothing in this job + # talks to git after checkout, so the token has no reason to persist. + persist-credentials: false - uses: actions/setup-go@v5 with: go-version-file: go.mod diff --git a/cmd/gadfly/model.go b/cmd/gadfly/model.go index 0f8cce4..1c5b14a 100644 --- a/cmd/gadfly/model.go +++ b/cmd/gadfly/model.go @@ -107,8 +107,22 @@ func resolveModel() (llm.Model, error) { // check and then 401s. if isOpenAICompatProvider(provider) { opts := []openai.Option{openai.WithBaseURL(baseURL)} - if apiKey != "" { + switch { + case provider == "kimi" || provider == "qwen": + // Pass the key UNCONDITIONALLY, even when empty. openai.New defaults + // its credential to OPENAI_API_KEY, so omitting the option sends an + // OpenAI key to Moonshot or Alibaba — a credential handed to the + // wrong vendor, which is exactly what majordomo's built-ins go out + // of their way to prevent. An empty key instead yields a synthetic + // 401 naming the knob that fixes it. + opts = append(opts, + openai.WithAPIKey(apiKey), + openai.WithAPIKeyName("GADFLY_API_KEY"), + ) + case apiKey != "": opts = append(opts, openai.WithAPIKey(apiKey)) + // openai/openai-compatible with no explicit key keep the + // OPENAI_API_KEY default: for those names it IS the right key. } return openai.New(opts...).Model(model) } diff --git a/cmd/gadfly/model_test.go b/cmd/gadfly/model_test.go index 20d42d1..f38e1f1 100644 --- a/cmd/gadfly/model_test.go +++ b/cmd/gadfly/model_test.go @@ -1,9 +1,8 @@ package main import ( - "os" + "os/exec" "path/filepath" - "regexp" "strings" "testing" ) @@ -169,15 +168,29 @@ func TestBuildSpec(t *testing.T) { // - scripts/preflight.sh needs a credential arm, or a missing key for that // provider skips the pre-flight and arrives as five unexplained per-lens // failures — the exact thing the pre-flight exists to replace. +// +// The shell side is queried, not parsed: preflight.sh exports +// gadfly_preflight_providers precisely so this test asks it what it covers. +// Regexing the case statement would make that file's formatting a contract no +// linter enforces, and a harmless reformat would fail a test in another +// language. func TestOpenAICompatProvidersAreFullyWired(t *testing.T) { advertised := make(map[string]bool) for _, n := range strings.Split(endpointProviderNames, "/") { advertised[strings.TrimSpace(n)] = true } - preflight, err := os.ReadFile(filepath.Join("..", "..", "scripts", "preflight.sh")) + script := filepath.Join("..", "..", "scripts", "preflight.sh") + out, err := exec.Command("bash", "-c", ". "+script+"; gadfly_preflight_providers").Output() if err != nil { - t.Fatalf("read preflight.sh: %v", err) + t.Fatalf("query gadfly_preflight_providers from %s: %v", script, err) + } + preflighted := make(map[string]bool) + for _, line := range strings.Fields(string(out)) { + preflighted[line] = true + } + if len(preflighted) == 0 { + t.Fatal("gadfly_preflight_providers returned nothing — this test would pass vacuously") } for _, p := range openAICompatProviders { @@ -185,11 +198,31 @@ func TestOpenAICompatProvidersAreFullyWired(t *testing.T) { t.Errorf("openAICompatProviders has %q but endpointProviderNames does not list it — "+ "the error message operators read would omit a name that works", p) } - // The arm may be shared ("openai|openai-compatible)"), so match the - // bare name as a case alternative rather than a whole line. - if !regexp.MustCompile(`(?m)^\s*(\w[\w-]*\|)*` + regexp.QuoteMeta(p) + `(\|[\w-]+)*\)`).Match(preflight) { + if !preflighted[p] { t.Errorf("openAICompatProviders has %q but scripts/preflight.sh has no credential arm for it — "+ "a missing key for %s would skip the pre-flight and surface as unexplained lens failures", p, p) } } } + +// TestBuiltinCompatProvidersResolveViaRegistry exercises the PRIMARY path: +// a plain "qwen/" in GADFLY_MODELS, with no GADFLY_BASE_URL, resolved +// through majordomo's registry rather than constructed here. +// +// Every other test in this file builds the client directly, so all of them +// passed against a majordomo release that had never heard of qwen — the +// dependency bump this feature depends on was missing and nothing said so. A +// compile error eventually caught it, which is luck, not cover. +func TestBuiltinCompatProvidersResolveViaRegistry(t *testing.T) { + for _, spec := range []string{"qwen/qwen3.8-max", "kimi/kimi-k2-0711-preview"} { + t.Run(spec, func(t *testing.T) { + t.Setenv("GADFLY_MODEL", spec) + t.Setenv("GADFLY_BASE_URL", "") + t.Setenv("GADFLY_PROVIDER", "") + if _, err := resolveModel(); err != nil { + t.Fatalf("resolveModel(%q): %v — the pinned majordomo may not "+ + "provide this built-in; a bump is required, not just gadfly-side wiring", spec, err) + } + }) + } +} diff --git a/go.mod b/go.mod index efecde8..1534410 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.26.2 require ( gitea.stevedudenhoeffer.com/steve/executus v0.1.4 - gitea.stevedudenhoeffer.com/steve/majordomo v0.0.0-20260627225659-aa25b2c33462 + gitea.stevedudenhoeffer.com/steve/majordomo v0.0.0-20260812210334-f837115a55c9 gopkg.in/yaml.v3 v3.0.1 ) diff --git a/go.sum b/go.sum index 581c478..d8be64c 100644 --- a/go.sum +++ b/go.sum @@ -6,8 +6,8 @@ cloud.google.com/go/compute/metadata v0.9.0 h1:pDUj4QMoPejqq20dK0Pg2N4yG9zIkYGdB cloud.google.com/go/compute/metadata v0.9.0/go.mod h1:E0bWwX5wTnLPedCKqk3pJmVgCBSM6qQI1yTBdEb3C10= gitea.stevedudenhoeffer.com/steve/executus v0.1.4 h1:4F99uCV3OVaE9ITFp0FjPiYxLUQO+WpE+wU2HCnpXNM= gitea.stevedudenhoeffer.com/steve/executus v0.1.4/go.mod h1:WQP/lH+meU06OSNF0TQO/wQLcJCrMwpi0EMj5vSpVtk= -gitea.stevedudenhoeffer.com/steve/majordomo v0.0.0-20260627225659-aa25b2c33462 h1:1crjE1YkWHLZ91tUDOxN/Y5cuOnJ56e0U9UADoFfEPY= -gitea.stevedudenhoeffer.com/steve/majordomo v0.0.0-20260627225659-aa25b2c33462/go.mod h1:UZLveG17SmENt4sne2RSLIbioix30RZbRIQUzBAnOyY= +gitea.stevedudenhoeffer.com/steve/majordomo v0.0.0-20260812210334-f837115a55c9 h1:ExY2S6RN1UaA97ju4jzkuEGpfBx0p3vv9FY8B7Npy2I= +gitea.stevedudenhoeffer.com/steve/majordomo v0.0.0-20260812210334-f837115a55c9/go.mod h1:UZLveG17SmENt4sne2RSLIbioix30RZbRIQUzBAnOyY= github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= diff --git a/scripts/preflight.sh b/scripts/preflight.sh index f8586ec..26fdb77 100755 --- a/scripts/preflight.sh +++ b/scripts/preflight.sh @@ -55,12 +55,16 @@ gadfly_preflight_key() { # If that copy ever moves after this call, this arm reports a missing key for # a configured run. case "$provider" in - ollama-cloud) key_env="OLLAMA_API_KEY"; key_hint="OLLAMA_CLOUD_API_KEY" ;; - qwen) key_env="QWEN_API_KEY"; key_hint="QWEN_API_KEY" ;; - kimi) key_env="KIMI_API_KEY"; key_hint="KIMI_API_KEY" ;; - openai|openai-compatible) key_env="OPENAI_API_KEY"; key_hint="OPENAI_API_KEY" ;; - anthropic) key_env="ANTHROPIC_API_KEY"; key_hint="ANTHROPIC_API_KEY" ;; + ollama-cloud) key_env="OLLAMA_API_KEY" ;; + qwen) key_env="QWEN_API_KEY" ;; + kimi) key_env="KIMI_API_KEY" ;; + openai|openai-compatible) key_env="OPENAI_API_KEY" ;; + anthropic) key_env="ANTHROPIC_API_KEY" ;; esac + # The hint is the variable the operator sets, which equals the one the code + # reads everywhere except ollama-cloud (see the note above). + key_hint="$key_env" + [ "$provider" = "ollama-cloud" ] && key_hint="OLLAMA_CLOUD_API_KEY" if [ -z "$key_env" ]; then echo "" # provider needs no pre-flight @@ -75,3 +79,18 @@ gadfly_preflight_key() { fi echo "$key_hint" } + +# gadfly_preflight_providers echoes every provider this file has a credential +# arm for, one per line. +# +# It exists so callers can ASK which providers are covered instead of parsing +# the case statement. A Go test cross-checks this list against the provider +# table in cmd/gadfly/model.go; having it regex this file would make the shell +# formatting a contract no linter enforces, where a reformat breaks a test in +# another language for no visible reason. +# +# Keep in step with the case arms above — the Go test fails if a provider in +# either list is missing from the other. +gadfly_preflight_providers() { + printf '%s\n' ollama-cloud qwen kimi openai openai-compatible anthropic +}