Merge pull request 'feat(qwen): let Qwen (and Kimi) join the swarm' (#30)

.gitea/workflows/adversarial-review.yml

@@ -46,6 +46,16 @@ jobs:
     secrets:
       OLLAMA_CLOUD_API_KEY:    ${{ secrets.OLLAMA_CLOUD_API_KEY }}
       CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
+      # Forwarded so a "qwen/<model>" or "kimi/<model>" entry can join the
+      # swarm by editing the GADFLY_DEFAULT_MODELS var alone — no workflow
+      # edit, no re-release. Both are forwarded together on purpose: the
+      # reusable workflow declares both, and forwarding only one is a config
+      # that looks complete and 401s on the model you didn't wire. Empty until
+      # the repo secret exists, which is a 401 on that one model, not a broken
+      # review. NB kimi/<model> is Moonshot's own API — a different route than
+      # the kimi-k2.6:cloud swarm entry, which rides OLLAMA_CLOUD_API_KEY.
+      QWEN_API_KEY:            ${{ secrets.QWEN_API_KEY }}
+      KIMI_API_KEY:            ${{ secrets.KIMI_API_KEY }}
       GADFLY_FINDINGS_URL:     ${{ secrets.GADFLY_FINDINGS_URL }}
       GADFLY_FINDINGS_TOKEN:   ${{ secrets.GADFLY_FINDINGS_TOKEN }}
     with:

.gitea/workflows/build-image.yml

@@ -45,6 +45,108 @@ env:
   IMAGE_NAME: gitea.stevedudenhoeffer.com/steve/gadfly
 
 jobs:
+  # Runs alongside the image build rather than gating it: a red test should be
+  # loud on the PR without standing between Steve and a rebuild. Added because
+  # this repo had NO test job at all — `go test` and scripts/preflight_test.sh
+  # both existed and neither was ever executed by CI, which is worse than
+  # having no tests, since it reads as coverage.
+  test:
+    runs-on: ubuntu-latest
+    timeout-minutes: 15
+    # This job executes repository code (`go test`) on pull_request, so it gets
+    # the narrowest token the platform will give it. Nothing here writes.
+    permissions:
+      contents: read
+    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
+      # Fetch dependencies, then DESTROY the credential before any step that
+      # executes repository code. REGISTRY_PASSWORD is push-capable, this repo
+      # is public so pull_request runs can carry attacker-authored code, and
+      # `go test` runs that code — a plaintext ~/.gitconfig left in place is a
+      # credential any test could print. The image build faces the same
+      # question and answers it the same way: its creds are BuildKit secrets
+      # scoped to the module-download RUN, never present while code runs.
+      - name: Fetch private modules
+        env:
+          REGISTRY_USER: ${{ secrets.REGISTRY_USER }}
+          REGISTRY_PASSWORD: ${{ secrets.REGISTRY_PASSWORD }}
+        run: |
+          set -euo pipefail
+
+          # Own the config path outright. `git config --global` writes to
+          # GIT_CONFIG_GLOBAL, else $XDG_CONFIG_HOME/git/config when that
+          # directory exists, else ~/.gitconfig — so "delete ~/.gitconfig"
+          # scrubs a file the credential may never have been in. Naming the
+          # path leaves exactly one file to remove.
+          export GIT_CONFIG_GLOBAL="$(mktemp)"
+
+          # Scrub on ANY exit, not just success. `set -e` means a failed
+          # `go mod download` aborts this step, and a cleanup written as the
+          # next line would never run — leaving a push-capable credential on a
+          # long-lived self-hosted runner for whatever job lands there next.
+          trap 'rm -f "$GIT_CONFIG_GLOBAL"' EXIT
+
+          go env -w GOPRIVATE=gitea.stevedudenhoeffer.com/*
+          # Basic-auth header rather than credentials inside the URL: a
+          # password containing @ : / or # breaks URL parsing, and the failure
+          # would look like a bad password rather than a quoting bug.
+          git config --global \
+            "http.https://gitea.stevedudenhoeffer.com/.extraheader" \
+            "Authorization: Basic $(printf '%s:%s' "$REGISTRY_USER" "$REGISTRY_PASSWORD" | base64 | tr -d '\n')"
+          go mod download
+
+          rm -f "$GIT_CONFIG_GLOBAL"
+          test ! -e "$GIT_CONFIG_GLOBAL"
+
+          # Prove the scrub across the whole home dir, not just the file we
+          # deleted — that check would pass no matter what, and git/go can also
+          # write ~/.netrc or ~/.config/go/env. Guarded on a non-empty secret:
+          # `grep -F ""` matches every file, so a secretless run (fork PR) would
+          # fail here with a message accusing it of leaking nothing.
+          # -e, so a password beginning with "-" is a pattern and not options.
+          # And distinguish grep's three exits: 0 found, 1 clean, >=2 ERROR. As
+          # a bare condition an error reads as "not found" and the guard is
+          # skipped — a check that fails OPEN in exactly the case where it can no
+          # longer see the filesystem it is supposed to be searching.
+          if [ -n "${REGISTRY_PASSWORD:-}" ]; then
+            set +e
+            grep -rqF -e "$REGISTRY_PASSWORD" "$HOME" 2>/dev/null
+            rc=$?
+            set -e
+            case "$rc" in
+              0) echo "::error::registry credential still present under \$HOME after scrub"; exit 1 ;;
+              1) : ;;  # clean
+              *) echo "::error::credential scrub check could not run (grep exit $rc); refusing to continue"; exit 1 ;;
+            esac
+          fi
+
+      # GOPROXY=off from here on: the module cache is already warm, so any
+      # attempt to reach the network is a bug — and it fails loudly instead of
+      # quietly looking for the credential that is now gone.
+      - name: go build
+        env: { GOPROXY: "off" }
+        run: go build ./...
+      - name: go vet
+        env: { GOPROXY: "off" }
+        run: go vet ./...
+      - name: gofmt
+        env: { GOPROXY: "off" }
+        run: test -z "$(gofmt -l .)" || { gofmt -l .; exit 1; }
+      - name: go test
+        env: { GOPROXY: "off" }
+        run: go test -count=1 ./...
+      - name: pre-flight credential table
+        run: bash scripts/preflight_test.sh
+
   build-and-push:
     runs-on: ubuntu-latest
     timeout-minutes: 20

.gitea/workflows/review-reusable.yml

@@ -87,6 +87,15 @@ on:
       OPENAI_API_KEY:          { required: false }
       ANTHROPIC_API_KEY:       { required: false }
       GOOGLE_API_KEY:          { required: false }
+      # Alibaba Model Studio (Qwen), for GADFLY_MODELS entries like
+      # "qwen/qwen3.8-max". NOT interchangeable with OPENAI_API_KEY: majordomo's
+      # qwen built-in reads QWEN_API_KEY only and deliberately refuses to fall
+      # back to the OpenAI key, so an unforwarded secret is a 401, not a
+      # mis-billed OpenAI call.
+      QWEN_API_KEY:            { required: false }
+      # Moonshot (Kimi) over its own API — distinct from the ollama-cloud
+      # "kimi-k2.6:cloud" entry, which is keyed by OLLAMA_CLOUD_API_KEY.
+      KIMI_API_KEY:            { required: false }
       GADFLY_API_KEY:          { required: false }
       CLAUDE_CODE_OAUTH_TOKEN: { required: false }
       GADFLY_FINDINGS_URL:     { required: false }
@@ -146,6 +155,12 @@ jobs:
           OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
           ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }}
           GOOGLE_API_KEY: ${{ secrets.GOOGLE_API_KEY }}
+          # Qwen (Alibaba Model Studio) and Kimi (Moonshot) over their own APIs,
+          # for GADFLY_MODELS entries like "qwen/qwen3.8-max". Each built-in
+          # reads ONLY its own variable — no cross-provider fallback — so a
+          # missing line here is a clean 401, never a silently mis-keyed call.
+          QWEN_API_KEY: ${{ secrets.QWEN_API_KEY }}
+          KIMI_API_KEY: ${{ secrets.KIMI_API_KEY }}
           GADFLY_API_KEY: ${{ secrets.GADFLY_API_KEY }}
           CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
           # Named LAN endpoints, defined in user/org vars (format

README.md

@@ -73,11 +73,43 @@ majordomo failover chain / alias) is used verbatim.
 | **[llama-swap](https://github.com/mostlygeek/llama-swap)** (model-swapping proxy) | `llama-swap`/`llama-swaps` (un-hyphenated `llamaswap`/`llamaswaps` also accepted) + `GADFLY_BASE_URL` or a `GADFLY_ENDPOINT_*` entry, or an `LLM_*` `llama-swap://` / `llama-swaps://` DSN | optional bearer | ⚠️ wired, **untested** |
 | **OpenAI-compatible** (incl. local Ollama's `/v1`) | `openai` + `GADFLY_BASE_URL` | `OPENAI_API_KEY` (any non-empty for Ollama) | ✅ tested against Ollama |
 | **OpenAI** | `openai` | `OPENAI_API_KEY` | ⚠️ wired, **untested** |
+| **Qwen** (Alibaba Model Studio) | `qwen` | `QWEN_API_KEY` | ⚠️ wired, **untested** |
+| **Kimi** (Moonshot) | `kimi` | `KIMI_API_KEY` | ⚠️ wired, **untested** |
 | **Anthropic** | `anthropic` | `ANTHROPIC_API_KEY` | ⚠️ wired, **untested** |
 | **Google (Gemini)** | `google` | `GOOGLE_API_KEY` / `GEMINI_API_KEY` | ⚠️ wired, **untested** |
 
+Qwen and Kimi are majordomo built-ins that speak the OpenAI protocol at their own
+endpoints, so `qwen/qwen3.8-max` or `kimi/kimi-k2-0711-preview` work as
+`GADFLY_MODELS` entries with only the matching key set. Each reads **only** its own
+variable — no cross-provider fallback — so forgetting to forward `QWEN_API_KEY`
+gets you a skip notice naming it, not a mis-keyed call. Note `kimi/<model>` (Moonshot's
+API, `KIMI_API_KEY`) is a different route than the `kimi-k2.6:cloud` entry in the
+default swarm, which is Ollama Cloud and keyed by `OLLAMA_CLOUD_API_KEY`.
+
+> **Qwen keys are endpoint-scoped, and the failure looks like a bad key.**
+> Alibaba Model Studio issues *workspace-scoped* endpoints of the form
+> `https://<workspace>.<region>.maas.aliyuncs.com/compatible-mode/v1`. A key
+> issued for one host is rejected by another with a genuine
+> `401 Incorrect API key provided` — so a perfectly good key reads as invalid if
+> the endpoint doesn't match. The built-in defaults to the shared international
+> host; point at your own with a named endpoint, which needs no code change:
+>
+> ```
+> GADFLY_ENDPOINT_QWENWS = "qwen|https://<workspace>.ap-southeast-1.maas.aliyuncs.com/compatible-mode/v1"
+> GADFLY_MODELS          = "qwenws/qwen3.8-max,..."
+> QWEN_API_KEY           = <secret>
+> ```
+>
+> **Leave the key out of the endpoint var.** `GADFLY_ENDPOINT_*` are Gitea
+> *variables*, which are not masked in logs; the third `|<key>` field would put
+> a credential there. Omit it and a `qwen`/`kimi` endpoint falls back to its own
+> `QWEN_API_KEY` / `KIMI_API_KEY` secret — its own vendor's key, never another's.
+>
+> (Verified the hard way against a live deployment.)
+
 > ### 🧪 Honest status
-> Only the **Ollama** paths above are actually exercised. The OpenAI / Anthropic / Google
+> Only the **Ollama** paths above are actually exercised. The OpenAI / Qwen / Kimi /
+> Anthropic / Google
 > providers come "for free" from majordomo's abstraction and *should* work, but I haven't
 > spent money verifying them — treat them as untested. The OpenAI-**compatible** path **is**
 > tested, because you can point it at a local Ollama (`GADFLY_BASE_URL=http://localhost:11434/v1`)

cmd/gadfly/model.go

@@ -3,6 +3,7 @@ package main
 import (
 	"fmt"
 	"os"
+	"slices"
 	"strings"
 
 	"gitea.stevedudenhoeffer.com/steve/majordomo"
@@ -19,6 +20,89 @@ import (
 // model list is just ids like "qwen3-coder:480b-cloud" — working unchanged.
 const defaultProvider = "ollama-cloud"
 
+// openAICompatProviders are the provider names that resolve to the plain
+// openai client at an explicit base URL. openai-compatible is the generic
+// spelling; kimi (Moonshot) and qwen (Alibaba Model Studio) are majordomo
+// built-ins that ARE that client pointed elsewhere, so an explicit endpoint for
+// either belongs on the same branch.
+//
+// One slice, because three places must agree: resolveModel's endpoint
+// override, endpointProvider's GADFLY_ENDPOINT_* parser, and the test that
+// pins them. A name accepted by one and rejected by another is a config that
+// works when written one way and errors the other, for no reason a user could
+// guess.
+var openAICompatProviders = []string{"openai", "openai-compatible", "kimi", "qwen"}
+
+// builtinCompatProviders are the openai-compat names that belong to a DIFFERENT
+// vendor. openai.New defaults its credential to OPENAI_API_KEY, so any of these
+// constructed without an explicit key would put an OpenAI key on the wire to
+// Moonshot or Alibaba. Membership here means "pass the key unconditionally,
+// even empty" — an absent key must be a 401, never a foreign credential.
+var builtinCompatProviders = []string{"kimi", "qwen"}
+
+func isOpenAICompatProvider(name string) bool {
+	return slices.Contains(openAICompatProviders, name)
+}
+
+// isBuiltinCompatProvider mirrors isOpenAICompatProvider rather than testing
+// the slice inline, so both memberships are asked the same way.
+func isBuiltinCompatProvider(name string) bool {
+	return slices.Contains(builtinCompatProviders, name)
+}
+
+// builtinCompatKeyEnv is the provider's own credential variable, matching the
+// name majordomo's built-in reads on the registry path — so the same secret
+// works whether or not an explicit endpoint is configured.
+func builtinCompatKeyEnv(provider string) string {
+	return strings.ToUpper(strings.ReplaceAll(provider, "-", "_")) + "_API_KEY"
+}
+
+// openAICompatOptions builds the option set for an openai-compat provider, and
+// is the ONE place the no-cross-vendor-fallback rule lives.
+//
+// Both resolution paths call it — resolveModel's GADFLY_BASE_URL override and
+// endpointProvider's GADFLY_ENDPOINT_* parser. They had separate copies of this
+// decision once, the guard was added to one of them, and the other kept leaking
+// OPENAI_API_KEY to another vendor. keyHint names the variable to set when the
+// key is absent, since the two paths take it from different places.
+func openAICompatOptions(provider, baseURL, key, keyHint string) []openai.Option {
+	opts := []openai.Option{openai.WithBaseURL(baseURL)}
+	switch {
+	case isBuiltinCompatProvider(provider):
+		// With no explicit key, fall back to the provider's OWN variable
+		// (QWEN_API_KEY, KIMI_API_KEY). That is not the cross-vendor fallback
+		// this function exists to prevent — it is the same vendor's key — and
+		// it lets an operator keep the credential in a masked secret while the
+		// endpoint URL lives in a var, which is NOT masked.
+		//
+		// The hint always names that secret, never the caller's keyHint: on the
+		// GADFLY_ENDPOINT_* path the caller's is the endpoint variable, and
+		// pointing a keyless operator at it advises them to put a credential
+		// somewhere Gitea does not mask.
+		if key == "" {
+			key = os.Getenv(builtinCompatKeyEnv(provider))
+		}
+		opts = append(opts, openai.WithAPIKey(key), openai.WithAPIKeyName(builtinCompatKeyEnv(provider)))
+	case key != "":
+		opts = append(opts, openai.WithAPIKey(key))
+		// openai/openai-compatible with no explicit key keep openai.New's
+		// OPENAI_API_KEY default: for those names it IS the right key.
+	}
+	return opts
+}
+
+// endpointProviderNames is the operator-facing list of providers that accept an
+// explicit endpoint. resolveModel and endpointProvider accept the SAME set, so
+// one message serves both rather than each carrying a copy that drifts in
+// order and spelling.
+//
+// Every accepted spelling belongs here, aliases included —
+// TestEndpointProviderNamesAreAllAccepted asserts that each name listed
+// actually resolves, so an omission fails the build rather than misleading an
+// operator who is already debugging.
+const endpointProviderNames = "openai/openai-compatible/kimi/qwen/ollama/ollama-cloud/" +
+	"llama-swap/llama-swaps/llamaswap/llamaswaps/foreman/anthropic/google/gemini"
+
 // resolveModel builds the review model from the environment. Gadfly is powered
 // by majordomo, so it can target any provider majordomo supports — Ollama
 // (local or cloud), OpenAI, Anthropic, Google, or any OpenAI/Ollama-compatible
@@ -33,10 +117,12 @@ const defaultProvider = "ollama-cloud"
 //	GADFLY_BASE_URL  override the backend endpoint (OpenAI/Ollama-compatible
 //	                 servers, a remote Ollama, an OpenRouter-style gateway…).
 //	                 When set, the provider is constructed directly at that URL.
-//	GADFLY_API_KEY   bearer/API key for the chosen provider. Optional; when
-//	                 unset the provider falls back to its standard env var
-//	                 (OLLAMA_API_KEY / OPENAI_API_KEY / ANTHROPIC_API_KEY /
-//	                 GOOGLE_API_KEY|GEMINI_API_KEY). Local Ollama needs none.
+//	GADFLY_API_KEY   bearer/API key for the chosen provider, used ONLY on the
+//	                 GADFLY_BASE_URL override path. With no base URL the
+//	                 provider reads its own standard variable and this is never
+//	                 consulted: OLLAMA_API_KEY / OPENAI_API_KEY /
+//	                 QWEN_API_KEY / KIMI_API_KEY / ANTHROPIC_API_KEY /
+//	                 GOOGLE_API_KEY|GEMINI_API_KEY. Local Ollama needs none.
 //
 // With GADFLY_BASE_URL unset, resolution goes through majordomo's registry, so
 // LLM_* env DSNs and registered aliases/tiers work too.
@@ -67,13 +153,17 @@ func resolveModel() (llm.Model, error) {
 	}
 
 	// Endpoint override: construct the provider directly at the given URL.
+	// The openai-compat family is matched by the shared predicate, not a
+	// repeated case list. The credential on THIS path is GADFLY_API_KEY; the
+	// built-ins' own KIMI_API_KEY / QWEN_API_KEY are read only on the registry
+	// path above, where GADFLY_BASE_URL is unset. The two paths never share a
+	// credential rule — assuming they do produces a config that passes every
+	// check and then 401s.
+	if isOpenAICompatProvider(provider) {
+		return openai.New(openAICompatOptions(provider, baseURL, apiKey, "GADFLY_API_KEY")...).Model(model)
+	}
+
 	switch provider {
-	case "openai", "openai-compatible":
-		opts := []openai.Option{openai.WithBaseURL(baseURL)}
-		if apiKey != "" {
-			opts = append(opts, openai.WithAPIKey(apiKey))
-		}
-		return openai.New(opts...).Model(model)
 	case "ollama", "ollama-cloud":
 		opts := []ollama.Option{ollama.WithBaseURL(baseURL)}
 		if apiKey != "" {
@@ -108,7 +198,7 @@ func resolveModel() (llm.Model, error) {
 		}
 		return google.New(opts...).Model(model)
 	default:
-		return nil, fmt.Errorf("GADFLY_BASE_URL is set but GADFLY_PROVIDER %q has no endpoint-override support (use openai/openai-compatible/ollama/llama-swap/foreman/anthropic/google, or unset GADFLY_BASE_URL to resolve via majordomo)", provider)
+		return nil, fmt.Errorf("GADFLY_BASE_URL is set but GADFLY_PROVIDER %q has no endpoint-override support (use %s, or unset GADFLY_BASE_URL to resolve via majordomo)", provider, endpointProviderNames)
 	}
 }
 
@@ -188,8 +278,10 @@ func modelProvider() string {
 //	    plaintext local Ollama (or foreman queue) works:
 //	        GADFLY_ENDPOINT_BIGBOX="ollama|http://192.168.1.50:11434"
 //	        GADFLY_MODEL=bigbox/qwen2.5-coder:7b
-//	    provider is one of ollama/llama-swap(s)/foreman/openai/anthropic/google; "foreman"
-//	    targets a foreman daemon (native Ollama on the wire):
+//	    provider is ollama/openai/anthropic/google/foreman/llama-swap(s) or an
+//	    openai-compat built-in (kimi, qwen) — endpointProviderNames is the
+//	    authoritative list. "foreman" targets a foreman daemon (native Ollama
+//	    on the wire):
 //	        GADFLY_ENDPOINT_M1="foreman|http://foreman-m1:8080|tok"
 //
 //	GADFLY_ALIAS_<NAME> = "<majordomo spec>"
@@ -240,6 +332,16 @@ func endpointProvider(name, raw string) (llm.Provider, error) {
 		return nil, fmt.Errorf("missing base URL in %q", raw)
 	}
 
+	// Same shared predicate as resolveModel: the two must accept an identical
+	// set, and a hand-copied case list cannot guarantee that.
+	if isOpenAICompatProvider(provider) {
+		// The key for a named endpoint comes from the third DSN field, so that
+		// is what an absent one points at.
+		opts := append([]openai.Option{openai.WithName(name)},
+			openAICompatOptions(provider, baseURL, key, "GADFLY_ENDPOINT_"+strings.ToUpper(name))...)
+		return openai.New(opts...), nil
+	}
+
 	switch provider {
 	case "ollama", "ollama-cloud":
 		opts := []ollama.Option{ollama.WithName(name), ollama.WithBaseURL(baseURL)}
@@ -258,12 +360,6 @@ func endpointProvider(name, raw string) (llm.Provider, error) {
 		// its non-streaming degradation. Unlike the HTTPS-only LLM_* foreman://
 		// DSN, the base URL here is verbatim, so a plaintext http:// foreman works.
 		return ollama.Foreman(baseURL, key, ollama.WithName(name)), nil
-	case "openai", "openai-compatible":
-		opts := []openai.Option{openai.WithName(name), openai.WithBaseURL(baseURL)}
-		if key != "" {
-			opts = append(opts, openai.WithAPIKey(key))
-		}
-		return openai.New(opts...), nil
 	case "anthropic":
 		opts := []anthropic.Option{anthropic.WithName(name), anthropic.WithBaseURL(baseURL)}
 		if key != "" {
@@ -277,6 +373,6 @@ func endpointProvider(name, raw string) (llm.Provider, error) {
 		}
 		return google.New(opts...), nil
 	default:
-		return nil, fmt.Errorf("unknown provider %q (use ollama/llama-swap(s)/foreman/openai/openai-compatible/anthropic/google)", provider)
+		return nil, fmt.Errorf("unknown provider %q (use %s)", provider, endpointProviderNames)
 	}
 }

cmd/gadfly/model_test.go

@@ -1,6 +1,17 @@
 package main
 
-import "testing"
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"os/exec"
+	"path/filepath"
+	"runtime"
+	"strings"
+	"testing"
+
+	llm "gitea.stevedudenhoeffer.com/steve/majordomo/llm"
+)
 
 func TestEndpointProvider(t *testing.T) {
 	t.Run("ollama http endpoint registers under its name", func(t *testing.T) {
@@ -68,6 +79,70 @@ func TestEndpointProvider(t *testing.T) {
 	}
 }
 
+// TestOpenAICompatProvidersResolveOnBothPaths pins the two provider switches
+// together. kimi and qwen are majordomo built-ins that ARE the openai client at
+// a different base URL, and two independent places have to know it:
+// resolveModel's GADFLY_BASE_URL override, and endpointProvider's
+// GADFLY_ENDPOINT_* parser. A name accepted by one and rejected by the other is
+// a provider that works when configured one way and errors the other, for no
+// reason a user could guess. Asserting both from one table makes the pair fail
+// together.
+func TestOpenAICompatProvidersResolveOnBothPaths(t *testing.T) {
+	// Ranges the SHARED slice: a test that pins a list against drift must not
+	// be able to drift from it.
+	for _, provider := range openAICompatProviders {
+		t.Run(provider+" via GADFLY_ENDPOINT_*", func(t *testing.T) {
+			p, err := endpointProvider("ep", provider+"|https://host.example/v1|sk-x")
+			if err != nil {
+				t.Fatalf("endpointProvider(%q): %v", provider, err)
+			}
+			if p.Name() != "ep" {
+				t.Errorf("Name() = %q, want %q", p.Name(), "ep")
+			}
+		})
+		t.Run(provider+" via GADFLY_BASE_URL", func(t *testing.T) {
+			t.Setenv("GADFLY_PROVIDER", provider)
+			t.Setenv("GADFLY_BASE_URL", "https://host.example/v1")
+			t.Setenv("GADFLY_API_KEY", "sk-x")
+			t.Setenv("GADFLY_MODEL", "some-model")
+			if _, err := resolveModel(); err != nil {
+				t.Fatalf("resolveModel with GADFLY_PROVIDER=%q: %v", provider, err)
+			}
+		})
+	}
+}
+
+// TestEndpointProviderNamesAreAllAccepted keeps the operator-facing list
+// honest: every name endpointProviderNames advertises must actually resolve.
+// The constant is read by somebody whose config just failed, so a name listed
+// there and rejected by the code sends them to debug a spelling that was never
+// going to work.
+func TestEndpointProviderNamesAreAllAccepted(t *testing.T) {
+	for _, name := range strings.Split(endpointProviderNames, "/") {
+		name = strings.TrimSpace(name)
+		if name == "" {
+			continue
+		}
+		// Both switches, not one: this constant is the error text for BOTH
+		// GADFLY_ENDPOINT_* and GADFLY_BASE_URL, so a name accepted by only
+		// half of them still misleads whichever operator hits the other path.
+		t.Run(name+" via GADFLY_ENDPOINT_*", func(t *testing.T) {
+			if _, err := endpointProvider("ep", name+"|https://host.example/v1|sk-x"); err != nil {
+				t.Errorf("endpointProviderNames advertises %q but endpointProvider rejects it: %v", name, err)
+			}
+		})
+		t.Run(name+" via GADFLY_BASE_URL", func(t *testing.T) {
+			t.Setenv("GADFLY_PROVIDER", name)
+			t.Setenv("GADFLY_BASE_URL", "https://host.example/v1")
+			t.Setenv("GADFLY_API_KEY", "sk-x")
+			t.Setenv("GADFLY_MODEL", "some-model")
+			if _, err := resolveModel(); err != nil {
+				t.Errorf("endpointProviderNames advertises %q but resolveModel rejects it: %v", name, err)
+			}
+		})
+	}
+}
+
 func TestBuildSpec(t *testing.T) {
 	tests := []struct {
 		name     string
@@ -89,3 +164,220 @@ func TestBuildSpec(t *testing.T) {
 		})
 	}
 }
+
+// TestOpenAICompatProvidersAreFullyWired closes the two remaining ways this
+// provider family can be half-added. Adding one means touching three places in
+// two languages, and nothing but this test connects them:
+//
+//   - endpointProviderNames is the operator-facing list. A provider the code
+//     accepts but the list omits sends someone debugging a name that works.
+//   - 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
+	}
+
+	// Locate the script relative to THIS source file rather than the working
+	// directory, so moving the package does not silently break the lookup.
+	_, thisFile, _, ok := runtime.Caller(0)
+	if !ok {
+		t.Fatal("runtime.Caller failed; cannot locate scripts/preflight.sh")
+	}
+	script := filepath.Join(filepath.Dir(thisFile), "..", "..", "scripts", "preflight.sh")
+	out, err := exec.Command("bash", "-c", ". "+script+"; gadfly_preflight_providers").Output()
+	if err != nil {
+		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 {
+		if !advertised[p] {
+			t.Errorf("openAICompatProviders has %q but endpointProviderNames does not list it — "+
+				"the error message operators read would omit a name that works", p)
+		}
+		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/<model>" 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)
+			}
+		})
+	}
+}
+
+// TestBuiltinCompatProvidersNeverInheritOpenAIKey pins the rule that has now
+// been broken on one path or the other three separate times: kimi and qwen are
+// other vendors, openai.New defaults its credential to OPENAI_API_KEY, and a
+// provider built without an explicit key therefore puts an OpenAI key on the
+// wire to Moonshot or Alibaba.
+//
+// Both construction paths are asserted from one loop deliberately. Each time
+// this was fixed on a single path the sibling kept leaking, so a test covering
+// one of them would have passed through every one of those bugs.
+//
+// The provider is pointed at a LOCAL server, and the test demands two things:
+// that no request arrives carrying the foreign key, and that the call fails
+// closed naming the variable to set. Without the second half the test would
+// pass on a provider that simply did nothing.
+func TestBuiltinCompatProvidersNeverInheritOpenAIKey(t *testing.T) {
+	const foreign = "sk-openai-must-not-travel"
+
+	for _, provider := range builtinCompatProviders {
+		t.Run(provider+" via GADFLY_BASE_URL", func(t *testing.T) {
+			srv, seen := leakServer(t)
+			t.Setenv("OPENAI_API_KEY", foreign)
+			t.Setenv("GADFLY_PROVIDER", provider)
+			t.Setenv("GADFLY_BASE_URL", srv.URL+"/v1")
+			t.Setenv("GADFLY_API_KEY", "") // the operator forgot the key
+			t.Setenv("GADFLY_MODEL", "some-model")
+
+			m, err := resolveModel()
+			if err != nil {
+				t.Fatalf("resolveModel: %v", err)
+			}
+			assertFailsClosed(t, m, seen, foreign, "QWEN_API_KEY", "KIMI_API_KEY")
+		})
+
+		t.Run(provider+" via GADFLY_ENDPOINT_*", func(t *testing.T) {
+			srv, seen := leakServer(t)
+			t.Setenv("OPENAI_API_KEY", foreign)
+			p, err := endpointProvider("ep", provider+"|"+srv.URL+"/v1") // no key field
+			if err != nil {
+				t.Fatalf("endpointProvider: %v", err)
+			}
+			m, err := p.Model("some-model")
+			if err != nil {
+				t.Fatalf("Model: %v", err)
+			}
+			assertFailsClosed(t, m, seen, foreign, "QWEN_API_KEY", "KIMI_API_KEY")
+		})
+	}
+}
+
+// leakServer returns a server that records every Authorization header it is
+// sent. A request arriving at all means the client did not fail closed.
+func leakServer(t *testing.T) (*httptest.Server, *[]string) {
+	t.Helper()
+	var seen []string
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		seen = append(seen, r.Header.Get("Authorization"))
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{"id":"c1","object":"chat.completion","choices":[{"index":0,` +
+			`"message":{"role":"assistant","content":"ok"},"finish_reason":"stop"}]}`))
+	}))
+	t.Cleanup(srv.Close)
+	return srv, &seen
+}
+
+func assertFailsClosed(t *testing.T, m llm.Model, seen *[]string, foreign string, wantAnyHint ...string) {
+	t.Helper()
+	_, err := m.Generate(context.Background(), llm.Request{Messages: []llm.Message{llm.UserText("hi")}})
+
+	for _, auth := range *seen {
+		if strings.Contains(auth, foreign) {
+			t.Errorf("Authorization carried the OpenAI key to another vendor: %q", auth)
+		}
+	}
+	if len(*seen) > 0 {
+		t.Errorf("a keyless provider reached the network (%d request(s)) instead of failing closed", len(*seen))
+	}
+	// The positive half: prove it refused for the right reason, so the test
+	// cannot pass on a provider that quietly did nothing at all.
+	if err == nil {
+		t.Fatal("keyless provider returned no error; expected a missing-key failure")
+	}
+	// The hint must name a MASKED secret the operator can set, never the
+	// unmasked GADFLY_ENDPOINT_* variable.
+	named := false
+	for _, h := range wantAnyHint {
+		if strings.Contains(err.Error(), h) {
+			named = true
+		}
+	}
+	if !named {
+		t.Errorf("error = %v, want it to name one of %v so the operator knows what to set", err, wantAnyHint)
+	}
+}
+
+// TestBuiltinCompatOwnKeyFallback: with no key in the endpoint definition, a
+// built-in falls back to its OWN variable (QWEN_API_KEY, KIMI_API_KEY) — never
+// to another vendor's. This is what lets the credential live in a masked
+// secret while the endpoint URL lives in a GADFLY_ENDPOINT_* var, which Gitea
+// does not mask; the README used to advise embedding the key in that var.
+func TestBuiltinCompatOwnKeyFallback(t *testing.T) {
+	const own, foreign = "sk-qwen-own", "sk-openai-must-not-travel"
+
+	srv, seen := leakServer(t)
+	t.Setenv("OPENAI_API_KEY", foreign)
+	t.Setenv("QWEN_API_KEY", own)
+
+	p, err := endpointProvider("ep", "qwen|"+srv.URL+"/v1") // no key field
+	if err != nil {
+		t.Fatalf("endpointProvider: %v", err)
+	}
+	m, err := p.Model("some-model")
+	if err != nil {
+		t.Fatalf("Model: %v", err)
+	}
+	if _, err := m.Generate(context.Background(), llm.Request{Messages: []llm.Message{llm.UserText("hi")}}); err != nil {
+		t.Fatalf("Generate: %v", err)
+	}
+	if len(*seen) == 0 {
+		t.Fatal("no request reached the server — the own-key fallback did not take effect")
+	}
+	for _, auth := range *seen {
+		if strings.Contains(auth, foreign) {
+			t.Errorf("Authorization carried the OpenAI key: %q", auth)
+		}
+		if !strings.Contains(auth, own) {
+			t.Errorf("Authorization = %q, want the provider's own QWEN_API_KEY", auth)
+		}
+	}
+}
+
+// TestBuiltinCompatProvidersAreOpenAICompat: the two slices are parallel, and a
+// built-in missing from openAICompatProviders would never reach the branch that
+// applies its unconditional-key rule — it would fall through to the generic
+// switch and silently lose the protection.
+func TestBuiltinCompatProvidersAreOpenAICompat(t *testing.T) {
+	for _, p := range builtinCompatProviders {
+		if !isOpenAICompatProvider(p) {
+			t.Errorf("%q is in builtinCompatProviders but not openAICompatProviders, so the "+
+				"no-cross-vendor-fallback branch never runs for it", p)
+		}
+	}
+}

entrypoint.sh

@@ -33,9 +33,17 @@
 # Optional config:
 #   GADFLY_MODELS         comma-separated model ids/specs (alias: OLLAMA_REVIEW_MODELS)
 #   GADFLY_PROVIDER       majordomo provider for bare model ids (default ollama-cloud;
-#                         e.g. "ollama" local, "openai", "anthropic", "google")
+#                         e.g. "ollama" local, "openai", "anthropic", "google",
+#                         "qwen" Alibaba Model Studio, "kimi" Moonshot)
 #   GADFLY_BASE_URL       override backend endpoint (OpenAI/Ollama-compatible servers)
 #   GADFLY_API_KEY        provider key (else provider's standard env: OPENAI_API_KEY, …)
+#   QWEN_API_KEY          Alibaba Model Studio key, for GADFLY_MODELS entries like
+#                         "qwen/qwen3.8-max". Read ONLY by the qwen provider — it
+#                         does not fall back to OPENAI_API_KEY, so a forgotten key
+#                         is a clean skip notice naming this variable, not a 401.
+#   KIMI_API_KEY          Moonshot key, same deal for "kimi/<model>". Distinct from
+#                         the ollama-cloud "kimi-k2.6:cloud" entry, which is keyed
+#                         by OLLAMA_CLOUD_API_KEY.
 #   CLAUDE_CODE_OAUTH_TOKEN  auth for the claude-code engine (GADFLY_MODELS entry
 #                         "claude-code"/"claude-code/<model>"); Pro/Max subscription
 #                         token from `claude setup-token`. Else ANTHROPIC_API_KEY.

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
 )
 

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=

scripts/preflight.sh

@@ -0,0 +1,135 @@
+#!/usr/bin/env bash
+# Credential pre-flight for the agentic reviewer, in ONE definition.
+#
+# Sourced by run.sh (production) and by preflight_test.sh (the table test), so
+# the tested bytes and the running bytes are the same. Keep it that way: a test
+# that reimplements this logic can agree with a stale copy of it.
+#
+# Why pre-flight at all, when majordomo already fails closed with a 401:
+# without it a missing key surfaces as five identical per-lens agent failures
+# that name no variable, and the operator reads a stack trace to learn which
+# secret they forgot to forward.
+
+# gadfly_preflight_key <provider> -> echoes "" when the run may proceed, or the
+# name of the environment variable the operator must set.
+#
+# Scope: the REGISTRY path only — GADFLY_BASE_URL unset — and deliberately so.
+# The two resolution paths have DIFFERENT credential rules: with an explicit
+# endpoint the credential is GADFLY_API_KEY (falling back to the client's own
+# default, OPENAI_API_KEY for the openai family) and a built-in's own variable
+# is never consulted; without one, the reverse. Applying either path's rule to
+# the other yields a check that passes a run which then 401s — the precise
+# failure this exists to prevent. So it covers the path whose rules it can state
+# exactly and stays silent on the other. That is also the useful half: an
+# override-path config is hand-written, while the registry path is what somebody
+# hits by adding a model id to a var and forgetting the secret.
+gadfly_preflight_key() {
+  local provider="$1" model="${2:-}" key_env="" key_hint=""
+
+  # claude-code carries its OWN auth (CLAUDE_CODE_OAUTH_TOKEN, else
+  # ANTHROPIC_API_KEY) and needs no Ollama key. A bare "claude-code" has no "/",
+  # so the caller's provider falls back to ollama-cloud and the table below
+  # would skip a perfectly configured reviewer.
+  #
+  # opencode is deliberately NOT exempt: that engine drives an ollama-cloud
+  # model through the bundled CLI and authenticates with OLLAMA_API_KEY, so it
+  # needs exactly the key the table checks. Exempting it — which an earlier
+  # version of this guard did — turns the pre-flight off for the one engine
+  # whose missing key it could still catch.
+  model="$(printf '%s' "$model" | tr -d '[:space:]')"   # Go trims GADFLY_MODEL
+  case "$model" in
+    claude-code|claude-code/*) echo ""; return 0 ;;
+  esac
+
+  # Trim before testing: resolveModel does strings.TrimSpace on GADFLY_BASE_URL,
+  # so a whitespace-only value takes the REGISTRY path there. Testing the raw
+  # value here would call it "set", skip the check, and let the missing key
+  # arrive as a 401 with no notice — the two must agree on what "unset" means.
+  local base_url
+  base_url="$(printf '%s' "${GADFLY_BASE_URL:-}" | tr -d '[:space:]')"
+
+  if [ -n "$base_url" ]; then
+    # Endpoint-override path. Most providers take their credential from
+    # GADFLY_API_KEY here with a client-specific fallback, and those rules are
+    # not worth restating — this stays silent for them.
+    #
+    # The built-ins are the exception, and only since they gained an own-key
+    # fallback: a keyless kimi/qwen endpoint reads QWEN_API_KEY / KIMI_API_KEY
+    # on THIS path too, so "own key or GADFLY_API_KEY" is a rule that can be
+    # stated exactly. Leaving them unchecked here would let a keyless override
+    # config sail past the pre-flight and fail as a 401 — the failure the
+    # pre-flight exists to replace.
+    case "$provider" in
+      qwen|kimi) ;;
+      *) echo ""; return 0 ;;
+    esac
+    local own_env="$(printf '%s' "$provider" | tr '[:lower:]-' '[:upper:]_')_API_KEY"
+    if [ -n "${!own_env:-}" ] || [ -n "${GADFLY_API_KEY:-}" ]; then
+      echo ""
+    else
+      echo "$own_env"
+    fi
+    return 0
+  fi
+
+  local row
+  row="$(_gadfly_preflight_table | awk -F: -v p="$provider" '$1 == p {print; exit}')"
+  if [ -z "$row" ]; then
+    echo ""                       # provider needs no pre-flight
+    return 0
+  fi
+  key_env="$(printf '%s' "$row" | cut -d: -f2)"
+  key_hint="$(printf '%s' "$row" | cut -d: -f3)"
+  [ -n "$key_hint" ] || key_hint="$key_env"
+
+  # Indirect expansion (bash). Each majordomo built-in reads ONLY its own
+  # variable — cross-provider fallback is refused by design — so the named hint
+  # is always the actual fix.
+  if [ -n "${!key_env:-}" ]; then
+    echo ""
+    return 0
+  fi
+  echo "$key_hint"
+}
+
+# _gadfly_preflight_table is the single source for both the credential lookup
+# and the provider list: "<provider>:<env-var-read>:<env-var-to-suggest>".
+#
+# The third field is normally empty, meaning "same as the second". ollama-cloud
+# is the exception: run.sh copies the consumer-facing OLLAMA_CLOUD_API_KEY onto
+# the OLLAMA_API_KEY the provider reads BEFORE calling in here, so the check and
+# the hint name different variables on purpose. If that copy ever moves after
+# the call, this arm reports a missing key for a configured run.
+#
+# A provider absent from this table is absent for one of TWO reasons — do not
+# assume the first and add a row:
+#   1. It needs no key, or carries one in its endpoint/DSN: local ollama,
+#      llama-swap, foreman.
+#   2. It needs a key but accepts more than one variable, so a single-name check
+#      would skip a correctly-configured run. **google** is this case
+#      (GOOGLE_API_KEY *or* GEMINI_API_KEY); pre-flighting it needs an
+#      either-variable check, not this table's one-name shape.
+_gadfly_preflight_table() {
+  printf '%s\n' \
+    'ollama-cloud:OLLAMA_API_KEY:OLLAMA_CLOUD_API_KEY' \
+    'opencode:OLLAMA_API_KEY:OLLAMA_CLOUD_API_KEY' \
+    'open-code:OLLAMA_API_KEY:OLLAMA_CLOUD_API_KEY' \
+    'qwen:QWEN_API_KEY:' \
+    'kimi:KIMI_API_KEY:' \
+    'openai:OPENAI_API_KEY:' \
+    'openai-compatible:OPENAI_API_KEY:' \
+    'anthropic:ANTHROPIC_API_KEY:'
+}
+
+# gadfly_preflight_providers echoes every provider covered above, one per line.
+# Callers ASK rather than parse: a Go test cross-checks this against the
+# openai-compat provider table in cmd/gadfly/model.go, and regexing this file
+# would make its formatting a contract no linter enforces.
+#
+# The cross-check runs ONE direction — every openai-compat provider in Go must
+# appear here. The reverse is not required and must not be asserted:
+# ollama-cloud and anthropic belong in this table and are deliberately not in
+# that Go list.
+gadfly_preflight_providers() {
+  _gadfly_preflight_table | cut -d: -f1
+}

scripts/preflight_test.sh

@@ -0,0 +1,115 @@
+#!/usr/bin/env bash
+# Table test for the credential pre-flight in preflight.sh.
+#
+# It SOURCES the real implementation rather than copying it, so there is no
+# second definition that can pass while production fails.
+#
+# Run: scripts/preflight_test.sh   (exit 0 = all cases pass)
+set -u
+
+SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
+# shellcheck source=preflight.sh
+. "$SCRIPT_DIR/preflight.sh"
+
+fail=0
+check() { # description, want, got
+  if [ "$2" = "$3" ]; then
+    echo "ok   $1"
+  else
+    echo "FAIL $1 — want '$2', got '$3'"
+    fail=1
+  fi
+}
+
+# probe <provider> [VAR=VAL ...] — run the real function in a clean environment
+# under the same shell options production uses (set -u), so an unset-variable
+# bug surfaces here instead of in a live review.
+probe() {
+  local provider="$1" model="${GADFLY_TEST_MODEL:-}"; shift
+  env -i PATH="$PATH" HOME="$HOME" "$@" bash -c "
+    set -u
+    . '$SCRIPT_DIR/preflight.sh'
+    gadfly_preflight_key '$provider' '$model'
+  "
+}
+
+echo "== registry path: keyed providers with no key must name their variable =="
+check "qwen, no key"             "QWEN_API_KEY"          "$(probe qwen)"
+check "kimi, no key"             "KIMI_API_KEY"          "$(probe kimi)"
+check "ollama-cloud, no key"     "OLLAMA_CLOUD_API_KEY"  "$(probe ollama-cloud)"
+check "openai, no key"           "OPENAI_API_KEY"        "$(probe openai)"
+check "openai-compatible, none"  "OPENAI_API_KEY"        "$(probe openai-compatible)"
+check "anthropic, no key"        "ANTHROPIC_API_KEY"     "$(probe anthropic)"
+
+echo "== registry path: the provider's own key lets it run =="
+check "qwen, keyed"              "" "$(probe qwen QWEN_API_KEY=k)"
+check "kimi, keyed"              "" "$(probe kimi KIMI_API_KEY=k)"
+check "ollama-cloud, keyed"      "" "$(probe ollama-cloud OLLAMA_API_KEY=k)"
+check "openai-compatible, keyed" "" "$(probe openai-compatible OPENAI_API_KEY=k)"
+
+echo "== a wrong-provider key never satisfies a provider (no cross-fallback) =="
+check "qwen w/ only OPENAI key"  "QWEN_API_KEY" "$(probe qwen OPENAI_API_KEY=k)"
+check "kimi w/ only QWEN key"    "KIMI_API_KEY" "$(probe kimi QWEN_API_KEY=k)"
+
+echo "== an empty-string key counts as missing, not present =="
+check "qwen, empty key"          "QWEN_API_KEY" "$(probe qwen QWEN_API_KEY=)"
+
+echo "== GADFLY_API_KEY does NOT substitute on the registry path =="
+# resolveModel reads GADFLY_API_KEY only after its `baseURL == ""` early
+# return, so on this path the built-in reads its own variable and a set
+# GADFLY_API_KEY changes nothing. Treating it as sufficient was a false pass.
+check "qwen w/ GADFLY_API_KEY only" "QWEN_API_KEY" "$(probe qwen GADFLY_API_KEY=k)"
+
+echo "== override path: built-ins ARE checked; others are not =="
+# The credential there is GADFLY_API_KEY with a client-specific fallback, and
+# the built-ins' own variables are never read. Checking one path's rules
+# against the other produced a false pass in BOTH directions, so this path is
+# left alone rather than guessed at.
+# A built-in reads its own key on the override path too (openAICompatOptions
+# falls back to QWEN_API_KEY/KIMI_API_KEY there), so "own key or GADFLY_API_KEY"
+# is statable and worth checking — leaving it unchecked let a keyless config
+# sail past and fail as a 401.
+check "qwen + BASE_URL, no keys"     "QWEN_API_KEY" "$(probe qwen GADFLY_BASE_URL=https://x)"
+check "qwen + BASE_URL + own key"    "" "$(probe qwen GADFLY_BASE_URL=https://x QWEN_API_KEY=k)"
+check "qwen + BASE_URL + GADFLY key" "" "$(probe qwen GADFLY_BASE_URL=https://x GADFLY_API_KEY=k)"
+check "kimi + BASE_URL, no keys"     "KIMI_API_KEY" "$(probe kimi GADFLY_BASE_URL=https://x)"
+# Other providers' override-path rules are not statable, so this stays quiet.
+check "openai + BASE_URL, no keys"   "" "$(probe openai GADFLY_BASE_URL=https://x)"
+check "anthropic + BASE_URL, none"   "" "$(probe anthropic GADFLY_BASE_URL=https://x)"
+
+echo "== providers needing no key are never blocked, with nothing set =="
+for p in ollama llama-swap llama-swaps llamaswap llamaswaps foreman google gemini some-dsn-name; do
+  check "unkeyed $p" "" "$(probe "$p")"
+done
+
+# google is absent from the table on purpose: it accepts GOOGLE_API_KEY *or*
+# GEMINI_API_KEY, so a one-name arm would skip a correctly-configured run.
+check "google w/ only GEMINI_API_KEY" "" "$(probe google GEMINI_API_KEY=k)"
+
+echo "== a whitespace-only GADFLY_BASE_URL counts as unset, as it does in Go =="
+# resolveModel TrimSpaces it and takes the registry path; if this check
+# disagreed, the missing key would arrive as a bare 401 with no skip notice.
+check "qwen + blank BASE_URL" "QWEN_API_KEY" "$(probe qwen GADFLY_BASE_URL="   ")"
+
+echo "== engine specs carry their own auth and are never pre-flighted =="
+# A bare "claude-code" has no "/", so the caller's provider falls back to
+# ollama-cloud; judging it by that would skip a reviewer using
+# CLAUDE_CODE_OAUTH_TOKEN, which needs no Ollama key.
+check "bare claude-code, no ollama key" "" "$(GADFLY_TEST_MODEL=claude-code probe ollama-cloud)"
+# Go trims GADFLY_MODEL, so padding must not bypass the exemption.
+check "claude-code w/ whitespace"      "" "$(GADFLY_TEST_MODEL="  claude-code  " probe ollama-cloud)"
+check "claude-code/opus, no ollama key" "" "$(GADFLY_TEST_MODEL=claude-code/opus probe ollama-cloud)"
+# opencode is NOT exempt: it drives an ollama-cloud model and needs that key,
+# so skipping it would disable the pre-flight for the one engine it can help.
+check "opencode/x, no ollama key"       "OLLAMA_CLOUD_API_KEY" "$(GADFLY_TEST_MODEL=opencode/x probe opencode)"
+check "bare opencode, no ollama key"    "OLLAMA_CLOUD_API_KEY" "$(GADFLY_TEST_MODEL=opencode probe ollama-cloud)"
+check "open-code/x, no ollama key"      "OLLAMA_CLOUD_API_KEY" "$(GADFLY_TEST_MODEL=open-code/x probe open-code)"
+check "opencode/x, keyed"               "" "$(GADFLY_TEST_MODEL=opencode/x probe opencode OLLAMA_API_KEY=k)"
+# ...but a genuine ollama-cloud model still is.
+check "ollama-cloud model, no key"      "OLLAMA_CLOUD_API_KEY" "$(GADFLY_TEST_MODEL=glm-5.2:cloud probe ollama-cloud)"
+
+if [ "$fail" -ne 0 ]; then
+  echo "RESULT: preflight table FAILED"
+  exit 1
+fi
+echo "RESULT: all pre-flight cases pass"

scripts/run.sh

@@ -48,6 +48,11 @@ set -uo pipefail
 SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 MAX_DIFF_CHARS="${MAX_DIFF_CHARS:-60000}"
 
+# Credential pre-flight, shared verbatim with scripts/preflight_test.sh so the
+# tested logic and the running logic are the same bytes.
+# shellcheck source=preflight.sh
+. "$SCRIPT_DIR/preflight.sh"
+
 : "${GITEA_API:?GITEA_API required}"
 : "${GITEA_TOKEN:?GITEA_TOKEN required}"
 : "${PR:?PR required}"
@@ -162,10 +167,12 @@ case "$PROVIDER" in
     fi
     GADFLY_PROVIDER_EFF="$MODEL_PROVIDER"
 
-    # Only the default cloud provider strictly needs a key up front; local Ollama
-    # and other providers either need none or read their own standard env var.
-    if [ "$GADFLY_PROVIDER_EFF" = "ollama-cloud" ] && [ -z "${OLLAMA_API_KEY:-}" ] && [ -z "${GADFLY_API_KEY:-}" ]; then
-      REVIEW="⚠️ No Ollama Cloud key configured (set \`OLLAMA_CLOUD_API_KEY\`) and \`GADFLY_PROVIDER\` is the default \`ollama-cloud\`; this reviewer was skipped."
+    # Credential pre-flight — one definition, shared with preflight_test.sh.
+    # Pass the raw spec too: engine specs (claude-code/opencode) carry their
+    # own auth and must not be judged by the provider fallback.
+    MISSING_KEY="$(gadfly_preflight_key "$GADFLY_PROVIDER_EFF" "$MODEL")"
+    if [ -n "$MISSING_KEY" ]; then
+      REVIEW="⚠️ No API key configured for provider \`${GADFLY_PROVIDER_EFF}\` (set \`${MISSING_KEY}\`); this reviewer was skipped."
     else
       BIN="${GADFLY_BIN:-gadfly}"
       if ! command -v "$BIN" >/dev/null 2>&1 && [ ! -x "$BIN" ]; then