Adding dismiss review type

cmd/pr-bot/main.go

@@ -171,7 +171,7 @@ func downloadOCIArtifacts(svc *prbot.Service, cfg *prbot.Config) {
 func setupReviewer(svc *prbot.Service, cfg *prbot.Config, api gh.API) review.Reviewer {
 	log.Info().Msg("Setting up reviewer")
 	// mutex -> dedup -> precond -> rate limited -> reviewer
-	base := review.NewReviewer(api, svc.Metrics)
+	base := review.NewReviewer(api, svc.Metrics, cfg.GHE.ServiceAccount)
 	throttler := setupThrottlers(svc, cfg)
 	rateLimited := review.NewRateLimitedReviewer(base, api, throttler)
 	precond := review.NewPreCondValidationReviewer(rateLimited)

github/api.go

@@ -45,6 +45,7 @@ More details on PR bot policy evaluation: <a href="%v">link</a>
 type API interface {
 	ListReviews(ctx context.Context, id id.PR) ([]*github.PullRequestReview, error)
 	AddReview(ctx context.Context, id id.PR, summary, event string) error
+	DismissReview(ctx context.Context, id id.PR, reviewID int64, message string) error
 	EnableAutoMerge(ctx context.Context, id id.PR, method githubv4.PullRequestMergeMethod) error
 	IssueComment(ctx context.Context, id id.PR, comment string) error
 	IssueCommentForError(ctx context.Context, id id.PR, err pe.APIError) error

github/github.go

@@ -207,6 +207,19 @@ func (gh *githubDao) AddReview(ctx context.Context, id id.PR, summary, event str
 	return nil
 }
 
+// DismissReview implements API
+func (gh *githubDao) DismissReview(ctx context.Context, id id.PR, reviewID int64, message string) error {
+	_, resp, err := gh.v3.PullRequests.DismissReview(ctx, id.Owner, id.Repo, id.Number, reviewID,
+		&github.PullRequestReviewDismissalRequest{
+			Message: &message,
+		})
+	if err != nil {
+		return err
+	}
+	gh.emitTokenExpiration(ctx, resp)
+	return nil
+}
+
 // IssueCommentForError implements Dao
 func (gh *githubDao) IssueCommentForError(ctx context.Context, id id.PR, apiError pe.APIError) error {
 	b, err := json.MarshalIndent(apiError, "", "  ")

go.mod

@@ -19,6 +19,7 @@ require (
 	github.com/go-chi/httplog v0.3.2
 	github.com/go-chi/render v1.0.3
 	github.com/google/go-github/v50 v50.2.0
+	github.com/google/uuid v1.6.0
 	github.com/ilyakaznacheev/cleanenv v1.5.0
 	github.com/jonboulle/clockwork v0.4.0
 	github.com/mennanov/limiters v1.2.2
@@ -75,7 +76,6 @@ require (
 	github.com/gogo/protobuf v1.3.2 // indirect
 	github.com/golang/protobuf v1.5.4 // indirect
 	github.com/google/go-querystring v1.1.0 // indirect
-	github.com/google/uuid v1.6.0 // indirect
 	github.com/gorilla/mux v1.8.1 // indirect
 	github.com/hashicorp/consul/api v1.26.1 // indirect
 	github.com/hashicorp/errwrap v1.1.0 // indirect

opa/types/result.go

@@ -17,6 +17,7 @@ const (
 	Approve
 	Comment
 	RequestChanges
+	Dismiss
 )
 
 var (
@@ -25,6 +26,7 @@ var (
 		1: "APPROVE",
 		2: "COMMENT",
 		3: "REQUEST_CHANGES",
+		4: "DISMISS",
 	}
 	reviewTypeValues = reverseMap(reviewTypeNames)
 
@@ -33,6 +35,7 @@ var (
 		1: "APPROVED",
 		2: "COMMENTED",
 		3: "CHANGES_REQUESTED",
+		4: "DISMISSED",
 	}
 	reviewStateValues = reverseMap(reviewStateNames)
 )

pullrequest/event_handler.go

@@ -94,6 +94,8 @@ func (eh *eventHandler) EvalAndReview(ctx context.Context, id id.PR, ghe input.G
 		return eh.reviewer.RequestChanges(ctx, id, opaResult.Review.Body)
 	case types.Comment:
 		return eh.reviewer.Comment(ctx, id, opaResult.Review.Body)
+	case types.Dismiss:
+		return eh.reviewer.Dismiss(ctx, id, opaResult.Review.Body)
 	default:
 		oplog.Info().Msg("skipping review")
 	}

pullrequest/review/dedup_reviewer.go

@@ -89,3 +89,7 @@ func (d *DedupReviewer) checkForReview(reviews []*github.PullRequestReview, revi
 	}
 	return false
 }
+
+func (d *DedupReviewer) Dismiss(ctx context.Context, id id.PR, body string) error {
+	return d.delegate.Dismiss(ctx, id, body)
+}

pullrequest/review/mutex_reviewer.go

@@ -94,3 +94,12 @@ func NewMutexReviewer(delegate Reviewer, locker Locker) Reviewer {
 		locker:   locker,
 	}
 }
+
+func (r *mutexReviewer) Dismiss(ctx context.Context, id id.PR, body string) error {
+	lock, err := r.acquireLock(ctx, id)
+	if err != nil {
+		return err
+	}
+	defer r.releaseLock(ctx, lock, id)
+	return r.delegate.Dismiss(ctx, id, body)
+}

pullrequest/review/pre_cond_validation_reviewer.go

@@ -43,3 +43,7 @@ func NewPreCondValidationReviewer(delegate Reviewer) Reviewer {
 		delegate: delegate,
 	}
 }
+
+func (p *preCondValidationReviewer) Dismiss(ctx context.Context, id id.PR, body string) error {
+	return p.delegate.Dismiss(ctx, id, body)
+}

pullrequest/review/rate_limited_reviewer.go

@@ -52,3 +52,7 @@ func NewRateLimitedReviewer(delegate Reviewer, api gh.API, throttler rate.Thrott
 		throttler: throttler,
 	}
 }
+
+func (r *rateLimitedReviewer) Dismiss(ctx context.Context, id id.PR, body string) error {
+	return r.delegate.Dismiss(ctx, id, body)
+}

pullrequest/review/reviewer.go

@@ -28,11 +28,13 @@ type Reviewer interface {
 	Approve(ctx context.Context, id id.PR, body string, opts ApproveOptions) error
 	RequestChanges(ctx context.Context, id id.PR, body string) error
 	Comment(ctx context.Context, id id.PR, body string) error
+	Dismiss(ctx context.Context, id id.PR, body string) error
 }
 
 type reviewer struct {
-	api     gh.API
-	metrics metrics.Emitter
+	api           gh.API
+	metrics       metrics.Emitter
+	serviceAccount string
 }
 
 // Approve implements Reviewer.
@@ -94,8 +96,46 @@ func (r *reviewer) RequestChanges(ctx context.Context, id id.PR, body string) er
 	return nil
 }
 
-func NewReviewer(dao gh.API, metrics metrics.Emitter) Reviewer {
-	return &reviewer{api: dao, metrics: metrics}
+// Dismiss implements Reviewer.
+func (r *reviewer) Dismiss(ctx context.Context, id id.PR, body string) error {
+	oplog := httplog.LogEntry(ctx)
+
+	// Get existing reviews to find CHANGES_REQUESTED reviews to dismiss
+	reviews, err := r.api.ListReviews(ctx, id)
+	if err != nil {
+		oplog.Err(err).Msgf("error listing reviews for PR %v", id.URL)
+		ae := pe.ServiceFault(ctx, "Error listing reviews for PR", err)
+		return ae
+	}
+
+	// Find and dismiss only CHANGES_REQUESTED reviews from the configured service account
+	dismissed := false
+	for _, review := range reviews {
+		if review.GetState() == "CHANGES_REQUESTED" && 
+		   review.GetUser().GetLogin() == r.serviceAccount {
+			err = r.api.DismissReview(ctx, id, review.GetID(), body)
+			if err != nil {
+				oplog.Err(err).Msgf("error dismissing review %d for PR %v", review.GetID(), id.URL)
+				ae := pe.ServiceFault(ctx, "Error dismissing review", err)
+				return ae
+			}
+			dismissed = true
+			oplog.Info().Msgf("dismissed review %d from %s for PR %v", review.GetID(), r.serviceAccount, id.URL)
+		}
+	}
+
+	if dismissed {
+		tags := append(id.ToTags(), fmt.Sprintf("reviewType:%s", "dismiss"))
+		r.metrics.EmitDist(ctx, "reviewedPRs", 1.0, tags)
+		r.metrics.EmitDist(ctx, "dismissedPRs", 1.0, tags)
+		oplog.Info().Msgf("reviewed PR reviewType:dismiss")
+	}
+
+	return nil
+}
+
+func NewReviewer(dao gh.API, metrics metrics.Emitter, serviceAccount string) Reviewer {
+	return &reviewer{api: dao, metrics: metrics, serviceAccount: serviceAccount}
 }
 
 func (r *reviewer) handleAutoMergeError(ctx context.Context, id id.PR, err error) error {

pullrequest/review/reviewer_test.go

@@ -165,7 +165,7 @@ func Test_reviewer_Approve(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			mockAPI := gh.NewMockAPI(t)
 			metrics := metrics.NewNoopEmitter()
-			r := review.NewReviewer(mockAPI, metrics)
+			r := review.NewReviewer(mockAPI, metrics, "test-service-account")
 			tt.args.setExpectations(mockAPI)
 			if err := r.Approve(ctx, tt.args.id, tt.args.body, review.ApproveOptions{
 				MergeMethod: tt.args.mergeMethod,
@@ -220,7 +220,7 @@ func Test_reviewer_Comment(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			mockAPI := gh.NewMockAPI(t)
 			metrics := metrics.NewNoopEmitter()
-			r := review.NewReviewer(mockAPI, metrics)
+			r := review.NewReviewer(mockAPI, metrics, "test-service-account")
 			tt.args.setExpectations(mockAPI)
 			if err := r.Comment(ctx, tt.args.id, tt.args.body); (err != nil) != tt.wantErr {
 				t.Errorf("reviewer.Comment() error = %v, wantErr %v", err, tt.wantErr)
@@ -273,7 +273,7 @@ func Test_reviewer_RequestChanges(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			mockAPI := gh.NewMockAPI(t)
 			metrics := metrics.NewNoopEmitter()
-			r := review.NewReviewer(mockAPI, metrics)
+			r := review.NewReviewer(mockAPI, metrics, "test-service-account")
 			tt.args.setExpectations(mockAPI)
 			if err := r.RequestChanges(ctx, tt.args.id, tt.args.body); (err != nil) != tt.wantErr {
 				t.Errorf("reviewer.RequestChanges() error = %v, wantErr %v", err, tt.wantErr)