[rooms] roles

apps/notify/package.json

@@ -15,6 +15,7 @@
 		"test": "run-vitest"
 	},
 	"dependencies": {
+		"@repo/domain": "workspace:*",
 		"@repo/hono-helpers": "workspace:*",
 		"@repo/jwt": "workspace:*",
 		"hono": "4.13.5",

apps/notify/src/context.ts

@@ -5,6 +5,12 @@ import type { NotificationsHub } from './notifications-hub'
 export type Env = SharedHonoEnv & {
 	/** Durable Object hosting the SignalR notifications hub. */
 	RECFLARE_NOTIFICATIONS_HUB: DurableObjectNamespace<NotificationsHub>
+	/**
+	 * The shared `recflare` D1 database. Used only for the notification store — a coach
+	 * message to one player is written there before it is pushed, so it survives the hub's
+	 * best-effort queue. The hub DO itself keeps its own state in its own SQLite.
+	 */
+	DB: D1Database
 	/**
 	 * HS256 JWT signing key, read with `await env.JWT_SECRET.get()`; all workers
 	 * bind the same store so tokens signed by `auth` verify here. Used to gate the

apps/notify/src/notification-payloads.ts

@@ -236,7 +236,12 @@ export interface MessageReceivedPayload {
 	/** ISO-8601. */
 	SentTime: string
 	Type: number
-	Data: string
+	/**
+	 * Always a STRING on the wire — a payload with structure to it goes in escaped, never
+	 * as a nested object. Null on the message types that carry no data of their own (a
+	 * room-role invite, say), which the hub then drops from the frame entirely.
+	 */
+	Data: string | null
 	RoomId: number | null
 	PlayerEventId: number | null
 }

apps/notify/src/notifications-hub.ts

@@ -115,11 +115,15 @@ export interface HubState {
  */
 export const MAX_PENDING_PER_PLAYER = 500
 
-/** The Coach system account — the `FromPlayerId` on a coach message (see coachMessageAll). */
-const COACH_PLAYER_ID = 1
+/**
+ * The Coach system account — the `FromPlayerId` on a coach message (see coachMessageAll).
+ * Exported because the targeted send stores its message before pushing it, and the row has
+ * to name the same sender the frame does.
+ */
+export const COACH_PLAYER_ID = 1
 
 /** The Message `Type` a coach/system message carries (a Message-model enum, not a NotificationType). */
-const COACH_MESSAGE_TYPE = 100
+export const COACH_MESSAGE_TYPE = 100
 
 export class NotificationsHub extends DurableObject<Env> {
 	constructor(ctx: DurableObjectState, env: Env) {
@@ -131,7 +135,7 @@ export class NotificationsHub extends DurableObject<Env> {
 		// way — it just leaves a trace when it happens.
 		ctx
 			.blockConcurrencyWhile(async () => {
-			this.ctx.storage.sql.exec(`
+				this.ctx.storage.sql.exec(`
 				CREATE TABLE IF NOT EXISTS subscriptions (
 					connectionId TEXT NOT NULL,
 					playerId INTEGER NOT NULL
@@ -550,20 +554,26 @@ export class NotificationsHub extends DurableObject<Env> {
 	 * the Coach account (player 1), addressed to one player with a `ToPlayerId` — the shape
 	 * `api`'s player-to-player send uses.
 	 *
+	 * Takes the ALREADY-STORED message rather than building one, so the frame carries the
+	 * row's `Id` and `SentTime` and the player can read the same message back from
+	 * `GET /api/messages/v2/get`. The worker route writes the row (the DO has no business
+	 * doing D1 writes — every hub call funnels through this one global instance).
+	 *
 	 * Unlike the broadcast this one QUEUES when the recipient is offline (see
 	 * {@link notifyPlayer}). The broadcast is online-only because it has no recipient to
-	 * hold anything for; a message written to a named player is worth keeping until they
-	 * next connect.
+	 * hold anything for — and, for the same reason, nothing to store: a message written to
+	 * a named player is worth keeping until they next connect.
 	 */
-	async coachMessage(
-		playerId: number,
-		content: string
-	): Promise<{ delivered: number; queued: boolean }> {
-		return this.notifyPlayer(playerId, NotificationType.MessageReceived, {
-			FromPlayerId: COACH_PLAYER_ID,
-			ToPlayerId: playerId,
-			Type: COACH_MESSAGE_TYPE,
-			Data: content,
+	async coachMessage(message: {
+		Id: number
+		FromPlayerId: number
+		ToPlayerId: number
+		SentTime: string
+		Type: number
+		Data: string | null
+	}): Promise<{ delivered: number; queued: boolean }> {
+		return this.notifyPlayer(message.ToPlayerId, NotificationType.MessageReceived, {
+			...message,
 		})
 	}
 

apps/notify/src/notify.app.ts

@@ -1,10 +1,16 @@
 import { Hono } from 'hono'
 import { useWorkersLogger } from 'workers-tagged-logger'
 
+import { createNotification } from '@repo/domain'
 import { logger, withDefaultCors, withNotFound, withOnError } from '@repo/hono-helpers'
 import { validateAndGetAccountId, validateAndGetRoles } from '@repo/jwt'
 
-import { NotificationsHub, OWNER_HEADER } from './notifications-hub'
+import {
+	COACH_MESSAGE_TYPE,
+	COACH_PLAYER_ID,
+	NotificationsHub,
+	OWNER_HEADER,
+} from './notifications-hub'
 
 import type { Context, MiddlewareHandler } from 'hono'
 import type { App } from './context'
@@ -228,9 +234,13 @@ const app = new Hono<App>()
 		return c.json({ success: true, ...result })
 	})
 
-	// The targeted form of the broadcast above: one coach message to ONE player. Queued
-	// by the hub when they're offline (unlike coach-message-all, which reaches only
-	// whoever is connected), so this arrives either way.
+	// The targeted form of the broadcast above: one coach message to ONE player.
+	//
+	// Unlike the broadcast, this one is a real message: it is STORED first, so the player
+	// reads it from `GET /api/messages/v2/get` whether or not the hub reached them, and the
+	// frame carries the stored row's id. The broadcast stays unstored — it reaches everyone
+	// online at once, so a row per player would be thousands of writes per maintenance
+	// notice, for something nobody needs to re-read.
 	.post('/internal/coach-message', async (c) => {
 		const body = await c.req
 			.json<{ playerId?: number; messageContent?: string }>()
@@ -240,9 +250,15 @@ const app = new Hono<App>()
 			return c.json({ error: 'playerId is required' }, 400)
 		}
 		if (content === '') return c.json({ error: 'messageContent is required' }, 400)
-		const { playerId } = body
-		const result = await hubCall(c, (hub) => hub.coachMessage(playerId, content))
-		return c.json({ success: true, ...result })
+
+		const notification = await createNotification(c.env.DB, {
+			FromPlayerId: COACH_PLAYER_ID,
+			ToPlayerId: body.playerId,
+			Type: COACH_MESSAGE_TYPE,
+			Data: content,
+		})
+		const result = await hubCall(c, (hub) => hub.coachMessage(notification))
+		return c.json({ success: true, notificationId: notification.Id, ...result })
 	})
 
 	// Read-only view of the hub's routing state, for working out why a notification

apps/notify/src/test/integration/api.test.ts

@@ -2,11 +2,13 @@ import { adminSecretsStore, env, runInDurableObject } from 'cloudflare:test'
 import { exports } from 'cloudflare:workers'
 import { beforeAll, describe, expect, test } from 'vitest'
 
+import { NOTIFICATION_SCHEMA_DDL } from '@repo/domain'
+
 import '../../notify.app'
 
-import type { Env } from '../../context'
 import { MAX_PENDING_PER_PLAYER } from '../../notifications-hub'
 
+import type { Env } from '../../context'
 import type { HubState } from '../../notifications-hub'
 
 declare module 'cloudflare:test' {
@@ -50,6 +52,9 @@ async function bearer(
 // Seed the shared JWT signing key into the local Secrets Store so .get() resolves.
 beforeAll(async () => {
 	await adminSecretsStore(env.JWT_SECRET).create(TEST_SECRET)
+	// The notification store a targeted coach message writes to. Owned by the `api` worker's
+	// migrations, which don't run here, so build it from the mirrored DDL.
+	for (const ddl of NOTIFICATION_SCHEMA_DDL) await env.DB.prepare(ddl).run()
 })
 
 /** Who `connect` is by default — kept clear of the player ids the tests notify. */
@@ -402,6 +407,43 @@ describe('notification delivery', () => {
 		ws.close()
 	})
 
+	test('coach-message stores a notification; the broadcast does not', async () => {
+		const stored = async (playerId: number) =>
+			(
+				await env.DB.prepare(
+					'SELECT notification_id, from_player_id, type, data FROM notification WHERE to_player_id = ?1'
+				)
+					.bind(playerId)
+					.all<{ notification_id: number; from_player_id: number; type: number; data: string }>()
+			).results
+
+		// A message to ONE player is a real message: stored, so they read it from their
+		// inbox on next login whether or not the hub reached them, and the frame carries
+		// the stored row's id.
+		const playerId = 9013
+		const res = await post('/internal/coach-message', { playerId, messageContent: 'kept' })
+		const { notificationId } = (await res.json()) as { notificationId: number }
+
+		const rows = await stored(playerId)
+		expect(rows).toHaveLength(1)
+		expect(rows[0]).toMatchObject({
+			notification_id: notificationId,
+			from_player_id: 1,
+			type: 100,
+			data: 'kept',
+		})
+
+		// The BROADCAST stores nothing — it reaches everyone online at once, so a row per
+		// player would be thousands of writes per maintenance notice.
+		const listener = await connect('coach-nostore', {
+			headers: await bearer('9014', ['gameClient']),
+		})
+		await post('/internal/coach-message-all', { messageContent: 'everyone' })
+		await listener.waitFor((r) => r.type === 1 && r.target === 'Notification')
+		expect(await stored(9014)).toEqual([])
+		listener.ws.close()
+	})
+
 	test('coach-message 400s without a player or a message', async () => {
 		expect((await post('/internal/coach-message', { messageContent: 'nobody' })).status).toBe(400)
 		expect((await post('/internal/coach-message', { playerId: 9012 })).status).toBe(400)

apps/notify/wrangler.jsonc

@@ -8,6 +8,21 @@
 		"bindings": [{ "name": "RECFLARE_NOTIFICATIONS_HUB", "class_name": "NotificationsHub" }]
 	},
 	"migrations": [{ "tag": "v1", "new_sqlite_classes": ["NotificationsHub"] }],
+	// The shared `recflare` D1 database, for the message store behind
+	// `GET /api/messages/v2/get`: a coach message to ONE player is a durable message and is
+	// written here before it is pushed. No `migrations_dir` — the `api` worker owns this
+	// schema (apps/api/migrations/0019_message.sql); notify only writes rows.
+	//
+	// The coach BROADCAST is deliberately not stored: it reaches everyone online at once, so
+	// persisting it would write a row per player — thousands per maintenance notice, for
+	// something nobody needs to re-read.
+	"d1_databases": [
+		{
+			"binding": "DB",
+			"database_name": "recflare",
+			"database_id": "local"
+		}
+	],
 	// Shared Secrets Store holding the HS256 JWT signing key. Every worker binds the
 	// same store as JWT_SECRET so tokens signed by `auth` verify here. The "local"
 	// store_id placeholder is replaced with RECFLARE_SECRETS_STORE at deploy time.