Commit 66304b6cd8
Verified · cmc ci/build: success ci/test: success
CHANGELOG.org +11
| @@ -4,6 +4,17 @@ Versioning follows semver from v0.1.0. Database migrations run | ||
| 4 | 4 | automatically on daemon start; upgrade notes appear per release when |
| 5 | 5 | anything beyond "replace the binary and restart" is needed. |
| 6 | 6 | |
| 7 | * v1.32.1 — 2026-09-20 | |
| 8 | ||
| 9 | A push alert names the account it is for (#89). | |
| 10 | ||
| 11 | - The payload carried =path= and nothing else, so a device signed in to | |
| 12 | more than one account could not tell which instance a notice came | |
| 13 | from: two instances can hold the same =owner/name=. It now carries | |
| 14 | =instance=, the instance's =site_url=, and =user=, the recipient's | |
| 15 | username — together the account's identity. | |
| 16 | - =DuePush= joins =users= for the recipient; no schema change. | |
| 17 | ||
| 7 | 18 | * v1.32.0 — 2026-09-20 |
| 8 | 19 | |
| 9 | 20 | Push notifications to iOS devices (#89): activity reaches a registered |
cmd/gitbayd/main.go +1 −1
| @@ -178,7 +178,7 @@ func serveCmd() *cobra.Command { | ||
| 178 | 178 | go notify.New(st, cfg, retryBase).Run(whCtx) |
| 179 | 179 | } |
| 180 | 180 | if cfg.Push.Enabled { |
| 181 | p, err := push.New(st, cfg.Push, retryBase) | |
| 181 | p, err := push.New(st, cfg.Push, cfg.Server.SiteURL, retryBase) | |
| 182 | 182 | if err != nil { |
| 183 | 183 | // Config validation already parsed the key, so this |
| 184 | 184 | // is not a misconfiguration; fail loudly rather than |
docs/specs/2026-09-20-ios-push-notifications-design.md +10 −1
| @@ -207,7 +207,9 @@ The payload: | ||
| 207 | 207 | "sound": "default", |
| 208 | 208 | "thread-id": "krz/gitbay" |
| 209 | 209 | }, |
| 210 | "path": "krz/gitbay/issues/12" | |
| 210 | "path": "krz/gitbay/issues/12", | |
| 211 | "instance": "https://gitbay.org", | |
| 212 | "user": "cmc" | |
| 211 | 213 | } |
| 212 | 214 | ``` |
| 213 | 215 | |
| @@ -217,6 +219,13 @@ string the inbox row carries, so the two surfaces cannot disagree. | ||
| 217 | 219 | `path` is the inbox row's `path` field, which the app already knows how |
| 218 | 220 | to turn into a link. |
| 219 | 221 | |
| 222 | `instance` is this instance's `site_url` and `user` the recipient's | |
| 223 | username. A device token is one install, and an install registers | |
| 224 | against every account signed in on it, so `path` alone cannot say which | |
| 225 | account a notice belongs to — two instances can hold the same | |
| 226 | `owner/name`. The pair is the account's identity, and the client | |
| 227 | resolves it before routing. | |
| 228 | ||
| 220 | 229 | `apns-push-type: alert`, `apns-topic` from config, and |
| 221 | 230 | `apns-collapse-id` unset — collapsing is wrong here, two comments are |
| 222 | 231 | two notices. |
e2e/push_test.go +8
| @@ -125,6 +125,14 @@ environment = "production" | ||
| 125 | 125 | if got[0]["path"] != "alice/app/issues/1" { |
| 126 | 126 | t.Fatalf("path = %v", got[0]["path"]) |
| 127 | 127 | } |
| 128 | // The account the notice is for. A device signed in to several | |
| 129 | // accounts cannot tell from the path alone which one this is. | |
| 130 | if got[0]["user"] != "bob" { | |
| 131 | t.Fatalf("user = %v", got[0]["user"]) | |
| 132 | } | |
| 133 | if inst, _ := got[0]["instance"].(string); !strings.HasPrefix(inst, "https://") { | |
| 134 | t.Fatalf("instance = %v, want this instance's site_url", inst) | |
| 135 | } | |
| 128 | 136 | |
| 129 | 137 | // Apple retires the token. The next push reaps the device. |
| 130 | 138 | mu.Lock() |
internal/push/apns.go +17 −6
| @@ -40,16 +40,21 @@ type Client struct { | ||
| 40 | 40 | host string |
| 41 | 41 | scheme string |
| 42 | 42 | topic string |
| 43 | // siteURL is this instance, as the alert reports it. With the | |
| 44 | // recipient's username it identifies the account a notice belongs | |
| 45 | // to, which a device signed in to several cannot otherwise tell. | |
| 46 | siteURL string | |
| 43 | 47 | } |
| 44 | 48 | |
| 45 | func NewClient(cfg config.Push) (*Client, error) { | |
| 49 | func NewClient(cfg config.Push, siteURL string) (*Client, error) { | |
| 46 | 50 | c := &Client{ |
| 47 | 51 | // stdlib negotiates HTTP/2 over ALPN, which is what APNs |
| 48 | 52 | // requires; no explicit http2 transport is needed. |
| 49 | http: &http.Client{Timeout: 30 * time.Second}, | |
| 50 | host: cfg.Host(), | |
| 51 | scheme: apnsScheme(), | |
| 52 | topic: cfg.Topic, | |
| 53 | http: &http.Client{Timeout: 30 * time.Second}, | |
| 54 | host: cfg.Host(), | |
| 55 | scheme: apnsScheme(), | |
| 56 | topic: cfg.Topic, | |
| 57 | siteURL: siteURL, | |
| 53 | 58 | } |
| 54 | 59 | if cfg.KeyFile != "" { |
| 55 | 60 | key, err := config.LoadAPNSKey(cfg.KeyFile) |
| @@ -106,7 +111,7 @@ func loopbackHost(hostport string) bool { | ||
| 106 | 111 | |
| 107 | 112 | // Send delivers one alert. The returned duration is the server's |
| 108 | 113 | // Retry-After when it gave one, zero otherwise. |
| 109 | func (c *Client) Send(ctx context.Context, token, title, body, path string) (result, time.Duration, error) { | |
| 114 | func (c *Client) Send(ctx context.Context, token, user, title, body, path string) (result, time.Duration, error) { | |
| 110 | 115 | if len(body) > maxBodyBytes { |
| 111 | 116 | // A raw byte cut can land mid-rune on multi-byte UTF-8 (emoji, |
| 112 | 117 | // accents, non-Latin usernames). ToValidUTF8 drops the |
| @@ -121,6 +126,12 @@ func (c *Client) Send(ctx context.Context, token, title, body, path string) (res | ||
| 121 | 126 | "thread-id": title, |
| 122 | 127 | }, |
| 123 | 128 | "path": path, |
| 129 | // Which account this is for. A device token is one install, and | |
| 130 | // an install registers against every account signed in on it, so | |
| 131 | // path alone is ambiguous — two instances can hold the same | |
| 132 | // owner/name. Together these are the account's identity. | |
| 133 | "instance": c.siteURL, | |
| 134 | "user": user, | |
| 124 | 135 | }) |
| 125 | 136 | if err != nil { |
| 126 | 137 | return resultDead, 0, err |
internal/push/apns_test.go +33 −4
| @@ -24,7 +24,7 @@ func fakeAPNs(t *testing.T, h http.HandlerFunc) (*Client, *httptest.Server) { | ||
| 24 | 24 | c, err := NewClient(config.Push{ |
| 25 | 25 | Enabled: true, KeyID: "K", TeamID: "T", |
| 26 | 26 | Topic: "org.gitbay.gitbay", Environment: "production", |
| 27 | }) | |
| 27 | }, "https://gitbay.example") | |
| 28 | 28 | if err != nil { |
| 29 | 29 | t.Fatal(err) |
| 30 | 30 | } |
| @@ -45,7 +45,7 @@ func TestSendShapesTheRequest(t *testing.T) { | ||
| 45 | 45 | json.Unmarshal(raw, &payload) |
| 46 | 46 | w.WriteHeader(200) |
| 47 | 47 | }) |
| 48 | res, _, err := c.Send(context.Background(), "DEVTOKEN", "krz/gitbay", "cmc opened issue #12", "krz/gitbay/issues/12") | |
| 48 | res, _, err := c.Send(context.Background(), "DEVTOKEN", "cmc", "krz/gitbay", "cmc opened issue #12", "krz/gitbay/issues/12") | |
| 49 | 49 | if err != nil || res != resultSent { |
| 50 | 50 | t.Fatalf("res = %v, err = %v", res, err) |
| 51 | 51 | } |
| @@ -103,7 +103,7 @@ func TestSendMapsResponses(t *testing.T) { | ||
| 103 | 103 | w.WriteHeader(tc.status) |
| 104 | 104 | io.WriteString(w, tc.body) |
| 105 | 105 | }) |
| 106 | res, after, err := c.Send(context.Background(), "T", "t", "b", "p") | |
| 106 | res, after, err := c.Send(context.Background(), "T", "u", "t", "b", "p") | |
| 107 | 107 | // Only a delivered push has no error. Every other result |
| 108 | 108 | // carries the status and reason, which is what the drainer |
| 109 | 109 | // records on the queue row. |
| @@ -134,7 +134,7 @@ func TestSendTruncatesBodyOnRuneBoundary(t *testing.T) { | ||
| 134 | 134 | // even offset, so a raw cut at maxBodyBytes is guaranteed to land on |
| 135 | 135 | // the second byte of one of them rather than a rune boundary. |
| 136 | 136 | long := "x" + strings.Repeat("é", 2000) |
| 137 | res, _, err := c.Send(context.Background(), "T", "t", long, "p") | |
| 137 | res, _, err := c.Send(context.Background(), "T", "u", "t", long, "p") | |
| 138 | 138 | if err != nil || res != resultSent { |
| 139 | 139 | t.Fatalf("res = %v, err = %v", res, err) |
| 140 | 140 | } |
| @@ -172,3 +172,32 @@ func TestAPNSSchemeDowngradesOnlyOnLoopback(t *testing.T) { | ||
| 172 | 172 | } |
| 173 | 173 | } |
| 174 | 174 | } |
| 175 | ||
| 176 | // The alert names the account it belongs to. One device token is one | |
| 177 | // install, and an install registers against every account signed in on | |
| 178 | // it, so `path` alone cannot say which instance a notice came from — two | |
| 179 | // instances can hold the same owner/name. | |
| 180 | func TestSendNamesTheAccount(t *testing.T) { | |
| 181 | var payload map[string]any | |
| 182 | c, _ := fakeAPNs(t, func(w http.ResponseWriter, r *http.Request) { | |
| 183 | raw, _ := io.ReadAll(r.Body) | |
| 184 | json.Unmarshal(raw, &payload) | |
| 185 | w.WriteHeader(200) | |
| 186 | }) | |
| 187 | c.siteURL = "https://gitbay.org" | |
| 188 | ||
| 189 | if _, _, err := c.Send(context.Background(), "DEVTOKEN", "cmc", | |
| 190 | "krz/gitbay", "cmc opened issue #12", "krz/gitbay/issues/12"); err != nil { | |
| 191 | t.Fatalf("Send: %v", err) | |
| 192 | } | |
| 193 | if payload["instance"] != "https://gitbay.org" { | |
| 194 | t.Fatalf("instance = %v", payload["instance"]) | |
| 195 | } | |
| 196 | if payload["user"] != "cmc" { | |
| 197 | t.Fatalf("user = %v", payload["user"]) | |
| 198 | } | |
| 199 | // Still carries what it always did. | |
| 200 | if payload["path"] != "krz/gitbay/issues/12" { | |
| 201 | t.Fatalf("path = %v", payload["path"]) | |
| 202 | } | |
| 203 | } | |
internal/push/push.go +3 −3
| @@ -20,8 +20,8 @@ type Deliverer struct { | ||
| 20 | 20 | MaxAttempts int |
| 21 | 21 | } |
| 22 | 22 | |
| 23 | func New(st *store.Store, cfg config.Push, retryBase time.Duration) (*Deliverer, error) { | |
| 24 | cl, err := NewClient(cfg) | |
| 23 | func New(st *store.Store, cfg config.Push, siteURL string, retryBase time.Duration) (*Deliverer, error) { | |
| 24 | cl, err := NewClient(cfg, siteURL) | |
| 25 | 25 | if err != nil { |
| 26 | 26 | return nil, err |
| 27 | 27 | } |
| @@ -49,7 +49,7 @@ func (d *Deliverer) drain(ctx context.Context) { | ||
| 49 | 49 | return |
| 50 | 50 | } |
| 51 | 51 | for _, q := range due { |
| 52 | res, after, sendErr := d.Cl.Send(ctx, q.Token, q.Title, q.Body, q.Path) | |
| 52 | res, after, sendErr := d.Cl.Send(ctx, q.Token, q.Username, q.Title, q.Body, q.Path) | |
| 53 | 53 | msg := "" |
| 54 | 54 | if sendErr != nil { |
| 55 | 55 | msg = sendErr.Error() |
internal/store/push.go +9 −3
| @@ -120,6 +120,10 @@ type QueuedPush struct { | ||
| 120 | 120 | ID int64 |
| 121 | 121 | DeviceID int64 |
| 122 | 122 | Token string |
| 123 | // Username is the recipient. One device token is one install, and an | |
| 124 | // install registers against every account signed in on it, so the | |
| 125 | // alert has to name which of them it is for. | |
| 126 | Username string | |
| 123 | 127 | Title string |
| 124 | 128 | Body string |
| 125 | 129 | Path string |
| @@ -145,8 +149,10 @@ func (s *Store) EnqueuePush(userID int64, title, body, path string) error { | ||
| 145 | 149 | |
| 146 | 150 | func (s *Store) DuePush(limit int) ([]QueuedPush, error) { |
| 147 | 151 | rows, err := s.DB.Query(` |
| 148 | SELECT q.id, q.device_id, d.token, q.title, q.body, q.path, q.attempts | |
| 149 | FROM push_queue q JOIN push_devices d ON d.id = q.device_id | |
| 152 | SELECT q.id, q.device_id, d.token, u.username, q.title, q.body, q.path, q.attempts | |
| 153 | FROM push_queue q | |
| 154 | JOIN push_devices d ON d.id = q.device_id | |
| 155 | JOIN users u ON u.id = d.user_id | |
| 150 | 156 | WHERE q.sent_at IS NULL AND q.failed_at IS NULL |
| 151 | 157 | AND (q.next_attempt_at IS NULL OR q.next_attempt_at <= ?) |
| 152 | 158 | ORDER BY q.id LIMIT ?`, fmtTime(time.Now()), limit) |
| @@ -157,7 +163,7 @@ func (s *Store) DuePush(limit int) ([]QueuedPush, error) { | ||
| 157 | 163 | var out []QueuedPush |
| 158 | 164 | for rows.Next() { |
| 159 | 165 | var p QueuedPush |
| 160 | if err := rows.Scan(&p.ID, &p.DeviceID, &p.Token, &p.Title, &p.Body, &p.Path, &p.Attempts); err != nil { | |
| 166 | if err := rows.Scan(&p.ID, &p.DeviceID, &p.Token, &p.Username, &p.Title, &p.Body, &p.Path, &p.Attempts); err != nil { | |
| 161 | 167 | return nil, err |
| 162 | 168 | } |
| 163 | 169 | out = append(out, p) |
internal/store/push_test.go +28
| @@ -247,3 +247,31 @@ func TestDeletePushDeviceByTokenTakesItsQueue(t *testing.T) { | ||
| 247 | 247 | t.Fatalf("queued rows outlived their device") |
| 248 | 248 | } |
| 249 | 249 | } |
| 250 | ||
| 251 | // A queued push carries the recipient's username, so the alert can name | |
| 252 | // the account it belongs to. A device token is one install, and one | |
| 253 | // install registers against every account signed in on it; without the | |
| 254 | // username the client cannot tell which of them a push is for. | |
| 255 | func TestDuePushCarriesTheUsername(t *testing.T) { | |
| 256 | s := pushFixture(t) | |
| 257 | uid, err := s.CreateUser("alice", false) | |
| 258 | if err != nil { | |
| 259 | t.Fatal(err) | |
| 260 | } | |
| 261 | if _, err := s.AddPushDevice(uid, "tok-a", "iphone"); err != nil { | |
| 262 | t.Fatal(err) | |
| 263 | } | |
| 264 | if err := s.EnqueuePush(uid, "alice/app", "bob opened issue #1", "alice/app/issues/1"); err != nil { | |
| 265 | t.Fatal(err) | |
| 266 | } | |
| 267 | due, err := s.DuePush(20) | |
| 268 | if err != nil { | |
| 269 | t.Fatal(err) | |
| 270 | } | |
| 271 | if len(due) != 1 { | |
| 272 | t.Fatalf("want one queued push, got %d", len(due)) | |
| 273 | } | |
| 274 | if due[0].Username != "alice" { | |
| 275 | t.Fatalf("Username = %q, want alice", due[0].Username) | |
| 276 | } | |
| 277 | } | |