krz/gitbay

A CLI-first git forge.

cli forge git self-hosted

https://gitbay.org

Commit 235f40647d

235f40647d539a1d91e4fddecfd70276285ec5fe

parent: 7ad3de789e

Verified · cmc ci/build: success

cmc <hello@cleberg.net> · 2026-08-26T02:41:56Z 

web: review, resolve, merge, and close on the merge request page

Web writes dispatch the same control commands the CLI runs, through the
registry the JSON API already uses, so merge gates, review rules, and
audit entries keep one implementation. The dispatcher runs them as the
session user with ViaAPI set, which refuses SSHOnly commands: anything
whose input is a credential stays on SSH.

The aside gains review verdicts, a merge control with a strategy
choice, and close, all gated on write access; review threads gain
resolve and reopen on the diff view. A refused action redirects back
with the command's own message — the merge gates explain themselves.

Ref #35
e2e/mrweb_test.go added +157
@@ -0,0 +1,157 @@
1package e2e
2
3import (
4 "encoding/json"
5 "net/http"
6 "net/url"
7 "os"
8 "path/filepath"
9 "regexp"
10 "strings"
11 "testing"
12)
13
14// login returns a browser holding a session for the given key's account.
15func (i *instance) login(t *testing.T, key string) *http.Client {
16 t.Helper()
17 out, errOut, code := i.ssh(t, key, "", "web", "login", "--json")
18 if code != 0 {
19 t.Fatalf("web login: %s", errOut)
20 }
21 var env struct {
22 Data struct {
23 URL string `json:"url"`
24 } `json:"data"`
25 }
26 json.Unmarshal([]byte(out), &env)
27 c := newBrowser(t)
28 path := env.Data.URL[strings.Index(env.Data.URL, "/login"):]
29 if status, _ := browserGet(t, c, i.base()+path); status != 200 {
30 t.Fatalf("login landed: %d", status)
31 }
32 return c
33}
34
35// TestMRWebReviewLoop drives review, thread resolution, and merge from the
36// browser. Every action runs the same control command the CLI runs, so the
37// test also proves the merge gates apply to web merges.
38func TestMRWebReviewLoop(t *testing.T) {
39 inst := startInstanceWith(t, "[web]\nmode = \"accounts\"\n")
40 aliceKey := inst.newKey(t, "alice")
41 bobKey := inst.newKey(t, "bob")
42 inst.admin(t, "admin", "user", "create", "alice",
43 "--key", aliceKey+".pub", "--email", "alice@example.test", "--verified")
44 inst.admin(t, "admin", "user", "create", "bob",
45 "--key", bobKey+".pub", "--email", "bob@example.test", "--verified")
46
47 if _, errOut, code := inst.ssh(t, aliceKey, "", "repo", "create", "alice/lib"); code != 0 {
48 t.Fatalf("repo create: %s", errOut)
49 }
50 if _, errOut, code := inst.ssh(t, aliceKey, "", "repo", "access", "grant", "alice/lib", "bob", "write"); code != 0 {
51 t.Fatalf("grant: %s", errOut)
52 }
53 // Unresolved review threads block merges, so the gate is observable.
54 if _, errOut, code := inst.ssh(t, aliceKey, "", "repo", "settings", "require-resolved", "alice/lib", "on"); code != 0 {
55 t.Fatalf("require-resolved: %s", errOut)
56 }
57
58 env := inst.gitEnv(aliceKey)
59 work := t.TempDir()
60 mustGit(t, work, env, "clone", inst.sshURL("alice/lib"), "w")
61 dir := filepath.Join(work, "w")
62 os.WriteFile(filepath.Join(dir, "lib.txt"), []byte("v1\n"), 0o644)
63 mustGit(t, dir, env, "checkout", "-q", "-b", "main")
64 mustGit(t, dir, env, "add", ".")
65 mustGit(t, dir, env, "commit", "-q", "-m", "base")
66 mustGit(t, dir, env, "push", "-q", "origin", "main")
67
68 // Bob proposes a change and leaves a review thread on it.
69 bobEnv := inst.gitEnv(bobKey)
70 bobWork := t.TempDir()
71 mustGit(t, bobWork, bobEnv, "clone", inst.sshURL("alice/lib"), "w")
72 bobDir := filepath.Join(bobWork, "w")
73 mustGit(t, bobDir, bobEnv, "checkout", "-q", "-b", "feature", "origin/main")
74 os.WriteFile(filepath.Join(bobDir, "feature.txt"), []byte("bob's work\n"), 0o644)
75 mustGit(t, bobDir, bobEnv, "add", ".")
76 mustGit(t, bobDir, bobEnv, "commit", "-q", "-m", "add feature")
77 mustGit(t, bobDir, bobEnv, "push", "-q", "origin", "feature")
78 if _, errOut, code := inst.ssh(t, bobKey, "", "mr", "create", "alice/lib",
79 "--source", "feature", "--target", "main", "--title", "'add feature'"); code != 0 {
80 t.Fatalf("mr create: %s", errOut)
81 }
82 if _, errOut, code := inst.ssh(t, bobKey, "", "mr", "diff-comment", "alice/lib", "1",
83 "--path", "feature.txt", "--line", "1", "--message", "'is this right?'"); code != 0 {
84 t.Fatalf("diff-comment: %s", errOut)
85 }
86
87 mrURL := inst.base() + "/alice/lib/mrs/1"
88 alice := inst.login(t, aliceKey)
89
90 // The controls are on the page, and carry the thread to resolve.
91 _, body := browserGet(t, alice, mrURL)
92 for _, want := range []string{`value="approve"`, `action="/alice/lib/mrs/1/merge"`} {
93 if !strings.Contains(body, want) {
94 t.Fatalf("MR page missing %q", want)
95 }
96 }
97 // Review threads live on the diff view, where their lines are.
98 _, diffBody := browserGet(t, alice, mrURL+"?view=diff")
99 m := regexp.MustCompile(`name="thread" value="(\d+)"`).FindStringSubmatch(diffBody)
100 if m == nil {
101 t.Fatalf("no thread control on the diff view:\n%s", diffBody)
102 }
103 threadID := m[1]
104
105 // Approve from the browser; the CLI sees the review.
106 if status, _ := browserPost(t, alice, mrURL+"/review", url.Values{"verdict": {"approve"}}); status != 200 {
107 t.Fatalf("review post: %d", status)
108 }
109 show := inst.mrShow(t, aliceKey, "alice/lib", "1")
110 if len(show.Reviews) != 1 || show.Reviews[0].Reviewer != "alice" || show.Reviews[0].Verdict != "approve" {
111 t.Fatalf("review not recorded: %+v", show.Reviews)
112 }
113
114 // Merging is refused while the thread is open, and the page says why.
115 _, body = browserPost(t, alice, mrURL+"/merge", url.Values{"strategy": {"auto"}})
116 if !strings.Contains(body, "unresolved") {
117 t.Fatalf("merge gate not surfaced:\n%s", body)
118 }
119 if st := inst.mrShow(t, aliceKey, "alice/lib", "1").State; st != "open" {
120 t.Fatalf("blocked merge changed state to %s", st)
121 }
122
123 // Resolve the thread, then merge.
124 if status, _ := browserPost(t, alice, mrURL+"/thread",
125 url.Values{"thread": {threadID}, "action": {"resolve"}}); status != 200 {
126 t.Fatalf("resolve post: %d", status)
127 }
128 out, _, _ := inst.ssh(t, aliceKey, "", "mr", "threads", "alice/lib", "1")
129 if !strings.Contains(out, "resolved") {
130 t.Fatalf("thread not resolved:\n%s", out)
131 }
132 if status, _ := browserPost(t, alice, mrURL+"/merge", url.Values{"strategy": {"auto"}}); status != 200 {
133 t.Fatalf("merge post: %d", status)
134 }
135 if st := inst.mrShow(t, aliceKey, "alice/lib", "1").State; st != "merged" {
136 t.Fatalf("MR state after web merge: %s", st)
137 }
138 mustGit(t, dir, env, "pull", "-q", "origin", "main")
139 if _, err := os.Stat(filepath.Join(dir, "feature.txt")); err != nil {
140 t.Fatal("merged content missing from main")
141 }
142
143 // Readers get no controls, and a forged POST is refused by the command.
144 _, anon := browserGet(t, newBrowser(t), mrURL)
145 if strings.Contains(anon, `value="approve"`) {
146 t.Fatal("anonymous visitor sees review controls")
147 }
148 carol := inst.newKey(t, "carol")
149 inst.admin(t, "admin", "user", "create", "carol", "--key", carol+".pub")
150 if _, errOut, code := inst.ssh(t, carol, "", "repo", "create", "carol/own"); code != 0 {
151 t.Fatalf("carol repo: %s", errOut)
152 }
153 _, denied := browserPost(t, inst.login(t, carol), mrURL+"/close", url.Values{})
154 if !strings.Contains(denied, `class="error"`) || !strings.Contains(denied, "write access") {
155 t.Fatalf("reader was not refused:\n%s", denied)
156 }
157}
internal/httpd/control.go added +38
@@ -0,0 +1,38 @@
1package httpd
2
3import (
4 "bytes"
5 "strings"
6
7 "gitbay.org/gitbay/internal/control"
8 "gitbay.org/gitbay/internal/protocol"
9 "gitbay.org/gitbay/internal/store"
10)
11
12// runControl executes a control command as the browser session's user,
13// through the same registry the CLI and the JSON API reach. Web writes
14// never reimplement command logic — merge gates, review rules, and audit
15// entries stay in one place — so the surfaces cannot drift apart.
16//
17// ViaAPI is set, which refuses SSHOnly commands: anything whose input is a
18// credential (secrets, mirror tokens, session minting) stays on SSH.
19func (s *Server) runControl(u store.User, argv []string) (out string, msg string, ok bool) {
20 var stdout, stderr bytes.Buffer
21 ctx := &control.Ctx{
22 User: u,
23 Source: "web",
24 Scope: "full",
25 Store: s.st,
26 Cfg: s.cfg,
27 Stdin: strings.NewReader(""),
28 Stdout: &stdout,
29 Stderr: &stderr,
30 ViaAPI: true,
31 }
32 code := control.Dispatch(ctx, argv)
33 m := strings.TrimSpace(stderr.String())
34 if m == "" {
35 m = strings.TrimSpace(stdout.String())
36 }
37 return stdout.String(), m, code == protocol.ExitOK
38}
internal/httpd/mractions.go added +91
@@ -0,0 +1,91 @@
1package httpd
2
3import (
4 "fmt"
5 "net/http"
6 "net/url"
7 "strconv"
8 "strings"
9
10 "gitbay.org/gitbay/internal/store"
11)
12
13// Merge request actions. Each one runs the control command the CLI runs,
14// so review rules, merge gates, and audit entries have a single
15// implementation; the browser only chooses arguments and shows the
16// result.
17
18// mrRedirect returns to the merge request, carrying a failure message the
19// page renders as a banner.
20func (s *Server) mrRedirect(w http.ResponseWriter, r *http.Request, msg string) {
21 dest := fmt.Sprintf("/%s/%s/mrs/%s",
22 r.PathValue("owner"), r.PathValue("repo"), r.PathValue("n"))
23 if msg != "" {
24 if len(msg) > 300 {
25 msg = msg[:300]
26 }
27 dest += "?e=" + url.QueryEscape(msg)
28 }
29 http.Redirect(w, r, dest, http.StatusSeeOther)
30}
31
32// mrArgs builds "<verb> owner/name <n>" for the mr command family.
33func mrArgs(r *http.Request, verb string, extra ...string) []string {
34 repo := r.PathValue("owner") + "/" + r.PathValue("repo")
35 return append([]string{"mr", verb, repo, r.PathValue("n")}, extra...)
36}
37
38func (s *Server) mrReviewSubmit(w http.ResponseWriter, r *http.Request, u store.User) {
39 flag := map[string]string{
40 "approve": "--approve",
41 "request-changes": "--request-changes",
42 "comment": "--comment",
43 }[r.FormValue("verdict")]
44 if flag == "" {
45 s.mrRedirect(w, r, "pick approve, request changes, or comment")
46 return
47 }
48 _, msg, ok := s.runControl(u, mrArgs(r, "review", flag))
49 if ok {
50 msg = ""
51 }
52 s.mrRedirect(w, r, msg)
53}
54
55func (s *Server) mrMergeSubmit(w http.ResponseWriter, r *http.Request, u store.User) {
56 args := []string{}
57 if st := strings.TrimSpace(r.FormValue("strategy")); st != "" && st != "auto" {
58 args = append(args, "--strategy", st)
59 }
60 _, msg, ok := s.runControl(u, mrArgs(r, "merge", args...))
61 if ok {
62 msg = ""
63 }
64 s.mrRedirect(w, r, msg)
65}
66
67func (s *Server) mrCloseSubmit(w http.ResponseWriter, r *http.Request, u store.User) {
68 _, msg, ok := s.runControl(u, mrArgs(r, "close"))
69 if ok {
70 msg = ""
71 }
72 s.mrRedirect(w, r, msg)
73}
74
75// mrThreadSubmit resolves or reopens one review thread.
76func (s *Server) mrThreadSubmit(w http.ResponseWriter, r *http.Request, u store.User) {
77 verb := "resolve"
78 if r.FormValue("action") == "unresolve" {
79 verb = "unresolve"
80 }
81 id := strings.TrimSpace(r.FormValue("thread"))
82 if _, err := strconv.ParseInt(id, 10, 64); err != nil {
83 s.mrRedirect(w, r, "bad thread id")
84 return
85 }
86 _, msg, ok := s.runControl(u, mrArgs(r, verb, id))
87 if ok {
88 msg = ""
89 }
90 s.mrRedirect(w, r, msg)
91}
internal/httpd/routes.go +9
@@ -114,6 +114,15 @@ func (s *Server) Routes() []Route {
114114 Handler: s.checkOrigin(s.requireUser(s.mrEditSubmit))},
115115 Route{Method: "POST", Pattern: "/{owner}/{repo}/mrs/{n}/comment", Mutating: true,
116116 Handler: s.checkOrigin(s.requireUser(s.mrCommentSubmit))},
117 // Review loop: each runs the matching mr command.
118 Route{Method: "POST", Pattern: "/{owner}/{repo}/mrs/{n}/review", Mutating: true,
119 Handler: s.checkOrigin(s.requireUser(s.mrReviewSubmit))},
120 Route{Method: "POST", Pattern: "/{owner}/{repo}/mrs/{n}/merge", Mutating: true,
121 Handler: s.checkOrigin(s.requireUser(s.mrMergeSubmit))},
122 Route{Method: "POST", Pattern: "/{owner}/{repo}/mrs/{n}/close", Mutating: true,
123 Handler: s.checkOrigin(s.requireUser(s.mrCloseSubmit))},
124 Route{Method: "POST", Pattern: "/{owner}/{repo}/mrs/{n}/thread", Mutating: true,
125 Handler: s.checkOrigin(s.requireUser(s.mrThreadSubmit))},
117126 Route{Method: "GET", Pattern: "/{owner}/{repo}/edit/{ref}/{path...}",
118127 Handler: s.requireUser(s.editForm)},
119128 Route{Method: "POST", Pattern: "/{owner}/{repo}/edit/{ref}/{path...}", Mutating: true,
internal/httpd/web.go +20 −1
@@ -1361,6 +1361,20 @@ func (s *Server) issue(w http.ResponseWriter, r *http.Request) {
13611361 }
13621362
13631363 // canEditItem: the author or anyone with write access may edit.
1364// canWriteRepo reports whether the browser session may push to the repo,
1365// which is what gates the review and merge controls.
1366func (s *Server) canWriteRepo(r *http.Request, repo store.Repo) bool {
1367 if s.cfg.Web.Mode != "accounts" {
1368 return false
1369 }
1370 u := s.viewer(r)
1371 if u.ID == 0 {
1372 return false
1373 }
1374 grant, _ := s.st.AccessRole(repo.ID, u.ID)
1375 return policy.CanWrite(u, repo, grant)
1376}
1377
13641378 func (s *Server) canEditItem(r *http.Request, repo store.Repo, author string) bool {
13651379 if s.cfg.Web.Mode != "accounts" {
13661380 return false
@@ -1480,6 +1494,7 @@ func (s *Server) mr(w http.ResponseWriter, r *http.Request) {
14801494 // The diff is the reason most people open a merge request, so it gets
14811495 // its own view rather than a fold at the foot of the conversation.
14821496 // A query parameter keeps this working without JavaScript.
1497 unresolved, _ := s.st.UnresolvedThreadCount(m.ID)
14831498 view := r.URL.Query().Get("view")
14841499 if view != "commits" && view != "diff" {
14851500 view = "conversation"
@@ -1497,9 +1512,13 @@ func (s *Server) mr(w http.ResponseWriter, r *http.Request) {
14971512 Stat diffStat
14981513 Commits []commitRow
14991514 CanEdit bool
1515 CanWrite bool
1516 Unresolved int
1517 Notice string
15001518 DetachedThreads []diffThread
15011519 }{p, m, view, md(m.Body), checks, store.CombinedStatus(checks), renderComments(comments, md),
1502 reviews, lines, stat, commits, s.canEditItem(r, p.Repo, m.Author), detachedThreads})
1520 reviews, lines, stat, commits, s.canEditItem(r, p.Repo, m.Author),
1521 s.canWriteRepo(r, p.Repo), unresolved, r.URL.Query().Get("e"), detachedThreads})
15031522 }
15041523
15051524 func (s *Server) refs(w http.ResponseWriter, r *http.Request) {
internal/web/static/style.css +11
@@ -540,6 +540,17 @@ code.fullsha { color: var(--muted); overflow-wrap: anywhere; }
540540 padding: var(--sp-2) var(--sp-3);
541541 margin-bottom: var(--sp-4);
542542 }
543/* merge request actions in the aside: stacked controls, full width */
544.aside form.actions {
545 display: flex;
546 flex-wrap: wrap;
547 gap: var(--sp-2);
548 margin-top: var(--sp-2);
549}
550.aside form.actions button { flex: 1 1 auto; }
551.aside form.actions select { width: 100%; }
552form.threadact { margin: var(--sp-1) 0 0; padding: 0 var(--sp-3) var(--sp-2); }
553
543554 .meta { color: var(--muted); font-size: var(--fs-1); }
544555 svg.icon { vertical-align: -0.125em; }
545556 .lede { font-size: var(--fs-3); margin: var(--sp-2) 0; }
internal/web/templates/mr.html +33 −2
@@ -5,6 +5,8 @@
55 <p class="issuemeta"><span class="chip chip-{{.MR.State}}">{{.MR.State}}</span>
66 <a href="/{{.MR.Author}}">{{.MR.Author}}</a> wants to merge <code>{{if .MR.SourcePath}}{{.MR.SourcePath}}:{{end}}{{.MR.SourceRef}}</code> into <code>{{.MR.TargetRef}}</code></p>
77
8{{if .Notice}}<p class="error" role="alert">{{.Notice}}</p>{{end}}
9
810 <div class="withaside">
911 <div class="mainside">
1012
@@ -33,7 +35,7 @@
3335 </article>{{end}}
3436 {{end}}
3537 {{if .DetachedThreads}}<h2>Threads on earlier revisions</h2>
36{{range .DetachedThreads}}<div class="thread stale"><p class="threadstate">{{if .Stale}}stale{{end}}{{if .Resolved}}{{if .Stale}} · {{end}}resolved by {{.Resolved}}{{end}}</p>{{range .Comments}}<p class="commenthead"><strong>{{.Author}}</strong> <span class="when">{{when .CreatedAt}}</span></p><div class="rendered">{{.BodyHTML}}</div>{{end}}</div>{{end}}{{end}}
38{{range .DetachedThreads}}<div class="thread stale"><p class="threadstate">{{if .Stale}}stale{{end}}{{if .Resolved}}{{if .Stale}} · {{end}}resolved by {{.Resolved}}{{end}}</p>{{range .Comments}}<p class="commenthead"><strong>{{.Author}}</strong> <span class="when">{{when .CreatedAt}}</span></p><div class="rendered">{{.BodyHTML}}</div>{{end}}{{template "threadact" dict "ID" .ID "Resolved" .Resolved "Base" $base "Can" $.CanWrite}}</div>{{end}}{{end}}
3739 {{if .Viewer}}
3840 <form method="post" action="{{$base}}/comment" class="commentform">
3941 <p><textarea name="body" aria-label="Comment" rows="4" placeholder="Comment as {{.Viewer}}"></textarea></p>
@@ -60,12 +62,39 @@
6062 {{else}}
6163 <p class="diffstat">{{.Stat.Files}} file{{if ne .Stat.Files 1}}s{{end}} changed, <span class="add">+{{.Stat.Adds}}</span> <span class="del">−{{.Stat.Dels}}</span></p>
6264 <pre class="diff">{{range .DiffLines}}<span class="{{.Class}}">{{.Text}}</span>
63{{range .Threads}}</pre><div class="thread{{if .Resolved}} resolved{{end}}">{{if .Resolved}}<p class="threadstate">resolved by {{.Resolved}}</p>{{end}}{{range .Comments}}<p class="commenthead"><strong>{{.Author}}</strong> <span class="when">{{when .CreatedAt}}</span></p><div class="rendered">{{.BodyHTML}}</div>{{end}}</div><pre class="diff">{{end}}{{end}}</pre>
65{{range .Threads}}</pre><div class="thread{{if .Resolved}} resolved{{end}}">{{if .Resolved}}<p class="threadstate">resolved by {{.Resolved}}</p>{{end}}{{range .Comments}}<p class="commenthead"><strong>{{.Author}}</strong> <span class="when">{{when .CreatedAt}}</span></p><div class="rendered">{{.BodyHTML}}</div>{{end}}{{template "threadact" dict "ID" .ID "Resolved" .Resolved "Base" $base "Can" $.CanWrite}}</div><pre class="diff">{{end}}{{end}}</pre>
6466 {{end}}
6567
6668 </div>
6769
6870 <aside class="aside">
71 {{if and .CanWrite (or (eq .MR.State "open") (eq .MR.State "source_gone"))}}
72 <div class="grp">
73 <h2>Review</h2>
74 <form method="post" action="{{$base}}/review" class="actions">
75 <button type="submit" name="verdict" value="approve">Approve</button>
76 <button type="submit" name="verdict" value="request-changes">Request changes</button>
77 </form>
78 </div>
79 <div class="grp">
80 <h2>Merge</h2>
81 {{if .Unresolved}}<p class="row none">{{.Unresolved}} unresolved thread{{if ne .Unresolved 1}}s{{end}}</p>{{end}}
82 <form method="post" action="{{$base}}/merge" class="actions">
83 <label class="none" for="strategy">Strategy</label>
84 <select id="strategy" name="strategy">
85 <option value="auto">Automatic</option>
86 <option value="ff">Fast-forward</option>
87 <option value="merge">Merge commit</option>
88 <option value="squash">Squash</option>
89 <option value="rebase">Rebase</option>
90 </select>
91 <button type="submit" class="primary">Merge</button>
92 </form>
93 <form method="post" action="{{$base}}/close" class="actions">
94 <button type="submit">Close without merging</button>
95 </form>
96 </div>
97 {{end}}
6998 <div class="grp">
7099 <h2>Reviews</h2>
71100 {{range .Reviews}}<p class="row"><span class="dot {{if eq .Verdict "approve"}}ok{{else}}pend{{end}}"></span><a href="/{{.Reviewer}}">{{.Reviewer}}</a> {{.Verdict}}{{if .Stale}} <span class="chip chip-stale">stale</span>{{end}}</p>
@@ -89,3 +118,5 @@
89118 </aside>
90119 </div>
91120 {{end}}
121
122{{define "threadact"}}{{if .Can}}<form method="post" action="{{.Base}}/thread" class="threadact"><input type="hidden" name="thread" value="{{.ID}}"><button type="submit" name="action" value="{{if .Resolved}}unresolve{{else}}resolve{{end}}" class="linklike">{{if .Resolved}}Reopen thread{{else}}Resolve thread{{end}}</button></form>{{end}}{{end}}
internal/web/web.go +10
@@ -76,6 +76,16 @@ var funcs = template.FuncMap{
7676 }
7777 return s
7878 },
79 // dict builds a map for {{template}} calls that need several values.
80 "dict": func(pairs ...any) map[string]any {
81 m := map[string]any{}
82 for i := 0; i+1 < len(pairs); i += 2 {
83 if k, ok := pairs[i].(string); ok {
84 m[k] = pairs[i+1]
85 }
86 }
87 return m
88 },
7989 "add": func(a, b int) int { return a + b },
8090 "sub": func(a, b int) int { return a - b },
8191 // topTab maps a page's Tab to the repo header tab that should read as