docs/plans/2026-09-27-web-ux.md
3045 lines · 110761 bytes
41 symbols in this file
Web UX sweep implementation planGlobal constraintsOrder and dependenciesMR 1: architecture-review small fixes (branch `web-audit-fixes`)Task 1.1: run `foreign_key_check` inside the migration transaction, before commitTask 1.2: `pinToggle` and `watchToggle` dispatch through their commandsTask 1.3: Cache-Control: no-store on the login-link consuming requestTask 1.4: doc drift — API.org, Parity.org, Threat-Model.orgTask 1.5: open MR 1MR 2: settings page quotes working commands (branch `web-settings-commands`)Task 2.1: fix the two known-wrong quoted commandsTask 2.2: auth summary and help — done in the CLI UX planTask 2.3: a test that runs every quoted command through the registryTask 2.4: open MR 2MR 3: MR range-diff page (branch `web-mr-range-diff`)Task 3.1: `/{owner}/{repo}/mrs/{n}/range-diff`Task 3.2: revisions list with a "compare to previous" link per rowTask 3.3: update ParityTask 3.4: open MR 3MR 4: empty states and contribution hints sweep (branch `web-empty-states`)Task 4.1: apply the tableTask 4.2: MR list contribution hint by access levelTask 4.3: search scope caption always visible; tab zero-count ruleTask 4.4: open MR 4MR 5: UX review small fixes (branch `web-ux-small-fixes`)Task 5.1: issue form gains milestone and assigneeTask 5.2: "Muted" reachable on the watch controlTask 5.3: rail and phone "More" menu render from one listTask 5.4: "Discussion" heading before the comment threadTask 5.5: build page's "Live" note says the page updates itselfTask 5.6: open MR 5MR 6: wiki non-page links go to `_raw` (branch `wiki-raw-links`)Task 6.1: `rewriteWikiLinks` sends a non-page file link to `_raw`Task 6.2: open MR 6MR 7: API token page (branch `web-api-tokens`)Task 7.1: `Settings → Tokens`: create, list, revokeTask 7.2: `registered.html` next steps as a numbered listTask 7.3: update ParityTask 7.4: open MR 7Self-reviewOpen questions
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 withRef #N, and the last commit closing an issue ends withCloses #N. No attribution to any assistant, model or AI anywhere: commits, MR bodies, comments. gitbay mr create --source <branch> --target main --title "..."; merge withgitbay mr merge <n> --strategy ffonce CI is green, then delete the branch locally and on the remote. If the merge reports the branch is behind, rebase ontomain, 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 ./...), includingTestMainWidthClass,e2e/readonly_test.go'sreadArgscoverage, and thecmd/gitbaycoverage test overpass()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 inTestMainWidthClass'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 forpinToggleandwatchToggle, and this plan does not introduce a new instance of it. Reads may call the store directly (the existing convention throughoutinternal/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
web-audit-fixes(branchweb-audit-fixes) — closes #261. Independent. Landing this first matters because MR 6 (#271's mute option) depends on the toggle-dispatch fix here.web-settings-commands(branchweb-settings-commands) — closes #263. Independent of 1. Lands aftercli-ux-help(CLI UX plan), which carries the auth summary and help part of #263.web-mr-range-diff(branchweb-mr-range-diff) — closes #269. Independent.web-empty-states(branchweb-empty-states) — closes #270. Independent, but touchesmrs.htmlandglobalsearch.html; land before MR 6 to avoid the same files diverging on two branches at once (MR 6 does not touch either).web-ux-small-fixes(branchweb-ux-small-fixes) — closes #271. Depends on MR 1 (repo watch/repo mutedispatch and the cycling toggle it introduces; #271's Muted option builds directly on it).wiki-raw-links(branchwiki-raw-links) — closes #283. Independent; small, can land anywhere, placed last only because it is unrelated to the rest.web-api-tokens(branchweb-api-tokens) — closes #264. Depends on plan 1 (credentials-and-sessions, #257): that plan changestoken create's own default scope toread. This plan's Task 7.1 makes the web form always send an explicit--scopevalue 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 totoken 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, thefkOffbranch) - Test:
internal/store/store_test.go(create the case if no existing migration test exercises anfkOffstep; 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:
// 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
stepto 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:
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:
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
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.goorinternal/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 bybookmarkToggle, 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):
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:
// 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:
// 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:
// 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:
// 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
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:
// 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:
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
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 <n> — 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
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
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
gitbay mr merge <n> --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
whoamiline
Find (account.html:199-200):
<p class="meta">All of it works from stock OpenSSH too:
<code>ssh git@{{.Host}} auth whoami</code>.</p>
auth is a CLI-only grouping (cmd/gitbay/main.go's authCmd); the
server command is whoami (internal/control/identity.go:18). Replace:
<p class="meta">All of it works from stock OpenSSH too:
<code>ssh git@{{.Host}} whoami</code>.</p>
- Step 2: Show both forms for the token line
Find (account.html:194-197):
<pre class="message" tabindex="0">gitbay auth token create --name laptop # API tokens
gitbay web sessions list # browser sessions
gitbay admin ... # instance administration</pre>
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:
<pre class="message" tabindex="0">gitbay auth token create --name laptop # API tokens
gitbay web sessions list # browser sessions
gitbay admin ... # instance administration</pre>
<p class="meta">All of it works from stock OpenSSH too, with the CLI's
grouping words dropped: <code>ssh git@{{.Host}} whoami</code>,
<code>ssh git@{{.Host}} token create --name laptop</code>.</p>
(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 <pre>
block, then one <p class="meta"> with both stock-SSH examples.)
- Step 3: Fix
gitbay account export
Find (account.html:186, in the Export section):
<p class="meta">Your profile, repositories, issues and merge requests as one
JSON bundle, the same one <code>gitbay account export</code> writes. Keys are
never included; a replayed bundle's emails arrive unverified.</p>
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:
<p class="meta">Your profile, repositories, issues and merge requests as one
JSON bundle, the same one <code>gitbay auth export</code> writes. Keys are
never included; a replayed bundle's emails arrive unverified.</p>
- 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 packagemain),control.Lookup(internal/control/control.go:102),web.Pages/web.TemplateSource(internal/web/web.go:271,277). -
Step 1: Write the test
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 <code>, or one per line in a <pre class="quickstart"> quickstart
// block. Both need (?s) so a multi-line <pre> is captured as one match.
var quotedRe = regexp.MustCompile(`(?s)<code>(.*?)</code>|<pre class="quickstart"[^>]*>(.*?)</pre>`)
// 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
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
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 <n> --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'swidemap) - 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
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:
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:
{{define "width"}}wide{{end}}
{{define "title"}}range-diff · !{{.MR.Number}} · {{.Repo.OwnerName}}/{{.Repo.Name}}{{end}}
{{define "content"}}
<h1>Range-diff <span class="issuenumber">!{{.MR.Number}}</span></h1>
<p class="meta"><a href="/{{.Repo.OwnerName}}/{{.Repo.Name}}/mrs/{{.MR.Number}}">back to !{{.MR.Number}} {{.MR.Title}}</a></p>
{{if .Error}}<p class="error" role="alert">{{.Error}}</p>
{{else if .Diff}}<pre class="code buildlog" tabindex="0">{{.Diff}}</pre>
{{else}}<p class="empty-note">Nothing to compare: this merge request has one revision.</p>{{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):
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
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:
// 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):
{{if gt (len .Revisions) 1}}<p class="row none">{{len .Revisions}} revisions pushed. What changed between the last two:
<code>gitbay mr range-diff {{.Repo.OwnerName}}/{{.Repo.Name}} {{.MR.Number}}</code></p>{{end}}
</div>
Replace with:
</div>
{{if .Revisions}}<div class="grp">
<h2>Revisions</h2>
{{range $i, $rv := .Revisions}}<p class="row none">{{add $i 1}}. <code>{{short $rv.SHA}}</code> {{when $rv.CreatedAt}}{{if $i}} · <a href="{{$base}}/range-diff?from={{(index $.Revisions (sub $i 1)).SHA}}&to={{$rv.SHA}}">compare to previous</a>{{end}}</p>
{{end}}
</div>{{end}}
(The closing </div> 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
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
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
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 <code>gitbay mr create {{.Repo.OwnerName}}/{{.Repo.Name}} --source ... --target {{.Repo.DefaultBranch}}</code> |
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 <code>.gitbay/ci.yml</code> |
no builds when the viewer cannot push (.CanWrite false or absent); no builds — push a commit with a .gitbay/ci.yml (drop <code>, 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 <li class="empty"> 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 — <a>show all</a> |
no unread notifications — <a href="/notifications?all=1">show all</a> (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
queuepartial before touchingdashboard.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 abuilds.htmlrender test already lives — check withgrep -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:
// 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):
// 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.htmlreviewers and labels
Find (mr.html:143):
{{else}}<p class="none">nobody yet</p>{{end}}
(in the Reviewers grp). Replace:
{{else}}<p class="none">no reviewers</p>{{end}}
Find (mr.html:174, in the Labels grp):
{{else}}<p class="none">none yet</p>{{end}}
Replace:
{{else}}<p class="none">{{if or (eq .MR.State "merged") (eq .MR.State "closed")}}no labels{{else}}none yet{{end}}</p>{{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):
{{else}}<li class="empty">no builds{{if .CanWrite}} — push a commit with a <code>.gitbay/ci.yml</code>{{end}}</li>{{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):
{{else}}<p class="none">Nothing pinned yet. Press Pin on a repository.</p>{{end}}
Replace:
{{else}}<p class="none">nothing pinned — press Pin on a repository you visit</p>{{end}}
- Step 6:
notifications.html
Find (notifications.html:21):
{{else}}<li class="empty">{{if .All}}nothing here yet{{else}}nothing unread — <a href="/notifications?all=1">show all</a>{{end}}</li>{{end}}
Replace:
{{else}}<li class="empty">{{if .All}}nothing to show{{else}}no unread notifications — <a href="/notifications?all=1">show all</a>{{end}}</li>{{end}}
- Step 7:
mrs.html
Find (mrs.html:28-29):
{{else}}{{if .Query}}<li class="empty">no {{if ne .State "all"}}{{.State}} {{end}}merge requests matching “{{.Query}}”</li>
{{else}}<li class="empty">no {{if ne .State "all"}}{{.State}} {{end}}merge requests — open one with <code>gitbay mr create {{.Repo.OwnerName}}/{{.Repo.Name}} --source ... --target {{.Repo.DefaultBranch}}</code></li>{{end}}{{end}}
Replace (the create instruction moves to Task 4.2's contribution-hint line, so the empty state itself states only the fact):
{{else}}{{if .Query}}<li class="empty">no {{if ne .State "all"}}{{.State}} {{end}}merge requests matching “{{.Query}}”</li>
{{else}}<li class="empty">no {{if ne .State "all"}}{{.State}} {{end}}merge requests</li>{{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
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(mrshandler, addCanWrite) -
Modify:
internal/web/templates/mrs.html:16 -
Test:
internal/httpd/mrslist_test.go(create, or add beside an existingmrs.htmlrender test if one exists — check first) -
Step 1: Write the failing test
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
CanWriteto themrspage struct
In internal/httpd/web.go, in mrs (around line 2061), after
p.Tab = "merge requests":
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):
{{if .Viewer}}<p class="meta"><a href="/{{.Repo.OwnerName}}/{{.Repo.Name}}/mrs/new">New merge request</a></p>{{end}}
Replace:
{{if .CanWrite}}<p class="meta"><a href="/{{.Repo.OwnerName}}/{{.Repo.Name}}/mrs/new">New merge request</a></p>
{{else if .Viewer}}<p class="meta"><a href="/{{.Repo.OwnerName}}/{{.Repo.Name}}/fork">Fork this repository to propose a change</a></p>
{{else}}<p class="meta"><a href="/login">Sign in to propose a change</a></p>{{end}}
- Step 5: Run
Run: go test ./internal/httpd -run TestMRsListContributionHintByAccess -count=1 && go test ./internal/httpd -count=1
Expected: PASS.
- Step 6: Commit
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 aglobalsearch.htmlrender test already lives) -
Step 1: Write the failing test
// 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):
{{/* The count line above already says nothing matched, so this one
carries the way out instead of repeating it. */}}
{{else}}<p class="empty-note">Try fewer words{{if .Kind}}, <a href="?q={{.Query}}">search everything</a>,{{end}} or <a href="/explore">browse the repositories</a>.</p>{{end}}
{{else}}
<p class="empty-note">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.</p>
{{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):
{{/* The count line above already says nothing matched, so this one
carries the way out instead of repeating it. */}}
{{else}}<p class="empty-note">Try fewer words{{if .Kind}}, <a href="?q={{.Query}}">search everything</a>,{{end}} or <a href="/explore">browse the repositories</a>.</p>{{end}}
{{end}}
<p class="meta">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.</p>
(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 .}} <i>{{.}}</i>{{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 (<b{{if .Issues}} class="wants"{{end}}>{{.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):
<b{{if .Issues}} class="wants"{{end}}>{{.Issues}} <span class="vh">open issues</span></b> <b{{if .MRs}} class="wants"{{end}}>{{.MRs}} <span class="vh">open merge requests</span></b>
Replace:
<b{{if .Issues}} class="wants"{{end}}>{{if .Issues}}{{.Issues}}{{else}}0{{end}} <span class="vh">open issues</span></b> <b{{if .MRs}} class="wants"{{end}}>{{if .MRs}}{{.MRs}}{{else}}0{{end}} <span class="vh">open merge requests</span></b>
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
<b> 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 <i> badge entirely at zero because an empty <i> 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 <nav class="tabs">)
recording the decision so a future pass does not "fix" this again:
{{/* A tab's own count badge is omitted at zero (an empty <i> reads as
stray punctuation next to the tab word); the dashboard pin row's
count chip always shows its number, zero included, the same as
every other count chip in the app. Two elements, two rules,
decided once here (#270). */}}
<nav class="tabs" aria-label="Repository">
- Step 6: Run the full package once more
Run: go test ./internal/httpd ./internal/web -count=1
Expected: PASS.
- Step 7: Commit
git add internal/web/templates/globalsearch.html internal/web/templates/layout.html internal/httpd/searchweb_test.go
git commit -m "web: search scope caption is permanent; document the tab zero-count rule" -m "Closes #270"
Task 4.4: open MR 4
git push -u origin web-empty-states
gitbay mr create --source web-empty-states --target main --title "Web: empty-state and contribution-hint sweep"
Wait for CI, merge, delete the branch both places.
MR 5: UX review small fixes (branch web-ux-small-fixes)
Closes #271. Depends on MR 1 (repo watch/repo mute dispatch and the
cycling toggle).
Task 5.1: issue form gains milestone and assignee
Files:
- Modify:
internal/web/templates/issuenew.html - Modify:
internal/httpd/accounts.go:455-477(issueCreateSubmit) - Test:
internal/httpd/accounts_test.goor wherever an existingissueCreateSubmittest lives (grep -rln "issueCreateSubmit" internal/httpd/*_test.go)
Ground truth from reading the code: issue create
(internal/control/issue.go:18-31) takes only --title, --body/
--file, and --format — no --milestone/--assignee flags.
issueCreateSubmit (internal/httpd/accounts.go:455-477) already
handles this shape for labels: it creates the issue first (decoding the
created issue's number via dispatchIntoStdin into control.Created),
then, only if the labels field was non-empty, makes a second dispatch
(issue label ... --add ...) with that number. Milestone and assignee
follow the same two-step shape, using the existing commands issue milestone <owner/name> <n> <title> and issue assign <owner/name> <n> [--add <user>] (internal/control/issue.go:101-109 for assign; the
milestone command's exact path is confirmed by
internal/httpd/issueactions.go's issueMilestoneSubmit, which already
calls issueArgs(r, "milestone", title)).
- Step 1: Write the failing test
// The new-issue form takes milestone and assignee, the same as the
// issue page's own edit controls already do (#271).
func TestIssueCreateFormHasMilestoneAndAssignee(t *testing.T) {
var sb strings.Builder
if err := web.Render(&sb, "issuenew.html", struct {
basePage
Repo store.Repo
Milestones []string
Draft *draft
}{Repo: store.Repo{OwnerName: "alice", Name: "app"}}); err != nil {
t.Fatalf("render: %v", err)
}
out := sb.String()
if !strings.Contains(out, `name="milestone"`) {
t.Error("no milestone field")
}
if !strings.Contains(out, `name="assignee"`) {
t.Error("no assignee field")
}
}
(Match the render struct to whatever issueCreateForm actually passes —
read its handler first, per Step 1, and adjust field names here.)
- Step 2: Run and see it fail
Run: go test ./internal/httpd -run TestIssueCreateFormHasMilestoneAndAssignee -count=1
Expected: FAIL.
- Step 3: Add the fields to the form
Add to issuenew.html, alongside the existing labels input (matching
its markup style exactly — an <input> with the same classes/attributes
the labels field uses, adjusted for name and placeholder):
<p><input type="text" name="milestone" aria-label="Milestone" placeholder="milestone (optional)"></p>
<p><input type="text" name="assignee" aria-label="Assignee" placeholder="assignee, one username (optional)"></p>
Place these after the existing labels <input> and before the submit
button, matching the vertical rhythm (<p> wrapping) the rest of the
form uses.
- Step 4: Wire them into the handler as follow-up dispatches
In internal/httpd/accounts.go, issueCreateSubmit (lines 455-477),
add two more follow-up dispatches after the existing labels one, using
the same n (the created issue's number, already decoded from
created.Number):
if args := fieldArgs("--add", r.FormValue("labels")); len(args) > 0 {
s.runControl(u, append([]string{"issue", "label", repoPath, fmt.Sprint(n)}, args...))
}
if milestone := strings.TrimSpace(r.FormValue("milestone")); milestone != "" {
s.runControl(u, []string{"issue", "milestone", repoPath, fmt.Sprint(n), milestone})
}
if assignee := strings.TrimSpace(r.FormValue("assignee")); assignee != "" {
s.runControl(u, []string{"issue", "assign", repoPath, fmt.Sprint(n), "--add", assignee})
}
http.Redirect(w, r, fmt.Sprintf("/%s/issues/%d", repoPath, n), http.StatusSeeOther)
(The first block — the existing labels dispatch — is unchanged; the milestone and assignee blocks are new, inserted between it and the final redirect.)
- Step 5: Run
Run: go test ./internal/httpd -run TestIssueCreateFormHasMilestoneAndAssignee -count=1 && go test ./internal/httpd -count=1
Expected: PASS.
- Step 6: Commit
git add internal/web/templates/issuenew.html internal/httpd/accounts.go internal/httpd/*_test.go
git commit -m "web: new-issue form takes milestone and assignee" -m "Ref #271"
Task 5.2: "Muted" reachable on the watch control
MR 1 (Task 1.2) already made watchToggle cycle default → watching →
muted → default, closing the functional half of this. This task is the
UI half: the header button's label and title must describe all three
states (today it only ever renders "Watch" or "Watching").
Files:
-
Modify:
internal/web/templates/layout.html:64 -
Test: a
layout.htmlrender test, or a repo-page test that already checks the watch button (grep -rln 'aria-pressed.*Watch\|"Watching"' internal/httpd/*_test.go) -
Step 1: Write the failing test
// The watch button names all three states it cycles through, including
// muted, which MR 1 made reachable (#271).
func TestRepoHeaderWatchButtonNamesMutedState(t *testing.T) {
var sb strings.Builder
rp := testRepoPage()
rp.Watch = "muted"
if err := web.Render(&sb, "dashboard.html", struct {
repoPage
}{rp}); err != nil {
t.Fatalf("render: %v", err)
}
if !strings.Contains(sb.String(), "Muted") {
t.Error(`watch button does not render "Muted" for a muted repo`)
}
}
dashboard.html is not a repo page and will not carry the header at
all — use whatever page template this plan's other tasks already found
does render field $ "Repo" (any repoPage-embedding page works,
e.g. mr.html); adjust the render call to a page that actually shows
the header (check with grep -n 'field \$ "Repo"' internal/web/templates/layout.html
and pick any page in the repoPage family, such as mrs.html, matching
whatever fixture data that page's own tests already use).
- Step 2: Run and see it fail
Run: go test ./internal/httpd -run TestRepoHeaderWatchButtonNamesMutedState -count=1
Expected: FAIL.
- Step 3: Update the button
Find (layout.html:64):
<form method="post" action="/{{.OwnerName}}/{{.Name}}/watch" class="inline"><button type="submit" class="btn" aria-pressed="{{if eq (str $ "Watch") "watching"}}true{{else}}false{{end}}" title="Watching sends every issue, request and build to your inbox">{{if eq (str $ "Watch") "watching"}}Watching{{else}}Watch{{end}}</button></form>
Replace:
<form method="post" action="/{{.OwnerName}}/{{.Name}}/watch" class="inline"><button type="submit" class="btn" aria-pressed="{{if ne (str $ "Watch") ""}}true{{else}}false{{end}}" title="{{if eq (str $ "Watch") "watching"}}Watching: every issue, request and build. Click to mute.{{else if eq (str $ "Watch") "muted"}}Muted: nothing from this repository. Click to stop muting.{{else}}Only what involves you. Click to watch everything.{{end}}">{{if eq (str $ "Watch") "watching"}}Watching{{else if eq (str $ "Watch") "muted"}}Muted{{else}}Watch{{end}}</button></form>
- Step 4: Run
Run: go test ./internal/httpd -run TestRepoHeaderWatchButtonNamesMutedState -count=1 && go test ./internal/httpd -count=1
Expected: PASS.
- Step 5: Commit
git add internal/web/templates/layout.html internal/httpd/*_test.go
git commit -m "web: watch button names all three states, muted included" -m "Ref #271"
Task 5.3: rail and phone "More" menu render from one list
Files:
-
Modify:
internal/web/web.go(addrailItemtype,railOptItemsfunc, register it infuncs) -
Modify:
internal/web/templates/layout.html:26,30-31,37-40 -
Test:
internal/web/web_test.go(TestRailIconsAreLabelledalready parseslayout.html; add a focused new test rather than folding into that one) -
Step 1: Write the failing test
Add to internal/web/web_test.go:
// The main rail and the phone "More" menu render New repository,
// Settings, Admin and Log out from one list, so adding a destination in
// one place reaches both (#271).
func TestRailOptItemsDriveBothRailAndMoreMenu(t *testing.T) {
items := railOptItems(struct {
Tab string
Admin bool
}{Tab: "admin", Admin: true})
if len(items) != 4 {
t.Fatalf("got %d items, want 4 (New repository, Settings, Admin, Log out)", len(items))
}
if items[2].Name != "Admin" || !items[2].Show {
t.Errorf("Admin item: %+v", items[2])
}
if !items[2].Current {
t.Error("Admin item should be Current when Tab is admin")
}
nonAdmin := railOptItems(struct {
Tab string
Admin bool
}{Tab: "account"})
if nonAdmin[2].Show {
t.Error("Admin item should not Show for a non-admin viewer")
}
if !nonAdmin[1].Current {
t.Error("Settings item should be Current when Tab is account")
}
}
- Step 2: Run and see it fail to compile
Run: go test ./internal/web -run TestRailOptItemsDriveBothRailAndMoreMenu -count=1
Expected: FAIL (railOptItems undefined).
- Step 3: Add the type and function
In internal/web/web.go, near the other template-data helpers (before
the funcs map, so it can be referenced there):
// railItem is one destination the rail's icon strip and the phone
// "More" menu both render — from this one list, so a destination added
// here reaches both instead of the two being hand-kept in step (#271).
type railItem struct {
Href string
Icon string
Name string
Current bool
Count int64 // unused by railOptItems; present so "raillink" can read it uniformly
Show bool
}
// railField and railBool read a named field off the page value the
// layout was given — the same reflection str/field already do for the
// repo header, duplicated narrowly here rather than exported, since
// railOptItems is their only other caller.
func railField(v any, name string) string {
rv := reflect.ValueOf(v)
for rv.Kind() == reflect.Ptr || rv.Kind() == reflect.Interface {
rv = rv.Elem()
}
if rv.Kind() != reflect.Struct {
return ""
}
f := rv.FieldByName(name)
if !f.IsValid() || f.Kind() != reflect.String {
return ""
}
return f.String()
}
func railBool(v any, name string) bool {
rv := reflect.ValueOf(v)
for rv.Kind() == reflect.Ptr || rv.Kind() == reflect.Interface {
rv = rv.Elem()
}
if rv.Kind() != reflect.Struct {
return false
}
f := rv.FieldByName(name)
return f.IsValid() && f.Kind() == reflect.Bool && f.Bool()
}
// railOptItems is the rail's "New repository", "Settings", "Admin" and
// "Log out" destinations, in the order the rail shows them. v is the
// page value the layout renders (any page struct that embeds
// basePage), read by field name since the layout has no single common
// type for every page.
func railOptItems(v any) []railItem {
tab := railField(v, "Tab")
admin := railBool(v, "Admin")
return []railItem{
{Href: "/new", Icon: "plus", Name: "New repository", Show: true},
{Href: "/settings", Icon: "gear", Name: "Settings", Current: tab == "account", Show: true},
{Href: "/admin", Icon: "shield", Name: "Admin", Current: tab == "admin", Show: admin},
{Href: "/logout", Icon: "signout", Name: "Log out", Show: true},
}
}
Add "reflect" to the file's imports if not already present (it almost
certainly is, since str/field already use it).
Register it in the funcs map (anywhere in the literal, e.g. beside
"initial"):
"railOptItems": railOptItems,
- Step 4: Run the new test
Run: go test ./internal/web -run TestRailOptItemsDriveBothRailAndMoreMenu -count=1
Expected: PASS.
- Step 5: Use it in
layout.html
Find (lines 21-32):
<ul class="raillist">
{{if .Viewer}}<li>{{template "raillink" dict "Href" "/" "Icon" "home" "Name" "Dashboard" "Current" (eq (str . "Tab") "dashboard")}}</li>{{end}}
<li>{{template "raillink" dict "Href" "/explore" "Icon" "compass" "Name" "Explore" "Current" (eq (str . "Tab") "explore")}}</li>
<li>{{template "raillink" dict "Href" "/search" "Icon" "search" "Name" "Search" "Current" (eq (str . "Tab") "sitesearch")}}</li>
{{if .Viewer}}<li>{{template "raillink" dict "Href" "/notifications" "Icon" "bell" "Name" "Notifications" "Current" (eq (str . "Tab") "notifications") "Count" .Rail.Unread}}</li>
<li class="railopt">{{template "raillink" dict "Href" "/new" "Icon" "plus" "Name" "New repository"}}</li>{{end}}
</ul>
<span class="railgap"></span>
<ul class="raillist">
{{if .Viewer}}<li class="railopt">{{template "raillink" dict "Href" "/settings" "Icon" "gear" "Name" "Settings" "Current" (eq (str . "Tab") "account")}}</li>{{end}}
{{if .Admin}}<li class="railopt">{{template "raillink" dict "Href" "/admin" "Icon" "shield" "Name" "Admin" "Current" (eq (str . "Tab") "admin")}}</li>{{end}}
</ul>
Replace:
<ul class="raillist">
{{if .Viewer}}<li>{{template "raillink" dict "Href" "/" "Icon" "home" "Name" "Dashboard" "Current" (eq (str . "Tab") "dashboard")}}</li>{{end}}
<li>{{template "raillink" dict "Href" "/explore" "Icon" "compass" "Name" "Explore" "Current" (eq (str . "Tab") "explore")}}</li>
<li>{{template "raillink" dict "Href" "/search" "Icon" "search" "Name" "Search" "Current" (eq (str . "Tab") "sitesearch")}}</li>
{{if .Viewer}}<li>{{template "raillink" dict "Href" "/notifications" "Icon" "bell" "Name" "Notifications" "Current" (eq (str . "Tab") "notifications") "Count" .Rail.Unread}}</li>
<li class="railopt">{{template "raillink" (index (railOptItems .) 0)}}</li>{{end}}
</ul>
<span class="railgap"></span>
<ul class="raillist">
{{if .Viewer}}<li class="railopt">{{template "raillink" (index (railOptItems .) 1)}}</li>
{{if (index (railOptItems .) 2).Show}}<li class="railopt">{{template "raillink" (index (railOptItems .) 2)}}</li>{{end}}{{end}}
</ul>
Find (lines 34-42):
{{if .Viewer}}<details class="railmore">
<summary class="railicon" aria-label="More" title="More">{{template "icon" "ellipsis"}}<span class="vh">More</span></summary>
<div class="raildrop">
<a href="/new">{{template "icon" "plus"}} New repository</a>
<a href="/settings">{{template "icon" "gear"}} Settings</a>
{{if .Admin}}<a href="/admin">{{template "icon" "shield"}} Admin</a>{{end}}
<a href="/logout">{{template "icon" "signout"}} Log out</a>
</div>
</details>
Replace:
{{if .Viewer}}<details class="railmore">
<summary class="railicon" aria-label="More" title="More">{{template "icon" "ellipsis"}}<span class="vh">More</span></summary>
<div class="raildrop">
{{range railOptItems .}}{{if .Show}}<a href="{{.Href}}">{{template "icon" .Icon}} {{.Name}}</a>{{end}}{{end}}
</div>
</details>
- Step 6: Run
Run: go test ./internal/web -count=1 && go test ./internal/httpd -count=1
Expected: PASS. TestRailIconsAreLabelled must still pass unchanged —
raillink's template still receives the same field names (Href,
Icon, Name, Current, Count), now off a railItem struct instead
of a dict map, which html/template's field access treats
identically.
- Step 7: Commit
git add internal/web/web.go internal/web/web_test.go internal/web/templates/layout.html
git commit -m "web: rail and phone More menu render New repository/Settings/Admin/Log out from one list" -m "Ref #271"
Task 5.4: "Discussion" heading before the comment thread
Files:
-
Modify:
internal/web/templates/mr.html(inside theconversationview) -
Modify:
internal/web/templates/issue.html -
Test: existing render tests in
mrpage_test.go; a new or existingissue.htmlrender test -
Step 1: Write the failing tests
Add to internal/httpd/mrpage_test.go:
// A heading precedes the comment thread, so a screen-reader user
// skimming by heading does not fall from the aside's groups straight
// into the first comment with no landmark (#271).
func TestMRPageHasDiscussionHeading(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(), "<h2>Discussion</h2>") {
t.Error("no Discussion heading")
}
}
Add the equivalent for issue.html in whatever file already tests it
(grep -rln '"issue.html"' internal/httpd/*_test.go).
- Step 2: Run and see them fail
Run: go test ./internal/httpd -run TestMRPageHasDiscussionHeading -count=1
Expected: FAIL.
- Step 3:
mr.html
Find, inside the {{if eq .View "conversation"}} block, right after
<div class="prose">:
{{if eq .View "conversation"}}
<div class="prose">
{{if .CanEdit}}<details class="editbox"{{if .Draft.Is "edit"}} open{{end}}><summary>Edit</summary>
Replace:
{{if eq .View "conversation"}}
<div class="prose">
<h2>Discussion</h2>
{{if .CanEdit}}<details class="editbox"{{if .Draft.Is "edit"}} open{{end}}><summary>Edit</summary>
- Step 4:
issue.html
Find, right after <div class="mainside">:
<div class="mainside">
{{if .CanEdit}}<details class="editbox"{{if .Draft.Is "edit"}} open{{end}}><summary>edit</summary>
Replace:
<div class="mainside">
<h2>Discussion</h2>
{{if .CanEdit}}<details class="editbox"{{if .Draft.Is "edit"}} open{{end}}><summary>edit</summary>
- Step 5: Run
Run: go test ./internal/httpd -count=1
Expected: PASS.
- Step 6: Commit
git add internal/web/templates/mr.html internal/web/templates/issue.html internal/httpd/*_test.go
git commit -m "web: Discussion heading before the comment thread on issue and MR pages" -m "Ref #271"
Task 5.5: build page's "Live" note says the page updates itself
Files:
-
Modify:
internal/web/templates/build.html:15 -
Test:
internal/httpd/builds_test.go(or wherever a live-build render test already exists — check withgrep -rln '"Live"' internal/httpd/*_test.go) -
Step 1: Write the failing test
// The Live note says the page updates itself, in plain words, rather
// than the more technical "streams here" (#271).
func TestBuildPageLiveNoteSaysItUpdatesItself(t *testing.T) {
var sb strings.Builder
if err := web.Render(&sb, "build.html", buildView{
repoPage: testRepoPage(), Live: true,
}); err != nil {
t.Fatalf("render: %v", err)
}
if !strings.Contains(sb.String(), "This page updates itself") {
t.Error(`Live note does not say the page updates itself`)
}
}
- Step 2: Run and see it fail
Run: go test ./internal/httpd -run TestBuildPageLiveNoteSaysItUpdatesItself -count=1
Expected: FAIL.
- Step 3: Update the note
Find (build.html:15):
{{if .Live}}<p class="meta">Live: the log streams here until the build ends. If it stops without a “build finished” line, reload to pick it up again. <a href="?follow=0">Show it without updates</a></p>
Replace:
{{if .Live}}<p class="meta">This page updates itself until the build ends. If it stops without a “build finished” line, reload to pick it up again. <a href="?follow=0">Show it without updates</a></p>
- Step 4: Run
Run: go test ./internal/httpd -run TestBuildPageLiveNoteSaysItUpdatesItself -count=1 && go test ./internal/httpd -count=1
Expected: PASS.
- Step 5: Commit
git add internal/web/templates/build.html internal/httpd/builds_test.go
git commit -m "web: build page's Live note says the page updates itself" -m "Closes #271"
Task 5.6: open MR 5
git push -u origin web-ux-small-fixes
gitbay mr create --source web-ux-small-fixes --target main --title "Web UX review small fixes: issue form, mute, rail list, Discussion heading"
Wait for CI, merge, delete the branch both places.
MR 6: wiki non-page links go to _raw (branch wiki-raw-links)
Closes #283.
Task 6.1: rewriteWikiLinks sends a non-page file link to _raw
Files:
- Modify:
internal/httpd/wiki.go:250-253 - Test:
internal/httpd/wiki_test.go
Interfaces:
-
No signature changes —
rewriteWikiLinks's parameters and theisPage/isFilepredicates it already takes are unchanged; only its href branch's internal logic changes. -
Step 1: Write the failing test
Add to internal/httpd/wiki_test.go, in TestRewriteWikiLinksInSubfolder
(extend the existing test rather than adding a new one — it already sets
up exactly the pages/files fixtures this needs):
in := template.HTML(`<a href="Identity.org">i</a><a href="Admin.org">a</a>` +
`<a href="b.svg">diagram</a>` +
`<img src="b.svg"><img src="diagrams/a.svg"><a href="https://x.test/">x</a>`)
out := string(rewriteWikiLinks(in, p, "Architecture/Trust",
func(s string) bool { return pages[s] }, func(s string) bool { return files[s] }))
for _, want := range []string{
`href="/krz/gitbay/wiki/Architecture/Identity"`,
`href="/krz/gitbay/wiki/Admin"`,
`href="/krz/gitbay/wiki/_raw/Architecture/b.svg"`,
`src="/krz/gitbay/wiki/_raw/Architecture/b.svg"`,
`src="/krz/gitbay/wiki/_raw/diagrams/a.svg"`,
`href="https://x.test/"`,
} {
(This adds the <a href="b.svg"> link to the input and the matching
href="...wiki/_raw/Architecture/b.svg" expectation to the existing
for _, want := range loop — the surrounding test body, p, pages
and files setup, and the final if !strings.Contains check stay
exactly as they are.)
- Step 2: Run and see it fail
Run: go test ./internal/httpd -run TestRewriteWikiLinksInSubfolder -count=1
Expected: FAIL — the new href expectation
(href="/krz/gitbay/wiki/_raw/Architecture/b.svg") is missing; today's
code rewrites that link to href="/krz/gitbay/wiki/Architecture/b.svg"
instead (a page-style link to a file that is not a page, which 404s —
the bug #283 reports).
- Step 3: Fix
rewriteWikiLinks
Find (internal/httpd/wiki.go:250-253):
if target, ok := wikiResolve(page, trimPageExt(v), isPage); ok {
n.Attr[i].Val = base + "/" + target
}
Replace:
// A plain link is usually to another page, but a link to
// an existing non-page file (an .svg, .txt, .pdf) must
// go to _raw the same as an image src, or it 404s
// against the page route (#283).
if target, ok := wikiResolve(page, trimPageExt(v), isPage); ok {
if raw, rok := wikiResolve(page, v, isFile); rok && isFile(raw) && !isPage(target) {
n.Attr[i].Val = base + "/_raw/" + raw
} else {
n.Attr[i].Val = base + "/" + target
}
}
- Step 4: Run
Run: go test ./internal/httpd -run TestRewriteWikiLinksInSubfolder -count=1
Expected: PASS.
- Step 5: Run the package
Run: go test ./internal/httpd -count=1
Expected: PASS.
- Step 6: Commit
git add internal/httpd/wiki.go internal/httpd/wiki_test.go
git commit -m "wiki: a link to an existing non-page file resolves to _raw, not the page route" -m "Closes #283"
Task 6.2: open MR 6
git push -u origin wiki-raw-links
gitbay mr create --source wiki-raw-links --target main --title "Wiki: links to non-page files resolve to _raw"
Wait for CI, merge, delete the branch both places.
MR 7: API token page (branch web-api-tokens)
Closes #264. Depends on plan 1 (credentials-and-sessions, #257) per
the "Order and dependencies" section above — implementable and testable
independently, merge after plan 1 lands.
Task 7.1: Settings → Tokens: create, list, revoke
Files:
- Modify:
internal/httpd/account.go(accountPage,accountSubmit) - Modify:
internal/httpd/flash.go(a token-shown-once cookie, parallel to the existing flash cookie) - Modify:
internal/web/templates/account.html - Test:
internal/httpd/account_test.go
Interfaces:
-
Consumes:
store.ListAPITokens/RevokeAPIToken(internal/store/tokens.go:53,74, read/delete directly, matching howaccountPagealready reads keys and PGP keys);s.runControldispatchingtoken create(a write, so it goes through the command, matching every other write on this page). -
Produces:
func (s *Server) setTokenFlash(w http.ResponseWriter, msg string),func (s *Server) takeTokenFlash(w http.ResponseWriter, r *http.Request) string(internal/httpd/flash.go). -
Step 1: Write the failing tests
Add to internal/httpd/account_test.go:
// The settings page lists a user's API tokens with scope and expiry,
// and creating one shows the token exactly once, never in the URL
// (#264).
func TestAccountPageListsTokens(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)
}
if err := st.CreateAPIToken(uid, "laptop", "somehash", "read", nil); err != nil {
t.Fatal(err)
}
s := New(config.Default(), st)
rr := httptest.NewRecorder()
req := httptest.NewRequest("GET", "/settings", nil)
s.accountPage(rr, req, store.User{ID: uid, Username: "alice"})
body := rr.Body.String()
if !strings.Contains(body, "laptop") || !strings.Contains(body, "read") {
t.Fatalf("token row missing: %s", body)
}
if strings.Contains(body, "somehash") {
t.Fatal("the page printed a token hash")
}
}
// Creating a token always sends an explicit --scope, defaulting the
// form to read regardless of what token create itself defaults to, so
// this page's behaviour does not depend on that command's default
// (#264, #257).
func TestAccountSubmitTokenCreateDefaultsToReadScope(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"}
s := New(config.Default(), st)
rr := submitAccountForm(t, s, u, url.Values{"field": {"token-create"}, "name": {"laptop"}})
if rr.Code != http.StatusSeeOther {
t.Fatalf("status %d, body %s", rr.Code, rr.Body.String())
}
if !strings.Contains(strings.Join(rr.Result().Header.Values("Set-Cookie"), ";"), "gitbay_token=") {
t.Fatal("no token-shown cookie set")
}
tokens, err := st.ListAPITokens(uid)
if err != nil || len(tokens) != 1 {
t.Fatalf("tokens: %v %v", tokens, err)
}
if tokens[0].Scope != "read" {
t.Errorf("scope = %q, want read", tokens[0].Scope)
}
}
// Revoking a token requires the name typed back, the same guard every
// other removal on this page uses.
func TestAccountSubmitTokenRevokeRequiresConfirm(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)
}
if err := st.CreateAPIToken(uid, "laptop", "somehash", "read", nil); err != nil {
t.Fatal(err)
}
u := store.User{ID: uid, Username: "alice"}
s := New(config.Default(), st)
submitAccountForm(t, s, u, url.Values{"field": {"token-revoke"}, "name": {"laptop"}})
if tokens, _ := st.ListAPITokens(uid); len(tokens) != 1 {
t.Fatal("token revoked without confirmation")
}
rr := submitAccountForm(t, s, u, url.Values{"field": {"token-revoke"}, "name": {"laptop"}, "confirm": {"laptop"}})
if rr.Code != http.StatusSeeOther {
t.Fatalf("status %d, body %s", rr.Code, rr.Body.String())
}
if tokens, _ := st.ListAPITokens(uid); len(tokens) != 0 {
t.Fatal("token not revoked")
}
}
- Step 2: Run and see them fail
Run: go test ./internal/httpd -run 'TestAccountPageListsTokens|TestAccountSubmitTokenCreateDefaultsToReadScope|TestAccountSubmitTokenRevokeRequiresConfirm' -count=1
Expected: FAIL to compile (no token-create/token-revoke cases, no
token rows on the page).
- Step 3: Add the token-shown-once cookie
In internal/httpd/flash.go, alongside flashCookie/setFlash/takeFlash:
// tokenFlashCookie carries a freshly minted API token to the settings
// page exactly once. A cookie, not the ?m= query parameter the other
// account forms use for their success text, because a token is a
// secret and must never ride a URL a browser might history, bookmark,
// or hand to a proxy's access log (#264).
const tokenFlashCookie = "gitbay_token"
// setTokenFlash queues a freshly minted token's display text for the
// next render of the settings page.
func (s *Server) setTokenFlash(w http.ResponseWriter, msg string) {
if msg == "" {
return
}
http.SetCookie(w, &http.Cookie{
Name: tokenFlashCookie, Value: url.QueryEscape(msg), Path: "/settings",
HttpOnly: true, SameSite: http.SameSiteLaxMode,
Secure: s.cfg.HTTP.TLS != "off",
MaxAge: 60,
})
}
// takeTokenFlash returns the queued token text, if any, and clears it.
func (s *Server) takeTokenFlash(w http.ResponseWriter, r *http.Request) string {
c, err := r.Cookie(tokenFlashCookie)
if err != nil || c.Value == "" {
return ""
}
http.SetCookie(w, s.clearCookie(tokenFlashCookie, http.SameSiteLaxMode))
msg, err := url.QueryUnescape(c.Value)
if err != nil {
return ""
}
return msg
}
clearCookie takes only name and sameSite and hard-codes Path: "/" (internal/httpd/flash.go:101-108) — confirm this still clears a
cookie set with Path: "/settings" (it does: browsers key deletion on
name+domain+path, and "/settings" is under "/"... actually a
Path=/ clearing cookie does not delete a Path=/settings cookie —
paths must match exactly for deletion semantics in most browsers).
Fix this by setting Path: "/settings" on both the set and the clear:
add a path parameter to a small local variant, or simplest, write
takeTokenFlash's clear inline instead of reusing clearCookie:
func (s *Server) takeTokenFlash(w http.ResponseWriter, r *http.Request) string {
c, err := r.Cookie(tokenFlashCookie)
if err != nil || c.Value == "" {
return ""
}
http.SetCookie(w, &http.Cookie{
Name: tokenFlashCookie, Value: "", Path: "/settings",
HttpOnly: true, SameSite: http.SameSiteLaxMode,
Secure: s.cfg.HTTP.TLS != "off", MaxAge: -1,
})
msg, err := url.QueryUnescape(c.Value)
if err != nil {
return ""
}
return msg
}
(Use this version; drop the clearCookie call from the draft above.)
- Step 4: Add token data to
accountPage
In internal/httpd/account.go, add a view type near accountDevice:
// accountToken is one API token as the settings page shows it: never
// the token itself, only what identifies and describes it.
type accountToken struct {
Name string
Scope string
Created string
Expires string // "never" or a formatted timestamp
LastUsed string // "never" or a formatted timestamp
}
In accountPage, alongside the existing devices collection:
var tokens []accountToken
if list, err := s.st.ListAPITokens(u.ID); err == nil {
for _, tk := range list {
expires, lastUsed := "never", "never"
if tk.ExpiresAt != nil {
expires = tk.ExpiresAt.UTC().Format("2006-01-02 15:04 UTC")
}
if tk.LastUsedAt != nil {
lastUsed = tk.LastUsedAt.UTC().Format("2006-01-02 15:04 UTC")
}
tokens = append(tokens, accountToken{tk.Name, tk.Scope, tk.CreatedAt, expires, lastUsed})
}
}
Add Tokens []accountToken and TokenShown string to the struct passed
to s.render(w, "account.html", ...), with tokens and
s.takeTokenFlash(w, r) in the corresponding literal positions.
- Step 5: Add
token-createandtoken-revoketoaccountSubmit
In internal/httpd/account.go, accountSubmit's switch, add two
cases (alongside email-primary and before theme, or anywhere in the
switch — order does not matter):
case "token-create":
name := strings.TrimSpace(r.FormValue("name"))
if name == "" {
back("name the token", "")
return
}
scope := r.FormValue("scope")
if scope != "full" {
scope = "read" // this page's own default, regardless of what token create defaults to (#257, #264)
}
argv := []string{"token", "create", "--name", name, "--scope", scope}
if ttl := strings.TrimSpace(r.FormValue("ttl")); ttl != "" {
argv = append(argv, "--ttl", ttl)
}
out, msg, ok := s.runControl(u, argv)
if !ok {
back(msg, "")
return
}
s.setTokenFlash(w, out)
http.Redirect(w, r, "/settings#tokens", http.StatusSeeOther)
return
case "token-revoke":
name := r.FormValue("name")
if ok, msg := confirmed(r, name); !ok {
back(msg, "")
return
}
if _, msg, ok := s.runControl(u, []string{"token", "revoke", name}); !ok {
back(msg, "")
return
}
back("", "token revoked")
(This switch does not use back's redirect-with-query pattern for
token-create's success path, since the token's display text cannot go
through ?m=; it returns directly after the redirect, matching the
early-return shape every other case already uses.)
- Step 6: Add the Tokens section to
account.html
Add a new <section id="tokens"> — placed before the existing <section id="cli"> (which the sidebar's existing anchor list under "On the
command line" leaves in place; add a <li><a href="#tokens">API tokens</a></li> to that anchor list too, alongside the other section
links):
<section id="tokens"><h2>API tokens</h2>
<p class="meta">A token signs in the iOS app, or a script, without your
password. A phone app needs full scope to comment and merge; full scope
on an admin account can administer the instance, so give a token the
narrowest scope and shortest lifetime the job needs.</p>
{{if .TokenShown}}<pre class="message" tabindex="0">{{.TokenShown}}</pre>{{end}}
<table>
<tr><th>Name</th><th>Scope</th><th>Created</th><th>Expires</th><th>Last used</th><th></th></tr>
{{range .Tokens}}<tr>
<td>{{.Name}}</td><td>{{.Scope}}</td><td>{{.Created}}</td><td>{{.Expires}}</td><td>{{.LastUsed}}</td>
<td class="act"><form method="post" action="/settings"><input type="hidden" name="field" value="token-revoke"><input type="hidden" name="name" value="{{.Name}}">{{template "confirmfield" .Name}} <button type="submit" class="danger">Revoke</button></form></td>
</tr>{{else}}<tr><td colspan="6">no tokens</td></tr>{{end}}
</table>
<form method="post" action="/settings" class="actions">
<input type="hidden" name="field" value="token-create">
<input type="text" name="name" aria-label="Token name" placeholder="name, e.g. iphone" required>
<select name="scope" aria-label="Scope">
<option value="read" selected>read</option>
<option value="full">full</option>
</select>
<input type="text" name="ttl" aria-label="Expires after" placeholder="expires after, e.g. 30d (optional)">
<button type="submit" class="btn">Create token</button>
</form>
</section>
- Step 7: Run
Run: go test ./internal/httpd -run 'TestAccountPageListsTokens|TestAccountSubmitTokenCreateDefaultsToReadScope|TestAccountSubmitTokenRevokeRequiresConfirm' -count=1 && go test ./internal/httpd -count=1
Expected: PASS.
- Step 8: Commit
git add internal/httpd/flash.go internal/httpd/account.go internal/web/templates/account.html internal/httpd/account_test.go
git commit -m "web: Settings → Tokens — create, list, revoke API tokens" -m "Ref #264"
Task 7.2: registered.html next steps as a numbered list
Files:
-
Modify:
internal/web/templates/registered.html -
Test: a render test in whatever file covers signup (
grep -rln '"registered.html"' internal/httpd/*_test.go; create one if none exists) -
Step 1: Write the failing test
func TestRegisteredPageNumberedStepsAndTokenMention(t *testing.T) {
var sb strings.Builder
if err := web.Render(&sb, "registered.html", struct {
basePage
Username, Message, Host string
}{Username: "alice", Host: "gitbay.org"}); err != nil {
t.Fatalf("render: %v", err)
}
out := sb.String()
if !strings.Contains(out, "<ol>") {
t.Error("next steps are not a numbered list")
}
if !strings.Contains(out, "Settings → Tokens") {
t.Error("no mention of Settings → Tokens for the iOS app")
}
}
- Step 2: Run and see it fail
Run: go test ./internal/httpd -run TestRegisteredPageNumberedStepsAndTokenMention -count=1
Expected: FAIL.
- Step 3: Rewrite the section
Find (registered.html:7-10):
<h2>On the web</h2>
<p>Check your mail for the code, <a href="/login">sign in</a> with an emailed
link, and paste the code under <a href="/settings">Settings</a>. Then +
creates your first repository.</p>
Replace:
<h2>On the web</h2>
<ol>
<li>Copy the verification code from the mail you were just sent.</li>
<li><a href="/login">Sign in</a> with an emailed link.</li>
<li>Paste the code in <a href="/settings#emails">Settings → Email</a>.</li>
</ol>
<p>Then + creates your first repository. Using the iOS app? Create a
token in <a href="/settings#tokens">Settings → Tokens</a>.</p>
(#emails matches the existing section id in account.html; confirm
with grep -n 'id="email' internal/web/templates/account.html — it may
be id="emails" plural or singular, match whichever is actually there.)
- Step 4: Run
Run: go test ./internal/httpd -run TestRegisteredPageNumberedStepsAndTokenMention -count=1 && go test ./internal/httpd -count=1
Expected: PASS.
- Step 5: Commit
git add internal/web/templates/registered.html internal/httpd/*_test.go
git commit -m "web: registered page's next steps as a numbered list, with a token mention for the iOS app" -m "Ref #264"
Task 7.3: update Parity
Files:
-
Modify:
.gitbay/wiki/Parity.org -
Step 1: Fix the API token mint row
Find:
| API token mint | yes | no | no |
Replace:
| API token mint | yes | yes | no |
(The iOS side — linking to this page from sign-in — is filed separately
in krz/gitbay-ios, per the issue text, so its column stays no here.)
- Step 2: Commit
git add .gitbay/wiki/Parity.org
git commit -m "wiki: Parity reflects the web API-token page" -m "Closes #264"
Task 7.4: open MR 7
git push -u origin web-api-tokens
gitbay mr create --source web-api-tokens --target main --title "Web: Settings → Tokens page"
Wait for CI, merge, delete the branch both places.
Self-review
Spec coverage (against the seven issue texts):
- #261: FK check ordering (Task 1.1), pin/watch dispatch (Task 1.2), Cache-Control (Task 1.3), all four doc-drift bullets (Task 1.4). ✓.
- #263: whoami line, token line both forms, auth summary, registry test (Tasks 2.1-2.3). ✓.
- #264: create/list/revoke with confirmfield, scope defaults to read on this page regardless of the command default, registered.html numbered steps + token mention, Parity (Tasks 7.1-7.3). ✓.
- #269: range-diff page, compare-to-previous per row, Parity (Tasks 3.1-3.3). ✓.
- #270: the empty-state table (Task 4.1), MR list contribution hint (Task 4.2), search caption + tab zero-count rule (Task 4.3). ✓.
- #271: issue form fields, Muted reachable, rail/More unification, Discussion heading, Live note (Tasks 5.1-5.5). ✓.
- #283:
_rawfix and test (Task 6.1). ✓.
Placeholder scan: no task stops short of real code. The three points
this plan's first draft could not pin down from the code — group()'s
help-rendering mechanism (Task 2.2), issue create's flag set (Task
5.1), and how an internal/httpd test authenticates a GET as a given
viewer (Task 4.2) — were each resolved by reading the relevant source
(cmd/gitbay/main.go's group()/serverHelp(), internal/control/help.go's
runHelp(), internal/control/issue.go's issue create/issue milestone/issue assign registrations, and internal/httpd/logincookie_test.go's
sessionCookieFor plus store.CreateWebSession) before this plan was
finished; the tasks above carry the resolved code directly, not a
placeholder.
Type consistency: railItem (Task 5.3) is used identically in
web.go and layout.html; accountToken (Task 7.1) fields match the
template's .Name/.Scope/.Created/.Expires/.LastUsed access;
mrRangeDiff's render struct (Task 3.1) matches mrrangediff.html's
.MR/.Diff/.Error.
Open questions
None outstanding — the three research gaps found while first drafting
this plan (Task 2.2's help mechanism, Task 4.2's session-cookie test
fixture, Task 5.1's issue create flag set) were each resolved by
reading the relevant source before this plan was finished; see
"Placeholder scan" above for what was read.