fix: correct unlock key format to match lock key format (runatlantis#5781)

Signed-off-by: PePe Amengual <2208324+jamengual@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
Signed-off-by: Ramon Vermeulen <ramonvermeulen98@gmail.com>

Author: 2 people

server/core/db/boltdb.go

@@ -485,7 +485,7 @@ func (b *BoltDB) commandLockKey(cmdName command.Name) string {
 }
 
 func (b *BoltDB) lockKey(p models.Project, workspace string) string {
-	return fmt.Sprintf("%s/%s/%s", p.RepoFullName, p.Path, workspace)
+	return models.GenerateLockKey(p, workspace)
 }
 
 func (b *BoltDB) getPullFromBucket(bucket *bolt.Bucket, key []byte) (*models.PullStatus, error) {

server/core/locking/locking.go

@@ -18,7 +18,6 @@ package locking
 
 import (
 	"errors"
-	"fmt"
 	"regexp"
 	"time"
 
@@ -146,7 +145,7 @@ func (c *Client) GetLock(key string) (*models.ProjectLock, error) {
 }
 
 func (c *Client) key(p models.Project, workspace string) string {
-	return fmt.Sprintf("%s/%s/%s", p.RepoFullName, p.Path, workspace)
+	return models.GenerateLockKey(p, workspace)
 }
 
 func (c *Client) lockKeyToProjectWorkspace(key string) (models.Project, string, error) {
@@ -199,5 +198,5 @@ func (c *NoOpLocker) GetLock(_ string) (*models.ProjectLock, error) {
 }
 
 func (c *NoOpLocker) key(p models.Project, workspace string) string {
-	return fmt.Sprintf("%s/%s/%s", p.RepoFullName, p.Path, workspace)
+	return models.GenerateLockKey(p, workspace)
 }

server/core/redis/redis.go

@@ -414,7 +414,7 @@ func (r *RedisDB) deletePull(key string) error {
 }
 
 func (r *RedisDB) lockKey(p models.Project, workspace string) string {
-	return fmt.Sprintf("pr/%s/%s/%s", p.RepoFullName, p.Path, workspace)
+	return fmt.Sprintf("pr/%s", models.GenerateLockKey(p, workspace))
 }
 
 func (r *RedisDB) commandLockKey(cmdName command.Name) string {

server/events/command_runner_test.go

@@ -17,7 +17,6 @@ import (
 	"errors"
 	"fmt"
 	"regexp"
-	"strconv"
 	"strings"
 	"testing"
 
@@ -961,13 +960,9 @@ func TestRunGenericPlanCommand_DeletePlans(t *testing.T) {
 	When(eventParsing.ParseGithubPull(Any[logging.SimpleLogging](), Eq(pull))).ThenReturn(modelPull, modelPull.BaseRepo, testdata.GithubRepo, nil)
 	testdata.Pull.BaseRepo = testdata.GithubRepo
 	ch.RunCommentCommand(testdata.GithubRepo, nil, nil, testdata.User, testdata.Pull.Num, &events.CommentCommand{Name: command.Plan})
-	lockingLocker.VerifyWasCalledOnce().Unlock(fmt.Sprintf("%s/%d/%s/%s", testdata.Pull.BaseRepo.FullName, testdata.Pull.Num, projectCtx.ProjectName, projectCtx.Workspace))
-
-	lockingLocker.VerifyWasCalledOnce().
-		Unlock(testdata.Pull.BaseRepo.FullName + "/" +
-			strconv.Itoa(testdata.Pull.Num) + "/" +
-			projectCtx.ProjectName + "/" +
-			projectCtx.Workspace)
+	// Verify that the unlock operation uses the same lock ID format as generateLockID
+	expectedLockID := events.GenerateLockID(projectCtx)
+	lockingLocker.VerifyWasCalledOnce().Unlock(expectedLockID)
 }
 
 func TestRunSpecificPlanCommandDoesnt_DeletePlans(t *testing.T) {

server/events/models/models.go

@@ -276,6 +276,12 @@ type Plan struct {
 	LocalPath string
 }
 
+// GenerateLockKey creates a consistent lock key from a project and workspace.
+// This ensures the same format is used across all locking operations.
+func GenerateLockKey(project Project, workspace string) string {
+	return fmt.Sprintf("%s/%s/%s", project.RepoFullName, project.Path, workspace)
+}
+
 // NewProject constructs a Project. Use this constructor because it
 // sets Path correctly.
 func NewProject(repoFullName string, path string, projectName string) Project {

server/events/plan_command_runner.go

@@ -1,14 +1,20 @@
 package events
 
 import (
-	"strconv"
-
 	"github.com/runatlantis/atlantis/server/core/locking"
 	"github.com/runatlantis/atlantis/server/events/command"
 	"github.com/runatlantis/atlantis/server/events/models"
 	"github.com/runatlantis/atlantis/server/events/vcs"
 )
 
+// GenerateLockID creates a consistent lock ID for a project context.
+// This ensures the same format is used for both locking and unlocking operations.
+func GenerateLockID(projCtx command.ProjectContext) string {
+	// Use models.NewProject to ensure consistent path cleaning
+	project := models.NewProject(projCtx.BaseRepo.FullName, projCtx.RepoRelDir, "")
+	return models.GenerateLockKey(project, projCtx.Workspace)
+}
+
 func NewPlanCommandRunner(
 	silenceVCSStatusNoPlans bool,
 	silenceVCSStatusNoProjects bool,
@@ -269,7 +275,7 @@ func (p *PlanCommandRunner) run(ctx *command.Context, cmd *CommentCommand) {
 
 		// delete lock only if there are changes
 		ctx.Log.Info("Deleting lock for project '%s' (changes detected)", projCtx.ProjectName)
-		lockID := projCtx.BaseRepo.FullName + "/" + strconv.Itoa(projCtx.Pull.Num) + "/" + projCtx.ProjectName + "/" + projCtx.Workspace
+		lockID := GenerateLockID(projCtx)
 
 		_, err := p.lockingLocker.Unlock(lockID)
 		if err != nil {