Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions internal/agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,12 @@ func resolveAlias(name string) string {
return name
}

// CanonicalName resolves an agent alias to its canonical name.
// Returns the name unchanged if it is not an alias.
func CanonicalName(name string) string {
return resolveAlias(name)
}

// Register adds an agent to the registry
func Register(a Agent) {
registry[a.Name()] = a
Expand Down
124 changes: 111 additions & 13 deletions internal/daemon/ci_poller.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"log"
"os"
"os/exec"
"sort"
"strings"
"sync"
"time"
Expand Down Expand Up @@ -853,20 +854,34 @@ func (p *CIPoller) postBatchResults(batch *storage.CIPRBatch) {
}

// Set commit status based on job outcomes:
// all succeeded → success
// mixed → failure (some reviews did not complete)
// all failed → error
// all succeeded → success
// all failures are quota skips → success (with note)
// mixed real failures → failure
// all failed (real) → error
quotaSkips := countQuotaFailures(reviews)
realFailures := batch.FailedJobs - quotaSkips
statusState := "success"
statusDesc := "Review complete"
switch {
case batch.CompletedJobs == 0 && realFailures == 0 && quotaSkips > 0:
// All failures are quota skips — not the code's fault
statusDesc = fmt.Sprintf(
"Review complete (%d agent(s) skipped — quota)",
quotaSkips,
)
case batch.CompletedJobs == 0:
statusState = "error"
statusDesc = "All reviews failed"
case batch.FailedJobs > 0:
case realFailures > 0:
statusState = "failure"
statusDesc = fmt.Sprintf(
"Review complete (%d/%d jobs failed)",
batch.FailedJobs, batch.TotalJobs,
realFailures, batch.TotalJobs,
)
case quotaSkips > 0:
statusDesc = fmt.Sprintf(
"Review complete (%d agent(s) skipped — quota)",
quotaSkips,
)
}
if err := p.callSetCommitStatus(batch.GithubRepo, batch.HeadSHA, statusState, statusDesc); err != nil {
Expand Down Expand Up @@ -1120,11 +1135,15 @@ Rules:

for i, r := range reviews {
fmt.Fprintf(&b, "---\n### Review %d: Agent=%s, Type=%s", i+1, r.Agent, r.ReviewType)
if r.Status == "failed" {
if isQuotaFailure(r) {
b.WriteString(" [SKIPPED]")
} else if r.Status == "failed" {
b.WriteString(" [FAILED]")
}
b.WriteString("\n")
if r.Output != "" {
if isQuotaFailure(r) {
b.WriteString("(review skipped — agent quota exhausted)")
} else if r.Output != "" {
output := r.Output
if len(output) > maxPerReview {
output = output[:maxPerReview] + "\n\n...(truncated)"
Expand Down Expand Up @@ -1167,6 +1186,10 @@ func formatSynthesizedComment(output string, reviews []storage.BatchReviewResult
fmt.Fprintf(&b, "\n\n---\n*Synthesized from %d reviews (agents: %s | types: %s)*\n",
len(reviews), strings.Join(agents, ", "), strings.Join(types, ", "))

if note := skippedAgentNote(reviews); note != "" {
b.WriteString(note)
}

return b.String()
}

Expand All @@ -1178,9 +1201,15 @@ func formatRawBatchComment(reviews []storage.BatchReviewResult, headSHA string)
b.WriteString("> Synthesis unavailable. Showing raw review outputs.\n\n")

for _, r := range reviews {
summary := fmt.Sprintf("Agent: %s | Type: %s | Status: %s", r.Agent, r.ReviewType, r.Status)
status := r.Status
if isQuotaFailure(r) {
status = "skipped (quota)"
}
summary := fmt.Sprintf("Agent: %s | Type: %s | Status: %s", r.Agent, r.ReviewType, status)
fmt.Fprintf(&b, "<details>\n<summary>%s</summary>\n\n", summary)
if r.Status == "failed" {
if isQuotaFailure(r) {
b.WriteString("Review skipped — agent quota exhausted.\n")
} else if r.Status == "failed" {
b.WriteString("**Error:** Review failed. Check daemon logs for details.\n")
} else if r.Output != "" {
output := r.Output
Expand All @@ -1195,24 +1224,93 @@ func formatRawBatchComment(reviews []storage.BatchReviewResult, headSHA string)
b.WriteString("\n\n</details>\n\n")
}

if note := skippedAgentNote(reviews); note != "" {
b.WriteString(note)
}

return b.String()
}

// formatAllFailedComment formats a comment when every job in a batch failed.
func formatAllFailedComment(reviews []storage.BatchReviewResult, headSHA string) string {
quotaSkips := countQuotaFailures(reviews)
allQuota := len(reviews) > 0 && quotaSkips == len(reviews)

var b strings.Builder
fmt.Fprintf(&b, "## roborev: Review Failed (`%s`)\n\n", shortSHA(headSHA))
b.WriteString("All review jobs in this batch failed.\n\n")
if allQuota {
fmt.Fprintf(&b, "## roborev: Review Skipped (`%s`)\n\n", shortSHA(headSHA))
b.WriteString("All review agents were skipped due to quota exhaustion.\n\n")
} else {
fmt.Fprintf(&b, "## roborev: Review Failed (`%s`)\n\n", shortSHA(headSHA))
b.WriteString("All review jobs in this batch failed.\n\n")
}

for _, r := range reviews {
fmt.Fprintf(&b, "- **%s** (%s): failed\n", r.Agent, r.ReviewType)
if isQuotaFailure(r) {
fmt.Fprintf(&b, "- **%s** (%s): skipped (quota)\n", r.Agent, r.ReviewType)
} else {
fmt.Fprintf(&b, "- **%s** (%s): failed\n", r.Agent, r.ReviewType)
}
}

b.WriteString("\nCheck daemon logs for error details.")
if !allQuota {
b.WriteString("\nCheck daemon logs for error details.")
}

if note := skippedAgentNote(reviews); note != "" {
b.WriteString(note)
}

return b.String()
}

// isQuotaFailure returns true if a batch review's error indicates a quota
// skip rather than a real failure. Matches the prefix set by worker.go.
func isQuotaFailure(r storage.BatchReviewResult) bool {
return r.Status == "failed" && strings.HasPrefix(r.Error, quotaErrorPrefix)
}

// countQuotaFailures returns the number of reviews that failed due to
// agent quota exhaustion rather than a real error.
func countQuotaFailures(reviews []storage.BatchReviewResult) int {
n := 0
for _, r := range reviews {
if isQuotaFailure(r) {
n++
}
}
return n
}

// skippedAgentNote returns a markdown note listing agents that were
// skipped due to quota exhaustion. Returns "" if none were skipped.
func skippedAgentNote(reviews []storage.BatchReviewResult) string {
agents := make(map[string]struct{})
for _, r := range reviews {
if isQuotaFailure(r) {
agents[r.Agent] = struct{}{}
}
}
if len(agents) == 0 {
return ""
}
names := make([]string, 0, len(agents))
for a := range agents {
names = append(names, a)
}
sort.Strings(names)
if len(names) == 1 {
return fmt.Sprintf(
"\n*Note: %s review skipped (agent quota exhausted)*\n",
names[0],
)
}
return fmt.Sprintf(
"\n*Note: %s reviews skipped (agent quota exhausted)*\n",
strings.Join(names, ", "),
)
}

// shortSHA returns the first 8 characters of a SHA, or the full string if shorter.
func shortSHA(sha string) string {
if len(sha) > 8 {
Expand Down
127 changes: 127 additions & 0 deletions internal/daemon/ci_poller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1550,6 +1550,133 @@ func TestCIPollerPostBatchResults_SetsFailureStatusOnMixedOutcome(t *testing.T)
}
}

func TestCIPollerPostBatchResults_QuotaSkippedNotFailure(t *testing.T) {
h := newCIPollerHarness(t, "git@github.com:acme/api.git")

// One success, one quota-skipped
batch, jobs := h.seedBatchWithJobs(t, 70, "quota-sha",
jobSpec{Agent: "codex", ReviewType: "security", Status: "done", Output: "No issues found."},
jobSpec{Agent: "gemini", ReviewType: "security", Status: "failed", Error: quotaErrorPrefix + "gemini quota exhausted"},
)

capturedStatuses := h.CaptureCommitStatuses()
h.CaptureComments()

// Simulate: job1 succeeded, job2 failed (quota)
h.Poller.handleBatchJobDone(batch, jobs[0].ID, true)
h.Poller.handleBatchJobDone(batch, jobs[1].ID, false)

if len(*capturedStatuses) != 1 {
t.Fatalf("expected 1 status call, got %d", len(*capturedStatuses))
}
sc := (*capturedStatuses)[0]
// Quota skip with at least one success → success, not failure
if sc.State != "success" {
t.Errorf("state=%q, want success (quota skip not a failure)", sc.State)
}
if !strings.Contains(sc.Desc, "skipped") {
t.Errorf("desc=%q, expected mention of skipped", sc.Desc)
}
}

func TestCIPollerPostBatchResults_AllQuotaSkippedIsSuccess(t *testing.T) {
h := newCIPollerHarness(t, "git@github.com:acme/api.git")

batch, jobs := h.seedBatchWithJobs(t, 71, "all-quota-sha",
jobSpec{Agent: "gemini", ReviewType: "security", Status: "failed", Error: quotaErrorPrefix + "gemini quota exhausted"},
)

capturedStatuses := h.CaptureCommitStatuses()
h.CaptureComments()

h.Poller.handleBatchJobDone(batch, jobs[0].ID, false)

if len(*capturedStatuses) != 1 {
t.Fatalf("expected 1 status call, got %d", len(*capturedStatuses))
}
sc := (*capturedStatuses)[0]
if sc.State != "success" {
t.Errorf("state=%q, want success (all-quota batch is not an error)", sc.State)
}
if !strings.Contains(sc.Desc, "skipped") {
t.Errorf("desc=%q, expected mention of skipped", sc.Desc)
}
}

func TestCIPollerPostBatchResults_MixedQuotaAndRealFailure(t *testing.T) {
h := newCIPollerHarness(t, "git@github.com:acme/api.git")

batch, jobs := h.seedBatchWithJobs(t, 72, "mixed-quota-sha",
jobSpec{Agent: "codex", ReviewType: "security", Status: "failed", Error: "timeout"},
jobSpec{Agent: "gemini", ReviewType: "security", Status: "failed", Error: quotaErrorPrefix + "quota exhausted"},
)

capturedStatuses := h.CaptureCommitStatuses()
h.CaptureComments()

h.Poller.handleBatchJobDone(batch, jobs[0].ID, false)
h.Poller.handleBatchJobDone(batch, jobs[1].ID, false)

if len(*capturedStatuses) != 1 {
t.Fatalf("expected 1 status call, got %d", len(*capturedStatuses))
}
sc := (*capturedStatuses)[0]
// Real failure + quota skip → error (CompletedJobs == 0 and realFailures > 0)
if sc.State != "error" {
t.Errorf("state=%q, want error (real failure present)", sc.State)
}
}

func TestFormatAllFailedComment_AllQuotaSkipped(t *testing.T) {
reviews := []storage.BatchReviewResult{
{JobID: 1, Agent: "gemini", ReviewType: "security", Status: "failed", Error: quotaErrorPrefix + "quota exhausted"},
}

comment := formatAllFailedComment(reviews, "abc123def456")

assertContainsAll(t, comment, "comment",
"## roborev: Review Skipped",
"quota exhaustion",
"skipped (quota)",
)
if strings.Contains(comment, "Check daemon logs") {
t.Error("all-quota comment should not mention daemon logs")
}
}

func TestFormatRawBatchComment_QuotaSkippedNote(t *testing.T) {
reviews := []storage.BatchReviewResult{
{JobID: 1, Agent: "codex", ReviewType: "security", Output: "Finding A", Status: "done"},
{JobID: 2, Agent: "gemini", ReviewType: "security", Status: "failed", Error: quotaErrorPrefix + "quota exhausted"},
}

comment := formatRawBatchComment(reviews, "abc123def456")

assertContainsAll(t, comment, "comment",
"skipped (quota)",
"gemini review skipped",
)
}

func TestBuildSynthesisPrompt_QuotaSkippedLabel(t *testing.T) {
reviews := []storage.BatchReviewResult{
{JobID: 1, Agent: "codex", ReviewType: "security", Output: "No issues.", Status: "done"},
{JobID: 2, Agent: "gemini", ReviewType: "security", Status: "failed", Error: quotaErrorPrefix + "quota exhausted"},
}

prompt := buildSynthesisPrompt(reviews, "")

assertContainsAll(t, prompt, "prompt",
"[SKIPPED]",
"review skipped",
)
// Should NOT contain [FAILED] for the quota-skipped review
// Count occurrences of [FAILED]
if strings.Contains(prompt, "[FAILED]") {
t.Error("quota-skipped review should use [SKIPPED], not [FAILED]")
}
}

func TestCIPollerPostBatchResults_SetsErrorStatusOnCommentPostFailure(t *testing.T) {
h := newCIPollerHarness(t, "git@github.com:acme/api.git")
batch, _ := h.seedBatchWithJobs(t, 63, "post-fail-sha", jobSpec{
Expand Down
Loading
Loading