eba8ed7 fix(scopes): keep the owner rights of full tokens with mr:force-merge, refresh the owner's merge check controls live · ermite

M internal/adapters/kvstore/issues_test.go +55 -8


@@ -271,7 +271,7 @@ func TestRepoSettingsKeepTheIssuesMode(t *testing.T) {
271	271
272	272		// TestSchema4UpgradesTheTokensWithAllScopes: the tokens that held every
273	273		// original scope keep their owner rights, gaining mr:review and issue:write
274		-	// (each only if missing); no other token changes.
	274	+	// (each only if missing), then mr:force-merge in schema 5; no other token changes.
275	275		func TestSchema4UpgradesTheTokensWithAllScopes(t *testing.T) {
276	276		ctx := context.Background()
277	277		dir := t.TempDir()

@@ -309,17 +309,20 @@ func TestSchema4UpgradesTheTokensWithAllScopes(t *testing.T) {
309	309		full := get("full")
310	310		assert.Contains(t, full.Scopes, domain.ScopeIssueWrite)
311	311		assert.Contains(t, full.Scopes, domain.ScopeMRReview)
312		-	assert.Len(t, full.Scopes, 11)
	312	+	assert.Len(t, full.Scopes, 12)
313	313		full10 := get("full10")
314		-	assert.Len(t, full10.Scopes, 11)
	314	+	assert.Len(t, full10.Scopes, 12)
315	315		assert.Equal(t, 1, countScope(full10.Scopes, domain.ScopeMRReview))
316	316		assert.Contains(t, full10.Scopes, domain.ScopeIssueWrite)
317		-	assert.False(t, domain.TokenPrincipal(full10).HasOwnerRights(), "historical migration does not grant mr:force-merge")
	317	+	assert.True(t, domain.TokenPrincipal(full10).HasOwnerRights())
318	318		assert.Len(t, get("reposmr").Scopes, 10)
319	319		assert.NotContains(t, get("reposmr").Scopes, domain.ScopeIssueWrite)
320		-	assert.False(t, domain.TokenPrincipal(full).HasOwnerRights(), "historical migration does not grant mr:force-merge")
321		-	assert.False(t, domain.TokenPrincipal(get("otherfull")).HasOwnerRights(), "historical migration does not grant mr:force-merge")
	320	+	assert.True(t, domain.TokenPrincipal(full).HasOwnerRights())
	321	+	assert.True(t, domain.TokenPrincipal(get("otherfull")).HasOwnerRights())
322	322
	323	+	for _, id := range []string{"missing", "reposmr", "repos", "prefix", "small"} {
	324	+	assert.NotContains(t, get(id).Scopes, domain.ScopeMRForceMerge)
	325	+	}
323	326		assert.NotContains(t, get("missing").Scopes, domain.ScopeIssueWrite)
324	327		assert.NotContains(t, get("missing").Scopes, domain.ScopeMRReview)
325	328		assert.NotContains(t, get("repos").Scopes, domain.ScopeMRReview)

@@ -335,7 +338,7 @@ func TestSchema4UpgradesTheTokensWithAllScopes(t *testing.T) {
335	338		assert.Equal(t, "full", full.ID)
336	339
337	340		require.NoError(t, s.Close(ctx))
338		-	assert.Equal(t, uint32(4), readSchema(t, dir))
	341	+	assert.Equal(t, uint32(5), readSchema(t, dir))
339	342
340	343		// A second open changes nothing.
341	344		s2, err := kvstore.Open(ctx, dir)

@@ -355,7 +358,7 @@ func TestSchemaNewerThanThisBuildStopsIssuesData(t *testing.T) {
355	358		putRaw(t, dir, "meta/schema", mustProto(t, &storagepb.SchemaVersion{Version: kvstore.SchemaVersion + 1}))
356	359		_, err := kvstore.Open(ctx, dir)
357	360		require.ErrorIs(t, err, kvstore.ErrSchemaTooNew)
358		-	assert.Equal(t, uint32(4), uint32(kvstore.SchemaVersion))
	361	+	assert.Equal(t, uint32(5), uint32(kvstore.SchemaVersion))
359	362		}
360	363
361	364		// A reopen racing the delete of the repository either lands before the delete

@@ -561,3 +564,47 @@ func countScope(scopes []domain.Scope, want domain.Scope) int {
561	564		}
562	565		return n
563	566		}
	567	+
	568	+	func TestSchema5UpgradesOwnerEquivalentTokens(t *testing.T) {
	569	+	ctx := context.Background()
	570	+	dir := t.TempDir()
	571	+	putRaw(t, dir, "meta/schema", mustProto(t, &storagepb.SchemaVersion{Version: 4}))
	572	+	var previous []string
	573	+	for _, sc := range domain.AllScopes() {
	574	+	if sc != domain.ScopeMRForceMerge {
	575	+	previous = append(previous, sc.String())
	576	+	}
	577	+	}
	578	+	tokens := map[string]*storagepb.Token{
	579	+	"owner": {Id: "owner", Scopes: previous},
	580	+	"repos": {Id: "repos", Scopes: previous, Repos: []string{"proj"}},
	581	+	"branch": {Id: "branch", Scopes: previous, BranchPrefixes: []string{"agent/"}},
	582	+	"partial": {Id: "partial", Scopes: previous[:10]},
	583	+	}
	584	+	for id, tok := range tokens {
	585	+	tok.Hash = make([]byte, 32)
	586	+	putRaw(t, dir, "token/"+id, mustProto(t, tok))
	587	+	}
	588	+	for range 2 {
	589	+	s, err := kvstore.Open(ctx, dir)
	590	+	require.NoError(t, err)
	591	+	for id, original := range tokens {
	592	+	tok, err := s.Tokens().Get(ctx, id)
	593	+	require.NoError(t, err)
	594	+	if id == "owner" {
	595	+	assert.True(t, domain.TokenPrincipal(tok).HasOwnerRights())
	596	+	assert.Equal(t, 1, countScope(tok.Scopes, domain.ScopeMRForceMerge))
	597	+	assert.Len(t, tok.Scopes, 12)
	598	+	} else {
	599	+	assert.NotContains(t, tok.Scopes, domain.ScopeMRForceMerge)
	600	+	var got []string
	601	+	for _, sc := range tok.Scopes {
	602	+	got = append(got, sc.String())
	603	+	}
	604	+	assert.Equal(t, original.Scopes, got)
	605	+	}
	606	+	}
	607	+	require.NoError(t, s.Close(ctx))
	608	+	assert.Equal(t, uint32(5), readSchema(t, dir))
	609	+	}
	610	+	}


@@ -271,7 +271,7 @@ func TestRepoSettingsKeepTheIssuesMode(t *testing.T) {
271			271
272		// TestSchema4UpgradesTheTokensWithAllScopes: the tokens that held every	272		// TestSchema4UpgradesTheTokensWithAllScopes: the tokens that held every
273		// original scope keep their owner rights, gaining mr:review and issue:write	273		// original scope keep their owner rights, gaining mr:review and issue:write
274	-	// (each only if missing); no other token changes.	274	+	// (each only if missing), then mr:force-merge in schema 5; no other token changes.
275		func TestSchema4UpgradesTheTokensWithAllScopes(t *testing.T) {	275		func TestSchema4UpgradesTheTokensWithAllScopes(t *testing.T) {
276		ctx := context.Background()	276		ctx := context.Background()
277		dir := t.TempDir()	277		dir := t.TempDir()

@@ -309,17 +309,20 @@ func TestSchema4UpgradesTheTokensWithAllScopes(t *testing.T) {
309		full := get("full")	309		full := get("full")
310		assert.Contains(t, full.Scopes, domain.ScopeIssueWrite)	310		assert.Contains(t, full.Scopes, domain.ScopeIssueWrite)
311		assert.Contains(t, full.Scopes, domain.ScopeMRReview)	311		assert.Contains(t, full.Scopes, domain.ScopeMRReview)
312	-	assert.Len(t, full.Scopes, 11)	312	+	assert.Len(t, full.Scopes, 12)
313		full10 := get("full10")	313		full10 := get("full10")
314	-	assert.Len(t, full10.Scopes, 11)	314	+	assert.Len(t, full10.Scopes, 12)
315		assert.Equal(t, 1, countScope(full10.Scopes, domain.ScopeMRReview))	315		assert.Equal(t, 1, countScope(full10.Scopes, domain.ScopeMRReview))
316		assert.Contains(t, full10.Scopes, domain.ScopeIssueWrite)	316		assert.Contains(t, full10.Scopes, domain.ScopeIssueWrite)
317	-	assert.False(t, domain.TokenPrincipal(full10).HasOwnerRights(), "historical migration does not grant mr:force-merge")	317	+	assert.True(t, domain.TokenPrincipal(full10).HasOwnerRights())
318		assert.Len(t, get("reposmr").Scopes, 10)	318		assert.Len(t, get("reposmr").Scopes, 10)
319		assert.NotContains(t, get("reposmr").Scopes, domain.ScopeIssueWrite)	319		assert.NotContains(t, get("reposmr").Scopes, domain.ScopeIssueWrite)
320	-	assert.False(t, domain.TokenPrincipal(full).HasOwnerRights(), "historical migration does not grant mr:force-merge")	320	+	assert.True(t, domain.TokenPrincipal(full).HasOwnerRights())
321	-	assert.False(t, domain.TokenPrincipal(get("otherfull")).HasOwnerRights(), "historical migration does not grant mr:force-merge")	321	+	assert.True(t, domain.TokenPrincipal(get("otherfull")).HasOwnerRights())
322			322
			323	+	for _, id := range []string{"missing", "reposmr", "repos", "prefix", "small"} {
			324	+	assert.NotContains(t, get(id).Scopes, domain.ScopeMRForceMerge)
			325	+	}
323		assert.NotContains(t, get("missing").Scopes, domain.ScopeIssueWrite)	326		assert.NotContains(t, get("missing").Scopes, domain.ScopeIssueWrite)
324		assert.NotContains(t, get("missing").Scopes, domain.ScopeMRReview)	327		assert.NotContains(t, get("missing").Scopes, domain.ScopeMRReview)
325		assert.NotContains(t, get("repos").Scopes, domain.ScopeMRReview)	328		assert.NotContains(t, get("repos").Scopes, domain.ScopeMRReview)

@@ -335,7 +338,7 @@ func TestSchema4UpgradesTheTokensWithAllScopes(t *testing.T) {
335		assert.Equal(t, "full", full.ID)	338		assert.Equal(t, "full", full.ID)
336			339
337		require.NoError(t, s.Close(ctx))	340		require.NoError(t, s.Close(ctx))
338	-	assert.Equal(t, uint32(4), readSchema(t, dir))	341	+	assert.Equal(t, uint32(5), readSchema(t, dir))
339			342
340		// A second open changes nothing.	343		// A second open changes nothing.
341		s2, err := kvstore.Open(ctx, dir)	344		s2, err := kvstore.Open(ctx, dir)

@@ -355,7 +358,7 @@ func TestSchemaNewerThanThisBuildStopsIssuesData(t *testing.T) {
355		putRaw(t, dir, "meta/schema", mustProto(t, &storagepb.SchemaVersion{Version: kvstore.SchemaVersion + 1}))	358		putRaw(t, dir, "meta/schema", mustProto(t, &storagepb.SchemaVersion{Version: kvstore.SchemaVersion + 1}))
356		_, err := kvstore.Open(ctx, dir)	359		_, err := kvstore.Open(ctx, dir)
357		require.ErrorIs(t, err, kvstore.ErrSchemaTooNew)	360		require.ErrorIs(t, err, kvstore.ErrSchemaTooNew)
358	-	assert.Equal(t, uint32(4), uint32(kvstore.SchemaVersion))	361	+	assert.Equal(t, uint32(5), uint32(kvstore.SchemaVersion))
359		}	362		}
360			363
361		// A reopen racing the delete of the repository either lands before the delete	364		// A reopen racing the delete of the repository either lands before the delete

@@ -561,3 +564,47 @@ func countScope(scopes []domain.Scope, want domain.Scope) int {
561		}	564		}
562		return n	565		return n
563		}	566		}
			567	+
			568	+	func TestSchema5UpgradesOwnerEquivalentTokens(t *testing.T) {
			569	+	ctx := context.Background()
			570	+	dir := t.TempDir()
			571	+	putRaw(t, dir, "meta/schema", mustProto(t, &storagepb.SchemaVersion{Version: 4}))
			572	+	var previous []string
			573	+	for _, sc := range domain.AllScopes() {
			574	+	if sc != domain.ScopeMRForceMerge {
			575	+	previous = append(previous, sc.String())
			576	+	}
			577	+	}
			578	+	tokens := map[string]*storagepb.Token{
			579	+	"owner": {Id: "owner", Scopes: previous},
			580	+	"repos": {Id: "repos", Scopes: previous, Repos: []string{"proj"}},
			581	+	"branch": {Id: "branch", Scopes: previous, BranchPrefixes: []string{"agent/"}},
			582	+	"partial": {Id: "partial", Scopes: previous[:10]},
			583	+	}
			584	+	for id, tok := range tokens {
			585	+	tok.Hash = make([]byte, 32)
			586	+	putRaw(t, dir, "token/"+id, mustProto(t, tok))
			587	+	}
			588	+	for range 2 {
			589	+	s, err := kvstore.Open(ctx, dir)
			590	+	require.NoError(t, err)
			591	+	for id, original := range tokens {
			592	+	tok, err := s.Tokens().Get(ctx, id)
			593	+	require.NoError(t, err)
			594	+	if id == "owner" {
			595	+	assert.True(t, domain.TokenPrincipal(tok).HasOwnerRights())
			596	+	assert.Equal(t, 1, countScope(tok.Scopes, domain.ScopeMRForceMerge))
			597	+	assert.Len(t, tok.Scopes, 12)
			598	+	} else {
			599	+	assert.NotContains(t, tok.Scopes, domain.ScopeMRForceMerge)
			600	+	var got []string
			601	+	for _, sc := range tok.Scopes {
			602	+	got = append(got, sc.String())
			603	+	}
			604	+	assert.Equal(t, original.Scopes, got)
			605	+	}
			606	+	}
			607	+	require.NoError(t, s.Close(ctx))
			608	+	assert.Equal(t, uint32(5), readSchema(t, dir))
			609	+	}
			610	+	}

M internal/adapters/kvstore/store.go +36 -8


@@ -39,8 +39,9 @@ import (
39	39		// (meta/issuekey). The upgrade also adds mr:review and issue:write (each only
40	40		// if missing) to the stored tokens that held all nine original scopes with no
41	41		// repository list and no branch prefix, so that they keep the owner rights (see
42		-	// upgradeTokens).
43		-	const SchemaVersion = 4
	42	+	// upgradeTokens). Version 5 grants mr:force-merge to unrestricted tokens
	43	+	// holding all eleven previous scopes, preserving their owner rights.
	44	+	const SchemaVersion = 5
44	45
45	46		const schemaKey = "meta/schema"
46	47

@@ -109,7 +110,7 @@ func (s *Store) Check(ctx context.Context) error {
109	110		func (s *Store) checkSchema(ctx context.Context) error {
110	111		log := zerowrap.FromCtx(ctx)
111	112		key := []byte(schemaKey)
112		-	for range 2 {
	113	+	for range 4 {
113	114		item, ok := s.db.Get(ctx, key)
114	115		if !ok {
115	116		var b kv.Batch

@@ -140,6 +141,10 @@ func (s *Store) checkSchema(ctx context.Context) error {
140	141		// the new version is recorded so that older builds refuse it.
141	142		// The tokens are upgraded in the same batch, so that a crash
142	143		// leaves the old version and the upgrade runs again.
	144	+	target := uint32(SchemaVersion)
	145	+	if sv.GetVersion() < 4 {
	146	+	target = 4
	147	+	}
143	148		var b kv.Batch
144	149		b.RequireVersion(key, item.Version)
145	150		if sv.GetVersion() < 4 {

@@ -147,7 +152,12 @@ func (s *Store) checkSchema(ctx context.Context) error {
147	152		return err
148	153		}
149	154		}
150		-	b.Put(key, mustMarshal(&storagepb.SchemaVersion{Version: SchemaVersion}))
	155	+	if sv.GetVersion() == 4 {
	156	+	if err := s.upgradeForceMergeTokens(ctx, &b); err != nil {
	157	+	return err
	158	+	}
	159	+	}
	160	+	b.Put(key, mustMarshal(&storagepb.SchemaVersion{Version: target}))
151	161		_, err := s.db.Apply(ctx, &b)
152	162		if errors.Is(err, kv.ErrConflict) {
153	163		continue // changed meanwhile: read it again

@@ -155,7 +165,10 @@ func (s *Store) checkSchema(ctx context.Context) error {
155	165		if err != nil {
156	166		return fmt.Errorf("upgrade storage schema: %w", err)
157	167		}
158		-	log.Info().Uint32("from", sv.GetVersion()).Uint32("schema", SchemaVersion).Msg("storage schema upgraded")
	168	+	log.Info().Uint32("from", sv.GetVersion()).Uint32("schema", target).Msg("storage schema upgraded")
	169	+	if target < SchemaVersion {
	170	+	continue
	171	+	}
159	172		return nil
160	173		}
161	174		log.Info().Uint32("schema", sv.GetVersion()).Msg("storage schema checked")

@@ -182,11 +195,26 @@ var preIssueScopes = []domain.Scope{
182	195
183	196		// upgradeTokens adds to b the schema 4 upgrade of the tokens: a token that
184	197		// holds all of preIssueScopes, limited to no repository and no branch prefix,
185		-	// had the owner rights, which AllScopes now needs mr:review and issue:write
	198	+	// had the owner rights, which schema 4 needed mr:review and issue:write
186	199		// for, so it gets each of them if missing (this is no escalation). Every other
187	200		// token is left as it is. The batch requires each token to be at the version it
188	201		// was read at.
189	202		func (s Store) upgradeTokens(ctx context.Context, b kv.Batch) error {
	203	+	return s.upgradeTokenScopes(ctx, b, preIssueScopes, []domain.Scope{domain.ScopeMRReview, domain.ScopeIssueWrite})
	204	+	}
	205	+
	206	+	// preForceMergeScopes are the eleven scopes of schema 4, before mr:force-merge.
	207	+	var preForceMergeScopes = append(slices.Clone(preIssueScopes), domain.ScopeMRReview, domain.ScopeIssueWrite)
	208	+
	209	+	// upgradeForceMergeTokens adds to b the schema 5 upgrade of the tokens: a
	210	+	// token that holds all of preForceMergeScopes, limited to no repository and no
	211	+	// branch prefix, had the owner rights, which AllScopes also needs
	212	+	// mr:force-merge for, so it gets it. Every other token is left as it is.
	213	+	func (s Store) upgradeForceMergeTokens(ctx context.Context, b kv.Batch) error {
	214	+	return s.upgradeTokenScopes(ctx, b, preForceMergeScopes, []domain.Scope{domain.ScopeMRForceMerge})
	215	+	}
	216	+
	217	+	func (s Store) upgradeTokenScopes(ctx context.Context, b kv.Batch, required, addedScopes []domain.Scope) error {
190	218		for key, item := range s.db.Snapshot(ctx).Prefix(ctx, []byte(tokenPrefix)) {
191	219		pb := &storagepb.Token{}
192	220		if err := proto.Unmarshal(item.Value, pb); err != nil {

@@ -196,7 +224,7 @@ func (s Store) upgradeTokens(ctx context.Context, b kv.Batch) error {
196	224		continue
197	225		}
198	226		full := true
199		-	for _, sc := range preIssueScopes {
	227	+	for _, sc := range required {
200	228		if !slices.Contains(pb.GetScopes(), sc.String()) {
201	229		full = false
202	230		break

@@ -206,7 +234,7 @@ func (s Store) upgradeTokens(ctx context.Context, b kv.Batch) error {
206	234		continue
207	235		}
208	236		added := false
209		-	for _, sc := range []domain.Scope{domain.ScopeMRReview, domain.ScopeIssueWrite} {
	237	+	for _, sc := range addedScopes {
210	238		if !slices.Contains(pb.GetScopes(), sc.String()) {
211	239		pb.Scopes = append(pb.Scopes, sc.String())
212	240		added = true

 // (meta/issuekey). The upgrade also adds mr:review and issue:write (each only
 // if missing) to the stored tokens that held all nine original scopes with no
 // repository list and no branch prefix, so that they keep the owner rights (see
-// upgradeTokens).
+// upgradeTokens). Version 5 grants mr:force-merge to unrestricted tokens
-const SchemaVersion = 4
+// holding all eleven previous scopes, preserving their owner rights.
+const SchemaVersion = 5
 const schemaKey = "meta/schema"
 func (s *Store) checkSchema(ctx context.Context) error {
 	log := zerowrap.FromCtx(ctx)
 	key := []byte(schemaKey)
-	for range 2 {
+	for range 4 {
 		item, ok := s.db.Get(ctx, key)
 		if !ok {
 			var b kv.Batch
 			// the new version is recorded so that older builds refuse it.
 			// The tokens are upgraded in the same batch, so that a crash
 			// leaves the old version and the upgrade runs again.
+			target := uint32(SchemaVersion)
+			if sv.GetVersion() < 4 {
+				target = 4
+			}
 			var b kv.Batch
 			b.RequireVersion(key, item.Version)
 			if sv.GetVersion() < 4 {
 					return err
 				}
 			}
-			b.Put(key, mustMarshal(&storagepb.SchemaVersion{Version: SchemaVersion}))
+			if sv.GetVersion() == 4 {
+				if err := s.upgradeForceMergeTokens(ctx, &b); err != nil {
+					return err
+				}
+			}
+			b.Put(key, mustMarshal(&storagepb.SchemaVersion{Version: target}))
 			_, err := s.db.Apply(ctx, &b)
 			if errors.Is(err, kv.ErrConflict) {
 				continue // changed meanwhile: read it again
 			if err != nil {
 				return fmt.Errorf("upgrade storage schema: %w", err)
 			}
-			log.Info().Uint32("from", sv.GetVersion()).Uint32("schema", SchemaVersion).Msg("storage schema upgraded")
+			log.Info().Uint32("from", sv.GetVersion()).Uint32("schema", target).Msg("storage schema upgraded")
+			if target < SchemaVersion {
+				continue
+			}
 			return nil
 		}
 		log.Info().Uint32("schema", sv.GetVersion()).Msg("storage schema checked")
 // upgradeTokens adds to b the schema 4 upgrade of the tokens: a token that
 // holds all of preIssueScopes, limited to no repository and no branch prefix,
-// had the owner rights, which AllScopes now needs mr:review and issue:write
+// had the owner rights, which schema 4 needed mr:review and issue:write
 // for, so it gets each of them if missing (this is no escalation). Every other
 // token is left as it is. The batch requires each token to be at the version it
 // was read at.
 func (s *Store) upgradeTokens(ctx context.Context, b *kv.Batch) error {
+	return s.upgradeTokenScopes(ctx, b, preIssueScopes, []domain.Scope{domain.ScopeMRReview, domain.ScopeIssueWrite})
+}
+// preForceMergeScopes are the eleven scopes of schema 4, before mr:force-merge.
+var preForceMergeScopes = append(slices.Clone(preIssueScopes), domain.ScopeMRReview, domain.ScopeIssueWrite)
+// upgradeForceMergeTokens adds to b the schema 5 upgrade of the tokens: a
+// token that holds all of preForceMergeScopes, limited to no repository and no
+// branch prefix, had the owner rights, which AllScopes also needs
+// mr:force-merge for, so it gets it. Every other token is left as it is.
+func (s *Store) upgradeForceMergeTokens(ctx context.Context, b *kv.Batch) error {
+	return s.upgradeTokenScopes(ctx, b, preForceMergeScopes, []domain.Scope{domain.ScopeMRForceMerge})
+}
+func (s *Store) upgradeTokenScopes(ctx context.Context, b *kv.Batch, required, addedScopes []domain.Scope) error {
 	for key, item := range s.db.Snapshot(ctx).Prefix(ctx, []byte(tokenPrefix)) {
 		pb := &storagepb.Token{}
 		if err := proto.Unmarshal(item.Value, pb); err != nil {
 			continue
 		}
 		full := true
-		for _, sc := range preIssueScopes {
+		for _, sc := range required {
 			if !slices.Contains(pb.GetScopes(), sc.String()) {
 				full = false
 				break
 			continue
 		}
 		added := false
-		for _, sc := range []domain.Scope{domain.ScopeMRReview, domain.ScopeIssueWrite} {
+		for _, sc := range addedScopes {
 			if !slices.Contains(pb.GetScopes(), sc.String()) {
 				pb.Scopes = append(pb.Scopes, sc.String())
 				added = true

M internal/adapters/web/fragments.go +18 -1


@@ -36,6 +36,8 @@ type liveStream struct {
36	36		// head shown (its checks are on that head); a job event does not carry
37	37		// its commit, so its run is read once. One stream reads it, no lock.
38	38		headRuns map[int64]bool
	39	+	// mergeChecksHTML is the last merge-form part sent, not sent again unchanged.
	40	+	mergeChecksHTML string
39	41		}
40	42
41	43		// maxHeadRuns bounds headRuns: past it the map starts again.

@@ -625,7 +627,22 @@ func (l *liveStream) mrChecks(ctx context.Context) ([]fragment, error) {
625	627		return nil, err
626	628		}
627	629		sum, err := renderFragment(ctx, "ci-run", MRChecksSum(checksSummary(name, l.head, checks)))
628		-	return []fragment{f, sum}, err
	630	+	if err != nil {
	631	+	return nil, err
	632	+	}
	633	+	// The merge form is the owner's: a visitor's stream never carries it.
	634	+	if !SignedIn(ctx) {
	635	+	return []fragment{f, sum}, nil
	636	+	}
	637	+	controls, err := renderFragment(ctx, "ci-run", MRMergeChecks(checksSummary(name, l.head, checks)))
	638	+	if err != nil {
	639	+	return nil, err
	640	+	}
	641	+	if controls.html == l.mergeChecksHTML {
	642	+	return []fragment{f, sum}, nil
	643	+	}
	644	+	l.mergeChecksHTML = controls.html
	645	+	return []fragment{f, sum, controls}, nil
629	646		}
630	647
631	648		// recentActions is the "Recent actions" block of the summary rail, read again


@@ -36,6 +36,8 @@ type liveStream struct {
36		// head shown (its checks are on that head); a job event does not carry	36		// head shown (its checks are on that head); a job event does not carry
37		// its commit, so its run is read once. One stream reads it, no lock.	37		// its commit, so its run is read once. One stream reads it, no lock.
38		headRuns map[int64]bool	38		headRuns map[int64]bool
			39	+	// mergeChecksHTML is the last merge-form part sent, not sent again unchanged.
			40	+	mergeChecksHTML string
39		}	41		}
40			42
41		// maxHeadRuns bounds headRuns: past it the map starts again.	43		// maxHeadRuns bounds headRuns: past it the map starts again.

@@ -625,7 +627,22 @@ func (l *liveStream) mrChecks(ctx context.Context) ([]fragment, error) {
625		return nil, err	627		return nil, err
626		}	628		}
627		sum, err := renderFragment(ctx, "ci-run", MRChecksSum(checksSummary(name, l.head, checks)))	629		sum, err := renderFragment(ctx, "ci-run", MRChecksSum(checksSummary(name, l.head, checks)))
628	-	return []fragment{f, sum}, err	630	+	if err != nil {
			631	+	return nil, err
			632	+	}
			633	+	// The merge form is the owner's: a visitor's stream never carries it.
			634	+	if !SignedIn(ctx) {
			635	+	return []fragment{f, sum}, nil
			636	+	}
			637	+	controls, err := renderFragment(ctx, "ci-run", MRMergeChecks(checksSummary(name, l.head, checks)))
			638	+	if err != nil {
			639	+	return nil, err
			640	+	}
			641	+	if controls.html == l.mergeChecksHTML {
			642	+	return []fragment{f, sum}, nil
			643	+	}
			644	+	l.mergeChecksHTML = controls.html
			645	+	return []fragment{f, sum, controls}, nil
629		}	646		}
630			647
631		// recentActions is the "Recent actions" block of the summary rail, read again	648		// recentActions is the "Recent actions" block of the summary rail, read again

M internal/adapters/web/mr.templ +12 -4


@@ -545,10 +545,7 @@ templ mergeMenu(p MRPage) {
545	545		</label>
546	546		}
547	547		</fieldset>
548		-	if p.ChecksSum.Kind == "bad" \|\| p.ChecksSum.Kind == "running" {
549		-	<p class="d-note">{ p.ChecksSum.State + ": a failing or unfinished check blocks the merge." }</p>
550		-	<label class="d-check"><input type="checkbox" name="ignore_checks"/> Ignore checks</label>
551		-	}
	548	+	@MRMergeChecks(p.ChecksSum)
552	549		if p.Box.CanDeleteBranch {
553	550		<label class="d-check"><input type="checkbox" name="delete_branch" value="1"/> delete { p.Source } after the merge <kbd aria-hidden="true">D</kbd></label>
554	551		}

@@ -770,3 +767,14 @@ templ compareRail(p ComparePage) {
770	767		@CloneBlock(p.CLI)
771	768		</section>
772	769		}
	770	+
	771	+	// MRMergeChecks is the part of the owner's merge form that depends on the
	772	+	// checks: a live update replaces it and keeps "Ignore checks" ticked.
	773	+	templ MRMergeChecks(c ChecksSummary) {
	774	+	<div id="mr-merge-checks" data-keep-checks>
	775	+	if c.Kind == "bad" \|\| c.Kind == "running" {
	776	+	<p class="d-note">{ c.State + ": a failing or unfinished check blocks the merge." }</p>
	777	+	<label class="d-check"><input type="checkbox" name="ignore_checks"/> Ignore checks</label>
	778	+	}
	779	+	</div>
	780	+	}

 							</label>
 						}
 					</fieldset>
-					if p.ChecksSum.Kind == "bad" || p.ChecksSum.Kind == "running" {
+					@MRMergeChecks(p.ChecksSum)
-						<p class="d-note">{ p.ChecksSum.State + ": a failing or unfinished check blocks the merge." }</p>
-						<label class="d-check"><input type="checkbox" name="ignore_checks"/> Ignore checks</label>
-					}
 					if p.Box.CanDeleteBranch {
 						<label class="d-check"><input type="checkbox" name="delete_branch" value="1"/> delete { p.Source } after the merge <kbd aria-hidden="true">D</kbd></label>
 					}
 		@CloneBlock(p.CLI)
 	</section>
 }
+// MRMergeChecks is the part of the owner's merge form that depends on the
+// checks: a live update replaces it and keeps "Ignore checks" ticked.
+templ MRMergeChecks(c ChecksSummary) {
+	<div id="mr-merge-checks" data-keep-checks>
+		if c.Kind == "bad" || c.Kind == "running" {
+			<p class="d-note">{ c.State + ": a failing or unfinished check blocks the merge." }</p>
+			<label class="d-check"><input type="checkbox" name="ignore_checks"/> Ignore checks</label>
+		}
+	</div>
+}

M internal/adapters/web/mr_templ.go +393 -354 generated file: collapsed

load diff

M internal/adapters/web/static/js/ermite.js +7 -0


@@ -966,6 +966,13 @@ window.ermiteLive = (() => {
966	966		continue;
967	967		}
968	968		if (old.hasAttribute('data-cursor')) el.setAttribute('data-cursor', '');
	969	+	// data-keep-checks: the checkboxes the user ticked stay ticked.
	970	+	if (old.hasAttribute('data-keep-checks')) {
	971	+	for (const box of old.querySelectorAll('input[type=checkbox][name]:checked')) {
	972	+	const same = el.querySelector(`input[type=checkbox][name="${CSS.escape(box.name)}"]`);
	973	+	if (same) same.checked = true;
	974	+	}
	975	+	}
969	976		old.replaceWith(el);
970	977		const rows = document.getElementById('repo-rows');
971	978		if (rows && el.parentElement === rows && rows.dataset.sort === 'activity' &&


@@ -966,6 +966,13 @@ window.ermiteLive = (() => {
966	continue;	966		continue;
967	}	967		}
968	if (old.hasAttribute('data-cursor')) el.setAttribute('data-cursor', '');	968		if (old.hasAttribute('data-cursor')) el.setAttribute('data-cursor', '');
		969	+	// data-keep-checks: the checkboxes the user ticked stay ticked.
		970	+	if (old.hasAttribute('data-keep-checks')) {
		971	+	for (const box of old.querySelectorAll('input[type=checkbox][name]:checked')) {
		972	+	const same = el.querySelector(`input[type=checkbox][name="${CSS.escape(box.name)}"]`);
		973	+	if (same) same.checked = true;
		974	+	}
		975	+	}
969	old.replaceWith(el);	976		old.replaceWith(el);
970	const rows = document.getElementById('repo-rows');	977		const rows = document.getElementById('repo-rows');
971	if (rows && el.parentElement === rows && rows.dataset.sort === 'activity' &&	978		if (rows && el.parentElement === rows && rows.dataset.sort === 'activity' &&

M internal/adapters/web/summary_actions_test.go +52 -0


@@ -4,6 +4,7 @@ package web
4	4
5	5		import (
6	6		"bytes"
	7	+	"context"
7	8		"errors"
8	9		"strings"
9	10		"testing"

@@ -125,3 +126,54 @@ func TestRunUpdatedOnSummaryWhenCIIsNotReadable(t *testing.T) {
125	126		assert.Contains(t, got[0].html, "hidden", "the block empties and hides, no run leaks")
126	127		assert.NotContains(t, got[0].html, "rr-name")
127	128		}
	129	+
	130	+	func TestMRChecksLiveMergeControls(t *testing.T) {
	131	+	t.Parallel()
	132	+	repo := repoName(t, "proj")
	133	+	head := strings.Repeat("a", domain.ObjectIDLen)
	134	+	for _, kind := range []string{"bad", "running", "ok", ""} {
	135	+	t.Run(kind, func(t *testing.T) {
	136	+	runs := mocks.NewRunService(t)
	137	+	var checks []ports.CheckView
	138	+	if kind != "" {
	139	+	check := domain.CICheck{Name: "ci", Status: domain.CICompleted, Conclusion: domain.CISuccess}
	140	+	if kind == "bad" {
	141	+	check.Conclusion = domain.CIFailure
	142	+	}
	143	+	if kind == "running" {
	144	+	check.Status, check.Conclusion = domain.CIQueued, ""
	145	+	}
	146	+	checks = []ports.CheckView{{CICheck: check}}
	147	+	}
	148	+	runs.EXPECT().ListChecks(mock.Anything, mock.Anything, repo, head).Return(ports.ChecksResult{Checks: checks}, nil)
	149	+	srv := newTestServer(t, Config{Repos: reposServing(t, publicRepo(t, "proj")), Browse: mocks.NewBrowseService(t), Runs: runs})
	150	+
	151	+	// A visitor's stream never carries the owner's merge form.
	152	+	visitor, err := (&liveStream{s: srv, repo: "proj", head: head, view: "mr"}).mrChecks(t.Context())
	153	+	require.NoError(t, err)
	154	+	require.Len(t, visitor, 2)
	155	+	for _, f := range visitor {
	156	+	assert.NotContains(t, f.html, `mr-merge-checks`)
	157	+	}
	158	+
	159	+	owner := context.WithValue(t.Context(), signedInKey{}, struct{}{})
	160	+	ls := &liveStream{s: srv, repo: "proj", head: head, view: "mr"}
	161	+	got, err := ls.mrChecks(owner)
	162	+	require.NoError(t, err)
	163	+	require.Len(t, got, 3)
	164	+	assert.Contains(t, got[2].html, `data-keep-checks`, "a swap keeps the ticked box")
	165	+	assert.Equal(t, "ci-run", got[2].event)
	166	+	assert.Contains(t, got[2].html, `id="mr-merge-checks"`)
	167	+	assert.Equal(t, renderToString(t, MRMergeChecks(checksSummary(repo, head, checks))), got[2].html)
	168	+	if kind == "bad" \|\| kind == "running" {
	169	+	assert.Contains(t, got[2].html, `name="ignore_checks"`)
	170	+	} else {
	171	+	assert.NotContains(t, got[2].html, `name="ignore_checks"`)
	172	+	}
	173	+	// An unrelated job event with identical checks must not reset the checkbox.
	174	+	got, err = ls.mrChecks(owner)
	175	+	require.NoError(t, err)
	176	+	require.Len(t, got, 2)
	177	+	})
	178	+	}
	179	+	}


@@ -4,6 +4,7 @@ package web
4		4
5	import (	5		import (
6	"bytes"	6		"bytes"
		7	+	"context"
7	"errors"	8		"errors"
8	"strings"	9		"strings"
9	"testing"	10		"testing"

@@ -125,3 +126,54 @@ func TestRunUpdatedOnSummaryWhenCIIsNotReadable(t *testing.T) {
125	assert.Contains(t, got[0].html, "hidden", "the block empties and hides, no run leaks")	126		assert.Contains(t, got[0].html, "hidden", "the block empties and hides, no run leaks")
126	assert.NotContains(t, got[0].html, "rr-name")	127		assert.NotContains(t, got[0].html, "rr-name")
127	}	128		}
		129	+
		130	+	func TestMRChecksLiveMergeControls(t *testing.T) {
		131	+	t.Parallel()
		132	+	repo := repoName(t, "proj")
		133	+	head := strings.Repeat("a", domain.ObjectIDLen)
		134	+	for _, kind := range []string{"bad", "running", "ok", ""} {
		135	+	t.Run(kind, func(t *testing.T) {
		136	+	runs := mocks.NewRunService(t)
		137	+	var checks []ports.CheckView
		138	+	if kind != "" {
		139	+	check := domain.CICheck{Name: "ci", Status: domain.CICompleted, Conclusion: domain.CISuccess}
		140	+	if kind == "bad" {
		141	+	check.Conclusion = domain.CIFailure
		142	+	}
		143	+	if kind == "running" {
		144	+	check.Status, check.Conclusion = domain.CIQueued, ""
		145	+	}
		146	+	checks = []ports.CheckView{{CICheck: check}}
		147	+	}
		148	+	runs.EXPECT().ListChecks(mock.Anything, mock.Anything, repo, head).Return(ports.ChecksResult{Checks: checks}, nil)
		149	+	srv := newTestServer(t, Config{Repos: reposServing(t, publicRepo(t, "proj")), Browse: mocks.NewBrowseService(t), Runs: runs})
		150	+
		151	+	// A visitor's stream never carries the owner's merge form.
		152	+	visitor, err := (&liveStream{s: srv, repo: "proj", head: head, view: "mr"}).mrChecks(t.Context())
		153	+	require.NoError(t, err)
		154	+	require.Len(t, visitor, 2)
		155	+	for _, f := range visitor {
		156	+	assert.NotContains(t, f.html, `mr-merge-checks`)
		157	+	}
		158	+
		159	+	owner := context.WithValue(t.Context(), signedInKey{}, struct{}{})
		160	+	ls := &liveStream{s: srv, repo: "proj", head: head, view: "mr"}
		161	+	got, err := ls.mrChecks(owner)
		162	+	require.NoError(t, err)
		163	+	require.Len(t, got, 3)
		164	+	assert.Contains(t, got[2].html, `data-keep-checks`, "a swap keeps the ticked box")
		165	+	assert.Equal(t, "ci-run", got[2].event)
		166	+	assert.Contains(t, got[2].html, `id="mr-merge-checks"`)
		167	+	assert.Equal(t, renderToString(t, MRMergeChecks(checksSummary(repo, head, checks))), got[2].html)
		168	+	if kind == "bad" \|\| kind == "running" {
		169	+	assert.Contains(t, got[2].html, `name="ignore_checks"`)
		170	+	} else {
		171	+	assert.NotContains(t, got[2].html, `name="ignore_checks"`)
		172	+	}
		173	+	// An unrelated job event with identical checks must not reset the checkbox.
		174	+	got, err = ls.mrChecks(owner)
		175	+	require.NoError(t, err)
		176	+	require.Len(t, got, 2)
		177	+	})
		178	+	}
		179	+	}

M internal/usecase/mergerequests.go +3 -2


@@ -995,8 +995,9 @@ func (m *MergeRequests) MarkRead(ctx context.Context, p domain.Principal, name d
995	995		// allowed by the settings of the repository, the source branch no longer
996	996		// points to the head ("branch moved, read again") or is gone, the target is
997	997		// gone, the mode's own condition does not hold (see below), or the branch
998		-	// brings nothing. Checks come with CI (CI-1): there are none yet, so the
999		-	// setting that lets failing checks block the merge has nothing to act on.
	998	+	// brings nothing. Checks on the head must pass: none may be failed, queued
	999	+	// or running; a head with no checks may be merged. params.IgnoreChecks bypasses
	1000	+	// this requirement and needs mr:force-merge.
1000	1001		//
1001	1002		// The conditions are always checked with Git, under the lock, never from the
1002	1003		// stored mergeability. A fast-forward needs the target to be an ancestor of

 // allowed by the settings of the repository, the source branch no longer
 // points to the head ("branch moved, read again") or is gone, the target is
 // gone, the mode's own condition does not hold (see below), or the branch
-// brings nothing. Checks come with CI (CI-1): there are none yet, so the
+// brings nothing. Checks on the head must pass: none may be failed, queued
-// setting that lets failing checks block the merge has nothing to act on.
+// or running; a head with no checks may be merged. params.IgnoreChecks bypasses
+// this requirement and needs mr:force-merge.
 //
 // The conditions are always checked with Git, under the lock, never from the
 // stored mergeability. A fast-forward needs the target to be an ancestor of