diff --git a/doc/changes/workflow-review-queue-delta.md b/doc/changes/workflow-review-queue-delta.md index f4a218a..3159516 100644 --- a/doc/changes/workflow-review-queue-delta.md +++ b/doc/changes/workflow-review-queue-delta.md @@ -27,12 +27,15 @@ gitlink-cli workflow +review-queue \ ## 等待效率信号 -队列结果现在同时记录 `created_at`、`updated_at`、`age_hours`、`waiting_hours`、`stale`、`review_state`、`reviewers` 和 `waiting_on`。通过 `--as-of` 可固定计算时刻，通过 `--stale-after-hours` 可按仓库 SLA 调整阈值；默认阈值为 72 小时。`waiting_on` 只根据明确的 review 状态推断作者、reviewer 或维护者，不会把未知状态伪装成责任归属。等待达到阈值的条目会增加队列优先级，并在 Markdown 中单独显示 SLA 摘要。 +队列结果现在同时记录 `created_at`、`updated_at`、`activity_source`、`age_hours`、`waiting_hours`、`stale`、`review_state`、`reviewers` 和 `waiting_on`。通过 `--as-of` 可固定计算时刻，通过 `--stale-after-hours` 可按仓库 SLA 调整阈值；默认阈值为 72 小时。真实列表响应没有更新时间时，使用创建时间计算等待时长，并通过 `activity_source=created_at_fallback` 明确证据来源；没有 reviewer 的 open PR 标记为 `unassigned` 并归为等待维护者分配，已请求 Review 的 PR 才归为等待 reviewer。 + +GitLink 的列表接口可能在 `state=open` 响应中混入 closed PR。客户端优先按官方数值字段 `pull_request_status`（0=open、1=merged、2=closed）二次过滤，同时兼容字符串字段 `pull_request_staus`；状态缺失或不匹配的条目不会进入 open 队列。标题、作者、分支和创建时间同时兼容真实响应的 `name`、`issue.author`、`pull_request_base/head`、`pr_full_time/pr_created_unix` 字段。 ## 兼容性与验证 - 不传 `--previous` 时原有输出和优先级排序保持不变。 - `changes` 是可选 JSON 字段，旧消费者可以忽略。 - 非法快照会给出明确错误，不会静默生成不完整差异。 +- 真实 API 回归验证确认 open 队列不再包含 closed PR，标题、作者、时间、SLA 和责任字段均可解释。 - `go test ./shortcuts/workflow -run 'TestAnalyzeReviewQueue|TestCompareReviewQueue|TestReadReviewQueue|TestFetchReviewQueue|TestRenderReviewQueue' -count=1` 通过。 - `go build ./...` 和 `git diff --check` 通过。 diff --git a/shortcuts/workflow/pr_fetch.go b/shortcuts/workflow/pr_fetch.go index d15468c..a8fe351 100644 --- a/shortcuts/workflow/pr_fetch.go +++ b/shortcuts/workflow/pr_fetch.go @@ -157,17 +157,17 @@ func prAPIObject(data interface{}) map[string]interface{} { func normalizePRSummaryItem(item map[string]interface{}) (PRSummaryInput, bool) { number := firstPRInt(item, "number", "index", "iid", "pull_request_number") - title := firstPRString(item, "title", "subject") + title := firstPRString(item, "title", "subject", "name") if number == 0 && strings.TrimSpace(title) == "" { return PRSummaryInput{}, false } body := firstPRString(item, "body", "description", "content") - state := firstPRString(item, "state", "status") + state := firstPRState(item) author := firstPRAuthor(item) issueID := firstPRIssueID(item) - base := firstPRBranch(item, "base_branch", "target_branch", "base") - head := firstPRBranch(item, "head_branch", "source_branch", "head") - createdAt := firstPRTime(item, "created_at", "createdAt") + base := firstPRBranch(item, "base_branch", "target_branch", "pull_request_base", "base") + head := firstPRBranch(item, "head_branch", "source_branch", "pull_request_head", "head") + createdAt := firstPRTime(item, "created_at", "createdAt", "pr_full_time", "pr_created_unix") updatedAt := apiLatestTime( firstPRTime(item, "updated_at", "updatedAt"), firstPRTime(item, "last_updated_at", "lastUpdatedAt"), @@ -338,6 +338,9 @@ func firstPRTime(item map[string]interface{}, keys ...string) time.Time { } func firstPRAuthor(item map[string]interface{}) string { + if author := firstPRString(item, "author_login", "creator_login", "create_user", "author_name"); author != "" { + return author + } for _, key := range []string{"author", "user", "creator"} { if value, ok := item[key]; ok { if s := apiAuthor(value); s != "" { @@ -345,6 +348,20 @@ func firstPRAuthor(item map[string]interface{}) string { } } } + for _, key := range []string{"issue", "issue_info"} { + nested, ok := item[key].(map[string]interface{}) + if !ok { + continue + } + for _, authorKey := range []string{"author", "user", "creator"} { + if author := apiAuthor(nested[authorKey]); author != "" { + return author + } + } + if author := firstPRString(nested, "author_login", "author_name"); author != "" { + return author + } + } return "" } @@ -376,9 +393,41 @@ func firstPRIssueID(item map[string]interface{}) int { } } } + if _, ok := item["pull_request_id"]; ok { + return firstPRInt(item, "id") + } return 0 } +func firstPRState(item map[string]interface{}) string { + if value, ok := item["pull_request_status"]; ok { + switch strings.TrimSpace(apiString(value)) { + case "0": + return "open" + case "1": + return "merged" + case "2": + return "closed" + } + } + for _, key := range []string{"pull_request_staus", "pull_request_state", "state", "status"} { + value, ok := item[key] + if !ok { + continue + } + state := strings.ToLower(strings.TrimSpace(apiString(value))) + switch state { + case "open", "opened": + return "open" + case "closed", "close": + return "closed" + case "merged", "merge": + return "merged" + } + } + return "" +} + func firstPRBranch(item map[string]interface{}, keys ...string) string { for _, key := range keys { if value, ok := item[key]; ok { diff --git a/shortcuts/workflow/review_queue.go b/shortcuts/workflow/review_queue.go index d07c5b0..8ae9d03 100644 --- a/shortcuts/workflow/review_queue.go +++ b/shortcuts/workflow/review_queue.go @@ -78,6 +78,7 @@ type ReviewQueueItem struct { ReviewFocus []string `json:"review_focus,omitempty"` CreatedAt time.Time `json:"created_at,omitempty"` UpdatedAt time.Time `json:"updated_at,omitempty"` + ActivitySource string `json:"activity_source,omitempty"` AgeHours int `json:"age_hours,omitempty"` WaitingHours int `json:"waiting_hours,omitempty"` Stale bool `json:"stale"` @@ -254,6 +255,9 @@ func fetchReviewQueuePullRequests(ctx *common.RuntimeContext, state string, page if !ok { continue } + if !reviewQueueStateMatches(input.State, state) { + continue + } input.Repository = fmt.Sprintf("%s/%s", owner, repo) input.Source = "remote-read-only-fetch:list-metadata" if strings.TrimSpace(input.State) == "" { @@ -424,8 +428,10 @@ func sortReviewQueueDeltaItems(items []ReviewQueueDeltaItem, previousOnly bool) func buildReviewQueueItem(pr PRSummaryInput, summary PRSummaryResult, lang string, asOf time.Time, staleAfterHours int) ReviewQueueItem { updatedAt := pr.UpdatedAt + activitySource := "updated_at" if updatedAt.IsZero() { updatedAt = pr.CreatedAt + activitySource = "created_at_fallback" } ageHours := elapsedHours(asOf, pr.CreatedAt) waitingHours := elapsedHours(asOf, updatedAt) @@ -437,6 +443,14 @@ func buildReviewQueueItem(pr PRSummaryInput, summary PRSummaryResult, lang strin } else if score >= 40 { priority = "medium" } + reviewState := normalizeReviewQueueState(pr.ReviewState) + if reviewState == "" && strings.EqualFold(pr.State, "open") { + if len(pr.Reviewers) == 0 { + reviewState = "unassigned" + } else { + reviewState = "review_requested" + } + } return ReviewQueueItem{ Number: summary.Number, Title: summary.Title, @@ -455,13 +469,14 @@ func buildReviewQueueItem(pr PRSummaryInput, summary PRSummaryResult, lang strin ReviewFocus: summary.ReviewFocus, CreatedAt: pr.CreatedAt, UpdatedAt: updatedAt, + ActivitySource: activitySource, AgeHours: ageHours, WaitingHours: waitingHours, Stale: stale, - ReviewState: normalizeReviewQueueState(pr.ReviewState), + ReviewState: reviewState, Reviewers: append([]string(nil), pr.Reviewers...), ReviewerCount: len(pr.Reviewers), - WaitingOn: reviewQueueWaitingOn(pr.ReviewState), + WaitingOn: reviewQueueWaitingOn(reviewState, pr.Reviewers, pr.State), } } @@ -539,7 +554,7 @@ func normalizeReviewQueueState(value string) string { return strings.ToLower(strings.NewReplacer("-", "_", " ", "_").Replace(strings.TrimSpace(value))) } -func reviewQueueWaitingOn(state string) string { +func reviewQueueWaitingOn(state string, reviewers []string, prState string) string { switch normalizeReviewQueueState(state) { case "changes_requested", "changes_request", "request_changes": return "author" @@ -547,10 +562,33 @@ func reviewQueueWaitingOn(state string) string { return "maintainer" case "requested", "review_requested", "pending", "unreviewed", "reviewing": return "reviewer" + case "unassigned": + return "maintainer" + } + if strings.EqualFold(strings.TrimSpace(prState), "open") { + if len(reviewers) > 0 { + return "reviewer" + } + return "maintainer" } return "" } +func reviewQueueStateMatches(actual, requested string) bool { + requested = strings.ToLower(strings.TrimSpace(requested)) + if requested == "" || requested == "all" { + return true + } + actual = strings.ToLower(strings.TrimSpace(actual)) + if requested == "opened" { + requested = "open" + } + if requested == "close" { + requested = "closed" + } + return actual != "" && actual == requested +} + func hasReviewQueueTestSignal(pr PRSummaryInput) bool { for _, file := range pr.ChangedFiles { if isTestPath(normalizedPath(file.Filename)) { @@ -720,7 +758,7 @@ func writeReviewQueueMarkdown(buf *bytes.Buffer, result ReviewQueueResult, lang return err } if item.WaitingHours > 0 || item.AgeHours > 0 || item.WaitingOn != "" { - if _, err := fmt.Fprintf(buf, " - SLA: age `%dh`, waiting `%dh`, stale `%t`, waiting on `%s`, reviewers `%d`\n", item.AgeHours, item.WaitingHours, item.Stale, item.WaitingOn, item.ReviewerCount); err != nil { + if _, err := fmt.Fprintf(buf, " - SLA: age `%dh`, waiting `%dh`, stale `%t`, waiting on `%s`, reviewers `%d`, activity source `%s`\n", item.AgeHours, item.WaitingHours, item.Stale, item.WaitingOn, item.ReviewerCount, item.ActivitySource); err != nil { return err } } diff --git a/shortcuts/workflow/review_queue_test.go b/shortcuts/workflow/review_queue_test.go index e366a05..d177955 100644 --- a/shortcuts/workflow/review_queue_test.go +++ b/shortcuts/workflow/review_queue_test.go @@ -199,20 +199,30 @@ func TestFetchReviewQueuePullRequestsUsesReadOnlyQuery(t *testing.T) { t.Fatalf("limit query = %q, want 5", got) } writeWorkflowJSON(t, w, map[string]interface{}{ - "pulls": []map[string]interface{}{ + "issues": []map[string]interface{}{ { + "id": 101, + "pull_request_id": 1001, "pull_request_number": 11, - "title": "feat: remote review queue", - "description": "Adds queue analysis.", - "status": "open", - "creator": map[string]interface{}{"login": "alice"}, - "created_at": "2026-07-18T12:00:00Z", - "updated_at": "2026-07-19T12:00:00Z", + "name": "feat: remote review queue", + "pull_request_staus": "open", + "pull_request_status": 0, + "issue": map[string]interface{}{"author": map[string]interface{}{"login": "alice"}}, + "pr_full_time": "2026-07-18T12:00:00.000Z", + "pull_request_base": "master", + "pull_request_head": "feature/review-queue", "review_state": "requested", "reviewers": []interface{}{map[string]interface{}{"login": "reviewer"}}, "additions": 130, "deletions": 5, }, + { + "id": 102, + "pull_request_id": 1002, + "pull_request_number": 10, + "name": "closed PR returned by open endpoint", + "pull_request_status": 2, + }, }, }) })) @@ -237,11 +247,40 @@ func TestFetchReviewQueuePullRequestsUsesReadOnlyQuery(t *testing.T) { if prs[0].Number != 11 || prs[0].Repository != "owner/repo" || prs[0].State != "open" { t.Fatalf("normalized PR = %+v, want owner/repo #11 open", prs[0]) } + if prs[0].IssueID != 101 || prs[0].Title != "feat: remote review queue" || prs[0].Author != "alice" || prs[0].BaseBranch != "master" || prs[0].HeadBranch != "feature/review-queue" || prs[0].CreatedAt.IsZero() { + t.Fatalf("real GitLink fields not normalized: %+v", prs[0]) + } if prs[0].ReviewState != "requested" || len(prs[0].Reviewers) != 1 || prs[0].Reviewers[0] != "reviewer" { t.Fatalf("review metadata = state:%q reviewers:%v", prs[0].ReviewState, prs[0].Reviewers) } } +func TestAnalyzeReviewQueueInfersUnassignedOwnershipAndCreationFallback(t *testing.T) { + asOf := time.Date(2026, 7, 22, 12, 0, 0, 0, time.UTC) + createdAt := asOf.Add(-96 * time.Hour) + result := AnalyzeReviewQueue(ReviewQueueInput{ + Repository: "owner/repo", + AsOf: asOf, + StaleAfterHours: 72, + PullRequests: []PRSummaryInput{{ + Number: 42, + Title: "unassigned pull request", + State: "open", + CreatedAt: createdAt, + }}, + }, "en") + if len(result.Items) != 1 { + t.Fatalf("items = %d, want 1", len(result.Items)) + } + item := result.Items[0] + if item.ReviewState != "unassigned" || item.WaitingOn != "maintainer" { + t.Fatalf("ownership = state:%q waiting_on:%q, want unassigned/maintainer", item.ReviewState, item.WaitingOn) + } + if item.WaitingHours != 96 || !item.Stale || item.ActivitySource != "created_at_fallback" || !item.UpdatedAt.Equal(createdAt) { + t.Fatalf("fallback freshness = waiting:%d stale:%t source:%q updated:%s", item.WaitingHours, item.Stale, item.ActivitySource, item.UpdatedAt) + } +} + func TestRenderReviewQueueMarkdownAndTable(t *testing.T) { result := AnalyzeReviewQueue(ReviewQueueInput{ Repository: "owner/repo",