feat(mr): note model with threads, ranges, verdicts and the mr:review scope

commit: 3a784d4efe9143a78b0e3dd2f02c0d980edd031d
author: brice · 3d ago · 2026-10-19 05:26 UTC
parent: 0039612
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

23 files · +385 -49

M gen/ermite/storage/merge_request.pb.go +122 -18 generated file: collapsed

M internal/adapters/client/scopes_test.go +1 -0


@@ -16,6 +16,7 @@ import (
16	16
17	17		func TestScopeWireNames(t *testing.T) {
18	18		want := map[ermite.Scope]string{
	19	+	ermite.Scope_SCOPE_MR_REVIEW: "mr:review",
19	20		ermite.Scope_SCOPE_REPO_READ: "repo:read",
20	21		ermite.Scope_SCOPE_REPO_WRITE: "repo:write",
21	22		ermite.Scope_SCOPE_REPO_ADMIN: "repo:admin",


@@ -16,6 +16,7 @@ import (
16		16
17	func TestScopeWireNames(t *testing.T) {	17		func TestScopeWireNames(t *testing.T) {
18	want := map[ermite.Scope]string{	18		want := map[ermite.Scope]string{
		19	+	ermite.Scope_SCOPE_MR_REVIEW: "mr:review",
19	ermite.Scope_SCOPE_REPO_READ: "repo:read",	20		ermite.Scope_SCOPE_REPO_READ: "repo:read",
20	ermite.Scope_SCOPE_REPO_WRITE: "repo:write",	21		ermite.Scope_SCOPE_REPO_WRITE: "repo:write",
21	ermite.Scope_SCOPE_REPO_ADMIN: "repo:admin",	22		ermite.Scope_SCOPE_REPO_ADMIN: "repo:admin",

M internal/adapters/kvstore/mergerequests.go +6 -0


@@ -570,6 +570,11 @@ func encodeComment(c domain.Comment) ([]byte, error) {
570	570		return nil, err
571	571		}
572	572		return mustMarshal(&storagepb.Comment{
	573	+	ReplyTo: c.ReplyTo,
	574	+	StartLine: int32(c.StartLine), //nolint:gosec // validated against the diff
	575	+	Verdict: storagepb.Verdict(c.Verdict),
	576	+	EditedAt: encodeTime(c.EditedAt),
	577	+	Deleted: c.Deleted,
573	578		Id: c.ID,
574	579		Author: c.Author,
575	580		Path: c.Path,

@@ -593,6 +598,7 @@ func decodeComment(item kv.Item) (domain.Comment, error) {
593	598		return domain.Comment{}, fmt.Errorf("decode comment %d: %w", pb.GetId(), err)
594	599		}
595	600		c := domain.Comment{
	601	+	ReplyTo: pb.GetReplyTo(), StartLine: int(pb.GetStartLine()), Verdict: domain.Verdict(pb.GetVerdict()), EditedAt: decodeTime(pb.GetEditedAt()), Deleted: pb.GetDeleted(),
596	602		ID: pb.GetId(), Author: pb.GetAuthor(), Path: pb.GetPath(), Line: int(pb.GetLine()),
597	603		Side: decodeSide(pb.GetSide()), Text: text, Resolved: pb.GetResolved(),
598	604		CreatedAt: decodeTime(pb.GetCreatedAt()), Version: item.Version,


@@ -570,6 +570,11 @@ func encodeComment(c domain.Comment) ([]byte, error) {
570	return nil, err	570		return nil, err
571	}	571		}
572	return mustMarshal(&storagepb.Comment{	572		return mustMarshal(&storagepb.Comment{
		573	+	ReplyTo: c.ReplyTo,
		574	+	StartLine: int32(c.StartLine), //nolint:gosec // validated against the diff
		575	+	Verdict: storagepb.Verdict(c.Verdict),
		576	+	EditedAt: encodeTime(c.EditedAt),
		577	+	Deleted: c.Deleted,
573	Id: c.ID,	578		Id: c.ID,
574	Author: c.Author,	579		Author: c.Author,
575	Path: c.Path,	580		Path: c.Path,

@@ -593,6 +598,7 @@ func decodeComment(item kv.Item) (domain.Comment, error) {
593	return domain.Comment{}, fmt.Errorf("decode comment %d: %w", pb.GetId(), err)	598		return domain.Comment{}, fmt.Errorf("decode comment %d: %w", pb.GetId(), err)
594	}	599		}
595	c := domain.Comment{	600		c := domain.Comment{
		601	+	ReplyTo: pb.GetReplyTo(), StartLine: int(pb.GetStartLine()), Verdict: domain.Verdict(pb.GetVerdict()), EditedAt: decodeTime(pb.GetEditedAt()), Deleted: pb.GetDeleted(),
596	ID: pb.GetId(), Author: pb.GetAuthor(), Path: pb.GetPath(), Line: int(pb.GetLine()),	602		ID: pb.GetId(), Author: pb.GetAuthor(), Path: pb.GetPath(), Line: int(pb.GetLine()),
597	Side: decodeSide(pb.GetSide()), Text: text, Resolved: pb.GetResolved(),	603		Side: decodeSide(pb.GetSide()), Text: text, Resolved: pb.GetResolved(),
598	CreatedAt: decodeTime(pb.GetCreatedAt()), Version: item.Version,	604		CreatedAt: decodeTime(pb.GetCreatedAt()), Version: item.Version,

M internal/adapters/kvstore/mergerequests_test.go +25 -0


@@ -547,3 +547,28 @@ func TestSigningSeedRejectsBadGenerators(t *testing.T) {
547	547		_, err = s.SigningSeed(ctx, func() ([]byte, error) { return nil, assert.AnError }, created)
548	548		require.ErrorIs(t, err, assert.AnError)
549	549		}
	550	+
	551	+	func TestNoteFieldsRoundTrip(t *testing.T) {
	552	+	ctx := context.Background()
	553	+	_, store, name := repoWithMRs(t)
	554	+	mr, err := store.Create(ctx, newMR(t, name, "notes"))
	555	+	require.NoError(t, err)
	556	+	for id := int64(1); id <= 2; id++ {
	557	+	mr, err = store.Get(ctx, name, mr.Number)
	558	+	require.NoError(t, err)
	559	+	c := newComment(t, id, "note")
	560	+	if id == 2 {
	561	+	c.ReplyTo = 1
	562	+	c.StartLine = 1
	563	+	c.Verdict = domain.VerdictApprove
	564	+	c.EditedAt = created
	565	+	c.Deleted = true
	566	+	}
	567	+	mr.CommentSeq = id
	568	+	require.NoError(t, store.SaveComment(ctx, mr, c))
	569	+	got, err := store.GetComment(ctx, name, mr.Number, id)
	570	+	require.NoError(t, err)
	571	+	c.Version = got.Version
	572	+	assert.Equal(t, c, got)
	573	+	}
	574	+	}


@@ -547,3 +547,28 @@ func TestSigningSeedRejectsBadGenerators(t *testing.T) {
547	_, err = s.SigningSeed(ctx, func() ([]byte, error) { return nil, assert.AnError }, created)	547		_, err = s.SigningSeed(ctx, func() ([]byte, error) { return nil, assert.AnError }, created)
548	require.ErrorIs(t, err, assert.AnError)	548		require.ErrorIs(t, err, assert.AnError)
549	}	549		}
		550	+
		551	+	func TestNoteFieldsRoundTrip(t *testing.T) {
		552	+	ctx := context.Background()
		553	+	_, store, name := repoWithMRs(t)
		554	+	mr, err := store.Create(ctx, newMR(t, name, "notes"))
		555	+	require.NoError(t, err)
		556	+	for id := int64(1); id <= 2; id++ {
		557	+	mr, err = store.Get(ctx, name, mr.Number)
		558	+	require.NoError(t, err)
		559	+	c := newComment(t, id, "note")
		560	+	if id == 2 {
		561	+	c.ReplyTo = 1
		562	+	c.StartLine = 1
		563	+	c.Verdict = domain.VerdictApprove
		564	+	c.EditedAt = created
		565	+	c.Deleted = true
		566	+	}
		567	+	mr.CommentSeq = id
		568	+	require.NoError(t, store.SaveComment(ctx, mr, c))
		569	+	got, err := store.GetComment(ctx, name, mr.Number, id)
		570	+	require.NoError(t, err)
		571	+	c.Version = got.Version
		572	+	assert.Equal(t, c, got)
		573	+	}
		574	+	}

M internal/adapters/rpc/convert.go +1 -0


@@ -124,6 +124,7 @@ func mergeModesFromPB(in []pb.MergeMode) ([]domain.MergeMode, error) {
124	124		}
125	125
126	126		var scopeToPB = map[domain.Scope]pb.Scope{
	127	+	domain.ScopeMRReview: pb.Scope_SCOPE_MR_REVIEW,
127	128		domain.ScopeRepoRead: pb.Scope_SCOPE_REPO_READ,
128	129		domain.ScopeRepoWrite: pb.Scope_SCOPE_REPO_WRITE,
129	130		domain.ScopeRepoAdmin: pb.Scope_SCOPE_REPO_ADMIN,


@@ -124,6 +124,7 @@ func mergeModesFromPB(in []pb.MergeMode) ([]domain.MergeMode, error) {
124	}	124		}
125		125
126	var scopeToPB = map[domain.Scope]pb.Scope{	126		var scopeToPB = map[domain.Scope]pb.Scope{
		127	+	domain.ScopeMRReview: pb.Scope_SCOPE_MR_REVIEW,
127	domain.ScopeRepoRead: pb.Scope_SCOPE_REPO_READ,	128		domain.ScopeRepoRead: pb.Scope_SCOPE_REPO_READ,
128	domain.ScopeRepoWrite: pb.Scope_SCOPE_REPO_WRITE,	129		domain.ScopeRepoWrite: pb.Scope_SCOPE_REPO_WRITE,
129	domain.ScopeRepoAdmin: pb.Scope_SCOPE_REPO_ADMIN,	130		domain.ScopeRepoAdmin: pb.Scope_SCOPE_REPO_ADMIN,

M internal/adapters/rpc/mergerequest.go +10 -0


@@ -17,6 +17,16 @@ import (
17	17		// mergeRequestService implements ermiteconnect.MergeRequestServiceHandler.
18	18		type mergeRequestService struct{ mrs ports.MergeRequestService }
19	19
	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"))
	23	+	}
	24	+
	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"))
	28	+	}
	29	+
20	30		func mergeRequestStateToPB(s domain.MRState) pb.MergeRequestState {
21	31		switch s {
22	32		case domain.MRStateOpen:


@@ -17,6 +17,16 @@ 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.
		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"))
		23	+	}
		24	+
		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"))
		28	+	}
		29	+
20	func mergeRequestStateToPB(s domain.MRState) pb.MergeRequestState {	30		func mergeRequestStateToPB(s domain.MRState) pb.MergeRequestState {
21	switch s {	31		switch s {
22	case domain.MRStateOpen:	32		case domain.MRStateOpen:

M internal/domain/event.go +39 -0


@@ -173,6 +173,45 @@ type CommentAdded struct {
173	173		// EventKind implements Event.
174	174		func (CommentAdded) EventKind() string { return "mr.comment_added" }
175	175
	176	+	// CommentEdited is published when a comment was added to a merge request.
	177	+	type CommentEdited struct {
	178	+	Repo RepoName
	179	+	Number int64
	180	+	Comment Comment
	181	+	At time.Time
	182	+	Visibility Visibility
	183	+	CreatedAt time.Time
	184	+	}
	185	+
	186	+	// EventKind implements Event.
	187	+	func (CommentEdited) EventKind() string { return "mr.comment_edited" }
	188	+
	189	+	// CommentDeleted is published when a comment was added to a merge request.
	190	+	type CommentDeleted struct {
	191	+	Repo RepoName
	192	+	Number int64
	193	+	Comment Comment
	194	+	At time.Time
	195	+	Visibility Visibility
	196	+	CreatedAt time.Time
	197	+	}
	198	+
	199	+	// EventKind implements Event.
	200	+	func (CommentDeleted) EventKind() string { return "mr.comment_deleted" }
	201	+
	202	+	// CommentResolved is published when a comment was added to a merge request.
	203	+	type CommentResolved struct {
	204	+	Repo RepoName
	205	+	Number int64
	206	+	Comment Comment
	207	+	At time.Time
	208	+	Visibility Visibility
	209	+	CreatedAt time.Time
	210	+	}
	211	+
	212	+	// EventKind implements Event.
	213	+	func (CommentResolved) EventKind() string { return "mr.comment_resolved" }
	214	+
176	215		// EventGap tells a subscriber that it missed events because it was too slow;
177	216		// it must refresh its whole view.
178	217		type EventGap struct{}


@@ -173,6 +173,45 @@ type CommentAdded struct {
173	// EventKind implements Event.	173		// EventKind implements Event.
174	func (CommentAdded) EventKind() string { return "mr.comment_added" }	174		func (CommentAdded) EventKind() string { return "mr.comment_added" }
175		175
		176	+	// CommentEdited is published when a comment was added to a merge request.
		177	+	type CommentEdited struct {
		178	+	Repo RepoName
		179	+	Number int64
		180	+	Comment Comment
		181	+	At time.Time
		182	+	Visibility Visibility
		183	+	CreatedAt time.Time
		184	+	}
		185	+
		186	+	// EventKind implements Event.
		187	+	func (CommentEdited) EventKind() string { return "mr.comment_edited" }
		188	+
		189	+	// CommentDeleted is published when a comment was added to a merge request.
		190	+	type CommentDeleted struct {
		191	+	Repo RepoName
		192	+	Number int64
		193	+	Comment Comment
		194	+	At time.Time
		195	+	Visibility Visibility
		196	+	CreatedAt time.Time
		197	+	}
		198	+
		199	+	// EventKind implements Event.
		200	+	func (CommentDeleted) EventKind() string { return "mr.comment_deleted" }
		201	+
		202	+	// CommentResolved is published when a comment was added to a merge request.
		203	+	type CommentResolved struct {
		204	+	Repo RepoName
		205	+	Number int64
		206	+	Comment Comment
		207	+	At time.Time
		208	+	Visibility Visibility
		209	+	CreatedAt time.Time
		210	+	}
		211	+
		212	+	// EventKind implements Event.
		213	+	func (CommentResolved) EventKind() string { return "mr.comment_resolved" }
		214	+
176	// EventGap tells a subscriber that it missed events because it was too slow;	215		// EventGap tells a subscriber that it missed events because it was too slow;
177	// it must refresh its whole view.	216		// it must refresh its whole view.
178	type EventGap struct{}	217		type EventGap struct{}

M internal/domain/mergerequest.go +80 -0


@@ -186,6 +186,11 @@ func (m MergeRequest) IsOpen() bool { return m.State == MRStateOpen }
186	186
187	187		// Comment is a note on a merge request, general or on a line of its diff.
188	188		type Comment struct {
	189	+	ReplyTo int64
	190	+	StartLine int
	191	+	Verdict Verdict
	192	+	EditedAt time.Time
	193	+	Deleted bool
189	194		// ID is unique within the merge request, in order of creation.
190	195		ID int64
191	196		Author string

@@ -348,3 +353,78 @@ func singleLine(s string) string {
348	353		return r
349	354		}, s)
350	355		}
	356	+
	357	+	// Verdict is an immutable review decision on a root note.
	358	+	type Verdict uint8
	359	+
	360	+	// Review verdicts.
	361	+	const (
	362	+	VerdictNone Verdict = iota
	363	+	VerdictApprove
	364	+	VerdictChangesRequested
	365	+	)
	366	+
	367	+	// String returns the wire name of a verdict.
	368	+	func (v Verdict) String() string {
	369	+	switch v {
	370	+	case VerdictApprove:
	371	+	return "approve"
	372	+	case VerdictChangesRequested:
	373	+	return "changes_requested"
	374	+	}
	375	+	return ""
	376	+	}
	377	+
	378	+	// ParseVerdict parses a review decision.
	379	+	func ParseVerdict(s string) (Verdict, error) {
	380	+	switch s {
	381	+	case "":
	382	+	return VerdictNone, nil
	383	+	case "approve":
	384	+	return VerdictApprove, nil
	385	+	case "changes_requested":
	386	+	return VerdictChangesRequested, nil
	387	+	}
	388	+	return VerdictNone, fmt.Errorf("%w: verdict %q", ErrInvalid, s)
	389	+	}
	390	+
	391	+	// IsReply reports whether this note replies to a root.
	392	+	func (c Comment) IsReply() bool { return c.ReplyTo != 0 }
	393	+
	394	+	// IsRange reports whether this note anchors a range.
	395	+	func (c Comment) IsRange() bool { return c.StartLine != 0 }
	396	+
	397	+	// AuthorVerdict is an author's latest review decision.
	398	+	type AuthorVerdict struct {
	399	+	Author string
	400	+	Verdict Verdict
	401	+	CommentID int64
	402	+	CommitID ObjectID
	403	+	At time.Time
	404	+	Stale bool
	405	+	}
	406	+
	407	+	// CurrentVerdicts returns the latest verdict per author, including deleted notes,
	408	+	// in note ID order. Only approvals become stale when the head changes.
	409	+	func CurrentVerdicts(comments []Comment, head ObjectID) []AuthorVerdict {
	410	+	latest := make(map[string]Comment)
	411	+	for _, c := range comments {
	412	+	if c.Verdict != VerdictNone && c.ID > latest[c.Author].ID {
	413	+	latest[c.Author] = c
	414	+	}
	415	+	}
	416	+	out := make([]AuthorVerdict, 0, len(latest))
	417	+	for _, c := range latest {
	418	+	out = append(out, AuthorVerdict{Author: c.Author, Verdict: c.Verdict, CommentID: c.ID, CommitID: c.CommitID, At: c.CreatedAt, Stale: c.Verdict == VerdictApprove && (head == (ObjectID{}) \|\| c.CommitID != head)})
	419	+	}
	420	+	slices.SortFunc(out, func(a, b AuthorVerdict) int {
	421	+	if a.CommentID < b.CommentID {
	422	+	return -1
	423	+	}
	424	+	if a.CommentID > b.CommentID {
	425	+	return 1
	426	+	}
	427	+	return 0
	428	+	})
	429	+	return out
	430	+	}


@@ -186,6 +186,11 @@ func (m MergeRequest) IsOpen() bool { return m.State == MRStateOpen }
186		186
187	// Comment is a note on a merge request, general or on a line of its diff.	187		// Comment is a note on a merge request, general or on a line of its diff.
188	type Comment struct {	188		type Comment struct {
		189	+	ReplyTo int64
		190	+	StartLine int
		191	+	Verdict Verdict
		192	+	EditedAt time.Time
		193	+	Deleted bool
189	// ID is unique within the merge request, in order of creation.	194		// ID is unique within the merge request, in order of creation.
190	ID int64	195		ID int64
191	Author string	196		Author string

@@ -348,3 +353,78 @@ func singleLine(s string) string {
348	return r	353		return r
349	}, s)	354		}, s)
350	}	355		}
		356	+
		357	+	// Verdict is an immutable review decision on a root note.
		358	+	type Verdict uint8
		359	+
		360	+	// Review verdicts.
		361	+	const (
		362	+	VerdictNone Verdict = iota
		363	+	VerdictApprove
		364	+	VerdictChangesRequested
		365	+	)
		366	+
		367	+	// String returns the wire name of a verdict.
		368	+	func (v Verdict) String() string {
		369	+	switch v {
		370	+	case VerdictApprove:
		371	+	return "approve"
		372	+	case VerdictChangesRequested:
		373	+	return "changes_requested"
		374	+	}
		375	+	return ""
		376	+	}
		377	+
		378	+	// ParseVerdict parses a review decision.
		379	+	func ParseVerdict(s string) (Verdict, error) {
		380	+	switch s {
		381	+	case "":
		382	+	return VerdictNone, nil
		383	+	case "approve":
		384	+	return VerdictApprove, nil
		385	+	case "changes_requested":
		386	+	return VerdictChangesRequested, nil
		387	+	}
		388	+	return VerdictNone, fmt.Errorf("%w: verdict %q", ErrInvalid, s)
		389	+	}
		390	+
		391	+	// IsReply reports whether this note replies to a root.
		392	+	func (c Comment) IsReply() bool { return c.ReplyTo != 0 }
		393	+
		394	+	// IsRange reports whether this note anchors a range.
		395	+	func (c Comment) IsRange() bool { return c.StartLine != 0 }
		396	+
		397	+	// AuthorVerdict is an author's latest review decision.
		398	+	type AuthorVerdict struct {
		399	+	Author string
		400	+	Verdict Verdict
		401	+	CommentID int64
		402	+	CommitID ObjectID
		403	+	At time.Time
		404	+	Stale bool
		405	+	}
		406	+
		407	+	// CurrentVerdicts returns the latest verdict per author, including deleted notes,
		408	+	// in note ID order. Only approvals become stale when the head changes.
		409	+	func CurrentVerdicts(comments []Comment, head ObjectID) []AuthorVerdict {
		410	+	latest := make(map[string]Comment)
		411	+	for _, c := range comments {
		412	+	if c.Verdict != VerdictNone && c.ID > latest[c.Author].ID {
		413	+	latest[c.Author] = c
		414	+	}
		415	+	}
		416	+	out := make([]AuthorVerdict, 0, len(latest))
		417	+	for _, c := range latest {
		418	+	out = append(out, AuthorVerdict{Author: c.Author, Verdict: c.Verdict, CommentID: c.ID, CommitID: c.CommitID, At: c.CreatedAt, Stale: c.Verdict == VerdictApprove && (head == (ObjectID{}) \|\| c.CommitID != head)})
		419	+	}
		420	+	slices.SortFunc(out, func(a, b AuthorVerdict) int {
		421	+	if a.CommentID < b.CommentID {
		422	+	return -1
		423	+	}
		424	+	if a.CommentID > b.CommentID {
		425	+	return 1
		426	+	}
		427	+	return 0
		428	+	})
		429	+	return out
		430	+	}

M internal/domain/mergerequest_test.go +48 -0


@@ -235,3 +235,51 @@ func TestMergeRequestDraft(t *testing.T) {
235	235		assert.Equal(t, tc.ready, ready.String(), tc.title)
236	236		}
237	237		}
	238	+
	239	+	func TestNoteVerdicts(t *testing.T) {
	240	+	head, old := oid(t, 'a'), oid(t, 'b')
	241	+	for _, tc := range []struct {
	242	+	text string
	243	+	verdict domain.Verdict
	244	+	}{{"", domain.VerdictNone}, {"approve", domain.VerdictApprove}, {"changes_requested", domain.VerdictChangesRequested}} {
	245	+	v, err := domain.ParseVerdict(tc.text)
	246	+	require.NoError(t, err)
	247	+	assert.Equal(t, tc.verdict, v)
	248	+	assert.Equal(t, tc.text, v.String())
	249	+	}
	250	+	_, err := domain.ParseVerdict("unknown")
	251	+	require.ErrorIs(t, err, domain.ErrInvalid)
	252	+	comments := []domain.Comment{{ID: 3, Author: "a", Verdict: domain.VerdictApprove, CommitID: old, Deleted: true}, {ID: 1, Author: "a", Verdict: domain.VerdictChangesRequested}, {ID: 2, Author: "b", Verdict: domain.VerdictChangesRequested, CommitID: old}, {ID: 4, Author: "a"}}
	253	+	for _, h := range []domain.ObjectID{head, {}} {
	254	+	got := domain.CurrentVerdicts(comments, h)
	255	+	require.Len(t, got, 2)
	256	+	assert.Equal(t, int64(2), got[0].CommentID)
	257	+	assert.False(t, got[0].Stale)
	258	+	assert.Equal(t, int64(3), got[1].CommentID)
	259	+	assert.True(t, got[1].Stale)
	260	+	}
	261	+	comments[0].CommitID = head
	262	+	assert.False(t, domain.CurrentVerdicts(comments, head)[1].Stale)
	263	+	assert.True(t, (domain.Comment{ReplyTo: 1}).IsReply())
	264	+	assert.True(t, (domain.Comment{StartLine: 1}).IsRange())
	265	+	}
	266	+
	267	+	func TestReviewScopeOwnerRights(t *testing.T) {
	268	+	var old []domain.Scope
	269	+	for _, s := range domain.AllScopes() {
	270	+	if s != domain.ScopeMRReview {
	271	+	old = append(old, s)
	272	+	}
	273	+	}
	274	+	assert.False(t, domain.TokenPrincipal(domain.Token{Scopes: old}).HasOwnerRights())
	275	+	assert.True(t, domain.TokenPrincipal(domain.Token{Scopes: domain.AllScopes()}).HasOwnerRights())
	276	+	s, err := domain.ParseScope("mr:review")
	277	+	require.NoError(t, err)
	278	+	assert.Equal(t, domain.ScopeMRReview, s)
	279	+	for _, tc := range []struct {
	280	+	p domain.Principal
	281	+	want string
	282	+	}{{domain.Root(), "root"}, {domain.Owner(), "owner"}, {domain.Anonymous(), "owner"}, {domain.TokenPrincipal(domain.Token{Label: "bot"}), "bot"}} {
	283	+	assert.Equal(t, tc.want, domain.AuthorName(tc.p))
	284	+	}
	285	+	}


@@ -235,3 +235,51 @@ func TestMergeRequestDraft(t *testing.T) {
235	assert.Equal(t, tc.ready, ready.String(), tc.title)	235		assert.Equal(t, tc.ready, ready.String(), tc.title)
236	}	236		}
237	}	237		}
		238	+
		239	+	func TestNoteVerdicts(t *testing.T) {
		240	+	head, old := oid(t, 'a'), oid(t, 'b')
		241	+	for _, tc := range []struct {
		242	+	text string
		243	+	verdict domain.Verdict
		244	+	}{{"", domain.VerdictNone}, {"approve", domain.VerdictApprove}, {"changes_requested", domain.VerdictChangesRequested}} {
		245	+	v, err := domain.ParseVerdict(tc.text)
		246	+	require.NoError(t, err)
		247	+	assert.Equal(t, tc.verdict, v)
		248	+	assert.Equal(t, tc.text, v.String())
		249	+	}
		250	+	_, err := domain.ParseVerdict("unknown")
		251	+	require.ErrorIs(t, err, domain.ErrInvalid)
		252	+	comments := []domain.Comment{{ID: 3, Author: "a", Verdict: domain.VerdictApprove, CommitID: old, Deleted: true}, {ID: 1, Author: "a", Verdict: domain.VerdictChangesRequested}, {ID: 2, Author: "b", Verdict: domain.VerdictChangesRequested, CommitID: old}, {ID: 4, Author: "a"}}
		253	+	for _, h := range []domain.ObjectID{head, {}} {
		254	+	got := domain.CurrentVerdicts(comments, h)
		255	+	require.Len(t, got, 2)
		256	+	assert.Equal(t, int64(2), got[0].CommentID)
		257	+	assert.False(t, got[0].Stale)
		258	+	assert.Equal(t, int64(3), got[1].CommentID)
		259	+	assert.True(t, got[1].Stale)
		260	+	}
		261	+	comments[0].CommitID = head
		262	+	assert.False(t, domain.CurrentVerdicts(comments, head)[1].Stale)
		263	+	assert.True(t, (domain.Comment{ReplyTo: 1}).IsReply())
		264	+	assert.True(t, (domain.Comment{StartLine: 1}).IsRange())
		265	+	}
		266	+
		267	+	func TestReviewScopeOwnerRights(t *testing.T) {
		268	+	var old []domain.Scope
		269	+	for _, s := range domain.AllScopes() {
		270	+	if s != domain.ScopeMRReview {
		271	+	old = append(old, s)
		272	+	}
		273	+	}
		274	+	assert.False(t, domain.TokenPrincipal(domain.Token{Scopes: old}).HasOwnerRights())
		275	+	assert.True(t, domain.TokenPrincipal(domain.Token{Scopes: domain.AllScopes()}).HasOwnerRights())
		276	+	s, err := domain.ParseScope("mr:review")
		277	+	require.NoError(t, err)
		278	+	assert.Equal(t, domain.ScopeMRReview, s)
		279	+	for _, tc := range []struct {
		280	+	p domain.Principal
		281	+	want string
		282	+	}{{domain.Root(), "root"}, {domain.Owner(), "owner"}, {domain.Anonymous(), "owner"}, {domain.TokenPrincipal(domain.Token{Label: "bot"}), "bot"}} {
		283	+	assert.Equal(t, tc.want, domain.AuthorName(tc.p))
		284	+	}
		285	+	}

M internal/domain/principal.go +15 -0


@@ -230,3 +230,18 @@ func (p Principal) ArchiveGrant() (repo RepoName, sha string, ok bool) {
230	230		}
231	231		return p.archiveRepo, p.archiveSha, true
232	232		}
	233	+
	234	+	// AuthorName names who acts, for the author of a merge request or a comment: the
	235	+	// label of the token, "root" for the root token, "owner" for the owner signed
	236	+	// in on the web and for any other caller without a label.
	237	+	func AuthorName(p Principal) string {
	238	+	switch {
	239	+	case p.IsOwner():
	240	+	return "owner"
	241	+	case p.IsRoot():
	242	+	return "root"
	243	+	case p.TokenLabel() != "":
	244	+	return p.TokenLabel()
	245	+	}
	246	+	return "owner"
	247	+	}


@@ -230,3 +230,18 @@ func (p Principal) ArchiveGrant() (repo RepoName, sha string, ok bool) {
230	}	230		}
231	return p.archiveRepo, p.archiveSha, true	231		return p.archiveRepo, p.archiveSha, true
232	}	232		}
		233	+
		234	+	// AuthorName names who acts, for the author of a merge request or a comment: the
		235	+	// label of the token, "root" for the root token, "owner" for the owner signed
		236	+	// in on the web and for any other caller without a label.
		237	+	func AuthorName(p Principal) string {
		238	+	switch {
		239	+	case p.IsOwner():
		240	+	return "owner"
		241	+	case p.IsRoot():
		242	+	return "root"
		243	+	case p.TokenLabel() != "":
		244	+	return p.TokenLabel()
		245	+	}
		246	+	return "owner"
		247	+	}

M internal/domain/token.go +4 -2


@@ -30,6 +30,8 @@ const (
30	30		ScopeMRWrite Scope = "mr:write"
31	31		// ScopeMRMerge merges, and deletes the merged source branch on request.
32	32		ScopeMRMerge Scope = "mr:merge"
	33	+	// ScopeMRReview sets a verdict on a merge request.
	34	+	ScopeMRReview Scope = "mr:review"
33	35		// ScopeCIWrite re-runs or cancels CI runs.
34	36		ScopeCIWrite Scope = "ci:write"
35	37		// ScopeSecretsWrite sets and deletes secrets and variables.

@@ -40,11 +42,11 @@ const (
40	42		ScopeTokenAdmin Scope = "token:admin"
41	43		)
42	44
43		-	// AllScopes returns the nine scopes in a stable order.
	45	+	// AllScopes returns the ten scopes in a stable order.
44	46		func AllScopes() []Scope {
45	47		return []Scope{
46	48		ScopeRepoRead, ScopeRepoWrite, ScopeRepoAdmin,
47		-	ScopeMRWrite, ScopeMRMerge,
	49	+	ScopeMRWrite, ScopeMRMerge, ScopeMRReview,
48	50		ScopeCIWrite, ScopeSecretsWrite, ScopeRunnerRegister,
49	51		ScopeTokenAdmin,
50	52		}

 	ScopeMRWrite Scope = "mr:write"
 	// ScopeMRMerge merges, and deletes the merged source branch on request.
 	ScopeMRMerge Scope = "mr:merge"
+	// ScopeMRReview sets a verdict on a merge request.
+	ScopeMRReview Scope = "mr:review"
 	// ScopeCIWrite re-runs or cancels CI runs.
 	ScopeCIWrite Scope = "ci:write"
 	// ScopeSecretsWrite sets and deletes secrets and variables.
 	ScopeTokenAdmin Scope = "token:admin"
 )
-// AllScopes returns the nine scopes in a stable order.
+// AllScopes returns the ten scopes in a stable order.
 func AllScopes() []Scope {
 	return []Scope{
 		ScopeRepoRead, ScopeRepoWrite, ScopeRepoAdmin,
-		ScopeMRWrite, ScopeMRMerge,
+		ScopeMRWrite, ScopeMRMerge, ScopeMRReview,
 		ScopeCIWrite, ScopeSecretsWrite, ScopeRunnerRegister,
 		ScopeTokenAdmin,
 	}

M internal/domain/token_test.go +1 -1


@@ -16,7 +16,7 @@ import (
16	16		func TestScopes(t *testing.T) {
17	17		t.Parallel()
18	18		all := domain.AllScopes()
19		-	assert.Len(t, all, 9)
	19	+	assert.Len(t, all, 10)
20	20		for _, s := range all {
21	21		got, err := domain.ParseScope(s.String())
22	22		require.NoError(t, err)


@@ -16,7 +16,7 @@ import (
16		func TestScopes(t *testing.T) {	16		func TestScopes(t *testing.T) {
17		t.Parallel()	17		t.Parallel()
18		all := domain.AllScopes()	18		all := domain.AllScopes()
19	-	assert.Len(t, all, 9)	19	+	assert.Len(t, all, 10)
20		for _, s := range all {	20		for _, s := range all {
21		got, err := domain.ParseScope(s.String())	21		got, err := domain.ParseScope(s.String())
22		require.NoError(t, err)	22		require.NoError(t, err)

M internal/ports/dto.go +8 -0


@@ -212,6 +212,8 @@ type MergeRequestDetail struct {
212	212		Diff domain.Diff
213	213		// Comments are the comments, oldest first.
214	214		Comments []domain.Comment
	215	+	// Verdicts are the latest verdict per author in note ID order.
	216	+	Verdicts []domain.AuthorVerdict
215	217		// LastReadID is the head recorded by MarkRead; zero when none.
216	218		LastReadID domain.ObjectID
217	219		// LastReadLost tells that since-last-read was asked for, a head was

@@ -241,6 +243,12 @@ type FileWindow struct{ From, To int }
241	243
242	244		// AddCommentParams describes the comment to add.
243	245		type AddCommentParams struct {
	246	+	// ReplyTo names a non-deleted root; replies cannot specify an anchor or verdict.
	247	+	ReplyTo int64
	248	+	// StartLine starts an inclusive range, requiring a path and 1 <= StartLine < Line.
	249	+	StartLine int
	250	+	// Verdict requires mr:review and is only permitted on roots.
	251	+	Verdict domain.Verdict
244	252		// Path, Line and Side designate a line of the diff; all empty for a
245	253		// general comment.
246	254		Path string


@@ -212,6 +212,8 @@ type MergeRequestDetail struct {
212	Diff domain.Diff	212		Diff domain.Diff
213	// Comments are the comments, oldest first.	213		// Comments are the comments, oldest first.
214	Comments []domain.Comment	214		Comments []domain.Comment
		215	+	// Verdicts are the latest verdict per author in note ID order.
		216	+	Verdicts []domain.AuthorVerdict
215	// LastReadID is the head recorded by MarkRead; zero when none.	217		// LastReadID is the head recorded by MarkRead; zero when none.
216	LastReadID domain.ObjectID	218		LastReadID domain.ObjectID
217	// LastReadLost tells that since-last-read was asked for, a head was	219		// LastReadLost tells that since-last-read was asked for, a head was

@@ -241,6 +243,12 @@ type FileWindow struct{ From, To int }
241		243
242	// AddCommentParams describes the comment to add.	244		// AddCommentParams describes the comment to add.
243	type AddCommentParams struct {	245		type AddCommentParams struct {
		246	+	// ReplyTo names a non-deleted root; replies cannot specify an anchor or verdict.
		247	+	ReplyTo int64
		248	+	// StartLine starts an inclusive range, requiring a path and 1 <= StartLine < Line.
		249	+	StartLine int
		250	+	// Verdict requires mr:review and is only permitted on roots.
		251	+	Verdict domain.Verdict
244	// Path, Line and Side designate a line of the diff; all empty for a	252		// Path, Line and Side designate a line of the diff; all empty for a
245	// general comment.	253		// general comment.
246	Path string	254		Path string

M internal/usecase/access.go +0 -15


@@ -91,18 +91,3 @@ func scopeAccess(p domain.Principal, repo domain.Repo, scope domain.Scope) error
91	91		}
92	92		return fmt.Errorf("%w: this needs the %s scope on %s", domain.ErrForbidden, scope, repo.Name)
93	93		}
94		-
95		-	// authorOf names who acts, for the author of a merge request or a comment: the
96		-	// label of the token, "root" for the root token, "owner" for the owner signed
97		-	// in on the web and for any other caller without a label.
98		-	func authorOf(p domain.Principal) string {
99		-	switch {
100		-	case p.IsOwner():
101		-	return "owner"
102		-	case p.IsRoot():
103		-	return "root"
104		-	case p.TokenLabel() != "":
105		-	return p.TokenLabel()
106		-	}
107		-	return "owner"
108		-	}


@@ -91,18 +91,3 @@ func scopeAccess(p domain.Principal, repo domain.Repo, scope domain.Scope) error
91		}	91	}
92		return fmt.Errorf("%w: this needs the %s scope on %s", domain.ErrForbidden, scope, repo.Name)	92	return fmt.Errorf("%w: this needs the %s scope on %s", domain.ErrForbidden, scope, repo.Name)
93		}	93	}
94	-
95	-	// authorOf names who acts, for the author of a merge request or a comment: the
96	-	// label of the token, "root" for the root token, "owner" for the owner signed
97	-	// in on the web and for any other caller without a label.
98	-	func authorOf(p domain.Principal) string {
99	-	switch {
100	-	case p.IsOwner():
101	-	return "owner"
102	-	case p.IsRoot():
103	-	return "root"
104	-	case p.TokenLabel() != "":
105	-	return p.TokenLabel()
106	-	}
107	-	return "owner"
108	-	}

M internal/usecase/ci_dispatch.go +1 -1


@@ -135,7 +135,7 @@ func (c *CI) dispatchCaller(ctx context.Context, p domain.Principal, name domain
135	135		if err != nil {
136	136		return repo, "", pusher, false, err
137	137		}
138		-	return repo, authorOf(p), domain.PusherOf(p), domain.TrustedActor(p), nil
	138	+	return repo, domain.AuthorName(p), domain.PusherOf(p), domain.TrustedActor(p), nil
139	139		}
140	140
141	141		// dispatchPath checks the workflow file of a dispatch: a file of


@@ -135,7 +135,7 @@ func (c *CI) dispatchCaller(ctx context.Context, p domain.Principal, name domain
135		if err != nil {	135		if err != nil {
136		return repo, "", pusher, false, err	136		return repo, "", pusher, false, err
137		}	137		}
138	-	return repo, authorOf(p), domain.PusherOf(p), domain.TrustedActor(p), nil	138	+	return repo, domain.AuthorName(p), domain.PusherOf(p), domain.TrustedActor(p), nil
139		}	139		}
140			140
141		// dispatchPath checks the workflow file of a dispatch: a file of	141		// dispatchPath checks the workflow file of a dispatch: a file of

M internal/usecase/ci_pull.go +1 -1


@@ -203,7 +203,7 @@ func (c *CI) OnPullRequest(ctx context.Context, repo domain.RepoName, ev ports.P
203	203		}
204	204		e := pullEvent{
205	205		repo: repo, number: ev.Number, action: ev.Action, head: ev.Head, base: ev.Base,
206		-	actor: authorOf(p), pusher: domain.PusherOf(p), run: !p.ViaJobToken(), log: zerowrap.FromCtx(ctx),
	206	+	actor: domain.AuthorName(p), pusher: domain.PusherOf(p), run: !p.ViaJobToken(), log: zerowrap.FromCtx(ctx),
207	207		actorTrusted: domain.TrustedActor(p), target: c.defaultTipOf(ctx, repo),
208	208		}
209	209		if e.actorTrusted {


@@ -203,7 +203,7 @@ func (c *CI) OnPullRequest(ctx context.Context, repo domain.RepoName, ev ports.P
203		}	203		}
204		e := pullEvent{	204		e := pullEvent{
205		repo: repo, number: ev.Number, action: ev.Action, head: ev.Head, base: ev.Base,	205		repo: repo, number: ev.Number, action: ev.Action, head: ev.Head, base: ev.Base,
206	-	actor: authorOf(p), pusher: domain.PusherOf(p), run: !p.ViaJobToken(), log: zerowrap.FromCtx(ctx),	206	+	actor: domain.AuthorName(p), pusher: domain.PusherOf(p), run: !p.ViaJobToken(), log: zerowrap.FromCtx(ctx),
207		actorTrusted: domain.TrustedActor(p), target: c.defaultTipOf(ctx, repo),	207		actorTrusted: domain.TrustedActor(p), target: c.defaultTipOf(ctx, repo),
208		}	208		}
209		if e.actorTrusted {	209		if e.actorTrusted {

M internal/usecase/ci_runs.go +1 -1


@@ -332,7 +332,7 @@ func (c *CI) CancelRun(ctx context.Context, p domain.Principal, name domain.Repo
332	332		}
333	333		if !rs.run.CancelRequested {
334	334		rs.run.CancelRequested, rs.run.CancelRequestedAt, rs.runDirty = true, c.now(), true
335		-	rs.cancelAll("Canceled by " + authorOf(p))
	335	+	rs.cancelAll("Canceled by " + domain.AuthorName(p))
336	336		}
337	337		if err := rs.flush(ctx); err != nil {
338	338		return err


@@ -332,7 +332,7 @@ func (c *CI) CancelRun(ctx context.Context, p domain.Principal, name domain.Repo
332		}	332		}
333		if !rs.run.CancelRequested {	333		if !rs.run.CancelRequested {
334		rs.run.CancelRequested, rs.run.CancelRequestedAt, rs.runDirty = true, c.now(), true	334		rs.run.CancelRequested, rs.run.CancelRequestedAt, rs.runDirty = true, c.now(), true
335	-	rs.cancelAll("Canceled by " + authorOf(p))	335	+	rs.cancelAll("Canceled by " + domain.AuthorName(p))
336		}	336		}
337		if err := rs.flush(ctx); err != nil {	337		if err := rs.flush(ctx); err != nil {
338		return err	338		return err

M internal/usecase/ci_tips.go +1 -1


@@ -85,7 +85,7 @@ func (c *CI) scheduleActor(ctx context.Context, repo domain.RepoName, branch str
85	85		if t, ok := c.refTip(ctx, repo, "refs/heads/"+branch, tip); ok {
86	86		return t.Actor, t.Pusher, t.Trusted
87	87		}
88		-	return authorOf(domain.Owner()), domain.CIPusher{}, false
	88	+	return domain.AuthorName(domain.Owner()), domain.CIPusher{}, false
89	89		}
90	90
91	91		// refTrusted reports whether the ref was set to exactly the pinned object by a


@@ -85,7 +85,7 @@ func (c *CI) scheduleActor(ctx context.Context, repo domain.RepoName, branch str
85		if t, ok := c.refTip(ctx, repo, "refs/heads/"+branch, tip); ok {	85		if t, ok := c.refTip(ctx, repo, "refs/heads/"+branch, tip); ok {
86		return t.Actor, t.Pusher, t.Trusted	86		return t.Actor, t.Pusher, t.Trusted
87		}	87		}
88	-	return authorOf(domain.Owner()), domain.CIPusher{}, false	88	+	return domain.AuthorName(domain.Owner()), domain.CIPusher{}, false
89		}	89		}
90			90
91		// refTrusted reports whether the ref was set to exactly the pinned object by a	91		// refTrusted reports whether the ref was set to exactly the pinned object by a

M internal/usecase/ci_trigger.go +4 -4


@@ -59,13 +59,13 @@ func (c *CI) OnPush(ctx context.Context, repo domain.RepoName, updates []domain.
59	59		c.markSchedulesDirty(repo, updates)
60	60		// The merge requests follow the push whoever made it (their refs); only a
61	61		// push that is not a job's starts pull_request runs.
62		-	defer c.onBranchesMoved(ctx, repo, updates, authorOf(pusher), domain.PusherOf(pusher), domain.TrustedActor(pusher), !pusher.ViaJobToken())
	62	+	defer c.onBranchesMoved(ctx, repo, updates, domain.AuthorName(pusher), domain.PusherOf(pusher), domain.TrustedActor(pusher), !pusher.ViaJobToken())
63	63		if pusher.ViaJobToken() {
64	64		log := zerowrap.FromCtx(ctx)
65	65		log.Debug().Str("repo", repo.String()).Int("refs", len(updates)).Msg("push by a job token starts no run")
66	66		return
67	67		}
68		-	c.enqueue(ctx, repo, updates, authorOf(pusher), domain.PusherOf(pusher), domain.TrustedActor(pusher))
	68	+	c.enqueue(ctx, repo, updates, domain.AuthorName(pusher), domain.PusherOf(pusher), domain.TrustedActor(pusher))
69	69		}
70	70
71	71		// OnMerge queues the move of a target branch by a merge. The actor of the run is

@@ -79,13 +79,13 @@ func (c *CI) OnMerge(ctx context.Context, repo domain.RepoName, updates []domain
79	79		// The merge moved a target branch: the test merges of the other requests
80	80		// into it are computed again (a merge starts no synchronize run), after the
81	81		// push workflows are queued.
82		-	defer c.onBranchesMoved(ctx, repo, updates, authorOf(domain.Owner()), domain.PusherOf(merger), domain.TrustedActor(merger), false)
	82	+	defer c.onBranchesMoved(ctx, repo, updates, domain.AuthorName(domain.Owner()), domain.PusherOf(merger), domain.TrustedActor(merger), false)
83	83		if merger.ViaJobToken() {
84	84		log := zerowrap.FromCtx(ctx)
85	85		log.Debug().Str("repo", repo.String()).Int("refs", len(updates)).Msg("merge by a job token starts no run")
86	86		return
87	87		}
88		-	c.enqueue(ctx, repo, updates, authorOf(domain.Owner()), domain.PusherOf(merger), domain.TrustedActor(merger))
	88	+	c.enqueue(ctx, repo, updates, domain.AuthorName(domain.Owner()), domain.PusherOf(merger), domain.TrustedActor(merger))
89	89		}
90	90
91	91		func (c *CI) enqueue(ctx context.Context, repo domain.RepoName, updates []domain.RefUpdate, actor string, pusher domain.CIPusher, trusted bool) {


@@ -59,13 +59,13 @@ func (c *CI) OnPush(ctx context.Context, repo domain.RepoName, updates []domain.
59		c.markSchedulesDirty(repo, updates)	59		c.markSchedulesDirty(repo, updates)
60		// The merge requests follow the push whoever made it (their refs); only a	60		// The merge requests follow the push whoever made it (their refs); only a
61		// push that is not a job's starts pull_request runs.	61		// push that is not a job's starts pull_request runs.
62	-	defer c.onBranchesMoved(ctx, repo, updates, authorOf(pusher), domain.PusherOf(pusher), domain.TrustedActor(pusher), !pusher.ViaJobToken())	62	+	defer c.onBranchesMoved(ctx, repo, updates, domain.AuthorName(pusher), domain.PusherOf(pusher), domain.TrustedActor(pusher), !pusher.ViaJobToken())
63		if pusher.ViaJobToken() {	63		if pusher.ViaJobToken() {
64		log := zerowrap.FromCtx(ctx)	64		log := zerowrap.FromCtx(ctx)
65		log.Debug().Str("repo", repo.String()).Int("refs", len(updates)).Msg("push by a job token starts no run")	65		log.Debug().Str("repo", repo.String()).Int("refs", len(updates)).Msg("push by a job token starts no run")
66		return	66		return
67		}	67		}
68	-	c.enqueue(ctx, repo, updates, authorOf(pusher), domain.PusherOf(pusher), domain.TrustedActor(pusher))	68	+	c.enqueue(ctx, repo, updates, domain.AuthorName(pusher), domain.PusherOf(pusher), domain.TrustedActor(pusher))
69		}	69		}
70			70
71		// OnMerge queues the move of a target branch by a merge. The actor of the run is	71		// OnMerge queues the move of a target branch by a merge. The actor of the run is

@@ -79,13 +79,13 @@ func (c *CI) OnMerge(ctx context.Context, repo domain.RepoName, updates []domain
79		// The merge moved a target branch: the test merges of the other requests	79		// The merge moved a target branch: the test merges of the other requests
80		// into it are computed again (a merge starts no synchronize run), after the	80		// into it are computed again (a merge starts no synchronize run), after the
81		// push workflows are queued.	81		// push workflows are queued.
82	-	defer c.onBranchesMoved(ctx, repo, updates, authorOf(domain.Owner()), domain.PusherOf(merger), domain.TrustedActor(merger), false)	82	+	defer c.onBranchesMoved(ctx, repo, updates, domain.AuthorName(domain.Owner()), domain.PusherOf(merger), domain.TrustedActor(merger), false)
83		if merger.ViaJobToken() {	83		if merger.ViaJobToken() {
84		log := zerowrap.FromCtx(ctx)	84		log := zerowrap.FromCtx(ctx)
85		log.Debug().Str("repo", repo.String()).Int("refs", len(updates)).Msg("merge by a job token starts no run")	85		log.Debug().Str("repo", repo.String()).Int("refs", len(updates)).Msg("merge by a job token starts no run")
86		return	86		return
87		}	87		}
88	-	c.enqueue(ctx, repo, updates, authorOf(domain.Owner()), domain.PusherOf(merger), domain.TrustedActor(merger))	88	+	c.enqueue(ctx, repo, updates, domain.AuthorName(domain.Owner()), domain.PusherOf(merger), domain.TrustedActor(merger))
89		}	89		}
90			90
91		func (c *CI) enqueue(ctx context.Context, repo domain.RepoName, updates []domain.RefUpdate, actor string, pusher domain.CIPusher, trusted bool) {	91		func (c *CI) enqueue(ctx context.Context, repo domain.RepoName, updates []domain.RefUpdate, actor string, pusher domain.CIPusher, trusted bool) {

M internal/usecase/export_test.go +1 -1


@@ -16,7 +16,7 @@ import (
16	16		)
17	17
18	18		// AuthorOf exposes authorOf to the tests.
19		-	func AuthorOf(p domain.Principal) string { return authorOf(p) }
	19	+	func AuthorOf(p domain.Principal) string { return domain.AuthorName(p) }
20	20
21	21		// Sweep exposes the daily sweep to the tests.
22	22		func (c *CI) Sweep(ctx context.Context) { c.sweep(ctx) }


@@ -16,7 +16,7 @@ import (
16		)	16		)
17			17
18		// AuthorOf exposes authorOf to the tests.	18		// AuthorOf exposes authorOf to the tests.
19	-	func AuthorOf(p domain.Principal) string { return authorOf(p) }	19	+	func AuthorOf(p domain.Principal) string { return domain.AuthorName(p) }
20			20
21		// Sweep exposes the daily sweep to the tests.	21		// Sweep exposes the daily sweep to the tests.
22		func (c *CI) Sweep(ctx context.Context) { c.sweep(ctx) }	22		func (c *CI) Sweep(ctx context.Context) { c.sweep(ctx) }

M internal/usecase/mergerequests.go +3 -3


@@ -241,7 +241,7 @@ func (m *MergeRequests) Create(ctx context.Context, p domain.Principal, name dom
241	241		now := m.clock.Now().UTC()
242	242		mr, err := m.store.Create(ctx, domain.MergeRequest{
243	243		Repo: name, Title: title, Description: desc, Source: source, Target: target,
244		-	State: domain.MRStateOpen, Author: authorOf(p), CreatedAt: now, UpdatedAt: now,
	244	+	State: domain.MRStateOpen, Author: domain.AuthorName(p), CreatedAt: now, UpdatedAt: now,
245	245		})
246	246		if err != nil {
247	247		return domain.MergeRequest{}, fmt.Errorf("create merge request of %q: %w", name, err)

@@ -600,7 +600,7 @@ func (m *MergeRequests) AddComment(ctx context.Context, p domain.Principal, name
600	600		return domain.Comment{}, fmt.Errorf("%w: !%d already has %d comments", domain.ErrResourceExhausted, number, MaxCommentsPerMergeRequest)
601	601		}
602	602		c = domain.Comment{
603		-	ID: mr.CommentSeq + 1, Author: authorOf(p), Path: params.Path, Line: params.Line, Side: params.Side,
	603	+	ID: mr.CommentSeq + 1, Author: domain.AuthorName(p), Path: params.Path, Line: params.Line, Side: params.Side,
604	604		CommitID: mr.HeadID, Text: text, CreatedAt: m.clock.Now().UTC(),
605	605		}
606	606		mr.CommentSeq, mr.UpdatedAt = c.ID, c.CreatedAt

@@ -1090,7 +1090,7 @@ func (m *MergeRequests) moveTarget(ctx context.Context, mr domain.MergeRequest,
1090	1090		// does, under the merge lock (domain.CIRefTip). The run shows the owner, as a
1091	1091		// merge's push does on GitHub.
1092	1092		if err := recordRefTips(ctx, m.tips, mr.Repo, []domain.RefUpdate{{Old: tip.String(), New: newTip.String(), Ref: "refs/heads/" + mr.Target}},
1093		-	authorOf(domain.Owner()), domain.PusherOf(p), domain.TrustedActor(p)); err != nil {
	1093	+	domain.AuthorName(domain.Owner()), domain.PusherOf(p), domain.TrustedActor(p)); err != nil {
1094	1094		return domain.ObjectID{}, err
1095	1095		}
1096	1096		err := m.git.UpdateBranch(ctx, mr.Repo, mr.Target, tip, newTip)


@@ -241,7 +241,7 @@ func (m *MergeRequests) Create(ctx context.Context, p domain.Principal, name dom
241		now := m.clock.Now().UTC()	241		now := m.clock.Now().UTC()
242		mr, err := m.store.Create(ctx, domain.MergeRequest{	242		mr, err := m.store.Create(ctx, domain.MergeRequest{
243		Repo: name, Title: title, Description: desc, Source: source, Target: target,	243		Repo: name, Title: title, Description: desc, Source: source, Target: target,
244	-	State: domain.MRStateOpen, Author: authorOf(p), CreatedAt: now, UpdatedAt: now,	244	+	State: domain.MRStateOpen, Author: domain.AuthorName(p), CreatedAt: now, UpdatedAt: now,
245		})	245		})
246		if err != nil {	246		if err != nil {
247		return domain.MergeRequest{}, fmt.Errorf("create merge request of %q: %w", name, err)	247		return domain.MergeRequest{}, fmt.Errorf("create merge request of %q: %w", name, err)

@@ -600,7 +600,7 @@ func (m *MergeRequests) AddComment(ctx context.Context, p domain.Principal, name
600		return domain.Comment{}, fmt.Errorf("%w: !%d already has %d comments", domain.ErrResourceExhausted, number, MaxCommentsPerMergeRequest)	600		return domain.Comment{}, fmt.Errorf("%w: !%d already has %d comments", domain.ErrResourceExhausted, number, MaxCommentsPerMergeRequest)
601		}	601		}
602		c = domain.Comment{	602		c = domain.Comment{
603	-	ID: mr.CommentSeq + 1, Author: authorOf(p), Path: params.Path, Line: params.Line, Side: params.Side,	603	+	ID: mr.CommentSeq + 1, Author: domain.AuthorName(p), Path: params.Path, Line: params.Line, Side: params.Side,
604		CommitID: mr.HeadID, Text: text, CreatedAt: m.clock.Now().UTC(),	604		CommitID: mr.HeadID, Text: text, CreatedAt: m.clock.Now().UTC(),
605		}	605		}
606		mr.CommentSeq, mr.UpdatedAt = c.ID, c.CreatedAt	606		mr.CommentSeq, mr.UpdatedAt = c.ID, c.CreatedAt

@@ -1090,7 +1090,7 @@ func (m *MergeRequests) moveTarget(ctx context.Context, mr domain.MergeRequest,
1090		// does, under the merge lock (domain.CIRefTip). The run shows the owner, as a	1090		// does, under the merge lock (domain.CIRefTip). The run shows the owner, as a
1091		// merge's push does on GitHub.	1091		// merge's push does on GitHub.
1092		if err := recordRefTips(ctx, m.tips, mr.Repo, []domain.RefUpdate{{Old: tip.String(), New: newTip.String(), Ref: "refs/heads/" + mr.Target}},	1092		if err := recordRefTips(ctx, m.tips, mr.Repo, []domain.RefUpdate{{Old: tip.String(), New: newTip.String(), Ref: "refs/heads/" + mr.Target}},
1093	-	authorOf(domain.Owner()), domain.PusherOf(p), domain.TrustedActor(p)); err != nil {	1093	+	domain.AuthorName(domain.Owner()), domain.PusherOf(p), domain.TrustedActor(p)); err != nil {
1094		return domain.ObjectID{}, err	1094		return domain.ObjectID{}, err
1095		}	1095		}
1096		err := m.git.UpdateBranch(ctx, mr.Repo, mr.Target, tip, newTip)	1096		err := m.git.UpdateBranch(ctx, mr.Repo, mr.Target, tip, newTip)

M internal/usecase/pushes.go +1 -1


@@ -179,7 +179,7 @@ func (u *Pushes) Check(ctx context.Context, p domain.Principal, name domain.Repo
179	179		// recorded before Git moves them, so that a tip never has the record of an
180	180		// earlier pusher at the same commit (domain.CIRefTip). A record that cannot
181	181		// be written refuses the push, as it cannot be run later with a known trust.
182		-	if err := recordRefTips(ctx, u.tips, name, updates, authorOf(p), domain.PusherOf(p), domain.TrustedActor(p)); err != nil {
	182	+	if err := recordRefTips(ctx, u.tips, name, updates, domain.AuthorName(p), domain.PusherOf(p), domain.TrustedActor(p)); err != nil {
183	183		return err
184	184		}
185	185		log := zerowrap.FromCtx(ctx)


@@ -179,7 +179,7 @@ func (u *Pushes) Check(ctx context.Context, p domain.Principal, name domain.Repo
179		// recorded before Git moves them, so that a tip never has the record of an	179		// recorded before Git moves them, so that a tip never has the record of an
180		// earlier pusher at the same commit (domain.CIRefTip). A record that cannot	180		// earlier pusher at the same commit (domain.CIRefTip). A record that cannot
181		// be written refuses the push, as it cannot be run later with a known trust.	181		// be written refuses the push, as it cannot be run later with a known trust.
182	-	if err := recordRefTips(ctx, u.tips, name, updates, authorOf(p), domain.PusherOf(p), domain.TrustedActor(p)); err != nil {	182	+	if err := recordRefTips(ctx, u.tips, name, updates, domain.AuthorName(p), domain.PusherOf(p), domain.TrustedActor(p)); err != nil {
183		return err	183		return err
184		}	184		}
185		log := zerowrap.FromCtx(ctx)	185		log := zerowrap.FromCtx(ctx)

M proto/ermite/storage/merge_request.proto +12 -0


@@ -64,6 +64,13 @@ message MergeRequest {
64	64		int64 comment_seq = 14;
65	65		}
66	66
	67	+	// Verdict is the immutable review decision of a note.
	68	+	enum Verdict {
	69	+	VERDICT_UNSPECIFIED = 0;
	70	+	VERDICT_APPROVE = 1;
	71	+	VERDICT_CHANGES_REQUESTED = 2;
	72	+	}
	73	+
67	74		// Comment is stored under the key "mrc/<repo>/<number:010d>/<id:010d>".
68	75		message Comment {
69	76		int64 id = 1;

@@ -75,6 +82,11 @@ message Comment {
75	82		string text = 7;
76	83		bool resolved = 8;
77	84		google.protobuf.Timestamp created_at = 9;
	85	+	int64 reply_to = 10;
	86	+	int32 start_line = 11;
	87	+	Verdict verdict = 12;
	88	+	google.protobuf.Timestamp edited_at = 13;
	89	+	bool deleted = 14;
78	90		}
79	91
80	92		// Sequence is stored under the key "mrseq/<repo>": the last merge request


@@ -64,6 +64,13 @@ message MergeRequest {
64	int64 comment_seq = 14;	64		int64 comment_seq = 14;
65	}	65		}
66		66
		67	+	// Verdict is the immutable review decision of a note.
		68	+	enum Verdict {
		69	+	VERDICT_UNSPECIFIED = 0;
		70	+	VERDICT_APPROVE = 1;
		71	+	VERDICT_CHANGES_REQUESTED = 2;
		72	+	}
		73	+
67	// Comment is stored under the key "mrc/<repo>/<number:010d>/<id:010d>".	74		// Comment is stored under the key "mrc/<repo>/<number:010d>/<id:010d>".
68	message Comment {	75		message Comment {
69	int64 id = 1;	76		int64 id = 1;

@@ -75,6 +82,11 @@ message Comment {
75	string text = 7;	82		string text = 7;
76	bool resolved = 8;	83		bool resolved = 8;
77	google.protobuf.Timestamp created_at = 9;	84		google.protobuf.Timestamp created_at = 9;
		85	+	int64 reply_to = 10;
		86	+	int32 start_line = 11;
		87	+	Verdict verdict = 12;
		88	+	google.protobuf.Timestamp edited_at = 13;
		89	+	bool deleted = 14;
78	}	90		}
79		91
80	// Sequence is stored under the key "mrseq/<repo>": the last merge request	92		// Sequence is stored under the key "mrseq/<repo>": the last merge request