dreampiler/kandev
1
0
Fork 0
Code Issues 43 Pull Requests Actions 6 Packages Projects Releases Wiki Activity

feat: unlink task pull requests (#2114)

Browse Source
This commit is contained in:
Carlos Florêncio
2026-08-02 17:00:32 +01:00
committed by GitHub
parent 554c33f24d
commit a6326fdc5d
38 changed files with 1808 additions and 109 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/backend/internal/events/types.go
+1
View File
@@ -270,6 +270,7 @@ const (
GitHubNewReviewPR = "github.new_pr_to_review" // New PR found needing review
GitHubNewIssue = "github.new_issue" // New issue found matching issue watch
GitHubTaskPRUpdated = "github.task_pr.updated" // TaskPR record updated (for UI refresh)
GitHubTaskPRDeleted = "github.task_pr.deleted" // TaskPR association detached (for UI refresh)
GitHubTaskCIOptionsUpdated = "github.task_ci_options.updated" // Task CI automation options updated
GitHubWatchEvent = "github.watch.event" // Watch created/deleted
GitHubRateLimitUpdated = "github.rate_limit.updated" // GitHub API rate-limit snapshot changed
apps/backend/internal/gateway/websocket/task_notifications.go
+1
View File
@@ -83,6 +83,7 @@ func RegisterTaskNotifications(ctx context.Context, eventBus bus.EventBus, hub *
b.subscribe(eventBus, events.TurnCompleted, ws.ActionSessionTurnCompleted)
b.subscribe(eventBus, events.MessageQueueStatusChanged, ws.ActionMessageQueueStatusChanged)
b.subscribe(eventBus, events.GitHubTaskPRUpdated, ws.ActionGitHubTaskPRUpdated)
b.subscribe(eventBus, events.GitHubTaskPRDeleted, ws.ActionGitHubTaskPRDeleted)
b.subscribe(eventBus, events.GitHubTaskCIOptionsUpdated, ws.ActionGitHubTaskCIOptionsUpdated)
b.subscribe(eventBus, events.GitHubRateLimitUpdated, ws.ActionGitHubRateLimitUpdated)
b.subscribe(eventBus, events.GitLabTaskMRUpdated, ws.ActionGitLabTaskMRUpdated)
apps/backend/internal/gateway/websocket/task_notifications_test.go
+1
View File
@@ -123,6 +123,7 @@ func TestTaskEventBroadcaster_NoDuplicateSubscriptions(t *testing.T) {
events.TaskSessionStateChanged,
events.TaskSessionActivityChanged,
events.GitHubTaskPRUpdated,
events.GitHubTaskPRDeleted,
events.GitLabTaskMRUpdated,
} {
subject := subject
apps/backend/internal/github/controller.go
+21
View File
@@ -71,6 +71,7 @@ func (c *Controller) RegisterHTTPRoutes(router *gin.Engine) {
api.GET("/task-prs", c.httpListTaskPRs)
api.POST("/task-prs", c.httpCreateTaskPR)
api.DELETE("/task-prs/:associationId", c.httpDeleteTaskPR)
api.GET("/task-prs/:taskId", c.httpGetTaskPR)
api.GET("/task-issues", c.httpListTaskIssues)
api.PUT("/tasks/:taskId/issue", c.httpLinkTaskIssue)
@@ -249,6 +250,26 @@ func (c *Controller) httpCreateTaskPR(ctx *gin.Context) {
ctx.JSON(http.StatusOK, tp)
}
func (c *Controller) httpDeleteTaskPR(ctx *gin.Context) {
workspaceID := ctx.Query("workspace_id")
if workspaceID == "" {
ctx.JSON(http.StatusBadRequest, gin.H{"error": "workspace_id query parameter required"})
return
}
associationID := ctx.Param("associationId")
if _, err := c.service.DetachTaskPR(ctx.Request.Context(), workspaceID, associationID); err != nil {
if errors.Is(err, ErrTaskPRNotFound) || writeWatchWorkspaceError(ctx, "task PR", err) {
if !errors.Is(err, repoerrors.ErrWorkspaceNotFound) {
ctx.JSON(http.StatusNotFound, gin.H{"error": "task PR not found"})
}
return
}
ctx.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
return
}
ctx.JSON(http.StatusOK, gin.H{"deleted": true})
}
func (c *Controller) httpGetTaskPR(ctx *gin.Context) {
taskID := ctx.Param("taskId")
tp, err := c.service.GetTaskPR(ctx.Request.Context(), taskID)
apps/backend/internal/github/controller_test.go
+51
View File
@@ -312,6 +312,57 @@ func TestCredentialBrokerReadinessUsesExactResolveRoute(t *testing.T) {
}
}
func TestHttpDeleteTaskPRDetachesOnlyTheRequestedWorkspaceAssociation(t *testing.T) {
router, store := setupControllerStoreTest(t)
ctx := context.Background()
first := &TaskPR{
WorkspaceID: "ws-1", TaskID: "task-1", RepositoryID: "repo-1", Owner: "acme", Repo: "demo",
PRNumber: 10, PRURL: "https://github.com/acme/demo/pull/10", PRTitle: "old", HeadBranch: "old",
BaseBranch: "main", State: "merged", CreatedAt: time.Now().UTC(),
}
second := &TaskPR{
WorkspaceID: "ws-1", TaskID: "task-1", RepositoryID: "repo-1", Owner: "acme", Repo: "demo",
PRNumber: 11, PRURL: "https://github.com/acme/demo/pull/11", PRTitle: "new", HeadBranch: "new",
BaseBranch: "main", State: "open", CreatedAt: time.Now().UTC().Add(time.Second),
}
if err := store.CreateTaskPR(ctx, first); err != nil {
t.Fatalf("create first PR: %v", err)
}
if err := store.CreateTaskPR(ctx, second); err != nil {
t.Fatalf("create second PR: %v", err)
}
request := httptest.NewRequest(http.MethodDelete, "/api/v1/github/task-prs/"+first.ID+"?workspace_id=ws-1", nil)
response := httptest.NewRecorder()
router.ServeHTTP(response, request)
if response.Code != http.StatusOK {
t.Fatalf("delete status = %d, body = %s", response.Code, response.Body.String())
}
var body struct {
Deleted bool `json:"deleted"`
}
if err := json.NewDecoder(response.Body).Decode(&body); err != nil {
t.Fatalf("decode delete response: %v", err)
}
if !body.Deleted {
t.Fatal("delete response deleted=false, want true")
}
active, err := store.ListTaskPRsByTask(ctx, "task-1")
if err != nil {
t.Fatalf("list active PRs: %v", err)
}
if len(active) != 1 || active[0].ID != second.ID {
t.Fatalf("active PRs = %+v, want second association only", active)
}
request = httptest.NewRequest(http.MethodDelete, "/api/v1/github/task-prs/"+second.ID+"?workspace_id=other", nil)
response = httptest.NewRecorder()
router.ServeHTTP(response, request)
if response.Code != http.StatusNotFound {
t.Fatalf("cross-workspace delete status = %d, want 404", response.Code)
}
}
func TestHttpListTaskIssues_WorkspaceScopedMetadataLinks(t *testing.T) {
router, store := setupControllerStoreTest(t)
rows := []struct {
apps/backend/internal/github/models.go
+1
View File
@@ -432,6 +432,7 @@ type TaskPR struct {
MergedAt *time.Time `json:"merged_at,omitempty" db:"merged_at"`
ClosedAt *time.Time `json:"closed_at,omitempty" db:"closed_at"`
LastSyncedAt *time.Time `json:"last_synced_at,omitempty" db:"last_synced_at"`
DetachedAt *time.Time `json:"-" db:"detached_at"`
UpdatedAt time.Time `json:"updated_at" db:"updated_at"`
}
apps/backend/internal/github/service_pr_watch.go
+21 -8
View File
@@ -284,7 +284,7 @@ func (s *Service) ensurePRWatch(
// callers MUST pass it — empty causes ReplaceTaskPR to wipe the entire task's
// PR rows (legacy "delete all" branch), which is what older code relied on.
func (s *Service) AssociatePRWithTask(ctx context.Context, taskID, repositoryID string, pr *PR) (*TaskPR, error) {
return s.associatePRWithTask(ctx, "", taskID, repositoryID, pr)
return s.associatePRWithTask(ctx, "", taskID, repositoryID, pr, false)
}
func (s *Service) AssociatePRWithTaskForWorkspace(
@@ -293,11 +293,11 @@ func (s *Service) AssociatePRWithTaskForWorkspace(
if strings.TrimSpace(workspaceID) == "" {
return nil, ErrGitHubWorkspaceRequired
}
return s.associatePRWithTask(ctx, workspaceID, taskID, repositoryID, pr)
return s.associatePRWithTask(ctx, workspaceID, taskID, repositoryID, pr, false)
}
func (s *Service) associatePRWithTask(
ctx context.Context, workspaceID, taskID, repositoryID string, pr *PR,
ctx context.Context, workspaceID, taskID, repositoryID string, pr *PR, restoreDetached bool,
) (*TaskPR, error) {
// Multi-branch: scope the "already-current" short-circuit by exact
// pr_number too. A task can hold multiple PR rows per (task, repo) on
@@ -306,11 +306,24 @@ func (s *Service) associatePRWithTask(
// secondary PR is already there (wrong PR number) and skip the insert
// — or worse, fall through to ReplaceTaskPR which used to delete the
// sibling row.
existing, err := s.store.GetTaskPRByRepoAndNumber(ctx, taskID, repositoryID, pr.Number)
existing, err := s.store.GetTaskPRByRepoAndNumberIncludingDetached(ctx, taskID, repositoryID, pr.Number)
if err != nil {
return nil, err
}
if existing != nil {
if existing.DetachedAt != nil && restoreDetached {
restored, restoreErr := s.store.RestoreTaskPR(ctx, taskID, repositoryID, pr)
if restoreErr != nil || restored == nil {
return restored, restoreErr
}
if s.eventBus != nil {
event := bus.NewEvent(events.GitHubTaskPRUpdated, "github", restored)
if err := s.eventBus.Publish(ctx, events.GitHubTaskPRUpdated, event); err != nil {
s.logger.Debug("failed to publish restored task PR event", zap.Error(err))
}
}
return restored, nil
}
return existing, nil
}
tp := &TaskPR{
@@ -380,7 +393,7 @@ func (s *Service) AssociateExistingPRByURL(ctx context.Context, taskID, reposito
if err != nil {
return nil, fmt.Errorf("fetch PR: %w", err)
}
tp, err := s.AssociatePRWithTask(ctx, taskID, repositoryID, pr)
tp, err := s.associatePRWithTask(ctx, "", taskID, repositoryID, pr, true)
if err != nil {
return nil, fmt.Errorf("associate PR with task: %w", err)
}
@@ -405,7 +418,7 @@ func (s *Service) AssociateExistingPRByURLForWorkspace(
if err != nil {
return nil, fmt.Errorf("fetch PR: %w", err)
}
tp, err := s.AssociatePRWithTaskForWorkspace(ctx, workspaceID, taskID, repositoryID, pr)
tp, err := s.associatePRWithTask(ctx, workspaceID, taskID, repositoryID, pr, true)
if err != nil {
return nil, fmt.Errorf("associate PR with task: %w", err)
}
@@ -445,7 +458,7 @@ func (s *Service) AssociatePRByURL(ctx context.Context, sessionID, taskID, repos
}
// Associate PR with task (persists + publishes WS event)
if _, assocErr := s.AssociatePRWithTask(ctx, taskID, repositoryID, pr); assocErr != nil {
if _, assocErr := s.associatePRWithTask(ctx, "", taskID, repositoryID, pr, true); assocErr != nil {
s.logger.Error("failed to associate PR with task after creation",
zap.String("task_id", taskID), zap.Error(assocErr))
}
@@ -477,7 +490,7 @@ func (s *Service) AssociatePRByURLForWorkspace(
); err != nil {
return fmt.Errorf("create PR watch: %w", err)
}
if _, err := s.AssociatePRWithTaskForWorkspace(ctx, workspaceID, taskID, repositoryID, pr); err != nil {
if _, err := s.associatePRWithTask(ctx, workspaceID, taskID, repositoryID, pr, true); err != nil {
return fmt.Errorf("associate PR with task: %w", err)
}
return nil
apps/backend/internal/github/service_task_pr_detach.go
+67
View File
@@ -0,0 +1,67 @@
package github
import (
"context"
"errors"
"strings"
"github.com/kandev/kandev/internal/events"
"github.com/kandev/kandev/internal/events/bus"
"go.uber.org/zap"
)
var ErrTaskPRNotFound = errors.New("github task PR not found")
// TaskPRDeletedEvent identifies one task-PR association removed from active
// task surfaces. The remote GitHub pull request is intentionally untouched.
type TaskPRDeletedEvent struct {
WorkspaceID string `json:"workspace_id"`
TaskID string `json:"task_id"`
AssociationID string `json:"association_id"`
}
// GetWorkspaceID lets the websocket broadcaster route the typed event to the
// owning workspace instead of treating it as an instance-wide notification.
func (e TaskPRDeletedEvent) GetWorkspaceID() string { return e.WorkspaceID }
// DetachTaskPR removes one association from active task views while retaining
// a durable tombstone that prevents background discovery from resurrecting it.
func (s *Service) DetachTaskPR(ctx context.Context, workspaceID, associationID string) (*TaskPR, error) {
if strings.TrimSpace(workspaceID) == "" || strings.TrimSpace(associationID) == "" {
return nil, ErrTaskPRNotFound
}
tp, err := s.store.GetTaskPRByID(ctx, associationID)
if err != nil {
return nil, err
}
if tp == nil || (tp.WorkspaceID != "" && tp.WorkspaceID != workspaceID) {
return nil, ErrTaskPRNotFound
}
if err := s.authorizeWorkspaceAccess(ctx, workspaceID); err != nil {
return nil, err
}
if tp.DetachedAt != nil {
return tp, nil
}
detached, transitioned, err := s.store.DetachTaskPR(ctx, associationID)
if err != nil {
return nil, err
}
if detached == nil {
return nil, ErrTaskPRNotFound
}
if !transitioned {
return detached, nil
}
if s.eventBus != nil {
event := bus.NewEvent(events.GitHubTaskPRDeleted, "github", &TaskPRDeletedEvent{
WorkspaceID: workspaceID,
TaskID: detached.TaskID,
AssociationID: detached.ID,
})
if err := s.eventBus.Publish(ctx, events.GitHubTaskPRDeleted, event); err != nil {
s.logger.Debug("failed to publish task PR deleted event", zap.Error(err))
}
}
return detached, nil
}
apps/backend/internal/github/service_task_pr_detach_test.go
+220
View File
@@ -0,0 +1,220 @@
package github
import (
"context"
"reflect"
"testing"
"time"
)
func TestServiceDetachTaskPRAuthorizesWorkspaceAndPublishesRemoval(t *testing.T) {
svc, store, eventBus := setupSyncTest(t)
contract, ok := any(svc).(interface {
DetachTaskPR(context.Context, string, string) (*TaskPR, error)
})
if !ok {
t.Fatal("Service does not implement DetachTaskPR")
}
ctx := context.Background()
first := &TaskPR{
WorkspaceID: "ws-1", TaskID: "task-1", RepositoryID: "repo-1", Owner: "acme", Repo: "demo",
PRNumber: 1, PRURL: "https://github.com/acme/demo/pull/1", PRTitle: "old", HeadBranch: "old",
BaseBranch: "main", State: "merged", CreatedAt: time.Now().UTC(),
}
second := &TaskPR{
WorkspaceID: "ws-1", TaskID: "task-1", RepositoryID: "repo-1", Owner: "acme", Repo: "demo",
PRNumber: 2, PRURL: "https://github.com/acme/demo/pull/2", PRTitle: "new", HeadBranch: "new",
BaseBranch: "main", State: "open", CreatedAt: time.Now().UTC().Add(time.Second),
}
if err := store.CreateTaskPR(ctx, first); err != nil {
t.Fatalf("create first PR: %v", err)
}
if err := store.CreateTaskPR(ctx, second); err != nil {
t.Fatalf("create second PR: %v", err)
}
var authorizedWorkspace string
svc.SetWorkspaceAuthorizer(func(_ context.Context, workspaceID string) error {
authorizedWorkspace = workspaceID
return nil
})
detached, err := contract.DetachTaskPR(ctx, "ws-1", first.ID)
if err != nil {
t.Fatalf("detach task PR: %v", err)
}
if detached == nil || detached.ID != first.ID || detached.DetachedAt == nil {
t.Fatalf("detached = %+v, want stamped first row", detached)
}
if authorizedWorkspace != "ws-1" {
t.Fatalf("authorized workspace = %q, want ws-1", authorizedWorkspace)
}
active, err := store.ListTaskPRsByTask(ctx, "task-1")
if err != nil {
t.Fatalf("list active task PRs: %v", err)
}
if len(active) != 1 || active[0].ID != second.ID {
t.Fatalf("active task PRs = %+v, want sibling only", active)
}
if eventBus.publishedCount() != 1 {
t.Fatalf("published events = %d, want one deletion event", eventBus.publishedCount())
}
eventBus.mu.Lock()
event := eventBus.events[0]
eventBus.mu.Unlock()
if event.Type != "github.task_pr.deleted" {
t.Fatalf("event type = %q, want github.task_pr.deleted", event.Type)
}
payload := reflect.ValueOf(event.Data)
if payload.Kind() == reflect.Pointer {
payload = payload.Elem()
}
if !payload.IsValid() || payload.FieldByName("AssociationID").String() != first.ID || payload.FieldByName("TaskID").String() != first.TaskID || payload.FieldByName("WorkspaceID").String() != first.WorkspaceID {
t.Fatalf("event payload = %#v, want first association identity", event.Data)
}
}
func TestServiceDetachTaskPRFailsClosedForWorkspaceMismatch(t *testing.T) {
svc, store, _ := setupSyncTest(t)
contract, ok := any(svc).(interface {
DetachTaskPR(context.Context, string, string) (*TaskPR, error)
})
if !ok {
t.Fatal("Service does not implement DetachTaskPR")
}
pr := &TaskPR{WorkspaceID: "ws-1", TaskID: "task-1", Owner: "acme", Repo: "demo", PRNumber: 1, CreatedAt: time.Now().UTC()}
if err := store.CreateTaskPR(context.Background(), pr); err != nil {
t.Fatalf("create PR: %v", err)
}
_, err := contract.DetachTaskPR(context.Background(), "ws-other", pr.ID)
if err == nil || err.Error() != "github task PR not found" {
t.Fatalf("error = %v, want github task PR not found", err)
}
active, err := store.ListTaskPRsByTask(context.Background(), pr.TaskID)
if err != nil {
t.Fatalf("list task PRs: %v", err)
}
if len(active) != 1 || active[0].ID != pr.ID {
t.Fatalf("active task PRs = %+v, want association unchanged", active)
}
}
func TestServiceDetachTaskPRAllowsLegacyEmptyWorkspace(t *testing.T) {
svc, store, eventBus := setupSyncTest(t)
ctx := context.Background()
pr := &TaskPR{
TaskID: "task-legacy", Owner: "acme", Repo: "demo", PRNumber: 7,
PRURL: "https://github.com/acme/demo/pull/7", CreatedAt: time.Now().UTC(),
}
if err := store.CreateTaskPR(ctx, pr); err != nil {
t.Fatalf("create legacy PR: %v", err)
}
var authorizedWorkspace string
svc.SetWorkspaceAuthorizer(func(_ context.Context, workspaceID string) error {
authorizedWorkspace = workspaceID
return nil
})
detached, err := svc.DetachTaskPR(ctx, "ws-legacy", pr.ID)
if err != nil {
t.Fatalf("detach legacy PR: %v", err)
}
if detached == nil || detached.DetachedAt == nil {
t.Fatalf("detached legacy PR = %+v, want stamped row", detached)
}
if authorizedWorkspace != "ws-legacy" {
t.Fatalf("authorized workspace = %q, want ws-legacy", authorizedWorkspace)
}
if eventBus.publishedCount() != 1 {
t.Fatalf("published events = %d, want one deletion event", eventBus.publishedCount())
}
eventBus.mu.Lock()
event := eventBus.events[0]
eventBus.mu.Unlock()
payload := reflect.ValueOf(event.Data)
if payload.Kind() == reflect.Pointer {
payload = payload.Elem()
}
if payload.FieldByName("WorkspaceID").String() != "ws-legacy" {
t.Fatalf("event workspace = %q, want ws-legacy", payload.FieldByName("WorkspaceID").String())
}
}
func TestAssociatePRWithTaskDoesNotResurrectDetachedAssociation(t *testing.T) {
svc, store, _ := setupSyncTest(t)
ctx := context.Background()
row := &TaskPR{
WorkspaceID: "ws-1", TaskID: "task-1", RepositoryID: "repo-1", Owner: "acme", Repo: "demo",
PRNumber: 3, PRURL: "https://github.com/acme/demo/pull/3", PRTitle: "old", HeadBranch: "old",
BaseBranch: "main", State: "open", CreatedAt: time.Now().UTC(),
}
if err := store.CreateTaskPR(ctx, row); err != nil {
t.Fatalf("create PR: %v", err)
}
if _, _, err := store.DetachTaskPR(ctx, row.ID); err != nil {
t.Fatalf("detach PR: %v", err)
}
associated, err := svc.AssociatePRWithTask(ctx, row.TaskID, row.RepositoryID, &PR{
Number: 3, RepoOwner: "acme", RepoName: "demo", HTMLURL: row.PRURL, Title: "old", HeadBranch: "old", BaseBranch: "main", State: "open",
})
if err != nil {
t.Fatalf("associate PR: %v", err)
}
if associated == nil || associated.DetachedAt == nil {
t.Fatalf("associated = %+v, want detached tombstone to remain", associated)
}
active, err := store.ListTaskPRsByTask(ctx, row.TaskID)
if err != nil {
t.Fatalf("list active PRs: %v", err)
}
if len(active) != 0 {
t.Fatalf("active PRs = %+v, want no resurrected row", active)
}
}
func TestAssociatePRWithTaskExplicitLinkRestoresDetachedAssociation(t *testing.T) {
svc, store, eventBus := setupSyncTest(t)
ctx := context.Background()
row := &TaskPR{
WorkspaceID: "ws-1", TaskID: "task-1", RepositoryID: "repo-1", Owner: "acme", Repo: "demo",
PRNumber: 4, PRURL: "https://github.com/acme/demo/pull/4", PRTitle: "old", HeadBranch: "old",
BaseBranch: "main", State: "open", CreatedAt: time.Now().UTC(),
}
if err := store.CreateTaskPR(ctx, row); err != nil {
t.Fatalf("create PR: %v", err)
}
if _, _, err := store.DetachTaskPR(ctx, row.ID); err != nil {
t.Fatalf("detach PR: %v", err)
}
freshPR := &PR{
Number: 4, RepoOwner: "acme", RepoName: "demo", HTMLURL: "https://github.com/acme/demo/pull/4",
Title: "fresh title", HeadBranch: "fresh-branch", BaseBranch: "develop", AuthorLogin: "bob",
State: "closed", MergeableState: "dirty", Additions: 8, Deletions: 3,
}
associated, err := svc.associatePRWithTask(ctx, row.WorkspaceID, row.TaskID, row.RepositoryID, freshPR, true)
if err != nil {
t.Fatalf("explicitly associate PR: %v", err)
}
if associated == nil || associated.DetachedAt != nil {
t.Fatalf("associated = %+v, want active restored row", associated)
}
if associated.PRURL != freshPR.HTMLURL || associated.PRTitle != freshPR.Title ||
associated.HeadBranch != freshPR.HeadBranch || associated.BaseBranch != freshPR.BaseBranch ||
associated.AuthorLogin != freshPR.AuthorLogin || associated.State != freshPR.State ||
associated.MergeableState != freshPR.MergeableState || associated.Additions != freshPR.Additions ||
associated.Deletions != freshPR.Deletions {
t.Fatalf("restored PR = %+v, want fresh GitHub data", associated)
}
active, err := store.ListTaskPRsByTask(ctx, row.TaskID)
if err != nil {
t.Fatalf("list active PRs: %v", err)
}
if len(active) != 1 || active[0].ID != row.ID {
t.Fatalf("active PRs = %+v, want restored row", active)
}
if eventBus.publishedCount() != 1 {
t.Fatalf("published events = %d, want one restoration update", eventBus.publishedCount())
}
}
apps/backend/internal/github/store.go
+85 -16
View File
@@ -134,6 +134,7 @@ const createTablesSQL = `
merged_at DATETIME,
closed_at DATETIME,
last_synced_at DATETIME,
detached_at DATETIME,
updated_at DATETIME NOT NULL,
UNIQUE(task_id, repository_id, pr_number)
);
@@ -494,6 +495,7 @@ func (s *Store) applyIdempotentSchemaColumns() {
_, _ = s.db.Exec(`ALTER TABLE github_task_prs ADD COLUMN unresolved_review_threads INTEGER DEFAULT 0`)
_, _ = s.db.Exec(`ALTER TABLE github_task_prs ADD COLUMN checks_total INTEGER DEFAULT 0`)
_, _ = s.db.Exec(`ALTER TABLE github_task_prs ADD COLUMN checks_passing INTEGER DEFAULT 0`)
_, _ = s.db.Exec(`ALTER TABLE github_task_prs ADD COLUMN detached_at DATETIME`)
// Per-watch cleanup policy for review/issue watches: controls whether the
// poller deletes auto-created tasks when the underlying PR/issue reaches
// a terminal state. Values: 'auto' (default — preserve only when user
@@ -1140,6 +1142,7 @@ func (s *Store) migratePRTablesForMultiRepo() error {
merged_at DATETIME,
closed_at DATETIME,
last_synced_at DATETIME,
detached_at DATETIME,
updated_at DATETIME NOT NULL,
UNIQUE(task_id, repository_id, pr_number)
)`,
@@ -1147,12 +1150,12 @@ func (s *Store) migratePRTablesForMultiRepo() error {
id, workspace_id, task_id, repository_id, owner, repo, pr_number, pr_url, pr_title,
head_branch, base_branch, author_login, state, review_state, checks_state,
mergeable_state, review_count, pending_review_count, comment_count,
additions, deletions, created_at, merged_at, closed_at, last_synced_at, updated_at
additions, deletions, created_at, merged_at, closed_at, last_synced_at, detached_at, updated_at
) SELECT
id, COALESCE(workspace_id, ''), task_id, COALESCE(repository_id, ''), owner, repo, pr_number, pr_url, pr_title,
head_branch, base_branch, author_login, state, review_state, checks_state,
mergeable_state, review_count, pending_review_count, comment_count,
additions, deletions, created_at, merged_at, closed_at, last_synced_at, updated_at
additions, deletions, created_at, merged_at, closed_at, last_synced_at, detached_at, updated_at
FROM github_task_prs`,
)
}
@@ -1460,12 +1463,12 @@ func (s *Store) CreateTaskPR(ctx context.Context, tp *TaskPR) error {
INSERT INTO github_task_prs (id, workspace_id, task_id, repository_id, owner, repo, pr_number, pr_url, pr_title, head_branch, base_branch, author_login,
state, review_state, checks_state, mergeable_state, review_count, pending_review_count, required_reviews, comment_count,
unresolved_review_threads, checks_total, checks_passing, additions, deletions,
created_at, merged_at, closed_at, last_synced_at, updated_at)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
created_at, merged_at, closed_at, last_synced_at, detached_at, updated_at)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
tp.ID, tp.WorkspaceID, tp.TaskID, tp.RepositoryID, tp.Owner, tp.Repo, tp.PRNumber, tp.PRURL, tp.PRTitle, tp.HeadBranch, tp.BaseBranch, tp.AuthorLogin,
tp.State, tp.ReviewState, tp.ChecksState, tp.MergeableState, tp.ReviewCount, tp.PendingReviewCount, tp.RequiredReviews, tp.CommentCount,
tp.UnresolvedReviewThreads, tp.ChecksTotal, tp.ChecksPassing, tp.Additions, tp.Deletions,
tp.CreatedAt, tp.MergedAt, tp.ClosedAt, tp.LastSyncedAt, tp.UpdatedAt)
tp.CreatedAt, tp.MergedAt, tp.ClosedAt, tp.LastSyncedAt, tp.DetachedAt, tp.UpdatedAt)
return err
}
@@ -1482,7 +1485,7 @@ const taskPRColumns = `id, workspace_id, task_id, repository_id, owner, repo, pr
pr_title, head_branch, base_branch, author_login, state, review_state, checks_state,
mergeable_state, review_count, pending_review_count, required_reviews, comment_count,
unresolved_review_threads, checks_total, checks_passing, additions, deletions,
created_at, merged_at, closed_at, last_synced_at, updated_at`
created_at, merged_at, closed_at, last_synced_at, detached_at, updated_at`
// taskPRColumnsQualified is taskPRColumns with each column qualified by the
// `gtp` alias, for queries that join github_task_prs against another table.
@@ -1491,13 +1494,26 @@ const taskPRColumnsQualified = `gtp.id, gtp.workspace_id, gtp.task_id, gtp.repos
gtp.state, gtp.review_state, gtp.checks_state, gtp.mergeable_state, gtp.review_count,
gtp.pending_review_count, gtp.required_reviews, gtp.comment_count, gtp.unresolved_review_threads,
gtp.checks_total, gtp.checks_passing, gtp.additions, gtp.deletions,
gtp.created_at, gtp.merged_at, gtp.closed_at, gtp.last_synced_at, gtp.updated_at`
gtp.created_at, gtp.merged_at, gtp.closed_at, gtp.last_synced_at, gtp.detached_at, gtp.updated_at`
// GetTaskPR returns the first PR association for a task. For multi-repo tasks
// the result is non-deterministic across repos — use ListTaskPRsByTask instead.
func (s *Store) GetTaskPR(ctx context.Context, taskID string) (*TaskPR, error) {
var tp TaskPR
err := s.ro.GetContext(ctx, &tp, `SELECT `+taskPRColumns+` FROM github_task_prs WHERE task_id = ? LIMIT 1`, taskID)
err := s.ro.GetContext(ctx, &tp, `SELECT `+taskPRColumns+` FROM github_task_prs WHERE task_id = ? AND detached_at IS NULL LIMIT 1`, taskID)
if errors.Is(err, sql.ErrNoRows) {
return nil, nil
}
return &tp, err
}
// GetTaskPRByID returns an association regardless of whether it was detached.
// Mutation paths use this exact lookup so detached rows remain durable
// tombstones and can be explicitly restored by a later link action.
func (s *Store) GetTaskPRByID(ctx context.Context, associationID string) (*TaskPR, error) {
var tp TaskPR
err := s.ro.GetContext(ctx, &tp,
`SELECT `+taskPRColumns+` FROM github_task_prs WHERE id = ? LIMIT 1`, associationID)
if errors.Is(err, sql.ErrNoRows) {
return nil, nil
}
@@ -1514,7 +1530,7 @@ func (s *Store) GetTaskPR(ctx context.Context, taskID string) (*TaskPR, error) {
func (s *Store) GetTaskPRByRepository(ctx context.Context, taskID, repositoryID string) (*TaskPR, error) {
var tp TaskPR
err := s.ro.GetContext(ctx, &tp,
`SELECT `+taskPRColumns+` FROM github_task_prs WHERE task_id = ? AND repository_id = ?
`SELECT `+taskPRColumns+` FROM github_task_prs WHERE task_id = ? AND repository_id = ? AND detached_at IS NULL
ORDER BY updated_at DESC LIMIT 1`,
taskID, repositoryID)
if errors.Is(err, sql.ErrNoRows) {
@@ -1529,6 +1545,21 @@ func (s *Store) GetTaskPRByRepository(ctx context.Context, taskID, repositoryID
// the same PR number, not a sibling PR that happens to be the first
// row returned by the legacy by-repo query.
func (s *Store) GetTaskPRByRepoAndNumber(ctx context.Context, taskID, repositoryID string, prNumber int) (*TaskPR, error) {
var tp TaskPR
err := s.ro.GetContext(ctx, &tp,
`SELECT `+taskPRColumns+` FROM github_task_prs
WHERE task_id = ? AND repository_id = ? AND pr_number = ? AND detached_at IS NULL LIMIT 1`,
taskID, repositoryID, prNumber)
if errors.Is(err, sql.ErrNoRows) {
return nil, nil
}
return &tp, err
}
// GetTaskPRByRepoAndNumberIncludingDetached returns the exact association
// row, including a detached tombstone, for automatic-versus-explicit link
// decisions.
func (s *Store) GetTaskPRByRepoAndNumberIncludingDetached(ctx context.Context, taskID, repositoryID string, prNumber int) (*TaskPR, error) {
var tp TaskPR
err := s.ro.GetContext(ctx, &tp,
`SELECT `+taskPRColumns+` FROM github_task_prs
@@ -1545,7 +1576,7 @@ func (s *Store) GetTaskPRByRepoAndNumber(ctx context.Context, taskID, repository
func (s *Store) ListTaskPRsByTask(ctx context.Context, taskID string) ([]*TaskPR, error) {
var prs []TaskPR
if err := s.ro.SelectContext(ctx, &prs,
`SELECT `+taskPRColumns+` FROM github_task_prs WHERE task_id = ? ORDER BY created_at ASC`, taskID); err != nil {
`SELECT `+taskPRColumns+` FROM github_task_prs WHERE task_id = ? AND detached_at IS NULL ORDER BY created_at ASC`, taskID); err != nil {
return nil, err
}
out := make([]*TaskPR, 0, len(prs))
@@ -1555,6 +1586,44 @@ func (s *Store) ListTaskPRsByTask(ctx context.Context, taskID string) ([]*TaskPR
return out, nil
}
// DetachTaskPR marks an association as removed from active task surfaces while
// retaining the row so automatic PR discovery cannot silently resurrect it.
// 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,
`UPDATE github_task_prs SET detached_at = ?, updated_at = ?
WHERE id = ? AND detached_at IS NULL`, now, now, associationID)
if err != nil {
return nil, false, err
}
count, err := result.RowsAffected()
if err != nil {
return nil, false, err
}
tp, err := s.GetTaskPRByID(ctx, associationID)
return tp, count > 0, err
}
// RestoreTaskPR clears a detached tombstone for an explicit link action and
// refreshes the persisted fields available from the fetched GitHub PR.
func (s *Store) RestoreTaskPR(ctx context.Context, taskID, repositoryID string, pr *PR) (*TaskPR, error) {
if pr == nil {
return nil, errors.New("restore task PR: missing PR data")
}
if _, err := s.db.ExecContext(ctx,
`UPDATE github_task_prs SET owner = ?, repo = ?, pr_url = ?, pr_title = ?,
head_branch = ?, base_branch = ?, author_login = ?, state = ?, mergeable_state = ?,
additions = ?, deletions = ?, merged_at = ?, closed_at = ?, detached_at = NULL, updated_at = ?
WHERE task_id = ? AND repository_id = ? AND pr_number = ?`,
pr.RepoOwner, pr.RepoName, pr.HTMLURL, pr.Title, pr.HeadBranch, pr.BaseBranch, pr.AuthorLogin,
pr.State, pr.MergeableState, pr.Additions, pr.Deletions, pr.MergedAt, pr.ClosedAt, time.Now().UTC(),
taskID, repositoryID, pr.Number); err != nil {
return nil, err
}
return s.GetTaskPRByRepoAndNumber(ctx, taskID, repositoryID, pr.Number)
}
// ListTaskPRsByTaskIDs returns PR associations for multiple tasks. Each task
// may have multiple PRs (one per repository for multi-repo tasks); rows are
// returned grouped by task_id, ordered by created_at ascending within a group.
@@ -1563,7 +1632,7 @@ func (s *Store) ListTaskPRsByTaskIDs(ctx context.Context, taskIDs []string) (map
return make(map[string][]*TaskPR), nil
}
query, args, err := sqlx.In(
`SELECT `+taskPRColumns+` FROM github_task_prs WHERE task_id IN (?) ORDER BY created_at ASC`,
`SELECT `+taskPRColumns+` FROM github_task_prs WHERE task_id IN (?) AND detached_at IS NULL ORDER BY created_at ASC`,
taskIDs,
)
if err != nil {
@@ -1585,7 +1654,7 @@ func (s *Store) ListTaskPRsByWorkspaceID(ctx context.Context, workspaceID string
if err := s.ro.SelectContext(ctx, &prs,
`SELECT `+taskPRColumnsQualified+` FROM github_task_prs gtp
INNER JOIN tasks t ON gtp.task_id = t.id
WHERE t.workspace_id = ?
WHERE t.workspace_id = ? AND gtp.detached_at IS NULL
ORDER BY gtp.created_at ASC`, workspaceID); err != nil {
return nil, err
}
@@ -1615,7 +1684,7 @@ func (s *Store) ListTaskIDsByPRNumber(ctx context.Context, workspaceID string, p
if err := s.ro.SelectContext(ctx, &ids,
`SELECT DISTINCT gtp.task_id FROM github_task_prs gtp
INNER JOIN tasks t ON gtp.task_id = t.id
WHERE t.workspace_id = ? AND gtp.pr_number = ?`, workspaceID, prNumber); err != nil {
WHERE t.workspace_id = ? AND gtp.pr_number = ? AND gtp.detached_at IS NULL`, workspaceID, prNumber); err != nil {
return nil, err
}
return ids, nil
@@ -1673,12 +1742,12 @@ func (s *Store) ReplaceTaskPR(ctx context.Context, tp *TaskPR) error {
INSERT INTO github_task_prs (id, workspace_id, task_id, repository_id, owner, repo, pr_number, pr_url, pr_title, head_branch, base_branch, author_login,
state, review_state, checks_state, mergeable_state, review_count, pending_review_count, required_reviews, comment_count,
unresolved_review_threads, checks_total, checks_passing, additions, deletions,
created_at, merged_at, closed_at, last_synced_at, updated_at)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
created_at, merged_at, closed_at, last_synced_at, detached_at, updated_at)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
tp.ID, tp.WorkspaceID, tp.TaskID, tp.RepositoryID, tp.Owner, tp.Repo, tp.PRNumber, tp.PRURL, tp.PRTitle, tp.HeadBranch, tp.BaseBranch, tp.AuthorLogin,
tp.State, tp.ReviewState, tp.ChecksState, tp.MergeableState, tp.ReviewCount, tp.PendingReviewCount, tp.RequiredReviews, tp.CommentCount,
tp.UnresolvedReviewThreads, tp.ChecksTotal, tp.ChecksPassing, tp.Additions, tp.Deletions,
tp.CreatedAt, tp.MergedAt, tp.ClosedAt, tp.LastSyncedAt, tp.UpdatedAt); err != nil {
tp.CreatedAt, tp.MergedAt, tp.ClosedAt, tp.LastSyncedAt, tp.DetachedAt, tp.UpdatedAt); err != nil {
return err
}
return tx.Commit()
apps/backend/internal/github/store_task_pr_detach_test.go
+151
View File
@@ -0,0 +1,151 @@
package github
import (
"context"
"reflect"
"testing"
"time"
)
func TestTaskPRDetachFiltersActiveRowsAndPersistsTombstone(t *testing.T) {
store := newTestStore(t)
detacher, ok := any(store).(interface {
DetachTaskPR(context.Context, string) (*TaskPR, bool, error)
})
if !ok {
t.Fatal("Store does not implement DetachTaskPR")
}
ctx := context.Background()
now := time.Now().UTC()
first := &TaskPR{
WorkspaceID: "ws-1", TaskID: "task-1", RepositoryID: "repo-1",
Owner: "acme", Repo: "demo", PRNumber: 1, PRURL: "https://github.com/acme/demo/pull/1",
PRTitle: "old", HeadBranch: "old", BaseBranch: "main", State: "merged", CreatedAt: now,
}
second := &TaskPR{
WorkspaceID: "ws-1", TaskID: "task-1", RepositoryID: "repo-1",
Owner: "acme", Repo: "demo", PRNumber: 2, PRURL: "https://github.com/acme/demo/pull/2",
PRTitle: "new", HeadBranch: "new", BaseBranch: "main", State: "open", CreatedAt: now.Add(time.Second),
}
if err := store.CreateTaskPR(ctx, first); err != nil {
t.Fatalf("create first PR: %v", err)
}
if err := store.CreateTaskPR(ctx, second); err != nil {
t.Fatalf("create second PR: %v", err)
}
detached, transitioned, err := detacher.DetachTaskPR(ctx, first.ID)
if err != nil {
t.Fatalf("detach first PR: %v", err)
}
if !transitioned {
t.Fatal("first detach did not report a transition")
}
detachedAt := reflect.Value{}
if detached != nil {
detachedAt = reflect.ValueOf(detached).Elem().FieldByName("DetachedAt")
}
if detached == nil || detached.ID != first.ID || !detachedAt.IsValid() || detachedAt.IsNil() {
t.Fatalf("detached row = %+v, want stamped row", detached)
}
active, err := store.ListTaskPRsByTask(ctx, "task-1")
if err != nil {
t.Fatalf("list active PRs: %v", err)
}
if len(active) != 1 || active[0].ID != second.ID {
t.Fatalf("active PRs = %+v, want only second PR", active)
}
_, transitioned, err = detacher.DetachTaskPR(ctx, first.ID)
if err != nil {
t.Fatalf("repeat detach first PR: %v", err)
}
if transitioned {
t.Fatal("repeat detach reported a transition")
}
reopened, err := NewStore(store.db, store.ro)
if err != nil {
t.Fatalf("reopen store: %v", err)
}
active, err = reopened.ListTaskPRsByTask(ctx, "task-1")
if err != nil {
t.Fatalf("list active PRs after reopen: %v", err)
}
if len(active) != 1 || active[0].ID != second.ID {
t.Fatalf("active PRs after reopen = %+v, want only second PR", active)
}
}
func TestLegacyTaskPRRebuildPreservesDetachedAt(t *testing.T) {
store := newTestStore(t)
ctx := context.Background()
now := time.Now().UTC()
if _, err := store.db.ExecContext(ctx, `
INSERT INTO workspaces (id) VALUES ('ws-legacy');
INSERT INTO tasks (id, workspace_id) VALUES ('task-legacy', 'ws-legacy');
DROP TABLE github_task_prs;
CREATE TABLE github_task_prs (
id TEXT PRIMARY KEY,
workspace_id TEXT NOT NULL DEFAULT '',
task_id TEXT NOT NULL,
owner TEXT NOT NULL,
repo TEXT NOT NULL,
pr_number INTEGER NOT NULL,
pr_url TEXT NOT NULL,
pr_title TEXT NOT NULL,
head_branch TEXT NOT NULL,
base_branch TEXT NOT NULL,
author_login TEXT NOT NULL,
state TEXT NOT NULL DEFAULT 'open',
review_state TEXT NOT NULL DEFAULT '',
checks_state TEXT NOT NULL DEFAULT '',
mergeable_state TEXT NOT NULL DEFAULT '',
review_count INTEGER DEFAULT 0,
pending_review_count INTEGER DEFAULT 0,
required_reviews INTEGER,
comment_count INTEGER DEFAULT 0,
unresolved_review_threads INTEGER DEFAULT 0,
checks_total INTEGER DEFAULT 0,
checks_passing INTEGER DEFAULT 0,
additions INTEGER DEFAULT 0,
deletions INTEGER DEFAULT 0,
created_at DATETIME NOT NULL,
merged_at DATETIME,
closed_at DATETIME,
last_synced_at DATETIME,
detached_at DATETIME,
updated_at DATETIME NOT NULL,
UNIQUE(task_id, pr_number)
);
INSERT INTO github_task_prs (
id, workspace_id, task_id, owner, repo, pr_number, pr_url, pr_title,
head_branch, base_branch, author_login, state, created_at, detached_at, updated_at
) VALUES (
'legacy-detached', 'ws-legacy', 'task-legacy', 'acme', 'demo', 42,
'https://github.com/acme/demo/pull/42', 'Legacy detached PR', 'feat/legacy',
'main', 'octocat', 'merged', ?, ?, ?
)
`, now, now, now); err != nil {
t.Fatalf("seed legacy task PR schema: %v", err)
}
reopened, err := NewStore(store.db, store.ro)
if err != nil {
t.Fatalf("reopen store with legacy task PR schema: %v", err)
}
got, err := reopened.GetTaskPRByID(ctx, "legacy-detached")
if err != nil {
t.Fatalf("get migrated detached PR: %v", err)
}
if got == nil || got.DetachedAt == nil || !got.DetachedAt.Equal(now) {
t.Fatalf("migrated detached PR = %+v, want detached_at %s", got, now)
}
active, err := reopened.ListTaskPRsByTask(ctx, "task-legacy")
if err != nil {
t.Fatalf("list migrated active PRs: %v", err)
}
if len(active) != 0 {
t.Fatalf("migrated active PRs = %+v, want detached row filtered", active)
}
}
apps/backend/pkg/websocket/actions.go
+1
View File
@@ -448,6 +448,7 @@ const (
ActionGitHubPRFilesGet = "github.pr_files.get"
ActionGitHubPRCommitsGet = "github.pr_commits.get"
ActionGitHubTaskPRUpdated = "github.task_pr.updated" // Notification
ActionGitHubTaskPRDeleted = "github.task_pr.deleted" // Notification
ActionGitHubTaskCIOptionsUpdated = "github.task_ci_options.updated" // Notification
ActionGitHubRateLimitUpdated = "github.rate_limit.updated" // Notification
ActionGitHubPRFeedbackNotify = "github.pr_feedback.notify" // Notification
apps/web/components/github/multi-pr-ci-popover.tsx
+92 -24
View File
@@ -1,13 +1,14 @@
"use client";
import { useId, useState, type KeyboardEvent } from "react";
import { IconGitPullRequest } from "@tabler/icons-react";
import { IconGitPullRequest, IconLoader2, IconX } from "@tabler/icons-react";
import { cn } from "@/lib/utils";
import { PRCIPopover } from "@/components/github/pr-ci-popover";
import { getPRStatusColor, pickDefaultPR } from "@/components/github/pr-task-icon";
import { prIdentitySlug } from "@/components/github/pr-utils";
import { usePRFeedbackBackgroundSync } from "@/hooks/domains/github/use-pr-ci-popover";
import { useAppStore } from "@/components/state-provider";
import { useToast } from "@/components/toast-provider";
import type { TaskPR } from "@/lib/types/github";
/**
@@ -39,38 +40,66 @@ function PRTab({
panelId,
active,
onSelect,
canRemove,
removing,
onRemove,
}: {
pr: TaskPR;
uid: string;
panelId: string;
active: boolean;
onSelect: (pr: TaskPR) => void;
canRemove: boolean;
removing: boolean;
onRemove?: (pr: TaskPR) => void;
}) {
return (
<button
type="button"
role="tab"
id={prTabDomId(uid, pr)}
// Roving tabindex (WAI-ARIA tabs pattern): only the active tab is in the
// page tab order; ArrowLeft/ArrowRight move between tabs.
tabIndex={active ? 0 : -1}
aria-selected={active}
aria-controls={panelId}
data-testid={`pr-popover-tab-${prIdentitySlug(pr)}`}
data-active={active ? "true" : "false"}
onClick={() => onSelect(pr)}
className={cn(
"flex shrink-0 cursor-pointer items-center gap-1 rounded-md border border-transparent px-2 py-1 text-xs whitespace-nowrap transition-colors",
active
? "border-primary/50 bg-card text-foreground"
: "text-muted-foreground hover:bg-muted",
<div className="flex shrink-0 items-center rounded-md border border-transparent">
<button
type="button"
role="tab"
id={prTabDomId(uid, pr)}
// Roving tabindex (WAI-ARIA tabs pattern): only the active tab is in the
// page tab order; ArrowLeft/ArrowRight move between tabs.
tabIndex={active ? 0 : -1}
aria-selected={active}
aria-controls={panelId}
data-testid={`pr-popover-tab-${prIdentitySlug(pr)}`}
data-active={active ? "true" : "false"}
onClick={() => onSelect(pr)}
className={cn(
"flex min-h-6 cursor-pointer items-center gap-1 rounded-md border border-transparent px-2 py-1 text-xs whitespace-nowrap transition-colors [@media(pointer:coarse)]:min-h-11",
active
? "border-primary/50 bg-card text-foreground"
: "text-muted-foreground hover:bg-muted",
)}
>
<IconGitPullRequest className={cn("h-3.5 w-3.5", getPRStatusColor(pr))} />
<span className="font-medium">
{pr.repo} #{pr.pr_number}
</span>
</button>
{canRemove && (
<button
type="button"
data-testid={`pr-popover-remove-${prIdentitySlug(pr)}`}
aria-label={`Remove ${pr.repo} #${pr.pr_number} from task`}
disabled={removing}
onClick={(event) => {
event.preventDefault();
event.stopPropagation();
onRemove?.(pr);
}}
className="flex h-6 w-6 shrink-0 cursor-pointer items-center justify-center rounded-md text-muted-foreground hover:bg-muted hover:text-foreground disabled:cursor-wait disabled:opacity-60 [@media(pointer:coarse)]:h-11 [@media(pointer:coarse)]:w-11"
>
{removing ? (
<IconLoader2 className="h-3.5 w-3.5 animate-spin" aria-hidden="true" />
) : (
<IconX className="h-3.5 w-3.5" aria-hidden="true" />
)}
</button>
)}
>
<IconGitPullRequest className={cn("h-3.5 w-3.5", getPRStatusColor(pr))} />
<span className="font-medium">
{pr.repo} #{pr.pr_number}
</span>
</button>
</div>
);
}
@@ -85,11 +114,15 @@ export function MultiPRCIPopover({
enabled,
onOpenDetailPanel,
refreshTaskPR,
onRemovePR,
onCollapseFocus,
}: {
prs: TaskPR[];
enabled: boolean;
onOpenDetailPanel?: (pr: TaskPR) => void;
refreshTaskPR?: () => void;
onRemovePR?: (pr: TaskPR) => Promise<void>;
onCollapseFocus?: (remainingPR: TaskPR) => void;
}) {
// `overrideId` is only set when the user activates a tab. The displayed PR is
// derived: honour the override while it still exists, otherwise fall back to
@@ -101,9 +134,41 @@ export function MultiPRCIPopover({
// always references an element that exists in the DOM.
const uid = useId();
const panelId = `${uid}-pr-ci-tabpanel`;
const [removingId, setRemovingId] = useState<string | null>(null);
const { toast } = useToast();
const selected = prs.find((p) => p.id === overrideId) ?? pickDefaultPR(prs);
if (!selected) return null;
const removePR = async (pr: TaskPR) => {
if (!onRemovePR || removingId) return;
setRemovingId(pr.id);
try {
await onRemovePR(pr);
if (selected.id === pr.id) {
const removedIndex = prs.findIndex((candidate) => candidate.id === pr.id);
const next = prs[removedIndex + 1] ?? prs[removedIndex - 1];
if (next && prs.length > 2) {
setOverrideId(next.id);
// Keep keyboard focus on the deterministic adjacent tab while the
// multi-PR surface remains mounted.
document.getElementById(prTabDomId(uid, next))?.focus();
} else if (next) {
// The parent collapses to its single-PR surface after a two-to-one
// removal, so let it restore focus to the surviving trigger.
onCollapseFocus?.(next);
}
}
} catch (error) {
toast({
title: "Failed to unlink pull request",
description: error instanceof Error ? error.message : "The pull request is still linked.",
variant: "error",
});
} finally {
setRemovingId(null);
}
};
// ArrowLeft/ArrowRight move selection (and focus) within the tablist,
// wrapping at the ends — selection follows focus, per the ARIA tabs pattern.
const onTablistKeyDown = (e: KeyboardEvent<HTMLDivElement>) => {
@@ -138,6 +203,9 @@ export function MultiPRCIPopover({
panelId={panelId}
active={pr.id === selected.id}
onSelect={(p) => setOverrideId(p.id)}
canRemove={prs.length > 1 && Boolean(onRemovePR)}
removing={removingId === pr.id}
onRemove={(p) => void removePR(p)}
/>
))}
</div>
apps/web/components/github/pr-ci-popover.automation.test.tsx
+115 -2
View File
@@ -1,4 +1,5 @@
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { useState } from "react";
import { cleanup, render, screen, fireEvent, waitFor } from "@testing-library/react";
import { TooltipProvider } from "@kandev/ui/tooltip";
import { StateProvider } from "@/components/state-provider";
@@ -54,6 +55,8 @@ const MERGED_PROMPT_LABEL = "PR merged";
const CLOSED_PROMPT_LABEL = "PR closed without merging";
const REVIEW_REQUEST_PROMPT_LABEL = "Your review is requested";
const REVIEW_FOLLOW_UP_TRIGGER = "ci-review-follow-up-trigger";
const REMOVE_FIRST_PR_LABEL = "Remove r #1 from task";
const BACKEND_UNAVAILABLE = "backend unavailable";
function makeOptions(overrides: Partial<TaskCIAutomationOptions> = {}): TaskCIAutomationOptions {
return {
@@ -290,6 +293,116 @@ describe("PRCIPopover automation status", () => {
});
});
describe("MultiPRCIPopover unlink", () => {
beforeEach(() => {
resetHookMocks();
});
afterEach(() => {
cleanup();
});
it("unlinks one multi-PR tab while keeping its sibling visible", async () => {
const onCollapseFocus = vi.fn();
function RemovablePopover() {
const [prs, setPrs] = useState([
makePR({ id: "a", pr_number: 1, pr_title: "First PR" }),
makePR({ id: "b", pr_number: 2, pr_title: "Second PR", checks_state: "success" }),
]);
return (
<MultiPRCIPopover
prs={prs}
enabled={true}
onRemovePR={async (pr) =>
setPrs((current) => current.filter((item) => item.id !== pr.id))
}
onCollapseFocus={onCollapseFocus}
/>
);
}
render(
<TooltipProvider>
<StateProvider>
<ToastProvider>
<RemovablePopover />
</ToastProvider>
</StateProvider>
</TooltipProvider>,
);
fireEvent.click(screen.getByRole("button", { name: REMOVE_FIRST_PR_LABEL }));
await waitFor(() =>
expect(screen.queryByRole("button", { name: REMOVE_FIRST_PR_LABEL })).toBeNull(),
);
expect(screen.getByRole("tab", { name: "r #2" })).not.toBeNull();
expect(onCollapseFocus).toHaveBeenCalledWith(expect.objectContaining({ id: "b" }));
});
it("keeps a failed unlink tab and reports the error", async () => {
const onRemovePR = vi.fn().mockRejectedValue(new Error(BACKEND_UNAVAILABLE));
render(
<TooltipProvider>
<StateProvider>
<ToastProvider>
<MultiPRCIPopover
prs={[makePR({ id: "a", pr_number: 1 }), makePR({ id: "b", pr_number: 2 })]}
enabled={true}
onRemovePR={onRemovePR}
/>
</ToastProvider>
</StateProvider>
</TooltipProvider>,
);
fireEvent.click(screen.getByRole("button", { name: REMOVE_FIRST_PR_LABEL }));
await waitFor(() =>
expect(screen.getByTestId("toast-message").textContent).toContain(BACKEND_UNAVAILABLE),
);
expect(screen.getByRole("button", { name: REMOVE_FIRST_PR_LABEL })).not.toBeNull();
expect(onRemovePR).toHaveBeenCalledWith(expect.objectContaining({ id: "a" }));
});
it("focuses the next adjacent tab when a selected tab is removed from a larger set", async () => {
function RemovablePopover() {
const [prs, setPrs] = useState([
makePR({ id: "a", pr_number: 1, pr_title: "First PR", checks_state: "success" }),
makePR({ id: "b", pr_number: 2, pr_title: "Second PR" }),
makePR({ id: "c", pr_number: 3, pr_title: "Third PR", checks_state: "success" }),
]);
return (
<MultiPRCIPopover
prs={prs}
enabled={true}
onRemovePR={async (pr) =>
setPrs((current) => current.filter((item) => item.id !== pr.id))
}
/>
);
}
render(
<TooltipProvider>
<StateProvider>
<ToastProvider>
<RemovablePopover />
</ToastProvider>
</StateProvider>
</TooltipProvider>,
);
expect(screen.getByRole("tab", { name: "r #2" }).getAttribute("aria-selected")).toBe("true");
fireEvent.click(screen.getByRole("button", { name: "Remove r #2 from task" }));
await waitFor(() => expect(screen.queryByRole("tab", { name: "r #2" })).toBeNull());
const nextTab = screen.getByRole("tab", { name: "r #3" });
expect(nextTab.getAttribute("aria-selected")).toBe("true");
expect(document.activeElement).toBe(nextTab);
});
});
describe("PRCIPopover task prompts", () => {
beforeEach(() => {
resetHookMocks();
@@ -338,10 +451,10 @@ describe("PRCIPopover task prompts", () => {
});
it("offers retry after CI automation options fail to load", () => {
hookMocks.error = "backend unavailable";
hookMocks.error = BACKEND_UNAVAILABLE;
renderPopover();
expect(screen.getByText("backend unavailable")).not.toBeNull();
expect(screen.getByText(BACKEND_UNAVAILABLE)).not.toBeNull();
fireEvent.click(screen.getByRole("button", { name: "Retry" }));
expect(hookMocks.refreshMock).toHaveBeenCalledTimes(1);
apps/web/components/github/pr-status-chip.test.tsx
+6 -15
View File
@@ -129,6 +129,7 @@ afterEach(() => {
const CHIP_TESTID = "pr-status-chip";
const ATTR_PR_NUMBER = "data-pr-number";
const ATTR_PR_COUNT = "data-pr-count";
const ATTR_STATUS = "data-status";
const ATTR_READY_TO_MERGE = "data-pr-ready-to-merge";
const DRAWER_SELECTOR = "[data-testid='pr-status-chip-drawer']";
@@ -636,8 +637,7 @@ describe("PRStatusChip — multiple PRs", () => {
it("renders one aggregate chip with a PR count and worst-of status", () => {
renderWithStore(multiState(TWO_OPEN), <PRStatusChip taskId="task-1" />);
const chip = screen.getByTestId(CHIP_TESTID);
expect(chip.getAttribute("data-pr-count")).toBe("2");
// PR #2 is failing, so the aggregate glyph is red.
expect(chip.getAttribute(ATTR_PR_COUNT)).toBe("2");
expect(chip.getAttribute(ATTR_STATUS)).toBe("failed");
});
@@ -646,21 +646,14 @@ describe("PRStatusChip — multiple PRs", () => {
await expectDesktopHoverPopoverConstrained();
});
it("stays visible while at least one PR is still open", () => {
it("keeps terminal siblings in the multi-PR unlink surface", () => {
renderWithStore(
multiState([makePR({ id: "a", state: "merged" }), makePR({ id: "b", pr_number: 2 })]),
<PRStatusChip taskId="task-1" />,
);
// Only one PR is open, so it renders as the single-PR chip (its number).
expect(screen.getByTestId(CHIP_TESTID).getAttribute(ATTR_PR_NUMBER)).toBe("2");
});
it("returns null only when every PR is terminal", () => {
renderWithStore(
multiState([makePR({ id: "a", state: "merged" }), makePR({ id: "b", state: "closed" })]),
<PRStatusChip taskId="task-1" />,
);
expect(screen.queryByTestId(CHIP_TESTID)).toBeNull();
const chip = screen.getByTestId(CHIP_TESTID);
expect(chip.getAttribute(ATTR_PR_COUNT)).toBe("2");
expect(chip.getAttribute(ATTR_STATUS)).toBe("passed");
});
describe("mobile drawer", () => {
@@ -676,8 +669,6 @@ describe("PRStatusChip — multiple PRs", () => {
});
expect(document.querySelector(DRAWER_SELECTOR)).not.toBeNull();
expect(document.querySelector("[data-testid='pr-multi-popover']")).not.toBeNull();
// One tab per PR (testid is owner+repo-scoped so same-number PRs on
// different repos, or repos sharing a name across owners, stay unique).
expect(document.querySelector("[data-testid='pr-popover-tab-acme-demo-1']")).not.toBeNull();
expect(document.querySelector("[data-testid='pr-popover-tab-acme-demo-2']")).not.toBeNull();
});
apps/web/components/github/pr-status-chip.tsx
+70 -27
View File
@@ -46,8 +46,8 @@ import type { TaskCIAutomationOptions, TaskPR } from "@/lib/types/github";
const HOVER_OPEN_DELAY_MS = 150;
const HOVER_CLOSE_DELAY_MS = 150;
// Terminal states (merged / closed) never reach here — PRStatusChip returns
// null for them before rendering — so the chip status union omits them.
// Terminal states (merged / closed) are omitted from CI status aggregation,
// but remain in multi-PR surfaces so users can unlink old associations.
type ChipStatus =
| "passed"
| "failed"
@@ -63,17 +63,27 @@ type AutomationFlags = {
autoMerge: boolean;
autoFixRound: AutoFixRoundInfo | null;
};
type TriggerRef = { current: HTMLButtonElement | null };
type SingleChipProps = {
pr: TaskPR;
automation: AutomationFlags;
refreshTaskPR: () => void;
triggerRef?: TriggerRef;
};
type MultiChipProps = {
prs: TaskPR[];
statusPrs?: TaskPR[];
automation: AutomationFlags;
refreshTaskPR: () => void;
onRemovePR?: (pr: TaskPR) => Promise<void>;
triggerRef?: TriggerRef;
};
function focusAfterCollapse(triggerRef?: TriggerRef) {
if (!triggerRef) return;
setTimeout(() => triggerRef.current?.focus(), 0);
}
function chipStatus(pr: TaskPR): ChipStatus {
if (pr.review_state === "changes_requested" || pr.checks_state === "failure") return "failed";
// Merge conflicts / behind-base block the merge even when CI is green — the
@@ -132,8 +142,9 @@ const CHIP_BUTTON_CLASS =
* via hover. Returns the trigger ref plus a memoised handler that reads the ref
* lazily (inside the callback, never during render).
*/
function useChipTriggerGuard() {
const ref = useRef<HTMLButtonElement>(null);
function useChipTriggerGuard(externalRef?: TriggerRef) {
const fallbackRef = useRef<HTMLButtonElement>(null);
const ref = externalRef ?? fallbackRef;
const onPointerDownOutside = useCallback(
(e: { target: EventTarget | null; preventDefault: () => void }) => {
if (ref.current && ref.current.contains(e.target as Node)) {
@@ -171,42 +182,46 @@ function useChipPopoverInteractions() {
* Mobile: tapping opens the same popover content inside a bottom-sheet Drawer
* — hover is unreachable on touch devices.
*
* Returns null when the task has no PR yet, or once the PR reaches a terminal
* state (merged / closed) — the chat-input banner already conveys that, so the
* CI chip would be redundant.
* Returns null when the task has no PR yet, or when its only PR is terminal
* (merged / closed). With multiple associations, terminal PRs stay in the
* multi-PR surface so old links can still be removed while CI status continues
* to reflect open PRs only.
*/
export function PRStatusChip({ taskId }: { taskId: string | null }) {
const workspaceId = useAppStore((state) => state.workspaces.activeId);
const { prs, refresh } = useTaskPR(taskId);
const { prs, refresh, unlink } = useTaskPR(taskId);
const { options: automationOptions } = useTaskCIAutomationOptions(taskId);
const triggerRef = useRef<HTMLButtonElement>(null);
// Defensive Array.isArray: a partial hydration can briefly seed the store
// with a non-array value (same guard as PRTaskIcon).
// Only open PRs are worth a CI chip — terminal PRs (merged/closed) are
// already conveyed by the chat-input banner. With multiple PRs the chip
// stays visible as long as at least one is still open.
const openPRs = Array.isArray(prs)
? prs.filter((p) => p.state !== "merged" && p.state !== "closed")
: [];
const allPRs = Array.isArray(prs) ? prs : [];
// Terminal PRs are excluded from CI status and background warming, but are
// kept in the multi-PR association list so old links remain unlinkable.
const openPRs = allPRs.filter((p) => p.state !== "merged" && p.state !== "closed");
// Subscribe at the chip level so the cache warms even when the top-bar PR
// button isn't mounted (e.g. small viewport that hides it). Warm the PR the
// popover will actually open first (worst-status via pickDefaultPR — for a
// single PR that's just the PR itself); the remaining PRs in a multi-PR
// task warm when the popover opens.
usePRFeedbackBackgroundSync(workspaceId, pickDefaultPR(openPRs));
if (openPRs.length === 0) return null;
if (openPRs.length === 1)
if (allPRs.length === 0 || (allPRs.length === 1 && openPRs.length === 0)) return null;
if (allPRs.length === 1)
return (
<PRStatusChipInner
pr={openPRs[0]}
automation={automationForPR(automationOptions, openPRs[0])}
refreshTaskPR={refresh}
triggerRef={triggerRef}
/>
);
return (
<PRStatusChipMultiInner
prs={openPRs}
prs={allPRs}
statusPrs={openPRs}
automation={automationForPRs(automationOptions, openPRs)}
refreshTaskPR={refresh}
onRemovePR={(pr) => unlink(pr.id)}
triggerRef={triggerRef}
/>
);
}
@@ -328,9 +343,9 @@ function PRStatusChipInner(props: SingleChipProps) {
return <PRStatusChipHoverCard {...props} />;
}
function PRStatusChipHoverCard({ pr, automation, refreshTaskPR }: SingleChipProps) {
function PRStatusChipHoverCard({ pr, automation, refreshTaskPR, triggerRef }: SingleChipProps) {
const status = chipStatus(pr);
const { ref, onPointerDownOutside } = useChipTriggerGuard();
const { ref, onPointerDownOutside } = useChipTriggerGuard(triggerRef);
const { open, onOpenChange, onTriggerEnter, onTriggerLeave, onContentEnter, onContentLeave } =
useChipPopoverInteractions();
return (
@@ -420,9 +435,16 @@ function MultiChipGlyph({
);
}
function PRStatusChipMultiHoverCard({ prs, automation, refreshTaskPR }: MultiChipProps) {
const status = aggregateChipStatus(prs);
const { ref, onPointerDownOutside } = useChipTriggerGuard();
function PRStatusChipMultiHoverCard({
prs,
statusPrs,
automation,
refreshTaskPR,
onRemovePR,
triggerRef,
}: MultiChipProps) {
const status = aggregateChipStatus(statusPrs ?? prs);
const { ref, onPointerDownOutside } = useChipTriggerGuard(triggerRef);
const { open, onOpenChange, onTriggerEnter, onTriggerLeave, onContentEnter, onContentLeave } =
useChipPopoverInteractions();
return (
@@ -457,18 +479,32 @@ function PRStatusChipMultiHoverCard({ prs, automation, refreshTaskPR }: MultiChi
onPointerDownOutside={onPointerDownOutside}
onOpenAutoFocus={(e) => e.preventDefault()}
>
<MultiPRCIPopover prs={prs} enabled={open} refreshTaskPR={refreshTaskPR} />
<MultiPRCIPopover
prs={prs}
enabled={open}
refreshTaskPR={refreshTaskPR}
onRemovePR={onRemovePR}
onCollapseFocus={() => focusAfterCollapse(triggerRef)}
/>
</PopoverContent>
</Popover>
);
}
function PRStatusChipMultiDrawer({ prs, automation, refreshTaskPR }: MultiChipProps) {
const status = aggregateChipStatus(prs);
function PRStatusChipMultiDrawer({
prs,
statusPrs,
automation,
refreshTaskPR,
onRemovePR,
triggerRef,
}: MultiChipProps) {
const status = aggregateChipStatus(statusPrs ?? prs);
const [open, setOpen] = useState(false);
return (
<Drawer open={open} onOpenChange={setOpen}>
<button
ref={triggerRef}
type="button"
aria-haspopup="dialog"
aria-expanded={open}
@@ -496,19 +532,26 @@ function PRStatusChipMultiDrawer({ prs, automation, refreshTaskPR }: MultiChipPr
</DrawerClose>
</DrawerHeader>
<div className="flex-1 min-h-0 overflow-y-auto p-3" data-vaul-no-drag>
<MultiPRCIPopover prs={prs} enabled={open} refreshTaskPR={refreshTaskPR} />
<MultiPRCIPopover
prs={prs}
enabled={open}
refreshTaskPR={refreshTaskPR}
onRemovePR={onRemovePR}
onCollapseFocus={() => focusAfterCollapse(triggerRef)}
/>
</div>
</DrawerContent>
</Drawer>
);
}
function PRStatusChipDrawer({ pr, automation, refreshTaskPR }: SingleChipProps) {
function PRStatusChipDrawer({ pr, automation, refreshTaskPR, triggerRef }: SingleChipProps) {
const status = chipStatus(pr);
const [open, setOpen] = useState(false);
return (
<Drawer open={open} onOpenChange={setOpen}>
<button
ref={triggerRef}
type="button"
aria-haspopup="dialog"
aria-expanded={open}
apps/web/components/github/pr-topbar-button.tsx
+38 -6
View File
@@ -1,6 +1,6 @@
"use client";
import { memo, useState, type ReactNode } from "react";
import { memo, useRef, useState, type ReactNode } from "react";
import {
IconGitPullRequest,
IconCheck,
@@ -42,6 +42,12 @@ import type { TaskPR } from "@/lib/types/github";
const POPOVER_OPEN_DELAY_MS = 150;
const POPOVER_CLOSE_DELAY_MS = 150;
type TriggerRef = { current: HTMLButtonElement | null };
function focusAfterCollapse(triggerRef?: TriggerRef) {
if (!triggerRef) return;
setTimeout(() => triggerRef.current?.focus(), 0);
}
// Badge for the hard merge blockers that must beat ready/awaiting-review:
// conflicts (red) and behind-base (amber). Mirrors openMergeBlockerColor so
@@ -100,11 +106,15 @@ export const PRTopbarButton = memo(function PRTopbarButton() {
// useTaskPR fetches if not in store and returns the full per-task list so
// multi-repo tasks can surface every PR (one button for single-repo, a
// dropdown summary for 2+ so the topbar doesn't blow up horizontally).
const { prs, refresh } = useTaskPR(activeTaskId);
const { prs, refresh, unlink } = useTaskPR(activeTaskId);
const triggerRef = useRef<HTMLButtonElement>(null);
if (prs.length === 0) return null;
if (prs.length === 1) return <PRSingleButton pr={prs[0]} refreshTaskPR={refresh} />;
return <PRMultiButton prs={prs} refreshTaskPR={refresh} />;
if (prs.length === 1)
return <PRSingleButton pr={prs[0]} refreshTaskPR={refresh} triggerRef={triggerRef} />;
return (
<PRMultiButton prs={prs} refreshTaskPR={refresh} onRemovePR={unlink} triggerRef={triggerRef} />
);
});
/**
@@ -128,7 +138,15 @@ function usePopoverInteractions() {
return { usesTouchDrawer, ...hover };
}
function PRSingleButton({ pr, refreshTaskPR }: { pr: TaskPR; refreshTaskPR: () => void }) {
function PRSingleButton({
pr,
refreshTaskPR,
triggerRef,
}: {
pr: TaskPR;
refreshTaskPR: () => void;
triggerRef?: TriggerRef;
}) {
const addPRPanel = useDockviewStore((s) => s.addPRPanel);
const tooltip = `${pr.owner}/${pr.repo} #${pr.pr_number} — ${pr.pr_title}`;
const {
@@ -146,6 +164,7 @@ function PRSingleButton({ pr, refreshTaskPR }: { pr: TaskPR; refreshTaskPR: () =
const button = (
<Button
ref={triggerRef}
data-testid="pr-topbar-button"
data-pr-number={pr.pr_number}
data-pr-state={pr.state}
@@ -202,7 +221,17 @@ function PRSingleButton({ pr, refreshTaskPR }: { pr: TaskPR; refreshTaskPR: () =
);
}
function PRMultiButton({ prs, refreshTaskPR }: { prs: TaskPR[]; refreshTaskPR: () => void }) {
function PRMultiButton({
prs,
refreshTaskPR,
onRemovePR,
triggerRef,
}: {
prs: TaskPR[];
refreshTaskPR: () => void;
onRemovePR: (associationId: string) => Promise<void>;
triggerRef?: TriggerRef;
}) {
// Click still drives the dropdown (the explicit "jump to this PR's panel"
// affordance, and the only interaction on touch). Hover adds the aggregate
// CI popover with a tab per PR — desktop only, suppressed on touch where
@@ -229,6 +258,7 @@ function PRMultiButton({ prs, refreshTaskPR }: { prs: TaskPR[]; refreshTaskPR: (
const triggerButton = (
<DropdownMenuTrigger asChild>
<Button
ref={triggerRef}
data-testid="pr-topbar-button"
data-pr-count={prs.length}
size="sm"
@@ -288,6 +318,8 @@ function PRMultiButton({ prs, refreshTaskPR }: { prs: TaskPR[]; refreshTaskPR: (
prs={prs}
enabled={open}
refreshTaskPR={refreshTaskPR}
onRemovePR={(pr) => onRemovePR(pr.id)}
onCollapseFocus={() => focusAfterCollapse(triggerRef)}
onOpenDetailPanel={(pr) => {
addPRPanel(prTaskKey(pr));
onOpenChange(false);
apps/web/e2e/pages/session-page.ts
+6
View File
@@ -669,6 +669,12 @@ export class SessionPage {
return this.page.getByTestId(`pr-popover-tab-${owner}-${repo}-${prNumber}`);
}
/** Unlink control for one PR association in the multi-PR popover. */
prMultiPopoverRemove(owner: string, repo: string, prNumber: number): Locator {
const activePopover = this.page.locator("[data-testid='pr-multi-popover']:visible").last();
return activePopover.getByTestId(`pr-popover-remove-${owner}-${repo}-${prNumber}`);
}
/**
* A specific bucket group inside the popover by kind.
*
apps/web/e2e/tests/pr/mobile-pr-ci-chip.spec.ts
+103
View File
@@ -46,6 +46,7 @@ async function seedTaskWithPR({
},
);
await apiClient.mockGitHubAssociateTaskPR({
workspace_id: seedData.workspaceId,
task_id: task.id,
owner: OWNER,
repo: REPO,
@@ -79,6 +80,29 @@ async function seedTaskWithPRAndTodos(args: SeedTaskArgs): Promise<string> {
});
}
async function seedTaskWithTwoPRs(args: SeedTaskArgs): Promise<string> {
const taskId = await seedTaskWithPR(args);
await args.apiClient.mockGitHubAssociateTaskPR({
workspace_id: args.seedData.workspaceId,
task_id: taskId,
owner: OWNER,
repo: "api",
pr_number: 100,
pr_url: `https://github.com/${OWNER}/api/pull/100`,
pr_title: "Follow-up mobile PR",
head_branch: "feat/mobile-follow-up",
base_branch: "main",
author_login: "test-user",
state: "open",
checks_state: "success",
checks_total: 2,
checks_passing: 2,
review_state: "approved",
review_count: 1,
});
return taskId;
}
async function openTask(testPage: import("@playwright/test").Page, taskId: string) {
await testPage.goto(`/t/${taskId}`);
const session = new SessionPage(testPage);
@@ -199,4 +223,83 @@ test.describe("mobile PR CI chip drawer", () => {
await session.prStatusChipDrawerClose().tap();
await expect(session.prStatusChipDrawer()).toHaveCount(0, { timeout: 5_000 });
});
test("unlink control works inside the multi-PR drawer with a touch-sized target", async ({
testPage,
apiClient,
seedData,
prCapture,
}) => {
test.setTimeout(120_000);
const taskId = await seedTaskWithTwoPRs({
apiClient,
seedData,
title: "Mobile multi PR unlink",
prOverrides: {
checks_state: "success",
checks_total: 2,
checks_passing: 2,
},
});
const session = await openTask(testPage, taskId);
await expect(session.prStatusChip()).toHaveAttribute("data-pr-count", "2", {
timeout: 15_000,
});
await session.tapPRStatusChip();
await prCapture.screenshot("mobile-pr-unlink-drawer", {
caption: "Mobile multi-PR drawer with touch-sized unlink controls",
});
const removeFirst = session.prMultiPopoverRemove(OWNER, REPO, PR_NUMBER);
await expect(removeFirst).toBeVisible();
const box = await removeFirst.boundingBox();
expect(box?.height).toBeGreaterThanOrEqual(44);
await removeFirst.tap();
await expect(session.prStatusChip()).toHaveAttribute("data-pr-number", "100");
await expect(session.prStatusChip()).toBeFocused();
await expect(session.prMultiPopoverRemove(OWNER, REPO, PR_NUMBER)).toHaveCount(0);
await expect(session.prStatusChipDrawer()).toHaveCount(0);
await expect
.poll(() =>
testPage.evaluate(() => document.documentElement.scrollWidth <= window.innerWidth),
)
.toBe(true);
await testPage.reload();
const reloaded = new SessionPage(testPage);
await reloaded.waitForLoad();
await expect(reloaded.prStatusChip()).toHaveAttribute("data-pr-number", "100");
});
test("keeps a terminal sibling unlinkable on touch", async ({
testPage,
apiClient,
seedData,
}) => {
test.setTimeout(120_000);
const taskId = await seedTaskWithTwoPRs({
apiClient,
seedData,
title: "Mobile terminal PR unlink",
prOverrides: {
state: "merged",
},
});
const session = await openTask(testPage, taskId);
await expect(session.prStatusChip()).toHaveAttribute("data-pr-count", "2", {
timeout: 15_000,
});
await session.tapPRStatusChip();
const removeMerged = session.prMultiPopoverRemove(OWNER, REPO, PR_NUMBER);
await expect(removeMerged).toBeVisible();
await removeMerged.tap();
await expect(session.prStatusChip()).toHaveAttribute("data-pr-number", "100");
await expect(session.prMultiPopoverRemove(OWNER, REPO, PR_NUMBER)).toHaveCount(0);
await expect(session.prStatusChipDrawer()).toHaveCount(0);
});
});
apps/web/e2e/tests/pr/pr-multi-popover.spec.ts
+48 -6
View File
@@ -94,8 +94,9 @@ async function seedTask(
}
/** Associate two open PRs with the task: web#42 failing, api#77 all green. */
async function associateTwoPRs(apiClient: ApiClient, taskId: string) {
async function associateTwoPRs(apiClient: ApiClient, workspaceId: string, taskId: string) {
await apiClient.mockGitHubAssociateTaskPR({
workspace_id: workspaceId,
task_id: taskId,
owner: OWNER,
repo: "web",
@@ -113,6 +114,7 @@ async function associateTwoPRs(apiClient: ApiClient, taskId: string) {
review_count: 1,
});
await apiClient.mockGitHubAssociateTaskPR({
workspace_id: workspaceId,
task_id: taskId,
owner: OWNER,
repo: "api",
@@ -166,7 +168,7 @@ test.describe("Multi-PR CI popover", () => {
seedData.repositoryId,
title,
);
await associateTwoPRs(apiClient, seed.taskId);
await associateTwoPRs(apiClient, seedData.workspaceId, seed.taskId);
const session = await openTaskAndWait(testPage, apiClient, seed, title);
await expect(session.prTopbarButton()).toHaveAttribute("data-pr-count", "2");
@@ -209,7 +211,7 @@ test.describe("Multi-PR CI popover", () => {
seedData.repositoryId,
title,
);
await associateTwoPRs(apiClient, seed.taskId);
await associateTwoPRs(apiClient, seedData.workspaceId, seed.taskId);
const session = await openTaskAndWait(testPage, apiClient, seed, title);
const chip = session.prStatusChip();
@@ -226,6 +228,46 @@ test.describe("Multi-PR CI popover", () => {
);
});
test("unlinks one PR tab and keeps the association detached after reload", async ({
testPage,
apiClient,
seedData,
prCapture,
}) => {
test.setTimeout(120_000);
const title = "Multi Popover Unlink";
const seed = await seedTask(
apiClient,
seedData.workspaceId,
seedData.agentProfileId,
seedData.repositoryId,
title,
);
await associateTwoPRs(apiClient, seedData.workspaceId, seed.taskId);
const session = await openTaskAndWait(testPage, apiClient, seed, title);
await session.hoverPRTopbar();
await prCapture.screenshot("desktop-pr-unlink-popover", {
caption: "Desktop multi-PR popover with per-tab unlink controls",
});
const removeWeb = session.prMultiPopoverRemove(OWNER, "web", 42);
await expect(removeWeb).toBeVisible();
await removeWeb.click();
// Removing the first association collapses the multi-PR control to the
// remaining single PR without touching the remote PR itself.
await expect(session.prTopbarButton()).toHaveAttribute("data-pr-number", "77");
await expect(session.prTopbarButton()).toBeFocused();
await expect(session.prMultiPopoverRemove(OWNER, "web", 42)).toHaveCount(0);
// The tombstone is persisted in Kandev, so a fresh page load does not
// resurrect the older merged/follow-up PR association.
await testPage.reload();
const reloaded = new SessionPage(testPage);
await reloaded.waitForLoad();
await expect(reloaded.prTopbarButton()).toHaveAttribute("data-pr-number", "77");
});
test("clicking the multi-PR button still opens the dropdown with per-PR rows", async ({
testPage,
apiClient,
@@ -240,7 +282,7 @@ test.describe("Multi-PR CI popover", () => {
seedData.repositoryId,
title,
);
await associateTwoPRs(apiClient, seed.taskId);
await associateTwoPRs(apiClient, seedData.workspaceId, seed.taskId);
const session = await openTaskAndWait(testPage, apiClient, seed, title);
await session.prTopbarButton().click();
@@ -262,7 +304,7 @@ test.describe("Multi-PR CI popover", () => {
seedData.repositoryId,
title,
);
await associateTwoPRs(apiClient, seed.taskId);
await associateTwoPRs(apiClient, seedData.workspaceId, seed.taskId);
const session = await openTaskAndWait(testPage, apiClient, seed, title);
const expectedMuted = await mutedBackgroundColor(testPage);
const expectedForeground = await foregroundColor(testPage);
@@ -300,7 +342,7 @@ test.describe("Multi-PR CI popover", () => {
seedData.repositoryId,
title,
);
await associateTwoPRs(apiClient, seed.taskId);
await associateTwoPRs(apiClient, seedData.workspaceId, seed.taskId);
const session = await openTaskAndWait(testPage, apiClient, seed, title);
// The layout-owned canonical panel follows the primary/first-associated PR (web#42).
apps/web/hooks/domains/github/use-task-pr.test.tsx
+64
View File
@@ -2,8 +2,11 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { createElement, type ReactNode } from "react";
import { act, cleanup, renderHook } from "@testing-library/react";
import { StateProvider } from "@/components/state-provider";
import type { AppState } from "@/lib/state/store";
import type { TaskPR } from "@/lib/types/github";
const requestMock = vi.fn();
const deleteTaskPRMock = vi.hoisted(() => vi.fn());
vi.mock("@/lib/ws/connection", () => ({
getWebSocketClient: () => ({ request: requestMock }),
@@ -13,6 +16,7 @@ vi.mock("@/lib/ws/connection", () => ({
// here). Stub it so the module import doesn't fail in jsdom.
vi.mock("@/lib/api/domains/github-api", () => ({
listWorkspaceTaskPRs: vi.fn().mockResolvedValue(null),
deleteTaskPR: deleteTaskPRMock,
}));
import { useTaskPR } from "./use-task-pr";
@@ -22,6 +26,12 @@ function wrapper({ children }: { children: ReactNode }) {
return createElement(StateProvider, null, children);
}
function createStateWrapper(initialState: Partial<AppState>) {
return function StateTestWrapper({ children }: { children: ReactNode }) {
return createElement(StateProvider, { initialState, children });
};
}
// Renders the hook alongside the store api so a test can drive
// `connection.status` transitions the way `useWebSocket` does in prod.
function useTaskPRWithStore(taskId: string | null) {
@@ -30,9 +40,20 @@ function useTaskPRWithStore(taskId: string | null) {
return { result, store };
}
const linkedPR = { id: "association-1", task_id: "task-1" } as TaskPR;
function unlinkState(workspaceId: string | null): Partial<AppState> {
return {
workspaces: { items: [], activeId: workspaceId },
taskPRs: { byTaskId: { "task-1": [linkedPR] } },
};
}
beforeEach(() => {
vi.useFakeTimers();
requestMock.mockReset();
requestMock.mockResolvedValue({ prs: [] });
deleteTaskPRMock.mockReset();
});
afterEach(() => {
cleanup();
@@ -90,6 +111,49 @@ describe("useTaskPR — permanent flag", () => {
});
});
describe("useTaskPR — unlink", () => {
it("rejects without a task or active workspace and skips the API", async () => {
const noTask = renderHook(() => useTaskPR(null), {
wrapper: createStateWrapper(unlinkState("ws-1")),
});
await expect(noTask.result.current.unlink(linkedPR.id)).rejects.toThrow(
"No active workspace is selected.",
);
const noWorkspace = renderHook(() => useTaskPR("task-1"), {
wrapper: createStateWrapper(unlinkState(null)),
});
await expect(noWorkspace.result.current.unlink(linkedPR.id)).rejects.toThrow(
"No active workspace is selected.",
);
expect(deleteTaskPRMock).not.toHaveBeenCalled();
});
it("deletes through the active workspace before removing local state", async () => {
deleteTaskPRMock.mockResolvedValue(undefined);
const view = renderHook(() => useTaskPRWithStore("task-1"), {
wrapper: createStateWrapper(unlinkState("ws-1")),
});
await act(async () => {
await view.result.current.result.unlink(linkedPR.id);
});
expect(deleteTaskPRMock).toHaveBeenCalledWith(linkedPR.id, "ws-1");
expect(view.result.current.store.getState().taskPRs.byTaskId["task-1"]).toBeUndefined();
});
it("propagates API failures without removing the local association", async () => {
deleteTaskPRMock.mockRejectedValue(new Error("unlink failed"));
const view = renderHook(() => useTaskPRWithStore("task-1"), {
wrapper: createStateWrapper(unlinkState("ws-1")),
});
await expect(view.result.current.result.unlink(linkedPR.id)).rejects.toThrow("unlink failed");
expect(view.result.current.store.getState().taskPRs.byTaskId["task-1"]).toEqual([linkedPR]);
});
});
describe("useTaskPR — reconnect resync", () => {
// Reproduces the missing "Pull Request" tab: the task is opened while the
// WS is down, all 6 retry attempts elapse without a response, and the store
apps/web/hooks/domains/github/use-task-pr.ts
+14 -2
View File
@@ -1,7 +1,7 @@
"use client";
import { useEffect, useCallback, useRef, useState } from "react";
import { listWorkspaceTaskPRs } from "@/lib/api/domains/github-api";
import { deleteTaskPR, listWorkspaceTaskPRs } from "@/lib/api/domains/github-api";
import { getWebSocketClient } from "@/lib/ws/connection";
import { useAppStore } from "@/components/state-provider";
import type { TaskPR } from "@/lib/types/github";
@@ -76,6 +76,8 @@ export function useTaskPR(taskId: string | null) {
const prs = useAppStore((state) => (taskId ? (state.taskPRs.byTaskId[taskId] ?? null) : null));
const pr = getPrimaryTaskPR(prs ?? undefined);
const setTaskPR = useAppStore((state) => state.setTaskPR);
const removeTaskPR = useAppStore((state) => state.removeTaskPR);
const workspaceId = useAppStore((state) => state.workspaces.activeId);
const connectionStatus = useAppStore((state) => state.connection.status);
const retryRef = useRef(0);
const permanentRef = useRef(false);
@@ -136,6 +138,15 @@ export function useTaskPR(taskId: string | null) {
});
}, [taskId, setTaskPR]);
const unlink = useCallback(
async (associationId: string) => {
if (!taskId || !workspaceId) throw new Error("No active workspace is selected.");
await deleteTaskPR(associationId, workspaceId);
removeTaskPR(taskId, associationId);
},
[removeTaskPR, taskId, workspaceId],
);
// Reset retry/permanent state when taskId changes. Bumping requestRef
// here invalidates any still-in-flight .then() closure from the prior
// taskId so it can't write to the new task's refs.
@@ -198,10 +209,11 @@ export function useTaskPR(taskId: string | null) {
setReconnectGeneration((g) => g + 1);
}, [taskId, connectionStatus, refresh]);
return { pr, prs: prs ?? [], refresh } as {
return { pr, prs: prs ?? [], refresh, unlink } as {
pr: TaskPR | null;
prs: TaskPR[];
refresh: () => void;
unlink: (associationId: string) => Promise<void>;
};
}
apps/web/lib/api/domains/github-api.test.ts
+15
View File
@@ -9,6 +9,7 @@ import {
configureGitHubToken,
copyGitHubWorkspaceSettings,
createTaskPR,
deleteTaskPR,
deletePRWatch,
deleteGitHubAppRegistration,
disconnectGitHubPersonal,
@@ -529,6 +530,20 @@ describe("task issue link helpers", () => {
});
});
describe("task PR association helpers", () => {
it("deletes a task PR association scoped to the workspace", async () => {
fetchSpy.mockResolvedValueOnce(jsonResponse({ deleted: true }));
await deleteTaskPR("association/1", "workspace/1");
const call = fetchSpy.mock.calls.at(-1);
expect(String(call?.[0])).toBe(
"http://api.test/api/v1/github/task-prs/association%2F1?workspace_id=workspace%2F1",
);
expect(call?.[1]?.method).toBe("DELETE");
});
});
describe("fetchAccessibleRepos — errors & signal", () => {
it("throws GitHubUnavailableError on 503 with code: github_not_configured", async () => {
fetchSpy.mockResolvedValueOnce(
apps/web/lib/api/domains/github-api.ts
+18
View File
@@ -89,6 +89,24 @@ export async function createTaskPR(
});
}
export async function deleteTaskPR(
associationId: string,
workspaceId: string,
options?: ApiRequestOptions,
) {
const query = new URLSearchParams({ workspace_id: workspaceId });
return fetchJson<{ deleted: boolean }>(
`/api/v1/github/task-prs/${encodeURIComponent(associationId)}?${query.toString()}`,
{
...options,
init: {
...(options?.init ?? {}),
method: "DELETE",
},
},
);
}
export async function linkTaskIssue(
taskId: string,
data: { issue: string; owner?: string; repo?: string; number?: number },
apps/web/lib/state/slices/github/github-slice.test.ts
+27
View File
@@ -272,6 +272,33 @@ describe("setTaskPR", () => {
});
});
describe("removeTaskPR", () => {
it("removes only the selected association and preserves sibling tasks", () => {
const store = makeStore();
const first = makePR({ id: "remove", pr_number: 1 });
const sibling = makePR({ id: "keep", pr_number: 2 });
const otherTask = makePR({ id: "other", task_id: "task-2", pr_number: 3 });
store.getState().setTaskPRs({
"task-1": [first, sibling],
"task-2": [otherTask],
});
store.getState().removeTaskPR("task-1", "remove");
expect(store.getState().taskPRs.byTaskId["task-1"]?.map((pr) => pr.id)).toEqual(["keep"]);
expect(store.getState().taskPRs.byTaskId["task-2"]?.map((pr) => pr.id)).toEqual(["other"]);
});
it("removes an empty task bucket after the last association is detached", () => {
const store = makeStore();
store.getState().setTaskPRs({ "task-1": [makePR({ id: "remove" })] });
store.getState().removeTaskPR("task-1", "remove");
expect(store.getState().taskPRs.byTaskId["task-1"]).toBeUndefined();
});
});
describe("setTaskIssues", () => {
const link: TaskIssueLink = {
task_id: "task-1",
apps/web/lib/state/slices/github/github-slice.ts
+14 -1
View File
@@ -110,13 +110,26 @@ function createTaskPRActions(
set: ImmerSet,
): Pick<
GitHubSlice,
"setTaskPRs" | "setTaskPR" | "setPendingPrUrlForTask" | "setTaskIssues" | "upsertTaskIssue"
| "setTaskPRs"
| "removeTaskPR"
| "setTaskPR"
| "setPendingPrUrlForTask"
| "setTaskIssues"
| "upsertTaskIssue"
> {
return {
setTaskPRs: (prs) =>
set((draft) => {
draft.taskPRs.byTaskId = prs;
}),
removeTaskPR: (taskId, associationId) =>
set((draft) => {
const current = draft.taskPRs.byTaskId[taskId];
if (!Array.isArray(current)) return;
const remaining = current.filter((pr) => pr.id !== associationId);
if (remaining.length === 0) delete draft.taskPRs.byTaskId[taskId];
else draft.taskPRs.byTaskId[taskId] = remaining;
}),
setTaskIssues: (workspaceId, issues) =>
set((draft) => {
draft.taskIssues.workspaceId = workspaceId;
apps/web/lib/state/slices/github/types.ts
+1
View File
@@ -117,6 +117,7 @@ export type GitHubSliceActions = {
setGitHubAppRegistrationsLoading: (workspaceId: string, loading: boolean) => void;
resetGitHubAppRegistrations: (workspaceId: string) => void;
setTaskPRs: (prs: Record<string, TaskPR[]>) => void;
removeTaskPR: (taskId: string, associationId: string) => void;
setTaskIssues: (workspaceId: string, issues: Record<string, TaskIssueLink>) => void;
upsertTaskIssue: (workspaceId: string, issue: TaskIssueLink) => void;
setTaskPR: (taskId: string, pr: TaskPR) => void;
apps/web/lib/types/backend.ts
+7 -1
View File
@@ -21,7 +21,12 @@ import type {
} from "@/lib/types/http";
import type { SecretListItem } from "@/lib/types/http-secrets";
import type { GitEventPayload } from "@/lib/types/git-events";
import type { GitHubRateLimitUpdate, TaskCIAutomationOptions, TaskPR } from "@/lib/types/github";
import type {
GitHubRateLimitUpdate,
TaskCIAutomationOptions,
TaskPR,
TaskPRDeletedEvent,
} from "@/lib/types/github";
import type { TaskMR } from "@/lib/types/gitlab";
import type { TaskStatusSummary } from "@/lib/types/task-status-summary";
import type { SystemMetricsSnapshot } from "./system";
@@ -623,6 +628,7 @@ export type BackendMessageMap = OfficeBackendMessageMap &
QueueStatusChangedPayload
>;
"github.task_pr.updated": BackendMessage<"github.task_pr.updated", TaskPR>;
"github.task_pr.deleted": BackendMessage<"github.task_pr.deleted", TaskPRDeletedEvent>;
"github.task_ci_options.updated": BackendMessage<
"github.task_ci_options.updated",
TaskCIAutomationOptions
apps/web/lib/types/github.ts
+7
View File
@@ -256,6 +256,13 @@ export type TaskPR = {
updated_at: string;
};
/** Workspace-scoped websocket payload emitted when a task PR association is detached. */
export type TaskPRDeletedEvent = {
workspace_id: string;
task_id: string;
association_id: string;
};
export type TaskCIPRAutomationState = {
task_id: string;
repository_id: string;
apps/web/lib/ws/handlers/github.test.ts
+26
View File
@@ -12,6 +12,32 @@ const baseStatus: GitHubStatus = {
};
describe("registerGitHubHandlers", () => {
it("removes the detached PR association without touching sibling PRs", () => {
const store = createAppStore();
const first = {
id: "association-1",
task_id: "task-1",
owner: "acme",
repo: "kandev",
pr_number: 1,
};
const sibling = { ...first, id: "association-2", pr_number: 2 };
store.getState().setTaskPRs({ "task-1": [first as never, sibling as never] });
const handler = registerGitHubHandlers(store)["github.task_pr.deleted"]!;
handler({
payload: {
workspace_id: "workspace-1",
task_id: "task-1",
association_id: "association-1",
},
} as Parameters<typeof handler>[0]);
expect(store.getState().taskPRs.byTaskId["task-1"]?.map((pr) => pr.id)).toEqual([
"association-2",
]);
});
it("applies unscoped rate-limit events only to legacy shared connections", () => {
const store = createAppStore();
store.getState().resetGitHubStatus("legacy-workspace");
apps/web/lib/ws/handlers/github.ts
+12 -1
View File
@@ -1,7 +1,12 @@
import type { StoreApi } from "zustand";
import type { AppState } from "@/lib/state/store";
import type { WsHandlers } from "@/lib/ws/handlers/types";
import type { GitHubRateLimitUpdate, TaskCIAutomationOptions, TaskPR } from "@/lib/types/github";
import type {
GitHubRateLimitUpdate,
TaskCIAutomationOptions,
TaskPR,
TaskPRDeletedEvent,
} from "@/lib/types/github";
export function registerGitHubHandlers(store: StoreApi<AppState>): WsHandlers {
return {
@@ -11,6 +16,12 @@ export function registerGitHubHandlers(store: StoreApi<AppState>): WsHandlers {
store.getState().setTaskPR(pr.task_id, pr);
}
},
"github.task_pr.deleted": (message) => {
const deleted = message.payload as TaskPRDeletedEvent;
if (deleted.task_id && deleted.association_id) {
store.getState().removeTaskPR(deleted.task_id, deleted.association_id);
}
},
"github.task_ci_options.updated": (message) => {
const options = message.payload as TaskCIAutomationOptions;
if (options.task_id) {
docs/plans/unlink-task-github-pr/plan.md
+187
View File
@@ -0,0 +1,187 @@
---
spec: docs/specs/tasks/link-existing-task-github-issue.md
created: 2026-07-31
status: implemented
---
# Implementation Plan: Unlink A Task GitHub Pull Request
## Overview
Add a workspace-authorized unlink contract for one persisted GitHub task-PR
association, broadcast the removal to connected clients, and expose it through
the existing multi-PR tab strip. The backend lands first so frontend state can
consume a stable API and WebSocket contract; desktop and mobile behavior then
share one mutation path and receive focused Playwright coverage.
## Backend
### Durable association detachment
- Update `apps/backend/internal/github/store.go` and
`apps/backend/internal/github/models.go` so `github_task_prs` gains an
idempotently migrated nullable `detached_at` timestamp on SQLite and
Postgres-compatible startup paths.
- Keep detached rows as durable tombstones so a poller or later session does
not silently rediscover an explicitly removed PR. Active association reads,
task search, review sources, and automation list paths must filter
`detached_at IS NULL`.
- Add an exact association lookup by ID and a workspace-scoped detach mutation.
The mutation stamps only the requested row. Explicit URL linking of the same
`(task_id, repository_id, pr_number)` clears the tombstone; automatic watch
discovery does not.
- Preserve task deletion cleanup and existing task/repository/PR uniqueness.
No GitHub API mutation occurs during detach.
### Service and HTTP contract
- Add a service entry point in
`apps/backend/internal/github/service_pr_watch.go` (or a focused sibling
file) that resolves the association, authorizes its stored workspace, and
detaches it without leaking whether a cross-workspace ID exists.
- Register
`DELETE /api/v1/github/task-prs/:associationId?workspace_id=<workspaceId>` in
`apps/backend/internal/github/controller.go`. Return `{ "deleted": true }`
on success and `404` for unknown or workspace-mismatched associations.
- Publish a typed removal payload containing `workspace_id`, `task_id`, and
`association_id` only after the detach transaction commits.
### Live update contract
- Add `github.task_pr.deleted` to
`apps/backend/internal/events/types.go` and
`apps/backend/pkg/websocket/actions.go`.
- Subscribe it in
`apps/backend/internal/gateway/websocket/task_notifications.go`; the payload's
workspace ID keeps routing scoped to clients for the owning workspace.
## Frontend
### API, store, and live updates
- Add `deleteTaskPR(associationId, workspaceId)` to
`apps/web/lib/api/domains/github-api.ts`.
- Extend the GitHub Zustand slice in
`apps/web/lib/state/slices/github/{types.ts,github-slice.ts}` with
`removeTaskPR(taskId, associationId)`, removing only the selected row and
deleting an empty task bucket.
- Type `github.task_pr.deleted` in `apps/web/lib/types/backend.ts` and handle it
in `apps/web/lib/ws/handlers/github.ts` so other tabs and windows converge.
- Expose a shared unlink mutation from
`apps/web/hooks/domains/github/use-task-pr.ts`. It waits for the HTTP success
before removing local state and reports failure without hiding the PR.
### Multi-PR tab close action
- Refactor `apps/web/components/github/multi-pr-ci-popover.tsx` so the tab label
and close action are sibling semantic buttons rather than nested buttons.
Render the close action only while the task has multiple PRs.
- Give each close action an association-specific accessible name, stop its
pointer/click activation from selecting or opening the PR, disable it while
its request is pending, and retain the tab with an error toast on failure.
- After a successful removal, select/focus a deterministic adjacent remaining
tab when the multi-PR surface remains mounted. When two PRs become one, the
existing parent surface collapses naturally to its single-PR variant and
restores focus to that surviving trigger.
- Wire the same mutation into both
`apps/web/components/github/pr-topbar-button.tsx` and
`apps/web/components/github/pr-status-chip.tsx`.
### Mobile design contract
- **Desktop outcome:** hovering the topbar's multi-PR control shows the existing
tabbed CI popover; each tab has a compact close action.
- **Mobile entry point:** tap the existing PR status chip to open
`PRStatusChipMultiDrawer`; no new route or overlay is introduced.
- **Nearest shipped exemplar:** the current
`PRStatusChipMultiDrawer` + `MultiPRCIPopover` composition supplies the inset
bottom drawer, fixed header, tab hierarchy, and single internally scrolling
body.
- **Hierarchy and action:** the selected PR remains the focal content; unlink is
a secondary action attached to each PR tab. Mobile close targets use an
actual minimum 44px hitbox and an association-specific accessible label.
- **Presentation rationale:** unlink is a short contextual action inside an
already-open status surface, so the existing drawer is more direct than a
second confirmation drawer or route.
- **Shared behavior:** API mutation, pending state, error handling, selected-PR
fallback, and store removal are shared; only responsive geometry differs.
- **Viewport behavior:** retain the drawer's `min-h-0` internal scroll owner,
safe-area behavior, focus return, and zero document-level horizontal
overflow.
## Tests
- **What:** detaching one association is workspace-scoped, persists across a
store reopen/replayed migration, active lists omit it, siblings remain, and
explicit relinking restores it.
**File:** focused tests beside `apps/backend/internal/github/store.go` and
`service_pr_watch.go`.
**How:** table-driven store/service tests using the real test database and a
workspace authorizer.
- **What:** the HTTP endpoint returns success for the owning workspace and
fails closed for unknown/cross-workspace IDs without mutating rows.
**File:** `apps/backend/internal/github/controller_test.go`.
**How:** Gin handler integration tests against the real GitHub store/service.
- **What:** a committed detach broadcasts the typed workspace-scoped removal
notification.
**File:**
`apps/backend/internal/gateway/websocket/task_notifications_test.go`.
**How:** event-bus notification test.
- **What:** API URL/method, exact store removal, WebSocket removal, pending
close state, selected fallback, focus behavior, and failure retention.
**Files:** `apps/web/lib/api/domains/github-api.test.ts`,
`apps/web/lib/state/slices/github/github-slice.test.ts`,
`apps/web/lib/ws/handlers/github.test.ts`, and the focused
`apps/web/components/github/pr-ci-popover.automation.test.tsx`.
**How:** Vitest unit/component tests with mocked API and store state.
## E2E Tests
- **Scenario:** GIVEN a desktop task with two linked PRs, WHEN the user hovers
the topbar PR control and closes the older PR tab, THEN only the newer PR
remains before and after reload.
**File:** `apps/web/e2e/tests/pr/pr-multi-popover.spec.ts`.
**What to verify:** close action is reachable, no sibling tab is removed, the
topbar changes to the single-PR state, and persistence survives reload.
- **Scenario:** GIVEN the same task on a coarse-pointer phone, WHEN the user
opens the PR status drawer and taps one tab's unlink target, THEN the same
association is removed with a touch-sized control and no horizontal page
overflow.
**File:** `apps/web/e2e/tests/pr/mobile-pr-ci-chip.spec.ts`.
**What to verify:** drawer entry, accessible close action, minimum hitbox,
resulting single-PR chip, reload persistence, and document containment.
## Public Documentation
- Update `docs/public/sessions-and-review.md` beside the existing multi-PR
selector guidance. Explain where unlink is available and that it removes the
Kandev association only, leaving the remote PR and branch untouched.
## Implementation Waves And Parallel Candidates
The default execution order is sequential in the primary conversation.
- [x] [Task 01: Backend unlink contract](task-01-backend-unlink-contract.md)
- [x] [Task 02: Frontend multi-PR unlink UI](task-02-frontend-multi-pr-unlink.md)
- [x] [Task 03: Desktop and mobile E2E coverage](task-03-pr-unlink-e2e.md)
- [x] [Task 04: Public documentation](task-04-public-documentation.md)
Tasks 01 and 04 are parallel-safe because they own disjoint implementation and
documentation files, but parallel execution still requires explicit user
authorization. Tasks 02 and 03 are sequential after the backend contract and
UI respectively.
## Risks
- Filtering detached rows must cover every task-PR consumer; a missed query
could leave an old PR visible in search, review, or automation even when the
topbar is correct.
- Automatic PR-watch discovery and explicit manual relinking need different
tombstone behavior so background sync cannot resurrect a user removal while
deliberate relinking still works.
- A close button nested inside the current tab button would produce invalid
interactive markup and broken keyboard behavior; the tab composition must be
split without regressing roving tabindex.
- Removing the selected tab can unmount the multi-PR surface when only one PR
remains, so focus management must tolerate both the retained and collapsed
cases.
docs/plans/unlink-task-github-pr/task-01-backend-unlink-contract.md
+70
View File
@@ -0,0 +1,70 @@
---
id: "01-backend-unlink-contract"
title: "Backend unlink contract"
status: done
wave: 1
depends_on: []
plan: "plan.md"
spec: "../../specs/tasks/link-existing-task-github-issue.md"
---
# Task 01: Backend Unlink Contract
## Acceptance
- One workspace-authorized endpoint detaches exactly one GitHub task-PR
association, leaves sibling associations and the remote PR untouched, and
fails closed for unknown or cross-workspace IDs.
- Detached associations survive restart/migration replay, disappear from all
active association reads and automation inputs, are not resurrected by
automatic watch discovery, and can be restored by explicit URL linking.
- A successful detach publishes a workspace-routed `github.task_pr.deleted`
notification only after persistence succeeds.
## Verification
```bash
cd apps/backend && go test ./internal/github ./internal/gateway/websocket ./pkg/websocket
```
## Files likely touched
- `apps/backend/internal/github/models.go`
- `apps/backend/internal/github/store.go`
- `apps/backend/internal/github/service_pr_watch.go` or a focused sibling
- `apps/backend/internal/github/controller.go`
- `apps/backend/internal/github/*_test.go`
- `apps/backend/internal/events/types.go`
- `apps/backend/pkg/websocket/actions.go`
- `apps/backend/internal/gateway/websocket/task_notifications.go`
- `apps/backend/internal/gateway/websocket/task_notifications_test.go`
## Dependencies
None.
## Parallelism
Parallel-safe with Task 04 only: implementation and public-doc files are
disjoint, and Task 04 does not depend on generated contracts.
## Inputs
- Spec sections: `What`, `Scenarios`, `Data And API`, `Failure Modes`.
- Plan sections: `Durable association detachment`, `Service and HTTP contract`,
`Live update contract`.
- Patterns: GitLab's workspace-scoped `DeleteTaskMRForWorkspace`, GitHub's
`ReplaceTaskPR`, and workspace-routed task notification payloads.
## Risks
- Existing databases and both supported SQL dialects must receive the nullable
column through replay-safe migration order.
- Internal automatic association paths must not clear the tombstone intended
for explicit relinking only.
## Output contract
Report the behavioral tests added, files changed, exact command result,
remaining risks/blockers, and update this task plus `plan.md` status in the same
conversation.
docs/plans/unlink-task-github-pr/task-02-frontend-multi-pr-unlink.md
+75
View File
@@ -0,0 +1,75 @@
---
id: "02-frontend-multi-pr-unlink"
title: "Frontend multi-PR unlink UI"
status: done
wave: 2
depends_on: ["01-backend-unlink-contract"]
plan: "plan.md"
spec: "../../specs/tasks/link-existing-task-github-issue.md"
---
# Task 02: Frontend Multi-PR Unlink UI
## Acceptance
- Every tab in a multi-PR desktop popover and mobile drawer has an accessible
unlink button; the label button and close button are valid sibling controls,
and the mobile target is at least 44px.
- Successful unlink removes only the selected association from local state and
live updates do the same in other clients; failure retains the tab and shows
an error.
- Tab selection, roving keyboard navigation, pending state, adjacent fallback,
and the two-to-one PR surface collapse remain deterministic.
## Verification
```bash
cd apps/web && pnpm exec vitest run components/github/pr-ci-popover.automation.test.tsx lib/api/domains/github-api.test.ts lib/state/slices/github/github-slice.test.ts lib/ws/handlers/github.test.ts
cd apps/web && pnpm run typecheck
```
## Files likely touched
- `apps/web/lib/api/domains/github-api.ts`
- `apps/web/lib/api/domains/github-api.test.ts`
- `apps/web/lib/state/slices/github/types.ts`
- `apps/web/lib/state/slices/github/github-slice.ts`
- `apps/web/lib/state/slices/github/github-slice.test.ts`
- `apps/web/lib/types/backend.ts`
- `apps/web/lib/ws/handlers/github.ts`
- `apps/web/lib/ws/handlers/github.test.ts`
- `apps/web/hooks/domains/github/use-task-pr.ts`
- `apps/web/components/github/multi-pr-ci-popover.tsx`
- `apps/web/components/github/pr-ci-popover.automation.test.tsx`
- `apps/web/components/github/pr-topbar-button.tsx`
- `apps/web/components/github/pr-status-chip.tsx`
## Dependencies
Task 01's HTTP and WebSocket contracts.
## Parallelism
Sequential. This task consumes Task 01's exact payload and endpoint shapes and
owns shared GitHub frontend state used by the E2E task.
## Inputs
- Spec scenarios: `Unlink one of several pull requests`, `Keep an association
when unlink fails`, `Reach unlink from a touch device`.
- Plan sections: `API, store, and live updates`, `Multi-PR tab close action`,
`Mobile design contract`.
- Patterns: GitLab `MRTopbarButton` unlink error handling and the existing
`PRStatusChipMultiDrawer` touch composition.
## Risks
- Do not nest an icon button inside the existing `role="tab"` button.
- A successful two-to-one removal can unmount the active multi-PR component;
focus logic must not assume the tablist still exists.
## Output contract
Report the RED/GREEN evidence, files changed, exact command results, mobile
interaction details, remaining risks/blockers, and update this task plus
`plan.md` status in the same conversation.
docs/plans/unlink-task-github-pr/task-03-pr-unlink-e2e.md
+66
View File
@@ -0,0 +1,66 @@
---
id: "03-pr-unlink-e2e"
title: "PR unlink E2E coverage"
status: done
wave: 3
depends_on: ["01-backend-unlink-contract", "02-frontend-multi-pr-unlink"]
plan: "plan.md"
spec: "../../specs/tasks/link-existing-task-github-issue.md"
---
# Task 03: PR Unlink E2E Coverage
## Acceptance
- Desktop Playwright coverage unlinks the older of two PRs from the hover
popover and proves the sibling plus persisted single-PR state remain after
reload.
- Mobile Playwright coverage performs the same action through the existing PR
status drawer using a touch event, verifies the close target hitbox, and
proves document containment.
- Shared page-object/API seed helpers identify associations without relying on
unstable visible copy.
## Verification
```bash
cd apps/web && pnpm e2e:run --host --project chromium e2e/tests/pr/pr-multi-popover.spec.ts
cd apps/web && pnpm e2e:run --host --project mobile-chrome e2e/tests/pr/mobile-pr-ci-chip.spec.ts
```
## Files likely touched
- `apps/web/e2e/tests/pr/pr-multi-popover.spec.ts`
- `apps/web/e2e/tests/pr/mobile-pr-ci-chip.spec.ts`
- `apps/web/e2e/pages/session-page.ts`
- `apps/web/e2e/helpers/api-client.ts` only if the existing seed helpers cannot
express the required association identity
## Dependencies
Tasks 01 and 02.
## Parallelism
Sequential. The scenarios exercise the completed backend contract and shared
frontend component.
## Inputs
- Spec scenarios: desktop unlink and coarse-pointer unlink.
- Plan section: `E2E Tests` and the complete mobile design contract.
- Existing seed/open patterns in the two named PR specs.
## Risks
- The mobile project only discovers `mobile-*.spec.ts`; keep the existing file
name and run the managed E2E runner so production frontend/backend artifacts
are rebuilt.
- When the second PR disappears the drawer/popover may unmount; assert the
resulting task state rather than waiting on a detached close control.
## Output contract
Report RED/GREEN Playwright evidence, screenshots or rendered inspection,
files changed, exact command result, remaining risks/blockers, and update this
task plus `plan.md` status in the same conversation.
docs/plans/unlink-task-github-pr/task-04-public-documentation.md
+58
View File
@@ -0,0 +1,58 @@
---
id: "04-public-documentation"
title: "Public documentation"
status: done
wave: 1
depends_on: []
plan: "plan.md"
spec: "../../specs/tasks/link-existing-task-github-issue.md"
---
# Task 04: Public Documentation
## Acceptance
- The public session/review guide tells desktop and mobile users where the
multi-PR unlink action appears.
- The guide states that unlinking removes only Kandev's association and leaves
the GitHub PR, branch, commits, task repositories, and sibling links intact.
- Public documentation validation passes without navigation or link errors.
## Verification
```bash
node --test scripts/validate-public-docs.test.mjs
node scripts/validate-public-docs.mjs
```
## Files likely touched
- `docs/public/sessions-and-review.md`
## Dependencies
None for wording; re-check the final UI label after Task 02 before marking this
task done.
## Parallelism
Parallel-safe with Task 01 because it owns only public documentation. Execution
still defaults to sequential without explicit user authorization.
## Inputs
- Spec `What` and unlink scenarios.
- Plan `Public Documentation` section.
- Existing multi-PR selector guidance in
`docs/public/sessions-and-review.md`.
## Risks
- Do not imply that unlinking closes the provider PR or disables GitHub
automation globally; only the selected task association leaves the task.
## Output contract
Report the exact wording location, files changed, validation results,
remaining risks/blockers, and update this task plus `plan.md` status in the same
conversation.
docs/public/sessions-and-review.md
+2
View File
@@ -158,6 +158,8 @@ Select **Review** in the Changes header. Kandev builds a repository-aware file l
When a task has multiple linked pull requests, use the PR selector in the Changes diff header or Review toolbar to inspect one PR revision at a time. The selection is scoped to that task for the current app session. Switching PRs replaces only the remote PR contribution; uncommitted and committed sources keep their normal precedence. Selecting a file from a specific PR row opens that exact PR revision, even when a sibling PR changes the same path.
When several pull requests are linked to a task, hover the PR control in the desktop top bar—or tap the PR status chip on mobile—to open the tabbed CI surface. Each PR tab has a **Remove from task** button. Removing a tab only detaches that Kandev task association; it does not close or modify the GitHub pull request, its branch or commits, the task repositories, or sibling PR associations. Explicitly linking that PR again restores the association.
<DocsVideo
webm="./media/feature-guides/diff-line-feedback.webm"
mp4="./media/feature-guides/diff-line-feedback.mp4"
docs/specs/tasks/link-existing-task-github-issue.md
+46
View File
@@ -27,6 +27,17 @@ issue, or Sentry issue is identified later.
- Creating a task from a GitHub issue on the GitHub integration page automatically applies the same metadata-backed link after task creation.
- The GitHub issues list resolves all metadata-backed issue links in the active workspace and shows the linked task title with navigation to that task. Links created manually, by the GitHub issue quick launcher, and by issue watches use the same indicator.
- Pull request linking reuses the existing task PR association model and rendering.
- When a task has multiple linked GitHub pull requests, every PR tab in the
task PR status surface includes an explicit close action that unlinks that PR
from the task. The action is available in the desktop hover popover and the
existing touch drawer.
- Unlinking removes only the selected Kandev task-PR association. It does not
close or modify the GitHub pull request, delete its branch, change task
repositories, or affect sibling PR associations.
- A successfully unlinked PR disappears from every task PR surface and no
longer participates in task-level PR automation. The removal survives page
reloads and Kandev restarts. Explicitly linking the same PR again restores
the association.
- A linked issue can be explicitly changed or unlinked from the same dialog.
- Jira ticket linking is shown only when Jira is enabled and healthy for the
current workspace. It accepts a Jira key or URL, validates it through the
@@ -88,6 +99,24 @@ GIVEN an existing task that already has GitHub issue metadata
WHEN the user opens Link > GitHub Issue and chooses Unlink
THEN Kandev removes only the issue metadata keys and preserves unrelated task metadata
### Unlink one of several pull requests
GIVEN a task has two or more linked GitHub pull requests
WHEN the user activates the close action on one PR tab
THEN Kandev removes only that task-PR association, leaves the remote GitHub pull request and sibling associations unchanged, and immediately renders the remaining PRs
### Keep an association when unlink fails
GIVEN a task has two or more linked GitHub pull requests
WHEN unlinking one association fails
THEN Kandev keeps the PR tab and association visible and shows an actionable error
### Reach unlink from a touch device
GIVEN a task has two or more linked GitHub pull requests on a coarse-pointer device
WHEN the user opens the PR status drawer
THEN every PR tab exposes a touch-sized, accessible unlink action with the same result as desktop
### Create and link a task from the GitHub issues list
GIVEN a GitHub issue shown on the GitHub integration page
@@ -118,12 +147,27 @@ THEN no task indicator is shown and the existing issue actions remain available
- Each task links to at most one GitHub issue, while one GitHub issue may link to multiple tasks.
- `GET /api/v1/github/task-issues?workspace_id=<id>` returns links grouped by task ID for the requested workspace. The lookup derives owner, repository, and issue number from the canonical GitHub issue URL and never returns links from another workspace.
- The workspace lookup uses the indexed task workspace boundary and does not depend on the GitHub token being currently configured because it reads persisted task metadata.
- `github_task_prs.detached_at` is nullable. A null value identifies an active
association; unlinking stamps the selected row, and active task-PR reads,
search results, review sources, and automation omit detached rows. Explicit
relinking clears the detached state for the exact task, repository, and PR
number.
- `DELETE /api/v1/github/task-prs/:associationId?workspace_id=<id>` unlinks one
association inside the authorized workspace and returns `{ "deleted": true
}`. An unknown association or workspace mismatch returns `404` without
revealing cross-workspace data.
- Successful unlink publishes `github.task_pr.deleted` with the workspace,
task, and association IDs so all connected clients remove the same PR from
local state.
## Failure Modes
- A failed automatic link attempt does not roll back a successfully created task or block navigation to it.
- Invalid or incomplete GitHub issue metadata is ignored by the workspace reverse lookup instead of producing a misleading issue-row association.
- Existing link validation continues to reject issues from repositories not attached to the task.
- A failed PR unlink leaves the persisted association active and keeps the
current UI selection intact. The UI reports the failure instead of
optimistically hiding the tab.
### Link a Jira ticket to an existing task
@@ -154,5 +198,7 @@ THEN Kandev renames the task to `ENG-20: Fix login` instead of stacking prefixes
- Linking does not change task state, session history, repositories, or unrelated metadata.
- Invalid issue references and repository mismatches return clear errors.
- Right-click context menu, sidebar menu, and touch/dropdown menu users can reach the Link submenu.
- Desktop pointer and coarse-pointer users can unlink one PR when at least two
are associated, and the remaining association still renders after reload.
- Jira, Linear, and Sentry actions are hidden when the corresponding integration is disabled or unauthenticated.
- GitHub issue links are visible only in their task workspace, including after page reload.