Revert "fix issue with subroom save list not working in room with no current save"

This reverts commit aeff7d50cd.

CLAUDE.md

@@ -90,29 +90,20 @@ inconsistency here without checking the client first.
   no asset arrays; but `unityAsset`/`unityAssetHash`). Don't unify the two projections.
 - A subroom's saved scene loads from `CurrentSave.DataBlob` (`rooms`: `GET /rooms/:id`),
   NOT the flat `DataBlob` on the subroom — a subroom with no `CurrentSave` silently loads
-  nothing. Read it via `subRoomDataBlob()` so `match`/`auth` instance payloads resolve it
-  the same way. It resolves in `attachCurrentSaves`, which every room read goes through:
-  the PUBLISHED save (`current_save_id`), falling back to the STAGED one
-  (`staged_save_id`) when nothing has been published yet, and with NEITHER the key is
-  omitted entirely — a `CurrentSave: null` breaks the client on every endpoint that serves
-  rooms (not just the save flow — `/rooms/ownedby/me` and friends too). Don't reintroduce
-  a null in a hand-built subroom payload.
+  nothing. The key must be present (null before the first publish); read it via
+  `subRoomDataBlob()` so `match`/`auth` instance payloads resolve it the same way.
 - A room save (`rooms`: `POST …/subrooms/:sid/data`) publishes only when the body says
   `AutoPublish: true`; otherwise it STAGES onto `StagedSubRoomDataSaveId` and leaves
