docs/superpowers/specs/2026-09-20-push-notifications-design.md

v1.13.1
gitbay-ios/docs/superpowers/specs/2026-09-20-push-notifications-design.md rendered · source · history · blame · raw

280 lines · 12720 bytes

  1# Push notifications
  2
  3Closes krz/gitbay#89 for the client. The app receives APNs alerts for
  4activity on every account signed in on the device, and a tap opens the
  5issue, merge request or build it names.
  6
  7The server half shipped in gitbay v1.32.0 and is running on gitbay.org.
  8Its design is `docs/specs/2026-09-20-ios-push-notifications-design.md`
  9in `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
 14on foreground, use background refresh. Not planned; propose if the app
 15makes the case." The server now delivers, so the app is what is left.
 16
 17Background refresh was never going to close it. `BGAppRefreshTask` runs
 18when the system feels like it, routinely fifteen minutes to hours after
 19the event. A failed build is exactly the notice that is worthless late.
 20
 21## Decision
 22
 23An `AppDelegate` adaptor receives the device token and notification
 24taps. A `PushRegistrar` registers that token against every signed-in
 25account. A `PushRouter`, owned above the account-keyed view tree,
 26carries a tapped notification's destination across the account switch
 27it may require. `ContentView` drains it and routes.
 28
 29Decisions 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
 63Two additions to `krz/gitbay`, landing before any Swift is written.
 64Both fold into the unreleased v1.32.1.
 65
 66**`notifications device add` returns the device id.** It currently
 67returns `{"status": "registered"}`, so an app that wants to deregister
 68must call `device list` and match its own token against the *truncated*
 69display value. That works — eight hex characters is four billion — but
 70it is identity by rendered string. `AddPushDevice` already returns the
 71id; the command just has to emit it.
 72
 73**The payload carries `aps.badge`.** `DuePush` gains a correlated
 74subquery counting the recipient's unread inbox rows, the same way it
 75now carries the username:
 76
 77```sql
 78(SELECT COUNT(*) FROM inbox WHERE user_id = d.user_id AND read_at IS NULL)
 79```
 80
 81Counting at send rather than at enqueue costs nothing extra and is
 82fresher: an inbox cleared in the seconds before delivery is reflected.
 83No migration.
 84
 85The payload's `instance` and `user` fields landed already (gitbay
 8666304b6) and are deployed.
 87
 88## Components
 89
 90Each 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
101parsing `InboxNotification.destination` already does — four
102slash-separated segments, the last an integer, mapping to `IssueRoute`,
103`MRRoute` or `BuildRoute`. That parsing moves to a free function both
104call rather than being duplicated; a repository root or an unknown kind
105yields no route, and such a tap opens the app without navigating.
106
107`PushRouter` is owned by `gitbayApp` beside `SessionStore`. That
108placement is the design: the `TabView` inside `ContentView` carries
109`.id(account.id)`, so a cross-account tap tears that subtree down and
110rebuilds it. A target held inside would go with it; held above, the
111rebuilt tree drains it.
112
113Note the `.id` is on the `TabView`, not on `ContentView` itself —
114`ContentView`'s own `@State` survives an account switch, which is why
115the Dashboard path has to be reset explicitly rather than by identity.
116
117## Registration
118
119`PushRegistrar` needs a client for accounts that are not active.
120`SessionStore` has the parts, but `store` and `makeClient` are both
121`private`, so it grows one method:
122
123```swift
124func client(for account: Account) -> GitbayClient?
125```
126
127It returns nil for an account with no stored token. The active-client
128model is untouched: this vends a transient client and does not change
129`current` or `client`.
130
131It registers on three events:
132
133- the APNs device token arriving, which is the only time the token is
134  known;
135- an account signing in, which adds one that has no row yet;
136- launch, when permission is already granted. Apple rotates device
137  tokens, so re-registering on launch is ordinary practice, and
138  `device add` upserts on token, so a repeat costs one call and
139  changes nothing.
140
141Each registration stores the returned device id against the account.
142
143Deregistration has an ordering constraint: `notifications device
144remove <id>` needs that account's token, which `SessionStore.remove`
145discards. There is exactly one sign-out call site today
146(`RepoListView.swift:283`), so a caller could do it first — but a
147future second call site would silently skip it, and the failure is
148invisible: the server keeps pushing for an account no longer signed
149in, and those taps resolve to nothing.
150
151So `SessionStore` gains a hook rather than the obligation:
152
153```swift
154var willRemoveAccount: ((Account) async -> Void)?
155```
156
157`remove` awaits it before discarding anything, which makes `remove`
158`async`; its one call site wraps the call in a `Task`. Awaiting rather
159than firing and forgetting is the point — `remove`'s first act is
160`store.deleteToken(for:)`, and a concurrent hook would race the
161credential it needs out from under itself.
162
163`gitbayApp` points the hook at the registrar's deregistration at
164construction, in the same place the two objects are already wired
165together. `SessionStore` stays ignorant of push — it knows only that
166something wants to run first.
167
168A deregistration that fails still removes the account. Blocking
169sign-out on a network call to the instance you are leaving would be
170the wrong trade; the stale device row is recoverable from the web
171settings page, and the account is gone either way.
172
173A registration that fails is not retried and not surfaced. Push is a
174side channel; the account still works, and the next launch registers
175again.
176
177## Routing
178
179A tap decodes the payload and resolves it against
180`SessionStore.accounts`, matching `instance` against `Account.instance`
181and `user` against `Account.username` — together exactly what
182`Account.id` is built from.
183
184- **Same account.** The subtree is live. Select the Dashboard tab,
185  append the route to its path, clear `pending`.
186- **Different account.** `session.activate()` first. That changes
187  `account.id` and rebuilds the `TabView`. `pending` survives, because
188  `PushRouter` sits above it. The new subtree drains it on appear.
189- **No matching account.** You signed out of it. Nothing sensible to
190  show, so the app opens without navigating. With deregistration
191  working this is transient rather than a standing condition.
192
193The drain is the mechanism, not a workaround: a rebuilt tree appearing
194with a target still pending is the signal to route, and the same code
195path serves both cases.
196
197Only the Dashboard tab gains a bound `NavigationPath`. It is first, and
198it is where your own work already lives. The other four stacks are
199unchanged. `TabView` gains a selection binding. That is the entire
200change to `ContentView`.
201
202A notification arriving while the app is in the foreground shows a
203banner with sound rather than being suppressed — you may be reading a
204different repository when one lands.
205
206## The notifications screen
207
208`NotificationsView` gains a section above the list with one **Push
209notifications** toggle, representing two independent states:
210
211| State | Toggle | Behaviour |
212|---|---|---|
213| Not yet asked | off | Turning on asks iOS; on approval registers the device and sets `notifications settings push on` |
214| Granted, server on | on | Turning off calls `notifications settings push off`; the OS permission is left alone |
215| Denied in iOS | off, disabled | One line saying notifications are off in Settings, and a button opening them |
216
217The denied row is what the explicit-toggle decision buys. Without it a
218single reflexive "Don't Allow" leaves push silent forever with nothing
219accounting for it.
220
221Below the toggle, this account's registered devices with a remove
222action — the same list the web settings page shows, tokens truncated
223the same way. Removal dispatches `notifications device remove`.
224
225There is no add-a-device control, on any surface. Only the app can mint
226an APNs token, and it does so itself.
227
228## Project and submission
229
230- The **Push Notifications** capability on `org.gitbay.gitbay`, already
231  enabled in the developer portal, needs its matching entitlement in
232  the Xcode project.
233- The **privacy nutrition label** does not gain the device token. It is
234  per-app, it rotates, it is not the advertising identifier, and it is
235  not used for tracking under any reading Apple's rules care about —
236  nothing is shared with a data broker and nothing is linked to
237  third-party data. Decided, not skipped; do not add it on a later pass
238  without raising it again. Note the token *is* stored against the
239  account (`push_devices.user_id`), so if it were ever declared it would
240  be "linked to the user", not unlinked.
241- An App Store resubmission. If Apple asks what push is for, it is
242  activity on repositories the account follows.
243
244`DESIGN.org`'s "No push notifications" gap row is rewritten to point at
245the feature. The Parity page in `krz/gitbay`'s wiki has three `no`s in
246its ios column for the device commands; they become `yes`.
247
248## Testing
249
250Unit tests in the repository's existing Swift Testing style:
251
252- a payload decodes to the right `PushPayload`, including a missing
253  `badge`;
254- a payload for a signed-in account resolves to that account's id and
255  the right route;
256- a payload for an account signed out resolves to nil;
257- a payload whose path is a repository root, or names an unknown kind,
258  resolves to an account but no route;
259- `PushRegistrar` against a stubbed store and client calls `device add`
260  once per account, and on sign-out calls `device remove` with the id
261  it stored for that account and no other.
262
263Not unit tested: the delegate callbacks and the navigation drain, both
264of which need a running app and a real APNs round trip. Those go in the
265live smoke suite (`smoke-account.sh` and the live suite steps), run
266against a device. The Simulator cannot receive a real push.
267
268## Scope
269
270Two stages, in order:
271
2721. `krz/gitbay`: the device id in `device add`'s response, and
273   `aps.badge` in the payload. One MR, folding into v1.32.1.
2742. `krz/gitbay-ios`: everything above.
275
276The implementation plan that follows this spec owns both stages: stage
2771 is its Task 1, in the other repository, and must merge before the
278registration and settings work can run against a real server. A
279prerequisite tracked outside the plan is a prerequisite that gets
280forgotten.