Commit ee971df409

ee971df4092ff50fdcfac65a3b80abd1cdec9edc

parent: 749296f8f3

Verified · cmc

cmc <hello@cleberg.net> · 2026-09-11 15:40 UTC

control: Closes owner/name#N acts on a repository the actor can write to

Ref #203
internal/control/commitrefs.go +79 −28
@@ -8,17 +8,26 @@ import (
88 "strings"
99
1010 "gitbay.org/gitbay/internal/gitutil"
11 "gitbay.org/gitbay/internal/policy"
1112 "gitbay.org/gitbay/internal/store"
1213)
1314
14// closePat matches closing keywords; refPat matches any same-repo issue
15// reference. Cross-repo references stay display-only (autolink) — acting
16// across repositories would need its own authorization story.
15// closePat matches closing keywords, with an optional owner/name before
16// the number for an issue in another repository; refPat matches any bare
17// same-repo reference. A cross-repo close acts only when the actor holds
18// write on the target (closeTarget); a bare cross-repo reference stays
19// display-only.
1720var (
18 closePat = regexp.MustCompile(`(?i)\b(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)[ :]+#(\d+)\b`)
21 closePat = regexp.MustCompile(`(?i)\b(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)[ :]+(?:([a-z0-9][a-z0-9._-]*/[a-z0-9][a-z0-9._-]*))?#(\d+)\b`)
1922 refPat = regexp.MustCompile(`(^|[\s([{:])#(\d+)\b`)
2023)
2124
25// closeRef is one closing reference: Path is "" for the same repository.
26type closeRef struct {
27 Path string
28 N int64
29}
30
2231const maxMessageCommits = 100
2332
2433// ProcessCommitMessages acts on issue references in commits that just
@@ -34,23 +43,30 @@ func ProcessCommitMessages(st *store.Store, dir string, repo store.Repo, actorID
3443 return
3544 }
3645 for _, m := range msgs {
37 closes := map[int64]bool{}
38 for _, n := range closingRefs(m.Message) {
39 closes[n] = true
46 closes := closingRefs(m.Message)
47 local := map[int64]bool{}
48 for _, ref := range closes {
49 if ref.Path == "" {
50 local[ref.N] = true
51 }
4052 }
4153 refs := map[int64]bool{}
4254 for _, g := range refPat.FindAllStringSubmatch(m.Message, -1) {
43 if n, err := strconv.ParseInt(g[2], 10, 64); err == nil && !closes[n] {
55 if n, err := strconv.ParseInt(g[2], 10, 64); err == nil && !local[n] {
4456 refs[n] = true
4557 }
4658 }
4759 subject, _, _ := strings.Cut(m.Message, "\n")
4860 author := authorLink(st, m.AuthorName, m.AuthorEmail)
49 for n := range closes {
50 actOnIssue(st, repo, actorID, m.SHA, n, true, subject, author)
61 for _, ref := range closes {
62 target, ok := closeTarget(st, repo, actorID, ref.Path)
63 if !ok {
64 continue
65 }
66 actOnIssue(st, repo, target, actorID, m.SHA, ref.N, true, subject, author)
5167 }
5268 for n := range refs {
53 actOnIssue(st, repo, actorID, m.SHA, n, false, subject, author)
69 actOnIssue(st, repo, repo, actorID, m.SHA, n, false, subject, author)
5470 }
5571 }
5672}
@@ -66,8 +82,12 @@ func ProcessCommitMessages(st *store.Store, dir string, repo store.Repo, actorID
6682// the sha let a bare "#N" in a commit message claim it first and silently
6783// suppress the close.
6884func ProcessMRDescription(st *store.Store, repo store.Repo, mr store.MR, actorID int64) {
69 for _, n := range closingRefs(mr.Title + "\n" + mr.Body) {
70 issue, err := st.IssueByNumber(repo.ID, n)
85 for _, ref := range closingRefs(mr.Title + "\n" + mr.Body) {
86 target, ok := closeTarget(st, repo, actorID, ref.Path)
87 if !ok {
88 continue
89 }
90 issue, err := st.IssueByNumber(target.ID, ref.N)
7191 if err != nil || issue.State != "open" {
7292 continue // no such issue, or a commit already closed it
7393 }
@@ -76,14 +96,14 @@ func ProcessMRDescription(st *store.Store, repo store.Repo, mr store.MR, actorID
7696 continue // this merge request already acted on this issue
7797 }
7898 if err := st.SetIssueState(issue.ID, "closed"); err != nil {
79 slog.Error("mr refs: closing issue", "issue", n, "err", err)
99 slog.Error("mr refs: closing issue", "issue", ref.N, "err", err)
80100 continue
81101 }
82102 link := fmt.Sprintf("[!%d](/%s/mrs/%d)", mr.Number, repo.Path(), mr.Number)
83103 st.AddIssueSystemComment(issue.ID, actorID,
84104 fmt.Sprintf("closed by merge request %s: %s", link, mr.Title))
85 st.RecordEvent(repo.ID, actorID, "issue.closed",
86 fmt.Sprintf(`{"number":%d,"mr":%d}`, n, mr.Number))
105 st.RecordEvent(target.ID, actorID, "issue.closed",
106 fmt.Sprintf(`{"number":%d,"mr":%d}`, ref.N, mr.Number))
87107 }
88108}
89109
@@ -93,21 +113,52 @@ func mrRefKey(number int64) string {
93113 return fmt.Sprintf("mr-%d", number)
94114}
95115
96// closingRefs returns the issue numbers a text closes, in no order.
97func closingRefs(text string) []int64 {
98 seen := map[int64]bool{}
99 var out []int64
116// closingRefs returns the references a text closes, in no order.
117func closingRefs(text string) []closeRef {
118 seen := map[closeRef]bool{}
119 var out []closeRef
100120 for _, g := range closePat.FindAllStringSubmatch(text, -1) {
101 n, err := strconv.ParseInt(g[1], 10, 64)
102 if err != nil || seen[n] {
121 n, err := strconv.ParseInt(g[2], 10, 64)
122 if err != nil {
123 continue
124 }
125 ref := closeRef{Path: strings.ToLower(g[1]), N: n}
126 if seen[ref] {
103127 continue
104128 }
105 seen[n] = true
106 out = append(out, n)
129 seen[ref] = true
130 out = append(out, ref)
107131 }
108132 return out
109133}
110134
135// closeTarget resolves where a closing reference acts: the source
136// repository for a bare #N, or the named repository when the actor holds
137// write there. false means the reference stays text; nothing is logged
138// above debug, since a refusal must not confirm the target exists.
139func closeTarget(st *store.Store, source store.Repo, actorID int64, path string) (store.Repo, bool) {
140 if path == "" {
141 return source, true
142 }
143 target, err := st.RepoByPath(path)
144 if err != nil {
145 return store.Repo{}, false
146 }
147 actor, err := st.UserByID(actorID)
148 if err != nil {
149 return store.Repo{}, false
150 }
151 grant, err := st.AccessRole(target.ID, actorID)
152 if err != nil {
153 return store.Repo{}, false
154 }
155 if !policy.CanWrite(actor, target, grant) {
156 slog.Debug("commit refs: cross-repo close refused", "source", source.Path(), "target", path)
157 return store.Repo{}, false
158 }
159 return target, true
160}
161
111162// RecordLandedCommits attributes commits that just landed on the default
112163// branch to accounts by verified author email, for the activity graph.
113164// Dedup by (repo, sha) makes rebases and re-runs harmless; unresolvable
@@ -137,8 +188,8 @@ func authorLink(st *store.Store, name, email string) string {
137188 return name
138189}
139190
140func actOnIssue(st *store.Store, repo store.Repo, actorID int64, sha string, number int64, close bool, subject, author string) {
141 issue, err := st.IssueByNumber(repo.ID, number)
191func actOnIssue(st *store.Store, source, target store.Repo, actorID int64, sha string, number int64, close bool, subject, author string) {
192 issue, err := st.IssueByNumber(target.ID, number)
142193 if err != nil {
143194 return // no such issue: the reference is just text
144195 }
@@ -152,14 +203,14 @@ func actOnIssue(st *store.Store, repo store.Repo, actorID int64, sha string, num
152203 }
153204 // Informational system entries, not comments from the pusher; the
154205 // linked sha renders clickable on the web.
155 link := fmt.Sprintf("[%s](/%s/commit/%s)", short, repo.Path(), sha)
206 link := fmt.Sprintf("[%s](/%s/commit/%s)", short, source.Path(), sha)
156207 if close && issue.State == "open" {
157208 if err := st.SetIssueState(issue.ID, "closed"); err != nil {
158209 slog.Error("commit refs: closing issue", "issue", number, "err", err)
159210 return
160211 }
161212 st.AddIssueSystemComment(issue.ID, actorID, fmt.Sprintf("closed by commit %s by %s: %s", link, author, subject))
162 st.RecordEvent(repo.ID, actorID, "issue.closed", fmt.Sprintf(`{"number":%d,"sha":%q}`, number, sha))
213 st.RecordEvent(target.ID, actorID, "issue.closed", fmt.Sprintf(`{"number":%d,"sha":%q}`, number, sha))
163214 return
164215 }
165216 st.AddIssueSystemComment(issue.ID, actorID, fmt.Sprintf("referenced in commit %s by %s: %s", link, author, subject))
internal/control/commitrefs_test.go +52 −8
@@ -2,7 +2,10 @@ package control
22
33import (
44 "slices"
5 "strings"
56 "testing"
7
8 "gitbay.org/gitbay/internal/store"
69)
710
811// The same keyword set has to work wherever the intent is written: a
@@ -11,23 +14,64 @@ func TestClosingRefs(t *testing.T) {
1114 for _, tc := range []struct {
1215 name string
1316 text string
14 want []int64
17 want []closeRef
1518 }{
16 {"closes", "Closes #50", []int64{50}},
17 {"lowercase and fix", "fixes #7", []int64{7}},
18 {"resolved", "resolved: #12", []int64{12}},
19 {"several", "Closes #1\n\nAlso fixes #2 and resolves #3", []int64{1, 2, 3}},
20 {"repeats collapse", "closes #4, closes #4", []int64{4}},
19 {"closes", "Closes #50", []closeRef{{"", 50}}},
20 {"lowercase and fix", "fixes #7", []closeRef{{"", 7}}},
21 {"resolved", "resolved: #12", []closeRef{{"", 12}}},
22 {"several", "Closes #1\n\nAlso fixes #2 and resolves #3", []closeRef{{"", 1}, {"", 2}, {"", 3}}},
23 {"repeats collapse", "closes #4, closes #4", []closeRef{{"", 4}}},
2124 {"bare references do not close", "see #9 for context", nil},
22 {"cross-repo stays display-only", "closes krz/other#3", nil},
25 {"cross-repo carries the path", "closes krz/other#3", []closeRef{{"krz/other", 3}}},
26 {"same number in two repos", "closes #3, closes krz/other#3", []closeRef{{"", 3}, {"krz/other", 3}}},
2327 {"keyword must be its own word", "unclosed #5", nil},
2428 } {
2529 t.Run(tc.name, func(t *testing.T) {
2630 got := closingRefs(tc.text)
27 slices.Sort(got)
31 slices.SortFunc(got, func(a, b closeRef) int {
32 if a.Path != b.Path {
33 return strings.Compare(a.Path, b.Path)
34 }
35 return int(a.N - b.N)
36 })
2837 if !slices.Equal(got, tc.want) {
2938 t.Errorf("closingRefs(%q) = %v, want %v", tc.text, got, tc.want)
3039 }
3140 })
3241 }
3342}
43
44// A merged merge request's description closes an issue in another
45// repository only when the merger holds write there. This drives the
46// same target resolution the commit path uses, without needing git.
47func TestMRDescriptionClosesAcrossRepos(t *testing.T) {
48 f := newOrgFixture(t)
49 libIssue, _ := f.st.CreateIssue(f.priv.ID, f.alice, "in priv", "", "md")
50 appIssue, _ := f.st.CreateIssue(f.app.ID, f.alice, "in app", "", "md")
51 _ = libIssue
52 _ = appIssue
53 mr := func(n int64, title string) store.MR {
54 return store.MR{Number: n, Title: title, Body: ""}
55 }
56 // carol cannot write acme/priv: the issue stays open and no comment
57 // lands.
58 ProcessMRDescription(f.st, f.app, mr(1, "Closes acme/priv#1"), f.carol)
59 if iss, _ := f.st.IssueByNumber(f.priv.ID, 1); iss.State != "open" {
60 t.Fatal("outsider closed a private repo's issue")
61 }
62 // alice can: it closes with a comment naming the source repository.
63 ProcessMRDescription(f.st, f.app, mr(2, "Closes acme/priv#1"), f.alice)
64 iss, _ := f.st.IssueByNumber(f.priv.ID, 1)
65 if iss.State != "closed" {
66 t.Fatal("writer did not close across repos")
67 }
68 comments, _ := f.st.ListIssueComments(iss.ID)
69 if len(comments) != 1 || !strings.Contains(comments[0].Body, "(/alice/app/mrs/2)") {
70 t.Fatalf("close comment = %+v", comments)
71 }
72 // An unknown path is text; a bare #N still acts in the source repo.
73 ProcessMRDescription(f.st, f.app, mr(3, "Closes nobody/nothing#1 and closes #1"), f.alice)
74 if iss, _ := f.st.IssueByNumber(f.app.ID, 1); iss.State != "closed" {
75 t.Fatal("bare #N stopped working")
76 }
77}