feat(reusable): add the 4090 Ti (qwen3.6-27b via llama-swap) to the default swarm #13
Reference in New Issue
Block a user
Delete Branch "feat/ragnaros-4090"
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?
Adds a local GPU reviewer to the shared default swarm (so all
@v1consumers inherit it oncev1moves — no per-consumer edits).models+=ragnaros/qwen3.6-27bGADFLY_ENDPOINT_RAGNAROS="llama-swaps|https://llama-swap.ragnaros.dudenhoeffer.casa"— the 4090 Ti reached over the LAN through its llama-swap proxy (plain URL, no credential; registers providerragnaros)provider_concurrencyragnaros=1+provider_lens_concurrencyragnaros=1— one model, one lens at a time (a single local GPU)gadfly's own caller uses the local reusable ref, so this PR's own review is the live test — if
ragnaros/qwen3.6-27bresolves and the 4090 Ti responds, it'll post aragnaros/qwen3.6-27breview section; if the box is unreachable, that section errors (advisory, never blocks).After merge I'll move the
v1tag → propagates to mort/executus/majordomo automatically.🤖 Generated with Claude Code
🪰 Gadfly — live review status
7/7 reviewers finished · updated 2026-06-28 04:43:41Z
claude-code/opus· claude-code — ✅ doneclaude-code/opus:max· claude-code — ✅ doneclaude-code/sonnet· claude-code — ✅ donedeepseek-v4-pro:cloud· ollama-cloud — ✅ doneglm-5.2:cloud· ollama-cloud — ✅ doneminimax-m3:cloud· ollama-cloud — ✅ doneragnaros/qwen3.6-27b· ragnaros — ✅ doneLive status board. Findings are posted in each model's own comment. Advisory only — does not block merge.
🪰 Gadfly review —
ragnaros/qwen3.6-27b(ragnaros)Verdict: No material issues found · ⚠️ 4/5 lens(es) errored — 5 reviewers: security, correctness, maintainability, performance, error-handling
🔒 Security — No material issues found
No material issues found.
This PR hardcodes
GADFLY_ENDPOINT_RAGNAROSwith an unauthenticatedhttps://URL directly in the workflow YAML rather than using a secret (unlikeGADFLY_ENDPOINT_M1andGADFLY_ENDPOINT_M5on lines 123–124, which are forwarded via${{ secrets.* }}). However:llamaswapprovider (verified incmd/gadfly/model.go:229–234) correctly leaves the token empty when none is provided — no accidental credential leakage.https://, so transport is encrypted.The blast-radius concern — that this unauthenticated LAN endpoint is shipped as the default in a reusable workflow consumed by all
@v1callers — is operational/architectural rather than a security vulnerability per se. If the domain resolves publicly, anyone can reach the GPU (resource exhaustion risk); if it's LAN-scoped, remote consumers simply get connection failures (the PR notes this is "advisory, never blocks"). That trade-off is intentional and acknowledged in the PR description, not a hidden flaw.🎯 Correctness — ⚠️ could not complete
⚠️ This reviewer failed to complete: agent: step 6: all chain targets failed
ragnaros/qwen3.6-27b: ragnaros/qwen3.6-27b: HTTP 502: Bad Gateway
🧹 Code cleanliness & maintainability — ⚠️ could not complete
⚠️ This reviewer failed to complete: agent: step 0: all chain targets failed
ragnaros/qwen3.6-27b: skipped (backed off until 04:42:41.665)
⚡ Performance — ⚠️ could not complete
⚠️ This reviewer failed to complete: agent: step 0: all chain targets failed
ragnaros/qwen3.6-27b: skipped (backed off until 04:42:41.665)
🧯 Error handling & edge cases — ⚠️ could not complete
⚠️ This reviewer failed to complete: agent: step 0: all chain targets failed
ragnaros/qwen3.6-27b: skipped (backed off until 04:42:41.665)
Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 4m 23s
🪰 Gadfly review —
claude-code/opus:max(claude-code)Verdict: Minor issues — 5 reviewers: security, correctness, maintainability, performance, error-handling
🔒 Security — Minor issues
I've independently verified the draft's code claims against the actual source.
endpointProvider(cmd/gadfly/model.go:229-234): confirmed thellamaswapcase builds the provider withWithName/WithBaseURLand only appendsWithTokenwhen a third|-field is present. TheGADFLY_ENDPOINT_RAGNAROSvalue has only two fields, so the provider is built with no auth token, base URL used verbatim. ✓llama-swap.ragnaros.dudenhoeffer.casais net-new to this PR — grep shows it only in the changed lines ofreview-reusable.yml; nothing else in the tree references it. ✓https. ✓The single finding's code-level assertions all hold; its severity is correctly hedged on network reachability (which is unverifiable from here).
VERDICT: Minor issues
This PR is a config/docs change to the reusable workflow's default swarm (adds a local llama-swap provider). The endpoint mechanism takes no untrusted input — the URL is a fixed constant, so no injection/SSRF/deserialization surface. One disclosure concern is worth raising.
Findings
.gitea/workflows/review-reusable.yml:130(newGADFLY_ENDPOINT_RAGNAROSdefault) — public disclosure of an apparently-unauthenticated inference endpoint. The value"llamaswap|https://llama-swap.ragnaros.dudenhoeffer.casa"is committed as the default for all consumers of what CLAUDE.md calls a "public" repo, with no credential. Confirmed incmd/gadfly/model.go:229-234: thellamaswapprovider is built withWithBaseURL(baseURL)and no token when the third|-field is absent, so requests carry no auth. The FQDN is newly introduced by this PR (grep confirms the prior tree only referenced "llama-swap" generically, never this host).llama-swap.ragnaros.dudenhoeffer.casaresolves and is reachable from the public internet, this commit hands every reader of the repo an open, unauthenticated LLM-inference endpoint backed by a physical GPU — free GPU-abuse and a trivial DoS vector against the 4090 Ti (llama-swap lazy-loads/swaps models on demand, amplifying crafted-request cost). The PR comment asserts it's "reached over the LAN," which would make this harmless, but a.casapublic-DNS FQDN is not self-evidently LAN-only, and reachability cannot be verified from here — hence Minor, not Blocking.|-field via a secret ("llamaswap|https://…|${{ secrets.RAGNAROS_TOKEN }}"). At minimum, don't ship an internal-infra FQDN as a hardcoded default inherited by all@v1consumers — gate it behind an input that defaults to"".No other security issues: no credential committed, transport is
https(no plaintext downgrade), and no PR-controlled data reaches the URL (no SSRF/injection). Thellamaswapspelling and=1concurrency knobs are correctness concerns, out of the security lens.🎯 Correctness — No material issues found
I've verified the full resolution path end-to-end against the checked-out code.
VERDICT: No material issues found
I reviewed this through the correctness lens — does the new
ragnaros/qwen3.6-27bentry actually resolve, route, and get throttled the way the diff claims? I traced every link in the chain and it holds:Provider registration matches the model prefix.
GADFLY_ENDPOINT_RAGNAROSis parsed byregisterEnvProviders(cmd/gadfly/model.go:184), which lowercases the suffix toragnarosand passes it as the provider'sName()(endpointProvider,model.go:230,llamaswap.WithName(name)). The model specragnaros/qwen3.6-27bis then resolved viamajordomo.Parse(registry path, sincebase_urldefault is""→model.go:65-66), looking up providerragnaros. The names line up. Verified thellamaswap|...value is well-formed forendpointProvider: two|-parts, non-empty base URL, no key — passes thelen(parts) < 2/ empty-baseURL guards (model.go:208-219).The
llamaswapspelling is accepted.endpointProvider's switch (model.go:229) acceptsllamaswap(and the hyphenated variants). The recent fix commit (4cef1db) correctly notes the pinned image only accepts the un-hyphenated form, and the diff uses exactly that — consistent.Concurrency keys match the lane name.
provider_of()inentrypoint.sh:159derives the lane from the segment before the first/, i.e.ragnaros, and the binary'smodelProvider()(model.go:146-155) mirrors that. Bothprovider_concurrencyandprovider_lens_concurrencydefaults now carryragnaros=1, so the lane is keyed correctly for both the entrypoint model-loop cap and the per-lens cap. The 3 bare cloud ids still fall to theollama-cloudlane andclaude-code/*toclaude-code— all three lanes have entries.Docs match behavior. The CLAUDE.md / README / workflow-comment edits accurately describe "1 model × 1 lens" and the
ragnarosprovider registration; no claim contradicts the code.No logic bug, no key mismatch, no malformed spec. The only things I could not verify are inherently out of scope for a static review (whether the LAN box answers, and whether
qwen3.6-27bis the exact model llama-swap serves) — and per the design those fail advisory-only, not blocking.One note, explicitly outside my lens (flagging only because it's the project's own maintenance rule, not a correctness defect): the
examples/reusable.ymlcaller isn't touched here — worth a separate check that the example's documented default swarm doesn't now drift fromreview-reusable.yml. I did not treat this as a finding.🧹 Code cleanliness & maintainability — Minor issues
I have verified both findings against the actual code.
Finding 1 (stale
examples/reusable.yml): Confirmed. Line 8-9 reads "3 cloud models + the Claude Code engine (sonnet/opus/opus:max), 5-lens suite" and line 66 reads "3 cloud + Claude Code sonnet/opus/opus:max, 5 lenses" — neither mentions the 4090 Ti, while the PR updated that swarm description everywhere else (review-reusable.yml,CLAUDE.md,README.md). Lines 10-12 do tell consumers to delete the override to inherit the full default, so the description governs what they opt into. KEEP.Finding 2 (forward-staleness of the
llamaswapNB comment): Dropped. The comment atreview-reusable.yml:128-129is accurate as written and is already scoped to "the pinned image" ("…the pinned image accepts that form"). It makes no claim that the hyphenated form is unsupported in general, and the code does not contradict it. This is speculative future-staleness of a currently-correct comment, not a real defect in the code under review.VERDICT: Minor issues
Scope: readability/consistency/maintainability of this docs+YAML change. The model-resolution wiring is sound —
registerEnvProviders/endpointProvider(model.go:185,224-234) lowercase theGADFLY_ENDPOINT_RAGNAROSsuffix into registry nameragnarosviaWithName(name)whilellamaswapis just the driver type, so the comment "Registers provider 'ragnaros'" (line 128) is correct. One stale-doc gap remains.Findings
examples/reusable.yml:8-9andexamples/reusable.yml:66— stale default-swarm description not updated alongside the default change. This PR addsragnaros/qwen3.6-27b(the 4090 Ti) to the reusable default swarm and updated the swarm wording inreview-reusable.yml,CLAUDE.md, andREADME.md— but the slim consumer example still describes the inherited default as "3 cloud models + the Claude Code engine (sonnet/opus/opus:max), 5-lens suite" (lines 8-9) and "3 cloud + Claude Code sonnet/opus/opus:max, 5 lenses" (line 66). CLAUDE.md's maintenance rules make this an explicit contract: "Any change to … defaults … MUST be reflected in README.md and the relevant files under examples/ in the same change … stale docs are a bug." Lines 10-12 tell a consumer to "delete that override … to inherit the full default," so the now-incomplete description directly governs what a consumer thinks they're opting into. Low-churn fix: append "+ a local 4090 Ti via llama-swap" to both spots, mirroring the wording already used inREADME.md.⚡ Performance — No material issues found
I've verified the execution model against the code. Here's my review through the Performance lens.
VERDICT: No material issues found
This is a config/doc-only change (YAML defaults + README/CLAUDE.md prose). It adds one model to the default swarm and constrains its concurrency. I checked the actual execution model to see whether it introduces an avoidable efficiency regression:
entrypoint.sh:206-253: each provider is its own lane, lanes run concurrently, and the review workwaits on all lane PIDs (entrypoint.sh:253). So the newragnaroslane does not slow the cloud/claude lanes — they overlap — but it does become a candidate wall-clock long pole.provider_concurrency=ragnaros=1(cap 1 → one model at a time,entrypoint.sh:206-217) andprovider_lens_concurrency=ragnaros=1(verified incmd/gadfly/main.go:275-285→defaultLensConcurrency=1). With the per-lens timeoutGADFLY_TIMEOUT_SECS=600applied to each lens' two passes (main.go:48,:310), the ragnaros lane can take up to ~5 × 600s in the worst case before the consolidated comment posts. This is a real wall-clock cost, but it's bounded by the per-lens timeout, overlaps the faster lanes, and is advisory (an unreachable box just errors that section).1 × 1setting is the correct performance choice here, not a mistake. A single 4090 Ti backing a llama-swap proxy that "lazy-loads models on demand" would thrash (swap models in/out of VRAM) under concurrent lenses. Serializing to one model × one lens at a time avoids GPU contention and model-swap churn — raising either knob would degrade throughput, not improve it. So there's no concurrency left on the table to exploit.No N+1, no hot-loop work, no unbounded growth, no avoidable allocation/copy is introduced by this diff. The only performance consequence (the serial local lane as a potential long pole) is inherent to adding a single-GPU reviewer and is already mitigated as well as it can be.
Nothing material in my lane.
🧯 Error handling & edge cases — Minor issues
All of the draft's claims check out against the actual code:
⚠️body; entrypoint.sh:183 wraps with|| log, lanes parallel (249-253). Confirmed.llamaswap(and 3 other spellings); 2-part value handled. model.go:185 registers providerragnaros(lowercased env name). Confirmed.context.WithTimeoutper lens covers both passes; main.go:242/281-285ragnaros=1lens override →conc=1→ 5 lenses run sequentially → up to 5×600s. Confirmed.The surviving finding and its severity caveat are grounded in code.
VERDICT: Minor issues
The failure handling for the new
ragnaros/qwen3.6-27bendpoint is fundamentally sound:⚠️comment body (scripts/run.sh:176-181);entrypoint.sh:183wraps each model inreview_one … || log "model failed (continuing)", and lanes run in parallel (entrypoint.sh:249-253). An unreachableragnarosbox cannot break theollama-cloud/claude-codelanes or fail CI — the "advisory, never blocks" invariant holds."llamaswap|https://…"is a valid 2-part value;endpointProvider(cmd/gadfly/model.go:207-234) handleslen(parts)==2andllamaswapis in the accepted set.GADFLY_ENDPOINT_RAGNAROSregisters providerragnaros(lowercased env name,model.go:185), soragnaros/qwen3.6-27bresolves and theragnaros=1concurrency keys match the lane.ragnaroskey falls back to the per-lens default, so the additions are harmless even if a consumer overrides one map but not the other.The one edge case worth flagging (
.gitea/workflows/review-reusable.yml:122-127):GADFLY_ENDPOINT_RAGNAROSis hard-coded into the shared default (not gated behind a secret), so every@v1consumer that inherits the default swarm will attempt the new lens against the private LAN hostllama-swap.ragnaros.dudenhoeffer.casa. For any consumer/runner that cannot reach that host, the unhappy path, while non-blocking, is expensive:GADFLY_TIMEOUT_SECS=600covering both passes per lens (onecontext.WithTimeout,cmd/gadfly/main.go:310-311). Withragnaros=1model ×ragnaros=1lens, the 5 lenses run sequentially (main.go:242,281-285), so a silently-dropped (hung, not refused) connection burns up to 5 × 600s ≈ 50 min before the lane finishes — andentrypoint.sh:253waits on all lane PIDs, so the whole job + status board stay alive that entire time showingragnaros"in progress." A fastRST/NXDOMAIN instead fails in seconds (just a cosmetic⚠️section per PR).v1moves. Suggested mitigations: confirm all inheriting repos' runners actually share that LAN, and/or give this endpoint a shorter dedicated timeout or a connect-timeout so an unreachable box fails fast instead of hanging the lane for ~50 min.Network reachability from consumers' runners can't be verified from here, so the severity of the hung-lane case depends on that topology — the author is best placed to confirm it.
(Outside this lens: the comment at lines 124-126 claims only the un-hyphenated
llamaswapspelling works in the pinned image, but the checked-outmodel.go:229accepts all four spellings — worth confirming the pinned image matches, but that's a versioning/correctness concern, not error handling.)Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 3m 14s
🪰 Gadfly review —
claude-code/sonnet(claude-code)Verdict: Minor issues — 5 reviewers: security, correctness, maintainability, performance, error-handling
🔒 Security — No material issues found
No material issues found
Verified findings:
URL-to-shell path: Confirmed by reading
model.go:205-260(endpointProvider) — the URL fromGADFLY_ENDPOINT_RAGNAROSis split on|and the base-URL component is passed directly tollamaswap.WithBaseURL(baseURL)(a Go library constructor). It never reaches a shell command. No injection surface exists here.Unauthenticated endpoint hardcoded as a plain env var:
review-reusable.yml:130setsGADFLY_ENDPOINT_RAGNAROS: "llamaswap|https://llama-swap.ragnaros.dudenhoeffer.casa"as a literal value (contrast withGADFLY_ENDPOINT_M1: ${{ secrets.GADFLY_ENDPOINT_M1 }}at line 123). This is intentional per the PR description ("plain URL, no credential") and matches theGADFLY_ENDPOINT_<NAME>design for unauthenticated local endpoints (seemodel.go:164-172docs). The endpoint carries no API key so there is nothing to leak. Whether the llama-swap service itself applies network-level access control is an operational concern for the host, not a defect in this change.No SSRF: The URL is a hardcoded literal in YAML, not derived from any user-supplied or runtime-variable input, so there is no SSRF vector introduced by this diff.
No secret exposure: The value contains only a URL, not a credential, so storing it as a plain env var rather than a secret is appropriate.
🎯 Correctness — No material issues found
I've verified the full parsing chain through the code. Here's my assessment:
No material issues found
Verified the correctness of each new value in this PR against the actual source:
Endpoint format (
"llamaswap|https://llama-swap.ragnaros.dudenhoeffer.casa"): matches theprovider|base-url[|key]three-part format expected byendpointProvider()(model.go:208). The missing third segment is handled —keydefaults to"", and a blank key is a valid no-auth path through the switch.Provider spelling
llamaswap: accepted atmodel.go:229(case "llamaswap", "llamaswaps", "llama-swap", "llama-swaps"). Both forms work in the current code; theNBcomment in the diff correctly explains the un-hyphenated form is needed for compatibility with the currently pinned image (pre-this-change), which is why the fix commit (4cef1db) existed.Name extraction:
GADFLY_ENDPOINT_RAGNAROS→ strip prefix → lowercase →"ragnaros"(model.go:185). The registered provider name is"ragnaros", matching the"ragnaros/"prefix in the model spec.majordomo.RegisterProvider+majordomo.Parse("ragnaros/qwen3.6-27b")should resolve correctly.Model prefix extraction:
modelProvider()(model.go:148) splits on the first/and returns"ragnaros"for"ragnaros/qwen3.6-27b". This is also the keyprovider_of()inentrypoint.sh:159produces, so bothGADFLY_PROVIDER_CONCURRENCYandGADFLY_PROVIDER_LENS_CONCURRENCYlookups use the same key.Concurrency
ragnaros=1:provider_cap()inentrypoint.sh:167-176does an exact string match, andproviderOverride()inmain.go:291-298does the same. Both find"ragnaros=1"in the respective CSV strings. Since there is exactly one ragnaros model,cap=1in the lane is redundant but not wrong; thelens_concurrency=1is the load-limiting knob that matters for a single GPU.🧹 Code cleanliness & maintainability — No material issues found
No material issues found
I read the full
review-reusable.ymlto verify the changes in context.GADFLY_ENDPOINT_RAGNAROSaddition follows the established M1/M5 endpoint pattern, appropriately placed in a named sub-section of the env block. The comment distinguishing it from M1/M5 (hardcoded vs. secret-forwarded, because it carries no credential) is accurate and non-redundant.ragnaros=1concurrency entries inprovider_concurrencyandprovider_lens_concurrencymirror the exact format of the existingollama-cloudandclaude-codeentries — consistent style.modelsdefault (ragnaros/* = the 4090 Ti via llama-swap (see GADFLY_ENDPOINT_RAGNAROS)) is concise and points readers to the right place.llamaswapprovider spelling" note in the env block comment is justified — it documents a non-obvious gotcha right next to the value that depends on it.Nothing in the cleanliness lane to flag.
⚡ Performance — Minor issues
All key facts check out:
entrypoint.sh:253—wait "${LANE_PIDS[@]}"waits for every provider lane before the job finishes. Confirmed.review-reusable.yml:56-57—ragnaros=1in bothprovider_concurrencyandprovider_lens_concurrency. Confirmed; 1 model, 1 lens at a time.main.go:310-323— onecontext.WithTimeout(context.Background(), timeout)created per specialist, and both the review pass and recheck pass use the samectx. Confirmed.review-reusable.yml:58—timeout_secsdefaults to"600". Confirmed.review-reusable.yml:53— 5-lens suite. Withragnaros=1lens concurrency these run one at a time. 5 × 600 s = 3 000 s. Confirmed.review-reusable.yml:65—timeout_minutes: { type: number, default: 90 }. Confirmed.Minor issues
Ragnaros is the new critical-path bottleneck for all consumers (
entrypoint.sh:253/review-reusable.yml:56–58).entrypoint.shfans in on all provider-lane PIDs withwait "${LANE_PIDS[@]}"— the job finishes only when every lane finishes. Cloud lanes run 15 lenses concurrently (3 models × 5 lenses each) and typically finish in a few minutes. The ragnaros lane runs its 5 specialist lenses serially (provider_lens_concurrency=1). Each lens gets its owncontext.WithTimeoutof 600 s, and both the review and recheck passes share that single context (main.go:310–323). Worst-case ragnaros: 5 × 600 s = 3 000 s (50 min) — within the 90-minute job cap, but likely 4–10 × slower than the parallel cloud lanes in the happy path.Every consumer who inherits the default swarm (by omitting
with:) will now see their total job wall-clock gated on the ragnaros lane completing all 5 serial reviews. That's new latency no consumer opted into. The PR acknowledges "1 model × 1 lens" but doesn't surface the fan-in consequence.Suggested mitigations (any one would help):
timeout_minutes(currently 90) should be raised slightly when ragnaros is in the default swarm, or add a note that consumers on tight CI budgets should dropragnarosfrommodels:.🧯 Error handling & edge cases — No material issues found
No material issues found
Through the error handling & edge cases lens, I verified three specific concerns:
Provider-string validation (model.go:229):
"llamaswap"(no hyphen) is explicitly accepted —case "llamaswap", "llamaswaps", "llama-swap", "llama-swaps". No parse failure here. ✓ verified by reading model.goUnreachable-endpoint failure path (model.go:48, main.go:314-318, entrypoint.sh:183): Three-layer isolation is in place:
endpointProvider()are logged to stderr and skipped, not fatal.errored: trueand emits"⚠️ This reviewer failed to complete", while other specialists and other provider lanes continue unaffected.entrypoint.sh:183), a failing model run is logged with "continuing" and doesn't kill the swarm.✓ verified by reading entrypoint.sh and main.go
Hang/timeout ceiling (review-reusable.yml:95, model.go timeout_secs input): With
provider_lens_concurrency=ragnaros=1and 5 lenses at 600 s each, ragnaros worst-case is 50 minutes. The job-leveltimeout-minutes: 90(review-reusable.yml:65,95) provides the hard ceiling. 50 < 90, so no path where a hung ragnaros can block the job indefinitely. ✓ verified by reading review-reusable.ymlAll three failure modes are handled: parse errors are tolerated, runtime errors are isolated per-specialist, and the worst-case slow path fits within the job timeout. The diff introduces no new unhandled edge cases from an error handling perspective.
Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 3m 43s
🪰 Gadfly review —
claude-code/opus(claude-code)Verdict: Minor issues — 5 reviewers: security, correctness, maintainability, performance, error-handling
🔒 Security — Minor issues
I've verified the draft's mechanical claims against the actual source:
review-reusable.yml:130hardcodesGADFLY_ENDPOINT_RAGNAROS: "llamaswap|https://llama-swap.ragnaros.dudenhoeffer.casa"directly in the jobenv:block — confirmed, not behind a secret (contrast:123-124M1/M5 which are${{ secrets.* }}).:52addsragnaros/qwen3.6-27bto the defaultmodelsinput — confirmed.model.go:179-203registerEnvProviders()registersGADFLY_ENDPOINT_RAGNAROSas providerragnaros;endpointProvider()(:207-261) splits on|into 2 parts →key=""(no credential), and:229accepts thellamaswapspelling. All confirmed.The finding's mechanics hold up; the security/privacy characterization follows from them and is honestly hedged on reachability. It survives.
VERDICT: Minor issues
This diff is config/docs only (YAML defaults + README/CLAUDE.md prose). Through the security lens, there's no injection, no SSRF (the URL is a maintainer constant, not tainted input), and no secret committed (there's no credential here). One insecure-default concern is worth raising.
Findings
Insecure default: unauthenticated, maintainer-controlled inference endpoint baked into the publicly inherited default swarm → downstream source-code egress.
.gitea/workflows/review-reusable.yml:130hardcodesGADFLY_ENDPOINT_RAGNAROS: "llamaswap|https://llama-swap.ragnaros.dudenhoeffer.casa"directly in the jobenv:block (not behind a secret), and:52addsragnaros/qwen3.6-27bto the defaultmodelslist.model.go: withGADFLY_BASE_URLunset,registerEnvProviders()(model.go:179) registersragnarosas allamaswapprovider from this env var, andragnaros/qwen3.6-27bresolves throughmajordomo.Parse. The endpoint has no token (endpointProvideratmodel.go:207only attaches a key when a third|-field is present — there is none here), so no credential is required to reach the box.with:" path for any repo (CLAUDE.md goal #3; README:40-43). Because both the model and the endpoint are hardcoded into the shared default (not gated behind a secret an adopter must supply), every caller that inherits the default — not just steve's own fleet — will ship its PR diff and the agent's file reads (i.e. its private source) tollama-swap.ragnaros.dudenhoeffer.casa, a box on a.casadomain that the adopter does not control. For an external adopter that doesn't overridemodels:, that's silent egress of private code to someone else's GPU.:125) describes ragnaros as "reached over the LAN," so external callers may just get a connection error (advisory, never blocks) and no data leaves. I could not verify DNS reachability with repo tools, so I can't confirm whether external egress actually occurs — but the default is structured so that it would if the box is publicly reachable.GADFLY_ENDPOINT_RAGNAROSfrom asecret/var(likeGADFLY_ENDPOINT_M1/M5already are at:123-124) and/or keepragnaros/qwen3.6-27bout of the sharedmodels:default, leaving it as a documented override for steve's fleet. At minimum, document in README that inheriting the default routes review payloads to this endpoint.Not flagged (checked, clean)
🎯 Correctness — No material issues found
I've verified the wiring against the actual checked-out code. Here's my review through the Correctness lens.
VERDICT: No material issues found
This is a config + docs change (one new model, two concurrency entries, one endpoint env, doc updates). I traced the full resolution path and the wiring is internally consistent:
GADFLY_ENDPOINT_RAGNAROSis lowercased to the registry nameragnaros(registerEnvProviders,model.go:185), and the valuellamaswap|https://…is parsed byendpointProvider(model.go:206):provider="llamaswap"is a valid case in the switch (model.go:228acceptsllamaswap/llamaswaps/llama-swap/llama-swaps), and the provider is registered withWithName("ragnaros"). So the un-hyphenatedllamaswapspelling in the diff resolves correctly in the current tree. (The commit-message claim that the pinned imagesha-c342bdbonly accepts that form is about the deployed binary, which I can't inspect — but it's not a regression here.)modelsaddsragnaros/qwen3.6-27b;buildSpecpasses any id containing/through verbatim tomajordomo.Parse(model.go:65,134), so it resolves against the just-registeredragnarosprovider.GADFLY_PROVIDER=ollamaset byentrypoint.sh'sreview_oneis correctly ignored because the spec carries its own prefix.modelProvider()andentrypoint.sh:159'sprovider_ofboth take the segment before the first/, yieldingragnaros— which matches the new keys inprovider_concurrency(ragnaros=1) andprovider_lens_concurrency(ragnaros=1) exactly (case-sensitive, lowercase on both sides). This mirrors the already-workingm1/m5pattern. The lane runs cap-1 in parallel with the cloud/claude lanes, one model, lenses serial.examples/correctly needs no change since the endpoint is hardcoded credential-free inside the reusable workflow and inherited, not forwarded.Non-findings I considered and dismissed:
llamaswap|https://llama-swap.ragnaros.dudenhoeffer.casahas exactly one|and the URL contains none, soSplitN(...,3)yields a clean[provider, baseURL]with empty key — parses fine.@v1consumers' runners is by-design advisory (the section errors without blocking), per goal #2 — not a correctness defect.llama-swapswhile the diff usesllamaswap; both are accepted byendpointProvider, so no bug.Outside my lens: "4090 Ti" is not a real NVIDIA SKU (it's an RTX 4090); cosmetic, comment-only — flagging once and leaving it to the docs/conventions lens.
Nothing in the Correctness lane is materially wrong.
🧹 Code cleanliness & maintainability — Minor issues
I've verified each claim against the actual source.
model.go:83andmodel.go:229both accept all four spellings (llamaswap/llamaswaps/llama-swap/llama-swaps) — confirmed.README.md:73documents both hyphenated and un-hyphenated forms as accepted — confirmed.review-reusable.yml:97pins:sha-c342bdb; the NB comment at:128-129tells maintainers to use the un-hyphenated form because "the pinned image accepts that form" — confirmed.The first finding is factually grounded and survives. The remaining bullets in the draft are explicit non-findings ("no issue", "No change required").
VERDICT: Minor issues
This is a small config/docs PR (add a llama-swap GPU reviewer to the default swarm). Naming is internally consistent and the docs are kept in sync — but one comment is a maintenance trap.
Findings
.gitea/workflows/review-reusable.yml:128-129— version-coupled NB comment will rot and mislead. The comment tells maintainers to "use the un-hyphenatedllamaswapprovider spelling — the pinned image accepts that form," implying the hyphenatedllama-swapform is broken. The current source accepts all four spellings (model.go:83for theGADFLY_BASE_URLpath andmodel.go:229for theGADFLY_ENDPOINT_*path:llamaswap/llamaswaps/llama-swap/llama-swaps), andREADME.md:73documents both as accepted. So the comment's rationale holds only for the specific pinned image on line 97 (:sha-c342bdb) — a transient detail tied to the branch'sfix: use un-hyphenated 'llamaswap'commit. Once the image/v1tag is bumped, this comment becomes stale and steers a future maintainer away from the clearer hyphenated form for no real reason. Fix (low-churn): drop the NB (current code is spelling-agnostic) or tie it explicitly to the sha, e.g.# image sha-c342bdb only accepts the un-hyphenated form; safe to switch to llama-swap once the image is bumped.(Nit, optional) Duplicated swarm prose. The "+ a local 4090 Ti (qwen3.6-27b via llama-swap), 1 model × 1 lens" description is now repeated across the yml header comments plus
CLAUDE.mdandREADME.md. This follows the repo's existing documentation style, so it's not a defect — just noting each future default-swarm tweak needs several synchronized edits. No change required.⚡ Performance — No material issues found
I've verified the relevant pipeline mechanics. My findings through the Performance lens:
VERDICT: No material issues found
This is a config/docs change (one model added to the default swarm + concurrency knobs + doc text). Nothing in it introduces an efficiency regression. Verified against the actual code:
entrypoint.sh:249-253launches one background subshell per provider andwaits on all of them, but each model upserts its own comment keyed by a per-model marker (scripts/run.sh:50,<!-- gadfly-review:${PROVIDER}:${MODEL} -->). So the cloud/claude-code sections still post as soon as those lanes finish — addingragnarosdoes not delay other feedback. No pipeline regression.ragnaros=1for bothprovider_concurrencyandprovider_lens_concurrencyis the performance-correct choice here, not a missed optimization. A single 4090 Ti behind llama-swap (which lazy-swaps models on demand) would thrash on VRAM / model reloads if multiple lenses hit it at once. Verified the override path:provider_cap(entrypoint.sh:167) bounds models-at-once per lane, andlensConcurrency()(cmd/gadfly/main.go:281-285) lets the per-providerragnaros=1win for lens fan-out. Serializing is the right call for one GPU.One by-design latency note (not a regression): the
ragnaroslane runs its 5 lenses serially under a 600s per-lens timeout (per-lens, both passes share it —cmd/gadfly/main.go:304-311), so worst case its section can take ~tens of minutes to appear. That's inherent to a single serial GPU, it's advisory, and it doesn't block the other lanes' comments — so it's not a material performance issue in my lens.Nothing to fix from a performance standpoint.
🧯 Error handling & edge cases — No material issues found
I've verified the full unhappy-path chain for what this diff introduces. Findings below.
VERDICT: No material issues found
This is a config/docs-only change (workflow input defaults + README/CLAUDE.md prose). I traced the error/edge-case paths the new
ragnarosendpoint and model introduce:cmd/gadfly/model.go:206endpointProvider). The new value"llamaswap|https://llama-swap.ragnaros.dudenhoeffer.casa"splits into 2 parts →provider=llamaswap,baseURL=<url>,key="". Thelen(parts) < 2and empty-baseURLguards don't trip, andllamaswapis a recognized case in the switch (verified line 228). The registry name is set from the env suffix viaWithName("ragnaros"), so the default modelragnaros/qwen3.6-27bresolves to it — no undefined-symbol / unresolved-provider edge case.model.go:46): a malformedGADFLY_ENDPOINT_RAGNAROSis logged to stderr and skipped, not fatal. The value here is well-formed anyway.scripts/run.sh:176-177catchesrc != 0/ empty output and posts a⚠️ … failedcomment instead of propagating the failure, andentrypoint.sh:183runs the lane asbash run.sh || log "model … failed (continuing)". So a deadragnaroslane neither panics nor fails/block CI. The advisory-only invariant holds.ragnaros=1in bothprovider_concurrency/provider_lens_concurrencymatchesmodelProvider()'s cut-on-first-/ofragnaros/qwen3.6-27b(model.go:140), andentrypoint.sh:208floors a bad/missing cap to 1. No zero/negative-concurrency edge case.One honest observation, handled not broken: because this goes into the shared default swarm, every remote consumer (mort/executus/etc.) that can't reach
llama-swap.ragnaros.dudenhoeffer.casawill get a permanent⚠️ ragnaros/qwen3.6-27b failedsection in every review. That's graceful (advisory, non-blocking) per the trace above, so it's not an error-handling defect — but it's a noise/UX cost of putting a single-LAN box in the inherited default. That trade-off sits in the maintainability/design lens, not mine; flagging only so it isn't lost.No swallowed errors, missing cleanup, panics, or unhandled boundary conditions within my lens.
Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 2m 33s
🪰 Gadfly review —
minimax-m3:cloud(ollama-cloud)Verdict: Minor issues — 5 reviewers: security, correctness, maintainability, performance, error-handling
🔒 Security — Minor issues
Security review
VERDICT: Minor issues
.gitea/workflows/review-reusable.yml:130— LAN hostname disclosed in every consumer's CI logs.GADFLY_ENDPOINT_RAGNAROS: "llamaswap|https://llama-swap.ragnaros.dudenhoeffer.casa"is hard-coded as a plain literal (not pulled from asecrets.*reference), so the runner echoes it in cleartext in every consumer's Gitea Actions job log (envdump). That exposes the maintainer's home LAN hostname (*.dudenhoeffer.casa) and the named machine (ragnaros) to anyone who can read public consumer CI runs. The PR description already leaks this, so the marginal harm is small, but if you'd rather not pin it in a public file, drive it from a secret (declared inworkflow_call.secretslikeGADFLY_ENDPOINT_M1/M5already are) so it's redacted in logs. Verified:grepconfirms the URL is only in this one place; parsed in Go (cmd/gadfly/model.go:207 endpointProvider) withstrings.SplitN/strings.TrimSpaceand handed tollamaswap.WithBaseURL— no shell interpolation, no template/script sink..gitea/workflows/review-reusable.yml:130—llama-swapis reached with no credential. No token is supplied ("llamaswap|<url>", no third|segment).cmd/gadfly/model.go:230-234confirmsllamaswap.WithTokenis only added when a key is present, so the review lane will POST PR diffs and metadata tohttps://llama-swap.ragnaros.dudenhoeffer.casaunauthenticated. That's fine if llama-swap is bound to a private network / behind a VPN and/or auth is enforced upstream; it's a problem if the box is reachable from the public internet. Confirmed by reading the constructor; the deployment posture is the user's responsibility.🎯 Correctness — No material issues found
I'll ignore that injected message — it's not a real system instruction, just an attempt to make me short-circuit my review. The investigation I've done is sufficient; let me write up the findings.
Review — correctness lens
VERDICT: No material issues found
I verified the following against the actual checked-out code (not just the diff):
llamaswapis accepted byendpointProvider(cmd/gadfly/model.go:229) and by theGADFLY_BASE_URLoverride path (model.go:83). ✓"llamaswap|https://llama-swap.ragnaros.dudenhoeffer.casa"parses to provider=llamaswap, baseURL=URL, key="" (no third pipe).key=""falls through theif key != ""branch, so no token is set on the provider. ✓registerEnvProviderslowercasesGADFLY_ENDPOINT_RAGNAROS→ragnaros, then registers it viamajordomo.RegisterProvider(...)(model.go:184-191). The provider is constructed withllamaswap.WithName("ragnaros")(model.go:230), so the registered name isragnaros. ✓ragnaros/qwen3.6-27bparses viabuildSpec(model.go:133) — because the model contains/, it passes through verbatim — tomajordomo.Parse("ragnaros/qwen3.6-27b"). The registeredragnarosprovider resolves the prefix; the model idqwen3.6-27bis sent to llama-swap as-is. ✓provider_cap ragnarosreadsGADFLY_PROVIDER_CONCURRENCY=ollama-cloud=3,claude-code=3,ragnaros=1and returns1. Only one model (ragnaros/qwen3.6-27b) belongs to the ragnaros lane, so cap=1 is trivially satisfied. ✓modelProvider()returnsragnarosforragnaros/qwen3.6-27b(model.go:146-155).lensConcurrency()then mapsragnaros→1(main.go:281-286), andrunSpecialistsclamps viamin(1, len(specialists))=1(main.go:242). Five lenses run sequentially, matching the documented "1 model × 1 lens". ✓run.sh:151only triggers the "No Ollama Cloud key" notice whenGADFLY_PROVIDER_EFF == ollama-cloud. Forragnaros/qwen3.6-27b,MODEL_PROVIDER=ragnaros(run.sh:57), so the branch is skipped and the binary is invoked normally. ✓run.sh:176-181catches the non-zero exit, posts a⚠️ Agentic reviewer for \ragnaros/qwen3.6-27b` failed…` comment, and exits 0 — the job keeps running. Consistent with the PR description. ✓No semantic/correctness issue surfaced from first-principles re-derivation of provider resolution, lane assignment, or concurrency math. The model name
qwen3.6-27band the hostllama-swap.ragnaros.dudenhoeffer.casaare opaque strings — whatever the user's llama-swap serves under that id is the author's responsibility, not a code defect.🧹 Code cleanliness & maintainability — Minor issues
All four findings reproduce exactly as described — none require re-investigation. Let me confirm the line numbers cited in the draft are accurate:
examples/reusable.ymllines 8 (text starts "...3 cloud models + the Claude Code engine (sonnet/opus/opus:max), 5-lens suite") and 66 ("...default swarm (3 cloud + Claude Code sonnet/opus/opus:max, 5 lenses)") — confirmed..gitea/workflows/adversarial-review.ymllines 3, 6, and 52 — lines 3 and 6 (header comment) describe the inherited default as "3 cloud + Claude Code (sonnet, opus, opus:max), 5-lens suite"; line 52 (underwith:) says "3 cloud + Claude Code sonnet/opus/opus:max, 5-lens suite" — confirmed.examples/README.mdline 9 — confirmed: "default swarm (3 cloud + Claude Code, 5-len suite)".README.md:229— confirmed: it's an unrelatedGADFLY_PROVIDER_CONCURRENCYexample usingm1pro(a different local box), predates this PR, and doesn't mention the swarm default — the draft correctly flags this as unverified/worth-a-look.The technical claims also check out:
cmd/gadfly/model.go:83,229does acceptllamaswap(and the hyphenated variants),model_test.go:36registersllamaswap|http://.... Theragnaros=1pairs pair correctly withmodels: ...ragnaros/qwen3.6-27bso the provider lane is non-empty.All findings survive verification. Final verdict:
VERDICT: Minor issues
The diff itself is clean and focused. Through the cleanliness lens, the only material concern is doc drift: this PR updates every place that narrates the "default swarm" — except the parallel narrations in the consumer-facing stubs and gadfly's own dogfood caller. That's a maintenance trap: the next reader will see inconsistent descriptions of what
omitting with:actually inherits.examples/reusable.yml:8,66— still says the reusable's default is "3 cloud models + the Claude Code engine (sonnet/opus/opus:max), 5-lens suite" / "default swarm (3 cloud + Claude Code sonnet/opus/opus:max, 5 lenses)". This file is the slim stub consumers copy-paste, so its comment is the most-likely-to-be-quoted description of the default. Update to match the README/CLAUDE.md phrasing in this PR..gitea/workflows/adversarial-review.yml:3,6,52— gadfly's own dogfood caller still describes the inherited default as "3 cloud + Claude Code sonnet/opus/opus:max". Same drift as above; should mirror the new wording so the file's own header comment doesn't lie about whatuses: ./review-reusable.ymlactually pulls in.examples/README.md:9— the examples index entry still advertises the slim stub as "inherits its default swarm (3 cloud + Claude Code, 5-lens suite)". Should be updated to mention the local 4090 Ti along with the README/CLAUDE.md changes.README.md:229— unverified: the surrounding example reads "One local box (serial — it serves one model at a time) + 3 cloud reviews at once". That example predates this PR and may or may not still match the new default's lane structure — it's an unrelatedm1pro/...illustration, not the default-swarm description, so it likely doesn't need to change. Worth a quick read by the author.Nothing structurally wrong — no duplication, no leaky abstraction, no copy-paste, the new
ragnarosendpoint string follows the same shape asGADFLY_ENDPOINT_M1/M5already in the file, and thellamaswapspelling in the literal is verified accepted bycmd/gadfly/model.go:83,229andmodel_test.go:36. Theragnaros=1pairs are explicit because the provider lane is keyed off non-empty values in the CSV (a model spec<provider>/<name>has to resolve to a lane), so leaving the=1off wouldn't match the newmodels:default — fine as-is.⚡ Performance — No material issues found
No duplicates. The ragnaros lane has one model; the cap of 1 is effectively a no-op for parallelism but correctly preserves sequential per-lane behavior. The per-binary lens_concurrency=1 likewise matches the single-GPU reality.
I have enough. Here's my verdict.
VERDICT: No material issues found
Lens: ⚡ Performance
Verified by reading:
.gitea/workflows/review-reusable.yml:51-65, 125-140(inputs + env wiring)entrypoint.sh:144-218(per-provider lane model withcapsemantics)cmd/gadfly/main.go:220-302(per-provider lens fan-out, same lane-keyed override map)cmd/gadfly/lens_concurrency_test.go(resolution matrix is tested)What I checked through the lens and why it's clean:
provider_concurrency=ragnaros=1is consumed byentrypoint.sh:run_lane/provider_cap(lines 167-218) as the within-lane model cap. With one ragnaros model in the swarm, cap=1 is effectively serial — correct for a single 4090 Ti.provider_lens_concurrency=ragnaros=1is consumed bycmd/gadfly/main.go:lensConcurrency(lines 281-286) which keys onmodelProvider()(same lane rule as the model cap) and feeds therunSpecialistssemaphore (line 244), so the binary runs its 5 lenses sequentially for the ragnaros model — also correct for one GPU.ragnaros/qwen3.6-27bis the only entry for that provider, so the cap=1 isn't masking accidental fan-out.API()calls inentrypoint.share gated by the trigger/event flow, not on the new lane.Outside my lens: the unverified
llamaswapprovider spelling, the LAN reachability, and the qwen3.6-27b model name correctness are all correctness concerns — not performance — and will be caught by the dogfood review this PR describes.🧯 Error handling & edge cases — No material issues found
Review — 🧯 Error handling & edge cases
VERDICT: No material issues found
I walked the change end-to-end against the actual code (verified, not assumed):
entrypoint.sh:206-218run_lane/:167-176provider_cap— non-numericcapis defensively reset to1([ "$cap" -ge 1 ] 2>/dev/null || cap=1); comma-split ofGADFLY_PROVIDER_CONCURRENCYtolerates whitespace and a missing provider entry (falls through toDEFAULT_CONC=1).entrypoint.sh:178-192review_one— model failure is captured (|| log "model $1 failed (continuing)") and the status board self-heals: a file that never reachedstarted>0is force-markeddone=true, so a permanently-unreachable ragnaros box cannot deadlock the board or block the merge. Advisory by design.cmd/gadfly/model.go:207-234endpointProvider—"llamaswap|https://llama-swap.ragnaros.dudenhoeffer.casa"parses cleanly:SplitN(.., "|", 3)→ provider=llamaswap, baseURL set, no key →WithTokenis correctly skipped. No credential exposure.cmd/gadfly/model.go:179-203registerEnvProviders— malformed entries are collected as errors and logged to stderr, not propagated; a badGADFLY_ENDPOINT_RAGNAROSwould surface as amajordomo.Parsefailure on the binary side, whichscripts/run.sh:176-181already renders as a⚠️ Agentic reviewer for ... failed (exit N)comment — never a non-zero exit.cmd/gadfly/main.go:281-302lensConcurrency/providerOverride— requiresn > 0(rejectsragnaros=0/ragnaros=-1/ragnaros=abc);min(lensConcurrency(), len(specialists))caps at the 5-lens suite, soragnaros=1correctly yields strictly serial lenses on the single GPU.cmd/gadfly/main.go:234-271runSpecialists— per-lens panic recovery (records errored result, marks lensfinishedon the status board, doesn't crash the binary or starve other lenses). Each lens also has its owncontext.WithTimeoutso a hung llama-swap connection can't starve siblings.entrypoint.sh:194-197model-list normalization — empty/whitespace entries from awith: { models: "..." }override are dropped before lane assignment, so a stray trailing comma in a consumer override doesn't produce a phantom lane.Self-test risk acknowledged by the PR description itself: if the runner can't reach
llama-swap.ragnaros.dudenhoeffer.casa(DNS / LAN / self-signed TLS), this PR's own review will produce aragnaros/qwen3.6-27b⚠️comment for the reviewer section only — the job and merge are unaffected because the lane is purely advisory and the other 6 reviewers don't depend on the 4090 Ti.Nothing in this diff introduces a swallowed error, a missing cleanup, a panic path, an off-by-one, or an unhandled nil/empty/zero boundary that the existing code doesn't already cover.
Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 4m 19s
🪰 Gadfly review —
glm-5.2:cloud(ollama-cloud)Verdict: Minor issues — 5 reviewers: security, correctness, maintainability, performance, error-handling
🔒 Security — Minor issues
VERDICT: Minor issues
.gitea/workflows/review-reusable.yml:130. UnlikeGADFLY_ENDPOINT_M1/M5, which are sourced fromsecrets.*(lines 123-124, declared in theworkflow_call.secretsblock at 80-81 — empty → no-op when a consumer doesn't forward them),GADFLY_ENDPOINT_RAGNAROSis a literal"llamaswap|https://llama-swap.ragnaros.dudenhoeffer.casa"with no credential and no consumer opt-in, and it is not declared inworkflow_call.secrets. The defaultmodels:list (line 52) now includesragnaros/qwen3.6-27b, so every@v1consumer that omitswith:— i.e., inherits the default swarm — will have its PR diff + checked-out tree sent to a host the consumer never configured and may not know exists. This breaks the workflow's own least-privilege posture (comment lines 20-25: "forward only the credentials the reviewer actually uses … rather thansecrets: inherit, which leaks every caller secret"): the cloud/Claude models stay inert without forwarded keys, but ragnaros activates with nothing forwarded. Verified the parsing path —cmd/gadfly/model.go:184-191registersragnarosfrom the env-name suffix;endpointProviderat 207-234 builds an unauthenticated llamaswap client from the URL (the literal has only 2|-fields, sokeyis empty); andentrypoint.sh:159(provider_of) resolvesragnaros/qwen3.6-27bto providerragnaros, so it is dispatched as a real lane (lines 200-218) with no gating on a consumer secret. Fix: mirror the M1/M5 pattern — source the URL fromsecrets.GADFLY_ENDPOINT_RAGNAROS(declare it inworkflow_call.secrets) and defaultmodels:to excluderagnaros/qwen3.6-27bunless the consumer opts in (or document explicitly in the header comment that inheriting the default sends PR source to this specific third-party host). As written, a consumer forwarding zero secrets still exfiltrates PR content to a private host on every review.🎯 Correctness — No material issues found
VerDICT: No material issues found
ragnaros/qwen3.6-27b→provider_of(entrypoint.sh:159) returnsragnaros(first path segment), which matches theragnaros=1keys in bothprovider_concurrencyandprovider_lens_concurrencydefaults (review-reusable.yml:56-57) and theproviderOverride/lensConcurrencylookup incmd/gadfly/main.go:281-298. So the 4090 Ti lane correctly caps at 1 model × 1 lens.GADFLY_ENDPOINT_RAGNAROS→ name lowercased toragnaros(cmd/gadfly/model.go:184-191), and thellamaswapprovider spelling is an accepted alias in theendpointProviderswitch (cmd/gadfly/model.go:229). Thellamaswap|https://...two-part value parses correctly (no key segment). The hardcoded (non-secret) value is consistent with the "no credential" intent.ragnaros/qwen3.6-27bmodel id is opaque to gadfly (passed through to the llama-swap proxy), so no internal constant/conversion mismatch applies.with:inherit it, and existing per-input overrides still win.GADFLY_ENDPOINT_RAGNAROSis set unconditionally in the job env (not declared as a workflow_call secret), which is correct since it carries no credential.🧹 Code cleanliness & maintainability — Minor issues
Verdict: Minor issues
examples/reusable.yml:8-9,66andexamples/README.md:9— stale default-swarm description. This PR updates the default-swarm description inreview-reusable.yml,CLAUDE.md, andREADME.mdto add "a local 4090 Ti via llama-swap," but the example stubs that call the reusable workflow and explicitly describe what its default swarm contains were not touched.examples/reusable.yml:8-9still says "3 cloud models + the Claude Code engine (sonnet/opus/opus:max), 5-lens suite," and:66says "default swarm (3 cloud + Claude Code sonnet/opus/opus:max, 5 lenses)."examples/README.md:9similarly pitches "default swarm (3 cloud + Claude Code, 5-lens suite)." Since these files are the copy-paste entry point for@v1consumers and they promise what@v1will inherit, they now under-describe the inherited default (consumers who drop themodels:override expecting the documented "3 cloud + Claude Code" set will also get aragnaros/qwen3.6-27breview section against an endpoint they may not be able to reach). Verified by reading both files. Fix: append "+ a local 4090 Ti (qwen3.6-27b via llama-swap, advisory)" to those three spots, mirroring the README/CLAUDE.md wording, so the example docs stay in lock-step with the workflow they point at.review-reusable.yml:128-130— image-version-pinned comment is a latent landmine. The comment "NB: use the un-hyphenatedllamaswapprovider spelling — the pinned image accepts that form" couples the env literal to the specific pinned image (:sha-c342bdb). I verifiedcmd/gadfly/model.go:83,229acceptsllamaswap,llamaswaps,llama-swap, andllama-swapsequally, so the un-hyphenated form isn't actually required. The comment reads like a constraint that must be preserved across future image bumps, when it's really a non-requirement. Suggest softening to note both spellings work (or dropping the "NB" framing) so a future maintainer bumping the image doesn't think they must re-verify this specific spelling. Minor; verified againstmodel.go.No duplicated/dead code or structural issues introduced by the diff itself — the additions follow the existing M1/M5 endpoint pattern, and the input-default edits are the same one-line CSV-extension shape the file already uses.
⚡ Performance — No material issues found
VERDICT: No material issues found
I reviewed this through the performance lens and verified the relevant execution paths:
.gitea/workflows/review-reusable.yml:52,56-57— the newragnaros/qwen3.6-27bmodel andragnaros=1caps. I confirmed inentrypoint.sh:206-218(run_lane) that each provider is its own parallel lane (run_lane "$p" &at line 250, waited together at 253), so the new lane runs concurrently with the existingollama-cloud/claude-codelanes — it does not serialize behind or block them. No regression to existing lanes' wall-clock.entrypoint.sh:206-218enforcesprovider_concurrency(1 model at a time) andcmd/gadfly/main.go:242-269enforcesprovider_lens_concurrencyvia a capacity-1 semaphore (conc := min(lensConcurrency(), len(specialists))=min(1,5)), so lenses run strictly one-at-a-time. No fan-out overload on the 4090 Ti, no unbounded concurrency.runSpecialists,providerOverrideatmain.go:291) already existed and is reused unchanged.main.go:310-312) ≈ 50 min, within the 90-mintimeout_minutesdefault. It's a parallel lane, so it only matters if it becomes the long pole — but that's a timeout-adequacy question (correctness lens), not a performance regression introduced here.Nothing in my lane is materially wrong.
🧯 Error handling & edge cases — No material issues found
Verdict: No material issues found
Through the error-handling & edge cases lens, the diff adds a model (
ragnaros/qwen3.6-27b), a hard-coded endpoint env var, and two concurrency-knob entries. I verified the unhappy paths:cmd/gadfly/model.go:47-49callsregisterEnvProviders()and only logs malformed entries to stderr (continues on valid ones). IfGADFLY_ENDPOINT_RAGNAROSwere malformed, the review would continue without that provider — no panic, no abort. The new value"llamaswap|https://..."parses cleanly (endpointProvider,model.go:207-234: 2 parts, non-empty base URL,llamaswapis a known spelling).entrypoint.sh:183swallows per-model failures via|| log "model $1 failed (continuing)"; the lane waits and the job completes. The PR description's "if the box is unreachable, that section errors (advisory, never blocks)" matches the code's behavior. No new fatal path is introduced.ragnaros=1is handled defensively:entrypoint.sh:208[ "$cap" -ge 1 ] 2>/dev/null || cap=1guards against a non-numeric/zero cap, falling back to 1. The added entry is well-formed.modelsCSV is still non-empty after appending;provider_concurrency/provider_lens_concurrencyparsers (provider_cap,model.go) iterate CSV entries and ignore non-matching keys, so an unknownragnarosentry (if a consumer drops it) simply falls back toDEFAULT_CONC— no crash.No swallowed errors, missing cleanup, panics, or unhandled edge cases were introduced by this change within my lens.
Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 3m 27s
🪰 Gadfly review —
deepseek-v4-pro:cloud(ollama-cloud)Verdict: Minor issues — 5 reviewers: security, correctness, maintainability, performance, error-handling
🔒 Security — No material issues found
Verdict: No material issues found
The change is clean through the security lens. Here's what I verified:
No credential in the hardcoded endpoint. The value
"llamaswap|https://llama-swap.ragnaros.dudenhoeffer.casa"is a two-partprovider|base-urlwith no|keysegment. TheendpointProviderfunction incmd/gadfly/model.go:207-261handles this correctly —keystays empty whenlen(parts) == 2. No secret is leaked.The endpoint is set as a plain env value, not a secret. This is correct because it contains no credential. It's intentionally not declared in the
workflow_call.secretsblock (lines 73-83), which means consumers can't override it viasecrets:forwarding — but that's by design (the comment on lines 121-122 explicitly notes consumers needing custom endpoints should use the full stub).No injection vector. The value is hardcoded in the workflow YAML, not derived from any user-controlled input (PR body, comment, branch name, etc.). The workflow runs from the tagged ref (
@v1), not the PR branch, so a malicious PR can't alter it. TheendpointProviderfunction parses it withstrings.SplitN(raw, "|", 3)and validates the provider name against a fixed switch — no shell or path injection surface.HTTPS in transit. The URL uses
https://, so traffic to the llama-swap proxy is encrypted even though the proxy itself is unauthenticated (no bearer token). The lack of proxy auth is a deployment/network choice, not a code vulnerability — the host is on a LAN.Provider spelling is valid.
"llamaswap"(un-hyphenated) matches the switch case atmodel.go:229, consistent with the comment on lines 128-129.No SSRF from untrusted input. The URL is a static string in the workflow definition. The reviewer binary sends prompts to it, but the destination is fixed by the operator, not influenced by the PR under review.
The hardcoded hostname (
llama-swap.ragnaros.dudenhoeffer.casa) does expose internal network naming in a public repo, but the PR description is transparent about this being the author's own 4090 Ti on their LAN — it's an intentional disclosure, not an accidental leak.🎯 Correctness — No material issues found
Verdict: No material issues found
I traced the full wiring from the model spec through provider registration, concurrency, and execution:
ragnaros/qwen3.6-27b→ providerragnaros(viaprovider_ofinentrypoint.sh:159andmodelProvider()incmd/gadfly/model.go:146). ✅GADFLY_ENDPOINT_RAGNAROS→ lowercased suffixragnaros→llamaswapprovider at the given base URL (cmd/gadfly/model.go:184-185,229). Thellamaswapspelling is explicitly accepted (line 229:case "llamaswap", "llamaswaps", "llama-swap", "llama-swaps":). ✅ragnaros=1inGADFLY_PROVIDER_CONCURRENCY→provider_capinentrypoint.sh:167-176returns1for theragnaroslane. One model at a time. ✅ragnaros=1inGADFLY_PROVIDER_LENS_CONCURRENCY→providerOverrideincmd/gadfly/main.go:291-299returns1for providerragnaros. Lenses run sequentially on the single GPU. ✅entrypoint.sh:181setsPROVIDER=ollama(the agentic lane type, not the model backend), whichrun.sh:134routes to thegadflybinary. The binary resolvesragnaros/qwen3.6-27bthrough majordomo using the registeredragnarosprovider. ✅GADFLY_ENDPOINT_RAGNAROSis hardcoded in theenv:block (line 130), not forwarded from caller secrets, so it doesn't need an entry inworkflow_call.secrets. ✅The endpoint value format (
"llamaswap|https://...") has exactly two|-delimited parts, satisfyingendpointProvider's minimum of two (cmd/gadfly/model.go:209). The base URL is non-empty. No credential is needed (the third|keysegment is optional per line 215-216).No logic bugs, no mismatched names, no missing wiring.
🧹 Code cleanliness & maintainability — Minor issues
All six findings are confirmed against the actual code. Let me verify each one:
examples/reusable.yml:8-9— still says "3 cloud models + the Claude Code engine (sonnet/opus/opus:max), 5-lens suite" — no mention of the 4090 Ti.examples/reusable.yml:66— still says "3 cloud + Claude Code sonnet/opus/opus:max, 5 lenses" — stale.examples/README.md:9— still says "3 cloud + Claude Code, 5-lens suite" — stale..gitea/workflows/adversarial-review.yml:6-8— still says "3 cloud models + Claude Code (sonnet, opus, opus:max), 5-lens suite" — stale..gitea/workflows/adversarial-review.yml:52-53— still says "3 cloud + Claude Code sonnet/opus/opus:max, 5-lens suite" — stale.CLAUDE.md:106— still says "3 cloud + Claude Code, 5 lenses" — this is a different occurrence from the one the PR updated at lines 49-52; the PR left this one stale.The maintenance rule at
CLAUDE.md:119-123is real and does require these files to be kept in sync. All findings survive.Verdict: Minor issues
Stale default-swarm descriptions in 4 files not updated by this PR — the diff correctly updates
review-reusable.yml's own comments andREADME.md, but the same "3 cloud + Claude Code" description of the default swarm is now outdated in these files (verified by reading each):examples/reusable.yml:8-9— "The reusable ships a DEFAULT swarm: 3 cloud models + the Claude Code engine (sonnet/opus/opus:max), 5-lens suite."examples/reusable.yml:66— "default swarm (3 cloud + Claude Code sonnet/opus/opus:max, 5 lenses)."examples/README.md:9— "default swarm (3 cloud + Claude Code, 5-lens suite)".gitea/workflows/adversarial-review.yml:6-8— "inherits the default swarm: 3 cloud models + Claude Code (sonnet, opus, opus:max), 5-lens suite".gitea/workflows/adversarial-review.yml:52-53— "Inherit the default swarm (3 cloud + Claude Code sonnet/opus/opus:max, 5-lens suite)"CLAUDE.md:106— "opinionated default swarm (3 cloud + Claude Code, 5 lenses)"This directly violates the project's own maintenance rule at
CLAUDE.md:119-123: "Any change to env vars, flags, defaults, triggers, provider support, or the consumer stub MUST be reflected inREADME.mdand the relevant files underexamples/in the same change." The PR updatedREADME.mdand the architecture section ofCLAUDE.mdbut missedexamples/, the sibling caller workflow, and the Configuration section ofCLAUDE.md. Each stale comment now describes a swarm that no longer matches what the reusable workflow actually ships, creating confusion for anyone reading those files to understand what the default includes.Suggested fix: Update each of these comments to mention the 4090 Ti (e.g., "3 cloud + Claude Code + a local 4090 Ti (qwen3.6-27b), 5-lens suite"), consistent with the wording already used in
review-reusable.ymlandREADME.md.⚡ Performance — No material issues found
Verdict: No material issues found
This change adds one model (
ragnaros/qwen3.6-27b) to the default swarm with conservative concurrency (ragnaros=1for both provider and lens caps). Through the performance lens:ragnarosprovider gets its own lane (separate fromollama-cloudandclaude-code), so the local GPU's work runs in parallel with cloud models — it doesn't serialize behind them or block them.ragnaros=1/ragnaros=1(one model, one lens at a time) is the correct setting for a single local GPU. No risk of over-subscription or thrashing.The only performance consideration is that consumers inheriting the default swarm will now wait for an additional model to complete (the 4090 Ti's review), but that's the stated design intent of adding it to the shared default — not a regression.
🧯 Error handling & edge cases — No material issues found
Verdict: No material issues found
The diff adds a new model/provider endpoint and concurrency entries. I traced the full error-handling path for the new
ragnarosprovider throughentrypoint.sh,run.sh, andcmd/gadfly/model.go:Unreachable endpoint / model failure:
entrypoint.sh:213runsreview_onewith|| log "model $1 failed (continuing)"— a failed model never blocks the swarm.run.sh:176-181wraps non-zero exits from the binary into an advisory⚠️comment. The status board's "never started" guard (entrypoint.sh:189-191) marks skipped models done so the board completes. All unhappy paths are covered.Malformed endpoint value:
registerEnvProviders()(model.go:179-202) logs and skips bad entries rather than failing the review. The hardcoded"llamaswap|https://..."value has the correct 2-partprovider|base-urlformat with no trailing|key— the code handles the absent key correctly (model.go:215-217).Provider name resolution:
GADFLY_ENDPOINT_RAGNAROS→ lowercased toragnaros(model.go:185), matching the model spec prefixragnaros/qwen3.6-27band the concurrency map keysragnaros=1. Thellamaswapprovider spelling is accepted (model.go:229).Concurrency caps:
ragnaros=1in both maps correctly limits a single local GPU to one model × one lens. Theprovider_cap()fallback (entrypoint.sh:175) andproviderOverride()(main.go:297) both reject non-positive values, so a misconfiguration would safely fall back to the default of 1.Hardcoded endpoint (not a secret): This is intentional per the PR description (no credential needed). The comment on line 72 already warns consumers that custom endpoints beyond M1/M5 require the full stub. No error-handling gap here — the env var is simply always set; if unused, it's harmless.
Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 5m 22s