15 Commits
Author SHA1 Message Date
steveandClaude Opus 5 8adeeeabe0 fix: drop the accidentally committed worktree gitlink, ignore .claude/worktrees
Build & push image / build-and-push (push) Successful in 6s
Build & push image / test (push) Successful in 9m42s
The pin bump used `git add -A` from a checkout that had a git worktree under
.claude/worktrees/, so the previous commit recorded it as a gitlink — a
submodule pointer to a commit no clone can resolve. Removed, and .gitignore now
covers the directory so it cannot recur.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
2026-08-12 18:57:53 -04:00
steveandClaude Opus 5 88f44cf141 chore(reusable): bump the fallback reviewer pin to sha-b850e35 [skip ci]
PR #30 (qwen/kimi built-ins) is merged and the image is published, so the
last-resort pin points at it. Verified the tag exists in the registry before
pointing consumers at it — a pin to a tag that was never pushed breaks every
consumer at once, and the build succeeding is not by itself proof the tag
landed.

GADFLY_REVIEWER_TAG still overrides this, and an explicit reviewer_tag input
overrides both.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
2026-08-12 18:57:34 -04:00
steve b850e35b35 Merge pull request 'feat(qwen): let Qwen (and Kimi) join the swarm' (#30)
Build & push image / build-and-push (push) Successful in 39s
Build & push image / test (push) Successful in 9m42s
2026-08-12 22:46:04 +00:00
steveandClaude Opus 5 d8b023efd6 fix(qwen): the own-key fallback made the override path checkable
Build & push image / build-and-push (pull_request) Successful in 4s
Build & push image / test (pull_request) Successful in 9m35s
Round 9's best finding is that my own round-8 change falsified a rationale I
wrote in round 5. The pre-flight skips the endpoint-override path because "a
built-in's own variable is never consulted there" — then I gave kimi/qwen an
own-key fallback that consults exactly that variable on exactly that path. So a
keyless override config sailed past the check and failed as a 401, which is the
failure the check exists to replace.

Now that the rule is statable for those two providers, they are checked on both
paths ("own key or GADFLY_API_KEY"), while everything else stays silent on the
override path because its rules still are not.

The missing-key hint on the GADFLY_ENDPOINT_* path named the endpoint variable
— telling a keyless operator to put a credential in a Gitea var, which is not
masked, and contradicting the README warning added one round earlier. It now
always names the provider's own masked secret.

Also: the model argument is trimmed, since Go trims GADFLY_MODEL and padding
would otherwise slip past the claude-code exemption; the test job takes
`permissions: contents: read`, being the one job that executes PR-authored
code; and the ollama-cloud rationale is stated once.

Deliberately not taken, with reasons rather than silence: the credential-scrub
bash could be extracted to a testable script like preflight.sh was — fair, and
a follow-up, since moving it now would be a fresh untested surface at merge
time. `tr -d [:space:]` strips POSIX whitespace where Go strips Unicode, which
differs only for a GADFLY_BASE_URL made entirely of non-ASCII spaces. And the
two provider tests overlap but assert different contracts that should be able
to fail independently.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
2026-08-12 18:36:12 -04:00
steveandClaude Opus 5 e67f95d777 fix(qwen): un-exempt opencode, and keep the Qwen key in a secret
Build & push image / build-and-push (pull_request) Successful in 4s
Build & push image / test (pull_request) Successful in 9m40s
Two findings this round contradicted each other — one asked me to extend the
engine-spec exemption to a bare "opencode", the other said opencode should not
be exempt at all. The code settles it: that engine drives an ollama-cloud model
through the bundled CLI and authenticates with OLLAMA_API_KEY, so it needs
exactly the key the pre-flight checks. Exempting it, which I did last round,
switched the check off for the one engine it could still help. Only claude-code
is exempt now — it carries CLAUDE_CODE_OAUTH_TOKEN and needs no Ollama key —
and opencode/open-code get table rows so both spellings are covered.

The README told operators to embed the Qwen key in a GADFLY_ENDPOINT_* var,
while the workflow that forwards those vars warns in its own comments that vars
are NOT masked. Rather than only rewording the docs, a keyless kimi/qwen
endpoint now falls back to its own QWEN_API_KEY / KIMI_API_KEY — the same
vendor's key, so the no-cross-vendor rule is untouched — which lets the URL live
in a var and the credential in a secret. Break-checked by pointing that fallback
at OPENAI_API_KEY: the leak test catches it.

Smaller: isBuiltinCompatProvider mirrors isOpenAICompatProvider instead of an
inline slices.Contains, with a test that every builtin is also in the compat
list (a builtin missing from it would never reach the branch that protects it);
the preflight.sh rationale is stated once rather than in two comment blocks;
the Go test locates the shell script relative to its own source file; and the
gofmt step takes GOPROXY=off like its neighbours.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
2026-08-12 18:23:33 -04:00
steveandClaude Opus 5 3af0f09387 fix(qwen): one credential rule for both paths — seven findings said so
Build & push image / build-and-push (pull_request) Successful in 5s
Build & push image / test (pull_request) Successful in 9m40s
Fourteen findings, and seven of them from all four models are the same one:
endpointProvider was missing the no-cross-vendor-fallback guard I had just
added to resolveModel. I fixed a credential leak on one path and left its
sibling leaking, in the commit whose own message argued those two paths must
move together. That is the third time in this PR.

So it is no longer a rule written twice. openAICompatOptions owns it and both
paths call it; builtinCompatProviders names the vendors that must never inherit
OPENAI_API_KEY, replacing a `provider == "kimi" || provider == "qwen"` literal
that was a fourth uncounted copy of the list.

The test drives a real request at a local server and demands two things: that
no request arrives carrying the OpenAI key, AND that the call fails closed
naming the variable to set — the second half because my first draft pointed the
provider at vendor.example, so the server saw nothing and the assertion held
for a reason unrelated to the fix. Break-checked: removing the guard puts
"Bearer sk-openai-must-not-travel" on the wire to the other vendor.

The scrub check failed open. As a bare condition, a grep ERROR (exit >= 2)
reads as "not found" and skips the guard — a credential check that passes
precisely when it cannot see the filesystem it is searching. It now
distinguishes 0/1/>=2 and refuses to continue on error.

A bare "claude-code" spec has no "/", so the provider fell back to ollama-cloud
and the pre-flight would skip a reviewer that authenticates with
CLAUDE_CODE_OAUTH_TOKEN and needs no Ollama key. Engine specs are now exempt.

preflight.sh's provider list duplicated its own case arms; both now read one
table. And its comment claimed the Go cross-check fails if either list misses
an entry from the other, when only one direction is checked — the reverse is
not even desirable, since ollama-cloud and anthropic belong in that table and
not in the Go one. The comment now says what is enforced.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
2026-08-12 18:09:18 -04:00
steveandClaude Opus 5 274451e89c fix(ci): actually add grep -e (a3d3a45 claimed it and did not)
Build & push image / build-and-push (pull_request) Successful in 4s
Build & push image / test (pull_request) Successful in 9m38s
The previous commit message listed this fix; the edit never made it into the
tree. Checked because I have done exactly that before, and the message is not
the change.

Verified rather than assumed, and the behaviour is worth recording: with a
password beginning with "-", `grep -rqF "$PW"` returns 0 against a directory
containing no such string, so the scrub check would have reported a leak that
did not exist and failed the step with a misleading message. With -e it
correctly returns 1 when clean and 0 when the credential really is present.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
2026-08-12 17:52:57 -04:00
steveandClaude Opus 5 a3d3a45e7e fix(qwen): bump majordomo, and stop handing keys to the wrong vendor
Build & push image / build-and-push (pull_request) Successful in 9s
Build & push image / test (pull_request) Canceled after 59s
Round 6, and one finding exposed something no reviewer mentioned: the majordomo
bump this whole PR depends on was never made. Every test here builds the openai
client directly, so all of them passed against a majordomo release that had
never heard of qwen — a plain "qwen/<model>" in GADFLY_MODELS, the primary way
anyone will use this, would not have resolved at all. A compile error caught it,
which is luck. TestBuiltinCompatProvidersResolveViaRegistry now exercises that
path; the build is what guards the dep itself, since the old release cannot
compile the code below.

On the endpoint-override path, kimi and qwen fell through to openai.New's
OPENAI_API_KEY default whenever GADFLY_API_KEY was unset — sending an OpenAI
key to Moonshot or Alibaba. That is a credential handed to the wrong vendor,
and it is the exact failure majordomo's built-ins are written to prevent; I
reintroduced it one layer up. Both now pass the key unconditionally, so an
absent key is a 401 naming GADFLY_API_KEY rather than a foreign credential on
the wire.

The test job scrubbed the registry credential and left the checkout token in
.git/config, readable by the `go test` it then runs — fixing one credential
while its neighbour sat in the open. persist-credentials: false; nothing in
that job talks to git after checkout.

The cross-language wiring test now QUERIES preflight.sh via a new
gadfly_preflight_providers function instead of regexing its case statement.
Parsing made that file's formatting a contract no linter enforces, where a
harmless reformat breaks a test in another language. Two models flagged it.

Also: grep for the scrub check takes -e, so a password starting with a hyphen
is not read as options; and key_hint stopped repeating key_env in four of five
arms.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
2026-08-12 17:51:55 -04:00
steveandClaude Opus 5 0abcd16e9e fix(ci): make the credential scrub failure-safe, and stop the lists drifting
Build & push image / build-and-push (pull_request) Successful in 1m13s
Build & push image / test (pull_request) Successful in 9m41s
Round 5, and the best findings are again about the fix from round 4.

The scrub only ran on success. `set -e` aborts the step when `go mod download`
fails, so the cleanup line after it never executed — leaving a push-capable
credential on a long-lived self-hosted runner for whatever job landed there
next. It is now a `trap ... EXIT`, verified against a simulated failure.

It also scrubbed the wrong file in principle: `git config --global` writes to
GIT_CONFIG_GLOBAL, else $XDG_CONFIG_HOME/git/config when that exists, else
~/.gitconfig — so deleting ~/.gitconfig can scrub a path the credential was
never in. The step now names GIT_CONFIG_GLOBAL itself, leaving exactly one file
to remove.

And the verification failed open in the case that matters most: `grep -F ""`
matches every file, so a run WITHOUT the secret — a fork PR, the threat model —
failed the check with a message accusing it of leaking a credential it never
had. Guarded on a non-empty secret.

Credentials move to an Authorization header instead of being embedded in the
URL, so a password containing @ : / or # can no longer break URL parsing in a
way that reads as a bad password.

Two list-drift holes closed with one test that reads across languages:
TestOpenAICompatProvidersAreFullyWired asserts every openAICompatProviders
entry is both advertised in endpointProviderNames and has a credential arm in
scripts/preflight.sh. Adding a compat provider touches three places in two
languages and nothing connected them. Break-checked in both directions.

Finally, a whitespace-only GADFLY_BASE_URL disagreed across the boundary: Go
TrimSpaces it and takes the registry path, bash called it "set" and skipped the
pre-flight, so the missing key arrived as a bare 401 with no notice. Both now
agree on what unset means.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
2026-08-12 17:40:03 -04:00
steveandClaude Opus 5 14f8533e38 fix(ci): scrub the registry credential before running repo code
Build & push image / build-and-push (pull_request) Successful in 4s
Build & push image / test (pull_request) Successful in 9m47s
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]>
2026-08-12 17:28:08 -04:00
steveandClaude Opus 5 ebfaeba07e docs(qwen): warn that Model Studio keys are endpoint-scoped
A workspace-scoped Qwen endpoint rejects a key issued for the shared
international host with a genuine 'Incorrect API key provided', so a valid key
reads as invalid and the obvious next move — checking the key — confirms it is
fine and leads nowhere. Watched this cost real debugging time on a live
deployment today; gadfly would hit it identically. Documents the
GADFLY_ENDPOINT_* form, which reaches a workspace host with no code change.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
2026-08-12 17:22:08 -04:00
steveandClaude Opus 5 67a73616e1 fix(qwen): gadfly round 3 — stop guarding a duplicate, delete it
Build & push image / build-and-push (pull_request) Successful in 5s
Build & push image / test (pull_request) Canceled after 7m27s
Twelve findings, all real, and the two that matter are about the pre-flight I
added rather than about qwen.

