From 36bda3340990f06cb4addb688249b589e3583c49 Mon Sep 17 00:00:00 2001 From: kunchenguid Date: Thu, 14 May 2026 22:45:35 -0700 Subject: [PATCH 01/18] fix: kunchenguid/treehouse#21 --- README.md | 20 +++++- cmd/destroy.go | 4 +- cmd/get.go | 2 +- internal/config/config.go | 6 ++ internal/config/hooks_test.go | 51 ++++++++++++++ internal/hooks/hooks.go | 50 ++++++++++++++ internal/hooks/hooks_test.go | 95 +++++++++++++++++++++++++ internal/pool/pool.go | 22 ++++-- internal/pool/pool_test.go | 126 ++++++++++++++++++++++++++++++++++ treehouse.toml.example | 6 ++ 10 files changed, 373 insertions(+), 9 deletions(-) create mode 100644 internal/config/hooks_test.go create mode 100644 internal/hooks/hooks.go create mode 100644 internal/hooks/hooks_test.go create mode 100644 internal/pool/pool_test.go diff --git a/README.md b/README.md index 85f7dea..b449f08 100644 --- a/README.md +++ b/README.md @@ -165,7 +165,25 @@ Create a config file with `treehouse init`, or add one manually: max_trees = 16 ``` -The repo-level config takes precedence. If no config is found, the default pool size is 16. +The repo-level config takes precedence. +If no config is found, the default pool size is 16. + +### Hooks + +You can run commands automatically at worktree lifecycle points by adding a `[hooks]` section: + +```toml +[hooks] +post_create = ["./scripts/setup-venv.sh"] +pre_destroy = ["./scripts/teardown.sh"] +``` + +- `post_create` runs after a worktree is provisioned or reset and right before `treehouse get` hands it to you. +- `pre_destroy` runs before a worktree is removed by `treehouse destroy` (and `treehouse destroy --all`). + +Commands in each list run sequentially in the worktree directory, via the OS shell (`/bin/sh -c` on Linux/macOS, `%COMSPEC% /c` on Windows). +If a command exits non-zero, treehouse logs the command, exit code, and stderr, then continues with the remaining commands. +A failing hook does not fail the overall `get` or `destroy` operation. ## Development diff --git a/cmd/destroy.go b/cmd/destroy.go index 6fb28f5..3cf8c83 100644 --- a/cmd/destroy.go +++ b/cmd/destroy.go @@ -45,7 +45,7 @@ var destroyCmd = &cobra.Command{ return nil } } - if err := pool.DestroyAll(repoRoot, poolDir, destroyForce); err != nil { + if err := pool.DestroyAll(repoRoot, poolDir, destroyForce, cfg.Hooks.PreDestroy); err != nil { return err } fmt.Fprintln(os.Stderr, "🌳 All worktrees destroyed.") @@ -69,7 +69,7 @@ var destroyCmd = &cobra.Command{ } } - if err := pool.Destroy(repoRoot, poolDir, wtPath, destroyForce); err != nil { + if err := pool.Destroy(repoRoot, poolDir, wtPath, destroyForce, cfg.Hooks.PreDestroy); err != nil { return err } fmt.Fprintln(os.Stderr, "🌳 Worktree destroyed.") diff --git a/cmd/get.go b/cmd/get.go index 02adad8..98718d6 100644 --- a/cmd/get.go +++ b/cmd/get.go @@ -47,7 +47,7 @@ func getRunE(cmd *cobra.Command, args []string) error { fmt.Fprintf(os.Stderr, "warning: failed to update .gitignore: %v\n", err) } - wtPath, err := pool.Acquire(repoRoot, poolDir, cfg.MaxTrees) + wtPath, err := pool.Acquire(repoRoot, poolDir, cfg.MaxTrees, cfg.Hooks.PostCreate) if err != nil { return err } diff --git a/internal/config/config.go b/internal/config/config.go index e6b2dca..7584db5 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -11,6 +11,12 @@ import ( type Config struct { MaxTrees int `toml:"max_trees"` Root string `toml:"root"` + Hooks Hooks `toml:"hooks,omitempty"` +} + +type Hooks struct { + PostCreate []string `toml:"post_create,omitempty"` + PreDestroy []string `toml:"pre_destroy,omitempty"` } func DefaultConfig() Config { diff --git a/internal/config/hooks_test.go b/internal/config/hooks_test.go new file mode 100644 index 0000000..2b8063d --- /dev/null +++ b/internal/config/hooks_test.go @@ -0,0 +1,51 @@ +package config + +import ( + "os" + "path/filepath" + "reflect" + "testing" +) + +func TestLoad_Hooks(t *testing.T) { + repoDir := t.TempDir() + + cfgTOML := `max_trees = 4 + +[hooks] +post_create = ["echo a", "echo b"] +pre_destroy = ["echo c"] +` + if err := os.WriteFile(filepath.Join(repoDir, "treehouse.toml"), []byte(cfgTOML), 0o644); err != nil { + t.Fatal(err) + } + + cfg, err := Load(repoDir) + if err != nil { + t.Fatalf("Load failed: %v", err) + } + + wantPost := []string{"echo a", "echo b"} + wantPre := []string{"echo c"} + if !reflect.DeepEqual(cfg.Hooks.PostCreate, wantPost) { + t.Errorf("PostCreate: got %v, want %v", cfg.Hooks.PostCreate, wantPost) + } + if !reflect.DeepEqual(cfg.Hooks.PreDestroy, wantPre) { + t.Errorf("PreDestroy: got %v, want %v", cfg.Hooks.PreDestroy, wantPre) + } +} + +func TestLoad_HooksDefaultEmpty(t *testing.T) { + repoDir := t.TempDir() + + cfg, err := Load(repoDir) + if err != nil { + t.Fatalf("Load failed: %v", err) + } + if len(cfg.Hooks.PostCreate) != 0 { + t.Errorf("expected empty PostCreate, got %v", cfg.Hooks.PostCreate) + } + if len(cfg.Hooks.PreDestroy) != 0 { + t.Errorf("expected empty PreDestroy, got %v", cfg.Hooks.PreDestroy) + } +} diff --git a/internal/hooks/hooks.go b/internal/hooks/hooks.go new file mode 100644 index 0000000..911ef19 --- /dev/null +++ b/internal/hooks/hooks.go @@ -0,0 +1,50 @@ +// Package hooks runs user-configured shell commands at worktree lifecycle +// points. Commands run sequentially in a given working directory. A failing +// command is logged but does not stop later commands or fail the caller. +package hooks + +import ( + "fmt" + "io" + "os" + "os/exec" + "runtime" +) + +// Run executes each command in commands sequentially in workDir. Each command +// is passed to the OS shell (/bin/sh -c on Unix, %COMSPEC% /c on Windows). +// Stdout and stderr from the commands are streamed to the given writers. +// Failures are logged to stderr and do not stop subsequent commands. +func Run(commands []string, workDir string, stdout, stderr io.Writer) { + for _, command := range commands { + runOne(command, workDir, stdout, stderr) + } +} + +func runOne(command, workDir string, stdout, stderr io.Writer) { + shell, flag := shellAndFlag() + + cmd := exec.Command(shell, flag, command) + cmd.Dir = workDir + cmd.Stdout = stdout + cmd.Stderr = stderr + + if err := cmd.Run(); err != nil { + exitCode := -1 + if exitErr, ok := err.(*exec.ExitError); ok { + exitCode = exitErr.ExitCode() + } + fmt.Fprintf(stderr, "🌳 hook command failed: %q (exit %d): %v\n", command, exitCode, err) + } +} + +func shellAndFlag() (string, string) { + if runtime.GOOS == "windows" { + shell := os.Getenv("COMSPEC") + if shell == "" { + shell = "cmd.exe" + } + return shell, "/c" + } + return "/bin/sh", "-c" +} diff --git a/internal/hooks/hooks_test.go b/internal/hooks/hooks_test.go new file mode 100644 index 0000000..e6cddc5 --- /dev/null +++ b/internal/hooks/hooks_test.go @@ -0,0 +1,95 @@ +package hooks + +import ( + "bytes" + "os" + "path/filepath" + "runtime" + "strings" + "testing" +) + +// quotePath returns a path suitably quoted for use as a shell argument. On +// Windows we use double quotes for cmd.exe; on Unix we use single quotes for +// /bin/sh. +func quotePath(p string) string { + if runtime.GOOS == "windows" { + return `"` + p + `"` + } + return `'` + p + `'` +} + +func TestRun_Success(t *testing.T) { + dir := t.TempDir() + sentinel := filepath.Join(dir, "ran.txt") + + var script string + if runtime.GOOS == "windows" { + script = "echo hi > " + quotePath(sentinel) + } else { + script = "echo hi > " + quotePath(sentinel) + } + + var stdout, stderr bytes.Buffer + Run([]string{script}, dir, &stdout, &stderr) + + if _, err := os.Stat(sentinel); err != nil { + t.Fatalf("expected sentinel %s to exist: %v\nstderr: %s", sentinel, err, stderr.String()) + } +} + +func TestRun_FailingCommandDoesNotStopRemaining(t *testing.T) { + dir := t.TempDir() + sentinel := filepath.Join(dir, "after.txt") + + // First command: a non-existent program that should fail. + // Second command: writes the sentinel - must run despite the first failure. + var fail, ok string + if runtime.GOOS == "windows" { + fail = "this-command-definitely-does-not-exist-xyzzy" + ok = "echo hi > " + quotePath(sentinel) + } else { + fail = "this-command-definitely-does-not-exist-xyzzy" + ok = "echo hi > " + quotePath(sentinel) + } + + var stdout, stderr bytes.Buffer + Run([]string{fail, ok}, dir, &stdout, &stderr) + + if _, err := os.Stat(sentinel); err != nil { + t.Fatalf("expected second command to still run despite first failing: %v\nstderr: %s", err, stderr.String()) + } + + // The failure should be logged to stderr. + if !strings.Contains(stderr.String(), "hook command failed") { + t.Errorf("expected stderr to log hook failure, got: %s", stderr.String()) + } +} + +func TestRun_EmptyListIsNoop(t *testing.T) { + dir := t.TempDir() + var stdout, stderr bytes.Buffer + Run(nil, dir, &stdout, &stderr) + Run([]string{}, dir, &stdout, &stderr) + if stderr.Len() != 0 || stdout.Len() != 0 { + t.Errorf("expected no output for empty hooks, got stdout=%q stderr=%q", stdout.String(), stderr.String()) + } +} + +func TestRun_RunsInGivenDir(t *testing.T) { + dir := t.TempDir() + // Write a relative-path sentinel so we can confirm cwd is dir. + var script string + if runtime.GOOS == "windows" { + script = "echo hi > cwd-sentinel.txt" + } else { + script = "echo hi > cwd-sentinel.txt" + } + + var stdout, stderr bytes.Buffer + Run([]string{script}, dir, &stdout, &stderr) + + if _, err := os.Stat(filepath.Join(dir, "cwd-sentinel.txt")); err != nil { + t.Fatalf("expected hook to run in %s: %v\nstderr: %s", dir, err, stderr.String()) + } +} diff --git a/internal/pool/pool.go b/internal/pool/pool.go index 32df474..a119b4d 100644 --- a/internal/pool/pool.go +++ b/internal/pool/pool.go @@ -8,6 +8,7 @@ import ( "time" "github.com/kunchenguid/treehouse/internal/git" + "github.com/kunchenguid/treehouse/internal/hooks" "github.com/kunchenguid/treehouse/internal/process" ) @@ -25,7 +26,7 @@ type WorktreeStatus struct { Processes []process.ProcessInfo } -func Acquire(repoRoot, poolDir string, poolSize int) (string, error) { +func Acquire(repoRoot, poolDir string, poolSize int, postCreate []string) (string, error) { branch, err := git.GetDefaultBranch(repoRoot) if err != nil { return "", err @@ -63,7 +64,11 @@ func Acquire(repoRoot, poolDir string, poolSize int) (string, error) { continue } acquired = wt.Path - return WriteState(poolDir, state) + if err := WriteState(poolDir, state); err != nil { + return err + } + hooks.Run(postCreate, acquired, os.Stdout, os.Stderr) + return nil } // No available worktree — create new if pool allows @@ -90,7 +95,11 @@ func Acquire(repoRoot, poolDir string, poolSize int) (string, error) { }) acquired = wtPath - return WriteState(poolDir, state) + if err := WriteState(poolDir, state); err != nil { + return err + } + hooks.Run(postCreate, acquired, os.Stdout, os.Stderr) + return nil }) return acquired, err @@ -151,7 +160,7 @@ func List(poolDir string) ([]WorktreeStatus, error) { return result, err } -func Destroy(repoRoot, poolDir, worktreePath string, force bool) error { +func Destroy(repoRoot, poolDir, worktreePath string, force bool, preDestroy []string) error { return WithStateLock(poolDir, func() error { state, err := ReadState(poolDir) if err != nil { @@ -176,6 +185,8 @@ func Destroy(repoRoot, poolDir, worktreePath string, force bool) error { } } + hooks.Run(preDestroy, worktreePath, os.Stdout, os.Stderr) + _ = git.RemoveWorktree(repoRoot, worktreePath) // also clean up the parent numbered directory os.RemoveAll(filepath.Dir(worktreePath)) @@ -185,7 +196,7 @@ func Destroy(repoRoot, poolDir, worktreePath string, force bool) error { }) } -func DestroyAll(repoRoot, poolDir string, force bool) error { +func DestroyAll(repoRoot, poolDir string, force bool, preDestroy []string) error { return WithStateLock(poolDir, func() error { state, err := ReadState(poolDir) if err != nil { @@ -202,6 +213,7 @@ func DestroyAll(repoRoot, poolDir string, force bool) error { } for _, wt := range state.Worktrees { + hooks.Run(preDestroy, wt.Path, os.Stdout, os.Stderr) _ = git.RemoveWorktree(repoRoot, wt.Path) os.RemoveAll(filepath.Dir(wt.Path)) } diff --git a/internal/pool/pool_test.go b/internal/pool/pool_test.go new file mode 100644 index 0000000..da7b217 --- /dev/null +++ b/internal/pool/pool_test.go @@ -0,0 +1,126 @@ +package pool + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +func setupRepo(t *testing.T) (repoDir, poolDir string) { + t.Helper() + base := t.TempDir() + base, err := filepath.EvalSymlinks(base) + if err != nil { + t.Fatal(err) + } + + bareDir := filepath.Join(base, "remote.git") + repoDir = filepath.Join(base, "myrepo") + poolDir = filepath.Join(base, "pool") + + runGit(t, "", "init", "--bare", "--initial-branch=main", bareDir) + runGit(t, "", "init", "--initial-branch=main", repoDir) + runGit(t, repoDir, "config", "user.email", "test@test.com") + runGit(t, repoDir, "config", "user.name", "Test") + runGit(t, repoDir, "remote", "add", "origin", bareDir) + + if err := os.WriteFile(filepath.Join(repoDir, "README.md"), []byte("hi\n"), 0o644); err != nil { + t.Fatal(err) + } + runGit(t, repoDir, "add", ".") + runGit(t, repoDir, "commit", "-m", "initial") + runGit(t, repoDir, "push", "-u", "origin", "main") + return repoDir, poolDir +} + +func runGit(t *testing.T, dir string, args ...string) { + t.Helper() + cmd := exec.Command("git", args...) + if dir != "" { + cmd.Dir = dir + } + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %s failed: %v\n%s", strings.Join(args, " "), err, out) + } +} + +func TestAcquire_RunsPostCreateHookInWorktree(t *testing.T) { + repoDir, poolDir := setupRepo(t) + + // `echo X > sentinel.txt` works in both /bin/sh and cmd.exe. + hook := "echo created > hook-sentinel.txt" + + wtPath, err := Acquire(repoDir, poolDir, 4, []string{hook}) + if err != nil { + t.Fatalf("Acquire failed: %v", err) + } + if wtPath == "" { + t.Fatal("Acquire returned empty path") + } + + sentinel := filepath.Join(wtPath, "hook-sentinel.txt") + if _, err := os.Stat(sentinel); err != nil { + t.Fatalf("expected post_create hook to create %s: %v", sentinel, err) + } +} + +func TestAcquire_HookFailureDoesNotFailAcquire(t *testing.T) { + repoDir, poolDir := setupRepo(t) + + hooks := []string{ + "this-command-does-not-exist-xyzzy", + "echo ok > second-ran.txt", + } + + wtPath, err := Acquire(repoDir, poolDir, 4, hooks) + if err != nil { + t.Fatalf("Acquire should not fail when a hook fails: %v", err) + } + if wtPath == "" { + t.Fatal("Acquire returned empty path") + } + + // The second hook must still have run despite the first failing. + if _, err := os.Stat(filepath.Join(wtPath, "second-ran.txt")); err != nil { + t.Fatalf("expected second hook to run despite first failing: %v", err) + } +} + +func TestDestroy_RunsPreDestroyHook(t *testing.T) { + repoDir, poolDir := setupRepo(t) + + wtPath, err := Acquire(repoDir, poolDir, 4, nil) + if err != nil { + t.Fatalf("Acquire failed: %v", err) + } + + // Capture the location to verify after destroy. We write a sentinel into + // the *parent* of the worktree, since the worktree itself is removed. + sentinelDir := filepath.Dir(filepath.Dir(wtPath)) + sentinel := filepath.Join(sentinelDir, "predestroy-ran.txt") + + // Hook writes to an absolute path so it survives worktree removal. + hook := "echo bye > " + quoteForShell(sentinel) + + if err := Destroy(repoDir, poolDir, wtPath, true, []string{hook}); err != nil { + t.Fatalf("Destroy failed: %v", err) + } + + if _, err := os.Stat(sentinel); err != nil { + t.Fatalf("expected pre_destroy hook to create %s: %v", sentinel, err) + } + if _, err := os.Stat(wtPath); !os.IsNotExist(err) { + t.Fatalf("expected worktree dir to be removed after Destroy") + } +} + +// quoteForShell wraps a path so it survives splitting by /bin/sh or cmd.exe. +// Tests only use temp-dir paths which don't contain quotes, so simple quoting +// is sufficient. +func quoteForShell(p string) string { + // Double-quote works in both sh and cmd.exe for paths without quotes. + return `"` + p + `"` +} diff --git a/treehouse.toml.example b/treehouse.toml.example index 97239d3..817fa72 100644 --- a/treehouse.toml.example +++ b/treehouse.toml.example @@ -3,3 +3,9 @@ # Maximum number of worktrees in the pool max_trees = 16 + +# Lifecycle hooks. Each list runs sequentially in the worktree directory, +# via the OS shell. Failures are logged and do not fail the operation. +# [hooks] +# post_create = ["./scripts/setup-venv.sh"] +# pre_destroy = ["./scripts/teardown.sh"] From bd8e81644f05347b5feb51d0a640a79dc81cf03c Mon Sep 17 00:00:00 2001 From: kunchenguid Date: Thu, 14 May 2026 22:49:17 -0700 Subject: [PATCH 02/18] no-mistakes(review): Run hooks outside state lock --- internal/pool/pool.go | 62 ++++++++++++++++++++++++++++++++------ internal/pool/pool_test.go | 46 ++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 9 deletions(-) diff --git a/internal/pool/pool.go b/internal/pool/pool.go index a119b4d..bf7b284 100644 --- a/internal/pool/pool.go +++ b/internal/pool/pool.go @@ -40,6 +40,7 @@ func Acquire(repoRoot, poolDir string, poolSize int, postCreate []string) (strin } var acquired string + var runPostCreate bool err = WithStateLock(poolDir, func() error { state, err := ReadState(poolDir) @@ -67,7 +68,7 @@ func Acquire(repoRoot, poolDir string, poolSize int, postCreate []string) (strin if err := WriteState(poolDir, state); err != nil { return err } - hooks.Run(postCreate, acquired, os.Stdout, os.Stderr) + runPostCreate = true return nil } @@ -98,11 +99,17 @@ func Acquire(repoRoot, poolDir string, poolSize int, postCreate []string) (strin if err := WriteState(poolDir, state); err != nil { return err } - hooks.Run(postCreate, acquired, os.Stdout, os.Stderr) + runPostCreate = true return nil }) + if err != nil { + return "", err + } + if runPostCreate { + hooks.Run(postCreate, acquired, os.Stdout, os.Stderr) + } - return acquired, err + return acquired, nil } func Release(poolDir, worktreePath string) error { @@ -161,7 +168,7 @@ func List(poolDir string) ([]WorktreeStatus, error) { } func Destroy(repoRoot, poolDir, worktreePath string, force bool, preDestroy []string) error { - return WithStateLock(poolDir, func() error { + if err := WithStateLock(poolDir, func() error { state, err := ReadState(poolDir) if err != nil { return err @@ -185,7 +192,29 @@ func Destroy(repoRoot, poolDir, worktreePath string, force bool, preDestroy []st } } - hooks.Run(preDestroy, worktreePath, os.Stdout, os.Stderr) + return nil + }); err != nil { + return err + } + + hooks.Run(preDestroy, worktreePath, os.Stdout, os.Stderr) + + return WithStateLock(poolDir, func() error { + state, err := ReadState(poolDir) + if err != nil { + return err + } + + idx := -1 + for i, wt := range state.Worktrees { + if wt.Path == worktreePath { + idx = i + break + } + } + if idx == -1 { + return fmt.Errorf("worktree %s is not managed by treehouse", worktreePath) + } _ = git.RemoveWorktree(repoRoot, worktreePath) // also clean up the parent numbered directory @@ -197,7 +226,8 @@ func Destroy(repoRoot, poolDir, worktreePath string, force bool, preDestroy []st } func DestroyAll(repoRoot, poolDir string, force bool, preDestroy []string) error { - return WithStateLock(poolDir, func() error { + var worktrees []WorktreeEntry + if err := WithStateLock(poolDir, func() error { state, err := ReadState(poolDir) if err != nil { return err @@ -212,12 +242,26 @@ func DestroyAll(repoRoot, poolDir string, force bool, preDestroy []string) error } } - for _, wt := range state.Worktrees { - hooks.Run(preDestroy, wt.Path, os.Stdout, os.Stderr) + worktrees = append([]WorktreeEntry(nil), state.Worktrees...) + return nil + }); err != nil { + return err + } + + for _, wt := range worktrees { + hooks.Run(preDestroy, wt.Path, os.Stdout, os.Stderr) + } + + return WithStateLock(poolDir, func() error { + state, err := ReadState(poolDir) + if err != nil { + return err + } + + for _, wt := range worktrees { _ = git.RemoveWorktree(repoRoot, wt.Path) os.RemoveAll(filepath.Dir(wt.Path)) } - state.Worktrees = nil return WriteState(poolDir, state) }) diff --git a/internal/pool/pool_test.go b/internal/pool/pool_test.go index da7b217..3dba5fb 100644 --- a/internal/pool/pool_test.go +++ b/internal/pool/pool_test.go @@ -6,6 +6,7 @@ import ( "path/filepath" "strings" "testing" + "time" ) func setupRepo(t *testing.T) (repoDir, poolDir string) { @@ -89,6 +90,51 @@ func TestAcquire_HookFailureDoesNotFailAcquire(t *testing.T) { } } +func TestAcquire_RunsPostCreateHookAfterReleasingStateLock(t *testing.T) { + repoDir, poolDir := setupRepo(t) + sentinel := filepath.Join(t.TempDir(), "lock-probe.txt") + hook := quoteForShell(os.Args[0]) + " -test.run=TestHookLockProbe -- " + quoteForShell(poolDir) + " " + quoteForShell(sentinel) + + if _, err := Acquire(repoDir, poolDir, 4, []string{hook}); err != nil { + t.Fatalf("Acquire failed: %v", err) + } + + data, err := os.ReadFile(sentinel) + if err != nil { + t.Fatalf("expected lock probe output: %v", err) + } + if got := strings.TrimSpace(string(data)); got != "unlocked" { + t.Fatalf("expected hook to run after state lock release, got %q", got) + } +} + +func TestHookLockProbe(t *testing.T) { + if len(os.Args) < 3 || os.Args[len(os.Args)-3] != "--" { + return + } + + poolDir := os.Args[len(os.Args)-2] + sentinel := os.Args[len(os.Args)-1] + done := make(chan error, 1) + go func() { + done <- WithStateLock(poolDir, func() error { + return os.WriteFile(sentinel, []byte("unlocked\n"), 0o644) + }) + }() + + select { + case err := <-done: + if err != nil { + t.Fatal(err) + } + case <-time.After(500 * time.Millisecond): + if err := os.WriteFile(sentinel, []byte("locked\n"), 0o644); err != nil { + t.Fatal(err) + } + t.Fatal("state lock was still held while hook ran") + } +} + func TestDestroy_RunsPreDestroyHook(t *testing.T) { repoDir, poolDir := setupRepo(t) From d3c49f74b98ad2e0e91d888fe6576ed5c416968f Mon Sep 17 00:00:00 2001 From: kunchenguid Date: Thu, 14 May 2026 22:52:43 -0700 Subject: [PATCH 03/18] no-mistakes(review): Guard destroy hooks against lifecycle races --- internal/pool/pool.go | 28 +++++++++++-- internal/pool/pool_test.go | 80 ++++++++++++++++++++++++++++++++++++++ internal/pool/state.go | 7 ++-- 3 files changed, 109 insertions(+), 6 deletions(-) diff --git a/internal/pool/pool.go b/internal/pool/pool.go index bf7b284..793b89c 100644 --- a/internal/pool/pool.go +++ b/internal/pool/pool.go @@ -52,6 +52,9 @@ func Acquire(repoRoot, poolDir string, poolSize int, postCreate []string) (strin // Try to find an available worktree (clean and not in-use) for _, wt := range state.Worktrees { + if wt.Destroying { + continue + } inUse, _ := process.IsWorktreeInUse(wt.Path) if inUse { continue @@ -141,6 +144,9 @@ func List(poolDir string) ([]WorktreeStatus, error) { cwd, _ := os.Getwd() for _, wt := range state.Worktrees { + if wt.Destroying { + continue + } ws := WorktreeStatus{ Name: wt.Name, Path: wt.Path, @@ -192,7 +198,8 @@ func Destroy(repoRoot, poolDir, worktreePath string, force bool, preDestroy []st } } - return nil + state.Worktrees[idx].Destroying = true + return WriteState(poolDir, state) }); err != nil { return err } @@ -235,6 +242,9 @@ func DestroyAll(repoRoot, poolDir string, force bool, preDestroy []string) error if !force { for _, wt := range state.Worktrees { + if wt.Destroying { + continue + } inUse, _ := process.IsWorktreeInUse(wt.Path) if inUse { return fmt.Errorf("worktree %s is in use by an agent. Use --force to override", wt.Path) @@ -243,7 +253,10 @@ func DestroyAll(repoRoot, poolDir string, force bool, preDestroy []string) error } worktrees = append([]WorktreeEntry(nil), state.Worktrees...) - return nil + for i := range state.Worktrees { + state.Worktrees[i].Destroying = true + } + return WriteState(poolDir, state) }); err != nil { return err } @@ -258,11 +271,20 @@ func DestroyAll(repoRoot, poolDir string, force bool, preDestroy []string) error return err } + remove := make(map[string]struct{}, len(worktrees)) for _, wt := range worktrees { + remove[wt.Path] = struct{}{} _ = git.RemoveWorktree(repoRoot, wt.Path) os.RemoveAll(filepath.Dir(wt.Path)) } - state.Worktrees = nil + + kept := state.Worktrees[:0] + for _, wt := range state.Worktrees { + if _, ok := remove[wt.Path]; !ok { + kept = append(kept, wt) + } + } + state.Worktrees = kept return WriteState(poolDir, state) }) } diff --git a/internal/pool/pool_test.go b/internal/pool/pool_test.go index 3dba5fb..e2dc375 100644 --- a/internal/pool/pool_test.go +++ b/internal/pool/pool_test.go @@ -163,6 +163,86 @@ func TestDestroy_RunsPreDestroyHook(t *testing.T) { } } +func TestDestroy_DoesNotAllowHookAcquireToReusePendingDestroyWorktree(t *testing.T) { + repoDir, poolDir := setupRepo(t) + + wtPath, err := Acquire(repoDir, poolDir, 4, nil) + if err != nil { + t.Fatalf("Acquire failed: %v", err) + } + + sentinel := filepath.Join(t.TempDir(), "acquired.txt") + hook := quoteForShell(os.Args[0]) + " -test.run=TestAcquireDuringHookProbe -- " + quoteForShell(repoDir) + " " + quoteForShell(poolDir) + " " + quoteForShell(sentinel) + + if err := Destroy(repoDir, poolDir, wtPath, true, []string{hook}); err != nil { + t.Fatalf("Destroy failed: %v", err) + } + + acquiredData, err := os.ReadFile(sentinel) + if err != nil { + t.Fatalf("expected hook acquire output: %v", err) + } + acquired := strings.TrimSpace(string(acquiredData)) + if acquired == wtPath { + t.Fatalf("hook acquire reused pending destroy worktree %s", wtPath) + } + if _, err := os.Stat(acquired); err != nil { + t.Fatalf("expected hook-acquired worktree to remain on disk: %v", err) + } +} + +func TestDestroyAll_PreservesWorktreeAcquiredByHook(t *testing.T) { + repoDir, poolDir := setupRepo(t) + + if _, err := Acquire(repoDir, poolDir, 4, nil); err != nil { + t.Fatalf("first Acquire failed: %v", err) + } + if _, err := Acquire(repoDir, poolDir, 4, nil); err != nil { + t.Fatalf("second Acquire failed: %v", err) + } + + sentinel := filepath.Join(t.TempDir(), "acquired.txt") + hook := quoteForShell(os.Args[0]) + " -test.run=TestAcquireDuringHookProbe -- " + quoteForShell(repoDir) + " " + quoteForShell(poolDir) + " " + quoteForShell(sentinel) + + if err := DestroyAll(repoDir, poolDir, true, []string{hook}); err != nil { + t.Fatalf("DestroyAll failed: %v", err) + } + + acquiredData, err := os.ReadFile(sentinel) + if err != nil { + t.Fatalf("expected hook acquire output: %v", err) + } + acquired := strings.TrimSpace(string(acquiredData)) + if _, err := os.Stat(acquired); err != nil { + t.Fatalf("expected hook-acquired worktree to remain on disk: %v", err) + } + + state, err := ReadState(poolDir) + if err != nil { + t.Fatalf("ReadState failed: %v", err) + } + if len(state.Worktrees) != 1 || state.Worktrees[0].Path != acquired { + t.Fatalf("expected state to preserve hook-acquired worktree %s, got %#v", acquired, state.Worktrees) + } +} + +func TestAcquireDuringHookProbe(t *testing.T) { + if len(os.Args) < 5 || os.Args[len(os.Args)-4] != "--" { + return + } + + repoDir := os.Args[len(os.Args)-3] + poolDir := os.Args[len(os.Args)-2] + sentinel := os.Args[len(os.Args)-1] + wtPath, err := Acquire(repoDir, poolDir, 4, nil) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(sentinel, []byte(wtPath+"\n"), 0o644); err != nil { + t.Fatal(err) + } +} + // quoteForShell wraps a path so it survives splitting by /bin/sh or cmd.exe. // Tests only use temp-dir paths which don't contain quotes, so simple quoting // is sufficient. diff --git a/internal/pool/state.go b/internal/pool/state.go index b2806fa..f0d358a 100644 --- a/internal/pool/state.go +++ b/internal/pool/state.go @@ -8,9 +8,10 @@ import ( ) type WorktreeEntry struct { - Name string `json:"name"` - Path string `json:"path"` - CreatedAt time.Time `json:"created_at"` + Name string `json:"name"` + Path string `json:"path"` + CreatedAt time.Time `json:"created_at"` + Destroying bool `json:"destroying,omitempty"` } type State struct { From af7118e95e78bc98a8f84d48fbc4b272d2b966f8 Mon Sep 17 00:00:00 2001 From: kunchenguid Date: Thu, 14 May 2026 22:58:27 -0700 Subject: [PATCH 04/18] no-mistakes(review): Guard hook lifecycle reservations --- internal/pool/pool.go | 37 ++++++++++++-- internal/pool/pool_test.go | 99 ++++++++++++++++++++++++++++++++++++-- internal/pool/state.go | 1 + internal/process/detect.go | 5 ++ 4 files changed, 134 insertions(+), 8 deletions(-) diff --git a/internal/pool/pool.go b/internal/pool/pool.go index 793b89c..5e4c28c 100644 --- a/internal/pool/pool.go +++ b/internal/pool/pool.go @@ -51,8 +51,8 @@ func Acquire(repoRoot, poolDir string, poolSize int, postCreate []string) (strin state = healState(state) // Try to find an available worktree (clean and not in-use) - for _, wt := range state.Worktrees { - if wt.Destroying { + for i, wt := range state.Worktrees { + if wt.Destroying || ownerAlive(wt) { continue } inUse, _ := process.IsWorktreeInUse(wt.Path) @@ -67,6 +67,7 @@ func Acquire(repoRoot, poolDir string, poolSize int, postCreate []string) (strin if err := git.ResetWorktree(wt.Path, branch); err != nil { continue } + state.Worktrees[i].OwnerPID = int32(os.Getpid()) acquired = wt.Path if err := WriteState(poolDir, state); err != nil { return err @@ -96,6 +97,7 @@ func Acquire(repoRoot, poolDir string, poolSize int, postCreate []string) (strin Name: name, Path: wtPath, CreatedAt: time.Now(), + OwnerPID: int32(os.Getpid()), }) acquired = wtPath @@ -124,7 +126,22 @@ func Release(poolDir, worktreePath string) error { if err != nil { return err } - return git.ResetWorktree(worktreePath, branch) + if err := git.ResetWorktree(worktreePath, branch); err != nil { + return err + } + return WithStateLock(poolDir, func() error { + state, err := ReadState(poolDir) + if err != nil { + return err + } + for i := range state.Worktrees { + if state.Worktrees[i].Path == worktreePath { + state.Worktrees[i].OwnerPID = 0 + break + } + } + return WriteState(poolDir, state) + }) } func List(poolDir string) ([]WorktreeStatus, error) { @@ -156,7 +173,9 @@ func List(poolDir string) ([]WorktreeStatus, error) { procs, _ := process.FindProcessesInWorktree(wt.Path) ws.Processes = procs - if len(procs) > 0 { + if ownerAlive(wt) { + ws.Status = StatusInUse + } else if len(procs) > 0 { ws.Status = StatusInUse if cwdInWorktree(cwd, wt.Path) { ws.Status = StatusHere @@ -199,6 +218,7 @@ func Destroy(repoRoot, poolDir, worktreePath string, force bool, preDestroy []st } state.Worktrees[idx].Destroying = true + state.Worktrees[idx].OwnerPID = int32(os.Getpid()) return WriteState(poolDir, state) }); err != nil { return err @@ -255,6 +275,7 @@ func DestroyAll(repoRoot, poolDir string, force bool, preDestroy []string) error worktrees = append([]WorktreeEntry(nil), state.Worktrees...) for i := range state.Worktrees { state.Worktrees[i].Destroying = true + state.Worktrees[i].OwnerPID = int32(os.Getpid()) } return WriteState(poolDir, state) }); err != nil { @@ -306,6 +327,10 @@ func healState(state State) State { var healed []WorktreeEntry for _, wt := range state.Worktrees { if _, err := os.Stat(wt.Path); err == nil { + if wt.OwnerPID != 0 && !ownerAlive(wt) { + wt.OwnerPID = 0 + wt.Destroying = false + } healed = append(healed, wt) } } @@ -313,6 +338,10 @@ func healState(state State) State { return state } +func ownerAlive(wt WorktreeEntry) bool { + return wt.OwnerPID != 0 && process.Exists(wt.OwnerPID) +} + func cwdInWorktree(cwd, worktreePath string) bool { absCwd, err := filepath.Abs(cwd) if err != nil { diff --git a/internal/pool/pool_test.go b/internal/pool/pool_test.go index e2dc375..b5e1c9c 100644 --- a/internal/pool/pool_test.go +++ b/internal/pool/pool_test.go @@ -108,6 +108,82 @@ func TestAcquire_RunsPostCreateHookAfterReleasingStateLock(t *testing.T) { } } +func TestAcquire_DoesNotReuseWorktreeReservedByPostCreateHook(t *testing.T) { + repoDir, poolDir := setupRepo(t) + sentinel := filepath.Join(t.TempDir(), "acquired.txt") + hookCwd := t.TempDir() + hook := quoteForShell(os.Args[0]) + " -test.run=TestAcquireDuringHookProbe -- " + quoteForShell(repoDir) + " " + quoteForShell(poolDir) + " " + quoteForShell(sentinel) + " " + quoteForShell(hookCwd) + + wtPath, err := Acquire(repoDir, poolDir, 4, []string{hook}) + if err != nil { + t.Fatalf("Acquire failed: %v", err) + } + + acquiredData, err := os.ReadFile(sentinel) + if err != nil { + t.Fatalf("expected hook acquire output: %v", err) + } + acquired := strings.TrimSpace(string(acquiredData)) + if acquired == wtPath { + t.Fatalf("hook acquire reused reserved worktree %s", wtPath) + } +} + +func TestList_RecoversDestroyingWorktreeWhenOwnerIsGone(t *testing.T) { + repoDir, poolDir := setupRepo(t) + + wtPath, err := Acquire(repoDir, poolDir, 4, nil) + if err != nil { + t.Fatalf("Acquire failed: %v", err) + } + if err := Release(poolDir, wtPath); err != nil { + t.Fatalf("Release failed: %v", err) + } + + state, err := ReadState(poolDir) + if err != nil { + t.Fatalf("ReadState failed: %v", err) + } + state.Worktrees[0].Destroying = true + state.Worktrees[0].OwnerPID = 999999 + if err := WriteState(poolDir, state); err != nil { + t.Fatalf("WriteState failed: %v", err) + } + + statuses, err := List(poolDir) + if err != nil { + t.Fatalf("List failed: %v", err) + } + if len(statuses) != 1 || statuses[0].Path != wtPath { + t.Fatalf("expected stale destroying worktree to be visible, got %#v", statuses) + } + + state, err = ReadState(poolDir) + if err != nil { + t.Fatalf("ReadState failed: %v", err) + } + if state.Worktrees[0].Destroying || state.Worktrees[0].OwnerPID != 0 { + t.Fatalf("expected stale destroy reservation to be cleared, got %#v", state.Worktrees[0]) + } +} + +func TestList_ShowsReservedWorktreeAsInUse(t *testing.T) { + repoDir, poolDir := setupRepo(t) + + wtPath, err := Acquire(repoDir, poolDir, 4, nil) + if err != nil { + t.Fatalf("Acquire failed: %v", err) + } + + statuses, err := List(poolDir) + if err != nil { + t.Fatalf("List failed: %v", err) + } + if len(statuses) != 1 || statuses[0].Path != wtPath || statuses[0].Status != StatusInUse { + t.Fatalf("expected reserved worktree to be listed as in-use, got %#v", statuses) + } +} + func TestHookLockProbe(t *testing.T) { if len(os.Args) < 3 || os.Args[len(os.Args)-3] != "--" { return @@ -227,13 +303,28 @@ func TestDestroyAll_PreservesWorktreeAcquiredByHook(t *testing.T) { } func TestAcquireDuringHookProbe(t *testing.T) { - if len(os.Args) < 5 || os.Args[len(os.Args)-4] != "--" { + if len(os.Args) < 5 { + return + } + argStart := -1 + for i := len(os.Args) - 1; i >= 0; i-- { + if os.Args[i] == "--" { + argStart = i + break + } + } + if argStart == -1 || len(os.Args)-argStart < 4 { return } - repoDir := os.Args[len(os.Args)-3] - poolDir := os.Args[len(os.Args)-2] - sentinel := os.Args[len(os.Args)-1] + repoDir := os.Args[argStart+1] + poolDir := os.Args[argStart+2] + sentinel := os.Args[argStart+3] + if len(os.Args) > argStart+4 { + if err := os.Chdir(os.Args[argStart+4]); err != nil { + t.Fatal(err) + } + } wtPath, err := Acquire(repoDir, poolDir, 4, nil) if err != nil { t.Fatal(err) diff --git a/internal/pool/state.go b/internal/pool/state.go index f0d358a..2ffef5f 100644 --- a/internal/pool/state.go +++ b/internal/pool/state.go @@ -12,6 +12,7 @@ type WorktreeEntry struct { Path string `json:"path"` CreatedAt time.Time `json:"created_at"` Destroying bool `json:"destroying,omitempty"` + OwnerPID int32 `json:"owner_pid,omitempty"` } type State struct { diff --git a/internal/process/detect.go b/internal/process/detect.go index cabebe3..3c185bf 100644 --- a/internal/process/detect.go +++ b/internal/process/detect.go @@ -25,6 +25,11 @@ func IsWorktreeInUse(worktreePath string) (bool, error) { return len(procs) > 0, nil } +func Exists(pid int32) bool { + exists, err := process.PidExists(pid) + return err == nil && exists +} + func FindProcessesInWorktree(worktreePath string) ([]ProcessInfo, error) { procs, err := process.Processes() if err != nil { From f6afaa3820bd475e9377110e67312d47bb80c276 Mon Sep 17 00:00:00 2001 From: kunchenguid Date: Thu, 14 May 2026 23:08:22 -0700 Subject: [PATCH 05/18] no-mistakes(review): Honor owner reservations during destroy --- internal/pool/pool.go | 11 +++++++++-- internal/pool/pool_test.go | 40 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/internal/pool/pool.go b/internal/pool/pool.go index 5e4c28c..2c2eac8 100644 --- a/internal/pool/pool.go +++ b/internal/pool/pool.go @@ -211,7 +211,7 @@ func Destroy(repoRoot, poolDir, worktreePath string, force bool, preDestroy []st } if !force { - inUse, _ := process.IsWorktreeInUse(worktreePath) + inUse, _ := worktreeInUse(state.Worktrees[idx]) if inUse { return fmt.Errorf("worktree %s is in use by an agent. Use --force to override", worktreePath) } @@ -265,7 +265,7 @@ func DestroyAll(repoRoot, poolDir string, force bool, preDestroy []string) error if wt.Destroying { continue } - inUse, _ := process.IsWorktreeInUse(wt.Path) + inUse, _ := worktreeInUse(wt) if inUse { return fmt.Errorf("worktree %s is in use by an agent. Use --force to override", wt.Path) } @@ -342,6 +342,13 @@ func ownerAlive(wt WorktreeEntry) bool { return wt.OwnerPID != 0 && process.Exists(wt.OwnerPID) } +func worktreeInUse(wt WorktreeEntry) (bool, error) { + if ownerAlive(wt) { + return true, nil + } + return process.IsWorktreeInUse(wt.Path) +} + func cwdInWorktree(cwd, worktreePath string) bool { absCwd, err := filepath.Abs(cwd) if err != nil { diff --git a/internal/pool/pool_test.go b/internal/pool/pool_test.go index b5e1c9c..bba5747 100644 --- a/internal/pool/pool_test.go +++ b/internal/pool/pool_test.go @@ -267,6 +267,26 @@ func TestDestroy_DoesNotAllowHookAcquireToReusePendingDestroyWorktree(t *testing } } +func TestDestroy_NonForceRejectsReservedWorktree(t *testing.T) { + repoDir, poolDir := setupRepo(t) + + wtPath, err := Acquire(repoDir, poolDir, 4, nil) + if err != nil { + t.Fatalf("Acquire failed: %v", err) + } + + err = Destroy(repoDir, poolDir, wtPath, false, nil) + if err == nil { + t.Fatal("expected non-force Destroy to reject reserved worktree") + } + if !strings.Contains(err.Error(), "is in use") { + t.Fatalf("expected in-use error, got %v", err) + } + if _, err := os.Stat(wtPath); err != nil { + t.Fatalf("expected reserved worktree to remain on disk: %v", err) + } +} + func TestDestroyAll_PreservesWorktreeAcquiredByHook(t *testing.T) { repoDir, poolDir := setupRepo(t) @@ -302,6 +322,26 @@ func TestDestroyAll_PreservesWorktreeAcquiredByHook(t *testing.T) { } } +func TestDestroyAll_NonForceRejectsReservedWorktree(t *testing.T) { + repoDir, poolDir := setupRepo(t) + + wtPath, err := Acquire(repoDir, poolDir, 4, nil) + if err != nil { + t.Fatalf("Acquire failed: %v", err) + } + + err = DestroyAll(repoDir, poolDir, false, nil) + if err == nil { + t.Fatal("expected non-force DestroyAll to reject reserved worktree") + } + if !strings.Contains(err.Error(), "is in use") { + t.Fatalf("expected in-use error, got %v", err) + } + if _, err := os.Stat(wtPath); err != nil { + t.Fatalf("expected reserved worktree to remain on disk: %v", err) + } +} + func TestAcquireDuringHookProbe(t *testing.T) { if len(os.Args) < 5 { return From f482697de995b8ba43e8adc098a8a176c0449c68 Mon Sep 17 00:00:00 2001 From: kunchenguid Date: Thu, 14 May 2026 23:11:59 -0700 Subject: [PATCH 06/18] no-mistakes(review): Reject live destroy reservations --- internal/pool/pool.go | 3 --- internal/pool/pool_test.go | 40 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/internal/pool/pool.go b/internal/pool/pool.go index 2c2eac8..b759422 100644 --- a/internal/pool/pool.go +++ b/internal/pool/pool.go @@ -262,9 +262,6 @@ func DestroyAll(repoRoot, poolDir string, force bool, preDestroy []string) error if !force { for _, wt := range state.Worktrees { - if wt.Destroying { - continue - } inUse, _ := worktreeInUse(wt) if inUse { return fmt.Errorf("worktree %s is in use by an agent. Use --force to override", wt.Path) diff --git a/internal/pool/pool_test.go b/internal/pool/pool_test.go index bba5747..d5f2e29 100644 --- a/internal/pool/pool_test.go +++ b/internal/pool/pool_test.go @@ -342,6 +342,46 @@ func TestDestroyAll_NonForceRejectsReservedWorktree(t *testing.T) { } } +func TestDestroyAll_NonForceRejectsLiveDestroyingWorktree(t *testing.T) { + repoDir, poolDir := setupRepo(t) + + wtPath, err := Acquire(repoDir, poolDir, 4, nil) + if err != nil { + t.Fatalf("Acquire failed: %v", err) + } + if err := Release(poolDir, wtPath); err != nil { + t.Fatalf("Release failed: %v", err) + } + + state, err := ReadState(poolDir) + if err != nil { + t.Fatalf("ReadState failed: %v", err) + } + state.Worktrees[0].Destroying = true + state.Worktrees[0].OwnerPID = int32(os.Getpid()) + if err := WriteState(poolDir, state); err != nil { + t.Fatalf("WriteState failed: %v", err) + } + + err = DestroyAll(repoDir, poolDir, false, nil) + if err == nil { + t.Fatal("expected non-force DestroyAll to reject live destroying worktree") + } + if !strings.Contains(err.Error(), "is in use") { + t.Fatalf("expected in-use error, got %v", err) + } + if _, err := os.Stat(wtPath); err != nil { + t.Fatalf("expected live destroying worktree to remain on disk: %v", err) + } + state, err = ReadState(poolDir) + if err != nil { + t.Fatalf("ReadState failed: %v", err) + } + if len(state.Worktrees) != 1 || state.Worktrees[0].Path != wtPath || state.Worktrees[0].OwnerPID != int32(os.Getpid()) { + t.Fatalf("expected live destroy reservation to remain unchanged, got %#v", state.Worktrees) + } +} + func TestAcquireDuringHookProbe(t *testing.T) { if len(os.Args) < 5 { return From 3893dbf8973338024348ca9b3eeea1f9be104095 Mon Sep 17 00:00:00 2001 From: kunchenguid Date: Thu, 14 May 2026 23:17:57 -0700 Subject: [PATCH 07/18] no-mistakes(review): Validate owner reservations by process identity --- internal/pool/pool.go | 40 ++++++++++++++++++++++++++++------- internal/pool/pool_test.go | 43 +++++++++++++++++++++++++++++++++++++- internal/pool/state.go | 11 +++++----- internal/process/detect.go | 9 ++++++++ 4 files changed, 90 insertions(+), 13 deletions(-) diff --git a/internal/pool/pool.go b/internal/pool/pool.go index b759422..614259b 100644 --- a/internal/pool/pool.go +++ b/internal/pool/pool.go @@ -67,7 +67,9 @@ func Acquire(repoRoot, poolDir string, poolSize int, postCreate []string) (strin if err := git.ResetWorktree(wt.Path, branch); err != nil { continue } - state.Worktrees[i].OwnerPID = int32(os.Getpid()) + if err := reserveOwner(&state.Worktrees[i]); err != nil { + return err + } acquired = wt.Path if err := WriteState(poolDir, state); err != nil { return err @@ -93,12 +95,15 @@ func Acquire(repoRoot, poolDir string, poolSize int, postCreate []string) (strin return fmt.Errorf("failed to create worktree: %w", err) } - state.Worktrees = append(state.Worktrees, WorktreeEntry{ + entry := WorktreeEntry{ Name: name, Path: wtPath, CreatedAt: time.Now(), - OwnerPID: int32(os.Getpid()), - }) + } + if err := reserveOwner(&entry); err != nil { + return err + } + state.Worktrees = append(state.Worktrees, entry) acquired = wtPath if err := WriteState(poolDir, state); err != nil { @@ -137,6 +142,7 @@ func Release(poolDir, worktreePath string) error { for i := range state.Worktrees { if state.Worktrees[i].Path == worktreePath { state.Worktrees[i].OwnerPID = 0 + state.Worktrees[i].OwnerStartedAt = 0 break } } @@ -218,7 +224,9 @@ func Destroy(repoRoot, poolDir, worktreePath string, force bool, preDestroy []st } state.Worktrees[idx].Destroying = true - state.Worktrees[idx].OwnerPID = int32(os.Getpid()) + if err := reserveOwner(&state.Worktrees[idx]); err != nil { + return err + } return WriteState(poolDir, state) }); err != nil { return err @@ -272,7 +280,9 @@ func DestroyAll(repoRoot, poolDir string, force bool, preDestroy []string) error worktrees = append([]WorktreeEntry(nil), state.Worktrees...) for i := range state.Worktrees { state.Worktrees[i].Destroying = true - state.Worktrees[i].OwnerPID = int32(os.Getpid()) + if err := reserveOwner(&state.Worktrees[i]); err != nil { + return err + } } return WriteState(poolDir, state) }); err != nil { @@ -326,6 +336,7 @@ func healState(state State) State { if _, err := os.Stat(wt.Path); err == nil { if wt.OwnerPID != 0 && !ownerAlive(wt) { wt.OwnerPID = 0 + wt.OwnerStartedAt = 0 wt.Destroying = false } healed = append(healed, wt) @@ -336,7 +347,22 @@ func healState(state State) State { } func ownerAlive(wt WorktreeEntry) bool { - return wt.OwnerPID != 0 && process.Exists(wt.OwnerPID) + if wt.OwnerPID == 0 || wt.OwnerStartedAt == 0 { + return false + } + startedAt, ok := process.StartedAt(wt.OwnerPID) + return ok && startedAt == wt.OwnerStartedAt +} + +func reserveOwner(wt *WorktreeEntry) error { + pid := int32(os.Getpid()) + startedAt, ok := process.StartedAt(pid) + if !ok { + return fmt.Errorf("failed to determine owner process identity") + } + wt.OwnerPID = pid + wt.OwnerStartedAt = startedAt + return nil } func worktreeInUse(wt WorktreeEntry) (bool, error) { diff --git a/internal/pool/pool_test.go b/internal/pool/pool_test.go index d5f2e29..eb70823 100644 --- a/internal/pool/pool_test.go +++ b/internal/pool/pool_test.go @@ -167,6 +167,45 @@ func TestList_RecoversDestroyingWorktreeWhenOwnerIsGone(t *testing.T) { } } +func TestList_RecoversDestroyingWorktreeWhenOwnerIdentityDoesNotMatch(t *testing.T) { + repoDir, poolDir := setupRepo(t) + + wtPath, err := Acquire(repoDir, poolDir, 4, nil) + if err != nil { + t.Fatalf("Acquire failed: %v", err) + } + if err := Release(poolDir, wtPath); err != nil { + t.Fatalf("Release failed: %v", err) + } + + state, err := ReadState(poolDir) + if err != nil { + t.Fatalf("ReadState failed: %v", err) + } + state.Worktrees[0].Destroying = true + state.Worktrees[0].OwnerPID = int32(os.Getpid()) + state.Worktrees[0].OwnerStartedAt = 1 + if err := WriteState(poolDir, state); err != nil { + t.Fatalf("WriteState failed: %v", err) + } + + statuses, err := List(poolDir) + if err != nil { + t.Fatalf("List failed: %v", err) + } + if len(statuses) != 1 || statuses[0].Path != wtPath { + t.Fatalf("expected stale destroying worktree to be visible, got %#v", statuses) + } + + state, err = ReadState(poolDir) + if err != nil { + t.Fatalf("ReadState failed: %v", err) + } + if state.Worktrees[0].Destroying || state.Worktrees[0].OwnerPID != 0 || state.Worktrees[0].OwnerStartedAt != 0 { + t.Fatalf("expected stale owner reservation to be cleared, got %#v", state.Worktrees[0]) + } +} + func TestList_ShowsReservedWorktreeAsInUse(t *testing.T) { repoDir, poolDir := setupRepo(t) @@ -358,7 +397,9 @@ func TestDestroyAll_NonForceRejectsLiveDestroyingWorktree(t *testing.T) { t.Fatalf("ReadState failed: %v", err) } state.Worktrees[0].Destroying = true - state.Worktrees[0].OwnerPID = int32(os.Getpid()) + if err := reserveOwner(&state.Worktrees[0]); err != nil { + t.Fatalf("reserveOwner failed: %v", err) + } if err := WriteState(poolDir, state); err != nil { t.Fatalf("WriteState failed: %v", err) } diff --git a/internal/pool/state.go b/internal/pool/state.go index 2ffef5f..aded62b 100644 --- a/internal/pool/state.go +++ b/internal/pool/state.go @@ -8,11 +8,12 @@ import ( ) type WorktreeEntry struct { - Name string `json:"name"` - Path string `json:"path"` - CreatedAt time.Time `json:"created_at"` - Destroying bool `json:"destroying,omitempty"` - OwnerPID int32 `json:"owner_pid,omitempty"` + Name string `json:"name"` + Path string `json:"path"` + CreatedAt time.Time `json:"created_at"` + Destroying bool `json:"destroying,omitempty"` + OwnerPID int32 `json:"owner_pid,omitempty"` + OwnerStartedAt int64 `json:"owner_started_at,omitempty"` } type State struct { diff --git a/internal/process/detect.go b/internal/process/detect.go index 3c185bf..cbd4998 100644 --- a/internal/process/detect.go +++ b/internal/process/detect.go @@ -30,6 +30,15 @@ func Exists(pid int32) bool { return err == nil && exists } +func StartedAt(pid int32) (int64, bool) { + proc, err := process.NewProcess(pid) + if err != nil { + return 0, false + } + startedAt, err := proc.CreateTime() + return startedAt, err == nil +} + func FindProcessesInWorktree(worktreePath string) ([]ProcessInfo, error) { procs, err := process.Processes() if err != nil { From 19e725c0e0d398e501f2e19e1251cb7a83566cb2 Mon Sep 17 00:00:00 2001 From: kunchenguid Date: Thu, 14 May 2026 23:23:32 -0700 Subject: [PATCH 08/18] no-mistakes(review): Validate destroy reservation cleanup --- internal/pool/pool.go | 26 +++++++++- internal/pool/pool_test.go | 97 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+), 2 deletions(-) diff --git a/internal/pool/pool.go b/internal/pool/pool.go index 614259b..4a4c157 100644 --- a/internal/pool/pool.go +++ b/internal/pool/pool.go @@ -199,6 +199,7 @@ func List(poolDir string) ([]WorktreeStatus, error) { } func Destroy(repoRoot, poolDir, worktreePath string, force bool, preDestroy []string) error { + var reserved WorktreeEntry if err := WithStateLock(poolDir, func() error { state, err := ReadState(poolDir) if err != nil { @@ -227,6 +228,7 @@ func Destroy(repoRoot, poolDir, worktreePath string, force bool, preDestroy []st if err := reserveOwner(&state.Worktrees[idx]); err != nil { return err } + reserved = state.Worktrees[idx] return WriteState(poolDir, state) }); err != nil { return err @@ -248,7 +250,10 @@ func Destroy(repoRoot, poolDir, worktreePath string, force bool, preDestroy []st } } if idx == -1 { - return fmt.Errorf("worktree %s is not managed by treehouse", worktreePath) + return nil + } + if !sameDestroyReservation(state.Worktrees[idx], reserved) { + return nil } _ = git.RemoveWorktree(repoRoot, worktreePath) @@ -277,13 +282,13 @@ func DestroyAll(repoRoot, poolDir string, force bool, preDestroy []string) error } } - worktrees = append([]WorktreeEntry(nil), state.Worktrees...) for i := range state.Worktrees { state.Worktrees[i].Destroying = true if err := reserveOwner(&state.Worktrees[i]); err != nil { return err } } + worktrees = append([]WorktreeEntry(nil), state.Worktrees...) return WriteState(poolDir, state) }); err != nil { return err @@ -301,6 +306,16 @@ func DestroyAll(repoRoot, poolDir string, force bool, preDestroy []string) error remove := make(map[string]struct{}, len(worktrees)) for _, wt := range worktrees { + idx := -1 + for i := range state.Worktrees { + if state.Worktrees[i].Path == wt.Path { + idx = i + break + } + } + if idx == -1 || !sameDestroyReservation(state.Worktrees[idx], wt) { + continue + } remove[wt.Path] = struct{}{} _ = git.RemoveWorktree(repoRoot, wt.Path) os.RemoveAll(filepath.Dir(wt.Path)) @@ -372,6 +387,13 @@ func worktreeInUse(wt WorktreeEntry) (bool, error) { return process.IsWorktreeInUse(wt.Path) } +func sameDestroyReservation(current, reserved WorktreeEntry) bool { + return current.Path == reserved.Path && + current.Destroying && + current.OwnerPID == reserved.OwnerPID && + current.OwnerStartedAt == reserved.OwnerStartedAt +} + func cwdInWorktree(cwd, worktreePath string) bool { absCwd, err := filepath.Abs(cwd) if err != nil { diff --git a/internal/pool/pool_test.go b/internal/pool/pool_test.go index eb70823..9619d0a 100644 --- a/internal/pool/pool_test.go +++ b/internal/pool/pool_test.go @@ -326,6 +326,36 @@ func TestDestroy_NonForceRejectsReservedWorktree(t *testing.T) { } } +func TestDestroy_PreservesSupersededReservationAfterHook(t *testing.T) { + repoDir, poolDir := setupRepo(t) + + wtPath, err := Acquire(repoDir, poolDir, 4, nil) + if err != nil { + t.Fatalf("Acquire failed: %v", err) + } + if err := Release(poolDir, wtPath); err != nil { + t.Fatalf("Release failed: %v", err) + } + + sentinel := filepath.Join(wtPath, "superseded.txt") + hook := quoteForShell(os.Args[0]) + " -test.run=TestSupersedeDestroyReservationProbe -- " + quoteForShell(poolDir) + " " + quoteForShell(wtPath) + " " + quoteForShell(sentinel) + + if err := Destroy(repoDir, poolDir, wtPath, true, []string{hook}); err != nil { + t.Fatalf("Destroy failed: %v", err) + } + if _, err := os.Stat(sentinel); err != nil { + t.Fatalf("expected superseded worktree to remain on disk: %v", err) + } + + state, err := ReadState(poolDir) + if err != nil { + t.Fatalf("ReadState failed: %v", err) + } + if len(state.Worktrees) != 1 || state.Worktrees[0].Path != wtPath || state.Worktrees[0].Destroying { + t.Fatalf("expected superseded state entry to remain available, got %#v", state.Worktrees) + } +} + func TestDestroyAll_PreservesWorktreeAcquiredByHook(t *testing.T) { repoDir, poolDir := setupRepo(t) @@ -361,6 +391,36 @@ func TestDestroyAll_PreservesWorktreeAcquiredByHook(t *testing.T) { } } +func TestDestroyAll_PreservesSupersededReservationAfterHook(t *testing.T) { + repoDir, poolDir := setupRepo(t) + + wtPath, err := Acquire(repoDir, poolDir, 4, nil) + if err != nil { + t.Fatalf("Acquire failed: %v", err) + } + if err := Release(poolDir, wtPath); err != nil { + t.Fatalf("Release failed: %v", err) + } + + sentinel := filepath.Join(wtPath, "superseded.txt") + hook := quoteForShell(os.Args[0]) + " -test.run=TestSupersedeDestroyReservationProbe -- " + quoteForShell(poolDir) + " " + quoteForShell(wtPath) + " " + quoteForShell(sentinel) + + if err := DestroyAll(repoDir, poolDir, true, []string{hook}); err != nil { + t.Fatalf("DestroyAll failed: %v", err) + } + if _, err := os.Stat(sentinel); err != nil { + t.Fatalf("expected superseded worktree to remain on disk: %v", err) + } + + state, err := ReadState(poolDir) + if err != nil { + t.Fatalf("ReadState failed: %v", err) + } + if len(state.Worktrees) != 1 || state.Worktrees[0].Path != wtPath || state.Worktrees[0].Destroying { + t.Fatalf("expected superseded state entry to remain available, got %#v", state.Worktrees) + } +} + func TestDestroyAll_NonForceRejectsReservedWorktree(t *testing.T) { repoDir, poolDir := setupRepo(t) @@ -455,6 +515,43 @@ func TestAcquireDuringHookProbe(t *testing.T) { } } +func TestSupersedeDestroyReservationProbe(t *testing.T) { + if len(os.Args) < 5 { + return + } + argStart := -1 + for i := len(os.Args) - 1; i >= 0; i-- { + if os.Args[i] == "--" { + argStart = i + break + } + } + if argStart == -1 || len(os.Args)-argStart < 4 { + return + } + + poolDir := os.Args[argStart+1] + wtPath := os.Args[argStart+2] + sentinel := os.Args[argStart+3] + state, err := ReadState(poolDir) + if err != nil { + t.Fatal(err) + } + for i := range state.Worktrees { + if state.Worktrees[i].Path == wtPath { + state.Worktrees[i].Destroying = false + state.Worktrees[i].OwnerPID = 0 + state.Worktrees[i].OwnerStartedAt = 0 + } + } + if err := os.WriteFile(sentinel, []byte("superseded\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := WriteState(poolDir, state); err != nil { + t.Fatal(err) + } +} + // quoteForShell wraps a path so it survives splitting by /bin/sh or cmd.exe. // Tests only use temp-dir paths which don't contain quotes, so simple quoting // is sufficient. From 3286365f3ba9f23a7eaca6d029e602f4ef275bd8 Mon Sep 17 00:00:00 2001 From: kunchenguid Date: Thu, 14 May 2026 23:28:48 -0700 Subject: [PATCH 09/18] no-mistakes(review): Trust only user-defined hooks --- internal/config/config.go | 26 ++++++++++++------- internal/config/hooks_test.go | 47 ++++++++++++++++++++++++++++++++++- 2 files changed, 63 insertions(+), 10 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 7584db5..093a087 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -28,20 +28,28 @@ func DefaultConfig() Config { func Load(repoRoot string) (Config, error) { cfg := DefaultConfig() - paths := []string{ - filepath.Join(repoRoot, "treehouse.toml"), + repoPath := filepath.Join(repoRoot, "treehouse.toml") + hasRepoConfig := false + if _, err := os.Stat(repoPath); err == nil { + hasRepoConfig = true + if _, err := toml.DecodeFile(repoPath, &cfg); err != nil { + return cfg, err + } + cfg.Hooks = Hooks{} } if home, err := os.UserHomeDir(); err == nil { - paths = append(paths, filepath.Join(home, ".config", "treehouse", "config.toml")) - } - - for _, p := range paths { - if _, err := os.Stat(p); err == nil { - if _, err := toml.DecodeFile(p, &cfg); err != nil { + userPath := filepath.Join(home, ".config", "treehouse", "config.toml") + if _, err := os.Stat(userPath); err == nil { + userCfg := DefaultConfig() + if _, err := toml.DecodeFile(userPath, &userCfg); err != nil { return cfg, err } - return cfg, nil + if !hasRepoConfig { + cfg = userCfg + } else { + cfg.Hooks = userCfg.Hooks + } } } diff --git a/internal/config/hooks_test.go b/internal/config/hooks_test.go index 2b8063d..660b363 100644 --- a/internal/config/hooks_test.go +++ b/internal/config/hooks_test.go @@ -4,11 +4,13 @@ import ( "os" "path/filepath" "reflect" + "runtime" "testing" ) -func TestLoad_Hooks(t *testing.T) { +func TestLoad_IgnoresRepoHooks(t *testing.T) { repoDir := t.TempDir() + setUserHome(t, t.TempDir()) cfgTOML := `max_trees = 4 @@ -25,6 +27,39 @@ pre_destroy = ["echo c"] t.Fatalf("Load failed: %v", err) } + if cfg.MaxTrees != 4 { + t.Errorf("MaxTrees: got %d, want 4", cfg.MaxTrees) + } + if len(cfg.Hooks.PostCreate) != 0 { + t.Errorf("expected repo post_create hooks to be ignored, got %v", cfg.Hooks.PostCreate) + } + if len(cfg.Hooks.PreDestroy) != 0 { + t.Errorf("expected repo pre_destroy hooks to be ignored, got %v", cfg.Hooks.PreDestroy) + } +} + +func TestLoad_UserHooks(t *testing.T) { + repoDir := t.TempDir() + userHome := t.TempDir() + setUserHome(t, userHome) + + configDir := filepath.Join(userHome, ".config", "treehouse") + if err := os.MkdirAll(configDir, 0o755); err != nil { + t.Fatal(err) + } + cfgTOML := `[hooks] +post_create = ["echo a", "echo b"] +pre_destroy = ["echo c"] +` + if err := os.WriteFile(filepath.Join(configDir, "config.toml"), []byte(cfgTOML), 0o644); err != nil { + t.Fatal(err) + } + + cfg, err := Load(repoDir) + if err != nil { + t.Fatalf("Load failed: %v", err) + } + wantPost := []string{"echo a", "echo b"} wantPre := []string{"echo c"} if !reflect.DeepEqual(cfg.Hooks.PostCreate, wantPost) { @@ -37,6 +72,7 @@ pre_destroy = ["echo c"] func TestLoad_HooksDefaultEmpty(t *testing.T) { repoDir := t.TempDir() + setUserHome(t, t.TempDir()) cfg, err := Load(repoDir) if err != nil { @@ -49,3 +85,12 @@ func TestLoad_HooksDefaultEmpty(t *testing.T) { t.Errorf("expected empty PreDestroy, got %v", cfg.Hooks.PreDestroy) } } + +func setUserHome(t *testing.T, home string) { + t.Helper() + if runtime.GOOS == "windows" { + t.Setenv("USERPROFILE", home) + } else { + t.Setenv("HOME", home) + } +} From 0208c76413814d5db0cc5c291b1a3221f8d31ba4 Mon Sep 17 00:00:00 2001 From: kunchenguid Date: Thu, 14 May 2026 23:31:46 -0700 Subject: [PATCH 10/18] no-mistakes(review): Document user-only lifecycle hooks --- README.md | 7 ++++--- treehouse.toml.example | 8 +++++--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index b449f08..e187ca3 100644 --- a/README.md +++ b/README.md @@ -154,7 +154,7 @@ Treehouse manages a **pool of git worktrees** per repository, stored under `~/.t ## Configuration -Create a config file with `treehouse init`, or add one manually: +Create a repo config file with `treehouse init`, or add one manually: **Repo-level:** `treehouse.toml` in the repository root @@ -165,12 +165,13 @@ Create a config file with `treehouse init`, or add one manually: max_trees = 16 ``` -The repo-level config takes precedence. +The repo-level config takes precedence for repo-safe settings. If no config is found, the default pool size is 16. ### Hooks -You can run commands automatically at worktree lifecycle points by adding a `[hooks]` section: +You can run commands automatically at worktree lifecycle points by adding a `[hooks]` section to the user-level config at `~/.config/treehouse/config.toml`. +Hooks in repo-level `treehouse.toml` are ignored for safety. ```toml [hooks] diff --git a/treehouse.toml.example b/treehouse.toml.example index 817fa72..3f66069 100644 --- a/treehouse.toml.example +++ b/treehouse.toml.example @@ -1,11 +1,13 @@ # Treehouse configuration -# Place this file at /treehouse.toml or ~/.config/treehouse/config.toml +# Place repo-safe settings at /treehouse.toml. +# Place executable hooks only in ~/.config/treehouse/config.toml. # Maximum number of worktrees in the pool max_trees = 16 -# Lifecycle hooks. Each list runs sequentially in the worktree directory, -# via the OS shell. Failures are logged and do not fail the operation. +# Lifecycle hooks are ignored in repo-level config for safety. +# Each list runs sequentially in the worktree directory via the OS shell. +# Failures are logged and do not fail the operation. # [hooks] # post_create = ["./scripts/setup-venv.sh"] # pre_destroy = ["./scripts/teardown.sh"] From 8fbb2451ea530537258e059fdbfa87537a0181f5 Mon Sep 17 00:00:00 2001 From: kunchenguid Date: Thu, 14 May 2026 23:37:10 -0700 Subject: [PATCH 11/18] no-mistakes(review): Reject release during destroy --- internal/pool/pool.go | 3 +++ internal/pool/pool_test.go | 41 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/internal/pool/pool.go b/internal/pool/pool.go index 4a4c157..da0d1d5 100644 --- a/internal/pool/pool.go +++ b/internal/pool/pool.go @@ -141,6 +141,9 @@ func Release(poolDir, worktreePath string) error { } for i := range state.Worktrees { if state.Worktrees[i].Path == worktreePath { + if state.Worktrees[i].Destroying { + return fmt.Errorf("worktree %s is being destroyed", worktreePath) + } state.Worktrees[i].OwnerPID = 0 state.Worktrees[i].OwnerStartedAt = 0 break diff --git a/internal/pool/pool_test.go b/internal/pool/pool_test.go index 9619d0a..0290d21 100644 --- a/internal/pool/pool_test.go +++ b/internal/pool/pool_test.go @@ -483,6 +483,47 @@ func TestDestroyAll_NonForceRejectsLiveDestroyingWorktree(t *testing.T) { } } +func TestRelease_RejectsDestroyingWorktree(t *testing.T) { + repoDir, poolDir := setupRepo(t) + + wtPath, err := Acquire(repoDir, poolDir, 4, nil) + if err != nil { + t.Fatalf("Acquire failed: %v", err) + } + if err := Release(poolDir, wtPath); err != nil { + t.Fatalf("Release failed: %v", err) + } + + state, err := ReadState(poolDir) + if err != nil { + t.Fatalf("ReadState failed: %v", err) + } + state.Worktrees[0].Destroying = true + if err := reserveOwner(&state.Worktrees[0]); err != nil { + t.Fatalf("reserveOwner failed: %v", err) + } + reserved := state.Worktrees[0] + if err := WriteState(poolDir, state); err != nil { + t.Fatalf("WriteState failed: %v", err) + } + + err = Release(poolDir, wtPath) + if err == nil { + t.Fatal("expected Release to reject destroying worktree") + } + if !strings.Contains(err.Error(), "is being destroyed") { + t.Fatalf("expected destroying error, got %v", err) + } + + state, err = ReadState(poolDir) + if err != nil { + t.Fatalf("ReadState failed: %v", err) + } + if len(state.Worktrees) != 1 || state.Worktrees[0] != reserved { + t.Fatalf("expected destroy reservation to remain unchanged, got %#v", state.Worktrees) + } +} + func TestAcquireDuringHookProbe(t *testing.T) { if len(os.Args) < 5 { return From bcec4a79892e7bb1af0a949763830fa78769d2b3 Mon Sep 17 00:00:00 2001 From: kunchenguid Date: Thu, 14 May 2026 23:40:31 -0700 Subject: [PATCH 12/18] no-mistakes(review): Check destroy before release reset --- internal/pool/pool.go | 14 ++++++++++++++ internal/pool/pool_test.go | 7 +++++++ 2 files changed, 21 insertions(+) diff --git a/internal/pool/pool.go b/internal/pool/pool.go index da0d1d5..1b46d8f 100644 --- a/internal/pool/pool.go +++ b/internal/pool/pool.go @@ -131,6 +131,20 @@ func Release(poolDir, worktreePath string) error { if err != nil { return err } + if err := WithStateLock(poolDir, func() error { + state, err := ReadState(poolDir) + if err != nil { + return err + } + for _, wt := range state.Worktrees { + if wt.Path == worktreePath && wt.Destroying { + return fmt.Errorf("worktree %s is being destroyed", worktreePath) + } + } + return nil + }); err != nil { + return err + } if err := git.ResetWorktree(worktreePath, branch); err != nil { return err } diff --git a/internal/pool/pool_test.go b/internal/pool/pool_test.go index 0290d21..a7304c6 100644 --- a/internal/pool/pool_test.go +++ b/internal/pool/pool_test.go @@ -506,6 +506,10 @@ func TestRelease_RejectsDestroyingWorktree(t *testing.T) { if err := WriteState(poolDir, state); err != nil { t.Fatalf("WriteState failed: %v", err) } + dirtyPath := filepath.Join(wtPath, "pre-destroy-work.txt") + if err := os.WriteFile(dirtyPath, []byte("keep me"), 0o644); err != nil { + t.Fatalf("WriteFile failed: %v", err) + } err = Release(poolDir, wtPath) if err == nil { @@ -522,6 +526,9 @@ func TestRelease_RejectsDestroyingWorktree(t *testing.T) { if len(state.Worktrees) != 1 || state.Worktrees[0] != reserved { t.Fatalf("expected destroy reservation to remain unchanged, got %#v", state.Worktrees) } + if _, err := os.Stat(dirtyPath); err != nil { + t.Fatalf("expected Release to leave destroying worktree untouched: %v", err) + } } func TestAcquireDuringHookProbe(t *testing.T) { From 671ccfe17f3680a55416066bd1341e45f4d01c1c Mon Sep 17 00:00:00 2001 From: kunchenguid Date: Thu, 14 May 2026 23:45:10 -0700 Subject: [PATCH 13/18] no-mistakes(document): Update docs for hook reservations --- AGENTS.md | 13 ++++++++++--- README.md | 2 +- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 9577fdd..dee8ccd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -35,8 +35,8 @@ make test - No daemon — all operations are inline CLI commands - Detached HEAD worktrees reset to whichever of local or origin default branch is further ahead (prefers origin on divergence) -- In-use detection is runtime-only (process scanning), never persisted -- State file only tracks pool membership, not usage status +- In-use detection uses process scanning plus short-lived persisted owner reservations for lifecycle operations +- State file tracks pool membership and temporary owner/destroy reservations, not long-term usage status - Git operations shell out to `git` (go-git has incomplete worktree support) - Self-healing: stale state entries are auto-removed @@ -53,8 +53,15 @@ This project targets Linux, macOS, and Windows. All new code **must** work on Wi ## Config -Place `treehouse.toml` in repo root or `~/.config/treehouse/config.toml`: +Place repo-safe settings in repo root `treehouse.toml` or user-level `~/.config/treehouse/config.toml`: ```toml max_trees = 16 + +# User-level config only: +[hooks] +post_create = ["./scripts/setup-venv.sh"] +pre_destroy = ["./scripts/teardown.sh"] ``` + +Hooks are ignored in repo-level config for safety. diff --git a/README.md b/README.md index e187ca3..188105b 100644 --- a/README.md +++ b/README.md @@ -130,7 +130,7 @@ Treehouse manages a **pool of git worktrees** per repository, stored under `~/.t - **Detached HEAD** — worktrees use detached HEAD mode, reset to whichever of the local or remote default branch is further ahead, avoiding branch name conflicts entirely. - **No daemon** — all operations are inline CLI commands. No background processes, no state to get corrupted. -- **In-use detection** — treehouse scans running processes to determine which worktrees are in-use. Usage state is never persisted, so it's always accurate. +- **In-use detection** — treehouse scans running processes and short-lived owner reservations to determine which worktrees are in-use. Reservations are persisted only while `get` and `destroy` lifecycle work is running. ## CLI Reference From b0d707525c7c74aaf8e3a7d41356ddd736e8af0c Mon Sep 17 00:00:00 2001 From: kunchenguid Date: Thu, 14 May 2026 23:48:15 -0700 Subject: [PATCH 14/18] no-mistakes(document): Document internal hooks package --- AGENTS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/AGENTS.md b/AGENTS.md index dee8ccd..ad0bd8e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -9,6 +9,7 @@ Treehouse is a Go CLI tool that manages a pool of git worktrees for parallel AI - `main.go` — entry point, calls `cmd.Execute()` - `cmd/` — CLI commands (cobra): `get`, `return`, `status`, `destroy` - `internal/config/` — config file loading (`treehouse.toml`) +- `internal/hooks/` — user-configured lifecycle hook command execution - `internal/pool/` — pool manager (acquire, release, list, destroy) + state file - `internal/git/` — git operations (shells out to `git` binary) - `internal/process/` — in-use detection and lingering process termination for worktrees From f69b92c1ef7d79db9162ce28ae20aaaed99b4979 Mon Sep 17 00:00:00 2001 From: kunchenguid Date: Thu, 14 May 2026 23:53:24 -0700 Subject: [PATCH 15/18] no-mistakes: apply CI fixes --- internal/pool/pool.go | 2 +- internal/pool/pool_test.go | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/internal/pool/pool.go b/internal/pool/pool.go index 1b46d8f..02db948 100644 --- a/internal/pool/pool.go +++ b/internal/pool/pool.go @@ -123,7 +123,7 @@ func Acquire(repoRoot, poolDir string, poolSize int, postCreate []string) (strin } func Release(poolDir, worktreePath string) error { - repoRoot, err := git.FindRepoRoot() + repoRoot, err := git.FindRepoRootFrom(worktreePath) if err != nil { return err } diff --git a/internal/pool/pool_test.go b/internal/pool/pool_test.go index a7304c6..596e1bb 100644 --- a/internal/pool/pool_test.go +++ b/internal/pool/pool_test.go @@ -129,6 +129,32 @@ func TestAcquire_DoesNotReuseWorktreeReservedByPostCreateHook(t *testing.T) { } } +func TestRelease_DoesNotDependOnCurrentWorkingDirectory(t *testing.T) { + repoDir, poolDir := setupRepo(t) + + wtPath, err := Acquire(repoDir, poolDir, 4, nil) + if err != nil { + t.Fatalf("Acquire failed: %v", err) + } + + originalCwd, err := os.Getwd() + if err != nil { + t.Fatalf("Getwd failed: %v", err) + } + t.Cleanup(func() { + if err := os.Chdir(originalCwd); err != nil { + t.Fatalf("restore cwd failed: %v", err) + } + }) + if err := os.Chdir(t.TempDir()); err != nil { + t.Fatalf("Chdir failed: %v", err) + } + + if err := Release(poolDir, wtPath); err != nil { + t.Fatalf("Release failed: %v", err) + } +} + func TestList_RecoversDestroyingWorktreeWhenOwnerIsGone(t *testing.T) { repoDir, poolDir := setupRepo(t) From 421a3322cd728170b807fea7274e78f23e8f5c23 Mon Sep 17 00:00:00 2001 From: kunchenguid Date: Fri, 15 May 2026 00:02:27 -0700 Subject: [PATCH 16/18] no-mistakes: apply CI fixes --- internal/hooks/command_unix.go | 9 +++++++++ internal/hooks/command_windows.go | 17 +++++++++++++++++ internal/hooks/hooks.go | 19 ++++--------------- internal/hooks/hooks_test.go | 11 +++++++++++ 4 files changed, 41 insertions(+), 15 deletions(-) create mode 100644 internal/hooks/command_unix.go create mode 100644 internal/hooks/command_windows.go diff --git a/internal/hooks/command_unix.go b/internal/hooks/command_unix.go new file mode 100644 index 0000000..01f5792 --- /dev/null +++ b/internal/hooks/command_unix.go @@ -0,0 +1,9 @@ +//go:build !windows + +package hooks + +import "os/exec" + +func newHookCommand(command string) *exec.Cmd { + return exec.Command("/bin/sh", "-c", command) +} diff --git a/internal/hooks/command_windows.go b/internal/hooks/command_windows.go new file mode 100644 index 0000000..58e96a0 --- /dev/null +++ b/internal/hooks/command_windows.go @@ -0,0 +1,17 @@ +//go:build windows + +package hooks + +import ( + "os" + "os/exec" +) + +func newHookCommand(command string) *exec.Cmd { + shell := os.Getenv("COMSPEC") + if shell == "" { + shell = "cmd.exe" + } + + return exec.Command(shell, windowsShellArgs(command)...) +} diff --git a/internal/hooks/hooks.go b/internal/hooks/hooks.go index 911ef19..c028434 100644 --- a/internal/hooks/hooks.go +++ b/internal/hooks/hooks.go @@ -6,13 +6,11 @@ package hooks import ( "fmt" "io" - "os" "os/exec" - "runtime" ) // Run executes each command in commands sequentially in workDir. Each command -// is passed to the OS shell (/bin/sh -c on Unix, %COMSPEC% /c on Windows). +// is passed to the OS shell (/bin/sh -c on Unix, %COMSPEC% /d /s /c on Windows). // Stdout and stderr from the commands are streamed to the given writers. // Failures are logged to stderr and do not stop subsequent commands. func Run(commands []string, workDir string, stdout, stderr io.Writer) { @@ -22,9 +20,7 @@ func Run(commands []string, workDir string, stdout, stderr io.Writer) { } func runOne(command, workDir string, stdout, stderr io.Writer) { - shell, flag := shellAndFlag() - - cmd := exec.Command(shell, flag, command) + cmd := newHookCommand(command) cmd.Dir = workDir cmd.Stdout = stdout cmd.Stderr = stderr @@ -38,13 +34,6 @@ func runOne(command, workDir string, stdout, stderr io.Writer) { } } -func shellAndFlag() (string, string) { - if runtime.GOOS == "windows" { - shell := os.Getenv("COMSPEC") - if shell == "" { - shell = "cmd.exe" - } - return shell, "/c" - } - return "/bin/sh", "-c" +func windowsShellArgs(command string) []string { + return []string{"/d", "/s", "/c", command} } diff --git a/internal/hooks/hooks_test.go b/internal/hooks/hooks_test.go index e6cddc5..998e53c 100644 --- a/internal/hooks/hooks_test.go +++ b/internal/hooks/hooks_test.go @@ -93,3 +93,14 @@ func TestRun_RunsInGivenDir(t *testing.T) { t.Fatalf("expected hook to run in %s: %v\nstderr: %s", dir, err, stderr.String()) } } + +func TestWindowsShellArgsPassHookCommandAsSingleArgument(t *testing.T) { + command := `echo hi > "C:\Temp\ran.txt"` + + got := windowsShellArgs(command) + want := []string{"/d", "/s", "/c", command} + + if strings.Join(got, "\x00") != strings.Join(want, "\x00") { + t.Fatalf("windows shell args mismatch\nwant: %#v\n got: %#v", want, got) + } +} From b4619f402d7f60d000aa36d3f893d82d76e72b4c Mon Sep 17 00:00:00 2001 From: kunchenguid Date: Fri, 15 May 2026 00:06:09 -0700 Subject: [PATCH 17/18] no-mistakes: apply CI fixes --- internal/hooks/hooks.go | 4 ++-- internal/hooks/hooks_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/hooks/hooks.go b/internal/hooks/hooks.go index c028434..41034c8 100644 --- a/internal/hooks/hooks.go +++ b/internal/hooks/hooks.go @@ -10,7 +10,7 @@ import ( ) // Run executes each command in commands sequentially in workDir. Each command -// is passed to the OS shell (/bin/sh -c on Unix, %COMSPEC% /d /s /c on Windows). +// is passed to the OS shell (/bin/sh -c on Unix, %COMSPEC% /d /c on Windows). // Stdout and stderr from the commands are streamed to the given writers. // Failures are logged to stderr and do not stop subsequent commands. func Run(commands []string, workDir string, stdout, stderr io.Writer) { @@ -35,5 +35,5 @@ func runOne(command, workDir string, stdout, stderr io.Writer) { } func windowsShellArgs(command string) []string { - return []string{"/d", "/s", "/c", command} + return []string{"/d", "/c", command} } diff --git a/internal/hooks/hooks_test.go b/internal/hooks/hooks_test.go index 998e53c..5862882 100644 --- a/internal/hooks/hooks_test.go +++ b/internal/hooks/hooks_test.go @@ -94,11 +94,11 @@ func TestRun_RunsInGivenDir(t *testing.T) { } } -func TestWindowsShellArgsPassHookCommandAsSingleArgument(t *testing.T) { +func TestWindowsShellArgsPassQuotedHookCommandWithoutQuoteStripping(t *testing.T) { command := `echo hi > "C:\Temp\ran.txt"` got := windowsShellArgs(command) - want := []string{"/d", "/s", "/c", command} + want := []string{"/d", "/c", command} if strings.Join(got, "\x00") != strings.Join(want, "\x00") { t.Fatalf("windows shell args mismatch\nwant: %#v\n got: %#v", want, got) From d456b2e39e6d6522433416cd445501ce88fd347b Mon Sep 17 00:00:00 2001 From: kunchenguid Date: Fri, 15 May 2026 00:10:59 -0700 Subject: [PATCH 18/18] no-mistakes: apply CI fixes --- internal/hooks/command_windows.go | 5 ++++- internal/hooks/hooks.go | 6 +++--- internal/hooks/hooks_test.go | 11 ++++++----- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/internal/hooks/command_windows.go b/internal/hooks/command_windows.go index 58e96a0..e31afa2 100644 --- a/internal/hooks/command_windows.go +++ b/internal/hooks/command_windows.go @@ -5,6 +5,7 @@ package hooks import ( "os" "os/exec" + "syscall" ) func newHookCommand(command string) *exec.Cmd { @@ -13,5 +14,7 @@ func newHookCommand(command string) *exec.Cmd { shell = "cmd.exe" } - return exec.Command(shell, windowsShellArgs(command)...) + cmd := exec.Command(shell) + cmd.SysProcAttr = &syscall.SysProcAttr{CmdLine: windowsShellCommandLine(shell, command)} + return cmd } diff --git a/internal/hooks/hooks.go b/internal/hooks/hooks.go index 41034c8..7cbf7c9 100644 --- a/internal/hooks/hooks.go +++ b/internal/hooks/hooks.go @@ -10,7 +10,7 @@ import ( ) // Run executes each command in commands sequentially in workDir. Each command -// is passed to the OS shell (/bin/sh -c on Unix, %COMSPEC% /d /c on Windows). +// is passed to the OS shell (/bin/sh -c on Unix, %COMSPEC% /d /s /c on Windows). // Stdout and stderr from the commands are streamed to the given writers. // Failures are logged to stderr and do not stop subsequent commands. func Run(commands []string, workDir string, stdout, stderr io.Writer) { @@ -34,6 +34,6 @@ func runOne(command, workDir string, stdout, stderr io.Writer) { } } -func windowsShellArgs(command string) []string { - return []string{"/d", "/c", command} +func windowsShellCommandLine(shell, command string) string { + return `"` + shell + `" /d /s /c "` + command + `"` } diff --git a/internal/hooks/hooks_test.go b/internal/hooks/hooks_test.go index 5862882..b1033d8 100644 --- a/internal/hooks/hooks_test.go +++ b/internal/hooks/hooks_test.go @@ -94,13 +94,14 @@ func TestRun_RunsInGivenDir(t *testing.T) { } } -func TestWindowsShellArgsPassQuotedHookCommandWithoutQuoteStripping(t *testing.T) { +func TestWindowsShellCommandLineWrapsQuotedHookCommandForCmd(t *testing.T) { + shell := `C:\Windows\System32\cmd.exe` command := `echo hi > "C:\Temp\ran.txt"` - got := windowsShellArgs(command) - want := []string{"/d", "/c", command} + got := windowsShellCommandLine(shell, command) + want := `"C:\Windows\System32\cmd.exe" /d /s /c "echo hi > "C:\Temp\ran.txt""` - if strings.Join(got, "\x00") != strings.Join(want, "\x00") { - t.Fatalf("windows shell args mismatch\nwant: %#v\n got: %#v", want, got) + if got != want { + t.Fatalf("windows shell command line mismatch\nwant: %q\n got: %q", want, got) } }