# CLI UX small fixes 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 #265, #267 and #268: the dashboard's activity feed reads as sentences instead of raw event payloads and stops repeating assigned issues; CLI help and usage print the form the caller actually typed (`gitbay ...` or `ssh git@host ...`), with the `auth` grouping, the `--help` flag and command summaries fixed to match; and five smaller UX findings (the unregistered-key message, `issue create` flags, `mr show` plurals, a `repo readme` command, and a truncated mirror timestamp). **Architecture:** No schema changes and no new migrations. The dashboard and web feed currently keep two copies of "turn a stored event into a sentence" (`internal/httpd/feed.go`) and "the worst of a set of build statuses" (`internal/httpd/builds.go`); both move into `internal/control` so the CLI can call them directly (same package) and `internal/httpd` calls the exported forms. Help and usage already carry a `Ctx.Term` set from the CLI's `--term=[,color]`; `c.program()` already picks `"gitbay"` or `"ssh git@"` from it for the `--help` path, but `c.usage()`/`c.usageWith()` (the wrong-argument path) do not yet call it. Separately, eighteen CLI commands resolve to a server path that differs from what cobra's tree spells (`cmd/gitbay/main.go`'s `serverPath` annotations): everything under `auth` except a few whose noun already matches the registry (`auth email ...` -> `email ...`, `auth export` -> `account export`, `auth keys ...` -> `keys ...`, `auth pgp ...` -> `pgp ...`, `auth token ...` -> `token ...`, `auth whoami` -> `whoami`), and `repo topics list` -> `repo topics`. Printing the registered path verbatim for one of these gives a command that does not exist — `gitbay keys remove ` is `unknown command "keys"`. #267 decided the CLI sends its own invoking path and the server prints that instead of the registered one wherever they differ; the CLI's `auth` grouping, still not a registry path at all, gets the same treatment for the several registry prefixes it gathers. **Tech stack:** Go, `golang.org/x/crypto/ssh`, cobra. **Spec:** none — these are small, independently-scoped fixes; this plan is its own spec. ## Global constraints - Three MRs, each on its own branch off `main`: `cli-ux-activity` (#265), `cli-ux-help` (#267), `cli-ux-fixes` (#268). No MR depends on either of the others; land in any order. - Commits are signed (the repository refuses unsigned ones); messages reference the issue they touch (`Ref #N`), and the last commit that finishes an issue says `Closes #N`. No attribution to any assistant, model or AI anywhere: commits, MR bodies, comments. - MR: `gitbay mr create --source --target main --title "..."`; merge with `gitbay mr merge --strategy ff` once CI is green (this repository requires signed commits, so `squash`/`merge` are refused), then delete the branch locally and on the remote. If the merge reports the branch is behind, rebase onto `main`, force-push, merge again. - Locally: `go build ./...`, `go vet ./...`, and the unit tests of every touched package. Run at most the one e2e test being written per task (`go test ./e2e -run TestName -count=1`); CI on bay1 runs the full suite. - No new migrations; none of #265/#267/#268 touch the schema. The plan numbers 0078–0079 pre-assigned to "plan 6" go unused. - Registries that fail CI when a new thing lacks its row: a `ReadOnly` command needs an entry in `readArgs` in `e2e/readonly_test.go`; a new control command needs a `pass()` entry in `cmd/gitbay/main.go` (`cmd/gitbay/summaries_test.go`'s coverage and `summaries_gen.go` currency checks); a command reading stdin needs `ReadsStdin: true`. - `--json` output: field shapes are unchanged throughout this plan. Where a task changes plain-text wording it says so; JSON error strings for usage refusals do change in Part 2 (Task 2.1), which is called out there specifically since no other task touches JSON text. - The `.gitbay/wiki/Parity.org` page is updated in the same commit that changes the row it describes (Task 3.4). - Writing style: plain, direct, no hype; code comments match the surrounding density; no before/after narration in comments or docs. ## Order and dependencies 1. **`cli-ux-activity`** — closes #265. Independent. 2. **`cli-ux-help`** — closes #267. Independent. 3. **`cli-ux-fixes`** — closes #268. Independent. None of these three depend on any of the other five plans running in parallel (credentials-and-sessions, ci-trust-and-build-reporting, server-hardening, data-at-rest-and-backup, web-ux); nothing here touches authentication, secrets, CI, backups or the pages those plans change. --- # Part 1: dashboard activity, no duplicates, one empty-state wording (branch `cli-ux-activity`, closes #265) ### Task 1.1: move the feed-line sentence renderer into `internal/control` The web renders "recent activity" as a sentence (`cmc opened issue #12`) via `internal/httpd/feed.go`'s unexported `feedLine`/`feedLines`, which the CLI cannot reach — `internal/httpd` imports `internal/control`, not the other way around. Move the renderer into `internal/control` so both sides call the same code; `internal/httpd` becomes a thin caller of the exported form. **Files:** - Create: `internal/control/feedline.go` (from `internal/httpd/feed.go`) - Create: `internal/control/feedline_test.go` (from `internal/httpd/feed_test.go`) - Modify: `internal/httpd/builds.go:150-183` (`worstStatus`, `runStatusPriority` move out; `combinedStatus` calls the moved form) - Modify: `internal/httpd/web.go:219`, `:470`, `:493`, `:511` (`feedLine`/`feedLines` → `control.FeedLine`/`control.FeedLines`) - Modify: `internal/httpd/ownerpage_test.go:58` (`feedLine{...}` → `control.FeedLine{...}`) - Delete: `internal/httpd/feed.go`, `internal/httpd/feed_test.go` **Interfaces:** - Produces: `type FeedLine struct{ Actor, Verb, Ref, Repo, URL string; When string; WhenT time.Time; State string; Jobs []string; sha string }` (exported type, one unexported field kept for the fold logic — same package as its only user); `func FeedLines(events []store.FeedEvent) []FeedLine`; `func WorstStatus(statuses []string) string`. - Consumes (Task 1.2, 1.3): the same `FeedLines`/`FeedLine`. - [ ] **Step 1: Run the existing web feed tests to see the baseline pass** Run: `go test ./internal/httpd -run TestFeedLines -count=1` Expected: PASS (nothing changed yet). - [ ] **Step 2: Move the renderer** `git mv internal/httpd/feed.go internal/control/feedline.go` and `git mv internal/httpd/feed_test.go internal/control/feedline_test.go`. In `internal/control/feedline.go`, change `package httpd` to `package control`, capitalize the moved identifiers, and drop the now- unused `"gitbay.org/gitbay/internal/store"` import path prefix adjustments are unnecessary (the import path is the same from either package). Concretely: ```go package control import ( "encoding/json" "fmt" "slices" "strings" "time" "gitbay.org/gitbay/internal/store" ) // FeedLine is one activity entry, already phrased and linked. type FeedLine struct { Actor string Verb string // "opened issue", "merged", "ran 2 jobs on" Ref string // "#12", "!35", "v0.4.0", a short sha Repo string URL string When string // the stored timestamp, for anything still reading it raw WhenT time.Time // parsed from When, for ago/whenT rendering State string // a build run's combined status; empty for anything else Jobs []string // job names folded into a build run sha string // the commit a build event fired on, for fold-matching } ``` Keep the rest of the function bodies (`FeedLines`, `issueVerb`, `mrVerb`, `parseEventTime`) unchanged apart from `feedLines` → `FeedLines` and `feedLine{` → `FeedLine{`; `issueVerb`/`mrVerb`/`parseEventTime` stay unexported (nothing outside the package calls them directly). In `internal/control/feedline_test.go`, change `package httpd` to `package control` and `feedLines(` → `FeedLines(` throughout (ten call sites, all named `feedLines(events)`). - [ ] **Step 3: Move `worstStatus`** In `internal/httpd/builds.go`, cut `runStatusPriority` and `worstStatus` (the two declarations at lines 150–183) and paste them into `internal/control/feedline.go`, renaming `worstStatus` to `WorstStatus` and updating its one internal call site in `FeedLines` (`out[i].State = worstStatus(statuses[i])` → `WorstStatus(...)`). In `internal/httpd/builds.go`, `combinedStatus` becomes: ```go func combinedStatus(builds []control.BuildOut) string { statuses := make([]string, len(builds)) for i, b := range builds { statuses[i] = b.Status } return control.WorstStatus(statuses) } ``` - [ ] **Step 4: Update `internal/httpd/web.go`'s call sites** Line 219 (`dashboard`'s anonymous struct): `Feed []control.FeedLine`. Line 470: `func (s *Server) ownerFeed(tab, kind, name string) []control.FeedLine`, its final `return feedLines(events)` becomes `return control.FeedLines(events)`. Line 493 inside `dashboard`: `feedLines(events)` → `control.FeedLines(events)`. Line 511 (`ownerPage.Log`): `Log []control.FeedLine`. - [ ] **Step 5: Update `internal/httpd/ownerpage_test.go:58`** ```go d.Log = []control.FeedLine{{Actor: "cmc", Verb: "opened issue", Ref: "#12", Repo: "krz/gitbay", URL: "/krz/gitbay/issues/12"}} ``` - [ ] **Step 6: Build and test both packages** Run: `go build ./... && go vet ./... && go test ./internal/control ./internal/httpd -count=1` Expected: PASS. A compile error naming `feedLine`/`feedLines`/`worstStatus` means a call site in `internal/httpd` was missed — `grep -rn "feedLine\|worstStatus" internal/httpd/*.go` should come back empty except inside comments. - [ ] **Step 7: Commit** ```bash git add internal/control/feedline.go internal/control/feedline_test.go internal/httpd/feed.go internal/httpd/feed_test.go internal/httpd/builds.go internal/httpd/web.go internal/httpd/ownerpage_test.go git commit -m "control: move the feed-line sentence renderer from httpd, so the CLI can share it" -m "Ref #265" ``` ### Task 1.2: a labelled event's sentence names the labels `issue.labeled`/`mr.labeled` events currently fall through `issueVerb`/ `mrVerb`'s default case (`"issue " + s"`, i.e. "issue labeled"), naming neither what changed nor which labels — on the web today, not only in the CLI this plan is fixing. Give both label events their own verb and carry the label list alongside the ref. **Files:** - Modify: `internal/control/feedline.go` (`FeedLine`, `FeedLines`, `issueVerb`, `mrVerb`) - Modify: `internal/control/feedline_test.go` - Modify: `internal/web/templates/dashboard.html:49`, `internal/web/templates/owner.html:39` **Interfaces:** - Produces: `FeedLine.Extra string` — trailing detail rendered after the ref; empty for every event kind but a labelled one. - [ ] **Step 1: Write the failing test** ```go func TestFeedLinesNamesTheLabelsOnALabelledIssue(t *testing.T) { events := []store.FeedEvent{ {RepoPath: "krz/gitbay", Actor: "cmc", Kind: "issue.labeled", Data: `{"number":262,"labels":["ops","security"]}`}, } lines := FeedLines(events) if len(lines) != 1 { t.Fatalf("FeedLines returned %d lines, want 1", len(lines)) } l := lines[0] if l.Verb != "labelled" || l.Ref != "#262" || l.Extra != "ops, security" { t.Errorf("got %+v", l) } } func TestFeedLinesNamesTheLabelsOnALabelledMR(t *testing.T) { events := []store.FeedEvent{ {RepoPath: "krz/gitbay", Actor: "cmc", Kind: "mr.labeled", Data: `{"number":471,"labels":["review"]}`}, } lines := FeedLines(events) if len(lines) != 1 || lines[0].Verb != "labelled" || lines[0].Ref != "!471" || lines[0].Extra != "review" { t.Errorf("got %+v", lines) } } ``` - [ ] **Step 2: Run and see them fail** Run: `go test ./internal/control -run TestFeedLinesNamesTheLabels -count=1` Expected: FAIL (`Verb = "issue labeled"`/`"merge request labeled"`, `Extra` unset). - [ ] **Step 3: Implement** Add a field to the local decode struct and the `FeedLine` type, then set `Extra` for the two label kinds. In `FeedLines`, the local `d` struct gains `Labels []string`: ```go var d struct { Number int64 `json:"number"` Job string `json:"job"` Tag string `json:"tag"` SHA string `json:"sha"` Labels []string `json:"labels"` } ``` `FeedLine` gains, after `Jobs`: ```go // Extra is trailing detail shown after the ref: the label list on a // labelled event, empty for everything else. Extra string ``` In the `case "issue":`/`case "mr":` arms, after setting `l.Verb, l.Ref`: ```go case "issue": l.Verb, l.Ref = issueVerb(rest), fmt.Sprintf("#%d", d.Number) l.URL = fmt.Sprintf("/%s/issues/%d", e.RepoPath, d.Number) if rest == "labeled" { l.Extra = strings.Join(d.Labels, ", ") } case "mr": l.Verb, l.Ref = mrVerb(rest), fmt.Sprintf("!%d", d.Number) l.URL = fmt.Sprintf("/%s/mrs/%d", e.RepoPath, d.Number) if rest == "labeled" { l.Extra = strings.Join(d.Labels, ", ") } ``` `issueVerb` and `mrVerb` each gain a case: ```go case "labeled": return "labelled" ``` - [ ] **Step 4: Run** Run: `go test ./internal/control -count=1` Expected: PASS. - [ ] **Step 5: Templates carry `Extra`** `internal/web/templates/dashboard.html:49` and `internal/web/templates/owner.html:39` both gain `{{if .Extra}} {{.Extra}}{{end}}` right after the closing `` of the ref link, before the `
`: ``` {{range .Feed}}

