From a852be3c330cfe0347830e13c36c5decfc4d5d1e Mon Sep 17 00:00:00 2001 From: Luan Martins Date: Wed, 20 May 2026 22:38:44 +0100 Subject: [PATCH] feat(web): show task linkage on GitHub PR list (#952) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(web): show task linkage on GitHub PR list Add a compact indicator next to each PR showing whether a task already exists. When tasks are linked, show a clickable button with the task title and a count badge for multiple tasks. When no task exists, show 'No task created yet'. - Add usePRKeyToTasks hook that maps PR keys to task associations - Add PRRowTaskIndicator component for the per-PR task UI - Wire the data through github-page-client into PRList - Add tests for the indicator and hook utility * feat(web): improve PR task indicator with task titles and multi-task dropdown - Show actual task title instead of PR title in single-task case - Replace count badge with dropdown menu for multiple linked tasks - Switch icon from GitPullRequest to Checklist - Load all workflow snapshots on GitHub page to hydrate task titles - Fix test to use unique task_id per fixture * feat(web): neutralize PR task indicator color and add tooltip The checklist icon previously mirrored PR aggregate status, which duplicated information already shown on the PR row and misleadingly suggested it reflected task state. Drop the color and add a tooltip surfacing the actual task title and workflow step. Adds e2e coverage for the GitHub PR list task indicator: empty badge, single-task button, and tooltip content. * fix(web): navigate to task page when clicking PR task indicator replaceTaskUrl only mutates history.state, which sets active task in the store but does not trigger a Next.js route change. From /github, this left the URL pointing to /t/ while the page still rendered the GitHub list. Use router.push so the click actually navigates to the task. * test(web): cover PR task indicator click navigation Extends the GitHub PR list task indicator spec with a click assertion verifying the URL changes to /t/ and the PR list unmounts. Regression guard for the replaceTaskUrl → router.push fix. * fix(github): link task to PR at creation time, not via branch poller Tasks started from the GitHub PR list "+ Task" button were left without a github_task_prs row when the worktree branch did not match the PR's head branch on GitHub. This happened for PR-review-style tasks, which check out synthetic branches (e.g. feature/review-...-h92) that don't exist as PR heads. The poller's FindPRByBranch could never resolve them, so only the first task whose branch happened to match got linked — every other task created from the same PR was missing from the indicator. The PR is already known at "+ Task" time, so persist the linkage immediately rather than waiting on branch-based discovery: - AssociateExistingPRByURL: new service method that creates the github_task_prs row from a PR URL without creating a PR watch. The watch is still created later at agent start via ensureSessionPRWatch. - POST /api/v1/github/task-prs/associate: HTTP endpoint exposing it. - QuickTaskLauncher.handleSuccess: after task creation, fires a best-effort associate call with the task's repository_id and PR URL. Failures fall back to the legacy poller path silently. Backend test covers two distinct tasks linking to the same PR. E2e adds a PR with two tasks and asserts the multi-task indicator ("Tasks 2") renders alongside the single-task and empty cases. * refactor(github): make task-PR create endpoint RESTful Rename POST /api/v1/github/task-prs/associate to POST /api/v1/github/task-prs. The action-named "/associate" was RPC-styled; the resource collection is the right URL for creation alongside the existing GET. Frontend client renamed associateTaskPR → createTaskPR to match. * fix(github): address PR review findings - Map invalid PR URL to HTTP 400 (was 500). New ErrInvalidPRURL sentinel lets the controller distinguish parse failures from genuine server errors. Other failure paths still return 500. - Prefer dialog-time repositoryId in pickRepositoryIdForPR. Multi-repo tasks with shared branch names were ambiguous under pure branch matching; the matched repo id captured by the launcher dialog wins when present. - Cover usePRKeyToTasks (not only the prKey util) with hook tests: empty input, multi-task grouping, distinct keys, and the non-array defensive branch. - Lock createTaskPR request semantics: method/body now win over a caller's options.init so the POST JSON shape can't be overridden. * fix(web): use TaskPR row id for dropdown key Multi-repo tasks can produce more than one TaskPR row for the same PR (one row per repository_id), so keying the multi-task dropdown by task_id risked a duplicate-key warning and unstable reconciliation. Switch to the row id, which is unique per row. --- apps/backend/internal/github/controller.go | 32 ++++ apps/backend/internal/github/service.go | 34 +++++ .../github/service_associate_existing_test.go | 57 +++++++ apps/package.json | 8 +- apps/web/app/github/github-page-client.tsx | 10 +- .../components/github/my-github/pr-list.tsx | 10 +- .../my-github/pr-row-task-indicator.test.tsx | 91 +++++++++++ .../my-github/pr-row-task-indicator.tsx | 144 ++++++++++++++++++ .../github/my-github/quick-task-launcher.tsx | 43 +++++- .../github/pr-list-task-indicator.spec.ts | 130 ++++++++++++++++ .../github/use-pr-key-to-tasks.test.ts | 141 +++++++++++++++++ .../domains/github/use-pr-key-to-tasks.ts | 32 ++++ apps/web/lib/api/domains/github-api.ts | 14 ++ 13 files changed, 741 insertions(+), 5 deletions(-) create mode 100644 apps/backend/internal/github/service_associate_existing_test.go create mode 100644 apps/web/components/github/my-github/pr-row-task-indicator.test.tsx create mode 100644 apps/web/components/github/my-github/pr-row-task-indicator.tsx create mode 100644 apps/web/e2e/tests/github/pr-list-task-indicator.spec.ts create mode 100644 apps/web/hooks/domains/github/use-pr-key-to-tasks.test.ts create mode 100644 apps/web/hooks/domains/github/use-pr-key-to-tasks.ts diff --git a/apps/backend/internal/github/controller.go b/apps/backend/internal/github/controller.go index e21f3c1de7..ebcb85bd9d 100644 --- a/apps/backend/internal/github/controller.go +++ b/apps/backend/internal/github/controller.go @@ -32,6 +32,7 @@ func (c *Controller) RegisterHTTPRoutes(router *gin.Engine) { api.DELETE("/token", c.httpClearToken) api.GET("/task-prs", c.httpListTaskPRs) + api.POST("/task-prs", c.httpCreateTaskPR) api.GET("/task-prs/:taskId", c.httpGetTaskPR) api.GET("/prs/:owner/:repo/:number", c.httpGetPRFeedback) @@ -136,6 +137,37 @@ func (c *Controller) httpListTaskPRs(ctx *gin.Context) { ctx.JSON(http.StatusOK, gin.H{"task_prs": result}) } +// httpCreateTaskPR creates a task-PR association from a known PR URL. Used +// by the GitHub PR list "+ Task" flow: the PR is already known, so we +// persist the linkage immediately instead of waiting for branch-based +// discovery (which fails for review tasks that use synthetic worktree +// branches that don't exist on GitHub). +func (c *Controller) httpCreateTaskPR(ctx *gin.Context) { + var req struct { + TaskID string `json:"task_id"` + RepositoryID string `json:"repository_id"` + PRURL string `json:"pr_url"` + } + if err := ctx.ShouldBindJSON(&req); err != nil { + ctx.JSON(http.StatusBadRequest, gin.H{"error": "invalid payload"}) + return + } + if req.TaskID == "" || req.PRURL == "" { + ctx.JSON(http.StatusBadRequest, gin.H{"error": "task_id and pr_url are required"}) + return + } + tp, err := c.service.AssociateExistingPRByURL(ctx.Request.Context(), req.TaskID, req.RepositoryID, req.PRURL) + if err != nil { + status := http.StatusInternalServerError + if errors.Is(err, ErrInvalidPRURL) { + status = http.StatusBadRequest + } + ctx.JSON(status, gin.H{"error": err.Error()}) + return + } + ctx.JSON(http.StatusOK, tp) +} + func (c *Controller) httpGetTaskPR(ctx *gin.Context) { taskID := ctx.Param("taskId") tp, err := c.service.GetTaskPR(ctx.Request.Context(), taskID) diff --git a/apps/backend/internal/github/service.go b/apps/backend/internal/github/service.go index c357cb7e53..09dabda50b 100644 --- a/apps/backend/internal/github/service.go +++ b/apps/backend/internal/github/service.go @@ -28,6 +28,11 @@ const ( AuthMethodPAT = "pat" ) +// ErrInvalidPRURL signals that a caller-supplied PR URL could not be parsed. +// Used by AssociateExistingPRByURL so HTTP callers can translate the failure +// into a 400 instead of a generic 500. +var ErrInvalidPRURL = errors.New("invalid PR URL") + // defaultBranchMain and defaultBranchMaster are the conventional default branch // names sorted to the top of branch pickers. const ( @@ -596,6 +601,35 @@ func (s *Service) AssociatePRWithTask(ctx context.Context, taskID, repositoryID return tp, nil } +// AssociateExistingPRByURL parses a GitHub PR URL, fetches the PR data, and +// associates it with the given task. No PR watch is created — this is used +// when the caller already knows the PR (e.g. user clicked "+ Task" on a PR +// in the GitHub page), so branch-based discovery is unnecessary. The watch +// for ongoing status sync is still created later when the agent session +// starts (see ensureSessionPRWatch). +// +// Returns the persisted TaskPR row so callers can confirm the association +// and react to errors synchronously, in contrast to AssociatePRByURL's +// fire-and-forget logging. +func (s *Service) AssociateExistingPRByURL(ctx context.Context, taskID, repositoryID, prURL string) (*TaskPR, error) { + if s.client == nil { + return nil, fmt.Errorf("github client not available") + } + owner, repo, prNumber, err := parsePRURL(prURL) + if err != nil { + return nil, fmt.Errorf("%w: %v", ErrInvalidPRURL, err) + } + pr, err := s.client.GetPR(ctx, owner, repo, prNumber) + if err != nil { + return nil, fmt.Errorf("fetch PR: %w", err) + } + tp, err := s.AssociatePRWithTask(ctx, taskID, repositoryID, pr) + if err != nil { + return nil, fmt.Errorf("associate PR with task: %w", err) + } + return tp, nil +} + // AssociatePRByURL parses a GitHub PR URL, fetches the PR data, creates a PR // watch, and associates it with the given task. Called after the user // creates a PR from the UI. `repositoryID` scopes the watch + association to diff --git a/apps/backend/internal/github/service_associate_existing_test.go b/apps/backend/internal/github/service_associate_existing_test.go new file mode 100644 index 0000000000..2df019f997 --- /dev/null +++ b/apps/backend/internal/github/service_associate_existing_test.go @@ -0,0 +1,57 @@ +package github + +import ( + "context" + "testing" +) + +// TestAssociateExistingPRByURL_CreatesTwoRowsForDifferentTasks regression-tests +// the bug where two tasks created from the same PR via the GitHub page "+ +// Task" launcher ended up with only one github_task_prs row. The launcher +// now calls AssociateExistingPRByURL directly so the linkage no longer +// depends on branch-based discovery (which fails for review tasks that use +// synthetic worktree branches). +func TestAssociateExistingPRByURL_CreatesTwoRowsForDifferentTasks(t *testing.T) { + _, svc, mockClient, _ := setupPollerTest(t) + ctx := context.Background() + + mockClient.AddPR(&PR{ + Number: 42, + Title: "Feature PR", + State: "open", + HeadSHA: "abc", + HeadBranch: "feat/x", + RepoOwner: "org", + RepoName: "repo", + HTMLURL: "https://github.com/org/repo/pull/42", + }) + + prURL := "https://github.com/org/repo/pull/42" + tp1, err := svc.AssociateExistingPRByURL(ctx, "task-A", "repo-1", prURL) + if err != nil { + t.Fatalf("first associate: %v", err) + } + if tp1 == nil || tp1.TaskID != "task-A" || tp1.PRNumber != 42 { + t.Fatalf("unexpected first TaskPR: %+v", tp1) + } + + tp2, err := svc.AssociateExistingPRByURL(ctx, "task-B", "repo-1", prURL) + if err != nil { + t.Fatalf("second associate: %v", err) + } + if tp2 == nil || tp2.TaskID != "task-B" || tp2.PRNumber != 42 { + t.Fatalf("unexpected second TaskPR: %+v", tp2) + } + if tp1.ID == tp2.ID { + t.Fatalf("expected distinct rows for distinct tasks, got duplicate id %s", tp1.ID) + } +} + +func TestAssociateExistingPRByURL_RejectsBadURL(t *testing.T) { + _, svc, _, _ := setupPollerTest(t) + ctx := context.Background() + + if _, err := svc.AssociateExistingPRByURL(ctx, "t", "r", "not-a-pr-url"); err == nil { + t.Fatal("expected error for malformed PR URL") + } +} diff --git a/apps/package.json b/apps/package.json index a9b80c55d3..50e41fef66 100644 --- a/apps/package.json +++ b/apps/package.json @@ -27,6 +27,12 @@ "overrides": { "@codemirror/state": "6.5.3", "@codemirror/view": "6.39.10" - } + }, + "onlyBuiltDependencies": [ + "esbuild", + "msw", + "sharp", + "unrs-resolver" + ] } } diff --git a/apps/web/app/github/github-page-client.tsx b/apps/web/app/github/github-page-client.tsx index b2fec59c74..649c45916c 100644 --- a/apps/web/app/github/github-page-client.tsx +++ b/apps/web/app/github/github-page-client.tsx @@ -6,8 +6,9 @@ import { IconBrandGithub } from "@tabler/icons-react"; import { Alert, AlertDescription } from "@kandev/ui/alert"; import { PageTopbar } from "@/components/page-topbar"; import { useGitHubStatus } from "@/hooks/domains/github/use-github-status"; +import { usePRKeyToTasks } from "@/hooks/domains/github/use-pr-key-to-tasks"; import type { Repository, Workflow, WorkflowStep } from "@/lib/types/http"; -import type { GitHubIssue, GitHubPR } from "@/lib/types/github"; +import type { GitHubIssue, GitHubPR, TaskPR } from "@/lib/types/github"; import { PRList } from "@/components/github/my-github/pr-list"; import { IssueList } from "@/components/github/my-github/issue-list"; import { @@ -40,6 +41,7 @@ import { resolveIssuePresets, } from "@/components/github/my-github/action-presets"; import { useGitHubActionPresets } from "@/hooks/domains/github/use-github-action-presets"; +import { useAllWorkflowSnapshots } from "@/hooks/domains/kanban/use-all-workflow-snapshots"; type GitHubPageClientProps = { workspaceId?: string; @@ -109,6 +111,7 @@ function ResultsList({ prPresets, issuePresets, onStartTask, + prKeyToTasks, }: { selection: SidebarSelection; items: Array; @@ -117,6 +120,7 @@ function ResultsList({ prPresets: TaskPreset[]; issuePresets: TaskPreset[]; onStartTask: (payload: LaunchPayload) => void; + prKeyToTasks: Map; }) { if (selection.kind === "pr") { return ( @@ -126,6 +130,7 @@ function ResultsList({ error={error} presets={prPresets} onStartTask={onStartTask} + prKeyToTasks={prKeyToTasks} /> ); } @@ -286,6 +291,8 @@ function AuthenticatedLayout({ onStartTask: (payload: LaunchPayload) => void; }) { const { selection, search, repoOptions, title } = state; + const prKeyToTasks = usePRKeyToTasks(workspaceId ?? null); + useAllWorkflowSnapshots(workspaceId ?? null); return (
void; + prKeyToTasks?: Map; }; // Prefer the enriched PR returned by the batched status endpoint — the search @@ -100,11 +102,13 @@ function PRRow({ status, presets, onStartTask, + tasks, }: { pr: GitHubPR; status: GitHubPRStatus | null | undefined; presets: TaskPreset[]; onStartTask: PRListProps["onStartTask"]; + tasks: TaskPR[] | undefined; }) { const { Icon: StateIcon, className: stateIconClass } = prStateIcon(pr); return ( @@ -132,6 +136,7 @@ function PRRow({ by {pr.author_login} · opened {formatRelativeTime(pr.created_at)} +
@@ -145,7 +150,7 @@ function PRRow({ ); } -function PRListBody({ loading, error, items, presets, onStartTask }: PRListProps) { +function PRListBody({ loading, error, items, presets, onStartTask, prKeyToTasks }: PRListProps) { const statuses = usePRStatuses(items); if (loading) { return ( @@ -175,6 +180,7 @@ function PRListBody({ loading, error, items, presets, onStartTask }: PRListProps status={statuses.get(key)} presets={presets} onStartTask={onStartTask} + tasks={prKeyToTasks?.get(key)} /> ); })} diff --git a/apps/web/components/github/my-github/pr-row-task-indicator.test.tsx b/apps/web/components/github/my-github/pr-row-task-indicator.test.tsx new file mode 100644 index 0000000000..6e9d79b1a1 --- /dev/null +++ b/apps/web/components/github/my-github/pr-row-task-indicator.test.tsx @@ -0,0 +1,91 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import type { ReactNode } from "react"; +import { cleanup, render, screen } from "@testing-library/react"; +import { TooltipProvider } from "@kandev/ui/tooltip"; + +vi.mock("next/navigation", () => ({ + useRouter: () => ({ push: vi.fn(), replace: vi.fn(), prefetch: vi.fn() }), +})); + +import { StateProvider } from "@/components/state-provider"; +import { PRRowTaskIndicator } from "./pr-row-task-indicator"; +import type { TaskPR } from "@/lib/types/github"; + +function renderWithStore(ui: ReactNode) { + return render( + + {ui} + , + ); +} + +function makeTaskPR(overrides: Partial = {}): TaskPR { + return { + id: "pr-1", + task_id: "task-1", + owner: "o", + repo: "r", + pr_number: 1, + pr_url: "https://github.com/o/r/pull/1", + pr_title: "Test PR", + head_branch: "feat", + base_branch: "main", + author_login: "alice", + state: "open", + review_state: "", + checks_state: "", + mergeable_state: "", + review_count: 0, + pending_review_count: 0, + comment_count: 0, + unresolved_review_threads: 0, + checks_total: 0, + checks_passing: 0, + additions: 0, + deletions: 0, + created_at: "", + merged_at: null, + closed_at: null, + last_synced_at: null, + updated_at: "", + ...overrides, + }; +} + +afterEach(() => cleanup()); + +describe("PRRowTaskIndicator", () => { + it("shows 'No task created yet' when tasks is undefined", () => { + renderWithStore(); + expect(screen.getByText("No task created yet")).toBeTruthy(); + }); + + it("shows 'No task created yet' when tasks is empty", () => { + renderWithStore(); + expect(screen.getByText("No task created yet")).toBeTruthy(); + }); + + it("renders a clickable button with task title for a single task", () => { + renderWithStore(); + const btn = screen.getByRole("button"); + expect(btn.textContent).toContain("My Feature PR"); + }); + + it("shows 'Tasks' trigger with count badge for multiple tasks", () => { + const tasks = [ + makeTaskPR({ id: "a", task_id: "t1", pr_title: "First PR" }), + makeTaskPR({ id: "b", task_id: "t2", pr_title: "Second PR" }), + ]; + const { container } = renderWithStore(); + expect(screen.getByRole("button").textContent).toContain("Tasks"); + expect(container.textContent).toContain("2"); + }); + + it("truncates long task titles (>40 chars)", () => { + const longTitle = "This is an extremely long pull request title that should be truncated"; + renderWithStore(); + const btn = screen.getByRole("button"); + expect(btn.textContent).toContain("…"); + expect(btn.textContent!.length).toBeLessThan(longTitle.length + 5); + }); +}); diff --git a/apps/web/components/github/my-github/pr-row-task-indicator.tsx b/apps/web/components/github/my-github/pr-row-task-indicator.tsx new file mode 100644 index 0000000000..ce43467d88 --- /dev/null +++ b/apps/web/components/github/my-github/pr-row-task-indicator.tsx @@ -0,0 +1,144 @@ +"use client"; + +import { useCallback } from "react"; +import { useRouter } from "next/navigation"; +import { IconChecklist } from "@tabler/icons-react"; +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuTrigger, +} from "@kandev/ui/dropdown-menu"; +import { Badge } from "@kandev/ui/badge"; +import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip"; +import { useAppStore } from "@/components/state-provider"; +import { cn } from "@/lib/utils"; +import { linkToTask } from "@/lib/links"; +import { useTaskById } from "@/hooks/domains/kanban/use-task-by-id"; +import type { KanbanState } from "@/lib/state/slices"; +import type { TaskPR } from "@/lib/types/github"; + +type PRRowTaskIndicatorProps = { + tasks: TaskPR[] | undefined; +}; + +function useTaskStepTitle(workflowStepId: string | undefined): string | null { + return useAppStore((state) => { + if (!workflowStepId) return null; + const findIn = (steps: KanbanState["steps"]) => + steps.find((s) => s.id === workflowStepId)?.title ?? null; + const fromActive = findIn(state.kanban.steps); + if (fromActive) return fromActive; + for (const snap of Object.values(state.kanbanMulti.snapshots)) { + const t = findIn(snap.steps); + if (t) return t; + } + return null; + }); +} + +function truncateTitle(title: string): string { + return title.length > 40 ? title.slice(0, 40) + "…" : title; +} + +function TaskTitle({ taskId, fallback }: { taskId: string; fallback: string }) { + const taskData = useTaskById(taskId); + const title = taskData?.title ?? fallback; + return {truncateTitle(title)}; +} + +function SingleTaskButton({ + task, + onNavigate, +}: { + task: TaskPR; + onNavigate: (taskId: string) => void; +}) { + const taskData = useTaskById(task.task_id); + const stepTitle = useTaskStepTitle(taskData?.workflowStepId); + const title = taskData?.title ?? task.pr_title; + return ( + + + + + +
+ Task: {title} + {stepTitle ? Step: {stepTitle} : null} +
+ + + ); +} + +const buttonClass = cn( + "inline-flex items-center gap-1.5 rounded px-1.5 py-0.5 text-xs", + "hover:bg-muted/70 transition-colors cursor-pointer w-fit max-w-full", +); + +const iconClass = "h-3 w-3 shrink-0 text-muted-foreground"; + +export function PRRowTaskIndicator({ tasks }: PRRowTaskIndicatorProps) { + const setActiveTask = useAppStore((state) => state.setActiveTask); + const router = useRouter(); + + const navigate = useCallback( + (taskId: string) => { + setActiveTask(taskId); + router.push(linkToTask(taskId)); + }, + [setActiveTask, router], + ); + + if (!tasks || tasks.length === 0) { + return ( + + No task created yet + + ); + } + + if (tasks.length === 1) { + return ; + } + + return ( + + + + + + {tasks.map((t) => ( + // Use the TaskPR row id rather than task_id — multi-repo tasks can + // produce more than one TaskPR row for the same PR, which would + // collide on task_id. + navigate(t.task_id)} + > + + + ))} + + + ); +} diff --git a/apps/web/components/github/my-github/quick-task-launcher.tsx b/apps/web/components/github/my-github/quick-task-launcher.tsx index f13ac32774..93153ecccb 100644 --- a/apps/web/components/github/my-github/quick-task-launcher.tsx +++ b/apps/web/components/github/my-github/quick-task-launcher.tsx @@ -4,7 +4,8 @@ import { useMemo } from "react"; import { useRouter } from "next/navigation"; import type { Icon } from "@tabler/icons-react"; import { TaskCreateDialog } from "@/components/task-create-dialog"; -import type { Repository, Task, Workflow, WorkflowStep } from "@/lib/types/http"; +import { createTaskPR } from "@/lib/api/domains/github-api"; +import type { Repository, Task, TaskRepository, Workflow, WorkflowStep } from "@/lib/types/http"; import type { GitHubPR, GitHubIssue } from "@/lib/types/github"; export type TaskPreset = { @@ -40,6 +41,28 @@ function emptyToUndefined(value: string | undefined): string | undefined { return value ? value : undefined; } +// Multi-repo tasks have one task_repository row per repo; pick the one that +// matches the PR's owner/repo. Preference order: +// 1. preferredRepositoryId (the id captured at dialog-time by repo matching) +// 2. checkout_branch matching the PR head — handles cases where dialog +// didn't set a repositoryId (legacy github_url flow) +// 3. first row (safest fallback for single-repo tasks) +// Branch-only matching can mis-select when two task repos share branch names, +// so the dialog-time hint wins when present. +function pickRepositoryIdForPR( + taskRepos: TaskRepository[] | undefined, + pr: GitHubPR, + preferredRepositoryId?: string, +): string | undefined { + if (!taskRepos || taskRepos.length === 0) return preferredRepositoryId; + if (preferredRepositoryId) { + const preferred = taskRepos.find((r) => r.repository_id === preferredRepositoryId); + if (preferred) return preferred.repository_id; + } + const byBranch = taskRepos.find((r) => r.checkout_branch === pr.head_branch); + return (byBranch ?? taskRepos[0]).repository_id; +} + function extractPayload(payload: LaunchPayload) { if (payload.kind === "pr") { return { @@ -129,6 +152,24 @@ export function QuickTaskLauncher({ if (!open) onClose(); }; const handleSuccess = (task: Task) => { + if (payload?.kind === "pr") { + const repositoryId = pickRepositoryIdForPR( + task.repositories, + payload.pr, + dialog?.repositoryId, + ); + // Fire-and-forget: associating the PR is best-effort. A failure (network, + // missing GH client) shouldn't block navigation — the existing + // branch-based poller will still try once the agent starts. + void createTaskPR({ + task_id: task.id, + repository_id: repositoryId, + pr_url: payload.pr.html_url, + }).catch(() => { + // Silently ignore — the indicator will populate via the poller path + // (legacy behavior) if branch matching succeeds. + }); + } onClose(); router.push(`/tasks/${task.id}`); }; diff --git a/apps/web/e2e/tests/github/pr-list-task-indicator.spec.ts b/apps/web/e2e/tests/github/pr-list-task-indicator.spec.ts new file mode 100644 index 0000000000..872d382e13 --- /dev/null +++ b/apps/web/e2e/tests/github/pr-list-task-indicator.spec.ts @@ -0,0 +1,130 @@ +import { test, expect } from "../../fixtures/test-base"; + +test.describe("GitHub PR list task indicator", () => { + test("renders indicator states, tooltip content, and navigates on click", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(60_000); + + await apiClient.mockGitHubReset(); + await apiClient.mockGitHubSetUser("test-user"); + + await apiClient.mockGitHubAddPRs([ + { + number: 100, + title: "PR with linked task", + state: "open", + head_branch: "feat/linked", + base_branch: "main", + author_login: "test-user", + repo_owner: "testorg", + repo_name: "testrepo", + }, + { + number: 101, + title: "PR without task", + state: "open", + head_branch: "feat/orphan", + base_branch: "main", + author_login: "test-user", + repo_owner: "testorg", + repo_name: "testrepo", + }, + { + number: 102, + title: "PR with multiple tasks", + state: "open", + head_branch: "feat/multi", + base_branch: "main", + author_login: "test-user", + repo_owner: "testorg", + repo_name: "testrepo", + }, + ]); + + const task = await apiClient.createTask(seedData.workspaceId, "Linked task title", { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + agent_profile_id: seedData.agentProfileId, + repository_ids: [seedData.repositoryId], + }); + + await apiClient.mockGitHubAssociateTaskPR({ + task_id: task.id, + owner: "testorg", + repo: "testrepo", + pr_number: 100, + pr_url: "https://github.com/testorg/testrepo/pull/100", + pr_title: "PR with linked task", + head_branch: "feat/linked", + base_branch: "main", + author_login: "test-user", + }); + + // Multi-task case: two distinct tasks linked to the same PR (PR #102). + // Regression guard for the bug where review tasks created via "+ Task" + // on the GitHub page never got their github_task_prs row written + // (synthetic worktree branches never matched the PR head on GitHub). + const multiA = await apiClient.createTask(seedData.workspaceId, "Multi task A", { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + agent_profile_id: seedData.agentProfileId, + repository_ids: [seedData.repositoryId], + }); + const multiB = await apiClient.createTask(seedData.workspaceId, "Multi task B", { + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + agent_profile_id: seedData.agentProfileId, + repository_ids: [seedData.repositoryId], + }); + for (const t of [multiA, multiB]) { + await apiClient.mockGitHubAssociateTaskPR({ + task_id: t.id, + owner: "testorg", + repo: "testrepo", + pr_number: 102, + pr_url: "https://github.com/testorg/testrepo/pull/102", + pr_title: "PR with multiple tasks", + head_branch: "feat/multi", + base_branch: "main", + author_login: "test-user", + }); + } + + await testPage.goto("/github"); + + const singleIndicator = testPage.getByTestId("pr-row-task-indicator-single"); + await expect(singleIndicator).toBeVisible({ timeout: 15_000 }); + await expect(singleIndicator).toContainText("Linked task title"); + + const emptyIndicator = testPage.getByTestId("pr-row-task-indicator-empty"); + await expect(emptyIndicator).toBeVisible(); + await expect(emptyIndicator).toHaveText("No task created yet"); + + const multiIndicator = testPage.getByTestId("pr-row-task-indicator-multi"); + await expect(multiIndicator).toBeVisible(); + await expect(multiIndicator).toContainText("2"); + + await singleIndicator.hover(); + + const startStep = seedData.steps.find((s) => s.id === seedData.startStepId); + const tooltip = testPage.getByRole("tooltip"); + await expect(tooltip).toContainText("Task: Linked task title", { timeout: 5_000 }); + if (startStep?.title) { + await expect(tooltip).toContainText(`Step: ${startStep.title}`); + } + + // Click the indicator and confirm a real route change to /t/. + // Regression guard: `replaceTaskUrl` previously only mutated history.state + // without triggering Next.js navigation, leaving the PR list rendered. + await singleIndicator.click(); + await expect(testPage).toHaveURL(new RegExp(`/t/${task.id}(?:\\?|$)`), { + timeout: 15_000, + }); + await expect(testPage.getByTestId("pr-row-task-indicator-single")).toHaveCount(0, { + timeout: 10_000, + }); + }); +}); diff --git a/apps/web/hooks/domains/github/use-pr-key-to-tasks.test.ts b/apps/web/hooks/domains/github/use-pr-key-to-tasks.test.ts new file mode 100644 index 0000000000..c858cbf793 --- /dev/null +++ b/apps/web/hooks/domains/github/use-pr-key-to-tasks.test.ts @@ -0,0 +1,141 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { createElement, type ReactNode } from "react"; +import { act, renderHook, cleanup } from "@testing-library/react"; +import { StateProvider, useAppStore } from "@/components/state-provider"; +import { prKey, usePRKeyToTasks } from "./use-pr-key-to-tasks"; +import type { TaskPR } from "@/lib/types/github"; + +vi.mock("./use-task-pr", () => ({ + // The hook is fire-and-forget side-effect; mocking it keeps the test + // focused on the inversion logic and avoids a real network call. + useWorkspacePRs: () => undefined, +})); + +afterEach(() => cleanup()); + +function makeTaskPR(overrides: Partial = {}): TaskPR { + return { + id: "pr", + task_id: "task-1", + owner: "kdlbs", + repo: "kandev", + pr_number: 1, + pr_url: "", + pr_title: "", + head_branch: "", + base_branch: "", + author_login: "", + state: "open", + review_state: "", + checks_state: "", + mergeable_state: "", + review_count: 0, + pending_review_count: 0, + comment_count: 0, + unresolved_review_threads: 0, + checks_total: 0, + checks_passing: 0, + additions: 0, + deletions: 0, + created_at: "", + merged_at: null, + closed_at: null, + last_synced_at: null, + updated_at: "", + ...overrides, + }; +} + +function wrapper({ children }: { children: ReactNode }) { + return createElement(StateProvider, null, children); +} + +// Render usePRKeyToTasks alongside the store's setTaskPRs action so tests can +// seed the store and re-read the inverted map within the same render tree. +function renderUsePRKeyToTasks() { + return renderHook( + () => { + const setTaskPRs = useAppStore((s) => s.setTaskPRs); + const map = usePRKeyToTasks("ws-1"); + return { setTaskPRs, map }; + }, + { wrapper }, + ); +} + +describe("prKey", () => { + it("formats owner/repo#number", () => { + expect(prKey("kdlbs", "kandev", 42)).toBe("kdlbs/kandev#42"); + }); + + it("handles underscores and dashes in owner/repo", () => { + expect(prKey("my-org", "my_repo", 1)).toBe("my-org/my_repo#1"); + }); + + it("handles large PR numbers", () => { + expect(prKey("foo", "bar", 99999)).toBe("foo/bar#99999"); + }); + + it("handles zero PR number", () => { + expect(prKey("o", "r", 0)).toBe("o/r#0"); + }); + + it("handles single-character owner and repo", () => { + expect(prKey("a", "b", 7)).toBe("a/b#7"); + }); +}); + +describe("usePRKeyToTasks", () => { + it("returns an empty map when no task PRs are loaded", () => { + const { result } = renderUsePRKeyToTasks(); + expect(result.current.map.size).toBe(0); + }); + + it("groups distinct tasks linked to the same PR under one key", () => { + const { result } = renderUsePRKeyToTasks(); + act(() => { + result.current.setTaskPRs({ + "task-a": [ + makeTaskPR({ id: "row-1", task_id: "task-a", owner: "o", repo: "r", pr_number: 7 }), + ], + "task-b": [ + makeTaskPR({ id: "row-2", task_id: "task-b", owner: "o", repo: "r", pr_number: 7 }), + ], + }); + }); + const entries = result.current.map.get("o/r#7"); + expect(entries?.length).toBe(2); + expect(entries?.map((e) => e.task_id).sort()).toEqual(["task-a", "task-b"]); + }); + + it("keeps PRs that belong to different keys separate", () => { + const { result } = renderUsePRKeyToTasks(); + act(() => { + result.current.setTaskPRs({ + "task-a": [ + makeTaskPR({ id: "row-1", task_id: "task-a", owner: "o", repo: "r", pr_number: 1 }), + makeTaskPR({ id: "row-2", task_id: "task-a", owner: "o", repo: "r", pr_number: 2 }), + ], + }); + }); + expect(result.current.map.get("o/r#1")?.length).toBe(1); + expect(result.current.map.get("o/r#2")?.length).toBe(1); + }); + + it("skips entries whose value is not an array (defensive against partial hydration)", () => { + const { result } = renderUsePRKeyToTasks(); + act(() => { + result.current.setTaskPRs({ + "task-a": [ + makeTaskPR({ id: "row-1", task_id: "task-a", owner: "o", repo: "r", pr_number: 1 }), + ], + // Partial hydration may briefly seed byTaskId[task] with a non-array + // (e.g. an empty object). The hook should ignore those rows instead of + // throwing. + "task-bad": {} as unknown as TaskPR[], + }); + }); + expect(result.current.map.get("o/r#1")?.length).toBe(1); + expect(result.current.map.size).toBe(1); + }); +}); diff --git a/apps/web/hooks/domains/github/use-pr-key-to-tasks.ts b/apps/web/hooks/domains/github/use-pr-key-to-tasks.ts new file mode 100644 index 0000000000..d7bc20d5c9 --- /dev/null +++ b/apps/web/hooks/domains/github/use-pr-key-to-tasks.ts @@ -0,0 +1,32 @@ +"use client"; + +import { useMemo } from "react"; +import { useAppStore } from "@/components/state-provider"; +import { useWorkspacePRs } from "./use-task-pr"; +import type { TaskPR } from "@/lib/types/github"; + +function prKey(owner: string, repo: string, prNumber: number): string { + return `${owner}/${repo}#${prNumber}`; +} + +export function usePRKeyToTasks(workspaceId: string | null): Map { + useWorkspacePRs(workspaceId); + const byTaskId = useAppStore((state) => state.taskPRs.byTaskId); + + return useMemo(() => { + const map = new Map(); + for (const taskId of Object.keys(byTaskId)) { + const prs = byTaskId[taskId]; + if (!Array.isArray(prs)) continue; + for (const pr of prs) { + const key = prKey(pr.owner, pr.repo, pr.pr_number); + const existing = map.get(key) ?? []; + existing.push(pr); + map.set(key, existing); + } + } + return map; + }, [byTaskId]); +} + +export { prKey }; diff --git a/apps/web/lib/api/domains/github-api.ts b/apps/web/lib/api/domains/github-api.ts index de6c729ee3..8a8c459fe8 100644 --- a/apps/web/lib/api/domains/github-api.ts +++ b/apps/web/lib/api/domains/github-api.ts @@ -65,6 +65,20 @@ export async function getTaskPR(taskId: string, options?: ApiRequestOptions) { return fetchJson(`/api/v1/github/task-prs/${taskId}`, options); } +export async function createTaskPR( + data: { task_id: string; repository_id?: string; pr_url: string }, + options?: ApiRequestOptions, +) { + return fetchJson(`/api/v1/github/task-prs`, { + ...options, + init: { + ...(options?.init ?? {}), + method: "POST", + body: JSON.stringify(data), + }, + }); +} + // PR feedback (live from GitHub) export async function getPRFeedback( owner: string,