diff --git a/agents-docs/LESSONS.md b/agents-docs/LESSONS.md index 0dc2f3b..940b36e 100644 --- a/agents-docs/LESSONS.md +++ b/agents-docs/LESSONS.md @@ -25,6 +25,13 @@ Durable rules for AI agents working on this project. Read this file at session s ## Lessons +### Scope per-user UI state by user id, not by the client database [persistence] [multi-user] [custom-emoji] + +- **Trigger:** custom emoji "saved library" membership was a single `savedByUser` flag on the shared emoji row plus a long-lived singleton (`CustomEmojiService`) that merged state across logins — so a second account on the same client (and the Electron shared SQLite DB) inherited the first user's picker. +- **Rule:** when state is "per signed-in user" but the asset/row store is shared (Electron `custom_emojis`, or a renderer singleton that survives logout), key the membership by user id in its own store (`localStorage` `metoyou_custom_emoji_saved:`, mirroring the existing per-user usage ranking) and rebuild it in `loadForUser`; never rely on a global row flag or assume the singleton was reset on logout. +- **Why:** the browser already isolates rows per-user database, so the leak only reproduces in-session (no reload) and on Electron's shared DB — both invisible if you only test reloads; a row-level flag also can't represent two local users saving the same asset. +- **Example:** `CustomEmojiService.resolveSavedIds(userId, emojis)` reads/seeds a per-user id set; e2e `e2e/tests/chat/custom-emoji-user-binding.spec.ts` runs the whole user switch in ONE page load (client-side router nav only) so the singleton-retention leak is actually exercised, and the second user *joins* the first user's server instead of creating one (in-session "create a second server" leaves `sourceId` empty and the submit disabled). + ### Don't strand signed-out mobile users on a logged-out dashboard [auth] [mobile] [routing] - **Trigger:** `App.ngOnInit` special-cased mobile — signed-out visitors landing on `/` or `/dashboard` were kept on `/dashboard` (the "login form has no mobile chrome" rationale), so mobile users got a logged-out dashboard and never saw a login screen on startup. diff --git a/agents-docs/features/custom-emoji.md b/agents-docs/features/custom-emoji.md index 135f012..f22638b 100644 --- a/agents-docs/features/custom-emoji.md +++ b/agents-docs/features/custom-emoji.md @@ -19,7 +19,7 @@ Custom emoji lets users upload small image emoji, use them in chat messages and - **Custom emoji asset**: A user-created image stored as a data URL with id, name, mime, size, hash, creator, timestamps, and optional saved-library membership. - **Known custom emoji**: A synced asset available for message rendering and forwarding, but not shown in the current user's picker unless saved. -- **Saved custom emoji**: A known asset with `savedByUser` enabled; saved emoji appear in the picker and shortcut ranking. +- **Saved custom emoji**: A known asset the current user added to their library; saved emoji appear in the picker and shortcut ranking. Library membership is **user-bound, not client-bound** — it is tracked per signed-in user (keyed by user id), so a second account on the same device never inherits the first account's library. - **Emoji shortcut row**: The seven most-used emoji entries for the current user plus an eighth control that opens the full selector. - **Custom emoji token**: The stable message/reaction representation `:emoji[id](name)`, resolved locally to the synced image asset when rendering. - **Composer emoji alias**: The readable inline draft representation `:name:`. The composer rewrites known aliases to stable custom emoji tokens only when sending. @@ -40,6 +40,7 @@ When a peer connects, each side sends a summary of known assets. The receiver re - Uploads are capped at 1 MB. - Accepted image types match profile avatars: WebP, GIF, JPG, and JPEG. - Local shortcut ranking is keyed by the active user and includes Unicode emoji plus saved custom emoji only. +- Saved-library membership is bound to the user, not the client: `CustomEmojiService` tracks the set of saved emoji ids per user id in `localStorage` (`metoyou_custom_emoji_saved:`, mirroring the per-user usage ranking). The picker shows only emoji in the active user's saved set, so signing in as a different account on the same client never exposes the previous account's library. On first load after this change the set is seeded from legacy `savedByUser` rows the user actually created (`creatorUserId === userId`), so creators keep their library while other local accounts stay empty. - Message rendering reserves inline emoji space with a transparent placeholder image while a referenced custom emoji asset is not yet available; deferred markdown placeholders rewrite tokens to readable `:name:` aliases so raw `:emoji[id](name)` text never flashes in chat. - Seen custom emoji are not added to the picker automatically; right-click a rendered custom emoji in chat or on a custom emoji reaction and choose **Add to emoji library** from the app context menu (`NativeContextMenuComponent`). - Saved custom emoji can be removed from the picker library by right-clicking them inside the emoji picker and choosing **Remove from emoji library**; the asset stays available for rendering messages that already reference it. @@ -50,9 +51,9 @@ When a peer connects, each side sends a summary of known assets. The receiver re ## Data Access -- Browser runtime stores custom emoji in IndexedDB store `customEmojis`. -- Electron runtime stores custom emoji in SQLite table `custom_emojis`, created by migration `1000000000011-AddCustomEmojis`. -- Renderer access goes through `DatabaseService` methods `saveCustomEmoji`, `getCustomEmojis`, and `deleteCustomEmoji`. +- Browser runtime stores custom emoji image assets in IndexedDB store `customEmojis` (per-user database scope). +- Electron runtime stores custom emoji image assets in SQLite table `custom_emojis`, created by migration `1000000000011-AddCustomEmojis` (a single shared desktop database). +- Renderer access goes through `DatabaseService` methods `saveCustomEmoji`, `getCustomEmojis`, and `deleteCustomEmoji`. These persist the image **assets** only; they are not scoped per user (the Electron table is shared across local accounts). Per-user **library membership** lives separately in `localStorage` (`metoyou_custom_emoji_saved:`), which is what keeps the picker user-bound even on a shared client database. ## Testing diff --git a/e2e/tests/chat/custom-emoji-user-binding.spec.ts b/e2e/tests/chat/custom-emoji-user-binding.spec.ts new file mode 100644 index 0000000..3c84a0e --- /dev/null +++ b/e2e/tests/chat/custom-emoji-user-binding.spec.ts @@ -0,0 +1,204 @@ +import { + test, + expect, + type Page +} from '@playwright/test'; +import { test as multiClientTest } from '../../fixtures/multi-client'; +import { LoginPage } from '../../pages/login.page'; +import { RegisterPage } from '../../pages/register.page'; +import { ServerSearchPage } from '../../pages/server-search.page'; +import { ChatMessagesPage } from '../../pages/chat-messages.page'; + +interface TestUser { + username: string; + displayName: string; + password: string; +} + +/** + * Regression coverage for: "Emojis should be user bound not client bound". + * + * A custom emoji belongs to the user who saved it, not to the client. A second + * account signing in on the same client must NOT inherit the first user's emoji + * library/picker. + * + * The whole scenario runs in a SINGLE page load (only the very first navigation + * reloads). All user switching is client-side via the router, because the leak + * lived in the long-lived singleton CustomEmojiService that used to keep the + * previous user's library after a logout + login without a reload. To avoid the + * (separate) in-session "create a second server" limitation, the second user + * joins the first user's server rather than creating their own. + */ + +// Minimal valid 1x1 transparent GIF; the emoji pipeline validates mime + size only. +const TINY_GIF = Buffer.from( + '47494638396101000100800000000000ffffff21f90401000000002c00000000010001000002024401003b', + 'hex' +); + +multiClientTest.describe('Custom emoji are user bound, not client bound', () => { + multiClientTest.describe.configure({ timeout: 180_000 }); + + multiClientTest('a second user on the same client does not inherit the first user library', async ({ createClient }) => { + const { page } = await createClient(); + const suffix = uniqueName('emoji-bound'); + const alice: TestUser = { username: `alice_${suffix}`, displayName: 'Alice', password: 'TestPass123!' }; + const bob: TestUser = { username: `bob_${suffix}`, displayName: 'Bob', password: 'TestPass123!' }; + const serverName = `Shared Emoji Server ${suffix}`; + const libraryEmoji = page.locator('app-custom-emoji-picker [data-custom-emoji-library]'); + + await test.step('Alice registers, creates a server and uploads a custom emoji', async () => { + await new RegisterPage(page).goto(); + await submitRegistration(page, alice); + await expect(page).toHaveURL(/\/dashboard/, { timeout: 15_000 }); + + await createServer(page, serverName); + await openComposerEmojiModal(page); + await page.locator('app-custom-emoji-picker input[type="file"]').setInputFiles({ + name: `partyblob_${suffix}.gif`, + mimeType: 'image/gif', + buffer: TINY_GIF + }); + }); + + await test.step('Alice sees her own uploaded emoji in her library', async () => { + await openComposerEmojiModal(page); + await expect(libraryEmoji).toHaveCount(1, { timeout: 15_000 }); + await page.keyboard.press('Escape'); + }); + + await test.step('Bob signs in on the same client (no reload) and joins the same server', async () => { + await logoutClientSide(page); + await registerClientSide(page, bob); + await joinServerClientSide(page, serverName); + }); + + await test.step('Bob does not inherit Alice custom emoji library', async () => { + await openComposerEmojiModal(page); + // The modal is open (the file input is asserted inside the helper), so an + // empty grid is a genuine assertion rather than a timing artifact. + await expect(libraryEmoji).toHaveCount(0); + }); + }); +}); + +async function createServer(page: Page, serverName: string): Promise { + const searchPage = new ServerSearchPage(page); + + await expect(searchPage.createServerButton).toBeVisible({ timeout: 15_000 }); + await searchPage.createServerButton.click(); + + await expect(searchPage.serverNameInput).toBeVisible({ timeout: 15_000 }); + + // Client-side nav can render the form before its `(ngModelChange)` handler is + // wired, so an early fill never reaches the backing signal. Clear + refill + // until the submit button actually enables. + await expect.poll(async () => { + await searchPage.serverNameInput.fill(''); + await searchPage.serverNameInput.fill(serverName); + + return searchPage.createSubmitButton.isEnabled(); + }, { timeout: 15_000 }).toBe(true); + + await searchPage.createSubmitButton.click(); + + await expect(page).toHaveURL(/\/room\//, { timeout: 20_000 }); + await new ChatMessagesPage(page).waitForReady(); +} + +async function joinServerClientSide(page: Page, serverName: string): Promise { + const searchPage = new ServerSearchPage(page); + + await page.locator('a[href="/servers"]').first() + .click(); + + await expect(searchPage.searchInput).toBeVisible({ timeout: 15_000 }); + await searchPage.searchInput.fill(serverName); + + const serverCard = page.locator('div[title]', { hasText: serverName }).first(); + + await expect(serverCard).toBeVisible({ timeout: 20_000 }); + await serverCard.dblclick(); + + await expect(page).toHaveURL(/\/room\//, { timeout: 20_000 }); + await new ChatMessagesPage(page).waitForReady(); +} + +async function openComposerEmojiModal(page: Page): Promise { + const picker = page.locator('app-custom-emoji-picker'); + const fileInput = picker.locator('input[type="file"]'); + + // Reset to a known state: dismiss any open picker, then open it fresh. + await page.keyboard.press('Escape').catch(() => {}); + await expect(picker).toHaveCount(0, { timeout: 5_000 }) + .catch(() => {}); + + await page.locator('app-chat-message-composer') + .getByRole('button', { name: 'Open emoji selector' }) + .first() + .click(); + + await expect(picker).toBeVisible({ timeout: 10_000 }); + + // The compact picker exposes a button that opens the full panel (with the + // upload field and the custom-emoji grid). + await picker.getByRole('button', { name: 'Open emoji selector' }).click(); + await expect(fileInput).toBeAttached({ timeout: 10_000 }); +} + +async function registerClientSide(page: Page, user: TestUser): Promise { + const loginPage = new LoginPage(page); + const registerPage = new RegisterPage(page); + + await expect(loginPage.registerLink).toBeVisible({ timeout: 15_000 }); + await loginPage.registerLink.click(); + await expect(registerPage.usernameInput).toBeVisible({ timeout: 15_000 }); + await submitRegistration(page, user); + await expect(page).toHaveURL(/\/dashboard/, { timeout: 15_000 }); +} + +/** + * Fills the registration form resiliently. On client-side navigation the + * template-driven `ngModel` can attach a tick after the input is visible, so an + * early `fill` is overwritten back to empty. Re-fill until every value sticks. + */ +async function submitRegistration(page: Page, user: TestUser): Promise { + const username = page.locator('#register-username'); + const displayName = page.locator('#register-display-name'); + const password = page.locator('#register-password'); + + await expect.poll(async () => { + await username.fill(user.username); + await displayName.fill(user.displayName); + await password.fill(user.password); + + return [ + await username.inputValue(), + await displayName.inputValue(), + await password.inputValue() + ].join('|'); + }, { timeout: 15_000 }).toBe([ + user.username, + user.displayName, + user.password + ].join('|')); + + await page.getByRole('button', { name: 'Create Account' }).click(); +} + +async function logoutClientSide(page: Page): Promise { + const menuButton = page.getByRole('button', { name: 'Menu' }); + const logoutButton = page.getByRole('button', { name: 'Logout' }); + + await expect(menuButton).toBeVisible({ timeout: 10_000 }); + await menuButton.click(); + await expect(logoutButton).toBeVisible({ timeout: 10_000 }); + await logoutButton.click(); + await expect(page).toHaveURL(/\/login/, { timeout: 15_000 }); + await expect(new LoginPage(page).usernameInput).toBeVisible({ timeout: 10_000 }); +} + +function uniqueName(prefix: string): string { + return `${prefix}-${Date.now().toString(36)}-${Math.random().toString(36) + .slice(2, 8)}`; +} diff --git a/toju-app/src/app/domains/custom-emoji/application/custom-emoji.service.spec.ts b/toju-app/src/app/domains/custom-emoji/application/custom-emoji.service.spec.ts index 30d0b1f..d8281f4 100644 --- a/toju-app/src/app/domains/custom-emoji/application/custom-emoji.service.spec.ts +++ b/toju-app/src/app/domains/custom-emoji/application/custom-emoji.service.spec.ts @@ -163,6 +163,62 @@ describe('CustomEmojiService', () => { ]); }); + it('binds the saved library to the active user so switching users on one client does not leak emoji', async () => { + const dataUrl = 'data:image/webp;base64,QUJDRA=='; + const hash = await hashText(dataUrl); + const ownerEmoji = customEmoji({ + id: 'owner-emoji', + creatorUserId: 'user-1', + dataUrl, + hash, + size: 4, + savedByUser: true + }); + + // A shared client database (Electron-style) returns user-1's row for everyone. + vi.mocked(db.getCustomEmojis).mockResolvedValue([ownerEmoji]); + const service = createService(); + + await service.loadForUser('user-1'); + + expect(service.emojis().map((emoji) => emoji.id)).toEqual(['owner-emoji']); + expect(service.isEmojiInLibrary('owner-emoji')).toBe(true); + + await service.loadForUser('user-2'); + + expect(service.emojis()).toEqual([]); + expect(service.isEmojiInLibrary('owner-emoji')).toBe(false); + + await service.loadForUser('user-1'); + + expect(service.emojis().map((emoji) => emoji.id)).toEqual(['owner-emoji']); + }); + + it('keeps a peer emoji the active user explicitly saved scoped to that user', async () => { + const dataUrl = 'data:image/webp;base64,QUJDRA=='; + const peerEmoji = customEmoji({ + id: 'peer-emoji', + creatorUserId: 'peer-9', + dataUrl, + hash: await hashText(dataUrl), + size: 4 + }); + + vi.mocked(db.getCustomEmojis).mockResolvedValue([peerEmoji]); + const service = createService(); + + await service.loadForUser('user-1'); + expect(service.emojis()).toEqual([]); + + await service.saveEmojiToLibrary('peer-emoji'); + expect(service.isEmojiInLibrary('peer-emoji')).toBe(true); + expect(service.emojis().map((emoji) => emoji.id)).toEqual(['peer-emoji']); + + await service.loadForUser('user-2'); + expect(service.isEmojiInLibrary('peer-emoji')).toBe(false); + expect(service.emojis()).toEqual([]); + }); + it('pushes referenced custom emoji assets to every connected peer without waiting for a request', async () => { const service = createService(); const dataUrl = 'data:image/webp;base64,QUJDRA=='; diff --git a/toju-app/src/app/domains/custom-emoji/application/custom-emoji.service.ts b/toju-app/src/app/domains/custom-emoji/application/custom-emoji.service.ts index cb26a1e..4cdebde 100644 --- a/toju-app/src/app/domains/custom-emoji/application/custom-emoji.service.ts +++ b/toju-app/src/app/domains/custom-emoji/application/custom-emoji.service.ts @@ -33,6 +33,7 @@ import { } from '../domain/custom-emoji.rules'; const USAGE_STORAGE_PREFIX = 'metoyou_custom_emoji_usage:'; +const SAVED_STORAGE_PREFIX = 'metoyou_custom_emoji_saved:'; interface PendingCustomEmojiTransfer { chunks: (string | undefined)[]; @@ -46,21 +47,32 @@ export class CustomEmojiService { private readonly webrtc = inject(RealtimeSessionFacade); private readonly emojisState = signal([]); private readonly usageState = signal>(new Map()); + private readonly savedIdsState = signal>(new Set()); private readonly pendingTransfers = new Map(); + private activeUserId: string | null = null; private loaded = false; - readonly emojis = computed(() => this.emojisState().filter((emoji) => this.isSavedEmoji(emoji))); + readonly emojis = computed(() => { + const savedIds = this.savedIdsState(); + + return this.emojisState().filter((emoji) => savedIds.has(emoji.id)); + }); readonly shortcutEntries = computed(() => selectEmojiShortcutEntries({ customEmojis: this.emojis(), usage: this.usageState() })); async loadForUser(userId: string | null | undefined): Promise { + this.activeUserId = userId ?? null; + const emojis = await this.db.getCustomEmojis(); const merged = new Map(this.emojisState().map((emoji) => [emoji.id, emoji])); + const validEmojis: CustomEmoji[] = []; for (const emoji of emojis) { if (await this.isValidRemoteEmoji(emoji)) { + validEmojis.push(emoji); + const existing = merged.get(emoji.id); if (!existing || existing.updatedAt < emoji.updatedAt) { @@ -73,6 +85,7 @@ export class CustomEmojiService { } this.emojisState.set([...merged.values()].sort((first, second) => second.updatedAt - first.updatedAt)); + this.savedIdsState.set(this.resolveSavedIds(this.activeUserId, validEmojis)); this.usageState.set(this.readUsage(userId)); this.loaded = true; } @@ -92,6 +105,8 @@ export class CustomEmojiService { throw new Error(validation.reason ?? 'Invalid emoji image.'); } + this.activeUserId = userId; + const dataUrl = await this.readFileAsDataUrl(file); const now = Date.now(); const emoji: CustomEmoji = { @@ -134,6 +149,14 @@ export class CustomEmojiService { const nextEmojis = [nextEmoji, ...this.emojisState().filter((entry) => entry.id !== nextEmoji.id)]; this.emojisState.set(nextEmojis.sort((first, second) => second.updatedAt - first.updatedAt)); + + // Library membership is bound to the active user. The asset is now known to + // everyone on this client, but it only enters *this* user's picker when the + // payload says it is saved (own creation / own broadcast), never when another + // local account synced or received it. + if (emoji.savedByUser === true || existing?.savedByUser) { + this.markEmojiSaved(nextEmoji.id); + } } findEmoji(id: string): CustomEmoji | undefined { @@ -147,15 +170,13 @@ export class CustomEmojiService { } isEmojiInLibrary(id: string): boolean { - const emoji = this.findEmoji(id); - - return !!emoji && this.isSavedEmoji(emoji); + return this.savedIdsState().has(id); } async saveEmojiToLibrary(id: string): Promise { const emoji = this.findEmoji(id); - if (!emoji || this.isSavedEmoji(emoji)) { + if (!emoji || this.isEmojiInLibrary(id)) { return; } @@ -166,12 +187,13 @@ export class CustomEmojiService { await this.db.saveCustomEmoji(savedEmoji); this.emojisState.set(this.emojisState().map((entry) => entry.id === id ? savedEmoji : entry)); + this.markEmojiSaved(id); } async removeEmojiFromLibrary(id: string): Promise { const emoji = this.findEmoji(id); - if (!emoji || !this.isSavedEmoji(emoji)) { + if (!emoji || !this.isEmojiInLibrary(id)) { return; } @@ -182,6 +204,7 @@ export class CustomEmojiService { await this.db.saveCustomEmoji(unsavedEmoji); this.emojisState.set(this.emojisState().map((entry) => entry.id === id ? unsavedEmoji : entry)); + this.unmarkEmojiSaved(id); } recordUsage(entry: EmojiShortcutEntry, userId: string | null | undefined): void { @@ -414,8 +437,95 @@ export class CustomEmojiService { return CUSTOM_EMOJI_ALLOWED_MIME_TYPES.includes(mime.toLowerCase() as typeof CUSTOM_EMOJI_ALLOWED_MIME_TYPES[number]); } - private isSavedEmoji(emoji: CustomEmoji): boolean { - return emoji.savedByUser !== false; + /** + * Resolve the active user's saved-library membership. Membership is bound to + * the user (not the client) so a second account on the same device never + * inherits another user's picker. A persisted per-user set wins; on first run + * we seed it from legacy `savedByUser` rows the user actually created, so the + * creator keeps their library after the upgrade while other local accounts + * stay empty. + */ + private resolveSavedIds(userId: string | null, emojis: readonly CustomEmoji[]): ReadonlySet { + const stored = this.readSavedIds(userId); + + if (stored) { + return stored; + } + + if (!userId) { + return new Set(); + } + + const seeded = new Set( + emojis + .filter((emoji) => emoji.savedByUser !== false && emoji.creatorUserId === userId) + .map((emoji) => emoji.id) + ); + + this.writeSavedIds(userId, seeded); + + return seeded; + } + + private markEmojiSaved(id: string): void { + if (this.savedIdsState().has(id)) { + return; + } + + const next = new Set(this.savedIdsState()); + + next.add(id); + this.savedIdsState.set(next); + this.writeSavedIds(this.activeUserId, next); + } + + private unmarkEmojiSaved(id: string): void { + if (!this.savedIdsState().has(id)) { + return; + } + + const next = new Set(this.savedIdsState()); + + next.delete(id); + this.savedIdsState.set(next); + this.writeSavedIds(this.activeUserId, next); + } + + private readSavedIds(userId: string | null): ReadonlySet | null { + if (!userId || typeof localStorage === 'undefined') { + return null; + } + + try { + const raw = localStorage.getItem(`${SAVED_STORAGE_PREFIX}${userId}`); + + if (raw === null) { + return null; + } + + const parsed = JSON.parse(raw) as unknown; + + if (!Array.isArray(parsed)) { + return null; + } + + return new Set(parsed.filter((value): value is string => typeof value === 'string')); + } catch { + return null; + } + } + + private writeSavedIds(userId: string | null, savedIds: ReadonlySet): void { + if (!userId || typeof localStorage === 'undefined') { + return; + } + + try { + localStorage.setItem(`${SAVED_STORAGE_PREFIX}${userId}`, JSON.stringify([...savedIds])); + } catch { + // localStorage may be unavailable (private mode / quota); membership then + // lives in-memory for this session, which is acceptable. + } } private readUsage(userId: string | null | undefined): ReadonlyMap { diff --git a/toju-app/src/app/infrastructure/persistence/README.md b/toju-app/src/app/infrastructure/persistence/README.md index 6aa535d..83659d6 100644 --- a/toju-app/src/app/infrastructure/persistence/README.md +++ b/toju-app/src/app/infrastructure/persistence/README.md @@ -45,7 +45,7 @@ Both backends store the same entity types: | `users` | `oderId` | | User profiles | | `rooms` | `id` | | Server/room metadata | | `reactions` | `oderId-emoji-messageId` | | Emoji reactions, deduplicated per user | -| `customEmojis` / `custom_emojis` | `id` | `updatedAt`, `creatorUserId` | Known custom emoji image assets synced over peer data channels; `savedByUser` controls picker/library membership | +| `customEmojis` / `custom_emojis` | `id` | `updatedAt`, `creatorUserId` | Known custom emoji image assets synced over peer data channels. Asset store only (the Electron table is shared across local accounts); picker/library membership is **user-bound**, tracked per user id in `localStorage` (`metoyou_custom_emoji_saved:`), not by the row's legacy `savedByUser` flag | | `bans` | `oderId` | | Active bans per room | | `attachments` | `id` | | File/image metadata tied to messages | | `meta` | `key` | | Key-value pairs (e.g. `currentUserId`) |