mr: close in favour of another merge request !413

merged merged by cmc on 2026-09-19 00:45 UTC · krz/gitbay:supersede-223 into main

13 files changed, +449 −25

Layout: unified · split

.gitbay/wiki/Parity.org +1

	@@ -41,6 +41,7 @@ browser-only and the iOS build screen unable to say more than the log.
41	\| comment on a diff line \| yes \| yes \| yes \|	41	\| comment on a diff line \| yes \| yes \| yes \|
42	\| merge (all strategies) \| yes \| yes \| yes \|	42	\| merge (all strategies) \| yes \| yes \| yes \|
43	\| close \| yes \| yes \| yes \|	43	\| close \| yes \| yes \| yes \|
		44	\| close in favour of another \| yes \| yes \| no \|
44	\| create \| yes \| yes \| yes \|	45	\| create \| yes \| yes \| yes \|
45	\| draft, ready \| yes \| yes \| yes \|	46	\| draft, ready \| yes \| yes \| yes \|
46	\| search title and body \| yes \| yes \| yes \|	47	\| search title and body \| yes \| yes \| yes \|

.gitbay/wiki/Users.org +5

 gitbay mr close 4
 #+end_src
+A merge request closed without merging can name the one that carries
+its change forward: =mr close 4 --by 7= records it and both pages show
+it, and =mr edit 4 --superseded-by 7|none= sets or clears it
+afterwards, refused on anything but a closed merge request.
 Semantics worth knowing:
 - the MR head lives in the *target* repository as

e2e/mrweb_test.go +65

	@@ -388,3 +388,68 @@ func TestMRDiffEmptyExplained(t *testing.T) {
388	t.Fatalf("empty diff unexplained:\n%s", body)	388	t.Fatalf("empty diff unexplained:\n%s", body)
389	}	389	}
390	}	390	}
		391
		392	// TestMRSupersedes closes one merge request in favour of another from the
		393	// web form, and checks both pages say so; clearing it over ssh removes
		394	// both lines again (#223).
		395	func TestMRSupersedes(t *testing.T) {
		396	inst := startInstanceWith(t, "[web]\nmode = \"accounts\"\n")
		397	aliceKey := inst.newKey(t, "alice")
		398	inst.admin(t, "admin", "user", "create", "alice",
		399	"--key", aliceKey+".pub", "--email", "alice@example.test", "--verified")
		400	if _, errOut, code := inst.ssh(t, aliceKey, "", "repo", "create", "alice/app"); code != 0 {
		401	t.Fatalf("repo create: %s", errOut)
		402	}
		403	env := inst.gitEnv(aliceKey)
		404	work := t.TempDir()
		405	mustGit(t, work, env, "clone", inst.sshURL("alice/app"), "w")
		406	dir := filepath.Join(work, "w")
		407	os.WriteFile(filepath.Join(dir, "README"), []byte("base\n"), 0o644)
		408	mustGit(t, dir, env, "checkout", "-q", "-b", "main")
		409	mustGit(t, dir, env, "add", ".")
		410	mustGit(t, dir, env, "commit", "-q", "-m", "base")
		411	mustGit(t, dir, env, "push", "-q", "origin", "main")
		412
		413	for _, branch := range []string{"one", "two"} {
		414	mustGit(t, dir, env, "checkout", "-q", "main")
		415	mustGit(t, dir, env, "checkout", "-q", "-b", branch)
		416	os.WriteFile(filepath.Join(dir, branch+".txt"), []byte(branch+"\n"), 0o644)
		417	mustGit(t, dir, env, "add", ".")
		418	mustGit(t, dir, env, "commit", "-q", "-m", branch)
		419	mustGit(t, dir, env, "push", "-q", "origin", branch)
		420	}
		421	if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "create", "alice/app",
		422	"--source", "one", "--target", "main", "--title", "one"); code != 0 {
		423	t.Fatalf("mr create one: %s", errOut)
		424	}
		425	if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "create", "alice/app",
		426	"--source", "two", "--target", "main", "--title", "two"); code != 0 {
		427	t.Fatalf("mr create two: %s", errOut)
		428	}
		429
		430	alice := inst.login(t, aliceKey)
		431	if status, body := browserPost(t, alice, inst.base()+"/alice/app/mrs/1/close", url.Values{"by": {"2"}}); status != 200 {
		432	t.Fatalf("close post: %d\n%s", status, body)
		433	}
		434
		435	_, body1 := browserGet(t, alice, inst.base()+"/alice/app/mrs/1")
		436	if !strings.Contains(body1, `in favour of <a href="/alice/app/mrs/2">!2</a>`) {
		437	t.Fatalf("!1 does not say it was superseded:\n%s", body1)
		438	}
		439	_, body2 := browserGet(t, alice, inst.base()+"/alice/app/mrs/2")
		440	if !strings.Contains(body2, `supersedes <a href="/alice/app/mrs/1">!1</a>`) {
		441	t.Fatalf("!2 does not say what it supersedes:\n%s", body2)
		442	}
		443
		444	if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "edit", "alice/app", "1", "--superseded-by", "none"); code != 0 {
		445	t.Fatalf("mr edit --superseded-by none: %s", errOut)
		446	}
		447	_, body1 = browserGet(t, alice, inst.base()+"/alice/app/mrs/1")
		448	if strings.Contains(body1, "in favour of") {
		449	t.Fatalf("!1 still says it was superseded after clearing:\n%s", body1)
		450	}
		451	_, body2 = browserGet(t, alice, inst.base()+"/alice/app/mrs/2")
		452	if strings.Contains(body2, "supersedes") {
		453	t.Fatalf("!2 still says it supersedes after clearing:\n%s", body2)
		454	}
		455	}