-  `current_save_id` alone, so players keep loading the last published version until the
-  owner posts `…/subrooms/:sid/publish_save` with `subRoomDataSaveId=<id>` — except before
-  the FIRST publish, where `CurrentSave` falls back to the staged save (there is no older
-  version to keep serving, and a subroom that has been saved shouldn't read as empty).
-  DORMS always
+  `CurrentSave` alone, so players keep loading the last published version until the owner
+  posts `…/subrooms/:sid/publish_save` with `subRoomDataSaveId=<id>`. DORMS always
   publish: no publish step exists in the client for them. Saves live in the
   `subroom_save` table with globally-unique ids (a bare id has to resolve —
   `StagedSubRoomDataSaveId` carries no subroom context), and nothing is overwritten, so
   `…/saves` is real history and `publish_save` doubles as restore-a-save. `…/saves` is
   auth-gated and CREATOR-only (not co-owners) — it lists unpublished staged saves. There
   is no `GET …/subrooms/:sid/data`; only the POST (the room save) exists on that path.
-- Matchmaking (`match`: `/matchmake/room/:roomId/:subRoomId`) serves whatever
-  `CurrentSave` resolves to — the published save once there is one — to everyone alike,
-  creator included. Joining a private instance, the client itself asks
+- Matchmaking (`match`: `/matchmake/room/:roomId/:subRoomId`) always serves the PUBLISHED
+  `CurrentSave` blob, creator included. Joining a private instance, the client itself asks
   the owner whether to load the latest or the published version and resolves it from the
   `/subrooms/:sid/saves` list — the matchmake call is identical either way. Don't make
   this server-side: it would put two people in one instance on different versions.

apps/rooms/src/openapi.ts

@@ -213,10 +213,9 @@ export const SubRoomDataSaveDto = z.object({
  * sequence, not per-room); a room's `SubRooms` array is reconstructed on read.
  *
  * `CreatorAccountId` starts null on the seeded rooms and is filled in on the first save —
- * the client NREs on a null one. `CurrentSave` is the published save, or the staged one
- * when nothing has been published; with neither it is ABSENT rather than null (a null one
- * breaks the client's parser). The flat `DataBlob`/`RoomDataBlob`/`DataSavedAt` fields are
- * legacy and are NOT what the client loads from.
+ * the client NREs on a null one. `CurrentSave` is null until the first save; the flat
+ * `DataBlob`/`RoomDataBlob`/`DataSavedAt` fields are legacy and are NOT what the client
+ * loads from.
  */
 export const SubRoomDto = z.object({
 	SubRoomId: z.int(),
@@ -232,8 +231,8 @@ export const SubRoomDto = z.object({
 		.describe('0 Private, 1 Public, 2 Unlisted, 3 Dev_only, 4 Dev_Unlisted — set independently'),
 	ShouldAutoStageSaves: z.boolean(),
 	StagedSubRoomDataSaveId: z.int().nullable(),
-	CurrentSave: SubRoomDataSaveDto.optional().describe(
-		'The room save the client loads the scene blob from: the published one, falling back to the STAGED one when nothing has been published yet. Omitted entirely (never null) when the subroom has neither'
+	CurrentSave: SubRoomDataSaveDto.nullable().describe(
+		'The latest room save — where the client finds the scene blob. Null until first save'
 	),
 	DataBlob: z.string().optional().describe('Legacy flat key; the client reads `CurrentSave`'),
 	RoomDataBlob: z.string().optional().describe('Uploaded room-data key; absent until first save'),

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

@@ -1470,22 +1470,19 @@ describe('rooms endpoints', () => {
 		expect(coOwnerEnv.value.subRoomDataSave).toMatchObject({ savedByAccountId: 2 })
 	})
 
-	it('GET /rooms/:id omits CurrentSave entirely on a never-saved subroom', async () => {
-		// A null CurrentSave breaks the client's room parser, so an unsaved subroom has to
-		// carry no such key at all. `in` rather than a value check: absent and null differ
-		// here, and null is exactly what this must not be.
+	it('GET /rooms/:id gives every subroom a CurrentSave key (null before the first save)', async () => {
+		// The client loads a subroom's scene data from CurrentSave and nothing else, so
+		// the key must be PRESENT — the seeded rooms predate it and have no such field in
+		// their stored blob. `in` rather than a value check: absent and null differ here.
 		// Room 3 is seeded and never saved by another test (room 2 is the save fixture).
 		const room = (await (await SELF.fetch(`${ORIGIN}/rooms/3`)).json()) as {
 			SubRooms: Array<Record<string, unknown>>
 		}
 		expect(room.SubRooms.length).toBeGreaterThan(0)
 		for (const sub of room.SubRooms) {
-			expect('CurrentSave' in sub).toBe(false)
+			expect('CurrentSave' in sub).toBe(true)
+			expect(sub.CurrentSave).toBeNull()
 		}
-		// Not just missing from the parsed object — absent from the wire bytes too, which is
-		// what the client's parser actually reads.
-		const raw = await (await SELF.fetch(`${ORIGIN}/rooms/3`)).text()
-		expect(raw).not.toContain('"CurrentSave"')
 	})
 
 	it('a real client room-save body stages, and publish_save makes it live', async () => {
@@ -1496,7 +1493,7 @@ describe('rooms endpoints', () => {
 				body: JSON.stringify(body),
 			})
 		type Sub = {
-			CurrentSave?: Record<string, unknown>
+			CurrentSave: Record<string, unknown> | null
 			StagedSubRoomDataSaveId: number | null
 		}
 		const subOf = async () => (await subRoomOf(5, 5)) as unknown as Sub
@@ -1523,16 +1520,11 @@ describe('rooms endpoints', () => {
 		})
 		expect(res.status).toBe(200)
 
-		// Room 5 is not a dorm → the save is STAGED, not published. Nothing has ever been
-		// published here, so there is no older version to keep serving: CurrentSave falls
-		// back to the staged save rather than leaving the subroom reading as never-saved.
+		// Room 5 is not a dorm → staged, nothing live yet.
 		const stagedSub = await subOf()
+		expect(stagedSub.CurrentSave).toBeNull()
 		const firstId = stagedSub.StagedSubRoomDataSaveId!
 		expect(firstId).toBeGreaterThan(0)
-		expect(stagedSub.CurrentSave).toMatchObject({
-			SubRoomDataSaveId: firstId,
-			DataBlob: '2026-07-28/f176fc3b-scene',
-		})
 
 		// Publishing makes it what the loader fetches, and clears the staging slot.
 		expect((await publish(firstId)).status).toBe(200)
@@ -1556,7 +1548,7 @@ describe('rooms endpoints', () => {
 
 		// It's on the room read too — that's what the loader actually fetches.
 		const room = (await (await SELF.fetch(`${ORIGIN}/rooms/5`)).json()) as {
-			SubRooms: Array<{ SubRoomId: number; CurrentSave?: { SubRoomDataSaveId: number } }>
+			SubRooms: Array<{ SubRoomId: number; CurrentSave: { SubRoomDataSaveId: number } | null }>
 		}
 		expect(room.SubRooms.find((s) => s.SubRoomId === 5)!.CurrentSave!.SubRoomDataSaveId).toBe(
 			firstId
@@ -1701,9 +1693,9 @@ describe('rooms endpoints', () => {
 			headers: await bearer('1'),
 		})
 		const body = (await res.json()) as {
-			value: { SubRooms: Array<{ SubRoomId: number; CurrentSave?: { SubRoomId: number } }> }
+			value: { SubRooms: Array<{ SubRoomId: number; CurrentSave: { SubRoomId: number } | null }> }
 		}
-		const clone = body.value.SubRooms.find((s) => s.SubRoomId !== 2 && s.CurrentSave !== undefined)!
+		const clone = body.value.SubRooms.find((s) => s.SubRoomId !== 2 && s.CurrentSave !== null)!
 		expect(clone).toBeDefined()
 		// The copy's save must claim the COPY, or the client resolves it against the source.
 		expect(clone.CurrentSave!.SubRoomId).toBe(clone.SubRoomId)

packages/domain/src/rooms-db.ts

@@ -797,7 +797,7 @@ export async function createSubRoom(
 		LastModeratedSaveModerationState: 0,
 		ShouldAutoStageSaves: true,
 		// Nothing saved yet — the first room save mints one and points current_save_id
-		// at it. Until then the subroom reads with no `CurrentSave` key at all.
+		// at it. Until then the subroom reads with `CurrentSave: null`.
 	})
 	// Refresh the hydrated SubRooms so the returned room includes the one just inserted.
 	await attachSubRooms(db, [room])
@@ -863,11 +863,10 @@ const SUBROOM_COLUMNS = 'sub_room_id, room_id, data, current_save_id, staged_sav
 
 /**
  * Materialize a subroom row into its client shape, with the columns authoritative.
- * `CurrentSave` is left alone here and resolved by {@link attachCurrentSaves} — it lives
- * in `subroom_save`, and resolving it per row would be a query each. Callers must go
- * through the helpers below rather than this: the client reads the scene blob from
- * `CurrentSave` and nowhere else, so a subroom served without one loads nothing, and a
- * stale one spread out of the stored blob would point at another subroom's save.
+ * `CurrentSave` is left undefined here and filled in by {@link attachCurrentSaves} — it
+ * lives in `subroom_save`, and resolving it per row would be a query each. Callers must
+ * go through the helpers below so the key is never missing: the client reads the scene
+ * blob from `CurrentSave` and nowhere else, so a subroom without one loads nothing.
  */
 const parseSubRoomRow = (row: SubRoomRow): SubRoom => ({
 	...(JSON.parse(row.data) as SubRoom),
@@ -904,31 +903,12 @@ const serializeRoom = (room: Room): string => {
 	return JSON.stringify({ ...rest, Stats: storedStats(stats) })
 }
 
-/**
- * The save a subroom actually serves as its `CurrentSave`: the published one, or the
- * staged one when nothing has been published yet. Null when it has neither.
- */
-const liveSaveId = (row: SubRoomRow): number | null => row.current_save_id ?? row.staged_save_id
-
 /**
  * Fill in each subroom's `CurrentSave` from `subroom_save`, in ONE query for the whole
- * batch.
+ * batch. Every subroom ends up with the key present — null when it points at no save
+ * (never saved) or the pointer dangles — because the client's loader reads it directly.
  *
- * `current_save_id` (the published save) wins; a subroom that has none FALLS BACK to
- * `staged_save_id`, so a subroom whose only save is unpublished still serves that save as
- * its `CurrentSave` rather than reading as never-saved. Note what this means for staging:
- * before the first publish there is nothing older to keep serving, so the staged work is
- * what everyone loads — but once a save has been published, staging behaves as it did and
- * players keep loading the published version.
- *
- * A subroom with neither pointer (never saved, or both dangling) has the key REMOVED
- * rather than set to null: a null `CurrentSave` breaks the client's room parser, so an
- * unsaved subroom has to look like it has no such field at all. The key is deleted rather
- * than merely left unset because the stored blob of a subroom written before saves moved
- * to their own table can still carry a `CurrentSave: null` of its own, which
- * {@link parseSubRoomRow} would spread straight through.
- *
- * `rows` must line up with `subs` positionally; the pointers live on the row, not the
+ * `rows` must line up with `subs` positionally; the pointer lives on the row, not the
  * parsed blob.
  */
 async function attachCurrentSaves(
@@ -936,7 +916,7 @@ async function attachCurrentSaves(
 	subs: SubRoom[],
 	rows: SubRoomRow[]
 ): Promise<void> {
-	const saveIds = [...new Set(rows.map(liveSaveId).filter((id) => id != null))]
+	const saveIds = [...new Set(rows.map((r) => r.current_save_id).filter((id) => id != null))]
 	const byId = new Map<number, SubRoomDataSave>()
 	if (saveIds.length > 0) {
 		const placeholders = saveIds.map((_, i) => `?${i + 1}`).join(',')
@@ -950,10 +930,8 @@ async function attachCurrentSaves(
 		for (const r of results) byId.set(r.sub_room_data_save_id, parseSubRoomSaveRow(r))
 	}
 	subs.forEach((sub, i) => {
-		const id = liveSaveId(rows[i]!)
-		const save = id == null ? undefined : byId.get(id)
-		if (save) sub.CurrentSave = save
-		else delete sub.CurrentSave
+		const id = rows[i]!.current_save_id
+		sub.CurrentSave = id == null ? null : (byId.get(id) ?? null)
 	})
 }
 
@@ -1324,12 +1302,9 @@ export async function insertSubRoom(
 		...sub,
 		SubRoomId: subRoomId,
 		RoomId: roomId,
+		CurrentSave: null,
 		StagedSubRoomDataSaveId: null,
 	}
-	// A fresh subroom has no save, so the key is absent rather than null — same rule the
-	// read path applies (see {@link attachCurrentSaves}). The delete also drops the SOURCE's
-	// save that the spread carried on a clone; the copy below re-points it at its own row.
-	delete created.CurrentSave
 	// The permission overrides follow the copy too — they live in their own table (keyed by
 	// the id the caller is cloning FROM), so unlike the rest of the settings they aren't
 	// carried by the blob. A fresh subroom (`createSubRoom`) passes no id and copies nothing.