Commit `dd665998be`

dd665998be8cb066e65ba05fb9d0289ff03d55ba

parent: 31c1362e52

Verified · cmc

cmc <hello@cleberg.net> · 2026-08-24T16:05:57Z

inline review comments (#2)

Diff-anchored threads on merge requests: mr diff-comment opens a
thread at (path, line, side) — validated against the MR's changed
files — or replies to a root; mr threads lists them; resolve/
unresolve by the thread author, MR author, or write access. Anchors
record the head they were made on: a force-push marks threads stale
(the review convention) instead of re-anchoring. The web diff parser
now tracks file and old/new line numbers per line, rendering fresh
threads inline and stale ones under 'threads on earlier revisions';
mr show carries the unresolved count. Diff comments notify MR
participants and join the participant set.

cmd/gitbay/main.go +4

		@@ -264,6 +264,10 @@ func mrCmd() *cobra.Command {
264	264	pass("diff", "show the diff", passOpts{server: []string{"mr", "diff"}, needsRepo: true}),
265	265	local("checkout", "fetch and check out the MR head locally: gitbay mr checkout <n>", cmdMRCheckout),
266	266	pass("comment", "comment on a merge request", passOpts{server: []string{"mr", "comment"}, needsRepo: true, stdinOK: true, editor: "comment"}),
	267	pass("diff-comment", "comment on a diff line: --path <f> --line <l> [--old] [--reply <id>]", passOpts{server: []string{"mr", "diff-comment"}, needsRepo: true, stdinOK: true, editor: "comment"}),
	268	pass("threads", "review threads on an MR", passOpts{server: []string{"mr", "threads"}, needsRepo: true}),
	269	pass("resolve", "resolve a review thread: <n> <thread-id>", passOpts{server: []string{"mr", "resolve"}, needsRepo: true}),
	270	pass("unresolve", "reopen a review thread: <n> <thread-id>", passOpts{server: []string{"mr", "unresolve"}, needsRepo: true}),
267	271	pass("review", "review: --approve\|--request-changes\|--comment", passOpts{server: []string{"mr", "review"}, needsRepo: true}),
268	272	pass("merge", "merge: [--strategy ff\|merge\|squash\|rebase]", passOpts{server: []string{"mr", "merge"}, needsRepo: true}),
269	273	pass("close", "close without merging", passOpts{server: []string{"mr", "close"}, needsRepo: true}),

docs/users.org +14

		@@ -166,6 +166,20 @@ Semantics worth knowing:
166	166	verified commits merge; everything server-created is refused with
167	167	instructions to rebase locally.
168	168
	169	Review threads anchor to diff lines:
	170
	171	#+begin_src sh
	172	gitbay mr diff-comment 4 --path main.go --line 12 --message "use log here"
	173	gitbay mr diff-comment 4 --reply 7 --message "done" # join thread 7
	174	gitbay mr threads 4 # threads with staleness
	175	gitbay mr resolve 4 7 / unresolve 4 7
	176	#+end_src
	177
	178	Threads render inline on the MR page. A force-push marks them stale
	179	(shown under "threads on earlier revisions") rather than guessing new
	180	anchors; =mr show= reports the unresolved count. Resolving is for the
	181	thread author, the MR author, or anyone with write.
	182
169	183	* Notifications
170	184
171	185	When the instance has SMTP configured, activity mails you: someone

e2e/diffcomment_test.go added +132

		@@ -0,0 +1,132 @@
	1	package e2e
	2
	3	import (
	4	"encoding/json"
	5	"fmt"
	6	"os"
	7	"path/filepath"
	8	"strings"
	9	"testing"
	10	)
	11
	12	func TestDiffComments(t *testing.T) {
	13	inst := startInstance(t)
	14	aliceKey := inst.newKey(t, "alice")
	15	bobKey := inst.newKey(t, "bob")
	16	eveKey := inst.newKey(t, "eve")
	17	inst.admin(t, "admin", "user", "create", "alice",
	18	"--key", aliceKey+".pub", "--email", "alice@example.test", "--verified")
	19	inst.admin(t, "admin", "user", "create", "bob", "--key", bobKey+".pub")
	20	inst.admin(t, "admin", "user", "create", "eve", "--key", eveKey+".pub")
	21
	22	// MR with a real multi-line diff.
	23	if _, errOut, code := inst.ssh(t, aliceKey, "", "repo", "create", "alice/lib"); code != 0 {
	24	t.Fatalf("repo create: %s", errOut)
	25	}
	26	if _, _, code := inst.ssh(t, aliceKey, "", "repo", "access", "grant", "alice/lib", "bob", "write"); code != 0 {
	27	t.Fatal("grant failed")
	28	}
	29	work := t.TempDir()
	30	env := inst.gitEnv(aliceKey)
	31	mustGit(t, work, env, "clone", inst.sshURL("alice/lib"), "w")
	32	dir := filepath.Join(work, "w")
	33	os.WriteFile(filepath.Join(dir, "main.go"), []byte("package main\n\nfunc main() {\n}\n"), 0o644)
	34	mustGit(t, dir, env, "checkout", "-q", "-b", "main")
	35	mustGit(t, dir, env, "add", ".")
	36	mustGit(t, dir, env, "commit", "-q", "-m", "base")
	37	mustGit(t, dir, env, "push", "-q", "origin", "main")
	38	mustGit(t, dir, env, "checkout", "-q", "-b", "feat")
	39	os.WriteFile(filepath.Join(dir, "main.go"), []byte("package main\n\nimport \"fmt\"\n\nfunc main() {\n\tfmt.Println(\"hi\")\n}\n"), 0o644)
	40	mustGit(t, dir, env, "add", ".")
	41	mustGit(t, dir, env, "commit", "-q", "-m", "add greeting")
	42	mustGit(t, dir, env, "push", "-q", "origin", "feat")
	43	if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "create", "alice/lib",
	44	"--source", "feat", "--target", "main", "--title", "'greeting'"); code != 0 {
	45	t.Fatalf("mr create: %s", errOut)
	46	}
	47
	48	// A thread on a real diff line; a path outside the diff is refused.
	49	if _, errOut, code := inst.ssh(t, bobKey, "", "mr", "diff-comment", "alice/lib", "1",
	50	"--path", "nope.go", "--line", "1", "--message", "'x'"); code != 2 \|\| !strings.Contains(errOut, "not part of") {
	51	t.Fatalf("off-diff path: exit %d, %s", code, errOut)
	52	}
	53	out, errOut, code := inst.ssh(t, bobKey, "", "mr", "diff-comment", "alice/lib", "1",
	54	"--path", "main.go", "--line", "6", "--message", "'use log instead of fmt'", "--json")
	55	if code != 0 {
	56	t.Fatalf("diff-comment: %s", errOut)
	57	}
	58	var env2 struct {
	59	Data struct {
	60	Thread int64 `json:"thread"`
	61	} `json:"data"`
	62	}
	63	json.Unmarshal([]byte(out), &env2)
	64	thread := env2.Data.Thread
	65
	66	// Reply joins the thread; replying to a reply is refused.
	67	out, _, code = inst.ssh(t, aliceKey, "", "mr", "diff-comment", "alice/lib", "1",
	68	"--reply", fmt.Sprint(thread), "--message", "'will do'", "--json")
	69	if code != 0 {
	70	t.Fatalf("reply failed: %s", out)
	71	}
	72	var env3 struct {
	73	Data struct {
	74	ID int64 `json:"id"`
	75	} `json:"data"`
	76	}
	77	json.Unmarshal([]byte(out), &env3)
	78	if _, errOut, code = inst.ssh(t, bobKey, "", "mr", "diff-comment", "alice/lib", "1",
	79	"--reply", fmt.Sprint(env3.Data.ID), "--message", "'nested'"); code != 2 \|\| !strings.Contains(errOut, "thread root") {
	80	t.Fatalf("nested reply: exit %d, %s", code, errOut)
	81	}
	82
	83	// Threads listing shows the thread, both comments, fresh.
	84	out, _, _ = inst.ssh(t, aliceKey, "", "mr", "threads", "alice/lib", "1", "--json")
	85	if !strings.Contains(out, `"path":"main.go"`) \|\| !strings.Contains(out, `"line":6`) \|\|
	86	!strings.Contains(out, "will do") \|\| strings.Contains(out, `"stale":true`) {
	87	t.Fatalf("threads: %s", out)
	88	}
	89
	90	// mr show counts the unresolved thread; the web renders it inline.
	91	out, _, _ = inst.ssh(t, aliceKey, "", "mr", "show", "alice/lib", "1", "--json")
	92	if !strings.Contains(out, `"unresolved_threads":1`) {
	93	t.Fatalf("mr show count: %s", out)
	94	}
	95	status, body := inst.get(t, "/alice/lib/mrs/1")
	96	if status != 200 \|\| !strings.Contains(body, "use log instead of fmt") \|\|
	97	!strings.Contains(body, `class="thread`) {
	98	t.Fatalf("web thread: %d", status)
	99	}
	100	if strings.Contains(body, "threads on earlier revisions") {
	101	t.Fatal("fresh thread rendered as detached")
	102	}
	103
	104	// Resolution: eve (read-only outsider) cannot; the thread author can;
	105	// count drops; unresolve restores it.
	106	if _, _, code = inst.ssh(t, eveKey, "", "mr", "resolve", "alice/lib", "1", fmt.Sprint(thread)); code != 4 {
	107	t.Fatal("outsider resolved a thread")
	108	}
	109	if _, errOut, code = inst.ssh(t, bobKey, "", "mr", "resolve", "alice/lib", "1", fmt.Sprint(thread)); code != 0 {
	110	t.Fatalf("resolve: %s", errOut)
	111	}
	112	out, _, _ = inst.ssh(t, aliceKey, "", "mr", "show", "alice/lib", "1", "--json")
	113	if strings.Contains(out, `"unresolved_threads"`) {
	114	t.Fatalf("resolved thread still counted: %s", out)
	115	}
	116	if _, _, code = inst.ssh(t, bobKey, "", "mr", "unresolve", "alice/lib", "1", fmt.Sprint(thread)); code != 0 {
	117	t.Fatal("unresolve failed")
	118	}
	119
	120	// Force-push moves the head: the thread goes stale and the web moves it
	121	// to the earlier-revisions section.
	122	mustGit(t, dir, env, "commit", "-q", "--amend", "-m", "add greeting (amended)")
	123	mustGit(t, dir, env, "push", "-q", "--force", "origin", "feat")
	124	out, _, _ = inst.ssh(t, aliceKey, "", "mr", "threads", "alice/lib", "1", "--json")
	125	if !strings.Contains(out, `"stale":true`) {
	126	t.Fatalf("thread not stale after force-push: %s", out)
	127	}
	128	_, body = inst.get(t, "/alice/lib/mrs/1")
	129	if !strings.Contains(body, "threads on earlier revisions") {
	130	t.Fatal("stale thread not moved to detached section")
	131	}
	132	}

internal/control/diffcomment.go added +241

		@@ -0,0 +1,241 @@
	1	package control
	2
	3	import (
	4	"errors"
	5	"fmt"
	6	"io"
	7	"slices"
	8	"strconv"
	9	"strings"
	10
	11	"gitbay.org/gitbay/internal/gitutil"
	12	"gitbay.org/gitbay/internal/policy"
	13	"gitbay.org/gitbay/internal/protocol"
	14	"gitbay.org/gitbay/internal/store"
	15	)
	16
	17	func init() {
	18	register(Command{Path: []string{"mr", "diff-comment"},
	19	Summary: "comment on a diff line: mr diff-comment <owner/name> <n> --path <file> --line <l> [--old] [--reply <id>] [--message <m> \| --file -]",
	20	ReadsStdin: true, Run: runDiffComment})
	21	register(Command{Path: []string{"mr", "threads"},
	22	Summary: "review threads on an MR: mr threads <owner/name> <n>", ReadOnly: true, Run: runMRThreads})
	23	register(Command{Path: []string{"mr", "resolve"},
	24	Summary: "resolve a review thread: mr resolve <owner/name> <n> <thread-id>", Run: runMRResolve})
	25	register(Command{Path: []string{"mr", "unresolve"},
	26	Summary: "reopen a review thread: mr unresolve <owner/name> <n> <thread-id>", Run: runMRUnresolve})
	27	}
	28
	29	func runDiffComment(c *Ctx, args []string) int {
	30	var rest []string
	31	var path, message, file string
	32	var line, replyTo int64
	33	old := false
	34	for i := 0; i < len(args); i++ {
	35	switch args[i] {
	36	case "--path", "--line", "--reply", "--message", "--file":
	37	if i+1 >= len(args) {
	38	return c.fail(protocol.ExitUsage, "%s requires a value", args[i])
	39	}
	40	v := args[i+1]
	41	switch args[i] {
	42	case "--path":
	43	path = v
	44	case "--line":
	45	n, err := strconv.ParseInt(v, 10, 64)
	46	if err != nil \|\| n < 1 {
	47	return c.fail(protocol.ExitUsage, "--line must be a positive number")
	48	}
	49	line = n
	50	case "--reply":
	51	n, err := strconv.ParseInt(v, 10, 64)
	52	if err != nil \|\| n < 1 {
	53	return c.fail(protocol.ExitUsage, "--reply must be a thread id")
	54	}
	55	replyTo = n
	56	case "--message":
	57	message = v
	58	case "--file":
	59	file = v
	60	}
	61	i++
	62	case "--old":
	63	old = true
	64	default:
	65	rest = append(rest, args[i])
	66	}
	67	}
	68	repo, mr, code := mrRef(c, rest, policy.CanRead)
	69	if code >= 0 {
	70	return code
	71	}
	72	if replyTo == 0 && (path == "" \|\| line == 0) {
	73	return c.fail(protocol.ExitUsage, "a new thread needs --path and --line (or reply to one with --reply <id>)")
	74	}
	75	body, err := bodyFrom(c, message, file)
	76	if err != nil {
	77	return c.fail(protocol.ExitUsage, "%v", err)
	78	}
	79	if strings.TrimSpace(body) == "" {
	80	return c.fail(protocol.ExitUsage, "empty comment; use --message or --file -")
	81	}
	82
	83	side := "new"
	84	if old {
	85	side = "old"
	86	}
	87	if replyTo == 0 {
	88	// The path must actually be part of the MR's diff.
	89	dir := RepoDir(c.Cfg.Server.Root, repo.OwnerName, repo.Name)
	90	base := mr.MergedBase
	91	if base == "" {
	92	b, err := gitutil.MergeBase(dir, "refs/heads/"+mr.TargetRef, mrHeadRef(mr.Number))
	93	if err != nil {
	94	return c.fail(protocol.ExitFailure, "%v", err)
	95	}
	96	base = b
	97	}
	98	files, err := gitutil.DiffFiles(dir, base, mrHeadRef(mr.Number))
	99	if err != nil {
	100	return c.fail(protocol.ExitFailure, "%v", err)
	101	}
	102	if !slices.Contains(files, path) {
	103	return c.fail(protocol.ExitUsage, "%s is not part of this merge request's diff", path)
	104	}
	105	}
	106
	107	id, err := c.Store.AddDiffComment(mr.ID, c.User.ID, mr.HeadSHA, path, side, line, body, replyTo)
	108	if err != nil {
	109	if errors.Is(err, store.ErrNotFound) {
	110	return c.fail(protocol.ExitNotFound, "%v", err)
	111	}
	112	return c.fail(protocol.ExitUsage, "%v", err)
	113	}
	114	if parts, err := c.Store.MRParticipants(mr.ID); err == nil {
	115	notifyUsers(c, parts, mrSubject(repo, mr.Number, mr.Title),
	116	notifyBody(c, fmt.Sprintf("commented on %s:%d in !%d", path, line, mr.Number), body,
	117	fmt.Sprintf("%s/mrs/%d", repo.Path(), mr.Number)))
	118	}
	119	return c.emit(map[string]any{"id": id, "thread": firstNonZero(replyTo, id)}, func(w io.Writer) {
	120	if replyTo != 0 {
	121	fmt.Fprintf(w, "replied to thread %d on %s!%d\n", replyTo, repo.Path(), mr.Number)
	122	} else {
	123	fmt.Fprintf(w, "thread %d opened on %s:%d in %s!%d\n", id, path, line, repo.Path(), mr.Number)
	124	}
	125	})
	126	}
	127
	128	func firstNonZero(a, b int64) int64 {
	129	if a != 0 {
	130	return a
	131	}
	132	return b
	133	}
	134
	135	func runMRThreads(c *Ctx, args []string) int {
	136	repo, mr, code := mrRef(c, args, policy.CanRead)
	137	if code >= 0 {
	138	return code
	139	}
	140	if len(args) != 2 {
	141	return c.fail(protocol.ExitUsage, "usage: mr threads <owner/name> <n>")
	142	}
	143	comments, err := c.Store.ListDiffComments(mr.ID)
	144	if err != nil {
	145	return c.fail(protocol.ExitFailure, "%v", err)
	146	}
	147	type commentOut struct {
	148	ID int64 `json:"id"`
	149	Author string `json:"author"`
	150	Body string `json:"body"`
	151	CreatedAt string `json:"created_at"`
	152	}
	153	type threadOut struct {
	154	ID int64 `json:"id"`
	155	Path string `json:"path"`
	156	Side string `json:"side"`
	157	Line int64 `json:"line"`
	158	Stale bool `json:"stale"`
	159	Resolved string `json:"resolved_by,omitempty"`
	160	Comments []commentOut `json:"comments"`
	161	}
	162	byRoot := map[int64]*threadOut{}
	163	var order []int64
	164	for _, cm := range comments {
	165	if cm.ReplyTo == 0 {
	166	byRoot[cm.ID] = &threadOut{
	167	ID: cm.ID, Path: cm.Path, Side: cm.Side, Line: cm.Line,
	168	Stale: cm.HeadSHA != mr.HeadSHA, Resolved: cm.ResolvedBy,
	169	Comments: []commentOut{{cm.ID, cm.Author, cm.Body, cm.CreatedAt}},
	170	}
	171	order = append(order, cm.ID)
	172	} else if th, ok := byRoot[cm.ReplyTo]; ok {
	173	th.Comments = append(th.Comments, commentOut{cm.ID, cm.Author, cm.Body, cm.CreatedAt})
	174	}
	175	}
	176	var ds []threadOut
	177	for _, id := range order {
	178	ds = append(ds, *byRoot[id])
	179	}
	180	_ = repo
	181	return c.emit(ds, func(w io.Writer) {
	182	for _, th := range ds {
	183	marks := ""
	184	if th.Resolved != "" {
	185	marks += " [resolved by " + th.Resolved + "]"
	186	}
	187	if th.Stale {
	188	marks += " [stale]"
	189	}
	190	fmt.Fprintf(w, "thread %d %s:%d (%s)%s\n", th.ID, th.Path, th.Line, th.Side, marks)
	191	for _, cm := range th.Comments {
	192	fmt.Fprintf(w, " %s: %s\n", cm.Author, cm.Body)
	193	}
	194	}
	195	})
	196	}
	197
	198	func setThreadResolved(c *Ctx, args []string, resolved bool) int {
	199	if len(args) != 3 {
	200	return c.fail(protocol.ExitUsage, "usage: mr resolve\|unresolve <owner/name> <n> <thread-id>")
	201	}
	202	repo, mr, code := mrRef(c, args[:2], policy.CanRead)
	203	if code >= 0 {
	204	return code
	205	}
	206	threadID, err := strconv.ParseInt(args[2], 10, 64)
	207	if err != nil {
	208	return c.fail(protocol.ExitUsage, "bad thread id %q", args[2])
	209	}
	210	// Thread author, MR author, or anyone with write may resolve.
	211	author, err := c.Store.DiffCommentAuthor(mr.ID, threadID)
	212	if errors.Is(err, store.ErrNotFound) {
	213	return c.fail(protocol.ExitNotFound, "no thread %d on %s!%d", threadID, repo.Path(), mr.Number)
	214	}
	215	if err != nil {
	216	return c.fail(protocol.ExitFailure, "%v", err)
	217	}
	218	grant, err := c.Store.AccessRole(repo.ID, c.User.ID)
	219	if err != nil {
	220	return c.fail(protocol.ExitFailure, "%v", err)
	221	}
	222	if author != c.User.ID && mr.Author != c.User.Username && !policy.CanWrite(c.User, repo, grant) {
	223	return c.fail(protocol.ExitDenied, "only the thread author, the MR author, or users with write access can resolve threads")
	224	}
	225	if err := c.Store.SetThreadResolved(mr.ID, threadID, c.User.ID, resolved); err != nil {
	226	if errors.Is(err, store.ErrNotFound) {
	227	return c.fail(protocol.ExitNotFound, "no thread %d (replies cannot be resolved; use the root id)", threadID)
	228	}
	229	return c.fail(protocol.ExitFailure, "%v", err)
	230	}
	231	verb := "resolved"
	232	if !resolved {
	233	verb = "reopened"
	234	}
	235	return c.emit(map[string]any{"thread": threadID, "resolved": resolved}, func(w io.Writer) {
	236	fmt.Fprintf(w, "%s thread %d on %s!%d\n", verb, threadID, repo.Path(), mr.Number)
	237	})
	238	}
	239
	240	func runMRResolve(c *Ctx, args []string) int { return setThreadResolved(c, args, true) }
	241	func runMRUnresolve(c *Ctx, args []string) int { return setThreadResolved(c, args, false) }

internal/control/mr.go +13 −5

		@@ -332,6 +332,10 @@ func runMRShow(c *Ctx, args []string) int {
332	332	if err != nil {
333	333	return c.fail(protocol.ExitFailure, "%v", err)
334	334	}
	335	unresolved, err := c.Store.UnresolvedThreadCount(mr.ID)
	336	if err != nil {
	337	return c.fail(protocol.ExitFailure, "%v", err)
	338	}
335	339	type commentOut struct {
336	340	Author string `json:"author"`
337	341	Body string `json:"body"`
		@@ -361,11 +365,12 @@ func runMRShow(c *Ctx, args []string) int {
361	365	}
362	366	d := struct {
363	367	mrOut
364		Checks []checkOut `json:"checks,omitempty"`
365		Combined string `json:"checks_combined,omitempty"`
366		Comments []commentOut `json:"comments,omitempty"`
367		Reviews []reviewOut `json:"reviews,omitempty"`
368		}{mrToOut(repo, mr, true), checks, store.CombinedStatus(statuses), cs, rs}
	368	Checks []checkOut `json:"checks,omitempty"`
	369	Combined string `json:"checks_combined,omitempty"`
	370	UnresolvedThreads int `json:"unresolved_threads,omitempty"`
	371	Comments []commentOut `json:"comments,omitempty"`
	372	Reviews []reviewOut `json:"reviews,omitempty"`
	373	}{mrToOut(repo, mr, true), checks, store.CombinedStatus(statuses), unresolved, cs, rs}
369	374	return c.emit(d, func(w io.Writer) {
370	375	fmt.Fprintf(w, "!%d %s [%s] by %s\n%s -> %s @ %.10s\n", d.Number, d.Title, d.State, d.Author, d.Source, d.TargetRef, d.HeadSHA)
371	376	if d.Body != "" {
		@@ -374,6 +379,9 @@ func runMRShow(c *Ctx, args []string) int {
374	379	for _, x := range checks {
375	380	fmt.Fprintf(w, "check: %s %s\n", x.Context, x.State)
376	381	}
	382	if d.UnresolvedThreads > 0 {
	383	fmt.Fprintf(w, "unresolved threads: %d\n", d.UnresolvedThreads)
	384	}
377	385	for _, r := range rs {
378	386	stale := ""
379	387	if r.Stale {

internal/gitutil/merge.go +15

		@@ -255,3 +255,18 @@ func ResolveTree(dir, sha string) (string, error) {
255	255	}
256	256	return strings.TrimSpace(string(out)), nil
257	257	}
	258
	259	// DiffFiles lists the paths changed between old and new.
	260	func DiffFiles(dir, old, new string) ([]string, error) {
	261	out, err := exec.Command("git", "-C", dir, "diff", "--name-only", old, new).Output()
	262	if err != nil {
	263	return nil, fmt.Errorf("diff --name-only: %w", err)
	264	}
	265	var files []string
	266	for _, l := range strings.Split(strings.TrimSpace(string(out)), "\n") {
	267	if l != "" {
	268	files = append(files, l)
	269	}
	270	}
	271	return files, nil
	272	}

internal/httpd/web.go +100 −17

		@@ -8,6 +8,7 @@ import (
8	8	"html/template"
9	9	"net/http"
10	10	"path"
	11	"regexp"
11	12	"strconv"
12	13	"strings"
13	14	"time"
		@@ -424,29 +425,107 @@ func renderReadme(name string, raw []byte) template.HTML {
424	425	}
425	426
426	427	type diffLine struct {
427		Class string
428		Text string
	428	Class string
	429	Text string
	430	Path string // file this line belongs to
	431	NewLine int64 // line number in the new file (0 when absent)
	432	OldLine int64 // line number in the old file (0 when absent)
	433	Threads []diffThread
429	434	}
430	435
	436	var hunkPat = regexp.MustCompile(`^@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@`)
	437
	438	// classifyDiff parses a unified diff into rendered lines, tracking the
	439	// file and old/new line numbers so review threads can anchor inline.
431	440	func classifyDiff(patch string) []diffLine {
432	441	var lines []diffLine
	442	path := ""
	443	var oldN, newN int64
433	444	for _, l := range strings.Split(patch, "\n") {
434		class := ""
	445	d := diffLine{Text: l}
435	446	switch {
436		case strings.HasPrefix(l, "+++"), strings.HasPrefix(l, "---"), strings.HasPrefix(l, "diff "), strings.HasPrefix(l, "index "):
437		class = "meta"
	447	case strings.HasPrefix(l, "+++ "):
	448	d.Class = "meta"
	449	path = strings.TrimPrefix(strings.TrimPrefix(l, "+++ "), "b/")
	450	case strings.HasPrefix(l, "--- "), strings.HasPrefix(l, "diff "), strings.HasPrefix(l, "index "):
	451	d.Class = "meta"
438	452	case strings.HasPrefix(l, "@@"):
439		class = "hunk"
	453	d.Class = "hunk"
	454	if m := hunkPat.FindStringSubmatch(l); m != nil {
	455	oldN, _ = strconv.ParseInt(m[1], 10, 64)
	456	newN, _ = strconv.ParseInt(m[2], 10, 64)
	457	}
440	458	case strings.HasPrefix(l, "+"):
441		class = "add"
	459	d.Class, d.Path, d.NewLine = "add", path, newN
	460	newN++
442	461	case strings.HasPrefix(l, "-"):
443		class = "del"
	462	d.Class, d.Path, d.OldLine = "del", path, oldN
	463	oldN++
	464	default:
	465	d.Path, d.OldLine, d.NewLine = path, oldN, newN
	466	oldN++
	467	newN++
444	468	}
445		lines = append(lines, diffLine{class, l})
	469	lines = append(lines, d)
446	470	}
447	471	return lines
448	472	}
449	473
	474	type diffThread struct {
	475	ID int64
	476	Resolved string
	477	Stale bool
	478	Comments []renderedComment
	479	}
	480
	481	// attachThreads injects review threads under their anchored diff lines;
	482	// threads whose anchor no longer appears (stale after force-push, or on a
	483	// context line outside the current diff) are returned separately.
	484	func attachThreads(lines []diffLine, comments []store.DiffComment, headSHA string) ([]diffLine, []diffThread) {
	485	type anchor struct {
	486	path string
	487	side string
	488	line int64
	489	}
	490	threads := map[int64]*diffThread{}
	491	anchors := map[int64]anchor{}
	492	var order []int64
	493	for _, cm := range comments {
	494	if cm.ReplyTo == 0 {
	495	threads[cm.ID] = &diffThread{ID: cm.ID, Resolved: cm.ResolvedBy, Stale: cm.HeadSHA != headSHA,
	496	Comments: []renderedComment{{cm.Author, cm.CreatedAt, mdHTML(cm.Body)}}}
	497	anchors[cm.ID] = anchor{cm.Path, cm.Side, cm.Line}
	498	order = append(order, cm.ID)
	499	} else if th, ok := threads[cm.ReplyTo]; ok {
	500	th.Comments = append(th.Comments, renderedComment{cm.Author, cm.CreatedAt, mdHTML(cm.Body)})
	501	}
	502	}
	503	placed := map[int64]bool{}
	504	for i := range lines {
	505	for _, id := range order {
	506	if placed[id] \|\| threads[id].Stale {
	507	continue
	508	}
	509	a := anchors[id]
	510	if lines[i].Path != a.path {
	511	continue
	512	}
	513	if (a.side == "new" && lines[i].NewLine == a.line && lines[i].Class != "del") \|\|
	514	(a.side == "old" && lines[i].OldLine == a.line && lines[i].Class == "del") {
	515	lines[i].Threads = append(lines[i].Threads, *threads[id])
	516	placed[id] = true
	517	}
	518	}
	519	}
	520	var unplaced []diffThread
	521	for _, id := range order {
	522	if !placed[id] {
	523	unplaced = append(unplaced, *threads[id])
	524	}
	525	}
	526	return lines, unplaced
	527	}
	528
450	529	type sigView struct {
451	530	State string
452	531	Signer string
		@@ -644,6 +723,7 @@ func (s Server) mr(w http.ResponseWriter, r http.Request) {
644	723	comments, _ := s.st.ListMRComments(m.ID)
645	724	reviews, _ := s.st.ListMRReviews(m.ID)
646	725	checks, _ := s.st.ListCommitStatuses(p.Repo.ID, m.HeadSHA)
	726	diffComments, _ := s.st.ListDiffComments(m.ID)
647	727
648	728	headRef := fmt.Sprintf("refs/merge-requests/%d/head", m.Number)
649	729	var lines []diffLine
		@@ -658,16 +738,19 @@ func (s Server) mr(w http.ResponseWriter, r http.Request) {
658	738	lines = classifyDiff(patch)
659	739	}
660	740	}
	741	var detachedThreads []diffThread
	742	lines, detachedThreads = attachThreads(lines, diffComments, m.HeadSHA)
661	743	s.render(w, "mr.html", struct {
662	744	repoPage
663		MR store.MR
664		BodyHTML template.HTML
665		Checks []store.CommitStatus
666		Combined string
667		Comments []renderedComment
668		Reviews []store.MRReview
669		DiffLines []diffLine
670		}{p, m, mdHTML(m.Body), checks, store.CombinedStatus(checks), renderComments(comments), reviews, lines})
	745	MR store.MR
	746	BodyHTML template.HTML
	747	Checks []store.CommitStatus
	748	Combined string
	749	Comments []renderedComment
	750	Reviews []store.MRReview
	751	DiffLines []diffLine
	752	DetachedThreads []diffThread
	753	}{p, m, mdHTML(m.Body), checks, store.CombinedStatus(checks), renderComments(comments), reviews, lines, detachedThreads})
671	754	}
672	755
673	756	func (s Server) refs(w http.ResponseWriter, r http.Request) {

internal/store/diffcomments.go added +130

		@@ -0,0 +1,130 @@
	1	package store
	2
	3	import (
	4	"database/sql"
	5	"errors"
	6	"fmt"
	7	)
	8
	9	type DiffComment struct {
	10	ID int64
	11	Author string
	12	HeadSHA string
	13	Path string
	14	Side string
	15	Line int64
	16	Body string
	17	ReplyTo int64 // 0 for thread roots
	18	ResolvedBy string
	19	CreatedAt string
	20	}
	21
	22	// AddDiffComment creates a thread root (replyTo 0) or a reply. Replies
	23	// inherit the root's anchor and must belong to the same MR.
	24	func (s *Store) AddDiffComment(mrID, authorID int64, headSHA, path, side string, line int64, body string, replyTo int64) (int64, error) {
	25	if replyTo != 0 {
	26	var rootMR int64
	27	var rootReply sql.NullInt64
	28	err := s.DB.QueryRow(
	29	"SELECT mr_id, reply_to FROM mr_diff_comments WHERE id = ?", replyTo).Scan(&rootMR, &rootReply)
	30	if errors.Is(err, sql.ErrNoRows) {
	31	return 0, fmt.Errorf("no thread %d: %w", replyTo, ErrNotFound)
	32	}
	33	if err != nil {
	34	return 0, err
	35	}
	36	if rootMR != mrID {
	37	return 0, fmt.Errorf("thread %d belongs to a different merge request", replyTo)
	38	}
	39	if rootReply.Valid {
	40	return 0, fmt.Errorf("reply to the thread root %d, not to a reply", rootReply.Int64)
	41	}
	42	err = s.DB.QueryRow(
	43	"SELECT head_sha, path, side, line FROM mr_diff_comments WHERE id = ?", replyTo).
	44	Scan(&headSHA, &path, &side, &line)
	45	if err != nil {
	46	return 0, err
	47	}
	48	}
	49	var reply any
	50	if replyTo != 0 {
	51	reply = replyTo
	52	}
	53	res, err := s.DB.Exec(`
	54	INSERT INTO mr_diff_comments (mr_id, author_id, head_sha, path, side, line, body, reply_to)
	55	VALUES (?, ?, ?, ?, ?, ?, ?, ?)`,
	56	mrID, authorID, headSHA, path, side, line, body, reply)
	57	if err != nil {
	58	return 0, err
	59	}
	60	return res.LastInsertId()
	61	}
	62
	63	// ListDiffComments returns every diff comment on an MR, roots and replies,
	64	// oldest first.
	65	func (s *Store) ListDiffComments(mrID int64) ([]DiffComment, error) {
	66	rows, err := s.DB.Query(`
	67	SELECT c.id, u.username, c.head_sha, c.path, c.side, c.line, c.body,
	68	COALESCE(c.reply_to, 0), COALESCE(r.username, ''), c.created_at
	69	FROM mr_diff_comments c
	70	JOIN users u ON u.id = c.author_id
	71	LEFT JOIN users r ON r.id = c.resolved_by
	72	WHERE c.mr_id = ? ORDER BY c.id`, mrID)
	73	if err != nil {
	74	return nil, err
	75	}
	76	defer rows.Close()
	77	var out []DiffComment
	78	for rows.Next() {
	79	var c DiffComment
	80	if err := rows.Scan(&c.ID, &c.Author, &c.HeadSHA, &c.Path, &c.Side, &c.Line, &c.Body,
	81	&c.ReplyTo, &c.ResolvedBy, &c.CreatedAt); err != nil {
	82	return nil, err
	83	}
	84	out = append(out, c)
	85	}
	86	return out, rows.Err()
	87	}
	88
	89	// SetThreadResolved resolves or unresolves a thread root.
	90	func (s *Store) SetThreadResolved(mrID, rootID, byUser int64, resolved bool) error {
	91	var q string
	92	var args []any
	93	if resolved {
	94	q = `UPDATE mr_diff_comments SET resolved_at = strftime('%Y-%m-%dT%H:%M:%fZ','now'), resolved_by = ?
	95	WHERE id = ? AND mr_id = ? AND reply_to IS NULL`
	96	args = []any{byUser, rootID, mrID}
	97	} else {
	98	q = `UPDATE mr_diff_comments SET resolved_at = NULL, resolved_by = NULL
	99	WHERE id = ? AND mr_id = ? AND reply_to IS NULL`
	100	args = []any{rootID, mrID}
	101	}
	102	res, err := s.DB.Exec(q, args...)
	103	if err != nil {
	104	return err
	105	}
	106	if n, _ := res.RowsAffected(); n == 0 {
	107	return ErrNotFound
	108	}
	109	return nil
	110	}
	111
	112	// DiffCommentAuthor returns the author id of one comment.
	113	func (s *Store) DiffCommentAuthor(mrID, id int64) (int64, error) {
	114	var author int64
	115	err := s.DB.QueryRow(
	116	"SELECT author_id FROM mr_diff_comments WHERE id = ? AND mr_id = ?", id, mrID).Scan(&author)
	117	if errors.Is(err, sql.ErrNoRows) {
	118	return 0, ErrNotFound
	119	}
	120	return author, err
	121	}
	122
	123	// UnresolvedThreadCount counts unresolved thread roots on an MR.
	124	func (s *Store) UnresolvedThreadCount(mrID int64) (int, error) {
	125	var n int
	126	err := s.DB.QueryRow(
	127	"SELECT COUNT(*) FROM mr_diff_comments WHERE mr_id = ? AND reply_to IS NULL AND resolved_at IS NULL",
	128	mrID).Scan(&n)
	129	return n, err
	130	}

internal/store/migrations/0010_diff_comments.down.sql added +1

		@@ -0,0 +1 @@
	1	DROP TABLE mr_diff_comments;

internal/store/migrations/0010_diff_comments.up.sql added +15

		@@ -0,0 +1,15 @@
	1	CREATE TABLE mr_diff_comments (
	2	id INTEGER PRIMARY KEY,
	3	mr_id INTEGER NOT NULL REFERENCES merge_requests(id) ON DELETE CASCADE,
	4	author_id INTEGER NOT NULL REFERENCES users(id) ON DELETE CASCADE,
	5	head_sha TEXT NOT NULL,
	6	path TEXT NOT NULL,
	7	side TEXT NOT NULL DEFAULT 'new' CHECK (side IN ('new','old')),
	8	line INTEGER NOT NULL,
	9	body TEXT NOT NULL,
	10	reply_to INTEGER REFERENCES mr_diff_comments(id) ON DELETE CASCADE,
	11	resolved_at TEXT,
	12	resolved_by INTEGER REFERENCES users(id) ON DELETE SET NULL,
	13	created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ','now'))
	14	);
	15	CREATE INDEX mr_diff_comments_mr ON mr_diff_comments(mr_id);

internal/store/notify.go +2 −1

		@@ -71,7 +71,8 @@ func (s *Store) MRParticipants(mrID int64) ([]int64, error) {
71	71	return s.idQuery(`
72	72	SELECT author_id FROM merge_requests WHERE id = ?
73	73	UNION SELECT author_id FROM mr_comments WHERE mr_id = ?
74		UNION SELECT reviewer_id FROM mr_reviews WHERE mr_id = ?`, mrID, mrID, mrID)
	74	UNION SELECT reviewer_id FROM mr_reviews WHERE mr_id = ?
	75	UNION SELECT author_id FROM mr_diff_comments WHERE mr_id = ?`, mrID, mrID, mrID, mrID)
75	76	}
76	77
77	78	// RepoNotifyTargets returns who should hear about new activity on a repo:

internal/web/static/style.css +3

		@@ -60,3 +60,6 @@ pre.diff .meta { color: var(--muted); }
60	60	.check-success { color: var(--ok); border-color: var(--ok); }
61	61	.check-pending { color: var(--warn); border-color: var(--warn); }
62	62	.check-failure, .check-error { color: var(--bad); border-color: var(--bad); }
	63	.thread { border: 1px solid var(--line); border-left: 3px solid var(--link); border-radius: 6px; padding: 0.5rem 0.8rem; margin: 0.3rem 0 0.3rem 2rem; }
	64	.thread.resolved { border-left-color: var(--ok); opacity: 0.75; }
	65	.thread.stale { border-left-color: var(--warn); }

internal/web/templates/mr.html +3 −1

		@@ -19,5 +19,7 @@
19	19	{{end}}
20	20	<h3>diff</h3>
21	21	<pre class="diff">{{range .DiffLines}}<span class="{{.Class}}">{{.Text}}</span>
22		{{end}}</pre>
	22	{{range .Threads}}</pre><div class="thread{{if .Resolved}} resolved{{end}}">{{if .Resolved}}<p class="crumbs">resolved by {{.Resolved}}</p>{{end}}{{range .Comments}}<p class="crumbs">{{.Author}} at {{.CreatedAt}}</p><div class="rendered">{{.BodyHTML}}</div>{{end}}</div><pre class="diff">{{end}}{{end}}</pre>
	23	{{if .DetachedThreads}}<h3>threads on earlier revisions</h3>
	24	{{range .DetachedThreads}}<div class="thread stale"><p class="crumbs">{{if .Stale}}stale · {{end}}{{if .Resolved}}resolved by {{.Resolved}}{{end}}</p>{{range .Comments}}<p class="crumbs">{{.Author}} at {{.CreatedAt}}</p><div class="rendered">{{.BodyHTML}}</div>{{end}}</div>{{end}}{{end}}
23	25	{{end}}

Commit dd665998be

Commit `dd665998be`