Compare commits
19
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
d272695c16 | ||
|
|
4dafac0d13 | ||
|
|
c9dab69d14 | ||
|
|
b37cd09dc9 | ||
|
|
6a74b64c7a | ||
|
|
74831368ab | ||
|
|
0d51879450 | ||
|
|
bb98fae5f0 | ||
|
|
2477e50230 | ||
|
|
463aa01ddb | ||
|
|
3973e469a8 | ||
|
|
5ab4074e9c | ||
|
|
f468fe6245 | ||
|
|
64d34bd33b | ||
|
|
d8580bc193 | ||
|
|
24d1ee1ebd | ||
|
|
56392368dd | ||
|
|
4ff63d988a | ||
|
|
256344d3e1 |
@@ -53,3 +53,6 @@ jobs:
|
||||
# 5-lens suite) from review-reusable.yml. Only the consumer-specific
|
||||
# allow-list is set here.
|
||||
allowed_users: "steve,fizi,dazed"
|
||||
# Gitea >= 1.27 does not propagate dispatch inputs into a called workflow's
|
||||
# github.event — thread the PR number explicitly (empty on non-dispatch events).
|
||||
pr_number: ${{ github.event.inputs.pr_number }}
|
||||
|
||||
@@ -24,11 +24,14 @@
|
||||
# if you accept that exposure; the explicit form is recommended. GITEA_TOKEN is
|
||||
# the automatic job token (no need to forward it).
|
||||
#
|
||||
# Advisory only — never blocks a merge. The image is pinned to an immutable
|
||||
# :sha- tag here (act_runner caches :latest); bump it per Gadfly release.
|
||||
# Consumers should pin `uses: ...@v1` — a curated release tag moved on deliberate
|
||||
# releases, so central tuning here propagates without per-consumer edits — or a
|
||||
# full `@<sha>` for an immutable pin. Avoid `@main` (moves on every push).
|
||||
# Advisory only — never blocks a merge. The reviewer image tag ALSO resolves at
|
||||
# runtime (inputs.reviewer_tag → user var GADFLY_REVIEWER_TAG → the fallback pin
|
||||
# baked into the `container:` line below), so a Gadfly release is: build the
|
||||
# image, update ONE variable — no consumer re-pin. Re-pin the workflow ref only
|
||||
# for structural changes to this file.
|
||||
# Consumers should pin `uses: ...@<sha>` — long-lived act_runners cache this file
|
||||
# by ref, so a moved tag (@v1) or @main is often NOT re-fetched and silently runs
|
||||
# a stale copy.
|
||||
|
||||
name: Gadfly review (reusable)
|
||||
|
||||
@@ -43,9 +46,10 @@ on:
|
||||
# NOT re-fetched; only a runtime value or a fresh @<sha> bypasses the cache).
|
||||
#
|
||||
# Owner-set user-scope variables (see README "Central config via variables"):
|
||||
# GADFLY_REVIEWER_TAG (the reviewer image tag this reusable runs),
|
||||
# GADFLY_DEFAULT_MODELS, GADFLY_DEFAULT_SPECIALISTS,
|
||||
# GADFLY_DEFAULT_PROVIDER_CONCURRENCY, GADFLY_DEFAULT_PROVIDER_LENS_CONCURRENCY,
|
||||
# GADFLY_ENDPOINT_RAGNAROS (the 4090 Ti endpoint).
|
||||
# GADFLY_DEFAULT_PROVIDER_LENS_CONCURRENCY (the provider-wide lens budget),
|
||||
# GADFLY_ENDPOINT_NETHERSTORM (the local GPU box endpoint).
|
||||
# An unset variable + no input → the image default (one model, default suite),
|
||||
# so a public consumer with neither still gets a sane minimal review.
|
||||
inputs:
|
||||
@@ -53,12 +57,18 @@ on:
|
||||
specialists: { type: string, default: "" } # GADFLY_SPECIALISTS — empty falls back to user var GADFLY_DEFAULT_SPECIALISTS
|
||||
provider: { type: string, default: "" } # GADFLY_PROVIDER
|
||||
base_url: { type: string, default: "" } # GADFLY_BASE_URL
|
||||
provider_concurrency: { type: string, default: "" } # GADFLY_PROVIDER_CONCURRENCY — empty falls back to user var GADFLY_DEFAULT_PROVIDER_CONCURRENCY
|
||||
provider_lens_concurrency: { type: string, default: "" } # GADFLY_PROVIDER_LENS_CONCURRENCY — empty falls back to user var GADFLY_DEFAULT_PROVIDER_LENS_CONCURRENCY
|
||||
provider_concurrency: { type: string, default: "" } # DEPRECATED / ignored — the per-provider MODEL cap was removed; the lens budget below is the single throttle. Kept so existing callers don't error.
|
||||
provider_lens_concurrency: { type: string, default: "" } # GADFLY_PROVIDER_LENS_CONCURRENCY — the per-provider lens budget (shared across the provider's models); empty falls back to user var GADFLY_DEFAULT_PROVIDER_LENS_CONCURRENCY
|
||||
timeout_secs: { type: string, default: "600" } # GADFLY_TIMEOUT_SECS (per lens)
|
||||
max_steps: { type: string, default: "14" } # GADFLY_MAX_STEPS
|
||||
worker_model: { type: string, default: "" } # GADFLY_WORKER_MODEL
|
||||
allowed_users: { type: string, default: "" } # GADFLY_ALLOWED_USERS (consumer-specific; set in your stub)
|
||||
reviewer_tag: { type: string, default: "" } # reviewer image tag (e.g. "sha-b37cd09") — empty falls back to user var GADFLY_REVIEWER_TAG, then the pin baked into the container: line
|
||||
# Gitea >= 1.27 does not propagate the CALLER's workflow_dispatch inputs into a
|
||||
# called workflow's github.event, so a manual "review PR #N" dispatch arrived
|
||||
# here with an empty PR and died at the entrypoint's "PR required" check. The
|
||||
# caller stub must thread it explicitly: `pr_number: ${{ github.event.inputs.pr_number }}`.
|
||||
pr_number: { type: string, default: "" }
|
||||
trigger_phrase: { type: string, default: "" } # GADFLY_TRIGGER_PHRASE
|
||||
consolidate: { type: string, default: "" } # GADFLY_CONSOLIDATE — "" => auto (one consensus comment for >=2 models); "0" => one comment per model
|
||||
inline_review: { type: string, default: "" } # GADFLY_INLINE_REVIEW — "" => on (post a COMMENT-state PR review with inline comments on changed lines); "0" => off
|
||||
@@ -93,8 +103,27 @@ jobs:
|
||||
review:
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: ${{ inputs.timeout_minutes }}
|
||||
# The reviewer runs as the JOB container (steps exec inside it), not a
|
||||
# `uses: docker://` step: a job container image accepts ${{ }} expressions,
|
||||
# while a `uses:` ref is parsed before any context exists. That lets the tag
|
||||
# resolve per-run — inputs.reviewer_tag → user var GADFLY_REVIEWER_TAG → the
|
||||
# baked fallback — so bumping the reviewer image is a ONE-variable edit that
|
||||
# reaches every consumer despite their cached workflow ref.
|
||||
#
|
||||
# Always point the variable at an immutable :sha-<short> tag, never :latest —
|
||||
# act_runner caches :latest and often does NOT re-pull a moved one; a fresh
|
||||
# unique tag forces the pull. NB: vars are editable at will — whoever can edit
|
||||
# the owner's variables redirects every consumer's reviewer image (same blast
|
||||
# radius as editing this file, but without a commit trail).
|
||||
#
|
||||
# Fallback pin: sha-b37cd09 — the provider-wide lens budget (PR #27) on top of
|
||||
# the opencode CLI engine (PR #26) and the Gitea >= 1.27 workflow_call
|
||||
# reclassification. Keep it current-ish when touching this file anyway.
|
||||
container:
|
||||
image: gitea.stevedudenhoeffer.com/steve/gadfly:${{ inputs.reviewer_tag || vars.GADFLY_REVIEWER_TAG || 'sha-b37cd09' }}
|
||||
steps:
|
||||
- uses: docker://gitea.stevedudenhoeffer.com/steve/gadfly:sha-3095ebf
|
||||
- name: Run the gadfly reviewer
|
||||
run: /entrypoint.sh
|
||||
env:
|
||||
# --- event context (from the CALLER's github.*) -------------------
|
||||
GITEA_API: ${{ github.server_url }}/api/v1/repos/${{ github.repository }}
|
||||
@@ -104,13 +133,15 @@ jobs:
|
||||
# forwarding, since the auto token isn't a forwarded workflow_call secret.
|
||||
GITEA_TOKEN: ${{ github.token }}
|
||||
EVENT_NAME: ${{ github.event_name }}
|
||||
PR: ${{ github.event.pull_request.number || github.event.issue.number || github.event.inputs.pr_number }}
|
||||
PR: ${{ github.event.pull_request.number || github.event.issue.number || github.event.inputs.pr_number || inputs.pr_number }}
|
||||
PR_BRANCH: ${{ github.head_ref }}
|
||||
IS_DRAFT: ${{ github.event.pull_request.draft }}
|
||||
COMMENT_BODY: ${{ github.event.comment.body }}
|
||||
COMMENT_ID: ${{ github.event.comment.id }}
|
||||
ACTOR: ${{ github.actor }}
|
||||
# --- provider auth (forwarded workflow_call secrets; empty if the caller doesn't forward it) -
|
||||
# OLLAMA_CLOUD_API_KEY powers both the ollama-cloud majordomo path AND
|
||||
# the opencode engine (GADFLY_MODELS entry "opencode/<model>").
|
||||
OLLAMA_CLOUD_API_KEY: ${{ secrets.OLLAMA_CLOUD_API_KEY }}
|
||||
OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
|
||||
ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }}
|
||||
@@ -124,12 +155,14 @@ jobs:
|
||||
# token, keep that one a secret instead.
|
||||
GADFLY_ENDPOINT_M1: ${{ vars.GADFLY_ENDPOINT_M1 }}
|
||||
GADFLY_ENDPOINT_M5: ${{ vars.GADFLY_ENDPOINT_M5 }}
|
||||
# ragnaros = the 4090 Ti via its llama-swap proxy. Defined in the user
|
||||
# var GADFLY_ENDPOINT_RAGNAROS (format "<provider>|<base-url>[|<key>]")
|
||||
# so the URL can change without editing this file; the matching model is
|
||||
# ragnaros/qwen3.6-27b in GADFLY_DEFAULT_MODELS. NB: use the un-hyphenated
|
||||
# `llamaswap` provider spelling in the var — the pinned image needs it.
|
||||
GADFLY_ENDPOINT_RAGNAROS: ${{ vars.GADFLY_ENDPOINT_RAGNAROS }}
|
||||
# netherstorm = the local GPU box via its llama-swap proxy. Defined in the
|
||||
# user var GADFLY_ENDPOINT_NETHERSTORM (format "<provider>|<base-url>[|<key>]",
|
||||
# e.g. "llamaswap|https://llama-swap.netherstorm...") so the URL can change
|
||||
# without editing this file; the matching model is netherstorm/qwen3.6-27b in
|
||||
# GADFLY_DEFAULT_MODELS. NB: use the un-hyphenated `llamaswap` provider
|
||||
# spelling in the var — the pinned image needs it. Without this line the var
|
||||
# is silently dropped at the workflow_call boundary ('unknown provider').
|
||||
GADFLY_ENDPOINT_NETHERSTORM: ${{ vars.GADFLY_ENDPOINT_NETHERSTORM }}
|
||||
# --- findings telemetry (optional) --------------------------------
|
||||
GADFLY_FINDINGS_URL: ${{ secrets.GADFLY_FINDINGS_URL }}
|
||||
GADFLY_FINDINGS_TOKEN: ${{ secrets.GADFLY_FINDINGS_TOKEN }}
|
||||
@@ -138,7 +171,9 @@ jobs:
|
||||
GADFLY_SPECIALISTS: ${{ inputs.specialists || vars.GADFLY_DEFAULT_SPECIALISTS }}
|
||||
GADFLY_PROVIDER: ${{ inputs.provider }}
|
||||
GADFLY_BASE_URL: ${{ inputs.base_url }}
|
||||
GADFLY_PROVIDER_CONCURRENCY: ${{ inputs.provider_concurrency || vars.GADFLY_DEFAULT_PROVIDER_CONCURRENCY }}
|
||||
# NB: GADFLY_PROVIDER_CONCURRENCY (the old model cap) is intentionally no
|
||||
# longer forwarded — entrypoint.sh ignores it. The lens budget is the one
|
||||
# throttle now, shared across a provider's models.
|
||||
GADFLY_PROVIDER_LENS_CONCURRENCY: ${{ inputs.provider_lens_concurrency || vars.GADFLY_DEFAULT_PROVIDER_LENS_CONCURRENCY }}
|
||||
GADFLY_TIMEOUT_SECS: ${{ inputs.timeout_secs }}
|
||||
GADFLY_MAX_STEPS: ${{ inputs.max_steps }}
|
||||
|
||||
@@ -34,7 +34,10 @@ verifies each one against the actual code, and posts its findings as a comment.
|
||||
cmd/gadfly/ the reviewer binary — pure producer of review markdown (stdout)
|
||||
main.go orchestration: fan specialists out (executus/fanout), each a review pass + recheck
|
||||
engine.go reviewEngine abstraction: executus run.Executor (majordomo agent loop +
|
||||
compaction/bounding/budget/critic) vs claude-code CLI shell-out
|
||||
compaction/bounding/budget/critic) vs claude-code / opencode CLI shell-outs
|
||||
opencode.go the opencode CLI engine: ollama-cloud model through the OpenCode harness
|
||||
(read-only via a generated OPENCODE_CONFIG_CONTENT agent+provider); for
|
||||
benchmarking the boutique harness vs a free one on the same model
|
||||
executus.go executus wiring: tool.Registry over the repo tools, the run.Executor build
|
||||
(compact + model context-limit threshold + per-PR budget + wrap-up critic)
|
||||
specialists.go specialist lenses: built-ins, default suite, env + .gadfly.yml resolution
|
||||
@@ -54,7 +57,7 @@ Dockerfile multi-stage; private-module creds via BuildKit secrets ne
|
||||
.gitea/workflows/build-image.yml push main → :latest; tag v* → :<tag>+:latest; PR → build-only
|
||||
.gitea/workflows/review-reusable.yml reusable (workflow_call) review job; resolves swarm config at
|
||||
RUNTIME: consumer `with:` input → owner user-scope var (GADFLY_DEFAULT_MODELS /
|
||||
_SPECIALISTS / _PROVIDER_CONCURRENCY / _PROVIDER_LENS_CONCURRENCY, +
|
||||
_SPECIALISTS / _PROVIDER_LENS_CONCURRENCY, +
|
||||
GADFLY_ENDPOINT_RAGNAROS) → image default. Vars are injected per-run, so editing
|
||||
one var retunes the whole fleet even though long-lived act_runners CACHE this file
|
||||
by ref (a moved tag is NOT re-fetched — only a runtime value or a fresh @<sha>
|
||||
@@ -151,10 +154,19 @@ are actually exercised. OpenAI/Anthropic/Google come from majordomo's abstractio
|
||||
NOT re-pulled, so the job silently runs the previous image. For a run that must use a specific
|
||||
build (e.g. validating a just-pushed fix), pin the consumer stub to the immutable
|
||||
`:sha-<short>` tag the build publishes, not `:latest`.
|
||||
- **Concurrency is per-provider** (`entrypoint.sh`): each provider is a lane, lanes run in
|
||||
parallel, `cap` (from `GADFLY_PROVIDER_CONCURRENCY` else `GADFLY_CONCURRENCY`, default 1) bounds
|
||||
models-at-once within a lane. The review timeout (`GADFLY_TIMEOUT_SECS`) is **per-lens**, not
|
||||
shared across the suite — a slow model can't starve later lenses (the original timeout bug).
|
||||
- **Concurrency is one per-provider lens budget** (`entrypoint.sh` + `cmd/gadfly/lenssem.go`):
|
||||
each provider is a lane, lanes run in parallel, and within a lane ALL of the provider's models
|
||||
run at once — the only throttle is a **provider-wide lens budget** (max lens passes in flight,
|
||||
a lens = one specialist's review+recheck). The budget comes from
|
||||
`GADFLY_PROVIDER_LENS_CONCURRENCY` (`provider=N` map) else `GADFLY_LENS_CONCURRENCY` (default 1).
|
||||
Because models are separate processes, the budget is a **cross-process permit pool**: entrypoint
|
||||
seeds a per-lane dir of N flock files; each model's binary acquires one before a lens pass and
|
||||
releases it after (flock auto-drops on process death). This replaced the old two-level
|
||||
`GADFLY_PROVIDER_CONCURRENCY` MODEL cap × per-model `GADFLY_LENS_CONCURRENCY`, which multiplied
|
||||
and let a model hold its slot through its last lens — stalling the next model with idle lens
|
||||
capacity. Those two model-cap vars are now **ignored**. The review timeout
|
||||
(`GADFLY_TIMEOUT_SECS`) is **per-lens**, not shared across the suite — a slow lens can't starve
|
||||
the others (the original timeout bug).
|
||||
- **Large-PR token burn**: the agent loop re-sends the whole transcript every step, so a giant
|
||||
diff (the old `get_diff` dumped it untruncated, and it was embedded in both the review and
|
||||
recheck task) was re-transmitted ~steps × lenses × passes × models times — a ~250 K-token PR
|
||||
|
||||
+20
@@ -33,6 +33,26 @@ RUN apk add --no-cache bash git curl jq ca-certificates nodejs npm procps
|
||||
# CLI to the image (notably larger); ollama-only users pay the size but nothing
|
||||
# else. Auth is provided at runtime via CLAUDE_CODE_OAUTH_TOKEN / ANTHROPIC_API_KEY.
|
||||
RUN npm install -g @anthropic-ai/claude-code && npm cache clean --force
|
||||
# Bundle the OpenCode CLI (opencode.ai) for the `opencode` review engine
|
||||
# (GADFLY_MODELS=opencode/<model>): a freely-available agentic harness driving an
|
||||
# ollama-cloud model, used to benchmark it against gadfly's own executus harness
|
||||
# on the same model. Auth reuses OLLAMA_CLOUD_API_KEY at runtime. opencode ships a
|
||||
# compiled (Bun) binary; it publishes musl variants (opencode-linux-*-musl) that
|
||||
# npm auto-selects on alpine via the package "libc" field. libstdc++/libgcc are
|
||||
# the Bun binary's runtime deps; gcompat is a belt-and-suspenders fallback in case
|
||||
# npm ever resolves a glibc build here.
|
||||
RUN apk add --no-cache gcompat libstdc++ libgcc \
|
||||
&& npm install -g opencode-ai \
|
||||
&& npm cache clean --force
|
||||
# Best-effort: confirm the binary runs and pre-warm the openai-compatible provider
|
||||
# package into opencode's cache so a review doesn't pay a first-run npm fetch. The
|
||||
# warm-up model call intentionally fails against a dead URL. Never fail the build:
|
||||
# a musl/runtime quirk here must not break the shared image for ollama/claude
|
||||
# users — a broken opencode engine degrades to a normal (advisory) pass error.
|
||||
RUN opencode --version >/dev/null 2>&1 \
|
||||
&& OPENCODE_CONFIG_CONTENT='{"provider":{"gadfly":{"npm":"@ai-sdk/openai-compatible","options":{"baseURL":"http://127.0.0.1:9/v1"},"models":{"x":{}}}}}' \
|
||||
timeout 120 opencode run --model gadfly/x "warm" >/dev/null 2>&1 \
|
||||
; true
|
||||
COPY --from=build /out/gadfly /usr/local/bin/gadfly
|
||||
COPY scripts /app/scripts
|
||||
COPY entrypoint.sh /entrypoint.sh
|
||||
|
||||
@@ -140,6 +140,58 @@ as an example, not wired or tested here.
|
||||
> specialist selection and the `delegate_investigation` worker are majordomo-only and are skipped
|
||||
> with this engine (Claude Code does its own legwork).
|
||||
|
||||
### OpenCode engine (`opencode`)
|
||||
|
||||
The same shell-out idea, but with a **freely-available** harness: Gadfly can review through the
|
||||
**[OpenCode](https://opencode.ai) CLI**, which — like Claude Code — brings its own read tools and
|
||||
verifies findings against the checked-out repo, but drives an **ollama-cloud** model. The point is
|
||||
to benchmark gadfly's boutique executus harness against a good open harness *on the same model*:
|
||||
run `ollama-cloud/glm-5.2` (majordomo loop) and `opencode/glm-5.2` (OpenCode) side by side and
|
||||
compare their findings. This is the wired, no-proxy version of the "alternate backends" comparison
|
||||
described above. The CLI is bundled in the image (Node + `opencode-ai`).
|
||||
|
||||
Select it as a model id:
|
||||
|
||||
| Spec | Meaning |
|
||||
|------|---------|
|
||||
| `opencode/glm-5.2` | serve `glm-5.2` via ollama-cloud through OpenCode |
|
||||
| `open-code/glm-5.2` | accepted alias spelling (`opencode` is canonical) |
|
||||
| `opencode/qwen3-coder:480b-cloud` | model ids are taken **verbatim** — colons are preserved (no `:thinking` suffix here, unlike claude-code) |
|
||||
| `opencode/<provider>/<model>` | escape hatch: pass `<provider>/<model>` straight to OpenCode's own provider registry/auth (e.g. `opencode/anthropic/claude-sonnet-4-6`) |
|
||||
| `opencode` | bare: OpenCode's configured default model |
|
||||
|
||||
```yaml
|
||||
GADFLY_MODELS: "ollama-cloud/glm-5.2,opencode/glm-5.2" # the benchmark pairing
|
||||
```
|
||||
|
||||
Auth reuses **`OLLAMA_CLOUD_API_KEY`** (the same secret the ollama-cloud path uses; it's mapped to
|
||||
`OLLAMA_API_KEY`, which the generated provider references as `{env:OLLAMA_API_KEY}` — never a literal
|
||||
secret in config). Tuning knobs (all optional):
|
||||
|
||||
| Env | Default | Meaning |
|
||||
|-----|---------|---------|
|
||||
| `GADFLY_OPENCODE_MODEL` | *(from the spec suffix)* | overrides the model |
|
||||
| `GADFLY_OPENCODE_BASE_URL` | `https://ollama.com/v1` | ollama-cloud endpoint; point at a local Ollama (`http://localhost:11434/v1`) or any OpenAI-compatible server |
|
||||
| `GADFLY_OPENCODE_EXTRA_ARGS` | *(unset)* | extra `opencode run` args, **whitespace-split**, appended before the positional task |
|
||||
| `GADFLY_OPENCODE_BIN` | `opencode` | CLI binary path |
|
||||
|
||||
> **Read-only is enforced through config, not a flag.** OpenCode has no `--append-system-prompt`, so
|
||||
> Gadfly generates a per-lens config — the lens system prompt as a `gadfly` agent's prompt, with the
|
||||
> mutating and network tools (`edit`/`bash`/`webfetch`/`websearch`/`external_directory`) denied at both
|
||||
> the global and agent level — and injects it via `OPENCODE_CONFIG_CONTENT`. That env var is the
|
||||
> highest-precedence config source in the container, so it **outranks any `opencode.json` a reviewed
|
||||
> repo ships** — a repo can't re-enable edits on the reviewer. The subprocess runs with a **reduced
|
||||
> environment**: the provider keys OpenCode needs to authenticate (`OLLAMA_API_KEY` for the primary
|
||||
> path, plus `ANTHROPIC_*`/`OPENAI_*`/`GOOGLE_*`/`GEMINI_*` for the `opencode/<provider>/<model>`
|
||||
> pass-through) alongside `PATH`/`HOME`/locale/`OPENCODE_*`/`GADFLY_OPENCODE_*` — but **not** gadfly's
|
||||
> own secrets (the Gitea token, the findings token, or the claude-code subscription token), which the
|
||||
> CLI has no use for.
|
||||
|
||||
> **Newly wired, lightly tested.** Like the claude-code engine, `auto` specialist selection and the
|
||||
> `delegate_investigation` worker are majordomo-only and are skipped here (OpenCode does its own
|
||||
> legwork). Output capture reads OpenCode's default text output, so treat the engine as new and
|
||||
> sanity-check a run before trusting a benchmark.
|
||||
|
||||
### Endpoint aliases via env vars
|
||||
|
||||
For multiple named backends (e.g. a couple of Ollama boxes on your LAN), register them by
|
||||
@@ -220,38 +272,32 @@ Unset = no delegation (current behavior).
|
||||
### Concurrency (per-provider lanes)
|
||||
|
||||
With multiple models, each **provider** is its own lane and lanes run in **parallel**, so a fast
|
||||
cloud provider isn't stuck behind a slow local box. Within a lane, at most `cap` models run at
|
||||
once — `cap` comes from `GADFLY_PROVIDER_CONCURRENCY` (a `provider=N` map) else `GADFLY_CONCURRENCY`
|
||||
(default `1`). The timeout is **per-lens** (`GADFLY_TIMEOUT_SECS`), so a slow model on one lens
|
||||
can't starve the others.
|
||||
cloud provider isn't stuck behind a slow local box. There is **one throttle**: a per-provider
|
||||
**lens budget** — the max number of lens passes (a lens = one specialist's review+recheck) in
|
||||
flight at once for that provider. Every model in the lane runs concurrently and its lenses draw
|
||||
from that single shared budget, so nothing else caps how many models run. The budget comes from
|
||||
`GADFLY_PROVIDER_LENS_CONCURRENCY` (a `provider=N` map) else the `GADFLY_LENS_CONCURRENCY` scalar
|
||||
(default `1`). The timeout is **per-lens** (`GADFLY_TIMEOUT_SECS`), so a slow lens can't starve
|
||||
the others.
|
||||
|
||||
```yaml
|
||||
# One local box (serial — it serves one model at a time) + 3 cloud reviews at once,
|
||||
# both lanes running concurrently:
|
||||
GADFLY_PROVIDER_CONCURRENCY: "ollama-cloud=3,m1pro=1"
|
||||
# The local box gets 1 lens at a time (serial); the cloud lane runs up to 3 lens passes at once,
|
||||
# shared across ALL its models. Both lanes run concurrently.
|
||||
GADFLY_PROVIDER_LENS_CONCURRENCY: "ollama-cloud=3,m1pro=1"
|
||||
GADFLY_MODELS: "m1pro/qwen3:14b,qwen3-coder:480b-cloud,gpt-oss:120b-cloud"
|
||||
```
|
||||
|
||||
A model's provider is the spec's first segment (`m1pro/…` → `m1pro`), or `GADFLY_PROVIDER`/
|
||||
`ollama-cloud` for a bare id. Default (`cap 1`) keeps a single-provider pool fully sequential.
|
||||
`ollama-cloud` for a bare id. The budget is **shared across the provider's models**: with a
|
||||
budget of 3 and two cloud models, you get 3 lens passes in flight in any mix — as one model
|
||||
finishes a lens, the freed slot immediately goes to another model's next lens, so a model
|
||||
winding down to its last lens never stalls the others (the pre-2026-07 design capped *models*
|
||||
separately and did stall — that `GADFLY_PROVIDER_CONCURRENCY`/`GADFLY_CONCURRENCY` model cap is
|
||||
**gone**; those vars are now ignored). Default (budget `1`) keeps a provider fully sequential.
|
||||
|
||||
**Lens fan-out (within a model).** By default the specialist lenses run **sequentially** inside
|
||||
each model (`GADFLY_LENS_CONCURRENCY=1`). Raise it to overlap the independent per-lens
|
||||
review+recheck passes — the model then posts its consolidated comment as soon as its lenses
|
||||
finish (so with sequential models, results stream in per model and per-model timings stay
|
||||
clean). Like the model cap, it's **per-provider configurable**: `GADFLY_PROVIDER_LENS_CONCURRENCY`
|
||||
takes a `provider=N` map keyed by the **same provider lanes** as `GADFLY_PROVIDER_CONCURRENCY`,
|
||||
falling back to the `GADFLY_LENS_CONCURRENCY` scalar (default `1`). **It multiplies with the
|
||||
model cap:** total in-flight requests ≈ *models-at-once × lenses-at-once*, so to fan lenses out
|
||||
without oversubscribing a backend, keep its model cap low and raise its lens cap:
|
||||
|
||||
```yaml
|
||||
# Per provider: cloud runs one model at a time but fans its 3 lenses out (3 concurrent requests);
|
||||
# the slow local box stays fully serial. Both provider lanes still run in parallel.
|
||||
GADFLY_PROVIDER_CONCURRENCY: "ollama-cloud=1,m1=1"
|
||||
GADFLY_PROVIDER_LENS_CONCURRENCY: "ollama-cloud=3,m1=1"
|
||||
GADFLY_SPECIALISTS: "security,correctness,error-handling"
|
||||
```
|
||||
> Under the hood the shared budget is a small cross-process permit pool (flock files, seeded per
|
||||
> lane by `entrypoint.sh`); permits release automatically if a model process dies, so a crashed
|
||||
> lens can't leak budget.
|
||||
|
||||
### Live status board
|
||||
|
||||
@@ -355,6 +401,12 @@ pinned version (plus `:latest`). Pin full-stub consumers to a `:vN` image tag fo
|
||||
(`@v1`) or `@main` is often **not** re-fetched and silently runs a stale copy. A fresh `@<sha>` is the
|
||||
only reliable way to roll out a *structural* change to the reusable.
|
||||
|
||||
Structural changes are the rare case, though: the reviewer **image tag** the reusable runs resolves at
|
||||
runtime (`reviewer_tag` input → user var `GADFLY_REVIEWER_TAG` → the fallback pin baked into the
|
||||
reusable), so a routine Gadfly release is *build the image → update `GADFLY_REVIEWER_TAG` to the new
|
||||
`sha-<short>`* — every pinned consumer picks it up on its next review, no re-pin. Always point the
|
||||
variable at an immutable `sha-` tag, never `:latest` (the runner caches `:latest`).
|
||||
|
||||
### Central config via variables
|
||||
|
||||
So you don't have to re-pin every consumer just to retune the swarm, the reusable resolves its config
|
||||
@@ -364,10 +416,10 @@ on its next review **without** a re-pin or a tag move:
|
||||
|
||||
| Variable (user/org scope) | Sets |
|
||||
|---|---|
|
||||
| `GADFLY_REVIEWER_TAG` | the reviewer **image tag** the reusable runs (e.g. `sha-b37cd09`); empty ⇒ the fallback pin baked into the reusable |
|
||||
| `GADFLY_DEFAULT_MODELS` | `GADFLY_MODELS` (csv) |
|
||||
| `GADFLY_DEFAULT_SPECIALISTS` | the lens suite |
|
||||
| `GADFLY_DEFAULT_PROVIDER_CONCURRENCY` | models-at-once per provider |
|
||||
| `GADFLY_DEFAULT_PROVIDER_LENS_CONCURRENCY` | lenses-at-once per provider |
|
||||
| `GADFLY_DEFAULT_PROVIDER_LENS_CONCURRENCY` | the per-provider lens budget (lens passes in flight per provider, shared across its models) |
|
||||
| `GADFLY_ENDPOINT_RAGNAROS` | a named endpoint, e.g. `llamaswap\|https://host` |
|
||||
|
||||
Adding a *new* named endpoint still needs a one-line reusable edit (Gitea can't auto-expose arbitrary
|
||||
@@ -384,14 +436,14 @@ The reviewer binary reads these (the stub/entrypoint set sane defaults):
|
||||
| `GADFLY_BASE_URL` | — | override endpoint (OpenAI/Ollama-compatible servers) |
|
||||
| `GADFLY_API_KEY` | — | provider key; falls back to the provider's standard env |
|
||||
| `claude-code` model id | — | route a model through the bundled Claude Code CLI (`claude-code` / `claude-code/<model>`); see [Claude Code engine](#claude-code-engine-claude-code) for its `GADFLY_CLAUDE_*` knobs |
|
||||
| `opencode` model id | — | route an ollama-cloud model through the bundled OpenCode CLI (`opencode/<model>`); see [OpenCode engine](#opencode-engine-opencode) for its `GADFLY_OPENCODE_*` knobs |
|
||||
| `GADFLY_SPECIALISTS` | default suite | csv of lenses, `all`, or `auto` (dynamic selection) |
|
||||
| `GADFLY_SELECTOR_MODEL` | review model | model that picks lenses in `auto` mode |
|
||||
| `GADFLY_WORKER_MODEL` | — | cheap model for `delegate_investigation`; unset = no delegation |
|
||||
| `GADFLY_WORKER_MAX_STEPS` | 8 | tool-step cap for a delegated worker run |
|
||||
| `GADFLY_CONCURRENCY` | 1 | default max models run at once **per provider** |
|
||||
| `GADFLY_PROVIDER_CONCURRENCY` | — | per-provider overrides, e.g. `ollama-cloud=3,m1pro=1` |
|
||||
| `GADFLY_LENS_CONCURRENCY` | 1 | specialist lenses run at once **within a model** (× model cap = total in-flight) |
|
||||
| `GADFLY_PROVIDER_LENS_CONCURRENCY` | — | per-provider lens overrides, same lanes as `GADFLY_PROVIDER_CONCURRENCY`, e.g. `ollama-cloud=3,m1=1` |
|
||||
| `GADFLY_LENS_CONCURRENCY` | 1 | **per-provider lens budget** — lens passes in flight per provider, shared across all its models (all a provider's models run at once; this is the only throttle) |
|
||||
| `GADFLY_PROVIDER_LENS_CONCURRENCY` | — | per-provider lens-budget overrides, a `provider=N` map, e.g. `ollama-cloud=3,m1=1` |
|
||||
| `GADFLY_CONCURRENCY` / `GADFLY_PROVIDER_CONCURRENCY` | — | **removed** (was the per-provider models-at-once cap; now ignored — the lens budget is the single throttle) |
|
||||
| `GADFLY_MAX_STEPS` | 24 | review-pass tool-step cap |
|
||||
| `GADFLY_TIMEOUT_SECS` | 300 | deadline **per specialist lens** (review+recheck) |
|
||||
| `GADFLY_RECHECK` | on | set `0`/`false` to skip the recheck pass |
|
||||
|
||||
+34
-21
@@ -19,16 +19,19 @@ import (
|
||||
// the model's text answer. It is the one primitive both review passes use — the
|
||||
// draft review and the adversarial recheck — so the rest of the pipeline
|
||||
// (specialist composition, recheck orchestration, consolidation, emit) is
|
||||
// engine-agnostic. Two implementations:
|
||||
// engine-agnostic. Three implementations:
|
||||
//
|
||||
// - majordomoEngine: the original path — a majordomo tool-using agent loop
|
||||
// (read_file/grep/… over a sandboxed repoFS).
|
||||
// - claudeCodeEngine: shells out to the `claude` CLI in print mode, which
|
||||
// brings its OWN repo tools; gadfly just feeds it the prompt and reads back
|
||||
// the final text.
|
||||
// - openCodeEngine (opencode.go): shells out to the `opencode` CLI likewise,
|
||||
// but driving an ollama-cloud model — for benchmarking the two harnesses on
|
||||
// the same model.
|
||||
//
|
||||
// maxSteps is the tool-step budget for engines that have one (majordomo); the
|
||||
// claude-code engine manages its own loop and ignores it.
|
||||
// shell-out engines manage their own loop and ignore it.
|
||||
type reviewEngine interface {
|
||||
runPass(ctx context.Context, system, task string, maxSteps int) (string, error)
|
||||
}
|
||||
@@ -157,16 +160,7 @@ func (e *claudeCodeEngine) runPass(ctx context.Context, system, task string, _ i
|
||||
// Force an extended-thinking budget for this run (a "...:max" spec).
|
||||
cmd.Env = append(cmd.Env, "MAX_THINKING_TOKENS="+strconv.Itoa(e.thinkingTokens))
|
||||
}
|
||||
// Put the CLI and the Node children it spawns in their own process group and
|
||||
// kill the WHOLE group on context cancel, so a timed-out lens can't leave
|
||||
// orphaned claude/node processes behind in the container.
|
||||
cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
|
||||
cmd.Cancel = func() error {
|
||||
if cmd.Process != nil {
|
||||
_ = syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
killGroupOnCancel(cmd)
|
||||
var stdout, stderr bytes.Buffer
|
||||
cmd.Stdout = &stdout
|
||||
cmd.Stderr = &stderr
|
||||
@@ -217,13 +211,39 @@ func (e *claudeCodeEngine) runPass(ctx context.Context, system, task string, _ i
|
||||
return "", fmt.Errorf("claude -p produced no parseable output")
|
||||
}
|
||||
|
||||
// killGroupOnCancel puts cmd in its own process group and, on context cancel,
|
||||
// SIGKILLs the whole group — so a timed-out shell-out CLI (claude/opencode) can't
|
||||
// leave orphaned Node children behind in the container. Call before cmd.Run.
|
||||
func killGroupOnCancel(cmd *exec.Cmd) {
|
||||
cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
|
||||
cmd.Cancel = func() error {
|
||||
if cmd.Process != nil {
|
||||
_ = syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
}
|
||||
|
||||
// filterEnv returns the current process environment reduced to the variables for
|
||||
// which keep returns true. Shared by the shell-out engines' minimal-env builders
|
||||
// (claudeEnv, openCodeEnv), which differ only in their keep predicate.
|
||||
func filterEnv(keep func(string) bool) []string {
|
||||
var env []string
|
||||
for _, kv := range os.Environ() {
|
||||
if k, _, ok := strings.Cut(kv, "="); ok && keep(k) {
|
||||
env = append(env, kv)
|
||||
}
|
||||
}
|
||||
return env
|
||||
}
|
||||
|
||||
// claudeEnv builds a minimal environment for the `claude` subprocess: only what
|
||||
// the CLI needs (PATH/HOME, its auth tokens, locale, Node/XDG/GADFLY_CLAUDE_*
|
||||
// knobs), deliberately dropping the rest of the runner's secrets — GITEA_TOKEN,
|
||||
// GADFLY_FINDINGS_TOKEN, provider keys — so they never reach the third-party
|
||||
// CLI. Defense in depth: the parent already holds them, but the CLI has no need.
|
||||
func claudeEnv() []string {
|
||||
keep := func(k string) bool {
|
||||
return filterEnv(func(k string) bool {
|
||||
switch k {
|
||||
case "PATH", "HOME", "USER", "LOGNAME", "TMPDIR", "LANG", "TERM", "SHELL", "MAX_THINKING_TOKENS":
|
||||
return true
|
||||
@@ -234,14 +254,7 @@ func claudeEnv() []string {
|
||||
strings.HasPrefix(k, "GADFLY_CLAUDE_") ||
|
||||
strings.HasPrefix(k, "NODE_") ||
|
||||
strings.HasPrefix(k, "XDG_")
|
||||
}
|
||||
var env []string
|
||||
for _, kv := range os.Environ() {
|
||||
if k, _, ok := strings.Cut(kv, "="); ok && keep(k) {
|
||||
env = append(env, kv)
|
||||
}
|
||||
}
|
||||
return env
|
||||
})
|
||||
}
|
||||
|
||||
// truncateForErr caps CLI error detail so a stderr dump can't bloat the comment,
|
||||
|
||||
@@ -156,7 +156,7 @@ func TestRunSpecialists_PerProviderFanOut(t *testing.T) {
|
||||
|
||||
// TestLensConcurrency covers the resolution matrix: scalar default, scalar
|
||||
// override, and per-provider override keyed by the model's resolved lane (same
|
||||
// lane rule entrypoint.sh uses for GADFLY_PROVIDER_CONCURRENCY).
|
||||
// lane rule entrypoint.sh uses for GADFLY_PROVIDER_LENS_CONCURRENCY).
|
||||
func TestLensConcurrency(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
|
||||
@@ -0,0 +1,115 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"syscall"
|
||||
"time"
|
||||
)
|
||||
|
||||
// lensSem is the PROVIDER-WIDE lens-permit pool. Historically each model's binary
|
||||
// throttled its own lenses in-process (GADFLY_LENS_CONCURRENCY) while entrypoint.sh
|
||||
// separately capped how many MODELS ran at once — two multiplicative gates in
|
||||
// different processes. That let a model hold its whole slot until its last lens
|
||||
// finished, stalling the next model even with idle lens capacity.
|
||||
//
|
||||
// Instead, entrypoint.sh now runs all of a provider's models concurrently and
|
||||
// seeds ONE directory of N permit files per provider (N = the provider's lens
|
||||
// budget). Every model process in that lane draws from the same pool: a 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. Permits are held with flock, which the kernel drops when the
|
||||
// holding process exits — so a killed/crashed model frees its permits for free.
|
||||
type lensSem struct {
|
||||
dir string
|
||||
size int
|
||||
}
|
||||
|
||||
// lensSemPollInterval is how often a blocked acquirer re-sweeps the permit files.
|
||||
// Lens passes run for many seconds to minutes, so a coarse poll adds negligible
|
||||
// latency while keeping the mechanism a few lines of stdlib (no IPC primitives).
|
||||
const lensSemPollInterval = 150 * time.Millisecond
|
||||
|
||||
// activeLensSem returns the shared semaphore configured by entrypoint.sh, or nil
|
||||
// when it isn't set (standalone/local runs, tests, or an older entrypoint) — in
|
||||
// which case runSpecialists falls back to the in-process fanout limit alone, i.e.
|
||||
// the pre-existing per-model behavior.
|
||||
func activeLensSem() *lensSem {
|
||||
dir := os.Getenv("GADFLY_LENS_SEM_DIR")
|
||||
if dir == "" {
|
||||
return nil
|
||||
}
|
||||
n := envInt("GADFLY_LENS_SEM_SIZE", 0)
|
||||
if n < 1 {
|
||||
// A dir was requested but the size is missing/blank/invalid. Rather than
|
||||
// silently run unthrottled, say so on stderr — it's almost certainly a
|
||||
// misconfiguration in entrypoint.sh (the two are set together).
|
||||
fmt.Fprintf(os.Stderr, "gadfly: GADFLY_LENS_SEM_DIR set but GADFLY_LENS_SEM_SIZE=%q invalid; lenses run unthrottled\n", os.Getenv("GADFLY_LENS_SEM_SIZE"))
|
||||
return nil
|
||||
}
|
||||
return &lensSem{dir: dir, size: n}
|
||||
}
|
||||
|
||||
// acquire blocks until a permit is free (or ctx is done) and returns a release
|
||||
// func. It FAILS OPEN: if the pool directory is structurally unusable (missing,
|
||||
// unwritable, or on a filesystem without flock) it logs once and returns a no-op
|
||||
// release with a nil error, so the lens runs UNTHROTTLED rather than hanging the
|
||||
// whole review forever — this is an advisory reviewer, a slightly oversubscribed
|
||||
// backend beats a stuck one. Only a full-but-healthy pool actually blocks; on ctx
|
||||
// cancellation it returns a no-op release plus ctx.Err() so the caller can surface
|
||||
// the lens as "did not run" instead of leaking a permit.
|
||||
func (s *lensSem) acquire(ctx context.Context) (func(), error) {
|
||||
for {
|
||||
release, ok, err := s.tryAcquire()
|
||||
if ok {
|
||||
return release, nil
|
||||
}
|
||||
if err != nil {
|
||||
fmt.Fprintf(os.Stderr, "gadfly: lens permit pool %q unusable (%v); proceeding without a permit\n", s.dir, err)
|
||||
return func() {}, nil
|
||||
}
|
||||
timer := time.NewTimer(lensSemPollInterval)
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
timer.Stop() // don't leak the timer when we bail on cancellation
|
||||
return func() {}, ctx.Err()
|
||||
case <-timer.C:
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// tryAcquire makes one non-blocking sweep over the permit files. It returns a
|
||||
// release func for the first one it locks; otherwise it distinguishes a healthy
|
||||
// FULL pool (some permit was openable but flock-busy → ok=false, err=nil → keep
|
||||
// polling) from a structurally BROKEN pool (no permit was even acquirable and none
|
||||
// was merely busy → err set → caller fails open). Permit files are created lazily
|
||||
// (entrypoint.sh only guarantees the directory exists). Closing the *os.File in
|
||||
// the returned func drops the flock (the lock lives on the open file description).
|
||||
func (s *lensSem) tryAcquire() (func(), bool, error) {
|
||||
sawBusy := false
|
||||
var structural error
|
||||
for i := 0; i < s.size; i++ {
|
||||
path := filepath.Join(s.dir, fmt.Sprintf("permit.%d", i))
|
||||
f, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE, 0o600)
|
||||
if err != nil {
|
||||
structural = err // e.g. the pool dir is gone or unwritable
|
||||
continue
|
||||
}
|
||||
if err := syscall.Flock(int(f.Fd()), syscall.LOCK_EX|syscall.LOCK_NB); err == nil {
|
||||
return func() { f.Close() }, true, nil
|
||||
} else if errors.Is(err, syscall.EWOULDBLOCK) {
|
||||
sawBusy = true // this permit is held by someone else — pool has capacity
|
||||
f.Close()
|
||||
} else {
|
||||
structural = err // flock unsupported / other → not a transient "busy"
|
||||
f.Close()
|
||||
}
|
||||
}
|
||||
if sawBusy {
|
||||
return nil, false, nil // full but healthy: another lens will free a permit
|
||||
}
|
||||
return nil, false, structural
|
||||
}
|
||||
@@ -0,0 +1,136 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"context"
|
||||
"path/filepath"
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// flock permits are per open-file-description, so two separate opens of the same
|
||||
// permit file conflict even within one process — the exhaustion, blocking, and
|
||||
// max-in-flight tests below therefore exercise the same semantics a real
|
||||
// multi-process lane would see.
|
||||
|
||||
func TestActiveLensSemUnset(t *testing.T) {
|
||||
t.Setenv("GADFLY_LENS_SEM_DIR", "")
|
||||
if s := activeLensSem(); s != nil {
|
||||
t.Fatalf("expected nil sem when GADFLY_LENS_SEM_DIR unset, got %+v", s)
|
||||
}
|
||||
// A dir with a zero/blank size is inert too (must not divide the pool by 0).
|
||||
t.Setenv("GADFLY_LENS_SEM_DIR", t.TempDir())
|
||||
t.Setenv("GADFLY_LENS_SEM_SIZE", "0")
|
||||
if s := activeLensSem(); s != nil {
|
||||
t.Fatalf("expected nil sem when size < 1, got %+v", s)
|
||||
}
|
||||
}
|
||||
|
||||
func TestLensSemExhaustionAndRelease(t *testing.T) {
|
||||
s := &lensSem{dir: t.TempDir(), size: 2}
|
||||
|
||||
r1, ok, err := s.tryAcquire()
|
||||
if !ok || err != nil {
|
||||
t.Fatalf("first acquire should succeed: ok=%v err=%v", ok, err)
|
||||
}
|
||||
r2, ok, err := s.tryAcquire()
|
||||
if !ok || err != nil {
|
||||
t.Fatalf("second acquire should succeed: ok=%v err=%v", ok, err)
|
||||
}
|
||||
// Pool full but healthy: ok=false with NO error (keep polling), not a
|
||||
// structural failure.
|
||||
if _, ok, err := s.tryAcquire(); ok || err != nil {
|
||||
t.Fatalf("third acquire should be full-but-healthy: ok=%v err=%v", ok, err)
|
||||
}
|
||||
|
||||
r1() // free one permit
|
||||
r3, ok, err := s.tryAcquire()
|
||||
if !ok || err != nil {
|
||||
t.Fatalf("acquire should succeed after a release: ok=%v err=%v", ok, err)
|
||||
}
|
||||
r2()
|
||||
r3()
|
||||
}
|
||||
|
||||
// A structurally broken pool (dir missing/unwritable) must FAIL OPEN — tryAcquire
|
||||
// surfaces the error and acquire returns promptly with a no-op release and no
|
||||
// error, so a lens runs unthrottled instead of spinning forever.
|
||||
func TestLensSemBrokenPoolFailsOpen(t *testing.T) {
|
||||
s := &lensSem{dir: filepath.Join(t.TempDir(), "does", "not", "exist"), size: 2}
|
||||
|
||||
if _, ok, err := s.tryAcquire(); ok || err == nil {
|
||||
t.Fatalf("tryAcquire on a broken pool should report a structural error: ok=%v err=%v", ok, err)
|
||||
}
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
|
||||
defer cancel()
|
||||
start := time.Now()
|
||||
release, err := s.acquire(ctx)
|
||||
if err != nil {
|
||||
t.Fatalf("acquire should fail open (nil err), got %v", err)
|
||||
}
|
||||
if waited := time.Since(start); waited > time.Second {
|
||||
t.Fatalf("acquire on a broken pool should return promptly, waited %v", waited)
|
||||
}
|
||||
release() // must be a safe no-op
|
||||
}
|
||||
|
||||
func TestLensSemAcquireBlocksThenCancels(t *testing.T) {
|
||||
s := &lensSem{dir: t.TempDir(), size: 1}
|
||||
|
||||
// Immediate success while a permit is free.
|
||||
release, err := s.acquire(context.Background())
|
||||
if err != nil {
|
||||
t.Fatalf("acquire on a free pool: %v", err)
|
||||
}
|
||||
|
||||
// With the only permit held, acquire must block until ctx expires.
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
|
||||
defer cancel()
|
||||
start := time.Now()
|
||||
if _, err := s.acquire(ctx); err == nil {
|
||||
t.Fatal("acquire on an exhausted pool should return ctx error, not a permit")
|
||||
}
|
||||
if waited := time.Since(start); waited < 50*time.Millisecond {
|
||||
t.Fatalf("acquire returned too fast (%v); it should have blocked on the full pool", waited)
|
||||
}
|
||||
release()
|
||||
}
|
||||
|
||||
func TestLensSemNeverExceedsSize(t *testing.T) {
|
||||
const size = 3
|
||||
s := &lensSem{dir: t.TempDir(), size: size}
|
||||
|
||||
var inFlight, peak int64
|
||||
var wg sync.WaitGroup
|
||||
for i := 0; i < 12; i++ {
|
||||
wg.Add(1)
|
||||
go func() {
|
||||
defer wg.Done()
|
||||
release, err := s.acquire(context.Background())
|
||||
if err != nil {
|
||||
t.Errorf("acquire: %v", err)
|
||||
return
|
||||
}
|
||||
defer release()
|
||||
n := atomic.AddInt64(&inFlight, 1)
|
||||
for {
|
||||
p := atomic.LoadInt64(&peak)
|
||||
if n <= p || atomic.CompareAndSwapInt64(&peak, p, n) {
|
||||
break
|
||||
}
|
||||
}
|
||||
time.Sleep(20 * time.Millisecond)
|
||||
atomic.AddInt64(&inFlight, -1)
|
||||
}()
|
||||
}
|
||||
wg.Wait()
|
||||
|
||||
if peak > size {
|
||||
t.Fatalf("max in-flight lens permits = %d, exceeds budget %d", peak, size)
|
||||
}
|
||||
if peak == 0 {
|
||||
t.Fatal("no permits were ever acquired")
|
||||
}
|
||||
}
|
||||
+77
-44
@@ -22,7 +22,11 @@
|
||||
//
|
||||
// GADFLY_MODEL model id, or a full "provider/model" spec / majordomo
|
||||
// alias / failover chain (required). A bare id is
|
||||
// prefixed with GADFLY_PROVIDER.
|
||||
// prefixed with GADFLY_PROVIDER. Two prefixes select a
|
||||
// shell-out CLI engine instead of the in-process loop:
|
||||
// "claude-code/<model>" (Claude Code, see engine.go) and
|
||||
// "opencode/<model>" (OpenCode over ollama-cloud, see
|
||||
// opencode.go) — both bring their own repo tools.
|
||||
// GADFLY_PROVIDER provider for bare model ids (default "ollama-cloud";
|
||||
// e.g. "ollama" for a local daemon, "openai", …).
|
||||
// GADFLY_BASE_URL override the backend endpoint (OpenAI/Ollama-compatible
|
||||
@@ -45,15 +49,18 @@
|
||||
// GADFLY_RECHECK set to 0/false to skip the recheck pass (optional, default on).
|
||||
// GADFLY_RECHECK_MAX_STEPS recheck-pass step cap (optional, default 16).
|
||||
// GADFLY_TIMEOUT_SECS overall deadline in seconds, shared by both passes (optional, default 300).
|
||||
// GADFLY_LENS_CONCURRENCY how many specialist lenses run concurrently within this
|
||||
// model (optional, default 1 = sequential). Total in-flight
|
||||
// model requests ≈ this × entrypoint.sh's per-provider model
|
||||
// concurrency, so keep the product within the backend's budget.
|
||||
// GADFLY_LENS_CONCURRENCY how many specialist lenses run concurrently (optional,
|
||||
// default 1 = sequential). Under entrypoint.sh this is the
|
||||
// PROVIDER-WIDE lens budget, shared across all of that
|
||||
// provider's models via a permit pool (GADFLY_LENS_SEM_DIR),
|
||||
// so it bounds total lens passes in flight per provider.
|
||||
// GADFLY_PROVIDER_LENS_CONCURRENCY per-provider override for the above, as a
|
||||
// "provider=N,provider=N" map keyed by the SAME provider
|
||||
// lanes as GADFLY_PROVIDER_CONCURRENCY (e.g.
|
||||
// "ollama-cloud=3,m1=1"). Wins over GADFLY_LENS_CONCURRENCY
|
||||
// for the model's provider; falls back to it otherwise.
|
||||
// "provider=N,provider=N" map keyed by the provider lanes
|
||||
// (e.g. "ollama-cloud=3,m1=1"). Wins over
|
||||
// GADFLY_LENS_CONCURRENCY for the model's provider.
|
||||
// GADFLY_LENS_SEM_DIR / GADFLY_LENS_SEM_SIZE set by entrypoint.sh: the shared
|
||||
// cross-process lens-permit pool (dir of N flock files)
|
||||
// and its size. Unset => in-process lensConcurrency only.
|
||||
// GADFLY_MAX_DIFF_CHARS diff chars embedded in the review prompt (optional, default 60000;
|
||||
// the full diff is reachable via the paginated get_diff tool).
|
||||
//
|
||||
@@ -90,9 +97,11 @@ const (
|
||||
// calls and then hard-failing with "max steps reached without a final
|
||||
// answer" — it always has a few steps left to wrap up.
|
||||
defaultWrapUpReserve = 4
|
||||
// defaultLensConcurrency is how many specialist lenses run at once within a
|
||||
// single model. 1 keeps the suite sequential (the historical behavior);
|
||||
// higher values overlap the independent per-lens passes. See runSpecialists.
|
||||
// defaultLensConcurrency is the fallback lens budget when neither
|
||||
// GADFLY_PROVIDER_LENS_CONCURRENCY nor GADFLY_LENS_CONCURRENCY is set. 1 keeps
|
||||
// the suite sequential (the historical behavior); higher values overlap the
|
||||
// independent per-lens passes. Under entrypoint.sh the resolved value is the
|
||||
// provider-wide budget shared across the provider's models. See runSpecialists.
|
||||
defaultLensConcurrency = 1
|
||||
)
|
||||
|
||||
@@ -147,15 +156,18 @@ func run() error {
|
||||
return err
|
||||
}
|
||||
|
||||
// Resolve the review engine. The claude-code engine shells out to the
|
||||
// `claude` CLI (its own repo tools); every other spec is a majordomo model.
|
||||
// auto-selection and the delegate worker are majordomo-only — with
|
||||
// claude-code they're skipped (Claude Code does its own legwork).
|
||||
ccSpec := isClaudeCodeSpec(os.Getenv("GADFLY_MODEL"))
|
||||
// Resolve the review engine. The shell-out engines (claude-code, opencode)
|
||||
// bring their OWN repo tools; every other spec is an in-process majordomo
|
||||
// model. auto-selection and the delegate worker are majordomo-only — with a
|
||||
// shell-out engine they're skipped (the CLI does its own legwork).
|
||||
spec := os.Getenv("GADFLY_MODEL")
|
||||
var eng reviewEngine
|
||||
if ccSpec {
|
||||
eng = newClaudeCodeEngine(os.Getenv("GADFLY_MODEL"), fsTools.root)
|
||||
} else {
|
||||
switch {
|
||||
case isClaudeCodeSpec(spec):
|
||||
eng = newClaudeCodeEngine(spec, fsTools.root)
|
||||
case isOpenCodeSpec(spec):
|
||||
eng = newOpenCodeEngine(spec, fsTools.root)
|
||||
default:
|
||||
mdl, merr := resolveModel()
|
||||
if merr != nil {
|
||||
return fmt.Errorf("resolve model: %w", merr)
|
||||
@@ -187,13 +199,15 @@ func run() error {
|
||||
}
|
||||
|
||||
// Dynamic selection: a (cheap) model picks the lenses this diff needs.
|
||||
// Majordomo-only — the selector is an llm.Model.
|
||||
// Majordomo-only — the selector is an llm.Model, so a shell-out engine
|
||||
// (claude-code, opencode) can't provide one; fall back to the default suite.
|
||||
if auto {
|
||||
if ccSpec {
|
||||
fmt.Fprintln(os.Stderr, "gadfly: auto-select is not supported with the claude-code engine; using the default suite")
|
||||
md, ok := eng.(*majordomoEngine)
|
||||
if !ok {
|
||||
fmt.Fprintln(os.Stderr, "gadfly: auto-select requires an in-process model engine; using the default suite")
|
||||
specialists = suiteFromRegistry(registry, defaultSuite)
|
||||
} else {
|
||||
selector, serr := resolveSelectorModel(eng.(*majordomoEngine).mdl)
|
||||
selector, serr := resolveSelectorModel(md.mdl)
|
||||
if serr != nil {
|
||||
return fmt.Errorf("resolve selector model: %w", serr)
|
||||
}
|
||||
@@ -233,25 +247,34 @@ func run() error {
|
||||
|
||||
// runSpecialists reviews the diff through each lens and returns the results in
|
||||
// the SAME order as specialists, regardless of finish order. It uses executus's
|
||||
// fanout primitive: up to GADFLY_LENS_CONCURRENCY lenses run concurrently (the
|
||||
// default of 1 keeps the suite sequential, exactly as before), and fanout.Run
|
||||
// returns one result per lens in input order. Each lens already runs under its
|
||||
// own per-lens timeout (reviewWithSpecialist) and the lenses only read the
|
||||
// immutable repoFS, so concurrency simply overlaps independent passes.
|
||||
// fanout primitive to overlap independent lens passes (fanout.Run returns one
|
||||
// result per lens in input order); each lens runs under its own per-lens timeout
|
||||
// (reviewWithSpecialist) and the lenses only read the immutable repoFS.
|
||||
//
|
||||
// Caution: this fans out WITHIN one model. It multiplies with entrypoint.sh's
|
||||
// per-provider model concurrency, so total concurrent backend requests ≈
|
||||
// (models at once) × (lenses at once). To fan lenses out without oversubscribing
|
||||
// the backend, run models one at a time (provider lane cap 1) and raise this.
|
||||
// Throttling: when entrypoint.sh runs several of a provider's models at once it
|
||||
// seeds a shared lens-permit pool (activeLensSem) that every model's lenses draw
|
||||
// from, so the real cap is total lens passes in flight per PROVIDER — not
|
||||
// (models at once) × (lenses at once). Absent that pool (local runs, tests) the
|
||||
// in-process GADFLY_LENS_CONCURRENCY limit applies alone (default 1 = sequential).
|
||||
func runSpecialists(eng reviewEngine, base string, specialists []Specialist, task, diff string) []specialistResult {
|
||||
// Optional live status board: publishes this model's per-lens progress to a
|
||||
// file the entrypoint board renders. Inert (no-op) unless GADFLY_STATUS_FILE
|
||||
// is set, so plain runs are unaffected.
|
||||
sw := newStatusWriter(os.Getenv("GADFLY_MODEL"), modelProvider(), specialists)
|
||||
|
||||
// The cross-process pool (if any) is the real ceiling; size the in-process
|
||||
// fanout to it so a lone model in its lane can use the whole provider budget,
|
||||
// while extra goroutines simply block in sem.acquire until a permit frees.
|
||||
// Absent a pool, fall back to the in-process lens limit.
|
||||
sem := activeLensSem()
|
||||
maxConcurrent := lensConcurrency()
|
||||
if sem != nil {
|
||||
maxConcurrent = sem.size // the shared pool is the real ceiling
|
||||
}
|
||||
|
||||
fanResults := fanout.Run(context.Background(), specialists, fanout.Options[Specialist]{
|
||||
MaxConcurrent: lensConcurrency(),
|
||||
}, func(_ context.Context, sp Specialist) (res specialistResult, _ error) {
|
||||
MaxConcurrent: maxConcurrent,
|
||||
}, func(ctx context.Context, sp Specialist) (res specialistResult, _ error) {
|
||||
// A panic in one lens must not crash the whole binary (which would kill
|
||||
// every other lens's output) or leave this lens stuck at "running" on the
|
||||
// status board. fanout does not recover fn panics, so we do it here:
|
||||
@@ -262,6 +285,17 @@ func runSpecialists(eng reviewEngine, base string, specialists []Specialist, tas
|
||||
sw.set(sp.Name, lensFinished, "", true)
|
||||
}
|
||||
}()
|
||||
// Hold a provider-wide permit for this lens's whole review+recheck pass.
|
||||
// While waiting the lens stays "queued" on the board; cancellation before
|
||||
// a permit frees surfaces as a did-not-run lens rather than a leaked slot.
|
||||
if sem != nil {
|
||||
release, err := sem.acquire(ctx)
|
||||
if err != nil {
|
||||
sw.set(sp.Name, lensFinished, "", true)
|
||||
return specialistResult{spec: sp, out: fmt.Sprintf("⚠️ This reviewer did not run: %v", err), verdict: verdictUnknown, errored: true}, nil
|
||||
}
|
||||
defer release()
|
||||
}
|
||||
sw.set(sp.Name, lensRunning, "", false)
|
||||
out, errored := reviewWithSpecialist(eng, base, sp, task, diff)
|
||||
v := parseVerdict(out)
|
||||
@@ -284,14 +318,13 @@ func runSpecialists(eng reviewEngine, base string, specialists []Specialist, tas
|
||||
return results
|
||||
}
|
||||
|
||||
// lensConcurrency resolves how many specialist lenses run at once for THIS run's
|
||||
// model. It mirrors entrypoint.sh's per-provider MODEL concurrency: a
|
||||
// per-provider override in GADFLY_PROVIDER_LENS_CONCURRENCY ("provider=N,...")
|
||||
// wins for the model's provider, otherwise the GADFLY_LENS_CONCURRENCY scalar
|
||||
// (default 1). The provider is resolved by modelProvider() — the SAME lane rule
|
||||
// entrypoint uses for GADFLY_PROVIDER_CONCURRENCY — so e.g.
|
||||
// "ollama-cloud=3,m1=1" fans cloud lenses out while keeping a slow local box
|
||||
// serial, exactly the way the model map does for whole models.
|
||||
// lensConcurrency resolves the lens budget for THIS run's provider: a per-provider
|
||||
// override in GADFLY_PROVIDER_LENS_CONCURRENCY ("provider=N,...") wins for the
|
||||
// model's provider (resolved by modelProvider()), otherwise the
|
||||
// GADFLY_LENS_CONCURRENCY scalar (default 1). Under entrypoint.sh the SAME value
|
||||
// seeds the shared cross-process permit pool (activeLensSem), so it is the
|
||||
// provider-wide budget rather than a per-model one; standalone it caps the single
|
||||
// model's in-process fanout.
|
||||
func lensConcurrency() int {
|
||||
if n, ok := providerOverride("GADFLY_PROVIDER_LENS_CONCURRENCY", modelProvider()); ok {
|
||||
return n
|
||||
@@ -301,7 +334,7 @@ func lensConcurrency() int {
|
||||
|
||||
// providerOverride parses a "provider=N,provider=N" env map and returns the
|
||||
// value for provider when present and valid (>0). Mirrors entrypoint.sh's
|
||||
// provider_cap lookup so the two concurrency maps share one syntax.
|
||||
// provider_lens_cap lookup so the two share one syntax.
|
||||
func providerOverride(envName, provider string) (int, bool) {
|
||||
for _, item := range strings.Split(os.Getenv(envName), ",") {
|
||||
k, v, ok := strings.Cut(item, "=")
|
||||
|
||||
+2
-2
@@ -162,8 +162,8 @@ func buildSpec(provider, model string) string {
|
||||
// entrypoint.sh's provider_of: the segment before the first "/" in GADFLY_MODEL,
|
||||
// else GADFLY_PROVIDER, else the default (ollama-cloud). The binary reviews one
|
||||
// model per invocation, so this is that model's provider — used to resolve
|
||||
// per-provider policy (e.g. lens concurrency) against the SAME provider keys
|
||||
// entrypoint uses for GADFLY_PROVIDER_CONCURRENCY.
|
||||
// per-provider policy (e.g. the lens budget) against the SAME provider keys
|
||||
// entrypoint uses for GADFLY_PROVIDER_LENS_CONCURRENCY.
|
||||
func modelProvider() string {
|
||||
model := strings.TrimSpace(os.Getenv("GADFLY_MODEL"))
|
||||
if pfx, _, ok := strings.Cut(model, "/"); ok {
|
||||
|
||||
@@ -0,0 +1,273 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"os"
|
||||
"os/exec"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// openCodeEngine reviews by shelling out to the `opencode` CLI (opencode.ai) in
|
||||
// non-interactive `run` mode. Like the claude-code engine it is a pure shell-out
|
||||
// — OpenCode brings its OWN read tools (read/grep/glob/list) and reads the
|
||||
// checked-out tree, so findings are verified against real code — but it drives an
|
||||
// ollama-cloud model instead of a Claude subscription. The point is to benchmark
|
||||
// gadfly's boutique executus harness against a freely-available agentic harness
|
||||
// on the SAME models (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 the
|
||||
// OPENCODE_CONFIG_CONTENT env var (see config): a custom "gadfly" agent whose
|
||||
// prompt is the system prompt with edit/bash denied, plus a "gadfly" provider
|
||||
// pointing at ollama-cloud. OPENCODE_CONFIG_CONTENT is the highest-precedence
|
||||
// config source that matters in the container — it outranks any opencode.json a
|
||||
// reviewed repo might ship — so a repo can't re-enable edits on us.
|
||||
type openCodeEngine struct {
|
||||
bin string // CLI binary (GADFLY_OPENCODE_BIN, default "opencode")
|
||||
providerModel string // ollama model id for the generated "gadfly" provider ("" = none)
|
||||
modelRef string // --model value ("gadfly/<id>", a pass-through "<prov>/<id>", or "" = CLI default)
|
||||
baseURL string // ollama-cloud base URL (GADFLY_OPENCODE_BASE_URL)
|
||||
repoDir string // cwd for the CLI, so its tools read the checked-out tree
|
||||
extraArgs []string // appended verbatim (GADFLY_OPENCODE_EXTRA_ARGS)
|
||||
}
|
||||
|
||||
// openCodeProviderName / openCodeAgentName are the internal names of the provider
|
||||
// and agent gadfly generates in the injected config. "gadfly" won't collide with
|
||||
// OpenCode's models.dev provider registry.
|
||||
const (
|
||||
openCodeProviderName = "gadfly"
|
||||
openCodeAgentName = "gadfly"
|
||||
)
|
||||
|
||||
// defaultOpenCodeBaseURL is ollama-cloud's OpenAI-compatible endpoint.
|
||||
const defaultOpenCodeBaseURL = "https://ollama.com/v1"
|
||||
|
||||
// isOpenCodeSpec reports whether a GADFLY_MODEL spec selects the opencode engine:
|
||||
// the bare id "opencode"/"open-code" or an "opencode/<model>" form (both
|
||||
// spellings accepted; "opencode" is canonical).
|
||||
func isOpenCodeSpec(model string) bool {
|
||||
m := strings.TrimSpace(model)
|
||||
for _, p := range []string{"opencode", "open-code"} {
|
||||
if m == p || strings.HasPrefix(m, p+"/") {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// newOpenCodeEngine builds the engine from the GADFLY_MODEL spec and the optional
|
||||
// GADFLY_OPENCODE_* overrides. The part after the FIRST slash is the model, taken
|
||||
// verbatim — no ":"-suffix parsing, because ollama model ids legitimately contain
|
||||
// colons (e.g. "qwen3-coder:480b-cloud"). Three spec forms:
|
||||
//
|
||||
// opencode → bare: no --model, no generated provider (CLI default model)
|
||||
// opencode/<model> → wrap <model> in the generated ollama-cloud "gadfly" provider
|
||||
// opencode/<provider>/<model> → pass-through: --model <provider>/<model>, using OpenCode's
|
||||
// own provider registry/auth (escape hatch, no generated provider)
|
||||
//
|
||||
// GADFLY_OPENCODE_MODEL overrides the model taken from the spec (and is itself run
|
||||
// through the same slash logic). It does not verify the CLI is installed — a
|
||||
// missing binary surfaces as a normal pass error (advisory, never fatal).
|
||||
func newOpenCodeEngine(spec, repoDir string) *openCodeEngine {
|
||||
e := &openCodeEngine{
|
||||
bin: envOr("GADFLY_OPENCODE_BIN", "opencode"),
|
||||
baseURL: envOr("GADFLY_OPENCODE_BASE_URL", defaultOpenCodeBaseURL),
|
||||
repoDir: repoDir,
|
||||
extraArgs: strings.Fields(os.Getenv("GADFLY_OPENCODE_EXTRA_ARGS")),
|
||||
}
|
||||
var after string
|
||||
if _, a, ok := strings.Cut(strings.TrimSpace(spec), "/"); ok {
|
||||
after = strings.TrimSpace(a)
|
||||
}
|
||||
if env := strings.TrimSpace(os.Getenv("GADFLY_OPENCODE_MODEL")); env != "" {
|
||||
after = env
|
||||
}
|
||||
switch {
|
||||
case after == "":
|
||||
// bare spec: let OpenCode's configured default model apply.
|
||||
case strings.Contains(after, "/"):
|
||||
// "<provider>/<model>" pass-through to an OpenCode built-in provider.
|
||||
e.modelRef = after
|
||||
default:
|
||||
// A bare model id → serve it via the generated ollama-cloud provider.
|
||||
e.providerModel = after
|
||||
e.modelRef = openCodeProviderName + "/" + after
|
||||
}
|
||||
return e
|
||||
}
|
||||
|
||||
// args assembles the `opencode` argv for one pass. Factored out (and pure) so it
|
||||
// can be unit-tested without invoking the CLI. The task is the positional message
|
||||
// and MUST come last; it never begins with '-' (buildTask output starts with "PR
|
||||
// title:"/"Review …"), so no "--" terminator is needed. Note: in `opencode run`,
|
||||
// -p is basic-auth password, NOT the prompt — the message is positional.
|
||||
func (e *openCodeEngine) args(task string) []string {
|
||||
a := []string{"run", "--agent", openCodeAgentName}
|
||||
if e.modelRef != "" {
|
||||
a = append(a, "--model", e.modelRef)
|
||||
}
|
||||
a = append(a, e.extraArgs...)
|
||||
return append(a, task)
|
||||
}
|
||||
|
||||
// openCode config structs — the minimal shape gadfly generates. Marshaled to JSON
|
||||
// and handed to the CLI via OPENCODE_CONFIG_CONTENT.
|
||||
type openCodeConfig struct {
|
||||
Schema string `json:"$schema"`
|
||||
Permission openCodePermission `json:"permission"`
|
||||
Provider map[string]openCodeProvider `json:"provider,omitempty"`
|
||||
Agent map[string]openCodeAgent `json:"agent"`
|
||||
}
|
||||
|
||||
// openCodePermission is OpenCode's permission map: tool key → "allow"|"ask"|"deny".
|
||||
type openCodePermission map[string]string
|
||||
|
||||
// denyMutations denies every OpenCode permission that could change the repo, run
|
||||
// commands, or reach the network / the filesystem outside the checked-out tree —
|
||||
// keeping the reviewer strictly read-only. The read/search tools OpenCode gates
|
||||
// separately (read/glob/grep/list/lsp) stay at their default so the agent can
|
||||
// still verify findings against the code. OpenCode's permission keys are
|
||||
// enumerated at https://opencode.ai/docs/agents; a key it doesn't recognize is
|
||||
// simply ignored, so listing extras is safe.
|
||||
func denyMutations() openCodePermission {
|
||||
return openCodePermission{
|
||||
"edit": "deny",
|
||||
"bash": "deny",
|
||||
"webfetch": "deny",
|
||||
"websearch": "deny",
|
||||
"external_directory": "deny",
|
||||
}
|
||||
}
|
||||
|
||||
type openCodeProvider struct {
|
||||
NPM string `json:"npm"`
|
||||
Name string `json:"name"`
|
||||
Options openCodeProviderOptions `json:"options"`
|
||||
Models map[string]struct{} `json:"models"`
|
||||
}
|
||||
|
||||
type openCodeProviderOptions struct {
|
||||
BaseURL string `json:"baseURL"`
|
||||
APIKey string `json:"apiKey"`
|
||||
}
|
||||
|
||||
type openCodeAgent struct {
|
||||
Description string `json:"description"`
|
||||
Mode string `json:"mode"`
|
||||
Prompt string `json:"prompt"`
|
||||
Permission openCodePermission `json:"permission"`
|
||||
}
|
||||
|
||||
// config builds the OpenCode config JSON for one pass. The system prompt becomes
|
||||
// the "gadfly" agent's prompt; the mutating/network tools are denied at BOTH the
|
||||
// global and agent level (defense in depth — OpenCode's read tools stay
|
||||
// available). For a bare or pass-through spec the generated ollama-cloud provider
|
||||
// block is omitted. The API key is always the "{env:OLLAMA_API_KEY}" reference,
|
||||
// never a literal secret baked into the config.
|
||||
func (e *openCodeEngine) config(system string) ([]byte, error) {
|
||||
deny := denyMutations()
|
||||
cfg := openCodeConfig{
|
||||
Schema: "https://opencode.ai/config.json",
|
||||
Permission: deny,
|
||||
Agent: map[string]openCodeAgent{
|
||||
openCodeAgentName: {
|
||||
Description: "Gadfly adversarial code-review lens (read-only).",
|
||||
Mode: "primary",
|
||||
Prompt: system,
|
||||
Permission: deny,
|
||||
},
|
||||
},
|
||||
}
|
||||
if e.providerModel != "" {
|
||||
cfg.Provider = map[string]openCodeProvider{
|
||||
openCodeProviderName: {
|
||||
NPM: "@ai-sdk/openai-compatible",
|
||||
Name: openCodeProviderName,
|
||||
Options: openCodeProviderOptions{
|
||||
BaseURL: e.baseURL,
|
||||
APIKey: "{env:OLLAMA_API_KEY}",
|
||||
},
|
||||
Models: map[string]struct{}{e.providerModel: {}},
|
||||
},
|
||||
}
|
||||
}
|
||||
return json.Marshal(cfg)
|
||||
}
|
||||
|
||||
func (e *openCodeEngine) runPass(ctx context.Context, system, task string, _ int) (string, error) {
|
||||
cfg, err := e.config(system)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("opencode config: %w", err)
|
||||
}
|
||||
cmd := exec.CommandContext(ctx, e.bin, e.args(task)...)
|
||||
cmd.Dir = e.repoDir
|
||||
// Inject the review config (system prompt as the agent prompt + read-only
|
||||
// permissions + the ollama-cloud provider) via OPENCODE_CONFIG_CONTENT, which
|
||||
// outranks any opencode.json the reviewed repo itself ships. NO_COLOR keeps the
|
||||
// captured stdout free of ANSI decoration.
|
||||
cmd.Env = append(openCodeEnv(), "OPENCODE_CONFIG_CONTENT="+string(cfg), "NO_COLOR=1")
|
||||
killGroupOnCancel(cmd) // don't orphan the CLI's Node children on a timed-out lens
|
||||
var stdout, stderr bytes.Buffer
|
||||
cmd.Stdout = &stdout
|
||||
cmd.Stderr = &stderr
|
||||
|
||||
runErr := cmd.Run()
|
||||
|
||||
// A cancelled/timed-out run must surface as an error, never as whatever partial
|
||||
// bytes the CLI flushed before it was killed.
|
||||
if ctx.Err() != nil {
|
||||
return "", fmt.Errorf("opencode run %v", ctx.Err())
|
||||
}
|
||||
if runErr != nil {
|
||||
detail := truncateForErr(stderr.String())
|
||||
if detail == "" {
|
||||
detail = truncateForErr(stdout.String())
|
||||
}
|
||||
if detail != "" {
|
||||
return "", fmt.Errorf("opencode run failed: %v: %s", runErr, detail)
|
||||
}
|
||||
return "", fmt.Errorf("opencode run failed: %v", runErr)
|
||||
}
|
||||
// OpenCode's default (non-JSON) format prints the assistant's final text; trust
|
||||
// it as the review. An empty result on a clean exit is an error, never "".
|
||||
if out := strings.TrimSpace(stdout.String()); out != "" {
|
||||
return out, nil
|
||||
}
|
||||
return "", fmt.Errorf("opencode run returned no output")
|
||||
}
|
||||
|
||||
// openCodeEnv builds a minimal environment for the `opencode` subprocess. It
|
||||
// forwards what the CLI needs to reach a model provider: OLLAMA_API_KEY for the
|
||||
// generated ollama-cloud provider (the primary opencode/<model> path), PLUS the
|
||||
// standard provider keys — ANTHROPIC_*, OPENAI_*, GOOGLE_*, GEMINI_* — so the
|
||||
// opencode/<provider>/<model> pass-through form can authenticate against
|
||||
// OpenCode's own built-in providers (those keys are otherwise stripped, which
|
||||
// broke the documented escape hatch). It still withholds gadfly's OWN secrets —
|
||||
// GITEA_TOKEN, GADFLY_API_KEY, GADFLY_FINDINGS_TOKEN, and the claude-code
|
||||
// subscription token (CLAUDE_CODE_OAUTH_TOKEN, which OpenCode can't use anyway) —
|
||||
// so they never reach the third-party CLI. OPENCODE_CONFIG_CONTENT is never
|
||||
// inherited: runPass sets it, and a duplicate key would be ambiguous (getenv
|
||||
// returns the first occurrence).
|
||||
func openCodeEnv() []string {
|
||||
return filterEnv(func(k string) bool {
|
||||
if k == "OPENCODE_CONFIG_CONTENT" {
|
||||
return false // set explicitly by runPass; never inherit a competing value
|
||||
}
|
||||
switch k {
|
||||
case "PATH", "HOME", "USER", "LOGNAME", "TMPDIR", "LANG", "TERM", "SHELL", "OLLAMA_API_KEY":
|
||||
return true
|
||||
}
|
||||
return strings.HasPrefix(k, "LC_") ||
|
||||
strings.HasPrefix(k, "OPENCODE_") ||
|
||||
strings.HasPrefix(k, "GADFLY_OPENCODE_") ||
|
||||
strings.HasPrefix(k, "NODE_") ||
|
||||
strings.HasPrefix(k, "XDG_") ||
|
||||
strings.HasPrefix(k, "ANTHROPIC_") ||
|
||||
strings.HasPrefix(k, "OPENAI_") ||
|
||||
strings.HasPrefix(k, "GOOGLE_") ||
|
||||
strings.HasPrefix(k, "GEMINI_")
|
||||
})
|
||||
}
|
||||
@@ -0,0 +1,303 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"os"
|
||||
"slices"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestIsOpenCodeSpec(t *testing.T) {
|
||||
cases := map[string]bool{
|
||||
"opencode": true,
|
||||
"opencode/glm-5.2": true,
|
||||
"open-code/glm-5.2": true, // accepted alias spelling
|
||||
"opencode/qwen3-coder:480b-cloud": true, // colon-bearing model id
|
||||
"opencode/anthropic/claude": true, // pass-through form
|
||||
" opencode ": true, // trimmed
|
||||
"opencode-extra": false, // not the bare id, not a "/" form
|
||||
"qwen3-coder:480b-cloud": false,
|
||||
"claude-code/opus": false,
|
||||
"": false,
|
||||
}
|
||||
for spec, want := range cases {
|
||||
if got := isOpenCodeSpec(spec); got != want {
|
||||
t.Errorf("isOpenCodeSpec(%q) = %v, want %v", spec, got, want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestNewOpenCodeEngineModel(t *testing.T) {
|
||||
t.Setenv("GADFLY_OPENCODE_MODEL", "")
|
||||
|
||||
// "opencode/<model>" → wrapped in the generated "gadfly" provider.
|
||||
if e := newOpenCodeEngine("opencode/glm-5.2", "/repo"); e.providerModel != "glm-5.2" || e.modelRef != "gadfly/glm-5.2" {
|
||||
t.Errorf("glm-5.2: providerModel=%q modelRef=%q, want glm-5.2 / gadfly/glm-5.2", e.providerModel, e.modelRef)
|
||||
}
|
||||
// Colon-bearing ollama id is preserved verbatim — NOT split on ":".
|
||||
if e := newOpenCodeEngine("opencode/qwen3-coder:480b-cloud", "/repo"); e.providerModel != "qwen3-coder:480b-cloud" {
|
||||
t.Errorf("colon id: providerModel=%q, want qwen3-coder:480b-cloud (no split)", e.providerModel)
|
||||
}
|
||||
// "open-code/" spelling behaves identically.
|
||||
if e := newOpenCodeEngine("open-code/glm-5.2", "/repo"); e.modelRef != "gadfly/glm-5.2" {
|
||||
t.Errorf("open-code alias: modelRef=%q, want gadfly/glm-5.2", e.modelRef)
|
||||
}
|
||||
// Pass-through "opencode/<provider>/<model>" → no generated provider.
|
||||
if e := newOpenCodeEngine("opencode/anthropic/claude-sonnet-4-6", "/repo"); e.providerModel != "" || e.modelRef != "anthropic/claude-sonnet-4-6" {
|
||||
t.Errorf("pass-through: providerModel=%q modelRef=%q, want '' / anthropic/claude-sonnet-4-6", e.providerModel, e.modelRef)
|
||||
}
|
||||
// Bare spec → no model, no provider (CLI default applies).
|
||||
if e := newOpenCodeEngine("opencode", "/repo"); e.providerModel != "" || e.modelRef != "" {
|
||||
t.Errorf("bare: providerModel=%q modelRef=%q, want both empty", e.providerModel, e.modelRef)
|
||||
}
|
||||
// GADFLY_OPENCODE_MODEL overrides the spec suffix.
|
||||
t.Setenv("GADFLY_OPENCODE_MODEL", "deepseek-v3")
|
||||
if e := newOpenCodeEngine("opencode/glm-5.2", "/repo"); e.providerModel != "deepseek-v3" || e.modelRef != "gadfly/deepseek-v3" {
|
||||
t.Errorf("env override: providerModel=%q modelRef=%q, want deepseek-v3 / gadfly/deepseek-v3", e.providerModel, e.modelRef)
|
||||
}
|
||||
}
|
||||
|
||||
func TestOpenCodeEngineDefaults(t *testing.T) {
|
||||
t.Setenv("GADFLY_OPENCODE_BIN", "")
|
||||
t.Setenv("GADFLY_OPENCODE_BASE_URL", "")
|
||||
t.Setenv("GADFLY_OPENCODE_EXTRA_ARGS", "")
|
||||
e := newOpenCodeEngine("opencode/glm-5.2", "/repo")
|
||||
if e.bin != "opencode" {
|
||||
t.Errorf("bin = %q, want opencode", e.bin)
|
||||
}
|
||||
if e.baseURL != defaultOpenCodeBaseURL {
|
||||
t.Errorf("baseURL = %q, want %q", e.baseURL, defaultOpenCodeBaseURL)
|
||||
}
|
||||
if e.repoDir != "/repo" {
|
||||
t.Errorf("repoDir = %q, want /repo", e.repoDir)
|
||||
}
|
||||
}
|
||||
|
||||
func TestOpenCodeArgs(t *testing.T) {
|
||||
t.Setenv("GADFLY_OPENCODE_MODEL", "")
|
||||
t.Setenv("GADFLY_OPENCODE_EXTRA_ARGS", "--variant reasoning")
|
||||
e := newOpenCodeEngine("opencode/glm-5.2", "/repo")
|
||||
args := e.args("TASK-PROMPT")
|
||||
|
||||
// "run" is the subcommand and must be first.
|
||||
if len(args) == 0 || args[0] != "run" {
|
||||
t.Fatalf("args[0] = %q, want run (args=%v)", args, args)
|
||||
}
|
||||
if argAfter(args, "--agent") != openCodeAgentName {
|
||||
t.Errorf("--agent = %q, want %q", argAfter(args, "--agent"), openCodeAgentName)
|
||||
}
|
||||
if argAfter(args, "--model") != "gadfly/glm-5.2" {
|
||||
t.Errorf("--model = %q, want gadfly/glm-5.2", argAfter(args, "--model"))
|
||||
}
|
||||
// extra args appended verbatim (split on whitespace).
|
||||
if !strings.Contains(strings.Join(args, " "), "--variant reasoning") {
|
||||
t.Errorf("extra args not appended: %v", args)
|
||||
}
|
||||
// task is the positional message and must be LAST.
|
||||
if args[len(args)-1] != "TASK-PROMPT" {
|
||||
t.Errorf("last arg = %q, want TASK-PROMPT (args=%v)", args[len(args)-1], args)
|
||||
}
|
||||
}
|
||||
|
||||
func TestOpenCodeArgsBareModelOmitsFlag(t *testing.T) {
|
||||
t.Setenv("GADFLY_OPENCODE_MODEL", "")
|
||||
t.Setenv("GADFLY_OPENCODE_EXTRA_ARGS", "")
|
||||
e := newOpenCodeEngine("opencode", "/repo")
|
||||
args := e.args("t")
|
||||
if slices.Contains(args, "--model") {
|
||||
t.Errorf("--model should be omitted for a bare opencode spec: %v", args)
|
||||
}
|
||||
if args[len(args)-1] != "t" {
|
||||
t.Errorf("last arg = %q, want t", args[len(args)-1])
|
||||
}
|
||||
}
|
||||
|
||||
func TestOpenCodeConfig(t *testing.T) {
|
||||
t.Setenv("GADFLY_OPENCODE_MODEL", "")
|
||||
t.Setenv("GADFLY_OPENCODE_BASE_URL", "")
|
||||
|
||||
// Round-trip a system prompt containing quotes and newlines.
|
||||
sys := "Line one with \"quotes\".\nLine two."
|
||||
e := newOpenCodeEngine("opencode/glm-5.2", "/repo")
|
||||
raw, err := e.config(sys)
|
||||
if err != nil {
|
||||
t.Fatalf("config: %v", err)
|
||||
}
|
||||
var cfg openCodeConfig
|
||||
if err := json.Unmarshal(raw, &cfg); err != nil {
|
||||
t.Fatalf("generated config is not valid JSON: %v\n%s", err, raw)
|
||||
}
|
||||
|
||||
// Agent carries the system prompt verbatim and denies edit+bash.
|
||||
ag, ok := cfg.Agent[openCodeAgentName]
|
||||
if !ok {
|
||||
t.Fatalf("agent %q missing from config", openCodeAgentName)
|
||||
}
|
||||
if ag.Prompt != sys {
|
||||
t.Errorf("agent prompt = %q, want it to round-trip the system prompt", ag.Prompt)
|
||||
}
|
||||
// Mutating/network tools are denied at BOTH the agent and global level (defense
|
||||
// in depth); the read/search tools stay at OpenCode's default.
|
||||
for _, k := range []string{"edit", "bash", "webfetch", "websearch", "external_directory"} {
|
||||
if ag.Permission[k] != "deny" {
|
||||
t.Errorf("agent permission[%q] = %q, want deny", k, ag.Permission[k])
|
||||
}
|
||||
if cfg.Permission[k] != "deny" {
|
||||
t.Errorf("global permission[%q] = %q, want deny", k, cfg.Permission[k])
|
||||
}
|
||||
}
|
||||
|
||||
// Provider block: correct npm, default baseURL, env-ref apiKey, model in map.
|
||||
prov, ok := cfg.Provider[openCodeProviderName]
|
||||
if !ok {
|
||||
t.Fatalf("provider %q missing from config", openCodeProviderName)
|
||||
}
|
||||
if prov.NPM != "@ai-sdk/openai-compatible" {
|
||||
t.Errorf("provider npm = %q, want @ai-sdk/openai-compatible", prov.NPM)
|
||||
}
|
||||
if prov.Options.BaseURL != defaultOpenCodeBaseURL {
|
||||
t.Errorf("provider baseURL = %q, want %q", prov.Options.BaseURL, defaultOpenCodeBaseURL)
|
||||
}
|
||||
if prov.Options.APIKey != "{env:OLLAMA_API_KEY}" {
|
||||
t.Errorf("provider apiKey = %q, want {env:OLLAMA_API_KEY} (never a literal secret)", prov.Options.APIKey)
|
||||
}
|
||||
if _, ok := prov.Models["glm-5.2"]; !ok {
|
||||
t.Errorf("provider models = %v, want it to contain glm-5.2", prov.Models)
|
||||
}
|
||||
|
||||
// GADFLY_OPENCODE_BASE_URL override reaches the provider.
|
||||
t.Setenv("GADFLY_OPENCODE_BASE_URL", "http://localhost:11434/v1")
|
||||
e2 := newOpenCodeEngine("opencode/glm-5.2", "/repo")
|
||||
raw2, err := e2.config(sys)
|
||||
if err != nil {
|
||||
t.Fatalf("config override: %v", err)
|
||||
}
|
||||
var cfg2 openCodeConfig
|
||||
if err := json.Unmarshal(raw2, &cfg2); err != nil {
|
||||
t.Fatalf("override config invalid JSON: %v", err)
|
||||
}
|
||||
if got := cfg2.Provider[openCodeProviderName].Options.BaseURL; got != "http://localhost:11434/v1" {
|
||||
t.Errorf("override baseURL = %q, want http://localhost:11434/v1", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestOpenCodeConfigNoProviderForPassThroughAndBare(t *testing.T) {
|
||||
t.Setenv("GADFLY_OPENCODE_MODEL", "")
|
||||
for _, spec := range []string{"opencode", "opencode/anthropic/claude-sonnet-4-6"} {
|
||||
e := newOpenCodeEngine(spec, "/repo")
|
||||
raw, err := e.config("sys")
|
||||
if err != nil {
|
||||
t.Fatalf("config(%q): %v", spec, err)
|
||||
}
|
||||
if strings.Contains(string(raw), "\"provider\"") {
|
||||
t.Errorf("spec %q: config should omit the provider block, got %s", spec, raw)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestOpenCodeEnvFilters(t *testing.T) {
|
||||
t.Setenv("GITEA_TOKEN", "secret-gitea")
|
||||
t.Setenv("OLLAMA_API_KEY", "keep-ollama")
|
||||
t.Setenv("GADFLY_API_KEY", "secret-gadfly")
|
||||
t.Setenv("GADFLY_FINDINGS_TOKEN", "secret-findings")
|
||||
t.Setenv("ANTHROPIC_API_KEY", "keep-anthropic") // pass-through provider auth
|
||||
t.Setenv("OPENAI_API_KEY", "keep-openai") // pass-through provider auth
|
||||
t.Setenv("CLAUDE_CODE_OAUTH_TOKEN", "secret-claude")
|
||||
t.Setenv("GADFLY_OPENCODE_MODEL", "keep-knob")
|
||||
t.Setenv("OPENCODE_CONFIG_CONTENT", "should-not-inherit")
|
||||
|
||||
env := openCodeEnv()
|
||||
has := func(k string) bool {
|
||||
for _, kv := range env {
|
||||
if strings.HasPrefix(kv, k+"=") {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
// kept: the ollama key + the standard provider keys the opencode/<provider>/<model>
|
||||
// pass-through form needs + opencode knobs + PATH
|
||||
for _, k := range []string{"OLLAMA_API_KEY", "ANTHROPIC_API_KEY", "OPENAI_API_KEY", "GADFLY_OPENCODE_MODEL", "PATH"} {
|
||||
if !has(k) {
|
||||
t.Errorf("openCodeEnv dropped %s, but it should be kept", k)
|
||||
}
|
||||
}
|
||||
// dropped: gadfly's own secrets + the claude engine's subscription token
|
||||
// (OpenCode's anthropic provider uses ANTHROPIC_API_KEY, not this OAuth token).
|
||||
for _, k := range []string{"GITEA_TOKEN", "GADFLY_API_KEY", "GADFLY_FINDINGS_TOKEN", "CLAUDE_CODE_OAUTH_TOKEN"} {
|
||||
if has(k) {
|
||||
t.Errorf("openCodeEnv leaked %s into the subprocess env", k)
|
||||
}
|
||||
}
|
||||
// OPENCODE_CONFIG_CONTENT must NOT be inherited — runPass sets it, and a
|
||||
// duplicate key would be ambiguous.
|
||||
if has("OPENCODE_CONFIG_CONTENT") {
|
||||
t.Errorf("openCodeEnv inherited OPENCODE_CONFIG_CONTENT; runPass sets it explicitly")
|
||||
}
|
||||
}
|
||||
|
||||
// stubOpenCode writes an executable shell stub that prints body and exits code,
|
||||
// and returns an engine pointed at it.
|
||||
func stubOpenCode(t *testing.T, body string, code int) *openCodeEngine {
|
||||
t.Helper()
|
||||
dir := t.TempDir()
|
||||
path := dir + "/opencode-stub.sh"
|
||||
script := "#!/bin/sh\nprintf '%s' " + shSingleQuote(body) + "\nexit " + itoa(code) + "\n"
|
||||
if err := os.WriteFile(path, []byte(script), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return &openCodeEngine{bin: path, repoDir: dir}
|
||||
}
|
||||
|
||||
func TestOpenCodeRunPassCleanResult(t *testing.T) {
|
||||
e := stubOpenCode(t, " REVIEW TEXT ", 0)
|
||||
out, err := e.runPass(context.Background(), "sys", "task", 0)
|
||||
if err != nil || out != "REVIEW TEXT" {
|
||||
t.Fatalf("clean result: got (%q, %v), want (REVIEW TEXT, nil)", out, err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestOpenCodeRunPassEmptyIsError(t *testing.T) {
|
||||
e := stubOpenCode(t, " ", 0)
|
||||
out, err := e.runPass(context.Background(), "sys", "task", 0)
|
||||
if err == nil {
|
||||
t.Fatalf("empty output should be an error, got out=%q", out)
|
||||
}
|
||||
}
|
||||
|
||||
func TestOpenCodeRunPassNonZero(t *testing.T) {
|
||||
e := stubOpenCode(t, "fatal: provider auth failed", 1)
|
||||
_, err := e.runPass(context.Background(), "sys", "task", 0)
|
||||
if err == nil || !strings.Contains(err.Error(), "opencode run failed") {
|
||||
t.Fatalf("non-zero exit should error with detail, got %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestOpenCodeRunPassInjectsConfig proves the end-to-end env plumbing: the stub
|
||||
// echoes OPENCODE_CONFIG_CONTENT back, and the emitted JSON must carry the exact
|
||||
// system prompt as the gadfly agent's prompt.
|
||||
func TestOpenCodeRunPassInjectsConfig(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
stub := dir + "/opencode-stub.sh"
|
||||
script := "#!/bin/sh\nprintf '%s' \"$OPENCODE_CONFIG_CONTENT\"\n"
|
||||
if err := os.WriteFile(stub, []byte(script), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
e := newOpenCodeEngine("opencode/glm-5.2", dir)
|
||||
e.bin = stub
|
||||
|
||||
sys := "SYSTEM-PROMPT-SENTINEL\nwith a second line"
|
||||
out, err := e.runPass(context.Background(), sys, "task", 0)
|
||||
if err != nil {
|
||||
t.Fatalf("runPass: %v", err)
|
||||
}
|
||||
var cfg openCodeConfig
|
||||
if err := json.Unmarshal([]byte(out), &cfg); err != nil {
|
||||
t.Fatalf("injected config is not valid JSON: %v\n%s", err, out)
|
||||
}
|
||||
if got := cfg.Agent[openCodeAgentName].Prompt; got != sys {
|
||||
t.Errorf("injected agent prompt = %q, want the system prompt", got)
|
||||
}
|
||||
}
|
||||
+48
-28
@@ -39,6 +39,10 @@
|
||||
# CLAUDE_CODE_OAUTH_TOKEN auth for the claude-code engine (GADFLY_MODELS entry
|
||||
# "claude-code"/"claude-code/<model>"); Pro/Max subscription
|
||||
# token from `claude setup-token`. Else ANTHROPIC_API_KEY.
|
||||
# OLLAMA_CLOUD_API_KEY also feeds the opencode engine (GADFLY_MODELS entry
|
||||
# "opencode/<model>"): the bundled `opencode` CLI drives
|
||||
# that ollama-cloud model, for benchmarking the two
|
||||
# harnesses on the same model. Tune via GADFLY_OPENCODE_*.
|
||||
# GADFLY_TRIGGER_PHRASE comment phrase that triggers a re-review (default "@gadfly review")
|
||||
# GADFLY_ALLOWED_USERS comma-separated usernames allowed to comment-trigger;
|
||||
# empty => fall back to "is a repo collaborator"
|
||||
@@ -194,14 +198,18 @@ export GADFLY_FINDINGS_TOKEN="${GADFLY_FINDINGS_TOKEN:-}"
|
||||
# provider key envs (OPENAI_API_KEY, …) are inherited by run.sh and the binary.
|
||||
#
|
||||
# Concurrency: each PROVIDER is its own lane and lanes run in PARALLEL, so a fast
|
||||
# cloud provider isn't stuck behind a slow local box. Within a lane, at most
|
||||
# `cap` models run at once. cap = GADFLY_PROVIDER_CONCURRENCY's "provider=N"
|
||||
# entry, else GADFLY_CONCURRENCY (default 1). A model's provider is the spec's
|
||||
# first path segment ("m1pro/qwen3.6:35b-mlx" -> m1pro), or GADFLY_PROVIDER /
|
||||
# ollama-cloud for a bare id. Default (cap 1) keeps a single-provider pool fully
|
||||
# sequential, exactly as before.
|
||||
# cloud provider isn't stuck behind a slow local box. Within a lane ALL of the
|
||||
# provider's models run at once; the real throttle is a single PROVIDER-WIDE lens
|
||||
# budget (a shared permit pool, seeded per lane) that every model's lenses draw
|
||||
# from — so total lens passes in flight per provider is bounded, but a model
|
||||
# winding down to its last lens immediately yields its freed permits to another
|
||||
# model's queued lenses instead of holding a whole "model slot" (the old
|
||||
# GADFLY_PROVIDER_CONCURRENCY model cap, now removed, caused that tail stall). The
|
||||
# budget = GADFLY_PROVIDER_LENS_CONCURRENCY's "provider=N" entry, else
|
||||
# GADFLY_LENS_CONCURRENCY (default 1). A model's provider is the spec's first path
|
||||
# segment ("m1pro/qwen3.6:35b-mlx" -> m1pro), or GADFLY_PROVIDER / ollama-cloud
|
||||
# for a bare id.
|
||||
MODELS="${GADFLY_MODELS:-${OLLAMA_REVIEW_MODELS:-$DEFAULT_MODELS}}"
|
||||
DEFAULT_CONC="${GADFLY_CONCURRENCY:-1}"
|
||||
|
||||
# --- huge-PR downshift ------------------------------------------------------
|
||||
# A very large diff is what burns the model budget: every review step re-sends
|
||||
@@ -241,24 +249,33 @@ provider_of() { case "$1" in */*) echo "${1%%/*}";; *) echo "${GADFLY_PROVIDER:-
|
||||
STATUS_DIR="${WORKDIR}/status"
|
||||
status_file_for() { echo "${STATUS_DIR}/$(echo "$1" | tr -c '[:alnum:]._-' '_').json"; }
|
||||
|
||||
provider_cap() { # provider -> concurrency (override map "p=N,...", else default)
|
||||
# Root of the per-provider lens permit pools (one subdir per lane, seeded by
|
||||
# run_lane). Cleared up front so a reused WORKDIR can't leak stale permit files.
|
||||
LENS_SEM_ROOT="${WORKDIR}/lenssem"
|
||||
rm -rf "$LENS_SEM_ROOT" 2>/dev/null || true
|
||||
|
||||
provider_lens_cap() { # provider -> provider-wide lens budget (permit-pool size)
|
||||
local p="$1" item k v
|
||||
IFS=',' read -ra _caps <<< "${GADFLY_PROVIDER_CONCURRENCY:-}"
|
||||
for item in "${_caps[@]}"; do
|
||||
IFS=',' read -ra _lcaps <<< "${GADFLY_PROVIDER_LENS_CONCURRENCY:-}"
|
||||
for item in "${_lcaps[@]}"; do
|
||||
k="$(echo "${item%%=*}" | tr -d '[:space:]')"
|
||||
v="$(echo "${item#*=}" | tr -d '[:space:]')"
|
||||
if [ "$k" = "$p" ] && [ -n "$v" ]; then echo "$v"; return; fi
|
||||
done
|
||||
echo "$DEFAULT_CONC"
|
||||
echo "${GADFLY_LENS_CONCURRENCY:-1}"
|
||||
}
|
||||
|
||||
review_one() {
|
||||
local sf="" ff=""
|
||||
[ "${GADFLY_STATUS_BOARD:-1}" != "0" ] && sf="$(status_file_for "$1")"
|
||||
[ "$CONSOLIDATE" = "1" ] && ff="$(findings_file_for "$1")"
|
||||
PROVIDER=ollama MODEL="$1" GADFLY_BIN="/usr/local/bin/gadfly" GADFLY_REPO_DIR="$REPO_DIR" \
|
||||
local m="$1" sem_dir="${2:-}" sem_size="${3:-}" sf="" ff=""
|
||||
[ "${GADFLY_STATUS_BOARD:-1}" != "0" ] && sf="$(status_file_for "$m")"
|
||||
[ "$CONSOLIDATE" = "1" ] && ff="$(findings_file_for "$m")"
|
||||
# GADFLY_LENS_SEM_DIR/_SIZE point the binary at this provider's shared lens
|
||||
# permit pool (empty => the binary just uses its in-process lens limit). These
|
||||
# are inherited by the binary through run.sh's environment.
|
||||
PROVIDER=ollama MODEL="$m" GADFLY_BIN="/usr/local/bin/gadfly" GADFLY_REPO_DIR="$REPO_DIR" \
|
||||
GADFLY_STATUS_FILE="$sf" GADFLY_FINDINGS_OUT="$ff" GADFLY_CONSOLIDATE="$CONSOLIDATE" \
|
||||
bash "${SCRIPTS_DIR}/run.sh" || log "model $1 failed (continuing)"
|
||||
GADFLY_LENS_SEM_DIR="$sem_dir" GADFLY_LENS_SEM_SIZE="$sem_size" \
|
||||
bash "${SCRIPTS_DIR}/run.sh" || log "model $m failed (continuing)"
|
||||
# If the binary never wrote real status (run.sh skipped it: empty diff, no key,
|
||||
# binary missing), the pre-seed stays {started:0, done:false} and the board
|
||||
# would show this model "waiting to start" forever and never reach N/N. Mark
|
||||
@@ -309,16 +326,19 @@ for m in "${MODEL_LIST[@]}"; do
|
||||
case " $PROVIDERS " in *" $p "*) ;; *) PROVIDERS="${PROVIDERS}${PROVIDERS:+ }$p" ;; esac
|
||||
done
|
||||
|
||||
run_lane() { # $1=provider: run its models, at most `cap` at a time
|
||||
local p="$1" cap inflight=0 m
|
||||
cap="$(provider_cap "$p")"; [ "$cap" -ge 1 ] 2>/dev/null || cap=1
|
||||
run_lane() { # $1=provider: run ALL its models at once, throttled only by a shared
|
||||
# provider-wide lens permit pool (no per-model cap).
|
||||
local p="$1" budget sem_dir m
|
||||
budget="$(provider_lens_cap "$p")"; [ "$budget" -ge 1 ] 2>/dev/null || budget=1
|
||||
local mine=()
|
||||
for m in "${MODEL_LIST[@]}"; do [ "$(provider_of "$m")" = "$p" ] && mine+=("$m"); done
|
||||
log "lane ${p}: cap ${cap}; models: ${mine[*]}"
|
||||
# Seed this provider's lens permit pool: a directory the binary flocks N permit
|
||||
# files in (created lazily), one shared budget across every model in the lane.
|
||||
sem_dir="${LENS_SEM_ROOT}/$(echo "$p" | tr -c '[:alnum:]._-' '_')"
|
||||
mkdir -p "$sem_dir"
|
||||
log "lane ${p}: lens budget ${budget} shared across ${#mine[@]} model(s): ${mine[*]}"
|
||||
for m in "${mine[@]}"; do
|
||||
review_one "$m" &
|
||||
inflight=$((inflight+1))
|
||||
if [ "$inflight" -ge "$cap" ]; then wait -n 2>/dev/null || wait; inflight=$((inflight-1)); fi
|
||||
review_one "$m" "$sem_dir" "$budget" &
|
||||
done
|
||||
wait
|
||||
}
|
||||
@@ -334,8 +354,8 @@ BOARD_PID=""
|
||||
if [ "${GADFLY_STATUS_BOARD:-1}" != "0" ]; then
|
||||
rm -rf "$STATUS_DIR"; mkdir -p "$STATUS_DIR"
|
||||
# Pre-seed every model as queued so the board shows the full swarm from t=0,
|
||||
# even models still waiting on their provider lane's concurrency cap. Each
|
||||
# binary overwrites its own file with real per-lens detail once it starts.
|
||||
# even models whose lenses are still waiting on their provider's lens budget.
|
||||
# Each binary overwrites its own file with real per-lens detail once it starts.
|
||||
for m in "${MODEL_LIST[@]}"; do
|
||||
jq -n --arg model "$m" --arg provider "$(provider_of "$m")" \
|
||||
'{model:$model, provider:$provider, started:0, updated:0, done:false, lenses:[]}' \
|
||||
@@ -374,9 +394,9 @@ if [ "${GADFLY_PR_BUDGET_SECS:-0}" -gt 0 ] 2>/dev/null; then
|
||||
fi
|
||||
|
||||
log "providers: ${PROVIDERS:-none}"
|
||||
# Each provider lane runs in parallel; cap is enforced within each lane. Track
|
||||
# the lane PIDs so we wait ONLY for the review work — not the status board,
|
||||
# which intentionally runs until we signal it below.
|
||||
# Each provider lane runs in parallel; the shared lens budget throttles within
|
||||
# each lane. Track the lane PIDs so we wait ONLY for the review work — not the
|
||||
# status board, which intentionally runs until we signal it below.
|
||||
LANE_PIDS=()
|
||||
for p in $PROVIDERS; do
|
||||
run_lane "$p" &
|
||||
|
||||
@@ -12,6 +12,7 @@ set the secrets/vars it references. Gadfly is advisory only — it never blocks
|
||||
| [`openai-compatible.yml`](openai-compatible.yml) | any **OpenAI-compatible** endpoint (local Ollama `/v1`, gateway, vLLM, OpenRouter…) | `GADFLY_BASE_URL` (+ a key for most gateways) |
|
||||
| [`endpoint-aliases.yml`](endpoint-aliases.yml) | **several named backends** at once (one comment each) | repo vars `GADFLY_ENDPOINT_<NAME>` |
|
||||
| [`claude-code.yml`](claude-code.yml) | the bundled **Claude Code CLI** engine (`claude-code/<model>`) | secret `CLAUDE_CODE_OAUTH_TOKEN` (or `ANTHROPIC_API_KEY`) |
|
||||
| [`opencode.yml`](opencode.yml) | the bundled **OpenCode CLI** engine (`opencode/<model>`) driving an ollama-cloud model — benchmark it against the majordomo loop on the same model | secret `OLLAMA_CLOUD_API_KEY` |
|
||||
| [`.gadfly.yml`](.gadfly.yml) | **per-repo specialist config** (not a workflow — goes at your repo root) | — |
|
||||
|
||||
Common to all:
|
||||
|
||||
@@ -55,14 +55,14 @@ jobs:
|
||||
# csv to choose; "all" for everything; or define custom ones via a repo
|
||||
# .gadfly.yml / GADFLY_SPECIALIST_<NAME>. See README "Specialists".
|
||||
GADFLY_SPECIALISTS: ${{ vars.GADFLY_SPECIALISTS }}
|
||||
# Lens fan-out (optional; default 1 = lenses run sequentially within a
|
||||
# model). Raise it to run a model's lenses concurrently so each model
|
||||
# posts its comment sooner. Total in-flight requests = (models at once)
|
||||
# × (lenses at once), so to fan out without oversubscribing a backend,
|
||||
# keep its model cap low and raise its lens cap. Per-provider configurable
|
||||
# via GADFLY_PROVIDER_LENS_CONCURRENCY (same lanes as the model map):
|
||||
# GADFLY_PROVIDER_CONCURRENCY: "ollama-cloud=1,m1=1"
|
||||
# Concurrency (optional; default 1 = fully sequential per provider). The
|
||||
# ONE throttle is a per-provider LENS BUDGET: the max lens passes (a lens =
|
||||
# one specialist's review+recheck) in flight at once for a provider, shared
|
||||
# across ALL that provider's models — every model in a lane runs at once and
|
||||
# its lenses draw from the shared budget. Raise it to overlap lenses; set it
|
||||
# per provider with GADFLY_PROVIDER_LENS_CONCURRENCY:
|
||||
# GADFLY_PROVIDER_LENS_CONCURRENCY: "ollama-cloud=3,m1=1"
|
||||
# (The old GADFLY_PROVIDER_CONCURRENCY model cap was removed and is ignored.)
|
||||
# GADFLY_LENS_CONCURRENCY: ${{ vars.GADFLY_LENS_CONCURRENCY }}
|
||||
# GADFLY_PROVIDER_LENS_CONCURRENCY: ${{ vars.GADFLY_PROVIDER_LENS_CONCURRENCY }}
|
||||
# Live status board (optional; ON by default): one consolidated comment
|
||||
|
||||
@@ -0,0 +1,76 @@
|
||||
# Gadfly reviewing via the OpenCode CLI engine.
|
||||
# Copy to .gitea/workflows/adversarial-review.yml in your repo.
|
||||
#
|
||||
# Instead of gadfly's own majordomo loop, each lens shells out to the bundled
|
||||
# `opencode` CLI (opencode.ai) inside the checked-out repo — it uses its own read
|
||||
# tools to verify findings — while driving an ollama-cloud model. Gadfly then runs
|
||||
# its usual verdict + recheck + consolidate pipeline.
|
||||
#
|
||||
# Why: benchmark gadfly's boutique harness against a freely-available one ON THE
|
||||
# SAME MODEL. List both entries to get one comment section each and compare:
|
||||
# GADFLY_MODELS: "ollama-cloud/glm-5.2,opencode/glm-5.2"
|
||||
#
|
||||
# Auth: reuses the OLLAMA_CLOUD_API_KEY secret (same as the ollama-cloud path) —
|
||||
# no OpenCode-specific credential is needed for the ollama-cloud provider.
|
||||
#
|
||||
# Heads-up: this engine is newly wired and lightly tested — read the README's
|
||||
# "OpenCode engine" note before relying on it.
|
||||
|
||||
name: Adversarial Review (Gadfly)
|
||||
|
||||
on:
|
||||
pull_request:
|
||||
types: [opened, reopened, ready_for_review]
|
||||
issue_comment:
|
||||
types: [created]
|
||||
workflow_dispatch:
|
||||
inputs:
|
||||
pr_number: { description: "PR number to review", required: true }
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
issues: write
|
||||
pull-requests: write
|
||||
|
||||
concurrency:
|
||||
group: gadfly-${{ github.event.issue.number || github.event.pull_request.number || github.event.inputs.pr_number }}
|
||||
cancel-in-progress: true
|
||||
|
||||
jobs:
|
||||
review:
|
||||
# Security: only trusted users may trigger a secret-bearing run via a PR
|
||||
# comment. Replace the username(s) below with your maintainers — keep them in
|
||||
# sync with GADFLY_ALLOWED_USERS (the in-container belt-and-suspenders check).
|
||||
if: >-
|
||||
github.event_name != 'issue_comment'
|
||||
|| github.actor == 'your-username'
|
||||
runs-on: ubuntu-latest
|
||||
timeout-minutes: 30
|
||||
steps:
|
||||
- uses: docker://gitea.stevedudenhoeffer.com/steve/gadfly:latest
|
||||
env:
|
||||
GITEA_API: ${{ github.server_url }}/api/v1/repos/${{ github.repository }}
|
||||
GITEA_TOKEN: ${{ secrets.GITEA_TOKEN }}
|
||||
# --- OpenCode engine ---
|
||||
# Reuses the ollama-cloud key; mapped to OLLAMA_API_KEY in-container and
|
||||
# referenced by the generated provider as {env:OLLAMA_API_KEY}.
|
||||
OLLAMA_CLOUD_API_KEY: ${{ secrets.OLLAMA_CLOUD_API_KEY }}
|
||||
# "opencode/<model>" serves that model via ollama-cloud through OpenCode.
|
||||
# Model ids are verbatim (colons preserved). List an "ollama-cloud/<model>"
|
||||
# entry too to benchmark the two harnesses on the same model.
|
||||
GADFLY_MODELS: "opencode/glm-5.2"
|
||||
# Optional CLI tuning:
|
||||
# GADFLY_OPENCODE_BASE_URL: "https://ollama.com/v1" # or a local Ollama /v1
|
||||
# GADFLY_OPENCODE_MODEL: "glm-5.2" # overrides the spec suffix
|
||||
# GADFLY_OPENCODE_EXTRA_ARGS: "--variant reasoning" # whitespace-split
|
||||
# Escape hatch: "opencode/<provider>/<model>" passes straight to OpenCode's
|
||||
# own provider registry/auth (e.g. opencode/anthropic/claude-sonnet-4-6).
|
||||
GADFLY_ALLOWED_USERS: "your-username"
|
||||
# --- event context (leave as-is) ---
|
||||
EVENT_NAME: ${{ github.event_name }}
|
||||
PR: ${{ github.event.pull_request.number || github.event.issue.number || github.event.inputs.pr_number }}
|
||||
PR_BRANCH: ${{ github.head_ref }}
|
||||
IS_DRAFT: ${{ github.event.pull_request.draft }}
|
||||
COMMENT_BODY: ${{ github.event.comment.body }}
|
||||
COMMENT_ID: ${{ github.event.comment.id }}
|
||||
ACTOR: ${{ github.actor }}
|
||||
@@ -17,8 +17,9 @@
|
||||
# review never touches), so prefer the explicit form. Pin to an immutable
|
||||
# @<sha>: long-lived act_runners CACHE the reusable by ref, so a moved tag (@v1)
|
||||
# or @main is often not re-fetched and silently runs a stale copy. Bump the @<sha>
|
||||
# to adopt a structural change; routine swarm tuning rides owner variables (see
|
||||
# the gadfly README "Central config via variables") with no re-pin needed.
|
||||
# only to adopt a structural change; routine swarm tuning AND reviewer image
|
||||
# releases ride owner variables (GADFLY_DEFAULT_*, GADFLY_REVIEWER_TAG — see the
|
||||
# gadfly README "Central config via variables") with no re-pin needed.
|
||||
#
|
||||
# For custom named endpoints (GADFLY_ENDPOINT_<NAME>) or a provider the reusable
|
||||
# doesn't map, use the full stub in adversarial-review.yml instead.
|
||||
|
||||
@@ -29,6 +29,13 @@
|
||||
# tuning are read straight from the inherited environment — same as the other
|
||||
# provider keys (OPENAI_API_KEY, …) — so no extra wiring is needed here.
|
||||
#
|
||||
# opencode engine: when MODEL is "opencode" or "opencode/<model>" the binary
|
||||
# shells out to the bundled `opencode` CLI, driving an ollama-cloud model (for
|
||||
# benchmarking against the majordomo path on the same model). Its auth reuses
|
||||
# OLLAMA_CLOUD_API_KEY (mapped to OLLAMA_API_KEY below, same as the ollama-cloud
|
||||
# path) and GADFLY_OPENCODE_* tuning is read from the inherited environment — so
|
||||
# no extra wiring is needed here either.
|
||||
#
|
||||
# Optional:
|
||||
# MAX_DIFF_CHARS diff truncation cap for the prompt (default 60000)
|
||||
# GADFLY_STATUS_FILE per-model JSON path for the live status board (set by
|
||||
|
||||
Reference in New Issue
Block a user