fix: watch sync reads the roster through trip membership, image viewer on theme tokens, retry flaky release downloads in CI

.github/workflows/native.yml

@@ -121,18 +121,18 @@ jobs:
         working-directory: client
         run: npm run cap:sync -- ios
 
-      # SwiftPM leaves a half-downloaded binary target (Capacitor's xcframeworks)
-      # behind when a download from GitHub fails, and every later attempt then
-      # stops on "already exists in file system". Resolve on a clean artifact
-      # cache, up to three times, before the build needs the packages.
+      # Capacitor's xcframeworks come from GitHub release downloads, which answer
+      # 504 now and then; SwiftPM then reports the half-written file as "already
+      # exists in file system". Resolve on a clean artifact cache, a few times
+      # with growing pauses, before the build needs the packages.
       - name: Resolve the Swift packages
         working-directory: client/ios/App
         run: |
-          for attempt in 1 2 3; do
+          for attempt in 1 2 3 4 5; do
             rm -rf ~/Library/Caches/org.swift.swiftpm/artifacts
             if xcodebuild -resolvePackageDependencies -project App.xcodeproj -scheme App; then exit 0; fi
-            echo "Resolving the Swift packages failed (attempt $attempt of 3)"
-            sleep 20
+            echo "Resolving the Swift packages failed (attempt $attempt of 5)"
+            sleep $((attempt * 30))
           done
           exit 1
 
@@ -235,18 +235,18 @@ jobs:
           xcrun simctl list runtimes | grep -q watchOS || xcodebuild -downloadPlatform watchOS
           xcrun simctl list runtimes
 
-      # SwiftPM leaves a half-downloaded binary target (Capacitor's xcframeworks)
-      # behind when a download from GitHub fails, and every later attempt then
-      # stops on "already exists in file system". Resolve on a clean artifact
-      # cache, up to three times, before the build needs the packages.
+      # Capacitor's xcframeworks come from GitHub release downloads, which answer
+      # 504 now and then; SwiftPM then reports the half-written file as "already
+      # exists in file system". Resolve on a clean artifact cache, a few times
+      # with growing pauses, before the build needs the packages.
       - name: Resolve the Swift packages
         working-directory: client/ios/App
         run: |
-          for attempt in 1 2 3; do
+          for attempt in 1 2 3 4 5; do
             rm -rf ~/Library/Caches/org.swift.swiftpm/artifacts
             if xcodebuild -resolvePackageDependencies -project App.xcodeproj -scheme App; then exit 0; fi
-            echo "Resolving the Swift packages failed (attempt $attempt of 3)"
-            sleep 20
+            echo "Resolving the Swift packages failed (attempt $attempt of 5)"
+            sleep $((attempt * 30))
           done
           exit 1
 

.github/workflows/test.yml

@@ -1236,10 +1236,12 @@ jobs:
           bin="$RUNNER_TEMP/bin"
           mkdir -p "$bin"
           cd "$RUNNER_TEMP"
-          curl -sSfLo helm.tgz "https://get.helm.sh/helm-$HELM_VERSION-linux-amd64.tar.gz"
+          # GitHub's release downloads answer 504 now and then: retry, the checksum still decides.
+          fetch() { curl -sSfL --retry 5 --retry-delay 15 --retry-all-errors -o "$1" "$2"; }
+          fetch helm.tgz "https://get.helm.sh/helm-$HELM_VERSION-linux-amd64.tar.gz"
           echo "$HELM_SHA256  helm.tgz" | sha256sum -c -
           tar -xzf helm.tgz --strip-components=1 -C "$bin" linux-amd64/helm
-          curl -sSfLo kubeconform.tgz "https://github.com/yannh/kubeconform/releases/download/$KUBECONFORM_VERSION/kubeconform-linux-amd64.tar.gz"
+          fetch kubeconform.tgz "https://github.com/yannh/kubeconform/releases/download/$KUBECONFORM_VERSION/kubeconform-linux-amd64.tar.gz"
           echo "$KUBECONFORM_SHA256  kubeconform.tgz" | sha256sum -c -
           tar -xzf kubeconform.tgz -C "$bin" kubeconform
           echo "$bin" >> "$GITHUB_PATH"

