diff --git a/apps/backend/internal/backendapp/status_summary_adapter.go b/apps/backend/internal/backendapp/status_summary_adapter.go index beae5f3140..7a153ee606 100644 --- a/apps/backend/internal/backendapp/status_summary_adapter.go +++ b/apps/backend/internal/backendapp/status_summary_adapter.go @@ -47,6 +47,7 @@ func (r *githubTaskStatusSummaryPRReader) ListTaskStatusSummaryPullRequests( ReviewState: pr.ReviewState, ChecksState: pr.ChecksState, MergeableState: pr.MergeableState, + MergeQueueState: pr.MergeQueueState, UnresolvedReviewCount: pr.UnresolvedReviewThreads, PendingReviewCount: pr.PendingReviewCount, RequiredReviews: requiredReviews, diff --git a/apps/backend/internal/backendapp/status_summary_adapter_test.go b/apps/backend/internal/backendapp/status_summary_adapter_test.go index 0f1aff82a0..37dba3bf8f 100644 --- a/apps/backend/internal/backendapp/status_summary_adapter_test.go +++ b/apps/backend/internal/backendapp/status_summary_adapter_test.go @@ -1,11 +1,37 @@ package backendapp import ( + "context" + "path/filepath" "testing" + "time" + "github.com/jmoiron/sqlx" + "github.com/kandev/kandev/internal/db" "github.com/kandev/kandev/internal/github" ) +func newStatusSummaryTestStore(t *testing.T) *github.Store { + t.Helper() + dbConn, err := db.OpenSQLite(filepath.Join(t.TempDir(), "github.db")) + if err != nil { + t.Fatalf("open db: %v", err) + } + sqlxDB := sqlx.NewDb(dbConn, "sqlite3") + t.Cleanup(func() { _ = sqlxDB.Close() }) + if _, err := sqlxDB.Exec(`CREATE TABLE tasks (id TEXT PRIMARY KEY, workspace_id TEXT, archived_at DATETIME)`); err != nil { + t.Fatalf("create tasks table: %v", err) + } + if _, err := sqlxDB.Exec(`CREATE TABLE workspaces (id TEXT PRIMARY KEY)`); err != nil { + t.Fatalf("create workspaces table: %v", err) + } + store, err := github.NewStore(sqlxDB, sqlxDB) + if err != nil { + t.Fatalf("new store: %v", err) + } + return store +} + func TestTaskStatusSummaryPRKeyMatchesLiveEventIdentity(t *testing.T) { tests := []struct { name string @@ -60,3 +86,31 @@ func TestTaskStatusRuntimeProvidersTreatNilOrchestratorAsAbsent(t *testing.T) { t.Fatal("queued prompt counter should be nil without an orchestrator") } } + +func TestGitHubTaskStatusSummaryPRReaderPreservesMergeQueueState(t *testing.T) { + ctx := context.Background() + store := newStatusSummaryTestStore(t) + queuePR := &github.TaskPR{ + TaskID: "task-queue-summary", RepositoryID: "repo-queue-summary", PRNumber: 42, + PRURL: "https://example.test/42", State: "open", CreatedAt: time.Now().UTC(), + MergeQueueState: "queued", + } + if err := store.CreateTaskPR(ctx, queuePR); err != nil { + t.Fatalf("CreateTaskPR: %v", err) + } + + reader := &githubTaskStatusSummaryPRReader{ + gh: github.NewService(nil, "", nil, store, nil, nil), + } + result, err := reader.ListTaskStatusSummaryPullRequests(ctx, []string{queuePR.TaskID}) + if err != nil { + t.Fatalf("ListTaskStatusSummaryPullRequests: %v", err) + } + inputs := result[queuePR.TaskID] + if len(inputs) != 1 { + t.Fatalf("summary inputs = %+v, want one input", inputs) + } + if inputs[0].MergeQueueState != queuePR.MergeQueueState { + t.Fatalf("MergeQueueState = %q, want %q", inputs[0].MergeQueueState, queuePR.MergeQueueState) + } +} diff --git a/apps/backend/internal/github/graphql.go b/apps/backend/internal/github/graphql.go index 36bbbd10d8..bb1230df59 100644 --- a/apps/backend/internal/github/graphql.go +++ b/apps/backend/internal/github/graphql.go @@ -183,9 +183,14 @@ type batchedPRResult struct { // IsDraft is a pointer: AC-12a requires distinguishing an upstream // response that omits or nulls isDraft from one that genuinely reports // false, and a plain bool can't tell the two apart after decode. - IsDraft *bool `json:"isDraft"` - Mergeable string `json:"mergeable"` - MergeStatus string `json:"mergeStateStatus"` + IsDraft *bool `json:"isDraft"` + Mergeable string `json:"mergeable"` + MergeStatus string `json:"mergeStateStatus"` + MergeQueueEntry *struct { + State string `json:"state"` + Position *int `json:"position"` + EstimatedTimeToMerge *int `json:"estimatedTimeToMerge"` + } `json:"mergeQueueEntry"` HeadRefName string `json:"headRefName"` BaseRefName string `json:"baseRefName"` HeadRefOid string `json:"headRefOid"` @@ -413,6 +418,7 @@ func prFieldsBlock() string { return `state title url isDraft mergeable mergeStateStatus ` + `headRefName baseRefName headRefOid additions deletions changedFiles ` + `author { login } mergedBy { login } autoMergeRequest { enabledAt } ` + + `mergeQueueEntry { state position estimatedTimeToMerge } ` + `createdAt updatedAt mergedAt closedAt ` + `reviews(last: 100) { nodes { state author { login } submittedAt } } ` + `reviewRequests(first: 0) { totalCount } ` + @@ -490,22 +496,58 @@ func convertBatchedPRResult(raw *batchedPRResult, owner, repo string, number int } } closedByLogin, closureAttributionPopulated := closedEventActor(raw.TimelineItems.Nodes) + mergeQueueState, mergeQueuePosition, mergeQueueEstimate := convertMergeQueueEntry(raw.MergeQueueEntry) return &PRStatus{ - PR: pr, - ReviewState: reviewState, - ChecksState: checksState, - MergeableState: pr.MergeableState, - ReviewCount: countApprovedReviewerNodes(raw.Reviews.Nodes), - PendingReviewCount: raw.ReviewRequests.TotalCount, - ReviewCountsPopulated: true, - UnresolvedReviewThreads: unresolved, - UnresolvedReviewThreadsPopulated: true, - OutcomeFieldsPopulated: true, - ClosedByLogin: closedByLogin, - ClosureAttributionPopulated: closureAttributionPopulated, + PR: pr, + ReviewState: reviewState, + ChecksState: checksState, + MergeableState: pr.MergeableState, + ReviewCount: countApprovedReviewerNodes(raw.Reviews.Nodes), + PendingReviewCount: raw.ReviewRequests.TotalCount, + ReviewCountsPopulated: true, + UnresolvedReviewThreads: unresolved, + UnresolvedReviewThreadsPopulated: true, + OutcomeFieldsPopulated: true, + ClosedByLogin: closedByLogin, + ClosureAttributionPopulated: closureAttributionPopulated, + MergeQueueState: mergeQueueState, + MergeQueuePosition: mergeQueuePosition, + MergeQueueEstimatedTimeToMergeSeconds: mergeQueueEstimate, + mergeQueuePopulated: true, } } +func convertMergeQueueEntry(entry *struct { + State string `json:"state"` + Position *int `json:"position"` + EstimatedTimeToMerge *int `json:"estimatedTimeToMerge"` +}) (string, *int, *int) { + if entry == nil { + return "", nil, nil + } + return normalizeMergeQueueState(entry.State), positiveIntPtr(entry.Position), nonNegativeIntPtr(entry.EstimatedTimeToMerge) +} + +func normalizeMergeQueueState(raw string) string { + return strings.ToLower(strings.TrimSpace(raw)) +} + +func positiveIntPtr(value *int) *int { + if value == nil || *value <= 0 { + return nil + } + copy := *value + return © +} + +func nonNegativeIntPtr(value *int) *int { + if value == nil || *value < 0 { + return nil + } + copy := *value + return © +} + // closedEventActor extracts the closed-event actor's login from a // `timelineItems(itemTypes: CLOSED_EVENT)` node list. Returns // populated=false when there is no node or the node's actor is null (e.g. a diff --git a/apps/backend/internal/github/graphql_merge_queue_status_test.go b/apps/backend/internal/github/graphql_merge_queue_status_test.go new file mode 100644 index 0000000000..2f7c78ced5 --- /dev/null +++ b/apps/backend/internal/github/graphql_merge_queue_status_test.go @@ -0,0 +1,77 @@ +package github + +import ( + "encoding/json" + "strings" + "testing" +) + +func TestPRFieldsBlockRequestsMergeQueueEntry(t *testing.T) { + block := prFieldsBlock() + if !strings.Contains(block, "mergeQueueEntry { state position estimatedTimeToMerge }") { + t.Fatalf("prFieldsBlock() = %s, want merge queue fields", block) + } +} + +func TestConvertBatchedPRResultPreservesMergeQueueObservation(t *testing.T) { + tests := []struct { + name string + entry string + wantState string + wantPosition *int + wantEstimate *int + wantObserved bool + }{ + { + name: "queued with metadata", + entry: `{"state":"QUEUED","position":3,"estimatedTimeToMerge":125}`, + wantState: "queued", + wantPosition: intPtr(3), + wantEstimate: intPtr(125), + wantObserved: true, + }, + { + name: "active entry without estimate", + entry: `{"state":"MERGEABLE","position":1,"estimatedTimeToMerge":null}`, + wantState: "mergeable", + wantPosition: intPtr(1), + wantObserved: true, + }, + { + name: "queue exit", + entry: `null`, + wantObserved: true, + }, + { + name: "unknown future state", + entry: `{"state":"PAUSED","position":7,"estimatedTimeToMerge":60}`, + wantState: "paused", + wantPosition: intPtr(7), + wantEstimate: intPtr(60), + wantObserved: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + raw := &batchedPRResult{} + payload := []byte(`{"state":"OPEN","mergeQueueEntry":` + test.entry + `}`) + if err := json.Unmarshal(payload, raw); err != nil { + t.Fatalf("decode GraphQL payload: %v", err) + } + status := convertBatchedPRResult(raw, "owner", "repo", 42) + if status.MergeQueueState != test.wantState { + t.Errorf("MergeQueueState = %q, want %q", status.MergeQueueState, test.wantState) + } + if !intPtrEqual(status.MergeQueuePosition, test.wantPosition) { + t.Errorf("MergeQueuePosition = %v, want %v", status.MergeQueuePosition, test.wantPosition) + } + if !intPtrEqual(status.MergeQueueEstimatedTimeToMergeSeconds, test.wantEstimate) { + t.Errorf("MergeQueueEstimatedTimeToMergeSeconds = %v, want %v", status.MergeQueueEstimatedTimeToMergeSeconds, test.wantEstimate) + } + if status.mergeQueuePopulated != test.wantObserved { + t.Errorf("mergeQueuePopulated = %v, want %v", status.mergeQueuePopulated, test.wantObserved) + } + }) + } +} diff --git a/apps/backend/internal/github/mock_controller.go b/apps/backend/internal/github/mock_controller.go index ba901b34da..a65895ae6f 100644 --- a/apps/backend/internal/github/mock_controller.go +++ b/apps/backend/internal/github/mock_controller.go @@ -604,29 +604,32 @@ func (c *MockController) addRepoFiles(ctx *gin.Context) { // associateTaskPR endpoint. Pointer fields are optional — leave them nil // to skip the corresponding TaskPR column update. type associateTaskPRRequest struct { - TaskID string `json:"task_id"` - WorkspaceID string `json:"workspace_id,omitempty"` - RepositoryID string `json:"repository_id,omitempty"` - Owner string `json:"owner"` - Repo string `json:"repo"` - PRNumber int `json:"pr_number"` - PRURL string `json:"pr_url"` - PRTitle string `json:"pr_title"` - HeadBranch string `json:"head_branch"` - BaseBranch string `json:"base_branch"` - AuthorLogin string `json:"author_login"` - State string `json:"state"` - ReviewState string `json:"review_state"` - ChecksState string `json:"checks_state"` - MergeableState string `json:"mergeable_state"` - Additions int `json:"additions"` - Deletions int `json:"deletions"` - ReviewCount *int `json:"review_count,omitempty"` - PendingReviewCount *int `json:"pending_review_count,omitempty"` - RequiredReviews *int `json:"required_reviews,omitempty"` - ChecksTotal *int `json:"checks_total,omitempty"` - ChecksPassing *int `json:"checks_passing,omitempty"` - UnresolvedReviewThreads *int `json:"unresolved_review_threads,omitempty"` + TaskID string `json:"task_id"` + WorkspaceID string `json:"workspace_id,omitempty"` + RepositoryID string `json:"repository_id,omitempty"` + Owner string `json:"owner"` + Repo string `json:"repo"` + PRNumber int `json:"pr_number"` + PRURL string `json:"pr_url"` + PRTitle string `json:"pr_title"` + HeadBranch string `json:"head_branch"` + BaseBranch string `json:"base_branch"` + AuthorLogin string `json:"author_login"` + State string `json:"state"` + ReviewState string `json:"review_state"` + ChecksState string `json:"checks_state"` + MergeableState string `json:"mergeable_state"` + MergeQueueState string `json:"merge_queue_state"` + MergeQueuePosition *int `json:"merge_queue_position,omitempty"` + MergeQueueEstimatedTimeToMergeSeconds *int `json:"merge_queue_estimated_time_to_merge_seconds,omitempty"` + Additions int `json:"additions"` + Deletions int `json:"deletions"` + ReviewCount *int `json:"review_count,omitempty"` + PendingReviewCount *int `json:"pending_review_count,omitempty"` + RequiredReviews *int `json:"required_reviews,omitempty"` + ChecksTotal *int `json:"checks_total,omitempty"` + ChecksPassing *int `json:"checks_passing,omitempty"` + UnresolvedReviewThreads *int `json:"unresolved_review_threads,omitempty"` } // associateTaskPR directly creates (or replaces) a github_task_prs record for @@ -669,24 +672,27 @@ func (c *MockController) associateTaskPR(ctx *gin.Context) { // a TaskPR and applies the optional pointer fields when present. func buildTaskPRFromRequest(req *associateTaskPRRequest, now time.Time) *TaskPR { tp := &TaskPR{ - TaskID: req.TaskID, - WorkspaceID: req.WorkspaceID, - RepositoryID: req.RepositoryID, - Owner: req.Owner, - Repo: req.Repo, - PRNumber: req.PRNumber, - PRURL: req.PRURL, - PRTitle: req.PRTitle, - HeadBranch: req.HeadBranch, - BaseBranch: req.BaseBranch, - AuthorLogin: req.AuthorLogin, - State: req.State, - ReviewState: req.ReviewState, - ChecksState: req.ChecksState, - MergeableState: req.MergeableState, - Additions: req.Additions, - Deletions: req.Deletions, - CreatedAt: now, + TaskID: req.TaskID, + WorkspaceID: req.WorkspaceID, + RepositoryID: req.RepositoryID, + Owner: req.Owner, + Repo: req.Repo, + PRNumber: req.PRNumber, + PRURL: req.PRURL, + PRTitle: req.PRTitle, + HeadBranch: req.HeadBranch, + BaseBranch: req.BaseBranch, + AuthorLogin: req.AuthorLogin, + State: req.State, + ReviewState: req.ReviewState, + ChecksState: req.ChecksState, + MergeableState: req.MergeableState, + MergeQueueState: req.MergeQueueState, + MergeQueuePosition: req.MergeQueuePosition, + MergeQueueEstimatedTimeToMergeSeconds: req.MergeQueueEstimatedTimeToMergeSeconds, + Additions: req.Additions, + Deletions: req.Deletions, + CreatedAt: now, } if req.ReviewCount != nil { tp.ReviewCount = *req.ReviewCount diff --git a/apps/backend/internal/github/mock_controller_merge_queue_status_test.go b/apps/backend/internal/github/mock_controller_merge_queue_status_test.go new file mode 100644 index 0000000000..56152f3b4c --- /dev/null +++ b/apps/backend/internal/github/mock_controller_merge_queue_status_test.go @@ -0,0 +1,18 @@ +package github + +import ( + "testing" + "time" +) + +func TestBuildTaskPRFromRequestCopiesMergeQueueStatus(t *testing.T) { + position := 9 + estimate := 360 + req := &associateTaskPRRequest{ + TaskID: "task-queue-fixture", PRNumber: 42, State: "open", + MergeQueueState: "queued", MergeQueuePosition: &position, + MergeQueueEstimatedTimeToMergeSeconds: &estimate, + } + tp := buildTaskPRFromRequest(req, time.Now().UTC()) + assertTaskPRMergeQueueFields(t, "mock fixture", tp, "queued", &position, &estimate) +} diff --git a/apps/backend/internal/github/models.go b/apps/backend/internal/github/models.go index 18e377370d..fa50773bcd 100644 --- a/apps/backend/internal/github/models.go +++ b/apps/backend/internal/github/models.go @@ -365,11 +365,15 @@ type PRStatus struct { ReviewState string `json:"review_state"` // "approved", "changes_requested", "pending", "" ChecksState string `json:"checks_state"` // "success", "failure", "pending", "" MergeableState string `json:"mergeable_state"` // "clean", "blocked", "behind", "dirty", "has_hooks", "unstable", "draft", "unknown", "" - ReviewCount int `json:"review_count"` - PendingReviewCount int `json:"pending_review_count"` - RequiredReviews *int `json:"required_reviews,omitempty"` // nil when no branch protection rule found - ChecksTotal int `json:"checks_total"` - ChecksPassing int `json:"checks_passing"` + MergeQueueState string `json:"merge_queue_state"` + MergeQueuePosition *int `json:"merge_queue_position,omitempty"` + // A nil estimate is an observed absence when mergeQueuePopulated is true. + MergeQueueEstimatedTimeToMergeSeconds *int `json:"merge_queue_estimated_time_to_merge_seconds,omitempty"` + ReviewCount int `json:"review_count"` + PendingReviewCount int `json:"pending_review_count"` + RequiredReviews *int `json:"required_reviews,omitempty"` // nil when no branch protection rule found + ChecksTotal int `json:"checks_total"` + ChecksPassing int `json:"checks_passing"` // ChecksPopulated reports whether the sync path actually computed // ChecksTotal / ChecksPassing. The batched GraphQL poller doesn't (it // only carries the rollup state), so SyncTaskPR uses this flag to @@ -403,6 +407,9 @@ type PRStatus struct { // closed-event actor with a non-empty login. REST and gh CLI single-PR // syncs can never set this — they have no closing-actor field to read. ClosureAttributionPopulated bool `json:"closure_attribution_populated,omitempty"` + // mergeQueuePopulated distinguishes a GraphQL observation (including a + // null mergeQueueEntry) from REST and gh CLI reads that have no queue data. + mergeQueuePopulated bool } // PRSearchPage is a paginated slice of PR search results, with the total @@ -447,24 +454,27 @@ type PRWatch struct { // repository this PR belongs to (multi-repo tasks can have one PR per repo). // Empty for legacy rows persisted before multi-repo support. type TaskPR struct { - ID string `json:"id" db:"id"` - WorkspaceID string `json:"workspace_id" db:"workspace_id"` - TaskID string `json:"task_id" db:"task_id"` - RepositoryID string `json:"repository_id,omitempty" db:"repository_id"` - Owner string `json:"owner" db:"owner"` - Repo string `json:"repo" db:"repo"` - PRNumber int `json:"pr_number" db:"pr_number"` - PRURL string `json:"pr_url" db:"pr_url"` - PRTitle string `json:"pr_title" db:"pr_title"` - HeadBranch string `json:"head_branch" db:"head_branch"` - BaseBranch string `json:"base_branch" db:"base_branch"` - AuthorLogin string `json:"author_login" db:"author_login"` - State string `json:"state" db:"state"` // open, closed, merged - ReviewState string `json:"review_state" db:"review_state"` // approved, changes_requested, pending, "" - ChecksState string `json:"checks_state" db:"checks_state"` // success, failure, pending, "" - MergeableState string `json:"mergeable_state" db:"mergeable_state"` // clean, blocked, behind, dirty, has_hooks, unstable, draft, unknown, "" - ReviewCount int `json:"review_count" db:"review_count"` - PendingReviewCount int `json:"pending_review_count" db:"pending_review_count"` + ID string `json:"id" db:"id"` + WorkspaceID string `json:"workspace_id" db:"workspace_id"` + TaskID string `json:"task_id" db:"task_id"` + RepositoryID string `json:"repository_id,omitempty" db:"repository_id"` + Owner string `json:"owner" db:"owner"` + Repo string `json:"repo" db:"repo"` + PRNumber int `json:"pr_number" db:"pr_number"` + PRURL string `json:"pr_url" db:"pr_url"` + PRTitle string `json:"pr_title" db:"pr_title"` + HeadBranch string `json:"head_branch" db:"head_branch"` + BaseBranch string `json:"base_branch" db:"base_branch"` + AuthorLogin string `json:"author_login" db:"author_login"` + State string `json:"state" db:"state"` // open, closed, merged + ReviewState string `json:"review_state" db:"review_state"` // approved, changes_requested, pending, "" + ChecksState string `json:"checks_state" db:"checks_state"` // success, failure, pending, "" + MergeableState string `json:"mergeable_state" db:"mergeable_state"` // clean, blocked, behind, dirty, has_hooks, unstable, draft, unknown, "" + MergeQueueState string `json:"merge_queue_state" db:"merge_queue_state"` + MergeQueuePosition *int `json:"merge_queue_position" db:"merge_queue_position"` + MergeQueueEstimatedTimeToMergeSeconds *int `json:"merge_queue_estimated_time_to_merge_seconds" db:"merge_queue_estimated_time_to_merge_seconds"` + ReviewCount int `json:"review_count" db:"review_count"` + PendingReviewCount int `json:"pending_review_count" db:"pending_review_count"` // RequiredReviews is the branch protection's required_approving_review_count. // Nil when no protection rule exists or the token lacks scope to read it. RequiredReviews *int `json:"required_reviews,omitempty" db:"required_reviews"` diff --git a/apps/backend/internal/github/service_pr_merge_queue_status_test.go b/apps/backend/internal/github/service_pr_merge_queue_status_test.go new file mode 100644 index 0000000000..3b2c7f21ba --- /dev/null +++ b/apps/backend/internal/github/service_pr_merge_queue_status_test.go @@ -0,0 +1,99 @@ +package github + +import ( + "context" + "testing" + "time" +) + +func TestSyncTaskPRPersistsMergeQueueObservationAndPublishesChange(t *testing.T) { + svc, store, eventBus := setupSyncTest(t) + ctx := context.Background() + position := 3 + estimate := 125 + taskPR := &TaskPR{ + TaskID: "task-queue-sync", Owner: "owner", Repo: "repo", PRNumber: 42, + PRURL: "https://example.test/42", PRTitle: "queued", State: "open", + MergeQueueState: "", CreatedAt: nowUTC(), + } + if err := store.CreateTaskPR(ctx, taskPR); err != nil { + t.Fatalf("seed TaskPR: %v", err) + } + status := &PRStatus{ + PR: &PR{Number: 42, State: "open", RepoOwner: "owner", RepoName: "repo"}, + MergeQueueState: "queued", MergeQueuePosition: &position, + MergeQueueEstimatedTimeToMergeSeconds: &estimate, mergeQueuePopulated: true, + } + if err := svc.SyncTaskPR(ctx, taskPR.TaskID, status); err != nil { + t.Fatalf("SyncTaskPR: %v", err) + } + got := mustGetTaskPR(t, store, ctx, taskPR.ID) + assertTaskPRMergeQueueFields(t, "SyncTaskPR", got, "queued", &position, &estimate) + if eventBus.publishedCount() != 1 { + t.Fatalf("published events = %d, want 1", eventBus.publishedCount()) + } +} + +func TestSyncTaskPRPreservesQueueOnQueueUnawareRead(t *testing.T) { + svc, store, _ := setupSyncTest(t) + ctx := context.Background() + position := 6 + estimate := 240 + taskPR := &TaskPR{ + TaskID: "task-queue-preserve", Owner: "owner", Repo: "repo", PRNumber: 43, + PRURL: "https://example.test/43", PRTitle: "queued", State: "open", + MergeQueueState: "queued", MergeQueuePosition: &position, + MergeQueueEstimatedTimeToMergeSeconds: &estimate, CreatedAt: nowUTC(), + } + if err := store.CreateTaskPR(ctx, taskPR); err != nil { + t.Fatalf("seed TaskPR: %v", err) + } + if err := svc.SyncTaskPR(ctx, taskPR.TaskID, &PRStatus{ + PR: &PR{Number: 43, State: "open", RepoOwner: "owner", RepoName: "repo"}, + }); err != nil { + t.Fatalf("queue-unaware SyncTaskPR: %v", err) + } + assertTaskPRMergeQueueFields(t, "queue-unaware SyncTaskPR", mustGetTaskPR(t, store, ctx, taskPR.ID), "queued", &position, &estimate) +} + +func TestSyncTaskPRClearsQueueOnAuthoritativeExitAndTerminalState(t *testing.T) { + tests := []struct { + name string + state string + }{ + {name: "queue exit", state: "open"}, + {name: "merged", state: "merged"}, + {name: "closed", state: "closed"}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + svc, store, _ := setupSyncTest(t) + ctx := context.Background() + position := 2 + estimate := 60 + taskPR := &TaskPR{ + TaskID: "task-queue-clear-" + test.name, Owner: "owner", Repo: "repo", PRNumber: 44, + PRURL: "https://example.test/44", PRTitle: "queued", State: "open", + MergeQueueState: "queued", MergeQueuePosition: &position, + MergeQueueEstimatedTimeToMergeSeconds: &estimate, CreatedAt: nowUTC(), + } + if err := store.CreateTaskPR(ctx, taskPR); err != nil { + t.Fatalf("seed TaskPR: %v", err) + } + status := &PRStatus{ + PR: &PR{Number: 44, State: test.state, RepoOwner: "owner", RepoName: "repo"}, + } + if test.name == "queue exit" { + status.mergeQueuePopulated = true + } + if err := svc.SyncTaskPR(ctx, taskPR.TaskID, status); err != nil { + t.Fatalf("SyncTaskPR: %v", err) + } + assertTaskPRMergeQueueFields(t, test.name, mustGetTaskPR(t, store, ctx, taskPR.ID), "", nil, nil) + }) + } +} + +func nowUTC() time.Time { + return time.Now().UTC() +} diff --git a/apps/backend/internal/github/service_pr_watch.go b/apps/backend/internal/github/service_pr_watch.go index e5682be17c..c2179f8bbf 100644 --- a/apps/backend/internal/github/service_pr_watch.go +++ b/apps/backend/internal/github/service_pr_watch.go @@ -741,6 +741,8 @@ type taskPRSyncState struct { unresolved, reviewCount, pendingReviewCount int requiredReviews *int baseBranch, mergeableState, state string + mergeQueueState string + mergeQueuePosition, mergeQueueEstimate *int mergedAt, closedAt *time.Time isDraft *bool changedFiles *int @@ -748,6 +750,26 @@ type taskPRSyncState struct { autoMergeObservedAt *time.Time } +func resolveTaskPRMergeQueueFields(tp *TaskPR, status *PRStatus) (string, *int, *int) { + var state string + var position, estimate *int + if tp != nil { + state = tp.MergeQueueState + position = tp.MergeQueuePosition + estimate = tp.MergeQueueEstimatedTimeToMergeSeconds + } + if status == nil || status.PR == nil { + return state, position, estimate + } + if strings.EqualFold(status.PR.State, prStateMerged) || strings.EqualFold(status.PR.State, prStateClosed) { + return "", nil, nil + } + if !status.mergeQueuePopulated { + return state, position, estimate + } + return normalizeMergeQueueState(status.MergeQueueState), positiveIntPtr(status.MergeQueuePosition), nonNegativeIntPtr(status.MergeQueueEstimatedTimeToMergeSeconds) +} + // resolveTaskPROutcomeFields applies the populated/preserve dance for the // five outcome-attribution columns, split out of prepareTaskPRSyncState to // keep that function's complexity within the repo's lint limits (see the @@ -873,6 +895,7 @@ func (s *Service) prepareTaskPRSyncState(ctx context.Context, tp *TaskPR, status if status.PR.Draft { nextMergeableState = "draft" } + mergeQueueState, mergeQueuePosition, mergeQueueEstimate := resolveTaskPRMergeQueueFields(tp, status) nextState, nextMergedAt, nextClosedAt := resolveTerminalMergeState(tp, status.PR) nextIsDraft, nextChangedFiles, nextMergedByLogin, nextClosedByLogin, nextAutoMergeObservedAt := resolveTaskPROutcomeFields(tp, status) @@ -880,6 +903,7 @@ func (s *Service) prepareTaskPRSyncState(ctx context.Context, tp *TaskPR, status checksTotal: nextChecksTotal, checksPassing: nextChecksPassing, unresolved: nextUnresolved, reviewCount: nextReviewCount, pendingReviewCount: nextPendingReviewCount, requiredReviews: nextRequiredReviews, baseBranch: nextBaseBranch, mergeableState: nextMergeableState, + mergeQueueState: mergeQueueState, mergeQueuePosition: mergeQueuePosition, mergeQueueEstimate: mergeQueueEstimate, state: nextState, mergedAt: nextMergedAt, closedAt: nextClosedAt, isDraft: nextIsDraft, changedFiles: nextChangedFiles, mergedByLogin: nextMergedByLogin, closedByLogin: nextClosedByLogin, @@ -899,6 +923,9 @@ func taskPRChangedFields(tp *TaskPR, status *PRStatus, next taskPRSyncState) []s changed = appendChangedField(changed, "review_state", tp.ReviewState != status.ReviewState) changed = appendChangedField(changed, "checks_state", tp.ChecksState != status.ChecksState) changed = appendChangedField(changed, "mergeable_state", tp.MergeableState != next.mergeableState) + changed = appendChangedField(changed, "merge_queue_state", tp.MergeQueueState != next.mergeQueueState) + changed = appendChangedField(changed, "merge_queue_position", !intPtrEqual(tp.MergeQueuePosition, next.mergeQueuePosition)) + changed = appendChangedField(changed, "merge_queue_estimated_time_to_merge_seconds", !intPtrEqual(tp.MergeQueueEstimatedTimeToMergeSeconds, next.mergeQueueEstimate)) changed = appendChangedField(changed, "review_count", tp.ReviewCount != next.reviewCount) changed = appendChangedField( changed, @@ -969,6 +996,9 @@ func (s *Service) SyncTaskPR(ctx context.Context, taskID string, status *PRStatu tp.ReviewState = status.ReviewState tp.ChecksState = status.ChecksState tp.MergeableState = next.mergeableState + tp.MergeQueueState = next.mergeQueueState + tp.MergeQueuePosition = next.mergeQueuePosition + tp.MergeQueueEstimatedTimeToMergeSeconds = next.mergeQueueEstimate tp.ReviewCount = next.reviewCount tp.PendingReviewCount = next.pendingReviewCount tp.RequiredReviews = next.requiredReviews diff --git a/apps/backend/internal/github/store.go b/apps/backend/internal/github/store.go index c7c91c72e7..3867b1fa5c 100644 --- a/apps/backend/internal/github/store.go +++ b/apps/backend/internal/github/store.go @@ -122,6 +122,9 @@ const createTablesSQL = ` review_state TEXT NOT NULL DEFAULT '', checks_state TEXT NOT NULL DEFAULT '', mergeable_state TEXT NOT NULL DEFAULT '', + merge_queue_state TEXT NOT NULL DEFAULT '', + merge_queue_position INTEGER, + merge_queue_estimated_time_to_merge_seconds INTEGER, review_count INTEGER DEFAULT 0, pending_review_count INTEGER DEFAULT 0, required_reviews INTEGER, @@ -593,6 +596,35 @@ func (s *Store) initSchemaUpgrades() error { if err := s.addTaskPROutcomeColumns(); err != nil { return err } + if err := s.addTaskPRMergeQueueColumns(); err != nil { + return err + } + return nil +} + +var taskPRMergeQueueColumnDDL = []struct { + name string + ddl string +}{ + {"merge_queue_state", "TEXT NOT NULL DEFAULT ''"}, + {"merge_queue_position", "INTEGER"}, + {"merge_queue_estimated_time_to_merge_seconds", "INTEGER"}, +} + +func (s *Store) addTaskPRMergeQueueColumns() error { + columns, err := s.tableColumns("github_task_prs") + if err != nil { + return fmt.Errorf("read github_task_prs columns: %w", err) + } + for _, column := range taskPRMergeQueueColumnDDL { + if _, ok := columns[column.name]; ok { + continue + } + stmt := "ALTER TABLE github_task_prs ADD COLUMN " + column.name + " " + column.ddl + if _, err := s.db.Exec(stmt); err != nil && !dbutil.IsDuplicateColumnError(err) { + return fmt.Errorf("add github_task_prs.%s: %w", column.name, err) + } + } return nil } @@ -1331,6 +1363,9 @@ func (s *Store) migratePRTablesForMultiRepo() error { review_state TEXT NOT NULL DEFAULT '', checks_state TEXT NOT NULL DEFAULT '', mergeable_state TEXT NOT NULL DEFAULT '', + merge_queue_state TEXT NOT NULL DEFAULT '', + merge_queue_position INTEGER, + merge_queue_estimated_time_to_merge_seconds INTEGER, review_count INTEGER DEFAULT 0, pending_review_count INTEGER DEFAULT 0, required_reviews INTEGER, @@ -1361,13 +1396,15 @@ func (s *Store) migratePRTablesForMultiRepo() error { `INSERT INTO github_task_prs_new ( 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, + mergeable_state, merge_queue_state, merge_queue_position, merge_queue_estimated_time_to_merge_seconds, + review_count, pending_review_count, comment_count, additions, deletions, created_at, merged_at, closed_at, last_synced_at, detached_at, updated_at, is_draft, changed_files, merged_by_login, closed_by_login, auto_merge_observed_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, + mergeable_state, merge_queue_state, merge_queue_position, merge_queue_estimated_time_to_merge_seconds, + review_count, pending_review_count, comment_count, additions, deletions, created_at, merged_at, closed_at, last_synced_at, detached_at, updated_at, is_draft, changed_files, merged_by_login, closed_by_login, auto_merge_observed_at FROM github_task_prs`, @@ -1685,13 +1722,16 @@ func (s *Store) CreateTaskPR(ctx context.Context, tp *TaskPR) error { tp.UpdatedAt = now _, err := s.db.ExecContext(ctx, ` 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, + state, review_state, checks_state, mergeable_state, merge_queue_state, merge_queue_position, merge_queue_estimated_time_to_merge_seconds, 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, detached_at, updated_at, is_draft, changed_files, merged_by_login, closed_by_login, auto_merge_observed_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + 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.State, tp.ReviewState, tp.ChecksState, tp.MergeableState, tp.MergeQueueState, tp.MergeQueuePosition, tp.MergeQueueEstimatedTimeToMergeSeconds, 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.DetachedAt, tp.UpdatedAt, tp.IsDraft, tp.ChangedFiles, tp.MergedByLogin, tp.ClosedByLogin, tp.AutoMergeObservedAt) @@ -1709,7 +1749,7 @@ func (s *Store) CreateTaskPR(ctx context.Context, tp *TaskPR) error { // read working regardless of what the table has picked up beyond them. const taskPRColumns = `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, + mergeable_state, merge_queue_state, merge_queue_position, merge_queue_estimated_time_to_merge_seconds, 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, detached_at, updated_at, is_draft, changed_files, merged_by_login, closed_by_login, auto_merge_observed_at` @@ -1718,7 +1758,7 @@ const taskPRColumns = `id, workspace_id, task_id, repository_id, owner, repo, pr // `gtp` alias, for queries that join github_task_prs against another table. const taskPRColumnsQualified = `gtp.id, gtp.workspace_id, gtp.task_id, gtp.repository_id, gtp.owner, gtp.repo, gtp.pr_number, gtp.pr_url, gtp.pr_title, gtp.head_branch, gtp.base_branch, gtp.author_login, - gtp.state, gtp.review_state, gtp.checks_state, gtp.mergeable_state, gtp.review_count, + gtp.state, gtp.review_state, gtp.checks_state, gtp.mergeable_state, gtp.merge_queue_state, gtp.merge_queue_position, gtp.merge_queue_estimated_time_to_merge_seconds, 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.detached_at, gtp.updated_at, @@ -1883,16 +1923,18 @@ func (s *Store) RestoreTaskPR(ctx context.Context, taskID, repositoryID string, isDraft, changedFiles, mergedByLogin, closedByLogin, autoMergeObservedAt := resolveTaskPROutcomeFields(&outgoing, status) + mergeQueueState, mergeQueuePosition, mergeQueueEstimate := resolveTaskPRMergeQueueFields(&outgoing, status) if _, err := tx.ExecContext(ctx, `UPDATE github_task_prs SET owner = ?, repo = ?, pr_url = ?, pr_title = ?, head_branch = ?, base_branch = ?, author_login = ?, state = ?, mergeable_state = ?, + merge_queue_state = ?, merge_queue_position = ?, merge_queue_estimated_time_to_merge_seconds = ?, additions = ?, deletions = ?, merged_at = ?, closed_at = ?, detached_at = NULL, updated_at = ?, is_draft = ?, changed_files = ?, merged_by_login = ?, closed_by_login = ?, auto_merge_observed_at = COALESCE(auto_merge_observed_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(), + pr.State, pr.MergeableState, mergeQueueState, mergeQueuePosition, mergeQueueEstimate, pr.Additions, pr.Deletions, pr.MergedAt, pr.ClosedAt, time.Now().UTC(), isDraft, changedFiles, mergedByLogin, closedByLogin, autoMergeObservedAt, taskID, repositoryID, pr.Number); err != nil { return nil, err @@ -2045,6 +2087,12 @@ func (s *Store) ReplaceTaskPR(ctx context.Context, tp *TaskPR, status *PRStatus) } tp.IsDraft, tp.ChangedFiles, tp.MergedByLogin, tp.ClosedByLogin, tp.AutoMergeObservedAt = resolveTaskPROutcomeFields(outgoing, status) + queueSource := tp + if outgoing.ID != "" { + queueSource = outgoing + } + tp.MergeQueueState, tp.MergeQueuePosition, tp.MergeQueueEstimatedTimeToMergeSeconds = + resolveTaskPRMergeQueueFields(queueSource, status) if tp.RepositoryID != "" { if _, err := tx.ExecContext(ctx, @@ -2063,13 +2111,13 @@ func (s *Store) ReplaceTaskPR(ctx context.Context, tp *TaskPR, status *PRStatus) } if _, err := tx.ExecContext(ctx, ` 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, + state, review_state, checks_state, mergeable_state, merge_queue_state, merge_queue_position, merge_queue_estimated_time_to_merge_seconds, 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, detached_at, updated_at, is_draft, changed_files, merged_by_login, closed_by_login, auto_merge_observed_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + 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.State, tp.ReviewState, tp.ChecksState, tp.MergeableState, tp.MergeQueueState, tp.MergeQueuePosition, tp.MergeQueueEstimatedTimeToMergeSeconds, 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.DetachedAt, tp.UpdatedAt, tp.IsDraft, tp.ChangedFiles, tp.MergedByLogin, tp.ClosedByLogin, tp.AutoMergeObservedAt); err != nil { @@ -2124,6 +2172,7 @@ func (s *Store) UpdateTaskPR(ctx context.Context, tp *TaskPR) error { tp.UpdatedAt = time.Now().UTC() _, err := s.db.ExecContext(ctx, ` UPDATE github_task_prs SET state = ?, review_state = ?, checks_state = ?, mergeable_state = ?, + merge_queue_state = ?, merge_queue_position = ?, merge_queue_estimated_time_to_merge_seconds = ?, review_count = ?, pending_review_count = ?, required_reviews = ?, comment_count = ?, unresolved_review_threads = ?, checks_total = ?, checks_passing = ?, additions = ?, deletions = ?, pr_title = ?, base_branch = ?, @@ -2131,7 +2180,7 @@ func (s *Store) UpdateTaskPR(ctx context.Context, tp *TaskPR) error { is_draft = ?, changed_files = ?, merged_by_login = ?, closed_by_login = ?, auto_merge_observed_at = COALESCE(auto_merge_observed_at, ?) WHERE id = ?`, - tp.State, tp.ReviewState, tp.ChecksState, tp.MergeableState, + tp.State, tp.ReviewState, tp.ChecksState, tp.MergeableState, tp.MergeQueueState, tp.MergeQueuePosition, tp.MergeQueueEstimatedTimeToMergeSeconds, tp.ReviewCount, tp.PendingReviewCount, tp.RequiredReviews, tp.CommentCount, tp.UnresolvedReviewThreads, tp.ChecksTotal, tp.ChecksPassing, tp.Additions, tp.Deletions, tp.PRTitle, tp.BaseBranch, diff --git a/apps/backend/internal/github/store_merge_queue_status_test.go b/apps/backend/internal/github/store_merge_queue_status_test.go new file mode 100644 index 0000000000..8a74a45908 --- /dev/null +++ b/apps/backend/internal/github/store_merge_queue_status_test.go @@ -0,0 +1,140 @@ +package github + +import ( + "context" + "strings" + "testing" + "time" +) + +var taskPRMergeQueueColumnNames = []string{ + "merge_queue_state", + "merge_queue_position", + "merge_queue_estimated_time_to_merge_seconds", +} + +func TestTaskPRMergeQueueSchemaIsReplayable(t *testing.T) { + store := newTestStore(t) + + columns, err := store.tableColumns("github_task_prs") + if err != nil { + t.Fatalf("read github_task_prs columns: %v", err) + } + for _, name := range taskPRMergeQueueColumnNames { + if _, ok := columns[name]; !ok { + t.Errorf("github_task_prs is missing %q", name) + } + } + if err := store.initSchema(false); err != nil { + t.Fatalf("replay github schema: %v", err) + } +} + +func TestTaskPRMergeQueueColumnsRoundTripEveryWritePath(t *testing.T) { + store := newTestStore(t) + ctx := context.Background() + position := 4 + estimate := 125 + now := time.Now().UTC().Truncate(time.Second) + seed := &TaskPR{ + TaskID: "task-queue-columns", WorkspaceID: "workspace-queue-columns", + Owner: "owner", Repo: "repo", PRNumber: 42, PRURL: "https://example.test/42", + PRTitle: "queued", HeadBranch: "feature", BaseBranch: "main", State: "open", + MergeQueueState: "queued", MergeQueuePosition: &position, + MergeQueueEstimatedTimeToMergeSeconds: &estimate, CreatedAt: now, + } + if err := store.CreateTaskPR(ctx, seed); err != nil { + t.Fatalf("CreateTaskPR: %v", err) + } + assertTaskPRMergeQueueFields(t, "CreateTaskPR", mustGetTaskPR(t, store, ctx, seed.ID), "queued", &position, &estimate) + + position = 5 + estimate = 185 + seed.MergeQueueState = "awaiting_checks" + if err := store.UpdateTaskPR(ctx, seed); err != nil { + t.Fatalf("UpdateTaskPR: %v", err) + } + assertTaskPRMergeQueueFields(t, "UpdateTaskPR", mustGetTaskPR(t, store, ctx, seed.ID), "awaiting_checks", &position, &estimate) + + replacedPosition := 2 + replacedEstimate := 60 + replaced := &TaskPR{ + TaskID: seed.TaskID, WorkspaceID: seed.WorkspaceID, Owner: seed.Owner, Repo: seed.Repo, + PRNumber: seed.PRNumber, PRURL: seed.PRURL, PRTitle: "replaced", HeadBranch: seed.HeadBranch, + BaseBranch: seed.BaseBranch, State: "open", CreatedAt: now, + MergeQueueState: "mergeable", MergeQueuePosition: &replacedPosition, + MergeQueueEstimatedTimeToMergeSeconds: &replacedEstimate, + } + if _, err := store.ReplaceTaskPR(ctx, replaced, &PRStatus{ + PR: &PR{State: "open"}, + MergeQueueState: "mergeable", MergeQueuePosition: &replacedPosition, + MergeQueueEstimatedTimeToMergeSeconds: &replacedEstimate, mergeQueuePopulated: true, + }); err != nil { + t.Fatalf("ReplaceTaskPR: %v", err) + } + assertTaskPRMergeQueueFields(t, "ReplaceTaskPR", mustGetTaskPR(t, store, ctx, replaced.ID), "mergeable", &replacedPosition, &replacedEstimate) +} + +func TestTaskPRColumnListsIncludeMergeQueueColumns(t *testing.T) { + for _, name := range taskPRMergeQueueColumnNames { + if !strings.Contains(taskPRColumns, name) { + t.Errorf("taskPRColumns missing %q", name) + } + if !strings.Contains(taskPRColumnsQualified, "gtp."+name) { + t.Errorf("taskPRColumnsQualified missing %q", name) + } + } +} + +func TestRestoreTaskPRPreservesMergeQueueFields(t *testing.T) { + store := newTestStore(t) + ctx := context.Background() + position := 3 + estimate := 90 + seed := &TaskPR{ + TaskID: "task-queue-restore", WorkspaceID: "workspace-queue-restore", + Owner: "owner", Repo: "repo", PRNumber: 43, PRURL: "https://example.test/43", + PRTitle: "queued restore", State: "open", CreatedAt: time.Now().UTC(), + MergeQueueState: "awaiting_checks", MergeQueuePosition: &position, + MergeQueueEstimatedTimeToMergeSeconds: &estimate, + } + if err := store.CreateTaskPR(ctx, seed); err != nil { + t.Fatalf("CreateTaskPR: %v", err) + } + if _, changed, err := store.DetachTaskPR(ctx, seed.ID); err != nil || !changed { + t.Fatalf("DetachTaskPR: changed=%v err=%v", changed, err) + } + + restored, err := store.RestoreTaskPR(ctx, seed.TaskID, seed.RepositoryID, &PRStatus{ + PR: &PR{Number: seed.PRNumber, RepoOwner: seed.Owner, RepoName: seed.Repo, HTMLURL: seed.PRURL, Title: seed.PRTitle, State: "open"}, + }) + if err != nil { + t.Fatalf("RestoreTaskPR: %v", err) + } + assertTaskPRMergeQueueFields(t, "RestoreTaskPR", restored, seed.MergeQueueState, &position, &estimate) +} + +func mustGetTaskPR(t *testing.T, store *Store, ctx context.Context, id string) *TaskPR { + t.Helper() + got, err := store.GetTaskPRByID(ctx, id) + if err != nil { + t.Fatalf("GetTaskPRByID: %v", err) + } + if got == nil { + t.Fatalf("TaskPR %q not found", id) + } + return got +} + +func assertTaskPRMergeQueueFields(t *testing.T, name string, got *TaskPR, state string, position, estimate *int) { + t.Helper() + if got.MergeQueueState != state { + t.Errorf("%s: MergeQueueState = %q, want %q", name, got.MergeQueueState, state) + } + if !intPtrEqual(got.MergeQueuePosition, position) { + t.Errorf("%s: MergeQueuePosition = %v, want %v", name, got.MergeQueuePosition, position) + } + if !intPtrEqual(got.MergeQueueEstimatedTimeToMergeSeconds, estimate) { + t.Errorf("%s: MergeQueueEstimatedTimeToMergeSeconds = %v, want %v", name, got.MergeQueueEstimatedTimeToMergeSeconds, estimate) + } +} diff --git a/apps/backend/internal/task/statussummary/constants.go b/apps/backend/internal/task/statussummary/constants.go index b81e4095d5..c34e7631df 100644 --- a/apps/backend/internal/task/statussummary/constants.go +++ b/apps/backend/internal/task/statussummary/constants.go @@ -29,6 +29,7 @@ const ( prStatePending = "pending" prStateAwaiting = "awaiting_review" prStateReady = "ready" + prStateQueued = "queued" prStatePassing = "passing" prStateNeutral = "neutral" prStateOpen = "open" diff --git a/apps/backend/internal/task/statussummary/merge_queue_status_test.go b/apps/backend/internal/task/statussummary/merge_queue_status_test.go new file mode 100644 index 0000000000..35c7038b18 --- /dev/null +++ b/apps/backend/internal/task/statussummary/merge_queue_status_test.go @@ -0,0 +1,78 @@ +package statussummary + +import ( + "context" + "testing" + + "github.com/kandev/kandev/internal/events" + "github.com/kandev/kandev/internal/events/bus" +) + +func TestQueuedPullRequestIsProjectedByLiveAndRestartPaths(t *testing.T) { + input := PullRequestInput{ + Key: "repo-1#42", State: prStateOpen, Number: 42, + URL: "https://example.test/42", MergeQueueState: "queued", + ChecksState: prStateSuccess, + } + rebuilt := BuildFromAuthoritative(RebuildInput{PRObserved: true, PullRequests: []PullRequestInput{input}}) + if rebuilt.PullRequest == nil || rebuilt.PullRequest.AggregateState != prStateQueued { + t.Fatalf("rebuilt pull request summary = %+v, want queued", rebuilt.PullRequest) + } + + projector, store, _, _, _ := newProjectorTest(t) + if err := projector.HandleEvent(context.Background(), bus.NewEvent(events.GitHubTaskPRUpdated, "test", map[string]interface{}{ + "task_id": "task-queue-live", + "repository_id": "repo-1", + "pr_number": 42, + "state": prStateOpen, + "pr_url": input.URL, + "merge_queue_state": "queued", + "checks_state": prStateSuccess, + })); err != nil { + t.Fatalf("live PR event: %v", err) + } + live := store.summary("task-queue-live") + if live == nil || live.PullRequest == nil || live.PullRequest.AggregateState != prStateQueued { + t.Fatalf("live pull request summary = %+v, want queued", live) + } +} + +func TestQueuedPullRequestRanksBelowMoreAttentionWorthySibling(t *testing.T) { + got := BuildFromAuthoritative(RebuildInput{ + PRObserved: true, + PullRequests: []PullRequestInput{ + {Key: "queued", State: prStateOpen, Number: 1, MergeQueueState: "queued"}, + {Key: "failed", State: prStateOpen, Number: 2, ChecksState: prStateFailure}, + }, + }) + if got.PullRequest == nil || got.PullRequest.AggregateState != prStateFailure { + t.Fatalf("aggregate state = %+v, want failure over queued sibling", got.PullRequest) + } +} + +func TestQueuedPullRequestTakesPrecedenceOverItsOtherNonTerminalStates(t *testing.T) { + got := BuildFromAuthoritative(RebuildInput{ + PRObserved: true, + PullRequests: []PullRequestInput{{ + Key: "queued", State: prStateOpen, Number: 1, + MergeQueueState: "queued", MergeableState: "dirty", + ReviewState: prStateChanges, ChecksState: prStateFailure, + }}, + }) + if got.PullRequest == nil || got.PullRequest.AggregateState != prStateQueued { + t.Fatalf("aggregate state = %+v, want queued", got.PullRequest) + } +} + +func TestQueuedPullRequestTakesPrecedenceOverDraftMergeability(t *testing.T) { + got := BuildFromAuthoritative(RebuildInput{ + PRObserved: true, + PullRequests: []PullRequestInput{{ + Key: "queued", State: prStateOpen, Number: 1, + MergeQueueState: "queued", MergeableState: prStateDraft, + }}, + }) + if got.PullRequest == nil || got.PullRequest.AggregateState != prStateQueued { + t.Fatalf("aggregate state = %+v, want queued", got.PullRequest) + } +} diff --git a/apps/backend/internal/task/statussummary/projector.go b/apps/backend/internal/task/statussummary/projector.go index a89a979cb9..c81e2262af 100644 --- a/apps/backend/internal/task/statussummary/projector.go +++ b/apps/backend/internal/task/statussummary/projector.go @@ -183,6 +183,7 @@ type pullRequestObservation struct { reviewState string checksState string mergeableState string + mergeQueueState string unresolvedReviewCount int pendingReviewCount int requiredReviews int diff --git a/apps/backend/internal/task/statussummary/projector_events.go b/apps/backend/internal/task/statussummary/projector_events.go index e2b52fd0a1..595b1ddf5e 100644 --- a/apps/backend/internal/task/statussummary/projector_events.go +++ b/apps/backend/internal/task/statussummary/projector_events.go @@ -771,6 +771,7 @@ func (p *Projector) applyPREventLocked(state *projectionState, data map[string]i reviewState: stringField(data, "review_state"), checksState: stringField(data, "checks_state"), mergeableState: stringField(data, "mergeable_state"), + mergeQueueState: stringField(data, "merge_queue_state"), unresolvedReviewCount: intValueOrZero(data["unresolved_review_threads"]), pendingReviewCount: intValueOrZero(data["pending_review_count"]), checksTotal: intValueOrZero(data["checks_total"]), diff --git a/apps/backend/internal/task/statussummary/projector_pr.go b/apps/backend/internal/task/statussummary/projector_pr.go index c8227f53a8..a5f9bea140 100644 --- a/apps/backend/internal/task/statussummary/projector_pr.go +++ b/apps/backend/internal/task/statussummary/projector_pr.go @@ -25,6 +25,7 @@ func applyPullRequestInputs(state *projectionState, inputs []PullRequestInput) { reviewState: input.ReviewState, checksState: input.ChecksState, mergeableState: input.MergeableState, + mergeQueueState: input.MergeQueueState, unresolvedReviewCount: maxInt(input.UnresolvedReviewCount, 0), pendingReviewCount: maxInt(input.PendingReviewCount, 0), requiredReviews: maxInt(input.RequiredReviews, 0), @@ -121,6 +122,12 @@ func pullRequestAggregateState(pr pullRequestObservation) string { mergeable := strings.ToLower(strings.TrimSpace(pr.mergeableState)) checks := strings.ToLower(strings.TrimSpace(pr.checksState)) review := strings.ToLower(strings.TrimSpace(pr.reviewState)) + if state == prStateMerged || state == prStateClosed { + return pullRequestLifecycleState(state, mergeable) + } + if strings.TrimSpace(pr.mergeQueueState) != "" { + return prStateQueued + } if lifecycle := pullRequestLifecycleState(state, mergeable); lifecycle != "" { return lifecycle } @@ -186,6 +193,8 @@ func pullRequestStateRank(state string) int { return 70 case prStateReady: return 60 + case prStateQueued: + return 55 case prStatePassing: return 50 case prStateDraft: diff --git a/apps/backend/internal/task/statussummary/rebuild.go b/apps/backend/internal/task/statussummary/rebuild.go index 7c683148f6..57914dfe84 100644 --- a/apps/backend/internal/task/statussummary/rebuild.go +++ b/apps/backend/internal/task/statussummary/rebuild.go @@ -35,6 +35,7 @@ type PullRequestInput struct { ReviewState string ChecksState string MergeableState string + MergeQueueState string UnresolvedReviewCount int PendingReviewCount int RequiredReviews int diff --git a/apps/web/components/github/pr-detail-panel.test.tsx b/apps/web/components/github/pr-detail-panel.test.tsx index 9c6f4a9472..c0e1f20fb8 100644 --- a/apps/web/components/github/pr-detail-panel.test.tsx +++ b/apps/web/components/github/pr-detail-panel.test.tsx @@ -9,6 +9,8 @@ import type { PRFeedback, TaskPR } from "@/lib/types/github"; import { getPRFeedback } from "@/lib/api/domains/github-api"; import { PRDetailPanelComponent } from "./pr-detail-panel"; +const BOT_COMMENTS_LABEL = "Bot comments (1)"; + vi.mock("@/lib/api/domains/github-api", async (importOriginal) => { const actual = await importOriginal(); return { @@ -58,14 +60,14 @@ function makePR(overrides: Partial = {}): TaskPR { }; } -function makeFeedback(pr: TaskPR, botCommentBody: string): PRFeedback { +function makeFeedback(pr: TaskPR, botCommentBody: string, state = "open"): PRFeedback { return { pr: { number: pr.pr_number, title: pr.pr_title, url: pr.pr_url, html_url: pr.pr_url, - state: "open", + state, head_branch: pr.head_branch, base_branch: pr.base_branch, author_login: pr.author_login, @@ -161,8 +163,8 @@ describe("PRDetailPanelComponent — task switch on the singleton legacy panel", const { getStoreApi } = renderPanel(taskState("task-1", "session-1", [pr1])); // Task 1: expand the bot-comments disclosure. - await waitFor(() => expect(screen.getByText("Bot comments (1)")).toBeTruthy()); - fireEvent.click(screen.getByText("Bot comments (1)")); + await waitFor(() => expect(screen.getByText(BOT_COMMENTS_LABEL)).toBeTruthy()); + fireEvent.click(screen.getByText(BOT_COMMENTS_LABEL)); await waitFor(() => expect(screen.getByText("PR 1 bot comment")).toBeTruthy()); // Switch to a different task whose PR also has exactly one bot comment — @@ -175,10 +177,24 @@ describe("PRDetailPanelComponent — task switch on the singleton legacy panel", })); }); - await waitFor(() => expect(screen.getByText("Bot comments (1)")).toBeTruthy()); + await waitFor(() => expect(screen.getByText(BOT_COMMENTS_LABEL)).toBeTruthy()); // Task 2's bot comment must stay collapsed by default, not inherit task 1's // expanded disclosure state. expect(screen.queryByText("PR 2 bot comment")).toBeNull(); expect(screen.queryByText("PR 1 bot comment")).toBeNull(); }); + + it("does not show a stale queue notice after live feedback closes the PR", async () => { + const pr = makePR({ + merge_queue_state: "queued", + merge_queue_position: 2, + merge_queue_estimated_time_to_merge_seconds: 120, + }); + vi.mocked(getPRFeedback).mockResolvedValue(makeFeedback(pr, "Closed PR", "closed")); + + renderPanel(taskState("task-1", "session-1", [pr])); + + await waitFor(() => expect(screen.getByText(BOT_COMMENTS_LABEL)).toBeTruthy()); + expect(screen.queryByTestId("pr-merge-queue-status")).toBeNull(); + }); }); diff --git a/apps/web/components/github/pr-detail-panel.tsx b/apps/web/components/github/pr-detail-panel.tsx index 8d16d99e39..da1b822e5c 100644 --- a/apps/web/components/github/pr-detail-panel.tsx +++ b/apps/web/components/github/pr-detail-panel.tsx @@ -22,6 +22,7 @@ import { submitPRReview } from "@/lib/api/domains/github-pr-api"; import type { TaskPR, PRFeedback } from "@/lib/types/github"; import { PRMergeButton } from "./pr-merge-button"; import { PRMergeabilityNotice, buildConflictResolutionMessage } from "./pr-mergeability-notice"; +import { hasActiveMergeQueueEntry, PRMergeQueueStatus } from "./pr-merge-queue-status"; import { usePRScopedReviewRequest } from "./use-pr-scoped-review-request"; // --- Dockview panel wrapper --- @@ -426,6 +427,7 @@ export function PRDetailContent({ taskPR, sessionId }: { taskPR: TaskPR; session const isDraft = feedback?.pr.draft ?? false; const isMergeable = feedback?.pr.mergeable ?? true; const mergeableState = feedback?.pr.mergeable_state ?? taskPR.mergeable_state; + const displayPR = { ...taskPR, state: liveState }; return ( } notice={ - + hasActiveMergeQueueEntry(displayPR) ? ( + + ) : ( + + ) } /> ); diff --git a/apps/web/components/github/pr-merge-queue-status.test.tsx b/apps/web/components/github/pr-merge-queue-status.test.tsx new file mode 100644 index 0000000000..e9ec73db74 --- /dev/null +++ b/apps/web/components/github/pr-merge-queue-status.test.tsx @@ -0,0 +1,156 @@ +import { afterEach, describe, expect, it } from "vitest"; +import { cleanup, render } from "@testing-library/react"; +import type { TaskPR } from "@/lib/types/github"; +import { aggregatePRStatusColor, getPRStatusColor } from "./pr-task-icon"; +import { + describeMergeQueueEstimate, + getMergeQueueSummaryStatus, + PRMergeQueueStatus, +} from "./pr-merge-queue-status"; + +const QUEUE_STATUS_TEST_ID = "pr-merge-queue-status"; + +function makePR(overrides: Partial = {}): TaskPR { + return { + id: "id", + task_id: "task-1", + owner: "acme", + repo: "demo", + pr_number: 42, + pr_url: "", + pr_title: "Queued PR", + head_branch: "feature", + base_branch: "main", + author_login: "alice", + state: "open", + review_state: "", + checks_state: "success", + mergeable_state: "blocked", + review_count: 0, + pending_review_count: 0, + comment_count: 0, + unresolved_review_threads: 0, + checks_total: 1, + checks_passing: 1, + additions: 0, + deletions: 0, + created_at: "", + merged_at: null, + closed_at: null, + last_synced_at: null, + updated_at: "", + ...overrides, + }; +} + +afterEach(() => cleanup()); + +describe("merge queue estimate formatting", () => { + it.each([ + [undefined, { kind: "none" }], + [null, { kind: "none" }], + [0, { kind: "sub_minute" }], + [59, { kind: "sub_minute" }], + [60, { kind: "minutes", minutes: 1 }], + [61, { kind: "minutes", minutes: 2 }], + [3600, { kind: "minutes", minutes: 60 }], + ])("describes %s seconds without exposing raw provider units", (seconds, expected) => { + expect(describeMergeQueueEstimate(seconds)).toEqual(expected); + }); +}); + +describe("merge queue status", () => { + it.each([ + ["queued", "queue_queued"], + ["awaiting_checks", "queue_awaiting_checks"], + ["mergeable", "queue_mergeable"], + ["unmergeable", "queue_unmergeable"], + ["locked", "queue_locked"], + ["future_state", "queue_queued"], + ])("maps %s to %s", (state, expected) => { + expect(getMergeQueueSummaryStatus(state)).toBe(expected); + }); + + it("uses generic queued copy for a future provider state", () => { + const { getByTestId } = render( + , + ); + + expect(getByTestId(QUEUE_STATUS_TEST_ID).textContent).toContain("Merge queue: Queued"); + }); + + it("renders queue state, one-based position, and an available estimate", () => { + const { getByTestId } = render( + , + ); + + const notice = getByTestId(QUEUE_STATUS_TEST_ID); + expect(notice.textContent).toContain("Merge queue: Queued"); + expect(notice.textContent).toContain("Position 4"); + expect(notice.textContent).toContain("2 minutes"); + }); + + it("renders a localized sub-minute estimate", () => { + const { getByTestId } = render( + , + ); + + expect(getByTestId(QUEUE_STATUS_TEST_ID).textContent).toContain("less than a minute"); + }); + + it("renders nothing for an empty or terminal queue state", () => { + const empty = render(); + expect(empty.queryByTestId(QUEUE_STATUS_TEST_ID)).toBeNull(); + empty.unmount(); + + const terminal = render( + , + ); + expect(terminal.queryByTestId(QUEUE_STATUS_TEST_ID)).toBeNull(); + }); + + it("keeps terminal colors ahead of a stale queue entry", () => { + expect(getPRStatusColor(makePR({ state: "merged", merge_queue_state: "queued" }))).toBe( + "text-purple-500", + ); + expect(getPRStatusColor(makePR({ state: "closed", merge_queue_state: "queued" }))).toBe( + "text-red-500", + ); + }); + + it("keeps queue color ahead of other non-terminal PR states", () => { + const overrides: Array> = [ + { checks_state: "failure" }, + { review_state: "changes_requested" }, + { mergeable_state: "dirty" }, + { mergeable_state: "behind" }, + { mergeable_state: "draft" }, + ]; + for (const override of overrides) { + expect(getPRStatusColor(makePR({ merge_queue_state: "queued", ...override }))).toBe( + "text-[#966600]", + ); + } + }); + + it("lets a failing sibling outrank a queued PR in the aggregate", () => { + expect( + aggregatePRStatusColor([ + makePR({ merge_queue_state: "queued" }), + makePR({ pr_number: 2, checks_state: "failure" }), + ]), + ).toBe("text-red-500"); + }); +}); diff --git a/apps/web/components/github/pr-merge-queue-status.tsx b/apps/web/components/github/pr-merge-queue-status.tsx new file mode 100644 index 0000000000..4e7b481e2f --- /dev/null +++ b/apps/web/components/github/pr-merge-queue-status.tsx @@ -0,0 +1,122 @@ +"use client"; + +import { IconClockHour4 } from "@tabler/icons-react"; +import { useTranslation } from "react-i18next"; +import type { ChangeRequestTaskSummaryRowDetail } from "@/components/integrations/change-request-task-status-summary"; +import type { TaskPR } from "@/lib/types/github"; + +export type MergeQueueSummaryStatus = + | "queue_queued" + | "queue_awaiting_checks" + | "queue_mergeable" + | "queue_unmergeable" + | "queue_locked"; + +export type MergeQueueEstimateDescription = + | { kind: "none" } + | { kind: "sub_minute" } + | { kind: "minutes"; minutes: number }; + +const KNOWN_QUEUE_STATES: Record = { + queued: "queue_queued", + awaiting_checks: "queue_awaiting_checks", + mergeable: "queue_mergeable", + unmergeable: "queue_unmergeable", + locked: "queue_locked", +}; + +const QUEUE_STATE_LABEL_KEYS: Record = { + queue_queued: "github:mergeQueueStateQueued", + queue_awaiting_checks: "github:mergeQueueStateAwaitingChecks", + queue_mergeable: "github:mergeQueueStateMergeable", + queue_unmergeable: "github:mergeQueueStateUnmergeable", + queue_locked: "github:mergeQueueStateLocked", +}; + +export function normalizeMergeQueueState(state: string | null | undefined): string { + return state?.trim().toLowerCase() ?? ""; +} + +export function getMergeQueueSummaryStatus( + state: string | null | undefined, +): MergeQueueSummaryStatus { + return KNOWN_QUEUE_STATES[normalizeMergeQueueState(state)] ?? "queue_queued"; +} + +export function describeMergeQueueEstimate( + seconds: number | null | undefined, +): MergeQueueEstimateDescription { + if (seconds == null || !Number.isFinite(seconds) || seconds < 0) { + return { kind: "none" }; + } + if (seconds < 60) return { kind: "sub_minute" }; + return { kind: "minutes", minutes: Math.ceil(seconds / 60) }; +} + +function validQueuePosition(position: number | null | undefined): number | null { + if (position == null || !Number.isInteger(position) || position < 1) return null; + return position; +} + +export function getMergeQueueSummaryDetail( + position: number | null | undefined, + estimateSeconds: number | null | undefined, +): ChangeRequestTaskSummaryRowDetail | undefined { + const queuePosition = validQueuePosition(position); + const estimate = describeMergeQueueEstimate(estimateSeconds); + if (queuePosition != null) { + if (estimate.kind === "sub_minute") { + return { + key: "github:mergeQueuePositionAndSubMinuteEstimate", + values: { position: queuePosition }, + }; + } + if (estimate.kind === "minutes") { + return { + key: "github:mergeQueuePositionAndEstimate", + values: { position: queuePosition, count: estimate.minutes }, + }; + } + return { key: "github:mergeQueuePosition", values: { position: queuePosition } }; + } + if (estimate.kind === "sub_minute") { + return { key: "github:mergeQueueSubMinuteEstimate" }; + } + if (estimate.kind === "minutes") { + return { key: "github:mergeQueueEstimate", values: { count: estimate.minutes } }; + } + return undefined; +} + +export function hasActiveMergeQueueEntry(pr: Pick): boolean { + return pr.state === "open" && normalizeMergeQueueState(pr.merge_queue_state) !== ""; +} + +export function PRMergeQueueStatus({ pr }: { pr: TaskPR }) { + const { t } = useTranslation(); + if (!hasActiveMergeQueueEntry(pr)) return null; + + const status = getMergeQueueSummaryStatus(pr.merge_queue_state); + const stateLabel = t(QUEUE_STATE_LABEL_KEYS[status]); + const detail = getMergeQueueSummaryDetail( + pr.merge_queue_position, + pr.merge_queue_estimated_time_to_merge_seconds, + ); + + return ( +
+
+ ); +} diff --git a/apps/web/components/github/pr-mergeability-row.test.tsx b/apps/web/components/github/pr-mergeability-row.test.tsx index c95786bf06..fab90f3e20 100644 --- a/apps/web/components/github/pr-mergeability-row.test.tsx +++ b/apps/web/components/github/pr-mergeability-row.test.tsx @@ -9,6 +9,8 @@ import type { AppState } from "@/lib/state/store"; import type { MergeableState, TaskPR } from "@/lib/types/github"; const SESSION_ID = "sess-1"; +const QUEUE_STATUS_TEST_ID = "pr-merge-queue-status"; +const CONFLICT_BANNER_TEST_ID = "pr-conflict-banner"; function makePR(overrides: Partial = {}): TaskPR { return { @@ -71,7 +73,7 @@ afterEach(() => cleanup()); describe("PRMergeabilityRow", () => { it("shows the conflict banner with a Resolve conflicts CTA for a dirty PR", () => { const { queryByTestId } = renderRow(makePR({ mergeable_state: "dirty" })); - expect(queryByTestId("pr-conflict-banner")).not.toBeNull(); + expect(queryByTestId(CONFLICT_BANNER_TEST_ID)).not.toBeNull(); expect(queryByTestId("pr-resolve-conflicts-button")).not.toBeNull(); }); @@ -79,7 +81,7 @@ describe("PRMergeabilityRow", () => { const { container, queryByTestId } = renderRow( makePR({ mergeable_state: "blocked", checks_state: "failure" }), ); - expect(queryByTestId("pr-conflict-banner")).toBeNull(); + expect(queryByTestId(CONFLICT_BANNER_TEST_ID)).toBeNull(); expect(queryByTestId("pr-blocked-note")).not.toBeNull(); expect(queryByTestId("pr-blocked-shield-icon")).not.toBeNull(); expect(container.textContent).toContain("Blocked by branch protection"); @@ -147,11 +149,49 @@ describe("PRMergeabilityRow", () => { it("hides the Resolve conflicts CTA when there is no active session", () => { const { queryByTestId } = renderRow(makePR({ mergeable_state: "dirty" }), null); // Banner still surfaces the conflict, but the CTA needs a session to target. - expect(queryByTestId("pr-conflict-banner")).not.toBeNull(); + expect(queryByTestId(CONFLICT_BANNER_TEST_ID)).not.toBeNull(); expect(queryByTestId("pr-resolve-conflicts-button")).toBeNull(); }); }); +describe("PRMergeabilityRow queue state", () => { + it("replaces mergeability with queue state and metadata for an active entry", () => { + const { getByTestId, queryByTestId, container } = renderRow( + makePR({ + merge_queue_state: "mergeable", + merge_queue_position: 2, + merge_queue_estimated_time_to_merge_seconds: 120, + mergeable_state: "dirty", + }), + ); + + expect(getByTestId(QUEUE_STATUS_TEST_ID)).not.toBeNull(); + expect(container.textContent).toContain("Merge queue: Mergeable"); + expect(container.textContent).toContain("Position 2"); + expect(container.textContent).toContain("2 minutes"); + expect(queryByTestId(CONFLICT_BANNER_TEST_ID)).toBeNull(); + }); + + it("keeps queue state visible when position and estimate are absent", () => { + const { getByTestId, container } = renderRow( + makePR({ merge_queue_state: "locked", mergeable_state: "blocked" }), + ); + + expect(getByTestId(QUEUE_STATUS_TEST_ID)).not.toBeNull(); + expect(container.textContent).toContain("Merge queue: Locked"); + expect(container.textContent).not.toContain("Position"); + }); + + it("does not show a stale queue entry after a terminal transition", () => { + const { queryByTestId, getByTestId } = renderRow( + makePR({ state: "merged", merge_queue_state: "queued" }), + ); + + expect(queryByTestId(QUEUE_STATUS_TEST_ID)).toBeNull(); + expect(getByTestId("row-host").childElementCount).toBe(0); + }); +}); + describe("blockedReason", () => { it("names the approval shortfall when required reviews aren't met", () => { expect(blockedReason(makePR({ required_reviews: 2, review_count: 0 }), t)).toContain( diff --git a/apps/web/components/github/pr-mergeability-row.tsx b/apps/web/components/github/pr-mergeability-row.tsx index c623162841..d878d63cf2 100644 --- a/apps/web/components/github/pr-mergeability-row.tsx +++ b/apps/web/components/github/pr-mergeability-row.tsx @@ -12,6 +12,7 @@ import { import type { TaskPR } from "@/lib/types/github"; import { isPRAwaitingReview, isPRWaitingOnBranchProtection } from "./pr-task-icon"; import { PRMergeabilityNotice, buildConflictResolutionMessage } from "./pr-mergeability-notice"; +import { hasActiveMergeQueueEntry, PRMergeQueueStatus } from "./pr-merge-queue-status"; import { useTranslation } from "react-i18next"; /** @@ -78,6 +79,7 @@ function useResolveConflicts(pr: TaskPR): { export function PRMergeabilityRow({ pr }: { pr: TaskPR }) { const { t } = useTranslation(); const { onResolveConflicts, conflictQueued } = useResolveConflicts(pr); + if (hasActiveMergeQueueEntry(pr)) return ; // "blocked" gets a richer note than the bare chip: it explains *why* the // merge is gated. But when the block is only an outstanding requested review, // stay silent — the review row above already conveys that and the diff --git a/apps/web/components/github/pr-status-chip.test.tsx b/apps/web/components/github/pr-status-chip.test.tsx index 53362836cc..e616f85023 100644 --- a/apps/web/components/github/pr-status-chip.test.tsx +++ b/apps/web/components/github/pr-status-chip.test.tsx @@ -582,6 +582,37 @@ describe("aggregateChipStatus", () => { const failing = makePR({ id: "fail", checks_state: "failure" }); expect(aggregateChipStatus([conflict, failing])).toBe("failed"); }); + + it("uses the dedicated queued chip status", () => { + expect(aggregateChipStatus([makePR({ merge_queue_state: "queued" })])).toBe("queued"); + }); + + it("keeps queued status ahead of dirty mergeability", () => { + expect( + aggregateChipStatus([makePR({ merge_queue_state: "queued", mergeable_state: "dirty" })]), + ).toBe("queued"); + }); + + it("keeps queued status ahead of failure on the same PR", () => { + expect( + aggregateChipStatus([ + makePR({ + merge_queue_state: "queued", + checks_state: "failure", + review_state: "changes_requested", + }), + ]), + ).toBe("queued"); + }); + + it("lets a failing sibling retain chip priority over a queued PR", () => { + expect( + aggregateChipStatus([ + makePR({ merge_queue_state: "queued" }), + makePR({ pr_number: 2, checks_state: "failure" }), + ]), + ).toBe("failed"); + }); }); describe("PRStatusChip — multiple PRs", () => { diff --git a/apps/web/components/github/pr-status-chip.tsx b/apps/web/components/github/pr-status-chip.tsx index c0929c0ab7..5ec630e1dc 100644 --- a/apps/web/components/github/pr-status-chip.tsx +++ b/apps/web/components/github/pr-status-chip.tsx @@ -27,6 +27,7 @@ import { isPRDraft, isPRAwaitingReview, isPRReadyToMerge, + isPRQueued, isPRWaitingOnBranchProtection, pickDefaultPR, } from "@/components/github/pr-task-icon"; @@ -60,6 +61,7 @@ type ChipStatus = | "behind" | "draft" | "waiting" + | "queued" | "in_progress" | "neutral"; type TriggerRef = { current: HTMLButtonElement | null }; @@ -83,13 +85,17 @@ function focusAfterCollapse(triggerRef?: TriggerRef) { } function chipStatus(pr: TaskPR): ChipStatus { + // Terminal PRs are filtered before this helper. For active PRs, queue + // membership is the authoritative non-terminal state, so stale failure, + // draft, or mergeability fields cannot override the queue indicator. + if (isPRQueued(pr)) return "queued"; if (pr.review_state === "changes_requested" || pr.checks_state === "failure") return "failed"; + if (isPRDraft(pr)) return "draft"; // Merge conflicts / behind-base block the merge even when CI is green — the // chip must never read as a passed check in that case. Mirrors // getPRStatusColor + PRStatusIcon (dirty = red, behind = amber). if (pr.mergeable_state === "dirty") return "conflict"; if (pr.mergeable_state === "behind") return "behind"; - if (isPRDraft(pr)) return "draft"; // Pending checks / pending review must beat checks_state === "success" so a // PR with all checks green but reviewers still outstanding renders as // in-progress, not passed. Without this order, the chip flips to green the @@ -114,6 +120,7 @@ const CHIP_STATUS_RANK: Record = { conflict: 5, blocked: 4, behind: 3, + queued: 2.5, draft: 0.5, in_progress: 2, waiting: 1.5, @@ -450,6 +457,14 @@ function ChipStatusGlyph({ status }: { status: ChipStatus }) { return