fix(branch): reject extra args with --rename/-m and add cycle detection (#718)

jainankit · web-flow · commit 0bf7c8f34cdd · 2026-04-16T21:36:13.000Z
## Summary Fixes #717 - Reject unexpected extra positional arguments when `--rename`/`-m` flag is used with `av branch`, preventing silent misuse where `-m 'message'` triggers a rename instead of branch creation - Add visited-set cycle detection to `SubsequentBranches` and `SubsequentBranchesFiltered` so a corrupted branch graph (e.g., self-referencing parent) logs a warning instead of crashing with a stack overflow ## Test plan - [ ] `av branch foo -m 'bar'` now returns a clear error instead of silently renaming - [ ] `av branch --rename new-name` still works correctly - [ ] `av branch old:new -m` still works (colon syntax for rename) - [ ] Existing tests pass (`go test ./...`)
diff --git a/cmd/av/branch.go b/cmd/av/branch.go
@@ -71,6 +71,11 @@ internal tracking metadata that defines the order of branches within a stack.`),
 		}
 
 		if branchFlags.Rename {
+			if len(args) > 1 {
+				return errors.New("unexpected extra arguments with --rename/-m flag\n" +
+					"  To rename a branch: av branch --rename <new-name>\n" +
+					"  To create a branch: av branch <name> (without -m)")
+			}
 			return branchMove(ctx, repo, db, branchName, branchFlags.Force)
 		}
 		if branchFlags.Split {
diff --git a/internal/meta/branch.go b/internal/meta/branch.go
@@ -122,12 +122,22 @@ func PreviousBranches(tx ReadTx, name string) ([]string, error) {
 // If the tree is not a straight line (which isn't explicitly supported!), the
 // branches will be returned in depth-first traversal order.
 func SubsequentBranches(tx ReadTx, name string) []string {
+	visited := map[string]bool{name: true}
+	return subsequentBranches(tx, name, visited)
+}
+
+func subsequentBranches(tx ReadTx, name string, visited map[string]bool) []string {
 	logrus.Debugf("finding subsequent branches for %q", name)
 	var res []string
 	children := Children(tx, name)
 	for _, child := range children {
+		if visited[child.Name] {
+			logrus.Warnf("cycle detected in branch graph: %q already visited while traversing children of %q", child.Name, name)
+			continue
+		}
+		visited[child.Name] = true
 		res = append(res, child.Name)
-		res = append(res, SubsequentBranches(tx, child.Name)...)
+		res = append(res, subsequentBranches(tx, child.Name, visited)...)
 	}
 	return res
 }
@@ -136,17 +146,27 @@ func SubsequentBranches(tx ReadTx, name string) []string {
 // name in "dependency order", optionally skipping branches that are excluded
 // from sync --all and their descendants.
 func SubsequentBranchesFiltered(tx ReadTx, name string, skipExcluded bool) []string {
+	visited := map[string]bool{name: true}
+	return subsequentBranchesFiltered(tx, name, skipExcluded, visited)
+}
+
+func subsequentBranchesFiltered(tx ReadTx, name string, skipExcluded bool, visited map[string]bool) []string {
 	logrus.Debugf("finding subsequent branches for %q (skipExcluded=%v)", name, skipExcluded)
 	var res []string
 	children := Children(tx, name)
 	for _, child := range children {
+		if visited[child.Name] {
+			logrus.Warnf("cycle detected in branch graph: %q already visited while traversing children of %q", child.Name, name)
+			continue
+		}
 		if skipExcluded && child.ExcludeFromSyncAll {
 			// Skip this branch and its entire subtree
 			logrus.Debugf("skipping excluded branch %q and its descendants", child.Name)
 			continue
 		}
+		visited[child.Name] = true
 		res = append(res, child.Name)
-		res = append(res, SubsequentBranchesFiltered(tx, child.Name, skipExcluded)...)
+		res = append(res, subsequentBranchesFiltered(tx, child.Name, skipExcluded, visited)...)
 	}
 	return res
 }

Original file line number	Diff line number	Diff line change
@@ -71,6 +71,11 @@ internal tracking metadata that defines the order of branches within a stack.`),
`71`	`71`	`}`
`72`	`72`
`73`	`73`	`if branchFlags.Rename {`
	`74`	`+ if len(args) > 1 {`
	`75`	`+ return errors.New("unexpected extra arguments with --rename/-m flag\n" +`
	`76`	`+ " To rename a branch: av branch --rename <new-name>\n" +`
	`77`	`+ " To create a branch: av branch <name> (without -m)")`
	`78`	`+ }`
`74`	`79`	`return branchMove(ctx, repo, db, branchName, branchFlags.Force)`
`75`	`80`	`}`
`76`	`81`	`if branchFlags.Split {`