homelab/review-bot/review_test.go

package main

import (
	"context"
	"crypto/hmac"
	"crypto/sha256"
	"encoding/hex"
	"encoding/json"
	"io"
	"log/slog"
	"net/http"
	"net/http/httptest"
	"strings"
	"sync"
	"testing"
	"time"
)

// fixture runs the service against fake Gitea and Mistral servers.
type fixture struct {
	t       *testing.T
	gitea   *httptest.Server
	mistral *httptest.Server
	cfg     Config
	rev     *Reviewer
	metrics *Metrics
	status  *Tracker

	mu        sync.Mutex
	prState   string // state of PR 7 on every read after the first
	title     string // title of PR 7
	reviews   string // JSON array returned for PR 7's reviews
	answer    string // JSON returned for a conversation
	posted    []map[string]string
	statuses  []map[string]string // commit statuses set, with "sha" added
	agentCall []string
	agents    string // JSON array returned for the agent list

	convStatus []int    // statuses for the next conversations; 429 simulates the connector rate limit
	answers    []string // answers for the next conversations, then answer
	convCalls  int
	retryAfter string // Retry-After header sent with a simulated 429
	head       string // head commit of PR 7
	pushAfter  string // when set, the next conversation answer is followed by a push of this head
	// breakStreams breaks the next conversation streams off after their first event.
	breakStreams int
	history      string // JSON returned for a conversation's history; "" answers 404
	historyCalls int
	streamError  string // when set, conversations end in a conversation.response.error event

	mistralBackend *Mistral
}

// sse renders a conversation answer ({"outputs":[...]}) as the event stream Mistral sends.
func sse(t *testing.T, answer string, broken bool, streamErr string) string {
	var c struct {
		Outputs []map[string]json.RawMessage `json:"outputs"`
	}
	if err := json.Unmarshal([]byte(answer), &c); err != nil {
		t.Fatalf("bad answer fixture: %v", err)
	}
	var b strings.Builder
	write := func(v map[string]any) {
		data, _ := json.Marshal(v)
		b.WriteString("event: " + v["type"].(string) + "\ndata: " + string(data) + "\n\n")
	}
	write(map[string]any{"type": "conversation.response.started", "conversation_id": "conv_1"})
	if broken {
		return b.String()
	}
	for i, o := range c.Outputs {
		var typ string
		_ = json.Unmarshal(o["type"], &typ)
		switch typ {
		case "tool.execution":
			var name string
			_ = json.Unmarshal(o["name"], &name)
			write(map[string]any{"type": "tool.execution.started", "id": "t", "name": name, "arguments": "{}"})
		case "message.output":
			write(map[string]any{"type": "message.output.delta", "id": "m" + string(rune('0'+i)), "content": o["content"]})
		}
	}
	if streamErr != "" {
		write(map[string]any{"type": "conversation.response.error", "message": streamErr, "code": 3000})
		return b.String()
	}
	write(map[string]any{"type": "conversation.response.done", "usage": map[string]int{"total_tokens": 1}})
	return b.String()
}

const goodAnswer = `{"outputs":[
  {"type":"tool.execution","name":"conn_get_file_contents"},
  {"type":"message.output","content":[{"type":"thinking","thinking":"x"},{"type":"text","text":"I looked.\n\n## Summary\n\nAdds a field.\n\n## 🔴 Blocking\n\n- a.yaml:3 is wrong.\n\n## 🟡 Suggestions\n\nNone.\n\n## Recommendation\n\nRequest changes — 1 blocking issue(s)\n\nVerdict: request-changes"}]}]}`

