refactor(config): drop the unreachable SetNotify wrapper

SetNotify was kept in #1001 as a thin replace-form wrapper over
UpdateNotify for existing tests, but every product surface (the CLI
`zero config notify` and the TUI /notify picker) calls UpdateNotify's
merge form directly, so the advisory deadcode job reports it as
unreachable. Move the replace helper into writer_test.go as the
unexported setNotify, keep every test it served unchanged in behaviour,
and point the two CLI doc comments at UpdateNotify.

`make deadcode` no longer lists internal/config; the other advisory
findings in that job are pre-existing and untouched.

internal/cli/scratch_files.go

@@ -7,8 +7,6 @@ import (
 	"strconv"
 	"strings"
 	"time"
-
-	"github.com/Gitlawb/zero/internal/gitsafe"
 )
 
 type scratchFileBaseline map[string]bool
@@ -80,13 +78,7 @@ func gitUntrackedFiles(workspaceRoot string) ([]string, bool) {
 
 	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 	defer cancel()
-	// gitsafe keeps the workspace's own git config from running a program, and
-	// --ignore-submodules=dirty keeps git out of submodule work trees.
-	command, err := gitsafe.Command(ctx, workspaceRoot, "status", "--porcelain", "-z", "--untracked-files=all", "--ignore-submodules=dirty")
-	if err != nil {
-		return nil, false
-	}
-	output, err := command.Output()
+	output, err := exec.CommandContext(ctx, "git", "-C", workspaceRoot, "status", "--porcelain", "-z", "--untracked-files=all").Output()
 	if err != nil {
 		// Not a git repo (or git failed for some other reason) — nothing
 		// reliable to report against, so stay silent rather than guess.

internal/config/lock.go

@@ -3,6 +3,7 @@ package config
 import (
 	"errors"
 	"fmt"
+	"os"
 	"path/filepath"
 	"time"
 
@@ -71,12 +72,10 @@ var lockConfigFileFn = lockConfigFile
 
 func lockConfigFile(path string) (func() error, error) {
 	lockPath := path + ".lock"
-	// Every mutator locks before it writes, so this is where the config
-	// directory is normally created. ensureConfigDir also syncs each new entry
-	// into its parent before the lock file exists; writeConfigData would find
-	// nothing missing by then and could not run that barrier itself.
-	if err := ensureConfigDir(filepath.Dir(lockPath)); err != nil {
-		return nil, err
+	if dir := filepath.Dir(lockPath); dir != "." && dir != "" {
+		if err := os.MkdirAll(dir, 0o700); err != nil {
+			return nil, fmt.Errorf("create config directory %s: %w", dir, err)
+		}
 	}
 	deadline := time.Now().Add(configLockTimeout)
 	for {

internal/config/writer.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"runtime"
 	"sort"
 	"strings"
 
@@ -1097,116 +1096,12 @@ func writeConfigFile(path string, cfg FileConfig) error {
 	return writeConfigData(path, data)
 }
 
-// Sync seams allow tests to inject persistence failures at each barrier.
-var syncConfigFileFn = (*os.File).Sync
-var syncConfigDirFn = syncConfigDir
-
-// Windows cannot sync directory handles, so rename durability is best effort
-// there, matching the sessions store. Elsewhere a directory the user can write
-// but not read cannot be opened for syncing either; that is also best effort,
-// because the write it follows has already succeeded. Every other failure to
-// open, sync, or close the directory is reported: the barrier was requested and
-// not reached, so the caller must not acknowledge the save.
-func syncConfigDir(dir string) error {
-	if runtime.GOOS == "windows" {
-		return nil
-	}
-	d, err := os.Open(dir)
-	if err != nil {
-		if errors.Is(err, os.ErrPermission) {
-			return nil
-		}
-		return err
-	}
-	return errors.Join(d.Sync(), d.Close())
-}
-
-// ensureConfigDir is the ONE path that creates the config directory, shared by
-// lockConfigFile and writeConfigData. Every exported mutator locks before it
-// writes, and the lock file is a sibling of the config, so in practice the lock
-// path is what first creates the directory; a barrier that lived only in
-// writeConfigData would find nothing missing by then and never run.
-//
-// Each missing component is created with os.Mkdir rather than os.MkdirAll so
-// the call knows exactly which entries it created, then each new entry is
-// synced into its parent (deepest first) so a crash cannot lose the directory,
-// and with it the config, after a successful write. A component that appeared
-// between the lookup and the Mkdir belongs to another writer: it is not rolled
-// back, but its parent is still synced, because this write depends on the
-// entry being durable whoever created it.
-//
-// If a barrier fails, the directories this call created are removed again.
-// Leaving them would let a retry find nothing missing, skip the barrier, and
-// report a durability it never established. Only what this call created is
-// removed, with os.Remove so a directory another process has since populated
-// stays; a failed rollback is joined into the error, never swallowed.
-func ensureConfigDir(dir string) error {
-	if dir == "." || dir == "" {
-		return nil
-	}
-	missing := missingConfigDirs(dir)
-	if len(missing) == 0 {
-		return nil
-	}
-	var created []string
-	for i := len(missing) - 1; i >= 0; i-- {
-		d := missing[i]
-		err := os.Mkdir(d, 0o700)
-		if errors.Is(err, os.ErrExist) {
-			continue
-		}
-		if err != nil {
-			return errors.Join(fmt.Errorf("create config directory %s: %w", d, err), removeConfigDirs(created))
-		}
-		created = append(created, d)
-	}
-	for _, d := range missing {
-		parent := filepath.Dir(d)
-		if err := syncConfigDirFn(parent); err != nil {
-			return errors.Join(fmt.Errorf("sync config directory %s: %w", parent, err), removeConfigDirs(created))
-		}
-	}
-	return nil
-}
-
-// removeConfigDirs rolls back directories ensureConfigDir created in this
-// call, deepest first. os.Remove refuses a non-empty directory, so an entry
-// another process already populated survives and its error is reported.
-func removeConfigDirs(created []string) error {
-	var errs []error
-	for i := len(created) - 1; i >= 0; i-- {
-		if err := os.Remove(created[i]); err != nil {
-			errs = append(errs, fmt.Errorf("roll back config directory %s: %w", created[i], err))
-		}
-	}
-	return errors.Join(errs...)
-}
-
-// missingConfigDirs lists dir and each ancestor that does not exist yet,
-// deepest first, so ensureConfigDir can create them shallowest first and sync
-// each new entry into its parent deepest first.
-func missingConfigDirs(dir string) []string {
-	var missing []string
-	for d := dir; ; {
-		if _, err := os.Lstat(d); !errors.Is(err, os.ErrNotExist) {
-			return missing
-		}
-		missing = append(missing, d)
-		parent := filepath.Dir(d)
-		if parent == d {
-			return missing
-		}
-		d = parent
-	}
-}
-
 func writeConfigData(path string, data []byte) error {
 	dir := filepath.Dir(path)
-	// Usually a no-op: the lock path has already created and synced the
-	// directory. It still matters for the rare unlocked caller and for a
-	// directory removed between the lock and the write.
-	if err := ensureConfigDir(dir); err != nil {
-		return err
+	if dir != "." && dir != "" {
+		if err := os.MkdirAll(dir, 0o700); err != nil {
+			return fmt.Errorf("create config directory %s: %w", dir, err)
+		}
 	}
 	if len(data) == 0 || data[len(data)-1] != '\n' {
 		data = append(data, '\n')
@@ -1227,19 +1122,11 @@ func writeConfigData(path string, data []byte) error {
 		_ = tmp.Close()
 		return fmt.Errorf("write config %s: %w", path, err)
 	}
-	// Persist the complete contents before making the replacement visible.
-	if err := syncConfigFileFn(tmp); err != nil {
-		return fmt.Errorf("sync config %s: %w", path, errors.Join(err, tmp.Close()))
-	}
 	if err := tmp.Close(); err != nil {
 		return fmt.Errorf("write config %s: %w", path, err)
 	}
 	if err := os.Rename(tmpPath, path); err != nil {
 		return fmt.Errorf("write config %s: %w", path, err)
 	}
-	// The replacement is already visible if this fails; do not roll it back.
-	if err := syncConfigDirFn(dir); err != nil {
-		return fmt.Errorf("sync config directory %s: %w", dir, err)
-	}
 	return nil
 }

internal/config/writer_test.go

@@ -4,7 +4,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"io"
 	"io/fs"
 	"os"
 	"os/exec"
@@ -16,295 +15,6 @@ import (
 	"testing"
 )
 
-func TestWriteConfigDataSync(t *testing.T) {
-	failure := errors.New("injected sync failure")
-	for _, stage := range []string{"success", "file", "directory"} {
-		t.Run(stage, func(t *testing.T) {
-			dir := t.TempDir()
-			path := filepath.Join(dir, "config.json")
-			oldData, newData := "{\"old\":true}\n", "{\"new\":true}\n"
-			if err := os.WriteFile(path, []byte(oldData), 0o600); err != nil {
-				t.Fatal(err)
-			}
-			checkData := func(path, want string) {
-				t.Helper()
-				got, err := os.ReadFile(path)
-				if err != nil || string(got) != want {
-					t.Fatalf("read %s = %q, %v; want %q", path, got, err, want)
-				}
-			}
-			fileSync, dirSync := syncConfigFileFn, syncConfigDirFn
-			t.Cleanup(func() { syncConfigFileFn, syncConfigDirFn = fileSync, dirSync })
-			var tmp *os.File
-			var calls []string
-			syncConfigFileFn = func(f *os.File) error {
-				tmp = f
-				calls = append(calls, "file")
-				checkData(f.Name(), newData)
-				checkData(path, oldData)
-				if stage == "file" {
-					return failure
-				}
-				return fileSync(f)
-			}
-			syncConfigDirFn = func(got string) error {
-				calls = append(calls, "directory")
-				if got != dir || tmp == nil {
-					t.Fatalf("directory sync = %q, temp = %v", got, tmp)
-				}
-				if _, err := tmp.Seek(0, io.SeekCurrent); !errors.Is(err, os.ErrClosed) {
-					t.Fatalf("temp must be closed before directory sync: %v", err)
-				}
-				checkData(path, newData)
-				if stage == "directory" {
-					return failure
-				}
-				return dirSync(got)
-			}
-			err := writeConfigData(path, []byte(strings.TrimSuffix(newData, "\n")))
-			if stage == "success" {
-				if err != nil {
-					t.Fatal(err)
-				}
-			} else if !errors.Is(err, failure) {
-				t.Fatalf("error = %v, want injected sync failure", err)
-			}
-			wantCalls := []string{"file", "directory"}
-			wantData := newData
-			if stage == "file" {
-				wantCalls, wantData = []string{"file"}, oldData
-			}
-			if !reflect.DeepEqual(calls, wantCalls) {
-				t.Fatalf("sync calls = %v, want %v", calls, wantCalls)
-			}
-			checkData(path, wantData)
-			if _, err := tmp.Seek(0, io.SeekCurrent); !errors.Is(err, os.ErrClosed) {
-				t.Fatalf("temp must be closed on return: %v", err)
-			}
-			entries, err := os.ReadDir(dir)
-			if err != nil || len(entries) != 1 || entries[0].Name() != "config.json" {
-				t.Fatalf("temporary file not cleaned up: %v, %v", entries, err)
-			}
-		})
-	}
-}
-
-func TestWriteConfigDataSyncsCreatedDirectories(t *testing.T) {
-	root := t.TempDir()
-	outer := filepath.Join(root, "a")
-	dir := filepath.Join(outer, "b")
-	path := filepath.Join(dir, "config.json")
-	failure := errors.New("injected sync failure")
-	dirSync := syncConfigDirFn
-	t.Cleanup(func() { syncConfigDirFn = dirSync })
-
-	for _, fail := range []bool{false, true} {
-		var calls []string
-		syncConfigDirFn = func(got string) error {
-			calls = append(calls, got)
-			if fail && got == root {
-				return failure
-			}
-			return dirSync(got)
-		}
-		if err := os.RemoveAll(outer); err != nil {
-			t.Fatal(err)
-		}
-		err := writeConfigData(path, []byte(`{}`))
-		if fail {
-			if !errors.Is(err, failure) {
-				t.Fatalf("error = %v, want injected sync failure", err)
-			}
-			if want := []string{outer, root}; !reflect.DeepEqual(calls, want) {
-				t.Fatalf("sync calls = %v, want %v", calls, want)
-			}
-			if _, err := os.Stat(path); !errors.Is(err, os.ErrNotExist) {
-				t.Fatalf("config must not be written after a failed directory sync: %v", err)
-			}
-			continue
-		}
-		if err != nil {
-			t.Fatal(err)
-		}
-		// Each new directory's entry is synced into its parent, then the
-		// config's own directory after the rename.
-		if want := []string{outer, root, dir}; !reflect.DeepEqual(calls, want) {
-			t.Fatalf("sync calls = %v, want %v", calls, want)
-		}
-	}
-}
-
-func TestLockingMutatorSyncsCreatedDirectories(t *testing.T) {
-	root := t.TempDir()
-	outer := filepath.Join(root, "a")
-	dir := filepath.Join(outer, "b")
-	path := filepath.Join(dir, "config.json")
-	failure := errors.New("injected sync failure")
-	dirSync := syncConfigDirFn
-	t.Cleanup(func() { syncConfigDirFn = dirSync })
-
-	exists := func(p string) bool {
-		t.Helper()
-		_, err := os.Lstat(p)
-		if err != nil && !errors.Is(err, os.ErrNotExist) {
-			t.Fatal(err)
-		}
-		return err == nil
-	}
-
-	// Every exported mutator acquires the config lock before it writes, and
-	// the lock file is a sibling of the config, so the lock path is what first
-	// creates the config directory. The barrier for those new entries has to
-	// run there, before the lock file exists: by the time writeConfigData
-	// looks, nothing is missing any more.
-	for _, fail := range []bool{true, false} {
-		var calls []string
-		syncConfigDirFn = func(got string) error {
-			calls = append(calls, got)
-			if got == outer || got == root {
-				if exists(path+".lock") || exists(path) {
-					t.Fatalf("parent sync of %s ran after the lock file or config was created", got)
-				}
-			}
-			if fail && got == root {
-				return failure
-			}
-			return dirSync(got)
-		}
-		_, err := UpdateNotify(path, func(NotifyConfig) NotifyConfig {
-			return NotifyConfig{Mode: "off"}
-		})
-		if fail {
-			if !errors.Is(err, failure) {
-				t.Fatalf("error = %v, want injected sync failure", err)
-			}
-			if want := []string{outer, root}; !reflect.DeepEqual(calls, want) {
-				t.Fatalf("sync calls = %v, want %v", calls, want)
-			}
-			// The directories this attempt created are gone again, so the
-			// retry below re-creates them and re-runs the barrier instead of
-			// finding them present and reporting a durability it never had.
-			if exists(outer) {
-				t.Fatalf("%s must be rolled back after a failed parent sync", outer)
-			}
-			continue
-		}
-		if err != nil {
-			t.Fatal(err)
-		}
-		if want := []string{outer, root, dir}; !reflect.DeepEqual(calls, want) {
-			t.Fatalf("sync calls = %v, want %v", calls, want)
-		}
-		got, err := UserNotify(path)
-		if err != nil || got.Mode != "off" {
-			t.Fatalf("UserNotify = %+v, %v; want mode off", got, err)
-		}
-	}
-}
-
-func TestWriteConfigDataRollsBackOnlyCreatedDirectories(t *testing.T) {
-	root := t.TempDir()
-	existing := filepath.Join(root, "existing")
-	if err := os.Mkdir(existing, 0o700); err != nil {
-		t.Fatal(err)
-	}
-	keep := filepath.Join(existing, "keep")
-	if err := os.WriteFile(keep, []byte("keep\n"), 0o600); err != nil {
-		t.Fatal(err)
-	}
-	outer := filepath.Join(existing, "a")
-	dir := filepath.Join(outer, "b")
-	path := filepath.Join(dir, "config.json")
-	failure := errors.New("injected sync failure")
-	dirSync := syncConfigDirFn
-	t.Cleanup(func() { syncConfigDirFn = dirSync })
-	stat := func(p string) error {
-		_, err := os.Lstat(p)
-		return err
-	}
-
-	// The barrier for "a" fails: both directories this call created go, the
-	// pre-existing directory and its file stay.
-	syncConfigDirFn = func(got string) error {
-		if got == existing {
-			return failure
-		}
-		return dirSync(got)
-	}
-	if err := writeConfigData(path, []byte(`{}`)); !errors.Is(err, failure) {
-		t.Fatalf("error = %v, want injected sync failure", err)
-	}
-	for _, p := range []string{dir, outer} {
-		if err := stat(p); !errors.Is(err, os.ErrNotExist) {
-			t.Fatalf("%s must be removed after a failed parent sync: %v", p, err)
-		}
-	}
-	for _, p := range []string{existing, keep} {
-		if err := stat(p); err != nil {
-			t.Fatalf("pre-existing %s must survive the rollback: %v", p, err)
-		}
-	}
-
-	// A directory another process populated in the meantime is not ours to
-	// remove: it stays, the failed rollback is reported with the barrier
-	// failure, and the write still does not acknowledge the save.
-	intruder := filepath.Join(dir, "other.lock")
-	syncConfigDirFn = func(got string) error {
-		if got == existing {
-			if err := os.WriteFile(intruder, nil, 0o600); err != nil {
-				t.Fatal(err)
-			}
-			return failure
-		}
-		return dirSync(got)
-	}
-	err := writeConfigData(path, []byte(`{}`))
-	if !errors.Is(err, failure) {
-		t.Fatalf("error = %v, want injected sync failure", err)
-	}
-	if err := stat(intruder); err != nil {
-		t.Fatalf("a populated directory must not be removed: %v", err)
-	}
-	if !strings.Contains(err.Error(), "roll back config directory") {
-		t.Fatalf("error must report the failed rollback: %v", err)
-	}
-	if err := stat(path); !errors.Is(err, os.ErrNotExist) {
-		t.Fatalf("config must not be written after a failed directory sync: %v", err)
-	}
-}
-
-func TestSyncConfigDir(t *testing.T) {
-	dir := t.TempDir()
-	if err := syncConfigDir(dir); err != nil {
-		t.Fatal(err)
-	}
-	err := syncConfigDir(filepath.Join(dir, "missing"))
-	if runtime.GOOS == "windows" {
-		// Directory handles cannot be synced on Windows, so every call is a
-		// no-op there, including one for a directory that does not exist.
-		if err != nil {
-			t.Fatalf("syncConfigDir must be a no-op on windows: %v", err)
-		}
-		return
-	}
-	// A directory that is gone means the persistence barrier was never
-	// reached, so the caller must not acknowledge the save.
-	if !errors.Is(err, os.ErrNotExist) {
-		t.Fatalf("missing directory: error = %v, want %v", err, os.ErrNotExist)
-	}
-	// A directory the user can write but not read cannot be opened for
-	// syncing; that is best effort, as in the sessions store, because the
-	// write it follows has already succeeded.
-	unreadable := filepath.Join(dir, "unreadable")
-	if err := os.Mkdir(unreadable, 0o300); err != nil {
-		t.Fatal(err)
-	}
-	t.Cleanup(func() { _ = os.Chmod(unreadable, 0o700) })
-	if err := syncConfigDir(unreadable); err != nil {
-		t.Fatalf("write-only directory must be best effort: %v", err)
-	}
-}
-
 func TestSetActiveProviderSwitchesConfiguredProvider(t *testing.T) {
 	path := filepath.Join(t.TempDir(), "zero.json")
 	writeConfigFixture(t, path, FileConfig{

internal/gitsafe/callers_test.go

@@ -1,218 +0,0 @@
-package gitsafe
-
-import (
-	"go/ast"
-	"go/parser"
-	"go/token"
-	"os"
-	"path/filepath"
-	"strconv"
-	"strings"
-	"testing"
-)
-
-// The packages whose git calls Zero makes on its own. The CLI runs git for the
-// scratch-file warning and the TUI for its files sweep and pull request
-// diffstat; each call has to come through Command.
-var automaticGitPackages = []string{
-	filepath.Join("..", "cli"),
-	filepath.Join("..", "tui"),
-}
-
-// parsePackage parses the non-test Go files in dir.
-func parsePackage(t *testing.T, dir string) (*token.FileSet, []*ast.File) {
-	t.Helper()
-	entries, err := os.ReadDir(dir)
-	if err != nil {
-		t.Fatalf("read %s: %v", dir, err)
-	}
-	fset := token.NewFileSet()
-	var files []*ast.File
-	for _, entry := range entries {
-		name := entry.Name()
-		if entry.IsDir() || !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") {
-			continue
-		}
-		file, err := parser.ParseFile(fset, filepath.Join(dir, name), nil, 0)
-		if err != nil {
-			t.Fatalf("parse %s: %v", name, err)
-		}
-		files = append(files, file)
-	}
-	if len(files) == 0 {
-		t.Fatalf("no Go files in %s", dir)
-	}
-	return fset, files
-}
-
-// importName is the name file uses for the package at path, or "" when the
-// file does not import it.
-func importName(file *ast.File, path string) string {
-	for _, spec := range file.Imports {
-		if value, err := strconv.Unquote(spec.Path.Value); err != nil || value != path {
-			continue
-		}
-		if spec.Name != nil {
-			return spec.Name.Name
-		}
-		return filepath.Base(path)
-	}
-	return ""
-}
-
-// directGitExecs lists the exec.Command and exec.CommandContext calls in files
-// whose program is the literal "git".
-func directGitExecs(fset *token.FileSet, files []*ast.File) []string {
-	var found []string
-	for _, file := range files {
-		execName := importName(file, "os/exec")
-		if execName == "" {
-			continue
-		}
-		ast.Inspect(file, func(node ast.Node) bool {
-			call, ok := node.(*ast.CallExpr)
-			if !ok {
-				return true
-			}
-			selector, ok := call.Fun.(*ast.SelectorExpr)
-			if !ok {
-				return true
-			}
-			if pkg, ok := selector.X.(*ast.Ident); !ok || pkg.Name != execName {
-				return true
-			}
-			program := 0
-			switch selector.Sel.Name {
-			case "Command":
-			case "CommandContext":
-				program = 1
-			default:
-				return true
-			}
-			if len(call.Args) <= program {
-				return true
-			}
-			if literal, ok := call.Args[program].(*ast.BasicLit); ok && literal.Kind == token.STRING {
-				if value, err := strconv.Unquote(literal.Value); err == nil && value == "git" {
-					found = append(found, fset.Position(call.Pos()).String())
-				}
-			}
-			return true
-		})
-	}
-	return found
-}
-
-func TestAutomaticGitCallsDoNotExecGitDirectly(t *testing.T) {
-	for _, dir := range automaticGitPackages {
-		fset, files := parsePackage(t, dir)
-		for _, position := range directGitExecs(fset, files) {
-			t.Errorf("%s runs git directly; go through gitsafe.Command so the repository's config cannot make it run a program", position)
-		}
-	}
-}
-
-// gitArguments returns the git arguments of call when it is gitsafe.Command or a
-// runner called with the program "git" (the TUI's runner(ctx, cwd, "git", ...)),
-// as far as they are string literals.
-func gitArguments(call *ast.CallExpr, gitsafeName string) ([]string, bool) {
-	start := -1
-	if selector, ok := call.Fun.(*ast.SelectorExpr); ok && gitsafeName != "" && selector.Sel.Name == "Command" {
-		if pkg, ok := selector.X.(*ast.Ident); ok && pkg.Name == gitsafeName {
-			start = 2
-		}
-	}
-	if start < 0 {
-		for index, argument := range call.Args {
-			if literal, ok := argument.(*ast.BasicLit); ok && literal.Kind == token.STRING && literal.Value == `"git"` {
-				start = index + 1
-				break
-			}
-		}
-	}
-	if start < 0 || start > len(call.Args) {
-		return nil, false
-	}
-	var arguments []string
-	for _, argument := range call.Args[start:] {
-		if literal, ok := argument.(*ast.BasicLit); ok && literal.Kind == token.STRING {
-			if value, err := strconv.Unquote(literal.Value); err == nil {
-				arguments = append(arguments, value)
-				continue
-			}
-		}
-		arguments = append(arguments, "")
-	}
-	return arguments, true
-}
-
-// A submodule has its own config, which Command does not check, so the
-// automatic status and diff calls stay out of submodule work trees.
-func TestAutomaticStatusAndDiffSkipSubmoduleWorkTrees(t *testing.T) {
-	checked := 0
-	for _, dir := range automaticGitPackages {
-		fset, files := parsePackage(t, dir)
-		for _, file := range files {
-			gitsafeName := importName(file, "github.com/Gitlawb/zero/internal/gitsafe")
-			ast.Inspect(file, func(node ast.Node) bool {
-				call, ok := node.(*ast.CallExpr)
-				if !ok {
-					return true
-				}
-				arguments, ok := gitArguments(call, gitsafeName)
-				if !ok || len(arguments) == 0 || (arguments[0] != "status" && arguments[0] != "diff") {
-					return true
-				}
-				checked++
-				for _, argument := range arguments {
-					if argument == "--ignore-submodules=dirty" {
-						return true
-					}
-				}
-				t.Errorf("%s runs git %s without --ignore-submodules=dirty", fset.Position(call.Pos()), arguments[0])
-				return true
-			})
-		}
-	}
-	// The scratch-file status, the files sweep's status and diff, and the pull
-	// request diffstat.
-	if checked != 4 {
-		t.Fatalf("checked %d automatic status or diff calls, want 4; update this test when they change", checked)
-	}
-}
-
-// The TUI runs every git command through one runner, whose program name is a
-// parameter, so the check above cannot see it. The runner has to hand git to
-// Command itself.
-func TestTUICommandRunnerSendsGitThroughCommand(t *testing.T) {
-	_, files := parsePackage(t, filepath.Join("..", "tui"))
-	var runner *ast.FuncDecl
-	var runnerFile *ast.File
-	for _, file := range files {
-		for _, declaration := range file.Decls {
-			if function, ok := declaration.(*ast.FuncDecl); ok && function.Recv == nil && function.Name.Name == "defaultPRCommandRunner" {
-				runner, runnerFile = function, file
-			}
-		}
-	}
-	if runner == nil {
-		t.Fatal("defaultPRCommandRunner not found in internal/tui")
-	}
-	gitsafeName := importName(runnerFile, "github.com/Gitlawb/zero/internal/gitsafe")
-	calls := false
-	if gitsafeName != "" {
-		ast.Inspect(runner.Body, func(node ast.Node) bool {
-			if call, ok := node.(*ast.CallExpr); ok {
-				if selector, ok := call.Fun.(*ast.SelectorExpr); ok && selector.Sel.Name == "Command" {
-					if pkg, ok := selector.X.(*ast.Ident); ok && pkg.Name == gitsafeName {
-						calls = true
-					}
-				}
-			}
-			return !calls
-		})
-	}
-	if !calls {
-		t.Fatal("defaultPRCommandRunner does not call gitsafe.Command, so the TUI's git calls skip it")
-	}
-}

internal/gitsafe/gitsafe.go

@@ -1,192 +0,0 @@
-// Package gitsafe builds the read-only git commands Zero runs on its own, as
-// opposed to a git command the user or the model asked for: the status and diff
-// calls behind the TUI's changed-files sweep, its pull request diffstat and the
-// scratch-file warning, which run without anyone asking for git.
-//
-// A REPOSITORY CAN MAKE THOSE COMMANDS RUN A PROGRAM. git status and git diff read
-// the repository's own .git/config, and several keys there name programs git then
-// runs: core.fsmonitor on every status, a clean filter or diff driver assigned
-// through .gitattributes whenever a file's content has to be read, and hooks such
-// as post-index-change when the index is rewritten. A repository unpacked from an
-// archive or handed over as a directory carries that config, and Zero would run it
-// on the first turn, although it refuses the same workspace's project hooks and
-// plugins until the user trusts it.
-//
-// The single-key cases are switched off on the command line. Filter and diff
-// driver commands are named per driver, so they cannot be switched off without
-// knowing the names; a repository whose own config defines any is skipped instead.
-// Commands defined in the user's own global config, such as git-lfs, are the
-// user's choice and are not affected.
-package gitsafe
-
-import (
-	"bytes"
-	"context"
-	"errors"
-	"os"
-	"os/exec"
-	"runtime"
-	"strings"
-)
-
-// ErrRepositoryDefinesCommands is returned for a repository whose own config
-// defines a filter or diff driver command. The caller goes without the git
-// information rather than run it.
-var ErrRepositoryDefinesCommands = errors.New("the repository's own git config defines a filter or diff command, so Zero does not run git there on its own")
-
-// commandKeys matches the config keys whose value is a program git can run while
-// computing status or a diff, and which are named per driver. Git prints section
-// and variable names in lower case, so the patterns do not need to fold case.
-const commandKeys = `filter\..+\.(clean|smudge|process)|diff\..+\.(command|textconv)|diff\.external`
-
-// worktreeConfigKey makes git also read per-worktree config (config.worktree),
-// which a --local read leaves out. Without it git ignores that file.
-const worktreeConfigKey = "extensions.worktreeconfig"
-
-const (
-	localConfigPattern    = `^(` + commandKeys + `|extensions\.worktreeconfig)$`
-	worktreeConfigPattern = `^(` + commandKeys + `)$`
-)
-
-// Command returns git with args, run in dir, with the programs a repository can
-// configure turned off. It fails with ErrRepositoryDefinesCommands when the
-// repository's own config defines a filter or diff driver command, and with git's
-// own error when dir is not inside a repository.
-//
-// Callers that inspect a work tree that may contain submodules should also pass
-// --ignore-submodules=dirty: a submodule has its own config, which this does not
-// read, and git only runs commands there when it looks inside the submodule.
-func Command(ctx context.Context, dir string, args ...string) (*exec.Cmd, error) {
-	env := environment()
-	defines, err := repositoryDefinesCommands(ctx, dir, env)
-	if err != nil {
-		return nil, err
-	}
-	if defines {
-		return nil, ErrRepositoryDefinesCommands
-	}
-	command := exec.CommandContext(ctx, "git", append(disabledPrograms(), args...)...)
-	command.Dir = dir
-	// No opportunistic index refresh: a read-only call never rewrites the index,
-	// so it never triggers the hooks that run when the index is written. Git
-	// before 2.15 does not know the variable and ignores it.
-	command.Env = append(env, "GIT_OPTIONAL_LOCKS=0")
-	return command, nil
-}
-
-// disabledPrograms turns off the single-key ways a repository runs a program
-// during status or diff. Git ignores a key it does not know, so an older git
-// without hooksPath (2.9) or fsmonitor (2.16) accepts these unchanged.
-func disabledPrograms() []string {
-	return []string{"-c", "core.fsmonitor=false", "-c", "core.hooksPath=" + os.DevNull}
-}
-
-// repositoryLocationVariables make git use a given repository, work tree or
-// index instead of discovering them from the working directory. Git exports
-// them to hooks, so a Zero started from a hook inherits the hook's repository.
-var repositoryLocationVariables = []string{
-	"GIT_DIR", "GIT_WORK_TREE", "GIT_INDEX_FILE", "GIT_COMMON_DIR",
-	"GIT_OBJECT_DIRECTORY", "GIT_ALTERNATE_OBJECT_DIRECTORIES", "GIT_NAMESPACE", "GIT_PREFIX",
-}
-
-// environment is Zero's environment without the repository-location variables,
-// so git finds the repository from dir. The config check and the command share
-// it: checking one repository's config and then running git in another would
-// let the second one's commands through.
-func environment() []string {
-	parent := os.Environ()
-	env := make([]string, 0, len(parent)+1)
-	for _, entry := range parent {
-		name, _, _ := strings.Cut(entry, "=")
-		if !isRepositoryLocationVariable(name) {
-			env = append(env, entry)
-		}
-	}
-	return env
-}
-
-func isRepositoryLocationVariable(name string) bool {
-	for _, variable := range repositoryLocationVariables {
-		// Windows environment names are case-insensitive.
-		if name == variable || (runtime.GOOS == "windows" && strings.EqualFold(name, variable)) {
-			return true
-		}
-	}
-	return false
-}
-
-// repositoryDefinesCommands reports whether the repository's own config, with
-// includes followed, defines a filter or diff driver command. The local config is
-// read once, together with the key that turns on per-worktree config, and that
-// file is read only when it is on, so most repositories cost one git process.
-// Reading config runs nothing.
-func repositoryDefinesCommands(ctx context.Context, dir string, env []string) (bool, error) {
-	entries, err := readConfig(ctx, dir, env, "--local", localConfigPattern)
-	if err != nil {
-		return false, err
-	}
-	worktreeConfig := false
-	for _, entry := range entries {
-		if entry.key != worktreeConfigKey {
-			return true, nil
-		}
-		worktreeConfig = !configFalse(entry.value)
-	}
-	if !worktreeConfig {
-		return false, nil
-	}
-	// The extension needs git 2.20, which also has --worktree, so a failure here
-	// is real and the caller goes without git.
-	entries, err = readConfig(ctx, dir, env, "--worktree", worktreeConfigPattern)
-	if err != nil {
-		return false, err
-	}
-	return len(entries) > 0, nil
-}
-
-// configFalse reports whether a config value is one of git's spellings of false.
-// Anything else, including a key with no value, counts as on, so an unexpected
-// spelling costs one more read rather than skipping the file.
-func configFalse(value string) bool {
-	switch strings.ToLower(strings.TrimSpace(value)) {
-	case "false", "no", "off", "0":
-		return true
-	}
-	return false
-}
-
-type configEntry struct {
-	key   string
-	value string
-}
-
-// readConfig returns the entries in one config scope whose key matches pattern.
-// It is a variable so tests can answer for git.
-var readConfig = func(ctx context.Context, dir string, env []string, scope string, pattern string) ([]configEntry, error) {
-	command := exec.CommandContext(ctx, "git", "config", "-z", scope, "--includes", "--get-regexp", pattern)
-	command.Dir = dir
-	command.Env = env
-	output, err := command.Output()
-	if err != nil {
-		var exit *exec.ExitError
-		if errors.As(err, &exit) && exit.ExitCode() == 1 {
-			return nil, nil // no key matched
-		}
-		return nil, err
-	}
-	return parseConfigEntries(output), nil
-}
-
-// parseConfigEntries splits git config -z output: each entry ends with a NUL,
-// and a newline separates the key from its value when there is one.
-func parseConfigEntries(output []byte) []configEntry {
-	var entries []configEntry
-	for _, record := range bytes.Split(output, []byte{0}) {
-		if len(record) == 0 {
-			continue
-		}
-		key, value, _ := bytes.Cut(record, []byte{'\n'})
-		entries = append(entries, configEntry{key: string(key), value: string(value)})
-	}
-	return entries
-}

internal/gitsafe/gitsafe_test.go

@@ -1,327 +0,0 @@
-package gitsafe
-
-import (
-	"context"
-	"errors"
-	"fmt"
-	"os"
-	"os/exec"
-	"path/filepath"
-	"reflect"
-	"regexp"
-	"runtime"
-	"strings"
-	"testing"
-)
-
-type configCall struct {
-	scope string
-	env   []string
-}
-
-// answerConfig stands in for git's config read and records each call.
-func answerConfig(t *testing.T, answer func(scope string) ([]configEntry, error)) *[]configCall {
-	t.Helper()
-	var calls []configCall
-	previous := readConfig
-	readConfig = func(_ context.Context, _ string, env []string, scope string, _ string) ([]configEntry, error) {
-		calls = append(calls, configCall{scope: scope, env: env})
-		return answer(scope)
-	}
-	t.Cleanup(func() { readConfig = previous })
-	return &calls
-}
-
-func noEntries(string) ([]configEntry, error) { return nil, nil }
-
-func scopes(calls *[]configCall) []string {
-	var read []string
-	for _, call := range *calls {
-		read = append(read, call.scope)
-	}
-	return read
-}
-
-// effectiveEnv returns the value a child process gets for name: os/exec keeps
-// the last entry for a name, compared without case on Windows.
-func effectiveEnv(env []string, name string) (string, bool) {
-	value, found := "", false
-	for _, entry := range env {
-		key, rest, ok := strings.Cut(entry, "=")
-		if ok && (key == name || (runtime.GOOS == "windows" && strings.EqualFold(key, name))) {
-			value, found = rest, true
-		}
-	}
-	return value, found
-}
-
-func TestCommandTurnsOffRepositoryPrograms(t *testing.T) {
-	answerConfig(t, noEntries)
-	t.Setenv("GIT_OPTIONAL_LOCKS", "1")
-	dir := t.TempDir()
-
-	command, err := Command(context.Background(), dir, "status", "--porcelain")
-	if err != nil {
-		t.Fatalf("Command: %v", err)
-	}
-	want := []string{"git", "-c", "core.fsmonitor=false", "-c", "core.hooksPath=" + os.DevNull, "status", "--porcelain"}
-	if !reflect.DeepEqual(command.Args, want) {
-		t.Fatalf("args = %q, want %q", command.Args, want)
-	}
-	if command.Dir != dir {
-		t.Fatalf("dir = %q, want %q", command.Dir, dir)
-	}
-	// An inherited GIT_OPTIONAL_LOCKS must not turn the index refresh back on.
-	if value, _ := effectiveEnv(command.Env, "GIT_OPTIONAL_LOCKS"); value != "0" {
-		t.Fatalf("effective GIT_OPTIONAL_LOCKS = %q, want 0", value)
-	}
-}
-
-func TestCommandDropsRepositoryLocationVariables(t *testing.T) {
-	// Turn per-worktree config on so both reads happen and both are checked.
-	calls := answerConfig(t, func(scope string) ([]configEntry, error) {
-		if scope == "--local" {
-			return []configEntry{{key: worktreeConfigKey, value: "true"}}, nil
-		}
-		return nil, nil
-	})
-	elsewhere := filepath.Join(t.TempDir(), "elsewhere")
-	for _, name := range repositoryLocationVariables {
-		t.Setenv(name, elsewhere)
-	}
-	t.Setenv("ZERO_GITSAFE_TEST_KEPT", "kept")
-
-	command, err := Command(context.Background(), t.TempDir(), "status")
-	if err != nil {
-		t.Fatalf("Command: %v", err)
-	}
-	for _, name := range repositoryLocationVariables {
-		if value, found := effectiveEnv(command.Env, name); found {
-			t.Fatalf("%s=%q reached git", name, value)
-		}
-	}
-	if value, _ := effectiveEnv(command.Env, "ZERO_GITSAFE_TEST_KEPT"); value != "kept" {
-		t.Fatalf("an unrelated variable was dropped: %q", value)
-	}
-	// The config check has to look at the repository the command runs in, so it
-	// gets the same environment, minus only the lock variable added afterwards.
-	if got := scopes(calls); !reflect.DeepEqual(got, []string{"--local", "--worktree"}) {
-		t.Fatalf("config reads = %q, want --local then --worktree", got)
-	}
-	for _, call := range *calls {
-		if !reflect.DeepEqual(call.env, command.Env[:len(command.Env)-1]) {
-			t.Fatalf("the %s config read ran with a different environment than the command", call.scope)
-		}
-	}
-}
-
-func TestLocationVariableNamesFoldCaseOnlyOnWindows(t *testing.T) {
-	if !isRepositoryLocationVariable("GIT_DIR") {
-		t.Fatal("GIT_DIR not recognised")
-	}
-	if got, want := isRepositoryLocationVariable("Git_Dir"), runtime.GOOS == "windows"; got != want {
-		t.Fatalf("Git_Dir recognised = %v, want %v on %s", got, want, runtime.GOOS)
-	}
-	for _, name := range []string{"GIT_DIRECTORY", "MY_GIT_DIR", "GIT_CONFIG_GLOBAL", ""} {
-		if isRepositoryLocationVariable(name) {
-			t.Fatalf("%q treated as a repository-location variable", name)
-		}
-	}
-}
-
-func TestCommandRefusesRepositoryThatDefinesCommands(t *testing.T) {
-	enabled := configEntry{key: worktreeConfigKey, value: "true"}
-	command := configEntry{key: "filter.x.clean", value: "x"}
-	cases := map[string]map[string][]configEntry{
-		"local":                     {"--local": {command}},
-		"local after the extension": {"--local": {enabled, command}},
-		"per-worktree":              {"--local": {enabled}, "--worktree": {{key: "diff.x.textconv", value: "x"}}},
-	}
-	for name, answers := range cases {
-		t.Run(name, func(t *testing.T) {
-			answerConfig(t, func(scope string) ([]configEntry, error) { return answers[scope], nil })
-			got, err := Command(context.Background(), t.TempDir(), "status")
-			if !errors.Is(err, ErrRepositoryDefinesCommands) {
-				t.Fatalf("err = %v, want ErrRepositoryDefinesCommands", err)
-			}
-			if got != nil {
-				t.Fatal("a command came back for a repository that defines commands")
-			}
-		})
-	}
-}
-
-func TestCommandReadsPerWorktreeConfigOnlyWhenItIsOn(t *testing.T) {
-	cases := map[string]struct {
-		local []configEntry
-		want  []string
-	}{
-		"no extension":            {nil, []string{"--local"}},
-		"extension off":           {[]configEntry{{key: worktreeConfigKey, value: "false"}}, []string{"--local"}},
-		"extension off as no":     {[]configEntry{{key: worktreeConfigKey, value: "No"}}, []string{"--local"}},
-		"extension on":            {[]configEntry{{key: worktreeConfigKey, value: "true"}}, []string{"--local", "--worktree"}},
-		"extension with no value": {[]configEntry{{key: worktreeConfigKey}}, []string{"--local", "--worktree"}},
-		"turned off again":        {[]configEntry{{key: worktreeConfigKey, value: "true"}, {key: worktreeConfigKey, value: "off"}}, []string{"--local"}},
-	}
-	for name, tc := range cases {
-		t.Run(name, func(t *testing.T) {
-			calls := answerConfig(t, func(scope string) ([]configEntry, error) {
-				if scope == "--local" {
-					return tc.local, nil
-				}
-				return nil, nil
-			})
-			if _, err := Command(context.Background(), t.TempDir(), "status"); err != nil {
-				t.Fatalf("Command: %v", err)
-			}
-			if got := scopes(calls); !reflect.DeepEqual(got, tc.want) {
-				t.Fatalf("config reads = %q, want %q", got, tc.want)
-			}
-		})
-	}
-}
-
-func TestCommandFailsWhenConfigCannotBeRead(t *testing.T) {
-	unreadable := errors.New("not a git repository")
-	for _, failing := range []string{"--local", "--worktree"} {
-		t.Run(failing, func(t *testing.T) {
-			answerConfig(t, func(scope string) ([]configEntry, error) {
-				if scope == failing {
-					return nil, unreadable
-				}
-				return []configEntry{{key: worktreeConfigKey, value: "true"}}, nil
-			})
-			if _, err := Command(context.Background(), t.TempDir(), "status"); !errors.Is(err, unreadable) {
-				t.Fatalf("err = %v, want the config read error", err)
-			}
-		})
-	}
-}
-
-// Go's regexp stands in for git's extended regex here; the patterns use nothing
-// the two treat differently.
-func TestConfigPatterns(t *testing.T) {
-	local := regexp.MustCompile(localConfigPattern)
-	worktree := regexp.MustCompile(worktreeConfigPattern)
-	for _, key := range []string{
-		"filter.lfs.clean", "filter.lfs.smudge", "filter.lfs.process", "filter.a.b.clean",
-		"diff.word.textconv", "diff.tool.command", "diff.external",
-	} {
-		if !local.MatchString(key) || !worktree.MatchString(key) {
-			t.Errorf("%s is not matched in both scopes", key)
-		}
-	}
-	if !local.MatchString(worktreeConfigKey) || worktree.MatchString(worktreeConfigKey) {
-		t.Errorf("%s belongs to the local read only", worktreeConfigKey)
-	}
-	for _, key := range []string{
-		"filter.lfs.required", "diff.word.cachetextconv", "diff.word.binary",
-		"core.fsmonitor", "diff.external.x", "diff.renames", "extensions.worktreeconfigx",
-	} {
-		if local.MatchString(key) || worktree.MatchString(key) {
-			t.Errorf("%s is matched", key)
-		}
-	}
-}
-
-func TestParseConfigEntries(t *testing.T) {
-	got := parseConfigEntries([]byte("filter.x.clean\nrun this\nand that\x00extensions.worktreeconfig\x00"))
-	want := []configEntry{
-		{key: "filter.x.clean", value: "run this\nand that"},
-		{key: "extensions.worktreeconfig"},
-	}
-	if !reflect.DeepEqual(got, want) {
-		t.Fatalf("entries = %+v, want %+v", got, want)
-	}
-}
-
-// Real git: a fresh repository with nothing configured, the same repository with
-// per-worktree config turned on, and a directory that is not a repository. This
-// pins the output format and exit codes readConfig relies on; the command keys
-// themselves are covered above.
-func TestCommandAgainstRealGit(t *testing.T) {
-	if _, err := exec.LookPath("git"); err != nil {
-		t.Skip("git not available")
-	}
-	home := t.TempDir()
-	t.Setenv("HOME", home)
-	t.Setenv("USERPROFILE", home)
-	t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config"))
-	t.Setenv("GIT_CONFIG_GLOBAL", os.DevNull)
-	t.Setenv("GIT_CONFIG_NOSYSTEM", "1")
-	repo := t.TempDir()
-	outside := t.TempDir()
-	// Both share a parent; stop git from finding an enclosing repository there.
-	t.Setenv("GIT_CEILING_DIRECTORIES", filepath.Dir(repo))
-	// Run as if from a hook in another repository. The setup below has to reach
-	// repo, as Command does, and never the repository these name.
-	decoy := t.TempDir()
-	t.Setenv("GIT_DIR", filepath.Join(decoy, ".git"))
-	t.Setenv("GIT_WORK_TREE", decoy)
-	decoyUntouched := func(step string) {
-		t.Helper()
-		if _, err := os.Stat(filepath.Join(decoy, ".git")); !errors.Is(err, os.ErrNotExist) {
-			t.Fatalf("%s reached the repository GIT_DIR names instead of repo: %v", step, err)
-		}
-	}
-	git := func(args ...string) {
-		t.Helper()
-		command := exec.Command("git", append([]string{"-C", repo}, args...)...)
-		command.Env = environment()
-		if output, err := command.CombinedOutput(); err != nil {
-			t.Fatalf("git %v: %v: %s", args, err, output)
-		}
-	}
-	git("init", "-q")
-	decoyUntouched("git init")
-	gitRead := readConfig
-	var read []string
-	readConfig = func(ctx context.Context, dir string, env []string, scope string, pattern string) ([]configEntry, error) {
-		read = append(read, scope)
-		return gitRead(ctx, dir, env, scope, pattern)
-	}
-	t.Cleanup(func() { readConfig = gitRead })
-
-	command, err := Command(context.Background(), repo, "status", "--porcelain")
-	if err != nil {
-		t.Fatalf("Command in a plain repository: %v", err)
-	}
-	if output, err := command.CombinedOutput(); err != nil {
-		t.Fatalf("git status: %v: %s", err, output)
-	}
-	if !reflect.DeepEqual(read, []string{"--local"}) {
-		t.Fatalf("config reads in a plain repository = %q, want --local only", read)
-	}
-
-	if !gitAtLeast(t, 2, 20) {
-		t.Log("skipping the per-worktree case: it needs git 2.20 for extensions.worktreeConfig")
-	} else {
-		git("config", "extensions.worktreeConfig", "true")
-		decoyUntouched("git config")
-		read = nil
-		if _, err := Command(context.Background(), repo, "status"); err != nil {
-			t.Fatalf("Command with per-worktree config on: %v", err)
-		}
-		if !reflect.DeepEqual(read, []string{"--local", "--worktree"}) {
-			t.Fatalf("config reads with per-worktree config on = %q, want --local then --worktree", read)
-		}
-	}
-
-	if _, err := Command(context.Background(), outside, "status"); err == nil {
-		t.Fatal("Command outside a repository succeeded")
-	}
-}
-
-// gitAtLeast reports whether the git on PATH is at least major.minor.
-func gitAtLeast(t *testing.T, major, minor int) bool {
-	t.Helper()
-	output, err := exec.Command("git", "version").Output()
-	if err != nil {
-		t.Fatalf("git version: %v", err)
-	}
-	var gotMajor, gotMinor int
-	if _, err := fmt.Sscanf(strings.TrimPrefix(strings.TrimSpace(string(output)), "git version "), "%d.%d", &gotMajor, &gotMinor); err != nil {
-		t.Fatalf("parse %q: %v", output, err)
-	}
-	return gotMajor > major || (gotMajor == major && gotMinor >= minor)
-}

internal/tui/files_git_sweep.go

@@ -54,9 +54,7 @@ func gitSweepCmd(parent context.Context, cwd string, baseline bool) tea.Cmd {
 		defer cancel()
 		// --untracked-files=all enumerates files inside new directories; plain
 		// porcelain collapses them to "dir/", which is useless as a FILES row.
-		// --ignore-submodules=dirty keeps git out of submodule work trees, whose
-		// own config gitsafe does not check.
-		status, err := defaultPRCommandRunner(ctx, cwd, "git", "status", "--porcelain", "--untracked-files=all", "--ignore-submodules=dirty")
+		status, err := defaultPRCommandRunner(ctx, cwd, "git", "status", "--porcelain", "--untracked-files=all")
 		if err != nil {
 			return gitSweepMsg{baseline: baseline, ok: false}
 		}
@@ -68,7 +66,7 @@ func gitSweepCmd(parent context.Context, cwd string, baseline bool) tea.Cmd {
 		// would never match a porcelain path). Best-effort: a failure (e.g. unborn
 		// HEAD in a fresh repo) keeps the file list with zero counts rather than
 		// dropping the sweep.
-		if numstat, err := defaultPRCommandRunner(ctx, cwd, "git", "diff", "HEAD", "--numstat", "-z", "--ignore-submodules=dirty"); err == nil {
+		if numstat, err := defaultPRCommandRunner(ctx, cwd, "git", "diff", "HEAD", "--numstat", "-z"); err == nil {
 			stats := parseGitNumstat(numstat)
 			for i := range files {
 				if counts, ok := stats[files[i].path]; ok {

internal/tui/pr_status.go

@@ -13,8 +13,6 @@ import (
 	"time"
 
 	"charm.land/lipgloss/v2"
-
-	"github.com/Gitlawb/zero/internal/gitsafe"
 )
 
 type PrStatus string
@@ -245,7 +243,7 @@ func getLocalDiffStats(ctx context.Context, cwd string, baseBranch string, run p
 	if err != nil {
 		mergeBase = originBase
 	}
-	output, err := run(ctx, cwd, "git", "diff", "--numstat", "--ignore-submodules=dirty", strings.TrimSpace(mergeBase))
+	output, err := run(ctx, cwd, "git", "diff", "--numstat", strings.TrimSpace(mergeBase))
 	if err != nil {
 		return 0, 0, err
 	}
@@ -292,21 +290,9 @@ func jsonScalarString(raw json.RawMessage) string {
 }
 
 func defaultPRCommandRunner(ctx context.Context, cwd string, name string, args ...string) (string, error) {
-	var command *exec.Cmd
-	if name == "git" {
-		// Every git call the TUI makes on its own comes through here (the files
-		// sweep and the pull request diffstat), so the repository being shown
-		// cannot make them run a program.
-		safe, err := gitsafe.Command(ctx, cwd, args...)
-		if err != nil {
-			return "", fmt.Errorf("%s: %w", name, err)
-		}
-		command = safe
-	} else {
-		command = exec.CommandContext(ctx, name, args...)
-		if strings.TrimSpace(cwd) != "" {
-			command.Dir = cwd
-		}
+	command := exec.CommandContext(ctx, name, args...)
+	if strings.TrimSpace(cwd) != "" {
+		command.Dir = cwd
 	}
 	var stdout bytes.Buffer
 	var stderr bytes.Buffer

internal/tui/pr_status_test.go

@@ -15,7 +15,7 @@ func TestPrServiceDetectsGitHubPRAndDiffStats(t *testing.T) {
 		"git merge-base origin/main HEAD": {
 			output: "abc123\n",
 		},
-		"git diff --numstat --ignore-submodules=dirty abc123": {
+		"git diff --numstat abc123": {
 			output: "42\t7\tinternal/tui/view.go\n",
 		},
 	}}
@@ -49,7 +49,7 @@ func TestPrServiceFallsBackToGitLabMR(t *testing.T) {
 		"git merge-base origin/develop HEAD": {
 			output: "def456\n",
 		},
-		"git diff --numstat --ignore-submodules=dirty def456": {
+		"git diff --numstat def456": {
 			output: "4\t5\ta.go\n5\t6\tb.go\n",
 		},
 	}}
@@ -71,7 +71,7 @@ func TestGetLocalDiffStatsFallsBackToOriginBase(t *testing.T) {
 		"git merge-base origin/main HEAD": {
 			err: errors.New("unknown revision"),
 		},
-		"git diff --numstat --ignore-submodules=dirty origin/main": {
+		"git diff --numstat origin/main": {
 			output: "3\t0\tREADME.md\n",
 		},
 	}}