fix: reject incompatible inherited workspace repositories (#3838)

Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Kandev Agent <agent@kandev.dev>
2026-09-21 15:08:26 +01:00
parent ac397c5486
commit 10726dfe8e
9 changed files with 644 additions and 7 deletions
@@ -875,15 +875,21 @@ func (h *Handlers) handleCreateTask(ctx context.Context, msg *ws.Message) (*ws.M
 		return ws.NewError(msg.ID, msg.Action, code, message, nil)
 	}

+	workspacePolicy, err := h.resolveMCPWorkspacePolicy(req.ParentID, req.WorkspaceMode)
+	if err != nil {
+		return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, err.Error(), nil)
+	}
+
 	identity, _ := authn.IdentityFromContext(ctx)
 	contributions, err := h.resolveMCPRemoteContributions(ctx, req.WorkspaceID, identity.UserID, repos)
 	if err != nil {
 		return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, err.Error(), nil)
 	}
-
-	workspacePolicy, err := h.resolveMCPWorkspacePolicy(req.ParentID, req.WorkspaceMode)
-	if err != nil {
-		return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, err.Error(), nil)
+	if workspacePolicy.Mode == mcpWorkspaceModeInheritParent &&
+		(len(req.Repositories) > 0 || req.BaseBranch != "") {
+		if err := h.taskSvc.ValidateInheritedWorkspaceRepositorySelection(ctx, req.ParentID, repos); err != nil {
+			return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, err.Error(), nil)
+		}
 	}

 	// Resolve the destination step before launch metadata. The profile lookup
@@ -105,6 +105,7 @@ func newTestTaskServiceWithEventBus(t *testing.T) (*service.Service, *sqliterepo
 		RepoEntities:     repo,
 		Executors:        repo,
 		Environments:     repo,
+		TaskEnvironments: repo,
 		Reviews:          repo,
 	}, eventBus, log, service.RepositoryDiscoveryConfig{})
 	svc.SetWorkspacePolicyAttacher(testWorkspacePolicyAttacher{})
@@ -2603,6 +2604,89 @@ func TestHandleCreateTask_SubtaskBaseBranchOverride_ExplicitReposWin(t *testing.
 		"explicit per-repo base_branch must win over the top-level override")
 }