func newFixture(t *testing.T) *fixture {
	f := &fixture{t: t, prState: "open", title: "feat: x", head: "abc123", reviews: "[]", answer: goodAnswer, agents: "[]"}
	pr := func() string {
		f.mu.Lock()
		defer f.mu.Unlock()
		return `{"number":7,"state":"` + f.prState + `","title":"` + f.title + `","additions":3,"deletions":1,"changed_files":1,` +
			`"html_url":"https://gitea/pr/7","head":{"sha":"` + f.head + `"}}`
	}
	f.gitea = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		if r.Header.Get("Authorization") != "token review-token" {
			http.Error(w, "unauthorized", http.StatusUnauthorized)
			return
		}
		prJSON := pr()
		f.mu.Lock()
		defer f.mu.Unlock()
		switch {
		case r.Method == "GET" && r.URL.Path == "/api/v1/repos/konairius/homelab/pulls/7":
			_, _ = io.WriteString(w, prJSON)
		case r.Method == "GET" && r.URL.Path == "/api/v1/repos/konairius/homelab/pulls/7/reviews":
			_, _ = io.WriteString(w, f.reviews)
		case r.Method == "GET" && r.URL.Path == "/api/v1/repos/konairius/homelab/pulls":
			_, _ = io.WriteString(w, "["+prJSON+"]")
		case r.Method == "POST" && r.URL.Path == "/api/v1/repos/konairius/homelab/pulls/7/reviews":
			var in map[string]string
			_ = json.NewDecoder(r.Body).Decode(&in)
			f.posted = append(f.posted, in)
			w.WriteHeader(http.StatusOK)
			_, _ = io.WriteString(w, `{"html_url":"https://gitea/pr/7#review"}`)
		case r.Method == "POST" && strings.HasPrefix(r.URL.Path, "/api/v1/repos/konairius/homelab/statuses/"):
			var in map[string]string
			_ = json.NewDecoder(r.Body).Decode(&in)
			in["sha"] = strings.TrimPrefix(r.URL.Path, "/api/v1/repos/konairius/homelab/statuses/")
			f.statuses = append(f.statuses, in)
			w.WriteHeader(http.StatusCreated)
			_, _ = io.WriteString(w, "{}")
		default:
			http.NotFound(w, r)
		}
	}))
	f.mistral = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		if r.Header.Get("Authorization") != "Bearer mistral-key" {
			http.Error(w, "unauthorized", http.StatusUnauthorized)
			return
		}
		f.mu.Lock()
		defer f.mu.Unlock()
		switch {
		case r.Method == "GET" && r.URL.Path == "/agents":
			_, _ = io.WriteString(w, f.agents)
		case r.Method == "POST" && r.URL.Path == "/agents":
			f.agentCall = append(f.agentCall, "POST")
			_, _ = io.WriteString(w, `{"id":"ag_new","name":"pr-review","model":"mistral-large-4-0"}`)
		case r.Method == "PATCH" && strings.HasPrefix(r.URL.Path, "/agents/"):
			f.agentCall = append(f.agentCall, "PATCH "+strings.TrimPrefix(r.URL.Path, "/agents/"))
			_, _ = io.WriteString(w, `{"name":"pr-review","model":"mistral-large-4-0"}`)
		case r.Method == "POST" && r.URL.Path == "/conversations":
			var in map[string]any
			_ = json.NewDecoder(r.Body).Decode(&in)
			if in["agent_id"] != "ag_1" || in["stream"] != true || in["store"] != true ||
				!strings.Contains(in["inputs"].(string), "pull request #7 of konairius/homelab") {
				http.Error(w, "unexpected conversation request", http.StatusBadRequest)
				return
			}
			f.convCalls++
			if len(f.convStatus) > 0 {
				status := f.convStatus[0]
				f.convStatus = f.convStatus[1:]
				if f.retryAfter != "" {
					w.Header().Set("Retry-After", f.retryAfter)
				}
				http.Error(w, `{"detail":"Custom connector rate limit reached."}`, status)
				return
			}
			answer := f.answer
			if len(f.answers) > 0 {
				answer, f.answers = f.answers[0], f.answers[1:]
			}
			broken := f.breakStreams > 0
			if broken {
				f.breakStreams--
			}
			w.Header().Set("Content-Type", "text/event-stream")
			_, _ = io.WriteString(w, sse(f.t, answer, broken, f.streamError))
			if f.pushAfter != "" {
				f.head, f.pushAfter = f.pushAfter, ""
			}
		case r.Method == "GET" && r.URL.Path == "/conversations/conv_1/history":
			f.historyCalls++
			if f.history == "" {
				http.NotFound(w, r)
				return
			}
			_, _ = io.WriteString(w, f.history)
		default:
			http.NotFound(w, r)
		}
	}))
	t.Cleanup(f.gitea.Close)
	t.Cleanup(f.mistral.Close)

	f.cfg = Config{
		GiteaURL: f.gitea.URL, BotLogin: "review-bot", ReviewToken: "review-token", WebhookSecret: "hook-secret",
		MistralURL: f.mistral.URL, MistralKey: "mistral-key", AgentName: "pr-review",
		Models:      []Model{{Backend: "mistral", Name: "mistral-large-4-0"}},
		ConnectorID: "c1", Repos: []string{"konairius/homelab"}, Workers: 1, QueueSize: 4,
		ReviewTimeout: 5 * time.Second, RateLimitRetries: 2, RateLimitWait: time.Millisecond,
	}
	f.metrics = newMetrics()
	gitea := &Gitea{base: f.gitea.URL, token: "review-token", http: http.DefaultClient}
	log := slog.New(slog.NewTextHandler(io.Discard, nil))
	f.status = newTracker(gitea, log, f.metrics)
	ctx, cancel := context.WithCancel(context.Background())
	t.Cleanup(cancel)
	go f.status.Run(ctx)
	f.mistralBackend = &Mistral{base: f.mistral.URL, key: "mistral-key", agentPrefix: "pr-review", connectorID: "c1",
		http: http.DefaultClient, recoverTries: 2, recoverWait: time.Millisecond,
		agents: map[string]string{"mistral-large-4-0": "ag_1"}}
	f.rev = &Reviewer{
		cfg:      f.cfg,
		gitea:    gitea,
		backends: map[string]Backend{"mistral": f.mistralBackend},
		log:      log,
		metrics:  f.metrics,
		status:   f.status,
	}
	f.rev.setReady("mistral", true)
	return f
}

