From 110ff247e0b28b3a52f37f7a85d2c0aba9c69d4c Mon Sep 17 00:00:00 2001
From: Sebastien Tardif <sebtardif@ncf.ca>
Date: Wed, 13 May 2026 06:37:32 -0700
Subject: [PATCH] fix(cli/alpha): remove redundant guard and check checkout
 error in alpha update push

Remove a redundant nested if opts.Push guard and check the git checkout
error that was previously discarded with _ in the alpha update push path.

The inner if opts.Push on line 209 is always true because the outer guard
on line 208 already checks the same field. This is a copy-paste artifact.

The git checkout error on line 211 is silently discarded with _. While the
checkout is typically a no-op (the squash path already leaves the user on
the output branch), silently discarding the error means any systemic git
failure (corrupted state, locked index) is hidden from the user, who sees
Update completed successfully despite the error.

The error check pattern matches the identical checkout handling at
lines 197-198 for the ShowCommits path.

Introduced in #5013 (2025-08-18).

Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
---
 internal/cli/alpha/internal/update/update.go  | 12 +++----
 .../cli/alpha/internal/update/update_test.go  | 31 +++++++++++++++++++
 2 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/internal/cli/alpha/internal/update/update.go b/internal/cli/alpha/internal/update/update.go
index 2a5e18f3867..898135b7af3 100644
--- a/internal/cli/alpha/internal/update/update.go
+++ b/internal/cli/alpha/internal/update/update.go
@@ -206,12 +206,12 @@ func (opts *Update) Update() error {
 
 	// Push the output branch if requested
 	if opts.Push {
-		if opts.Push {
-			out := opts.getOutputBranchName()
-			_ = helpers.GitCmd(opts.GitConfig, "checkout", out).Run()
-			if err := helpers.GitCmd(opts.GitConfig, "push", "-u", "origin", out).Run(); err != nil {
-				return fmt.Errorf("failed to push %s: %w", out, err)
-			}
+		out := opts.getOutputBranchName()
+		if err := helpers.GitCmd(opts.GitConfig, "checkout", out).Run(); err != nil {
+			return fmt.Errorf("failed to checkout %s: %w", out, err)
+		}
+		if err := helpers.GitCmd(opts.GitConfig, "push", "-u", "origin", out).Run(); err != nil {
+			return fmt.Errorf("failed to push %s: %w", out, err)
 		}
 	}
 
diff --git a/internal/cli/alpha/internal/update/update_test.go b/internal/cli/alpha/internal/update/update_test.go
index aa8c99ffee6..9aa5d102844 100644
--- a/internal/cli/alpha/internal/update/update_test.go
+++ b/internal/cli/alpha/internal/update/update_test.go
@@ -167,6 +167,37 @@ exit 1`
 				fmt.Sprintf("checkout %s", opts.FromBranch),
 			))
 		})
+
+		It("pushes the output branch when Push is enabled", func() {
+			opts.Push = true
+			err = opts.Update()
+			Expect(err).ToNot(HaveOccurred())
+
+			logs, readErr := os.ReadFile(logFile)
+			Expect(readErr).ToNot(HaveOccurred())
+			s := string(logs)
+
+			out := opts.getOutputBranchName()
+			Expect(s).To(ContainSubstring(
+				fmt.Sprintf("push -u origin %s", out),
+			))
+		})
+
+		It("returns error when checkout fails before push", func() {
+			opts.Push = true
+			out := opts.getOutputBranchName()
+			failOnPushCheckout := fmt.Sprintf(`#!/bin/bash
+echo "$@" >> "%s"
+if [[ "$1" == "checkout" && "$2" == "%s" ]]; then exit 1; fi
+exit 0`, logFile, out)
+			Expect(mockBinResponse(failOnPushCheckout, mockGit)).To(Succeed())
+
+			err = opts.Update()
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring(
+				fmt.Sprintf("failed to checkout %s", out),
+			))
+		})
 	})
 
 	Context("RegenerateProjectWithVersion", func() {