fix(qwen): gadfly round 1 — three real findings, all sibling drift
Build & push image / build-and-push (pull_request) Successful in 3s
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]>
This commit is contained in:
@@ -46,11 +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>" entry can join the swarm by editing the
|
||||
# GADFLY_DEFAULT_MODELS var alone — no workflow edit, no re-release.
|
||||
# Empty until the repo secret exists: that's a 401 on that one model,
|
||||
# not a broken review.
|
||||
# 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:
|
||||
|
||||
+9
-2
@@ -19,6 +19,13 @@ import (
|
||||
// model list is just ids like "qwen3-coder:480b-cloud" — working unchanged.
|
||||
const defaultProvider = "ollama-cloud"
|
||||
|
||||
// endpointProviderNames is the operator-facing list of providers that accept an
|
||||
// explicit endpoint. resolveModel and endpointProvider accept the SAME set, so
|
||||
// they share one message rather than each carrying a hand-maintained copy that
|
||||
// drifts in order and spelling — which is exactly what happened when kimi/qwen
|
||||
// were added to both switches.
|
||||
const endpointProviderNames = "openai/openai-compatible/kimi/qwen/ollama/ollama-cloud/llama-swap(s)/foreman/anthropic/google"
|
||||
|
||||
// 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
|
||||
@@ -115,7 +122,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/kimi/qwen/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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -288,6 +295,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/kimi/qwen/anthropic/google)", provider)
|
||||
return nil, fmt.Errorf("unknown provider %q (use %s)", provider, endpointProviderNames)
|
||||
}
|
||||
}
|
||||
|
||||
+13
-3
@@ -166,9 +166,19 @@ case "$PROVIDER" in
|
||||
# majordomo already fails closed with a 401: without this, a missing key
|
||||
# surfaces as five identical per-lens agent failures that name no variable,
|
||||
# and the operator has to read a stack trace to learn which secret they
|
||||
# forgot to forward. Providers absent from this table need no key (local
|
||||
# ollama, llama-swap) or carry it in their endpoint/DSN (foreman), and are
|
||||
# left alone. GADFLY_API_KEY overrides any of them.
|
||||
# forgot to forward. GADFLY_API_KEY overrides any entry.
|
||||
#
|
||||
# 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 has more than one acceptable variable, so a
|
||||
# single-variable check would skip a correctly-configured run.
|
||||
# **google** is this case: it accepts GOOGLE_API_KEY *or*
|
||||
# GEMINI_API_KEY. Adding `google) KEY_ENV="GOOGLE_API_KEY"` here would
|
||||
# silently skip every reviewer configured with GEMINI_API_KEY. If you
|
||||
# want google pre-flighted, the check has to accept either variable,
|
||||
# not the table's one-name shape.
|
||||
KEY_ENV=""; KEY_HINT=""
|
||||
case "$GADFLY_PROVIDER_EFF" in
|
||||
ollama-cloud) KEY_ENV="OLLAMA_API_KEY"; KEY_HINT="OLLAMA_CLOUD_API_KEY" ;;
|
||||
|
||||
Reference in New Issue
Block a user