fix(ci): scrub the registry credential before running repo code

Both Claude reviewers caught this independently, and they are right. The test
job I added wrote a PUSH-CAPABLE REGISTRY_PASSWORD into a plaintext
~/.gitconfig and then ran `go build`/`go vet`/`go test` — repository code — on
pull_request events. This repo is public, so a fork PR could ship a test whose
only job is to print that file. The image build had already answered this
question correctly: its credentials are BuildKit secrets scoped to the
module-download RUN and are never present while code executes. I bolted on a
job that skipped the boundary its neighbour maintains.

Dependencies are now fetched in their own step which deletes ~/.gitconfig
before anything else runs, and asserts the scrub — against the whole home
directory, not against the file it just removed, because the credential can
also land in ~/.netrc or ~/.config/go/env. Verified the assertion is not
vacuous: planting the secret in ~/.netrc trips it. Later steps run with
GOPROXY=off, so any attempt to reach the network fails loudly rather than
quietly hunting for the credential that is now gone.

Also from round 4: TestEndpointProviderNamesAreAllAccepted pinned only
endpointProvider, while the constant is the error text for BOTH resolution
paths — it now asserts each advertised name resolves either way (break-checked
by dropping the gemini alias from resolveModel alone). preflight.sh documents
that ollama-cloud is checked on OLLAMA_API_KEY but hinted as
OLLAMA_CLOUD_API_KEY because run.sh copies one to the other first, an ordering
dependency that was invisible from the file.

And the comments that narrated this PR's own edit history ("the first version
of this change...") are rewritten as invariants. That history stops being true
the moment this merges, and the repo's doc policy says as much.

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

