[rooms] fix subroom permissions UI for some photon perms

apps/rooms/src/openapi.ts

@@ -1023,7 +1023,12 @@ export const BakedUnityAssetBulkList = z.array(BakedUnityAssetBulkDto)
 
 /** One entry of the permission table the client applies when it spawns into a room. */
 export const RoomPermissionDto = z.object({
-	Override: z.boolean().describe('Always true on an entry that came from a subroom’s overrides'),
+	Override: z
+		.boolean()
+		.describe(
+			'True on an entry the room’s creator stored on the subroom at this role; false on a ' +
+				'default or inherited cell of a manager’s table. Always true on a non-manager’s rows'
+		),
 	Permission: z.string().describe('e.g. `CAN_USE_MAKER_PEN`, `CAN_SAVE_INVENTIONS`'),
 	Role: z.int().describe('The role tier the permission applies to (0 = everyone)'),
 	Type: z.int(),
@@ -1039,10 +1044,11 @@ export const RoomPermissionDto = z.object({
  * secret/algorithm we don't have, and our Photon setup accepts an empty token. The
  * global (Role 0) maker pen is granted only to the hardcoded dev accounts.
  *
- * `Permissions` is the default table with the overrides stored on the subroom the caller
- * is standing in merged over it (see
- * `PUT /rooms/{roomId}/subrooms/{subRoomId}/permissions`): an override replaces the
- * default with the same (`Permission`, `Role`), and one naming a new pair is appended.
+ * `Permissions` is built from the overrides stored on the subroom the caller is standing in
+ * (see `PUT /rooms/{roomId}/subrooms/{subRoomId}/permissions`). A manager (creator or
+ * co-owner) gets a row per (`Permission`, `Role`) for every role tier — what the permissions
+ * screen reads back cell by cell; everyone else gets only the values that reach their own
+ * role, at Role 0 and at their role.
  */
 export const PhotonAccessTokenDto = z.object({
 	Permissions: z.array(RoomPermissionDto),

apps/rooms/src/rooms.app.ts

@@ -291,24 +291,44 @@ function roleIn(room: Room, accountId: number): number {
 		.reduce((best, r) => Math.max(best, r.Role), Role.None as number)
 }
 
+/**
+ * The role tiers a manager's permission table carries a row for, lowest first — every
+ * member of the `Role` enum. The permissions screen draws a cell per (permission, role) and
+ * reads each cell from the row at exactly that role, so every tier needs its rows whether
+ * or not anything is stored there.
+ */
+const PERMISSION_ROLES = [Role.None, Role.Host, Role.Moderator, Role.CoOwner, Role.Creator]
+
 /**
  * Room permissions + Photon token the client needs to spawn into a room. `RoomInstanceId`
  * is the caller's current instance from presence (null when they aren't in one).
  * `PhotonAccessToken` stays empty — the reference server signs it via `ClientSecurity`,
  * whose secret/algorithm we don't have; our Photon setup accepts an empty token.
  *
- * The table is the CALLER'S: it is built for the role they hold in the room they are
- * standing in (`role`), from the subroom's stored overrides (`overrides`, see
- * `PUT …/subrooms/{subRoomId}/permissions`) over the role's defaults. A permission's value
- * for the caller is the override stored at their own role, else the one stored at the
- * highest lower role (a grant to "everyone" reaches a host too), else the default — managers
- * may build, nobody else may. Pairs the creator stored under a role the caller doesn't hold
- * are not theirs and are left out, so a visitor never sees — or gets — a co-owner's grants.
+ * The table depends on who is asking, because two different screens read it:
  *
- * Every value is served at Role 0 AND, for a role holder, again at their role, and every row
- * says `Override: true`: the client has been seen applying Role 0 rows to a visitor, and
- * which rows it reads for a role holder is not known, so both readings land on the same
- * values. The global maker pen stays with the hardcoded dev accounts, whatever the room says.
+ * A MANAGER — the creator or a co-owner (`role` at or above {@link Role.CoOwner}, the same
+ * set `canManageRoom` admits) — gets the WHOLE table, one row per (permission, role) for
+ * every tier in {@link PERMISSION_ROLES}. The permissions screen (fed by
+ * `PUT …/subrooms/{subRoomId}/permissions`) draws a cell per pair and reads each cell from
+ * the row at exactly that role, so a stored entry is served at its own role with
+ * `Override: true`, and every other cell carries the value that reaches that role with
+ * `Override: false` — the entry stored at the highest lower role, else the default. A
+ * cell with no row snaps back to the default the moment it is toggled (a Host grant served
+ * only at Role 0 and the creator's own role, as this once did, did exactly that), and a
+ * default served with `Override: true` can never be un-overridden, so the flags matter as
+ * much as the values.
+ *
+ * Everyone else gets THEIR rows only: each permission's value for the role they hold — the
+ * override stored at their own role, else the one stored at the highest lower role (a grant
+ * to "everyone" reaches a host too), else the default — served at Role 0 AND, for a role
+ * holder, again at their role, every row `Override: true`. The client has been seen applying
+ * Role 0 rows to a visitor, and which rows it reads for a host or moderator is not known, so
+ * both readings land on the same values. Pairs stored under a role they don't hold are not
+ * theirs and are left out, so a visitor never sees — or gets — a co-owner's grants.
+ *
+ * Defaults: managers may build, nobody else may ({@link buildsByDefault}). The global maker
+ * pen stays with the hardcoded dev accounts, whatever the room says.
  */
 function photonAccessToken(
 	accountId: number,
@@ -316,41 +336,64 @@ function photonAccessToken(
 	role: number = Role.None,
 	overrides: RoomPermission[] = []
 ) {
-	const perm = (Permission: string, Role: number, Value: string): RoomPermission => ({
-		Override: true,
-		Permission,
-		Role,
-		Type: 0,
-		Value,
-	})
-
-	// The override that reaches the caller for a permission: their own role's, else the
-	// highest role below theirs that has one.
-	const reaching = (permission: string): RoomPermission | undefined =>
+	const isBuild = (name: string): boolean =>
+		BUILD_PERMISSIONS.includes(name as (typeof BUILD_PERMISSIONS)[number])
+	const names = [...BUILD_PERMISSIONS, ...overrides.map((o) => o.Permission)].filter(
+		(name, i, all) => all.indexOf(name) === i
+	)
+	const storedAt = (permission: string, at: number): RoomPermission | undefined =>
+		overrides.find((o) => o.Permission === permission && o.Role === at)
+	// The override that reaches a role from below it: the highest role under `at` with one.
+	const reachingFromBelow = (permission: string, at: number): RoomPermission | undefined =>
 		overrides
-			.filter((o) => o.Permission === permission && o.Role <= role)
+			.filter((o) => o.Permission === permission && o.Role < at)
 			.reduce<RoomPermission | undefined>(
 				(best, o) => (best && best.Role > o.Role ? best : o),
 				undefined
 			)
-
-	const names = [...BUILD_PERMISSIONS, ...overrides.map((o) => o.Permission)].filter(
-		(name, i, all) => all.indexOf(name) === i
-	)
-	const values = new Map<string, string>()
-	for (const name of names) {
-		const stored = reaching(name)
-		if (stored !== undefined) values.set(name, stored.Value)
-		else if (BUILD_PERMISSIONS.includes(name as (typeof BUILD_PERMISSIONS)[number])) {
-			values.set(name, buildsByDefault(role) ? 'True' : 'False')
-		}
-		// A creator-added pair stored only under a higher role than the caller's is not theirs.
+	// The value a role holds for a permission with nothing stored at that role; undefined
+	// for a creator-added pair nothing below the role names either.
+	const inherited = (permission: string, at: number): string | undefined => {
+		const below = reachingFromBelow(permission, at)
+		if (below) return below.Value
+		return isBuild(permission) ? (buildsByDefault(at) ? 'True' : 'False') : undefined
 	}
-	if (MAKER_PEN_ACCOUNT_IDS.has(accountId)) values.set('CAN_USE_MAKER_PEN', 'True')
+	const row = (
+		Permission: string,
+		Role: number,
+		Value: string,
+		Override: boolean,
+		Type = 0
+	): RoomPermission => ({ Override, Permission, Role, Type, Value })
 
 	const permissions: RoomPermission[] = []
-	for (const roleRow of role === Role.None ? [Role.None] : [Role.None, role]) {
-		for (const [name, value] of values) permissions.push(perm(name, roleRow, value))
+	if (buildsByDefault(role)) {
+		// A manager: the whole table, grouped by role.
+		for (const at of PERMISSION_ROLES) {
+			for (const name of names) {
+				const own = storedAt(name, at)
+				if (own) permissions.push(row(name, at, own.Value, true, own.Type))
+				else {
+					const value = inherited(name, at)
+					if (value !== undefined) permissions.push(row(name, at, value, false))
+				}
+			}
+		}
+		if (MAKER_PEN_ACCOUNT_IDS.has(accountId)) {
+			const own = permissions.find((p) => p.Permission === 'CAN_USE_MAKER_PEN' && p.Role === role)
+			if (own) own.Value = 'True'
+		}
+	} else {
+		// Anyone else: their own values, at Role 0 and at their role.
+		const values = new Map<string, string>()
+		for (const name of names) {
+			const value = storedAt(name, role)?.Value ?? inherited(name, role)
+			if (value !== undefined) values.set(name, value)
+		}
+		if (MAKER_PEN_ACCOUNT_IDS.has(accountId)) values.set('CAN_USE_MAKER_PEN', 'True')
+		for (const at of role === Role.None ? [Role.None] : [Role.None, role]) {
+			for (const [name, value] of values) permissions.push(row(name, at, value, true))
+		}
 	}
 	return {
 		Permissions: permissions,
@@ -4087,15 +4130,19 @@ const app = new Hono<App>()
 				'',
 				'`Override` is the checkbox the client draws beside each permission, not data:',
 				'`true` stores `Value` for that pair, and `false` means “fall back to the default”, so',
-				'it DELETES any stored entry. Nothing is stored with `Override: false`, and reads',
-				'always serve `true`. `Value` is a string — usually `True`/`False`, but it is kept',
-				'verbatim, since not every permission’s UI is a True/False picker.',
+				'it DELETES any stored entry. Nothing is stored with `Override: false`; a stored entry',
+				'reads back with `true` at its own role. `Value` is a string — usually `True`/`False`,',
+				'but it is kept verbatim, since not every permission’s UI is a True/False picker.',
 				'',
-				'What this feeds is `GET /photon_access_token`, which builds each player’s table from',
-				'the entries that reach THEIR role in the room: an entry at their role, else one at a',
-				'lower role (a Role 0 grant reaches everyone), over the role’s defaults. One naming a',
-				'pair the defaults don’t carry (e.g. `CAN_INVITE`) is served too. The overrides apply',
-				'to the subroom the caller is standing in, resolved from presence.',
+				'What this feeds is `GET /photon_access_token`. For a manager it serves the WHOLE',
+				'table — a row per (permission, role) for every role tier, stored entries with',
+				'`Override: true` and the rest `false` — which is what this screen reads back: a',
+				'cell with no row at its exact role snaps back to the default as soon as it is',
+				'toggled. For everyone else it serves only the entries that reach THEIR role: one at',
+				'their role, else one at a lower role (a Role 0 grant reaches everyone), over the',
+				'role’s defaults. One naming a pair the defaults don’t carry (e.g. `CAN_INVITE`) is',
+				'served too. The overrides apply to the subroom the caller is standing in, resolved',
+				'from presence.',
 				'',
 				'Gated to the room’s managers — its creator or a co-owner (`canManageRoom`), as the',
 				'other room-admin writes are. Answers the whole ROOM under `Value` in the PascalCase',
@@ -4633,13 +4680,19 @@ const app = new Hono<App>()
 				'`RoomInstanceId` is the caller’s current instance, read from the shared `presence`',
 				'table (null when they’re in none).',
 				'',
-				'The table is built for the CALLER: their role in the room they are standing in',
-				'(Creator, co-owner, moderator, host, or a visitor) decides each permission’s value —',
-				'the entry the room’s creator stored on that subroom at their role',
-				'(`PUT …/subrooms/{subRoomId}/permissions`), else one stored at a lower role, else the',
-				'default: managers (co-owner and up) may build, nobody else may. Entries stored under',
-				'a role the caller does not hold are left out. Every value is served at Role 0 and,',
-				'for a role holder, again at their role, all with `Override: true`.',
+				'The table depends on who asks. A MANAGER — the creator or a co-owner — gets the',
+				'whole table: a row per (permission, role) for every role tier (0, 10, 20, 30, 255),',
+				'because the permissions screen (`PUT …/subrooms/{subRoomId}/permissions`) reads',
+				'each cell from the row at exactly that role. An entry stored on the subroom is',
+				'served at its own role with `Override: true`; every other cell carries the value',
+				'that reaches that role (the entry at the highest lower role, else the default) with',
+				'`Override: false`, so an un-overridden cell shows and stays at its default.',
+				'',
+				'Everyone else gets their own rows only: each permission’s value for the role they',
+				'hold in the room they are standing in — the entry stored at their role, else one',
+				'stored at a lower role, else the default: managers may build, nobody else may.',
+				'Entries stored under a role they do not hold are left out. Every value is served at',
+				'Role 0 and, for a host or moderator, again at their role, all with `Override: true`.',
 				'',
 				'`PhotonAccessToken` is always empty: the reference server signs it with a',
 				'secret/algorithm we don’t have, and our Photon setup accepts an empty token. The',

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

@@ -4456,24 +4456,24 @@ describe('rooms endpoints', () => {
 			const res = await SELF.fetch(`${ORIGIN}/photon_access_token`, {
 				headers: await bearer(String(accountId)),
 			})
-			return ((await res.json()) as { Permissions: Array<{ Role: number; Value: string }> })
-				.Permissions
+			return (
+				(await res.json()) as {
+					Permissions: Array<{ Role: number; Override: boolean; Value: string }>
+				}
+			).Permissions
+		}
+		// A manager gets the WHOLE table — a row per permission for every role tier, grouped
+		// by role — because the permissions screen reads each cell from the row at exactly
+		// that role. Nothing is stored on subroom 2 here, so every row is a default
+		// (`Override: false`): co-owner and creator rows True, the rest False.
+		const roles = [0, 10, 20, 30, 255]
+		for (const table of [await tableFor(1), await tableFor(2)]) {
+			expect(table.length).toBe(30)
+			expect(table.map((p) => p.Role)).toEqual(roles.flatMap((r) => Array<number>(6).fill(r)))
+			for (const p of table) {
+				expect(p).toMatchObject({ Override: false, Value: p.Role >= 30 ? 'True' : 'False' })
+			}
 		}
-		// The creator's values are served at Role 0 and again at Role 255, all True; the
-		// co-owner's at 0 and 30. Both readings of the table land on the same values.
-		const creator = await tableFor(1)
-		expect(creator.length).toBe(12)
-		expect(creator.map((p) => p.Role)).toEqual([
-			...Array<number>(6).fill(0),
-			...Array<number>(6).fill(255),
-		])
-		expect(creator.every((p) => p.Value === 'True')).toBe(true)
-		const coOwner = await tableFor(2)
-		expect(coOwner.map((p) => p.Role)).toEqual([
-			...Array<number>(6).fill(0),
-			...Array<number>(6).fill(30),
-		])
-		expect(coOwner.every((p) => p.Value === 'True')).toBe(true)
 		await env.DB.prepare('DELETE FROM presence WHERE account_id IN (1, 2)').run()
 	})
 
@@ -4795,12 +4795,22 @@ describe('rooms endpoints', () => {
 			return ((await res.json()) as { Permissions: Permission[] }).Permissions
 		}
 
-		/** The value a table serves for a permission (every row of a table agrees). */
-		const valueOf = (list: Permission[], permission: string) => {
-			const values = new Set(list.filter((p) => p.Permission === permission).map((p) => p.Value))
+		/**
+		 * The value a non-manager's table serves for a permission (every row of theirs
+		 * agrees), or the value a manager's table serves at one role (`at`): the cell the
+		 * permissions screen draws for that pair.
+		 */
+		const valueOf = (list: Permission[], permission: string, at?: number) => {
+			const rows = list.filter(
+				(p) => p.Permission === permission && (at === undefined || p.Role === at)
+			)
+			const values = new Set(rows.map((p) => p.Value))
 			expect(values.size).toBeLessThanOrEqual(1)
 			return [...values][0]
 		}
+		/** One cell of a manager's table, as the permissions screen reads it. */
+		const cell = (list: Permission[], permission: string, at: number) =>
+			list.find((p) => p.Permission === permission && p.Role === at)
 		// Room 2's seeded co-owner is account 2 — one of the hardcoded dev accounts, whose
 		// global maker pen no table takes away — so the role-holder cases use 748 (a Host)
 		// and 749 (a co-owner), given their roles here.
@@ -4917,13 +4927,69 @@ describe('rooms endpoints', () => {
 			const after = await permissionsIn(743, 2)
 			expect(after.find((p) => p.Permission === 'CAN_INVITE')).toBeUndefined()
 			expect(after.length).toBe(6)
-			// The co-owner gets it, at Role 0 and at 30, beside their default grants.
+			// The co-owner is a manager and reads the whole table: that entry at its own role
+			// with `Override: true`, inherited (`false`) by the creator above it, and absent
+			// below it where nothing reaches — a pair the defaults don't carry has no cell.
 			await setRoles()
 			const coOwner = await permissionsIn(749, 2)
-			expect(valueOf(coOwner, 'CAN_INVITE')).toBe('True')
+			expect(cell(coOwner, 'CAN_INVITE', 30)).toMatchObject({ Override: true, Value: 'True' })
+			expect(cell(coOwner, 'CAN_INVITE', 255)).toMatchObject({ Override: false, Value: 'True' })
 			expect(coOwner.filter((p) => p.Permission === 'CAN_INVITE').map((p) => p.Role)).toEqual([
-				0, 30,
+				30, 255,
 			])
+			// And the Role 0 grant stored above: its own cell overridden, every tier above
+			// inheriting it — except Role 30, where the envelope test stored its own entry.
+			expect(cell(coOwner, 'CAN_SPAWN_INVENTIONS', 0)).toMatchObject({ Override: true })
+			expect(cell(coOwner, 'CAN_SPAWN_INVENTIONS', 30)).toMatchObject({ Override: true })
+			for (const at of [10, 20, 255]) {
+				expect(cell(coOwner, 'CAN_SPAWN_INVENTIONS', at)).toMatchObject({
+					Override: false,
+					Value: 'True',
+				})
+			}
+		})
+
+		it('serves a manager every role’s row, so a Host grant reads back at Role 10', async () => {
+			// The creator switches a permission on for hosts. Served only at Role 0 and the
+			// creator's own role (as this once was), the Host cell had no row to read and the
+			// switch snapped back to the default every time.
+			await putPermissions(
+				'/rooms/2/subrooms/2/permissions',
+				[{ Permission: 'CAN_SELF_REVIVE', Role: 10, Override: true, Type: 0, Value: 'True' }],
+				'1'
+			)
+			const creator = await permissionsIn(1, 2)
+			expect(cell(creator, 'CAN_SELF_REVIVE', 10)).toEqual({
+				Override: true,
+				Permission: 'CAN_SELF_REVIVE',
+				Role: 10,
+				Type: 0,
+				Value: 'True',
+			})
+			// Nothing reaches "everyone", so there is no Role 0 cell; the tiers above inherit.
+			expect(cell(creator, 'CAN_SELF_REVIVE', 0)).toBeUndefined()
+			expect(cell(creator, 'CAN_SELF_REVIVE', 255)).toMatchObject({
+				Override: false,
+				Value: 'True',
+			})
+			// A default cell is NOT an override — otherwise un-checking it could never stick.
+			expect(cell(creator, 'CAN_USE_DELETE_ALL_BUTTON', 10)).toMatchObject({
+				Override: false,
+				Value: 'False',
+			})
+			// The host themself (not a manager) gets only their own rows, at 0 and 10.
+			const host = await permissionsIn(748, 2)
+			expect(host.filter((p) => p.Permission === 'CAN_SELF_REVIVE').map((p) => p.Role)).toEqual([
+				0, 10,
+			])
+			expect(valueOf(host, 'CAN_SELF_REVIVE')).toBe('True')
+			await putPermissions(
+				'/rooms/2/subrooms/2/permissions',
+				[{ Permission: 'CAN_SELF_REVIVE', Role: 10, Override: false, Type: 0, Value: 'True' }],
+				'1'
+			)
+			expect(cell(await permissionsIn(1, 2), 'CAN_SELF_REVIVE', 10)).toBeUndefined()
+			await env.DB.prepare('DELETE FROM presence WHERE account_id = 1').run()
 		})
 
 		it('a stored entry at the caller’s role beats one below it, and revokes a default', async () => {
@@ -4934,10 +5000,14 @@ describe('rooms endpoints', () => {
 				'1'
 			)
 			const coOwner = await permissionsIn(749, 2)
-			expect(valueOf(coOwner, 'CAN_USE_MAKER_PEN')).toBe('False')
-			expect(valueOf(coOwner, 'CAN_SPAWN_INVENTIONS')).toBe('True')
-			// The creator is above Role 30: that entry doesn't reach them.
-			expect(valueOf(await permissionsIn(1, 2), 'CAN_USE_MAKER_PEN')).toBe('True')
+			expect(cell(coOwner, 'CAN_USE_MAKER_PEN', 30)).toMatchObject({
+				Override: true,
+				Value: 'False',
+			})
+			expect(cell(coOwner, 'CAN_SPAWN_INVENTIONS', 30)).toMatchObject({ Value: 'True' })
+			// The revocation reaches the creator's cell too (the highest lower role with an
+			// entry) — but account 1 is a dev account, whose own-role maker pen nothing takes.
+			expect(valueOf(await permissionsIn(1, 2), 'CAN_USE_MAKER_PEN', 255)).toBe('True')
 
 			// A Host (Role 10) sits between: the Role 0 grant reaches them, the Role 30
 			// revocation does not, and their own default is no maker pen either way.
@@ -4962,7 +5032,8 @@ describe('rooms endpoints', () => {
 			)
 			expect(valueOf(await permissionsIn(748, 2), 'CAN_USE_DELETE_ALL_BUTTON')).toBe('True')
 			// Which reaches a co-owner (above 10) but not a visitor (below it).
-			expect(valueOf(await permissionsIn(749, 2), 'CAN_USE_DELETE_ALL_BUTTON')).toBe('True')
+			expect(valueOf(await permissionsIn(749, 2), 'CAN_USE_DELETE_ALL_BUTTON', 30)).toBe('True')
+			expect(valueOf(await permissionsIn(749, 2), 'CAN_USE_DELETE_ALL_BUTTON', 0)).toBe('False')
 			expect(valueOf(await permissionsIn(743, 2), 'CAN_USE_DELETE_ALL_BUTTON')).toBe('False')
 			await env.DB.prepare('DELETE FROM presence WHERE account_id = 1').run()
 		})
@@ -4997,9 +5068,13 @@ describe('rooms endpoints', () => {
 				).status
 			).toBe(200)
 
-			// The row is gone, and the co-owner has their default maker pen again.
+			// The row is gone, and the co-owner has their default maker pen again — a default
+			// cell, no longer an override.
 			expect(await stored()).toBe(0)
-			expect(valueOf(await permissionsIn(749, 2), 'CAN_USE_MAKER_PEN')).toBe('True')
+			expect(cell(await permissionsIn(749, 2), 'CAN_USE_MAKER_PEN', 30)).toMatchObject({
+				Override: false,
+				Value: 'True',
+			})
 
 			// Clearing a pair that was never overridden is a no-op, not an insert.
 			await putPermissions(