feat: push dispatch, file soft/hard-delete, push hygiene & coalescing

apps/backend/drizzle/0009_push_file_hygiene.sql

@@ -0,0 +1,18 @@
+-- #231: files table for tracking S3 storage objects with soft/hard delete
+CREATE TABLE IF NOT EXISTS "files" (
+  "id" uuid PRIMARY KEY DEFAULT gen_random_uuid() NOT NULL,
+  "storage_key" text NOT NULL,
+  "deleted_at" timestamp,
+  "hard_deleted_at" timestamp,
+  "created_at" timestamp NOT NULL DEFAULT now()
+);--> statement-breakpoint
+ALTER TABLE "files" ADD CONSTRAINT "files_storage_key_unique" UNIQUE("storage_key");--> statement-breakpoint
+
+-- #231: link messages to their S3 file object
+ALTER TABLE "messages" ADD COLUMN "file_id" uuid;--> statement-breakpoint
+ALTER TABLE "messages" ADD CONSTRAINT "messages_file_id_files_id_fk" FOREIGN KEY ("file_id") REFERENCES "public"."files"("id") ON DELETE set null ON UPDATE no action;--> statement-breakpoint
+
+-- #237: push subscription hygiene columns
+ALTER TABLE "push_subscriptions" ADD COLUMN "last_used_at" timestamp;--> statement-breakpoint
+ALTER TABLE "push_subscriptions" ADD COLUMN "disabled_at" timestamp;--> statement-breakpoint
+CREATE INDEX IF NOT EXISTS "push_subscriptions_device_active_idx" ON "push_subscriptions" ("device_id") WHERE "disabled_at" IS NULL;

apps/backend/package.json

@@ -40,6 +40,7 @@
     "postgres": "^3.4.9",
     "redis": "^6.0.0",
     "socket.io": "^4.8.3",