+// @covers AC-TASKS-MCP-WORKSPACE-MODE-004.5
+func TestHandleCreateTask_InheritParentRejectsUnmatchedMaterializedRepository(t *testing.T) {
+	svc, repo := newTestTaskService(t)
+	ctx := context.Background()
+	parentID := seedParentWithRepo(t, svc, repo)
+	parentEnv := &models.TaskEnvironment{
+		ID:            "env-parent-admission",
+		TaskID:        parentID,
+		ExecutorType:  string(models.ExecutorTypeWorktree),
+		Status:        models.TaskEnvironmentStatusCreating,
+		WorkspacePath: "/tmp/parent-admission",
+	}
+	require.NoError(t, repo.CreateTaskEnvironment(ctx, parentEnv))
+	require.NoError(t, repo.CreateTaskEnvironmentRepo(ctx, &models.TaskEnvironmentRepo{
+		TaskEnvironmentID: parentEnv.ID,
+		RepositoryID:      "repo-parent",
+		BranchSlug:        "feature-parent",
+		Status:            "active",
+	}))
+	parentEnv.Status = models.TaskEnvironmentStatusReady
+	require.NoError(t, repo.UpdateTaskEnvironment(ctx, parentEnv))
+
+	h := &Handlers{taskSvc: svc, logger: testLogger(t).WithFields()}
+	msg := makeWSMessage(t, ws.ActionMCPCreateTask, map[string]interface{}{
+		"title":            "Child",
+		"description":      "do the thing",
+		"parent_id":        parentID,
+		"repositories":     []map[string]interface{}{{"repository_id": "repo-parent", "base_branch": "main"}},
+		"agent_profile_id": "profile-1",
+		"start_agent":      false,
+	})
+
+	resp, err := h.handleCreateTask(mcpTestExternalContext(ctx), msg)
+	require.NoError(t, err)
+	assertWSError(t, resp, ws.ErrorCodeValidation)
+	require.Contains(t, string(resp.Payload), "workspace_mode=new_workspace")
+
+	children, err := repo.ListChildren(ctx, parentID)
+	require.NoError(t, err)
+	assert.Empty(t, children, "rejected admission must not create a child task")
+}
+
+func TestHandleCreateTask_InheritParentRejectsUnmatchedBaseBranchOverride(t *testing.T) {
+	svc, repo := newTestTaskService(t)
+	ctx := context.Background()
+	parentID := seedParentWithRepo(t, svc, repo)
+	parentEnv := &models.TaskEnvironment{
+		ID:            "env-parent-base-override",
+		TaskID:        parentID,
+		ExecutorType:  string(models.ExecutorTypeWorktree),
+		Status:        models.TaskEnvironmentStatusCreating,
+		WorkspacePath: "/tmp/parent-base-override",
+	}
+	require.NoError(t, repo.CreateTaskEnvironment(ctx, parentEnv))
+	require.NoError(t, repo.CreateTaskEnvironmentRepo(ctx, &models.TaskEnvironmentRepo{
+		TaskEnvironmentID: parentEnv.ID,
+		RepositoryID:      "repo-parent",
+		BranchSlug:        "feature-parent",
+		Status:            "active",
+	}))
+	parentEnv.Status = models.TaskEnvironmentStatusReady
+	require.NoError(t, repo.UpdateTaskEnvironment(ctx, parentEnv))
+
+	h := &Handlers{taskSvc: svc, logger: testLogger(t).WithFields()}
+	msg := makeWSMessage(t, ws.ActionMCPCreateTask, map[string]interface{}{
+		"title":            "Child",
+		"description":      "do the thing",
+		"parent_id":        parentID,
+		"base_branch":      "main",
+		"agent_profile_id": "profile-1",
+		"start_agent":      false,
+	})
+
+	resp, err := h.handleCreateTask(mcpTestExternalContext(ctx), msg)
+	require.NoError(t, err)
+	assertWSError(t, resp, ws.ErrorCodeValidation)
+	require.Contains(t, string(resp.Payload), "workspace_mode=new_workspace")
+
+	children, err := repo.ListChildren(ctx, parentID)
+	require.NoError(t, err)
+	assert.Empty(t, children, "rejected branch override must not create a child task")
+}
+
 func TestHandleCreateTask_SubtaskDefaultsToParentWorkspaceAndWorkflow(t *testing.T) {
 	svc, repo := newTestTaskService(t)
 	ctx := context.Background()
@@ -0,0 +1,166 @@
+package service
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/kandev/kandev/internal/task/models"
+	sqliterepo "github.com/kandev/kandev/internal/task/repository/sqlite"
+)
+
+// @covers AC-TASKS-MCP-WORKSPACE-MODE-004.5
+func TestValidateInheritedWorkspaceRepositorySelectionRejectsMissingBranchSlot(t *testing.T) {
+	svc, repo := setupOfficeTest(t)
+	ctx := context.Background()
+	workspace, err := repo.GetWorkspace(ctx, "ws-1")
+	if err != nil {
+		t.Fatalf("get workspace: %v", err)
+	}
+	if err := repo.CreateRepository(ctx, &models.Repository{
+		ID: "repo-parent", WorkspaceID: "ws-1", Name: "Parent", DefaultBranch: "main",
+	}); err != nil {
+		t.Fatalf("create repository: %v", err)
+	}
+	parentResult, err := svc.CreateTask(ctx, &CreateTaskRequest{
+		WorkspaceID: "ws-1",
+		WorkflowID:  workspace.OfficeWorkflowID,
+		Title:       "Parent",
+		Repositories: []TaskRepositoryInput{{
+			RepositoryID: "repo-parent",
+			BaseBranch:   "main",
+		}},
+	})
+	if err != nil {
+		t.Fatalf("create parent: %v", err)
+	}
+	env := &models.TaskEnvironment{
+		ID:            "env-parent",
+		TaskID:        parentResult.Task.ID,
+		ExecutorType:  string(models.ExecutorTypeWorktree),
+		Status:        models.TaskEnvironmentStatusCreating,
+		WorkspacePath: "/tmp/parent-workspace",
+	}
+	if err := repo.CreateTaskEnvironment(ctx, env); err != nil {
+		t.Fatalf("create environment: %v", err)
+	}
+	if err := repo.CreateTaskEnvironmentRepo(ctx, &models.TaskEnvironmentRepo{
+		TaskEnvironmentID: env.ID,
+		RepositoryID:      "repo-parent",
+		BranchSlug:        "feature-parent",
+		Status:            "active",
+	}); err != nil {
+		t.Fatalf("create environment repository: %v", err)
+	}
+	env.Status = models.TaskEnvironmentStatusReady
+	if err := repo.UpdateTaskEnvironment(ctx, env); err != nil {
+		t.Fatalf("ready environment: %v", err)
+	}
+
+	err = svc.ValidateInheritedWorkspaceRepositorySelection(ctx, parentResult.Task.ID, []TaskRepositoryInput{{
+		RepositoryID: "repo-parent",
+		BaseBranch:   "main",
+	}})
+	if err == nil {
+		t.Fatal("validation succeeded for a missing canonical branch slot")
+	}
+	if !errors.Is(err, models.ErrWorkspaceReuseUnsafe) {
+		t.Fatalf("validation error = %v, want ErrWorkspaceReuseUnsafe", err)
+	}
+}
+
+func TestValidateInheritedWorkspaceRepositorySelectionAcceptsExactSlot(t *testing.T) {
+	svc, repo, parentID := seedInheritedAdmissionEnvironment(t, "feature-parent", models.TaskEnvironmentStatusReady)
+
+	err := svc.ValidateInheritedWorkspaceRepositorySelection(context.Background(), parentID, []TaskRepositoryInput{{
+		RepositoryID: "repo-parent",
+		BaseBranch:   "feature-parent",
+	}})
+	if err != nil {
+		t.Fatalf("validation error = %v, want nil for the exact canonical slot", err)
+	}
+	_ = repo
+}
+
+func TestValidateInheritedWorkspaceRepositorySelectionPassesWithoutEnvironment(t *testing.T) {
+	svc, _, parentID := seedInheritedAdmissionParent(t)
+
+	err := svc.ValidateInheritedWorkspaceRepositorySelection(context.Background(), parentID, []TaskRepositoryInput{{
+		RepositoryID: "repo-parent",
+		BaseBranch:   "main",
+	}})
+	if err != nil {
+		t.Fatalf("validation error = %v, want nil before parent materialization", err)
+	}
+}
+
+func TestValidateInheritedWorkspaceRepositorySelectionAcceptsLegacyUnscopedSlot(t *testing.T) {
+	svc, _, parentID := seedInheritedAdmissionEnvironment(t, "", models.TaskEnvironmentStatusReady)
+
+	err := svc.ValidateInheritedWorkspaceRepositorySelection(context.Background(), parentID, []TaskRepositoryInput{{
+		RepositoryID: "repo-parent",
+		BaseBranch:   "main",
+	}})
+	if err != nil {
+		t.Fatalf("validation error = %v, want nil for a legacy unscoped slot", err)
+	}
+}
+
+func seedInheritedAdmissionEnvironment(
+	t *testing.T,
+	branchSlug string,
+	status models.TaskEnvironmentStatus,
+) (*Service, *sqliterepo.Repository, string) {
+	svc, repo, parentID := seedInheritedAdmissionParent(t)
+	env := &models.TaskEnvironment{
+		ID:            "env-parent",
+		TaskID:        parentID,
+		ExecutorType:  string(models.ExecutorTypeWorktree),
+		Status:        models.TaskEnvironmentStatusCreating,
+		WorkspacePath: "/tmp/parent-workspace",
+	}
+	if err := repo.CreateTaskEnvironment(context.Background(), env); err != nil {
+		t.Fatalf("create environment: %v", err)
+	}
+	if err := repo.CreateTaskEnvironmentRepo(context.Background(), &models.TaskEnvironmentRepo{
+		TaskEnvironmentID: env.ID,
+		RepositoryID:      "repo-parent",
+		BranchSlug:        branchSlug,
+		Status:            "active",
+	}); err != nil {
+		t.Fatalf("create environment repository: %v", err)
+	}
+	env.Status = status
+	if err := repo.UpdateTaskEnvironment(context.Background(), env); err != nil {
+		t.Fatalf("update environment: %v", err)
+	}
+	return svc, repo, parentID
+}
+
+func seedInheritedAdmissionParent(t *testing.T) (*Service, *sqliterepo.Repository, string) {
+	t.Helper()
+	svc, repo := setupOfficeTest(t)
+	ctx := context.Background()
+	workspace, err := repo.GetWorkspace(ctx, "ws-1")
+	if err != nil {
+		t.Fatalf("get workspace: %v", err)
+	}
+	if err := repo.CreateRepository(ctx, &models.Repository{
+		ID: "repo-parent", WorkspaceID: "ws-1", Name: "Parent", DefaultBranch: "main",
+	}); err != nil {
+		t.Fatalf("create repository: %v", err)
+	}
+	parentResult, err := svc.CreateTask(ctx, &CreateTaskRequest{
+		WorkspaceID: "ws-1",
+		WorkflowID:  workspace.OfficeWorkflowID,
+		Title:       "Parent",
+		Repositories: []TaskRepositoryInput{{
+			RepositoryID: "repo-parent",
+			BaseBranch:   "main",
+		}},
+	})
+	if err != nil {
+		t.Fatalf("create parent: %v", err)
+	}
+	return svc, repo, parentResult.Task.ID
+}
@@ -0,0 +1,152 @@
+package service
+
+import (
+	"context"
+	"fmt"
+	"path/filepath"
+	"strings"
+
+	"github.com/kandev/kandev/internal/task/models"
+	"github.com/kandev/kandev/internal/worktree"
+)
+
+// ValidateInheritedWorkspaceRepositorySelection verifies explicit repository
+// slots before a subtask attaches to a parent's materialized environment.
+// A missing environment is left to the normal launch path because the parent
+// may not have materialized its workspace yet.
+func (s *Service) ValidateInheritedWorkspaceRepositorySelection(
+	ctx context.Context,
+	parentTaskID string,
+	repositories []TaskRepositoryInput,
+) error {
+	if parentTaskID == "" || len(repositories) == 0 {
+		return nil
+	}
+	if s.taskEnvironments == nil {
+		return nil
+	}
+	target, err := s.resolveBranchMaterializationTarget(ctx, parentTaskID)
+	if err != nil {
+		return fmt.Errorf("%w: inspect inherited workspace", models.ErrWorkspaceReuseUnsafe)
+	}
+	if target == nil || target.environment == nil ||
+		target.environment.Status != models.TaskEnvironmentStatusReady {
+		return nil
+	}
+	rows, err := s.taskEnvironments.ListTaskEnvironmentRepos(ctx, target.environment.ID)
+	if err != nil {
+		return fmt.Errorf("%w: inspect inherited workspace inventory", models.ErrWorkspaceReuseUnsafe)
+	}
+	for _, repository := range repositories {
+		repository, err = s.resolveExistingInheritedRepository(ctx, target.environment.TaskID, repository)
+		if err != nil {
+			return err
+		}
+		if countActiveInventoryMatches(repository, rows) != 1 {
+			return fmt.Errorf(
+				"%w: repository %q branch %q is not an exact match in the inherited workspace; use workspace_mode=new_workspace for a different checkout",
+				models.ErrWorkspaceReuseUnsafe,
+				repository.RepositoryID,
+				worktree.SanitizeBranchSlug(repository.BaseBranch),
+			)
+		}
+	}
+	return nil
+}
+
+func (s *Service) resolveExistingInheritedRepository(
+	ctx context.Context,
+	taskID string,
+	repository TaskRepositoryInput,
+) (TaskRepositoryInput, error) {
+	if repository.RepositoryID != "" {
+		return repository, nil
+	}
+	task, err := s.tasks.GetTask(ctx, taskID)
+	if err != nil || task == nil {
+		return repository, fmt.Errorf("%w: resolve inherited repository workspace", models.ErrWorkspaceReuseUnsafe)
+	}
+	found, err := s.findExistingInheritedRepository(ctx, task.WorkspaceID, repository)
+	if err != nil {
+		return repository, fmt.Errorf("%w: resolve inherited repository locator", models.ErrWorkspaceReuseUnsafe)
+	}
+	if found == nil {
+		return repository, fmt.Errorf(
+			"%w: repository locator is not an existing inherited repository; use workspace_mode=new_workspace for a different checkout",
+			models.ErrWorkspaceReuseUnsafe,
+		)
+	}
+	repository.RepositoryID = found.ID
+	if repository.BaseBranch == "" {
+		repository.BaseBranch = found.DefaultBranch
+	}
+	return repository, nil
+}
+
+func (s *Service) findExistingInheritedRepository(
+	ctx context.Context,
+	workspaceID string,
+	repository TaskRepositoryInput,
+) (*models.Repository, error) {
+	if path := strings.TrimSpace(repository.LocalPath); path != "" {
+		return s.repoEntities.GetRepositoryByLocalPath(ctx, workspaceID, filepath.Clean(path))
+	}
+	rawURL := effectiveRemoteURL(repository)
+	if rawURL == "" {
+		return nil, nil
+	}
+	provider, owner, name, _, err := parseRemoteRepositoryURL(rawURL, repository.Provider)
+	if err != nil {
+		return nil, err
+	}
+	repositories, err := s.repoEntities.ListRepositories(ctx, workspaceID)
+	if err != nil {
+		return nil, err
+	}
+	return findRepositoryByProviderIdentity(repositories, provider, owner, name), nil
+}
+
+func findRepositoryByProviderIdentity(
+	repositories []*models.Repository,
+	provider, owner, name string,
+) *models.Repository {
+	for _, candidate := range repositories {
+		if candidate != nil &&
+			strings.EqualFold(candidate.Provider, provider) &&
+			candidate.ProviderOwner == owner &&
+			candidate.ProviderName == name {
+			return candidate
+		}
+	}
+	return nil
+}
+
+func countActiveInventoryMatches(
+	repository TaskRepositoryInput,
+	rows []*models.TaskEnvironmentRepo,
+) int {
+	expectedBranch := worktree.SanitizeBranchSlug(repository.BaseBranch)
+	hasBranchScopedRow := false
+	for _, row := range rows {
+		if row != nil && row.RepositoryID == repository.RepositoryID &&
+			worktree.SanitizeBranchSlug(row.BranchSlug) != "" {
+			hasBranchScopedRow = true
+			break
+		}
+	}
+	matches := 0
+	for _, row := range rows {
+		if row == nil || row.RepositoryID != repository.RepositoryID ||
+			row.DeletedAt != nil || row.Status == "failed" || row.Status == "deleted" {
+			continue
+		}
+		branchMatches := worktree.SanitizeBranchSlug(row.BranchSlug) == expectedBranch
+		if expectedBranch != "" && !hasBranchScopedRow && row.BranchSlug == "" {
+			branchMatches = true
+		}
+		if branchMatches {
+			matches++
+		}
+	}
+	return matches
+}
@@ -658,7 +658,7 @@ test.describe("MCP subtask creation", () => {
    const script = [
      'e2e:thinking("Creating cross-repo subtask...")',
      "e2e:delay(100)",
-      `e2e:mcp:kandev:create_task_kandev({"parent_id":"self","title":"${subtaskTitle}","description":"E2E subtask: verify repository override","repository_id":"${otherRepo.id}"})`,
+      `e2e:mcp:kandev:create_task_kandev({"parent_id":"self","workspace_mode":"new_workspace","title":"${subtaskTitle}","description":"E2E subtask: verify repository override","repository_id":"${otherRepo.id}"})`,
      "e2e:delay(100)",
      'e2e:message("Done.")',
    ].join("\n");
@@ -823,7 +823,7 @@ test.describe("MCP subtask creation", () => {
    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:mcp:kandev:create_task_kandev({"parent_id":"self","workspace_mode":"new_workspace","title":"${subtaskTitle}","description":"E2E cross-repo subtask","repository_id":"${otherRepo.id}"})`,
      "e2e:delay(100)",
      'e2e:message("Done.")',
    ].join("\n");