The credential check had a false pass in the OTHER direction from round 2's: on
the GADFLY_BASE_URL override path, resolveModel builds the client with
GADFLY_API_KEY and never reads QWEN_API_KEY/KIMI_API_KEY, so treating the
provider's own key as sufficient there let a doomed run proceed. Having now
been wrong about these rules in both directions, the check no longer tries to
model both paths: it covers the REGISTRY path, whose rules it can state
exactly, and says nothing about the override path — which is hand-configured by
definition, while the registry path is the one you hit by adding a model id to
a var and forgetting the secret.

The logic moves to scripts/preflight.sh, sourced by both run.sh and the test.
The previous answer to "this test duplicates production logic" was a regex
drift-guard, and that guard compared only the provider table — not the decision
logic, which is precisely the half that carried the bug. A duplicate you guard
is still a duplicate; this deletes it, and the test now runs under `set -u`
like production does.

Also: the test that pins the shared provider slice held its own copy of the
list (now ranges the slice); endpointProviderNames had nothing tying it to the
switches it describes, which is how it shipped without "gemini" (a new test
asserts every advertised name resolves); two godoc lists had drifted; and the
"sanity" line that asserted nothing is gone.

And the repo had NO test job — `go test` and the pre-flight table both existed
and neither was ever executed by CI, which reads as coverage while providing
none. Added one (build/vet/gofmt/test/pre-flight), running alongside the image
build rather than gating it, so red is loud without standing between a push and
a rebuild.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
2026-08-12 17:20:44 -04:00
steveandClaude Opus 5 1d6eaa08c5 fix(qwen): gadfly round 2 — the anti-drift list had already drifted
Build & push image / build-and-push (pull_request) Successful in 3s
Eight findings, all real, and the sharpest ones are about this PR's own fixes.