{{.Actor}} {{.Verb}} {{.Ref}}{{if .Extra}} {{.Extra}}{{end}}
{{.Repo}} · {{ago .WhenT}}

``` - [ ] **Step 6: Build** Run: `go build ./... && go test ./internal/control ./internal/httpd -count=1` Expected: PASS. - [ ] **Step 7: Commit** ```bash git add internal/control/feedline.go internal/control/feedline_test.go internal/web/templates/dashboard.html internal/web/templates/owner.html git commit -m "feed: a labelled event names the labels" -m "Ref #265" ``` ### Task 1.3: dashboard and feed render activity as sentences, not raw payloads `gitbay dashboard`'s "recent activity" table and `gitbay feed` both print the event's kind and its raw JSON payload (`2026-09-28T02:36:51Z cmc issue.labeled krz/gitbay {"number":262,"labels":["ops","security"]}`). Render the same sentence the web shows instead; the payload stays available under `--json` (`DashboardOut.Activity`/`FeedOut.Data` are untouched). **Files:** - Modify: `internal/control/dashboard.go` (`runDashboard`'s `activityRows`, `runFeed`'s plain formatter) - Test: `internal/control/dashboard_test.go` **Interfaces:** - Consumes: `FeedLines`, `FeedLine` (Task 1.1/1.2). - [ ] **Step 1: Write the failing test** ```go func TestDashboardActivityIsASentence(t *testing.T) { c := notifTestCtx(t, "cmc") repoID, err := c.Store.CreateRepo("user", c.User.ID, "gitbay", "public") if err != nil { t.Fatal(err) } repo, err := c.Store.RepoByID(repoID) if err != nil { t.Fatal(err) } c.Store.RecordEvent(repo.ID, c.User.ID, "issue.labeled", `{"number":262,"labels":["ops","security"]}`) var out bytes.Buffer c.Stdout, c.Stderr = &out, &out if code := runDashboard(c, nil); code != 0 { t.Fatalf("exit %d: %s", code, out.String()) } if strings.Contains(out.String(), `{"number"`) { t.Errorf("raw payload leaked into plain output:\n%s", out.String()) } if !strings.Contains(out.String(), "cmc labelled krz/gitbay#262 ops, security") { t.Errorf("no sentence in output:\n%s", out.String()) } } ``` - [ ] **Step 2: Run and see it fail** Run: `go test ./internal/control -run TestDashboardActivityIsASentence -count=1` Expected: FAIL (output has `KIND`/`DATA` columns and the raw JSON). - [ ] **Step 3: Implement in `runDashboard`** Replace the `activityRows`/`section("recent activity:", ...)` block: ```go lines := FeedLines(events) activityRows := make([][]cell, len(lines)) for i, l := range lines { sentence := fmt.Sprintf("%s %s %s%s", l.Actor, l.Verb, l.Repo, l.Ref) if l.Extra != "" { sentence += " " + l.Extra } activityRows[i] = []cell{cAge(l.When), cFlex(sentence)} } section("recent activity:", []string{"WHEN", "EVENT"}, activityRows) ``` `events` is already in scope (it is what `d.Activity = feedOutputs(events)` was built from, a few lines above); nothing else in `runDashboard` reads it again, so no variable needs renaming. - [ ] **Step 4: Run** Run: `go test ./internal/control -run TestDashboardActivityIsASentence -count=1` Expected: PASS. - [ ] **Step 5: Same fix in `runFeed`, its own failing test first** ```go func TestFeedIsASentence(t *testing.T) { c := notifTestCtx(t, "cmc") repoID, err := c.Store.CreateRepo("user", c.User.ID, "gitbay", "public") if err != nil { t.Fatal(err) } repo, err := c.Store.RepoByID(repoID) if err != nil { t.Fatal(err) } c.Store.RecordEvent(repo.ID, c.User.ID, "issue.created", `{"number":1}`) var out bytes.Buffer c.Stdout, c.Stderr = &out, &out if code := runFeed(c, nil); code != 0 { t.Fatalf("exit %d: %s", code, out.String()) } if !strings.Contains(out.String(), "cmc opened issue krz/gitbay#1") { t.Errorf("no sentence in output:\n%s", out.String()) } } ``` Run: `go test ./internal/control -run TestFeedIsASentence -count=1` Expected: FAIL. Implement: `runFeed`'s plain closure changes from the five-column `WHEN`/`ACTOR`/`KIND`/`REPO`/`DATA` table to the same two-column shape, built from `FeedLines(events)` (the same `events` slice `runFeed` already queried, before `feedOutputs(events)` is called for `ds`): ```go lines := FeedLines(events) return c.emitPage(p, ds, next, func(w io.Writer) { tb := c.table(w, "WHEN", "EVENT") for _, l := range lines { sentence := fmt.Sprintf("%s %s %s%s", l.Actor, l.Verb, l.Repo, l.Ref) if l.Extra != "" { sentence += " " + l.Extra } tb.row(cAge(l.When), cFlex(sentence)) } tb.flush() }) ``` `ds` (the `FeedOut` slice) stays exactly as it was: it is what `--json` still emits, and `emitPage`'s cursor logic pages `ds`, not `lines` — the two slices are always the same length and order since both come from the same `events`. - [ ] **Step 6: Run** Run: `go test ./internal/control -count=1` Expected: PASS. A failure elsewhere in the package on a "recent activity"/"WHEN\tACTOR\tKIND" assertion means an existing test asserted the old five-column shape; update its expectation to the new sentence (the test name will say `TestDashboard...` or `TestFeed...`). - [ ] **Step 7: Commit** ```bash git add internal/control/dashboard.go internal/control/dashboard_test.go git commit -m "dashboard, feed: render activity as the web's sentence, not the raw payload" -m "Ref #265" ``` ### Task 1.4: an issue assigned to you no longer repeats in "open issues" `dashboardIssuesQuery` (open issues you are involved in) and `assignedIssuesQuery` (open issues assigned to you) overlap whenever an assigned issue also sits in a repository you can otherwise reach — the common case — so the same issue prints under both "assigned to you:" and "open issues:" on the CLI, and under both lists on the web dashboard, which calls the same two store methods (`internal/httpd/web.go:206-209`). Exclude assigned issues from the "open issues" query; the fix is in the store, so both surfaces get it at once. **Files:** - Modify: `internal/store/dashboard.go` (`dashboardIssuesQuery`) - Test: `internal/store/dashboard_test.go` (create) **Interfaces:** - Consumes: nothing new. - Produces: nothing new (`DashboardIssues` keeps its signature). - [ ] **Step 1: Write the failing test** ```go package store import "testing" // An issue assigned to the user is not repeated under DashboardIssues: // AssignedIssues already covers it, and a repository the user can // otherwise reach (here, one they own) is the common case where the two // queries used to overlap (#265). func TestDashboardIssuesExcludesAssignedIssues(t *testing.T) { s := open(t) if err := s.MigrateUp(); err != nil { t.Fatal(err) } uid, err := s.CreateUser("cmc", false) if err != nil { t.Fatal(err) } repoID, err := s.CreateRepo("user", uid, "gitbay", "public") if err != nil { t.Fatal(err) } repo, err := s.RepoByID(repoID) if err != nil { t.Fatal(err) } assignedNum, err := s.CreateIssue(repo.ID, uid, "assigned to me", "", "markdown") if err != nil { t.Fatal(err) } if _, err := s.CreateIssue(repo.ID, uid, "not assigned", "", "markdown"); err != nil { t.Fatal(err) } assigned, err := s.IssueByNumber(repo.ID, assignedNum) if err != nil { t.Fatal(err) } if err := s.SetIssueAssignee(assigned.ID, uid, true); err != nil { t.Fatal(err) } issues, err := s.DashboardIssues(uid) if err != nil { t.Fatal(err) } if len(issues) != 1 || issues[0].Title != "not assigned" { t.Fatalf("DashboardIssues = %+v, want only the unassigned issue", issues) } assignedList, err := s.AssignedIssues(uid) if err != nil { t.Fatal(err) } if len(assignedList) != 1 || assignedList[0].Title != "assigned to me" { t.Fatalf("AssignedIssues = %+v, want the assigned issue", assignedList) } } ``` - [ ] **Step 2: Run and see it fail** Run: `go test ./internal/store -run TestDashboardIssuesExcludesAssignedIssues -count=1` Expected: FAIL (`DashboardIssues` returns both issues). - [ ] **Step 3: Implement** `dashboardIssuesQuery` in `internal/store/dashboard.go` gains one `NOT EXISTS` clause: ```go const dashboardIssuesQuery = ` SELECT COALESCE(u.username, o.name) || '/' || r.name, x.number, x.title, au.username, x.state, x.updated_at FROM issues x JOIN repos r ON r.id = x.repo_id LEFT JOIN users u ON r.owner_kind = 'user' AND u.id = r.owner_id LEFT JOIN orgs o ON r.owner_kind = 'org' AND o.id = r.owner_id JOIN users au ON au.id = x.author_id WHERE x.state = 'open' AND ` + involvedCond + ` AND NOT EXISTS (SELECT 1 FROM issue_assignees ia WHERE ia.issue_id = x.id AND ia.user_id = ?1) ORDER BY x.updated_at DESC LIMIT 50` ``` - [ ] **Step 4: Run the new test, then the package and the query-plan guard** Run: `go test ./internal/store -count=1` Expected: PASS, `TestDashboardQueriesUseIndexes`'s `DashboardIssues` case included — a correlated `NOT EXISTS` does not change which index drives the `ORDER BY`, so the plan should still show `issues_recent` with no `USE TEMP B-TREE FOR ORDER BY`. If it does regress, the `NOT EXISTS` subquery needs `issue_assignees`'s existing `(issue_id, user_id)` index (check `migrations/` for its name) rather than a new one — this task does not add a migration. - [ ] **Step 5: Run the CLI package too** Run: `go test ./internal/control -count=1` Expected: PASS. `TestDashboardEmptySectionsSayNone` and any other dashboard test that seeded an assigned issue and expected it under "open issues" needs its expectation updated to match the new, non-overlapping behavior. - [ ] **Step 6: Commit** ```bash git add internal/store/dashboard.go internal/store/dashboard_test.go git commit -m "dashboard: an assigned issue no longer repeats under open issues" -m "Ref #265" ``` ### Task 1.5: `notifications list`'s empty state names `--all` An inbox with only read notifications prints the generic `nothing to list` on stderr when `notifications list` is run without `--all`, without saying unread items are what it shows by default. **Files:** - Modify: `internal/control/notifications.go` (`runNotificationsList`) - Test: `internal/control/notifications_test.go` - [ ] **Step 1: Write the failing test** ```go func TestNotificationsListEmptyUnreadSaysHowToSeeRead(t *testing.T) { c, repo, bob := testRepoWithWatcher(t) // Give bob one notice, then mark it read, so his inbox has rows but // no unread ones. c.User = store.User{ID: bob, Username: "bob"} notify(c, []int64{bob}, notice{repo: repo, kind: "issue", subject: "s", action: "a", path: "x"}) if code := runNotificationsRead(c, []string{"--all"}); code != protocol.ExitOK { t.Fatalf("mark read: exit %d", code) } var out, errOut bytes.Buffer c.Stdout, c.Stderr = &out, &errOut if code := runNotificationsList(c, nil); code != protocol.ExitOK { t.Fatalf("exit %d: %s", code, errOut.String()) } if got := errOut.String(); got != "no unread notifications (--all for read ones)\n" { t.Errorf("stderr = %q", got) } // --all sees it and stays the generic message when that too is empty. out.Reset() errOut.Reset() if code := runNotificationsList(c, []string{"--all"}); code != protocol.ExitOK { t.Fatalf("exit %d: %s", code, errOut.String()) } if !strings.Contains(out.String(), "s") { t.Errorf("--all did not show the read notice: %q", out.String()) } } ``` (This test needs `notify` and `notice` — the same helpers `testRepoWithWatcher`'s package already exercises in `notifications_test.go`'s other tests; if their exact names differ, `grep -n "^func notify\b\|^type notice\b" internal/control/*.go` and use what is actually there.) - [ ] **Step 2: Run and see it fail** Run: `go test ./internal/control -run TestNotificationsListEmptyUnreadSaysHowToSeeRead -count=1` Expected: FAIL (stderr is `nothing to list`). - [ ] **Step 3: Implement** In `runNotificationsList`, after `ds` is built and before the `return c.emitPage(...)`: ```go if !c.JSON && !p.active && len(ds) == 0 { msg := "nothing to list" if !all { msg = "no unread notifications (--all for read ones)" } fmt.Fprintln(c.Stderr, msg) return protocol.ExitOK } return c.emitPage(p, ds, next, func(w io.Writer) { ``` This runs before pagination wraps the result (`p.active`, from `--limit`/`--cursor`) and before JSON, both of which already have their own well-defined empty shape (`{"items":[],...}` or a bare `[]`) that this task leaves alone. - [ ] **Step 4: Run** Run: `go test ./internal/control -run TestNotifications -count=1` Expected: PASS. - [ ] **Step 5: Run the package, commit, open the MR** Run: `go test ./internal/control ./internal/store ./internal/httpd -count=1` Expected: PASS. ```bash git add internal/control/notifications.go internal/control/notifications_test.go git commit -m "notifications list: name --all when the empty inbox is just read items" -m "Closes #265" git push -u origin cli-ux-activity gitbay mr create --source cli-ux-activity --target main --title "dashboard activity as sentences, no duplicate issues" ``` Wait for CI, merge with `--strategy ff`, delete the branch both places. --- # Part 2: help and usage print the form the caller typed (branch `cli-ux-help`, closes #267) ### Task 2.1: the CLI sends the path it typed; usage and help print it `c.usage()` prints the bare *registered* usage (`usage: keys remove `), with neither the `gitbay` nor the `ssh git@host` prefix `c.program()`/`helpVerb` already use for `--help` — and for eighteen commands (the ones listed in Architecture above) the registered path is not even something a caller can type: `gitbay keys remove ` is `unknown command "keys"`, because the real command is `gitbay auth keys remove `. Fix both: give every usage/help line the program prefix, mark a leading `` optional at a terminal (the CLI fills it in from the clone's origin remote, `cmd/gitbay/ssh.go`'s `withRepo`; stock ssh never does), and have the CLI tell the server what it was actually typed as, so the server can print that instead of the registered path wherever the two differ. The CLI already tells the server one thing about the calling session this way: `--term=[,color]`, prepended to the command line by `runSSHPaged` and stripped off `argv[0]` by `Dispatch` before `Lookup` (`internal/control/control.go`). A second, sibling prefix, `--path=`, carries what cobra resolved the call to (`cobra.Command.CommandPath()`, minus the leading `gitbay `). It travels as its own `--path=` argument rather than a new field packed into the `--term=` value: that value's `cols[,color]` grammar has no room for a string containing spaces (a CLI path always does), and a second prefix is one more `strings.CutPrefix` in the same loop, not a new mini-parser. Only the gitbay CLI ever sends it — stock ssh has no notion of a "path it resolved to" that differs from what was typed, because what was typed *is* the dispatch path — so `Dispatch` never invents one, and the field stays empty for the web and the API exactly like `Term` does. **Files:** - Modify: `internal/control/control.go` (`Ctx.CLIPath`, `Dispatch`, `usage`, `usageWith`) - Modify: `internal/control/help.go` (`cliUsage`, `shownAs`, `cmdUsage`, `helpVerb`, `helpNoun`) - Modify: `internal/control/control_test.go` (`TestArgumentRefusalsNameTheUsage`, new `TestPathArgument`) - Test: `internal/control/help_test.go` - Modify: `cmd/gitbay/ssh.go` (`cliPathOf`, `withCLIPath`, new) - Modify: `cmd/gitbay/main.go` (`pass`, `runServerHelp`, `runPass`, `group`, `serverHelp`) - Test: `cmd/gitbay/term_test.go` (the two new pure helpers) - Test: `cmd/gitbay/serverpath_test.go` (create) - Test: `e2e/cliusage_test.go` (create) **Interfaces:** - Produces: `Ctx.CLIPath string`; `func cliUsage(usage string) string`; `func (c *Ctx) shownAs(registered, full string) string`; `func (c *Ctx) cmdUsage() string`; `func cliPathOf(cmd *cobra.Command) string`; `func withCLIPath(cliPath string, argv []string) []string`. - Consumes (Task 2.2, 2.3): `Ctx.CLIPath`, `shownAs`, `withCLIPath`, `cliPathOf`. - [ ] **Step 1: Write the failing test for the transport** ```go // TestPathArgument: --path= is read only as a leading argument (in // either order with --term=), and never over HTTP — mirrors // TestTermArgument, the mechanism it rides alongside. func TestPathArgument(t *testing.T) { cases := []struct { name string viaAPI bool argv []string want string wantArgv []string }{ {"leading", false, []string{"--path=auth keys remove", "keys", "remove", "abc"}, "auth keys remove", []string{"abc"}}, {"after term", false, []string{"--term=80", "--path=auth keys remove", "keys", "remove", "abc"}, "auth keys remove", []string{"abc"}}, {"before term", false, []string{"--path=auth keys remove", "--term=80", "keys", "remove", "abc"}, "auth keys remove", []string{"abc"}}, {"over HTTP", true, []string{"--path=auth keys remove", "keys", "remove", "abc"}, "", []string{"abc"}}, } for _, tc := range cases { c := &Ctx{Scope: "git", ViaAPI: tc.viaAPI, Stdout: io.Discard, Stderr: io.Discard} Dispatch(c, tc.argv) if c.CLIPath != tc.want { t.Errorf("%s: CLIPath %q, want %q", tc.name, c.CLIPath, tc.want) } if !slices.Equal(c.Argv, tc.wantArgv) { t.Errorf("%s: Argv %q, want %q", tc.name, c.Argv, tc.wantArgv) } } } ``` - [ ] **Step 2: Run and see it fail** Run: `go test ./internal/control -run TestPathArgument -count=1` Expected: FAIL to compile (`c.CLIPath undefined`). - [ ] **Step 3: Add the field and route it through `Dispatch`** In `internal/control/control.go`, `Ctx` gains a field next to `Term`: ```go // CLIPath is the path the gitbay CLI actually resolved this call to // (cobra.Command.CommandPath(), from a leading --path=), when it // differs from the registered path being dispatched (#267) — auth's // several groupings and repo topics list, today. Empty for stock // ssh, the web and the API: nothing but the gitbay CLI sends one. CLIPath string ``` `Dispatch` strips both leading pseudo-flags in a loop, in whichever order the caller sent them, replacing the single `--term=` check: ```go // A leading --term= selects terminal output for this session, the // same as GITBAY_TERM; a leading --path= carries the CLI's own // invoking path when it differs from the one being dispatched // (#267). Both come off before Lookup, in whichever order the // caller sent them: Lookup matches argv against a command's Path, // and either prefix in front would never match one. Over HTTP both // are dropped unread: the web and the API render no terminal and // have no CLI path of their own. for len(argv) > 0 { if v, ok := strings.CutPrefix(argv[0], "--term="); ok { if !c.ViaAPI { c.Term = ParseTerm(v) } argv = argv[1:] continue } if v, ok := strings.CutPrefix(argv[0], "--path="); ok { if !c.ViaAPI { c.CLIPath = v } argv = argv[1:] continue } break } if len(argv) == 0 { return c.fail(protocol.ExitUsage, "no command given; try: ssh help") } ``` - [ ] **Step 4: Run** Run: `go test ./internal/control -run "TestPathArgument|TestTermArgument" -count=1` Expected: PASS. - [ ] **Step 5: Write the failing test for rendering** ```go func TestCmdUsagePrefixesTheProgram(t *testing.T) { c := &Ctx{Cmd: Command{Path: []string{"keys", "remove"}, Usage: "keys remove "}, Cfg: config.Config{Server: config.Server{SiteURL: "https://forge.test"}}} if got := c.cmdUsage(); got != "ssh git@forge.test keys remove " { t.Errorf("ssh form: %q", got) } c.Term = Term{Cols: 100} if got := c.cmdUsage(); got != "gitbay keys remove " { t.Errorf("cli form, no CLIPath sent: %q", got) } c.CLIPath = "auth keys remove" if got := c.cmdUsage(); got != "gitbay auth keys remove " { t.Errorf("cli form, mismatched registered path: %q", got) } c2 := &Ctx{Cmd: Command{Path: []string{"repo", "tree"}, Usage: "repo tree [] [--ref ]"}, Term: Term{Cols: 100}} if got := c2.cmdUsage(); got != "gitbay repo tree [] [] [--ref ]" { t.Errorf("optional owner/name: %q", got) } c2.CLIPath = "repo tree" // matches the registered path: a no-op if got := c2.cmdUsage(); got != "gitbay repo tree [] [] [--ref ]" { t.Errorf("matching CLIPath changes nothing: %q", got) } } ``` - [ ] **Step 6: Run and see it fail** Run: `go test ./internal/control -run TestCmdUsagePrefixesTheProgram -count=1` Expected: FAIL to compile (`c.cmdUsage undefined`). - [ ] **Step 7: Implement `cliUsage`, `shownAs` and `cmdUsage` in `internal/control/help.go`** ```go // cliUsage marks a leading optional in a CLI-rendered usage // line: the CLI infers it inside a clone (cmd/gitbay/ssh.go's withRepo), // stock ssh never does. Only the first occurrence is marked — a usage // line never repeats the placeholder. func cliUsage(usage string) string { return strings.Replace(usage, "", "[]", 1) } // shownAs returns how a registered path should print to this caller: // the CLI path it sent (Ctx.CLIPath) standing in for the leading // portion that corresponds to registered, with full's remainder kept // as-is; or full unchanged for stock ssh, the API, or a caller whose // CLI path already agrees with the registered one. registered must be // a genuine leading substring of full (a command's own registered path // always is, against its own Usage or a sibling's full path). func (c *Ctx) shownAs(registered, full string) string { if c.CLIPath == "" || c.CLIPath == registered { return full } return c.CLIPath + strings.TrimPrefix(full, registered) } // cmdUsage is the registered usage as this call should see it: the CLI // path this session actually typed when it differs from the registered // one (#267), the gitbay form otherwise, the ssh form when there is no // terminal — with a leading marked optional at a terminal. // Every usage message — the --help path and a wrong-argument refusal // alike — goes through this, so a caller never sees a command it // cannot actually run. func (c *Ctx) cmdUsage() string { registered := joinPath(c.Cmd.Path) shape := c.shownAs(registered, c.Cmd.Usage) if c.Term.Cols > 0 { shape = cliUsage(shape) } return c.program() + " " + shape } ``` - [ ] **Step 8: Run** Run: `go test ./internal/control -run TestCmdUsagePrefixesTheProgram -count=1` Expected: PASS. - [ ] **Step 9: Route `usage`/`usageWith` through it** In `internal/control/control.go`: ```go // usage reports a bad invocation with the command's registered usage, // the one source of it. func (c *Ctx) usage() int { return c.fail(protocol.ExitUsage, "usage: %s", c.cmdUsage()) } // usageWith reports a specific problem with the arguments, then the // registered usage, so a person always sees the shape that was expected. func (c *Ctx) usageWith(msg string) int { return c.fail(protocol.ExitUsage, "%s\nusage: %s", msg, c.cmdUsage()) } ``` - [ ] **Step 10: `helpVerb` gets the same treatment** `helpVerb` (`internal/control/help.go`) recomputes a "cut at ` [--`" shape independently, and lists sibling commands under SEE ALSO by their full registered path. Both go through `shownAs` now: ```go func (c *Ctx) helpVerb(w io.Writer, cmd Command, below []Command) { fmt.Fprintln(w, cmd.Summary) fmt.Fprintln(w) c.heading(w, "USAGE") // Cutting at the first optional flag drops the rest of the usage // syntax behind "[flags]" — safe only for what is actually optional. // A required flag (repo delete --yes) or an alternative // (notifications read ... | --all) has no " [--" to cut at, so // the usage prints whole. registered := joinPath(cmd.Path) shape := c.shownAs(registered, cmd.Usage) if c.Term.Cols > 0 { shape = cliUsage(shape) } if i := strings.Index(shape, " [--"); i >= 0 { shape = shape[:i] + " [flags]" } fmt.Fprintf(w, " %s %s\n", c.program(), shape) fmt.Fprintln(w) c.heading(w, "FLAGS") rows := make([][2]string, 0, len(cmd.Flags)+1) for _, f := range cmd.Flags { name := f.Name if f.Arg != "" { name += " " + f.Arg } desc := f.Desc if f.Default != "" { desc += " (default " + f.Default + ")" } rows = append(rows, [2]string{name, desc}) } rows = append(rows, [2]string{"--json", "machine-readable output"}) wide := 0 for _, r := range rows { wide = max(wide, cells(r[0])) } for _, r := range rows { c.wrapLine(w, " "+pad(r[0], wide)+" ", r[1]) } if len(cmd.Examples) > 0 { fmt.Fprintln(w) c.heading(w, "EXAMPLES") for _, ex := range cmd.Examples { c.wrapLine(w, " "+c.program()+" ", ex) } } if len(below) > 0 { fmt.Fprintln(w) c.heading(w, "SEE ALSO") for _, b := range below { fmt.Fprintf(w, " %s %s\n", c.program(), c.shownAs(registered, joinPath(b.Path))) } } } ``` (Only the `USAGE` and `SEE ALSO` lines change; `FLAGS`/`EXAMPLES` are reproduced above unchanged, for the diff to apply against the current file — do not re-type them from scratch.) - [ ] **Step 11: `helpNoun` gets the same treatment** `helpNoun` prints `{program} {prefix} ...` and, per command, the verb relative to `prefix`; the prefix itself needs the same swap (Task 2.3 also gives it an `override` map, threaded through here empty for every noun but the CLI-only `auth` alias): ```go func (c *Ctx) helpNoun(w io.Writer, prefix string, cmds []Command, override map[string]string) { head := nounSummaries[strings.Fields(prefix)[0]] fmt.Fprintln(w, head) fmt.Fprintln(w) c.heading(w, "USAGE") display := c.shownAs(prefix, prefix) fmt.Fprintf(w, " %s %s ...\n", c.program(), display) rowText := func(cmd Command) string { full := joinPath(cmd.Path) if d, ok := override[full]; ok { return d } return strings.TrimPrefix(full, prefix+" ") } wide := 0 for _, cmd := range cmds { wide = max(wide, cells(rowText(cmd))) } for _, section := range []struct { title string read bool }{{"READ", true}, {"WRITE", false}} { first := true for _, cmd := range cmds { if cmd.ReadOnly != section.read { continue } if first { fmt.Fprintln(w) c.heading(w, section.title) first = false } fmt.Fprintf(w, " %s %s\n", pad(rowText(cmd), wide), cmd.Summary) } } fmt.Fprintln(w) fmt.Fprintf(w, "%s %s --help for flags.\n", c.program(), display) } ``` `runHelp`'s one call site becomes `c.helpNoun(w, prefix, matched, nil)` for now; Task 2.3 gives it a real map for the `auth` alias. A `nil` map's zero value behaves like an empty one — `override[full]` on a `nil` map is always `"", false` — so every other noun is unaffected. - [ ] **Step 12: Update the test `usage`/`usageWith` changes** `TestArgumentRefusalsNameTheUsage` in `internal/control/control_test.go` asserts `errOut.String()` contains the bare `"usage: " + strings.Join(argv, " ")`; with no `Cfg.Server.SiteURL` and no `Term` set on its `Ctx`, the message now reads `usage: ssh git@ build show` (an empty host — `hostOf("")` returns `""`). None of these four commands is one of the eighteen with a mismatched CLI path, and the test sends no `--path=`, so `c.CLIPath` stays empty throughout — set a `SiteURL` and assert the plain ssh-prefixed registered form: ```go func TestArgumentRefusalsNameTheUsage(t *testing.T) { for _, argv := range [][]string{{"build", "show"}, {"release", "show"}, {"mr", "resolve"}, {"issue", "show"}} { var out, errOut bytes.Buffer c := &Ctx{Scope: "full", Stdout: &out, Stderr: &errOut, Cfg: config.Config{Server: config.Server{SiteURL: "https://forge.test"}}} if code := Dispatch(c, argv); code != protocol.ExitUsage { t.Errorf("%v: exit %d, want %d (%s)", argv, code, protocol.ExitUsage, errOut.String()) continue } want := "usage: ssh git@forge.test " + strings.Join(argv, " ") if !strings.Contains(errOut.String(), want) { t.Errorf("%v: no usage line: got %q, want to contain %q", argv, errOut.String(), want) } } } ``` (Add `"gitbay.org/gitbay/internal/config"` to the file's imports if it is not already there.) - [ ] **Step 13: Run the package** Run: `go test ./internal/control -count=1` Expected: PASS. Any other test asserting a bare `"usage: ..."` with no program prefix needs the same treatment — `grep -rn '"usage: ' internal/control/*_test.go` finds them all; a `help`-rendering test asserting a bare `"gitbay keys ..."` line for one of the eighteen commands needs the CLI-prefixed form instead. - [ ] **Step 14: The CLI side — write the failing tests for the two pure helpers** `cmd/gitbay/ssh.go` needs a way to read a cobra command's own path, and a way to fold it onto a server command line, both pure and cheap to unit test the way `termValue`/`pagerArgv`/`pages` already are (`cmd/gitbay/term_test.go`): ```go func TestCLIPathOf(t *testing.T) { root := &cobra.Command{Use: "gitbay"} auth := &cobra.Command{Use: "auth"} keys := &cobra.Command{Use: "keys"} remove := &cobra.Command{Use: "remove"} keys.AddCommand(remove) auth.AddCommand(keys) root.AddCommand(auth) if got := cliPathOf(remove); got != "auth keys remove" { t.Errorf("cliPathOf = %q", got) } } func TestWithCLIPath(t *testing.T) { if got := withCLIPath("", []string{"keys", "remove", "abc"}); !slices.Equal(got, []string{"keys", "remove", "abc"}) { t.Errorf("empty cliPath: %v", got) } got := withCLIPath("auth keys remove", []string{"keys", "remove", "abc"}) want := []string{"--path=auth keys remove", "keys", "remove", "abc"} if !slices.Equal(got, want) { t.Errorf("got %v, want %v", got, want) } } ``` Add these to `cmd/gitbay/term_test.go`, alongside `TestTermValue` and `TestPagerArgv`; add `"slices"` and `"github.com/spf13/cobra"` to its imports if not already there. - [ ] **Step 15: Run and see them fail** Run: `go test ./cmd/gitbay -run "TestCLIPathOf|TestWithCLIPath" -count=1` Expected: FAIL to compile (`cliPathOf`/`withCLIPath` undefined). - [ ] **Step 16: Implement the two helpers in `cmd/gitbay/ssh.go`** Add `"github.com/spf13/cobra"` to the file's imports, then: ```go // cliPathOf is the path this cobra command was actually reached by, // stripped of the root's own name: "auth keys remove" for a command // nested under auth > keys > remove. It is sent to the server as // --path=, so usage and help can print what the caller can actually // run even where that differs from the registered path being // dispatched (cmd.Annotations[serverPath]) — #267. func cliPathOf(cmd *cobra.Command) string { return strings.TrimPrefix(cmd.CommandPath(), "gitbay ") } // withCLIPath prepends --path= to a server command line, the // same way runSSHPaged prepends --term=: a leading pseudo-flag Dispatch // strips before Lookup, never confused for a real argument. Empty // cliPath is a no-op — nothing to add for a caller with no cobra tree // of its own to have resolved. func withCLIPath(cliPath string, argv []string) []string { if cliPath == "" { return argv } return append([]string{"--path=" + cliPath}, argv...) } ``` - [ ] **Step 17: Run** Run: `go test ./cmd/gitbay -run "TestCLIPathOf|TestWithCLIPath" -count=1` Expected: PASS. - [ ] **Step 18: Thread `cliPath` through `pass`, `runServerHelp`, `runPass`** In `cmd/gitbay/main.go`: ```go func pass(use string, o passOpts) *cobra.Command { return &cobra.Command{ Use: use, Short: summaries[strings.Join(o.server, " ")], Annotations: map[string]string{ serverPath: strings.Join(o.server, " "), stdinMode: o.stdinModeName(), stdinWhat: o.stdinWhat, }, DisableFlagParsing: true, RunE: func(cmd *cobra.Command, args []string) error { // The registry is the only place flags are written down, so // --help asks the server rather than reprinting the one-line // summary cobra holds. cliPath := cliPathOf(cmd) for _, a := range args { if a == "--help" || a == "-h" { os.Exit(runServerHelp(o, cliPath)) } } os.Exit(runPass(o, cliPath, args)) return nil }, } } ``` ```go // runServerHelp prints the registry's usage for one command. func runServerHelp(o passOpts, cliPath string) int { t, err := resolveTarget() if err != nil { fmt.Fprintln(os.Stderr, "gitbay:", err) return protocol.ExitFailure } return runSSH(t, withCLIPath(cliPath, append([]string{"help"}, o.server...)), strings.NewReader("")) } func runPass(o passOpts, cliPath string, args []string) int { ``` (`runPass`'s body is otherwise unchanged; only its signature gains `cliPath string` as the second parameter, and its final line becomes:) ```go return runSSHPaged(t, withCLIPath(cliPath, append(o.server, args...)), stdin, pages(o.server, args)) ``` - [ ] **Step 19: Thread `cliPath` through `group`/`serverHelp`** ```go func group(use, short string, subs ...*cobra.Command) *cobra.Command { c := &cobra.Command{Use: use, Short: short} c.AddCommand(subs...) // A noun's help is the server's, like a command's: the registry is // the only place flags are written down, and cobra's subcommand list // carried none (#130). Offline, or for a noun the server does not // know by that name, cobra's own tree still prints. local := c.HelpFunc() c.SetHelpFunc(func(cmd *cobra.Command, args []string) { if !serverHelp(use, cliPathOf(cmd)) { local(cmd, args) } }) return c } // serverHelp prints the registry's usage for a prefix and reports whether // it did. cliPath is this invocation's own resolved cobra path (empty for // a noun whose CLI path already matches its registered prefix). At a // terminal it goes through the terminal-aware path, so it gets the same // --term=[,color] treatment (and layout) as any other command; // piped, it stays a quiet capture, so a network or lookup failure falls // back to cobra's local help without noise. func serverHelp(prefix, cliPath string) bool { t, err := resolveTarget() if err != nil { return false } argv := withCLIPath(cliPath, []string{"help", prefix}) if term.IsTerminal(int(os.Stdout.Fd())) { return runSSH(t, argv, strings.NewReader("")) == 0 } out, code := sshCapture(t, argv) if code != 0 || out == "" { return false } fmt.Print(out) return true } ``` - [ ] **Step 20: Build** Run: `go build ./... && go vet ./...` Expected: builds clean. `keysAdd.RunE`/`pgpAdd.RunE` in `authCmd()` (`cmd/gitbay/main.go`) are hand-built, not `pass()`-generated, so they do not pick up `cliPath` from this step — Task 2.2 gives them the same treatment where it already rewrites their bodies. - [ ] **Step 21: The cobra tree is the one place the eighteen mismatches are allowed to be listed — a coverage test, not a hand check** A future command wired with a `serverPath` that does not match its own `CommandPath()` is exactly this defect happening again; nothing should have to remember to re-check it by hand. `TestEveryCommandIsReachable` (`cmd/gitbay/coverage_test.go`) already walks `newRoot()` comparing `Annotations[serverPath]` against the registry — this test walks the same tree comparing it against `cliPathOf`, and pins today's known set so any change to it (a new mismatch, or one of these being fixed to match) shows up as a diff a reviewer has to look at. ```go package main import ( "slices" "strings" "testing" "github.com/spf13/cobra" ) // TestServerPathMismatches pins the commands whose CLI path differs from // the server path they dispatch (#267) — cmdUsage/help print the CLI // path for exactly these, from cliPathOf, not the registered one. A new // mismatch changes this list; update it deliberately, alongside the // wiki's Parity page if it changes what a stock-ssh caller must type. func TestServerPathMismatches(t *testing.T) { want := []string{ "auth email add", "auth email list", "auth email primary", "auth email remove", "auth email verify", "auth export", "auth keys add", "auth keys label", "auth keys list", "auth keys remove", "auth pgp add", "auth pgp list", "auth pgp remove", "auth token create", "auth token list", "auth token revoke", "auth whoami", "repo topics list", } var got []string var walk func(*cobra.Command) walk = func(c *cobra.Command) { if p := c.Annotations[serverPath]; p != "" { if cli := cliPathOf(c); cli != p { got = append(got, cli) } } for _, sub := range c.Commands() { walk(sub) } } root := newRoot() root.InitDefaultHelpCmd() walk(root) slices.Sort(got) if !slices.Equal(got, want) { t.Errorf("mismatched CLI paths = %v\nwant %v", got, want) } } ``` - [ ] **Step 22: Run** Run: `go test ./cmd/gitbay -run TestServerPathMismatches -count=1` Expected: PASS (the eighteen are already there today; this step only adds the guard, it changes no behavior). - [ ] **Step 23: One end-to-end proof, real ssh and the real binary** Every other test here is a unit test against `Ctx`/cobra values built by hand; this is the one e2e test for this task (Global Constraints caps it at one), proving the wiring — `runSSHPaged`'s `--path=` prepend, the server's `--path=` parse, `cmdUsage`'s substitution — actually reaches an instance over real ssh, for the CLI and for stock ssh alike. Model it on `e2e/term_test.go`'s `TestTermEnvSelectsTerminalOutput`. ```go package e2e import ( "strings" "testing" ) // The CLI sends its own invoking path so usage and help print a command // that exists — gitbay auth keys remove, never the unregistered gitbay // keys remove (#267). Stock ssh, which never sends one, keeps seeing // the registered path: it is the only one it could ever type. func TestCLIUsagePrintsTheInvokingPath(t *testing.T) { t.Parallel() inst := startInstance(t) key := inst.newKey(t, "alice") inst.admin(t, "admin", "user", "create", "alice", "--key", key+".pub", "--email", "alice@example.test", "--verified") c := &cli{bin: buildGitbayCLI(t), configDir: t.TempDir(), inst: inst, key: key} c.must(t, "", "", "remote", "add", "test", "127.0.0.1", "--port", instPort(inst), "--ssh-option", "-i", "--ssh-option", key, "--ssh-option", "-oIdentitiesOnly=yes", "--ssh-option", "-oStrictHostKeyChecking=no", "--ssh-option", "-oUserKnownHostsFile="+inst.sshDir+"/kh", "--ssh-option", "-oBatchMode=yes", "--default") // A mismatched command: the CLI path (auth keys remove) differs from // the registered one (keys remove). No fingerprint given, an actual // wrong-argument refusal. _, errOut, code := c.run(t, "", "", "auth", "keys", "remove") if code == 0 || !strings.Contains(errOut, "usage: gitbay auth keys remove") { t.Errorf("mismatched command: exit %d, stderr %q", code, errOut) } if strings.Contains(errOut, "usage: gitbay keys remove") { t.Errorf("mismatched command leaked the registered path: %q", errOut) } // A matching command: no CLI/registered difference, still the gitbay // form (it is a terminal-adjacent test binary run, isTTY is false // here, so this exercises the non-terminal ssh form instead — // assert on the registered path itself, which is all cmdUsage can // tell apart in that mode). _, errOut2, code2 := c.run(t, "", "", "repo", "show") if code2 == 0 || !strings.Contains(errOut2, "usage: ") || !strings.Contains(errOut2, "repo show") { t.Errorf("matching command: exit %d, stderr %q", code2, errOut2) } // Stock ssh, no CLI involved: the registered path, because it is the // only one this caller could have typed. _, errOut3, code3 := inst.ssh(t, key, "", "keys", "remove") if code3 == 0 || !strings.Contains(errOut3, "usage: ssh git@") || !strings.Contains(errOut3, "keys remove") { t.Errorf("stock ssh: exit %d, stderr %q", code3, errOut3) } if strings.Contains(errOut3, "auth keys remove") { t.Errorf("stock ssh should never see the CLI-only auth prefix: %q", errOut3) } } ``` (`instPort`/`inst.sshDir`/`c.run`/`inst.ssh` are whatever `e2e/cli_test.go` and `e2e/term_test.go` already expose — read both before writing this file and use their actual helper names and signatures rather than the ones guessed here; `TestCLI` in `e2e/cli_test.go` is the fullest existing example of standing up a `cli` value against a live `instance`.) - [ ] **Step 24: Run the one e2e test** Run: `go test ./e2e -run TestCLIUsagePrintsTheInvokingPath -count=1` Expected: PASS. - [ ] **Step 25: Run everything this task touched, commit** Run: `go build ./... && go vet ./... && go test ./internal/control ./cmd/gitbay -count=1` Expected: PASS. ```bash git add internal/control/help.go internal/control/control.go internal/control/control_test.go internal/control/help_test.go cmd/gitbay/ssh.go cmd/gitbay/main.go cmd/gitbay/term_test.go cmd/gitbay/serverpath_test.go e2e/cliusage_test.go git commit -m "usage, help: print the CLI's own invoking path where it differs from the registered one, the ssh form otherwise" -m "Ref #267" ``` ### Task 2.2: `--help` check in `keys add` and `pgp add` Every other passthrough command checks for `--help`/`-h` in `pass()` before reading stdin; `keysAdd.RunE` and `pgpAdd.RunE` in `cmd/gitbay/main.go`'s `authCmd()` were given their own `RunE` (to wire stdin directly) and lost that check, so `gitbay auth keys add --help` tries to read a public key from stdin instead of showing help, and blocks or fails depending on what stdin happens to be. **Files:** - Modify: `cmd/gitbay/main.go` (`authCmd`'s `keysAdd.RunE`, `pgpAdd.RunE`) - Test: `cmd/gitbay/main_test.go` - [ ] **Step 1: Write the failing test** ```go func TestKeysAddAndPGPAddCheckHelpBeforeStdin(t *testing.T) { for _, args := range [][]string{{"auth", "keys", "add", "--help"}, {"auth", "pgp", "add", "--help"}} { root := newRoot() root.SetArgs(args) root.SetIn(strings.NewReader("")) // would block/fail if read as the key body if err := root.Execute(); err != nil { t.Errorf("%v: %v", args, err) } } } ``` (`cmd/gitbay` runs its `RunE` through `os.Exit`, so this test only proves the command does not attempt to read stdin as a key before exiting — check with `go test ./cmd/gitbay -run TestKeysAddAndPGPAddCheckHelpBeforeStdin -count=1 -v` that it does not hang; if the harness needs the process not to call `os.Exit` at all, grep `main_test.go` for how existing `--help` tests in this package already handle that and follow the same pattern rather than inventing a new one.) - [ ] **Step 2: Run and see it fail (or hang)** Run: `go test ./cmd/gitbay -run TestKeysAddAndPGPAddCheckHelpBeforeStdin -count=1 -timeout 5s` Expected: FAIL or timeout (stdin read attempted). - [ ] **Step 3: Implement** `keysAdd.RunE` and `pgpAdd.RunE` in `cmd/gitbay/main.go` each gain the same loop `pass()` already has, before resolving the target. They also pick up `cliPath` here (Task 2.1 gave `runServerHelp` a second parameter but could not touch these two hand-built `RunE`s, since this task is what rewrites their bodies): `auth keys add` and `auth pgp add` are two of the eighteen commands whose CLI path differs from the registered one, so their own usage refusals need it exactly like every `pass()`-generated command's do. ```go keysAdd.RunE = func(cmd *cobra.Command, args []string) error { cliPath := cliPathOf(cmd) for _, a := range args { if a == "--help" || a == "-h" { os.Exit(runServerHelp(passOpts{server: []string{"keys", "add"}}, cliPath)) } } t, err := resolveTarget() if err != nil { return err } in, err := stdinPayload(os.Stdin, "an SSH public key", false) if err != nil { return err } os.Exit(runSSH(t, withCLIPath(cliPath, append([]string{"keys", "add"}, args...)), in)) return nil } ``` and, for `pgpAdd`: ```go RunE: func(cmd *cobra.Command, args []string) error { cliPath := cliPathOf(cmd) for _, a := range args { if a == "--help" || a == "-h" { os.Exit(runServerHelp(passOpts{server: []string{"pgp", "add"}}, cliPath)) } } t, err := resolveTarget() if err != nil { return err } in, err := stdinPayload(os.Stdin, "an armored OpenPGP public key", false) if err != nil { return err } os.Exit(runSSH(t, withCLIPath(cliPath, append([]string{"pgp", "add"}, args...)), in)) return nil }, ``` - [ ] **Step 4: Run** Run: `go test ./cmd/gitbay -run TestKeysAddAndPGPAddCheckHelpBeforeStdin -count=1 -timeout 5s` Expected: PASS. - [ ] **Step 5: Build and run the package** Run: `go build ./... && go test ./cmd/gitbay -count=1` Expected: PASS. - [ ] **Step 6: Commit** ```bash git add cmd/gitbay/main.go cmd/gitbay/main_test.go git commit -m "auth keys add, pgp add: check --help before reading stdin" -m "Ref #267" ``` ### Task 2.3: `auth --help` renders with the registry layout, in the CLI's own paths `auth` is a CLI-only grouping — no registry command's path starts with `auth`, so `gitbay auth --help` asks the server for help on prefix `"auth"`, gets `ExitNotFound`, and `cmd/gitbay/main.go`'s `group()` falls back to cobra's own subcommand listing, which carries no flags or examples (the reason `group()` exists at all, per its own comment). Give the registry an alias table for CLI-only groupings so `auth` renders the same READ/WRITE, aligned-summary layout every real noun gets — and, since none of the rows it gathers (`keys add`, `account export`, ...) are commands a caller can actually type, each row prints the CLI path it really takes (`auth keys add`, `auth export`), the same substitution Task 2.1 gave a single command's own usage line. Unlike Task 2.1's mismatches, which are one registered path to one CLI path, `auth` gathers several unrelated registered prefixes into one grouping, and one of them (`account export` -> `auth export`) does not even keep the same word count — the alias table has to carry the CLI form alongside each registered prefix explicitly; it cannot be derived by pattern-matching the prefix the way Task 2.1's single-command swap is. **Files:** - Modify: `internal/control/help.go` (`runHelp`, `nounAliases`, `nounSummaries`) - Modify: `cmd/gitbay/main.go` (`authCmd`'s `group("auth", ...)` description) - Test: `internal/control/help_test.go` **Interfaces:** - Produces: `type nounAlias struct { Registered, CLI string }`; `var nounAliases map[string][]nounAlias` — a CLI-only noun name to the registered prefixes it gathers, each paired with the CLI path that reaches it. - [ ] **Step 1: Write the failing tests** Two: the CLI form (a caller that sent `--path=auth`, as `gitbay auth --help` now does per Task 2.1's `group`/`serverHelp` change), and the ssh form (a caller that sent nothing, which cannot run an `auth whatever` command and must not be told to). ```go func TestHelpRendersAnAliasedNounWithTheRegistryLayout(t *testing.T) { var out bytes.Buffer c := &Ctx{Stdout: &out, Term: Term{Cols: 100}, CLIPath: "auth"} if code := runHelp(c, []string{"auth"}); code != protocol.ExitOK { t.Fatalf("exit %d", code) } got := out.String() for _, want := range []string{"auth whoami", "auth keys list", "auth pgp add", "auth token create", "auth export"} { if !strings.Contains(got, want) { t.Errorf("missing %q in:\n%s", want, got) } } if strings.Contains(got, "no command matches") { t.Errorf("auth did not resolve: %s", got) } } func TestHelpRendersAnAliasedNounInRegisteredFormOverSSH(t *testing.T) { var out bytes.Buffer c := &Ctx{Stdout: &out} // no Term, no CLIPath: exactly stock ssh if code := runHelp(c, []string{"auth"}); code != protocol.ExitOK { t.Fatalf("exit %d", code) } got := out.String() for _, want := range []string{"whoami", "keys list", "pgp add", "token create", "account export"} { if !strings.Contains(got, want) { t.Errorf("missing %q in:\n%s", want, got) } } if strings.Contains(got, "auth keys list") { t.Errorf("stock ssh should not see the CLI-only auth prefix: %s", got) } } ``` - [ ] **Step 2: Run and see them fail** Run: `go test ./internal/control -run TestHelpRendersAnAliasedNoun -count=1` Expected: FAIL (`no command matches "auth"`). - [ ] **Step 3: Implement the alias table and the lookup change** In `internal/control/help.go`, near `nounSummaries`: ```go // nounAlias is one bucket of registered commands, reachable under a // CLI-only noun that is not itself a registry path (auth, gathering // several unrelated registry prefixes): Registered is what runHelp // matches against the registry, CLI is the path a gitbay caller // actually types to reach it — not always Registered with the alias's // own name stitched on (account export -> auth export drops a word), // so the two are paired explicitly rather than derived. type nounAlias struct { Registered string CLI string } // nounAliases groups a CLI-only noun into the real prefixes it gathers, // so `help auth` renders with the same layout a real noun gets instead // of falling back to whatever a caller does when help fails. A stock // ssh caller — the only one who could ever ask for a bare "auth" and // get nothing back from the registry — sees the Registered forms // unchanged; the CLI, having sent its own path, sees CLI. var nounAliases = map[string][]nounAlias{ "auth": { {"account export", "auth export"}, {"whoami", "auth whoami"}, {"keys", "auth keys"}, {"email", "auth email"}, {"pgp", "auth pgp"}, {"token", "auth token"}, }, } ``` and add, to `nounSummaries`: ```go "auth": "whoami, SSH and PGP keys, email, API tokens", ``` In `runHelp`, widen the match to every aliased prefix, and — only for a caller that sent its own `CLIPath` — build the per-row override `helpNoun` (Task 2.1) now accepts: ```go func runHelp(c *Ctx, args []string) int { prefix := joinPath(args) prefixes := []string{prefix} override := map[string]string{} if aliased, ok := nounAliases[prefix]; ok { prefixes = nil for _, a := range aliased { prefixes = append(prefixes, a.Registered) } if c.CLIPath != "" { for _, cmd := range registry { p := joinPath(cmd.Path) for _, a := range aliased { if p == a.Registered || strings.HasPrefix(p, a.Registered+" ") { override[p] = a.CLI + strings.TrimPrefix(p, a.Registered) break } } } } } var matched []Command for _, cmd := range registry { p := joinPath(cmd.Path) for _, pfx := range prefixes { if pfx == "" || p == pfx || strings.HasPrefix(p, pfx+" ") { matched = append(matched, cmd) break } } } if len(matched) == 0 { return c.fail(protocol.ExitNotFound, "no command matches %q; try: help", prefix) } slices.SortFunc(matched, func(a, b Command) int { return strings.Compare(joinPath(a.Path), joinPath(b.Path)) }) entries := make([]helpEntry, len(matched)) for i, cmd := range matched { entries[i] = helpEntry{Path: joinPath(cmd.Path), Summary: cmd.Summary, Usage: cmd.Usage, Flags: cmd.Flags, Examples: cmd.Examples} } return c.emit(entries, func(w io.Writer) { switch { case prefix == "": for _, e := range entries { summary := e.Summary if c.Term.Cols > 0 { if avail := c.Term.Cols - max(cells(e.Path), 24) - 1; avail > 0 { summary = clip(summary, avail) } } fmt.Fprintf(w, "%-24s %s\n", e.Path, summary) } case joinPath(matched[0].Path) == prefix: c.helpVerb(w, matched[0], matched[1:]) default: c.helpNoun(w, prefix, matched, override) } }) } ``` `matched[0].Path` never equals `"auth"` literally (nothing in the registry is named that), so an aliased noun always takes the `helpNoun` branch. `override` stays an empty (non-nil) map for every ordinary noun — `override[full]` misses for every row, and `helpNoun` falls back to its plain `strings.TrimPrefix` — so this changes nothing for `help repo` or any other real prefix. - [ ] **Step 4: Sync the CLI's own description** `cmd/gitbay/main.go`'s `authCmd()`: ```go return group("auth", "whoami, SSH and PGP keys, email, API tokens", ``` (`TestGroupsSayWhatTheServerSays` checks this against `nounSummaries["auth"]`, added above.) - [ ] **Step 5: Run** Run: `go test ./internal/control -run TestHelpRendersAnAliasedNoun -count=1` Expected: PASS. - [ ] **Step 6: Run both packages** Run: `go test ./internal/control ./cmd/gitbay -count=1` Expected: PASS. - [ ] **Step 7: Commit** ```bash git add internal/control/help.go internal/control/help_test.go cmd/gitbay/main.go git commit -m "help: auth (and any future CLI-only grouping) renders with the registry layout, in the CLI's own paths" -m "Ref #267" ``` ### Task 2.4: verb-phrase summaries Six commands' one-line summaries are bare nouns rather than a phrase saying what the command does: `issue comment`/`mr comment` ("comment"), `issue label`/`mr label` ("labels"), `issue assign` ("assignees"), `mr review` ("review"). **Files:** - Modify: `internal/control/issue.go:70`, `:93`, `:102` - Modify: `internal/control/mr.go:146`, `:156`, `:176` - Modify: `cmd/gitbay/summaries_gen.go` (regenerated, not hand-edited) - Test: `cmd/gitbay/summaries_test.go` (existing `TestSummariesAreCurrent` enforces this) - [ ] **Step 1: Change the six `Summary` strings** `internal/control/issue.go:70`: `Summary: "add a comment",` `internal/control/issue.go:93`: `Summary: "add or remove labels",` `internal/control/issue.go:102`: `Summary: "add or remove assignees",` `internal/control/mr.go:146`: `Summary: "add a comment",` `internal/control/mr.go:156`: `Summary: "record a review verdict",` `internal/control/mr.go:176`: `Summary: "add or remove labels",` - [ ] **Step 2: Regenerate `summaries_gen.go`** Run: `go test ./cmd/gitbay -run TestSummariesAreCurrent -update` This rewrites `cmd/gitbay/summaries_gen.go`'s six affected map entries (`"issue comment"`, `"issue label"`, `"issue assign"`, `"mr comment"`, `"mr review"`, `"mr label"`) to the new strings; nothing else in the generated file changes. - [ ] **Step 3: Run** Run: `go test ./internal/control ./cmd/gitbay -count=1` Expected: PASS. - [ ] **Step 4: Commit and open the MR** ```bash git add internal/control/issue.go internal/control/mr.go cmd/gitbay/summaries_gen.go git commit -m "summaries: verb phrases instead of bare nouns" -m "Closes #267" git push -u origin cli-ux-help gitbay mr create --source cli-ux-help --target main --title "CLI help and usage print the form the caller typed" ``` Wait for CI, merge with `--strategy ff`, delete the branch both places. --- # Part 3: unregistered key, issue create flags, mr show plurals, repo readme, mirror time (branch `cli-ux-fixes`, closes #268) ### Task 3.1: the unregistered-key message names the fingerprint and the real host `runAnonymous` in `internal/sshd/sshd.go:332` tells a connecting stranger to register with a literal `` placeholder and no fingerprint, whether they are truly unknown or someone on a new laptop whose existing account has a different key. Print the fingerprint and the real host, and offer both the web and the ssh path. **Files:** - Modify: `internal/sshd/sshd.go` (`runAnonymous`) - Test: `internal/sshd/sshd_test.go` - [ ] **Step 1: Write the failing test** ```go func TestUnregisteredKeyMessageNamesFingerprintAndHost(t *testing.T) { st, cleanup := newTestStore(t) // reuse whatever helper sshd_test.go's other tests use to open a migrated store defer cleanup() srv := &Server{st: st, cfg: config.Config{Server: config.Server{SiteURL: "https://forge.test"}}} pub, _, err := ed25519.GenerateKey(rand.Reader) if err != nil { t.Fatal(err) } sshPub, err := ssh.NewPublicKey(pub) if err != nil { t.Fatal(err) } var out bytes.Buffer ch := &fakeChannel{stderr: &out} // sshd_test.go's existing fake ssh.Channel, if it has one code := srv.runAnonymous(ch, base64.StdEncoding.EncodeToString(sshPub.Marshal()), "whoami") if code != protocol.ExitDenied { t.Fatalf("exit %d", code) } fp := ssh.FingerprintSHA256(sshPub) for _, want := range []string{fp, "forge.test", "https://forge.test/settings#keys", "ssh git@forge.test register"} { if !strings.Contains(out.String(), want) { t.Errorf("message missing %q:\n%s", want, out.String()) } } } ``` `newTestStore`/`fakeChannel` are placeholders for whatever `internal/sshd/sshd_test.go` already provides for its other `runAnonymous`-adjacent tests — read the top of that file (`grep -n "^func " internal/sshd/sshd_test.go`) and use its actual helpers rather than the names guessed here. - [ ] **Step 2: Run and see it fail** Run: `go test ./internal/sshd -run TestUnregisteredKeyMessageNamesFingerprintAndHost -count=1` Expected: FAIL (message contains the literal string ``, no fingerprint). - [ ] **Step 3: Implement** ```go if len(argv) == 0 || argv[0] != "register" { host := strings.TrimSuffix(strings.TrimPrefix(strings.TrimPrefix(s.cfg.Server.SiteURL, "https://"), "http://"), "/") fp := ssh.FingerprintSHA256(pub) flag := map[string]string{"open": "--email
", "invite": "--invite "}[s.cfg.Registration.Mode] fmt.Fprintf(ch.Stderr(), "this key (%s) is not registered on %s.\n"+ "already have an account? add it at https://%s/settings#keys\n"+ "new here? ssh git@%s register --username %s\n", fp, host, host, host, flag) return protocol.ExitDenied } ``` - [ ] **Step 4: Run** Run: `go test ./internal/sshd -run TestUnregisteredKeyMessageNamesFingerprintAndHost -count=1` Expected: PASS. - [ ] **Step 5: Run the package** Run: `go test ./internal/sshd -count=1` Expected: PASS. A failing e2e-adjacent unit test asserting the old `this key is not registered here` text needs its expectation updated the same way. - [ ] **Step 6: Commit** ```bash git add internal/sshd/sshd.go internal/sshd/sshd_test.go git commit -m "sshd: unregistered-key message names the fingerprint and the real host" -m "Ref #268" ``` ### Task 3.2: `issue create` takes `--label`, `--milestone`, `--assignee` `issue create` only sets title, body and format; labels, milestone and assignees each need a separate call afterward, unlike the web form. Add the three flags (label repeatable) and document that `$EDITOR` already opens when neither `--body` nor `--file` is given (`cmd/gitbay/ssh.go`'s `withRepo`/`maybeEditor` machinery already does this via `issueCmd()`'s `editor: "issue"` — this task only adds the missing flags and says so in the registered help). **Files:** - Modify: `internal/control/issue.go` (`init`'s `issue create` registration, `runIssueCreate`) - Test: `internal/control/issue_test.go` **Interfaces:** - Consumes: `parseFlags`/`flagSpec.Multi` (existing), `c.Store.SetIssueLabel`, `c.Store.MilestoneByTitle`, `c.Store.SetIssueMilestone`, `c.Store.UserByUsername`, `c.Store.SetIssueAssignee` (all existing store methods). - [ ] **Step 1: Write the failing test** ```go func TestIssueCreateSetsLabelsMilestoneAndAssignee(t *testing.T) { c := notifTestCtx(t, "alice") repoID, err := c.Store.CreateRepo("user", c.User.ID, "app", "public") if err != nil { t.Fatal(err) } repo, err := c.Store.RepoByID(repoID) if err != nil { t.Fatal(err) } if err := c.Store.SetLabel(repo, "bug", "ff0000"); err != nil { t.Fatal(err) } if _, err := c.Store.CreateMilestone(repo.ID, "m1", ""); err != nil { t.Fatal(err) } if _, err := c.Store.CreateUser("bob", false); err != nil { t.Fatal(err) } if code := runIssueCreate(c, []string{repo.Path(), "--title", "t", "--label", "bug", "--milestone", "m1", "--assignee", "bob"}); code != 0 { t.Fatalf("exit %d: %s", code, c.Stderr.(*bytes.Buffer).String()) } issue, err := c.Store.IssueByNumber(repo.ID, 1) if err != nil { t.Fatal(err) } if len(issue.Labels) != 1 || issue.Labels[0] != "bug" { t.Errorf("labels = %v", issue.Labels) } if issue.Milestone != "m1" { t.Errorf("milestone = %q", issue.Milestone) } if len(issue.Assignees) != 1 || issue.Assignees[0] != "bob" { t.Errorf("assignees = %v", issue.Assignees) } } ``` (`c.Store.SetLabel`/`CreateMilestone` are placeholders for the real label/milestone creation helpers — `grep -n "func (s \*Store) SetLabel\|func (s \*Store) CreateMilestone" internal/store/*.go` for their actual names and signatures and use those; `issue.Milestone` similarly needs to match whatever field `store.Issue` actually carries for its milestone title, e.g. via `grep -n "Milestone" internal/store/issues.go`.) - [ ] **Step 2: Run and see it fail** Run: `go test ./internal/control -run TestIssueCreateSetsLabelsMilestoneAndAssignee -count=1` Expected: FAIL, exit 2 (`--label` not accepted). - [ ] **Step 3: Register the new flags** ```go register(Command{Path: []string{"issue", "create"}, Summary: "open an issue", Usage: "issue create --title [--body | --file -] [--format md|org] [--label ]... [--milestone ] [--assignee <user>]...", Flags: []Flag{ {"--title", "<t>", "the issue's title", ""}, {"--body", "<b>", "the issue's body", ""}, {"--file", "-", "read the body from stdin", ""}, {"--format", "md|org", "the body's markup", "md"}, {"--label", "<l>", "label to add, may repeat", ""}, {"--milestone", "<title>", "milestone to set", ""}, {"--assignee", "<user>", "user to assign, may repeat", ""}, }, Examples: []string{ `issue create krz/gitbay --title "crash on empty repo" --body "steps to reproduce..."`, "issue create krz/gitbay --title notes --file - < notes.md", "issue create krz/gitbay --title bug --label bug --label priority --milestone v1 --assignee cmc", }, ReadsStdin: true, Run: runIssueCreate}) ``` Note in a doc comment above `runIssueCreate`, since the flags list above has no room for prose: `$EDITOR` opens for the body when the CLI is asked for neither `--body` nor `--file` — that behavior is entirely client-side (`cmd/gitbay/main.go`'s `issueCmd()` already sets `editor: "issue"`), this registration only documents it: ```go // runIssueCreate opens an issue. The CLI opens $EDITOR for the body // when neither --body nor --file is given (cmd/gitbay's issueCmd, // editor: "issue"); over stock ssh the body must be one of the two. func runIssueCreate(c *Ctx, args []string) int { f, err := parseFlags(args, flagSpec{ Values: []string{"--format", "--title", "--body", "--file", "--milestone"}, Multi: []string{"--label", "--assignee"}, MaxPos: 1, Usage: "issue create <owner/name> --title <t> [--body <b> | --file -] [--format md|org] [--label <l>]... [--milestone <title>] [--assignee <user>]...", }) if err != nil { return c.fail(protocol.ExitUsage, "%v", err) } ``` - [ ] **Step 4: Set labels, milestone and assignees after creation** After the existing `n, err := c.Store.CreateIssue(...)` block and its `RecordEvent`/notify calls, before the final `return c.emit(...)`: ```go for _, l := range f.List("--label") { if err := c.Store.SetIssueLabel(repo, n, l, true); err != nil { return c.failErr(err) } } if m := f.Value("--milestone"); m != "" { ms, err := c.Store.MilestoneByTitle(repo, m) if err != nil { return milestoneErr(c, repo, m, err) } if err := c.Store.SetIssueMilestone(n, ms.ID); err != nil { return c.fail(protocol.ExitFailure, "%v", err) } } for _, name := range f.List("--assignee") { u, err := c.Store.UserByUsername(name) if errors.Is(err, store.ErrNotFound) { return c.fail(protocol.ExitNotFound, "no such user %q", name) } if err != nil { return c.fail(protocol.ExitFailure, "%v", err) } if err := c.Store.SetIssueAssignee(n, u.ID, true); err != nil { return c.fail(protocol.ExitFailure, "%v", err) } } ``` `SetIssueLabel`'s second parameter in `runIssueLabel` is `issue.ID`, not the issue number — `CreateIssue` returns the number `n`, so fetch the row first if `SetIssueLabel`/`SetIssueMilestone`/`SetIssueAssignee` all key on the database id rather than the number (check each store method's actual first parameter — `grep -n "func (s \*Store) SetIssueLabel\|SetIssueMilestone\|SetIssueAssignee" internal/store/*.go` and adjust to fetch `issue, err := c.Store.IssueByNumber(repo.ID, n)` first if any of them needs `issue.ID` rather than `n`). Add `"errors"` to the file's imports if not already present. - [ ] **Step 5: Run** Run: `go test ./internal/control -run TestIssueCreateSetsLabelsMilestoneAndAssignee -count=1` Expected: PASS. - [ ] **Step 6: Run the package, regenerate the CLI summary if `Usage` changed its flag list** Run: `go test ./internal/control -count=1` Expected: PASS (the `Summary` string is unchanged, so `summaries_gen.go` does not need regenerating — only `Usage`/`Flags` changed, which is not part of that generated file). - [ ] **Step 7: Commit** ```bash git add internal/control/issue.go internal/control/issue_test.go git commit -m "issue create: --label, --milestone, --assignee" -m "Ref #268" ``` ### Task 3.3: `mr show` pluralizes its multi-row section headings `mr show`'s commit/check/review sub-tables print a singular label (`commit:`, `check:`) even when they hold several rows. **Files:** - Modify: `internal/control/mr.go` (the three `v.section(...)` calls around lines 793, 802, 811) - Test: `internal/control/mr_test.go` - [ ] **Step 1: Write the failing test** Find `mr show`'s existing plain-output test (`grep -n "func Test.*MRShow" internal/control/mr_test.go`) and add a case with more than one commit, check and review, asserting the plural, counted heading: ```go func TestMRShowPluralizesMultiRowSections(t *testing.T) { // build on whatever fixture the existing MR-show tests in this file // use to get a repo with an open MR; push two commits onto its // source branch, set two statuses, and record two reviews before // calling runMRShow, following that fixture's own setup exactly. ... out := ... // runMRShow's plain stdout for _, want := range []string{"commits (2):", "checks (2):", "reviews (2):"} { if !strings.Contains(out, want) { t.Errorf("missing %q in:\n%s", want, out) } } } ``` - [ ] **Step 2: Run and see it fail** Run: `go test ./internal/control -run TestMRShowPluralizesMultiRowSections -count=1` Expected: FAIL (headings read `commit:`, `check:`, `review:`). - [ ] **Step 3: Implement** ```go if len(commits) > 1 { v.section(fmt.Sprintf("commits (%d)", len(commits))) tb := c.table(w, "SHA", "SUBJECT") ... } if len(checks) > 1 { v.section(fmt.Sprintf("checks (%d)", len(checks))) tb := c.table(w, "CHECK", "STATE", "DURATION", "UPDATED") ... } if len(rs) > 1 { v.section(fmt.Sprintf("reviews (%d)", len(rs))) tb := c.table(w, "REVIEWER", "VERDICT", "WHEN") ... } ``` (`v.section` prints `label + ":"` in plain mode already — do not add a trailing colon inside the `fmt.Sprintf` string.) - [ ] **Step 4: Run** Run: `go test ./internal/control -run TestMRShow -count=1` Expected: PASS. - [ ] **Step 5: Run the package** Run: `go test ./internal/control -count=1` Expected: PASS. - [ ] **Step 6: Commit** ```bash git add internal/control/mr.go internal/control/mr_test.go git commit -m "mr show: pluralize commits/checks/reviews section headings" -m "Ref #268" ``` ### Task 3.4: `repo readme` prints a repository's README No command prints a repository's README; the web page's own README-picking logic (`pickReadme` in `internal/httpd/web.go`) is not reachable from `internal/control`. Move it into `internal/control`, exported, and add `repo readme <owner/name> [--ref <ref>]` following `repo cat`'s shape. **Files:** - Modify: `internal/control/read.go` (new `repo readme` registration and `runRepoReadme`, model on `runRepoCat`/`runRepoTree`) - Modify: `internal/httpd/web.go` (move `readmeRank`/`pickReadme` out, call site at line 660 updated) - Modify: `cmd/gitbay/main.go` (`repoCmd`, new `pass("readme", ...)`) - Modify: `e2e/readonly_test.go` (`readArgs["repo readme"]`) - Modify: `.gitbay/wiki/Parity.org` (Repositories table) - Test: `internal/control/read_test.go` **Interfaces:** - Produces: `func PickReadme(entries []gitutil.TreeEntry) string` (moved from `internal/httpd`, exported). - [ ] **Step 1: Move `readmeRank`/`pickReadme`** Cut both from `internal/httpd/web.go` (around lines 1185–1210) and paste into `internal/control/read.go`, renaming `pickReadme` to `PickReadme`: ```go // readmeRank orders competing README files: richer renderers win. var readmeRank = map[string]int{".md": 1, ".markdown": 1, ".org": 2, ".html": 3, ".htm": 3} // PickReadme returns the best README-ish blob in a tree listing: any // file named "readme" or "readme.<ext>" (case-insensitive), preferring // formats we can render richly. func PickReadme(entries []gitutil.TreeEntry) string { best, bestRank := "", 1<<30 for _, e := range entries { if e.Type != "blob" { continue } lower := strings.ToLower(e.Name) if lower != "readme" && !strings.HasPrefix(lower, "readme.") { continue } rank, ok := readmeRank[path.Ext(lower)] if !ok { rank = 10 // plaintext fallback } if rank < bestRank { best, bestRank = e.Name, rank } } return best } ``` In `internal/httpd/web.go`, the call site at line 660 becomes `readmeName := control.PickReadme(entries)`. Remove the unused `"path"` import from `web.go` only if nothing else in the file still uses it (`grep -n '"path"' internal/httpd/web.go` and `grep -n "path\." internal/httpd/web.go` — this file is large and almost certainly uses `path` elsewhere, so this removal is likely a no-op check, not an edit). - [ ] **Step 2: Build to confirm the move alone is clean** Run: `go build ./... && go vet ./...` Expected: no errors. - [ ] **Step 3: Write the failing test for the new command** ```go func TestRepoReadmePicksTheRichestFormat(t *testing.T) { st, repo, uid := newQueueTestRepo(t) dir := RepoDir(config.Config{}.Server.Root, repo.OwnerName, repo.Name) // adjust to however read_test.go's existing repo-cat tests get a working tree with committed files — reuse that helper rather than re-deriving RepoDir's root git := gitRunner(t) git(dir, "init", "--bare") // only if newQueueTestRepo does not already leave a real git repo on disk; check runRepoCat's own test setup and mirror it exactly ... c, errOut := pruneCtx(st, t.TempDir(), store.User{ID: uid}) if code := Dispatch(c, []string{"repo", "readme", repo.Path()}); code != protocol.ExitOK { t.Fatalf("exit %d: %s", code, errOut) } if got := c.Stdout.(*bytes.Buffer).String(); got != "# app\n\nhello\n" { t.Errorf("readme = %q", got) } } ``` `runRepoCat`'s own test in `internal/control/read_test.go` already sets up a real on-disk repository with a committed file — copy that setup exactly (bare repo, a work tree pushed into it, matching `gitTestEnv()`/`gitRunner(t)` from `build_test.go`) rather than reinventing it; commit a `README.md` instead of whatever file that test uses. - [ ] **Step 4: Run and see it fail** Run: `go test ./internal/control -run TestRepoReadmePicksTheRichestFormat -count=1` Expected: FAIL (`unknown command "readme"`). - [ ] **Step 5: Register the command and implement it** In `internal/control/read.go`'s `init()`, after the `repo cat` registration: ```go register(Command{ Path: []string{"repo", "readme"}, Summary: "print a repository's README", Usage: "repo readme <owner/name> [--ref <ref>]", Flags: []Flag{ {"--ref", "<ref>", "branch, tag or commit to read", "the default branch"}, }, Examples: []string{"repo readme krz/gitbay"}, ReadOnly: true, Run: runRepoReadme, }) ``` ```go func runRepoReadme(c *Ctx, args []string) int { pos, ref, code := readArgs(c, args, c.Cmd.Usage, 1) if code >= 0 { return code } if len(pos) != 1 { return c.usage() } repo, code := resolveRepo(c, pos[0], policy.CanRead) if code >= 0 { return code } if ref == "" { ref = repo.DefaultBranch } dir := RepoDir(c.Cfg.Server.Root, repo.OwnerName, repo.Name) if _, err := gitutil.ResolveRef(dir, ref); err != nil { return c.fail(protocol.ExitNotFound, "no ref %q in %s", ref, repo.Path()) } entries, err := gitutil.ListTree(dir, ref, "") if err != nil { return c.fail(protocol.ExitNotFound, "no such path in %s at %s", repo.Path(), ref) } name := PickReadme(entries) if name == "" { return c.fail(protocol.ExitNotFound, "%s has no README at %s", repo.Path(), ref) } limit := c.Cfg.Limits.MaxBlobBytes data, err := gitutil.ReadBlob(dir, ref, name, limit+1) if err != nil { return c.fail(protocol.ExitFailure, "%v", err) } truncated := int64(len(data)) > limit if truncated { data = data[:limit] } binary := gitutil.IsBinary(data) type out struct { Path string `json:"path"` Ref string `json:"ref"` File string `json:"file"` Size int `json:"size"` Truncated bool `json:"truncated,omitempty"` Binary bool `json:"binary,omitempty"` Content string `json:"content,omitempty"` Base64 string `json:"base64,omitempty"` } d := out{Path: repo.Path(), Ref: ref, File: name, Size: len(data), Truncated: truncated, Binary: binary} if binary { d.Base64 = base64.StdEncoding.EncodeToString(data) } else { d.Content = string(data) } return c.emit(d, func(w io.Writer) { if binary { fmt.Fprintf(w, "%s is binary (%d bytes)\n", d.File, d.Size) return } io.WriteString(w, d.Content) if truncated { fmt.Fprintln(w, "... truncated") } }) } ``` (Match `runRepoCat`'s actual truncation/binary field names and JSON tags exactly — read the rest of its `out` struct at `internal/control/read.go:355` onward and copy its shape rather than inventing a divergent one, so a client handles both commands the same way.) - [ ] **Step 6: Run** Run: `go test ./internal/control -run TestRepoReadmePicksTheRichestFormat -count=1` Expected: PASS. - [ ] **Step 7: Wire the CLI passthrough** `cmd/gitbay/main.go`'s `repoCmd()`, next to `pass("cat", ...)`: ```go pass("readme", passOpts{server: []string{"repo", "readme"}, needsRepo: true}), ``` - [ ] **Step 8: Add it to the ReadOnly coverage list** `e2e/readonly_test.go`'s `readArgs` map gains, next to `"repo refs"`: ```go "repo readme": {"alice/app"}, ``` (the fixture's `alice/app` already has a committed `README.md`, so this does not need a `notFoundOK` entry.) - [ ] **Step 9: Update Parity** `.gitbay/wiki/Parity.org`'s Repositories table gains a row, next to `| read a file | yes | yes | yes |`: ``` | render a README | yes | yes | yes | ``` - [ ] **Step 10: Run the full local suite for touched packages** Run: `go build ./... && go vet ./... && go test ./internal/control ./internal/httpd ./cmd/gitbay -count=1` Expected: PASS. - [ ] **Step 11: Commit** ```bash git add internal/control/read.go internal/control/read_test.go internal/httpd/web.go cmd/gitbay/main.go e2e/readonly_test.go .gitbay/wiki/Parity.org git commit -m "repo readme: print a repository's README, the web page's file order" -m "Ref #268" ``` ### Task 3.5: `repo show`'s mirror time drops the milliseconds `repo show`'s mirror sub-table prints `LAST SYNC` with milliseconds (`2026-09-24T15:31:50.839Z`) instead of the second-truncated form every other timestamp in a `view` uses. **Files:** - Modify: `internal/control/repo.go` (`runRepoShow`'s mirror table row, around line 474) - Test: `internal/control/repo_test.go` - [ ] **Step 1: Write the failing test** Find `repo show`'s existing mirror-table test (`grep -n "func Test.*Mirror" internal/control/repo_test.go`), or add one if none exists: ```go func TestRepoShowMirrorTimeIsTruncatedToTheSecond(t *testing.T) { c, repo, _ := newQueueTestRepo(t) // adjust to whatever gives an admin Ctx over a repo with a mirror row in repo_test.go's existing fixtures if err := c.Store.CreateMirror(repo.ID, "push", "ssh://example.test/x.git", ""); err != nil { t.Fatal(err) } if err := c.Store.MarkMirrorSynced(repo.ID, "ssh://example.test/x.git", "2026-09-24T15:31:50.839Z"); err != nil { t.Fatal(err) } var out bytes.Buffer c.Stdout, c.User.IsAdmin = &out, true // repo show's mirror section is admin-only in this Ctx if code := runRepoShow(c, []string{repo.Path()}); code != 0 { t.Fatalf("exit %d", code) } if strings.Contains(out.String(), ".839Z") { t.Errorf("milliseconds leaked: %s", out.String()) } if !strings.Contains(out.String(), "2026-09-24T15:31:50Z") { t.Errorf("no truncated timestamp: %s", out.String()) } } ``` (`CreateMirror`/`MarkMirrorSynced` are placeholders — `grep -n "func (s \*Store) .*Mirror" internal/store/*.go` for the real names/signatures that get a `ListMirrors` row with a non-empty `LastSync`, and use those; `runRepoShow`'s mirror section additionally requires `policy.CanAdmin(c.User, repo, grant)` to hold for the caller, so the test's `Ctx` needs to be the repo's owner or otherwise admin over it — `newQueueTestRepo`'s `uid` already owns the repo it returns, which satisfies that.) - [ ] **Step 2: Run and see it fail** Run: `go test ./internal/control -run TestRepoShowMirrorTimeIsTruncatedToTheSecond -count=1` Expected: FAIL (`.839Z` present). - [ ] **Step 3: Implement** ```go tb.row(cText(m.Direction), cFlex(m.URL), cText(orDash(c.when(m.LastSync))), cState(status)) ``` (`c.when` is already what every other timestamp in a `view` goes through: RFC3339-to-the-second in plain output, `2006-01-02 15:04 UTC` at a terminal; `orDash` keeps an empty `LastSync` — a mirror that has never synced — printing `-` rather than an empty cell, since `c.when("")` returns `""` unchanged.) - [ ] **Step 4: Run** Run: `go test ./internal/control -run TestRepoShowMirrorTimeIsTruncatedToTheSecond -count=1` Expected: PASS. - [ ] **Step 5: Run the package** Run: `go test ./internal/control -count=1` Expected: PASS. - [ ] **Step 6: Commit, open the MR** ```bash git add internal/control/repo.go internal/control/repo_test.go git commit -m "repo show: truncate the mirror's last-sync time to the second" -m "Closes #268" git push -u origin cli-ux-fixes gitbay mr create --source cli-ux-fixes --target main --title "CLI UX review small fixes" ``` Wait for CI, merge with `--strategy ff`, delete the branch both places. --- ## Self-review **Spec coverage** (against #265/#267/#268's text, this plan's spec): - #265: sentences for activity (Task 1.1–1.3), no duplicate assigned issues (Task 1.4), one empty-state wording for `notifications list` (Task 1.5). The issue's other empty-state line ("Empty sections print none; empty lists elsewhere print nothing to list on stderr") already matches current behavior (`internal/control/dashboard.go`'s `section` helper, `internal/control/control.go`'s `emit`) — no task needed. - #267: CLI sends `--term`/`Ctx.Term` (already present; verified, not re-implemented) and now also `--path`/`Ctx.CLIPath`, its own invoking path — the same mechanism, a sibling prefix — used by `usage()`/`usageWith()`/`helpVerb`/`helpNoun` so the eighteen commands whose CLI path differs from the registered one print a command that exists (Task 2.1); `[<owner/name>]` optional from the CLI (Task 2.1); `--help` check in `keys add`/`pgp add`, now also sending their own `cliPath` (Task 2.2); `auth` rendered with the registry layout, each row in the CLI's own path (Task 2.3); verb-phrase summaries (Task 2.4). - #268: unregistered-key message (Task 3.1); `issue create` flags (Task 3.2); `mr show` plurals (Task 3.3); `repo readme` (Task 3.4); `repo show` mirror time (Task 3.5). **Placeholder scan:** Tasks 3.3 and 3.4's tests name real assertions but lean on "copy this file's existing fixture setup" rather than spelling out git plumbing calls verbatim, and Task 3.1's test invents `newTestStore`/`fakeChannel` names to be replaced by whatever `internal/sshd/sshd_test.go` actually has. That is intentional, not a placeholder in the sense the skill warns against: the actual assertions (what strings must appear, what exit code, what store rows) are concrete; only the test-scaffolding names are marked as needing a look at each file's neighbors before typing them in, because this plan was written from reading the production code, not the test helper's exact current shape in every file it touches. Anyone executing this plan reads the named test file's other tests first, per each step's own instruction, before writing the step. **Type consistency:** `FeedLine`/`FeedLines`/`WorstStatus` (Task 1.1) are used with the same names in Tasks 1.2 and 1.3. `Ctx.CLIPath`, `cmdUsage`/`cliUsage`/`shownAs` and the CLI-side `cliPathOf`/ `withCLIPath` (Task 2.1) are used with the same names and signatures in Task 2.2 (`keysAdd.RunE`/`pgpAdd.RunE` sending their own `cliPath`) and Task 2.3 (`helpNoun`'s `override` map, the alias table's `CLI` field built from the same substitution `shownAs` performs for a single command). `PickReadme` (Task 3.4) is the only name introduced for that logic and is used consistently in its own task.