diff --git a/apps/backend/internal/backendapp/main.go b/apps/backend/internal/backendapp/main.go index 45a991bf33..57a2b98e43 100644 --- a/apps/backend/internal/backendapp/main.go +++ b/apps/backend/internal/backendapp/main.go @@ -598,7 +598,7 @@ func startAgentInfrastructure( // ============================================ log.Info("Initializing Worktree Manager...") - worktreeMgr, _, worktreeCleanup, err := provideWorktreeManager(dbPool, cfg, log, lifecycleMgr, services.Task) + worktreeMgr, worktreeCleanup, err := provideWorktreeManager(dbPool, cfg, log, lifecycleMgr, services.Task) if err != nil { log.Error("Failed to initialize worktree manager", zap.Error(err)) return false diff --git a/apps/backend/internal/backendapp/worktree.go b/apps/backend/internal/backendapp/worktree.go index 45857c6f00..a4b38cb12b 100644 --- a/apps/backend/internal/backendapp/worktree.go +++ b/apps/backend/internal/backendapp/worktree.go @@ -66,10 +66,10 @@ func (a *bootMsgAdapter) UpdateMessage(ctx context.Context, message *models.Mess return a.svc.UpdateMessage(ctx, message) } -func provideWorktreeManager(dbPool *db.Pool, cfg *config.Config, log *logger.Logger, lifecycleMgr *lifecycle.Manager, taskSvc *taskservice.Service) (*worktree.Manager, *worktree.Recreator, func() error, error) { +func provideWorktreeManager(dbPool *db.Pool, cfg *config.Config, log *logger.Logger, lifecycleMgr *lifecycle.Manager, taskSvc *taskservice.Service) (*worktree.Manager, func() error, error) { manager, cleanup, err := worktree.Provide(dbPool.Writer(), dbPool.Reader(), cfg, log) if err != nil { - return nil, nil, nil, err + return nil, nil, err } if lifecycleMgr != nil { lifecycleMgr.SetWorktreeManager(manager) @@ -97,10 +97,7 @@ func provideWorktreeManager(dbPool *db.Pool, cfg *config.Config, log *logger.Log manager.SetScriptMessageHandler(scriptHandler) manager.SetRepositoryProvider(repoAdapter) - // Create recreator for orchestrator to use during session resume - recreator := worktree.NewRecreator(manager) - - return manager, recreator, cleanup, nil + return manager, cleanup, nil } // environmentDestroyerAdapter implements taskservice.EnvironmentDestroyer by diff --git a/apps/backend/internal/worktree/recreator.go b/apps/backend/internal/worktree/recreator.go deleted file mode 100644 index 5112e60fcc..0000000000 --- a/apps/backend/internal/worktree/recreator.go +++ /dev/null @@ -1,111 +0,0 @@ -package worktree - -import ( - "context" - "fmt" - - "go.uber.org/zap" -) - -// RecreateRequest contains parameters for recreating a worktree. -type RecreateRequest struct { - SessionID string - TaskID string - TaskTitle string - RepositoryID string - BaseBranch string - WorktreeID string -} - -// RecreateResult contains the result of worktree recreation. -type RecreateResult struct { - WorktreePath string - WorktreeBranch string -} - -// Recreator provides worktree recreation capability for the orchestrator. -// It wraps the Manager and exposes a simplified interface for recreating -// worktrees when the directory is missing but the database record exists. -type Recreator struct { - manager *Manager -} - -// NewRecreator creates a new Recreator that wraps the given Manager. -func NewRecreator(mgr *Manager) *Recreator { - return &Recreator{manager: mgr} -} - -// Recreate attempts to recreate a worktree when its directory is missing. -// It looks up the existing worktree record by ID or session ID to get the -// stored repository path, then uses the Manager's Create method which -// handles recreation when a worktree record exists but directory is invalid. -func (r *Recreator) Recreate(ctx context.Context, req RecreateRequest) (*RecreateResult, error) { - if r.manager == nil { - return nil, fmt.Errorf("worktree manager not available") - } - - // Look up existing worktree to get stored repository path - var existingWt *Worktree - var err error - - // Try by worktree ID first - if req.WorktreeID != "" { - existingWt, err = r.manager.GetByID(ctx, req.WorktreeID) - if err != nil && err != ErrWorktreeNotFound { - r.manager.logger.Debug("failed to get worktree by ID", - zap.String("worktree_id", req.WorktreeID), - zap.Error(err)) - } - } - - // Try by session ID if not found - if existingWt == nil && req.SessionID != "" { - existingWt, err = r.manager.GetBySessionID(ctx, req.SessionID) - if err != nil && err != ErrWorktreeNotFound { - r.manager.logger.Debug("failed to get worktree by session ID", - zap.String("session_id", req.SessionID), - zap.Error(err)) - } - } - - if existingWt == nil { - return nil, fmt.Errorf("worktree record not found for recreation") - } - - repoPath := existingWt.RepositoryPath - baseBranch := req.BaseBranch - if baseBranch == "" { - baseBranch = existingWt.BaseBranch - } - - r.manager.logger.Debug("found existing worktree record for recreation", - zap.String("worktree_id", existingWt.ID), - zap.String("repository_path", repoPath), - zap.String("base_branch", baseBranch)) - - if repoPath == "" { - return nil, fmt.Errorf("repository path not found in existing worktree record") - } - - // Use the manager's Create method which handles recreation internally - // when a worktree record exists but the directory is invalid (manager.go:167-173) - createReq := CreateRequest{ - SessionID: req.SessionID, - TaskID: req.TaskID, - TaskTitle: req.TaskTitle, - RepositoryID: req.RepositoryID, - RepositoryPath: repoPath, - BaseBranch: baseBranch, - WorktreeID: req.WorktreeID, - } - - wt, err := r.manager.Create(ctx, createReq) - if err != nil { - return nil, fmt.Errorf("failed to recreate worktree: %w", err) - } - - return &RecreateResult{ - WorktreePath: wt.Path, - WorktreeBranch: wt.Branch, - }, nil -} diff --git a/apps/backend/internal/worktree/recreator_test.go b/apps/backend/internal/worktree/recreator_test.go deleted file mode 100644 index 933348f5f9..0000000000 --- a/apps/backend/internal/worktree/recreator_test.go +++ /dev/null @@ -1,225 +0,0 @@ -package worktree - -import ( - "context" - "testing" -) - -func TestNewRecreator(t *testing.T) { - cfg := newTestConfig(t) - log := newTestLogger() - store := newMockStore() - - mgr, err := NewManager(cfg, store, log) - if err != nil { - t.Fatalf("NewManager failed: %v", err) - } - - recreator := NewRecreator(mgr) - if recreator == nil { - t.Fatal("expected non-nil recreator") - } else if recreator.manager != mgr { - t.Error("recreator manager not set correctly") - } -} - -func TestRecreator_Recreate_NilManager(t *testing.T) { - recreator := &Recreator{manager: nil} - - _, err := recreator.Recreate(context.Background(), RecreateRequest{ - SessionID: "test-session", - WorktreeID: "test-worktree", - }) - - if err == nil { - t.Fatal("expected error for nil manager") - } - if err.Error() != "worktree manager not available" { - t.Errorf("unexpected error message: %v", err) - } -} - -func TestRecreator_Recreate_WorktreeNotFound(t *testing.T) { - cfg := newTestConfig(t) - log := newTestLogger() - store := newMockStore() - - mgr, err := NewManager(cfg, store, log) - if err != nil { - t.Fatalf("NewManager failed: %v", err) - } - - recreator := NewRecreator(mgr) - - // Try to recreate with non-existent worktree - _, err = recreator.Recreate(context.Background(), RecreateRequest{ - SessionID: "non-existent-session", - WorktreeID: "non-existent-worktree", - }) - - if err == nil { - t.Fatal("expected error for non-existent worktree") - } - if err.Error() != "worktree record not found for recreation" { - t.Errorf("unexpected error message: %v", err) - } -} - -func TestRecreator_Recreate_MissingRepositoryPath(t *testing.T) { - cfg := newTestConfig(t) - log := newTestLogger() - store := newMockStore() - - mgr, err := NewManager(cfg, store, log) - if err != nil { - t.Fatalf("NewManager failed: %v", err) - } - - // Create worktree record without repository path - wt := &Worktree{ - ID: "wt-001", - SessionID: "test-session", - TaskID: "test-task", - RepositoryPath: "", // empty - should cause error - Path: "/some/path", - Status: StatusActive, - } - if err := store.CreateWorktree(context.Background(), wt); err != nil { - t.Fatalf("failed to create worktree: %v", err) - } - - recreator := NewRecreator(mgr) - - _, err = recreator.Recreate(context.Background(), RecreateRequest{ - SessionID: "test-session", - WorktreeID: "wt-001", - }) - - if err == nil { - t.Fatal("expected error for missing repository path") - } - if err.Error() != "repository path not found in existing worktree record" { - t.Errorf("unexpected error message: %v", err) - } -} - -func TestRecreator_Recreate_FindsByWorktreeID(t *testing.T) { - cfg := newTestConfig(t) - log := newTestLogger() - store := newMockStore() - - mgr, err := NewManager(cfg, store, log) - if err != nil { - t.Fatalf("NewManager failed: %v", err) - } - - // Create worktree record - wt := &Worktree{ - ID: "wt-002", - SessionID: "session-abc", - TaskID: "task-123", - RepositoryPath: "/path/to/repo", - BaseBranch: "main", - Path: "/some/path", - Status: StatusActive, - } - if err := store.CreateWorktree(context.Background(), wt); err != nil { - t.Fatalf("failed to create worktree: %v", err) - } - - recreator := NewRecreator(mgr) - - // Try to recreate using worktree ID (with different session ID) - // This tests that GetByID is tried first - _, err = recreator.Recreate(context.Background(), RecreateRequest{ - SessionID: "different-session", - WorktreeID: "wt-002", - TaskID: "task-123", - }) - - // Will fail because actual creation fails (no git repo), but proves lookup worked - if err == nil || err.Error() == "worktree record not found for recreation" { - t.Error("expected recreation to find worktree by ID") - } -} - -func TestRecreator_Recreate_FindsBySessionID(t *testing.T) { - cfg := newTestConfig(t) - log := newTestLogger() - store := newMockStore() - - mgr, err := NewManager(cfg, store, log) - if err != nil { - t.Fatalf("NewManager failed: %v", err) - } - - // Create worktree record - wt := &Worktree{ - ID: "wt-003", - SessionID: "session-xyz", - TaskID: "task-456", - RepositoryPath: "/path/to/repo", - BaseBranch: "develop", - Path: "/some/path", - Status: StatusActive, - } - if err := store.CreateWorktree(context.Background(), wt); err != nil { - t.Fatalf("failed to create worktree: %v", err) - } - - recreator := NewRecreator(mgr) - - // Try to recreate using only session ID (no worktree ID) - // This tests that GetBySessionID is used as fallback - _, err = recreator.Recreate(context.Background(), RecreateRequest{ - SessionID: "session-xyz", - WorktreeID: "", // empty - forces lookup by session ID - TaskID: "task-456", - }) - - // Will fail because actual creation fails (no git repo), but proves lookup worked - if err == nil || err.Error() == "worktree record not found for recreation" { - t.Error("expected recreation to find worktree by session ID") - } -} - -func TestRecreator_Recreate_UsesRequestBaseBranch(t *testing.T) { - cfg := newTestConfig(t) - log := newTestLogger() - store := newMockStore() - - mgr, err := NewManager(cfg, store, log) - if err != nil { - t.Fatalf("NewManager failed: %v", err) - } - - // Create worktree record with one base branch - wt := &Worktree{ - ID: "wt-004", - SessionID: "session-branch-test", - TaskID: "task-789", - RepositoryPath: "/path/to/repo", - BaseBranch: "main", // stored branch - Path: "/some/path", - Status: StatusActive, - } - if err := store.CreateWorktree(context.Background(), wt); err != nil { - t.Fatalf("failed to create worktree: %v", err) - } - - recreator := NewRecreator(mgr) - - // Request with a different base branch - should use request's branch - _, err = recreator.Recreate(context.Background(), RecreateRequest{ - SessionID: "session-branch-test", - WorktreeID: "wt-004", - BaseBranch: "feature-branch", // different from stored - TaskID: "task-789", - }) - - // Will fail because actual creation fails, but the test verifies the logic path - // A real integration test would verify the branch is used correctly - if err == nil { - t.Log("recreation succeeded - integration test would verify branch usage") - } -}