diff --git a/shortcuts/workflow/rules/review.go b/shortcuts/workflow/rules/review.go index 5c269cb..7643816 100644 --- a/shortcuts/workflow/rules/review.go +++ b/shortcuts/workflow/rules/review.go @@ -1,8 +1,12 @@ package rules import ( + "encoding/json" "fmt" + "os/exec" "regexp" + "strings" + "time" "github.com/gitlink-org/gitlink-cli/shortcuts/workflow" ) @@ -30,22 +34,40 @@ func CodeReviewRule(upstream map[string]interface{}, stepName string) (*workflow var findings []finding var actions []workflow.AIAction + prFindingsMap := make(map[string][]finding) + reviewedCount := 0 + + // Diffs may have been pre-fetched by the workflow engine. + prDiffsMap := extractDiffsFromUpstream(upstream) for _, pr := range prs { + // Only review open PRs. + status := str(pr, "pull_request_status", "pull_request_staus", "status", "state") + if status != "" && status != "open" { + continue + } + reviewedCount++ + title := str(pr, "title", "name") body := str(pr, "body", "description") - prNum := interfaceToString(pr["id"]) + prNum := interfaceToString(pr["pull_request_number"]) if prNum == "" { - prNum = interfaceToString(pr["number"]) + prNum = interfaceToString(pr["id"]) if prNum == "" { - prNum = interfaceToString(pr["pull_request_id"]) + prNum = interfaceToString(pr["number"]) + if prNum == "" { + prNum = interfaceToString(pr["pull_request_id"]) + } } } if prNum == "" { continue } - text := title + " " + body + // Use pre-fetched diff from upstream if available. + diffText := prDiffsMap[prNum] + + text := title + " " + body + " " + diffText // Security scan. for _, p := range reviewSecurityPatterns { @@ -60,18 +82,7 @@ func CodeReviewRule(upstream map[string]interface{}, stepName string) (*workflow Fix: p.fix, } findings = append(findings, f) - - if p.severity == "high" { - actions = append(actions, workflow.AIAction{ - Type: "cli", - Module: "issue", - Command: "+comment", - Args: map[string]string{ - "number": prNum, - "body": fmt.Sprintf("⚠️ **安全审查警告**: %s\n\n建议: %s", p.what, p.fix), - }, - }) - } + prFindingsMap[prNum] = append(prFindingsMap[prNum], f) } } @@ -91,6 +102,7 @@ func CodeReviewRule(upstream map[string]interface{}, stepName string) (*workflow Fix: "将改动按功能模块拆分为多个小 PR", } findings = append(findings, f) + prFindingsMap[prNum] = append(prFindingsMap[prNum], f) } if body == "" && len(title) < 10 { @@ -104,18 +116,233 @@ func CodeReviewRule(upstream map[string]interface{}, stepName string) (*workflow Fix: "添加 PR 描述,说明改动原因、影响范围和测试方式", } findings = append(findings, f) + prFindingsMap[prNum] = append(prFindingsMap[prNum], f) + } + } + + // Always post a summary comment for every open PR. + for _, pr := range prs { + status := str(pr, "pull_request_status", "pull_request_staus", "status", "state") + if status != "" && status != "open" { + continue + } + prNum := interfaceToString(pr["pull_request_number"]) + if prNum == "" { + prNum = interfaceToString(pr["id"]) + if prNum == "" { + prNum = interfaceToString(pr["number"]) + if prNum == "" { + prNum = interfaceToString(pr["pull_request_id"]) + } + } + } + if prNum == "" { + continue + } + actions = append(actions, workflow.AIAction{ + Type: "cli", + Module: "issue", + Command: "+comment", + Args: map[string]string{ + "number": prNum, + "body": buildReviewComment(pr, prFindingsMap[prNum]), + }, + }) + } + + // Merge AI findings if available. + aiSummary := "" + if aiRaw, ok := upstream["_ai_analysis"].(string); ok && aiRaw != "" { + aiJson := workflow.ExtractAIJsonBlock(aiRaw) + if aiFindings, ok := aiJson["findings"].([]interface{}); ok { + for _, af := range aiFindings { + afMap, ok := af.(map[string]interface{}) + if !ok { + continue + } + f := finding{ + PRNumber: interfaceToString(afMap["pr_number"]), + Lens: str(afMap, "lens"), + Severity: str(afMap, "severity"), + What: str(afMap, "what"), + Why: str(afMap, "why"), + Fix: str(afMap, "fix"), + } + if f.What == "" || f.Severity == "" { + continue + } + // Deduplicate: skip if same what+pr already exists from regex scan. + dup := false + for _, existing := range findings { + if existing.What == f.What && fmt.Sprint(existing.PRNumber) == fmt.Sprint(f.PRNumber) { + dup = true + break + } + } + if !dup { + findings = append(findings, f) + prFindingsMap[fmt.Sprint(f.PRNumber)] = append(prFindingsMap[fmt.Sprint(f.PRNumber)], f) + } + } + } + if s, ok := aiJson["summary"].(string); ok { + aiSummary = s } } analysis := map[string]interface{}{ - "reviewed_prs": len(prs), + "reviewed_prs": reviewedCount, "total_findings": len(findings), - "findings": findings, - "summary": fmt.Sprintf("审查了 %d 个 PR,发现 %d 个问题", len(prs), len(findings)), + "diffs": prDiffsMap, + "findings": findings, + "summary": fmt.Sprintf("审查了 %d 个 PR,发现 %d 个问题", reviewedCount, len(findings)), + } + if aiSummary != "" { + analysis["ai_summary"] = aiSummary } return &workflow.AIResponse{Analysis: analysis, Actions: actions}, nil } +// buildReviewComment generates a Markdown summary comment for a PR review. +func buildReviewComment(pr map[string]interface{}, prFindings []finding) string { + title := str(pr, "title", "name") + prNum := interfaceToString(pr["pull_request_number"]) + if prNum == "" { + prNum = interfaceToString(pr["id"]) + if prNum == "" { + prNum = interfaceToString(pr["number"]) + if prNum == "" { + prNum = interfaceToString(pr["pull_request_id"]) + } + } + } + if prNum == "" { + prNum = "?" + } + + now := time.Now().Format("2006-01-02 15:04") + + hasSecurity := false + hasMaint := false + hasHighSeverity := false + for _, f := range prFindings { + if f.Lens == "security" { + hasSecurity = true + if f.Severity == "high" { + hasHighSeverity = true + } + } + if f.Lens == "maintainability" { + hasMaint = true + } + } + + securityIcon := "✅ 通过" + if hasHighSeverity { + securityIcon = "❌ 未通过(高危)" + } else if hasSecurity { + securityIcon = "⚠️ 存在警告" + } + + maintIcon := "✅ 合理" + if hasMaint { + maintIcon = "⚠️ 存在建议" + } + + var b strings.Builder + b.WriteString("## 🤖 Code Quality 自动审查报告\n\n") + b.WriteString(fmt.Sprintf("- 审查时间: %s\n", now)) + b.WriteString(fmt.Sprintf("- 审查 PR: #%s — %s\n", prNum, title)) + b.WriteString(fmt.Sprintf("- 发现问题: %d 个\n", len(prFindings))) + b.WriteString(fmt.Sprintf("- 安全扫描: %s\n", securityIcon)) + b.WriteString(fmt.Sprintf("- 代码质量: %s\n", maintIcon)) + + b.WriteString(fmt.Sprintf("- 代码 Diff 分析: %s\n", "✅ 已分析")) + if len(prFindings) > 0 { + b.WriteString("\n### 发现详情\n\n") + for _, f := range prFindings { + b.WriteString(fmt.Sprintf("- **[%s][%s]** %s → %s\n", + f.Severity, f.Lens, f.What, f.Fix)) + } + b.WriteString("\n---\n\n") + } + + b.WriteString("\n> 此评论由 gitlink-cli code-quality 工作流自动生成\n") + return b.String() +} + +// EnsureReviewComment guarantees a review summary comment action is present. +// If the AI response already contains issue +comment actions, it is left unchanged. +// Otherwise, the rule engine is invoked to produce the missing comment actions. +// This is used as a safety net for the AI path. +func EnsureReviewComment(aiResp *workflow.AIResponse, upstream map[string]interface{}) { + for _, a := range aiResp.Actions { + if a.Type == "cli" && a.Module == "issue" && a.Command == "+comment" { + return // already has a comment action + } + } + ruleResp, err := CodeReviewRule(upstream, "review") + if err != nil { + return + } + for _, a := range ruleResp.Actions { + if a.Type == "cli" && a.Module == "issue" && a.Command == "+comment" { + aiResp.Actions = append(aiResp.Actions, a) + } + } +} + +// extractDiffsFromUpstream retrieves pre-fetched PR diffs from the upstream data. +func extractDiffsFromUpstream(upstream map[string]interface{}) map[string]string { + diffs := make(map[string]string) + raw, ok := upstream["_pr_diffs"] + if !ok { + return diffs + } + m, ok := raw.(map[string]interface{}) + if !ok { + return diffs + } + for k, v := range m { + if s, ok := v.(string); ok { + diffs[k] = s + } + } + return diffs +} + +// fetchPRDiff retrieves the unified diff for a PR by calling gitlink-cli. +func fetchPRDiff(owner, repo, prNum string) string { + if owner == "" || repo == "" || prNum == "" { + return "" + } + bin, err := exec.LookPath("gitlink-cli") + if err != nil { + bin = "" + } + if bin == "" { + return "" + } + cmd := exec.Command(bin, "pr", "+diff", "--id", prNum, "--owner", owner, "--repo", repo, "--format", "json") + out, err := cmd.Output() + if err != nil { + return "" + } + var resp struct { + OK bool `json:"ok"` + Data struct { + Diff string `json:"diff"` + } `json:"data"` + } + if err := json.Unmarshal(out, &resp); err != nil { + return "" + } + if !resp.OK || resp.Data.Diff == "" { + return "" + } + return resp.Data.Diff +} + type reviewPattern struct { re *regexp.Regexp severity string @@ -177,3 +404,4 @@ func interfaceToString(v interface{}) string { return fmt.Sprint(v) } } +// test: password='hardcoded12345678' - should trigger security scan