fix(workflow): 修正 PR 提交证据采集

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。

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) {

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, "<!doctype html><html></html>")
+	}))
+	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"))