GADFLY_API_KEY was treated as a universal substitute in the pre-flight. It is
not: resolveModel reads it only AFTER the `baseURL == ""` early return, so on
the registry path — the documented primary path — a qwen/kimi built-in reads
its own variable and GADFLY_API_KEY is never consulted. A mis-set
GADFLY_API_KEY therefore passed pre-flight and 401'd five times anyway, which
is precisely the failure this check exists to prevent. It now only substitutes
when GADFLY_BASE_URL is also set.

`openai-compatible` was missing from the pre-flight table while both switches
accept it as an OPENAI_API_KEY alias, so that one spelling still fell through
to the cryptic five-failure mode.

endpointProviderNames — the constant I introduced *to stop* the two error
messages drifting — omitted the `gemini` alias both switches accept. It now
lists every accepted spelling.

And the case list itself was still duplicated across both switches plus the
test that pins them: three copies of the thing whose duplication started this.
Both switches now call isOpenAICompatProvider over one shared slice, and
endpointProvider's doc comment points at endpointProviderNames instead of
carrying a fourth hand-written copy.

scripts/preflight_test.sh moves into the repo (20 cases, up from 17, covering
openai-compatible and both GADFLY_API_KEY directions). It carries a drift guard
that diffs its copy of the provider table against run.sh's and aborts if they
differ — break-checked by deleting an arm from run.sh, which fails it loudly.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
2026-08-12 16:59:01 -04:00
steveandClaude Opus 5 2367e696b5 fix(qwen): gadfly round 1 — three real findings, all sibling drift
Build & push image / build-and-push (pull_request) Successful in 3s
The pre-flight comment was the worst of them, and three models agreed. It said
providers absent from the table "need no key or carry it in their endpoint/DSN"
— false for google, which needs a key and is absent for an entirely different
reason: it accepts GOOGLE_API_KEY *or* GEMINI_API_KEY, so a single-variable arm
would silently skip a correctly-configured reviewer. That reasoning was in the
PR description and not in the code, so the comment invited exactly the wrong
edit. It now states both exclusion reasons and names google's.

