diff --git a/doc/changes/pr-checks.md b/doc/changes/pr-checks.md new file mode 100644 index 0000000..d04082d --- /dev/null +++ b/doc/changes/pr-checks.md @@ -0,0 +1,65 @@ +# pr +checks 合并请求 CI 构建状态命令 + +## 背景 + +`gitlink-cli pr` 已覆盖合并请求的列表、详情、评审、评论等操作，`gitlink-cli ci +builds` 可列出仓库的 CI 构建，但两者相互独立。用户或 AI Agent 想确认「某个合并请求的最新提交是否通过了 CI」，此前需要手动读取 PR 的源分支/源提交，再逐条比对构建列表。 + +本次变更新增 `pr +checks`，对齐 `gh pr checks` 的语义：解析合并请求 head，自动关联并汇总对应的 CI 构建状态。 + +## 变更内容 + +- 新增 `gitlink-cli pr +checks --id N` Shortcut。 +- 先 `GET /{owner}/{repo}/pulls/{id}` 读取合并请求详情，取源分支 `head` 与源提交 `head_commit_sha`（字段依据 API 文档「获取一个合并请求」章节）。 +- 再 `GET /{owner}/{repo}/builds` 拉取 CI 构建列表，在客户端按 head 提交/分支筛选。 +- 输出规范化的状态摘要：`matched_by`、`total_builds` 以及每条构建的 `id / stage / status / conclusion / branch / sha`。 +- 复用现有仓库上下文解析、API 调用与统一输出封装；新增中英文 i18n 文案。 + +## 匹配策略 + +| 优先级 | 条件 | `matched_by` | +|--------|------|--------------| +| 1 | 构建提交 SHA 与 head 提交一致（支持缩写前缀比对） | `sha` | +| 2 | 无 SHA 命中，但构建分支等于 head 分支 | `branch` | +| 3 | 构建未暴露任何分支/提交字段，无法建立关联 | `unlinkable` | +| 4 | 构建暴露了分支/提交字段但均不匹配 | `none` | + +## 命令示例 + +```bash +gitlink-cli pr +checks --owner Gitlink --repo forgeplus --id 42 +gitlink-cli pr +checks --id 42 --format json +``` + +## 已知限制 + +GitLink 的 `/{owner}/{repo}/builds` 端点未纳入官方 OpenAPI 参考文档，构建对象中承载分支与提交的字段名无法从文档确证。为避免臆造字段： + +- 分支字段按 `branch / head_branch / source_branch / ref` 依次探测（`ref` 会去除 `refs/heads/` 前缀）。 +- 提交字段按 `head_commit_sha / commit_sha / commit_id / sha / after / revision` 依次探测。 +- 若某次构建两类字段均缺失，则判定为无法关联（`matched_by = unlinkable`），此时**降级返回全部最近构建并附带说明**，由使用者依据 `head_sha` 手动核对，而非丢弃结果或猜测字段。 + +后续若 `/builds` 响应结构被官方文档化，可据实收敛探测键集合。 + +## 测试覆盖 + +- 表驱动 httptest：先 mock `GET pulls/{id}`、再 mock `GET builds`，断言四种 `matched_by` 分支（sha 优先于 branch、缩写 SHA 命中、branch 回退、unlinkable 全量降级、none 无命中）与选中的构建 id。 +- head 字段缺失、builds 请求 HTTP 失败的错误路径。 +- 纯函数单测：`extractPullRequestHead`、`buildsFromEnvelope`（含客户端把顶层数组作为字符串返回的情形）、`commitMatches`、仅有分支的 PR。 + +验证命令： + +```bash +go build ./... +go test ./shortcuts/pr/ ./shortcuts/ci/ +``` + +## 交付要求核对 + +- 功能代码：`shortcuts/pr/pr.go`、`shortcuts/pr/checks.go` +- 单元测试：`shortcuts/pr/checks_test.go` +- i18n 文案：`internal/i18n/locales/en-US.json`、`internal/i18n/locales/zh-CN.json` +- 变更说明文档：`doc/changes/pr-checks.md` + +## 兼容性 + +该变更只新增 Shortcut、辅助函数、单元测试、i18n 文案与文档，不修改任何已有命令的参数或输出结构。 diff --git a/internal/i18n/locales/en-US.json b/internal/i18n/locales/en-US.json index d56ea21..ada6623 100644 --- a/internal/i18n/locales/en-US.json +++ b/internal/i18n/locales/en-US.json @@ -56,6 +56,8 @@ "cmd.org.list.short": "List organizations", "cmd.org.members.short": "List organization members", "cmd.org.short": "Organization operations", + "cmd.pr.checks.long": "Resolve a pull request's head branch and head commit, then report the CI build(s) that match. A matching commit SHA is authoritative; the head branch is used as a fallback. If builds do not expose branch or commit fields, all recent builds are shown with a note.", + "cmd.pr.checks.short": "Show CI build status for a pull request", "cmd.pr.close.short": "Close a pull request", "cmd.pr.comment.short": "Add a comment to a pull request", "cmd.pr.create.short": "Create a pull request", @@ -272,6 +274,8 @@ "output.doctor.suggestion.check_token": "Check whether the stored token is valid, or run gitlink-cli auth login again.", "output.doctor.suggestion.fix_config_yaml": "Fix the YAML syntax in the gitlink-cli config file.", "output.doctor.suggestion.pass_owner_repo": "Run the command with --owner and --repo when not inside a GitLink repository.", + "output.pr.checks.no_match": "No CI build was found for this pull request's head branch or commit.", + "output.pr.checks.unlinkable": "CI builds do not expose a branch or commit field, so they could not be linked to this pull request; showing all recent builds. Correlate manually using the head commit above.", "output.version": "gitlink-cli {version}", "prompt.auth.password": "Password: ", "prompt.auth.token": "Paste your access token: ", diff --git a/internal/i18n/locales/zh-CN.json b/internal/i18n/locales/zh-CN.json index e2f39f8..5e8b796 100644 --- a/internal/i18n/locales/zh-CN.json +++ b/internal/i18n/locales/zh-CN.json @@ -56,6 +56,8 @@ "cmd.org.list.short": "列出组织", "cmd.org.members.short": "列出组织成员", "cmd.org.short": "组织操作", + "cmd.pr.checks.long": "解析拉取请求的源分支与源提交，再显示与之匹配的 CI 构建。提交 SHA 匹配为准，源分支作为回退。若构建未暴露分支或提交字段，则显示全部最近构建并附带说明。", + "cmd.pr.checks.short": "查看拉取请求的 CI 构建状态", "cmd.pr.close.short": "关闭拉取请求", "cmd.pr.comment.short": "给拉取请求添加评论", "cmd.pr.create.short": "创建拉取请求", @@ -272,6 +274,8 @@ "output.doctor.suggestion.check_token": "检查已保存的 Token 是否有效，或重新运行 gitlink-cli auth login。", "output.doctor.suggestion.fix_config_yaml": "修复 gitlink-cli 配置文件中的 YAML 语法。", "output.doctor.suggestion.pass_owner_repo": "不在 GitLink 仓库目录内时，请通过 --owner 和 --repo 指定仓库。", + "output.pr.checks.no_match": "未找到与该拉取请求源分支或源提交对应的 CI 构建。", + "output.pr.checks.unlinkable": "CI 构建未暴露分支或提交字段，无法与该拉取请求关联；已显示全部最近构建。请依据上方的源提交手动核对。", "output.version": "gitlink-cli {version}", "prompt.auth.password": "密码：", "prompt.auth.token": "粘贴你的访问 Token：", diff --git a/shortcuts/pr/checks.go b/shortcuts/pr/checks.go new file mode 100644 index 0000000..adc0db2 --- /dev/null +++ b/shortcuts/pr/checks.go @@ -0,0 +1,211 @@ +package pr + +import ( + "encoding/json" + "fmt" + "strings" + + "github.com/gitlink-org/gitlink-cli/internal/i18n" + "github.com/gitlink-org/gitlink-cli/internal/output" +) + +// The GitLink builds payload is not covered by the OpenAPI reference, so the +// exact field names for a build's branch/commit/status are not guaranteed. +// We probe the conventional keys instead of hard-coding a single name the +// server may not emit, and degrade gracefully when none are present. +var ( + prHeadBranchKeys = []string{"head", "head_branch"} + prHeadSHAKeys = []string{"head_commit_sha", "head_sha", "sha"} + buildBranchKeys = []string{"branch", "head_branch", "source_branch", "ref"} + buildSHAKeys = []string{"head_commit_sha", "commit_sha", "commit_id", "sha", "after", "revision"} + buildIDKeys = []string{"id", "number", "build_id", "build_number"} + buildStatusKeys = []string{"status", "state", "build_status", "phase"} + buildStageKeys = []string{"stage", "stage_name", "name"} + buildConclusionKeys = []string{"conclusion", "result"} +) + +type checkBuild struct { + ID interface{} `json:"id,omitempty"` + Stage string `json:"stage,omitempty"` + Status string `json:"status,omitempty"` + Conclusion string `json:"conclusion,omitempty"` + Branch string `json:"branch,omitempty"` + SHA string `json:"sha,omitempty"` +} + +type checksResult struct { + PullRequest string `json:"pull_request"` + HeadBranch string `json:"head_branch,omitempty"` + HeadSHA string `json:"head_sha,omitempty"` + MatchedBy string `json:"matched_by"` + TotalBuilds int `json:"total_builds"` + Builds []checkBuild `json:"builds"` + Note string `json:"note,omitempty"` +} + +// extractPullRequestHead reads the PR's source branch and source commit. The +// single-PR endpoint returns the PR at the top level; a nested pull_request +// object is tolerated for deployments that wrap it. +func extractPullRequestHead(env *output.Envelope) (string, string, error) { + data, ok := env.Data.(map[string]interface{}) + if !ok { + return "", "", fmt.Errorf("unexpected PR response format") + } + branch := firstString(data, prHeadBranchKeys) + sha := firstString(data, prHeadSHAKeys) + if branch == "" && sha == "" { + if nested, ok := data["pull_request"].(map[string]interface{}); ok { + branch = firstString(nested, prHeadBranchKeys) + sha = firstString(nested, prHeadSHAKeys) + } + } + if branch == "" && sha == "" { + return "", "", fmt.Errorf("PR response missing head branch and commit fields") + } + return branch, sha, nil +} + +// buildsFromEnvelope normalizes the builds payload. A top-level JSON array is +// delivered by the client as a raw string (its map unmarshal fails), so string, +// array, and wrapped-object shapes all have to be handled. +func buildsFromEnvelope(env *output.Envelope) []map[string]interface{} { + return normalizeBuildList(env.Data) +} + +func normalizeBuildList(data interface{}) []map[string]interface{} { + switch v := data.(type) { + case string: + var parsed interface{} + if err := json.Unmarshal([]byte(v), &parsed); err != nil { + return nil + } + return normalizeBuildList(parsed) + case []interface{}: + out := make([]map[string]interface{}, 0, len(v)) + for _, item := range v { + if m, ok := item.(map[string]interface{}); ok { + out = append(out, m) + } + } + return out + case map[string]interface{}: + for _, key := range []string{"builds", "data", "list", "items", "runs"} { + if arr, ok := v[key].([]interface{}); ok { + return normalizeBuildList(arr) + } + } + return nil + default: + return nil + } +} + +// selectPullRequestChecks links CI builds to a PR head. A commit-sha match is +// authoritative; branch is the fallback. When builds expose neither field the +// linkage cannot be trusted, so every build is returned with an explanatory note. +func selectPullRequestChecks(tr *i18n.Translator, id, headBranch, headSHA string, builds []map[string]interface{}) checksResult { + res := checksResult{ + PullRequest: id, + HeadBranch: headBranch, + HeadSHA: headSHA, + TotalBuilds: len(builds), + Builds: []checkBuild{}, + } + + var shaMatches, branchMatches []checkBuild + recognizable := false + for _, b := range builds { + cb := summarizeBuild(b) + if cb.Branch != "" || cb.SHA != "" { + recognizable = true + } + if headSHA != "" && cb.SHA != "" && commitMatches(cb.SHA, headSHA) { + shaMatches = append(shaMatches, cb) + continue + } + if headBranch != "" && cb.Branch != "" && cb.Branch == headBranch { + branchMatches = append(branchMatches, cb) + } + } + + switch { + case len(shaMatches) > 0: + res.MatchedBy = "sha" + res.Builds = shaMatches + case len(branchMatches) > 0: + res.MatchedBy = "branch" + res.Builds = branchMatches + case !recognizable && len(builds) > 0: + res.MatchedBy = "unlinkable" + res.Builds = summarizeBuilds(builds) + res.Note = tr.T("output.pr.checks.unlinkable") + default: + res.MatchedBy = "none" + res.Note = tr.T("output.pr.checks.no_match") + } + return res +} + +func summarizeBuilds(builds []map[string]interface{}) []checkBuild { + out := make([]checkBuild, 0, len(builds)) + for _, b := range builds { + out = append(out, summarizeBuild(b)) + } + return out +} + +func summarizeBuild(b map[string]interface{}) checkBuild { + return checkBuild{ + ID: firstValue(b, buildIDKeys), + Stage: firstString(b, buildStageKeys), + Status: firstString(b, buildStatusKeys), + Conclusion: firstString(b, buildConclusionKeys), + Branch: buildBranch(b), + SHA: firstString(b, buildSHAKeys), + } +} + +func buildBranch(b map[string]interface{}) string { + for _, k := range buildBranchKeys { + if s, ok := b[k].(string); ok && s != "" { + return strings.TrimPrefix(s, "refs/heads/") + } + } + return "" +} + +// commitMatches compares two commit ids allowing an abbreviated form on either +// side, since builds may record a short SHA while the PR carries the full one. +func commitMatches(a, b string) bool { + a = strings.ToLower(strings.TrimSpace(a)) + b = strings.ToLower(strings.TrimSpace(b)) + if a == "" || b == "" { + return false + } + if a == b { + return true + } + const minPrefix = 7 + if len(a) >= minPrefix && len(b) >= minPrefix { + return strings.HasPrefix(a, b) || strings.HasPrefix(b, a) + } + return false +} + +func firstString(m map[string]interface{}, keys []string) string { + for _, k := range keys { + if s, ok := m[k].(string); ok && s != "" { + return s + } + } + return "" +} + +func firstValue(m map[string]interface{}, keys []string) interface{} { + for _, k := range keys { + if v, ok := m[k]; ok && v != nil { + return v + } + } + return nil +} diff --git a/shortcuts/pr/checks_test.go b/shortcuts/pr/checks_test.go new file mode 100644 index 0000000..f1a9aeb --- /dev/null +++ b/shortcuts/pr/checks_test.go @@ -0,0 +1,314 @@ +package pr + +import ( + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "os" + "testing" + + "github.com/gitlink-org/gitlink-cli/internal/i18n" + "github.com/gitlink-org/gitlink-cli/internal/output" +) + +const ( + checksHeadBranch = "feature/x" + checksHeadSHA = "82861402ada099d3e288fc41680596dde297d022" +) + +type checksEnvelope struct { + OK bool `json:"ok"` + Data checksResult `json:"data"` +} + +// TestPRChecksSelection mocks the single-PR GET followed by the builds GET and +// asserts which build(s) the shortcut links to the PR head under each shape of +// the (undocumented) builds payload. +func TestPRChecksSelection(t *testing.T) { + cases := []struct { + name string + builds []interface{} + wantMatchedBy string + wantIDs []float64 + wantNote bool + }{ + { + name: "sha match wins over branch", + builds: []interface{}{ + map[string]interface{}{"id": float64(1), "branch": checksHeadBranch, "sha": "deadbeef1234567", "status": "failure"}, + map[string]interface{}{"id": float64(2), "branch": checksHeadBranch, "sha": checksHeadSHA, "status": "success", "stage": "build"}, + }, + wantMatchedBy: "sha", + wantIDs: []float64{2}, + }, + { + name: "abbreviated sha still matches", + builds: []interface{}{ + map[string]interface{}{"number": float64(9), "commit_id": checksHeadSHA[:8], "state": "success"}, + }, + wantMatchedBy: "sha", + wantIDs: []float64{9}, + }, + { + name: "branch fallback when no sha field present", + builds: []interface{}{ + map[string]interface{}{"number": float64(7), "ref": "refs/heads/feature/x", "status": "running"}, + map[string]interface{}{"number": float64(8), "ref": "refs/heads/other", "status": "success"}, + }, + wantMatchedBy: "branch", + wantIDs: []float64{7}, + }, + { + name: "unlinkable builds return all with a note", + builds: []interface{}{ + map[string]interface{}{"id": float64(1), "status": "success"}, + map[string]interface{}{"id": float64(2), "status": "failure"}, + }, + wantMatchedBy: "unlinkable", + wantIDs: []float64{1, 2}, + wantNote: true, + }, + { + name: "recognizable builds but none match the head", + builds: []interface{}{ + map[string]interface{}{"id": float64(1), "branch": "other", "sha": "aaaaaaa1111111", "status": "success"}, + }, + wantMatchedBy: "none", + wantIDs: nil, + wantNote: true, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == "GET" && r.URL.Path == "/owner/repo/pulls/42.json": + writeJSON(t, w, map[string]interface{}{ + "id": float64(42), + "status": "open", + "head": checksHeadBranch, + "head_commit_sha": checksHeadSHA, + "issue": map[string]interface{}{"id": float64(100)}, + }) + case r.Method == "GET" && r.URL.Path == "/owner/repo/builds.json": + writeJSON(t, w, tc.builds) + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } + })) + defer server.Close() + + out, err := captureStdout(t, func() error { + return runPRShortcut(t, server, "checks", map[string]string{"id": "42"}) + }) + if err != nil { + t.Fatalf("checks failed: %v", err) + } + + result := decodeChecksEnvelope(t, out).Data + if result.MatchedBy != tc.wantMatchedBy { + t.Fatalf("matched_by = %q, want %q", result.MatchedBy, tc.wantMatchedBy) + } + if result.HeadBranch != checksHeadBranch || result.HeadSHA != checksHeadSHA { + t.Fatalf("head = %q/%q, want %q/%q", result.HeadBranch, result.HeadSHA, checksHeadBranch, checksHeadSHA) + } + if result.TotalBuilds != len(tc.builds) { + t.Fatalf("total_builds = %d, want %d", result.TotalBuilds, len(tc.builds)) + } + assertBuildIDs(t, result.Builds, tc.wantIDs) + if tc.wantNote && result.Note == "" { + t.Fatalf("expected a note for matched_by=%s", tc.wantMatchedBy) + } + if !tc.wantNote && result.Note != "" { + t.Fatalf("unexpected note: %q", result.Note) + } + }) + } +} + +func TestPRChecksErrorsWhenHeadMissing(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/owner/repo/pulls/42.json" { + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } + writeJSON(t, w, map[string]interface{}{"id": float64(42), "status": "open"}) + })) + defer server.Close() + + _, err := captureStdout(t, func() error { + return runPRShortcut(t, server, "checks", map[string]string{"id": "42"}) + }) + if err == nil { + t.Fatal("expected error when PR response lacks head fields") + } +} + +func TestPRChecksBuildsHTTPError(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/owner/repo/pulls/42.json": + writeJSON(t, w, map[string]interface{}{ + "head": checksHeadBranch, + "head_commit_sha": checksHeadSHA, + }) + default: + w.WriteHeader(http.StatusInternalServerError) + w.Write([]byte("server error")) + } + })) + defer server.Close() + + _, err := captureStdout(t, func() error { + return runPRShortcut(t, server, "checks", map[string]string{"id": "42"}) + }) + if err == nil { + t.Fatal("expected error when builds request fails") + } +} + +func TestExtractPullRequestHead(t *testing.T) { + cases := []struct { + name string + data interface{} + wantBranch string + wantSHA string + wantErr bool + }{ + { + name: "top level fields", + data: map[string]interface{}{"head": "feature/x", "head_commit_sha": "abc123def4567"}, + wantBranch: "feature/x", + wantSHA: "abc123def4567", + }, + { + name: "nested pull_request wrapper", + data: map[string]interface{}{"pull_request": map[string]interface{}{"head": "feature/y", "head_commit_sha": "def456"}}, + wantBranch: "feature/y", + wantSHA: "def456", + }, + { + name: "missing head fields", + data: map[string]interface{}{"id": float64(1)}, + wantErr: true, + }, + { + name: "not a map", + data: "raw", + wantErr: true, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + branch, sha, err := extractPullRequestHead(&output.Envelope{Data: tc.data}) + if tc.wantErr { + if err == nil { + t.Fatal("expected error, got nil") + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if branch != tc.wantBranch || sha != tc.wantSHA { + t.Fatalf("= %q/%q, want %q/%q", branch, sha, tc.wantBranch, tc.wantSHA) + } + }) + } +} + +func TestBuildsFromEnvelope(t *testing.T) { + cases := []struct { + name string + data interface{} + want int + }{ + {name: "raw json string array (client array quirk)", data: `[{"id":1},{"id":2}]`, want: 2}, + {name: "already parsed array", data: []interface{}{map[string]interface{}{"id": float64(1)}}, want: 1}, + {name: "wrapped under builds key", data: map[string]interface{}{"builds": []interface{}{map[string]interface{}{"id": float64(1)}}}, want: 1}, + {name: "non json string", data: "not json", want: 0}, + {name: "unrelated map", data: map[string]interface{}{"message": "ok"}, want: 0}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := buildsFromEnvelope(&output.Envelope{Data: tc.data}) + if len(got) != tc.want { + t.Fatalf("len = %d, want %d", len(got), tc.want) + } + }) + } +} + +func TestCommitMatches(t *testing.T) { + cases := []struct { + a, b string + want bool + }{ + {"82861402ada099d3e288fc41680596dde297d022", "82861402ada099d3e288fc41680596dde297d022", true}, + {"82861402ada099d3e288fc41680596dde297d022", "8286140", true}, + {"8286140", "82861402ada099d3e288fc41680596dde297d022", true}, + {"82861402", "deadbeef", false}, + {"abc", "abc123", false}, + {"", "abc1234", false}, + } + for _, tc := range cases { + if got := commitMatches(tc.a, tc.b); got != tc.want { + t.Fatalf("commitMatches(%q,%q) = %v, want %v", tc.a, tc.b, got, tc.want) + } + } +} + +func TestSelectPullRequestChecksBranchOnlyPR(t *testing.T) { + // A PR with only a head branch (no SHA) still links via branch. + builds := []map[string]interface{}{ + {"id": float64(1), "branch": "feature/x", "status": "success"}, + } + res := selectPullRequestChecks(i18n.Default(), "42", "feature/x", "", builds) + if res.MatchedBy != "branch" || len(res.Builds) != 1 { + t.Fatalf("matched_by=%q builds=%d, want branch/1", res.MatchedBy, len(res.Builds)) + } +} + +func captureStdout(t *testing.T, fn func() error) (string, error) { + t.Helper() + orig := os.Stdout + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("pipe: %v", err) + } + os.Stdout = w + runErr := fn() + w.Close() + os.Stdout = orig + data, readErr := io.ReadAll(r) + if readErr != nil { + t.Fatalf("read captured output: %v", readErr) + } + return string(data), runErr +} + +func decodeChecksEnvelope(t *testing.T, out string) checksEnvelope { + t.Helper() + var env checksEnvelope + if err := json.Unmarshal([]byte(out), &env); err != nil { + t.Fatalf("decode output %q: %v", out, err) + } + return env +} + +func assertBuildIDs(t *testing.T, builds []checkBuild, want []float64) { + t.Helper() + if len(builds) != len(want) { + t.Fatalf("got %d builds, want %d (%v)", len(builds), len(want), want) + } + for i, b := range builds { + got, ok := b.ID.(float64) + if !ok { + t.Fatalf("build[%d].ID = %v (%T), want float64", i, b.ID, b.ID) + } + if got != want[i] { + t.Fatalf("build[%d].ID = %v, want %v", i, got, want[i]) + } + } +} diff --git a/shortcuts/pr/pr.go b/shortcuts/pr/pr.go index 4983f1b..76dc33a 100644 --- a/shortcuts/pr/pr.go +++ b/shortcuts/pr/pr.go @@ -1,24 +1,52 @@ package pr import ( - "encoding/base64" "fmt" "net/url" "strings" + "github.com/gitlink-org/gitlink-cli/internal/i18n" "github.com/gitlink-org/gitlink-cli/internal/output" "github.com/gitlink-org/gitlink-cli/shortcuts/common" ) -func Shortcuts() []*common.Shortcut { +func v1RepoPath(ctx *common.RuntimeContext) string { + return fmt.Sprintf("/v1/%s/%s", ctx.Owner, ctx.Repo) +} + +func normalizePullRequestListState(state string) string { + switch strings.ToLower(strings.TrimSpace(state)) { + case "open", "opened": + return "0" + case "merged": + return "1" + case "closed": + return "2" + case "all", "": + return "" + default: + return state + } +} + +func Shortcuts(translators ...*i18n.Translator) []*common.Shortcut { + tr := shortcutTranslator(translators...) return []*common.Shortcut{ { Name: "list", - Description: "List pull requests", + Description: tr.T("cmd.pr.list.short"), Flags: []common.Flag{ - {Name: "state", Short: "s", Usage: "Filter: open, merged, closed", Default: "open"}, - {Name: "page", Short: "p", Usage: "Page number", Default: "1"}, - {Name: "limit", Short: "l", Usage: "Items per page", Default: "20"}, + {Name: "state", Short: "s", Usage: tr.T("flag.pr.state"), Default: "open"}, + {Name: "keyword", Short: "k", Usage: tr.T("flag.search.keyword")}, + {Name: "priority-id", Usage: tr.T("flag.pr.priority_id")}, + {Name: "tag-id", Usage: tr.T("flag.pr.tag_id")}, + {Name: "milestone-id", Usage: tr.T("flag.pr.milestone_id")}, + {Name: "reviewer-id", Usage: tr.T("flag.pr.reviewer_id")}, + {Name: "assignee-id", Usage: tr.T("flag.pr.assignee_id")}, + {Name: "sort-by", Usage: tr.T("flag.sort_by")}, + {Name: "sort-direction", Usage: tr.T("flag.sort_direction")}, + {Name: "page", Short: "p", Usage: tr.T("flag.page"), Default: "1"}, + {Name: "limit", Short: "l", Usage: tr.T("flag.limit"), Default: "20"}, }, Run: func(ctx *common.RuntimeContext) error { if err := ctx.ResolveOwnerRepo(); err != nil { @@ -27,10 +55,34 @@ func Shortcuts() []*common.Shortcut { q := url.Values{} q.Set("page", ctx.Arg("page")) q.Set("limit", ctx.Arg("limit")) - if s := ctx.Arg("state"); s != "" { - q.Set("state", s) + if s := normalizePullRequestListState(ctx.Arg("state")); s != "" { + q.Set("status", s) } - env, err := ctx.CallAPIWithQuery("GET", ctx.RepoPath()+"/pulls", q) + if keyword := ctx.Arg("keyword"); keyword != "" { + q.Set("keyword", keyword) + } + if priorityID := ctx.Arg("priority-id"); priorityID != "" { + q.Set("priority_id", priorityID) + } + if tagID := ctx.Arg("tag-id"); tagID != "" { + q.Set("issue_tag_id", tagID) + } + if milestoneID := ctx.Arg("milestone-id"); milestoneID != "" { + q.Set("version_id", milestoneID) + } + if reviewerID := ctx.Arg("reviewer-id"); reviewerID != "" { + q.Set("reviewer_id", reviewerID) + } + if assigneeID := ctx.Arg("assignee-id"); assigneeID != "" { + q.Set("assign_user_id", assigneeID) + } + if sortBy := ctx.Arg("sort-by"); sortBy != "" { + q.Set("sort_by", sortBy) + } + if sortDirection := ctx.Arg("sort-direction"); sortDirection != "" { + q.Set("sort_direction", sortDirection) + } + env, err := ctx.CallAPIWithQuery("GET", v1RepoPath(ctx)+"/pulls", q) if err != nil { return err } @@ -39,12 +91,12 @@ func Shortcuts() []*common.Shortcut { }, { Name: "create", - Description: "Create a pull request", + Description: tr.T("cmd.pr.create.short"), Flags: []common.Flag{ - {Name: "title", Short: "t", Usage: "PR title", Required: true}, - {Name: "body", Short: "b", Usage: "PR description"}, - {Name: "head", Usage: "Source branch", Required: true}, - {Name: "base", Usage: "Target branch", Default: "master"}, + {Name: "title", Short: "t", Usage: tr.T("flag.pr.title"), Required: true}, + {Name: "body", Short: "b", Usage: tr.T("flag.pr.body")}, + {Name: "head", Usage: tr.T("flag.pr.head"), Required: true}, + {Name: "base", Usage: tr.T("flag.pr.base"), Default: "master"}, }, Run: func(ctx *common.RuntimeContext) error { if err := ctx.ResolveOwnerRepo(); err != nil { @@ -56,9 +108,13 @@ func Shortcuts() []*common.Shortcut { if base == "" { base = "master" } - payload, err := buildCreatePRPayload(ctx, title, head, base, ctx.Arg("body")) - if err != nil { - return err + payload := map[string]interface{}{ + "title": title, + "head": head, + "base": base, + } + if b := ctx.Arg("body"); b != "" { + payload["body"] = b } env, err := ctx.CallAPI("POST", ctx.RepoPath()+"/pulls", payload) if err != nil { @@ -69,9 +125,9 @@ func Shortcuts() []*common.Shortcut { }, { Name: "view", - Description: "View pull request details", + Description: tr.T("cmd.pr.view.short"), Flags: []common.Flag{ - {Name: "id", Short: "i", Usage: "PR number", Required: true}, + {Name: "id", Short: "i", Usage: tr.T("flag.pr.id"), Required: true}, }, Run: func(ctx *common.RuntimeContext) error { if err := ctx.ResolveOwnerRepo(); err != nil { @@ -90,10 +146,10 @@ func Shortcuts() []*common.Shortcut { }, { Name: "merge", - Description: "Merge a pull request", + Description: tr.T("cmd.pr.merge.short"), Flags: []common.Flag{ - {Name: "id", Short: "i", Usage: "PR number", Required: true}, - {Name: "method", Short: "m", Usage: "Merge method: merge, rebase, squash", Default: "merge"}, + {Name: "id", Short: "i", Usage: tr.T("flag.pr.id"), Required: true}, + {Name: "method", Short: "m", Usage: tr.T("flag.pr.merge_method"), Default: "merge"}, }, Run: func(ctx *common.RuntimeContext) error { if err := ctx.ResolveOwnerRepo(); err != nil { @@ -115,10 +171,10 @@ func Shortcuts() []*common.Shortcut { }, }, { - Name: "close", - Description: "Close a pull request", + Name: "refuse", + Description: "Refuse and close a pull request", Flags: []common.Flag{ - {Name: "id", Short: "i", Usage: "PR number", Required: true}, + {Name: "id", Short: "i", Usage: tr.T("flag.pr.id"), Required: true}, }, Run: func(ctx *common.RuntimeContext) error { if err := ctx.ResolveOwnerRepo(); err != nil { @@ -155,9 +211,9 @@ func Shortcuts() []*common.Shortcut { }, { Name: "files", - Description: "List changed files in a pull request", + Description: tr.T("cmd.pr.files.short"), Flags: []common.Flag{ - {Name: "id", Short: "i", Usage: "PR number", Required: true}, + {Name: "id", Short: "i", Usage: tr.T("flag.pr.id"), Required: true}, }, Run: func(ctx *common.RuntimeContext) error { if err := ctx.ResolveOwnerRepo(); err != nil { @@ -173,9 +229,9 @@ func Shortcuts() []*common.Shortcut { }, { Name: "diff", - Description: "Show diff for a pull request", + Description: tr.T("cmd.pr.diff.short"), Flags: []common.Flag{ - {Name: "id", Short: "i", Usage: "PR number", Required: true}, + {Name: "id", Short: "i", Usage: tr.T("flag.pr.id"), Required: true}, }, Run: func(ctx *common.RuntimeContext) error { if err := ctx.ResolveOwnerRepo(); err != nil { @@ -191,9 +247,9 @@ func Shortcuts() []*common.Shortcut { }, { Name: "versions", - Description: "List pull request patchset versions", + Description: tr.T("cmd.pr.versions.short"), Flags: []common.Flag{ - {Name: "id", Short: "i", Usage: "PR number", Required: true}, + {Name: "id", Short: "i", Usage: tr.T("flag.pr.id"), Required: true}, }, Run: func(ctx *common.RuntimeContext) error { if err := ctx.ResolveOwnerRepo(); err != nil { @@ -212,11 +268,11 @@ func Shortcuts() []*common.Shortcut { }, { Name: "version-diff", - Description: "Show diff for a pull request patchset version", + Description: tr.T("cmd.pr.version_diff.short"), Flags: []common.Flag{ - {Name: "id", Short: "i", Usage: "PR number", Required: true}, - {Name: "version-id", Short: "v", Usage: "Patchset version ID", Required: true}, - {Name: "file", Short: "f", Usage: "Filter diff by file path"}, + {Name: "id", Short: "i", Usage: tr.T("flag.pr.id"), Required: true}, + {Name: "version-id", Short: "v", Usage: tr.T("flag.pr.version_id"), Required: true}, + {Name: "file", Short: "f", Usage: tr.T("flag.pr.file")}, }, Run: func(ctx *common.RuntimeContext) error { if err := ctx.ResolveOwnerRepo(); err != nil { @@ -249,10 +305,10 @@ func Shortcuts() []*common.Shortcut { }, { Name: "reviews", - Description: "List pull request reviews", + Description: tr.T("cmd.pr.reviews.short"), Flags: []common.Flag{ - {Name: "id", Short: "i", Usage: "PR number", Required: true}, - {Name: "status", Short: "s", Usage: "Filter review status: common, approved, rejected"}, + {Name: "id", Short: "i", Usage: tr.T("flag.pr.id"), Required: true}, + {Name: "status", Short: "s", Usage: tr.T("flag.pr.review_status_filter")}, }, Run: func(ctx *common.RuntimeContext) error { if err := ctx.ResolveOwnerRepo(); err != nil { @@ -278,13 +334,13 @@ func Shortcuts() []*common.Shortcut { }, { Name: "review", - Description: "Create a pull request review", + Description: tr.T("cmd.pr.review.short"), Flags: []common.Flag{ - {Name: "id", Short: "i", Usage: "PR number", Required: true}, - {Name: "status", Short: "s", Usage: "Review status: common, approved, rejected", Default: "common"}, - {Name: "content", Short: "c", Usage: "Review content", Required: true}, - {Name: "commit", Short: "m", Usage: "Commit SHA to attach the review to"}, - {Name: "dry-run", Usage: "Preview the review request without creating it", Bool: true, Default: "false"}, + {Name: "id", Short: "i", Usage: tr.T("flag.pr.id"), Required: true}, + {Name: "status", Short: "s", Usage: tr.T("flag.pr.review_status"), Default: "common"}, + {Name: "content", Short: "c", Usage: tr.T("flag.pr.review_content"), Required: true}, + {Name: "commit", Short: "m", Usage: tr.T("flag.pr.review_commit")}, + {Name: "dry-run", Usage: tr.T("flag.dry_run"), Bool: true, Default: "false"}, }, Run: func(ctx *common.RuntimeContext) error { if err := ctx.ResolveOwnerRepo(); err != nil { @@ -325,15 +381,29 @@ func Shortcuts() []*common.Shortcut { if err != nil { return err } + + // Also post a journal comment so the review is visible in the PR conversation. + prEnv, journalErr := ctx.CallAPI("GET", fmt.Sprintf("%s/pulls/%s", ctx.RepoPath(), id), nil) + if journalErr == nil { + if issueID, extractErr := extractIssueID(prEnv); extractErr == nil { + statusLabel := map[string]string{ + "approved": "approved", "rejected": "rejected", "common": "commented", + }[status] + summary := fmt.Sprintf("## Review: %s\n\n%s", statusLabel, content) + ctx.CallAPI("POST", fmt.Sprintf("/v1/%s/%s/issues/%d/journals", ctx.Owner, ctx.Repo, issueID), + map[string]interface{}{"notes": summary}) + } + } + return ctx.Output(env) }, }, { Name: "comment", - Description: "Add a comment to a pull request", + Description: tr.T("cmd.pr.comment.short"), Flags: []common.Flag{ - {Name: "id", Short: "i", Usage: "PR number", Required: true}, - {Name: "body", Short: "b", Usage: "Comment body", Required: true}, + {Name: "id", Short: "i", Usage: tr.T("flag.pr.id"), Required: true}, + {Name: "body", Short: "b", Usage: tr.T("flag.comment.body"), Required: true}, }, Run: func(ctx *common.RuntimeContext) error { if err := ctx.ResolveOwnerRepo(); err != nil { @@ -362,10 +432,11 @@ func Shortcuts() []*common.Shortcut { }, }, { - Name: "commits", - Description: "List commits in a pull request", + Name: "checks", + Description: tr.T("cmd.pr.checks.short"), + Long: tr.T("cmd.pr.checks.long"), Flags: []common.Flag{ - {Name: "id", Short: "i", Usage: "PR number", Required: true}, + {Name: "id", Short: "i", Usage: tr.T("flag.pr.id"), Required: true}, }, Run: func(ctx *common.RuntimeContext) error { if err := ctx.ResolveOwnerRepo(); err != nil { @@ -375,61 +446,36 @@ func Shortcuts() []*common.Shortcut { if err != nil { return err } - env, err := ctx.CallAPI("GET", prV1Path(ctx, id)+"/commits", nil) + prEnv, err := ctx.CallAPI("GET", fmt.Sprintf("%s/pulls/%s", ctx.RepoPath(), id), nil) if err != nil { return err } - return ctx.Output(env) - }, - }, - { - Name: "branches", - Description: "List branches for pull request creation", - Flags: []common.Flag{}, - Run: func(ctx *common.RuntimeContext) error { - if err := ctx.ResolveOwnerRepo(); err != nil { - return err - } - env, err := ctx.CallAPI("GET", ctx.RepoPath()+"/pulls/get_branches", nil) + headBranch, headSHA, err := extractPullRequestHead(prEnv) if err != nil { return err } - return ctx.Output(env) - }, - }, - { - Name: "check-merge", - Description: "Check if two branches can be merged", - Flags: []common.Flag{ - {Name: "head", Usage: "Source branch", Required: true}, - {Name: "base", Usage: "Target branch", Required: true}, - }, - Run: func(ctx *common.RuntimeContext) error { - if err := ctx.ResolveOwnerRepo(); err != nil { - return err - } - head, err := ctx.RequireArg("head") + buildsEnv, err := ctx.CallAPI("GET", ctx.RepoPath()+"/builds", nil) if err != nil { return err } - base, err := ctx.RequireArg("base") - if err != nil { - return err + tr := ctx.Tr + if tr == nil { + tr = i18n.Default() } - payload := map[string]interface{}{ - "head": head, - "base": base, - } - env, err := ctx.CallAPI("POST", ctx.RepoPath()+"/pulls/check_can_merge", payload) - if err != nil { - return err - } - return ctx.Output(env) + result := selectPullRequestChecks(tr, id, headBranch, headSHA, buildsFromEnvelope(buildsEnv)) + return ctx.OutputData(result) }, }, } } +func shortcutTranslator(translators ...*i18n.Translator) *i18n.Translator { + if len(translators) > 0 && translators[0] != nil { + return translators[0] + } + return i18n.Default() +} + func prV1Path(ctx *common.RuntimeContext, id string) string { return fmt.Sprintf("/v1/%s/%s/pulls/%s", ctx.Owner, ctx.Repo, id) } @@ -548,146 +594,3 @@ func numberField(m map[string]interface{}, key string) (float64, bool) { return 0, false } } - -type prHeadSpec struct { - Branch string - ForkOwner string - ForkRepo string - IsFork bool - CompareHead string -} - -func buildCreatePRPayload(ctx *common.RuntimeContext, title, head, base, body string) (map[string]interface{}, error) { - spec, err := parsePRHead(head) - if err != nil { - return nil, err - } - - payload := map[string]interface{}{ - "title": title, - "head": spec.Branch, - "base": base, - "assigned_to_id": "", - "fixed_version_id": "", - "issue_tag_ids": []string{}, - "reviewer_ids": []string{}, - "receivers_login": []string{}, - "priority_id": "2", - "is_original": spec.IsFork, - } - if body != "" { - payload["body"] = body - } - - if spec.IsFork { - repoInfo, err := fetchProjectInfo(ctx, spec.ForkOwner, spec.ForkRepo) - if err != nil { - return nil, err - } - projectID, err := extractFloatField(repoInfo, "project_id", "id") - if err != nil { - return nil, fmt.Errorf("resolve fork project id: %w", err) - } - identifier, err := extractStringField(repoInfo, "project_identifier", "identifier") - if err != nil { - return nil, fmt.Errorf("resolve fork project identifier: %w", err) - } - payload["merge_user_login"] = spec.ForkOwner - payload["merge_project_identifier"] = identifier - payload["fork_project_id"] = int(projectID) - } - - compareCounts, err := fetchPRCompareCounts(ctx, spec.CompareHead, base) - if err == nil { - if commits, ok := compareCounts["commits_count"]; ok { - payload["commits_count"] = commits - } - if files, ok := compareCounts["files_count"]; ok { - payload["files_count"] = files - } - } - - return payload, nil -} - -func parsePRHead(head string) (*prHeadSpec, error) { - if head == "" { - return nil, fmt.Errorf("source branch cannot be empty") - } - if !strings.Contains(head, ":") { - return &prHeadSpec{ - Branch: head, - IsFork: false, - CompareHead: head, - }, nil - } - - parts := strings.SplitN(head, ":", 2) - if len(parts) != 2 || parts[0] == "" || parts[1] == "" { - return nil, fmt.Errorf("invalid --head %q, expected owner/repo:branch", head) - } - - repoParts := strings.Split(parts[0], "/") - if len(repoParts) != 2 || repoParts[0] == "" || repoParts[1] == "" { - return nil, fmt.Errorf("invalid --head %q, expected owner/repo:branch", head) - } - - return &prHeadSpec{ - Branch: parts[1], - ForkOwner: repoParts[0], - ForkRepo: repoParts[1], - IsFork: true, - CompareHead: repoParts[0] + ":" + parts[1], - }, nil -} - -func fetchProjectInfo(ctx *common.RuntimeContext, owner, repo string) (map[string]interface{}, error) { - env, err := ctx.CallAPI("GET", fmt.Sprintf("/%s/%s", owner, repo), nil) - if err != nil { - return nil, fmt.Errorf("fetch project info: %w", err) - } - data, ok := env.Data.(map[string]interface{}) - if !ok { - return nil, fmt.Errorf("unexpected project info response format") - } - return data, nil -} - -func fetchPRCompareCounts(ctx *common.RuntimeContext, head, base string) (map[string]int, error) { - encodedHead := base64.RawURLEncoding.EncodeToString([]byte(head)) - encodedBase := base64.RawURLEncoding.EncodeToString([]byte(base)) - env, err := ctx.CallAPI("GET", fmt.Sprintf("%s/compare/%s...%s", ctx.RepoPath(), encodedHead, encodedBase), nil) - if err != nil { - return nil, err - } - data, ok := env.Data.(map[string]interface{}) - if !ok { - return nil, fmt.Errorf("unexpected compare response format") - } - result := map[string]int{} - if v, ok := data["commits_count"].(float64); ok { - result["commits_count"] = int(v) - } - if v, ok := data["files_count"].(float64); ok { - result["files_count"] = int(v) - } - return result, nil -} - -func extractFloatField(data map[string]interface{}, keys ...string) (float64, error) { - for _, key := range keys { - if v, ok := data[key].(float64); ok { - return v, nil - } - } - return 0, fmt.Errorf("missing numeric field %v", keys) -} - -func extractStringField(data map[string]interface{}, keys ...string) (string, error) { - for _, key := range keys { - if v, ok := data[key].(string); ok && v != "" { - return v, nil - } - } - return "", fmt.Errorf("missing string field %v", keys) -}