fix: async workspace preparation and worktree branch fallback (#433)

This commit is contained in:
Carlos Florêncio
2026-03-14 12:00:35 +00:00
committed by GitHub
parent 50e19bcdac
commit aba9271fd5
15 changed files with 646 additions and 90 deletions
@@ -360,6 +360,7 @@ func (s *Service) backfillRepoProvider(store repoStore, repo *models.Repository)
// ensureSessionPRWatch creates a PRWatch (pr_number=0) for a session's branch
// so the poller will search for a PR on GitHub. Runs as a background goroutine.
func (s *Service) ensureSessionPRWatch(ctx context.Context, taskID, sessionID, branch string) {
branch = s.resolvePRWatchBranch(ctx, taskID, sessionID, branch)
if s.githubService == nil || branch == "" {
return
}
@@ -377,6 +378,33 @@ func (s *Service) ensureSessionPRWatch(ctx context.Context, taskID, sessionID, b
}
}
func (s *Service) resolvePRWatchBranch(ctx context.Context, taskID, sessionID, fallback string) string {
store, ok := s.repo.(repoStore)
if !ok {
return fallback
}
taskRepo, err := store.GetPrimaryTaskRepository(ctx, taskID)
if err == nil && taskRepo != nil && strings.TrimSpace(taskRepo.CheckoutBranch) != "" {
return strings.TrimSpace(taskRepo.CheckoutBranch)
}
if fallback != "" {
return fallback
}
session, err := s.repo.GetTaskSession(ctx, sessionID)
if err != nil || session == nil {
return ""
}
for _, wt := range session.Worktrees {
if wt.WorktreeBranch != "" {
return wt.WorktreeBranch
}
}
return ""
}
// resolveTaskRepo looks up the GitHub owner and repo name for a task's primary repository.
// If provider info is missing but a local path exists, it detects from the git remote
// and backfills the DB record for future calls.
@@ -449,18 +477,7 @@ func (s *Service) CheckSessionPR(ctx context.Context, taskID, sessionID string)
return false, nil
}
// Get the session's branch from the first worktree with a non-empty branch
session, err := s.repo.GetTaskSession(ctx, sessionID)
if err != nil || session == nil {
return false, nil
}
branch := ""
for _, wt := range session.Worktrees {
if wt.WorktreeBranch != "" {
branch = wt.WorktreeBranch
break
}
}
branch := s.resolvePRWatchBranch(ctx, taskID, sessionID, "")
if branch == "" {
return false, nil
}
@@ -11,27 +11,31 @@ import (
// mockGitHubService implements GitHubService for testing CheckSessionPR.
type mockGitHubService struct {
client github.Client
taskPR *github.TaskPR
taskPRErr error
ensureWatchCalls int
createWatchCalls int
associateCalls int
client github.Client
taskPR *github.TaskPR
taskPRErr error
ensureWatchCalls int
createWatchCalls int
associateCalls int
ensureWatchBranch string
createWatchBranch string
}
func (m *mockGitHubService) Client() github.Client { return m.client }
func (m *mockGitHubService) GetTaskPR(_ context.Context, _ string) (*github.TaskPR, error) {
return m.taskPR, m.taskPRErr
}
func (m *mockGitHubService) EnsurePRWatch(_ context.Context, _, _, _, _, _ string) (*github.PRWatch, error) {
func (m *mockGitHubService) EnsurePRWatch(_ context.Context, _, _, _, _, branch string) (*github.PRWatch, error) {
m.ensureWatchCalls++
m.ensureWatchBranch = branch
return &github.PRWatch{}, nil
}
func (m *mockGitHubService) GetPRWatchBySession(_ context.Context, _ string) (*github.PRWatch, error) {
return nil, nil
}
func (m *mockGitHubService) CreatePRWatch(_ context.Context, _, _, _, _ string, _ int, _ string) (*github.PRWatch, error) {
func (m *mockGitHubService) CreatePRWatch(_ context.Context, _, _, _, _ string, _ int, branch string) (*github.PRWatch, error) {
m.createWatchCalls++
m.createWatchBranch = branch
return &github.PRWatch{}, nil
}
func (m *mockGitHubService) AssociatePRWithTask(_ context.Context, _ string, _ *github.PR) (*github.TaskPR, error) {
@@ -108,7 +112,7 @@ func TestCheckSessionPR(t *testing.T) {
// seedWithRepo creates task + session + repository + task-repository + worktree
// so that resolveTaskRepo and GetTaskSession succeed.
seedWithRepo := func(t *testing.T, branch string) *Service {
seedWithRepo := func(t *testing.T, branch, checkoutBranch string) *Service {
t.Helper()
repo := setupTestRepo(t)
seedSession(t, repo, "t1", "s1", "step1")
@@ -131,12 +135,13 @@ func TestCheckSessionPR(t *testing.T) {
// Link task to repository
taskRepo := &models.TaskRepository{
ID: "tr1",
TaskID: "t1",
RepositoryID: "repo1",
Position: 0,
CreatedAt: now,
UpdatedAt: now,
ID: "tr1",
TaskID: "t1",
RepositoryID: "repo1",
CheckoutBranch: checkoutBranch,
Position: 0,
CreatedAt: now,
UpdatedAt: now,
}
if err := repo.CreateTaskRepository(ctx, taskRepo); err != nil {
t.Fatalf("failed to create task repository: %v", err)
@@ -210,7 +215,7 @@ func TestCheckSessionPR(t *testing.T) {
})
t.Run("returns false when session has no worktree branch", func(t *testing.T) {
svc := seedWithRepo(t, "") // empty branch
svc := seedWithRepo(t, "", "") // empty branch
ghSvc := &mockGitHubService{taskPRErr: nil, taskPR: nil}
svc.SetGitHubService(ghSvc)
@@ -224,7 +229,7 @@ func TestCheckSessionPR(t *testing.T) {
})
t.Run("returns false when no PR exists on branch", func(t *testing.T) {
svc := seedWithRepo(t, "feature-branch")
svc := seedWithRepo(t, "feature-branch", "")
mockClient := github.NewMockClient()
// No PR added to mock client
ghSvc := &mockGitHubService{client: mockClient}
@@ -240,10 +245,13 @@ func TestCheckSessionPR(t *testing.T) {
if ghSvc.ensureWatchCalls != 1 {
t.Errorf("expected 1 EnsurePRWatch call, got %d", ghSvc.ensureWatchCalls)
}
if ghSvc.ensureWatchBranch != "feature-branch" {
t.Errorf("expected EnsurePRWatch branch %q, got %q", "feature-branch", ghSvc.ensureWatchBranch)
}
})
t.Run("finds PR and associates it", func(t *testing.T) {
svc := seedWithRepo(t, "feature-branch")
svc := seedWithRepo(t, "feature-branch", "")
mockClient := github.NewMockClient()
mockClient.AddPR(testPR)
ghSvc := &mockGitHubService{client: mockClient}
@@ -262,8 +270,51 @@ func TestCheckSessionPR(t *testing.T) {
if ghSvc.createWatchCalls != 1 {
t.Errorf("expected 1 CreatePRWatch call (from associatePRFromPush), got %d", ghSvc.createWatchCalls)
}
if ghSvc.ensureWatchBranch != "feature-branch" {
t.Errorf("expected EnsurePRWatch branch %q, got %q", "feature-branch", ghSvc.ensureWatchBranch)
}
if ghSvc.createWatchBranch != "feature-branch" {
t.Errorf("expected CreatePRWatch branch %q, got %q", "feature-branch", ghSvc.createWatchBranch)
}
if ghSvc.associateCalls != 1 {
t.Errorf("expected 1 AssociatePRWithTask call, got %d", ghSvc.associateCalls)
}
})
t.Run("prefers checkout branch over synthetic worktree branch", func(t *testing.T) {
svc := seedWithRepo(t, "kandev/pr-review-abc", "feature-branch")
mockClient := github.NewMockClient()
mockClient.AddPR(testPR)
ghSvc := &mockGitHubService{client: mockClient}
svc.SetGitHubService(ghSvc)
found, err := svc.CheckSessionPR(ctx, "t1", "s1")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if !found {
t.Error("expected found=true when PR exists on checkout branch")
}
if ghSvc.ensureWatchBranch != "feature-branch" {
t.Errorf("expected EnsurePRWatch branch %q, got %q", "feature-branch", ghSvc.ensureWatchBranch)
}
if ghSvc.createWatchBranch != "feature-branch" {
t.Errorf("expected CreatePRWatch branch %q, got %q", "feature-branch", ghSvc.createWatchBranch)
}
})
t.Run("ensureSessionPRWatch prefers checkout branch", func(t *testing.T) {
svc := seedWithRepo(t, "kandev/pr-review-abc", "feature-branch")
ghSvc := &mockGitHubService{}
svc.SetGitHubService(ghSvc)
svc.ensureSessionPRWatch(ctx, "t1", "s1", "kandev/pr-review-abc")
if ghSvc.ensureWatchCalls != 1 {
t.Errorf("expected 1 EnsurePRWatch call, got %d", ghSvc.ensureWatchCalls)
}
if ghSvc.ensureWatchBranch != "feature-branch" {
t.Errorf("expected EnsurePRWatch branch %q, got %q", "feature-branch", ghSvc.ensureWatchBranch)
}
})
}
@@ -74,11 +74,10 @@ func (s *Service) EnqueueTask(ctx context.Context, task *v1.Task) error {
}
// PrepareTaskSession creates a session entry without launching the agent.
// This allows the HTTP handler to return the session ID immediately while the agent setup
// This allows the WS handler to return the session ID immediately while workspace setup
// continues in the background. Use StartTaskWithSession to continue with agent launch.
// When launchWorkspace is true, workspace infrastructure (agentctl) is launched synchronously
// so file browsing works immediately. When false, the workspace launch is deferred to
// StartTaskWithSession (useful for remote executors where provisioning takes 30-60s).
// When launchWorkspace is true, workspace infrastructure (agentctl) is launched asynchronously;
// the frontend receives preparation progress via executor.prepare.progress WS events.
func (s *Service) PrepareTaskSession(ctx context.Context, taskID string, agentProfileID string, executorID string, executorProfileID string, workflowStepID string, launchWorkspace bool) (string, error) {
s.logger.Debug("preparing task session",
zap.String("task_id", taskID),
@@ -133,17 +132,23 @@ func (s *Service) PrepareTaskSession(ctx context.Context, taskID string, agentPr
}
if launchWorkspace {
// Launch workspace infrastructure (agentctl) without starting the agent subprocess.
// This enables file browsing, editing, etc. while the session is in CREATED state.
if prepExec, launchErr := s.executor.LaunchPreparedSession(ctx, task, sessionID, executor.LaunchOptions{AgentProfileID: agentProfileID, ExecutorID: executorID, WorkflowStepID: workflowStepID}); launchErr != nil {
s.logger.Warn("failed to launch workspace for prepared session (file browsing may be unavailable)",
zap.String("task_id", taskID),
zap.String("session_id", sessionID),
zap.Error(launchErr))
// Non-fatal: session is still usable, workspace will be launched when agent starts
} else if prepExec != nil && prepExec.WorktreeBranch != "" {
go s.ensureSessionPRWatch(context.Background(), taskID, prepExec.SessionID, prepExec.WorktreeBranch)
}
// Launch workspace infrastructure (agentctl) in the background so the WS response
// returns the session ID immediately. The frontend navigates to the session page
// and shows preparation progress via executor.prepare.progress WS events.
go func() {
bgCtx := context.Background()
prepExec, launchErr := s.executor.LaunchPreparedSession(bgCtx, task, sessionID, executor.LaunchOptions{AgentProfileID: agentProfileID, ExecutorID: executorID, WorkflowStepID: workflowStepID})
if launchErr != nil {
s.logger.Warn("failed to launch workspace for prepared session (file browsing may be unavailable)",
zap.String("task_id", taskID),
zap.String("session_id", sessionID),
zap.Error(launchErr))
return
}
if prepExec != nil && prepExec.WorktreeBranch != "" {
s.ensureSessionPRWatch(bgCtx, taskID, prepExec.SessionID, prepExec.WorktreeBranch)
}
}()
}
s.logger.Info("task session prepared",
@@ -475,9 +480,11 @@ func (s *Service) recordInitialMessage(ctx context.Context, taskID, sessionID, p
// buildWorkflowPrompt constructs the effective prompt using workflow step configuration.
// If step.Prompt contains {{task_prompt}}, it is replaced with the base prompt.
// Otherwise, step.Prompt is prepended to the base prompt.
// System-injected content is wrapped in <kandev-system> tags so it can be stripped when displaying to users.
// Otherwise, step.Prompt fully replaces the base prompt.
// If the step has enable_plan_mode in on_enter events, plan mode prefix is also prepended.
// Only true internal instructions are wrapped in <kandev-system> tags so they can be stripped from the visible chat.
func (s *Service) buildWorkflowPrompt(basePrompt string, step *wfmodels.WorkflowStep, taskID string, sessionID string) string {
_ = sessionID
var parts []string
// Build the prompt from step.Prompt template and base prompt
@@ -488,9 +495,8 @@ func (s *Service) buildWorkflowPrompt(basePrompt string, step *wfmodels.Workflow
combined := strings.Replace(interpolatedPrompt, "{{task_prompt}}", basePrompt, 1)
parts = append(parts, combined)
} else {
// Prepend step prompt, then base prompt
parts = append(parts, sysprompt.Wrap(interpolatedPrompt))
parts = append(parts, basePrompt)
// A step prompt without {{task_prompt}} is treated as the full visible prompt.
parts = append(parts, interpolatedPrompt)
}
} else {
// No step prompt, just use base prompt
@@ -0,0 +1,74 @@
package orchestrator
import (
"context"
"strings"
"testing"
wfmodels "github.com/kandev/kandev/internal/workflow/models"
)
func TestBuildWorkflowPrompt_ReplacesTaskPromptPlaceholder(t *testing.T) {
svc := createTestService(setupTestRepo(t), newMockStepGetter(), newMockTaskRepo())
step := &wfmodels.WorkflowStep{
ID: "step-1",
Prompt: "Implement this exactly:\n\n{{task_prompt}}",
}
got := svc.buildWorkflowPrompt("Migrate Atlantis datasource.", step, "task-1", "session-1")
want := "Implement this exactly:\n\nMigrate Atlantis datasource."
if got != want {
t.Fatalf("buildWorkflowPrompt() = %q, want %q", got, want)
}
}
func TestBuildWorkflowPrompt_UsesStepPromptOnlyWithoutTaskPromptPlaceholder(t *testing.T) {
svc := createTestService(setupTestRepo(t), newMockStepGetter(), newMockTaskRepo())
step := &wfmodels.WorkflowStep{
ID: "step-1",
Prompt: "Commit the changes, push and create a draft PR.",
}
got := svc.buildWorkflowPrompt("Migrate Atlantis datasource.", step, "task-1", "session-1")
want := "Commit the changes, push and create a draft PR."
if got != want {
t.Fatalf("buildWorkflowPrompt() = %q, want %q", got, want)
}
}
func TestApplyWorkflowAndPlanMode_KeepsWorkflowPromptVisibleWhenStepEnablesPlanMode(t *testing.T) {
repo := setupTestRepo(t)
stepGetter := newMockStepGetter()
stepGetter.steps["step-1"] = &wfmodels.WorkflowStep{
ID: "step-1",
Prompt: "Commit the changes, push and create a draft PR.",
Events: wfmodels.StepEvents{
OnEnter: []wfmodels.OnEnterAction{{Type: wfmodels.OnEnterEnablePlanMode}},
},
}
svc := createTestService(repo, stepGetter, newMockTaskRepo())
got, planModeActive := svc.applyWorkflowAndPlanMode(
context.Background(),
"Migrate Atlantis datasource.",
"task-1",
"session-1",
"step-1",
false,
)
if !planModeActive {
t.Fatal("expected plan mode to be active")
}
if !strings.Contains(got, "Commit the changes, push and create a draft PR.") {
t.Fatalf("expected visible workflow prompt in effective prompt, got %q", got)
}
if strings.Contains(got, "Migrate Atlantis datasource.") {
t.Fatalf("expected base prompt to be omitted when step prompt lacks {{task_prompt}}, got %q", got)
}
if strings.Contains(got, "<kandev-system>") {
t.Fatalf("expected workflow prompt to remain visible without hidden system wrapping, got %q", got)
}
}
@@ -0,0 +1,53 @@
package models
import (
"testing"
"time"
)
func TestMessageToAPI_StripsOnlySystemContent(t *testing.T) {
msg := &Message{
ID: "msg-1",
TaskSessionID: "session-1",
TaskID: "task-1",
TurnID: "turn-1",
AuthorType: MessageAuthorUser,
Content: "<kandev-system>PLAN MODE ACTIVE</kandev-system>\n\nCommit the changes, push and create a draft PR.",
Type: MessageTypeMessage,
CreatedAt: time.Now().UTC(),
}
apiMsg := msg.ToAPI()
if apiMsg.Content != "Commit the changes, push and create a draft PR." {
t.Fatalf("expected visible content to keep workflow prompt, got %q", apiMsg.Content)
}
if apiMsg.RawContent != msg.Content {
t.Fatalf("expected raw_content to preserve original content, got %q", apiMsg.RawContent)
}
if apiMsg.Metadata["has_hidden_prompts"] != true {
t.Fatalf("expected has_hidden_prompts metadata, got %v", apiMsg.Metadata["has_hidden_prompts"])
}
}
func TestMessageToAPI_KeepsWorkflowPromptVisibleWithoutSystemTags(t *testing.T) {
msg := &Message{
ID: "msg-1",
TaskSessionID: "session-1",
TaskID: "task-1",
TurnID: "turn-1",
AuthorType: MessageAuthorUser,
Content: "Commit the changes, push and create a draft PR.",
Type: MessageTypeMessage,
CreatedAt: time.Now().UTC(),
}
apiMsg := msg.ToAPI()
if apiMsg.Content != msg.Content {
t.Fatalf("expected visible content %q, got %q", msg.Content, apiMsg.Content)
}
if apiMsg.RawContent != "" {
t.Fatalf("expected empty raw_content when there is no hidden prompt, got %q", apiMsg.RawContent)
}
}
+2 -2
View File
@@ -141,8 +141,8 @@ type Message struct {
}
// ToAPI converts internal Message to API type.
// System-injected content (wrapped in <kandev-system> tags) is stripped from the content
// so users don't see workflow step prompt modifications in the UI.
// Only true system-injected content (wrapped in <kandev-system> tags) is stripped
// from the visible content sent to the UI.
func (m *Message) ToAPI() *v1.Message {
messageType := string(m.Type)
if messageType == "" {
@@ -214,8 +214,8 @@ func (s *Service) publishEnvironmentEvent(ctx context.Context, eventType string,
}
// publishMessageEvent publishes message events to the event bus.
// System-injected content (wrapped in <kandev-system> tags) is stripped from the content
// so users don't see workflow step prompt modifications in the UI.
// Only true system-injected content (wrapped in <kandev-system> tags) is stripped
// from the visible message content delivered to clients.
func (s *Service) publishMessageEvent(ctx context.Context, eventType string, message *models.Message) {
if s.eventBus == nil {
s.logger.Warn("publishMessageEvent: eventBus is nil, skipping")
+12 -4
View File
@@ -54,18 +54,26 @@ func containsAuthFailure(lowerOutput string) bool {
strings.Contains(lowerOutput, "askpass")
}
// isBranchCheckedOutError checks if git output indicates a branch is already
// checked out in another worktree. Different git versions use different messages:
// "is already checked out at" or "is already used by worktree at".
func isBranchCheckedOutError(output string) bool {
out := strings.ToLower(output)
return strings.Contains(out, "is already checked out at") ||
strings.Contains(out, "is already used by worktree at")
}
// ClassifyGitError wraps a raw git error with a user-friendly sentinel error
// based on the command output.
func ClassifyGitError(output string, _ error) error {
out := strings.ToLower(output)
trimmed := strings.TrimSpace(output)
switch {
case strings.Contains(out, "is already checked out at"):
case isBranchCheckedOutError(output):
return fmt.Errorf("%w: %s", ErrBranchCheckedOut, trimmed)
case containsAuthFailure(out):
case containsAuthFailure(strings.ToLower(output)):
return fmt.Errorf("%w: %s", ErrAuthFailed, trimmed)
case strings.Contains(out, "non-fast-forward"):
case strings.Contains(strings.ToLower(output), "non-fast-forward"):
return fmt.Errorf("%w: %s", ErrNonFastForward, trimmed)
default:
return fmt.Errorf("%w: %s", ErrGitCommandFailed, trimmed)
+52 -27
View File
@@ -250,9 +250,11 @@ func (m *Manager) Create(ctx context.Context, req CreateRequest) (*Worktree, err
// createWorktree performs the actual git worktree creation.
func (m *Manager) createWorktree(ctx context.Context, req CreateRequest, baseRef string) (*Worktree, error) {
worktreeDirName, branchName := m.buildWorktreeNames(req)
startPoint := baseRef
// If a checkout branch is specified (e.g. a PR's head branch), fetch it
// and checkout it directly in the worktree instead of creating a new branch.
// and try to check it out directly. If the branch is already checked out
// in another worktree, fall back to a unique branch with a random suffix.
var fetchResult *FetchBranchResult
if req.CheckoutBranch != "" {
var err error
@@ -260,7 +262,7 @@ func (m *Manager) createWorktree(ctx context.Context, req CreateRequest, baseRef
if err != nil {
return nil, err
}
branchName = req.CheckoutBranch
startPoint = req.CheckoutBranch
}
worktreePath, err := m.config.WorktreePath(worktreeDirName)
@@ -268,14 +270,7 @@ func (m *Manager) createWorktree(ctx context.Context, req CreateRequest, baseRef
return nil, fmt.Errorf("failed to get worktree path: %w", err)
}
var worktreeID string
if req.CheckoutBranch != "" {
// Checkout the existing PR branch directly in the worktree.
worktreeID, err = m.gitAddWorktreeExisting(ctx, req.RepositoryPath, branchName, worktreePath)
} else {
// Create a new branch based on the base ref.
worktreeID, err = m.gitAddWorktree(ctx, req.RepositoryPath, branchName, worktreePath, baseRef)
}
worktreeID, branchName, err := m.addWorktreeForBranch(ctx, req, worktreePath, branchName, startPoint, baseRef)
if err != nil {
return nil, err
}
@@ -303,6 +298,30 @@ func (m *Manager) createWorktree(ctx context.Context, req CreateRequest, baseRef
return wt, nil
}
// addWorktreeForBranch creates the git worktree, trying the checkout branch directly first
// and falling back to a suffixed branch if the checkout branch is already in use.
func (m *Manager) addWorktreeForBranch(ctx context.Context, req CreateRequest, worktreePath, fallbackBranch, startPoint, baseRef string) (string, string, error) {
if req.CheckoutBranch == "" {
id, err := m.gitAddWorktree(ctx, req.RepositoryPath, fallbackBranch, worktreePath, baseRef)
return id, fallbackBranch, err
}
// Try checking out the PR branch directly (common case: single task per PR).
id, err := m.gitAddWorktreeExisting(ctx, req.RepositoryPath, req.CheckoutBranch, worktreePath)
if err == nil {
return id, req.CheckoutBranch, nil
}
if !errors.Is(err, ErrBranchCheckedOut) {
return "", "", err
}
// Branch is in use by another worktree — create a unique fallback branch
// using the original branch name with a random suffix.
suffixed := req.CheckoutBranch + "-" + SmallSuffix(3)
id, err = m.gitAddWorktree(ctx, req.RepositoryPath, suffixed, worktreePath, startPoint)
return id, suffixed, err
}
// FetchBranchResult holds the outcome of a fetchBranchToLocal call.
type FetchBranchResult struct {
Warning string // User-friendly warning (non-empty when fell back to local)
@@ -362,25 +381,31 @@ func (m *Manager) gitAddWorktreeExisting(ctx context.Context, repoPath, branchNa
}
outStr := string(output)
if !strings.Contains(strings.ToLower(outStr), "is already checked out at") {
if !isBranchCheckedOutError(outStr) {
m.logger.Error("git worktree add (existing branch) failed",
zap.String("output", outStr), zap.Error(err))
return "", ClassifyGitError(outStr, err)
}
if recoveryErr := m.tryRecoverCheckedOutBranch(ctx, repoPath, branchName, outStr); recoveryErr != nil {
m.logger.Error("git worktree add (existing branch) failed",
zap.String("output", outStr), zap.Error(err))
return "", ClassifyGitError(outStr, err)
// Branch is genuinely in use by an active worktree — return sentinel error
// so callers can fall back to creating a unique branch.
m.logger.Warn("branch is checked out in active worktree",
zap.String("branch", branchName), zap.Error(recoveryErr))
return "", ErrBranchCheckedOut
}
// Retry after pruning stale worktree.
retryCmd := exec.CommandContext(ctx, "git", "worktree", "add", worktreePath, branchName)
retryCmd.Dir = repoPath
if retryOutput, retryErr := retryCmd.CombinedOutput(); retryErr != nil {
retryOutStr := string(retryOutput)
if isBranchCheckedOutError(retryOutStr) {
return "", ErrBranchCheckedOut
}
m.logger.Error("git worktree add retry failed",
zap.String("output", string(retryOutput)), zap.Error(retryErr))
return "", ClassifyGitError(string(retryOutput), retryErr)
zap.String("output", retryOutStr), zap.Error(retryErr))
return "", ClassifyGitError(retryOutStr, retryErr)
}
m.logger.Info("recovered from stale worktree checkout", zap.String("branch", branchName))
return worktreeID, nil
@@ -420,19 +445,19 @@ func (m *Manager) tryRecoverCheckedOutBranch(ctx context.Context, repoPath, bran
return nil
}
// parseCheckedOutPath extracts the worktree path from git's
// "fatal: 'branch' is already checked out at '/path'" error message.
// parseCheckedOutPath extracts the worktree path from git's error message.
// Handles both "is already checked out at '/path'" and "is already used by worktree at '/path'".
func parseCheckedOutPath(gitOutput string) string {
const marker = "checked out at '"
_, after, found := strings.Cut(gitOutput, marker)
if !found {
return ""
for _, marker := range []string{"checked out at '", "used by worktree at '"} {
_, after, found := strings.Cut(gitOutput, marker)
if found {
path, _, ok := strings.Cut(after, "'")
if ok {
return path
}
}
}
path, _, found := strings.Cut(after, "'")
if !found {
return ""
}
return path
return ""
}
// buildWorktreeNames derives the filesystem directory name and git branch name for a new worktree.
@@ -0,0 +1,143 @@
package worktree
import (
"context"
"os"
"os/exec"
"path/filepath"
"strings"
"testing"
)
func TestCreateWorktree_CheckoutBranchDirectCheckout(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)
}
repoPath := initGitRepoForWorktreeTest(t)
// First worktree with checkout branch should use the branch directly.
wt, err := mgr.Create(context.Background(), CreateRequest{
TaskID: "task-1",
SessionID: "session-1",
TaskTitle: "PR Review",
RepositoryID: "repo-1",
RepositoryPath: repoPath,
BaseBranch: "main",
CheckoutBranch: "feature/pr-branch",
})
if err != nil {
t.Fatalf("Create() unexpected error: %v", err)
}
if wt.Branch != "feature/pr-branch" {
t.Fatalf("expected direct checkout branch %q, got %q", "feature/pr-branch", wt.Branch)
}
if wt.FetchWarning == "" {
t.Fatal("expected fetch warning when origin is unavailable and local branch is reused")
}
gotBranch := strings.TrimSpace(runGit(t, wt.Path, "rev-parse", "--abbrev-ref", "HEAD"))
if gotBranch != "feature/pr-branch" {
t.Fatalf("worktree HEAD branch = %q, want %q", gotBranch, "feature/pr-branch")
}
prHeadSHA := strings.TrimSpace(runGit(t, repoPath, "rev-parse", "feature/pr-branch"))
worktreeSHA := strings.TrimSpace(runGit(t, wt.Path, "rev-parse", "HEAD"))
if worktreeSHA != prHeadSHA {
t.Fatalf("worktree HEAD SHA = %q, want %q", worktreeSHA, prHeadSHA)
}
}
func TestCreateWorktree_CheckoutBranchFallsBackToSuffix(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)
}
repoPath := initGitRepoForWorktreeTest(t)
// First worktree checks out the branch directly.
wt1, err := mgr.Create(context.Background(), CreateRequest{
TaskID: "task-1",
SessionID: "session-1",
TaskTitle: "PR Review 1",
RepositoryID: "repo-1",
RepositoryPath: repoPath,
BaseBranch: "main",
CheckoutBranch: "feature/pr-branch",
})
if err != nil {
t.Fatalf("Create() first worktree: %v", err)
}
if wt1.Branch != "feature/pr-branch" {
t.Fatalf("first worktree: expected direct branch, got %q", wt1.Branch)
}
// Second worktree with the same checkout branch should fall back to a suffixed name.
wt2, err := mgr.Create(context.Background(), CreateRequest{
TaskID: "task-2",
SessionID: "session-2",
TaskTitle: "PR Review 2",
RepositoryID: "repo-1",
RepositoryPath: repoPath,
BaseBranch: "main",
CheckoutBranch: "feature/pr-branch",
})
if err != nil {
t.Fatalf("Create() second worktree: %v", err)
}
if wt2.Branch == "feature/pr-branch" {
t.Fatal("second worktree should NOT use the checkout branch directly")
}
if !strings.HasPrefix(wt2.Branch, "feature/pr-branch-") {
t.Fatalf("expected suffixed branch like %q, got %q", "feature/pr-branch-xxx", wt2.Branch)
}
// Both worktrees should point to the same commit.
sha1 := strings.TrimSpace(runGit(t, wt1.Path, "rev-parse", "HEAD"))
sha2 := strings.TrimSpace(runGit(t, wt2.Path, "rev-parse", "HEAD"))
if sha1 != sha2 {
t.Fatalf("worktree SHAs differ: wt1=%q, wt2=%q", sha1, sha2)
}
}
func initGitRepoForWorktreeTest(t *testing.T) string {
t.Helper()
repoPath := t.TempDir()
runGit(t, repoPath, "init", "-b", "main")
runGit(t, repoPath, "config", "user.email", "test@example.com")
runGit(t, repoPath, "config", "user.name", "Test User")
runGit(t, repoPath, "config", "commit.gpgsign", "false")
filePath := filepath.Join(repoPath, "README.md")
if err := os.WriteFile(filePath, []byte("initial\n"), 0644); err != nil {
t.Fatalf("failed to write README.md: %v", err)
}
runGit(t, repoPath, "add", "README.md")
runGit(t, repoPath, "commit", "-m", "initial commit")
runGit(t, repoPath, "branch", "feature/pr-branch")
return repoPath
}
func runGit(t *testing.T, repoPath string, args ...string) string {
t.Helper()
cmd := exec.Command("git", args...)
cmd.Dir = repoPath
output, err := cmd.CombinedOutput()
if err != nil {
t.Fatalf("git %s failed: %v\n%s", strings.Join(args, " "), err, string(output))
}
return string(output)
}
+3 -3
View File
@@ -97,9 +97,9 @@ type CreateRequest struct {
// Typically "main" or "master".
BaseBranch string
// CheckoutBranch is a branch to fetch from origin and checkout after worktree creation.
// Used for PR reviews where the worktree is created from the default branch, then
// the PR's head branch is fetched and checked out. Empty means no post-creation checkout.
// CheckoutBranch is a branch to fetch from origin and check out directly in the
// worktree. If the branch is already checked out in another worktree, a unique
// fallback branch is created using the original name with a random suffix.
CheckoutBranch string
// WorktreeBranchPrefix is the prefix to use for the worktree branch name.
@@ -0,0 +1,18 @@
import { describe, expect, it, vi } from "vitest";
import { renderToStaticMarkup } from "react-dom/server";
import { MermaidBlock } from "./mermaid-block";
vi.mock("next-themes", () => ({
useTheme: () => ({ resolvedTheme: "dark" }),
}));
describe("MermaidBlock", () => {
it("renders a constrained outer container with a horizontal scroll viewport", () => {
const html = renderToStaticMarkup(<MermaidBlock code={"graph LR\nA-->B"} />);
expect(html).toContain("mermaid-block");
expect(html).toContain("block w-full max-w-full min-w-0");
expect(html).toContain("mermaid-scroll-region w-full overflow-x-auto overflow-y-hidden");
expect(html).not.toContain("inline-block");
});
});
+5 -2
View File
@@ -82,8 +82,11 @@ export function MermaidBlock({ code }: MermaidBlockProps) {
};
return (
<div className="mermaid-block group/mermaid relative my-3 rounded-md border border-border/50 bg-muted/20 inline-block">
<div className="overflow-auto p-3" style={{ display: showCode ? "none" : undefined }}>
<div className="mermaid-block group/mermaid relative my-3 block w-full max-w-full min-w-0 rounded-md border border-border/50 bg-muted/20">
<div
className="mermaid-scroll-region w-full overflow-x-auto overflow-y-hidden p-3"
style={{ display: showCode ? "none" : undefined }}
>
<div style={wrapperStyle}>
<div ref={containerRef} style={containerStyle} />
</div>
@@ -741,6 +741,159 @@ test.describe("Task creation from GitHub URL", () => {
await expect(warningBanner).not.toBeVisible();
});
test("two tasks from the same PR URL create independent worktrees", async ({
testPage,
apiClient,
seedData,
backend,
}) => {
test.setTimeout(120_000);
const { execSync } = await import("child_process");
const fs = await import("fs");
const gitEnv = {
...process.env,
HOME: backend.tmpDir,
GIT_AUTHOR_NAME: "E2E Test",
GIT_AUTHOR_EMAIL: "e2e@test.local",
GIT_COMMITTER_NAME: "E2E Test",
GIT_COMMITTER_EMAIL: "e2e@test.local",
};
// Create a repo with the PR branch locally.
const repoDir = `${backend.tmpDir}/repos/e2e-shared-pr`;
fs.mkdirSync(repoDir, { recursive: true });
execSync("git init -b main", { cwd: repoDir, env: gitEnv });
execSync('git commit --allow-empty -m "init"', { cwd: repoDir, env: gitEnv });
execSync("git checkout -b feature/shared-pr", { cwd: repoDir, env: gitEnv });
execSync('git commit --allow-empty -m "shared pr commit"', { cwd: repoDir, env: gitEnv });
execSync("git checkout main", { cwd: repoDir, env: gitEnv });
await apiClient.createRepository(seedData.workspaceId, repoDir, "main", {
name: "shared-owner/shared-repo",
provider: "github",
provider_owner: "shared-owner",
provider_name: "shared-repo",
});
await apiClient.mockGitHubAddBranches("shared-owner", "shared-repo", [
{ name: "main" },
{ name: "feature/shared-pr" },
]);
await apiClient.mockGitHubAddPRs([
{
number: 50,
title: "Shared PR",
state: "open",
head_branch: "feature/shared-pr",
base_branch: "main",
author_login: "test-user",
repo_owner: "shared-owner",
repo_name: "shared-repo",
},
]);
// Look up the worktree executor profile
const { executors } = await apiClient.listExecutors();
const worktreeExec = executors.find((e) => e.type === "worktree");
const worktreeProfile = worktreeExec?.profiles?.[0];
if (!worktreeProfile) {
test.skip(true, "No worktree executor profile available");
return;
}
const kanban = new KanbanPage(testPage);
await kanban.goto();
// Helper: select worktree executor in the create dialog
const selectWorktreeExecutor = async () => {
const selector = testPage.getByTestId("executor-profile-selector");
// Only click if the selector doesn't already show "Worktree"
const selectorText = await selector.textContent();
if (!selectorText?.includes("Worktree")) {
await selector.click();
await testPage.getByRole("option", { name: /Worktree/i }).click();
}
};
// --- Task A: first task from PR URL ---
await kanban.createTaskButton.first().click();
const dialog = testPage.getByTestId("create-task-dialog");
await expect(dialog).toBeVisible();
await testPage.getByTestId("toggle-github-url").click();
await testPage
.getByTestId("github-url-input")
.fill("https://github.com/shared-owner/shared-repo/pull/50");
await expect(dialog.getByText("feature/shared-pr")).toBeVisible({ timeout: 10_000 });
await testPage.getByTestId("task-title-input").fill("Shared PR Task A");
await testPage.getByTestId("task-description-input").fill("/e2e:simple-message");
const startBtn = testPage.getByTestId("submit-start-agent");
await expect(startBtn).toBeEnabled({ timeout: 15_000 });
await selectWorktreeExecutor();
await startBtn.click();
await expect(dialog).not.toBeVisible({ timeout: 10_000 });
await expect(kanban.taskCardByTitle("Shared PR Task A")).toBeVisible({ timeout: 10_000 });
// Wait for Task A agent to complete before creating Task B
await kanban.taskCardByTitle("Shared PR Task A").click();
await expect(testPage).toHaveURL(/\/s\//, { timeout: 15_000 });
const sessionA = new SessionPage(testPage);
await sessionA.waitForLoad();
await expect(sessionA.chat.getByText("simple mock response", { exact: false })).toBeVisible({
timeout: 30_000,
});
await expect(sessionA.idleInput()).toBeVisible({ timeout: 15_000 });
// Task A should have the direct PR branch
await expect(sessionA.terminal).toBeVisible({ timeout: 15_000 });
await sessionA.typeInTerminal("git branch --show-current");
await sessionA.expectTerminalHasText("feature/shared-pr");
// --- Task B: second task from the same PR URL ---
await testPage.goto("/");
await kanban.board.waitFor({ state: "visible" });
await kanban.createTaskButton.first().click();
await expect(dialog).toBeVisible();
await testPage.getByTestId("toggle-github-url").click();
await testPage
.getByTestId("github-url-input")
.fill("https://github.com/shared-owner/shared-repo/pull/50");
await expect(dialog.getByText("feature/shared-pr")).toBeVisible({ timeout: 10_000 });
await testPage.getByTestId("task-title-input").fill("Shared PR Task B");
await testPage.getByTestId("task-description-input").fill("/e2e:simple-message");
await expect(startBtn).toBeEnabled({ timeout: 15_000 });
await selectWorktreeExecutor();
await startBtn.click();
await expect(dialog).not.toBeVisible({ timeout: 10_000 });
// Task B should also succeed (this would have failed before the fix)
await expect(kanban.taskCardByTitle("Shared PR Task B")).toBeVisible({ timeout: 10_000 });
await kanban.taskCardByTitle("Shared PR Task B").click();
await expect(testPage).toHaveURL(/\/s\//, { timeout: 15_000 });
const sessionB = new SessionPage(testPage);
await sessionB.waitForLoad();
await expect(sessionB.chat.getByText("simple mock response", { exact: false })).toBeVisible({
timeout: 30_000,
});
// Task B should have a suffixed branch (not the original PR branch)
await expect(sessionB.terminal).toBeVisible({ timeout: 15_000 });
await sessionB.typeInTerminal("git branch --show-current");
// Branch should start with the PR branch name but have a random suffix
await sessionB.expectTerminalHasText("feature/shared-pr-");
});
test("can toggle between GitHub URL and repository selector", async ({ testPage }) => {
const kanban = new KanbanPage(testPage);
await kanban.goto();
@@ -551,4 +551,9 @@ test.describe("Workflow automation", () => {
timeout: 15_000,
});
});
// Note: "step prompt without {{task_prompt}} replaces task description" is tested
// by backend unit tests (TestBuildWorkflowPrompt_UsesStepPromptOnlyWithoutTaskPromptPlaceholder).
// The auto_start flow for new sessions uses a different prompt resolution path
// that can't be reliably tested e2e without an existing session.
});