#1 feat(mr): notes with threads, line ranges, edits and verdicts

state: merged · 86ac875 · fast-forward
branch: feat/mr-notes → main
opened: bnema · 3d ago
updated: 3d ago

Merge request notes (ADR 0032): one uniform review object, published at once, no draft review.

Threads (one level, resolution on the root), line ranges on one side, author edit and delete.
Notes and descriptions rendered as safe Markdown (no raw HTML, link policy), ^N links to note N.
Optional verdict on a root note (approve, changes requested), needs the new mr:review scope; informative only; latest per author; approvals stale after a push; changes requested stay until the author's next verdict; immutable.
Events comment_edited, comment_deleted, comment_resolved.
RPC, CLI (mr comment --reply/--line A-B/--approve/--request-changes, mr edit-comment, mr delete-comment, show and watch), web (threads, quote, Shift+click ranges with a no-JS fallback, verdict list), docs and the ermite skill.

Depends on the ermite-api MR (feat/mr-notes). Until ermite-api v0.8.0 is tagged this branch builds only with a go.work that uses that branch; a last commit will require v0.8.0.

Upgrade: mr:review is a new scope, so a token that held every scope loses the owner's rights until it is recreated (--preset cli) with the root token or from an owner web session.

Checks (with the API branch in a go.work): build, go test -race, go test ./internal/app, golangci-lint --new-from-rev=main, hexcheck, templ drift, buf lint/breaking/generate, mocks drift, node tests (145). Reviewed by three read-only reviewers plus a targeted re-review; all findings fixed. Not verified yet: manual browser check of the merge request page.

Commit	Subject	Author	Date	Signature
86ac875	Merge remote-tracking branch 'origin/main' into feat/mr-notes	brice	3d ago	GPG Verified
77b3426	build(deps): ermite-api v0.8.0	brice	3d ago	GPG Verified
c064166	fix(web): split-only note links and disjoint heading ids of split note copies	brice	3d ago	GPG Verified
06682af	fix(mr): deleted verdict notes, split-layout note links and verdict labels	brice	3d ago	GPG Verified
3872b20	test(mr): stronger note retry and range tests	brice	3d ago	GPG Verified
31c9416	docs(mr): notes, threads, ranges and verdicts	brice	3d ago	GPG Verified
f8d5320	merge: web for merge request notes	brice	3d ago	GPG Verified
aec37bf	feat(web): threads, line ranges, Markdown notes, edits and verdicts on merge requests	brice	3d ago	GPG Verified
945925c	merge: RPC and CLI for merge request notes	brice	3d ago	GPG Verified
8def6de	feat(cli): replies, ranges, verdicts, edit and delete for merge request notes	brice	3d ago	GPG Verified
aa64b62	fix(mr): recheck the reply target when a note save is retried	brice	3d ago	GPG Verified
b31a354	feat(mr): replies, line ranges, edits, deletes and verdicts in the merge request use case	brice	3d ago	GPG Verified
3a784d4	feat(mr): note model with threads, ranges, verdicts and the mr:review scope	brice	3d ago	GPG Verified

Checks 8

CI · pull_request
Image success
Lint success
Protobuf and generated code success
Test success
CI · push
Image success
Lint success
Protobuf and generated code success
Test failure

62 files · +3086 -650

7/8

M docs/cli.md +2 -1


@@ -6,3 +6,7 @@
6	6		}
7	7		.eol { color: var(--ink-mute); padding-left: 1ch; user-select: none; }
8	8		}
	9	+
	10	+	@layer components {
	11	+	.sel { outline: 1px solid var(--ink-dim); outline-offset: -1px; }
	12	+	}


@@ -11,7 +11,6 @@
11	11		.mr-desc.prose { max-width: none; padding: var(--s4) var(--s5); background: var(--bg-surface); border: 1px solid var(--border); border-radius: var(--r); }
12	12		.mr-desc.prose > * { max-width: var(--measure); }
13	13		.mr-desc.prose > pre, .mr-desc.prose > table { max-width: 100%; }
14		-	.c-text a { text-decoration: underline; }
15	14
16	15		.since {
17	16		display: inline-flex; flex-wrap: wrap; gap: var(--s1); padding: var(--s1);

@@ -147,12 +146,11 @@
147	146		& p { white-space: normal; }
148	147		& .c-who { font-weight: bold; }
149	148		& .c-state { color: var(--ink-dim); }
150		-	& .c-text { white-space: pre-wrap; overflow-wrap: anywhere; }
151	149		& .c-where { font-size: var(--fs-s); overflow-wrap: anywhere; }
152	150		& .c-act { margin-top: var(--s2); }
153	151		& .c-act button { height: 26px; font-size: var(--fs-xs); }
154	152		&[data-resolved="true"] { border-left-color: var(--border-strong); }
155		-	&[data-resolved="true"] .c-text { color: var(--ink-dim); }
	153	+	&[data-resolved="true"] .c-body { color: var(--ink-dim); }
156	154		}
157	155		.dt .fnote td { background: var(--bg-canvas); padding: var(--s2) var(--s3); white-space: normal; }
158	156		.dt .fnote .cmt, .dt .fnote .cform { margin: 0; }

@@ -190,3 +188,10 @@
190	188		.list.mrs .c-who { display: none; }
191	189		}
192	190		}
	191	+
	192	+	@layer components {
	193	+	.c-body { white-space: normal; overflow-wrap: anywhere; }
	194	+	.c-body.prose > * { margin-block: .5em; }
	195	+	.c-body .ref { font-variant-numeric: tabular-nums; text-decoration: underline; }
	196	+	.cmt > .cmt { margin-inline-start: 2ch; }
	197	+	}

   .mr-desc.prose { max-width: none; padding: var(--s4) var(--s5); background: var(--bg-surface); border: 1px solid var(--border); border-radius: var(--r); }
   .mr-desc.prose > * { max-width: var(--measure); }
   .mr-desc.prose > pre, .mr-desc.prose > table { max-width: 100%; }
-  .c-text a { text-decoration: underline; }
   .since {
     display: inline-flex; flex-wrap: wrap; gap: var(--s1); padding: var(--s1);
     & p { white-space: normal; }
     & .c-who { font-weight: bold; }
     & .c-state { color: var(--ink-dim); }
-    & .c-text { white-space: pre-wrap; overflow-wrap: anywhere; }
     & .c-where { font-size: var(--fs-s); overflow-wrap: anywhere; }
     & .c-act { margin-top: var(--s2); }
     & .c-act button { height: 26px; font-size: var(--fs-xs); }
     &[data-resolved="true"] { border-left-color: var(--border-strong); }
-    &[data-resolved="true"] .c-text { color: var(--ink-dim); }
+    &[data-resolved="true"] .c-body { color: var(--ink-dim); }
   }
   .dt .fnote td { background: var(--bg-canvas); padding: var(--s2) var(--s3); white-space: normal; }
   .dt .fnote .cmt, .dt .fnote .cform { margin: 0; }
     .list.mrs .c-who { display: none; }
   }
 }
+@layer components {
+  .c-body { white-space: normal; overflow-wrap: anywhere; }
+  .c-body.prose > * { margin-block: .5em; }
+  .c-body .ref { font-variant-numeric: tabular-nums; text-decoration: underline; }
+  .cmt > .cmt { margin-inline-start: 2ch; }
+}


