fix(emotes): keep emote bar reads distinct, lenient and atomic on save

src/modules/emote-bar/application/GetMyEmoteBar.ts

@@ -3,9 +3,9 @@ import { EMOTE_PACK_MANIFEST } from "../../catalog/domain/EmotePack";
 import { Cosmetic } from "../../catalog/domain/Cosmetic";
 import { CosmeticRepository } from "../../catalog/domain/CosmeticRepository";
 import { EntitlementsGatekeeper } from "../../entitlements/application/EntitlementsGatekeeper";
-import { EmoteBar } from "../domain/EmoteBar";
+import { EMOTE_BAR_SLOTS } from "../domain/EmoteBar";
 import { EmoteBarRepository } from "../domain/EmoteBarRepository";
-import { DEFAULT_EMOTE_IDS } from "../domain/EmoteRef";
+import { DEFAULT_EMOTE_IDS, EmoteRef } from "../domain/EmoteRef";
 import { activeEmotePacksBySlug, emoteFileName, packHasEmote } from "./activeEmotePacks";
 
 export interface EmoteBarSlot {
@@ -23,19 +23,30 @@ export interface EmoteBarView {
 	slots: EmoteBarSlot[];
 }
 
-function defaultSlot(index: number): EmoteBarSlot {
-	return {
-		ref: DEFAULT_EMOTE_IDS[index],
-		packId: null,
-		assetUrl: null,
-		packUrl: null,
-		assetsExpiresAt: null,
-	};
+function defaultSlot(ref: string): EmoteBarSlot {
+	return { ref, packId: null, assetUrl: null, packUrl: null, assetsExpiresAt: null };
+}
+
+function parseRef(raw: string): EmoteRef | null {
+	try {
+		return EmoteRef.parse(raw);
+	} catch {
+		return null;
+	}
+}
+
+/** Each stale slot takes the first default emote that no other slot of the bar shows. */
+function fillStaleSlots(resolved: readonly (EmoteBarSlot | null)[]): EmoteBarSlot[] {
+	const used = new Set(resolved.flatMap((slot) => (slot ? [slot.ref] : [])));
+	const spare = DEFAULT_EMOTE_IDS.filter((id) => !used.has(id));
+	return resolved.map((slot) => slot ?? defaultSlot(spare.shift() as string));
 }
 
 /**
- * Resolves any user's bar against the catalog and their current entitlements. A saved slot
- * that is no longer usable reads as the default emote of that slot; storage is not rewritten.
+ * Resolves any user's bar against the catalog and their current entitlements. Stored refs are
+ * read leniently: a slot that is malformed, repeated, or no longer usable reads as the first
+ * default emote the rest of the bar does not show, so the result always has distinct emotes;
+ * storage is not rewritten. A stored bar of the wrong length reads as the default bar.
  */
 export class GetMyEmoteBar {
 	constructor(
@@ -46,14 +57,22 @@ export class GetMyEmoteBar {
 	) {}
 
 	async run(userId: string): Promise<EmoteBarView> {
-		const bar = (await this.bars.findByUserId(userId)) ?? EmoteBar.defaults();
-		const refs = bar.refs();
-		if (refs.every((ref) => ref.isDefault)) {
-			return { slots: refs.map((ref, index) => ({ ...defaultSlot(index), ref: ref.toString() })) };
-		}
+		const stored = await this.bars.findByUserId(userId);
+		const raws = stored?.length === EMOTE_BAR_SLOTS ? stored : DEFAULT_EMOTE_IDS;
+
+		const seen = new Set<string>();
+		const refs = raws.map((raw) => {
+			const ref = parseRef(raw);
+			if (!ref || seen.has(raw)) return null;
+			seen.add(raw);
+			return ref;
+		});
 
-		const packs = activeEmotePacksBySlug(await this.cosmetics.findAll());
-		const access = await this.gatekeeper.accessFor(userId);
+		const needsPacks = refs.some((ref) => ref && !ref.isDefault);
+		const packs = needsPacks
+			? activeEmotePacksBySlug(await this.cosmetics.findAll())
+			: new Map<string, Cosmetic>();
+		const access = needsPacks ? await this.gatekeeper.accessFor(userId) : null;
 		const manifests = new Map<string, Promise<SignedAssetManifest>>();
 		const manifestOf = (pack: Cosmetic) => {
 			let manifest = manifests.get(pack.id);
@@ -64,17 +83,18 @@ export class GetMyEmoteBar {
 			return manifest;
 		};
 
-		const slots = await Promise.all(
-			refs.map(async (ref, index): Promise<EmoteBarSlot> => {
-				if (ref.isDefault) return { ...defaultSlot(index), ref: ref.toString() };
+		const resolved = await Promise.all(
+			refs.map(async (ref): Promise<EmoteBarSlot | null> => {
+				if (!ref) return null;
+				if (ref.isDefault) return defaultSlot(ref.toString());
 
 				const pack = packs.get(ref.packSlug as string);
 				const key = ref.emoteKey as string;
-				if (!pack || !access.canUse(pack) || !packHasEmote(pack, key)) return defaultSlot(index);
+				if (!pack || !access?.canUse(pack) || !packHasEmote(pack, key)) return null;
 
 				const manifest = await manifestOf(pack);
 				const assetUrl = manifest.assets[emoteFileName(key)];
-				if (!assetUrl) return defaultSlot(index);
+				if (!assetUrl) return null;
 				return {
 					ref: ref.toString(),
 					packId: pack.id,
@@ -84,6 +104,6 @@ export class GetMyEmoteBar {
 				};
 			}),
 		);
-		return { slots };
+		return { slots: fillStaleSlots(resolved) };
 	}
 }

src/modules/emote-bar/application/activeEmotePacks.ts

@@ -4,13 +4,20 @@ import { EMOTE_PACK_MANIFEST } from "../../catalog/domain/EmotePack";
 
 const PACK_PREFIX = "emote-packs/";
 
+function slugOf(assetRef: string): string | null {
+	const rest = assetRef.slice(PACK_PREFIX.length);
+	const slug = rest.endsWith("/") ? rest.slice(0, -1) : rest;
+	return slug === "" || slug.includes("/") ? null : slug;
+}
+
 /** Active EMOTE_PACK cosmetics keyed by the slug in their `emote-packs/<slug>/` folder. */
 export function activeEmotePacksBySlug(cosmetics: readonly Cosmetic[]): Map<string, Cosmetic> {
 	const bySlug = new Map<string, Cosmetic>();
 	for (const cosmetic of cosmetics) {
 		if (cosmetic.type !== CosmeticType.EMOTE_PACK || !cosmetic.active) continue;
 		if (!cosmetic.assetRef.startsWith(PACK_PREFIX)) continue;
-		bySlug.set(cosmetic.assetRef.slice(PACK_PREFIX.length, -1), cosmetic);
+		const slug = slugOf(cosmetic.assetRef);
+		if (slug) bySlug.set(slug, cosmetic);
 	}
 	return bySlug;
 }

src/modules/emote-bar/domain/EmoteBarRepository.ts

@@ -1,8 +1,12 @@
 import { EmoteBar } from "./EmoteBar";
 
 export interface EmoteBarRepository {
-	/** The saved bar, or null when the user never saved one. */
-	findByUserId(userId: string): Promise<EmoteBar | null>;
+	/**
+	 * The stored refs exactly as saved, or null when the user never saved a bar or the stored
+	 * array does not hold one ref per slot. Stored refs are not validated: they may predate
+	 * the current rules, so the reader decides what each one means.
+	 */
+	findByUserId(userId: string): Promise<readonly string[] | null>;
 
 	/** Replaces the whole bar atomically. */
 	save(userId: string, bar: EmoteBar): Promise<void>;

src/modules/emote-bar/infrastructure/EmoteBarPostgresRepository.ts

@@ -1,18 +1,23 @@
 import { cosmeticsDataSource } from "../../../cosmetics-data-source";
-import { EmoteBar } from "../domain/EmoteBar";
+import { EMOTE_BAR_SLOTS, EmoteBar } from "../domain/EmoteBar";
 import { EmoteBarRepository } from "../domain/EmoteBarRepository";
 import { UserEmoteBarEntity } from "./UserEmoteBarEntity";
 
 export class EmoteBarPostgresRepository implements EmoteBarRepository {
-	async findByUserId(userId: string): Promise<EmoteBar | null> {
+	async findByUserId(userId: string): Promise<readonly string[] | null> {
 		const row = await cosmeticsDataSource.getRepository(UserEmoteBarEntity).findOneBy({ userId });
-		return row ? EmoteBar.create(row.emoteRefs) : null;
+		return row?.emoteRefs?.length === EMOTE_BAR_SLOTS ? row.emoteRefs : null;
 	}
 
 	async save(userId: string, bar: EmoteBar): Promise<void> {
-		// Upsert by primary key: the whole bar is replaced in a single statement.
+		// A single INSERT ... ON CONFLICT (user_id) DO UPDATE replaces the whole bar, so
+		// concurrent first saves for one user cannot collide on the primary key.
 		await cosmeticsDataSource
-			.getRepository(UserEmoteBarEntity)
-			.save({ userId, emoteRefs: bar.refs().map(String) });
+			.createQueryBuilder()
+			.insert()
+			.into(UserEmoteBarEntity)
+			.values({ userId, emoteRefs: bar.refs().map(String) })
+			.orUpdate(["emote_refs", "updated_at"], ["user_id"])
+			.execute();
 	}
 }

tests/unit/modules/emote-bar/application/GetMyEmoteBar.test.ts

@@ -75,7 +75,109 @@ describe("GetMyEmoteBar", () => {
 		const result = await build({ bar }).run("user-1");
 
 		expect(result.slots[0].ref).toBe("halloween/ghost");
-		expect(result.slots[1]).toEqual({ ref: "thumbsup", ...DEFAULT_SLOT });
+		expect(result.slots[1]).toEqual({ ref: "wave", ...DEFAULT_SLOT });
+	});
+
+	it("never repeats a default when a stale slot falls back", async () => {
+		const bar = barWith([0, "fire"], [2, "halloween/ghost"]);
+
+		const result = await build({ bar, cosmetics: [] }).run("user-1");
+
+		const refs = result.slots.map((slot) => slot.ref);
+		expect(refs).toEqual([
+			"fire",
+			"thumbsup",
+			"wave",
+			"thinking",
+			"cool",
+			"mindblown",
+			"sweat",
+			"cry",
+		]);
+		expect(new Set(refs).size).toBe(8);
+	});
+
+	it("gives each of several stale slots a different unused default", async () => {
+		const bar = barWith(
+			[0, "halloween/ghost"],
+			[1, "halloween/bat"],
+			[4, "gone/emote"],
+			[7, "gone/other"],
+		);
+
+		const result = await build({ bar, cosmetics: [] }).run("user-1");
+
+		const refs = result.slots.map((slot) => slot.ref);
+		expect(refs).toEqual([
+			"wave",
+			"thumbsup",
+			"fire",
+			"thinking",
+			"cool",
+			"mindblown",
+			"sweat",
+			"cry",
+		]);
+		expect(new Set(refs).size).toBe(8);
+	});
+
+	it("reads stored refs that are no longer valid as stale slots", async () => {
+		const stored = [
+			"removed-default",
+			"Not/A/Ref",
+			"fire",
+			"halloween/ghost",
+			"cool",
+			"mindblown",
+			"sweat",
+			"cry",
+		];
+		const useCase = new GetMyEmoteBar(
+			cosmeticsOf(pack()),
+			barsOf(stored).repository,
+			gatekeeperOf(grant(PACK_ID)),
+			signer,
+		);
+
+		const result = await useCase.run("user-1");
+
+		expect(result.slots.map((slot) => slot.ref)).toEqual([
+			"wave",
+			"thumbsup",
+			"fire",
+			"halloween/ghost",
+			"cool",
+			"mindblown",
+			"sweat",
+			"cry",
+		]);
+	});
+
+	it("reads a stored array of the wrong length as the default bar", async () => {
+		const useCase = new GetMyEmoteBar(
+			cosmeticsOf(pack()),
+			barsOf(["wave", "fire"]).repository,
+			gatekeeperOf(),
+			signer,
+		);
+
+		const result = await useCase.run("user-1");
+
+		expect(result.slots.map((slot) => slot.ref)).toEqual([...DEFAULT_EMOTE_IDS]);
+	});
+
+	it("reads repeated stored refs as one slot plus fallbacks", async () => {
+		const stored = ["wave", "wave", ...DEFAULT_EMOTE_IDS.slice(2)];
+		const useCase = new GetMyEmoteBar(
+			cosmeticsOf(pack()),
+			barsOf(stored).repository,
+			gatekeeperOf(),
+			signer,
+		);
+
+		const result = await useCase.run("user-1");
+
+		expect(result.slots.map((slot) => slot.ref)).toEqual([...DEFAULT_EMOTE_IDS]);
 	});
 
 	it("resolves the bar for whichever user id it is asked about", async () => {

tests/unit/modules/emote-bar/application/activeEmotePacks.test.ts

@@ -0,0 +1,32 @@
+import { describe, expect, it } from "bun:test";
+
+import { activeEmotePacksBySlug } from "../../../../../src/modules/emote-bar/application/activeEmotePacks";
+import { pack } from "./fixtures";
+
+describe("activeEmotePacksBySlug", () => {
+	it("keys a pack by the slug in its folder, with or without a trailing slash", () => {
+		const withSlash = pack({ id: "a", assetRef: "emote-packs/halloween/" });
+		const withoutSlash = pack({ id: "b", assetRef: "emote-packs/winter" });
+
+		const bySlug = activeEmotePacksBySlug([withSlash, withoutSlash]);
+
+		expect(bySlug.get("halloween")).toBe(withSlash);
+		expect(bySlug.get("winter")).toBe(withoutSlash);
+	});
+
+	it.each([
+		"emote-packs/",
+		"emote-packs//",
+		"emote-packs/a/b/",
+		"emote-packs/a/b",
+		"playmats/x/",
+	])("ignores the asset ref %s", (assetRef) => {
+		const bySlug = activeEmotePacksBySlug([pack({ assetRef })]);
+
+		expect(bySlug.size).toBe(0);
+	});
+
+	it("ignores inactive packs", () => {
+		expect(activeEmotePacksBySlug([pack({ active: false })]).size).toBe(0);
+	});
+});

tests/unit/modules/emote-bar/application/fixtures.ts

@@ -51,10 +51,11 @@ export function gatekeeperOf(...entitlements: Entitlement[]): EntitlementsGateke
 	});
 }
 
-export function barsOf(initial: EmoteBar | null) {
+export function barsOf(initial: EmoteBar | readonly string[] | null) {
+	const stored = initial instanceof EmoteBar ? initial.refs().map(String) : initial;
 	const saved: Array<{ userId: string; bar: EmoteBar }> = [];
 	const repository: EmoteBarRepository = {
-		findByUserId: async () => initial,
+		findByUserId: async () => stored,
 		save: async (userId, bar) => {
 			saved.push({ userId, bar });
 		},