.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,61 @@ 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
+    steps:
+      - uses: actions/checkout@v4
+      - 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: |
+          go env -w GOPRIVATE=gitea.stevedudenhoeffer.com/*
+          git config --global url."https://${REGISTRY_USER}:${REGISTRY_PASSWORD}@gitea.stevedudenhoeffer.com/".insteadOf "https://gitea.stevedudenhoeffer.com/"
+          go mod download
+          rm -f "$HOME/.gitconfig"
+          # Prove the scrub worked, and prove it against the whole home dir —
+          # checking only the file just deleted would pass no matter what, and
+          # the credential can also reach ~/.netrc or ~/.config/go/env.
+          test ! -e "$HOME/.gitconfig"
+          if grep -rqF "${REGISTRY_PASSWORD}" "$HOME" 2>/dev/null; then
+            echo "::error::registry credential still present under \$HOME after scrub"
+            exit 1
+          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
+        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

@@ -44,7 +44,7 @@ on:
     #
     # Owner-set user-scope variables (see README "Central config via variables"):
     #   GADFLY_DEFAULT_MODELS, GADFLY_DEFAULT_SPECIALISTS,
-    #   GADFLY_DEFAULT_PROVIDER_CONCURRENCY, GADFLY_DEFAULT_PROVIDER_LENS_CONCURRENCY,
+    #   GADFLY_DEFAULT_PROVIDER_LENS_CONCURRENCY (the provider-wide lens budget),
     #   GADFLY_ENDPOINT_NETHERSTORM (the local GPU box endpoint).
     # An unset variable + no input → the image default (one model, default suite),
     # so a public consumer with neither still gets a sane minimal review.
@@ -53,8 +53,8 @@ on:
       specialists:               { type: string, default: "" }   # GADFLY_SPECIALISTS — empty falls back to user var GADFLY_DEFAULT_SPECIALISTS
       provider:                  { type: string, default: "" }   # GADFLY_PROVIDER
       base_url:                  { type: string, default: "" }   # GADFLY_BASE_URL
-      provider_concurrency:      { type: string, default: "" }   # GADFLY_PROVIDER_CONCURRENCY — empty falls back to user var GADFLY_DEFAULT_PROVIDER_CONCURRENCY
+      provider_concurrency:      { type: string, default: "" }   # DEPRECATED / ignored — the per-provider MODEL cap was removed; the lens budget below is the single throttle. Kept so existing callers don't error.
-      provider_lens_concurrency: { type: string, default: "" }   # GADFLY_PROVIDER_LENS_CONCURRENCY — empty falls back to user var GADFLY_DEFAULT_PROVIDER_LENS_CONCURRENCY
+      provider_lens_concurrency: { type: string, default: "" }   # GADFLY_PROVIDER_LENS_CONCURRENCY — the per-provider lens budget (shared across the provider's models); empty falls back to user var GADFLY_DEFAULT_PROVIDER_LENS_CONCURRENCY
       timeout_secs:              { type: string, default: "600" }   # GADFLY_TIMEOUT_SECS (per lens)
       max_steps:                 { type: string, default: "14" }   # GADFLY_MAX_STEPS
       worker_model:              { type: string, default: "" }   # GADFLY_WORKER_MODEL
@@ -82,6 +82,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 }
@@ -99,11 +108,12 @@ jobs:
     runs-on: ubuntu-latest
     timeout-minutes: ${{ inputs.timeout_minutes }}
     steps:
-      # sha-3095ebf predated the Gitea >= 1.27 workflow_call reclassification
+      # Pin the reviewer image to an immutable sha (act_runner caches :latest, so a
-      # (entrypoint.sh, added in 9d74cb9) — so the dogfood self-review skipped every
+      # moved :latest is often NOT re-pulled). sha-b37cd09 adds the provider-wide
-      # PR with "event 'workflow_call' not handled". Bumped to sha-f468fe6 (current
+      # lens budget (PR #27: one shared lens-permit pool per provider, the model cap
-      # main HEAD) which contains that fix. Bump per Gadfly release.
+      # removed) on top of the opencode CLI engine (PR #26) and the Gitea >= 1.27
-      - uses: docker://gitea.stevedudenhoeffer.com/steve/gadfly:sha-f468fe6
+      # workflow_call reclassification. Bump per Gadfly release.
+      - uses: docker://gitea.stevedudenhoeffer.com/steve/gadfly:sha-b37cd09
         env:
           # --- event context (from the CALLER's github.*) -------------------
           GITEA_API: ${{ github.server_url }}/api/v1/repos/${{ github.repository }}
@@ -126,6 +136,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
@@ -151,7 +167,9 @@ jobs:
           GADFLY_SPECIALISTS: ${{ inputs.specialists || vars.GADFLY_DEFAULT_SPECIALISTS }}
           GADFLY_PROVIDER: ${{ inputs.provider }}
           GADFLY_BASE_URL: ${{ inputs.base_url }}
-          GADFLY_PROVIDER_CONCURRENCY: ${{ inputs.provider_concurrency || vars.GADFLY_DEFAULT_PROVIDER_CONCURRENCY }}
+          # NB: GADFLY_PROVIDER_CONCURRENCY (the old model cap) is intentionally no
+          # longer forwarded — entrypoint.sh ignores it. The lens budget is the one
+          # throttle now, shared across a provider's models.
           GADFLY_PROVIDER_LENS_CONCURRENCY: ${{ inputs.provider_lens_concurrency || vars.GADFLY_DEFAULT_PROVIDER_LENS_CONCURRENCY }}
           GADFLY_TIMEOUT_SECS: ${{ inputs.timeout_secs }}
           GADFLY_MAX_STEPS: ${{ inputs.max_steps }}

CLAUDE.md

@@ -57,7 +57,7 @@ Dockerfile             multi-stage; private-module creds via BuildKit secrets ne
 .gitea/workflows/build-image.yml   push main → :latest; tag v* → :<tag>+:latest; PR → build-only
 .gitea/workflows/review-reusable.yml  reusable (workflow_call) review job; resolves swarm config at
                        RUNTIME: consumer `with:` input → owner user-scope var (GADFLY_DEFAULT_MODELS /
-                       _SPECIALISTS / _PROVIDER_CONCURRENCY / _PROVIDER_LENS_CONCURRENCY, +
+                       _SPECIALISTS / _PROVIDER_LENS_CONCURRENCY, +
                        GADFLY_ENDPOINT_RAGNAROS) → image default. Vars are injected per-run, so editing
                        one var retunes the whole fleet even though long-lived act_runners CACHE this file
                        by ref (a moved tag is NOT re-fetched — only a runtime value or a fresh @<sha>
@@ -154,10 +154,19 @@ are actually exercised. OpenAI/Anthropic/Google come from majordomo's abstractio
   NOT re-pulled, so the job silently runs the previous image. For a run that must use a specific
   build (e.g. validating a just-pushed fix), pin the consumer stub to the immutable
   `:sha-<short>` tag the build publishes, not `:latest`.
-- **Concurrency is per-provider** (`entrypoint.sh`): each provider is a lane, lanes run in
+- **Concurrency is one per-provider lens budget** (`entrypoint.sh` + `cmd/gadfly/lenssem.go`):
-  parallel, `cap` (from `GADFLY_PROVIDER_CONCURRENCY` else `GADFLY_CONCURRENCY`, default 1) bounds
+  each provider is a lane, lanes run in parallel, and within a lane ALL of the provider's models
-  models-at-once within a lane. The review timeout (`GADFLY_TIMEOUT_SECS`) is **per-lens**, not
+  run at once — the only throttle is a **provider-wide lens budget** (max lens passes in flight,
-  shared across the suite — a slow model can't starve later lenses (the original timeout bug).
+  a lens = one specialist's review+recheck). The budget comes from
+  `GADFLY_PROVIDER_LENS_CONCURRENCY` (`provider=N` map) else `GADFLY_LENS_CONCURRENCY` (default 1).
+  Because models are separate processes, the budget is a **cross-process permit pool**: entrypoint
+  seeds a per-lane dir of N flock files; each model's binary acquires one before a lens pass and
+  releases it after (flock auto-drops on process death). This replaced the old two-level
+  `GADFLY_PROVIDER_CONCURRENCY` MODEL cap × per-model `GADFLY_LENS_CONCURRENCY`, which multiplied
+  and let a model hold its slot through its last lens — stalling the next model with idle lens
+  capacity. Those two model-cap vars are now **ignored**. The review timeout
+  (`GADFLY_TIMEOUT_SECS`) is **per-lens**, not shared across the suite — a slow lens can't starve
+  the others (the original timeout bug).
 - **Large-PR token burn**: the agent loop re-sends the whole transcript every step, so a giant
   diff (the old `get_diff` dumped it untruncated, and it was embedded in both the review and
   recheck task) was re-transmitted ~steps × lenses × passes × models times — a ~250 K-token PR

README.md

@@ -73,11 +73,37 @@ 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|<key>"
+> GADFLY_MODELS          = "qwenws/qwen3.8-max,..."
+> ```
+>
+> (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`)
@@ -272,38 +298,32 @@ Unset = no delegation (current behavior).
 ### Concurrency (per-provider lanes)
 
 With multiple models, each **provider** is its own lane and lanes run in **parallel**, so a fast
-cloud provider isn't stuck behind a slow local box. Within a lane, at most `cap` models run at
+cloud provider isn't stuck behind a slow local box. There is **one throttle**: a per-provider
-once — `cap` comes from `GADFLY_PROVIDER_CONCURRENCY` (a `provider=N` map) else `GADFLY_CONCURRENCY`
+**lens budget** — the max number of lens passes (a lens = one specialist's review+recheck) in
-(default `1`). The timeout is **per-lens** (`GADFLY_TIMEOUT_SECS`), so a slow model on one lens
+flight at once for that provider. Every model in the lane runs concurrently and its lenses draw
-can't starve the others.
+from that single shared budget, so nothing else caps how many models run. The budget comes from
+`GADFLY_PROVIDER_LENS_CONCURRENCY` (a `provider=N` map) else the `GADFLY_LENS_CONCURRENCY` scalar
+(default `1`). The timeout is **per-lens** (`GADFLY_TIMEOUT_SECS`), so a slow lens can't starve
+the others.
 
 ```yaml
-# One local box (serial — it serves one model at a time) + 3 cloud reviews at once,
+# The local box gets 1 lens at a time (serial); the cloud lane runs up to 3 lens passes at once,
-# both lanes running concurrently:
+# shared across ALL its models. Both lanes run concurrently.
-GADFLY_PROVIDER_CONCURRENCY: "ollama-cloud=3,m1pro=1"
+GADFLY_PROVIDER_LENS_CONCURRENCY: "ollama-cloud=3,m1pro=1"
 GADFLY_MODELS: "m1pro/qwen3:14b,qwen3-coder:480b-cloud,gpt-oss:120b-cloud"
 ```
 
 A model's provider is the spec's first segment (`m1pro/…` → `m1pro`), or `GADFLY_PROVIDER`/
-`ollama-cloud` for a bare id. Default (`cap 1`) keeps a single-provider pool fully sequential.
+`ollama-cloud` for a bare id. The budget is **shared across the provider's models**: with a
+budget of 3 and two cloud models, you get 3 lens passes in flight in any mix — as one model
+finishes a lens, the freed slot immediately goes to another model's next lens, so a model
+winding down to its last lens never stalls the others (the pre-2026-07 design capped *models*
+separately and did stall — that `GADFLY_PROVIDER_CONCURRENCY`/`GADFLY_CONCURRENCY` model cap is
+**gone**; those vars are now ignored). Default (budget `1`) keeps a provider fully sequential.
 
-**Lens fan-out (within a model).** By default the specialist lenses run **sequentially** inside
+> Under the hood the shared budget is a small cross-process permit pool (flock files, seeded per
-each model (`GADFLY_LENS_CONCURRENCY=1`). Raise it to overlap the independent per-lens
+> lane by `entrypoint.sh`); permits release automatically if a model process dies, so a crashed
-review+recheck passes — the model then posts its consolidated comment as soon as its lenses
+> lens can't leak budget.
-finish (so with sequential models, results stream in per model and per-model timings stay
-clean). Like the model cap, it's **per-provider configurable**: `GADFLY_PROVIDER_LENS_CONCURRENCY`
-takes a `provider=N` map keyed by the **same provider lanes** as `GADFLY_PROVIDER_CONCURRENCY`,
-falling back to the `GADFLY_LENS_CONCURRENCY` scalar (default `1`). **It multiplies with the
-model cap:** total in-flight requests ≈ *models-at-once × lenses-at-once*, so to fan lenses out
-without oversubscribing a backend, keep its model cap low and raise its lens cap:
-
-```yaml
-# Per provider: cloud runs one model at a time but fans its 3 lenses out (3 concurrent requests);
-# the slow local box stays fully serial. Both provider lanes still run in parallel.
-GADFLY_PROVIDER_CONCURRENCY:      "ollama-cloud=1,m1=1"
-GADFLY_PROVIDER_LENS_CONCURRENCY: "ollama-cloud=3,m1=1"
-GADFLY_SPECIALISTS: "security,correctness,error-handling"
-```
 
 ### Live status board
 
@@ -418,8 +438,7 @@ on its next review **without** a re-pin or a tag move:
 |---|---|
 | `GADFLY_DEFAULT_MODELS` | `GADFLY_MODELS` (csv) |
 | `GADFLY_DEFAULT_SPECIALISTS` | the lens suite |
-| `GADFLY_DEFAULT_PROVIDER_CONCURRENCY` | models-at-once per provider |
+| `GADFLY_DEFAULT_PROVIDER_LENS_CONCURRENCY` | the per-provider lens budget (lens passes in flight per provider, shared across its models) |
-| `GADFLY_DEFAULT_PROVIDER_LENS_CONCURRENCY` | lenses-at-once per provider |
 | `GADFLY_ENDPOINT_RAGNAROS` | a named endpoint, e.g. `llamaswap\|https://host` |
 
 Adding a *new* named endpoint still needs a one-line reusable edit (Gitea can't auto-expose arbitrary
@@ -441,10 +460,9 @@ The reviewer binary reads these (the stub/entrypoint set sane defaults):
 | `GADFLY_SELECTOR_MODEL` | review model | model that picks lenses in `auto` mode |
 | `GADFLY_WORKER_MODEL` | — | cheap model for `delegate_investigation`; unset = no delegation |
 | `GADFLY_WORKER_MAX_STEPS` | 8 | tool-step cap for a delegated worker run |
-| `GADFLY_CONCURRENCY` | 1 | default max models run at once **per provider** |
+| `GADFLY_LENS_CONCURRENCY` | 1 | **per-provider lens budget** — lens passes in flight per provider, shared across all its models (all a provider's models run at once; this is the only throttle) |
-| `GADFLY_PROVIDER_CONCURRENCY` | — | per-provider overrides, e.g. `ollama-cloud=3,m1pro=1` |
+| `GADFLY_PROVIDER_LENS_CONCURRENCY` | — | per-provider lens-budget overrides, a `provider=N` map, e.g. `ollama-cloud=3,m1=1` |
-| `GADFLY_LENS_CONCURRENCY` | 1 | specialist lenses run at once **within a model** (× model cap = total in-flight) |
+| `GADFLY_CONCURRENCY` / `GADFLY_PROVIDER_CONCURRENCY` | — | **removed** (was the per-provider models-at-once cap; now ignored — the lens budget is the single throttle) |
-| `GADFLY_PROVIDER_LENS_CONCURRENCY` | — | per-provider lens overrides, same lanes as `GADFLY_PROVIDER_CONCURRENCY`, e.g. `ollama-cloud=3,m1=1` |
 | `GADFLY_MAX_STEPS` | 24 | review-pass tool-step cap |
 | `GADFLY_TIMEOUT_SECS` | 300 | deadline **per specialist lens** (review+recheck) |
 | `GADFLY_RECHECK` | on | set `0`/`false` to skip the recheck pass |

cmd/gadfly/lens_concurrency_test.go

@@ -156,7 +156,7 @@ func TestRunSpecialists_PerProviderFanOut(t *testing.T) {
 
 // TestLensConcurrency covers the resolution matrix: scalar default, scalar
 // override, and per-provider override keyed by the model's resolved lane (same
-// lane rule entrypoint.sh uses for GADFLY_PROVIDER_CONCURRENCY).
+// lane rule entrypoint.sh uses for GADFLY_PROVIDER_LENS_CONCURRENCY).
 func TestLensConcurrency(t *testing.T) {
 	tests := []struct {
 		name     string

cmd/gadfly/lenssem.go

@@ -0,0 +1,115 @@
+package main
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"os"
+	"path/filepath"
+	"syscall"
+	"time"
+)
+
+// lensSem is the PROVIDER-WIDE lens-permit pool. Historically each model's binary
+// throttled its own lenses in-process (GADFLY_LENS_CONCURRENCY) while entrypoint.sh
+// separately capped how many MODELS ran at once — two multiplicative gates in
+// different processes. That let a model hold its whole slot until its last lens
+// finished, stalling the next model even with idle lens capacity.
+//
+// Instead, entrypoint.sh now runs all of a provider's models concurrently and
+// seeds ONE directory of N permit files per provider (N = the provider's lens
+// budget). Every model process in that lane draws from the same pool: a lens pass
+// (review + recheck) acquires a permit before it runs and releases it after, so a
+// model winding down immediately yields its freed permits to another model's
+// queued lenses. Permits are held with flock, which the kernel drops when the
+// holding process exits — so a killed/crashed model frees its permits for free.
+type lensSem struct {
+	dir  string
+	size int
+}
+
+// lensSemPollInterval is how often a blocked acquirer re-sweeps the permit files.
+// Lens passes run for many seconds to minutes, so a coarse poll adds negligible
+// latency while keeping the mechanism a few lines of stdlib (no IPC primitives).
+const lensSemPollInterval = 150 * time.Millisecond
+
+// activeLensSem returns the shared semaphore configured by entrypoint.sh, or nil
+// when it isn't set (standalone/local runs, tests, or an older entrypoint) — in
+// which case runSpecialists falls back to the in-process fanout limit alone, i.e.
+// the pre-existing per-model behavior.
+func activeLensSem() *lensSem {
+	dir := os.Getenv("GADFLY_LENS_SEM_DIR")
+	if dir == "" {
+		return nil
+	}
+	n := envInt("GADFLY_LENS_SEM_SIZE", 0)
+	if n < 1 {
+		// A dir was requested but the size is missing/blank/invalid. Rather than
+		// silently run unthrottled, say so on stderr — it's almost certainly a
+		// misconfiguration in entrypoint.sh (the two are set together).
+		fmt.Fprintf(os.Stderr, "gadfly: GADFLY_LENS_SEM_DIR set but GADFLY_LENS_SEM_SIZE=%q invalid; lenses run unthrottled\n", os.Getenv("GADFLY_LENS_SEM_SIZE"))
+		return nil
+	}
+	return &lensSem{dir: dir, size: n}
+}
+
+// acquire blocks until a permit is free (or ctx is done) and returns a release
+// func. It FAILS OPEN: if the pool directory is structurally unusable (missing,
+// unwritable, or on a filesystem without flock) it logs once and returns a no-op
+// release with a nil error, so the lens runs UNTHROTTLED rather than hanging the
+// whole review forever — this is an advisory reviewer, a slightly oversubscribed
+// backend beats a stuck one. Only a full-but-healthy pool actually blocks; on ctx
+// cancellation it returns a no-op release plus ctx.Err() so the caller can surface
+// the lens as "did not run" instead of leaking a permit.
+func (s *lensSem) acquire(ctx context.Context) (func(), error) {
+	for {
+		release, ok, err := s.tryAcquire()
+		if ok {
+			return release, nil
+		}
+		if err != nil {
+			fmt.Fprintf(os.Stderr, "gadfly: lens permit pool %q unusable (%v); proceeding without a permit\n", s.dir, err)
+			return func() {}, nil
+		}
+		timer := time.NewTimer(lensSemPollInterval)
+		select {
+		case <-ctx.Done():
+			timer.Stop() // don't leak the timer when we bail on cancellation
+			return func() {}, ctx.Err()
+		case <-timer.C:
+		}
+	}
+}
+
+// tryAcquire makes one non-blocking sweep over the permit files. It returns a
+// release func for the first one it locks; otherwise it distinguishes a healthy
+// FULL pool (some permit was openable but flock-busy → ok=false, err=nil → keep
+// polling) from a structurally BROKEN pool (no permit was even acquirable and none
+// was merely busy → err set → caller fails open). Permit files are created lazily
+// (entrypoint.sh only guarantees the directory exists). Closing the *os.File in
+// the returned func drops the flock (the lock lives on the open file description).
+func (s *lensSem) tryAcquire() (func(), bool, error) {
+	sawBusy := false
+	var structural error
+	for i := 0; i < s.size; i++ {
+		path := filepath.Join(s.dir, fmt.Sprintf("permit.%d", i))
+		f, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE, 0o600)
+		if err != nil {
+			structural = err // e.g. the pool dir is gone or unwritable
+			continue
+		}
+		if err := syscall.Flock(int(f.Fd()), syscall.LOCK_EX|syscall.LOCK_NB); err == nil {
+			return func() { f.Close() }, true, nil
+		} else if errors.Is(err, syscall.EWOULDBLOCK) {
+			sawBusy = true // this permit is held by someone else — pool has capacity
+			f.Close()
+		} else {
+			structural = err // flock unsupported / other → not a transient "busy"
+			f.Close()
+		}
+	}
+	if sawBusy {
+		return nil, false, nil // full but healthy: another lens will free a permit
+	}
+	return nil, false, structural
+}

cmd/gadfly/lenssem_test.go

@@ -0,0 +1,136 @@
+package main
+
+import (
+	"context"
+	"path/filepath"
+	"sync"
+	"sync/atomic"
+	"testing"
+	"time"
+)
+
+// flock permits are per open-file-description, so two separate opens of the same
+// permit file conflict even within one process — the exhaustion, blocking, and
+// max-in-flight tests below therefore exercise the same semantics a real
+// multi-process lane would see.
+
+func TestActiveLensSemUnset(t *testing.T) {
+	t.Setenv("GADFLY_LENS_SEM_DIR", "")
+	if s := activeLensSem(); s != nil {
+		t.Fatalf("expected nil sem when GADFLY_LENS_SEM_DIR unset, got %+v", s)
+	}
+	// A dir with a zero/blank size is inert too (must not divide the pool by 0).
+	t.Setenv("GADFLY_LENS_SEM_DIR", t.TempDir())
+	t.Setenv("GADFLY_LENS_SEM_SIZE", "0")
+	if s := activeLensSem(); s != nil {
+		t.Fatalf("expected nil sem when size < 1, got %+v", s)
+	}
+}
+
+func TestLensSemExhaustionAndRelease(t *testing.T) {
+	s := &lensSem{dir: t.TempDir(), size: 2}
+
+	r1, ok, err := s.tryAcquire()
+	if !ok || err != nil {
+		t.Fatalf("first acquire should succeed: ok=%v err=%v", ok, err)
+	}
+	r2, ok, err := s.tryAcquire()
+	if !ok || err != nil {
+		t.Fatalf("second acquire should succeed: ok=%v err=%v", ok, err)
+	}
+	// Pool full but healthy: ok=false with NO error (keep polling), not a
+	// structural failure.
+	if _, ok, err := s.tryAcquire(); ok || err != nil {
+		t.Fatalf("third acquire should be full-but-healthy: ok=%v err=%v", ok, err)
+	}
+
+	r1() // free one permit
+	r3, ok, err := s.tryAcquire()
+	if !ok || err != nil {
+		t.Fatalf("acquire should succeed after a release: ok=%v err=%v", ok, err)
+	}
+	r2()
+	r3()
+}
+
+// A structurally broken pool (dir missing/unwritable) must FAIL OPEN — tryAcquire
+// surfaces the error and acquire returns promptly with a no-op release and no
+// error, so a lens runs unthrottled instead of spinning forever.
+func TestLensSemBrokenPoolFailsOpen(t *testing.T) {
+	s := &lensSem{dir: filepath.Join(t.TempDir(), "does", "not", "exist"), size: 2}
+
+	if _, ok, err := s.tryAcquire(); ok || err == nil {
+		t.Fatalf("tryAcquire on a broken pool should report a structural error: ok=%v err=%v", ok, err)
+	}
+
+	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
+	defer cancel()
+	start := time.Now()
+	release, err := s.acquire(ctx)
+	if err != nil {
+		t.Fatalf("acquire should fail open (nil err), got %v", err)
+	}
+	if waited := time.Since(start); waited > time.Second {
+		t.Fatalf("acquire on a broken pool should return promptly, waited %v", waited)
+	}
+	release() // must be a safe no-op
+}
+
+func TestLensSemAcquireBlocksThenCancels(t *testing.T) {
+	s := &lensSem{dir: t.TempDir(), size: 1}
+
+	// Immediate success while a permit is free.
+	release, err := s.acquire(context.Background())
+	if err != nil {
+		t.Fatalf("acquire on a free pool: %v", err)
+	}
+
+	// With the only permit held, acquire must block until ctx expires.
+	ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
+	defer cancel()
+	start := time.Now()
+	if _, err := s.acquire(ctx); err == nil {
+		t.Fatal("acquire on an exhausted pool should return ctx error, not a permit")
+	}
+	if waited := time.Since(start); waited < 50*time.Millisecond {
+		t.Fatalf("acquire returned too fast (%v); it should have blocked on the full pool", waited)
+	}
+	release()
+}
+
+func TestLensSemNeverExceedsSize(t *testing.T) {
+	const size = 3
+	s := &lensSem{dir: t.TempDir(), size: size}
+
+	var inFlight, peak int64
+	var wg sync.WaitGroup
+	for i := 0; i < 12; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			release, err := s.acquire(context.Background())
+			if err != nil {
+				t.Errorf("acquire: %v", err)
+				return
+			}
+			defer release()
+			n := atomic.AddInt64(&inFlight, 1)
+			for {
+				p := atomic.LoadInt64(&peak)
+				if n <= p || atomic.CompareAndSwapInt64(&peak, p, n) {
+					break
+				}
+			}
+			time.Sleep(20 * time.Millisecond)
+			atomic.AddInt64(&inFlight, -1)
+		}()
+	}
+	wg.Wait()
+
+	if peak > size {
+		t.Fatalf("max in-flight lens permits = %d, exceeds budget %d", peak, size)
+	}
+	if peak == 0 {
+		t.Fatal("no permits were ever acquired")
+	}
+}

cmd/gadfly/main.go

@@ -49,15 +49,18 @@
 //	GADFLY_RECHECK        set to 0/false to skip the recheck pass (optional, default on).
 //	GADFLY_RECHECK_MAX_STEPS recheck-pass step cap (optional, default 16).
 //	GADFLY_TIMEOUT_SECS   overall deadline in seconds, shared by both passes (optional, default 300).
-//	GADFLY_LENS_CONCURRENCY how many specialist lenses run concurrently within this
+//	GADFLY_LENS_CONCURRENCY how many specialist lenses run concurrently (optional,
-//	                        model (optional, default 1 = sequential). Total in-flight
+//	                        default 1 = sequential). Under entrypoint.sh this is the
-//	                        model requests ≈ this × entrypoint.sh's per-provider model
+//	                        PROVIDER-WIDE lens budget, shared across all of that
-//	                        concurrency, so keep the product within the backend's budget.
+//	                        provider's models via a permit pool (GADFLY_LENS_SEM_DIR),
+//	                        so it bounds total lens passes in flight per provider.
 //	GADFLY_PROVIDER_LENS_CONCURRENCY per-provider override for the above, as a
-//	                        "provider=N,provider=N" map keyed by the SAME provider
+//	                        "provider=N,provider=N" map keyed by the provider lanes
-//	                        lanes as GADFLY_PROVIDER_CONCURRENCY (e.g.
+//	                        (e.g. "ollama-cloud=3,m1=1"). Wins over
-//	                        "ollama-cloud=3,m1=1"). Wins over GADFLY_LENS_CONCURRENCY
+//	                        GADFLY_LENS_CONCURRENCY for the model's provider.
-//	                        for the model's provider; falls back to it otherwise.
+//	GADFLY_LENS_SEM_DIR / GADFLY_LENS_SEM_SIZE  set by entrypoint.sh: the shared
+//	                        cross-process lens-permit pool (dir of N flock files)
+//	                        and its size. Unset => in-process lensConcurrency only.
 //	GADFLY_MAX_DIFF_CHARS diff chars embedded in the review prompt (optional, default 60000;
 //	                        the full diff is reachable via the paginated get_diff tool).
 //
@@ -94,9 +97,11 @@ const (
 	// calls and then hard-failing with "max steps reached without a final
 	// answer" — it always has a few steps left to wrap up.
 	defaultWrapUpReserve = 4
-	// defaultLensConcurrency is how many specialist lenses run at once within a
+	// defaultLensConcurrency is the fallback lens budget when neither
-	// single model. 1 keeps the suite sequential (the historical behavior);
+	// GADFLY_PROVIDER_LENS_CONCURRENCY nor GADFLY_LENS_CONCURRENCY is set. 1 keeps
-	// higher values overlap the independent per-lens passes. See runSpecialists.
+	// the suite sequential (the historical behavior); higher values overlap the
+	// independent per-lens passes. Under entrypoint.sh the resolved value is the
+	// provider-wide budget shared across the provider's models. See runSpecialists.
 	defaultLensConcurrency = 1
 )
 
@@ -242,25 +247,34 @@ func run() error {
 
 // runSpecialists reviews the diff through each lens and returns the results in
 // the SAME order as specialists, regardless of finish order. It uses executus's
-// fanout primitive: up to GADFLY_LENS_CONCURRENCY lenses run concurrently (the
+// fanout primitive to overlap independent lens passes (fanout.Run returns one
-// default of 1 keeps the suite sequential, exactly as before), and fanout.Run
+// result per lens in input order); each lens runs under its own per-lens timeout
-// returns one result per lens in input order. Each lens already runs under its
+// (reviewWithSpecialist) and the lenses only read the immutable repoFS.
-// own per-lens timeout (reviewWithSpecialist) and the lenses only read the
-// immutable repoFS, so concurrency simply overlaps independent passes.
 //
-// Caution: this fans out WITHIN one model. It multiplies with entrypoint.sh's
+// Throttling: when entrypoint.sh runs several of a provider's models at once it
-// per-provider model concurrency, so total concurrent backend requests ≈
+// seeds a shared lens-permit pool (activeLensSem) that every model's lenses draw
-// (models at once) × (lenses at once). To fan lenses out without oversubscribing
+// from, so the real cap is total lens passes in flight per PROVIDER — not
-// the backend, run models one at a time (provider lane cap 1) and raise this.
+// (models at once) × (lenses at once). Absent that pool (local runs, tests) the
+// in-process GADFLY_LENS_CONCURRENCY limit applies alone (default 1 = sequential).
 func runSpecialists(eng reviewEngine, base string, specialists []Specialist, task, diff string) []specialistResult {
 	// Optional live status board: publishes this model's per-lens progress to a
 	// file the entrypoint board renders. Inert (no-op) unless GADFLY_STATUS_FILE
 	// is set, so plain runs are unaffected.
 	sw := newStatusWriter(os.Getenv("GADFLY_MODEL"), modelProvider(), specialists)
 
+	// The cross-process pool (if any) is the real ceiling; size the in-process
+	// fanout to it so a lone model in its lane can use the whole provider budget,
+	// while extra goroutines simply block in sem.acquire until a permit frees.
+	// Absent a pool, fall back to the in-process lens limit.
+	sem := activeLensSem()
+	maxConcurrent := lensConcurrency()
+	if sem != nil {
+		maxConcurrent = sem.size // the shared pool is the real ceiling
+	}
+
 	fanResults := fanout.Run(context.Background(), specialists, fanout.Options[Specialist]{
-		MaxConcurrent: lensConcurrency(),
+		MaxConcurrent: maxConcurrent,
-	}, func(_ context.Context, sp Specialist) (res specialistResult, _ error) {
+	}, func(ctx context.Context, sp Specialist) (res specialistResult, _ error) {
 		// A panic in one lens must not crash the whole binary (which would kill
 		// every other lens's output) or leave this lens stuck at "running" on the
 		// status board. fanout does not recover fn panics, so we do it here:
@@ -271,6 +285,17 @@ func runSpecialists(eng reviewEngine, base string, specialists []Specialist, tas
 				sw.set(sp.Name, lensFinished, "", true)
 			}
 		}()
+		// Hold a provider-wide permit for this lens's whole review+recheck pass.
+		// While waiting the lens stays "queued" on the board; cancellation before
+		// a permit frees surfaces as a did-not-run lens rather than a leaked slot.
+		if sem != nil {
+			release, err := sem.acquire(ctx)
+			if err != nil {
+				sw.set(sp.Name, lensFinished, "", true)
+				return specialistResult{spec: sp, out: fmt.Sprintf("⚠️ This reviewer did not run: %v", err), verdict: verdictUnknown, errored: true}, nil
+			}
+			defer release()
+		}
 		sw.set(sp.Name, lensRunning, "", false)
 		out, errored := reviewWithSpecialist(eng, base, sp, task, diff)
 		v := parseVerdict(out)
@@ -293,14 +318,13 @@ func runSpecialists(eng reviewEngine, base string, specialists []Specialist, tas
 	return results
 }
 
-// lensConcurrency resolves how many specialist lenses run at once for THIS run's
+// lensConcurrency resolves the lens budget for THIS run's provider: a per-provider
-// model. It mirrors entrypoint.sh's per-provider MODEL concurrency: a
+// override in GADFLY_PROVIDER_LENS_CONCURRENCY ("provider=N,...") wins for the
-// per-provider override in GADFLY_PROVIDER_LENS_CONCURRENCY ("provider=N,...")
+// model's provider (resolved by modelProvider()), otherwise the
-// wins for the model's provider, otherwise the GADFLY_LENS_CONCURRENCY scalar
+// GADFLY_LENS_CONCURRENCY scalar (default 1). Under entrypoint.sh the SAME value
-// (default 1). The provider is resolved by modelProvider() — the SAME lane rule
+// seeds the shared cross-process permit pool (activeLensSem), so it is the
-// entrypoint uses for GADFLY_PROVIDER_CONCURRENCY — so e.g.
+// provider-wide budget rather than a per-model one; standalone it caps the single
-// "ollama-cloud=3,m1=1" fans cloud lenses out while keeping a slow local box
+// model's in-process fanout.
-// serial, exactly the way the model map does for whole models.
 func lensConcurrency() int {
 	if n, ok := providerOverride("GADFLY_PROVIDER_LENS_CONCURRENCY", modelProvider()); ok {
 		return n
@@ -310,7 +334,7 @@ func lensConcurrency() int {
 
 // providerOverride parses a "provider=N,provider=N" env map and returns the
 // value for provider when present and valid (>0). Mirrors entrypoint.sh's
-// provider_cap lookup so the two concurrency maps share one syntax.
+// provider_lens_cap lookup so the two share one syntax.
 func providerOverride(envName, provider string) (int, bool) {
 	for _, item := range strings.Split(os.Getenv(envName), ",") {
 		k, v, ok := strings.Cut(item, "=")

cmd/gadfly/model.go

@@ -3,6 +3,7 @@ package main
 import (
 	"fmt"
 	"os"
+	"slices"
 	"strings"
 
 	"gitea.stevedudenhoeffer.com/steve/majordomo"
@@ -19,6 +20,35 @@ 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"}
+
+func isOpenAICompatProvider(name string) bool {
+	return slices.Contains(openAICompatProviders, name)
+}
+
+// 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 +63,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
+//	GADFLY_API_KEY   bearer/API key for the chosen provider, used ONLY on the
-//	                 unset the provider falls back to its standard env var
+//	                 GADFLY_BASE_URL override path. With no base URL the
-//	                 (OLLAMA_API_KEY / OPENAI_API_KEY / ANTHROPIC_API_KEY /
+//	                 provider reads its own standard variable and this is never
-//	                 GOOGLE_API_KEY|GEMINI_API_KEY). Local Ollama needs none.
+//	                 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 +99,21 @@ func resolveModel() (llm.Model, error) {
 	}
 
 	// Endpoint override: construct the provider directly at the given URL.
-	switch provider {
+	// The openai-compat family is matched by the shared predicate, not a
-	case "openai", "openai-compatible":
+	// 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) {
 		opts := []openai.Option{openai.WithBaseURL(baseURL)}
 		if apiKey != "" {
 			opts = append(opts, openai.WithAPIKey(apiKey))
 		}
 		return openai.New(opts...).Model(model)
+	}
+
+	switch provider {
 	case "ollama", "ollama-cloud":
 		opts := []ollama.Option{ollama.WithBaseURL(baseURL)}
 		if apiKey != "" {
@@ -108,7 +148,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)
 	}
 }
 
@@ -162,8 +202,8 @@ func buildSpec(provider, model string) string {
 // entrypoint.sh's provider_of: the segment before the first "/" in GADFLY_MODEL,
 // else GADFLY_PROVIDER, else the default (ollama-cloud). The binary reviews one
 // model per invocation, so this is that model's provider — used to resolve
-// per-provider policy (e.g. lens concurrency) against the SAME provider keys
+// per-provider policy (e.g. the lens budget) against the SAME provider keys
-// entrypoint uses for GADFLY_PROVIDER_CONCURRENCY.
+// entrypoint uses for GADFLY_PROVIDER_LENS_CONCURRENCY.
 func modelProvider() string {
 	model := strings.TrimSpace(os.Getenv("GADFLY_MODEL"))
 	if pfx, _, ok := strings.Cut(model, "/"); ok {
@@ -188,8 +228,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"
+//	    provider is ollama/openai/anthropic/google/foreman/llama-swap(s) or an
-//	    targets a foreman daemon (native Ollama on the wire):
+//	    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 +282,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) {
+		opts := []openai.Option{openai.WithName(name), openai.WithBaseURL(baseURL)}
+		if key != "" {
+			opts = append(opts, openai.WithAPIKey(key))
+		}
+		return openai.New(opts...), nil
+	}
+
 	switch provider {
 	case "ollama", "ollama-cloud":
 		opts := []ollama.Option{ollama.WithName(name), ollama.WithBaseURL(baseURL)}
@@ -258,12 +310,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 +323,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,9 @@
 package main
 
-import "testing"
+import (
+	"strings"
+	"testing"
+)
 
 func TestEndpointProvider(t *testing.T) {
 	t.Run("ollama http endpoint registers under its name", func(t *testing.T) {
@@ -68,6 +71,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

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.
@@ -198,14 +206,18 @@ export GADFLY_FINDINGS_TOKEN="${GADFLY_FINDINGS_TOKEN:-}"
 # provider key envs (OPENAI_API_KEY, …) are inherited by run.sh and the binary.
 #
 # Concurrency: each PROVIDER is its own lane and lanes run in PARALLEL, so a fast
-# cloud provider isn't stuck behind a slow local box. Within a lane, at most
+# cloud provider isn't stuck behind a slow local box. Within a lane ALL of the
-# `cap` models run at once. cap = GADFLY_PROVIDER_CONCURRENCY's "provider=N"
+# provider's models run at once; the real throttle is a single PROVIDER-WIDE lens
-# entry, else GADFLY_CONCURRENCY (default 1). A model's provider is the spec's
+# budget (a shared permit pool, seeded per lane) that every model's lenses draw
-# first path segment ("m1pro/qwen3.6:35b-mlx" -> m1pro), or GADFLY_PROVIDER /
+# from — so total lens passes in flight per provider is bounded, but a model
-# ollama-cloud for a bare id. Default (cap 1) keeps a single-provider pool fully
+# winding down to its last lens immediately yields its freed permits to another
-# sequential, exactly as before.
+# model's queued lenses instead of holding a whole "model slot" (the old
+# GADFLY_PROVIDER_CONCURRENCY model cap, now removed, caused that tail stall). The
+# budget = GADFLY_PROVIDER_LENS_CONCURRENCY's "provider=N" entry, else
+# GADFLY_LENS_CONCURRENCY (default 1). A model's provider is the spec's first path
+# segment ("m1pro/qwen3.6:35b-mlx" -> m1pro), or GADFLY_PROVIDER / ollama-cloud
+# for a bare id.
 MODELS="${GADFLY_MODELS:-${OLLAMA_REVIEW_MODELS:-$DEFAULT_MODELS}}"
-DEFAULT_CONC="${GADFLY_CONCURRENCY:-1}"
 
 # --- huge-PR downshift ------------------------------------------------------
 # A very large diff is what burns the model budget: every review step re-sends
@@ -245,24 +257,33 @@ provider_of() { case "$1" in */*) echo "${1%%/*}";; *) echo "${GADFLY_PROVIDER:-
 STATUS_DIR="${WORKDIR}/status"
 status_file_for() { echo "${STATUS_DIR}/$(echo "$1" | tr -c '[:alnum:]._-' '_').json"; }
 
