# Web UX sweep implementation plan > **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. **Goal:** Close out the architecture-review web findings (#261), the two web UX-review issues (#263, #271), the empty-state sweep (#270), the missing API-token page (#264), the MR range-diff view (#269), and the wiki non-page-link 404 (#283). **Architecture:** No new subsystems. Every write goes through `s.runControl`/`s.runControlCode` into the existing control registry (`internal/httpd/control.go`), the same rule every other web write already follows — this plan fixes the three handlers that did not (`pinToggle`, `watchToggle`, and adds a mute state to the latter). Reads either dispatch into a command (range-diff, whose text output the CLI and iOS already render, so the web reuses it rather than re-implementing revision resolution) or read the store directly, matching the existing convention for GET handlers (`accountPage` already reads `ListSSHKeys`/`ListPGPKeys` directly). Template copy changes are text-only; one new page (`mrrangediff.html`) and one new settings section (`Settings → Tokens`) are added following the existing settings-page and repo-page patterns. **Tech stack:** Go, `html/template`, the existing SQLite store, cobra (`cmd/gitbay`). **Spec:** none — this plan is written directly from the issue texts (`.gitbay/wiki` doc drift, architecture-review findings) and the current source; there is no separate design doc. ## Global constraints - Five MRs (one skipped: this plan issues #261/#263/#269/#270/#271/#283 are covered by five MRs; #264 is a sixth), each on its own branch off `main`. Commits are signed (the repository refuses unsigned ones); messages end with `Ref #N`, and the last commit closing an issue ends with `Closes #N`. No attribution to any assistant, model or AI anywhere: commits, MR bodies, comments. - `gitbay mr create --source --target main --title "..."`; merge with `gitbay mr merge --strategy ff` once CI is green, then delete the branch locally and on the remote. If the merge reports the branch is behind, rebase onto `main`, force-push, merge again. - Locally: `go build ./...`, `go vet ./...`, the unit tests of touched packages, and at most the one e2e test being written (`go test ./e2e -run TestName -count=1`). CI on bay1 runs the full suite (`go test ./...`), including `TestMainWidthClass`, `e2e/readonly_test.go`'s `readArgs` coverage, and the `cmd/gitbay` coverage test over `pass()` registrations — none of this plan's tasks add a new control command, so none of those three registries gain a new required row, but a new page template (`mrrangediff.html`) **does** need a row in `TestMainWidthClass`'s width map (`internal/web/web_test.go`). - No migrations in this plan (no schema changes). - Web writes dispatch through control commands (`internal/httpd/control.go`: `runControl`/`runControlCode`/ `runControlStdin`); a handler that calls the store directly for a *write* is exactly the bug #261 reports for `pinToggle` and `watchToggle`, and this plan does not introduce a new instance of it. Reads may call the store directly (the existing convention throughout `internal/httpd`) or dispatch when the logic they need (e.g. revision resolution for range-diff) already lives in a command. - Secrets travel on stdin or, for the one case that already carries one in a URL by design (the emailed login link), never in a place the code cannot document and cache-guard; never logged. - Wiki pages live in `.gitbay/wiki/`: `Parity.org`, `API.org`, `Threat-Model.org`. Update the page in the same MR that changes the behaviour it describes. - Writing style: plain, direct, no hype; UI copy uses the register fixed in MR 4 (Task 4.1) everywhere else it appears afterward. - CLAUDE.md's rules apply throughout: surgical changes only, no unrelated refactors, no speculative flexibility. ## Order and dependencies 1. **`web-audit-fixes`** (branch `web-audit-fixes`) — closes #261. Independent. Landing this first matters because MR 6 (#271's mute option) depends on the toggle-dispatch fix here. 2. **`web-settings-commands`** (branch `web-settings-commands`) — closes #263. Independent of 1. Lands after `cli-ux-help` (CLI UX plan), which carries the auth summary and help part of #263. 3. **`web-mr-range-diff`** (branch `web-mr-range-diff`) — closes #269. Independent. 4. **`web-empty-states`** (branch `web-empty-states`) — closes #270. Independent, but touches `mrs.html` and `globalsearch.html`; land before MR 6 to avoid the same files diverging on two branches at once (MR 6 does not touch either). 5. **`web-ux-small-fixes`** (branch `web-ux-small-fixes`) — closes #271. Depends on MR 1 (`repo watch`/`repo mute` dispatch and the cycling toggle it introduces; #271's Muted option builds directly on it). 6. **`wiki-raw-links`** (branch `wiki-raw-links`) — closes #283. Independent; small, can land anywhere, placed last only because it is unrelated to the rest. 7. **`web-api-tokens`** (branch `web-api-tokens`) — closes #264. Depends on plan 1 (`credentials-and-sessions`, #257): that plan changes `token create`'s own default scope to `read`. This plan's Task 7.1 makes the *web form* always send an explicit `--scope` value regardless of what the command defaults to, so this MR does not have to wait for plan 1 to land — but merge it after plan 1 to pick up the release-note and any command-message changes plan 1 makes to `token create`. If plan 1 has not landed yet, this MR still works correctly (the form never relies on the flag's default); note in the MR description that it does not depend on plan 1 having merged, only on eventually being consistent with it. --- # MR 1: architecture-review small fixes (branch `web-audit-fixes`) Closes #261. ### Task 1.1: run `foreign_key_check` inside the migration transaction, before commit **Files:** - Modify: `internal/store/store.go:182-259` (`step`, the `fkOff` branch) - Test: `internal/store/store_test.go` (create the case if no existing migration test exercises an `fkOff` step; check first) **Interfaces:** - No signature changes; `step`'s behaviour changes only. - [ ] **Step 1: Confirm there is no existing FK-violation test to build on** Run: `grep -n "foreign_key_check\|fkOff\|foreign_keys: off" internal/store/store_test.go` If nothing matches, the test below is new. - [ ] **Step 2: Write the failing test** Add to `internal/store/store_test.go`: ```go // A migration marked "-- foreign_keys: off" must have its // foreign_key_check run before the transaction commits, not after — // otherwise a violation is reported once the bad schema and // user_version are already persisted (#261). func TestFKOffMigrationChecksBeforeCommit(t *testing.T) { dir := t.TempDir() st, err := Open(dir + "/test.db") if err != nil { t.Fatal(err) } defer st.Close() // A minimal two-step schema: a parent table, then a child that // references it, or el se this migration wouldn't exercise anything. // Reach in through the exported entry point rather than duplicating // migration internals: two ad hoc migrations appended to the real // list would require touching the embedded migration files, so this // test instead runs the real migration set up to its current head // and then drives step() through a synthetic single-migration // upgrade using the unexported hook the package already has for // tests, if one exists. if err := st.MigrateUp(); err != nil { t.Fatal(err) } before, err := st.DB.Query("PRAGMA user_version") if err != nil { t.Fatal(err) } before.Close() // Insert a row through a raw statement that a fkOff rebuild would // have to preserve or complain about: a milestone with no matching // repo_id (the deliberately impossible case a corrupt migration // would produce). if _, err := st.DB.Exec("PRAGMA foreign_keys = OFF"); err != nil { t.Fatal(err) } if _, err := st.DB.Exec( "INSERT INTO milestones (repo_id, title, state, created_at) VALUES (99999, 'orphan', 'open', datetime('now'))"); err != nil { t.Fatal(err) } if _, err := st.DB.Exec("PRAGMA foreign_keys = ON"); err != nil { t.Fatal(err) } // A no-op fkOff step (rewriting milestones to itself) must now // refuse — before it commits, not after — because the orphan row // fails foreign_key_check. Confirm today's ordering leaves the // schema version bumped despite the row it can never satisfy: this // is the bug. Run the check directly the way step() will, and // compare against the version left behind. versionBefore := currentUserVersion(t, st) err = st.runFKOffStepForTest( "UPDATE sqlite_master SET name = name WHERE 0", versionBefore+1) if err == nil { t.Fatal("expected foreign_key_check to refuse the orphaned row") } if got := currentUserVersion(t, st); got != versionBefore { t.Fatalf("user_version changed to %d despite the refused check (should stay %d)", got, versionBefore) } } func currentUserVersion(t *testing.T, st *Store) int { t.Helper() var v int if err := st.DB.QueryRow("PRAGMA user_version").Scan(&v); err != nil { t.Fatal(err) } return v } ``` This calls an unexported `runFKOffStepForTest` that does not exist yet — it is Step 3's job to expose the already-unexported `step` closure's `fkOff` path under a name the test package can call. `step` is currently a closure local to `MigrateTo`; Step 3 promotes it to a package-level method so this test (and the real migration loop) can call the same code. - [ ] **Step 2: Run it and see it fail to compile** Run: `go test ./internal/store -run TestFKOffMigrationChecksBeforeCommit -count=1` Expected: FAIL to compile (`runFKOffStepForTest` undefined). - [ ] **Step 3: Promote `step` to a method and fix the ordering** In `internal/store/store.go`, replace the `step` closure inside `MigrateTo` (the whole `step := func(sqlText string, newVersion int, fkOff bool) (retErr error) { ... }` block at lines 182-259) with a call to a new method, and move its body there: ```go func (s *Store) migrateStep(sqlText string, newVersion int, fkOff bool) (retErr error) { if !fkOff { tx, err := s.DB.Begin() if err != nil { return err } defer tx.Rollback() if _, err := tx.Exec(sqlText); err != nil { return err } if _, err := tx.Exec(fmt.Sprintf("PRAGMA user_version = %d", newVersion)); err != nil { return err } return tx.Commit() } // A script whose first line is "-- foreign_keys: off" rebuilds a // table that other tables reference (labels, milestones): with // foreign keys on, the rebuild-by-rename loses the children's // rows. PRAGMA foreign_keys is a no-op inside a transaction, and // the pool gives no guarantee that a pragma set on one connection // is seen by the connection Begin() draws next, so the whole step // — pragma off, transaction, foreign_key_check, commit, pragma on — // runs on a single pinned connection. The check runs before commit: // checking after would report a violation once the bad schema and // user_version were already persisted. ctx := context.Background() conn, err := s.DB.Conn(ctx) if err != nil { return err } defer conn.Close() if _, err := conn.ExecContext(ctx, "PRAGMA foreign_keys = OFF"); err != nil { return err } // The connection goes back to the pool when this returns, so every // path out of here has to put foreign keys back on first. restoreFK := func() error { _, err := conn.ExecContext(ctx, "PRAGMA foreign_keys = ON") return err } defer func() { if err := restoreFK(); err != nil && retErr == nil { retErr = err } }() tx, err := conn.BeginTx(ctx, nil) if err != nil { return err } defer tx.Rollback() if _, err := tx.Exec(sqlText); err != nil { return err } if _, err := tx.Exec(fmt.Sprintf("PRAGMA user_version = %d", newVersion)); err != nil { return err } // foreign_key_check works with enforcement off: it inspects the // data directly rather than consulting the pragma. Running it here, // inside the transaction, means a violation rolls back the whole // rebuild (deferred tx.Rollback fires) instead of leaving the bad // schema and version committed. rows, err := tx.QueryContext(ctx, "PRAGMA foreign_key_check") if err != nil { return err } if rows.Next() { var table string var rowid sql.NullInt64 var referredTable string var fkid int if err := rows.Scan(&table, &rowid, &referredTable, &fkid); err != nil { rows.Close() return err } rows.Close() return fmt.Errorf("foreign_key_check failed after migration: %s", table) } if err := rows.Err(); err != nil { rows.Close() return err } rows.Close() return tx.Commit() } // runFKOffStepForTest exposes migrateStep's fkOff path to the package's // own tests, which need to drive one step in isolation rather than the // whole migration list MigrateTo runs. func (s *Store) runFKOffStepForTest(sqlText string, newVersion int) error { return s.migrateStep(sqlText, newVersion, true) } ``` Update `MigrateTo`'s two loops to call the method instead of the removed closure: ```go for cur < target { m := ms[cur] if err := s.migrateStep(m.up, m.version, m.upFKOff); err != nil { return fmt.Errorf("migration %d up: %w", m.version, err) } cur = m.version } for cur > target { m := ms[cur-1] if err := s.migrateStep(m.down, m.version-1, m.downFKOff); err != nil { return fmt.Errorf("migration %d down: %w", m.version, err) } cur = m.version - 1 } ``` `runFKOffStepForTest` is exported to the test file only in the sense that it is an ordinary method in a `_test.go`-adjacent non-test file, so it ships in the binary; that is acceptable here since it is a one-line wrapper with no side effect beyond calling the real path, and keeping it out of the production file would mean either duplicating `migrateStep` in a test-only file or using an unexported test hook pattern the package does not otherwise have. If review prefers it test-only, move it to `internal/store/storetest_export_test.go` (package `store`) instead — functionally identical either way. - [ ] **Step 4: Run the test** Run: `go test ./internal/store -run TestFKOffMigrationChecksBeforeCommit -count=1` Expected: PASS (the orphan row now fails the check before commit, and `user_version` is left unchanged because `tx.Rollback()` fires). - [ ] **Step 5: Run the full package** Run: `go test ./internal/store -count=1` Expected: PASS — this is a reordering, not a behaviour change, for every migration that does not already violate its own foreign keys. - [ ] **Step 6: Commit** ```bash git add internal/store/store.go internal/store/store_test.go git commit -m "store: run foreign_key_check inside the migration transaction, before commit" -m "Ref #261" ``` ### Task 1.2: `pinToggle` and `watchToggle` dispatch through their commands **Files:** - Modify: `internal/httpd/accounts.go:241-250` (`pinToggle`) - Modify: `internal/httpd/notifyweb.go:61-72` (`watchToggle`) - Test: `internal/httpd/accounts_test.go` or `internal/httpd/account_test.go` (check which file already has repo-toggle tests; add beside them) **Interfaces:** - Consumes: `s.runControl` (`internal/httpd/control.go:25`, already used by `bookmarkToggle`, the correct existing model for this fix). - Produces: no new exported names; `watchToggle`'s behaviour becomes a three-way cycle (`""` → `watching` → `muted` → `""`), which Task 5.x in MR 5 (`web-ux-small-fixes`) builds on to expose "Muted" as a reachable state rather than adding a new endpoint. - [ ] **Step 1: Write the failing tests** Add to `internal/httpd/accounts_test.go` (create the file if repo pin/watch tests do not already live somewhere; check with `grep -rln "pinToggle\|watchToggle" internal/httpd/*_test.go` first and add beside whatever that finds): ```go package httpd import ( "net/http/httptest" "testing" "gitbay.org/gitbay/internal/config" "gitbay.org/gitbay/internal/store" ) // Pinning writes through the repo pin command, not the store directly, // so it carries the same audit trail and write budget as every other // mutating command (#261). func TestPinToggleDispatchesRepoPin(t *testing.T) { st, err := store.Open(":memory:") if err != nil { t.Fatal(err) } defer st.Close() if err := st.MigrateUp(); err != nil { t.Fatal(err) } uid, err := st.CreateUser("alice", false) if err != nil { t.Fatal(err) } u := store.User{ID: uid, Username: "alice"} if _, err := st.CreateRepo("user", uid, "app", "public"); err != nil { t.Fatal(err) } s := New(config.Default(), st) req := httptest.NewRequest("POST", "/alice/app/pin", nil) req.SetPathValue("owner", "alice") req.SetPathValue("repo", "app") rr := httptest.NewRecorder() s.pinToggle(rr, req, u) repo, err := st.RepoByPath("alice/app") if err != nil { t.Fatal(err) } if !st.IsPinned(uid, repo.ID) { t.Fatal("pin did not take effect") } rr2 := httptest.NewRecorder() s.pinToggle(rr2, req, u) if st.IsPinned(uid, repo.ID) { t.Fatal("second toggle should have unpinned") } } // The watch button cycles default, watching, muted — the three states // repo watch/repo mute/repo unwatch already support — rather than the // two the store-writing version offered (#261, #271). func TestWatchToggleCyclesThroughMuted(t *testing.T) { st, err := store.Open(":memory:") if err != nil { t.Fatal(err) } defer st.Close() if err := st.MigrateUp(); err != nil { t.Fatal(err) } uid, err := st.CreateUser("alice", false) if err != nil { t.Fatal(err) } u := store.User{ID: uid, Username: "alice"} if _, err := st.CreateRepo("user", uid, "app", "public"); err != nil { t.Fatal(err) } repo, err := st.RepoByPath("alice/app") if err != nil { t.Fatal(err) } s := New(config.Default(), st) req := httptest.NewRequest("POST", "/alice/app/watch", nil) req.SetPathValue("owner", "alice") req.SetPathValue("repo", "app") click := func() string { rr := httptest.NewRecorder() s.watchToggle(rr, req, u) return st.RepoWatchState(repo.ID, uid) } if got := click(); got != "watching" { t.Fatalf("first click: got %q, want watching", got) } if got := click(); got != "muted" { t.Fatalf("second click: got %q, want muted", got) } if got := click(); got != "" { t.Fatalf("third click: got %q, want default (unwatched)", got) } } ``` (Check `CreateRepo`'s exact signature with `grep -n "func (s \*Store) CreateRepo" internal/store/*.go` before using it — adjust argument order/names to match if it differs from the guess above.) - [ ] **Step 2: Run and see them fail** Run: `go test ./internal/httpd -run 'TestPinToggleDispatchesRepoPin|TestWatchToggleCyclesThroughMuted' -count=1` Expected: FAIL — pin toggles once but not twice cleanly is unlikely to be the failure; more likely the watch test fails because today's `watchToggle` only ever sets `"watching"` or clears it, never `"muted"`. - [ ] **Step 3: Fix `pinToggle`** In `internal/httpd/accounts.go`, replace: ```go // pinToggle pins or unpins the repo for the logged-in viewer. func (s *Server) pinToggle(w http.ResponseWriter, r *http.Request, u store.User) { repo, ok := s.repoForUser(w, r, u, policy.CanRead) if !ok { return } if s.st.IsPinned(u.ID, repo.ID) { s.st.UnpinRepo(u.ID, repo.ID) } else { s.st.PinRepo(u.ID, repo.ID) } http.Redirect(w, r, "/"+repo.Path(), http.StatusSeeOther) } ``` with: ```go // pinToggle pins or unpins the repo for the logged-in viewer, through // repo pin/repo unpin — the same commands the CLI runs — rather than // writing the store directly (#261). func (s *Server) pinToggle(w http.ResponseWriter, r *http.Request, u store.User) { repo, ok := s.repoForUser(w, r, u, policy.CanRead) if !ok { return } verb := "pin" if s.st.IsPinned(u.ID, repo.ID) { verb = "unpin" } if _, msg, ok := s.runControl(u, []string{"repo", verb, repo.Path()}); !ok { s.setFlash(w, msg) } http.Redirect(w, r, "/"+repo.Path(), http.StatusSeeOther) } ``` - [ ] **Step 4: Fix `watchToggle`** In `internal/httpd/notifyweb.go`, replace: ```go // watchToggle turns watching a repository on and off from its header, // the way the pin button does. func (s *Server) watchToggle(w http.ResponseWriter, r *http.Request, u store.User) { repo, ok := s.repoForUser(w, r, u, policy.CanRead) if !ok { return } if s.st.RepoWatchState(repo.ID, u.ID) == "watching" { s.st.ClearRepoWatch(repo.ID, u.ID) } else { s.st.SetRepoWatch(repo.ID, u.ID, "watching") } http.Redirect(w, r, "/"+repo.Path(), http.StatusSeeOther) } ``` with: ```go // watchToggle cycles the viewer's watch state on a repository: default, // watching, muted, back to default — through repo watch/repo mute/repo // unwatch, the same commands the CLI runs (#261, #271). func (s *Server) watchToggle(w http.ResponseWriter, r *http.Request, u store.User) { repo, ok := s.repoForUser(w, r, u, policy.CanRead) if !ok { return } next := map[string]string{"": "watch", "watching": "mute", "muted": "unwatch"} verb := next[s.st.RepoWatchState(repo.ID, u.ID)] if _, msg, ok := s.runControl(u, []string{"repo", verb, repo.Path()}); !ok { s.setFlash(w, msg) } http.Redirect(w, r, "/"+repo.Path(), http.StatusSeeOther) } ``` - [ ] **Step 5: Run** Run: `go test ./internal/httpd -run 'TestPinToggleDispatchesRepoPin|TestWatchToggleCyclesThroughMuted' -count=1 && go test ./internal/httpd -count=1` Expected: PASS. If an existing test asserted the old two-state watch behaviour, update its expectation to the three-state cycle rather than reverting the fix. - [ ] **Step 6: Commit** ```bash git add internal/httpd/accounts.go internal/httpd/notifyweb.go internal/httpd/accounts_test.go git commit -m "web: pin and watch toggles dispatch through repo pin/watch/mute/unwatch" -m "Ref #261" ``` ### Task 1.3: Cache-Control: no-store on the login-link consuming request **Files:** - Modify: `internal/httpd/accounts.go:123-157` (`login`) - Test: `internal/httpd/logincookie_test.go` (add beside its existing login tests) - [ ] **Step 1: Write the failing test** Add to `internal/httpd/logincookie_test.go`: ```go // The login link's token rides in the query string — the one // documented exception to "never in a URL" — so the response that // consumes it must never be cached by an intermediary that might log // or replay the URL (#261). func TestLoginNoStoreHeader(t *testing.T) { st, err := store.Open(":memory:") if err != nil { t.Fatal(err) } defer st.Close() if err := st.MigrateUp(); err != nil { t.Fatal(err) } s := New(config.Default(), st) rr := httptest.NewRecorder() req := httptest.NewRequest("GET", "/login?token=bogus", nil) s.login(rr, req) if got := rr.Header().Get("Cache-Control"); got != "no-store" { t.Errorf("Cache-Control = %q, want no-store", got) } } ``` Check the file's existing imports (`config`, `store`, `httptest`, `testing`) before adding — they are almost certainly already present given the file already tests `/login`. - [ ] **Step 2: Run and see it fail** Run: `go test ./internal/httpd -run TestLoginNoStoreHeader -count=1` Expected: FAIL (`Cache-Control` header absent). - [ ] **Step 3: Set the header** In `internal/httpd/accounts.go`, at the top of `login`: ```go func (s *Server) login(w http.ResponseWriter, r *http.Request) { // token, when present, is a single-use secret in the query string — // the documented exception to "never in a URL" (Threat-Model). No // cache may keep a copy of this response. w.Header().Set("Cache-Control", "no-store") token := r.URL.Query().Get("token") ``` - [ ] **Step 4: Run** Run: `go test ./internal/httpd -run TestLoginNoStoreHeader -count=1 && go test ./internal/httpd -count=1` Expected: PASS. - [ ] **Step 5: Commit** ```bash git add internal/httpd/accounts.go internal/httpd/logincookie_test.go git commit -m "web: Cache-Control: no-store on the login-link request" -m "Ref #261" ``` ### Task 1.4: doc drift — API.org, Parity.org, Threat-Model.org **Files:** - Modify: `.gitbay/wiki/API.org` (the token-refusal line, near "Git transport commands and the token commands are refused by name.") - Modify: `.gitbay/wiki/Parity.org` (the "Batched review is not built." line; the watch/pin dispatch paragraph at lines 249-251) - Modify: `.gitbay/wiki/Threat-Model.org` (the "never as command arguments... or query strings" line) No test — these are prose fixes; CI has no wiki-content check beyond what already exists (link and page-name tests in `internal/httpd/wiki_test.go`, untouched by this task). - [ ] **Step 1: Fix API.org's incorrect claim about token commands** The 401 message (`internal/httpd/api.go:131`, unchanged by this task) already reads `missing bearer token; mint one over SSH: token create --name ` — it does not say token commands are refused on the API, because they are not: `token create`/`token list`/`token revoke` all dispatch normally through `POST /api/v1/cmd` like any other command (minting a token from a token is exactly what a full-scope token can do, per `#234`/the "one registry" rule). Replace the false claim: ``` Commands that emit raw text rather than an envelope (=help=, =mr diff=) come wrapped as ={"output": "..."}=. Git transport commands are refused by name; the token commands are not — a full-scope token can mint, list and revoke tokens the same way it can run anything else. ``` (Replaces the sentence "Git transport commands and the token commands are refused by name.") - [ ] **Step 2: Fix Parity.org's stale "batched review" claim** Find (near "view. Batched review is not built."): ``` =mr range-diff= compares two heads: the iOS client shows it from a revision to the one before, as text; the web has no view. Batched review is not built. ``` `mr comment --pending`/`--discard` and `PublishPendingComments` (`internal/control/mr.go:161-162,1044,1052`) are the batched-review mechanism, and the web's diff-comment form already composes pending comments before publishing (`internal/httpd/mractions.go`'s `mrDiffCommentSubmit`, unchanged by this task — confirm with `grep -n "diff-comment\|PendingComments" internal/httpd/*.go`). Replace: ``` =mr range-diff= compares two heads: the iOS client shows it from a revision to the one before, as text; the web renders the same view (krz/gitbay#269). Batched review — draft diff comments held with =mr comment --pending= and sent together with =--comment=/=--discard= or a verdict — is built and the web uses it: composing a review comments before publishing them is the same round trip as the CLI's =--pending= flag. ``` Leave the `mr range-diff` web-view claim as `krz/gitbay#269` for now; MR 3 of this plan (`web-mr-range-diff`) lands the actual view and updates this sentence again to drop the issue reference — do not pre-empt that here, since this task's branch may merge before or after MR 3 and the wiki text must describe what is actually deployed at each point. (If MR 3 has already merged when this task is done, skip the issue-reference wording and write the view as already existing instead; check `ls internal/web/templates/mrrangediff.html` first.) - [ ] **Step 3: Fix the pin/watch dispatch paragraph** Find (lines 249-251): ``` The web's watch and pin controls write the store directly instead of dispatching =repo watch= and =repo pin=. That is why the web cannot mute: its toggle knows watching and default only. ``` Replace: ``` The web's watch and pin controls dispatch =repo pin=/=repo unpin= and =repo watch=/=repo mute=/=repo unwatch=, the same commands the CLI runs (krz/gitbay#261). The single watch button cycles default, watching and muted. ``` And update the `mute` row in the capability table (around line 189) from: ``` | mute | yes | no | yes | ``` to: ``` | mute | yes | yes | yes | ``` - [ ] **Step 4: Fix Threat-Model.org's "never in a URL" claim** Find (in "What gitbay never does"): ``` - *Put secrets in argv, URLs, or logs.* Import and mirror credentials, registration invites, and API tokens travel on stdin or in request bodies, never as command arguments (visible in =/proc=) or query strings. Tokens are stored only as SHA-256 hashes. ``` Replace with (documenting the one deliberate exception and its mitigations from Task 1.3): ``` - *Put secrets in argv, URLs, or logs, with one documented exception.* Import and mirror credentials, registration invites, and API tokens travel on stdin or in request bodies, never as command arguments (visible in =/proc=) or query strings. The one exception is the emailed login link, =/login?token=...=: single-use, 15-minute expiry, and the response that consumes it carries =Cache-Control: no-store= so no intermediary keeps a copy. An operator running gitbay behind a reverse proxy should configure that proxy to strip the query string from its own access logs. Tokens are stored only as SHA-256 hashes. ``` - [ ] **Step 5: Commit** ```bash git add .gitbay/wiki/API.org .gitbay/wiki/Parity.org .gitbay/wiki/Threat-Model.org git commit -m "wiki: fix API token-refusal claim, batched-review status, watch/pin dispatch, login-link URL exception" -m "Ref #261" ``` ### Task 1.5: open MR 1 - [ ] **Step 1: Push and open the MR** ```bash git push -u origin web-audit-fixes gitbay mr create --source web-audit-fixes --target main --title "Architecture review small fixes: FK check, web toggles, doc drift" ``` - [ ] **Step 2: Wait for CI, merge, delete the branch** ```bash gitbay mr merge --strategy ff git branch -d web-audit-fixes git push origin --delete web-audit-fixes ``` The last commit in this branch (Task 1.4's) should be amended in message only if not already — reference `Closes #261` there instead of `Ref #261` before pushing, since this MR closes the issue in full. --- # MR 2: settings page quotes working commands (branch `web-settings-commands`) Closes #263. ### Task 2.1: fix the two known-wrong quoted commands **Files:** - Modify: `internal/web/templates/account.html:186,199-200` - [ ] **Step 1: Fix the `whoami` line** Find (`account.html:199-200`): ```html

