diff --git a/apps/backend/internal/mcp/handlers/handlers.go b/apps/backend/internal/mcp/handlers/handlers.go index 5bb76d3bdf..0fe7962f3f 100644 --- a/apps/backend/internal/mcp/handlers/handlers.go +++ b/apps/backend/internal/mcp/handlers/handlers.go @@ -738,6 +738,8 @@ func (h *Handlers) handleAddBranchToTask(ctx context.Context, msg *ws.Message) ( var req struct { TaskID string `json:"task_id"` RepositoryID string `json:"repository_id"` + LocalPath string `json:"local_path"` + GitHubURL string `json:"github_url"` BaseBranch string `json:"base_branch"` CheckoutBranch string `json:"checkout_branch"` } @@ -747,12 +749,27 @@ func (h *Handlers) handleAddBranchToTask(ctx context.Context, msg *ws.Message) ( if req.TaskID == "" { return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "task_id is required", nil) } - // repository_id is optional: the service defaults to the task's only - // repository (or its primary row) when omitted. Multi-repo tasks force - // the agent to pass one explicitly via the service-level error. + // Mutual exclusion across the three repo identifiers. resolveRepoInput + // applies a silent precedence (repository_id > github_url > local_path), + // so an agent that accidentally passes two of them gets a behaviour + // change with no signal. Reject early instead so the agent sees the + // mistake. + if locatorCount := boolCount(req.RepositoryID != "", req.LocalPath != "", req.GitHubURL != ""); locatorCount > 1 { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, + "pass at most one of repository_id, github_url, local_path", nil) + } + // repository_id / local_path / github_url are all optional: the service + // defaults to the task's only repository (or its primary row) when none + // is supplied. Multi-repo tasks force the agent to pass one explicitly + // via the service-level error. local_path and github_url are + // agent-ergonomic alternatives — when supplied the service resolves + // them through the same workspace-scoped find-or-create path used by + // create_task. taskRepo, err := h.taskSvc.AddBranchToTask(ctx, service.AddBranchToTaskRequest{ TaskID: req.TaskID, RepositoryID: req.RepositoryID, + LocalPath: req.LocalPath, + GitHubURL: req.GitHubURL, BaseBranch: req.BaseBranch, CheckoutBranch: req.CheckoutBranch, }) @@ -771,6 +788,19 @@ func (h *Handlers) handleAddBranchToTask(ctx context.Context, msg *ws.Message) ( }) } +// boolCount returns how many of the supplied boolean flags are true. Used +// to enforce mutual exclusion across optional input fields without a chain +// of nested ifs. +func boolCount(flags ...bool) int { + n := 0 + for _, b := range flags { + if b { + n++ + } + } + return n +} + // classifyAddBranchError maps service-layer add_branch failures to ws error // codes so MCP agents can react to user-fixable input mistakes (missing // task, duplicate branch, wrong executor) instead of treating them as @@ -793,6 +823,15 @@ func classifyAddBranchError(err error) string { strings.Contains(msg, "only supported on the worktree executor"), strings.Contains(msg, "task_id is required"): return ws.ErrorCodeValidation + case strings.Contains(msg, "GitHub URL"), + strings.Contains(msg, "github.com/owner/repo"), + strings.Contains(msg, "does not belong to workspace"): + // User-fixable failures from ResolveRepositoryRef / parseGitHubRepoURL: + // malformed URL, non-github host, cross-workspace repository_id. + // Narrow patterns (not a broad "resolve repository:" prefix) so + // downstream DB / system errors from CreateRepository / ListRepositories + // still classify as InternalError. + return ws.ErrorCodeValidation } return ws.ErrorCodeInternalError } diff --git a/apps/backend/internal/mcp/handlers/handlers_test.go b/apps/backend/internal/mcp/handlers/handlers_test.go index 5556c2cae4..e0e0784a28 100644 --- a/apps/backend/internal/mcp/handlers/handlers_test.go +++ b/apps/backend/internal/mcp/handlers/handlers_test.go @@ -100,6 +100,24 @@ func (b *mcpRecordingEventBus) Publish(_ context.Context, _ string, event *bus.E return nil } +// TestHandleAddBranchToTask_RejectsMultipleLocators pins the mutual-exclusion +// check at the WS handler tier: supplying two of repository_id / +// repository_url / local_path is an agent mistake that previously got +// silently resolved by the resolveRepoInput precedence chain. Now it +// surfaces as a validation error so the agent sees the contradiction. +func TestHandleAddBranchToTask_RejectsMultipleLocators(t *testing.T) { + h := &Handlers{} + msg := makeWSMessage(t, ws.ActionMCPAddBranchToTask, map[string]interface{}{ + "task_id": "task-1", + "local_path": "/tmp/x", + "github_url": "https://github.com/acme/widgets", + }) + + resp, err := h.handleAddBranchToTask(context.Background(), msg) + require.NoError(t, err) + assertWSError(t, resp, ws.ErrorCodeValidation) +} + func TestHandleCreateTask_MissingTitle(t *testing.T) { h := &Handlers{} msg := makeWSMessage(t, ws.ActionMCPCreateTask, map[string]interface{}{ diff --git a/apps/backend/internal/mcp/server/handlers_test.go b/apps/backend/internal/mcp/server/handlers_test.go index 098939fc46..665d139ad8 100644 --- a/apps/backend/internal/mcp/server/handlers_test.go +++ b/apps/backend/internal/mcp/server/handlers_test.go @@ -5,6 +5,7 @@ import ( "testing" ws "github.com/kandev/kandev/pkg/websocket" + "github.com/mark3labs/mcp-go/mcp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -333,6 +334,78 @@ func TestCreateTask_LocalPath_AllowedForSubtasks(t *testing.T) { assert.Equal(t, "/Users/me/projects/sibling", repos[0]["local_path"]) } +// TestAddBranchToTask_ForwardsRepositoryURL verifies the agent-facing alias: +// repository_url on the MCP tool surface translates to github_url on the WS +// payload — mirroring create_task_kandev's wire format so the backend handler +// 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"}, + } + s := newTaskModeServer(t, backend, "task-current") + + result := callTool(t, s, "add_branch_to_task_kandev", map[string]interface{}{ + "repository_url": "https://github.com/acme/widgets", + "checkout_branch": "feature/x", + }) + + assert.False(t, result.IsError) + assert.Equal(t, ws.ActionMCPAddBranchToTask, backend.lastAction) + + payload, ok := backend.lastPayload.(map[string]interface{}) + require.True(t, ok) + assert.Equal(t, "task-current", payload["task_id"], "task_id should default to current task") + assert.Equal(t, "https://github.com/acme/widgets", payload["github_url"], + "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"]) +} + +// TestAddBranchToTask_ForwardsLocalPath verifies local_path is plumbed through +// to the WS payload so the backend can find-or-create the repo in the task's +// workspace. +func TestAddBranchToTask_ForwardsLocalPath(t *testing.T) { + backend := &testBackend{ + response: map[string]interface{}{"id": "tr-1", "task_id": "task-current"}, + } + s := newTaskModeServer(t, backend, "task-current") + + result := callTool(t, s, "add_branch_to_task_kandev", map[string]interface{}{ + "local_path": "/Users/me/projects/sibling", + "checkout_branch": "feature/y", + }) + + assert.False(t, result.IsError) + payload, ok := backend.lastPayload.(map[string]interface{}) + require.True(t, ok) + assert.Equal(t, "task-current", payload["task_id"], "task_id should default to current task") + assert.Equal(t, "/Users/me/projects/sibling", payload["local_path"]) + assert.Equal(t, "feature/y", payload["checkout_branch"]) + assert.Equal(t, "", payload["repository_id"]) +} + +// TestAddBranchToTask_RejectsMultipleLocators verifies the MCP-tier +// mutual-exclusion check fires before the request hits the WS handler, so +// the error names the agent-facing alias (repository_url) instead of the +// wire field (github_url). +func TestAddBranchToTask_RejectsMultipleLocators(t *testing.T) { + backend := &testBackend{} + s := newTaskModeServer(t, backend, "task-current") + + result := callTool(t, s, "add_branch_to_task_kandev", map[string]interface{}{ + "repository_url": "https://github.com/acme/widgets", + "local_path": "/Users/me/projects/sibling", + }) + + assert.True(t, result.IsError, "passing both repository_url and local_path should error at the MCP tier") + require.NotEmpty(t, result.Content) + text, ok := result.Content[0].(mcp.TextContent) + require.True(t, ok) + assert.Contains(t, text.Text, "repository_url", + "MCP-tier error should name the agent-facing alias, not the wire key") + assert.Nil(t, backend.lastPayload, "request must not be forwarded to the backend") +} + func TestMessageTask_ForwardsToBackend(t *testing.T) { backend := &testBackend{ response: map[string]interface{}{ diff --git a/apps/backend/internal/mcp/server/server.go b/apps/backend/internal/mcp/server/server.go index d5a8cceae0..075e0e435a 100644 --- a/apps/backend/internal/mcp/server/server.go +++ b/apps/backend/internal/mcp/server/server.go @@ -46,10 +46,27 @@ const ( const ( mcpKeyTaskID = "task_id" mcpKeyRepositoryID = "repository_id" + mcpKeyRepositoryURL = "repository_url" + mcpKeyLocalPath = "local_path" + mcpKeyGitHubURL = "github_url" mcpKeyBaseBranch = "base_branch" mcpKeyCheckoutBranch = "checkout_branch" ) +// locatorCount returns how many of the supplied repository-locator strings +// are non-empty. Used by add_branch / create_task mutual-exclusion checks +// so a chain of `if a != "" && b != "" { ... }` doesn't repeat at each call +// site. +func locatorCount(locators ...string) int { + n := 0 + for _, s := range locators { + if s != "" { + n++ + } + } + return n +} + // normalizeMode returns a valid MCP mode, defaulting unknown values to ModeTask. func normalizeMode(mode string) string { switch mode { @@ -558,12 +575,14 @@ Use this when the task should open more than one PR — same repo with different 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. - task_id defaults to your CURRENT task when omitted — pass it explicitly only to target a different task. -- repository_id is OPTIONAL when the task has a single repository attached (the common case). The service auto-resolves to that repo. Multi-repo tasks must pass it explicitly. +- Repository selection (matches create_task_kandev): pass exactly one of repository_id / repository_url / local_path. For single-repo tasks all three are optional — the service auto-resolves to the task's only repository. Multi-repo tasks must identify the target repo explicitly. - checkout_branch is the branch the new worktree will check out. Leave empty to create a fresh feature branch from base_branch. - base_branch is optional; defaults to the repository's default_branch. - The (task_id, repository_id, base_branch, checkout_branch) tuple must be unique on the task — re-adding the same combination is an error, not a no-op.`), mcp.WithString("task_id", mcp.Description("The task to attach the branch to. Defaults to the current task when omitted.")), - mcp.WithString("repository_id", mcp.Description("Repository to attach. Optional for single-repo tasks (auto-resolved). Required for multi-repo tasks.")), + mcp.WithString("repository_id", mcp.Description("Repository UUID. Optional for single-repo tasks (auto-resolved). Required for multi-repo tasks unless repository_url or local_path is supplied.")), + mcp.WithString("repository_url", mcp.Description("GitHub repository URL (e.g. 'https://github.com/owner/repo'). Alternative to repository_id when you don't have the UUID handy. The repository is found-or-created in the task's workspace.")), + mcp.WithString("local_path", mcp.Description("Local repository folder path (e.g. '/Users/me/projects/myrepo'). Alternative to repository_id for the local worktree flow. The repository is found-or-created in the task's workspace.")), mcp.WithString("checkout_branch", mcp.Description("Existing branch to check out in the new worktree (e.g. a PR head branch). Empty to create a fresh feature branch from base_branch.")), mcp.WithString("base_branch", mcp.Description("Branch to base the worktree on. Defaults to the repository's default_branch.")), ), @@ -580,9 +599,24 @@ func (s *Server) addBranchToTaskHandler() server.ToolHandlerFunc { if taskID == "" { return mcp.NewToolResultError("task_id is required (no current task context to default to)"), nil } + // Mutual-exclusion gate at the MCP tier so the error names the + // agent-facing alias (repository_url) instead of the WS wire field + // (github_url). The WS handler still re-validates for direct WS + // callers that don't go through this tool. + repositoryID := req.GetString(mcpKeyRepositoryID, "") + repositoryURL := req.GetString(mcpKeyRepositoryURL, "") + localPath := req.GetString(mcpKeyLocalPath, "") + if locatorCount(repositoryID, repositoryURL, localPath) > 1 { + return mcp.NewToolResultError("pass at most one of repository_id, repository_url, local_path"), nil + } + // repository_url is the tool-facing alias used by create_task_kandev; + // translate to github_url on the wire so the WS handler can reuse the + // same field name as the rest of the multi-repo payloads. payload := map[string]interface{}{ mcpKeyTaskID: taskID, - mcpKeyRepositoryID: req.GetString(mcpKeyRepositoryID, ""), + mcpKeyRepositoryID: repositoryID, + mcpKeyLocalPath: localPath, + mcpKeyGitHubURL: repositoryURL, mcpKeyCheckoutBranch: req.GetString(mcpKeyCheckoutBranch, ""), mcpKeyBaseBranch: req.GetString(mcpKeyBaseBranch, ""), } diff --git a/apps/backend/internal/task/service/service_branches.go b/apps/backend/internal/task/service/service_branches.go index ad5b0b29ea..88f5d30a99 100644 --- a/apps/backend/internal/task/service/service_branches.go +++ b/apps/backend/internal/task/service/service_branches.go @@ -15,9 +15,18 @@ import ( // AddBranchToTaskRequest carries the parameters for adding a new branch // (worktree) to an existing task. The (RepositoryID, CheckoutBranch) pair // must not already exist on the task. +// +// RepositoryID, LocalPath, and GitHubURL are alternative ways to identify the +// target repository — mirrors the create_task input shape. When RepositoryID +// is empty and either LocalPath or GitHubURL is set, the service resolves to +// (or creates) the matching repository in the task's workspace. Pure +// RepositoryID is the fast path; the other two are agent-ergonomic +// alternatives so callers don't have to look up the UUID first. type AddBranchToTaskRequest struct { TaskID string RepositoryID string + LocalPath string + GitHubURL string BaseBranch string CheckoutBranch string } @@ -31,16 +40,20 @@ type AddBranchToTaskRequest struct { // // Constraints: // - TaskID is required. -// - RepositoryID is optional. When omitted, the service defaults to the -// task's existing repository (the only one for single-repo tasks; the -// primary by lowest position otherwise). Multi-repo tasks must pass an -// explicit RepositoryID — defaulting would be ambiguous. +// - RepositoryID, LocalPath, and GitHubURL are alternative ways to identify +// the target repository. All three are optional: when none is supplied +// the service defaults to the task's existing repository (the only one +// for single-repo tasks; the primary by lowest position otherwise). +// Multi-repo tasks must pass one explicitly — defaulting would be +// ambiguous. LocalPath and GitHubURL are resolved (find-or-create) +// through the same workspace-scoped path used by create_task. // - The (TaskID, RepositoryID, BaseBranch, CheckoutBranch) tuple must be // unique on the task — duplicates surface as a typed error so callers // can render a user-friendly message instead of a raw DB error. -// - The repository must belong to the task's workspace; the caller is -// responsible for that check (service_tasks.resolveRepoInput enforces it -// during create, this method assumes the same upstream gating). +// - The repository must belong to the task's workspace. The resolution +// 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) { task, existing, err := s.prepareBranchAdd(ctx, &req) if err != nil { @@ -107,9 +120,33 @@ func (s *Service) prepareBranchAdd(ctx context.Context, req *AddBranchToTaskRequ // rather than substring-matching the formatted UUID. return nil, nil, fmt.Errorf("%w: %s", taskrepo.ErrTaskNotFound, req.TaskID) } + // Executor gate runs BEFORE repository resolution so a rejection on a + // non-worktree executor can't leak an orphan Repository row into the + // workspace via FindOrCreateRepository / CreateRepository inside + // ResolveRepositoryRef. Mirrors the "keeps the DB clean" invariant + // documented on requireWorktreeExecutorForBranchAdd itself. if err := s.requireWorktreeExecutorForBranchAdd(ctx, req.TaskID); err != nil { return nil, nil, err } + // Resolve LocalPath / GitHubURL to a concrete RepositoryID up front so the + // rest of the flow (duplicate scan, row insert) operates on a stable UUID. + // Skipped when RepositoryID is already set or neither alternative + // identifier was supplied (the single-repo task default applies after + // the existing-rows lookup). + if req.RepositoryID == "" && (req.LocalPath != "" || req.GitHubURL != "") { + resolvedID, resolvedBranch, resolveErr := s.ResolveRepositoryRef(ctx, task.WorkspaceID, TaskRepositoryInput{ + LocalPath: req.LocalPath, + GitHubURL: req.GitHubURL, + BaseBranch: req.BaseBranch, + }) + if resolveErr != nil { + return nil, nil, fmt.Errorf("resolve repository: %w", resolveErr) + } + req.RepositoryID = resolvedID + if req.BaseBranch == "" { + req.BaseBranch = resolvedBranch + } + } existing, err := s.taskRepos.ListTaskRepositories(ctx, req.TaskID) if err != nil { return nil, nil, fmt.Errorf("list existing branches: %w", err) diff --git a/apps/backend/internal/task/service/service_branches_test.go b/apps/backend/internal/task/service/service_branches_test.go index 5aed524331..16d70826b6 100644 --- a/apps/backend/internal/task/service/service_branches_test.go +++ b/apps/backend/internal/task/service/service_branches_test.go @@ -439,6 +439,181 @@ func TestCreateTask_AllowsSameRepoDifferentBaseBranches(t *testing.T) { } } +// TestAddBranchToTask_ResolvesGitHubURL exercises the agent-ergonomic path +// where the caller knows the GitHub URL but not the repository UUID. The +// service must find-or-create the repository in the task's workspace and +// attach the new branch row against the resolved id. +func TestAddBranchToTask_ResolvesGitHubURL(t *testing.T) { + svc, _, repo := createTestService(t) + ctx := context.Background() + + _ = repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-1", Name: "WS"}) + _ = repo.CreateWorkflow(ctx, &models.Workflow{ID: "wf-1", WorkspaceID: "ws-1", Name: "WF"}) + // Seed the target repo with provider info so FindOrCreateRepository + // returns the existing row instead of creating a duplicate. + _ = repo.CreateRepository(ctx, &models.Repository{ + ID: "repo-target", WorkspaceID: "ws-1", Name: "acme/widgets", + Provider: "github", ProviderOwner: "acme", ProviderName: "widgets", + DefaultBranch: "main", + }) + _ = repo.CreateRepository(ctx, &models.Repository{ + ID: "repo-primary", WorkspaceID: "ws-1", Name: "primary", DefaultBranch: "main", + }) + + // Multi-repo task — auto-default would fail, so URL resolution must + // supply the repository_id. + task, err := svc.CreateTask(ctx, &CreateTaskRequest{ + WorkspaceID: "ws-1", + WorkflowID: "wf-1", + WorkflowStepID: "step-1", + Title: "Multi-repo task", + Repositories: []TaskRepositoryInput{ + {RepositoryID: "repo-primary", BaseBranch: "main"}, + }, + }) + if err != nil { + t.Fatalf("CreateTask: %v", err) + } + + added, err := svc.AddBranchToTask(ctx, AddBranchToTaskRequest{ + TaskID: task.ID, + GitHubURL: "https://github.com/acme/widgets", + CheckoutBranch: "feature/x", + }) + if err != nil { + t.Fatalf("AddBranchToTask with github_url: %v", err) + } + if added.RepositoryID != "repo-target" { + t.Errorf("expected resolved repository_id=repo-target, got %q", added.RepositoryID) + } + if added.BaseBranch != "main" { + t.Errorf("expected base_branch defaulted to repo default (main), got %q", added.BaseBranch) + } +} + +// TestAddBranchToTask_ResolvesLocalPath exercises the local-worktree flow: +// caller supplies the on-disk folder path, the service finds the matching +// repository row in the task's workspace and attaches against it. +func TestAddBranchToTask_ResolvesLocalPath(t *testing.T) { + svc, _, repo := createTestService(t) + ctx := context.Background() + + _ = repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-1", Name: "WS"}) + _ = repo.CreateWorkflow(ctx, &models.Workflow{ID: "wf-1", WorkspaceID: "ws-1", Name: "WF"}) + _ = repo.CreateRepository(ctx, &models.Repository{ + ID: "repo-primary", WorkspaceID: "ws-1", Name: "primary", DefaultBranch: "main", + }) + _ = repo.CreateRepository(ctx, &models.Repository{ + ID: "repo-by-path", WorkspaceID: "ws-1", Name: "sibling", + LocalPath: "/tmp/sibling", DefaultBranch: "develop", + }) + + task, err := svc.CreateTask(ctx, &CreateTaskRequest{ + WorkspaceID: "ws-1", + WorkflowID: "wf-1", + WorkflowStepID: "step-1", + Title: "Multi-repo task", + Repositories: []TaskRepositoryInput{ + {RepositoryID: "repo-primary", BaseBranch: "main"}, + }, + }) + if err != nil { + t.Fatalf("CreateTask: %v", err) + } + + added, err := svc.AddBranchToTask(ctx, AddBranchToTaskRequest{ + TaskID: task.ID, + LocalPath: "/tmp/sibling", + CheckoutBranch: "feature/y", + }) + if err != nil { + t.Fatalf("AddBranchToTask with local_path: %v", err) + } + if added.RepositoryID != "repo-by-path" { + t.Errorf("expected resolved repository_id=repo-by-path, got %q", added.RepositoryID) + } + if added.BaseBranch != "develop" { + t.Errorf("expected base_branch defaulted to repo default (develop), got %q", added.BaseBranch) + } +} + +// TestAddBranchToTask_RejectsNonWorktreeExecutor_NoOrphanRepo pins the +// ordering guarantee documented on requireWorktreeExecutorForBranchAdd: +// the executor gate must fire BEFORE ResolveRepositoryRef so a rejected +// add_branch call on a non-worktree task never leaks a freshly-created +// Repository row into the workspace via the github_url / local_path paths. +// +// Without this ordering, FindOrCreateRepository (github_url) and +// CreateRepository (local_path) write before the executor check rejects, +// leaving an orphan repository the user never asked for. +func TestAddBranchToTask_RejectsNonWorktreeExecutor_NoOrphanRepo(t *testing.T) { + svc, _, repo := createTestService(t) + ctx := context.Background() + + _ = repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-1", Name: "WS"}) + _ = repo.CreateWorkflow(ctx, &models.Workflow{ID: "wf-1", WorkspaceID: "ws-1", Name: "WF"}) + _ = repo.CreateRepository(ctx, &models.Repository{ID: "repo-primary", WorkspaceID: "ws-1", Name: "primary", DefaultBranch: "main"}) + + task, err := svc.CreateTask(ctx, &CreateTaskRequest{ + WorkspaceID: "ws-1", + WorkflowID: "wf-1", + WorkflowStepID: "step-1", + Title: "Docker task", + Repositories: []TaskRepositoryInput{ + {RepositoryID: "repo-primary", BaseBranch: "main"}, + }, + }) + if err != nil { + t.Fatalf("CreateTask: %v", err) + } + + // Seed a docker executor — add_branch must reject. + now := time.Now().UTC() + if err := repo.CreateTaskEnvironment(ctx, &models.TaskEnvironment{ + ID: "env-1", + TaskID: task.ID, + ExecutorType: string(models.ExecutorTypeLocalDocker), + Status: "ready", + CreatedAt: now, + UpdatedAt: now, + }); err != nil { + t.Fatalf("CreateTaskEnvironment: %v", err) + } + + before, err := repo.ListRepositories(ctx, "ws-1") + if err != nil { + t.Fatalf("ListRepositories before: %v", err) + } + beforeCount := len(before) + + // github_url points at a repo NOT yet in the workspace — would trigger + // FindOrCreateRepository if resolution ran. The executor check must + // reject first. + _, err = svc.AddBranchToTask(ctx, AddBranchToTaskRequest{ + TaskID: task.ID, + GitHubURL: "https://github.com/acme/never-created", + }) + if err == nil { + t.Fatal("expected error rejecting non-worktree executor") + } + if !strings.Contains(err.Error(), "worktree executor") { + t.Errorf("unexpected error: %v", err) + } + + after, err := repo.ListRepositories(ctx, "ws-1") + if err != nil { + t.Fatalf("ListRepositories after: %v", err) + } + if len(after) != beforeCount { + t.Errorf("expected no new repository rows, got %d before vs %d after", beforeCount, len(after)) + } + for _, r := range after { + if r.ProviderOwner == "acme" && r.ProviderName == "never-created" { + t.Errorf("orphan repository row leaked into workspace: %+v", r) + } + } +} + // TestCreateTask_RejectsSameRepoSameBranch verifies the dedup guard still // fires when the exact (repo, branch) pair is duplicated in the create // payload — the migration-layer constraint mirrors the in-memory guard. diff --git a/apps/backend/internal/task/service/service_tasks.go b/apps/backend/internal/task/service/service_tasks.go index 8d4642145b..df32540521 100644 --- a/apps/backend/internal/task/service/service_tasks.go +++ b/apps/backend/internal/task/service/service_tasks.go @@ -373,6 +373,34 @@ func (s *Service) repoDisplayLabel(ctx context.Context, repoInput TaskRepository return repositoryID } +// ResolveRepositoryRef resolves a single TaskRepositoryInput to a +// (repositoryID, baseBranch) pair within the given workspace, creating the +// repository if necessary. Mirrors the resolution used during task creation +// (`createTaskRepositories`), but builds the local-path lookup map on demand +// so callers that only resolve one input (e.g. add_branch) don't need to +// thread the map themselves. +// +// Accepts inputs identified by RepositoryID, GitHubURL, or LocalPath. Returns +// an empty repositoryID with no error when none of those are set, letting +// callers decide whether to fall back to other defaults. +func (s *Service) ResolveRepositoryRef(ctx context.Context, workspaceID string, repoInput TaskRepositoryInput) (repositoryID, baseBranch string, err error) { + var repoByPath map[string]*models.Repository + if repoInput.RepositoryID == "" && repoInput.LocalPath != "" { + repos, listErr := s.repoEntities.ListRepositories(ctx, workspaceID) + if listErr != nil { + return "", "", listErr + } + repoByPath = make(map[string]*models.Repository, len(repos)) + for _, repo := range repos { + if repo.LocalPath == "" { + continue + } + repoByPath[repo.LocalPath] = repo + } + } + return s.resolveRepoInput(ctx, workspaceID, repoInput, repoByPath) +} + // resolveRepoInput resolves a RepositoryInput to a repositoryID and baseBranch, // creating the repository if it doesn't exist yet. func (s *Service) resolveRepoInput(ctx context.Context, workspaceID string, repoInput TaskRepositoryInput, repoByPath map[string]*models.Repository) (repositoryID, baseBranch string, err error) {