From f5ea6367dbe1880e090ecc1147982a0781121340 Mon Sep 17 00:00:00 2001 From: Haryel Date: Sat, 23 May 2026 19:12:01 -0300 Subject: [PATCH 1/3] fix(gitexec): isolate git commands from hook env --- internal/gitexec/git_test.go | 33 ++++++++++++++++++++++++ internal/gitexec/runner.go | 40 ++++++++++++++++++++++++++++++ test/e2e/skeeper_lifecycle_test.go | 36 +++++++++++++++++++++++++++ 3 files changed, 109 insertions(+) diff --git a/internal/gitexec/git_test.go b/internal/gitexec/git_test.go index 1c1672b..00b2d5c 100644 --- a/internal/gitexec/git_test.go +++ b/internal/gitexec/git_test.go @@ -22,6 +22,39 @@ func TestExecRunnerCommandErrorCapturesExitCode(t *testing.T) { } } +func TestExecRunnerGitIgnoresInheritedLinkedWorktreeEnv(t *testing.T) { + ctx := context.Background() + main := newRepo(t) + writeFile(t, main, "README.md", "project\n") + git(t, main, "add", "README.md") + git(t, main, "commit", "-m", "bootstrap") + + linked := filepath.Join(t.TempDir(), "linked") + git(t, main, "worktree", "add", "-b", "feature", linked) + + sidecar := newRepo(t) + writeFile(t, sidecar, "project/SPEC.md", "# Spec\n") + git(t, sidecar, "add", "project/SPEC.md") + git(t, sidecar, "commit", "-m", "sidecar") + remoteRef := "refs/remotes/origin/project/__branches__/feature" + want := gitOutput(t, sidecar, "rev-parse", "HEAD") + git(t, sidecar, "update-ref", remoteRef, want) + + gitDir := gitOutput(t, linked, "rev-parse", "--absolute-git-dir") + t.Setenv("GIT_DIR", gitDir) + t.Setenv("GIT_WORK_TREE", linked) + t.Setenv("GIT_INDEX_FILE", filepath.Join(gitDir, "index")) + t.Setenv("GIT_PREFIX", "") + + got, err := (&ExecRunner{}).Run(ctx, sidecar, "git", "rev-parse", remoteRef) + if err != nil { + t.Fatalf("rev-parse sidecar ref with inherited hook env: %v", err) + } + if TrimmedStdout(got) != want { + t.Fatalf("sidecar ref mismatch: got %q want %q", TrimmedStdout(got), want) + } +} + func TestGitLocalOperationsWithGoGit(t *testing.T) { t.Parallel() diff --git a/internal/gitexec/runner.go b/internal/gitexec/runner.go index 4605cef..9c76f5e 100644 --- a/internal/gitexec/runner.go +++ b/internal/gitexec/runner.go @@ -6,7 +6,9 @@ import ( "context" "errors" "fmt" + "os" "os/exec" + "path/filepath" "strings" ) @@ -27,6 +29,22 @@ type ExecRunner struct{} var _ Runner = (*ExecRunner)(nil) +var gitRepositoryEnv = map[string]struct{}{ + "GIT_ALTERNATE_OBJECT_DIRECTORIES": {}, + "GIT_COMMON_DIR": {}, + "GIT_DIR": {}, + "GIT_GRAFT_FILE": {}, + "GIT_IMPLICIT_WORK_TREE": {}, + "GIT_INDEX_FILE": {}, + "GIT_NAMESPACE": {}, + "GIT_NO_REPLACE_OBJECTS": {}, + "GIT_OBJECT_DIRECTORY": {}, + "GIT_PREFIX": {}, + "GIT_REPLACE_REF_BASE": {}, + "GIT_SHALLOW_FILE": {}, + "GIT_WORK_TREE": {}, +} + // Run executes name with args in dir and returns stdout and stderr. func (r *ExecRunner) Run(ctx context.Context, dir, name string, args ...string) (Result, error) { return r.RunWithInput(ctx, dir, "", name, args...) @@ -42,6 +60,9 @@ func (r *ExecRunner) RunWithInput( ) (Result, error) { cmd := exec.CommandContext(ctx, name, args...) cmd.Dir = dir + if isGitCommand(name) { + cmd.Env = withoutGitRepositoryEnv(os.Environ()) + } if stdin != "" { cmd.Stdin = strings.NewReader(stdin) } @@ -69,6 +90,25 @@ func (r *ExecRunner) RunWithInput( return result, nil } +func isGitCommand(name string) bool { + base := filepath.Base(name) + return base == "git" || strings.EqualFold(base, "git.exe") +} + +func withoutGitRepositoryEnv(env []string) []string { + cleaned := make([]string, 0, len(env)) + for _, entry := range env { + key, _, ok := strings.Cut(entry, "=") + if ok { + if _, blocked := gitRepositoryEnv[key]; blocked { + continue + } + } + cleaned = append(cleaned, entry) + } + return cleaned +} + // CommandError wraps an external command failure with useful context. type CommandError struct { Name string diff --git a/test/e2e/skeeper_lifecycle_test.go b/test/e2e/skeeper_lifecycle_test.go index 0417252..09cb022 100644 --- a/test/e2e/skeeper_lifecycle_test.go +++ b/test/e2e/skeeper_lifecycle_test.go @@ -78,6 +78,42 @@ func TestSkeeperLifecycleAcrossRealGitClones(t *testing.T) { env.assertContainsFile(filepath.Join(fresh, ".git", "hooks", "pre-merge-commit"), "skeeper internal pre-commit") } +func TestSkeeperPreCommitHookWorksInLinkedWorktree(t *testing.T) { + env := newE2EEnv(t) + project := env.newMainRepo("project") + env.run(project, "skeeper", + "init", + "--sidecar-name", "project-specs", + "--namespace", "project", + "--patterns", "**/SPEC.md", + ) + env.writeFile(project, "README.md", "# project\n") + env.git(project, "add", "README.md", ".skeeper.yml", ".gitignore", ".gitattributes") + env.git(project, "commit", "-m", "bootstrap skeeper") + + linked := filepath.Join(env.root, "linked-project") + env.git(project, "worktree", "add", "-b", "feature/docs", linked) + env.writeFile(linked, "src/auth/service.go", "package auth\n") + env.writeFile(linked, "src/auth/SPEC.md", "# Initial auth spec\n") + env.git(linked, "add", "src/auth/service.go") + env.git(linked, "commit", "-m", "auth: add linked worktree spec") + env.assertSidecarFile( + "project/__branches__/feature/docs", + "project/src/auth/SPEC.md", + "# Initial auth spec\n", + ) + + env.writeFile(linked, "src/auth/service.go", "package auth\n\nconst version = 2\n") + env.writeFile(linked, "src/auth/SPEC.md", "# Updated auth spec\n") + env.git(linked, "add", "src/auth/service.go") + env.git(linked, "commit", "-m", "auth: update linked worktree spec") + env.assertSidecarFile( + "project/__branches__/feature/docs", + "project/src/auth/SPEC.md", + "# Updated auth spec\n", + ) +} + func TestSkeeperSharedSidecarNamespaceIsolationAcrossRepos(t *testing.T) { env := newE2EEnv(t) sharedRemote := env.newBareRepo("shared-specs.git") From dd8c4455745744b75172af646a8a2b695f7918af Mon Sep 17 00:00:00 2001 From: Haryel Date: Sat, 23 May 2026 19:23:58 -0300 Subject: [PATCH 2/3] test(gitexec): wrap linked worktree env test in subtest --- internal/gitexec/git_test.go | 62 +++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 30 deletions(-) diff --git a/internal/gitexec/git_test.go b/internal/gitexec/git_test.go index 00b2d5c..4553174 100644 --- a/internal/gitexec/git_test.go +++ b/internal/gitexec/git_test.go @@ -23,36 +23,38 @@ func TestExecRunnerCommandErrorCapturesExitCode(t *testing.T) { } func TestExecRunnerGitIgnoresInheritedLinkedWorktreeEnv(t *testing.T) { - ctx := context.Background() - main := newRepo(t) - writeFile(t, main, "README.md", "project\n") - git(t, main, "add", "README.md") - git(t, main, "commit", "-m", "bootstrap") - - linked := filepath.Join(t.TempDir(), "linked") - git(t, main, "worktree", "add", "-b", "feature", linked) - - sidecar := newRepo(t) - writeFile(t, sidecar, "project/SPEC.md", "# Spec\n") - git(t, sidecar, "add", "project/SPEC.md") - git(t, sidecar, "commit", "-m", "sidecar") - remoteRef := "refs/remotes/origin/project/__branches__/feature" - want := gitOutput(t, sidecar, "rev-parse", "HEAD") - git(t, sidecar, "update-ref", remoteRef, want) - - gitDir := gitOutput(t, linked, "rev-parse", "--absolute-git-dir") - t.Setenv("GIT_DIR", gitDir) - t.Setenv("GIT_WORK_TREE", linked) - t.Setenv("GIT_INDEX_FILE", filepath.Join(gitDir, "index")) - t.Setenv("GIT_PREFIX", "") - - got, err := (&ExecRunner{}).Run(ctx, sidecar, "git", "rev-parse", remoteRef) - if err != nil { - t.Fatalf("rev-parse sidecar ref with inherited hook env: %v", err) - } - if TrimmedStdout(got) != want { - t.Fatalf("sidecar ref mismatch: got %q want %q", TrimmedStdout(got), want) - } + t.Run("Should ignore inherited linked worktree env", func(t *testing.T) { + ctx := context.Background() + main := newRepo(t) + writeFile(t, main, "README.md", "project\n") + git(t, main, "add", "README.md") + git(t, main, "commit", "-m", "bootstrap") + + linked := filepath.Join(t.TempDir(), "linked") + git(t, main, "worktree", "add", "-b", "feature", linked) + + sidecar := newRepo(t) + writeFile(t, sidecar, "project/SPEC.md", "# Spec\n") + git(t, sidecar, "add", "project/SPEC.md") + git(t, sidecar, "commit", "-m", "sidecar") + remoteRef := "refs/remotes/origin/project/__branches__/feature" + want := gitOutput(t, sidecar, "rev-parse", "HEAD") + git(t, sidecar, "update-ref", remoteRef, want) + + gitDir := gitOutput(t, linked, "rev-parse", "--absolute-git-dir") + t.Setenv("GIT_DIR", gitDir) + t.Setenv("GIT_WORK_TREE", linked) + t.Setenv("GIT_INDEX_FILE", filepath.Join(gitDir, "index")) + t.Setenv("GIT_PREFIX", "") + + got, err := (&ExecRunner{}).Run(ctx, sidecar, "git", "rev-parse", remoteRef) + if err != nil { + t.Fatalf("rev-parse sidecar ref with inherited hook env: %v", err) + } + if TrimmedStdout(got) != want { + t.Fatalf("sidecar ref mismatch: got %q want %q", TrimmedStdout(got), want) + } + }) } func TestGitLocalOperationsWithGoGit(t *testing.T) { From 7a7a12573e2028103b333b33cd06675c5af2bb42 Mon Sep 17 00:00:00 2001 From: Haryel Date: Sat, 23 May 2026 19:25:56 -0300 Subject: [PATCH 3/3] test(e2e): wrap linked worktree hook scenario in subtest --- test/e2e/skeeper_lifecycle_test.go | 68 +++++++++++++++--------------- 1 file changed, 35 insertions(+), 33 deletions(-) diff --git a/test/e2e/skeeper_lifecycle_test.go b/test/e2e/skeeper_lifecycle_test.go index 09cb022..7610c3d 100644 --- a/test/e2e/skeeper_lifecycle_test.go +++ b/test/e2e/skeeper_lifecycle_test.go @@ -79,39 +79,41 @@ func TestSkeeperLifecycleAcrossRealGitClones(t *testing.T) { } func TestSkeeperPreCommitHookWorksInLinkedWorktree(t *testing.T) { - env := newE2EEnv(t) - project := env.newMainRepo("project") - env.run(project, "skeeper", - "init", - "--sidecar-name", "project-specs", - "--namespace", "project", - "--patterns", "**/SPEC.md", - ) - env.writeFile(project, "README.md", "# project\n") - env.git(project, "add", "README.md", ".skeeper.yml", ".gitignore", ".gitattributes") - env.git(project, "commit", "-m", "bootstrap skeeper") - - linked := filepath.Join(env.root, "linked-project") - env.git(project, "worktree", "add", "-b", "feature/docs", linked) - env.writeFile(linked, "src/auth/service.go", "package auth\n") - env.writeFile(linked, "src/auth/SPEC.md", "# Initial auth spec\n") - env.git(linked, "add", "src/auth/service.go") - env.git(linked, "commit", "-m", "auth: add linked worktree spec") - env.assertSidecarFile( - "project/__branches__/feature/docs", - "project/src/auth/SPEC.md", - "# Initial auth spec\n", - ) - - env.writeFile(linked, "src/auth/service.go", "package auth\n\nconst version = 2\n") - env.writeFile(linked, "src/auth/SPEC.md", "# Updated auth spec\n") - env.git(linked, "add", "src/auth/service.go") - env.git(linked, "commit", "-m", "auth: update linked worktree spec") - env.assertSidecarFile( - "project/__branches__/feature/docs", - "project/src/auth/SPEC.md", - "# Updated auth spec\n", - ) + t.Run("Should sync spec changes from linked worktree pre-commit hook", func(t *testing.T) { + env := newE2EEnv(t) + project := env.newMainRepo("project") + env.run(project, "skeeper", + "init", + "--sidecar-name", "project-specs", + "--namespace", "project", + "--patterns", "**/SPEC.md", + ) + env.writeFile(project, "README.md", "# project\n") + env.git(project, "add", "README.md", ".skeeper.yml", ".gitignore", ".gitattributes") + env.git(project, "commit", "-m", "bootstrap skeeper") + + linked := filepath.Join(env.root, "linked-project") + env.git(project, "worktree", "add", "-b", "feature/docs", linked) + env.writeFile(linked, "src/auth/service.go", "package auth\n") + env.writeFile(linked, "src/auth/SPEC.md", "# Initial auth spec\n") + env.git(linked, "add", "src/auth/service.go") + env.git(linked, "commit", "-m", "auth: add linked worktree spec") + env.assertSidecarFile( + "project/__branches__/feature/docs", + "project/src/auth/SPEC.md", + "# Initial auth spec\n", + ) + + env.writeFile(linked, "src/auth/service.go", "package auth\n\nconst version = 2\n") + env.writeFile(linked, "src/auth/SPEC.md", "# Updated auth spec\n") + env.git(linked, "add", "src/auth/service.go") + env.git(linked, "commit", "-m", "auth: update linked worktree spec") + env.assertSidecarFile( + "project/__branches__/feature/docs", + "project/src/auth/SPEC.md", + "# Updated auth spec\n", + ) + }) } func TestSkeeperSharedSidecarNamespaceIsolationAcrossRepos(t *testing.T) {