Forwarded KIMI_API_KEY alongside QWEN_API_KEY in the dogfooding stub. This PR
argues that sibling call sites must move together, and I declared both secrets
in the reusable workflow and forwarded one — a config that looks complete and
401s on the model you didn't wire.

The two endpoint-provider error messages listed the same accepted set in
different order and spelling. Both functions accept an identical set, so they
now share one endpointProviderNames constant and cannot disagree.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
2026-08-12 16:43:00 -04:00
steveandClaude Opus 5 0f40b21d79 feat(qwen): let Qwen (and Kimi) join the swarm
Build & push image / build-and-push (pull_request) Successful in 14s
Gadfly review (reusable) / review (pull_request) Successful in 8m46s
Adversarial Review (Gadfly) / review (pull_request) Successful in 8m46s
majordomo now ships qwen and kimi as built-ins that ARE the openai client at
their own base URL, so "qwen/qwen3.8-max" works as a GADFLY_MODELS entry once
the key reaches the container. This wires up the parts that key has to pass
through.

Two provider switches had to learn the names, not one. resolveModel's
GADFLY_BASE_URL override was the obvious one; endpointProvider's
GADFLY_ENDPOINT_* parser is its sibling, and I fixed the first and missed the
second on the first pass — a config that resolves one way and errors the other
for no reason a user could guess. TestOpenAICompatProvidersResolveOnBothPaths
now asserts both from one table so the pair fails together; break-checked in
both directions.