// commitStatuses waits for the commit statuses set so far and returns them as "sha state: description".
func (f *fixture) commitStatuses() []string {
	f.status.Flush()
	f.mu.Lock()
	defer f.mu.Unlock()
	var out []string
	for _, s := range f.statuses {
		if s["context"] != "review-bot" {
			f.t.Errorf("status context %q, want review-bot", s["context"])
		}
		out = append(out, s["sha"]+" "+s["state"]+": "+s["description"])
	}
	return out
}

func (f *fixture) postedReviews() []map[string]string {
	f.mu.Lock()
	defer f.mu.Unlock()
	return append([]map[string]string(nil), f.posted...)
}

func TestReviewPostsTheCleanedReport(t *testing.T) {
	f := newFixture(t)
	if err := f.rev.Review(context.Background(), "konairius/homelab", 7); err != nil {
		t.Fatal(err)
	}
	got := f.postedReviews()
	if len(got) != 1 {
		t.Fatalf("posted %d reviews, want 1", len(got))
	}
	p := got[0]
	if p["event"] != "REQUEST_CHANGES" || p["commit_id"] != "abc123" {
		t.Errorf("event/commit: %v", p)
	}
	for _, want := range []string{"model `mistral-large-4-0`", "commit `abc123`", "## 🔴 Blocking", "- a.yaml:3 is wrong.",
		"## Recommendation", "<!-- pr-review:sha=abc123 -->"} {
		if !strings.Contains(p["body"], want) {
			t.Errorf("body lacks %q:\n%s", want, p["body"])
		}
	}
	for _, unwanted := range []string{"I looked.", "Suggestions"} {
		if strings.Contains(p["body"], unwanted) {
			t.Errorf("body should not contain %q", unwanted)
		}
	}
}

func TestReviewSkipsWhatIsAlreadyReviewed(t *testing.T) {
	f := newFixture(t)
	f.reviews = `[{"body":"<!-- pr-review:sha=abc123 -->","user":{"login":"review-bot"}}]`
	if err := f.rev.Review(context.Background(), "konairius/homelab", 7); err != nil {
		t.Fatal(err)
	}
	if n := len(f.postedReviews()); n != 0 {
		t.Errorf("posted %d reviews, want none", n)
	}
}

