diff --git a/apps/backend/cmd/kandev/adapters.go b/apps/backend/cmd/kandev/adapters.go index ae5b6ac130..c87c772a2b 100644 --- a/apps/backend/cmd/kandev/adapters.go +++ b/apps/backend/cmd/kandev/adapters.go @@ -93,6 +93,7 @@ func (a *lifecycleAdapter) LaunchAgent(ctx context.Context, req *executor.Launch RepositoryID: req.RepositoryID, RepositoryPath: req.RepositoryPath, BaseBranch: req.BaseBranch, + DefaultBranch: req.DefaultBranch, CheckoutBranch: req.CheckoutBranch, WorktreeBranchPrefix: req.WorktreeBranchPrefix, PullBeforeWorktree: req.PullBeforeWorktree, @@ -111,6 +112,7 @@ func (a *lifecycleAdapter) LaunchAgent(ctx context.Context, req *executor.Launch RepositoryURL: r.RepositoryURL, RepoName: r.RepoName, BaseBranch: r.BaseBranch, + DefaultBranch: r.DefaultBranch, CheckoutBranch: r.CheckoutBranch, WorktreeID: r.WorktreeID, WorktreeBranchPrefix: r.WorktreeBranchPrefix, diff --git a/apps/backend/internal/agent/lifecycle/env_preparer.go b/apps/backend/internal/agent/lifecycle/env_preparer.go index e9b7a5b6d0..83cc9ae181 100644 --- a/apps/backend/internal/agent/lifecycle/env_preparer.go +++ b/apps/backend/internal/agent/lifecycle/env_preparer.go @@ -29,6 +29,7 @@ type RepoPrepareSpec struct { RepositoryPath string RepoName string BaseBranch string + DefaultBranch string // Repository's default_branch, used as fallback when BaseBranch is missing CheckoutBranch string WorktreeID string WorktreeBranch string @@ -51,6 +52,7 @@ type EnvPrepareRequest struct { SetupScript string RepoSetupScript string // Repository-level setup script (e.g. "make install") BaseBranch string + DefaultBranch string // Repository's default_branch, used as fallback when BaseBranch is missing CheckoutBranch string WorktreeID string WorktreeBranch string @@ -86,6 +88,7 @@ func (r *EnvPrepareRequest) RepoSpecs() []RepoPrepareSpec { RepositoryPath: r.RepositoryPath, RepoName: r.RepoName, BaseBranch: r.BaseBranch, + DefaultBranch: r.DefaultBranch, CheckoutBranch: r.CheckoutBranch, WorktreeID: r.WorktreeID, WorktreeBranch: r.WorktreeBranch, diff --git a/apps/backend/internal/agent/lifecycle/env_preparer_worktree.go b/apps/backend/internal/agent/lifecycle/env_preparer_worktree.go index a0ef0b8a8d..bdb66c3732 100644 --- a/apps/backend/internal/agent/lifecycle/env_preparer_worktree.go +++ b/apps/backend/internal/agent/lifecycle/env_preparer_worktree.go @@ -166,6 +166,7 @@ func (p *WorktreePreparer) createWorktreeWithSync( RepositoryID: req.RepositoryID, RepositoryPath: req.RepositoryPath, BaseBranch: req.BaseBranch, + FallbackBaseBranch: req.DefaultBranch, CheckoutBranch: req.CheckoutBranch, WorktreeBranchPrefix: req.WorktreeBranchPrefix, PullBeforeWorktree: req.PullBeforeWorktree, @@ -191,6 +192,14 @@ func (p *WorktreePreparer) createWorktreeWithSync( } completeStepSuccess(&step) + if wt.BaseBranchFallbackWarning != "" { + step.Warning = wt.BaseBranchFallbackWarning + step.WarningDetail = wt.BaseBranchFallbackDetail + } + // wt.FetchWarning is surfaced in the separate "Fetch PR branch" step + // rendered later in the pipeline. It can only be non-empty when + // req.CheckoutBranch != "" (it is set inside fetchBranchToLocal), so the + // two warnings always co-occur and FetchWarning is never silently dropped. steps = append(steps, step) reportProgress(onProgress, step, stepIdx, totalSteps) return wt, steps, stepIdx + 1, nil @@ -356,6 +365,7 @@ func (p *WorktreePreparer) prepareOneRepo( subReq.RepositoryPath = spec.RepositoryPath subReq.RepoName = spec.RepoName subReq.BaseBranch = spec.BaseBranch + subReq.DefaultBranch = spec.DefaultBranch subReq.CheckoutBranch = spec.CheckoutBranch subReq.WorktreeID = spec.WorktreeID subReq.WorktreeBranchPrefix = spec.WorktreeBranchPrefix diff --git a/apps/backend/internal/agent/lifecycle/manager_launch.go b/apps/backend/internal/agent/lifecycle/manager_launch.go index e58f6e83ce..1ca14afd30 100644 --- a/apps/backend/internal/agent/lifecycle/manager_launch.go +++ b/apps/backend/internal/agent/lifecycle/manager_launch.go @@ -384,6 +384,7 @@ func (m *Manager) runEnvironmentPreparer( SetupScript: req.SetupScript, RepoSetupScript: repoSetupScript, BaseBranch: req.BaseBranch, + DefaultBranch: req.DefaultBranch, CheckoutBranch: req.CheckoutBranch, WorktreeBranchPrefix: req.WorktreeBranchPrefix, PullBeforeWorktree: req.PullBeforeWorktree, @@ -406,6 +407,7 @@ func (m *Manager) runEnvironmentPreparer( RepositoryPath: r.RepositoryPath, RepoName: r.RepoName, BaseBranch: r.BaseBranch, + DefaultBranch: r.DefaultBranch, CheckoutBranch: r.CheckoutBranch, WorktreeID: r.WorktreeID, WorktreeBranchPrefix: r.WorktreeBranchPrefix, diff --git a/apps/backend/internal/agent/lifecycle/types.go b/apps/backend/internal/agent/lifecycle/types.go index 8f8e3d1b64..0a39507ef9 100644 --- a/apps/backend/internal/agent/lifecycle/types.go +++ b/apps/backend/internal/agent/lifecycle/types.go @@ -272,6 +272,7 @@ type RepoLaunchSpec struct { RepositoryURL string // Clone URL for remote executors that need to clone RepoName string // Repository name used as subdirectory inside TaskDirName BaseBranch string + DefaultBranch string // Repository's default_branch, used as fallback when BaseBranch is missing CheckoutBranch string WorktreeID string // Existing worktree ID to reuse (skip creation if set) WorktreeBranchPrefix string @@ -314,6 +315,7 @@ type LaunchRequest struct { RepositoryID string // Repository ID for worktree tracking RepositoryPath string // Path to the main repository (for worktree creation) BaseBranch string // Base branch for the worktree (e.g., "main") + DefaultBranch string // Repository's default_branch, used as fallback when BaseBranch is missing CheckoutBranch string // Branch to fetch and checkout after worktree creation (e.g., PR head branch) WorktreeBranchPrefix string // Branch prefix for worktree branches PullBeforeWorktree bool // Whether to pull from remote before creating the worktree @@ -345,6 +347,7 @@ func (r *LaunchRequest) RepoSpecs() []RepoLaunchSpec { RepositoryPath: r.RepositoryPath, RepoName: r.RepoName, BaseBranch: r.BaseBranch, + DefaultBranch: r.DefaultBranch, CheckoutBranch: r.CheckoutBranch, WorktreeID: r.WorktreeID, WorktreeBranchPrefix: r.WorktreeBranchPrefix, diff --git a/apps/backend/internal/mcp/handlers/handlers.go b/apps/backend/internal/mcp/handlers/handlers.go index 9c6601cbd3..eaf99fc819 100644 --- a/apps/backend/internal/mcp/handlers/handlers.go +++ b/apps/backend/internal/mcp/handlers/handlers.go @@ -450,7 +450,7 @@ func (h *Handlers) resolveTaskRepositories( parentID, sourceTaskID string, explicit []mcpRepositoryInput, ) (taskRepoResult, error) { - explicitRepos := explicitRepoInputs(explicit) + explicitRepos := h.explicitRepoInputsWithDefaults(ctx, explicit) if parentID != "" { parent, err := h.taskSvc.GetTask(ctx, parentID) @@ -504,27 +504,47 @@ func (h *Handlers) resolveTaskRepositories( return taskRepoResult{}, nil } -// explicitRepoInputs maps the MCP-side explicit repo list to service inputs. -// Returns nil when no explicit repos were supplied so callers can distinguish -// "agent didn't pass repos" from "agent passed an empty list". -func explicitRepoInputs(explicit []mcpRepositoryInput) []service.TaskRepositoryInput { +// explicitRepoInputsWithDefaults maps the MCP-side explicit repo list to +// service inputs. Returns nil when no explicit repos were supplied so callers +// can distinguish "agent didn't pass repos" from "agent passed an empty list". +// +// When an explicit entry pins a repository_id without a base_branch, the +// repository's default_branch is filled in. This anchors cross-repo subtasks +// (a parent on feature/foo creating a child in another repo) to a known-good +// branch instead of an empty value that would force every downstream consumer +// to recompute the default. +func (h *Handlers) explicitRepoInputsWithDefaults(ctx context.Context, explicit []mcpRepositoryInput) []service.TaskRepositoryInput { if len(explicit) == 0 { return nil } repos := make([]service.TaskRepositoryInput, 0, len(explicit)) for _, r := range explicit { + baseBranch := r.BaseBranch + if baseBranch == "" && r.RepositoryID != "" && h.taskSvc != nil { + if repo, err := h.taskSvc.GetRepository(ctx, r.RepositoryID); err == nil && repo != nil { + baseBranch = repo.DefaultBranch + } + } repos = append(repos, service.TaskRepositoryInput{ RepositoryID: r.RepositoryID, LocalPath: r.LocalPath, GitHubURL: r.GitHubURL, - BaseBranch: r.BaseBranch, + BaseBranch: baseBranch, }) } return repos } // inheritedRepoInputs maps an existing task's repository list onto service -// inputs for a new task that inherits from it. +// inputs for a new task that inherits from it. RepositoryID and BaseBranch +// carry over so a same-repo subtask branches off the same point as the +// parent (sibling branches off the same base, ergonomically aligned for +// stacked PRs). CheckoutBranch is dropped on purpose: two worktrees cannot +// share a working branch, so the subtask's session generates a fresh one. +// Agents that need a different base for a same-repo subtask must pass +// base_branch explicitly. If the inherited base_branch is missing on the +// remote at launch time, the worktree manager's fallback recovers to the +// repository's default_branch and surfaces a warning. func inheritedRepoInputs(src []*models.TaskRepository) []service.TaskRepositoryInput { if len(src) == 0 { return nil @@ -535,9 +555,8 @@ func inheritedRepoInputs(src []*models.TaskRepository) []service.TaskRepositoryI continue } repos = append(repos, service.TaskRepositoryInput{ - RepositoryID: r.RepositoryID, - BaseBranch: r.BaseBranch, - CheckoutBranch: r.CheckoutBranch, + RepositoryID: r.RepositoryID, + BaseBranch: r.BaseBranch, }) } return repos diff --git a/apps/backend/internal/mcp/handlers/handlers_test.go b/apps/backend/internal/mcp/handlers/handlers_test.go index 3ac93bfcbd..f9aa04e72d 100644 --- a/apps/backend/internal/mcp/handlers/handlers_test.go +++ b/apps/backend/internal/mcp/handlers/handlers_test.go @@ -266,26 +266,37 @@ func TestResolveTaskRepositories_NoInputs_ReturnsEmpty(t *testing.T) { // --- Integration tests using real task service --- // seedParentWithRepo creates a workspace, workflow, repository, and a parent -// task linked to that repository. Returns the parent task ID. +// task linked to that repository. Returns the parent task ID. The parent's +// task_repository row is anchored to a non-default branch ("pr-metrics") on +// purpose so inheritance tests can assert what subtasks do with the parent's +// branch (same-repo subtasks inherit it for stacked-PR ergonomics; the +// worktree manager's fallback rescues launches if the branch went stale). func seedParentWithRepo(t *testing.T, svc *service.Service, repo *sqliterepo.Repository) string { t.Helper() ctx := context.Background() require.NoError(t, repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-1", Name: "Test"})) require.NoError(t, repo.CreateWorkflow(ctx, &models.Workflow{ID: "wf-1", WorkspaceID: "ws-1", Name: "Board"})) - require.NoError(t, repo.CreateRepository(ctx, &models.Repository{ID: "repo-parent", WorkspaceID: "ws-1", Name: "Parent Repo"})) + require.NoError(t, repo.CreateRepository(ctx, &models.Repository{ + ID: "repo-parent", WorkspaceID: "ws-1", Name: "Parent Repo", DefaultBranch: "main", + })) parent, err := svc.CreateTask(ctx, &service.CreateTaskRequest{ WorkspaceID: "ws-1", WorkflowID: "wf-1", Title: "Parent task", Repositories: []service.TaskRepositoryInput{ - {RepositoryID: "repo-parent", BaseBranch: "main"}, + {RepositoryID: "repo-parent", BaseBranch: "pr-metrics"}, }, }) require.NoError(t, err) return parent.ID } -func TestResolveTaskRepositories_ParentWithoutExplicitRepos_InheritsAll(t *testing.T) { +// TestResolveTaskRepositories_ParentWithoutExplicitRepos_InheritsRepoAndBaseBranch +// asserts the same-repo subtask path: the parent's RepositoryID and +// BaseBranch carry over so the subtask branches off the same starting point +// (sibling branches, ergonomic for stacked PRs). CheckoutBranch is dropped +// because two worktrees cannot share a working branch. +func TestResolveTaskRepositories_ParentWithoutExplicitRepos_InheritsRepoAndBaseBranch(t *testing.T) { svc, repo := newTestTaskService(t) parentID := seedParentWithRepo(t, svc, repo) @@ -296,10 +307,79 @@ func TestResolveTaskRepositories_ParentWithoutExplicitRepos_InheritsAll(t *testi require.NoError(t, err) require.Len(t, result.Repos, 1, "subtask without explicit repos inherits parent's repos") assert.Equal(t, "repo-parent", result.Repos[0].RepositoryID) + assert.Equal(t, "pr-metrics", result.Repos[0].BaseBranch, "same-repo subtask should inherit parent's base_branch for stacked-PR ergonomics") + assert.Empty(t, result.Repos[0].CheckoutBranch, "subtask must not inherit parent's checkout_branch (worktrees cannot share a branch)") assert.Equal(t, "ws-1", result.WorkspaceID) assert.Equal(t, "wf-1", result.WorkflowID) } +// TestCreateSubtaskFromParent_SameRepoInheritsParentBaseBranch is the +// end-to-end check that the parent's base_branch is persisted onto the +// subtask's task_repository row when the subtask targets the same repo. +// This is the desired behaviour: agents stacking work on top of a parent +// PR get a subtask anchored to the same starting point. The worktree +// manager's fallback (covered in worktree tests) is the safety net for +// cases where the inherited branch later goes stale. +func TestCreateSubtaskFromParent_SameRepoInheritsParentBaseBranch(t *testing.T) { + svc, repo := newTestTaskService(t) + ctx := context.Background() + parentID := seedParentWithRepo(t, svc, repo) + + log := testLogger(t) + h := &Handlers{taskSvc: svc, logger: log.WithFields()} + + resolved, err := h.resolveTaskRepositories(ctx, parentID, "", nil) + require.NoError(t, err) + + subtask, err := svc.CreateTask(ctx, &service.CreateTaskRequest{ + WorkspaceID: resolved.WorkspaceID, + WorkflowID: resolved.WorkflowID, + ParentID: parentID, + Title: "Child", + Description: "do the thing", + Repositories: resolved.Repos, + }) + require.NoError(t, err) + require.Len(t, subtask.Repositories, 1) + assert.Equal(t, "repo-parent", subtask.Repositories[0].RepositoryID) + assert.Equal(t, "pr-metrics", subtask.Repositories[0].BaseBranch, "same-repo subtask should inherit parent's base_branch") +} + +// TestCreateSubtaskFromParent_DifferentRepoUsesNewRepoDefault verifies the +// cross-repo path: when the agent points the subtask at a different repo +// (via repository_id / repository_url / local_path) without an explicit +// base_branch, the subtask anchors to that repo's default_branch — never +// the parent's branch. +func TestCreateSubtaskFromParent_DifferentRepoUsesNewRepoDefault(t *testing.T) { + svc, repo := newTestTaskService(t) + ctx := context.Background() + parentID := seedParentWithRepo(t, svc, repo) + + require.NoError(t, repo.CreateRepository(ctx, &models.Repository{ + ID: "repo-sibling", WorkspaceID: "ws-1", Name: "Sibling", DefaultBranch: "trunk", + })) + + log := testLogger(t) + h := &Handlers{taskSvc: svc, logger: log.WithFields()} + + explicit := []mcpRepositoryInput{{RepositoryID: "repo-sibling"}} + resolved, err := h.resolveTaskRepositories(ctx, parentID, "", explicit) + require.NoError(t, err) + + subtask, err := svc.CreateTask(ctx, &service.CreateTaskRequest{ + WorkspaceID: resolved.WorkspaceID, + WorkflowID: resolved.WorkflowID, + ParentID: parentID, + Title: "Cross-repo child", + Description: "do the thing", + Repositories: resolved.Repos, + }) + require.NoError(t, err) + require.Len(t, subtask.Repositories, 1) + assert.Equal(t, "repo-sibling", subtask.Repositories[0].RepositoryID) + assert.Equal(t, "trunk", subtask.Repositories[0].BaseBranch, "cross-repo subtask should anchor to the new repo's default_branch, not parent's pr-metrics") +} + func TestResolveTaskRepositories_ParentWithExplicitRepos_OverridesRepoButInheritsWorkspace(t *testing.T) { svc, repo := newTestTaskService(t) parentID := seedParentWithRepo(t, svc, repo) diff --git a/apps/backend/internal/mcp/server/config_handlers.go b/apps/backend/internal/mcp/server/config_handlers.go index 268c86f13f..8a5ad17de6 100644 --- a/apps/backend/internal/mcp/server/config_handlers.go +++ b/apps/backend/internal/mcp/server/config_handlers.go @@ -28,7 +28,7 @@ func (s *Server) registerConfigWorkflowTools() { ) s.mcpServer.AddTool( mcp.NewTool("list_repositories_kandev", - mcp.WithDescription("List repositories in a workspace. Use this to find a repository_id to attach to create_task_kandev when the task should target a specific codebase."), + mcp.WithDescription("List repositories in a workspace. Use this to find a repository_id to attach to create_task_kandev when the task should target a specific codebase. Each repository entry includes default_branch — use it to pick or confirm a base_branch when creating a subtask in a different repo."), mcp.WithString("workspace_id", mcp.Required(), mcp.Description("The workspace ID")), ), s.wrapHandler("list_repositories_kandev", s.listRepositoriesHandler()), diff --git a/apps/backend/internal/mcp/server/server.go b/apps/backend/internal/mcp/server/server.go index 5fa83def93..be3ac1de70 100644 --- a/apps/backend/internal/mcp/server/server.go +++ b/apps/backend/internal/mcp/server/server.go @@ -444,6 +444,10 @@ WHEN TO OMIT parent_id (top-level task): IMPORTANT: - Subtasks inherit workspace, workflow, agent profile, and executor from the parent - Subtasks inherit the parent's repository unless you supply repository_url, repository_id, or local_path — in which case the subtask targets that repo instead (must live in the parent's workspace) +- base_branch behaviour: + - Same repo as parent (no repo args): subtask inherits the parent's base_branch (sibling branches off the same starting point — useful for PR stacks) + - Different repo (you passed repository_url / repository_id / local_path): subtask defaults to that repo's default_branch + - Pass base_branch explicitly to override either default. Use list_repositories_kandev to see each repo's default_branch. - Top-level tasks need a repository via repository_url, repository_id, or local_path - 'description' is the sub-agent's initial prompt — be specific and detailed - Set start_agent=false to create without starting an agent` @@ -476,7 +480,7 @@ IMPORTANT: mcp.WithString("repository_id", mcp.Description("Repository ID. Required for top-level tasks unless local_path or repository_url is provided. For subtasks: optional — supply only when the subtask should target a different repo than the parent.")), mcp.WithString("local_path", mcp.Description("Local repository folder path (e.g. '/Users/me/projects/myrepo'). Will create/find the repository automatically. Preferred for local worktree flow. For subtasks: supply only when the subtask should target a different repo than the parent.")), mcp.WithString("repository_url", mcp.Description("GitHub repository URL (e.g. 'https://github.com/owner/repo'). The repository will be cloned automatically on first use. For subtasks: supply only when the subtask should target a different repo than the parent.")), - mcp.WithString("base_branch", mcp.Description("Base branch for the repository (e.g. 'main'). Optional, defaults to repository's default branch.")), + mcp.WithString("base_branch", mcp.Description("Base branch for the repository (e.g. 'main'). Optional. Defaults: same-repo subtasks inherit the parent's base_branch; cross-repo subtasks and top-level tasks fall back to the repository's default_branch (visible via list_repositories_kandev).")), ), s.wrapHandler("create_task_kandev", s.createTaskHandler()), ) diff --git a/apps/backend/internal/orchestrator/executor/executor.go b/apps/backend/internal/orchestrator/executor/executor.go index 80770a62d9..31f85dcc7d 100644 --- a/apps/backend/internal/orchestrator/executor/executor.go +++ b/apps/backend/internal/orchestrator/executor/executor.go @@ -257,6 +257,7 @@ type LaunchAgentRequest struct { RepositoryID string // Repository ID for worktree tracking RepositoryPath string // Path to the main repository (for worktree creation) BaseBranch string // Base branch for the worktree (e.g., "main") + DefaultBranch string // Repository's default_branch, used as a fallback when BaseBranch is missing CheckoutBranch string // Branch to fetch and checkout after worktree creation (e.g., PR head branch) WorktreeBranchPrefix string // Branch prefix for worktree branches PullBeforeWorktree bool // Whether to pull from remote before creating the worktree @@ -282,6 +283,7 @@ type RepoSpec struct { RepositoryURL string RepoName string BaseBranch string + DefaultBranch string // Repository's default_branch, used as fallback when BaseBranch is missing CheckoutBranch string WorktreeID string WorktreeBranchPrefix string diff --git a/apps/backend/internal/orchestrator/executor/executor_execute.go b/apps/backend/internal/orchestrator/executor/executor_execute.go index e9b7bf207a..2d5362e9ea 100644 --- a/apps/backend/internal/orchestrator/executor/executor_execute.go +++ b/apps/backend/internal/orchestrator/executor/executor_execute.go @@ -639,6 +639,7 @@ func buildRepoSpecs(allRepos []*repoInfo) []RepoSpec { spec.RepoName = info.Repository.Name spec.RepoSetupScript = info.Repository.SetupScript spec.RepoCleanupScript = info.Repository.CleanupScript + spec.DefaultBranch = info.Repository.DefaultBranch } // Containerized executors need a clone URL; reuse the same helper as // the single-repo path (best-effort — skipped if Repository is nil). @@ -661,6 +662,9 @@ func (e *Executor) applyRepositoryConfig(req *LaunchAgentRequest, task *v1.Task, req.CheckoutBranch = repoInfo.CheckoutBranch req.WorktreeBranchPrefix = repoInfo.WorktreeBranchPrefix req.PullBeforeWorktree = repoInfo.PullBeforeWorktree + if repoInfo.Repository != nil { + req.DefaultBranch = repoInfo.Repository.DefaultBranch + } // Task directory mode: place worktree inside per-task directory if req.UseWorktree && repoInfo.Repository != nil && repoInfo.Repository.Name != "" { req.TaskDirName = worktree.SemanticWorktreeName(task.Title, worktree.SmallSuffix(3)) diff --git a/apps/backend/internal/worktree/manager.go b/apps/backend/internal/worktree/manager.go index 327fb81643..e59d0810cd 100644 --- a/apps/backend/internal/worktree/manager.go +++ b/apps/backend/internal/worktree/manager.go @@ -274,9 +274,44 @@ func (m *Manager) Create(ctx context.Context, req CreateRequest) (*Worktree, err baseRef = m.pullBaseBranch(ctx, req.RepositoryPath, req.BaseBranch, req.OnSyncProgress) } - // Check base branch exists + // Check base branch exists. When the requested branch is missing and the + // caller supplied a FallbackBaseBranch (typically the repo's default_branch) + // that does exist, recover automatically and surface a non-fatal warning on + // the resulting worktree. This handles the common case where a parent task + // was anchored to a PR head or fresh branch that has been deleted upstream + // or never existed in a sibling repo. + fallbackWarning, fallbackDetail := "", "" if !m.branchExists(ctx, req.RepositoryPath, baseRef) { - return nil, fmt.Errorf("%w: %s", ErrInvalidBaseBranch, baseRef) + fallback := strings.TrimSpace(req.FallbackBaseBranch) + if fallback == "" || fallback == baseRef { + return nil, fmt.Errorf("%w: %s", ErrInvalidBaseBranch, baseRef) + } + // Best-effort fetch of the fallback so it is available locally in + // containerized / shallow-clone environments where the fallback may + // only exist on the remote. pullBaseBranch may resolve the name to a + // remote-tracking ref (e.g. "main" -> "origin/main") which we must use + // for the existence check and downstream git operations. + resolvedFallback := fallback + if req.PullBeforeWorktree { + resolvedFallback = m.pullBaseBranch(ctx, req.RepositoryPath, fallback, nil) + } + if !m.branchExists(ctx, req.RepositoryPath, resolvedFallback) { + return nil, fmt.Errorf("%w: %s (fallback %q also not found)", ErrInvalidBaseBranch, baseRef, fallback) + } + m.logger.Warn("requested base branch not found, falling back", + zap.String("repository_path", req.RepositoryPath), + zap.String("requested_branch", baseRef), + zap.String("fallback_branch", fallback)) + // Use req.BaseBranch (the user-supplied name) in the user-facing warning + // rather than baseRef, which may carry an internal "origin/" form + // produced by pullBaseBranch when PullBeforeWorktree is set. + fallbackWarning = fmt.Sprintf("Requested base branch %q not found, used %q instead", req.BaseBranch, fallback) + fallbackDetail = fmt.Sprintf("git rev-parse --verify %s failed; recovered using fallback branch %q (typically the repository's default_branch)", baseRef, fallback) + baseRef = resolvedFallback + // Reflect the resolved branch in the persisted worktree record so + // downstream consumers (UI, queries, debug logs) see the actual base + // rather than the requested-but-missing one. + req.BaseBranch = fallback } // Worktrees are always placed under ~/.kandev/tasks/{taskDir}/{repo}/. @@ -285,7 +320,15 @@ func (m *Manager) Create(ctx context.Context, req CreateRequest) (*Worktree, err if req.TaskDirName == "" || req.RepoName == "" { return nil, ErrTaskDirRequired } - return m.createInTaskDir(ctx, req, baseRef) + wt, err := m.createInTaskDir(ctx, req, baseRef) + if err != nil { + return nil, err + } + if fallbackWarning != "" { + wt.BaseBranchFallbackWarning = fallbackWarning + wt.BaseBranchFallbackDetail = fallbackDetail + } + return wt, nil } // createInTaskDir creates a worktree inside the task directory structure: diff --git a/apps/backend/internal/worktree/manager_base_branch_fallback_test.go b/apps/backend/internal/worktree/manager_base_branch_fallback_test.go new file mode 100644 index 0000000000..674b3e95b6 --- /dev/null +++ b/apps/backend/internal/worktree/manager_base_branch_fallback_test.go @@ -0,0 +1,142 @@ +package worktree + +import ( + "context" + "errors" + "strings" + "testing" +) + +// TestCreateWorktree_BaseBranchFallback_RecoversWhenRequestedMissing covers +// the regression where a subtask inherited a stale base_branch (e.g. the +// parent's PR head) that did not exist in a freshly inherited worktree. With +// FallbackBaseBranch populated (typically the repo's default_branch) the +// manager retries with the fallback and surfaces a non-fatal warning instead +// of failing the launch. +func TestCreateWorktree_BaseBranchFallback_RecoversWhenRequestedMissing(t *testing.T) { + cfg := newTestConfig(t) + mgr, err := NewManager(cfg, newMockStore(), newTestLogger()) + if err != nil { + t.Fatalf("NewManager failed: %v", err) + } + repoPath := initGitRepoForWorktreeTest(t) + + wt, err := mgr.Create(context.Background(), CreateRequest{ + TaskID: "task-1", + SessionID: "session-1", + TaskTitle: "Inherits stale branch", + RepositoryID: "repo-1", + RepositoryPath: repoPath, + BaseBranch: "pr-metrics", + FallbackBaseBranch: "main", + TaskDirName: "task-1", + RepoName: "repo-1", + }) + if err != nil { + t.Fatalf("Create() unexpected error: %v", err) + } + if wt.BaseBranch != "main" { + t.Fatalf("worktree BaseBranch = %q, want %q", wt.BaseBranch, "main") + } + if wt.BaseBranchFallbackWarning == "" { + t.Fatal("expected BaseBranchFallbackWarning to be set when fallback was used") + } + if !strings.Contains(wt.BaseBranchFallbackWarning, "pr-metrics") { + t.Fatalf("warning %q does not mention requested branch", wt.BaseBranchFallbackWarning) + } + if !strings.Contains(wt.BaseBranchFallbackWarning, "main") { + t.Fatalf("warning %q does not mention fallback branch", wt.BaseBranchFallbackWarning) + } + if wt.BaseBranchFallbackDetail == "" { + t.Fatal("expected BaseBranchFallbackDetail to be set when fallback was used") + } +} + +// TestCreateWorktree_BaseBranchFallback_NoFallbackProvided keeps the legacy +// loud failure when the caller did not supply a FallbackBaseBranch — we must +// not silently invent a branch out of thin air. +func TestCreateWorktree_BaseBranchFallback_NoFallbackProvided(t *testing.T) { + cfg := newTestConfig(t) + mgr, err := NewManager(cfg, newMockStore(), newTestLogger()) + if err != nil { + t.Fatalf("NewManager failed: %v", err) + } + repoPath := initGitRepoForWorktreeTest(t) + + _, err = mgr.Create(context.Background(), CreateRequest{ + TaskID: "task-1", + SessionID: "session-1", + RepositoryID: "repo-1", + RepositoryPath: repoPath, + BaseBranch: "pr-metrics", + TaskDirName: "task-1", + RepoName: "repo-1", + }) + if !errors.Is(err, ErrInvalidBaseBranch) { + t.Fatalf("Create() err = %v, want ErrInvalidBaseBranch", err) + } +} + +// TestCreateWorktree_BaseBranchFallback_FallbackAlsoMissing surfaces the +// original error when neither branch exists — the fallback only helps when it +// actually exists in the repository. +func TestCreateWorktree_BaseBranchFallback_FallbackAlsoMissing(t *testing.T) { + cfg := newTestConfig(t) + mgr, err := NewManager(cfg, newMockStore(), newTestLogger()) + if err != nil { + t.Fatalf("NewManager failed: %v", err) + } + repoPath := initGitRepoForWorktreeTest(t) + + _, err = mgr.Create(context.Background(), CreateRequest{ + TaskID: "task-1", + SessionID: "session-1", + RepositoryID: "repo-1", + RepositoryPath: repoPath, + BaseBranch: "pr-metrics", + FallbackBaseBranch: "release", // also missing in this repo + TaskDirName: "task-1", + RepoName: "repo-1", + }) + if !errors.Is(err, ErrInvalidBaseBranch) { + t.Fatalf("Create() err = %v, want ErrInvalidBaseBranch", err) + } + // The error must name both branches so an operator can tell at a glance + // that a fallback was attempted and also failed. + msg := err.Error() + if !strings.Contains(msg, "pr-metrics") || !strings.Contains(msg, "release") { + t.Fatalf("error %q should mention both requested and fallback branches", msg) + } +} + +// TestCreateWorktree_BaseBranchFallback_NotUsedWhenBaseExists guards against +// the fallback being applied unnecessarily — when the requested branch is +// valid the worktree uses it and no warning is emitted. +func TestCreateWorktree_BaseBranchFallback_NotUsedWhenBaseExists(t *testing.T) { + cfg := newTestConfig(t) + mgr, err := NewManager(cfg, newMockStore(), newTestLogger()) + if err != nil { + t.Fatalf("NewManager failed: %v", err) + } + repoPath := initGitRepoForWorktreeTest(t) + + wt, err := mgr.Create(context.Background(), CreateRequest{ + TaskID: "task-1", + SessionID: "session-1", + RepositoryID: "repo-1", + RepositoryPath: repoPath, + BaseBranch: "main", + FallbackBaseBranch: "feature/pr-branch", // exists, but should never be used + TaskDirName: "task-1", + RepoName: "repo-1", + }) + if err != nil { + t.Fatalf("Create() unexpected error: %v", err) + } + if wt.BaseBranch != "main" { + t.Fatalf("worktree BaseBranch = %q, want %q", wt.BaseBranch, "main") + } + if wt.BaseBranchFallbackWarning != "" { + t.Fatalf("expected no fallback warning, got %q", wt.BaseBranchFallbackWarning) + } +} diff --git a/apps/backend/internal/worktree/worktree.go b/apps/backend/internal/worktree/worktree.go index 3ff00e3d49..c592800cb5 100644 --- a/apps/backend/internal/worktree/worktree.go +++ b/apps/backend/internal/worktree/worktree.go @@ -72,6 +72,17 @@ type Worktree struct { // FetchWarningDetail contains the raw git command output for debugging. // Shown as collapsible content alongside the user-friendly FetchWarning. FetchWarningDetail string `json:"fetch_warning_detail,omitempty"` + + // BaseBranchFallbackWarning is set when the requested BaseBranch did not + // exist in the repository and the worktree was created from a fallback + // branch (typically the repository's default_branch) instead. Empty when + // the original BaseBranch was used. + BaseBranchFallbackWarning string `json:"base_branch_fallback_warning,omitempty"` + + // BaseBranchFallbackDetail mirrors FetchWarningDetail: a longer message + // describing why the original branch was not used. Surfaced as collapsible + // detail alongside BaseBranchFallbackWarning. + BaseBranchFallbackDetail string `json:"base_branch_fallback_detail,omitempty"` } // CreateRequest contains the parameters for creating a new worktree. @@ -97,6 +108,12 @@ type CreateRequest struct { // Typically "main" or "master". BaseBranch string + // FallbackBaseBranch is an optional branch to retry with when BaseBranch + // does not exist in the repository. Typically populated with the + // repository's default_branch by the caller. When empty, a missing + // BaseBranch returns ErrInvalidBaseBranch as before. + FallbackBaseBranch string + // 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. diff --git a/apps/web/e2e/helpers/api-client.ts b/apps/web/e2e/helpers/api-client.ts index def00be491..f0f2719546 100644 --- a/apps/web/e2e/helpers/api-client.ts +++ b/apps/web/e2e/helpers/api-client.ts @@ -108,25 +108,36 @@ function buildCreateTaskBody( return body; } -/** Build the optional fields object for createTaskWithAgent requests. */ -function buildOptionalAgentTaskFields(opts?: { +type OptionalAgentTaskOpts = { workflow_id?: string; workflow_step_id?: string; repository_ids?: string[]; + repositories?: Array<{ repository_id: string; base_branch?: string; checkout_branch?: string }>; executor_id?: string; executor_profile_id?: string; metadata?: Record; parent_id?: string; -}): Record { +}; + +/** `repositories` (with per-entry branches) takes precedence over the shorthand + * `repository_ids` so callers can pin a non-default base_branch. */ +function pickRepositories(opts: OptionalAgentTaskOpts): unknown { + if (opts.repositories) return opts.repositories; + if (opts.repository_ids) return opts.repository_ids.map((id) => ({ repository_id: id })); + return undefined; +} + +/** Build the optional fields object for createTaskWithAgent requests. */ +function buildOptionalAgentTaskFields(opts?: OptionalAgentTaskOpts): Record { const fields: Record = {}; - if (opts?.workflow_id) fields.workflow_id = opts.workflow_id; - if (opts?.workflow_step_id) fields.workflow_step_id = opts.workflow_step_id; - if (opts?.repository_ids) - fields.repositories = opts.repository_ids.map((id) => ({ repository_id: id })); - if (opts?.executor_id) fields.executor_id = opts.executor_id; - if (opts?.executor_profile_id) fields.executor_profile_id = opts.executor_profile_id; - if (opts?.metadata) fields.metadata = opts.metadata; - if (opts?.parent_id) fields.parent_id = opts.parent_id; + if (!opts) return fields; + setIf(fields, "workflow_id", opts.workflow_id); + setIf(fields, "workflow_step_id", opts.workflow_step_id); + setIf(fields, "repositories", pickRepositories(opts)); + setIf(fields, "executor_id", opts.executor_id); + setIf(fields, "executor_profile_id", opts.executor_profile_id); + setIf(fields, "metadata", opts.metadata); + setIf(fields, "parent_id", opts.parent_id); return fields; } @@ -320,6 +331,12 @@ export class ApiClient { workflow_id?: string; workflow_step_id?: string; repository_ids?: string[]; + /** Full repository entries with optional checkout_branch / base_branch. */ + repositories?: Array<{ + repository_id: string; + base_branch?: string; + checkout_branch?: string; + }>; executor_id?: string; executor_profile_id?: string; metadata?: Record; diff --git a/apps/web/e2e/tests/task/subtask.spec.ts b/apps/web/e2e/tests/task/subtask.spec.ts index 5469d4a5b0..7c72699120 100644 --- a/apps/web/e2e/tests/task/subtask.spec.ts +++ b/apps/web/e2e/tests/task/subtask.spec.ts @@ -12,7 +12,7 @@ const DONE_STATES = ["COMPLETED", "WAITING_FOR_INPUT"]; type TaskWithRepos = { id: string; - repositories?: Array<{ repository_id: string }>; + repositories?: Array<{ repository_id: string; base_branch?: string }>; }; type SessionInfo = { @@ -440,6 +440,167 @@ test.describe("MCP subtask creation", () => { ).toContain(otherRepo.id); expect(repoIds).not.toContain(seedData.repositoryId); }); + + test("MCP-created same-repo subtask inherits parent's base_branch (stacked PR ergonomics)", async ({ + testPage, + apiClient, + seedData, + backend, + }) => { + test.setTimeout(90_000); + + // 1. Set up a repo with a non-default branch the parent will be pinned to. + // A same-repo subtask should inherit that branch so its work stacks on + // the same starting point as the parent (sibling PRs off the same + // base, useful for stacked PR workflows). + const repoDir = path.join(backend.tmpDir, "repos", "subtask-same-repo-inherit"); + fs.mkdirSync(repoDir, { recursive: true }); + const gitEnv = makeGitEnv(backend.tmpDir); + execSync("git init -b main", { cwd: repoDir, env: gitEnv }); + execSync('git commit --allow-empty -m "init"', { cwd: repoDir, env: gitEnv }); + execSync("git branch feature/parent-branch", { cwd: repoDir, env: gitEnv }); + const repo = await apiClient.createRepository(seedData.workspaceId, repoDir, "main", { + name: "Subtask Same Repo Inherit", + }); + + const subtaskTitle = `Same-Repo Subtask ${Date.now()}`; + const script = [ + 'e2e:thinking("Creating subtask...")', + "e2e:delay(100)", + `e2e:mcp:kandev:create_task_kandev({"parent_id":"self","title":"${subtaskTitle}","description":"E2E subtask: verify base_branch is inherited from parent in same repo"})`, + "e2e:delay(100)", + 'e2e:message("Done.")', + ].join("\n"); + + // 2. Create the parent pinned to a non-default branch. + const parentTask = await apiClient.createTaskWithAgent( + seedData.workspaceId, + "Same-Repo Parent Task", + seedData.agentProfileId, + { + description: script, + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repositories: [{ repository_id: repo.id, base_branch: "feature/parent-branch" }], + }, + ); + + // Sanity: parent persisted the pin. + const parentRaw = await apiClient.rawRequest("GET", `/api/v1/tasks/${parentTask.id}`); + const parentData = (await parentRaw.json()) as TaskWithRepos; + expect(parentData.repositories?.[0]?.base_branch).toBe("feature/parent-branch"); + + // 3. Open the parent so the execution stays alive while the script runs. + await testPage.goto(`/t/${parentTask.id}`); + const session = new SessionPage(testPage); + await session.waitForLoad(); + + // 4. Poll for the subtask. + type TaskEntry = { id: string; title: string }; + let subtask: TaskEntry | undefined; + await expect + .poll( + async () => { + const allTasks = await apiClient.listTasks(seedData.workspaceId); + subtask = allTasks.tasks.find((t: TaskEntry) => t.title === subtaskTitle); + return subtask; + }, + { timeout: 60_000, message: "Subtask should be created by the agent's MCP call" }, + ) + .toBeDefined(); + + // 5. The same-repo subtask should inherit the parent's base_branch so the + // agent can stack work on top of the parent's PR. The worktree + // manager's fallback (covered separately in worktree unit tests) is + // the safety net if the inherited branch later goes stale on the + // remote — it is not the primary mechanism. + const subtaskRaw = await apiClient.rawRequest("GET", `/api/v1/tasks/${subtask!.id}`); + const subtaskData = (await subtaskRaw.json()) as TaskWithRepos; + expect( + subtaskData.repositories?.[0]?.repository_id, + "same-repo subtask should inherit the parent's repository_id", + ).toBe(repo.id); + expect( + subtaskData.repositories?.[0]?.base_branch, + `same-repo subtask base_branch should inherit parent's 'feature/parent-branch'; got: ${subtaskData.repositories?.[0]?.base_branch}`, + ).toBe("feature/parent-branch"); + }); + + test("MCP-created cross-repo subtask uses the new repo's default_branch", async ({ + testPage, + apiClient, + seedData, + backend, + }) => { + test.setTimeout(90_000); + + // Parent lives in the seed repo on a non-default branch. The subtask + // explicitly retargets a different repo via repository_id and supplies + // no base_branch. The subtask must anchor to the new repo's + // default_branch ("trunk") and never carry the parent's branch over. + const otherRepoDir = path.join(backend.tmpDir, "repos", "subtask-cross-repo"); + fs.mkdirSync(otherRepoDir, { recursive: true }); + const gitEnv = makeGitEnv(backend.tmpDir); + execSync("git init -b trunk", { cwd: otherRepoDir, env: gitEnv }); + execSync('git commit --allow-empty -m "init"', { cwd: otherRepoDir, env: gitEnv }); + const otherRepo = await apiClient.createRepository( + seedData.workspaceId, + otherRepoDir, + "trunk", + { + name: "Subtask Cross Repo", + }, + ); + + const subtaskTitle = `Cross-Repo Subtask ${Date.now()}`; + const script = [ + 'e2e:thinking("Creating cross-repo subtask...")', + "e2e:delay(100)", + `e2e:mcp:kandev:create_task_kandev({"parent_id":"self","title":"${subtaskTitle}","description":"E2E cross-repo subtask","repository_id":"${otherRepo.id}"})`, + "e2e:delay(100)", + 'e2e:message("Done.")', + ].join("\n"); + + const parentTask = await apiClient.createTaskWithAgent( + seedData.workspaceId, + "Cross-Repo Parent Task", + seedData.agentProfileId, + { + description: script, + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }, + ); + + await testPage.goto(`/t/${parentTask.id}`); + const session = new SessionPage(testPage); + await session.waitForLoad(); + + type TaskEntry = { id: string; title: string }; + let subtask: TaskEntry | undefined; + await expect + .poll( + async () => { + const allTasks = await apiClient.listTasks(seedData.workspaceId); + subtask = allTasks.tasks.find((t: TaskEntry) => t.title === subtaskTitle); + return subtask; + }, + { + timeout: 60_000, + message: "Cross-repo subtask should be created by the agent's MCP call", + }, + ) + .toBeDefined(); + + const subtaskRaw = await apiClient.rawRequest("GET", `/api/v1/tasks/${subtask!.id}`); + const subtaskData = (await subtaskRaw.json()) as TaskWithRepos; + expect(subtaskData.repositories?.[0]?.repository_id).toBe(otherRepo.id); + expect( + subtaskData.repositories?.[0]?.base_branch, + `cross-repo subtask base_branch should anchor to new repo's default 'trunk'; got: ${subtaskData.repositories?.[0]?.base_branch}`, + ).toBe("trunk"); + }); }); /**