fix: correct unlock key format to match lock key format#5781
Merged
Conversation
When PR #5570 added logic to preserve apply locks after plan when no changes are detected, it introduced a bug in the unlock key format. The lock key uses: repo/path/workspace But the unlock key was using: repo/pullnum/projectname/workspace This mismatch caused locks to never be released, blocking subsequent operations on projects in the same directory. This commit fixes the unlock key to match the lock key format. Fixes #5722 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
server/events/plan_command_runner.go
Outdated
| // 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 := fmt.Sprintf("%s/%s/%s", projCtx.BaseRepo.FullName, projCtx.RepoRelDir, projCtx.Workspace) |
Contributor
There was a problem hiding this comment.
Is this lockID used only for unlocking, effectively making it an unlockID? If so, and if we want to match the actual lock key format, could we create a function that enforces the same format for both locking and unlocking IDs? Or am I misunderstanding the underlying problem here?
Contributor
Author
There was a problem hiding this comment.
that is a great idea, jus pushed the changes
Address review feedback by introducing generateLockID() function that ensures consistent lock key format between lock acquisition and release. This function uses models.NewProject() internally to apply consistent path cleaning logic. The lock ID format is: repo/path/workspace where path is cleaned via path.Clean() and empty paths become "." as per Go's path package behavior. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…separate strings - Changed generateLockID to accept command.ProjectContext instead of 3 separate strings - Added validation to ensure required fields (BaseRepo.FullName, Workspace) are not empty - Made function exportable (GenerateLockID) for better testability - Updated test to use the actual GenerateLockID function instead of hard-coded expected format - This prevents passing arbitrary strings that could break lock format consistency Addresses feedback on PR #5781 to enforce consistent lock format generation. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
The BaseRepo.FullName and Workspace fields are guaranteed to be valid by the time ProjectContext reaches GenerateLockID, as they are populated from validated VCS pull request data during the BuildPlanCommands process. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
bschaatsbergen
previously approved these changes
Sep 11, 2025
bschaatsbergen
approved these changes
Sep 11, 2025
bschaatsbergen
approved these changes
Sep 11, 2025
- Added GenerateLockKey function to models package for consistent lock key format - Updated all lock key generation to use the centralized function: - BoltDB backend now uses models.GenerateLockKey - Redis backend now uses models.GenerateLockKey (preserving "pr/" prefix) - Locking client uses models.GenerateLockKey - NoOpLocker uses models.GenerateLockKey - Modified GenerateLockID to use models.GenerateLockKey internally - This ensures all locking operations use the exact same key format - Removed duplicate fmt.Sprintf implementations across different components All tests pass, maintaining backward compatibility while improving consistency. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: PePe Amengual <2208324+jamengual@users.noreply.github.com>
jamengual
force-pushed
the
fix/issue-5722-unlock-key-format
branch
from
998d9eb to
7b33ae0
Compare
This was referenced Sep 12, 2025
ramonvermeulen
pushed a commit
to bschaatsbergen/atlantis
that referenced
this pull request
Oct 13, 2025
…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>
dimisjim
pushed a commit
to dimisjim/atlantis
that referenced
this pull request
Oct 29, 2025
…5781) Signed-off-by: PePe Amengual <2208324+jamengual@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: dimisjim <dimitris.moraitidis@gmail.com>
aidansteele
pushed a commit
to aidansteele/atlantis
that referenced
this pull request
Mar 12, 2026
…5781) Signed-off-by: PePe Amengual <2208324+jamengual@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
repo/pullnum/projectname/workspacetorepo/path/workspaceto match the lock key formatProblem
PR #5570 added logic to preserve apply locks after plan when no changes are detected. However, it introduced a bug where the unlock key format didn't match the lock key format:
repo/path/workspacerepo/pullnum/projectname/workspaceThis mismatch caused locks to never be released, blocking subsequent operations on projects in the same directory.
Solution
This PR corrects the unlock key to use the same format as the lock key, ensuring locks can be properly released when changes are detected.
Test plan
Fixes #5722
🤖 Generated with Claude Code