// A pending review (an event Gitea did not know) is visible only to the bot: the PR is reviewed
// again and the approval goes out as APPROVED.
func TestPendingReviewIsReviewedAgain(t *testing.T) {
	f := newFixture(t)
	f.reviews = `[{"body":"<!-- pr-review:sha=abc123 -->","state":"PENDING","user":{"login":"review-bot"}}]`
	f.answer = `{"outputs":[{"type":"message.output","content":"## Summary\n\nfine\n\n## Recommendation\n\nLooks reasonable.\n\nVerdict: approve"}]}`
	if err := f.rev.Review(context.Background(), "konairius/homelab", 7); err != nil {
		t.Fatal(err)
	}
	posted := f.postedReviews()
	if len(posted) != 1 || posted[0]["event"] != "APPROVED" {
		t.Fatalf("want one APPROVED review, got %v", posted)
	}
}

func TestReviewRefusesWriteToolsAndPostsNothing(t *testing.T) {
	f := newFixture(t)
	f.answer = `{"outputs":[{"type":"tool.execution","name":"conn_create_or_update_file"},
	  {"type":"message.output","content":"## Summary\n\nx"}]}`
	err := f.rev.Review(context.Background(), "konairius/homelab", 7)
	if err == nil || !strings.Contains(err.Error(), "conn_create_or_update_file") {
		t.Fatalf("want an error naming the tool, got %v", err)
	}
	if n := len(f.postedReviews()); n != 0 {
		t.Errorf("posted %d reviews, want none", n)
	}
}

func TestReviewWithoutAReportPostsNothing(t *testing.T) {
	f := newFixture(t)
	f.answer = `{"outputs":[{"type":"message.output","content":"I could not do it."}]}`
	if err := f.rev.Review(context.Background(), "konairius/homelab", 7); err == nil {
		t.Fatal("want an error")
	}
	if n := len(f.postedReviews()); n != 0 {
		t.Errorf("posted %d reviews, want none", n)
	}
}

func TestReviewNeedsTheAgent(t *testing.T) {
	f := newFixture(t)
	f.rev.setReady("mistral", false)
	if err := f.rev.Review(context.Background(), "konairius/homelab", 7); err == nil {
		t.Fatal("want an error while the agent is not ready")
	}
	if f.rev.Ready() {
		t.Error("not ready without an agent")
	}
}

func TestReviewAsksAgainAfterTheRateLimit(t *testing.T) {
	f := newFixture(t)
	f.convStatus = []int{429, 429}
	if err := f.rev.Review(context.Background(), "konairius/homelab", 7); err != nil {
		t.Fatal(err)
	}
	if f.convCalls != 3 || len(f.postedReviews()) != 1 {
		t.Errorf("conversations %d, reviews %d; want 3 and 1", f.convCalls, len(f.postedReviews()))
	}
}

// Mistral's Retry-After replaces the guessed wait: with a guess of ten minutes this only finishes
// within the test's five seconds when the header is honoured.
func TestReviewHonoursRetryAfter(t *testing.T) {
	f := newFixture(t)
	f.rev.cfg.RateLimitWait = 10 * time.Minute
	f.retryAfter = "0"
	f.convStatus = []int{429}
	if err := f.rev.Review(context.Background(), "konairius/homelab", 7); err != nil {
		t.Fatal(err)
	}
	if f.convCalls != 2 || len(f.postedReviews()) != 1 {
		t.Errorf("conversations %d, reviews %d; want 2 and 1", f.convCalls, len(f.postedReviews()))
	}
}

func TestLimitHeaders(t *testing.T) {
	h := http.Header{}
	h.Set("Retry-After", "7")
	h.Set("X-RateLimit-Remaining", "0")
	h.Set("X-Ratelimit-Limit-Tokens-Minute", "100")
	h.Set("Content-Type", "application/json")
	got := limitHeaders(h)
	if len(got) != 3 || got["Retry-After"] != "7" || got["X-Ratelimit-Remaining"] != "0" {
		t.Errorf("unexpected headers: %v", got)
	}
}

