From d705b3c2f308a275211770ca495880e349da8db4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Mon, 27 Jul 2026 22:08:18 +0100 Subject: [PATCH] fix(mcp): restore live add-branch worktrees (#1990) * fix(mcp): restore live add-branch worktrees * fix(mcp): reject deferred live branch additions * docs(mcp): clarify live branch response contract --- .../backendapp/branch_materializer.go | 15 +- .../backendapp/branch_materializer_test.go | 10 +- .../backend/internal/mcp/handlers/handlers.go | 24 +- .../internal/mcp/handlers/handlers_test.go | 35 +++ .../internal/mcp/server/handlers_test.go | 9 +- apps/backend/internal/mcp/server/server.go | 2 +- apps/backend/internal/task/service/service.go | 11 +- .../internal/task/service/service_branches.go | 53 +++-- .../task/service/service_branches_test.go | 75 ++++++- .../task/service/service_workspace_sources.go | 3 + ...-runtime-mutable-task-workspace-sources.md | 30 ++- ...026-07-27-legacy-add-branch-live-rescan.md | 68 ++++++ docs/decisions/INDEX.md | 3 +- docs/plans/restore-live-add-branch/plan.md | 210 ++++++++++++++++++ .../task-01-restore-live-materialization.md | 74 ++++++ .../task-02-return-materialized-paths.md | 68 ++++++ .../task-03-update-public-docs.md | 52 +++++ docs/public/automation-and-mcp.md | 2 +- docs/public/coordination.md | 2 +- docs/public/feature-status.md | 2 +- docs/specs/tasks/attach-workspace-sources.md | 63 +++++- 21 files changed, 740 insertions(+), 71 deletions(-) create mode 100644 docs/decisions/2026-07-27-legacy-add-branch-live-rescan.md create mode 100644 docs/plans/restore-live-add-branch/plan.md create mode 100644 docs/plans/restore-live-add-branch/task-01-restore-live-materialization.md create mode 100644 docs/plans/restore-live-add-branch/task-02-return-materialized-paths.md create mode 100644 docs/plans/restore-live-add-branch/task-03-update-public-docs.md diff --git a/apps/backend/internal/backendapp/branch_materializer.go b/apps/backend/internal/backendapp/branch_materializer.go index c6f49cfac3..a58e80aed9 100644 --- a/apps/backend/internal/backendapp/branch_materializer.go +++ b/apps/backend/internal/backendapp/branch_materializer.go @@ -81,13 +81,13 @@ func newBranchMaterializer(repo branchMaterializerRepo, mgr *worktree.Manager, l // Designed to be idempotent: the worktree manager's reuse path catches a // rerun for the same (session, repo, branch_slug) triple and returns the // existing worktree. -func (b *branchMaterializer) MaterializeBranch(ctx context.Context, taskID, taskRepositoryID string) error { +func (b *branchMaterializer) MaterializeBranch(ctx context.Context, taskID, taskRepositoryID string) (*taskservice.BranchMaterializationResult, error) { materialization, err := b.materializeUnfinalized(ctx, taskID, taskRepositoryID) if err != nil || materialization == nil { - return err + return nil, err } - b.finalize(materialization, ctx) - return nil + taskRoot := b.finalize(materialization, ctx) + return &taskservice.BranchMaterializationResult{WorktreePath: materialization.worktree.Path, TaskWorkspacePath: taskRoot}, nil } // materializeUnfinalized creates the worktree but deliberately does not @@ -118,8 +118,8 @@ func (b *branchMaterializer) materializeUnfinalized(ctx context.Context, taskID, return &branchMaterialization{environment: env, session: session, worktree: wt, repositoryID: req.RepositoryID, slug: slug, taskID: taskID}, nil } -func (b *branchMaterializer) finalize(materialization *branchMaterialization, ctx context.Context) { - b.finalizeMaterialize(ctx, materialization.environment, materialization.session, materialization.worktree, materialization.repositoryID, materialization.slug, materialization.taskID) +func (b *branchMaterializer) finalize(materialization *branchMaterialization, ctx context.Context) string { + return b.finalizeMaterialize(ctx, materialization.environment, materialization.session, materialization.worktree, materialization.repositoryID, materialization.slug, materialization.taskID) } // prepareMaterializeRequest builds the worktree.CreateRequest plus the @@ -189,7 +189,7 @@ func (b *branchMaterializer) finalizeMaterialize( session *models.TaskSession, wt *worktree.Worktree, repositoryID, slug, taskID string, -) { +) string { taskRoot := b.promoteWorkspacePathIfNeeded(ctx, env, wt.Path) b.notifyAgentctlRescan(ctx, session, taskRoot) if b.rescanner != nil { @@ -204,6 +204,7 @@ func (b *branchMaterializer) finalizeMaterialize( TaskWorkspacePath: taskRoot, }) } + return taskRoot } // promoteWorkspacePathIfNeeded switches task_environments.workspace_path diff --git a/apps/backend/internal/backendapp/branch_materializer_test.go b/apps/backend/internal/backendapp/branch_materializer_test.go index de3719df7e..80c521eca3 100644 --- a/apps/backend/internal/backendapp/branch_materializer_test.go +++ b/apps/backend/internal/backendapp/branch_materializer_test.go @@ -78,11 +78,15 @@ func TestBranchMaterializer_PromotesWorkspacePathAndTriggersRescan(t *testing.T) t.Fatalf("CreateTaskRepository: %v", err) } - if err := mat.MaterializeBranch(ctx, "task-1", tr.ID); err != nil { + materialized, err := mat.MaterializeBranch(ctx, "task-1", tr.ID) + if err != nil { t.Fatalf("MaterializeBranch: %v", err) } wantSiblingDir := filepath.Join(taskRoot, "kandev-branch-2") + if materialized == nil || materialized.WorktreePath != wantSiblingDir || materialized.TaskWorkspacePath != taskRoot { + t.Fatalf("materialization result = %#v, want sibling worktree and task root", materialized) + } if _, err := os.Stat(wantSiblingDir); err != nil { t.Fatalf("expected sibling worktree at %s: %v", wantSiblingDir, err) } @@ -147,7 +151,7 @@ func TestBranchMaterializer_SecondBranchKeepsTaskRootPromoted(t *testing.T) { if err := repoSqlite.CreateTaskRepository(ctx, tr2); err != nil { t.Fatalf("CreateTaskRepository branch-2: %v", err) } - if err := mat.MaterializeBranch(ctx, "task-1", tr2.ID); err != nil { + if _, err := mat.MaterializeBranch(ctx, "task-1", tr2.ID); err != nil { t.Fatalf("MaterializeBranch branch-2: %v", err) } @@ -159,7 +163,7 @@ func TestBranchMaterializer_SecondBranchKeepsTaskRootPromoted(t *testing.T) { if err := repoSqlite.CreateTaskRepository(ctx, tr3); err != nil { t.Fatalf("CreateTaskRepository branch-3: %v", err) } - if err := mat.MaterializeBranch(ctx, "task-1", tr3.ID); err != nil { + if _, err := mat.MaterializeBranch(ctx, "task-1", tr3.ID); err != nil { t.Fatalf("MaterializeBranch branch-3: %v", err) } diff --git a/apps/backend/internal/mcp/handlers/handlers.go b/apps/backend/internal/mcp/handlers/handlers.go index 2894e83a20..adb4773269 100644 --- a/apps/backend/internal/mcp/handlers/handlers.go +++ b/apps/backend/internal/mcp/handlers/handlers.go @@ -1343,14 +1343,22 @@ func (h *Handlers) handleAddBranchToTask(ctx context.Context, msg *ws.Message) ( code := classifyAddBranchError(err) return ws.NewError(msg.ID, msg.Action, code, "Failed to add branch: "+err.Error(), nil) } - return ws.NewResponse(msg.ID, msg.Action, map[string]interface{}{ - "id": taskRepo.ID, - keyTaskID: taskRepo.TaskID, - keyRepositoryID: taskRepo.RepositoryID, - keyBaseBranch: taskRepo.BaseBranch, - keyCheckoutBranch: taskRepo.CheckoutBranch, - keyPosition: taskRepo.Position, - }) + response := map[string]interface{}{ + "id": taskRepo.ID, + keyTaskID: taskRepo.TaskID, + keyRepositoryID: taskRepo.RepositoryID, + keyBaseBranch: taskRepo.BaseBranch, + keyCheckoutBranch: taskRepo.CheckoutBranch, + keyPosition: taskRepo.Position, + "agent_cwd_changed": taskRepo.AgentCWDChanged, + } + if taskRepo.WorktreePath != "" { + response["worktree_path"] = taskRepo.WorktreePath + } + if taskRepo.TaskWorkspacePath != "" { + response["task_workspace_path"] = taskRepo.TaskWorkspacePath + } + return ws.NewResponse(msg.ID, msg.Action, response) } // handleAddWorkspaceSources forwards the documented discriminated source diff --git a/apps/backend/internal/mcp/handlers/handlers_test.go b/apps/backend/internal/mcp/handlers/handlers_test.go index a6d49e94b7..606dc4ccbf 100644 --- a/apps/backend/internal/mcp/handlers/handlers_test.go +++ b/apps/backend/internal/mcp/handlers/handlers_test.go @@ -221,6 +221,41 @@ func TestHandleAddBranchToTask_RejectsMultipleLocators(t *testing.T) { assertWSError(t, resp, ws.ErrorCodeValidation) } +type pathBranchMaterializer struct{} + +func (pathBranchMaterializer) MaterializeBranch(context.Context, string, string) (*service.BranchMaterializationResult, error) { + return &service.BranchMaterializationResult{WorktreePath: "/task/kandev-feature-source", TaskWorkspacePath: "/task"}, nil +} + +func TestHandleAddBranchToTask_ReturnsMaterializedPaths(t *testing.T) { + ctx := context.Background() + svc, repo := newTestTaskService(t) + svc.SetWorkflowStepGetter(&staticWorkflowStepGetter{steps: map[string]*workflowmodels.WorkflowStep{ + "step": {ID: "step", WorkflowID: "wf-branch-path", Name: "Step"}, + }}) + require.NoError(t, repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-branch-path", Name: "Workspace"})) + require.NoError(t, repo.CreateWorkflow(ctx, &models.Workflow{ID: "wf-branch-path", WorkspaceID: "ws-branch-path", Name: "Workflow"})) + require.NoError(t, repo.CreateRepository(ctx, &models.Repository{ID: "repo-branch-path", WorkspaceID: "ws-branch-path", Name: "app", DefaultBranch: "main"})) + task, err := svc.CreateTask(ctx, &service.CreateTaskRequest{WorkspaceID: "ws-branch-path", WorkflowID: "wf-branch-path", WorkflowStepID: "step", Title: "Task", Repositories: []service.TaskRepositoryInput{{RepositoryID: "repo-branch-path", BaseBranch: "main"}}}) + require.NoError(t, err) + require.NoError(t, repo.CreateTaskSession(ctx, &models.TaskSession{ID: "session-branch-path", TaskID: task.ID})) + _, err = svc.StartTurn(ctx, "session-branch-path") + require.NoError(t, err) + svc.SetBranchMaterializer(pathBranchMaterializer{}) + h := &Handlers{taskSvc: svc, logger: testLogger(t).WithFields()} + + resp, err := h.handleAddBranchToTask(ctx, makeWSMessage(t, ws.ActionMCPAddBranchToTask, map[string]interface{}{ + "task_id": task.ID, "repository_id": "repo-branch-path", "base_branch": "main", "checkout_branch": "feature/source", + })) + require.NoError(t, err) + require.Equal(t, ws.MessageTypeResponse, resp.Type) + var payload map[string]interface{} + require.NoError(t, json.Unmarshal(resp.Payload, &payload)) + assert.Equal(t, "/task/kandev-feature-source", payload["worktree_path"]) + assert.Equal(t, "/task", payload["task_workspace_path"]) + assert.Equal(t, false, payload["agent_cwd_changed"]) +} + func TestHandleAddWorkspaceSourcesRejectsUnknownKind(t *testing.T) { h := &Handlers{} msg := makeWSMessage(t, ws.ActionMCPAddWorkspaceSources, map[string]interface{}{ diff --git a/apps/backend/internal/mcp/server/handlers_test.go b/apps/backend/internal/mcp/server/handlers_test.go index bfcae402b9..bcda16576c 100644 --- a/apps/backend/internal/mcp/server/handlers_test.go +++ b/apps/backend/internal/mcp/server/handlers_test.go @@ -397,7 +397,9 @@ func TestCreateTask_LocalPath_AllowedForSubtasks(t *testing.T) { // can resolve through the same code path. func TestAddBranchToTask_ForwardsRepositoryURL(t *testing.T) { backend := &testBackend{ - response: map[string]interface{}{"id": "tr-1", "task_id": "task-current"}, + response: map[string]interface{}{ + "id": "tr-1", "task_id": "task-current", "worktree_path": "/task/kandev-feature-x", "task_workspace_path": "/task", "agent_cwd_changed": false, + }, } s := newTaskModeServer(t, backend, "task-current") @@ -417,6 +419,11 @@ func TestAddBranchToTask_ForwardsRepositoryURL(t *testing.T) { "repository_url should be forwarded as github_url to match create_task wire format") assert.Equal(t, "feature/x", payload["checkout_branch"]) assert.Equal(t, "", payload["repository_id"]) + text, ok := result.Content[0].(mcp.TextContent) + require.True(t, ok) + assert.Contains(t, text.Text, `"worktree_path": "/task/kandev-feature-x"`) + assert.Contains(t, text.Text, `"task_workspace_path": "/task"`) + assert.Contains(t, text.Text, `"agent_cwd_changed": false`) } func TestAddBranchToTask_RejectsAnotherTask(t *testing.T) { diff --git a/apps/backend/internal/mcp/server/server.go b/apps/backend/internal/mcp/server/server.go index 8d53c0adf5..7613d9cd84 100644 --- a/apps/backend/internal/mcp/server/server.go +++ b/apps/backend/internal/mcp/server/server.go @@ -640,7 +640,7 @@ func (s *Server) registerAddBranchToTaskTool() { mcp.NewTool("add_branch_to_task_kandev", mcp.WithDescription(`Attach an additional (repository, branch) worktree to an existing task. -Use this when the task should open more than one PR — same repo with different branches, or a second repository entirely. The new branch gets its own worktree under the task directory and behaves like any other multi-repo entry for changes, PRs, and review surfaces. +Use this when the task should open more than one PR — same repo with different branches, or a second repository entirely. The new branch gets its own sibling worktree under the task directory and behaves like any other multi-repo entry for changes, PRs, and review surfaces. The running agent and terminals keep their current working directory; use the returned worktree_path for the exact new location. task_workspace_path is the promoted task root used by the Files tree. IMPORTANT: - Only works on tasks running the WORKTREE executor. Tasks on docker / sprites / local-pc / SSH / remote_docker reject this tool because sibling worktrees are a git-worktree-specific layout — other executors bind one workspace path per task and the new branch would silently never appear on disk. diff --git a/apps/backend/internal/task/service/service.go b/apps/backend/internal/task/service/service.go index 8df4fc8ced..ad813450d1 100644 --- a/apps/backend/internal/task/service/service.go +++ b/apps/backend/internal/task/service/service.go @@ -100,7 +100,16 @@ type BranchMaterializer interface { // task_repositories row. Best-effort: when no active session exists yet // the implementation may choose to no-op and let the next session launch // create the worktree via the standard multi-repo prepare path. - MaterializeBranch(ctx context.Context, taskID, taskRepositoryID string) error + MaterializeBranch(ctx context.Context, taskID, taskRepositoryID string) (*BranchMaterializationResult, error) +} + +// BranchMaterializationResult describes the live worktree created for a +// branch attachment. A nil result with a nil error means materialization was +// intentionally deferred until the next session launch; callers must check +// for a nil result rather than empty path fields. +type BranchMaterializationResult struct { + WorktreePath string + TaskWorkspacePath string } // GitArchiveCapture captures git state (commits, cumulative diff) when a task is archived. diff --git a/apps/backend/internal/task/service/service_branches.go b/apps/backend/internal/task/service/service_branches.go index 3403928523..45560b29a2 100644 --- a/apps/backend/internal/task/service/service_branches.go +++ b/apps/backend/internal/task/service/service_branches.go @@ -31,12 +31,23 @@ type AddBranchToTaskRequest struct { CheckoutBranch string } +// AddBranchToTaskResult contains the persisted branch attachment plus the +// paths created by an immediate live materialization. The embedded row keeps +// the legacy service fields available to existing callers. +type AddBranchToTaskResult struct { + *models.TaskRepository + WorktreePath string + TaskWorkspacePath string + AgentCWDChanged bool +} + // AddBranchToTask appends a new task_repositories row to an existing task, // effectively adding a branch (or a second branch of an already-attached repo) // without recreating the task. The row's position is set to the next free // slot after the highest existing position. // -// Returns the persisted TaskRepository on success. +// Returns the persisted TaskRepository and any immediate materialization +// paths on success. The agent process CWD is never changed. // // Constraints: // - TaskID is required. @@ -54,16 +65,13 @@ type AddBranchToTaskRequest struct { // path (ResolveRepositoryRef → resolveRepoInput) enforces this for the // LocalPath / GitHubURL flows; for the RepositoryID fast path the // workspace-membership check happens in resolveBranchRepo below. -func (s *Service) AddBranchToTask(ctx context.Context, req AddBranchToTaskRequest) (*models.TaskRepository, error) { +func (s *Service) AddBranchToTask(ctx context.Context, req AddBranchToTaskRequest) (*AddBranchToTaskResult, error) { unlock := s.lockWorkspaceSources(req.TaskID) defer unlock() task, existing, cleanupOrphanRepo, err := s.prepareBranchAdd(ctx, &req) if err != nil { return nil, err } - if err := s.workspaceSourcesIdle(ctx, task.ID); err != nil { - return nil, err - } repo, err := s.resolveBranchRepo(ctx, task, req.RepositoryID) if err != nil { cleanupOrphanRepo() @@ -105,15 +113,16 @@ func (s *Service) AddBranchToTask(ctx context.Context, req AddBranchToTaskReques cleanupOrphanRepo() return nil, err } - materialize := s.materializeLegacyBranch - if s.workspaceSourceMaterializer != nil { - materialize = s.materializeWorkspaceSources - } - _, err = s.commitWorkspaceSourceBatch(ctx, task, batch, func(context.Context) { cleanupOrphanRepo() }, materialize) + result, err := s.commitWorkspaceSourceBatch(ctx, task, batch, func(context.Context) { cleanupOrphanRepo() }, s.materializeLegacyBranch) if err != nil { return nil, err } - return taskRepo, nil + return &AddBranchToTaskResult{ + TaskRepository: taskRepo, + WorktreePath: result.WorktreePath, + TaskWorkspacePath: result.WorkspacePath, + AgentCWDChanged: false, + }, nil } // materializeLegacyBranch adapts the legacy one-row worktree capability to @@ -122,9 +131,13 @@ func (s *Service) AddBranchToTask(ctx context.Context, req AddBranchToTaskReques func (s *Service) materializeLegacyBranch(ctx context.Context, taskID string, batch *models.WorkspaceSourceBatch) (*WorkspaceSourceMaterializationResult, error) { for _, source := range batch.Sources { if source.Repository != nil { - if err := s.materializeBranch(ctx, taskID, source.Repository.ID); err != nil { + materialization, err := s.materializeBranch(ctx, taskID, source.Repository.ID) + if err != nil { return nil, err } + if materialization != nil { + return &WorkspaceSourceMaterializationResult{WorkspacePath: materialization.TaskWorkspacePath, WorktreePath: materialization.WorktreePath}, nil + } } } return &WorkspaceSourceMaterializationResult{}, nil @@ -293,9 +306,9 @@ func branchAddDuplicateError( // failure to the caller so AddBranchToTask can roll back the dangling // task_repositories row — leaving it in place produced the silent-success // orphan that the original bug report describes. -func (s *Service) materializeBranch(ctx context.Context, taskID, taskRepositoryID string) error { +func (s *Service) materializeBranch(ctx context.Context, taskID, taskRepositoryID string) (*BranchMaterializationResult, error) { if s.branchMaterializer == nil { - return nil + return nil, nil } // Snapshot launch state BEFORE MaterializeBranch so a caller-side cancel // during materialize can't poison the post-failure check: launch state @@ -305,21 +318,25 @@ func (s *Service) materializeBranch(ctx context.Context, taskID, taskRepositoryI // rollback to a best-effort no-op, leaving the orphan row the PR exists // to prevent. alreadyLaunched := s.taskAlreadyLaunched(ctx, taskID) - if err := s.branchMaterializer.MaterializeBranch(ctx, taskID, taskRepositoryID); err != nil { + result, err := s.branchMaterializer.MaterializeBranch(ctx, taskID, taskRepositoryID) + if err != nil { if !alreadyLaunched { s.logger.Warn("branch materialization failed; worktree will be created on next session launch", zap.String("task_id", taskID), zap.String("task_repository_id", taskRepositoryID), zap.Error(err)) - return nil + return nil, nil } s.logger.Error("branch materialization failed on a live task; rolling back", zap.String("task_id", taskID), zap.String("task_repository_id", taskRepositoryID), zap.Error(err)) - return fmt.Errorf("materialize branch: %w", err) + return nil, fmt.Errorf("materialize branch: %w", err) } - return nil + if alreadyLaunched && result == nil { + return nil, fmt.Errorf("materialize branch: live task did not create a worktree") + } + return result, nil } // taskAlreadyLaunched reports whether the task already has a task_environments diff --git a/apps/backend/internal/task/service/service_branches_test.go b/apps/backend/internal/task/service/service_branches_test.go index 4e7e79c97a..b8f732e3c7 100644 --- a/apps/backend/internal/task/service/service_branches_test.go +++ b/apps/backend/internal/task/service/service_branches_test.go @@ -2,7 +2,6 @@ package service import ( "context" - "errors" "fmt" "strings" "testing" @@ -669,13 +668,14 @@ func (p *stubProber) ProbeDefaultBranch(_ context.Context, provider, owner, name // stubMaterializer implements BranchMaterializer for tests; the err field // controls whether the materialize step succeeds. type stubMaterializer struct { - err error - calls int + err error + result *BranchMaterializationResult + calls int } -func (m *stubMaterializer) MaterializeBranch(_ context.Context, _ string, _ string) error { +func (m *stubMaterializer) MaterializeBranch(_ context.Context, _ string, _ string) (*BranchMaterializationResult, error) { m.calls++ - return m.err + return m.result, m.err } // seedWorktreeTaskEnv attaches a worktree-executor task_environments row so @@ -930,7 +930,7 @@ func TestAddBranchToTask_MaterializeSkippedPreLaunchStillSucceeds(t *testing.T) } } -func TestAddBranchToTask_RejectsActiveTaskBeforePersisting(t *testing.T) { +func TestAddBranchToTask_ActiveTurnUsesLegacyMaterializer(t *testing.T) { svc, _, repo := createTestService(t) ctx := context.Background() if err := repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-add-idle", Name: "WS"}); err != nil { @@ -949,19 +949,74 @@ func TestAddBranchToTask_RejectsActiveTaskBeforePersisting(t *testing.T) { if err := repo.CreateTaskSession(ctx, &models.TaskSession{ID: "session-add-idle", TaskID: task.ID}); err != nil { t.Fatal(err) } + seedWorktreeTaskEnv(t, repo, task.ID, "env-add-idle") if _, err := svc.StartTurn(ctx, "session-add-idle"); err != nil { t.Fatal(err) } + legacy := &stubMaterializer{result: &BranchMaterializationResult{ + WorktreePath: "/task/kandev-feature-source", + TaskWorkspacePath: "/task", + }} + svc.SetBranchMaterializer(legacy) + batch := &recordingWorkspaceSourceMaterializer{} + svc.SetWorkspaceSourceMaterializer(batch) - _, err = svc.AddBranchToTask(ctx, AddBranchToTaskRequest{TaskID: task.ID, RepositoryID: "repo-add-idle", BaseBranch: "main", CheckoutBranch: "feature/source"}) - if !errors.Is(err, ErrWorkspaceSourceActive) { - t.Fatalf("error = %v, want active-task rejection", err) + added, err := svc.AddBranchToTask(ctx, AddBranchToTaskRequest{TaskID: task.ID, RepositoryID: "repo-add-idle", BaseBranch: "main", CheckoutBranch: "feature/source"}) + if err != nil { + t.Fatalf("AddBranchToTask: %v", err) + } + if added.CheckoutBranch != "feature/source" { + t.Fatalf("checkout branch = %q, want feature/source", added.CheckoutBranch) + } + if added.WorktreePath != "/task/kandev-feature-source" || added.TaskWorkspacePath != "/task" { + t.Fatalf("materialized paths = (%q, %q), want worktree and task root", added.WorktreePath, added.TaskWorkspacePath) + } + if added.AgentCWDChanged { + t.Fatal("agent CWD must remain unchanged") + } + if legacy.calls != 1 { + t.Fatalf("legacy materializer calls = %d, want 1", legacy.calls) + } + if batch.called { + t.Fatal("workspace-source materializer was called") + } + rows, err := repo.ListTaskRepositories(ctx, task.ID) + if err != nil { + t.Fatal(err) + } + if len(rows) != 2 { + t.Fatalf("task repositories = %#v, want added branch", rows) + } +} + +func TestAddBranchToTask_LiveTaskRollsBackDeferredMaterialization(t *testing.T) { + svc, _, repo := createTestService(t) + ctx := context.Background() + if err := repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-live-deferred", Name: "WS"}); err != nil { + t.Fatal(err) + } + if err := repo.CreateWorkflow(ctx, &models.Workflow{ID: "wf-live-deferred", WorkspaceID: "ws-live-deferred", Name: "WF"}); err != nil { + t.Fatal(err) + } + if err := repo.CreateRepository(ctx, &models.Repository{ID: "repo-live-deferred", WorkspaceID: "ws-live-deferred", Name: "app", DefaultBranch: "main"}); err != nil { + t.Fatal(err) + } + task, err := svc.CreateTask(ctx, &CreateTaskRequest{WorkspaceID: "ws-live-deferred", WorkflowID: "wf-live-deferred", WorkflowStepID: "step", Title: "Task", Repositories: []TaskRepositoryInput{{RepositoryID: "repo-live-deferred", BaseBranch: "main"}}}) + if err != nil { + t.Fatal(err) + } + seedWorktreeTaskEnv(t, repo, task.ID, "env-live-deferred") + svc.SetBranchMaterializer(&stubMaterializer{}) + + _, err = svc.AddBranchToTask(ctx, AddBranchToTaskRequest{TaskID: task.ID, RepositoryID: "repo-live-deferred", BaseBranch: "main", CheckoutBranch: "feature/deferred"}) + if err == nil { + t.Fatal("AddBranchToTask succeeded after live materialization was deferred") } rows, err := repo.ListTaskRepositories(ctx, task.ID) if err != nil { t.Fatal(err) } if len(rows) != 1 { - t.Fatalf("task repositories = %#v, want no added branch", rows) + t.Fatalf("task repositories = %#v, want deferred attachment rolled back", rows) } } diff --git a/apps/backend/internal/task/service/service_workspace_sources.go b/apps/backend/internal/task/service/service_workspace_sources.go index 9e4f577d07..534ce0f764 100644 --- a/apps/backend/internal/task/service/service_workspace_sources.go +++ b/apps/backend/internal/task/service/service_workspace_sources.go @@ -41,6 +41,7 @@ type AttachWorkspaceSourcesRequest struct { type AttachWorkspaceSourcesResult struct { Task *models.Task WorkspacePath string + WorktreePath string SessionIDs []string } @@ -49,6 +50,7 @@ type AttachWorkspaceSourcesResult struct { // authoritative: callers must not perform fallible hydration after this point. type WorkspaceSourceMaterializationResult struct { WorkspacePath string + WorktreePath string SessionIDs []string } @@ -445,6 +447,7 @@ func (s *Service) commitWorkspaceSourceBatch(ctx context.Context, task *models.T } if materialized != nil { result.WorkspacePath = materialized.WorkspacePath + result.WorktreePath = materialized.WorktreePath result.SessionIDs = materialized.SessionIDs } s.publishTaskEvent(context.WithoutCancel(ctx), events.TaskUpdated, result.Task, nil) diff --git a/docs/decisions/2026-07-22-runtime-mutable-task-workspace-sources.md b/docs/decisions/2026-07-22-runtime-mutable-task-workspace-sources.md index 92c88155c2..2485b3cdfa 100644 --- a/docs/decisions/2026-07-22-runtime-mutable-task-workspace-sources.md +++ b/docs/decisions/2026-07-22-runtime-mutable-task-workspace-sources.md @@ -1,6 +1,6 @@ # ADR-2026-07-22-runtime-mutable-task-workspace-sources: Runtime-Mutable Task Workspace Sources -**Status:** accepted +**Status:** accepted (amended by ADR-2026-07-27-legacy-add-branch-live-rescan) **Date:** 2026-07-22 **Area:** backend, frontend, protocol @@ -23,12 +23,17 @@ separate `task_workspace_folders` relation rather than migrating every repositor polymorphic table. Services expose a combined source projection when callers need the complete workspace. -A batch `AttachWorkspaceSources` service becomes the single mutation boundary for HTTP, UI, and -MCP callers. It validates the entire batch, persists attachments transactionally, materializes them -through an executor-capability interface, asks agentctl to adopt and rescan the effective workspace -root, and publishes events only after success. Failure rolls back newly created durable records and -owned filesystem entries without touching pre-existing sources. The legacy -`add_branch_to_task_kandev` tool becomes a compatibility wrapper over a one-item batch. +A batch `AttachWorkspaceSources` service becomes the mutation boundary for HTTP, UI, and the +`add_workspace_sources_kandev` MCP tool. It validates the entire batch, persists attachments +transactionally, materializes them through an executor-capability interface, asks agentctl to adopt +and rescan the effective workspace root, and publishes events only after success. Failure rolls +back newly created durable records and owned filesystem entries without touching pre-existing +sources. + +The legacy `add_branch_to_task_kandev` tool retains its worktree-only live-rescan behavior. It may +reuse transactional persistence helpers, but it does not use runtime workspace adoption or its +idle/restart policy. See +[ADR-2026-07-27-legacy-add-branch-live-rescan](2026-07-27-legacy-add-branch-live-rescan.md). Executor adapters own placement and transport: @@ -39,15 +44,18 @@ Executor adapters own placement and transport: - agentctl owns workspace-root adoption, repository tracker reconciliation, and the file-tree refresh boundary common to every runtime. -Source attachment is rejected while a turn or tool call is active. An idle environment may be -restarted when its process working directory cannot be changed safely in place. Provider +Batch workspace-source attachment is rejected while a turn or tool call is active. An idle +environment may be restarted when its process working directory cannot be changed safely in place. +The legacy worktree-only `add_branch_to_task_kandev` compatibility lane may run during its invoking +turn and updates tracker scope without changing the agent process working directory. Provider credentials stay behind the existing backend/executor clone boundary and never enter persisted URLs or agent-visible source metadata. ## Consequences -- The Files panel, MCP tools, and future automation use one source-attachment contract instead of - reimplementing worktree-specific behavior. +- The Files panel, `add_workspace_sources_kandev`, and future batch automation use one + source-attachment contract. The legacy add-branch tool remains an explicit worktree-only + compatibility exception. - Existing repository, diff, PR, and branch consumers keep their current relational model. - Arbitrary folders can coexist with Git sources without pretending to be repositories. - Runtime backends must implement explicit source materialization and rollback capabilities; diff --git a/docs/decisions/2026-07-27-legacy-add-branch-live-rescan.md b/docs/decisions/2026-07-27-legacy-add-branch-live-rescan.md new file mode 100644 index 0000000000..6f2453759c --- /dev/null +++ b/docs/decisions/2026-07-27-legacy-add-branch-live-rescan.md @@ -0,0 +1,68 @@ +# ADR-2026-07-27-legacy-add-branch-live-rescan: Preserve Live Rescan for Legacy Add Branch + +**Status:** accepted +**Date:** 2026-07-27 +**Area:** backend, protocol + +## Context + +`add_branch_to_task_kandev` predates runtime-mutable workspace-source batches. Its worktree-only +contract lets an agent add a repository/branch during its current turn by creating a sibling +worktree and updating Kandev's file and Git trackers without restarting the agent. The workspace +sources implementation routed this compatibility tool through the idle-only host rebind path, +making an in-turn MCP invocation fail and making an idle invocation stop and restart the agent and +agentctl-managed processes. + +The compatibility tool also returned repository metadata without the materialized filesystem path. +An agent whose current working directory remains the original worktree could therefore know that +the operation succeeded without knowing the exact sibling path to use. + +## Decision + +`add_branch_to_task_kandev` remains a worktree-executor compatibility lane separate from +`AttachWorkspaceSources` runtime adoption: + +- The tool may run during the invoking task's active turn. Its per-task workspace-source lock, + validation, transactional repository-row persistence, and rollback behavior remain in force. +- A live materialization creates the new repository/branch worktree as a sibling beneath the + Kandev-owned task root. It never nests another Git repository inside the current worktree. +- Kandev promotes the persisted task environment `workspace_path` to that task root, asks agentctl + to rescan its file and repository trackers at the promoted root, and publishes the existing + materialized-worktree event so the Files and Changes surfaces refresh. +- The live rescan changes tracker scope only. It does not call `RebindWorkspaceForSession`, stop or + restart the agent, change the running agent's working directory, or stop existing terminals, + editor servers, dev servers, or other agentctl-managed workspace processes. +- The MCP result includes the materialized `worktree_path`, promoted `task_workspace_path`, and + `agent_cwd_changed: false`. Materialized paths may be absent for a pre-launch task where durable + attachment succeeds and worktree creation is intentionally deferred until launch. + +`add_workspace_sources_kandev`, the Files-panel action, and +`POST /api/v1/tasks/:id/workspace-sources` retain the batch workspace-source contract from +ADR-2026-07-22-runtime-mutable-task-workspace-sources: they require an idle task and may rebind or +restart a host execution when adopting a new root. + +## Consequences + +- Agents retain their current ACP conversation, process, working directory, and terminals while a + sibling worktree becomes visible through Kandev's promoted Files and per-repository Git trackers. +- Agents receive the exact absolute sibling path instead of having to infer or discover it. The + returned path does not bypass a provider's own filesystem sandbox. +- `add_branch_to_task_kandev` and batch workspace-source attachment deliberately have different + activity and runtime-adoption rules. Shared persistence helpers must not silently select the + batch materializer for the compatibility tool. +- Git status in the original worktree remains clean because the new worktree is outside that Git + working tree. + +## Alternatives Considered + +1. **Keep routing the compatibility tool through batch workspace adoption.** Rejected because an + in-session MCP tool call necessarily occurs during a turn, while the batch path requires an idle + execution and may restart the process that is making the call. +2. **Create the new repository inside the current worktree.** Rejected because the outer + repository reports the nested repository as untracked, and a broad `git add` stages it as an + embedded Git link without a valid submodule contract. +3. **Rebind the agent to the task root after every legacy call.** Rejected because it discards the + compatibility tool's process-continuity guarantee and stops agentctl-managed workspace + processes. +4. **Return only repository and branch identifiers.** Rejected because the unchanged agent working + directory gives the caller no reliable path to the new sibling. diff --git a/docs/decisions/INDEX.md b/docs/decisions/INDEX.md index 2a1e79a1ee..61064820ce 100644 --- a/docs/decisions/INDEX.md +++ b/docs/decisions/INDEX.md @@ -75,9 +75,10 @@ Read individual ADRs for full context. Create new ones via `/record decision` or | 2026-07-20-repository-provider-origin-identity | [Persist Provider Origin In Repository Identity](2026-07-20-repository-provider-origin-identity.md) | accepted | backend, frontend | 2026-07-20 | | 2026-07-21-portable-status-bar-order | [Portable Status Bar Order](2026-07-21-portable-status-bar-order.md) | accepted | backend, frontend, protocol | 2026-07-21 | | 2026-07-21-work-item-reference-search | [Backend-normalized work-item references](2026-07-21-work-item-reference-search.md) | accepted | backend, frontend, protocol | 2026-07-21 | -| 2026-07-22-runtime-mutable-task-workspace-sources | [Runtime-Mutable Task Workspace Sources](2026-07-22-runtime-mutable-task-workspace-sources.md) | accepted | backend, frontend, protocol | 2026-07-22 | +| 2026-07-22-runtime-mutable-task-workspace-sources | [Runtime-Mutable Task Workspace Sources](2026-07-22-runtime-mutable-task-workspace-sources.md) | accepted (amended by 2026-07-27) | backend, frontend, protocol | 2026-07-22 | | 2026-07-24-opt-in-authentication | [Opt-in Authentication and Per-User Workspace Scoping](2026-07-24-opt-in-authentication.md) | accepted | backend, frontend, protocol, security | 2026-07-24 | | 2026-07-24-semantic-notification-events | [Semantic notification events come from domain occurrences](2026-07-24-semantic-notification-events.md) | accepted | backend, frontend, desktop | 2026-07-24 | | 2026-07-26-user-managed-agent-runtime-updates | [User-Managed Agent Runtime Updates](2026-07-26-user-managed-agent-runtime-updates.md) | accepted | backend, frontend, protocol | 2026-07-26 | | 2026-07-26-single-session-model-switching | [Single-Session Model Switching](2026-07-26-single-session-model-switching.md) | accepted (amended 2026-07-27) | workflow, infra | 2026-07-26 | | 2026-07-27-task-git-credential-policy | [Separate GitHub Automation From Task Git Credential Policy](2026-07-27-task-git-credential-policy.md) | accepted | backend, frontend, security | 2026-07-27 | +| 2026-07-27-legacy-add-branch-live-rescan | [Preserve Live Rescan for Legacy Add Branch](2026-07-27-legacy-add-branch-live-rescan.md) | accepted | backend, protocol | 2026-07-27 | diff --git a/docs/plans/restore-live-add-branch/plan.md b/docs/plans/restore-live-add-branch/plan.md new file mode 100644 index 0000000000..52884424c6 --- /dev/null +++ b/docs/plans/restore-live-add-branch/plan.md @@ -0,0 +1,210 @@ +--- +spec: docs/specs/tasks/attach-workspace-sources.md +decision: docs/decisions/2026-07-27-legacy-add-branch-live-rescan.md +created: 2026-07-27 +status: completed +--- + +# Implementation Plan: Restore Live Add-Branch Compatibility + +## Overview + +Restore `add_branch_to_task_kandev` as a worktree-only live compatibility path while retaining the +transactional persistence introduced by workspace-source batches. The repair first pins active-turn +routing and materialized-path propagation in the backend, then exposes those paths through the MCP +result and updates public documentation. The Files-panel and `add_workspace_sources_kandev` batch +flow remain idle-only and may continue to rebind or restart host executions. + +## Confirmed Root Cause + +PR #1900 added two incompatible behaviors to `Service.AddBranchToTask`: + +1. `workspaceSourcesIdle` rejects the MCP mutation while the invoking agent has the active turn + required to make the tool call. +2. Production wiring sets `workspaceSourceMaterializer`, causing `AddBranchToTask` to replace + `materializeLegacyBranch` with `materializeWorkspaceSources`. On a live Worktree environment that + reaches `RebindWorkspaceForSession`, which stops and restarts the agent instead of using the + existing tracker-only rescan. + +The preserved `branchMaterializer` already creates the sibling worktree, promotes the persisted +workspace path, calls `RescanWorkspaceForSession`, and publishes the frontend materialization event. +The missing contract is propagation of its materialized worktree and task-root paths back through +the service and MCP response. + +--- + +## Backend + +### Live compatibility result contract + +Files: + +- `apps/backend/internal/task/service/service.go` +- `apps/backend/internal/task/service/service_branches.go` + +Changes: + +- Add `BranchMaterializationResult` with `WorktreePath` and `TaskWorkspacePath`. +- Change `BranchMaterializer.MaterializeBranch` to return + `(*BranchMaterializationResult, error)`; `nil` means pre-launch/deferred materialization. +- Add `AddBranchToTaskResult` containing the persisted `TaskRepository`, optional materialized + paths, and `AgentCWDChanged` fixed to `false`. +- Carry the legacy materialization result through the existing transactional + `commitWorkspaceSourceBatch` callback without changing the batch materializer's public behavior. +- Keep the per-task workspace-source lock, repository validation, atomic row persistence, + compensation, and post-materialization `task.updated` publication. + +### Active-turn routing and sibling materialization + +Files: + +- `apps/backend/internal/task/service/service_branches.go` +- `apps/backend/internal/backendapp/branch_materializer.go` +- `apps/backend/internal/backendapp/workspace_source_materializer.go` + +Changes: + +- Remove the `workspaceSourcesIdle` gate only from `AddBranchToTask`. +- Always pass the legacy branch materializer to `commitWorkspaceSourceBatch`, even when the + production `WorkspaceSourceMaterializer` is configured. +- Return the created worktree path and the effective promoted task root from + `branchMaterializer.MaterializeBranch`. +- Preserve `promoteWorkspacePathIfNeeded`, `RescanWorkspaceForSession`, and + `NotifyWorktreeMaterialized` as the live adoption sequence. +- Do not call `RebindWorkspaceForSession`, mutate the live execution CWD, stop the agent, or stop + agentctl-managed processes. +- Leave `AttachWorkspaceSources` idle gating and workspace-source materialization unchanged. + +### MCP response + +Files: + +- `apps/backend/internal/mcp/handlers/handlers.go` +- `apps/backend/internal/mcp/server/server.go` + +Changes: + +- Map `AddBranchToTaskResult` to the existing response fields plus optional `worktree_path`, + optional `task_workspace_path`, and `agent_cwd_changed: false`. +- Omit the two path fields when materialization is deferred until launch. +- Update the tool description to state sibling placement, unchanged CWD/processes, and that the + returned path is the location the agent should use. +- Keep task scoping, locator mutual exclusion, and request arguments unchanged. + +--- + +## Frontend + +No frontend production change. The existing materialized-worktree +`session.agentctl_ready` handling already repoints the Files tree to `task_workspace_path`, records +the new worktree, and refreshes repository-aware surfaces. The compatibility repair must continue +to publish that event rather than introducing another browser contract. + +--- + +## Public Documentation + +Files: + +- `docs/public/automation-and-mcp.md` +- `docs/public/coordination.md` +- `docs/public/feature-status.md` + +Changes: + +- Distinguish idle-only batch source attachment from live + `add_branch_to_task_kandev`. +- Document sibling placement, unchanged agent/terminal CWD, no process restart, exact returned path, + and clean outer-repository Git status. +- Keep the recommendation to use `add_workspace_sources_kandev` for new multi-source automation + when its idle/rebind semantics are acceptable. + +--- + +## Tests + +- **What:** an active turn may call `AddBranchToTask` even when both production materializers are + configured; only `BranchMaterializer` is invoked. + **File:** `apps/backend/internal/task/service/service_branches_test.go` + **How:** replace the active-task rejection regression with spies for both materializers and assert + legacy call count `1`, workspace-source call count `0`, persisted row count `2`, and returned + paths. +- **What:** a live branch materialization returns the sibling worktree path and promoted task root + while retaining the live rescan/event sequence. + **File:** `apps/backend/internal/backendapp/branch_materializer_test.go` + **How:** extend the existing real-Git happy path to assert the returned result and the same single + rescan/materialization notification. +- **What:** pre-launch add-branch persists successfully with omitted materialized paths. + **File:** `apps/backend/internal/task/service/service_branches_test.go` + **How:** use the existing pre-launch fixture with a nil materialization result and assert the new + service result contract. +- **What:** a live materialization error compensates the repository attachment without routing to a + restart-capable materializer. + **File:** `apps/backend/internal/task/service/service_branches_test.go` + **How:** inject a failing legacy materializer plus a workspace-source spy and assert rollback and + call counts. +- **What:** the MCP WS handler exercises handler → service → SQLite persistence and returns + `worktree_path`, `task_workspace_path`, and `agent_cwd_changed: false`. + **File:** `apps/backend/internal/mcp/handlers/handlers_test.go` + **How:** build the real task service/repository fixture, seed an active turn and Worktree + environment, inject a branch-materializer stub returning paths, invoke + `handleAddBranchToTask`, and inspect the response plus durable row. +- **What:** the task-mode MCP tool preserves the backend response and advertises the no-restart + sibling contract. + **File:** `apps/backend/internal/mcp/server/handlers_test.go`, + `apps/backend/internal/mcp/server/server_test.go` + **How:** return a path-bearing fake backend response, parse the tool result JSON, and inspect the + registered tool description. +- **What:** tracker-only rescan still transitions the Files root from the primary repository to the + task root containing both sibling repositories. + **File:** existing `apps/backend/internal/agentctl/server/process/manager_rescan_test.go` + **How:** rerun the focused single-to-multi and file-tree-root regression tests unchanged. + +## E2E Tests + +No browser E2E change. The repair changes a task-mode MCP response and restores an already-covered +backend event/file-tree path; focused backend integration and existing process-manager tests are +the faithful validation surface. + +--- + +## Implementation Waves + +Wave 1: + +- [x] [task-01-restore-live-materialization](task-01-restore-live-materialization.md) + +Wave 2: + +- [x] [task-02-return-materialized-paths](task-02-return-materialized-paths.md) + +Wave 3: + +- [x] [task-03-update-public-docs](task-03-update-public-docs.md) + +Execution is sequential in the primary conversation. No task is marked parallel-safe, and these +waves do not authorize subagents. + +## Risks + +- `AddBranchToTask` runs inside an active turn, so its transaction and rescan must remain bounded + and must not wait for the turn to become idle. +- Interface result changes affect service and backendapp test stubs even though production behavior + changes only for the legacy MCP path. +- A provider-owned filesystem sandbox may still exclude a sibling path. Returning the path is + informational and must not be described as bypassing that sandbox. +- Publishing both legacy materialization and workspace-source adoption events would duplicate + refreshes. The legacy path should retain its existing materialized-worktree event and avoid + inventing a second session-adoption event. + +## Out of Scope + +- Making `add_workspace_sources_kandev` callable during an active turn. +- Changing the live agent or terminal CWD. +- Nesting repositories inside the current Git worktree. +- Widening provider-owned filesystem sandboxes. +- Changing non-Worktree executor support for `add_branch_to_task_kandev`. + +## Open Questions + +None. diff --git a/docs/plans/restore-live-add-branch/task-01-restore-live-materialization.md b/docs/plans/restore-live-add-branch/task-01-restore-live-materialization.md new file mode 100644 index 0000000000..2d186efcac --- /dev/null +++ b/docs/plans/restore-live-add-branch/task-01-restore-live-materialization.md @@ -0,0 +1,74 @@ +--- +id: "01-restore-live-materialization" +title: "Restore live add-branch materialization" +status: completed +wave: 1 +depends_on: [] +plan: "plan.md" +spec: "../../specs/tasks/attach-workspace-sources.md" +--- + +# Task 01: Restore live add-branch materialization + +## Acceptance + +- An active Worktree task can execute `Service.AddBranchToTask` while both the legacy branch and + batch workspace-source materializers are configured. +- The operation invokes only the legacy materializer, creates/persists the sibling attachment, and + returns its worktree and promoted task-root paths without stopping or rebinding the execution. +- Pre-launch success returns no materialized paths, and live materialization failure compensates the + new attachment. + +## Verification + +```bash +cd apps/backend && rtk go test ./internal/task/service -run 'TestAddBranchToTask' -count=1 +cd apps/backend && rtk go test ./internal/backendapp -run 'TestBranchMaterializer' -count=1 +cd apps/backend && rtk go test ./internal/agentctl/server/process -run 'TestRescanRepositories_(TransitionsSingleToMultiRepo|FileTreeReturnsTaskRootContentsAfterTransition)$' -count=1 +``` + +## Files Likely Touched + +- `apps/backend/internal/task/service/service.go` +- `apps/backend/internal/task/service/service_branches.go` +- `apps/backend/internal/task/service/service_branches_test.go` +- `apps/backend/internal/task/service/service_branches_qa_test.go` +- `apps/backend/internal/backendapp/branch_materializer.go` +- `apps/backend/internal/backendapp/branch_materializer_test.go` +- `apps/backend/internal/backendapp/workspace_source_materializer.go` +- `apps/backend/internal/backendapp/workspace_source_materializer_test.go` + +## Dependencies + +None. + +## Parallelism + +`sequential`. This task establishes the shared service/materializer result contract consumed by +Task 02. + +## Inputs + +- Spec sections: **What**, **API surface**, **Failure modes**, and the three legacy add-branch + scenarios. +- ADR-2026-07-27-legacy-add-branch-live-rescan. +- Existing patterns: + `branchMaterializer.finalizeMaterialize`, + `Service.commitWorkspaceSourceBatch`, + `TestBranchMaterializer_PromotesWorkspacePathAndTriggersRescan`, and + `TestAddBranchToTask_RejectsActiveTaskBeforePersisting`. + +## TDD Sequence + +1. Replace the active-task rejection expectation with a path-bearing active-turn regression and + configure spies for both materializers. +2. Run the focused service test and confirm RED because the active-turn gate rejects the call. +3. Remove only the legacy gate/reroute, implement the minimal result propagation, and reach GREEN. +4. Extend the real-Git branch materializer test to assert sibling and task-root paths. +5. Run the exact verification commands above. + +## Output Contract + +Report the expected RED failure, minimal routing/result changes, files changed, exact command +results, any interface-stub updates, risks, and blockers. Mark this task `done` and update its plan +checkbox in the primary conversation. diff --git a/docs/plans/restore-live-add-branch/task-02-return-materialized-paths.md b/docs/plans/restore-live-add-branch/task-02-return-materialized-paths.md new file mode 100644 index 0000000000..79fef09909 --- /dev/null +++ b/docs/plans/restore-live-add-branch/task-02-return-materialized-paths.md @@ -0,0 +1,68 @@ +--- +id: "02-return-materialized-paths" +title: "Return materialized paths from the MCP tool" +status: completed +wave: 2 +depends_on: ["01-restore-live-materialization"] +plan: "plan.md" +spec: "../../specs/tasks/attach-workspace-sources.md" +--- + +# Task 02: Return materialized paths from the MCP tool + +## Acceptance + +- A live `add_branch_to_task_kandev` result contains `worktree_path`, + `task_workspace_path`, and `agent_cwd_changed: false` alongside all existing fields. +- Deferred pre-launch materialization omits both path fields and still returns + `agent_cwd_changed: false`. +- The tool description tells agents that the worktree is a sibling, their CWD/processes remain + unchanged, and `worktree_path` is the exact location to use. + +## Verification + +```bash +cd apps/backend && rtk go test ./internal/mcp/handlers -run 'Test.*AddBranchToTask' -count=1 +cd apps/backend && rtk go test ./internal/mcp/server -run 'Test.*AddBranchToTask' -count=1 +``` + +## Files Likely Touched + +- `apps/backend/internal/mcp/handlers/handlers.go` +- `apps/backend/internal/mcp/handlers/handlers_test.go` +- `apps/backend/internal/mcp/server/server.go` +- `apps/backend/internal/mcp/server/handlers_test.go` +- `apps/backend/internal/mcp/server/server_test.go` + +## Dependencies + +- Task 01 provides `AddBranchToTaskResult` and materialized path values. + +## Parallelism + +`sequential`. This task depends on the service result contract and owns the agent-facing protocol +surface. + +## Inputs + +- Spec **API surface** response shape. +- ADR-2026-07-27-legacy-add-branch-live-rescan. +- Existing patterns: + `Handlers.handleAddBranchToTask`, + `Server.addBranchToTaskHandler`, + `TestAddBranchToTask_ForwardsRepositoryURL`, and the real service fixtures in + `internal/mcp/handlers/handlers_test.go`. + +## TDD Sequence + +1. Add a handler → service → SQLite regression with an active turn and path-returning legacy + materializer; confirm the current handler result lacks the three new fields. +2. Add a task-mode MCP result/description regression and confirm RED. +3. Map the Task 01 result without changing request arguments or task scoping. +4. Run the exact verification commands above. + +## Output Contract + +Report both RED failures, response/description changes, durable integration evidence, files changed, +exact command results, risks, and blockers. Mark this task `done` and update its plan checkbox in the +primary conversation. diff --git a/docs/plans/restore-live-add-branch/task-03-update-public-docs.md b/docs/plans/restore-live-add-branch/task-03-update-public-docs.md new file mode 100644 index 0000000000..d349937093 --- /dev/null +++ b/docs/plans/restore-live-add-branch/task-03-update-public-docs.md @@ -0,0 +1,52 @@ +--- +id: "03-update-public-docs" +title: "Document live add-branch compatibility" +status: completed +wave: 3 +depends_on: ["02-return-materialized-paths"] +plan: "plan.md" +spec: "../../specs/tasks/attach-workspace-sources.md" +--- + +# Task 03: Document live add-branch compatibility + +## Acceptance + +- Public MCP and coordination documentation distinguishes idle/restart-capable batch attachment + from active-turn legacy add-branch. +- Documentation states sibling placement, unchanged agent/terminal CWD, no process restart, exact + returned path, and clean Git status in the original repository. +- Public documentation validation passes. + +## Verification + +```bash +rtk rg -n 'add_branch_to_task_kandev|worktree_path|agent_cwd_changed' docs/public/automation-and-mcp.md docs/public/coordination.md docs/public/feature-status.md +node --test scripts/validate-public-docs.test.mjs +node scripts/validate-public-docs.mjs +``` + +## Files Likely Touched + +- `docs/public/automation-and-mcp.md` +- `docs/public/coordination.md` +- `docs/public/feature-status.md` + +## Dependencies + +- Task 02 finalizes the MCP response and tool wording. + +## Parallelism + +`sequential`. Public documentation must reflect the implemented response contract exactly. + +## Inputs + +- Spec **What**, **API surface**, **Scenarios**, and **Out of scope**. +- ADR-2026-07-27-legacy-add-branch-live-rescan. +- `/docs-maintainer` task-oriented public documentation guidance. + +## Output Contract + +Report the public pages updated, terminology changes, exact validation results, files changed, +risks, and blockers. Mark this task `done` and update its plan checkbox in the primary conversation. diff --git a/docs/public/automation-and-mcp.md b/docs/public/automation-and-mcp.md index fef61b129b..4e58e85d66 100644 --- a/docs/public/automation-and-mcp.md +++ b/docs/public/automation-and-mcp.md @@ -169,7 +169,7 @@ After an accepted stop, Kandev attempts to move an unarchived, non-Office task f `add_workspace_sources_kandev` adds one or more sources to an idle task and defaults `task_id` to the current task. Its `sources` input accepts the same atomic mixed batch as the Files panel: `repository` sources use exactly one saved repository ID, local Git path, or remote repository locator plus branch fields; `folder` sources use a local path and optional display name. Repository sources work on Worktree, Local/Local PC, Local Docker, SSH, and Sprites; folders work only on Worktree and Local/Local PC. The task must be repository-backed and have no active turn or tool call. Invalid, duplicate, unsupported, or failed sources roll back the entire batch. -`add_branch_to_task_kandev` is preserved for compatibility with its one-repository/branch input. Use `add_workspace_sources_kandev` for all new automation. `update_repository_base_branch_kandev` changes the base used for Kandev's diff, not a pull request's target branch. +`add_branch_to_task_kandev` is the Worktree-only compatibility path for adding one repository/branch during an active agent turn. It creates the worktree as a sibling under the task directory, promotes the persisted Files root to that parent, and rescans it without restarting the agent, terminals, or workspace processes. The response returns `worktree_path` (the exact new repository location), `task_workspace_path` (the Files root), and `agent_cwd_changed: false`; deferred pre-launch materialization omits both paths. The original repository stays a separate Git worktree, so the sibling is not reported as an embedded repository or untracked files by its Git status. Use `add_workspace_sources_kandev` for mixed batch attachments to an idle task. `update_repository_base_branch_kandev` changes the base used for Kandev's diff, not a pull request's target branch. The HTTP equivalent is `POST /api/v1/tasks/:id/workspace-sources`, with `{ "sources": [...] }`. It returns `400` for invalid input, `404` for a missing task/source outside the workspace, `409` for duplicates or an active task, and `422` when materialization or executor capability fails. Successful adoption publishes `task.updated` and `session.workspace_sources.updated`; clients should refresh their Files and repository state from those updates. diff --git a/docs/public/coordination.md b/docs/public/coordination.md index bde4d3f062..ac1ff62b54 100644 --- a/docs/public/coordination.md +++ b/docs/public/coordination.md @@ -195,7 +195,7 @@ processes after attachment. Local Docker, SSH, and Sprites instead clone the new the current remote workspace and rescan it without changing the agent CWD or restarting the agent and workspace processes. -Task agents can call `add_workspace_sources_kandev` with the same mixed batch; `task_id` defaults to the current task. `add_branch_to_task_kandev` remains available for its legacy one-repository/branch input. Neither operation can run during an active turn or tool call. +Task agents can call `add_workspace_sources_kandev` with the same mixed batch; `task_id` defaults to the current task, and the operation remains idle-only. `add_branch_to_task_kandev` is the Worktree-only legacy one-repository/branch path and may run during an active turn: it creates a sibling worktree under the task directory, promotes the Files root to that parent, and rescans without restarting the agent, terminals, or workspace processes. Its `worktree_path` is the exact new location, `task_workspace_path` is the Files root, and `agent_cwd_changed` is always `false`; the agent's current directory stays unchanged. Use `update_repository_base_branch_kandev` with a task-repository ID to change the comparison base. The database update is authoritative. Resetting cached session bases, refreshing Changes, base commit, ahead/behind counts, and cumulative diff in a live tracker are best-effort side effects; a failure is logged without rolling back the new base, and the persisted value is rebuilt on the next session launch. The tool does not rewrite commits, switch the checkout, or change an existing pull request's target branch. diff --git a/docs/public/feature-status.md b/docs/public/feature-status.md index c43e5e03c8..8cb6a145b3 100644 --- a/docs/public/feature-status.md +++ b/docs/public/feature-status.md @@ -89,7 +89,7 @@ Released builds can lag this page. Check **Settings > System > About** or `kande | Capability | Status | Current boundary | | ------------------------------------------------------------------------------- | ---------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| Task-scoped Kandev MCP | Dependency-bound | Injected into compatible task sessions with scoped task/session, plan, walkthrough, workspace-source, workflow-step, and coordination tools. `add_workspace_sources_kandev` adds an atomic source batch to an idle task; the legacy `add_branch_to_task_kandev` remains available. | +| Task-scoped Kandev MCP | Dependency-bound | Injected into compatible task sessions with scoped task/session, plan, walkthrough, workspace-source, workflow-step, and coordination tools. `add_workspace_sources_kandev` adds an atomic source batch to an idle task; Worktree-only `add_branch_to_task_kandev` can add one sibling worktree during an active turn without changing agent CWD or restarting task processes, returning `worktree_path`, `task_workspace_path`, and `agent_cwd_changed: false`. | | Profile-defined external MCP servers | Limited | Stdio, HTTP, SSE, and streamable HTTP are implemented, but current launch resolution starts from an allow-all baseline and overlays only explicit executor policy. Blank remote policies therefore allow transports instead of applying the separate deny-all remote default. A stdio process is per-session and cannot be shared like a network server. | | External Kandev MCP endpoint | Supported | **Settings > External MCP** exposes streamable HTTP at `/mcp` and SSE at `/mcp/sse` for configuration and task management. It has no Kandev user authentication; keep it on loopback/VPN or place it behind an authenticated proxy. It intentionally omits `stop_task_kandev` along with live-session question, plan, walkthrough, spawn, and message tools. | | External MCP tool preview | Limited | The endpoint registration is authoritative. The current settings preview omits several registered tools and should not be used as a complete allow-list. | diff --git a/docs/specs/tasks/attach-workspace-sources.md b/docs/specs/tasks/attach-workspace-sources.md index c357870706..38532e8465 100644 --- a/docs/specs/tasks/attach-workspace-sources.md +++ b/docs/specs/tasks/attach-workspace-sources.md @@ -61,11 +61,19 @@ manually moving files into the task workspace. the reason in touch-visible text rather than relying on a tooltip. - Existing conversations, task state, plan, sessions, and repository attachments remain intact. - Agents receive a batch `add_workspace_sources_kandev` MCP tool that uses the same validation and - materialization path. The existing `add_branch_to_task_kandev` tool remains as a compatibility - wrapper for one repository/branch source. + materialization path. +- The existing worktree-only `add_branch_to_task_kandev` tool remains a live compatibility path + for one repository/branch source. It may run during the invoking agent turn, creates the new + worktree as a sibling under the Kandev-owned task root, promotes the persisted task workspace + path to that root, and refreshes Files and repository trackers without restarting the agent or + agentctl-managed workspace processes. +- A legacy add-branch call does not nest the new worktree inside the current repository and does + not change the running agent or terminal working directory. Its MCP result returns the exact new + worktree path, the promoted task workspace path, and that the agent CWD did not change. Decisions: [ADR-2026-07-22-runtime-mutable-task-workspace-sources](../../decisions/2026-07-22-runtime-mutable-task-workspace-sources.md) -and [ADR-2026-07-23-workspace-source-root-move-boundary](../../decisions/2026-07-23-workspace-source-root-move-boundary.md). +[ADR-2026-07-23-workspace-source-root-move-boundary](../../decisions/2026-07-23-workspace-source-root-move-boundary.md), +and [ADR-2026-07-27-legacy-add-branch-live-rescan](../../decisions/2026-07-27-legacy-add-branch-live-rescan.md). ## Data model @@ -125,7 +133,28 @@ refresh the Files tree and repository trackers from those events rather than ass response is the only writer. `add_workspace_sources_kandev` accepts the same source union and defaults `task_id` to the current -task. `add_branch_to_task_kandev` translates its existing arguments to a one-item repository batch. +task. + +`add_branch_to_task_kandev` preserves its existing request arguments and returns: + +```json +{ + "id": "task-repository-id", + "task_id": "task-id", + "repository_id": "repository-id", + "base_branch": "main", + "checkout_branch": "feature/example", + "position": 1, + "worktree_path": "/absolute/task-root/repository-feature-example", + "task_workspace_path": "/absolute/task-root", + "agent_cwd_changed": false +} +``` + +`worktree_path` and `task_workspace_path` are omitted when a pre-launch attachment succeeds but +materialization is intentionally deferred until the task launches. `agent_cwd_changed` is always +false. A live call returns the materialized sibling path so the invoking agent can address it +without inferring a directory name. ## Permissions @@ -139,7 +168,8 @@ persisted in source URLs or copied into agent-visible metadata. | Condition | Observable behavior | | -------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------- | -| A turn or tool call is active | The UI disables the action when known; a racing request returns `409` without mutation. | +| A turn or tool call is active for batch attachment | The UI disables the action when known; a racing batch request returns `409` without mutation. | +| The invoking agent calls legacy add-branch during its turn | The worktree is created and trackers refresh without stopping the active agent, terminals, or workspace processes. | | Any source is invalid or duplicated | The full batch is rejected before persistence or materialization. | | A host materializer fails | New filesystem entries and source records are rolled back; existing task contents remain. | | A container/remote repository clone fails | Newly created remote entries are removed best-effort, durable attachments are rolled back, and the response identifies the failed source. | @@ -201,6 +231,19 @@ must be retried. - **GIVEN** an active agent turn, **WHEN** the user attempts to add sources, **THEN** no source is attached, the **Add Repositories to workspace** menu item explains that the task must be idle first, and **Open workspace folder** remains available. +- **GIVEN** an active worktree-executor agent whose CWD is the initial repository, **WHEN** it calls + `add_branch_to_task_kandev`, **THEN** Kandev creates the new repository/branch worktree as a + sibling under the task root, promotes the persisted workspace path, refreshes Files and + repository trackers, and does not restart the agent or change its CWD. +- **GIVEN** a live legacy add-branch materialization, **WHEN** the MCP result returns, **THEN** it + includes the absolute new `worktree_path`, the promoted `task_workspace_path`, and + `agent_cwd_changed: false`. +- **GIVEN** the original repository has no pending changes, **WHEN** a legacy add-branch call creates + a sibling worktree, **THEN** Git status in the original repository does not report the sibling as + an untracked or changed path. +- **GIVEN** a live legacy add-branch materialization fails, **WHEN** the MCP call returns an error, + **THEN** the new `task_repositories` row and any newly created repository entity are rolled back + while the agent and existing processes continue running. - **GIVEN** the same repository/branch or canonical folder is already attached, **WHEN** it is submitted again, **THEN** the request returns a conflict naming the duplicate and leaves the task unchanged. @@ -218,7 +261,12 @@ must be retried. - Removing or detaching sources after they have been attached. - Promoting a repository-less task into a repository-backed task. - Copying, mounting, or synchronizing arbitrary host folders into container or remote executors. -- Attaching sources while an agent turn or tool call is running. +- Running batch workspace-source attachment while an agent turn or tool call is active; the legacy + worktree-only `add_branch_to_task_kandev` compatibility path is the explicit exception. +- Changing the running agent or terminal CWD during a legacy add-branch call. +- Nesting a new Git repository or worktree inside the current repository. +- Expanding or bypassing a provider-owned filesystem sandbox when it excludes the returned sibling + path. - Reordering sources after attachment. - Sharing task-creation state, its mode switch, or its **None**/scratch semantics with Add sources; only the repository picker leaves are shared. @@ -227,4 +275,5 @@ must be retried. ## Implementation plan -See [Attach Workspace Sources plan](../../plans/attach-workspace-sources/plan.md). +See [Attach Workspace Sources plan](../../plans/attach-workspace-sources/plan.md) and the +[live add-branch compatibility repair plan](../../plans/restore-live-add-branch/plan.md).