| @@ -0,0 +1,688 @@ |
| 1 | # Web UI/UX sweep: implementation plan |
| 2 | |
| 3 | > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. |
| 4 | |
| 5 | **Goal:** Fix the seventeen findings posted on #182 under the rules in the spec, one commit per pattern, on one branch. Closes #182. |
| 6 | |
| 7 | **Architecture:** Every change is in the web layer (`internal/httpd`, `internal/web/templates`, `internal/web/static/style.css`) except three message rewrites at their source in `internal/control` and one sort helper in `internal/gitutil`. No new commands, no schema change. Web forms keep dispatching control commands; the typed confirmation is a check in the handler before dispatch. |
| 8 | |
| 9 | **Tech Stack:** Go, Go `html/template`, the control registry, the e2e harness in `e2e/` (real sshd and HTTP against a temp instance; `startInstanceWith(t, "[web]\nmode = \"accounts\"\n")`, `inst.login(t, key)`, `browserGet`, `browserPost`, `inst.get`). |
| 10 | |
| 11 | **Spec:** `docs/specs/2026-09-11-web-ux-sweep-design.md` |
| 12 | |
| 13 | ## Global Constraints |
| 14 | |
| 15 | - No JavaScript in templates: the instance CSP is `script-src 'none'`. |
| 16 | - Every `<input>`/`<textarea>`/`<select>` a person uses carries an `aria-label` or a `<label for>` (`internal/httpd/inputlabels_test.go`); one `<h1>` per page. |
| 17 | - Every `Mutating: true` route stays wrapped in `checkOrigin`; no new routes in this plan. |
| 18 | - Web handlers never reimplement a rule; a refused command's message reaches the page through the flash (`s.setFlash` / `s.done` / `backTo`). |
| 19 | - Never mention an assistant or model anywhere: commit messages, comments, docs. |
| 20 | - Commit messages: imperative subject, a body only where a why is needed, `Ref #182` as the last line; the docs task's commit ends `Closes #182`. |
| 21 | - Run locally: `go build ./... && go vet ./...`, `go test ./internal/httpd ./internal/web ./internal/control ./internal/gitutil`, and only the e2e tests the task names. CI on bay1 runs the whole suite. |
| 22 | - Before changing any user-visible string, grep `e2e/` and `internal/httpd/*_test.go` for it and update the assertions in the same commit; a task's step says which strings. |
| 23 | - Plain-sentence comments; match the surrounding code. |
| 24 | - Work on branch `ux-sweep` in the worktree `/Users/cmc/git/krz/gitbay-ux`, which already holds the spec. |
| 25 | |
| 26 | Facts every task can rely on (from reading the tree at 3e09d58): |
| 27 | |
| 28 | - `s.setFlash(w, msg)` / `s.takeFlash(w, r)` / `s.clearCookie(name, sameSite)` are in `internal/httpd/flash.go`; pages render the flash as `{{if .Notice}}<p class="error" role="alert">{{.Notice}}</p>{{end}}`. |
| 29 | - `s.done(w, r, code, msg, redirectFn)` in `internal/httpd/control.go:57`; `s.backTo(w, r, page, msg)` in `internal/httpd/releaseactions.go:15` redirects to `/{owner}/{repo}/{page}` with the flash. |
| 30 | - Template helpers live in `internal/web/web.go` (`funcs`, line 62): `when(s string) string` parses RFC3339Nano and formats `2006-01-02 15:04`; `ago(t time.Time) string` is relative. |
| 31 | - `repoPage` is built in `repoFor` (`internal/httpd/web.go:289`, fields set around line 341: `Host: s.cfg.SiteHost()`, `CloneURL: s.cfg.Server.SiteURL + "/" + repo.Path() + ".git"`). |
| 32 | |
| 33 | --- |
| 34 | |
| 35 | ### Task 1: Typed confirmation on destructive controls |
| 36 | |
| 37 | **Files:** |
| 38 | - Create: `internal/httpd/confirm.go` |
| 39 | - Modify: `internal/web/templates/account.html:34,64,90`; `internal/httpd/account.go:150-182` |
| 40 | - Modify: `internal/web/templates/releases.html:35-38`; `internal/httpd/releaseactions.go:34-38` |
| 41 | - Modify: `internal/web/templates/snippet.html:22-25,46-48`; `internal/httpd/snippets.go:215,227` |
| 42 | - Modify: `internal/web/templates/owner.html:98-102`; `internal/httpd/orgweb.go:82` |
| 43 | - Modify: `internal/web/templates/labels.html:15-19`; `internal/httpd/labels.go:50-53` |
| 44 | - Modify: `internal/web/static/style.css` (one rule) |
| 45 | - Test: `e2e/accountweb_test.go:78`, `e2e/releaseweb_test.go:102`, `e2e/snippetweb_test.go:141,148,173`, `e2e/labelweb_test.go:56`, `e2e/orgweb_test.go` (new team-delete steps) |
| 46 | |
| 47 | **Interfaces:** |
| 48 | - Produces `func confirmed(r *http.Request, want string) (ok bool, msg string)` in `internal/httpd/confirm.go`: `ok` when `strings.TrimSpace(r.FormValue("confirm")) == want`; otherwise `msg` is `type ` + want + ` to confirm`. |
| 49 | - Produces the template partial `confirmfield` in `internal/web/templates/layout.html`: `{{define "confirmfield"}}<input type="text" name="confirm" aria-label="Type {{.}} to confirm" placeholder="type {{.}} to confirm" size="{{len .}}" autocomplete="off">{{end}}`, called as `{{template "confirmfield" "v1.0"}}`. |
| 50 | |
| 51 | What each control asks for (the `want` value), from the spec: |
| 52 | |
| 53 | | control | template | want | |
| 54 | |---|---|---| |
| 55 | | SSH key remove | account.html:34 | the 8 characters after `SHA256:` in the fingerprint | |
| 56 | | email remove | account.html:64 | the address | |
| 57 | | PGP key remove | account.html:90 | the first 8 characters of the fingerprint | |
| 58 | | release delete | releases.html:35 | the tag | |
| 59 | | snippet delete | snippet.html:46 | the snippet's public id | |
| 60 | | snippet file remove | snippet.html:22 | the file name | |
| 61 | | team delete | owner.html:98 | the team name | |
| 62 | | label remove | labels.html:15 | the label | |
| 63 | |
| 64 | Note the spec says the SSH key's label; a key's label can be empty, so the fingerprint prefix is used instead and the spec's Rules section is amended in this task (one line). |
| 65 | |
| 66 | - [ ] **Step 1: Write the failing e2e assertions** |
| 67 | |
| 68 | In `e2e/labelweb_test.go` around line 56, replace the label-remove post with two posts: |
| 69 | |
| 70 | ```go |
| 71 | // Removing a label needs its name typed; a bare post is refused and |
| 72 | // the label stays. |
| 73 | _, body := browserPost(t, alice, base+"/labels", url.Values{ |
| 74 | "action": {"remove"}, "name": {"bug"}}) |
| 75 | if !strings.Contains(body, "type bug to confirm") { |
| 76 | t.Fatalf("unconfirmed remove was not refused:\n%s", body) |
| 77 | } |
| 78 | if out, _, _ := inst.ssh(t, aliceKey, "", "label", "list", "alice/app", "--json"); !strings.Contains(out, `"name":"bug"`) { |
| 79 | t.Fatalf("label removed without confirmation: %s", out) |
| 80 | } |
| 81 | if status, _ := browserPost(t, alice, base+"/labels", url.Values{ |
| 82 | "action": {"remove"}, "name": {"bug"}, "confirm": {"bug"}}); status != 200 { |
| 83 | t.Fatal("label remove failed") |
| 84 | } |
| 85 | ``` |
| 86 | |
| 87 | In `e2e/releaseweb_test.go` around line 102, the same shape: a post without `confirm` gets a body containing `type v1.0 to confirm` and `release list --json` still lists `v1.0`; then the post with `"confirm": {"v1.0"}` succeeds and the existing "still listed after delete" assertion stays. |
| 88 | |
| 89 | In `e2e/accountweb_test.go` around line 78, the key-remove post: first without `confirm`, assert the response body contains `to confirm` and `keys list --json` still lists the fingerprint; then with `"confirm": {prefix}` where `prefix := strings.TrimPrefix(fp, "SHA256:")[:8]`; keep the existing assertion that the key is gone. |
| 90 | |
| 91 | In `e2e/snippetweb_test.go`: line 141 (file remove `b.txt`) adds `"confirm": {"b.txt"}`; line 148 (the last-file refusal) adds `"confirm": {"notes.md"}` so the refusal under test is still the command's; line 173 (delete) becomes `url.Values{"confirm": {created}}`; and before line 173 add a post with no `confirm` asserting the body contains `type `+created+` to confirm` and `snippet show` still exits 0. |
| 92 | |
| 93 | In `e2e/orgweb_test.go` after the team-revoke step (around line 86), add: a post with `"field": {"team-delete"}, "team": {"builders"}` and no `confirm` whose body contains `type builders to confirm`, then `org team show acme builders --json` still exits 0; then the same post with `"confirm": {"builders"}`, after which `org team show` exits 3. |
| 94 | |
| 95 | - [ ] **Step 2: Run them to see them fail** |
| 96 | |
| 97 | Run: `go test ./e2e -run 'TestLabelsWeb$|TestReleaseAndBuildWeb$|TestAccountSettingsWeb$|TestSnippetsWeb$|TestOrgManagementWeb$'` |
| 98 | Expected: each new "unconfirmed … was not refused" assertion fails, because the handlers act without a confirm field. |
| 99 | |
| 100 | - [ ] **Step 3: The helper and the partial** |
| 101 | |
| 102 | `internal/httpd/confirm.go`: |
| 103 | |
| 104 | ```go |
| 105 | package httpd |
| 106 | |
| 107 | import ( |
| 108 | "net/http" |
| 109 | "strings" |
| 110 | ) |
| 111 | |
| 112 | // confirmed reports whether the form typed want into its confirm field. |
| 113 | // It guards controls that destroy data nothing else holds; the person |
| 114 | // is already authorised, so this is a check against a slip, not a |
| 115 | // permission. |
| 116 | func confirmed(r *http.Request, want string) (bool, string) { |
| 117 | if strings.TrimSpace(r.FormValue("confirm")) == want { |
| 118 | return true, "" |
| 119 | } |
| 120 | return false, "type " + want + " to confirm" |
| 121 | } |
| 122 | ``` |
| 123 | |
| 124 | Add the `confirmfield` partial to `internal/web/templates/layout.html` next to `formatpicker` (line 132), exactly as in Interfaces. |
| 125 | |
| 126 | - [ ] **Step 4: Handlers** |
| 127 | |
| 128 | Each handler checks before dispatch and reports through the page's existing failure path: |
| 129 | |
| 130 | - `internal/httpd/account.go`: in `case "key-remove"` compute `want := strings.TrimPrefix(r.FormValue("fingerprint"), "SHA256:")`, `if len(want) > 8 { want = want[:8] }`; `if ok, msg := confirmed(r, want); !ok { back(msg, ""); return }` before `s.runControl`. `case "pgp-remove"`: `want` is the fingerprint's first 8 characters. `case "email-remove"`: `want` is `r.FormValue("address")`. Read `back`'s signature at account.go:150 and call it as the other failures do. |
| 131 | - `internal/httpd/releaseactions.go:34`: in the delete branch, `if ok, msg := confirmed(r, tag); !ok { back(w, r, msg); return }`. |
| 132 | - `internal/httpd/snippets.go`: in `snippetDeleteSubmit` check against `r.PathValue("id")`; in `snippetFileRemoveSubmit` against the trimmed `name`. On refusal `s.setFlash(w, msg)` and redirect to the snippet page, as `snippetAction`'s back closure does. |
| 133 | - `internal/httpd/orgweb.go:82`: in `case "team-delete"` check against `team`; on refusal `back(msg); return`. |
| 134 | - `internal/httpd/labels.go:51`: inside `if r.FormValue("action") == "remove"`, check against `name`; on refusal `s.backTo(w, r, "labels", msg); return`. |
| 135 | |
| 136 | - [ ] **Step 5: Templates** |
| 137 | |
| 138 | Put `{{template "confirmfield" X}}` immediately before the button in each form, with X the same value the handler wants: |
| 139 | |
| 140 | - account.html:34 key remove: `{{template "confirmfield" (slice (trimSHA .Fingerprint) 0 8)}}` needs no new helper if you compute the prefix in Go instead: add `Confirm string` beside `Fingerprint` in the key row struct the account page builds (find it in `internal/httpd/account.go`'s GET handler) and use `{{template "confirmfield" .Confirm}}`. Same for the PGP row (`Confirm` = first 8 of the fingerprint). Email: `{{template "confirmfield" .Address}}`. |
| 141 | - releases.html:35: `{{template "confirmfield" $rel.Tag}}`. |
| 142 | - snippet.html:22: `{{template "confirmfield" .Name}}`; :46: `{{template "confirmfield" .Snippet.PublicID}}`. |
| 143 | - owner.html:98: `{{template "confirmfield" .Name}}` (the team's name in that range). |
| 144 | - labels.html:15: `{{template "confirmfield" .Name}}`. |
| 145 | |
| 146 | Style: in `internal/web/static/style.css` add `input[name="confirm"] { width: auto; margin-right: var(--sp-2); }` near the other form rules (grep `.inline` to find them; use the spacing token the neighbours use). |
| 147 | |
| 148 | - [ ] **Step 6: Spec line** |
| 149 | |
| 150 | In `docs/specs/2026-09-11-web-ux-sweep-design.md`, Rules, change "SSH key remove (the key's label)" to "SSH key remove (the 8 characters after `SHA256:` in the fingerprint; a label can be empty)". |
| 151 | |
| 152 | - [ ] **Step 7: Run the tests** |
| 153 | |
| 154 | Run: `go build ./... && go vet ./... && go test ./internal/httpd && go test ./e2e -run 'TestLabelsWeb$|TestReleaseAndBuildWeb$|TestAccountSettingsWeb$|TestSnippetsWeb$|TestOrgManagementWeb$'` |
| 155 | Expected: PASS. `internal/httpd`'s input-label test sees the new input's `aria-label`. |
| 156 | |
| 157 | - [ ] **Step 8: Commit** |
| 158 | |
| 159 | ```bash |
| 160 | git add internal/httpd/confirm.go internal/httpd/account.go internal/httpd/releaseactions.go internal/httpd/snippets.go internal/httpd/orgweb.go internal/httpd/labels.go internal/web/templates/layout.html internal/web/templates/account.html internal/web/templates/releases.html internal/web/templates/snippet.html internal/web/templates/owner.html internal/web/templates/labels.html internal/web/static/style.css docs/specs/2026-09-11-web-ux-sweep-design.md e2e/accountweb_test.go e2e/releaseweb_test.go e2e/snippetweb_test.go e2e/labelweb_test.go e2e/orgweb_test.go |
| 161 | git commit -m "web: type the name to confirm a destructive control |
| 162 | |
| 163 | Release delete, snippet delete and file remove, team delete, label |
| 164 | remove, and SSH key, email and PGP key removal ask for the object's |
| 165 | name in a text field; the handler refuses a mismatch with a flash. |
| 166 | Reversible controls keep a plain button. |
| 167 | |
| 168 | Ref #182" |
| 169 | ``` |
| 170 | |
| 171 | --- |
| 172 | |
| 173 | ### Task 2: Login returns to the page that asked for it |
| 174 | |
| 175 | **Files:** |
| 176 | - Modify: `internal/httpd/flash.go` (two helpers) |
| 177 | - Modify: `internal/httpd/accounts.go:48-57` (`requireUser`), `:117-145` (`login`), and `renderLogin` |
| 178 | - Modify: `internal/web/templates/login.html:3-4` |
| 179 | - Test: `e2e/websessions_test.go` (extend `TestWebSessionsListRevoke`) |
| 180 | |
| 181 | **Interfaces:** |
| 182 | - Produces `setNext(w, path string)` and `takeNext(w, r) string` in `flash.go`, cookie name `gitbay_next`, `MaxAge: 600`, same flags as the flash cookie. `takeNext` returns `""` unless the value starts with `/` and not `//`. |
| 183 | - `renderLogin` gains a `next string` argument rendered as `Next`. |
| 184 | |
| 185 | - [ ] **Step 1: Write the failing e2e test** |
| 186 | |
| 187 | Append to the session test in `e2e/websessions_test.go`, using its existing instance and key (read the file first; it starts an accounts-mode instance and mints a login link): |
| 188 | |
| 189 | ```go |
| 190 | // An anonymous visit to a page that needs a session lands on the |
| 191 | // login page, which says where the visitor was going; the login |
| 192 | // link then returns them there. |
| 193 | anon := newBrowser(t) |
| 194 | status, body := browserGet(t, anon, inst.base()+"/settings") |
| 195 | if status != 200 || !strings.Contains(body, "continue to <code>/settings</code>") { |
| 196 | t.Fatalf("login page without the destination: %d\n%s", status, body) |
| 197 | } |
| 198 | out, _, _ := inst.ssh(t, aliceKey, "", "web", "login", "--json") |
| 199 | var env struct { |
| 200 | Data struct { |
| 201 | URL string `json:"url"` |
| 202 | } `json:"data"` |
| 203 | } |
| 204 | json.Unmarshal([]byte(out), &env) |
| 205 | link := inst.base() + env.Data.URL[strings.Index(env.Data.URL, "/login"):] |
| 206 | if status, body := browserGet(t, anon, link); status != 200 || !strings.Contains(body, "Account settings") { |
| 207 | t.Fatalf("login did not return to /settings: %d\n%s", status, body) |
| 208 | } |
| 209 | // The destination is used once. |
| 210 | if _, body := browserGet(t, anon, inst.base()+"/login"); strings.Contains(body, "continue to") { |
| 211 | t.Fatal("next survived its use") |
| 212 | } |
| 213 | ``` |
| 214 | |
| 215 | Adjust `aliceKey` to the key variable the test already has, and add `encoding/json` to the imports if missing. |
| 216 | |
| 217 | - [ ] **Step 2: Run it to see it fail** |
| 218 | |
| 219 | Run: `go test ./e2e -run 'TestWebSessionsListRevoke$'` |
| 220 | Expected: FAIL at "login page without the destination". |
| 221 | |
| 222 | - [ ] **Step 3: Cookie helpers** |
| 223 | |
| 224 | In `internal/httpd/flash.go`, after `takeFlash`: |
| 225 | |
| 226 | ```go |
| 227 | const nextCookie = "gitbay_next" |
| 228 | |
| 229 | // setNext remembers the local path an anonymous visitor asked for, so |
| 230 | // the login that follows can return there. Only a GET path is stored: |
| 231 | // a POST must not be replayed. |
| 232 | func (s *Server) setNext(w http.ResponseWriter, path string) { |
| 233 | if !strings.HasPrefix(path, "/") || strings.HasPrefix(path, "//") || len(path) > 300 { |
| 234 | return |
| 235 | } |
| 236 | http.SetCookie(w, &http.Cookie{ |
| 237 | Name: nextCookie, Value: url.QueryEscape(path), Path: "/", |
| 238 | HttpOnly: true, SameSite: http.SameSiteLaxMode, |
| 239 | Secure: s.cfg.HTTP.TLS != "off", MaxAge: 600, |
| 240 | }) |
| 241 | } |
| 242 | |
| 243 | // takeNext returns the remembered path once and clears it. Anything |
| 244 | // that is not a local path comes back empty. |
| 245 | func (s *Server) takeNext(w http.ResponseWriter, r *http.Request) string { |
| 246 | c, err := r.Cookie(nextCookie) |
| 247 | if err != nil || c.Value == "" { |
| 248 | return "" |
| 249 | } |
| 250 | http.SetCookie(w, s.clearCookie(nextCookie, http.SameSiteLaxMode)) |
| 251 | p, err := url.QueryUnescape(c.Value) |
| 252 | if err != nil || !strings.HasPrefix(p, "/") || strings.HasPrefix(p, "//") { |
| 253 | return "" |
| 254 | } |
| 255 | return p |
| 256 | } |
| 257 | |
| 258 | // peekNext reads the remembered path without clearing it, for the |
| 259 | // login page to say where the visitor is going. |
| 260 | func (s *Server) peekNext(r *http.Request) string { |
| 261 | c, err := r.Cookie(nextCookie) |
| 262 | if err != nil { |
| 263 | return "" |
| 264 | } |
| 265 | p, err := url.QueryUnescape(c.Value) |
| 266 | if err != nil || !strings.HasPrefix(p, "/") || strings.HasPrefix(p, "//") { |
| 267 | return "" |
| 268 | } |
| 269 | return p |
| 270 | } |
| 271 | ``` |
| 272 | |
| 273 | Add `"strings"` to the file's imports if absent. |
| 274 | |
| 275 | - [ ] **Step 4: requireUser and login** |
| 276 | |
| 277 | `requireUser` (accounts.go:48): before the redirect, `if r.Method == http.MethodGet { s.setNext(w, r.URL.RequestURI()) }`. |
| 278 | |
| 279 | `login` (accounts.go:117): the no-token branch passes `s.peekNext(r)` into `renderLogin`; the success branch replaces `http.Redirect(w, r, "/", ...)` with: |
| 280 | |
| 281 | ```go |
| 282 | dest := s.takeNext(w, r) |
| 283 | if dest == "" { |
| 284 | dest = "/" |
| 285 | } |
| 286 | http.Redirect(w, r, dest, http.StatusSeeOther) |
| 287 | ``` |
| 288 | |
| 289 | `renderLogin` gains `next string` and puts it in the page struct as `Next`; update its other callers (`loginSubmit` passes `""`). |
| 290 | |
| 291 | `login.html` after the `<h1>`: `{{if .Next}}<p class="meta">Log in to continue to <code>{{.Next}}</code>.</p>{{end}}`. |
| 292 | |
| 293 | - [ ] **Step 5: Run the tests** |
| 294 | |
| 295 | Run: `go build ./... && go vet ./... && go test ./internal/httpd && go test ./e2e -run 'TestWebSessionsListRevoke$|TestEmailLogin'` (the six `TestEmailLogin*` tests consume links too). |
| 296 | Expected: PASS. |
| 297 | |
| 298 | - [ ] **Step 6: Commit** |
| 299 | |
| 300 | ```bash |
| 301 | git add internal/httpd/flash.go internal/httpd/accounts.go internal/web/templates/login.html e2e/websessions_test.go |
| 302 | git commit -m "web: login returns to the page that needed it |
| 303 | |
| 304 | requireUser remembers a GET path in a short-lived cookie; the login |
| 305 | page names it and both login paths redirect there once. |
| 306 | |
| 307 | Ref #182" |
| 308 | ``` |
| 309 | |
| 310 | --- |
| 311 | |
| 312 | ### Task 3: One date format |
| 313 | |
| 314 | **Files:** |
| 315 | - Modify: `internal/web/web.go:220-227` (`when`) |
| 316 | - Modify: `internal/web/templates/commit.html:8`, `log.html:15`, `compare.html:9`, `mr.html:65`, `blame.html:16`, `snippets.html:12`, `snippet.html:5`, `settings.html:147,163` |
| 317 | - Modify: `internal/httpd/web.go:1516,1564,1890`, `internal/httpd/compare.go:72`, `internal/httpd/web.go:860-862` (blame) |
| 318 | - Test: `internal/web/web_test.go` (create if absent) and the e2e assertions the grep in Step 1 finds |
| 319 | |
| 320 | **Interfaces:** |
| 321 | - `when` renders `2006-01-02 15:04 UTC`; input stays RFC3339/RFC3339Nano. |
| 322 | |
| 323 | - [ ] **Step 1: Find every assertion on the old formats** |
| 324 | |
| 325 | Run: `grep -rn '[0-9]\{4\}-[0-9]\{2\}-[0-9]\{2\} [0-9]\{2\}:[0-9]\{2\}' e2e/ internal/httpd/*_test.go internal/web/*_test.go | grep -v 'Z"'` and `grep -rn '"when"\|when(' internal/web/*_test.go`. List the hits in the report; each is updated in Step 5. |
| 326 | |
| 327 | - [ ] **Step 2: Write the failing unit test** |
| 328 | |
| 329 | `internal/web/web_test.go` (append, or create with `package web`): |
| 330 | |
| 331 | ```go |
| 332 | func TestWhenNamesTheZone(t *testing.T) { |
| 333 | got := funcs["when"].(func(string) string)("2026-09-12T02:18:07.123Z") |
| 334 | if got != "2026-09-12 02:18 UTC" { |
| 335 | t.Fatalf("when: %q", got) |
| 336 | } |
| 337 | if got := funcs["when"].(func(string) string)("not a time"); got != "not a time" { |
| 338 | t.Fatalf("passthrough: %q", got) |
| 339 | } |
| 340 | } |
| 341 | ``` |
| 342 | |
| 343 | Run: `go test ./internal/web -run TestWhenNamesTheZone` → FAIL (`2026-09-12 02:18`). |
| 344 | |
| 345 | - [ ] **Step 3: The helper** |
| 346 | |
| 347 | In `internal/web/web.go:226` change the format to `"2006-01-02 15:04 UTC"`. |
| 348 | |
| 349 | - [ ] **Step 4: The pages** |
| 350 | |
| 351 | - `commit.html:8`: `{{when .Date}}` (the value is already RFC3339 from `web.go:1564`). |
| 352 | - `log.html:15`, `compare.html:9`, `mr.html:65`, `blame.html:16`: `{{when .Date}}`, and change the four producers to emit RFC3339 instead of `2006-01-02`: `web.go:1516`, `compare.go:72`, `web.go:1890`, `web.go:860-862` (each is a `time.Unix(...).UTC().Format("2006-01-02")` or a re-parse; make it `.Format(time.RFC3339)`). Read each site; if one already carries a `time.Time`, format it once. |
| 353 | - `snippets.html:12` → `{{when .UpdatedAt}}`; `snippet.html:5` → `updated {{when .Snippet.UpdatedAt}}`. |
| 354 | - `settings.html:147` → `{{when .Deps.LastCheck}}`; `:163` → `last poll {{when .LastSeen}}`. |
| 355 | - Tree and blob listings keep `ago`; where `ago` is used in `tree.html`, add `title="{{when .When}}"` on the element if the row carries the raw time (read the row struct; if it only has a `time.Time`, add a `whenT` helper: `"whenT": func(t time.Time) string { return t.UTC().Format("2006-01-02 15:04 UTC") }` and use it in the title). Skip the title if the tree row has no time value at all; say so in the report. |
| 356 | |
| 357 | - [ ] **Step 5: Update the assertions from Step 1, run the tests** |
| 358 | |
| 359 | Run: `go build ./... && go vet ./... && go test ./internal/web ./internal/httpd && go test ./e2e -run '<the tests whose assertions changed>|TestSnippetsWeb$'` |
| 360 | Expected: PASS. |
| 361 | |
| 362 | - [ ] **Step 6: Commit** |
| 363 | |
| 364 | ```bash |
| 365 | git add internal/web/web.go internal/web/web_test.go internal/web/templates internal/httpd e2e |
| 366 | git commit -m "web: one timestamp format, with the zone named |
| 367 | |
| 368 | when renders 2006-01-02 15:04 UTC on every page; commit, log, |
| 369 | compare, blame, snippet and settings pages use it instead of |
| 370 | date-only, ISO, or raw stored strings. |
| 371 | |
| 372 | Ref #182" |
| 373 | ``` |
| 374 | |
| 375 | --- |
| 376 | |
| 377 | ### Task 4: Version-aware tag order |
| 378 | |
| 379 | **Files:** |
| 380 | - Create: `internal/gitutil/versions.go`, `internal/gitutil/versions_test.go` |
| 381 | - Modify: `internal/httpd/web.go:1958-1969` (`refs`) and `:639-646` (`FreeTags` for the release form) |
| 382 | |
| 383 | **Interfaces:** |
| 384 | - Produces `func SortVersions(refs []Ref)` in `internal/gitutil`: in place, newest version first; refs that do not parse as a version follow, by name ascending. |
| 385 | |
| 386 | - [ ] **Step 1: Write the failing unit test** |
| 387 | |
| 388 | `internal/gitutil/versions_test.go`: |
| 389 | |
| 390 | ```go |
| 391 | package gitutil |
| 392 | |
| 393 | import "testing" |
| 394 | |
| 395 | func TestSortVersionsNewestFirst(t *testing.T) { |
| 396 | refs := []Ref{{Name: "v1.2.0"}, {Name: "v1.10.0"}, {Name: "nightly"}, {Name: "v1.2.1"}, {Name: "v0.9"}, {Name: "beta"}, {Name: "2.0.0"}} |
| 397 | SortVersions(refs) |
| 398 | var got []string |
| 399 | for _, r := range refs { |
| 400 | got = append(got, r.Name) |
| 401 | } |
| 402 | want := []string{"2.0.0", "v1.10.0", "v1.2.1", "v1.2.0", "v0.9", "beta", "nightly"} |
| 403 | for i := range want { |
| 404 | if i >= len(got) || got[i] != want[i] { |
| 405 | t.Fatalf("order %v, want %v", got, want) |
| 406 | } |
| 407 | } |
| 408 | } |
| 409 | ``` |
| 410 | |
| 411 | Run: `go test ./internal/gitutil -run TestSortVersionsNewestFirst` → compile error, `SortVersions` undefined. |
| 412 | |
| 413 | - [ ] **Step 2: The helper** |
| 414 | |
| 415 | `internal/gitutil/versions.go`: |
| 416 | |
| 417 | ```go |
| 418 | package gitutil |
| 419 | |
| 420 | import ( |
| 421 | "sort" |
| 422 | "strconv" |
| 423 | "strings" |
| 424 | ) |
| 425 | |
| 426 | // version parses "v1.2.3" or "1.2" into numeric parts. Anything else is |
| 427 | // not a version. |
| 428 | func version(name string) ([]int, bool) { |
| 429 | s := strings.TrimPrefix(name, "v") |
| 430 | if s == "" { |
| 431 | return nil, false |
| 432 | } |
| 433 | var parts []int |
| 434 | for _, p := range strings.Split(s, ".") { |
| 435 | n, err := strconv.Atoi(p) |
| 436 | if err != nil || n < 0 { |
| 437 | return nil, false |
| 438 | } |
| 439 | parts = append(parts, n) |
| 440 | } |
| 441 | return parts, true |
| 442 | } |
| 443 | |
| 444 | func versionLess(a, b []int) bool { |
| 445 | for i := 0; i < len(a) && i < len(b); i++ { |
| 446 | if a[i] != b[i] { |
| 447 | return a[i] < b[i] |
| 448 | } |
| 449 | } |
| 450 | return len(a) < len(b) |
| 451 | } |
| 452 | |
| 453 | // SortVersions orders refs newest version first. Names that are not |
| 454 | // versions follow, by name. |
| 455 | func SortVersions(refs []Ref) { |
| 456 | sort.SliceStable(refs, func(i, j int) bool { |
| 457 | vi, oki := version(refs[i].Name) |
| 458 | vj, okj := version(refs[j].Name) |
| 459 | switch { |
| 460 | case oki && okj: |
| 461 | return versionLess(vj, vi) |
| 462 | case oki != okj: |
| 463 | return oki |
| 464 | } |
| 465 | return refs[i].Name < refs[j].Name |
| 466 | }) |
| 467 | } |
| 468 | ``` |
| 469 | |
| 470 | - [ ] **Step 3: Use it** |
| 471 | |
| 472 | In `refs` (`internal/httpd/web.go:1958`): after `tags, _ := gitutil.Refs(p.Dir, "tags")` add `gitutil.SortVersions(tags)`. In the release form's tag list (`web.go:639-646`), sort the tags the same way before filtering so the select offers the newest first. |
| 473 | |
| 474 | - [ ] **Step 4: Run the tests** |
| 475 | |
| 476 | Run: `go test ./internal/gitutil -run TestSortVersionsNewestFirst && go build ./... && go vet ./... && go test ./e2e -run 'TestWebUI$|TestReleaseAndBuildWeb$'` (if an assertion depends on the old order, update it). |
| 477 | Expected: PASS. |
| 478 | |
| 479 | - [ ] **Step 5: Commit** |
| 480 | |
| 481 | ```bash |
| 482 | git add internal/gitutil/versions.go internal/gitutil/versions_test.go internal/httpd/web.go |
| 483 | git commit -m "web: refs and the release form order tags by version |
| 484 | |
| 485 | Ref #182" |
| 486 | ``` |
| 487 | |
| 488 | --- |
| 489 | |
| 490 | ### Task 5: The editor says no before the textarea |
| 491 | |
| 492 | **Files:** |
| 493 | - Modify: `internal/httpd/accounts.go:476-494` (`editForm`) |
| 494 | - Modify: `internal/web/templates/edit.html` |
| 495 | - Test: `e2e/accounts_test.go` (`TestWebAccounts` is the test that drives the file editor at `/edit/`; extend it) |
| 496 | |
| 497 | **Interfaces:** |
| 498 | - The edit page struct gains `Blocked string`; when set, the template renders it and no form. |
| 499 | |
| 500 | - [ ] **Step 1: Write the failing e2e test** |
| 501 | |
| 502 | Append to `TestWebAccounts` in `e2e/accounts_test.go`, after its existing successful edit and using its instance, key and logged-in client: |
| 503 | |
| 504 | ```go |
| 505 | // With signed commits required the editor cannot succeed, so the page |
| 506 | // says so instead of offering a textarea. |
| 507 | if _, errOut, code := inst.ssh(t, aliceKey, "", "repo", "settings", "require-signed", "alice/app", "on"); code != 0 { |
| 508 | t.Fatalf("require-signed: %s", errOut) |
| 509 | } |
| 510 | status, body := browserGet(t, alice, inst.base()+"/alice/app/edit/main/README.md") |
| 511 | if status != 200 || !strings.Contains(body, "requires signed commits") || strings.Contains(body, "<textarea") { |
| 512 | t.Fatalf("edit page under require-signed: %d\n%s", status, body) |
| 513 | } |
| 514 | ``` |
| 515 | |
| 516 | Use the repository path, file and variable names the test already has; the setting's command name is in `internal/control/repo.go` (grep `require-signed`). |
| 517 | |
| 518 | - [ ] **Step 2: Run it to see it fail** |
| 519 | |
| 520 | Run: `go test ./e2e -run 'TestWebAccounts$'` → FAIL: the page still has a textarea. |
| 521 | |
| 522 | - [ ] **Step 3: The handler** |
| 523 | |
| 524 | In `editForm`, after `repo` is resolved and before reading the blob, compute: |
| 525 | |
| 526 | ```go |
| 527 | blocked := "" |
| 528 | switch { |
| 529 | case repo.Settings.RequireSignedCommits: |
| 530 | blocked = repo.Path() + " requires signed commits and the web editor cannot sign; edit locally and push a signed commit." |
| 531 | case repo.Settings.RequireMR && slices.Contains(repo.Settings.ProtectedBranches, ref): |
| 532 | blocked = "branch " + ref + " accepts changes through merge requests only; edit on another branch and open one." |
| 533 | } |
| 534 | ``` |
| 535 | |
| 536 | Pass `Blocked: blocked` in the page struct. Still read the blob so a missing file is a 404 either way. Add `"slices"` to the imports. |
| 537 | |
| 538 | - [ ] **Step 4: The template** |
| 539 | |
| 540 | `edit.html`: wrap the `<form>` in `{{if .Blocked}}<p class="empty-note">{{.Blocked}}</p>{{else}} … {{end}}`, keeping the `<h1>` and the error line outside. |
| 541 | |
| 542 | - [ ] **Step 5: Run the tests** |
| 543 | |
| 544 | Run: `go build ./... && go vet ./... && go test ./internal/httpd && go test ./e2e -run 'TestWebAccounts$'` |
| 545 | Expected: PASS. |
| 546 | |
| 547 | - [ ] **Step 6: Commit** |
| 548 | |
| 549 | ```bash |
| 550 | git add internal/httpd/accounts.go internal/web/templates/edit.html e2e/accounts_test.go |
| 551 | git commit -m "web: the editor explains a refusal before the textarea |
| 552 | |
| 553 | Ref #182" |
| 554 | ``` |
| 555 | |
| 556 | --- |
| 557 | |
| 558 | ### Task 6: Messages a page shows |
| 559 | |
| 560 | **Files:** |
| 561 | - Modify: `internal/control/mr.go:1057,1132,1178` |
| 562 | - Modify: `internal/control/commitrefs.go:243` |
| 563 | - Modify: `internal/web/static/style.css` (`.syscomment`) |
| 564 | - Test: the e2e assertions the greps find (`grep -rn 'strategy merge\|--strategy\|closed by commit' e2e/ internal/`) |
| 565 | |
| 566 | Two commits: the merge messages, then the close-event line. |
| 567 | |
| 568 | - [ ] **Step 1: Grep the assertions** |
| 569 | |
| 570 | Run the grep above; list the hits. |
| 571 | |
| 572 | - [ ] **Step 2: Rewrite the three merge refusals in `internal/control/mr.go`** |
| 573 | |
| 574 | - line 1057: `"fast-forward not possible: %s has diverged from the MR head; merge with the merge strategy, or rebase and push again"` |
| 575 | - line 1132: `"the MR contains merge commit %.10s; a rebase merge needs linear history — choose the merge or squash strategy"` |
| 576 | - line 1178: keep the sentence about the stack and replace `--strategy ff or merge` with `the fast-forward or merge strategy`. |
| 577 | |
| 578 | Update the assertions found in Step 1 (they are substring checks; match the new wording). Run `go test ./internal/control && go test ./e2e -run '<the tests that assert them>'` and commit: |
| 579 | |
| 580 | ```bash |
| 581 | git commit -am "control: merge refusals name the strategy, not the flag |
| 582 | |
| 583 | The same text reaches the web merge form, which has no flags. |
| 584 | |
| 585 | Ref #182" |
| 586 | ``` |
| 587 | |
| 588 | - [ ] **Step 3: The close-event line** |
| 589 | |
| 590 | `internal/control/commitrefs.go:243`: `fmt.Sprintf("closed by %s in commit %s: %s", author, link, subject)`. Update any assertion on `closed by commit` (Step 1). In `style.css`, find `.syscomment` and add `.syscomment p { display: inline; margin: 0; }` so the rendered body and the `when` span sit on one line. Run `go test ./internal/control && go test ./e2e -run 'TestCommitMessageIssueActions$'` and commit: |
| 591 | |
| 592 | ```bash |
| 593 | git commit -am "control, web: the close event reads closed by <who> in commit <sha> |
| 594 | |
| 595 | Ref #182" |
| 596 | ``` |
| 597 | |
| 598 | --- |
| 599 | |
| 600 | ### Task 7: Small template fixes, one commit each |
| 601 | |
| 602 | **Files:** |
| 603 | - `internal/web/templates/account.html:129`; `landing.html:16` |
| 604 | - `internal/web/templates/settings.html` (the Save buttons listed below) |
| 605 | - `internal/web/templates/labels.html:6-13`; `internal/httpd/labels.go:31-37` |
| 606 | - `internal/web/templates/issue.html:53,65,77`; `mr.html:132,144` |
| 607 | - `internal/web/templates/issues.html:19-26`; `mrs.html` (the matching row) |
| 608 | - `internal/web/templates/globalsearch.html:4-19` |
| 609 | - Tests: `grep -rn` for each changed string in `e2e/` and `internal/httpd/*_test.go`, updated per commit |
| 610 | |
| 611 | Do these in order, each its own commit with `Ref #182`: |
| 612 | |
| 613 | - [ ] **Step 1: Copy.** `account.html:129`: `gitbay auth token mint --name laptop` → `gitbay auth token create --name laptop`. `landing.html:16`: "have an account? mint a browser session from your terminal:" → "have an account? log in from your terminal:". Commit `web: the settings page names the real token command`. |
| 614 | |
| 615 | - [ ] **Step 2: Save buttons name their field.** In `settings.html` change each plain `Save` to: line 12 `Save description`, 18 `Save website`, 26 `Save default branch`, 46 `Save visibility`, 52 `Save git://`, 61 `Save checks`, 67 `Save approvals`, 73 `Save threads`, 79 `Save CODEOWNERS`, 85 `Save signing`, 112 `Save merge-only`, 140 `Save dependency checks`, 185 `Save archive`. Grep `>Save<` in `e2e/settingsweb_test.go` and the httpd tests first; a test that finds the button by its text needs the new text. Commit `web: every Save on repository settings names its field`. |
| 616 | |
| 617 | - [ ] **Step 3: Labels colour column only when it means something.** In `labels.go` add `AnyColor bool` to the page struct, true when any label's `Color != ""`. In `labels.html` render the `colour` header and cell only `{{if or $.CanWrite $.AnyColor}}`. Commit `web: the labels page hides an empty colour column`. |
| 618 | |
| 619 | - [ ] **Step 4: Empty states.** `issue.html:53` → `none yet`, `:65` → `nobody yet`, `:77` → `none yet`; `mr.html:132` → `nobody yet`, `:144` → `none yet`. Grep `None yet\|Nobody yet\|Nobody asked yet\|No reviews yet` in tests first. Commit `web: one shape for empty sidebars`. |
| 620 | |
| 621 | - [ ] **Step 5: Issue and MR rows.** In `issues.html:25` wrap the state chip in `{{if eq $.State "all"}} … {{end}}`; in the meta line change `· <a …>{{.Milestone}}</a>` to `· in <a …>{{.Milestone}}</a>`. Apply the same two changes to the row in `mrs.html`. Grep `chip-open` in tests. Commit `web: list rows show the state only under all, and name the milestone`. |
| 622 | |
| 623 | - [ ] **Step 6: Search count.** In `globalsearch.html` after the `<nav>`: `{{if .Query}}<p class="meta">{{len .Results}} {{if eq (len .Results) 1}}result{{else}}results{{end}} for <q>{{.Query}}</q>{{if .Kind}} in {{.Kind}}{{end}}</p>{{end}}`. Keep the existing `no matches` empty note. Commit `web: global search counts its results and echoes the query`. |
| 624 | |
| 625 | - [ ] **Step 7: Run the checks once at the end** |
| 626 | |
| 627 | `go build ./... && go vet ./... && go test ./internal/httpd && go test ./e2e -run 'TestRepoSettingsWeb$|TestLabelsWeb$|TestIssueWebTriage$|TestMRWebReviewLoop$|TestGlobalSearchAndNotificationsWeb$'`. |
| 628 | |
| 629 | --- |
| 630 | |
| 631 | ### Task 8: MR source line and both clone URLs |
| 632 | |
| 633 | **Files:** |
| 634 | - Modify: `internal/httpd/web.go:1815-1920` (`mr` handler) and `internal/web/templates/mr.html:162-166` |
| 635 | - Modify: `internal/httpd/web.go:341-345` (`repoPage` in `repoFor`) and `internal/web/templates/tree.html:13` |
| 636 | - Test: `e2e/mrweb_test.go`, `e2e/web_test.go` (extend) |
| 637 | |
| 638 | Two commits. |
| 639 | |
| 640 | - [ ] **Step 1: MR source line.** In the `mr` handler compute `SourceGone bool`: true when `m.State == "source_gone"`, or when `m.SourcePath == ""` and `gitutil.ResolveRef(p.Dir, "refs/heads/"+m.SourceRef)` errors. Pass it in the page struct. In `mr.html:165` render: `into <code>{{.MR.TargetRef}}</code> · {{if eq .MR.State "merged"}}merged at{{else}}head{{end}} <code>{{short .MR.HeadSHA}}</code>{{if .SourceGone}} · <span class="chip chip-neutral">branch deleted</span>{{end}}`. Test: in the MR web test after a merge, delete the source branch over git (`git push origin --delete <branch>` with the test's env) and assert the MR page contains `branch deleted` and `merged at`. Commit `web: a merged MR shows its merged head and a deleted source branch`. |
| 641 | |
| 642 | - [ ] **Step 2: Both clone URLs.** In `repoFor` add `SSHCloneURL` to `repoPage`: `"ssh://git@" + s.cfg.SiteHost() + port + "/" + repo.Path() + ".git"` where `port` is `""` when `s.cfg.SSH.Port == 22` and `":" + strconv.Itoa(port)` otherwise. In `tree.html:13`: `Clone: <code>git clone {{.SSHCloneURL}}</code> · <code>git clone {{.CloneURL}}</code>`. Test: in `e2e/web_test.go`'s repo-home test assert the body contains `ssh://git@127.0.0.1:` followed by the instance's ssh port (the harness knows it; read `startInstanceWith` for the field). Commit `web: the repository home shows the SSH clone URL beside HTTPS`. |
| 643 | |
| 644 | - [ ] **Step 3: Run** `go build ./... && go vet ./... && go test ./internal/httpd && go test ./e2e -run 'TestMRWebReviewLoop$|TestWebUI$'`. |
| 645 | |
| 646 | --- |
| 647 | |
| 648 | ### Task 9: Changelog and the issue |
| 649 | |
| 650 | **Files:** |
| 651 | - Modify: `CHANGELOG.org` |
| 652 | |
| 653 | - [ ] **Step 1:** Above `* v1.20.1 — 2026-09-11` add: |
| 654 | |
| 655 | ```org |
| 656 | * v1.21.0 — unreleased |
| 657 | |
| 658 | The web UI/UX sweep (#182). |
| 659 | |
| 660 | - Destructive controls ask for the object's name typed beside the |
| 661 | button: release delete, snippet delete and file remove, team delete, |
| 662 | label remove, and SSH key, email and PGP key removal. Reversible |
| 663 | controls keep a plain button. |
| 664 | - Login returns to the page that asked for it, and says so. |
| 665 | - One timestamp format everywhere, =2006-01-02 15:04 UTC=. |
| 666 | - Tags on the refs page and in the release form are in version order, |
| 667 | newest first. |
| 668 | - The file editor explains up front when signed commits or |
| 669 | merge-requests-only protection would refuse the commit. |
| 670 | - Merge refusals name the strategy rather than the flag; an issue |
| 671 | closed by a commit reads "closed by <who> in commit <sha>". |
| 672 | - Repository settings: every Save names its field. Labels: the colour |
| 673 | column appears only when it means something. Sidebars use one shape |
| 674 | for empty. List rows show the state only under "all" and say "in |
| 675 | <milestone>". Global search counts its results. A merged MR shows |
| 676 | its merged head and a deleted source branch. The repository home |
| 677 | shows the SSH clone URL beside HTTPS. The settings page names |
| 678 | =auth token create=. |
| 679 | ``` |
| 680 | |
| 681 | - [ ] **Step 2: Commit** |
| 682 | |
| 683 | ```bash |
| 684 | git add CHANGELOG.org |
| 685 | git commit -m "CHANGELOG: web UI/UX sweep |
| 686 | |
| 687 | Closes #182" |
| 688 | ``` |