All of it works from stock OpenSSH too: ssh git@{{.Host}} auth whoami.

``` `auth` is a CLI-only grouping (`cmd/gitbay/main.go`'s `authCmd`); the server command is `whoami` (`internal/control/identity.go:18`). Replace: ```html

All of it works from stock OpenSSH too: ssh git@{{.Host}} whoami.

``` - [ ] **Step 2: Show both forms for the token line** Find (`account.html:194-197`): ```html 
gitbay auth token create --name laptop # API tokens
gitbay web sessions list               # browser sessions
gitbay admin ...                       # instance administration
``` The CLI form (`gitbay auth token create ...`) and the literal stock-SSH form (`ssh git@host token create ...`) differ because `token` is nested under the CLI-only `auth` group but is a top-level server command (`internal/control/token.go`). Replace the pre block and the sentence after it: ```html 
gitbay auth token create --name laptop # API tokens
gitbay web sessions list               # browser sessions
gitbay admin ...                       # instance administration

All of it works from stock OpenSSH too, with the CLI's grouping words dropped: ssh git@{{.Host}} whoami, ssh git@{{.Host}} token create --name laptop.

``` (This merges the "works from stock OpenSSH" sentence that Step 1 edited with the new token example, so it appears once rather than twice — remove the now-duplicate sentence Step 1 produced and keep this single combined one instead. After this step, the section reads: the `
`
block, then one `
` with both stock-SSH examples.)

- [ ] **Step 3: Fix `gitbay account export`**

Find (`account.html:186`, in the Export section):

```html

