Fix release alignment for submodules

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 <path>`
 
@@ -197,15 +197,16 @@ Behavior:
    that database before continuing.
 2. `git fetch origin`.
 3. `git reset --hard origin/<default_branch>`.
-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/<default_branch>` 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,

SKILL.md

@@ -54,10 +54,11 @@ worktree-manager release <worktree-path>
 The tool will:
 - fetch `origin`,
 - reset the worktree to `origin/<default-branch>`,
+- 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

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
 

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 {

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,34 +265,30 @@ func (m *Manager) Release(worktreePath string) error {
 	}
 
 	// Reset to origin/<default>.
-	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)
 	}
 
 	// 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

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)