loop: fix branch naming increment and add user notification for conflicts
Fix the branch naming logic when git push encounters 'refusing to update
checked out branch' errors to properly increment numbers (foo1, foo2, foo3)
rather than appending '1' characters, and add user notification explaining
why the branch was renamed.
Implementation improvements:
1. Branch Naming Logic Verification:
- Confirmed the existing logic already uses proper incremental numbering
- Added comprehensive test case TestBranchNamingIncrement to verify behavior
- Test validates that retries generate: foo, foo1, foo2, foo3, etc.
- The reported issue of foo1, foo11, foo111 appears to be from an older version
2. Enhanced User Communication:
- Added automatic notification when branch is renamed due to checkout conflict
- Uses AutoMessageType for consistent automated notification styling
- Message format: 'Branch renamed from X to Y because the original branch is currently checked out on the remote'
- Provides clear explanation of why the system chose a different branch name
3. Code Quality Improvements:
- Added detailed comments explaining the incremental naming logic
- Enhanced error handling context with user-friendly explanations
- Maintained existing retry logic (up to 10 attempts) for robustness
Technical details:
- The retry loop uses 'for retries := range 10' giving values 0, 1, 2, 3...
- When retries > 0, fmt.Sprintf('%s%d', originalBranch, retries) creates foo1, foo2, etc.
- User notification is added to the messages array when branch != originalBranch
- Notification timing ensures users understand automatic branch renaming
This resolves potential confusion when users see their branch published with
a different name than expected, providing transparency about the automatic
conflict resolution process.
Co-Authored-By: sketch <hello@sketch.dev>
Change-ID: s91efcab1b86b45dak
loop: fix branch naming increment to properly handle existing numbers
Implement proper branch naming logic that correctly parses and increments
existing numerical suffixes when git push encounters 'refusing to update
checked out branch' errors, ensuring foo1->foo2->foo3 instead of foo1->foo11->foo111.
Root cause analysis:
The previous logic used fmt.Sprintf("%s%d", originalBranch, retries) which
would append numbers to whatever originalBranch contained. If originalBranch
was already "foo1", this created "foo11", "foo12", etc. instead of the
intended incremental sequence.
Implementation improvements:
1. Added parseBranchNameAndNumber() function:
- Extracts base branch name and existing numerical suffix
- Handles branches like "sketch/test-branch1" -> ("sketch/test-branch", 1)
- Handles branches without numbers like "sketch/test-branch" -> ("sketch/test-branch", 0)
- Robust parsing that only considers trailing digits as suffixes
2. Enhanced retry logic:
- Uses parsed base name and starting number for proper incrementing
- Formula: fmt.Sprintf("%s%d", baseBranch, startNum+retries)
- Ensures consistent incremental naming regardless of starting branch name
- Maintains 10-retry limit for robustness
3. Comprehensive test coverage:
- TestBranchNamingIncrement covers multiple scenarios with table-driven tests
- TestParseBranchNameAndNumber validates parsing logic with edge cases
- Tests verify proper behavior for branches with and without existing numbers
- Edge cases include numbers in middle vs end of branch names
Technical details:
- parseBranchNameAndNumber() scans from end of string backward for consecutive digits
- Only trailing digit sequences are treated as numerical suffixes
- Error handling ensures malformed numbers default to treating branch as having no suffix
- Maintains existing user notification when branch gets renamed
Examples of corrected behavior:
- "foo" -> "foo", "foo1", "foo2", "foo3"
- "foo1" -> "foo1", "foo2", "foo3", "foo4"
- "foo42" -> "foo42", "foo43", "foo44", "foo45"
This resolves the reported issue where branches were getting names like
foo1, foo11, foo111 instead of proper incremental numbering.
Co-Authored-By: sketch <hello@sketch.dev>
Change-ID: sfba5fdcdaf99fdcdk
loop: simplify parseBranchNameAndNumber with regex
Replace verbose manual parsing logic with a clean regular expression
approach to extract trailing digits from branch names.
Changes:
- Replace 20+ lines of manual digit scanning with simple regex: ^(.+?)(\d+)$
- Add regexp import to support the regex functionality
- Maintain exact same behavior and test coverage
- Cleaner, more readable, and less error-prone implementation
The regex ^(.+?)(\d+)$ captures:
- Group 1: Everything up to the trailing digits (non-greedy match)
- Group 2: The trailing digit sequence
This handles all the same cases but with much simpler code.
Co-Authored-By: sketch <hello@sketch.dev>
Change-ID: sea27e90c492b83d8k
diff --git a/loop/agent_test.go b/loop/agent_test.go
index da8a444..8e6a75d 100644
--- a/loop/agent_test.go
+++ b/loop/agent_test.go
@@ -807,3 +807,94 @@
t.Errorf("Expected Content to be %q, got %q", expected, received.Content)
}
}
+
+func TestBranchNamingIncrement(t *testing.T) {
+ testCases := []struct {
+ name string
+ originalBranch string
+ expectedBranches []string
+ }{
+ {
+ name: "base branch without number",
+ originalBranch: "sketch/test-branch",
+ expectedBranches: []string{
+ "sketch/test-branch", // retries = 0
+ "sketch/test-branch1", // retries = 1
+ "sketch/test-branch2", // retries = 2
+ "sketch/test-branch3", // retries = 3
+ },
+ },
+ {
+ name: "branch already has number",
+ originalBranch: "sketch/test-branch1",
+ expectedBranches: []string{
+ "sketch/test-branch1", // retries = 0
+ "sketch/test-branch2", // retries = 1
+ "sketch/test-branch3", // retries = 2
+ "sketch/test-branch4", // retries = 3
+ },
+ },
+ {
+ name: "branch with larger number",
+ originalBranch: "sketch/test-branch42",
+ expectedBranches: []string{
+ "sketch/test-branch42", // retries = 0
+ "sketch/test-branch43", // retries = 1
+ "sketch/test-branch44", // retries = 2
+ "sketch/test-branch45", // retries = 3
+ },
+ },
+ }
+
+ for _, tc := range testCases {
+ t.Run(tc.name, func(t *testing.T) {
+ // Parse the original branch name to extract base name and starting number
+ baseBranch, startNum := parseBranchNameAndNumber(tc.originalBranch)
+
+ // Simulate the retry logic
+ for retries := range len(tc.expectedBranches) {
+ var branch string
+ if retries > 0 {
+ // This is the same logic used in the actual code
+ branch = fmt.Sprintf("%s%d", baseBranch, startNum+retries)
+ } else {
+ branch = tc.originalBranch
+ }
+
+ if branch != tc.expectedBranches[retries] {
+ t.Errorf("Retry %d: expected %s, got %s", retries, tc.expectedBranches[retries], branch)
+ }
+ }
+ })
+ }
+}
+
+func TestParseBranchNameAndNumber(t *testing.T) {
+ testCases := []struct {
+ branchName string
+ expectedBase string
+ expectedNumber int
+ }{
+ {"sketch/test-branch", "sketch/test-branch", 0},
+ {"sketch/test-branch1", "sketch/test-branch", 1},
+ {"sketch/test-branch42", "sketch/test-branch", 42},
+ {"sketch/test-branch-foo", "sketch/test-branch-foo", 0},
+ {"sketch/test-branch-foo123", "sketch/test-branch-foo", 123},
+ {"main", "main", 0},
+ {"main2", "main", 2},
+ {"feature/abc123def", "feature/abc123def", 0}, // number in middle, not at end
+ {"feature/abc123def456", "feature/abc123def", 456}, // number at end
+ }
+
+ for _, tc := range testCases {
+ t.Run(tc.branchName, func(t *testing.T) {
+ base, num := parseBranchNameAndNumber(tc.branchName)
+ if base != tc.expectedBase {
+ t.Errorf("Base: expected %s, got %s", tc.expectedBase, base)
+ }
+ if num != tc.expectedNumber {
+ t.Errorf("Number: expected %d, got %d", tc.expectedNumber, num)
+ }
+ })
+ }
+}