Your profile, repositories, issues and merge requests as one
JSON bundle, the same one gitbay account export writes. Keys are
never included; a replayed bundle's emails arrive unverified.

```

`account` is not a real top-level CLI command — the real path is `auth
export` (`cmd/gitbay/main.go:471`, nested under `authCmd`), as
`privacy.html:20` already correctly says. Replace:

```html

Your profile, repositories, issues and merge requests as one
JSON bundle, the same one gitbay auth export writes. Keys are
never included; a replayed bundle's emails arrive unverified.

```

- [ ] **Step 4: Commit (folded into Task 2.3, which adds the test these
  fixes make pass — do not commit yet; Task 2.2 and 2.3 come first so
  the fixes and their proof land together)**

Skip committing here; continue to Task 2.2.

### Task 2.2: auth summary and help — done in the CLI UX plan

The auth summary ("whoami, SSH and PGP keys, email, API tokens") and
the registry-layout `gitbay auth --help` are Task 2.3 of
`docs/plans/2026-09-27-cli-ux.md` (MR `cli-ux-help`, Ref #267), which
adds `nounAliases` to `internal/control/help.go` so `help auth` renders
in one pass. Land `cli-ux-help` before this MR; nothing to do here.
Check after rebasing: `go run ./cmd/gitbay auth --help` lists email and
token commands.

### Task 2.3: a test that runs every quoted command through the registry

**Files:**
- Create: `cmd/gitbay/templatecmds_test.go`

**Interfaces:**
- Consumes: `newRoot()` (`cmd/gitbay/main.go:30`, unexported — this test
  must live in package `main`), `control.Lookup`
  (`internal/control/control.go:102`), `web.Pages`/`web.TemplateSource`
  (`internal/web/web.go:271,277`).

- [ ] **Step 1: Write the test**

```go
package main

