fix(reset): retry reset push on non-fast-forward, re-applying the baseline

internal/reset/reset.go

@@ -9,6 +9,7 @@ import (
 	"os/exec"
 	"path/filepath"
 	"strings"
+	"time"
 
 	"gopkg.in/yaml.v3"
 
@@ -317,19 +318,117 @@ func (r *Resetter) commitAndPush() error {
 		return err
 	}
 
-	// Add, commit, push
+	// Add and commit the baseline reset.
 	if err := r.gitRun("add", configPath); err != nil {
 		return err
 	}
-	if err := r.gitRun("commit", "-m", "chore: reset state for testing [skip ci]"); err != nil {
+	if err := r.gitRun("commit", "-m", resetCommitMessage); err != nil {
 		return err
 	}
-	if err := r.gitRun("push"); err != nil {
-		return err
+
+	return r.pushWithRetry(configPath)
+}
+
+// resetCommitMessage is the commit subject used for every state-reset commit,
+// including each re-applied attempt after a non-fast-forward push.
+const resetCommitMessage = "chore: reset state for testing [skip ci]"
+
+// maxPushAttempts bounds the non-fast-forward recovery loop, matching the bound
+// used by the other state-write push paths.
+const maxPushAttempts = 3
+
+// pushWithRetry pushes the committed reset, recovering from a non-fast-forward
+// rejection. Concurrent writers can advance the trunk between the reset's read
+// and its push; on rejection it fetches the updated trunk, re-applies the
+// baseline reset on top of it, recommits, and retries. The reset always wins:
+// the baseline overwrites the state section rather than merging a concurrent
+// writer's state back in.
+func (r *Resetter) pushWithRetry(configPath string) error {
+	for attempt := 1; ; attempt++ {
+		pushErr := r.gitRun("push")
+		if pushErr == nil {
+			log.Info("Committed and pushed state reset")
+			return nil
+		}
+
+		if attempt >= maxPushAttempts {
+			return fmt.Errorf("push rejected after %d attempts: %w", maxPushAttempts, pushErr)
+		}
+
+		log.Warn("Push rejected (attempt %d/%d); re-applying reset onto updated trunk", attempt, maxPushAttempts)
+		recommitted, err := r.reapplyResetOntoTrunk(configPath)
+		if err != nil {
+			return err
+		}
+		if !recommitted {
+			// The trunk already carries the baseline state, so there is nothing
+			// left for this reset to push.
+			log.Info("Trunk already at baseline state after fetch; nothing to push")
+			return nil
+		}
+
+		time.Sleep(pushRetryBackoff)
 	}
+}
 
-	log.Info("Committed and pushed state reset")
-	return nil
+// pushRetryBackoff is the short pause between non-fast-forward recovery attempts.
+const pushRetryBackoff = 2 * time.Second
+
+// reapplyResetOntoTrunk fetches the current trunk, hard-resets the working tree
+// onto it, re-applies the baseline reset to the freshly fetched manifest, and
+// commits the result. It reports whether a new commit was created; a false value
+// means the trunk is already at the baseline and there is nothing to push.
+func (r *Resetter) reapplyResetOntoTrunk(configPath string) (bool, error) {
+	branch, err := r.currentBranch()
+	if err != nil {
+		return false, err
+	}
+
+	if err := r.gitRun("fetch", "origin", branch); err != nil {
+		return false, fmt.Errorf("fetch origin during non-fast-forward recovery: %w", err)
+	}
+	if err := r.gitRun("reset", "--hard", "origin/"+branch); err != nil {
+		return false, fmt.Errorf("reset to trunk tip during non-fast-forward recovery: %w", err)
+	}
+
+	// Re-read the freshly fetched manifest so the baseline is applied on top of
+	// the current trunk, not the stale copy the reset first read.
+	cicdFile, err := config.ParseManifestFile(r.configPath, r.manifestKey)
+	if err != nil {
+		return false, fmt.Errorf("reload manifest during non-fast-forward recovery: %w", err)
+	}
+	r.cicdFile = cicdFile
+	r.cicdFile.State = nil
+	r.cicdFile.LatestRelease = nil
+	if err := r.writeConfig(); err != nil {
+		return false, fmt.Errorf("rewrite baseline during non-fast-forward recovery: %w", err)
+	}
+
+	status, _ := r.gitOutput("status", "--porcelain", configPath)
+	if strings.TrimSpace(status) == "" {
+		return false, nil
+	}
+
+	if err := r.gitRun("add", configPath); err != nil {
+		return false, err
+	}
+	if err := r.gitRun("commit", "-m", resetCommitMessage); err != nil {
+		return false, err
+	}
+	return true, nil
+}
+
+// currentBranch returns the checked-out branch name, scoped to the repo path.
+func (r *Resetter) currentBranch() (string, error) {
+	branch, err := r.gitOutput("rev-parse", "--abbrev-ref", "HEAD")
+	if err != nil {
+		return "", fmt.Errorf("resolve current branch: %w", err)
+	}
+	branch = strings.TrimSpace(branch)
+	if branch == "" || branch == "HEAD" {
+		return "", fmt.Errorf("resolve current branch: detached or empty HEAD")
+	}
+	return branch, nil
 }
 
 // Helper functions

internal/reset/reset_push_test.go

@@ -0,0 +1,150 @@
+package reset
+
+import (
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/stablekernel/cascade/internal/config"
+)
+
+// gitInDir runs a git command scoped to dir (matching how the Resetter scopes
+// every git invocation to r.repoPath) and fails the test on error.
+func gitInDir(t *testing.T, dir string, args ...string) {
+	t.Helper()
+	full := append([]string{"-C", dir}, args...)
+	if out, err := exec.Command("git", full...).CombinedOutput(); err != nil {
+		t.Fatalf("git %s: %v\n%s", strings.Join(args, " "), err, out)
+	}
+}
+
+// configureGitIdentity sets a committer identity and disables signing so commits
+// succeed in a bare CI sandbox without a configured user or GPG key.
+func configureGitIdentity(t *testing.T, dir string) {
+	t.Helper()
+	gitInDir(t, dir, "config", "user.email", "tester@example.com")
+	gitInDir(t, dir, "config", "user.name", "Reset Tester")
+	gitInDir(t, dir, "config", "commit.gpgsign", "false")
+}
+
+// originHeadManifest returns the manifest contents at the origin's main tip.
+func originHeadManifest(t *testing.T, origin, relPath string) string {
+	t.Helper()
+	out, err := exec.Command("git", "-C", origin, "show", "main:"+relPath).Output()
+	if err != nil {
+		t.Fatalf("git show main:%s: %v", relPath, err)
+	}
+	return string(out)
+}
+
+// TestCommitAndPush_RecoversFromNonFastForward proves the seeding race that broke
+// the multi-environment lifecycle job: a concurrent writer advances origin/main
+// between the reset's read and its push, so a plain push is rejected
+// non-fast-forward. commitAndPush must fetch, re-apply the baseline reset on top
+// of the updated trunk, and push. The baseline reset must WIN, leaving the
+// concurrent writer's state overwritten rather than merged back in.
+func TestCommitAndPush_RecoversFromNonFastForward(t *testing.T) {
+	const relPath = "manifest.yaml"
+
+	// Bare origin shared by the reset checkout and the concurrent writer.
+	origin := t.TempDir()
+	if out, err := exec.Command("git", "init", "--bare", "-b", "main", origin).CombinedOutput(); err != nil {
+		t.Fatalf("git init --bare: %v\n%s", err, out)
+	}
+
+	// Seed origin with a manifest that already carries state. This is the value
+	// the reset checkout reads before the race.
+	const seed = `ci:
+  config:
+    trunk_branch: main
+    environments:
+      - dev
+  state:
+    dev:
+      sha: seedsha
+      version: v1.0.0
+`
+	bootstrap := t.TempDir()
+	gitInDir(t, "", "clone", origin, bootstrap)
+	configureGitIdentity(t, bootstrap)
+	require.NoError(t, os.WriteFile(filepath.Join(bootstrap, relPath), []byte(seed), 0o600))
+	gitInDir(t, bootstrap, "add", relPath)
+	gitInDir(t, bootstrap, "commit", "-m", "seed manifest")
+	gitInDir(t, bootstrap, "push", "origin", "HEAD:main")
+
+	// The reset checkout: this is the repo under test, scoped via repoPath.
+	resetRepo := t.TempDir()
+	gitInDir(t, "", "clone", origin, resetRepo)
+	configureGitIdentity(t, resetRepo)
+
+	configPath := filepath.Join(resetRepo, relPath)
+	cicdFile, err := config.ParseManifestFile(configPath, config.DefaultManifestKey)
+	require.NoError(t, err)
+
+	r := &Resetter{
+		opts: Options{
+			RepoPath:   resetRepo,
+			ResetState: true,
+			Push:       true,
+		},
+		repoPath:    resetRepo,
+		repoOwner:   "test",
+		repoName:    "repo",
+		configPath:  configPath,
+		manifestKey: config.DefaultManifestKey,
+		cicdFile:    cicdFile,
+	}
+
+	// The reset clears state on disk, mirroring Reset() calling resetState()
+	// before commitAndPush().
+	require.NoError(t, r.resetState())
+
+	// A concurrent writer advances origin/main AFTER the reset read its copy,
+	// writing a fresh promotion into the same state section. The reset checkout's
+	// parent is now stale, so its next push is rejected non-fast-forward.
+	writer := t.TempDir()
+	gitInDir(t, "", "clone", origin, writer)
+	configureGitIdentity(t, writer)
+	const writerManifest = `ci:
+  config:
+    trunk_branch: main
+    environments:
+      - dev
+  state:
+    dev:
+      sha: writersha
+      version: v2.0.0
+`
+	require.NoError(t, os.WriteFile(filepath.Join(writer, relPath), []byte(writerManifest), 0o600))
+	gitInDir(t, writer, "add", relPath)
+	gitInDir(t, writer, "commit", "-m", "concurrent promote")
+	gitInDir(t, writer, "push", "origin", "HEAD:main")
+
+	// commitAndPush must self-heal across the rejected push.
+	if err := r.commitAndPush(); err != nil {
+		t.Fatalf("commitAndPush() error = %v, want nil (must recover from non-fast-forward)", err)
+	}
+
+	// The baseline reset must win: origin's tip carries neither the seed state nor
+	// the concurrent writer's state.
+	got := originHeadManifest(t, origin, relPath)
+	if strings.Contains(got, "writersha") {
+		t.Errorf("reset did not win: origin HEAD still carries concurrent writer state; got:\n%s", got)
+	}
+	if strings.Contains(got, "seedsha") {
+		t.Errorf("origin HEAD still carries the pre-reset seed state; got:\n%s", got)
+	}
+	if strings.Contains(got, "state:") {
+		t.Errorf("state section was not reset to baseline; got:\n%s", got)
+	}
+	// The non-state config the writer left in place must survive the rebase.
+	if !strings.Contains(got, "trunk_branch: main") {
+		t.Errorf("origin HEAD lost config section after recovery; got:\n%s", got)
+	}
+
+	_ = bootstrap
+}