krz/gitbay

A CLI-first git forge.

cli forge git self-hosted

https://gitbay.org

Commit dde9591208

dde95912081319c5dd693cfd886a1043ac4ca43f

parent: dd665998be

Verified · cmc

cmc <hello@cleberg.net> · 2026-08-24T16:12:34Z 

merge requirements: approvals, CODEOWNERS, resolved threads (#19)

repo settings require-approvals <n>: fresh non-author approvals, each
reviewer's latest review decides their stance, a fresh request-changes
blocks with the reviewer named. With approvals on, a CODEOWNERS file
on the target branch (root or .gitbay/) demands an approval from an
owner of every owned changed file — gitignore-style subset, last
match wins, unit-tested matcher. require-resolved refuses merges with
open review threads. Gates run after the checks gate and before any
strategy work; every refusal names what is missing.
cmd/gitbay/main.go +2
@@ -232,6 +232,8 @@ func repoCmd() *cobra.Command {
232232 pass("show", "show settings", passOpts{server: []string{"repo", "settings", "show"}, needsRepo: true}),
233233 pass("protect", "protect a branch", passOpts{server: []string{"repo", "settings", "protect"}, needsRepo: true}),
234234 pass("unprotect", "unprotect a branch", passOpts{server: []string{"repo", "settings", "unprotect"}, needsRepo: true}),
235 pass("require-approvals", "require N fresh approvals to merge: <n>", passOpts{server: []string{"repo", "settings", "require-approvals"}, needsRepo: true}),
236 pass("require-resolved", "require threads resolved to merge: on|off", passOpts{server: []string{"repo", "settings", "require-resolved"}, needsRepo: true}),
235237 pass("require-checks", "gate merges on green statuses: ... on|off", passOpts{server: []string{"repo", "settings", "require-checks"}, needsRepo: true}),
236238 pass("require-signed", "require verified commit signatures: ... on|off", passOpts{server: []string{"repo", "settings", "require-signed"}, needsRepo: true}),
237239 pass("description", "set the repository description: <text>", passOpts{server: []string{"repo", "settings", "description"}, needsRepo: true}),
docs/users.org +8
@@ -166,6 +166,14 @@ Semantics worth knowing:
166166 verified commits merge; everything server-created is refused with
167167 instructions to rebase locally.
168168
169Repo admins can gate merges (=repo settings ...=): =require-approvals
170<n>= (fresh, non-author approvals; each reviewer's latest review is
171their stance, and a fresh request-changes blocks), =require-resolved=
172(no open review threads), =require-checks= (all statuses green). With
173approvals required, a =CODEOWNERS= file on the target branch (root or
174=.gitbay/=) additionally demands an approval from an owner of every
175owned changed file — gitignore-style patterns, last match wins.
176
169177 Review threads anchor to diff lines:
170178
171179 #+begin_src sh
e2e/approvals_test.go added +138
@@ -0,0 +1,138 @@
1package e2e
2
3import (
4 "encoding/json"
5 "fmt"
6 "os"
7 "path/filepath"
8 "strings"
9 "testing"
10)
11
12func TestMergeRequirements(t *testing.T) {
13 inst := startInstance(t)
14 aliceKey := inst.newKey(t, "alice")
15 bobKey := inst.newKey(t, "bob")
16 carolKey := inst.newKey(t, "carol")
17 inst.admin(t, "admin", "user", "create", "alice",
18 "--key", aliceKey+".pub", "--email", "alice@example.test", "--verified")
19 inst.admin(t, "admin", "user", "create", "bob", "--key", bobKey+".pub")
20 inst.admin(t, "admin", "user", "create", "carol", "--key", carolKey+".pub")
21
22 // Repo with CODEOWNERS on main: carol owns *.go.
23 if _, errOut, code := inst.ssh(t, aliceKey, "", "repo", "create", "alice/svc"); code != 0 {
24 t.Fatalf("repo create: %s", errOut)
25 }
26 for _, u := range []string{"bob", "carol"} {
27 if _, _, code := inst.ssh(t, aliceKey, "", "repo", "access", "grant", "alice/svc", u, "write"); code != 0 {
28 t.Fatal("grant failed")
29 }
30 }
31 work := t.TempDir()
32 env := inst.gitEnv(aliceKey)
33 mustGit(t, work, env, "clone", inst.sshURL("alice/svc"), "w")
34 dir := filepath.Join(work, "w")
35 os.WriteFile(filepath.Join(dir, "CODEOWNERS"), []byte("*.go @carol\n"), 0o644)
36 os.WriteFile(filepath.Join(dir, "svc.go"), []byte("package svc\n"), 0o644)
37 mustGit(t, dir, env, "checkout", "-q", "-b", "main")
38 mustGit(t, dir, env, "add", ".")
39 mustGit(t, dir, env, "commit", "-q", "-m", "base")
40 mustGit(t, dir, env, "push", "-q", "origin", "main")
41
42 // MR by alice touching a .go file.
43 mustGit(t, dir, env, "checkout", "-q", "-b", "feat")
44 os.WriteFile(filepath.Join(dir, "svc.go"), []byte("package svc\n\nvar V = 1\n"), 0o644)
45 mustGit(t, dir, env, "add", ".")
46 mustGit(t, dir, env, "commit", "-q", "-m", "change")
47 mustGit(t, dir, env, "push", "-q", "origin", "feat")
48 if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "create", "alice/svc",
49 "--source", "feat", "--target", "main", "--title", "'change'"); code != 0 {
50 t.Fatalf("mr create: %s", errOut)
51 }
52 if _, _, code := inst.ssh(t, aliceKey, "", "repo", "settings", "require-approvals", "alice/svc", "1"); code != 0 {
53 t.Fatal("require-approvals failed")
54 }
55
56 // No approvals: refused. The author's own approval does not count.
57 _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "merge", "alice/svc", "1")
58 if code != 4 || !strings.Contains(errOut, "requires 1 fresh approval") {
59 t.Fatalf("no-approval merge: exit %d, %s", code, errOut)
60 }
61 if _, _, code = inst.ssh(t, aliceKey, "", "mr", "review", "alice/svc", "1", "--approve"); code != 0 {
62 t.Fatal("self review failed")
63 }
64 if _, _, code = inst.ssh(t, aliceKey, "", "mr", "merge", "alice/svc", "1"); code != 4 {
65 t.Fatal("author self-approval counted")
66 }
67
68 // Bob approves — but CODEOWNERS demands carol for *.go.
69 if _, _, code = inst.ssh(t, bobKey, "", "mr", "review", "alice/svc", "1", "--approve"); code != 0 {
70 t.Fatal("bob review failed")
71 }
72 _, errOut, code = inst.ssh(t, aliceKey, "", "mr", "merge", "alice/svc", "1")
73 if code != 4 || !strings.Contains(errOut, "CODEOWNERS") || !strings.Contains(errOut, "carol") {
74 t.Fatalf("codeowners gate: exit %d, %s", code, errOut)
75 }
76
77 // A fresh request-changes blocks even with approvals present.
78 if _, _, code = inst.ssh(t, carolKey, "", "mr", "review", "alice/svc", "1", "--request-changes"); code != 0 {
79 t.Fatal("carol review failed")
80 }
81 _, errOut, code = inst.ssh(t, aliceKey, "", "mr", "merge", "alice/svc", "1")
82 if code != 4 || !strings.Contains(errOut, "carol requested changes") {
83 t.Fatalf("request-changes block: exit %d, %s", code, errOut)
84 }
85
86 // Carol's latest review wins: her approval satisfies both the count
87 // and CODEOWNERS.
88 if _, _, code = inst.ssh(t, carolKey, "", "mr", "review", "alice/svc", "1", "--approve"); code != 0 {
89 t.Fatal("carol approve failed")
90 }
91
92 // require-resolved: an open thread still blocks; resolving unblocks.
93 if _, _, code = inst.ssh(t, aliceKey, "", "repo", "settings", "require-resolved", "alice/svc", "on"); code != 0 {
94 t.Fatal("require-resolved failed")
95 }
96 tout, _, code2 := inst.ssh(t, bobKey, "", "mr", "diff-comment", "alice/svc", "1",
97 "--path", "svc.go", "--line", "3", "--message", "'name it better'", "--json")
98 if code2 != 0 {
99 t.Fatal("diff-comment failed")
100 }
101 var tenv struct {
102 Data struct {
103 Thread int64 `json:"thread"`
104 } `json:"data"`
105 }
106 json.Unmarshal([]byte(tout), &tenv)
107 _, errOut, code = inst.ssh(t, aliceKey, "", "mr", "merge", "alice/svc", "1")
108 if code != 4 || !strings.Contains(errOut, "threads resolved") {
109 t.Fatalf("resolved gate: exit %d, %s", code, errOut)
110 }
111 if _, _, code = inst.ssh(t, bobKey, "", "mr", "resolve", "alice/svc", "1", fmt.Sprint(tenv.Data.Thread)); code != 0 {
112 t.Fatal("resolve failed")
113 }
114 if _, errOut, code = inst.ssh(t, aliceKey, "", "mr", "merge", "alice/svc", "1"); code != 0 {
115 t.Fatalf("fully gated merge: %s", errOut)
116 }
117
118 // Stale approvals never count: new MR, approve, force-push, refused.
119 mustGit(t, dir, env, "fetch", "-q", "origin")
120 mustGit(t, dir, env, "checkout", "-q", "-b", "feat2", "origin/main")
121 os.WriteFile(filepath.Join(dir, "notes.txt"), []byte("n\n"), 0o644)
122 mustGit(t, dir, env, "add", ".")
123 mustGit(t, dir, env, "commit", "-q", "-m", "notes")
124 mustGit(t, dir, env, "push", "-q", "origin", "feat2")
125 if _, _, code := inst.ssh(t, aliceKey, "", "mr", "create", "alice/svc",
126 "--source", "feat2", "--target", "main", "--title", "'notes'"); code != 0 {
127 t.Fatal("mr2 create failed")
128 }
129 if _, _, code = inst.ssh(t, bobKey, "", "mr", "review", "alice/svc", "2", "--approve"); code != 0 {
130 t.Fatal("bob approve 2 failed")
131 }
132 mustGit(t, dir, env, "commit", "-q", "--amend", "-m", "notes v2")
133 mustGit(t, dir, env, "push", "-q", "--force", "origin", "feat2")
134 _, errOut, code = inst.ssh(t, aliceKey, "", "mr", "merge", "alice/svc", "2")
135 if code != 4 || !strings.Contains(errOut, "requires 1 fresh approval") {
136 t.Fatalf("stale approval counted: exit %d, %s", code, errOut)
137 }
138}
internal/control/mr.go +157
@@ -4,6 +4,7 @@ import (
44 "errors"
55 "fmt"
66 "io"
7 "slices"
78 "strconv"
89 "strings"
910
@@ -16,6 +17,10 @@ import (
1617 func init() {
1718 register(Command{Path: []string{"repo", "fork"},
1819 Summary: "fork a repository under your account: repo fork <owner/name> [--name <n>]", Run: runRepoFork})
20 register(Command{Path: []string{"repo", "settings", "require-approvals"},
21 Summary: "require N fresh approvals to merge: repo settings require-approvals <owner/name> <n> (0 = off)", Run: runRequireApprovals})
22 register(Command{Path: []string{"repo", "settings", "require-resolved"},
23 Summary: "require all review threads resolved to merge: repo settings require-resolved <owner/name> on|off", Run: runRequireResolved})
1924 register(Command{Path: []string{"repo", "settings", "require-checks"},
2025 Summary: "gate merges on green statuses: repo settings require-checks <owner/name> on|off", Run: runRequireChecks})
2126 register(Command{Path: []string{"repo", "settings", "require-signed"},
@@ -99,6 +104,46 @@ func runRepoFork(c *Ctx, args []string) int {
99104 })
100105 }
101106
107func runRequireApprovals(c *Ctx, args []string) int {
108 if len(args) != 2 {
109 return c.fail(protocol.ExitUsage, "usage: repo settings require-approvals <owner/name> <n>")
110 }
111 n, err := strconv.Atoi(args[1])
112 if err != nil || n < 0 || n > 20 {
113 return c.fail(protocol.ExitUsage, "approvals must be 0..20")
114 }
115 repo, code := resolveRepo(c, args[0], policy.CanAdmin)
116 if code >= 0 {
117 return code
118 }
119 s := repo.Settings
120 s.RequireApprovals = n
121 if err := c.Store.SetRepoSettings(repo.ID, s); err != nil {
122 return c.fail(protocol.ExitFailure, "%v", err)
123 }
124 return c.emit(s, func(w io.Writer) {
125 fmt.Fprintf(w, "require_approvals %d on %s\n", n, repo.Path())
126 })
127}
128
129func runRequireResolved(c *Ctx, args []string) int {
130 if len(args) != 2 || (args[1] != "on" && args[1] != "off") {
131 return c.fail(protocol.ExitUsage, "usage: repo settings require-resolved <owner/name> on|off")
132 }
133 repo, code := resolveRepo(c, args[0], policy.CanAdmin)
134 if code >= 0 {
135 return code
136 }
137 s := repo.Settings
138 s.RequireResolved = args[1] == "on"
139 if err := c.Store.SetRepoSettings(repo.ID, s); err != nil {
140 return c.fail(protocol.ExitFailure, "%v", err)
141 }
142 return c.emit(s, func(w io.Writer) {
143 fmt.Fprintf(w, "require_resolved %s on %s\n", args[1], repo.Path())
144 })
145}
146
102147 func runRequireChecks(c *Ctx, args []string) int {
103148 if len(args) != 2 || (args[1] != "on" && args[1] != "off") {
104149 return c.fail(protocol.ExitUsage, "usage: repo settings require-checks <owner/name> on|off")
@@ -560,6 +605,11 @@ func runMRMerge(c *Ctx, args []string) int {
560605 }
561606 }
562607
608 // Review gates: approvals, CODEOWNERS, resolved threads.
609 if code := c.reviewGates(repo, mr, dir, targetSHA, headSHA); code >= 0 {
610 return code
611 }
612
563613 upToDate, err := gitutil.IsAncestor(dir, headSHA, targetSHA)
564614 if err != nil {
565615 return c.fail(protocol.ExitFailure, "%v", err)
@@ -764,6 +814,113 @@ func runMRMerge(c *Ctx, args []string) int {
764814 })
765815 }
766816
817// reviewGates enforces require_approvals (fresh, non-author, latest review
818// per reviewer; a fresh request-changes blocks), CODEOWNERS coverage, and
819// require_resolved. Returns -1 to proceed.
820func (c *Ctx) reviewGates(repo store.Repo, mr store.MR, dir, targetSHA, headSHA string) int {
821 set := repo.Settings
822 if set.RequireApprovals == 0 && !set.RequireResolved {
823 return -1
824 }
825
826 if set.RequireApprovals > 0 {
827 reviews, err := c.Store.ListMRReviews(mr.ID)
828 if err != nil {
829 return c.fail(protocol.ExitFailure, "%v", err)
830 }
831 // Latest fresh review per reviewer decides their stance.
832 latest := map[string]string{}
833 for _, r := range reviews {
834 if r.Stale || r.Reviewer == mr.Author {
835 continue
836 }
837 latest[r.Reviewer] = r.Verdict
838 }
839 var approvers []string
840 var blockers []string
841 for who, verdict := range latest {
842 switch verdict {
843 case "approve":
844 approvers = append(approvers, who)
845 case "request_changes":
846 blockers = append(blockers, who)
847 }
848 }
849 if len(blockers) > 0 {
850 slices.Sort(blockers)
851 return c.fail(protocol.ExitDenied,
852 "%s requested changes on !%d; resolve their review before merging", strings.Join(blockers, ", "), mr.Number)
853 }
854 if len(approvers) < set.RequireApprovals {
855 return c.fail(protocol.ExitDenied,
856 "%s requires %d fresh approval(s); !%d has %d", repo.Path(), set.RequireApprovals, mr.Number, len(approvers))
857 }
858
859 // CODEOWNERS: every owned changed file needs an approval from one
860 // of its owners.
861 content, err := gitutil.ReadBlob(dir, "refs/heads/"+mr.TargetRef, "CODEOWNERS", 1<<20)
862 if err != nil {
863 content, err = gitutil.ReadBlob(dir, "refs/heads/"+mr.TargetRef, ".gitbay/CODEOWNERS", 1<<20)
864 }
865 if err == nil && len(content) > 0 {
866 rules := policy.ParseCodeowners(string(content))
867 base, err := gitutil.MergeBase(dir, targetSHA, headSHA)
868 if err != nil {
869 return c.fail(protocol.ExitFailure, "%v", err)
870 }
871 files, err := gitutil.DiffFiles(dir, base, headSHA)
872 if err != nil {
873 return c.fail(protocol.ExitFailure, "%v", err)
874 }
875 approved := map[string]bool{}
876 for _, a := range approvers {
877 approved[a] = true
878 }
879 missing := map[string][]string{} // owner-set key -> example paths
880 for _, f := range files {
881 owners := policy.OwnersFor(rules, f)
882 if owners == nil {
883 continue
884 }
885 ok := false
886 for _, o := range owners {
887 if approved[o] {
888 ok = true
889 break
890 }
891 }
892 if !ok {
893 key := strings.Join(owners, ",")
894 if len(missing[key]) < 3 {
895 missing[key] = append(missing[key], f)
896 }
897 }
898 }
899 if len(missing) > 0 {
900 var parts []string
901 for owners, paths := range missing {
902 parts = append(parts, fmt.Sprintf("%s (owned by %s)", strings.Join(paths, ", "), owners))
903 }
904 slices.Sort(parts)
905 return c.fail(protocol.ExitDenied,
906 "CODEOWNERS approval missing for: %s", strings.Join(parts, "; "))
907 }
908 }
909 }
910
911 if set.RequireResolved {
912 n, err := c.Store.UnresolvedThreadCount(mr.ID)
913 if err != nil {
914 return c.fail(protocol.ExitFailure, "%v", err)
915 }
916 if n > 0 {
917 return c.fail(protocol.ExitDenied,
918 "%s requires review threads resolved; !%d has %d open (mr threads %s %d)", repo.Path(), mr.Number, n, repo.Path(), mr.Number)
919 }
920 }
921 return -1
922}
923
767924 func runMRClose(c *Ctx, args []string) int {
768925 repo, mr, code := mrRef(c, args, policy.CanRead)
769926 if code >= 0 {
internal/policy/codeowners.go added +79
@@ -0,0 +1,79 @@
1package policy
2
3import (
4 "path"
5 "strings"
6)
7
8// CodeownersRule is one line of a CODEOWNERS file: a pattern and the users
9// who own paths matching it.
10type CodeownersRule struct {
11 Pattern string
12 Owners []string // usernames, @ stripped
13}
14
15// ParseCodeowners reads CODEOWNERS content: one rule per line, gitignore-
16// style pattern followed by @user owners; #-comments and blanks ignored.
17func ParseCodeowners(content string) []CodeownersRule {
18 var rules []CodeownersRule
19 for _, line := range strings.Split(content, "\n") {
20 line = strings.TrimSpace(line)
21 if line == "" || strings.HasPrefix(line, "#") {
22 continue
23 }
24 fields := strings.Fields(line)
25 if len(fields) < 2 {
26 continue
27 }
28 var owners []string
29 for _, f := range fields[1:] {
30 owners = append(owners, strings.TrimPrefix(f, "@"))
31 }
32 rules = append(rules, CodeownersRule{Pattern: fields[0], Owners: owners})
33 }
34 return rules
35}
36
37// OwnersFor returns the owners of a path: the last matching rule wins,
38// CODEOWNERS convention. nil means unowned.
39func OwnersFor(rules []CodeownersRule, filePath string) []string {
40 var owners []string
41 for _, r := range rules {
42 if codeownersMatch(r.Pattern, filePath) {
43 owners = r.Owners
44 }
45 }
46 return owners
47}
48
49// codeownersMatch implements the pattern subset gitbay supports:
50// - "*" everything
51// - "*.go" extension match on the basename, anywhere
52// - "docs/" directory prefix (anywhere unless anchored with /)
53// - "/cmd/x.go" exact or glob path anchored at the root
54// - "internal/*" glob against the full path (one segment per *)
55func codeownersMatch(pattern, filePath string) bool {
56 anchored := strings.HasPrefix(pattern, "/")
57 pattern = strings.TrimPrefix(pattern, "/")
58 if pattern == "*" {
59 return true
60 }
61 // Directory rule: everything under it.
62 if strings.HasSuffix(pattern, "/") {
63 dir := strings.TrimSuffix(pattern, "/")
64 if strings.HasPrefix(filePath, dir+"/") {
65 return true
66 }
67 if !anchored && strings.Contains(filePath, "/"+dir+"/") {
68 return true
69 }
70 return false
71 }
72 // Bare pattern without a slash: match the basename anywhere.
73 if !strings.Contains(pattern, "/") && !anchored {
74 ok, _ := path.Match(pattern, path.Base(filePath))
75 return ok
76 }
77 ok, _ := path.Match(pattern, filePath)
78 return ok
79}
internal/policy/codeowners_test.go added +45
@@ -0,0 +1,45 @@
1package policy
2
3import (
4 "reflect"
5 "testing"
6)
7
8func TestCodeowners(t *testing.T) {
9 rules := ParseCodeowners(`
10# comment
11* @alice
12*.go @bob @carol
13docs/ @dana
14/deploy/ @erin
15/cmd/gitbay/main.go @frank
16internal/* @grace
17`)
18 cases := []struct {
19 path string
20 want []string
21 }{
22 {"README.org", []string{"alice"}},
23 {"x/y/z.txt", []string{"alice"}},
24 {"main.go", []string{"bob", "carol"}},
25 {"deep/nested/thing.go", []string{"bob", "carol"}},
26 {"docs/users.org", []string{"dana"}},
27 {"sub/docs/x.md", []string{"dana"}}, // unanchored dir matches anywhere
28 {"deploy/cloud-init.yaml", []string{"erin"}}, // anchored dir
29 {"cmd/gitbay/main.go", []string{"frank"}}, // exact anchored path beats *.go (later rule)
30 {"internal/policy", []string{"grace"}}, // single-segment glob
31 }
32 for _, tc := range cases {
33 if got := OwnersFor(rules, tc.path); !reflect.DeepEqual(got, tc.want) {
34 t.Errorf("OwnersFor(%q) = %v, want %v", tc.path, got, tc.want)
35 }
36 }
37 // Later rules win: a .go file under docs/ belongs to dana, not bob.
38 if got := OwnersFor(rules, "docs/gen.go"); !reflect.DeepEqual(got, []string{"dana"}) {
39 t.Errorf("last-match-wins failed: %v", got)
40 }
41 // Anchored dir does not match nested occurrences.
42 if got := OwnersFor(rules, "x/deploy/f"); !reflect.DeepEqual(got, []string{"alice"}) {
43 t.Errorf("anchored dir leaked: %v", got)
44 }
45}
internal/store/repos.go +2
@@ -24,6 +24,8 @@ type RepoSettings struct {
2424 ProtectedBranches []string `json:"protected_branches,omitempty"`
2525 RequireSignedCommits bool `json:"require_signed_commits,omitempty"`
2626 RequireChecks bool `json:"require_checks,omitempty"`
27 RequireApprovals int `json:"require_approvals,omitempty"`
28 RequireResolved bool `json:"require_resolved,omitempty"`
2729 GitDaemon bool `json:"git_daemon,omitempty"`
2830 }
2931