fix(lifecycle): compare committed files through Git's clean filters

cmd/entire/cli/state.go

@@ -21,8 +21,11 @@ import (
 	"github.com/entireio/cli/cmd/entire/cli/paths"
 	"github.com/entireio/cli/cmd/entire/cli/strategy"
 	"github.com/entireio/cli/cmd/entire/cli/validation"
+	"github.com/entireio/cli/cmd/entire/cli/worktreedir"
 
 	"github.com/go-git/go-git/v6"
+	"github.com/go-git/go-git/v6/plumbing"
+	"github.com/go-git/go-git/v6/plumbing/object"
 )
 
 // PrePromptState stores the state captured before a user prompt
@@ -329,6 +332,17 @@ func detectFileChanges(ctx context.Context, previouslyUntracked []string, status
 // (already condensed by PostCommit) back to FilesTouched via SaveStep. Files not in
 // HEAD or with different content in the working tree are kept. Fails open: if any git
 // operation errors, returns the original list unchanged.
+//
+// "Same content" is decided by Git's own clean filters, via
+// gitrepo.HashWorktreeFiles (git hash-object), not by comparing the working
+// tree's raw bytes against the blob. Under core.autocrlf — the Git for Windows
+// default, and what e2e/testutil/repo.go sets — the working tree holds CRLF
+// while the blob holds LF, so a byte comparison reports every committed text
+// file as still modified. That defeats the caller's "no changes, skip" gate and
+// mints a fresh shadow branch on the new HEAD *after* PostCommit condensed and
+// deleted the old one; nothing condenses that one away, so it outlives the
+// session. The same reasoning applies to .gitattributes eol/text rules and to
+// clean filters such as Git LFS, which a byte comparison also gets wrong.
 func filterToUncommittedFiles(ctx context.Context, files []string, repoRoot string) []string {
 	if len(files) == 0 {
 		return files
@@ -357,7 +371,14 @@ func filterToUncommittedFiles(ctx context.Context, files []string, repoRoot stri
 
 	logCtx := logging.WithComponent(ctx, "filter-uncommitted")
 
+	// Pass 1: split into "not in HEAD" (uncommitted by definition) and
+	// candidates that need a content comparison.
+	type candidate struct {
+		path     string
+		headFile *object.File
+	}
 	var result []string
+	var candidates []candidate
 	for _, relPath := range files {
 		headFile, err := headTree.File(relPath)
 		if err != nil {
@@ -368,26 +389,68 @@ func filterToUncommittedFiles(ctx context.Context, files []string, repoRoot stri
 			result = append(result, relPath)
 			continue
 		}
+		candidates = append(candidates, candidate{path: relPath, headFile: headFile})
+	}
+	if len(candidates) == 0 {
+		return result
+	}
+
+	// Hash the regular files through native Git so the comparison sees the
+	// same clean-filtered bytes Git would have committed. HashableWorktreeEntry
+	// withholds the paths hash-object would answer wrongly or block on — a
+	// symlink on either side, a FIFO, a tracked file replaced by another type —
+	// and those take the raw fallback below instead.
+	hashPaths := make([]string, 0, len(candidates))
+	for _, c := range candidates {
+		if !worktreedir.HashableEntry(repoRoot, c.path, c.headFile.Mode) {
+			continue
+		}
+		hashPaths = append(hashPaths, c.path)
+	}
+	var worktreeHashes map[string]plumbing.Hash
+	if len(hashPaths) > 0 {
+		var hashErr error
+		worktreeHashes, hashErr = gitrepo.HashWorktreeFiles(ctx, repoRoot, hashPaths)
+		if hashErr != nil {
+			// Partial results are still usable; only the paths Git could not
+			// hash fall back to the raw comparison below.
+			logging.Debug(logCtx, "native git could not hash every candidate; falling back to raw comparison for those paths",
+				slog.String("error", hashErr.Error()))
+		}
+	}
+
+	// Pass 2: decide each candidate, preserving the input order.
+	for _, c := range candidates {
+		if worktreeHash, ok := worktreeHashes[c.path]; ok {
+			// Equal, not ==: plumbing.Hash carries an object-format field
+			// alongside its bytes and == compares that field too, which the
+			// tree decoder and FromHex do not always agree on.
+			if !worktreeHash.Equal(c.headFile.Hash) {
+				result = append(result, c.path)
+			}
+			continue
+		}
 
-		// File is in HEAD — compare content with working tree, through the
-		// worktree's shared root. relPath comes from git, so it is already the
-		// coordinate the root reads in.
-		workingContent, ok := readWorktreeFileSafely(repoRoot, relPath)
+		// Fallback for every path withheld above and for anything native Git
+		// could not hash: compare the raw working-tree bytes. Filter-unaware,
+		// so it can report a clean file as modified, which keeps a file rather
+		// than dropping one — the same direction this function already fails.
+		workingContent, ok := readWorktreeFileSafely(repoRoot, c.path)
 		if !ok {
 			// Can't read working tree file (deleted?) — keep it
-			result = append(result, relPath)
+			result = append(result, c.path)
 			continue
 		}
 
-		headContent, err := headFile.Contents()
+		headContent, err := c.headFile.Contents()
 		if err != nil {
-			result = append(result, relPath)
+			result = append(result, c.path)
 			continue
 		}
 
 		if string(workingContent) != headContent {
 			// Working tree differs from HEAD — uncommitted changes
-			result = append(result, relPath)
+			result = append(result, c.path)
 		}
 		// else: content matches HEAD — already committed, skip
 	}

cmd/entire/cli/state_test.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/entireio/cli/cmd/entire/cli/agent/claudecode"
@@ -922,3 +923,113 @@ func TestFilterToUncommittedFiles_ReallyModified(t *testing.T) {
 		t.Errorf("filterToUncommittedFiles() = %v, want [file.txt]", result)
 	}
 }
+
+// TestFilterToUncommittedFiles_CleanFilteredFileIsDropped pins the condition
+// that left a stale shadow branch behind after every commit on Windows.
+//
+// Under core.autocrlf (the Git for Windows default, and what InitRepo and
+// e2e/testutil/repo.go both set) a committed text file holds CRLF in the
+// working tree and LF in its blob. Comparing the two raw byte strings reports
+// the file as still modified, which defeats turn-end's "no changes, skip" gate
+// and mints a fresh shadow branch on the new HEAD right after PostCommit
+// condensed and deleted the old one — and nothing condenses that one away.
+//
+// The file here is fully committed with no subsequent edit, so the only
+// correct answer is to drop it.
+func TestFilterToUncommittedFiles_CleanFilteredFileIsDropped(t *testing.T) {
+	tmpDir := t.TempDir()
+	t.Chdir(tmpDir)
+
+	testutil.InitRepo(t, tmpDir) // sets core.autocrlf=true
+
+	// Written with CRLF and committed through native Git, so the clean filter
+	// normalises the blob to LF while the working tree keeps CRLF.
+	const relPath = "docs/blue.md"
+	testutil.WriteFile(t, tmpDir, relPath, "# Blue\r\n\r\nAbout the colour blue.\r\n")
+	testutil.RunGit(t, tmpDir, "add", relPath)
+	testutil.RunGit(t, tmpDir, "commit", "-m", "Add documentation about the colour blue")
+
+	// Assert the premise rather than assume it: if Git ever stops normalising
+	// here, this test would pass for the wrong reason and stop guarding
+	// anything.
+	blob := testutil.RunGit(t, tmpDir, "cat-file", "-p", "HEAD:"+relPath)
+	if strings.Contains(blob, "\r\n") {
+		t.Fatalf("precondition failed: blob still contains CRLF, so core.autocrlf did not apply")
+	}
+	onDisk, err := os.ReadFile(filepath.Join(tmpDir, filepath.FromSlash(relPath)))
+	if err != nil {
+		t.Fatalf("failed to read working tree file: %v", err)
+	}
+	if !strings.Contains(string(onDisk), "\r\n") {
+		t.Fatalf("precondition failed: working tree file has no CRLF, so the byte comparison would agree anyway")
+	}
+
+	if got := filterToUncommittedFiles(context.Background(), []string{relPath}, tmpDir); len(got) != 0 {
+		t.Errorf("filterToUncommittedFiles() = %v, want [] — the file is committed, so it is not an uncommitted change", got)
+	}
+}
+
+// TestFilterToUncommittedFiles_CleanFilteredFileEditedIsKept is the other half:
+// the clean-filter-aware comparison must still notice a real edit to a file
+// whose line endings are converted, rather than dropping every committed path.
+func TestFilterToUncommittedFiles_CleanFilteredFileEditedIsKept(t *testing.T) {
+	tmpDir := t.TempDir()
+	t.Chdir(tmpDir)
+
+	testutil.InitRepo(t, tmpDir)
+
+	const relPath = "docs/blue.md"
+	testutil.WriteFile(t, tmpDir, relPath, "# Blue\r\n")
+	testutil.RunGit(t, tmpDir, "add", relPath)
+	testutil.RunGit(t, tmpDir, "commit", "-m", "Add blue")
+
+	// A genuine edit, still CRLF-terminated.
+	testutil.WriteFile(t, tmpDir, relPath, "# Blue\r\n\r\nNow with more content.\r\n")
+
+	got := filterToUncommittedFiles(context.Background(), []string{relPath}, tmpDir)
+	if len(got) != 1 || got[0] != relPath {
+		t.Errorf("filterToUncommittedFiles() = %v, want [%s]", got, relPath)
+	}
+}
+
+// TestFilterToUncommittedFiles_TypechangeToSymlinkIsKept pins the hole the
+// clean-filter comparison would otherwise open. git hash-object follows a
+// working-tree symlink and hashes the target's content, so a tracked regular
+// file replaced by a link to identical content hashes equal to its HEAD blob
+// and would be dropped as "already committed" — even though git status calls
+// it a typechange (" T"). Checking only the HEAD mode does not catch it,
+// because the HEAD entry is still a regular file.
+func TestFilterToUncommittedFiles_TypechangeToSymlinkIsKept(t *testing.T) {
+	testutil.SkipWithoutSymlinks(t)
+
+	tmpDir := t.TempDir()
+	t.Chdir(tmpDir)
+
+	testutil.InitRepo(t, tmpDir)
+
+	const relPath = "foo.txt"
+	testutil.WriteFile(t, tmpDir, relPath, "hello\n")
+	testutil.RunGit(t, tmpDir, "add", relPath)
+	testutil.RunGit(t, tmpDir, "commit", "-m", "Add foo")
+
+	// Replace the tracked regular file with a symlink whose target holds the
+	// identical content, so hash-object returns exactly the HEAD blob hash.
+	testutil.WriteFile(t, tmpDir, "elsewhere.txt", "hello\n")
+	if err := os.Remove(filepath.Join(tmpDir, relPath)); err != nil {
+		t.Fatalf("failed to remove tracked file: %v", err)
+	}
+	if err := os.Symlink("elsewhere.txt", filepath.Join(tmpDir, relPath)); err != nil {
+		t.Fatalf("failed to create symlink: %v", err)
+	}
+
+	// Assert the premise: native Git really does hash the link to the blob it
+	// replaced, which is what makes the naive comparison unsafe.
+	if got := strings.TrimSpace(testutil.RunGit(t, tmpDir, "hash-object", "--", relPath)); got !=
+		strings.TrimSpace(testutil.RunGit(t, tmpDir, "rev-parse", "HEAD:"+relPath)) {
+		t.Fatalf("precondition failed: hash-object did not follow the symlink to the HEAD blob (got %s)", got)
+	}
+
+	if got := filterToUncommittedFiles(context.Background(), []string{relPath}, tmpDir); len(got) != 1 || got[0] != relPath {
+		t.Errorf("filterToUncommittedFiles() = %v, want [%s] — a typechange is an uncommitted change", got, relPath)
+	}
+}

cmd/entire/cli/strategy/content_overlap.go

@@ -3,7 +3,6 @@ package strategy
 import (
 	"context"
 	"io"
-	"io/fs"
 	"log/slog"
 
 	"github.com/entireio/cli/cmd/entire/cli/gitrepo"
@@ -517,7 +516,7 @@ func filesWithRemainingAgentChanges(
 			// hash-object follows symlinks and hashes target content, while a Git
 			// symlink blob stores the target path. Compare either side of a mode
 			// mismatch through the confined fallback instead.
-			if !requiresConfinedWorktreeHash(worktreeRoot, candidate.path, candidate.commitMode) {
+			if worktreedir.HashableEntry(worktreeRoot, candidate.path, candidate.commitMode) {
 				paths = append(paths, candidate.path)
 			}
 		}
@@ -576,29 +575,6 @@ func filesWithRemainingAgentChanges(
 	return remaining
 }
 
-func requiresConfinedWorktreeHash(worktreeRoot, filePath string, commitMode filemode.FileMode) bool {
-	if commitMode == filemode.Symlink {
-		return true
-	}
-	root, err := worktreedir.OpenAt(worktreeRoot)
-	if err != nil {
-		return true
-	}
-	name, err := worktreedir.Name(worktreeRoot, filePath)
-	if err != nil {
-		return true
-	}
-	info, err := root.Lstat(name)
-	return err != nil || requiresConfinedWorktreeMode(info.Mode())
-}
-
-func requiresConfinedWorktreeMode(mode fs.FileMode) bool {
-	// Windows uses ModeIrregular for OneDrive Files On-Demand placeholders.
-	// Mask it so placeholder files still receive Git's clean-filter handling,
-	// while every substantive non-regular type remains confined.
-	return mode.Type()&^fs.ModeIrregular != 0
-}
-
 // workingTreeMatchesBlob checks whether the raw file representation hashes to
 // commitHash. It is the filter-unaware fallback for when native Git cannot hash
 // a regular file and the symlink-aware path for Git symlink blobs.

cmd/entire/cli/strategy/content_overlap_test.go

@@ -2,7 +2,6 @@ package strategy
 
 import (
 	"context"
-	"io/fs"
 	"os"
 	"path/filepath"
 	"testing"
@@ -502,30 +501,6 @@ func TestFilesWithRemainingAgentChanges_ComparesWorktreeToCommitNotIndex(t *test
 	assert.Equal(t, []string{"config.go"}, remaining)
 }
 
-func TestRequiresConfinedWorktreeModeAllowsWindowsCloudPlaceholders(t *testing.T) {
-	t.Parallel()
-
-	tests := []struct {
-		name string
-		mode fs.FileMode
-		want bool
-	}{
-		{name: "regular", mode: 0, want: false},
-		{name: "cloud placeholder file", mode: fs.ModeIrregular, want: false},
-		{name: "directory", mode: fs.ModeDir, want: true},
-		{name: "cloud placeholder directory", mode: fs.ModeDir | fs.ModeIrregular, want: true},
-		{name: "symlink", mode: fs.ModeSymlink, want: true},
-		{name: "symlink irregular", mode: fs.ModeSymlink | fs.ModeIrregular, want: true},
-		{name: "named pipe", mode: fs.ModeNamedPipe, want: true},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-			assert.Equal(t, tt.want, requiresConfinedWorktreeMode(tt.mode))
-		})
-	}
-}
-
 func TestWorkingTreeMatchesBlobSymlinkHashesTheTargetPath(t *testing.T) {
 	testutil.SkipWithoutSymlinks(t)
 	t.Parallel()
@@ -537,9 +512,9 @@ func TestWorkingTreeMatchesBlobSymlinkHashesTheTargetPath(t *testing.T) {
 	_, err := h.Write([]byte(target))
 	require.NoError(t, err)
 
-	assert.True(t, requiresConfinedWorktreeHash(dir, "link.txt", filemode.Symlink))
-	assert.True(t, requiresConfinedWorktreeHash(dir, "link.txt", filemode.Regular),
-		"a working-tree symlink must not be sent to hash-object even if the commit is regular")
+	// Withholding a worktree symlink from hash-object is gitrepo's rule now
+	// (TestHashableWorktreeEntry_WorktreeSymlinkIsWithheld); what stays here is
+	// what this fallback must then answer for one.
 	assert.True(t, workingTreeMatchesBlob(dir, "link.txt", filemode.Symlink, h.Sum()))
 	assert.False(t, workingTreeMatchesBlob(dir, "link.txt", filemode.Regular, h.Sum()),
 		"a symlink must not compare clean against a regular-file commit")