Skip to content

Commit 0706796

Browse files
committed
address review comments
1 parent 0f69416 commit 0706796

2 files changed

Lines changed: 64 additions & 15 deletions

File tree

cmd/init.go

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,9 @@ func runInit(cfg *config.Config, opts *initOptions) error {
180180
cfg.Errorf("branch %q already exists in a stack", branchName)
181181
return ErrInvalidArgs
182182
}
183-
if !git.BranchExists(branchName) {
183+
if git.BranchExists(branchName) {
184+
adopted[branchName] = true
185+
} else {
184186
if err := git.CreateBranch(branchName, trunk); err != nil {
185187
cfg.Errorf("creating branch %s: %s", branchName, err)
186188
return ErrSilent
@@ -195,10 +197,14 @@ func runInit(cfg *config.Config, opts *initOptions) error {
195197
return ErrInvalidArgs
196198
}
197199

198-
branches, err = runInteractiveInit(cfg, sf, trunk, currentBranch, opts)
200+
var interactiveAdopted bool
201+
branches, interactiveAdopted, err = runInteractiveInit(cfg, sf, trunk, currentBranch, opts)
199202
if err != nil {
200203
return err
201204
}
205+
if interactiveAdopted {
206+
adopted[branches[0]] = true
207+
}
202208
}
203209

204210
// --- Build stack ---
@@ -285,6 +291,13 @@ func resolveArgBranches(cfg *config.Config, opts *initOptions, sf *stack.StackFi
285291
if opts.prefix != "" {
286292
b = opts.prefix + "/" + b
287293
}
294+
295+
// Validate ref name before checking existence or creating
296+
if err := git.ValidateRefName(b); err != nil {
297+
cfg.Errorf("invalid branch name %q: must be a valid git ref", b)
298+
return nil, nil, ErrInvalidArgs
299+
}
300+
288301
exists := git.BranchExists(b)
289302

290303
if err := sf.ValidateNoDuplicateBranch(b); err != nil {
@@ -318,8 +331,9 @@ func resolveArgBranches(cfg *config.Config, opts *initOptions, sf *stack.StackFi
318331

319332
// runInteractiveInit runs the interactive init flow: prints hint about
320333
// multi-branch args, offers current branch or new branch, then runs
321-
// prefix detection.
322-
func runInteractiveInit(cfg *config.Config, sf *stack.StackFile, trunk, currentBranch string, opts *initOptions) ([]string, error) {
334+
// prefix detection. Returns the branches and whether the branch was adopted
335+
// (already existed).
336+
func runInteractiveInit(cfg *config.Config, sf *stack.StackFile, trunk, currentBranch string, opts *initOptions) ([]string, bool, error) {
323337
p := prompter.New(cfg.In, cfg.Out, cfg.Err)
324338

325339
cfg.Printf("Initializing a stack from %s.", trunk)
@@ -348,45 +362,48 @@ func runInteractiveInit(cfg *config.Config, sf *stack.StackFile, trunk, currentB
348362
clearSelectPrompt(cfg, len(options))
349363
}
350364
printInterrupt(cfg)
351-
return nil, ErrSilent
365+
return nil, false, ErrSilent
352366
}
353367
cfg.Errorf("failed to read selection: %s", err)
354-
return nil, ErrSilent
368+
return nil, false, ErrSilent
355369
}
356370

357371
if selected == 0 {
358372
// Use current branch
359373
if err := sf.ValidateNoDuplicateBranch(currentBranch); err != nil {
360374
cfg.Errorf("branch %q already exists in a stack", currentBranch)
361-
return nil, ErrInvalidArgs
375+
return nil, false, ErrInvalidArgs
362376
}
363377
branchName = currentBranch
364378
} else {
365379
// Create a new branch — fall through to input prompt
366380
name, err := promptBranchName(cfg, p, opts.prefix)
367381
if err != nil {
368-
return nil, err
382+
return nil, false, err
369383
}
370384
branchName = name
371385
}
372386
} else {
373387
// On trunk or detached HEAD — prompt for name directly
374388
name, err := promptBranchName(cfg, p, opts.prefix)
375389
if err != nil {
376-
return nil, err
390+
return nil, false, err
377391
}
378392
branchName = name
379393
}
380394

381-
// Validate and create branch
395+
// Validate and create branch (track whether it was adopted)
396+
wasAdopted := false
382397
if err := sf.ValidateNoDuplicateBranch(branchName); err != nil {
383398
cfg.Errorf("branch %q already exists in a stack", branchName)
384-
return nil, ErrInvalidArgs
399+
return nil, false, ErrInvalidArgs
385400
}
386-
if !git.BranchExists(branchName) {
401+
if git.BranchExists(branchName) {
402+
wasAdopted = true
403+
} else {
387404
if err := git.CreateBranch(branchName, trunk); err != nil {
388405
cfg.Errorf("creating branch %s: %s", branchName, err)
389-
return nil, ErrSilent
406+
return nil, false, ErrSilent
390407
}
391408
}
392409

@@ -401,7 +418,7 @@ func runInteractiveInit(cfg *config.Config, sf *stack.StackFile, trunk, currentB
401418
if err != nil {
402419
if isInterruptError(err) {
403420
printInterrupt(cfg)
404-
return nil, ErrSilent
421+
return nil, false, ErrSilent
405422
}
406423
// Not fatal — just skip prefix
407424
} else if usePrefix {
@@ -410,7 +427,7 @@ func runInteractiveInit(cfg *config.Config, sf *stack.StackFile, trunk, currentB
410427
}
411428
}
412429

413-
return []string{branchName}, nil
430+
return []string{branchName}, wasAdopted, nil
414431
}
415432

416433
// promptBranchName prompts the user for a branch name, applying the

cmd/init_test.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -721,6 +721,8 @@ func TestInit_Interactive_OnFeatureBranch_UseCurrent(t *testing.T) {
721721

722722
require.NoError(t, err)
723723
assert.Contains(t, output, "Initializing a stack from main")
724+
// Branch already exists → should be treated as adopted
725+
assert.Contains(t, output, "Adopted")
724726

725727
sf, _ := stack.Load(gitDir)
726728
require.Len(t, sf.Stacks, 1)
@@ -764,6 +766,36 @@ func TestInit_TwoPassValidation_NoBranchCreatedOnError(t *testing.T) {
764766
assert.Empty(t, created, "no branches should be created when later arg fails validation")
765767
}
766768

769+
func TestInit_TwoPassValidation_InvalidRefName(t *testing.T) {
770+
// Verify that an invalid ref name in the args list prevents any branch creation
771+
gitDir := t.TempDir()
772+
var created []string
773+
restore := git.SetOps(&git.MockOps{
774+
GitDirFn: func() (string, error) { return gitDir, nil },
775+
DefaultBranchFn: func() (string, error) { return "main", nil },
776+
CurrentBranchFn: func() (string, error) { return "main", nil },
777+
ValidateRefNameFn: func(name string) error {
778+
if name == "invalid..name" {
779+
return fmt.Errorf("invalid ref name: %s", name)
780+
}
781+
return nil
782+
},
783+
CreateBranchFn: func(name, base string) error {
784+
created = append(created, name)
785+
return nil
786+
},
787+
})
788+
defer restore()
789+
790+
cfg, outR, errR := config.NewTestConfig()
791+
err := runInit(cfg, &initOptions{branches: []string{"valid-branch", "invalid..name", "another-branch"}})
792+
output := collectOutput(cfg, outR, errR)
793+
794+
assert.ErrorIs(t, err, ErrInvalidArgs)
795+
assert.Contains(t, output, "invalid branch name")
796+
assert.Empty(t, created, "no branches should be created when an arg has an invalid ref name")
797+
}
798+
767799
func TestDetectPrefix(t *testing.T) {
768800
tests := []struct {
769801
name string

0 commit comments

Comments
 (0)