@@ -18,6 +18,25 @@
18	18		window.ermiteMR = (() => {
19	19		// core: the decisions, without the DOM.
20	20		const core = {
	21	+	rangeURL(previous, current, base) {
	22	+	const a = new URL(previous, base), b = new URL(current, base);
	23	+	const parse = (u) => /^(new\|old):(\d+):(.+)$/.exec(u.searchParams.get('on') \|\| '');
	24	+	const x = parse(a), y = parse(b);
	25	+	if (!x \|\| !y \|\| a.pathname !== b.pathname \|\| x[1] !== y[1] \|\| x[3] !== y[3] \|\| x[2] === y[2]) return null;
	26	+	const nums = [Number(x[2]), Number(y[2])].sort((a,b) => a-b);
	27	+	b.searchParams.set('on', `${y[1]}:${nums[0]}-${nums[1]}:${y[3]}`);
	28	+	return b.href;
	29	+	},
	30	+	quote(button) {
	31	+	const thread = button.closest('.cmt').closest('[data-thread]');
	32	+	const details = thread?.querySelector(':scope > details[data-reply]');
	33	+	const field = details?.querySelector('textarea');
	34	+	if (!field) return false;
	35	+	details.open = true;
	36	+	field.value = button.dataset.quote;
	37	+	field.focus();
	38	+	return true;
	39	+	},
21	40		// windowOf is the start of the window of file i (files are f1, f2, ...).
22	41		windowOf(id, size) {
23	42		const n = Number(String(id).replace(/^f/, '')) - 1;

@@ -107,16 +126,34 @@ window.ermiteMR = (() => {
107	126		function reveal(id, scroll = true) {
108	127		// Any navigation, loaded or not, makes the reveals still loading stale.
109	128		const gen = ++revealGen;
	129	+	const target = () => {
	130	+	// A page with both layouts says which shows in data-diff; a large diff
	131	+	// rendered in one layout only says it in data-view.
	132	+	const diffs = $('#diffs')?.dataset;
	133	+	if (/^c\d+$/.test(id) && (diffs?.diff \|\| diffs?.view) === 'split') {
	134	+	const split = document.getElementById(id + '-s');
	135	+	if (split) return split;
	136	+	}
	137	+	return document.getElementById(id);
	138	+	};
110	139		let ph = document.getElementById(id)?.querySelector(':scope > .dlazy');
111	140		if (!ph && /^c\d+/.test(id) && !document.getElementById(id)) {
112	141		const base = id.replace(/-s$/, '');
113	142		ph = $$('.dlazy[data-notes]').find((p) => p.dataset.notes.split(' ').includes(base));
114	143		}
115		-	if (!ph) return false;
	144	+	if (!ph) {
	145	+	const el = target();
	146	+	if (el && el.id !== id) {
	147	+	const d = el.closest('details');
	148	+	if (d) d.open = true;
	149	+	if (scroll) el.scrollIntoView({ block: 'start' });
	150	+	}
	151	+	return false;
	152	+	}
116	153		load(ph).then(() => {
117	154		// A later reveal (another file asked meanwhile) owns the view.
118	155		if (gen !== revealGen) return;
119		-	const el = document.getElementById(id);
	156	+	const el = target();
120	157		if (!el) return;
121	158		const d = el.closest('details');
122	159		if (d) d.open = true;

@@ -262,6 +299,18 @@ window.ermiteMR = (() => {
262	299		};
263	300		addEventListener('hashchange', hash);
264	301		hash();
	302	+	let lastLine = null;
	303	+	try { lastLine = sessionStorage.getItem('mr-last-line'); } catch { /* storage optional */ }
	304	+	addEventListener('click', (e) => {
	305	+	const quote = e.target.closest?.('[data-quote]');
	306	+	if (quote) { core.quote(quote); return; }
	307	+	const link = e.target.closest?.('a[href*="on="]');
	308	+	if (!link \|\| e.ctrlKey \|\| e.metaKey \|\| e.altKey) return;
	309	+	const next = e.shiftKey && lastLine && core.rangeURL(lastLine, link.href, location.href);
	310	+	lastLine = link.href;
	311	+	try { sessionStorage.setItem('mr-last-line', lastLine); } catch { /* storage optional */ }
	312	+	if (next) { e.preventDefault(); location.assign(next); }
	313	+	});
265	314		// A click outside an open menu closes it.
266	315		addEventListener('click', (e) => {
267	316		for (const d of $$('details.menu[open]')) if (!d.contains(e.target)) d.open = false;


@@ -190,6 +190,8 @@ func (s Server) routes() http.ServeMux {
190	190		mux.HandleFunc("POST /{repo}/merge-requests", s.mrAction(s.mrCreate))
191	191		mux.HandleFunc("POST /{repo}/merge-requests/{n}/comments", s.mrAction(s.mrComment))
192	192		mux.HandleFunc("POST /{repo}/merge-requests/{n}/comments/{id}/resolve", s.mrAction(s.mrResolve))
	193	+	mux.HandleFunc("POST /{repo}/merge-requests/{n}/comments/{id}/edit", s.mrAction(s.mrEditComment))
	194	+	mux.HandleFunc("POST /{repo}/merge-requests/{n}/comments/{id}/delete", s.mrAction(s.mrDeleteComment))
193	195		mux.HandleFunc("POST /{repo}/merge-requests/{n}/merge", s.mrAction(s.mrMerge))
194	196		mux.HandleFunc("POST /{repo}/merge-requests/{n}/close", s.mrAction(s.mrClose))
195	197		mux.HandleFunc("POST /{repo}/merge-requests/{n}/ready", s.mrAction(s.mrReady))


@@ -144,12 +144,28 @@ type jsonComment struct {
144	144		Text string `json:"text"`
145	145		Resolved bool `json:"resolved"`
146	146		CreatedAt *string `json:"created_at"`
	147	+	ReplyTo string `json:"reply_to"`
	148	+	StartLine int32 `json:"start_line"`
	149	+	Verdict string `json:"verdict"`
	150	+	EditedAt *string `json:"edited_at"`
	151	+	Deleted bool `json:"deleted"`
	152	+	}
	153	+
	154	+	func verdictName(v ermite.Verdict) string {
	155	+	switch v {
	156	+	case ermite.Verdict_VERDICT_APPROVE:
	157	+	return "approve"
	158	+	case ermite.Verdict_VERDICT_CHANGES_REQUESTED:
	159	+	return "changes_requested"
	160	+	}
	161	+	return ""
147	162		}
148	163
149	164		func toJSONComment(c *ermite.Comment) jsonComment {
150	165		return jsonComment{
151	166		ID: c.GetId(), Author: c.GetAuthor(), Path: c.GetPath(), Line: c.GetLine(), Side: sideName(c.GetSide()),
152	167		CommitID: c.GetCommitId(), Text: c.GetText(), Resolved: c.GetResolved(), CreatedAt: jsonTime(c.GetCreatedAt()),
	168	+	ReplyTo: c.GetReplyTo(), StartLine: c.GetStartLine(), Verdict: verdictName(c.GetVerdict()), EditedAt: jsonTime(c.GetEditedAt()), Deleted: c.GetDeleted(),
153	169		}
154	170		}
155	171

@@ -163,7 +179,8 @@ unless merged).`
163	179		const jsonCommentHelp = `
164	180		JSON fields of a comment: id, author, path, line and side ("old" or "new"; empty path, line 0
165	181		and empty side for a general comment), commit_id (the head when it was written), text,
166		-	resolved, created_at.`
	182	+	resolved, created_at, reply_to (empty for roots), start_line (0 for a single line),
	183	+	verdict ("", "approve", "changes_requested"), edited_at (nullable RFC 3339), deleted.`
167	184
168	185		func newMRCmd(env cliEnv) cobra.Command {
169	186		mr := &cobra.Command{

@@ -175,7 +192,7 @@ func newMRCmd(env cliEnv) cobra.Command {
175	192		}
176	193		mr.AddCommand(
177	194		newMRCreateCmd(env), newMRListCmd(env), newMRShowCmd(env), newMRCommentCmd(env),
178		-	newMRResolveCmd(env), newMRMergeCmd(env), newMRCloseCmd(env),
	195	+	newMRResolveCmd(env), newMREditCommentCmd(env), newMRDeleteCommentCmd(env), newMRMergeCmd(env), newMRCloseCmd(env),
179	196		)
180	197		return mr
181	198		}

@@ -395,15 +412,25 @@ type jsonCommit struct {
395	412		// jsonMRDetail is the stable JSON shape of `mr show --json`.
396	413		type jsonMRDetail struct {
397	414		jsonMergeRequest
398		-	Commits []jsonCommit `json:"commits"`
399		-	CommitsTruncated bool `json:"commits_truncated"`
400		-	DiffBaseID string `json:"diff_base_id"`
401		-	Stats jsonStats `json:"stats"`
402		-	DiffTruncated bool `json:"diff_truncated"`
403		-	Files []jsonFile `json:"files"`
404		-	Comments []jsonComment `json:"comments"`
405		-	LastReadID string `json:"last_read_id"`
406		-	LastReadLost bool `json:"last_read_lost"`
	415	+	Commits []jsonCommit `json:"commits"`
	416	+	CommitsTruncated bool `json:"commits_truncated"`
	417	+	DiffBaseID string `json:"diff_base_id"`
	418	+	Stats jsonStats `json:"stats"`
	419	+	DiffTruncated bool `json:"diff_truncated"`
	420	+	Files []jsonFile `json:"files"`
	421	+	Comments []jsonComment `json:"comments"`
	422	+	Verdicts []jsonAuthorVerdict `json:"verdicts"`
	423	+	LastReadID string `json:"last_read_id"`
	424	+	LastReadLost bool `json:"last_read_lost"`
	425	+	}
	426	+
	427	+	type jsonAuthorVerdict struct {
	428	+	Author string `json:"author"`
	429	+	Verdict string `json:"verdict"`
	430	+	CommentID string `json:"comment_id"`
	431	+	CommitID string `json:"commit_id"`
	432	+	CreatedAt *string `json:"created_at"`
	433	+	Stale bool `json:"stale"`
407	434		}
408	435
409	436		func fileStatusName(s ermite.FileStatus) string {

@@ -423,7 +450,7 @@ func toJSONDetail(d *ermite.GetMergeRequestResponse, withDiff bool, baseURL stri
423	450		FilesChanged: d.GetStats().GetFilesChanged(), Additions: d.GetStats().GetAdditions(), Deletions: d.GetStats().GetDeletions(),
424	451		},
425	452		DiffTruncated: d.GetDiff().GetTruncated(),
426		-	Files: []jsonFile{}, Comments: []jsonComment{},
	453	+	Files: []jsonFile{}, Comments: []jsonComment{}, Verdicts: []jsonAuthorVerdict{},
427	454		LastReadID: d.GetLastReadId(), LastReadLost: d.GetLastReadLost(),
428	455		}
429	456		for _, c := range d.GetCommits() {

@@ -453,6 +480,9 @@ func toJSONDetail(d *ermite.GetMergeRequestResponse, withDiff bool, baseURL stri
453	480		for _, c := range d.GetComments() {
454	481		out.Comments = append(out.Comments, toJSONComment(c))
455	482		}
	483	+	for _, v := range d.GetVerdicts() {
	484	+	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()})
	485	+	}
456	486		return out
457	487		}
458	488

@@ -471,7 +501,7 @@ func newMRShowCmd(env cliEnv) cobra.Command {
471	501		"diff_base_id (the commit the diff starts from), stats (files_changed, additions, deletions),\n" +
472	502		"diff_truncated, files (old_path, new_path, status, additions, deletions, binary, truncated and,\n" +
473	503		"with --diff, hunks of lines with kind, old_number, new_number and text), comments, last_read_id\n" +
474		-	"and last_read_lost.",
	504	+	"and last_read_lost; verdicts (author, verdict, comment_id, commit_id, created_at, stale).",
475	505		Args: cobra.ExactArgs(2),
476	506		ValidArgsFunction: completeMRRepoThenNumber(env),
477	507		}

@@ -539,6 +569,25 @@ func printMRDetail(w io.Writer, d *ermite.GetMergeRequestResponse, withDiff, sin
539	569		if m.GetState() == ermite.MergeRequestState_MERGE_REQUEST_STATE_MERGED {
540	570		pairs = append(pairs, [2]string{"merged", orDash(m.GetMergedCommitId()) + " (" + modeName(m.GetMergeMode()) + ")"})
541	571		}
	572	+	if len(d.GetVerdicts()) > 0 {
	573	+	var verdicts []string
	574	+	for _, v := range d.GetVerdicts() {
	575	+	var label string
	576	+	switch v.GetVerdict() {
	577	+	case ermite.Verdict_VERDICT_APPROVE:
	578	+	label = "approved"
	579	+	case ermite.Verdict_VERDICT_CHANGES_REQUESTED:
	580	+	label = "changes requested"
	581	+	default:
	582	+	label = fmt.Sprintf("verdict %d", v.GetVerdict())
	583	+	}
	584	+	if v.GetStale() {
	585	+	label += " (stale)"
	586	+	}
	587	+	verdicts = append(verdicts, v.GetAuthor()+" "+label)
	588	+	}
	589	+	pairs = append(pairs, [2]string{"verdicts", strings.Join(verdicts, ", ")})
	590	+	}
542	591		if err := fields(w, pairs); err != nil {
543	592		return err
544	593		}

@@ -571,16 +620,46 @@ func printMRDetail(w io.Writer, d *ermite.GetMergeRequestResponse, withDiff, sin
571	620		}
572	621
573	622		_, _ = fmt.Fprintf(w, "\ncomments (%d)\n", len(d.GetComments()))
574		-	for _, c := range d.GetComments() {
	623	+	var ordered []*ermite.Comment
	624	+	for _, root := range d.GetComments() {
	625	+	if root.GetReplyTo() != "" {
	626	+	continue
	627	+	}
	628	+	ordered = append(ordered, root)
	629	+	for _, reply := range d.GetComments() {
	630	+	if reply.GetReplyTo() == root.GetId() {
	631	+	ordered = append(ordered, reply)
	632	+	}
	633	+	}
	634	+	}
	635	+	for _, c := range ordered {
575	636		where := "general"
576	637		if c.GetPath() != "" {
577	638		where = fmt.Sprintf("%s:%d (%s)", clean(c.GetPath()), c.GetLine(), sideName(c.GetSide()))
578	639		}
	640	+	if c.GetStartLine() > 0 {
	641	+	where = fmt.Sprintf("%s:lines %d-%d (%s)", clean(c.GetPath()), c.GetStartLine(), c.GetLine(), sideName(c.GetSide()))
	642	+	}
579	643		mark := ""
	644	+	if c.GetEditedAt() != nil {
	645	+	mark += " (edited)"
	646	+	}
	647	+	if c.GetDeleted() {
	648	+	mark += " [deleted]"
	649	+	}
	650	+	if c.GetVerdict() != ermite.Verdict_VERDICT_UNSPECIFIED {
	651	+	mark += " [" + verdictName(c.GetVerdict()) + "]"
	652	+	}
580	653		if c.GetResolved() {
581		-	mark = " [resolved]"
	654	+	mark += " [resolved]"
582	655		}
583		-	_, _ = fmt.Fprintf(w, " #%s %s, %s, %s%s\n%s\n", c.GetId(), clean(c.GetAuthor()), shortTime(c.GetCreatedAt(), "-"), where, mark, indentText(c.GetText()))
	656	+	prefix := " "
	657	+	body := indentText(c.GetText())
	658	+	if c.GetReplyTo() != "" {
	659	+	prefix += " "
	660	+	body = " " + strings.ReplaceAll(body, "\n", "\n ")
	661	+	}
	662	+	_, _ = fmt.Fprintf(w, "%s#%s %s, %s, %s%s\n%s\n", prefix, c.GetId(), clean(c.GetAuthor()), shortTime(c.GetCreatedAt(), "-"), where, mark, body)
584	663		}
585	664		if withDiff {
586	665		printDiff(w, d.GetDiff())

@@ -653,12 +732,14 @@ func printDiff(w io.Writer, d *ermite.Diff) {
653	732		}
654	733
655	734		func newMRCommentCmd(env cliEnv) cobra.Command {
656		-	var message, path, side string
657		-	var line int
	735	+	var message, path, side, line, reply string
	736	+	var approve, requestChanges bool
658	737		cmd := &cobra.Command{
659	738		Use: "comment <repo> <number>",
660	739		Short: "Comment on a merge request",
661		-	Long: "Add a plain-text comment (at most 8000 characters, no control characters except line breaks).\n" +
	740	+	Long: "Add a Markdown note (at most 8000 characters); ^N refers to a note in this merge request.\n" +
	741	+	"--reply replies to a root note. --line accepts N or A-B. --approve and --request-changes\n" +
	742	+	"set a verdict (needs mr:review) and allow an empty message.\n" +
662	743		"-m - reads it from standard input. Without --path it is a general comment. With --path, --line and\n" +
663	744		"--side (default new) it is attached to a line of the current diff: the path as in `mr show --diff`\n" +
664	745		"(the new path, the old one for a deleted file), the line number on the old or new side; a line\n" +

@@ -666,11 +747,18 @@ func newMRCommentCmd(env cliEnv) cobra.Command {
666	747		Args: cobra.ExactArgs(2),
667	748		ValidArgsFunction: completeMRRepoThenNumber(env),
668	749		}
669		-	cmd.Flags().StringVarP(&message, "message", "m", "", "text of the comment, or - to read standard input (required)")
	750	+	cmd.Flags().StringVarP(&message, "message", "m", "", "Markdown text, or - to read standard input (required unless setting a verdict)")
670	751		cmd.Flags().StringVar(&path, "path", "", "file the comment is attached to")
671		-	cmd.Flags().IntVar(&line, "line", 0, "1-based line number on --side")
	752	+	cmd.Flags().StringVar(&line, "line", "", "1-based line N or range A-B on --side")
	753	+	cmd.Flags().StringVar(&reply, "reply", "", "reply to root note (^12, #12 or 12)")
	754	+	cmd.Flags().BoolVar(&approve, "approve", false, "approve (needs mr:review)")
	755	+	cmd.Flags().BoolVar(&requestChanges, "request-changes", false, "request changes (needs mr:review)")
672	756		cmd.Flags().StringVar(&side, "side", sideNew, "side the line counts on: old or new")
673		-	_ = cmd.MarkFlagRequired("message")
	757	+	cmd.MarkFlagsMutuallyExclusive("approve", "request-changes")
	758	+	for _, flag := range []string{"path", "line", "side", "approve", "request-changes"} {
	759	+	cmd.MarkFlagsMutuallyExclusive("reply", flag)
	760	+	}
	761	+	// A verdict note may omit its message.
674	762		_ = cmd.RegisterFlagCompletionFunc("side", func(*cobra.Command, []string, string) ([]string, cobra.ShellCompDirective) {
675	763		return []string{sideOld, sideNew}, cobra.ShellCompDirectiveNoFileComp
676	764		})

@@ -684,17 +772,36 @@ func newMRCommentCmd(env cliEnv) cobra.Command {
684	772		if err != nil {
685	773		return err
686	774		}
	775	+	if !cmd.Flags().Changed("message") && !approve && !requestChanges {
	776	+	return errors.New("--message is required unless --approve or --request-changes is set")
	777	+	}
687	778		req := &ermite.AddCommentRequest{Repo: args[0], Number: n, Text: text}
	779	+	if approve {
	780	+	req.Verdict = ermite.Verdict_VERDICT_APPROVE
	781	+	}
	782	+	if requestChanges {
	783	+	req.Verdict = ermite.Verdict_VERDICT_CHANGES_REQUESTED
	784	+	}
	785	+	if cmd.Flags().Changed("reply") {
	786	+	req.ReplyTo, err = parseNoteID(reply)
	787	+	if err != nil {
	788	+	return err
	789	+	}
	790	+	}
688	791		switch {
689	792		case path != "":
690		-	if line < 1 {
	793	+	if line == "" {
691	794		return errors.New("--line is required with --path: give the line number")
692	795		}
	796	+	req.StartLine, req.Line, err = parseLineRange(line)
	797	+	if err != nil {
	798	+	return err
	799	+	}
693	800		if req.Side, err = parseSide(side); err != nil {
694	801		return err
695	802		}
696		-	req.Path, req.Line = path, int32(line) //nolint:gosec // a line number typed by a person
697		-	case line != 0 \|\| cmd.Flags().Changed("side"):
	803	+	req.Path = path
	804	+	case cmd.Flags().Changed("line") \|\| cmd.Flags().Changed("side"):
698	805		return errors.New("--line and --side need --path")
699	806		}
700	807		c, err := env.client(cmd)

@@ -714,6 +821,100 @@ func newMRCommentCmd(env cliEnv) cobra.Command {
714	821		return cmd
715	822		}
716	823
	824	+	func parseNoteID(s string) (string, error) {
	825	+	id := strings.TrimLeft(strings.TrimSpace(s), "^#")
	826	+	n, err := strconv.ParseInt(id, 10, 64)
	827	+	if err != nil \|\| n < 1 {
	828	+	return "", fmt.Errorf("invalid note id %q: use ^12, #12 or 12", s)
	829	+	}
	830	+	return strconv.FormatInt(n, 10), nil
	831	+	}
	832	+
	833	+	func parseLineRange(s string) (start, end int32, err error) {
	834	+	parts := strings.Split(s, "-")
	835	+	invalid := fmt.Errorf("invalid --line %q: use N or A-B with 1 <= A < B", s)
	836	+	if len(parts) < 1 \|\| len(parts) > 2 {
	837	+	return 0, 0, invalid
	838	+	}
	839	+	parse := func(v string) (int32, error) {
	840	+	n, e := strconv.ParseInt(v, 10, 32)
	841	+	if e != nil \|\| n < 1 {
	842	+	return 0, invalid
	843	+	}
	844	+	return int32(n), nil
	845	+	}
	846	+	end, err = parse(parts[len(parts)-1])
	847	+	if err != nil {
	848	+	return 0, 0, err
	849	+	}
	850	+	if len(parts) == 2 {
	851	+	start, err = parse(parts[0])
	852	+	if err != nil \|\| start >= end {
	853	+	return 0, 0, invalid
	854	+	}
	855	+	}
	856	+	return start, end, nil
	857	+	}
	858	+
	859	+	func newMREditCommentCmd(env cliEnv) cobra.Command { return newMRNoteMutationCmd(env, true) }
	860	+	func newMRDeleteCommentCmd(env cliEnv) cobra.Command { return newMRNoteMutationCmd(env, false) }
	861	+
	862	+	func newMRNoteMutationCmd(env cliEnv, edit bool) cobra.Command {
	863	+	verb := "delete"
	864	+	if edit {
	865	+	verb = "edit"
	866	+	}
	867	+	var message string
	868	+	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)}
	869	+	if edit {
	870	+	cmd.Flags().StringVarP(&message, "message", "m", "", "Markdown text, or - to read standard input (required)")
	871	+	_ = cmd.MarkFlagRequired("message")
	872	+	}
	873	+	asJSON := addJSONFlag(cmd)
	874	+	cmd.RunE = func(cmd *cobra.Command, args []string) error {
	875	+	n, err := parseNumber(args[1])
	876	+	if err != nil {
	877	+	return err
	878	+	}
	879	+	id, err := parseNoteID(args[2])
	880	+	if err != nil {
	881	+	return err
	882	+	}
	883	+	text, err := readText(cmd, message)
	884	+	if err != nil {
	885	+	return err
	886	+	}
	887	+	c, err := env.client(cmd)
	888	+	if err != nil {
	889	+	return err
	890	+	}
	891	+	var note *ermite.Comment
	892	+	if edit {
	893	+	res, e := c.MergeRequests().EditComment(env.ctx(cmd), connect.NewRequest(&ermite.EditCommentRequest{Repo: args[0], Number: n, CommentId: id, Text: text}))
	894	+	if e != nil {
	895	+	return c.Describe(e)
	896	+	}
	897	+	note = res.Msg.GetComment()
	898	+	} else {
	899	+	res, e := c.MergeRequests().DeleteComment(env.ctx(cmd), connect.NewRequest(&ermite.DeleteCommentRequest{Repo: args[0], Number: n, CommentId: id}))
	900	+	if e != nil {
	901	+	return c.Describe(e)
	902	+	}
	903	+	note = res.Msg.GetComment()
	904	+	}
	905	+	if *asJSON {
	906	+	return writeJSON(cmd.OutOrStdout(), toJSONComment(note))
	907	+	}
	908	+	past := "deleted"
	909	+	if edit {
	910	+	past = "edited"
	911	+	}
	912	+	_, err = fmt.Fprintf(cmd.OutOrStdout(), "comment %s of %s!%d %s\n", clean(id), clean(args[0]), n, past)
	913	+	return err
	914	+	}
	915	+	return cmd
	916	+	}
	917	+
717	918		func newMRResolveCmd(env cliEnv) cobra.Command {
718	919		var undo bool
719	920		cmd := &cobra.Command{


@@ -421,3 +421,75 @@ func (w watchProc) nextOfType(t testing.T, typ string) eventOut {
421	421		t.Fatalf("no %s event", typ)
422	422		return eventOut{}
423	423		}
	424	+
	425	+	func TestMergeRequestNotesCLIEndToEnd(t *testing.T) {
	426	+	c := newCLIHarness(t)
	427	+	c.cli(t, adminToken, "repo", "create", "notes", "--visibility", "public").ok(t)
	428	+	wt := c.workTree("main", "README.md")
	429	+	c.mustGit(wt, "checkout", "-q", "-b", "feat/notes")
	430	+	c.writeFile(wt, "notes.txt", []byte("one\ntwo\nthree\nfour\n"))
	431	+	c.mustGit(wt, "push", "-q", c.urlAuth(adminToken, "/notes.git"), "main", "feat/notes")
	432	+	c.cli(t, adminToken, "mr", "create", "notes", "--source", "feat/notes", "--title", "Notes").ok(t)
	433	+	w := c.startWatch(t, adminToken)
	434	+	res := c.cli(t, adminToken, "mr", "comment", "notes", "1", "--path", "notes.txt", "--line", "1-3", "-m", "range", "--json").ok(t)
	435	+	root := decode[jsonComment](t, res.stdout)
	436	+	assert.EqualValues(t, 1, root.StartLine)
	437	+	assert.EqualValues(t, 3, root.Line)
	438	+	w.nextOfType(t, "comment_added")
	439	+	res = c.cli(t, adminToken, "mr", "comment", "notes", "1", "--reply", "^"+root.ID, "-m", "reply to ^"+root.ID, "--json").ok(t)
	440	+	reply := decode[jsonComment](t, res.stdout)
	441	+	assert.Equal(t, root.ID, reply.ReplyTo)
	442	+	assert.Equal(t, root.Path, reply.Path)
	443	+	assert.Equal(t, root.StartLine, reply.StartLine)
	444	+	assert.Equal(t, root.Line, reply.Line)
	445	+	w.nextOfType(t, "comment_added")
	446	+	require.Error(t, c.cli(t, adminToken, "mr", "comment", "notes", "1", "--reply", reply.ID, "-m", "nested").err)
	447	+	require.Error(t, c.cli(t, adminToken, "mr", "resolve", "notes", "1", reply.ID).err)
	448	+	writer := c.token([]domain.Scope{domain.ScopeRepoRead, domain.ScopeMRWrite})
	449	+	denied := c.cli(t, writer, "mr", "comment", "notes", "1", "--approve")
	450	+	require.Error(t, denied.err)
	451	+	assert.Contains(t, denied.err.Error(), "mr:review")
	452	+	res = c.cli(t, adminToken, "mr", "comment", "notes", "1", "--approve", "--json").ok(t)
	453	+	verdict := decode[jsonComment](t, res.stdout)
	454	+	assert.Equal(t, "approve", verdict.Verdict)
	455	+	assert.True(t, verdict.Resolved)
	456	+	assert.Empty(t, verdict.Text)
	457	+	w.nextOfType(t, "comment_added")
	458	+	require.Error(t, c.cli(t, writer, "mr", "edit-comment", "notes", "1", verdict.ID, "-m", "not mine").err)
	459	+	res = runCLI(t, c.vars(adminToken), "edited from stdin\n", "mr", "edit-comment", "notes", "1", "#"+reply.ID, "-m", "-", "--json").ok(t)
	460	+	edited := decode[jsonComment](t, res.stdout)
	461	+	assert.Equal(t, "edited from stdin", edited.Text)
	462	+	require.NotNil(t, edited.EditedAt)
	463	+	w.nextOfType(t, "comment_edited")
	464	+	res = c.cli(t, adminToken, "mr", "delete-comment", "notes", "1", verdict.ID, "--json").ok(t)
	465	+	deleted := decode[jsonComment](t, res.stdout)
	466	+	assert.True(t, deleted.Deleted)
	467	+	assert.Equal(t, "approve", deleted.Verdict)
	468	+	assert.Empty(t, deleted.Text)
	469	+	require.NotNil(t, deleted.EditedAt)
	470	+	w.nextOfType(t, "comment_deleted")
	471	+	c.cli(t, adminToken, "mr", "delete-comment", "notes", "1", verdict.ID).ok(t)
	472	+	c.cli(t, adminToken, "mr", "resolve", "notes", "1", root.ID).ok(t)
	473	+	w.nextOfType(t, "comment_resolved")
	474	+	res = c.cli(t, "", "mr", "show", "notes", "1", "--json").ok(t)
	475	+	detail := decode[jsonMRDetail](t, res.stdout)
	476	+	require.Len(t, detail.Comments, 3)
	477	+	require.Len(t, detail.Verdicts, 1)
	478	+	assert.Equal(t, verdict.ID, detail.Verdicts[0].CommentID)
	479	+	assert.Equal(t, "approve", detail.Verdicts[0].Verdict)
	480	+	assert.False(t, detail.Verdicts[0].Stale)
	481	+	res = c.cli(t, "", "mr", "show", "notes", "1").ok(t)
	482	+	assert.Contains(t, res.stdout, "lines 1-3")
	483	+	assert.Contains(t, res.stdout, "(edited)")
	484	+	assert.Contains(t, res.stdout, "[deleted]")
	485	+	assert.Contains(t, res.stdout, "approved")
	486	+	assert.Contains(t, res.stdout, " #"+reply.ID)
	487	+	c.cli(t, adminToken, "mr", "comment", "notes", "1", "--request-changes", "-m", "needs work").ok(t)
	488	+	res = c.cli(t, "", "mr", "show", "notes", "1", "--json").ok(t)
	489	+	detail = decode[jsonMRDetail](t, res.stdout)
	490	+	require.Len(t, detail.Verdicts, 1)
	491	+	assert.Equal(t, "changes_requested", detail.Verdicts[0].Verdict)
	492	+	c.cli(t, adminToken, "mr", "close", "notes", "1").ok(t)
	493	+	require.Error(t, c.cli(t, adminToken, "mr", "edit-comment", "notes", "1", root.ID, "-m", "late").err)
	494	+	require.Error(t, c.cli(t, adminToken, "mr", "delete-comment", "notes", "1", root.ID).err)
	495	+	}


@@ -388,6 +388,95 @@ func TestSigningKeyShowOutput(t *testing.T) {
388	388		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)
389	389		}
390	390
	391	+	func TestNoteRangesAndIDs(t *testing.T) {
	392	+	for _, tc := range []struct {
	393	+	input string
	394	+	start, end int32
	395	+	}{{"1", 0, 1}, {"10-24", 10, 24}, {"2147483647", 0, 2147483647}} {
	396	+	start, end, err := parseLineRange(tc.input)
	397	+	require.NoError(t, err)
	398	+	assert.Equal(t, tc.start, start)
	399	+	assert.Equal(t, tc.end, end)
	400	+	}
	401	+	for _, input := range []string{"", "0", "-1", "1-1", "24-10", "0-2", "1-2-3", "2147483648", "a", "1-"} {
	402	+	_, _, err := parseLineRange(input)
	403	+	require.Error(t, err, input)
	404	+	}
	405	+	for _, input := range []string{"12", "^12", "#12"} {
	406	+	got, err := parseNoteID(input)
	407	+	require.NoError(t, err)
	408	+	assert.Equal(t, "12", got)
	409	+	}
	410	+	}
	411	+
	412	+	func TestNoteFlags(t *testing.T) {
	413	+	f := &fakeAPI{addComment: func(r ermite.AddCommentRequest) (ermite.AddCommentResponse, error) {
	414	+	return &ermite.AddCommentResponse{Comment: &ermite.Comment{Id: "1", ReplyTo: r.GetReplyTo(), StartLine: r.GetStartLine(), Line: r.GetLine(), Verdict: r.GetVerdict()}}, nil
	415	+	}}
	416	+	url := f.serve(t)
	417	+	for _, tc := range []struct {
	418	+	flags []string
	419	+	reply string
	420	+	start int32
	421	+	verdict string
	422	+	}{
	423	+	{[]string{"--reply", "^12", "-m", "reply"}, "12", 0, ""},
	424	+	{[]string{"--path", "x", "--line", "10-24", "-m", "range"}, "", 10, ""},
	425	+	{[]string{"--approve"}, "", 0, "approve"},
	426	+	{[]string{"--request-changes"}, "", 0, "changes_requested"},
	427	+	} {
	428	+	res := runCLI(t, envFor(url), "", append([]string{"mr", "comment", "alpha", "1", "--json"}, tc.flags...)...).ok(t)
	429	+	got := decode[jsonComment](t, res.stdout)
	430	+	assert.Equal(t, tc.reply, got.ReplyTo)
	431	+	assert.Equal(t, tc.start, got.StartLine)
	432	+	assert.Equal(t, tc.verdict, got.Verdict)
	433	+	}
	434	+	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"}} {
	435	+	require.Error(t, runCLI(t, envFor(url), "", append([]string{"mr", "comment", "alpha", "1", "-m", "x"}, flags...)...).err)
	436	+	}
	437	+	}
	438	+
	439	+	func TestNoteDetailOutput(t *testing.T) {
	440	+	d := &ermite.GetMergeRequestResponse{MergeRequest: mrOne, Comments: []*ermite.Comment{
	441	+	{Id: "1", Path: "x", StartLine: 10, Line: 24, Text: "root"},
	442	+	{Id: "3", Text: "other", Deleted: true},
	443	+	{Id: "2", ReplyTo: "1", Text: "reply", EditedAt: ts("2026-10-10T12:00:00Z")},
	444	+	}, Verdicts: []*ermite.AuthorVerdict{
	445	+	{Author: "owner", Verdict: ermite.Verdict_VERDICT_APPROVE, CommentId: "1", Stale: true},
	446	+	{Author: "bot", Verdict: ermite.Verdict_VERDICT_CHANGES_REQUESTED, CommentId: "3"},
	447	+	}}
	448	+	var out strings.Builder
	449	+	require.NoError(t, printMRDetail(&out, d, false, false))
	450	+	text := out.String()
	451	+	assert.Contains(t, text, "owner approved (stale), bot changes requested")
	452	+	assert.Contains(t, text, "lines 10-24")
	453	+	assert.Contains(t, text, "(edited)")
	454	+	assert.Contains(t, text, "[deleted]")
	455	+	assert.Less(t, strings.Index(text, " #2"), strings.Index(text, " #3"))
	456	+	got := toJSONDetail(d, false, "https://git.example.org")
	457	+	require.Len(t, got.Verdicts, 2)
	458	+	assert.True(t, got.Verdicts[0].Stale)
	459	+	assert.Equal(t, "changes_requested", got.Verdicts[1].Verdict)
	460	+	assert.Equal(t, "1", got.Comments[2].ReplyTo)
	461	+	assert.NotNil(t, got.Comments[2].EditedAt)
	462	+	}
	463	+
	464	+	func TestUnknownVerdictDetailOutput(t *testing.T) {
	465	+	var out strings.Builder
	466	+	require.NoError(t, printMRDetail(&out, &ermite.GetMergeRequestResponse{MergeRequest: mrOne, Verdicts: []*ermite.AuthorVerdict{{Author: "future", Verdict: ermite.Verdict(99)}}}, false, false))
	467	+	assert.Contains(t, out.String(), "future verdict 99")
	468	+	assert.NotContains(t, out.String(), "changes requested")
	469	+	assert.NotContains(t, out.String(), "approved")
	470	+	}
	471	+
	472	+	func TestMRReviewScopeCompletion(t *testing.T) {
	473	+	cmd := newTokenCreateCmd(&cliEnv{})
	474	+	complete, ok := cmd.GetFlagCompletionFunc("scope")
	475	+	require.True(t, ok)
	476	+	values, _ := complete(cmd, nil, "")
	477	+	assert.Contains(t, values, "mr:review")
	478	+	}
	479	+
391	480		func TestMRWebURL(t *testing.T) {
392	481		for _, tt := range []struct{ name, base, repo, want string }{
393	482		{"plain", "https://git.example.org", "my-repo", "https://git.example.org/my-repo/merge-requests/42"},


@@ -86,7 +86,7 @@ func newTokenCreateCmd(env cliEnv) cobra.Command {
86	86		"or store it right away); details go to standard error. With --json, standard output is one\n" +
87	87		"object holding the token fields and the secret.\n\n" +
88	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	90		" agent repo:write, mr:write\n" +
91	91		" runner runner:register (a CI runner host)\n" +
92	92		" ci repo:read\n" +


@@ -45,10 +45,13 @@ const (
45	45		eventPushed = "pushed"
46	46		eventGap = "gap"
47	47
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"
52	55		)
53	56
54	57		// toJSONEvent maps an event; ok is false for kinds this client does not know,

@@ -82,6 +85,15 @@ func toJSONEvent(ev *ermite.Event, baseURL string) (jsonEvent, bool) {
82	85		case *ermite.Event_CommentAdded:
83	86		c := toJSONComment(k.CommentAdded.GetComment())
84	87		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	97		case *ermite.Event_Gap:
86	98		out.Type = eventGap
87	99		default:

@@ -112,7 +124,7 @@ func eventLine(e jsonEvent) string {
112	124		detail = e.Repo + " " + strings.Join(parts, ", ")
113	125		case eventMROpened, eventMRMerged, eventMRClosed:
114	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	128		detail = fmt.Sprintf("%s!%d by %s", e.Repo, e.Number, e.Comment.Author)
117	129		case eventGap:
118	130		detail = "events were lost, reload what you display"

@@ -142,17 +154,18 @@ func newWatchCmd(env cliEnv) cobra.Command {
142	154		Use: "watch [repo]",
143	155		Short: "Print repository events as they happen",
144	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	158		"repository you can see, or only for [repo]. It runs until interrupted. When the connection drops it\n" +
147	159		"reconnects with a growing delay and resumes after the last event it printed; if the server no longer\n" +
148	160		"has that event it prints a gap event: reload what you display. Connection notices go to standard\n" +
149	161		"error.\n\n" +
150	162		"--json prints one JSON object per line (NDJSON). Fields: id, time (RFC 3339 UTC), type\n" +
151	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	166		"repository (the repository fields of `repo show --json`), ref_updates (array of ref, old_id,\n" +
154	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	169		"event types are skipped.",
157	170		Args: cobra.MaximumNArgs(1),
158	171		ValidArgsFunction: completeRepoNames(env),


@@ -199,3 +199,27 @@ func TestWatchMergeRequestEvents(t *testing.T) {
199	199		assert.Equal(t, "merge_request_merged", decode[eventOut](t, lines[2]).Type)
200	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, "https://git.example.org")
	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, "https://git.example.org")
		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	+	}


@@ -284,7 +284,7 @@ func TestWebMergeRequestReviewAndFastForward(t *testing.T) {
284	284		require.GreaterOrEqual(t, i, 0, "a note row under the line")
285	285		note := anon.body[i:]
286	286		assert.Less(t, strings.Index(note, "Why <i>empty</i>?"), strings.Index(note, "</tr>"), "the note sits in the row under its line")
287		-	assert.Contains(t, anon.body, `<a href="https://example.org/a" rel="noopener nofollow ugc">https://example.org/a</a>.`)
	287	+	assert.Contains(t, anon.body, `<a href="https://example.org/a" rel="nofollow noopener noreferrer">https://example.org/a</a>.`)
288	288		assert.NotContains(t, anon.body, "<i>empty</i>")
289	289		assert.Contains(t, anon.body, "Looks fine overall.")
290	290		assert.Equal(t, 2, len(reNoteID.FindAllString(anon.body, -1)))

@@ -338,7 +338,8 @@ func TestWebMergeRequestReviewAndFastForward(t *testing.T) {
338	338		assert.Contains(t, since.body, "changes since "+head1[:7])
339	339		assert.Contains(t, since.body, "extra</td>")
340	340		assert.NotContains(t, since.body, `<span class="kn">package</span> search</td>`, "only what changed after the read head")
341		-	assert.Contains(t, since.body, "Why <i>empty</i>?", "the notes are listed below, their lines are not in this diff")
	341	+	assert.Contains(t, since.body, "empty", "the notes are listed below, their lines are not in this diff")
	342	+	assert.NotContains(t, since.body, "<i>", "raw HTML is dropped")
342	343		assert.Contains(t, since.body, "search.go:3 (new)")
343	344		assert.NotContains(t, since.body, `<form class="cform"`, "no comment form on this diff")
344	345

@@ -554,7 +555,9 @@ func TestWebMergeRequestEscapesText(t *testing.T) {
554	555		assert.NotContains(t, body, "<img src=x")
555	556		assert.NotContains(t, body, "<script>x</script>")
556	557		assert.Contains(t, html.UnescapeString(body), `"><script>alert(1)</script>`)
557		-	assert.Contains(t, body, "<script>x</script> & more")
	558	+	// A note is safe Markdown: a line that opens with an HTML tag is an HTML
	559	+	// block, dropped whole.
	560	+	assert.Contains(t, body, `class="c-body prose"><!-- raw HTML omitted -->`)
558	561		assertNoInline(t, body)
559	562		assert.NotContains(t, body, `onmouseover="alert`, "an address stops at a quote")
560	563		}


@@ -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 the text of a note was edited.
	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 the text of a note was deleted.
	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 thread was resolved or reopened.
	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	+	}

 	eventPushed      = "pushed"
 	eventGap         = "gap"
-	eventMROpened     = "merge_request_opened"
+	eventMROpened        = "merge_request_opened"
-	eventMRMerged     = "merge_request_merged"
+	eventMRMerged        = "merge_request_merged"
-	eventMRClosed     = "merge_request_closed"
+	eventMRClosed        = "merge_request_closed"
-	eventCommentAdded = "comment_added"
+	eventCommentAdded    = "comment_added"
+	eventCommentEdited   = "comment_edited"
+	eventCommentDeleted  = "comment_deleted"
+	eventCommentResolved = "comment_resolved"
 )
 // toJSONEvent maps an event; ok is false for kinds this client does not know,
 	case *ermite.Event_CommentAdded:
 		c := toJSONComment(k.CommentAdded.GetComment())
 		out.Type, out.Repo, out.Number, out.Comment = eventCommentAdded, k.CommentAdded.GetRepo(), k.CommentAdded.GetNumber(), &c
+	case *ermite.Event_CommentEdited:
+		c := toJSONComment(k.CommentEdited.GetComment())
+		out.Type, out.Repo, out.Number, out.Comment = eventCommentEdited, k.CommentEdited.GetRepo(), k.CommentEdited.GetNumber(), &c
+	case *ermite.Event_CommentDeleted:
+		c := toJSONComment(k.CommentDeleted.GetComment())
+		out.Type, out.Repo, out.Number, out.Comment = eventCommentDeleted, k.CommentDeleted.GetRepo(), k.CommentDeleted.GetNumber(), &c
+	case *ermite.Event_CommentResolved:
+		c := toJSONComment(k.CommentResolved.GetComment())
+		out.Type, out.Repo, out.Number, out.Comment = eventCommentResolved, k.CommentResolved.GetRepo(), k.CommentResolved.GetNumber(), &c
 	case *ermite.Event_Gap:
 		out.Type = eventGap
 	default:
 		detail = e.Repo + " " + strings.Join(parts, ", ")
 	case eventMROpened, eventMRMerged, eventMRClosed:
 		detail = fmt.Sprintf("%s!%d %s -> %s", e.Repo, e.MergeRequest.Number, e.MergeRequest.SourceBranch, e.MergeRequest.TargetBranch)
-	case eventCommentAdded:
+	case eventCommentAdded, eventCommentEdited, eventCommentDeleted, eventCommentResolved:
 		detail = fmt.Sprintf("%s!%d by %s", e.Repo, e.Number, e.Comment.Author)
 	case eventGap:
 		detail = "events were lost, reload what you display"
 		Use:   "watch [repo]",
 		Short: "Print repository events as they happen",
 		Long: "Print one line per event (repository created, updated or deleted, refs pushed, merge requests\n" +
-			"opened, merged or closed, comments added) for every\n" +
+			"opened, merged or closed, notes added, edited, deleted or resolved) for every\n" +
 			"repository you can see, or only for [repo]. It runs until interrupted. When the connection drops it\n" +
 			"reconnects with a growing delay and resumes after the last event it printed; if the server no longer\n" +
 			"has that event it prints a gap event: reload what you display. Connection notices go to standard\n" +
 			"error.\n\n" +
 			"--json prints one JSON object per line (NDJSON). Fields: id, time (RFC 3339 UTC), type\n" +
 			"(repo_created, repo_updated, repo_deleted, pushed, merge_request_opened, merge_request_merged,\n" +
-			"merge_request_closed, comment_added or gap) and, depending on the type, repo (name),\n" +
+			"merge_request_closed, comment_added, comment_edited, comment_deleted, comment_resolved or gap)\n" +
+			"and, depending on the type, repo (name),\n" +
 			"repository (the repository fields of `repo show --json`), ref_updates (array of ref, old_id,\n" +
 			"new_id; an empty id means the ref was created or deleted), merge_request (the fields of\n" +
-			"`mr show --json`) and, for comment_added, number (of the merge request) and comment. Unknown\n" +
+			"`mr show --json`) and, for comment events, number (of the merge request) and comment. Unknown\n" +
 			"event types are skipped.",
 		Args:              cobra.MaximumNArgs(1),
 		ValidArgsFunction: completeRepoNames(env),

 	require.GreaterOrEqual(t, i, 0, "a note row under the line")
 	note := anon.body[i:]
 	assert.Less(t, strings.Index(note, "Why &lt;i&gt;empty&lt;/i&gt;?"), strings.Index(note, "</tr>"), "the note sits in the row under its line")
-	assert.Contains(t, anon.body, `<a href="https://example.org/a" rel="noopener nofollow ugc">https://example.org/a</a>.`)
+	assert.Contains(t, anon.body, `<a href="https://example.org/a" rel="nofollow noopener noreferrer">https://example.org/a</a>.`)
 	assert.NotContains(t, anon.body, "<i>empty</i>")
 	assert.Contains(t, anon.body, "Looks fine overall.")
 	assert.Equal(t, 2, len(reNoteID.FindAllString(anon.body, -1)))
 	assert.Contains(t, since.body, "changes since "+head1[:7])
 	assert.Contains(t, since.body, "extra</td>")
 	assert.NotContains(t, since.body, `<span class="kn">package</span> search</td>`, "only what changed after the read head")
-	assert.Contains(t, since.body, "Why &lt;i&gt;empty&lt;/i&gt;?", "the notes are listed below, their lines are not in this diff")
+	assert.Contains(t, since.body, "empty", "the notes are listed below, their lines are not in this diff")
+	assert.NotContains(t, since.body, "<i>", "raw HTML is dropped")
 	assert.Contains(t, since.body, "search.go:3 (new)")
 	assert.NotContains(t, since.body, `<form class="cform"`, "no comment form on this diff")
 		assert.NotContains(t, body, "<img src=x")
 		assert.NotContains(t, body, "<script>x</script>")
 		assert.Contains(t, html.UnescapeString(body), `"><script>alert(1)</script>`)
-		assert.Contains(t, body, "&lt;script&gt;x&lt;/script&gt; &amp; more")
+		// A note is safe Markdown: a line that opens with an HTML tag is an HTML
+		// block, dropped whole.
+		assert.Contains(t, body, `class="c-body prose"><!-- raw HTML omitted -->`)
 		assertNoInline(t, body)
 		assert.NotContains(t, body, `onmouseover="alert`, "an address stops at a quote")
 	}


@@ -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	+	}


@@ -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	+	}


@@ -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	+	}


@@ -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,
 	}


@@ -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)


@@ -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


@@ -360,6 +360,180 @@ func (_c *MergeRequestService_Create_Call) RunAndReturn(run func(ctx context.Con
360	360		return _c
361	361		}
362	362
	363	+	// DeleteComment provides a mock function for the type MergeRequestService
	364	+	func (_mock *MergeRequestService) DeleteComment(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, commentID int64) (domain.Comment, error) {
	365	+	ret := _mock.Called(ctx, p, name, number, commentID)
	366	+
	367	+	if len(ret) == 0 {
	368	+	panic("no return value specified for DeleteComment")
	369	+	}
	370	+
	371	+	var r0 domain.Comment
	372	+	var r1 error
	373	+	if returnFunc, ok := ret.Get(0).(func(context.Context, domain.Principal, domain.RepoName, int64, int64) (domain.Comment, error)); ok {
	374	+	return returnFunc(ctx, p, name, number, commentID)
	375	+	}
	376	+	if returnFunc, ok := ret.Get(0).(func(context.Context, domain.Principal, domain.RepoName, int64, int64) domain.Comment); ok {
	377	+	r0 = returnFunc(ctx, p, name, number, commentID)
	378	+	} else {
	379	+	r0 = ret.Get(0).(domain.Comment)
	380	+	}
	381	+	if returnFunc, ok := ret.Get(1).(func(context.Context, domain.Principal, domain.RepoName, int64, int64) error); ok {
	382	+	r1 = returnFunc(ctx, p, name, number, commentID)
	383	+	} else {
	384	+	r1 = ret.Error(1)
	385	+	}
	386	+	return r0, r1
	387	+	}
	388	+
	389	+	// MergeRequestService_DeleteComment_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'DeleteComment'
	390	+	type MergeRequestService_DeleteComment_Call struct {
	391	+	*mock.Call
	392	+	}
	393	+
	394	+	// DeleteComment is a helper method to define mock.On call
	395	+	// - ctx context.Context
	396	+	// - p domain.Principal
	397	+	// - name domain.RepoName
	398	+	// - number int64
	399	+	// - commentID int64
	400	+	func (_e MergeRequestService_Expecter) DeleteComment(ctx any, p any, name any, number any, commentID any) MergeRequestService_DeleteComment_Call {
	401	+	return &MergeRequestService_DeleteComment_Call{Call: _e.mock.On("DeleteComment", ctx, p, name, number, commentID)}
	402	+	}
	403	+
	404	+	func (_c MergeRequestService_DeleteComment_Call) Run(run func(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, commentID int64)) MergeRequestService_DeleteComment_Call {
	405	+	_c.Call.Run(func(args mock.Arguments) {
	406	+	var arg0 context.Context
	407	+	if args[0] != nil {
	408	+	arg0 = args[0].(context.Context)
	409	+	}
	410	+	var arg1 domain.Principal
	411	+	if args[1] != nil {
	412	+	arg1 = args[1].(domain.Principal)
	413	+	}
	414	+	var arg2 domain.RepoName
	415	+	if args[2] != nil {
	416	+	arg2 = args[2].(domain.RepoName)
	417	+	}
	418	+	var arg3 int64
	419	+	if args[3] != nil {
	420	+	arg3 = args[3].(int64)
	421	+	}
	422	+	var arg4 int64
	423	+	if args[4] != nil {
	424	+	arg4 = args[4].(int64)
	425	+	}
	426	+	run(
	427	+	arg0,
	428	+	arg1,
	429	+	arg2,
	430	+	arg3,
	431	+	arg4,
	432	+	)
	433	+	})
	434	+	return _c
	435	+	}
	436	+
	437	+	func (_c MergeRequestService_DeleteComment_Call) Return(comment domain.Comment, err error) MergeRequestService_DeleteComment_Call {
	438	+	_c.Call.Return(comment, err)
	439	+	return _c
	440	+	}
	441	+
	442	+	func (_c MergeRequestService_DeleteComment_Call) RunAndReturn(run func(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, commentID int64) (domain.Comment, error)) MergeRequestService_DeleteComment_Call {
	443	+	_c.Call.Return(run)
	444	+	return _c
	445	+	}
	446	+
	447	+	// EditComment provides a mock function for the type MergeRequestService
	448	+	func (_mock *MergeRequestService) EditComment(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, commentID int64, text string) (domain.Comment, error) {
	449	+	ret := _mock.Called(ctx, p, name, number, commentID, text)
	450	+
	451	+	if len(ret) == 0 {
	452	+	panic("no return value specified for EditComment")
	453	+	}
	454	+
	455	+	var r0 domain.Comment
	456	+	var r1 error
	457	+	if returnFunc, ok := ret.Get(0).(func(context.Context, domain.Principal, domain.RepoName, int64, int64, string) (domain.Comment, error)); ok {
	458	+	return returnFunc(ctx, p, name, number, commentID, text)
	459	+	}
	460	+	if returnFunc, ok := ret.Get(0).(func(context.Context, domain.Principal, domain.RepoName, int64, int64, string) domain.Comment); ok {
	461	+	r0 = returnFunc(ctx, p, name, number, commentID, text)
	462	+	} else {
	463	+	r0 = ret.Get(0).(domain.Comment)
	464	+	}
	465	+	if returnFunc, ok := ret.Get(1).(func(context.Context, domain.Principal, domain.RepoName, int64, int64, string) error); ok {
	466	+	r1 = returnFunc(ctx, p, name, number, commentID, text)
	467	+	} else {
	468	+	r1 = ret.Error(1)
	469	+	}
	470	+	return r0, r1
	471	+	}
	472	+
	473	+	// MergeRequestService_EditComment_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'EditComment'
	474	+	type MergeRequestService_EditComment_Call struct {
	475	+	*mock.Call
	476	+	}
	477	+
	478	+	// EditComment is a helper method to define mock.On call
	479	+	// - ctx context.Context
	480	+	// - p domain.Principal
	481	+	// - name domain.RepoName
	482	+	// - number int64
	483	+	// - commentID int64
	484	+	// - text string
	485	+	func (_e MergeRequestService_Expecter) EditComment(ctx any, p any, name any, number any, commentID any, text any) MergeRequestService_EditComment_Call {
	486	+	return &MergeRequestService_EditComment_Call{Call: _e.mock.On("EditComment", ctx, p, name, number, commentID, text)}
	487	+	}
	488	+
	489	+	func (_c MergeRequestService_EditComment_Call) Run(run func(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, commentID int64, text string)) MergeRequestService_EditComment_Call {
	490	+	_c.Call.Run(func(args mock.Arguments) {
	491	+	var arg0 context.Context
	492	+	if args[0] != nil {
	493	+	arg0 = args[0].(context.Context)
	494	+	}
	495	+	var arg1 domain.Principal
	496	+	if args[1] != nil {
	497	+	arg1 = args[1].(domain.Principal)
	498	+	}
	499	+	var arg2 domain.RepoName
	500	+	if args[2] != nil {
	501	+	arg2 = args[2].(domain.RepoName)
	502	+	}
	503	+	var arg3 int64
	504	+	if args[3] != nil {
	505	+	arg3 = args[3].(int64)
	506	+	}
	507	+	var arg4 int64
	508	+	if args[4] != nil {
	509	+	arg4 = args[4].(int64)
	510	+	}
	511	+	var arg5 string
	512	+	if args[5] != nil {
	513	+	arg5 = args[5].(string)
	514	+	}
	515	+	run(
	516	+	arg0,
	517	+	arg1,
	518	+	arg2,
	519	+	arg3,
	520	+	arg4,
	521	+	arg5,
	522	+	)
	523	+	})
	524	+	return _c
	525	+	}
	526	+
	527	+	func (_c MergeRequestService_EditComment_Call) Return(comment domain.Comment, err error) MergeRequestService_EditComment_Call {
	528	+	_c.Call.Return(comment, err)
	529	+	return _c
	530	+	}
	531	+
	532	+	func (_c MergeRequestService_EditComment_Call) RunAndReturn(run func(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, commentID int64, text string) (domain.Comment, error)) MergeRequestService_EditComment_Call {
	533	+	_c.Call.Return(run)
	534	+	return _c
	535	+	}
	536	+
363	537		// Get provides a mock function for the type MergeRequestService
364	538		func (_mock *MergeRequestService) Get(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, opts ports.GetMergeRequestOptions) (ports.MergeRequestDetail, error) {
365	539		ret := _mock.Called(ctx, p, name, number, opts)

Merge settings

Comments0 open / 0


@@ -113,6 +113,8 @@ type MergeRequestService interface {
113	113		List(ctx context.Context, p domain.Principal, name domain.RepoName, params ListMergeRequestsParams) (MergeRequestPage, error)
114	114		Get(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, opts GetMergeRequestOptions) (MergeRequestDetail, error)
115	115		AddComment(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, params AddCommentParams) (domain.Comment, error)
	116	+	EditComment(ctx context.Context, p domain.Principal, name domain.RepoName, number, commentID int64, text string) (domain.Comment, error)
	117	+	DeleteComment(ctx context.Context, p domain.Principal, name domain.RepoName, number, commentID int64) (domain.Comment, error)
116	118		ResolveComment(ctx context.Context, p domain.Principal, name domain.RepoName, number, commentID int64, resolved bool) (domain.Comment, error)
117	119		Close(ctx context.Context, p domain.Principal, name domain.RepoName, number int64) (domain.MergeRequest, error)
118	120		MarkRead(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, head domain.ObjectID) (domain.ObjectID, error)


@@ -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		-	}


@@ -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


@@ -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 {


@@ -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


@@ -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