chore: lint and test fixes

boneskull · boneskull · commit 7676fcbec8ed · 2026-01-26T15:13:00.000-08:00
diff --git a/.golangci.yml b/.golangci.yml
@@ -12,6 +12,13 @@ linters:
     - ineffassign
     - misspell
     # gosec disabled - false positives for CLI tools that shell out to git
+  settings:
+    errcheck:
+      check-type-assertions: true
+      check-blank: true
+    govet:
+      enable:
+        - shadow
   exclusions:
     rules:
       # Test files can have unchecked errors - tests use t.Fatal patterns
@@ -24,14 +31,6 @@ formatters:
     - gofmt
     - goimports
 
-linters-settings:
-  errcheck:
-    check-type-assertions: true
-    check-blank: true
-  govet:
-    enable:
-      - shadow
-
 issues:
   max-issues-per-linter: 0
   max-same-issues: 0
diff --git a/cmd/abort.go b/cmd/abort.go
@@ -44,7 +44,7 @@ func runAbort(cmd *cobra.Command, args []string) error {
 	}
 
 	// Clean up state (ignore error - best effort cleanup)
-	_ = state.Remove(g.GetGitDir())
+	_ = state.Remove(g.GetGitDir()) //nolint:errcheck // best effort cleanup
 
 	fmt.Printf("Cascade aborted. Original HEAD was %s\n", st.OriginalHead)
 	return nil
diff --git a/cmd/adopt.go b/cmd/adopt.go
@@ -56,7 +56,7 @@ func runAdopt(cmd *cobra.Command, args []string) error {
 	}
 
 	// Check if already tracked
