Commit a4707bc1b0
Verified · cmc
Layout: unified · split
docs/superpowers/specs/2026-09-20-push-notifications-design.md added +267
| @@ -0,0 +1,267 @@ | |||
| 1 | # Push notifications | ||
| 2 | |||
| 3 | Closes krz/gitbay#89 for the client. The app receives APNs alerts for | ||
| 4 | activity on every account signed in on the device, and a tap opens the | ||
| 5 | issue, merge request or build it names. | ||
| 6 | |||
| 7 | The server half shipped in gitbay v1.32.0 and is running on gitbay.org. | ||
| 8 | Its design is `docs/specs/2026-09-20-ios-push-notifications-design.md` | ||
| 9 | in `krz/gitbay`; this spec is the other half and does not restate it. | ||
| 10 | |||
| 11 | ## Problem | ||
| 12 | |||
| 13 | `DESIGN.org` records push as a known gap: "No push notifications — poll | ||
| 14 | on foreground, use background refresh. Not planned; propose if the app | ||
| 15 | makes the case." The server now delivers, so the app is what is left. | ||
| 16 | |||
| 17 | Background refresh was never going to close it. `BGAppRefreshTask` runs | ||
| 18 | when the system feels like it, routinely fifteen minutes to hours after | ||
| 19 | the event. A failed build is exactly the notice that is worthless late. | ||
| 20 | |||
| 21 | ## Decision | ||
| 22 | |||
| 23 | An `AppDelegate` adaptor receives the device token and notification | ||
| 24 | taps. A `PushRegistrar` registers that token against every signed-in | ||
| 25 | account. A `PushRouter`, owned above the account-keyed view tree, | ||
| 26 | carries a tapped notification's destination across the account switch | ||
| 27 | it may require. `ContentView` drains it and routes. | ||
| 28 | |||
| 29 | Decisions taken on the way, with the alternatives rejected: | ||
| 30 | |||
| 31 | - **Every signed-in account, not just the active one.** The server | ||
| 32 | keys a device row on (user, token), so one install can hold a row per | ||
| 33 | account. Registering only the active account would silently stop | ||
| 34 | notifications from the others the moment you switched — surprising | ||
| 35 | for the feature whose whole job is reaching you when the app is | ||
| 36 | closed. The cost is that the app must talk to accounts that are not | ||
| 37 | active, which it never does today. | ||
| 38 | - **A tap goes to the item, not to the inbox.** Opening the | ||
| 39 | notifications list would need no routing machinery and cost one more | ||
| 40 | tap. It is also not what a notification is for. | ||
| 41 | - **A cross-account tap switches accounts silently.** Honouring the tap | ||
| 42 | requires being in that account's context. A confirmation on every | ||
| 43 | such tap would wear thin, and the switch is visible anyway — the | ||
| 44 | tab bar resets. What is lost is unsaved state in the account being | ||
| 45 | left, in practice a scroll position or an unsent `ComposeSheet` | ||
| 46 | draft. | ||
| 47 | - **Permission is asked when you turn push on**, from an explicit | ||
| 48 | toggle, not on first sight of the notifications screen. iOS asks | ||
| 49 | once. A prompt sprung by mere navigation earns a reflexive "Don't | ||
| 50 | Allow", after which push is permanently silent with nothing in the | ||
| 51 | app explaining why. A toggle the user deliberately taps both raises | ||
| 52 | the grant rate and gives the denied state somewhere to live. | ||
| 53 | - **The badge comes from the server.** Badging from | ||
| 54 | `dashboard.unread` on launch would need no server change and be close | ||
| 55 | to useless: the count would refresh only when the app opens, reading | ||
| 56 | zero at exactly the moment it should be climbing. | ||
| 57 | - **No background modes.** These are alerts. Silent pushes, background | ||
| 58 | refresh and Live Activities are all out of scope and none of them is | ||
| 59 | needed to close the gap. | ||
| 60 | |||
| 61 | ## Server prerequisite | ||
| 62 | |||
| 63 | Two additions to `krz/gitbay`, landing before any Swift is written. | ||
| 64 | Both fold into the unreleased v1.32.1. | ||
| 65 | |||
| 66 | **`notifications device add` returns the device id.** It currently | ||
| 67 | returns `{"status": "registered"}`, so an app that wants to deregister | ||
| 68 | must call `device list` and match its own token against the *truncated* | ||
| 69 | display value. That works — eight hex characters is four billion — but | ||
| 70 | it is identity by rendered string. `AddPushDevice` already returns the | ||
| 71 | id; the command just has to emit it. | ||
| 72 | |||
| 73 | **The payload carries `aps.badge`.** `DuePush` gains a correlated | ||
| 74 | subquery counting the recipient's unread inbox rows, the same way it | ||
| 75 | now carries the username: | ||
| 76 | |||
| 77 | ```sql | ||
| 78 | (SELECT COUNT(*) FROM inbox WHERE user_id = d.user_id AND read_at IS NULL) | ||
| 79 | ``` | ||
| 80 | |||
| 81 | Counting at send rather than at enqueue costs nothing extra and is | ||
| 82 | fresher: an inbox cleared in the seconds before delivery is reflected. | ||
| 83 | No migration. | ||
| 84 | |||
| 85 | The payload's `instance` and `user` fields landed already (gitbay | ||
| 86 | 66304b6) and are deployed. | ||
| 87 | |||
| 88 | ## Components | ||
| 89 | |||
| 90 | Each is small, has one job, and is testable without a view tree. | ||
| 91 | |||
| 92 | | Unit | Does | Depends on | | ||
| 93 | |---|---|---| | ||
| 94 | | `PushPayload` | Decodes APNs `userInfo` into `instance`, `user`, `path`, `badge` | nothing | | ||
| 95 | | `PushTarget` | `(accountID, route)`; resolves a payload against known accounts | `SessionStore.accounts` | | ||
| 96 | | `PushRouter` | `@Observable`, holds `pending: PushTarget?` | nothing | | ||
| 97 | | `PushRegistrar` | Holds the APNs token; registers and deregisters per account | `SessionStore.client(for:)` | | ||
| 98 | | `AppDelegate` | Receives the token and taps; `UNUserNotificationCenterDelegate` | the four above | | ||
| 99 | |||
| 100 | `PushTarget` builds its route from the payload's `path` using the | ||
| 101 | parsing `InboxNotification.destination` already does — four | ||
| 102 | slash-separated segments, the last an integer, mapping to `IssueRoute`, | ||
| 103 | `MRRoute` or `BuildRoute`. That parsing moves to a free function both | ||
| 104 | call rather than being duplicated; a repository root or an unknown kind | ||
| 105 | yields no route, and such a tap opens the app without navigating. | ||
| 106 | |||
| 107 | `PushRouter` is owned by `gitbayApp` beside `SessionStore`. That | ||
| 108 | placement is the design: `ContentView` is `.id(account.id)`, so | ||
| 109 | anything held inside it is destroyed by the account switch a | ||
| 110 | cross-account tap performs. | ||
| 111 | |||
| 112 | ## Registration | ||
| 113 | |||
| 114 | `PushRegistrar` needs a client for accounts that are not active. | ||
| 115 | `SessionStore` has the parts, but `store` and `makeClient` are both | ||
| 116 | `private`, so it grows one method: | ||
| 117 | |||
| 118 | ```swift | ||
| 119 | func client(for account: Account) -> GitbayClient? | ||
| 120 | ``` | ||
| 121 | |||
| 122 | It returns nil for an account with no stored token. The active-client | ||
| 123 | model is untouched: this vends a transient client and does not change | ||
| 124 | `current` or `client`. | ||
| 125 | |||
| 126 | It registers on three events: | ||
| 127 | |||
| 128 | - the APNs device token arriving, which is the only time the token is | ||
| 129 | known; | ||
| 130 | - an account signing in, which adds one that has no row yet; | ||
| 131 | - launch, when permission is already granted. Apple rotates device | ||
| 132 | tokens, so re-registering on launch is ordinary practice, and | ||
| 133 | `device add` upserts on token, so a repeat costs one call and | ||
| 134 | changes nothing. | ||
| 135 | |||
| 136 | Each registration stores the returned device id against the account. | ||
| 137 | |||
| 138 | Deregistration has an ordering constraint: `notifications device | ||
| 139 | remove <id>` needs that account's token, which `SessionStore.remove` | ||
| 140 | discards. There is exactly one sign-out call site today | ||
| 141 | (`RepoListView.swift:283`), so a caller could do it first — but a | ||
| 142 | future second call site would silently skip it, and the failure is | ||
| 143 | invisible: the server keeps pushing for an account no longer signed | ||
| 144 | in, and those taps resolve to nothing. | ||
| 145 | |||
| 146 | So `SessionStore` gains a hook rather than the obligation: | ||
| 147 | |||
| 148 | ```swift | ||
| 149 | var willRemoveAccount: ((Account) async -> Void)? | ||
| 150 | ``` | ||
| 151 | |||
| 152 | `remove` awaits it before discarding anything, which makes `remove` | ||
| 153 | `async`; its one call site wraps the call in a `Task`. Awaiting rather | ||
| 154 | than firing and forgetting is the point — `remove`'s first act is | ||
| 155 | `store.deleteToken(for:)`, and a concurrent hook would race the | ||
| 156 | credential it needs out from under itself. | ||
| 157 | |||
| 158 | `gitbayApp` points the hook at the registrar's deregistration at | ||
| 159 | construction, in the same place the two objects are already wired | ||
| 160 | together. `SessionStore` stays ignorant of push — it knows only that | ||
| 161 | something wants to run first. | ||
| 162 | |||
| 163 | A deregistration that fails still removes the account. Blocking | ||
| 164 | sign-out on a network call to the instance you are leaving would be | ||
| 165 | the wrong trade; the stale device row is recoverable from the web | ||
| 166 | settings page, and the account is gone either way. | ||
| 167 | |||
| 168 | A registration that fails is not retried and not surfaced. Push is a | ||
| 169 | side channel; the account still works, and the next launch registers | ||
| 170 | again. | ||
| 171 | |||
| 172 | ## Routing | ||
| 173 | |||
| 174 | A tap decodes the payload and resolves it against | ||
| 175 | `SessionStore.accounts`, matching `instance` against `Account.instance` | ||
| 176 | and `user` against `Account.username` — together exactly what | ||
| 177 | `Account.id` is built from. | ||
| 178 | |||
| 179 | - **Same account.** The subtree is live. Select the Dashboard tab, | ||
| 180 | append the route to its path, clear `pending`. | ||
| 181 | - **Different account.** `session.activate()` first. That changes | ||
| 182 | `account.id` and rebuilds the `TabView`. `pending` survives, because | ||
| 183 | `PushRouter` sits above it. The new subtree drains it on appear. | ||
| 184 | - **No matching account.** You signed out of it. Nothing sensible to | ||
| 185 | show, so the app opens without navigating. With deregistration | ||
| 186 | working this is transient rather than a standing condition. | ||
| 187 | |||
| 188 | The drain is the mechanism, not a workaround: a rebuilt tree appearing | ||
| 189 | with a target still pending is the signal to route, and the same code | ||
| 190 | path serves both cases. | ||
| 191 | |||
| 192 | Only the Dashboard tab gains a bound `NavigationPath`. It is first, and | ||
| 193 | it is where your own work already lives. The other four stacks are | ||
| 194 | unchanged. `TabView` gains a selection binding. That is the entire | ||
| 195 | change to `ContentView`. | ||
| 196 | |||
| 197 | A notification arriving while the app is in the foreground shows a | ||
| 198 | banner with sound rather than being suppressed — you may be reading a | ||
| 199 | different repository when one lands. | ||
| 200 | |||
| 201 | ## The notifications screen | ||
| 202 | |||
| 203 | `NotificationsView` gains a section above the list with one **Push | ||
| 204 | notifications** toggle, representing two independent states: | ||
| 205 | |||
| 206 | | State | Toggle | Behaviour | | ||
| 207 | |---|---|---| | ||
| 208 | | Not yet asked | off | Turning on asks iOS; on approval registers the device and sets `notifications settings push on` | | ||
| 209 | | Granted, server on | on | Turning off calls `notifications settings push off`; the OS permission is left alone | | ||
| 210 | | Denied in iOS | off, disabled | One line saying notifications are off in Settings, and a button opening them | | ||
| 211 | |||
| 212 | The denied row is what the explicit-toggle decision buys. Without it a | ||
| 213 | single reflexive "Don't Allow" leaves push silent forever with nothing | ||
| 214 | accounting for it. | ||
| 215 | |||
| 216 | Below the toggle, this account's registered devices with a remove | ||
| 217 | action — the same list the web settings page shows, tokens truncated | ||
| 218 | the same way. Removal dispatches `notifications device remove`. | ||
| 219 | |||
| 220 | There is no add-a-device control, on any surface. Only the app can mint | ||
| 221 | an APNs token, and it does so itself. | ||
| 222 | |||
| 223 | ## Project and submission | ||
| 224 | |||
| 225 | - The **Push Notifications** capability on `org.gitbay.gitbay`, already | ||
| 226 | enabled in the developer portal, needs its matching entitlement in | ||
| 227 | the Xcode project. | ||
| 228 | - The **privacy nutrition label** gains the device token under | ||
| 229 | Identifiers: not linked to the user, not used for tracking. | ||
| 230 | - An App Store resubmission. If Apple asks what push is for, it is | ||
| 231 | activity on repositories the account follows. | ||
| 232 | |||
| 233 | `DESIGN.org`'s "No push notifications" gap row is rewritten to point at | ||
| 234 | the feature. The Parity page in `krz/gitbay`'s wiki has three `no`s in | ||
| 235 | its ios column for the device commands; they become `yes`. | ||
| 236 | |||
| 237 | ## Testing | ||
| 238 | |||
| 239 | Unit tests in the repository's existing Swift Testing style: | ||
| 240 | |||
| 241 | - a payload decodes to the right `PushPayload`, including a missing | ||
| 242 | `badge`; | ||
| 243 | - a payload for a signed-in account resolves to that account's id and | ||
| 244 | the right route; | ||
| 245 | - a payload for an account signed out resolves to nil; | ||
| 246 | - a payload whose path is a repository root, or names an unknown kind, | ||
| 247 | resolves to an account but no route; | ||
| 248 | - `PushRegistrar` against a stubbed store and client calls `device add` | ||
| 249 | once per account, and on sign-out calls `device remove` with the id | ||
| 250 | it stored for that account and no other. | ||
| 251 | |||
| 252 | Not unit tested: the delegate callbacks and the navigation drain, both | ||
| 253 | of which need a running app and a real APNs round trip. Those go in the | ||
| 254 | live smoke suite (`smoke-account.sh` and the live suite steps), run | ||
| 255 | against a device. The Simulator cannot receive a real push. | ||
| 256 | |||
| 257 | ## Scope | ||
| 258 | |||
| 259 | Two stages, in order: | ||
| 260 | |||
| 261 | 1. `krz/gitbay`: the device id in `device add`'s response, and | ||
| 262 | `aps.badge` in the payload. One MR, folding into v1.32.1. | ||
| 263 | 2. `krz/gitbay-ios`: everything above. | ||
| 264 | |||
| 265 | The implementation plan that follows this spec covers stage 2. Stage 1 | ||
| 266 | is small enough to land as an ordinary change against the shipped | ||
| 267 | server spec, which it amends. | ||