fix(workflow): 严格过滤审查队列并补齐责任信号
This commit is contained in:
parent
914df4b06a
commit
21603f519c
|
|
@ -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` 通过。
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
Loading…
Reference in New Issue