-	if _, err := cfg.GetParent(branchName); err == nil {
+	if _, getParentErr := cfg.GetParent(branchName); getParentErr == nil {
 		return fmt.Errorf("branch %q is already tracked", branchName)
 	}
 
@@ -73,7 +73,7 @@ func runAdopt(cmd *cobra.Command, args []string) error {
 	}
 
 	if parent != trunk {
-		if _, err := cfg.GetParent(parent); err != nil {
+		if _, parentErr := cfg.GetParent(parent); parentErr != nil {
 			return fmt.Errorf("parent %q is not tracked", parent)
 		}
 	}
diff --git a/cmd/cascade.go b/cmd/cascade.go
@@ -88,8 +88,14 @@ func runCascade(cmd *cobra.Command, args []string) error {
 }
 
 func doCascade(g *git.Git, cfg *config.Config, branches []*tree.Node, dryRun bool) error {
-	originalBranch, _ := g.CurrentBranch()
-	originalHead, _ := g.GetTip(originalBranch)
+	originalBranch, err := g.CurrentBranch()
+	if err != nil {
+		return err
+	}
+	originalHead, err := g.GetTip(originalBranch)
+	if err != nil {
+		return err
+	}
 
 	for i, b := range branches {
 		parent, err := cfg.GetParent(b.Name)
@@ -132,7 +138,7 @@ func doCascade(g *git.Git, cfg *config.Config, branches []*tree.Node, dryRun boo
 				Pending:      remaining,
 				OriginalHead: originalHead,
 			}
-			_ = state.Save(g.GetGitDir(), st) // Best effort - user can recover manually
+			_ = state.Save(g.GetGitDir(), st) //nolint:errcheck // best effort - user can recover manually
 
 			fmt.Printf("\nCONFLICT: Resolve conflicts and run 'gh stack continue', or 'gh stack abort' to cancel.\n")
 			fmt.Printf("Remaining branches: %v\n", remaining)
@@ -144,7 +150,7 @@ func doCascade(g *git.Git, cfg *config.Config, branches []*tree.Node, dryRun boo
 
 	// Return to original branch
 	if !dryRun {
-		_ = g.Checkout(originalBranch) // Best effort - cascade succeeded
+		_ = g.Checkout(originalBranch) //nolint:errcheck // best effort - cascade succeeded
 	}
 
 	return nil
diff --git a/cmd/continue.go b/cmd/continue.go
@@ -40,7 +40,7 @@ func runContinue(cmd *cobra.Command, args []string) error {
 	// Complete the in-progress rebase
 	if g.IsRebaseInProgress() {
 		fmt.Println("Continuing rebase...")
-		if err := g.RebaseContinue(); err != nil {
+		if rebaseErr := g.RebaseContinue(); rebaseErr != nil {
 			return fmt.Errorf("rebase --continue failed; resolve conflicts first")
 		}
 	}
@@ -49,7 +49,7 @@ func runContinue(cmd *cobra.Command, args []string) error {
 
 	// Continue with remaining branches
 	if len(st.Pending) == 0 {
-		_ = state.Remove(g.GetGitDir()) // Cleanup
+		_ = state.Remove(g.GetGitDir()) //nolint:errcheck // cleanup
 		fmt.Println("Cascade complete!")
 		return nil
 	}
@@ -73,7 +73,7 @@ func runContinue(cmd *cobra.Command, args []string) error {
 	}
 
 	// Remove state file before continuing (will be recreated if conflict)
-	_ = state.Remove(g.GetGitDir())
+	_ = state.Remove(g.GetGitDir()) //nolint:errcheck // cleanup
 
 	return doCascade(g, cfg, branches, false)
 }
diff --git a/cmd/create.go b/cmd/create.go
@@ -57,7 +57,7 @@ func runCreate(cmd *cobra.Command, args []string) error {
 	}
 
 	if currentBranch != trunk {
-		if _, err := cfg.GetParent(currentBranch); err != nil {
+		if _, parentErr := cfg.GetParent(currentBranch); parentErr != nil {
 			return fmt.Errorf("current branch %q is not tracked; use 'gh stack adopt' first", currentBranch)
 		}
 	}
diff --git a/cmd/log.go b/cmd/log.go
@@ -46,7 +46,7 @@ func runLog(cmd *cobra.Command, args []string) error {
 	}
 
 	g := git.New(cwd)
-	currentBranch, _ := g.CurrentBranch()
+	currentBranch, _ := g.CurrentBranch() //nolint:errcheck // empty string is fine for display
 
 	if logPorcelainFlag {
 		printPorcelain(root, currentBranch)
diff --git a/cmd/orphan.go b/cmd/orphan.go
@@ -70,15 +70,15 @@ func runOrphan(cmd *cobra.Command, args []string) error {
 	if orphanForceFlag {
 		descendants := tree.GetDescendants(node)
 		for _, desc := range descendants {
-			_ = cfg.RemoveParent(desc.Name) // Best effort cleanup
-			_ = cfg.RemovePR(desc.Name)
+			_ = cfg.RemoveParent(desc.Name) //nolint:errcheck // best effort cleanup
+			_ = cfg.RemovePR(desc.Name)     //nolint:errcheck // best effort cleanup
 			fmt.Printf("Orphaned %q\n", desc.Name)
 		}
 	}
 
 	// Orphan the branch
-	_ = cfg.RemoveParent(branchName) // Best effort cleanup
-	_ = cfg.RemovePR(branchName)
+	_ = cfg.RemoveParent(branchName) //nolint:errcheck // best effort cleanup
+	_ = cfg.RemovePR(branchName)     //nolint:errcheck // best effort cleanup
 	fmt.Printf("Orphaned %q\n", branchName)
 
 	return nil
diff --git a/cmd/pr.go b/cmd/pr.go
@@ -67,23 +67,23 @@ func runPR(cmd *cobra.Command, args []string) error {
 	}
 
 	// Check if PR already exists
-	existingPR, _ := cfg.GetPR(branch)
+	existingPR, _ := cfg.GetPR(branch) //nolint:errcheck // 0 is fine if no PR
 	if existingPR > 0 {
 		// Update existing PR's base if needed
 		fmt.Printf("PR #%d already exists, updating base to %q\n", existingPR, base)
-		if err := gh.UpdatePRBase(existingPR, base); err != nil {
-			return fmt.Errorf("failed to update PR base: %w", err)
+		if updateErr := gh.UpdatePRBase(existingPR, base); updateErr != nil {
+			return fmt.Errorf("failed to update PR base: %w", updateErr)
 		}
 
 		// Update stack comment
-		root, err := tree.Build(cfg)
-		if err != nil {
-			return fmt.Errorf("build tree: %w", err)
+		root, buildErr := tree.Build(cfg)
+		if buildErr != nil {
+			return fmt.Errorf("build tree: %w", buildErr)
 		}
 		comment := github.GenerateStackComment(root, branch, trunk)
 		if comment != "" {
-			if err := gh.CreateOrUpdateStackComment(existingPR, comment); err != nil {
-				fmt.Printf("Warning: failed to update stack comment: %v\n", err)
+			if commentErr := gh.CreateOrUpdateStackComment(existingPR, comment); commentErr != nil {
+				fmt.Printf("Warning: failed to update stack comment: %v\n", commentErr)
 			}
 		}
 		return nil
@@ -109,14 +109,14 @@ func runPR(cmd *cobra.Command, args []string) error {
 	}
 
 	// Store PR number
-	if err := cfg.SetPR(branch, prNumber); err != nil {
-		return err
+	if setPRErr := cfg.SetPR(branch, prNumber); setPRErr != nil {
+		return setPRErr
 	}
 
 	// Post stack navigation comment
-	root, err := tree.Build(cfg)
-	if err != nil {
-		return fmt.Errorf("build tree: %w", err)
+	root, buildErr := tree.Build(cfg)
+	if buildErr != nil {
+		return fmt.Errorf("build tree: %w", buildErr)
 	}
 
 	comment := github.GenerateStackComment(root, branch, trunk)
diff --git a/cmd/push.go b/cmd/push.go
@@ -58,7 +58,10 @@ func runPush(cmd *cobra.Command, args []string) error {
 
 	// Get downstack (ancestors from current to trunk, reversed)
 	ancestors := tree.GetAncestors(node)
-	trunk, _ := cfg.GetTrunk()
+	trunk, err := cfg.GetTrunk()
+	if err != nil {
+		return err
+	}
 
 	// Build list: current + ancestors (excluding trunk)
 	var branches []*tree.Node
@@ -76,7 +79,7 @@ func runPush(cmd *cobra.Command, args []string) error {
 
 	// Update PR bases and push
 	for _, b := range branches {
-		parent, _ := cfg.GetParent(b.Name)
+		parent, _ := cfg.GetParent(b.Name) //nolint:errcheck // empty string is fine
 
 		// Update PR base if needed
 		if b.PR > 0 {
diff --git a/cmd/sync.go b/cmd/sync.go
@@ -93,20 +93,20 @@ func runSync(cmd *cobra.Command, args []string) error {
 	// Fetch
 	fmt.Println("Fetching from origin...")
 	if !syncDryRunFlag {
-		if err := g.Fetch(); err != nil {
-			return fmt.Errorf("fetch failed: %w", err)
+		if fetchErr := g.Fetch(); fetchErr != nil {
+			return fmt.Errorf("fetch failed: %w", fetchErr)
 		}
 	}
 
 	// Fast-forward trunk
-	currentBranch, _ := g.CurrentBranch()
+	currentBranch, _ := g.CurrentBranch() //nolint:errcheck // empty string is fine
 	fmt.Printf("Fast-forwarding %s...\n", trunk)
 	if !syncDryRunFlag {
-		if err := g.FastForward(trunk); err != nil {
-			fmt.Printf("Warning: could not fast-forward %s: %v\n", trunk, err)
+		if ffErr := g.FastForward(trunk); ffErr != nil {
+			fmt.Printf("Warning: could not fast-forward %s: %v\n", trunk, ffErr)
 		}
 		// Return to original branch
-		_ = g.Checkout(currentBranch) // Best effort
+		_ = g.Checkout(currentBranch) //nolint:errcheck // best effort
 	}
 
 	// Check for merged PRs
@@ -117,14 +117,14 @@ func runSync(cmd *cobra.Command, args []string) error {
 
 	var merged []string
 	for _, branch := range branches {
-		prNum, err := cfg.GetPR(branch)
-		if err != nil || prNum == 0 {
+		prNum, prErr := cfg.GetPR(branch)
+		if prErr != nil || prNum == 0 {
 			continue
 		}
 
-		pr, err := gh.GetPR(prNum)
-		if err != nil {
-			fmt.Printf("Warning: could not fetch PR #%d: %v\n", prNum, err)
+		pr, getPRErr := gh.GetPR(prNum)
+		if getPRErr != nil {
+			fmt.Printf("Warning: could not fetch PR #%d: %v\n", prNum, getPRErr)
 			continue
 		}
 
@@ -134,7 +134,7 @@ func runSync(cmd *cobra.Command, args []string) error {
 	}
 
 	// Handle merged branches
-	root, _ := tree.Build(cfg)
+	root, _ := tree.Build(cfg) //nolint:errcheck // nil root is fine, FindNode handles it
 	for _, branch := range merged {
 		node := tree.FindNode(root, branch)
 		if node == nil {
@@ -147,26 +147,26 @@ func runSync(cmd *cobra.Command, args []string) error {
 				fmt.Printf("Would retarget %s from %s to %s\n", child.Name, branch, trunk)
 			} else {
 				fmt.Printf("Retargeting %s from %s to %s\n", child.Name, branch, trunk)
-				_ = cfg.SetParent(child.Name, trunk)
+				_ = cfg.SetParent(child.Name, trunk) //nolint:errcheck // best effort
 
 				// Update PR base on GitHub
-				childPR, _ := cfg.GetPR(child.Name)
+				childPR, _ := cfg.GetPR(child.Name) //nolint:errcheck // 0 is fine
 				if childPR > 0 {
-					if err := gh.UpdatePRBase(childPR, trunk); err != nil {
-						fmt.Printf("Warning: failed to update PR #%d base: %v\n", childPR, err)
+					if updateErr := gh.UpdatePRBase(childPR, trunk); updateErr != nil {
+						fmt.Printf("Warning: failed to update PR #%d base: %v\n", childPR, updateErr)
 					}
 
 					// Check if this was a draft and now targets trunk
-					pr, err := gh.GetPR(childPR)
-					if err == nil && pr.Draft {
+					pr, getPRErr := gh.GetPR(childPR)
+					if getPRErr == nil && pr.Draft {
 						fmt.Printf("PR #%d (%s) now targets %s.\n", childPR, child.Name, trunk)
 						fmt.Print("Mark as ready for review? [y/N]: ")
 
 						var response string
 						if _, scanErr := fmt.Scanln(&response); scanErr == nil {
 							if strings.ToLower(strings.TrimSpace(response)) == "y" {
-								if err := gh.MarkPRReady(childPR); err != nil {
-									fmt.Printf("Warning: failed to mark PR ready: %v\n", err)
+								if readyErr := gh.MarkPRReady(childPR); readyErr != nil {
+									fmt.Printf("Warning: failed to mark PR ready: %v\n", readyErr)
 								} else {
 									fmt.Printf("PR #%d marked as ready for review.\n", childPR)
 								}
@@ -182,9 +182,9 @@ func runSync(cmd *cobra.Command, args []string) error {
 			fmt.Printf("Would delete merged branch %s\n", branch)
 		} else {
 			fmt.Printf("Deleting merged branch %s (PR was merged)\n", branch)
-			_ = cfg.RemoveParent(branch) // Best effort cleanup
-			_ = cfg.RemovePR(branch)
-			_ = g.DeleteBranch(branch)
+			_ = cfg.RemoveParent(branch) //nolint:errcheck // best effort cleanup
+			_ = cfg.RemovePR(branch)     //nolint:errcheck // best effort cleanup
+			_ = g.DeleteBranch(branch)   //nolint:errcheck // best effort cleanup
 		}
 	}
 
diff --git a/e2e/helpers_test.go b/e2e/helpers_test.go
@@ -72,6 +72,7 @@ func NewTestEnvWithRemote(t *testing.T) *TestEnv {
 	env.RemoteDir = remoteDir
 
 	cmd := exec.Command("git", "clone", "--bare", env.WorkDir, remoteDir)
+	cmd.Env = append(os.Environ(), "GIT_EDITOR=true")
 	if err := cmd.Run(); err != nil {
 		t.Fatalf("failed to create bare remote: %v", err)
 	}
@@ -131,6 +132,8 @@ func (e *TestEnv) Git(args ...string) string {
 	cmd.Dir = e.WorkDir
 	cmd.Stdout = &stdout
 	cmd.Stderr = &stderr
+	// Prevent git from opening an editor (for commits, rebases, etc.)
+	cmd.Env = append(os.Environ(), "GIT_EDITOR=true")
 
 	if err := cmd.Run(); err != nil {
 		e.t.Fatalf("git %s failed: %v\nstderr: %s", strings.Join(args, " "), err, stderr.String())
@@ -152,6 +155,7 @@ func (e *TestEnv) GitRemote(args ...string) string {
 	cmd.Dir = e.RemoteDir
 	cmd.Stdout = &stdout
 	cmd.Stderr = &stderr
+	cmd.Env = append(os.Environ(), "GIT_EDITOR=true")
 
 	if err := cmd.Run(); err != nil {
 		e.t.Fatalf("git (remote) %s failed: %v\nstderr: %s", strings.Join(args, " "), err, stderr.String())
@@ -220,6 +224,7 @@ func (e *TestEnv) AssertBranchExists(branch string) {
 	e.t.Helper()
 	cmd := exec.Command("git", "rev-parse", "--verify", "refs/heads/"+branch)
 	cmd.Dir = e.WorkDir
+	cmd.Env = append(os.Environ(), "GIT_EDITOR=true")
 	if err := cmd.Run(); err != nil {
 		e.t.Errorf("expected branch %q to exist", branch)
 	}
@@ -267,6 +272,7 @@ func (e *TestEnv) AssertAncestor(ancestor, descendant string) {
 	e.t.Helper()
 	cmd := exec.Command("git", "merge-base", "--is-ancestor", ancestor, descendant)
 	cmd.Dir = e.WorkDir
+	cmd.Env = append(os.Environ(), "GIT_EDITOR=true")
 	if err := cmd.Run(); err != nil {
 		e.t.Errorf("expected %q to be ancestor of %q", ancestor, descendant)
 	}
diff --git a/e2e/scenario_helpers_test.go b/e2e/scenario_helpers_test.go
diff --git a/e2e/smoke_test.go b/e2e/smoke_test.go
diff --git a/internal/config/config.go b/internal/config/config.go
diff --git a/internal/config/config_test.go b/internal/config/config_test.go
diff --git a/internal/tree/tree.go b/internal/tree/tree.go

Original file line number	Diff line number	Diff line change
`@@ -44,7 +44,7 @@ func runAbort(cmd *cobra.Command, args []string) error {`
`44`	`44`	`}`
`45`	`45`
`46`	`46`	`// Clean up state (ignore error - best effort cleanup)`
`47`		`- _ = state.Remove(g.GetGitDir())`
	`47`	`+ _ = state.Remove(g.GetGitDir()) //nolint:errcheck // best effort cleanup`
`48`	`48`
`49`	`49`	`fmt.Printf("Cascade aborted. Original HEAD was %s\n", st.OriginalHead)`
`50`	`50`	`return nil`
Original file line number	Diff line number	Diff line change
`@@ -56,7 +56,7 @@ func runAdopt(cmd *cobra.Command, args []string) error {`
`56`	`56`	`}`
`57`	`57`
`58`	`58`	`// Check if already tracked`
`59`		`- if _, err := cfg.GetParent(branchName); err == nil {`
	`59`	`+ if _, getParentErr := cfg.GetParent(branchName); getParentErr == nil {`
`60`	`60`	`return fmt.Errorf("branch %q is already tracked", branchName)`
`61`	`61`	`}`
`62`	`62`
`@@ -73,7 +73,7 @@ func runAdopt(cmd *cobra.Command, args []string) error {`
`73`	`73`	`}`
`74`	`74`
`75`	`75`	`if parent != trunk {`
`76`		`- if _, err := cfg.GetParent(parent); err != nil {`
	`76`	`+ if _, parentErr := cfg.GetParent(parent); parentErr != nil {`
`77`	`77`	`return fmt.Errorf("parent %q is not tracked", parent)`
`78`	`78`	`}`
`79`	`79`	`}`
Original file line number	Diff line number	Diff line change
`@@ -40,7 +40,7 @@ func runContinue(cmd *cobra.Command, args []string) error {`
`40`	`40`	`// Complete the in-progress rebase`
`41`	`41`	`if g.IsRebaseInProgress() {`
`42`	`42`	`fmt.Println("Continuing rebase...")`
`43`		`- if err := g.RebaseContinue(); err != nil {`
	`43`	`+ if rebaseErr := g.RebaseContinue(); rebaseErr != nil {`
`44`	`44`	`return fmt.Errorf("rebase --continue failed; resolve conflicts first")`
`45`	`45`	`}`
`46`	`46`	`}`
`@@ -49,7 +49,7 @@ func runContinue(cmd *cobra.Command, args []string) error {`
`49`	`49`
`50`	`50`	`// Continue with remaining branches`
`51`	`51`	`if len(st.Pending) == 0 {`
`52`		`- _ = state.Remove(g.GetGitDir()) // Cleanup`
	`52`	`+ _ = state.Remove(g.GetGitDir()) //nolint:errcheck // cleanup`
`53`	`53`	`fmt.Println("Cascade complete!")`
`54`	`54`	`return nil`
`55`	`55`	`}`
`@@ -73,7 +73,7 @@ func runContinue(cmd *cobra.Command, args []string) error {`
`73`	`73`	`}`
`74`	`74`
`75`	`75`	`// Remove state file before continuing (will be recreated if conflict)`
`76`		`- _ = state.Remove(g.GetGitDir())`
	`76`	`+ _ = state.Remove(g.GetGitDir()) //nolint:errcheck // cleanup`
`77`	`77`
`78`	`78`	`return doCascade(g, cfg, branches, false)`
`79`	`79`	`}`
Original file line number	Diff line number	Diff line change
`@@ -57,7 +57,7 @@ func runCreate(cmd *cobra.Command, args []string) error {`
`57`	`57`	`}`
`58`	`58`
`59`	`59`	`if currentBranch != trunk {`
`60`		`- if _, err := cfg.GetParent(currentBranch); err != nil {`
	`60`	`+ if _, parentErr := cfg.GetParent(currentBranch); parentErr != nil {`
`61`	`61`	`return fmt.Errorf("current branch %q is not tracked; use 'gh stack adopt' first", currentBranch)`
`62`	`62`	`}`
`63`	`63`	`}`
Original file line number	Diff line number	Diff line change
`@@ -46,7 +46,7 @@ func runLog(cmd *cobra.Command, args []string) error {`
`46`	`46`	`}`
`47`	`47`
`48`	`48`	`g := git.New(cwd)`
`49`		`- currentBranch, _ := g.CurrentBranch()`
	`49`	`+ currentBranch, _ := g.CurrentBranch() //nolint:errcheck // empty string is fine for display`
`50`	`50`
`51`	`51`	`if logPorcelainFlag {`
`52`	`52`	`printPorcelain(root, currentBranch)`