feat: [CODE-4774]: close PR if merge-base is non-unique (#4738)

* c1dd3a merge direct to target sha

* afbe46 close PR if merge-base is non-unique

app/api/controller/pullreq/merge.go

@@ -29,10 +29,12 @@ import (
 	"github.com/harness/gitness/app/services/codeowners"
 	"github.com/harness/gitness/app/services/instrument"
 	"github.com/harness/gitness/app/services/protection"
+	"github.com/harness/gitness/app/services/pullreq"
 	"github.com/harness/gitness/audit"
 	"github.com/harness/gitness/contextutil"
 	"github.com/harness/gitness/errors"
 	"github.com/harness/gitness/git"
+	gitapi "github.com/harness/gitness/git/api"
 	gitenum "github.com/harness/gitness/git/enum"
 	"github.com/harness/gitness/git/sha"
 	gitness_store "github.com/harness/gitness/store"
@@ -304,6 +306,16 @@ func (c *Controller) Merge(
 		}, nil, nil
 	}
 
+	targetBranch, err := c.git.GetBranch(ctx, &git.GetBranchParams{
+		ReadParams: git.ReadParams{RepoUID: targetRepo.GitUID},
+		BranchName: pr.TargetBranch,
+	})
+	if err != nil {
+		return nil, nil, fmt.Errorf("failed to get pull request target branch: %w", err)
+	}
+
+	targetSHA := targetBranch.Branch.SHA
+
 	// we want to complete the merge independent of request cancel - start with new, time restricted context.
 	// TODO: This is a small change to reduce likelihood of dirty state.
 	// We still require a proper solution to handle an application crash or very slow execution times
@@ -357,11 +369,28 @@ func (c *Controller) Merge(
 
 			mergeOutput, err = c.git.Merge(ctx, &git.MergeParams{
 				WriteParams: writeParams,
-				BaseBranch:  pr.TargetBranch,
+				BaseSHA:     targetSHA,
 				HeadSHA:     sourceSHA,
 				Refs:        nil, // update no refs -> no commit will be created
 				Method:      gitenum.MergeMethod(in.Method),
 			})
+			if errors.IsInvalidArgument(err) || gitapi.IsUnrelatedHistoriesError(err) {
+				inClose := pullreq.NonUniqueMergeBaseInput{
+					PullReqStore:      c.pullreqStore,
+					ActivityStore:     c.activityStore,
+					PullReqEvReporter: c.eventReporter,
+					SSEStreamer:       c.sseStreamer,
+				}
+
+				errClose := pullreq.CloseBecauseNonUniqueMergeBase(ctx, inClose, targetSHA, sourceSHA, pr)
+				if errClose != nil {
+					return nil, nil,
+						fmt.Errorf("failed to close pull request after non-unique merge base: %w", errClose)
+				}
+
+				return nil, nil, err
+			}
+
 			if err != nil {
 				return nil, nil, fmt.Errorf("failed merge check with method=%s: %w", in.Method, err)
 			}
@@ -530,7 +559,7 @@ func (c *Controller) Merge(
 	// Update the target branch to the result of the merge.
 	refUpdates = append(refUpdates, git.RefUpdate{
 		Name: refTargetBranch,
-		Old:  sha.SHA{}, // don't care about the current commit SHA of the target branch.
+		Old:  targetSHA,
 		New:  sha.SHA{}, // update to the result of the merge.
 	})
 
@@ -551,7 +580,7 @@ func (c *Controller) Merge(
 	now := time.Now()
 	mergeOutput, err := c.git.Merge(ctx, &git.MergeParams{
 		WriteParams:   targetWriteParams,
-		BaseBranch:    pr.TargetBranch,
+		BaseSHA:       targetSHA,
 		HeadSHA:       sourceSHA,
 		Message:       git.CommitMessage(in.Title, in.Message),
 		Committer:     committer,
@@ -561,6 +590,22 @@ func (c *Controller) Merge(
 		Refs:          refUpdates,
 		Method:        gitenum.MergeMethod(in.Method),
 	})
+	if errors.IsInvalidArgument(err) || gitapi.IsUnrelatedHistoriesError(err) {
+		inClose := pullreq.NonUniqueMergeBaseInput{
+			PullReqStore:      c.pullreqStore,
+			ActivityStore:     c.activityStore,
+			PullReqEvReporter: c.eventReporter,
+			SSEStreamer:       c.sseStreamer,
+		}
+
+		errClose := pullreq.CloseBecauseNonUniqueMergeBase(ctx, inClose, targetSHA, sourceSHA, pr)
+		if errClose != nil {
+			return nil, nil,
+				fmt.Errorf("failed to close pull request after non-unique merge base: %w", errClose)
+		}
+
+		return nil, nil, err
+	}
 	if err != nil {
 		return nil, nil, fmt.Errorf("merge execution failed: %w", err)
 	}

app/services/pullreq/close.go

@@ -0,0 +1,118 @@
+// Copyright 2023 Harness, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package pullreq
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/harness/gitness/app/bootstrap"
+	pullreqevents "github.com/harness/gitness/app/events/pullreq"
+	"github.com/harness/gitness/app/sse"
+	"github.com/harness/gitness/app/store"
+	"github.com/harness/gitness/errors"
+	"github.com/harness/gitness/git/sha"
+	"github.com/harness/gitness/types"
+	"github.com/harness/gitness/types/enum"
+
+	"github.com/gotidy/ptr"
+	"github.com/rs/zerolog/log"
+)
+
+type NonUniqueMergeBaseInput struct {
+	PullReqStore      store.PullReqStore
+	ActivityStore     store.PullReqActivityStore
+	PullReqEvReporter *pullreqevents.Reporter
+	SSEStreamer       sse.Streamer
+}
+
+func CloseBecauseNonUniqueMergeBase(
+	ctx context.Context,
+	in NonUniqueMergeBaseInput,
+	targetSHA sha.SHA,
+	sourceSHA sha.SHA,
+	pr *types.PullReq,
+) error {
+	systemPrincipal := bootstrap.NewSystemServiceSession().Principal
+	systemPrincipalID := systemPrincipal.ID
+
+	var activitySeqMergeBase, activitySeqPRClosed int64
+	pr, err := in.PullReqStore.UpdateOptLock(ctx, pr, func(pr *types.PullReq) error {
+		// to avoid racing conditions with merge
+		if pr.State != enum.PullReqStateOpen {
+			return errPRNotOpen
+		}
+
+		pr.ActivitySeq += 2
+		activitySeqMergeBase = pr.ActivitySeq - 1
+		activitySeqPRClosed = pr.ActivitySeq
+
+		pr.SourceSHA = sourceSHA.String()
+		pr.MergeTargetSHA = ptr.String(targetSHA.String())
+
+		pr.State = enum.PullReqStateClosed
+		pr.MergeSHA = nil
+		pr.MarkAsMergeUnchecked()
+
+		return nil
+	})
+	if errors.Is(err, errPRNotOpen) {
+		return nil
+	}
+	if err != nil {
+		return fmt.Errorf("failed to close pull request after non-unique merge base: %w", err)
+	}
+
+	pr.ActivitySeq = activitySeqMergeBase
+	payloadNonUniqueMergeBase := &types.PullRequestActivityPayloadNonUniqueMergeBase{
+		TargetSHA: targetSHA,
+		SourceSHA: sourceSHA,
+	}
+	_, err = in.ActivityStore.CreateWithPayload(ctx, pr, systemPrincipalID, payloadNonUniqueMergeBase, nil)
+	if err != nil {
+		// non-critical error
+		log.Ctx(ctx).Err(err).Msg("failed to write pull request activity for non-unique merge-base")
+	}
+
+	pr.ActivitySeq = activitySeqPRClosed
+	payloadStateChange := &types.PullRequestActivityPayloadStateChange{
+		Old:      enum.PullReqStateOpen,
+		New:      enum.PullReqStateClosed,
+		OldDraft: pr.IsDraft,
+		NewDraft: pr.IsDraft,
+	}
+	if _, err := in.ActivityStore.CreateWithPayload(ctx, pr, systemPrincipalID, payloadStateChange, nil); err != nil {
+		// non-critical error
+		log.Ctx(ctx).Err(err).Msg(
+			"failed to write pull request activity for pull request closure after non-unique merge-base",
+		)
+	}
+
+	in.PullReqEvReporter.Closed(ctx, &pullreqevents.ClosedPayload{
+		Base: pullreqevents.Base{
+			PullReqID:    pr.ID,
+			SourceRepoID: pr.SourceRepoID,
+			TargetRepoID: pr.TargetRepoID,
+			PrincipalID:  systemPrincipalID,
+			Number:       pr.Number,
+		},
+		SourceSHA:    pr.SourceSHA,
+		SourceBranch: pr.SourceBranch,
+	})
+
+	in.SSEStreamer.Publish(ctx, pr.TargetRepoID, enum.SSETypePullReqUpdated, pr)
+
+	return nil
+}

app/services/pullreq/handlers_branch.go

@@ -16,15 +16,16 @@ package pullreq
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"strconv"
 	"strings"
 
 	gitevents "github.com/harness/gitness/app/events/git"
 	pullreqevents "github.com/harness/gitness/app/events/pullreq"
+	"github.com/harness/gitness/errors"
 	"github.com/harness/gitness/events"
 	"github.com/harness/gitness/git"
+	gitapi "github.com/harness/gitness/git/api"
 	gitenum "github.com/harness/gitness/git/enum"
 	"github.com/harness/gitness/git/sha"
 	gitness_store "github.com/harness/gitness/store"
@@ -169,6 +170,20 @@ func (s *Service) updatePullReqOnBranchUpdate(ctx context.Context,
 			Ref1:       event.Payload.NewSHA,
 			Ref2:       targetSHA.String(),
 		})
+		if errors.IsInvalidArgument(err) || gitapi.IsUnrelatedHistoriesError(err) {
+			in := NonUniqueMergeBaseInput{
+				PullReqStore:      s.pullreqStore,
+				ActivityStore:     s.activityStore,
+				PullReqEvReporter: s.pullreqEvReporter,
+				SSEStreamer:       s.sseStreamer,
+			}
+			err = CloseBecauseNonUniqueMergeBase(ctx, in, targetSHA, newSHA, pr)
+			if err != nil {
+				return fmt.Errorf("failed to close pull request after non-unique merge base: %w", err)
+			}
+
+			return nil
+		}
 		if err != nil {
 			return fmt.Errorf("failed to get merge base after branch update to=%s for PR=%d: %w",
 				event.Payload.NewSHA, pr.Number, err)

types/enum/pullreq.go

@@ -93,6 +93,7 @@ const (
 	PullReqActivityTypeTargetBranchChange      PullReqActivityType = "target-branch-change"
 	PullReqActivityTypeMerge                   PullReqActivityType = "merge"
 	PullReqActivityTypeLabelModify             PullReqActivityType = "label-modify"
+	PullReqActivityTypeNonUniqueMergeBase      PullReqActivityType = "non-unique-merge-base"
 )
 
 var pullReqActivityTypes = sortEnum([]PullReqActivityType{

types/pullreq_activity_payload.go

@@ -18,6 +18,7 @@ import (
 	"errors"
 	"fmt"
 
+	"github.com/harness/gitness/git/sha"
 	"github.com/harness/gitness/types/enum"
 )
 
@@ -232,3 +233,12 @@ type PullRequestActivityLabels struct {
 func (a *PullRequestActivityLabels) ActivityType() enum.PullReqActivityType {
 	return enum.PullReqActivityTypeLabelModify
 }
+
+type PullRequestActivityPayloadNonUniqueMergeBase struct {
+	TargetSHA sha.SHA `json:"target_sha"`
+	SourceSHA sha.SHA `json:"source_sha"`
+}
+
+func (a *PullRequestActivityPayloadNonUniqueMergeBase) ActivityType() enum.PullReqActivityType {
+	return enum.PullReqActivityTypeNonUniqueMergeBase
+}