feat: Phase 4 — reusable "subscribe" workflow (+ dogfood it) #8
Reference in New Issue
Block a user
Delete Branch "feat/phase4-reusable-wf"
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?
Phase 4 of the gadfly-games build — the last one.
Centralizes the ~90-line consumer stub into a reusable Gitea workflow so a repo can subscribe to Gadfly with a tiny caller. Feasibility was probe-verified on your act_runner first:
workflow_callruns,secrets: inheritdelivers the secret, and a fully-qualifiedowner/repo/path@refresolves (both probes cleaned up).What's here
.gitea/workflows/review-reusable.yml—on: workflow_calljob holding the image pin + all env plumbing. Inputs (models,specialists,provider, concurrency, timeouts,allowed_users, …) default to"", so an empty value falls back to the image's own default — the caller overrides only what it wants. Provider secrets come viasecrets: inherit(optional ones resolve empty when unset).adversarial-review.yml— gadfly's own dogfood is now a thin caller of the reusable workflow. This proves it end-to-end (this PR's own review runs through it) and is the project's dogfooding ethos; safe because reviews are advisory.examples/reusable.yml— the slim ~8-line consumer stub (uses: steve/gadfly/.gitea/workflows/review-reusable.yml@…+secrets: inherit).Known limitation
Consumers with arbitrary
GADFLY_ENDPOINT_<NAME>secrets still need the full self-contained stub — a reusable workflow can't enumerate dynamic secret names (the common M1/M5 + provider keys are mapped). Documented.Note
This PR's own review is the live test of the reusable path. If Gitea doesn't propagate the caller's
github.eventcontext into the reusable workflow, the review may not run (advisory — harmless, and I'll see it + fix). YAML validated; Go unchanged (build +test -racegreen).🤖 Generated with Claude Code
🪰 Gadfly — live review status
9/9 reviewers finished · updated 2026-06-27 23:29:11Z
claude-code/opus· claude-code — ✅ doneclaude-code/opus:max· claude-code — ✅ doneclaude-code/sonnet· claude-code — ✅ donedeepseek-v4-pro:cloud· ollama-cloud — ✅ doneglm-5.1:cloud· ollama-cloud — ✅ doneglm-5.2:cloud· ollama-cloud — ✅ doneminimax-m3:cloud· ollama-cloud — ✅ donenemotron-3-super:cloud· ollama-cloud — ✅ doneqwen3-coder:480b-cloud· ollama-cloud — ✅ doneLive status board. Findings are posted in each model's own comment. Advisory only — does not block merge.
🪰 Gadfly review —
glm-5.1:cloud(ollama-cloud)Verdict: Minor issues — 3 reviewers: security, correctness, error-handling
🔒 Security — Minor issues
Now I have all the information I need. Let me assess each finding:
Finding 1:
examples/reusable.yml:43—@mainmutable ref withsecrets: inherit@main, andsecrets: inheritis on line 44.Finding 2:
review-reusable.yml— nopermissions:blockpermissionsinreview-reusable.ymland found no matches. The reusable workflow indeed lacks apermissions:block.uses:inherit the permissions from the calling workflow unless the reusable workflow itself specifiespermissions:. The callers (adversarial-review.ymlandexamples/reusable.yml) both setpermissions: {contents: read, issues: write, pull-requests: write}.permissionswould run with defaults. This is a valid point. The finding is confirmed.Finding 3:
secrets: inheritpasses all caller secrets without explicit declarationsecrets: inherit(both in the dogfood caller and the example), and theworkflow_call:section has nosecrets:declarations — it only hasinputs:.secrets: inheritwas chosen and that undefined secrets resolve to empty. The dynamic secret name issue is also documented in the comment aboutGADFLY_ENDPOINT_<NAME>.All three findings are confirmed against the actual code. Let me output the corrected review.VERDICT: Minor issues
examples/reusable.yml:43—@mainmutable ref withsecrets: inherit. The example usesuses: steve/gadfly/.gitea/workflows/review-reusable.yml@main, a mutable reference that receives all of the caller's secrets viasecrets: inherit(line 44). If the gadfly repo'smainbranch is compromised, an attacker could modify the reusable workflow to exfiltrate every secret from every consumer. The comment on line 10 says "Pin @ to a Gadfly tag/branch," but the actual code defaults to@main. Most users copy-paste examples without reading inline comments. Suggest changing the example to use a pinned SHA/tag as the default (e.g.,@v1or@sha-c342bdb) and demoting@mainto a comment, or adding a prominent WARNING comment about the supply-chain risk of an un-pinned ref combined withsecrets: inherit.review-reusable.yml— nopermissions:block (defense-in-depth gap). The reusable workflow (lines 42-84) lacks an explicitpermissions:block. Both existing callers (adversarial-review.ymlline 22 andexamples/reusable.ymlline 27) setpermissions: {contents: read, issues: write, pull-requests: write}, which scopesGITEA_TOKENcorrectly. However, the reusable workflow itself doesn't enforce this — a future caller that omitspermissionswould run with the Gitea default (potentially write-all). Addingpermissions:directly to the reusable job would provide belt-and-suspenders least-privilege, ensuringGITEA_TOKENis always scoped regardless of caller configuration.review-reusable.yml—secrets: inheritpasses all caller secrets without explicit declaration. The reusable workflow accesses secrets exclusively throughsecrets: inherit, meaning it receives every org/repo secret the caller has — not just the ~8 it actually maps as env vars (lines 60-72). Theworkflow_call:section (lines 26-40) declaresinputs:but nosecrets:, so there is no enumerated contract limiting which secrets are received. The alternative (declaring explicitsecrets:underworkflow_call:) would limit the blast radius if the gadfly image or reusable YAML is ever compromised. The comments (lines 13-18) acknowledge this as a deliberate convenience trade-off for dynamic secret names, and theGADFLY_ENDPOINT_<NAME>limitation is documented (lines 65-67). No action required, but any future secret-mapping additions expand the trust surface — explicit declarations would enforce least-privilege by default.🎯 Correctness — Minor issues
All five findings check out against the actual code. Let me verify one detail — whether the
specialist_suitecomment already addresses the "worth a comment" suggestion:Line 34 already has
# reserved / future, so the suggestion to add a comment is already partially satisfied, though the input still silently discards values since there's noGADFLY_SPECIALIST_SUITEenv mapping.All findings confirmed. Verdict remains "Minor issues."
🧯 Error handling & edge cases — Minor issues
Now I have enough information to verify both findings.
Finding 1 (specialist_suite not wired): Confirmed. The input is declared at line 34 with comment "reserved / future", but there is no
GADFLY_SPECIALIST_SUITEenv var in the env block (lines 73–84). A grep forGADFLY_SPECIALIST_SUITEacross the entire repo returns zero results. A caller who setsspecialist_suiteviawith:gets no effect and no error.Finding 2 (timeout_minutes default): The draft claims "the old self-contained stubs all used 90." This is false. All five existing example stubs use
timeout-minutes: 30(verified:adversarial-review.yml:44,claude-code.yml:45,endpoint-aliases.yml:48,local-ollama.yml:39,openai-compatible.yml:41). The only workflow that ever used 90 was gadfly's ownadversarial-review.yml, and it still does (viawith: timeout_minutes: 90). The reusable workflow's default of 30 matches the established convention in all examples, so there is no regression. This finding's core claim is factually wrong and must be dropped.VERDICT: Minor issues
review-reusable.yml:34—specialist_suiteinput is accepted but never wired to an env var. The input is declared withtype: string, default: ""and the comment says "reserved / future", but it's passed through thewith:interface and then silently dropped — there's noGADFLY_SPECIALIST_SUITE: ${{ inputs.specialist_suite }}in the env block. A caller who setsspecialist_suitegets no error and no effect, which is a swallowed/no-op config path. If it's truly reserved, consider either wiring it (even to a no-op env var the image can later consume) or removing it from the interface until it's functional, so callers don't form incorrect expectations.Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 3m 8s
🪰 Gadfly review —
claude-code/opus(claude-code)Verdict: Minor issues — 3 reviewers: security, correctness, error-handling
🔒 Security — No material issues found
I've verified the security-critical paths. Here's my review.
VERDICT: No material issues found (Security lens)
This change is a structural refactor — it moves the image-pin and env plumbing into a
workflow_callreusable workflow and turnsadversarial-review.ymlinto a thin caller. I traced the trust boundary that actually matters (who can trigger a secret-bearing run) and it is preserved.What I verified:
Actor gate survives the move. The comment-trigger gate (
github.actor∈ steve/fizi/dazed) stays on the caller job in.gitea/workflows/adversarial-review.yml:36-39, which now doesuses: ./.gitea/workflows/review-reusable.yml. A reusable-workflow call is gated by the calling job'sif:, so an untrustedissue_commentis dropped before the reusable (and thus the secret-bearing container) ever runs. No regression vs. the old inline gate.Empty
allowed_usersdefault is fail-safe, not fail-open. The reusable defaults every input — includingallowed_users— to""(review-reusable.yml:38), soGADFLY_ALLOWED_USERScan reach the container empty. I checkedentrypoint.sh:68-83: with an empty allow-list,actor_allowed()falls back to a Gitea collaborator (write-access) check (/collaborators/${actor}→ HTTP 204), not to allowing everyone. So even a consumer who omits the job-levelif:gate still only lets write-access users comment-trigger. Good insecure-default avoidance.secrets: inheritdoes not broaden exposure. The reusable references only named secrets (GITEA_TOKEN, provider keys,GADFLY_ENDPOINT_M1/M5, findings URL/token).inheritpasses the caller's existing secrets; for a fork PR no secrets are available to inherit, matching the prior behavior. Optional secrets resolve to empty when unset.No new injection vector. The untrusted-input env vars (
COMMENT_BODY,ACTOR,PR_BRANCH) are copied verbatim from the old workflow.COMMENT_BODYis only glob-matched in acase(entrypoint.sh:95-98) andACTORonly interpolated into a Gitea API path for a username — both unchanged frommainand neithereval'd.One non-blocking observation (verified, not a defect): splitting the gate (now in the consumer's caller file) from the run (in the reusable) makes it easier for a consumer to forget the job-level
if:than when both lived in one file. The in-container collaborator fallback above backstops this, so the worst case is limited to write-access users — acceptable. Theexamples/reusable.ymltemplate correctly ships both theif:gate andallowed_users:(belt-and-suspenders), which is the right guidance to keep.Nothing in the security lens blocks this PR.
🎯 Correctness — Minor issues
Both files match the diff exactly. I've confirmed:
review-reusable.yml:75) readsGADFLY_SPECIALISTSonly from${{ inputs.specialists }}— there is novars.GADFLY_SPECIALISTSfallback anywhere in it.adversarial-review.yml:46) hardcodesspecialists: "security,correctness,error-handling".${{ vars.GADFLY_SPECIALISTS || 'security,correctness,error-handling' }}.So the repo-var override path is genuinely removed for gadfly's own repo — the minor finding holds against the actual code. The behavior-preservation and "empty ⇒ image default" claims are background, not findings, and don't change the verdict.
VERDICT: Minor issues
.gitea/workflows/adversarial-review.yml:46— The old stub setGADFLY_SPECIALISTS: ${{ vars.GADFLY_SPECIALISTS || 'security,correctness,error-handling' }}, letting the repo varGADFLY_SPECIALISTSoverride the lens suite without editing the workflow. The new caller hardcodesspecialists: "security,correctness,error-handling", and the reusable workflow (review-reusable.yml:75) reads only${{ inputs.specialists }}with novars.GADFLY_SPECIALISTSpath, so that override is silently lost for gadfly's own repo. Impact is small and likely intentional (config simplification for the dogfood caller), but it's a real behavior change. If the override is meant to survive, the caller should passspecialists: ${{ vars.GADFLY_SPECIALISTS || 'security,correctness,error-handling' }}.🧯 Error handling & edge cases — No material issues found
I've verified the central edge-case risk in this change. Since the diff is config plumbing, the one real error-handling concern it introduces is that the reusable workflow always sets the config env vars — to
""when the caller doesn't override — whereas a hand-written stub previously left them absent. So the whole design rests on "empty string is treated identically to unset" everywhere those vars are consumed. I checked each consumer:cmd/gadfly/main.go:450envInt— empty → default; also guardsn <= 0. ✓ (coversGADFLY_TIMEOUT_SECS,GADFLY_MAX_STEPS)cmd/gadfly/main.go:291providerOverride— empty splits to a single un-Cut-able item →(0,false)fallback. ✓ (GADFLY_PROVIDER_CONCURRENCY/..._LENS_CONCURRENCY)cmd/gadfly/specialists.go:136— gated onTrimSpace(...) != "", so empty → default suite. ✓cmd/gadfly/model.go:55,59,115—TrimSpace+!= ""checks forGADFLY_PROVIDER/GADFLY_BASE_URL/GADFLY_WORKER_MODEL. ✓entrypoint.sh:53,71,156,159—${VAR:-default}and[ -n ... ]treat empty as unset forGADFLY_TRIGGER_PHRASE,GADFLY_ALLOWED_USERS,GADFLY_MODELS,GADFLY_PROVIDER. ✓The "empty ⇒ image default" contract the PR depends on holds at every read site I could find. The unhappy paths the change actually touches (missing required event context) are unchanged from before:
entrypoint.sh:60-63still:?-exits on absentPR/EVENT_NAME, which is a config-error exit explicitly permitted by goal #2 — not introduced here.VERDICT: No material issues found
Through the error-handling & edge-cases lens, this change is clean:
""is read through a path that treats empty as "use default" (envInt,providerOverride,TrimSpace != ""guards, shell:-/-n). No empty value reaches astrconv.Atoi/parse that would error or yield a degenerate0. The one numeric input rendered directly in YAML,timeout_minutes(review-reusable.yml:31→ used at:37), istype: number, default: 30, so it can't arrive empty.reviewWithSpecialist,main.go:304-309) and entrypoint's|| log "... (continuing)"fault tolerance are untouched.One adjacent observation I could not verify and am therefore not filing as a finding: the PR's own "Note" flags that Gitea may not propagate the caller's
github.eventinto the reusable workflow. If it doesn't,PR/EVENT_NAMEresolve empty andentrypoint.sh:60-63exits 1 — but that's a permitted config-error exit, advisory, and on a non-required check, so even in the worst case it doesn't violate the never-block invariant. Whether the propagation actually works is a behavior/CI question outside my lens (and the author already plans to confirm it via this PR's live run).Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 6m 22s
🪰 Gadfly review —
claude-code/sonnet(claude-code)Verdict: Minor issues — 3 reviewers: security, correctness, error-handling
🔒 Security — Minor issues
Minor issues
Finding 1 —
examples/reusable.ymlpins to@mainwhile passingsecrets: inheritFile:
examples/reusable.yml:43Any consumer who copies this stub verbatim flows their entire secret store (
secrets: inherit) into whatever code happens to be at the tip of Gadfly'smainbranch at run-time — including any future push, force-push, or repo compromise. This is textbook supply-chain risk: the trust boundary is the live branch pointer, not a reviewed, immutable commit.The in-file comment at line 10 says "Pin @<ref> to a Gadfly tag/branch" — but the actual
uses:line still reads@mainand does not show what the pinned form looks like. A user copying the file verbatim will not pin.The docker image in
review-reusable.ymlis already correctly pinned to:sha-c342bdb(line 47) — the same discipline should apply to the workflow ref in the example stub.Suggested fix: Change the example
uses:to a concrete pinned ref (tag or commit SHA) and add a# TODO: update on each Gadfly releasecomment so the instruction is actionable:🎯 Correctness — Minor issues
Both findings are confirmed. Here is the corrected review:
Minor issues
specialist_suiteinput wired to nothing —review-reusable.yml:34declaresspecialist_suite: { type: string, default: "" }underworkflow_call.inputs, but no corresponding env entry appears in theenv:block (lines 74–84 map every other input;specialist_suiteis absent). The inline comment# reserved / futureshows this is intentional, but a consumer who passesspecialist_suite: "some-suite"will see it silently ignored with no warning. Fix: either addGADFLY_SPECIALIST_SUITE: ${{ inputs.specialist_suite }}to the env block when the feature is implemented, or remove the declaration and reintroduce it alongside the implementation.vars.GADFLY_SPECIALISTSruntime override silently dropped — The oldadversarial-review.ymlsetGADFLY_SPECIALISTS: ${{ vars.GADFLY_SPECIALISTS || 'security,correctness,error-handling' }}, explicitly advertisingvars.GADFLY_SPECIALISTSas an operational override knob. The new file (adversarial-review.yml:46) replaces that with a hard-codedspecialists: "security,correctness,error-handling"in thewith:block, andreview-reusable.yml:75maps that toGADFLY_SPECIALISTS: ${{ inputs.specialists }}. There is no path by which the repo variable now reaches the container. Repos wherevars.GADFLY_SPECIALISTSwas set to"all"or"auto"will silently revert to the three-lens suite. Fix: usespecialists: "${{ vars.GADFLY_SPECIALISTS || 'security,correctness,error-handling' }}"as thewith:value inadversarial-review.yml, or document the removal explicitly.🧯 Error handling & edge cases — Minor issues
Both findings are confirmed against the actual code:
entrypoint.sh:62does contain: "${PR:?PR required}"— a strict bail-out that fires beforerun.shis ever called, with no graceful PR comment fallback.specialist_suiteappears only atreview-reusable.yml:34in theinputs:stanza and has no corresponding entry in theenv:block (lines 73–84 map every other input but skipspecialist_suite).Minor issues
PRrequired but no graceful fallback onworkflow_callcontext propagation failure —review-reusable.ymlevaluates${{ github.event.pull_request.number || github.event.issue.number || github.event.inputs.pr_number }}to populatePR. In a standardworkflow_callthe called workflow inherits the caller'sgithub.event, sogithub.event.inputs.pr_numbershould resolve when the caller was triggered byworkflow_dispatch. But if Gitea'sworkflow_callisolates the event context (the author calls this out as a known risk), all three expressions evaluate to empty,PRis"", andentrypoint.sh:62hard-exits via${PR:?PR required}beforerun.shis ever invoked. Consequence: no PR comment is posted, the Docker step exits non-zero, the reusable workflow job fails, and the caller'sreview:job shows as failed — silently from the reviewer's point of view (no explanation on the PR). Verified:entrypoint.shline 62 is a strict-expansion bail-out;run.sh's graceful error-comment logic is only reached after the model loop, which is never entered. This is a new failure mode — in the pre-refactor self-contained job the expression was always evaluated in the triggering event's context. Suggested fix: addcontinue-on-error: trueon the docker step inreview-reusable.yml, or add a pre-check inentrypoint.shthat posts a "could not determine PR number" notice and exits 0 whenPRis unset.specialist_suiteinput declared but never wired —review-reusable.ymldefinesspecialist_suite: { type: string, default: "" }(marked "reserved / future") but there is no corresponding env var in theenv:block. Verified:specialist_suiteappears only atreview-reusable.yml:34in theinputs:stanza; no other file in the repo references it. A consumer who sets this input gets silent discard. The comment documents it as reserved, so this won't surprise readers of the source, but at runtime there is no warning. Low severity given the explicit "reserved" label, but worth noting: if a consumer setsspecialist_suite: "...", they will see no effect and no error.Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 14m 40s
🪰 Gadfly review —
minimax-m3:cloud(ollama-cloud)Verdict: Blocking issues found — 3 reviewers: security, correctness, error-handling
🔒 Security — No material issues found
Confirmed:
GADFLY_FINDINGS_URLflows unvalidated intohttp.NewRequest. This is a pre-existing issue not introduced by this PR.Now I have enough to render a verdict.
Verdict: No material issues found (security lens)
This is a refactor — it relocates ~90 lines of workflow YAML into a reusable
workflow_callworkflow and has gadfly dogfood it. Walking the change through the 🔒 Security lens specifically:if:comment-trigger actor gate is unchanged (was already caller-side).permissions:block (contents: read; issues: write; pull-requests: write) is preserved verbatim. The reusable workflow deliberately doesn't re-gate — the same trust model as before (caller's secrets, caller's allow-list).GADFLY_MODELS,GADFLY_SPECIALISTS,GADFLY_PROVIDER_CONCURRENCY, etc.) that the binary already parses.GADFLY_FINDINGS_URLwas already consumed as a base URL incmd/gadfly/emit.go:97(passed straight tohttp.NewRequestatemit.go:370); this PR only moves where the env var is plumbed, not how it's used. The pre-existing lack of URL/scheme validation is not introduced here — out of scope for this diff.secrets: inherit: Deliberate; the whole point. The reusable workflow is in the same repo (or pinned via@reffor cross-repo consumers), so the trust boundary equals the prior stub's. Undefined secrets resolve to empty per Gitea — verified by readingreview-reusable.yml:60-69. No secret is logged.gitea.stevedudenhoeffer.com/steve/gadfly:sha-c342bdbis moved from caller into reusable (line 47) — no downgrade, same digest-style tag.examples/reusable.yml:42ships the literal placeholdergithub.actor == 'your-username'— it's a copy-paste template, not real config, andsecrets: inheritis required for it to function; consumers must edit before merging. This is consistent withexamples/adversarial-review.yml:42(sameyour-usernamepattern). Not a regression — and clearly a template.specialist_suiteinput (review-reusable.yml:34) is declared but unused (commentedreserved / future). Dead config, no security impact.Verified by reading:
.gitea/workflows/review-reusable.yml,.gitea/workflows/adversarial-review.yml,examples/reusable.yml,examples/adversarial-review.yml,entrypoint.sh(lines 67–114 for the in-container allow-list + collaborator fallback),cmd/gadfly/emit.go(lines 96–161 for the findings telemetry consumer), and the relevant env flow throughentrypoint.sh:139-142.Outside my lens — and pre-existing, not introduced here — but worth flagging once:
cmd/gadfly/emit.go:97and:153,157build the findings POST URL asstrings.TrimRight(base, "/") + "/runs|"/reports"with no scheme check, no host allowlist, and no rejection offile:///http://internal-host. Anyone who can set theGADFLY_FINDINGS_URLsecret on a consumer repo can turn this into an internal-network POST. Sincesecrets: inheritnow delivers this secret into the reusable workflow without per-caller scrutiny, a misconfigured consumer could exfiltrate review telemetry to an arbitrary URL. The PR doesn't change the behavior, only the plumbing — so it's not a regression in this diff — but the refactor is a reasonable moment to harden it (e.g. requirehttps://and an optional allowlist of hosts).🎯 Correctness — Blocking issues found
VERDICT: Blocking issues found
.gitea/workflows/adversarial-review.yml:46—specialistsinput hardcoded, drops thevars.GADFLY_SPECIALISTSrepo-var override. The previous inline form wasGADFLY_SPECIALISTS: ${{ vars.GADFLY_SPECIALISTS || 'security,correctness,error-handling' }}. The new caller hardcodesspecialists: "security,correctness,error-handling"and ignoresvars.GADFLY_SPECIALISTSentirely, so a repo var set toautoorall(or any custom csv) is silently lost. Fix:specialists: ${{ vars.GADFLY_SPECIALISTS || '' }}so the var wins and empty falls back..gitea/workflows/review-reusable.yml:75+cmd/gadfly/specialists.go:136— "empty ⇒ image default" is FALSE forspecialists.cmd/gadfly/specialists.go:136iscase strings.TrimSpace(os.Getenv("GADFLY_SPECIALISTS")) != "":— when the env is empty, control falls through to the file config (.gadfly.yml) and then todefaultSuite(line 141:security, correctness, maintainability, performance, error-handling). For a consumer with no.gadfly.ymlthis is the 5-lens default suite, NOT the 3-lenssecurity,correctness,error-handlingthe old inline form hardcoded. So the reusable's "caller overrides only what it wants" promise holds for the file-absent case (you get the full default suite, which is a strict superset of the old behavior) but breaks the old contract: callers that relied on the inline form getting exactlysecurity,correctness,error-handlingnow silently get two extra lenses (maintainability,performance). The reusable should declare a non-empty default forspecialists(the oldsecurity,correctness,error-handling), or the caller should passspecialists: "security,correctness,error-handling"explicitly — which it does, so this is partially mitigated for the dogfood caller but the contract is still misleading..gitea/workflows/review-reusable.yml:40—timeout_minutes: 30default is too low. The documented fleet is 6 cloud + 3 claude-code models × 3 lenses × 2 passes, all concurrent.cmd/gadfly/main.go:86setsdefaultTimeoutSecs = 300per lens, andentrypoint.sh:156defaults to one cloud model. The old inline dogfood caller usedtimeout-minutes: 90. Any consumer copyingexamples/reusable.ymlwithout overridingtimeout_minuteswill hit the 30-min wall-clock cap mid-suite and get an incomplete truncated review. Either raise the default to 90 or document the override prominently. (examples/reusable.yml:46comment "All optional — omit to take Gadfly's defaults" does not warn of truncation.)examples/reusable.yml:42— actor gate missing thepull_requestcheck. The dogfood caller at.gitea/workflows/adversarial-review.yml:36-41gates issue_comment ongithub.event.issue.pull_request && (github.actor == 'steve' || ...). The example's gategithub.event_name != 'issue_comment' || github.actor == 'your-username'lacks thepull_requestsub-check, so a comment on a plain issue byyour-usernamecontaining the trigger phrase passes the workflow-level gate, thenentrypoint.sh:103(API "${GITEA_API}/pulls/${PR}") 404s and exits 0 — harmless but wasteful (full image pull + clone before bail). Match the dogfood pattern..gitea/workflows/review-reusable.yml:34—specialist_suiteinput is dead code. Verified:grepforspecialist_suiteandGADFLY_SPECIALIST_SUITEacross the repo returns only this one declaration. No env consumer inentrypoint.sh(which readsGADFLY_TRIGGER_PHRASE,GADFLY_ALLOWED_USERS,GADFLY_FINDINGS_*etc.) and no binary reader incmd/gadfly/. The comment "reserved / future" is honest but the input is misleading cruft — drop it or implement it.🧯 Error handling & edge cases — Minor issues
VERDICT: Minor issues
.gitea/workflows/review-reusable.yml:40+examples/reusable.yml— defaulttimeout_minutes: 30is a footgun for new callers. Verified: the reusable's input default is 30 (review-reusable.yml:40), andexamples/reusable.yml:43-46passes notimeout_minutes, so a copy-pasting consumer inherits 30. Confirmed againstentrypoint.sh:52(image default model isqwen3-coder:480b-cloud) andcmd/gadfly/main.go:86/main.go:310(per-lens timeout default 300s × 3 lenses already exceeds the 30-min cap under contention). The dogfood file.gitea/workflows/adversarial-review.yml:52overrides totimeout_minutes: 90, confirming the author's own floor. Fix: bump default to 90 (matching dogfood) or haveexamples/reusable.ymlpasstimeout_minutes: 90..gitea/workflows/review-reusable.yml:55—IS_DRAFT: ${{ github.event.pull_request.draft }}is fragile for non-PR triggers. Verified:entrypoint.sh:90guards withif [ "${IS_DRAFT:-false}" = "true" ], so empty falls through correctly today. Note this exact expression is unchanged from the pre-PR pattern (still present unchanged inexamples/adversarial-review.yml:108,examples/claude-code.yml:75,examples/endpoint-aliases.yml:64,examples/local-ollama.yml:53,examples/openai-compatible.yml:56), so the PR doesn't introduce new breakage — it inherits an existing brittleness into the new reusable. Fix (light): defensively|| falseor wrap intoJSON(...)for Gitea's expression evaluator.(Dropped: the third claim about "caller-event context propagation" — speculative, author-flagged, depends on host
workflow_callsemantics which the verification cannot confirm against the repo. The author's own comment block in the reusable yml is the right place for that caveat.)Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 1m 59s
🪰 Gadfly review —
glm-5.2:cloud(ollama-cloud)Verdict: Minor issues — 3 reviewers: security, correctness, error-handling
🔒 Security — Minor issues
VERDICT: Minor issues
Investigated through the security lens. The core authz model is preserved: the actor
if:gate stays at the caller (adversarial-review.yml:36-41,examples/reusable.yml:40-42), and the image remains pinned to an immutable:sha-c342bdbtag in the reusable workflow (review-reusable.yml:47).secrets: inheritonly exposes secrets that are explicitly mapped into the container env, so unmapped secret names aren't leaked into the run. No injection surface is added (inputs are maintainer-authored config, not PR-author-controlled). Verified by readingreview-reusable.yml,adversarial-review.yml,examples/reusable.yml.Findings:
examples/reusable.yml:43— insecure mutable ref for the consumer template. The example tells consumers to useuses: steve/gadfly/.gitea/workflows/review-reusable.yml@main, and the header only says "Pin@<ref>to a Gadfly tag/branch." Because the caller passessecrets: inherit, a compromise ofsteve/gadfly'smainbranch (e.g. a merged malicious PR) would let the attacker author a new workflow file that exfiltrates all of the consumer's inherited secrets (OLLAMA_CLOUD_API_KEY,GITEA_TOKEN,CLAUDE_CODE_OAUTH_TOKEN, etc.) or swap thedocker://image pin for a malicious image. This is a real supply-chain / insecure-default issue for the recommended consumer path. Suggest defaulting the template to an immutable sha (or at least a non-moving signed tag) and changing the guidance to "pin to an immutable commit sha, not a branch." Verified by readingexamples/reusable.yml:10,43.review-reusable.ymlhas nopermissions:block. Not a regression (the dogfood calleradversarial-review.yml:22-25and the exampleexamples/reusable.yml:27-30both setcontents: read, issues: write, pull-requests: write, and reusable workflows inherit the caller's permissions), but worth a note: a consumer who drops thepermissions:block from their caller would get the runner's default (often broad) token scope for a job that handles secrets and writes comments. Consider setting an explicit restrictivepermissions:in the reusable workflow as defense-in-depth. Verified by readingreview-reusable.yml:42-84.No other material security issues found in this lens.
🎯 Correctness — Minor issues
Verdict: Minor issues
.gitea/workflows/review-reusable.yml:49-58— correctness depends ongithub.event.*propagation into theworkflow_calljob, which is the one part of the refactor not probe-verified. The reusable job readsgithub.event.pull_request.number,github.event.comment.body,github.event.comment.id,github.head_ref,github.event.pull_request.draftfrom the called workflow's context. I confirmedentrypoint.sh:62-63does: "${PR:?PR required}"and: "${EVENT_NAME:?EVENT_NAME required}", andPR:in the reusable is built solely from thosegithub.event.*references (line 53). If this Gitea act_runner version does not forward the caller's event payload into the called workflow,PRis empty,entrypoint.shdies withPR required, and no review runs (this PR's review included). The file header (lines 15-16) only claimssecrets: inheritwas verified — notgithub.event.*context inheritance. Suggested fix: run one real PR through the reusable path and confirmPR/COMMENT_BODYare non-empty in the called job; if Gitea blanks them, pass the event context explicitly viawith:inputs (pr_number,event_name,comment_body, …) from the caller'sgithub.*and forward them as env, removing the dependence on context inheritance entirely..gitea/workflows/review-reusable.yml:59-64—GADFLY_API_KEY(the generic provider-key env) is not mapped; only the provider-specific keys are. I verifiedentrypoint.sh:38documentsGADFLY_API_KEYas the provider-key fallback,cmd/gadfly/model.go:60readsos.Getenv("GADFLY_API_KEY"), andscripts/run.sh:151checks it for the ollama-cloud key path. The reusable mapsOLLAMA_CLOUD_API_KEY/OPENAI_API_KEY/ANTHROPIC_API_KEY/GOOGLE_API_KEY/CLAUDE_CODE_OAUTH_TOKEN, but notGADFLY_API_KEY. A consumer whose provider key is only available as the secretGADFLY_API_KEY(not one of the named provider envs) gets an empty key through the reusable path and silently fails auth. Suggested fix: addGADFLY_API_KEY: ${{ secrets.GADFLY_API_KEY }}to the provider-auth block (it resolves empty when unset, same as the others)..gitea/workflows/review-reusable.yml:34—specialist_suiteinput is declared but never plumbed to any env var. The input is commentedreserved / future, and a repo-wide grep confirms no env references it — it is dead, user-visible config surface that does nothing. Either wire it to a planned env or drop it to avoid implying it works.🧯 Error handling & edge cases — Minor issues
Minor issues
examples/reusable.yml:40-42— the public template'sif:gate (github.event_name != 'issue_comment' || github.actor == 'your-username') lacks thegithub.event.issue.pull_requestguard that the dogfood caller (adversarial-review.yml:36-41) has. Verified: anissue_commenton a plain issue by the matching actor passes the gate and only gets rejected later inside the container atentrypoint.sh:103-105(whichexit 0s harmlessly). Impact is minor — no crash and the run no-ops — but it wastes a secret-bearing runner invocation for every qualifying issue comment. Worth addinggithub.event.issue.pull_request &&to the example'sif:, matching the dogfood file.No swallowed errors, missing defers, panics, or nil/empty-collection crashes were introduced by the diff. The reusable workflow's
default: ""inputs degrade cleanly to the image/entrypoint built-in defaults (verified inentrypoint.sh:53forGADFLY_TRIGGER_PHRASEandentrypoint.sh:71-82forGADFLY_ALLOWED_USERS), and the dogfood caller passes explicit values for the config it needs.Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 1m 54s
🪰 Gadfly review —
deepseek-v4-pro:cloud(ollama-cloud)Verdict: Minor issues — 3 reviewers: security, correctness, error-handling
🔒 Security — Minor issues
Verdict: Minor issues
examples/reusable.yml:40-42— Actor gate is a single-username placeholder with no defense-in-depth for theissue_commenttrigger. The example'sif:gate checks onlygithub.actor == 'your-username', and theallowed_usersinput is also"your-username". If a consumer copies this verbatim and only changes the username in one place but not the other, the two gates will be out of sync. More importantly, the example'sif:gate has nogithub.event.issue.pull_requestguard — unlike the dogfood caller atadversarial-review.yml:38which checksgithub.event.issue.pull_requestbefore evaluating the actor. Without that guard, a comment on a plain issue (not a PR) would pass theif:gate (sincegithub.event_name != 'issue_comment'is false, andgithub.actor == 'your-username'could be true), causing the job to start, pull the image, and inject secrets — only to have the entrypoint later discover the issue isn't a PR and exit. The entrypoint'sactor_allowed+ PR-existence check (line 103) still prevents any actual review, but secrets are already in the container's environment at that point. The dogfood caller correctly nests the actor check insidegithub.event.issue.pull_request; the example should mirror that pattern. (Verified by reading bothadversarial-review.yml:36-41andexamples/reusable.yml:40-42.).gitea/workflows/review-reusable.yml:51—GITEA_TOKENis implicitly required but never declared as a secret input. The reusable workflow consumes${{ secrets.GITEA_TOKEN }}(the built-in Actions token) viasecrets: inherit. This works because Gitea automatically providesGITEA_TOKENto every workflow. However, if a consumer were to usesecrets: inheritfrom a context whereGITEA_TOKENis somehow not available (e.g., a workflow withpermissions: {}or a misconfigured runner), the container would fail at the: "${GITEA_TOKEN:?GITEA_TOKEN required}"check inentrypoint.sh:61. This is a robustness concern rather than a security vulnerability — the token is always present in normal Gitea Actions operation. The example caller correctly setspermissions: { issues: write, pull-requests: write, contents: read }which ensures the token has the needed scopes. (Verified by readingentrypoint.sh:60-63andexamples/reusable.yml:27-30.)No injection, authz-bypass, or credential-leakage vulnerabilities introduced by this PR. The entrypoint's
actor_allowedcheck (line 68-83) and theCOMMENT_BODYglob-match (line 95-96) remain the same safe patterns as before. TheACTORvalue interpolated into the API URL path atentrypoint.sh:81is a pre-existing concern (Gitea usernames are restricted to safe characters, so it's low-risk), not introduced here.🎯 Correctness — Minor issues
Verdict: Minor issues
specialist_suiteinput is declared but never plumbed to any env var —.gitea/workflows/review-reusable.yml:34. Theworkflow_callacceptsspecialist_suite(comment says "reserved / future"), but theenv:block (lines 73–84) has noGADFLY_SPECIALIST_SUITE: ${{ inputs.specialist_suite }}mapping. I verified by reading the full file and grepping the entire repo: no Go code readsGADFLY_SPECIALIST_SUITEeither. A consumer who setsspecialist_suite: "something"gets a silent no-op — the value is accepted and discarded. Either remove the input until it's actually wired up, or add the env mapping (and the Go-side reader) now.🧯 Error handling & edge cases — No material issues found
No material issues found.
I verified the full chain — reusable workflow → entrypoint.sh → Go binary — for every env var the diff introduces. The design consistently handles the empty-string default correctly at each layer:
GADFLY_SPECIALISTS,GADFLY_TIMEOUT_SECS,GADFLY_MAX_STEPS,GADFLY_WORKER_MODEL: empty string → Go'senvInt/strings.TrimSpacechecks fall through to built-in defaults (specialists.go:136,main.go:450-459,model.go:115).GADFLY_PROVIDER_CONCURRENCY: empty string →provider_capinentrypoint.sh:167-176iterates one empty element, finds no match, returnsDEFAULT_CONC(1). Malformed values like"abc"or"0"are caught by[ "$cap" -ge 1 ] || cap=1at line 208.GADFLY_PROVIDER_LENS_CONCURRENCY: empty string →providerOverrideinmain.go:291-301splits""into[""],strings.Cutreturns!ok, loop exits, returns0, false→ default used.GADFLY_TRIGGER_PHRASE: empty string →${GADFLY_TRIGGER_PHRASE:-@gadfly review}atentrypoint.sh:53substitutes the default (bash:-covers both unset and empty).GADFLY_ALLOWED_USERS: empty string →[ -n "${GADFLY_ALLOWED_USERS:-}" ]is false atentrypoint.sh:71, falls through to the collaborator check.GADFLY_PROVIDER: empty string →${GADFLY_PROVIDER:-ollama-cloud}atentrypoint.sh:159substitutes the default.GADFLY_MODELS: empty string → the chained${GADFLY_MODELS:-${OLLAMA_REVIEW_MODELS:-$DEFAULT_MODELS}}atentrypoint.sh:156resolves toqwen3-coder:480b-cloud.GITEA_TOKEN, etc.): if a consumer lacks them, they resolve empty →entrypoint.sh:61fails with: "${GITEA_TOKEN:?GITEA_TOKEN required}"— a clear, early failure. Optional secrets (Claude Code, findings store, foreman endpoints) are empty → harmlessly disabled.timeout_minutes: defaults to 30; the dogfood caller overrides to 90. Thetype: numberinput with a sensible default means a consumer who omits it gets a reasonable wall-clock cap.The
if:guard inexamples/reusable.yml:41-42omits thegithub.event.issue.pull_requestcheck that the dogfood caller has, so a comment on a plain issue would briefly start a job — butentrypoint.sh:103-105catches it and exits 0 before any review work. Minor inefficiency, not a correctness or safety issue.Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 8m 11s
🪰 Gadfly review —
nemotron-3-super:cloud(ollama-cloud)Verdict: No material issues found — 3 reviewers: security, correctness, error-handling
🔒 Security — No material issues found
No material issues found.
🎯 Correctness — No material issues found
No material issues found
The changes introduce a reusable workflow for the adversarial review job, replacing the previous inline workflow with a call to
.gitea/workflows/review-reusable.yml. After verifying the changes:secrets: inheritNo correctness issues in logic, behavior, or configuration mapping were found. The change correctly centralizes the workflow while maintaining identical functionality.
Note: Verified that the reusable workflow's input definitions match usage in the dogfood workflow and examples, and that all environment variable mappings are correct. The change is purely refactoring with no behavioral alteration.
🧯 Error handling & edge cases — No material issues found
No material issues found
The changes introduce a reusable workflow and update the caller workflows and documentation. From an error handling and edge cases perspective:
secrets: inheritwhich safely passes through secrets; undefined secrets resolve to empty strings, which the entrypoint handles gracefully (e.g., empty OLLAMA_CLOUD_API_KEY results in a harmless "not configured" notice).All error handling and edge cases appear properly addressed in the context of this PR's changes.
Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 8m 1s
🪰 Gadfly review —
qwen3-coder:480b-cloud(ollama-cloud)Verdict: Minor issues — 3 reviewers: security, correctness, error-handling
🔒 Security — Minor issues
VERDICT: Minor issues
.gitea/workflows/review-reusable.yml:51— TheGITEA_TOKENsecret is used directly from the caller's context. While this is necessary for the workflow to post comments, it's critical that consumers only call this reusable workflow from trusted contexts, as it will have write access to pull requests. The documentation should more strongly emphasize this trust requirement..gitea/workflows/review-reusable.yml:68-69— Hardcoded references toGADFLY_ENDPOINT_M1andGADFLY_ENDPOINT_M5mean that consumers with different endpoint names must use the full stub. This is documented but creates a slight inconsistency in the "reusable" abstraction.examples/reusable.yml:49— The example placeholder"your-username"forallowed_usersmight be overlooked by users copying the workflow, potentially leading to unauthorized comment triggers if not updated.🎯 Correctness — No material issues found
VERDICT: No material issues found
adversarial-review.ymlnow calls the reusable workflow with the correct parameters.timeout_minutesas a number type with a default of 30 minutes, overridden to 90 minutes by the caller.🧯 Error handling & edge cases — Minor issues
VERDICT: Minor issues
.gitea/workflows/review-reusable.yml:40— Thetimeout_minutesinput defaults to 30, but the calling workflow (.gitea/workflows/adversarial-review.yml) explicitly sets it to 90. This is correct behavior, but the default of 30 may be too low for complex reviews, potentially leading to premature job termination. Consider raising the default to 60 or 90 minutes to accommodate longer-running review tasks..gitea/workflows/review-reusable.yml:68-69— The workflow only maps two specificGADFLY_ENDPOINT_*secrets (M1andM5). As noted in the comments, consumers with other named endpoints must use the full stub. While this is documented, it's an edge case that could lead to misconfiguration if users aren't careful. No error handling is present for missing endpoint secrets, which could result in silent failures or degraded functionality.Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 2m 10s
🪰 Gadfly review —
claude-code/opus:max(claude-code)Verdict: Minor issues — 3 reviewers: security, correctness, error-handling
🔒 Security — Minor issues
I've verified the three files involved. Let me confirm the factual basis of the surviving finding.
The draft's "verified" items (authz boundary preserved) are confirmations, not findings — the gate only needs to adjudicate the one Minor finding about
examples/reusable.yml.Confirmed against the code:
examples/reusable.yml:43—uses: steve/gadfly/.gitea/workflows/review-reusable.yml@main(mutable ref) ✓ — note the draft cited line 42; theuses:@mainline is 43.examples/reusable.yml:44—secrets: inherit✓:sha-c342bdbatreview-reusable.yml:47(draft cited line 42; correct line is 47).The core security reasoning — a mutable workflow-definition ref combined with
secrets: inheritwidens the trust surface beyond the mapped keys — is factually grounded and valid. I'll keep the finding with corrected line references and tightened framing.VERDICT: Minor issues
Minor:
examples/reusable.yml:43(uses: steve/gadfly/.gitea/workflows/review-reusable.yml@main) — the documented subscribe example pins the reusable workflow to the mutable@mainref while usingsecrets: inherit(line 44). Withsecrets: inherit, the consumer's entire secret set is exposed to whatever YAML is on gadfly'smainat trigger time. The image is correctly pinned to an immutable:sha-c342bdb(review-reusable.yml:47), but the workflow definition itself is a second, mutable trust point: a future edit toreview-reusable.ymlonmain(or a branch compromise) could add an exfil step running with the consumer's inherited secrets. The header comment (line 10) already notes you can pin@<ref>, but the shipped example demonstrates the less-pinned@main.secrets: inheritwidens the blast radius beyond the explicitly mapped keys.uses:pin an immutable release tag (e.g.@v1) and add a one-line note thatsecrets: inheritgrants the pinned ref access to all caller secrets, so pinning to a tag (notmain) is the safer default.Nothing else in the security lane is materially wrong — the actor/authn gate (
adversarial-review.yml:36-42keeps the identicalsteve/fizi/dazedallow-list;allowed_users→GADFLY_ALLOWED_USERSbelt-and-suspenders preserved) and the secret plumbing are carried over faithfully from the previous inline stub.🎯 Correctness — Minor issues
I've verified the single finding against the actual files. The draft's one finding is confirmed:
adversarial-review.yml:46hardcodesspecialists: "security,correctness,error-handling", and the diff shows the old job usedGADFLY_SPECIALISTS: ${{ vars.GADFLY_SPECIALISTS || 'security,correctness,error-handling' }}. The repo-var override is genuinely dropped (it now no-ops for anyone who had set it), and since a reusable workflow can't read the caller'svars, the caller would need to forward the expression to preserve it. The supporting confirmations (env-var parity, empty⇒default plumbing, claude-code lane) are accurate.VERDICT: Minor issues
The refactor is a faithful move of logic from the self-contained stub into the reusable workflow. Env-var parity holds, every input that defaults to
""is consumed by code that treats empty as unset (so a passed-through empty string yields the built-in default), and theclaude-codelane plumbing is unchanged.Finding (minor)
.gitea/workflows/adversarial-review.yml:46— silently drops thevars.GADFLY_SPECIALISTSrepo-var override. The old job setGADFLY_SPECIALISTS: ${{ vars.GADFLY_SPECIALISTS || 'security,correctness,error-handling' }}, letting a maintainer override the lens suite via a repo variable without editing YAML. The new caller hardcodesspecialists: "security,correctness,error-handling". Impact: low — the hardcoded value equals the prior default, so behavior only changes for someone who had set the repo var (the override now no-ops). A reusable workflow can't read the caller'svars, but the caller can still forward it:specialists: ${{ vars.GADFLY_SPECIALISTS || 'security,correctness,error-handling' }}. Suggest restoring that expression if the override was intended to stay functional.🧯 Error handling & edge cases — No material issues found
I've verified the core of this diff. Here's my review through the error-handling & edge-cases lens.
VERDICT: No material issues found
This PR is workflow-plumbing + docs. The only edge-case-bearing claim is the design's load-bearing assumption — "an empty input value falls back to the image's own built-in default" — since the reusable workflow now unconditionally sets every
GADFLY_*env var, including to"". I traced that assumption through the actual consuming code rather than trusting the comments:GADFLY_MODELS/GADFLY_TRIGGER_PHRASE—entrypoint.sh:156,53use${VAR:-default}, which fires on empty and unset. Empty ⇒ default. ✓GADFLY_TIMEOUT_SECS/GADFLY_MAX_STEPS— both go throughenvInt(cmd/gadfly/main.go:450-460), which explicitly returns the default on"", on non-numeric, and onn <= 0. Empty ⇒ default. ✓ (Verified by reading the function — the<= 0guard also means a stray"0"can't disable the timeout.)GADFLY_PROVIDER_LENS_CONCURRENCY—providerOverride(main.go:291-302) splits on,, and an empty string yields no match →(0, false)→ caller falls back. ✓GADFLY_PROVIDER/GADFLY_SPECIALISTS/GADFLY_WORKER_MODEL— all read viastrings.TrimSpace(os.Getenv(...))then== ""checks (model.go:55-57,specialists.go:136,model.go:115). Empty ⇒ off/default. ✓GADFLY_BASE_URL— the one I was most worried about, since the reusable always sets it andmodel.go:107errors when it's "set but unsupported." Confirmed atmodel.go:59,65: it'sTrimSpace'd and an empty value takes thebaseURL == ""branch (majordomo registry resolution) before reaching the error path. Setting it to""is harmless. ✓So the empty-string fallback holds for every plumbed variable — no silent no-op or parse-panic on the unhappy "caller overrides nothing" path.
Two non-issues I checked and am deliberately not flagging:
PR:/ event-context expressions (${{ github.event.pull_request.number || ... }}) are moved verbatim from the oldadversarial-review.ymlinto the reusable — no new edge case introduced by this diff; whether the caller'sgithub.eventreaches the reusable is a will-it-run/correctness concern (and the PR author already flagged it), not an error-handling regression.timeout-minutes: ${{ inputs.timeout_minutes }}istype: number, default: 30, so it can't be empty/unparseable; the slim stub inheriting a 30-min wall-clock vs. the old 90 is a config-default tradeoff, not a swallowed error.Outside my lens: the reusable workflow has no
permissions:block and no actor gate of its own — both are pushed onto the caller (the example stub includes them). Whether that's safe is a security-lens question; flagging only because a consumer who copies the slim stub but deletes theif:gate would lose the comment-trigger guard.Automated adversarial review by Gadfly. Advisory only — does not block merge. · ⏱️ reviewed in 7m 11s