feat: delete previous review before posting new one (#6)
Before posting a review, the bot now: 1. Calls GET /api/v1/user to identify its own login 2. Lists all reviews on the PR 3. Deletes any existing reviews from itself 4. Posts the fresh review This keeps PR threads clean — one review per bot at any time. New Gitea client methods: - GetAuthenticatedUser() — token self-identification - ListReviews() — fetch reviews on a PR - DeleteReview() — delete a review by ID Flag: --update-existing / UPDATE_EXISTING (default true) Set to false to preserve old behavior (stack reviews). All delete failures are non-fatal (logged as warnings). Closes #6
This commit is contained in:
@@ -66,6 +66,10 @@ inputs:
|
|||||||
description: 'Print review to stdout instead of posting'
|
description: 'Print review to stdout instead of posting'
|
||||||
required: false
|
required: false
|
||||||
default: 'false'
|
default: 'false'
|
||||||
|
update-existing:
|
||||||
|
description: 'Delete previous review from same bot before posting (default true)'
|
||||||
|
required: false
|
||||||
|
default: 'true'
|
||||||
|
|
||||||
runs:
|
runs:
|
||||||
using: 'composite'
|
using: 'composite'
|
||||||
@@ -145,6 +149,7 @@ runs:
|
|||||||
LLM_TEMPERATURE: ${{ inputs.temperature }}
|
LLM_TEMPERATURE: ${{ inputs.temperature }}
|
||||||
LLM_TIMEOUT: ${{ inputs.timeout }}
|
LLM_TIMEOUT: ${{ inputs.timeout }}
|
||||||
LLM_PROVIDER: ${{ inputs.llm-provider }}
|
LLM_PROVIDER: ${{ inputs.llm-provider }}
|
||||||
|
UPDATE_EXISTING: ${{ inputs.update-existing }}
|
||||||
run: |
|
run: |
|
||||||
ARGS=""
|
ARGS=""
|
||||||
if [ "${{ inputs.dry-run }}" = "true" ]; then
|
if [ "${{ inputs.dry-run }}" = "true" ]; then
|
||||||
|
|||||||
@@ -33,6 +33,7 @@ func main() {
|
|||||||
patternsRepo := flag.String("patterns-repo", envOrDefault("PATTERNS_REPO", ""), "Repo with language patterns (e.g. rodin/elixir-patterns)")
|
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")
|
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")
|
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)")
|
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)")
|
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")
|
llmProvider := flag.String("llm-provider", envOrDefault("LLM_PROVIDER", "openai"), "LLM API provider: openai or anthropic")
|
||||||
@@ -195,6 +196,29 @@ func main() {
|
|||||||
return
|
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)
|
log.Printf("Posting review (event=%s)...", event)
|
||||||
if err := giteaClient.PostReview(ctx, owner, repoName, prNumber, event, reviewBody); err != nil {
|
if err := giteaClient.PostReview(ctx, owner, repoName, prNumber, event, reviewBody); err != nil {
|
||||||
log.Fatalf("Failed to post review: %v", err)
|
log.Fatalf("Failed to post review: %v", err)
|
||||||
@@ -333,3 +357,11 @@ func envOrDefaultInt(key string, defaultVal int) int {
|
|||||||
}
|
}
|
||||||
return defaultVal
|
return defaultVal
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func envOrDefaultBool(key string, defaultVal bool) bool {
|
||||||
|
v := os.Getenv(key)
|
||||||
|
if v == "" {
|
||||||
|
return defaultVal
|
||||||
|
}
|
||||||
|
return v == "true" || v == "1" || v == "yes"
|
||||||
|
}
|
||||||
|
|||||||
@@ -266,3 +266,78 @@ func (c *Client) GetAllFilesInPath(ctx context.Context, owner, repo, path string
|
|||||||
}
|
}
|
||||||
return results, nil
|
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
|
||||||
|
}
|
||||||
|
|||||||
@@ -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")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user