import (
	"regexp"
	"strings"
	"testing"

	"gitbay.org/gitbay/internal/control"
	"gitbay.org/gitbay/internal/web"
)

// quotedRe finds the two shapes a command appears in on a page: inline
// in , or one per line in a 
 quickstart
// block. Both need (?s) so a multi-line 
 is captured as one match.
var quotedRe = regexp.MustCompile(`(?s)(.*?)|
]*>(.*?)
`)

// commandArgv reads the literal words at the front of a quoted command
// line — the part naming the command rather than its arguments — and
// stops at the first flag, template action, or literal ellipsis, since
// those mark the boundary between "what command" and "what argument".
func commandArgv(rest string) []string {
	var argv []string
	for _, tok := range strings.Fields(rest) {
		if strings.HasPrefix(tok, "-") || strings.Contains(tok, "{{") || strings.Contains(tok, "...") {
			break
		}
		argv = append(argv, tok)
	}
	return argv
}

// TestTemplateQuotedCommandsResolve runs every command quoted in a web
// template through the same registry the server uses, so a renamed
// command fails CI instead of shipping a dead instruction (#263).
//
// A line starting "gitbay " is checked against the CLI's own command
// tree with cobra's Find, since the CLI's grouping words (like "auth")
// are not part of the server's argv. A line starting "ssh git@{{.Host}}
// " is checked directly against control.Lookup, since that is exactly
// the argv the server receives.
func TestTemplateQuotedCommandsResolve(t *testing.T) {
	root := newRoot()
	for _, name := range web.Pages() {
		src, err := web.TemplateSource(name)
		if err != nil {
			t.Fatalf("%s: %v", name, err)
		}
		for _, m := range quotedRe.FindAllStringSubmatch(src, -1) {
			block := m[1] + m[2]
			for _, line := range strings.Split(block, "\n") {
				if i := strings.Index(line, "#"); i >= 0 {
					line = line[:i]
				}
				line = strings.TrimSpace(line)
				switch {
				case strings.HasPrefix(line, "gitbay "):
					argv := commandArgv(strings.TrimPrefix(line, "gitbay "))
					if len(argv) == 0 {
						continue
					}
					found, _, err := root.Find(argv)
					if err != nil || found == root {
						t.Errorf("%s: %q: gitbay %s does not resolve (%v)", name, line, strings.Join(argv, " "), err)
					}
				case strings.HasPrefix(line, "ssh git@{{.Host}} "):
					argv := commandArgv(strings.TrimPrefix(line, "ssh git@{{.Host}} "))
					if len(argv) == 0 {
						continue
					}
					if _, _, ok := control.Lookup(argv); !ok {
						t.Errorf("%s: %q: %s is not in the control registry", name, line, strings.Join(argv, " "))
					}
				}
			}
		}
	}
}
```

- [ ] **Step 2: Run and see it fail on the two known bugs**

Run: `go test ./cmd/gitbay -run TestTemplateQuotedCommandsResolve -count=1`
Expected: FAIL on `account.html`'s `ssh git@{{.Host}} auth whoami` (not
in the registry — `auth` is not a server path) and `gitbay account
export` (not a real CLI path — the top-level command is `auth`, not
`account`), unless Task 2.1's edits are already applied (do Task 2.1 and
Task 2.2 first if not already committed, then this test should already
pass on those — if it still fails, the fixes in Task 2.1 or 2.2 are
incomplete).

- [ ] **Step 3: Confirm it passes with Tasks 2.1 and 2.2 applied**

Run: `go test ./cmd/gitbay -run TestTemplateQuotedCommandsResolve -count=1`
Expected: PASS. If it fails on a *different* template than
`account.html`, that is a genuine additional bug this test caught —
fix the template's text the same way (correct the command to what
`control.Lookup`/cobra's tree actually accepts), do not weaken the test.
As of this plan being written, every other quoted command in the
templates (`admin.html`, `adminusers.html`, `issues.html`, `landing.html`,
`login.html`, `mrs.html`, `mrnew.html`, `privacy.html`, `registered.html`,
plus the ones this task edits) was checked by hand against
`cmd/gitbay/main.go` and `internal/control/*.go` and resolves correctly —
see the research notes in this plan's Order section — but the test is
the source of truth, not that hand check.

- [ ] **Step 4: Run the full package**

Run: `go build ./... && go vet ./... && go test ./cmd/gitbay -count=1`
Expected: PASS.

- [ ] **Step 5: Commit everything for this MR**

```bash
git add internal/web/templates/account.html cmd/gitbay/main.go cmd/gitbay/templatecmds_test.go
git commit -m "web, cli: fix two dead quoted commands; test every quoted command against the registry" -m "Closes #263"
```

### Task 2.4: open MR 2

```bash
git push -u origin web-settings-commands
gitbay mr create --source web-settings-commands --target main --title "Settings page: working stock-OpenSSH commands, registry-checked"
```

Wait for CI, `gitbay mr merge  --strategy ff`, delete the branch both
places.

---

# MR 3: MR range-diff page (branch `web-mr-range-diff`)

Closes #269.

### Task 3.1: `/{owner}/{repo}/mrs/{n}/range-diff`

**Files:**
- Create: `internal/httpd/mrrangediff.go`
- Create: `internal/web/templates/mrrangediff.html`
- Modify: `internal/httpd/routes.go` (add the route beside
  `/{owner}/{repo}/mrs/{n}` at line 102, in the always-registered block)
- Modify: `internal/web/web_test.go` (`TestMainWidthClass`'s `wide` map)
- Test: `internal/httpd/mrrangediff_test.go`

**Interfaces:**
- Consumes: `mrArgs` (`internal/httpd/mractions.go:37`), `s.repoFor`,
  `s.runControlCode`, `s.webViewer`.
- Produces: `func (s *Server) mrRangeDiff(w http.ResponseWriter, r *http.Request)`

- [ ] **Step 1: Write the failing test**

```go
package httpd

import (
	"net/http/httptest"
	"strings"
	"testing"

	"gitbay.org/gitbay/internal/config"
	"gitbay.org/gitbay/internal/store"
)

// The range-diff page dispatches mr range-diff and renders its text
// output, the same comparison the CLI and iOS already show (#269).
func TestMRRangeDiffPageRendersCommandOutput(t *testing.T) {
	st, err := store.Open(":memory:")
	if err != nil {
		t.Fatal(err)
	}
	defer st.Close()
	if err := st.MigrateUp(); err != nil {
		t.Fatal(err)
	}
	uid, err := st.CreateUser("alice", false)
	if err != nil {
		t.Fatal(err)
	}
	u := store.User{ID: uid, Username: "alice"}
	if _, err := st.CreateRepo("user", uid, "app", "public"); err != nil {
		t.Fatal(err)
	}

	s := New(config.Default(), st)
	req := httptest.NewRequest("GET", "/alice/app/mrs/1/range-diff", nil)
	req.SetPathValue("owner", "alice")
	req.SetPathValue("repo", "app")
	req.SetPathValue("n", "1")
	rr := httptest.NewRecorder()
	s.mrRangeDiff(rr, req)

	// No merge request 1 exists yet, so this must 404 rather than error.
	if rr.Code != 404 {
		t.Fatalf("status %d, body %s", rr.Code, rr.Body.String())
	}
	_ = strings.TrimSpace // placeholder import use removed once a real MR fixture is added below
}
```

(`CreateRepo`'s signature: confirm with
`grep -n "func (s \*Store) CreateRepo" internal/store/*.go` and adjust
the call above to match — the guess here follows the shape used
elsewhere in this plan's other tests.)

- [ ] **Step 2: Run and see it fail to compile**

Run: `go test ./internal/httpd -run TestMRRangeDiffPageRendersCommandOutput -count=1`
Expected: FAIL to compile (`s.mrRangeDiff` undefined).

- [ ] **Step 3: Write the handler**

Create `internal/httpd/mrrangediff.go`:

```go
package httpd

import (
	"net/http"
	"strconv"

	"gitbay.org/gitbay/internal/protocol"
	"gitbay.org/gitbay/internal/store"
)

// mrRangeDiff renders what changed between two revisions of a merge
// request — the same comparison `mr range-diff` prints on the CLI and
// the iOS app already show — so a reviewer whose approval a force-push
// staled can see what moved without leaving the browser (#269).
func (s *Server) mrRangeDiff(w http.ResponseWriter, r *http.Request) {
	p, ok := s.repoFor(w, r, "")
	if !ok {
		return
	}
	p.Tab = "merge requests"
	n, err := strconv.ParseInt(r.PathValue("n"), 10, 64)
	if err != nil {
		s.notFound(w, r)
		return
	}
	m, err := s.st.MRByNumber(p.Repo.ID, n)
	if err != nil {
		s.notFound(w, r)
		return
	}

	viewer := s.webViewer(r)
	argv := mrArgs(r, "range-diff")
	if from := r.URL.Query().Get("from"); from != "" {
		argv = append(argv, "--from", from)
	}
	if to := r.URL.Query().Get("to"); to != "" {
		argv = append(argv, "--to", to)
	}
	out, msg, code := s.runControlCode(viewer, argv)
	if code == protocol.ExitNotFound {
		s.notFound(w, r)
		return
	}
	errMsg := ""
	if code != protocol.ExitOK {
		errMsg = msg
	}
	s.render(w, "mrrangediff.html", struct {
		repoPage
		MR    store.MR
		Diff  string
		Error string
	}{p, m, out, errMsg})
}
```

- [ ] **Step 4: Write the template**

Create `internal/web/templates/mrrangediff.html`:

```html
{{define "width"}}wide{{end}}
{{define "title"}}range-diff · !{{.MR.Number}} · {{.Repo.OwnerName}}/{{.Repo.Name}}{{end}}
{{define "content"}}

Range-diff !{{.MR.Number}}


back to !{{.MR.Number}} {{.MR.Title}}

{{if .Error}}
{{else if .Diff}}
{{.Diff}}

{{else}}
Nothing to compare: this merge request has one revision.
{{end}}
{{end}}
```

- [ ] **Step 5: Register the route**

In `internal/httpd/routes.go`, right after the existing
`/{owner}/{repo}/mrs/{n}` route (line 102, in the block registered
regardless of `web.mode`, so range-diff reads the same way the MR page
itself does — anonymously on a public repository):

```go
		Route{Method: "GET", Pattern: "/{owner}/{repo}/mrs/{n}", Handler: s.mr},
		Route{Method: "GET", Pattern: "/{owner}/{repo}/mrs/{n}/range-diff", Handler: s.mrRangeDiff},
```

- [ ] **Step 6: Add the width-map row**

In `internal/web/web_test.go`, `TestMainWidthClass`, add
`"mrrangediff.html": true` to the `wide` map (alongside `"mrs.html"` and
`"build.html"`, which it resembles).

- [ ] **Step 7: Run**

Run: `go build ./... && go test ./internal/httpd -run TestMRRangeDiffPageRendersCommandOutput -count=1 && go test ./internal/web -run TestMainWidthClass -count=1`
Expected: PASS.

- [ ] **Step 8: Commit**

```bash
git add internal/httpd/mrrangediff.go internal/httpd/mrrangediff_test.go internal/httpd/routes.go internal/web/templates/mrrangediff.html internal/web/web_test.go
git commit -m "web: range-diff page for a merge request's revisions" -m "Ref #269"
```

### Task 3.2: revisions list with a "compare to previous" link per row

**Files:**
- Modify: `internal/web/templates/mr.html:136-138`
- Test: `internal/httpd/mrpage_test.go`

- [ ] **Step 1: Write the failing test**

Add to `internal/httpd/mrpage_test.go`:

```go
// Each revision after the first carries a link comparing it to the one
// before, so a reviewer does not have to type mr range-diff by hand
// (#269).
func TestMRPageListsRevisionsWithCompareLinks(t *testing.T) {
	var sb strings.Builder
	revs := []store.MRHead{
		{SHA: "aaaa1111", CreatedAt: "2026-09-23T10:00:00Z"},
		{SHA: "bbbb2222", CreatedAt: "2026-09-24T10:00:00Z"},
	}
	if err := web.Render(&sb, "mr.html", mrPageData{
		repoPage: testRepoPage(), MR: testMR("open"), View: "conversation", Revisions: revs,
	}); err != nil {
		t.Fatalf("render: %v", err)
	}
	out := sb.String()
	for _, want := range []string{"aaaa1111", "bbbb2222", "compare to previous", "from=aaaa1111", "to=bbbb2222"} {
		if !strings.Contains(out, want) {
			t.Errorf("missing %q in:\n%s", want, out)
		}
	}
	if strings.Contains(out, "gitbay mr range-diff") {
		t.Error("still quotes the CLI command instead of linking the new page")
	}
}
```

- [ ] **Step 2: Run and see it fail**

Run: `go test ./internal/httpd -run TestMRPageListsRevisionsWithCompareLinks -count=1`
Expected: FAIL (no "compare to previous" text yet).

- [ ] **Step 3: Replace the one-liner with a revisions list**

Find (`mr.html:136-138`):

```html
    {{if gt (len .Revisions) 1}}
{{len .Revisions}} revisions pushed. What changed between the last two:
      gitbay mr range-diff {{.Repo.OwnerName}}/{{.Repo.Name}} {{.MR.Number}}
{{end}}
  
```

Replace with:

```html
  
  {{if .Revisions}}

    
Revisions

    {{range $i, $rv := .Revisions}}
{{add $i 1}}. {{short $rv.SHA}} {{when $rv.CreatedAt}}{{if $i}} · compare to previous{{end}}

    {{end}}
  
{{end}}
```

(The closing `` that used to end the Reviews `grp` stays where it
is — this adds a new sibling `grp` right after it, using `add`/`sub`,
already registered template funcs in `internal/web/web.go`.)

- [ ] **Step 4: Run**

Run: `go test ./internal/httpd -run TestMRPageListsRevisionsWithCompareLinks -count=1 && go test ./internal/httpd -count=1`
Expected: PASS.

- [ ] **Step 5: Commit**

```bash
git add internal/web/templates/mr.html internal/httpd/mrpage_test.go
git commit -m "web: list each MR revision with a compare-to-previous link" -m "Ref #269"
```

### Task 3.3: update Parity

**Files:**
- Modify: `.gitbay/wiki/Parity.org`

- [ ] **Step 1: Fix the range-diff line**

Find:

```
=mr range-diff= compares two heads: the iOS client
shows it from a revision to the one before, as text; the web has no
view. Batched review is not built.
```

(If MR 1's Task 1.4 already changed this sentence to reference
`krz/gitbay#269`, edit that version instead — the end state either way
is:)

```
=mr range-diff= compares two heads: the iOS client shows it from a
revision to the one before, as text; the web renders the same view, with
a "compare to previous" link on each revision after the first. Batched
review — draft diff comments held with =mr comment --pending= and sent
together with =--comment=/=--discard= or a verdict — is built and the
web uses it.
```

- [ ] **Step 2: Commit**

```bash
git add .gitbay/wiki/Parity.org
git commit -m "wiki: Parity reflects the web range-diff view" -m "Closes #269"
```

### Task 3.4: open MR 3

```bash
git push -u origin web-mr-range-diff
gitbay mr create --source web-mr-range-diff --target main --title "Web: MR range-diff page"
```

Wait for CI, merge (`--strategy ff`), delete the branch both places.

---

# MR 4: empty states and contribution hints sweep (branch `web-empty-states`)

Closes #270.

This MR is one register applied across templates. The table below is
the full set of strings this task changes — every empty-state or
contribution-hint string identified in the issue and confirmed against
the current template source. Implement exactly this table; do not
invent additional wording beyond it.

| File:line | Current | New |
|---|---|---|
| `mrs.html:29` (no query) | `no {{if ne .State "all"}}{{.State}} {{end}}merge requests — open one with gitbay mr create {{.Repo.OwnerName}}/{{.Repo.Name}} --source ... --target {{.Repo.DefaultBranch}}` | `no {{if ne .State "all"}}{{.State}} {{end}}merge requests` (the create instruction moves to the new "New merge request"/fork/sign-in line — Task 4.2 — and is never a CLI command on the web) |
| `mrs.html:28` (search, no match) | `no {{if ne .State "all"}}{{.State}} {{end}}merge requests matching "{{.Query}}"` | unchanged (already follows the register: lower-case, no CLI command, states the fact) |
| `mr.html:136` (reviews, none) | `none yet` | unchanged — "none yet" is correct register for a section that can still gain entries (open MR); Task 4.1's rule is "no 'yet' on a *finished* item", and reviews on an open MR are not finished |
| `mr.html:143` (reviewers, none) | `nobody yet` | `no reviewers` (drops "yet" for consistency with the rest of the sweep's noun-first register even though the MR could still be open — "nobody yet" reads as a placeholder guess about who *will* review, which is not information the page has; "no reviewers" states the fact plainly, matching `dashboard.html`'s "No open merge requests") |
| `mr.html:174` (labels, none, on a merged/closed MR) | `none yet` | `no labels` when `.MR.State` is `merged` or `closed` (a finished item gets no "yet"); keep `none yet` when open |
| `builds.html:53` | `no builds — push a commit with a .gitbay/ci.yml` | `no builds` when the viewer cannot push (`.CanWrite` false or absent); `no builds — push a commit with a .gitbay/ci.yml` (drop ``, since a filename is not a command) stays for a viewer who can push. Never plain "no builds" for a writer, since that leaves them without the one instruction the page can give them |
| `dashboard.html:29` | `Nothing pinned yet. Press Pin on a repository.` | `nothing pinned — press Pin on a repository you visit` |
| `dashboard.html:42` (`Empty` value for MRs queue) | `No open merge requests` | unchanged (already matches the register: capital first word is this partial's own convention — see Step 1 below — lower-case the whole sweep *within* `
  • ` items, leave `queue` partial's own `Empty` string as-is since it is Title Case by that partial's design, confirmed by reading the `queue` template define before changing it) |
| `notifications.html:21` (all read) | `nothing here yet` | `no unread notifications` when `.All` is false is already separate; the `.All` branch (nothing at all, read or unread, in the *whole* inbox) becomes `nothing to show` |
| `notifications.html:21` (unread, default) | `nothing unread — show all` | `no unread notifications — show all` (already close; #265, a different plan, covers the CLI side of this exact wording — keep the two in step: `no unread notifications` matches what plan 6's CLI task sets for `notifications list`) |
| `globalsearch.html:47` | shown only when `.Query` is empty | shown always, as a permanent caption under the search input (Task 4.3) |

- [ ] **Step 1: Read the `queue` partial before touching `dashboard.html`**

Run: `grep -n '{{define "queue"}}' -A 15 internal/web/templates/dashboard.html`
Confirm whether its `Empty` value is rendered as given (in which case
`"No open merge requests"` stays capitalised by the caller's choice) or
lower-cased by the partial itself. Write down which, then leave that
line's casing exactly as the partial expects — do not change
`dashboard.html:42-43`'s `"Empty"` values in this task; the table above
already reflects "unchanged" for it.

### Task 4.1: apply the table

**Files:**
- Modify: `internal/web/templates/mrs.html:28-29`
- Modify: `internal/web/templates/mr.html:143,174`
- Modify: `internal/web/templates/builds.html:53`
- Modify: `internal/web/templates/dashboard.html:29`
- Modify: `internal/web/templates/notifications.html:21`
- Test: `internal/httpd/mrpage_test.go`, `internal/httpd/buildpages_test.go`
  (or wherever a `builds.html` render test already lives — check with
  `grep -rln '"builds.html"' internal/httpd/*_test.go`), a new or
  existing dashboard render test, a new or existing notifications test.

- [ ] **Step 1: Write the failing tests**

Add to `internal/httpd/mrpage_test.go`:

```go
// A finished merge request states an empty label list as a fact, not a
// promise something is still coming (#270).
func TestMRPageLabelsNoYetOnFinishedState(t *testing.T) {
	var sb strings.Builder
	if err := web.Render(&sb, "mr.html", mrPageData{
		repoPage: testRepoPage(), MR: testMR("merged"), View: "conversation",
	}); err != nil {
		t.Fatalf("render: %v", err)
	}
	if !strings.Contains(sb.String(), "no labels") {
		t.Error(`merged MR with no labels should read "no labels", not "none yet"`)
	}
}

func TestMRPageReviewersEmptyStateDropsNobody(t *testing.T) {
	var sb strings.Builder
	if err := web.Render(&sb, "mr.html", mrPageData{
		repoPage: testRepoPage(), MR: testMR("open"), View: "conversation",
	}); err != nil {
		t.Fatalf("render: %v", err)
	}
	if strings.Contains(sb.String(), "nobody yet") {
		t.Error(`reviewers empty state should read "no reviewers"`)
	}
	if !strings.Contains(sb.String(), "no reviewers") {
		t.Error(`missing "no reviewers"`)
	}
}
```

Add to whichever file already renders `builds.html` (or create
`internal/httpd/buildslist_test.go` if none does; check first with the
grep in the Files list above):

```go
// A writer sees the instruction to add CI; a reader without push access
// sees only the fact, since the instruction is not theirs to act on
// (#270).
func TestBuildsEmptyStateOmitsInstructionForReaders(t *testing.T) {
	var sb strings.Builder
	if err := web.Render(&sb, "builds.html", buildsPageData{
		repoPage: testRepoPage(), CanWrite: false,
	}); err != nil {
		t.Fatalf("render: %v", err)
	}
	out := sb.String()
	if !strings.Contains(out, "no builds") {
		t.Error(`missing "no builds"`)
	}
	if strings.Contains(out, "ci.yml") {
		t.Error("a reader without push access should not see the push instruction")
	}
}
```

(`buildsPageData` may not exist as a named type the way `mrPageData`
does for `mr.html` — check
`grep -n '"builds.html"' internal/httpd/web.go` for the anonymous
struct `builds` (the list handler, not `build`, the single-build one)
renders with, and mirror its fields the way `mrPageData` mirrors
`mr.html`'s, the same pattern `mrpage_test.go` already uses.)

- [ ] **Step 2: Run and see them fail**

Run: `go test ./internal/httpd -run 'TestMRPageLabelsNoYetOnFinishedState|TestMRPageReviewersEmptyStateDropsNobody|TestBuildsEmptyStateOmitsInstructionForReaders' -count=1`
Expected: FAIL.

- [ ] **Step 3: `mr.html` reviewers and labels**

Find (`mr.html:143`):

```html
    {{else}}
    nobody yet
    {{end}}
```

(in the Reviewers `grp`). Replace:

```html
    {{else}}
    no reviewers
    {{end}}
```

Find (`mr.html:174`, in the Labels `grp`):

```html
    {{else}}
    none yet
    {{end}}
```

Replace:

```html
    {{else}}
    {{if or (eq .MR.State "merged") (eq .MR.State "closed")}}no labels{{else}}none yet{{end}}
    {{end}}
```

- [ ] **Step 4: `builds.html`**

Read the current line first: `grep -n "no builds" internal/web/templates/builds.html`.
Replace it (adjust the exact surrounding markup to match what that grep
shows; the text change is):

```html
{{else}}
  • no builds{{if .CanWrite}} — push a commit with a .gitbay/ci.yml{{end}}
    • {{end}}
```

Check whether `builds.html`'s page struct already carries `CanWrite`
(`grep -n "CanWrite" internal/httpd/builds.go internal/web/templates/builds.html`);
if it does not, add it the way `mr.html`'s does
(`s.canWriteRepo(r, p.Repo)` in the handler, a new `CanWrite bool` field
in the render struct).

- [ ] **Step 5: `dashboard.html`**

Find (`dashboard.html:29`):

```html
  {{else}}
    Nothing pinned yet. Press Pin on a repository.
    {{end}}
```

Replace:

```html
  {{else}}
    nothing pinned — press Pin on a repository you visit
    {{end}}
```

- [ ] **Step 6: `notifications.html`**

Find (`notifications.html:21`):

```html
{{else}}
  • {{if .All}}nothing here yet{{else}}nothing unread — show all{{end}}
    • {{end}}
```

Replace:

```html
{{else}}
  • {{if .All}}nothing to show{{else}}no unread notifications — show all{{end}}
    • {{end}}
```

- [ ] **Step 7: `mrs.html`**

Find (`mrs.html:28-29`):

```html
{{else}}{{if .Query}}
  • no {{if ne .State "all"}}{{.State}} {{end}}merge requests matching “{{.Query}}”
    • 
{{else}}
  • no {{if ne .State "all"}}{{.State}} {{end}}merge requests — open one with gitbay mr create {{.Repo.OwnerName}}/{{.Repo.Name}} --source ... --target {{.Repo.DefaultBranch}}
    • {{end}}{{end}}
```

Replace (the create instruction moves to Task 4.2's contribution-hint
line, so the empty state itself states only the fact):

```html
{{else}}{{if .Query}}
  • no {{if ne .State "all"}}{{.State}} {{end}}merge requests matching “{{.Query}}”
    • 
{{else}}
  • no {{if ne .State "all"}}{{.State}} {{end}}merge requests
    • {{end}}{{end}}
```

- [ ] **Step 8: Run**

Run: `go test ./internal/httpd -run 'TestMRPageLabelsNoYetOnFinishedState|TestMRPageReviewersEmptyStateDropsNobody|TestBuildsEmptyStateOmitsInstructionForReaders' -count=1 && go test ./internal/httpd -count=1`
Expected: PASS. Fix any pre-existing test that asserted the old strings
("nobody yet", "none yet" on a merged MR's labels, "Nothing pinned yet",
"nothing here yet", "nothing unread", the old `mrs.html` CLI-command
text) to expect the new ones — these are exactly the tests this sweep
is supposed to change.

- [ ] **Step 9: Commit**

```bash
git add internal/web/templates/mr.html internal/web/templates/builds.html internal/web/templates/dashboard.html internal/web/templates/notifications.html internal/web/templates/mrs.html internal/httpd/mrpage_test.go internal/httpd/*_test.go
git commit -m "web: one empty-state register — no CLI commands, no 'yet' on finished items" -m "Ref #270"
```

### Task 4.2: MR list contribution hint by access level

**Files:**
- Modify: `internal/httpd/web.go:2061-2126` (`mrs` handler, add `CanWrite`)
- Modify: `internal/web/templates/mrs.html:16`
- Test: `internal/httpd/mrslist_test.go` (create, or add beside an
  existing `mrs.html` render test if one exists — check first)

- [ ] **Step 1: Write the failing test**

```go
package httpd

import (
	"net/http"
	"net/http/httptest"
	"strings"
	"testing"
	"time"

	"gitbay.org/gitbay/internal/config"
	"gitbay.org/gitbay/internal/store"
)

// A repository's MR list offers the right next step by access level: a
// writer gets "New merge request", a signed-in reader without push gets
// a fork link, and a signed-out visitor gets a sign-in prompt (#270).
func TestMRsListContributionHintByAccess(t *testing.T) {
	st, err := store.Open(":memory:")
	if err != nil {
		t.Fatal(err)
	}
	defer st.Close()
	if err := st.MigrateUp(); err != nil {
		t.Fatal(err)
	}
	owner, err := st.CreateUser("alice", false)
	if err != nil {
		t.Fatal(err)
	}
	reader, err := st.CreateUser("bob", false)
	if err != nil {
		t.Fatal(err)
	}
	if _, err := st.CreateRepo("user", owner, "app", "public"); err != nil {
		t.Fatal(err)
	}

	cfg := config.Default()
	cfg.Web.Mode = "accounts"
	s := New(cfg, st)

	// mrs reads the viewer through s.viewer(r), which resolves a
	// session cookie (internal/httpd/accounts.go:37-47) rather than
	// taking the viewer as a parameter the way a POST handler test
	// does. Give a real viewer a real session; leave the request
	// cookie-less for the anonymous case.
	sessionFor := func(uid int64) *http.Cookie {
		tok, hash, err := store.NewToken()
		if err != nil {
			t.Fatal(err)
		}
		if err := st.CreateWebSession(hash, uid, time.Hour); err != nil {
			t.Fatal(err)
		}
		return s.sessionCookieFor(tok)
	}

	get := func(uid int64) string {
		req := httptest.NewRequest("GET", "/alice/app/mrs", nil)
		req.SetPathValue("owner", "alice")
		req.SetPathValue("repo", "app")
		if uid != 0 {
			req.AddCookie(sessionFor(uid))
		}
		rr := httptest.NewRecorder()
		s.mrs(rr, req)
		return rr.Body.String()
	}
	anonymous := get(0)
	if !strings.Contains(anonymous, "Sign in to propose a change") {
		t.Errorf("signed-out visitor: missing sign-in prompt:\n%s", anonymous)
	}
	if strings.Contains(anonymous, "New merge request") {
		t.Error("signed-out visitor should not see New merge request")
	}

	readerOut := get(reader)
	if !strings.Contains(readerOut, "Fork this repository to propose a change") {
		t.Errorf("reader without push: missing fork hint:\n%s", readerOut)
	}

	ownerOut := get(owner)
	if !strings.Contains(ownerOut, "New merge request") {
		t.Errorf("owner: missing New merge request link:\n%s", ownerOut)
	}
}
```

- [ ] **Step 2: Run and see it fail**

Run: `go test ./internal/httpd -run TestMRsListContributionHintByAccess -count=1`
Expected: FAIL (no such text yet — `mrs.html:16` still only checks
`.Viewer`).

- [ ] **Step 3: Add `CanWrite` to the `mrs` page struct**

In `internal/httpd/web.go`, in `mrs` (around line 2061), after
`p.Tab = "merge requests"`:

```go
	canWrite := s.canWriteRepo(r, p.Repo)
```

and add `CanWrite bool` to the anonymous struct passed to `s.render`,
with `canWrite` in the corresponding position of the literal.

- [ ] **Step 4: Update `mrs.html`**

Find (`mrs.html:16`):

```html
{{if .Viewer}}
    New merge request
    {{end}}
```

Replace:

```html
{{if .CanWrite}}
    New merge request
    
{{else if .Viewer}}
    Fork this repository to propose a change
    
{{else}}
    Sign in to propose a change
    {{end}}
```

- [ ] **Step 5: Run**

Run: `go test ./internal/httpd -run TestMRsListContributionHintByAccess -count=1 && go test ./internal/httpd -count=1`
Expected: PASS.

- [ ] **Step 6: Commit**

```bash
git add internal/httpd/web.go internal/web/templates/mrs.html internal/httpd/mrslist_test.go
git commit -m "web: MR list offers a fork link or a sign-in prompt to visitors who cannot open one directly" -m "Ref #270"
```

### Task 4.3: search scope caption always visible; tab zero-count rule

**Files:**
- Modify: `internal/web/templates/globalsearch.html:45-48`
- Modify: `internal/web/templates/layout.html:71-72` (comment only — see
  Step 2)
- Test: `internal/httpd/searchweb_test.go` (or wherever a
  `globalsearch.html` render test already lives)

- [ ] **Step 1: Write the failing test**

```go
// The scope sentence is a permanent caption, not a first-visit-only
// hint: a visitor who has already searched still needs to know what a
// search here does and does not cover (#270).
func TestGlobalSearchScopeCaptionAlwaysShown(t *testing.T) {
	var sb strings.Builder
	if err := web.Render(&sb, "globalsearch.html", struct {
		basePage
		Query, Kind string
		Results     []searchHit
		QueryErr    string
	}{Query: "gitbay"}); err != nil {
		t.Fatalf("render: %v", err)
	}
	if !strings.Contains(sb.String(), "File contents are searched per repository") {
		t.Error("scope caption missing once a query is present")
	}
}
```

(Check the real render struct's field names and the `searchHit` type
name with `grep -n '"globalsearch.html"' internal/httpd/*.go` and match
them exactly — the struct above is a best guess at the shape from
reading the template, not a verified signature.)

- [ ] **Step 2: Run and see it fail**

Run: `go test ./internal/httpd -run TestGlobalSearchScopeCaptionAlwaysShown -count=1`
Expected: FAIL (the caption is currently inside the `{{else}}` branch
that only renders when `.Query` is empty).

- [ ] **Step 3: Move the caption out of the conditional**

Find (`globalsearch.html:45-48`):

```html
{{/* The count line above already says nothing matched, so this one
     carries the way out instead of repeating it. */}}
{{else}}
    Try fewer words{{if .Kind}}, search everything,{{end}} or browse the repositories.
    {{end}}
{{else}}

    Repository names, descriptions and topics, and the title and body of every issue and merge request you can read. File contents are searched per repository, from a repository's Code tab.
    
