From a1b0691a1ef7a7c760263e29da0bff90d615dfc7 Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Sat, 27 Jun 2026 14:56:41 -0400 Subject: [PATCH] fix: fold in gadfly's own review findings (3 real bugs) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dogfood swarm reviewed PR #1; folding in the warranted findings (graded via the gadfly MCP — 18 real / 18 false-positive across the 4 completed reviewers): - entrypoint.sh: finalize a never-written status file when run.sh skips the binary (empty diff / no key / missing binary). The pre-seed stayed {started:0, done:false}, so the board showed that model "waiting to start" forever and the N/N counter never completed — breaking the board's own "tell when everything is finished" invariant. (glm-5.2, correctness — the strongest finding.) - main.go: recover() in the per-lens goroutine. A panic previously crashed the whole binary (killing every other lens's output) and left the lens stuck "running" on the board. Now it's recorded as an errored result and the lens is marked finished. (glm-5.2 + minimax-m3.) - status-board.sh: coerce a non-numeric GADFLY_STATUS_POLL_SECS back to 12. Under `set -uo pipefail` a bad `sleep "$POLL"` failed silently and the loop spun, hammering the Gitea API. (glm-5.2, error-handling.) The remaining real findings (sanitizer collision, page-10 pagination, markdown-injection via PR-controlled lens names, cosmetic blank line) were graded trivial and left as-is — documented in the finding notes. gofmt clean, go vet quiet, go build + go test -race green. Co-Authored-By: Claude Opus 4.8 (1M context) --- cmd/gadfly/main.go | 10 ++++++++++ entrypoint.sh | 8 ++++++++ scripts/status-board.sh | 5 +++++ 3 files changed, 23 insertions(+) diff --git a/cmd/gadfly/main.go b/cmd/gadfly/main.go index caf41d7..3718c56 100644 --- a/cmd/gadfly/main.go +++ b/cmd/gadfly/main.go @@ -233,6 +233,16 @@ func runSpecialists(mdl llm.Model, fsTools *repoFS, base string, specialists []S go func(i int, sp Specialist) { defer wg.Done() defer func() { <-sem }() + // 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. Recover, record it as an errored + // result, and mark the lens finished so the board can complete. + defer func() { + if r := recover(); r != nil { + results[i] = 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(mdl, fsTools, base, sp, task, diff) v := parseVerdict(out) diff --git a/entrypoint.sh b/entrypoint.sh index f53a670..23649b3 100644 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -178,6 +178,14 @@ review_one() { PROVIDER=ollama MODEL="$1" GADFLY_BIN="/usr/local/bin/gadfly" GADFLY_REPO_DIR="$REPO_DIR" \ GADFLY_STATUS_FILE="$sf" \ bash "${SCRIPTS_DIR}/run.sh" || log "model $1 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 + # such a never-started file done so the board can complete. The binary stamps a + # nonzero `started`, so that reliably distinguishes "ran" from "skipped". + if [ -n "$sf" ] && [ -f "$sf" ] && [ "$(jq -r '.started // 0' "$sf" 2>/dev/null)" = "0" ]; then + tmp="$(jq '.done = true' "$sf" 2>/dev/null)" && printf '%s' "$tmp" > "$sf" + fi } # Normalize the model list (trim, drop blanks) into MODEL_LIST. diff --git a/scripts/status-board.sh b/scripts/status-board.sh index 1da535b..f207a1d 100755 --- a/scripts/status-board.sh +++ b/scripts/status-board.sh @@ -29,6 +29,11 @@ set -uo pipefail : "${GADFLY_STATUS_DIR:?GADFLY_STATUS_DIR required}" POLL="${GADFLY_STATUS_POLL_SECS:-12}" +# Guard against a non-numeric poll interval: with `set -uo pipefail` (no set -e) +# a bad `sleep "$POLL"` would fail silently and the `while :` loop would spin, +# hammering the Gitea API. Coerce anything non-integer (or <1) back to 12. +case "$POLL" in ''|*[!0-9]*) POLL=12 ;; esac +[ "$POLL" -ge 1 ] 2>/dev/null || POLL=12 DONE_FILE="${GADFLY_STATUS_DIR}/.done" MARKER="" API_TIMEOUT="--connect-timeout 20 --max-time 30"