krz/gitbay

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

Commit ae4dce6203

ae4dce6203c6d4d5369eaf9a4df067d0135e72e1

parent: c333b152d5

Verified · cmc

cmc <hello@cleberg.net> · 2026-09-19 00:29 UTC 

control: mr close --by and mr edit --superseded-by

mr close <owner/name> <n> --by <m> closes and records the merge request
that carries the change forward in one step; the mr.closed event's data
carries "by":M. mr edit --superseded-by <m>|none sets or clears it after
the fact, refused on anything but a closed merge request. Both refuse a
self-reference (exit 2) and a missing !m in the same repository (exit
3). mrOut and mr show's text output carry the field.

editText grows an optional extra-flags list so mr edit can add
--superseded-by without loosening what issue edit accepts.

Ref #223
internal/control/issue.go +21 −11
@@ -289,12 +289,15 @@ func setIssueState(c *Ctx, args []string, state string) int {
289289}
290290
291291// editText parses --title/--body/--file -/--format and authorizes: author or
292// write. A nil format means the stored markup format stays as it is.
293func editText(c *Ctx, args []string, kind string) (rest []string, title, body, format *string, code int) {
294 f, err := parseFlags(args, flagSpec{Values: []string{"--title", "--body", "--file", "--format"}, MaxPos: -1,
292// write. A nil format means the stored markup format stays as it is. extra
293// names further value flags a caller wants (mr edit's --superseded-by):
294// they are accepted and reported in the returned flags, and count toward
295// "at least one edit was given" alongside title/body/format.
296func editText(c *Ctx, args []string, kind string, extra ...string) (rest []string, title, body, format *string, f flags, code int) {
297 f, err := parseFlags(args, flagSpec{Values: append([]string{"--title", "--body", "--file", "--format"}, extra...), MaxPos: -1,
295298 Usage: kind + " edit <owner/name> <n> [--title <t>] [--body <b> | --file -] [--format md|org]"})
296299 if err != nil {
297 return nil, nil, nil, nil, c.fail(protocol.ExitUsage, "%v", err)
300 return nil, nil, nil, nil, flags{}, c.fail(protocol.ExitUsage, "%v", err)
298301 }
299302 rest = f.Pos
300303 titleV, bodyV, file, formatV := f.Value("--title"), f.Value("--body"), f.Value("--file"), f.Value("--format")
@@ -302,20 +305,27 @@ func editText(c *Ctx, args []string, kind string) (rest []string, title, body, f
302305 if file != "" {
303306 b, err := bodyFrom(c, "", file)
304307 if err != nil {
305 return nil, nil, nil, nil, c.failInput(err)
308 return nil, nil, nil, nil, flags{}, c.failInput(err)
306309 }
307310 bodyV, haveBody = b, true
308311 }
309312 fmtName, err := markupFormat(formatV)
310313 if err != nil {
311 return nil, nil, nil, nil, c.failInput(err)
314 return nil, nil, nil, nil, flags{}, c.failInput(err)
312315 }
313 if !haveTitle && !haveBody && fmtName == "" {
314 return nil, nil, nil, nil, c.usage()
316 anyExtra := false
317 for _, e := range extra {
318 if f.Has(e) {
319 anyExtra = true
320 break
321 }
322 }
323 if !haveTitle && !haveBody && fmtName == "" && !anyExtra {
324 return nil, nil, nil, nil, flags{}, c.usage()
315325 }
316326 if haveTitle {
317327 if strings.TrimSpace(titleV) == "" {
318 return nil, nil, nil, nil, c.fail(protocol.ExitUsage, "--title must not be empty")
328 return nil, nil, nil, nil, flags{}, c.fail(protocol.ExitUsage, "--title must not be empty")
319329 }
320330 title = &titleV
321331 }
@@ -325,11 +335,11 @@ func editText(c *Ctx, args []string, kind string) (rest []string, title, body, f
325335 if fmtName != "" {
326336 format = &fmtName
327337 }
328 return rest, title, body, format, -1
338 return rest, title, body, format, f, -1
329339}
330340
331341func runIssueEdit(c *Ctx, args []string) int {
332 rest, title, body, format, code := editText(c, args, "issue")
342 rest, title, body, format, _, code := editText(c, args, "issue")
333343 if code >= 0 {
334344 return code
335345 }
internal/control/mr.go +76 −7
@@ -68,7 +68,7 @@ func init() {
6868 Usage: "mr diff <owner/name> <n>", ReadOnly: true, Run: runMRDiff})
6969 register(Command{Path: []string{"mr", "edit"},
7070 Summary: "edit title or body",
71 Usage: "mr edit <owner/name> <n> [--title <t>] [--body <b> | --file -] [--format md|org]",
71 Usage: "mr edit <owner/name> <n> [--title <t>] [--body <b> | --file -] [--format md|org] [--superseded-by <m>|none]",
7272 ReadsStdin: true, Run: runMREdit})
7373 register(Command{Path: []string{"mr", "retarget"},
7474 Summary: "retarget onto another branch",
@@ -88,7 +88,7 @@ func init() {
8888 Usage: "mr merge <owner/name> <n> [--strategy ff|merge|squash|rebase]", Run: runMRMerge})
8989 register(Command{Path: []string{"mr", "close"},
9090 Summary: "close without merging",
91 Usage: "mr close <owner/name> <n>", Run: runMRClose})
91 Usage: "mr close <owner/name> <n> [--by <m>]", Run: runMRClose})
9292}
9393
9494// ForkOut is what `repo fork` emits: where the fork landed, and what it
@@ -384,6 +384,9 @@ type mrOut struct {
384384 MergedBy string `json:"merged_by,omitempty"`
385385 ClosedAt string `json:"closed_at,omitempty"`
386386 ClosedBy string `json:"closed_by,omitempty"`
387 // SupersededBy is the merge request, by number, this one was closed
388 // in favour of. 0 means none.
389 SupersededBy int64 `json:"superseded_by,omitempty"`
387390}
388391
389392type stackRef struct {
@@ -429,7 +432,7 @@ func mrToOut(repo store.Repo, m store.MR, withBody bool) mrOut {
429432 Source: src, TargetRef: m.TargetRef, HeadSHA: m.HeadSHA, Milestone: m.Milestone,
430433 ReviewRequests: m.ReviewRequests,
431434 CreatedAt: m.CreatedAt, MergedAt: m.MergedAt, MergedBy: m.MergedBy,
432 ClosedAt: m.ClosedAt, ClosedBy: m.ClosedBy}
435 ClosedAt: m.ClosedAt, ClosedBy: m.ClosedBy, SupersededBy: m.SupersededBy}
433436 if withBody {
434437 o.Body = m.Body
435438 o.BodyFormat = m.BodyFormat
@@ -598,6 +601,9 @@ func runMRShow(c *Ctx, args []string) int {
598601 if d.ClosedAt != "" {
599602 fmt.Fprintf(w, "closed %s%s\n", d.ClosedAt, byWhom(d.ClosedBy))
600603 }
604 if d.SupersededBy != 0 {
605 fmt.Fprintf(w, "superseded by: !%d\n", d.SupersededBy)
606 }
601607 if d.Body != "" {
602608 fmt.Fprintf(w, "\n%s\n", d.Body)
603609 }
@@ -677,7 +683,7 @@ func runMRDiff(c *Ctx, args []string) int {
677683}
678684
679685func runMREdit(c *Ctx, args []string) int {
680 rest, title, body, format, code := editText(c, args, "mr")
686 rest, title, body, format, fl, code := editText(c, args, "mr", "--superseded-by")
681687 if code >= 0 {
682688 return code
683689 }
@@ -691,9 +697,33 @@ func runMREdit(c *Ctx, args []string) int {
691697 if code := authorOrWrite(c, repo, mr.Author, "edit this merge request"); code >= 0 {
692698 return code
693699 }
700 var clearSuperseded bool
701 var supersededBy int64
702 if fl.Has("--superseded-by") {
703 if mr.State != "closed" {
704 return c.fail(protocol.ExitUsage, "only a closed merge request can be superseded")
705 }
706 if v := fl.Value("--superseded-by"); v == "none" {
707 clearSuperseded = true
708 } else {
709 supersededBy, code = resolveSupersededBy(c, repo, mr.Number, v)
710 if code >= 0 {
711 return code
712 }
713 }
714 }
694715 if err := c.Store.UpdateMRText(mr.ID, title, body, format); err != nil {
695716 return c.fail(protocol.ExitFailure, "%v", err)
696717 }
718 if clearSuperseded {
719 if err := c.Store.SetSupersededBy(mr.ID, 0); err != nil {
720 return c.fail(protocol.ExitFailure, "%v", err)
721 }
722 } else if supersededBy != 0 {
723 if err := c.Store.SetSupersededBy(mr.ID, supersededBy); err != nil {
724 return c.fail(protocol.ExitFailure, "%v", err)
725 }
726 }
697727 c.Store.RecordEvent(repo.ID, c.User.ID, "mr.edited", fmt.Sprintf(`{"number":%d}`, mr.Number))
698728 return c.emit(map[string]any{"number": mr.Number}, func(w io.Writer) {
699729 fmt.Fprintf(w, "edited %s!%d\n", repo.Path(), mr.Number)
@@ -1499,14 +1529,19 @@ func setMRDraft(c *Ctx, args []string, draft bool) int {
14991529}
15001530
15011531func runMRClose(c *Ctx, args []string) int {
1502 repo, mr, code := mrRef(c, args, policy.CanRead)
1532 f, err := parseFlags(args, flagSpec{Values: []string{"--by"}, MaxPos: 2,
1533 Usage: "mr close <owner/name> <n> [--by <m>]"})
1534 if err != nil {
1535 return c.fail(protocol.ExitUsage, "%v", err)
1536 }
1537 repo, mr, code := mrRef(c, f.Pos, policy.CanRead)
15031538 if code >= 0 {
15041539 return code
15051540 }
15061541 if code := refuseArchived(c, repo); code >= 0 {
15071542 return code
15081543 }
1509 if len(args) != 2 {
1544 if len(f.Pos) != 2 {
15101545 return c.usage()
15111546 }
15121547 if code := authorOrWrite(c, repo, mr.Author, "close this merge request"); code >= 0 {
@@ -1515,10 +1550,24 @@ func runMRClose(c *Ctx, args []string) int {
15151550 if mr.State == "merged" || mr.State == "closed" {
15161551 return c.fail(protocol.ExitUsage, "MR !%d is already %s", mr.Number, mr.State)
15171552 }
1553 var by int64
1554 if f.Has("--by") {
1555 by, code = resolveSupersededBy(c, repo, mr.Number, f.Value("--by"))
1556 if code >= 0 {
1557 return code
1558 }
1559 }
15181560 if err := c.Store.MarkClosed(mr.ID, c.User.ID, ""); err != nil {
15191561 return c.fail(protocol.ExitFailure, "%v", err)
15201562 }
1521 c.Store.RecordEvent(repo.ID, c.User.ID, "mr.closed", fmt.Sprintf(`{"number":%d}`, mr.Number))
1563 eventData := fmt.Sprintf(`{"number":%d}`, mr.Number)
1564 if by != 0 {
1565 if err := c.Store.SetSupersededBy(mr.ID, by); err != nil {
1566 return c.fail(protocol.ExitFailure, "%v", err)
1567 }
1568 eventData = fmt.Sprintf(`{"number":%d,"by":%d}`, mr.Number, by)
1569 }
1570 c.Store.RecordEvent(repo.ID, c.User.ID, "mr.closed", eventData)
15221571 if parts, err := c.Store.MRParticipants(mr.ID); err == nil {
15231572 notify(c, parts, notice{repo: repo, kind: "mr",
15241573 subject: mrSubject(repo, mr.Number, mr.Title),
@@ -1530,6 +1579,26 @@ func runMRClose(c *Ctx, args []string) int {
15301579 })
15311580}
15321581
1582// resolveSupersededBy validates a --superseded-by/--by value against the
1583// merge request it would be set on: it must parse, name another merge
1584// request in the same repository (never itself), and that request must
1585// exist. -1 as the returned code means the value is good to use.
1586func resolveSupersededBy(c *Ctx, repo store.Repo, number int64, v string) (int64, int) {
1587 m, err := strconv.ParseInt(v, 10, 64)
1588 if err != nil {
1589 return 0, c.fail(protocol.ExitUsage, "bad MR number %q", v)
1590 }
1591 if m == number {
1592 return 0, c.fail(protocol.ExitUsage, "a merge request cannot supersede itself")
1593 }
1594 if _, err := c.Store.MRByNumber(repo.ID, m); errors.Is(err, store.ErrNotFound) {
1595 return 0, c.fail(protocol.ExitNotFound, "no merge request !%d on %s", m, repo.Path())
1596 } else if err != nil {
1597 return 0, c.fail(protocol.ExitFailure, "%v", err)
1598 }
1599 return m, -1
1600}
1601
15331602// reviewAction is what a review notification says it was. A verdict with
15341603// a batch behind it is a different thing from a bare verdict, and the
15351604// person reading the mail is deciding whether to open it.
internal/control/mr_test.go added +179
@@ -0,0 +1,179 @@
1package control
2
3import (
4 "bytes"
5 "encoding/json"
6 "strconv"
7 "strings"
8 "testing"
9
10 "gitbay.org/gitbay/internal/protocol"
11 "gitbay.org/gitbay/internal/store"
12)
13
14// mrTestCtx runs control commands as owner against st, capturing output.
15func mrTestCtx(st *store.Store, owner store.User) (*Ctx, *bytes.Buffer, *bytes.Buffer) {
16 out, errOut := &bytes.Buffer{}, &bytes.Buffer{}
17 c := &Ctx{User: owner, Scope: "full", Store: st, Stdout: out, Stderr: errOut}
18 return c, out, errOut
19}
20
21// twoMRTestRepo is a repository with two open merge requests, both
22// authored by the returned owner, so authorOrWrite never gets in the way.
23func twoMRTestRepo(t *testing.T) (*store.Store, store.Repo, store.User) {
24 t.Helper()
25 st, repo, uid := newQueueTestRepo(t)
26 owner := store.User{ID: uid, Username: "alice"}
27 if _, err := st.CreateMR(repo.ID, uid, repo.ID, "feature1", "main", "one", "", "abc111", "md", false); err != nil {
28 t.Fatal(err)
29 }
30 if _, err := st.CreateMR(repo.ID, uid, repo.ID, "feature2", "main", "two", "", "abc222", "md", false); err != nil {
31 t.Fatal(err)
32 }
33 return st, repo, owner
34}
35
36func mrShowJSON(t *testing.T, st *store.Store, owner store.User, path string, n int64) mrOut {
37 t.Helper()
38 c, out, errOut := mrTestCtx(st, owner)
39 if code := Dispatch(c, []string{"mr", "show", path, strconv.FormatInt(n, 10), "--json"}); code != protocol.ExitOK {
40 t.Fatalf("mr show: exit %d, %s", code, errOut.String())
41 }
42 var env struct {
43 Data mrOut `json:"data"`
44 }
45 if err := json.Unmarshal(out.Bytes(), &env); err != nil {
46 t.Fatalf("mr show JSON: %v\n%s", err, out.String())
47 }
48 return env.Data
49}
50
51// eventDataFor pulls the most recent data_json for a kind, so a test can
52// check what mr close recorded without a store accessor built just for it.
53func eventDataFor(t *testing.T, st *store.Store, kind string) string {
54 t.Helper()
55 var data string
56 err := st.DB.QueryRow("SELECT data_json FROM events WHERE kind = ? ORDER BY id DESC LIMIT 1", kind).Scan(&data)
57 if err != nil {
58 t.Fatalf("event %s: %v", kind, err)
59 }
60 return data
61}
62
63// Closing a merge request can name the one that carries its change
64// forward; mr show and the mr.closed event both then carry it (#223).
65func TestMRCloseWithBy(t *testing.T) {
66 st, repo, owner := twoMRTestRepo(t)
67 c, _, errOut := mrTestCtx(st, owner)
68 if code := Dispatch(c, []string{"mr", "close", repo.Path(), "1", "--by", "2"}); code != protocol.ExitOK {
69 t.Fatalf("mr close: exit %d, %s", code, errOut.String())
70 }
71 got := mrShowJSON(t, st, owner, repo.Path(), 1)
72 if got.State != "closed" || got.SupersededBy != 2 {
73 t.Fatalf("mr show !1 = %+v, want closed superseded_by 2", got)
74 }
75 if data := eventDataFor(t, st, "mr.closed"); !strings.Contains(data, `"by":2`) {
76 t.Fatalf("mr.closed event = %s, want it to carry by:2", data)
77 }
78}
79
80// mr close --by refuses a merge request naming itself.
81func TestMRCloseBySelfRefused(t *testing.T) {
82 st, repo, owner := twoMRTestRepo(t)
83 c, _, errOut := mrTestCtx(st, owner)
84 code := Dispatch(c, []string{"mr", "close", repo.Path(), "1", "--by", "1"})
85 if code != protocol.ExitUsage {
86 t.Fatalf("exit = %d, want %d; stderr: %s", code, protocol.ExitUsage, errOut.String())
87 }
88 if !strings.Contains(errOut.String(), "cannot supersede itself") {
89 t.Fatalf("stderr = %q, want it to say a merge request cannot supersede itself", errOut.String())
90 }
91}
92
93// mr close --by refuses a merge request number that does not exist in
94// the repository.
95func TestMRCloseByMissingRefused(t *testing.T) {
96 st, repo, owner := twoMRTestRepo(t)
97 c, _, errOut := mrTestCtx(st, owner)
98 code := Dispatch(c, []string{"mr", "close", repo.Path(), "1", "--by", "99"})
99 if code != protocol.ExitNotFound {
100 t.Fatalf("exit = %d, want %d; stderr: %s", code, protocol.ExitNotFound, errOut.String())
101 }
102 if !strings.Contains(errOut.String(), "no merge request !99") {
103 t.Fatalf("stderr = %q, want it to name !99 as missing", errOut.String())
104 }
105}
106
107// mr edit --superseded-by sets and clears the field on a closed merge
108// request.
109func TestMREditSupersededBySetAndClear(t *testing.T) {
110 st, repo, owner := twoMRTestRepo(t)
111 c, _, errOut := mrTestCtx(st, owner)
112 if code := Dispatch(c, []string{"mr", "close", repo.Path(), "1"}); code != protocol.ExitOK {
113 t.Fatalf("mr close: exit %d, %s", code, errOut.String())
114 }
115 c, _, errOut = mrTestCtx(st, owner)
116 if code := Dispatch(c, []string{"mr", "edit", repo.Path(), "1", "--superseded-by", "2"}); code != protocol.ExitOK {
117 t.Fatalf("mr edit --superseded-by 2: exit %d, %s", code, errOut.String())
118 }
119 if got := mrShowJSON(t, st, owner, repo.Path(), 1); got.SupersededBy != 2 {
120 t.Fatalf("SupersededBy = %d, want 2", got.SupersededBy)
121 }
122 c, _, errOut = mrTestCtx(st, owner)
123 if code := Dispatch(c, []string{"mr", "edit", repo.Path(), "1", "--superseded-by", "none"}); code != protocol.ExitOK {
124 t.Fatalf("mr edit --superseded-by none: exit %d, %s", code, errOut.String())
125 }
126 if got := mrShowJSON(t, st, owner, repo.Path(), 1); got.SupersededBy != 0 {
127 t.Fatalf("SupersededBy after clear = %d, want 0", got.SupersededBy)
128 }
129}
130
131// mr edit --superseded-by refuses a self-reference the same way mr close
132// --by does.
133func TestMREditSupersededBySelfRefused(t *testing.T) {
134 st, repo, owner := twoMRTestRepo(t)
135 c, _, errOut := mrTestCtx(st, owner)
136 if code := Dispatch(c, []string{"mr", "close", repo.Path(), "1"}); code != protocol.ExitOK {
137 t.Fatalf("mr close: exit %d, %s", code, errOut.String())
138 }
139 c, _, errOut = mrTestCtx(st, owner)
140 code := Dispatch(c, []string{"mr", "edit", repo.Path(), "1", "--superseded-by", "1"})
141 if code != protocol.ExitUsage {
142 t.Fatalf("exit = %d, want %d; stderr: %s", code, protocol.ExitUsage, errOut.String())
143 }
144 if !strings.Contains(errOut.String(), "cannot supersede itself") {
145 t.Fatalf("stderr = %q, want it to say a merge request cannot supersede itself", errOut.String())
146 }
147}
148
149// mr edit --superseded-by refuses a merge request number that does not
150// exist in the repository.
151func TestMREditSupersededByMissingRefused(t *testing.T) {
152 st, repo, owner := twoMRTestRepo(t)
153 c, _, errOut := mrTestCtx(st, owner)
154 if code := Dispatch(c, []string{"mr", "close", repo.Path(), "1"}); code != protocol.ExitOK {
155 t.Fatalf("mr close: exit %d, %s", code, errOut.String())
156 }
157 c, _, errOut = mrTestCtx(st, owner)
158 code := Dispatch(c, []string{"mr", "edit", repo.Path(), "1", "--superseded-by", "99"})
159 if code != protocol.ExitNotFound {
160 t.Fatalf("exit = %d, want %d; stderr: %s", code, protocol.ExitNotFound, errOut.String())
161 }
162 if !strings.Contains(errOut.String(), "no merge request !99") {
163 t.Fatalf("stderr = %q, want it to name !99 as missing", errOut.String())
164 }
165}
166
167// mr edit --superseded-by refuses an open merge request: only a closed
168// one can be superseded.
169func TestMREditSupersededByOnOpenMRRefused(t *testing.T) {
170 st, repo, owner := twoMRTestRepo(t)
171 c, _, errOut := mrTestCtx(st, owner)
172 code := Dispatch(c, []string{"mr", "edit", repo.Path(), "1", "--superseded-by", "2"})
173 if code != protocol.ExitUsage {
174 t.Fatalf("exit = %d, want %d; stderr: %s", code, protocol.ExitUsage, errOut.String())
175 }
176 if !strings.Contains(errOut.String(), "only a closed merge request can be superseded") {
177 t.Fatalf("stderr = %q, want the closed-only refusal", errOut.String())
178 }
179}