internal/control/issue.go +21 −11

 }
 // editText parses --title/--body/--file -/--format and authorizes: author or
-// write. A nil format means the stored markup format stays as it is.
+// write. A nil format means the stored markup format stays as it is. extra
-func editText(c *Ctx, args []string, kind string) (rest []string, title, body, format *string, code int) {
+// names further value flags a caller wants (mr edit's --superseded-by):
-	f, err := parseFlags(args, flagSpec{Values: []string{"--title", "--body", "--file", "--format"}, MaxPos: -1,
+// they are accepted and reported in the returned flags, and count toward
+// "at least one edit was given" alongside title/body/format.
+func editText(c *Ctx, args []string, kind string, extra ...string) (rest []string, title, body, format *string, f flags, code int) {
+	f, err := parseFlags(args, flagSpec{Values: append([]string{"--title", "--body", "--file", "--format"}, extra...), MaxPos: -1,
 		Usage: kind + " edit <owner/name> <n> [--title <t>] [--body <b> | --file -] [--format md|org]"})
 	if err != nil {
-		return nil, nil, nil, nil, c.fail(protocol.ExitUsage, "%v", err)
+		return nil, nil, nil, nil, flags{}, c.fail(protocol.ExitUsage, "%v", err)
 	}
 	rest = f.Pos
 	titleV, bodyV, file, formatV := f.Value("--title"), f.Value("--body"), f.Value("--file"), f.Value("--format")
 	if file != "" {
 		b, err := bodyFrom(c, "", file)
 		if err != nil {
-			return nil, nil, nil, nil, c.failInput(err)
+			return nil, nil, nil, nil, flags{}, c.failInput(err)
 		}
 		bodyV, haveBody = b, true
 	}
 	fmtName, err := markupFormat(formatV)
 	if err != nil {
-		return nil, nil, nil, nil, c.failInput(err)
+		return nil, nil, nil, nil, flags{}, c.failInput(err)
 	}
-	if !haveTitle && !haveBody && fmtName == "" {
+	anyExtra := false
-		return nil, nil, nil, nil, c.usage()
+	for _, e := range extra {
+		if f.Has(e) {
+			anyExtra = true
+			break
+		}
+	}
+	if !haveTitle && !haveBody && fmtName == "" && !anyExtra {
+		return nil, nil, nil, nil, flags{}, c.usage()
 	}
 	if haveTitle {
 		if strings.TrimSpace(titleV) == "" {
-			return nil, nil, nil, nil, c.fail(protocol.ExitUsage, "--title must not be empty")
+			return nil, nil, nil, nil, flags{}, c.fail(protocol.ExitUsage, "--title must not be empty")
 		}
 		title = &titleV
 	}
 	if fmtName != "" {
 		format = &fmtName
 	}
-	return rest, title, body, format, -1
+	return rest, title, body, format, f, -1
 }
 func runIssueEdit(c *Ctx, args []string) int {
-	rest, title, body, format, code := editText(c, args, "issue")
+	rest, title, body, format, _, code := editText(c, args, "issue")
 	if code >= 0 {
 		return code
 	}

internal/control/mr.go +76 −7

 		Usage:   "mr diff <owner/name> <n>", ReadOnly: true, Run: runMRDiff})
 	register(Command{Path: []string{"mr", "edit"},
 		Summary:    "edit title or body",
-		Usage:      "mr edit <owner/name> <n> [--title <t>] [--body <b> | --file -] [--format md|org]",
+		Usage:      "mr edit <owner/name> <n> [--title <t>] [--body <b> | --file -] [--format md|org] [--superseded-by <m>|none]",
 		ReadsStdin: true, Run: runMREdit})
 	register(Command{Path: []string{"mr", "retarget"},
 		Summary: "retarget onto another branch",
 		Usage:   "mr merge <owner/name> <n> [--strategy ff|merge|squash|rebase]", Run: runMRMerge})
 	register(Command{Path: []string{"mr", "close"},
 		Summary: "close without merging",
-		Usage:   "mr close <owner/name> <n>", Run: runMRClose})
+		Usage:   "mr close <owner/name> <n> [--by <m>]", Run: runMRClose})
 }
 // ForkOut is what `repo fork` emits: where the fork landed, and what it
 	MergedBy  string     `json:"merged_by,omitempty"`
 	ClosedAt  string     `json:"closed_at,omitempty"`
 	ClosedBy  string     `json:"closed_by,omitempty"`
