fix(qwen): gadfly round 2 — the anti-drift list had already drifted
Build & push image / build-and-push (pull_request) Successful in 3s
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]>
This commit is contained in:
Claude Opus 5
@@ -0,0 +1,93 @@
|
||||
#!/usr/bin/env bash
|
||||
# Exercise the run.sh credential pre-flight in isolation: every provider x
|
||||
# key-present/absent x GADFLY_API_KEY/GADFLY_BASE_URL combination.
|
||||
#
|
||||
# The preflight() below is a COPY of run.sh's logic, which makes it exactly the
|
||||
# kind of duplicated pair this whole PR keeps finding. The drift guard runs
|
||||
# first: it compares the provider→variable arms in both files and aborts if
|
||||
# they differ, so a table edited in run.sh and not here fails loudly instead of
|
||||
# certifying stale logic.
|
||||
set -u
|
||||
|
||||
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
|
||||
RUN_SH="${RUN_SH:-$SCRIPT_DIR/run.sh}"
|
||||
|
||||
# Normalized case arms: "provider) KEY_ENV=... KEY_HINT=..." with runs of
|
||||
# whitespace collapsed, so alignment changes don't trip the guard.
|
||||
arms_of() { grep -oE '^[[:space:]]*[a-z|-]+\)[[:space:]]+KEY_ENV="[A-Z_]+";[[:space:]]+KEY_HINT="[A-Z_]+"' "$1" | tr -s ' \t' ' ' | sed 's/^ //'; }
|
||||
|
||||
if [ -r "$RUN_SH" ]; then
|
||||
if ! diff <(arms_of "$RUN_SH") <(arms_of "$0") >/dev/null; then
|
||||
echo "FAIL drift guard: the pre-flight table here no longer matches $RUN_SH"
|
||||
diff <(arms_of "$RUN_SH") <(arms_of "$0") | sed 's/^/ /'
|
||||
exit 1
|
||||
fi
|
||||
echo "ok drift guard: table matches run.sh"
|
||||
else
|
||||
echo "FAIL drift guard: cannot read $RUN_SH — cannot prove this tests the real table"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
preflight() { # $1=provider ; env carries the keys
|
||||
local GADFLY_PROVIDER_EFF="$1" KEY_ENV="" KEY_HINT="" KEY_OK=0
|
||||
case "$GADFLY_PROVIDER_EFF" 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
|
||||
[ -n "$KEY_ENV" ] && [ -n "${!KEY_ENV:-}" ] && KEY_OK=1
|
||||
[ -n "${GADFLY_BASE_URL:-}" ] && [ -n "${GADFLY_API_KEY:-}" ] && KEY_OK=1
|
||||
if [ -n "$KEY_ENV" ] && [ "$KEY_OK" -eq 0 ]; then
|
||||
echo "SKIP:$KEY_HINT"
|
||||
else
|
||||
echo "RUN"
|
||||
fi
|
||||
}
|
||||
|
||||
fail=0
|
||||
check() { # desc, want, got
|
||||
if [ "$2" = "$3" ]; then echo "ok $1"; else echo "FAIL $1 — want $2, got $3"; fail=1; fi
|
||||
}
|
||||
|
||||
env -i bash -c 'true' >/dev/null 2>&1 # sanity
|
||||
|
||||
# --- keyed providers with NO key -> skip, naming the right variable ---
|
||||
check "qwen, no key" "SKIP:QWEN_API_KEY" "$(env -u QWEN_API_KEY -u GADFLY_API_KEY bash -c "$(declare -f preflight); preflight qwen")"
|
||||
check "kimi, no key" "SKIP:KIMI_API_KEY" "$(env -u KIMI_API_KEY -u GADFLY_API_KEY bash -c "$(declare -f preflight); preflight kimi")"
|
||||
check "ollama-cloud, none" "SKIP:OLLAMA_CLOUD_API_KEY" "$(env -u OLLAMA_API_KEY -u GADFLY_API_KEY bash -c "$(declare -f preflight); preflight ollama-cloud")"
|
||||
check "openai, no key" "SKIP:OPENAI_API_KEY" "$(env -u OPENAI_API_KEY -u GADFLY_API_KEY bash -c "$(declare -f preflight); preflight openai")"
|
||||
check "anthropic, no key" "SKIP:ANTHROPIC_API_KEY" "$(env -u ANTHROPIC_API_KEY -u GADFLY_API_KEY bash -c "$(declare -f preflight); preflight anthropic")"
|
||||
|
||||
# --- keyed providers WITH their key -> run ---
|
||||
check "qwen, keyed" "RUN" "$(env -u GADFLY_API_KEY -u GADFLY_BASE_URL QWEN_API_KEY=k bash -c "$(declare -f preflight); preflight qwen")"
|
||||
check "ollama-cloud, keyed" "RUN" "$(env -u GADFLY_API_KEY -u GADFLY_BASE_URL OLLAMA_API_KEY=k bash -c "$(declare -f preflight); preflight ollama-cloud")"
|
||||
|
||||
# --- the WRONG key must not satisfy a provider (no cross-provider fallback) ---
|
||||
check "qwen w/ only OPENAI key" "SKIP:QWEN_API_KEY" "$(env -u GADFLY_API_KEY -u GADFLY_BASE_URL -u QWEN_API_KEY OPENAI_API_KEY=k bash -c "$(declare -f preflight); preflight qwen")"
|
||||
|
||||
# --- GADFLY_API_KEY substitutes ONLY with GADFLY_BASE_URL (the override path) ---
|
||||
check "qwen via GADFLY_API_KEY + BASE_URL" "RUN" \
|
||||
"$(env -u QWEN_API_KEY GADFLY_API_KEY=k GADFLY_BASE_URL=https://x bash -c "$(declare -f preflight); preflight qwen")"
|
||||
# The false-pass this check exists to prevent: on the registry path (no
|
||||
# GADFLY_BASE_URL) the qwen built-in reads QWEN_API_KEY and never consults
|
||||
# GADFLY_API_KEY, so pre-flighting it as sufficient lets a doomed run proceed.
|
||||
check "qwen w/ GADFLY_API_KEY but no BASE_URL" "SKIP:QWEN_API_KEY" \
|
||||
"$(env -u QWEN_API_KEY -u GADFLY_BASE_URL GADFLY_API_KEY=k bash -c "$(declare -f preflight); preflight qwen")"
|
||||
|
||||
# --- openai-compatible is its own spelling and must be pre-flighted too ---
|
||||
check "openai-compatible, no key" "SKIP:OPENAI_API_KEY" \
|
||||
"$(env -u OPENAI_API_KEY -u GADFLY_API_KEY -u GADFLY_BASE_URL bash -c "$(declare -f preflight); preflight openai-compatible")"
|
||||
check "openai-compatible, keyed" "RUN" \
|
||||
"$(env -u GADFLY_API_KEY -u GADFLY_BASE_URL -u GADFLY_BASE_URL OPENAI_API_KEY=k bash -c "$(declare -f preflight); preflight openai-compatible")"
|
||||
|
||||
# --- empty-string key counts as missing, not present ---
|
||||
check "qwen, empty key" "SKIP:QWEN_API_KEY" "$(env -u GADFLY_API_KEY -u GADFLY_BASE_URL QWEN_API_KEY= bash -c "$(declare -f preflight); preflight qwen")"
|
||||
|
||||
# --- unkeyed providers are never blocked, even with nothing set ---
|
||||
for p in ollama llama-swap llamaswap foreman google gemini some-dsn-name; do
|
||||
check "unkeyed $p" "RUN" "$(env -i bash -c "$(declare -f preflight); preflight $p")"
|
||||
done
|
||||
|
||||
exit $fail
|
||||
+16
-10
@@ -181,17 +181,23 @@ case "$PROVIDER" in
|
||||
# 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" ;;
|
||||
qwen) KEY_ENV="QWEN_API_KEY"; KEY_HINT="QWEN_API_KEY" ;;
|
||||
kimi) KEY_ENV="KIMI_API_KEY"; KEY_HINT="KIMI_API_KEY" ;;
|
||||
openai) KEY_ENV="OPENAI_API_KEY"; KEY_HINT="OPENAI_API_KEY" ;;
|
||||
anthropic) KEY_ENV="ANTHROPIC_API_KEY"; KEY_HINT="ANTHROPIC_API_KEY" ;;
|
||||
ollama-cloud) KEY_ENV="OLLAMA_API_KEY"; 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
|
||||
# Indirect expansion (bash), so the table above stays a table. 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" ] && [ -z "${!KEY_ENV:-}" ] && [ -z "${GADFLY_API_KEY:-}" ]; then
|
||||
REVIEW="⚠️ No API key configured for provider \`${GADFLY_PROVIDER_EFF}\` (set \`${KEY_HINT}\`, or \`GADFLY_API_KEY\`); this reviewer was skipped."
|
||||
# GADFLY_API_KEY substitutes for the provider's own variable ONLY on the
|
||||
# endpoint-override path: resolveModel reads it after the `baseURL == ""`
|
||||
# early return, so with GADFLY_BASE_URL unset the built-in reads its own
|
||||
# env var and GADFLY_API_KEY is never consulted. Treating it as a universal
|
||||
# substitute made a mis-set GADFLY_API_KEY pass pre-flight and then 401 five
|
||||
# times anyway — the exact failure this check exists to prevent.
|
||||
KEY_OK=0
|
||||
[ -n "$KEY_ENV" ] && [ -n "${!KEY_ENV:-}" ] && KEY_OK=1 # indirect expansion (bash)
|
||||
[ -n "${GADFLY_BASE_URL:-}" ] && [ -n "${GADFLY_API_KEY:-}" ] && KEY_OK=1
|
||||
if [ -n "$KEY_ENV" ] && [ "$KEY_OK" -eq 0 ]; then
|
||||
REVIEW="⚠️ No API key configured for provider \`${GADFLY_PROVIDER_EFF}\` (set \`${KEY_HINT}\`, or \`GADFLY_API_KEY\` together with \`GADFLY_BASE_URL\`); this reviewer was skipped."
|
||||
else
|
||||
BIN="${GADFLY_BIN:-gadfly}"
|
||||
if ! command -v "$BIN" >/dev/null 2>&1 && [ ! -x "$BIN" ]; then
|
||||
|
||||
Reference in New Issue
Block a user