Commit 5cbcadb314
Verified · cmc
Layout: unified · split
docs/superpowers/plans/2026-09-06-mr10-profile-set.md added +237
| @@ -0,0 +1,237 @@ | |||
| 1 | # MR 10: Edit your profile Implementation Plan | ||
| 2 | |||
| 3 | > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. | ||
| 4 | |||
| 5 | **Goal:** Close the `profile set` parity row — edit your description, website, about text and links from the app. | ||
| 6 | |||
| 7 | **Architecture:** One write command behind an edit sheet on your own profile screen. The complication is entirely in `profile set`'s flag semantics, which are unlike anything else this app sends. | ||
| 8 | |||
| 9 | **Spec:** `docs/superpowers/specs/2026-09-06-ios-parity-design.md` | ||
| 10 | |||
| 11 | ## The command, and why it needs care | ||
| 12 | |||
| 13 | ``` | ||
| 14 | profile set [--description <d>] [--website <url>] [--about <text>|--file -] | ||
| 15 | [--about-format md|org] [--link <label|url>]... ('' clears) | ||
| 16 | ``` | ||
| 17 | |||
| 18 | Five things, all verified against `internal/control/profile.go`: | ||
| 19 | |||
| 20 | 1. **`--link` replaces the whole set, it does not append.** `applyProfile` does `p.Links = *e.Links` (`:159-161`). So the editor sends **every link it wants to keep** on every save — dropping one from the form and saving is how you delete it. | ||
| 21 | 2. **`--link ""` clears all links** (`:81-84`). | ||
| 22 | 3. **`''` clears any field.** An empty `--description` is "remove my description", which is different from omitting the flag entirely. | ||
| 23 | 4. **At least one flag is required.** With none, the command fails: *"nothing to set: pass --description, --website, --about and/or --link"* (`:356-358`). A save with no changes must not be sent. | ||
| 24 | 5. **A link is `label|url`**, split on the first `|`. Without a separator the whole value is the URL (`:106-113`). Labels are trimmed and truncated at 32 characters; a link with no URL is an error. **At most 5 links** (`maxProfileLinks = 5`, `:31,100-102`). | ||
| 25 | |||
| 26 | **About text goes over stdin.** It is long-form and contains newlines — the live profile on gitbay.org is org-mode with tables and links. Use `--file -` with the text in `stdin`, the same discipline the compose sheet already uses for issue and merge request bodies. Do not put it in argv. | ||
| 27 | |||
| 28 | **`--about-format md|org`** is stored alongside the text, so changing it reinterprets prose that already exists. The editor must default to the format the profile was **stored** with, read from `about_format` on `profile show`. | ||
| 29 | |||
| 30 | ## Global Constraints | ||
| 31 | |||
| 32 | - Swift 6 language mode, default `MainActor` isolation. Wire models are `nonisolated struct`s. | ||
| 33 | - Swift Testing only — never XCTest. | ||
| 34 | - `gitbayTests` is hermetic and offline; network goes through `StubProtocol`. | ||
| 35 | - New files under `gitbay/` and `gitbayTests/` need **no** `project.pbxproj` edit. | ||
| 36 | - The label model is `IssueLabel`; the notification model is `InboxNotification`. | ||
| 37 | - Never mention Claude, LLMs or AI in commits, comments, or the merge request. No `Co-Authored-By` trailer. | ||
| 38 | - Never commit to `main`. | ||
| 39 | |||
| 40 | **Counting tests — `xcresulttool`, not grep:** | ||
| 41 | |||
| 42 | ```bash | ||
| 43 | RES=$(ls -td ~/Library/Developer/Xcode/DerivedData/gitbay-*/Logs/Test/*.xcresult | head -1) | ||
| 44 | xcrun xcresulttool get test-results summary --path "$RES" | ||
| 45 | ``` | ||
| 46 | |||
| 47 | Baseline: **331 total, 330 passed, 1 skipped, 0 failed.** | ||
| 48 | |||
| 49 | --- | ||
| 50 | |||
| 51 | ## File Structure | ||
| 52 | |||
| 53 | | File | Responsibility | | ||
| 54 | |------|----------------| | ||
| 55 | | `gitbay/Discovery/ProfileEdit.swift` (create) | `ProfileEdit` — the form's value type and its argv rendering | | ||
| 56 | | `gitbay/Discovery/ProfileViewModel.swift` (modify) | `saveProfile(_:) async -> Bool` | | ||
| 57 | | `gitbayTests/ProfileEditTests.swift` (create) | Every test in this plan | | ||
| 58 | | `gitbay/Views/Discovery/ProfileEditSheet.swift` (create) | The edit form | | ||
| 59 | | `gitbay/Views/Discovery/ProfileView.swift` (modify) | Edit button on your own profile | | ||
| 60 | |||
| 61 | --- | ||
| 62 | |||
| 63 | ### Task 1: The value type and its argv | ||
| 64 | |||
| 65 | **Files:** Create `gitbay/Discovery/ProfileEdit.swift`; test in `gitbayTests/ProfileEditTests.swift` | ||
| 66 | |||
| 67 | This task is pure argv construction — the same shape as MR 2's `IssueFilter`, and the same reason: a flag this command does not accept is a usage error the app cannot catch at compile time. | ||
| 68 | |||
| 69 | **Interfaces produced:** | ||
| 70 | - `nonisolated struct ProfileEdit: Equatable, Sendable` — `description: String`, `website: String`, `about: String`, `aboutFormat: String`, `links: [Link]`; nested `Link` with `label: String`, `url: String` | ||
| 71 | - `init(from profile: ProfileViewModel.Profile)` — seeds the form from what the server reported | ||
| 72 | - `var isEmpty: Bool` — true when nothing would be sent | ||
| 73 | - `func flags() -> [String]` — everything except the about text | ||
| 74 | - `var aboutStdin: String?` — the about text, when it is being set | ||
| 75 | |||
| 76 | Rules, each pinned by a test: | ||
| 77 | 1. **`--link` sends the entire set**, one flag per link, `label|url`. A link with an empty label sends just the URL (no leading `|`). | ||
| 78 | 2. **An empty link list sends `--link ""`** — that is how the set is cleared. It must not simply omit the flag, which would leave the existing links untouched. | ||
| 79 | 3. **A field cleared to empty sends `--flag ""`**, not omission. Empty means "remove this"; omitted means "leave it alone". Since the form is always seeded from the current profile, every field it holds is being set. | ||
| 80 | 4. **The about text never appears in argv.** It goes to stdin via `--file -`. | ||
| 81 | 5. **`--about-format` is sent whenever the about text is**, using the stored format unless the user changed it. | ||
| 82 | 6. **More than 5 links is refused locally** before sending — the server caps at 5 and the form should not let it get there. | ||
| 83 | 7. **A link with no URL is refused locally.** | ||
| 84 | |||
| 85 | - [ ] **Step 1: Write the failing tests** | ||
| 86 | |||
| 87 | ```swift | ||
| 88 | import Foundation | ||
| 89 | import Testing | ||
| 90 | @testable import gitbay | ||
| 91 | |||
| 92 | struct ProfileEditFlagTests { | ||
| 93 | |||
| 94 | private func edit( | ||
| 95 | description: String = "", website: String = "", | ||
| 96 | about: String = "", aboutFormat: String = "md", | ||
| 97 | links: [ProfileEdit.Link] = [] | ||
| 98 | ) -> ProfileEdit { | ||
| 99 | ProfileEdit(description: description, website: website, | ||
| 100 | about: about, aboutFormat: aboutFormat, links: links) | ||
| 101 | } | ||
| 102 | |||
| 103 | @Test func everyFieldSendsItsFlag() { | ||
| 104 | let e = edit(description: "hi", website: "https://x.test", | ||
| 105 | links: [.init(label: "blog", url: "https://b.test")]) | ||
| 106 | let flags = e.flags() | ||
| 107 | #expect(flags.contains("--description")) | ||
| 108 | #expect(flags.contains("hi")) | ||
| 109 | #expect(flags.contains("--website")) | ||
| 110 | #expect(flags.contains("https://x.test")) | ||
| 111 | #expect(flags.contains("blog|https://b.test")) | ||
| 112 | } | ||
| 113 | |||
| 114 | /// An emptied field CLEARS it. Omitting the flag would leave the old | ||
| 115 | /// value in place, which is a different outcome. | ||
| 116 | @Test func anEmptiedFieldSendsAnEmptyValueNotNothing() { | ||
| 117 | let flags = edit(description: "", website: "https://x.test").flags() | ||
| 118 | let i = try! #require(flags.firstIndex(of: "--description")) | ||
| 119 | #expect(flags[i + 1] == "") | ||
| 120 | } | ||
| 121 | |||
| 122 | /// `--link ""` is how the whole set is cleared. Omitting it would | ||
| 123 | /// leave the existing links untouched. | ||
| 124 | @Test func noLinksSendsAnEmptyLinkFlag() { | ||
| 125 | let flags = edit(description: "hi").flags() | ||
| 126 | let i = try! #require(flags.firstIndex(of: "--link")) | ||
| 127 | #expect(flags[i + 1] == "") | ||
| 128 | } | ||
| 129 | |||
| 130 | /// The set replaces wholesale, so every kept link is sent every time. | ||
| 131 | @Test func everyLinkIsSentSoTheSetReplaces() { | ||
| 132 | let flags = edit(links: [ | ||
| 133 | .init(label: "a", url: "https://a.test"), | ||
| 134 | .init(label: "b", url: "https://b.test"), | ||
| 135 | ]).flags() | ||
| 136 | #expect(flags.filter { $0 == "--link" }.count == 2) | ||
| 137 | #expect(flags.contains("a|https://a.test")) | ||
| 138 | #expect(flags.contains("b|https://b.test")) | ||
| 139 | } | ||
| 140 | |||
| 141 | /// Without a separator the whole value is the URL, so a label-less | ||
| 142 | /// link must not send a leading pipe. | ||
| 143 | @Test func aLabellessLinkSendsJustTheUrl() { | ||
| 144 | let flags = edit(links: [.init(label: "", url: "https://a.test")]).flags() | ||
| 145 | #expect(flags.contains("https://a.test")) | ||
| 146 | #expect(flags.contains("|https://a.test") == false) | ||
| 147 | } | ||
| 148 | |||
| 149 | /// Long-form text with newlines belongs in stdin, not argv. | ||
| 150 | @Test func theAboutTextGoesToStdinNotArgv() { | ||
| 151 | let e = edit(about: "line one\n\nline two", aboutFormat: "org") | ||
| 152 | #expect(e.flags().contains { $0.contains("line one") } == false) | ||
| 153 | #expect(e.flags().contains("--file")) | ||
| 154 | #expect(e.flags().contains("-")) | ||
| 155 | #expect(e.aboutStdin == "line one\n\nline two") | ||
| 156 | } | ||
| 157 | |||
| 158 | @Test func theAboutFormatAccompaniesTheText() { | ||
| 159 | let flags = edit(about: "x", aboutFormat: "org").flags() | ||
| 160 | let i = try! #require(flags.firstIndex(of: "--about-format")) | ||
| 161 | #expect(flags[i + 1] == "org") | ||
| 162 | } | ||
| 163 | |||
| 164 | @Test func moreThanFiveLinksIsInvalid() { | ||
| 165 | let six = (1...6).map { ProfileEdit.Link(label: "l\($0)", url: "https://\($0).test") } | ||
| 166 | #expect(edit(links: six).validationError != nil) | ||
| 167 | let five = (1...5).map { ProfileEdit.Link(label: "l\($0)", url: "https://\($0).test") } | ||
| 168 | #expect(edit(links: five).validationError == nil) | ||
| 169 | } | ||
| 170 | |||
| 171 | @Test func aLinkWithoutAUrlIsInvalid() { | ||
| 172 | #expect(edit(links: [.init(label: "blog", url: " ")]).validationError != nil) | ||
| 173 | } | ||
| 174 | } | ||
| 175 | ``` | ||
| 176 | |||
| 177 | Add `var validationError: String?` to the interface list — the tests above use it. | ||
| 178 | |||
| 179 | - [ ] **Step 2: Run to verify failure.** | ||
| 180 | - [ ] **Step 3: Implement.** | ||
| 181 | - [ ] **Step 4: Run the tests.** | ||
| 182 | - [ ] **Step 5: Commit** — `git commit -m "Profile edit value type and its flags"` | ||
| 183 | |||
| 184 | --- | ||
| 185 | |||
| 186 | ### Task 2: Saving, and the edit sheet | ||
| 187 | |||
| 188 | **Files:** Modify `gitbay/Discovery/ProfileViewModel.swift`; create `gitbay/Views/Discovery/ProfileEditSheet.swift`; modify `gitbay/Views/Discovery/ProfileView.swift` | ||
| 189 | |||
| 190 | **Interfaces produced:** `ProfileViewModel.saveProfile(_ edit: ProfileEdit) async -> Bool` | ||
| 191 | |||
| 192 | `saveProfile` sends `["profile", "set"] + edit.flags()` with `edit.aboutStdin` as stdin, then reloads so the screen shows what the server stored. It must refuse to send an edit whose `validationError` is non-nil, and refuse to send when nothing would change. | ||
| 193 | |||
| 194 | **The sheet:** description and website as single-line fields; about as a `TextEditor` with a `md`/`org` picker; links as an editable list capped at five, each with a label and a URL. | ||
| 195 | |||
| 196 | **The format picker defaults to the profile's stored `about_format`.** Changing it reinterprets prose that already exists — the same rule that governs issue and merge request bodies. Do not default to markdown when the stored value is org; the live profile on gitbay.org is org, with tables and links that would render as flat text under the wrong renderer. | ||
| 197 | |||
| 198 | **The edit button appears only on your own profile** — `profile set` always writes the caller's own. The screen already computes this: MR 8 added an org-create button gated on `!profile.isOrg && session.current?.username == profile.name`. Reuse that condition rather than inventing a second one. | ||
| 199 | |||
| 200 | Add tests for `saveProfile` in `gitbayTests/ProfileEditTests.swift`: | ||
| 201 | - The full argv reaches the command, with the about text in stdin and not in argv. | ||
| 202 | - A save reloads, so the screen shows what the server stored rather than what was typed. | ||
| 203 | - A refusal surfaces and returns false. | ||
| 204 | - An invalid edit (six links) sends nothing at all. | ||
| 205 | |||
| 206 | **Provision stubs for every request each flow makes** — `saveProfile` writes *and* reloads. Three tests in this repo have already shipped under-provisioned, silently passing against bugs. | ||
| 207 | |||
| 208 | - [ ] **Step 1: `saveProfile` and its tests** | ||
| 209 | - [ ] **Step 2: The edit sheet** | ||
| 210 | - [ ] **Step 3: The edit button on your own profile** | ||
| 211 | - [ ] **Step 4: Build and run the full suite.** | ||
| 212 | - [ ] **Step 5: Commit** — `git commit -m "Edit your profile from the app"` | ||
| 213 | |||
| 214 | --- | ||
| 215 | |||
| 216 | ### Task 3: Flip the parity row and open the merge request | ||
| 217 | |||
| 218 | - [ ] **Step 1:** Set `profile set` to `yes` for iOS in the Accounts table. Touch no other row. | ||
| 219 | |||
| 220 | The wiki note for that row currently says no surface has ever offered a form, and that the `no` is missing UI rather than a rule — **update that prose too**, since it stops being true. The web still has no form, so say that iOS has one and the web does not. | ||
| 221 | |||
| 222 | Land it via a worktree off `origin/main` in `krz/gitbay`, merged `--strategy ff`. That repo requires signed commits, so squash and rebase merges are refused, and its working tree usually holds unrelated work — never switch its branch. **Read the merge request number back from `mr create`'s JSON.** | ||
| 223 | |||
| 224 | - [ ] **Step 2:** Run the full suite via `xcresulttool`; record the real numbers. | ||
| 225 | - [ ] **Step 3:** Open the merge request. | ||
| 226 | |||
| 227 | --- | ||
| 228 | |||
| 229 | ## Notes for whoever executes this | ||
| 230 | |||
| 231 | **`--link` replaces the set.** Every link you want to keep goes on every save, and `--link ""` clears them. This is the one flag semantic in the whole app that works this way, and getting it wrong silently loses or duplicates a user's links. | ||
| 232 | |||
| 233 | **Empty is not omitted.** `--description ""` removes the description; omitting `--description` leaves it. The form always sends what it holds. | ||
| 234 | |||
| 235 | **About text goes in stdin.** It is long-form with newlines. `--file -`, never argv. | ||
| 236 | |||
| 237 | **Default the format to what was stored.** The live profile is org-mode; defaulting to markdown would reinterpret prose that already exists. | ||