+    "web-push": "^3.6.7",
     "zod": "^4.4.3"
   },
   "devDependencies": {
@@ -50,6 +51,7 @@
     "@types/morgan": "^1.9.10",
     "@types/node": "^20.19.37",
     "@types/supertest": "^7.2.0",
+    "@types/web-push": "^3.6.4",
     "@vitest/coverage-v8": "^4.1.6",
     "drizzle-kit": "^0.31.10",
     "eslint": "^9.39.4",

apps/backend/src/__tests__/fileCleanup.test.ts

@@ -0,0 +1,110 @@
+/**
+ * Tests for file cleanup service (#231).
+ */
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+
+// ── S3 mock (must use vi.hoisted so it's available in the factory) ────────────
+const mockS3Send = vi.hoisted(() => vi.fn());
+
+vi.mock('@aws-sdk/client-s3', () => ({
+  S3Client: class MockS3 {
+    send = mockS3Send;
+  },
+  DeleteObjectCommand: class MockDeleteCmd {
+    constructor(public input: unknown) {}
+  },
+}));
+
+// ── DB mock ───────────────────────────────────────────────────────────────────
+const mockFindMany = vi.fn();
+const mockUpdate = vi.fn();
+const mockExecute = vi.fn();
+
+vi.mock('../db/index.js', () => ({
+  db: {
+    query: { files: { findMany: mockFindMany } },
+    update: mockUpdate,
+    execute: mockExecute,
+  },
+}));
+
+vi.mock('../db/schema.js', () => ({
+  files: { id: 'id', deletedAt: 'deleted_at', hardDeletedAt: 'hard_deleted_at' },
+}));
+
+vi.mock('drizzle-orm', () => ({
+  isNotNull: vi.fn((col: unknown) => ({ col, isNotNull: true })),
+  isNull: vi.fn((col: unknown) => ({ col, isNull: true })),
+  sql: Object.assign(
+    vi.fn((strings: TemplateStringsArray, ...vals: unknown[]) => ({ strings, vals })),
+    { raw: vi.fn() },
+  ),
+}));
+
+vi.mock('../services/pushNotification.js', () => ({
+  reenableExpiredBackoffs: vi.fn().mockResolvedValue(undefined),
+}));
+
+const mockSetFn = vi.fn().mockReturnThis();
+const mockWhereFn = vi.fn().mockResolvedValue(undefined);
+
+beforeEach(() => {
+  vi.clearAllMocks();
+  mockS3Send.mockResolvedValue(undefined);
+  mockUpdate.mockReturnValue({ set: mockSetFn });
+  mockSetFn.mockReturnValue({ where: mockWhereFn });
+});
+
+const { softDeleteFile, runHardDeletePass } = await import('../services/fileCleanup.js');
+
+describe('#231 – softDeleteFile', () => {
+  it('calls db.update with deletedAt set on the matching file', async () => {
+    await softDeleteFile('file-uuid-1');
+    expect(mockUpdate).toHaveBeenCalled();
+    expect(mockSetFn).toHaveBeenCalledWith({ deletedAt: expect.any(Date) });
+  });
+});
+
+describe('#231 – runHardDeletePass', () => {
+  it('skips files that still have live message references', async () => {
+    mockFindMany.mockResolvedValue([{ id: 'file-1', storageKey: 'key-1' }]);
+    mockExecute.mockResolvedValueOnce([{ '?column?': 1 }]); // live ref exists
+
+    await runHardDeletePass();
+
+    expect(mockS3Send).not.toHaveBeenCalled();
+    expect(mockSetFn).not.toHaveBeenCalled();
+  });
+
+  it('hard-deletes from S3 and marks hardDeletedAt when no live refs', async () => {
+    mockFindMany.mockResolvedValue([{ id: 'file-2', storageKey: 'key-2' }]);
+    mockExecute.mockResolvedValueOnce([]); // no live refs
+
+    await runHardDeletePass();
+
+    expect(mockS3Send).toHaveBeenCalledTimes(1);
+    expect(mockSetFn).toHaveBeenCalledWith({ hardDeletedAt: expect.any(Date) });
+  });
+
+  it('does not mark hardDeletedAt when S3 delete throws (safe retry)', async () => {
+    mockFindMany.mockResolvedValue([{ id: 'file-3', storageKey: 'key-3' }]);
+    mockExecute.mockResolvedValueOnce([]);
+    mockS3Send.mockRejectedValueOnce(new Error('NoSuchKey'));
+
+    await runHardDeletePass();
+
+    expect(mockSetFn).not.toHaveBeenCalledWith({ hardDeletedAt: expect.any(Date) });
+  });
+
+  it('processes multiple files in one pass', async () => {
+    mockFindMany.mockResolvedValue([
+      { id: 'file-a', storageKey: 'key-a' },
+      { id: 'file-b', storageKey: 'key-b' },
+    ]);
+    mockExecute.mockResolvedValue([]); // no live refs for either
+
+    await runHardDeletePass();
+
+    expect(mockS3Send).toHaveBeenCalledTimes(2);
+  });
+});

apps/backend/src/__tests__/messages.routes.test.ts

@@ -59,6 +59,10 @@ vi.mock('drizzle-orm', () => ({
   sql: vi.fn(),
 }));
 
+vi.mock('../services/fileCleanup.js', () => ({
+  softDeleteFile: vi.fn().mockResolvedValue(undefined),
+}));
+
 vi.mock('../middleware/auth.js', () => ({
   requireAuth: (req: express.Request, _res: express.Response, next: express.NextFunction) => {
     (req as express.Request & { auth: { userId: string } }).auth = { userId: 'user-1' };

apps/backend/src/__tests__/pushNotification.test.ts

@@ -0,0 +1,215 @@
+/**
+ * Tests for push notification service (#236, #237, #239).
+ */
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+
+// ── DB mock ────────────────────────────────────────────────────────────────────
+const mockUpdate = vi.fn();
+const mockDelete = vi.fn();
+const mockExecute = vi.fn();
+const mockFindMany = vi.fn();
+
+vi.mock('../db/index.js', () => ({
+  db: {
+    query: { pushSubscriptions: { findMany: mockFindMany } },
+    update: mockUpdate,
+    delete: mockDelete,
+    execute: mockExecute,
+  },
+}));
+
+vi.mock('../db/schema.js', () => ({
+  pushSubscriptions: { id: 'id', deviceId: 'device_id', disabledAt: 'disabled_at' },
+}));
+
+vi.mock('drizzle-orm', () => ({
+  and: vi.fn((...args: unknown[]) => args),
+  eq: vi.fn((col: unknown, val: unknown) => ({ col, val })),
+  isNull: vi.fn((col: unknown) => ({ col, isNull: true })),
+  sql: Object.assign(
+    vi.fn((strings: TemplateStringsArray, ...vals: unknown[]) => ({ strings, vals })),
+    { raw: vi.fn() },
+  ),
+}));
+
+// ── web-push mock ──────────────────────────────────────────────────────────────
+const mockSendNotification = vi.fn();
+vi.mock('web-push', () => ({
+  default: {
+    setVapidDetails: vi.fn(),
+    sendNotification: mockSendNotification,
+  },
+}));
+
+// ── deviceRevocation mock ──────────────────────────────────────────────────────
+const mockIsDeviceConnected = vi.fn();
+vi.mock('../services/deviceRevocation.js', () => ({
+  isDeviceConnected: mockIsDeviceConnected,
+}));
+
+process.env['VAPID_PUBLIC_KEY'] = 'test-public-key';
+process.env['VAPID_PRIVATE_KEY'] = 'test-private-key';
+
+const { dispatchOfflinePush } = await import('../services/pushNotification.js');
+
+const mockSetFn = vi.fn().mockReturnThis();
+const mockWhereFn = vi.fn().mockResolvedValue(undefined);
+const mockDeleteWhereFn = vi.fn().mockResolvedValue(undefined);
+
+beforeEach(() => {
+  vi.clearAllMocks();
+  mockUpdate.mockReturnValue({ set: mockSetFn });
+  mockSetFn.mockReturnValue({ where: mockWhereFn });
+  mockDelete.mockReturnValue({ where: mockDeleteWhereFn });
+  mockExecute.mockResolvedValue(undefined);
+});
+
+afterEach(() => {
+  vi.useRealTimers();
+});
+
+// Use unique device IDs per test to avoid rate-limit state bleed between tests.
+let testDeviceCounter = 0;
+function uniqueDevice(): string {
+  return `dev-push-test-${++testDeviceCounter}`;
+}
+
+describe('#236 – dispatchOfflinePush', () => {
+  it('skips devices that are connected', async () => {
+    vi.useFakeTimers();
+    mockIsDeviceConnected.mockReturnValue(true);
+
+    await dispatchOfflinePush('conv-1', 'msg-1', [uniqueDevice()]);
+    await vi.runAllTimersAsync();
+
+    expect(mockFindMany).not.toHaveBeenCalled();
+  });
+
+  it('queues push for offline devices and sends after coalesce window', async () => {
+    vi.useFakeTimers();
+    mockIsDeviceConnected.mockReturnValue(false);
+    mockFindMany.mockResolvedValue([
+      { id: 'sub-1', endpoint: 'https://push.example.com/sub1', p256dh: 'p256', auth: 'auth' },
+    ]);
+    mockSendNotification.mockResolvedValue(undefined);
+
+    await dispatchOfflinePush('conv-2', 'msg-2', [uniqueDevice()]);
+    await vi.runAllTimersAsync();
+
+    expect(mockFindMany).toHaveBeenCalled();
+    expect(mockSendNotification).toHaveBeenCalledWith(
+      { endpoint: 'https://push.example.com/sub1', keys: { p256dh: 'p256', auth: 'auth' } },
+      expect.stringContaining('"type":"new_message"'),
+    );
+  });
+
+  it('payload is content-free: no ciphertext or sender data', async () => {
+    vi.useFakeTimers();
+    mockIsDeviceConnected.mockReturnValue(false);
+    mockFindMany.mockResolvedValue([
+      { id: 'sub-2', endpoint: 'https://push.example.com/s2', p256dh: 'p', auth: 'a' },
+    ]);
+    mockSendNotification.mockResolvedValue(undefined);
+
+    await dispatchOfflinePush('conv-3', 'msg-3', [uniqueDevice()]);
+    await vi.runAllTimersAsync();
+
+    const [, payloadStr] = mockSendNotification.mock.calls[0] as [unknown, string];
+    const payload = JSON.parse(payloadStr) as Record<string, unknown>;
+    expect(payload).toHaveProperty('type', 'new_message');
+    expect(payload).toHaveProperty('conversationId', 'conv-3');
+    expect(payload).toHaveProperty('messageId', 'msg-3');
+    expect(payload).not.toHaveProperty('ciphertext');
+    expect(payload).not.toHaveProperty('content');
+    expect(payload).not.toHaveProperty('sender');
+  });
+});
+
+describe('#239 – coalescing', () => {
+  it('coalesces burst into single push with accurate count', async () => {
+    vi.useFakeTimers();
+    mockIsDeviceConnected.mockReturnValue(false);
+    mockFindMany.mockResolvedValue([
+      { id: 'sub-c', endpoint: 'https://push.example.com/sc', p256dh: 'p', auth: 'a' },
+    ]);
+    mockSendNotification.mockResolvedValue(undefined);
+
+    const dev = uniqueDevice();
+    await dispatchOfflinePush('conv-burst', 'msg-b1', [dev]);
+    await dispatchOfflinePush('conv-burst', 'msg-b2', [dev]);
+    await dispatchOfflinePush('conv-burst', 'msg-b3', [dev]);
+    await vi.runAllTimersAsync();
+
+    // Only one push must be sent
+    expect(mockSendNotification).toHaveBeenCalledTimes(1);
+
+    const [, payloadStr] = mockSendNotification.mock.calls[0] as [unknown, string];
+    const payload = JSON.parse(payloadStr) as Record<string, unknown>;
+    expect(payload).toHaveProperty('count', 3);
+    expect(payload).toHaveProperty('messageId', 'msg-b3');
+  });
+});
+
+describe('#237 – push hygiene', () => {
+  it('prunes dead subscription on 410 Gone', async () => {
+    vi.useFakeTimers();
+    mockIsDeviceConnected.mockReturnValue(false);
+    mockFindMany.mockResolvedValue([
+      { id: 'sub-dead', endpoint: 'https://gone.example.com', p256dh: 'p', auth: 'a' },
+    ]);
+    const err = Object.assign(new Error('Gone'), { statusCode: 410 });
+    mockSendNotification.mockRejectedValue(err);
+
+    await dispatchOfflinePush('conv-4', 'msg-4', [uniqueDevice()]);
+    await vi.runAllTimersAsync();
+
+    expect(mockDelete).toHaveBeenCalled();
+    expect(mockUpdate).not.toHaveBeenCalled();
+  });
+
+  it('prunes dead subscription on 404 Not Found', async () => {
+    vi.useFakeTimers();
+    mockIsDeviceConnected.mockReturnValue(false);
+    mockFindMany.mockResolvedValue([
+      { id: 'sub-404', endpoint: 'https://notfound.example.com', p256dh: 'p', auth: 'a' },
+    ]);
+    const err = Object.assign(new Error('Not Found'), { statusCode: 404 });
+    mockSendNotification.mockRejectedValue(err);
+
+    await dispatchOfflinePush('conv-5', 'msg-5', [uniqueDevice()]);
+    await vi.runAllTimersAsync();
+
+    expect(mockDelete).toHaveBeenCalled();
+  });
+
+  it('backs off on transient 500 error (sets disabledAt)', async () => {
+    vi.useFakeTimers();
+    mockIsDeviceConnected.mockReturnValue(false);
+    mockFindMany.mockResolvedValue([
+      { id: 'sub-500', endpoint: 'https://push.example.com/s500', p256dh: 'p', auth: 'a' },
+    ]);
+    const err = Object.assign(new Error('Server Error'), { statusCode: 500 });
+    mockSendNotification.mockRejectedValue(err);
+
+    await dispatchOfflinePush('conv-6', 'msg-6', [uniqueDevice()]);
+    await vi.runAllTimersAsync();
+
+    expect(mockUpdate).toHaveBeenCalled();
+    expect(mockSetFn).toHaveBeenCalledWith({ disabledAt: expect.any(Date) });
+    expect(mockDelete).not.toHaveBeenCalled();
+  });
+
+  it('updates lastUsedAt on successful delivery', async () => {
+    vi.useFakeTimers();
+    mockIsDeviceConnected.mockReturnValue(false);
+    mockFindMany.mockResolvedValue([
+      { id: 'sub-ok', endpoint: 'https://push.example.com/sok', p256dh: 'p', auth: 'a' },
+    ]);
+    mockSendNotification.mockResolvedValue(undefined);
+
+    await dispatchOfflinePush('conv-7', 'msg-7', [uniqueDevice()]);
+    await vi.runAllTimersAsync();
+
+    expect(mockSetFn).toHaveBeenCalledWith({ lastUsedAt: expect.any(Date) });
+  });
+});