detached 8def6de

feat(cli): replies, ranges, verdicts, edit and delete for merge request notes

commit
8def6de7519f526a5b59085f451ec68bc181c9f3
author
brice · · 2026-10-19 05:43 UTC
parent
b31a354
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
9 files · +619 -40
Unified
UnifiedSplit
M internal/adapters/rpc/convert.go +9 -0
@@ -472,6 +472,15 @@ func watchEventToPB(we ports.WatchEvent) *pb.Event {
472472 case domain.CommentAdded:
473473 out.Time = timestamp(ev.At)
474474 out.Kind = &pb.Event_CommentAdded{CommentAdded: &pb.CommentAdded{Repo: ev.Repo.String(), Number: ev.Number, Comment: commentToPB(ev.Comment)}}
475+ case domain.CommentEdited:
476+ out.Time = timestamp(ev.At)
477+ out.Kind = &pb.Event_CommentEdited{CommentEdited: &pb.CommentEdited{Repo: ev.Repo.String(), Number: ev.Number, Comment: commentToPB(ev.Comment)}}
478+ case domain.CommentDeleted:
479+ out.Time = timestamp(ev.At)
480+ out.Kind = &pb.Event_CommentDeleted{CommentDeleted: &pb.CommentDeleted{Repo: ev.Repo.String(), Number: ev.Number, Comment: commentToPB(ev.Comment)}}
481+ case domain.CommentResolved:
482+ out.Time = timestamp(ev.At)
483+ out.Kind = &pb.Event_CommentResolved{CommentResolved: &pb.CommentResolved{Repo: ev.Repo.String(), Number: ev.Number, Comment: commentToPB(ev.Comment)}}
475484 case domain.RunUpdated:
476485 out.Time = timestamp(ev.At)
477486 out.Kind = runEventToPB(ev)
@@ -472,6 +472,15 @@ func watchEventToPB(we ports.WatchEvent) *pb.Event {
472 case domain.CommentAdded:472 case domain.CommentAdded:
473 out.Time = timestamp(ev.At)473 out.Time = timestamp(ev.At)
474 out.Kind = &pb.Event_CommentAdded{CommentAdded: &pb.CommentAdded{Repo: ev.Repo.String(), Number: ev.Number, Comment: commentToPB(ev.Comment)}}474 out.Kind = &pb.Event_CommentAdded{CommentAdded: &pb.CommentAdded{Repo: ev.Repo.String(), Number: ev.Number, Comment: commentToPB(ev.Comment)}}
475+ case domain.CommentEdited:
476+ out.Time = timestamp(ev.At)
477+ out.Kind = &pb.Event_CommentEdited{CommentEdited: &pb.CommentEdited{Repo: ev.Repo.String(), Number: ev.Number, Comment: commentToPB(ev.Comment)}}
478+ case domain.CommentDeleted:
479+ out.Time = timestamp(ev.At)
480+ out.Kind = &pb.Event_CommentDeleted{CommentDeleted: &pb.CommentDeleted{Repo: ev.Repo.String(), Number: ev.Number, Comment: commentToPB(ev.Comment)}}
481+ case domain.CommentResolved:
482+ out.Time = timestamp(ev.At)
483+ out.Kind = &pb.Event_CommentResolved{CommentResolved: &pb.CommentResolved{Repo: ev.Repo.String(), Number: ev.Number, Comment: commentToPB(ev.Comment)}}
475 case domain.RunUpdated:484 case domain.RunUpdated:
476 out.Time = timestamp(ev.At)485 out.Time = timestamp(ev.At)
477 out.Kind = runEventToPB(ev)486 out.Kind = runEventToPB(ev)
M internal/adapters/rpc/mergerequest.go +66 -6
@@ -17,14 +17,50 @@ import (
1717 // mergeRequestService implements ermiteconnect.MergeRequestServiceHandler.
1818 type mergeRequestService struct{ mrs ports.MergeRequestService }
1919
20-// EditComment is implemented with the note RPC adapter in phase P4a.
21-func (s *mergeRequestService) EditComment(context.Context, *connect.Request[pb.EditCommentRequest]) (*connect.Response[pb.EditCommentResponse], error) {
22- return nil, connect.NewError(connect.CodeUnimplemented, fmt.Errorf("EditComment is not implemented"))
20+func (s *mergeRequestService) EditComment(ctx context.Context, req *connect.Request[pb.EditCommentRequest]) (*connect.Response[pb.EditCommentResponse], error) {
21+ name, err := domain.ParseRepoName(req.Msg.GetRepo())
22+ if err != nil {
23+ return nil, connectError(ctx, err)
24+ }
25+ c, err := s.mrs.EditComment(ctx, caller(ctx), name, req.Msg.GetNumber(), commentID(req.Msg.GetCommentId()), req.Msg.GetText())
26+ if err != nil {
27+ return nil, connectError(ctx, err)
28+ }
29+ return connect.NewResponse(&pb.EditCommentResponse{Comment: commentToPB(c)}), nil
30+}
31+
32+func (s *mergeRequestService) DeleteComment(ctx context.Context, req *connect.Request[pb.DeleteCommentRequest]) (*connect.Response[pb.DeleteCommentResponse], error) {
33+ name, err := domain.ParseRepoName(req.Msg.GetRepo())
34+ if err != nil {
35+ return nil, connectError(ctx, err)
36+ }
37+ c, err := s.mrs.DeleteComment(ctx, caller(ctx), name, req.Msg.GetNumber(), commentID(req.Msg.GetCommentId()))
38+ if err != nil {
39+ return nil, connectError(ctx, err)
40+ }
41+ return connect.NewResponse(&pb.DeleteCommentResponse{Comment: commentToPB(c)}), nil
2342 }
2443
25-// DeleteComment is implemented with the note RPC adapter in phase P4a.
26-func (s *mergeRequestService) DeleteComment(context.Context, *connect.Request[pb.DeleteCommentRequest]) (*connect.Response[pb.DeleteCommentResponse], error) {
27- return nil, connect.NewError(connect.CodeUnimplemented, fmt.Errorf("DeleteComment is not implemented"))
44+func verdictToPB(v domain.Verdict) pb.Verdict {
45+ switch v {
46+ case domain.VerdictApprove:
47+ return pb.Verdict_VERDICT_APPROVE
48+ case domain.VerdictChangesRequested:
49+ return pb.Verdict_VERDICT_CHANGES_REQUESTED
50+ }
51+ return pb.Verdict_VERDICT_UNSPECIFIED
52+}
53+
54+func verdictFromPB(v pb.Verdict) (domain.Verdict, error) {
55+ switch v {
56+ case pb.Verdict_VERDICT_UNSPECIFIED:
57+ return domain.VerdictNone, nil
58+ case pb.Verdict_VERDICT_APPROVE:
59+ return domain.VerdictApprove, nil
60+ case pb.Verdict_VERDICT_CHANGES_REQUESTED:
61+ return domain.VerdictChangesRequested, nil
62+ }
63+ return domain.VerdictNone, fmt.Errorf("%w: verdict %v", domain.ErrInvalid, v)
2864 }
2965
3066 func mergeRequestStateToPB(s domain.MRState) pb.MergeRequestState {
@@ -106,9 +142,19 @@ func commentToPB(c domain.Comment) *pb.Comment {
106142 Text: c.Text.String(),
107143 Resolved: c.Resolved,
108144 CreatedAt: timestamp(c.CreatedAt),
145+ ReplyTo: optionalCommentID(c.ReplyTo),
146+ StartLine: int32(c.StartLine), //nolint:gosec // validated diff line
147+ Verdict: verdictToPB(c.Verdict), EditedAt: timestamp(c.EditedAt), Deleted: c.Deleted,
109148 }
110149 }
111150
151+func optionalCommentID(id int64) string {
152+ if id == 0 {
153+ return ""
154+ }
155+ return strconv.FormatInt(id, 10)
156+}
157+
112158 // commentID parses the opaque id of a comment. One that this server never
113159 // issued is 0, which names no comment.
114160 func commentID(s string) int64 {
@@ -195,6 +241,9 @@ func (s *mergeRequestService) GetMergeRequest(ctx context.Context, req *connect.
195241 for _, c := range d.Comments {
196242 out.Comments = append(out.Comments, commentToPB(c))
197243 }
244+ for _, v := range d.Verdicts {
245+ out.Verdicts = append(out.Verdicts, &pb.AuthorVerdict{Author: v.Author, Verdict: verdictToPB(v.Verdict), CommentId: strconv.FormatInt(v.CommentID, 10), CommitId: v.CommitID.String(), CreatedAt: timestamp(v.At), Stale: v.Stale})
246+ }
198247 return connect.NewResponse(out), nil
199248 }
200249
@@ -207,7 +256,18 @@ func (s *mergeRequestService) AddComment(ctx context.Context, req *connect.Reque
207256 if err != nil {
208257 return nil, connectError(ctx, err)
209258 }
259+ verdict, err := verdictFromPB(req.Msg.GetVerdict())
260+ if err != nil {
261+ return nil, connectError(ctx, err)
262+ }
263+ replyTo := commentID(req.Msg.GetReplyTo())
264+ if req.Msg.GetReplyTo() != "" && replyTo == 0 {
265+ var v domain.Violations
266+ v.Add("reply_to", "is not a note id")
267+ return nil, connectError(ctx, v.Err())
268+ }
210269 c, err := s.mrs.AddComment(ctx, caller(ctx), name, req.Msg.GetNumber(), ports.AddCommentParams{
270+ ReplyTo: replyTo, StartLine: int(req.Msg.GetStartLine()), Verdict: verdict,
211271 Path: req.Msg.GetPath(), Line: int(req.Msg.GetLine()), Side: side, Text: req.Msg.GetText(),
212272 })
213273 if err != nil {
@@ -17,14 +17,50 @@ import (
17 // mergeRequestService implements ermiteconnect.MergeRequestServiceHandler.17 // mergeRequestService implements ermiteconnect.MergeRequestServiceHandler.
18 type mergeRequestService struct{ mrs ports.MergeRequestService }18 type mergeRequestService struct{ mrs ports.MergeRequestService }
19 19
20-// EditComment is implemented with the note RPC adapter in phase P4a.20+func (s *mergeRequestService) EditComment(ctx context.Context, req *connect.Request[pb.EditCommentRequest]) (*connect.Response[pb.EditCommentResponse], error) {
21-func (s *mergeRequestService) EditComment(context.Context, *connect.Request[pb.EditCommentRequest]) (*connect.Response[pb.EditCommentResponse], error) {21+ name, err := domain.ParseRepoName(req.Msg.GetRepo())
22- return nil, connect.NewError(connect.CodeUnimplemented, fmt.Errorf("EditComment is not implemented"))22+ if err != nil {
23+ return nil, connectError(ctx, err)
24+ }
25+ c, err := s.mrs.EditComment(ctx, caller(ctx), name, req.Msg.GetNumber(), commentID(req.Msg.GetCommentId()), req.Msg.GetText())
26+ if err != nil {
27+ return nil, connectError(ctx, err)
28+ }
29+ return connect.NewResponse(&pb.EditCommentResponse{Comment: commentToPB(c)}), nil
30+}
31+
32+func (s *mergeRequestService) DeleteComment(ctx context.Context, req *connect.Request[pb.DeleteCommentRequest]) (*connect.Response[pb.DeleteCommentResponse], error) {
33+ name, err := domain.ParseRepoName(req.Msg.GetRepo())
34+ if err != nil {
35+ return nil, connectError(ctx, err)
36+ }
37+ c, err := s.mrs.DeleteComment(ctx, caller(ctx), name, req.Msg.GetNumber(), commentID(req.Msg.GetCommentId()))
38+ if err != nil {
39+ return nil, connectError(ctx, err)
40+ }
41+ return connect.NewResponse(&pb.DeleteCommentResponse{Comment: commentToPB(c)}), nil
23 }42 }
24 43
25-// DeleteComment is implemented with the note RPC adapter in phase P4a.44+func verdictToPB(v domain.Verdict) pb.Verdict {
26-func (s *mergeRequestService) DeleteComment(context.Context, *connect.Request[pb.DeleteCommentRequest]) (*connect.Response[pb.DeleteCommentResponse], error) {45+ switch v {
27- return nil, connect.NewError(connect.CodeUnimplemented, fmt.Errorf("DeleteComment is not implemented"))46+ case domain.VerdictApprove:
47+ return pb.Verdict_VERDICT_APPROVE
48+ case domain.VerdictChangesRequested:
49+ return pb.Verdict_VERDICT_CHANGES_REQUESTED
50+ }
51+ return pb.Verdict_VERDICT_UNSPECIFIED
52+}
53+
54+func verdictFromPB(v pb.Verdict) (domain.Verdict, error) {
55+ switch v {
56+ case pb.Verdict_VERDICT_UNSPECIFIED:
57+ return domain.VerdictNone, nil
58+ case pb.Verdict_VERDICT_APPROVE:
59+ return domain.VerdictApprove, nil
60+ case pb.Verdict_VERDICT_CHANGES_REQUESTED:
61+ return domain.VerdictChangesRequested, nil
62+ }
63+ return domain.VerdictNone, fmt.Errorf("%w: verdict %v", domain.ErrInvalid, v)
28 }64 }
29 65
30 func mergeRequestStateToPB(s domain.MRState) pb.MergeRequestState {66 func mergeRequestStateToPB(s domain.MRState) pb.MergeRequestState {
@@ -106,9 +142,19 @@ func commentToPB(c domain.Comment) *pb.Comment {
106 Text: c.Text.String(),142 Text: c.Text.String(),
107 Resolved: c.Resolved,143 Resolved: c.Resolved,
108 CreatedAt: timestamp(c.CreatedAt),144 CreatedAt: timestamp(c.CreatedAt),
145+ ReplyTo: optionalCommentID(c.ReplyTo),
146+ StartLine: int32(c.StartLine), //nolint:gosec // validated diff line
147+ Verdict: verdictToPB(c.Verdict), EditedAt: timestamp(c.EditedAt), Deleted: c.Deleted,
109 }148 }
110 }149 }
111 150
151+func optionalCommentID(id int64) string {
152+ if id == 0 {
153+ return ""
154+ }
155+ return strconv.FormatInt(id, 10)
156+}
157+
112 // commentID parses the opaque id of a comment. One that this server never158 // commentID parses the opaque id of a comment. One that this server never
113 // issued is 0, which names no comment.159 // issued is 0, which names no comment.
114 func commentID(s string) int64 {160 func commentID(s string) int64 {
@@ -195,6 +241,9 @@ func (s *mergeRequestService) GetMergeRequest(ctx context.Context, req *connect.
195 for _, c := range d.Comments {241 for _, c := range d.Comments {
196 out.Comments = append(out.Comments, commentToPB(c))242 out.Comments = append(out.Comments, commentToPB(c))
197 }243 }
244+ for _, v := range d.Verdicts {
245+ out.Verdicts = append(out.Verdicts, &pb.AuthorVerdict{Author: v.Author, Verdict: verdictToPB(v.Verdict), CommentId: strconv.FormatInt(v.CommentID, 10), CommitId: v.CommitID.String(), CreatedAt: timestamp(v.At), Stale: v.Stale})
246+ }
198 return connect.NewResponse(out), nil247 return connect.NewResponse(out), nil
199 }248 }
200 249
@@ -207,7 +256,18 @@ func (s *mergeRequestService) AddComment(ctx context.Context, req *connect.Reque
207 if err != nil {256 if err != nil {
208 return nil, connectError(ctx, err)257 return nil, connectError(ctx, err)
209 }258 }
259+ verdict, err := verdictFromPB(req.Msg.GetVerdict())
260+ if err != nil {
261+ return nil, connectError(ctx, err)
262+ }
263+ replyTo := commentID(req.Msg.GetReplyTo())
264+ if req.Msg.GetReplyTo() != "" && replyTo == 0 {
265+ var v domain.Violations
266+ v.Add("reply_to", "is not a note id")
267+ return nil, connectError(ctx, v.Err())
268+ }
210 c, err := s.mrs.AddComment(ctx, caller(ctx), name, req.Msg.GetNumber(), ports.AddCommentParams{269 c, err := s.mrs.AddComment(ctx, caller(ctx), name, req.Msg.GetNumber(), ports.AddCommentParams{
270+ ReplyTo: replyTo, StartLine: int(req.Msg.GetStartLine()), Verdict: verdict,
211 Path: req.Msg.GetPath(), Line: int(req.Msg.GetLine()), Side: side, Text: req.Msg.GetText(),271 Path: req.Msg.GetPath(), Line: int(req.Msg.GetLine()), Side: side, Text: req.Msg.GetText(),
212 })272 })
213 if err != nil {273 if err != nil {
A internal/adapters/rpc/mr_api_test.go +124 -0
@@ -0,0 +1,124 @@
1+// SPDX-License-Identifier: AGPL-3.0-or-later
2+
3+package rpc
4+
5+import (
6+ "testing"
7+ "time"
8+
9+ pb "bnema.dev/ermite-api/gen/ermite"
10+ "bnema.dev/ermite/internal/domain"
11+ "bnema.dev/ermite/internal/ports"
12+ "bnema.dev/ermite/internal/ports/mocks"
13+ "connectrpc.com/connect"
14+ "github.com/stretchr/testify/assert"
15+ "github.com/stretchr/testify/mock"
16+ "github.com/stretchr/testify/require"
17+)
18+
19+func TestNoteRPCFields(t *testing.T) {
20+ now := time.Now().UTC()
21+ note := domain.Comment{ID: 4, ReplyTo: 2, StartLine: 10, Line: 24, Verdict: domain.VerdictApprove, EditedAt: now, Deleted: true}
22+ got := commentToPB(note)
23+ assert.Equal(t, "2", got.GetReplyTo())
24+ assert.EqualValues(t, 10, got.GetStartLine())
25+ assert.Equal(t, pb.Verdict_VERDICT_APPROVE, got.GetVerdict())
26+ assert.Equal(t, now, got.GetEditedAt().AsTime())
27+ assert.True(t, got.GetDeleted())
28+ assert.Empty(t, commentToPB(domain.Comment{}).GetReplyTo())
29+ assert.Nil(t, commentToPB(domain.Comment{}).GetEditedAt())
30+ for _, v := range []domain.Verdict{domain.VerdictNone, domain.VerdictApprove, domain.VerdictChangesRequested} {
31+ back, err := verdictFromPB(verdictToPB(v))
32+ require.NoError(t, err)
33+ assert.Equal(t, v, back)
34+ }
35+ m := mocks.NewMergeRequestService(t)
36+ m.EXPECT().AddComment(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(1), ports.AddCommentParams{Text: "note", StartLine: 10, Line: 24, Verdict: domain.VerdictApprove}).Return(note, nil).Once()
37+ res, err := (&mergeRequestService{m}).AddComment(ownerCtx(), connect.NewRequest(&pb.AddCommentRequest{Repo: "demo", Number: 1, Text: "note", StartLine: 10, Line: 24, Verdict: pb.Verdict_VERDICT_APPROVE}))
38+ require.NoError(t, err)
39+ assert.Equal(t, got, res.Msg.GetComment())
40+ _, err = (&mergeRequestService{m}).AddComment(ownerCtx(), connect.NewRequest(&pb.AddCommentRequest{Repo: "demo", Verdict: pb.Verdict(99)}))
41+ requireCode(t, connect.CodeInvalidArgument, err)
42+}
43+
44+func TestNoteMutationRPCCodes(t *testing.T) {
45+ for _, edit := range []bool{false, true} {
46+ for _, failure := range []error{nil, domain.ErrForbidden, domain.ErrPreconditionFailed, domain.ErrNotFound} {
47+ m := mocks.NewMergeRequestService(t)
48+ note := domain.Comment{ID: 2, Deleted: !edit}
49+ var err error
50+ if edit {
51+ m.EXPECT().EditComment(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(1), int64(2), "new").Return(note, failure).Once()
52+ _, err = (&mergeRequestService{m}).EditComment(ownerCtx(), connect.NewRequest(&pb.EditCommentRequest{Repo: "demo", Number: 1, CommentId: "2", Text: "new"}))
53+ } else {
54+ m.EXPECT().DeleteComment(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(1), int64(2)).Return(note, failure).Once()
55+ _, err = (&mergeRequestService{m}).DeleteComment(ownerCtx(), connect.NewRequest(&pb.DeleteCommentRequest{Repo: "demo", Number: 1, CommentId: "2"}))
56+ }
57+ switch failure {
58+ case nil:
59+ require.NoError(t, err)
60+ case domain.ErrForbidden:
61+ requireCode(t, connect.CodePermissionDenied, err)
62+ case domain.ErrPreconditionFailed:
63+ requireCode(t, connect.CodeFailedPrecondition, err)
64+ case domain.ErrNotFound:
65+ requireCode(t, connect.CodeNotFound, err)
66+ }
67+ }
68+ }
69+}
70+
71+func TestNoteEventsRPC(t *testing.T) {
72+ repo := mustRepoName(t, "demo")
73+ note := domain.Comment{ID: 3, Verdict: domain.VerdictApprove, Deleted: true}
74+ at := time.Now().UTC()
75+ events := []domain.Event{
76+ domain.CommentEdited{Repo: repo, Number: 1, Comment: note, At: at},
77+ domain.CommentDeleted{Repo: repo, Number: 1, Comment: note, At: at},
78+ domain.CommentResolved{Repo: repo, Number: 1, Comment: note, At: at},
79+ }
80+ for i, ev := range events {
81+ got := watchEventToPB(ports.WatchEvent{ID: "e", Event: ev})
82+ require.NotNil(t, got)
83+ assert.Equal(t, at, got.GetTime().AsTime())
84+ var c *pb.Comment
85+ switch i {
86+ case 0:
87+ c = got.GetCommentEdited().GetComment()
88+ case 1:
89+ c = got.GetCommentDeleted().GetComment()
90+ case 2:
91+ c = got.GetCommentResolved().GetComment()
92+ }
93+ assert.Equal(t, "3", c.GetId())
94+ assert.True(t, c.GetDeleted())
95+ assert.Equal(t, pb.Verdict_VERDICT_APPROVE, c.GetVerdict())
96+ }
97+}
98+
99+func TestNoteReplyIDValidation(t *testing.T) {
100+ for _, id := range []string{"abc", "0", "-1", "01", "9223372036854775808"} {
101+ _, err := (&mergeRequestService{mocks.NewMergeRequestService(t)}).AddComment(ownerCtx(), connect.NewRequest(&pb.AddCommentRequest{Repo: "demo", Number: 1, ReplyTo: id, Text: "reply"}))
102+ requireCode(t, connect.CodeInvalidArgument, err)
103+ assert.Contains(t, err.Error(), "reply_to")
104+ assert.Contains(t, err.Error(), "is not a note id")
105+ }
106+ m := mocks.NewMergeRequestService(t)
107+ m.EXPECT().AddComment(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(1), ports.AddCommentParams{ReplyTo: 2, Text: "reply"}).Return(domain.Comment{ID: 3, ReplyTo: 2}, nil).Once()
108+ res, err := (&mergeRequestService{m}).AddComment(ownerCtx(), connect.NewRequest(&pb.AddCommentRequest{Repo: "demo", Number: 1, ReplyTo: "2", Text: "reply"}))
109+ require.NoError(t, err)
110+ assert.Equal(t, "2", res.Msg.GetComment().GetReplyTo())
111+}
112+
113+func TestNoteVerdictsDetailAndScope(t *testing.T) {
114+ m := mocks.NewMergeRequestService(t)
115+ m.EXPECT().Get(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(1), ports.GetMergeRequestOptions{}).Return(ports.MergeRequestDetail{Verdicts: []domain.AuthorVerdict{{Author: "owner", Verdict: domain.VerdictApprove, CommentID: 4, Stale: true}}}, nil).Once()
116+ res, err := (&mergeRequestService{m}).GetMergeRequest(ownerCtx(), connect.NewRequest(&pb.GetMergeRequestRequest{Repo: "demo", Number: 1}))
117+ require.NoError(t, err)
118+ require.Len(t, res.Msg.GetVerdicts(), 1)
119+ assert.Equal(t, "4", res.Msg.GetVerdicts()[0].GetCommentId())
120+ assert.True(t, res.Msg.GetVerdicts()[0].GetStale())
121+ m.EXPECT().AddComment(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(1), ports.AddCommentParams{Verdict: domain.VerdictApprove}).Return(domain.Comment{}, domain.ErrForbidden).Once()
122+ _, err = (&mergeRequestService{m}).AddComment(ownerCtx(), connect.NewRequest(&pb.AddCommentRequest{Repo: "demo", Number: 1, Verdict: pb.Verdict_VERDICT_APPROVE}))
123+ requireCode(t, connect.CodePermissionDenied, err)
124+}
@@ -0,0 +1,124 @@
1+// SPDX-License-Identifier: AGPL-3.0-or-later
2+
3+package rpc
4+
5+import (
6+ "testing"
7+ "time"
8+
9+ pb "bnema.dev/ermite-api/gen/ermite"
10+ "bnema.dev/ermite/internal/domain"
11+ "bnema.dev/ermite/internal/ports"
12+ "bnema.dev/ermite/internal/ports/mocks"
13+ "connectrpc.com/connect"
14+ "github.com/stretchr/testify/assert"
15+ "github.com/stretchr/testify/mock"
16+ "github.com/stretchr/testify/require"
17+)
18+
19+func TestNoteRPCFields(t *testing.T) {
20+ now := time.Now().UTC()
21+ note := domain.Comment{ID: 4, ReplyTo: 2, StartLine: 10, Line: 24, Verdict: domain.VerdictApprove, EditedAt: now, Deleted: true}
22+ got := commentToPB(note)
23+ assert.Equal(t, "2", got.GetReplyTo())
24+ assert.EqualValues(t, 10, got.GetStartLine())
25+ assert.Equal(t, pb.Verdict_VERDICT_APPROVE, got.GetVerdict())
26+ assert.Equal(t, now, got.GetEditedAt().AsTime())
27+ assert.True(t, got.GetDeleted())
28+ assert.Empty(t, commentToPB(domain.Comment{}).GetReplyTo())
29+ assert.Nil(t, commentToPB(domain.Comment{}).GetEditedAt())
30+ for _, v := range []domain.Verdict{domain.VerdictNone, domain.VerdictApprove, domain.VerdictChangesRequested} {
31+ back, err := verdictFromPB(verdictToPB(v))
32+ require.NoError(t, err)
33+ assert.Equal(t, v, back)
34+ }
35+ m := mocks.NewMergeRequestService(t)
36+ m.EXPECT().AddComment(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(1), ports.AddCommentParams{Text: "note", StartLine: 10, Line: 24, Verdict: domain.VerdictApprove}).Return(note, nil).Once()
37+ res, err := (&mergeRequestService{m}).AddComment(ownerCtx(), connect.NewRequest(&pb.AddCommentRequest{Repo: "demo", Number: 1, Text: "note", StartLine: 10, Line: 24, Verdict: pb.Verdict_VERDICT_APPROVE}))
38+ require.NoError(t, err)
39+ assert.Equal(t, got, res.Msg.GetComment())
40+ _, err = (&mergeRequestService{m}).AddComment(ownerCtx(), connect.NewRequest(&pb.AddCommentRequest{Repo: "demo", Verdict: pb.Verdict(99)}))
41+ requireCode(t, connect.CodeInvalidArgument, err)
42+}
43+
44+func TestNoteMutationRPCCodes(t *testing.T) {
45+ for _, edit := range []bool{false, true} {
46+ for _, failure := range []error{nil, domain.ErrForbidden, domain.ErrPreconditionFailed, domain.ErrNotFound} {
47+ m := mocks.NewMergeRequestService(t)
48+ note := domain.Comment{ID: 2, Deleted: !edit}
49+ var err error
50+ if edit {
51+ m.EXPECT().EditComment(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(1), int64(2), "new").Return(note, failure).Once()
52+ _, err = (&mergeRequestService{m}).EditComment(ownerCtx(), connect.NewRequest(&pb.EditCommentRequest{Repo: "demo", Number: 1, CommentId: "2", Text: "new"}))
53+ } else {
54+ m.EXPECT().DeleteComment(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(1), int64(2)).Return(note, failure).Once()
55+ _, err = (&mergeRequestService{m}).DeleteComment(ownerCtx(), connect.NewRequest(&pb.DeleteCommentRequest{Repo: "demo", Number: 1, CommentId: "2"}))
56+ }
57+ switch failure {
58+ case nil:
59+ require.NoError(t, err)
60+ case domain.ErrForbidden:
61+ requireCode(t, connect.CodePermissionDenied, err)
62+ case domain.ErrPreconditionFailed:
63+ requireCode(t, connect.CodeFailedPrecondition, err)
64+ case domain.ErrNotFound:
65+ requireCode(t, connect.CodeNotFound, err)
66+ }
67+ }
68+ }
69+}
70+
71+func TestNoteEventsRPC(t *testing.T) {
72+ repo := mustRepoName(t, "demo")
73+ note := domain.Comment{ID: 3, Verdict: domain.VerdictApprove, Deleted: true}
74+ at := time.Now().UTC()
75+ events := []domain.Event{
76+ domain.CommentEdited{Repo: repo, Number: 1, Comment: note, At: at},
77+ domain.CommentDeleted{Repo: repo, Number: 1, Comment: note, At: at},
78+ domain.CommentResolved{Repo: repo, Number: 1, Comment: note, At: at},
79+ }
80+ for i, ev := range events {
81+ got := watchEventToPB(ports.WatchEvent{ID: "e", Event: ev})
82+ require.NotNil(t, got)
83+ assert.Equal(t, at, got.GetTime().AsTime())
84+ var c *pb.Comment
85+ switch i {
86+ case 0:
87+ c = got.GetCommentEdited().GetComment()
88+ case 1:
89+ c = got.GetCommentDeleted().GetComment()
90+ case 2:
91+ c = got.GetCommentResolved().GetComment()
92+ }
93+ assert.Equal(t, "3", c.GetId())
94+ assert.True(t, c.GetDeleted())
95+ assert.Equal(t, pb.Verdict_VERDICT_APPROVE, c.GetVerdict())
96+ }
97+}
98+
99+func TestNoteReplyIDValidation(t *testing.T) {
100+ for _, id := range []string{"abc", "0", "-1", "01", "9223372036854775808"} {
101+ _, err := (&mergeRequestService{mocks.NewMergeRequestService(t)}).AddComment(ownerCtx(), connect.NewRequest(&pb.AddCommentRequest{Repo: "demo", Number: 1, ReplyTo: id, Text: "reply"}))
102+ requireCode(t, connect.CodeInvalidArgument, err)
103+ assert.Contains(t, err.Error(), "reply_to")
104+ assert.Contains(t, err.Error(), "is not a note id")
105+ }
106+ m := mocks.NewMergeRequestService(t)
107+ m.EXPECT().AddComment(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(1), ports.AddCommentParams{ReplyTo: 2, Text: "reply"}).Return(domain.Comment{ID: 3, ReplyTo: 2}, nil).Once()
108+ res, err := (&mergeRequestService{m}).AddComment(ownerCtx(), connect.NewRequest(&pb.AddCommentRequest{Repo: "demo", Number: 1, ReplyTo: "2", Text: "reply"}))
109+ require.NoError(t, err)
110+ assert.Equal(t, "2", res.Msg.GetComment().GetReplyTo())
111+}
112+
113+func TestNoteVerdictsDetailAndScope(t *testing.T) {
114+ m := mocks.NewMergeRequestService(t)
115+ m.EXPECT().Get(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(1), ports.GetMergeRequestOptions{}).Return(ports.MergeRequestDetail{Verdicts: []domain.AuthorVerdict{{Author: "owner", Verdict: domain.VerdictApprove, CommentID: 4, Stale: true}}}, nil).Once()
116+ res, err := (&mergeRequestService{m}).GetMergeRequest(ownerCtx(), connect.NewRequest(&pb.GetMergeRequestRequest{Repo: "demo", Number: 1}))
117+ require.NoError(t, err)
118+ require.Len(t, res.Msg.GetVerdicts(), 1)
119+ assert.Equal(t, "4", res.Msg.GetVerdicts()[0].GetCommentId())
120+ assert.True(t, res.Msg.GetVerdicts()[0].GetStale())
121+ m.EXPECT().AddComment(mock.Anything, domain.Owner(), mustRepoName(t, "demo"), int64(1), ports.AddCommentParams{Verdict: domain.VerdictApprove}).Return(domain.Comment{}, domain.ErrForbidden).Once()
122+ _, err = (&mergeRequestService{m}).AddComment(ownerCtx(), connect.NewRequest(&pb.AddCommentRequest{Repo: "demo", Number: 1, Verdict: pb.Verdict_VERDICT_APPROVE}))
123+ requireCode(t, connect.CodePermissionDenied, err)
124+}
M internal/app/cli_mr.go +221 -25
@@ -141,12 +141,28 @@ type jsonComment struct {
141141 Text string `json:"text"`
142142 Resolved bool `json:"resolved"`
143143 CreatedAt *string `json:"created_at"`
144+ ReplyTo string `json:"reply_to"`
145+ StartLine int32 `json:"start_line"`
146+ Verdict string `json:"verdict"`
147+ EditedAt *string `json:"edited_at"`
148+ Deleted bool `json:"deleted"`
149+}
150+
151+func verdictName(v ermite.Verdict) string {
152+ switch v {
153+ case ermite.Verdict_VERDICT_APPROVE:
154+ return "approve"
155+ case ermite.Verdict_VERDICT_CHANGES_REQUESTED:
156+ return "changes_requested"
157+ }
158+ return ""
144159 }
145160
146161 func toJSONComment(c *ermite.Comment) jsonComment {
147162 return jsonComment{
148163 ID: c.GetId(), Author: c.GetAuthor(), Path: c.GetPath(), Line: c.GetLine(), Side: sideName(c.GetSide()),
149164 CommitID: c.GetCommitId(), Text: c.GetText(), Resolved: c.GetResolved(), CreatedAt: jsonTime(c.GetCreatedAt()),
165+ ReplyTo: c.GetReplyTo(), StartLine: c.GetStartLine(), Verdict: verdictName(c.GetVerdict()), EditedAt: jsonTime(c.GetEditedAt()), Deleted: c.GetDeleted(),
150166 }
151167 }
152168
@@ -160,7 +176,8 @@ unless merged).`
160176 const jsonCommentHelp = `
161177 JSON fields of a comment: id, author, path, line and side ("old" or "new"; empty path, line 0
162178 and empty side for a general comment), commit_id (the head when it was written), text,
163-resolved, created_at.`
179+resolved, created_at, reply_to (empty for roots), start_line (0 for a single line),
180+verdict ("", "approve", "changes_requested"), edited_at (nullable RFC 3339), deleted.`
164181
165182 func newMRCmd(env *cliEnv) *cobra.Command {
166183 mr := &cobra.Command{
@@ -172,7 +189,7 @@ func newMRCmd(env *cliEnv) *cobra.Command {
172189 }
173190 mr.AddCommand(
174191 newMRCreateCmd(env), newMRListCmd(env), newMRShowCmd(env), newMRCommentCmd(env),
175- newMRResolveCmd(env), newMRMergeCmd(env), newMRCloseCmd(env),
192+ newMRResolveCmd(env), newMREditCommentCmd(env), newMRDeleteCommentCmd(env), newMRMergeCmd(env), newMRCloseCmd(env),
176193 )
177194 return mr
178195 }
@@ -385,15 +402,25 @@ type jsonCommit struct {
385402 // jsonMRDetail is the stable JSON shape of `mr show --json`.
386403 type jsonMRDetail struct {
387404 jsonMergeRequest
388- Commits []jsonCommit `json:"commits"`
389- CommitsTruncated bool `json:"commits_truncated"`
390- DiffBaseID string `json:"diff_base_id"`
391- Stats jsonStats `json:"stats"`
392- DiffTruncated bool `json:"diff_truncated"`
393- Files []jsonFile `json:"files"`
394- Comments []jsonComment `json:"comments"`
395- LastReadID string `json:"last_read_id"`
396- LastReadLost bool `json:"last_read_lost"`
405+ Commits []jsonCommit `json:"commits"`
406+ CommitsTruncated bool `json:"commits_truncated"`
407+ DiffBaseID string `json:"diff_base_id"`
408+ Stats jsonStats `json:"stats"`
409+ DiffTruncated bool `json:"diff_truncated"`
410+ Files []jsonFile `json:"files"`
411+ Comments []jsonComment `json:"comments"`
412+ Verdicts []jsonAuthorVerdict `json:"verdicts"`
413+ LastReadID string `json:"last_read_id"`
414+ LastReadLost bool `json:"last_read_lost"`
415+}
416+
417+type jsonAuthorVerdict struct {
418+ Author string `json:"author"`
419+ Verdict string `json:"verdict"`
420+ CommentID string `json:"comment_id"`
421+ CommitID string `json:"commit_id"`
422+ CreatedAt *string `json:"created_at"`
423+ Stale bool `json:"stale"`
397424 }
398425
399426 func fileStatusName(s ermite.FileStatus) string {
@@ -413,7 +440,7 @@ func toJSONDetail(d *ermite.GetMergeRequestResponse, withDiff bool) jsonMRDetail
413440 FilesChanged: d.GetStats().GetFilesChanged(), Additions: d.GetStats().GetAdditions(), Deletions: d.GetStats().GetDeletions(),
414441 },
415442 DiffTruncated: d.GetDiff().GetTruncated(),
416- Files: []jsonFile{}, Comments: []jsonComment{},
443+ Files: []jsonFile{}, Comments: []jsonComment{}, Verdicts: []jsonAuthorVerdict{},
417444 LastReadID: d.GetLastReadId(), LastReadLost: d.GetLastReadLost(),
418445 }
419446 for _, c := range d.GetCommits() {
@@ -443,6 +470,9 @@ func toJSONDetail(d *ermite.GetMergeRequestResponse, withDiff bool) jsonMRDetail
443470 for _, c := range d.GetComments() {
444471 out.Comments = append(out.Comments, toJSONComment(c))
445472 }
473+ for _, v := range d.GetVerdicts() {
474+ out.Verdicts = append(out.Verdicts, jsonAuthorVerdict{Author: v.GetAuthor(), Verdict: verdictName(v.GetVerdict()), CommentID: v.GetCommentId(), CommitID: v.GetCommitId(), CreatedAt: jsonTime(v.GetCreatedAt()), Stale: v.GetStale()})
475+ }
446476 return out
447477 }
448478
@@ -461,7 +491,7 @@ func newMRShowCmd(env *cliEnv) *cobra.Command {
461491 "diff_base_id (the commit the diff starts from), stats (files_changed, additions, deletions),\n" +
462492 "diff_truncated, files (old_path, new_path, status, additions, deletions, binary, truncated and,\n" +
463493 "with --diff, hunks of lines with kind, old_number, new_number and text), comments, last_read_id\n" +
464- "and last_read_lost.",
494+ "and last_read_lost; verdicts (author, verdict, comment_id, commit_id, created_at, stale).",
465495 Args: cobra.ExactArgs(2),
466496 ValidArgsFunction: completeMRRepoThenNumber(env),
467497 }
@@ -529,6 +559,20 @@ func printMRDetail(w io.Writer, d *ermite.GetMergeRequestResponse, withDiff, sin
529559 if m.GetState() == ermite.MergeRequestState_MERGE_REQUEST_STATE_MERGED {
530560 pairs = append(pairs, [2]string{"merged", orDash(m.GetMergedCommitId()) + " (" + modeName(m.GetMergeMode()) + ")"})
531561 }
562+ if len(d.GetVerdicts()) > 0 {
563+ var verdicts []string
564+ for _, v := range d.GetVerdicts() {
565+ label := "changes requested"
566+ if v.GetVerdict() == ermite.Verdict_VERDICT_APPROVE {
567+ label = "approved"
568+ }
569+ if v.GetStale() {
570+ label += " (stale)"
571+ }
572+ verdicts = append(verdicts, v.GetAuthor()+" "+label)
573+ }
574+ pairs = append(pairs, [2]string{"verdicts", strings.Join(verdicts, ", ")})
575+ }
532576 if err := fields(w, pairs); err != nil {
533577 return err
534578 }
@@ -561,16 +605,46 @@ func printMRDetail(w io.Writer, d *ermite.GetMergeRequestResponse, withDiff, sin
561605 }
562606
563607 _, _ = fmt.Fprintf(w, "\ncomments (%d)\n", len(d.GetComments()))
564- for _, c := range d.GetComments() {
608+ var ordered []*ermite.Comment
609+ for _, root := range d.GetComments() {
610+ if root.GetReplyTo() != "" {
611+ continue
612+ }
613+ ordered = append(ordered, root)
614+ for _, reply := range d.GetComments() {
615+ if reply.GetReplyTo() == root.GetId() {
616+ ordered = append(ordered, reply)
617+ }
618+ }
619+ }
620+ for _, c := range ordered {
565621 where := "general"
566622 if c.GetPath() != "" {
567623 where = fmt.Sprintf("%s:%d (%s)", clean(c.GetPath()), c.GetLine(), sideName(c.GetSide()))
568624 }
625+ if c.GetStartLine() > 0 {
626+ where = fmt.Sprintf("%s:lines %d-%d (%s)", clean(c.GetPath()), c.GetStartLine(), c.GetLine(), sideName(c.GetSide()))
627+ }
569628 mark := ""
629+ if c.GetEditedAt() != nil {
630+ mark += " (edited)"
631+ }
632+ if c.GetDeleted() {
633+ mark += " [deleted]"
634+ }
635+ if c.GetVerdict() != ermite.Verdict_VERDICT_UNSPECIFIED {
636+ mark += " [" + verdictName(c.GetVerdict()) + "]"
637+ }
570638 if c.GetResolved() {
571- mark = " [resolved]"
639+ mark += " [resolved]"
572640 }
573- _, _ = fmt.Fprintf(w, " #%s %s, %s, %s%s\n%s\n", c.GetId(), clean(c.GetAuthor()), shortTime(c.GetCreatedAt(), "-"), where, mark, indentText(c.GetText()))
641+ prefix := " "
642+ body := indentText(c.GetText())
643+ if c.GetReplyTo() != "" {
644+ prefix += " "
645+ body = " " + strings.ReplaceAll(body, "\n", "\n ")
646+ }
647+ _, _ = fmt.Fprintf(w, "%s#%s %s, %s, %s%s\n%s\n", prefix, c.GetId(), clean(c.GetAuthor()), shortTime(c.GetCreatedAt(), "-"), where, mark, body)
574648 }
575649 if withDiff {
576650 printDiff(w, d.GetDiff())
@@ -643,12 +717,14 @@ func printDiff(w io.Writer, d *ermite.Diff) {
643717 }
644718
645719 func newMRCommentCmd(env *cliEnv) *cobra.Command {
646- var message, path, side string
647- var line int
720+ var message, path, side, line, reply string
721+ var approve, requestChanges bool
648722 cmd := &cobra.Command{
649723 Use: "comment <repo> <number>",
650724 Short: "Comment on a merge request",
651- Long: "Add a plain-text comment (at most 8000 characters, no control characters except line breaks).\n" +
725+ Long: "Add a Markdown note (at most 8000 characters); ^N refers to a note in this merge request.\n" +
726+ "--reply replies to a root note. --line accepts N or A-B. --approve and --request-changes\n" +
727+ "set a verdict (needs mr:review) and allow an empty message.\n" +
652728 "-m - reads it from standard input. Without --path it is a general comment. With --path, --line and\n" +
653729 "--side (default new) it is attached to a line of the current diff: the path as in `mr show --diff`\n" +
654730 "(the new path, the old one for a deleted file), the line number on the old or new side; a line\n" +
@@ -656,11 +732,18 @@ func newMRCommentCmd(env *cliEnv) *cobra.Command {
656732 Args: cobra.ExactArgs(2),
657733 ValidArgsFunction: completeMRRepoThenNumber(env),
658734 }
659- cmd.Flags().StringVarP(&message, "message", "m", "", "text of the comment, or - to read standard input (required)")
735+ cmd.Flags().StringVarP(&message, "message", "m", "", "Markdown text, or - to read standard input (required unless setting a verdict)")
660736 cmd.Flags().StringVar(&path, "path", "", "file the comment is attached to")
661- cmd.Flags().IntVar(&line, "line", 0, "1-based line number on --side")
737+ cmd.Flags().StringVar(&line, "line", "", "1-based line N or range A-B on --side")
738+ cmd.Flags().StringVar(&reply, "reply", "", "reply to root note (^12, #12 or 12)")
739+ cmd.Flags().BoolVar(&approve, "approve", false, "approve (needs mr:review)")
740+ cmd.Flags().BoolVar(&requestChanges, "request-changes", false, "request changes (needs mr:review)")
662741 cmd.Flags().StringVar(&side, "side", sideNew, "side the line counts on: old or new")
663- _ = cmd.MarkFlagRequired("message")
742+ cmd.MarkFlagsMutuallyExclusive("approve", "request-changes")
743+ for _, flag := range []string{"path", "line", "side", "approve", "request-changes"} {
744+ cmd.MarkFlagsMutuallyExclusive("reply", flag)
745+ }
746+ // A verdict note may omit its message.
664747 _ = cmd.RegisterFlagCompletionFunc("side", func(*cobra.Command, []string, string) ([]string, cobra.ShellCompDirective) {
665748 return []string{sideOld, sideNew}, cobra.ShellCompDirectiveNoFileComp
666749 })
@@ -674,17 +757,36 @@ func newMRCommentCmd(env *cliEnv) *cobra.Command {
674757 if err != nil {
675758 return err
676759 }
760+ if !cmd.Flags().Changed("message") && !approve && !requestChanges {
761+ return errors.New("--message is required unless --approve or --request-changes is set")
762+ }
677763 req := &ermite.AddCommentRequest{Repo: args[0], Number: n, Text: text}
764+ if approve {
765+ req.Verdict = ermite.Verdict_VERDICT_APPROVE
766+ }
767+ if requestChanges {
768+ req.Verdict = ermite.Verdict_VERDICT_CHANGES_REQUESTED
769+ }
770+ if cmd.Flags().Changed("reply") {
771+ req.ReplyTo, err = parseNoteID(reply)
772+ if err != nil {
773+ return err
774+ }
775+ }
678776 switch {
679777 case path != "":
680- if line < 1 {
778+ if line == "" {
681779 return errors.New("--line is required with --path: give the line number")
682780 }
781+ req.StartLine, req.Line, err = parseLineRange(line)
782+ if err != nil {
783+ return err
784+ }
683785 if req.Side, err = parseSide(side); err != nil {
684786 return err
685787 }
686- req.Path, req.Line = path, int32(line) //nolint:gosec // a line number typed by a person
687- case line != 0 || cmd.Flags().Changed("side"):
788+ req.Path = path
789+ case cmd.Flags().Changed("line") || cmd.Flags().Changed("side"):
688790 return errors.New("--line and --side need --path")
689791 }
690792 c, err := env.client(cmd)
@@ -704,6 +806,100 @@ func newMRCommentCmd(env *cliEnv) *cobra.Command {
704806 return cmd
705807 }
706808
809+func parseNoteID(s string) (string, error) {
810+ id := strings.TrimLeft(strings.TrimSpace(s), "^#")
811+ n, err := strconv.ParseInt(id, 10, 64)
812+ if err != nil || n < 1 {
813+ return "", fmt.Errorf("invalid note id %q: use ^12, #12 or 12", s)
814+ }
815+ return strconv.FormatInt(n, 10), nil
816+}
817+
818+func parseLineRange(s string) (start, end int32, err error) {
819+ parts := strings.Split(s, "-")
820+ invalid := fmt.Errorf("invalid --line %q: use N or A-B with 1 <= A < B", s)
821+ if len(parts) < 1 || len(parts) > 2 {
822+ return 0, 0, invalid
823+ }
824+ parse := func(v string) (int32, error) {
825+ n, e := strconv.ParseInt(v, 10, 32)
826+ if e != nil || n < 1 {
827+ return 0, invalid
828+ }
829+ return int32(n), nil
830+ }
831+ end, err = parse(parts[len(parts)-1])
832+ if err != nil {
833+ return 0, 0, err
834+ }
835+ if len(parts) == 2 {
836+ start, err = parse(parts[0])
837+ if err != nil || start >= end {
838+ return 0, 0, invalid
839+ }
840+ }
841+ return start, end, nil
842+}
843+
844+func newMREditCommentCmd(env *cliEnv) *cobra.Command { return newMRNoteMutationCmd(env, true) }
845+func newMRDeleteCommentCmd(env *cliEnv) *cobra.Command { return newMRNoteMutationCmd(env, false) }
846+
847+func newMRNoteMutationCmd(env *cliEnv, edit bool) *cobra.Command {
848+ verb := "delete"
849+ if edit {
850+ verb = "edit"
851+ }
852+ var message string
853+ cmd := &cobra.Command{Use: verb + "-comment <repo> <number> <comment-id>", Short: verb + " your own merge request note", Long: "Edit or delete your own note on an open merge request. Needs mr:write. Verdicts are unchanged." + jsonCommentHelp, Args: cobra.ExactArgs(3), ValidArgsFunction: completeMRRepoThenNumber(env)}
854+ if edit {
855+ cmd.Flags().StringVarP(&message, "message", "m", "", "Markdown text, or - to read standard input (required)")
856+ _ = cmd.MarkFlagRequired("message")
857+ }
858+ asJSON := addJSONFlag(cmd)
859+ cmd.RunE = func(cmd *cobra.Command, args []string) error {
860+ n, err := parseNumber(args[1])
861+ if err != nil {
862+ return err
863+ }
864+ id, err := parseNoteID(args[2])
865+ if err != nil {
866+ return err
867+ }
868+ text, err := readText(cmd, message)
869+ if err != nil {
870+ return err
871+ }
872+ c, err := env.client(cmd)
873+ if err != nil {
874+ return err
875+ }
876+ var note *ermite.Comment
877+ if edit {
878+ res, e := c.MergeRequests().EditComment(env.ctx(cmd), connect.NewRequest(&ermite.EditCommentRequest{Repo: args[0], Number: n, CommentId: id, Text: text}))
879+ if e != nil {
880+ return c.Describe(e)
881+ }
882+ note = res.Msg.GetComment()
883+ } else {
884+ res, e := c.MergeRequests().DeleteComment(env.ctx(cmd), connect.NewRequest(&ermite.DeleteCommentRequest{Repo: args[0], Number: n, CommentId: id}))
885+ if e != nil {
886+ return c.Describe(e)
887+ }
888+ note = res.Msg.GetComment()
889+ }
890+ if *asJSON {
891+ return writeJSON(cmd.OutOrStdout(), toJSONComment(note))
892+ }
893+ past := "deleted"
894+ if edit {
895+ past = "edited"
896+ }
897+ _, err = fmt.Fprintf(cmd.OutOrStdout(), "comment %s of %s!%d %s\n", clean(id), clean(args[0]), n, past)
898+ return err
899+ }
900+ return cmd
901+}
902+
707903 func newMRResolveCmd(env *cliEnv) *cobra.Command {
708904 var undo bool
709905 cmd := &cobra.Command{
@@ -141,12 +141,28 @@ type jsonComment struct {
141 Text string `json:"text"`141 Text string `json:"text"`
142 Resolved bool `json:"resolved"`142 Resolved bool `json:"resolved"`
143 CreatedAt *string `json:"created_at"`143 CreatedAt *string `json:"created_at"`
144+ ReplyTo string `json:"reply_to"`
145+ StartLine int32 `json:"start_line"`
146+ Verdict string `json:"verdict"`
147+ EditedAt *string `json:"edited_at"`
148+ Deleted bool `json:"deleted"`
149+}
150+
151+func verdictName(v ermite.Verdict) string {
152+ switch v {
153+ case ermite.Verdict_VERDICT_APPROVE:
154+ return "approve"
155+ case ermite.Verdict_VERDICT_CHANGES_REQUESTED:
156+ return "changes_requested"
157+ }
158+ return ""
144 }159 }
145 160
146 func toJSONComment(c *ermite.Comment) jsonComment {161 func toJSONComment(c *ermite.Comment) jsonComment {
147 return jsonComment{162 return jsonComment{
148 ID: c.GetId(), Author: c.GetAuthor(), Path: c.GetPath(), Line: c.GetLine(), Side: sideName(c.GetSide()),163 ID: c.GetId(), Author: c.GetAuthor(), Path: c.GetPath(), Line: c.GetLine(), Side: sideName(c.GetSide()),
149 CommitID: c.GetCommitId(), Text: c.GetText(), Resolved: c.GetResolved(), CreatedAt: jsonTime(c.GetCreatedAt()),164 CommitID: c.GetCommitId(), Text: c.GetText(), Resolved: c.GetResolved(), CreatedAt: jsonTime(c.GetCreatedAt()),
165+ ReplyTo: c.GetReplyTo(), StartLine: c.GetStartLine(), Verdict: verdictName(c.GetVerdict()), EditedAt: jsonTime(c.GetEditedAt()), Deleted: c.GetDeleted(),
150 }166 }
151 }167 }
152 168
@@ -160,7 +176,8 @@ unless merged).`
160 const jsonCommentHelp = `176 const jsonCommentHelp = `
161 JSON fields of a comment: id, author, path, line and side ("old" or "new"; empty path, line 0177 JSON fields of a comment: id, author, path, line and side ("old" or "new"; empty path, line 0
162 and empty side for a general comment), commit_id (the head when it was written), text,178 and empty side for a general comment), commit_id (the head when it was written), text,
163-resolved, created_at.`179+resolved, created_at, reply_to (empty for roots), start_line (0 for a single line),
180+verdict ("", "approve", "changes_requested"), edited_at (nullable RFC 3339), deleted.`
164 181
165 func newMRCmd(env *cliEnv) *cobra.Command {182 func newMRCmd(env *cliEnv) *cobra.Command {
166 mr := &cobra.Command{183 mr := &cobra.Command{
@@ -172,7 +189,7 @@ func newMRCmd(env *cliEnv) *cobra.Command {
172 }189 }
173 mr.AddCommand(190 mr.AddCommand(
174 newMRCreateCmd(env), newMRListCmd(env), newMRShowCmd(env), newMRCommentCmd(env),191 newMRCreateCmd(env), newMRListCmd(env), newMRShowCmd(env), newMRCommentCmd(env),
175- newMRResolveCmd(env), newMRMergeCmd(env), newMRCloseCmd(env),192+ newMRResolveCmd(env), newMREditCommentCmd(env), newMRDeleteCommentCmd(env), newMRMergeCmd(env), newMRCloseCmd(env),
176 )193 )
177 return mr194 return mr
178 }195 }
@@ -385,15 +402,25 @@ type jsonCommit struct {
385 // jsonMRDetail is the stable JSON shape of `mr show --json`.402 // jsonMRDetail is the stable JSON shape of `mr show --json`.
386 type jsonMRDetail struct {403 type jsonMRDetail struct {
387 jsonMergeRequest404 jsonMergeRequest
388- Commits []jsonCommit `json:"commits"`405+ Commits []jsonCommit `json:"commits"`
389- CommitsTruncated bool `json:"commits_truncated"`406+ CommitsTruncated bool `json:"commits_truncated"`
390- DiffBaseID string `json:"diff_base_id"`407+ DiffBaseID string `json:"diff_base_id"`
391- Stats jsonStats `json:"stats"`408+ Stats jsonStats `json:"stats"`
392- DiffTruncated bool `json:"diff_truncated"`409+ DiffTruncated bool `json:"diff_truncated"`
393- Files []jsonFile `json:"files"`410+ Files []jsonFile `json:"files"`
394- Comments []jsonComment `json:"comments"`411+ Comments []jsonComment `json:"comments"`
395- LastReadID string `json:"last_read_id"`412+ Verdicts []jsonAuthorVerdict `json:"verdicts"`
396- LastReadLost bool `json:"last_read_lost"`413+ LastReadID string `json:"last_read_id"`
414+ LastReadLost bool `json:"last_read_lost"`
415+}
416+
417+type jsonAuthorVerdict struct {
418+ Author string `json:"author"`
419+ Verdict string `json:"verdict"`
420+ CommentID string `json:"comment_id"`
421+ CommitID string `json:"commit_id"`
422+ CreatedAt *string `json:"created_at"`
423+ Stale bool `json:"stale"`
397 }424 }
398 425
399 func fileStatusName(s ermite.FileStatus) string {426 func fileStatusName(s ermite.FileStatus) string {
@@ -413,7 +440,7 @@ func toJSONDetail(d *ermite.GetMergeRequestResponse, withDiff bool) jsonMRDetail
413 FilesChanged: d.GetStats().GetFilesChanged(), Additions: d.GetStats().GetAdditions(), Deletions: d.GetStats().GetDeletions(),440 FilesChanged: d.GetStats().GetFilesChanged(), Additions: d.GetStats().GetAdditions(), Deletions: d.GetStats().GetDeletions(),
414 },441 },
415 DiffTruncated: d.GetDiff().GetTruncated(),442 DiffTruncated: d.GetDiff().GetTruncated(),
416- Files: []jsonFile{}, Comments: []jsonComment{},443+ Files: []jsonFile{}, Comments: []jsonComment{}, Verdicts: []jsonAuthorVerdict{},
417 LastReadID: d.GetLastReadId(), LastReadLost: d.GetLastReadLost(),444 LastReadID: d.GetLastReadId(), LastReadLost: d.GetLastReadLost(),
418 }445 }
419 for _, c := range d.GetCommits() {446 for _, c := range d.GetCommits() {
@@ -443,6 +470,9 @@ func toJSONDetail(d *ermite.GetMergeRequestResponse, withDiff bool) jsonMRDetail
443 for _, c := range d.GetComments() {470 for _, c := range d.GetComments() {
444 out.Comments = append(out.Comments, toJSONComment(c))471 out.Comments = append(out.Comments, toJSONComment(c))
445 }472 }
473+ for _, v := range d.GetVerdicts() {
474+ out.Verdicts = append(out.Verdicts, jsonAuthorVerdict{Author: v.GetAuthor(), Verdict: verdictName(v.GetVerdict()), CommentID: v.GetCommentId(), CommitID: v.GetCommitId(), CreatedAt: jsonTime(v.GetCreatedAt()), Stale: v.GetStale()})
475+ }
446 return out476 return out
447 }477 }
448 478
@@ -461,7 +491,7 @@ func newMRShowCmd(env *cliEnv) *cobra.Command {
461 "diff_base_id (the commit the diff starts from), stats (files_changed, additions, deletions),\n" +491 "diff_base_id (the commit the diff starts from), stats (files_changed, additions, deletions),\n" +
462 "diff_truncated, files (old_path, new_path, status, additions, deletions, binary, truncated and,\n" +492 "diff_truncated, files (old_path, new_path, status, additions, deletions, binary, truncated and,\n" +
463 "with --diff, hunks of lines with kind, old_number, new_number and text), comments, last_read_id\n" +493 "with --diff, hunks of lines with kind, old_number, new_number and text), comments, last_read_id\n" +
464- "and last_read_lost.",494+ "and last_read_lost; verdicts (author, verdict, comment_id, commit_id, created_at, stale).",
465 Args: cobra.ExactArgs(2),495 Args: cobra.ExactArgs(2),
466 ValidArgsFunction: completeMRRepoThenNumber(env),496 ValidArgsFunction: completeMRRepoThenNumber(env),
467 }497 }
@@ -529,6 +559,20 @@ func printMRDetail(w io.Writer, d *ermite.GetMergeRequestResponse, withDiff, sin
529 if m.GetState() == ermite.MergeRequestState_MERGE_REQUEST_STATE_MERGED {559 if m.GetState() == ermite.MergeRequestState_MERGE_REQUEST_STATE_MERGED {
530 pairs = append(pairs, [2]string{"merged", orDash(m.GetMergedCommitId()) + " (" + modeName(m.GetMergeMode()) + ")"})560 pairs = append(pairs, [2]string{"merged", orDash(m.GetMergedCommitId()) + " (" + modeName(m.GetMergeMode()) + ")"})
531 }561 }
562+ if len(d.GetVerdicts()) > 0 {
563+ var verdicts []string
564+ for _, v := range d.GetVerdicts() {
565+ label := "changes requested"
566+ if v.GetVerdict() == ermite.Verdict_VERDICT_APPROVE {
567+ label = "approved"
568+ }
569+ if v.GetStale() {
570+ label += " (stale)"
571+ }
572+ verdicts = append(verdicts, v.GetAuthor()+" "+label)
573+ }
574+ pairs = append(pairs, [2]string{"verdicts", strings.Join(verdicts, ", ")})
575+ }
532 if err := fields(w, pairs); err != nil {576 if err := fields(w, pairs); err != nil {
533 return err577 return err
534 }578 }
@@ -561,16 +605,46 @@ func printMRDetail(w io.Writer, d *ermite.GetMergeRequestResponse, withDiff, sin
561 }605 }
562 606
563 _, _ = fmt.Fprintf(w, "\ncomments (%d)\n", len(d.GetComments()))607 _, _ = fmt.Fprintf(w, "\ncomments (%d)\n", len(d.GetComments()))
564- for _, c := range d.GetComments() {608+ var ordered []*ermite.Comment
609+ for _, root := range d.GetComments() {
610+ if root.GetReplyTo() != "" {
611+ continue
612+ }
613+ ordered = append(ordered, root)
614+ for _, reply := range d.GetComments() {
615+ if reply.GetReplyTo() == root.GetId() {
616+ ordered = append(ordered, reply)
617+ }
618+ }
619+ }
620+ for _, c := range ordered {
565 where := "general"621 where := "general"
566 if c.GetPath() != "" {622 if c.GetPath() != "" {
567 where = fmt.Sprintf("%s:%d (%s)", clean(c.GetPath()), c.GetLine(), sideName(c.GetSide()))623 where = fmt.Sprintf("%s:%d (%s)", clean(c.GetPath()), c.GetLine(), sideName(c.GetSide()))
568 }624 }
625+ if c.GetStartLine() > 0 {
626+ where = fmt.Sprintf("%s:lines %d-%d (%s)", clean(c.GetPath()), c.GetStartLine(), c.GetLine(), sideName(c.GetSide()))
627+ }
569 mark := ""628 mark := ""
629+ if c.GetEditedAt() != nil {
630+ mark += " (edited)"
631+ }
632+ if c.GetDeleted() {
633+ mark += " [deleted]"
634+ }
635+ if c.GetVerdict() != ermite.Verdict_VERDICT_UNSPECIFIED {
636+ mark += " [" + verdictName(c.GetVerdict()) + "]"
637+ }
570 if c.GetResolved() {638 if c.GetResolved() {
571- mark = " [resolved]"639+ mark += " [resolved]"
572 }640 }
573- _, _ = fmt.Fprintf(w, " #%s %s, %s, %s%s\n%s\n", c.GetId(), clean(c.GetAuthor()), shortTime(c.GetCreatedAt(), "-"), where, mark, indentText(c.GetText()))641+ prefix := " "
642+ body := indentText(c.GetText())
643+ if c.GetReplyTo() != "" {
644+ prefix += " "
645+ body = " " + strings.ReplaceAll(body, "\n", "\n ")
646+ }
647+ _, _ = fmt.Fprintf(w, "%s#%s %s, %s, %s%s\n%s\n", prefix, c.GetId(), clean(c.GetAuthor()), shortTime(c.GetCreatedAt(), "-"), where, mark, body)
574 }648 }
575 if withDiff {649 if withDiff {
576 printDiff(w, d.GetDiff())650 printDiff(w, d.GetDiff())
@@ -643,12 +717,14 @@ func printDiff(w io.Writer, d *ermite.Diff) {
643 }717 }
644 718
645 func newMRCommentCmd(env *cliEnv) *cobra.Command {719 func newMRCommentCmd(env *cliEnv) *cobra.Command {
646- var message, path, side string720+ var message, path, side, line, reply string
647- var line int721+ var approve, requestChanges bool
648 cmd := &cobra.Command{722 cmd := &cobra.Command{
649 Use: "comment <repo> <number>",723 Use: "comment <repo> <number>",
650 Short: "Comment on a merge request",724 Short: "Comment on a merge request",
651- Long: "Add a plain-text comment (at most 8000 characters, no control characters except line breaks).\n" +725+ Long: "Add a Markdown note (at most 8000 characters); ^N refers to a note in this merge request.\n" +
726+ "--reply replies to a root note. --line accepts N or A-B. --approve and --request-changes\n" +
727+ "set a verdict (needs mr:review) and allow an empty message.\n" +
652 "-m - reads it from standard input. Without --path it is a general comment. With --path, --line and\n" +728 "-m - reads it from standard input. Without --path it is a general comment. With --path, --line and\n" +
653 "--side (default new) it is attached to a line of the current diff: the path as in `mr show --diff`\n" +729 "--side (default new) it is attached to a line of the current diff: the path as in `mr show --diff`\n" +
654 "(the new path, the old one for a deleted file), the line number on the old or new side; a line\n" +730 "(the new path, the old one for a deleted file), the line number on the old or new side; a line\n" +
@@ -656,11 +732,18 @@ func newMRCommentCmd(env *cliEnv) *cobra.Command {
656 Args: cobra.ExactArgs(2),732 Args: cobra.ExactArgs(2),
657 ValidArgsFunction: completeMRRepoThenNumber(env),733 ValidArgsFunction: completeMRRepoThenNumber(env),
658 }734 }
659- cmd.Flags().StringVarP(&message, "message", "m", "", "text of the comment, or - to read standard input (required)")735+ cmd.Flags().StringVarP(&message, "message", "m", "", "Markdown text, or - to read standard input (required unless setting a verdict)")
660 cmd.Flags().StringVar(&path, "path", "", "file the comment is attached to")736 cmd.Flags().StringVar(&path, "path", "", "file the comment is attached to")
661- cmd.Flags().IntVar(&line, "line", 0, "1-based line number on --side")737+ cmd.Flags().StringVar(&line, "line", "", "1-based line N or range A-B on --side")
738+ cmd.Flags().StringVar(&reply, "reply", "", "reply to root note (^12, #12 or 12)")
739+ cmd.Flags().BoolVar(&approve, "approve", false, "approve (needs mr:review)")
740+ cmd.Flags().BoolVar(&requestChanges, "request-changes", false, "request changes (needs mr:review)")
662 cmd.Flags().StringVar(&side, "side", sideNew, "side the line counts on: old or new")741 cmd.Flags().StringVar(&side, "side", sideNew, "side the line counts on: old or new")
663- _ = cmd.MarkFlagRequired("message")742+ cmd.MarkFlagsMutuallyExclusive("approve", "request-changes")
743+ for _, flag := range []string{"path", "line", "side", "approve", "request-changes"} {
744+ cmd.MarkFlagsMutuallyExclusive("reply", flag)
745+ }
746+ // A verdict note may omit its message.
664 _ = cmd.RegisterFlagCompletionFunc("side", func(*cobra.Command, []string, string) ([]string, cobra.ShellCompDirective) {747 _ = cmd.RegisterFlagCompletionFunc("side", func(*cobra.Command, []string, string) ([]string, cobra.ShellCompDirective) {
665 return []string{sideOld, sideNew}, cobra.ShellCompDirectiveNoFileComp748 return []string{sideOld, sideNew}, cobra.ShellCompDirectiveNoFileComp
666 })749 })
@@ -674,17 +757,36 @@ func newMRCommentCmd(env *cliEnv) *cobra.Command {
674 if err != nil {757 if err != nil {
675 return err758 return err
676 }759 }
760+ if !cmd.Flags().Changed("message") && !approve && !requestChanges {
761+ return errors.New("--message is required unless --approve or --request-changes is set")
762+ }
677 req := &ermite.AddCommentRequest{Repo: args[0], Number: n, Text: text}763 req := &ermite.AddCommentRequest{Repo: args[0], Number: n, Text: text}
764+ if approve {
765+ req.Verdict = ermite.Verdict_VERDICT_APPROVE
766+ }
767+ if requestChanges {
768+ req.Verdict = ermite.Verdict_VERDICT_CHANGES_REQUESTED
769+ }
770+ if cmd.Flags().Changed("reply") {
771+ req.ReplyTo, err = parseNoteID(reply)
772+ if err != nil {
773+ return err
774+ }
775+ }
678 switch {776 switch {
679 case path != "":777 case path != "":
680- if line < 1 {778+ if line == "" {
681 return errors.New("--line is required with --path: give the line number")779 return errors.New("--line is required with --path: give the line number")
682 }780 }
781+ req.StartLine, req.Line, err = parseLineRange(line)
782+ if err != nil {
783+ return err
784+ }
683 if req.Side, err = parseSide(side); err != nil {785 if req.Side, err = parseSide(side); err != nil {
684 return err786 return err
685 }787 }
686- req.Path, req.Line = path, int32(line) //nolint:gosec // a line number typed by a person788+ req.Path = path
687- case line != 0 || cmd.Flags().Changed("side"):789+ case cmd.Flags().Changed("line") || cmd.Flags().Changed("side"):
688 return errors.New("--line and --side need --path")790 return errors.New("--line and --side need --path")
689 }791 }
690 c, err := env.client(cmd)792 c, err := env.client(cmd)
@@ -704,6 +806,100 @@ func newMRCommentCmd(env *cliEnv) *cobra.Command {
704 return cmd806 return cmd
705 }807 }
706 808
809+func parseNoteID(s string) (string, error) {
810+ id := strings.TrimLeft(strings.TrimSpace(s), "^#")
811+ n, err := strconv.ParseInt(id, 10, 64)
812+ if err != nil || n < 1 {
813+ return "", fmt.Errorf("invalid note id %q: use ^12, #12 or 12", s)
814+ }
815+ return strconv.FormatInt(n, 10), nil
816+}
817+
818+func parseLineRange(s string) (start, end int32, err error) {
819+ parts := strings.Split(s, "-")
820+ invalid := fmt.Errorf("invalid --line %q: use N or A-B with 1 <= A < B", s)
821+ if len(parts) < 1 || len(parts) > 2 {
822+ return 0, 0, invalid
823+ }
824+ parse := func(v string) (int32, error) {
825+ n, e := strconv.ParseInt(v, 10, 32)
826+ if e != nil || n < 1 {
827+ return 0, invalid
828+ }
829+ return int32(n), nil
830+ }
831+ end, err = parse(parts[len(parts)-1])
832+ if err != nil {
833+ return 0, 0, err
834+ }
835+ if len(parts) == 2 {
836+ start, err = parse(parts[0])
837+ if err != nil || start >= end {
838+ return 0, 0, invalid
839+ }
840+ }
841+ return start, end, nil
842+}
843+
844+func newMREditCommentCmd(env *cliEnv) *cobra.Command { return newMRNoteMutationCmd(env, true) }
845+func newMRDeleteCommentCmd(env *cliEnv) *cobra.Command { return newMRNoteMutationCmd(env, false) }
846+
847+func newMRNoteMutationCmd(env *cliEnv, edit bool) *cobra.Command {
848+ verb := "delete"
849+ if edit {
850+ verb = "edit"
851+ }
852+ var message string
853+ cmd := &cobra.Command{Use: verb + "-comment <repo> <number> <comment-id>", Short: verb + " your own merge request note", Long: "Edit or delete your own note on an open merge request. Needs mr:write. Verdicts are unchanged." + jsonCommentHelp, Args: cobra.ExactArgs(3), ValidArgsFunction: completeMRRepoThenNumber(env)}
854+ if edit {
855+ cmd.Flags().StringVarP(&message, "message", "m", "", "Markdown text, or - to read standard input (required)")
856+ _ = cmd.MarkFlagRequired("message")
857+ }
858+ asJSON := addJSONFlag(cmd)
859+ cmd.RunE = func(cmd *cobra.Command, args []string) error {
860+ n, err := parseNumber(args[1])
861+ if err != nil {
862+ return err
863+ }
864+ id, err := parseNoteID(args[2])
865+ if err != nil {
866+ return err
867+ }
868+ text, err := readText(cmd, message)
869+ if err != nil {
870+ return err
871+ }
872+ c, err := env.client(cmd)
873+ if err != nil {
874+ return err
875+ }
876+ var note *ermite.Comment
877+ if edit {
878+ res, e := c.MergeRequests().EditComment(env.ctx(cmd), connect.NewRequest(&ermite.EditCommentRequest{Repo: args[0], Number: n, CommentId: id, Text: text}))
879+ if e != nil {
880+ return c.Describe(e)
881+ }
882+ note = res.Msg.GetComment()
883+ } else {
884+ res, e := c.MergeRequests().DeleteComment(env.ctx(cmd), connect.NewRequest(&ermite.DeleteCommentRequest{Repo: args[0], Number: n, CommentId: id}))
885+ if e != nil {
886+ return c.Describe(e)
887+ }
888+ note = res.Msg.GetComment()
889+ }
890+ if *asJSON {
891+ return writeJSON(cmd.OutOrStdout(), toJSONComment(note))
892+ }
893+ past := "deleted"
894+ if edit {
895+ past = "edited"
896+ }
897+ _, err = fmt.Fprintf(cmd.OutOrStdout(), "comment %s of %s!%d %s\n", clean(id), clean(args[0]), n, past)
898+ return err
899+ }
900+ return cmd
901+}
902+
707 func newMRResolveCmd(env *cliEnv) *cobra.Command {903 func newMRResolveCmd(env *cliEnv) *cobra.Command {
708 var undo bool904 var undo bool
709 cmd := &cobra.Command{905 cmd := &cobra.Command{
M internal/app/cli_mr_e2e_test.go +72 -0
@@ -420,3 +420,75 @@ func (w *watchProc) nextOfType(t *testing.T, typ string) eventOut {
420420 t.Fatalf("no %s event", typ)
421421 return eventOut{}
422422 }
423+
424+func TestMergeRequestNotesCLIEndToEnd(t *testing.T) {
425+ c := newCLIHarness(t)
426+ c.cli(t, adminToken, "repo", "create", "notes", "--visibility", "public").ok(t)
427+ wt := c.workTree("main", "README.md")
428+ c.mustGit(wt, "checkout", "-q", "-b", "feat/notes")
429+ c.writeFile(wt, "notes.txt", []byte("one\ntwo\nthree\nfour\n"))
430+ c.mustGit(wt, "push", "-q", c.urlAuth(adminToken, "/notes.git"), "main", "feat/notes")
431+ c.cli(t, adminToken, "mr", "create", "notes", "--source", "feat/notes", "--title", "Notes").ok(t)
432+ w := c.startWatch(t, adminToken)
433+ res := c.cli(t, adminToken, "mr", "comment", "notes", "1", "--path", "notes.txt", "--line", "1-3", "-m", "**range**", "--json").ok(t)
434+ root := decode[jsonComment](t, res.stdout)
435+ assert.EqualValues(t, 1, root.StartLine)
436+ assert.EqualValues(t, 3, root.Line)
437+ w.nextOfType(t, "comment_added")
438+ res = c.cli(t, adminToken, "mr", "comment", "notes", "1", "--reply", "^"+root.ID, "-m", "reply to ^"+root.ID, "--json").ok(t)
439+ reply := decode[jsonComment](t, res.stdout)
440+ assert.Equal(t, root.ID, reply.ReplyTo)
441+ assert.Equal(t, root.Path, reply.Path)
442+ assert.Equal(t, root.StartLine, reply.StartLine)
443+ assert.Equal(t, root.Line, reply.Line)
444+ w.nextOfType(t, "comment_added")
445+ require.Error(t, c.cli(t, adminToken, "mr", "comment", "notes", "1", "--reply", reply.ID, "-m", "nested").err)
446+ require.Error(t, c.cli(t, adminToken, "mr", "resolve", "notes", "1", reply.ID).err)
447+ writer := c.token([]domain.Scope{domain.ScopeRepoRead, domain.ScopeMRWrite})
448+ denied := c.cli(t, writer, "mr", "comment", "notes", "1", "--approve")
449+ require.Error(t, denied.err)
450+ assert.Contains(t, denied.err.Error(), "mr:review")
451+ res = c.cli(t, adminToken, "mr", "comment", "notes", "1", "--approve", "--json").ok(t)
452+ verdict := decode[jsonComment](t, res.stdout)
453+ assert.Equal(t, "approve", verdict.Verdict)
454+ assert.True(t, verdict.Resolved)
455+ assert.Empty(t, verdict.Text)
456+ w.nextOfType(t, "comment_added")
457+ require.Error(t, c.cli(t, writer, "mr", "edit-comment", "notes", "1", verdict.ID, "-m", "not mine").err)
458+ res = runCLI(t, c.vars(adminToken), "edited from stdin\n", "mr", "edit-comment", "notes", "1", "#"+reply.ID, "-m", "-", "--json").ok(t)
459+ edited := decode[jsonComment](t, res.stdout)
460+ assert.Equal(t, "edited from stdin", edited.Text)
461+ require.NotNil(t, edited.EditedAt)
462+ w.nextOfType(t, "comment_edited")
463+ res = c.cli(t, adminToken, "mr", "delete-comment", "notes", "1", verdict.ID, "--json").ok(t)
464+ deleted := decode[jsonComment](t, res.stdout)
465+ assert.True(t, deleted.Deleted)
466+ assert.Equal(t, "approve", deleted.Verdict)
467+ assert.Empty(t, deleted.Text)
468+ require.NotNil(t, deleted.EditedAt)
469+ w.nextOfType(t, "comment_deleted")
470+ c.cli(t, adminToken, "mr", "delete-comment", "notes", "1", verdict.ID).ok(t)
471+ c.cli(t, adminToken, "mr", "resolve", "notes", "1", root.ID).ok(t)
472+ w.nextOfType(t, "comment_resolved")
473+ res = c.cli(t, "", "mr", "show", "notes", "1", "--json").ok(t)
474+ detail := decode[jsonMRDetail](t, res.stdout)
475+ require.Len(t, detail.Comments, 3)
476+ require.Len(t, detail.Verdicts, 1)
477+ assert.Equal(t, verdict.ID, detail.Verdicts[0].CommentID)
478+ assert.Equal(t, "approve", detail.Verdicts[0].Verdict)
479+ assert.False(t, detail.Verdicts[0].Stale)
480+ res = c.cli(t, "", "mr", "show", "notes", "1").ok(t)
481+ assert.Contains(t, res.stdout, "lines 1-3")
482+ assert.Contains(t, res.stdout, "(edited)")
483+ assert.Contains(t, res.stdout, "[deleted]")
484+ assert.Contains(t, res.stdout, "approved")
485+ assert.Contains(t, res.stdout, " #"+reply.ID)
486+ c.cli(t, adminToken, "mr", "comment", "notes", "1", "--request-changes", "-m", "needs work").ok(t)
487+ res = c.cli(t, "", "mr", "show", "notes", "1", "--json").ok(t)
488+ detail = decode[jsonMRDetail](t, res.stdout)
489+ require.Len(t, detail.Verdicts, 1)
490+ assert.Equal(t, "changes_requested", detail.Verdicts[0].Verdict)
491+ c.cli(t, adminToken, "mr", "close", "notes", "1").ok(t)
492+ require.Error(t, c.cli(t, adminToken, "mr", "edit-comment", "notes", "1", root.ID, "-m", "late").err)
493+ require.Error(t, c.cli(t, adminToken, "mr", "delete-comment", "notes", "1", root.ID).err)
494+}
@@ -420,3 +420,75 @@ func (w *watchProc) nextOfType(t *testing.T, typ string) eventOut {
420 t.Fatalf("no %s event", typ)420 t.Fatalf("no %s event", typ)
421 return eventOut{}421 return eventOut{}
422 }422 }
423+
424+func TestMergeRequestNotesCLIEndToEnd(t *testing.T) {
425+ c := newCLIHarness(t)
426+ c.cli(t, adminToken, "repo", "create", "notes", "--visibility", "public").ok(t)
427+ wt := c.workTree("main", "README.md")
428+ c.mustGit(wt, "checkout", "-q", "-b", "feat/notes")
429+ c.writeFile(wt, "notes.txt", []byte("one\ntwo\nthree\nfour\n"))
430+ c.mustGit(wt, "push", "-q", c.urlAuth(adminToken, "/notes.git"), "main", "feat/notes")
431+ c.cli(t, adminToken, "mr", "create", "notes", "--source", "feat/notes", "--title", "Notes").ok(t)
432+ w := c.startWatch(t, adminToken)
433+ res := c.cli(t, adminToken, "mr", "comment", "notes", "1", "--path", "notes.txt", "--line", "1-3", "-m", "**range**", "--json").ok(t)
434+ root := decode[jsonComment](t, res.stdout)
435+ assert.EqualValues(t, 1, root.StartLine)
436+ assert.EqualValues(t, 3, root.Line)
437+ w.nextOfType(t, "comment_added")
438+ res = c.cli(t, adminToken, "mr", "comment", "notes", "1", "--reply", "^"+root.ID, "-m", "reply to ^"+root.ID, "--json").ok(t)
439+ reply := decode[jsonComment](t, res.stdout)
440+ assert.Equal(t, root.ID, reply.ReplyTo)
441+ assert.Equal(t, root.Path, reply.Path)
442+ assert.Equal(t, root.StartLine, reply.StartLine)
443+ assert.Equal(t, root.Line, reply.Line)
444+ w.nextOfType(t, "comment_added")
445+ require.Error(t, c.cli(t, adminToken, "mr", "comment", "notes", "1", "--reply", reply.ID, "-m", "nested").err)
446+ require.Error(t, c.cli(t, adminToken, "mr", "resolve", "notes", "1", reply.ID).err)
447+ writer := c.token([]domain.Scope{domain.ScopeRepoRead, domain.ScopeMRWrite})
448+ denied := c.cli(t, writer, "mr", "comment", "notes", "1", "--approve")
449+ require.Error(t, denied.err)
450+ assert.Contains(t, denied.err.Error(), "mr:review")
451+ res = c.cli(t, adminToken, "mr", "comment", "notes", "1", "--approve", "--json").ok(t)
452+ verdict := decode[jsonComment](t, res.stdout)
453+ assert.Equal(t, "approve", verdict.Verdict)
454+ assert.True(t, verdict.Resolved)
455+ assert.Empty(t, verdict.Text)
456+ w.nextOfType(t, "comment_added")
457+ require.Error(t, c.cli(t, writer, "mr", "edit-comment", "notes", "1", verdict.ID, "-m", "not mine").err)
458+ res = runCLI(t, c.vars(adminToken), "edited from stdin\n", "mr", "edit-comment", "notes", "1", "#"+reply.ID, "-m", "-", "--json").ok(t)
459+ edited := decode[jsonComment](t, res.stdout)
460+ assert.Equal(t, "edited from stdin", edited.Text)
461+ require.NotNil(t, edited.EditedAt)
462+ w.nextOfType(t, "comment_edited")
463+ res = c.cli(t, adminToken, "mr", "delete-comment", "notes", "1", verdict.ID, "--json").ok(t)
464+ deleted := decode[jsonComment](t, res.stdout)
465+ assert.True(t, deleted.Deleted)
466+ assert.Equal(t, "approve", deleted.Verdict)
467+ assert.Empty(t, deleted.Text)
468+ require.NotNil(t, deleted.EditedAt)
469+ w.nextOfType(t, "comment_deleted")
470+ c.cli(t, adminToken, "mr", "delete-comment", "notes", "1", verdict.ID).ok(t)
471+ c.cli(t, adminToken, "mr", "resolve", "notes", "1", root.ID).ok(t)
472+ w.nextOfType(t, "comment_resolved")
473+ res = c.cli(t, "", "mr", "show", "notes", "1", "--json").ok(t)
474+ detail := decode[jsonMRDetail](t, res.stdout)
475+ require.Len(t, detail.Comments, 3)
476+ require.Len(t, detail.Verdicts, 1)
477+ assert.Equal(t, verdict.ID, detail.Verdicts[0].CommentID)
478+ assert.Equal(t, "approve", detail.Verdicts[0].Verdict)
479+ assert.False(t, detail.Verdicts[0].Stale)
480+ res = c.cli(t, "", "mr", "show", "notes", "1").ok(t)
481+ assert.Contains(t, res.stdout, "lines 1-3")
482+ assert.Contains(t, res.stdout, "(edited)")
483+ assert.Contains(t, res.stdout, "[deleted]")
484+ assert.Contains(t, res.stdout, "approved")
485+ assert.Contains(t, res.stdout, " #"+reply.ID)
486+ c.cli(t, adminToken, "mr", "comment", "notes", "1", "--request-changes", "-m", "needs work").ok(t)
487+ res = c.cli(t, "", "mr", "show", "notes", "1", "--json").ok(t)
488+ detail = decode[jsonMRDetail](t, res.stdout)
489+ require.Len(t, detail.Verdicts, 1)
490+ assert.Equal(t, "changes_requested", detail.Verdicts[0].Verdict)
491+ c.cli(t, adminToken, "mr", "close", "notes", "1").ok(t)
492+ require.Error(t, c.cli(t, adminToken, "mr", "edit-comment", "notes", "1", root.ID, "-m", "late").err)
493+ require.Error(t, c.cli(t, adminToken, "mr", "delete-comment", "notes", "1", root.ID).err)
494+}
M internal/app/cli_mr_test.go +81 -0
@@ -381,3 +381,84 @@ func TestSigningKeyShowOutput(t *testing.T) {
381381 res = runCLI(t, envFor(url), "", "signing-key", "show", "--json").ok(t)
382382 assert.JSONEq(t, `{"public_key":"ssh-ed25519 AAAAC3Nz","fingerprint":"SHA256:abc","committer_name":"ermite","committer_email":"ermite@git.example.org","allowed_signers":"ermite@git.example.org ssh-ed25519 AAAAC3Nz"}`, res.stdout)
383383 }
384+
385+func TestNoteRangesAndIDs(t *testing.T) {
386+ for _, tc := range []struct {
387+ input string
388+ start, end int32
389+ }{{"1", 0, 1}, {"10-24", 10, 24}, {"2147483647", 0, 2147483647}} {
390+ start, end, err := parseLineRange(tc.input)
391+ require.NoError(t, err)
392+ assert.Equal(t, tc.start, start)
393+ assert.Equal(t, tc.end, end)
394+ }
395+ for _, input := range []string{"", "0", "-1", "1-1", "24-10", "0-2", "1-2-3", "2147483648", "a", "1-"} {
396+ _, _, err := parseLineRange(input)
397+ require.Error(t, err, input)
398+ }
399+ for _, input := range []string{"12", "^12", "#12"} {
400+ got, err := parseNoteID(input)
401+ require.NoError(t, err)
402+ assert.Equal(t, "12", got)
403+ }
404+}
405+
406+func TestNoteFlags(t *testing.T) {
407+ f := &fakeAPI{addComment: func(r *ermite.AddCommentRequest) (*ermite.AddCommentResponse, error) {
408+ return &ermite.AddCommentResponse{Comment: &ermite.Comment{Id: "1", ReplyTo: r.GetReplyTo(), StartLine: r.GetStartLine(), Line: r.GetLine(), Verdict: r.GetVerdict()}}, nil
409+ }}
410+ url := f.serve(t)
411+ for _, tc := range []struct {
412+ flags []string
413+ reply string
414+ start int32
415+ verdict string
416+ }{
417+ {[]string{"--reply", "^12", "-m", "reply"}, "12", 0, ""},
418+ {[]string{"--path", "x", "--line", "10-24", "-m", "range"}, "", 10, ""},
419+ {[]string{"--approve"}, "", 0, "approve"},
420+ {[]string{"--request-changes"}, "", 0, "changes_requested"},
421+ } {
422+ res := runCLI(t, envFor(url), "", append([]string{"mr", "comment", "alpha", "1", "--json"}, tc.flags...)...).ok(t)
423+ got := decode[jsonComment](t, res.stdout)
424+ assert.Equal(t, tc.reply, got.ReplyTo)
425+ assert.Equal(t, tc.start, got.StartLine)
426+ assert.Equal(t, tc.verdict, got.Verdict)
427+ }
428+ for _, flags := range [][]string{{"--approve", "--request-changes"}, {"--reply", "1", "--path", "x"}, {"--reply", "1", "--line", "2"}, {"--reply", "1", "--side", "new"}, {"--reply", "1", "--approve"}, {"--reply", "1", "--request-changes"}} {
429+ require.Error(t, runCLI(t, envFor(url), "", append([]string{"mr", "comment", "alpha", "1", "-m", "x"}, flags...)...).err)
430+ }
431+}
432+
433+func TestNoteDetailOutput(t *testing.T) {
434+ d := &ermite.GetMergeRequestResponse{MergeRequest: mrOne, Comments: []*ermite.Comment{
435+ {Id: "1", Path: "x", StartLine: 10, Line: 24, Text: "root"},
436+ {Id: "3", Text: "other", Deleted: true},
437+ {Id: "2", ReplyTo: "1", Text: "reply", EditedAt: ts("2026-10-10T12:00:00Z")},
438+ }, Verdicts: []*ermite.AuthorVerdict{
439+ {Author: "owner", Verdict: ermite.Verdict_VERDICT_APPROVE, CommentId: "1", Stale: true},
440+ {Author: "bot", Verdict: ermite.Verdict_VERDICT_CHANGES_REQUESTED, CommentId: "3"},
441+ }}
442+ var out strings.Builder
443+ require.NoError(t, printMRDetail(&out, d, false, false))
444+ text := out.String()
445+ assert.Contains(t, text, "owner approved (stale), bot changes requested")
446+ assert.Contains(t, text, "lines 10-24")
447+ assert.Contains(t, text, "(edited)")
448+ assert.Contains(t, text, "[deleted]")
449+ assert.Less(t, strings.Index(text, " #2"), strings.Index(text, " #3"))
450+ got := toJSONDetail(d, false)
451+ require.Len(t, got.Verdicts, 2)
452+ assert.True(t, got.Verdicts[0].Stale)
453+ assert.Equal(t, "changes_requested", got.Verdicts[1].Verdict)
454+ assert.Equal(t, "1", got.Comments[2].ReplyTo)
455+ assert.NotNil(t, got.Comments[2].EditedAt)
456+}
457+
458+func TestMRReviewScopeCompletion(t *testing.T) {
459+ cmd := newTokenCreateCmd(&cliEnv{})
460+ complete, ok := cmd.GetFlagCompletionFunc("scope")
461+ require.True(t, ok)
462+ values, _ := complete(cmd, nil, "")
463+ assert.Contains(t, values, "mr:review")
464+}
@@ -381,3 +381,84 @@ func TestSigningKeyShowOutput(t *testing.T) {
381 res = runCLI(t, envFor(url), "", "signing-key", "show", "--json").ok(t)381 res = runCLI(t, envFor(url), "", "signing-key", "show", "--json").ok(t)
382 assert.JSONEq(t, `{"public_key":"ssh-ed25519 AAAAC3Nz","fingerprint":"SHA256:abc","committer_name":"ermite","committer_email":"ermite@git.example.org","allowed_signers":"ermite@git.example.org ssh-ed25519 AAAAC3Nz"}`, res.stdout)382 assert.JSONEq(t, `{"public_key":"ssh-ed25519 AAAAC3Nz","fingerprint":"SHA256:abc","committer_name":"ermite","committer_email":"ermite@git.example.org","allowed_signers":"ermite@git.example.org ssh-ed25519 AAAAC3Nz"}`, res.stdout)
383 }383 }
384+
385+func TestNoteRangesAndIDs(t *testing.T) {
386+ for _, tc := range []struct {
387+ input string
388+ start, end int32
389+ }{{"1", 0, 1}, {"10-24", 10, 24}, {"2147483647", 0, 2147483647}} {
390+ start, end, err := parseLineRange(tc.input)
391+ require.NoError(t, err)
392+ assert.Equal(t, tc.start, start)
393+ assert.Equal(t, tc.end, end)
394+ }
395+ for _, input := range []string{"", "0", "-1", "1-1", "24-10", "0-2", "1-2-3", "2147483648", "a", "1-"} {
396+ _, _, err := parseLineRange(input)
397+ require.Error(t, err, input)
398+ }
399+ for _, input := range []string{"12", "^12", "#12"} {
400+ got, err := parseNoteID(input)
401+ require.NoError(t, err)
402+ assert.Equal(t, "12", got)
403+ }
404+}
405+
406+func TestNoteFlags(t *testing.T) {
407+ f := &fakeAPI{addComment: func(r *ermite.AddCommentRequest) (*ermite.AddCommentResponse, error) {
408+ return &ermite.AddCommentResponse{Comment: &ermite.Comment{Id: "1", ReplyTo: r.GetReplyTo(), StartLine: r.GetStartLine(), Line: r.GetLine(), Verdict: r.GetVerdict()}}, nil
409+ }}
410+ url := f.serve(t)
411+ for _, tc := range []struct {
412+ flags []string
413+ reply string
414+ start int32
415+ verdict string
416+ }{
417+ {[]string{"--reply", "^12", "-m", "reply"}, "12", 0, ""},
418+ {[]string{"--path", "x", "--line", "10-24", "-m", "range"}, "", 10, ""},
419+ {[]string{"--approve"}, "", 0, "approve"},
420+ {[]string{"--request-changes"}, "", 0, "changes_requested"},
421+ } {
422+ res := runCLI(t, envFor(url), "", append([]string{"mr", "comment", "alpha", "1", "--json"}, tc.flags...)...).ok(t)
423+ got := decode[jsonComment](t, res.stdout)
424+ assert.Equal(t, tc.reply, got.ReplyTo)
425+ assert.Equal(t, tc.start, got.StartLine)
426+ assert.Equal(t, tc.verdict, got.Verdict)
427+ }
428+ for _, flags := range [][]string{{"--approve", "--request-changes"}, {"--reply", "1", "--path", "x"}, {"--reply", "1", "--line", "2"}, {"--reply", "1", "--side", "new"}, {"--reply", "1", "--approve"}, {"--reply", "1", "--request-changes"}} {
429+ require.Error(t, runCLI(t, envFor(url), "", append([]string{"mr", "comment", "alpha", "1", "-m", "x"}, flags...)...).err)
430+ }
431+}
432+
433+func TestNoteDetailOutput(t *testing.T) {
434+ d := &ermite.GetMergeRequestResponse{MergeRequest: mrOne, Comments: []*ermite.Comment{
435+ {Id: "1", Path: "x", StartLine: 10, Line: 24, Text: "root"},
436+ {Id: "3", Text: "other", Deleted: true},
437+ {Id: "2", ReplyTo: "1", Text: "reply", EditedAt: ts("2026-10-10T12:00:00Z")},
438+ }, Verdicts: []*ermite.AuthorVerdict{
439+ {Author: "owner", Verdict: ermite.Verdict_VERDICT_APPROVE, CommentId: "1", Stale: true},
440+ {Author: "bot", Verdict: ermite.Verdict_VERDICT_CHANGES_REQUESTED, CommentId: "3"},
441+ }}
442+ var out strings.Builder
443+ require.NoError(t, printMRDetail(&out, d, false, false))
444+ text := out.String()
445+ assert.Contains(t, text, "owner approved (stale), bot changes requested")
446+ assert.Contains(t, text, "lines 10-24")
447+ assert.Contains(t, text, "(edited)")
448+ assert.Contains(t, text, "[deleted]")
449+ assert.Less(t, strings.Index(text, " #2"), strings.Index(text, " #3"))
450+ got := toJSONDetail(d, false)
451+ require.Len(t, got.Verdicts, 2)
452+ assert.True(t, got.Verdicts[0].Stale)
453+ assert.Equal(t, "changes_requested", got.Verdicts[1].Verdict)
454+ assert.Equal(t, "1", got.Comments[2].ReplyTo)
455+ assert.NotNil(t, got.Comments[2].EditedAt)
456+}
457+
458+func TestMRReviewScopeCompletion(t *testing.T) {
459+ cmd := newTokenCreateCmd(&cliEnv{})
460+ complete, ok := cmd.GetFlagCompletionFunc("scope")
461+ require.True(t, ok)
462+ values, _ := complete(cmd, nil, "")
463+ assert.Contains(t, values, "mr:review")
464+}
M internal/app/cli_token.go +1 -1
@@ -86,7 +86,7 @@ func newTokenCreateCmd(env *cliEnv) *cobra.Command {
8686 "or store it right away); details go to standard error. With --json, standard output is one\n" +
8787 "object holding the token fields and the secret.\n\n" +
8888 "What the token may do is chosen with --preset and/or --scope (they add up); there is no default:\n" +
89- " cli every scope, for your own daily use\n" +
89+ " cli every scope, including mr:review for verdicts, for your own daily use\n" +
9090 " agent repo:write, mr:write\n" +
9191 " runner runner:register (a CI runner host)\n" +
9292 " ci repo:read\n" +
@@ -86,7 +86,7 @@ func newTokenCreateCmd(env *cliEnv) *cobra.Command {
86 "or store it right away); details go to standard error. With --json, standard output is one\n" +86 "or store it right away); details go to standard error. With --json, standard output is one\n" +
87 "object holding the token fields and the secret.\n\n" +87 "object holding the token fields and the secret.\n\n" +
88 "What the token may do is chosen with --preset and/or --scope (they add up); there is no default:\n" +88 "What the token may do is chosen with --preset and/or --scope (they add up); there is no default:\n" +
89- " cli every scope, for your own daily use\n" +89+ " cli every scope, including mr:review for verdicts, for your own daily use\n" +
90 " agent repo:write, mr:write\n" +90 " agent repo:write, mr:write\n" +
91 " runner runner:register (a CI runner host)\n" +91 " runner runner:register (a CI runner host)\n" +
92 " ci repo:read\n" +92 " ci repo:read\n" +
M internal/app/cli_watch.go +21 -8
@@ -45,10 +45,13 @@ const (
4545 eventPushed = "pushed"
4646 eventGap = "gap"
4747
48- eventMROpened = "merge_request_opened"
49- eventMRMerged = "merge_request_merged"
50- eventMRClosed = "merge_request_closed"
51- eventCommentAdded = "comment_added"
48+ eventMROpened = "merge_request_opened"
49+ eventMRMerged = "merge_request_merged"
50+ eventMRClosed = "merge_request_closed"
51+ eventCommentAdded = "comment_added"
52+ eventCommentEdited = "comment_edited"
53+ eventCommentDeleted = "comment_deleted"
54+ eventCommentResolved = "comment_resolved"
5255 )
5356
5457 // toJSONEvent maps an event; ok is false for kinds this client does not know,
@@ -82,6 +85,15 @@ func toJSONEvent(ev *ermite.Event) (jsonEvent, bool) {
8285 case *ermite.Event_CommentAdded:
8386 c := toJSONComment(k.CommentAdded.GetComment())
8487 out.Type, out.Repo, out.Number, out.Comment = eventCommentAdded, k.CommentAdded.GetRepo(), k.CommentAdded.GetNumber(), &c
88+ case *ermite.Event_CommentEdited:
89+ c := toJSONComment(k.CommentEdited.GetComment())
90+ out.Type, out.Repo, out.Number, out.Comment = eventCommentEdited, k.CommentEdited.GetRepo(), k.CommentEdited.GetNumber(), &c
91+ case *ermite.Event_CommentDeleted:
92+ c := toJSONComment(k.CommentDeleted.GetComment())
93+ out.Type, out.Repo, out.Number, out.Comment = eventCommentDeleted, k.CommentDeleted.GetRepo(), k.CommentDeleted.GetNumber(), &c
94+ case *ermite.Event_CommentResolved:
95+ c := toJSONComment(k.CommentResolved.GetComment())
96+ out.Type, out.Repo, out.Number, out.Comment = eventCommentResolved, k.CommentResolved.GetRepo(), k.CommentResolved.GetNumber(), &c
8597 case *ermite.Event_Gap:
8698 out.Type = eventGap
8799 default:
@@ -112,7 +124,7 @@ func eventLine(e jsonEvent) string {
112124 detail = e.Repo + " " + strings.Join(parts, ", ")
113125 case eventMROpened, eventMRMerged, eventMRClosed:
114126 detail = fmt.Sprintf("%s!%d %s -> %s", e.Repo, e.MergeRequest.Number, e.MergeRequest.SourceBranch, e.MergeRequest.TargetBranch)
115- case eventCommentAdded:
127+ case eventCommentAdded, eventCommentEdited, eventCommentDeleted, eventCommentResolved:
116128 detail = fmt.Sprintf("%s!%d by %s", e.Repo, e.Number, e.Comment.Author)
117129 case eventGap:
118130 detail = "events were lost, reload what you display"
@@ -142,17 +154,18 @@ func newWatchCmd(env *cliEnv) *cobra.Command {
142154 Use: "watch [repo]",
143155 Short: "Print repository events as they happen",
144156 Long: "Print one line per event (repository created, updated or deleted, refs pushed, merge requests\n" +
145- "opened, merged or closed, comments added) for every\n" +
157+ "opened, merged or closed, notes added, edited, deleted or resolved) for every\n" +
146158 "repository you can see, or only for [repo]. It runs until interrupted. When the connection drops it\n" +
147159 "reconnects with a growing delay and resumes after the last event it printed; if the server no longer\n" +
148160 "has that event it prints a gap event: reload what you display. Connection notices go to standard\n" +
149161 "error.\n\n" +
150162 "--json prints one JSON object per line (NDJSON). Fields: id, time (RFC 3339 UTC), type\n" +
151163 "(repo_created, repo_updated, repo_deleted, pushed, merge_request_opened, merge_request_merged,\n" +
152- "merge_request_closed, comment_added or gap) and, depending on the type, repo (name),\n" +
164+ "merge_request_closed, comment_added, comment_edited, comment_deleted, comment_resolved or gap)\n" +
165+ "and, depending on the type, repo (name),\n" +
153166 "repository (the repository fields of `repo show --json`), ref_updates (array of ref, old_id,\n" +
154167 "new_id; an empty id means the ref was created or deleted), merge_request (the fields of\n" +
155- "`mr show --json`) and, for comment_added, number (of the merge request) and comment. Unknown\n" +
168+ "`mr show --json`) and, for comment events, number (of the merge request) and comment. Unknown\n" +
156169 "event types are skipped.",
157170 Args: cobra.MaximumNArgs(1),
158171 ValidArgsFunction: completeRepoNames(env),
@@ -45,10 +45,13 @@ const (
45 eventPushed = "pushed"45 eventPushed = "pushed"
46 eventGap = "gap"46 eventGap = "gap"
47 47
48- eventMROpened = "merge_request_opened"48+ eventMROpened = "merge_request_opened"
49- eventMRMerged = "merge_request_merged"49+ eventMRMerged = "merge_request_merged"
50- eventMRClosed = "merge_request_closed"50+ eventMRClosed = "merge_request_closed"
51- eventCommentAdded = "comment_added"51+ eventCommentAdded = "comment_added"
52+ eventCommentEdited = "comment_edited"
53+ eventCommentDeleted = "comment_deleted"
54+ eventCommentResolved = "comment_resolved"
52 )55 )
53 56
54 // toJSONEvent maps an event; ok is false for kinds this client does not know,57 // toJSONEvent maps an event; ok is false for kinds this client does not know,
@@ -82,6 +85,15 @@ func toJSONEvent(ev *ermite.Event) (jsonEvent, bool) {
82 case *ermite.Event_CommentAdded:85 case *ermite.Event_CommentAdded:
83 c := toJSONComment(k.CommentAdded.GetComment())86 c := toJSONComment(k.CommentAdded.GetComment())
84 out.Type, out.Repo, out.Number, out.Comment = eventCommentAdded, k.CommentAdded.GetRepo(), k.CommentAdded.GetNumber(), &c87 out.Type, out.Repo, out.Number, out.Comment = eventCommentAdded, k.CommentAdded.GetRepo(), k.CommentAdded.GetNumber(), &c
88+ case *ermite.Event_CommentEdited:
89+ c := toJSONComment(k.CommentEdited.GetComment())
90+ out.Type, out.Repo, out.Number, out.Comment = eventCommentEdited, k.CommentEdited.GetRepo(), k.CommentEdited.GetNumber(), &c
91+ case *ermite.Event_CommentDeleted:
92+ c := toJSONComment(k.CommentDeleted.GetComment())
93+ out.Type, out.Repo, out.Number, out.Comment = eventCommentDeleted, k.CommentDeleted.GetRepo(), k.CommentDeleted.GetNumber(), &c
94+ case *ermite.Event_CommentResolved:
95+ c := toJSONComment(k.CommentResolved.GetComment())
96+ out.Type, out.Repo, out.Number, out.Comment = eventCommentResolved, k.CommentResolved.GetRepo(), k.CommentResolved.GetNumber(), &c
85 case *ermite.Event_Gap:97 case *ermite.Event_Gap:
86 out.Type = eventGap98 out.Type = eventGap
87 default:99 default:
@@ -112,7 +124,7 @@ func eventLine(e jsonEvent) string {
112 detail = e.Repo + " " + strings.Join(parts, ", ")124 detail = e.Repo + " " + strings.Join(parts, ", ")
113 case eventMROpened, eventMRMerged, eventMRClosed:125 case eventMROpened, eventMRMerged, eventMRClosed:
114 detail = fmt.Sprintf("%s!%d %s -> %s", e.Repo, e.MergeRequest.Number, e.MergeRequest.SourceBranch, e.MergeRequest.TargetBranch)126 detail = fmt.Sprintf("%s!%d %s -> %s", e.Repo, e.MergeRequest.Number, e.MergeRequest.SourceBranch, e.MergeRequest.TargetBranch)
115- case eventCommentAdded:127+ case eventCommentAdded, eventCommentEdited, eventCommentDeleted, eventCommentResolved:
116 detail = fmt.Sprintf("%s!%d by %s", e.Repo, e.Number, e.Comment.Author)128 detail = fmt.Sprintf("%s!%d by %s", e.Repo, e.Number, e.Comment.Author)
117 case eventGap:129 case eventGap:
118 detail = "events were lost, reload what you display"130 detail = "events were lost, reload what you display"
@@ -142,17 +154,18 @@ func newWatchCmd(env *cliEnv) *cobra.Command {
142 Use: "watch [repo]",154 Use: "watch [repo]",
143 Short: "Print repository events as they happen",155 Short: "Print repository events as they happen",
144 Long: "Print one line per event (repository created, updated or deleted, refs pushed, merge requests\n" +156 Long: "Print one line per event (repository created, updated or deleted, refs pushed, merge requests\n" +
145- "opened, merged or closed, comments added) for every\n" +157+ "opened, merged or closed, notes added, edited, deleted or resolved) for every\n" +
146 "repository you can see, or only for [repo]. It runs until interrupted. When the connection drops it\n" +158 "repository you can see, or only for [repo]. It runs until interrupted. When the connection drops it\n" +
147 "reconnects with a growing delay and resumes after the last event it printed; if the server no longer\n" +159 "reconnects with a growing delay and resumes after the last event it printed; if the server no longer\n" +
148 "has that event it prints a gap event: reload what you display. Connection notices go to standard\n" +160 "has that event it prints a gap event: reload what you display. Connection notices go to standard\n" +
149 "error.\n\n" +161 "error.\n\n" +
150 "--json prints one JSON object per line (NDJSON). Fields: id, time (RFC 3339 UTC), type\n" +162 "--json prints one JSON object per line (NDJSON). Fields: id, time (RFC 3339 UTC), type\n" +
151 "(repo_created, repo_updated, repo_deleted, pushed, merge_request_opened, merge_request_merged,\n" +163 "(repo_created, repo_updated, repo_deleted, pushed, merge_request_opened, merge_request_merged,\n" +
152- "merge_request_closed, comment_added or gap) and, depending on the type, repo (name),\n" +164+ "merge_request_closed, comment_added, comment_edited, comment_deleted, comment_resolved or gap)\n" +
165+ "and, depending on the type, repo (name),\n" +
153 "repository (the repository fields of `repo show --json`), ref_updates (array of ref, old_id,\n" +166 "repository (the repository fields of `repo show --json`), ref_updates (array of ref, old_id,\n" +
154 "new_id; an empty id means the ref was created or deleted), merge_request (the fields of\n" +167 "new_id; an empty id means the ref was created or deleted), merge_request (the fields of\n" +
155- "`mr show --json`) and, for comment_added, number (of the merge request) and comment. Unknown\n" +168+ "`mr show --json`) and, for comment events, number (of the merge request) and comment. Unknown\n" +
156 "event types are skipped.",169 "event types are skipped.",
157 Args: cobra.MaximumNArgs(1),170 Args: cobra.MaximumNArgs(1),
158 ValidArgsFunction: completeRepoNames(env),171 ValidArgsFunction: completeRepoNames(env),
M internal/app/cli_watch_test.go +24 -0
@@ -199,3 +199,27 @@ func TestWatchMergeRequestEvents(t *testing.T) {
199199 assert.Equal(t, "merge_request_merged", decode[eventOut](t, lines[2]).Type)
200200 assert.Equal(t, "merge_request_closed", decode[eventOut](t, lines[3]).Type)
201201 }
202+
203+func TestNoteWatchEvents(t *testing.T) {
204+ note := &ermite.Comment{Id: "4", ReplyTo: "2", StartLine: 10, Line: 24, Verdict: ermite.Verdict_VERDICT_APPROVE, EditedAt: ts("2026-10-10T12:00:00Z"), Deleted: true, Author: "owner"}
205+ events := []*ermite.Event{
206+ {Kind: &ermite.Event_CommentEdited{CommentEdited: &ermite.CommentEdited{Repo: "alpha", Number: 1, Comment: note}}},
207+ {Kind: &ermite.Event_CommentDeleted{CommentDeleted: &ermite.CommentDeleted{Repo: "alpha", Number: 1, Comment: note}}},
208+ {Kind: &ermite.Event_CommentResolved{CommentResolved: &ermite.CommentResolved{Repo: "alpha", Number: 1, Comment: note}}},
209+ }
210+ for i, ev := range events {
211+ got, ok := toJSONEvent(ev)
212+ require.True(t, ok)
213+ assert.Equal(t, []string{"comment_edited", "comment_deleted", "comment_resolved"}[i], got.Type)
214+ assert.Equal(t, "alpha", got.Repo)
215+ assert.EqualValues(t, 1, got.Number)
216+ require.NotNil(t, got.Comment)
217+ assert.True(t, got.Comment.Deleted)
218+ assert.Equal(t, "2", got.Comment.ReplyTo)
219+ assert.Equal(t, "approve", got.Comment.Verdict)
220+ assert.Contains(t, eventLine(got), got.Type+" alpha!1 by owner")
221+ raw, err := json.Marshal(got)
222+ require.NoError(t, err)
223+ assert.Contains(t, string(raw), `"edited_at":"2026-10-10T12:00:00Z"`)
224+ }
225+}
@@ -199,3 +199,27 @@ func TestWatchMergeRequestEvents(t *testing.T) {
199 assert.Equal(t, "merge_request_merged", decode[eventOut](t, lines[2]).Type)199 assert.Equal(t, "merge_request_merged", decode[eventOut](t, lines[2]).Type)
200 assert.Equal(t, "merge_request_closed", decode[eventOut](t, lines[3]).Type)200 assert.Equal(t, "merge_request_closed", decode[eventOut](t, lines[3]).Type)
201 }201 }
202+
203+func TestNoteWatchEvents(t *testing.T) {
204+ note := &ermite.Comment{Id: "4", ReplyTo: "2", StartLine: 10, Line: 24, Verdict: ermite.Verdict_VERDICT_APPROVE, EditedAt: ts("2026-10-10T12:00:00Z"), Deleted: true, Author: "owner"}
205+ events := []*ermite.Event{
206+ {Kind: &ermite.Event_CommentEdited{CommentEdited: &ermite.CommentEdited{Repo: "alpha", Number: 1, Comment: note}}},
207+ {Kind: &ermite.Event_CommentDeleted{CommentDeleted: &ermite.CommentDeleted{Repo: "alpha", Number: 1, Comment: note}}},
208+ {Kind: &ermite.Event_CommentResolved{CommentResolved: &ermite.CommentResolved{Repo: "alpha", Number: 1, Comment: note}}},
209+ }
210+ for i, ev := range events {
211+ got, ok := toJSONEvent(ev)
212+ require.True(t, ok)
213+ assert.Equal(t, []string{"comment_edited", "comment_deleted", "comment_resolved"}[i], got.Type)
214+ assert.Equal(t, "alpha", got.Repo)
215+ assert.EqualValues(t, 1, got.Number)
216+ require.NotNil(t, got.Comment)
217+ assert.True(t, got.Comment.Deleted)
218+ assert.Equal(t, "2", got.Comment.ReplyTo)
219+ assert.Equal(t, "approve", got.Comment.Verdict)
220+ assert.Contains(t, eventLine(got), got.Type+" alpha!1 by owner")
221+ raw, err := json.Marshal(got)
222+ require.NoError(t, err)
223+ assert.Contains(t, string(raw), `"edited_at":"2026-10-10T12:00:00Z"`)
224+ }
225+}

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 2

Tags 42

↑↓ moveenter openesc close