From 3026c9d71eb302e4f788a7f113a45d180b9f27b9 Mon Sep 17 00:00:00 2001 From: Mengz <2567587994@qq.com> Date: Mon, 20 Jul 2026 16:07:15 +0800 Subject: [PATCH 1/3] =?UTF-8?q?feat(workflow):=20=E6=89=A9=E5=B1=95=20PR?= =?UTF-8?q?=20=E5=AE=A1=E6=9F=A5=E8=AF=81=E6=8D=AE=E4=B8=8A=E4=B8=8B?= =?UTF-8?q?=E6=96=87?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- README.md | 7 ++ .../workflow-review-context-evidence.md | 38 +++++++++ shortcuts/workflow/review_context.go | 78 ++++++++++++++++++- shortcuts/workflow/review_context_test.go | 78 +++++++++++++++++++ 4 files changed, 197 insertions(+), 4 deletions(-) create mode 100644 doc/changes/workflow-review-context-evidence.md diff --git a/README.md b/README.md index d48d41a..42a6806 100644 --- a/README.md +++ b/README.md @@ -507,6 +507,7 @@ gitlink-cli search +users -k "zhangsan" - `workflow +health` - `workflow +pr-summary` - `workflow +repo-report` +- `workflow +review-context` `workflow +pr-summary` defaults to `table` when `--format` is omitted. `workflow +repo-report` defaults to `markdown` when `--format` is omitted. @@ -582,6 +583,11 @@ gitlink-cli workflow +repo-report --owner Gitlink --repo gitlink-cli --format ma # Repository workflow report from a local JSON file gitlink-cli workflow +repo-report --from shortcuts/workflow/testdata/repo_report.json --format json + +# Build a bounded PR evidence bundle for review and integration Skills +gitlink-cli workflow +review-context \ + --owner Gitlink --repo gitlink-cli --number 1 \ + --include-commits=true --include-ci=true --format json ``` Output formats: @@ -597,6 +603,7 @@ Safety: - They do not depend on LLM APIs. - `workflow +pr-summary` does not comment, approve, reject, or merge pull requests. - `workflow +repo-report` aggregates health, issue triage, and PR review summary signals without remote writes. +- `workflow +review-context` is a read-only, bounded evidence bundle. Commits and CI builds are opt-in so existing scripts keep their previous request and output behavior. ### Raw API diff --git a/doc/changes/workflow-review-context-evidence.md b/doc/changes/workflow-review-context-evidence.md new file mode 100644 index 0000000..50e8e97 --- /dev/null +++ b/doc/changes/workflow-review-context-evidence.md @@ -0,0 +1,38 @@ +# 工作流 PR 证据包 + +## 变更说明 + +在现有 `workflow +review-context` 的基础上增加可选的提交记录和 CI 构建结果，并为文件、Review、提交和构建结果统一提供数量上限。默认行为保持不变；只有显式启用 `--include-commits=true` 或 `--include-ci=true` 时才会请求新增接口。 + +这项基础能力用于支撑已经合并的 PR 维护 Skills：代码审查可以把提交记录、变更文件和 Review 放在同一份证据中，集成检查可以使用 CI 结果作为合并门禁，契约守卫可以验证新增输出仍然是有界且可机器读取的，维护者雷达和 PR 拓扑分析也可以复用同一份上下文而不重复请求 API。该 PR 只提供只读证据，不替代五个 Skill 各自的判断职责。 + +## 使用示例 + +```bash +gitlink-cli workflow +review-context \ + --owner Gitlink --repo gitlink-cli --number 1 \ + --include-commits=true --commit-limit 30 \ + --include-ci=true --ci-limit 20 \ + --format json +``` + +新增结果字段： + +- `commits`：PR 提交记录，只有启用 `include-commits` 时出现。 +- `ci_builds`：仓库 CI 构建记录，只有启用 `include-ci` 时出现。 +- `sections`：成功获取的证据分区，便于 Agent 判断证据是否完整。 +- `notes`：单个证据探针失败时的可解释记录，不会伪造成功数据。 + +## 兼容性与安全 + +- 旧参数、旧默认请求和已有 JSON 字段保持兼容。 +- 所有新增请求均为 `GET`，不会评论、审批、合并、关闭或修改远程资源。 +- 每个列表都带有上限，避免大仓库响应无限膨胀。 +- CI 获取失败时保留其他证据，并在 `notes` 中标记为不可用；只有所有启用分区都失败时才返回错误。 + +## 验证 + +```bash +go test ./shortcuts/workflow -run 'TestFetchReviewContextEvidence|TestRenderReviewContextFormats' +go build ./... +``` diff --git a/shortcuts/workflow/review_context.go b/shortcuts/workflow/review_context.go index 6522c2c..3d70fcf 100644 --- a/shortcuts/workflow/review_context.go +++ b/shortcuts/workflow/review_context.go @@ -14,14 +14,20 @@ type ReviewContextOptions struct { Owner string Repo string Number int + FileLimit int + ReviewLimit int IssueLimit int LabelLimit int + CommitLimit int + BuildLimit int IncludeRepo bool IncludePR bool IncludeFiles bool IncludeReviews bool IncludeIssues bool IncludeLabels bool + IncludeCommits bool + IncludeCI bool } type ReviewContext struct { @@ -35,6 +41,8 @@ type ReviewContext struct { Reviews []map[string]interface{} `json:"reviews,omitempty"` OpenIssues []map[string]interface{} `json:"open_issues,omitempty"` Labels []map[string]interface{} `json:"labels,omitempty"` + Commits []map[string]interface{} `json:"commits,omitempty"` + Builds []map[string]interface{} `json:"ci_builds,omitempty"` Notes []ScoringNote `json:"notes,omitempty"` } @@ -44,14 +52,20 @@ func newReviewContextShortcut() *common.Shortcut { Description: "Fetch read-only PR review context from shortcut-backed endpoints", Flags: []common.Flag{ {Name: "number", Short: "n", Usage: "Pull request number", Required: true}, + {Name: "file-limit", Usage: "Maximum changed files to include", Default: "100"}, + {Name: "review-limit", Usage: "Maximum reviews to include", Default: "100"}, {Name: "issue-limit", Usage: "Maximum open issues to include", Default: "20"}, {Name: "label-limit", Usage: "Maximum labels to include", Default: "50"}, + {Name: "commit-limit", Usage: "Maximum commits to include", Default: "100"}, + {Name: "ci-limit", Usage: "Maximum CI builds to include", Default: "20"}, {Name: "include-repo", Usage: "Include repository info", Bool: true, Default: "true"}, {Name: "include-pr", Usage: "Include pull request details", Bool: true, Default: "true"}, {Name: "include-files", Usage: "Include pull request changed files", Bool: true, Default: "true"}, {Name: "include-reviews", Usage: "Include pull request reviews", Bool: true, Default: "true"}, {Name: "include-issues", Usage: "Include open issue context", Bool: true, Default: "true"}, {Name: "include-labels", Usage: "Include issue labels", Bool: true, Default: "true"}, + {Name: "include-commits", Usage: "Include pull request commits", Bool: true, Default: "false"}, + {Name: "include-ci", Usage: "Include repository CI builds for evidence checks", Bool: true, Default: "false"}, }, Run: runReviewContext, } @@ -65,10 +79,26 @@ func runReviewContext(ctx *common.RuntimeContext) error { if number <= 0 { return fmt.Errorf("workflow +review-context requires --number with --owner and --repo for read-only fetch") } + fileLimit, err := parseIntArg(ctx.Arg("file-limit"), 100, "file-limit") + if err != nil { + return err + } + reviewLimit, err := parseIntArg(ctx.Arg("review-limit"), 100, "review-limit") + if err != nil { + return err + } issueLimit, err := parseIntArg(ctx.Arg("issue-limit"), 20, "issue-limit") if err != nil { return err } + commitLimit, err := parseIntArg(ctx.Arg("commit-limit"), 100, "commit-limit") + if err != nil { + return err + } + ciLimit, err := parseIntArg(ctx.Arg("ci-limit"), 20, "ci-limit") + if err != nil { + return err + } labelLimit, err := parseIntArg(ctx.Arg("label-limit"), 50, "label-limit") if err != nil { return err @@ -76,14 +106,20 @@ func runReviewContext(ctx *common.RuntimeContext) error { context, err := FetchReviewContext(ctx, ReviewContextOptions{ Number: number, + FileLimit: fileLimit, + ReviewLimit: reviewLimit, IssueLimit: issueLimit, LabelLimit: labelLimit, + CommitLimit: commitLimit, + BuildLimit: ciLimit, IncludeRepo: parseBoolDefault(ctx.Arg("include-repo"), true), IncludePR: parseBoolDefault(ctx.Arg("include-pr"), true), IncludeFiles: parseBoolDefault(ctx.Arg("include-files"), true), IncludeReviews: parseBoolDefault(ctx.Arg("include-reviews"), true), IncludeIssues: parseBoolDefault(ctx.Arg("include-issues"), true), IncludeLabels: parseBoolDefault(ctx.Arg("include-labels"), true), + IncludeCommits: parseBoolDefault(ctx.Arg("include-commits"), false), + IncludeCI: parseBoolDefault(ctx.Arg("include-ci"), false), }) if err != nil { return err @@ -115,6 +151,18 @@ func FetchReviewContext(ctx *common.RuntimeContext, opts ReviewContextOptions) ( if opts.LabelLimit <= 0 { opts.LabelLimit = 50 } + if opts.FileLimit <= 0 { + opts.FileLimit = 100 + } + if opts.ReviewLimit <= 0 { + opts.ReviewLimit = 100 + } + if opts.CommitLimit <= 0 { + opts.CommitLimit = 100 + } + if opts.BuildLimit <= 0 { + opts.BuildLimit = 20 + } result := ReviewContext{ Repository: fmt.Sprintf("%s/%s", owner, repo), @@ -144,7 +192,7 @@ func FetchReviewContext(ctx *common.RuntimeContext, opts ReviewContextOptions) ( } } if opts.IncludeFiles { - if files, err := fetchReviewContextList(ctx, fmt.Sprintf("%s/pulls/%d/files", plainRepoPath(owner, repo), opts.Number), nil, 100); err != nil { + if files, err := fetchReviewContextList(ctx, fmt.Sprintf("%s/pulls/%d/files", plainRepoPath(owner, repo), opts.Number), nil, opts.FileLimit); err != nil { result.Notes = append(result.Notes, ScoringNote{Metric: "pr_files", Note: fmt.Sprintf("pr +files equivalent failed: %v", err)}) } else { result.Files = files @@ -153,7 +201,7 @@ func FetchReviewContext(ctx *common.RuntimeContext, opts ReviewContextOptions) ( } } if opts.IncludeReviews { - if reviews, err := fetchReviewContextList(ctx, fmt.Sprintf("%s/pulls/%d/reviews", workflowRepoPath(owner, repo), opts.Number), nil, 100); err != nil { + if reviews, err := fetchReviewContextList(ctx, fmt.Sprintf("%s/pulls/%d/reviews", workflowRepoPath(owner, repo), opts.Number), nil, opts.ReviewLimit); err != nil { result.Notes = append(result.Notes, ScoringNote{Metric: "pr_reviews", Note: fmt.Sprintf("pr +reviews equivalent failed: %v", err)}) } else { result.Reviews = reviews @@ -161,6 +209,24 @@ func FetchReviewContext(ctx *common.RuntimeContext, opts ReviewContextOptions) ( successes++ } } + if opts.IncludeCommits { + if commits, err := fetchReviewContextList(ctx, fmt.Sprintf("%s/pulls/%d/commits", workflowRepoPath(owner, repo), opts.Number), nil, opts.CommitLimit); err != nil { + result.Notes = append(result.Notes, ScoringNote{Metric: "pr_commits", Note: fmt.Sprintf("pr +commits equivalent failed: %v", err)}) + } else { + result.Commits = commits + result.Sections = append(result.Sections, "commits") + successes++ + } + } + if opts.IncludeCI { + if builds, err := fetchReviewContextList(ctx, plainRepoPath(owner, repo)+"/builds", nil, opts.BuildLimit); err != nil { + result.Notes = append(result.Notes, ScoringNote{Metric: "ci_builds", Note: fmt.Sprintf("ci +builds equivalent failed: %v", err)}) + } else { + result.Builds = builds + result.Sections = append(result.Sections, "ci_builds") + successes++ + } + } if opts.IncludeIssues { query := url.Values{} query.Set("category", "opened") @@ -258,6 +324,8 @@ func writeReviewContextMarkdown(w *strings.Builder, context ReviewContext) error _, _ = fmt.Fprintf(w, "\n## Summary\n\n") _, _ = fmt.Fprintf(w, "- Changed files: `%d`\n", len(context.Files)) _, _ = fmt.Fprintf(w, "- Reviews: `%d`\n", len(context.Reviews)) + _, _ = fmt.Fprintf(w, "- Commits: `%d`\n", len(context.Commits)) + _, _ = fmt.Fprintf(w, "- CI builds: `%d`\n", len(context.Builds)) _, _ = fmt.Fprintf(w, "- Open issues included: `%d`\n", len(context.OpenIssues)) _, _ = fmt.Fprintf(w, "- Labels included: `%d`\n", len(context.Labels)) if len(context.Notes) > 0 { @@ -270,13 +338,15 @@ func writeReviewContextMarkdown(w *strings.Builder, context ReviewContext) error } func writeReviewContextTable(w *strings.Builder, context ReviewContext) error { - _, _ = fmt.Fprintf(w, "REPOSITORY\tPR\tSECTIONS\tFILES\tREVIEWS\tISSUES\tLABELS\tNOTES\n") - _, _ = fmt.Fprintf(w, "%s\t#%d\t%d\t%d\t%d\t%d\t%d\t%d\n", + _, _ = fmt.Fprintf(w, "REPOSITORY\tPR\tSECTIONS\tFILES\tREVIEWS\tCOMMITS\tCI\tISSUES\tLABELS\tNOTES\n") + _, _ = fmt.Fprintf(w, "%s\t#%d\t%d\t%d\t%d\t%d\t%d\t%d\t%d\t%d\n", context.Repository, context.PullRequest, len(context.Sections), len(context.Files), len(context.Reviews), + len(context.Commits), + len(context.Builds), len(context.OpenIssues), len(context.Labels), len(context.Notes), diff --git a/shortcuts/workflow/review_context_test.go b/shortcuts/workflow/review_context_test.go index 87c458e..10fabac 100644 --- a/shortcuts/workflow/review_context_test.go +++ b/shortcuts/workflow/review_context_test.go @@ -122,6 +122,79 @@ func TestFetchReviewContextPartialFailureKeepsNotes(t *testing.T) { } } +func TestFetchReviewContextEvidenceSectionsAreBounded(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/v1/owner/repo/pulls/12/commits.json": + if got := r.URL.Query().Get("limit"); got != "2" { + t.Fatalf("commit limit = %q, want 2", got) + } + writeWorkflowJSON(t, w, map[string]interface{}{ + "commits": []map[string]interface{}{ + {"sha": "abc123", "message": "feat: add evidence"}, + {"sha": "def456", "message": "test: cover evidence"}, + {"sha": "ignored", "message": "should be bounded"}, + }, + }) + case "/owner/repo/builds.json": + if got := r.URL.Query().Get("limit"); got != "1" { + t.Fatalf("CI limit = %q, want 1", got) + } + writeWorkflowJSON(t, w, map[string]interface{}{ + "builds": []map[string]interface{}{{"id": 9, "status": "success"}, {"id": 10, "status": "failed"}}, + }) + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.String()) + } + })) + defer server.Close() + + got, err := FetchReviewContext(workflowTestContext(server), ReviewContextOptions{ + Number: 12, + CommitLimit: 2, + BuildLimit: 1, + IncludeCommits: true, + IncludeCI: true, + }) + if err != nil { + t.Fatalf("FetchReviewContext returned error: %v", err) + } + if len(got.Commits) != 2 || len(got.Builds) != 1 { + t.Fatalf("evidence sizes = commits:%d builds:%d, want 2/1", len(got.Commits), len(got.Builds)) + } + if !strings.Contains(strings.Join(got.Sections, ","), "commits") || !strings.Contains(strings.Join(got.Sections, ","), "ci_builds") { + t.Fatalf("sections = %v, want commits and ci_builds", got.Sections) + } +} + +func TestFetchReviewContextEvidenceFailureIsNonFatal(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/v1/owner/repo.json" { + writeWorkflowJSON(t, w, map[string]interface{}{"name": "repo"}) + return + } + if r.URL.Path == "/owner/repo/builds.json" { + w.WriteHeader(http.StatusBadGateway) + _, _ = w.Write([]byte("CI unavailable")) + return + } + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.String()) + })) + defer server.Close() + + got, err := FetchReviewContext(workflowTestContext(server), ReviewContextOptions{ + Number: 13, + IncludeRepo: true, + IncludeCI: true, + }) + if err != nil { + t.Fatalf("FetchReviewContext returned error: %v", err) + } + if len(got.Notes) != 1 || got.Notes[0].Metric != "ci_builds" { + t.Fatalf("notes = %+v, want one ci_builds note", got.Notes) + } +} + func TestFetchReviewContextAllSectionsFail(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusInternalServerError) @@ -208,6 +281,8 @@ func TestRenderReviewContextFormats(t *testing.T) { Source: "shortcut-backed-read-only-fetch", Sections: []string{"repo_info", "pr"}, Files: []map[string]interface{}{{"filename": "README.md"}}, + Commits: []map[string]interface{}{{"sha": "abc123"}}, + Builds: []map[string]interface{}{{"id": 9, "status": "success"}}, Notes: []ScoringNote{{Metric: "labels", Note: "label +list equivalent failed"}}, } @@ -226,6 +301,9 @@ func TestRenderReviewContextFormats(t *testing.T) { if !strings.Contains(markdown, "# PR Review Context") || !strings.Contains(markdown, "label +list") { t.Fatalf("markdown output = %q", markdown) } + if !strings.Contains(markdown, "Commits: `1`") || !strings.Contains(markdown, "CI builds: `1`") { + t.Fatalf("markdown output missing evidence counts = %q", markdown) + } } func TestParseBoolDefault(t *testing.T) { -- 2.34.1 From 64da98ec54ef57573267eb7c8da4e4b47c6c07ce Mon Sep 17 00:00:00 2001 From: Mengz <2567587994@qq.com> Date: Mon, 20 Jul 2026 17:33:53 +0800 Subject: [PATCH 2/3] =?UTF-8?q?feat(workflow):=20=E7=B2=BE=E7=A1=AE?= =?UTF-8?q?=E5=85=B3=E8=81=94=20PR=20=E7=9A=84=20CI=20=E8=AF=81=E6=8D=AE?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../workflow-review-context-evidence.md | 4 +- shortcuts/workflow/api_types.go | 2 +- shortcuts/workflow/review_context.go | 100 ++++++++++++++++++ shortcuts/workflow/review_context_test.go | 41 ++++++- 4 files changed, 144 insertions(+), 3 deletions(-) diff --git a/doc/changes/workflow-review-context-evidence.md b/doc/changes/workflow-review-context-evidence.md index 50e8e97..dd076d9 100644 --- a/doc/changes/workflow-review-context-evidence.md +++ b/doc/changes/workflow-review-context-evidence.md @@ -20,6 +20,7 @@ gitlink-cli workflow +review-context \ - `commits`：PR 提交记录，只有启用 `include-commits` 时出现。 - `ci_builds`：仓库 CI 构建记录，只有启用 `include-ci` 时出现。 +- `ci_summary`：优先按 PR head SHA 关联构建，找不到 SHA 匹配时回退到 head branch；状态统计只包含匹配构建，并显式给出 `unmatched`，避免把其他分支的失败构建误判为当前 PR 失败。 - `sections`：成功获取的证据分区，便于 Agent 判断证据是否完整。 - `notes`：单个证据探针失败时的可解释记录，不会伪造成功数据。 @@ -29,10 +30,11 @@ gitlink-cli workflow +review-context \ - 所有新增请求均为 `GET`，不会评论、审批、合并、关闭或修改远程资源。 - 每个列表都带有上限，避免大仓库响应无限膨胀。 - CI 获取失败时保留其他证据，并在 `notes` 中标记为不可用；只有所有启用分区都失败时才返回错误。 +- 标签列表兼容 `issue_tags` 和 `tags` 包装，避免接口返回标签数据时被错误解析为空。 ## 验证 ```bash -go test ./shortcuts/workflow -run 'TestFetchReviewContextEvidence|TestRenderReviewContextFormats' +go test ./shortcuts/workflow -run 'TestFetchReviewContext|TestSummarizeReviewCI|TestRenderReviewContextFormats' -count=1 go build ./... ``` diff --git a/shortcuts/workflow/api_types.go b/shortcuts/workflow/api_types.go index 8eb51fa..c31e058 100644 --- a/shortcuts/workflow/api_types.go +++ b/shortcuts/workflow/api_types.go @@ -104,7 +104,7 @@ func apiList(data interface{}) []interface{} { case []interface{}: return v case map[string]interface{}: - for _, key := range []string{"issues", "pulls", "pull_requests", "reviews", "journals", "comments", "notes", "files", "commits", "releases", "builds", "items", "records", "data"} { + for _, key := range []string{"issues", "pulls", "pull_requests", "reviews", "journals", "comments", "notes", "files", "commits", "releases", "builds", "issue_tags", "tags", "items", "records", "data"} { if raw, ok := v[key]; ok { if items := apiList(raw); len(items) > 0 { return items diff --git a/shortcuts/workflow/review_context.go b/shortcuts/workflow/review_context.go index 3d70fcf..7712acc 100644 --- a/shortcuts/workflow/review_context.go +++ b/shortcuts/workflow/review_context.go @@ -43,9 +43,23 @@ type ReviewContext struct { Labels []map[string]interface{} `json:"labels,omitempty"` Commits []map[string]interface{} `json:"commits,omitempty"` Builds []map[string]interface{} `json:"ci_builds,omitempty"` + CISummary *ReviewCISummary `json:"ci_summary,omitempty"` Notes []ScoringNote `json:"notes,omitempty"` } +type ReviewCISummary struct { + Total int `json:"total"` + Matched int `json:"matched"` + Unmatched int `json:"unmatched"` + Passed int `json:"passed"` + Failed int `json:"failed"` + Pending int `json:"pending"` + Unknown int `json:"unknown"` + MatchMode string `json:"match_mode"` + HeadBranch string `json:"head_branch,omitempty"` + HeadSHA string `json:"head_sha,omitempty"` +} + func newReviewContextShortcut() *common.Shortcut { return &common.Shortcut{ Name: "review-context", @@ -223,6 +237,7 @@ func FetchReviewContext(ctx *common.RuntimeContext, opts ReviewContextOptions) ( result.Notes = append(result.Notes, ScoringNote{Metric: "ci_builds", Note: fmt.Sprintf("ci +builds equivalent failed: %v", err)}) } else { result.Builds = builds + result.CISummary = summarizeReviewCI(result.PR, builds) result.Sections = append(result.Sections, "ci_builds") successes++ } @@ -255,6 +270,85 @@ func FetchReviewContext(ctx *common.RuntimeContext, opts ReviewContextOptions) ( return result, nil } +func summarizeReviewCI(pr map[string]interface{}, builds []map[string]interface{}) *ReviewCISummary { + summary := &ReviewCISummary{Total: len(builds), MatchMode: "unavailable"} + if pr == nil { + summary.MatchMode = "unavailable" + } else { + summary.HeadBranch = firstPRBranch(pr, "head_branch", "source_branch", "head") + summary.HeadSHA = firstPRString(pr, "head_commit_sha", "head_sha", "head_sha1", "last_commit_id") + if summary.HeadSHA != "" { + summary.MatchMode = "sha" + } else if summary.HeadBranch != "" { + summary.MatchMode = "branch" + } + } + + shaMatchedBuilds := make([]map[string]interface{}, 0, len(builds)) + branchMatchedBuilds := make([]map[string]interface{}, 0, len(builds)) + for _, build := range builds { + buildSHA := firstPRString(build, "head_commit_sha", "commit_sha", "commit_id", "sha", "after", "revision") + buildBranch := normalizeBuildBranch(firstPRString(build, "branch", "head_branch", "source_branch", "ref")) + if summary.HeadSHA != "" && commitPrefixMatches(buildSHA, summary.HeadSHA) { + shaMatchedBuilds = append(shaMatchedBuilds, build) + } + if summary.HeadBranch != "" && buildBranch == normalizeBuildBranch(summary.HeadBranch) && buildBranch != "" { + branchMatchedBuilds = append(branchMatchedBuilds, build) + } + } + matchedBuilds := shaMatchedBuilds + if len(matchedBuilds) > 0 { + summary.MatchMode = "sha" + } else if len(branchMatchedBuilds) > 0 { + matchedBuilds = branchMatchedBuilds + summary.MatchMode = "branch" + } else if len(builds) > 0 { + summary.MatchMode = "none" + } + summary.Matched = len(matchedBuilds) + summary.Unmatched = summary.Total - summary.Matched + for _, build := range matchedBuilds { + classifyReviewBuild(summary, build) + } + return summary +} + +func classifyReviewBuild(summary *ReviewCISummary, build map[string]interface{}) { + statusValues := make([]string, 0, 6) + for _, key := range []string{"status", "state", "build_status", "phase", "conclusion", "result"} { + if value := strings.ToLower(strings.TrimSpace(firstPRString(build, key))); value != "" { + statusValues = append(statusValues, value) + } + } + status := strings.Join(statusValues, " ") + switch { + case containsAny(status, []string{"failure", "failed", "error", "cancel", "red"}): + summary.Failed++ + case containsAny(status, []string{"success", "succeeded", "passed", "pass", "green"}): + summary.Passed++ + case containsAny(status, []string{"pending", "running", "queued", "waiting", "created", "started"}): + summary.Pending++ + default: + summary.Unknown++ + } +} + +func normalizeBuildBranch(branch string) string { + return strings.TrimPrefix(strings.TrimSpace(branch), "refs/heads/") +} + +func commitPrefixMatches(left, right string) bool { + left = strings.ToLower(strings.TrimSpace(left)) + right = strings.ToLower(strings.TrimSpace(right)) + if left == "" || right == "" { + return false + } + if left == right { + return true + } + return len(left) >= 7 && len(right) >= 7 && (strings.HasPrefix(left, right) || strings.HasPrefix(right, left)) +} + func fetchReviewContextPR(ctx *common.RuntimeContext, owner, repo string, number int) (map[string]interface{}, error) { env, err := ctx.CallAPI("GET", fmt.Sprintf("%s/pulls/%d", plainRepoPath(owner, repo), number), nil) if err != nil { @@ -326,6 +420,12 @@ func writeReviewContextMarkdown(w *strings.Builder, context ReviewContext) error _, _ = fmt.Fprintf(w, "- Reviews: `%d`\n", len(context.Reviews)) _, _ = fmt.Fprintf(w, "- Commits: `%d`\n", len(context.Commits)) _, _ = fmt.Fprintf(w, "- CI builds: `%d`\n", len(context.Builds)) + if context.CISummary != nil { + _, _ = fmt.Fprintf(w, "- CI matched: `%d/%d` (`%s`), unmatched `%d`, passed `%d`, failed `%d`, pending `%d`, unknown `%d`\n", + context.CISummary.Matched, context.CISummary.Total, context.CISummary.MatchMode, + context.CISummary.Unmatched, + context.CISummary.Passed, context.CISummary.Failed, context.CISummary.Pending, context.CISummary.Unknown) + } _, _ = fmt.Fprintf(w, "- Open issues included: `%d`\n", len(context.OpenIssues)) _, _ = fmt.Fprintf(w, "- Labels included: `%d`\n", len(context.Labels)) if len(context.Notes) > 0 { diff --git a/shortcuts/workflow/review_context_test.go b/shortcuts/workflow/review_context_test.go index 10fabac..8a60b70 100644 --- a/shortcuts/workflow/review_context_test.go +++ b/shortcuts/workflow/review_context_test.go @@ -136,12 +136,20 @@ func TestFetchReviewContextEvidenceSectionsAreBounded(t *testing.T) { {"sha": "ignored", "message": "should be bounded"}, }, }) + case "/owner/repo/pulls/12.json": + writeWorkflowJSON(t, w, map[string]interface{}{ + "pull_request": map[string]interface{}{ + "number": 12, + "head_branch": "feature/evidence", + "head_commit_sha": "abcdef1234567", + }, + }) case "/owner/repo/builds.json": if got := r.URL.Query().Get("limit"); got != "1" { t.Fatalf("CI limit = %q, want 1", got) } writeWorkflowJSON(t, w, map[string]interface{}{ - "builds": []map[string]interface{}{{"id": 9, "status": "success"}, {"id": 10, "status": "failed"}}, + "builds": []map[string]interface{}{{"id": 9, "sha": "abcdef1", "status": "completed", "conclusion": "success"}, {"id": 10, "sha": "other", "status": "failed"}}, }) default: t.Fatalf("unexpected request: %s %s", r.Method, r.URL.String()) @@ -155,6 +163,7 @@ func TestFetchReviewContextEvidenceSectionsAreBounded(t *testing.T) { BuildLimit: 1, IncludeCommits: true, IncludeCI: true, + IncludePR: true, }) if err != nil { t.Fatalf("FetchReviewContext returned error: %v", err) @@ -165,6 +174,36 @@ func TestFetchReviewContextEvidenceSectionsAreBounded(t *testing.T) { if !strings.Contains(strings.Join(got.Sections, ","), "commits") || !strings.Contains(strings.Join(got.Sections, ","), "ci_builds") { t.Fatalf("sections = %v, want commits and ci_builds", got.Sections) } + if got.CISummary == nil || got.CISummary.MatchMode != "sha" || got.CISummary.Matched != 1 || got.CISummary.Passed != 1 || got.CISummary.Failed != 0 || got.CISummary.Unmatched != 0 { + t.Fatalf("ci summary = %+v, want one matched passing build", got.CISummary) + } +} + +func TestSummarizeReviewCIUsesSHABeforeBranch(t *testing.T) { + summary := summarizeReviewCI( + map[string]interface{}{"head_branch": "feature/x", "head_commit_sha": "abcdef1234567"}, + []map[string]interface{}{ + {"sha": "abcdef1", "status": "completed", "conclusion": "success"}, + {"sha": "different", "branch": "feature/x", "status": "failed"}, + {"sha": "other", "branch": "feature/y", "status": "failed"}, + }, + ) + if summary.MatchMode != "sha" || summary.Matched != 1 || summary.Unmatched != 2 || summary.Passed != 1 || summary.Failed != 0 { + t.Fatalf("summary = %+v, want SHA-only matching with one pass", summary) + } +} + +func TestSummarizeReviewCIFallsBackToBranch(t *testing.T) { + summary := summarizeReviewCI( + map[string]interface{}{"head_branch": "feature/x"}, + []map[string]interface{}{ + {"ref": "refs/heads/feature/x", "status": "running"}, + {"branch": "feature/y", "status": "success"}, + }, + ) + if summary.MatchMode != "branch" || summary.Matched != 1 || summary.Unmatched != 1 || summary.Pending != 1 || summary.Passed != 0 { + t.Fatalf("summary = %+v, want branch fallback with one pending build", summary) + } } func TestFetchReviewContextEvidenceFailureIsNonFatal(t *testing.T) { -- 2.34.1 From 8094eeb38e92fc2e5dc3bc16628a16883a27c719 Mon Sep 17 00:00:00 2001 From: Mengz <2567587994@qq.com> Date: Wed, 22 Jul 2026 17:33:02 +0800 Subject: [PATCH 3/3] =?UTF-8?q?fix(workflow):=20=E4=BF=AE=E6=AD=A3=20PR=20?= =?UTF-8?q?=E6=8F=90=E4=BA=A4=E8=AF=81=E6=8D=AE=E9=87=87=E9=9B=86?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../workflow-review-context-evidence.md | 6 +- shortcuts/workflow/review_context.go | 88 +++++++++++++++++-- shortcuts/workflow/review_context_test.go | 38 +++++++- 3 files changed, 121 insertions(+), 11 deletions(-) diff --git a/doc/changes/workflow-review-context-evidence.md b/doc/changes/workflow-review-context-evidence.md index dd076d9..a7953c8 100644 --- a/doc/changes/workflow-review-context-evidence.md +++ b/doc/changes/workflow-review-context-evidence.md @@ -18,7 +18,7 @@ gitlink-cli workflow +review-context \ 新增结果字段： -- `commits`：PR 提交记录，只有启用 `include-commits` 时出现。 +- `commits`：PR 提交记录，只有启用 `include-commits` 时出现。命令先从 PR 详情解析内部 `pull_request_id`，再调用平台实际 commits 端点，不能把网页 HTML 响应当成空列表。 - `ci_builds`：仓库 CI 构建记录，只有启用 `include-ci` 时出现。 - `ci_summary`：优先按 PR head SHA 关联构建，找不到 SHA 匹配时回退到 head branch；状态统计只包含匹配构建，并显式给出 `unmatched`，避免把其他分支的失败构建误判为当前 PR 失败。 - `sections`：成功获取的证据分区，便于 Agent 判断证据是否完整。 @@ -30,6 +30,8 @@ gitlink-cli workflow +review-context \ - 所有新增请求均为 `GET`，不会评论、审批、合并、关闭或修改远程资源。 - 每个列表都带有上限，避免大仓库响应无限膨胀。 - CI 获取失败时保留其他证据，并在 `notes` 中标记为不可用；只有所有启用分区都失败时才返回错误。 +- PR 详情缺少 head SHA 时，从带时间戳的最新 commit 推导当前 SHA，再进行 CI 精确匹配；无法取得 commit 证据时才降级到分支匹配。 +- 列表探针必须返回结构化数组或已知列表包装；HTML、普通文本和未知对象会进入 `notes`，不会加入 `sections` 冒充成功。 - 标签列表兼容 `issue_tags` 和 `tags` 包装，避免接口返回标签数据时被错误解析为空。 ## 验证 @@ -38,3 +40,5 @@ gitlink-cli workflow +review-context \ go test ./shortcuts/workflow -run 'TestFetchReviewContext|TestSummarizeReviewCI|TestRenderReviewContextFormats' -count=1 go build ./... ``` + +真实平台验证：PR #429 返回 2 条 commits，最新 SHA 与 PR head 一致；仓库 CI 接口不可用时 `ci_builds` 不进入 `sections`，并保留可解释的 `ci_builds` note。 diff --git a/shortcuts/workflow/review_context.go b/shortcuts/workflow/review_context.go index 7712acc..84fc8b7 100644 --- a/shortcuts/workflow/review_context.go +++ b/shortcuts/workflow/review_context.go @@ -5,6 +5,7 @@ import ( "net/url" "os" "strings" + "time" "github.com/gitlink-org/gitlink-cli/cmd/cmdutil" "github.com/gitlink-org/gitlink-cli/shortcuts/common" @@ -186,6 +187,7 @@ func FetchReviewContext(ctx *common.RuntimeContext, opts ReviewContextOptions) ( Notes: []ScoringNote{}, } successes := 0 + var prEvidence map[string]interface{} if opts.IncludeRepo { if info, err := fetchRepoInfo(ctx, owner, repo); err != nil { @@ -196,13 +198,18 @@ func FetchReviewContext(ctx *common.RuntimeContext, opts ReviewContextOptions) ( successes++ } } - if opts.IncludePR { + if opts.IncludePR || opts.IncludeCommits || opts.IncludeCI { if pr, err := fetchReviewContextPR(ctx, owner, repo, opts.Number); err != nil { - result.Notes = append(result.Notes, ScoringNote{Metric: "pr_view", Note: fmt.Sprintf("pr +view equivalent failed: %v", err)}) + if opts.IncludePR { + result.Notes = append(result.Notes, ScoringNote{Metric: "pr_view", Note: fmt.Sprintf("pr +view equivalent failed: %v", err)}) + } } else { - result.PR = pr - result.Sections = append(result.Sections, "pr") - successes++ + prEvidence = pr + if opts.IncludePR { + result.PR = pr + result.Sections = append(result.Sections, "pr") + successes++ + } } } if opts.IncludeFiles { @@ -224,7 +231,10 @@ func FetchReviewContext(ctx *common.RuntimeContext, opts ReviewContextOptions) ( } } if opts.IncludeCommits { - if commits, err := fetchReviewContextList(ctx, fmt.Sprintf("%s/pulls/%d/commits", workflowRepoPath(owner, repo), opts.Number), nil, opts.CommitLimit); err != nil { + commitPath, err := reviewContextCommitPath(owner, repo, prEvidence) + if err != nil { + result.Notes = append(result.Notes, ScoringNote{Metric: "pr_commits", Note: err.Error()}) + } else if commits, err := fetchReviewContextList(ctx, commitPath, nil, opts.CommitLimit); err != nil { result.Notes = append(result.Notes, ScoringNote{Metric: "pr_commits", Note: fmt.Sprintf("pr +commits equivalent failed: %v", err)}) } else { result.Commits = commits @@ -237,7 +247,7 @@ func FetchReviewContext(ctx *common.RuntimeContext, opts ReviewContextOptions) ( result.Notes = append(result.Notes, ScoringNote{Metric: "ci_builds", Note: fmt.Sprintf("ci +builds equivalent failed: %v", err)}) } else { result.Builds = builds - result.CISummary = summarizeReviewCI(result.PR, builds) + result.CISummary = summarizeReviewCI(prEvidence, result.Commits, builds) result.Sections = append(result.Sections, "ci_builds") successes++ } @@ -270,13 +280,13 @@ func FetchReviewContext(ctx *common.RuntimeContext, opts ReviewContextOptions) ( return result, nil } -func summarizeReviewCI(pr map[string]interface{}, builds []map[string]interface{}) *ReviewCISummary { +func summarizeReviewCI(pr map[string]interface{}, commits, builds []map[string]interface{}) *ReviewCISummary { summary := &ReviewCISummary{Total: len(builds), MatchMode: "unavailable"} if pr == nil { summary.MatchMode = "unavailable" } else { summary.HeadBranch = firstPRBranch(pr, "head_branch", "source_branch", "head") - summary.HeadSHA = firstPRString(pr, "head_commit_sha", "head_sha", "head_sha1", "last_commit_id") + summary.HeadSHA = reviewContextHeadSHA(pr, commits) if summary.HeadSHA != "" { summary.MatchMode = "sha" } else if summary.HeadBranch != "" { @@ -313,6 +323,29 @@ func summarizeReviewCI(pr map[string]interface{}, builds []map[string]interface{ return summary } +func reviewContextHeadSHA(pr map[string]interface{}, commits []map[string]interface{}) string { + if sha := firstPRString(pr, "head_commit_sha", "head_sha", "head_sha1", "last_commit_id"); sha != "" { + return sha + } + var headSHA string + var headTime time.Time + for _, commit := range commits { + sha := firstPRString(commit, "sha", "commit_sha", "id") + if sha == "" { + continue + } + commitTime := apiLatestTime( + firstPRTime(commit, "timestamp"), + firstPRTime(commit, "created_at", "committed_at", "date"), + ) + if headSHA == "" || (!commitTime.IsZero() && (headTime.IsZero() || commitTime.After(headTime))) { + headSHA = sha + headTime = commitTime + } + } + return headSHA +} + func classifyReviewBuild(summary *ReviewCISummary, build map[string]interface{}) { statusValues := make([]string, 0, 6) for _, key := range []string{"status", "state", "build_status", "phase", "conclusion", "result"} { @@ -361,6 +394,17 @@ func fetchReviewContextPR(ctx *common.RuntimeContext, owner, repo string, number return item, nil } +func reviewContextCommitPath(owner, repo string, pr map[string]interface{}) (string, error) { + if pr == nil { + return "", fmt.Errorf("pr +commits requires PR details to resolve the internal pull_request_id") + } + id := firstPRInt(pr, "id", "pull_request_id") + if id <= 0 { + return "", fmt.Errorf("pr +commits response is unavailable: PR details did not contain pull_request_id") + } + return fmt.Sprintf("%s/pulls/%d/commits", plainRepoPath(owner, repo), id), nil +} + func fetchReviewContextList(ctx *common.RuntimeContext, path string, query url.Values, limit int) ([]map[string]interface{}, error) { if limit <= 0 { limit = 100 @@ -373,6 +417,9 @@ func fetchReviewContextList(ctx *common.RuntimeContext, path string, query url.V if err != nil { return nil, err } + if !isStructuredReviewContextList(env.Data) { + return nil, fmt.Errorf("response did not contain a structured list") + } items := apiList(env.Data) out := make([]map[string]interface{}, 0, len(items)) for _, raw := range items { @@ -388,6 +435,29 @@ func fetchReviewContextList(ctx *common.RuntimeContext, path string, query url.V return out, nil } +func isStructuredReviewContextList(data interface{}) bool { + normalized, err := normalizeAPIData(data) + if err != nil { + return false + } + switch value := normalized.(type) { + case []interface{}: + return true + case map[string]interface{}: + for _, key := range []string{"issues", "pulls", "pull_requests", "reviews", "journals", "comments", "notes", "files", "commits", "releases", "builds", "issue_tags", "tags", "items", "records"} { + if _, ok := value[key]; ok { + return true + } + } + if nested, ok := value["data"]; ok { + return isStructuredReviewContextList(nested) + } + return looksLikeIssueOrRepoItem(value) + default: + return false + } +} + func RenderReviewContext(context ReviewContext, format string) (string, error) { var rendered strings.Builder switch normalizeFormat(format) { diff --git a/shortcuts/workflow/review_context_test.go b/shortcuts/workflow/review_context_test.go index 8a60b70..f0eefe8 100644 --- a/shortcuts/workflow/review_context_test.go +++ b/shortcuts/workflow/review_context_test.go @@ -125,7 +125,7 @@ func TestFetchReviewContextPartialFailureKeepsNotes(t *testing.T) { func TestFetchReviewContextEvidenceSectionsAreBounded(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { - case "/v1/owner/repo/pulls/12/commits.json": + case "/owner/repo/pulls/9012/commits.json": if got := r.URL.Query().Get("limit"); got != "2" { t.Fatalf("commit limit = %q, want 2", got) } @@ -139,6 +139,7 @@ func TestFetchReviewContextEvidenceSectionsAreBounded(t *testing.T) { case "/owner/repo/pulls/12.json": writeWorkflowJSON(t, w, map[string]interface{}{ "pull_request": map[string]interface{}{ + "id": 9012, "number": 12, "head_branch": "feature/evidence", "head_commit_sha": "abcdef1234567", @@ -182,6 +183,7 @@ func TestFetchReviewContextEvidenceSectionsAreBounded(t *testing.T) { func TestSummarizeReviewCIUsesSHABeforeBranch(t *testing.T) { summary := summarizeReviewCI( map[string]interface{}{"head_branch": "feature/x", "head_commit_sha": "abcdef1234567"}, + nil, []map[string]interface{}{ {"sha": "abcdef1", "status": "completed", "conclusion": "success"}, {"sha": "different", "branch": "feature/x", "status": "failed"}, @@ -196,6 +198,7 @@ func TestSummarizeReviewCIUsesSHABeforeBranch(t *testing.T) { func TestSummarizeReviewCIFallsBackToBranch(t *testing.T) { summary := summarizeReviewCI( map[string]interface{}{"head_branch": "feature/x"}, + nil, []map[string]interface{}{ {"ref": "refs/heads/feature/x", "status": "running"}, {"branch": "feature/y", "status": "success"}, @@ -206,12 +209,45 @@ func TestSummarizeReviewCIFallsBackToBranch(t *testing.T) { } } +func TestSummarizeReviewCIDerivesHeadSHAFromLatestCommit(t *testing.T) { + summary := summarizeReviewCI( + map[string]interface{}{"head_branch": "feature/x"}, + []map[string]interface{}{ + {"sha": "older1234567", "timestamp": 100}, + {"sha": "newer1234567", "timestamp": 200}, + }, + []map[string]interface{}{ + {"sha": "newer12", "status": "success"}, + {"branch": "feature/x", "status": "failed"}, + }, + ) + if summary.HeadSHA != "newer1234567" || summary.MatchMode != "sha" || summary.Matched != 1 || summary.Passed != 1 || summary.Failed != 0 { + t.Fatalf("summary = %+v, want latest commit SHA with one matching pass", summary) + } +} + +func TestFetchReviewContextListRejectsUnstructuredBody(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + writeWorkflowJSON(t, w, "") + })) + defer server.Close() + + _, err := fetchReviewContextList(workflowTestContext(server), "/owner/repo/pulls/1/commits", nil, 10) + if err == nil || !strings.Contains(err.Error(), "structured list") { + t.Fatalf("error = %v, want unstructured response rejection", err) + } +} + func TestFetchReviewContextEvidenceFailureIsNonFatal(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path == "/v1/owner/repo.json" { writeWorkflowJSON(t, w, map[string]interface{}{"name": "repo"}) return } + if r.URL.Path == "/owner/repo/pulls/13.json" { + writeWorkflowJSON(t, w, map[string]interface{}{"pull_request": map[string]interface{}{"id": 9013, "head": "feature/evidence"}}) + return + } if r.URL.Path == "/owner/repo/builds.json" { w.WriteHeader(http.StatusBadGateway) _, _ = w.Write([]byte("CI unavailable")) -- 2.34.1