{{end}}
```

Replace with (the scope sentence moves out to render unconditionally,
right after the search form, and the "no results" hint keeps its own
conditional unchanged):

```html
{{/* The count line above already says nothing matched, so this one
     carries the way out instead of repeating it. */}}
{{else}}
    Try fewer words{{if .Kind}}, search everything,{{end}} or browse the repositories.
    {{end}}
{{end}}

    Repository names, descriptions and topics, and the title and body of every issue and merge request you can read. File contents are searched per repository, from a repository's Code tab.
    
```

(Dropping the outer `{{if .QueryErr}}...{{else if .Query}}...{{else}}...{{end}}`'s
final `{{else}}` branch this way requires re-reading the template's
actual brace nesting before editing — the three-way `{{if
.QueryErr}}{{else if .Query}}{{else}}{{end}}` collapses to a two-way
`{{if .QueryErr}}{{else}}...{{end}}` once the "no query yet" case no
longer needs its own branch for this sentence. Read
`globalsearch.html`'s full `{{if}}/{{else}}` structure before editing —
line numbers above are from this plan's research and may have shifted.)

- [ ] **Step 4: Run**

Run: `go test ./internal/httpd -run TestGlobalSearchScopeCaptionAlwaysShown -count=1 && go test ./internal/httpd -count=1`
Expected: PASS.

- [ ] **Step 5: Document the tab zero-count rule (no code change: the
  current behaviour is already the rule)**

Reading `layout.html:71-74`: `Issues` and `Merge requests` already hide
their count badge at zero (`{{with field $ "OpenIssues"}}{{if .}} {{.}}{{end}}{{end}}`,
same for `OpenMRs`); `Builds`, `Releases`, `Wiki` and `Settings` never
carry a count at all. The inconsistency the issue names ("Issues 9, then
Merge requests with no count") is two tabs following the same rule
producing different-looking output depending on the data, not a code
bug — but `dashboard.html`'s pin row (`{{.Issues}} ...`)
always prints the number, including `0`, which genuinely is a different
rule from the tabs'. Fix that inconsistency by hiding a zero the same
way the tabs do. Find (`dashboard.html:27`, inside the pin row):

```html
{{.Issues}} open issues {{.MRs}} open merge requests
```

Replace:

```html
{{if .Issues}}{{.Issues}}{{else}}0{{end}} open issues {{if .MRs}}{{.MRs}}{{else}}0{{end}} open merge requests
```

Wait — re-read this before implementing: this keeps `0` printed, which
does not change anything (`{{.Issues}}` and `{{if .Issues}}{{.Issues}}{{else}}0{{end}}`
render identically for an int, since Go's `%v`-style template output of
`0` is already `"0"`). The dashboard pin row is not actually
inconsistent with the tabs in a way a template edit can fix: it is a
`` badge that always shows a resting value (like a count chip
elsewhere in the app, e.g. label/milestone counts), whereas the tabs
hide their `` badge entirely at zero because an empty `` there
would look like stray punctuation next to the tab word. These are
two different UI elements with two different, both-reasonable rules.
Do not change `dashboard.html` in this task. Instead add a one-line
comment at `layout.html:69` (just above the `