Commit `39535a8e16`

39535a8e16d8dfc0189bce59511cfb0d7d019fb5

parent: c3d4ae8f26

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

cmc <hello@cleberg.net> · 2026-09-02 02:46 UTC

mr: stacked merge requests

Targeting another open merge request's source branch always worked;
what broke was the moment underneath it merged, when the upper one
kept targeting a branch whose commits had already landed.

A merge request whose target is the source branch of an open merge
request in the same repository is stacked on it. Nothing is stored: two
reads derive it. mr create says "stacked on !A" when it applies; mr
show carries stacked_on and stacked; mr list rows carry stacked_on; the
page says both directions in the header.

When the lower one merges, everything stacked on it is retargeted onto
what it merged into, with a system comment naming the merge and its
reviews kept: after a fast-forward or merge commit the diff against the
new target is the diff they were of. A squash or rebase merge under a
stack is refused, naming the stack, since it would rewrite the commits
the stack builds on.

Closes #87

Layout: unified · split

e2e/stack_test.go added +157

		@@ -0,0 +1,157 @@
	1	package e2e
	2
	3	import (
	4	"encoding/json"
	5	"os"
	6	"path/filepath"
	7	"strings"
	8	"testing"
	9	)
	10
	11	// A three-deep stack, merged bottom-up. Each merge moves the stack above
	12	// it onto the merged target with its reviews intact; a squash under a
	13	// stack is refused.
	14	func TestStackedMergeRequests(t *testing.T) {
	15	inst := startInstanceWith(t, "[web]\nmode = \"accounts\"\n")
	16	aliceKey := inst.newKey(t, "alice")
	17	bobKey := inst.newKey(t, "bob")
	18	inst.admin(t, "admin", "user", "create", "alice", "--key", aliceKey+".pub", "--email", "alice@example.test", "--verified")
	19	inst.admin(t, "admin", "user", "create", "bob", "--key", bobKey+".pub", "--email", "bob@example.test", "--verified")
	20	if _, errOut, code := inst.ssh(t, aliceKey, "", "repo", "create", "alice/lib"); code != 0 {
	21	t.Fatalf("repo create: %s", errOut)
	22	}
	23	if _, _, code := inst.ssh(t, aliceKey, "", "repo", "access", "grant", "alice/lib", "bob", "write"); code != 0 {
	24	t.Fatal("grant failed")
	25	}
	26	work := t.TempDir()
	27	env := inst.gitEnv(aliceKey)
	28	mustGit(t, work, env, "clone", inst.sshURL("alice/lib"), "w")
	29	dir := filepath.Join(work, "w")
	30	commit := func(branch, from, file string) {
	31	t.Helper()
	32	if from == "" {
	33	mustGit(t, dir, env, "checkout", "-q", "-b", branch)
	34	} else {
	35	mustGit(t, dir, env, "checkout", "-q", "-b", branch, from)
	36	}
	37	os.WriteFile(filepath.Join(dir, file), []byte(file+"\n"), 0o644)
	38	mustGit(t, dir, env, "add", ".")
	39	mustGit(t, dir, env, "commit", "-q", "-m", file)
	40	mustGit(t, dir, env, "push", "-q", "origin", branch)
	41	}
	42	commit("main", "", "base.txt")
	43	commit("feat-a", "main", "a.txt")
	44	commit("feat-b", "feat-a", "b.txt")
	45	commit("feat-c", "feat-b", "c.txt")
	46
	47	create := func(src, dst, title string) string {
	48	t.Helper()
	49	out, errOut, code := inst.ssh(t, aliceKey, "", "mr", "create", "alice/lib", "--source", src, "--target", dst, "--title", title, "--json")
	50	if code != 0 {
	51	t.Fatalf("mr create %s: %s", src, errOut)
	52	}
	53	return out
	54	}
	55	if out := create("feat-a", "main", "A"); strings.Contains(out, "stacked_on") {
	56	t.Fatalf("A is not stacked:\n%s", out)
	57	}
	58	if out := create("feat-b", "feat-a", "B"); !strings.Contains(out, `"stacked_on":{"number":1`) {
	59	t.Fatalf("B create does not report its stack:\n%s", out)
	60	}
	61	create("feat-c", "feat-b", "C")
	62
	63	type stackShow struct {
	64	mrShow
	65	StackedOn *struct {
	66	Number int64 `json:"number"`
	67	} `json:"stacked_on"`
	68	Stacked []struct {
	69	Number int64 `json:"number"`
	70	} `json:"stacked"`
	71	Comments []struct {
	72	Body string `json:"body"`
	73	} `json:"comments"`
	74	}
	75	show := func(n string) stackShow {
	76	t.Helper()
	77	out, errOut, code := inst.ssh(t, aliceKey, "", "mr", "show", "alice/lib", n, "--json")
	78	if code != 0 {
	79	t.Fatalf("mr show %s: %s", n, errOut)
	80	}
	81	var env struct {
	82	Data stackShow `json:"data"`
	83	}
	84	if err := json.Unmarshal([]byte(out), &env); err != nil {
	85	t.Fatalf("show: %v\n%s", err, out)
	86	}
	87	return env.Data
	88	}
	89	a, b, c := show("1"), show("2"), show("3")
	90	if a.StackedOn != nil \|\| len(a.Stacked) != 1 \|\| a.Stacked[0].Number != 2 {
	91	t.Fatalf("A stack: %+v", a)
	92	}
	93	if b.StackedOn == nil \|\| b.StackedOn.Number != 1 \|\| len(b.Stacked) != 1 \|\| b.Stacked[0].Number != 3 {
	94	t.Fatalf("B stack: %+v", b)
	95	}
	96	if c.StackedOn == nil \|\| c.StackedOn.Number != 2 \|\| len(c.Stacked) != 0 {
	97	t.Fatalf("C stack: %+v", c)
	98	}
	99	if out, _, _ := inst.ssh(t, aliceKey, "", "mr", "list", "alice/lib"); !strings.Contains(out, "stacked on !1") \|\| !strings.Contains(out, "stacked on !2") {
	100	t.Fatalf("mr list lacks the stack:\n%s", out)
	101	}
	102
	103	// Bob approves B; the approval must survive the retarget.
	104	if _, errOut, code := inst.ssh(t, bobKey, "", "mr", "review", "alice/lib", "2", "--approve"); code != 0 {
	105	t.Fatalf("review: %s", errOut)
	106	}
	107
	108	// A squash under the stack is refused, naming the stack.
	109	if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "merge", "alice/lib", "1", "--strategy", "squash"); code != 2 \|\| !strings.Contains(errOut, "!2") {
	110	t.Fatalf("squash under a stack: exit %d %s", code, errOut)
	111	}
	112	// Merge A: B moves onto main, C stays on feat-b.
	113	if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "merge", "alice/lib", "1"); code != 0 {
	114	t.Fatalf("merge A: %s", errOut)
	115	}
	116	b = show("2")
	117	if b.TargetRef != "main" \|\| b.StackedOn != nil \|\| len(b.Reviews) != 1 \|\| b.Reviews[0].Stale {
	118	t.Fatalf("B after A merged: %+v", b)
	119	}
	120	found := false
	121	for _, cm := range b.Comments {
	122	if strings.Contains(cm.Body, "retargeted from feat-a to main: !1 merged") {
	123	found = true
	124	}
	125	}
	126	if !found {
	127	t.Fatalf("B carries no retarget comment: %+v", b.Comments)
	128	}
	129	if c = show("3"); c.TargetRef != "feat-b" \|\| c.StackedOn == nil \|\| c.StackedOn.Number != 2 {
	130	t.Fatalf("C after A merged: %+v", c)
	131	}
	132	// The web page says both directions.
	133	root := inst.login(t, aliceKey)
	134	if _, body := browserGet(t, root, inst.base()+"/alice/lib/mrs/2"); !strings.Contains(body, "Builds on this") \|\| !strings.Contains(body, "/alice/lib/mrs/3") {
	135	t.Fatalf("B page lacks its stack:\n%s", body)
	136	}
	137	if _, body := browserGet(t, root, inst.base()+"/alice/lib/mrs/3"); !strings.Contains(body, "Stacked on") \|\| !strings.Contains(body, "/alice/lib/mrs/2") {
	138	t.Fatalf("C page lacks its parent:\n%s", body)
	139	}
	140	// Merge B, then C: each lands its own commit only.
	141	if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "merge", "alice/lib", "2"); code != 0 {
	142	t.Fatalf("merge B: %s", errOut)
	143	}
	144	if c = show("3"); c.TargetRef != "main" \|\| c.StackedOn != nil {
	145	t.Fatalf("C after B merged: %+v", c)
	146	}
	147	if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "merge", "alice/lib", "3"); code != 0 {
	148	t.Fatalf("merge C: %s", errOut)
	149	}
	150	mustGit(t, dir, env, "checkout", "-q", "main")
	151	mustGit(t, dir, env, "pull", "-q", "origin", "main")
	152	for _, f := range []string{"a.txt", "b.txt", "c.txt"} {
	153	if _, err := os.Stat(filepath.Join(dir, f)); err != nil {
	154	t.Fatalf("%s missing from main after the stack merged", f)
	155	}
	156	}
	157	}

