steve
ac6ce06cdd
Build & push image / build-and-push (push) Successful in 33s
Makes gadfly a consumer of executus (run.Executor compaction/bounding/budget/critic + fanout) and fixes the large-PR token burn in size-gated layers: paginated get_diff, downshift above GADFLY_HUGE_DIFF_BYTES, and a swarm-wide GADFLY_PR_BUDGET_SECS backstop. Small PRs untouched; advisory-only and the static binary preserved. Dogfood swarm reviewed it (6 models, 21 real findings graded + folded in). Co-authored-by: Steve Dudenhoeffer <[email protected]> Co-committed-by: Steve Dudenhoeffer <[email protected]>
410 lines
18 KiB
Go
410 lines
18 KiB
Go
// Command gadfly is the agentic backend for the PR adversarial-review
|
||
// workflow (.gitea/workflows/pr-adversarial-review.yml). Unlike the old
|
||
// one-shot chat call, it runs a tool-using agent (majordomo + Ollama Cloud)
|
||
// over the PR's CHECKED-OUT repository: the model can read_file / list_dir /
|
||
// grep / find_files / get_diff to VERIFY a finding before reporting it, which
|
||
// kills the "diff-only" false positives (claiming a missing import or a
|
||
// non-existent method it simply couldn't see).
|
||
//
|
||
// It is a pure producer of review text: it reads the diff + the repo and
|
||
// prints the review markdown to stdout. All Gitea I/O (fetching the diff,
|
||
// upserting the comment) stays in run.sh, so this binary needs no repo write
|
||
// access and is straightforward to unit-test.
|
||
//
|
||
// Two passes (unless the draft is a clean "no material issues" pass): a
|
||
// REVIEW pass produces a draft, then an adversarial RECHECK pass independently
|
||
// re-verifies every finding against the actual files with the same tools and
|
||
// drops the ones it cannot confirm, recomputing the verdict. This catches the
|
||
// "confident but wrong" findings that survive a single pass — e.g. claiming an
|
||
// env var is unset when a wrapper script sets it (see recheck.go).
|
||
//
|
||
// Inputs (env):
|
||
//
|
||
// GADFLY_MODEL model id, or a full "provider/model" spec / majordomo
|
||
// alias / failover chain (required). A bare id is
|
||
// prefixed with GADFLY_PROVIDER.
|
||
// 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
|
||
// servers, remote Ollama, gateways). See model.go.
|
||
// GADFLY_API_KEY provider key; optional — falls back to the provider's
|
||
// standard env (OLLAMA_API_KEY / OPENAI_API_KEY /
|
||
// ANTHROPIC_API_KEY / GOOGLE_API_KEY|GEMINI_API_KEY).
|
||
// GADFLY_SPECIALISTS csv of review lenses, "all", or "auto" (see specialists.go).
|
||
// GADFLY_SELECTOR_MODEL model that picks lenses in "auto" mode (default: review model).
|
||
// GADFLY_WORKER_MODEL cheap model for the delegate_investigation tool (unset = off).
|
||
// GADFLY_REPO_DIR path to the checked-out repo (required; the FS sandbox root).
|
||
// GADFLY_DIFF_FILE path to a file holding the full unified diff (required).
|
||
// GADFLY_SYSTEM_FILE path to the reviewer system prompt (required).
|
||
// GADFLY_TITLE PR title (optional).
|
||
// GADFLY_BODY PR description (optional).
|
||
// GADFLY_MAX_STEPS review-pass step cap (optional, default 24).
|
||
// GADFLY_WRAPUP_RESERVE steps before the cap at which the wrap-up critic nudges
|
||
// the agent to stop investigating and write its answer
|
||
// (optional, default 4).
|
||
// 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_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.
|
||
// 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).
|
||
//
|
||
// On success it prints the review to stdout and exits 0. On a usage/config or
|
||
// model error it prints a diagnostic to stderr and exits non-zero; run.sh then
|
||
// posts a "reviewer failed" notice (advisory — never fails the CI job).
|
||
package main
|
||
|
||
import (
|
||
"context"
|
||
"errors"
|
||
"fmt"
|
||
"os"
|
||
"strconv"
|
||
"strings"
|
||
"time"
|
||
|
||
"gitea.stevedudenhoeffer.com/steve/executus/fanout"
|
||
)
|
||
|
||
const (
|
||
defaultMaxSteps = 24
|
||
// defaultTimeoutSecs is the deadline for EACH specialist's passes (review +
|
||
// recheck). It is per-lens, not shared across the suite, so one slow lens
|
||
// (e.g. a big local model) can't starve the others. Slow local models may
|
||
// need this raised (and a higher job timeout to match the suite total).
|
||
defaultTimeoutSecs = 300
|
||
defaultMaxDiffChars = 60000
|
||
// autoSelectTimeout bounds the dynamic specialist-selection call.
|
||
autoSelectTimeout = 120 * time.Second
|
||
// defaultWrapUpReserve is how many steps before the cap the agent is told
|
||
// to stop investigating and write its final answer. Reserving a margin is
|
||
// what keeps a thorough reviewer from spending its whole budget on tool
|
||
// 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 = 1
|
||
)
|
||
|
||
// wrapUpInstruction is steered into a running agent once it comes within the
|
||
// wrap-up reserve of its step cap: a forceful nudge to stop calling tools and
|
||
// emit the final answer using only what it has already gathered.
|
||
const wrapUpInstruction = "⚠️ You are almost out of your investigation budget — only a few tool steps remain. " +
|
||
"STOP calling tools now and write your FINAL answer immediately, using only what you have already verified. " +
|
||
"Do not begin any new investigation. If a finding could not be confirmed, drop it or mark it explicitly as unverified. " +
|
||
"Output the review in the required format right now."
|
||
|
||
func main() {
|
||
if err := run(); err != nil {
|
||
fmt.Fprintln(os.Stderr, "gadfly:", err)
|
||
os.Exit(1)
|
||
}
|
||
}
|
||
|
||
func run() error {
|
||
start := time.Now()
|
||
|
||
// Consolidation mode: not a review at all — read the per-model findings the
|
||
// swarm wrote and print the single cross-model consensus comment. entrypoint.sh
|
||
// runs this once, after every model has finished.
|
||
if strings.TrimSpace(os.Getenv("GADFLY_CONSOLIDATE_DIR")) != "" {
|
||
return runConsolidate()
|
||
}
|
||
|
||
repoDir := os.Getenv("GADFLY_REPO_DIR")
|
||
diffFile := os.Getenv("GADFLY_DIFF_FILE")
|
||
systemFile := os.Getenv("GADFLY_SYSTEM_FILE")
|
||
if repoDir == "" || diffFile == "" || systemFile == "" {
|
||
return errors.New("GADFLY_REPO_DIR, GADFLY_DIFF_FILE and GADFLY_SYSTEM_FILE are all required")
|
||
}
|
||
|
||
diffBytes, err := os.ReadFile(diffFile)
|
||
if err != nil {
|
||
return fmt.Errorf("read diff file: %w", err)
|
||
}
|
||
diff := string(diffBytes)
|
||
if strings.TrimSpace(diff) == "" {
|
||
return errors.New("empty diff; nothing to review")
|
||
}
|
||
|
||
systemBytes, err := os.ReadFile(systemFile)
|
||
if err != nil {
|
||
return fmt.Errorf("read system prompt: %w", err)
|
||
}
|
||
|
||
fsTools, err := newRepoFS(repoDir, diff)
|
||
if err != nil {
|
||
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"))
|
||
var eng reviewEngine
|
||
if ccSpec {
|
||
eng = newClaudeCodeEngine(os.Getenv("GADFLY_MODEL"), fsTools.root)
|
||
} else {
|
||
mdl, merr := resolveModel()
|
||
if merr != nil {
|
||
return fmt.Errorf("resolve model: %w", merr)
|
||
}
|
||
// Optional cheap worker for delegate_investigation. Non-fatal: a bad
|
||
// worker spec just disables delegation rather than sinking the review.
|
||
if worker, werr := resolveWorkerModel(); werr != nil {
|
||
fmt.Fprintln(os.Stderr, "gadfly: worker model disabled:", werr)
|
||
} else if worker != nil {
|
||
fsTools.worker = worker
|
||
}
|
||
// The context compactor needs a cheap summarizer; reuse the worker model
|
||
// when present, else the review model. A bad explicit GADFLY_COMPACT_MODEL
|
||
// just disables compaction rather than sinking the review.
|
||
summarizer, serr := resolveSummarizerModel(mdl, fsTools.worker)
|
||
if serr != nil {
|
||
fmt.Fprintln(os.Stderr, "gadfly: compaction summarizer disabled:", serr)
|
||
}
|
||
rex, rerr := newReviewExecutor(fsTools, mdl, summarizer, os.Getenv("GADFLY_MODEL"), newPRBudget())
|
||
if rerr != nil {
|
||
return fmt.Errorf("build review executor: %w", rerr)
|
||
}
|
||
eng = &majordomoEngine{rex: rex, mdl: mdl}
|
||
}
|
||
|
||
specialists, registry, auto, serrs := resolveSpecialists(repoDir)
|
||
for _, e := range serrs {
|
||
fmt.Fprintln(os.Stderr, "gadfly:", e)
|
||
}
|
||
|
||
// Dynamic selection: a (cheap) model picks the lenses this diff needs.
|
||
// Majordomo-only — the selector is an llm.Model.
|
||
if auto {
|
||
if ccSpec {
|
||
fmt.Fprintln(os.Stderr, "gadfly: auto-select is not supported with the claude-code engine; using the default suite")
|
||
specialists = suiteFromRegistry(registry, defaultSuite)
|
||
} else {
|
||
selector, serr := resolveSelectorModel(eng.(*majordomoEngine).mdl)
|
||
if serr != nil {
|
||
return fmt.Errorf("resolve selector model: %w", serr)
|
||
}
|
||
selCtx, cancel := context.WithTimeout(context.Background(), autoSelectTimeout)
|
||
picked, aerr := autoSelectSpecialists(selCtx, selector, os.Getenv("GADFLY_TITLE"), os.Getenv("GADFLY_BODY"), diff, registry)
|
||
cancel()
|
||
if aerr != nil {
|
||
fmt.Fprintln(os.Stderr, "gadfly: auto-select failed; falling back to the default suite:", aerr)
|
||
specialists = suiteFromRegistry(registry, defaultSuite)
|
||
} else {
|
||
specialists = picked
|
||
fmt.Fprintln(os.Stderr, "gadfly: auto-selected specialists:", specialistNamesOf(specialists))
|
||
}
|
||
}
|
||
}
|
||
|
||
if len(specialists) == 0 {
|
||
return errors.New("no specialists resolved to run")
|
||
}
|
||
|
||
base := string(systemBytes)
|
||
task := buildTask(diff)
|
||
results := runSpecialists(eng, base, specialists, task, diff)
|
||
|
||
fmt.Println(renderConsolidated(results))
|
||
|
||
// Optional, best-effort telemetry. OFF unless GADFLY_FINDINGS_URL is set;
|
||
// any failure is logged to stderr and never affects stdout or the exit code.
|
||
emit(results, time.Since(start))
|
||
|
||
// Optional per-model findings artifact for the cross-model consolidation
|
||
// pass. No-op unless GADFLY_FINDINGS_OUT is set (entrypoint sets it for a
|
||
// multi-model swarm). Best-effort, never affects stdout or the exit code.
|
||
writeFindingsOut(results)
|
||
return nil
|
||
}
|
||
|
||
// 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.
|
||
//
|
||
// 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.
|
||
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)
|
||
|
||
fanResults := fanout.Run(context.Background(), specialists, fanout.Options[Specialist]{
|
||
MaxConcurrent: lensConcurrency(),
|
||
}, func(_ 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:
|
||
// record the panic as an errored result and mark the lens finished.
|
||
defer func() {
|
||
if r := recover(); r != nil {
|
||
res = specialistResult{spec: sp, out: fmt.Sprintf("⚠️ This reviewer panicked: %v", r), verdict: verdictUnknown, errored: true}
|
||
sw.set(sp.Name, lensFinished, "", true)
|
||
}
|
||
}()
|
||
sw.set(sp.Name, lensRunning, "", false)
|
||
out, errored := reviewWithSpecialist(eng, base, sp, task, diff)
|
||
v := parseVerdict(out)
|
||
sw.set(sp.Name, lensFinished, v.label(), errored)
|
||
return specialistResult{spec: sp, out: out, verdict: v, errored: errored}, nil
|
||
})
|
||
|
||
// fanout guarantees input order; its Result.Err is set only when the context
|
||
// is cancelled before a lens ran (reviewWithSpecialist embeds its own failures
|
||
// in the result), so surface that as an errored lens rather than dropping it.
|
||
results := make([]specialistResult, len(specialists))
|
||
for i, r := range fanResults {
|
||
if r.Err != nil {
|
||
results[i] = specialistResult{spec: specialists[i], out: fmt.Sprintf("⚠️ This reviewer did not run: %v", r.Err), verdict: verdictUnknown, errored: true}
|
||
sw.set(specialists[i].Name, lensFinished, "", true)
|
||
continue
|
||
}
|
||
results[i] = r.Value
|
||
}
|
||
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.
|
||
func lensConcurrency() int {
|
||
if n, ok := providerOverride("GADFLY_PROVIDER_LENS_CONCURRENCY", modelProvider()); ok {
|
||
return n
|
||
}
|
||
return envInt("GADFLY_LENS_CONCURRENCY", defaultLensConcurrency)
|
||
}
|
||
|
||
// 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.
|
||
func providerOverride(envName, provider string) (int, bool) {
|
||
for _, item := range strings.Split(os.Getenv(envName), ",") {
|
||
k, v, ok := strings.Cut(item, "=")
|
||
if !ok || strings.TrimSpace(k) != provider {
|
||
continue
|
||
}
|
||
if n, err := strconv.Atoi(strings.TrimSpace(v)); err == nil && n > 0 {
|
||
return n, true
|
||
}
|
||
}
|
||
return 0, false
|
||
}
|
||
|
||
// reviewWithSpecialist runs one lens end-to-end under its OWN timeout, so a slow
|
||
// model on one lens can't starve the others: a review pass under the
|
||
// specialist's composed prompt, then the shared adversarial recheck pass. The
|
||
// returned bool is true when the review pass failed (rendered as an inline
|
||
// notice — advisory; one lens failing never sinks the others or the job).
|
||
func reviewWithSpecialist(eng reviewEngine, base string, sp Specialist, task, diff string) (string, bool) {
|
||
ctx, cancel := context.WithTimeout(context.Background(), reviewTimeout())
|
||
defer cancel()
|
||
|
||
draft, err := eng.runPass(ctx, composeSpecialistPrompt(base, sp), task,
|
||
envInt("GADFLY_MAX_STEPS", defaultMaxSteps))
|
||
if err != nil {
|
||
fmt.Fprintf(os.Stderr, "gadfly: specialist %q review pass failed: %v\n", sp.Name, err)
|
||
return fmt.Sprintf("⚠️ This reviewer failed to complete: %v", err), true
|
||
}
|
||
|
||
final := draft
|
||
if shouldRecheck(draft) {
|
||
rechecked, rerr := eng.runPass(ctx, recheckSystemPrompt, buildRecheckTask(draft, diff),
|
||
envInt("GADFLY_RECHECK_MAX_STEPS", defaultRecheckMaxSteps))
|
||
if rerr != nil {
|
||
fmt.Fprintf(os.Stderr, "gadfly: specialist %q recheck failed; emitting unverified draft: %v\n", sp.Name, rerr)
|
||
} else {
|
||
final = rechecked
|
||
}
|
||
}
|
||
return final, false
|
||
}
|
||
|
||
// wrapUpReserve is how many steps before the cap the wrap-up nudge fires,
|
||
// overridable via GADFLY_WRAPUP_RESERVE.
|
||
func wrapUpReserve() int {
|
||
return envInt("GADFLY_WRAPUP_RESERVE", defaultWrapUpReserve)
|
||
}
|
||
|
||
// buildTask assembles the user message: PR metadata plus the unified diff,
|
||
// truncated for the prompt (the full diff stays available via get_diff).
|
||
func buildTask(diff string) string {
|
||
title := os.Getenv("GADFLY_TITLE")
|
||
body := os.Getenv("GADFLY_BODY")
|
||
|
||
maxDiff := envInt("GADFLY_MAX_DIFF_CHARS", defaultMaxDiffChars)
|
||
truncNote := ""
|
||
if maxDiff > 0 && len(diff) > maxDiff {
|
||
diff = diff[:maxDiff]
|
||
truncNote = fmt.Sprintf("\n\n[NOTE: diff truncated to %d chars in this message; page the full diff with get_diff (paginated; pass a `path` to scope it to one file) or read the changed files.]", maxDiff)
|
||
}
|
||
|
||
var b strings.Builder
|
||
if title != "" {
|
||
fmt.Fprintf(&b, "PR title: %s\n\n", title)
|
||
}
|
||
if strings.TrimSpace(body) != "" {
|
||
fmt.Fprintf(&b, "PR description:\n%s\n\n", body)
|
||
}
|
||
b.WriteString("Review the following unified diff. Before reporting any cross-file or compile-correctness issue, use your repository read tools to verify it against the actual checked-out code — do not rely on the diff alone.\n\n")
|
||
fmt.Fprintf(&b, "```diff\n%s\n```%s", diff, truncNote)
|
||
return b.String()
|
||
}
|
||
|
||
// envInt reads an integer env var, falling back to def when unset or unparseable.
|
||
func envInt(name string, def int) int {
|
||
v := strings.TrimSpace(os.Getenv(name))
|
||
if v == "" {
|
||
return def
|
||
}
|
||
n, err := strconv.Atoi(v)
|
||
if err != nil || n <= 0 {
|
||
return def
|
||
}
|
||
return n
|
||
}
|
||
|
||
// envBool reads a boolean-ish env var: def when unset, false for an explicit
|
||
// falsey value (0/false/no/off), true otherwise. The shared spelling for
|
||
// gadfly's "on unless disabled" opt-out flags (GADFLY_RECHECK, GADFLY_COMPACT).
|
||
func envBool(name string, def bool) bool {
|
||
switch strings.ToLower(strings.TrimSpace(os.Getenv(name))) {
|
||
case "":
|
||
return def
|
||
case "0", "false", "no", "off":
|
||
return false
|
||
default:
|
||
return true
|
||
}
|
||
}
|
||
|
||
// reviewTimeout is the per-specialist-lens deadline (GADFLY_TIMEOUT_SECS), shared
|
||
// across a lens's review+recheck passes and applied as each pass's run cap.
|
||
func reviewTimeout() time.Duration {
|
||
return time.Duration(envInt("GADFLY_TIMEOUT_SECS", defaultTimeoutSecs)) * time.Second
|
||
}
|