krz/gitbay

A CLI-first git forge. cli forge git self-hosted https://gitbay.org

Commit 37841c7286

37841c728628e5783e2c4e1ad6603435bc5efa12

parent: 94884a2232

Verified · cmc ci/build: success ci/test: success ci/vuln: success

cmc <hello@cleberg.net> · 2026-09-04 00:57 UTC 

control: issues and merge requests share their thread code

runIssueComment and runMRComment were the same fifty lines with the
noun swapped, as were issueRef and mrRef, the commentOut type declared
in both files, and the author-or-write check written out five times.
thread.go holds each once: refArgs resolves <owner/name> <n>,
authorOrWrite admits the author or write access with the action named
in the refusal, commentOut is one type, and runComment takes the
noun's resolver, comment insert and participant query.

Closes #110

Layout: unified · split

internal/control/issue.go +11 −66
@@ -49,17 +49,10 @@ func init() {
49 49
50// issueArgs parses "<owner/name> <n>" plus flags handled by the caller. 50// issueArgs parses "<owner/name> <n>" plus flags handled by the caller.
51func issueRef(c *Ctx, args []string, perm func(store.User, store.Repo, string) bool) (store.Repo, store.Issue, int) { 51func issueRef(c *Ctx, args []string, perm func(store.User, store.Repo, string) bool) (store.Repo, store.Issue, int) {
52 if len(args) < 2 { 52 repo, n, code := refArgs(c, args, perm, "issue")
53 return store.Repo{}, store.Issue{}, c.fail(protocol.ExitUsage, "expected <owner/name> <number>")
54 }
55 repo, code := resolveRepo(c, args[0], perm)
56 if code >= 0 { 53 if code >= 0 {
57 return repo, store.Issue{}, code 54 return repo, store.Issue{}, code
58 } 55 }
59 n, err := strconv.ParseInt(args[1], 10, 64)
60 if err != nil {
61 return repo, store.Issue{}, c.fail(protocol.ExitUsage, "bad issue number %q", args[1])
62 }
63 issue, err := c.Store.IssueByNumber(repo.ID, n) 56 issue, err := c.Store.IssueByNumber(repo.ID, n)
64 if errors.Is(err, store.ErrNotFound) { 57 if errors.Is(err, store.ErrNotFound) {
65 return repo, issue, c.fail(protocol.ExitNotFound, "issue #%d not found in %s", n, repo.Path()) 58 return repo, issue, c.fail(protocol.ExitNotFound, "issue #%d not found in %s", n, repo.Path())
@@ -219,12 +212,6 @@ func runIssueShow(c *Ctx, args []string) int {
219 if err != nil { 212 if err != nil {
220 return c.fail(protocol.ExitFailure, "%v", err) 213 return c.fail(protocol.ExitFailure, "%v", err)
221 } 214 }
222 type commentOut struct {
223 Author string `json:"author"`
224 Body string `json:"body"`
225 BodyFormat string `json:"body_format,omitempty"`
226 CreatedAt string `json:"created_at"`
227 }
228 var cs []commentOut 215 var cs []commentOut
229 for _, cm := range comments { 216 for _, cm := range comments {
230 cs = append(cs, commentOut{cm.Author, cm.Body, cm.BodyFormat, cm.CreatedAt}) 217 cs = append(cs, commentOut{cm.Author, cm.Body, cm.BodyFormat, cm.CreatedAt})
@@ -252,45 +239,12 @@ func runIssueShow(c *Ctx, args []string) int {
252} 239}
253 240
254func runIssueComment(c *Ctx, args []string) int { 241func runIssueComment(c *Ctx, args []string) int {
255 f, err := parseFlags(args, flagSpec{Values: []string{"--format", "--message", "--file"}, MaxPos: -1, 242 return runComment(c, args, issueThread, "issue",
256 Usage: "issue comment <owner/name> <n> [--message <m> | --file -] [--format md|org]"}) 243 func(rest []string) (store.Repo, int64, int64, string, int) {
257 if err != nil { 244 repo, issue, code := issueRef(c, rest, policy.CanRead)
258 return c.fail(protocol.ExitUsage, "%v", err) 245 return repo, issue.ID, issue.Number, issue.Title, code
259 } 246 },
260 rest := f.Pos 247 c.Store.AddIssueComment, c.Store.IssueParticipants)
261 message, file, format := f.Value("--message"), f.Value("--file"), f.Value("--format")
262 fmtName, err := markupFormat(format)
263 if err != nil {
264 return c.failErr(err)
265 }
266 if fmtName == "" {
267 fmtName = "md"
268 }
269 repo, issue, code := issueRef(c, rest, policy.CanRead)
270 if code >= 0 {
271 return code
272 }
273 if code := refuseArchived(c, repo); code >= 0 {
274 return code
275 }
276 body, err := bodyFrom(c, message, file)
277 if err != nil {
278 return c.failErr(err)
279 }
280 if strings.TrimSpace(body) == "" {
281 return c.fail(protocol.ExitUsage, "empty comment; use --message or --file -")
282 }
283 if err := c.Store.AddIssueComment(issue.ID, c.User.ID, body, fmtName); err != nil {
284 return c.fail(protocol.ExitFailure, "%v", err)
285 }
286 c.Store.RecordEvent(repo.ID, c.User.ID, "issue.commented", fmt.Sprintf(`{"number":%d}`, issue.Number))
287 if parts, err := c.Store.IssueParticipants(issue.ID); err == nil {
288 notifyUsers(c, parts, issueSubject(repo, issue.Number, issue.Title),
289 notifyBody(c, fmt.Sprintf("commented on #%d", issue.Number), body, fmt.Sprintf("%s/issues/%d", repo.Path(), issue.Number)))
290 }
291 return c.emit(map[string]any{"number": issue.Number}, func(w io.Writer) {
292 fmt.Fprintf(w, "commented on %s#%d\n", repo.Path(), issue.Number)
293 })
294} 248}
295 249
296func setIssueState(c *Ctx, args []string, state string) int { 250func setIssueState(c *Ctx, args []string, state string) int {
@@ -305,13 +259,8 @@ func setIssueState(c *Ctx, args []string, state string) int {
305 if len(args) != 2 { 259 if len(args) != 2 {
306 return c.fail(protocol.ExitUsage, "usage: issue %s <owner/name> <n>", state) 260 return c.fail(protocol.ExitUsage, "usage: issue %s <owner/name> <n>", state)
307 } 261 }
308 grant, err := c.Store.AccessRole(repo.ID, c.User.ID) 262 if code := authorOrWrite(c, repo, issue.Author, map[string]string{"open": "reopen", "closed": "close"}[state]+" this issue"); code >= 0 {
309 if err != nil { 263 return code
310 return c.fail(protocol.ExitFailure, "%v", err)
311 }
312 if issue.Author != c.User.Username && !policy.CanWrite(c.User, repo, grant) {
313 return c.fail(protocol.ExitDenied, "only the author or users with write access can %s this issue",
314 map[string]string{"open": "reopen", "closed": "close"}[state])
315 } 264 }
316 if issue.State == state { 265 if issue.State == state {
317 return c.fail(protocol.ExitUsage, "issue #%d is already %s", issue.Number, state) 266 return c.fail(protocol.ExitUsage, "issue #%d is already %s", issue.Number, state)
@@ -382,12 +331,8 @@ func runIssueEdit(c *Ctx, args []string) int {
382 if code := refuseArchived(c, repo); code >= 0 { 331 if code := refuseArchived(c, repo); code >= 0 {
383 return code 332 return code
384 } 333 }
385 grant, err := c.Store.AccessRole(repo.ID, c.User.ID) 334 if code := authorOrWrite(c, repo, issue.Author, "edit this issue"); code >= 0 {
386 if err != nil { 335 return code
387 return c.fail(protocol.ExitFailure, "%v", err)
388 }
389 if issue.Author != c.User.Username && !policy.CanWrite(c.User, repo, grant) {
390 return c.fail(protocol.ExitDenied, "only the author or users with write access can edit this issue")
391 } 336 }
392 if err := c.Store.UpdateIssueText(issue.ID, title, body, format); err != nil { 337 if err := c.Store.UpdateIssueText(issue.ID, title, body, format); err != nil {
393 return c.fail(protocol.ExitFailure, "%v", err) 338 return c.fail(protocol.ExitFailure, "%v", err)
internal/control/mr.go +13 −71
@@ -196,17 +196,10 @@ func runRequireSigned(c *Ctx, args []string) int {
196 196
197// mrRef parses "<owner/name> <n>" and loads the MR. 197// mrRef parses "<owner/name> <n>" and loads the MR.
198func mrRef(c *Ctx, args []string, perm func(store.User, store.Repo, string) bool) (store.Repo, store.MR, int) { 198func mrRef(c *Ctx, args []string, perm func(store.User, store.Repo, string) bool) (store.Repo, store.MR, int) {
199 if len(args) < 2 { 199 repo, n, code := refArgs(c, args, perm, "MR")
200 return store.Repo{}, store.MR{}, c.fail(protocol.ExitUsage, "expected <owner/name> <number>")
201 }
202 repo, code := resolveRepo(c, args[0], perm)
203 if code >= 0 { 200 if code >= 0 {
204 return repo, store.MR{}, code 201 return repo, store.MR{}, code
205 } 202 }
206 n, err := strconv.ParseInt(args[1], 10, 64)
207 if err != nil {
208 return repo, store.MR{}, c.fail(protocol.ExitUsage, "bad MR number %q", args[1])
209 }
210 mr, err := c.Store.MRByNumber(repo.ID, n) 203 mr, err := c.Store.MRByNumber(repo.ID, n)
211 if errors.Is(err, store.ErrNotFound) { 204 if errors.Is(err, store.ErrNotFound) {
212 return repo, mr, c.fail(protocol.ExitNotFound, "MR !%d not found in %s", n, repo.Path()) 205 return repo, mr, c.fail(protocol.ExitNotFound, "MR !%d not found in %s", n, repo.Path())
@@ -456,12 +449,6 @@ func runMRShow(c *Ctx, args []string) int {
456 if err != nil { 449 if err != nil {
457 return c.fail(protocol.ExitFailure, "%v", err) 450 return c.fail(protocol.ExitFailure, "%v", err)
458 } 451 }
459 type commentOut struct {
460 Author string `json:"author"`
461 Body string `json:"body"`
462 BodyFormat string `json:"body_format,omitempty"`
463 CreatedAt string `json:"created_at"`
464 }
465 type reviewOut struct { 452 type reviewOut struct {
466 Reviewer string `json:"reviewer"` 453 Reviewer string `json:"reviewer"`
467 Verdict string `json:"verdict"` 454 Verdict string `json:"verdict"`
@@ -607,12 +594,8 @@ func runMREdit(c *Ctx, args []string) int {
607 if code := refuseArchived(c, repo); code >= 0 { 594 if code := refuseArchived(c, repo); code >= 0 {
608 return code 595 return code
609 } 596 }
610 grant, err := c.Store.AccessRole(repo.ID, c.User.ID) 597 if code := authorOrWrite(c, repo, mr.Author, "edit this merge request"); code >= 0 {
611 if err != nil { 598 return code
612 return c.fail(protocol.ExitFailure, "%v", err)
613 }
614 if mr.Author != c.User.Username && !policy.CanWrite(c.User, repo, grant) {
615 return c.fail(protocol.ExitDenied, "only the author or users with write access can edit this merge request")
616 } 599 }
617 if err := c.Store.UpdateMRText(mr.ID, title, body, format); err != nil { 600 if err := c.Store.UpdateMRText(mr.ID, title, body, format); err != nil {
618 return c.fail(protocol.ExitFailure, "%v", err) 601 return c.fail(protocol.ExitFailure, "%v", err)
@@ -635,12 +618,8 @@ func runMRRetarget(c *Ctx, args []string) int {
635 if code := refuseArchived(c, repo); code >= 0 { 618 if code := refuseArchived(c, repo); code >= 0 {
636 return code 619 return code
637 } 620 }
638 grant, err := c.Store.AccessRole(repo.ID, c.User.ID) 621 if code := authorOrWrite(c, repo, mr.Author, "retarget this merge request"); code >= 0 {
639 if err != nil { 622 return code
640 return c.fail(protocol.ExitFailure, "%v", err)
641 }
642 if mr.Author != c.User.Username && !policy.CanWrite(c.User, repo, grant) {
643 return c.fail(protocol.ExitDenied, "only the author or users with write access can retarget this merge request")
644 } 623 }
645 if mr.State == "merged" || mr.State == "closed" { 624 if mr.State == "merged" || mr.State == "closed" {
646 return c.fail(protocol.ExitUsage, "!%d is %s; only an open merge request can be retargeted", mr.Number, mr.State) 625 return c.fail(protocol.ExitUsage, "!%d is %s; only an open merge request can be retargeted", mr.Number, mr.State)
@@ -680,45 +659,12 @@ func runMRRetarget(c *Ctx, args []string) int {
680} 659}
681 660
682func runMRComment(c *Ctx, args []string) int { 661func runMRComment(c *Ctx, args []string) int {
683 f, err := parseFlags(args, flagSpec{Values: []string{"--message", "--file", "--format"}, MaxPos: -1, 662 return runComment(c, args, mrThread, "mr",
684 Usage: "mr comment <owner/name> <n> [--message <m> | --file -] [--format md|org]"}) 663 func(rest []string) (store.Repo, int64, int64, string, int) {
685 if err != nil { 664 repo, mr, code := mrRef(c, rest, policy.CanRead)
686 return c.fail(protocol.ExitUsage, "%v", err) 665 return repo, mr.ID, mr.Number, mr.Title, code
687 } 666 },
688 rest := f.Pos 667 c.Store.AddMRComment, c.Store.MRParticipants)
689 message, file, format := f.Value("--message"), f.Value("--file"), f.Value("--format")
690 fmtName, err := markupFormat(format)
691 if err != nil {
692 return c.failErr(err)
693 }
694 if fmtName == "" {
695 fmtName = "md"
696 }
697 repo, mr, code := mrRef(c, rest, policy.CanRead)
698 if code >= 0 {
699 return code
700 }
701 if code := refuseArchived(c, repo); code >= 0 {
702 return code
703 }
704 body, err := bodyFrom(c, message, file)
705 if err != nil {
706 return c.failErr(err)
707 }
708 if strings.TrimSpace(body) == "" {
709 return c.fail(protocol.ExitUsage, "empty comment; use --message or --file -")
710 }
711 if err := c.Store.AddMRComment(mr.ID, c.User.ID, body, fmtName); err != nil {
712 return c.fail(protocol.ExitFailure, "%v", err)
713 }
714 c.Store.RecordEvent(repo.ID, c.User.ID, "mr.commented", fmt.Sprintf(`{"number":%d}`, mr.Number))
715 if parts, err := c.Store.MRParticipants(mr.ID); err == nil {
716 notifyUsers(c, parts, mrSubject(repo, mr.Number, mr.Title),
717 notifyBody(c, fmt.Sprintf("commented on !%d", mr.Number), body, fmt.Sprintf("%s/mrs/%d", repo.Path(), mr.Number)))
718 }
719 return c.emit(map[string]any{"number": mr.Number}, func(w io.Writer) {
720 fmt.Fprintf(w, "commented on %s!%d\n", repo.Path(), mr.Number)
721 })
722} 668}
723 669
724func runMRReview(c *Ctx, args []string) int { 670func runMRReview(c *Ctx, args []string) int {
@@ -1194,12 +1140,8 @@ func runMRClose(c *Ctx, args []string) int {
1194 if len(args) != 2 { 1140 if len(args) != 2 {
1195 return c.fail(protocol.ExitUsage, "usage: mr close <owner/name> <n>") 1141 return c.fail(protocol.ExitUsage, "usage: mr close <owner/name> <n>")
1196 } 1142 }
1197 grant, err := c.Store.AccessRole(repo.ID, c.User.ID) 1143 if code := authorOrWrite(c, repo, mr.Author, "close this merge request"); code >= 0 {
1198 if err != nil { 1144 return code
1199 return c.fail(protocol.ExitFailure, "%v", err)
1200 }
1201 if mr.Author != c.User.Username && !policy.CanWrite(c.User, repo, grant) {
1202 return c.fail(protocol.ExitDenied, "only the author or users with write access can close this MR")
1203 } 1145 }
1204 if mr.State == "merged" || mr.State == "closed" { 1146 if mr.State == "merged" || mr.State == "closed" {
1205 return c.fail(protocol.ExitUsage, "MR !%d is already %s", mr.Number, mr.State) 1147 return c.fail(protocol.ExitUsage, "MR !%d is already %s", mr.Number, mr.State)
internal/control/thread.go added +116
@@ -0,0 +1,116 @@
1package control
2
3import (
4 "fmt"
5 "io"
6 "strconv"
7 "strings"
8
9 "gitbay.org/gitbay/internal/policy"
10 "gitbay.org/gitbay/internal/protocol"
11 "gitbay.org/gitbay/internal/store"
12)
13
14// Issues and merge requests share their shape: a numbered thread in a
15// repository with an author, comments and participants. What used to be
16// two copies of the same code, one per noun, lives here once (#110).
17
18// refArgs resolves "<owner/name> <n>" to the repository and the number.
19func refArgs(c *Ctx, args []string, perm func(store.User, store.Repo, string) bool, noun string) (store.Repo, int64, int) {
20 if len(args) < 2 {
21 return store.Repo{}, 0, c.fail(protocol.ExitUsage, "expected <owner/name> <number>")
22 }
23 repo, code := resolveRepo(c, args[0], perm)
24 if code >= 0 {
25 return repo, 0, code
26 }
27 n, err := strconv.ParseInt(args[1], 10, 64)
28 if err != nil {
29 return repo, 0, c.fail(protocol.ExitUsage, "bad %s number %q", noun, args[1])
30 }
31 return repo, n, -1
32}
33
34// authorOrWrite admits the thread's author and anyone with write access,
35// which is who may change what a thread says or whether it is open.
36// -1 means proceed; what names the action in the refusal.
37func authorOrWrite(c *Ctx, repo store.Repo, author, what string) int {
38 if author == c.User.Username {
39 return -1
40 }
41 grant, err := c.Store.AccessRole(repo.ID, c.User.ID)
42 if err != nil {
43 return c.fail(protocol.ExitFailure, "%v", err)
44 }
45 if !policy.CanWrite(c.User, repo, grant) {
46 return c.fail(protocol.ExitDenied, "only the author or users with write access can %s", what)
47 }
48 return -1
49}
50
51// commentOut is one comment as show emits it, for both nouns.
52type commentOut struct {
53 Author string `json:"author"`
54 Body string `json:"body"`
55 BodyFormat string `json:"body_format,omitempty"`
56 CreatedAt string `json:"created_at"`
57}
58
59// thread is what a comment command needs to know about its noun.
60type thread struct {
61 symbol string // "#" or "!"
62 segment string // "issues" or "mrs"
63 event string // issue.commented or mr.commented
64}
65
66var (
67 issueThread = thread{"#", "issues", "issue.commented"}
68 mrThread = thread{"!", "mrs", "mr.commented"}
69)
70
71// runComment is issue comment and mr comment: the noun's resolver hands
72// back the thread's id, number and title, and the rest is the same.
73func runComment(c *Ctx, args []string, t thread, noun string,
74 resolve func(rest []string) (repo store.Repo, id, number int64, title string, code int),
75 add func(id, userID int64, body, format string) error,
76 participants func(id int64) ([]int64, error),
77) int {
78 f, err := parseFlags(args, flagSpec{Values: []string{"--message", "--file", "--format"}, MaxPos: -1,
79 Usage: noun + " comment <owner/name> <n> [--message <m> | --file -] [--format md|org]"})
80 if err != nil {
81 return c.fail(protocol.ExitUsage, "%v", err)
82 }
83 fmtName, err := markupFormat(f.Value("--format"))
84 if err != nil {
85 return c.failErr(err)
86 }
87 if fmtName == "" {
88 fmtName = "md"
89 }
90 repo, id, number, title, code := resolve(f.Pos)
91 if code >= 0 {
92 return code
93 }
94 if code := refuseArchived(c, repo); code >= 0 {
95 return code
96 }
97 body, err := bodyFrom(c, f.Value("--message"), f.Value("--file"))
98 if err != nil {
99 return c.failErr(err)
100 }
101 if strings.TrimSpace(body) == "" {
102 return c.fail(protocol.ExitUsage, "empty comment; use --message or --file -")
103 }
104 if err := add(id, c.User.ID, body, fmtName); err != nil {
105 return c.fail(protocol.ExitFailure, "%v", err)
106 }
107 c.Store.RecordEvent(repo.ID, c.User.ID, t.event, fmt.Sprintf(`{"number":%d}`, number))
108 if parts, err := participants(id); err == nil {
109 subject := fmt.Sprintf("[%s] %s%d: %s", repo.Path(), t.symbol, number, title)
110 notifyUsers(c, parts, subject,
111 notifyBody(c, fmt.Sprintf("commented on %s%d", t.symbol, number), body, fmt.Sprintf("%s/%s/%d", repo.Path(), t.segment, number)))
112 }
113 return c.emit(map[string]any{"number": number}, func(w io.Writer) {
114 fmt.Fprintf(w, "commented on %s%s%d\n", repo.Path(), t.symbol, number)
115 })
116}