QWEN_API_KEY (and KIMI_API_KEY) are declared as workflow_call secrets and
forwarded to the container, with gadfly's own stub forwarding QWEN_API_KEY so a
qwen entry can join the default swarm by editing GADFLY_DEFAULT_MODELS alone —
no workflow edit, no re-release.

The run.sh credential pre-flight is now a provider→variable table instead of an
ollama-cloud special case. Without it a forgotten key surfaces as five
identical per-lens agent failures naming no variable, and the operator reads a
stack trace to find out which secret they missed. Google stays out of the table
on purpose: it accepts either GOOGLE_API_KEY or GEMINI_API_KEY, and a one-var
entry would wrongly skip a correctly-configured run. Verified across 17
provider x key-state combinations, including that a wrong-provider key never
satisfies qwen (majordomo refuses cross-provider fallback) and that unkeyed
providers are never blocked.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
2026-08-12 16:30:29 -04:00
13 changed files with 853 additions and 35 deletions
+10
View File
@@ -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:
+102
View File
@@ -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
+23 -5
View File
@@ -63,7 +63,7 @@ on:
max_steps: { type: string, default: "14" } # GADFLY_MAX_STEPS
worker_model: { type: string, default: "" } # GADFLY_WORKER_MODEL
allowed_users: { type: string, default: "" } # GADFLY_ALLOWED_USERS (consumer-specific; set in your stub)
reviewer_tag: { type: string, default: "" } # reviewer image tag (e.g. "sha-b37cd09") — empty falls back to user var GADFLY_REVIEWER_TAG, then the pin baked into the container: line
reviewer_tag: { type: string, default: "" } # reviewer image tag (e.g. "sha-b850e35") — empty falls back to user var GADFLY_REVIEWER_TAG, then the pin baked into the container: line
# Gitea >= 1.27 does not propagate the CALLER's workflow_dispatch inputs into a
# called workflow's github.event, so a manual "review PR #N" dispatch arrived
# here with an empty PR and died at the entrypoint's "PR required" check. The
@@ -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 }
@@ -116,11 +125,14 @@ jobs:
# the owner's variables redirects every consumer's reviewer image (same blast
# radius as editing this file, but without a commit trail).
#
# Fallback pin: sha-b37cd09 — the provider-wide lens budget (PR #27) on top of
# the opencode CLI engine (PR #26) and the Gitea >= 1.27 workflow_call
# reclassification. Keep it current-ish when touching this file anyway.
# Fallback pin: sha-b850e35 — adds the qwen/kimi built-ins (PR #30: majordomo
# bumped for the qwen provider, a credential pre-flight that names the missing
# variable, and one no-cross-vendor-fallback rule so a keyless kimi/qwen
# endpoint can never send an OpenAI key to another vendor) on top of the
# provider-wide lens budget (PR #27). Keep it current-ish when touching this
# file anyway.
container:
image: gitea.stevedudenhoeffer.com/steve/gadfly:${{ inputs.reviewer_tag || vars.GADFLY_REVIEWER_TAG || 'sha-b37cd09' }}
image: gitea.stevedudenhoeffer.com/steve/gadfly:${{ inputs.reviewer_tag || vars.GADFLY_REVIEWER_TAG || 'sha-b850e35' }}
steps:
- name: Run the gadfly reviewer
run: /entrypoint.sh
@@ -146,6 +158,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
+3
View File
@@ -1,3 +1,6 @@
/gadfly
/out
*.orig
# Local worktrees created for agent work — never part of the repo.
.claude/worktrees/
+33 -1
View File
@@ -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`)
+116 -20
View File
@@ -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)
}
}
+293 -1
View File
@@ -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)
}
}
}
+9 -1
View File
@@ -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.
+1 -1
View File
@@ -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
)
+2 -2
View File
@@ -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=
+135
View File
@@ -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
}
+115
View File
@@ -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"
+11 -4
View File
@@ -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