krz/gitbay

store, control, autolink, wiki: an @mention files an inbox row and joins the thread !344

merged merged by cmc on 2026-09-08 02:35 UTC · krz/gitbay:mentions-notify into main

15 files changed, +224 −16

Layout: unified · split

.gitbay/wiki/Users.org +6 −1
@@ -517,7 +517,12 @@ handled.
517 517
518When the instance has SMTP configured, activity mails you: someone 518When the instance has SMTP configured, activity mails you: someone
519opens an issue or MR on your repository, comments where you are a 519opens an issue or MR on your repository, comments where you are a
520participant (author, commenter, reviewer), reviews, closes, or merges. 520participant (author, commenter, reviewer, or mentioned), reviews,
521closes, or merges. Writing =@name= in an issue, merge request or
522comment files "mentioned you" in that account's inbox and makes them a
523participant of the thread, provided they can read the repository and
524have not muted it; watchers are not told about a mention addressed to
525someone else.
521You are never mailed about your own actions, and only verified primary 526You are never mailed about your own actions, and only verified primary
522addresses receive anything. Delivery retries on relay failure. 527addresses receive anything. Delivery retries on relay failure.
523 528
e2e/mentions_test.go added +92
@@ -0,0 +1,92 @@
1package e2e
2
3import (
4 "strings"
5 "testing"
6)
7
8// An @mention files an inbox row for the mentioned account and makes them
9// a participant of the thread; watchers are not told about the mention,
10// mute holds, and someone who cannot read the repository is not reached
11// (#202).
12func TestMentionsNotify(t *testing.T) {
13 inst := startInstance(t)
14 keys := map[string]string{}
15 for _, u := range []string{"alice", "bob", "carol", "eve"} {
16 keys[u] = inst.newKey(t, u)
17 inst.admin(t, "admin", "user", "create", u, "--key", keys[u]+".pub")
18 }
19 if _, errOut, code := inst.ssh(t, keys["alice"], "", "repo", "create", "alice/app"); code != 0 {
20 t.Fatalf("repo create: %s", errOut)
21 }
22 if _, _, code := inst.ssh(t, keys["carol"], "", "repo", "watch", "alice/app"); code != 0 {
23 t.Fatal("watch failed")
24 }
25 summaries := func(key string) []string {
26 var out []string
27 for _, n := range notices(t, inst, key) {
28 out = append(out, n.Summary)
29 }
30 return out
31 }
32 has := func(list []string, want string) bool {
33 for _, s := range list {
34 if strings.Contains(s, want) {
35 return true
36 }
37 }
38 return false
39 }
40
41 // A mention in an issue body, with prose punctuation after the name.
42 if _, errOut, code := inst.ssh(t, keys["alice"], "", "issue", "create", "alice/app",
43 "--title", "'leak'", "--body", "'@bob, can you look? cc @nobody and @alice.'"); code != 0 {
44 t.Fatalf("issue create: %s", errOut)
45 }
46 if got := summaries(keys["bob"]); !has(got, "mentioned you in #1") {
47 t.Fatalf("bob not told about the mention: %v", got)
48 }
49 if got := summaries(keys["carol"]); has(got, "mentioned") || !has(got, "opened issue #1") {
50 t.Fatalf("watcher inbox: %v", got)
51 }
52 if got := summaries(keys["alice"]); has(got, "mentioned") {
53 t.Fatalf("self-mention filed: %v", got)
54 }
55
56 // bob is now a participant: a later comment reaches him.
57 if _, errOut, code := inst.ssh(t, keys["alice"], "", "issue", "comment", "alice/app", "1", "--message", "'more'"); code != 0 {
58 t.Fatalf("comment: %s", errOut)
59 }
60 if got := summaries(keys["bob"]); !has(got, "commented on #1") {
61 t.Fatalf("mentioned account not a participant: %v", got)
62 }
63
64 // Muted, a mention does not get through.
65 if _, _, code := inst.ssh(t, keys["bob"], "", "repo", "mute", "alice/app"); code != 0 {
66 t.Fatal("mute failed")
67 }
68 if _, errOut, code := inst.ssh(t, keys["alice"], "", "issue", "comment", "alice/app", "1", "--message", "'@bob again'"); code != 0 {
69 t.Fatalf("comment: %s", errOut)
70 }
71 if got := summaries(keys["bob"]); has(got, "mentioned you in #1") && len(got) > 2 {
72 t.Fatalf("mention reached a muted account: %v", got)
73 }
74
75 // A private repository: a mention of someone who cannot read it is
76 // dropped, one of someone who can is filed. Merge request bodies too.
77 for _, args := range [][]string{
78 {"repo", "create", "alice/secret", "--private"},
79 {"repo", "access", "grant", "alice/secret", "carol", "read"},
80 {"issue", "create", "alice/secret", "--title", "'hush'", "--body", "'@eve @carol'"},
81 } {
82 if _, errOut, code := inst.ssh(t, keys["alice"], "", args...); code != 0 {
83 t.Fatalf("%v: %s", args, errOut)
84 }
85 }
86 if got := summaries(keys["eve"]); len(got) != 0 {
87 t.Fatalf("outsider reached through a private repository: %v", got)
88 }
89 if got := summaries(keys["carol"]); !has(got, "mentioned you in #1") {
90 t.Fatalf("reader of a private repository not told: %v", got)
91 }
92}
internal/autolink/autolink.go +16
@@ -84,6 +84,22 @@ type span struct {
84 84
85// rewriteText returns replacement nodes for a text node, or nil when no 85// rewriteText returns replacement nodes for a text node, or nil when no
86// reference resolved. 86// reference resolved.
87// Mentions returns the distinct @names in text, in order of appearance,
88// as written. A name may carry trailing punctuation the writer meant as
89// prose ("@alice."); the caller resolves and, failing that, trims ._-
90// the way Rewrite does.
91func Mentions(text string) []string {
92 seen := map[string]bool{}
93 var out []string
94 for _, m := range mentionPat.FindAllStringSubmatch(text, -1) {
95 if who := m[2]; !seen[who] {
96 seen[who] = true
97 out = append(out, who)
98 }
99 }
100 return out
101}
102
87func rewriteText(text, owner, name string, r Resolver) []*html.Node { 103func rewriteText(text, owner, name string, r Resolver) []*html.Node {
88 var spans []span 104 var spans []span
89 105
internal/autolink/autolink_test.go +13
@@ -100,3 +100,16 @@ func TestRewriteEscaping(t *testing.T) {
100 t.Fatalf("ref not linked:\n%s", got) 100 t.Fatalf("ref not linked:\n%s", got)
101 } 101 }
102} 102}
103
104func TestMentions(t *testing.T) {
105 got := Mentions("cc @alice and (@bob) — @alice again; mail@example.org is not one, @carol.")
106 want := []string{"alice", "bob", "carol."}
107 if len(got) != len(want) {
108 t.Fatalf("Mentions = %v, want %v", got, want)
109 }
110 for i := range want {
111 if got[i] != want[i] {
112 t.Fatalf("Mentions = %v, want %v", got, want)
113 }
114 }
115}
internal/control/diffcomment.go +1
@@ -113,6 +113,7 @@ func runDiffComment(c *Ctx, args []string) int {
113 action: fmt.Sprintf("commented on %s:%d in !%d", path, line, mr.Number), 113 action: fmt.Sprintf("commented on %s:%d in !%d", path, line, mr.Number),
114 excerpt: body, path: fmt.Sprintf("%s/mrs/%d", repo.Path(), mr.Number)}) 114 excerpt: body, path: fmt.Sprintf("%s/mrs/%d", repo.Path(), mr.Number)})
115 } 115 }
116 notifyMentions(c, repo, mrThread, mr.ID, mr.Number, mr.Title, body)
116 } 117 }
117 return c.emit(map[string]any{"id": id, "thread": firstNonZero(replyTo, id), "pending": pending}, func(w io.Writer) { 118 return c.emit(map[string]any{"id": id, "thread": firstNonZero(replyTo, id), "pending": pending}, func(w io.Writer) {
118 what := "thread %d opened on %s:%d in %s!%d\n" 119 what := "thread %d opened on %s:%d in %s!%d\n"
internal/control/issue.go +3
@@ -155,6 +155,9 @@ func runIssueCreate(c *Ctx, args []string) int {
155 action: fmt.Sprintf("opened issue #%d", n), 155 action: fmt.Sprintf("opened issue #%d", n),
156 excerpt: b, path: fmt.Sprintf("%s/issues/%d", repo.Path(), n)}) 156 excerpt: b, path: fmt.Sprintf("%s/issues/%d", repo.Path(), n)})
157 } 157 }
158 if issue, err := c.Store.IssueByNumber(repo.ID, n); err == nil {
159 notifyMentions(c, repo, issueThread, issue.ID, n, title, b)
160 }
158 return c.emit(Created{Number: n}, func(w io.Writer) { 161 return c.emit(Created{Number: n}, func(w io.Writer) {
159 fmt.Fprintf(w, "created %s#%d\n", repo.Path(), n) 162 fmt.Fprintf(w, "created %s#%d\n", repo.Path(), n)
160 }) 163 })
internal/control/mr.go +3
@@ -344,6 +344,9 @@ func runMRCreate(c *Ctx, args []string) int {
344 action: fmt.Sprintf("opened merge request !%d (%s -> %s)", n, source, target), 344 action: fmt.Sprintf("opened merge request !%d (%s -> %s)", n, source, target),
345 excerpt: b, path: fmt.Sprintf("%s/mrs/%d", repo.Path(), n)}) 345 excerpt: b, path: fmt.Sprintf("%s/mrs/%d", repo.Path(), n)})
346 } 346 }
347 if created, err := c.Store.MRByNumber(repo.ID, n); err == nil {
348 notifyMentions(c, repo, mrThread, created.ID, n, title, b)
349 }
347 out := MRCreated{Number: n, HeadSHA: headSHA} 350 out := MRCreated{Number: n, HeadSHA: headSHA}
348 if p, ok, err := c.Store.OpenMRBySource(repo.ID, target); err == nil && ok { 351 if p, ok, err := c.Store.OpenMRBySource(repo.ID, target); err == nil && ok {
349 out.StackedOn = &stackRef{p.Number, p.Title} 352 out.StackedOn = &stackRef{p.Number, p.Title}
internal/control/notifications.go +44 −4
@@ -6,6 +6,7 @@ import (
6 "strconv" 6 "strconv"
7 "strings" 7 "strings"
8 8
9 "gitbay.org/gitbay/internal/autolink"
9 "gitbay.org/gitbay/internal/policy" 10 "gitbay.org/gitbay/internal/policy"
10 "gitbay.org/gitbay/internal/protocol" 11 "gitbay.org/gitbay/internal/protocol"
11 "gitbay.org/gitbay/internal/store" 12 "gitbay.org/gitbay/internal/store"
@@ -44,14 +45,17 @@ type notice struct {
44 // mail is not prose — a failed build's log tail is not an excerpt of 45 // mail is not prose — a failed build's log tail is not an excerpt of
45 // something someone wrote, and is not cut to an excerpt's length. 46 // something someone wrote, and is not cut to an excerpt's length.
46 body string 47 body string
48 // direct keeps the notice to the given accounts: watchers of the
49 // repository are not added. A mention is addressed to someone.
50 direct bool
47} 51}
48 52
49// notify delivers a notice to the given user ids widened by the 53// notify delivers a notice to the given user ids widened by the
50// repository's watchers, minus anyone who muted it and minus the acting 54// repository's watchers (unless direct), minus anyone who muted it and
51// user. A best-effort side channel: failures are ignored, the action 55// minus the acting user. A best-effort side channel: failures are
52// itself already succeeded. 56// ignored, the action itself already succeeded.
53func notify(c *Ctx, userIDs []int64, n notice) { 57func notify(c *Ctx, userIDs []int64, n notice) {
54 recipients, err := c.Store.NotifyRecipients(n.repo.ID, c.User.ID, userIDs) 58 recipients, err := c.Store.NotifyRecipients(n.repo.ID, c.User.ID, userIDs, !n.direct)
55 if err != nil { 59 if err != nil {
56 return 60 return
57 } 61 }
@@ -70,6 +74,42 @@ func notify(c *Ctx, userIDs []int64, n notice) {
70 } 74 }
71} 75}
72 76
77// notifyMentions files an inbox row for every account text mentions by
78// @name that can read the repository, and records them as participants
79// of the thread so they hear what follows (#202). Mute is honoured by
80// notify; the actor mentioning themselves is dropped there too.
81func notifyMentions(c *Ctx, repo store.Repo, t thread, itemID, number int64, title, text string) {
82 var ids []int64
83 for _, name := range autolink.Mentions(text) {
84 u, err := c.Store.UserByUsername(name)
85 if err != nil {
86 trimmed := strings.TrimRight(name, "._-")
87 if trimmed == "" || trimmed == name {
88 continue
89 }
90 if u, err = c.Store.UserByUsername(trimmed); err != nil {
91 continue
92 }
93 }
94 if u.ID == c.User.ID {
95 continue
96 }
97 grant, err := c.Store.AccessRole(repo.ID, u.ID)
98 if err != nil || !policy.CanRead(u, repo, grant) {
99 continue
100 }
101 ids = append(ids, u.ID)
102 }
103 if len(ids) == 0 {
104 return
105 }
106 c.Store.AddMentions(repo.ID, t.kind, itemID, ids)
107 notify(c, ids, notice{repo: repo, kind: t.kind, direct: true,
108 subject: fmt.Sprintf("[%s] %s%d: %s", repo.Path(), t.symbol, number, title),
109 action: fmt.Sprintf("mentioned you in %s%d", t.symbol, number),
110 excerpt: text, path: fmt.Sprintf("%s/%s/%d", repo.Path(), t.segment, number)})
111}
112
73// noticeBody builds the standard mail body: who did what, an excerpt, and 113// noticeBody builds the standard mail body: who did what, an excerpt, and
74// the web link. 114// the web link.
75func noticeBody(c *Ctx, n notice) string { 115func noticeBody(c *Ctx, n notice) string {
internal/control/thread.go +1
@@ -112,6 +112,7 @@ func runComment(c *Ctx, args []string, t thread, noun string,
112 action: fmt.Sprintf("commented on %s%d", t.symbol, number), 112 action: fmt.Sprintf("commented on %s%d", t.symbol, number),
113 excerpt: body, path: fmt.Sprintf("%s/%s/%d", repo.Path(), t.segment, number)}) 113 excerpt: body, path: fmt.Sprintf("%s/%s/%d", repo.Path(), t.segment, number)})
114 } 114 }
115 notifyMentions(c, repo, t, id, number, title, body)
115 return c.emit(Created{Number: number}, func(w io.Writer) { 116 return c.emit(Created{Number: number}, func(w io.Writer) {
116 fmt.Fprintf(w, "commented on %s%s%d\n", repo.Path(), t.symbol, number) 117 fmt.Fprintf(w, "commented on %s%s%d\n", repo.Path(), t.symbol, number)
117 }) 118 })
internal/deps/worker.go +1 −1
@@ -238,7 +238,7 @@ func (w *Worker) notify(repo store.Repo, number int64, action, body string) {
238 if err != nil { 238 if err != nil {
239 return 239 return
240 } 240 }
241 recipients, err := w.St.NotifyRecipients(repo.ID, author.ID, targets) 241 recipients, err := w.St.NotifyRecipients(repo.ID, author.ID, targets, true)
242 if err != nil { 242 if err != nil {
243 return 243 return
244 } 244 }
internal/store/inbox.go +4 −1
@@ -131,7 +131,7 @@ func (s *Store) RepoWatchState(repoID, userID int64) string {
131// the repository's watchers, minus the actor and minus anyone who muted 131// the repository's watchers, minus the actor and minus anyone who muted
132// it. Muting wins over every other reason to be told, including owning 132// it. Muting wins over every other reason to be told, including owning
133// the repository or having written the thread. 133// the repository or having written the thread.
134func (s *Store) NotifyRecipients(repoID, actorID int64, targets []int64) ([]int64, error) { 134func (s *Store) NotifyRecipients(repoID, actorID int64, targets []int64, widen bool) ([]int64, error) {
135 rows, err := s.DB.Query("SELECT user_id, state FROM repo_watchers WHERE repo_id = ?", repoID) 135 rows, err := s.DB.Query("SELECT user_id, state FROM repo_watchers WHERE repo_id = ?", repoID)
136 if err != nil { 136 if err != nil {
137 return nil, err 137 return nil, err
@@ -156,6 +156,9 @@ func (s *Store) NotifyRecipients(repoID, actorID int64, targets []int64) ([]int6
156 } 156 }
157 var out []int64 157 var out []int64
158 seen := map[int64]bool{} 158 seen := map[int64]bool{}
159 if !widen {
160 watching = nil
161 }
159 for _, id := range append(append([]int64{}, targets...), watching...) { 162 for _, id := range append(append([]int64{}, targets...), watching...) {
160 if skip[id] || seen[id] { 163 if skip[id] || seen[id] {
161 continue 164 continue
internal/store/inbox_test.go +9 −5
@@ -115,7 +115,7 @@ func TestNotifyRecipients(t *testing.T) {
115 t.Fatal(err) 115 t.Fatal(err)
116 } 116 }
117 117
118 got, err := s.NotifyRecipients(repoID, other, []int64{owner, other}) 118 got, err := s.NotifyRecipients(repoID, other, []int64{owner, other}, true)
119 if err != nil { 119 if err != nil {
120 t.Fatal(err) 120 t.Fatal(err)
121 } 121 }
@@ -126,20 +126,24 @@ func TestNotifyRecipients(t *testing.T) {
126 if err := s.SetRepoWatch(repoID, third, "watching"); err != nil { 126 if err := s.SetRepoWatch(repoID, third, "watching"); err != nil {
127 t.Fatal(err) 127 t.Fatal(err)
128 } 128 }
129 if got, _ := s.NotifyRecipients(repoID, other, []int64{owner}); len(got) != 2 { 129 if got, _ := s.NotifyRecipients(repoID, other, []int64{owner}, true); len(got) != 2 {
130 t.Fatalf("watcher not added: %v", got) 130 t.Fatalf("watcher not added: %v", got)
131 } 131 }
132 132
133 // A watcher who is also a target is listed once. 133 // A watcher who is also a target is listed once.
134 if got, _ := s.NotifyRecipients(repoID, other, []int64{owner, third}); len(got) != 2 { 134 if got, _ := s.NotifyRecipients(repoID, other, []int64{owner, third}, true); len(got) != 2 {
135 t.Fatalf("watcher duplicated: %v", got) 135 t.Fatalf("watcher duplicated: %v", got)
136 } 136 }
137 // A direct notice stays with its targets; watchers are not added.
138 if got, _ := s.NotifyRecipients(repoID, other, []int64{owner}, false); len(got) != 1 || got[0] != owner {
139 t.Fatalf("direct notice widened: %v", got)
140 }
137 141
138 // Muting beats owning the repository. 142 // Muting beats owning the repository.
139 if err := s.SetRepoWatch(repoID, owner, "muted"); err != nil { 143 if err := s.SetRepoWatch(repoID, owner, "muted"); err != nil {
140 t.Fatal(err) 144 t.Fatal(err)
141 } 145 }
142 got, _ = s.NotifyRecipients(repoID, other, []int64{owner}) 146 got, _ = s.NotifyRecipients(repoID, other, []int64{owner}, true)
143 if len(got) != 1 || got[0] != third { 147 if len(got) != 1 || got[0] != third {
144 t.Fatalf("muted owner still notified: %v", got) 148 t.Fatalf("muted owner still notified: %v", got)
145 } 149 }
@@ -153,7 +157,7 @@ func TestNotifyRecipients(t *testing.T) {
153 if s.RepoWatchState(repoID, owner) != "" { 157 if s.RepoWatchState(repoID, owner) != "" {
154 t.Fatal("clear left a state") 158 t.Fatal("clear left a state")
155 } 159 }
156 if got, _ := s.NotifyRecipients(repoID, other, []int64{owner}); len(got) != 2 { 160 if got, _ := s.NotifyRecipients(repoID, other, []int64{owner}, true); len(got) != 2 {
157 t.Fatalf("cleared owner not notified: %v", got) 161 t.Fatalf("cleared owner not notified: %v", got)
158 } 162 }
159 if err := s.SetRepoWatch(repoID, owner, "muted"); err != nil { 163 if err := s.SetRepoWatch(repoID, owner, "muted"); err != nil {
internal/store/migrations/0047_mentions.down.sql added +1
@@ -0,0 +1 @@
1DROP TABLE mentions;
internal/store/migrations/0047_mentions.up.sql added +11
@@ -0,0 +1,11 @@
1-- Who an issue or merge request body, or a comment on one, mentioned by
2-- @name. A mentioned account is a participant of the thread from then
3-- on, the way an author or commenter is (#202). repo_id carries the
4-- cascade, since item_id is polymorphic.
5CREATE TABLE mentions (
6 repo_id INTEGER NOT NULL REFERENCES repos(id) ON DELETE CASCADE,
7 kind TEXT NOT NULL CHECK (kind IN ('issue', 'mr')),
8 item_id INTEGER NOT NULL,
9 user_id INTEGER NOT NULL REFERENCES users(id) ON DELETE CASCADE,
10 PRIMARY KEY (kind, item_id, user_id)
11);
internal/store/notify.go +19 −4
@@ -58,21 +58,36 @@ func (s *Store) MarkMailFailed(id int64, errMsg string, nextAt *time.Time) error
58} 58}
59 59
60// IssueParticipants returns distinct user ids involved in an issue: the 60// IssueParticipants returns distinct user ids involved in an issue: the
61// author and every commenter. 61// author, every commenter, and everyone mentioned.
62func (s *Store) IssueParticipants(issueID int64) ([]int64, error) { 62func (s *Store) IssueParticipants(issueID int64) ([]int64, error) {
63 return s.idQuery(` 63 return s.idQuery(`
64 SELECT author_id FROM issues WHERE id = ? 64 SELECT author_id FROM issues WHERE id = ?
65 UNION SELECT author_id FROM issue_comments WHERE issue_id = ?`, issueID, issueID) 65 UNION SELECT author_id FROM issue_comments WHERE issue_id = ?
66 UNION SELECT user_id FROM mentions WHERE kind = 'issue' AND item_id = ?`, issueID, issueID, issueID)
66} 67}
67 68
68// MRParticipants returns distinct user ids involved in an MR: author, 69// MRParticipants returns distinct user ids involved in an MR: author,
69// commenters, reviewers. 70// commenters, reviewers, and everyone mentioned.
70func (s *Store) MRParticipants(mrID int64) ([]int64, error) { 71func (s *Store) MRParticipants(mrID int64) ([]int64, error) {
71 return s.idQuery(` 72 return s.idQuery(`
72 SELECT author_id FROM merge_requests WHERE id = ? 73 SELECT author_id FROM merge_requests WHERE id = ?
73 UNION SELECT author_id FROM mr_comments WHERE mr_id = ? 74 UNION SELECT author_id FROM mr_comments WHERE mr_id = ?
74 UNION SELECT reviewer_id FROM mr_reviews WHERE mr_id = ? 75 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) 76 UNION SELECT author_id FROM mr_diff_comments WHERE mr_id = ?
77 UNION SELECT user_id FROM mentions WHERE kind = 'mr' AND item_id = ?`, mrID, mrID, mrID, mrID, mrID)
78}
79
80// AddMentions records accounts mentioned in an issue or merge request
81// (kind "issue" or "mr"); a repeat mention is not an error.
82func (s *Store) AddMentions(repoID int64, kind string, itemID int64, userIDs []int64) error {
83 for _, id := range userIDs {
84 if _, err := s.DB.Exec(
85 "INSERT INTO mentions (repo_id, kind, item_id, user_id) VALUES (?, ?, ?, ?) ON CONFLICT DO NOTHING",
86 repoID, kind, itemID, id); err != nil {
87 return err
88 }
89 }
90 return nil
76} 91}
77 92
78// RepoNotifyTargets returns who should hear about new activity on a repo: 93// RepoNotifyTargets returns who should hear about new activity on a repo: