ci: inherit gadfly's default swarm (slim caller, re-pin @b02b11d) #14
Reference in New Issue
Block a user
Delete Branch "ci/gadfly-default"
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?
Follow-up to the merged #13: now that steve/gadfly#10 centralized the curated swarm as the reusable workflow's default, drop executus's explicit
with:swarm block and inherit it.uses: …@20a5c43 → @b02b11d(post-#10 gadfly commit).models/specialists/provider_concurrency/provider_lens_concurrency/timeout_secs/max_steps/timeout_minutes— all inherited now (3 cloud + Claude Code, 5-lens suite).allowed_users+ secret forwarding + actor gate.Net effect: executus moves from 6-cloud/3-lens to the shared default (3 cloud + Claude Code, 5 lenses), edited centrally in gadfly going forward. Advisory only.
🤖 Generated with Claude Code
🪰 Gadfly — live review status
6/6 reviewers finished · updated 2026-06-28 02:32:32Z
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 — ✅ doneLive status board. Findings are posted in each model's own comment. Advisory only — does not block merge.
🪰 Gadfly review —
minimax-m3:cloud(ollama-cloud)Verdict: No material issues found — 5 reviewers: security, correctness, maintainability, performance, error-handling
🔒 Security — No material issues found
Verdict: No material issues found
Reviewed through the security lens only. The change is configuration-only (no new code paths, no new taint sources, no new secrets surface).
Notes on what I checked:
.gitea/workflows/adversarial-review.yml:44-48still forwards exactly the four secrets the consumer expects, matching the inherited swarm the PR describes (3 ollama-cloud + Claude Code). No new secrets introduced, none dropped.b02b11d69139843665da4cdbf776bc0b3583490d), not a branch — so a push tosteve/gadflycannot silently swap the code that runs with the forwarded secrets. This is the correct mitigation for the cross-repo trust delegation that already existed before this PR.issue_commenttosteve | fizi | dazedand only when the issue is linked to a PR.pull_requestandworkflow_dispatchare correctly left ungated (those triggers are already trusted).allowed_users: "steve,fizi,dazed"on line 52 is still passed, so the in-container belt-and-suspenders check is intact.permissions:block stillcontents: read, issues: write, pull-requests: write— appropriately scoped; noactions: writeor wildcard.with:blocks actually shrinks the set of interpolated values flowing into the reusable workflow. No new template, path, SQL, command, or SSRF vectors are introduced.timeout_secs,max_steps,timeout_minutes,provider_concurrency,provider_lens_concurrency,specialists,modelsare pure behavior knobs — removing them is a delegation to upstream defaults and has no direct security implication on executus.steve/gadfly'sreview-reusable.ymlat@b02b11dto confirm its defaults don't, e.g., request additional secrets or relax input validation. The immutable-SHA pin + the unchanged explicitallowed_usersis the appropriate boundary given that constraint.Nothing in the diff opens a new authn/authz gap, injection vector, SSRF, deserialization risk, secret leak, or unsafe default from this lens.
🎯 Correctness — No material issues found
VERDICT: No material issues found
What I checked:
.gitea/workflows/adversarial-review.ymlpost-change (full file): the change is purely a deletion ofwith:keys plus a version-pin bump tob02b11d. No logic was added; nothing in the repo references the removed keys (specialists,provider_concurrency,provider_lens_concurrency,timeout_secs,max_steps,timeout_minutes), so there is no consumer-side reference to break. Verified bygrepacross the repo.steve/fizi/dazed) still mirrorsallowed_usersatadversarial-review.yml:34-39and:52— consistent.uses:at:42references an external, immutable commit hash, which is the right hardening per the in-file comment.One unverified semantic claim worth surfacing (not a blocker, just a "trust-but-verify" note):
:1-5asserts the inherited default is "3 cloud models + Claude Code sonnet/opus/opus:max, 5-lens suite." The upstream default swarm atsteve/gadfly/.gitea/workflows/review-reusable.yml@b02b11d…is not in this repo, so I cannot re-derive the model/specialist counts from first principles. Independently,examples/reviewer/main.go:17-21documents its ownDefaultLenses(which the file's comment says "mirrors gadfly's default") as exactly 3 lenses (security,correctness,error-handling) — not 5. So the canary comment and the new header comment disagree on the lens count. Either gadfly's upstream default has grown to 5 lenses since the canary was written (plausible —b02b11dis the whole point of this PR), or the new header's "5-lens suite" is wrong. I can't confirm without network access to gadfly at that commit. Suggested fix: after this PR merges, run a dogfood review once and have the reporter dump the actual swarm size back into this comment (or the canary's) so the two stay aligned.🧹 Code cleanliness & maintainability — No material issues found
I've verified the current state of
.gitea/workflows/adversarial-review.ymlagainst the diff.VERDICT: No material issues found
Reviewing through the code cleanliness & maintainability lens:
models,specialists,provider_concurrency,provider_lens_concurrency,timeout_secs,max_steps,timeout_minuteswere all duplicated state with gadfly's default; deleting them is a pure cleanup with no loss of information (the pin comment still points readers at the source of truth).with:follows the existing pattern of intent-explaining comments that sit above each block (compare to the comment aboveuses:and above theif:gate). Alignment ofsecrets:is kept intact.steve,fizi,dazedappears in both theif:actor gate andwith: allowed_users. This is intentional and explicitly documented in-file ("Mirrors the allowed_users input below (the in-container belt-and-suspenders check)") — not introduced by this PR, and not in my lane to flag.GITEA_TOKEN is autoinstead of the oldCLAUDE_CODE_OAUTH_TOKEN is auto(verified:.gitea/workflows/adversarial-review.yml:43), correcting a stale leftover from before #13.The change is a net positive for cleanliness: 7 duplicated config lines removed, 1 stale comment updated, and the file honestly describes what it now does.
⚡ Performance — No material issues found
VERDICT: No material issues found
Nothing in this diff is in the performance lane worth flagging.
provider_concurrency,provider_lens_concurrency) are removed in favor of upstream defaults. I can't verify what those defaults are without access tosteve/gadfly@…at the pinned commitb02b11d, but the override removal is the entire purpose of the PR ("inherit"), so flagging the unknown default as a regression would be guessing. Not flagging.timeout_secs,timeout_minutes) are removed. Same situation — I cannot read the upstreamreview-reusable.ymldefaults to compare, and removing an explicit override is a deliberate consumer choice, not a perf regression introduced by this diff.20a5c43 → b02b11d: pins to a newer upstream commit, which is the contract this PR is built on; not a perf concern in itself.No verification gaps worth raising in this lane — the file is 53 lines of declarative CI config and I've read the full state. The change is a perf-positive simplification.
🧯 Error handling & edge cases — No material issues found
Now I have enough context. Let me think through this strictly via my lens (error handling & edge cases).
Analysis through error-handling/edge-case lens:
Removed
timeout_minutes: 90— Before, the caller settimeout_minutes: 90. After the change, no caller override means the reusable workflow's defaulttimeout-minutesapplies. This is purely an inheritance-of-default situation — not an error-handling regression in the caller itself, as long as gadfly's default is sane. Not a finding for this lens (depends on the external default I cannot verify, and other lenses cover contract drift).Removed
timeout_secs: 600/max_steps: 14— same as above; inherited from defaults. Not a lens-of-mine finding.Pin
@b02b11d69139843665da4cdbf776bc0b3583490d— syntactically a 40-char hex SHA. Well-formed. Not in my lane.concurrency.groupexpression (unchanged by the diff but still in scope for the file):On
pull_requestevents,github.event.issueis absent andgithub.event.pull_request.numberis the natural source. Onissue_comment,github.event.issue.numberexists. Onworkflow_dispatch, bothgithub.event.issueandgithub.event.pull_requestare null; onlygithub.event.inputs.pr_numberprovides a number. Gitea's expression evaluator treats null as falsy, so the||chain falls through correctly. Not broken by the diff.if:actor gate (unchanged): Theif:short-circuits on non-issue_commentevents (no gate), and onissue_commentrequires the issue to actually be a PR (github.event.issue.pull_requesttruthy) AND the actor to be in the allow-list. If someone comments on a non-PR issue (just anissue), the gate evaluatesfalse && (… || … || …)→false, so the job is skipped — which is the intended behavior (don't run a secret-bearing reviewer on plain issues). No silent failure, no crash.workflow_dispatchwith emptypr_numberinput —pr_numberisrequired: truein theinputs:schema (verified at lines 18–19). Gitea enforcesrequired: trueat dispatch time, so an empty string can't get past the UI. Even if it could, the concurrency group expression with an empty string would just collapse runs on the same empty key. Not a defect in the changed code.Empty
with:allowed_users— explicitly set to"steve,fizi,dazed", non-empty. Fine.None of the removed values had a nil/empty-collection defensive angle — they were simple string scalars. Removing them can't introduce a "nil deref" or "empty collection" bug at the caller level; the defaults live in the callee.
No new
defer,cleanup, or rollback was added or removed in this diff — there is no Go code, only YAML. There's nothing to leak, no file descriptor to close, no goroutine to cancel.Net: the diff is a pure inheritance change at the YAML caller. Nothing it adds or removes introduces a new nil/empty/overflow/boundary error path in this file. The error-handling shape of the workflow (actor gate, concurrency group, required inputs, pinned ref) is unchanged.
VERDICT: No material issues found in the error-handling / edge-case lens.
Review — Error handling & edge cases
No material issues found in this lens.
The diff is a pure inheritance change at the YAML caller — it removes a handful of
with:string scalars (models,specialists,provider_concurrency,provider_lens_concurrency,timeout_secs,max_steps,timeout_minutes) and re-pins the reusable workflow to a new SHA. Verified directly in the checked-out file.gitea/workflows/adversarial-review.yml:42-52:issue_commentevents and rejects comments on non-PR issues (github.event.issue.pull_requestfalsy ⇒false && …⇒ safe skip, no partial evaluation).concurrency.groupexpression at line 26 falls throughissue.number → pull_request.number → inputs.pr_numbercorrectly for each of the three triggers.workflow_dispatch.inputs.pr_numberis declaredrequired: true(lines 18–19), so an empty/nil dispatch payload can't reach the job.@b02b11d69139843665da4cdbf776bc0b3583490dis a syntactically valid 40-char hex SHA.Whether the inherited defaults (3-cloud swarm, 5 lenses, new timeout/step limits) are themselves adequate is a contract/correctness question that belongs to another lens.
Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 2m 56s
🪰 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 by reading
.gitea/workflows/adversarial-review.ymlat line 42:Pin is a full 40-char commit SHA (
b02b11d69139843665da4cdbf776bc0b3583490d) — not a branch or tag. An upstream push to gadfly cannot silently change the code that runs with forwarded secrets. The prior pin (20a5c43…) was likewise a full SHA; no regression here.Actor gate (
if:) matchesallowed_usersexactly. Lines 37–39 enumeratesteve,fizi,dazed— identical to theallowed_users: "steve,fizi,dazed"input at line 52. The belt-and-suspenders model is intact: the gate blocks the job entirely for untrustedissue_commentactors before secrets are ever forwarded to the sub-workflow.Secret forwarding is unchanged and explicit. The four secrets (
OLLAMA_CLOUD_API_KEY,CLAUDE_CODE_OAUTH_TOKEN,GADFLY_FINDINGS_URL,GADFLY_FINDINGS_TOKEN) are the same set as before; nothing new is added or dropped. Least-privilege is preserved.Inheriting defaults does not widen the trust surface. Because the
uses:reference is SHA-pinned, gadfly's defaults are frozen atb02b11d. Any future change to gadfly's default swarm is inert until this repo re-pins.🎯 Correctness — No material issues found
The file reads clean. Here is my verdict:
No material issues found
The change is a straightforward delegation of swarm configuration to gadfly's centrally-managed reusable workflow. Verified:
if:comment-gate hardcodessteve,fizi,dazed;allowed_userspasses the same three names. These match exactly — no drift introduced by the removal of the otherwith:keys.with:block containing only a comment and a single key is valid YAML; GHA will parse it correctly.models,specialists,provider_concurrency,provider_lens_concurrency,timeout_secs,max_steps,timeout_minutes): All were optional inputs with defaults defined in gadfly's reusable workflow. Omitting them causes the downstream defaults to apply — the intended behavior.OLLAMA_CLOUD_API_KEYis still forwarded; the PR description says the new default still includes cloud models, so forwarding it is correct. Extra secrets passed to a reusable workflow that doesn't declare them are silently ignored, so this is safe regardless.b02b11d69139843665da4cdbf776bc0b3583490dis a full 40-character SHA — no truncation or copy-paste error detectable from within this repo.The only thing I cannot verify from within this repo is whether the gadfly default swarm at
@b02b11dtruly matches the header comment's description ("3 cloud models + Claude Code sonnet/opus/opus:max, 5-lens suite") — that requires reading the external gadfly repo. But the comment is documentation, not an executable claim; the actual runtime behaviour is owned by gadfly's commit.🧹 Code cleanliness & maintainability — No material issues found
No material issues found
The diff is a clean configuration reduction: it removes six explicit
with:parameters that are now inherited from the upstream reusable workflow, updates the pin commit, and refreshes the header comment to match reality. From a cleanliness standpoint:with:explains why onlyallowed_usersremains — no implicit reasoning for a future reader.Nothing to flag under this lens.
⚡ Performance — No material issues found
No material issues found
This change is pure CI YAML configuration — it removes explicit swarm parameters and inherits them from gadfly's reusable workflow. Through the performance lens, there is nothing to flag:
Total agent work is roughly neutral. The old explicit config ran 9 models × 3 lenses = 27 combinations; the inherited default (per the PR description: 3 cloud + 3 Claude Code variants × 5 lenses) is ~30 combinations. Slightly more, but within noise for an advisory-only job.
provider_concurrency/provider_lens_concurrencyremoved. The old explicit throttle (ollama-cloud=3,claude-code=2) is gone. The inherited defaults fromreview-reusable.yml@b02b11dgovern burst behavior instead. I cannot read the external gadfly repo to verify those defaults — but since the workflow is advisory-only and Gitea enforces platform-level queue limits, this is not a material regression.timeout_minutes: 90removed. The explicit cap on total workflow wall-time is gone. If the inherited default is longer or absent, a stuck run holds a CI slot for longer. Verified that this parameter no longer appears in the checked-out file (:52). Low impact given the advisory-only nature of the job.No efficiency regression, unbounded growth, or missing limit rises to a blocking level here.
🧯 Error handling & edge cases — Minor issues
The removal of
timeout_minutes: 90is confirmed at line 52 — thewith:block contains onlyallowed_users. The upstreamreview-reusable.ymlis external and inaccessible from this repo, so the inherited default cannot be verified here. The finding is factually grounded and appropriately scoped.Minor issues
timeout_minutessafety bound removed (.gitea/workflows/adversarial-review.yml,with:block at line 49–52). The previous config passedtimeout_minutes: 90as an explicit input to the reusable workflow, capping a stuck or runaway swarm at 90 minutes. The new stub omits this entirely; thewith:block now contains onlyallowed_users. The inherited default fromreview-reusable.yml@b02b11dcannot be confirmed from this repository — if that default is absent or substantially larger (Gitea Actions' job-level default is 360 minutes), a hung swarm now ties up CI resources for far longer with no local guard.Suggested fix: either confirm (and document in a comment) the upstream default for
timeout_minutes, or keep an explicit override (timeout_minutes: 90) to preserve the local safety net regardless of upstream drift.No other material issues found. The actor gate and
allowed_userslist are in sync (both enumeratesteve,fizi,dazed), and the secret-forwarding set is unchanged.Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 1m 32s
🪰 Gadfly review —
deepseek-v4-pro:cloud(ollama-cloud)Verdict: No material issues found — 5 reviewers: security, correctness, maintainability, performance, error-handling
🔒 Security — No material issues found
Verdict: No material issues found
Commit pinning is preserved. The reusable workflow remains pinned to a full immutable SHA (
@b02b11d…), so a push tosteve/gadflycannot silently change the code that runs with forwarded secrets. The old pin (@20a5c43…) is simply rotated to the new commit — no regression.Actor gate unchanged. The
if:condition still restrictsissue_comment-triggered runs to the same three usernames (steve,fizi,dazed), and theallowed_usersinput mirrors it. No new actors can trigger a secret-bearing run.Secret forwarding unchanged. The same four secrets (
OLLAMA_CLOUD_API_KEY,CLAUDE_CODE_OAUTH_TOKEN,GADFLY_FINDINGS_URL,GADFLY_FINDINGS_TOKEN) are forwarded — no additions, no removals. The "least privilege" comment remains accurate.Inheriting defaults does not expand the trust boundary. The removed
with:parameters (models,specialists, concurrency, timeouts) are now inherited from the reusable workflow's defaults at the pinned commit. The consumer already trusts the reusable workflow's code; inheriting its defaults does not introduce a new trust relationship or expose secrets to a new path. The defaults are immutable for that commit.No injection surface. The only consumer-supplied input (
allowed_users) is a hardcoded literal string of usernames, not derived from any attacker-controllable source.🎯 Correctness — No material issues found
No material issues found.
The change is a straightforward consumer-side cleanup: remove explicit
with:inputs and rely on defaults from a newly-pinned version of the external reusable workflow. Everything I can verify locally checks out:allowed_usersconsistency (verified by reading.gitea/workflows/adversarial-review.yml:34-39,52): theif:condition gates onsteve,fizi,dazed— exactly the same set passed asallowed_users. No drift.OLLAMA_CLOUD_API_KEY+CLAUDE_CODE_OAUTH_TOKENcover the "3 cloud + Claude Code" default described in the PR. The findings secrets are unchanged.grep): the removed parameters (timeout_secs,max_steps,provider_concurrency, etc.) are not referenced elsewhere in this repo's workflow or config in a way that would break.One unverified concern (cannot check — the reusable workflow lives in
steve/gadfly, outside this repo): if any of the seven removedwith:inputs (models,specialists,provider_concurrency,provider_lens_concurrency,timeout_secs,max_steps,timeout_minutes) lack a default in the new pinned commitb02b11d, the workflow will fail at runtime with a missing-input error. The PR description states these were centralized as defaults in steve/gadfly#10, but I have no tool access to confirm that the pinned commit actually contains those defaults.🧹 Code cleanliness & maintainability — No material issues found
No material issues found.
The diff is clean and well-scoped. It removes ~8 lines of explicit swarm configuration that duplicated the reusable workflow's defaults, updates the comment block to accurately describe what's now inherited, and leaves only the consumer-specific
allowed_usersinwith:. No dead code, no confusing names, no leaky abstractions introduced.The one pre-existing maintainability wart — the
if:actor gate (lines 36-39) hardcoding the same three usernames asallowed_users(line 52) — is already documented by the comment on line 33 ("belt-and-suspenders") and is not made worse by this PR. If anything, the PR makes the file easier to maintain going forward since swarm changes now happen in one place (gadfly'sreview-reusable.yml) rather than needing a synchronized edit here.⚡ Performance — No material issues found
No material issues found
The diff is a pure CI workflow configuration change that removes explicit swarm parameters in favor of inherited defaults. From a performance lens:
steve/gadfly/.gitea/workflows/review-reusable.yml, outside this repo), so I can't confirm whether the inheritedprovider_concurrency,timeout_secs, ortimeout_minutesdiffer materially from the removed values. The PR description frames this as a deliberate centralization, and the removed values (ollama-cloud=3,timeout_secs=600,timeout_minutes=90) are unremarkable — no obvious risk of a hidden regression.Nothing in this change raises a concrete, verifiable performance concern.
🧯 Error handling & edge cases — No material issues found
Verdict: No material issues found
The diff is a pure configuration simplification — it removes explicit
with:parameters in favor of defaults inherited from the upstream reusable workflow. From an error-handling & edge-cases perspective:models,specialists,timeout_secs,max_steps,timeout_minutes, etc.) are now supplied by the reusable workflow's defaults rather than being explicitly set. If those defaults were missing or invalid, the failure would surface as a workflow load/run error in the external repo — not a silent degradation here.Least privilegepattern.if:block andallowed_userslist are untouched; any pre-existing divergence risk between them is not introduced by this diff.The one operational shift —
timeout_minutes: 90being removed and falling back to the reusable workflow's (or the platform's) default — is an intentional centralization, not an oversight. If the inherited default is substantially longer, a hung job would burn more runner minutes, but that's a resource/governance concern outside this lens.Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 1m 7s
🪰 Gadfly review —
glm-5.2:cloud(ollama-cloud)Verdict: No material issues found — 5 reviewers: security, correctness, maintainability, performance, error-handling
🔒 Security — No material issues found
No material issues found
.gitea/workflows/adversarial-review.yml:1-52. The security-relevant controls are all preserved across the change:adversarial-review.yml:42): re-pinned from20a5c43…tob02b11d…, and the comment at:40-41still correctly documents that it's pinned to an immutable commit (not@main) so a push tosteve/gadflycannot silently alter the code that runs with forwarded secrets. Inheriting defaults from the reusable workflow is safe precisely because of this pin — the default swarm is frozen at the pinned commit, not subject to a mutable ref.adversarial-review.yml:44-48): unchanged and still least-privilege — onlyOLLAMA_CLOUD_API_KEY,CLAUDE_CODE_OAUTH_TOKEN,GADFLY_FINDINGS_URL,GADFLY_FINDINGS_TOKENare forwarded. No new secrets are exposed by dropping the explicitwith:model block, and removing those inputs doesn't widen the secret surface (the forwarded set is independent of themodels/specialists/concurrency inputs).adversarial-review.yml:34-39): unchanged —issue_commenttriggers remain restricted tosteve/fizi/dazed, andallowed_users(:52) is unchanged as the in-container belt-and-suspenders check.steve/gadfly'sreview-reusable.yml@b02b11d…(it lives in the external gadfly repo, not vendored into this checkout, andfind_files/grepconfirm no copy exists here), so I could not independently confirm the inherited default swarm only ever uses the four forwarded secrets. That is a trust boundary inherent to consuming a reusable workflow, but it's gated by the immutable pin and is outside what this repo's diff can establish — flagging as unverified rather than a defect.🎯 Correctness — No material issues found
VERDICT: No material issues found
The change is entirely within
.gitea/workflows/adversarial-review.ymland is a consumer stub for an external reusable workflow (steve/gadfly/.gitea/workflows/review-reusable.yml). I reviewed it through the correctness lens:uses:pin updated to@b02b11d…,secrets:forwarding unchanged,with:now contains onlyallowed_users.if:) still matches theallowed_userslist exactly (steve,fizi,dazed), so the belt-and-suspenders check remains self-consistent.One caveat (unverified, outside this repo): the correctness of inheriting the swarm depends on the reusable workflow at
@b02b11ddeclaring defaults formodels,specialists,provider_concurrency,provider_lens_concurrency,timeout_secs,max_steps, andtimeout_minutes. That file lives in thesteve/gadflyrepo, which is not present in this checkout, so I could not confirm those defaults exist. If any of those is a requiredwithinput without a default, the run would fail — but this is a cross-repo concern not verifiable with the tools available here, and it is exactly the migration the PR description asserts was done in gadfly#10.Verified by reading
.gitea/workflows/adversarial-review.ymlin full and confirming no other local files reference the reusable workflow's inputs.🧹 Code cleanliness & maintainability — No material issues found
Verdict: No material issues found
Through the code-cleanliness lens, this change is a straightforward simplification: it removes 6 explicit
with:overrides and relies on the reusable workflow's defaults, leaving only the consumer-specificallowed_users. The header comment was correctly updated to match the new behavior (3 cloud + Claude Code, 5 lenses) rather than the stale "6 cloud models / 3-lens" description. The remaining file is well-structured — comments are accurate, the least-privilege secret-forwarding rationale is retained, and the pin rationale stays consistent with the repin. No dead code, no duplication, no leaky abstractions introduced.⚡ Performance — No material issues found
Verdict: No material issues found
The diff is a CI-config change that trims an explicit
with:swarm block down toallowed_usersand re-pins the reusable workflow to a new immutable commit. From a performance lens:models/specialists/provider_concurrencyand inheriting the upstream default is neutral-to-positive: the caller no longer overrides concurrency caps, so whatever the reusable workflow declares is used. No new unbounded work, N+1 patterns, or hot-path allocations are introduced — this file just delegates to a pinned external workflow.allowed_users(a static string), so no per-run growth or repeated parsing concern.steve/gadfly'sreview-reusable.yml@b02b11d…from this checkout to verify the inherited default's concurrency/timeout bounds, so any claim about them would be unverified; but this PR does not introduce the regressions — it merely stops overriding values it previously set. The performance characteristics are determined by the pinned upstream commit, which is itself pinned for reviewability.Outside my lens: the commit-pin change (
@20a5c43…→@b02b11d…) is a supply-chain/trust concern, not a performance one.No blocking, minor, or material performance issues in this change.
🧯 Error handling & edge cases — No material issues found
VERDICT: No material issues found
Through the error-handling / edge-case lens, this diff is benign:
github.event.issue.pull_requesttruthiness guard before actor access;||fallthrough chain for the concurrency key) is pre-existing and not regressed.with:inputs (models,specials,timeout_secs,max_steps,timeout_minutes) and forwards defaults from the pinned reusable workflow. The one edge case worth noting — whether the pinnedsteve/gadfly@b02b11dreusable workflow defines a defaulttimeout_minutes/timeout_secssuch that omitting it doesn't leave a hung run with no outer cap — lives in the externalsteve/gadflyrepo, which I cannot inspect with my read-only tools over this checked-out repository. I verified there is no job-leveltimeout-minuteshere in either the old or new version (it was awith:input passed to the reusable workflow, not a Gitea job property), so the removal is purely "stop forwarding this input" and its handling is the reusable's responsibility. Cannot confirm a regression without access to that repo; flagging as unverified rather than asserting.allowed_usersremains the only consumer-specificwith:input, so no empty-collection edge arises from the trim.No error-handling or edge-case issue in this repository's code is introduced by the diff.
Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 1m 0s
🪰 Gadfly review —
claude-code/opus(claude-code)Verdict: Minor issues — 5 reviewers: security, correctness, maintainability, performance, error-handling
🔒 Security — No material issues found
This is a CI workflow config change, reviewed through the Security lens. The diff only re-pins the reusable-workflow SHA, removes the explicit swarm config (inheriting gadfly's defaults), and rewords comments. I verified the full file at its checked-out state.
VERDICT: No material issues found
Security-relevant properties I checked, all preserved by this diff:
Supply-chain pin intact (
.gitea/workflows/adversarial-review.yml:42). Theuses:still references a full 40-hex-char immutable commit SHA (b02b11d6…3583490d), not@mainor a mutable tag. This is the load-bearing protection — secrets are forwarded to frozen third-party workflow code. Inheriting gadfly's "default swarm" does not weaken this: the default is resolved at the pinned commit, so it cannot change under our feet without a future re-pin PR like this one. Verified by reading the line and counting the SHA length.Secret forwarding unchanged and still least-privilege (
:44-48). The same four secrets are forwarded as before; removing thewith:model/concurrency block does not broaden secret exposure. Narrowing 6→3 cloud models reduces, not increases, consumers ofOLLAMA_CLOUD_API_KEY. No secret is interpolated into a shell/run:context, so no leakage path is introduced.Actor gate untouched and consistent (
:34-39,:52). Theissue_commenttrust gate (steve/fizi/dazed) and theallowed_usersbelt-and-suspenders input are not modified by this diff and remain in sync. The diff does not alter triggers,permissions:, or the gate logic.Residual trust assumption (not a finding, and inherent to the design): the content of the inherited default swarm and how gadfly consumes the four forwarded secrets live in the external
steve/gadflyrepo atb02b11d, which is outside this checkout and therefore not verifiable from here. The immutable pin is the correct and intended mitigation for that boundary, and it is in place.Outside my lens: the
if:gate exemptspull_request/workflow_dispatchfrom the actor allow-list (onlyissue_commentis gated) — but that logic is pre-existing and unchanged by this diff, so it's not introduced here.🎯 Correctness — No material issues found
The file is valid YAML and internally consistent. Within my Correctness lens, here's my assessment.
What I verified:
.gitea/workflows/adversarial-review.yml. The diff applied cleanly: thewith:block now contains onlyallowed_users, and theuses:pin is@b02b11d…. YAML is well-formed (no dangling keys,with:still has a valid child).adversarial-review.yml:34-39) hardcodessteve/fizi/dazed, which is consistent with the retainedallowed_users: "steve,fizi,dazed"(line 52) — no mismatch introduced.OLLAMA_CLOUD_API_KEYis still forwarded (line 45), consistent with a swarm that still includes cloud models per the new comment. No now-orphaned or now-missing secret evident from what's reviewable here.What I could not verify (out of reach, not a defect I can confirm):
models,specialists,provider_concurrency,provider_lens_concurrency,timeout_secs,max_steps,timeout_minutes) are genuinely supplied as defaults bysteve/gadfly/.gitea/workflows/review-reusable.yml@b02b11d…— lives in an external repo at a pinned commit not present in this checkout. I cannot confirm those defaults exist or that their values equal the previous explicit ones. If gadfly#10 did not default one of these (or renamedallowed_users), the run would silently change behavior or fail input validation. This is the only real correctness risk, and it is unverifiable from here.@b02b11dactually defines.The change is config-only, advisory, and internally coherent. The behavioral correctness hinges entirely on the pinned gadfly commit's defaults, which is by design (centralization) but is the spot to double-check before merge.
VERDICT: No material issues found
adversarial-review.yml:34-52).review-reusable.yml@b02b11d…actually defaulting every removed input to the intended values. Worth confirming against that gadfly commit before merge, but it is outside what these repo tools can reach.🧹 Code cleanliness & maintainability — Minor issues
I've verified the draft against the actual file. Both findings accurately describe the file's content — the header comment at lines 1-5 does hardcode a specific snapshot of gadfly's swarm ("3 cloud models + Claude Code sonnet/opus/opus:max, 5-lens suite") while the change's stated purpose is to inherit that centrally, and the inline comment at 50-51 generically restates the same "inherit the default swarm" point.
VERDICT: Minor issues
.gitea/workflows/adversarial-review.yml:1-5— the header comment re-enumerates the very swarm details the PR set out to stop hardcoding. The point of this change (per the inline comment at lines 50-51) is that the model fleet and lens suite now live centrally in gadfly and are inherited, so this repo no longer tracks them. But the header comment hardcodes "3 cloud models + Claude Code sonnet/opus/opus:max, 5-lens suite" — a snapshot of gadfly's current default. When gadfly edits its default, this comment silently goes stale and nothing in this repo flags it, reintroducing the drift the change is trying to eliminate. Low-churn fix: make the header generic like the inline comment already is (e.g. "INHERITS its default swarm — models + lenses defined in gadfly's review-reusable.yml") and drop the enumerated specifics.adversarial-review.yml:1-5and:50-51— two comments now redundantly explain the same "inherit the default swarm" point. Minor; once the header is made generic per above, the inline comment at 50-51 largely repeats it. Consider trimming one so there's a single source of truth for the "why" in this file.⚡ Performance — No material issues found
VERDICT: No material issues found
This is a CI workflow config change that removes the explicit swarm parameters (
models,specialists,provider_concurrency,provider_lens_concurrency,timeout_secs,max_steps,timeout_minutes) so the caller inherits gadfly's reusable-workflow defaults. There is no application code or hot path here to regress.Through the Performance lens specifically:
adversarial-review.yml:25-27keeps aconcurrencygroup keyed on the PR number withcancel-in-progress: true, so superseded runs are still cancelled — no stacked/duplicate swarm runs piling up. The diff doesn't touch this.timeout_minutes: 90(caller-side job cap) is not a perf regression I can substantiate. The previoustimeout_secs: 600was also removed and is now inherited, so run-bounding moves into the reusable workflow rather than disappearing. The reusable workflow (steve/gadfly@b02b11d…/review-reusable.yml) lives in another repo and is not present in this checkout (grep/findforreview-reusablereturns nothing local), so I cannot confirm the inherited timeout value — but I have no basis to claim the bound was dropped, only relocated.Nothing in my lens (N+1, allocations, hot-loop work, unbounded growth, blocking calls, quadratic behavior) is applicable to a pure config-inheritance change. Clean from a performance standpoint.
🧯 Error handling & edge cases — No material issues found
VERDICT: No material issues found
This is a declarative Gitea Actions config change: it re-pins the reusable-workflow ref (
@20a5c43 → @b02b11d) and deletes sixwith:inputs (models,specialists,provider_concurrency,provider_lens_concurrency,timeout_secs,max_steps, plustimeout_minutes) so they inherit the reusable workflow's defaults. I read the full file (.gitea/workflows/adversarial-review.yml:1-53) and confirmed the diff is the only change in the working tree againstHEAD; the remainingif:gate, secret forwarding, andallowed_usersare untouched and internally consistent.Through the error-handling & edge-cases lens there is nothing to flag:
adversarial-review.yml:34-39) and concurrency key (:26) are unchanged; the||/&&short-circuit logic and thegithub.event.issue.number || ... || inputs.pr_numberfallback chain are not modified by this PR.One adjacent observation, explicitly unverified and outside my lane (it's a config/correctness concern, not error-handling, and I cannot check it because
review-reusable.ymllives in the externalsteve/gadflyrepo, not this checkout): deletingtimeout_secs/max_steps/timeout_minutesmeans run-bounding now relies entirely on defaults defined in commitb02b11d. If that commit does not actually set those defaults, runs could be unbounded. The PR description asserts the defaults exist post-#10; that claim is not verifiable from this repository alone. Worth a one-time confirmation, but not a blocking issue in my lens.Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 1m 10s
🪰 Gadfly review —
claude-code/opus:max(claude-code)Verdict: Minor issues — 5 reviewers: security, correctness, maintainability, performance, error-handling
🔒 Security — No material issues found
I've read the actual workflow file at its checked-out state and confirmed the diff matches. This is a CI config-slimming change. Let me reason through the security-relevant controls.
VERDICT: No material issues found
The change removes the explicit
with:swarm config (models, concurrency, step/timeout caps) and re-pins the gadfly reusable workflow. Through the security lens, the controls that actually matter all survive the slimming:adversarial-review.yml:42). The pin moved20a5c43 → b02b11d, but it's still a full 40-char commit SHA, not@main. The stated guarantee — "a push to gadfly can't silently change the code that runs with our forwarded secrets" — still holds, and critically it also freezes the now-inherited defaults (models,max_steps, timeouts) at that commit. Inheriting defaults does not widen the trust boundary, because the defaults live inside the pinned ref. (Verified by reading the file; I cannot verify the contents of the remotesteve/gadflyrepo atb02b11dfrom here — that trust is inherent to any cross-repo pin and is not introduced by this diff.):44-48). The same four secrets are still explicitly enumerated. The reusable workflow can only ever use secrets the caller forwards, so inheriting an unseen swarm config cannot cause it to consume a secret executus didn't hand over. This is the real security boundary and it's intact.:34-39):issue_commenttriggers remain restricted tosteve/fizi/dazed, andallowed_users(:52) still mirrors that list as the in-container belt-and-suspenders check — the two remain consistent after the edit.permissions:(read contents / write issues+PRs), and concurrency are not modified by this diff.Nothing in the removed lines (
models,provider_concurrency,max_steps,timeout_*) is a security control — those are resource/DoS-bounding knobs (a separate lens), and removing them here only delegates them to the pinned upstream default.No authn/authz gap, injection sink, secret-leak, or tainted-data path is introduced or weakened by this change.
Outside my lens: the removed
max_steps/timeout_secs/timeout_minutesnow rely entirely on gadfly's upstream defaults being sane — a resource-bounding concern worth a correctness/cost reviewer's glance, but not a security issue.(Note: I did not enter the plan workflow or create a plan file — this task is an adversarial security review whose deliverable is the review itself, and I confined myself to read-only investigation.)
🎯 Correctness — No material issues found
This is a CI-config change (a caller workflow that delegates to an external reusable workflow). I've read the full post-change file and checked the workflow directory. Let me note what's verifiable here versus what isn't.
The substance of this PR — that the removed
with:inputs (models,specialists,provider_concurrency,provider_lens_concurrency,timeout_secs,max_steps,timeout_minutes) now resolve to the intended defaults — lives entirely insteve/gadfly/.gitea/workflows/review-reusable.yml@b02b11d…, which is an external repo not present in this checkout. I cannot read it, so the central correctness claim is unverifiable from here. I'll flag that honestly rather than guess.VERDICT: No material issues found
Within the Correctness lens, the change to this file is sound:
.gitea/workflows/adversarial-review.yml:49-52— After removing the six swarm inputs, thewith:block still containsallowed_users, so it is non-empty and YAML-valid. Verified by reading the full file. No dangling/empty mapping..gitea/workflows/adversarial-review.yml:34-39— The actor gate (steve/fizi/dazed) is unchanged and remains consistent with the retainedallowed_users: "steve,fizi,dazed". The diff doesn't touch this, and the two lists still agree. Verified..gitea/workflows/adversarial-review.yml:42— Theuses:pin is re-pointed to a full 40-char commit SHA (b02b11d…490d), preserving the immutable-pin invariant the surrounding comment depends on. Well-formed.Caveats I could not verify (genuinely out of reach, not dropped nits):
review-reusable.yml@b02b11dactually declares defaults for all six removed inputs. If any ofmodels/specialists/provider_concurrency/provider_lens_concurrency/timeout_secs/max_stepsis arequired: trueinput there (ortimeout_minutesis referenced with no default), this caller will fail at dispatch. The PR body asserts gadfly#10 made these defaults; I take that on faith — it cannot be checked from this repo.No correctness defect is introduced by the lines actually changed here.
🧹 Code cleanliness & maintainability — Minor issues
VERDICT: Minor issues
My lens is code cleanliness & maintainability. The diff is a clean slimming of the workflow stub — removing the explicit
with:swarm block and inheriting gadfly's default is exactly the kind of de-duplication that reduces maintenance surface. One maintainability concern remains:.gitea/workflows/adversarial-review.yml:1-2— the header comment re-duplicates the very config the change just centralized. The whole point of this change (per the line 50-51 comment) is that the swarm config "lives centrally in gadfly's review-reusable.yml." But the file-top comment still enumerates the inherited default's contents: "3 cloud models + Claude Code sonnet/opus/opus:max, 5-lens suite." That enumeration is no longer set or verifiable in this repo — when gadfly's default swarm changes, this comment silently goes stale with nothing here to catch it (confirmed: grepping the repo, this is the only place describing the specific swarm shape; README.md and CLAUDE.md describe the swarm only generically as an "N×M swarm" and don't restate the model/lens counts). Suggested low-churn fix: drop the specific model/lens enumeration from lines 1-2 and describe it by reference only — e.g. "inherits its default review swarm (defined in gadfly's review-reusable.yml)." This keeps the doc honest without re-pinning a copy of the central config.⚡ Performance — No material issues found
This is a CI workflow config change that delegates the swarm configuration to an external reusable workflow. I've verified the checked-out file matches the diff. Evaluating strictly through the Performance lens:
There is no application code, hot path, query, loop, or allocation in this change — it's a YAML stub that forwards triggers/secrets and inherits swarm config from an externally-pinned gadfly workflow (
review-reusable.yml@b02b11d…), which is not in this repo and cannot be read.VERDICT: No material issues found
provider_concurrency,provider_lens_concurrency,timeout_secs, andmax_stepscaps are removed so they now inherit gadfly's defaults. This is the stated intent, advisory-only, and the actual concurrency/step bounds live in the external pinned workflow I cannot inspect — so I can't substantiate any concrete regression. Not a finding, just the boundary of what's verifiable here.concurrency.groupwithcancel-in-progress: true(adversarial-review.yml:25-27) is unchanged and still prevents redundant overlapping runs per PR — good for runner utilization; no regression.One thing worth a glance on the gadfly side (unverified, outside this repo): the removed
timeout_minutes: 90was a job-level wall-clock cap. Ifreview-reusable.ymldoesn't define its own job timeout, a hung swarm run could now occupy a runner up to the platform default (commonly 360 min) instead of 90. That's the only resource-occupancy concern, and it can only be confirmed by reading the pinned reusable workflow.Nothing material in the performance lane within this diff.
🧯 Error handling & edge cases — No material issues found
VERDICT: No material issues found
This diff is a pure CI workflow config change to
.gitea/workflows/adversarial-review.yml: it re-pins the reusable-workflow ref and removes the explicit swarmwith:inputs so they inherit gadfly's centralized defaults. I read the full file (53 lines) at its checked-out state to confirm there is no executable logic here.Through the error handling & edge cases lens, there is nothing material:
adversarial-review.yml:34-39) andallowed_users(:52) remain consistent (steve,fizi,dazed), so the comment-trigger guard's unhappy path is unchanged and still closed.:44-48) is intact, so no new "missing credential" failure mode is introduced on the caller side.One caveat I could not verify (and which is really CI-reliability, not strictly my lens): the call now relies on
models,specialists,timeout_secs,max_steps, etc. all having defaults inreview-reusable.yml@b02b11d. That file lives in the externalsteve/gadflyrepo and is not in this checkout, so I can't confirm those inputs are non-required. If any were declaredrequired: truewithout a default in that commit, the workflow call would fail at dispatch — but the PR description states #10 made them defaults, and this is unverifiable from here rather than a demonstrable defect.Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 1m 1s
75234c2768tod5a24c6f6e