func TestReviewGivesUpWhenTheRateLimitStays(t *testing.T) {
	f := newFixture(t)
	f.convStatus = []int{429, 429, 429, 429}
	err := f.rev.Review(context.Background(), "konairius/homelab", 7)
	if err == nil || !strings.Contains(err.Error(), "still failing") {
		t.Fatalf("want a rate limit error, got %v", err)
	}
	if f.convCalls != 3 || len(f.postedReviews()) != 0 {
		t.Errorf("conversations %d, reviews %d; want 3 and 0", f.convCalls, len(f.postedReviews()))
	}
}

func TestOtherErrorsAreNotRetried(t *testing.T) {
	f := newFixture(t)
	f.convStatus = []int{500}
	if err := f.rev.Review(context.Background(), "konairius/homelab", 7); err == nil {
		t.Fatal("want an error")
	}
	if f.convCalls != 1 {
		t.Errorf("conversations %d, want 1", f.convCalls)
	}
}

func TestEnsureAgent(t *testing.T) {
	f := newFixture(t)
	hash := promptHash(reviewPrompt)

	f.agents = "[]"
	a, err := f.mistralBackend.EnsureAgent(context.Background(), "pr-review", "mistral-large-4-0", "c1", reviewPrompt)
	if err != nil || a.ID != "ag_new" {
		t.Fatalf("create: %+v, %v", a, err)
	}

	f.mu.Lock()
	f.agentCall = nil
	f.agents = `[{"id":"ag_9","name":"pr-review","model":"mistral-large-4-0","description":"PR review, read-only Gitea connector. prompt sha256:` + hash + `"}]`
	f.mu.Unlock()
	a, err = f.mistralBackend.EnsureAgent(context.Background(), "pr-review", "mistral-large-4-0", "c1", reviewPrompt)
	if err != nil || a.ID != "ag_9" || len(f.agentCall) != 0 {
		t.Fatalf("unchanged agent must not be touched: %+v, %v, calls %v", a, err, f.agentCall)
	}

	f.mu.Lock()
	f.agents = `[{"id":"ag_9","name":"pr-review","model":"mistral-large-4-0","description":"PR review, read-only Gitea connector. prompt sha256:old"}]`
	f.mu.Unlock()
	a, err = f.mistralBackend.EnsureAgent(context.Background(), "pr-review", "mistral-large-4-0", "c1", reviewPrompt)
	if err != nil || a.ID != "ag_9" || len(f.agentCall) != 1 || f.agentCall[0] != "PATCH ag_9" {
		t.Fatalf("drifted agent must be updated in place: %+v, %v, calls %v", a, err, f.agentCall)
	}
}

func sign(secret string, body []byte) string {
	m := hmac.New(sha256.New, []byte(secret))
	m.Write(body)
	return hex.EncodeToString(m.Sum(nil))
}

func (f *fixture) hook(event, signature string, body string) *httptest.ResponseRecorder {
	q := newQueue(f.rev, 4, f.rev.log, f.metrics)
	s := &Server{cfg: f.cfg, queue: q, reviewer: f.rev, metrics: f.metrics, log: f.rev.log}
	req := httptest.NewRequest("POST", "/hook", strings.NewReader(body))
	req.Header.Set("X-Gitea-Event", event)
	if signature != "" {
		req.Header.Set("X-Gitea-Signature", signature)
	}
	rec := httptest.NewRecorder()
	s.routes().ServeHTTP(rec, req)
	return rec
}

