diff --git a/.gitea/actions/review/action.yml b/.gitea/actions/review/action.yml index f52307c..d005697 100644 --- a/.gitea/actions/review/action.yml +++ b/.gitea/actions/review/action.yml @@ -66,6 +66,10 @@ inputs: description: 'Print review to stdout instead of posting' required: false default: 'false' + update-existing: + description: 'Delete previous review from same bot before posting (default true)' + required: false + default: 'true' runs: using: 'composite' @@ -145,6 +149,7 @@ runs: LLM_TEMPERATURE: ${{ inputs.temperature }} LLM_TIMEOUT: ${{ inputs.timeout }} LLM_PROVIDER: ${{ inputs.llm-provider }} + UPDATE_EXISTING: ${{ inputs.update-existing }} run: | ARGS="" if [ "${{ inputs.dry-run }}" = "true" ]; then diff --git a/cmd/review-bot/main.go b/cmd/review-bot/main.go index 065afca..39865b4 100644 --- a/cmd/review-bot/main.go +++ b/cmd/review-bot/main.go @@ -33,6 +33,7 @@ func main() { patternsRepo := flag.String("patterns-repo", envOrDefault("PATTERNS_REPO", ""), "Repo with language patterns (e.g. rodin/elixir-patterns)") patternsFiles := flag.String("patterns-files", envOrDefault("PATTERNS_FILES", "README.md"), "Comma-separated file paths to fetch from patterns repo") dryRun := flag.Bool("dry-run", false, "Print review to stdout instead of posting") + updateExisting := flag.Bool("update-existing", envOrDefaultBool("UPDATE_EXISTING", true), "Delete previous review from same bot before posting (default true)") llmTemp := flag.Float64("llm-temperature", envOrDefaultFloat("LLM_TEMPERATURE", 0), "LLM temperature (0 = server default)") llmTimeout := flag.Int("llm-timeout", envOrDefaultInt("LLM_TIMEOUT", 300), "LLM request timeout in seconds (default 300)") llmProvider := flag.String("llm-provider", envOrDefault("LLM_PROVIDER", "openai"), "LLM API provider: openai or anthropic") @@ -195,6 +196,29 @@ func main() { return } + // Delete previous reviews from this bot if update-existing is enabled + if *updateExisting { + login, err := giteaClient.GetAuthenticatedUser(ctx) + if err != nil { + log.Printf("Warning: could not determine reviewer identity: %v", err) + } else { + reviews, err := giteaClient.ListReviews(ctx, owner, repoName, prNumber) + if err != nil { + log.Printf("Warning: could not list existing reviews: %v", err) + } else { + for _, r := range reviews { + if r.User.Login == login { + if err := giteaClient.DeleteReview(ctx, owner, repoName, prNumber, r.ID); err != nil { + log.Printf("Warning: could not delete old review %d: %v", r.ID, err) + } else { + log.Printf("Deleted previous review %d", r.ID) + } + } + } + } + } + } + log.Printf("Posting review (event=%s)...", event) if err := giteaClient.PostReview(ctx, owner, repoName, prNumber, event, reviewBody); err != nil { log.Fatalf("Failed to post review: %v", err) @@ -333,3 +357,11 @@ func envOrDefaultInt(key string, defaultVal int) int { } return defaultVal } + +func envOrDefaultBool(key string, defaultVal bool) bool { + v := os.Getenv(key) + if v == "" { + return defaultVal + } + return v == "true" || v == "1" || v == "yes" +} diff --git a/gitea/client.go b/gitea/client.go index 73a4abf..f21b1d5 100644 --- a/gitea/client.go +++ b/gitea/client.go @@ -266,3 +266,78 @@ func (c *Client) GetAllFilesInPath(ctx context.Context, owner, repo, path string } return results, nil } + +// Review represents a pull request review from the Gitea API. +type Review struct { + ID int64 `json:"id"` + User struct { + Login string `json:"login"` + } `json:"user"` + State string `json:"state"` + Stale bool `json:"stale"` +} + +// GetAuthenticatedUser returns the login name of the token's owner. +func (c *Client) GetAuthenticatedUser(ctx context.Context) (string, error) { + reqURL := fmt.Sprintf("%s/api/v1/user", c.baseURL) + body, err := c.doGet(ctx, reqURL) + if err != nil { + return "", fmt.Errorf("get authenticated user: %w", err) + } + var user struct { + Login string `json:"login"` + } + if err := json.Unmarshal(body, &user); err != nil { + return "", fmt.Errorf("parse user response: %w", err) + } + if user.Login == "" { + return "", fmt.Errorf("empty login in user response") + } + return user.Login, nil +} + +// ListReviews returns all reviews on a pull request. +func (c *Client) ListReviews(ctx context.Context, owner, repo string, number int) ([]Review, error) { + reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/reviews", + c.baseURL, + url.PathEscape(owner), + url.PathEscape(repo), + number) + body, err := c.doGet(ctx, reqURL) + if err != nil { + return nil, fmt.Errorf("list reviews: %w", err) + } + var reviews []Review + if err := json.Unmarshal(body, &reviews); err != nil { + return nil, fmt.Errorf("parse reviews: %w", err) + } + return reviews, nil +} + +// DeleteReview deletes a review by ID. The token must belong to the review author. +func (c *Client) DeleteReview(ctx context.Context, owner, repo string, number int, reviewID int64) error { + reqURL := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls/%d/reviews/%d", + c.baseURL, + url.PathEscape(owner), + url.PathEscape(repo), + number, + reviewID) + + req, err := http.NewRequestWithContext(ctx, http.MethodDelete, reqURL, nil) + if err != nil { + return fmt.Errorf("create delete request: %w", err) + } + req.Header.Set("Authorization", "token "+c.token) + + resp, err := c.http.Do(req) + if err != nil { + return fmt.Errorf("delete review: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + respBody, _ := io.ReadAll(resp.Body) + return fmt.Errorf("delete review failed (status %d): %s", resp.StatusCode, string(respBody)) + } + return nil +} diff --git a/gitea/client_test.go b/gitea/client_test.go index 24f60f8..fc93473 100644 --- a/gitea/client_test.go +++ b/gitea/client_test.go @@ -318,3 +318,99 @@ func TestEscapePath(t *testing.T) { }) } } + +func TestGetAuthenticatedUser(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/v1/user" { + t.Errorf("unexpected path: %s", r.URL.Path) + } + if r.Method != "GET" { + t.Errorf("expected GET, got %s", r.Method) + } + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"login":"review-bot","id":42}`)) + })) + defer server.Close() + + client := NewClient(server.URL, "test-token") + login, err := client.GetAuthenticatedUser(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if login != "review-bot" { + t.Errorf("expected login %q, got %q", "review-bot", login) + } +} + +func TestGetAuthenticatedUser_EmptyLogin(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"login":"","id":0}`)) + })) + defer server.Close() + + client := NewClient(server.URL, "test-token") + _, err := client.GetAuthenticatedUser(context.Background()) + if err == nil { + t.Fatal("expected error for empty login, got nil") + } +} + +func TestListReviews(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/v1/repos/owner/repo/pulls/5/reviews" { + t.Errorf("unexpected path: %s", r.URL.Path) + } + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`[{"id":10,"user":{"login":"bot-a"},"state":"APPROVED","stale":false},{"id":11,"user":{"login":"bot-b"},"state":"REQUEST_CHANGES","stale":true}]`)) + })) + defer server.Close() + + client := NewClient(server.URL, "test-token") + reviews, err := client.ListReviews(context.Background(), "owner", "repo", 5) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(reviews) != 2 { + t.Fatalf("expected 2 reviews, got %d", len(reviews)) + } + if reviews[0].User.Login != "bot-a" { + t.Errorf("expected bot-a, got %s", reviews[0].User.Login) + } + if reviews[1].ID != 11 { + t.Errorf("expected id 11, got %d", reviews[1].ID) + } +} + +func TestDeleteReview(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/v1/repos/owner/repo/pulls/5/reviews/10" { + t.Errorf("unexpected path: %s", r.URL.Path) + } + if r.Method != "DELETE" { + t.Errorf("expected DELETE, got %s", r.Method) + } + w.WriteHeader(http.StatusNoContent) + })) + defer server.Close() + + client := NewClient(server.URL, "test-token") + err := client.DeleteReview(context.Background(), "owner", "repo", 5, 10) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestDeleteReview_Forbidden(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusForbidden) + w.Write([]byte(`{"message":"forbidden"}`)) + })) + defer server.Close() + + client := NewClient(server.URL, "test-token") + err := client.DeleteReview(context.Background(), "owner", "repo", 5, 10) + if err == nil { + t.Fatal("expected error for 403, got nil") + } +}