From 51364e482c216d86967b242b2abc654736a8a1d1 Mon Sep 17 00:00:00 2001 From: Devin Zuczek Date: Thu, 6 Aug 2026 15:35:35 -0400 Subject: [PATCH] fix subroom save description clobbering room description --- CLAUDE.md | 6 +++++ apps/rooms/src/openapi.ts | 15 +++++++++--- apps/rooms/src/rooms.app.ts | 15 ++++++------ apps/rooms/src/test/integration/api.test.ts | 23 +++++++++++++++--- packages/domain/src/rooms-db.ts | 27 ++++++++++++++------- 5 files changed, 62 insertions(+), 24 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 334318c..5d2af0a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -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 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. +- 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 `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 diff --git a/apps/rooms/src/openapi.ts b/apps/rooms/src/openapi.ts index 5bbcf79..fcf66bf 100644 --- a/apps/rooms/src/openapi.ts +++ b/apps/rooms/src/openapi.ts @@ -238,6 +238,7 @@ export const SubRoomDto = z.object({ RoomDataBlob: z.string().optional().describe('Uploaded room-data key; absent until first save'), DataSavedAt: z.string().optional().describe('ISO timestamp of the last save'), 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. */ @@ -310,7 +311,10 @@ export const RoomDto = z.object({ PromoExternalContent: z.array(z.unknown()), LoadScreens: z.array(LoadScreenDto), 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`) — search, hot, similar. */ @@ -598,9 +602,12 @@ export const SaveSubRoomDataRequest = z.object({ .object({ Filename: z.string() }) .optional() .describe('The uploaded room-level data blob — becomes `RoomDataBlob`'), - Description: z.string().optional().describe('The save comment; also written to the ROOM'), - PersistenceVersion: z.int().optional(), - InventionUsage: z.string().optional().describe('Written to the room'), + Description: z + .string() + .optional() + .describe('The save comment — a description of THIS revision, not the room’s 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'), AutoPublish: z .boolean() diff --git a/apps/rooms/src/rooms.app.ts b/apps/rooms/src/rooms.app.ts index d116392..61ecc49 100644 --- a/apps/rooms/src/rooms.app.ts +++ b/apps/rooms/src/rooms.app.ts @@ -1931,9 +1931,9 @@ const app = new Hono() // 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 - // the uploaded data blobs and records the room-level save fields, notifies the - // owner, and returns the updated ROOM in the lowercase `{ success, error, value }` - // envelope the reference's SetRoomData uses. + // the uploaded data blobs and records the revision's fields against that SUBROOM, + // notifies the owner, and returns the updated ROOM in the lowercase + // `{ success, error, value }` envelope the reference's SetRoomData uses. .post( '/rooms/:roomId{[0-9]+}/subrooms/:subRoomId{[0-9]+}/data', describeRoute({ @@ -1941,10 +1941,11 @@ const app = new Hono() summary: 'Save a subroom’s data (room save)', description: [ '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`,', - '`PersistenceVersion`, `InventionUsage`) are written to the room. Editable by the', - 'room’s creator or a co-owner (403 otherwise); a missing token is an EMPTY-body 401,', - 'unlike the other room writes.', + 'through the `storage` worker. Everything the body carries describes THAT revision', + 'and lands on the save and its subroom — `Description` is the save comment, NOT the', + 'room’s description (only `PUT /rooms/{roomId}/description` sets that). Nothing here', + 'writes to the room. Editable by the room’s 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', 'lands on `StagedSubRoomDataSaveId` with the live `CurrentSave` untouched, so', diff --git a/apps/rooms/src/test/integration/api.test.ts b/apps/rooms/src/test/integration/api.test.ts index 953a7a6..e91966c 100644 --- a/apps/rooms/src/test/integration/api.test.ts +++ b/apps/rooms/src/test/integration/api.test.ts @@ -1376,6 +1376,15 @@ describe('rooms endpoints', () => { }) 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 // `error` is null (not ''). This fixture sends `AutoPublish: true`, so it goes live. const ok = await authed(2, 2, '1') @@ -1390,7 +1399,7 @@ describe('rooms endpoints', () => { } expect(saved.success).toBe(true) 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. expect(saved.value.subRoomDataSave).toEqual({ @@ -1430,6 +1439,7 @@ describe('rooms endpoints', () => { SubRoomDataSaveId: number SavedByAccountId: number PersistenceVersion: number + Description: string UnitySubAssets: unknown[] Tags: unknown[] } @@ -1446,13 +1456,18 @@ describe('rooms endpoints', () => { expect(sub.CurrentSave.SubRoomDataSaveId).toBeGreaterThan(0) 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 { Description: string PersistenceVersion: number + InventionUsage?: string } - expect(room.Description).toBe('mydescription here') - expect(room.PersistenceVersion).toBe(41) + expect(room.Description).toBe(before.Description) + 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 // with the room envelope. The creator stays account 1 (not clobbered). diff --git a/packages/domain/src/rooms-db.ts b/packages/domain/src/rooms-db.ts index c78d8e1..1fb962c 100644 --- a/packages/domain/src/rooms-db.ts +++ b/packages/domain/src/rooms-db.ts @@ -447,6 +447,11 @@ export interface SaveSubRoomDataInput { subRoomDataHash?: string /** Uploaded blob key for the room-level METADATA blob (a separate upload). */ 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 persistenceVersion?: number 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 - * save carries. Returns the updated (hydrated) room AND the save that was just created — - * the route answers with both — or null when the room or subroom doesn't exist. + * Persist a room-save against a specific subroom. Everything the save carries belongs to + * that subroom's revision — nothing is written to the room. Returns the updated + * (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 * 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), savedByAccountId: accountId, // 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 ?? '', createdAt: new Date().toISOString(), unityAssetId: input.unityAssetId, @@ -629,11 +635,15 @@ export async function saveSubRoomData( if (input.roomDataFilename) sub.RoomDataBlob = input.roomDataFilename sub.DataSavedAt = new Date().toISOString() if (input.persistenceVersion !== undefined) sub.PersistenceVersion = input.persistenceVersion + if (input.inventionUsage !== undefined) sub.InventionUsage = input.inventionUsage - // Room-level fields carried by the save. - if (typeof input.description === 'string') room.Description = input.description - if (input.persistenceVersion !== undefined) room.PersistenceVersion = input.persistenceVersion - if (input.inventionUsage !== undefined) room.InventionUsage = input.inventionUsage + // Nothing here touches the ROOM. A room save is a revision of one SUBROOM, and every + // field it carries describes that revision: `Description` is the save comment shown in + // the `…/saves` history, `PersistenceVersion` and `InventionUsage` describe the scene + // 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 // 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 .prepare('UPDATE subroom SET data = ?2 WHERE sub_room_id = ?1') .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.