feat: bump majordomo + support llama-swap(s) provider spellings #7
Reference in New Issue
Block a user
Delete Branch "feat/llamaswap-schemes"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Per Steve: bump majordomo again, and make sure gadfly supports the
llama-swap/llama-swapsprovider, including thellamaswapsspelling.…225659).llama-swap/llama-swaps(mirroring majordomo's DSN schemes:llama-swap= http,llama-swaps= https/TLS) and the un-hyphenatedllamaswap/llamaswaps— in both theGADFLY_PROVIDER+GADFLY_BASE_URLoverride and theGADFLY_ENDPOINT_<NAME>="llama-swap|url[|key]"form. Before, onlyllamaswapmatched, so a majordomo-canonicalllama-swaphit "unknown provider".LLM_*DSN path (llama-swap://…,llama-swaps://…) already resolves viamajordomo.Parse— no gadfly change needed there; verified it's covered.endpointProvideralias tests.gofmt/vet/build/test -racegreen.🤖 Generated with Claude Code
🪰 Gadfly — live review status
9/9 reviewers finished · updated 2026-06-27 23:15:42Z
claude-code/opus· claude-code — ✅ doneclaude-code/opus:max· claude-code — ✅ doneclaude-code/sonnet· claude-code — ✅ donedeepseek-v4-pro:cloud· ollama-cloud — ✅ doneglm-5.1:cloud· ollama-cloud — ✅ doneglm-5.2:cloud· ollama-cloud — ✅ doneminimax-m3:cloud· ollama-cloud — ✅ donenemotron-3-super:cloud· ollama-cloud — ✅ doneqwen3-coder:480b-cloud· ollama-cloud — ✅ doneLive status board. Findings are posted in each model's own comment. Advisory only — does not block merge.
🪰 Gadfly review —
claude-code/sonnet(claude-code)Verdict: No material issues found — 3 reviewers: security, correctness, error-handling
🔒 Security — No material issues found
Verdict: No material issues found
The change adds four provider-name spellings to two
switchstatements inendpointProviderandresolveModel, then bumps majordomo. I verified the full code in both functions.Input sources are operator-controlled env vars only.
GADFLY_PROVIDER,GADFLY_BASE_URL,GADFLY_API_KEY, andGADFLY_ENDPOINT_*all come from the container environment set by whoever deploys Gadfly — not from PR content or any untrusted input. No new tainted-input path is introduced.The new cases are mechanically identical to the pre-existing
"llamaswap"case.llamaswap.WithBaseURL(baseURL)/llamaswap.WithToken(apiKey)calls are unchanged; only the matching strings expanded. There is no new code executing, no new URL construction, and no new credential handling.endpointProviderraw-string parsing (strings.SplitN(raw, "|", 3)) is pre-existing and already used for all other providers. A|-separated format with a fixed max-split of 3 prevents any injection of extra fields. No change here.API keys are not logged. The only
fmt.Fprintln(os.Stderr, ...)at line 48 prints the error string fromendpointProvider, which never includes the key value — verified atmodel.go:207andmodel.go:216.Dependency bump:
go.sumhash integrity is the normal Go module trust mechanism. No code-level concern from the diff; I cannot inspect the private module's internals, but that is outside this lens's purview and no evidence of concern exists in the diff.Nothing in this change widens the security perimeter.
🎯 Correctness — No material issues found
No material issues found
Verified by reading
model.goandmodel_test.goin full.Switch consistency (
model.go:83,model.go:226): BothresolveModel()andendpointProvider()were updated with the same four spellings ("llamaswap","llamaswaps","llama-swap","llama-swaps"). The two switch arms are in sync — no spelling is handled in one but not the other.Semantics of
GADFLY_BASE_URLpath (model.go:83): WhenGADFLY_BASE_URLis set, the scheme is determined entirely by the URL itself (e.g.http://vshttps://), so conflating the TLS variants (llamaswaps/llama-swaps) with the plaintext ones in a single case arm is correct behavior, as the in-code comment accurately explains.baseURL == ""path: All four spellings pass throughbuildSpec()and thenmajordomo.Parse()before ever reaching the switch; majordomo owns DSN resolution there, and the PR description confirmsllama-swap:///llama-swaps://are already understood by majordomo. No gadfly-side correctness gap.Test coverage (
model_test.go:53–65): The new loop tests"llama-swap","llama-swaps", and"llamaswaps"againstendpointProvider. The original"llamaswap"is covered by the pre-existing"llamaswap registers under its name"/"llamaswap without token"tests. All four spellings are exercised. No loop-variable capture hazard:go.moddeclaresgo 1.26.2, where per-iteration scoping applies.Error messages: Both default-case messages were updated and neither is misleading — they guide users toward the canonical hyphenated form, which is fine.
The logic is internally consistent and the semantics match the stated intent.
🧯 Error handling & edge cases — No material issues found
No material issues found
Reviewed through the error-handling & edge-cases lens.
resolveModelnew cases (model.go:80): The three new spellings fall into identical code as the original"llamaswap"— same opts-building, samellamaswap.WithBaseURL(baseURL)+ optionalWithToken, samellamaswap.New(...).Model(model)return. No new error paths, no new nil/empty risks. TheGADFLY_BASE_URL == ""early-return at line 64 guards all four spellings consistently. Verified by reading the full switch.endpointProvidernew cases (model.go:181): Same pattern. The existing pre-condition checks (len(parts) < 2→ error,baseURL == ""→ error) cover all four spellings uniformly. Verified that no spelling bypasses these guards.Test loop (model_test.go:38–44): Tests the three newly-added spellings in
endpointProvider. Original"llamaswap"is already covered by the named subtests above (lines 27–36), so coverage is complete across all four spellings. No goroutine capture or global-state concern — loop is sequential,endpointProviderdoesn't callRegisterProvider.resolveModelwith no base URL: All four spellings fall through tomajordomo.Parse(buildSpec(provider, model))at line 65."llama-swap/model"as a majordomo spec behaves the same as the pre-existing"llamaswap/model"— both rely on the caller having anLLM_*DSN or alias configured. No regression.Nothing in the unhappy paths introduced by this diff is materially different from the existing
"llamaswap"handling.Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 5m 56s
🪰 Gadfly review —
minimax-m3:cloud(ollama-cloud)Verdict: No material issues found — 3 reviewers: security, correctness, error-handling
🔒 Security — No material issues found
VERDICT: No material issues found
Reviewed through the 🔒 Security lens only. The change is purely a provider-name spelling expansion and a dependency bump — there is no new authn/authz, injection surface, deserialization, SSRF, secret handling, or input-validation boundary introduced.
Notes verified by reading
cmd/gadfly/model.go(lines 43-261),cmd/gadfly/model_test.go(lines 5-69), and grepping forllamaswap|llama-swap|llamaswaps:GADFLY_API_KEY(line 60) and the per-endpointkeyparsed fromGADFLY_ENDPOINT_*(lines 214-217) are already plumbed intollamaswap.WithToken/openai.WithAPIKey/etc. The PR doesn't add new code paths that echo or log these values; the existing key-handling is unchanged.endpointProvideralready requires aprovider|base-url[|key]shape and rejects empty URLs (model.go:209-220); adding more accepted aliases to the switch (model.go:229) does not weaken that validation. The new alias names flow straight into the samellamaswap.New(...).WithBaseURL(baseURL)call, where baseURL was already validated non-empty.strings.SplitN(raw, "|", 3)parser is unchanged.GADFLY_BASE_URLand per-endpoint URL semantics already existed; alias expansion does not alter which strings reachWithBaseURL.llamaswapprovider with the same options; nothing becomes opt-in to a weaker mode.The majordomo version bump (
go.mod) is a transitive dependency upgrade outside the gadfly call surface I can audit from this repo's checked-out state; nothing in the diff itself propagates tainted input to new code paths.Outside my lens: nothing severe spotted that another lens would obviously miss; this PR is a cosmetic alias-expansion + dep bump.
🎯 Correctness — No material issues found
VERDICT: No material issues found
Verification summary:
cmd/gadfly/model.go:83(theresolveModeloverride switch) now accepts"llamaswap", "llamaswaps", "llama-swap", "llama-swaps", all routing to the identicalllamaswap.New(opts...).Model(model)construction withWithBaseURL(baseURL)+ optionalWithToken. Read full switch (lines 70-112). The comment correctly notes that with an explicitGADFLY_BASE_URLthe scheme is whatever the URL says, so collapsing the four spellings into one branch is semantically correct.cmd/gadfly/model.go:229(endpointProviderswitch) does the same. Read lines 222-258 — verified all four spellings produce an equivalentllamaswap.New(WithName, WithBaseURL, WithToken)provider.cmd/gadfly/model_test.go:49-61adds loop-driven tests for the three previously-untested spellings (llama-swap,llama-swaps,llamaswaps); the fourth (llamaswap) was already covered by the pre-existing subtests at lines 35-48. Read the whole test file — coverage of all four aliases is complete.model.go:111andmodel.go:259now mentionllama-swap. Note these only fire in thedefaultbranch when the provider is not recognized, so the four new aliases never hit them — the spelling-in-the-error concern doesn't apply.resolveWorkerModel,resolveSelectorModelinauto.go:34) both go throughmajordomo.Parseand don't touch the override switch, so no parallel alias list is needed there. Verified atcmd/gadfly/main.go:167andcmd/gadfly/auto.go:43.go.mod/go.summajordomo bump is a dependency-version change only; no correctness risk visible at the call sites used here.Minor observation (not material): the
resolveModeloverride switch (line 83) has no direct unit test for its new alias cases — coverage is only via the structurally identicalendpointProviderpath. Logic is trivial enough that this is a coverage nit, not a correctness defect.🧯 Error handling & edge cases — No material issues found
VERDICT: No material issues found
Through the error-handling & edge-cases lens:
cmd/gadfly/model.go:83andcmd/gadfly/model.go:229widen the switch consistently to all four spellings (llamaswap,llamaswaps,llama-swap,llama-swaps), both funneling into the samellamaswap.New(opts...).Model(model)/…Providerpath. No new failure mode or panic surface is introduced —apiKey/keyare guarded byif … != ""andbaseURLis trimmed-and-checked atcmd/gadfly/model.go:218-220.cmd/gadfly/model.go:207-220still rejects malformed entries (len(parts) < 2, emptybaseURL) and thedefault:arm atmodel.go:111/model.go:259returns a descriptive error rather than nil. No swallowed errors, no missing cleanup/rollback on the new paths.cmd/gadfly/model_test.go:51-61test loop exercises each spelling throughendpointProviderand assertsp.Name() == "ls"; the existing "rejects …" loop still covers empty/garbage inputs.GADFLY_MODEL(model.go:52-54), unsetGADFLY_PROVIDERfalling back todefaultProvider(model.go:56-58), and the no-GADFLY_BASE_URLdelegation tomajordomo.Parse(model.go:65-67) are untouched.Minor non-blocking observations (not in my lane enough to escalate, noting only for completeness):
model.go:111error message listsllama-swapbutmodel.go:259saysllama-swap(s); cosmetic, no behavioral impact.endpointProvider, not throughresolveModel's switch atmodel.go:83. Same constructor either way, so not a defect — just a coverage gap if you wanted symmetric tests.Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 52s
🪰 Gadfly review —
glm-5.1:cloud(ollama-cloud)Verdict: No material issues found — 3 reviewers: security, correctness, error-handling
🔒 Security — No material issues found
VERDICT: No material issues found
llamaswap,llamaswaps,llama-swap,llama-swaps) fall through to the same well-boundedcasebranch. Unrecognized providers still hit thedefaulterror path. No injection or authz gap introduced.GADFLY_API_KEYand the|keysegment fromGADFLY_ENDPOINT_*are passed tollamaswap.WithTokenthe same way every other provider passes keys — no new credential exposure.🎯 Correctness — No material issues found
VERDICT: No material issues found
The change is a straightforward alias expansion that is consistent and correct:
resolveModelat line 83,endpointProviderat line 229) accept the same four spellings (llamaswap,llamaswaps,llama-swap,llama-swaps), all routing to the samellamaswap.New()path.llamaswaptests plus the new loop coveringllama-swap,llama-swaps, andllamaswaps.GADFLY_BASE_URLthe scheme is whatever the URL says, so all behave the same") is semantically correct: the HTTP/HTTPS distinction betweenllama-swap/llama-swapsonly matters in majordomo DSN schemes (llama-swap://vsllama-swaps://), not when an explicit base URL overrides the scheme.defaultbranches to reflect the new canonical spelling (llama-swap(s)).modelProvider()returns the raw environment string and is used for exact-key matching in concurrency overrides. A user who setsGADFLY_PROVIDER=llama-swapmust writellama-swap=3(notllamaswap=3) in their concurrency map, but this is pre-existing exact-match semantics, not a bug introduced here.🧯 Error handling & edge cases — No material issues found
VERDICT: No material issues found
The change adds alias spellings (
llama-swap,llama-swaps,llamaswaps,llamaswap) to two existingswitchstatements incmd/gadfly/model.go(resolveModelline 83,endpointProviderline 229) and updates error message strings. Through the error-handling / edge-case lens:len(parts) < 2,baseURL == "") still applies uniformly to all spellings; no new input shapes are introduced.baseURL, emptykey, missing pipe delimiter) are unchanged and still cover all aliases.modelProvider()function returns the raw env value, which is only used for concurrency-map lookups — no new nil/zero/negative or boundary concern.llama-swap,llama-swaps, andllamaswaps; the pre-existing test coversllamaswap— all four aliases are exercised.No swallowed errors, missing cleanups, panics on bad input, or unhandled edge cases introduced.
Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 1m 14s
🪰 Gadfly review —
claude-code/opus(claude-code)Verdict: No material issues found — 3 reviewers: security, correctness, error-handling
🔒 Security — No material issues found
Investigation complete. The changed code only adds additional string cases to provider-name
switchstatements, plus doc/test/dependency updates. I traced the data flow to confirm there's no security-relevant change.VERDICT: No material issues found
Findings (Security lens):
No new tainted-input surface. The only behavioral change is widening
switch providercases to acceptllama-swap/llama-swaps/llamaswapsalongsidellamaswap, in bothresolveModel(cmd/gadfly/model.go:83) andendpointProvider(model.go:229). All inputs feeding these switches —GADFLY_PROVIDER,GADFLY_BASE_URL,GADFLY_API_KEY,GADFLY_ENDPOINT_*— are operator-supplied environment config (verified: read viaos.Getenv/os.Environatmodel.go:51-60,181-185), not PR/attacker-controlled data. No injection, SSRF, or path-traversal vector is introduced or widened.Credential handling unchanged. Token/key plumbing (
llamaswap.WithToken,WithBaseURL) is identical to the pre-existingllamaswapbranch; the new cases reuse the same option-building, so no new secret-leakage path. Keys still come only from explicit env (apiKey, or the optional 3rd|keyfield atmodel.go:215-216). Verified by reading both branches.Dependency bump is hash-pinned.
go.mod/go.summove majordomo to…225659-aa25b2c33462with bothh1:and/go.modhashes updated, so the module is integrity-checked at build (thego.modhash is even unchanged from the prior version). Nothing in this diff alters how untrusted data reaches majordomo. I cannot review the upstream module contents from here — that's an inherent trust assumption, not a flaw in this diff.Nothing in the security lane is materially wrong here.
🎯 Correctness — No material issues found
I've verified the change against the actual code. Here's my correctness review.
VERDICT: No material issues found
The change is purely additive — three new spellings (
llamaswaps,llama-swap,llama-swaps) added to the twoproviderswitch statements that previously matched onlyllamaswap, plus doc/error-string and README updates. I verified the following:cmd/gadfly/model.go:83(override path inresolveModel) andcmd/gadfly/model.go:229(endpointProvider) both gain the same four-spelling case.grepacross all.gofiles confirms these are the only places provider strings are matched againstllamaswap— no other comparator (e.g. inauto.go,modelProvider, delegate code) was left checking the old single spelling, so there's no spelling-divergence bug.baseURLverbatim intollamaswap.WithBaseURL, so all four spellings produce identical behavior. The inline comment atmodel.go:84-87correctly states that with an explicitGADFLY_BASE_URLthe URL's own scheme wins and the http/https spelling distinction is moot — that matches the code (the scheme is never re-derived from the provider word). No false implication of TLS enforcement.model_test.go:51-61loops{"llama-swap","llama-swaps","llamaswaps"}throughendpointProvider("ls", name+"|https://swap.lan:8080|tok"); the existing cases at lines 35-48 coverllamaswap, so all four spellings are exercised.SplitN(raw,"|",3)yields the expected(provider, baseURL, key)triple andWithName("ls")makes thep.Name()=="ls"assertion valid. Verified by tracingendpointProvider.Notes / things I could not verify from here (not findings):
LLM_*DSN path (llama-swap://,llama-swaps://) resolves viamajordomo.Parse, and the comment's mapping (llama-swap=http /llama-swaps=https), depend on the bumped majordomo module. The module cache is outside the sandboxed working directory, so I could not inspect it. These are not changed by this diff; gadfly's no-baseURLpath simply handsbuildSpec(provider, model)tomajordomo.Parse(model.go:66), so correctness there rests entirely on majordomo.README.md:69) now advertises only the hyphenatedllama-swap/llama-swapsasGADFLY_PROVIDERvalues, while the code still accepts the un-hyphenatedllamaswap/llamaswaps. Not a behavior bug — just an under-documentation of still-supported spellings.Nothing in the correctness lens is materially wrong.
🧯 Error handling & edge cases — No material issues found
I've read the full
model.goand the diff. This is a purely additive change — it widens existingswitchcases to accept more provider spellings and updates doc/error strings. Through my lens (error handling & edge cases), I checked the unhappy paths:resolveModel(model.go:83) andendpointProvider(model.go:229) — the new spellings are added to existing cases that already constructllamaswapwith the same option pattern. No new error path, no swallowed error.providerisTrimSpaced in both call sites (model.go:55, model.go:212), andbaseURLempty-check (model.go:218) is unchanged and still guards the endpoint path.defaulterror branches (model.go:111, model.go:259) — only the human-readable string changed; the failure behavior (return error, don't panic) is intact.Llama-Swap) is unchanged pre-existing behavior, not a regression here.VERDICT: No material issues found
switcharms and updates doc comments / error message wording; all error returns, trimming, and empty-input guards are unchanged and remain correct (verified by readingcmd/gadfly/model.go:43-261).(Anything about whether the new
llama-swap(s)spellings actually map to majordomo's intended http-vs-https DSN semantics is a correctness concern, which falls outside my lens.)Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 2m 37s
🪰 Gadfly review —
glm-5.2:cloud(ollama-cloud)Verdict: No material issues found — 3 reviewers: security, correctness, error-handling
🔒 Security — No material issues found
VERDICT: No material issues found
cmd/gadfly/model.go(bothresolveModelat line 83 andendpointProviderat line 229). The change only widens an exact-matchswitchcase set for the provider string to includellama-swap/llama-swaps/llamaswap/llamaswaps, all routing to the samellamaswap.New(...)constructor with identical option handling. No new code paths, no new parsing, no new trust boundary.GADFLY_PROVIDER,GADFLY_ENDPOINT_*), not from untrusted input, so expanding accepted spellings introduces no injection/authz surface.baseURL/keycontinue to be passed through verbatim — consistent with every other provider case and pre-existing behavior.TrimSpace-only (lines 55, 212), matching the prior case-sensitive compare used for all other providers; the added spellings are exact and lowercase, mirroring the existingllamaswaphandling. No aliasing ambiguity or privilege-routing concern results.Nothing in the security lens is materially affected by this change.
🎯 Correctness — No material issues found
VERDICT: No material issues found
cmd/gadfly/model.go:83and:229— both switches now route all four spellings (llamaswap,llamaswaps,llama-swap,llama-swaps) to the identicalllamaswap.New(...)construction. The behavior is purely additive aliasing; no semantic divergence between spellings in theGADFLY_BASE_URLoverride path (the URL's own scheme governs http/https, as the comment correctly states).modelProvider()(model.go:146) and its callers (main.go:282,emit.go:105): it returns the rawGADFLY_PROVIDER/model-prefix verbatim and is not expected to normalize aliases — this matches the pre-existing pattern whereollamavsollama-cloudare distinct lanes. The new spellings introduce no regression here; a user keying a concurrency override must simply use the same spelling they set, same as before.model_test.go:49-61) exercises three of the four new aliases viaendpointProvider; the originalllamaswapis covered by the existing cases at lines 35–48. No gap that affects correctness.go.mod/go.sumbump is a straight version hash change with matchingh1:/go.modlines; no correctness dimension to check beyond that.Nothing in the correctness lane is materially wrong.
🧯 Error handling & edge cases — No material issues found
Verdict: No material issues found
The change is purely additive within two
switchstatements (resolveModelandendpointProvider), expanding thellamaswapcase to accept four spellings. Through the error-handling / edge-case lens:default:branches still return a non-nil error, so a badGADFLY_PROVIDER/ endpoint provider string still fails cleanly rather than falling through. Verified atcmd/gadfly/model.go:111and:259.endpointProviderstill guardslen(parts) < 2(:209) and empty base URL (:218); the""and"ollama"(no-pipe) cases in the reject test table (model_test.go:62) continue to exercise these and are unaffected by the new aliases.llamaswap.Optionconstruction path; no new slicing, arithmetic, or dereferences introduced.modelis guaranteed non-empty upstream inresolveModel(:52), andendpointProviderdoesn't take a model.model_test.go:51) correctly asserts both no-error andName(), covering the happy path for the three new spellings (the originalllamaswapis covered by the earlier subtests at:35/:44).Nothing in the error-handling/edge-case lane is materially wrong here.
Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 26s
🪰 Gadfly review —
deepseek-v4-pro:cloud(ollama-cloud)Verdict: No material issues found — 3 reviewers: security, correctness, error-handling
🔒 Security — No material issues found
Verdict: No material issues found
Security review of this change:
No new injection vectors. The diff adds only new
caselabels ("llamaswaps","llama-swap","llama-swaps") to two existingswitchblocks. ThebaseURLandapiKey/keyvalues flow through the same pre-existing paths — no new parsing, no new interpolation, no new external calls. Verified by readingcmd/gadfly/model.go:70-112andcmd/gadfly/model.go:222-260in full.No secret leakage. The two error messages updated in the diff (lines 111 and 259) interpolate only the
providervariable — not the API key, not the base URL. Confirmed by reading the surrounding code; nofmt/logcall in the file includesapiKeyorkey.No authn/authz regression. The change doesn't alter how credentials are sourced (
GADFLY_API_KEYenv, or the|keysegment of endpoint specs) or how they're passed to provider constructors. Thellamaswap.WithToken()call is identical to the pre-existing one — only the set of matching provider strings expanded.No insecure defaults. The default provider remains
ollama-cloud(line 20). No new fallback behavior was introduced.Pre-existing concerns (not introduced by this diff): The
baseURLfromGADFLY_BASE_URLandGADFLY_ENDPOINT_*is passed to HTTP clients without URL validation or scheme restriction. This is a potential SSRF vector if an attacker controls the environment, but it exists identically for all providers (ollama,openai,anthropic,google,foreman) and is not worsened by this change.🎯 Correctness — No material issues found
Verdict: No material issues found
After verifying the full file at
cmd/gadfly/model.goand the test file atcmd/gadfly/model_test.go, the change is logically consistent:resolveModel()(line 83) andendpointProvider()(line 229) accept the identical set of four spellings:"llamaswap","llamaswaps","llama-swap","llama-swaps".llamaswap.New(...)provider with the same options — no divergence.GADFLY_BASE_URLpath uses the URL's own scheme, so the http-vs-https distinction that majordomo's DSN schemes encode (llama-swapvsllama-swaps) is correctly irrelevant there."llamaswap"via the pre-existing sub-tests (lines 35–48), and"llama-swap","llama-swaps","llamaswaps"via the new loop (line 51).baseURL == ""guard inendpointProvider()(line 218) fires before the switch, so the new cases can't accidentally accept an empty URL.One observation (not a bug — the concurrency system has always used exact string matching):
modelProvider()(line 146) returns the raw provider string from the environment. If a user switches fromGADFLY_PROVIDER=llamaswaptoGADFLY_PROVIDER=llama-swap, any existingGADFLY_PROVIDER_LENS_CONCURRENCY="llamaswap=N"entry would silently stop matching becauseproviderOverride()(line 291) does an exact!=comparison. The user just needs to use the same spelling in both places — no normalization is promised.🧯 Error handling & edge cases — No material issues found
Verdict: No material issues found
The diff is purely additive — it expands two
switchcases to accept hyphenated (llama-swap,llama-swaps) and the pre-existingllamaswapsspelling alongside the originalllamaswap. No new code paths, no new error conditions, no changed control flow.I verified the following by reading the full files (not just the diff hunks):
resolveModel()(line 83): The new spellings fall into the samellamaswap.New(opts...).Model(model)path. ThebaseURLis already guarded by theif baseURL == ""check at line 65, so it's always non-empty here. TheapiKeyempty-check at line 89 is unchanged. No regression.endpointProvider()(line 229): Same pattern. ThebaseURL == ""guard at line 218 and thekey != ""guard at line 231 are unchanged. Thenameparameter is always non-empty (guaranteed by thelen(key) > len("GADFLY_ENDPOINT_")check at line 184 in the caller).modelProvider()(line 146): Unchanged by this PR. It returns whatever raw provider string the user set. If a user setsGADFLY_PROVIDER=llama-swap, the concurrency key becomes"llama-swap"— they'd need the same spelling inGADFLY_PROVIDER_CONCURRENCY/GADFLY_PROVIDER_LENS_CONCURRENCY. This is consistent, transparent behavior (the function has always returned the verbatim prefix), and the hyphen passes throughproviderOverride()'sstrings.Cut/TrimSpaceparsing without issue.Error messages (lines 111, 259): Updated to mention
llama-swapinstead ofllamaswap. Purely cosmetic.Tests (model_test.go): The new loop at line 51 covers
llama-swap,llama-swaps, andllamaswaps— the three added spellings. The originalllamaswapspelling was already covered by the existing tests at lines 35–47. Coverage is complete for the new aliases.Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 1m 52s
🪰 Gadfly review —
nemotron-3-super:cloud(ollama-cloud)Verdict: No material issues found — 3 reviewers: security, correctness, error-handling
🔒 Security — No material issues found
No material issues found
After reviewing the changes through the security lens (authentication/authorization gaps, injection risks, SSRF, secrets leakage, input validation, unsafe defaults), no material security concerns were identified.
The changes primarily:
All changes are confined to:
The provider string validation uses simple string equality checks in switch statements, which poses no injection risk. The environment variables (GADFLY_PROVIDER, GADFLY_BASE_URL, GADFLY_ENDPOINT_, LLM_ DSNs) are used as-is but only passed to provider constructors after validation - no shell/SQL/template usage is introduced.
The dependency update to majordomo v0.0.0-20260627225659-aa25b2c33462 may include security fixes, but that's outside the scope of this diff review.
No authentication bypasses, credential leaks, SSRF vectors, or input validation gaps were introduced by these changes.
No material issues found
🎯 Correctness — No material issues found
No material issues found
🧯 Error handling & edge cases — No material issues found
No material issues found
Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 2m 51s
🪰 Gadfly review —
qwen3-coder:480b-cloud(ollama-cloud)Verdict: Blocking issues found — 3 reviewers: security, correctness, error-handling
🔒 Security — No material issues found
No material issues found.
🎯 Correctness — Blocking issues found
VERDICT: Blocking issues found
cmd/gadfly/model.go:83 and cmd/gadfly/model.go:229: The new
casestatements include"llamaswaps"and"llama-swaps"as aliases for the llama-swap provider. However, the existing test suite inmodel_test.godoes not include"llamaswap"(the original, unhyphenated singular form) in its alias verification loop. This is a logic gap: if"llamaswap"is still a valid and supported spelling (as implied by the PR description and prior code), it must also be included in the alias test to ensure consistency. The omission creates an untested code path for the base unhyphenated singular form under the new multi-case structure.README.md:69: The table entry for llama-swap now lists
llama-swap/llama-swapsas the primary spellings but omits mention of the backward-compatiblellamaswap/llamaswapsforms. This is inconsistent with the implementation inmodel.gowhich still accepts all four spellings. The documentation should either explicitly note all supported aliases or be corrected to match the reduced set — currently it misrepresents the full set of supported aliases.🧯 Error handling & edge cases — No material issues found
No material issues found.
The changes correctly add support for multiple spellings of the llama-swap provider ("llamaswap", "llamaswaps", "llama-swap", "llama-swaps") in both the
resolveModelandendpointProviderfunctions. The error messages and documentation have been updated accordingly. Tests have been added to verify the new provider spellings work correctly. No error handling or edge case issues were found in the diff.Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 44s
🪰 Gadfly review —
claude-code/opus:max(claude-code)Verdict: No material issues found — 3 reviewers: security, correctness, error-handling
🔒 Security — No material issues found
I've verified the changed code in
cmd/gadfly/model.go. My assessment through the security lens:VERDICT: No material issues found
llamaswaps/llama-swap/llama-swaps) to two existingswitch providerstatements (model.go:83andmodel.go:229), plus README/error-message/test updates and a dependency bump. I read both switches in full and confirmed the new cases route to the samellamaswap.New(...)construction the pre-existingllamaswapcase already used — no new code path, just additional labels selecting it.provider,baseURL, andkeyall originate from operator-controlled env vars (GADFLY_PROVIDER/GADFLY_BASE_URL/GADFLY_API_KEY/GADFLY_ENDPOINT_*), parsed atmodel.go:55-60andmodel.go:208-217. None of this is influenced by the untrusted PR diff under review, so the added cases don't widen any injection/SSRF surface. The base-URL-verbatim behavior (the one place that could matter for SSRF) is unchanged by this PR.WithToken/WithAPIKey) are wired identically to the existing cases; nothing new is logged. TheregisterEnvProviderserror path (model.go:48,188) prints the env key name and wrapped error, not the value — unchanged here.majordomo→…225659, go.mod/go.sum): hashes are pinned ingo.sum, so the supply-chain pin is intact. I cannot inspect the private module's contents with these tools, so its diff is unverified — but nothing in the gadfly-side change depends on new majordomo behavior in a security-relevant way.Nothing in the security lane is materially wrong with this change.
🎯 Correctness — No material issues found
I've verified the key paths by reading
cmd/gadfly/model.goin full and grepping everyllamaswap/llama-swapreference in the repo. I could not inspect the bumped majordomo module (it lives outside the sandboxed working directory), so the dependency-internal claims are noted as unverified below.VERDICT: No material issues found
Reviewed through the Correctness lens.
What I verified (by reading
cmd/gadfly/model.goand grepping the repo):resolveModel(model.go:83) and the named-endpoint path inendpointProvider(model.go:229) — are updated symmetrically to accept all four spellings (llamaswap,llamaswaps,llama-swap,llama-swaps), each routing to the samellamaswapprovider construction. No spelling is handled in one switch but missed in the other.llamaswappackage is imported (model.go:12) and the option constructors used (WithBaseURL/WithToken/WithName) are the same ones already in use pre-PR, so the new cases are well-typed.model_test.go) exercisesllama-swap,llama-swaps,llamaswapsthroughendpointProviderand assertsName() == "ls"with no error — these inputs do hit the new case, and the negative cases ("",mystery|..., etc.) still fall through to thedefaulterror branch. The test is correct (it intentionally omits the already-coveredllamaswap).model.go:84-87) holds for these code paths: with an explicitGADFLY_BASE_URL(or endpoint base URL), the wire scheme comes from the URL verbatim, so all four spellings produce an identical provider. There is no enforced http-vs-https difference here — but that is the PR's stated intent and is consistent with howforeman/ollamaalready treat the base URL as authoritative. Not a bug.model.go:111,:259) and the doc comment are cosmetic and don't affect control flow.Could not verify (dependency-internal, outside the sandbox):
LLM_*DSN path and the no-BASE_URLmajordomo.Parse("llama-swap/model")path resolvellama-swap/llama-swapscorrectly depends entirely on the bumped majordomo (…225659). WhenGADFLY_BASE_URLis empty,resolveModelbypasses these switches and callsmajordomo.Parsewith the provider prefix verbatim (model.go:66), so aGADFLY_PROVIDER=llama-swapwith no base URL relies on majordomo recognizing that exact provider name. I couldn't read the majordomo module to confirm, but this is dependency behavior, not a defect in the diff.Nothing in the correctness lane is materially wrong with this change.
Outside my lens (one line): the new README row and the accepted-spellings in code now diverge slightly — the table documents only
llama-swap/llama-swapswhile the code still accepts the un-hyphenatedllamaswap/llamaswaps; worth a docs-lens glance for completeness.🧯 Error handling & edge cases — No material issues found
VERDICT: No material issues found
I reviewed this change through the error-handling & edge-cases lens, verifying against
cmd/gadfly/model.go(the two switch statements at lines 70–113 and 222–259) and the surrounding error paths.Findings:
switch providercases to also match"llamaswaps","llama-swap","llama-swaps"alongside the existing"llamaswap". No error path, cleanup, or boundary logic is altered. Thedefault:arms still return an error (no panic, no fall-through), and the malformed-spec guards inendpointProvider(model.go:208-220— too few parts, empty base URL) remain intact and ahead of the switch."llamaswap"case, including theif key != ""/if apiKey != ""guards.resolveModel(model.go:47-49) logs and skips them viaregisterEnvProvidersrather than aborting — unchanged and still correct.One observation I checked and concluded is not a new bug:
providerisTrimSpace'd but not lowercased in either function, so a mixed-caseLlama-Swapwould still hitdefault. This is pre-existing behavior shared by every provider (ollama, openai, etc.), not something this diff introduces, so it's out of scope as a finding here.Nothing in the error-handling / edge-case lane is materially wrong.
Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 3m 5s