docs/superpowers/plans/2026-09-06-mr06-notifications.md
345 lines · 15964 bytes
1# MR 6: Notification inbox 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 `notification inbox` parity row — read the inbox, mark things read, and carry the unread count on the Dashboard.
6
7**Architecture:** `notifications list` is paginated, so it goes through the existing `PagedListModel` like every other list. `notifications read` marks one or all. The unread badge costs no extra request: `dashboard` already emits `unread` for exactly this purpose and iOS simply does not decode it yet.
8
9**Spec:** `docs/superpowers/specs/2026-09-06-ios-parity-design.md`
10
11## Global Constraints
12
13- Swift 6 language mode, default `MainActor` isolation. Wire models are `nonisolated struct`s.
14- Swift Testing only — never XCTest.
15- `gitbayTests` is hermetic and offline; network goes through `StubProtocol`.
16- New files under `gitbay/` and `gitbayTests/` need **no** `project.pbxproj` edit.
17- The label model is `IssueLabel`, never `Label`.
18- Never mention Claude, LLMs or AI in commits, comments, or the merge request. No `Co-Authored-By` trailer.
19- Never commit to `main`.
20
21**The commands, verbatim from the registry:**
22
23```
24notifications list [--all] [--limit <n>] [--cursor <c>]
25notifications read <id>... | --all
26```
27
28`notifications list` **is paginated** — unlike `search` in MR 3 — so it uses `PagedListModel`, which appends `--limit`/`--cursor` itself. The default listing is **unread only**; `--all` includes read ones.
29
30**The item shape** (`internal/control/notifications.go:117-125`):
31
32```json
33{"id":15,"repo":"krz/gitbay","kind":"build","actor":"ci",
34 "summary":"build 966 failed: test on runner-podman-exec-144",
35 "path":"krz/gitbay/builds/966","created_at":"2026-09-06T22:14:46.912Z",
36 "read_at":"2026-09-06T22:20:24.040Z"}
37```
38
39Everything is always present except `read_at`, which is `omitempty` — **absent means unread**, and that is the common case for the default listing.
40
41**Counting tests reliably:**
42
43```bash
44grep -cE "^Test case '[^']*' passed" /tmp/out.txt
45grep -cE "^Test case '[^']*' failed" /tmp/out.txt
46grep -cE "^Test case '[^']*' skipped" /tmp/out.txt
47grep -rhoE "@Test(\([^)]*\))? func" gitbayTests/*.swift | wc -l
48```
49
50`declared - skipped == passed`. One test is always skipped (`LiveInstanceTests`).
51
52Baseline: **285 declared, 284 passed, 1 skipped, 0 failed.**
53
54---
55
56## File Structure
57
58| File | Responsibility |
59|------|----------------|
60| `gitbay/Account/NotificationsViewModel.swift` (create) | `Notification` wire model, routing, and the paged view model |
61| `gitbayTests/NotificationTests.swift` (create) | Every test in this plan |
62| `gitbay/Views/Account/NotificationsView.swift` (create) | The inbox screen |
63| `gitbay/Dashboard/DashboardModels.swift` (modify) | Decode `unread` |
64| `gitbay/Views/Dashboard/DashboardView.swift` (modify) | Badge + entry point |
65| `gitbay/Views/Repos/RepoRoute.swift` (modify) | `notifications` case |
66| `gitbay/ContentView.swift` (modify) | Route to `NotificationsView` |
67
68---
69
70### Task 1: The model, its routing, and the view model
71
72**Files:** Create `gitbay/Account/NotificationsViewModel.swift`; test in `gitbayTests/NotificationTests.swift`
73
74**Interfaces produced:**
75- `nonisolated struct Notification: Decodable, Sendable, Hashable, Identifiable` — `id: Int64`, `repo: String`, `kind: String`, `actor: String`, `summary: String`, `path: String`, `createdAt: Date`, `readAt: Date?`; `var isUnread: Bool`; `var destination: NotificationDestination?`
76- `nonisolated enum NotificationDestination: Hashable, Sendable` — `.issue(repo:number:)`, `.mr(repo:number:)`, `.build(repo:number:)`
77- `@Observable @MainActor final class NotificationsViewModel` — `list: PagedListModel<Notification>`, `showAll: Bool`, `state`, `actionError`, `working`, `load()`, `markRead(_:)`, `markAllRead()`
78
79**The routing table**, derived from the server (`internal/control/issue.go:153,279,349`, `mr.go:322,702,778,832,1173,1198,1382`, `diffcomment.go:111`, `build.go:522`, `deps/worker.go:249`) and confirmed against live data. `path` is a **web-style route**, `<owner>/<repo>/<section>/<n>`:
80
81| `kind` | `path` | destination |
82|---------|------------------------------|-------------|
83| `issue` | `<owner>/<repo>/issues/<n>` | `.issue` |
84| `mr` | `<owner>/<repo>/mrs/<n>` | `.mr` |
85| `build` | `<owner>/<repo>/builds/<n>` | `.build` |
86
87**The merge request segment is `mrs`, not `merge_requests`.** Matching the wrong spelling produces a silently untappable row — no error, no crash, just a row that does nothing.
88
89Parse defensively: split on `/`, require exactly four components, require the last to be an integer, and return `nil` for anything else. A `nil` destination renders an untappable row rather than crashing — the same rule the search results screen follows for a missing number.
90
91- [ ] **Step 1: Write the failing tests**
92
93Create `gitbayTests/NotificationTests.swift`:
94
95```swift
96import Foundation
97import Testing
98@testable import gitbay
99
100private func makeClient() throws -> (GitbayClient, StubProtocol.Box) {
101 let box = StubProtocol.box()
102 let client = GitbayClient(
103 instance: try GitbayInstance(url: "https://gitbay.org"),
104 token: "test-token",
105 session: box.session()
106 )
107 return (client, box)
108}
109
110private func argvFrom(_ url: URL) -> [String] {
111 URLComponents(url: url, resolvingAgainstBaseURL: false)?
112 .queryItems?.filter { $0.name == "argv" }.compactMap(\.value) ?? []
113}
114
115private func argvOf(_ seen: StubProtocol.Seen) throws -> [String] {
116 let body = try #require(try JSONSerialization.jsonObject(with: seen.body) as? [String: Any])
117 return try #require(body["argv"] as? [String])
118}
119
120private func decode(_ json: String) throws -> Notification {
121 let decoder = JSONDecoder()
122 decoder.dateDecodingStrategy = .iso8601
123 return try decoder.decode(Notification.self, from: Data(json.utf8))
124}
125
126private let unreadBuild = """
127 {"id":15,"repo":"krz/gitbay","kind":"build","actor":"ci",\
128 "summary":"build 966 failed","path":"krz/gitbay/builds/966",\
129 "created_at":"2026-09-06T22:14:46Z"}
130 """
131
132struct NotificationDecodingTests {
133
134 /// read_at is omitempty; absent means unread, and that is the common
135 /// case for the default listing.
136 @Test func anAbsentReadAtMeansUnread() throws {
137 let n = try decode(unreadBuild)
138 #expect(n.readAt == nil)
139 #expect(n.isUnread)
140 }
141
142 @Test func aPresentReadAtMeansRead() throws {
143 let n = try decode("""
144 {"id":15,"repo":"krz/gitbay","kind":"build","actor":"ci",\
145 "summary":"x","path":"krz/gitbay/builds/966",\
146 "created_at":"2026-09-06T22:14:46Z","read_at":"2026-09-06T22:20:24Z"}
147 """)
148 #expect(n.readAt != nil)
149 #expect(n.isUnread == false)
150 }
151
152 @Test func everyKindRoutesToItsScreen() throws {
153 let issue = try decode("""
154 {"id":1,"repo":"krz/gitbay","kind":"issue","actor":"cmc","summary":"x",\
155 "path":"krz/gitbay/issues/168","created_at":"2026-09-06T22:14:46Z"}
156 """)
157 #expect(issue.destination == .issue(repo: "krz/gitbay", number: 168))
158
159 // The segment is "mrs", not "merge_requests".
160 let mr = try decode("""
161 {"id":2,"repo":"krz/gitbay","kind":"mr","actor":"cmc","summary":"x",\
162 "path":"krz/gitbay/mrs/282","created_at":"2026-09-06T22:14:46Z"}
163 """)
164 #expect(mr.destination == .mr(repo: "krz/gitbay", number: 282))
165
166 let build = try decode(unreadBuild)
167 #expect(build.destination == .build(repo: "krz/gitbay", number: 966))
168 }
169
170 /// A path the app cannot route must yield nil, not a crash and not a
171 /// wrong destination.
172 @Test func anUnroutablePathIsNil() throws {
173 for path in ["krz/gitbay/wiki/Home", "krz/gitbay/issues", "krz/gitbay/issues/notanumber",
174 "too/short", "krz/gitbay/issues/1/extra", ""] {
175 let n = try decode("""
176 {"id":1,"repo":"krz/gitbay","kind":"issue","actor":"cmc","summary":"x",\
177 "path":"\(path)","created_at":"2026-09-06T22:14:46Z"}
178 """)
179 #expect(n.destination == nil, "\(path) should not route")
180 }
181 }
182
183 /// The repo comes from the path, not the `repo` field, so a
184 /// mismatch cannot send the user to the wrong repository.
185 @Test func theDestinationRepoComesFromThePath() throws {
186 let n = try decode("""
187 {"id":1,"repo":"other/repo","kind":"issue","actor":"cmc","summary":"x",\
188 "path":"krz/gitbay/issues/7","created_at":"2026-09-06T22:14:46Z"}
189 """)
190 #expect(n.destination == .issue(repo: "krz/gitbay", number: 7))
191 }
192}
193
194@MainActor
195struct NotificationsViewModelTests {
196
197 private let page = """
198 {"protocol_version":1,"data":{"items":[\(unreadBuild)]},"exit_code":0}
199 """
200
201 @Test func theDefaultListingIsUnreadOnly() async throws {
202 let (client, stub) = try makeClient()
203 stub.enqueue(.init(status: 200, json: page))
204 let model = NotificationsViewModel(client: client)
205 await model.load()
206
207 let argv = argvFrom(try #require(stub.seen.last).url)
208 #expect(argv.prefix(2) == ["notifications", "list"])
209 #expect(argv.contains("--all") == false)
210 #expect(model.state.value?.count == 1)
211 }
212
213 @Test func showingAllAddsTheFlag() async throws {
214 let (client, stub) = try makeClient()
215 stub.enqueue(.init(status: 200, json: page))
216 let model = NotificationsViewModel(client: client)
217 await model.load()
218
219 stub.enqueue(.init(status: 200, json: page))
220 model.showAll = true
221 try await Task.sleep(for: .milliseconds(150))
222
223 #expect(argvFrom(try #require(stub.seen.last).url).contains("--all"))
224 }
225
226 @Test func markingOneReadSendsItsId() async throws {
227 let (client, stub) = try makeClient()
228 stub.enqueue(.init(status: 200, json: page))
229 let model = NotificationsViewModel(client: client)
230 await model.load()
231
232 stub.enqueue(.init(status: 200, json: """
233 {"protocol_version":1,"exit_code":0}
234 """))
235 stub.enqueue(.init(status: 200, json: page))
236 await model.markRead(15)
237
238 let write = try #require(stub.seen.first { $0.method == "POST" })
239 #expect(try argvOf(write) == ["notifications", "read", "15"])
240 }
241
242 @Test func markingAllReadUsesTheFlagNotAList() async throws {
243 let (client, stub) = try makeClient()
244 stub.enqueue(.init(status: 200, json: page))
245 let model = NotificationsViewModel(client: client)
246 await model.load()
247
248 stub.enqueue(.init(status: 200, json: """
249 {"protocol_version":1,"exit_code":0}
250 """))
251 stub.enqueue(.init(status: 200, json: page))
252 await model.markAllRead()
253
254 let write = try #require(stub.seen.first { $0.method == "POST" })
255 #expect(try argvOf(write) == ["notifications", "read", "--all"])
256 }
257
258 @Test func anEmptyInboxIsAnEmptyStateNotAFailure() async throws {
259 let (client, stub) = try makeClient()
260 stub.enqueue(.init(status: 200, json: """
261 {"protocol_version":1,"data":{"items":[]},"exit_code":0}
262 """))
263 let model = NotificationsViewModel(client: client)
264 await model.load()
265
266 guard case .empty = model.state else {
267 Testing.Issue.record("expected empty, got \(model.state)")
268 return
269 }
270 }
271
272 @Test func aRefusalSurfacesAndDoesNotReload() async throws {
273 let (client, stub) = try makeClient()
274 stub.enqueue(.init(status: 200, json: page))
275 let model = NotificationsViewModel(client: client)
276 await model.load()
277 let before = stub.seen.count
278
279 stub.enqueue(.init(status: 200, json: """
280 {"protocol_version":1,"error":"denied","exit_code":4}
281 """))
282 await model.markRead(15)
283
284 #expect(model.actionError?.isEmpty == false)
285 #expect(model.working == false)
286 // One POST, no reload after a refusal.
287 #expect(stub.seen.count == before + 1)
288 }
289}
290```
291
292- [ ] **Step 2: Run to verify failure.**
293- [ ] **Step 3: Implement.** Follow `IssueListViewModel` for the `PagedListModel` shape and `RepoSettingsViewModel.perform(argv:)` for the write half (copy it; do not extract a shared helper — that duplication is a standing decision in this codebase).
294- [ ] **Step 4: Run the tests.**
295- [ ] **Step 5: Commit** — `git commit -m "Notification inbox model and view model"`
296
297---
298
299### Task 2: The screen, the badge and the route
300
301**Files:** Create `gitbay/Views/Account/NotificationsView.swift`; modify `DashboardModels.swift`, `DashboardView.swift`, `RepoRoute.swift`, `ContentView.swift`
302
303**The badge is free.** `dashboard` already emits `unread: int` — the server comments it as existing "so a client showing one does not need a second read to fill it" (`internal/control/dashboard.go:78`). iOS does not decode it yet.
304
305**`DashboardData` has a hand-written `init(from:)`** (`DashboardModels.swift:27-37`) that decodes each field with `decodeIfPresent ?? []`. Adding `unread` means editing that initialiser — check field by field that you drop nothing, the same care MR 4's `MRDetail` initialiser needed. Decode it as `decodeIfPresent(Int.self, forKey: .unread) ?? 0`, since a server predating the field would omit it.
306
307**The screen:** a `List` over the paged notifications with `PageFooter`, a toggle for unread-only versus all, a "Mark all read" action, and per-row swipe-to-mark-read. An unread row should read as unread — the web uses the count, so weight or a dot is enough; do not invent a colour that means nothing elsewhere in the app.
308
309Each row shows `actor`, `summary` and the relative `createdAt`, and navigates to `destination` when it is non-nil. A row whose destination is `nil` renders plainly and is not tappable.
310
311**Entry point:** a toolbar bell on the Dashboard carrying the unread count, matching where the web puts it (its rail). `DashboardView` already has a `.toolbar` with a `.topBarTrailing` item — add beside it.
312
313Add `RepoRoute.notifications` and its `ContentView` destination.
314
315- [ ] **Step 1: Decode `unread` in `DashboardData`**
316- [ ] **Step 2: `NotificationsView`**
317- [ ] **Step 3: Route case + `ContentView` destination**
318- [ ] **Step 4: Dashboard bell with the count**
319- [ ] **Step 5: Build and run the full suite.** No drop.
320- [ ] **Step 6: Commit** — `git commit -m "Notification inbox screen and dashboard badge"`
321
322---
323
324### Task 3: Flip the parity row and open the merge request
325
326- [ ] **Step 1:** In the Accounts table, `notification inbox` goes to `yes` for iOS. Touch no other row.
327
328Land 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.
329
330- [ ] **Step 2:** Run the full suite; record the real number.
331- [ ] **Step 3:** Open the merge request.
332
333---
334
335## Notes for whoever executes this
336
337**`notifications list` IS paginated.** Use `PagedListModel`. This differs from `search` in MR 3, which is not.
338
339**The merge request path segment is `mrs`.** Not `merge_requests`. The wrong spelling gives a silently untappable row.
340
341**Absent `read_at` means unread**, and it is the common case. Do not require the key.
342
343**Take the repo from the path, not the `repo` field** — a test pins this, so a row can never send the user to a different repository than the one its link names.
344
345**The badge needs no extra request.** If you find yourself adding a `notifications list` call to fill the Dashboard count, stop: `dashboard` already carries it.