func TestHook(t *testing.T) {
	f := newFixture(t)
	body := `{"action":"opened","number":7,"repository":{"full_name":"konairius/homelab"}}`
	good := sign("hook-secret", []byte(body))
	cases := []struct {
		name, event, sig, body string
		want                   int
	}{
		{"valid", "pull_request", good, body, 202},
		{"missing signature", "pull_request", "", body, 401},
		{"wrong signature", "pull_request", sign("other", []byte(body)), body, 401},
		{"signature that is not hex", "pull_request", "zz", body, 401},
		{"other event", "push", good, body, 200},
		{"closed action", "pull_request", sign("hook-secret", []byte(strings.Replace(body, "opened", "closed", 1))),
			strings.Replace(body, "opened", "closed", 1), 200},
		{"synchronized", "pull_request", sign("hook-secret", []byte(strings.Replace(body, "opened", "synchronized", 1))),
			strings.Replace(body, "opened", "synchronized", 1), 202},
		{"repository that is not configured", "pull_request", sign("hook-secret", []byte(strings.Replace(body, "konairius/homelab", "x/y", 1))),
			strings.Replace(body, "konairius/homelab", "x/y", 1), 200},
		{"payload without a number", "pull_request", sign("hook-secret", []byte(`{"action":"opened","repository":{"full_name":"konairius/homelab"}}`)),
			`{"action":"opened","repository":{"full_name":"konairius/homelab"}}`, 400},
		{"payload that is not json", "pull_request", sign("hook-secret", []byte("nope")), "nope", 400},
	}
	for _, c := range cases {
		t.Run(c.name, func(t *testing.T) {
			if got := f.hook(c.event, c.sig, c.body).Code; got != c.want {
				t.Errorf("status %d, want %d", got, c.want)
			}
		})
	}
}

func TestQueueRunsAReviewAndDeduplicates(t *testing.T) {
	f := newFixture(t)
	q := newQueue(f.rev, 4, f.rev.log, f.metrics)
	if r := q.Enqueue("konairius/homelab", 7, "abc123"); r != "queued" {
		t.Fatalf("first enqueue: %s", r)
	}
	if r := q.Enqueue("konairius/homelab", 7, "abc123"); r != "duplicate" {
		t.Fatalf("second enqueue of the same PR: %s", r)
	}
	q.Start(context.Background(), 1)
	q.Stop(5 * time.Second)
	if n := len(f.postedReviews()); n != 1 {
		t.Errorf("posted %d reviews, want 1", n)
	}
	if r := q.Enqueue("konairius/homelab", 8, ""); r != "full" {
		t.Errorf("a stopped queue takes nothing, got %s", r)
	}
}

func TestSweepQueuesOnlyWhatNeedsAReview(t *testing.T) {
	f := newFixture(t)
	q := newQueue(f.rev, 4, f.rev.log, f.metrics)
	f.rev.Sweep(context.Background(), q)
	if len(q.jobs) != 1 {
		t.Fatalf("queued %d, want 1", len(q.jobs))
	}

	f.mu.Lock()
	f.reviews = `[{"body":"<!-- pr-review:sha=abc123 -->","user":{"login":"review-bot"}}]`
	f.mu.Unlock()
	q2 := newQueue(f.rev, 4, f.rev.log, f.metrics)
	f.rev.Sweep(context.Background(), q2)
	if len(q2.jobs) != 0 {
		t.Errorf("queued %d for an already reviewed PR, want 0", len(q2.jobs))
	}
}

func TestHealthEndpoints(t *testing.T) {
	f := newFixture(t)
	s := &Server{cfg: f.cfg, queue: newQueue(f.rev, 1, f.rev.log, f.metrics), reviewer: f.rev, metrics: f.metrics, log: f.rev.log}
	get := func(path string) int {
		rec := httptest.NewRecorder()
		s.routes().ServeHTTP(rec, httptest.NewRequest("GET", path, nil))
		return rec.Code
	}
	if get("/healthz") != 200 || get("/readyz") != 200 || get("/metrics") != 200 {
		t.Error("healthy service must answer 200 on healthz, readyz and metrics")
	}
	f.rev.setReady("mistral", false)
	if get("/healthz") != 200 || get("/readyz") != 503 {
		t.Error("without an agent the service is alive but not ready")
	}
}

func TestFirstRetryWaitsTheConfiguredTime(t *testing.T) {
	f := newFixture(t)
	f.rev.cfg.RateLimitWait = 300 * time.Millisecond
	f.convStatus = []int{429}
	start := time.Now()
	if err := f.rev.Review(context.Background(), "konairius/homelab", 7); err != nil {
		t.Fatal(err)
	}
	if got := time.Since(start); got < 250*time.Millisecond {
		t.Errorf("the first retry came after %v, want at least the configured 300ms", got)
	}
}

