diff --git a/apps/backend/internal/mcp/handlers/handlers.go b/apps/backend/internal/mcp/handlers/handlers.go index 5545e0e122..1f122f386d 100644 --- a/apps/backend/internal/mcp/handlers/handlers.go +++ b/apps/backend/internal/mcp/handlers/handlers.go @@ -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 diff --git a/apps/backend/internal/mcp/handlers/handlers_test.go b/apps/backend/internal/mcp/handlers/handlers_test.go index cda8eaf77a..f7d866dec8 100644 --- a/apps/backend/internal/mcp/handlers/handlers_test.go +++ b/apps/backend/internal/mcp/handlers/handlers_test.go @@ -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() diff --git a/apps/backend/internal/task/service/inherit_parent_repository_admission_test.go b/apps/backend/internal/task/service/inherit_parent_repository_admission_test.go new file mode 100644 index 0000000000..d48519ef4b --- /dev/null +++ b/apps/backend/internal/task/service/inherit_parent_repository_admission_test.go @@ -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 +} diff --git a/apps/backend/internal/task/service/inherited_workspace_admission.go b/apps/backend/internal/task/service/inherited_workspace_admission.go new file mode 100644 index 0000000000..a3a9a00228 --- /dev/null +++ b/apps/backend/internal/task/service/inherited_workspace_admission.go @@ -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 +} diff --git a/apps/web/e2e/tests/task/subtask.spec.ts b/apps/web/e2e/tests/task/subtask.spec.ts index c7916a29a7..4e2fcbadfe 100644 --- a/apps/web/e2e/tests/task/subtask.spec.ts +++ b/apps/web/e2e/tests/task/subtask.spec.ts @@ -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"); diff --git a/docs/plans/inherit-parent-repository-admission/plan.md b/docs/plans/inherit-parent-repository-admission/plan.md new file mode 100644 index 0000000000..f492098c69 --- /dev/null +++ b/docs/plans/inherit-parent-repository-admission/plan.md @@ -0,0 +1,104 @@ +--- +created: 2026-09-21 +status: implemented +requirements: + - REQ-TASKS-MCP-WORKSPACE-MODE-004 +system_design: + - ../../specs/tasks/system-design/mcp-workspace-mode.md +legacy_specs: [] +--- + +# Implementation Plan: Inherit-Parent Repository Admission + +## Overview + +Reject an MCP subtask before creation when it requests `inherit_parent` while +its explicit repository selection cannot reuse the parent's already +materialized repository inventory. The fix adds read-only task-service +validation, invokes it before creation side effects, and returns guidance to use +`new_workspace` for a different checkout. + +## Confirmed root cause + +MCP subtask creation independently resolves repository inputs and workspace +policy. Explicit repository input overrides the parent's logical task +repositories and fills an omitted branch from the repository default, while an +omitted workspace mode still defaults to `inherit_parent`. The task is inserted +successfully even when the parent's materialized environment has no matching +repository/branch slot. Launch then fails after task and session creation when +canonical inventory admission detects the mismatch. + +## Scope + +### In scope + +- Validate explicit repository inputs and top-level branch overrides for + `inherit_parent` against an existing parent environment before task creation. +- Require exactly one active canonical inventory match per requested slot. +- Return bounded validation guidance to use `new_workspace`. +- Preserve current behavior when no parent environment is materialized. +- Prove rejection creates no task or session. + +### Out of scope + +- Automatically changing workspace mode. +- Rewriting repository or branch input. +- Repairing already-failed tasks. +- Changing launch-time canonical inventory validation. +- UI changes. + +## Technical approach + +Add a read-only task-service method that loads the parent's active environment +and validates explicit `TaskRepositoryInput` values against its repository +inventory using the same repository and sanitized branch identity contract as +launch admission. A request is compatible only when each selected slot has one +active, non-deleted inventory row. + +Call the validator from MCP `create_task` after repository defaults and +`WorkspacePolicy` are resolved, only for an explicit repository list with +effective `inherit_parent` mode. Run it before remote contribution resolution +and `CreateTask`. Return a validation error that recommends +`workspace_mode=new_workspace` without exposing paths or worktree identifiers. + +## Tests + +- `AC-TASKS-MCP-WORKSPACE-MODE-004.5`: a materialized parent with only + feature-branch inventory rejects explicit repository and branch-only + default-branch child selections before creation. +- A uniquely matching explicit repository/branch slot remains allowed. +- A parent without a materialized environment preserves existing behavior. +- Rejection leaves task and session counts unchanged. + +## Work orders + +- [x] [Task 01: Validate inherited repository selections](task-01-validate-inherited-repository-selections.md) + +## Verification results + +- Service coverage passes for rejection, exact-slot acceptance, legacy + unscoped inventory, and pre-materialization passthrough. +- MCP coverage passes for rejection and existing workspace-mode behavior. +- Specification validation and diff checks pass. + +### CI remediation + +- Extracted locator lookup and candidate matching from inherited repository + resolution to satisfy the existing complexity and nesting limits. Admission + behavior and error messages are unchanged, so no requirement or public + documentation update is needed. +- Passed from `apps/backend`: + - `go test -race ./internal/task/service -run '^TestValidateInheritedWorkspaceRepositorySelection' -count=1` + - `go test -race ./internal/mcp/handlers -run '^TestHandleCreateTask_InheritParentRejects' -count=1` + - `golangci-lint run ./... --new-from-rev=db8131436fe06482c883a9027d95e5888ae4658d --timeout=5m` (zero issues). +- Passed `python3 scripts/list-docs.py validate`, + `python3 scripts/lint-spec-files.py --all`, and `git diff --check`. +- Remote CI and review verification remain pending for the remediation commit. + +## Risks + +- Repository matching must use the same sanitized branch identity semantics as + launch admission or creation and launch can disagree. +- Validation must distinguish explicit repository input from repositories + inherited by omission, because only the former can conflict with the caller's + request. diff --git a/docs/plans/inherit-parent-repository-admission/task-01-validate-inherited-repository-selections.md b/docs/plans/inherit-parent-repository-admission/task-01-validate-inherited-repository-selections.md new file mode 100644 index 0000000000..2a82d65f1d --- /dev/null +++ b/docs/plans/inherit-parent-repository-admission/task-01-validate-inherited-repository-selections.md @@ -0,0 +1,101 @@ +--- +id: "01-validate-inherited-repository-selections" +title: "Validate inherited repository selections" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +requirements: + - REQ-TASKS-MCP-WORKSPACE-MODE-004 +acceptance_criteria: + - AC-TASKS-MCP-WORKSPACE-MODE-004.5 +system_design: + - ../../specs/tasks/system-design/mcp-workspace-mode.md +--- + +# Task 01: Validate Inherited Repository Selections + +## Summary + +Add creation-time admission for explicit repository selections combined with +`inherit_parent`. Reject selections that cannot map to exactly one active +repository/branch slot in the parent's materialized environment before any task +or session is created. + +## In scope + +- Add task-service validation over parent environment repository inventory. +- Invoke it from MCP task creation before mutation. +- Return `new_workspace` guidance for incompatible selections. +- Add mismatch and no-side-effect regression coverage. + +## Out of scope + +- Automatic workspace-mode conversion. +- Existing task repair or migration. +- Launch-time inventory matching changes. +- Frontend changes. + +## Acceptance + +- An explicit `repository@main` selection or branch-only `base_branch: main` + override is rejected when the inherited environment contains that + repository only on feature-branch slots. +- One active exact slot is accepted; missing, failed, deleted, or multiple + matches are rejected. +- Rejection occurs before task and session creation and contains no filesystem + path or worktree identity. + +## Verification + +```bash +(cd apps/backend && go test -race ./internal/task/service -run '^TestValidateInheritedWorkspaceRepositorySelection' -count=1) +(cd apps/backend && go test -race ./internal/mcp/handlers -run '^TestHandleCreateTask_InheritParentRejects' -count=1) +(cd apps/backend && golangci-lint run ./... --new-from-rev=db8131436fe06482c883a9027d95e5888ae4658d --timeout=5m) +python3 scripts/list-docs.py validate +python3 scripts/lint-spec-files.py --all +git diff --check -- apps/backend/internal/task/service apps/backend/internal/mcp/handlers docs/specs/tasks docs/plans/inherit-parent-repository-admission +``` + +## Files touched + +- `apps/backend/internal/task/service/inherited_workspace_admission.go` +- `apps/backend/internal/task/service/inherit_parent_repository_admission_test.go` +- `apps/backend/internal/mcp/handlers/handlers.go` +- `apps/backend/internal/mcp/handlers/handlers_test.go` +- `apps/web/e2e/tests/task/subtask.spec.ts` +- `docs/specs/tasks/requirements/mcp-workspace-mode.md` +- `docs/specs/tasks/system-design/mcp-workspace-mode.md` + +## Dependencies + +None. + +## Risks + +- The task-service matcher must stay aligned with launch branch identity rules. + +## Parallelism + +`sequential` + +## Inputs + +- MCP workspace-mode requirements and system design. +- Canonical inventory matching in the orchestrator executor. +- Existing MCP subtask repository-resolution tests. + +## Results + +- RED: the service regression failed to compile because the admission method + did not exist. +- GREEN: service and MCP handler regressions pass after the guard was added. +- Review coverage now includes exact active slots, legacy unscoped inventory, + inherited environments bound through sessions, and pre-materialization + passthrough. +- CI remediation reproduced the `cyclop` and `nestif` failures in + `resolveExistingInheritedRepository`. Extracting locator lookup and candidate + matching preserves the existing behavior and satisfies both limits. +- Both focused race-enabled test commands above pass. The exact CI lint + command reports zero issues; documentation catalog, specification lint, and + diff checks also pass. Remote verification of the remediation remains pending. diff --git a/docs/specs/tasks/requirements/mcp-workspace-mode.md b/docs/specs/tasks/requirements/mcp-workspace-mode.md index c11b12808d..41f310097c 100644 --- a/docs/specs/tasks/requirements/mcp-workspace-mode.md +++ b/docs/specs/tasks/requirements/mcp-workspace-mode.md @@ -120,7 +120,13 @@ from the advertised contract. out-of-enum input before backend dispatch, including `shared`, `shared_group`, empty strings, whitespace-only strings, and padded mode names. Public caller guidance shall explain using omission instead of blank input for defaulting. - Backend handling of blank values shall remain unchanged. +- **AC-TASKS-MCP-WORKSPACE-MODE-004.5:** When an `inherit_parent` task request + supplies explicit repository input or a top-level branch override and the + parent has a materialized environment, every requested repository and branch + slot shall match exactly one active canonical inventory entry. A missing or + ambiguous match shall fail before task or session creation, without mutation, + and shall direct the caller to `new_workspace` when it needs a different + checkout. ## Exclusions diff --git a/docs/specs/tasks/system-design/mcp-workspace-mode.md b/docs/specs/tasks/system-design/mcp-workspace-mode.md index 82fc30bfa6..9a9357789b 100644 --- a/docs/specs/tasks/system-design/mcp-workspace-mode.md +++ b/docs/specs/tasks/system-design/mcp-workspace-mode.md @@ -101,6 +101,24 @@ Kanban root creation in another permitted Kanban workspace must not inherit source repositories owned by a different workspace. Preserve applicable profile precedence after admission. +### Inherited repository compatibility + +An explicit repository selection does not change what `inherit_parent` means: +the child must attach to the parent's existing materialized checkout. Before +task insertion, the MCP creation handler asks the task service to validate +explicit repository inputs against the parent's active task environment when +one exists. Each requested repository plus effective branch identity must match +exactly one active `task_environment_repos` row. Deleted, failed, missing, or +duplicate matches are unsafe. + +The validation runs after repository defaults and workspace policy are resolved +but before remote-contribution registration, task insertion, session creation, +or launch. It is read-only. A mismatch returns the existing validation envelope +with bounded guidance to select `workspace_mode=new_workspace`; it does not +rewrite the repository input, silently change workspace mode, or alter the +parent environment. A parent without a materialized environment retains the +existing launch-time validation path. + Kanban sessions cannot target Office workspaces, Office parents, or project-linked Office tasks, including inconsistent legacy combinations. External callers retain both modes; selecting an Office workspace must not