internal/control/mr.go +97 −8

		@@ -316,8 +316,17 @@ func runMRCreate(c *Ctx, args []string) int {
316	316	notifyUsers(c, targets, mrSubject(repo, n, title),
317	317	notifyBody(c, fmt.Sprintf("opened merge request !%d (%s -> %s)", n, source, target), b, fmt.Sprintf("%s/mrs/%d", repo.Path(), n)))
318	318	}
319		return c.emit(map[string]any{"number": n, "head_sha": headSHA}, func(w io.Writer) {
	319	out := map[string]any{"number": n, "head_sha": headSHA}
	320	var parent *stackRef
	321	if p, ok, err := c.Store.OpenMRBySource(repo.ID, target); err == nil && ok {
	322	parent = &stackRef{p.Number, p.Title}
	323	out["stacked_on"] = parent
	324	}
	325	return c.emit(out, func(w io.Writer) {
320	326	fmt.Fprintf(w, "created %s!%d (%s -> %s)\n", repo.Path(), n, source, target)
	327	if parent != nil {
	328	fmt.Fprintf(w, "stacked on !%d %s\n", parent.Number, parent.Title)
	329	}
321	330	})
322	331	}
323	332
		@@ -332,11 +341,45 @@ type mrOut struct {
332	341	Body string `json:"body,omitempty"`
333	342	BodyFormat string `json:"body_format,omitempty"`
334	343	Milestone string `json:"milestone,omitempty"`
335		CreatedAt string `json:"created_at"`
336		MergedAt string `json:"merged_at,omitempty"`
337		MergedBy string `json:"merged_by,omitempty"`
338		ClosedAt string `json:"closed_at,omitempty"`
339		ClosedBy string `json:"closed_by,omitempty"`
	344	// StackedOn is the open merge request whose source branch this one
	345	// targets; Stacked are the open ones targeting this one's source.
	346	StackedOn *stackRef `json:"stacked_on,omitempty"`
	347	Stacked []stackRef `json:"stacked,omitempty"`
	348	CreatedAt string `json:"created_at"`
	349	MergedAt string `json:"merged_at,omitempty"`
	350	MergedBy string `json:"merged_by,omitempty"`
	351	ClosedAt string `json:"closed_at,omitempty"`
	352	ClosedBy string `json:"closed_by,omitempty"`
	353	}
	354
	355	type stackRef struct {
	356	Number int64 `json:"number"`
	357	Title string `json:"title"`
	358	}
	359
	360	// stackOf derives the stack around m: the open merge request whose source
	361	// branch m targets, and the open ones targeting m's source. Both only
	362	// within m's repository; a fork's branch is not a target anything can
	363	// stack on.
	364	func stackOf(c Ctx, repo store.Repo, m store.MR) (stackRef, []stackRef) {
	365	if m.State != "open" {
	366	return nil, nil
	367	}
	368	var parent *stackRef
	369	if p, ok, err := c.Store.OpenMRBySource(repo.ID, m.TargetRef); err == nil && ok && p.ID != m.ID {
	370	parent = &stackRef{p.Number, p.Title}
	371	}
	372	var children []stackRef
	373	if m.SourceRepoID == repo.ID {
	374	if kids, err := c.Store.OpenMRsByTarget(repo.ID, m.SourceRef); err == nil {
	375	for _, k := range kids {
	376	if k.ID != m.ID {
	377	children = append(children, stackRef{k.Number, k.Title})
	378	}
	379	}
	380	}
	381	}
	382	return parent, children
340	383	}
341	384
342	385	func mrToOut(repo store.Repo, m store.MR, withBody bool) mrOut {
		@@ -398,11 +441,17 @@ func runMRList(c *Ctx, args []string) int {
398	441	})
399	442	var ds []mrOut
400	443	for _, m := range mrs {
401		ds = append(ds, mrToOut(repo, m, false))
	444	o := mrToOut(repo, m, false)
	445	o.StackedOn, _ = stackOf(c, repo, m)
	446	ds = append(ds, o)
402	447	}
403	448	return c.emitPage(p, ds, next, func(w io.Writer) {
404	449	for _, d := range ds {
405		fmt.Fprintf(w, "!%d\t%s\t%s\t%s -> %s\n", d.Number, d.State, d.Title, d.Source, d.TargetRef)
	450	stacked := ""
	451	if d.StackedOn != nil {
	452	stacked = fmt.Sprintf("\tstacked on !%d", d.StackedOn.Number)
	453	}
	454	fmt.Fprintf(w, "!%d\t%s\t%s\t%s -> %s%s\n", d.Number, d.State, d.Title, d.Source, d.TargetRef, stacked)
406	455	}
407	456	})
408	457	}
		@@ -510,8 +559,15 @@ func runMRShow(c *Ctx, args []string) int {
510	559	Comments []commentOut `json:"comments,omitempty"`
511	560	Reviews []reviewOut `json:"reviews,omitempty"`
512	561	}{mrToOut(repo, mr, true), checks, combined, unresolved, commits, cs, rs}
	562	d.StackedOn, d.Stacked = stackOf(c, repo, mr)
