feat(pr): 新增审查评论管理快捷命令

This commit is contained in:
Mengz 2026-06-09 21:46:01 +08:00
parent 71ca2bb683
commit e4ed2d1b45
5 changed files with 511 additions and 0 deletions

View File

@ -432,6 +432,16 @@ gitlink-cli pr +reviews --owner Gitlink --repo forgeplus -i 42
# Create a PR review (with dry-run preview)
gitlink-cli pr +review --owner Gitlink --repo forgeplus -i 42 --status approved -c "LGTM" --dry-run
gitlink-cli pr +review --owner Gitlink --repo forgeplus -i 42 --status approved -c "LGTM"
# List review comments and unresolved discussion threads
gitlink-cli pr +review-comments --owner Gitlink --repo forgeplus -i 42 --state opened --need-respond true --full
# Create a line-level review comment or reply
gitlink-cli pr +review-comment --owner Gitlink --repo forgeplus -i 42 -b "Please handle this edge case" --type problem --review-id 7 --line-code abc_1_2 --commit deadbeef --path main.go --dry-run
# Resolve, edit, or delete a review comment
gitlink-cli pr +review-comment-update --owner Gitlink --repo forgeplus -i 42 --comment-id 99 --state resolved
gitlink-cli pr +review-comment-delete --owner Gitlink --repo forgeplus -i 42 --comment-id 99
```
### Branch Management

View File

@ -442,6 +442,16 @@ gitlink-cli pr +reviews --owner Gitlink --repo forgeplus -i 42
# 创建 PR 审查(支持 dry-run 预览)
gitlink-cli pr +review --owner Gitlink --repo forgeplus -i 42 --status approved -c "LGTM" --dry-run
gitlink-cli pr +review --owner Gitlink --repo forgeplus -i 42 --status approved -c "LGTM"
# 查看行级审查评论和未解决讨论
gitlink-cli pr +review-comments --owner Gitlink --repo forgeplus -i 42 --state opened --need-respond true --full
# 创建行级审查评论或回复
gitlink-cli pr +review-comment --owner Gitlink --repo forgeplus -i 42 -b "请处理这个边界情况" --type problem --review-id 7 --line-code abc_1_2 --commit deadbeef --path main.go --dry-run
# 解决、编辑或删除审查评论
gitlink-cli pr +review-comment-update --owner Gitlink --repo forgeplus -i 42 --comment-id 99 --state resolved
gitlink-cli pr +review-comment-delete --owner Gitlink --repo forgeplus -i 42 --comment-id 99
```
### 发布管理

View File

@ -0,0 +1,25 @@
# Pull request review comment management shortcuts
This change adds first-class shortcuts for GitLink pull request review comment
threads. It complements `pr +review`, which creates an overall review decision,
with commands for line-level and threaded discussion records.
- `pr +review-comments` lists review comments with keyword, review ID,
need-response, state, parent, path, full-thread, and sorting filters.
- `pr +review-comment` creates review comments or replies and supports
`comment`/`problem` types, review IDs, line codes, commit IDs, paths,
parent IDs, raw diff JSON, and dry-run previews.
- `pr +review-comment-update` edits the note, commit, or state (`opened`,
`resolved`, `disabled`) with dry-run support.
- `pr +review-comment-delete` deletes a review comment by ID.
The implementation validates numeric IDs, boolean filters, comment type, state,
and diff JSON before sending API requests.
Verification:
- `go test ./shortcuts/pr`
- `go test ./shortcuts`
- `go test ./...`
- `go build ./...`
- `git diff --check`

View File

