diff --git a/README.md b/README.md index 9a4c51c..528dbc7 100644 --- a/README.md +++ b/README.md @@ -69,7 +69,7 @@ no CGO or system SQLite is required). ### `-v, --version` -Prints the worktree-manager version (`2.1.0`). +Prints the worktree-manager version (`2.1.1`). ### `-d, --database ` @@ -197,15 +197,16 @@ Behavior: that database before continuing. 2. `git fetch origin`. 3. `git reset --hard origin/`. -4. Reset and clean initialized submodules, then run `git clean -xfd` in the - worktree. This removes manager state accidentally created inside a +4. Initialize and update submodules to the commits recorded by the + superproject, then reset and clean them. Run `git clean -xfd` in the + worktree as well. This removes manager state accidentally created inside a submodule. 5. Detach the worktree at the refreshed default-branch commit. A detached checkout is required because Git does not allow the default branch to be checked out by both the primary worktree and a pooled worktree. -6. Delete the released local branch and clear its ownership. -7. Verify that the detached `HEAD` matches the successfully fetched +6. Verify that the detached `HEAD` matches the successfully fetched `origin/` commit and that the working directory is clean. +7. Delete the released local branch and clear its ownership. 8. Mark `FREE`. After a successful release, the pool worktree is guaranteed to be a clean, diff --git a/SKILL.md b/SKILL.md index de574fc..837a279 100644 --- a/SKILL.md +++ b/SKILL.md @@ -54,10 +54,11 @@ worktree-manager release The tool will: - fetch `origin`, - reset the worktree to `origin/`, +- initialize, update, reset, and clean submodules, - `git clean -xfd` (remove untracked files), - detach at the refreshed default-branch commit, -- delete the released task branch and clear its ownership, - verify that `HEAD` matches the fetched default branch and the worktree is clean, +- delete the released task branch and clear its ownership, - mark it `FREE`. After a successful release, the pool worktree is a clean detached snapshot of diff --git a/cmd/worktree-manager/main.go b/cmd/worktree-manager/main.go index 90f4b3f..96d2df6 100644 --- a/cmd/worktree-manager/main.go +++ b/cmd/worktree-manager/main.go @@ -10,7 +10,7 @@ import ( "github.com/bejo-dev/worktree-manager/internal/manager" ) -const version = "2.1.0" +const version = "2.1.1" const usage = `worktree-manager - manage a reusable pool of git worktrees diff --git a/internal/gitops/gitops.go b/internal/gitops/gitops.go index 9bae9f5..5e533e9 100644 --- a/internal/gitops/gitops.go +++ b/internal/gitops/gitops.go @@ -198,6 +198,13 @@ func (r *Repo) HardReset(path, ref string) error { return nil } +// SyncSubmodules initializes submodules and checks them out at the commits +// recorded by the superproject. +func (r *Repo) SyncSubmodules(path string) error { + _, err := runGit(path, "submodule", "update", "--init", "--recursive") + return err +} + // Clean removes untracked files and directories from the worktree at path. func (r *Repo) Clean(path string) error { if _, err := runGit(path, "submodule", "foreach", "--recursive", "git", "reset", "--hard"); err != nil { diff --git a/internal/manager/manager.go b/internal/manager/manager.go index b906778..697fa5f 100644 --- a/internal/manager/manager.go +++ b/internal/manager/manager.go @@ -175,7 +175,7 @@ func (m *Manager) Acquire(repoPath string, branchName string) (*AcquireResult, e // Release resets the worktree at the given path back to the default branch and // marks it FREE. -func (m *Manager) Release(worktreePath string) error { +func (m *Manager) Release(worktreePath string) (releaseErr error) { abs, err := filepath.Abs(worktreePath) if err != nil { return fmt.Errorf("abs path: %w", err) @@ -236,6 +236,26 @@ func (m *Manager) Release(worktreePath string) error { return fmt.Errorf("read worktree branch: %w", err) } + target := "origin/" + defaultBranch + if !gr.HasRemote() { + target = defaultBranch + } + checkoutAttempted := false + defer func() { + if releaseErr == nil || !checkoutAttempted { + return + } + var restoreErr error + if releasedBranch == "" { + restoreErr = gr.CheckoutDetached(abs, target) + } else { + restoreErr = gr.CheckoutNewBranch(abs, releasedBranch) + } + if restoreErr != nil { + m.logf("warning: could not restore worktree checkout after release failure: %v", restoreErr) + } + }() + // Fetch origin before resetting so a released worktree is never returned to // the pool on a stale default-branch commit. if gr.HasRemote() { @@ -245,16 +265,17 @@ func (m *Manager) Release(worktreePath string) error { } // Reset to origin/. - target := "origin/" + defaultBranch - if !gr.HasRemote() { - target = defaultBranch - } if err := gr.HardReset(abs, target); err != nil { m.markBroken(wt.ID) return fmt.Errorf("reset worktree: %w", err) } + if err := gr.SyncSubmodules(abs); err != nil { + m.markBroken(wt.ID) + return fmt.Errorf("sync submodules: %w", err) + } - // Clean untracked files. + // Clean untracked files. Clean also resets initialized submodules as a + // defensive pass after they have been aligned with the superproject. if err := gr.Clean(abs); err != nil { m.markBroken(wt.ID) return fmt.Errorf("clean worktree: %w", err) @@ -262,17 +283,12 @@ func (m *Manager) Release(worktreePath string) error { // A default branch is usually checked out in the primary worktree, so Git // will not let a pool worktree check it out too. Detach at the refreshed - // default-branch commit, then delete the branch that was released. + // default-branch commit and validate it before deleting the released branch. + checkoutAttempted = true if err := gr.CheckoutDetached(abs, target); err != nil { m.markBroken(wt.ID) return fmt.Errorf("detach worktree at default branch: %w", err) } - if releasedBranch != "" && releasedBranch != defaultBranch { - if err := gr.DeleteBranch(releasedBranch); err != nil { - m.markBroken(wt.ID) - return fmt.Errorf("delete released branch %q: %w", releasedBranch, err) - } - } baseCommit, err := gr.RevParse(target) if err != nil { @@ -298,6 +314,16 @@ func (m *Manager) Release(worktreePath string) error { return errors.New("released worktree is not clean") } + // Delete the released branch only after all final worktree validation has + // succeeded. The deferred restore puts the branch back if a later step + // fails. + if releasedBranch != "" && releasedBranch != defaultBranch { + if err := gr.DeleteBranch(releasedBranch); err != nil { + m.markBroken(wt.ID) + return fmt.Errorf("delete released branch %q: %w", releasedBranch, err) + } + } + // Mark FREE atomically. tx, err := m.db.BeginTx() if err != nil { @@ -545,6 +571,13 @@ func (m *Manager) Doctor() (*DoctorResult, error) { continue } + // Released worktrees are intentionally detached at the default + // branch. A failed release can also leave a BROKEN worktree + // detached, so there is no branch to rename when actual is empty. + if actual == "" { + continue + } + desired := wt.BranchName owner := wt.TaskID // Before the breaking change, an allocated worktree recorded the @@ -561,11 +594,6 @@ func (m *Manager) Doctor() (*DoctorResult, error) { } owner = desired } - // Released worktrees are intentionally detached at the default - // branch so the primary worktree can keep that branch checked out. - if wt.Status == db.StatusFree && actual == "" { - continue - } if desired == "" { continue diff --git a/internal/manager/manager_test.go b/internal/manager/manager_test.go index f3fdfa0..73ffe63 100644 --- a/internal/manager/manager_test.go +++ b/internal/manager/manager_test.go @@ -49,6 +49,41 @@ func setupRepo(t *testing.T) string { return work } +// setupRepoWithSubmodule creates a superproject whose initial commit records +// the first commit of a local submodule. +func setupRepoWithSubmodule(t *testing.T) string { + t.Helper() + dir := t.TempDir() + submoduleBare := filepath.Join(dir, "submodule.git") + run(t, dir, "git", "init", "--bare", "-b", "main", submoduleBare) + run(t, dir, "git", "clone", submoduleBare, "submodule-seed") + submoduleSeed := filepath.Join(dir, "submodule-seed") + run(t, submoduleSeed, "git", "config", "user.email", "t@t") + run(t, submoduleSeed, "git", "config", "user.name", "test") + writeFile(t, submoduleSeed, "README.md", "submodule A\n") + run(t, submoduleSeed, "git", "add", ".") + run(t, submoduleSeed, "git", "commit", "-m", "submodule A") + run(t, submoduleSeed, "git", "push", "origin", "main") + + superBare := filepath.Join(dir, "origin.git") + run(t, dir, "git", "init", "--bare", "-b", "main", superBare) + run(t, dir, "git", "clone", superBare, "work") + work := filepath.Join(dir, "work") + run(t, work, "git", "config", "user.email", "t@t") + run(t, work, "git", "config", "user.name", "test") + // Git blocks the file protocol by default. Keep this test's local remote + // usable by the production submodule commands without changing global Git + // configuration. + run(t, work, "git", "config", "protocol.file.allow", "always") + run(t, work, "git", "-c", "protocol.file.allow=always", "submodule", "add", submoduleBare, "core") + run(t, work, "git", "commit", "-m", "add submodule") + run(t, work, "git", "push", "origin", "main") + if r, err := filepath.EvalSymlinks(work); err == nil { + return r + } + return work +} + func newManagerDB(t *testing.T) *db.DB { t.Helper() d, err := db.Open(filepath.Join(t.TempDir(), "state.db")) @@ -351,6 +386,59 @@ func TestReleaseFetchesLatestDefaultBranch(t *testing.T) { } } +func TestReleaseAlignsSubmoduleWithSuperproject(t *testing.T) { + repo := setupRepoWithSubmodule(t) + d := newManagerDB(t) + m := newTestManager(t, d) + + result, err := m.Acquire(repo, "task-1") + if err != nil { + t.Fatalf("Acquire: %v", err) + } + run(t, result.WorktreePath, "git", "-c", "protocol.file.allow=always", "submodule", "update", "--init", "--recursive") + worktreeSubmodule := filepath.Join(result.WorktreePath, "core") + initialCommit := strings.TrimSpace(run(t, worktreeSubmodule, "git", "rev-parse", "HEAD")) + + dir := filepath.Dir(repo) + extra := filepath.Join(dir, "submodule-extra") + run(t, dir, "git", "clone", filepath.Join(dir, "submodule.git"), extra) + run(t, extra, "git", "config", "user.email", "t@t") + run(t, extra, "git", "config", "user.name", "test") + writeFile(t, extra, "README.md", "submodule B\n") + run(t, extra, "git", "add", ".") + run(t, extra, "git", "commit", "-m", "submodule B") + run(t, extra, "git", "push", "origin", "main") + run(t, result.WorktreePath, "git", "-c", "protocol.file.allow=always", "-C", "core", "fetch", "origin", "main") + + run(t, repo, "git", "-c", "protocol.file.allow=always", "-C", "core", "fetch", "origin", "main") + run(t, repo, "git", "-C", "core", "checkout", "--detach", "origin/main") + updatedCommit := strings.TrimSpace(run(t, repo, "git", "-C", "core", "rev-parse", "HEAD")) + if updatedCommit == initialCommit { + t.Fatal("expected submodule to advance to a new commit") + } + run(t, repo, "git", "add", "core") + run(t, repo, "git", "commit", "-m", "advance submodule") + run(t, repo, "git", "push", "origin", "main") + + if err := m.Release(result.WorktreePath); err != nil { + t.Fatalf("Release: %v", err) + } + gotCommit := strings.TrimSpace(run(t, worktreeSubmodule, "git", "rev-parse", "HEAD")) + if gotCommit != updatedCommit { + t.Fatalf("submodule is at %s, want superproject commit %s", gotCommit, updatedCommit) + } + if status := strings.TrimSpace(run(t, result.WorktreePath, "git", "status", "--porcelain", "--untracked-files=all", "--ignored")); status != "" { + t.Fatalf("released worktree is not clean:\n%s", status) + } + worktree, err := d.GetWorktreeByPath(result.WorktreePath) + if err != nil { + t.Fatal(err) + } + if worktree == nil || worktree.Status != db.StatusFree { + t.Fatalf("expected released worktree to be FREE, got %+v", worktree) + } +} + func TestAcquireFailsWhenFetchFails(t *testing.T) { repo := setupRepo(t) d := newManagerDB(t) @@ -417,6 +505,41 @@ func TestReleaseUnmanagedWorktreeFails(t *testing.T) { } } +func TestDoctorIgnoresDetachedBrokenWorktree(t *testing.T) { + repo := setupRepo(t) + d := newManagerDB(t) + m := newTestManager(t, d) + + result, err := m.Acquire(repo, "task-1") + if err != nil { + t.Fatalf("Acquire: %v", err) + } + run(t, result.WorktreePath, "git", "checkout", "--detach", "HEAD") + worktree, err := d.GetWorktreeByPath(result.WorktreePath) + if err != nil { + t.Fatal(err) + } + tx, err := d.BeginTx() + if err != nil { + t.Fatal(err) + } + if err := d.MarkBroken(tx, worktree.ID); err != nil { + _ = tx.Rollback() + t.Fatal(err) + } + if err := tx.Commit(); err != nil { + t.Fatal(err) + } + + report, err := m.Doctor() + if err != nil { + t.Fatalf("Doctor: %v", err) + } + if report.Checked != 1 || report.Repaired != 0 || len(report.Issues) != 0 { + t.Fatalf("unexpected doctor report: %+v", report) + } +} + func TestAcquireNoTaskID(t *testing.T) { repo := setupRepo(t) d := newManagerDB(t)