diff --git a/doc/changes/workflow-health-list-filter.md b/doc/changes/workflow-health-list-filter.md new file mode 100644 index 0000000..ce8d2ae --- /dev/null +++ b/doc/changes/workflow-health-list-filter.md @@ -0,0 +1,9 @@ +# workflow 健康评分列表筛选修正 + +`workflow +health`、`+repo-report`、`+triage` 的远程抓取此前用 `state=open` 过滤 Issue/PR 列表，但 GitLink v1 列表接口对 PR 用 `status`（0 开启 / 1 合并 / 2 关闭），对 Issue 用 `category`（opened / closed / all），`state` 会被服务端静默忽略并返回全部状态。结果 open+merged+closed 全被计入，`OpenPRs`/`OpenIssues` 与由此推导的健康分被污染。 + +现在 PR 抓取发送 `status=0`，Issue 抓取发送 `category=opened`，与 `shortcuts/health`、`shortcuts/pr`、`shortcuts/issue` 已有的映射保持一致；`+triage` 的本地态客户端二次过滤保持不变。 + +同时修正 `updateRecentActivity`：`RecentActivityDays==0` 表示"今天有活动"（`apiAgeInDays` 对 24 小时内返回 0），旧逻辑里 `|| input.RecentActivityDays == 0` 会让更早的信号覆盖"今天"，使仓库显得更陈旧。移除该项后，首次赋值仍由 `!RecentActivityKnown` 分支处理，之后仅当发现更近的活动才更新。 + +测试断言列表请求发出的键为 `status`/`category`（不再是 `state`），并验证"今天"的活动信号不会被更早的信号覆盖。 diff --git a/shortcuts/workflow/api_types_test.go b/shortcuts/workflow/api_types_test.go index ec549f3..880a17e 100644 --- a/shortcuts/workflow/api_types_test.go +++ b/shortcuts/workflow/api_types_test.go @@ -495,8 +495,36 @@ func TestQueryWithPageLimit(t *testing.T) { func TestIssueListQuery(t *testing.T) { q := issueListQuery("open") - if q.Get("state") != "open" { - t.Fatalf("issueListQuery state = %q", q.Get("state")) + if q.Get("state") != "" { + t.Fatalf("issueListQuery must not send state, got %q", q.Get("state")) + } + if q.Get("category") != "opened" { + t.Fatalf("issueListQuery category = %q, want opened", q.Get("category")) + } + if got := issueListQuery("closed").Get("category"); got != "closed" { + t.Fatalf("issueListQuery(closed) category = %q, want closed", got) + } + if got := issueListQuery("all").Get("category"); got != "all" { + t.Fatalf("issueListQuery(all) category = %q, want all", got) + } +} + +func TestPullListQuery(t *testing.T) { + q := pullListQuery("open") + if q.Get("state") != "" { + t.Fatalf("pullListQuery must not send state, got %q", q.Get("state")) + } + if q.Get("status") != "0" { + t.Fatalf("pullListQuery status = %q, want 0", q.Get("status")) + } + if got := pullListQuery("merged").Get("status"); got != "1" { + t.Fatalf("pullListQuery(merged) status = %q, want 1", got) + } + if got := pullListQuery("closed").Get("status"); got != "2" { + t.Fatalf("pullListQuery(closed) status = %q, want 2", got) + } + if _, ok := pullListQuery("all")["status"]; ok { + t.Fatal("pullListQuery(all) should omit status so the API returns every state") } } @@ -582,6 +610,14 @@ func TestUpdateRecentActivity(t *testing.T) { if !known2 || days2 != days { t.Fatalf("zero time update should not change: known=%v days=%d", known2, days2) } + + // A signal from today (days==0) must not be overwritten by an older one. + today := HealthInput{RecentActivityKnown: true, RecentActivityDays: 0} + old := time.Now().Add(-45 * 24 * time.Hour) + _, keptDays, _ := updateRecentActivity(today, old) + if keptDays != 0 { + t.Fatalf("today signal overwritten by older one: days=%d, want 0", keptDays) + } } func TestAPIIntStringFallback(t *testing.T) { diff --git a/shortcuts/workflow/health_fetch.go b/shortcuts/workflow/health_fetch.go index 62066c5..f9ae1ea 100644 --- a/shortcuts/workflow/health_fetch.go +++ b/shortcuts/workflow/health_fetch.go @@ -39,7 +39,7 @@ func FetchHealthInput(ctx *common.RuntimeContext, opts HealthFetchOptions) (Heal input.RecentActivityKnown, input.RecentActivityDays, input = updateRecentActivity(input, latestTimeFromItems(issues)) } - if prs, err := fetchAllListItems(ctx, workflowRepoPath(owner, repo)+"/pulls", issueListQuery("open"), 100); err != nil { + if prs, err := fetchAllListItems(ctx, workflowRepoPath(owner, repo)+"/pulls", pullListQuery("open"), 100); err != nil { notes = append(notes, ScoringNote{Metric: "open_prs", Note: fmt.Sprintf("pull request probe failed: %v", err)}) } else { input.OpenPRs = len(prs) @@ -200,7 +200,7 @@ func updateRecentActivity(input HealthInput, latest time.Time) (bool, int, Healt return input.RecentActivityKnown, input.RecentActivityDays, input } days := apiAgeInDays(latest) - if !input.RecentActivityKnown || days < input.RecentActivityDays || input.RecentActivityDays == 0 { + if !input.RecentActivityKnown || days < input.RecentActivityDays { input.RecentActivityKnown = true input.RecentActivityDays = days } @@ -220,12 +220,46 @@ func queryWithPageLimit(base url.Values, page, limit int) url.Values { return base } +// The GitLink v1 list API filters issues by category and pulls by status; a +// stray "state" param is silently ignored and every state is returned. func issueListQuery(state string) url.Values { q := url.Values{} - q.Set("state", state) + q.Set("category", normalizeIssueListCategory(state)) return q } +func pullListQuery(state string) url.Values { + q := url.Values{} + if status := normalizePullListStatus(state); status != "" { + q.Set("status", status) + } + return q +} + +func normalizeIssueListCategory(state string) string { + switch strings.ToLower(strings.TrimSpace(state)) { + case "open", "opened": + return "opened" + case "closed": + return "closed" + default: + return "all" + } +} + +func normalizePullListStatus(state string) string { + switch strings.ToLower(strings.TrimSpace(state)) { + case "open", "opened": + return "0" + case "merged": + return "1" + case "closed": + return "2" + default: + return "" + } +} + func fetchAllListItems(ctx *common.RuntimeContext, path string, baseQuery url.Values, pageSize int) ([]map[string]interface{}, error) { if pageSize <= 0 { pageSize = 100 diff --git a/shortcuts/workflow/health_fetch_test.go b/shortcuts/workflow/health_fetch_test.go index c9accd3..36f41ec 100644 --- a/shortcuts/workflow/health_fetch_test.go +++ b/shortcuts/workflow/health_fetch_test.go @@ -22,11 +22,23 @@ func TestFetchHealthInputCollectsSignals(t *testing.T) { "has_contributing": true, }) case r.Method == "GET" && r.URL.Path == "/v1/owner/repo/issues.json": + if got := r.URL.Query().Get("category"); got != "opened" { + t.Fatalf("issue category query = %q, want opened", got) + } + if got := r.URL.Query().Get("state"); got != "" { + t.Fatalf("issue list must not send state, got %q", got) + } writeWorkflowJSON(t, w, map[string]interface{}{"issues": []map[string]interface{}{ {"id": 1, "subject": "fresh issue", "updated_at": now.AddDate(0, 0, -1).Format(time.RFC3339)}, {"id": 2, "subject": "stale issue", "updated_at": old.Format(time.RFC3339)}, }}) case r.Method == "GET" && r.URL.Path == "/v1/owner/repo/pulls.json": + if got := r.URL.Query().Get("status"); got != "0" { + t.Fatalf("pull status query = %q, want 0", got) + } + if got := r.URL.Query().Get("state"); got != "" { + t.Fatalf("pull list must not send state, got %q", got) + } writeWorkflowJSON(t, w, map[string]interface{}{"pulls": []map[string]interface{}{ {"id": 3, "title": "stale pr", "updated_at": old.Format(time.RFC3339)}, }}) diff --git a/shortcuts/workflow/repo_report_fetch.go b/shortcuts/workflow/repo_report_fetch.go index 9610a71..30c47de 100644 --- a/shortcuts/workflow/repo_report_fetch.go +++ b/shortcuts/workflow/repo_report_fetch.go @@ -2,7 +2,6 @@ package workflow import ( "fmt" - "net/url" "strings" "github.com/gitlink-org/gitlink-cli/shortcuts/common" @@ -104,8 +103,7 @@ func fetchPRListForReport(ctx *common.RuntimeContext, owner, repo string, limit if limit <= 0 { limit = 10 } - query := url.Values{} - query.Set("state", "open") + query := pullListQuery("open") query.Set("page", "1") query.Set("limit", fmt.Sprintf("%d", limit)) diff --git a/shortcuts/workflow/repo_report_fetch_test.go b/shortcuts/workflow/repo_report_fetch_test.go index 37e9838..592a19c 100644 --- a/shortcuts/workflow/repo_report_fetch_test.go +++ b/shortcuts/workflow/repo_report_fetch_test.go @@ -146,6 +146,12 @@ func TestFetchRepoReportInputPRListMetadata(t *testing.T) { if r.Method != "GET" || r.URL.Path != "/v1/owner/repo/pulls.json" { t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) } + if got := r.URL.Query().Get("status"); got != "0" { + t.Fatalf("PR status query = %q, want 0", got) + } + if got := r.URL.Query().Get("state"); got != "" { + t.Fatalf("PR report list must not send state, got %q", got) + } if got := r.URL.Query().Get("limit"); got != "1" { t.Fatalf("PR limit = %q, want 1", got) } diff --git a/shortcuts/workflow/triage_fetch.go b/shortcuts/workflow/triage_fetch.go index a509cf8..b801307 100644 --- a/shortcuts/workflow/triage_fetch.go +++ b/shortcuts/workflow/triage_fetch.go @@ -29,7 +29,7 @@ func FetchIssuesForTriage(ctx *common.RuntimeContext, opts TriageFetchOptions) ( } query := url.Values{} - query.Set("state", state) + query.Set("category", normalizeIssueListCategory(state)) query.Set("limit", fmt.Sprintf("%d", limit)) query.Set("page", fmt.Sprintf("%d", page)) if len(opts.Labels) > 0 { diff --git a/shortcuts/workflow/triage_fetch_test.go b/shortcuts/workflow/triage_fetch_test.go index f7af88a..b805025 100644 --- a/shortcuts/workflow/triage_fetch_test.go +++ b/shortcuts/workflow/triage_fetch_test.go @@ -16,8 +16,11 @@ func TestFetchIssuesForTriageNormalizesAPIResponse(t *testing.T) { if r.Method != "GET" || r.URL.Path != "/v1/owner/repo/issues.json" { t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) } - if got := r.URL.Query().Get("state"); got != "open" { - t.Fatalf("state query = %q, want open", got) + if got := r.URL.Query().Get("category"); got != "opened" { + t.Fatalf("category query = %q, want opened", got) + } + if got := r.URL.Query().Get("state"); got != "" { + t.Fatalf("issue triage must not send state, got %q", got) } if got := r.URL.Query().Get("limit"); got != "30" { t.Fatalf("limit query = %q, want 30", got)