513	563	return c.emit(d, func(w io.Writer) {
514	564	fmt.Fprintf(w, "!%d %s [%s] by %s\n%s -> %s @ %.10s\n", d.Number, d.Title, d.State, d.Author, d.Source, d.TargetRef, d.HeadSHA)
	565	if d.StackedOn != nil {
	566	fmt.Fprintf(w, "stacked on !%d %s\n", d.StackedOn.Number, d.StackedOn.Title)
	567	}
	568	for _, k := range d.Stacked {
	569	fmt.Fprintf(w, "stacked: !%d %s\n", k.Number, k.Title)
	570	}
515	571	if d.MergedAt != "" {
516	572	fmt.Fprintf(w, "merged %s%s\n", d.MergedAt, byWhom(d.MergedBy))
517	573	}
		@@ -1007,6 +1063,25 @@ func runMRMerge(c *Ctx, args []string) int {
1007	1063	newSHA = onto
1008	1064	}
1009	1065
	1066	// A stacked merge request's diff is against this branch. After a
	1067	// fast-forward or merge commit the same commits are on the target and
	1068	// its diff is unchanged there; after a squash or rebase they are not,
	1069	// and the stack would carry this merge request's changes a second
	1070	// time. Refuse rather than leave the stack wrong.
	1071	var stack []store.MR
	1072	if mr.SourceRepoID == repo.ID {
	1073	stack, _ = c.Store.OpenMRsByTarget(repo.ID, mr.SourceRef)
	1074	}
	1075	if len(stack) > 0 && (strategy == "squash" \|\| strategy == "rebase") {
	1076	var nums []string
	1077	for _, k := range stack {
	1078	nums = append(nums, fmt.Sprintf("!%d", k.Number))
	1079	}
	1080	return c.fail(protocol.ExitUsage,
	1081	"%s is stacked on by %s; a %s merge rewrites the commits they build on. Merge with --strategy ff or merge, or merge the stack into %s first",
	1082	fmt.Sprintf("!%d", mr.Number), strings.Join(nums, ", "), strategy, mr.SourceRef)
	1083	}
	1084
1010	1085	// CAS so a concurrent push between our read and this write fails the
1011	1086	// merge instead of silently discarding the push.
1012	1087	if err := gitutil.UpdateRefCAS(dir, targetRef, newSHA, targetSHA); err != nil {
		@@ -1016,6 +1091,20 @@ func runMRMerge(c *Ctx, args []string) int {
1016	1091	return c.fail(protocol.ExitFailure, "%v", err)
1017	1092	}
1018	1093	c.Store.RecordEvent(repo.ID, c.User.ID, "mr.merged", fmt.Sprintf(`{"number":%d,"sha":%q}`, mr.Number, newSHA))
	1094	// The stack moves up: whatever targeted this branch now targets what
	1095	// it merged into, reviews intact, since that diff is the one they
	1096	// were of.
	1097	for _, k := range stack {
	1098	if err := c.Store.RetargetKeepingReviews(k.ID, mr.TargetRef); err != nil {
	1099	continue
	1100	}
	1101	c.Store.AddMRSystemComment(k.ID, c.User.ID, fmt.Sprintf("retargeted from %s to %s: !%d merged", mr.SourceRef, mr.TargetRef, mr.Number))
	1102	if parts, err := c.Store.MRParticipants(k.ID); err == nil {
	1103	notifyUsers(c, parts, mrSubject(repo, k.Number, k.Title),
	1104	notifyBody(c, fmt.Sprintf("retargeted !%d from %s to %s: !%d merged", k.Number, mr.SourceRef, mr.TargetRef, mr.Number), "",
	1105	fmt.Sprintf("%s/mrs/%d", repo.Path(), k.Number)))
	1106	}
	1107	}
1019	1108	// Merges bypass receive-pack, so the commit-message issue actions
1020	1109	// (closes #N, references) run here for the newly landed commits. The
1021	1110	// description is scanned after them, so a commit wins the attribution

internal/httpd/web.go +14 −1

		@@ -1691,6 +1691,17 @@ func (s Server) mr(w http.ResponseWriter, r http.Request) {
1691	1691	if view != "commits" && view != "diff" {
1692	1692	view = "conversation"
1693	1693	}
	1694	// The stack around an open merge request, for the header.
	1695	var stackedOn *store.MR
	1696	var stacked []store.MR
	1697	if m.State == "open" {
	1698	if parent, ok, err := s.st.OpenMRBySource(p.Repo.ID, m.TargetRef); err == nil && ok && parent.ID != m.ID {
	1699	stackedOn = &parent
	1700	}
	1701	if m.SourceRepoID == p.Repo.ID {
	1702	stacked, _ = s.st.OpenMRsByTarget(p.Repo.ID, m.SourceRef)
	1703	}
	1704	}
1694	1705	s.render(w, "mr.html", struct {
1695	1706	repoPage
1696	1707	MR store.MR
		@@ -1709,9 +1720,11 @@ func (s Server) mr(w http.ResponseWriter, r http.Request) {
1709	1720	Unresolved int
1710	1721	Notice string
1711	1722	DetachedThreads []diffThread
	1723	StackedOn *store.MR
	1724	Stacked []store.MR
1712	1725	}{p, m, view, md(m.Body, m.BodyFormat), checks, combined, renderComments(comments, md),
1713	1726	reviews, files, stat, commits, branches, s.canEditItem(r, p.Repo, m.Author),
1714		canWrite, unresolved, r.URL.Query().Get("e"), detachedThreads})
	1727	canWrite, unresolved, r.URL.Query().Get("e"), detachedThreads, stackedOn, stacked})
1715	1728	}
1716	1729
1717	1730	func (s Server) refs(w http.ResponseWriter, r http.Request) {

internal/store/stack.go added +59

		@@ -0,0 +1,59 @@
	1	package store
	2
	3	import (
	4	"database/sql"
	5	"errors"
	6	)
	7
	8	// Stacked merge requests: B is stacked on A when B's target branch is A's
	9	// source branch, both open, in the same repository. Nothing is stored;
	10	// these two reads derive it.
	11
	12	// OpenMRsByTarget lists the open merge requests in repoID targeting
	13	// targetRef, oldest first.
	14	func (s *Store) OpenMRsByTarget(repoID int64, targetRef string) ([]MR, error) {
	15	rows, err := s.DB.Query(
	16	mrSelect+" WHERE m.repo_id = ? AND m.target_ref = ? AND m.state = 'open' ORDER BY m.number",
	17	repoID, targetRef)
	18	if err != nil {
	19	return nil, err
	20	}
	21	defer rows.Close()
	22	var out []MR
	23	for rows.Next() {
	24	m, err := scanMR(rows)
	25	if err != nil {
	26	return nil, err
	27	}
	28	out = append(out, m)
	29	}
	30	return out, rows.Err()
	31	}
	32
	33	// OpenMRBySource finds the open merge request in repoID whose source is
	34	// the repository's own branch sourceRef. ok is false when there is none.
	35	func (s *Store) OpenMRBySource(repoID int64, sourceRef string) (MR, bool, error) {
	36	m, err := scanMR(s.DB.QueryRow(
	37	mrSelect+" WHERE m.repo_id = ? AND m.source_repo_id = ? AND m.source_ref = ? AND m.state = 'open' ORDER BY m.number LIMIT 1",
	38	repoID, repoID, sourceRef))
	39	if errors.Is(err, sql.ErrNoRows) {
	40	return MR{}, false, nil
	41	}
	42	return m, err == nil, err
	43	}
	44
	45	// RetargetKeepingReviews moves a merge request onto a new target without
	46	// staling its reviews: used when the branch it was stacked on has merged,
	47	// so the diff against the new target is the diff the reviews were of.
	48	func (s *Store) RetargetKeepingReviews(mrID int64, targetRef string) error {
	49	res, err := s.DB.Exec(
	50	"UPDATE merge_requests SET target_ref = ?, updated_at = strftime('%Y-%m-%dT%H:%M:%fZ','now') WHERE id = ?",
	51	targetRef, mrID)
	52	if err != nil {
	53	return err
	54	}
	55	if n, _ := res.RowsAffected(); n == 0 {
	56	return ErrNotFound
	57	}
	58	return nil
	59	}

internal/web/templates/mr.html +2

		@@ -15,6 +15,8 @@
15	15	{{else}}
16	16	<a href="/{{.MR.Author}}">{{.MR.Author}}</a> wants to merge {{template "mrrange" .MR}}
17	17	{{end}}</p>
	18	{{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}}
	19	{{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}}
18	20
19	21	{{if .Notice}}<p class="error" role="alert">{{.Notice}}</p>{{end}}
20	22

Commit 39535a8e16

Commit `39535a8e16`