func TestLimitDelay(t *testing.T) {
	limited := func(after time.Duration, has bool) error {
		return &apiError{Status: http.StatusTooManyRequests, RetryAfter: after, HasRetryAfter: has}
	}
	cases := []struct {
		name  string
		retry int
		err   error
		want  time.Duration
	}{
		{"guess, first retry", 1, limited(0, false), time.Minute},
		{"guess grows", 3, limited(0, false), 3 * time.Minute},
		{"guess is capped", 9, limited(0, false), 5 * time.Minute},
		{"Retry-After wins", 1, limited(7*time.Second, true), 7 * time.Second},
		{"Retry-After of zero is honoured", 2, limited(0, true), 0},
		{"Retry-After is capped", 1, limited(time.Hour, true), 5 * time.Minute},
	}
	for _, c := range cases {
		if got := limitDelay(time.Minute, c.retry, c.err); got != c.want {
			t.Errorf("%s: got %v, want %v", c.name, got, c.want)
		}
	}
}

func TestReviewOfAFileAboutRateLimitsIsNotDropped(t *testing.T) {
	f := newFixture(t)
	f.answer = `{"outputs":[
	  {"type":"tool.execution","name":"conn_get_file_contents","info":{"result":"var rateLimitText = rate limit handling"}},
	  {"type":"message.output","content":"## Summary\n\nreviewed\n\n## Recommendation\n\nApprove."}]}`
	if err := f.rev.Review(context.Background(), "konairius/homelab", 7); err != nil {
		t.Fatal(err)
	}
	if f.convCalls != 1 || len(f.postedReviews()) != 1 {
		t.Errorf("conversations %d, reviews %d; want 1 and 1", f.convCalls, len(f.postedReviews()))
	}
}

// A review whose head commit was replaced during the conversation is not posted; the PR is
// reviewed again at the new head instead.
func TestReviewDoesNotPostForAStaleHead(t *testing.T) {
	f := newFixture(t)
	f.pushAfter = "def456"
	if err := f.rev.Review(context.Background(), "konairius/homelab", 7); err != nil {
		t.Fatal(err)
	}
	posted := f.postedReviews()
	if f.convCalls != 2 || len(posted) != 1 {
		t.Fatalf("conversations %d, reviews %d; want 2 and 1", f.convCalls, len(posted))
	}
	if posted[0]["commit_id"] != "def456" || !strings.Contains(posted[0]["body"], "pr-review:sha=def456") {
		t.Errorf("the review is for %q, want def456", posted[0]["commit_id"])
	}
}

// A request that arrives while the review runs may be for a newer head, so it is not lost.
func TestQueueReviewsAgainWhenAskedForDuringTheReview(t *testing.T) {
	f := newFixture(t)
	q := newQueue(f.rev, 4, f.rev.log, f.metrics)
	if r := q.Enqueue("konairius/homelab", 7, "abc123"); r != "queued" {
		t.Fatalf("enqueue: %s", r)
	}
	q.mu.Lock()
	q.running["konairius/homelab#7"] = true // as if a worker had picked it up
	q.mu.Unlock()
	if r := q.Enqueue("konairius/homelab", 7, "abc123"); r != "duplicate" {
		t.Fatalf("enqueue while running: %s", r)
	}
	q.mu.Lock()
	again := q.again["konairius/homelab#7"]
	delete(q.running, "konairius/homelab#7")
	q.mu.Unlock()
	if !again {
		t.Error("the request during the review was forgotten")
	}
}

func TestReviewAsksAgainAfterAGatewayFailure(t *testing.T) {
	for _, status := range []int{502, 503, 504} {
		f := newFixture(t)
		f.convStatus = []int{status}
		if err := f.rev.Review(context.Background(), "konairius/homelab", 7); err != nil {
			t.Fatalf("%d: %v", status, err)
		}
		if f.convCalls != 2 || len(f.postedReviews()) != 1 {
			t.Errorf("%d: conversations %d, reviews %d; want 2 and 1", status, f.convCalls, len(f.postedReviews()))
		}
	}
}