fix subroom save description clobbering room description

This commit is contained in:
Devin Zuczek
2026-08-06 15:35:35 -04:00
parent 565d9b1aea
commit 51364e482c
5 changed files with 62 additions and 24 deletions
+6
View File
@@ -102,6 +102,12 @@ inconsistency here without checking the client first.
`…/saves` is real history and `publish_save` doubles as restore-a-save. `…/saves` is `…/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 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. is no `GET …/subrooms/:sid/data`; only the POST (the room save) exists on that path.
- A room save writes ONLY to the subroom and its save row — never to the room. Everything
the body carries describes that one revision: `Description` is the save comment shown in
`…/saves`, and `PersistenceVersion`/`InventionUsage` describe the scene just saved (the
latter lives on the SUBROOM). The room's public description is `PUT /rooms/:id/description`'s
alone; copying the save comment onto `room.Description` (as this once did) silently
replaces the room's description every time someone saves.
- Matchmaking (`match`: `/matchmake/room/:roomId/:subRoomId`) always serves the PUBLISHED - Matchmaking (`match`: `/matchmake/room/:roomId/:subRoomId`) always serves the PUBLISHED
`CurrentSave` blob, creator included. Joining a private instance, the client itself asks `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 the owner whether to load the latest or the published version and resolves it from the
+11 -4
View File
@@ -238,6 +238,7 @@ export const SubRoomDto = z.object({
RoomDataBlob: z.string().optional().describe('Uploaded room-data key; absent until first save'), RoomDataBlob: z.string().optional().describe('Uploaded room-data key; absent until first save'),
DataSavedAt: z.string().optional().describe('ISO timestamp of the last save'), DataSavedAt: z.string().optional().describe('ISO timestamp of the last save'),
PersistenceVersion: z.int().optional(), PersistenceVersion: z.int().optional(),
InventionUsage: z.string().optional().describe('Recorded by a room save; absent until then'),
}) })
/** A room's localization settings — carried through verbatim; nothing localizes yet. */ /** A room's localization settings — carried through verbatim; nothing localizes yet. */
@@ -310,7 +311,10 @@ export const RoomDto = z.object({
PromoExternalContent: z.array(z.unknown()), PromoExternalContent: z.array(z.unknown()),
LoadScreens: z.array(LoadScreenDto), LoadScreens: z.array(LoadScreenDto),
RestrictedCircuitsAllowListNames: z.array(z.string()), RestrictedCircuitsAllowListNames: z.array(z.string()),
InventionUsage: z.string().optional().describe('Recorded by a room save; absent until then'), InventionUsage: z
.string()
.optional()
.describe('Legacy: room saves used to write this here; it now lives on the SUBROOM'),
}) })
/** A paged room list (`PagedResultsDTO<RoomDTO>`) — search, hot, similar. */ /** A paged room list (`PagedResultsDTO<RoomDTO>`) — search, hot, similar. */
@@ -598,9 +602,12 @@ export const SaveSubRoomDataRequest = z.object({
.object({ Filename: z.string() }) .object({ Filename: z.string() })
.optional() .optional()
.describe('The uploaded room-level data blob — becomes `RoomDataBlob`'), .describe('The uploaded room-level data blob — becomes `RoomDataBlob`'),
Description: z.string().optional().describe('The save comment; also written to the ROOM'), Description: z
PersistenceVersion: z.int().optional(), .string()
InventionUsage: z.string().optional().describe('Written to the room'), .optional()
.describe('The save comment — a description of THIS revision, not the rooms description'),
PersistenceVersion: z.int().optional().describe('Recorded on the save and the subroom'),
InventionUsage: z.string().optional().describe('Recorded on the subroom'),
UnityAssetId: z.string().nullable().optional().describe('Recorded on the save when set'), UnityAssetId: z.string().nullable().optional().describe('Recorded on the save when set'),
AutoPublish: z AutoPublish: z
.boolean() .boolean()
+8 -7
View File
@@ -1931,9 +1931,9 @@ const app = new Hono<App>()
// Save a subroom's data (room save). Auth-gated (401 with empty body). Editable // Save a subroom's data (room save). Auth-gated (401 with empty body). Editable
// by the room creator or a Creator/CoOwner role holder. Points the subroom at // by the room creator or a Creator/CoOwner role holder. Points the subroom at
// the uploaded data blobs and records the room-level save fields, notifies the // the uploaded data blobs and records the revision's fields against that SUBROOM,
// owner, and returns the updated ROOM in the lowercase `{ success, error, value }` // notifies the owner, and returns the updated ROOM in the lowercase
// envelope the reference's SetRoomData uses. // `{ success, error, value }` envelope the reference's SetRoomData uses.
.post( .post(
'/rooms/:roomId{[0-9]+}/subrooms/:subRoomId{[0-9]+}/data', '/rooms/:roomId{[0-9]+}/subrooms/:subRoomId{[0-9]+}/data',
describeRoute({ describeRoute({
@@ -1941,10 +1941,11 @@ const app = new Hono<App>()
summary: 'Save a subrooms data (room save)', summary: 'Save a subrooms data (room save)',
description: [ description: [
'Records a save against the subroom from the blobs the client has already uploaded', 'Records a save against the subroom from the blobs the client has already uploaded',
'through the `storage` worker; the room-level fields it carries (`Description`,', 'through the `storage` worker. Everything the body carries describes THAT revision',
'`PersistenceVersion`, `InventionUsage`) are written to the room. Editable by the', 'and lands on the save and its subroom — `Description` is the save comment, NOT the',
'rooms creator or a co-owner (403 otherwise); a missing token is an EMPTY-body 401,', 'rooms description (only `PUT /rooms/{roomId}/description` sets that). Nothing here',
'unlike the other room writes.', 'writes to the room. Editable by the rooms creator or a co-owner (403 otherwise); a',
'missing token is an EMPTY-body 401, unlike the other room writes.',
'', '',
'`AutoPublish: true` makes the save live immediately. Otherwise it is STAGED: it', '`AutoPublish: true` makes the save live immediately. Otherwise it is STAGED: it',
'lands on `StagedSubRoomDataSaveId` with the live `CurrentSave` untouched, so', 'lands on `StagedSubRoomDataSaveId` with the live `CurrentSave` untouched, so',
+19 -4
View File
@@ -1376,6 +1376,15 @@ describe('rooms endpoints', () => {
}) })
expect(await envOf(await authed(2, 9999, '1'))).toMatchObject({ success: false }) expect(await envOf(await authed(2, 9999, '1'))).toMatchObject({ success: false })
// The room's own fields are read first: a save is a revision of a SUBROOM and must
// leave them alone. `Description` in the body is the save comment, not the room's
// description — that is `PUT /rooms/:id/description`'s to set.
const before = (await (await SELF.fetch(`${ORIGIN}/rooms/2`)).json()) as {
Description: string
PersistenceVersion: number
}
expect(before.Description).not.toBe('mydescription here')
// Owner saves → 200. `value` carries BOTH the updated room and the new save, and // Owner saves → 200. `value` carries BOTH the updated room and the new save, and
// `error` is null (not ''). This fixture sends `AutoPublish: true`, so it goes live. // `error` is null (not ''). This fixture sends `AutoPublish: true`, so it goes live.
const ok = await authed(2, 2, '1') const ok = await authed(2, 2, '1')
@@ -1390,7 +1399,7 @@ describe('rooms endpoints', () => {
} }
expect(saved.success).toBe(true) expect(saved.success).toBe(true)
expect(saved.error).toBeNull() expect(saved.error).toBeNull()
expect(saved.value.room).toMatchObject({ RoomId: 2, Description: 'mydescription here' }) expect(saved.value.room).toMatchObject({ RoomId: 2, Description: before.Description })
// The save is a camelCase projection, NOT the PascalCase CurrentSave shape. // The save is a camelCase projection, NOT the PascalCase CurrentSave shape.
expect(saved.value.subRoomDataSave).toEqual({ expect(saved.value.subRoomDataSave).toEqual({
@@ -1430,6 +1439,7 @@ describe('rooms endpoints', () => {
SubRoomDataSaveId: number SubRoomDataSaveId: number
SavedByAccountId: number SavedByAccountId: number
PersistenceVersion: number PersistenceVersion: number
Description: string
UnitySubAssets: unknown[] UnitySubAssets: unknown[]
Tags: unknown[] Tags: unknown[]
} }
@@ -1446,13 +1456,18 @@ describe('rooms endpoints', () => {
expect(sub.CurrentSave.SubRoomDataSaveId).toBeGreaterThan(0) expect(sub.CurrentSave.SubRoomDataSaveId).toBeGreaterThan(0)
expect(sub.StagedSubRoomDataSaveId).toBeNull() expect(sub.StagedSubRoomDataSaveId).toBeNull()
// Room-level fields land on the room too. // The save comment and the scene fields land on the SUBROOM's revision, and the
// room's own fields are untouched — a save must never rewrite the room.
expect(sub.CurrentSave.Description).toBe('mydescription here')
expect(sub).toMatchObject({ PersistenceVersion: 41, InventionUsage: 'CAE=' })
const room = (await (await SELF.fetch(`${ORIGIN}/rooms/2`)).json()) as { const room = (await (await SELF.fetch(`${ORIGIN}/rooms/2`)).json()) as {
Description: string Description: string
PersistenceVersion: number PersistenceVersion: number
InventionUsage?: string
} }
expect(room.Description).toBe('mydescription here') expect(room.Description).toBe(before.Description)
expect(room.PersistenceVersion).toBe(41) expect(room.PersistenceVersion).toBe(before.PersistenceVersion)
expect(room.InventionUsage).toBeUndefined()
// A CoOwner (account 2 holds Role 30 in the seeded rooms) may also save — 200 // A CoOwner (account 2 holds Role 30 in the seeded rooms) may also save — 200
// with the room envelope. The creator stays account 1 (not clobbered). // with the room envelope. The creator stays account 1 (not clobbered).
+18 -9
View File
@@ -447,6 +447,11 @@ export interface SaveSubRoomDataInput {
subRoomDataHash?: string subRoomDataHash?: string
/** Uploaded blob key for the room-level METADATA blob (a separate upload). */ /** Uploaded blob key for the room-level METADATA blob (a separate upload). */
roomDataFilename?: string roomDataFilename?: string
/**
* The save comment — a description of THIS revision, typed into the client's save box.
* It belongs to the save (and shows up in the `…/saves` history); it is not the room's
* public description, which only `PUT /rooms/:id/description` sets.
*/
description?: string description?: string
persistenceVersion?: number persistenceVersion?: number
inventionUsage?: string inventionUsage?: string
@@ -561,9 +566,10 @@ function legacySubRoomSave(sub: SubRoom): SubRoomDataSave | null {
} }
/** /**
* Persist a room-save against a specific subroom and record the room-level fields the * Persist a room-save against a specific subroom. Everything the save carries belongs to
* save carries. Returns the updated (hydrated) room AND the save that was just created * that subroom's revision — nothing is written to the room. Returns the updated
* the route answers with both — or null when the room or subroom doesn't exist. * (hydrated) room AND the save that was just created — the route answers with both — or
* null when the room or subroom doesn't exist.
* *
* Whether the save goes live is the client's call: `AutoPublish: true` publishes it * Whether the save goes live is the client's call: `AutoPublish: true` publishes it
* outright, otherwise it becomes the subroom's `staged_save_id` with the live * outright, otherwise it becomes the subroom's `staged_save_id` with the live
@@ -619,7 +625,7 @@ export async function saveSubRoomData(
input.persistenceVersion ?? (typeof priorVersion === 'number' ? priorVersion : 0), input.persistenceVersion ?? (typeof priorVersion === 'number' ? priorVersion : 0),
savedByAccountId: accountId, savedByAccountId: accountId,
// The save comment — empty string, not null, when the save carries none (the // The save comment — empty string, not null, when the save carries none (the
// reference's `roomDesc ?? ""`). Also written to the room below. // reference's `roomDesc ?? ""`).
description: input.description ?? '', description: input.description ?? '',
createdAt: new Date().toISOString(), createdAt: new Date().toISOString(),
unityAssetId: input.unityAssetId, unityAssetId: input.unityAssetId,
@@ -629,11 +635,15 @@ export async function saveSubRoomData(
if (input.roomDataFilename) sub.RoomDataBlob = input.roomDataFilename if (input.roomDataFilename) sub.RoomDataBlob = input.roomDataFilename
sub.DataSavedAt = new Date().toISOString() sub.DataSavedAt = new Date().toISOString()
if (input.persistenceVersion !== undefined) sub.PersistenceVersion = input.persistenceVersion if (input.persistenceVersion !== undefined) sub.PersistenceVersion = input.persistenceVersion
if (input.inventionUsage !== undefined) sub.InventionUsage = input.inventionUsage
// Room-level fields carried by the save. // Nothing here touches the ROOM. A room save is a revision of one SUBROOM, and every
if (typeof input.description === 'string') room.Description = input.description // field it carries describes that revision: `Description` is the save comment shown in
if (input.persistenceVersion !== undefined) room.PersistenceVersion = input.persistenceVersion // the `…/saves` history, `PersistenceVersion` and `InventionUsage` describe the scene
if (input.inventionUsage !== undefined) room.InventionUsage = input.inventionUsage // just saved. They used to be copied onto the room as well, which meant each save
// silently replaced the room's public description with the save comment. The room's own
// fields are edited through their own routes (`PUT /rooms/:id/description` and
// friends), so the room row is not rewritten here at all.
// Publish outright when the client asked to (`AutoPublish`), or for a dorm — a dorm is // Publish outright when the client asked to (`AutoPublish`), or for a dorm — a dorm is
// the player's own private space with no publish step in the client, so staging one // the player's own private space with no publish step in the client, so staging one
@@ -653,7 +663,6 @@ export async function saveSubRoomData(
db db
.prepare('UPDATE subroom SET data = ?2 WHERE sub_room_id = ?1') .prepare('UPDATE subroom SET data = ?2 WHERE sub_room_id = ?1')
.bind(subRoomId, serializeSubRoom(sub, roomId)), .bind(subRoomId, serializeSubRoom(sub, roomId)),
db.prepare('UPDATE room SET data = ?2 WHERE room_id = ?1').bind(roomId, serializeRoom(room)),
]) ])
// Re-hydrate so the returned room reflects the just-saved subroom. // Re-hydrate so the returned room reflects the just-saved subroom.