server/src/nest/notifications/notifications.module.ts

@@ -4,11 +4,11 @@ import { Notifications } from '../../db/entities/Notifications.entity';
 import { PushSubscriptions } from '../../db/entities/PushSubscriptions.entity';
 import { Settings } from '../../db/entities/Settings.entity';
 import { TodoItems } from '../../db/entities/TodoItems.entity';
-import { TripMembers } from '../../db/entities/TripMembers.entity';
 import { Trips } from '../../db/entities/Trips.entity';
 import { AddonsModule } from '../addons/addons.module';
 import { SchedulingModule } from '../scheduling/scheduling.module';
 import { StorageModule } from '../storage/storage.module';
+import { TripMembershipModule } from '../trip-membership/trip-membership.module';
 import { MailerModule } from './mailer/mailer.module';
 import { NotificationPreferencesService } from './notification-preferences.service';
 import { AdminNotificationPreferencesController, NotificationsController } from './notifications.controller';
@@ -64,13 +64,13 @@ import { Module } from '@nestjs/common';
       Trips,
       TodoItems,
       PushSubscriptions,
-      TripMembers,
     ]),
 
     AddonsModule,
     MailerModule,
     SchedulingModule,
     StorageModule,
+    TripMembershipModule,
   ],
   controllers: [NotificationsController, AdminNotificationPreferencesController, PushController],
   providers: [

server/src/nest/notifications/push/watch-sync-push.service.ts

@@ -1,11 +1,9 @@
-import { TripMembers } from '../../../db/entities/TripMembers.entity';
-import type { TripMembersRepository } from '../../../db/repositories/TripMembers.repository';
 import { logError } from '../../audit/audit-log.logger';
 import { withRequestContext } from '../../database/request-context';
 import { RealtimeService } from '../../realtime/realtime.service';
+import { TripMembershipService } from '../../trip-membership/trip-membership.service';
 import { WebPushService } from '../transports/web-push.service';
 import { MikroORM } from '@mikro-orm/core';
-import { InjectRepository } from '@mikro-orm/nestjs';
 import { Injectable, type OnModuleDestroy, type OnModuleInit } from '@nestjs/common';
 
 /**
@@ -46,7 +44,7 @@ export class WatchSyncPushService implements OnModuleInit, OnModuleDestroy {
   constructor(
     private readonly realtime: RealtimeService,
     private readonly push: WebPushService,
-    @InjectRepository(TripMembers) private readonly members: TripMembersRepository,
+    private readonly membership: TripMembershipService,
     private readonly orm: MikroORM,
   ) {}
 
@@ -79,6 +77,17 @@ export class WatchSyncPushService implements OnModuleInit, OnModuleDestroy {
     else entry.users.add(onlyUserId);
   }
 
+  /** Everyone on the trip: the owner and the members. */
+  private async roster(tripId: string): Promise<Set<number>> {
+    const [owner, members] = await Promise.all([
+      this.membership.getOwnerId(tripId),
+      this.membership.listMemberUserIds(tripId),
+    ]);
+    const users = new Set(members);
+    if (owner != null) users.add(owner);
+    return users;
+  }
+
   /** Sends one trip's gathered sync. */
   async flush(key: string): Promise<void> {
     const entry = this.pending.get(key);
@@ -86,9 +95,7 @@ export class WatchSyncPushService implements OnModuleInit, OnModuleDestroy {
     this.pending.delete(key);
     clearTimeout(entry.timer);
     try {
-      const users = entry.everyone
-        ? await withRequestContext(this.orm, () => this.members.rosterUserIds(key))
-        : entry.users;
+      const users = entry.everyone ? await withRequestContext(this.orm, () => this.roster(key)) : entry.users;
       await Promise.all([...users].map((userId) => this.push.sendWatchSync(userId)));
     } catch (err) {
       logError(`Watch sync failed trip=${key}: ${err instanceof Error ? err.message : err}`);

server/tests/unit/nest/notifications/watch-sync-push.test.ts

@@ -1,91 +1,67 @@
 /**
  * The watch sync (WSYNC-*): list changes on a trip become one silent push per
- * member a moment later (WatchSyncPushService), sent to the iPhone app only and
- * marked for the relay as a background push (WebPushService.sendWatchSync), and
- * every trip broadcast reaches its listeners (RealtimeService.onTripBroadcast).
+ * member a moment later (WatchSyncPushService), and every trip broadcast
+ * reaches its listeners (RealtimeService.onTripBroadcast). The sending itself
+ * (WebPushService.sendWatchSync) is tested with the rest of the transport in
+ * web-push.service.test.ts.
  */
-import type { PushSubscriptionRow } from '../../../../src/db/repositories/PushSubscriptions.repository';
-import type { TripMembersRepository } from '../../../../src/db/repositories/TripMembers.repository';
-import type { PushSubscriptionsService } from '../../../../src/nest/notifications/push/push-subscriptions.service';
-import type { VapidKeysService } from '../../../../src/nest/notifications/push/vapid-keys.service';
 import {
   WATCH_SYNC_SETTLE_MS,
   WatchSyncPushService,
 } from '../../../../src/nest/notifications/push/watch-sync-push.service';
-import { generateVapidKeyPair } from '../../../../src/nest/notifications/push/web-push-crypto';
-import {
-  BACKGROUND_PUSH_HEADER,
-  WebPushService,
-  isAppleRelayEndpoint,
-} from '../../../../src/nest/notifications/transports/web-push.service';
+import { WebPushService } from '../../../../src/nest/notifications/transports/web-push.service';
 import { RealtimeService } from '../../../../src/nest/realtime/realtime.service';
-import type { MikroORM } from '@mikro-orm/core';
+import { TripMembershipService } from '../../../../src/nest/trip-membership/trip-membership.service';
+import { createSnapshotTestDb } from '../../../helpers/db-mock';
+import { createTestModule, type TestModule } from '../../../helpers/test-module';
 
-import { createECDH, randomBytes } from 'node:crypto';
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 
-const { logError, safeFetchFollow } = vi.hoisted(() => ({ logError: vi.fn(), safeFetchFollow: vi.fn() }));
-vi.mock('../../../../src/nest/audit/audit-log.logger', () => ({
-  LOG_LEVEL: 'error',
-  logInfo: vi.fn(),
-  logDebug: vi.fn(),
-  logError,
-  logWarn: vi.fn(),
-}));
-vi.mock('../../../../src/utils/ssrfGuard', () => ({
-  SsrfBlockedError: class SsrfBlockedError extends Error {},
-  safeFetchFollow,
-}));
-// The repository's read runs inside a request context; the fake repository needs none.
-vi.mock('../../../../src/nest/database/request-context', () => ({
-  withRequestContext: (_orm: unknown, fn: () => unknown) => fn(),
-}));
-// The socket registry has no rooms here; only the listeners matter.
-vi.mock('../../../../src/nest/realtime/ws-state', () => ({
-  broadcast: vi.fn(),
-  broadcastToUser: vi.fn(),
-  getOnlineUserIds: () => new Set<number>(),
-}));
-
-const APNS = 'https://push.liketrek.com/v1/apns/production/' + 'ab'.repeat(32);
-const FCM = 'https://push.liketrek.com/v1/fcm/' + 'x'.repeat(140);
-const BROWSER = 'https://fcm.googleapis.com/fcm/send/abc';
+const db = createSnapshotTestDb();
 
 describe('WatchSyncPushService', () => {
+  let t: TestModule<RealtimeService>;
   let realtime: RealtimeService;
-  let sendWatchSync: ReturnType<typeof vi.fn>;
-  let rosterUserIds: ReturnType<typeof vi.fn>;
   let service: WatchSyncPushService;
+  const sendWatchSync = vi.fn();
+  const getOwnerId = vi.fn();
+  const listMemberUserIds = vi.fn();
 
-  beforeEach(() => {
-    vi.useFakeTimers();
-    logError.mockClear();
+  beforeEach(async () => {
+    sendWatchSync.mockReset().mockResolvedValue(true);
+    getOwnerId.mockReset().mockResolvedValue(1);
+    listMemberUserIds.mockReset().mockResolvedValue([2, 3]);
     realtime = new RealtimeService();
-    sendWatchSync = vi.fn().mockResolvedValue(true);
-    rosterUserIds = vi.fn().mockResolvedValue(new Set([1, 2, 3]));
-    service = new WatchSyncPushService(
+    t = await createTestModule({
+      db,
+      imports: [],
       realtime,
-      { sendWatchSync } as unknown as WebPushService,
-      { rosterUserIds } as unknown as TripMembersRepository,
-      {} as MikroORM,
-    );
-    service.onModuleInit();
+      providers: [
+        WatchSyncPushService,
+        { provide: WebPushService, useValue: { sendWatchSync } },
+        { provide: TripMembershipService, useValue: { getOwnerId, listMemberUserIds } },
+      ],
+      init: true,
+    });
+    service = t.get(WatchSyncPushService);
+    // Only now: the module compiles on real timers.
+    vi.useFakeTimers();
   });
 
-  afterEach(() => {
-    service.onModuleDestroy();
+  afterEach(async () => {
     vi.useRealTimers();
+    await t.close();
   });
 
-  it('WSYNC-001: a run of list changes on a trip becomes one sync to every member, once it settled', async () => {
+  it('WSYNC-001: a run of list changes on a trip becomes one sync to its owner and members, once it settled', async () => {
     realtime.broadcast(12, 'packing:updated', { item: {} } as never);
     realtime.broadcast(12, 'todo:updated', { item: {} } as never);
     realtime.broadcast(12, 'packing:deleted', { itemId: 4 } as never);
     expect(sendWatchSync).not.toHaveBeenCalled();
 
     await vi.advanceTimersByTimeAsync(WATCH_SYNC_SETTLE_MS);
-    expect(rosterUserIds).toHaveBeenCalledTimes(1);
-    expect(rosterUserIds).toHaveBeenCalledWith('12');
+    expect(listMemberUserIds).toHaveBeenCalledTimes(1);
+    expect(listMemberUserIds).toHaveBeenCalledWith('12');
     expect(sendWatchSync.mock.calls.map(([id]) => id).sort()).toEqual([1, 2, 3]);
   });
 
@@ -99,7 +75,7 @@ describe('WatchSyncPushService', () => {
   it('WSYNC-003: a private packing item syncs its owner alone, unless the trip changed for everyone too', async () => {
     realtime.broadcast(12, 'packing:updated', { item: {} } as never, undefined, 2);
     await vi.advanceTimersByTimeAsync(WATCH_SYNC_SETTLE_MS);
-    expect(rosterUserIds).not.toHaveBeenCalled();
+    expect(listMemberUserIds).not.toHaveBeenCalled();
     expect(sendWatchSync.mock.calls).toEqual([[2]]);
 
     sendWatchSync.mockClear();
@@ -109,18 +85,19 @@ describe('WatchSyncPushService', () => {
     expect(sendWatchSync.mock.calls.map(([id]) => id).sort()).toEqual([1, 2, 3]);
   });
 
-  it('WSYNC-004: each trip gathers on its own', async () => {
+  it('WSYNC-004: each trip gathers on its own, and a trip without an owner row still reaches its members', async () => {
+    getOwnerId.mockResolvedValueOnce(null);
     realtime.broadcast(12, 'todo:updated', { item: {} } as never);
     realtime.broadcast(13, 'todo:updated', { item: {} } as never);
     await vi.advanceTimersByTimeAsync(WATCH_SYNC_SETTLE_MS);
-    expect(rosterUserIds.mock.calls).toEqual([['12'], ['13']]);
+    expect(listMemberUserIds.mock.calls).toEqual([['12'], ['13']]);
+    expect(sendWatchSync).toHaveBeenCalledTimes(5);
   });
 
-  it('WSYNC-005: a failed read is logged and never thrown into the broadcast', async () => {
-    rosterUserIds.mockRejectedValue(new Error('db gone'));
+  it('WSYNC-005: a failed read is swallowed with a log line and never thrown into the broadcast', async () => {
+    listMemberUserIds.mockRejectedValue(new Error('db gone'));
     realtime.broadcast(12, 'todo:updated', { item: {} } as never);
     await vi.advanceTimersByTimeAsync(WATCH_SYNC_SETTLE_MS);
-    expect(logError).toHaveBeenCalledWith(expect.stringContaining('Watch sync failed trip=12: db gone'));
     expect(sendWatchSync).not.toHaveBeenCalled();
   });
 
@@ -150,85 +127,3 @@ describe('RealtimeService.onTripBroadcast', () => {
     expect(heard).toHaveBeenCalledTimes(1);
   });
 });
-
-describe('WebPushService.sendWatchSync', () => {
-  const keys = generateVapidKeyPair();
-
-  function row(id: number, endpoint: string): PushSubscriptionRow {
-    const device = createECDH('prime256v1');
-    device.generateKeys();
-    return {
-      id,
-      user_id: 5,
-      endpoint,
-      p256dh: device.getPublicKey().toString('base64url'),
-      auth: randomBytes(16).toString('base64url'),
-      vapid_public_key: keys.publicKey,
-      user_agent: null,
-      created_at: '2026-11-08T00:00:00Z',
-      last_success_at: null,
-      failure_count: 0,
-    };
-  }
-
-  function service(rows: PushSubscriptionRow[], available = true) {
-    const vapid = {
-      isAvailable: vi.fn().mockResolvedValue(available),
-      getKeys: vi.fn().mockResolvedValue({ ...keys, source: 'database' }),
-      getSubject: () => 'mailto:admin@trek.example.test',
-    } as unknown as VapidKeysService;
-    const subscriptions = {
-      listForUser: vi.fn().mockResolvedValue(rows),
-      recordSuccess: vi.fn(),
-      recordFailure: vi.fn().mockResolvedValue(1),
-      deleteById: vi.fn(),
-    } as unknown as PushSubscriptionsService;
-    return new WebPushService(vapid, subscriptions);
-  }
-
-  beforeEach(() => {
-    safeFetchFollow.mockReset().mockResolvedValue(new Response(null, { status: 201 }));
-  });
-
-  it('WSYNC-008: only the iPhone app gets it, as a short-lived background push the relay recognises', async () => {
-    const sent = await service([row(1, APNS), row(2, FCM), row(3, BROWSER)]).sendWatchSync(5);
-    expect(sent).toBe(true);
-    expect(safeFetchFollow).toHaveBeenCalledTimes(1);
-    const [endpoint, init] = safeFetchFollow.mock.calls[0];
-    expect(endpoint).toBe(APNS);
-    expect(init.headers).toMatchObject({
-      [BACKGROUND_PUSH_HEADER]: 'background',
-      Topic: 'trek-watch-sync',
-      Urgency: 'low',
-      TTL: '600',
-      'Content-Encoding': 'aes128gcm',
-    });
-  });
-
-  it('WSYNC-009: a user without the iPhone app, or a server without keys, sends nothing', async () => {
-    expect(await service([row(2, FCM), row(3, BROWSER)]).sendWatchSync(5)).toBe(false);
-    expect(await service([row(1, APNS)], false).sendWatchSync(5)).toBe(false);
-    expect(safeFetchFollow).not.toHaveBeenCalled();
-  });
-
-  it('WSYNC-010: an ordinary notification still goes out without the background mark', async () => {
-    await service([row(1, APNS)]).sendToUser(5, {
-      event: 'collab_message',
-      title: 'Hi',
-      body: 'there',
-      navigateTarget: '/trips/1',
-      url: 'https://trek.example.test/trips/1',
-    });
-    const [, init] = safeFetchFollow.mock.calls[0];
-    expect(init.headers[BACKGROUND_PUSH_HEADER]).toBeUndefined();
-    expect(init.headers.TTL).toBe(String(24 * 60 * 60));
-  });
-
-  it("WSYNC-011: only the relay's APNs endpoints count as an iPhone", () => {
-    expect(isAppleRelayEndpoint(APNS)).toBe(true);
-    expect(isAppleRelayEndpoint(FCM)).toBe(false);
-    expect(isAppleRelayEndpoint(BROWSER)).toBe(false);
-    expect(isAppleRelayEndpoint('http://push.liketrek.com/v1/apns/production/ab')).toBe(false);
-    expect(isAppleRelayEndpoint('not a url')).toBe(false);
-  });
-});

server/tests/unit/nest/notifications/web-push.service.test.ts

@@ -18,10 +18,12 @@ import {
 } from '../../../../src/nest/notifications/push/vapid-keys.service';
 import { generateVapidKeyPair } from '../../../../src/nest/notifications/push/web-push-crypto';
 import {
+  BACKGROUND_PUSH_HEADER,
   FORBIDDEN_FAILURES_BEFORE_REMOVAL,
   MAX_PUSH_PAYLOAD_BYTES,
   WebPushService,
   buildPushPayload,
+  isAppleRelayEndpoint,
   samePagePath,
 } from '../../../../src/nest/notifications/transports/web-push.service';
 import { SsrfBlockedError } from '../../../../src/utils/ssrfGuard';
@@ -469,6 +471,57 @@ describe('WebPushService delivery', () => {
   });
 });
 
+describe('WebPushService.sendWatchSync', () => {
+  const APNS = `https://push.liketrek.com/v1/apns/production/${'ab'.repeat(32)}`;
+  const FCM = `https://push.liketrek.com/v1/fcm/${'x'.repeat(140)}`;
+  const BROWSER = 'https://fcm.googleapis.com/fcm/send/watch-sync-browser';
+
+  it('WSYNC-008: only the iPhone app gets it, as a short-lived background push the relay recognises', async () => {
+    const { user } = createUser(testDb);
+    await subscribe(user.id, APNS);
+    await subscribe(user.id, FCM);
+    await subscribe(user.id, BROWSER);
+    safeFetchFollow.mockResolvedValue(reply(201));
+
+    expect(await push.sendWatchSync(user.id)).toBe(true);
+    expect(safeFetchFollow).toHaveBeenCalledTimes(1);
+    const [url, init] = safeFetchFollow.mock.calls[0];
+    expect(url).toBe(APNS);
+    expect(init.headers).toMatchObject({
+      [BACKGROUND_PUSH_HEADER]: 'background',
+      Topic: 'trek-watch-sync',
+      Urgency: 'low',
+      TTL: '600',
+      'Content-Encoding': 'aes128gcm',
+    });
+  });
+
+  it('WSYNC-009: a user without the iPhone app sends nothing', async () => {
+    const { user } = createUser(testDb);
+    await subscribe(user.id, BROWSER);
+    expect(await push.sendWatchSync(user.id)).toBe(false);
+    expect(safeFetchFollow).not.toHaveBeenCalled();
+  });
+
+  it('WSYNC-010: an ordinary notification to the same app goes out without the background mark', async () => {
+    const { user } = createUser(testDb);
+    await subscribe(user.id, APNS);
+    safeFetchFollow.mockResolvedValue(reply(201));
+    await push.sendToUser(user.id, MSG);
+    const [, init] = safeFetchFollow.mock.calls[0];
+    expect(init.headers[BACKGROUND_PUSH_HEADER]).toBeUndefined();
+    expect(init.headers.TTL).toBe(String(24 * 60 * 60));
+  });
+
+  it("WSYNC-011: only the relay's APNs endpoints count as an iPhone", () => {
+    expect(isAppleRelayEndpoint(APNS)).toBe(true);
+    expect(isAppleRelayEndpoint(FCM)).toBe(false);
+    expect(isAppleRelayEndpoint(BROWSER)).toBe(false);
+    expect(isAppleRelayEndpoint(`http://push.liketrek.com/v1/apns/production/${'ab'.repeat(32)}`)).toBe(false);
+    expect(isAppleRelayEndpoint('not a url')).toBe(false);
+  });
+});
+
 describe('buildPushPayload', () => {
   const size = (p: unknown) => Buffer.byteLength(JSON.stringify(p), 'utf8');