Commit 6c3141f419
Verified · cmc
Layout: unified · split
docs/superpowers/specs/2026-09-06-ios-parity-design.md added +313
| @@ -0,0 +1,313 @@ | |||
| 1 | # Closing the iOS parity gaps | ||
| 2 | |||
| 3 | Design for clearing every outstanding `no` in the iOS column of | ||
| 4 | `krz/gitbay`'s Parity wiki page, minus the rows that are `no` by design. | ||
| 5 | |||
| 6 | Instance: gitbay.org · Forge: krz/gitbay @ 6bcd070 · Wiki: krz/gitbay/wiki/Parity | ||
| 7 | |||
| 8 | ## The invariant | ||
| 9 | |||
| 10 | The app adds no capability the CLI does not have. Every screen below is a | ||
| 11 | rendering of a control command that already exists in the registry. Where a | ||
| 12 | capability is missing from the registry, the command is added on the server — | ||
| 13 | never special-cased here. Where a command refuses something, the app surfaces | ||
| 14 | the refusal. | ||
| 15 | |||
| 16 | ## Gap inventory | ||
| 17 | |||
| 18 | The iOS column carries twenty-four `no` rows. Nineteen are buildable, four are | ||
| 19 | `no` by design or should be `n/a`, and one is already done. | ||
| 20 | |||
| 21 | ### Already shipped — the wiki row is stale | ||
| 22 | |||
| 23 | `profile about and links`. `ProfileViewModel.Profile` decodes `about`, | ||
| 24 | `about_format` and `links`, and `ProfileView.aboutSection` renders the about | ||
| 25 | text through `ReadmeView` under `about.org` or `about.md` while the links | ||
| 26 | render as a list (commit `8d2421a`). The wiki's note that "the iOS `Profile` | ||
| 27 | decoder lists its keys explicitly, so it ignores `about` and `links`" describes | ||
| 28 | a state that no longer exists. | ||
| 29 | |||
| 30 | ### Not iOS work | ||
| 31 | |||
| 32 | | Row | Why | | ||
| 33 | |-----|-----| | ||
| 34 | | API token mint | SSH-only by design, per the wiki's own rule | | ||
| 35 | | delete, transfer | SSH-only by design; wants a typed confirmation | | ||
| 36 | | request a login link | The app authenticates with a pasted bearer token. A mailed one-time web link has nothing to unlock in a native client | | ||
| 37 | | account export | A JSON bundle has nowhere useful to land on a phone — the same argument the wiki already accepts for archive download | | ||
| 38 | |||
| 39 | The last two are proposed as `n/a` reclassifications, not deferrals. | ||
| 40 | |||
| 41 | `choose body markup` is *not* on this list. It is a genuine gap and MR 13 builds | ||
| 42 | it. The rule the wiki states — a capability lands over SSH first, then on the | ||
| 43 | web in the same merge request — governs SSH to web, not web to iOS, so the web | ||
| 44 | also being `no` does not order the two surfaces. The wiki says as much: both | ||
| 45 | `no`s are "the absence of a picker on the web form and in the iOS composer", | ||
| 46 | which is missing UI, not a rule. | ||
| 47 | |||
| 48 | ### Buildable | ||
| 49 | |||
| 50 | | Row | Command | MR | | ||
| 51 | |-----|---------|----| | ||
| 52 | | labels: list, colour | `label list`, `label set`, `label remove` | 1 | | ||
| 53 | | issue filter by label, assignee, author, milestone | `issue list --label --assignee --author --milestone` | 2 | | ||
| 54 | | issue search title and body | `issue list --search` | 2 | | ||
| 55 | | MR search title and body | `mr list --search` | 2 | | ||
| 56 | | search issues and merge requests | `search <q> [--kind repo\|issue\|mr]` | 3 | | ||
| 57 | | MR draft, ready | `mr draft`, `mr ready` | 4 | | ||
| 58 | | MR retarget | `mr retarget <repo> <n> <branch>` | 4 | | ||
| 59 | | stacked merge requests | `stacked_on` / `stacked` on `mr list`, `mr show` | 4 | | ||
| 60 | | compare two refs | `repo diff <repo> <base> <head>` | 5 | | ||
| 61 | | notification inbox | `notifications list`, `notifications read` | 6 | | ||
| 62 | | watch, mute | `repo watch`, `repo unwatch` | 7 | | ||
| 63 | | build cancel | `build cancel <repo> <n>` | 8 | | ||
| 64 | | dependency checks on/off | `repo deps enable`, `repo deps disable` | 9 | | ||
| 65 | | dependency status | `repo deps status` | 9 | | ||
| 66 | | release delete | `release delete <repo> <tag> --yes` | 10 | | ||
| 67 | | org create, rename | `org create`, `org rename` | 11 | | ||
| 68 | | profile set | `profile set` | 12 | | ||
| 69 | | choose body markup (MRs) | `--format md\|org` on `mr create`, `mr edit`, `mr comment` | 13 | | ||
| 70 | | choose body markup (issues) | `--format md\|org` on `issue create`, `issue edit`, `issue comment` | 13 | | ||
| 71 | |||
| 72 | ## What the wire already carries | ||
| 73 | |||
| 74 | Three rows need no server change and no new command, only decoding. The fields | ||
| 75 | exist and are `omitempty`, which is why they are absent from a sample response | ||
| 76 | rather than unimplemented: | ||
| 77 | |||
| 78 | - `draft` — `internal/control/mr.go:337`, on both `mr list` and `mr show` | ||
| 79 | - `stacked_on`, `stacked` — `internal/control/mr.go:349-350`, each a | ||
| 80 | `{number, title}` reference | ||
| 81 | - `color` — `internal/store/labels.go:8`, on `label list` rows | ||
| 82 | |||
| 83 | The decoding tests for these must cover the absent case, not just the present | ||
| 84 | one. A non-draft merge request decodes with the key missing entirely. | ||
| 85 | |||
| 86 | ## The one server gap | ||
| 87 | |||
| 88 | Watch state is not readable. `repo show` emits `default_branch`, `description`, | ||
| 89 | `mirrors`, `path`, `protected_branches`, `topics`, `visibility` and `website` — | ||
| 90 | no watch field. `store.RepoWatchState` (`internal/store/inbox.go:114`) returns | ||
| 91 | `watching`, `muted` or `""` but is internal to the server and reachable from no | ||
| 92 | command. | ||
| 93 | |||
| 94 | So MR 7 ships Watch and Mute as two stateless actions that report what the | ||
| 95 | command returned, and an issue is filed against `krz/gitbay` asking for the | ||
| 96 | state on `repo show`. When it lands, the two actions become one toggle. The | ||
| 97 | parity row closes on the stateless version, since the capability is present. | ||
| 98 | |||
| 99 | ## Components | ||
| 100 | |||
| 101 | ### Filter value types | ||
| 102 | |||
| 103 | `IssueListViewModel` and `MRListViewModel` are 43 and 44 lines. Each owns a | ||
| 104 | `StateFilter` enum and rebuilds `PagedListModel.argv` when it changes; | ||
| 105 | `PagedListModel.argv` is already mutable and documented as "set by the owning | ||
| 106 | view model when a filter changes". | ||
| 107 | |||
| 108 | Replace the bare enum with a struct that renders itself to flags: | ||
| 109 | |||
| 110 | ```swift | ||
| 111 | struct IssueFilter: Equatable { | ||
| 112 | var state: State = .open | ||
| 113 | var search: String = "" | ||
| 114 | var label: String? | ||
| 115 | var assignee: String? | ||
| 116 | var author: String? | ||
| 117 | var milestone: String? // a title, or "none" | ||
| 118 | |||
| 119 | /// `--state`, plus one flag per set field. Empty strings are omitted | ||
| 120 | /// rather than sent, since `--search ""` is not the same as no search. | ||
| 121 | var flags: [String] | ||
| 122 | } | ||
| 123 | ``` | ||
| 124 | |||
| 125 | `MRFilter` is the same shape minus `label` and `assignee`, which `mr list` does | ||
| 126 | not accept, plus `source_gone` in its state set. Both view models keep their | ||
| 127 | existing didSet-reload behaviour; only the type widens. | ||
| 128 | |||
| 129 | This one change carries four parity rows. It is the highest value per line in | ||
| 130 | the plan. | ||
| 131 | |||
| 132 | ### Label store | ||
| 133 | |||
| 134 | Labels render today as `GBChip(label, .secondary)` (`IssueListView.swift:89`) — | ||
| 135 | one flat colour for every label, in the list only. `label list` returns | ||
| 136 | `{name, color?, issues}` per label. | ||
| 137 | |||
| 138 | A repo-scoped store reads it once and feeds three consumers: chips in the issue | ||
| 139 | list, chips in the issue detail, and the label picker in `IssueFilter`. A label | ||
| 140 | with no stored colour derives one from its name, matching what the web does, so | ||
| 141 | a repository that has never set a colour still reads as coloured rather than | ||
| 142 | uniformly grey. The derivation must produce the same hue the web produces for | ||
| 143 | the same name — the two surfaces show the same label side by side. | ||
| 144 | |||
| 145 | The management screen (`RepoRoute.labels`) lists labels with their use counts | ||
| 146 | and dispatches `label set` to create or recolour and `label remove` to delete. | ||
| 147 | `label remove` takes the label off every issue, so it is destructive and gated. | ||
| 148 | |||
| 149 | ### Splitting DiffView | ||
| 150 | |||
| 151 | `DiffView.init(client:repo:number:)` (`DiffView.swift:9`) loads `mr diff`, | ||
| 152 | parses it with `UnifiedDiff.parse`, and renders it. Compare needs the same | ||
| 153 | renderer over a different source: `repo diff` returns | ||
| 154 | `{base, head, merge_base, patch, truncated}`, where `patch` is the same | ||
| 155 | unified-diff text. | ||
| 156 | |||
| 157 | Split it in two: a view over `[UnifiedDiff.File]` that does the rendering, and | ||
| 158 | two loaders that produce that array — one from `mr diff`, one from `repo diff`. | ||
| 159 | `DiffFileSection`, `HunkView` and `LineView` are already parameterised on | ||
| 160 | `UnifiedDiff` types and do not change. | ||
| 161 | |||
| 162 | This is the only existing code the plan modifies rather than extends. The | ||
| 163 | boundary is wrong for a second caller, and duplicating a diff renderer to avoid | ||
| 164 | touching it would be worse. | ||
| 165 | |||
| 166 | `truncated` must be surfaced. A compare across a wide range returns a partial | ||
| 167 | patch, and a renderer that silently shows less than the whole diff is a bug the | ||
| 168 | user cannot see. | ||
| 169 | |||
| 170 | ### Routes | ||
| 171 | |||
| 172 | New `RepoRoute` cases: `labels(repo:)`, `compare(repo:base:head:)`, and | ||
| 173 | `notifications`. Dependency status renders inside the existing | ||
| 174 | `RepoSettingsView` rather than taking a route, matching where the web puts it. | ||
| 175 | |||
| 176 | ### Write path | ||
| 177 | |||
| 178 | `AccountViewModel.perform(argv:stdin:)` and | ||
| 179 | `RepoSettingsViewModel.perform(argv:)` are already near-duplicates of each | ||
| 180 | other, eight lines each, setting `working`, running the command, and capturing | ||
| 181 | `actionError`. New view models copy the pattern. A third and fourth copy is | ||
| 182 | cheaper than a shared base class for eight lines, and the existing duplication | ||
| 183 | is not this plan's to fix. | ||
| 184 | |||
| 185 | ## Merge request sequence | ||
| 186 | |||
| 187 | Ordering is driven by shared machinery, not by the wiki's section order. | ||
| 188 | |||
| 189 | **1. Labels.** Label store, colour on every chip, management screen. First | ||
| 190 | because MR 2's label filter picks from real labels with real colours. | ||
| 191 | |||
| 192 | **2. List filters and search.** `IssueFilter` and `MRFilter`; a search field and | ||
| 193 | filter controls on the issue and merge request lists. Closes three rows. | ||
| 194 | |||
| 195 | **3. Instance-wide search.** The Explore tab searches repositories today through | ||
| 196 | `repo search` (`RepoListViewModel.swift:63`). Widen it to `search`, which covers | ||
| 197 | repositories plus the title and body of every issue and merge request the caller | ||
| 198 | can read, with a kind control for `repo | issue | mr`. Results carry | ||
| 199 | `{kind, repo, number, title, author, state, updated_at}` and route to the | ||
| 200 | existing repo, issue and merge request destinations. What the user types is | ||
| 201 | quoted term by term server-side, so an FTS5 operator is a word to match — the | ||
| 202 | app passes the query through unescaped. | ||
| 203 | |||
| 204 | **4. Draft, ready, retarget, stack.** Decode `draft`, `stacked_on` and | ||
| 205 | `stacked`. A draft badge on list rows and the detail header. Draft and Ready | ||
| 206 | actions. Retarget picks a branch from the repo's refs and states that the | ||
| 207 | reviews go stale, because an approval was of the diff against the old branch. A | ||
| 208 | stack section links both directions — what this is stacked on, and what is | ||
| 209 | stacked on it. A squash or rebase merge is refused while anything is stacked on | ||
| 210 | the merge request, so the merge sheet hides those strategies rather than letting | ||
| 211 | the user pick one the server will reject. `mr create` also takes `--draft`, so | ||
| 212 | the create form gains the checkbox in the same merge request. Closes three rows. | ||
| 213 | |||
| 214 | **5. Compare two refs.** The `DiffView` split, then a compare screen reachable | ||
| 215 | from the Refs screen with two ref pickers. Renders `merge_base` and handles | ||
| 216 | `truncated`. | ||
| 217 | |||
| 218 | **6. Notification inbox.** `notifications list` pages on `PagedListModel` with | ||
| 219 | the same cursors as every other list; `--all` toggles between unread and | ||
| 220 | everything. Items carry `{id, repo, kind, actor, summary, path, created_at, | ||
| 221 | read_at?}`; `path` routes to the thread. `notifications read <id>...` and | ||
| 222 | `--all` clear. The unread count sits on the Dashboard, where the web puts it in | ||
| 223 | its rail. | ||
| 224 | |||
| 225 | **7. Watch and mute.** Two actions in repo settings. Stateless, per the server | ||
| 226 | gap above. | ||
| 227 | |||
| 228 | **8. Build cancel.** An action on build detail, offered for queued and running | ||
| 229 | builds, confirmed before firing. Cancelling withdraws a queued build or ends a | ||
| 230 | running one within a couple of seconds; cancelling a duplicate of a commit that | ||
| 231 | already passed puts that result back on the commit. | ||
| 232 | |||
| 233 | **9. Dependency checks.** A toggle in repo settings dispatching `repo deps | ||
| 234 | enable` and `repo deps disable`, with the status report under it: `enabled`, | ||
| 235 | `last_check`, `last_error`, a link to the tracking issue `issue_number`, and the | ||
| 236 | `behind` rows of `{ecosystem, name, current, latest}`. Checks are off until a | ||
| 237 | repository's admin turns them on. Closes two rows. | ||
| 238 | |||
| 239 | **10. Release delete.** A destructive action on release detail. `release delete` | ||
| 240 | requires `--yes`, so the app confirms before sending it, and the confirmation | ||
| 241 | says the assets go too. | ||
| 242 | |||
| 243 | **11. Org create and rename.** Create from the org list. Rename in org settings, | ||
| 244 | stating that clone URLs change. Note `org delete` is `n/a` on iOS, and stays so. | ||
| 245 | |||
| 246 | **12. Profile set.** An edit form on your own profile: description, website, | ||
| 247 | about text with a `md | org` format picker, and up to five labelled links. The | ||
| 248 | server caps the list at five and `''` clears a field. `profile set` is not | ||
| 249 | `SSHOnly` — nothing about a bio is a credential, and the JSON API runs it — so | ||
| 250 | the `no` on every surface is missing UI rather than a rule. | ||
| 251 | |||
| 252 | **13. Body markup picker.** `--format md|org` on `issue create`, `issue edit`, | ||
| 253 | `issue comment`, `mr create`, `mr edit` and `mr comment`. Nothing in the compose | ||
| 254 | path mentions a format today: `ComposeSheet` is a title-and-body form shared by | ||
| 255 | the issue and merge request create and edit sheets, and comments compose | ||
| 256 | separately in `IssueView` and `MRView`. So the picker lands in two places, not | ||
| 257 | one — an optional format binding on `ComposeSheet` covers create and edit for | ||
| 258 | both nouns, and each comment field gains its own. The stored format is | ||
| 259 | what every surface renders from, so the picker's default matters: editing an | ||
| 260 | existing body defaults to the format that body was stored with, read from | ||
| 261 | `body_format` on `issue show` and `mr show`, and changing it reinterprets prose | ||
| 262 | that already exists. New bodies default to markdown, as they do now. Diff-line | ||
| 263 | comments have no format column and are always markdown, so `mr diff-comment` is | ||
| 264 | untouched. Closes two rows. | ||
| 265 | |||
| 266 | `release create` and `release edit` take `--format` too, but the wiki has no | ||
| 267 | parity row for release markup, so that is left alone rather than widened | ||
| 268 | silently. | ||
| 269 | |||
| 270 | Nineteen rows, thirteen merge requests. Each flips its own parity rows in the | ||
| 271 | same merge request, which is what the wiki page requires. | ||
| 272 | |||
| 273 | ## Work in krz/gitbay | ||
| 274 | |||
| 275 | One merge request, landing before the iOS work: | ||
| 276 | |||
| 277 | - Flip `profile about and links` to `yes` on iOS. | ||
| 278 | - Reclassify `request a login link` and `account export` from `no` to `n/a` in | ||
| 279 | the iOS column, each with its reason. `API token mint` and `delete, transfer` | ||
| 280 | are already covered by the SSH-only section and need no row change. | ||
| 281 | |||
| 282 | One issue: | ||
| 283 | |||
| 284 | - `repo show` should carry watch state, so a client can render a toggle rather | ||
| 285 | than two buttons. `store.RepoWatchState` already computes it. | ||
| 286 | |||
| 287 | ## Testing | ||
| 288 | |||
| 289 | `gitbayTests` is hermetic and runs offline; network calls go through a stubbed | ||
| 290 | `URLProtocol`. Every merge request adds view-model tests there. Two kinds carry | ||
| 291 | the weight: | ||
| 292 | |||
| 293 | **Argv shape.** A filter that renders a flag `issue list` does not accept is a | ||
| 294 | server error the app cannot catch at compile time, and the failure surfaces as | ||
| 295 | an empty list rather than a crash. Each filter combination asserts its exact | ||
| 296 | argv. | ||
| 297 | |||
| 298 | **Absent-field decoding.** For `draft`, `stacked_on`, `stacked` and `color`, the | ||
| 299 | test that matters is the one where the key is missing, since that is the common | ||
| 300 | case on the wire and the case a fixture written by hand tends to omit. | ||
| 301 | |||
| 302 | The live smoke suite in `gitbayUITests` stays opt-in and is not extended by this | ||
| 303 | plan. | ||
| 304 | |||
| 305 | ## Stopping early | ||
| 306 | |||
| 307 | The order is chosen so that stopping is cheap. Merge requests 1 through 6 clear | ||
| 308 | the whole triage, review and respond loop — the app's stated brief — and 13 | ||
| 309 | belongs to it too, but sits last because it touches the composer every other | ||
| 310 | merge request also touches and is cheaper once they have landed. 7 through 12 | ||
| 311 | are repository and account administration, which is the part of the surface a | ||
| 312 | phone is least likely to be the right tool for. If the plan is cut, it is cut | ||
| 313 | after 6, with 13 pulled forward. | ||