feat: inline COMMENT-state PR review (findings anchored to changed lines) #18
@@ -61,6 +61,7 @@ on:
|
|||||||
allowed_users: { type: string, default: "" } # GADFLY_ALLOWED_USERS (consumer-specific; set in your stub)
|
allowed_users: { type: string, default: "" } # GADFLY_ALLOWED_USERS (consumer-specific; set in your stub)
|
||||||
trigger_phrase: { type: string, default: "" } # GADFLY_TRIGGER_PHRASE
|
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
|
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
|
||||||
# Job wall-clock cap. 90 as a default: the 5-lens suite across a slow lane
|
# Job wall-clock cap. 90 as a default: the 5-lens suite across a slow lane
|
||||||
# (claude-code with extended thinking) over two passes can run long.
|
# (claude-code with extended thinking) over two passes can run long.
|
||||||
timeout_minutes: { type: number, default: 90 }
|
timeout_minutes: { type: number, default: 90 }
|
||||||
@@ -145,3 +146,4 @@ jobs:
|
|||||||
GADFLY_ALLOWED_USERS: ${{ inputs.allowed_users }}
|
GADFLY_ALLOWED_USERS: ${{ inputs.allowed_users }}
|
||||||
GADFLY_TRIGGER_PHRASE: ${{ inputs.trigger_phrase }}
|
GADFLY_TRIGGER_PHRASE: ${{ inputs.trigger_phrase }}
|
||||||
GADFLY_CONSOLIDATE: ${{ inputs.consolidate }}
|
GADFLY_CONSOLIDATE: ${{ inputs.consolidate }}
|
||||||
|
GADFLY_INLINE_REVIEW: ${{ inputs.inline_review }}
|
||||||
|
|||||||
@@ -306,6 +306,14 @@ back to posting the per-model comments. Controlled by `GADFLY_CONSOLIDATE`: `aut
|
|||||||
for ≥2 models), `1` (force on), `0` (force off, one comment per model). Single-model runs are
|
for ≥2 models), `1` (force on), `0` (force off, one comment per model). Single-model runs are
|
||||||
unaffected.
|
unaffected.
|
||||||
|
|
||||||
|
**Inline PR review.** Alongside the consensus comment, Gadfly also posts a single Gitea **pull
|
||||||
|
review** (state `COMMENT` — advisory, **never** request-changes or approve, so it can't block a
|
||||||
|
merge) whose inline comments anchor each consensus finding to the exact changed line it's about.
|
||||||
|
Only findings that land on a line in the diff are anchored (Gitea rejects comments off the diff);
|
||||||
|
the rest stay in the consensus comment. A re-run replaces the previous review instead of stacking.
|
||||||
|
It's the "reviewer integrated with Gitea" without the blocking — turn it off with
|
||||||
|
`GADFLY_INLINE_REVIEW=0`.
|
||||||
|
|
||||||
### Triggers
|
### Triggers
|
||||||
|
|
||||||
1. A **new/reopened/ready** non-draft PR — automatic.
|
1. A **new/reopened/ready** non-draft PR — automatic.
|
||||||
@@ -392,6 +400,7 @@ The reviewer binary reads these (the stub/entrypoint set sane defaults):
|
|||||||
| `GADFLY_STATUS_BOARD` | on | set `0` to disable the live status-board comment |
|
| `GADFLY_STATUS_BOARD` | on | set `0` to disable the live status-board comment |
|
||||||
| `GADFLY_STATUS_POLL_SECS` | 12 | how often the status board re-renders/upserts |
|
| `GADFLY_STATUS_POLL_SECS` | 12 | how often the status board re-renders/upserts |
|
||||||
| `GADFLY_CONSOLIDATE` | `auto` | cross-model consensus comment: `auto` (on for ≥2 models), `1` (force on), `0` (off — one comment per model) |
|
| `GADFLY_CONSOLIDATE` | `auto` | cross-model consensus comment: `auto` (on for ≥2 models), `1` (force on), `0` (off — one comment per model) |
|
||||||
|
| `GADFLY_INLINE_REVIEW` | on | when consolidating, also post a `COMMENT`-state PR review with inline comments on changed lines; `0` disables |
|
||||||
| `GADFLY_TRIGGER_PHRASE` | `@gadfly review` | comment phrase that re-triggers |
|
| `GADFLY_TRIGGER_PHRASE` | `@gadfly review` | comment phrase that re-triggers |
|
||||||
| `GADFLY_ALLOWED_USERS` | *(collaborators)* | comma-separated allow-list for comment triggers |
|
| `GADFLY_ALLOWED_USERS` | *(collaborators)* | comma-separated allow-list for comment triggers |
|
||||||
| `GADFLY_FINDINGS_URL` | — | gadfly-reports store base URL; set to enable findings telemetry (off when empty) |
|
| `GADFLY_FINDINGS_URL` | — | gadfly-reports store base URL; set to enable findings telemetry (off when empty) |
|
||||||
|
|||||||
+24
-3
@@ -170,10 +170,18 @@ func runConsolidate() error {
|
|||||||
if len(models) == 0 {
|
if len(models) == 0 {
|
||||||
return errors.New("no model findings to consolidate")
|
return errors.New("no model findings to consolidate")
|
||||||
}
|
}
|
||||||
|
// Cluster once, then render the consensus comment and (best-effort) the inline
|
||||||
|
// PR review from the same clusters so the two views can't drift.
|
||||||
|
clusters := clusterFindings(models)
|
||||||
|
|
||||||
// Lead with the marker so entrypoint.sh can upsert this comment in place
|
// Lead with the marker so entrypoint.sh can upsert this comment in place
|
||||||
// (same pattern as run.sh's per-model marker); it appends the advisory footer.
|
// (same pattern as run.sh's per-model marker); it appends the advisory footer.
|
||||||
fmt.Println(consensusMarker)
|
fmt.Println(consensusMarker)
|
||||||
fmt.Println(renderConsensus(models))
|
fmt.Println(renderConsensus(models, clusters))
|
||||||
|
|
||||||
|
// Inline PR review (COMMENT state) anchoring findings to changed lines.
|
||||||
|
// Best-effort: no-op without diff/API creds, never affects stdout/exit.
|
||||||
|
postInlineReview(clusters)
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -187,6 +195,7 @@ type cluster struct {
|
|||||||
maxLine int // largest line in the cluster — the span's upper edge
|
maxLine int // largest line in the cluster — the span's upper edge
|
||||||
severity string
|
severity string
|
||||||
title string
|
title string
|
||||||
|
detail string // highest-severity report's detail; rendered in the inline review comment
|
||||||
models map[string]bool
|
models map[string]bool
|
||||||
lenses map[string]bool
|
lenses map[string]bool
|
||||||
}
|
}
|
||||||
@@ -206,7 +215,7 @@ const lineTolerance = 3
|
|||||||
// renderConsensus builds the single consolidated comment body from every model's
|
// renderConsensus builds the single consolidated comment body from every model's
|
||||||
// findings. It does NOT emit the marker or advisory footer — entrypoint.sh wraps
|
// findings. It does NOT emit the marker or advisory footer — entrypoint.sh wraps
|
||||||
// it (mirroring run.sh's per-model framing).
|
// it (mirroring run.sh's per-model framing).
|
||||||
func renderConsensus(models []modelFindings) string {
|
func renderConsensus(models []modelFindings, clusters []cluster) string {
|
||||||
// effective = models that actually produced a review. Errored models are
|
// effective = models that actually produced a review. Errored models are
|
||||||
// shown (folded, below) but excluded from the agreement denominator so a
|
// shown (folded, below) but excluded from the agreement denominator so a
|
||||||
// failed model doesn't dilute every ratio.
|
// failed model doesn't dilute every ratio.
|
||||||
@@ -217,7 +226,6 @@ func renderConsensus(models []modelFindings) string {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
errored := len(models) - effective
|
errored := len(models) - effective
|
||||||
clusters := clusterFindings(models)
|
|
||||||
|
|
||||||
worst := verdictClean
|
worst := verdictClean
|
||||||
for _, m := range models {
|
for _, m := range models {
|
||||||
@@ -333,6 +341,7 @@ func clusterFindings(models []modelFindings) []cluster {
|
|||||||
maxLine: it.f.Line,
|
maxLine: it.f.Line,
|
||||||
severity: it.f.Severity,
|
severity: it.f.Severity,
|
||||||
title: it.f.Title,
|
title: it.f.Title,
|
||||||
|
detail: it.f.Detail,
|
||||||
models: map[string]bool{},
|
models: map[string]bool{},
|
||||||
lenses: map[string]bool{},
|
lenses: map[string]bool{},
|
||||||
}
|
}
|
||||||
@@ -373,11 +382,23 @@ func mergeIntoCluster(c *cluster, f outFinding, model string) {
|
|||||||
if f.Line > c.maxLine {
|
if f.Line > c.maxLine {
|
||||||
c.maxLine = f.Line
|
c.maxLine = f.Line
|
||||||
}
|
}
|
||||||
|
// Backfill an empty title/detail from any report, regardless of severity, so a
|
||||||
|
// higher-severity-but-terse finding doesn't leave the cluster without context.
|
||||||
|
if strings.TrimSpace(c.title) == "" && strings.TrimSpace(f.Title) != "" {
|
||||||
|
c.title = f.Title
|
||||||
|
}
|
||||||
|
if strings.TrimSpace(c.detail) == "" && strings.TrimSpace(f.Detail) != "" {
|
||||||
|
c.detail = f.Detail
|
||||||
|
}
|
||||||
|
// A strictly-higher-severity report takes over the title/detail.
|
||||||
if sevRank(f.Severity) > sevRank(c.severity) {
|
if sevRank(f.Severity) > sevRank(c.severity) {
|
||||||
c.severity = f.Severity
|
c.severity = f.Severity
|
||||||
if strings.TrimSpace(f.Title) != "" {
|
if strings.TrimSpace(f.Title) != "" {
|
||||||
c.title = f.Title
|
c.title = f.Title
|
||||||
}
|
}
|
||||||
|
if strings.TrimSpace(f.Detail) != "" {
|
||||||
|
c.detail = f.Detail
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -52,7 +52,7 @@ func TestRenderConsensusFoldsSingleModelNits(t *testing.T) {
|
|||||||
{Lens: "security", File: "a.go", Line: 10, Severity: "high", Title: "auth bypass"},
|
{Lens: "security", File: "a.go", Line: 10, Severity: "high", Title: "auth bypass"},
|
||||||
}},
|
}},
|
||||||
}
|
}
|
||||||
out := renderConsensus(models)
|
out := renderConsensus(models, clusterFindings(models))
|
||||||
// Headline table: the agreed finding with a 2/2 badge.
|
// Headline table: the agreed finding with a 2/2 badge.
|
||||||
if !strings.Contains(out, "2/2") {
|
if !strings.Contains(out, "2/2") {
|
||||||
t.Errorf("expected a 2/2 agreement badge in headline:\n%s", out)
|
t.Errorf("expected a 2/2 agreement badge in headline:\n%s", out)
|
||||||
@@ -78,7 +78,7 @@ func TestRenderConsensusHighSeverityLoneFindingStaysHeadline(t *testing.T) {
|
|||||||
{Lens: "security", File: "a.go", Line: 1, Severity: "critical", Title: "rce"},
|
{Lens: "security", File: "a.go", Line: 1, Severity: "critical", Title: "rce"},
|
||||||
}},
|
}},
|
||||||
}
|
}
|
||||||
out := renderConsensus(models)
|
out := renderConsensus(models, clusterFindings(models))
|
||||||
headline := out
|
headline := out
|
||||||
if i := strings.Index(out, "single-model finding"); i >= 0 {
|
if i := strings.Index(out, "single-model finding"); i >= 0 {
|
||||||
headline = out[:i]
|
headline = out[:i]
|
||||||
@@ -113,7 +113,7 @@ func TestRenderConsensusExcludesErroredFromDenominator(t *testing.T) {
|
|||||||
{Lens: "security", File: "a.go", Line: 9, Severity: "medium", Title: "leak"}}},
|
{Lens: "security", File: "a.go", Line: 9, Severity: "medium", Title: "leak"}}},
|
||||||
{Model: "broken", Verdict: "reviewer failed", Errored: true, Markdown: "boom"},
|
{Model: "broken", Verdict: "reviewer failed", Errored: true, Markdown: "boom"},
|
||||||
}
|
}
|
||||||
out := renderConsensus(models)
|
out := renderConsensus(models, clusterFindings(models))
|
||||||
// Denominator is the 2 effective models, not 3; the failure is noted.
|
// Denominator is the 2 effective models, not 3; the failure is noted.
|
||||||
if !strings.Contains(out, "2/2") {
|
if !strings.Contains(out, "2/2") {
|
||||||
t.Errorf("errored model must be excluded from the denominator (want 2/2):\n%s", out)
|
t.Errorf("errored model must be excluded from the denominator (want 2/2):\n%s", out)
|
||||||
@@ -133,7 +133,7 @@ func TestRenderConsensusLoneHighFolds(t *testing.T) {
|
|||||||
{Model: "solo", Verdict: "Blocking issues found", Markdown: "x", Findings: []outFinding{
|
{Model: "solo", Verdict: "Blocking issues found", Markdown: "x", Findings: []outFinding{
|
||||||
{Lens: "security", File: "a.go", Line: 1, Severity: "high", Title: "maybe-bug"}}},
|
{Lens: "security", File: "a.go", Line: 1, Severity: "high", Title: "maybe-bug"}}},
|
||||||
}
|
}
|
||||||
out := renderConsensus(models)
|
out := renderConsensus(models, clusterFindings(models))
|
||||||
head := out
|
head := out
|
||||||
if i := strings.Index(out, "single-model finding"); i >= 0 {
|
if i := strings.Index(out, "single-model finding"); i >= 0 {
|
||||||
head = out[:i]
|
head = out[:i]
|
||||||
|
|||||||
@@ -0,0 +1,296 @@
|
|||||||
|
package main
|
||||||
|
|
||||||
|
// Inline PR review. After the consensus comment is rendered, Gadfly also posts a
|
||||||
|
// single Gitea pull review (state COMMENT — advisory, NEVER request-changes or
|
||||||
|
// approve) whose inline comments anchor consensus findings to the exact changed
|
||||||
|
// lines. The issue comment stays the ranked overview; the review puts each
|
||||||
|
// finding next to the code it's about — the "reviewer integrated with Gitea" the
|
||||||
|
// project wanted, without ever blocking a merge.
|
||||||
|
//
|
||||||
|
// All of this is best-effort: disabled by GADFLY_INLINE_REVIEW=0 or when the diff
|
||||||
|
// / API creds aren't available, only anchors findings that land on a line present
|
||||||
|
// in the diff (Gitea rejects comments off the diff), and any error is logged to
|
||||||
|
// stderr without touching the consensus comment (already on stdout) or the exit
|
||||||
|
// code.
|
||||||
|
|
||||||
|
import (
|
||||||
|
"bytes"
|
||||||
|
"encoding/json"
|
||||||
|
"fmt"
|
||||||
|
"io"
|
||||||
|
"net/http"
|
||||||
|
"os"
|
||||||
|
"regexp"
|
||||||
|
"strconv"
|
||||||
|
"strings"
|
||||||
|
"time"
|
||||||
|
)
|
||||||
|
|
||||||
|
const (
|
||||||
|
// inlineReviewMarker tags our review body so a re-run can delete the previous
|
||||||
|
// one instead of stacking duplicate inline comments.
|
||||||
|
inlineReviewMarker = "<!-- gadfly-inline-review -->"
|
||||||
|
// maxInlineComments caps how many inline comments one review carries, so a
|
||||||
|
// huge diff can't produce a wall of annotations. Clusters are pre-sorted by
|
||||||
|
// agreement×severity, so the cap keeps the most important.
|
||||||
|
maxInlineComments = 25
|
||||||
|
inlineReviewHTTPTimeout = 20 * time.Second
|
||||||
|
)
|
||||||
|
|
||||||
|
// reviewComment is one inline comment in a Gitea pull review. Field names match
|
||||||
|
// the Gitea API EXACTLY (new_position = line in the new/head file).
|
||||||
|
type reviewComment struct {
|
||||||
|
Path string `json:"path"`
|
||||||
|
Body string `json:"body"`
|
||||||
|
NewPosition int `json:"new_position"`
|
||||||
|
}
|
||||||
|
|
||||||
|
// createReview is the POST /pulls/{n}/reviews body. event is ALWAYS "COMMENT".
|
||||||
|
type createReview struct {
|
||||||
|
Body string `json:"body"`
|
||||||
|
Event string `json:"event"`
|
||||||
|
Comments []reviewComment `json:"comments"`
|
||||||
|
}
|
||||||
|
|
||||||
|
// postInlineReview posts one COMMENT-state pull review with inline comments for
|
||||||
|
// consensus findings on changed lines. No-op + best-effort (see file comment).
|
||||||
|
func postInlineReview(clusters []cluster) {
|
||||||
|
if strings.EqualFold(strings.TrimSpace(os.Getenv("GADFLY_INLINE_REVIEW")), "0") {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
api := strings.TrimRight(strings.TrimSpace(os.Getenv("GITEA_API")), "/")
|
||||||
|
token := strings.TrimSpace(os.Getenv("GITEA_TOKEN"))
|
||||||
|
pr := strings.TrimSpace(os.Getenv("GADFLY_PR"))
|
||||||
|
diffPath := strings.TrimSpace(os.Getenv("GADFLY_DIFF_FILE"))
|
||||||
|
if api == "" || token == "" || pr == "" || diffPath == "" {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
diff, err := os.ReadFile(diffPath)
|
||||||
|
if err != nil {
|
||||||
|
fmt.Fprintln(os.Stderr, "gadfly: inline review: read diff:", err)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
comments := inlineComments(clusters, parseDiffNewLines(string(diff)))
|
||||||
|
if len(comments) == 0 {
|
||||||
|
return // nothing anchors to a changed line; the consensus comment covers it
|
||||||
|
}
|
||||||
|
|
||||||
|
client := &http.Client{Timeout: inlineReviewHTTPTimeout}
|
||||||
|
base := fmt.Sprintf("%s/pulls/%s/reviews", api, pr)
|
||||||
|
deletePriorReviews(client, base, token) // avoid stacking on re-runs
|
||||||
|
|
||||||
|
body := fmt.Sprintf("%s\n🪰 **Gadfly consensus review** — %d inline finding%s on changed lines. See the consensus comment for the full ranked summary.\n\n<sub>Advisory only — does not block merge.</sub>",
|
||||||
|
inlineReviewMarker, len(comments), plural(len(comments)))
|
||||||
|
if err := giteaSend(client, http.MethodPost, base, token, createReview{Body: body, Event: "COMMENT", Comments: comments}); err != nil {
|
||||||
|
fmt.Fprintln(os.Stderr, "gadfly: inline review post:", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// inlineComments builds inline comments for the clusters that anchor to a line
|
||||||
|
// present in the diff, in priority order (clusters are pre-sorted), capped.
|
||||||
|
func inlineComments(clusters []cluster, addable map[string]map[int]bool) []reviewComment {
|
||||||
|
var out []reviewComment
|
||||||
|
for _, c := range clusters {
|
||||||
|
path := normPath(c.file)
|
||||||
|
anchor := anchorLine(addable[path], c.line, c.maxLine)
|
||||||
|
if anchor == 0 {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
out = append(out, reviewComment{Path: path, NewPosition: anchor, Body: inlineBody(c)})
|
||||||
|
if len(out) >= maxInlineComments {
|
||||||
|
break
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return out
|
||||||
|
}
|
||||||
|
|
||||||
|
// anchorLine returns the first line in [lo,hi] that is an added line in the diff,
|
||||||
|
// or 0 if none. Scanning the cluster's whole span (not just its representative
|
||||||
|
// line) anchors a finding whose min line is just outside the diff but whose span
|
||||||
|
// still overlaps a changed line.
|
||||||
|
func anchorLine(added map[int]bool, lo, hi int) int {
|
||||||
|
if added == nil || lo <= 0 {
|
||||||
|
return 0
|
||||||
|
}
|
||||||
|
if hi < lo { // single-line cluster (maxLine unset)
|
||||||
|
hi = lo
|
||||||
|
}
|
||||||
|
for ln := lo; ln <= hi; ln++ {
|
||||||
|
if added[ln] {
|
||||||
|
return ln
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return 0
|
||||||
|
}
|
||||||
|
|
||||||
|
// inlineBody renders one inline comment: severity + title, who flagged it, detail.
|
||||||
|
func inlineBody(c cluster) string {
|
||||||
|
var b strings.Builder
|
||||||
|
fmt.Fprintf(&b, "%s **%s**", sevIcon(c.severity), strings.TrimSpace(c.title))
|
||||||
|
fmt.Fprintf(&b, "\n\n_%s · flagged by %d model%s_", lensList(c.lenses), len(c.models), plural(len(c.models)))
|
||||||
|
if d := strings.TrimSpace(c.detail); d != "" {
|
||||||
|
fmt.Fprintf(&b, "\n\n%s", d)
|
||||||
|
}
|
||||||
|
b.WriteString("\n\n<sub>🪰 Gadfly · advisory</sub>")
|
||||||
|
return b.String()
|
||||||
|
}
|
||||||
|
|
||||||
|
// parseDiffNewLines returns, per file, the set of NEW-file line numbers that were
|
||||||
|
// ADDED in the unified diff — the safest lines for an inline comment to anchor to
|
||||||
|
// (Gitea reliably accepts comments on added lines). Context lines are walked to
|
||||||
|
// keep the line counter correct but are NOT recorded: anchoring only to added
|
||||||
|
// lines avoids the all-or-nothing review POST being rejected for an off-change
|
||||||
|
// anchor. Hunk lengths from the @@ header bound each hunk, so a content line that
|
||||||
|
// happens to start with "+++ " or "@@" is still read as content, not a header.
|
||||||
|
func parseDiffNewLines(diff string) map[string]map[int]bool {
|
||||||
|
out := map[string]map[int]bool{}
|
||||||
|
var file string
|
||||||
|
var newLine, oldRem, newRem int
|
||||||
|
inHunk := false
|
||||||
|
for _, line := range strings.Split(diff, "\n") {
|
||||||
|
if inHunk && (newRem > 0 || oldRem > 0) {
|
||||||
|
switch {
|
||||||
|
case strings.HasPrefix(line, "+"):
|
||||||
|
record(out, file, newLine) // added line — anchorable
|
||||||
|
newLine++
|
||||||
|
newRem--
|
||||||
|
case strings.HasPrefix(line, "-"):
|
||||||
|
oldRem--
|
||||||
|
case strings.HasPrefix(line, "\\"): // "\ No newline at end of file"
|
||||||
|
default: // context line (leading space, or an empty line): advance, don't record
|
||||||
|
newLine++
|
||||||
|
newRem--
|
||||||
|
oldRem--
|
||||||
|
}
|
||||||
|
if newRem <= 0 && oldRem <= 0 {
|
||||||
|
inHunk = false
|
||||||
|
}
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
switch {
|
||||||
|
case strings.HasPrefix(line, "+++ "):
|
||||||
|
file = normPath(strings.TrimPrefix(line, "+++ "))
|
||||||
|
if file == "/dev/null" {
|
||||||
|
file = ""
|
||||||
|
}
|
||||||
|
case strings.HasPrefix(line, "@@"):
|
||||||
|
if m := hunkRe.FindStringSubmatch(line); m != nil && file != "" {
|
||||||
|
newLine, _ = strconv.Atoi(m[3])
|
||||||
|
oldRem = atoiOr(m[2], 1)
|
||||||
|
newRem = atoiOr(m[4], 1)
|
||||||
|
inHunk = newRem > 0 || oldRem > 0
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return out
|
||||||
|
}
|
||||||
|
|
||||||
|
// hunkRe captures a unified-diff hunk header's old/new start+length:
|
||||||
|
// @@ -<oldStart>[,<oldLen>] +<newStart>[,<newLen>] @@
|
||||||
|
var hunkRe = regexp.MustCompile(`^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@`)
|
||||||
|
|
||||||
|
// normPath trims a unified-diff path down to a repo-relative one: strip a single
|
||||||
|
// leading "a/" or "b/" prefix (and any "./"), and surrounding whitespace. Applied
|
||||||
|
// to BOTH the diff paths and a finding's file so they match even when a model
|
||||||
|
// writes "./pkg/x.go" or the diff carries the "b/" prefix.
|
||||||
|
func normPath(p string) string {
|
||||||
|
p = strings.TrimSpace(p)
|
||||||
|
p = strings.TrimPrefix(p, "./")
|
||||||
|
if strings.HasPrefix(p, "a/") || strings.HasPrefix(p, "b/") {
|
||||||
|
p = p[2:]
|
||||||
|
}
|
||||||
|
return p
|
||||||
|
}
|
||||||
|
|
||||||
|
func record(out map[string]map[int]bool, file string, line int) {
|
||||||
|
if file == "" {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
if out[file] == nil {
|
||||||
|
out[file] = map[int]bool{}
|
||||||
|
}
|
||||||
|
out[file][line] = true
|
||||||
|
}
|
||||||
|
|
||||||
|
// atoiOr parses s, returning def when s is empty or unparseable. Used for the
|
||||||
|
// optional hunk-length fields (absent => length 1).
|
||||||
|
func atoiOr(s string, def int) int {
|
||||||
|
if s == "" {
|
||||||
|
return def
|
||||||
|
}
|
||||||
|
if n, err := strconv.Atoi(s); err == nil {
|
||||||
|
return n
|
||||||
|
}
|
||||||
|
return def
|
||||||
|
}
|
||||||
|
|
||||||
|
// deletePriorReviews removes our previous inline reviews (matched by the body
|
||||||
|
// marker) so a re-run replaces rather than stacks. Best-effort and quiet.
|
||||||
|
func deletePriorReviews(client *http.Client, base, token string) {
|
||||||
|
const perPage = 50
|
||||||
|
for page := 1; page <= 10; page++ { // bound the scan, but page past 50 so a stale marked review isn't missed
|
||||||
|
req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("%s?limit=%d&page=%d", base, perPage, page), nil)
|
||||||
|
if err != nil {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
req.Header.Set("Authorization", "token "+token)
|
||||||
|
resp, err := client.Do(req)
|
||||||
|
if err != nil {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
if resp.StatusCode < 200 || resp.StatusCode >= 300 {
|
||||||
|
io.Copy(io.Discard, resp.Body)
|
||||||
|
resp.Body.Close()
|
||||||
|
return
|
||||||
|
}
|
||||||
|
var reviews []struct {
|
||||||
|
ID int `json:"id"`
|
||||||
|
Body string `json:"body"`
|
||||||
|
}
|
||||||
|
_ = json.NewDecoder(resp.Body).Decode(&reviews)
|
||||||
|
resp.Body.Close()
|
||||||
|
for _, r := range reviews {
|
||||||
|
if !strings.Contains(r.Body, inlineReviewMarker) {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
dreq, err := http.NewRequest(http.MethodDelete, fmt.Sprintf("%s/%d", base, r.ID), nil)
|
||||||
|
if err != nil {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
dreq.Header.Set("Authorization", "token "+token)
|
||||||
|
if dresp, err := client.Do(dreq); err == nil {
|
||||||
|
io.Copy(io.Discard, dresp.Body)
|
||||||
|
dresp.Body.Close()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if len(reviews) < perPage {
|
||||||
|
return // last page
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// giteaSend marshals payload and sends it with Gitea's "token" auth scheme,
|
||||||
|
// treating a non-2xx response as an error (with a snippet of the body).
|
||||||
|
func giteaSend(client *http.Client, method, url, token string, payload any) error {
|
||||||
|
body, err := json.Marshal(payload)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
req, err := http.NewRequest(method, url, bytes.NewReader(body))
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
req.Header.Set("Content-Type", "application/json")
|
||||||
|
req.Header.Set("Authorization", "token "+token)
|
||||||
|
resp, err := client.Do(req)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
defer resp.Body.Close()
|
||||||
|
snippet, _ := io.ReadAll(io.LimitReader(resp.Body, 2048))
|
||||||
|
if resp.StatusCode < 200 || resp.StatusCode >= 300 {
|
||||||
|
return fmt.Errorf("%s %s: status %d: %s", method, url, resp.StatusCode, strings.TrimSpace(string(snippet)))
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
}
|
||||||
@@ -0,0 +1,107 @@
|
|||||||
|
package main
|
||||||
|
|
||||||
|
import (
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestParseDiffNewLines(t *testing.T) {
|
||||||
|
diff := "diff --git a/a.go b/a.go\n" +
|
||||||
|
"index 111..222 100644\n" +
|
||||||
|
"--- a/a.go\n" +
|
||||||
|
"+++ b/a.go\n" +
|
||||||
|
"@@ -1,3 +1,4 @@\n" +
|
||||||
|
" line1\n" +
|
||||||
|
"-old2\n" +
|
||||||
|
"+new2\n" +
|
||||||
|
"+new3\n" +
|
||||||
|
" line4\n"
|
||||||
|
got := parseDiffNewLines(diff)
|
||||||
|
// Only ADDED lines anchor: new2 (line 2) and new3 (line 3). Context lines 1
|
||||||
|
// and 4 are walked for counting but not recorded.
|
||||||
|
want := map[int]bool{2: true, 3: true}
|
||||||
|
if len(got["a.go"]) != len(want) {
|
||||||
|
t.Fatalf("a.go lines = %v, want %v (added-only)", got["a.go"], want)
|
||||||
|
}
|
||||||
|
for ln := range want {
|
||||||
|
if !got["a.go"][ln] {
|
||||||
|
t.Errorf("expected a.go line %d anchorable", ln)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if got["a.go"][1] || got["a.go"][4] {
|
||||||
|
t.Error("context lines 1/4 should not be anchorable (added-only)")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestParseDiffNewLinesContentLooksLikeHeader(t *testing.T) {
|
||||||
|
// An added line whose CONTENT is "++ weird" appears as "+++ weird" in the
|
||||||
|
// diff. Hunk-length tracking must read it as content, not a file header.
|
||||||
|
diff := "--- a/x\n+++ b/x\n@@ -1,1 +1,2 @@\n ctx\n+++ weird\n"
|
||||||
|
got := parseDiffNewLines(diff)
|
||||||
|
if got["x"][1] { // context line, not recorded (added-only)
|
||||||
|
t.Errorf("line 1 is context, should not anchor: %v", got["x"])
|
||||||
|
}
|
||||||
|
if !got["x"][2] { // the "+++ weird" added line
|
||||||
|
t.Errorf("want added line 2 anchorable, got %v", got["x"])
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestAnchorLineScansSpan(t *testing.T) {
|
||||||
|
// A cluster spanning 10..14 whose min line (10) isn't in the diff but whose
|
||||||
|
// span includes added line 14 must anchor to 14, not be dropped.
|
||||||
|
added := map[string]map[int]bool{"a.go": {14: true}}
|
||||||
|
clusters := []cluster{{file: "a.go", line: 10, maxLine: 14, severity: "high", title: "t", models: set("m1"), lenses: set("x")}}
|
||||||
|
cs := inlineComments(clusters, added)
|
||||||
|
if len(cs) != 1 || cs[0].NewPosition != 14 {
|
||||||
|
t.Fatalf("want anchor at 14 via span scan, got %+v", cs)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestParseDiffNewLinesMultiFile(t *testing.T) {
|
||||||
|
diff := "diff --git a/one.go b/one.go\n--- a/one.go\n+++ b/one.go\n@@ -0,0 +1,2 @@\n+a\n+b\n" +
|
||||||
|
"diff --git a/two.go b/two.go\n--- a/two.go\n+++ b/two.go\n@@ -5,0 +6,1 @@\n+c\n"
|
||||||
|
got := parseDiffNewLines(diff)
|
||||||
|
if !got["one.go"][1] || !got["one.go"][2] {
|
||||||
|
t.Errorf("one.go: want 1,2 got %v", got["one.go"])
|
||||||
|
}
|
||||||
|
if !got["two.go"][6] {
|
||||||
|
t.Errorf("two.go: want 6 got %v", got["two.go"])
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestInlineCommentsNormalizesPaths(t *testing.T) {
|
||||||
|
// Diff path "b/pkg/x.go" -> "pkg/x.go"; a finding written as "./pkg/x.go" must
|
||||||
|
// still anchor (both normalize to the same repo-relative path).
|
||||||
|
addable := parseDiffNewLines("--- a/pkg/x.go\n+++ b/pkg/x.go\n@@ -0,0 +1,1 @@\n+bug\n")
|
||||||
|
clusters := []cluster{{file: "./pkg/x.go", line: 1, severity: "high", title: "t", models: set("m1"), lenses: set("x")}}
|
||||||
|
if cs := inlineComments(clusters, addable); len(cs) != 1 || cs[0].Path != "pkg/x.go" {
|
||||||
|
t.Errorf("path normalization failed to anchor: %+v", cs)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestInlineCommentsFiltersToDiffLines(t *testing.T) {
|
||||||
|
addable := map[string]map[int]bool{"a.go": {10: true, 11: true}}
|
||||||
|
clusters := []cluster{
|
||||||
|
{file: "a.go", line: 10, severity: "high", title: "anchored", models: set("m1", "m2"), lenses: set("security"), detail: "d"},
|
||||||
|
{file: "a.go", line: 99, severity: "high", title: "off-diff line", models: set("m1"), lenses: set("security")},
|
||||||
|
{file: "b.go", line: 1, severity: "high", title: "off-diff file", models: set("m1"), lenses: set("security")},
|
||||||
|
}
|
||||||
|
cs := inlineComments(clusters, addable)
|
||||||
|
if len(cs) != 1 {
|
||||||
|
t.Fatalf("want 1 anchorable inline comment, got %d", len(cs))
|
||||||
|
}
|
||||||
|
if cs[0].Path != "a.go" || cs[0].NewPosition != 10 {
|
||||||
|
t.Errorf("wrong anchor: %+v", cs[0])
|
||||||
|
}
|
||||||
|
if !strings.Contains(cs[0].Body, "anchored") || !strings.Contains(cs[0].Body, "2 model") {
|
||||||
|
t.Errorf("inline body missing title/agreement: %q", cs[0].Body)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func set(xs ...string) map[string]bool {
|
||||||
|
m := map[string]bool{}
|
||||||
|
for _, x := range xs {
|
||||||
|
m[x] = true
|
||||||
|
}
|
||||||
|
return m
|
||||||
|
}
|
||||||
+10
-1
@@ -47,6 +47,8 @@
|
|||||||
# GADFLY_FINDINGS_TOKEN optional bearer token for the gadfly-reports store
|
# GADFLY_FINDINGS_TOKEN optional bearer token for the gadfly-reports store
|
||||||
# GADFLY_CONSOLIDATE cross-model consensus comment: "auto" (default; on for >=2
|
# GADFLY_CONSOLIDATE cross-model consensus comment: "auto" (default; on for >=2
|
||||||
# models), "1" force on, "0" force off (one comment per model)
|
# models), "1" force on, "0" force off (one comment per model)
|
||||||
|
# GADFLY_INLINE_REVIEW when consolidating, also post a COMMENT-state PR review with
|
||||||
|
# inline comments on changed lines (default on; "0" disables)
|
||||||
set -uo pipefail
|
set -uo pipefail
|
||||||
|
|
||||||
# One model by default: the specialist suite already provides breadth, so a
|
# One model by default: the specialist suite already provides breadth, so a
|
||||||
@@ -320,7 +322,14 @@ fi
|
|||||||
if [ "$CONSOLIDATE" = "1" ]; then
|
if [ "$CONSOLIDATE" = "1" ]; then
|
||||||
n_files="$(ls -1 "${FINDINGS_DIR}"/*.json 2>/dev/null | wc -l | tr -d '[:space:]')"
|
n_files="$(ls -1 "${FINDINGS_DIR}"/*.json 2>/dev/null | wc -l | tr -d '[:space:]')"
|
||||||
log "consolidating findings from ${n_files} model(s)"
|
log "consolidating findings from ${n_files} model(s)"
|
||||||
CONSENSUS="$(GADFLY_CONSOLIDATE_DIR="$FINDINGS_DIR" /usr/local/bin/gadfly 2>"${WORKDIR}/consolidate.err" || true)"
|
# Fetch the PR diff so the consolidator can also post an inline PR review,
|
||||||
|
# anchoring findings to changed lines (GADFLY_DIFF_FILE). GITEA_API/GITEA_TOKEN/
|
||||||
|
# GADFLY_PR are already in the binary's environment. Best-effort: an empty diff
|
||||||
|
# just means no inline review.
|
||||||
|
DIFF_FILE="${WORKDIR}/pr.diff"
|
||||||
|
API "${GITEA_API}/pulls/${PR}.diff" > "$DIFF_FILE" 2>/dev/null || true
|
||||||
|
CONSENSUS="$(GADFLY_CONSOLIDATE_DIR="$FINDINGS_DIR" GADFLY_DIFF_FILE="$DIFF_FILE" \
|
||||||
|
/usr/local/bin/gadfly 2>"${WORKDIR}/consolidate.err" || true)"
|
||||||
if [ -n "$CONSENSUS" ]; then
|
if [ -n "$CONSENSUS" ]; then
|
||||||
BODY="$(printf '%s\n\n<sub>Automated adversarial review by Gadfly — consensus across the model swarm. Advisory only — does not block merge.</sub>' "$CONSENSUS")"
|
BODY="$(printf '%s\n\n<sub>Automated adversarial review by Gadfly — consensus across the model swarm. Advisory only — does not block merge.</sub>' "$CONSENSUS")"
|
||||||
upsert_comment_body "<!-- gadfly-consensus -->" "$BODY"
|
upsert_comment_body "<!-- gadfly-consensus -->" "$BODY"
|
||||||
|
|||||||
Reference in New Issue
Block a user