From 6eadd70aef79ee8f7fe3a86de06eb1f71f93b6e4 Mon Sep 17 00:00:00 2001 From: Virgil Date: Wed, 1 Apr 2026 15:47:48 +0000 Subject: [PATCH] feat(agentic): support both review queue mode Co-Authored-By: Virgil --- pkg/agentic/review_queue.go | 76 +++++++++++++++++++------------- pkg/agentic/review_queue_test.go | 38 ++++++++++++++++ 2 files changed, 83 insertions(+), 31 deletions(-) diff --git a/pkg/agentic/review_queue.go b/pkg/agentic/review_queue.go index a2b07e7..c224e64 100644 --- a/pkg/agentic/review_queue.go +++ b/pkg/agentic/review_queue.go @@ -56,10 +56,22 @@ func compileRetryAfterPattern() *regexp.Regexp { func (s *PrepSubsystem) registerReviewQueueTool(server *mcp.Server) { mcp.AddTool(server, &mcp.Tool{ Name: "agentic_review_queue", - Description: "Process the CodeRabbit review queue. Runs local CodeRabbit review on repos, auto-merges clean ones on GitHub, dispatches fix agents for findings. Respects rate limits.", + Description: "Process the review queue. Supports coderabbit, codex, or both reviewers, auto-merges clean ones on GitHub, dispatches fix agents for findings, and respects rate limits.", }, s.reviewQueue) } +// reviewers := reviewQueueReviewers("both") +func reviewQueueReviewers(reviewer string) []string { + switch core.Lower(core.Trim(reviewer)) { + case "codex": + return []string{"codex"} + case "both": + return []string{"codex", "coderabbit"} + default: + return []string{"coderabbit"} + } +} + // result := c.Command("pr-manage").Run(ctx, core.NewOptions( // // core.Option{Key: "limit", Value: 4}, @@ -118,35 +130,35 @@ func (s *PrepSubsystem) reviewQueue(ctx context.Context, _ *mcp.CallToolRequest, var rateInfo *RateLimitInfo for _, repo := range candidates { - if len(processed) >= limit { - skipped = append(skipped, core.Concat(repo, " (limit reached)")) - continue - } - - if rateInfo != nil && rateInfo.Limited && time.Now().Before(rateInfo.RetryAt) { - skipped = append(skipped, core.Concat(repo, " (rate limited)")) - continue - } - repoDir := core.JoinPath(basePath, repo) - reviewer := input.Reviewer - if reviewer == "" { - reviewer = "coderabbit" - } - result := s.reviewRepo(ctx, repoDir, repo, reviewer, input.DryRun, input.LocalOnly) - - if result.Verdict == "rate_limited" { - retryAfter := parseRetryAfter(result.Detail) - rateInfo = &RateLimitInfo{ - Limited: true, - RetryAt: time.Now().Add(retryAfter), - Message: result.Detail, + for _, reviewer := range reviewQueueReviewers(input.Reviewer) { + if len(processed) >= limit { + skipped = append(skipped, core.Concat(repo, " (limit reached)")) + break } - skipped = append(skipped, core.Concat(repo, " (rate limited: ", retryAfter.String(), ")")) - continue - } - processed = append(processed, result) + if reviewer == "coderabbit" && rateInfo != nil && rateInfo.Limited && time.Now().Before(rateInfo.RetryAt) { + skipped = append(skipped, core.Concat(repo, " (rate limited)")) + continue + } + + result := s.reviewRepo(ctx, repoDir, repo, reviewer, input.DryRun, input.LocalOnly) + + if result.Verdict == "rate_limited" { + if reviewer == "coderabbit" { + retryAfter := parseRetryAfter(result.Detail) + rateInfo = &RateLimitInfo{ + Limited: true, + RetryAt: time.Now().Add(retryAfter), + Message: result.Detail, + } + skipped = append(skipped, core.Concat(repo, " (rate limited: ", retryAfter.String(), ")")) + } + continue + } + + processed = append(processed, result) + } } if rateInfo != nil { @@ -187,10 +199,12 @@ func (s *PrepSubsystem) reviewRepo(ctx context.Context, repoDir, repo, reviewer result := ReviewResult{Repo: repo} process := s.Core().Process() - if rl := s.loadRateLimitState(); rl != nil && rl.Limited && time.Now().Before(rl.RetryAt) { - result.Verdict = "rate_limited" - result.Detail = core.Sprintf("retry after %s", rl.RetryAt.Format(time.RFC3339)) - return result + if reviewer != "codex" { + if rl := s.loadRateLimitState(); rl != nil && rl.Limited && time.Now().Before(rl.RetryAt) { + result.Verdict = "rate_limited" + result.Detail = core.Sprintf("retry after %s", rl.RetryAt.Format(time.RFC3339)) + return result + } } if reviewer == "" { diff --git a/pkg/agentic/review_queue_test.go b/pkg/agentic/review_queue_test.go index a589b05..7c38425 100644 --- a/pkg/agentic/review_queue_test.go +++ b/pkg/agentic/review_queue_test.go @@ -3,10 +3,15 @@ package agentic import ( + "context" + "os" + "path/filepath" "testing" "time" + core "dappco.re/go/core" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // --- countFindings (extended beyond paths_test.go) --- @@ -50,3 +55,36 @@ func TestReviewqueue_ParseRetryAfter_Bad_EmptyMessage(t *testing.T) { d := parseRetryAfter("") assert.Equal(t, 5*time.Minute, d) } + +func TestReviewqueue_ReviewQueueReviewers_Good_Both(t *testing.T) { + assert.Equal(t, []string{"codex", "coderabbit"}, reviewQueueReviewers("both")) +} + +func TestReviewqueue_ReviewRepo_Good_CodexBypassesCodeRabbitRateLimit(t *testing.T) { + home := t.TempDir() + t.Setenv("CORE_HOME", home) + + ratePath := filepath.Join(home, ".core", "coderabbit-ratelimit.json") + fs.EnsureDir(filepath.Dir(ratePath)) + fs.Write(ratePath, core.JSONMarshalString(&RateLimitInfo{ + Limited: true, + RetryAt: time.Now().Add(time.Hour), + Message: "rate limited", + })) + + binDir := t.TempDir() + scriptPath := filepath.Join(binDir, "codex") + require.NoError(t, os.WriteFile(scriptPath, []byte("#!/bin/sh\necho 'No findings'\n"), 0o755)) + t.Setenv("PATH", binDir+string(os.PathListSeparator)+os.Getenv("PATH")) + + s := &PrepSubsystem{ + ServiceRuntime: core.NewServiceRuntime(testCore, AgentOptions{}), + backoff: make(map[string]time.Time), + failCount: make(map[string]int), + } + + result := s.reviewRepo(context.Background(), t.TempDir(), "repo", "codex", true, true) + + assert.Equal(t, "clean", result.Verdict) + assert.Equal(t, "skipped (dry run)", result.Action) +}