diff --git a/internal/infra/git/client.go b/internal/infra/git/client.go index 1949a15..36f39bd 100644 --- a/internal/infra/git/client.go +++ b/internal/infra/git/client.go @@ -98,32 +98,86 @@ func (c *Client) RemoveWorktree(worktreePath string) error { // UpdateBaseBranch updates the specified branch to the latest remote version func (c *Client) UpdateBaseBranch(branch string) error { if branch == "" { - return NewGitError("fetch", "", "branch name is required", nil) + return NewGitError("pull", "", "branch name is required", nil) } // Check if branch exists cmd := exec.Command("git", "-C", c.workDir, "rev-parse", "--verify", branch) if err := cmd.Run(); err != nil { - return NewGitError("fetch", branch, "branch not found", err) + return NewGitError("pull", branch, "branch not found", err) } // Check if remote exists cmd = exec.Command("git", "-C", c.workDir, "remote") remoteOutput, err := cmd.Output() if err != nil || len(strings.TrimSpace(string(remoteOutput))) == 0 { - // No remote configured, skip fetch (for local testing) + // No remote configured, skip pull (for local testing) return nil } - // Fetch latest changes - cmd = exec.Command("git", "-C", c.workDir, "fetch", "origin", fmt.Sprintf("%s:%s", branch, branch)) - _, err = cmd.CombinedOutput() + // Get current branch + cmd = exec.Command("git", "-C", c.workDir, "branch", "--show-current") + currentBranchOutput, err := cmd.Output() if err != nil { - // Try simple fetch without force update - cmd = exec.Command("git", "-C", c.workDir, "fetch") - if fetchErr := cmd.Run(); fetchErr != nil { - // Log error but don't fail (for testing environments) - return nil + return NewGitError("pull", branch, "failed to get current branch", err) + } + currentBranch := strings.TrimSpace(string(currentBranchOutput)) + + // If we're not on the target branch, switch to it + needSwitch := currentBranch != branch + if needSwitch { + // Check for uncommitted changes before switching + cmd = exec.Command("git", "-C", c.workDir, "status", "--porcelain") + statusOutput, err := cmd.Output() + if err != nil { + return NewGitError("pull", branch, "failed to check working tree status", err) + } + if len(strings.TrimSpace(string(statusOutput))) > 0 { + // Stash changes if any + cmd = exec.Command("git", "-C", c.workDir, "stash", "push", "-m", "soba: auto-stash before updating base branch") + if err := cmd.Run(); err != nil { + return NewGitError("pull", branch, "failed to stash changes", err) + } + defer func() { + // Pop stash after switching back + cmd := exec.Command("git", "-C", c.workDir, "stash", "pop") + cmd.Run() // Ignore error as stash might be empty + }() + } + + // Switch to target branch + cmd = exec.Command("git", "-C", c.workDir, "checkout", branch) + if err := cmd.Run(); err != nil { + return NewGitError("pull", branch, "failed to checkout branch", err) + } + } + + // Pull latest changes with fast-forward only + cmd = exec.Command("git", "-C", c.workDir, "pull", "--ff-only", "origin", branch) + output, err := cmd.CombinedOutput() + if err != nil { + // If pull failed, try to provide meaningful error + if strings.Contains(string(output), "not possible to fast-forward") { + if needSwitch { + // Switch back to original branch + cmd = exec.Command("git", "-C", c.workDir, "checkout", currentBranch) + cmd.Run() + } + return NewGitError("pull", branch, "cannot fast-forward, manual merge required", err) + } + // For other errors, still try to switch back if needed + if needSwitch { + cmd = exec.Command("git", "-C", c.workDir, "checkout", currentBranch) + cmd.Run() + } + return NewGitError("pull", branch, string(output), err) + } + + // Switch back to original branch if we switched + if needSwitch { + cmd = exec.Command("git", "-C", c.workDir, "checkout", currentBranch) + if err := cmd.Run(); err != nil { + return NewGitError("pull", branch, "failed to switch back to original branch", err) } } diff --git a/internal/infra/git/client_test.go b/internal/infra/git/client_test.go index ec8709e..7bce874 100644 --- a/internal/infra/git/client_test.go +++ b/internal/infra/git/client_test.go @@ -476,11 +476,47 @@ func TestClient_RemoveWorktree(t *testing.T) { func TestClient_UpdateBaseBranch(t *testing.T) { tests := []struct { - name string - branch string - setup func(t *testing.T, dir string) - wantErr bool + name string + branch string + setup func(t *testing.T, dir string) + verifyResult func(t *testing.T, dir string) // Verify the branch was actually updated + wantErr bool }{ + { + name: "Update existing branch with fast-forward merge", + branch: "main", + setup: func(t *testing.T, dir string) { + // Create a local repository + createTestRepository(t, dir) + + // Create a bare repository to act as remote + remoteDir := filepath.Join(t.TempDir(), "remote.git") + os.MkdirAll(remoteDir, 0755) + runCommand(t, remoteDir, "git", "init", "--bare") + + // Add remote and push initial commit + runCommand(t, dir, "git", "remote", "add", "origin", remoteDir) + // Push current branch as main + runCommand(t, dir, "git", "push", "-u", "origin", "HEAD:main") + + // Simulate remote changes by creating a new commit in another clone + cloneDir := t.TempDir() + runCommand(t, cloneDir, "git", "clone", "-b", "main", remoteDir, ".") + runCommand(t, cloneDir, "git", "config", "user.email", "test@example.com") + runCommand(t, cloneDir, "git", "config", "user.name", "Test User") + writeFile(t, filepath.Join(cloneDir, "new-file.txt"), "new content") + runCommand(t, cloneDir, "git", "add", ".") + runCommand(t, cloneDir, "git", "commit", "-m", "Remote commit") + runCommand(t, cloneDir, "git", "push", "origin", "main") + }, + verifyResult: func(t *testing.T, dir string) { + // Verify the new file exists after pull + if _, err := os.Stat(filepath.Join(dir, "new-file.txt")); os.IsNotExist(err) { + t.Error("Expected new-file.txt to exist after pull") + } + }, + wantErr: false, + }, { name: "Update existing branch", branch: "main", @@ -489,6 +525,28 @@ func TestClient_UpdateBaseBranch(t *testing.T) { }, wantErr: false, }, + { + name: "Update when on different branch", + branch: "main", + setup: func(t *testing.T, dir string) { + createTestRepository(t, dir) + // Create and switch to a different branch + runCommand(t, dir, "git", "checkout", "-b", "feature-branch") + writeFile(t, filepath.Join(dir, "feature.txt"), "feature content") + runCommand(t, dir, "git", "add", ".") + runCommand(t, dir, "git", "commit", "-m", "Feature commit") + }, + verifyResult: func(t *testing.T, dir string) { + // Verify we're back on the feature branch + cmd := exec.Command("git", "-C", dir, "branch", "--show-current") + output, _ := cmd.Output() + currentBranch := strings.TrimSpace(string(output)) + if currentBranch != "feature-branch" { + t.Errorf("Expected to be on feature-branch, but on %s", currentBranch) + } + }, + wantErr: false, + }, { name: "Update non-existent branch", branch: "non-existent", @@ -513,10 +571,11 @@ func TestClient_UpdateBaseBranch(t *testing.T) { // Set Git configuration for CI environment runCommand(t, dir, "git", "config", "user.email", "test@example.com") runCommand(t, dir, "git", "config", "user.name", "Test User") - runCommand(t, dir, "git", "checkout", "-b", "main") + runCommand(t, dir, "git", "config", "init.defaultBranch", "main") writeFile(t, filepath.Join(dir, "README.md"), "# Test") runCommand(t, dir, "git", "add", ".") runCommand(t, dir, "git", "commit", "-m", "Initial commit") + runCommand(t, dir, "git", "branch", "-m", "main") }, wantErr: false, // Should not fail if no remote }, @@ -541,6 +600,11 @@ func TestClient_UpdateBaseBranch(t *testing.T) { if (err != nil) != tt.wantErr { t.Errorf("UpdateBaseBranch() error = %v, wantErr %v", err, tt.wantErr) } + + // Verify result if function is provided + if err == nil && tt.verifyResult != nil { + tt.verifyResult(t, tmpDir) + } }) } } @@ -612,19 +676,28 @@ func TestClient_WorktreeExists(t *testing.T) { func createTestRepository(t *testing.T, dir string) { t.Helper() - // Initialize git repository - runCommand(t, dir, "git", "init") + // Initialize git repository with main as default branch + runCommand(t, dir, "git", "init", "-b", "main") // Set Git configuration for CI environment runCommand(t, dir, "git", "config", "user.email", "test@example.com") runCommand(t, dir, "git", "config", "user.name", "Test User") - runCommand(t, dir, "git", "checkout", "-b", "main") - // Create initial commit writeFile(t, filepath.Join(dir, "README.md"), "# Test Repository") runCommand(t, dir, "git", "add", ".") runCommand(t, dir, "git", "commit", "-m", "Initial commit") + + // Ensure the branch is named "main" (some Git versions might not respect -b flag) + // Check current branch and rename if necessary + cmd := exec.Command("git", "-C", dir, "branch", "--show-current") + output, err := cmd.Output() + if err == nil { + currentBranch := strings.TrimSpace(string(output)) + if currentBranch != "main" && currentBranch != "" { + runCommand(t, dir, "git", "branch", "-m", currentBranch, "main") + } + } } func runCommand(t *testing.T, dir string, name string, args ...string) { diff --git a/internal/service/git_workspace_test.go b/internal/service/git_workspace_test.go index fac12c6..2681692 100644 --- a/internal/service/git_workspace_test.go +++ b/internal/service/git_workspace_test.go @@ -287,3 +287,29 @@ func TestGitWorkspaceManager_WithCustomBaseBranch(t *testing.T) { require.NoError(t, err) mockClient.AssertExpectations(t) } + +func TestGitWorkspaceManager_PrepareWorkspace_UpdatesBaseBranch(t *testing.T) { + // This test verifies that PrepareWorkspace always calls UpdateBaseBranch + // before creating a new worktree, ensuring we have the latest code + cfg := &config.Config{ + Git: config.GitConfig{ + WorktreeBasePath: ".git/soba/worktrees", + BaseBranch: "main", + }, + } + mockClient := new(mockGitClient) + + expectedPath := filepath.Join(".git/soba/worktrees", "issue-100") + + // Setup expectations: UpdateBaseBranch must be called before CreateWorktree + mockClient.On("WorktreeExists", expectedPath).Return(false) + mockClient.On("UpdateBaseBranch", "main").Return(nil).Once() + mockClient.On("CreateWorktree", expectedPath, "soba/100", "main").Return(nil) + + manager := NewGitWorkspaceManager(cfg, mockClient) + err := manager.PrepareWorkspace(100) + + require.NoError(t, err) + // Verify that UpdateBaseBranch was called exactly once + mockClient.AssertExpectations(t) +} diff --git a/internal/service/workflow_executor_test.go b/internal/service/workflow_executor_test.go index 73cecc0..70ed5d8 100644 --- a/internal/service/workflow_executor_test.go +++ b/internal/service/workflow_executor_test.go @@ -603,6 +603,111 @@ func TestGenerateSessionName(t *testing.T) { } } +func TestWorkflowExecutor_PrepareWorkspaceWithBaseBranchUpdate(t *testing.T) { + // この統合テストは、各フェーズ実行時にワークスペース準備を通じて + // ベースブランチが最新化されることを確認します + tests := []struct { + name string + phase domain.Phase + issueNumber int + setupMocks func(*MockTmuxClient, *MockWorkspaceManager, *MockIssueProcessorUpdater) + wantErr bool + }{ + { + name: "Plan phase prepares workspace with base branch update", + phase: domain.PhasePlan, + issueNumber: 201, + setupMocks: func(tmux *MockTmuxClient, workspace *MockWorkspaceManager, processor *MockIssueProcessorUpdater) { + processor.On("Configure", mock.Anything).Return(nil) + processor.On("UpdateLabels", mock.Anything, 201, domain.LabelQueued, domain.LabelPlanning).Return(nil) + // PrepareWorkspaceが呼ばれることで、内部的にUpdateBaseBranchが実行される + workspace.On("PrepareWorkspace", 201).Return(nil) + tmux.On("SessionExists", "soba-test-repo").Return(true) + tmux.On("WindowExists", "soba-test-repo", "issue-201").Return(false, nil) + tmux.On("CreateWindow", "soba-test-repo", "issue-201").Return(nil) + tmux.On("GetLastPaneIndex", "soba-test-repo", "issue-201").Return(0, nil) + tmux.On("SendCommand", "soba-test-repo", "issue-201", 0, mock.Anything).Return(nil) + }, + wantErr: false, + }, + { + name: "Implement phase prepares workspace with base branch update", + phase: domain.PhaseImplement, + issueNumber: 202, + setupMocks: func(tmux *MockTmuxClient, workspace *MockWorkspaceManager, processor *MockIssueProcessorUpdater) { + processor.On("Configure", mock.Anything).Return(nil) + processor.On("UpdateLabels", mock.Anything, 202, domain.LabelReady, domain.LabelDoing).Return(nil) + // PrepareWorkspaceが呼ばれることで、内部的にUpdateBaseBranchが実行される + workspace.On("PrepareWorkspace", 202).Return(nil) + tmux.On("SessionExists", "soba-test-repo").Return(true) + tmux.On("WindowExists", "soba-test-repo", "issue-202").Return(false, nil) + tmux.On("CreateWindow", "soba-test-repo", "issue-202").Return(nil) + tmux.On("GetLastPaneIndex", "soba-test-repo", "issue-202").Return(0, nil) + tmux.On("SendCommand", "soba-test-repo", "issue-202", 0, mock.Anything).Return(nil) + }, + wantErr: false, + }, + { + name: "Revise phase prepares workspace with base branch update", + phase: domain.PhaseRevise, + issueNumber: 203, + setupMocks: func(tmux *MockTmuxClient, workspace *MockWorkspaceManager, processor *MockIssueProcessorUpdater) { + processor.On("Configure", mock.Anything).Return(nil) + processor.On("UpdateLabels", mock.Anything, 203, domain.LabelRequiresChanges, domain.LabelRevising).Return(nil) + // PrepareWorkspaceが呼ばれることで、内部的にUpdateBaseBranchが実行される + workspace.On("PrepareWorkspace", 203).Return(nil) + tmux.On("SessionExists", "soba-test-repo").Return(true) + tmux.On("WindowExists", "soba-test-repo", "issue-203").Return(false, nil) + tmux.On("CreateWindow", "soba-test-repo", "issue-203").Return(nil) + tmux.On("GetLastPaneIndex", "soba-test-repo", "issue-203").Return(0, nil) + tmux.On("SendCommand", "soba-test-repo", "issue-203", 0, mock.Anything).Return(nil) + }, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockTmux := new(MockTmuxClient) + mockWorkspace := new(MockWorkspaceManager) + mockProcessor := new(MockIssueProcessorUpdater) + + tt.setupMocks(mockTmux, mockWorkspace, mockProcessor) + + executor := NewWorkflowExecutor(mockTmux, mockWorkspace, mockProcessor, logging.NewMockLogger()) + + cfg := &config.Config{ + Git: config.GitConfig{ + WorktreeBasePath: ".git/soba/worktrees", + BaseBranch: "main", + }, + GitHub: config.GitHubConfig{ + Repository: "test/repo", + }, + Phase: config.PhaseConfig{ + Plan: config.PhaseCommand{Command: "echo", Parameter: "Planning"}, + Implement: config.PhaseCommand{Command: "echo", Parameter: "Implementing"}, + Revise: config.PhaseCommand{Command: "echo", Parameter: "Revising"}, + }, + } + + err := executor.ExecutePhase(context.Background(), cfg, tt.issueNumber, tt.phase) + + if tt.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + + // PrepareWorkspaceが呼ばれたことを確認 + // これにより、内部的にUpdateBaseBranchが呼ばれることが保証される + mockWorkspace.AssertCalled(t, "PrepareWorkspace", tt.issueNumber) + mockTmux.AssertExpectations(t) + mockProcessor.AssertExpectations(t) + }) + } +} + func TestWorkflowExecutor_setupTmuxSession(t *testing.T) { tests := []struct { name string