-provider_cap() { # provider -> concurrency (override map "p=N,...", else default)
+# Root of the per-provider lens permit pools (one subdir per lane, seeded by
+# run_lane). Cleared up front so a reused WORKDIR can't leak stale permit files.
+LENS_SEM_ROOT="${WORKDIR}/lenssem"
+rm -rf "$LENS_SEM_ROOT" 2>/dev/null || true
+
+provider_lens_cap() { # provider -> provider-wide lens budget (permit-pool size)
   local p="$1" item k v
-  IFS=',' read -ra _caps <<< "${GADFLY_PROVIDER_CONCURRENCY:-}"
+  IFS=',' read -ra _lcaps <<< "${GADFLY_PROVIDER_LENS_CONCURRENCY:-}"
-  for item in "${_caps[@]}"; do
+  for item in "${_lcaps[@]}"; do
     k="$(echo "${item%%=*}" | tr -d '[:space:]')"
     v="$(echo "${item#*=}" | tr -d '[:space:]')"
     if [ "$k" = "$p" ] && [ -n "$v" ]; then echo "$v"; return; fi
   done
-  echo "$DEFAULT_CONC"
+  echo "${GADFLY_LENS_CONCURRENCY:-1}"
 }
 
 review_one() {
-  local sf="" ff=""
+  local m="$1" sem_dir="${2:-}" sem_size="${3:-}" sf="" ff=""
-  [ "${GADFLY_STATUS_BOARD:-1}" != "0" ] && sf="$(status_file_for "$1")"
+  [ "${GADFLY_STATUS_BOARD:-1}" != "0" ] && sf="$(status_file_for "$m")"
-  [ "$CONSOLIDATE" = "1" ] && ff="$(findings_file_for "$1")"
+  [ "$CONSOLIDATE" = "1" ] && ff="$(findings_file_for "$m")"
-  PROVIDER=ollama MODEL="$1" GADFLY_BIN="/usr/local/bin/gadfly" GADFLY_REPO_DIR="$REPO_DIR" \
+  # GADFLY_LENS_SEM_DIR/_SIZE point the binary at this provider's shared lens
+  # permit pool (empty => the binary just uses its in-process lens limit). These
+  # are inherited by the binary through run.sh's environment.
+  PROVIDER=ollama MODEL="$m" GADFLY_BIN="/usr/local/bin/gadfly" GADFLY_REPO_DIR="$REPO_DIR" \
     GADFLY_STATUS_FILE="$sf" GADFLY_FINDINGS_OUT="$ff" GADFLY_CONSOLIDATE="$CONSOLIDATE" \
-    bash "${SCRIPTS_DIR}/run.sh" || log "model $1 failed (continuing)"
+    GADFLY_LENS_SEM_DIR="$sem_dir" GADFLY_LENS_SEM_SIZE="$sem_size" \
+    bash "${SCRIPTS_DIR}/run.sh" || log "model $m failed (continuing)"
   # If the binary never wrote real status (run.sh skipped it: empty diff, no key,
   # binary missing), the pre-seed stays {started:0, done:false} and the board
   # would show this model "waiting to start" forever and never reach N/N. Mark
@@ -313,16 +334,19 @@ for m in "${MODEL_LIST[@]}"; do
   case " $PROVIDERS " in *" $p "*) ;; *) PROVIDERS="${PROVIDERS}${PROVIDERS:+ }$p" ;; esac
 done
 
-run_lane() { # $1=provider: run its models, at most `cap` at a time
+run_lane() { # $1=provider: run ALL its models at once, throttled only by a shared
-  local p="$1" cap inflight=0 m
+             # provider-wide lens permit pool (no per-model cap).
-  cap="$(provider_cap "$p")"; [ "$cap" -ge 1 ] 2>/dev/null || cap=1
+  local p="$1" budget sem_dir m
+  budget="$(provider_lens_cap "$p")"; [ "$budget" -ge 1 ] 2>/dev/null || budget=1
   local mine=()
   for m in "${MODEL_LIST[@]}"; do [ "$(provider_of "$m")" = "$p" ] && mine+=("$m"); done
-  log "lane ${p}: cap ${cap}; models: ${mine[*]}"
+  # Seed this provider's lens permit pool: a directory the binary flocks N permit
+  # files in (created lazily), one shared budget across every model in the lane.
+  sem_dir="${LENS_SEM_ROOT}/$(echo "$p" | tr -c '[:alnum:]._-' '_')"
+  mkdir -p "$sem_dir"
+  log "lane ${p}: lens budget ${budget} shared across ${#mine[@]} model(s): ${mine[*]}"
   for m in "${mine[@]}"; do
-    review_one "$m" &
+    review_one "$m" "$sem_dir" "$budget" &
-    inflight=$((inflight+1))
-    if [ "$inflight" -ge "$cap" ]; then wait -n 2>/dev/null || wait; inflight=$((inflight-1)); fi
   done
   wait
 }
@@ -338,8 +362,8 @@ BOARD_PID=""
 if [ "${GADFLY_STATUS_BOARD:-1}" != "0" ]; then
   rm -rf "$STATUS_DIR"; mkdir -p "$STATUS_DIR"
   # Pre-seed every model as queued so the board shows the full swarm from t=0,
-  # even models still waiting on their provider lane's concurrency cap. Each
+  # even models whose lenses are still waiting on their provider's lens budget.
-  # binary overwrites its own file with real per-lens detail once it starts.
+  # Each binary overwrites its own file with real per-lens detail once it starts.
   for m in "${MODEL_LIST[@]}"; do
     jq -n --arg model "$m" --arg provider "$(provider_of "$m")" \
       '{model:$model, provider:$provider, started:0, updated:0, done:false, lenses:[]}' \
@@ -378,9 +402,9 @@ if [ "${GADFLY_PR_BUDGET_SECS:-0}" -gt 0 ] 2>/dev/null; then
 fi
 
 log "providers: ${PROVIDERS:-none}"
-# Each provider lane runs in parallel; cap is enforced within each lane. Track
+# Each provider lane runs in parallel; the shared lens budget throttles within
-# the lane PIDs so we wait ONLY for the review work — not the status board,
+# each lane. Track the lane PIDs so we wait ONLY for the review work — not the
-# which intentionally runs until we signal it below.
+# status board, which intentionally runs until we signal it below.
 LANE_PIDS=()
 for p in $PROVIDERS; do
   run_lane "$p" &

examples/adversarial-review.yml

@@ -55,14 +55,14 @@ jobs:
           # csv to choose; "all" for everything; or define custom ones via a repo
           # .gadfly.yml / GADFLY_SPECIALIST_<NAME>. See README "Specialists".
           GADFLY_SPECIALISTS: ${{ vars.GADFLY_SPECIALISTS }}
-          # Lens fan-out (optional; default 1 = lenses run sequentially within a
+          # Concurrency (optional; default 1 = fully sequential per provider). The
-          # model). Raise it to run a model's lenses concurrently so each model
+          # ONE throttle is a per-provider LENS BUDGET: the max lens passes (a lens =
-          # posts its comment sooner. Total in-flight requests = (models at once)
+          # one specialist's review+recheck) in flight at once for a provider, shared
-          # × (lenses at once), so to fan out without oversubscribing a backend,
+          # across ALL that provider's models — every model in a lane runs at once and
-          # keep its model cap low and raise its lens cap. Per-provider configurable
+          # its lenses draw from the shared budget. Raise it to overlap lenses; set it
-          # via GADFLY_PROVIDER_LENS_CONCURRENCY (same lanes as the model map):
+          # per provider with GADFLY_PROVIDER_LENS_CONCURRENCY:
-          #   GADFLY_PROVIDER_CONCURRENCY:      "ollama-cloud=1,m1=1"
           #   GADFLY_PROVIDER_LENS_CONCURRENCY: "ollama-cloud=3,m1=1"
+          # (The old GADFLY_PROVIDER_CONCURRENCY model cap was removed and is ignored.)
           # GADFLY_LENS_CONCURRENCY: ${{ vars.GADFLY_LENS_CONCURRENCY }}
           # GADFLY_PROVIDER_LENS_CONCURRENCY: ${{ vars.GADFLY_PROVIDER_LENS_CONCURRENCY }}
           # Live status board (optional; ON by default): one consolidated comment

scripts/preflight.sh

@@ -0,0 +1,71 @@
+#!/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" key_env="" key_hint=""
+
+  # Only the registry path has knowable credential rules — see above.
+  if [ -n "${GADFLY_BASE_URL:-}" ]; then
+    echo ""
+    return 0
+  fi
+
+  # A provider is absent from this table for one of TWO different reasons — do
+  # not assume the first one and add an arm:
+  #   1. It needs no key, or carries it 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. Adding
+  #      `google) key_env="GOOGLE_API_KEY"` would silently skip every reviewer
+  #      configured with GEMINI_API_KEY. Pre-flighting google needs an
+  #      either-variable check, not this table's one-name shape.
+  # ollama-cloud is checked on OLLAMA_API_KEY but hinted as OLLAMA_CLOUD_API_KEY:
+  # run.sh copies the consumer-facing OLLAMA_CLOUD_API_KEY secret into the
+  # OLLAMA_API_KEY the provider reads, BEFORE calling this. The hint names the
+  # variable the operator actually sets; the check reads the one the code uses.
+  # 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" ;;
+  esac
+
+  if [ -z "$key_env" ]; then
+    echo ""                       # provider needs no pre-flight
+    return 0
+  fi
+  # 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"
+}

scripts/preflight_test.sh

@@ -0,0 +1,86 @@
+#!/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"; shift
+  env -i PATH="$PATH" HOME="$HOME" "$@" bash -c "
+    set -u
+    . '$SCRIPT_DIR/preflight.sh'
+    gadfly_preflight_key '$provider'
+  "
+}
+
+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 (GADFLY_BASE_URL set) is deliberately not pre-flighted =="
+# 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.
+check "qwen + BASE_URL, no keys"    "" "$(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 "openai + BASE_URL, no keys"  "" "$(probe openai 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)"
+
+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,10 @@ case "$PROVIDER" in
     fi
     GADFLY_PROVIDER_EFF="$MODEL_PROVIDER"
 
-    # Only the default cloud provider strictly needs a key up front; local Ollama
+    # Credential pre-flight — one definition, shared with preflight_test.sh.
-    # and other providers either need none or read their own standard env var.
+    MISSING_KEY="$(gadfly_preflight_key "$GADFLY_PROVIDER_EFF")"
-    if [ "$GADFLY_PROVIDER_EFF" = "ollama-cloud" ] && [ -z "${OLLAMA_API_KEY:-}" ] && [ -z "${GADFLY_API_KEY:-}" ]; then
+    if [ -n "$MISSING_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."
+      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