From 435b46d5adac547539139ff7b1c61f844b8f6706 Mon Sep 17 00:00:00 2001
From: Dane Schneider <dane@envkey.com>
Date: Wed, 8 May 2024 10:32:20 -0700
Subject: [PATCH] prevent (harmless) race condition in git repo when clearing
 uncommitted changes after stopping a plan

---
 app/server/db/git.go              |  5 +++++
 app/server/handlers/plans_exec.go | 20 +++++++++++++++-----
 app/server/model/plan/stop.go     | 10 ++++++++++
 3 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/app/server/db/git.go b/app/server/db/git.go
index dcac1018..e20bed50 100644
--- a/app/server/db/git.go
+++ b/app/server/db/git.go
@@ -180,6 +180,11 @@ func GitClearUncommittedChanges(orgId, planId string) error {
 	return nil
 }
 
+// Not used currently but may be good to handle these errors specifically later if locking can't fully prevent them
+// func isLockFileError(output string) bool {
+// 	return strings.Contains(output, "fatal: Unable to create") && strings.Contains(output, ".git/index.lock': File exists")
+// }
+
 func gitCheckoutBranch(repoDir, branch string) error {
 	// get current branch and only checkout if it's not the same
 	// trying to check out the same branch will result in an error
diff --git a/app/server/handlers/plans_exec.go b/app/server/handlers/plans_exec.go
index 395a5437..f5e95565 100644
--- a/app/server/handlers/plans_exec.go
+++ b/app/server/handlers/plans_exec.go
@@ -277,19 +277,29 @@ func StopPlanHandler(w http.ResponseWriter, r *http.Request) {
 	} else {
 		defer func() {
 			(*unlockFn)(err)
+
+			if err == nil {
+				err = modelPlan.Stop(planId, branch, auth.User.Id, auth.OrgId)
+
+				if err != nil {
+					log.Printf("Error stopping plan: %v\n", err)
+					http.Error(w, "Error stopping plan", http.StatusInternalServerError)
+					return
+				}
+
+				log.Println("Successfully processed request for StopPlanHandler")
+			}
 		}()
 	}
 
 	log.Println("Stopping plan")
-	err = modelPlan.Stop(planId, branch, auth.User.Id, auth.OrgId)
+	err = modelPlan.StorePartialReply(planId, branch, auth.User.Id, auth.OrgId)
 
 	if err != nil {
-		log.Printf("Error stopping plan: %v\n", err)
-		http.Error(w, "Error stopping plan", http.StatusInternalServerError)
+		log.Printf("Error storing partial reply: %v\n", err)
+		http.Error(w, "Error storing partial reply", http.StatusInternalServerError)
 		return
 	}
-
-	log.Println("Successfully processed request for StopPlanHandler")
 }
 
 func RespondMissingFileHandler(w http.ResponseWriter, r *http.Request) {
diff --git a/app/server/model/plan/stop.go b/app/server/model/plan/stop.go
index 4f511bc1..8433ea67 100644
--- a/app/server/model/plan/stop.go
+++ b/app/server/model/plan/stop.go
@@ -17,6 +17,16 @@ func Stop(planId, branch, currentUserId, currentOrgId string) error {
 	active.SummaryCancelFn()
 	active.CancelFn()
 
+	return nil
+}
+
+func StorePartialReply(planId, branch, currentUserId, currentOrgId string) error {
+	active := GetActivePlan(planId, branch)
+
+	if active == nil {
+		return fmt.Errorf("no active plan with id %s", planId)
+	}
+
 	if !active.BuildOnly && !active.RepliesFinished {
 		num := active.MessageNum + 1