+	// SupersededBy is the merge request, by number, this one was closed
+	// in favour of. 0 means none.
+	SupersededBy int64 `json:"superseded_by,omitempty"`
 }
 type stackRef struct {
 		Source: src, TargetRef: m.TargetRef, HeadSHA: m.HeadSHA, Milestone: m.Milestone,
 		ReviewRequests: m.ReviewRequests,
 		CreatedAt:      m.CreatedAt, MergedAt: m.MergedAt, MergedBy: m.MergedBy,
-		ClosedAt: m.ClosedAt, ClosedBy: m.ClosedBy}
+		ClosedAt: m.ClosedAt, ClosedBy: m.ClosedBy, SupersededBy: m.SupersededBy}
 	if withBody {
 		o.Body = m.Body
 		o.BodyFormat = m.BodyFormat
 		if d.ClosedAt != "" {
 			fmt.Fprintf(w, "closed %s%s\n", d.ClosedAt, byWhom(d.ClosedBy))
 		}
+		if d.SupersededBy != 0 {
+			fmt.Fprintf(w, "superseded by: !%d\n", d.SupersededBy)
+		}
 		if d.Body != "" {
 			fmt.Fprintf(w, "\n%s\n", d.Body)
 		}
 }
 func runMREdit(c *Ctx, args []string) int {
-	rest, title, body, format, code := editText(c, args, "mr")
+	rest, title, body, format, fl, code := editText(c, args, "mr", "--superseded-by")
 	if code >= 0 {
 		return code
 	}
 	if code := authorOrWrite(c, repo, mr.Author, "edit this merge request"); code >= 0 {
 		return code
 	}
+	var clearSuperseded bool
+	var supersededBy int64
+	if fl.Has("--superseded-by") {
+		if mr.State != "closed" {
+			return c.fail(protocol.ExitUsage, "only a closed merge request can be superseded")
+		}
+		if v := fl.Value("--superseded-by"); v == "none" {
+			clearSuperseded = true
+		} else {
+			supersededBy, code = resolveSupersededBy(c, repo, mr.Number, v)
+			if code >= 0 {
+				return code
+			}
+		}
+	}
 	if err := c.Store.UpdateMRText(mr.ID, title, body, format); err != nil {
 		return c.fail(protocol.ExitFailure, "%v", err)
 	}
+	if clearSuperseded {
+		if err := c.Store.SetSupersededBy(mr.ID, 0); err != nil {
+			return c.fail(protocol.ExitFailure, "%v", err)
+		}
+	} else if supersededBy != 0 {
+		if err := c.Store.SetSupersededBy(mr.ID, supersededBy); err != nil {
+			return c.fail(protocol.ExitFailure, "%v", err)
+		}
+	}
 	c.Store.RecordEvent(repo.ID, c.User.ID, "mr.edited", fmt.Sprintf(`{"number":%d}`, mr.Number))
 	return c.emit(map[string]any{"number": mr.Number}, func(w io.Writer) {
 		fmt.Fprintf(w, "edited %s!%d\n", repo.Path(), mr.Number)
 }
 func runMRClose(c *Ctx, args []string) int {
-	repo, mr, code := mrRef(c, args, policy.CanRead)
+	f, err := parseFlags(args, flagSpec{Values: []string{"--by"}, MaxPos: 2,
+		Usage: "mr close <owner/name> <n> [--by <m>]"})
+	if err != nil {
+		return c.fail(protocol.ExitUsage, "%v", err)
+	}
+	repo, mr, code := mrRef(c, f.Pos, policy.CanRead)
 	if code >= 0 {
 		return code
 	}
 	if code := refuseArchived(c, repo); code >= 0 {
 		return code
 	}
-	if len(args) != 2 {
+	if len(f.Pos) != 2 {
 		return c.usage()
 	}
 	if code := authorOrWrite(c, repo, mr.Author, "close this merge request"); code >= 0 {
 	if mr.State == "merged" || mr.State == "closed" {
 		return c.fail(protocol.ExitUsage, "MR !%d is already %s", mr.Number, mr.State)
 	}
+	var by int64
+	if f.Has("--by") {
+		by, code = resolveSupersededBy(c, repo, mr.Number, f.Value("--by"))
+		if code >= 0 {
+			return code
+		}
+	}
 	if err := c.Store.MarkClosed(mr.ID, c.User.ID, ""); err != nil {
 		return c.fail(protocol.ExitFailure, "%v", err)
 	}
-	c.Store.RecordEvent(repo.ID, c.User.ID, "mr.closed", fmt.Sprintf(`{"number":%d}`, mr.Number))
+	eventData := fmt.Sprintf(`{"number":%d}`, mr.Number)
+	if by != 0 {
+		if err := c.Store.SetSupersededBy(mr.ID, by); err != nil {
+			return c.fail(protocol.ExitFailure, "%v", err)
+		}
+		eventData = fmt.Sprintf(`{"number":%d,"by":%d}`, mr.Number, by)
+	}
+	c.Store.RecordEvent(repo.ID, c.User.ID, "mr.closed", eventData)
 	if parts, err := c.Store.MRParticipants(mr.ID); err == nil {
 		notify(c, parts, notice{repo: repo, kind: "mr",
 			subject: mrSubject(repo, mr.Number, mr.Title),
 	})
 }
+// resolveSupersededBy validates a --superseded-by/--by value against the
+// merge request it would be set on: it must parse, name another merge
+// request in the same repository (never itself), and that request must
+// exist. -1 as the returned code means the value is good to use.
+func resolveSupersededBy(c *Ctx, repo store.Repo, number int64, v string) (int64, int) {
+	m, err := strconv.ParseInt(v, 10, 64)
+	if err != nil {
+		return 0, c.fail(protocol.ExitUsage, "bad MR number %q", v)
+	}
+	if m == number {
+		return 0, c.fail(protocol.ExitUsage, "a merge request cannot supersede itself")
+	}
+	if _, err := c.Store.MRByNumber(repo.ID, m); errors.Is(err, store.ErrNotFound) {
+		return 0, c.fail(protocol.ExitNotFound, "no merge request !%d on %s", m, repo.Path())
+	} else if err != nil {
+		return 0, c.fail(protocol.ExitFailure, "%v", err)
+	}
+	return m, -1
+}
 // reviewAction is what a review notification says it was. A verdict with
 // a batch behind it is a different thing from a bare verdict, and the
 // person reading the mail is deciding whether to open it.

internal/control/mr_test.go added +179

@@ -0,0 +1,179 @@
	1	package control
	2
	3	import (
	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.
	15	func 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.
	23	func 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
	36	func 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.
	53	func 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).
	65	func 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.
	81	func 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.
	95	func 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.
	109	func 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.
	133	func 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.
	151	func 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.
	169	func 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	}

internal/httpd/mractions.go +9 −1

	@@ -76,7 +76,15 @@ func (s Server) mrMergeSubmit(w http.ResponseWriter, r http.Request, u store.U
76	}	76	}
77		77
78	func (s Server) mrCloseSubmit(w http.ResponseWriter, r http.Request, u store.User) {	78	func (s Server) mrCloseSubmit(w http.ResponseWriter, r http.Request, u store.User) {
79	_, msg, code := s.runControlCode(u, mrArgs(r, "close"))	79	args := []string{}
		80	if by := strings.TrimSpace(r.FormValue("by")); by != "" {
		81	if _, err := strconv.ParseInt(by, 10, 64); err != nil {
		82	s.mrRedirect(w, r, "the superseding request is a number")
		83	return
		84	}
		85	args = append(args, "--by", by)
		86	}
		87	_, msg, code := s.runControlCode(u, mrArgs(r, "close", args...))
80	s.done(w, r, code, msg, s.mrRedirect)	88	s.done(w, r, code, msg, s.mrRedirect)
81	}	89	}
82		90

internal/httpd/web.go +5 −1

 			stacked, _ = s.st.OpenMRsByTarget(p.Repo.ID, m.SourceRef)
 		}
 	}
+	// The merge requests this one superseded when it was closed, so the
+	// page it points to can also say what it supersedes.
+	supersedes, _ := s.st.MRsSuperseding(p.Repo.ID, m.Number)
 	s.render(w, "mr.html", struct {
 		repoPage
 		MR              store.MR
 		DetachedThreads []diffThread
 		StackedOn       *store.MR
 		Stacked         []store.MR
+		Supersedes      []store.MR
 		Gates           *control.GatesOut
 		SourceGone      bool
 		HeadMerged      bool
 		Base            string
 	}{p, m, view, md(m.Body, m.BodyFormat), checks, combined, renderComments(comments, md),
 		reviewRows, files, diffTruncated, stat, commits, commitsTotal, branches, s.canEditItem(r, p.Repo, m.Author),
-		canWrite, unresolved, revisions, s.takeFlash(w, r), detachedThreads, stackedOn, stacked, gates,
+		canWrite, unresolved, revisions, s.takeFlash(w, r), detachedThreads, stackedOn, stacked, supersedes, gates,
 		sourceGone(p, m), headMerged, headPruned, base})
 }

