From 262ecdf270b5171538a68e9937dda7894ee01673 Mon Sep 17 00:00:00 2001 From: Alassane <79718216+yattdev@users.noreply.github.com> Date: Mon, 14 Sep 2026 10:00:35 -0400 Subject: [PATCH] feat(mcp): handle task change links (#3506) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Carlos Florêncio Co-authored-by: ayattara --- .github/workflows/backend-tests.yml | 5 +- apps/backend/internal/backendapp/helpers.go | 67 ++- .../internal/backendapp/helpers_test.go | 43 ++ .../task_change_link_coordinator.go | 290 ++++++++++++ .../task_change_link_coordinator_test.go | 433 ++++++++++++++++++ apps/backend/internal/events/types.go | 1 + .../gateway/websocket/task_notifications.go | 3 +- .../websocket/task_notifications_test.go | 3 +- apps/backend/internal/github/store.go | 37 +- .../github/store_task_pr_detach_test.go | 44 ++ .../backend/internal/gitlab/client_helpers.go | 13 +- .../internal/gitlab/client_helpers_test.go | 13 + apps/backend/internal/gitlab/models.go | 3 + .../gitlab/security_regression_test.go | 1 + apps/backend/internal/gitlab/service.go | 13 + .../internal/gitlab/service_task_mr_link.go | 22 +- .../gitlab/service_task_mr_link_test.go | 42 ++ apps/backend/internal/gitlab/service_test.go | 4 +- apps/backend/internal/gitlab/store.go | 57 ++- .../internal/gitlab/store_task_mr_sha_test.go | 31 ++ apps/backend/internal/gitlab/store_test.go | 28 ++ .../backend/internal/mcp/handlers/handlers.go | 14 + .../internal/mcp/handlers/task_change_link.go | 142 ++++++ .../mcp/handlers/task_change_link_test.go | 168 +++++++ .../internal/mcp/handlers/task_pr_enrich.go | 163 +++++-- .../mcp/handlers/task_pr_enrich_test.go | 135 +++++- apps/backend/internal/mcp/server/handlers.go | 33 ++ .../internal/mcp/server/handlers_test.go | 47 ++ apps/backend/internal/mcp/server/server.go | 38 ++ .../internal/mcp/server/server_test.go | 19 +- apps/backend/internal/task/dto/dto.go | 3 + .../internal/task/service/handoff_service.go | 21 +- apps/backend/pkg/api/v1/task.go | 24 +- apps/backend/pkg/websocket/actions.go | 4 + .../git/fork-pr-comparison-target.spec.ts | 5 +- apps/web/lib/types/backend.ts | 4 +- apps/web/lib/types/gitlab.ts | 7 + apps/web/lib/ws/handlers/gitlab.test.ts | 33 ++ apps/web/lib/ws/handlers/gitlab.ts | 13 + docs/plans/manage-task-pr-and-m/plan.md | 49 ++ .../task-01-coordinator.md | 65 +++ docs/public/automation-and-mcp.md | 23 + docs/public/coverage.json | 11 +- .../requirements/task-change-link-mcp.md | 79 ++++ .../system-design/task-change-link-mcp.md | 91 ++++ 45 files changed, 2259 insertions(+), 85 deletions(-) create mode 100644 apps/backend/internal/backendapp/task_change_link_coordinator.go create mode 100644 apps/backend/internal/backendapp/task_change_link_coordinator_test.go create mode 100644 apps/backend/internal/gitlab/store_task_mr_sha_test.go create mode 100644 apps/backend/internal/mcp/handlers/task_change_link.go create mode 100644 apps/backend/internal/mcp/handlers/task_change_link_test.go create mode 100644 docs/plans/manage-task-pr-and-m/plan.md create mode 100644 docs/plans/manage-task-pr-and-m/task-01-coordinator.md create mode 100644 docs/specs/integrations/requirements/task-change-link-mcp.md create mode 100644 docs/specs/integrations/system-design/task-change-link-mcp.md diff --git a/.github/workflows/backend-tests.yml b/.github/workflows/backend-tests.yml index 5158177e75..c1b9cb1bd9 100644 --- a/.github/workflows/backend-tests.yml +++ b/.github/workflows/backend-tests.yml @@ -747,7 +747,10 @@ jobs: # a self-hosting Go helper (see testmain_test.go) so they run on # every platform. - name: Test windows-sensitive packages - run: go test -race -v ./internal/agentctl/server/process/... ./internal/agent/runtime/agentctl/launcher/... ./internal/common/ptyexec/... ./internal/backendapp/ownershiplock/... ./internal/system/storage/workspaces/... + # The process package is large and the race detector is slow on + # windows-latest. Go's default package timeout is 10 minutes, which can + # terminate a healthy suite before the workflow's 30-minute job cap. + run: go test -race -v -timeout 25m ./internal/agentctl/server/process/... ./internal/agent/runtime/agentctl/launcher/... ./internal/common/ptyexec/... ./internal/backendapp/ownershiplock/... ./internal/system/storage/workspaces/... - name: Test Windows managed runtime cache repair run: go test -race -v ./internal/agent/managedruntime -run TestRemoveNpxExecutionTreeTreatsMissingWindowsChildAsAbsent diff --git a/apps/backend/internal/backendapp/helpers.go b/apps/backend/internal/backendapp/helpers.go index d7eb0dbb68..2a01142a66 100644 --- a/apps/backend/internal/backendapp/helpers.go +++ b/apps/backend/internal/backendapp/helpers.go @@ -1845,11 +1845,60 @@ func (a mcpTaskPRListerAdapter) ListTaskPRsByTaskIDs( continue } infos = append(infos, mcphandlers.TaskPRInfo{ - Number: pr.PRNumber, - URL: pr.PRURL, - Title: pr.PRTitle, - State: pr.State, - MergedAt: pr.MergedAt, + RepositoryID: pr.RepositoryID, + Number: pr.PRNumber, + URL: pr.PRURL, + Title: pr.PRTitle, + State: pr.State, + Draft: pr.IsDraft, + BaseRef: pr.BaseBranch, + HeadRef: pr.HeadBranch, + HeadSHA: pr.HeadSHA, + MergedAt: pr.MergedAt, + ClosedAt: pr.ClosedAt, + }) + } + if len(infos) > 0 { + out[taskID] = infos + } + } + return out, nil +} + +type mcpTaskMRListerAdapter struct { + gl *gitlab.Service +} + +func (a mcpTaskMRListerAdapter) ListTaskMRsByTaskIDs( + ctx context.Context, taskIDs []string, +) (map[string][]mcphandlers.TaskMRInfo, error) { + out := make(map[string][]mcphandlers.TaskMRInfo) + if a.gl == nil || len(taskIDs) == 0 { + return out, nil + } + byTask, err := a.gl.ListTaskMRsByTaskIDs(ctx, taskIDs) + if err != nil { + return nil, err + } + for taskID, mrs := range byTask { + infos := make([]mcphandlers.TaskMRInfo, 0, len(mrs)) + for _, mr := range mrs { + if mr == nil { + continue + } + infos = append(infos, mcphandlers.TaskMRInfo{ + RepositoryID: mr.RepositoryID, + Number: mr.MRIID, + URL: mr.MRURL, + Title: mr.MRTitle, + State: mr.State, + Draft: mr.Draft, + BaseRef: mr.BaseBranch, + BaseSHA: mr.BaseSHA, + HeadRef: mr.HeadBranch, + HeadSHA: mr.HeadSHA, + MergedAt: mr.MergedAt, + ClosedAt: mr.ClosedAt, }) } if len(infos) > 0 { @@ -1936,8 +1985,8 @@ func registerMCPAndDebugRoutes( mcpHandlers.SetDiagnosticBundleServices(p.systemSvc.LogBundles, p.lifecycleMgr) } - // Enrich list_tasks responses with associated GitHub PRs (link, title, - // number, state) when the github service is available. + // Enrich task-listing responses with associated change requests when + // provider services are available. if p.services.GitHub != nil { mcpHandlers.SetTaskPRLister(mcpTaskPRListerAdapter{gh: p.services.GitHub}) mcpHandlers.SetTaskPRAutomationService(p.services.GitHub) @@ -1946,8 +1995,12 @@ func registerMCPAndDebugRoutes( mcpHandlers.SetTaskPRAutoFixOutcomeService(p.orchestratorSvc) } if p.services.GitLab != nil { + mcpHandlers.SetTaskMRLister(mcpTaskMRListerAdapter{gl: p.services.GitLab}) mcpHandlers.SetTaskMRAutomationService(p.services.GitLab) } + mcpHandlers.SetTaskChangeLinkService(taskChangeLinkCoordinator{ + tasks: p.taskSvc, github: p.services.GitHub, gitlab: p.services.GitLab, + }) // Reuse the cross-task handoff service constructed in registerRoutes — // the same instance backs the MCP path and the HTTP Kanban path so // workspace-group state stays consistent across both surfaces. diff --git a/apps/backend/internal/backendapp/helpers_test.go b/apps/backend/internal/backendapp/helpers_test.go index c7b92ecfe6..a9d5943101 100644 --- a/apps/backend/internal/backendapp/helpers_test.go +++ b/apps/backend/internal/backendapp/helpers_test.go @@ -22,6 +22,7 @@ import ( "github.com/kandev/kandev/internal/db" "github.com/kandev/kandev/internal/events/bus" gateways "github.com/kandev/kandev/internal/gateway/websocket" + "github.com/kandev/kandev/internal/github" "github.com/kandev/kandev/internal/quickterminal" quickterminalrepo "github.com/kandev/kandev/internal/quickterminal/repository" systemsvc "github.com/kandev/kandev/internal/system" @@ -153,6 +154,48 @@ func TestBuildGitStatusNotificationIncludesAncestryEvidence(t *testing.T) { } } +func TestMCPTaskPRListerAdapterPreservesGitHubChangeFacts(t *testing.T) { + ctx := context.Background() + store := newStatusSummaryTestStore(t) + draft := true + pr := &github.TaskPR{ + TaskID: "task-change-facts", + RepositoryID: "repo-change-facts", + PRNumber: 42, + PRURL: "https://github.com/acme/api/pull/42", + PRTitle: "Preserve exact facts", + State: "open", + BaseBranch: "main", + HeadBranch: "feature/facts", + HeadSHA: "head-sha-42", + IsDraft: &draft, + CreatedAt: time.Now().UTC(), + } + if err := store.CreateTaskPR(ctx, pr); err != nil { + t.Fatalf("CreateTaskPR: %v", err) + } + + adapter := mcpTaskPRListerAdapter{gh: github.NewService(nil, "", nil, store, nil, nil)} + byTask, err := adapter.ListTaskPRsByTaskIDs(ctx, []string{pr.TaskID}) + if err != nil { + t.Fatalf("ListTaskPRsByTaskIDs: %v", err) + } + infos := byTask[pr.TaskID] + if len(infos) != 1 { + t.Fatalf("infos = %#v, want one association", infos) + } + got := infos[0] + if got.RepositoryID != pr.RepositoryID || got.Number != pr.PRNumber || got.URL != pr.PRURL || got.Title != pr.PRTitle || got.State != pr.State || got.BaseRef != pr.BaseBranch || got.HeadRef != pr.HeadBranch { + t.Fatalf("adapter facts = %#v", got) + } + if got.Draft == nil || !*got.Draft { + t.Fatalf("Draft = %v, want true", got.Draft) + } + if got.HeadSHA != pr.HeadSHA { + t.Fatalf("HeadSHA = %q, want %q", got.HeadSHA, pr.HeadSHA) + } +} + // TestAppendSessionStateMessage_IncludesTaskEnvironmentID asserts the snapshot // the WS hub sends on `session.subscribe` carries `task_environment_id`. // diff --git a/apps/backend/internal/backendapp/task_change_link_coordinator.go b/apps/backend/internal/backendapp/task_change_link_coordinator.go new file mode 100644 index 0000000000..a194221f28 --- /dev/null +++ b/apps/backend/internal/backendapp/task_change_link_coordinator.go @@ -0,0 +1,290 @@ +package backendapp + +import ( + "context" + "errors" + "fmt" + "net/url" + "strings" + + "github.com/kandev/kandev/internal/auth/authn" + "github.com/kandev/kandev/internal/github" + "github.com/kandev/kandev/internal/gitlab" + mcp "github.com/kandev/kandev/internal/mcp/handlers" + "github.com/kandev/kandev/internal/task/models" + taskservice "github.com/kandev/kandev/internal/task/service" +) + +// taskChangeLinkCoordinator is the provider-neutral MCP seam. It keeps task +// reach checks and repository identity resolution at the host boundary, then +// delegates persistence and provider reads to the established services. +type taskChangeLinkCoordinator struct { + tasks *taskservice.Service + github githubChangeLinkProvider + gitlab gitlabChangeLinkProvider +} + +const ( + taskChangeProviderGitHub = "github" + taskChangeProviderGitLab = "gitlab" +) + +type githubChangeLinkProvider interface { + AssociateExistingPRByURLForWorkspace(context.Context, string, string, string, string, string) (*github.TaskPR, error) + DetachTaskPR(context.Context, string, string) (*github.TaskPR, error) + ListTaskPRs(context.Context, []string) (map[string][]*github.TaskPR, error) +} + +type gitlabChangeLinkProvider interface { + AssociateExistingMRByURL(context.Context, string, string, string, string) (*gitlab.TaskMR, error) + ListTaskMRsByTask(context.Context, string) ([]*gitlab.TaskMR, error) + UnlinkTaskMR(context.Context, string, string) error +} + +func (c taskChangeLinkCoordinator) LinkTaskChange(ctx context.Context, req mcp.TaskChangeLinkRequest) ([]mcp.TaskChangeLink, error) { + if err := c.link(ctx, req.TaskID, req.Link); err != nil { + return nil, err + } + return c.list(ctx, req.TaskID) +} + +func (c taskChangeLinkCoordinator) UnlinkTaskChange(ctx context.Context, req mcp.TaskChangeLinkRequest) ([]mcp.TaskChangeLink, error) { + if err := c.unlink(ctx, req.TaskID, req.Link); err != nil { + return nil, err + } + return c.list(ctx, req.TaskID) +} + +func (c taskChangeLinkCoordinator) ReplaceTaskChange(ctx context.Context, req mcp.TaskChangeLinkRequest) ([]mcp.TaskChangeLink, error) { + if req.Old == nil { + return nil, fmt.Errorf("current task change identity is required") + } + // Replacing an association with the exact same identity is already the + // requested final state. Do not detach it after the idempotent link. + if req.Link == *req.Old { + return c.list(ctx, req.TaskID) + } + if req.Link.Provider != req.Old.Provider { + return nil, fmt.Errorf("cross-provider task change replacement is not supported") + } + before, err := c.list(ctx, req.TaskID) + if err != nil { + return nil, err + } + newAlreadyLinked := taskChangeLinksContain(before, req.Link) + // Resolve and persist the incoming association first. A failed provider + // fetch therefore leaves the current association untouched. + if err := c.link(ctx, req.TaskID, req.Link); err != nil { + return nil, err + } + if err := c.unlink(ctx, req.TaskID, *req.Old); err != nil { + if newAlreadyLinked { + return nil, err + } + return c.rollbackReplacement(ctx, req.TaskID, req.Link, err) + } + return c.list(ctx, req.TaskID) +} + +func (c taskChangeLinkCoordinator) rollbackReplacement( + ctx context.Context, taskID string, newLink mcp.TaskChangeLink, originalErr error, +) ([]mcp.TaskChangeLink, error) { + rollbackErr := c.unlink(ctx, taskID, newLink) + if rollbackErr == nil { + return nil, originalErr + } + activeLinks, stateErr := c.list(ctx, taskID) + state := fmt.Errorf("active task change links after failed replacement: %v", activeLinks) + if stateErr != nil { + state = fmt.Errorf("active task change links unavailable after failed replacement: %w", stateErr) + } + return activeLinks, errors.Join(originalErr, fmt.Errorf("rollback new task change link: %w", rollbackErr), state) +} + +func taskChangeLinksContain(links []mcp.TaskChangeLink, target mcp.TaskChangeLink) bool { + for _, link := range links { + if link.Provider == target.Provider && link.RepositoryID == target.RepositoryID && link.Number == target.Number { + return true + } + } + return false +} + +func (c taskChangeLinkCoordinator) link(ctx context.Context, taskID string, link mcp.TaskChangeLink) error { + task, repo, err := c.taskRepository(ctx, taskID, link.RepositoryID) + if err != nil { + return err + } + switch link.Provider { + case taskChangeProviderGitHub: + if c.github == nil { + return fmt.Errorf("GitHub PR links are not available") + } + url, err := githubChangeURL(repo.ProviderHost, repo.ProviderOwner, repo.ProviderName, link.Number) + if err != nil { + return err + } + identity, ok := authn.IdentityFromContext(ctx) + if !ok || strings.TrimSpace(identity.UserID) == "" { + return fmt.Errorf("authenticated user identity is required for GitHub PR links") + } + _, err = c.github.AssociateExistingPRByURLForWorkspace(ctx, task.WorkspaceID, identity.UserID, task.ID, repo.ID, url) + return err + case taskChangeProviderGitLab: + if c.gitlab == nil { + return fmt.Errorf("GitLab MR links are not available") + } + url, err := gitlabChangeURL(repo.ProviderHost, repo.ProviderOwner, repo.ProviderName, link.Number) + if err != nil { + return err + } + _, err = c.gitlab.AssociateExistingMRByURL(ctx, task.WorkspaceID, task.ID, repo.ID, url) + return err + default: + return fmt.Errorf("unsupported provider %q", link.Provider) + } +} + +func (c taskChangeLinkCoordinator) unlink(ctx context.Context, taskID string, link mcp.TaskChangeLink) error { + task, _, err := c.taskRepository(ctx, taskID, "") + if err != nil { + return err + } + switch link.Provider { + case taskChangeProviderGitHub: + if c.github == nil { + return fmt.Errorf("GitHub PR links are not available") + } + prs, err := c.githubLinks(ctx, task.ID) + if err != nil { + return err + } + for _, pr := range prs { + if pr.RepositoryID == link.RepositoryID && pr.PRNumber == link.Number { + _, err = c.github.DetachTaskPR(ctx, task.WorkspaceID, pr.ID) + return err + } + } + return nil + case taskChangeProviderGitLab: + if c.gitlab == nil { + return fmt.Errorf("GitLab MR links are not available") + } + mrs, err := c.gitlab.ListTaskMRsByTask(ctx, task.ID) + if err != nil { + return err + } + for _, mr := range mrs { + if mr.RepositoryID == link.RepositoryID && mr.MRIID == link.Number { + return c.gitlab.UnlinkTaskMR(ctx, task.WorkspaceID, mr.ID) + } + } + return nil + default: + return fmt.Errorf("unsupported provider %q", link.Provider) + } +} + +func (c taskChangeLinkCoordinator) list(ctx context.Context, taskID string) ([]mcp.TaskChangeLink, error) { + if _, _, err := c.taskRepository(ctx, taskID, ""); err != nil { + return nil, err + } + links := []mcp.TaskChangeLink{} + if c.github != nil { + prs, err := c.githubLinks(ctx, taskID) + if err != nil { + return nil, err + } + for _, pr := range prs { + links = append(links, mcp.TaskChangeLink{Provider: taskChangeProviderGitHub, RepositoryID: pr.RepositoryID, Number: pr.PRNumber}) + } + } + if c.gitlab != nil { + mrs, err := c.gitlab.ListTaskMRsByTask(ctx, taskID) + if err != nil { + return nil, err + } + for _, mr := range mrs { + links = append(links, mcp.TaskChangeLink{Provider: taskChangeProviderGitLab, RepositoryID: mr.RepositoryID, Number: mr.MRIID}) + } + } + return links, nil +} + +func (c taskChangeLinkCoordinator) githubLinks(ctx context.Context, taskID string) ([]*github.TaskPR, error) { + byTask, err := c.github.ListTaskPRs(ctx, []string{taskID}) + if err != nil { + return nil, err + } + return byTask[taskID], nil +} + +func (c taskChangeLinkCoordinator) taskRepository(ctx context.Context, taskID, repositoryID string) (*models.Task, *models.Repository, error) { + if c.tasks == nil { + return nil, nil, fmt.Errorf("task service is not available") + } + task, err := c.tasks.GetTask(ctx, taskID) + if err != nil || task == nil { + return nil, nil, fmt.Errorf("task not found") + } + if repositoryID == "" { + return task, nil, nil + } + for _, taskRepo := range task.Repositories { + if taskRepo.RepositoryID != repositoryID { + continue + } + repo, getErr := c.tasks.GetRepository(ctx, repositoryID) + if getErr != nil || repo == nil || repo.WorkspaceID != task.WorkspaceID { + return nil, nil, fmt.Errorf("repository not found for task") + } + return task, repo, nil + } + return nil, nil, fmt.Errorf("repository not found for task") +} + +func githubChangeURL(host, owner, name string, number int) (string, error) { + normalizedHost, err := normalizedGitHubHost(host) + if err != nil { + return "", err + } + owner = strings.TrimSpace(owner) + name = strings.TrimSpace(name) + if owner == "" || name == "" { + return "", fmt.Errorf("repository has no canonical GitHub identity") + } + return fmt.Sprintf("https://%s/%s/%s/pull/%d", normalizedHost, owner, name, number), nil +} + +func normalizedGitHubHost(host string) (string, error) { + rawHost := strings.TrimSpace(host) + if rawHost == "" { + return "", fmt.Errorf("repository has no canonical GitHub host") + } + if !strings.Contains(rawHost, "://") { + rawHost = "https://" + rawHost + } + parsed, err := url.Parse(rawHost) + if err != nil || parsed.User != nil || parsed.Host == "" || parsed.Path != "" && parsed.Path != "/" || parsed.RawQuery != "" || parsed.Fragment != "" { + return "", fmt.Errorf("repository has invalid GitHub host") + } + if parsed.Scheme != "http" && parsed.Scheme != "https" || parsed.Port() != "" { + return "", fmt.Errorf("repository has invalid GitHub host") + } + normalizedHost := strings.ToLower(strings.TrimSuffix(parsed.Hostname(), ".")) + if normalizedHost != "github.com" && !strings.HasSuffix(normalizedHost, ".github.com") { + return "", fmt.Errorf("repository is not a GitHub repository") + } + return normalizedHost, nil +} + +func gitlabChangeURL(host, owner, name string, number int) (string, error) { + host = strings.TrimRight(strings.TrimSpace(host), "/") + if host == "" || strings.TrimSpace(owner) == "" || strings.TrimSpace(name) == "" { + return "", fmt.Errorf("repository has no canonical GitLab identity") + } + if !strings.HasPrefix(host, "https://") && !strings.HasPrefix(host, "http://") { + return "", fmt.Errorf("repository has invalid GitLab host") + } + return fmt.Sprintf("%s/%s/%s/-/merge_requests/%d", host, owner, name, number), nil +} diff --git a/apps/backend/internal/backendapp/task_change_link_coordinator_test.go b/apps/backend/internal/backendapp/task_change_link_coordinator_test.go new file mode 100644 index 0000000000..e045d4c653 --- /dev/null +++ b/apps/backend/internal/backendapp/task_change_link_coordinator_test.go @@ -0,0 +1,433 @@ +package backendapp + +import ( + "context" + "errors" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/jmoiron/sqlx" + + "github.com/kandev/kandev/internal/auth/authn" + "github.com/kandev/kandev/internal/db" + "github.com/kandev/kandev/internal/events/bus" + "github.com/kandev/kandev/internal/github" + "github.com/kandev/kandev/internal/gitlab" + mcphandlers "github.com/kandev/kandev/internal/mcp/handlers" + "github.com/kandev/kandev/internal/task/models" + "github.com/kandev/kandev/internal/task/repository" + sqliterepo "github.com/kandev/kandev/internal/task/repository/sqlite" + taskservice "github.com/kandev/kandev/internal/task/service" + "github.com/kandev/kandev/internal/worktree" + v1 "github.com/kandev/kandev/pkg/api/v1" +) + +type fakeGitLabChangeLinks struct { + mrs []*gitlab.TaskMR + linkedURL string + unlinkErr map[string]error + unlinked []string +} + +type fakeGitHubChangeLinks struct { + prs []*github.TaskPR + linkedURL string + linkedWorkspaceID string + linkedUserID string + unlinked []string +} + +func (f *fakeGitHubChangeLinks) AssociateExistingPRByURLForWorkspace(_ context.Context, workspaceID, userID, taskID, repositoryID, prURL string) (*github.TaskPR, error) { + f.linkedURL = prURL + f.linkedWorkspaceID = workspaceID + f.linkedUserID = userID + pr := &github.TaskPR{ID: "linked-github", TaskID: taskID, RepositoryID: repositoryID, PRNumber: 42, PRURL: prURL} + f.prs = append(f.prs, pr) + return pr, nil +} + +func (f *fakeGitHubChangeLinks) DetachTaskPR(_ context.Context, _ string, associationID string) (*github.TaskPR, error) { + f.unlinked = append(f.unlinked, associationID) + return nil, nil +} + +func (f *fakeGitHubChangeLinks) ListTaskPRs(_ context.Context, taskIDs []string) (map[string][]*github.TaskPR, error) { + out := make(map[string][]*github.TaskPR) + for _, taskID := range taskIDs { + for _, pr := range f.prs { + if pr.TaskID == taskID { + out[taskID] = append(out[taskID], pr) + } + } + } + return out, nil +} + +func (f *fakeGitLabChangeLinks) AssociateExistingMRByURL( + _ context.Context, _ string, taskID string, repositoryID string, mrURL string, +) (*gitlab.TaskMR, error) { + f.linkedURL = mrURL + for _, mr := range f.mrs { + if mr.TaskID == taskID && mr.RepositoryID == repositoryID && mr.MRURL == mrURL { + return mr, nil + } + } + mr := &gitlab.TaskMR{ + ID: "linked-new", TaskID: taskID, RepositoryID: repositoryID, + Host: "https://gitlab.example.test", ProjectPath: "group/project", MRIID: 42, + MRURL: mrURL, State: "open", + } + f.mrs = append(f.mrs, mr) + return mr, nil +} + +func (f *fakeGitLabChangeLinks) ListTaskMRsByTask(_ context.Context, taskID string) ([]*gitlab.TaskMR, error) { + out := make([]*gitlab.TaskMR, 0, len(f.mrs)) + for _, mr := range f.mrs { + if mr.TaskID == taskID { + out = append(out, mr) + } + } + return out, nil +} + +func (f *fakeGitLabChangeLinks) UnlinkTaskMR(_ context.Context, _ string, associationID string) error { + f.unlinked = append(f.unlinked, associationID) + if err := f.unlinkErr[associationID]; err != nil { + return err + } + filtered := f.mrs[:0] + for _, mr := range f.mrs { + if mr.ID != associationID { + filtered = append(filtered, mr) + } + } + f.mrs = filtered + return nil +} + +func newTaskChangeCoordinatorHarness(t *testing.T) (*taskservice.Service, *sqliterepo.Repository) { + t.Helper() + dbConn, err := db.OpenSQLite(filepath.Join(t.TempDir(), "task-change-links.db")) + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + database := sqlx.NewDb(dbConn, "sqlite3") + t.Cleanup(func() { _ = database.Close() }) + repos, cleanup, err := repository.Provide(database, database, nil) + if err != nil { + t.Fatalf("task repository: %v", err) + } + t.Cleanup(func() { _ = cleanup() }) + if _, err := worktree.NewSQLiteStore(database, database); err != nil { + t.Fatalf("worktree store: %v", err) + } + log := newTestLogger() + taskSvc := taskservice.NewService(taskservice.Repos{ + Workspaces: repos, + Tasks: repos, + TaskRepos: repos, + Workflows: repos, + Messages: repos, + Turns: repos, + Sessions: repos, + GitSnapshots: repos, + RepoEntities: repos, + Executors: repos, + Environments: repos, + Reviews: repos, + }, bus.NewMemoryEventBus(log), log, taskservice.RepositoryDiscoveryConfig{}) + return taskSvc, repos +} + +func seedTaskChangeCoordinatorTask( + t *testing.T, repos *sqliterepo.Repository, workspaceID, taskID, repositoryID, host, owner, name string, +) { + t.Helper() + ctx := context.Background() + now := time.Now().UTC() + if err := repos.CreateWorkspace(ctx, &models.Workspace{ID: workspaceID, Name: workspaceID, CreatedAt: now, UpdatedAt: now}); err != nil { + t.Fatalf("create workspace: %v", err) + } + if err := repos.CreateRepository(ctx, &models.Repository{ + ID: repositoryID, WorkspaceID: workspaceID, Name: name, ProviderHost: host, + ProviderOwner: owner, ProviderName: name, CreatedAt: now, UpdatedAt: now, + }); err != nil { + t.Fatalf("create repository: %v", err) + } + if err := repos.CreateTask(ctx, &models.Task{ID: taskID, WorkspaceID: workspaceID, Title: taskID, State: v1.TaskStateTODO, CreatedAt: now, UpdatedAt: now}); err != nil { + t.Fatalf("create task: %v", err) + } + if err := repos.CreateTaskRepository(ctx, &models.TaskRepository{ + ID: "tr-" + taskID + "-" + repositoryID, TaskID: taskID, RepositoryID: repositoryID, + CreatedAt: now, UpdatedAt: now, Metadata: map[string]interface{}{}, + }); err != nil { + t.Fatalf("create task repository: %v", err) + } +} + +func TestTaskChangeURLsRequireCanonicalRepositoryIdentity(t *testing.T) { + githubURL, err := githubChangeURL("https://github.com", "acme", "api", 7) + if err != nil || githubURL != "https://github.com/acme/api/pull/7" { + t.Fatalf("githubChangeURL() = %q, %v", githubURL, err) + } + gitlabURL, err := gitlabChangeURL("https://gitlab.example.test/", "group", "api", 9) + if err != nil || gitlabURL != "https://gitlab.example.test/group/api/-/merge_requests/9" { + t.Fatalf("gitlabChangeURL() = %q, %v", gitlabURL, err) + } + if _, err := githubChangeURL("https://gitlab.example.test", "acme", "api", 7); err == nil { + t.Fatal("githubChangeURL accepted a non-GitHub host") + } + if _, err := gitlabChangeURL("", "group", "api", 9); err == nil { + t.Fatal("gitlabChangeURL accepted an incomplete repository identity") + } +} + +func TestGitHubChangeURLRejectsSubstringHostSpoofing(t *testing.T) { + for _, host := range []string{ + "https://github.com.evil.test", + "https://mygithub.com", + "github.com.evil.test", + } { + if _, err := githubChangeURL(host, "acme", "api", 7); err == nil { + t.Fatalf("githubChangeURL accepted spoofed host %q", host) + } + } + if _, err := githubChangeURL("", "acme", "api", 7); err == nil { + t.Fatal("githubChangeURL accepted an unknown empty host") + } + url, err := githubChangeURL("https://enterprise.github.com", "acme", "api", 7) + if err != nil || url != "https://enterprise.github.com/acme/api/pull/7" { + t.Fatalf("githubChangeURL alternate origin = %q, %v", url, err) + } +} + +func TestTaskChangeCoordinatorDispatchesGitHubLinkWithWorkspaceIdentity(t *testing.T) { + taskSvc, repos := newTaskChangeCoordinatorHarness(t) + seedTaskChangeCoordinatorTask(t, repos, "ws-1", "task-1", "repo-1", "https://github.com", "acme", "api") + githubLinks := &fakeGitHubChangeLinks{} + coordinator := taskChangeLinkCoordinator{tasks: taskSvc, github: githubLinks} + ctx := authn.WithIdentity(context.Background(), authn.Identity{UserID: "user-1"}) + + links, err := coordinator.LinkTaskChange(ctx, mcphandlers.TaskChangeLinkRequest{ + TaskID: "task-1", + Link: mcphandlers.TaskChangeLink{Provider: "github", RepositoryID: "repo-1", Number: 42}, + }) + if err != nil { + t.Fatalf("LinkTaskChange: %v", err) + } + if githubLinks.linkedWorkspaceID != "ws-1" || githubLinks.linkedUserID != "user-1" { + t.Fatalf("GitHub identity = workspace %q user %q", githubLinks.linkedWorkspaceID, githubLinks.linkedUserID) + } + if githubLinks.linkedURL != "https://github.com/acme/api/pull/42" { + t.Fatalf("linked URL = %q", githubLinks.linkedURL) + } + if len(links) != 1 || links[0].Provider != "github" { + t.Fatalf("links = %#v, want one GitHub link", links) + } +} + +func TestTaskChangeCoordinatorDispatchesGitLabLinkAndReturnsLinkedSet(t *testing.T) { + taskSvc, repos := newTaskChangeCoordinatorHarness(t) + seedTaskChangeCoordinatorTask(t, repos, "ws-1", "task-1", "repo-1", "https://gitlab.example.test", "group", "project") + gitlabLinks := &fakeGitLabChangeLinks{} + coordinator := taskChangeLinkCoordinator{tasks: taskSvc, gitlab: gitlabLinks} + + links, err := coordinator.LinkTaskChange(context.Background(), mcphandlers.TaskChangeLinkRequest{ + TaskID: "task-1", + Link: mcphandlers.TaskChangeLink{Provider: "gitlab", RepositoryID: "repo-1", Number: 42}, + }) + if err != nil { + t.Fatalf("LinkTaskChange: %v", err) + } + if gitlabLinks.linkedURL != "https://gitlab.example.test/group/project/-/merge_requests/42" { + t.Fatalf("linked URL = %q", gitlabLinks.linkedURL) + } + if len(links) != 1 || links[0].Provider != "gitlab" || links[0].RepositoryID != "repo-1" || links[0].Number != 42 { + t.Fatalf("links = %#v, want gitlab repo-1 !42", links) + } +} + +func TestTaskChangeCoordinatorUnlinkAbsentIsIdempotent(t *testing.T) { + taskSvc, repos := newTaskChangeCoordinatorHarness(t) + seedTaskChangeCoordinatorTask(t, repos, "ws-1", "task-1", "repo-1", "https://gitlab.example.test", "group", "project") + gitlabLinks := &fakeGitLabChangeLinks{} + coordinator := taskChangeLinkCoordinator{tasks: taskSvc, gitlab: gitlabLinks} + + links, err := coordinator.UnlinkTaskChange(context.Background(), mcphandlers.TaskChangeLinkRequest{ + TaskID: "task-1", + Link: mcphandlers.TaskChangeLink{Provider: "gitlab", RepositoryID: "repo-1", Number: 42}, + }) + if err != nil { + t.Fatalf("UnlinkTaskChange absent: %v", err) + } + if len(links) != 0 { + t.Fatalf("links after absent unlink = %#v, want none", links) + } +} + +func TestTaskChangeCoordinatorReplaceSameIdentityIsANoOp(t *testing.T) { + taskSvc, repos := newTaskChangeCoordinatorHarness(t) + seedTaskChangeCoordinatorTask(t, repos, "ws-1", "task-1", "repo-1", "https://gitlab.example.test", "group", "project") + links := &fakeGitLabChangeLinks{mrs: []*gitlab.TaskMR{{ID: "current", TaskID: "task-1", RepositoryID: "repo-1", MRIID: 42}}} + coordinator := taskChangeLinkCoordinator{tasks: taskSvc, gitlab: links} + identity := mcphandlers.TaskChangeLink{Provider: "gitlab", RepositoryID: "repo-1", Number: 42} + + got, err := coordinator.ReplaceTaskChange(context.Background(), mcphandlers.TaskChangeLinkRequest{TaskID: "task-1", Link: identity, Old: &identity}) + if err != nil || len(got) != 1 || got[0] != identity { + t.Fatalf("ReplaceTaskChange() = %#v, %v", got, err) + } + if len(links.unlinked) != 0 || links.linkedURL != "" { + t.Fatalf("same-identity replace mutated provider: unlinked=%#v linked=%q", links.unlinked, links.linkedURL) + } +} + +func TestTaskChangeCoordinatorRejectsCrossProviderReplaceWithoutMutation(t *testing.T) { + taskSvc, repos := newTaskChangeCoordinatorHarness(t) + seedTaskChangeCoordinatorTask(t, repos, "ws-1", "task-1", "repo-1", "https://gitlab.example.test", "group", "project") + now := time.Now().UTC() + if err := repos.CreateRepository(context.Background(), &models.Repository{ + ID: "repo-gh", WorkspaceID: "ws-1", Name: "api", ProviderHost: "https://github.com", ProviderOwner: "acme", ProviderName: "api", CreatedAt: now, UpdatedAt: now, + }); err != nil { + t.Fatalf("create GitHub repository: %v", err) + } + if err := repos.CreateTaskRepository(context.Background(), &models.TaskRepository{ + ID: "tr-task-1-repo-gh", TaskID: "task-1", RepositoryID: "repo-gh", CreatedAt: now, UpdatedAt: now, Metadata: map[string]interface{}{}, + }); err != nil { + t.Fatalf("attach GitHub repository: %v", err) + } + gitlabLinks := &fakeGitLabChangeLinks{mrs: []*gitlab.TaskMR{{ + ID: "old", TaskID: "task-1", RepositoryID: "repo-1", MRIID: 7, + MRURL: "https://gitlab.example.test/group/project/-/merge_requests/7", + }}} + githubLinks := &fakeGitHubChangeLinks{} + coordinator := taskChangeLinkCoordinator{tasks: taskSvc, github: githubLinks, gitlab: gitlabLinks} + + _, err := coordinator.ReplaceTaskChange(context.Background(), mcphandlers.TaskChangeLinkRequest{ + TaskID: "task-1", + Link: mcphandlers.TaskChangeLink{Provider: "github", RepositoryID: "repo-gh", Number: 42}, + Old: &mcphandlers.TaskChangeLink{Provider: "gitlab", RepositoryID: "repo-1", Number: 7}, + }) + if err == nil { + t.Fatal("ReplaceTaskChange accepted a cross-provider replacement") + } + if len(gitlabLinks.mrs) != 1 || gitlabLinks.mrs[0].ID != "old" || githubLinks.linkedURL != "" || len(gitlabLinks.unlinked) != 0 { + t.Fatalf("cross-provider replace mutated links: mrs=%#v github=%q unlinked=%#v", gitlabLinks.mrs, githubLinks.linkedURL, gitlabLinks.unlinked) + } +} + +func TestTaskChangeCoordinatorReplaceRollsBackNewLinkWhenOldUnlinkFails(t *testing.T) { + taskSvc, repos := newTaskChangeCoordinatorHarness(t) + seedTaskChangeCoordinatorTask(t, repos, "ws-1", "task-1", "repo-1", "https://gitlab.example.test", "group", "project") + gitlabLinks := &fakeGitLabChangeLinks{ + mrs: []*gitlab.TaskMR{{ + ID: "old", TaskID: "task-1", RepositoryID: "repo-1", MRIID: 7, + MRURL: "https://gitlab.example.test/group/project/-/merge_requests/7", + }}, + unlinkErr: map[string]error{"old": errors.New("delete failed")}, + } + coordinator := taskChangeLinkCoordinator{tasks: taskSvc, gitlab: gitlabLinks} + + _, err := coordinator.ReplaceTaskChange(context.Background(), mcphandlers.TaskChangeLinkRequest{ + TaskID: "task-1", + Link: mcphandlers.TaskChangeLink{Provider: "gitlab", RepositoryID: "repo-1", Number: 42}, + Old: &mcphandlers.TaskChangeLink{Provider: "gitlab", RepositoryID: "repo-1", Number: 7}, + }) + if err == nil { + t.Fatal("ReplaceTaskChange succeeded despite old unlink failure") + } + if len(gitlabLinks.mrs) != 1 || gitlabLinks.mrs[0].ID != "old" { + t.Fatalf("MRs after failed replace = %#v, want only old association", gitlabLinks.mrs) + } + if len(gitlabLinks.unlinked) != 2 || gitlabLinks.unlinked[0] != "old" || gitlabLinks.unlinked[1] != "linked-new" { + t.Fatalf("unlink order = %#v, want old then rollback of new", gitlabLinks.unlinked) + } +} + +func TestTaskChangeCoordinatorReportsRollbackFailure(t *testing.T) { + taskSvc, repos := newTaskChangeCoordinatorHarness(t) + seedTaskChangeCoordinatorTask(t, repos, "ws-1", "task-1", "repo-1", "https://gitlab.example.test", "group", "project") + gitlabLinks := &fakeGitLabChangeLinks{ + mrs: []*gitlab.TaskMR{{ + ID: "old", TaskID: "task-1", RepositoryID: "repo-1", MRIID: 7, + MRURL: "https://gitlab.example.test/group/project/-/merge_requests/7", + }}, + unlinkErr: map[string]error{ + "old": errors.New("delete failed"), + "linked-new": errors.New("rollback failed"), + }, + } + coordinator := taskChangeLinkCoordinator{tasks: taskSvc, gitlab: gitlabLinks} + + _, err := coordinator.ReplaceTaskChange(context.Background(), mcphandlers.TaskChangeLinkRequest{ + TaskID: "task-1", + Link: mcphandlers.TaskChangeLink{Provider: "gitlab", RepositoryID: "repo-1", Number: 42}, + Old: &mcphandlers.TaskChangeLink{Provider: "gitlab", RepositoryID: "repo-1", Number: 7}, + }) + if err == nil || !strings.Contains(err.Error(), "delete failed") || !strings.Contains(err.Error(), "rollback failed") || + !strings.Contains(err.Error(), "active task change links") || !strings.Contains(err.Error(), "42") { + t.Fatalf("ReplaceTaskChange error = %v, want both failures and active links", err) + } +} + +func TestTaskChangeCoordinatorUnlinksStaleAssociationWithoutCurrentRepository(t *testing.T) { + taskSvc, repos := newTaskChangeCoordinatorHarness(t) + seedTaskChangeCoordinatorTask(t, repos, "ws-1", "task-1", "repo-1", "https://gitlab.example.test", "group", "project") + requireErr := repos.DeleteTaskRepository(context.Background(), "tr-task-1-repo-1") + if requireErr != nil { + t.Fatalf("remove current task repository: %v", requireErr) + } + gitlabLinks := &fakeGitLabChangeLinks{mrs: []*gitlab.TaskMR{{ + ID: "stale", TaskID: "task-1", RepositoryID: "repo-1", MRIID: 42, + }}} + coordinator := taskChangeLinkCoordinator{tasks: taskSvc, gitlab: gitlabLinks} + + links, err := coordinator.UnlinkTaskChange(context.Background(), mcphandlers.TaskChangeLinkRequest{ + TaskID: "task-1", + Link: mcphandlers.TaskChangeLink{Provider: "gitlab", RepositoryID: "repo-1", Number: 42}, + }) + if err != nil { + t.Fatalf("UnlinkTaskChange stale association: %v", err) + } + if len(links) != 0 || len(gitlabLinks.unlinked) != 1 || gitlabLinks.unlinked[0] != "stale" { + t.Fatalf("stale unlink result links=%#v unlinked=%#v", links, gitlabLinks.unlinked) + } +} + +func TestTaskChangeCoordinatorRejectsTaskRepositoryFromAnotherWorkspace(t *testing.T) { + taskSvc, repos := newTaskChangeCoordinatorHarness(t) + ctx := context.Background() + now := time.Now().UTC() + if err := repos.CreateWorkspace(ctx, &models.Workspace{ID: "ws-task", Name: "Task", CreatedAt: now, UpdatedAt: now}); err != nil { + t.Fatalf("create task workspace: %v", err) + } + if err := repos.CreateWorkspace(ctx, &models.Workspace{ID: "ws-repo", Name: "Repo", CreatedAt: now, UpdatedAt: now}); err != nil { + t.Fatalf("create repo workspace: %v", err) + } + if err := repos.CreateRepository(ctx, &models.Repository{ + ID: "repo-cross", WorkspaceID: "ws-repo", Name: "project", + ProviderHost: "https://gitlab.example.test", ProviderOwner: "group", ProviderName: "project", + CreatedAt: now, UpdatedAt: now, + }); err != nil { + t.Fatalf("create repository: %v", err) + } + if err := repos.CreateTask(ctx, &models.Task{ID: "task-1", WorkspaceID: "ws-task", Title: "Task", State: v1.TaskStateTODO, CreatedAt: now, UpdatedAt: now}); err != nil { + t.Fatalf("create task: %v", err) + } + if err := repos.CreateTaskRepository(ctx, &models.TaskRepository{ + ID: "tr-cross", TaskID: "task-1", RepositoryID: "repo-cross", CreatedAt: now, UpdatedAt: now, + }); err != nil { + t.Fatalf("create task repository: %v", err) + } + coordinator := taskChangeLinkCoordinator{tasks: taskSvc, gitlab: &fakeGitLabChangeLinks{}} + + _, err := coordinator.LinkTaskChange(ctx, mcphandlers.TaskChangeLinkRequest{ + TaskID: "task-1", + Link: mcphandlers.TaskChangeLink{Provider: "gitlab", RepositoryID: "repo-cross", Number: 1}, + }) + if err == nil { + t.Fatal("LinkTaskChange accepted a repository from another workspace") + } +} diff --git a/apps/backend/internal/events/types.go b/apps/backend/internal/events/types.go index 1684c26cb9..8a9414c228 100644 --- a/apps/backend/internal/events/types.go +++ b/apps/backend/internal/events/types.go @@ -354,6 +354,7 @@ const ( GitLabNewReviewMR = "gitlab.new_mr_to_review" // New MR found needing review GitLabNewIssue = "gitlab.new_issue" // New issue found matching issue watch GitLabTaskMRUpdated = "gitlab.task_mr.updated" // TaskMR record updated (for UI refresh) + GitLabTaskMRDeleted = "gitlab.task_mr.deleted" // TaskMR association detached (for UI refresh) GitLabWatchEvent = "gitlab.watch.event" // Watch created/deleted // GitLabTaskMROptionsUpdated fires after a task's MR lifecycle diff --git a/apps/backend/internal/gateway/websocket/task_notifications.go b/apps/backend/internal/gateway/websocket/task_notifications.go index b098fa0835..e6ce2c1dab 100644 --- a/apps/backend/internal/gateway/websocket/task_notifications.go +++ b/apps/backend/internal/gateway/websocket/task_notifications.go @@ -102,6 +102,7 @@ func RegisterTaskNotifications(ctx context.Context, eventBus bus.EventBus, hub * b.subscribe(eventBus, events.GitHubRateLimitUpdated, ws.ActionGitHubRateLimitUpdated) b.subscribe(eventBus, events.GitHubPRDiscoveryHealthUpdated, ws.ActionGitHubPRDiscoveryHealthUpdated) b.subscribe(eventBus, events.GitLabTaskMRUpdated, ws.ActionGitLabTaskMRUpdated) + b.subscribe(eventBus, events.GitLabTaskMRDeleted, ws.ActionGitLabTaskMRDeleted) b.subscribe(eventBus, events.GitLabTaskMROptionsUpdated, ws.ActionGitLabTaskMRAutomationUpdated) go func() { @@ -314,7 +315,7 @@ func (b *TaskEventBroadcaster) routeBroadcast( return nil case ws.ActionGitHubTaskPRUpdated, ws.ActionGitHubTaskPRDeleted, ws.ActionGitHubTaskCIOptionsUpdated, ws.ActionGitHubPRDiscoveryHealthUpdated, - ws.ActionGitLabTaskMRUpdated, ws.ActionGitLabTaskMRAutomationUpdated: + ws.ActionGitLabTaskMRUpdated, ws.ActionGitLabTaskMRDeleted, ws.ActionGitLabTaskMRAutomationUpdated: // These payloads carry per-task PR/MR automation and lifecycle state. Fail closed // (drop, don't fall back to a global broadcast) when workspace // resolution came back empty and auth is enforced — an unattributed diff --git a/apps/backend/internal/gateway/websocket/task_notifications_test.go b/apps/backend/internal/gateway/websocket/task_notifications_test.go index 0d05ca76ce..9dbe65c050 100644 --- a/apps/backend/internal/gateway/websocket/task_notifications_test.go +++ b/apps/backend/internal/gateway/websocket/task_notifications_test.go @@ -112,7 +112,7 @@ func TestTaskEventBroadcaster_NoDuplicateSubscriptions(t *testing.T) { // // Update this number when adding or removing event subscriptions in // RegisterTaskNotifications — it is intentionally exact. - const wantSubscriptions = 74 + const wantSubscriptions = 75 if got := len(b.subscriptions); got != wantSubscriptions { t.Errorf("RegisterTaskNotifications created %d subscriptions, want %d — "+ "did an event get subscribed twice?", got, wantSubscriptions) @@ -130,6 +130,7 @@ func TestTaskEventBroadcaster_NoDuplicateSubscriptions(t *testing.T) { events.GitHubTaskPRUpdated, events.GitHubTaskPRDeleted, events.GitLabTaskMRUpdated, + events.GitLabTaskMRDeleted, } { subject := subject t.Run(subject, func(t *testing.T) { diff --git a/apps/backend/internal/github/store.go b/apps/backend/internal/github/store.go index f6ed672e77..4806277697 100644 --- a/apps/backend/internal/github/store.go +++ b/apps/backend/internal/github/store.go @@ -2260,7 +2260,25 @@ func (s *Store) ListTaskPRsByTaskIncludingDetached(ctx context.Context, taskID s // The bool return reports whether this call performed the transition. func (s *Store) DetachTaskPR(ctx context.Context, associationID string) (*TaskPR, bool, error) { now := time.Now().UTC() - result, err := s.db.ExecContext(ctx, s.db.Rebind( + tx, err := s.db.BeginTxx(ctx, nil) + if err != nil { + return nil, false, err + } + defer func() { _ = tx.Rollback() }() + var identity struct { + TaskID string `db:"task_id"` + RepositoryID string `db:"repository_id"` + PRNumber int `db:"pr_number"` + } + if err := tx.GetContext(ctx, &identity, tx.Rebind(`SELECT task_id, repository_id, pr_number FROM github_task_prs WHERE id = ? AND detached_at IS NULL`), associationID); err != nil { + if errors.Is(err, sql.ErrNoRows) { + _ = tx.Rollback() + tp, getErr := s.GetTaskPRByID(ctx, associationID) + return tp, false, getErr + } + return nil, false, err + } + result, err := tx.ExecContext(ctx, tx.Rebind( `UPDATE github_task_prs SET detached_at = ?, updated_at = ? WHERE id = ? AND detached_at IS NULL`), now, now, associationID) if err != nil { @@ -2270,8 +2288,21 @@ func (s *Store) DetachTaskPR(ctx context.Context, associationID string) (*TaskPR if err != nil { return nil, false, err } - tp, err := s.GetTaskPRByID(ctx, associationID) - return tp, count > 0, err + if count > 0 { + for _, table := range []string{"github_task_pr_automation_options", "github_task_ci_pr_state"} { + if _, err := tx.ExecContext(ctx, tx.Rebind(`DELETE FROM `+table+` WHERE task_id = ? AND repository_id = ? AND pr_number = ?`), identity.TaskID, identity.RepositoryID, identity.PRNumber); err != nil { + return nil, false, err + } + } + } + var tp TaskPR + if err := tx.GetContext(ctx, &tp, tx.Rebind(`SELECT `+taskPRColumns+` FROM github_task_prs WHERE id = ? LIMIT 1`), associationID); err != nil { + return nil, false, err + } + if err := tx.Commit(); err != nil { + return nil, false, err + } + return &tp, count > 0, nil } // RestoreTaskPR clears a detached tombstone for an explicit link action and diff --git a/apps/backend/internal/github/store_task_pr_detach_test.go b/apps/backend/internal/github/store_task_pr_detach_test.go index 13dd6a5c42..0d390e2ee1 100644 --- a/apps/backend/internal/github/store_task_pr_detach_test.go +++ b/apps/backend/internal/github/store_task_pr_detach_test.go @@ -80,6 +80,50 @@ func TestTaskPRDetachFiltersActiveRowsAndPersistsTombstone(t *testing.T) { } } +func TestTaskPRDetachRetiresOnlyExactAutomationAndCIState(t *testing.T) { + store := newTestStore(t) + ctx := context.Background() + now := time.Now().UTC() + if _, err := store.db.Exec(`INSERT INTO tasks (id, workspace_id) VALUES ('task-1', 'ws-1')`); err != nil { + t.Fatalf("seed task: %v", err) + } + var firstID string + for _, number := range []int{1, 2} { + pr := &TaskPR{ + WorkspaceID: "ws-1", TaskID: "task-1", RepositoryID: "repo-1", Owner: "acme", Repo: "demo", + PRNumber: number, PRURL: "https://github.com/acme/demo/pull/1", PRTitle: "PR", HeadBranch: "feature", BaseBranch: "main", State: "open", CreatedAt: now, + } + if err := store.CreateTaskPR(ctx, pr); err != nil { + t.Fatalf("create PR %d: %v", number, err) + } + if number == 1 { + firstID = pr.ID + } + if _, err := store.db.ExecContext(ctx, `INSERT INTO github_task_pr_automation_options (task_id, repository_id, pr_number, created_at, updated_at) VALUES (?, ?, ?, ?, ?)`, "task-1", "repo-1", number, now, now); err != nil { + t.Fatalf("seed automation options for PR %d: %v", number, err) + } + if _, err := store.db.ExecContext(ctx, `INSERT INTO github_task_ci_pr_state (task_id, repository_id, pr_number, created_at, updated_at) VALUES (?, ?, ?, ?, ?)`, "task-1", "repo-1", number, now, now); err != nil { + t.Fatalf("seed CI state for PR %d: %v", number, err) + } + } + + if _, changed, err := store.DetachTaskPR(ctx, firstID); err != nil || !changed { + t.Fatalf("detach first PR: changed=%v err=%v", changed, err) + } + for _, table := range []string{"github_task_pr_automation_options", "github_task_ci_pr_state"} { + var removed, retained int + if err := store.db.GetContext(ctx, &removed, `SELECT COUNT(*) FROM `+table+` WHERE task_id = ? AND repository_id = ? AND pr_number = ?`, "task-1", "repo-1", 1); err != nil { + t.Fatalf("count removed %s rows: %v", table, err) + } + if err := store.db.GetContext(ctx, &retained, `SELECT COUNT(*) FROM `+table+` WHERE task_id = ? AND repository_id = ? AND pr_number = ?`, "task-1", "repo-1", 2); err != nil { + t.Fatalf("count retained %s rows: %v", table, err) + } + if removed != 0 || retained != 1 { + t.Errorf("%s rows after detach: removed=%d retained=%d, want 0 and 1", table, removed, retained) + } + } +} + func TestLegacyTaskPRRebuildPreservesDetachedAt(t *testing.T) { store := newTestStore(t) ctx := context.Background() diff --git a/apps/backend/internal/gitlab/client_helpers.go b/apps/backend/internal/gitlab/client_helpers.go index d6577d110f..caa811bea0 100644 --- a/apps/backend/internal/gitlab/client_helpers.go +++ b/apps/backend/internal/gitlab/client_helpers.go @@ -27,7 +27,11 @@ type rawMR struct { SourceBranch string `json:"source_branch"` TargetBranch string `json:"target_branch"` SHA string `json:"sha"` - References struct { + DiffRefs struct { + BaseSHA string `json:"base_sha"` + HeadSHA string `json:"head_sha"` + } `json:"diff_refs"` + References struct { Full string `json:"full"` } `json:"references"` Author rawUser `json:"author"` @@ -149,6 +153,10 @@ func convertRawMR(raw *rawMR) *MR { if targetProjectID == 0 { targetProjectID = raw.ProjectID } + headSHA := raw.SHA + if raw.DiffRefs.HeadSHA != "" { + headSHA = raw.DiffRefs.HeadSHA + } mr := &MR{ ID: raw.ID, IID: raw.IID, @@ -158,8 +166,9 @@ func convertRawMR(raw *rawMR) *MR { WebURL: raw.WebURL, State: state, HeadBranch: raw.SourceBranch, - HeadSHA: raw.SHA, + HeadSHA: headSHA, BaseBranch: raw.TargetBranch, + BaseSHA: raw.DiffRefs.BaseSHA, AuthorUsername: raw.Author.Username, ProjectNamespace: namespace, ProjectPath: projectPath, diff --git a/apps/backend/internal/gitlab/client_helpers_test.go b/apps/backend/internal/gitlab/client_helpers_test.go index 0bb0819713..b8b16c4519 100644 --- a/apps/backend/internal/gitlab/client_helpers_test.go +++ b/apps/backend/internal/gitlab/client_helpers_test.go @@ -28,6 +28,19 @@ func TestConvertRawMRHydratesLabels(t *testing.T) { } } +func TestConvertRawMRHydratesCommitSHAs(t *testing.T) { + raw := &rawMR{SHA: "legacy-head-sha"} + raw.DiffRefs.BaseSHA = "base-sha" + raw.DiffRefs.HeadSHA = "head-sha" + mr := convertRawMR(raw) + if got, want := mr.HeadSHA, "head-sha"; got != want { + t.Fatalf("HeadSHA = %q, want %q", got, want) + } + if got, want := mr.BaseSHA, "base-sha"; got != want { + t.Fatalf("BaseSHA = %q, want %q", got, want) + } +} + func TestConvertRawMR_DetailedMergeStatus(t *testing.T) { // Present (GitLab 15.6+). mr := convertRawMR(&rawMR{DetailedMergeStatus: "mergeable", BlockingDiscussionsResolved: true}) diff --git a/apps/backend/internal/gitlab/models.go b/apps/backend/internal/gitlab/models.go index c809cef110..57907f27d4 100644 --- a/apps/backend/internal/gitlab/models.go +++ b/apps/backend/internal/gitlab/models.go @@ -53,6 +53,7 @@ type MR struct { HeadBranch string `json:"head_branch"` HeadSHA string `json:"head_sha"` BaseBranch string `json:"base_branch"` + BaseSHA string `json:"base_sha"` AuthorUsername string `json:"author_username"` ProjectNamespace string `json:"project_namespace"` ProjectPath string `json:"project_path"` @@ -380,7 +381,9 @@ type TaskMR struct { MRURL string `json:"mr_url" db:"mr_url"` MRTitle string `json:"mr_title" db:"mr_title"` HeadBranch string `json:"head_branch" db:"head_branch"` + HeadSHA string `json:"head_sha" db:"head_sha"` BaseBranch string `json:"base_branch" db:"base_branch"` + BaseSHA string `json:"base_sha" db:"base_sha"` AuthorUsername string `json:"author_username" db:"author_username"` State string `json:"state" db:"state"` // open, closed, merged, locked ApprovalState string `json:"approval_state" db:"approval_state"` // approved, pending, "" diff --git a/apps/backend/internal/gitlab/security_regression_test.go b/apps/backend/internal/gitlab/security_regression_test.go index a319cc1540..d783caffc2 100644 --- a/apps/backend/internal/gitlab/security_regression_test.go +++ b/apps/backend/internal/gitlab/security_regression_test.go @@ -62,6 +62,7 @@ func TestWebSocketCatalogContainsOnlyAuthorizedGitLabActions(t *testing.T) { authorized := map[string]bool{ "ActionGitLabCheckSessionMR": true, "ActionGitLabTaskMRUpdated": true, + "ActionGitLabTaskMRDeleted": true, "ActionGitLabTaskMRAutomationUpdated": true, } _, sourceFile, _, ok := runtime.Caller(0) diff --git a/apps/backend/internal/gitlab/service.go b/apps/backend/internal/gitlab/service.go index 55acef6f66..e5791bbdc7 100644 --- a/apps/backend/internal/gitlab/service.go +++ b/apps/backend/internal/gitlab/service.go @@ -696,7 +696,9 @@ func (s *Service) syncTaskMRWithClient( MRURL: mr.WebURL, MRTitle: mr.Title, HeadBranch: mr.HeadBranch, + HeadSHA: mr.HeadSHA, BaseBranch: mr.BaseBranch, + BaseSHA: mr.BaseSHA, AuthorUsername: mr.AuthorUsername, State: mr.State, ApprovalState: status.ApprovalState, @@ -748,3 +750,14 @@ func (s *Service) ListTaskMRsByTask(ctx context.Context, taskID string) ([]*Task } return store.ListTaskMRsByTask(ctx, taskID) } + +// ListTaskMRsByTaskIDs surfaces GitLab MR associations grouped by task ID. +func (s *Service) ListTaskMRsByTaskIDs(ctx context.Context, taskIDs []string) (map[string][]*TaskMR, error) { + s.mu.RLock() + store := s.store + s.mu.RUnlock() + if store == nil { + return map[string][]*TaskMR{}, nil + } + return store.ListTaskMRsByTaskIDs(ctx, taskIDs) +} diff --git a/apps/backend/internal/gitlab/service_task_mr_link.go b/apps/backend/internal/gitlab/service_task_mr_link.go index f2663d6534..c4509b861c 100644 --- a/apps/backend/internal/gitlab/service_task_mr_link.go +++ b/apps/backend/internal/gitlab/service_task_mr_link.go @@ -315,6 +315,14 @@ func (s *Service) UnlinkTaskMR(ctx context.Context, workspaceID, associationID s if err := store.DeleteTaskMRForWorkspace(ctx, workspaceID, associationID); err != nil { return err } + if s.eventBus != nil && association != nil { + event := bus.NewEvent(events.GitLabTaskMRDeleted, eventSource, &TaskMRDeletedEvent{ + WorkspaceID: workspaceID, TaskID: association.TaskID, AssociationID: association.ID, + }) + if err := s.eventBus.Publish(ctx, events.GitLabTaskMRDeleted, event); err != nil { + s.logger.Debug("failed to publish GitLab task MR deletion event", zap.Error(err)) + } + } return nil } @@ -324,7 +332,7 @@ func taskMRFromStatus(taskID, repositoryID, host, projectPath string, status *MR return &TaskMR{ TaskID: taskID, RepositoryID: repositoryID, Host: host, ProjectPath: projectPath, MRIID: mr.IID, MRURL: mr.WebURL, MRTitle: mr.Title, - HeadBranch: mr.HeadBranch, BaseBranch: mr.BaseBranch, AuthorUsername: mr.AuthorUsername, + HeadBranch: mr.HeadBranch, HeadSHA: mr.HeadSHA, BaseBranch: mr.BaseBranch, BaseSHA: mr.BaseSHA, AuthorUsername: mr.AuthorUsername, State: mr.State, ApprovalState: status.ApprovalState, PipelineState: status.PipelineState, MergeStatus: status.MergeStatus, Draft: mr.Draft, ApprovalCount: status.ApprovalCount, RequiredApprovals: status.RequiredApprovals, PipelineJobsTotal: status.PipelineJobsTotal, @@ -363,6 +371,18 @@ func (e *TaskMRUpdatedEvent) GetWorkspaceID() string { return e.WorkspaceID } +// TaskMRDeletedEvent identifies one task-MR association removed from active +// task surfaces. The upstream merge request remains unchanged. +type TaskMRDeletedEvent struct { + WorkspaceID string `json:"workspace_id"` + TaskID string `json:"task_id"` + AssociationID string `json:"association_id"` +} + +// GetWorkspaceID lets the websocket broadcaster route the deletion to the +// owning workspace. +func (e TaskMRDeletedEvent) GetWorkspaceID() string { return e.WorkspaceID } + // publishTaskMRLifecycleSyncEvent publishes a TaskMRUpdatedEvent after the // poller's lifecycle sync pass refreshes a linked MR (AC22). Unlike // publishTaskMRUpdated (an HTTP/link-flow caller that already has a trusted diff --git a/apps/backend/internal/gitlab/service_task_mr_link_test.go b/apps/backend/internal/gitlab/service_task_mr_link_test.go index b14d39f6a1..81028cddf4 100644 --- a/apps/backend/internal/gitlab/service_task_mr_link_test.go +++ b/apps/backend/internal/gitlab/service_task_mr_link_test.go @@ -8,6 +8,9 @@ import ( "strings" "testing" "time" + + "github.com/kandev/kandev/internal/events" + "github.com/kandev/kandev/internal/events/bus" ) func seedTaskMRLinkFixture(t *testing.T, store *Store, workspaceID, taskID, repositoryID string) { @@ -214,6 +217,45 @@ func TestAssociateExistingMRByURLCreatesIdempotentWorkspaceScopedLink(t *testing } } +func TestUnlinkTaskMRPublishesWorkspaceScopedDeletedEvent(t *testing.T) { + const host = "https://gitlab.acme.test" + svc, store, _ := newTaskMRLinkService(t, host) + seedTaskMRLinkFixture(t, store, "ws-1", "task-1", "repo-1") + association := &TaskMR{ + ID: "mr-1", TaskID: "task-1", RepositoryID: "repo-1", Host: host, + ProjectPath: "group/project", MRIID: 17, + MRURL: host + "/group/project/-/merge_requests/17", + } + if err := store.UpsertTaskMR(context.Background(), association); err != nil { + t.Fatalf("seed task MR: %v", err) + } + eventBus := bus.NewMemoryEventBus(newTestLogger(t)) + svc.SetEventBus(eventBus) + received := make(chan *bus.Event, 1) + if _, err := eventBus.Subscribe(events.GitLabTaskMRDeleted, func(_ context.Context, event *bus.Event) error { + received <- event + return nil + }); err != nil { + t.Fatalf("subscribe: %v", err) + } + + if err := svc.UnlinkTaskMR(context.Background(), "ws-1", association.ID); err != nil { + t.Fatalf("UnlinkTaskMR: %v", err) + } + select { + case event := <-received: + payload, ok := event.Data.(*TaskMRDeletedEvent) + if !ok { + t.Fatalf("deleted event payload = %T, want *TaskMRDeletedEvent", event.Data) + } + if payload.WorkspaceID != "ws-1" || payload.TaskID != "task-1" || payload.AssociationID != association.ID { + t.Fatalf("deleted event payload = %+v", payload) + } + default: + t.Fatal("expected gitlab.task_mr.deleted event") + } +} + // TestAssociateExistingMRByURLForSessionEnsuresWatch pins the gap the // gateway wiring used to have: the Create-MR action and manual URL linking // triggered from a session both have a concrete sessionID available, and diff --git a/apps/backend/internal/gitlab/service_test.go b/apps/backend/internal/gitlab/service_test.go index 397cd1a6d6..1ca0609947 100644 --- a/apps/backend/internal/gitlab/service_test.go +++ b/apps/backend/internal/gitlab/service_test.go @@ -407,6 +407,7 @@ func TestService_SyncTaskMR_UpsertsFromMockClient(t *testing.T) { State: mrStateOpen, WebURL: "https://gitlab.example/acme/api/-/merge_requests/42", HeadSHA: "abc123", + BaseSHA: "base456", CreatedAt: time.Now().UTC(), }) @@ -414,7 +415,8 @@ func TestService_SyncTaskMR_UpsertsFromMockClient(t *testing.T) { if err != nil { t.Fatalf("SyncTaskMR err = %v", err) } - if row.MRTitle != "feat: thing" || row.HeadBranch != "feat/x" || row.BaseBranch != "main" { + if row.MRTitle != "feat: thing" || row.HeadBranch != "feat/x" || row.HeadSHA != "abc123" || + row.BaseBranch != "main" || row.BaseSHA != "base456" { t.Errorf("unexpected upserted row: %+v", row) } if row.LastSyncedAt == nil { diff --git a/apps/backend/internal/gitlab/store.go b/apps/backend/internal/gitlab/store.go index 8b0668d32f..c6177dc3e1 100644 --- a/apps/backend/internal/gitlab/store.go +++ b/apps/backend/internal/gitlab/store.go @@ -80,7 +80,9 @@ const createTablesSQL = ` mr_url TEXT NOT NULL, mr_title TEXT NOT NULL, head_branch TEXT NOT NULL, + head_sha TEXT NOT NULL DEFAULT '', base_branch TEXT NOT NULL, + base_sha TEXT NOT NULL DEFAULT '', author_username TEXT NOT NULL DEFAULT '', state TEXT NOT NULL DEFAULT 'open', approval_state TEXT NOT NULL DEFAULT '', @@ -240,6 +242,9 @@ func (s *Store) createTables() error { if err := s.migrateTaskMRAutomationFields(); err != nil { return err } + if err := s.migrateTaskMRSHAColumns(); err != nil { + return err + } if err := s.migrateMRWatchUniqueKey(); err != nil { return err } @@ -475,6 +480,23 @@ func (s *Store) migrateConfigRevision() error { return nil } +func (s *Store) migrateTaskMRSHAColumns() error { + columns, err := s.tableColumns("gitlab_task_mrs") + if err != nil { + return err + } + for _, column := range []string{"head_sha", "base_sha"} { + if _, ok := columns[column]; ok { + continue + } + statement := fmt.Sprintf("ALTER TABLE gitlab_task_mrs ADD COLUMN %s TEXT NOT NULL DEFAULT ''", column) + if _, err := s.db.Exec(gitlabSchemaSQLForDriver(statement, s.db.DriverName())); err != nil { + return fmt.Errorf("migrate gitlab_task_mrs.%s: %w", column, err) + } + } + return nil +} + func (s *Store) migrateWatchColumns() error { columns := []struct { name string @@ -585,7 +607,7 @@ func (s *Store) GetMentionScope(ctx context.Context, workspaceID string) (*Menti // still have dropped columns (older revisions defined `unresolved_threads` // and `discussion_count` here) don't break sqlx struct binding. const taskMRSelectCols = `id, task_id, repository_id, host, project_path, mr_iid, - mr_url, mr_title, head_branch, base_branch, author_username, state, + mr_url, mr_title, head_branch, head_sha, base_branch, base_sha, author_username, state, approval_state, pipeline_state, merge_status, draft, approval_count, required_approvals, pipeline_jobs_total, pipeline_jobs_pass, detailed_merge_status, reviewer_count, unapproved_reviewers, unresolved_discussions, @@ -596,7 +618,7 @@ const taskMRSelectCols = `id, task_id, repository_id, host, project_path, mr_iid // updated_at). const taskMRSelectColsQualified = `gtm.id, gtm.task_id, gtm.repository_id, gtm.host, gtm.project_path, gtm.mr_iid, gtm.mr_url, gtm.mr_title, - gtm.head_branch, gtm.base_branch, gtm.author_username, gtm.state, + gtm.head_branch, gtm.head_sha, gtm.base_branch, gtm.base_sha, gtm.author_username, gtm.state, gtm.approval_state, gtm.pipeline_state, gtm.merge_status, gtm.draft, gtm.approval_count, gtm.required_approvals, gtm.pipeline_jobs_total, gtm.pipeline_jobs_pass, gtm.detailed_merge_status, gtm.reviewer_count, @@ -619,14 +641,14 @@ func (s *Store) UpsertTaskMR(ctx context.Context, tm *TaskMR) error { rows, err := s.db.NamedQueryContext(ctx, ` INSERT INTO gitlab_task_mrs ( id, task_id, repository_id, host, project_path, mr_iid, mr_url, mr_title, - head_branch, base_branch, author_username, state, approval_state, pipeline_state, + head_branch, head_sha, base_branch, base_sha, author_username, state, approval_state, pipeline_state, merge_status, draft, approval_count, required_approvals, pipeline_jobs_total, pipeline_jobs_pass, detailed_merge_status, reviewer_count, unapproved_reviewers, unresolved_discussions, created_at, merged_at, closed_at, last_synced_at, updated_at ) VALUES ( :id, :task_id, :repository_id, :host, :project_path, :mr_iid, :mr_url, :mr_title, - :head_branch, :base_branch, :author_username, :state, :approval_state, :pipeline_state, + :head_branch, :head_sha, :base_branch, :base_sha, :author_username, :state, :approval_state, :pipeline_state, :merge_status, CASE WHEN :draft THEN 1 ELSE 0 END, :approval_count, :required_approvals, :pipeline_jobs_total, :pipeline_jobs_pass, :detailed_merge_status, :reviewer_count, :unapproved_reviewers, :unresolved_discussions, @@ -634,7 +656,7 @@ func (s *Store) UpsertTaskMR(ctx context.Context, tm *TaskMR) error { ) ON CONFLICT(task_id, repository_id, project_path, mr_iid) DO UPDATE SET host = excluded.host, mr_url = excluded.mr_url, mr_title = excluded.mr_title, - head_branch = excluded.head_branch, base_branch = excluded.base_branch, + head_branch = excluded.head_branch, head_sha = excluded.head_sha, base_branch = excluded.base_branch, base_sha = excluded.base_sha, author_username = excluded.author_username, state = excluded.state, approval_state = excluded.approval_state, pipeline_state = excluded.pipeline_state, merge_status = excluded.merge_status, draft = excluded.draft, @@ -728,6 +750,31 @@ func (s *Store) ListTaskMRsByTask(ctx context.Context, taskID string) ([]*TaskMR return out, nil } +// ListTaskMRsByTaskIDs returns every MR association for the supplied tasks, +// grouped by task_id and ordered by created_at within each task. +func (s *Store) ListTaskMRsByTaskIDs(ctx context.Context, taskIDs []string) (map[string][]*TaskMR, error) { + if len(taskIDs) == 0 { + return map[string][]*TaskMR{}, nil + } + query, args, err := sqlx.In( + `SELECT `+taskMRSelectCols+` FROM gitlab_task_mrs + WHERE task_id IN (?) ORDER BY created_at ASC`, taskIDs, + ) + if err != nil { + return nil, err + } + query = s.ro.Rebind(query) + var mrs []TaskMR + if err := s.ro.SelectContext(ctx, &mrs, query, args...); err != nil { + return nil, err + } + out := make(map[string][]*TaskMR) + for i := range mrs { + out[mrs[i].TaskID] = append(out[mrs[i].TaskID], &mrs[i]) + } + return out, nil +} + // ListTaskMRsByWorkspaceID returns every MR association for tasks inside the // given workspace, grouped by task_id. Empty map when the workspace has no // GitLab MRs. diff --git a/apps/backend/internal/gitlab/store_task_mr_sha_test.go b/apps/backend/internal/gitlab/store_task_mr_sha_test.go new file mode 100644 index 0000000000..c7b58450bf --- /dev/null +++ b/apps/backend/internal/gitlab/store_task_mr_sha_test.go @@ -0,0 +1,31 @@ +package gitlab + +import ( + "context" + "testing" +) + +func TestUpsertTaskMRRoundTripsCommitSHAs(t *testing.T) { + store := newTestStore(t) + ctx := context.Background() + mr := newTestMR("task-1", "repo-1", "group/project", 224) + mr.HeadSHA = "head-sha" + mr.BaseSHA = "base-sha" + if err := store.UpsertTaskMR(ctx, mr); err != nil { + t.Fatalf("upsert task MR: %v", err) + } + + got, err := store.GetTaskMR(ctx, "task-1", "repo-1", "group/project", 224) + if err != nil { + t.Fatalf("get task MR: %v", err) + } + if got == nil { + t.Fatal("task MR is missing") + } + if got.HeadSHA != "head-sha" { + t.Errorf("HeadSHA = %q, want head-sha", got.HeadSHA) + } + if got.BaseSHA != "base-sha" { + t.Errorf("BaseSHA = %q, want base-sha", got.BaseSHA) + } +} diff --git a/apps/backend/internal/gitlab/store_test.go b/apps/backend/internal/gitlab/store_test.go index e4eee0cca0..3a41227365 100644 --- a/apps/backend/internal/gitlab/store_test.go +++ b/apps/backend/internal/gitlab/store_test.go @@ -274,6 +274,34 @@ func TestStore_ListTaskMRsByTask_ReturnsEmptyForUnknownTask(t *testing.T) { } } +func TestStore_ListTaskMRsByTaskIDs_GroupsByTaskAndPreservesRepositoryIdentity(t *testing.T) { + store := newTestStore(t) + ctx := context.Background() + seedWorkspace(t, store, "ws-1") + seedTask(t, store, "task-1", "ws-1") + seedTask(t, store, "task-2", "ws-1") + if err := store.UpsertTaskMR(ctx, newTestMR("task-1", "repo-canonical", "group/project", 224)); err != nil { + t.Fatalf("upsert task-1 MR: %v", err) + } + if err := store.UpsertTaskMR(ctx, newTestMR("task-2", "repo-fork", "fork/project", 224)); err != nil { + t.Fatalf("upsert task-2 MR: %v", err) + } + + got, err := store.ListTaskMRsByTaskIDs(ctx, []string{"task-1", "task-2", "missing"}) + if err != nil { + t.Fatalf("ListTaskMRsByTaskIDs: %v", err) + } + if len(got["task-1"]) != 1 || got["task-1"][0].RepositoryID != "repo-canonical" { + t.Fatalf("task-1 MRs = %#v, want canonical repo row", got["task-1"]) + } + if len(got["task-2"]) != 1 || got["task-2"][0].RepositoryID != "repo-fork" { + t.Fatalf("task-2 MRs = %#v, want fork repo row", got["task-2"]) + } + if _, ok := got["missing"]; ok { + t.Fatal("missing task unexpectedly present in grouped results") + } +} + func TestStore_DeleteTaskMR_RemovesByID(t *testing.T) { store := newTestStore(t) ctx := context.Background() diff --git a/apps/backend/internal/mcp/handlers/handlers.go b/apps/backend/internal/mcp/handlers/handlers.go index eb7c5cc926..5f20b2207c 100644 --- a/apps/backend/internal/mcp/handlers/handlers.go +++ b/apps/backend/internal/mcp/handlers/handlers.go @@ -293,6 +293,7 @@ type Handlers struct { // Optional PR lister (set via SetTaskPRLister) used to enrich // task-listing responses with associated pull requests. taskPRLister TaskPRLister + taskMRLister TaskMRLister // Native code review (optional, set via SetReviewService / // SetReviewRunner). Without them the review actions are simply not // registered — see registerReviewHandlers. @@ -303,6 +304,7 @@ type Handlers struct { // Optional task-bound GitHub PR automation controls. taskPRAutomation TaskPRAutomationService + taskChangeLinks TaskChangeLinkService taskPRAutoFixOutcome TaskPRAutoFixOutcomeService remoteContributionSvc RemoteContributionService diagnosticBundles DiagnosticBundleProvider @@ -331,6 +333,11 @@ func (h *Handlers) releaseWorkspacePolicyAfterCreateRollback(ctx context.Context } } +// SetTaskChangeLinkService wires provider-neutral PR/MR association changes. +func (h *Handlers) SetTaskChangeLinkService(links TaskChangeLinkService) { + h.taskChangeLinks = links +} + // NewHandlers creates new MCP handlers. func NewHandlers( taskSvc *service.Service, @@ -488,6 +495,13 @@ func (h *Handlers) registerTaskMutationHandlers(d *guardedMCPDispatcher) { d.RegisterFunc(ws.ActionMCPCreateTask, h.handleCreateTask) d.RegisterFunc(ws.ActionMCPUpdateTask, h.handleUpdateTask) d.RegisterFunc(ws.ActionMCPSetTaskTitle, h.handleSetTaskTitle) + d.RegisterFunc(ws.ActionMCPGetTaskPRAutomation, h.handleGetTaskPRAutomation) + d.RegisterFunc(ws.ActionMCPUpdateTaskPRAutomation, h.handleUpdateTaskPRAutomation) + d.RegisterFunc(ws.ActionMCPGetTaskMRAutomation, h.handleGetTaskMRAutomation) + d.RegisterFunc(ws.ActionMCPUpdateTaskMRAutomation, h.handleUpdateTaskMRAutomation) + d.RegisterFunc(ws.ActionMCPLinkTaskPR, h.handleLinkTaskPR) + d.RegisterFunc(ws.ActionMCPUnlinkTaskPR, h.handleUnlinkTaskPR) + d.RegisterFunc(ws.ActionMCPReplaceTaskPR, h.handleReplaceTaskPR) d.RegisterFunc(ws.ActionMCPAddTaskDependency, h.handleAddTaskDependency) d.RegisterFunc(ws.ActionMCPRemoveTaskDependency, h.handleRemoveTaskDependency) d.RegisterFunc(ws.ActionMCPAddBranchToTask, h.handleAddBranchToTask) diff --git a/apps/backend/internal/mcp/handlers/task_change_link.go b/apps/backend/internal/mcp/handlers/task_change_link.go new file mode 100644 index 0000000000..6c5b044c21 --- /dev/null +++ b/apps/backend/internal/mcp/handlers/task_change_link.go @@ -0,0 +1,142 @@ +package handlers + +import ( + "context" + "encoding/json" + "errors" + "strings" + + mcpscope "github.com/kandev/kandev/internal/mcp/scope" + ws "github.com/kandev/kandev/pkg/websocket" +) + +// TaskChangeLink identifies an active GitHub pull request or GitLab merge +// request. RepositoryID and Number deliberately travel together so same-number +// changes in forks and canonical repositories remain distinct. +type TaskChangeLink struct { + Provider string `json:"provider"` + RepositoryID string `json:"repository_id"` + Number int `json:"number"` +} + +// TaskChangeLinkRequest is one provider-neutral change-request operation. +type TaskChangeLinkRequest struct { + TaskID string + Link TaskChangeLink + Old *TaskChangeLink +} + +// TaskChangeLinkService owns provider dispatch and returns the resulting +// active links after each mutation. +type TaskChangeLinkService interface { + LinkTaskChange(context.Context, TaskChangeLinkRequest) ([]TaskChangeLink, error) + UnlinkTaskChange(context.Context, TaskChangeLinkRequest) ([]TaskChangeLink, error) + ReplaceTaskChange(context.Context, TaskChangeLinkRequest) ([]TaskChangeLink, error) +} + +func (h *Handlers) handleLinkTaskPR(ctx context.Context, msg *ws.Message) (*ws.Message, error) { + req, response, err := h.taskChangeLinkRequest(ctx, msg, false) + if response != nil || err != nil { + return response, err + } + if h.taskChangeLinks == nil { + return taskChangeLinksUnavailable(msg) + } + return h.applyTaskChangeLink(ctx, msg, req, h.taskChangeLinks.LinkTaskChange) +} + +func (h *Handlers) handleUnlinkTaskPR(ctx context.Context, msg *ws.Message) (*ws.Message, error) { + req, response, err := h.taskChangeLinkRequest(ctx, msg, false) + if response != nil || err != nil { + return response, err + } + if h.taskChangeLinks == nil { + return taskChangeLinksUnavailable(msg) + } + return h.applyTaskChangeLink(ctx, msg, req, h.taskChangeLinks.UnlinkTaskChange) +} + +func (h *Handlers) handleReplaceTaskPR(ctx context.Context, msg *ws.Message) (*ws.Message, error) { + req, response, err := h.taskChangeLinkRequest(ctx, msg, true) + if response != nil || err != nil { + return response, err + } + if h.taskChangeLinks == nil { + return taskChangeLinksUnavailable(msg) + } + return h.applyTaskChangeLink(ctx, msg, req, h.taskChangeLinks.ReplaceTaskChange) +} + +func taskChangeLinksUnavailable(msg *ws.Message) (*ws.Message, error) { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeInternalError, "task PR link management is not available", nil) +} + +func (h *Handlers) applyTaskChangeLink(ctx context.Context, msg *ws.Message, req TaskChangeLinkRequest, apply func(context.Context, TaskChangeLinkRequest) ([]TaskChangeLink, error)) (*ws.Message, error) { + links, err := apply(ctx, req) + if err != nil { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, err.Error(), nil) + } + return ws.NewResponse(msg.ID, msg.Action, map[string]any{"task_id": req.TaskID, "links": links}) +} + +func (h *Handlers) taskChangeLinkRequest(ctx context.Context, msg *ws.Message, replace bool) (TaskChangeLinkRequest, *ws.Message, error) { + var payload struct { + TaskID string `json:"task_id"` + CallerTaskID string `json:"caller_task_id"` + Provider string `json:"provider"` + RepositoryID string `json:"repository_id"` + Number int `json:"number"` + OldProvider string `json:"old_provider"` + OldRepositoryID string `json:"old_repository_id"` + OldNumber int `json:"old_number"` + } + if err := json.Unmarshal(msg.Payload, &payload); err != nil { + response, responseErr := ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "Invalid payload: "+err.Error(), nil) + return TaskChangeLinkRequest{}, response, responseErr + } + link, err := validateTaskChangeLink(payload.Provider, payload.RepositoryID, payload.Number) + if err != nil || strings.TrimSpace(payload.TaskID) == "" { + response, responseErr := ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "task_id, provider, repository_id, and positive number are required", nil) + return TaskChangeLinkRequest{}, response, responseErr + } + if h.taskSvc == nil { + response, responseErr := ws.NewError(msg.ID, msg.Action, ws.ErrorCodeInternalError, "task service is not available", nil) + return TaskChangeLinkRequest{}, response, responseErr + } + target, targetErr := h.taskSvc.GetTask(ctx, payload.TaskID) + if targetErr != nil || target == nil { + response, responseErr := ws.NewError(msg.ID, msg.Action, ws.ErrorCodeNotFound, "task not found", nil) + return TaskChangeLinkRequest{}, response, responseErr + } + if strings.TrimSpace(payload.CallerTaskID) == "" { + response, responseErr := ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "caller_task_id is required", nil) + return TaskChangeLinkRequest{}, response, responseErr + } + principal, hasPrincipal := mcpscope.PrincipalFromContext(ctx) + if !hasPrincipal || strings.TrimSpace(principal.WorkspaceID) == "" { + response, responseErr := ws.NewError(msg.ID, msg.Action, ws.ErrorCodeForbidden, "trusted MCP caller identity is required", nil) + return TaskChangeLinkRequest{}, response, responseErr + } + if payload.CallerTaskID != principal.CallerTaskID || target.WorkspaceID != principal.WorkspaceID { + response, responseErr := ws.NewError(msg.ID, msg.Action, ws.ErrorCodeForbidden, "task is outside the caller workspace", nil) + return TaskChangeLinkRequest{}, response, responseErr + } + req := TaskChangeLinkRequest{TaskID: target.ID, Link: link} + if replace { + old, oldErr := validateTaskChangeLink(payload.OldProvider, payload.OldRepositoryID, payload.OldNumber) + if oldErr != nil { + response, responseErr := ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "old_provider, old_repository_id, and positive old_number are required", nil) + return TaskChangeLinkRequest{}, response, responseErr + } + req.Old = &old + } + return req, nil, nil +} + +func validateTaskChangeLink(provider, repositoryID string, number int) (TaskChangeLink, error) { + provider = strings.ToLower(strings.TrimSpace(provider)) + if (provider != "github" && provider != "gitlab") || strings.TrimSpace(repositoryID) == "" || number <= 0 { + return TaskChangeLink{}, errors.New("invalid task change identity") + } + return TaskChangeLink{Provider: provider, RepositoryID: repositoryID, Number: number}, nil +} diff --git a/apps/backend/internal/mcp/handlers/task_change_link_test.go b/apps/backend/internal/mcp/handlers/task_change_link_test.go new file mode 100644 index 0000000000..6ea1d95cc7 --- /dev/null +++ b/apps/backend/internal/mcp/handlers/task_change_link_test.go @@ -0,0 +1,168 @@ +package handlers + +import ( + "context" + "testing" + "time" + + "github.com/kandev/kandev/internal/mcp/scope" + "github.com/kandev/kandev/internal/task/models" + v1 "github.com/kandev/kandev/pkg/api/v1" + ws "github.com/kandev/kandev/pkg/websocket" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestValidateTaskChangeLinkRequiresProviderRepositoryAndNumber(t *testing.T) { + valid, err := validateTaskChangeLink("GitLab", "repo-1", 42) + if err != nil || valid.Provider != "gitlab" { + t.Fatalf("validateTaskChangeLink() = %#v, %v", valid, err) + } + for _, input := range []TaskChangeLink{ + {Provider: "", RepositoryID: "repo-1", Number: 1}, + {Provider: "github", RepositoryID: "", Number: 1}, + {Provider: "github", RepositoryID: "repo-1", Number: 0}, + {Provider: "other", RepositoryID: "repo-1", Number: 1}, + } { + if _, err := validateTaskChangeLink(input.Provider, input.RepositoryID, input.Number); err == nil { + t.Fatalf("validateTaskChangeLink(%#v) succeeded", input) + } + } +} + +func TestTaskChangeLinkRequestRejectsCrossWorkspaceCaller(t *testing.T) { + svc, repo := newTestTaskService(t) + ctx := context.Background() + now := time.Now().UTC() + require.NoError(t, repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-target", Name: "Target", CreatedAt: now, UpdatedAt: now})) + require.NoError(t, repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-caller", Name: "Caller", CreatedAt: now, UpdatedAt: now})) + require.NoError(t, repo.CreateTask(ctx, &models.Task{ID: "task-target", WorkspaceID: "ws-target", Title: "Target", State: v1.TaskStateTODO, CreatedAt: now, UpdatedAt: now})) + require.NoError(t, repo.CreateTask(ctx, &models.Task{ID: "task-caller", WorkspaceID: "ws-caller", Title: "Caller", State: v1.TaskStateTODO, CreatedAt: now, UpdatedAt: now})) + + h := &Handlers{taskSvc: svc} + ctx = scope.WithPrincipal(ctx, scope.Principal{ + WorkspaceID: "ws-caller", CallerTaskID: "task-caller", CallerSessionID: "session-caller", + }) + msg := makeWSMessage(t, ws.ActionMCPLinkTaskPR, map[string]any{ + "task_id": "task-target", "caller_task_id": "task-caller", + "provider": "github", "repository_id": "repo-1", "number": 1, + }) + + _, resp, err := h.taskChangeLinkRequest(ctx, msg, false) + require.NoError(t, err) + assertWSError(t, resp, ws.ErrorCodeForbidden) +} + +func TestTaskChangeLinkRequestRejectsMissingCallerTask(t *testing.T) { + svc, repo := newTestTaskService(t) + ctx := context.Background() + now := time.Now().UTC() + require.NoError(t, repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-target", Name: "Target", CreatedAt: now, UpdatedAt: now})) + require.NoError(t, repo.CreateTask(ctx, &models.Task{ID: "task-target", WorkspaceID: "ws-target", Title: "Target", State: v1.TaskStateTODO, CreatedAt: now, UpdatedAt: now})) + + h := &Handlers{taskSvc: svc} + msg := makeWSMessage(t, ws.ActionMCPLinkTaskPR, map[string]any{ + "task_id": "task-target", + "provider": "github", "repository_id": "repo-1", "number": 1, + }) + + _, resp, err := h.taskChangeLinkRequest(ctx, msg, false) + require.NoError(t, err) + assertWSError(t, resp, ws.ErrorCodeValidation) +} + +func TestTaskChangeLinkRequestParsesReplacementIdentity(t *testing.T) { + svc, repo := newTestTaskService(t) + ctx := context.Background() + now := time.Now().UTC() + require.NoError(t, repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-1", Name: "Workspace", CreatedAt: now, UpdatedAt: now})) + require.NoError(t, repo.CreateTask(ctx, &models.Task{ID: "task-target", WorkspaceID: "ws-1", Title: "Target", State: v1.TaskStateTODO, CreatedAt: now, UpdatedAt: now})) + require.NoError(t, repo.CreateTask(ctx, &models.Task{ID: "task-caller", WorkspaceID: "ws-1", Title: "Caller", State: v1.TaskStateTODO, CreatedAt: now, UpdatedAt: now})) + + h := &Handlers{taskSvc: svc} + ctx = scope.WithPrincipal(ctx, scope.Principal{ + WorkspaceID: "ws-1", CallerTaskID: "task-caller", CallerSessionID: "session-caller", + }) + msg := makeWSMessage(t, ws.ActionMCPReplaceTaskPR, map[string]any{ + "task_id": "task-target", "caller_task_id": "task-caller", + "provider": "gitlab", "repository_id": "repo-new", "number": 42, + "old_provider": "github", "old_repository_id": "repo-old", "old_number": 7, + }) + + req, resp, err := h.taskChangeLinkRequest(ctx, msg, true) + require.NoError(t, err) + require.Nil(t, resp) + assert.Equal(t, TaskChangeLink{Provider: "gitlab", RepositoryID: "repo-new", Number: 42}, req.Link) + require.NotNil(t, req.Old) + assert.Equal(t, TaskChangeLink{Provider: "github", RepositoryID: "repo-old", Number: 7}, *req.Old) +} + +func TestTaskChangeLinkRequestRejectsInvalidReplacementIdentity(t *testing.T) { + svc, repo := newTestTaskService(t) + ctx := context.Background() + now := time.Now().UTC() + require.NoError(t, repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-1", Name: "Workspace", CreatedAt: now, UpdatedAt: now})) + require.NoError(t, repo.CreateTask(ctx, &models.Task{ID: "task-target", WorkspaceID: "ws-1", Title: "Target", State: v1.TaskStateTODO, CreatedAt: now, UpdatedAt: now})) + require.NoError(t, repo.CreateTask(ctx, &models.Task{ID: "task-caller", WorkspaceID: "ws-1", Title: "Caller", State: v1.TaskStateTODO, CreatedAt: now, UpdatedAt: now})) + + h := &Handlers{taskSvc: svc} + ctx = scope.WithPrincipal(ctx, scope.Principal{ + WorkspaceID: "ws-1", CallerTaskID: "task-caller", CallerSessionID: "session-caller", + }) + msg := makeWSMessage(t, ws.ActionMCPReplaceTaskPR, map[string]any{ + "task_id": "task-target", "caller_task_id": "task-caller", + "provider": "gitlab", "repository_id": "repo-new", "number": 42, + }) + + _, resp, err := h.taskChangeLinkRequest(ctx, msg, true) + require.NoError(t, err) + assertWSError(t, resp, ws.ErrorCodeValidation) +} + +func TestTaskChangeLinkRequestRejectsPayloadCallerMismatch(t *testing.T) { + svc, repo := newTestTaskService(t) + ctx := context.Background() + now := time.Now().UTC() + require.NoError(t, repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-1", Name: "Workspace", CreatedAt: now, UpdatedAt: now})) + for _, task := range []*models.Task{ + {ID: "task-target", WorkspaceID: "ws-1", Title: "Target", State: v1.TaskStateTODO, CreatedAt: now, UpdatedAt: now}, + {ID: "task-caller", WorkspaceID: "ws-1", Title: "Caller", State: v1.TaskStateTODO, CreatedAt: now, UpdatedAt: now}, + {ID: "task-other", WorkspaceID: "ws-1", Title: "Other", State: v1.TaskStateTODO, CreatedAt: now, UpdatedAt: now}, + } { + require.NoError(t, repo.CreateTask(ctx, task)) + } + ctx = scope.WithPrincipal(ctx, scope.Principal{ + WorkspaceID: "ws-1", CallerTaskID: "task-caller", CallerSessionID: "session-caller", + }) + h := &Handlers{taskSvc: svc} + msg := makeWSMessage(t, ws.ActionMCPLinkTaskPR, map[string]any{ + "task_id": "task-target", "caller_task_id": "task-other", + "provider": "github", "repository_id": "repo-1", "number": 1, + }) + + _, resp, err := h.taskChangeLinkRequest(ctx, msg, false) + require.NoError(t, err) + assertWSError(t, resp, ws.ErrorCodeForbidden) +} + +func TestTaskChangeLinkRequestRequiresTrustedPrincipal(t *testing.T) { + svc, repo := newTestTaskService(t) + ctx := context.Background() + now := time.Now().UTC() + require.NoError(t, repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-1", Name: "Workspace", CreatedAt: now, UpdatedAt: now})) + for _, task := range []*models.Task{ + {ID: "task-target", WorkspaceID: "ws-1", Title: "Target", State: v1.TaskStateTODO, CreatedAt: now, UpdatedAt: now}, + {ID: "task-caller", WorkspaceID: "ws-1", Title: "Caller", State: v1.TaskStateTODO, CreatedAt: now, UpdatedAt: now}, + } { + require.NoError(t, repo.CreateTask(ctx, task)) + } + h := &Handlers{taskSvc: svc} + msg := makeWSMessage(t, ws.ActionMCPLinkTaskPR, map[string]any{ + "task_id": "task-target", "caller_task_id": "task-caller", + "provider": "github", "repository_id": "repo-1", "number": 1, + }) + + _, resp, err := h.taskChangeLinkRequest(ctx, msg, false) + require.NoError(t, err) + assertWSError(t, resp, ws.ErrorCodeForbidden) +} diff --git a/apps/backend/internal/mcp/handlers/task_pr_enrich.go b/apps/backend/internal/mcp/handlers/task_pr_enrich.go index 1437b177fa..6acb3dd403 100644 --- a/apps/backend/internal/mcp/handlers/task_pr_enrich.go +++ b/apps/backend/internal/mcp/handlers/task_pr_enrich.go @@ -11,15 +11,39 @@ import ( v1 "github.com/kandev/kandev/pkg/api/v1" ) -// TaskPRInfo is the decoupled view of a task↔PR association consumed by the +// TaskPRInfo is the decoupled view of a task<->PR association consumed by the // MCP task-listing handlers. The cmd wiring adapts *github.Service.TaskPR into // this shape so this package does not depend on internal/github. type TaskPRInfo struct { - Number int - URL string - Title string - State string // open, closed, merged - MergedAt *time.Time + RepositoryID string + Number int + URL string + Title string + State string // open, closed, merged + Draft *bool + BaseRef string + BaseSHA string + HeadRef string + HeadSHA string + MergedAt *time.Time + ClosedAt *time.Time +} + +// TaskMRInfo is the decoupled view of a GitLab task<->MR association consumed +// by the MCP task-listing handlers. +type TaskMRInfo struct { + RepositoryID string + Number int + URL string + Title string + State string + Draft bool + BaseRef string + BaseSHA string + HeadRef string + HeadSHA string + MergedAt *time.Time + ClosedAt *time.Time } // TaskPRLister returns PR associations grouped by task ID. Backed by @@ -29,68 +53,132 @@ type TaskPRLister interface { ListTaskPRsByTaskIDs(ctx context.Context, taskIDs []string) (map[string][]TaskPRInfo, error) } +// TaskMRLister returns MR associations grouped by task ID. Backed by +// *gitlab.Service in production; left nil in contexts without GitLab. +type TaskMRLister interface { + ListTaskMRsByTaskIDs(ctx context.Context, taskIDs []string) (map[string][]TaskMRInfo, error) +} + // SetTaskPRLister wires the optional PR lister used to enrich task-listing // responses with associated pull requests. func (h *Handlers) SetTaskPRLister(l TaskPRLister) { h.taskPRLister = l } -// prsByTaskID looks up PR associations for the given task IDs via the wired -// lister, mapping each to the shared v1.TaskPRSummary shape. Returns nil when -// the lister is unset, there are no IDs, or the lookup fails (logged) — callers -// treat a nil map as "no PRs to attach". -func (h *Handlers) prsByTaskID(ctx context.Context, taskIDs []string) map[string][]v1.TaskPRSummary { - if h.taskPRLister == nil || len(taskIDs) == 0 { - return nil +// SetTaskMRLister wires the optional MR lister used to enrich task-listing +// responses with associated merge requests. +func (h *Handlers) SetTaskMRLister(l TaskMRLister) { + h.taskMRLister = l +} + +type taskChangeRequestsByID struct { + prs map[string][]v1.TaskPRSummary + changes map[string][]v1.TaskChangeRequestSummary +} + +func (h *Handlers) changeRequestsByTaskID(ctx context.Context, taskIDs []string) taskChangeRequestsByID { + out := taskChangeRequestsByID{ + prs: map[string][]v1.TaskPRSummary{}, + changes: map[string][]v1.TaskChangeRequestSummary{}, } + if len(taskIDs) == 0 { + return out + } + if h.taskPRLister != nil { + h.addPRSummaries(ctx, taskIDs, out) + } + if h.taskMRLister != nil { + h.addMRSummaries(ctx, taskIDs, out) + } + return out +} + +func (h *Handlers) addPRSummaries(ctx context.Context, taskIDs []string, out taskChangeRequestsByID) { byTask, err := h.taskPRLister.ListTaskPRsByTaskIDs(ctx, taskIDs) if err != nil { h.logger.Warn("failed to list task PRs for MCP response", zap.Error(err)) - return nil + return } - out := make(map[string][]v1.TaskPRSummary, len(byTask)) for taskID, prs := range byTask { - if len(prs) == 0 { - continue - } - summaries := make([]v1.TaskPRSummary, 0, len(prs)) for _, pr := range prs { - summaries = append(summaries, v1.TaskPRSummary{ + out.prs[taskID] = append(out.prs[taskID], v1.TaskPRSummary{ Number: pr.Number, URL: pr.URL, Title: pr.Title, State: pr.State, MergedAt: pr.MergedAt, }) + out.changes[taskID] = append(out.changes[taskID], v1.TaskChangeRequestSummary{ + Provider: "github", + RepositoryID: pr.RepositoryID, + Number: pr.Number, + URL: pr.URL, + Title: pr.Title, + State: pr.State, + Draft: pr.Draft, + BaseRef: pr.BaseRef, + HeadRef: pr.HeadRef, + HeadSHA: pr.HeadSHA, + MergedAt: pr.MergedAt, + ClosedAt: pr.ClosedAt, + }) } - out[taskID] = summaries } - return out } -// enrichTasksWithPRs populates dto.TaskDTO.PRs for each task from the wired -// TaskPRLister. No-op when the lister is unset or there are no tasks. +func (h *Handlers) addMRSummaries(ctx context.Context, taskIDs []string, out taskChangeRequestsByID) { + byTask, err := h.taskMRLister.ListTaskMRsByTaskIDs(ctx, taskIDs) + if err != nil { + h.logger.Warn("failed to list task MRs for MCP response", zap.Error(err)) + return + } + for taskID, mrs := range byTask { + for _, mr := range mrs { + draft := mr.Draft + out.changes[taskID] = append(out.changes[taskID], v1.TaskChangeRequestSummary{ + Provider: "gitlab", + RepositoryID: mr.RepositoryID, + Number: mr.Number, + URL: mr.URL, + Title: mr.Title, + State: mr.State, + Draft: &draft, + BaseRef: mr.BaseRef, + BaseSHA: mr.BaseSHA, + HeadRef: mr.HeadRef, + HeadSHA: mr.HeadSHA, + MergedAt: mr.MergedAt, + ClosedAt: mr.ClosedAt, + }) + } + } +} + +// enrichTasksWithPRs populates dto.TaskDTO.PRs for compatibility and +// dto.TaskDTO.ChangeRequests with provider-neutral GitHub/GitLab links. func (h *Handlers) enrichTasksWithPRs(ctx context.Context, tasks []dto.TaskDTO) { - if h.taskPRLister == nil || len(tasks) == 0 { + if (h.taskPRLister == nil && h.taskMRLister == nil) || len(tasks) == 0 { return } ids := make([]string, len(tasks)) for i := range tasks { ids[i] = tasks[i].ID } - byTask := h.prsByTaskID(ctx, ids) + byTask := h.changeRequestsByTaskID(ctx, ids) for i := range tasks { - if prs := byTask[tasks[i].ID]; len(prs) > 0 { + if prs := byTask.prs[tasks[i].ID]; len(prs) > 0 { tasks[i].PRs = prs } + if changes := byTask.changes[tasks[i].ID]; len(changes) > 0 { + tasks[i].ChangeRequests = changes + } } } -// enrichRelatedTasksWithPRs populates RelatedTask.PRs across every relation -// surface (the task itself, parent, children, siblings, blockers, blocked-by) -// in a single batched lookup. No-op when the lister is unset. +// enrichRelatedTasksWithPRs populates related-task PR compatibility data and +// provider-neutral GitHub/GitLab change requests in one batched lookup. func (h *Handlers) enrichRelatedTasksWithPRs(ctx context.Context, related *service.RelatedTasks) { - if h.taskPRLister == nil || related == nil { + if (h.taskPRLister == nil && h.taskMRLister == nil) || related == nil { return } nodes := []*service.RelatedTask{&related.Task, related.Parent} @@ -99,9 +187,6 @@ func (h *Handlers) enrichRelatedTasksWithPRs(ctx context.Context, related *servi nodes = append(nodes, related.Blockers...) nodes = append(nodes, related.BlockedBy...) - // A task can appear in more than one relation group (e.g. both a sibling - // and a blocker), so dedup IDs before the lookup to avoid sending the same - // value twice to the lister's WHERE id IN (...) query. ids := make([]string, 0, len(nodes)) seen := make(map[string]struct{}, len(nodes)) for _, n := range nodes { @@ -114,16 +199,16 @@ func (h *Handlers) enrichRelatedTasksWithPRs(ctx context.Context, related *servi seen[n.ID] = struct{}{} ids = append(ids, n.ID) } - byTask := h.prsByTaskID(ctx, ids) - if byTask == nil { - return - } + byTask := h.changeRequestsByTaskID(ctx, ids) for _, n := range nodes { if n == nil { continue } - if prs := byTask[n.ID]; len(prs) > 0 { + if prs := byTask.prs[n.ID]; len(prs) > 0 { n.PRs = prs } + if changes := byTask.changes[n.ID]; len(changes) > 0 { + n.ChangeRequests = changes + } } } diff --git a/apps/backend/internal/mcp/handlers/task_pr_enrich_test.go b/apps/backend/internal/mcp/handlers/task_pr_enrich_test.go index 866703afe0..b8dd698b5d 100644 --- a/apps/backend/internal/mcp/handlers/task_pr_enrich_test.go +++ b/apps/backend/internal/mcp/handlers/task_pr_enrich_test.go @@ -30,11 +30,26 @@ func (f *fakeTaskPRLister) ListTaskPRsByTaskIDs(_ context.Context, taskIDs []str return f.byTask, nil } +type fakeTaskMRLister struct { + byTask map[string][]TaskMRInfo + err error + gotIDs []string +} + +func (f *fakeTaskMRLister) ListTaskMRsByTaskIDs(_ context.Context, taskIDs []string) (map[string][]TaskMRInfo, error) { + f.gotIDs = taskIDs + if f.err != nil { + return nil, f.err + } + return f.byTask, nil +} + func TestEnrichTasksWithPRs(t *testing.T) { + draft := false lister := &fakeTaskPRLister{ byTask: map[string][]TaskPRInfo{ "task-1": { - {Number: 42, URL: "https://github.com/o/r/pull/42", Title: "Fix bug", State: "merged"}, + {RepositoryID: "repo-gh", Number: 42, URL: "https://github.com/o/r/pull/42", Title: "Fix bug", State: "merged", Draft: &draft, BaseRef: "main", HeadRef: "fix", HeadSHA: "head-sha"}, {Number: 43, URL: "https://github.com/o/r/pull/43", Title: "Add feature", State: "open"}, }, }, @@ -50,9 +65,45 @@ func TestEnrichTasksWithPRs(t *testing.T) { Number: 42, URL: "https://github.com/o/r/pull/42", Title: "Fix bug", State: "merged", }, dtos[0].PRs[0]) assert.Equal(t, "open", dtos[0].PRs[1].State) + require.Len(t, dtos[0].ChangeRequests, 2) + assert.Equal(t, v1.TaskChangeRequestSummary{ + Provider: "github", RepositoryID: "repo-gh", Number: 42, + URL: "https://github.com/o/r/pull/42", Title: "Fix bug", State: "merged", + Draft: &draft, BaseRef: "main", HeadRef: "fix", HeadSHA: "head-sha", + }, dtos[0].ChangeRequests[0]) assert.Nil(t, dtos[1].PRs, "tasks without PRs stay nil") } +func TestEnrichTasksWithGitLabMRsAddsProviderNeutralChangeRequests(t *testing.T) { + merged := time.Now().UTC() + lister := &fakeTaskMRLister{byTask: map[string][]TaskMRInfo{ + "task-1": {{ + RepositoryID: "repo-gl", Number: 224, URL: "https://gitlab.example.test/group/project/-/merge_requests/224", + Title: "Ship MR", State: "merged", Draft: true, BaseRef: "main", BaseSHA: "base-sha", HeadRef: "feature", HeadSHA: "head-sha", MergedAt: &merged, + }}, + }} + h := &Handlers{taskMRLister: lister, logger: testLogger(t).WithFields()} + + dtos := []dto.TaskDTO{{ID: "task-1"}, {ID: "task-2"}} + h.enrichTasksWithPRs(context.Background(), dtos) + + assert.ElementsMatch(t, []string{"task-1", "task-2"}, lister.gotIDs) + assert.Nil(t, dtos[0].PRs, "GitLab MRs do not populate the legacy GitHub-only prs field") + require.Len(t, dtos[0].ChangeRequests, 1) + got := dtos[0].ChangeRequests[0] + assert.Equal(t, "gitlab", got.Provider) + assert.Equal(t, "repo-gl", got.RepositoryID) + assert.Equal(t, 224, got.Number) + assert.Equal(t, "merged", got.State) + require.NotNil(t, got.Draft) + assert.True(t, *got.Draft) + assert.Equal(t, "main", got.BaseRef) + assert.Equal(t, "base-sha", got.BaseSHA) + assert.Equal(t, "feature", got.HeadRef) + assert.Equal(t, "head-sha", got.HeadSHA) + require.NotNil(t, got.MergedAt) +} + func TestEnrichTasksWithPRs_NilListerIsNoop(t *testing.T) { h := &Handlers{logger: testLogger(t).WithFields()} dtos := []dto.TaskDTO{{ID: "task-1"}} @@ -97,6 +148,43 @@ func TestHandleListTasks_IncludesAssociatedPRs(t *testing.T) { assert.Equal(t, "https://github.com/o/r/pull/7", payload.Tasks[0].PRs[0].URL) } +func TestHandleListTasks_IncludesProviderNeutralGitLabMRs(t *testing.T) { + svc, repo := newTestTaskService(t) + ctx := context.Background() + now := time.Now().UTC() + + require.NoError(t, repo.CreateWorkspace(ctx, &models.Workspace{ + ID: "ws-mr", Name: "MR WS", CreatedAt: now, UpdatedAt: now, + })) + require.NoError(t, repo.CreateWorkflow(ctx, &models.Workflow{ + ID: "wf-mr", WorkspaceID: "ws-mr", Name: "MR Workflow", CreatedAt: now, UpdatedAt: now, + })) + require.NoError(t, repo.CreateTask(ctx, &models.Task{ + ID: "task-mr", WorkspaceID: "ws-mr", WorkflowID: "wf-mr", + Title: "Has an MR", State: v1.TaskStateReview, CreatedAt: now, UpdatedAt: now, + })) + + h := &Handlers{ + taskSvc: svc, + logger: testLogger(t).WithFields(), + taskMRLister: &fakeTaskMRLister{byTask: map[string][]TaskMRInfo{ + "task-mr": {{RepositoryID: "repo-gl", Number: 224, URL: "https://gitlab.example.test/g/p/-/merge_requests/224", Title: "Ship it", State: "open"}}, + }}, + } + + msg := makeWSMessage(t, ws.ActionMCPListTasks, map[string]any{"workflow_id": "wf-mr"}) + resp, err := h.handleListTasks(ctx, msg) + require.NoError(t, err) + + var payload dto.ListTasksResponse + require.NoError(t, json.Unmarshal(resp.Payload, &payload)) + require.Len(t, payload.Tasks, 1) + require.Empty(t, payload.Tasks[0].PRs) + require.Len(t, payload.Tasks[0].ChangeRequests, 1) + assert.Equal(t, "gitlab", payload.Tasks[0].ChangeRequests[0].Provider) + assert.Equal(t, 224, payload.Tasks[0].ChangeRequests[0].Number) +} + func TestHandleListRelatedTasks_IncludesAssociatedPRs(t *testing.T) { svc, repo := newTestTaskService(t) ctx := context.Background() @@ -132,13 +220,51 @@ func TestHandleListRelatedTasks_IncludesAssociatedPRs(t *testing.T) { require.NotNil(t, payload.Task.PRs[0].MergedAt, "merged_at should be surfaced") } +func TestHandleListRelatedTasks_IncludesProviderNeutralGitLabMRs(t *testing.T) { + svc, repo := newTestTaskService(t) + ctx := context.Background() + now := time.Now().UTC() + + require.NoError(t, repo.CreateWorkspace(ctx, &models.Workspace{ + ID: "ws-rel-mr", Name: "Rel MR WS", CreatedAt: now, UpdatedAt: now, + })) + require.NoError(t, repo.CreateTask(ctx, &models.Task{ + ID: "task-rel-mr", WorkspaceID: "ws-rel-mr", WorkflowID: "wf-rel-mr", + Title: "Self", State: v1.TaskStateReview, CreatedAt: now, UpdatedAt: now, + })) + + h := &Handlers{ + taskSvc: svc, + handoffSvc: service.NewHandoffService(repo, nil, nil, nil, nil, testLogger(t)), + logger: testLogger(t).WithFields(), + taskMRLister: &fakeTaskMRLister{byTask: map[string][]TaskMRInfo{ + "task-rel-mr": {{RepositoryID: "repo-gl", Number: 224, URL: "https://gitlab.example.test/g/p/-/merge_requests/224", Title: "Fix", State: "open"}}, + }}, + } + + msg := makeWSMessage(t, ws.ActionMCPListRelatedTasks, map[string]any{"task_id": "task-rel-mr"}) + resp, err := h.handleListRelatedTasks(ctx, msg) + require.NoError(t, err) + + var payload service.RelatedTasks + require.NoError(t, json.Unmarshal(resp.Payload, &payload)) + require.Empty(t, payload.Task.PRs) + require.Len(t, payload.Task.ChangeRequests, 1) + assert.Equal(t, "gitlab", payload.Task.ChangeRequests[0].Provider) + assert.Equal(t, 224, payload.Task.ChangeRequests[0].Number) +} + func TestEnrichRelatedTasksWithPRs(t *testing.T) { + draft := false lister := &fakeTaskPRLister{byTask: map[string][]TaskPRInfo{ - "self": {{Number: 1, URL: "u1", State: "open"}}, + "self": {{RepositoryID: "repo-gh", Number: 1, URL: "u1", State: "open", Draft: &draft}}, "parent": {{Number: 3, URL: "u3", State: "closed"}}, "child": {{Number: 2, URL: "u2", State: "merged"}}, }} - h := &Handlers{taskPRLister: lister, logger: testLogger(t).WithFields()} + mrLister := &fakeTaskMRLister{byTask: map[string][]TaskMRInfo{ + "self": {{RepositoryID: "repo-gl", Number: 4, URL: "m1", State: "open"}}, + }} + h := &Handlers{taskPRLister: lister, taskMRLister: mrLister, logger: testLogger(t).WithFields()} related := &service.RelatedTasks{ Task: service.RelatedTask{ID: "self"}, @@ -154,6 +280,9 @@ func TestEnrichRelatedTasksWithPRs(t *testing.T) { lister.gotIDs) require.Len(t, related.Task.PRs, 1) assert.Equal(t, "open", related.Task.PRs[0].State) + require.Len(t, related.Task.ChangeRequests, 2) + assert.Equal(t, "github", related.Task.ChangeRequests[0].Provider) + assert.Equal(t, "gitlab", related.Task.ChangeRequests[1].Provider) require.Len(t, related.Parent.PRs, 1) assert.Equal(t, "closed", related.Parent.PRs[0].State) require.Len(t, related.Children[0].PRs, 1) diff --git a/apps/backend/internal/mcp/server/handlers.go b/apps/backend/internal/mcp/server/handlers.go index a8a744e3c6..feab069ab2 100644 --- a/apps/backend/internal/mcp/server/handlers.go +++ b/apps/backend/internal/mcp/server/handlers.go @@ -320,6 +320,39 @@ func (s *Server) getTaskPRAutomationHandler() server.ToolHandlerFunc { } } +func (s *Server) taskPRLinkHandler(name string) server.ToolHandlerFunc { + return func(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallToolResult, error) { + actions := map[string]string{ + "link_task_pr_kandev": ws.ActionMCPLinkTaskPR, + "unlink_task_pr_kandev": ws.ActionMCPUnlinkTaskPR, + "replace_task_pr_kandev": ws.ActionMCPReplaceTaskPR, + } + action, ok := actions[name] + if !ok { + return mcp.NewToolResultError("unsupported task PR link operation"), nil + } + taskID, err := req.RequireString(mcpKeyTaskID) + if err != nil { + return mcp.NewToolResultError("task_id is required"), nil + } + payload := map[string]interface{}{ + mcpKeyTaskID: taskID, + "caller_task_id": s.taskID, + } + for _, key := range []string{"provider", "repository_id", "number", "old_provider", "old_repository_id", "old_number"} { + if value, ok := req.GetArguments()[key]; ok { + payload[key] = value + } + } + var result map[string]interface{} + if err := s.backend.RequestPayload(ctx, action, payload, &result); err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + data, _ := json.MarshalIndent(result, "", " ") + return mcp.NewToolResultText(string(data)), nil + } +} + func (s *Server) updateTaskPRAutomationHandler() server.ToolHandlerFunc { return func(ctx context.Context, req mcp.CallToolRequest) (*mcp.CallToolResult, error) { payload := map[string]interface{}{"task_id": s.taskID} diff --git a/apps/backend/internal/mcp/server/handlers_test.go b/apps/backend/internal/mcp/server/handlers_test.go index 727c8de79d..d655025fa6 100644 --- a/apps/backend/internal/mcp/server/handlers_test.go +++ b/apps/backend/internal/mcp/server/handlers_test.go @@ -1052,6 +1052,53 @@ func TestTaskMRAutomationToolsNoTaskIDArgument(t *testing.T) { assert.NotContains(t, properties, "task_id") } +func TestTaskPRLinkToolsRequireTaskAndCanonicalIdentity(t *testing.T) { + backend := &testBackend{response: map[string]interface{}{"task_id": "task-current"}} + s := newTaskModeServer(t, backend, "task-current") + + result := callTool(t, s, "link_task_pr_kandev", map[string]interface{}{ + "task_id": "task-target", "provider": "gitlab", "repository_id": "repo-1", "number": 42, + }) + assert.False(t, result.IsError) + assert.Equal(t, ws.ActionMCPLinkTaskPR, backend.lastAction) + payload, ok := backend.lastPayload.(map[string]interface{}) + require.True(t, ok) + assert.Equal(t, "task-target", payload["task_id"]) + assert.Equal(t, "task-current", payload["caller_task_id"]) + assert.Equal(t, "gitlab", payload["provider"]) + assert.Equal(t, "repo-1", payload["repository_id"]) + assert.Equal(t, float64(42), payload["number"]) + + properties := toolInputProperties(t, s, "link_task_pr_kandev") + assert.Contains(t, properties, "task_id") + assert.Contains(t, properties, "provider") + assert.Contains(t, properties, "repository_id") + assert.Contains(t, properties, "number") + assert.NotContains(t, properties, "caller_task_id") +} + +func TestReplaceTaskPRToolRequiresOldCanonicalIdentity(t *testing.T) { + backend := &testBackend{response: map[string]interface{}{"task_id": "task-target"}} + s := newTaskModeServer(t, backend, "task-current") + + result := callTool(t, s, "replace_task_pr_kandev", map[string]interface{}{ + "task_id": "task-target", "provider": "gitlab", "repository_id": "repo-new", "number": 42, + "old_provider": "github", "old_repository_id": "repo-old", "old_number": 7, + }) + assert.False(t, result.IsError) + payload, ok := backend.lastPayload.(map[string]interface{}) + require.True(t, ok) + assert.Equal(t, "github", payload["old_provider"]) + assert.Equal(t, "repo-old", payload["old_repository_id"]) + assert.Equal(t, float64(7), payload["old_number"]) + + properties := toolInputProperties(t, s, "replace_task_pr_kandev") + assert.Contains(t, properties, "old_provider") + assert.Contains(t, properties, "old_repository_id") + assert.Contains(t, properties, "old_number") + assert.Contains(t, s.mcpServer.ListTools()["replace_task_pr_kandev"].Tool.Description, "same provider") +} + // TestUpdateTaskMRAutomationToolForwardsMRIdentityAndAutoFixFields covers // AC31: repository_id/project_path/mr_iid must reach the backend payload // unchanged (including an explicit empty repository_id, R6) so the WS diff --git a/apps/backend/internal/mcp/server/server.go b/apps/backend/internal/mcp/server/server.go index 9179ac145b..3bd6814034 100644 --- a/apps/backend/internal/mcp/server/server.go +++ b/apps/backend/internal/mcp/server/server.go @@ -1085,6 +1085,10 @@ func (s *Server) profileToolGroups() []profileToolGroup { // Dependency edges are manageable wherever a task can be created. {name: "task-dependencies", enabled: func(ctx mcpprofile.Context) bool { return kanban(ctx) || external(ctx) }, register: func(s *Server) { s.registerTaskDependencyTools() }}, {name: "kanban-task", enabled: kanban, register: func(s *Server) { s.registerKanbanTools() }}, + {name: "task-pr-links", enabled: andProfilePredicates(kanban, func(ctx mcpprofile.Context) bool { + return mcpproviders.Contains(ctx.Providers, mcpproviders.GitHub) || + mcpproviders.Contains(ctx.Providers, mcpproviders.GitLab) + }), register: func(s *Server) { s.registerTaskPRLinkTools() }}, {name: "github-pr", enabled: andProfilePredicates(kanban, func(ctx mcpprofile.Context) bool { return mcpproviders.Contains(ctx.Providers, mcpproviders.GitHub) }), register: func(s *Server) { s.registerPRAutomationTools() }}, {name: "gitlab-mr", enabled: andProfilePredicates(kanban, func(ctx mcpprofile.Context) bool { return mcpproviders.Contains(ctx.Providers, mcpproviders.GitLab) }), register: func(s *Server) { s.registerMRAutomationTools() }}, {name: "user-question", enabled: capabilityEnabled(mcpprofile.CapabilityUserQuestion), register: func(s *Server) { s.registerInteractionTools() }}, @@ -1388,6 +1392,40 @@ func (s *Server) registerPRAutomationTools() { ) } +func (s *Server) registerTaskPRLinkTools() { + s.mcpServer.AddTool( + mcp.NewTool("link_task_pr_kandev", taskPRLinkToolOptions(false)...), + s.wrapHandler("link_task_pr_kandev", s.taskPRLinkHandler("link_task_pr_kandev")), + ) + s.mcpServer.AddTool( + mcp.NewTool("unlink_task_pr_kandev", taskPRLinkToolOptions(false)...), + s.wrapHandler("unlink_task_pr_kandev", s.taskPRLinkHandler("unlink_task_pr_kandev")), + ) + s.mcpServer.AddTool( + mcp.NewTool("replace_task_pr_kandev", taskPRLinkToolOptions(true)...), + s.wrapHandler("replace_task_pr_kandev", s.taskPRLinkHandler("replace_task_pr_kandev")), + ) +} + +func taskPRLinkToolOptions(replace bool) []mcp.ToolOption { + options := []mcp.ToolOption{ + mcp.WithDescription("Manage an explicit GitHub PR or GitLab MR association. Provide task_id, provider, canonical repository_id, and pull-request or merge-request number."), + mcp.WithString(mcpKeyTaskID, mcp.Required(), mcp.Description("Target task identifier")), + mcp.WithString("provider", mcp.Required(), mcp.Description("Provider identity: github or gitlab")), + mcp.WithString("repository_id", mcp.Required(), mcp.Description("Canonical task repository identity")), + mcp.WithNumber("number", mcp.Required(), mcp.Description("Pull request or merge request number")), + } + if replace { + options[0] = mcp.WithDescription("Replace an explicit GitHub PR or GitLab MR association. Provide task_id, provider, canonical repository_id, and pull-request or merge-request number. The old and new associations must use the same provider.") + options = append(options, + mcp.WithString("old_provider", mcp.Required(), mcp.Description("Current association provider identity: github or gitlab")), + mcp.WithString("old_repository_id", mcp.Required(), mcp.Description("Current association canonical repository identity")), + mcp.WithNumber("old_number", mcp.Required(), mcp.Description("Current pull request or merge request number")), + ) + } + return options +} + func (s *Server) registerMRAutomationTools() { s.mcpServer.AddTool( mcp.NewToolWithRawSchema("get_task_mr_automation_kandev", diff --git a/apps/backend/internal/mcp/server/server_test.go b/apps/backend/internal/mcp/server/server_test.go index 747db1b038..8554ecf759 100644 --- a/apps/backend/internal/mcp/server/server_test.go +++ b/apps/backend/internal/mcp/server/server_test.go @@ -449,6 +449,9 @@ func TestServerModeTask_RegistersCorrectTools(t *testing.T) { assert.Contains(t, tools, "get_diagnostic_bundle_kandev") assert.Contains(t, tools, "get_task_mr_automation_kandev") assert.Contains(t, tools, "update_task_mr_automation_kandev") + assert.Contains(t, tools, "link_task_pr_kandev") + assert.Contains(t, tools, "unlink_task_pr_kandev") + assert.Contains(t, tools, "replace_task_pr_kandev") // Task mode should have plan tools assert.Contains(t, tools, "create_task_plan_kandev") @@ -657,6 +660,9 @@ func TestServerModeTask_AbsentProvidersFailClosedForReviewAutomation(t *testing. assert.NotContains(t, tools, "update_task_pr_automation_kandev") assert.NotContains(t, tools, "get_task_mr_automation_kandev") assert.NotContains(t, tools, "update_task_mr_automation_kandev") + assert.NotContains(t, tools, "link_task_pr_kandev") + assert.NotContains(t, tools, "unlink_task_pr_kandev") + assert.NotContains(t, tools, "replace_task_pr_kandev") } func TestServerModeTask_ProviderMembership(t *testing.T) { @@ -681,11 +687,15 @@ func TestServerModeTask_ProviderMembership(t *testing.T) { t.Cleanup(backend.Close) s := New(backend, "test-session", "test-task", 10005, log, "", false, ModeTask, tt.providers) tools := getRegisteredToolNames(s) + wantLinkTools := tt.wantPR || tt.wantMR assert.Equal(t, tt.wantPR, containsTool(tools, "get_task_pr_automation_kandev")) assert.Equal(t, tt.wantPR, containsTool(tools, "update_task_pr_automation_kandev")) assert.Equal(t, tt.wantPR, containsTool(tools, "report_pr_auto_fix_outcome_kandev")) assert.Equal(t, tt.wantMR, containsTool(tools, "get_task_mr_automation_kandev")) assert.Equal(t, tt.wantMR, containsTool(tools, "update_task_mr_automation_kandev")) + assert.Equal(t, wantLinkTools, containsTool(tools, "link_task_pr_kandev")) + assert.Equal(t, wantLinkTools, containsTool(tools, "unlink_task_pr_kandev")) + assert.Equal(t, wantLinkTools, containsTool(tools, "replace_task_pr_kandev")) assert.Contains(t, tools, "stop_task_kandev") }) } @@ -704,6 +714,9 @@ func TestServerSetProvidersPreservesModeAndRebuildsTools(t *testing.T) { assert.Contains(t, tools, "set_task_title_kandev") assert.NotContains(t, tools, "get_task_pr_automation_kandev") assert.Contains(t, tools, "get_task_mr_automation_kandev") + assert.Contains(t, tools, "link_task_pr_kandev") + assert.Contains(t, tools, "unlink_task_pr_kandev") + assert.Contains(t, tools, "replace_task_pr_kandev") } type providerRefreshTestSession struct { @@ -755,7 +768,7 @@ drained: // as in TestServerModeTask_ToolCount and // TestRegisterTools_LoggedCountMatchesRegisteredTools (list_task_sessions_test.go), // which pin the per-mode registration rather than this SetProviders rebuild. - require.Len(t, tools, 36, "final registry should contain the complete GitLab-only task tool set") + require.Len(t, tools, 39, "final registry should contain the complete GitLab-only task tool set") assert.Contains(t, tools, "get_task_mr_automation_kandev") assert.NotContains(t, tools, "get_task_pr_automation_kandev") } @@ -924,7 +937,7 @@ func TestServerModeTask_ToolCount(t *testing.T) { // 1 add_workspace_sources + 1 update_repository_base_branch + // 1 step_complete (ADR 0015) + 1 interaction + 4 plan + 3 walkthrough + // 1 publish_review_findings + 1 related-tasks + 1 diagnostic bundle - // + 2 task-dependency (add/remove) + 1 rich-output = 39. + // + 2 task-dependency (add/remove) + 3 task change-link operations + 1 rich-output = 42. // Task-document tools (list/get/write) are office-only. assert.Contains(t, tools, "step_complete_kandev", "ADR 0015 explicit-completion signal must be registered in task mode") assert.Contains(t, tools, "show_walkthrough_kandev", "walkthrough tool must be registered in task mode") @@ -935,7 +948,7 @@ func TestServerModeTask_ToolCount(t *testing.T) { assert.Contains(t, tools, "add_task_dependency_kandev", "dependency edges must be manageable in task mode") assert.Contains(t, tools, "remove_task_dependency_kandev") assert.Contains(t, tools, "show_rich_output_kandev", "native rich output must be registered in task mode") - assert.Equal(t, 39, len(tools)) + assert.Equal(t, 42, len(tools)) } func TestServerStepCompleteTool_TaskAndOfficeOnlyAndDiscoverable(t *testing.T) { diff --git a/apps/backend/internal/task/dto/dto.go b/apps/backend/internal/task/dto/dto.go index c19fbc3583..0e3504db4a 100644 --- a/apps/backend/internal/task/dto/dto.go +++ b/apps/backend/internal/task/dto/dto.go @@ -296,6 +296,9 @@ type TaskDTO struct { // task-listing MCP tools so agents can reason about PR status (e.g. find // tasks whose PRs are merged). Omitted when empty. PRs []v1.TaskPRSummary `json:"prs,omitempty"` + // ChangeRequests lists the provider-neutral GitHub PR and GitLab MR + // associations for this task. Omitted when no associations exist. + ChangeRequests []v1.TaskChangeRequestSummary `json:"change_requests,omitempty"` // StatusSummary is the bounded task-level projection consumed by task rows. // It is loaded in batches and is absent when no projection exists yet; the diff --git a/apps/backend/internal/task/service/handoff_service.go b/apps/backend/internal/task/service/handoff_service.go index 73c7299cc9..d39c7a5f85 100644 --- a/apps/backend/internal/task/service/handoff_service.go +++ b/apps/backend/internal/task/service/handoff_service.go @@ -220,16 +220,17 @@ func (p WorkspacePolicy) NeedsAttachment() bool { // graph itself is the bound: only parent/children/siblings/blockers are ever // projected, so descriptions never leak from unrelated tasks. type RelatedTask struct { - ID string `json:"id"` - Identifier string `json:"identifier,omitempty"` - Title string `json:"title"` - Description string `json:"description,omitempty"` - State string `json:"state"` - WorkspaceID string `json:"workspace_id"` - ParentID string `json:"parent_id,omitempty"` - AssigneeLabel string `json:"assignee_label,omitempty"` - DocumentKeys []string `json:"document_keys,omitempty"` - PRs []v1.TaskPRSummary `json:"prs,omitempty"` + ID string `json:"id"` + Identifier string `json:"identifier,omitempty"` + Title string `json:"title"` + Description string `json:"description,omitempty"` + State string `json:"state"` + WorkspaceID string `json:"workspace_id"` + ParentID string `json:"parent_id,omitempty"` + AssigneeLabel string `json:"assignee_label,omitempty"` + DocumentKeys []string `json:"document_keys,omitempty"` + PRs []v1.TaskPRSummary `json:"prs,omitempty"` + ChangeRequests []v1.TaskChangeRequestSummary `json:"change_requests,omitempty"` } // RelatedTasks bundles every relation surface for a single task. diff --git a/apps/backend/pkg/api/v1/task.go b/apps/backend/pkg/api/v1/task.go index bb57616371..930efa4078 100644 --- a/apps/backend/pkg/api/v1/task.go +++ b/apps/backend/pkg/api/v1/task.go @@ -18,10 +18,28 @@ const ( TaskStateCancelled TaskState = "CANCELLED" ) +// TaskChangeRequestSummary is a compact provider-neutral view of a pull or +// merge request associated with a task. +type TaskChangeRequestSummary struct { + Provider string `json:"provider"` + RepositoryID string `json:"repository_id,omitempty"` + Number int `json:"number"` + URL string `json:"url"` + Title string `json:"title,omitempty"` + State string `json:"state"` + Draft *bool `json:"draft,omitempty"` + BaseRef string `json:"base_ref,omitempty"` + BaseSHA string `json:"base_sha"` + HeadRef string `json:"head_ref,omitempty"` + HeadSHA string `json:"head_sha"` + MergedAt *time.Time `json:"merged_at,omitempty"` + ClosedAt *time.Time `json:"closed_at,omitempty"` +} + // TaskPRSummary is a compact view of a GitHub pull request associated with a -// task. Surfaced through the task-listing MCP tools so agents can reason about -// PR status. State is one of "open", "closed", "merged"; MergedAt is set only -// when the PR has merged, so agents can report when the work landed. +// task. Surfaced through the task-listing MCP tools for compatibility with +// existing agents. State is one of "open", "closed", "merged"; MergedAt is set +// only when the PR has merged, so agents can report when the work landed. type TaskPRSummary struct { Number int `json:"number"` URL string `json:"url"` diff --git a/apps/backend/pkg/websocket/actions.go b/apps/backend/pkg/websocket/actions.go index 857c1fd036..385fbadcf8 100644 --- a/apps/backend/pkg/websocket/actions.go +++ b/apps/backend/pkg/websocket/actions.go @@ -447,6 +447,9 @@ const ( ActionMCPReportPRAutoFixOutcome = "mcp.report_pr_auto_fix_outcome" ActionMCPGetTaskMRAutomation = "mcp.get_task_mr_automation" ActionMCPUpdateTaskMRAutomation = "mcp.update_task_mr_automation" + ActionMCPLinkTaskPR = "mcp.link_task_pr" + ActionMCPUnlinkTaskPR = "mcp.unlink_task_pr" + ActionMCPReplaceTaskPR = "mcp.replace_task_pr" ActionMCPAddTaskDependency = "mcp.add_task_dependency" ActionMCPRemoveTaskDependency = "mcp.remove_task_dependency" ActionMCPAddBranchToTask = "mcp.add_branch_to_task" @@ -590,6 +593,7 @@ const ( // GitLab integration notifications const ( ActionGitLabTaskMRUpdated = "gitlab.task_mr.updated" // Notification + ActionGitLabTaskMRDeleted = "gitlab.task_mr.deleted" // Notification ActionGitLabTaskMRAutomationUpdated = "gitlab.task_mr_options.updated" // Notification ) diff --git a/apps/web/e2e/tests/git/fork-pr-comparison-target.spec.ts b/apps/web/e2e/tests/git/fork-pr-comparison-target.spec.ts index a5c8b9fc1c..bae2afd4fb 100644 --- a/apps/web/e2e/tests/git/fork-pr-comparison-target.spec.ts +++ b/apps/web/e2e/tests/git/fork-pr-comparison-target.spec.ts @@ -88,8 +88,11 @@ test.describe("Fork pull-request comparison target", () => { session.changes.locator('[data-changes-file="local-auth-recovery.txt"]'), ).toBeVisible(); + // Changes is a right-column panel and stays active while the chat session + // tab changes. Leave it explicitly so returning to Changes fires the + // activation refresh that recovers the comparison target. + await session.clickTab("Files"); comparisonTargetFixture?.setAvailable(true); - await session.clickSessionChatTab(); await session.clickTab("Changes"); await expect(testPage.getByTestId("comparison-target-notice")).toHaveCount(0, { timeout: 30_000, diff --git a/apps/web/lib/types/backend.ts b/apps/web/lib/types/backend.ts index dec42d45e8..f28c6f2731 100644 --- a/apps/web/lib/types/backend.ts +++ b/apps/web/lib/types/backend.ts @@ -42,9 +42,8 @@ import type { TaskPR, TaskPRDeletedEvent, } from "@/lib/types/github"; -import type { TaskMR } from "@/lib/types/gitlab"; +import type { TaskMR, TaskMRDeletedEvent, TaskMRAutomationOptions } from "@/lib/types/gitlab"; import type { TaskStatusSummary } from "@/lib/types/task-status-summary"; -import type { TaskMRAutomationOptions } from "@/lib/types/gitlab"; import type { AgentProfileRecentUseApiRecord } from "@/lib/types/http-agent-profile-recent-use"; import type { SystemMetricsSnapshot, StorageAnalysisUpdatedPayload } from "./system"; import type { AgentRuntimeAvailability } from "./agent-runtime"; @@ -581,6 +580,7 @@ export type BackendMessageMap = SessionBackendMessageMap & "gitlab.task_mr.updated", TaskMR & { workspace_id: string } >; + "gitlab.task_mr.deleted": BackendMessage<"gitlab.task_mr.deleted", TaskMRDeletedEvent>; "gitlab.task_mr_options.updated": BackendMessage< "gitlab.task_mr_options.updated", TaskMRAutomationOptions diff --git a/apps/web/lib/types/gitlab.ts b/apps/web/lib/types/gitlab.ts index 34bcca1275..152c3e3276 100644 --- a/apps/web/lib/types/gitlab.ts +++ b/apps/web/lib/types/gitlab.ts @@ -94,6 +94,13 @@ export type TaskMR = { updated_at: string; }; +/** Workspace-scoped websocket payload emitted when a task MR association is removed. */ +export type TaskMRDeletedEvent = { + workspace_id: string; + task_id: string; + association_id: string; +}; + /** Response shape for `GET /api/v1/gitlab/workspaces/:id/task-mrs`. */ export type TaskMRsResponse = { task_mrs: Record; diff --git a/apps/web/lib/ws/handlers/gitlab.test.ts b/apps/web/lib/ws/handlers/gitlab.test.ts index ce805108fb..7544004f8d 100644 --- a/apps/web/lib/ws/handlers/gitlab.test.ts +++ b/apps/web/lib/ws/handlers/gitlab.test.ts @@ -8,18 +8,21 @@ const WORKSPACE_A = "workspace-a"; function makeStore(activeWorkspaceId: string | null, options?: TaskMRAutomationOptions) { const setTaskMR = vi.fn(); + const removeTaskMR = vi.fn(); const setTaskMRAutomationOptions = vi.fn(); const markTaskMRAutomationExternalUpdate = vi.fn(); const state = { workspaces: { activeId: activeWorkspaceId }, taskMRAutomation: { byTaskId: options ? { [options.task_id]: options } : {} }, setTaskMR, + removeTaskMR, setTaskMRAutomationOptions, markTaskMRAutomationExternalUpdate, } as unknown as AppState; return { store: { getState: () => state } as StoreApi, setTaskMR, + removeTaskMR, setTaskMRAutomationOptions, markTaskMRAutomationExternalUpdate, }; @@ -68,6 +71,36 @@ describe("GitLab WebSocket handlers", () => { expect(setTaskMR).toHaveBeenCalledWith(WORKSPACE_A, "task-1", mr); }); + it("removes a task MR for the active workspace", () => { + const { store, removeTaskMR } = makeStore(WORKSPACE_A); + const handler = (registerGitLabHandlers(store) as Record void>)[ + "gitlab.task_mr.deleted" + ]!; + + handler({ + type: "notification", + action: "gitlab.task_mr.deleted", + payload: { workspace_id: WORKSPACE_A, task_id: "task-1", association_id: "mr-1" }, + } as never); + + expect(removeTaskMR).toHaveBeenCalledWith(WORKSPACE_A, "mr-1"); + }); + + it("ignores task MR deletion from another workspace", () => { + const { store, removeTaskMR } = makeStore("workspace-b"); + const handler = (registerGitLabHandlers(store) as Record void>)[ + "gitlab.task_mr.deleted" + ]!; + + handler({ + type: "notification", + action: "gitlab.task_mr.deleted", + payload: { workspace_id: WORKSPACE_A, task_id: "task-1", association_id: "mr-1" }, + } as never); + + expect(removeTaskMR).not.toHaveBeenCalled(); + }); + it("ignores task MRs from another workspace", () => { const { store, setTaskMR } = makeStore("workspace-b"); const handler = registerGitLabHandlers(store)["gitlab.task_mr.updated"]!; diff --git a/apps/web/lib/ws/handlers/gitlab.ts b/apps/web/lib/ws/handlers/gitlab.ts index 255651fe97..512a21d4ce 100644 --- a/apps/web/lib/ws/handlers/gitlab.ts +++ b/apps/web/lib/ws/handlers/gitlab.ts @@ -11,6 +11,19 @@ export function registerGitLabHandlers(store: StoreApi): WsHandlers { if (!mr.task_id || !mr.workspace_id || mr.workspace_id !== activeWorkspaceId) return; store.getState().setTaskMR(mr.workspace_id, mr.task_id, mr); }, + "gitlab.task_mr.deleted": (message) => { + const deleted = message.payload; + const activeWorkspaceId = store.getState().workspaces.activeId; + if ( + !deleted.task_id || + !deleted.association_id || + !deleted.workspace_id || + deleted.workspace_id !== activeWorkspaceId + ) { + return; + } + store.getState().removeTaskMR(deleted.workspace_id, deleted.association_id); + }, "gitlab.task_mr_options.updated": (message) => { const options = message.payload as TaskMRAutomationOptions; if (options.task_id) { diff --git a/docs/plans/manage-task-pr-and-m/plan.md b/docs/plans/manage-task-pr-and-m/plan.md new file mode 100644 index 0000000000..52ae2aeb83 --- /dev/null +++ b/docs/plans/manage-task-pr-and-m/plan.md @@ -0,0 +1,49 @@ +--- +status: implemented +requirements: + - REQ-INTEGRATIONS-TASK-CHANGE-LINK-MCP-001 +system_design: + - ../../specs/integrations/system-design/task-change-link-mcp.md +created: 2026-09-14 +--- + +# Implementation Plan: Manage Task Pull Requests and Merge Requests + +## Overview + +Expose one provider-neutral MCP link contract, +`link_task_pr_kandev` / `unlink_task_pr_kandev` / `replace_task_pr_kandev`, +that manages an existing task's GitHub pull request and GitLab merge request +associations. The backend routes all three operations through one coordinator +so validation, workspace authorization, and store dispatch stay identical for +both providers, and each mutation returns the resulting canonical linked set. + +## Delivery packages + +- [Task 01: Provider-neutral task change-request link + coordinator](task-01-coordinator.md) + - Backend coordinator, MCP handler and server registration, provider service + entry points, and GitLab deletion-event publication. This package is the + only work order: the merged diff is backend-only and public documentation + was part of the same delivery. + +## Requirements and design + +- Requirements: `docs/specs/integrations/requirements/task-change-link-mcp.md`. +- System design: `docs/specs/integrations/system-design/task-change-link-mcp.md`. + +## Risks + +- Fork and canonical repositories can share a number; identity is anchored to + the canonical repository identity, not the number. +- Legacy repositories may have an empty provider host; link operations fail + closed instead of guessing `github.com`. +- Replacement failure must not strand two active associations; compensation is + reported when it also fails. + +## Verification strategy + +Focused Go suites for the coordinator, MCP handler/server, GitHub, and GitLab +packages, plus race-enabled focused tests for identity, authorization, +idempotency, rollback, detach, and projection behavior. Public documentation +validation via `Validate public docs`. diff --git a/docs/plans/manage-task-pr-and-m/task-01-coordinator.md b/docs/plans/manage-task-pr-and-m/task-01-coordinator.md new file mode 100644 index 0000000000..d36f82c18f --- /dev/null +++ b/docs/plans/manage-task-pr-and-m/task-01-coordinator.md @@ -0,0 +1,65 @@ +--- +id: "01-coordinator" +title: "Provider-neutral task change-request link coordinator" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +requirements: + - REQ-INTEGRATIONS-TASK-CHANGE-LINK-MCP-001 +acceptance_criteria: + - AC-INTEGRATIONS-TASK-CHANGE-LINK-MCP-001.1 + - AC-INTEGRATIONS-TASK-CHANGE-LINK-MCP-001.2 + - AC-INTEGRATIONS-TASK-CHANGE-LINK-MCP-001.3 + - AC-INTEGRATIONS-TASK-CHANGE-LINK-MCP-001.4 + - AC-INTEGRATIONS-TASK-CHANGE-LINK-MCP-001.5 + - AC-INTEGRATIONS-TASK-CHANGE-LINK-MCP-001.6 +system_design: + - ../../specs/integrations/system-design/task-change-link-mcp.md +--- + +# Task 01: Provider-neutral task change-request link coordinator + +## Summary + +Add the backend MCP link contract for GitHub pull requests and GitLab merge +requests behind one provider-neutral coordinator: request validation, +workspace-scoped resolution, provider dispatch, and the resulting canonical +linked set returned to the caller. + +## In scope + +- `apps/backend/internal/mcp/handlers/task_change_link.go` and its tests. +- `apps/backend/internal/backendapp/task_change_link_coordinator.go` and its + tests. +- Provider service entry points used by the coordinator in + `apps/backend/internal/github` and `apps/backend/internal/gitlab`. +- GitLab unlink deletion-event publication consumed by + `apps/backend/pkg/websocket/actions.go`. + +## Out of scope + +- Frontend rendering of the linked set beyond existing provider stores. +- Automation redesign or workflow moves. +- Direct database editing outside product migrations. + +## Acceptance + +- A bare pull-request or merge-request number is rejected before any store + mutation, and fork versus canonical same-number identities stay distinct. +- A failed replacement removes the newly created association and reports both + failures without leaving two active links. +- Unlinks emit the provider deletion event after persistence succeeds. + +## Verification + +```bash +cd apps/backend && go test ./internal/mcp/handlers ./internal/mcp/server ./internal/backendapp ./internal/github ./internal/gitlab ./internal/task/dto ./pkg/api/v1 +``` + +## Results + +Implemented and verified on branch `feature/manage-task-pr-and-m-e6i` at +`b1ada7b95dd471752799fbbe78ed6ce8b2794564`. Focused coordinator, handler, +server, GitHub, and GitLab suites passed, including race-enabled focused +identity, authorization, idempotency, rollback, detach, and projection tests. diff --git a/docs/public/automation-and-mcp.md b/docs/public/automation-and-mcp.md index 06db4326b8..1a0ff0eea0 100644 --- a/docs/public/automation-and-mcp.md +++ b/docs/public/automation-and-mcp.md @@ -232,6 +232,29 @@ The same data is available directly over REST for scripting: `GET /api/v1/worksp Kandev automatically injects a task-aware MCP server into supported agent sessions. You do not need to add it to the profile. It lets the active agent use current IDs and structured operations instead of inferring board state from text. +### Link an existing pull or merge request + +Task MCP provides `link_task_pr_kandev`, `unlink_task_pr_kandev`, and +`replace_task_pr_kandev` for GitHub pull requests and GitLab merge requests. +Each request needs `task_id`, `provider` (`github` or `gitlab`), the canonical +`repository_id`, and a positive request number. A number by itself is rejected, +so a fork and its canonical repository can safely have the same number. + +For example, link GitLab merge request 42 to its target task with +`{ "task_id": "…", "provider": "gitlab", "repository_id": "…", "number": 42 }`. +`replace_task_pr_kandev` also requires `old_provider`, `old_repository_id`, and +`old_number`. Replacement supports associations from the same provider only; +to switch between GitHub and GitLab, unlink the current association and then +link the new one. Every successful mutation returns the resulting active link set. +The target task must be reachable from the calling task's workspace. +`list_tasks_kandev` and `list_related_tasks_kandev` expose active GitHub PR and +GitLab MR associations in the provider-neutral `change_requests` field; the +legacy `prs` field remains GitHub-only for compatibility. + +Unlinking changes only the active association and its matching automation +state. It does not delete conversation history, terminal receipts, or the +upstream pull request or merge request. + Names ending in `_kandev` are the canonical MCP protocol tool names. Some agent clients show or register a server-qualified alias instead. For example, a client may expose canonical `step_complete_kandev` as `mcp__kandev__step_complete_kandev`. That qualified form is client-specific, not a second tool or a universal name; use the form exposed by the active client. Task tools use normal client discovery. When `step_complete_kandev` is required but is not already visible, the agent should search the active tool catalog for its canonical name. Kandev does not request eager loading through client-specific metadata. diff --git a/docs/public/coverage.json b/docs/public/coverage.json index eea06808f2..439be4d5a9 100644 --- a/docs/public/coverage.json +++ b/docs/public/coverage.json @@ -444,7 +444,9 @@ "apps/backend/internal/github/app_registration_lifecycle_service.go", "apps/backend/internal/github/app_registration_import.go", "apps/web/lib/api/domains/github-auth-api.ts", - "apps/backend/internal/mcp/server/server.go" + "apps/backend/internal/mcp/server/server.go", + "apps/backend/internal/mcp/handlers/task_change_link.go", + "apps/backend/internal/backendapp/task_change_link_coordinator.go" ], "tests": [ "apps/web/components/app-sidebar/sections/integrations-section.test.tsx", @@ -454,7 +456,9 @@ "apps/backend/internal/github/controller_app_registration_test.go", "apps/backend/internal/github/app_registration_import_test.go", "apps/web/e2e/tests/settings/github-app-registration.spec.ts", - "apps/backend/internal/mcp/server/handlers_test.go" + "apps/backend/internal/mcp/server/handlers_test.go", + "apps/backend/internal/mcp/handlers/task_change_link_test.go", + "apps/backend/internal/backendapp/task_change_link_coordinator_test.go" ], "settingsRoutes": [ "/settings/integrations", @@ -468,6 +472,9 @@ "mcpTools": [ "get_task_mr_automation_kandev", "get_task_pr_automation_kandev", + "link_task_pr_kandev", + "replace_task_pr_kandev", + "unlink_task_pr_kandev", "report_pr_auto_fix_outcome_kandev", "update_task_mr_automation_kandev", "update_task_pr_automation_kandev" diff --git a/docs/specs/integrations/requirements/task-change-link-mcp.md b/docs/specs/integrations/requirements/task-change-link-mcp.md new file mode 100644 index 0000000000..2247bb73c3 --- /dev/null +++ b/docs/specs/integrations/requirements/task-change-link-mcp.md @@ -0,0 +1,79 @@ +--- +status: active +system: integrations +created: 2026-09-14 +owners: + - kandev +--- + +# Manage Task Pull Requests and Merge Requests Requirements + +## Overview + +Agents coordinating task delivery need to attach, swap, or remove the external +change request (a GitHub pull request or a GitLab merge request) that carries +the task's delivery after a task already exists. The MCP surface exposes three +provider-neutral operations so an agent can state intent without learning +provider-specific store mechanics, and every mutation answers with the +resulting canonical linked set so the caller can verify immediately. + +This requirement is owned by the integration system because the contract +spans both provider stacks and one shared task-facing tool surface, while the +task system owns the task identity and the workspace system owns repository +attachment. + +## Terminology + +- **Change request (CR):** A GitHub pull request or a GitLab merge request. +- **Canonical repository identity:** The persisted repository identity + (`repository_id`, resolved through the task's workspace) that distinguishes + a fork from the canonical repository even when both contain the same + change-request number. +- **Active association:** The stored link between the task and one + provider-specific change request; historical receipts (terminal transcripts, + conversation history) are unaffected by unlinking. + +## Requirements + +### REQ-INTEGRATIONS-TASK-CHANGE-LINK-MCP-001: Manage task change requests through MCP + +**Intent:** Give agents safe, verifiable control over the external delivery +record of an existing task. + +**User story:** As an agent coordinating a task, I want to link, unlink, or +replace a GitHub pull request or GitLab merge request on that task through +MCP, so that the task's delivery trail points at the real change request +without me editing task records by hand. + +#### Acceptance criteria + +- **AC-INTEGRATIONS-TASK-CHANGE-LINK-MCP-001.1:** When a mutation request is + authorized for the calling task's workspace and supplies the task ID, + provider (`github` or `gitlab`), canonical repository identity, and the + change-request number, the system applies the link, unlink, or replacement + and returns the resulting canonical linked set in the same response. + A bare number without repository identity is rejected, and same-number + change requests in a fork and a canonical repository remain distinct + associations. +- **AC-INTEGRATIONS-TASK-CHANGE-LINK-MCP-001.2:** When a caller requests an + operation for a task outside its reachable workspace set, the system + rejects the request without mutating any store. +- **AC-INTEGRATIONS-TASK-CHANGE-LINK-MCP-001.3:** When a link targets an + already-active association, the system succeeds without duplicating it; + when an unlink targets an absent association, the system succeeds. + Unlink removes only automation and settings scoped to that exact + provider, repository, and number, and leaves other linked change requests + and task metadata untouched. +- **AC-INTEGRATIONS-TASK-CHANGE-LINK-MCP-001.4:** When a replacement's new + target cannot be established, the previous association remains active; when + the established new association also cannot be removed during rollback, the + response reports both failures so the caller can reconcile the partial + state. +- **AC-INTEGRATIONS-TASK-CHANGE-LINK-MCP-001.5:** When a repository cannot be + resolved to a verified provider host (including a persisted identity with + no provider host), link operations fail closed instead of assuming a + default provider host. +- **AC-INTEGRATIONS-TASK-CHANGE-LINK-MCP-001.6:** When a mutation succeeds, + query surfaces and connected clients observe the new linked state after a + restart, and unlinking changes only the active association, never + conversation or terminal-receipt history. diff --git a/docs/specs/integrations/system-design/task-change-link-mcp.md b/docs/specs/integrations/system-design/task-change-link-mcp.md new file mode 100644 index 0000000000..9ecdf40fcf --- /dev/null +++ b/docs/specs/integrations/system-design/task-change-link-mcp.md @@ -0,0 +1,91 @@ +--- +status: current +system: integrations +requirements: + - REQ-INTEGRATIONS-TASK-CHANGE-LINK-MCP-001 +created: 2026-09-14 +owners: + - kandev +--- + +# Manage task change requests through MCP System Design + +## Purpose and boundaries + +This design gives agents one provider-neutral MCP surface for managing a +task's GitHub pull request and GitLab merge request associations. The MCP +tool surface and the task-facing handler are owned here; provider stores, +webhook discovery, and automation handling stay owned by their existing +GitHub and GitLab designs, and task identity, workspace reach authorization, +and list/serialization surfaces stay owned by the task system. + +## Requirement mapping + +| Requirement | Design section | +| --- | --- | +| `REQ-INTEGRATIONS-TASK-CHANGE-LINK-MCP-001` | [Components and responsibilities](#components-and-responsibilities), [Control flow](#control-flow) | + +## Components and responsibilities + +- `apps/backend/internal/mcp/handlers/task_change_link.go` validates the + request shape, binds the caller task identity through the ordinary task + reach path, and forwards one provider-neutral request to the coordinator. +- `apps/backend/internal/backendapp/task_change_link_coordinator.go` + resolves the canonical repository from the task's workspace, rejects + unknown or unverified provider hosts, dispatches to the provider service, + and returns the resulting linked set. Replacement compensates a failed + post-establishment exchange by removing the newly created association and + reports both errors when compensation itself fails. +- `apps/backend/internal/github` and `apps/backend/internal/gitlab` own + association persistence, idempotent link/removal, and exact automation + retirement scoped to one provider, repository, and change-request number. + GitHub detach keeps durable tombstones; GitLab unlink publishes a + workspace-scoped deletion event after persistence. +- `apps/backend/internal/mcp/server/server.go` registers + `link_task_pr_kandev`, `unlink_task_pr_kandev`, and + `replace_task_pr_kandev` with provider-aware availability. + +## Data and contracts + +Every mutation requires explicit `task_id`, a provider discriminator +(`github` or `gitlab`), the canonical repository identity (`repository_id`), +and the change-request number. A bare number is invalid. Replacement adds the +old association triple (`old_provider`, `old_repository_id`, +`old_number`). Responses return the current canonical linked set for the +task. + +Unlinking changes only the active association row. Conversation history and +terminal receipts remain untouched, and re-linking the same identity restores +an active association. + +## Control flow + +1. The MCP server receives the tool call and resolves the caller task. +2. The handler authorizes workspace reach for that task. +3. The handler validates provider, repository identity, and number, rejecting + a bare number or unresolved repository before any store mutation. +4. The coordinator loads the task's repositories, verifies the requested + canonical repository belongs to the task workspace and carries a verified + provider host, and dispatches link, unlink, or replace. +5. The provider service applies idempotent persistence, scoped automation + cleanup, and deletion-event publication. +6. The coordinator assembles the resulting linked set and the handler returns + it. List/query surfaces read the same stores, so no extra invalidation is + required for restart-consistent state. + +## Examples + +```json +{ + "task_id": "", + "provider": "github", + "repository_id": "", + "number": 3506 +} +``` + +`link_task_pr_kandev` and `unlink_task_pr_kandev` accept this shape; +`replace_task_pr_kandev` additionally requires the old association triple +(`old_provider`, `old_repository_id`, `old_number`). See +[docs/public/automation-and-mcp.md](../../../public/automation-and-mcp.md) +for the operator-facing contract.