From 7fbca753541fadedc0246bba9487ddb70edc2fca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Wed, 15 Apr 2026 14:14:19 +0100 Subject: [PATCH] fix(mcp): inherit repo and default to worktree executor for MCP-created tasks (#592) --- .../internal/agentctl/server/mcp/handlers.go | 1 + .../agentctl/server/mcp/handlers_test.go | 36 ++++ .../backend/internal/mcp/handlers/handlers.go | 26 ++- .../internal/mcp/handlers/handlers_test.go | 161 ++++++++++++++++++ 4 files changed, 223 insertions(+), 1 deletion(-) create mode 100644 apps/backend/internal/mcp/handlers/handlers_test.go diff --git a/apps/backend/internal/agentctl/server/mcp/handlers.go b/apps/backend/internal/agentctl/server/mcp/handlers.go index 7d725e7f10..d17e34c65e 100644 --- a/apps/backend/internal/agentctl/server/mcp/handlers.go +++ b/apps/backend/internal/agentctl/server/mcp/handlers.go @@ -105,6 +105,7 @@ func (s *Server) createTaskHandler() server.ToolHandlerFunc { "description": req.GetString("description", ""), "agent_profile_id": req.GetString("agent_profile_id", ""), "executor_profile_id": req.GetString("executor_profile_id", ""), + "source_task_id": s.taskID, } var result map[string]interface{} if err := s.backend.RequestPayload(ctx, ws.ActionMCPCreateTask, payload, &result); err != nil { diff --git a/apps/backend/internal/agentctl/server/mcp/handlers_test.go b/apps/backend/internal/agentctl/server/mcp/handlers_test.go index a0f13bb81a..97036c8295 100644 --- a/apps/backend/internal/agentctl/server/mcp/handlers_test.go +++ b/apps/backend/internal/agentctl/server/mcp/handlers_test.go @@ -66,6 +66,7 @@ func TestCreateTask_SelfResolvesToTaskID(t *testing.T) { require.True(t, ok) assert.Equal(t, "task-current", payload["parent_id"], "self should resolve to current task ID") assert.Equal(t, "Write tests", payload["title"]) + assert.Equal(t, "task-current", payload["source_task_id"], "source_task_id should be set to current task") } func TestCreateTask_SelfWithNoTaskContext_ReturnsError(t *testing.T) { @@ -129,4 +130,39 @@ func TestCreateTask_NoParentID_WithIDs_CreatesTopLevelTask(t *testing.T) { assert.Equal(t, "", payload["parent_id"]) assert.Equal(t, "ws-1", payload["workspace_id"]) assert.Equal(t, "wf-1", payload["workflow_id"]) + assert.Equal(t, "task-current", payload["source_task_id"]) +} + +func TestCreateTask_SourceTaskID_AlwaysSet(t *testing.T) { + backend := &testBackend{ + response: map[string]interface{}{"id": "task-new"}, + } + s := newTaskModeServer(t, backend, "my-task-123") + + callTool(t, s, "create_task_kandev", map[string]interface{}{ + "title": "New task", + "workspace_id": "ws-1", + "workflow_id": "wf-1", + }) + + payload, ok := backend.lastPayload.(map[string]string) + require.True(t, ok) + assert.Equal(t, "my-task-123", payload["source_task_id"]) +} + +func TestCreateTask_SourceTaskID_EmptyWhenNoTaskContext(t *testing.T) { + backend := &testBackend{ + response: map[string]interface{}{"id": "task-new"}, + } + s := newTaskModeServer(t, backend, "") + + callTool(t, s, "create_task_kandev", map[string]interface{}{ + "title": "New task", + "workspace_id": "ws-1", + "workflow_id": "wf-1", + }) + + payload, ok := backend.lastPayload.(map[string]string) + require.True(t, ok) + assert.Equal(t, "", payload["source_task_id"]) } diff --git a/apps/backend/internal/mcp/handlers/handlers.go b/apps/backend/internal/mcp/handlers/handlers.go index 5fd7deaad1..4029084154 100644 --- a/apps/backend/internal/mcp/handlers/handlers.go +++ b/apps/backend/internal/mcp/handlers/handlers.go @@ -293,6 +293,7 @@ func (h *Handlers) handleCreateTask(ctx context.Context, msg *ws.Message) (*ws.M // Use local struct with JSON tags since dto.CreateTaskRequest lacks them var req struct { ParentID string `json:"parent_id"` + SourceTaskID string `json:"source_task_id"` WorkspaceID string `json:"workspace_id"` WorkflowID string `json:"workflow_id"` WorkflowStepID string `json:"workflow_step_id"` @@ -336,6 +337,24 @@ func (h *Handlers) handleCreateTask(ctx context.Context, msg *ws.Message) (*ws.M } } + // For top-level tasks, inherit repositories from the calling agent's current task + // so the new task is associated with the same codebase. + if req.ParentID == "" && req.SourceTaskID != "" { + sourceTask, err := h.taskSvc.GetTask(ctx, req.SourceTaskID) + if err != nil { + h.logger.Warn("source task not found, skipping repo inheritance", + zap.String("source_task_id", req.SourceTaskID), zap.Error(err)) + } else { + for _, r := range sourceTask.Repositories { + inheritedRepos = append(inheritedRepos, service.TaskRepositoryInput{ + RepositoryID: r.RepositoryID, + BaseBranch: r.BaseBranch, + CheckoutBranch: r.CheckoutBranch, + }) + } + } + } + if req.WorkspaceID == "" { return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "workspace_id is required", nil) } @@ -366,19 +385,24 @@ func (h *Handlers) handleCreateTask(ctx context.Context, msg *ws.Message) (*ws.M // autoStartTask launches an agent session for a newly created task in the background. // It resolves the agent profile: explicit > parent's session > workspace default. // It resolves the executor: explicit executor_profile_id > parent's executor_profile_id > -// parent's executor_id (fallback for sessions that have no profile). +// parent's executor_id > "exec-worktree" (default for MCP-created tasks). func (h *Handlers) autoStartTask(task *models.Task, agentProfileID, executorProfileID string) { if h.sessionLauncher == nil { return } executorID := h.inheritFromParentSession(task.ParentID, &agentProfileID, &executorProfileID) + + // Fall back to workspace defaults for agent profile and worktree executor if agentProfileID == "" { workspace, err := h.taskSvc.GetWorkspace(context.Background(), task.WorkspaceID) if err == nil && workspace.DefaultAgentProfileID != nil { agentProfileID = *workspace.DefaultAgentProfileID } } + if executorID == "" && executorProfileID == "" { + executorID = models.ExecutorIDWorktree + } if agentProfileID == "" { h.logger.Warn("no agent profile available, skipping auto-start", diff --git a/apps/backend/internal/mcp/handlers/handlers_test.go b/apps/backend/internal/mcp/handlers/handlers_test.go new file mode 100644 index 0000000000..f13b964797 --- /dev/null +++ b/apps/backend/internal/mcp/handlers/handlers_test.go @@ -0,0 +1,161 @@ +package handlers + +import ( + "context" + "encoding/json" + "sync" + "testing" + "time" + + "github.com/kandev/kandev/internal/orchestrator" + "github.com/kandev/kandev/internal/task/models" + ws "github.com/kandev/kandev/pkg/websocket" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestHandleCreateTask_MissingTitle(t *testing.T) { + h := &Handlers{} + msg := makeWSMessage(t, ws.ActionMCPCreateTask, map[string]interface{}{ + "workspace_id": "ws-1", + "workflow_id": "wf-1", + }) + + resp, err := h.handleCreateTask(context.Background(), msg) + require.NoError(t, err) + assertWSError(t, resp, ws.ErrorCodeValidation) +} + +func TestHandleCreateTask_SubtaskMissingDescription(t *testing.T) { + h := &Handlers{} + msg := makeWSMessage(t, ws.ActionMCPCreateTask, map[string]interface{}{ + "title": "Fix bug", + "parent_id": "task-parent", + }) + + resp, err := h.handleCreateTask(context.Background(), msg) + require.NoError(t, err) + assertWSError(t, resp, ws.ErrorCodeValidation) +} + +func TestHandleCreateTask_InvalidPayload(t *testing.T) { + h := &Handlers{} + msg := &ws.Message{ + ID: "test-id", + Type: ws.MessageTypeRequest, + Action: ws.ActionMCPCreateTask, + Payload: json.RawMessage(`{invalid`), + } + + resp, err := h.handleCreateTask(context.Background(), msg) + require.NoError(t, err) + assertWSError(t, resp, ws.ErrorCodeBadRequest) +} + +func TestHandleCreateTask_TopLevel_MissingWorkspaceID(t *testing.T) { + h := &Handlers{} + msg := makeWSMessage(t, ws.ActionMCPCreateTask, map[string]interface{}{ + "title": "New task", + "workflow_id": "wf-1", + }) + + resp, err := h.handleCreateTask(context.Background(), msg) + require.NoError(t, err) + assertWSError(t, resp, ws.ErrorCodeValidation) +} + +func TestHandleCreateTask_TopLevel_MissingWorkflowID(t *testing.T) { + h := &Handlers{} + msg := makeWSMessage(t, ws.ActionMCPCreateTask, map[string]interface{}{ + "title": "New task", + "workspace_id": "ws-1", + }) + + resp, err := h.handleCreateTask(context.Background(), msg) + require.NoError(t, err) + assertWSError(t, resp, ws.ErrorCodeValidation) +} + +// mockSessionLauncher captures LaunchSession calls for testing autoStartTask. +type mockSessionLauncher struct { + mu sync.Mutex + req *orchestrator.LaunchSessionRequest + called chan struct{} +} + +func newMockSessionLauncher() *mockSessionLauncher { + return &mockSessionLauncher{called: make(chan struct{})} +} + +func (m *mockSessionLauncher) LaunchSession(_ context.Context, req *orchestrator.LaunchSessionRequest) (*orchestrator.LaunchSessionResponse, error) { + m.mu.Lock() + m.req = req + m.mu.Unlock() + close(m.called) + return &orchestrator.LaunchSessionResponse{ + Success: true, + TaskID: req.TaskID, + SessionID: "session-1", + }, nil +} + +func (m *mockSessionLauncher) getRequest() *orchestrator.LaunchSessionRequest { + m.mu.Lock() + defer m.mu.Unlock() + return m.req +} + +func TestAutoStartTask_DefaultsToWorktreeExecutor(t *testing.T) { + launcher := newMockSessionLauncher() + log := testLogger(t) + h := &Handlers{ + sessionLauncher: launcher, + logger: log.WithFields(), + } + + task := &models.Task{ + ID: "task-1", + WorkspaceID: "ws-1", + } + + // Call with agent profile but no executor info + h.autoStartTask(task, "agent-profile-1", "") + + select { + case <-launcher.called: + case <-time.After(2 * time.Second): + t.Fatal("LaunchSession was not called within timeout") + } + + req := launcher.getRequest() + assert.Equal(t, models.ExecutorIDWorktree, req.ExecutorID, "should default to exec-worktree") + assert.Equal(t, "", req.ExecutorProfileID) + assert.Equal(t, "agent-profile-1", req.AgentProfileID) +} + +func TestAutoStartTask_ExplicitExecutorProfilePreserved(t *testing.T) { + launcher := newMockSessionLauncher() + log := testLogger(t) + h := &Handlers{ + sessionLauncher: launcher, + logger: log.WithFields(), + } + + task := &models.Task{ + ID: "task-1", + WorkspaceID: "ws-1", + } + + // Call with explicit executor profile + h.autoStartTask(task, "agent-profile-1", "exec-profile-docker") + + select { + case <-launcher.called: + case <-time.After(2 * time.Second): + t.Fatal("LaunchSession was not called within timeout") + } + + req := launcher.getRequest() + assert.Equal(t, "exec-profile-docker", req.ExecutorProfileID, "explicit executor profile should be preserved") + assert.Equal(t, "", req.ExecutorID, "executorID should be empty when profile is set") +}