From a30dd395664522d5911e1839e2227c3de4f67dcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Wed, 1 Jul 2026 21:25:05 +0100 Subject: [PATCH] fix: avoid stale task worktree repository launches (#1568) --- apps/backend/internal/backendapp/services.go | 6 +- .../task/handlers/task_http_handlers.go | 2 + .../task/handlers/task_http_handlers_test.go | 22 + .../task/handlers/task_ws_handlers.go | 2 + .../task/service/repository_discovery.go | 12 +- .../task/service/service_resources.go | 11 +- .../internal/task/service/service_tasks.go | 177 ++++++- .../internal/task/service/service_test.go | 484 ++++++++++++++++++ .../my-github/quick-task-launcher.test.tsx | 254 +++++++++ .../github/my-github/quick-task-launcher.tsx | 68 ++- ...k-create-dialog-helpers.multi-repo.test.ts | 63 ++- .../components/task-create-dialog-helpers.ts | 90 +++- .../task-create-dialog-repositories-state.ts | 20 +- .../task-create-dialog-state.test.ts | 66 +++ .../components/task-create-dialog-state.ts | 10 +- .../components/task-create-dialog-types.ts | 5 + .../pr-action-create-task-dialog.spec.ts | 68 +++ .../task/create-task-disabled-tooltip.spec.ts | 22 +- .../task/create-task-from-pr-list.spec.ts | 21 +- apps/web/lib/api/domains/kanban-api.ts | 1 + 20 files changed, 1336 insertions(+), 68 deletions(-) create mode 100644 apps/web/components/github/my-github/quick-task-launcher.test.tsx create mode 100644 apps/web/e2e/tests/github/pr-action-create-task-dialog.spec.ts diff --git a/apps/backend/internal/backendapp/services.go b/apps/backend/internal/backendapp/services.go index e6e00e450f..81a0a80705 100644 --- a/apps/backend/internal/backendapp/services.go +++ b/apps/backend/internal/backendapp/services.go @@ -3,6 +3,7 @@ package backendapp import ( "context" "fmt" + "path/filepath" "go.uber.org/zap" @@ -70,8 +71,9 @@ func provideServices(cfg *config.Config, log *logger.Logger, repos *Repositories eventBus, log, taskservice.RepositoryDiscoveryConfig{ - Roots: cfg.RepositoryDiscovery.Roots, - MaxDepth: cfg.RepositoryDiscovery.MaxDepth, + Roots: cfg.RepositoryDiscovery.Roots, + MaxDepth: cfg.RepositoryDiscovery.MaxDepth, + TaskWorktreeRoots: []string{filepath.Join(cfg.ResolvedHomeDir(), "tasks")}, }, ) diff --git a/apps/backend/internal/task/handlers/task_http_handlers.go b/apps/backend/internal/task/handlers/task_http_handlers.go index 4f50465a80..d85cb8ab2d 100644 --- a/apps/backend/internal/task/handlers/task_http_handlers.go +++ b/apps/backend/internal/task/handlers/task_http_handlers.go @@ -396,6 +396,7 @@ type httpTaskRepositoryInput struct { RepositoryID string `json:"repository_id"` BaseBranch string `json:"base_branch"` CheckoutBranch string `json:"checkout_branch"` + PRNumber int `json:"pr_number,omitempty"` LocalPath string `json:"local_path"` Name string `json:"name"` DefaultBranch string `json:"default_branch"` @@ -811,6 +812,7 @@ func convertCreateTaskRepositories(c *gin.Context, inputs []httpTaskRepositoryIn RepositoryID: r.RepositoryID, BaseBranch: r.BaseBranch, CheckoutBranch: r.CheckoutBranch, + PRNumber: r.PRNumber, LocalPath: r.LocalPath, Name: r.Name, DefaultBranch: r.DefaultBranch, diff --git a/apps/backend/internal/task/handlers/task_http_handlers_test.go b/apps/backend/internal/task/handlers/task_http_handlers_test.go index 8cf618638f..9bfeaf94ff 100644 --- a/apps/backend/internal/task/handlers/task_http_handlers_test.go +++ b/apps/backend/internal/task/handlers/task_http_handlers_test.go @@ -317,6 +317,28 @@ func TestHTTPCreateTaskRecordsRepositoryWithoutProfileIDs(t *testing.T) { }, recorder.got) } +func TestConvertCreateTaskRepositoriesForwardsPRNumber(t *testing.T) { + gin.SetMode(gin.TestMode) + rec := httptest.NewRecorder() + c, _ := gin.CreateTestContext(rec) + + repos, ok := convertCreateTaskRepositories(c, []httpTaskRepositoryInput{{ + GitHubURL: "https://github.com/kdlbs/kandev", + BaseBranch: "main", + CheckoutBranch: "feature/fork-pr", + PRNumber: 1567, + }}) + + require.True(t, ok) + require.Len(t, repos, 1) + assert.Equal(t, dto.TaskRepositoryInput{ + BaseBranch: "main", + CheckoutBranch: "feature/fork-pr", + PRNumber: 1567, + GitHubURL: "https://github.com/kdlbs/kandev", + }, repos[0]) +} + func TestBuildTaskCreateLastUsedPatchRecordsFirstWorkspaceRepository(t *testing.T) { patch := buildTaskCreateLastUsedPatch(httpCreateTaskRequest{ AgentProfileID: "agent-2", diff --git a/apps/backend/internal/task/handlers/task_ws_handlers.go b/apps/backend/internal/task/handlers/task_ws_handlers.go index 67719b3e48..99d24de69e 100644 --- a/apps/backend/internal/task/handlers/task_ws_handlers.go +++ b/apps/backend/internal/task/handlers/task_ws_handlers.go @@ -123,6 +123,7 @@ func (h *TaskHandlers) wsCreateTask(ctx context.Context, msg *ws.Message) (*ws.M RepositoryID: r.RepositoryID, BaseBranch: r.BaseBranch, CheckoutBranch: r.CheckoutBranch, + PRNumber: r.PRNumber, LocalPath: r.LocalPath, Name: r.Name, DefaultBranch: r.DefaultBranch, @@ -258,6 +259,7 @@ func (h *TaskHandlers) wsUpdateTask(ctx context.Context, msg *ws.Message) (*ws.M RepositoryID: r.RepositoryID, BaseBranch: r.BaseBranch, CheckoutBranch: r.CheckoutBranch, + PRNumber: r.PRNumber, LocalPath: r.LocalPath, Name: r.Name, DefaultBranch: r.DefaultBranch, diff --git a/apps/backend/internal/task/service/repository_discovery.go b/apps/backend/internal/task/service/repository_discovery.go index 16191fff92..977a8b4d66 100644 --- a/apps/backend/internal/task/service/repository_discovery.go +++ b/apps/backend/internal/task/service/repository_discovery.go @@ -15,8 +15,9 @@ import ( ) type RepositoryDiscoveryConfig struct { - Roots []string - MaxDepth int + Roots []string + MaxDepth int + TaskWorktreeRoots []string } // LocalRepoStatus reports the current branch and dirty file paths for a @@ -59,10 +60,13 @@ var ErrPathNotAllowed = errors.New("path is not within an allowed root") const gitHEAD = "HEAD" // sourceTypeLocal is the Repository.SourceType value for on-machine repos -// (a path the user discovered or added manually). Provider-backed repos use -// other values like "provider". +// (a path the user discovered or added manually). const sourceTypeLocal = "local" +// sourceTypeProvider is the Repository.SourceType value for provider-backed +// repos that can be cloned/synced from their upstream identity. +const sourceTypeProvider = "provider" + func (s *Service) DiscoverLocalRepositories(ctx context.Context, root string) (RepositoryDiscoveryResult, error) { roots := s.discoveryRoots() if root != "" { diff --git a/apps/backend/internal/task/service/service_resources.go b/apps/backend/internal/task/service/service_resources.go index 06f527edcc..28b45f7dde 100644 --- a/apps/backend/internal/task/service/service_resources.go +++ b/apps/backend/internal/task/service/service_resources.go @@ -517,7 +517,7 @@ func (s *Service) ReorderWorkflows(ctx context.Context, workspaceID string, work func (s *Service) CreateRepository(ctx context.Context, req *CreateRepositoryRequest) (*models.Repository, error) { sourceType := req.SourceType if sourceType == "" { - sourceType = "local" + sourceType = sourceTypeLocal } prefix := strings.TrimSpace(req.WorktreeBranchPrefix) if err := worktree.ValidateBranchPrefix(prefix); err != nil { @@ -593,6 +593,11 @@ func (s *Service) FindOrCreateRepository(ctx context.Context, req *FindOrCreateR return nil, false, fmt.Errorf("lookup repository: %w", err) } if existing != nil { + replacement, replacementCreated, replacementErr := s.replaceTaskWorktreeRepositoryMatch(ctx, req.WorkspaceID, existing) + if replacementErr != nil { + return nil, false, replacementErr + } + existing = replacement dirty := false if existing.LocalPath == "" && req.LocalPath != "" { existing.LocalPath = req.LocalPath @@ -612,14 +617,14 @@ func (s *Service) FindOrCreateRepository(ctx context.Context, req *FindOrCreateR zap.String("repository_id", existing.ID), zap.Error(updateErr)) } } - return existing, false, nil + return existing, replacementCreated, nil } name := fmt.Sprintf("%s/%s", req.ProviderOwner, req.ProviderName) created, createErr := s.CreateRepository(ctx, &CreateRepositoryRequest{ WorkspaceID: req.WorkspaceID, Name: name, - SourceType: "provider", + SourceType: sourceTypeProvider, LocalPath: req.LocalPath, Provider: req.Provider, ProviderOwner: req.ProviderOwner, diff --git a/apps/backend/internal/task/service/service_tasks.go b/apps/backend/internal/task/service/service_tasks.go index 4c874b19bd..3628813240 100644 --- a/apps/backend/internal/task/service/service_tasks.go +++ b/apps/backend/internal/task/service/service_tasks.go @@ -25,6 +25,8 @@ import ( // Used when a caller omits priority so the DB CHECK constraint is satisfied. const defaultPriority = "medium" +const defaultKandevTaskWorktreePathSegment = "/.kandev/tasks/" + // ErrSubtaskDepthExceeded is returned when a caller tries to create a // subtask of a kanban subtask (nesting depth > 1). Office task trees are // intentionally exempt. @@ -421,19 +423,7 @@ func (s *Service) resolveRepoInput(ctx context.Context, workspaceID string, repo repositoryID = repoInput.RepositoryID baseBranch = repoInput.BaseBranch if repositoryID != "" { - // Verify the repository belongs to the target workspace. Without this - // check, an agent that knows a repository UUID from another workspace - // could associate it with a task in this workspace via the MCP tool's - // repository_id fast path (the github_url and local_path branches both - // scope through FindOrCreateRepository, which is workspace-bound). - repo, lookupErr := s.repoEntities.GetRepository(ctx, repositoryID) - if lookupErr != nil { - return "", "", false, fmt.Errorf("looking up repository %q: %w", repositoryID, lookupErr) - } - if repo == nil || repo.WorkspaceID != workspaceID { - return "", "", false, fmt.Errorf("repository %q does not belong to workspace %q", repositoryID, workspaceID) - } - return repositoryID, baseBranch, false, nil + return s.resolveRepoInputID(ctx, workspaceID, repositoryID, baseBranch) } // Handle GitHub URL: parse owner/name and use FindOrCreateRepository @@ -447,6 +437,157 @@ func (s *Service) resolveRepoInput(ctx context.Context, workspaceID string, repo return s.resolveRepoInputLocal(ctx, workspaceID, repoInput, repoByPath, baseBranch) } +func (s *Service) resolveRepoInputID(ctx context.Context, workspaceID, repositoryID, baseBranch string) (string, string, bool, error) { + // Verify the repository belongs to the target workspace. Without this + // check, an agent that knows a repository UUID from another workspace + // could associate it with a task in this workspace via the MCP tool's + // repository_id fast path (the github_url and local_path branches both + // scope through FindOrCreateRepository, which is workspace-bound). + repo, lookupErr := s.repoEntities.GetRepository(ctx, repositoryID) + if lookupErr != nil { + return "", "", false, fmt.Errorf("looking up repository %q: %w", repositoryID, lookupErr) + } + if repo == nil || repo.WorkspaceID != workspaceID { + return "", "", false, fmt.Errorf("repository %q does not belong to workspace %q", repositoryID, workspaceID) + } + replacementID, replacementCreated, replacementErr := s.safeRepositoryIDForTaskWorktree(ctx, workspaceID, repo) + if replacementErr != nil { + return "", "", false, replacementErr + } + if replacementID != "" { + return replacementID, baseBranch, replacementCreated, nil + } + return repositoryID, baseBranch, false, nil +} + +func (s *Service) safeRepositoryIDForTaskWorktree(ctx context.Context, workspaceID string, repo *models.Repository) (string, bool, error) { + if !s.isKandevTaskWorktreeRepository(repo) { + return "", false, nil + } + if repo.Provider == "" || repo.ProviderOwner == "" || repo.ProviderName == "" { + return "", false, fmt.Errorf("repository %q points at a Kandev task worktree; use the source repository or GitHub URL", repo.ID) + } + existing, err := s.findSafeReplacementRepository(ctx, workspaceID, repo) + if err != nil { + return "", false, err + } + if existing != nil { + return existing.ID, false, nil + } + created, createErr := s.CreateRepository(ctx, &CreateRepositoryRequest{ + WorkspaceID: workspaceID, + Name: repo.ProviderOwner + "/" + repo.ProviderName, + SourceType: sourceTypeProvider, + Provider: repo.Provider, + ProviderRepoID: repo.ProviderRepoID, + ProviderOwner: repo.ProviderOwner, + ProviderName: repo.ProviderName, + DefaultBranch: repo.DefaultBranch, + }) + if createErr != nil { + return "", false, fmt.Errorf("create provider repository for task worktree %q: %w", repo.ID, createErr) + } + return created.ID, true, nil +} + +func (s *Service) replaceTaskWorktreeRepositoryMatch(ctx context.Context, workspaceID string, repo *models.Repository) (*models.Repository, bool, error) { + replacementID, replacementCreated, err := s.safeRepositoryIDForTaskWorktree(ctx, workspaceID, repo) + if err != nil { + return nil, false, err + } + if replacementID == "" { + return repo, false, nil + } + replacement, lookupErr := s.repoEntities.GetRepository(ctx, replacementID) + if lookupErr != nil { + return nil, false, fmt.Errorf("looking up repository %q: %w", replacementID, lookupErr) + } + if replacement == nil { + return nil, false, fmt.Errorf("replacement repository %q no longer exists", replacementID) + } + return replacement, replacementCreated, nil +} + +// findSafeReplacementRepository prefers an existing safe local clone over a +// provider row so private/offline repositories can reuse the user's checkout. +func (s *Service) findSafeReplacementRepository(ctx context.Context, workspaceID string, repo *models.Repository) (*models.Repository, error) { + repos, err := s.repoEntities.ListRepositories(ctx, workspaceID) + if err != nil { + return nil, fmt.Errorf("list repositories for task worktree replacement: %w", err) + } + var localClone *models.Repository + var providerRepo *models.Repository + for _, candidate := range repos { + if candidate == nil || candidate.ID == repo.ID { + continue + } + if !sameProviderIdentity(repo, candidate) { + continue + } + if s.isKandevTaskWorktreeRepository(candidate) { + continue + } + if candidate.SourceType == sourceTypeLocal && candidate.LocalPath != "" { + if localClone == nil { + localClone = candidate + } + continue + } + if candidate.SourceType == sourceTypeProvider && providerRepo == nil { + providerRepo = candidate + } + } + if localClone != nil { + return localClone, nil + } + if providerRepo != nil { + return providerRepo, nil + } + return nil, nil +} + +func sameProviderIdentity(left, right *models.Repository) bool { + return left.Provider == right.Provider && + left.ProviderOwner == right.ProviderOwner && + left.ProviderName == right.ProviderName +} + +func (s *Service) isKandevTaskWorktreeRepository(repo *models.Repository) bool { + return repo != nil && isKandevTaskWorktreePath(repo.LocalPath, s.discoveryConfig.TaskWorktreeRoots) +} + +func isKandevTaskWorktreePath(path string, taskWorktreeRoots []string) bool { + normalized := normalizeTaskWorktreePath(path) + if normalized == "" { + return false + } + for _, root := range taskWorktreeRoots { + if pathAtOrInsideRoot(normalized, normalizeTaskWorktreePath(root)) { + return true + } + } + return strings.Contains(normalized, defaultKandevTaskWorktreePathSegment) || + strings.HasSuffix(normalized, strings.TrimSuffix(defaultKandevTaskWorktreePathSegment, "/")) +} + +func normalizeTaskWorktreePath(path string) string { + normalized := filepath.ToSlash(filepath.Clean(strings.TrimSpace(path))) + if normalized == "." || normalized == "" { + return "" + } + return normalized +} + +func pathAtOrInsideRoot(path, root string) bool { + if path == "" || root == "" { + return false + } + if root != "/" { + root = strings.TrimRight(root, "/") + } + return path == root || strings.HasPrefix(path, root+"/") +} + // resolveRepoInputLocal handles the LocalPath branch of resolveRepoInput. // Looks the path up in the workspace snapshot; on miss, calls // CreateRepository (and reports created=true). Extracted to keep @@ -458,6 +599,9 @@ func (s *Service) resolveRepoInputLocal( repo := repoByPath[repoInput.LocalPath] created := false if repo == nil { + if isKandevTaskWorktreePath(repoInput.LocalPath, s.discoveryConfig.TaskWorktreeRoots) { + return "", "", false, fmt.Errorf("local path %q points at a Kandev task worktree; use the source repository or GitHub URL", repoInput.LocalPath) + } name := strings.TrimSpace(repoInput.Name) if name == "" { name = filepath.Base(repoInput.LocalPath) @@ -497,6 +641,13 @@ func (s *Service) resolveRepoInputLocal( repoByPath[repoInput.LocalPath] = repo } created = true + } else { + replacement, replacementCreated, replaceErr := s.replaceTaskWorktreeRepositoryMatch(ctx, workspaceID, repo) + if replaceErr != nil { + return "", "", false, replaceErr + } + repo = replacement + created = replacementCreated } if baseBranch == "" { baseBranch = repo.DefaultBranch diff --git a/apps/backend/internal/task/service/service_test.go b/apps/backend/internal/task/service/service_test.go index e734e0176a..b4d664d2d9 100644 --- a/apps/backend/internal/task/service/service_test.go +++ b/apps/backend/internal/task/service/service_test.go @@ -336,6 +336,490 @@ func TestService_CreateTask_RejectsRepositoryFromOtherWorkspace(t *testing.T) { } } +func TestService_CreateTask_RewritesTaskWorktreeRepositoryIDToProviderRepository(t *testing.T) { + svc, _, repo := createTestService(t) + ctx := context.Background() + + const ( + workspaceID = "ws-1" + workflowID = "wf-1" + badRepoID = "repo-task-worktree" + providerRepoID = "repo-provider" + prHeadBranch = "feature/adding-a-download-ot-5sl" + ) + + _ = repo.CreateWorkspace(ctx, &models.Workspace{ID: workspaceID, Name: "Workspace"}) + _ = repo.CreateWorkflow(ctx, &models.Workflow{ID: workflowID, WorkspaceID: workspaceID, Name: "WF"}) + _ = repo.CreateRepository(ctx, &models.Repository{ + ID: badRepoID, + WorkspaceID: workspaceID, + Name: "kdlbs/kandev task worktree", + SourceType: sourceTypeLocal, + LocalPath: "/root/.kandev/tasks/pr-1541-fix-skip-cle_3bm/kdlbs-kandev", + Provider: "github", + ProviderOwner: "kdlbs", + ProviderName: "kandev", + DefaultBranch: "main", + }) + _ = repo.CreateRepository(ctx, &models.Repository{ + ID: providerRepoID, + WorkspaceID: workspaceID, + Name: "kdlbs/kandev", + SourceType: sourceTypeProvider, + Provider: "github", + ProviderOwner: "kdlbs", + ProviderName: "kandev", + DefaultBranch: "main", + }) + + task, err := svc.CreateTask(ctx, &CreateTaskRequest{ + WorkspaceID: workspaceID, + WorkflowID: workflowID, + WorkflowStepID: "step-1", + Title: "PR review", + Repositories: []TaskRepositoryInput{ + {RepositoryID: badRepoID, BaseBranch: prHeadBranch}, + }, + }) + if err != nil { + t.Fatalf("CreateTask: %v", err) + } + if len(task.Repositories) != 1 { + t.Fatalf("expected 1 task repository, got %d", len(task.Repositories)) + } + if task.Repositories[0].RepositoryID != providerRepoID { + t.Fatalf("expected provider repository %q, got %q", providerRepoID, task.Repositories[0].RepositoryID) + } + if task.Repositories[0].BaseBranch != prHeadBranch { + t.Fatalf("expected base branch %q, got %q", prHeadBranch, task.Repositories[0].BaseBranch) + } +} + +func TestService_CreateTask_RewritesTaskWorktreeRepositoryIDToSafeLocalRepository(t *testing.T) { + svc, _, repo := createTestService(t) + ctx := context.Background() + + const ( + workspaceID = "ws-1" + workflowID = "wf-1" + badRepoID = "repo-task-worktree" + localRepoID = "repo-local-source" + providerRepoID = "repo-provider" + prHeadBranch = "feature/adding-a-download-ot-5sl" + ) + + _ = repo.CreateWorkspace(ctx, &models.Workspace{ID: workspaceID, Name: "Workspace"}) + _ = repo.CreateWorkflow(ctx, &models.Workflow{ID: workflowID, WorkspaceID: workspaceID, Name: "WF"}) + _ = repo.CreateRepository(ctx, &models.Repository{ + ID: badRepoID, + WorkspaceID: workspaceID, + Name: "kdlbs/kandev task worktree", + SourceType: sourceTypeLocal, + LocalPath: "/root/.kandev/tasks/pr-1541-fix-skip-cle_3bm/kdlbs-kandev", + Provider: "github", + ProviderOwner: "kdlbs", + ProviderName: "kandev", + DefaultBranch: "main", + }) + _ = repo.CreateRepository(ctx, &models.Repository{ + ID: localRepoID, + WorkspaceID: workspaceID, + Name: "kdlbs/kandev", + SourceType: sourceTypeLocal, + LocalPath: "/workspaces/kandev", + Provider: "github", + ProviderOwner: "kdlbs", + ProviderName: "kandev", + DefaultBranch: "main", + }) + _ = repo.CreateRepository(ctx, &models.Repository{ + ID: providerRepoID, + WorkspaceID: workspaceID, + Name: "kdlbs/kandev provider", + SourceType: sourceTypeProvider, + Provider: "github", + ProviderOwner: "kdlbs", + ProviderName: "kandev", + DefaultBranch: "main", + }) + + task, err := svc.CreateTask(ctx, &CreateTaskRequest{ + WorkspaceID: workspaceID, + WorkflowID: workflowID, + WorkflowStepID: "step-1", + Title: "PR review", + Repositories: []TaskRepositoryInput{ + {RepositoryID: badRepoID, BaseBranch: prHeadBranch}, + }, + }) + if err != nil { + t.Fatalf("CreateTask: %v", err) + } + if len(task.Repositories) != 1 { + t.Fatalf("expected 1 task repository, got %d", len(task.Repositories)) + } + if task.Repositories[0].RepositoryID != localRepoID { + t.Fatalf("expected safe local repository %q, got %q", localRepoID, task.Repositories[0].RepositoryID) + } + if task.Repositories[0].BaseBranch != prHeadBranch { + t.Fatalf("expected base branch %q, got %q", prHeadBranch, task.Repositories[0].BaseBranch) + } + repos, listErr := repo.ListRepositories(ctx, workspaceID) + if listErr != nil { + t.Fatalf("ListRepositories: %v", listErr) + } + if len(repos) != 3 { + t.Fatalf("expected no additional repository to be created, got %d repositories", len(repos)) + } +} + +func TestService_CreateTask_RewritesTaskWorktreeLocalPathToSafeLocalRepository(t *testing.T) { + svc, _, repo := createTestService(t) + ctx := context.Background() + + const ( + workspaceID = "ws-1" + workflowID = "wf-1" + badRepoID = "repo-task-worktree" + localRepoID = "repo-local-source" + taskPath = "/root/.kandev/tasks/pr-1541-fix-skip-cle_3bm/kdlbs-kandev" + prHeadBranch = "feature/adding-a-download-ot-5sl" + ) + + _ = repo.CreateWorkspace(ctx, &models.Workspace{ID: workspaceID, Name: "Workspace"}) + _ = repo.CreateWorkflow(ctx, &models.Workflow{ID: workflowID, WorkspaceID: workspaceID, Name: "WF"}) + _ = repo.CreateRepository(ctx, &models.Repository{ + ID: badRepoID, + WorkspaceID: workspaceID, + Name: "kdlbs/kandev task worktree", + SourceType: sourceTypeLocal, + LocalPath: taskPath, + Provider: "github", + ProviderOwner: "kdlbs", + ProviderName: "kandev", + DefaultBranch: "main", + }) + _ = repo.CreateRepository(ctx, &models.Repository{ + ID: localRepoID, + WorkspaceID: workspaceID, + Name: "kdlbs/kandev", + SourceType: sourceTypeLocal, + LocalPath: "/workspaces/kandev", + Provider: "github", + ProviderOwner: "kdlbs", + ProviderName: "kandev", + DefaultBranch: "main", + }) + + task, err := svc.CreateTask(ctx, &CreateTaskRequest{ + WorkspaceID: workspaceID, + WorkflowID: workflowID, + WorkflowStepID: "step-1", + Title: "PR review", + Repositories: []TaskRepositoryInput{ + {LocalPath: taskPath, BaseBranch: prHeadBranch}, + }, + }) + if err != nil { + t.Fatalf("CreateTask: %v", err) + } + if len(task.Repositories) != 1 { + t.Fatalf("expected 1 task repository, got %d", len(task.Repositories)) + } + if task.Repositories[0].RepositoryID != localRepoID { + t.Fatalf("expected safe local repository %q, got %q", localRepoID, task.Repositories[0].RepositoryID) + } + if task.Repositories[0].BaseBranch != prHeadBranch { + t.Fatalf("expected base branch %q, got %q", prHeadBranch, task.Repositories[0].BaseBranch) + } +} + +func TestService_CreateTask_RejectsUnmatchedTaskWorktreeLocalPath(t *testing.T) { + svc, _, repo := createTestService(t) + ctx := context.Background() + + const ( + workspaceID = "ws-1" + workflowID = "wf-1" + taskPath = "/root/.kandev/tasks/pr-1541-fix-skip-cle_3bm/kdlbs-kandev" + ) + + _ = repo.CreateWorkspace(ctx, &models.Workspace{ID: workspaceID, Name: "Workspace"}) + _ = repo.CreateWorkflow(ctx, &models.Workflow{ID: workflowID, WorkspaceID: workspaceID, Name: "WF"}) + + _, err := svc.CreateTask(ctx, &CreateTaskRequest{ + WorkspaceID: workspaceID, + WorkflowID: workflowID, + WorkflowStepID: "step-1", + Title: "PR review", + Repositories: []TaskRepositoryInput{ + {LocalPath: taskPath, BaseBranch: "feature/pr-head"}, + }, + }) + if err == nil { + t.Fatal("expected CreateTask to reject unmatched task-worktree local path") + } + if !strings.Contains(err.Error(), "points at a Kandev task worktree") { + t.Fatalf("expected task-worktree local path error, got %v", err) + } + repos, listErr := repo.ListRepositories(ctx, workspaceID) + if listErr != nil { + t.Fatalf("ListRepositories: %v", listErr) + } + if len(repos) != 0 { + t.Fatalf("expected no repository to be created, got %d", len(repos)) + } +} + +func TestService_CreateTask_CreatesProviderRepositoryForTaskWorktreeRepositoryID(t *testing.T) { + svc, _, repo := createTestService(t) + ctx := context.Background() + + const ( + workspaceID = "ws-1" + workflowID = "wf-1" + badRepoID = "repo-task-worktree" + ) + + _ = repo.CreateWorkspace(ctx, &models.Workspace{ID: workspaceID, Name: "Workspace"}) + _ = repo.CreateWorkflow(ctx, &models.Workflow{ID: workflowID, WorkspaceID: workspaceID, Name: "WF"}) + _ = repo.CreateRepository(ctx, &models.Repository{ + ID: badRepoID, + WorkspaceID: workspaceID, + Name: "kdlbs/kandev task worktree", + SourceType: sourceTypeLocal, + LocalPath: "/root/.kandev/tasks/pr-1541-fix-skip-cle_3bm/kdlbs-kandev", + Provider: "github", + ProviderOwner: "kdlbs", + ProviderName: "kandev", + DefaultBranch: "main", + }) + + task, err := svc.CreateTask(ctx, &CreateTaskRequest{ + WorkspaceID: workspaceID, + WorkflowID: workflowID, + WorkflowStepID: "step-1", + Title: "PR review", + Repositories: []TaskRepositoryInput{ + {RepositoryID: badRepoID, BaseBranch: "feature/pr-head"}, + }, + }) + if err != nil { + t.Fatalf("CreateTask: %v", err) + } + if len(task.Repositories) != 1 { + t.Fatalf("expected 1 task repository, got %d", len(task.Repositories)) + } + createdRepoID := task.Repositories[0].RepositoryID + if createdRepoID == badRepoID { + t.Fatal("expected task to use a provider repository, got the task worktree repository") + } + createdRepo, err := repo.GetRepository(ctx, createdRepoID) + if err != nil { + t.Fatalf("GetRepository(%q): %v", createdRepoID, err) + } + if createdRepo.SourceType != sourceTypeProvider { + t.Fatalf("expected provider source type, got %q", createdRepo.SourceType) + } + if createdRepo.LocalPath != "" { + t.Fatalf("expected provider repository without local path, got %q", createdRepo.LocalPath) + } + if createdRepo.Provider != "github" || createdRepo.ProviderOwner != "kdlbs" || createdRepo.ProviderName != "kandev" { + t.Fatalf("unexpected provider identity: %+v", createdRepo) + } +} + +func TestService_CreateTask_ErrorsForTaskWorktreeRepositoryWithoutProviderIdentity(t *testing.T) { + svc, _, repo := createTestService(t) + ctx := context.Background() + + const ( + workspaceID = "ws-1" + workflowID = "wf-1" + badRepoID = "repo-task-worktree" + ) + + svc.discoveryConfig.TaskWorktreeRoots = []string{"/data/tasks"} + + _ = repo.CreateWorkspace(ctx, &models.Workspace{ID: workspaceID, Name: "Workspace"}) + _ = repo.CreateWorkflow(ctx, &models.Workflow{ID: workflowID, WorkspaceID: workspaceID, Name: "WF"}) + _ = repo.CreateRepository(ctx, &models.Repository{ + ID: badRepoID, + WorkspaceID: workspaceID, + Name: "task worktree without provider identity", + SourceType: sourceTypeLocal, + LocalPath: "/data/tasks/pr-1541-fix-skip-cle_3bm/kdlbs-kandev", + }) + + _, err := svc.CreateTask(ctx, &CreateTaskRequest{ + WorkspaceID: workspaceID, + WorkflowID: workflowID, + WorkflowStepID: "step-1", + Title: "PR review", + Repositories: []TaskRepositoryInput{ + {RepositoryID: badRepoID, BaseBranch: "feature/pr-head"}, + }, + }) + if err == nil { + t.Fatal("expected missing provider identity error") + } + if !strings.Contains(err.Error(), "points at a Kandev task worktree") { + t.Fatalf("expected task worktree provider identity error, got %v", err) + } + repos, listErr := repo.ListRepositories(ctx, workspaceID) + if listErr != nil { + t.Fatalf("ListRepositories: %v", listErr) + } + if len(repos) != 1 { + t.Fatalf("expected no provider repository to be created, got %d repositories", len(repos)) + } +} + +func TestService_CreateTask_GitHubURLIgnoresTaskWorktreeProviderMatch(t *testing.T) { + svc, _, repo := createTestService(t) + ctx := context.Background() + + const ( + workspaceID = "ws-1" + workflowID = "wf-1" + badRepoID = "repo-task-worktree" + ) + + _ = repo.CreateWorkspace(ctx, &models.Workspace{ID: workspaceID, Name: "Workspace"}) + _ = repo.CreateWorkflow(ctx, &models.Workflow{ID: workflowID, WorkspaceID: workspaceID, Name: "WF"}) + _ = repo.CreateRepository(ctx, &models.Repository{ + ID: badRepoID, + WorkspaceID: workspaceID, + Name: "kdlbs/kandev task worktree", + SourceType: sourceTypeLocal, + LocalPath: "/root/.kandev/tasks/pr-1541-fix-skip-cle_3bm/kdlbs-kandev", + Provider: "github", + ProviderOwner: "kdlbs", + ProviderName: "kandev", + DefaultBranch: "main", + }) + + task, err := svc.CreateTask(ctx, &CreateTaskRequest{ + WorkspaceID: workspaceID, + WorkflowID: workflowID, + WorkflowStepID: "step-1", + Title: "PR review", + Repositories: []TaskRepositoryInput{ + {GitHubURL: "https://github.com/kdlbs/kandev/pull/1567"}, + }, + }) + if err != nil { + t.Fatalf("CreateTask: %v", err) + } + if len(task.Repositories) != 1 { + t.Fatalf("expected 1 task repository, got %d", len(task.Repositories)) + } + createdRepoID := task.Repositories[0].RepositoryID + if createdRepoID == badRepoID { + t.Fatal("expected GitHub URL resolution to ignore the task worktree repository") + } + createdRepo, err := repo.GetRepository(ctx, createdRepoID) + if err != nil { + t.Fatalf("GetRepository(%q): %v", createdRepoID, err) + } + if createdRepo.SourceType != sourceTypeProvider { + t.Fatalf("expected provider source type, got %q", createdRepo.SourceType) + } + if createdRepo.LocalPath != "" { + t.Fatalf("expected provider repository without local path, got %q", createdRepo.LocalPath) + } +} + +func TestService_FindOrCreateRepository_ReturnsCreatedForTaskWorktreeReplacement(t *testing.T) { + svc, _, repo := createTestService(t) + ctx := context.Background() + + const workspaceID = "ws-1" + + _ = repo.CreateWorkspace(ctx, &models.Workspace{ID: workspaceID, Name: "Workspace"}) + _ = repo.CreateRepository(ctx, &models.Repository{ + ID: "repo-task-worktree", + WorkspaceID: workspaceID, + Name: "kdlbs/kandev task worktree", + SourceType: sourceTypeLocal, + LocalPath: "/root/.kandev/tasks/pr-1541-fix-skip-cle_3bm/kdlbs-kandev", + Provider: "github", + ProviderOwner: "kdlbs", + ProviderName: "kandev", + DefaultBranch: "main", + }) + + resolved, created, err := svc.FindOrCreateRepository(ctx, &FindOrCreateRepositoryRequest{ + WorkspaceID: workspaceID, + Provider: "github", + ProviderOwner: "kdlbs", + ProviderName: "kandev", + DefaultBranch: "main", + }) + if err != nil { + t.Fatalf("FindOrCreateRepository: %v", err) + } + if !created { + t.Fatal("expected created=true for provider replacement row") + } + if resolved.SourceType != sourceTypeProvider { + t.Fatalf("expected provider source type, got %q", resolved.SourceType) + } + if resolved.LocalPath != "" { + t.Fatalf("expected provider repository without local path, got %q", resolved.LocalPath) + } +} + +func TestService_FindOrCreateRepository_ErrorsWhenTaskWorktreeReplacementDisappears(t *testing.T) { + svc, _, repo := createTestService(t) + ctx := context.Background() + + const workspaceID = "ws-1" + + _ = repo.CreateWorkspace(ctx, &models.Workspace{ID: workspaceID, Name: "Workspace"}) + _ = repo.CreateRepository(ctx, &models.Repository{ + ID: "repo-task-worktree", + WorkspaceID: workspaceID, + Name: "kdlbs/kandev task worktree", + SourceType: sourceTypeLocal, + LocalPath: "/root/.kandev/tasks/pr-1541-fix-skip-cle_3bm/kdlbs-kandev", + Provider: "github", + ProviderOwner: "kdlbs", + ProviderName: "kandev", + DefaultBranch: "main", + }) + svc.repoEntities = missingReplacementLookupRepository{ + RepositoryEntityRepository: repo, + preserveID: "repo-task-worktree", + } + + _, _, err := svc.FindOrCreateRepository(ctx, &FindOrCreateRepositoryRequest{ + WorkspaceID: workspaceID, + Provider: "github", + ProviderOwner: "kdlbs", + ProviderName: "kandev", + DefaultBranch: "main", + }) + if err == nil { + t.Fatal("expected missing replacement repository error") + } + if !strings.Contains(err.Error(), "no longer exists") { + t.Fatalf("expected missing replacement error, got %v", err) + } +} + +type missingReplacementLookupRepository struct { + repository.RepositoryEntityRepository + preserveID string +} + +func (r missingReplacementLookupRepository) GetRepository(ctx context.Context, id string) (*models.Repository, error) { + if id != r.preserveID { + return nil, nil + } + return r.RepositoryEntityRepository.GetRepository(ctx, id) +} + func TestService_CreateRepository_DefaultWorktreeBranchPrefix(t *testing.T) { svc, _, repo := createTestService(t) ctx := context.Background() diff --git a/apps/web/components/github/my-github/quick-task-launcher.test.tsx b/apps/web/components/github/my-github/quick-task-launcher.test.tsx new file mode 100644 index 0000000000..5adec632eb --- /dev/null +++ b/apps/web/components/github/my-github/quick-task-launcher.test.tsx @@ -0,0 +1,254 @@ +import { render } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import type { Icon } from "@tabler/icons-react"; +import type { GitHubIssue, GitHubPR } from "@/lib/types/github"; +import { + repositoryId, + workspaceId, + type Repository, + type Workflow, + type WorkflowStep, +} from "@/lib/types/http"; +import { QuickTaskLauncher, type LaunchPayload, type TaskPreset } from "./quick-task-launcher"; + +const NOW = "2026-07-01T00:00:00Z"; +const WORKSPACE_ID = "workspace-1"; +const WORKFLOW_ID = "workflow-1"; +const PR_HEAD_BRANCH = "feature/adding-a-download-ot-5sl"; +const TASK_WORKTREE_ROOT = "/root/.kandev/tasks"; +const TASK_WORKTREE_PATH = "/root/.kandev/tasks/pr-1541-fix-skip-cle_3bm/kdlbs-kandev"; +const REPO_URL = "https://github.com/kdlbs/kandev/pull/1567"; +const ISSUE_URL = "https://github.com/kdlbs/kandev/issues/1567"; + +const mocks = vi.hoisted(() => ({ + dialogProps: undefined as { initialValues?: Record } | undefined, + push: vi.fn(), + createTaskPR: vi.fn(), +})); + +vi.mock("@/components/task-create-dialog", () => ({ + TaskCreateDialog: (props: { initialValues?: Record }) => { + mocks.dialogProps = props; + return null; + }, +})); + +vi.mock("@/lib/routing/client-router", () => ({ + useRouter: () => ({ push: mocks.push }), +})); + +vi.mock("@/lib/api/domains/github-api", () => ({ + createTaskPR: mocks.createTaskPR, +})); + +const preset: TaskPreset = { + id: "review", + label: "Review", + hint: "Review the PR", + icon: (() => null) as unknown as Icon, + prompt: ({ url }) => `Review ${url}`, +}; + +const workflow = { + id: WORKFLOW_ID, + workspace_id: WORKSPACE_ID, + name: "Workflow", + created_at: NOW, + updated_at: NOW, +} as Workflow; + +const step = { + id: "step-1", + workflow_id: WORKFLOW_ID, + name: "Backlog", + position: 1, + color: "gray", + created_at: NOW, + updated_at: NOW, +} as WorkflowStep; + +function pr(overrides: Partial = {}): GitHubPR { + return { + number: 1567, + title: "feat: add Download option", + url: REPO_URL, + html_url: REPO_URL, + state: "open", + head_branch: PR_HEAD_BRANCH, + base_branch: "main", + author_login: "contributor", + repo_owner: "kdlbs", + repo_name: "kandev", + draft: false, + mergeable: true, + additions: 0, + deletions: 0, + requested_reviewers: [], + created_at: NOW, + updated_at: NOW, + merged_at: null, + closed_at: null, + ...overrides, + }; +} + +function issue(overrides: Partial = {}): GitHubIssue { + return { + number: 1567, + title: "add Download option", + body: "", + url: ISSUE_URL, + html_url: ISSUE_URL, + state: "open", + author_login: "contributor", + repo_owner: "kdlbs", + repo_name: "kandev", + labels: [], + assignees: [], + created_at: NOW, + updated_at: NOW, + closed_at: null, + ...overrides, + }; +} + +type RepoOverrides = Omit, "id" | "workspace_id"> & { id: string }; + +function repo(overrides: RepoOverrides): Repository { + const { id, ...rest } = overrides; + return { + id: repositoryId(id), + workspace_id: workspaceId(WORKSPACE_ID), + name: "kdlbs/kandev", + source_type: "local", + local_path: "/work/kandev", + provider: "github", + provider_repo_id: "", + provider_owner: "kdlbs", + provider_name: "kandev", + default_branch: "main", + worktree_branch_prefix: "feature/", + pull_before_worktree: true, + setup_script: "", + cleanup_script: "", + dev_script: "", + copy_files: "", + created_at: NOW, + updated_at: NOW, + ...rest, + }; +} + +function renderLauncher(repositories: Repository[], prOverrides: Partial = {}) { + const payload: LaunchPayload = { kind: "pr", pr: pr(prOverrides), preset }; + render( + , + ); + return mocks.dialogProps?.initialValues; +} + +function renderIssueLauncher( + repositories: Repository[], + issueOverrides: Partial = {}, +) { + const payload: LaunchPayload = { kind: "issue", issue: issue(issueOverrides), preset }; + render( + , + ); + return mocks.dialogProps?.initialValues; +} + +afterEach(() => { + mocks.dialogProps = undefined; + mocks.push.mockClear(); + mocks.createTaskPR.mockClear(); +}); + +describe("QuickTaskLauncher repository defaults", () => { + it("falls back to Remote mode when the only matching repo is a task worktree path", () => { + const initialValues = renderLauncher([ + repo({ id: "task-worktree", local_path: TASK_WORKTREE_PATH }), + ]); + + expect(initialValues).toMatchObject({ + githubUrl: REPO_URL, + branch: PR_HEAD_BRANCH, + checkoutBranch: PR_HEAD_BRANCH, + prNumber: 1567, + prBaseBranch: "main", + }); + expect(initialValues?.repositoryId).toBeUndefined(); + }); + + it("opens PR launches in Remote mode even when a provider-backed repo exists", () => { + const initialValues = renderLauncher([ + repo({ id: "task-worktree", local_path: TASK_WORKTREE_PATH }), + repo({ + id: "provider-repo", + source_type: "provider", + local_path: "", + }), + ]); + + expect(initialValues).toMatchObject({ + githubUrl: REPO_URL, + branch: PR_HEAD_BRANCH, + checkoutBranch: PR_HEAD_BRANCH, + prNumber: 1567, + prBaseBranch: "main", + }); + expect(initialValues?.repositoryId).toBeUndefined(); + }); + + it("still preselects an ordinary matching local GitHub repo for issues", () => { + const initialValues = renderIssueLauncher([ + repo({ id: "local-repo", local_path: "/work/kandev" }), + ]); + + expect(initialValues).toMatchObject({ + repositoryId: "local-repo", + }); + expect(initialValues?.githubUrl).toBeUndefined(); + }); + + it("rejects task worktree roots without a trailing slash", () => { + const initialValues = renderIssueLauncher([ + repo({ id: "task-worktree-root", local_path: TASK_WORKTREE_ROOT }), + ]); + + expect(initialValues).toMatchObject({ + githubUrl: "github.com/kdlbs/kandev", + }); + expect(initialValues?.repositoryId).toBeUndefined(); + }); + + it("derives a PR URL when the GitHub payload omits URL fields", () => { + const initialValues = renderLauncher( + [repo({ id: "task-worktree", local_path: TASK_WORKTREE_PATH })], + { html_url: "", url: "" }, + ); + + expect(initialValues).toMatchObject({ + githubUrl: REPO_URL, + branch: PR_HEAD_BRANCH, + checkoutBranch: PR_HEAD_BRANCH, + prNumber: 1567, + prBaseBranch: "main", + }); + expect(initialValues?.repositoryId).toBeUndefined(); + }); +}); diff --git a/apps/web/components/github/my-github/quick-task-launcher.tsx b/apps/web/components/github/my-github/quick-task-launcher.tsx index d3ffd480b6..2ef841b265 100644 --- a/apps/web/components/github/my-github/quick-task-launcher.tsx +++ b/apps/web/components/github/my-github/quick-task-launcher.tsx @@ -27,20 +27,61 @@ type DialogState = { branch?: string; checkoutBranch?: string; githubUrl?: string; + prNumber?: number; + prBaseBranch?: string; }; +const LOCAL_SOURCE_TYPE = "local"; +const REJECT_REPO_MATCH = Number.POSITIVE_INFINITY; + function matchRepo(repos: Repository[], owner: string, name: string): Repository | undefined { - return repos.find( - (r) => - (r.provider_owner || "").toLowerCase() === owner.toLowerCase() && - (r.provider_name || "").toLowerCase() === name.toLowerCase(), + let best: Repository | undefined; + let bestRank = REJECT_REPO_MATCH; + for (const repo of repos) { + if (!matchesProviderIdentity(repo, owner, name)) continue; + const rank = repositoryLaunchRank(repo); + if (rank < bestRank) { + best = repo; + bestRank = rank; + } + } + return bestRank === REJECT_REPO_MATCH ? undefined : best; +} + +function matchesProviderIdentity(repo: Repository, owner: string, name: string): boolean { + return ( + (repo.provider_owner || "").toLowerCase() === owner.toLowerCase() && + (repo.provider_name || "").toLowerCase() === name.toLowerCase() ); } +function repositoryLaunchRank(repo: Repository): number { + if (!isGitHubRepository(repo)) return REJECT_REPO_MATCH; + if (isTaskWorktreePath(repo.local_path)) return REJECT_REPO_MATCH; + if ((repo.source_type || LOCAL_SOURCE_TYPE) !== LOCAL_SOURCE_TYPE) return 0; + return 1; +} + +function isGitHubRepository(repo: Repository): boolean { + const provider = (repo.provider || "").toLowerCase(); + return provider === "" || provider === "github"; +} + +function isTaskWorktreePath(path: string | undefined): boolean { + const normalized = (path || "").replaceAll("\\", "/"); + return normalized.includes("/.kandev/tasks/") || normalized.endsWith("/.kandev/tasks"); +} + function emptyToUndefined(value: string | undefined): string | undefined { return value ? value : undefined; } +function prURL(pr: GitHubPR): string { + return ( + pr.html_url || pr.url || `https://github.com/${pr.repo_owner}/${pr.repo_name}/pull/${pr.number}` + ); +} + // Multi-repo tasks have one task_repository row per repo; pick the one that // matches the PR's owner/repo. Preference order: // 1. preferredRepositoryId (the id captured at dialog-time by repo matching) @@ -66,7 +107,7 @@ function pickRepositoryIdForPR( function extractPayload(payload: LaunchPayload) { if (payload.kind === "pr") { return { - url: payload.pr.html_url, + url: prURL(payload.pr), title: payload.pr.title, owner: payload.pr.repo_owner, name: payload.pr.repo_name, @@ -84,7 +125,6 @@ function extractPayload(payload: LaunchPayload) { function buildDialogState(payload: LaunchPayload, repositories: Repository[]): DialogState { const data = extractPayload(payload); - const repo = matchRepo(repositories, data.owner, data.name); const description = payload.preset.prompt({ url: data.url, title: data.title }); const title = `${payload.preset.label}: ${data.title}`; // For a PR launch we want the dialog to display and check out the PR's head @@ -92,6 +132,18 @@ function buildDialogState(payload: LaunchPayload, repositories: Repository[]): D // auto-resolves to the PR head. Same branch for both: the chip shows it and // the worktree checks it out. const checkoutBranch = payload.kind === "pr" ? data.branch : undefined; + if (payload.kind === "pr") { + return { + title, + description, + githubUrl: data.url, + branch: data.branch, + checkoutBranch, + prNumber: payload.pr.number, + prBaseBranch: payload.pr.base_branch, + }; + } + const repo = matchRepo(repositories, data.owner, data.name); if (repo) { return { title, @@ -164,7 +216,7 @@ export function QuickTaskLauncher({ void createTaskPR({ task_id: task.id, repository_id: repositoryId, - pr_url: payload.pr.html_url, + pr_url: prURL(payload.pr), }).catch(() => { // Silently ignore — the indicator will populate via the poller path // (legacy behavior) if branch matching succeeds. @@ -192,6 +244,8 @@ export function QuickTaskLauncher({ branch: dialog.branch, checkoutBranch: dialog.checkoutBranch, githubUrl: dialog.githubUrl, + prNumber: dialog.prNumber, + prBaseBranch: dialog.prBaseBranch, }} onSuccess={handleSuccess} /> diff --git a/apps/web/components/task-create-dialog-helpers.multi-repo.test.ts b/apps/web/components/task-create-dialog-helpers.multi-repo.test.ts index 0e65889cfd..c7592c97b1 100644 --- a/apps/web/components/task-create-dialog-helpers.multi-repo.test.ts +++ b/apps/web/components/task-create-dialog-helpers.multi-repo.test.ts @@ -250,6 +250,7 @@ describe("buildRepositoriesPayload — PR URL inference (per-row prInfoByUrl)", repository_id: "", base_branch: "main", checkout_branch: "jira-hosted-path-auth", + pr_number: 977, github_url: url, }, ]); @@ -308,6 +309,7 @@ describe("buildRepositoriesPayload — PR URL inference (per-row prInfoByUrl)", repository_id: "", base_branch: "main", checkout_branch: "feature/x", + pr_number: 10, github_url: url, }, ]); @@ -338,6 +340,56 @@ describe("buildRepositoriesPayload — checkout_branch gating on PR-auto-selecti expect(payload[0]).toMatchObject({ checkout_branch: "feature/x" }); }); + it("seeded PR metadata carries checkout_branch before PR info cache loads", () => { + const payload = buildRepositoriesPayload({ + useRemote: true, + remoteRepos: [ + { + key: "remote-0", + url, + branch: "feature/x", + source: "paste", + prNumber: 42, + prBaseBranch: "main", + prHeadBranch: "feature/x", + }, + ], + repositories: [], + discoveredRepositories: [], + }); + + expect(payload[0]).toMatchObject({ + base_branch: "main", + checkout_branch: "feature/x", + pr_number: 42, + }); + }); + + it("seeded PR metadata is dropped when the user overrides the branch", () => { + const payload = buildRepositoriesPayload({ + useRemote: true, + remoteRepos: [ + { + key: "remote-0", + url, + branch: "develop", + source: "paste", + prNumber: 42, + prBaseBranch: "main", + prHeadBranch: "feature/x", + }, + ], + repositories: [], + discoveredRepositories: [], + }); + + expect(payload[0]).toMatchObject({ + base_branch: "develop", + checkout_branch: undefined, + pr_number: undefined, + }); + }); + it("row.branch !== prHeadBranch → payload has NO checkout_branch", () => { const payload = buildRepositoriesPayload({ useRemote: true, @@ -382,12 +434,14 @@ describe("buildRepositoriesPayload — multi-row PR/repo mix", () => { repository_id: "", base_branch: "main", checkout_branch: "fork-a", + pr_number: 1, github_url: urlA, }, { repository_id: "", base_branch: "trunk", checkout_branch: "fork-b", + pr_number: 2, github_url: urlB, }, ]); @@ -417,7 +471,13 @@ describe("buildRepositoriesPayload — multi-row PR/repo mix", () => { discoveredRepositories: [], }); expect(payload).toEqual([ - { repository_id: "", base_branch: "main", checkout_branch: "fork-a", github_url: prUrl }, + { + repository_id: "", + base_branch: "main", + checkout_branch: "fork-a", + pr_number: 1, + github_url: prUrl, + }, { repository_id: "", base_branch: "develop", @@ -456,6 +516,7 @@ describe("buildRepositoriesPayload — URL trimming for prInfoByUrl lookup", () repository_id: "", base_branch: "main", checkout_branch: "feature/x", + pr_number: 42, github_url: canonical, }, ]); diff --git a/apps/web/components/task-create-dialog-helpers.ts b/apps/web/components/task-create-dialog-helpers.ts index b1576d6d7f..7f1ead9d88 100644 --- a/apps/web/components/task-create-dialog-helpers.ts +++ b/apps/web/components/task-create-dialog-helpers.ts @@ -21,6 +21,12 @@ import type { FileAttachment } from "@/components/task/chat/file-attachment"; import type { MessageAttachment } from "@/lib/services/session-launch-service"; type CreateTaskParams = Parameters[0]; +type CreateTaskRepositoryPayload = NonNullable[number]; +type RemoteRepoPRMetadata = { + headBranch?: string; + baseBranch?: string; + number?: number; +}; const selectionDebug = createDebugLogger("task-create:selection"); const BRANCH_AUTOPICK_DEBUG = "branch-autopick"; @@ -361,32 +367,64 @@ function buildRemoteRepoPayload(opts: { }): NonNullable { const nonEmpty = opts.remoteRepos.filter((r) => r.url.trim() !== ""); if (nonEmpty.length === 0) return []; - return nonEmpty.map((row) => { - const url = row.url.trim(); - // The cache is keyed on the trimmed URL (ensure() also trims), so we - // must look it up with the trimmed value too. Passing `row.url` directly - // would miss the cache when the user has stray whitespace around their - // URL and silently lose the PR base-branch anchoring. - const prInfo = opts.prInfoByUrl?.info(url); - if (prInfo) { - const isPrAutoSelection = !!prInfo.prHeadBranch && row.branch === prInfo.prHeadBranch; - const baseBranch = isPrAutoSelection - ? prInfo.prBaseBranch || undefined - : row.branch || undefined; - return { - repository_id: "", - base_branch: baseBranch, - checkout_branch: isPrAutoSelection ? prInfo.prHeadBranch || undefined : undefined, - github_url: url, - }; - } - return { - repository_id: "", - base_branch: row.branch || undefined, - checkout_branch: undefined, - github_url: url, - }; - }); + return nonEmpty.map((row) => buildRemoteRepoPayloadRow(row, opts.prInfoByUrl)); +} + +function buildRemoteRepoPayloadRow( + row: TaskRemoteRepoRow, + prInfoByUrl?: Pick, +): CreateTaskRepositoryPayload { + const url = row.url.trim(); + const metadata = remoteRepoPRMetadata(row, url, prInfoByUrl); + if (metadata) return buildRemoteRepoPRPayload(row, url, metadata); + return buildPlainRemoteRepoPayload(row, url); +} + +function remoteRepoPRMetadata( + row: TaskRemoteRepoRow, + url: string, + prInfoByUrl?: Pick, +): RemoteRepoPRMetadata | null { + // The cache is keyed on the trimmed URL (ensure() also trims), so we + // must look it up with the trimmed value too. Passing `row.url` directly + // would miss the cache when the user has stray whitespace around their + // URL and silently lose the PR base-branch anchoring. + const prInfo = prInfoByUrl?.info(url); + const number = prInfo?.prNumber ?? row.prNumber; + if (!prInfo && !number) return null; + return { + headBranch: prInfo?.prHeadBranch ?? row.prHeadBranch, + baseBranch: prInfo?.prBaseBranch ?? row.prBaseBranch, + number, + }; +} + +function buildRemoteRepoPRPayload( + row: TaskRemoteRepoRow, + url: string, + metadata: RemoteRepoPRMetadata, +): CreateTaskRepositoryPayload { + const isPrAutoSelection = !!metadata.headBranch && row.branch === metadata.headBranch; + const baseBranch = isPrAutoSelection ? metadata.baseBranch || undefined : row.branch || undefined; + return { + repository_id: "", + base_branch: baseBranch, + checkout_branch: isPrAutoSelection ? metadata.headBranch || undefined : undefined, + pr_number: isPrAutoSelection ? metadata.number || undefined : undefined, + github_url: url, + }; +} + +function buildPlainRemoteRepoPayload( + row: TaskRemoteRepoRow, + url: string, +): CreateTaskRepositoryPayload { + return { + repository_id: "", + base_branch: row.branch || undefined, + checkout_branch: undefined, + github_url: url, + }; } function resolveRowDefaultBranch( diff --git a/apps/web/components/task-create-dialog-repositories-state.ts b/apps/web/components/task-create-dialog-repositories-state.ts index 8cd2057aed..7412661047 100644 --- a/apps/web/components/task-create-dialog-repositories-state.ts +++ b/apps/web/components/task-create-dialog-repositories-state.ts @@ -72,7 +72,9 @@ export function useRemoteReposState() { }, []); const updateRemoteRepo = useCallback((key: string, patch: Partial) => { - setRemoteRepos((rows) => rows.map((r) => (r.key === key ? { ...r, ...patch } : r))); + setRemoteRepos((rows) => + rows.map((row) => (row.key === key ? applyRemoteRepoPatch(row, patch) : row)), + ); }, []); return { @@ -85,6 +87,22 @@ export function useRemoteReposState() { }; } +function applyRemoteRepoPatch( + row: TaskRemoteRepoRow, + patch: Partial, +): TaskRemoteRepoRow { + if (typeof patch.url !== "string" || patch.url.trim() === row.url.trim()) { + return { ...row, ...patch }; + } + return { + ...row, + prNumber: undefined, + prBaseBranch: undefined, + prHeadBranch: undefined, + ...patch, + }; +} + /** * Mirrors `useRepositoryAutoSelectEffect` for the remote-repo list: when the * user flips Remote mode on and the list is empty, seed a single empty paste diff --git a/apps/web/components/task-create-dialog-state.test.ts b/apps/web/components/task-create-dialog-state.test.ts index 5a0df9c2f5..f25db185df 100644 --- a/apps/web/components/task-create-dialog-state.test.ts +++ b/apps/web/components/task-create-dialog-state.test.ts @@ -189,6 +189,72 @@ describe("useDialogFormState — remoteRepos mode", () => { }); }); +describe("useDialogFormState — remote PR metadata", () => { + it("clears seeded PR metadata when a remote repo URL changes", () => { + const initialValues = { + title: "", + githubUrl: PR_URL_42, + branch: "feature/x", + checkoutBranch: "feature/x", + prNumber: 42, + prBaseBranch: "main", + }; + const { result, rerender } = renderHook( + ({ open }: { open: boolean }) => useDialogFormState(open, "ws-1", null, initialValues), + { initialProps: { open: false } }, + ); + + rerender({ open: true }); + const key = result.current.remoteRepos[0]?.key; + expect(result.current.remoteRepos[0]).toMatchObject({ + url: PR_URL_42, + branch: "feature/x", + prNumber: 42, + prBaseBranch: "main", + prHeadBranch: "feature/x", + }); + + act(() => { + result.current.updateRemoteRepo(key!, { url: "https://github.com/acme/site/pull/99" }); + }); + + expect(result.current.remoteRepos[0]).toMatchObject({ + url: "https://github.com/acme/site/pull/99", + branch: "feature/x", + }); + expect(result.current.remoteRepos[0]?.prNumber).toBeUndefined(); + expect(result.current.remoteRepos[0]?.prBaseBranch).toBeUndefined(); + expect(result.current.remoteRepos[0]?.prHeadBranch).toBeUndefined(); + }); + + it("preserves PR metadata supplied with a remote repo URL change", () => { + const { result } = renderHook(() => useDialogFormState(true, "ws-1", null)); + + act(() => { + result.current.setUseRemote(true); + }); + const key = result.current.remoteRepos[0]?.key; + + act(() => { + result.current.updateRemoteRepo(key!, { + url: PR_URL_42, + branch: "feature/x", + prNumber: 42, + prBaseBranch: "main", + prHeadBranch: "feature/x", + }); + }); + + expect(result.current.remoteRepos[0]).toMatchObject({ + url: PR_URL_42, + branch: "feature/x", + prNumber: 42, + prBaseBranch: "main", + prHeadBranch: "feature/x", + }); + }); +}); + describe("useDialogFormState — remoteRepos key allocation", () => { // Regression: the per-hook counter starts at 0 and increments locally, so // a hydrated state that already contains `remote-1` (e.g. from the seed diff --git a/apps/web/components/task-create-dialog-state.ts b/apps/web/components/task-create-dialog-state.ts index 3881e34a55..311bd363be 100644 --- a/apps/web/components/task-create-dialog-state.ts +++ b/apps/web/components/task-create-dialog-state.ts @@ -198,7 +198,15 @@ function resetDiscoveryState(resetters: FormResetters, iv?: TaskCreateDialogInit if (ghUrl) { const seededBranch = iv?.checkoutBranch ?? iv?.branch ?? ""; resetters.setRemoteRepos([ - { key: "remote-0", url: ghUrl, branch: seededBranch, source: "paste" }, + { + key: "remote-0", + url: ghUrl, + branch: seededBranch, + source: "paste", + prNumber: iv?.prNumber, + prBaseBranch: iv?.prBaseBranch, + prHeadBranch: iv?.checkoutBranch, + }, ]); } else { resetters.setRemoteRepos([]); diff --git a/apps/web/components/task-create-dialog-types.ts b/apps/web/components/task-create-dialog-types.ts index d481c2b5bb..c80061c5f0 100644 --- a/apps/web/components/task-create-dialog-types.ts +++ b/apps/web/components/task-create-dialog-types.ts @@ -48,6 +48,9 @@ export type TaskRemoteRepoRow = { url: string; // canonical https://… or paste-as-typed branch: string; source: "picker" | "paste"; + prNumber?: number; + prBaseBranch?: string; + prHeadBranch?: string; // Optional metadata when source === "picker": provider?: "github" | "gitlab"; fullName?: string; // "owner/name" @@ -74,6 +77,8 @@ export type TaskCreateDialogInitialValues = { /** When set, opens the dialog in GitHub URL mode pre-filled with this value * (e.g. "github.com/owner/repo"). Used when no matching workspace repo exists. */ githubUrl?: string; + prNumber?: number; + prBaseBranch?: string; }; export type StoreSelections = { diff --git a/apps/web/e2e/tests/github/pr-action-create-task-dialog.spec.ts b/apps/web/e2e/tests/github/pr-action-create-task-dialog.spec.ts new file mode 100644 index 0000000000..29babba07e --- /dev/null +++ b/apps/web/e2e/tests/github/pr-action-create-task-dialog.spec.ts @@ -0,0 +1,68 @@ +import path from "node:path"; +import { test, expect } from "../../fixtures/test-base"; + +test.describe("GitHub PR task launcher", () => { + test("opens the create task dialog in Remote mode when the matching repo is a stale task worktree", async ({ + testPage, + apiClient, + seedData, + backend, + }) => { + const owner = "staleorg"; + const repo = "stalerepo"; + const prNumber = 1567; + const prTitle = "Remote fallback PR"; + const prURL = `https://github.com/${owner}/${repo}/pull/${prNumber}`; + + await apiClient.mockGitHubReset(); + await apiClient.mockGitHubSetUser("test-user"); + await apiClient.mockGitHubAddPRs([ + { + number: prNumber, + title: prTitle, + state: "open", + head_branch: "feature/remote-fallback", + base_branch: "main", + author_login: "test-user", + repo_owner: owner, + repo_name: repo, + }, + ]); + + await apiClient.createRepository( + seedData.workspaceId, + path.join(backend.tmpDir, ".kandev", "tasks", "pr-1541-fix-skip-cle_3bm", "stalerepo"), + "main", + { + name: `${owner}/${repo}`, + provider: "github", + provider_owner: owner, + provider_name: repo, + }, + ); + + await testPage.goto("/github"); + await expect(testPage.getByTestId("github-presets-scope-bar")).toBeVisible({ + timeout: 15_000, + }); + await expect(testPage.getByTestId("pr-row")).toHaveCount(1, { timeout: 15_000 }); + + const row = testPage.getByTestId("pr-row").filter({ hasText: prTitle }); + await row.getByTestId("pr-start-task-trigger").click(); + await testPage.locator('[data-testid="pr-start-task-preset"][data-preset-id="review"]').click(); + + await expect(testPage.getByTestId("task-title-input")).toHaveValue(`Review: ${prTitle}`); + await expect(testPage.getByTestId("source-mode-remote")).toHaveAttribute( + "aria-checked", + "true", + ); + await expect(testPage.getByTestId("remote-repo-chip")).toHaveAttribute( + "data-remote-url", + prURL, + ); + await expect(testPage.getByTestId("remote-repo-chip-trigger")).toContainText( + `pull/${prNumber}`, + ); + await expect(testPage.getByTestId("repo-chip")).toHaveCount(0); + }); +}); diff --git a/apps/web/e2e/tests/task/create-task-disabled-tooltip.spec.ts b/apps/web/e2e/tests/task/create-task-disabled-tooltip.spec.ts index 63b71c51e4..d5149afd98 100644 --- a/apps/web/e2e/tests/task/create-task-disabled-tooltip.spec.ts +++ b/apps/web/e2e/tests/task/create-task-disabled-tooltip.spec.ts @@ -1,4 +1,5 @@ import { test, expect } from "../../fixtures/test-base"; +import type { Page } from "@playwright/test"; import { useRegularMode } from "../../helpers/regular-mode"; import { KanbanPage } from "../../pages/kanban-page"; @@ -9,6 +10,19 @@ const START_ENABLED_TIMEOUT = 30_000; // Exercises the regular task-create dialog (New Task in the sidebar); run with office off. useRegularMode(); +async function ensureSeedRepositorySelected(testPage: Page, repositoryId: string) { + const repoChipWrapper = testPage.getByTestId("repo-chip").first(); + const repoChip = testPage.getByTestId("repo-chip-trigger").first(); + if ((await repoChipWrapper.getAttribute("data-repository-id")) !== repositoryId) { + await repoChip.click(); + await testPage.locator(`[cmdk-item][data-value="${repositoryId}"]`).click(); + } + await expect(repoChipWrapper).toHaveAttribute("data-repository-id", repositoryId); + await expect(testPage.getByTestId("branch-chip-trigger").first()).toContainText("main", { + timeout: 10_000, + }); +} + test.describe("Create task button: disabled-reason tooltip", () => { test("shows 'Add a task title' when title is empty", async ({ testPage }) => { const kanban = new KanbanPage(testPage); @@ -32,13 +46,17 @@ test.describe("Create task button: disabled-reason tooltip", () => { }); }); - test("tooltip omits any disabled reason once the form is valid", async ({ testPage }) => { + test("tooltip omits any disabled reason once the form is valid", async ({ + testPage, + seedData, + }) => { const kanban = new KanbanPage(testPage); await kanban.goto(); await kanban.createTaskButton.first().click(); const dialog = testPage.getByTestId("create-task-dialog"); await expect(dialog).toBeVisible(); + await ensureSeedRepositorySelected(testPage, seedData.repositoryId); await testPage.getByTestId("task-title-input").fill("Valid Task"); await testPage.getByTestId("task-description-input").fill("doing a thing"); @@ -59,6 +77,7 @@ test.describe("Create task button: disabled-reason tooltip", () => { test("shows 'Add a task title' after clearing a previously-filled title", async ({ testPage, + seedData, }) => { const kanban = new KanbanPage(testPage); await kanban.goto(); @@ -66,6 +85,7 @@ test.describe("Create task button: disabled-reason tooltip", () => { await kanban.createTaskButton.first().click(); const dialog = testPage.getByTestId("create-task-dialog"); await expect(dialog).toBeVisible(); + await ensureSeedRepositorySelected(testPage, seedData.repositoryId); const titleInput = testPage.getByTestId("task-title-input"); await titleInput.fill("Temp Title"); diff --git a/apps/web/e2e/tests/task/create-task-from-pr-list.spec.ts b/apps/web/e2e/tests/task/create-task-from-pr-list.spec.ts index 27a25ff12a..41af7bfde1 100644 --- a/apps/web/e2e/tests/task/create-task-from-pr-list.spec.ts +++ b/apps/web/e2e/tests/task/create-task-from-pr-list.spec.ts @@ -7,7 +7,7 @@ test.describe("Create task from GitHub PR list", () => { // Cold-start backend boots can race on the first test in a worker. test.describe.configure({ retries: 1 }); - test("starting a task from a PR pre-fills the matching repo and the PR head branch", async ({ + test("starting a task from a PR opens Remote mode with the PR URL and head branch", async ({ testPage, apiClient, seedData, @@ -35,7 +35,8 @@ test.describe("Create task from GitHub PR list", () => { execSync("git checkout main", { cwd: repoDir, env: gitEnv }); // Pre-seed a GitHub-backed repository that matches the mock PR's owner/repo. - // The launcher's matchRepo() looks up workspace repos by provider_owner/name. + // PR launch must still use Remote mode so stale local task worktrees cannot + // be selected by repository matching. await apiClient.createRepository(seedData.workspaceId, repoDir, "main", { name: `${ownerSlug}/${repoSlug}`, provider: "github", @@ -75,17 +76,19 @@ test.describe("Create task from GitHub PR list", () => { await prRow.getByTestId("pr-start-task-trigger").click(); await testPage.locator('[data-testid="pr-start-task-preset"][data-preset-id="review"]').click(); - // The create-task dialog should open with the matching workspace repo - // selected and the PR head branch pre-filled in the branch chip. + const prURL = `https://github.com/${ownerSlug}/${repoSlug}/pull/77`; + + // The create-task dialog should open in Remote mode with the PR URL and + // head branch pre-filled. const dialog = testPage.getByTestId("create-task-dialog"); await expect(dialog).toBeVisible(); - await expect(dialog.getByTestId("repo-chip-trigger").first()).toContainText( - `${ownerSlug}/${repoSlug}`, - ); - await expect(dialog.getByTestId("branch-chip-trigger").first()).toContainText( + await expect(dialog.getByTestId("source-mode-remote")).toHaveAttribute("aria-checked", "true"); + await expect(dialog.getByTestId("remote-repo-chip")).toHaveAttribute("data-remote-url", prURL); + await expect(dialog.getByTestId("remote-repo-chip-trigger")).toContainText("pull/77"); + await expect(dialog.getByTestId("remote-branch-chip-trigger")).toContainText( "feature/from-pr-list", - { timeout: 10_000 }, ); + await expect(dialog.getByTestId("repo-chip")).toHaveCount(0); // The dialog title should be derived from the preset + PR title. await expect(testPage.getByTestId("task-title-input")).toHaveValue( diff --git a/apps/web/lib/api/domains/kanban-api.ts b/apps/web/lib/api/domains/kanban-api.ts index 2d867c723d..995bcab656 100644 --- a/apps/web/lib/api/domains/kanban-api.ts +++ b/apps/web/lib/api/domains/kanban-api.ts @@ -56,6 +56,7 @@ export async function createTask( repository_id: string; base_branch?: string; checkout_branch?: string; + pr_number?: number; local_path?: string; name?: string; default_branch?: string;