internal/store/migrations/0055_mr_superseded_by.down.sql added +1

	@@ -0,0 +1 @@
		1	ALTER TABLE merge_requests DROP COLUMN superseded_by;

internal/store/migrations/0055_mr_superseded_by.up.sql added +3

@@ -0,0 +1,3 @@
	1	-- Which merge request, by number within the same repository, a closed
	2	-- merge request was closed in favour of. NULL means none (#223).
	3	ALTER TABLE merge_requests ADD COLUMN superseded_by INTEGER;

internal/store/mrs.go +38 −4

	@@ -29,8 +29,11 @@ type MR struct {
29	MergedBy string // "" when unknown (imports) or the account is gone	29	MergedBy string // "" when unknown (imports) or the account is gone
30	ClosedAt string // "" unless closed without merging	30	ClosedAt string // "" unless closed without merging
31	ClosedBy string	31	ClosedBy string
32	CreatedAt string	32	// SupersededBy is the number, within this repository, of the merge
33	UpdatedAt string	33	// request this one was closed in favour of. 0 means none.
		34	SupersededBy int64
		35	CreatedAt string
		36	UpdatedAt string
34	// ReviewRequests is who has been asked, directly, for a review — the	37	// ReviewRequests is who has been asked, directly, for a review — the
35	// mr review request counterpart of Issue.Assignees.	38	// mr review request counterpart of Issue.Assignees.
36	ReviewRequests []string	39	ReviewRequests []string
	@@ -92,7 +95,7 @@ const mrSelect = `
92	m.source_ref, m.target_ref, m.title, m.body, m.body_format, m.state, m.draft,	95	m.source_ref, m.target_ref, m.title, m.body, m.body_format, m.state, m.draft,
93	COALESCE(ms.title, ''), m.head_sha,	96	COALESCE(ms.title, ''), m.head_sha,
94	m.merged_base, m.merged_at, COALESCE(mu.username, ''),	97	m.merged_base, m.merged_at, COALESCE(mu.username, ''),
95	m.closed_at, COALESCE(cu.username, ''), m.created_at, m.updated_at	98	m.closed_at, COALESCE(cu.username, ''), COALESCE(m.superseded_by, 0), m.created_at, m.updated_at
96	FROM merge_requests m	99	FROM merge_requests m
97	JOIN users u ON u.id = m.author_id	100	JOIN users u ON u.id = m.author_id
98	LEFT JOIN users mu ON mu.id = m.merged_by	101	LEFT JOIN users mu ON mu.id = m.merged_by
	@@ -106,7 +109,7 @@ func scanMR(row interface{ Scan(...any) error }) (MR, error) {
106	var m MR	109	var m MR
107	err := row.Scan(&m.ID, &m.RepoID, &m.Number, &m.Author, &m.SourceRepoID, &m.SourcePath,	110	err := row.Scan(&m.ID, &m.RepoID, &m.Number, &m.Author, &m.SourceRepoID, &m.SourcePath,
108	&m.SourceRef, &m.TargetRef, &m.Title, &m.Body, &m.BodyFormat, &m.State, &m.Draft, &m.Milestone, &m.HeadSHA, &m.MergedBase,	111	&m.SourceRef, &m.TargetRef, &m.Title, &m.Body, &m.BodyFormat, &m.State, &m.Draft, &m.Milestone, &m.HeadSHA, &m.MergedBase,
109	&m.MergedAt, &m.MergedBy, &m.ClosedAt, &m.ClosedBy, &m.CreatedAt, &m.UpdatedAt)	112	&m.MergedAt, &m.MergedBy, &m.ClosedAt, &m.ClosedBy, &m.SupersededBy, &m.CreatedAt, &m.UpdatedAt)
110	return m, err	113	return m, err
111	}	114	}
112		115
	@@ -265,6 +268,37 @@ func (s *Store) MarkClosed(mrID, actorID int64, at string) error {
265	return err	268	return err
266	}	269	}
267		270
		271	// SetSupersededBy records which merge request, by number within the same
		272	// repository, this one was closed in favour of. n of 0 clears it.
		273	func (s *Store) SetSupersededBy(mrID, n int64) error {
		274	var v any
		275	if n != 0 {
		276	v = n
		277	}
		278	_, err := s.DB.Exec("UPDATE merge_requests SET superseded_by = ? WHERE id = ?", v, mrID)
		279	return err
		280	}
		281
		282	// MRsSuperseding returns the merge requests in a repository whose
		283	// superseded_by names number, oldest first — the reverse of
		284	// MR.SupersededBy.
		285	func (s *Store) MRsSuperseding(repoID, number int64) ([]MR, error) {
		286	rows, err := s.DB.Query(mrSelect+" WHERE m.repo_id = ? AND m.superseded_by = ? ORDER BY m.number ASC", repoID, number)
		287	if err != nil {
		288	return nil, err
		289	}
		290	defer rows.Close()
		291	var out []MR
		292	for rows.Next() {
		293	m, err := scanMR(rows)
		294	if err != nil {
		295	return nil, err
		296	}
		297	out = append(out, m)
		298	}
		299	return out, rows.Err()
		300	}
		301
268	// SetMRState moves an MR between states that carry no resolution stamp.	302	// SetMRState moves an MR between states that carry no resolution stamp.
269	// Returning to open (a source branch that came back) clears one.	303	// Returning to open (a source branch that came back) clears one.
270	func (s *Store) SetMRState(mrID int64, state string) error {	304	func (s *Store) SetMRState(mrID int64, state string) error {

internal/store/mrs_test.go +42

	@@ -59,6 +59,48 @@ func TestResolutionStamps(t *testing.T) {
59	}	59	}
60	}	60	}
61		61
		62	// A merge request closed without merging can record the request that
		63	// carried its change forward; MRsSuperseding is the reverse lookup, and
		64	// 0 clears the field (#223).
		65	func TestSupersededBy(t *testing.T) {
		66	s, repoID, uid := mrFixture(t)
		67	if _, err := s.CreateMR(repoID, uid, repoID, "feature2", "main", "t2", "", "def456", "md", false); err != nil {
		68	t.Fatal(err)
		69	}
		70	mr1, _ := s.MRByNumber(repoID, 1)
		71	if err := s.MarkClosed(mr1.ID, uid, ""); err != nil {
		72	t.Fatal(err)
		73	}
		74	if err := s.SetSupersededBy(mr1.ID, 2); err != nil {
		75	t.Fatal(err)
		76	}
		77	mr1, _ = s.MRByNumber(repoID, 1)
		78	if mr1.SupersededBy != 2 {
		79	t.Fatalf("SupersededBy = %d, want 2", mr1.SupersededBy)
		80	}
		81	superseding, err := s.MRsSuperseding(repoID, 2)
		82	if err != nil {
		83	t.Fatal(err)
		84	}
		85	if len(superseding) != 1 \|\| superseding[0].Number != 1 {
		86	t.Fatalf("MRsSuperseding(repoID, 2) = %+v", superseding)
		87	}
		88	if err := s.SetSupersededBy(mr1.ID, 0); err != nil {
		89	t.Fatal(err)
		90	}
		91	mr1, _ = s.MRByNumber(repoID, 1)
		92	if mr1.SupersededBy != 0 {
		93	t.Fatalf("SupersededBy after clear = %d, want 0", mr1.SupersededBy)
		94	}
		95	superseding, err = s.MRsSuperseding(repoID, 2)
		96	if err != nil {
		97	t.Fatal(err)
		98	}
		99	if len(superseding) != 0 {
		100	t.Fatalf("MRsSuperseding(repoID, 2) after clear = %+v", superseding)
		101	}
		102	}
		103
62	// An import carries the upstream time but no local account for the actor.	104	// An import carries the upstream time but no local account for the actor.
63	func TestResolutionStampImported(t *testing.T) {	105	func TestResolutionStampImported(t *testing.T) {
64	s, repoID, _ := mrFixture(t)	106	s, repoID, _ := mrFixture(t)

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

 <h1 class="issuetitle">{{.MR.Title}} <span class="issuenumber">!{{.MR.Number}}</span></h1>
 <p class="issuemeta"><span class="chip chip-{{.MR.State}}">{{if eq .MR.State "source_gone"}}source gone{{else}}{{.MR.State}}{{end}}</span>{{if .MR.Draft}} <span class="chip chip-neutral">draft</span>{{end}}
   {{if and (eq .MR.State "merged") .MR.MergedBy}}merged by <a href="/{{.MR.MergedBy}}">{{.MR.MergedBy}}</a>{{with .MR.MergedAt}} on {{when .}}{{end}}
-  {{else if and (eq .MR.State "closed") .MR.ClosedBy}}closed without merging by <a href="/{{.MR.ClosedBy}}">{{.MR.ClosedBy}}</a>{{with .MR.ClosedAt}} on {{when .}}{{end}}
+  {{else if and (eq .MR.State "closed") .MR.ClosedBy}}closed without merging by <a href="/{{.MR.ClosedBy}}">{{.MR.ClosedBy}}</a>{{with .MR.ClosedAt}} on {{when .}}{{end}}{{if .MR.SupersededBy}}, in favour of <a href="/{{.Repo.OwnerName}}/{{.Repo.Name}}/mrs/{{.MR.SupersededBy}}">!{{.MR.SupersededBy}}</a>{{end}}
   {{else if or (eq .MR.State "merged") (eq .MR.State "closed")}}{{if eq .MR.State "merged"}}merged{{else}}closed without merging{{end}}
   {{else}}opened by <a href="/{{.MR.Author}}">{{.MR.Author}}</a> on {{when .MR.CreatedAt}}{{end}}
   · <code>{{if .MR.SourcePath}}{{.MR.SourcePath}}:{{end}}{{.MR.SourceRef}}</code> into <code>{{.MR.TargetRef}}</code></p>
 {{with field . "StackedOn"}}<p class="meta">Stacked on <a href="/{{$.Repo.OwnerName}}/{{$.Repo.Name}}/mrs/{{.Number}}">!{{.Number}} {{.Title}}</a>: merges into its branch until that lands, then onto its target.</p>{{end}}
 {{with field . "Stacked"}}<p class="meta">Builds on this: {{range $i, $k := .}}{{if $i}}, {{end}}<a href="/{{$.Repo.OwnerName}}/{{$.Repo.Name}}/mrs/{{$k.Number}}">!{{$k.Number}} {{$k.Title}}</a>{{end}}. Merging with squash or rebase is refused while they are open.</p>{{end}}
+{{with field . "Supersedes"}}<p class="meta">supersedes {{range $i, $m := .}}{{if $i}}, {{end}}<a href="/{{$.Repo.OwnerName}}/{{$.Repo.Name}}/mrs/{{$m.Number}}">!{{$m.Number}}</a>{{end}}</p>{{end}}
 {{if .Notice}}<p class="error" role="alert">{{.Notice}}</p>{{end}}
       <button type="submit">Merge</button>
     </form>
     <form method="post" action="{{$base}}/close" class="actions">
+      <label class="vh" for="by">Closed in favour of</label>
+      <input type="text" id="by" name="by" inputmode="numeric" size="4" placeholder="!N">
       <button type="submit" class="danger">Close without merging</button>
     </form>
     <form method="post" action="{{$base}}/draft" class="actions">