| @@ -0,0 +1,2416 @@ |
| 1 | # iOS push notifications — server half — 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:** gitbayd delivers activity notices to registered Apple devices over APNs, as a third route beside the inbox row and the activity mail `notify()` already sends. |
| 6 | |
| 7 | **Architecture:** A notice becomes one `push_queue` row per registered device. An `internal/push.Deliverer` drains the queue on a ticker and POSTs each row to APNs over HTTP/2, authenticated by an ES256 JWT signed with an operator-supplied `.p8`. This is the third instance of a shape the repository already has twice: `internal/notify` (mail) and `internal/webhook` (HTTP POSTs) — a queue table, a drainer goroutine, exponential backoff, dead-lettering. |
| 8 | |
| 9 | **Tech Stack:** Go 1.27, SQLite (hand-written SQL, no ORM), stdlib only. No new module dependencies: `net/http` negotiates HTTP/2 over ALPN, and the JWT is `crypto/ecdsa` plus `encoding/json`. |
| 10 | |
| 11 | **Spec:** `docs/specs/2026-09-20-ios-push-notifications-design.md` |
| 12 | |
| 13 | ## Global Constraints |
| 14 | |
| 15 | - **No new Go module dependencies.** Nothing is added to `go.mod`. APNs needs HTTP/2, which stdlib `net/http` does over ALPN. The JWT is hand-rolled; do not reach for a JWT library. |
| 16 | - **Never attribute anything to an assistant or model.** Not in commits, not in code comments, not in MR bodies, not in docs. |
| 17 | - **Never push to `main`.** All work is on the `ios-push` branch in the worktree `/Users/cmc/git/krz/gitbay-push`. `require_mr` is on for this repository — a direct push to `main` is refused in pre-receive. |
| 18 | - **Commits must be signed.** This repository refuses unsigned commits. Use `git -c commit.gpgsign=true commit`. |
| 19 | - **Commit messages reference the issue:** `Ref #89`, and `Closes #89` on the last one. |
| 20 | - **Secrets on stdin, never argv.** `/proc` is world-readable. |
| 21 | - **A command that reads stdin must set `ReadsStdin: true`** on its `Command`. Otherwise `control.go` swaps in an empty reader and `--file -` silently stores nothing — it does not error. |
| 22 | - **A new control command needs a `pass()` entry** in `cmd/gitbay/main.go` or the CLI coverage test fails. |
| 23 | - **A new page template needs a row in `TestMainWidthClass`** (`internal/web/web_test.go`) or CI fails on it. |
| 24 | - **Test scope while working:** build, `go vet ./...`, and the unit tests of the packages you touched. Full `go test ./...` belongs to CI on bay1 — the e2e suite is most of the runtime. Run `go vet ./...` after any signature change; `go build` skips `_test.go` files and will not catch a stale test caller. |
| 25 | - **Never define a color only inside the dark media query** (relevant only to Task 10). |
| 26 | |
| 27 | --- |
| 28 | |
| 29 | ### Task 1: Migration 0059 and the device table |
| 30 | |
| 31 | **Files:** |
| 32 | - Create: `internal/store/migrations/0059_push.up.sql` |
| 33 | - Create: `internal/store/migrations/0059_push.down.sql` |
| 34 | - Create: `internal/store/push.go` |
| 35 | - Test: `internal/store/push_test.go` |
| 36 | |
| 37 | **Interfaces:** |
| 38 | - Consumes: nothing. |
| 39 | - Produces: |
| 40 | - `type PushDevice struct { ID int64; UserID int64; Token string; Label string; CreatedAt string; LastSeenAt string }` |
| 41 | - `func (s *Store) AddPushDevice(userID int64, token, label string) (int64, error)` |
| 42 | - `func (s *Store) PushDevices(userID int64) ([]PushDevice, error)` |
| 43 | - `func (s *Store) RemovePushDevice(userID, id int64) error` |
| 44 | - `func (s *Store) PushEnabled(userID int64) (bool, error)` |
| 45 | - `func (s *Store) SetPushEnabled(userID int64, on bool) error` |
| 46 | |
| 47 | - [ ] **Step 1: Write the migration** |
| 48 | |
| 49 | `internal/store/migrations/0059_push.up.sql`: |
| 50 | |
| 51 | ```sql |
| 52 | -- Apple devices an account has registered, and the queue of pushes bound |
| 53 | -- for them. The mail queue's table is named `notifications`, so this one |
| 54 | -- cannot be; the columns mirror it so the drainer is the mailer's loop. |
| 55 | CREATE TABLE push_devices ( |
| 56 | id INTEGER PRIMARY KEY, |
| 57 | user_id INTEGER NOT NULL REFERENCES users(id) ON DELETE CASCADE, |
| 58 | token TEXT NOT NULL UNIQUE, |
| 59 | label TEXT NOT NULL DEFAULT '', |
| 60 | created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ','now')), |
| 61 | last_seen_at TEXT |
| 62 | ); |
| 63 | CREATE INDEX push_devices_user ON push_devices(user_id); |
| 64 | |
| 65 | CREATE TABLE push_queue ( |
| 66 | id INTEGER PRIMARY KEY, |
| 67 | device_id INTEGER NOT NULL REFERENCES push_devices(id) ON DELETE CASCADE, |
| 68 | title TEXT NOT NULL, |
| 69 | body TEXT NOT NULL, |
| 70 | path TEXT NOT NULL, |
| 71 | attempts INTEGER NOT NULL DEFAULT 0, |
| 72 | next_attempt_at TEXT, |
| 73 | sent_at TEXT, |
| 74 | failed_at TEXT, |
| 75 | last_error TEXT, |
| 76 | created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ','now')) |
| 77 | ); |
| 78 | CREATE INDEX push_queue_due ON push_queue(next_attempt_at) |
| 79 | WHERE sent_at IS NULL AND failed_at IS NULL; |
| 80 | |
| 81 | -- Whether activity reaches the account's registered devices. Defaults on |
| 82 | -- and costs nothing for an account with no devices; it exists so a user |
| 83 | -- with a phone and an iPad silences both without deregistering each. |
| 84 | ALTER TABLE users ADD COLUMN notify_push INTEGER NOT NULL DEFAULT 1; |
| 85 | ``` |
| 86 | |
| 87 | `internal/store/migrations/0059_push.down.sql`: |
| 88 | |
| 89 | ```sql |
| 90 | DROP TABLE push_queue; |
| 91 | DROP TABLE push_devices; |
| 92 | ALTER TABLE users DROP COLUMN notify_push; |
| 93 | ``` |
| 94 | |
| 95 | - [ ] **Step 2: Write the failing test** |
| 96 | |
| 97 | `internal/store/push_test.go`: |
| 98 | |
| 99 | ```go |
| 100 | package store |
| 101 | |
| 102 | import "testing" |
| 103 | |
| 104 | func TestPushDevices(t *testing.T) { |
| 105 | s := testStore(t) |
| 106 | uid := testUser(t, s, "alice") |
| 107 | |
| 108 | id, err := s.AddPushDevice(uid, "tok-a", "iphone") |
| 109 | if err != nil { |
| 110 | t.Fatalf("AddPushDevice: %v", err) |
| 111 | } |
| 112 | devices, err := s.PushDevices(uid) |
| 113 | if err != nil { |
| 114 | t.Fatalf("PushDevices: %v", err) |
| 115 | } |
| 116 | if len(devices) != 1 || devices[0].Token != "tok-a" || devices[0].Label != "iphone" { |
| 117 | t.Fatalf("got %+v", devices) |
| 118 | } |
| 119 | |
| 120 | // Apple reuses tokens: re-registering updates the label and the owner |
| 121 | // rather than erroring, so a reinstall under another account works. |
| 122 | bob := testUser(t, s, "bob") |
| 123 | if _, err := s.AddPushDevice(bob, "tok-a", "ipad"); err != nil { |
| 124 | t.Fatalf("re-register: %v", err) |
| 125 | } |
| 126 | if d, _ := s.PushDevices(uid); len(d) != 0 { |
| 127 | t.Fatalf("token still owned by alice: %+v", d) |
| 128 | } |
| 129 | d, _ := s.PushDevices(bob) |
| 130 | if len(d) != 1 || d[0].Label != "ipad" { |
| 131 | t.Fatalf("got %+v", d) |
| 132 | } |
| 133 | |
| 134 | // Removal is scoped to the owner: alice cannot remove bob's device. |
| 135 | if err := s.RemovePushDevice(uid, d[0].ID); err != ErrNotFound { |
| 136 | t.Fatalf("cross-account remove: got %v, want ErrNotFound", err) |
| 137 | } |
| 138 | if err := s.RemovePushDevice(bob, d[0].ID); err != nil { |
| 139 | t.Fatalf("RemovePushDevice: %v", err) |
| 140 | } |
| 141 | if d, _ := s.PushDevices(bob); len(d) != 0 { |
| 142 | t.Fatalf("device survived removal: %+v", d) |
| 143 | } |
| 144 | _ = id |
| 145 | } |
| 146 | |
| 147 | func TestPushEnabledDefaultsOn(t *testing.T) { |
| 148 | s := testStore(t) |
| 149 | uid := testUser(t, s, "alice") |
| 150 | on, err := s.PushEnabled(uid) |
| 151 | if err != nil { |
| 152 | t.Fatalf("PushEnabled: %v", err) |
| 153 | } |
| 154 | if !on { |
| 155 | t.Fatal("notify_push should default on") |
| 156 | } |
| 157 | if err := s.SetPushEnabled(uid, false); err != nil { |
| 158 | t.Fatalf("SetPushEnabled: %v", err) |
| 159 | } |
| 160 | if on, _ := s.PushEnabled(uid); on { |
| 161 | t.Fatal("SetPushEnabled(false) did not stick") |
| 162 | } |
| 163 | } |
| 164 | ``` |
| 165 | |
| 166 | Check the helper names `testStore` and `testUser` against the existing |
| 167 | `internal/store/inbox_test.go` and use whatever that file uses; do not |
| 168 | invent new helpers. |
| 169 | |
| 170 | - [ ] **Step 3: Run the test to verify it fails** |
| 171 | |
| 172 | Run: `go test ./internal/store/ -run 'TestPush' -v` |
| 173 | Expected: FAIL — `s.AddPushDevice undefined`. |
| 174 | |
| 175 | - [ ] **Step 4: Write the implementation** |
| 176 | |
| 177 | `internal/store/push.go`: |
| 178 | |
| 179 | ```go |
| 180 | package store |
| 181 | |
| 182 | import ( |
| 183 | "database/sql" |
| 184 | "errors" |
| 185 | ) |
| 186 | |
| 187 | // PushDevice is one Apple device an account has registered. Token is the |
| 188 | // APNs device token: an address, not a credential, but device-identifying |
| 189 | // and never logged or echoed in full. |
| 190 | type PushDevice struct { |
| 191 | ID int64 |
| 192 | UserID int64 |
| 193 | Token string |
| 194 | Label string |
| 195 | CreatedAt string |
| 196 | LastSeenAt string |
| 197 | } |
| 198 | |
| 199 | // AddPushDevice registers a token to an account. A token already present |
| 200 | // changes hands rather than erroring: Apple reuses tokens, and a reinstall |
| 201 | // hands the same one to whichever account signs in next. |
| 202 | func (s *Store) AddPushDevice(userID int64, token, label string) (int64, error) { |
| 203 | res, err := s.DB.Exec(` |
| 204 | INSERT INTO push_devices (user_id, token, label) VALUES (?, ?, ?) |
| 205 | ON CONFLICT(token) DO UPDATE SET user_id = excluded.user_id, label = excluded.label`, |
| 206 | userID, token, label) |
| 207 | if err != nil { |
| 208 | return 0, err |
| 209 | } |
| 210 | return res.LastInsertId() |
| 211 | } |
| 212 | |
| 213 | func (s *Store) PushDevices(userID int64) ([]PushDevice, error) { |
| 214 | rows, err := s.DB.Query(` |
| 215 | SELECT id, user_id, token, label, created_at, COALESCE(last_seen_at, '') |
| 216 | FROM push_devices WHERE user_id = ? ORDER BY id`, userID) |
| 217 | if err != nil { |
| 218 | return nil, err |
| 219 | } |
| 220 | defer rows.Close() |
| 221 | var out []PushDevice |
| 222 | for rows.Next() { |
| 223 | var d PushDevice |
| 224 | if err := rows.Scan(&d.ID, &d.UserID, &d.Token, &d.Label, &d.CreatedAt, &d.LastSeenAt); err != nil { |
| 225 | return nil, err |
| 226 | } |
| 227 | out = append(out, d) |
| 228 | } |
| 229 | return out, rows.Err() |
| 230 | } |
| 231 | |
| 232 | // RemovePushDevice deletes one of the account's own devices. Scoping the |
| 233 | // delete by user_id rather than checking ownership first means another |
| 234 | // account's id is ErrNotFound, which is the same answer as an id that |
| 235 | // never existed — a caller learns nothing about other accounts' devices. |
| 236 | func (s *Store) RemovePushDevice(userID, id int64) error { |
| 237 | res, err := s.DB.Exec("DELETE FROM push_devices WHERE id = ? AND user_id = ?", id, userID) |
| 238 | if err != nil { |
| 239 | return err |
| 240 | } |
| 241 | n, err := res.RowsAffected() |
| 242 | if err != nil { |
| 243 | return err |
| 244 | } |
| 245 | if n == 0 { |
| 246 | return ErrNotFound |
| 247 | } |
| 248 | return nil |
| 249 | } |
| 250 | |
| 251 | func (s *Store) PushEnabled(userID int64) (bool, error) { |
| 252 | var on int |
| 253 | err := s.DB.QueryRow("SELECT notify_push FROM users WHERE id = ?", userID).Scan(&on) |
| 254 | if errors.Is(err, sql.ErrNoRows) { |
| 255 | return false, ErrNotFound |
| 256 | } |
| 257 | return on != 0, err |
| 258 | } |
| 259 | |
| 260 | func (s *Store) SetPushEnabled(userID int64, on bool) error { |
| 261 | v := 0 |
| 262 | if on { |
| 263 | v = 1 |
| 264 | } |
| 265 | _, err := s.DB.Exec("UPDATE users SET notify_push = ? WHERE id = ?", v, userID) |
| 266 | return err |
| 267 | } |
| 268 | ``` |
| 269 | |
| 270 | - [ ] **Step 5: Run the tests to verify they pass** |
| 271 | |
| 272 | Run: `go test ./internal/store/ -run 'TestPush' -v` |
| 273 | Expected: PASS, both tests. |
| 274 | |
| 275 | - [ ] **Step 6: Commit** |
| 276 | |
| 277 | ```bash |
| 278 | git add internal/store/migrations/0059_push.up.sql internal/store/migrations/0059_push.down.sql internal/store/push.go internal/store/push_test.go |
| 279 | git -c commit.gpgsign=true commit -m "store: push device registrations |
| 280 | |
| 281 | Migration 0059 adds push_devices, push_queue and users.notify_push. A |
| 282 | re-registered token changes hands rather than erroring, since Apple |
| 283 | reuses tokens across reinstalls. |
| 284 | |
| 285 | Ref #89" |
| 286 | ``` |
| 287 | |
| 288 | --- |
| 289 | |
| 290 | ### Task 2: The push queue and its retention |
| 291 | |
| 292 | **Files:** |
| 293 | - Modify: `internal/store/push.go` |
| 294 | - Modify: `internal/store/retention.go:30-35` (the `Retention` struct) and the `aged` table around `:66-75` |
| 295 | - Modify: `internal/config/config.go` (the `Retention` struct and its `Durations` method) |
| 296 | - Modify: `cmd/gitbayd/main.go:518-520` |
| 297 | - Test: `internal/store/push_test.go` |
| 298 | |
| 299 | **Interfaces:** |
| 300 | - Consumes: `PushDevice`, `PushEnabled` from Task 1. |
| 301 | - Produces: |
| 302 | - `type QueuedPush struct { ID int64; DeviceID int64; Token string; Title string; Body string; Path string; Attempts int }` |
| 303 | - `func (s *Store) EnqueuePush(userID int64, title, body, path string) error` |
| 304 | - `func (s *Store) DuePush(limit int) ([]QueuedPush, error)` |
| 305 | - `func (s *Store) MarkPushSent(id int64) error` |
| 306 | - `func (s *Store) MarkPushFailed(id int64, errMsg string, nextAt *time.Time) error` |
| 307 | - `func (s *Store) DeletePushDeviceByToken(token string) error` |
| 308 | - `config.Retention.Push string` with toml key `push`, and a fifth return from `Durations()` |
| 309 | - `store.Retention.Push time.Duration` |
| 310 | |
| 311 | - [ ] **Step 1: Write the failing test** |
| 312 | |
| 313 | Append to `internal/store/push_test.go`: |
| 314 | |
| 315 | ```go |
| 316 | func TestEnqueuePush(t *testing.T) { |
| 317 | s := testStore(t) |
| 318 | uid := testUser(t, s, "alice") |
| 319 | s.AddPushDevice(uid, "tok-a", "iphone") |
| 320 | s.AddPushDevice(uid, "tok-b", "ipad") |
| 321 | |
| 322 | // One row per device, so a retry to the phone does not resend to the |
| 323 | // iPad. |
| 324 | if err := s.EnqueuePush(uid, "krz/gitbay", "cmc opened issue #12", "krz/gitbay/issues/12"); err != nil { |
| 325 | t.Fatalf("EnqueuePush: %v", err) |
| 326 | } |
| 327 | due, err := s.DuePush(20) |
| 328 | if err != nil { |
| 329 | t.Fatalf("DuePush: %v", err) |
| 330 | } |
| 331 | if len(due) != 2 { |
| 332 | t.Fatalf("want a row per device, got %d", len(due)) |
| 333 | } |
| 334 | if due[0].Token == "" || due[0].Body != "cmc opened issue #12" { |
| 335 | t.Fatalf("got %+v", due[0]) |
| 336 | } |
| 337 | |
| 338 | // Sent rows stop being due. |
| 339 | if err := s.MarkPushSent(due[0].ID); err != nil { |
| 340 | t.Fatalf("MarkPushSent: %v", err) |
| 341 | } |
| 342 | if due, _ := s.DuePush(20); len(due) != 1 { |
| 343 | t.Fatalf("sent row still due") |
| 344 | } |
| 345 | |
| 346 | // A failure with a next attempt in the future is not due yet. |
| 347 | next := time.Now().Add(time.Hour) |
| 348 | if err := s.MarkPushFailed(due[1].ID, "503", &next); err != nil { |
| 349 | t.Fatalf("MarkPushFailed: %v", err) |
| 350 | } |
| 351 | if due, _ := s.DuePush(20); len(due) != 0 { |
| 352 | t.Fatalf("backed-off row is due too early") |
| 353 | } |
| 354 | } |
| 355 | |
| 356 | func TestEnqueuePushRespectsSettingAndDevices(t *testing.T) { |
| 357 | s := testStore(t) |
| 358 | uid := testUser(t, s, "alice") |
| 359 | |
| 360 | // No devices: nothing queued, no error. |
| 361 | if err := s.EnqueuePush(uid, "t", "b", "p"); err != nil { |
| 362 | t.Fatalf("EnqueuePush with no devices: %v", err) |
| 363 | } |
| 364 | if due, _ := s.DuePush(20); len(due) != 0 { |
| 365 | t.Fatalf("queued for an account with no devices") |
| 366 | } |
| 367 | |
| 368 | // Setting off: nothing queued. |
| 369 | s.AddPushDevice(uid, "tok-a", "iphone") |
| 370 | s.SetPushEnabled(uid, false) |
| 371 | if err := s.EnqueuePush(uid, "t", "b", "p"); err != nil { |
| 372 | t.Fatalf("EnqueuePush with push off: %v", err) |
| 373 | } |
| 374 | if due, _ := s.DuePush(20); len(due) != 0 { |
| 375 | t.Fatalf("queued with notify_push off") |
| 376 | } |
| 377 | } |
| 378 | |
| 379 | func TestDeletePushDeviceByTokenTakesItsQueue(t *testing.T) { |
| 380 | s := testStore(t) |
| 381 | uid := testUser(t, s, "alice") |
| 382 | s.AddPushDevice(uid, "tok-a", "iphone") |
| 383 | s.EnqueuePush(uid, "t", "b", "p") |
| 384 | |
| 385 | if err := s.DeletePushDeviceByToken("tok-a"); err != nil { |
| 386 | t.Fatalf("DeletePushDeviceByToken: %v", err) |
| 387 | } |
| 388 | if d, _ := s.PushDevices(uid); len(d) != 0 { |
| 389 | t.Fatalf("device survived") |
| 390 | } |
| 391 | // push_queue.device_id is ON DELETE CASCADE, so the queued rows go |
| 392 | // with it rather than being retried at a dead token forever. |
| 393 | if due, _ := s.DuePush(20); len(due) != 0 { |
| 394 | t.Fatalf("queued rows outlived their device") |
| 395 | } |
| 396 | } |
| 397 | ``` |
| 398 | |
| 399 | Add `"time"` to the test file's imports. |
| 400 | |
| 401 | - [ ] **Step 2: Run the test to verify it fails** |
| 402 | |
| 403 | Run: `go test ./internal/store/ -run 'TestEnqueuePush|TestDeletePushDevice' -v` |
| 404 | Expected: FAIL — `s.EnqueuePush undefined`. |
| 405 | |
| 406 | - [ ] **Step 3: Write the queue implementation** |
| 407 | |
| 408 | Append to `internal/store/push.go` (and add `"time"` to its imports): |
| 409 | |
| 410 | ```go |
| 411 | // QueuedPush is one pending push, joined to the token it is bound for so |
| 412 | // the drainer needs one query rather than two. |
| 413 | type QueuedPush struct { |
| 414 | ID int64 |
| 415 | DeviceID int64 |
| 416 | Token string |
| 417 | Title string |
| 418 | Body string |
| 419 | Path string |
| 420 | Attempts int |
| 421 | } |
| 422 | |
| 423 | // EnqueuePush writes one row per registered device, and nothing when the |
| 424 | // account has push off or no devices — the same shape as |
| 425 | // ActivityMailAddress returning "" when notify_mail is off. Mute, watch |
| 426 | // and actor-exclusion are already settled by NotifyRecipients before a |
| 427 | // caller reaches here. |
| 428 | func (s *Store) EnqueuePush(userID int64, title, body, path string) error { |
| 429 | on, err := s.PushEnabled(userID) |
| 430 | if err != nil || !on { |
| 431 | return err |
| 432 | } |
| 433 | _, err = s.DB.Exec(` |
| 434 | INSERT INTO push_queue (device_id, title, body, path) |
| 435 | SELECT id, ?, ?, ? FROM push_devices WHERE user_id = ?`, |
| 436 | title, body, path, userID) |
| 437 | return err |
| 438 | } |
| 439 | |
| 440 | func (s *Store) DuePush(limit int) ([]QueuedPush, error) { |
| 441 | rows, err := s.DB.Query(` |
| 442 | SELECT q.id, q.device_id, d.token, q.title, q.body, q.path, q.attempts |
| 443 | FROM push_queue q JOIN push_devices d ON d.id = q.device_id |
| 444 | WHERE q.sent_at IS NULL AND q.failed_at IS NULL |
| 445 | AND (q.next_attempt_at IS NULL OR q.next_attempt_at <= ?) |
| 446 | ORDER BY q.id LIMIT ?`, fmtTime(time.Now()), limit) |
| 447 | if err != nil { |
| 448 | return nil, err |
| 449 | } |
| 450 | defer rows.Close() |
| 451 | var out []QueuedPush |
| 452 | for rows.Next() { |
| 453 | var p QueuedPush |
| 454 | if err := rows.Scan(&p.ID, &p.DeviceID, &p.Token, &p.Title, &p.Body, &p.Path, &p.Attempts); err != nil { |
| 455 | return nil, err |
| 456 | } |
| 457 | out = append(out, p) |
| 458 | } |
| 459 | return out, rows.Err() |
| 460 | } |
| 461 | |
| 462 | func (s *Store) MarkPushSent(id int64) error { |
| 463 | _, err := s.DB.Exec( |
| 464 | "UPDATE push_queue SET sent_at = strftime('%Y-%m-%dT%H:%M:%fZ','now'), attempts = attempts + 1 WHERE id = ?", id) |
| 465 | return err |
| 466 | } |
| 467 | |
| 468 | func (s *Store) MarkPushFailed(id int64, errMsg string, nextAt *time.Time) error { |
| 469 | if nextAt == nil { |
| 470 | _, err := s.DB.Exec( |
| 471 | "UPDATE push_queue SET failed_at = strftime('%Y-%m-%dT%H:%M:%fZ','now'), attempts = attempts + 1, last_error = ? WHERE id = ?", |
| 472 | errMsg, id) |
| 473 | return err |
| 474 | } |
| 475 | _, err := s.DB.Exec( |
| 476 | "UPDATE push_queue SET attempts = attempts + 1, last_error = ?, next_attempt_at = ? WHERE id = ?", |
| 477 | errMsg, fmtTime(*nextAt), id) |
| 478 | return err |
| 479 | } |
| 480 | |
| 481 | // DeletePushDeviceByToken drops a device Apple has told us is gone. The |
| 482 | // queue rows cascade, so nothing is left retrying at a dead token. |
| 483 | func (s *Store) DeletePushDeviceByToken(token string) error { |
| 484 | _, err := s.DB.Exec("DELETE FROM push_devices WHERE token = ?", token) |
| 485 | return err |
| 486 | } |
| 487 | ``` |
| 488 | |
| 489 | - [ ] **Step 4: Run the tests to verify they pass** |
| 490 | |
| 491 | Run: `go test ./internal/store/ -run 'TestPush|TestEnqueuePush|TestDeletePushDevice' -v` |
| 492 | Expected: PASS. |
| 493 | |
| 494 | If `TestDeletePushDeviceByTokenTakesItsQueue` fails with the queue rows |
| 495 | surviving, foreign keys are not on for that connection. Check how |
| 496 | `testStore` opens the database against the rest of `internal/store` — |
| 497 | do not work around it by deleting the queue rows by hand. |
| 498 | |
| 499 | - [ ] **Step 5: Add the retention key** |
| 500 | |
| 501 | In `internal/config/config.go`, add to the `Retention` struct: |
| 502 | |
| 503 | ```go |
| 504 | // Push is the outbound device queue: rows already sent or given up on. |
| 505 | Push string `toml:"push"` |
| 506 | ``` |
| 507 | |
| 508 | Find `Retention.Durations()` in the same file and give it a fifth return |
| 509 | value parsed the same way as `Mail`. |
| 510 | |
| 511 | In `internal/store/retention.go`, add to the `Retention` struct: |
| 512 | |
| 513 | ```go |
| 514 | Push time.Duration |
| 515 | ``` |
| 516 | |
| 517 | and to the `aged` slice in `Sweep`, after the `notifications` row: |
| 518 | |
| 519 | ```go |
| 520 | {"push_queue", "created_at < ? AND (sent_at IS NOT NULL OR failed_at IS NOT NULL)", r.Push}, |
| 521 | ``` |
| 522 | |
| 523 | In `cmd/gitbayd/main.go`, the `sweep` function around line 518: |
| 524 | |
| 525 | ```go |
| 526 | audit, events, deliveries, mail, push := cfg.Retention.Durations() |
| 527 | r := store.Retention{Audit: audit, Events: events, |
| 528 | WebhookDeliveries: deliveries, Mail: mail, Push: push} |
| 529 | ``` |
| 530 | |
| 531 | - [ ] **Step 6: Build and vet** |
| 532 | |
| 533 | Run: `go build ./... && go vet ./...` |
| 534 | Expected: clean. `Durations()` gained a return value, so `go vet` is what |
| 535 | catches any caller `go build` skipped — check for callers in |
| 536 | `internal/config`'s own tests. |
| 537 | |
| 538 | Run: `go test ./internal/config/ ./internal/store/ ./cmd/gitbayd/` |
| 539 | Expected: PASS. |
| 540 | |
| 541 | - [ ] **Step 7: Commit** |
| 542 | |
| 543 | ```bash |
| 544 | git add internal/store/push.go internal/store/push_test.go internal/store/retention.go internal/config/config.go cmd/gitbayd/main.go |
| 545 | git -c commit.gpgsign=true commit -m "store: the push queue, swept like the mail queue |
| 546 | |
| 547 | One row per device per notice, so a retry to one device does not |
| 548 | resend to another. EnqueuePush writes nothing when the account has |
| 549 | push off or no devices. [retention] push caps the table. |
| 550 | |
| 551 | Ref #89" |
| 552 | ``` |
| 553 | |
| 554 | --- |
| 555 | |
| 556 | ### Task 3: The `[push]` config section |
| 557 | |
| 558 | **Files:** |
| 559 | - Modify: `internal/config/config.go` |
| 560 | - Test: `internal/config/config_test.go` |
| 561 | |
| 562 | **Interfaces:** |
| 563 | - Consumes: nothing. |
| 564 | - Produces: |
| 565 | - `type Push struct { Enabled bool; KeyFile string; KeyID string; TeamID string; Topic string; Environment string }` with toml keys `enabled`, `key_file`, `key_id`, `team_id`, `topic`, `environment` |
| 566 | - `Config.Push Push` with toml key `push` |
| 567 | - `func (p Push) Host() string` returning `api.push.apple.com` or `api.sandbox.push.apple.com`, overridden by `GITBAY_APNS_HOST` |
| 568 | |
| 569 | - [ ] **Step 1: Write the failing test** |
| 570 | |
| 571 | Append to `internal/config/config_test.go`. It already has |
| 572 | `writeConfig(t, body) string` and a `minimal` constant; use both rather |
| 573 | than adding a second way to load a config. |
| 574 | |
| 575 | ```go |
| 576 | // writeP8 writes a PEM-wrapped PKCS#8 P-256 key, the shape of Apple's |
| 577 | // .p8 provider key, and returns its path. |
| 578 | func writeP8(t *testing.T) string { |
| 579 | t.Helper() |
| 580 | key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) |
| 581 | if err != nil { |
| 582 | t.Fatal(err) |
| 583 | } |
| 584 | der, err := x509.MarshalPKCS8PrivateKey(key) |
| 585 | if err != nil { |
| 586 | t.Fatal(err) |
| 587 | } |
| 588 | p := filepath.Join(t.TempDir(), "apns.p8") |
| 589 | f, err := os.Create(p) |
| 590 | if err != nil { |
| 591 | t.Fatal(err) |
| 592 | } |
| 593 | defer f.Close() |
| 594 | if err := pem.Encode(f, &pem.Block{Type: "PRIVATE KEY", Bytes: der}); err != nil { |
| 595 | t.Fatal(err) |
| 596 | } |
| 597 | return p |
| 598 | } |
| 599 | |
| 600 | func TestPushConfigValidation(t *testing.T) { |
| 601 | keyPath := writeP8(t) |
| 602 | full := ` |
| 603 | [push] |
| 604 | enabled = true |
| 605 | key_file = "` + keyPath + `" |
| 606 | key_id = "KEYID" |
| 607 | team_id = "TEAMID" |
| 608 | topic = "org.gitbay.gitbay" |
| 609 | environment = "production" |
| 610 | ` |
| 611 | cases := []struct { |
| 612 | name string |
| 613 | body string |
| 614 | want string // substring of the expected error; "" means valid |
| 615 | }{ |
| 616 | {"disabled needs nothing", "\n[push]\nenabled = false\n", ""}, |
| 617 | {"complete is valid", full, ""}, |
| 618 | {"key_id required", strings.Replace(full, `key_id = "KEYID"`, "", 1), "push.key_id"}, |
| 619 | {"team_id required", strings.Replace(full, `team_id = "TEAMID"`, "", 1), "push.team_id"}, |
| 620 | {"topic required", strings.Replace(full, `topic = "org.gitbay.gitbay"`, "", 1), "push.topic"}, |
| 621 | {"environment must be a known name", |
| 622 | strings.Replace(full, `environment = "production"`, `environment = "staging"`, 1), |
| 623 | "push.environment"}, |
| 624 | } |
| 625 | for _, tc := range cases { |
| 626 | t.Run(tc.name, func(t *testing.T) { |
| 627 | _, err := Load(writeConfig(t, minimal+tc.body)) |
| 628 | if tc.want == "" { |
| 629 | if err != nil { |
| 630 | t.Fatalf("want valid, got %v", err) |
| 631 | } |
| 632 | return |
| 633 | } |
| 634 | if err == nil || !strings.Contains(err.Error(), tc.want) { |
| 635 | t.Fatalf("want an error mentioning %q, got %v", tc.want, err) |
| 636 | } |
| 637 | }) |
| 638 | } |
| 639 | } |
| 640 | |
| 641 | // A key_file that exists but is not a PKCS#8 EC key is refused at load, |
| 642 | // not at the first notice: the failure mode otherwise is a queue that |
| 643 | // fills and dead-letters with nobody watching. |
| 644 | func TestPushConfigRejectsAnUnparseableKey(t *testing.T) { |
| 645 | p := filepath.Join(t.TempDir(), "junk.p8") |
| 646 | if err := os.WriteFile(p, []byte("not a key\n"), 0o600); err != nil { |
| 647 | t.Fatal(err) |
| 648 | } |
| 649 | body := ` |
| 650 | [push] |
| 651 | enabled = true |
| 652 | key_file = "` + p + `" |
| 653 | key_id = "K" |
| 654 | team_id = "T" |
| 655 | topic = "org.gitbay.gitbay" |
| 656 | environment = "production" |
| 657 | ` |
| 658 | _, err := Load(writeConfig(t, minimal+body)) |
| 659 | if err == nil || !strings.Contains(err.Error(), "push.key_file") { |
| 660 | t.Fatalf("want a push.key_file error, got %v", err) |
| 661 | } |
| 662 | } |
| 663 | |
| 664 | func TestPushHost(t *testing.T) { |
| 665 | if got := (Push{Environment: "production"}).Host(); got != "api.push.apple.com" { |
| 666 | t.Fatalf("production host = %q", got) |
| 667 | } |
| 668 | if got := (Push{Environment: "sandbox"}).Host(); got != "api.sandbox.push.apple.com" { |
| 669 | t.Fatalf("sandbox host = %q", got) |
| 670 | } |
| 671 | t.Setenv("GITBAY_APNS_HOST", "127.0.0.1:1234") |
| 672 | if got := (Push{Environment: "production"}).Host(); got != "127.0.0.1:1234" { |
| 673 | t.Fatalf("GITBAY_APNS_HOST ignored: %q", got) |
| 674 | } |
| 675 | } |
| 676 | ``` |
| 677 | |
| 678 | Add `crypto/ecdsa`, `crypto/elliptic`, `crypto/rand`, `crypto/x509` and |
| 679 | `encoding/pem` to the test file's imports. |
| 680 | |
| 681 | - [ ] **Step 2: Run the test to verify it fails** |
| 682 | |
| 683 | Run: `go test ./internal/config/ -run TestPush -v` |
| 684 | Expected: FAIL — `Push` undefined. |
| 685 | |
| 686 | - [ ] **Step 3: Write the implementation** |
| 687 | |
| 688 | Add to `internal/config/config.go`, beside the other section structs: |
| 689 | |
| 690 | ```go |
| 691 | // Push is APNs delivery to registered Apple devices. A key belongs to a |
| 692 | // bundle ID, so an instance pushes to the app built under the topic named |
| 693 | // here and no other; a self-hoster points this at their own key and their |
| 694 | // own build. |
| 695 | type Push struct { |
| 696 | Enabled bool `toml:"enabled"` |
| 697 | KeyFile string `toml:"key_file"` |
| 698 | KeyID string `toml:"key_id"` |
| 699 | TeamID string `toml:"team_id"` |
| 700 | Topic string `toml:"topic"` // the app's bundle identifier |
| 701 | // Environment is a name rather than a URL so a typo cannot aim the |
| 702 | // key at a host that is not Apple's. |
| 703 | Environment string `toml:"environment"` // production | sandbox |
| 704 | } |
| 705 | |
| 706 | // Host is the APNs endpoint for the configured environment. |
| 707 | // GITBAY_APNS_HOST overrides it for tests, as GITBAY_SWEEP_TICK does for |
| 708 | // the retention sweep. |
| 709 | func (p Push) Host() string { |
| 710 | if h := os.Getenv("GITBAY_APNS_HOST"); h != "" { |
| 711 | return h |
| 712 | } |
| 713 | if p.Environment == "sandbox" { |
| 714 | return "api.sandbox.push.apple.com" |
| 715 | } |
| 716 | return "api.push.apple.com" |
| 717 | } |
| 718 | ``` |
| 719 | |
| 720 | Add the field to `Config`: |
| 721 | |
| 722 | ```go |
| 723 | Push Push `toml:"push"` |
| 724 | ``` |
| 725 | |
| 726 | In the validate function, beside the `MaxSnippetsPerUser` check: |
| 727 | |
| 728 | ```go |
| 729 | if c.Push.Enabled { |
| 730 | for _, f := range []struct{ name, val string }{ |
| 731 | {"push.key_file", c.Push.KeyFile}, |
| 732 | {"push.key_id", c.Push.KeyID}, |
| 733 | {"push.team_id", c.Push.TeamID}, |
| 734 | {"push.topic", c.Push.Topic}, |
| 735 | } { |
| 736 | if f.val == "" { |
| 737 | errs = append(errs, fmt.Errorf("%s is required when push.enabled", f.name)) |
| 738 | } |
| 739 | } |
| 740 | if err := oneOf("push.environment", c.Push.Environment, "production", "sandbox"); err != nil { |
| 741 | errs = append(errs, err) |
| 742 | } |
| 743 | if c.Push.KeyFile != "" { |
| 744 | if _, err := LoadAPNSKey(c.Push.KeyFile); err != nil { |
| 745 | errs = append(errs, fmt.Errorf("push.key_file: %w", err)) |
| 746 | } |
| 747 | } |
| 748 | } |
| 749 | ``` |
| 750 | |
| 751 | And the key loader, in the same file: |
| 752 | |
| 753 | ```go |
| 754 | // LoadAPNSKey reads Apple's .p8 provider key: a PEM-wrapped PKCS#8 |
| 755 | // P-256 private key. Read at startup and validated there, so a |
| 756 | // misconfigured [push] refuses to start rather than filling a queue |
| 757 | // nobody is watching. |
| 758 | func LoadAPNSKey(path string) (*ecdsa.PrivateKey, error) { |
| 759 | data, err := os.ReadFile(path) |
| 760 | if err != nil { |
| 761 | return nil, err |
| 762 | } |
| 763 | block, _ := pem.Decode(data) |
| 764 | if block == nil { |
| 765 | return nil, errors.New("not PEM") |
| 766 | } |
| 767 | any, err := x509.ParsePKCS8PrivateKey(block.Bytes) |
| 768 | if err != nil { |
| 769 | return nil, err |
| 770 | } |
| 771 | key, ok := any.(*ecdsa.PrivateKey) |
| 772 | if !ok { |
| 773 | return nil, errors.New("not an EC private key") |
| 774 | } |
| 775 | return key, nil |
| 776 | } |
| 777 | ``` |
| 778 | |
| 779 | Add `crypto/ecdsa`, `crypto/x509` and `encoding/pem` to the file's imports. |
| 780 | |
| 781 | - [ ] **Step 4: Run the tests to verify they pass** |
| 782 | |
| 783 | Run: `go test ./internal/config/ -v` |
| 784 | Expected: PASS. The whole package, because adding a `Config` field can |
| 785 | break a test that round-trips the struct or asserts on unknown keys. |
| 786 | |
| 787 | - [ ] **Step 5: Commit** |
| 788 | |
| 789 | ```bash |
| 790 | git add internal/config/config.go internal/config/config_test.go |
| 791 | git -c commit.gpgsign=true commit -m "config: the [push] section |
| 792 | |
| 793 | Validated at load: with push.enabled, the four fields are required, |
| 794 | environment is one of two names, and key_file must parse as a PKCS#8 |
| 795 | EC key. GITBAY_APNS_HOST redirects the endpoint for tests. |
| 796 | |
| 797 | Ref #89" |
| 798 | ``` |
| 799 | |
| 800 | --- |
| 801 | |
| 802 | ### Task 4: The APNs provider token |
| 803 | |
| 804 | **Files:** |
| 805 | - Create: `internal/push/token.go` |
| 806 | - Test: `internal/push/token_test.go` |
| 807 | |
| 808 | **Interfaces:** |
| 809 | - Consumes: `config.Push`, `config.LoadAPNSKey` from Task 3. |
| 810 | - Produces: |
| 811 | - `type tokenSource struct { key *ecdsa.PrivateKey; keyID, teamID string; now func() time.Time; mu sync.Mutex; cached string; issued time.Time }` |
| 812 | - `func newTokenSource(key *ecdsa.PrivateKey, keyID, teamID string) *tokenSource` |
| 813 | - `func (t *tokenSource) token() (string, error)` |
| 814 | |
| 815 | - [ ] **Step 1: Write the failing test** |
| 816 | |
| 817 | `internal/push/token_test.go`: |
| 818 | |
| 819 | ```go |
| 820 | package push |
| 821 | |
| 822 | import ( |
| 823 | "crypto/ecdsa" |
| 824 | "crypto/elliptic" |
| 825 | "crypto/rand" |
| 826 | "crypto/sha256" |
| 827 | "encoding/base64" |
| 828 | "encoding/json" |
| 829 | "math/big" |
| 830 | "strings" |
| 831 | "testing" |
| 832 | "time" |
| 833 | ) |
| 834 | |
| 835 | func testKey(t *testing.T) *ecdsa.PrivateKey { |
| 836 | t.Helper() |
| 837 | k, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) |
| 838 | if err != nil { |
| 839 | t.Fatal(err) |
| 840 | } |
| 841 | return k |
| 842 | } |
| 843 | |
| 844 | func TestTokenShapeAndSignature(t *testing.T) { |
| 845 | key := testKey(t) |
| 846 | ts := newTokenSource(key, "KEYID123", "TEAMID456") |
| 847 | tok, err := ts.token() |
| 848 | if err != nil { |
| 849 | t.Fatalf("token: %v", err) |
| 850 | } |
| 851 | parts := strings.Split(tok, ".") |
| 852 | if len(parts) != 3 { |
| 853 | t.Fatalf("want three dot-separated parts, got %d", len(parts)) |
| 854 | } |
| 855 | |
| 856 | var hdr struct{ Alg, Kid string } |
| 857 | raw, _ := base64.RawURLEncoding.DecodeString(parts[0]) |
| 858 | if err := json.Unmarshal(raw, &hdr); err != nil { |
| 859 | t.Fatalf("header: %v", err) |
| 860 | } |
| 861 | if hdr.Alg != "ES256" || hdr.Kid != "KEYID123" { |
| 862 | t.Fatalf("header = %+v", hdr) |
| 863 | } |
| 864 | |
| 865 | // APNs provider tokens carry iss (team id) and iat, and nothing else. |
| 866 | var claims map[string]any |
| 867 | raw, _ = base64.RawURLEncoding.DecodeString(parts[1]) |
| 868 | if err := json.Unmarshal(raw, &claims); err != nil { |
| 869 | t.Fatalf("claims: %v", err) |
| 870 | } |
| 871 | if claims["iss"] != "TEAMID456" { |
| 872 | t.Fatalf("iss = %v", claims["iss"]) |
| 873 | } |
| 874 | if _, ok := claims["iat"]; !ok { |
| 875 | t.Fatal("no iat") |
| 876 | } |
| 877 | if len(claims) != 2 { |
| 878 | t.Fatalf("unexpected claims: %v", claims) |
| 879 | } |
| 880 | |
| 881 | // The signature is raw r||s, 64 bytes — not the ASN.1 DER that |
| 882 | // ecdsa.SignASN1 returns. Sending DER gets every push rejected. |
| 883 | sig, err := base64.RawURLEncoding.DecodeString(parts[2]) |
| 884 | if err != nil { |
| 885 | t.Fatalf("signature not base64url: %v", err) |
| 886 | } |
| 887 | if len(sig) != 64 { |
| 888 | t.Fatalf("signature is %d bytes, want 64 (raw r||s)", len(sig)) |
| 889 | } |
| 890 | sum := sha256.Sum256([]byte(parts[0] + "." + parts[1])) |
| 891 | r := new(big.Int).SetBytes(sig[:32]) |
| 892 | s := new(big.Int).SetBytes(sig[32:]) |
| 893 | if !ecdsa.Verify(&key.PublicKey, sum[:], r, s) { |
| 894 | t.Fatal("signature does not verify") |
| 895 | } |
| 896 | } |
| 897 | |
| 898 | func TestTokenCachedThenReminted(t *testing.T) { |
| 899 | ts := newTokenSource(testKey(t), "K", "T") |
| 900 | base := time.Now() |
| 901 | ts.now = func() time.Time { return base } |
| 902 | |
| 903 | first, _ := ts.token() |
| 904 | second, _ := ts.token() |
| 905 | if first != second { |
| 906 | t.Fatal("token reminted inside the cache window; APNs answers TooManyProviderTokenUpdates") |
| 907 | } |
| 908 | |
| 909 | // Valid for an hour, not to be reminted faster than every twenty |
| 910 | // minutes: refresh at fifty. |
| 911 | ts.now = func() time.Time { return base.Add(51 * time.Minute) } |
| 912 | third, _ := ts.token() |
| 913 | if third == first { |
| 914 | t.Fatal("token not reminted after fifty minutes") |
| 915 | } |
| 916 | } |
| 917 | ``` |
| 918 | |
| 919 | - [ ] **Step 2: Run the test to verify it fails** |
| 920 | |
| 921 | Run: `go test ./internal/push/ -run TestToken -v` |
| 922 | Expected: FAIL — `newTokenSource` undefined. |
| 923 | |
| 924 | - [ ] **Step 3: Write the implementation** |
| 925 | |
| 926 | `internal/push/token.go`: |
| 927 | |
| 928 | ```go |
| 929 | // Package push delivers activity notices to Apple devices over APNs: the |
| 930 | // third delivery route beside the inbox row and the activity mail, with |
| 931 | // the bounded-retry discipline the mail queue and webhook deliverer use. |
| 932 | package push |
| 933 | |
| 934 | import ( |
| 935 | "crypto/ecdsa" |
| 936 | "crypto/rand" |
| 937 | "crypto/sha256" |
| 938 | "encoding/base64" |
| 939 | "encoding/json" |
| 940 | "sync" |
| 941 | "time" |
| 942 | ) |
| 943 | |
| 944 | // tokenLifetime is how long a provider token is reused. APNs accepts one |
| 945 | // for an hour and answers TooManyProviderTokenUpdates if they are minted |
| 946 | // faster than roughly once every twenty minutes, so the useful window is |
| 947 | // between the two. |
| 948 | const tokenLifetime = 50 * time.Minute |
| 949 | |
| 950 | type tokenSource struct { |
| 951 | key *ecdsa.PrivateKey |
| 952 | keyID string |
| 953 | teamID string |
| 954 | now func() time.Time |
| 955 | |
| 956 | mu sync.Mutex |
| 957 | cached string |
| 958 | issued time.Time |
| 959 | } |
| 960 | |
| 961 | func newTokenSource(key *ecdsa.PrivateKey, keyID, teamID string) *tokenSource { |
| 962 | return &tokenSource{key: key, keyID: keyID, teamID: teamID, now: time.Now} |
| 963 | } |
| 964 | |
| 965 | // token returns the cached provider token, minting a new one when the old |
| 966 | // one is near its end. |
| 967 | func (t *tokenSource) token() (string, error) { |
| 968 | t.mu.Lock() |
| 969 | defer t.mu.Unlock() |
| 970 | now := t.now() |
| 971 | if t.cached != "" && now.Sub(t.issued) < tokenLifetime { |
| 972 | return t.cached, nil |
| 973 | } |
| 974 | tok, err := t.sign(now) |
| 975 | if err != nil { |
| 976 | return "", err |
| 977 | } |
| 978 | t.cached, t.issued = tok, now |
| 979 | return tok, nil |
| 980 | } |
| 981 | |
| 982 | func (t *tokenSource) sign(now time.Time) (string, error) { |
| 983 | header, err := json.Marshal(map[string]string{"alg": "ES256", "kid": t.keyID}) |
| 984 | if err != nil { |
| 985 | return "", err |
| 986 | } |
| 987 | claims, err := json.Marshal(map[string]any{"iss": t.teamID, "iat": now.Unix()}) |
| 988 | if err != nil { |
| 989 | return "", err |
| 990 | } |
| 991 | enc := base64.RawURLEncoding |
| 992 | signing := enc.EncodeToString(header) + "." + enc.EncodeToString(claims) |
| 993 | sum := sha256.Sum256([]byte(signing)) |
| 994 | r, s, err := ecdsa.Sign(rand.Reader, t.key, sum[:]) |
| 995 | if err != nil { |
| 996 | return "", err |
| 997 | } |
| 998 | // JWS wants the raw pair, each left-padded to the curve's byte size — |
| 999 | // not ecdsa.SignASN1's DER. A DER signature is well-formed ECDSA and |
| 1000 | // is rejected by every JWT verifier, APNs included. |
| 1001 | sig := make([]byte, 64) |
| 1002 | r.FillBytes(sig[:32]) |
| 1003 | s.FillBytes(sig[32:]) |
| 1004 | return signing + "." + enc.EncodeToString(sig), nil |
| 1005 | } |
| 1006 | ``` |
| 1007 | |
| 1008 | - [ ] **Step 4: Run the tests to verify they pass** |
| 1009 | |
| 1010 | Run: `go test ./internal/push/ -run TestToken -v` |
| 1011 | Expected: PASS, both tests. |
| 1012 | |
| 1013 | - [ ] **Step 5: Commit** |
| 1014 | |
| 1015 | ```bash |
| 1016 | git add internal/push/token.go internal/push/token_test.go |
| 1017 | git -c commit.gpgsign=true commit -m "push: APNs provider tokens |
| 1018 | |
| 1019 | ES256 over iss and iat, cached fifty minutes. The signature is raw |
| 1020 | r||s rather than DER, which is the difference between a token APNs |
| 1021 | accepts and one it rejects. |
| 1022 | |
| 1023 | Ref #89" |
| 1024 | ``` |
| 1025 | |
| 1026 | --- |
| 1027 | |
| 1028 | ### Task 5: The APNs client |
| 1029 | |
| 1030 | **Files:** |
| 1031 | - Create: `internal/push/apns.go` |
| 1032 | - Test: `internal/push/apns_test.go` |
| 1033 | |
| 1034 | **Interfaces:** |
| 1035 | - Consumes: `tokenSource` from Task 4, `config.Push` from Task 3. |
| 1036 | - Produces: |
| 1037 | - `type Client struct { ... }` |
| 1038 | - `func NewClient(cfg config.Push) (*Client, error)` |
| 1039 | - `type result int` with constants `resultSent`, `resultRetry`, `resultReap`, `resultDead` |
| 1040 | - `func (c *Client) Send(ctx context.Context, token, title, body, path string) (res result, retryAfter time.Duration, err error)` |
| 1041 | |
| 1042 | - [ ] **Step 1: Write the failing test** |
| 1043 | |
| 1044 | `internal/push/apns_test.go`: |
| 1045 | |
| 1046 | ```go |
| 1047 | package push |
| 1048 | |
| 1049 | import ( |
| 1050 | "context" |
| 1051 | "encoding/json" |
| 1052 | "net/http" |
| 1053 | "net/http/httptest" |
| 1054 | "io" |
| 1055 | "strings" |
| 1056 | "testing" |
| 1057 | "time" |
| 1058 | |
| 1059 | "gitbay.org/gitbay/internal/config" |
| 1060 | ) |
| 1061 | |
| 1062 | // fakeAPNs stands in for Apple. It speaks HTTP/1.1; the real transport is |
| 1063 | // h2 by ALPN, which is stdlib behaviour and not this repository's to test. |
| 1064 | func fakeAPNs(t *testing.T, h http.HandlerFunc) (*Client, *httptest.Server) { |
| 1065 | t.Helper() |
| 1066 | srv := httptest.NewServer(h) |
| 1067 | t.Cleanup(srv.Close) |
| 1068 | t.Setenv("GITBAY_APNS_HOST", strings.TrimPrefix(srv.URL, "http://")) |
| 1069 | c, err := NewClient(config.Push{ |
| 1070 | Enabled: true, KeyID: "K", TeamID: "T", |
| 1071 | Topic: "org.gitbay.gitbay", Environment: "production", |
| 1072 | }) |
| 1073 | if err != nil { |
| 1074 | t.Fatal(err) |
| 1075 | } |
| 1076 | c.key = testKey(t) |
| 1077 | c.tokens = newTokenSource(c.key, "K", "T") |
| 1078 | c.scheme = "http" |
| 1079 | return c, srv |
| 1080 | } |
| 1081 | |
| 1082 | func TestSendShapesTheRequest(t *testing.T) { |
| 1083 | var gotPath, gotTopic, gotType, gotAuth string |
| 1084 | var payload map[string]any |
| 1085 | c, _ := fakeAPNs(t, func(w http.ResponseWriter, r *http.Request) { |
| 1086 | gotPath, gotTopic = r.URL.Path, r.Header.Get("apns-topic") |
| 1087 | gotType, gotAuth = r.Header.Get("apns-push-type"), r.Header.Get("authorization") |
| 1088 | raw, _ := io.ReadAll(r.Body) |
| 1089 | json.Unmarshal(raw, &payload) |
| 1090 | w.WriteHeader(200) |
| 1091 | }) |
| 1092 | res, _, err := c.Send(context.Background(), "DEVTOKEN", "krz/gitbay", "cmc opened issue #12", "krz/gitbay/issues/12") |
| 1093 | if err != nil || res != resultSent { |
| 1094 | t.Fatalf("res = %v, err = %v", res, err) |
| 1095 | } |
| 1096 | if gotPath != "/3/device/DEVTOKEN" { |
| 1097 | t.Fatalf("path = %q", gotPath) |
| 1098 | } |
| 1099 | if gotTopic != "org.gitbay.gitbay" || gotType != "alert" { |
| 1100 | t.Fatalf("topic = %q, push-type = %q", gotTopic, gotType) |
| 1101 | } |
| 1102 | if !strings.HasPrefix(gotAuth, "bearer ") { |
| 1103 | t.Fatalf("authorization = %q", gotAuth) |
| 1104 | } |
| 1105 | aps := payload["aps"].(map[string]any) |
| 1106 | alert := aps["alert"].(map[string]any) |
| 1107 | if alert["title"] != "krz/gitbay" || alert["body"] != "cmc opened issue #12" { |
| 1108 | t.Fatalf("alert = %v", alert) |
| 1109 | } |
| 1110 | if aps["thread-id"] != "krz/gitbay" { |
| 1111 | t.Fatalf("thread-id = %v", aps["thread-id"]) |
| 1112 | } |
| 1113 | if payload["path"] != "krz/gitbay/issues/12" { |
| 1114 | t.Fatalf("path = %v", payload["path"]) |
| 1115 | } |
| 1116 | // Collapsing is wrong here: two comments are two notices. |
| 1117 | if _, ok := payload["apns-collapse-id"]; ok { |
| 1118 | t.Fatal("collapse id set") |
| 1119 | } |
| 1120 | } |
| 1121 | |
| 1122 | func TestSendMapsResponses(t *testing.T) { |
| 1123 | cases := []struct { |
| 1124 | name string |
| 1125 | status int |
| 1126 | body string |
| 1127 | retryAfter string |
| 1128 | want result |
| 1129 | wantAfter time.Duration |
| 1130 | }{ |
| 1131 | {"ok", 200, "", "", resultSent, 0}, |
| 1132 | {"gone", 410, `{"reason":"Unregistered"}`, "", resultReap, 0}, |
| 1133 | {"bad token", 400, `{"reason":"BadDeviceToken"}`, "", resultReap, 0}, |
| 1134 | {"other 400 is permanent", 400, `{"reason":"PayloadTooLarge"}`, "", resultDead, 0}, |
| 1135 | {"forbidden is permanent", 403, `{"reason":"InvalidProviderToken"}`, "", resultDead, 0}, |
| 1136 | {"too many requests retries", 429, `{"reason":"TooManyRequests"}`, "7", resultRetry, 7 * time.Second}, |
| 1137 | {"server error retries", 503, `{"reason":"ServiceUnavailable"}`, "", resultRetry, 0}, |
| 1138 | } |
| 1139 | for _, tc := range cases { |
| 1140 | t.Run(tc.name, func(t *testing.T) { |
| 1141 | c, _ := fakeAPNs(t, func(w http.ResponseWriter, r *http.Request) { |
| 1142 | if tc.retryAfter != "" { |
| 1143 | w.Header().Set("Retry-After", tc.retryAfter) |
| 1144 | } |
| 1145 | w.WriteHeader(tc.status) |
| 1146 | io.WriteString(w, tc.body) |
| 1147 | }) |
| 1148 | res, after, err := c.Send(context.Background(), "T", "t", "b", "p") |
| 1149 | if err != nil && tc.want != resultDead && tc.want != resultReap { |
| 1150 | t.Fatalf("err = %v", err) |
| 1151 | } |
| 1152 | if res != tc.want { |
| 1153 | t.Fatalf("res = %v, want %v", res, tc.want) |
| 1154 | } |
| 1155 | if after != tc.wantAfter { |
| 1156 | t.Fatalf("retryAfter = %v, want %v", after, tc.wantAfter) |
| 1157 | } |
| 1158 | }) |
| 1159 | } |
| 1160 | } |
| 1161 | ``` |
| 1162 | |
| 1163 | - [ ] **Step 2: Run the test to verify it fails** |
| 1164 | |
| 1165 | Run: `go test ./internal/push/ -run TestSend -v` |
| 1166 | Expected: FAIL — `NewClient` undefined. |
| 1167 | |
| 1168 | - [ ] **Step 3: Write the implementation** |
| 1169 | |
| 1170 | `internal/push/apns.go`: |
| 1171 | |
| 1172 | ```go |
| 1173 | package push |
| 1174 | |
| 1175 | import ( |
| 1176 | "bytes" |
| 1177 | "context" |
| 1178 | "crypto/ecdsa" |
| 1179 | "encoding/json" |
| 1180 | "fmt" |
| 1181 | "io" |
| 1182 | "net/http" |
| 1183 | "strconv" |
| 1184 | "time" |
| 1185 | |
| 1186 | "gitbay.org/gitbay/internal/config" |
| 1187 | ) |
| 1188 | |
| 1189 | // result is what one send means for the queue row. |
| 1190 | type result int |
| 1191 | |
| 1192 | const ( |
| 1193 | resultSent result = iota // delivered |
| 1194 | resultRetry // transient; back off and try again |
| 1195 | resultReap // Apple says the token is dead; drop the device |
| 1196 | resultDead // permanent for this payload; dead-letter it |
| 1197 | ) |
| 1198 | |
| 1199 | // maxBodyBytes keeps an alert inside APNs' 4KB payload limit with room |
| 1200 | // for the rest of the JSON. A summary longer than this is cut rather |
| 1201 | // than rejected. |
| 1202 | const maxBodyBytes = 3000 |
| 1203 | |
| 1204 | type Client struct { |
| 1205 | http *http.Client |
| 1206 | tokens *tokenSource |
| 1207 | key *ecdsa.PrivateKey |
| 1208 | host string |
| 1209 | scheme string |
| 1210 | topic string |
| 1211 | } |
| 1212 | |
| 1213 | func NewClient(cfg config.Push) (*Client, error) { |
| 1214 | c := &Client{ |
| 1215 | // stdlib negotiates HTTP/2 over ALPN, which is what APNs |
| 1216 | // requires; no explicit http2 transport is needed. |
| 1217 | http: &http.Client{Timeout: 30 * time.Second}, |
| 1218 | host: cfg.Host(), |
| 1219 | scheme: "https", |
| 1220 | topic: cfg.Topic, |
| 1221 | } |
| 1222 | if cfg.KeyFile != "" { |
| 1223 | key, err := config.LoadAPNSKey(cfg.KeyFile) |
| 1224 | if err != nil { |
| 1225 | return nil, err |
| 1226 | } |
| 1227 | c.key = key |
| 1228 | c.tokens = newTokenSource(key, cfg.KeyID, cfg.TeamID) |
| 1229 | } |
| 1230 | return c, nil |
| 1231 | } |
| 1232 | ``` |
| 1233 | |
| 1234 | `c.tokens` is nil when `KeyFile` is empty, and `Send` would panic on it. |
| 1235 | Production cannot reach that: config validation requires `key_file` |
| 1236 | whenever `push.enabled`, and `Deliverer` is only started when it is. The |
| 1237 | tests above set `c.tokens` themselves. Leave it rather than adding a nil |
| 1238 | check that can only fire in a test that forgot one. |
| 1239 | |
| 1240 | ```go |
| 1241 | |
| 1242 | // Send delivers one alert. The returned duration is the server's |
| 1243 | // Retry-After when it gave one, zero otherwise. |
| 1244 | func (c *Client) Send(ctx context.Context, token, title, body, path string) (result, time.Duration, error) { |
| 1245 | if len(body) > maxBodyBytes { |
| 1246 | body = body[:maxBodyBytes] |
| 1247 | } |
| 1248 | payload, err := json.Marshal(map[string]any{ |
| 1249 | "aps": map[string]any{ |
| 1250 | "alert": map[string]string{"title": title, "body": body}, |
| 1251 | "sound": "default", |
| 1252 | "thread-id": title, |
| 1253 | }, |
| 1254 | "path": path, |
| 1255 | }) |
| 1256 | if err != nil { |
| 1257 | return resultDead, 0, err |
| 1258 | } |
| 1259 | bearer, err := c.tokens.token() |
| 1260 | if err != nil { |
| 1261 | return resultRetry, 0, err |
| 1262 | } |
| 1263 | url := c.scheme + "://" + c.host + "/3/device/" + token |
| 1264 | req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, bytes.NewReader(payload)) |
| 1265 | if err != nil { |
| 1266 | return resultDead, 0, err |
| 1267 | } |
| 1268 | req.Header.Set("authorization", "bearer "+bearer) |
| 1269 | req.Header.Set("apns-topic", c.topic) |
| 1270 | req.Header.Set("apns-push-type", "alert") |
| 1271 | req.Header.Set("apns-priority", "10") |
| 1272 | req.Header.Set("content-type", "application/json") |
| 1273 | |
| 1274 | resp, err := c.http.Do(req) |
| 1275 | if err != nil { |
| 1276 | return resultRetry, 0, err |
| 1277 | } |
| 1278 | defer resp.Body.Close() |
| 1279 | raw, _ := io.ReadAll(io.LimitReader(resp.Body, 4096)) |
| 1280 | |
| 1281 | var apnsErr struct { |
| 1282 | Reason string `json:"reason"` |
| 1283 | } |
| 1284 | json.Unmarshal(raw, &apnsErr) |
| 1285 | |
| 1286 | var after time.Duration |
| 1287 | if v := resp.Header.Get("Retry-After"); v != "" { |
| 1288 | if n, err := strconv.Atoi(v); err == nil && n > 0 { |
| 1289 | after = time.Duration(n) * time.Second |
| 1290 | } |
| 1291 | } |
| 1292 | |
| 1293 | switch { |
| 1294 | case resp.StatusCode == http.StatusOK: |
| 1295 | return resultSent, 0, nil |
| 1296 | case resp.StatusCode == http.StatusGone, |
| 1297 | apnsErr.Reason == "BadDeviceToken", |
| 1298 | apnsErr.Reason == "Unregistered": |
| 1299 | // Apple is authoritative about which tokens are live. |
| 1300 | return resultReap, 0, fmt.Errorf("apns %d %s", resp.StatusCode, apnsErr.Reason) |
| 1301 | case resp.StatusCode == http.StatusTooManyRequests, resp.StatusCode >= 500: |
| 1302 | return resultRetry, after, fmt.Errorf("apns %d %s", resp.StatusCode, apnsErr.Reason) |
| 1303 | default: |
| 1304 | // Retrying a rejected payload will not fix it. |
| 1305 | return resultDead, 0, fmt.Errorf("apns %d %s", resp.StatusCode, apnsErr.Reason) |
| 1306 | } |
| 1307 | } |
| 1308 | ``` |
| 1309 | |
| 1310 | - [ ] **Step 4: Run the tests to verify they pass** |
| 1311 | |
| 1312 | Run: `go test ./internal/push/ -v` |
| 1313 | Expected: PASS, all tests. |
| 1314 | |
| 1315 | - [ ] **Step 5: Commit** |
| 1316 | |
| 1317 | ```bash |
| 1318 | git add internal/push/apns.go internal/push/apns_test.go |
| 1319 | git -c commit.gpgsign=true commit -m "push: the APNs client |
| 1320 | |
| 1321 | POSTs one alert per call and maps the response: 200 sent, 410 and |
| 1322 | BadDeviceToken reap the device, 429 and 5xx retry honouring |
| 1323 | Retry-After, everything else dead-letters. |
| 1324 | |
| 1325 | Ref #89" |
| 1326 | ``` |
| 1327 | |
| 1328 | --- |
| 1329 | |
| 1330 | ### Task 6: The drainer, and gitbayd wiring |
| 1331 | |
| 1332 | **Files:** |
| 1333 | - Create: `internal/push/push.go` |
| 1334 | - Modify: `cmd/gitbayd/main.go:175-178` |
| 1335 | - Test: `internal/push/push_test.go` |
| 1336 | |
| 1337 | **Interfaces:** |
| 1338 | - Consumes: `Client`, `result` constants from Task 5; the store queue functions from Task 2. |
| 1339 | - Produces: |
| 1340 | - `type Deliverer struct { St *store.Store; Cl *Client; RetryBase time.Duration; MaxAttempts int }` |
| 1341 | - `func New(st *store.Store, cfg config.Push, retryBase time.Duration) (*Deliverer, error)` |
| 1342 | - `func (d *Deliverer) Run(ctx context.Context)` |
| 1343 | - `func (d *Deliverer) drain(ctx context.Context)` — one pass, for tests |
| 1344 | - `const DefaultMaxAttempts = 5` |
| 1345 | |
| 1346 | - [ ] **Step 1: Write the failing test** |
| 1347 | |
| 1348 | `internal/push/push_test.go`: |
| 1349 | |
| 1350 | ```go |
| 1351 | package push |
| 1352 | |
| 1353 | import ( |
| 1354 | "context" |
| 1355 | "net/http" |
| 1356 | "testing" |
| 1357 | "time" |
| 1358 | ) |
| 1359 | |
| 1360 | func TestDrainSendsAndMarks(t *testing.T) { |
| 1361 | var hits int |
| 1362 | c, _ := fakeAPNs(t, func(w http.ResponseWriter, r *http.Request) { |
| 1363 | hits++ |
| 1364 | w.WriteHeader(200) |
| 1365 | }) |
| 1366 | st := testStoreWithQueuedPush(t, "tok-a") |
| 1367 | d := &Deliverer{St: st, Cl: c, RetryBase: time.Millisecond, MaxAttempts: 5} |
| 1368 | |
| 1369 | d.drain(context.Background()) |
| 1370 | |
| 1371 | if hits != 1 { |
| 1372 | t.Fatalf("sent %d times, want 1", hits) |
| 1373 | } |
| 1374 | if due, _ := st.DuePush(20); len(due) != 0 { |
| 1375 | t.Fatalf("row still due after a 200") |
| 1376 | } |
| 1377 | } |
| 1378 | |
| 1379 | func TestDrainReapsADeadToken(t *testing.T) { |
| 1380 | c, _ := fakeAPNs(t, func(w http.ResponseWriter, r *http.Request) { |
| 1381 | w.WriteHeader(410) |
| 1382 | w.Write([]byte(`{"reason":"Unregistered"}`)) |
| 1383 | }) |
| 1384 | st := testStoreWithQueuedPush(t, "tok-a") |
| 1385 | d := &Deliverer{St: st, Cl: c, RetryBase: time.Millisecond, MaxAttempts: 5} |
| 1386 | |
| 1387 | d.drain(context.Background()) |
| 1388 | |
| 1389 | if due, _ := st.DuePush(20); len(due) != 0 { |
| 1390 | t.Fatalf("queue survived the reap") |
| 1391 | } |
| 1392 | // The device is gone, not merely its queue row. |
| 1393 | if n := countPushDevices(t, st); n != 0 { |
| 1394 | t.Fatalf("%d devices left after 410", n) |
| 1395 | } |
| 1396 | } |
| 1397 | |
| 1398 | func TestDrainBacksOffThenDeadLetters(t *testing.T) { |
| 1399 | c, _ := fakeAPNs(t, func(w http.ResponseWriter, r *http.Request) { |
| 1400 | w.WriteHeader(503) |
| 1401 | }) |
| 1402 | st := testStoreWithQueuedPush(t, "tok-a") |
| 1403 | d := &Deliverer{St: st, Cl: c, RetryBase: time.Nanosecond, MaxAttempts: 3} |
| 1404 | |
| 1405 | // Three passes: two back off, the third gives up. |
| 1406 | for i := 0; i < 3; i++ { |
| 1407 | d.drain(context.Background()) |
| 1408 | } |
| 1409 | if due, _ := st.DuePush(20); len(due) != 0 { |
| 1410 | t.Fatalf("row still due after MaxAttempts") |
| 1411 | } |
| 1412 | // A transient failure must not take the device with it. |
| 1413 | if n := countPushDevices(t, st); n != 1 { |
| 1414 | t.Fatalf("device reaped on a 503") |
| 1415 | } |
| 1416 | } |
| 1417 | ``` |
| 1418 | |
| 1419 | Write `testStoreWithQueuedPush` and `countPushDevices` as helpers in this |
| 1420 | file. `testStoreWithQueuedPush` opens a store the way `internal/store`'s |
| 1421 | own tests do, creates a user, calls `AddPushDevice` and `EnqueuePush`, |
| 1422 | and returns the store. If opening a store from `internal/push` is |
| 1423 | awkward, put the helpers in `internal/store/export_test.go` style — check |
| 1424 | what `internal/webhook`'s tests do for the same problem and follow it |
| 1425 | rather than inventing a third way. |
| 1426 | |
| 1427 | - [ ] **Step 2: Run the test to verify it fails** |
| 1428 | |
| 1429 | Run: `go test ./internal/push/ -run TestDrain -v` |
| 1430 | Expected: FAIL — `Deliverer` undefined. |
| 1431 | |
| 1432 | - [ ] **Step 3: Write the implementation** |
| 1433 | |
| 1434 | `internal/push/push.go`: |
| 1435 | |
| 1436 | ```go |
| 1437 | package push |
| 1438 | |
| 1439 | import ( |
| 1440 | "context" |
| 1441 | "log/slog" |
| 1442 | "time" |
| 1443 | |
| 1444 | "gitbay.org/gitbay/internal/config" |
| 1445 | "gitbay.org/gitbay/internal/store" |
| 1446 | ) |
| 1447 | |
| 1448 | // DefaultMaxAttempts matches the mailer's: a flaky APNs delays a |
| 1449 | // notification rather than losing it, up to a point. |
| 1450 | const DefaultMaxAttempts = 5 |
| 1451 | |
| 1452 | type Deliverer struct { |
| 1453 | St *store.Store |
| 1454 | Cl *Client |
| 1455 | RetryBase time.Duration |
| 1456 | MaxAttempts int |
| 1457 | } |
| 1458 | |
| 1459 | func New(st *store.Store, cfg config.Push, retryBase time.Duration) (*Deliverer, error) { |
| 1460 | cl, err := NewClient(cfg) |
| 1461 | if err != nil { |
| 1462 | return nil, err |
| 1463 | } |
| 1464 | return &Deliverer{St: st, Cl: cl, RetryBase: retryBase, MaxAttempts: DefaultMaxAttempts}, nil |
| 1465 | } |
| 1466 | |
| 1467 | // Run drains the push queue until ctx is done. |
| 1468 | func (d *Deliverer) Run(ctx context.Context) { |
| 1469 | tick := time.NewTicker(2 * time.Second) |
| 1470 | defer tick.Stop() |
| 1471 | for { |
| 1472 | select { |
| 1473 | case <-ctx.Done(): |
| 1474 | return |
| 1475 | case <-tick.C: |
| 1476 | d.drain(ctx) |
| 1477 | } |
| 1478 | } |
| 1479 | } |
| 1480 | |
| 1481 | func (d *Deliverer) drain(ctx context.Context) { |
| 1482 | due, err := d.St.DuePush(20) |
| 1483 | if err != nil { |
| 1484 | slog.Error("push: listing due", "err", err) |
| 1485 | return |
| 1486 | } |
| 1487 | for _, q := range due { |
| 1488 | res, after, sendErr := d.Cl.Send(ctx, q.Token, q.Title, q.Body, q.Path) |
| 1489 | msg := "" |
| 1490 | if sendErr != nil { |
| 1491 | msg = sendErr.Error() |
| 1492 | } |
| 1493 | switch res { |
| 1494 | case resultSent: |
| 1495 | d.St.MarkPushSent(q.ID) |
| 1496 | case resultReap: |
| 1497 | // The queued rows cascade with the device. |
| 1498 | if err := d.St.DeletePushDeviceByToken(q.Token); err != nil { |
| 1499 | slog.Error("push: reaping device", "device", q.DeviceID, "err", err) |
| 1500 | } |
| 1501 | case resultRetry: |
| 1502 | attempt := q.Attempts + 1 |
| 1503 | if attempt >= d.MaxAttempts { |
| 1504 | d.St.MarkPushFailed(q.ID, msg, nil) |
| 1505 | // The device id, never the token. |
| 1506 | slog.Warn("push dead-lettered", |
| 1507 | "push", q.ID, "device", q.DeviceID, "attempts", attempt, "err", msg) |
| 1508 | continue |
| 1509 | } |
| 1510 | wait := after |
| 1511 | if wait == 0 { |
| 1512 | wait = d.RetryBase << (attempt - 1) |
| 1513 | } |
| 1514 | next := time.Now().Add(wait) |
| 1515 | d.St.MarkPushFailed(q.ID, msg, &next) |
| 1516 | default: // resultDead |
| 1517 | d.St.MarkPushFailed(q.ID, msg, nil) |
| 1518 | slog.Warn("push rejected", "push", q.ID, "device", q.DeviceID, "err", msg) |
| 1519 | } |
| 1520 | } |
| 1521 | } |
| 1522 | ``` |
| 1523 | |
| 1524 | - [ ] **Step 4: Wire it into gitbayd** |
| 1525 | |
| 1526 | In `cmd/gitbayd/main.go`, beside the mailer at line 177: |
| 1527 | |
| 1528 | ```go |
| 1529 | if cfg.Push.Enabled { |
| 1530 | p, err := push.New(st, cfg.Push, retryBase) |
| 1531 | if err != nil { |
| 1532 | // Config validation already parsed the key, so this |
| 1533 | // is not a misconfiguration; fail loudly rather than |
| 1534 | // running with a silent delivery route. |
| 1535 | slog.Error("push: starting deliverer", "err", err) |
| 1536 | } else { |
| 1537 | go p.Run(whCtx) |
| 1538 | } |
| 1539 | } |
| 1540 | ``` |
| 1541 | |
| 1542 | Add `"gitbay.org/gitbay/internal/push"` to the file's imports. |
| 1543 | |
| 1544 | - [ ] **Step 5: Run the tests to verify they pass** |
| 1545 | |
| 1546 | Run: `go test ./internal/push/ -v && go build ./... && go vet ./...` |
| 1547 | Expected: PASS and clean. |
| 1548 | |
| 1549 | - [ ] **Step 6: Commit** |
| 1550 | |
| 1551 | ```bash |
| 1552 | git add internal/push/push.go internal/push/push_test.go cmd/gitbayd/main.go |
| 1553 | git -c commit.gpgsign=true commit -m "push: drain the queue, started by gitbayd |
| 1554 | |
| 1555 | Two-second ticker in the mailer's shape. A reap drops the device and |
| 1556 | its queued rows cascade; a retry honours Retry-After when APNs gave |
| 1557 | one. Log lines name the device id, never the token. |
| 1558 | |
| 1559 | Ref #89" |
| 1560 | ``` |
| 1561 | |
| 1562 | --- |
| 1563 | |
| 1564 | ### Task 7: The control commands |
| 1565 | |
| 1566 | **Files:** |
| 1567 | - Modify: `internal/control/notifications.go` |
| 1568 | - Modify: `cmd/gitbay/main.go:64-73` |
| 1569 | - Test: `internal/control/notifications_test.go` |
| 1570 | |
| 1571 | **Interfaces:** |
| 1572 | - Consumes: the store device functions from Task 1. |
| 1573 | - Produces: registry entries `notifications device add|list|remove` and `notifications settings push`; `emitNotificationSettings` gains a `push` key. |
| 1574 | |
| 1575 | - [ ] **Step 1: Write the failing test** |
| 1576 | |
| 1577 | Add to `internal/control/notifications_test.go` (create it if absent, |
| 1578 | following `internal/control/mr_test.go` for how a `Ctx` is built): |
| 1579 | |
| 1580 | ```go |
| 1581 | func TestNotificationsDeviceAddReadsStdin(t *testing.T) { |
| 1582 | c := testCtx(t, "alice") |
| 1583 | c.Stdin = strings.NewReader("DEVTOKEN\n") |
| 1584 | if code := runNotificationsDeviceAdd(c, []string{"--label", "iphone"}); code != 0 { |
| 1585 | t.Fatalf("exit %d", code) |
| 1586 | } |
| 1587 | devices, _ := c.Store.PushDevices(c.User.ID) |
| 1588 | if len(devices) != 1 || devices[0].Token != "DEVTOKEN" { |
| 1589 | t.Fatalf("got %+v", devices) |
| 1590 | } |
| 1591 | if devices[0].Label != "iphone" { |
| 1592 | t.Fatalf("label = %q", devices[0].Label) |
| 1593 | } |
| 1594 | } |
| 1595 | |
| 1596 | func TestNotificationsDeviceListTruncatesTheToken(t *testing.T) { |
| 1597 | c := testCtx(t, "alice") |
| 1598 | long := strings.Repeat("a", 64) |
| 1599 | c.Store.AddPushDevice(c.User.ID, long, "iphone") |
| 1600 | var out bytes.Buffer |
| 1601 | c.Stdout = &out |
| 1602 | if code := runNotificationsDeviceList(c, nil); code != 0 { |
| 1603 | t.Fatalf("exit %d", code) |
| 1604 | } |
| 1605 | if strings.Contains(out.String(), long) { |
| 1606 | t.Fatal("the full token was printed") |
| 1607 | } |
| 1608 | } |
| 1609 | |
| 1610 | func TestNotificationsSettingsShowsPush(t *testing.T) { |
| 1611 | c := testCtx(t, "alice") |
| 1612 | var out bytes.Buffer |
| 1613 | c.Stdout, c.JSON = &out, true |
| 1614 | if code := runNotificationsSettingsShow(c, nil); code != 0 { |
| 1615 | t.Fatalf("exit %d", code) |
| 1616 | } |
| 1617 | if !strings.Contains(out.String(), `"push":true`) { |
| 1618 | t.Fatalf("no push key: %s", out.String()) |
| 1619 | } |
| 1620 | } |
| 1621 | ``` |
| 1622 | |
| 1623 | - [ ] **Step 2: Run the test to verify it fails** |
| 1624 | |
| 1625 | Run: `go test ./internal/control/ -run TestNotifications -v` |
| 1626 | Expected: FAIL — `runNotificationsDeviceAdd` undefined. |
| 1627 | |
| 1628 | - [ ] **Step 3: Register and implement the commands** |
| 1629 | |
| 1630 | In the `init()` of `internal/control/notifications.go`: |
| 1631 | |
| 1632 | ```go |
| 1633 | register(Command{Path: []string{"notifications", "device", "add"}, |
| 1634 | Summary: "register an Apple device for push, token on stdin", |
| 1635 | Usage: "notifications device add [--label <name>] < token", |
| 1636 | // Mandatory: without it control.go swaps in an empty reader and |
| 1637 | // this command stores an empty token without erroring. |
| 1638 | ReadsStdin: true, Run: runNotificationsDeviceAdd}) |
| 1639 | register(Command{Path: []string{"notifications", "device", "list"}, |
| 1640 | Summary: "your registered devices", |
| 1641 | Usage: "notifications device list", |
| 1642 | ReadOnly: true, Run: runNotificationsDeviceList}) |
| 1643 | register(Command{Path: []string{"notifications", "device", "remove"}, |
| 1644 | Summary: "deregister a device", |
| 1645 | Usage: "notifications device remove <id>", Run: runNotificationsDeviceRemove}) |
| 1646 | register(Command{Path: []string{"notifications", "settings", "push"}, |
| 1647 | Summary: "activity on your registered devices as well as the inbox", |
| 1648 | Usage: "notifications settings push on|off", Run: runNotificationsSettingsPush}) |
| 1649 | ``` |
| 1650 | |
| 1651 | And the implementations: |
| 1652 | |
| 1653 | ```go |
| 1654 | // maxDeviceTokenBytes is well past APNs' 32-byte token rendered as 64 hex |
| 1655 | // characters, and stops a stdin that is not a token from becoming a row. |
| 1656 | const maxDeviceTokenBytes = 512 |
| 1657 | |
| 1658 | func runNotificationsDeviceAdd(c *Ctx, args []string) int { |
| 1659 | f, err := parseFlags(args, flagSpec{Values: []string{"--label"}, Usage: c.Cmd.Usage}) |
| 1660 | if err != nil { |
| 1661 | return c.fail(protocol.ExitUsage, "%v", err) |
| 1662 | } |
| 1663 | if len(f.Pos) != 0 { |
| 1664 | return c.usage() |
| 1665 | } |
| 1666 | raw, err := io.ReadAll(io.LimitReader(c.Stdin, maxDeviceTokenBytes+1)) |
| 1667 | if err != nil { |
| 1668 | return c.fail(protocol.ExitFailure, "reading stdin: %v", err) |
| 1669 | } |
| 1670 | token := strings.TrimSpace(string(raw)) |
| 1671 | if token == "" { |
| 1672 | return c.usageWith("no device token on stdin") |
| 1673 | } |
| 1674 | if len(token) > maxDeviceTokenBytes { |
| 1675 | return c.fail(protocol.ExitUsage, "device token is too long") |
| 1676 | } |
| 1677 | if _, err := c.Store.AddPushDevice(c.User.ID, token, f.Value("--label")); err != nil { |
| 1678 | return c.fail(protocol.ExitFailure, "%v", err) |
| 1679 | } |
| 1680 | return c.emit(map[string]string{"status": "registered"}, func(w io.Writer) { |
| 1681 | fmt.Fprintln(w, "device registered") |
| 1682 | }) |
| 1683 | } |
| 1684 | |
| 1685 | func runNotificationsDeviceList(c *Ctx, args []string) int { |
| 1686 | if len(args) != 0 { |
| 1687 | return c.usage() |
| 1688 | } |
| 1689 | devices, err := c.Store.PushDevices(c.User.ID) |
| 1690 | if err != nil { |
| 1691 | return c.fail(protocol.ExitFailure, "%v", err) |
| 1692 | } |
| 1693 | type row struct { |
| 1694 | ID int64 `json:"id"` |
| 1695 | Label string `json:"label"` |
| 1696 | Token string `json:"token"` // truncated; a token is not echoed in full |
| 1697 | Added string `json:"added"` |
| 1698 | } |
| 1699 | rows := make([]row, 0, len(devices)) |
| 1700 | for _, d := range devices { |
| 1701 | rows = append(rows, row{ID: d.ID, Label: d.Label, |
| 1702 | Token: shortToken(d.Token), Added: d.CreatedAt}) |
| 1703 | } |
| 1704 | return c.emit(rows, func(w io.Writer) { |
| 1705 | for _, r := range rows { |
| 1706 | fmt.Fprintf(w, "%d\t%s\t%s\t%s\n", r.ID, r.Label, r.Token, r.Added) |
| 1707 | } |
| 1708 | }) |
| 1709 | } |
| 1710 | |
| 1711 | // shortToken renders a device token as its first eight characters. Enough |
| 1712 | // to tell two devices apart in a list, not enough to push to one. |
| 1713 | func shortToken(t string) string { |
| 1714 | if len(t) <= 8 { |
| 1715 | return t |
| 1716 | } |
| 1717 | return t[:8] + "…" |
| 1718 | } |
| 1719 | |
| 1720 | func runNotificationsDeviceRemove(c *Ctx, args []string) int { |
| 1721 | if len(args) != 1 { |
| 1722 | return c.usage() |
| 1723 | } |
| 1724 | id, err := strconv.ParseInt(args[0], 10, 64) |
| 1725 | if err != nil { |
| 1726 | return c.usageWith("device id must be a number") |
| 1727 | } |
| 1728 | if err := c.Store.RemovePushDevice(c.User.ID, id); err != nil { |
| 1729 | if errors.Is(err, store.ErrNotFound) { |
| 1730 | return c.fail(protocol.ExitNotFound, "no such device; notifications device list shows yours") |
| 1731 | } |
| 1732 | return c.fail(protocol.ExitFailure, "%v", err) |
| 1733 | } |
| 1734 | return c.emit(map[string]string{"status": "removed"}, func(w io.Writer) { |
| 1735 | fmt.Fprintln(w, "device removed") |
| 1736 | }) |
| 1737 | } |
| 1738 | |
| 1739 | func runNotificationsSettingsPush(c *Ctx, args []string) int { |
| 1740 | if len(args) != 1 || (args[0] != "on" && args[0] != "off") { |
| 1741 | return c.usage() |
| 1742 | } |
| 1743 | if err := c.Store.SetPushEnabled(c.User.ID, args[0] == "on"); err != nil { |
| 1744 | return c.fail(protocol.ExitFailure, "%v", err) |
| 1745 | } |
| 1746 | return emitNotificationSettings(c) |
| 1747 | } |
| 1748 | ``` |
| 1749 | |
| 1750 | Add `"errors"` and `"gitbay.org/gitbay/internal/store"` to the imports if |
| 1751 | the file lacks them. |
| 1752 | |
| 1753 | - [ ] **Step 4: Extend `emitNotificationSettings`** |
| 1754 | |
| 1755 | Replace the body of `emitNotificationSettings` so it reads `push` too and |
| 1756 | adds it to both outputs: |
| 1757 | |
| 1758 | ```go |
| 1759 | push, err := c.Store.PushEnabled(c.User.ID) |
| 1760 | if err != nil { |
| 1761 | return c.fail(protocol.ExitFailure, "%v", err) |
| 1762 | } |
| 1763 | return c.emit(map[string]bool{"mail": mail, "watch": watch, "push": push}, func(w io.Writer) { |
| 1764 | ... |
| 1765 | fmt.Fprintf(w, "mail: %s\nwatch: %s\npush: %s\n", onOff(mail), onOff(watch), onOff(push)) |
| 1766 | }) |
| 1767 | ``` |
| 1768 | |
| 1769 | - [ ] **Step 5: Add the CLI passthroughs** |
| 1770 | |
| 1771 | In `cmd/gitbay/main.go`, inside the `notifications` group around line 64, |
| 1772 | add a `device` subgroup and the settings entry: |
| 1773 | |
| 1774 | ```go |
| 1775 | group("device", "Apple devices registered for push", |
| 1776 | pass("add", "register a device, token on stdin: [--label name]", |
| 1777 | passOpts{server: []string{"notifications", "device", "add"}, stdin: true}), |
| 1778 | pass("list", "your registered devices", |
| 1779 | passOpts{server: []string{"notifications", "device", "list"}}), |
| 1780 | pass("remove", "deregister a device: <id>", |
| 1781 | passOpts{server: []string{"notifications", "device", "remove"}}), |
| 1782 | ), |
| 1783 | ``` |
| 1784 | |
| 1785 | and beside `mail` and `watch` in the settings group: |
| 1786 | |
| 1787 | ```go |
| 1788 | pass("push", "activity on your registered devices: on|off", passOpts{server: []string{"notifications", "settings", "push"}}), |
| 1789 | ``` |
| 1790 | |
| 1791 | Check `passOpts`' real field for a stdin-reading command against how |
| 1792 | `snippet create` is registered in the same file — use that name, not |
| 1793 | `stdin:` if it differs. |
| 1794 | |
| 1795 | - [ ] **Step 6: Run the tests to verify they pass** |
| 1796 | |
| 1797 | Run: `go test ./internal/control/ ./cmd/gitbay/ -v` |
| 1798 | Expected: PASS. Four registry tests exercise the new commands without |
| 1799 | being edited: `TestStdinCommandsReadStdin` (which fails if `device add` |
| 1800 | lacks `ReadsStdin`), `TestReadOnlyCommandsWriteNothing`, the |
| 1801 | `cmd/gitbay` coverage test (which fails without the `pass()` entries), |
| 1802 | and the usage-literal check. |
| 1803 | |
| 1804 | - [ ] **Step 7: Commit** |
| 1805 | |
| 1806 | ```bash |
| 1807 | git add internal/control/notifications.go internal/control/notifications_test.go cmd/gitbay/main.go |
| 1808 | git -c commit.gpgsign=true commit -m "control: notifications device and settings push |
| 1809 | |
| 1810 | Token on stdin, never argv. device list truncates the token to eight |
| 1811 | characters: enough to tell two devices apart, not enough to push to |
| 1812 | one. settings show gains a third key. |
| 1813 | |
| 1814 | Ref #89" |
| 1815 | ``` |
| 1816 | |
| 1817 | --- |
| 1818 | |
| 1819 | ### Task 8: Push as the third route in `notify()` |
| 1820 | |
| 1821 | **Files:** |
| 1822 | - Modify: `internal/control/notifications.go:66-85` (`notify`) |
| 1823 | - Test: `internal/control/notifications_test.go` |
| 1824 | |
| 1825 | **Interfaces:** |
| 1826 | - Consumes: `EnqueuePush` from Task 2. |
| 1827 | - Produces: `func pushTitle(n notice) string` and `func pushBody(n notice) string`. |
| 1828 | |
| 1829 | - [ ] **Step 1: Write the failing test** |
| 1830 | |
| 1831 | ```go |
| 1832 | func TestNotifyQueuesPush(t *testing.T) { |
| 1833 | c, repo, bob := testRepoWithWatcher(t) // alice acts, bob watches |
| 1834 | c.Store.AddPushDevice(bob, "tok-b", "iphone") |
| 1835 | |
| 1836 | notify(c, []int64{bob}, notice{repo: repo, kind: "issue", |
| 1837 | subject: "[alice/app] #1: title", |
| 1838 | action: "opened issue #1", |
| 1839 | path: "alice/app/issues/1"}) |
| 1840 | |
| 1841 | due, err := c.Store.DuePush(20) |
| 1842 | if err != nil { |
| 1843 | t.Fatalf("DuePush: %v", err) |
| 1844 | } |
| 1845 | if len(due) != 1 { |
| 1846 | t.Fatalf("want one queued push, got %d", len(due)) |
| 1847 | } |
| 1848 | // The push body is the inbox row's summary, so the two surfaces |
| 1849 | // cannot disagree about what happened. |
| 1850 | if due[0].Title != "alice/app" { |
| 1851 | t.Fatalf("title = %q", due[0].Title) |
| 1852 | } |
| 1853 | if due[0].Body != "alice opened issue #1" { |
| 1854 | t.Fatalf("body = %q", due[0].Body) |
| 1855 | } |
| 1856 | if due[0].Path != "alice/app/issues/1" { |
| 1857 | t.Fatalf("path = %q", due[0].Path) |
| 1858 | } |
| 1859 | } |
| 1860 | |
| 1861 | func TestNotifyQueuesNoPushForTheActor(t *testing.T) { |
| 1862 | c, repo, _ := testRepoWithWatcher(t) |
| 1863 | c.Store.AddPushDevice(c.User.ID, "tok-self", "iphone") |
| 1864 | |
| 1865 | notify(c, []int64{c.User.ID}, notice{repo: repo, kind: "issue", |
| 1866 | subject: "s", action: "opened issue #1", path: "alice/app/issues/1"}) |
| 1867 | |
| 1868 | // NotifyRecipients already drops the actor; push inherits that and |
| 1869 | // must not find its own way around it. |
| 1870 | if due, _ := c.Store.DuePush(20); len(due) != 0 { |
| 1871 | t.Fatalf("queued a push to the actor") |
| 1872 | } |
| 1873 | } |
| 1874 | ``` |
| 1875 | |
| 1876 | Write `testRepoWithWatcher` to return a `*Ctx` acting as alice, a |
| 1877 | `store.Repo` she owns, and bob's user id with a watch row on it. Follow |
| 1878 | whatever `internal/control`'s existing tests do to build a repo. |
| 1879 | |
| 1880 | - [ ] **Step 2: Run the test to verify it fails** |
| 1881 | |
| 1882 | Run: `go test ./internal/control/ -run TestNotifyQueues -v` |
| 1883 | Expected: FAIL — one queued push wanted, none found. |
| 1884 | |
| 1885 | - [ ] **Step 3: Write the implementation** |
| 1886 | |
| 1887 | In `notify()`, inside the existing `for _, id := range recipients` loop, |
| 1888 | after `AddNotice` and before the `if !sendMail { continue }`: |
| 1889 | |
| 1890 | ```go |
| 1891 | c.Store.EnqueuePush(id, pushTitle(n), pushBody(c.User.Username, n), n.path) |
| 1892 | ``` |
| 1893 | |
| 1894 | Putting it above the `continue` matters — an instance without SMTP still |
| 1895 | pushes. |
| 1896 | |
| 1897 | And beside `noticeBody`: |
| 1898 | |
| 1899 | ```go |
| 1900 | // pushTitle and pushBody are the alert's two lines. The body is built |
| 1901 | // from the same two values AddNotice files, so the alert and the inbox |
| 1902 | // row cannot disagree about what happened. The title is the repository, |
| 1903 | // which also groups a repository's notices in Notification Center. |
| 1904 | func pushTitle(n notice) string { return n.repo.Path() } |
| 1905 | |
| 1906 | func pushBody(actor string, n notice) string { return actor + " " + n.action } |
| 1907 | ``` |
| 1908 | |
| 1909 | `notice` has no `actor` field and does not gain one: the actor is |
| 1910 | `c.User.Username`, already passed to `AddNotice` on the line above, so |
| 1911 | threading it through the struct would be a second copy of the same |
| 1912 | value. The call in the loop is therefore: |
| 1913 | |
| 1914 | ```go |
| 1915 | c.Store.EnqueuePush(id, pushTitle(n), pushBody(c.User.Username, n), n.path) |
| 1916 | ``` |
| 1917 | |
| 1918 | - [ ] **Step 4: Run the tests to verify they pass** |
| 1919 | |
| 1920 | Run: `go test ./internal/control/ -v` |
| 1921 | Expected: PASS, the whole package — `notify` has sixteen call sites and |
| 1922 | this changes all of them. |
| 1923 | |
| 1924 | - [ ] **Step 5: Commit** |
| 1925 | |
| 1926 | ```bash |
| 1927 | git add internal/control/notifications.go internal/control/notifications_test.go |
| 1928 | git -c commit.gpgsign=true commit -m "control: push as the third route in notify |
| 1929 | |
| 1930 | Queued in the same loop as the inbox row and the mail, above the SMTP |
| 1931 | check so an instance without a relay still pushes. The alert body is |
| 1932 | the inbox summary, so the surfaces cannot disagree. |
| 1933 | |
| 1934 | Ref #89" |
| 1935 | ``` |
| 1936 | |
| 1937 | --- |
| 1938 | |
| 1939 | ### Task 9: `issue assign` files a notice |
| 1940 | |
| 1941 | **Files:** |
| 1942 | - Modify: `internal/control/issue.go:423-470` |
| 1943 | - Test: `internal/control/issue_test.go` |
| 1944 | |
| 1945 | **Interfaces:** |
| 1946 | - Consumes: `notify`, `notice` from Task 8. |
| 1947 | - Produces: nothing new. |
| 1948 | |
| 1949 | - [ ] **Step 1: Write the failing test** |
| 1950 | |
| 1951 | ```go |
| 1952 | func TestIssueAssignNotifiesTheAssignee(t *testing.T) { |
| 1953 | c, repo, bob := testRepoWithWatcher(t) |
| 1954 | |
| 1955 | if code := runIssueAssign(c, []string{repo.Path(), "1", "--add", "bob"}); code != 0 { |
| 1956 | t.Fatalf("exit %d", code) |
| 1957 | } |
| 1958 | rows, _ := c.Store.Inbox(bob, false, 20, 0) |
| 1959 | if len(rows) != 1 || rows[0].Summary != "assigned you to #1" { |
| 1960 | t.Fatalf("got %+v", rows) |
| 1961 | } |
| 1962 | } |
| 1963 | |
| 1964 | func TestIssueAssignIsSilentForTheActorAndForRemovals(t *testing.T) { |
| 1965 | c, repo, bob := testRepoWithWatcher(t) |
| 1966 | |
| 1967 | // Assigning yourself announces nothing: notify drops the actor. |
| 1968 | runIssueAssign(c, []string{repo.Path(), "1", "--add", "alice"}) |
| 1969 | if rows, _ := c.Store.Inbox(c.User.ID, false, 20, 0); len(rows) != 0 { |
| 1970 | t.Fatalf("self-assignment notified: %+v", rows) |
| 1971 | } |
| 1972 | |
| 1973 | // Unassigning files nothing. |
| 1974 | runIssueAssign(c, []string{repo.Path(), "1", "--add", "bob"}) |
| 1975 | before, _ := c.Store.Inbox(bob, false, 20, 0) |
| 1976 | runIssueAssign(c, []string{repo.Path(), "1", "--remove", "bob"}) |
| 1977 | after, _ := c.Store.Inbox(bob, false, 20, 0) |
| 1978 | if len(after) != len(before) { |
| 1979 | t.Fatalf("removal filed a row: %d then %d", len(before), len(after)) |
| 1980 | } |
| 1981 | } |
| 1982 | ``` |
| 1983 | |
| 1984 | The helper needs an issue #1 on the repo; extend `testRepoWithWatcher` |
| 1985 | from Task 8 or add a sibling that also opens one. |
| 1986 | |
| 1987 | - [ ] **Step 2: Run the test to verify it fails** |
| 1988 | |
| 1989 | Run: `go test ./internal/control/ -run TestIssueAssign -v` |
| 1990 | Expected: FAIL — the inbox is empty. |
| 1991 | |
| 1992 | - [ ] **Step 3: Write the implementation** |
| 1993 | |
| 1994 | In `runIssueAssign`, collect the ids as the add loop resolves them: |
| 1995 | |
| 1996 | ```go |
| 1997 | var added []int64 |
| 1998 | for _, name := range adds { |
| 1999 | u, code := resolve(name) |
| 2000 | if code >= 0 { |
| 2001 | return code |
| 2002 | } |
| 2003 | if err := c.Store.SetIssueAssignee(issue.ID, u.ID, true); err != nil { |
| 2004 | return c.fail(protocol.ExitFailure, "%v", err) |
| 2005 | } |
| 2006 | added = append(added, u.ID) |
| 2007 | } |
| 2008 | ``` |
| 2009 | |
| 2010 | and after both loops succeed, before the function's existing return: |
| 2011 | |
| 2012 | ```go |
| 2013 | if len(added) > 0 { |
| 2014 | // direct, as a mention is: an assignment is addressed to someone, |
| 2015 | // and widening it to watchers would tell them "assigned you". |
| 2016 | // Removals file nothing, and notify drops the actor, so assigning |
| 2017 | // yourself is silent. |
| 2018 | notify(c, added, notice{repo: repo, kind: "issue", direct: true, |
| 2019 | subject: fmt.Sprintf("[%s] #%d: %s", repo.Path(), issue.Number, issue.Title), |
| 2020 | action: fmt.Sprintf("assigned you to #%d", issue.Number), |
| 2021 | path: fmt.Sprintf("%s/issues/%d", repo.Path(), issue.Number)}) |
| 2022 | } |
| 2023 | ``` |
| 2024 | |
| 2025 | - [ ] **Step 4: Run the tests to verify they pass** |
| 2026 | |
| 2027 | Run: `go test ./internal/control/ -run TestIssueAssign -v` |
| 2028 | Expected: PASS. |
| 2029 | |
| 2030 | - [ ] **Step 5: Commit** |
| 2031 | |
| 2032 | ```bash |
| 2033 | git add internal/control/issue.go internal/control/issue_test.go |
| 2034 | git -c commit.gpgsign=true commit -m "control: issue assign files a notice |
| 2035 | |
| 2036 | The dashboard surfaced assigned work and nothing announced it. Direct, |
| 2037 | as a mention is, so watchers are not told they were assigned. |
| 2038 | |
| 2039 | Ref #89" |
| 2040 | ``` |
| 2041 | |
| 2042 | --- |
| 2043 | |
| 2044 | ### Task 10: The web settings page |
| 2045 | |
| 2046 | **Files:** |
| 2047 | - Modify: `internal/httpd/account.go` — `accountPage` around `:65-66`, the page struct around `:76`, and the `accountSubmit` switch at `:246` |
| 2048 | - Modify: `internal/web/templates/account.html` — the `#notifications` section at `:132` |
| 2049 | - Test: `internal/httpd/` package tests |
| 2050 | |
| 2051 | The page is `/settings`, rendered from `account.html`, with a |
| 2052 | `#notifications` section that already carries the mail and watch |
| 2053 | toggles. No new template file, so `TestMainWidthClass` is not involved. |
| 2054 | |
| 2055 | **Interfaces:** |
| 2056 | - Consumes: the control commands from Task 7. |
| 2057 | - Produces: nothing other tasks use. |
| 2058 | |
| 2059 | - [ ] **Step 1: Write the failing test** |
| 2060 | |
| 2061 | Follow the existing `internal/httpd` account-page test for how a page is |
| 2062 | rendered and its body captured. |
| 2063 | |
| 2064 | ```go |
| 2065 | func TestAccountPagePushToggleAndDevices(t *testing.T) { |
| 2066 | // ... render /settings for a user with one registered device whose |
| 2067 | // token is `strings.Repeat("a", 64)`. |
| 2068 | if !strings.Contains(body, `value="notify-push"`) { |
| 2069 | t.Fatal("no push toggle") |
| 2070 | } |
| 2071 | if !strings.Contains(body, "iphone") { |
| 2072 | t.Fatal("the device is not listed") |
| 2073 | } |
| 2074 | // A token is device-identifying and is never printed in full. |
| 2075 | if strings.Contains(body, strings.Repeat("a", 64)) { |
| 2076 | t.Fatal("the page printed a device token in full") |
| 2077 | } |
| 2078 | } |
| 2079 | ``` |
| 2080 | |
| 2081 | - [ ] **Step 2: Run the test to verify it fails** |
| 2082 | |
| 2083 | Run: `go test ./internal/httpd/ -run TestAccountPage -v` |
| 2084 | Expected: FAIL — no push toggle. |
| 2085 | |
| 2086 | - [ ] **Step 3: Accept the new field** |
| 2087 | |
| 2088 | In `accountSubmit` (`internal/httpd/account.go:246`), the case derives |
| 2089 | `pref` by trimming `notify-`, so this is one token: |
| 2090 | |
| 2091 | ```go |
| 2092 | case "notify-mail", "notify-watch", "notify-push": |
| 2093 | ``` |
| 2094 | |
| 2095 | - [ ] **Step 4: Render the toggle and the list** |
| 2096 | |
| 2097 | In `accountPage`, beside `mailOn` and `watchOn`: |
| 2098 | |
| 2099 | ```go |
| 2100 | pushOn, _ := s.st.PushEnabled(u.ID) |
| 2101 | devices, _ := s.st.PushDevices(u.ID) |
| 2102 | ``` |
| 2103 | |
| 2104 | Add `PushOn bool` and a device slice to the anonymous page struct in the |
| 2105 | `s.render` call. Render each device with `prefix8` — the truncation |
| 2106 | helper this file already uses for key fingerprints — not the full token. |
| 2107 | |
| 2108 | In `account.html`, after the watch form in the `#notifications` section, |
| 2109 | copying the shape of the two forms already there: |
| 2110 | |
| 2111 | ```html |
| 2112 | <form method="post" action="/settings" class="setform"> |
| 2113 | <input type="hidden" name="field" value="notify-push"> |
| 2114 | <label for="notify-push">Activity on your registered devices</label> |
| 2115 | <input type="checkbox" id="notify-push" name="push" value="on"{{if .PushOn}} checked{{end}}> |
| 2116 | <button type="submit" class="btn">Save</button> |
| 2117 | </form> |
| 2118 | <p class="meta">Notification text is sent in full, including for private repositories, so a repository name and item number reach Apple and appear on a lock screen.</p> |
| 2119 | ``` |
| 2120 | |
| 2121 | Then the device list, each row posting `field=device-remove` with the |
| 2122 | id, dispatched through `s.runControl` to |
| 2123 | `[]string{"notifications", "device", "remove", id}` as a new case in the |
| 2124 | same switch. |
| 2125 | |
| 2126 | There is no add-a-device form: a browser cannot produce an APNs token. |
| 2127 | That is the Parity page's "CLI only, for now", not a refusal. |
| 2128 | |
| 2129 | - [ ] **Step 5: Run the tests to verify they pass** |
| 2130 | |
| 2131 | Run: `go test ./internal/httpd/ ./internal/web/ -v` |
| 2132 | Expected: PASS. Run `./internal/web/` too — the template registry tests |
| 2133 | live there and a malformed template fails at render, not at build. |
| 2134 | |
| 2135 | - [ ] **Step 6: Commit** |
| 2136 | |
| 2137 | ```bash |
| 2138 | git add internal/httpd/ internal/web/ |
| 2139 | git -c commit.gpgsign=true commit -m "web: push toggle and device list on notification settings |
| 2140 | |
| 2141 | No add-a-device form: a browser cannot produce an APNs token. |
| 2142 | |
| 2143 | Ref #89" |
| 2144 | ``` |
| 2145 | |
| 2146 | --- |
| 2147 | |
| 2148 | ### Task 11: End-to-end |
| 2149 | |
| 2150 | **Files:** |
| 2151 | - Create: `e2e/push_test.go` |
| 2152 | |
| 2153 | **Interfaces:** |
| 2154 | - Consumes: everything above. |
| 2155 | - Produces: nothing. |
| 2156 | |
| 2157 | - [ ] **Step 1: Write the test** |
| 2158 | |
| 2159 | `e2e/push_test.go`, following `e2e/bookmarks_test.go` for how an instance |
| 2160 | and accounts are set up: |
| 2161 | |
| 2162 | ```go |
| 2163 | package e2e |
| 2164 | |
| 2165 | import ( |
| 2166 | "encoding/json" |
| 2167 | "io" |
| 2168 | "net/http" |
| 2169 | "net/http/httptest" |
| 2170 | "strings" |
| 2171 | "sync" |
| 2172 | "testing" |
| 2173 | "time" |
| 2174 | ) |
| 2175 | |
| 2176 | // A push reaches a registered device with the same words the inbox row |
| 2177 | // carries, and a token Apple has retired takes its device with it. |
| 2178 | func TestPush(t *testing.T) { |
| 2179 | var mu sync.Mutex |
| 2180 | var got []map[string]any |
| 2181 | var gone bool |
| 2182 | |
| 2183 | apns := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
| 2184 | raw, _ := io.ReadAll(r.Body) |
| 2185 | var payload map[string]any |
| 2186 | json.Unmarshal(raw, &payload) |
| 2187 | mu.Lock() |
| 2188 | defer mu.Unlock() |
| 2189 | if gone { |
| 2190 | w.WriteHeader(410) |
| 2191 | io.WriteString(w, `{"reason":"Unregistered"}`) |
| 2192 | return |
| 2193 | } |
| 2194 | got = append(got, payload) |
| 2195 | w.WriteHeader(200) |
| 2196 | })) |
| 2197 | defer apns.Close() |
| 2198 | |
| 2199 | keyPath := writeTestAPNSKey(t) |
| 2200 | t.Setenv("GITBAY_APNS_HOST", strings.TrimPrefix(apns.URL, "http://")) |
| 2201 | inst := startInstanceWith(t, `[push] |
| 2202 | enabled = true |
| 2203 | key_file = "`+keyPath+`" |
| 2204 | key_id = "KEYID" |
| 2205 | team_id = "TEAMID" |
| 2206 | topic = "org.gitbay.gitbay" |
| 2207 | environment = "production" |
| 2208 | `) |
| 2209 | |
| 2210 | aliceKey := inst.newKey(t, "alice") |
| 2211 | bobKey := inst.newKey(t, "bob") |
| 2212 | inst.admin(t, "admin", "user", "create", "alice", "--key", aliceKey+".pub") |
| 2213 | inst.admin(t, "admin", "user", "create", "bob", "--key", bobKey+".pub") |
| 2214 | if _, errOut, code := inst.ssh(t, aliceKey, "", "repo", "create", "alice/app"); code != 0 { |
| 2215 | t.Fatalf("repo create: %s", errOut) |
| 2216 | } |
| 2217 | |
| 2218 | // Bob watches alice's repository and registers a device. |
| 2219 | if out, errOut, code := inst.ssh(t, bobKey, "", "repo", "watch", "alice/app"); code != 0 { |
| 2220 | t.Fatalf("watch: %s%s", out, errOut) |
| 2221 | } |
| 2222 | if out, errOut, code := inst.ssh(t, bobKey, "DEVTOKEN\n", "notifications", "device", "add", "--label", "iphone"); code != 0 { |
| 2223 | t.Fatalf("device add: %s%s", out, errOut) |
| 2224 | } |
| 2225 | if out, _, _ := inst.ssh(t, bobKey, "", "notifications", "device", "list", "--json"); !strings.Contains(out, `"label":"iphone"`) { |
| 2226 | t.Fatalf("device not listed:\n%s", out) |
| 2227 | } else if strings.Contains(out, "DEVTOKEN") { |
| 2228 | t.Fatalf("device list printed the token in full:\n%s", out) |
| 2229 | } |
| 2230 | |
| 2231 | // Alice opens an issue. Bob hears about it. |
| 2232 | if out, errOut, code := inst.ssh(t, aliceKey, "", "issue", "create", "alice/app", "--title", "a bug", "--body", "x"); code != 0 { |
| 2233 | t.Fatalf("issue create: %s%s", out, errOut) |
| 2234 | } |
| 2235 | |
| 2236 | waitFor(t, 20*time.Second, func() bool { |
| 2237 | mu.Lock() |
| 2238 | defer mu.Unlock() |
| 2239 | return len(got) == 1 |
| 2240 | }, "no push arrived") |
| 2241 | |
| 2242 | mu.Lock() |
| 2243 | aps := got[0]["aps"].(map[string]any) |
| 2244 | alert := aps["alert"].(map[string]any) |
| 2245 | mu.Unlock() |
| 2246 | if alert["title"] != "alice/app" { |
| 2247 | t.Fatalf("title = %v", alert["title"]) |
| 2248 | } |
| 2249 | // The same words the inbox row carries. |
| 2250 | if body, _ := alert["body"].(string); !strings.Contains(body, "opened issue #1") { |
| 2251 | t.Fatalf("body = %q", body) |
| 2252 | } |
| 2253 | if got[0]["path"] != "alice/app/issues/1" { |
| 2254 | t.Fatalf("path = %v", got[0]["path"]) |
| 2255 | } |
| 2256 | |
| 2257 | // Apple retires the token. The next push reaps the device. |
| 2258 | mu.Lock() |
| 2259 | gone = true |
| 2260 | mu.Unlock() |
| 2261 | if out, errOut, code := inst.ssh(t, aliceKey, "", "issue", "comment", "alice/app", "1", "--body", "ping"); code != 0 { |
| 2262 | t.Fatalf("issue comment: %s%s", out, errOut) |
| 2263 | } |
| 2264 | waitFor(t, 20*time.Second, func() bool { |
| 2265 | out, _, _ := inst.ssh(t, bobKey, "", "notifications", "device", "list", "--json") |
| 2266 | return !strings.Contains(out, "iphone") |
| 2267 | }, "the device survived a 410") |
| 2268 | |
| 2269 | // The inbox is untouched by any of it: push is a side channel. |
| 2270 | if out, _, _ := inst.ssh(t, bobKey, "", "notifications", "list", "--json"); !strings.Contains(out, "opened issue #1") { |
| 2271 | t.Fatalf("inbox missing the notice:\n%s", out) |
| 2272 | } |
| 2273 | } |
| 2274 | ``` |
| 2275 | |
| 2276 | Write `writeTestAPNSKey` (a P-256 PKCS#8 key in a `t.TempDir()`, as in |
| 2277 | Task 3) and reuse the e2e suite's existing polling helper rather than |
| 2278 | writing `waitFor` if one exists — check `e2e/` for it first. |
| 2279 | |
| 2280 | Note the `ssh` helper's second argument is stdin; that is how `DEVTOKEN` |
| 2281 | reaches `device add`. |
| 2282 | |
| 2283 | - [ ] **Step 2: Run it** |
| 2284 | |
| 2285 | Run: `go test ./e2e/ -run TestPush -v` |
| 2286 | Expected: PASS. This is the one e2e test to run locally; the rest of the |
| 2287 | suite belongs to CI on bay1. |
| 2288 | |
| 2289 | - [ ] **Step 3: Commit** |
| 2290 | |
| 2291 | ```bash |
| 2292 | git add e2e/push_test.go |
| 2293 | git -c commit.gpgsign=true commit -m "e2e: push delivery and device reaping |
| 2294 | |
| 2295 | A fake APNs over HTTP/1.1; the real transport is h2 by ALPN, which is |
| 2296 | stdlib behaviour and not ours to test. |
| 2297 | |
| 2298 | Ref #89" |
| 2299 | ``` |
| 2300 | |
| 2301 | --- |
| 2302 | |
| 2303 | ### Task 12: Documentation |
| 2304 | |
| 2305 | **Files:** |
| 2306 | - Modify: `.gitbay/wiki/Parity.md` |
| 2307 | - Modify: `.gitbay/wiki/Admin.md` |
| 2308 | - Modify: `.gitbay/wiki/Users.md` |
| 2309 | - Modify: `CHANGELOG.org` |
| 2310 | |
| 2311 | **Interfaces:** |
| 2312 | - Consumes: everything above. |
| 2313 | - Produces: nothing. |
| 2314 | |
| 2315 | - [ ] **Step 1: Parity** |
| 2316 | |
| 2317 | Add a row per new command — `notifications device add`, `device list`, |
| 2318 | `device remove`, `settings push` — with its SSH/CLI/web/API columns. |
| 2319 | `device add` is CLI only for now on the web column, because a browser |
| 2320 | cannot produce an APNs token; write it as "CLI only, for now", which is |
| 2321 | the page's current wording, not "no". |
| 2322 | |
| 2323 | - [ ] **Step 2: Admin** |
| 2324 | |
| 2325 | Document the `[push]` section: every key, how to obtain a `.p8` from the |
| 2326 | developer portal, where the file goes (`/etc/gitbay/apns.p8`, mode 0600, |
| 2327 | owned by the account gitbayd runs as), and the constraint that an APNs |
| 2328 | key belongs to a bundle ID — a self-hoster pushes to their own build |
| 2329 | under their own `topic`, not to the App Store app. |
| 2330 | |
| 2331 | - [ ] **Step 3: Users** |
| 2332 | |
| 2333 | Document `notifications settings push on|off` and what a device row is: |
| 2334 | registered by the app, listed and removable from the CLI and the web, |
| 2335 | and dropped automatically when Apple says the token is dead. |
| 2336 | |
| 2337 | State plainly that notification text is sent in full, private |
| 2338 | repositories included, so a repository name and item number reach Apple |
| 2339 | and appear on a lock screen. |
| 2340 | |
| 2341 | - [ ] **Step 4: CHANGELOG** |
| 2342 | |
| 2343 | Add the feature under the unreleased heading in `CHANGELOG.org`, in the |
| 2344 | style of the entries already there. |
| 2345 | |
| 2346 | - [ ] **Step 5: Commit** |
| 2347 | |
| 2348 | ```bash |
| 2349 | git add .gitbay/wiki/ CHANGELOG.org |
| 2350 | git -c commit.gpgsign=true commit -m "docs: push notifications |
| 2351 | |
| 2352 | Closes #89" |
| 2353 | ``` |
| 2354 | |
| 2355 | --- |
| 2356 | |
| 2357 | ### Task 13: Open the merge request |
| 2358 | |
| 2359 | - [ ] **Step 1: Verify the branch** |
| 2360 | |
| 2361 | Run: `go build ./... && go vet ./... && go test ./internal/... ./cmd/...` |
| 2362 | Expected: all PASS. Do not claim the branch is green without this output |
| 2363 | in front of you. |
| 2364 | |
| 2365 | - [ ] **Step 2: Push and open the MR** |
| 2366 | |
| 2367 | ```bash |
| 2368 | git push -u origin ios-push |
| 2369 | ``` |
| 2370 | |
| 2371 | ```bash |
| 2372 | gitbay mr create --source ios-push --target main --title "iOS push notifications (server)" |
| 2373 | ``` |
| 2374 | |
| 2375 | Body via `--file -` from a file, not a heredoc. It states what landed and |
| 2376 | references `Closes #89`. No attribution to any assistant or model. |
| 2377 | |
| 2378 | - [ ] **Step 3: Let CI run** |
| 2379 | |
| 2380 | The e2e suite runs on bay1. Watch it with `gitbay build list` and |
| 2381 | `gitbay build log <n>`. Do not poll with several ssh calls per tick — |
| 2382 | the auth limiter reads a burst as an attack. |
| 2383 | |
| 2384 | - [ ] **Step 4: Merge** |
| 2385 | |
| 2386 | Only with the full suite green: |
| 2387 | |
| 2388 | ```bash |
| 2389 | gitbay mr merge <n> --strategy ff |
| 2390 | ``` |
| 2391 | |
| 2392 | Signed commits are required, so `squash` and `merge` are refused — both |
| 2393 | would mint an unsigned commit. If the merge reports the branch is |
| 2394 | behind, rebase onto `main`, re-push, merge again. Then delete the branch |
| 2395 | locally and remotely. |
| 2396 | |
| 2397 | **Do not deploy.** `[push]` stays `enabled = false` on bay1 until the app |
| 2398 | is submitted; there is nothing to deliver to until a device registers. |
| 2399 | |
| 2400 | --- |
| 2401 | |
| 2402 | ## Notes for whoever executes this |
| 2403 | |
| 2404 | - **Tasks 1-9 are the working feature.** Task 10 (web) and Task 12 (docs) |
| 2405 | can be reordered or split into a follow-up MR if the branch is getting |
| 2406 | long, but Task 11's e2e should land with the code it tests. |
| 2407 | - **The classifier may refuse some of this.** Editing files under |
| 2408 | `internal/policy/` or anything that reads as relaxing an access-control |
| 2409 | flag has been refused before in auto mode. Nothing in this plan should |
| 2410 | trip it, but if a refusal happens, retry as a single-file edit with no |
| 2411 | chained build rather than treating it as a puzzle. |
| 2412 | - **The `krz/gitbay-ios` half is a separate plan** on that repository, |
| 2413 | written once this has shipped. The spec's "The app" section is the |
| 2414 | contract it has to meet — payload keys `aps.alert.title`, |
| 2415 | `aps.alert.body`, `aps.thread-id` and top-level `path`, and the |
| 2416 | `notifications device add|remove` calls. |