The pin bump used `git add -A` from a checkout that had a git worktree under
.claude/worktrees/, so the previous commit recorded it as a gitlink — a
submodule pointer to a commit no clone can resolve. Removed, and .gitignore now
covers the directory so it cannot recur.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
PR #30 (qwen/kimi built-ins) is merged and the image is published, so the
last-resort pin points at it. Verified the tag exists in the registry before
pointing consumers at it — a pin to a tag that was never pushed breaks every
consumer at once, and the build succeeding is not by itself proof the tag
landed.
GADFLY_REVIEWER_TAG still overrides this, and an explicit reviewer_tag input
overrides both.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Round 9's best finding is that my own round-8 change falsified a rationale I
wrote in round 5. The pre-flight skips the endpoint-override path because "a
built-in's own variable is never consulted there" — then I gave kimi/qwen an
own-key fallback that consults exactly that variable on exactly that path. So a
keyless override config sailed past the check and failed as a 401, which is the
failure the check exists to replace.
Now that the rule is statable for those two providers, they are checked on both
paths ("own key or GADFLY_API_KEY"), while everything else stays silent on the
override path because its rules still are not.
The missing-key hint on the GADFLY_ENDPOINT_* path named the endpoint variable
— telling a keyless operator to put a credential in a Gitea var, which is not
masked, and contradicting the README warning added one round earlier. It now
always names the provider's own masked secret.
Also: the model argument is trimmed, since Go trims GADFLY_MODEL and padding
would otherwise slip past the claude-code exemption; the test job takes
`permissions: contents: read`, being the one job that executes PR-authored
code; and the ollama-cloud rationale is stated once.
Deliberately not taken, with reasons rather than silence: the credential-scrub
bash could be extracted to a testable script like preflight.sh was — fair, and
a follow-up, since moving it now would be a fresh untested surface at merge
time. `tr -d [:space:]` strips POSIX whitespace where Go strips Unicode, which
differs only for a GADFLY_BASE_URL made entirely of non-ASCII spaces. And the
two provider tests overlap but assert different contracts that should be able
to fail independently.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Two findings this round contradicted each other — one asked me to extend the
engine-spec exemption to a bare "opencode", the other said opencode should not
be exempt at all. The code settles it: that engine drives an ollama-cloud model
through the bundled CLI and authenticates with OLLAMA_API_KEY, so it needs
exactly the key the pre-flight checks. Exempting it, which I did last round,
switched the check off for the one engine it could still help. Only claude-code
is exempt now — it carries CLAUDE_CODE_OAUTH_TOKEN and needs no Ollama key —
and opencode/open-code get table rows so both spellings are covered.
The README told operators to embed the Qwen key in a GADFLY_ENDPOINT_* var,
while the workflow that forwards those vars warns in its own comments that vars
are NOT masked. Rather than only rewording the docs, a keyless kimi/qwen
endpoint now falls back to its own QWEN_API_KEY / KIMI_API_KEY — the same
vendor's key, so the no-cross-vendor rule is untouched — which lets the URL live
in a var and the credential in a secret. Break-checked by pointing that fallback
at OPENAI_API_KEY: the leak test catches it.
Smaller: isBuiltinCompatProvider mirrors isOpenAICompatProvider instead of an
inline slices.Contains, with a test that every builtin is also in the compat
list (a builtin missing from it would never reach the branch that protects it);
the preflight.sh rationale is stated once rather than in two comment blocks;
the Go test locates the shell script relative to its own source file; and the
gofmt step takes GOPROXY=off like its neighbours.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Fourteen findings, and seven of them from all four models are the same one:
endpointProvider was missing the no-cross-vendor-fallback guard I had just
added to resolveModel. I fixed a credential leak on one path and left its
sibling leaking, in the commit whose own message argued those two paths must
move together. That is the third time in this PR.
So it is no longer a rule written twice. openAICompatOptions owns it and both
paths call it; builtinCompatProviders names the vendors that must never inherit
OPENAI_API_KEY, replacing a `provider == "kimi" || provider == "qwen"` literal
that was a fourth uncounted copy of the list.
The test drives a real request at a local server and demands two things: that
no request arrives carrying the OpenAI key, AND that the call fails closed
naming the variable to set — the second half because my first draft pointed the
provider at vendor.example, so the server saw nothing and the assertion held
for a reason unrelated to the fix. Break-checked: removing the guard puts
"Bearer sk-openai-must-not-travel" on the wire to the other vendor.
The scrub check failed open. As a bare condition, a grep ERROR (exit >= 2)
reads as "not found" and skips the guard — a credential check that passes
precisely when it cannot see the filesystem it is searching. It now
distinguishes 0/1/>=2 and refuses to continue on error.
A bare "claude-code" spec has no "/", so the provider fell back to ollama-cloud
and the pre-flight would skip a reviewer that authenticates with
CLAUDE_CODE_OAUTH_TOKEN and needs no Ollama key. Engine specs are now exempt.
preflight.sh's provider list duplicated its own case arms; both now read one
table. And its comment claimed the Go cross-check fails if either list misses
an entry from the other, when only one direction is checked — the reverse is
not even desirable, since ollama-cloud and anthropic belong in that table and
not in the Go one. The comment now says what is enforced.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
The previous commit message listed this fix; the edit never made it into the
tree. Checked because I have done exactly that before, and the message is not
the change.
Verified rather than assumed, and the behaviour is worth recording: with a
password beginning with "-", `grep -rqF "$PW"` returns 0 against a directory
containing no such string, so the scrub check would have reported a leak that
did not exist and failed the step with a misleading message. With -e it
correctly returns 1 when clean and 0 when the credential really is present.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Round 6, and one finding exposed something no reviewer mentioned: the majordomo
bump this whole PR depends on was never made. Every test here builds the openai
client directly, so all of them passed against a majordomo release that had
never heard of qwen — a plain "qwen/<model>" in GADFLY_MODELS, the primary way
anyone will use this, would not have resolved at all. A compile error caught it,
which is luck. TestBuiltinCompatProvidersResolveViaRegistry now exercises that
path; the build is what guards the dep itself, since the old release cannot
compile the code below.
On the endpoint-override path, kimi and qwen fell through to openai.New's
OPENAI_API_KEY default whenever GADFLY_API_KEY was unset — sending an OpenAI
key to Moonshot or Alibaba. That is a credential handed to the wrong vendor,
and it is the exact failure majordomo's built-ins are written to prevent; I
reintroduced it one layer up. Both now pass the key unconditionally, so an
absent key is a 401 naming GADFLY_API_KEY rather than a foreign credential on
the wire.
The test job scrubbed the registry credential and left the checkout token in
.git/config, readable by the `go test` it then runs — fixing one credential
while its neighbour sat in the open. persist-credentials: false; nothing in
that job talks to git after checkout.
The cross-language wiring test now QUERIES preflight.sh via a new
gadfly_preflight_providers function instead of regexing its case statement.
Parsing made that file's formatting a contract no linter enforces, where a
harmless reformat breaks a test in another language. Two models flagged it.
Also: grep for the scrub check takes -e, so a password starting with a hyphen
is not read as options; and key_hint stopped repeating key_env in four of five
arms.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Round 5, and the best findings are again about the fix from round 4.
The scrub only ran on success. `set -e` aborts the step when `go mod download`
fails, so the cleanup line after it never executed — leaving a push-capable
credential on a long-lived self-hosted runner for whatever job landed there
next. It is now a `trap ... EXIT`, verified against a simulated failure.
It also scrubbed the wrong file in principle: `git config --global` writes to
GIT_CONFIG_GLOBAL, else $XDG_CONFIG_HOME/git/config when that exists, else
~/.gitconfig — so deleting ~/.gitconfig can scrub a path the credential was
never in. The step now names GIT_CONFIG_GLOBAL itself, leaving exactly one file
to remove.
And the verification failed open in the case that matters most: `grep -F ""`
matches every file, so a run WITHOUT the secret — a fork PR, the threat model —
failed the check with a message accusing it of leaking a credential it never
had. Guarded on a non-empty secret.
Credentials move to an Authorization header instead of being embedded in the
URL, so a password containing @ : / or # can no longer break URL parsing in a
way that reads as a bad password.
Two list-drift holes closed with one test that reads across languages:
TestOpenAICompatProvidersAreFullyWired asserts every openAICompatProviders
entry is both advertised in endpointProviderNames and has a credential arm in
scripts/preflight.sh. Adding a compat provider touches three places in two
languages and nothing connected them. Break-checked in both directions.
Finally, a whitespace-only GADFLY_BASE_URL disagreed across the boundary: Go
TrimSpaces it and takes the registry path, bash called it "set" and skipped the
pre-flight, so the missing key arrived as a bare 401 with no notice. Both now
agree on what unset means.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Both Claude reviewers caught this independently, and they are right. The test
job I added wrote a PUSH-CAPABLE REGISTRY_PASSWORD into a plaintext
~/.gitconfig and then ran `go build`/`go vet`/`go test` — repository code — on
pull_request events. This repo is public, so a fork PR could ship a test whose
only job is to print that file. The image build had already answered this
question correctly: its credentials are BuildKit secrets scoped to the
module-download RUN and are never present while code executes. I bolted on a
job that skipped the boundary its neighbour maintains.
Dependencies are now fetched in their own step which deletes ~/.gitconfig
before anything else runs, and asserts the scrub — against the whole home
directory, not against the file it just removed, because the credential can
also land in ~/.netrc or ~/.config/go/env. Verified the assertion is not
vacuous: planting the secret in ~/.netrc trips it. Later steps run with
GOPROXY=off, so any attempt to reach the network fails loudly rather than
quietly hunting for the credential that is now gone.
Also from round 4: TestEndpointProviderNamesAreAllAccepted pinned only
endpointProvider, while the constant is the error text for BOTH resolution
paths — it now asserts each advertised name resolves either way (break-checked
by dropping the gemini alias from resolveModel alone). preflight.sh documents
that ollama-cloud is checked on OLLAMA_API_KEY but hinted as
OLLAMA_CLOUD_API_KEY because run.sh copies one to the other first, an ordering
dependency that was invisible from the file.
And the comments that narrated this PR's own edit history ("the first version
of this change...") are rewritten as invariants. That history stops being true
the moment this merges, and the repo's doc policy says as much.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
A workspace-scoped Qwen endpoint rejects a key issued for the shared
international host with a genuine 'Incorrect API key provided', so a valid key
reads as invalid and the obvious next move — checking the key — confirms it is
fine and leads nowhere. Watched this cost real debugging time on a live
deployment today; gadfly would hit it identically. Documents the
GADFLY_ENDPOINT_* form, which reaches a workspace host with no code change.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Twelve findings, all real, and the two that matter are about the pre-flight I
added rather than about qwen.
The credential check had a false pass in the OTHER direction from round 2's: on
the GADFLY_BASE_URL override path, resolveModel builds the client with
GADFLY_API_KEY and never reads QWEN_API_KEY/KIMI_API_KEY, so treating the
provider's own key as sufficient there let a doomed run proceed. Having now
been wrong about these rules in both directions, the check no longer tries to
model both paths: it covers the REGISTRY path, whose rules it can state
exactly, and says nothing about the override path — which is hand-configured by
definition, while the registry path is the one you hit by adding a model id to
a var and forgetting the secret.
The logic moves to scripts/preflight.sh, sourced by both run.sh and the test.
The previous answer to "this test duplicates production logic" was a regex
drift-guard, and that guard compared only the provider table — not the decision
logic, which is precisely the half that carried the bug. A duplicate you guard
is still a duplicate; this deletes it, and the test now runs under `set -u`
like production does.
Also: the test that pins the shared provider slice held its own copy of the
list (now ranges the slice); endpointProviderNames had nothing tying it to the
switches it describes, which is how it shipped without "gemini" (a new test
asserts every advertised name resolves); two godoc lists had drifted; and the
"sanity" line that asserted nothing is gone.
And the repo had NO test job — `go test` and the pre-flight table both existed
and neither was ever executed by CI, which reads as coverage while providing
none. Added one (build/vet/gofmt/test/pre-flight), running alongside the image
build rather than gating it, so red is loud without standing between a push and
a rebuild.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Eight findings, all real, and the sharpest ones are about this PR's own fixes.
GADFLY_API_KEY was treated as a universal substitute in the pre-flight. It is
not: resolveModel reads it only AFTER the `baseURL == ""` early return, so on
the registry path — the documented primary path — a qwen/kimi built-in reads
its own variable and GADFLY_API_KEY is never consulted. A mis-set
GADFLY_API_KEY therefore passed pre-flight and 401'd five times anyway, which
is precisely the failure this check exists to prevent. It now only substitutes
when GADFLY_BASE_URL is also set.
`openai-compatible` was missing from the pre-flight table while both switches
accept it as an OPENAI_API_KEY alias, so that one spelling still fell through
to the cryptic five-failure mode.
endpointProviderNames — the constant I introduced *to stop* the two error
messages drifting — omitted the `gemini` alias both switches accept. It now
lists every accepted spelling.
And the case list itself was still duplicated across both switches plus the
test that pins them: three copies of the thing whose duplication started this.
Both switches now call isOpenAICompatProvider over one shared slice, and
endpointProvider's doc comment points at endpointProviderNames instead of
carrying a fourth hand-written copy.
scripts/preflight_test.sh moves into the repo (20 cases, up from 17, covering
openai-compatible and both GADFLY_API_KEY directions). It carries a drift guard
that diffs its copy of the provider table against run.sh's and aborts if they
differ — break-checked by deleting an arm from run.sh, which fails it loudly.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
The pre-flight comment was the worst of them, and three models agreed. It said
providers absent from the table "need no key or carry it in their endpoint/DSN"
— false for google, which needs a key and is absent for an entirely different
reason: it accepts GOOGLE_API_KEY *or* GEMINI_API_KEY, so a single-variable arm
would silently skip a correctly-configured reviewer. That reasoning was in the
PR description and not in the code, so the comment invited exactly the wrong
edit. It now states both exclusion reasons and names google's.
Forwarded KIMI_API_KEY alongside QWEN_API_KEY in the dogfooding stub. This PR
argues that sibling call sites must move together, and I declared both secrets
in the reusable workflow and forwarded one — a config that looks complete and
401s on the model you didn't wire.
The two endpoint-provider error messages listed the same accepted set in
different order and spelling. Both functions accept an identical set, so they
now share one endpointProviderNames constant and cannot disagree.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
majordomo now ships qwen and kimi as built-ins that ARE the openai client at
their own base URL, so "qwen/qwen3.8-max" works as a GADFLY_MODELS entry once
the key reaches the container. This wires up the parts that key has to pass
through.
Two provider switches had to learn the names, not one. resolveModel's
GADFLY_BASE_URL override was the obvious one; endpointProvider's
GADFLY_ENDPOINT_* parser is its sibling, and I fixed the first and missed the
second on the first pass — a config that resolves one way and errors the other
for no reason a user could guess. TestOpenAICompatProvidersResolveOnBothPaths
now asserts both from one table so the pair fails together; break-checked in
both directions.
QWEN_API_KEY (and KIMI_API_KEY) are declared as workflow_call secrets and
forwarded to the container, with gadfly's own stub forwarding QWEN_API_KEY so a
qwen entry can join the default swarm by editing GADFLY_DEFAULT_MODELS alone —
no workflow edit, no re-release.
The run.sh credential pre-flight is now a provider→variable table instead of an
ollama-cloud special case. Without it a forgotten key surfaces as five
identical per-lens agent failures naming no variable, and the operator reads a
stack trace to find out which secret they missed. Google stays out of the table
on purpose: it accepts either GOOGLE_API_KEY or GEMINI_API_KEY, and a one-var
entry would wrongly skip a correctly-configured run. Verified across 17
provider x key-state combinations, including that a wrong-provider key never
satisfies qwen (majordomo refuses cross-provider fallback) and that unkeyed
providers are never blocked.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Nearly every consumer re-pin of review-reusable.yml was a reviewer IMAGE bump,
not a structural change — but the tag was baked into a `uses: docker://` step,
which is parsed before any expression context exists and so can't read a
variable. Run the reviewer as the JOB container instead (container.image DOES
accept expressions) with an explicit `run: /entrypoint.sh` step: the tag now
resolves per-run via inputs.reviewer_tag → user var GADFLY_REVIEWER_TAG → the
baked fallback pin (sha-b37cd09, unchanged).
A Gadfly release becomes: build the image, update the ONE user-level variable —
every consumer pinned to this file's @<sha> picks it up on its next review with
no re-pin. Workflow re-pins remain only for structural yml changes (this is one
— intended to be the last routine one).
- Entrypoint contract unchanged: same env block, same /entrypoint.sh brains
(bash + entrypoint already live in the image; ENTRYPOINT was /entrypoint.sh).
- Keep the variable on immutable sha- tags, never :latest (act_runner caches
:latest and often does not re-pull a moved one).
- Tradeoff noted in comments: vars are unmasked and move without a commit
trail — same blast radius as editing this file, minus the audit trail.
- README (image/pinning para + Central config table) and examples/reusable.yml
updated per the maintenance rule.
Co-Authored-By: Claude Fable 5 <[email protected]>
Adopt the provider-wide lens budget build (PR #27) for gadfly's own reviews
and for consumers pinning this reusable. No image rebuild needed — the tag
already exists — so skip CI.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Gadfly's own review of #27 surfaced a real robustness cluster (5 models,
error-handling) plus stale comments I missed.
Robustness — the flock permit pool could hang forever:
- tryAcquire swallowed os.OpenFile errors and treated every flock error as
"busy", so a broken/missing pool dir (or a filesystem without flock) would
spin-poll indefinitely; the fanout context is uncancellable and the per-lens
timeout only starts AFTER acquire returns. Can't trigger in the deploy
(entrypoint mkdir -p's the dir) but fixed defensively.
- tryAcquire now returns a structural error, distinguished from a healthy-full
pool (EWOULDBLOCK = busy → keep polling). acquire FAILS OPEN on a structural
error: logs once and runs the lens unthrottled rather than hanging the review.
- acquire uses time.NewTimer + Stop() (no per-poll timer leak on cancellation).
- activeLensSem warns on stderr when GADFLY_LENS_SEM_DIR is set but the size is
invalid (was a silent degrade to unthrottled).
- New test: a broken pool dir fails open promptly.
Doc drift (stale references to the removed model cap):
- main.go defaultLensConcurrency + runSpecialists doc, entrypoint.sh status
pre-seed + lane-launch comments, and the pre-existing lens_concurrency_test.go
header all updated to the provider-wide-budget wording.
Accepted (graded real, not changed): all-models-start-at-once startup burst
(intended tradeoff) and index-0 sweep bias (cosmetic). One false positive
(one model using the whole budget is the intended lone-model behavior).
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Concurrency was two multiplicative gates in two processes: entrypoint.sh
capped MODELS-at-once per provider (GADFLY_PROVIDER_CONCURRENCY) while each
model's binary separately capped its own lenses (GADFLY_LENS_CONCURRENCY).
A model therefore held its whole model-slot until its LAST lens finished,
stalling the next model even with idle lens capacity.
Collapse to one throttle: a provider-wide lens budget shared across all of
that provider's models. entrypoint now runs every model in a lane at once and
seeds a single cross-process permit pool per lane (a dir of N flock files,
sized by GADFLY_PROVIDER_LENS_CONCURRENCY -> GADFLY_LENS_CONCURRENCY). Each
lens pass (review+recheck) acquires a permit before it runs and releases it
after, so a model winding down immediately yields its freed permits to
another model's queued lenses. flock auto-releases on process death, so a
killed/crashed model can't leak budget.
- cmd/gadfly/lenssem.go: the flock permit pool (+ lenssem_test.go).
- main.go: runSpecialists holds a shared permit per lens; fanout sized to the
budget so a lone model can use all of it. Falls back to the in-process limit
when no pool is set (local runs, tests).
- entrypoint.sh: drop provider_cap/DEFAULT_CONC; run_lane runs all models and
seeds the per-lane pool.
- GADFLY_PROVIDER_CONCURRENCY / GADFLY_CONCURRENCY are now ignored; the
reusable workflow marks provider_concurrency deprecated and stops forwarding
it. Docs (README, CLAUDE.md, examples) updated per the maintenance rule.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
sha-bb98fae is the image built from the PR #26 merge, so it bundles the opencode
CLI and can run the new opencode/<model> engine. Consumers pinned to the gadfly
ref of this commit inherit it. Previous pin (sha-f468fe6) predated opencode.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Gadfly's own swarm reviewed PR #26 and reached consensus (3/3 models) on a real
bug, plus flagged security/maintainability items. Fixes:
- Pass-through auth (BLOCKING, 3/3 agreement): openCodeEnv() stripped every
provider key except OLLAMA_API_KEY, so the documented opencode/<provider>/<model>
escape hatch (e.g. opencode/anthropic/...) had no way to authenticate — the
reusable workflow forwards ANTHROPIC_API_KEY/OPENAI_API_KEY into the container
and the allowlist discarded them. Now forward ANTHROPIC_*/OPENAI_*/GOOGLE_*/
GEMINI_* so OpenCode's built-in providers can authenticate, while still
withholding gadfly's own secrets (Gitea/findings tokens, claude-code OAuth).
- Read-only hardening (security lens): the generated config denied only edit/bash.
Using OpenCode's documented permission schema, also deny webfetch/websearch/
external_directory — the network + out-of-sandbox tools — closing the
exfiltration surface a prompt-injected review could otherwise reach. Permission
is now a map so the deny set is extensible.
- Dedup (maintainability lens, 3/3): extract shared filterEnv() and
killGroupOnCancel() helpers in engine.go, used by both shell-out engines'
runPass/env builders instead of the copy-pasted blocks.
- Cosmetic: split the const block so defaultOpenCodeBaseURL's doc comment no
longer visually misattaches to the agent-name const.
README updated: the read-only note and the reduced-env note now reflect the
broader deny set and the forwarded provider keys.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
The self-review kept skipping every PR with "event 'workflow_call' not handled":
review-reusable.yml pinned the reviewer image at sha-3095ebf, which predates the
entrypoint.sh reclassification (added in 9d74cb9) that maps a called workflow's
github.event_name = 'workflow_call' back to pull_request/issue_comment. Under
Gitea >= 1.27 the container therefore saw an unhandled event and self-skipped in
~1s, even though PR was populated.
Bump the pin to sha-f468fe6 (current main HEAD, which contains the fix) so the
dogfood path actually reviews. The pr_number threading in the prior commit fixes
the manual-dispatch input propagation; this fixes the pull_request path.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
gadfly's self-review adversarial-review.yml calls review-reusable.yml but was
never updated for the Gitea >= 1.27 breaking change (go-gitea#37478): a called
workflow no longer receives the caller's workflow_dispatch inputs in
github.event, so a manual "review PR #N" dispatch reached the reusable with an
empty PR. Commit 64d34bd added the pr_number workflow_call input and mort's
caller threads it; this brings gadfly's own caller in line so the dogfood path
works on the same Gitea version.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Add a third review harness alongside the in-process majordomo loop and the
claude-code CLI shell-out: the OpenCode CLI (opencode.ai) driving an ollama-cloud
model, selected by an "opencode/<model>" spec. The goal is to benchmark gadfly's
boutique executus harness against a freely-available agentic harness on the SAME
model (e.g. "ollama-cloud/glm-5.2" vs "opencode/glm-5.2").
OpenCode has no --append-system-prompt flag, so the lens system prompt and the
read-only discipline are delivered through a generated config injected via
OPENCODE_CONFIG_CONTENT: a "gadfly" agent whose prompt is the system prompt with
edit/bash denied at both the global and agent level, plus a "gadfly" ollama-cloud
provider. That env var is the highest-precedence config source in the container,
so a reviewed repo's own opencode.json can't re-enable edits on the reviewer.
Spec forms: "opencode/<model>" (wrapped in the generated provider), the
"open-code/" alias, "opencode/<provider>/<model>" pass-through to OpenCode's own
registry, and bare "opencode". Model ids are taken verbatim so colon-bearing
ollama ids (qwen3-coder:480b-cloud) survive. Auth reuses OLLAMA_CLOUD_API_KEY
(mapped to OLLAMA_API_KEY, referenced as {env:OLLAMA_API_KEY} in config, never a
literal secret). Knobs mirror GADFLY_CLAUDE_*: GADFLY_OPENCODE_BIN/MODEL/BASE_URL/
EXTRA_ARGS. openCodeEnv() forwards OLLAMA_API_KEY (the inverse of claudeEnv) but
still withholds the Gitea/findings/Anthropic secrets.
main.go engine selection is now a switch (claude-code / opencode / majordomo), and
the auto-select path uses a type-check instead of a boolean so a shell-out engine
can never hit the *majordomoEngine assertion. auto-select and delegate_investigation
stay majordomo-only and are skipped for opencode (the CLI does its own legwork).
Dockerfile bundles opencode-ai (npm auto-selects its musl build on alpine) with a
best-effort version check + provider pre-warm that never fails the shared image
build. README/examples/CLAUDE.md/scripts updated per the maintenance rules.
Tests: new opencode_test.go mirrors engine_test.go (spec/model/args/config/env-
filter + stub-CLI runtime tests). Verified end-to-end with a fake opencode CLI:
correct argv, injected config, and consolidated markdown output.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Gitea >= 1.27 does not propagate the caller's workflow_dispatch inputs
into a called workflow's github.event (same rework that changed
event_name), so a manual 'review PR #N' dispatch arrived with an empty
PR and died at the entrypoint's 'PR required' check. Accept pr_number as
an explicit workflow_call input and fold it into the PR env fallback
chain; caller stubs pass github.event.inputs.pr_number through.
Co-Authored-By: Claude Fable 5 <[email protected]>
GADFLY_ENDPOINT_RAGNAROS is empty in Gitea vars and no ragnaros/<model>
is in the pool; netherstorm is the live local GPU endpoint. Requested by
Steve.
Co-Authored-By: Claude Fable 5 <[email protected]>
A reusable workflow can't enumerate arbitrary vars.GADFLY_ENDPOINT_*;
the netherstorm endpoint var was added after the forwarding list, so
every netherstorm/<model> reviewer failed with 'unknown provider:
netherstorm' — the correctly-formatted var never reached the container.
Co-Authored-By: Claude Fable 5 <[email protected]>
Gitea 1.27 (breaking change go-gitea#37478) runs a called (reusable)
workflow with github.event_name = 'workflow_call' instead of propagating
the caller's event. Every consumer stub forwards EVENT_NAME from
github.event_name, so since the server upgrade every review arrived as an
unhandled event and self-skipped in one second while reporting success —
mort PRs #1445-#1447 all went unreviewed.
Reclassify workflow_call from the forwarded payload: a non-empty
COMMENT_BODY can only come from issue_comment (trigger-phrase + actor
gates still apply); otherwise a PR number means a pull_request-shaped
trigger. Neither → the existing unhandled-event skip. Pre-1.27 servers
are unaffected.
Co-Authored-By: Claude Fable 5 <[email protected]>
Makes gadfly a consumer of executus (run.Executor compaction/bounding/budget/critic + fanout) and fixes the large-PR token burn in size-gated layers: paginated get_diff, downshift above GADFLY_HUGE_DIFF_BYTES, and a swarm-wide GADFLY_PR_BUDGET_SECS backstop. Small PRs untouched; advisory-only and the static binary preserved. Dogfood swarm reviewed it (6 models, 21 real findings graded + folded in).
Co-authored-by: Steve Dudenhoeffer <[email protected]>
Co-committed-by: Steve Dudenhoeffer <[email protected]>
Phase 3: gadfly's own multi-model reviews now also post a COMMENT-state PR
review with inline comments anchored to changed lines. External consumers
re-pin separately.
[skip ci]
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Phase 2: gadfly's own multi-model reviews now post ONE cross-model consensus
comment instead of N per-model comments. External consumers re-pin separately.
[skip ci]
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Makes the Phase 1 gadfly-findings contract live for gadfly's own dogfood
reviews (the local-ref reusable). External consumers re-pin separately.
[skip ci]
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Centralizes the consumer stub into a reusable Gitea workflow
(.gitea/workflows/review-reusable.yml, workflow_call + defaulted inputs +
secrets: inherit); gadfly's own dogfood is now a thin caller of it, which
proved end-to-end that github.event context propagates into the reusable
on this act_runner. Adds the slim examples/reusable.yml stub + docs.
Folded in the swarm's findings: timeout_minutes default 30->45, map
GADFLY_API_KEY, explicit permissions block, drop the dead specialist_suite
input, and harden the example's actor gate. ~70 findings graded.
Completes the gadfly-games build (Phases 1-4 + quality fixes).
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Co-authored-by: Steve Dudenhoeffer <[email protected]>
Co-committed-by: Steve Dudenhoeffer <[email protected]>
Bump majordomo to the latest build and accept every llama-swap spelling
(llama-swap/llama-swaps + un-hyphenated llamaswap/llamaswaps) in gadfly's
endpoint switches; the LLM_* llama-swap(s):// DSN path already worked via
majordomo.Parse. README + error messages + endpointProvider alias tests.
Swarm review: 8/9 clean; qwen3-coder's "Blocking" was a false positive
(claimed llamaswap was untested — it has dedicated test cases). Folded in
its one fair nit (README now lists the un-hyphenated aliases).
gofmt clean, go vet quiet, go test -race green.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Co-authored-by: Steve Dudenhoeffer <[email protected]>
Co-committed-by: Steve Dudenhoeffer <[email protected]>
Adds claude-code/opus:max to the dogfood swarm and pins to :sha-c342bdb
(which has the :thinking parse). Claude Code lineup is now sonnet + opus +
opus:max. All three ran end-to-end on this PR's own review; 0 findings
(clean PR + the telemetry fix suppressing phantom clean-verification
findings — working as intended).
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Co-authored-by: Steve Dudenhoeffer <[email protected]>
Co-committed-by: Steve Dudenhoeffer <[email protected]>
Adds claude-code/opus to gadfly's dogfood swarm (both sonnet and opus run
end-to-end), bumps the image pin to :sha-80d8f53 so the clean-lens
telemetry fix is live, and adds engine support for a
"claude-code/<model>:max" extended-thinking spec (MAX_THINKING_TOKENS,
best-effort). Validated: only 13 findings on this clean PR vs 43 on the
comparable #4 — the telemetry fix works.
Folded in the swarm's two real findings: a runPass env-injection test and
keeping MAX_THINKING_TOKENS in claudeEnv. Follow-up enables
claude-code/opus:max once this image builds.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Co-authored-by: Steve Dudenhoeffer <[email protected]>
Co-committed-by: Steve Dudenhoeffer <[email protected]>
emit() now skips findings extraction for a "No material issues found"
lens (its path:line refs are verification notes, not problems), fixing
the FP inflation that penalized thorough clean-pass reviewers. Also trims
the dogfood swarm to the strong reviewers: drops m5/qwen3.6 (last local
lane), gemma4, gpt-oss:120b, and kimi-k2.7-code — leaving 6 cloud +
claude-code/sonnet.
Fittingly, PR #4's own 11-model review produced 43 findings that were ALL
clean-verification bullets (zero real) — a live demonstration of the bug
this fixes. gofmt clean, go vet quiet, go test -race green.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Co-authored-by: Steve Dudenhoeffer <[email protected]>
Co-committed-by: Steve Dudenhoeffer <[email protected]>
Phase 2: bump majordomo to latest and wire its new llamaswap provider
into gadfly's endpoint switches; add claude-code/sonnet to gadfly's own
dogfood swarm (pin :sha-86f12c1, map CLAUDE_CODE_OAUTH_TOKEN) so the
Phase-1 engine runs as a live competitor; document the Ollama-through-CC
ANTHROPIC_BASE_URL proxy path as example-only.
The 11-model swarm (incl. claude-code/sonnet) reviewed it; 52 findings
graded via the MCP. Folded in the two real ones: a llamaswap
endpointProvider test (caught by claude-code/sonnet, citing CLAUDE.md)
and adding "openai-compatible" to the provider error messages (gpt-oss).
gofmt clean, go vet quiet, go build + go test -race green.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Co-authored-by: Steve Dudenhoeffer <[email protected]>
Co-committed-by: Steve Dudenhoeffer <[email protected]>
Phase 1: a second review engine alongside the majordomo agent loop. For
each lens, shell out to the Claude Code CLI (`claude -p --output-format
json`) inside the checked-out repo so it verifies findings with its own
read tools, then reuse gadfly's verdict-parse + recheck + consolidate +
emit pipeline. Select via GADFLY_MODELS `claude-code`/`claude-code/<model>`;
auth via CLAUDE_CODE_OAUTH_TOKEN (no --bare) else ANTHROPIC_API_KEY;
read-only by default; GADFLY_CLAUDE_* knobs. Dockerfile bundles Node +
@anthropic-ai/claude-code. Also bumped the dogfood pin to the status-board
image (PR #2 was the first dogfood with the live board + full fleet).
Folded in the swarm's own review findings: minimal subprocess env (no
GITEA_TOKEN leak to the CLI), runPass robustness (ctx/empty-result/runErr),
process-group cleanup on timeout, rune-safe error truncation, and
engine-neutral prompts (also de-mort-ified the recheck prompt). 66 findings
graded via the gadfly MCP.
gofmt clean, go vet quiet, go build + go test -race green.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Co-authored-by: Steve Dudenhoeffer <[email protected]>
Co-committed-by: Steve Dudenhoeffer <[email protected]>
Phase 3: one consolidated, live-updating PR comment aggregating every
model's per-lens progress (queued -> running -> finished + verdict), so
the swarm's progress is visible at a glance and a watcher can tell when
it's done. Opt-in statusWriter in the binary (atomic writes) + a
background status-board.sh renderer wired through entrypoint.sh; default
on, GADFLY_STATUS_BOARD=0 to disable.
Also restores gadfly's dogfood swarm to the full cloud fleet (9 cloud +
M5; M1 dropped as too slow) matching mort, and folds in the 3 real bugs
the swarm found on its own PR (skip-binary stuck-waiting, panic-stuck
lens, busy-loop on bad poll interval). All 36 findings graded via the
gadfly MCP (18 real / 18 false-positive).
gofmt clean, go vet quiet, go build + go test -race green.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Co-authored-by: Steve Dudenhoeffer <[email protected]>
Co-committed-by: Steve Dudenhoeffer <[email protected]>
Adds the adversarial-review workflow to gadfly itself (copied from mort: 3 cloud + m1/m5 via foreman, findings telemetry, sha-d7f364d). Future gadfly PRs get reviewed by the swarm.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
After each review the binary POSTs the run + its heuristically-extracted findings to GADFLY_FINDINGS_URL (off unless set). Advisory: any error only goes to stderr — never touches stdout, the exit code, or the review. stdlib net/http only (no new deps). entrypoint.sh derives GADFLY_REPO/GADFLY_PR and passes through GADFLY_FINDINGS_URL/GADFLY_FINDINGS_TOKEN. Also renames store references from the old 'docket' name to 'gadfly-reports'.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Specialist lenses ran strictly sequentially within a model. Add a
GADFLY_LENS_CONCURRENCY knob (default 1 = unchanged) that overlaps the
independent per-lens review+recheck passes, so a model posts its
consolidated comment as soon as its lenses finish.
Per-provider configurable, mirroring GADFLY_PROVIDER_CONCURRENCY:
GADFLY_PROVIDER_LENS_CONCURRENCY takes a "provider=N,..." map keyed by
the same provider lanes (modelProvider() mirrors entrypoint's provider_of;
providerOverride() mirrors provider_cap). The override wins for the model's
lane, else the scalar default.
runSpecialists fans out via a bounded worker pool, order-preserving
(results written by index) and keeping each lens's own timeout/recheck.
repoFS is immutable + fresh-toolbox-per-pass, so lenses share no mutable
state (verified under -race). Docs/examples updated; dropped a duplicate
GADFLY_TIMEOUT_SECS README row.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Accept "foreman" in both resolveModel (GADFLY_BASE_URL) and endpointProvider
(GADFLY_ENDPOINT_*) switches, mapping to majordomo's ollama.Foreman() preset
(handles foreman's non-streaming/long-poll quirks). Unlike the HTTPS-only
LLM_* foreman:// DSN, the base URL is verbatim, so a plaintext http:// foreman
queue works. Tests + README provider table + endpoint-aliases example updated.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
steveandClaude Opus 5