Create new commits even with no diff (#758)

Pushing to `HEAD` when there are no tracked/staged changes should create a new commit, rather than updating statuses on the resolved commit.

This removes the `len` check from the snapshot logic so that we always create a commit, even with no changes. It also updates the tests to reflect this new behaviour.

Author: mcncl

internal/preflight/snapshot.go

@@ -54,9 +54,8 @@ func WithDebug() SnapshotOption {
 }
 
 // Snapshot pushes the current working tree state to a remote preflight ref.
-// If there are uncommitted changes, it creates a temporary commit containing
-// them without touching the real git index. If the worktree is clean, it
-// pushes HEAD directly.
+// It always creates a distinct commit on top of HEAD (even when the worktree
+// is clean) without touching the real git index.
 func Snapshot(dir string, preflightID uuid.UUID, opts ...SnapshotOption) (*SnapshotResult, error) {
 	cfg := &snapshotConfig{}
 	for _, opt := range opts {
@@ -101,21 +100,20 @@ func Snapshot(dir string, preflightID uuid.UUID, opts ...SnapshotOption) (*Snaps
 
 	branch := fmt.Sprintf("bk/preflight/%s", preflightID.String())
 	ref := fmt.Sprintf("refs/heads/%s", branch)
-	commit := head
 
-	if len(files) > 0 {
-		// Write the tree object.
-		tree, err := gitOutput(dir, env, cfg.debug, "write-tree")
-		if err != nil {
-			return nil, err
-		}
+	// Always write a tree and create a new commit, even when there are no
+	// local changes. This ensures the preflight branch always points to a
+	// distinct commit (not shared with HEAD), which allows commit statuses to
+	// be attributed to the preflight run rather than the base commit.
+	tree, err := gitOutput(dir, env, cfg.debug, "write-tree")
+	if err != nil {
+		return nil, err
+	}
 
-		// Create a commit on top of HEAD.
-		msg := fmt.Sprintf("Preflight snapshot\n\nPreflight Run ID: %s\nBase Commit: %s", preflightID, head)
-		commit, err = gitOutput(dir, env, cfg.debug, "commit-tree", tree, "-p", head, "-m", msg)
-		if err != nil {
-			return nil, err
-		}
+	msg := fmt.Sprintf("Preflight snapshot\n\nPreflight Run ID: %s\nBase Commit: %s", preflightID, head)
+	commit, err := gitOutput(dir, env, cfg.debug, "commit-tree", tree, "-p", head, "-m", msg)
+	if err != nil {
+		return nil, err
 	}
 
 	// Push the commit to the remote branch.

internal/preflight/snapshot_test.go

@@ -373,19 +373,28 @@ func TestSnapshot_CleanWorktree(t *testing.T) {
 		t.Fatalf("Snapshot() error: %v", err)
 	}
 
-	// Should push HEAD directly with no files changed.
 	head := runGit(t, worktree, "rev-parse", "HEAD")
-	if result.Commit != head {
-		t.Errorf("expected HEAD %s, got %s", head, result.Commit)
+
+	// Even with a clean worktree a new commit should always be created so
+	// that commit statuses are attributed to the preflight run rather than
+	// the shared HEAD commit.
+	if result.Commit == head {
+		t.Errorf("expected a new commit distinct from HEAD %s, but got the same SHA", head)
 	}
 
 	if len(result.Files) != 0 {
 		t.Errorf("expected no changed files, got %d", len(result.Files))
 	}
 
-	// The remote branch should exist and point to HEAD.
+	// The new commit should be reachable and its parent should be HEAD.
+	parent := runGit(t, worktree, "rev-parse", result.Commit+"^")
+	if parent != head {
+		t.Errorf("expected parent of snapshot commit to be HEAD %s, got %s", head, parent)
+	}
+
+	// The remote branch should exist and point to the new commit.
 	remoteRef := runGit(t, worktree, "ls-remote", "origin", result.Ref)
-	if !strings.Contains(remoteRef, head) {
-		t.Errorf("remote branch should point to HEAD %s, got %q", head, remoteRef)
+	if !strings.Contains(remoteRef, result.Commit) {
+		t.Errorf("remote branch should point to snapshot commit %s, got %q", result.Commit, remoteRef)
 	}
 }