@ -1,8 +1,10 @@
package pr
import (
"encoding/json"
"fmt"
"net/url"
"strconv"
"strings"
"github.com/gitlink-org/gitlink-cli/internal/i18n"
@ -431,6 +433,62 @@ func Shortcuts(translators ...*i18n.Translator) []*common.Shortcut {
return ctx.Output(env)
},
},
{
Name: "review-comments",
Description: "List pull request review comments and discussion threads",
Flags: []common.Flag{
{Name: "id", Short: "i", Usage: tr.T("flag.pr.id"), Required: true},
{Name: "keyword", Short: "k", Usage: "Search review comment content"},
{Name: "review-id", Usage: "Filter by review ID"},
{Name: "need-respond", Usage: "Filter comments that need response: true or false"},
{Name: "state", Short: "s", Usage: "Filter by opened, resolved, or disabled"},
{Name: "parent-id", Usage: "Filter replies under a parent comment ID"},
{Name: "path", Usage: "Filter by file path"},
{Name: "full", Usage: "Include replies in the response", Bool: true, Default: "false"},
{Name: "sort-by", Usage: "Sort field: created_on or updated_on"},
{Name: "sort-direction", Usage: "Sort direction: asc or desc"},
},
Run: runPRReviewComments,
},
{
Name: "review-comment",
Description: "Create a pull request review comment or reply",
Flags: []common.Flag{
{Name: "id", Short: "i", Usage: tr.T("flag.pr.id"), Required: true},
{Name: "body", Short: "b", Usage: tr.T("flag.comment.body"), Required: true},
{Name: "type", Usage: "Comment type: comment or problem", Default: "comment"},
{Name: "review-id", Usage: "Review ID"},
{Name: "line-code", Usage: "GitLink diff line code"},
{Name: "commit", Usage: "Commit SHA for the commented diff"},
{Name: "path", Usage: "Commented file path"},
{Name: "parent-id", Usage: "Parent review comment ID for a reply"},
{Name: "diff-json", Usage: "Raw diff JSON object for line comments"},
{Name: "dry-run", Usage: tr.T("flag.dry_run"), Bool: true, Default: "false"},
},
Run: runPRReviewCommentCreate,
},
{
Name: "review-comment-update",
Description: "Update a pull request review comment note, commit, or state",
Flags: []common.Flag{
{Name: "id", Short: "i", Usage: tr.T("flag.pr.id"), Required: true},
{Name: "comment-id", Usage: "Review comment ID", Required: true},
{Name: "body", Short: "b", Usage: tr.T("flag.comment.body")},
{Name: "commit", Usage: "Commit SHA"},
{Name: "state", Short: "s", Usage: "New state: opened, resolved, or disabled"},
{Name: "dry-run", Usage: tr.T("flag.dry_run"), Bool: true, Default: "false"},
},
Run: runPRReviewCommentUpdate,
},
{
Name: "review-comment-delete",
Description: "Delete a pull request review comment",
Flags: []common.Flag{
{Name: "id", Short: "i", Usage: tr.T("flag.pr.id"), Required: true},
{Name: "comment-id", Usage: "Review comment ID", Required: true},
},
Run: runPRReviewCommentDelete,
},
}
}
@ -445,6 +503,265 @@ func prV1Path(ctx *common.RuntimeContext, id string) string {
return fmt.Sprintf("/v1/%s/%s/pulls/%s", ctx.Owner, ctx.Repo, id)
}
func prReviewCommentPath(ctx *common.RuntimeContext, prID string) string {
return fmt.Sprintf("%s/journals", prV1Path(ctx, url.PathEscape(prID)))
}
func prReviewCommentItemPath(ctx *common.RuntimeContext, prID, commentID string) string {
return fmt.Sprintf("%s/%s", prReviewCommentPath(ctx, prID), url.PathEscape(commentID))
}
func runPRReviewComments(ctx *common.RuntimeContext) error {
if err := ctx.ResolveOwnerRepo(); err != nil {
return err
}
id, err := ctx.RequireArg("id")
if err != nil {
return err
}
q := url.Values{}
setPRQueryIfPresent(q, "keyword", ctx.Arg("keyword"))
if reviewID := ctx.Arg("review-id"); reviewID != "" {
if _, err := parsePRPositiveID(reviewID, "review-id"); err != nil {
return err
}
q.Set("review_id", strings.TrimSpace(reviewID))
}
if needRespond := ctx.Arg("need-respond"); needRespond != "" {
if err := validatePRBoolString("need-respond", needRespond); err != nil {
return err
}
q.Set("need_respond", strings.ToLower(strings.TrimSpace(needRespond)))
}
if state := ctx.Arg("state"); state != "" {
if err := validatePRReviewCommentState(state); err != nil {
return err
}
q.Set("state", strings.TrimSpace(state))
}
if parentID := ctx.Arg("parent-id"); parentID != "" {
if _, err := parsePRPositiveID(parentID, "parent-id"); err != nil {
return err
}
q.Set("parent_id", strings.TrimSpace(parentID))
}
setPRQueryIfPresent(q, "path", ctx.Arg("path"))
if ctx.Arg("full") == "true" {
q.Set("is_full", "true")
}
setPRQueryIfPresent(q, "sort_by", ctx.Arg("sort-by"))
setPRQueryIfPresent(q, "sort_direction", ctx.Arg("sort-direction"))
env, err := ctx.CallAPIWithQuery("GET", prReviewCommentPath(ctx, id), q)
if err != nil {
return err
}
return ctx.Output(env)
}
func runPRReviewCommentCreate(ctx *common.RuntimeContext) error {
if err := ctx.ResolveOwnerRepo(); err != nil {
return err
}
id, err := ctx.RequireArg("id")
if err != nil {
return err
}
body, err := ctx.RequireArg("body")
if err != nil {
return err
}
payload, err := prReviewCommentCreatePayload(ctx, body)
if err != nil {
return err
}
if ctx.Arg("dry-run") == "true" {
return ctx.OutputData(map[string]interface{}{
"repository": fmt.Sprintf("%s/%s", ctx.Owner, ctx.Repo),
"pull_request": id,
"dry_run": true,
"action": "create_review_comment",
"payload": payload,
})
}
env, err := ctx.CallAPI("POST", prReviewCommentPath(ctx, id), payload)
if err != nil {
return err
}
return ctx.Output(env)
}
func runPRReviewCommentUpdate(ctx *common.RuntimeContext) error {
if err := ctx.ResolveOwnerRepo(); err != nil {
return err
}
prID, commentID, err := prReviewCommentTarget(ctx)
if err != nil {
return err
}
payload, err := prReviewCommentUpdatePayload(ctx)
if err != nil {
return err
}
if ctx.Arg("dry-run") == "true" {
return ctx.OutputData(map[string]interface{}{
"repository": fmt.Sprintf("%s/%s", ctx.Owner, ctx.Repo),
"pull_request": prID,
"review_comment": commentID,
"dry_run": true,
"action": "update_review_comment",
"payload": payload,
})
}
env, err := ctx.CallAPI("PUT", prReviewCommentItemPath(ctx, prID, commentID), payload)
if err != nil {
return err
}
return ctx.Output(env)
}
func runPRReviewCommentDelete(ctx *common.RuntimeContext) error {
if err := ctx.ResolveOwnerRepo(); err != nil {
return err
}
prID, commentID, err := prReviewCommentTarget(ctx)
if err != nil {
return err
}
env, err := ctx.CallAPI("DELETE", prReviewCommentItemPath(ctx, prID, commentID), nil)
if err != nil {
return err
}
return ctx.Output(env)
}
func prReviewCommentCreatePayload(ctx *common.RuntimeContext, body string) (map[string]interface{}, error) {
commentType := firstPRNonEmpty(ctx.Arg("type"), "comment")
if err := validatePRReviewCommentType(commentType); err != nil {
return nil, err
}
payload := map[string]interface{}{
"type": commentType,
"note": body,
}
for _, field := range []struct {
flag string
key string
}{
{"review-id", "review_id"},
{"parent-id", "parent_id"},
} {
if value := ctx.Arg(field.flag); value != "" {
id, err := parsePRPositiveID(value, field.flag)
if err != nil {
return nil, err
}
payload[field.key] = id
}
}
setPayloadStringIfPresent(payload, "line_code", ctx.Arg("line-code"))
setPayloadStringIfPresent(payload, "commit_id", ctx.Arg("commit"))
setPayloadStringIfPresent(payload, "path", ctx.Arg("path"))
if rawDiff := strings.TrimSpace(ctx.Arg("diff-json")); rawDiff != "" {
var diff map[string]interface{}
if err := json.Unmarshal([]byte(rawDiff), &diff); err != nil {
return nil, fmt.Errorf("invalid --diff-json: %w", err)
}
payload["diff"] = diff
}
return payload, nil
}
func prReviewCommentUpdatePayload(ctx *common.RuntimeContext) (map[string]interface{}, error) {
payload := map[string]interface{}{}
setPayloadStringIfPresent(payload, "note", ctx.Arg("body"))
setPayloadStringIfPresent(payload, "commit_id", ctx.Arg("commit"))
if state := ctx.Arg("state"); state != "" {
if err := validatePRReviewCommentState(state); err != nil {
return nil, err
}
payload["state"] = strings.TrimSpace(state)
}
if len(payload) == 0 {
return nil, fmt.Errorf("at least one of --body, --commit, or --state is required")
}
return payload, nil
}
func prReviewCommentTarget(ctx *common.RuntimeContext) (string, string, error) {
prID, err := ctx.RequireArg("id")
if err != nil {
return "", "", err
}
commentID, err := ctx.RequireArg("comment-id")
if err != nil {
return "", "", err
}
if _, err := parsePRPositiveID(commentID, "comment-id"); err != nil {
return "", "", err
}
return strings.TrimSpace(prID), strings.TrimSpace(commentID), nil
}
func validatePRReviewCommentType(value string) error {
switch strings.TrimSpace(value) {
case "comment", "problem":
return nil
default:
return fmt.Errorf("invalid --type value %q: use comment or problem", value)
}
}
func validatePRReviewCommentState(value string) error {
switch strings.TrimSpace(value) {
case "opened", "resolved", "disabled":
return nil
default:
return fmt.Errorf("invalid --state value %q: use opened, resolved, or disabled", value)
}
}
func validatePRBoolString(flagName, value string) error {
switch strings.ToLower(strings.TrimSpace(value)) {
case "true", "false":
return nil
default:
return fmt.Errorf("invalid --%s value %q: use true or false", flagName, value)
}
}
func parsePRPositiveID(value, flagName string) (int, error) {
trimmed := strings.TrimSpace(value)
if trimmed == "" {
return 0, fmt.Errorf("--%s contains an empty ID", flagName)
}
id, err := strconv.Atoi(trimmed)
if err != nil || id <= 0 {
return 0, fmt.Errorf("--%s must be a positive numeric ID", flagName)
}
return id, nil
}
func setPRQueryIfPresent(q url.Values, key, value string) {
if strings.TrimSpace(value) != "" {
q.Set(key, strings.TrimSpace(value))
}
}
func setPayloadStringIfPresent(payload map[string]interface{}, key, value string) {
if strings.TrimSpace(value) != "" {
payload[key] = strings.TrimSpace(value)
}
}
func firstPRNonEmpty(values ...string) string {
for _, value := range values {
if strings.TrimSpace(value) != "" {
return strings.TrimSpace(value)
}
}
return ""
}
func validatePRReviewStatus(status string) error {
switch status {
case "common", "approved", "rejected":

View File

@ -92,6 +92,155 @@ func TestPRCommentFailsWhenIssueFieldMissing(t *testing.T) {
}
}
func TestPRReviewCommentsListSendsFilters(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method != "GET" || r.URL.Path != "/v1/owner/repo/pulls/13/journals.json" {
t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path)
}
q := r.URL.Query()
assertEqual(t, q.Get("keyword"), "race")
assertEqual(t, q.Get("review_id"), "5")
assertEqual(t, q.Get("need_respond"), "true")
assertEqual(t, q.Get("state"), "opened")
assertEqual(t, q.Get("parent_id"), "7")
assertEqual(t, q.Get("path"), "main.go")
assertEqual(t, q.Get("is_full"), "true")
assertEqual(t, q.Get("sort_by"), "updated_on")
assertEqual(t, q.Get("sort_direction"), "desc")
writeJSON(t, w, map[string]interface{}{
"total_count": float64(1),
"journals": []interface{}{
map[string]interface{}{"id": float64(9), "note": "race"},
},
})
}))
defer server.Close()
err := runPRShortcut(t, server, "review-comments", map[string]string{
"id": "13",
"keyword": "race",
"review-id": "5",
"need-respond": "true",
"state": "opened",
"parent-id": "7",
"path": "main.go",
"full": "true",
"sort-by": "updated_on",
"sort-direction": "desc",
})
if err != nil {
t.Fatalf("review-comments failed: %v", err)
}
}
func TestPRReviewCommentCreateSendsPayload(t *testing.T) {
var payload map[string]interface{}
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method != "POST" || r.URL.Path != "/v1/owner/repo/pulls/13/journals.json" {
t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path)
}
payload = decodeJSON(t, r)
writeJSON(t, w, payload)
}))
defer server.Close()
err := runPRShortcut(t, server, "review-comment", map[string]string{
"id": "13",
"body": "Please handle this edge case",
"type": "problem",
"review-id": "5",
"line-code": "abc_1_2",
"commit": "deadbeef",
"path": "main.go",
"parent-id": "7",
"diff-json": `{"name":"main.go","addition":1}`,
})
if err != nil {
t.Fatalf("review-comment failed: %v", err)
}
assertEqual(t, payload["type"], "problem")
assertEqual(t, payload["note"], "Please handle this edge case")
assertEqual(t, payload["review_id"], float64(5))
assertEqual(t, payload["line_code"], "abc_1_2")
assertEqual(t, payload["commit_id"], "deadbeef")
assertEqual(t, payload["path"], "main.go")
assertEqual(t, payload["parent_id"], float64(7))
diff, ok := payload["diff"].(map[string]interface{})
if !ok {
t.Fatalf("diff = %v, want object", payload["diff"])
}
assertEqual(t, diff["name"], "main.go")
assertEqual(t, diff["addition"], float64(1))
}
func TestPRReviewCommentUpdateSendsPayload(t *testing.T) {
var payload map[string]interface{}
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method != "PUT" || r.URL.Path != "/v1/owner/repo/pulls/13/journals/9.json" {
t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path)
}
payload = decodeJSON(t, r)
writeJSON(t, w, payload)
}))
defer server.Close()
err := runPRShortcut(t, server, "review-comment-update", map[string]string{
"id": "13",
"comment-id": "9",
"body": "Resolved after follow-up",
"commit": "cafebabe",
"state": "resolved",
})
if err != nil {
t.Fatalf("review-comment-update failed: %v", err)
}
assertEqual(t, payload["note"], "Resolved after follow-up")
assertEqual(t, payload["commit_id"], "cafebabe")
assertEqual(t, payload["state"], "resolved")
}
func TestPRReviewCommentDelete(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method != "DELETE" || r.URL.Path != "/v1/owner/repo/pulls/13/journals/9.json" {
t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path)
}
writeJSON(t, w, map[string]interface{}{"status": float64(0), "message": "success"})
}))
defer server.Close()
err := runPRShortcut(t, server, "review-comment-delete", map[string]string{"id": "13", "comment-id": "9"})
if err != nil {
t.Fatalf("review-comment-delete failed: %v", err)
}
}
func TestPRReviewCommentRejectsInvalidArgs(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
t.Fatalf("invalid args should not call API, got %s %s", r.Method, r.URL.Path)
}))
defer server.Close()
cases := []struct {
name string
cmd string
args map[string]string
}{
{name: "bad list state", cmd: "review-comments", args: map[string]string{"id": "13", "state": "done"}},
{name: "bad bool", cmd: "review-comments", args: map[string]string{"id": "13", "need-respond": "maybe"}},
{name: "bad type", cmd: "review-comment", args: map[string]string{"id": "13", "body": "x", "type": "note"}},
{name: "bad diff json", cmd: "review-comment", args: map[string]string{"id": "13", "body": "x", "diff-json": "{"}},
{name: "missing update fields", cmd: "review-comment-update", args: map[string]string{"id": "13", "comment-id": "9"}},
{name: "bad comment id", cmd: "review-comment-delete", args: map[string]string{"id": "13", "comment-id": "abc"}},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
if err := runPRShortcut(t, server, tc.cmd, tc.args); err == nil {
t.Fatal("expected validation error")
}
})
}
}
// --- list ---
func TestPRList(t *testing.T) {